Repository navigation
fix(dogfood): record candidate argv independently of the execution adapter #2848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -483,6 +483,12 @@ const STAGE_EXPECTATIONS: Record<(typeof REQUIRED_STAGE_IDS)[number], StageExpec | |
| }, | ||
| }; | ||
|
|
||
| /** The real spawned vector: the adapter's argv when one wrapped the candidate. */ | ||
| function spawnArgv(command: JsonRecord): unknown[] { | ||
| if (Array.isArray(command.adapterArgv)) return command.adapterArgv; | ||
| return Array.isArray(command.argv) ? command.argv : []; | ||
| } | ||
|
|
||
| function validateCapturedCommand( | ||
| errors: string[], | ||
| label: string, | ||
|
|
@@ -506,6 +512,15 @@ function validateCapturedCommand( | |
| if (!Array.isArray(command.argv) || command.argv.some((arg) => typeof arg !== 'string')) { | ||
| errors.push(`${label}.argv must be a string array`); | ||
| } | ||
| // Optional: present only when an execution adapter (linux-x64-musl Alpine) | ||
| // spawned the candidate. `argv` stays the candidate's own arguments on every | ||
| // platform; `adapterArgv` records the real vector handed to `executable`. | ||
| if ( | ||
| command.adapterArgv !== undefined && | ||
| (!Array.isArray(command.adapterArgv) || command.adapterArgv.some((arg) => typeof arg !== 'string')) | ||
| ) { | ||
| errors.push(`${label}.adapterArgv must be a string array`); | ||
| } | ||
| if (typeof command.pid !== 'number' || !Number.isInteger(command.pid) || command.pid <= 0) { | ||
| errors.push(`${label}.pid must be a positive integer`); | ||
| } | ||
|
|
@@ -518,7 +533,7 @@ function validateCapturedCommand( | |
| if (typeof command.stderr !== 'string') errors.push(`${label}.stderr must be a string`); | ||
| return { | ||
| record: command, | ||
| summary: [String(command.executable), ...(Array.isArray(command.argv) ? command.argv.map(String) : [])].join(' '), | ||
| summary: [String(command.executable), ...spawnArgv(command).map(String)].join(' '), | ||
| exit: typeof command.exit === 'number' && Number.isInteger(command.exit) ? command.exit : null, | ||
| candidateSha256: command.candidateBinarySha256, | ||
| }; | ||
|
|
@@ -544,7 +559,8 @@ function validateStandaloneObservation( | |
| const hasBoard = argv.some((args) => Array.isArray(args) && args.join(' ') === 'board --json'); | ||
| if (!hasTask || !hasBoard) | ||
| errors.push(`${label} must contain standalone task list --json and board --json observations`); | ||
| if (argv.some((args) => Array.isArray(args) && args.some((arg) => String(arg).toLowerCase().includes('mcp')))) { | ||
| const vectors = commands.flatMap((value) => [value.record.argv, value.record.adapterArgv]); | ||
| if (vectors.some((args) => Array.isArray(args) && args.some((arg) => String(arg).toLowerCase().includes('mcp')))) { | ||
|
Comment on lines
+562
to
+563
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On adapter-backed runs, AGENTS.md reference: AGENTS.md:L55-L57 Useful? React with 👍 / 👎. |
||
| errors.push(`${label} must reject retired MCP evidence`); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import { afterEach, describe, expect, test } from 'bun:test'; | ||
| import { chmodSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { join } from 'node:path'; | ||
| import { captureCommand, resolveInvocation } from './codex-dogfood-harness.ts'; | ||
|
|
||
| const roots: string[] = []; | ||
|
|
||
| function fixture(): { root: string; binary: string; adapter: string } { | ||
| const root = realpathSync(mkdtempSync(join(tmpdir(), 'dogfood-harness-'))); | ||
| roots.push(root); | ||
| // Stands in for the authenticated candidate: it echoes the exact argv it was | ||
| // handed, so the evidence record can be checked against reality. | ||
| const binary = join(root, 'genie'); | ||
| writeFileSync(binary, '#!/usr/bin/env bash\nprintf \'%s\\n\' "$*"\n'); | ||
| chmodSync(binary, 0o755); | ||
| // Stands in for scripts/run-musl-dogfood.sh: takes the candidate binary as | ||
| // its first positional argument and execs it with the remaining argv. | ||
| const adapter = join(root, 'adapter.sh'); | ||
| writeFileSync(adapter, '#!/usr/bin/env bash\nset -eu\ncandidate=$1\nshift\nexec "$candidate" "$@"\n'); | ||
| chmodSync(adapter, 0o755); | ||
| return { root, binary, adapter }; | ||
| } | ||
|
|
||
| afterEach(() => { | ||
| while (roots.length > 0) rmSync(roots.pop() as string, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| describe('candidate invocation evidence', () => { | ||
| test('resolveInvocation keeps argv on the candidate and names the adapter vector separately', () => { | ||
| expect(resolveInvocation({ binary: '/bin/genie', args: ['board', '--json'] })).toEqual({ | ||
| command: '/bin/genie', | ||
| argv: ['board', '--json'], | ||
| spawnArgv: ['board', '--json'], | ||
| }); | ||
| expect( | ||
| resolveInvocation({ binary: '/bin/genie', args: ['board', '--json'], executionAdapter: '/bin/adapter' }), | ||
| ).toEqual({ | ||
| command: '/bin/adapter', | ||
| argv: ['board', '--json'], | ||
| adapterArgv: ['/bin/genie', 'board', '--json'], | ||
| spawnArgv: ['/bin/genie', 'board', '--json'], | ||
| }); | ||
| }); | ||
|
|
||
| test('records the candidate argv unchanged with and without an execution adapter', () => { | ||
| const { root, binary, adapter } = fixture(); | ||
| const args = ['task', 'list', '--json']; | ||
| const direct = captureCommand({ root, binary, args, cwd: root, env: { PATH: process.env.PATH ?? '' } }); | ||
| expect(direct.exit).toBe(0); | ||
| expect(direct.executable).toBe(binary); | ||
| expect(direct.candidateBinary).toBe(binary); | ||
| expect(direct.argv).toEqual(args); | ||
| expect(direct.adapterArgv).toBeUndefined(); | ||
| expect(direct.stdout.trim()).toBe('task list --json'); | ||
|
|
||
| const adapted = captureCommand({ | ||
| root, | ||
| binary, | ||
| args, | ||
| cwd: root, | ||
| env: { PATH: process.env.PATH ?? '' }, | ||
| executionAdapter: adapter, | ||
| }); | ||
| expect(adapted.exit).toBe(0); | ||
| // The evidence argv is the candidate's own arguments on BOTH legs: a | ||
| // linux-x64-musl run must be byte-comparable with every other platform. | ||
| expect(adapted.argv).toEqual(args); | ||
| expect(adapted.adapterArgv).toEqual([binary, ...args]); | ||
| expect(adapted.executable).toBe(adapter); | ||
| expect(adapted.candidateBinary).toBe(binary); | ||
| // The candidate really did receive exactly `args` through the adapter. | ||
| expect(adapted.stdout.trim()).toBe('task list --json'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For adapter-backed evidence, this only verifies that
adapterArgvis a string array; it never requires it to equal[candidateBinary, ...argv]. Consequently, evidence can claim standalonetask list --jsonandboard --jsoninargvwhile the recorded spawn vector invokes different arguments or even a different binary, and the release validator still accepts the supposed standalone proof. Enforce the documented relationship before usingargvas candidate-execution evidence.AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.