fix(snapshot): register filesystem tools through explicit overlay
This commit is contained in:
@@ -6,7 +6,7 @@ Three layers, importable separately:
|
||||
|
||||
- **`runScenario` (harness)** — boots the real agent bin as a subprocess via tsx (unbuilt, Loader path), drives it over ACP JSON-RPC stdio from a deterministic `input.json` script, tees raw stdout for the golden + purity check, and harvests every persisted session JSONL (parent + subagent children, primary-first) after a graceful stdin-EOF shutdown. Parameterized by `AgentUnderTest` (`binScript`, `configPath`, `tsconfigPath` — absolute paths; the subprocess cwd is a temp dir outside the repo).
|
||||
- **Normalizers** — pure functions turning the two captured surfaces into stable text: `normalizeStdout` (JSON-RPC ids → first-seen sequence; UUIDs/cwd → tokens; doubles as the stdout-purity check), `normalizeSessionLog` (times zeroed, `seq` kept), `scrubSystemPrompts` (prompt text → `{{system}}` in every JSONL), and `scrubRequestHeaders` (the remaining header bulk → `{{tools}}`/`{{messagePrefix}}` outside each pin, structure kept — [pinned-header RFC](../../../docs/rfc/implemented/testing/2026-07-06-pin-request-header-content-in-one-scenario.md)).
|
||||
- **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario golden + re-persisted-log compares, record/refresh fixture write-back, the per-header-class pin (`system-prompt.golden.md` plus the JSONL's full tool schemas) with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL prompt-scrubbed, non-pinning fixtures fully header-scrubbed). Must be called at vitest collection time.
|
||||
- **`defineAcpSnapshotSuite` (factory)** — registers the whole describe/it tree for a scenario table: per-scenario golden + re-persisted-log compares, record/refresh fixture write-back, rejection of structured `UNKNOWN_TOOL` results, the per-header-class pin (`system-prompt.golden.md` plus the JSONL's full tool schemas) with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL prompt-scrubbed, non-pinning fixtures fully header-scrubbed). Must be called at vitest collection time.
|
||||
|
||||
A consuming `*.snapshot.ts` is the scenario table plus one factory call:
|
||||
|
||||
@@ -35,7 +35,7 @@ defineAcpSnapshotSuite({
|
||||
})
|
||||
```
|
||||
|
||||
A scenario booting a differently-composed tree sets its own `configPath` (an overlay whose basename still ends in `cordis.yml`, so the bin's replay swap finds the sibling `*cordis.snapshot.yml`) and, when that composition changes the request header, its own `headerClass` with its own pinning scenario — the acp-agent example's Code Mode scenarios are the template. Each pinning directory's generated `system-prompt.golden.md` is the reviewable snapshot of the normalized composed prompt; `session.jsonl` stores `"system":"{{system}}"` while retaining the complete tool list.
|
||||
A scenario booting a differently-composed tree sets its own `configPath` (an overlay whose basename still ends in `cordis.yml`, so the bin's replay swap finds the sibling `*cordis.snapshot.yml`) and, when that composition changes the request header, its own `headerClass` with its own pinning scenario — the acp-agent example's Code Mode and filesystem scenarios are templates. Each pinning directory's generated `system-prompt.golden.md` is the reviewable snapshot of the normalized composed prompt; `session.jsonl` stores `"system":"{{system}}"` while retaining the complete tool list.
|
||||
|
||||
Examples use a `cordis.snapshot.yml` overlay with [`dsh-llm-replay`](../llm-replay/README.md). Recording calls the live model and updates model fixtures; keyless refresh replays those fixtures and updates derived stdout, session-log, and prompt snapshots. See the [snapshot RFC](../../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md).
|
||||
|
||||
|
||||
@@ -274,6 +274,27 @@ function parseJsonlRecords(text: string): Record<string, unknown>[] {
|
||||
.map(line => JSON.parse(line) as Record<string, unknown>)
|
||||
}
|
||||
|
||||
/**
|
||||
* Find tool calls whose structured result reports `UNKNOWN_TOOL`.
|
||||
*
|
||||
* Snapshot refresh must not turn a missing registration into accepted behavior;
|
||||
* intentional unknown-tool behavior belongs in a focused unit or e2e test.
|
||||
*
|
||||
* @param rawLog The session JSONL to inspect.
|
||||
* @returns The failing call ids in log order, using a diagnostic placeholder when absent.
|
||||
*/
|
||||
export function unknownToolCallIds(rawLog: string): string[] {
|
||||
return parseJsonlRecords(rawLog).flatMap((record) => {
|
||||
if (record.type !== 'tool/result') return []
|
||||
const data = record.data
|
||||
if (data === null || typeof data !== 'object') return []
|
||||
const { callId, error } = data as { callId?: unknown; error?: unknown }
|
||||
if (error === null || typeof error !== 'object') return []
|
||||
if ((error as { code?: unknown }).code !== 'UNKNOWN_TOOL') return []
|
||||
return [typeof callId === 'string' ? callId : '<missing callId>']
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the cross-log id/cwd replacements used by refresh write-back.
|
||||
*
|
||||
@@ -401,6 +422,11 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void {
|
||||
...scenario.configPath !== undefined ? { configPath: scenario.configPath } : {},
|
||||
})
|
||||
|
||||
for (const log of result.sessionLogs) {
|
||||
expect(unknownToolCallIds(log.content), `session ${log.id}: snapshot scenarios must not accept UNKNOWN_TOOL`)
|
||||
.toEqual([])
|
||||
}
|
||||
|
||||
// Scrub every volatile id the run produced: the ACP server-issued session id plus every
|
||||
// harvested log's recorded id (a subagent child id never surfaces over ACP, but it
|
||||
// appears in the child's own log header).
|
||||
@@ -598,5 +624,20 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void {
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('no committed session fixture accepts UNKNOWN_TOOL', async () => {
|
||||
for (const scenario of scenarios) {
|
||||
const dir = join(snapshotsDir, scenario.name)
|
||||
const files = [
|
||||
'session.jsonl',
|
||||
...Array.from({ length: scenario.childSessions ?? 0 }, (_, i) => `session.${i + 1}.jsonl`),
|
||||
]
|
||||
for (const file of files) {
|
||||
const fixture = await readFile(join(dir, file), 'utf8')
|
||||
expect(unknownToolCallIds(fixture), `${scenario.name}/${file} contains UNKNOWN_TOOL`)
|
||||
.toEqual([])
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
normalizedSystemPrompts,
|
||||
refreshFixtureReplacements,
|
||||
stabilizeRefreshLog,
|
||||
unknownToolCallIds,
|
||||
} from '../src/suite.ts'
|
||||
|
||||
/**
|
||||
@@ -278,6 +279,27 @@ describe('headerDeltaCount', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('unknownToolCallIds', () => {
|
||||
it('returns structured UNKNOWN_TOOL call ids and ignores other results', () => {
|
||||
const log = [
|
||||
'{"type":"tool/result","data":{"callId":"missing","error":{"code":"UNKNOWN_TOOL"}}}',
|
||||
'{"type":"tool/result","data":{"callId":"failed","error":{"code":"EXECUTION_FAILED"}}}',
|
||||
'{"type":"tool/result","data":null}',
|
||||
'{"type":"tool/result","data":"invalid"}',
|
||||
'{"type":"tool/result","data":{"error":null}}',
|
||||
'{"type":"tool/result","data":{"error":"invalid"}}',
|
||||
'{"type":"assistant/message","data":{"error":{"code":"UNKNOWN_TOOL"}}}',
|
||||
'{"type":"tool/result","data":{"error":{"code":"UNKNOWN_TOOL"}}}',
|
||||
'',
|
||||
].join('\n')
|
||||
expect(unknownToolCallIds(log)).toEqual(['missing', '<missing callId>'])
|
||||
})
|
||||
|
||||
it('returns no failures for ordinary tool results', () => {
|
||||
expect(unknownToolCallIds('{"type":"tool/result","data":{"callId":"ok"}}\n')).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('refreshFixtureReplacements', () => {
|
||||
it('maps fresh ids and cwd values to the existing fixture values, skipping non-replacements', () => {
|
||||
const log = (content: string): HarvestedLog => ({ id: 'diagnostic', createdAt: 1, content })
|
||||
|
||||
Reference in New Issue
Block a user