瀏覽代碼

fix(snapshot): isolate concurrent spill roots

Tianyi Cui 3 月之前
父節點
當前提交
07058e527c

+ 1 - 1
.agents/notes/implemented/testing/2026-06-19-acp-snapshot-tests.md

@@ -57,7 +57,7 @@ Normalization replaces session, cwd, protocol-id, timestamp, path, and process v
 
 ### Isolation: normalization now, sandbox later
 
-Tool determinism comes from a temporary cwd, scrubbed environment, fresh non-login shell, constrained commands, and normalization. It does not claim OS confinement. A sandboxed executor can replace the local backend through the existing [capability seam](../architecture/2026-06-13-capability-seams.md) if a stronger tier is needed.
+Tool determinism comes from a temporary cwd, scrubbed environment, fresh non-login shell, constrained commands, and normalization. Concurrent replay runs own separate cwd, persistence, and fixed-length scenario-keyed spill roots, so one scenario's teardown cannot delete another's in-flight full-output recovery while real-path preview budgets remain stable. This tier does not claim OS confinement. A sandboxed executor can replace the local backend through the existing [capability seam](../architecture/2026-06-13-capability-seams.md) if a stronger tier is needed.
 
 ### The replay plugin is its own package
 

+ 12 - 2
packages/support/acp-snapshot/src/harness.ts

@@ -18,8 +18,9 @@
 
 import { cp, mkdtemp, readFile, readdir, rm } from 'node:fs/promises'
 import { existsSync } from 'node:fs'
+import { createHash } from 'node:crypto'
 import { tmpdir } from 'node:os'
