From 2302efa5db0776a4364f92584196d6380c2f9404 Mon Sep 17 00:00:00 2001 From: lefarcen <935902669@qq.com> Date: Wed, 27 May 2026 23:40:22 +0800 Subject: [PATCH] fix(amr): close ACP stdin on abort so vela tears down OpenCode (#3097) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(amr): close ACP stdin on abort so vela tears down OpenCode When an AMR (vela) run is cancelled, attachAcpSession.abort() sent a `session/cancel` RPC but left the child's stdin open. The vela ACP bridge keeps running until it sees EOF (or is signaled), and it only shuts down its private OpenCode `serve` process on a clean exit — so on abort the OpenCode server lingered until the caller's SIGTERM fallback, and leaked entirely if the parent was killed before cleanup ran. End stdin after sending the cancel (mirroring the clean-completion path) so the agent receives EOF and shuts down its own runtime promptly, independent of signal timing. * fix(amr): end stdin on abort even before session/new resolves Addresses review on #3097: abort() still returned early when sessionId was unset, so the stdin EOF only happened after session/new completed. Cancelling during ACP startup (before the session exists) left the OpenCode-teardown window open until the caller's SIGTERM fallback — and a parent hard-kill before that could still strand the private OpenCode process. Move stdin.end() out of the sessionId guard so abort always closes stdin when the pipe is writable; gate only the session/cancel RPC on sessionId. Add a regression test that aborts during startup and asserts stdin is ended with no session/cancel emitted. --- apps/daemon/src/acp.ts | 23 +++++++++++++--- apps/daemon/tests/acp.test.ts | 49 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/apps/daemon/src/acp.ts b/apps/daemon/src/acp.ts index 7fee6fbca..b8597ef08 100644 --- a/apps/daemon/src/acp.ts +++ b/apps/daemon/src/acp.ts @@ -701,12 +701,27 @@ export function attachAcpSession({ aborted = true; finished = true; clearStageTimer(); - if (!sessionId || !child.stdin || child.stdin.destroyed || child.stdin.writableEnded) return; + if (!child.stdin || child.stdin.destroyed || child.stdin.writableEnded) + return; + // Only cancel an established session; before session/new resolves there + // is no sessionId to cancel, but we must still close stdin below. + if (sessionId) { + try { + sendRpc(child.stdin, nextId, 'session/cancel', { sessionId }); + nextId += 1; + } catch { + // The caller owns process-signal fallback if the ACP transport is gone. + } + } + // Always close stdin so the agent receives EOF and shuts down its own + // runtime — the vela ACP bridge tears down its private OpenCode server on + // EOF — instead of lingering (and leaking that server) until the caller's + // SIGTERM fallback fires. This also covers aborts during ACP startup, + // before session/new returns. Mirrors the clean-completion path above. try { - sendRpc(child.stdin, nextId, 'session/cancel', { sessionId }); - nextId += 1; + child.stdin.end(); } catch { - // The caller owns process-signal fallback if the ACP transport is gone. + // Best effort; the caller still owns the SIGTERM/SIGKILL fallback. } }, }; diff --git a/apps/daemon/tests/acp.test.ts b/apps/daemon/tests/acp.test.ts index a029afd4a..b85f9d41c 100644 --- a/apps/daemon/tests/acp.test.ts +++ b/apps/daemon/tests/acp.test.ts @@ -231,6 +231,55 @@ test('attachAcpSession exposes abort and sends session cancel after session crea assert.deepEqual(cancelRequest.params, { sessionId: 'session-1' }); }); +test('attachAcpSession.abort closes stdin so the agent shuts down on EOF', () => { + const child = new FakeAcpChild(); + + const session = attachAcpSession({ + child: child as never, + prompt: 'hello', + cwd: '/tmp/od-project', + model: null, + mcpServers: [], + send: () => {}, + }); + + child.stdout.write(`${JSON.stringify({ id: 1, result: {} })}\n`); + child.stdout.write(`${JSON.stringify({ id: 2, result: { sessionId: 'session-1' } })}\n`); + + assert.equal(child.stdin.writableEnded, false); + session.abort(); + // EOF on stdin lets the vela ACP bridge tear down its OpenCode server + // without waiting for the caller's SIGTERM fallback. + assert.equal(child.stdin.writableEnded, true); +}); + +test('attachAcpSession.abort during startup ends stdin without sending session/cancel', () => { + const child = new FakeAcpChild(); + const writes: string[] = []; + child.stdin.on('data', (chunk) => writes.push(String(chunk))); + + const session = attachAcpSession({ + child: child as never, + prompt: 'hello', + cwd: '/tmp/od-project', + model: null, + mcpServers: [], + send: () => {}, + }); + + // Abort before session/new resolves (no sessionId yet) — e.g. the user + // cancels during ACP startup. stdin must still close so OpenCode tears down. + assert.equal(child.stdin.writableEnded, false); + session.abort(); + assert.equal(child.stdin.writableEnded, true); + + // No session to cancel yet, so no session/cancel RPC should be emitted. + const cancelRequests = parseRpcWrites(writes).filter( + (entry) => entry.method === 'session/cancel', + ); + assert.equal(cancelRequests.length, 0); +}); + function parseRpcWrites(writes: string[]): Array> { return writes .join('')