From c432f85c075b0b083d93f6b6a1e4982bde413364 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Sun, 19 Jul 2026 15:21:33 +0800 Subject: [PATCH 1/6] fix(subagent-acp): resolve child cwd from the parent session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ACP backend resolved an unset cwd to the harness process's launch directory, so a delegated child ran — and announced its ACP session — in the server's cwd instead of the delegating session's workspace. Resolve the working directory explicitly: the config cwd override (made absolute and validated at load), else the parent session header's cwd (validated at start), failing loud before spawning when neither exists. --- docs/config-catalog.md | 9 +- packages/subagent/subagent-acp/README.md | 9 +- packages/subagent/subagent-acp/src/index.ts | 70 ++++++++- packages/subagent/subagent-acp/src/run.ts | 6 +- .../subagent-acp/tests/mock-acp-server.ts | 19 ++- .../subagent-acp/tests/subagent-acp.spec.ts | 133 +++++++++++++++++- 6 files changed, 226 insertions(+), 20 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index f77334b442..24e48b82f0 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -860,8 +860,11 @@ export interface Config { /** Arguments passed to {@link command}. */ args: string[] /** - * Working directory for the child process and its ACP session. Defaults to - * the parent process's cwd when omitted. + * Working directory override for the child process and its ACP session. A + * relative path resolves against the harness launch directory at load, and + * the result must be an existing directory. When omitted, each child + * inherits its delegating parent session's cwd — and starting one from a + * parent session that has no cwd fails. */ cwd?: string /** @@ -891,7 +894,7 @@ export interface Config { export type PermissionPolicy = 'allow' | 'reject' ``` -Source: [`packages/subagent/subagent-acp/src/index.ts:18`](../packages/subagent/subagent-acp/src/index.ts) +Source: [`packages/subagent/subagent-acp/src/index.ts:21`](../packages/subagent/subagent-acp/src/index.ts) ## `@deepseek-ai/dsh-subagent-fork` diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index a620051506..839bfd109f 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -4,7 +4,9 @@ The ACP provider runs each subagent in a fresh subprocess and drives it as an Ag ## Start and ownership -`start(request)` performs `spawn` → ACP `initialize` → `newSession` before it fulfills. Fulfillment therefore means a remote session is ready and ownership has transferred to the caller. A spawn, initialization, new-session, or pre-publication cancellation failure rejects only after the subprocess has been reaped. +`start(request)` resolves the child's working directory, then performs `spawn` → ACP `initialize` → `newSession` before it fulfills. Fulfillment therefore means a remote session is ready and ownership has transferred to the caller. A spawn, initialization, new-session, or pre-publication cancellation failure rejects only after the subprocess has been reaped; a working-directory resolution failure rejects before anything is spawned. + +The working directory is the configured `cwd` override when set, else the delegating parent session's cwd — never the server process's own cwd, because one server process serves sessions from many workspaces. The parent-derived value must be an absolute path naming an existing directory, and the same resolved path becomes both the subprocess cwd and the ACP `session/new` workspace. The returned run id is minted in the parent namespace. The child server's session id remains private to ACP wire calls because ACP guarantees it only within that fresh child process; using it as the parent lifecycle id could collide with another remote run or a local agent. @@ -14,7 +16,7 @@ After publication, the provider sends the prompt and collects streamed `agent_me ## Capabilities and context -ACP advertises no start-time capabilities because this process cannot enforce the remote child's depth, tool filter, persona, or structured-output runtime. It also reports `inheritsParentContext: false`: the remote session starts fresh and ignores `request.parent` beyond the seam's required attribution field. +ACP advertises no start-time capabilities because this process cannot enforce the remote child's depth, tool filter, persona, or structured-output runtime. It also reports `inheritsParentContext: false`: the remote session starts fresh, and the only parent-derived input is the workspace cwd described above — no conversation context crosses the process boundary. ## Configuration @@ -23,7 +25,7 @@ ACP advertises no start-time capabilities because this process cannot enforce th | `providerName` | `acp` | Registry name on `ctx.subagents`. | | `command` | required | Executable spawned for each run. | | `args` | `[]` | Command arguments. | -| `cwd` | process cwd | Child process and ACP session working directory. | +| `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; a relative value resolves against the harness launch directory at load and must name an existing directory. | | `permission` | `reject` | Auto-answer permission requests by rejecting or choosing the first allow-shaped option. | | `env` | `{}` | Explicit child environment layered over a credential-scrubbed parent environment. | | `disposeEofGraceMs` | `6000` | Grace after stdin EOF before SIGTERM. | @@ -76,6 +78,7 @@ Keyless tests drive a scripted ACP subprocess over real stdio. The with-key e2e ## Known Limitations and Deferred Work - **A fresh process per run** — persistent-process pooling is a future optimization ([the seam RFC](../../../docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md)). +- **Local workspaces only** — the resolved cwd is a local path handed to a child on the same machine; workspace mapping for a remote ACP agent would need its own backend capability and is not designed here. - **No optional start-time capabilities** — this provider cannot apply the local harness's `outputSchema`, depth cap, tool filter, or persona inside the remote process, so it advertises none and the service rejects requests that require them. - **Only `agent_message_chunk` text is collected** — the child's tool-call activity, thought chunks, and plan updates are not surfaced to the parent. - **Permission prompts are auto-answered** (`permission: allow | reject`) — no human is surfaced a child's `session/request_permission` in this cut. diff --git a/packages/subagent/subagent-acp/src/index.ts b/packages/subagent/subagent-acp/src/index.ts index 80766ed831..3722911f28 100644 --- a/packages/subagent/subagent-acp/src/index.ts +++ b/packages/subagent/subagent-acp/src/index.ts @@ -1,11 +1,14 @@ /** * Out-of-process ACP subagent backend. Each child has its own process, session, model, and - * tools, so it shares no Cordis context, ignores `request.parent`, and advertises no parent- - * enforced start capabilities. This plugin uses named exports only; a default would hide its + * tools, so it shares no Cordis context and advertises no parent-enforced start capabilities; + * the ONE thing it reads off `request.parent` is the session's workspace cwd (see + * {@link resolveCwd}). This plugin uses named exports only; a default would hide its * loader metadata (see `docs/postmortem/0001-acp-default-export-drops-inject.md`). * @module @deepseek-ai/dsh-subagent-acp */ +import { statSync } from 'node:fs' +import { isAbsolute, resolve } from 'node:path' import type { Context } from 'cordis' import z from 'schemastery' import type { SubagentCapabilities, SubagentProvider, SubagentStartRequest } from '@deepseek-ai/dsh-subagent' @@ -23,8 +26,11 @@ export interface Config { /** Arguments passed to {@link command}. */ args: string[] /** - * Working directory for the child process and its ACP session. Defaults to - * the parent process's cwd when omitted. + * Working directory override for the child process and its ACP session. A + * relative path resolves against the harness launch directory at load, and + * the result must be an existing directory. When omitted, each child + * inherits its delegating parent session's cwd — and starting one from a + * parent session that has no cwd fails. */ cwd?: string /** @@ -71,6 +77,53 @@ function assertPositiveFinite(name: string, value: number): void { /** The shape after schemastery applied the defaults (cwd has none). */ type ResolvedConfig = Required> & Pick +/** Whether `path` names an existing, accessible directory. */ +function isDirectory(path: string): boolean { + try { + return statSync(path).isDirectory() + } catch { + // statSync throws only filesystem access errors here (ENOENT/EACCES/ENOTDIR/…), + // and every one of them means the path cannot serve as the child's cwd. + return false + } +} + +/** + * Assert `cwd` can actually host the child: absolute (it doubles as the ACP + * session workspace, and a relative path would be re-anchored to the server + * process's launch directory) and an existing directory (fail here, before the + * process boundary, instead of as an ambiguous spawn ENOENT). + * @param label - which source supplied the value, for the diagnostic. + * @param cwd - the candidate working directory. + * @returns `cwd`, validated. + */ +function assertUsableCwd(label: string, cwd: string): string { + if (!isAbsolute(cwd)) { + throw new Error(`subagent-acp: ${label} must be an absolute path: ${cwd}`) + } + if (!isDirectory(cwd)) { + throw new Error(`subagent-acp: ${label} is not an accessible directory: ${cwd}`) + } + return cwd +} + +/** + * Resolve the child's working directory: the deployment `cwd` override when + * configured (already validated at load), else the parent session's workspace + * cwd (validated here, its earliest resolvable point). Fails loud when neither + * exists — falling back to the harness process cwd would silently bind the + * child to the server's launch directory instead of the delegating session's + * workspace (one server process serves many sessions, each with its own cwd). + */ +function resolveCwd(configured: string | undefined, request: SubagentStartRequest): string { + if (configured !== undefined) return configured + const parentCwd = request.parent.session.header.cwd + if (parentCwd === undefined) { + throw new Error('subagent-acp: no working directory for the child — configure `cwd` or delegate from a parent session that has one') + } + return assertUsableCwd('parent session cwd', parentCwd) +} + /** * The ACP provider. Advertises NO start-time capabilities: an out-of-process * child cannot honor `outputSchema`/`maxDepth`/`toolFilter` (the service rejects @@ -87,7 +140,7 @@ class AcpProvider implements SubagentProvider { const spec: AcpRunSpec = { command: this.config.command, args: this.config.args, - cwd: this.config.cwd ?? process.cwd(), + cwd: resolveCwd(this.config.cwd, request), permission: this.config.permission, env: this.config.env, disposeEofGraceMs: this.config.disposeEofGraceMs, @@ -107,5 +160,10 @@ export function apply(ctx: Context, config: Config): void { const resolved = config as ResolvedConfig assertPositiveFinite('disposeEofGraceMs', resolved.disposeEofGraceMs) assertPositiveFinite('disposeGraceMs', resolved.disposeGraceMs) - ctx.subagents.registerProvider(new AcpProvider(resolved.providerName, ctx, resolved)) + // Interpret a relative configured cwd against the harness launch directory + // ONCE, at load, and fail a misconfigured directory here — not per start. + const validated: ResolvedConfig = resolved.cwd === undefined + ? resolved + : { ...resolved, cwd: assertUsableCwd('config cwd', resolve(resolved.cwd)) } + ctx.subagents.registerProvider(new AcpProvider(validated.providerName, ctx, validated)) } diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index a10a87a940..82e7f1e08e 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -37,7 +37,11 @@ export interface AcpRunSpec { command: string /** Arguments passed to {@link command}. */ args: string[] - /** Working directory for the child process AND its ACP session `cwd`. */ + /** + * Absolute working directory for the child process AND its ACP session + * `cwd`. The provider resolves it before this spec exists: config override, + * else the delegating parent session's workspace. + */ cwd: string /** How to auto-answer the child's permission prompts. */ permission: PermissionPolicy diff --git a/packages/subagent/subagent-acp/tests/mock-acp-server.ts b/packages/subagent/subagent-acp/tests/mock-acp-server.ts index 2bbf457c18..6b3f8157e8 100644 --- a/packages/subagent/subagent-acp/tests/mock-acp-server.ts +++ b/packages/subagent/subagent-acp/tests/mock-acp-server.ts @@ -15,6 +15,11 @@ * `dispose()` must still kill the process. * - `MOCK_PERMISSION` — if `1`, the agent calls `session/request_permission` * before answering, to exercise the client's auto-answer. + * - `MOCK_ECHO_CWD` — if `1`, ignore MOCK_TEXT and stream two lines instead: + * the agent PROCESS's `process.cwd()` and the `cwd` the + * client announced in `session/new` — so a test can assert + * where the child actually ran and what workspace it was + * told it has. * - `MOCK_READY_FILE` — if set, the path the agent touches once its `prompt` * handler is in flight (it has streamed its chunk). A test * polls for this file to cancel on a CONDITION rather than @@ -63,6 +68,7 @@ import { } from '@agentclientprotocol/sdk' const TEXT = process.env.MOCK_TEXT ?? 'mock child answer' +const ECHO_CWD = process.env.MOCK_ECHO_CWD === '1' const STOP = (process.env.MOCK_STOP ?? 'end_turn') as StopReason const HANG = process.env.MOCK_HANG === '1' const WANT_PERMISSION = process.env.MOCK_PERMISSION === '1' @@ -83,6 +89,8 @@ function makeAgent(conn: AgentSideConnection): Agent { // Pending cancel resolver for the HANG path: a `session/cancel` resolves the // prompt with `cancelled`. let resolveCancel: ((reason: StopReason) => void) | undefined + // The cwd the client announced in `session/new`, echoed under MOCK_ECHO_CWD. + let sessionCwd: string | undefined return { initialize(_params: InitializeRequest): Promise { @@ -92,7 +100,8 @@ function makeAgent(conn: AgentSideConnection): Agent { authMethods: [], }) }, - async newSession(_params: NewSessionRequest): Promise { + async newSession(params: NewSessionRequest): Promise { + sessionCwd = params.cwd // Optionally signal "newSession reached" and block until released, so a // test can cancel DURING newSession (the early-cancel race window) on a // condition rather than a timeout. @@ -136,10 +145,14 @@ function makeAgent(conn: AgentSideConnection): Agent { update: { sessionUpdate: 'agent_thought_chunk', content: { type: 'text', text: 'thinking…' } }, }) } - // Stream the canned assistant text as one chunk. + // Stream the canned assistant text as one chunk (or, under MOCK_ECHO_CWD, + // the observable process cwd + announced session cwd). await conn.sessionUpdate({ sessionId: params.sessionId, - update: { sessionUpdate: 'agent_message_chunk', content: { type: 'text', text: TEXT } }, + update: { + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: ECHO_CWD ? `${process.cwd()}\n${sessionCwd ?? ''}` : TEXT }, + }, }) // Signal "prompt is in flight" by touching the readiness file, so a test // can wait on a CONDITION (file exists) rather than an arbitrary timeout diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index e4231c1598..9e32e3acf3 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from 'vitest' import { Context } from 'cordis' import Loader from '@cordisjs/plugin-loader' -import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import SubagentService from '@deepseek-ai/dsh-subagent' import { buildChildEnv } from '@deepseek-ai/dsh-subagent-subprocess' @@ -22,8 +22,8 @@ import { acpStopReason, acpContentText, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DI const mockServer = fileURLToPath(new URL('./mock-acp-server.ts', import.meta.url)) -/** A throwaway parent Agent — the ACP backend ignores it, but the seam requires one. */ -const fakeParent = { id: 'parent', session: { header: {} } } as unknown as Agent +/** A parent Agent stub. The ACP backend reads exactly one thing off it: the session header's cwd (the workspace its child inherits). */ +const fakeParent = { id: 'parent', session: { header: { cwd: process.cwd() } } } as unknown as Agent function request(text = 'p', signal = new AbortController().signal) { return { prompt: [{ type: 'text' as const, text }], parent: fakeParent, signal } @@ -115,6 +115,131 @@ describe('buildChildEnv', () => { }) }) +describe('cwd resolution', () => { + it('falls back to the parent session cwd for the child process AND its ACP session', async () => { + // realpath: on macOS `tmpdir()` sits behind a symlink (/var → /private/var), + // and the child reports its REAL process.cwd() — compare canonical paths. + const workdir = realpathSync(mkdtempSync(join(tmpdir(), 'acp-parent-cwd-'))) + try { + const ctx = await setup({ MOCK_ECHO_CWD: '1' }) + const parent = { id: 'parent', session: { header: { cwd: workdir } } } as unknown as Agent + const run = await ctx.subagents.start('acp', { prompt: [{ type: 'text' as const, text: 'p' }], parent, signal: new AbortController().signal }) + const result = await run.result + await run.dispose() + // Line 1: where the child process actually ran; line 2: the workspace the + // backend announced in `session/new`. Both must be the parent's workspace. + expect(text(result.output)).toBe(`${workdir}\n${workdir}`) + } finally { + rmSync(workdir, { recursive: true, force: true }) + } + }) + + it('rejects before spawning when neither config.cwd nor the parent session provides one', async () => { + const tmp = mkdtempSync(join(tmpdir(), 'acp-no-cwd-')) + const sentinel = join(tmp, 'spawned') + try { + const ctx = new Context() + await ctx.plugin(SubagentService) + // A command that would create the sentinel if the child were ever spawned. + await ctx.plugin(acp, { providerName: 'acp', command: 'touch', args: [sentinel], permission: 'reject', env: {} }) + const parent = { id: 'parent', session: { header: {} } } as unknown as Agent + await expect(ctx.subagents.start('acp', { prompt: [{ type: 'text' as const, text: 'p' }], parent, signal: new AbortController().signal })) + .rejects.toThrow('no working directory') + // Resolution failed BEFORE the process boundary — nothing was launched. + expect(existsSync(sentinel)).toBe(false) + } finally { + rmSync(tmp, { recursive: true, force: true }) + } + }) + + it('prefers the configured cwd override to the parent session cwd', async () => { + const configured = realpathSync(mkdtempSync(join(tmpdir(), 'acp-cfg-cwd-'))) + const parentDir = realpathSync(mkdtempSync(join(tmpdir(), 'acp-parent-cwd-'))) + try { + const ctx = new Context() + await ctx.plugin(SubagentService) + await ctx.plugin(acp, { + providerName: 'acp', + command: process.execPath, + args: [mockServer], + cwd: configured, + permission: 'reject', + env: { MOCK_ECHO_CWD: '1' }, + }) + const parent = { id: 'parent', session: { header: { cwd: parentDir } } } as unknown as Agent + const run = await ctx.subagents.start('acp', { prompt: [{ type: 'text' as const, text: 'p' }], parent, signal: new AbortController().signal }) + const result = await run.result + await run.dispose() + expect(text(result.output)).toBe(`${configured}\n${configured}`) + } finally { + rmSync(configured, { recursive: true, force: true }) + rmSync(parentDir, { recursive: true, force: true }) + } + }) + + it('resolves a relative config cwd against the launch directory at load', async () => { + // The child process AND its announced ACP session cwd must both get the + // ABSOLUTE form — DSH's own ACP server rejects a relative session cwd, and + // deferring resolution to spawn would hide the launch-dir dependency. + const relative = 'packages/subagent/subagent-acp' + const absolute = resolve(relative) + const ctx = new Context() + await ctx.plugin(SubagentService) + await ctx.plugin(acp, { + providerName: 'acp', + command: process.execPath, + args: [mockServer], + cwd: relative, + permission: 'reject', + env: { MOCK_ECHO_CWD: '1' }, + }) + const run = await ctx.subagents.start('acp', request()) + const result = await run.result + await run.dispose() + expect(text(result.output)).toBe(`${realpathSync(absolute)}\n${absolute}`) + }) + + it('rejects a config cwd that is not an accessible directory at load', async () => { + const ctx = new Context() + await ctx.plugin(SubagentService) + await expect(ctx.plugin(acp, { + providerName: 'acp', + command: 'true', + args: [], + cwd: '/nonexistent/acp-child-workspace', + permission: 'reject', + env: {}, + })).rejects.toThrow('not an accessible directory') + await ctx.fiber.dispose() + }) + + it('rejects a parent session cwd that is not absolute', async () => { + // SessionHeader documents cwd as absolute; a relative value here is a broken + // header, and resolving it against the server process cwd would silently + // re-introduce the launch-directory dependency this resolution removes. + const ctx = await setup({}) + const parent = { id: 'parent', session: { header: { cwd: 'relative/workspace' } } } as unknown as Agent + await expect(ctx.subagents.start('acp', { prompt: [{ type: 'text' as const, text: 'p' }], parent, signal: new AbortController().signal })) + .rejects.toThrow('must be an absolute path') + }) + + 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') + try { + const ctx = new Context() + await ctx.plugin(SubagentService) + await ctx.plugin(acp, { providerName: 'acp', command: 'touch', args: [sentinel], permission: 'reject', env: {} }) + const parent = { id: 'parent', session: { header: { cwd: join(tmp, 'vanished') } } } 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') + expect(existsSync(sentinel)).toBe(false) + } finally { + rmSync(tmp, { recursive: true, force: true }) + } + }) +}) + describe('dsh-subagent-acp', () => { it('drives child processes with parent-unique run ids and returns streamed output', async () => { const ctx = await setup({ MOCK_TEXT: 'hello from acp child', MOCK_STOP: 'end_turn', MOCK_SESSION_ID: 'acp-child-session' }) From b758a7496b759b34e429fd6ad300e79efaa88d40 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Sun, 19 Jul 2026 15:56:32 +0800 Subject: [PATCH 2/6] fix(subagent-acp): reject an empty config cwd at load path.resolve('') is the process cwd, so an empty configured cwd would silently reintroduce the launch-directory fallback the parent-session cwd resolution removed. Fail at plugin load with an actionable message instead. --- docs/config-catalog.md | 10 +++++----- packages/subagent/subagent-acp/README.md | 2 +- packages/subagent/subagent-acp/src/index.ts | 15 ++++++++++----- .../subagent-acp/tests/subagent-acp.spec.ts | 16 ++++++++++++++++ 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index d60bbeb727..87808de52e 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -896,11 +896,11 @@ export interface Config { /** Arguments passed to {@link command}. */ args: string[] /** - * Working directory override for the child process and its ACP session. A - * relative path resolves against the harness launch directory at load, and - * the result must be an existing directory. When omitted, each child - * inherits its delegating parent session's cwd — and starting one from a - * parent session that has no cwd fails. + * Working directory override for the child process and its ACP session. + * Must be non-empty; a relative path resolves against the harness launch + * directory at load, and the result must be an existing directory. When + * omitted, each child inherits its delegating parent session's cwd — and + * starting one from a parent session that has no cwd fails. */ cwd?: string /** diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index 839bfd109f..ca13967558 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -25,7 +25,7 @@ ACP advertises no start-time capabilities because this process cannot enforce th | `providerName` | `acp` | Registry name on `ctx.subagents`. | | `command` | required | Executable spawned for each run. | | `args` | `[]` | Command arguments. | -| `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; a relative value resolves against the harness launch directory at load and must name an existing directory. | +| `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; must be non-empty, a relative value resolves against the harness launch directory at load, and the result must name an existing directory. | | `permission` | `reject` | Auto-answer permission requests by rejecting or choosing the first allow-shaped option. | | `env` | `{}` | Explicit child environment layered over a credential-scrubbed parent environment. | | `disposeEofGraceMs` | `6000` | Grace after stdin EOF before SIGTERM. | diff --git a/packages/subagent/subagent-acp/src/index.ts b/packages/subagent/subagent-acp/src/index.ts index 3722911f28..e8f906ffcf 100644 --- a/packages/subagent/subagent-acp/src/index.ts +++ b/packages/subagent/subagent-acp/src/index.ts @@ -26,11 +26,11 @@ export interface Config { /** Arguments passed to {@link command}. */ args: string[] /** - * Working directory override for the child process and its ACP session. A - * relative path resolves against the harness launch directory at load, and - * the result must be an existing directory. When omitted, each child - * inherits its delegating parent session's cwd — and starting one from a - * parent session that has no cwd fails. + * Working directory override for the child process and its ACP session. + * Must be non-empty; a relative path resolves against the harness launch + * directory at load, and the result must be an existing directory. When + * omitted, each child inherits its delegating parent session's cwd — and + * starting one from a parent session that has no cwd fails. */ cwd?: string /** @@ -160,6 +160,11 @@ export function apply(ctx: Context, config: Config): void { const resolved = config as ResolvedConfig assertPositiveFinite('disposeEofGraceMs', resolved.disposeEofGraceMs) assertPositiveFinite('disposeGraceMs', resolved.disposeGraceMs) + // `path.resolve('')` is the process cwd — an empty string would silently + // reintroduce the launch-directory fallback this resolution removed. + if (resolved.cwd === '') { + throw new Error('subagent-acp: config cwd must not be empty — omit the key to inherit the parent session cwd') + } // Interpret a relative configured cwd against the harness launch directory // ONCE, at load, and fail a misconfigured directory here — not per start. const validated: ResolvedConfig = resolved.cwd === undefined diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 9e32e3acf3..8463900681 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -199,6 +199,22 @@ describe('cwd resolution', () => { expect(text(result.output)).toBe(`${realpathSync(absolute)}\n${absolute}`) }) + it('rejects an empty config cwd at load', async () => { + // `path.resolve('')` is the process cwd, so an empty string would silently + // reintroduce the launch-directory fallback this resolution removed. + const ctx = new Context() + await ctx.plugin(SubagentService) + await expect(ctx.plugin(acp, { + providerName: 'acp', + command: 'true', + args: [], + cwd: '', + permission: 'reject', + env: {}, + })).rejects.toThrow('config cwd must not be empty') + await ctx.fiber.dispose() + }) + it('rejects a config cwd that is not an accessible directory at load', async () => { const ctx = new Context() await ctx.plugin(SubagentService) From fcbbd22af7c80a6c47d2be5291fc2bac8d4546c0 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 21 Jul 2026 16:51:32 +0800 Subject: [PATCH 3/6] fix(subagent-acp): require search permission on the child cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit statSync().isDirectory() is true for a mode-600 directory, but a subprocess cwd needs X_OK — spawn would fail EACCES, deferring a self-contained configuration error past the load/start boundary the resolution owns. Probe accessSync(X_OK) alongside the type check so "not an accessible directory" is literally what the guard verifies. --- packages/subagent/subagent-acp/src/index.ts | 17 +++++++++---- .../subagent-acp/tests/subagent-acp.spec.ts | 25 ++++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/subagent/subagent-acp/src/index.ts b/packages/subagent/subagent-acp/src/index.ts index e8f906ffcf..4aaf9bc071 100644 --- a/packages/subagent/subagent-acp/src/index.ts +++ b/packages/subagent/subagent-acp/src/index.ts @@ -7,7 +7,7 @@ * @module @deepseek-ai/dsh-subagent-acp */ -import { statSync } from 'node:fs' +import { accessSync, constants, statSync } from 'node:fs' import { isAbsolute, resolve } from 'node:path' import type { Context } from 'cordis' import z from 'schemastery' @@ -77,13 +77,20 @@ function assertPositiveFinite(name: string, value: number): void { /** The shape after schemastery applied the defaults (cwd has none). */ type ResolvedConfig = Required> & Pick -/** Whether `path` names an existing, accessible directory. */ +/** + * Whether `path` names an existing directory the harness can ENTER. The + * search-permission probe matters: `statSync().isDirectory()` is true for a + * mode-600 directory, but a subprocess cwd needs `X_OK` or spawn fails EACCES. + */ function isDirectory(path: string): boolean { try { - return statSync(path).isDirectory() + if (!statSync(path).isDirectory()) return false + accessSync(path, constants.X_OK) + return true } catch { - // statSync throws only filesystem access errors here (ENOENT/EACCES/ENOTDIR/…), - // and every one of them means the path cannot serve as the child's cwd. + // statSync/accessSync throw only filesystem access errors here + // (ENOENT/EACCES/ENOTDIR/…), and every one of them means the path cannot + // serve as the child's cwd. return false } } diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index 8463900681..715b0d7caf 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import { Context } from 'cordis' import Loader from '@cordisjs/plugin-loader' -import { existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' @@ -215,6 +215,29 @@ describe('cwd resolution', () => { await ctx.fiber.dispose() }) + it('rejects a config cwd directory without search permission at load', async () => { + // statSync().isDirectory() is true for a mode-600 directory, but a + // subprocess cwd needs SEARCH permission — spawn would fail EACCES. + const tmp = mkdtempSync(join(tmpdir(), 'acp-noexec-')) + chmodSync(tmp, 0o600) + try { + const ctx = new Context() + await ctx.plugin(SubagentService) + await expect(ctx.plugin(acp, { + providerName: 'acp', + command: 'true', + args: [], + cwd: tmp, + permission: 'reject', + env: {}, + })).rejects.toThrow('not an accessible directory') + await ctx.fiber.dispose() + } finally { + chmodSync(tmp, 0o700) + rmSync(tmp, { recursive: true, force: true }) + } + }) + it('rejects a config cwd that is not an accessible directory at load', async () => { const ctx = new Context() await ctx.plugin(SubagentService) From 04117b58bd1515642f61e3bfa2e433ef021a952e Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 21 Jul 2026 16:51:49 +0800 Subject: [PATCH 4/6] docs(subagent): align the parent contract with ACP cwd inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SubagentStartRequest.parent, its core-data type-equiv mirror, and the ACP backend Agent Note still said the out-of-process backend ignores request.parent — no longer true: with cwd unconfigured it reads exactly the session header's cwd, and a parent session without one now rejects at start. State the one-field read at each owning contract, record the workspace-resolution decision in the Agent Note, and align the README's accessibility wording with the search-permission guard. --- .../implemented/feature/2026-06-22-acp-subagent-backend.md | 7 ++++++- docs/core-data-structures/subagent.md | 5 ++++- packages/subagent/subagent-acp/README.md | 6 +++--- packages/subagent/subagent/src/types.ts | 5 ++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md index 1c5b5b533d..d1b61b0af8 100644 --- a/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md +++ b/.agents/notes/implemented/feature/2026-06-22-acp-subagent-backend.md @@ -20,7 +20,11 @@ The client advertises NO optional capabilities (no `fs`, no `terminal`): the chi ### No start-time capabilities -The provider's `capabilities` are all `false`. An out-of-process child cannot honor the parent's `maxDepth` (it has no access to `parent.options.subagentDepth`) or `toolFilter` (it owns its own tool registry), and the first cut does not implement `outputSchema`. The service rejects a request needing any of them before `start` runs. The backend injects only `subagents` (not `ctx.agents`) and ignores `request.parent`. +The provider's `capabilities` are all `false`. An out-of-process child cannot honor the parent's `maxDepth` (it has no access to `parent.options.subagentDepth`) or `toolFilter` (it owns its own tool registry), and the first cut does not implement `outputSchema`. The service rejects a request needing any of them before `start` runs. The backend injects only `subagents` (not `ctx.agents`); the ONE thing it reads off `request.parent` is the session header's cwd (see the workspace resolution below) — no conversation context, depth, or tool state crosses the process boundary. + +### Workspace cwd resolution + +The child's working directory is an explicit resolution, never the harness process cwd: the deployment `cwd` override when configured (made absolute against the launch directory and validated at load), else the parent session header's cwd (validated at start), and a loud rejection before anything spawns when neither exists. One ACP server process serves sessions from many workspaces, so `process.cwd()` cannot stand in for a session's workspace — the old implicit fallback ran children in the server's launch directory. A candidate must be an absolute path naming a directory the harness can ENTER (`X_OK` — `statSync().isDirectory()` alone accepts a mode-600 directory that spawn would fail with EACCES), and the same resolved path becomes both the subprocess cwd and the ACP `session/new` workspace. ### StopReason mapping @@ -33,6 +37,7 @@ The child is a separate process, so it inherits an environment. Credential-shape ## Testing - **Keyless unit/integration:** A scripted ACP subprocess exercises real stdio for prompt/output flow, every stop-reason mapping, signal and disposal cancellation (including pre-abort, pre-session race, and torn-pipe cases), both permission policies, ignored non-message updates, missing-command cleanup, provider reload, and namespace exports. +- **Keyless Loader composition:** A test-only cordis.yml boots the stdio app through the real Loader with the backend's `cwd` omitted; a scripted model delegates once and the scripted child proves it ran in — and was announced — the parent session's workspace (the cwd-inheritance branch end to end). - **With-key e2e:** The backend spawns the real ACP example; its model answers `PONG`, writes `proof.txt`, and the parent verifies the file. - **Snapshot gap:** Each ACP child is a separate process with its own replay session, unlike in-process per-session replay. Deterministic mock-server coverage exists, while `TODO(acp-subagent-replay)` tracks parent replay against a replaying child. diff --git a/docs/core-data-structures/subagent.md b/docs/core-data-structures/subagent.md index 54e79aca68..e2c751b54f 100644 --- a/docs/core-data-structures/subagent.md +++ b/docs/core-data-structures/subagent.md @@ -49,7 +49,10 @@ interface SubagentStartRequest { * The spawning ("parent") agent — the one whose tool call started this * subagent. REQUIRED: in-process backends read `parent.session.header` for * the working directory, the `parentSession` lineage to stamp on the child, - * and the parent's delegation depth. Out-of-process backends (ACP) ignore it. + * and the parent's delegation depth. The out-of-process backend (ACP) reads + * exactly one field — the session header's cwd, the child's workspace when + * no deployment `cwd` override is configured; nothing else crosses the + * process boundary. */ readonly parent: Agent /** diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index d84887f7b0..fd3f0fb5a4 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -6,7 +6,7 @@ The ACP provider runs each subagent in a fresh subprocess and drives it as an Ag `start(request)` resolves the child's working directory, then performs `spawn` → ACP `initialize` → `newSession` before it fulfills. Fulfillment therefore means a remote session is ready and ownership has transferred to the caller. A spawn, initialization, new-session, or pre-publication cancellation failure rejects only after the subprocess has been reaped; a working-directory resolution failure rejects before anything is spawned. -The working directory is the configured `cwd` override when set, else the delegating parent session's cwd — never the server process's own cwd, because one server process serves sessions from many workspaces. The parent-derived value must be an absolute path naming an existing directory, and the same resolved path becomes both the subprocess cwd and the ACP `session/new` workspace. +The working directory is the configured `cwd` override when set, else the delegating parent session's cwd — never the server process's own cwd, because one server process serves sessions from many workspaces. The parent-derived value must be an absolute path naming a directory the harness can enter (search permission — what a subprocess cwd needs), and the same resolved path becomes both the subprocess cwd and the ACP `session/new` workspace. The returned run id is minted in the parent namespace. The child server's session id remains private to ACP wire calls because ACP guarantees it only within that fresh child process; using it as the parent lifecycle id could collide with another remote run or a local agent. @@ -25,7 +25,7 @@ ACP advertises no start-time capabilities because this process cannot enforce th | `providerName` | `acp` | Registry name on `ctx.subagents`. | | `command` | required | Executable spawned for each run. | | `args` | `[]` | Command arguments. | -| `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; must be non-empty, a relative value resolves against the harness launch directory at load, and the result must name an existing directory. | +| `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; must be non-empty, a relative value resolves against the harness launch directory at load, and the result must name a directory the harness can enter. | | `permission` | `reject` | Auto-answer permission requests by rejecting or choosing the first allow-shaped option. | | `env` | `{}` | Explicit child environment layered over a credential-scrubbed parent environment. | | `disposeEofGraceMs` | `6000` | Grace after stdin EOF before SIGTERM. | @@ -59,7 +59,7 @@ The child environment is built by [`buildChildEnv`](../subagent-subprocess/READM The package has no default export. Cordis loader unwrapping would otherwise hide the named `inject` metadata; see [postmortem 0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md). -Keyless tests drive a scripted ACP subprocess over real stdio. The with-key e2e drives the repository's real ACP agent and self-skips without `DEEPSEEK_API_KEY`. +Keyless tests drive a scripted ACP subprocess over real stdio, including a Loader-composed stdio app proving parent-session cwd inheritance end to end. The with-key e2e drives the repository's real ACP agent and self-skips without `DEEPSEEK_API_KEY`. ## Model Experience diff --git a/packages/subagent/subagent/src/types.ts b/packages/subagent/subagent/src/types.ts index 1b1645d89b..7031bd0ad3 100644 --- a/packages/subagent/subagent/src/types.ts +++ b/packages/subagent/subagent/src/types.ts @@ -56,7 +56,10 @@ export interface SubagentStartRequest { * The spawning ("parent") agent — the one whose tool call started this * subagent. REQUIRED: in-process backends read `parent.session.header` for * the working directory, the `parentSession` lineage to stamp on the child, - * and the parent's delegation depth. Out-of-process backends (ACP) ignore it. + * and the parent's delegation depth. The out-of-process backend (ACP) reads + * exactly one field — the session header's cwd, the child's workspace when + * no deployment `cwd` override is configured; nothing else crosses the + * process boundary. */ readonly parent: Agent /** From 822651a2c72677d82e02554201040e26c9a9d4ea Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 21 Jul 2026 16:51:51 +0800 Subject: [PATCH 5/6] test(subagent-acp): cover inherited cwd through a Loader composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inheritance branch ran only under hand-mounted plugin tests, and the with-key e2e always configures an explicit cwd. Add a test-only cordis.yml (scripted delegating model + the scripted mock ACP child) booting the stdio app through the real Loader with cwd omitted: the child proves it ran in — and was announced — the parent session's workspace, keylessly. --- .../fixtures/subagent/subagent-acp/cordis.yml | 37 +++++++ .../subagent-acp/mock-delegating-llm.ts | 48 +++++++++ examples/package.json | 1 + knip.json | 1 + .../tests/loader-composition.e2e.ts | 101 ++++++++++++++++++ pnpm-lock.yaml | 3 + 6 files changed, 191 insertions(+) create mode 100644 examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml create mode 100644 examples/acp-agent/tests/fixtures/subagent/subagent-acp/mock-delegating-llm.ts create mode 100644 packages/subagent/subagent-acp/tests/loader-composition.e2e.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 new file mode 100644 index 0000000000..76ac73078f --- /dev/null +++ b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/cordis.yml @@ -0,0 +1,37 @@ +# Test-only composition: the ACP subagent backend on the real Loader/app path. +# The scripted model delegates once; the scripted mock ACP child (MOCK_ECHO_CWD) +# echoes its process cwd and announced session cwd, so parent-session cwd +# inheritance is asserted keylessly end to end. `cwd` is deliberately omitted — +# the inheritance branch under test. The child command path is machine-absolute, +# so the driving e2e supplies it via DSH_TEST_MOCK_ACP_SERVER. +- id: mock-llm + name: './mock-delegating-llm.ts' + +- id: subagent + name: '@deepseek-ai/dsh-subagent' + +- id: subagent-acp + name: '@deepseek-ai/dsh-subagent-acp' + config: + providerName: acp + command: !!js process.execPath + args: + - !!js process.env.DSH_TEST_MOCK_ACP_SERVER + permission: reject + env: + MOCK_ECHO_CWD: '1' + +- id: tool-subagent + name: '@deepseek-ai/dsh-tool-subagent' + config: + provider: acp + toolName: subagent + +- id: stdio-agent + name: '@deepseek-ai/dsh-stdio-demo' + config: + provider: mock + model: mock-delegate + welcome: 'acp subagent cwd e2e ready.' + persistenceRoot: './.sessions' + workspaceContext: false diff --git a/examples/acp-agent/tests/fixtures/subagent/subagent-acp/mock-delegating-llm.ts b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/mock-delegating-llm.ts new file mode 100644 index 0000000000..9d3857ffc8 --- /dev/null +++ b/examples/acp-agent/tests/fixtures/subagent/subagent-acp/mock-delegating-llm.ts @@ -0,0 +1,48 @@ +import type { Context } from 'cordis' +import type { GenerateOptions, StreamChunk } from '@deepseek-ai/dsh-llm' +import { CallId, LlmAdapter } from '@deepseek-ai/dsh-llm' + +/** + * Test adapter for the `mock-delegate` model: the first request calls the + * `subagent` tool once, and the follow-up streams the tool result text back + * verbatim — so the ACP child's answer (the scripted mock server's cwd echo) + * reaches the REPL stdout for the driving e2e to assert. + */ +class MockDelegatingAdapter extends LlmAdapter { + async * stream(options: GenerateOptions): AsyncIterable { + const toolResultText = options.messages.at(-1)?.content + .filter(block => block.type === 'tool-result') + .flatMap(block => block.content) + .filter(block => block.type === 'text') + .map(block => block.text) + .join('') ?? '' + + if (toolResultText.length === 0) { + const args = JSON.stringify({ description: 'cwd probe', prompt: 'report your workspace' }) + yield { type: 'block-start', index: 0, blockType: 'tool-call' } + yield { type: 'tool-call-delta', index: 0, id: CallId('call-delegate'), name: 'subagent', argumentsDelta: args } + yield { type: 'block-end', index: 0, block: { type: 'tool-call', id: CallId('call-delegate'), name: 'subagent', arguments: args } } + yield { type: 'usage', usage: { inputTokens: 10, outputTokens: 5 } } + yield { type: 'finish', reason: { kind: 'tool-calls' } } + return + } + + const reply = `child reported:\n${toolResultText}` + yield { type: 'block-start', index: 0, blockType: 'text' } + yield { type: 'text-delta', index: 0, text: reply } + yield { type: 'block-end', index: 0, block: { type: 'text', text: reply } } + yield { type: 'usage', usage: { inputTokens: 10, outputTokens: reply.length } } + yield { type: 'finish', reason: { kind: 'stop' } } + } +} + +export const name = 'mock-llm' +export const inject = ['llm'] + +/** + * Register the delegating mock adapter under the `mock` provider. + * @param ctx - the plugin context supplying `ctx.llm`. + */ +export function apply(ctx: Context): void { + ctx.llm.registerAdapter(['mock'], new MockDelegatingAdapter()) +} diff --git a/examples/package.json b/examples/package.json index 3a5f90d30e..026968888b 100644 --- a/examples/package.json +++ b/examples/package.json @@ -30,6 +30,7 @@ "@deepseek-ai/dsh-spill-policy": "workspace:*", "@deepseek-ai/dsh-stdio-demo": "workspace:*", "@deepseek-ai/dsh-subagent": "workspace:*", + "@deepseek-ai/dsh-subagent-acp": "workspace:*", "@deepseek-ai/dsh-subagent-fork": "workspace:*", "@deepseek-ai/dsh-subagent-spawn": "workspace:*", "@deepseek-ai/dsh-time-context": "workspace:*", diff --git a/knip.json b/knip.json index 2c3465ab38..f37ceeb3a6 100644 --- a/knip.json +++ b/knip.json @@ -12,6 +12,7 @@ "echo-agent/src/*.ts", "headless-agent/tests/fixtures/cli-mock-llm.ts", "tui-agent/tests/fixtures/tui-scripted-llm.ts", + "acp-agent/tests/fixtures/subagent/subagent-acp/mock-delegating-llm.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 new file mode 100644 index 0000000000..bdebf64fbb --- /dev/null +++ b/packages/subagent/subagent-acp/tests/loader-composition.e2e.ts @@ -0,0 +1,101 @@ +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 { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { afterEach, describe, expect, it } from 'vitest' +import { resolveExampleLaunch } 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 + * 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 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') + }) +} + +describe('ACP subagent cwd inheritance through a real cordis.yml and stdio process', () => { + it('runs the child in the parent session workspace and announces it as the ACP session cwd', async () => { + const { stdout, stderr, cwd } = await runDelegation() + 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) +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dba3435f60..74cda32326 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -167,6 +167,9 @@ importers: '@deepseek-ai/dsh-subagent': specifier: workspace:* version: link:../packages/subagent/subagent + '@deepseek-ai/dsh-subagent-acp': + specifier: workspace:* + version: link:../packages/subagent/subagent-acp '@deepseek-ai/dsh-subagent-fork': specifier: workspace:* version: link:../packages/subagent/subagent-fork From c0d7ef22ec5408e33a3b34ff91e3fd83c7f03a60 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Tue, 21 Jul 2026 17:14:10 +0800 Subject: [PATCH 6/6] 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')