fix(coding-agent): keep hub-killed subagent from resurrecting as parked
The Agent Hub kill path called AgentLifecycleManager.release(), which disposes the session and then unregisters the ref while leaving the on-disk <id>.jsonl intact. On the next hub open, registerPersistedSubagents rescans the transcript tree and its `if (!registry.get(id))` guard cannot distinguish an explicit kill from a normally-parked agent, so it re-adopts the killed id as a fresh `parked` row. Add a `tombstone` option to release() that mirrors finalizeSubagentLifecycle's genuine-kill path: dispose and detach the session but keep the ref registered as terminal `aborted` instead of removing it. The kept-registered id makes the rescan guard skip it, and the transcript stays on disk (still reachable via history://<id>, per #5261). The hub kill button now passes tombstone: true. Fixes #7250
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed a subagent killed from the Agent Hub (`x`) reappearing as a `parked` row after closing and reopening the hub in a local session; the kill now leaves the ref registered as terminal `aborted` instead of unregistering it, so the persisted-subagent rescan no longer re-adopts the surviving transcript ([#7250](https://github.com/can1357/oh-my-pi/issues/7250)).
|
||||
|
||||
## [17.2.3] - 2026-08-01
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -636,7 +636,7 @@ export class AgentHubOverlayComponent extends Container {
|
||||
if (ref.status === "running" && ref.session) {
|
||||
await ref.session.abort({ reason: USER_INTERRUPT_LABEL });
|
||||
}
|
||||
await this.#lifecycle().release(ref.id, ref);
|
||||
await this.#lifecycle().release(ref.id, ref, { tombstone: true });
|
||||
} catch (error) {
|
||||
logger.warn("Agent hub: kill failed", { id: ref.id, error: String(error) });
|
||||
this.#notice = error instanceof Error ? error.message : String(error);
|
||||
|
||||
@@ -345,12 +345,19 @@ export class AgentLifecycleManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* Hard removal: dispose if live, unregister from registry, drop timers.
|
||||
* When `expected` is given, only a ref matching it is released; a stale
|
||||
* release can never take down a newer same-id ref. Returns true when a
|
||||
* matching ref was released.
|
||||
* Dispose if live and drop timers. When `expected` is given, only a ref
|
||||
* matching it is released; a stale release can never take down a newer
|
||||
* same-id ref. Returns true when a matching ref was released.
|
||||
*
|
||||
* By default the ref is unregistered (teardown / one-shot removal). Pass
|
||||
* `tombstone: true` for an explicit kill: the ref is kept registered as a
|
||||
* terminal `aborted` row (session detached) instead of being removed, so a
|
||||
* later persisted-subagent scan (e.g. Agent Hub reopen) skips it via its
|
||||
* `if (!registry.get(id))` guard rather than re-adopting the surviving
|
||||
* on-disk transcript as a fresh `parked` row. Mirrors
|
||||
* `finalizeSubagentLifecycle`'s genuine-kill path.
|
||||
*/
|
||||
async release(id: string, expected?: AgentRefExpectation): Promise<boolean> {
|
||||
async release(id: string, expected?: AgentRefExpectation, options?: { tombstone?: boolean }): Promise<boolean> {
|
||||
const adopted = this.#adopted.get(id);
|
||||
const current = this.#registry.get(id);
|
||||
const currentMatches =
|
||||
@@ -378,7 +385,12 @@ export class AgentLifecycleManager {
|
||||
logger.warn("AgentLifecycleManager.release: session dispose failed", { id, error: String(error) });
|
||||
}
|
||||
}
|
||||
this.#registry.unregister(id, ref);
|
||||
if (options?.tombstone) {
|
||||
this.#registry.detachSession(id, ref);
|
||||
this.#registry.setStatus(id, "aborted", ref);
|
||||
} else {
|
||||
this.#registry.unregister(id, ref);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { AgentLifecycleManager } from "@oh-my-pi/pi-coding-agent/registry/agent-lifecycle";
|
||||
import { AgentRegistry, MAIN_AGENT_ID } from "@oh-my-pi/pi-coding-agent/registry/agent-registry";
|
||||
import { registerPersistedSubagents } from "@oh-my-pi/pi-coding-agent/registry/persisted-agents";
|
||||
import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
interface SessionStub {
|
||||
session: AgentSession;
|
||||
@@ -512,4 +515,36 @@ describe("AgentLifecycleManager", () => {
|
||||
expect(stub.disposeCalls()).toBe(0);
|
||||
expect(lifecycle.has("8-Sub")).toBe(true);
|
||||
});
|
||||
|
||||
it("tombstone release keeps a killed ref as terminal `aborted` so a persisted-subagent rescan cannot resurrect it as parked", async () => {
|
||||
using tempDir = TempDir.createSync("@omp-lifecycle-tombstone-");
|
||||
const rootSessionFile = path.join(tempDir.path(), "main.jsonl");
|
||||
const workerId = "Killed-Sub";
|
||||
const workerSessionFile = path.join(tempDir.path(), "main", `${workerId}.jsonl`);
|
||||
await Bun.write(rootSessionFile, "");
|
||||
await Bun.write(workerSessionFile, "");
|
||||
|
||||
const stub = makeSessionStub();
|
||||
const ref = registry.register({
|
||||
id: workerId,
|
||||
displayName: "task",
|
||||
kind: "sub",
|
||||
session: stub.session,
|
||||
sessionFile: workerSessionFile,
|
||||
status: "running",
|
||||
});
|
||||
|
||||
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(registry.get(workerId)?.status).toBe("aborted");
|
||||
expect(registry.get(workerId)?.session).toBeNull();
|
||||
|
||||
// Reopening the Agent Hub rescans on-disk transcripts. The surviving
|
||||
// `.jsonl` must not be re-adopted as a fresh `parked` row, because the
|
||||
// id is still present in the registry.
|
||||
await registerPersistedSubagents(registry, rootSessionFile);
|
||||
expect(registry.get(workerId)?.status).toBe("aborted");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user