fix(coding-agent): preserve aborted tombstone across live-session dispose

A live-session hub kill did not stick: release(tombstone) awaited the
wrapped session dispose first, and createAgentSession's unregisterUnlessParked
removed any non-parked ref, so the subsequent detach/setStatus no-oped and the
ref was gone — leaving the reopen resurrection for idle/running agents.

- release(tombstone) now marks the ref `aborted` BEFORE disposing, so the
  dispose guard preserves it; the session is detached afterward.
- unregisterUnlessParked now also spares terminal `aborted` refs (matching
  the documented "hard-killed, terminal" retention and finalizeSubagentLifecycle).
- Regression test now uses a session stub that mirrors the real wrapped
  dispose (unregister unless parked/aborted), so it fails if the tombstone is
  set after dispose.

Fixes #7250
This commit is contained in:
roboomp
2026-08-01 09:34:43 +00:00
parent 65fd8cb0e8
commit d350dd8f84
3 changed files with 28 additions and 7 deletions
@@ -378,6 +378,14 @@ export class AgentLifecycleManager {
await park.promise;
}
if (options?.tombstone) {
// Mark the tombstone terminal BEFORE disposing. A live session's wrapped
// dispose (createAgentSession) unregisters any non-terminal ref via
// `unregisterUnlessParked`, which would otherwise delete the ref out from
// under the detach/status calls below. Setting `aborted` first makes that
// guard preserve the ref, so a later persisted-subagent rescan skips it.
this.#registry.setStatus(id, "aborted", ref);
}
if (this.#registry.get(id) === ref && ref.session) {
try {
await ref.session.dispose();
@@ -387,7 +395,6 @@ export class AgentLifecycleManager {
}
if (options?.tombstone) {
this.#registry.detachSession(id, ref);
this.#registry.setStatus(id, "aborted", ref);
} else {
this.#registry.unregister(id, ref);
}
+4 -3
View File
@@ -1634,13 +1634,14 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
let registeredAgentRef: AgentRef | undefined;
/**
* Forget the agent ref on teardown — unless the agent is being parked (or is
* already parked). Parking disposes the session but keeps the ref addressable
* (history://, revive); only process teardown / explicit kill unregisters.
* already parked/aborted). Parking disposes the session but keeps the ref
* addressable (history://, revive); a hard kill leaves it as a terminal
* `aborted` tombstone. Only process teardown / a plain release unregisters.
*/
const unregisterUnlessParked = (): void => {
const ref = registeredAgentRef;
if (!ref || agentRegistry.get(resolvedAgentId) !== ref) return;
if (ref.status === "parked") return;
if (ref.status === "parked" || ref.status === "aborted") return;
if (AgentLifecycleManager.global().isParking(resolvedAgentId, ref)) return;
agentRegistry.unregister(resolvedAgentId, ref);
};
@@ -524,12 +524,25 @@ describe("AgentLifecycleManager", () => {
await Bun.write(rootSessionFile, "");
await Bun.write(workerSessionFile, "");
const stub = makeSessionStub();
// Mirror the real wrapped session dispose (createAgentSession's
// `unregisterUnlessParked`): disposing a live session unregisters the ref
// unless it is already terminal (parked/aborted). This is what defeated the
// naive fix — the ref must be marked `aborted` *before* dispose runs.
let disposeCalls = 0;
const session = {
dispose: async () => {
disposeCalls++;
const live = registry.get(workerId);
if (live && live.status !== "parked" && live.status !== "aborted") {
registry.unregister(workerId, live);
}
},
} as unknown as AgentSession;
const ref = registry.register({
id: workerId,
displayName: "task",
kind: "sub",
session: stub.session,
session,
sessionFile: workerSessionFile,
status: "running",
});
@@ -537,7 +550,7 @@ describe("AgentLifecycleManager", () => {
expect(await lifecycle.release(workerId, ref, { tombstone: true })).toBe(true);
// The kill disposes the live session but keeps the ref registered as a
// terminal, hard-killed row (session detached) instead of removing it.
expect(stub.disposeCalls()).toBe(1);
expect(disposeCalls).toBe(1);
expect(registry.get(workerId)?.status).toBe("aborted");
expect(registry.get(workerId)?.session).toBeNull();