fix(sdk): keep primary AsyncJobManager when secondary top-level session disposes
Any in-process secondary createAgentSession() (e.g. the Agent Control Center's create flow in agent-dashboard.ts) was constructing its own AsyncJobManager, overwriting the process-global singleton, and then clearing it on its own dispose. The primary session still held its #ownedAsyncJobManager reference, but AsyncJobManager.instance() was undefined for the rest of the process — the task async path hard-failed with "Async execution is enabled but no async job manager is available" and only a full restart cleared it. - sdk.ts: skip constructing/installing a second AsyncJobManager when a singleton is already live, so secondary top-level sessions share the owning session's manager instead of clobbering it.\n- agent-session.ts: scope #cancelOwnAsyncJobs so a secondary session inheriting the singleton with the default MAIN_AGENT_ID can no longer cancel the primary session's running bash/task jobs at dispose time. Subagents still reach the inherited singleton via their unique agent ids; the owning session still cancels its own jobs through #ownedAsyncJobManager. Fixes #1923
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
|
||||
@@ -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<string>();
|
||||
const jobId = primaryManager!.register(
|
||||
"bash",
|
||||
"sleep",
|
||||
async ({ signal }) => {
|
||||
const aborted = Promise.withResolvers<void>();
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user