Attribute CLI-wrapped MCP calls in Codex + re-parse cached sessions (fixes #478)#656
Merged
Merged
Conversation
…ached sessions (#478) Users who run MCP tools through a CLI wrapper (e.g. philschmid/mcp-cli) instead of registering servers natively don't produce Codex mcp_tool_call_end events; Codex logs a plain exec_command. So their MCP usage showed only as a shell command and was absent from the MCP breakdown, even after #513 fixed the native path. - Recognize `mcp-cli [options] call <server> <tool>` in the exec command and also attribute it as mcp__<server>__<tool>. The exec still counts as Bash since it genuinely is a shell exec. Flag-tolerant, quote/path/bash-lc tolerant, and scoped to the mcp-cli binary (not foo-mcp-cli); only the `call` subcommand (a real execution) matches, not info/grep/listing. - Register `codex` in PROVIDER_PARSE_VERSIONS. session-cache.json serves unchanged session files without invoking the provider parser, so the codex-cache version bump alone would never re-attribute already-cached sessions. This also retroactively repairs #513's native-MCP fix for users whose files were cached before it shipped. - Bump CODEX_CACHE_VERSION 4->5 for the provider-internal layer. Tested: parse cases (bash -lc wrapper, flags-before-call, argv array, plus info/grep/foo-mcp-cli negatives), and a mutation-verified cache regression test that fails without the PROVIDER_PARSE_VERSIONS entry. ReDoS-checked. Reviewed with a Fable 5 adversarial pass (Codex is down); its two must-fixes (the cache gap and the flags-before-call miss that would have missed the reporter's own sessions) are folded in. Fixes #478
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #478.
Problem
The reporter runs MCP tools through a CLI wrapper (philschmid/mcp-cli) rather than registering servers natively in Codex. So Codex logs a plain
exec_commandwith nomcp_tool_call_endevent, and the MCP usage showed only as a shell command, absent from the MCP breakdown. #513 fixed the native path; this is the CLI-wrapped follow-up, and the reporter confirmed it was still broken for their setup.Fix
mcp-cli [options] call <server> <tool>in the exec command and also emitmcp__<server>__<tool>, so it flows into the MCP breakdown. The exec still counts as Bash (it genuinely is a shell exec, same call, two breakdowns, no token double-count). Flag-tolerant (mcp-cli -c ./cfg call ...), quote/path/bash -lctolerant, scoped to themcp-clibinary (rejectsfoo-mcp-cli), and only thecallsubcommand matches (info/grep/listing are lookups).codexinPROVIDER_PARSE_VERSIONS.session-cache.jsonserves unchanged files without invoking the provider parser, so a version bump alone would never re-attribute already-cached sessions, the fix would appear to do nothing for existing users. This also retroactively repairs fix(codex): attribute MCP calls emitted as event_msg/mcp_tool_call_end (#478) #513 for anyone whose files were cached before it shipped.CODEX_CACHE_VERSION4→5 for the provider-internal layer.Verification
bash -lcwrapper, flags-before-call, argv-array command, and negatives (info/grep/foo-mcp-cli/ls), asserting exactly thecallinvocations attribute to MCP while all execs stay Bash.PROVIDER_PARSE_VERSIONSentry and passes with it, guarding the load-bearing fix. Same class as the kiro precedent (fix(kiro): Fix Kiro IDE session parsing — context.messages, .kiro-server path, workspace-sessions #618/Kiro IDE provider returns 0 calls — parser doesn't handle current Kiro session format #619).Review
Reviewed with a Fable 5 adversarial pass (Codex CLI is temporarily unavailable). It mutation-verified the cache gap and caught that the reporter likely uses
mcp-cli -c(flags before the subcommand), which the first-draft regex missed entirely, so the fix would have appeared broken for the exact reporting user. Both are folded in.Honest scope note
Validated against synthetic sessions matching the documented mcp-cli command shape (the reporter didn't attach their raw session). Scoped to the mcp-cli binary; other CLI-MCP wrappers would need their own pattern. Substring matching means a command that merely mentions
mcp-cli ... call x y(a comment, an echo) can false-positive, an accepted tradeoff, documented in the code.