diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8846e99e6..d4af64036 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `AsyncJobManager.instance()` being cleared while the owning top-level session was still live, which broke the `task` async path with "Async execution is enabled but no async job manager is available" until process restart. Any in-process secondary top-level `createAgentSession()` call (e.g. the Agent Control Center's create flow in `agent-dashboard.ts`) constructed a fresh `AsyncJobManager`, overwrote the singleton, and then cleared it on its own dispose. The secondary now shares the live singleton instead of clobbering it, and its `cancelOwnAsyncJobs` dispose-time cleanup is scoped so it can no longer cancel the primary session's running bash/task jobs ([#1923](https://github.com/can1357/oh-my-pi/issues/1923)). + ## [15.9.1] - 2026-06-04 ### Added diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 0e02b1f9d..cf969e940 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -1168,12 +1168,16 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} return preview; }; - // Only top-level sessions own an AsyncJobManager. Subagents reach the - // parent's manager via `AsyncJobManager.instance()` (set below), so creating - // a second instance here just to leave it orphaned wastes a constructor and - // risks accidental disposal of the parent's manager on subagent teardown. + // Only the first top-level session in a process owns an AsyncJobManager. + // Subagents inherit the parent's manager via `AsyncJobManager.instance()` + // (set below), and any additional top-level session spun up in-process + // (e.g. the agent-creation architect in `agent-dashboard.ts`) must share + // the live singleton — otherwise its dispose path would clobber the + // owning session's manager and break the `task`/`bash` async paths + // (issue #1923). The `instance()` guard means later sessions also skip + // constructing an orphaned manager that nothing would ever route to. const asyncJobManager = - backgroundJobsEnabled && !options.parentTaskPrefix + backgroundJobsEnabled && !options.parentTaskPrefix && !AsyncJobManager.instance() ? new AsyncJobManager({ maxRunningJobs: asyncMaxJobs, onJobComplete: async (jobId, result, job) => { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b7760bb5c..d2c19fa93 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1398,11 +1398,22 @@ export class AgentSession { * Cancel async jobs registered by *this* agent only. Used by lifecycle * transitions (newSession, switchSession, handoff, dispose) so a subagent * cleans up its own background work without touching its parent's jobs. - * No-op when no manager is installed or this session has no agent id. + * + * Cancellation runs against the manager THIS session owns. Subagents have + * unique agent ids and may still reach the inherited singleton (which is + * the parent's manager) to clean up their own scoped jobs. A secondary + * in-process top-level session — which inherits the singleton without + * owning it AND defaults to `MAIN_AGENT_ID` — must NOT cancel via the + * inherited singleton, or it would tear down the owning primary session's + * bash/task jobs at dispose time (issue #1923). + * + * No-op when no manager is reachable or this session has no agent id. */ #cancelOwnAsyncJobs(): void { if (!this.#agentId) return; - AsyncJobManager.instance()?.cancelAll({ ownerId: this.#agentId }); + const manager = + this.#ownedAsyncJobManager ?? (this.#agentId === MAIN_AGENT_ID ? undefined : AsyncJobManager.instance()); + manager?.cancelAll({ ownerId: this.#agentId }); } // ========================================================================= diff --git a/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts b/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts new file mode 100644 index 000000000..4c0f14b5b --- /dev/null +++ b/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts @@ -0,0 +1,105 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { AsyncJobManager } from "@oh-my-pi/pi-coding-agent/async/job-manager"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import { Snowflake } from "@oh-my-pi/pi-utils"; + +describe("AsyncJobManager singleton across concurrent top-level sessions", () => { + const tempDirs: string[] = []; + + afterEach(async () => { + for (const tempDir of tempDirs.splice(0)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + AsyncJobManager.resetForTests(); + }); + + async function spawnTopLevelSession() { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-sdk-async-singleton-${Snowflake.next()}-`)); + tempDirs.push(tempDir); + const cwd = path.join(tempDir, `project-${Snowflake.next()}`); + const agentDir = path.join(tempDir, "agent"); + fs.mkdirSync(cwd, { recursive: true }); + const { session } = await createAgentSession({ + cwd, + agentDir, + settings: Settings.isolated({ "bash.autoBackground.enabled": true }), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }); + return session; + } + + it("keeps the primary session's manager installed after a secondary session disposes", async () => { + const primary = await spawnTopLevelSession(); + try { + const primaryManager = AsyncJobManager.instance(); + expect(primaryManager).toBeDefined(); + + const secondary = await spawnTopLevelSession(); + try { + // While the secondary is alive the global instance MUST still point at + // the primary's manager so background tools keep delivering completions + // to the primary session that owns them. + expect(AsyncJobManager.instance()).toBe(primaryManager); + } finally { + await secondary.dispose(); + } + + // After the secondary disposes, the primary's manager MUST still be the + // reachable singleton — otherwise the `task` async path errors with + // "Async execution is enabled but no async job manager is available". + expect(AsyncJobManager.instance()).toBe(primaryManager); + } finally { + await primary.dispose(); + } + + // Once the owning primary session disposes the singleton clears, matching + // the documented single-owner invariant. + expect(AsyncJobManager.instance()).toBeUndefined(); + }); + + it("does not cancel the primary session's running jobs when a secondary session disposes", async () => { + const primary = await spawnTopLevelSession(); + try { + const primaryManager = AsyncJobManager.instance(); + expect(primaryManager).toBeDefined(); + + // Register a long-running job on the primary's manager under the + // MAIN_AGENT_ID owner — the same owner the secondary would inherit by + // default. The secondary's dispose-time `cancelOwnAsyncJobs` must NOT + // cancel this job (issue #1923). + const release = Promise.withResolvers(); + const jobId = primaryManager!.register( + "bash", + "sleep", + async ({ signal }) => { + const aborted = Promise.withResolvers(); + signal.addEventListener("abort", () => aborted.resolve(), { once: true }); + await Promise.race([release.promise, aborted.promise]); + return signal.aborted ? "aborted" : "completed"; + }, + { ownerId: "Main" }, + ); + + const secondary = await spawnTopLevelSession(); + await secondary.dispose(); + + const job = primaryManager!.getJob(jobId); + expect(job?.status).toBe("running"); + + release.resolve("done"); + await primaryManager!.waitForAll(); + } finally { + await primary.dispose(); + } + }); +});