From c0d7ef22ec5408e33a3b34ff91e3fd83c7f03a60 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 21 Jul 2026 17:14:10 +0800 Subject: [PATCH] fix(subagent-acp): repair the composition fixture for the merged depth budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master landed the tool-subagent depth budget while this branch was in flight: the numeric default maxDepth now fails the mount against the ACP provider's depthLimit: false, so the composition fixture must state maxDepth: 'provider-managed' — the documented opt-out for a provider whose recursion budget lives in the child harness. The fixture also moves off the retired stdio-demo REPL onto the current app-boot driver pattern (runLoaderSmoke + one-shot cli), and the split isDirectory statements gain the file-not-a-directory case the single-expression form used to cover implicitly. --- .../fixtures/subagent/subagent-acp/cordis.yml | 10 +- .../fixtures/subagent/subagent-acp/driver.ts | 15 +++ knip.json | 1 + .../tests/loader-composition.e2e.ts | 126 +++++++----------- .../subagent-acp/tests/subagent-acp.spec.ts | 14 ++ 5 files changed, 86 insertions(+), 80 deletions(-) create mode 100644 examples/acp-agent/tests/fixtures/subagent/subagent-acp/driver.ts diff --git a/examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml index 76ac73078f..3bd5f5393c 100644 --- a/examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml +++ b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml @@ -26,12 +26,16 @@ config: provider: acp toolName: subagent + # ACP advertises no depthLimit: the child harness owns its own recursion + # budget, so the local numeric default cannot apply here. + maxDepth: 'provider-managed' -- id: stdio-agent - name: '@deepseek-ai/dsh-stdio-demo' +- id: cli-agent + name: '@deepseek-ai/dsh-cli-demo' config: provider: mock model: mock-delegate - welcome: 'acp subagent cwd e2e ready.' + persona: 'Test ACP subagent cwd inheritance.' persistenceRoot: './.sessions' + persistenceCompression: 'none' workspaceContext: false diff --git a/examples/acp-agent/tests/fixtures/subagent/subagent-acp/driver.ts b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/driver.ts new file mode 100644 index 0000000000..d146b8df80 --- /dev/null +++ b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/driver.ts @@ -0,0 +1,15 @@ +#!/usr/bin/env node +/** Test driver: one delegation turn through a headless Loader composition. */ + +import { boot, resolveConfigPath } from '@deepseek-ai/dsh-app-boot' +import { runOneShot } from '@deepseek-ai/dsh-cli-demo/src/cli.ts' + +const configPath = process.argv[2] +if (configPath === undefined) throw new Error('acp-subagent cwd driver requires a config path') + +const ctx = await boot('acp-subagent-cwd-e2e', resolveConfigPath(configPath, undefined)) +try { + await runOneShot(ctx, { task: 'delegate' }) +} finally { + await ctx.fiber.dispose() +} diff --git a/knip.json b/knip.json index 5e4de16202..9c5c1efada 100644 --- a/knip.json +++ b/knip.json @@ -15,6 +15,7 @@ "headless-agent/tests/fixtures/time-context-mock-llm.ts", "tui-agent/tests/fixtures/tui-scripted-llm.ts", "acp-agent/tests/fixtures/subagent/subagent-acp/mock-delegating-llm.ts", + "acp-agent/tests/fixtures/subagent/subagent-acp/driver.ts", "*/tests/**/*.e2e.ts", "*/tests/**/*.snapshot.ts" ], diff --git a/packages/subagent/subagent-acp/tests/loader-composition.e2e.ts b/packages/subagent/subagent-acp/tests/loader-composition.e2e.ts index bdebf64fbb..900a28f4ec 100644 --- a/packages/subagent/subagent-acp/tests/loader-composition.e2e.ts +++ b/packages/subagent/subagent-acp/tests/loader-composition.e2e.ts @@ -1,101 +1,73 @@ -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' import { realpathSync } from 'node:fs' -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' +import { readFile, readdir } from 'node:fs/promises' import { join } from 'node:path' import { fileURLToPath } from 'node:url' -import { afterEach, describe, expect, it } from 'vitest' -import { resolveExampleLaunch } from '@deepseek-ai/dsh-loader-smoke' +import { describe, expect, it } from 'vitest' +import { type SessionEvent } from '@deepseek-ai/dsh-session' +import { LOADER_SMOKE_TEST_TIMEOUT_MS, runLoaderSmoke } from '@deepseek-ai/dsh-loader-smoke' /** * Keyless REAL-composition coverage for parent-session cwd inheritance: a - * test-only cordis.yml boots the stdio app through the Loader with the ACP + * test-only cordis.yml boots the headless app through the Loader with the ACP * backend's `cwd` omitted, a scripted model delegates once, and the scripted * mock ACP child echoes where it actually ran plus the workspace it was * announced — both must be the parent session's cwd. Mock-only composition, so * only this keyless tier applies (the with-key tier lives in subagent-acp.e2e.ts). */ -const binScript = fileURLToPath(new URL('../../../examples/stdio-demo/src/bin.ts', import.meta.url)) +const driver = fileURLToPath(new URL( + '../../../../examples/acp-agent/tests/fixtures/subagent/subagent-acp/driver.ts', + import.meta.url, +)) const configPath = fileURLToPath(new URL( '../../../../examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml', import.meta.url, )) const mockServer = fileURLToPath(new URL('./mock-acp-server.ts', import.meta.url)) const repoTsconfig = fileURLToPath(new URL('../../../../tsconfig.json', import.meta.url)) -const PROCESS_TIMEOUT_MS = 30_000 -const TEST_TIMEOUT_MS = PROCESS_TIMEOUT_MS + 15_000 -let child: ChildProcessWithoutNullStreams | undefined -let workdir: string | undefined - -afterEach(async () => { - if (child !== undefined && child.exitCode === null) child.kill('SIGKILL') - child = undefined - if (workdir !== undefined) await rm(workdir, { recursive: true, force: true }) - workdir = undefined -}) - -async function runDelegation(): Promise<{ stdout: string; stderr: string; cwd: string }> { - workdir = await mkdtemp(join(tmpdir(), 'acp-subagent-composition-')) - const cwd = workdir - return new Promise((resolve, reject) => { - const launch = resolveExampleLaunch({ - srcBin: binScript, - configArgs: [configPath], - tsconfigPath: repoTsconfig, - exposeInternals: true, - env: { - DSH_TEST_MOCK_ACP_SERVER: mockServer, - DSH_HOME: join(cwd, '.dsh'), - DSH_AGENTS_HOME: join(cwd, '.agents'), - }, - }) - const proc = spawn(launch.command, launch.args, { - cwd, - env: { ...process.env, ...launch.env }, - stdio: ['pipe', 'pipe', 'pipe'], - }) - child = proc - let stdout = '' - let stderr = '' - let closedStdin = false - proc.stdout.setEncoding('utf8') - proc.stdout.on('data', (chunk: string) => { - stdout += chunk - // One full turn: delegation + the follow-up reply quoting the child. - if (!closedStdin && stdout.includes('child reported:')) { - closedStdin = true - proc.stdin.end() - } - }) - proc.stderr.setEncoding('utf8') - proc.stderr.on('data', (chunk: string) => { stderr += chunk }) - - const timer = setTimeout(() => { - proc.kill('SIGKILL') - reject(new Error(`acp-subagent composition e2e did not exit within ${PROCESS_TIMEOUT_MS / 1_000}s. stdout:\n${stdout}\nstderr:\n${stderr}`)) - }, PROCESS_TIMEOUT_MS) - - proc.on('exit', (code) => { - clearTimeout(timer) - if (code === 0) resolve({ stdout, stderr, cwd }) - else reject(new Error(`acp-subagent composition e2e exited ${code}. stdout:\n${stdout}\nstderr:\n${stderr}`)) - }) - proc.on('error', (error) => { clearTimeout(timer); reject(error) }) - proc.stdin.write('delegate\n') - }) +async function jsonlFiles(dir: string): Promise { + const entries = await readdir(dir, { withFileTypes: true }) + const paths = await Promise.all(entries.map(async (entry) => { + const path = join(dir, entry.name) + if (entry.isDirectory()) return jsonlFiles(path) + return entry.isFile() && entry.name.endsWith('.jsonl') ? [path] : [] + })) + return paths.flat() } -describe('ACP subagent cwd inheritance through a real cordis.yml and stdio process', () => { +describe('ACP subagent cwd inheritance through a real cordis.yml', () => { it('runs the child in the parent session workspace and announces it as the ACP session cwd', async () => { - const { stdout, stderr, cwd } = await runDelegation() + let events: SessionEvent[] = [] + let workspace = '' + const { stderr } = await runLoaderSmoke({ + label: 'acp-subagent cwd composition smoke', + tempDirPrefix: 'acp-subagent-cwd-e2e-', + binScript: driver, + libBinScript: driver, + configPath, + tsconfigPath: repoTsconfig, + env: { DSH_TEST_MOCK_ACP_SERVER: mockServer }, + inspect: async (cwd) => { + // The child reports realpaths; canonicalize the temp workspace to match. + workspace = realpathSync(cwd) + const logs = await jsonlFiles(join(cwd, '.sessions')) + expect(logs).toHaveLength(1) + const lines = (await readFile(logs[0] as string, 'utf8')).trimEnd().split('\n') + events = lines.slice(1).map(line => JSON.parse(line) as SessionEvent) + }, + }) expect(stderr).not.toContain('UNHANDLED') - expect(stdout).toContain('acp subagent cwd e2e ready.') - // The child streams two lines: its real process.cwd() and the cwd the - // backend announced in `session/new`. The parent session's workspace is the - // app's launch directory (canonical form — the child reports realpaths). - const workspace = realpathSync(cwd) - expect(stdout).toContain(`child reported:\n${workspace}\n${workspace}`) - }, TEST_TIMEOUT_MS) + + // The tool result carries the child's two-line echo: its real process.cwd() + // and the cwd the backend announced in `session/new` — both the parent + // session's workspace, never the harness process's launch directory. + const results = events.filter(event => event.type === 'tool/result') + expect(results).toHaveLength(1) + const resultText = results[0]!.data.content + .filter(block => block.type === 'text') + .map(block => block.text) + .join('') + expect(resultText).toBe(`${workspace}\n${workspace}`) + }, LOADER_SMOKE_TEST_TIMEOUT_MS) }) diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 715b0d7caf..f80ce2810c 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -262,6 +262,20 @@ describe('cwd resolution', () => { .rejects.toThrow('must be an absolute path') }) + it('rejects a parent session cwd that names a FILE, not a directory', async () => { + const tmp = mkdtempSync(join(tmpdir(), 'acp-file-cwd-')) + const file = join(tmp, 'a-file') + writeFileSync(file, 'x') + try { + const ctx = await setup({}) + const parent = { id: 'parent', session: { header: { cwd: file } } } as unknown as Agent + await expect(ctx.subagents.start('acp', { prompt: [{ type: 'text' as const, text: 'p' }], parent, signal: new AbortController().signal })) + .rejects.toThrow('not an accessible directory') + } finally { + rmSync(tmp, { recursive: true, force: true }) + } + }) + it('rejects a parent session cwd that is not an accessible directory, before spawning', async () => { const tmp = mkdtempSync(join(tmpdir(), 'acp-bad-parent-cwd-')) const sentinel = join(tmp, 'spawned')