From e481288a3a5fdf05f4bcc491dc120820975710d3 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:04:11 +0800 Subject: [PATCH 1/3] refactor: derive snapshot session fixtures from disk --- .../2026-07-08-shared-acp-snapshot-package.md | 2 +- .../2026-06-20-discover-package-inventory.md | 5 +- examples/acp-agent/tests/acp.snapshot.ts | 10 +-- packages/support/acp-snapshot/README.md | 2 +- packages/support/acp-snapshot/src/suite.ts | 68 +++++++++++-------- .../support/acp-snapshot/tests/suite.spec.ts | 44 +++++++++--- 6 files changed, 85 insertions(+), 46 deletions(-) diff --git a/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md b/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md index c622a7f1be..eca3661b33 100644 --- a/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md +++ b/docs/rfc/implemented/testing/2026-07-08-shared-acp-snapshot-package.md @@ -16,7 +16,7 @@ The machinery lives in [`packages/support/acp-snapshot`](../../../../packages/su **`src/normalize.ts`** — the pure normalizers, hook-free by policy: when a future event carries a new volatile field (an approval duration, say), the shared normalizer learns it in the same change, keeping one home for what "normalized" means rather than per-suite scrub extensions. -**`src/suite.ts`** — the `Scenario` type and `defineAcpSnapshotSuite(options)`, registering the per-scenario compares, record/refresh fixture write-back, the header pin with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL a `scrubSystemPrompts` fixed point, non-pinning fixtures also `scrubRequestHeaders` fixed points). The pinned-header contract ([pinned-header RFC](2026-07-06-pin-request-header-content-in-one-scenario.md)) is per-suite: each header class flags exactly one `pinsHeader` scenario, whose `system-prompt.golden.md` and JSONL tool list split the composed header into reviewable artifacts; the uniformity guard compares both against every live header in that class. A pinning scenario declares any legitimate changed-header count, and its Markdown artifact records every full changed prompt. The pure helpers (`childFixturePaths`, `fixtureContext`, `normalizedHeaders`, `normalizedSystemPrompts`, `formatSystemPromptSnapshot`, `headerChangeCount`) are exported from the module for direct unit coverage. +**`src/suite.ts`** — the `Scenario` type and `defineAcpSnapshotSuite(options)`, registering the per-scenario compares, record/refresh fixture write-back, the header pin with its live uniformity guard, and the fixture guard block (no orphan scenario dirs, required files present, exactly one pin per class, every JSONL a `scrubSystemPrompts` fixed point, non-pinning fixtures also `scrubRequestHeaders` fixed points). A scenario directory's `session.jsonl` plus contiguous `session..jsonl` siblings are its ordered primary/child inventory, so the scenario table declares policy without duplicating a child count. The pinned-header contract ([pinned-header RFC](2026-07-06-pin-request-header-content-in-one-scenario.md)) is per-suite: each header class flags exactly one `pinsHeader` scenario, whose `system-prompt.golden.md` and JSONL tool list split the composed header into reviewable artifacts; the uniformity guard compares both against every live header in that class. A pinning scenario declares any legitimate changed-header count, and its Markdown artifact records every full changed prompt. The pure helpers (`sessionFixtureNames`, `fixtureContext`, `normalizedHeaders`, `normalizedSystemPrompts`, `formatSystemPromptSnapshot`, `headerChangeCount`) are exported from the module for direct unit coverage. ## Alternatives considered diff --git a/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md index 3587393efe..a091203da9 100644 --- a/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` and the root `tsconfig.json` each hand-list every package as explicit project `references` — two identical sets that grow in lockstep, so a single generator can emit both — and `tsconfig.base.json`'s paths map hand-lists the per-group glob fan-out. `knip.json` restates a per-package `entry` stanza for each package that gains an `*.e2e.ts` suite — byte-identical overrides that exist only because the shared `packages/*/*` stanza omits the e2e glob (an entry glob matching no files is inert, so the default stanza could carry it for every package). The ACP snapshot suite's scenario table (`examples/acp-agent/tests/acp.snapshot.ts`) hand-maintains a `childSessions` count per scenario that duplicates the number of `session..jsonl` fixture siblings on disk. These lists are small today, but every new package or scenario class creates another manual synchronization point. +Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` and the root `tsconfig.json` each hand-list every package as explicit project `references` — two identical sets that grow in lockstep, so a single generator can emit both — and `tsconfig.base.json`'s paths map hand-lists the per-group glob fan-out. `knip.json` restates a per-package `entry` stanza for each package that gains an `*.e2e.ts` suite — byte-identical overrides that exist only because the shared `packages/*/*` stanza omits the e2e glob (an entry glob matching no files is inert, so the default stanza could carry it for every package). These lists are small today, but every new package creates another manual synchronization point. The [package hierarchy](../../implemented/architecture/2026-06-20-package-hierarchy.md) already removed several of these by hand: `scripts/publint-all.ts` now derives its list from the `packages//` layout, and the two `tsconfig` `paths` maps collapsed to one `@deepseek-ai/dsh-*` wildcard. What remains is the inventory that cannot be globbed away — chiefly `tsconfig.build.json`'s project `references`, which TypeScript requires as an explicit array (no wildcard form). @@ -16,7 +16,7 @@ Make the remaining package/gate inventories discoverable. A single canonical sou The hierarchy does not need to encode every fact about a package, but it should encode the broad maintenance policy: core/product packages, integrations, capability seams, and support/test/example packages should not all require a hand-maintained exception list before scripts can tell them apart. -Two of the cataloged items need no generator at all: folding the e2e entry glob into knip's default stanza deletes the per-package restatements outright, and `childSessions` can be discovered from each scenario's fixture directory, leaving the scenario table to declare only policy (`recorded`, `hasModelTurn`, `comparesLog`) — and even those track fixture-derivable facts today (`comparesLog` ⟺ the committed log has entries beyond its header line; `recorded` ⟺ `hasModelTurn` with no `replay.override.json` sibling), so each new scenario class keeps adding knobs the fixture directory already answers. +One cataloged item needs no generator at all: folding the e2e entry glob into knip's default stanza deletes the per-package restatements outright. ## Acceptance criteria @@ -25,7 +25,6 @@ Two of the cataloged items need no generator at all: folding the e2e entry glob - Docs describe the source of truth rather than repeating generated inventories. - CI invokes the aggregate commands and lets those commands own their sub-gate lists. - `knip.json` carries a per-package override only where it encodes real information (an extra entry file, an ignored dependency), never a restatement of the default stanza. -- Snapshot scenarios declare policy, not facts discoverable from their fixture directories. ## Risks diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index eaa4e0381a..b1a43b83db 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -67,14 +67,14 @@ const SCENARIOS: Scenario[] = [ // tiers (gentle at 3, detailed at 5) as context/message in transcript and log. { name: 'repeat-tool-guard', hasModelTurn: true, recorded: false }, { name: 'cancel', hasModelTurn: true, recorded: false, overridden: true }, - { name: 'subagent-spawn', hasModelTurn: true, recorded: true, childSessions: 1 }, - { name: 'subagent-multi', hasModelTurn: true, recorded: true, childSessions: 2 }, - { name: 'subagent-fork', hasModelTurn: true, recorded: true, childSessions: 1 }, - { name: 'subagent-mixed', hasModelTurn: true, recorded: true, childSessions: 2 }, + { name: 'subagent-spawn', hasModelTurn: true, recorded: true }, + { name: 'subagent-multi', hasModelTurn: true, recorded: true }, + { name: 'subagent-fork', hasModelTurn: true, recorded: true }, + { name: 'subagent-mixed', hasModelTurn: true, recorded: true }, // The workflow tool: the model writes a one-child orchestration script; the // child runs as a spawn subagent under the worker-thread engine (its session is the // child fixture), and the tool result carries the script's return value. - { name: 'workflow-run', hasModelTurn: true, recorded: true, childSessions: 1 }, + { name: 'workflow-run', hasModelTurn: true, recorded: true }, // Hook matrix — one scenario per hook point × its headline Decision outcome, // across BOTH bridges (Claude `hooks.json`, Codex `codex-hooks.json`, seeded in // workspace/). The block scenarios need no model call: a UserPromptSubmit hook diff --git a/packages/support/acp-snapshot/README.md b/packages/support/acp-snapshot/README.md index 13dd1ab247..ac9c0ceee3 100644 --- a/packages/support/acp-snapshot/README.md +++ b/packages/support/acp-snapshot/README.md @@ -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, 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). Each scenario directory's `session.jsonl` plus contiguous `session..jsonl` siblings are the ordered primary/child inventory; the scenario table does not duplicate their count. Must be called at vitest collection time. A consuming `*.snapshot.ts` is the scenario table plus one factory call: diff --git a/packages/support/acp-snapshot/src/suite.ts b/packages/support/acp-snapshot/src/suite.ts index 523092bf09..6311a15031 100644 --- a/packages/support/acp-snapshot/src/suite.ts +++ b/packages/support/acp-snapshot/src/suite.ts @@ -80,14 +80,6 @@ export interface Scenario { * false (replay derives from the fixture's `assistant/chunk` events). */ overridden?: boolean - /** - * How many SUBAGENT child sessions this scenario records beyond the top-level - * one (0 for a single-session scenario). Each child rides in a sibling fixture - * `session..jsonl` (1-based); replay forwards them to `dsh-llm-replay` so - * each child session replays from its own script, and record mode writes the - * harvested child logs back to those files. Defaults to 0. - */ - childSessions?: number /** * Whether THIS scenario pins its header class's model-facing request-header * content. Its actual composed prompt is maintained as a readable @@ -150,14 +142,40 @@ export interface SnapshotSuiteOptions { } /** - * The sibling child-fixture paths for a scenario (`session.1.jsonl` …). + * Validate and order a scenario directory's session-fixture filenames. * - * @param dir The scenario's snapshots directory (`/`). - * @param childSessions How many subagent child sessions the scenario records. - * @returns One path per child, 1-based, in fixture order. + * The primary fixture is always `session.jsonl`; child sessions are discovered + * from contiguous `session.1.jsonl` … filenames. The directory is the source of + * truth, so scenario tables do not duplicate a child count that can drift from + * the files. A session-like JSONL with any other suffix fails loud. + * + * @param names File names in one scenario directory. + * @returns The primary and child fixture names in replay/harvest order. */ -export function childFixturePaths(dir: string, childSessions: number): string[] { - return Array.from({ length: childSessions }, (_, i) => join(dir, `session.${i + 1}.jsonl`)) +export function sessionFixtureNames(names: readonly string[]): string[] { + if (!names.includes('session.jsonl')) throw new Error('missing session.jsonl') + const children: { name: string; index: number }[] = [] + for (const name of names) { + if (name === 'session.jsonl') continue + if (!name.startsWith('session.') || !name.endsWith('.jsonl')) continue + const match = /^session\.([1-9]\d*)\.jsonl$/.exec(name) + if (match === null) throw new Error(`invalid child session fixture name: ${name}`) + children.push({ name, index: Number(match[1]) }) + } + children.sort((a, b) => a.index - b.index) + for (const [offset, child] of children.entries()) { + const expected = offset + 1 + if (child.index !== expected) { + throw new Error(`child session fixtures must be contiguous: expected session.${expected}.jsonl, found ${child.name}`) + } + } + return ['session.jsonl', ...children.map(child => child.name)] +} + +/** Read one scenario directory's validated session-fixture inventory. */ +async function sessionFixtures(dir: string): Promise { + const entries = await readdir(dir, { withFileTypes: true }) + return sessionFixtureNames(entries.filter(entry => entry.isFile()).map(entry => entry.name)) } /** @@ -392,7 +410,9 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { const input = JSON.parse(await readFile(join(dir, 'input.json'), 'utf8')) as InputScript const overrideFile = join(dir, 'replay.override.json') const workspaceDir = join(dir, 'workspace') - const childSessions = scenario.childSessions ?? 0 + const fixtureFiles = await sessionFixtures(dir) + const childFixtureFiles = fixtureFiles.slice(1) + const childSessions = childFixtureFiles.length const comparesLog = scenario.comparesLog ?? scenario.hasModelTurn const result = await runScenario(input, { agent, @@ -401,7 +421,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { ...existsSync(overrideFile) ? { overrideFile } : {}, // In REPLAY, forward the recorded child fixtures so each subagent session // replays from its own script. In RECORD they are harvested, not read. - ...!RECORDING && childSessions > 0 ? { childFiles: childFixturePaths(dir, childSessions) } : {}, + ...!RECORDING && childSessions > 0 ? { childFiles: childFixtureFiles.map(file => join(dir, file)) } : {}, ...existsSync(workspaceDir) ? { workspaceDir } : {}, // A scenario booting an overlay tree passes its own live config; the // bin's replay swap derives the sibling `*cordis.snapshot.yml` from it. @@ -432,7 +452,6 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { const scrub = scenario.pinsHeader === true ? scrubSystemPrompts : scrubRequestHeaders - const fixtureFiles = ['session.jsonl', ...Array.from({ length: childSessions }, (_, i) => `session.${i + 1}.jsonl`)] const existingFixtures = REFRESHING ? await Promise.all(fixtureFiles.map(file => readFile(join(dir, file), 'utf8'))) : [] @@ -543,7 +562,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { expect(onDisk).toEqual(registered) }) - it('every registered scenario has its required fixture files', () => { + it('every registered scenario has its required fixture files', async () => { // Every scenario has an input script and an stdout golden. EVERY scenario // also needs `session.jsonl`: the suite boots `llm-replay` with that path // as the replay source for ALL scenarios (the factory passes @@ -556,7 +575,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { // `overridden` flag: required when set, forbidden when not — the harness // forwards the file purely on existence, so an unregistered stray sidecar // would silently replace the derived script. - for (const { name, overridden, childSessions, pinsHeader } of scenarios) { + for (const { name, overridden, pinsHeader } of scenarios) { const dir = join(snapshotsDir, name) expect(existsSync(join(dir, 'input.json')), `${name}/input.json`).toBe(true) expect(existsSync(join(dir, 'stdout.golden.jsonl')), `${name}/stdout.golden.jsonl`).toBe(true) @@ -565,11 +584,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { .toBe(overridden === true) expect(existsSync(join(dir, SYSTEM_PROMPT_SNAPSHOT)), `${name}/${SYSTEM_PROMPT_SNAPSHOT} presence must match \`pinsHeader\``) .toBe(pinsHeader === true) - // A nested-agent scenario ships one child fixture per recorded subagent - // session (`session.1.jsonl` …), the replay source for that child session. - for (const childFixture of childFixturePaths(dir, childSessions ?? 0)) { - expect(existsSync(childFixture), childFixture).toBe(true) - } + await expect(sessionFixtures(dir), `${name}: session fixture inventory`).resolves.toBeDefined() } }) @@ -615,10 +630,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { // all header bulk. Fixed-point checks make both storage rules fail loud. 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`), - ] + const files = await sessionFixtures(dir) for (const file of files) { const fixture = await readFile(join(dir, file), 'utf8') expect(scrubSystemPrompts(fixture), `${scenario.name}/${file} carries an unscrubbed system prompt`) diff --git a/packages/support/acp-snapshot/tests/suite.spec.ts b/packages/support/acp-snapshot/tests/suite.spec.ts index 4ca9c8573d..f27f36c564 100644 --- a/packages/support/acp-snapshot/tests/suite.spec.ts +++ b/packages/support/acp-snapshot/tests/suite.spec.ts @@ -6,13 +6,13 @@ import { fileURLToPath } from 'node:url' import { afterAll, describe, expect, it } from 'vitest' import { defineAcpSnapshotSuite, type HarvestedLog, type Scenario } from '../src/index.ts' import { - childFixturePaths, fixtureContext, formatSystemPromptSnapshot, headerChangeCount, normalizedHeaders, normalizedSystemPrompts, refreshFixtureReplacements, + sessionFixtureNames, stabilizeRefreshLog, } from '../src/suite.ts' @@ -51,7 +51,7 @@ const RECORD_SRC = fileURLToPath(new URL('./fixtures/record-suite', import.meta. // example's code-mode scenarios). const REPLAY_SCENARIOS: Scenario[] = [ { name: 'pin-turn', hasModelTurn: true, recorded: true, pinsHeader: true, expectedHeaderChanges: 1, headerClass: 'main' }, - { name: 'plain-turn', hasModelTurn: true, recorded: true, childSessions: 1, headerClass: 'main', configPath: AGENT.configPath }, + { name: 'plain-turn', hasModelTurn: true, recorded: true, headerClass: 'main', configPath: AGENT.configPath }, { name: 'no-model', hasModelTurn: false, recorded: false, headerClass: 'main' }, { name: 'blocked-log', hasModelTurn: false, comparesLog: true, recorded: false, headerClass: 'main' }, { name: 'authored-error', hasModelTurn: true, recorded: false, overridden: true, headerClass: 'main' }, @@ -59,7 +59,7 @@ const REPLAY_SCENARIOS: Scenario[] = [ const RECORD_SCENARIOS: Scenario[] = [ { name: 'rec-pin', hasModelTurn: true, recorded: true, pinsHeader: true }, - { name: 'rec-child', hasModelTurn: true, recorded: true, childSessions: 1 }, + { name: 'rec-child', hasModelTurn: true, recorded: true }, // recorded:false in record mode → registered but skipped (never re-recorded). { name: 'rec-skip', hasModelTurn: true, recorded: false, overridden: true }, ] @@ -180,13 +180,41 @@ describe('defineAcpSnapshotSuite: registration contract', () => { }) }) -describe('childFixturePaths', () => { - it('yields one sibling path per child, 1-based', () => { - expect(childFixturePaths('/snap/s', 2)).toEqual(['/snap/s/session.1.jsonl', '/snap/s/session.2.jsonl']) +describe('sessionFixtureNames', () => { + it('orders the primary and contiguous child fixtures while ignoring other files', () => { + expect(sessionFixtureNames([ + 'stdout.golden.jsonl', + 'session.2.jsonl', + 'session.jsonl', + 'session.1.jsonl', + 'input.json', + ])).toEqual(['session.jsonl', 'session.1.jsonl', 'session.2.jsonl']) }) - it('yields nothing for a single-session scenario', () => { - expect(childFixturePaths('/snap/s', 0)).toEqual([]) + it('accepts a primary-only scenario', () => { + expect(sessionFixtureNames(['session.jsonl'])).toEqual(['session.jsonl']) + }) + + it('rejects a directory without the primary fixture', () => { + expect(() => sessionFixtureNames(['session.1.jsonl'])).toThrow('missing session.jsonl') + }) + + it('rejects gapped child fixtures', () => { + expect(() => sessionFixtureNames(['session.jsonl', 'session.2.jsonl'])) + .toThrow('expected session.1.jsonl, found session.2.jsonl') + }) + + it.each(['session.0.jsonl', 'session.child.jsonl', 'session.01.jsonl'])( + 'rejects invalid child fixture name %s', + (name) => { + expect(() => sessionFixtureNames(['session.jsonl', name])) + .toThrow(`invalid child session fixture name: ${name}`) + }, + ) + + it('rejects duplicate child indexes', () => { + expect(() => sessionFixtureNames(['session.jsonl', 'session.1.jsonl', 'session.1.jsonl'])) + .toThrow('expected session.2.jsonl, found session.1.jsonl') }) }) From 6ad4b4fa42eaf296a2692c4e0886dee02cbfe75f Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:46:46 +0800 Subject: [PATCH 2/3] fix: let snapshot recording create fixture inventory --- packages/support/acp-snapshot/src/suite.ts | 38 ++++++++++++++----- .../support/acp-snapshot/tests/suite.spec.ts | 17 ++++++++- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/support/acp-snapshot/src/suite.ts b/packages/support/acp-snapshot/src/suite.ts index 6311a15031..4d87093c8a 100644 --- a/packages/support/acp-snapshot/src/suite.ts +++ b/packages/support/acp-snapshot/src/suite.ts @@ -31,7 +31,7 @@ * @module @deepseek-ai/dsh-acp-snapshot/suite */ -import { readFile, readdir, writeFile } from 'node:fs/promises' +import { readFile, readdir, rm, writeFile } from 'node:fs/promises' import { existsSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' @@ -410,9 +410,12 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { const input = JSON.parse(await readFile(join(dir, 'input.json'), 'utf8')) as InputScript const overrideFile = join(dir, 'replay.override.json') const workspaceDir = join(dir, 'workspace') - const fixtureFiles = await sessionFixtures(dir) + // Replay/refresh need the committed inventory up front because those + // files drive the model scripts. Record mode creates that inventory + // from the harvested live logs, so it must also work for a brand-new + // scenario with no session.jsonl yet. + let fixtureFiles = RECORDING ? [] : await sessionFixtures(dir) const childFixtureFiles = fixtureFiles.slice(1) - const childSessions = childFixtureFiles.length const comparesLog = scenario.comparesLog ?? scenario.hasModelTurn const result = await runScenario(input, { agent, @@ -421,7 +424,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { ...existsSync(overrideFile) ? { overrideFile } : {}, // In REPLAY, forward the recorded child fixtures so each subagent session // replays from its own script. In RECORD they are harvested, not read. - ...!RECORDING && childSessions > 0 ? { childFiles: childFixtureFiles.map(file => join(dir, file)) } : {}, + ...!RECORDING && childFixtureFiles.length > 0 ? { childFiles: childFixtureFiles.map(file => join(dir, file)) } : {}, ...existsSync(workspaceDir) ? { workspaceDir } : {}, // A scenario booting an overlay tree passes its own live config; the // bin's replay swap derives the sibling `*cordis.snapshot.yml` from it. @@ -460,18 +463,35 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { || (REFRESHING && comparesLog) if (writesSessionFixtures) { expect(result.sessionLogs.length, `${mode} produced no session log to harvest`).toBeGreaterThan(0) - expect(result.sessionLogs.length, `expected ${childSessions + 1} session logs (parent + children)`) - .toBe(childSessions + 1) + if (REFRESHING) { + expect(result.sessionLogs.length, `expected ${fixtureFiles.length} session logs (parent + children)`) + .toBe(fixtureFiles.length) + } + const outputFixtureFiles = [ + 'session.jsonl', + ...Array.from({ length: result.sessionLogs.length - 1 }, (_, i) => `session.${i + 1}.jsonl`), + ] const primary = (result.sessionLogs[0] as HarvestedLog).content - await writeFile(join(dir, 'session.jsonl'), scrub( + await writeFile(join(dir, outputFixtureFiles[0] as string), scrub( REFRESHING ? stabilizeRefreshLog(primary, existingFixtures[0] as string, replacements) : primary, )) for (let i = 1; i < result.sessionLogs.length; i++) { const child = (result.sessionLogs[i] as HarvestedLog).content - await writeFile(join(dir, `session.${i}.jsonl`), scrub( + await writeFile(join(dir, outputFixtureFiles[i] as string), scrub( REFRESHING ? stabilizeRefreshLog(child, existingFixtures[i] as string, replacements) : child, )) } + if (RECORDING) { + const outputNames = new Set(outputFixtureFiles) + const entries = await readdir(dir, { withFileTypes: true }) + await Promise.all(entries + .filter(entry => entry.isFile() + && entry.name.startsWith('session.') + && entry.name.endsWith('.jsonl') + && !outputNames.has(entry.name)) + .map(entry => rm(join(dir, entry.name)))) + fixtureFiles = outputFixtureFiles + } if (scenario.pinsHeader === true) { const primary = result.sessionLogs[0] as HarvestedLog const prompts = normalizedSystemPrompts(primary.content, ctx) @@ -498,7 +518,7 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { // prompt becomes the fixture's `{{system}}`; non-pinning scenarios // additionally tokenize tools/prefix. The dedicated header guard below // compares those omitted values against their class's pin artifacts. - expect(result.sessionLogs.length, 'this scenario must persist a session log').toBe(childSessions + 1) + expect(result.sessionLogs.length, 'this scenario must persist one log per session fixture').toBe(fixtureFiles.length) for (let i = 0; i < fixtureFiles.length; i++) { const harvested = scrub((result.sessionLogs[i] as HarvestedLog).content) const fixture = scrub(await readFile(join(dir, fixtureFiles[i] as string), 'utf8')) diff --git a/packages/support/acp-snapshot/tests/suite.spec.ts b/packages/support/acp-snapshot/tests/suite.spec.ts index f27f36c564..7aa218526c 100644 --- a/packages/support/acp-snapshot/tests/suite.spec.ts +++ b/packages/support/acp-snapshot/tests/suite.spec.ts @@ -1,4 +1,4 @@ -import { cpSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' +import { cpSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -69,7 +69,13 @@ const RECORD_SCENARIOS: Scenario[] = [ // committed record fixtures/goldens in place. const BOOTSTRAP = process.env.ACP_SNAPSHOT_SPEC_BOOTSTRAP === '1' const recordDir = BOOTSTRAP ? RECORD_SRC : mkdtempSync(join(tmpdir(), 'acp-snap-record-suite-')) -if (!BOOTSTRAP) cpSync(RECORD_SRC, recordDir, { recursive: true }) +if (!BOOTSTRAP) { + cpSync(RECORD_SRC, recordDir, { recursive: true }) + // Record mode owns its output inventory: a new scenario has no primary yet, + // while a changed child count can leave old numbered fixtures behind. + rmSync(join(recordDir, 'rec-pin', 'session.jsonl')) + writeFileSync(join(recordDir, 'rec-child', 'session.2.jsonl'), 'stale child\n') +} const refreshDir = mkdtempSync(join(tmpdir(), 'acp-snap-refresh-suite-')) cpSync(REPLAY_DIR, refreshDir, { recursive: true }) staleRefreshFixtures(refreshDir) @@ -141,6 +147,13 @@ describe('defineAcpSnapshotSuite: refresh write-back', () => { }) }) +describe('defineAcpSnapshotSuite: record inventory write-back', () => { + it('creates a missing primary fixture and prunes stale child fixtures', () => { + expect(readFileSync(join(recordDir, 'rec-pin', 'session.jsonl'), 'utf8')).toContain('"type":"session"') + expect(() => readFileSync(join(recordDir, 'rec-child', 'session.2.jsonl'), 'utf8')).toThrow() + }) +}) + describe('defineAcpSnapshotSuite: registration contract', () => { it("throws when a scenario's header class has no pinning scenario", () => { expect(() => { From 287041e39e93b368646f0fe8298b65feab692acc Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 14 Jul 2026 12:35:25 +0800 Subject: [PATCH 3/3] fix: preserve malformed snapshot fixtures --- packages/support/acp-snapshot/src/suite.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/support/acp-snapshot/src/suite.ts b/packages/support/acp-snapshot/src/suite.ts index 4d87093c8a..906521c42d 100644 --- a/packages/support/acp-snapshot/src/suite.ts +++ b/packages/support/acp-snapshot/src/suite.ts @@ -486,8 +486,10 @@ export function defineAcpSnapshotSuite(options: SnapshotSuiteOptions): void { const entries = await readdir(dir, { withFileTypes: true }) await Promise.all(entries .filter(entry => entry.isFile() - && entry.name.startsWith('session.') - && entry.name.endsWith('.jsonl') + // Only valid numbered children are record-owned stale output. + // Malformed session-like names stay for the inventory guard to + // reject instead of being silently deleted during mutation. + && /^session\.[1-9]\d*\.jsonl$/.test(entry.name) && !outputNames.has(entry.name)) .map(entry => rm(join(dir, entry.name)))) fixtureFiles = outputFixtureFiles