From 6fd3fa77db28ab6478627e93504583ca5ff1786e Mon Sep 17 00:00:00 2001 From: Brynne Taylor <7542439+brynne8@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:15:41 +0800 Subject: [PATCH] fix(subagents): stop showing an interrupted run as still running A child interrupted by a server restart keeps a persisted `running` status and no result entry. `get_subagent_result` already settled that to `interrupted`, but the session list served the raw file status, so the agent switcher and the subagent viewer spun forever while the top bar, which reads live wrappers, said the run had finished. - Settle a persisted `running` / `queued` status when a subagent row is read from disk, through `settleOrphanedSubagentStatus()`. - Let an alive wrapper's runtime row overlay that status in the merged list, so a genuinely queued child still reads as queued. - Derive the runtime row status from a running wrapper, the live run map, or the settled file, instead of trusting the file alone. - Cover the orphaned listing and the live overlay with tests, and record the rule in the subagents note. --- docs/agents/subagents.md | 1 + lib/rpc-manager.ts | 11 ++++-- lib/rpc-session-info.test.mjs | 43 ++++++++++++++++++++++++ lib/session-reader.test.mjs | 63 +++++++++++++++++++++++++++++++++++ lib/session-reader.ts | 15 +++++++-- lib/subagent-runtime.ts | 8 ++++- lib/subagents.ts | 8 +++++ 7 files changed, 143 insertions(+), 6 deletions(-) diff --git a/docs/agents/subagents.md b/docs/agents/subagents.md index bf3f1d4902..81a69ca167 100644 --- a/docs/agents/subagents.md +++ b/docs/agents/subagents.md @@ -6,6 +6,7 @@ - The global `builtInEnabled` in `~/.pi/agent/agents/settings.json` is off when the file or field is absent and when the file is malformed (fail closed); writes are atomic and keep unknown fields. The inline extension factory is always present, so a reload can apply the switch, but registers no tools while off; after changing it the user must reload the session explicitly, and `Agent` dispatch re-checks the setting so a stale call cannot start a subagent. When on, only a recognized legacy `pi-subagents` extension registering a reserved tool (`Agent`, `get_subagent_result`, `steer_subagent`) is removed, along with the conflict errors it caused; unrelated extensions stay. ADR 0003. - The three tools register with `exposure: "model-only"`, so `ctx.executeTool()` (a codemode script) cannot start, collect or steer a subagent: a script-started run would record the nested call id (`/`) as its `parentToolCallId`, which no transcript entry carries, and the chat would lose its link to the child session. `lib/subagent-extension.integration.test.mjs` pins this against a real `AgentSession`. - A run stays in `getSubagentRuns()` from dispatch until its result entry is written, so `get()` reports a persisted `running` / `queued` status with no result and no running wrapper as `interrupted`: its process stopped and nothing will finish it (otherwise `get_subagent_result({ wait: true })` polls forever after a restart). `start()` writes the child's session file at dispatch (`persistChildSession()`): pi creates one only once a user or assistant message exists, so a run stopped while queued or connecting MCP servers, or refused for missing credentials, kept its entries in memory only and was gone (from `get()`, `resume` and the sidebar) once its idle wrapper was released. `setSessionFile()` reloads the written file, so pi appends every later entry; a queued run a restart cuts off reads as `interrupted` like a running one. The file is the only record of a settled run: one kept in memory as well would outlive a deleted child, and `resume` would reopen its missing file as a new, unrestricted session in the server's cwd. +- The session list settles that status the same way (`settleOrphanedSubagentStatus()` in `mapScannedSession` and `getRpcSessionInfos()`), and a live wrapper's runtime row overlays it in `mergeSessionLists()`: only the live run map (`getLiveSubagentStatus()`) can tell a queued child from an orphan, so the list never spins on a run a restart cut off. - Built-in profiles (`general-purpose`, `explore`, `plan`) are switched off by name in that file's `disabledBuiltIns`, never by copying them to a `.md` file (ADR 0005). `builtInProfiles()` stamps `enabled` onto them so the panel, the `Agent` tool description and `resolveSubagentProfile` agree. Writes are minimal edits that keep names they did not touch, including ones no built-in claims; reading fails *open* (the feature switch has already failed closed). `PATCH /api/subagents/profiles` with `scope: "builtin"` writes it; `PUT`/`DELETE` refuse that scope. A same-name file replaces the built-in outright and is switched off through its own frontmatter. Only the switch is live for a built-in; the rest of the form is read-only. - A positive `max_turns` allows one wrap-up turn only when the SDK would otherwise continue. The limiter chains the SDK's awaited `finishTurn` hook and ends unfinished work without cancelling a completed answer. The wrap-up turn sets `steeringMode` to `all` so a steer queued earlier cannot hold the instruction back. An `end` decision alone does not stop the run: AgentSession starts another run for anything still queued, so the limiter clears the queues, lists those messages in the turn-limit error, and refuses `steer_subagent` from then on. Exhaustion is a failed run with that error and any partial output. A request that ends with `stopReason: "aborted"` is `aborted` even when the controller did not ask for it (Stop in the child's own chat); a provider error is `failed`. - `notifyParent` skips a background run's completion notification when the parent already collected that result with `get_subagent_result`, which marks it consumed. The mark names the run (`sessionId` + `completedAt`), never just the session, since `resume` reuses the session id; `resume` keeps the mark. While the parent `isRunning()` (often still inside that poll) the notification is held and the mark re-checked; an idle parent is notified at once. diff --git a/lib/rpc-manager.ts b/lib/rpc-manager.ts index af1692de01..1c944a5df8 100644 --- a/lib/rpc-manager.ts +++ b/lib/rpc-manager.ts @@ -41,11 +41,12 @@ import { hasSubagentResourceSnapshot, readSubagentRun, readSubagentSessionResources, + settleOrphanedSubagentStatus, subagentExtensionLoaderOptions, subagentToolOptions, SUBAGENT_CONTROL_TOOL_NAMES, } from "./subagents"; -import { createSubagentController } from "./subagent-runtime"; +import { createSubagentController, getLiveSubagentStatus } from "./subagent-runtime"; import { isBuiltInSubagentsEnabled } from "./subagent-settings"; import { resolveShellTools } from "./powershell-settings"; import { CHAT_ONLY_RESOURCE_LOADER_OPTIONS, contextFilesSystemPrompt } from "./chat-only"; @@ -2358,6 +2359,12 @@ export function getRpcSessionInfos(options: { includeTransient?: boolean } = {}) const sessionFile = manager.getSessionFile() ?? session.sessionFile; const persisted = Boolean(sessionFile && existsSync(sessionFile)); const subagent = readSubagentRun(entries as unknown as SessionEntry[], header?.id ?? session.sessionId, sessionFile ?? ""); + // A persisted `running` / `queued` status with no live run was left by a process that + // stopped mid-run, so the row settles it instead of spinning on a dead run. + const relationStatus = session.isRunning() + ? "running" as const + : getLiveSubagentStatus(header?.id ?? session.sessionId) + ?? (subagent ? settleOrphanedSubagentStatus(subagent.status) : undefined); // An ensure_session call creates an idle, empty runtime while the composer // loads commands. Do not leak it into history before a prompt is accepted. @@ -2389,7 +2396,7 @@ export function getRpcSessionInfos(options: { includeTransient?: boolean } = {}) parentSessionId: subagent.parentSessionId, profile: subagent.profile, description: subagent.description, - status: session.isRunning() ? "running" as const : subagent.status, + status: relationStatus ?? subagent.status, }, } : {}), transient: !persisted, diff --git a/lib/rpc-session-info.test.mjs b/lib/rpc-session-info.test.mjs index 548304446a..4ad7ce6388 100644 --- a/lib/rpc-session-info.test.mjs +++ b/lib/rpc-session-info.test.mjs @@ -70,6 +70,49 @@ test("lists an accepted new prompt before its session file exists", (t) => { assert.equal(infos[0].transient, true); }); +test("settles a runtime row whose child was left running by a stopped process", (t) => { + const previousRegistry = globalThis.__piSessions; + const previousRuns = globalThis.__piSubagentRuns; + const dir = mkdtempSync(join(tmpdir(), "pi-web-orphan-runtime-")); + const filePath = join(dir, "session.jsonl"); + writeFileSync(filePath, "persisted\n"); + const entries = [ + { + type: "custom", + customType: "pi-web:subagent", + id: "meta", + parentId: null, + timestamp: "2026-08-12T01:02:03.000Z", + data: { version: 1, parentSessionId: "parent-id", parentSessionPath: "/tmp/parent.jsonl", profile: "Explore", description: "Inspect" }, + }, + { + type: "custom", + customType: "pi-web:subagent-status", + id: "status", + parentId: "meta", + timestamp: "2026-08-12T01:02:04.000Z", + data: { version: 1, status: "running" }, + }, + ]; + globalThis.__piSessions = new Map([["orphaned-runtime", makeRuntimeSession({ + id: "orphaned-runtime", + filePath, + running: false, + entries, + })]]); + globalThis.__piSubagentRuns = new Map(); + t.after(() => { + globalThis.__piSessions = previousRegistry; + globalThis.__piSubagentRuns = previousRuns; + rmSync(dir, { recursive: true, force: true }); + }); + + const infos = getRpcSessionInfos(); + + assert.equal(infos.length, 1); + assert.equal(infos[0].relation.status, "interrupted"); +}); + test("keeps an idle runtime visible once its JSONL file exists", (t) => { const previousRegistry = globalThis.__piSessions; const dir = mkdtempSync(join(tmpdir(), "pi-web-runtime-session-")); diff --git a/lib/session-reader.test.mjs b/lib/session-reader.test.mjs index 27928af33a..0da404a020 100644 --- a/lib/session-reader.test.mjs +++ b/lib/session-reader.test.mjs @@ -657,6 +657,69 @@ test("session listing reads a subagent file again only when its scanned row chan assert.equal((await listAllSessions({ force: true }))[0].relation.status, "failed"); }); +test("session listing reads a subagent left running by a stopped process as interrupted", async (t) => { + const dir = mkdtempSync(join(tmpdir(), "pi-web-relation-orphan-")); + const filePath = join(dir, "child.jsonl"); + const parentPath = join(dir, "parent.jsonl"); + const childId = "orphaned-relation-child"; + writeFileSync(filePath, [ + { type: "session", version: 3, id: childId, timestamp: "2026-01-01T00:00:00.000Z", cwd: dir, parentSession: parentPath }, + { + type: "custom", customType: "pi-web:subagent", id: "meta", parentId: null, timestamp: "2026-01-01T00:00:00.000Z", + data: { version: 1, parentSessionId: "parent-id", parentSessionPath: parentPath, profile: "Explore", description: "Inspect" }, + }, + { + type: "custom", customType: "pi-web:subagent-status", id: "status", parentId: "meta", timestamp: "2026-01-01T00:00:01.000Z", + data: { version: 1, status: "running" }, + }, + ].map((entry) => JSON.stringify(entry)).join("\n")); + + const originalListSessions = listSessions; + listSessions = async () => [{ + path: filePath, + id: childId, + cwd: dir, + created: new Date("2026-01-01T00:00:00.000Z"), + modified: new Date("2026-01-01T00:00:01.000Z"), + messageCount: 0, + firstMessage: "(no messages)", + allMessagesText: "", + parentSessionPath: parentPath, + }]; + resetSessionListState(); + t.after(() => { + listSessions = originalListSessions; + invalidateSessionPathCache(childId); + resetSessionListState(); + rmSync(dir, { recursive: true, force: true }); + }); + + assert.equal((await listAllSessions({ force: true }))[0].relation.status, "interrupted"); +}); + +test("a live runtime row keeps its status over the settled disk row", () => { + const base = { + path: "/tmp/child.jsonl", + id: "child", + cwd: "/tmp", + created: "2026-01-01T00:00:00.000Z", + modified: "2026-01-01T00:00:01.000Z", + messageCount: 2, + firstMessage: "task", + }; + const relation = (status) => ({ + kind: "subagent", + parentSessionId: "parent", + profile: "Explore", + description: "Inspect", + status, + }); + + const merged = mergeSessionLists([{ ...base, relation: relation("interrupted") }], [{ ...base, relation: relation("queued") }]); + + assert.equal(merged[0].relation.status, "queued"); +}); + test("keeps forward and reverse session path caches in sync", async () => { const sessionId = "cache-test-session"; const filePath = join(tmpdir(), "pi-web-cache-test", "..", "cache-test", "session.jsonl"); diff --git a/lib/session-reader.ts b/lib/session-reader.ts index 6b8ee45c56..6c63e141d6 100644 --- a/lib/session-reader.ts +++ b/lib/session-reader.ts @@ -14,7 +14,7 @@ import { sessionPathKey } from "./session-path"; import { skillExpansionToCommand } from "./slash-display"; import { MAX_TOOL_RESULT_IMAGE_BYTES, TOOL_RESULT_IMAGE_MIMES } from "./tool-result-images"; import { resolveProject, type ProjectInfo } from "./worktree"; -import { readSubagentRun, SUBAGENT_META_TYPE } from "./subagents"; +import { readSubagentRun, settleOrphanedSubagentStatus, SUBAGENT_META_TYPE } from "./subagents"; import { checkSessionMembership, listSessionsIncremental, scanSessionFileInfo, type ScannedSessionInfo } from "./session-list-scanner"; export { getAgentDir }; @@ -181,7 +181,16 @@ export function mergeSessionLists( const byId = new Map(supplementalSessions.map((session) => [session.id, session])); // A disk scan is authoritative once the JSONL exists. In particular, this // replaces a transient registry snapshot without briefly rendering two rows. - for (const session of persistedSessions) byId.set(session.id, session); + for (const session of persistedSessions) { + const live = byId.get(session.id); + // The file's status is settled (a restart's `running` reads as interrupted), so a + // live wrapper's row is the only place that still knows a queued child is queued. + if (live?.relation?.kind === "subagent" && session.relation?.kind === "subagent") { + byId.set(session.id, { ...session, relation: { ...session.relation, status: live.relation.status } }); + continue; + } + byId.set(session.id, session); + } return [...byId.values()].sort((a, b) => b.modified.localeCompare(a.modified)); } @@ -255,7 +264,7 @@ function mapScannedSession( : catalogFirstMessage(scanned.firstMessage || "(no messages)"), parentSessionId: originSessionId, ...(subagent - ? { relation: { kind: "subagent" as const, parentSessionId: subagent.parentSessionId, profile: subagent.profile, description: subagent.description, status: subagent.status } } + ? { relation: { kind: "subagent" as const, parentSessionId: subagent.parentSessionId, profile: subagent.profile, description: subagent.description, status: settleOrphanedSubagentStatus(subagent.status) } } : scanned.parentSessionPath ? { relation: { kind: "fork" as const, ...(originSessionId ? { originSessionId } : {}) } } : {}), diff --git a/lib/subagent-runtime.ts b/lib/subagent-runtime.ts index f3886c8dd9..dc6c14b5f5 100644 --- a/lib/subagent-runtime.ts +++ b/lib/subagent-runtime.ts @@ -24,6 +24,7 @@ import { SUBAGENT_STATUS_TYPE, SUBAGENT_RESULT_TYPE, selectSubagentExtensionTools, + settleOrphanedSubagentStatus, subagentExtensionLoaderOptions, subagentToolOptions, withSubagentExtensionTools, @@ -223,7 +224,12 @@ function takeResultConsumed(run: SubagentRunIdentity): boolean { * `resume` can pick the session up again. */ function settleOrphanedRun(run: SubagentRunInfo): SubagentRunInfo { - return run.status === "running" || run.status === "queued" ? { ...run, status: "interrupted" } : run; + return { ...run, status: settleOrphanedSubagentStatus(run.status) }; +} + +/** The status of a run this process still holds, so a listing does not have to guess from its file. */ +export function getLiveSubagentStatus(sessionId: string): SubagentRunInfo["status"] | undefined { + return getSubagentRuns().get(sessionId)?.run.status; } function findSubagentModel(runtime: ModelRuntime, reference: string) { diff --git a/lib/subagents.ts b/lib/subagents.ts index dc12c69a63..0d60b477d4 100644 --- a/lib/subagents.ts +++ b/lib/subagents.ts @@ -961,6 +961,14 @@ export function subagentExtensionLoaderOptions(resources: { loadExtensions: bool }; } +/** + * A persisted `running` / `queued` status with no live child was left by a process that + * stopped mid-run. Every listing must settle it, or a restart leaves a spinner forever. + */ +export function settleOrphanedSubagentStatus(status: SubagentStatus): SubagentStatus { + return status === "running" || status === "queued" ? "interrupted" : status; +} + export function readSubagentRun(entries: readonly SessionEntry[], sessionId: string, sessionPath: string): SubagentRunInfo | null { const data = subagentMetadataData(entries); if (!data) return null;