-import { join, delimiter } from 'node:path'
+import { basename, dirname, join, delimiter } from 'node:path'
 import {
   ClientSideConnection,
   PROTOCOL_VERSION,
@@ -152,6 +153,13 @@ export interface RunOptions {
   configPath?: string
 }
 
+/** Derive one stable, fixed-length spill root owned by this scenario. */
+function scenarioSpillRoot(fixtureFile: string): string {
+  const scenario = basename(dirname(fixtureFile))
+  const key = createHash('sha256').update(scenario).digest('hex').slice(0, 9)
+  return `/tmp/dsh-acp-snap-${key}`
+}
+
 /**
  * Run a scenario end-to-end against a freshly-spawned subprocess. Owns the
  * child and its temp dirs; always tears them down. Returns the captured stdout
@@ -166,7 +174,9 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise
   const sessionsRoot = await mkdtemp(join(tmpdir(), 'acp-snap-sessions-'))
   // Fixed path length: spill-policy budgets the preview against the REAL path
   // before stdout normalization, so tmpdir() length differences churn expected outputs.
-  const spillRoot = '/tmp/dsh-acp-snapshot-spill'
+  // Scenario ownership also matters: replay runs concurrently, and one teardown
+  // must never delete another scenario's in-flight full-output recovery file.
+  const spillRoot = scenarioSpillRoot(opts.fixtureFile)
   // Everything past the temp-dir creation is followed by failure-safe cleanup,
   // so a failure in workspace seeding, spawn, or any step never leaks resources.
   let launched: LaunchedAcpTestAgent | undefined

+ 1 - 1
packages/support/acp-snapshot/src/normalize.ts

@@ -20,7 +20,7 @@ const LOCAL_SPILL_PATH_RE = new RegExp(
   'g',
 )
 const SNAPSHOT_SPILL_PATH_RE = new RegExp(
-  String.raw`/tmp/dsh-acp-snapshot-spill/session-[0-9a-f]{12}/[0-9a-f]{12}-([A-Za-z0-9._~-]+?)`
+  String.raw`/tmp/(?:dsh-acp-snap-[0-9a-f]{9}|dsh-acp-snapshot-spill)/session-[0-9a-f]{12}/[0-9a-f]{12}-([A-Za-z0-9._~-]+?)`
   + String.raw`(?=\. Use read with offset/limit|[\s)]|$)`,
   'g',
 )

+ 1 - 0
packages/support/acp-snapshot/tests/fixtures/fake-acp-agent.ts

@@ -152,6 +152,7 @@ async function handlePrompt(id: number | string): Promise<void> {
       mode: process.env.DSH_SNAPSHOT,
       override: process.env.DSH_SNAPSHOT_OVERRIDE ?? null,
       childFiles: process.env.DSH_SNAPSHOT_CHILD_FILES ?? null,
+      spillRoot: process.env.DSH_SNAPSHOT_SPILL_ROOT ?? null,
     })}`)
   }
   if (behavior.echoWorkspace === true) {

+ 22 - 0
packages/support/acp-snapshot/tests/harness.spec.ts

@@ -60,6 +60,15 @@ async function scenario(behavior: object): Promise<{ dir: string; fixtureFile: s
 
 const boot: InputStep[] = [{ op: 'initialize' }, { op: 'newSession' }]
 
+function environmentEcho(rawStdout: string): Record<string, unknown> {
+  const frames = rawStdout.trim().split('\n')
+    .map(line => JSON.parse(line) as { params?: { update?: { content?: { text?: unknown } } } })
+  const text = frames.map(frame => frame.params?.update?.content?.text)
+    .find(value => typeof value === 'string' && value.startsWith('env:'))
+  if (typeof text !== 'string') throw new Error('fake ACP agent did not echo its environment')
+  return JSON.parse(text.slice('env:'.length)) as Record<string, unknown>
+}
+
 describe('runScenario', () => {
   it('surfaces an asynchronous child spawn failure through startup and close', async () => {
     const { dir } = await scenario({})
@@ -309,6 +318,19 @@ describe('runScenario', () => {
     expect(result.rawStdout).toContain(JSON.stringify(childFiles.join(delimiter)).slice(1, -1))
   })
 
+  it('gives concurrent scenarios distinct equal-length spill roots', { timeout: 20_000 }, async () => {
+    const [first, second] = await Promise.all([scenario({ echoEnv: true }), scenario({ echoEnv: true })])
+    const results = await Promise.all([first, second].map(({ fixtureFile }) => runScenario(
+      { steps: [...boot, { op: 'prompt', text: 'env?' }] },
+      { agent: AGENT, mode: 'replay', fixtureFile },
+    )))
+    const roots = results.map(result => environmentEcho(result.rawStdout).spillRoot)
+    expect(roots.every(root => typeof root === 'string')).toBe(true)
+    expect(new Set(roots).size).toBe(2)
+    expect((roots[0] as string).length).toBe((roots[1] as string).length)
+    expect((roots[0] as string).length).toBe('/tmp/dsh-acp-snapshot-spill'.length)
+  })
+
   it('seeds the workspace dir into the temp cwd before the run', { timeout: 20_000 }, async () => {
     const { dir, fixtureFile } = await scenario({ echoWorkspace: true })
     const workspaceDir = join(dir, 'workspace')

+ 15 - 0
packages/support/acp-snapshot/tests/normalize.spec.ts

@@ -139,6 +139,21 @@ describe('normalizeSessionLog', () => {
     expect(out).not.toContain('/tmp/dsh-acp-snapshot-spill')
   })
 
+  it('scrubs scenario-owned snapshot spill paths', () => {
+    const ev = JSON.stringify({
+      type: 'tool/result', seq: 2, time: 5,
+      data: {
+        content: [{
+          type: 'text',
+          text: 'Full formatted result stored at: /tmp/dsh-acp-snap-012345678/session-c22bc3f1d2af/8a7b6c5d4e3f-bash.txt. Use read with offset/limit, or grep this path to search within it.',
+        }],
+      },
+    })
+    const out = normalizeSessionLog(`${header({ cwd: ctx.cwd })}\n${ev}\n`, ctx)
+    expect(out).toContain('{{spillLocator:bash.txt}}')
+    expect(out).not.toContain('/tmp/dsh-acp-snap-012345678')
+  })
+
   it('scrubs the session id in the header', () => {
     const out = normalizeSessionLog(`${header({ id: ctx.sessionIds[0] })}\n`, ctx)
     expect(out).toContain('{{sessionId}}')