mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-08-17 12:34:26 +00:00
Merge pull request #656 from getagentseal/fix/codex-cli-mcp-attribution
Attribute CLI-wrapped MCP calls in Codex + re-parse cached sessions (fixes #478)
This commit is contained in:
commit
b78e862f45
5 changed files with 203 additions and 1 deletions
|
|
@ -8,7 +8,10 @@ import type { ParsedProviderCall } from './providers/types.js'
|
|||
|
||||
// v4: attribute MCP calls emitted as event_msg/mcp_tool_call_end (issue #478).
|
||||
// Recent Codex sessions cached under v3 dropped these, so force a re-parse.
|
||||
const CODEX_CACHE_VERSION = 4
|
||||
// v5: also attribute CLI-wrapped MCP calls (`mcp-cli call server tool`) that
|
||||
// Codex logs as a plain exec_command (issue #478 follow-up). Force a re-parse
|
||||
// so sessions cached under v4 pick up the CLI-MCP attribution.
|
||||
const CODEX_CACHE_VERSION = 5
|
||||
const CACHE_FILE = 'codex-results.json'
|
||||
|
||||
type FileFingerprint = { mtimeMs: number; sizeBytes: number }
|
||||
|
|
|
|||
|
|
@ -41,6 +41,40 @@ const toolNameMap: Record<string, string> = {
|
|||
read_dir: 'Glob',
|
||||
}
|
||||
|
||||
// CLI-based MCP wrappers (e.g. philschmid/mcp-cli) let Codex call an MCP tool
|
||||
// through a shell command instead of registering the server natively. Codex
|
||||
// then logs a plain exec_command with no `mcp_tool_call_end` event, so the MCP
|
||||
// usage would only appear as a shell command and be absent from the MCP
|
||||
// breakdown (issue #478). Recognize the `mcp-cli [options] call <server>
|
||||
// <tool>` form and return the canonical mcp__<server>__<tool> so the call is
|
||||
// also attributed to MCP. Only the `call` subcommand (an actual tool execution)
|
||||
// is matched; info / grep / bare listing are lookups. The exec_command still
|
||||
// counts as Bash since it genuinely is a shell exec. Scoped to the mcp-cli
|
||||
// binary; other wrappers would need their own pattern.
|
||||
//
|
||||
// The negative lookbehind keeps `mcp-cli` a standalone binary (a leading
|
||||
// quote/space/slash from a `bash -lc "..."` wrapper or absolute path is fine,
|
||||
// but `foo-mcp-cli` is not). `(?:\s+(?!call\b)[^\s;|&]+)*` skips any options and
|
||||
// their values between the binary and the subcommand (e.g.
|
||||
// `mcp-cli -c ./mcp.json call ...`) without crossing a shell separator, and
|
||||
// stops at the `call` token. This is substring matching, so a command that
|
||||
// merely mentions the phrase (a comment, an echo, a commit message) can
|
||||
// false-positive, an accepted tradeoff for the common case. \s+ and the token
|
||||
// class don't overlap, so there is no catastrophic backtracking.
|
||||
const MCP_CLI_CALL = /(?<![\w.-])mcp-cli(?:\s+(?!call\b)[^\s;|&]+)*\s+call\s+(\S+)\s+(\S+)/
|
||||
function mcpToolFromShellCommand(command: unknown): string | null {
|
||||
const text = typeof command === 'string'
|
||||
? command
|
||||
: Array.isArray(command) ? command.filter(x => typeof x === 'string').join(' ') : ''
|
||||
if (!text) return null
|
||||
const m = MCP_CLI_CALL.exec(text)
|
||||
if (!m) return null
|
||||
const server = m[1]!.replace(/['"]/g, '')
|
||||
const tool = m[2]!.replace(/['"]/g, '')
|
||||
if (!server || !tool) return null
|
||||
return `mcp__${server}__${tool}`
|
||||
}
|
||||
|
||||
type CodexEntry = {
|
||||
type: string
|
||||
timestamp?: string
|
||||
|
|
@ -386,6 +420,13 @@ function createParser(source: SessionSource, seenKeys: Set<string>): SessionPars
|
|||
if (typeof fp === 'string') call.file = fp
|
||||
const cmd = args['command'] ?? args['cmd']
|
||||
if (typeof cmd === 'string') call.command = cmd
|
||||
// Attribute a CLI-wrapped MCP call (e.g. `mcp-cli call server tool`)
|
||||
// to the MCP breakdown too; the exec still counts as Bash above.
|
||||
const mcpTool = mcpToolFromShellCommand(cmd)
|
||||
if (mcpTool) {
|
||||
pendingTools.push(mcpTool)
|
||||
pendingToolSequence.push([{ tool: mcpTool }])
|
||||
}
|
||||
}
|
||||
pendingToolSequence.push([call])
|
||||
continue
|
||||
|
|
|
|||
|
|
@ -111,6 +111,11 @@ export const DURABLE_PROVIDER_NAMES: ReadonlySet<string> = new Set(['copilot'])
|
|||
const PROVIDER_PARSE_VERSIONS: Record<string, string> = {
|
||||
claude: 'cowork-space-grouping-v1',
|
||||
cline: 'worktree-project-grouping-v1',
|
||||
// Bump when the Codex parser changes attribution so unchanged, already-cached
|
||||
// session files re-parse (session-cache.json serves them without invoking the
|
||||
// provider parser otherwise). Covers native mcp_tool_call_end (#513) and
|
||||
// CLI-wrapped `mcp-cli call` (#478) MCP attribution.
|
||||
codex: 'mcp-attribution-v2',
|
||||
cursor: 'composer-anchored-crediting-v1',
|
||||
'cursor-agent': 'workspaceless-transcript-v1',
|
||||
copilot: 'otel-durable-v1',
|
||||
|
|
|
|||
108
tests/codex-cache-invalidation.test.ts
Normal file
108
tests/codex-cache-invalidation.test.ts
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
// Regression for the codex stale-cache path (#478 follow-up, same class as
|
||||
// the kiro bug #618/#619). session-cache.json serves unchanged session files
|
||||
// without invoking the provider parser, so bumping CODEX_CACHE_VERSION alone
|
||||
// does NOT re-attribute already-cached sessions. Registering `codex` in
|
||||
// PROVIDER_PARSE_VERSIONS changes the provider envFingerprint, which discards
|
||||
// the stale section and forces a re-parse. This exercises the full
|
||||
// parseAllSessions pipeline against a cache seeded with the PRE-fix
|
||||
// fingerprint and asserts the mcp-cli MCP attribution is recovered.
|
||||
|
||||
import { describe, it, expect, beforeEach, afterAll, vi } from 'vitest'
|
||||
import { mkdir, rm, readFile, writeFile } from 'fs/promises'
|
||||
import { createHash } from 'crypto'
|
||||
import { join } from 'path'
|
||||
|
||||
import { clearSessionCache, parseAllSessions } from '../src/parser.js'
|
||||
|
||||
const testRoot = vi.hoisted(() => {
|
||||
const root = `${process.env['TMPDIR'] || '/tmp'}/codex-stale-repro-${process.pid}-${Date.now()}`
|
||||
process.env['HOME'] = `${root}/home`
|
||||
process.env['USERPROFILE'] = `${root}/home`
|
||||
process.env['CODEX_HOME'] = `${root}/codex`
|
||||
return root
|
||||
})
|
||||
|
||||
const CODEX_HOME = join(testRoot, 'codex')
|
||||
const CACHE_DIR = join(testRoot, 'cache')
|
||||
|
||||
// computeEnvFingerprint('codex') as staged (no PROVIDER_PARSE_VERSIONS entry):
|
||||
// hash of just CODEX_HOME. This is what sits in every existing user cache.
|
||||
function preFixFingerprint(): string {
|
||||
return createHash('sha256').update(`CODEX_HOME=${CODEX_HOME}`).digest('hex').slice(0, 16)
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
process.env['HOME'] = join(testRoot, 'home')
|
||||
process.env['USERPROFILE'] = join(testRoot, 'home')
|
||||
process.env['CODEX_HOME'] = CODEX_HOME
|
||||
process.env['CODEBURN_CACHE_DIR'] = CACHE_DIR
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
await rm(testRoot, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
function allMcpServers(projects: Awaited<ReturnType<typeof parseAllSessions>>): string[] {
|
||||
const servers: string[] = []
|
||||
for (const p of projects) {
|
||||
for (const s of p.sessions) {
|
||||
servers.push(...Object.keys(s.mcpBreakdown))
|
||||
}
|
||||
}
|
||||
return servers
|
||||
}
|
||||
|
||||
describe('codex parser change invalidates stale session-cache (#478/#513)', () => {
|
||||
it('re-parses unchanged codex files after a parser attribution change', async () => {
|
||||
const sessionDir = join(CODEX_HOME, 'sessions', '2026', '04', '14')
|
||||
await mkdir(sessionDir, { recursive: true })
|
||||
await mkdir(CACHE_DIR, { recursive: true })
|
||||
const lines = [
|
||||
JSON.stringify({ type: 'session_meta', timestamp: '2026-04-14T10:00:00Z', payload: { session_id: 'sess-stale', model: 'gpt-5.5', cwd: '/Users/test/proj', originator: 'codex_cli_rs' } }),
|
||||
JSON.stringify({ type: 'response_item', timestamp: '2026-04-14T10:00:10Z', payload: { type: 'message', role: 'user', content: [{ type: 'input_text', text: 'call mcp via cli' }] } }),
|
||||
JSON.stringify({ type: 'response_item', timestamp: '2026-04-14T10:00:30Z', payload: { type: 'function_call', name: 'exec_command', arguments: JSON.stringify({ command: "bash -lc \"mcp-cli call github get_issue '{}'\"" }) } }),
|
||||
JSON.stringify({ type: 'event_msg', timestamp: '2026-04-14T10:01:00Z', payload: { type: 'token_count', info: { last_token_usage: { input_tokens: 300, output_tokens: 100 }, total_token_usage: { total_tokens: 400 } } } }),
|
||||
]
|
||||
await writeFile(join(sessionDir, 'rollout-stale.jsonl'), lines.join('\n') + '\n')
|
||||
|
||||
// Run 1: cold cache, fixed parser. MCP attribution present (sanity).
|
||||
clearSessionCache()
|
||||
const fresh = await parseAllSessions(undefined, 'codex')
|
||||
expect(allMcpServers(fresh)).toContain('github')
|
||||
|
||||
// Simulate a user whose session-cache.json was written by the PRE-fix
|
||||
// release: pre-fix envFingerprint, unchanged file fingerprint, cached
|
||||
// turns lack the mcp__ tool. Also reset codex-results.json to v4 so the
|
||||
// provider (if it runs at all) must genuinely re-parse.
|
||||
const cachePath = join(CACHE_DIR, 'session-cache.json')
|
||||
const cache = JSON.parse(await readFile(cachePath, 'utf8'))
|
||||
cache.providers.codex.envFingerprint = preFixFingerprint()
|
||||
for (const f of Object.values(cache.providers.codex.files) as any[]) {
|
||||
for (const turn of f.turns) {
|
||||
for (const call of turn.calls) {
|
||||
call.tools = call.tools.filter((t: string) => !t.startsWith('mcp__'))
|
||||
if (call.toolSequence) {
|
||||
call.toolSequence = call.toolSequence.filter((step: any[]) => !step.some(c => c.tool.startsWith('mcp__')))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
await writeFile(cachePath, JSON.stringify(cache))
|
||||
const codexCachePath = join(CACHE_DIR, 'codex-results.json')
|
||||
const codexCache = JSON.parse(await readFile(codexCachePath, 'utf8'))
|
||||
codexCache.version = 4
|
||||
for (const f of Object.values(codexCache.files) as any[]) {
|
||||
for (const call of f.calls ?? []) {
|
||||
call.tools = (call.tools ?? []).filter((t: string) => !t.startsWith('mcp__'))
|
||||
}
|
||||
}
|
||||
await writeFile(codexCachePath, JSON.stringify(codexCache))
|
||||
|
||||
clearSessionCache()
|
||||
const second = await parseAllSessions(undefined, 'codex')
|
||||
// FIXED: `codex` is now in PROVIDER_PARSE_VERSIONS, so the pre-fix
|
||||
// envFingerprint no longer matches, the stale section is discarded, the
|
||||
// unchanged file re-parses, and the mcp-cli attribution reappears.
|
||||
expect(allMcpServers(second)).toContain('github')
|
||||
})
|
||||
})
|
||||
|
|
@ -332,6 +332,51 @@ describe('codex provider - JSONL parsing', () => {
|
|||
expect(calls[0]!.tools).toEqual(['mcp__github__get_issue'])
|
||||
})
|
||||
|
||||
it('attributes CLI-wrapped MCP calls (mcp-cli call server tool) to MCP + Bash', async () => {
|
||||
const execStr = (command: string) => JSON.stringify({
|
||||
type: 'response_item',
|
||||
timestamp: '2026-04-14T10:00:30Z',
|
||||
payload: { type: 'function_call', name: 'exec_command', arguments: JSON.stringify({ command }) },
|
||||
})
|
||||
// command as an array (Codex sometimes logs argv form).
|
||||
const execArr = (command: string[]) => JSON.stringify({
|
||||
type: 'response_item',
|
||||
timestamp: '2026-04-14T10:00:30Z',
|
||||
payload: { type: 'function_call', name: 'exec_command', arguments: JSON.stringify({ command }) },
|
||||
})
|
||||
const filePath = await writeSession(tmpDir, '2026-04-14', 'rollout-mcpcli.jsonl', [
|
||||
sessionMeta({ session_id: 'sess-mcpcli', model: 'gpt-5.5' }),
|
||||
userMessage('look up an issue via the MCP CLI'),
|
||||
// Real invocation forms that MUST attribute to MCP:
|
||||
execStr("bash -lc \"mcp-cli call github get_issue '{\\\"id\\\": 5}'\""), // bash -lc wrapper
|
||||
execStr('mcp-cli -c ./mcp.json call linear list_issues'), // flags before subcommand
|
||||
execArr(['mcp-cli', 'call', 'slack', 'post_message', '{}']), // argv array form
|
||||
// Lookups and unrelated commands that must NOT attribute:
|
||||
execStr('mcp-cli info github'),
|
||||
execStr('mcp-cli grep "*issue*"'),
|
||||
execStr('my-mcp-cli-wrapper call github get_issue'), // not the mcp-cli binary
|
||||
execStr('ls -la'),
|
||||
tokenCount({ timestamp: '2026-04-14T10:01:00Z', last: { input: 300, output: 100 }, total: { total: 400 } }),
|
||||
])
|
||||
|
||||
const provider = createCodexProvider(tmpDir)
|
||||
const source = { path: filePath, project: 'test', provider: 'codex' }
|
||||
const parser = provider.createSessionParser(source, new Set())
|
||||
const calls: ParsedProviderCall[] = []
|
||||
for await (const call of parser.parse()) calls.push(call)
|
||||
|
||||
expect(calls).toHaveLength(1)
|
||||
const tools = calls[0]!.tools
|
||||
// Every exec still counts as Bash (7 exec_commands total).
|
||||
expect(tools.filter(t => t === 'Bash')).toHaveLength(7)
|
||||
// Exactly the three `call` invocations attribute to MCP; info/grep/wrapper/ls do not.
|
||||
expect(tools.filter(t => t.startsWith('mcp__')).sort()).toEqual([
|
||||
'mcp__github__get_issue',
|
||||
'mcp__linear__list_issues',
|
||||
'mcp__slack__post_message',
|
||||
])
|
||||
})
|
||||
|
||||
it('normalizes Codex subagent tool calls to Agent', async () => {
|
||||
const filePath = await writeSession(tmpDir, '2026-04-14', 'rollout-agent.jsonl', [
|
||||
sessionMeta({ session_id: 'sess-agent', model: 'gpt-5.5' }),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue