diff --git a/packages/subagent/subagent-acp/README.i18n.yaml b/packages/subagent/subagent-acp/README.i18n.yaml index 5bec6571f6..d8012da7b3 100644 --- a/packages/subagent/subagent-acp/README.i18n.yaml +++ b/packages/subagent/subagent-acp/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/subagent/subagent-acp/README.md -README.md: 6366a224b8f56f0b86466d6946f75ab45fee3997 -README.zh.md: 490c17dd883224bcfe129bd3b29eea973a4e69c7 +README.md: 00f084c8252c001d98d04c8ccc1dff5f14976683 +README.zh.md: 3b820805d23b39757376edc14a3d75860f2e8eae diff --git a/packages/subagent/subagent-acp/README.md b/packages/subagent/subagent-acp/README.md index 6366a224b8..00f084c825 100644 --- a/packages/subagent/subagent-acp/README.md +++ b/packages/subagent/subagent-acp/README.md @@ -31,7 +31,7 @@ ACP advertises no start-time capabilities because this process cannot enforce th | `permission` | `reject` | Auto-answer permission requests by rejecting or choosing the first `allow_once` or `allow_always` option. | | `env` | `{}` | Explicit child environment layered over a credential-scrubbed parent environment. | | `disposeEofGraceMs` | `6000` | Positive grace after stdin EOF before platform termination; it cannot exceed [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md). | -| `disposeGraceMs` | `3000` | Positive POSIX grace after SIGTERM before SIGKILL (Windows force-terminates directly); it cannot exceed [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md). | +| `disposeGraceMs` | `3000` | Positive bound for observing structured process facts after failure and, on POSIX, the SIGTERM-to-SIGKILL grace (Windows force-terminates directly); it cannot exceed [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.md). | ```yaml - id: subagent-acp diff --git a/packages/subagent/subagent-acp/README.zh.md b/packages/subagent/subagent-acp/README.zh.md index 490c17dd88..3b820805d2 100644 --- a/packages/subagent/subagent-acp/README.zh.md +++ b/packages/subagent/subagent-acp/README.zh.md @@ -31,7 +31,7 @@ ACP 不声明任何启动时能力，因为当前进程无法强制执行远程 | `permission` | `reject` | 自动回答权限请求：拒绝，或选择第一个 `allow_once` 或 `allow_always` 选项。 | | `env` | `{}` | 显式子进程环境，叠加到已清理凭据的父进程环境之上。 | | `disposeEofGraceMs` | `6000` | stdin EOF 之后、平台终止之前的宽限时间须为正值，且不得大于 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.zh.md)。 | -| `disposeGraceMs` | `3000` | POSIX 在 SIGTERM 后、SIGKILL 前的宽限时间（Windows 直接强制终止），须为正值且不得大于 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.zh.md)。 | +| `disposeGraceMs` | `3000` | 失败后观测结构化进程事实的正数时限；在 POSIX 上也作为 SIGTERM 到 SIGKILL 的宽限时间（Windows 直接强制终止），且不得大于 [`MAX_TIMER_DELAY_MS`](../../util/timeout/README.zh.md)。 | ```yaml - id: subagent-acp diff --git a/packages/subagent/subagent-acp/src/index.ts b/packages/subagent/subagent-acp/src/index.ts index 7cbe1c79c9..debae6b0f2 100644 --- a/packages/subagent/subagent-acp/src/index.ts +++ b/packages/subagent/subagent-acp/src/index.ts @@ -59,7 +59,7 @@ export interface Config { * `MAX_TIMER_DELAY_MS`. */ disposeEofGraceMs?: number - /** Termination-escalation grace (ms); must not exceed `MAX_TIMER_DELAY_MS`. */ + /** Failure-observation and termination-escalation grace (ms); must not exceed `MAX_TIMER_DELAY_MS`. */ disposeGraceMs?: number } @@ -74,7 +74,7 @@ export const Config: z = z.object({ disposeGraceMs: z.number().default(DEFAULT_DISPOSE_GRACE_MS), }) -/** A dispose grace must fit the single Node timer that owns its teardown tier. */ +/** A process grace must fit every Node timer that observes or terminates the child. */ function assertPositiveFinite(name: string, value: number): void { if (!Number.isFinite(value) || value <= 0 || value > MAX_TIMER_DELAY_MS) { throw new Error(`subagent-acp: ${name} must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`) diff --git a/packages/subagent/subagent-acp/src/run.ts b/packages/subagent/subagent-acp/src/run.ts index 11737fe78f..1cc76683cb 100644 --- a/packages/subagent/subagent-acp/src/run.ts +++ b/packages/subagent/subagent-acp/src/run.ts @@ -61,9 +61,11 @@ export interface AcpRunSpec { */ disposeEofGraceMs: number /** - * Termination-escalation grace (ms) in {@link SubagentRun.dispose}; POSIX - * waits this long after `SIGTERM` before `SIGKILL`, while Windows - * force-terminates directly. The plugin fills it from `disposeGraceMs`. + * Process-observation and termination-escalation grace (ms). Failure + * classification waits at most this long for structured exit facts; POSIX + * dispose also waits this long after `SIGTERM` before `SIGKILL`, while + * Windows force-terminates directly. The plugin fills it from + * `disposeGraceMs`. */ disposeGraceMs: number /** @@ -282,7 +284,6 @@ function startupFailure( child: SubprocessHandle, outcome: SubprocessOutcome | undefined, ): AcpRunFailure { - if (error instanceof AcpRunFailure) return error if (child.pid <= 0) { return new AcpRunFailure({ stage: 'process', category: 'process-start' }, error) } @@ -374,11 +375,12 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe ) spawnFailed.catch(() => { /* observed by the startup race; never unhandled */ }) - const observeProcessOutcome = async (): Promise => { + const observeProcessOutcome = async (signal?: AbortSignal): Promise => { if (processOutcome !== undefined || child.pid <= 0) return processOutcome try { + const timeout = AbortSignal.timeout(Math.ceil(spec.disposeGraceMs)) const exited = await child.waitForExit( - AbortSignal.timeout(Math.ceil(spec.disposeGraceMs)), + signal === undefined ? timeout : AbortSignal.any([signal, timeout]), ) if (exited) return await processDone } catch { @@ -495,7 +497,9 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe ? { kind: 'cancelled' } as const : { kind: 'failed', - failure: startupFailure(error, startupStage, child, await observeProcessOutcome()), + failure: error instanceof AcpRunFailure + ? error + : startupFailure(error, startupStage, child, await observeProcessOutcome()), } as const if (startup.kind === 'cancelled') { // Local cancellation owns the startup outcome; only cleanup failure is @@ -550,7 +554,7 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe } } catch (error: unknown) { if (!flags.cancelled) { - const outcome = await observeProcessOutcome() + const outcome = await observeProcessOutcome(request.signal) const facts = outcome === undefined ? { stage: 'prompt', category: 'transport' } as const : { stage: 'process', category: 'process-exit', outcome } as const diff --git a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts index b8b40a3750..3bbf75128a 100644 --- a/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts +++ b/packages/subagent/subagent-acp/tests/subagent-acp.spec.ts @@ -105,6 +105,22 @@ function rejectFinalExitWaitAfterExit(child: SubprocessHandle, message: string): } } +function tapBoundedExitWait(child: SubprocessHandle, onWait: () => void): SubprocessHandle { + return { + pid: child.pid, + stdin: child.stdin, + stdout: child.stdout, + stderr: child.stderr, + collected: child.collected, + done: child.done, + terminate: () => { child.terminate() }, + waitForExit: (signal?: AbortSignal) => { + if (signal !== undefined) onWait() + return child.waitForExit(signal) + }, + } +} + describe('acpStopReason', () => { it('maps each ACP stop reason to the harness vocabulary', () => { expect(acpStopReason('end_turn')).toBe('completed') @@ -593,6 +609,7 @@ describe('dsh-subagent-acp', () => { it('reaps a child whose session/new response omits the session id', async () => { const tmp = mkdtempSync(join(tmpdir(), 'acp-malformed-session-')) const flushed = join(tmp, 'flushed') + let boundedWaits = 0 try { await expect(startAcpRun(request(), { command: process.execPath, @@ -606,13 +623,14 @@ describe('dsh-subagent-acp', () => { }, disposeEofGraceMs: 1000, disposeGraceMs: 100, - spawn: spawnSubprocess, + spawn: spec => tapBoundedExitWait(spawnSubprocess(spec), () => { boundedWaits += 1 }), })).rejects.toThrow( `subagent-acp: ${expectedFailure('stage: new-session; category: protocol')}`, ) // Startup rejects only after its private child reaches quiescence. The // marker proves rollback closed stdin and allowed the child's EOF flush. expect(existsSync(flushed)).toBe(true) + expect(boundedWaits).toBe(1) } finally { rmSync(tmp, { recursive: true, force: true }) } @@ -939,6 +957,30 @@ describe('dsh-subagent-acp', () => { await run.dispose() }) + it('lets local cancellation interrupt prompt-failure process observation', async () => { + const controller = new AbortController() + const observing = Promise.withResolvers() + const run = await startAcpRun(request('p', controller.signal), { + command: process.execPath, + args: [mockServer], + cwd: process.cwd(), + permission: 'reject', + env: { MOCK_CLOSE_PROTOCOL_ON_PROMPT: '1' }, + disposeEofGraceMs: 100, + disposeGraceMs: 5000, + spawn: spec => tapBoundedExitWait(spawnSubprocess(spec), () => { observing.resolve(undefined) }), + }) + await observing.promise + controller.abort() + await expect(Promise.race([ + run.result, + new Promise((_resolve, reject) => { + setTimeout(() => { reject(new Error('cancellation waited for process observation')) }, 500) + }), + ])).resolves.toEqual({ output: [], stopReason: 'aborted' }) + await run.dispose() + }) + it('preserves partial output and structured process facts when the child exits', async () => { const ctx = await setup({ MOCK_TEXT: 'partial answer', MOCK_CRASH_AFTER_CHUNK: '1' }) const run = await ctx.subagents.start('acp', request())