From fb626e4ef095a85d59d7096c061c62e8be2c5ee0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 11 Aug 2026 06:51:52 +0000 Subject: [PATCH] fix(agent): let shutdown supersede a prior budget abort requestAbort's abortSent branch only upgraded incoming signal reasons, so a shutdown landing after a soft-budget hard-abort was discarded and abortKind() stayed budget. finalizeSubagentLifecycle then followed the budget-resumable path (idle + adopt) even though AgentLifecycleManager.dispose() had run, leaking the subagent session into SDK/process reuse. - Upgrade a prior budget abort to shutdown so the run takes the shutdown release path; genuine kills (signal/timeout/terminate) stay terminal and shutdown is never downgraded to signal. - Cover the shutdown-races-budget-hard-abort case. Fixes #8216 --- packages/coding-agent/src/task/executor.ts | 16 ++++++- .../test/task/executor-soft-budget.test.ts | 48 ++++++++++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 37332477b..b677887c2 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -1095,7 +1095,21 @@ function createSubagentRunMonitor(args: RunMonitorArgs): SubagentRunMonitor { budgetLimitExceeded = true; } if (abortSent) { - if (reason === "signal" && abortReason !== "signal" && abortReason !== "timeout") { + // Shutdown is a superseding external abort: a process teardown that + // races a self-inflicted budget hard-abort must still follow the + // shutdown release path (dispose + unregister) instead of the + // budget-resumable path, which would leave the subagent adopted and + // alive past AgentLifecycleManager.dispose(). Genuine kills + // (signal/timeout/terminate) already dispose terminally, and shutdown + // is never downgraded back to signal. + if (reason === "shutdown" && abortReason === "budget") { + abortReason = "shutdown"; + } else if ( + reason === "signal" && + abortReason !== "signal" && + abortReason !== "timeout" && + abortReason !== "shutdown" + ) { abortReason = "signal"; } return; diff --git a/packages/coding-agent/test/task/executor-soft-budget.test.ts b/packages/coding-agent/test/task/executor-soft-budget.test.ts index d082e0e6a..5571bd89c 100644 --- a/packages/coding-agent/test/task/executor-soft-budget.test.ts +++ b/packages/coding-agent/test/task/executor-soft-budget.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import { AsyncJobManager } from "@oh-my-pi/pi-coding-agent/async"; +import { ASYNC_JOB_MANAGER_SHUTDOWN_REASON, AsyncJobManager } from "@oh-my-pi/pi-coding-agent/async"; import type { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import type { LoadExtensionsResult } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/types"; @@ -344,6 +344,52 @@ describe("runSubprocess soft request budget", () => { rpcRegistry.dispose(); }); + it("a shutdown racing a budget hard-abort follows the shutdown release path", async () => { + // Regression: a process shutdown that lands right after the soft-budget + // grace hard-aborts must supersede the budget reason, so the subagent is + // released (disposed + unregistered, restorable as parked) instead of + // being left adopted and alive past AgentLifecycleManager.dispose(). + const id = "RacedScout"; + const rootSessionFile = `${tempDir.path()}/main.jsonl`; + const workerSessionFile = `${tempDir.path()}/main/${id}.jsonl`; + await Bun.write(rootSessionFile, ""); + await Bun.write(workerSessionFile, ""); + const controller = new AbortController(); + // abort #1 = budget soft-stop (abortSent still false); abort #2 = + // budget hard-abort's abortActiveSession (abortReason already "budget"). + // Fire the shutdown only on #2 so it must supersede the budget reason. + let abortInvocations = 0; + const handle = createMockSession( + ({ promptIndex, emit, pushMessage }) => { + if (promptIndex !== 1) return; + // Never yields: budget 2 → stop at 3, grace exhausted at 8. + for (let i = 1; i <= 8; i++) { + const message = assistantText(`burning request ${i}`); + pushMessage(message); + emit({ type: "message_end", message } as unknown as AgentSessionEvent); + } + }, + () => { + abortInvocations += 1; + if (abortInvocations >= 2 && !controller.signal.aborted) { + controller.abort(ASYNC_JOB_MANAGER_SHUTDOWN_REASON); + } + }, + ); + mockCreateAgentSession(handle.session); + registerRunning(id, handle.session, workerSessionFile); + + const result = await runSubprocess({ ...baseOptions(id), signal: controller.signal }); + + expect(result.aborted).toBe(true); + expect(AgentRegistry.global().get(id)).toBeUndefined(); + expect(handle.disposeCalls()).toBeGreaterThanOrEqual(1); + expect(await Bun.file(`${workerSessionFile}.tombstone`).exists()).toBe(false); + const restored = new AgentRegistry(); + await registerPersistedSubagents(restored, rootSessionFile); + expect(restored.get(id)?.status).toBe("parked"); + }); + it("manager shutdown restores a running kept-alive agent as parked without a tombstone", async () => { const id = "ShutdownScout"; const rootSessionFile = `${tempDir.path()}/main.jsonl`;