mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
fix(amr): close ACP stdin on abort so vela tears down OpenCode (#3097)
* 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.
This commit is contained in:
parent
68bbcfe9e2
commit
2302efa5db
2 changed files with 68 additions and 4 deletions
|
|
@ -701,12 +701,27 @@ export function attachAcpSession({
|
||||||
aborted = true;
|
aborted = true;
|
||||||
finished = true;
|
finished = true;
|
||||||
clearStageTimer();
|
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 {
|
try {
|
||||||
sendRpc(child.stdin, nextId, 'session/cancel', { sessionId });
|
child.stdin.end();
|
||||||
nextId += 1;
|
|
||||||
} catch {
|
} catch {
|
||||||
// The caller owns process-signal fallback if the ACP transport is gone.
|
// Best effort; the caller still owns the SIGTERM/SIGKILL fallback.
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -231,6 +231,55 @@ test('attachAcpSession exposes abort and sends session cancel after session crea
|
||||||
assert.deepEqual(cancelRequest.params, { sessionId: 'session-1' });
|
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<Record<string, unknown>> {
|
function parseRpcWrites(writes: string[]): Array<Record<string, unknown>> {
|
||||||
return writes
|
return writes
|
||||||
.join('')
|
.join('')
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue