diff --git a/.changeset/fixed-orchestrator-identity.md b/.changeset/fixed-orchestrator-identity.md new file mode 100644 index 000000000..578718126 --- /dev/null +++ b/.changeset/fixed-orchestrator-identity.md @@ -0,0 +1,10 @@ +"@aoagents/ao-cli": patch +"@aoagents/ao-core": patch +"@aoagents/ao-web": patch +--- + +Make project orchestrators deterministic and idempotent. + +- ensure each project uses the canonical `{prefix}-orchestrator` session instead of creating numbered main orchestrators +- make `ao start`, the dashboard, and the orchestrator API reuse or restore the canonical session +- keep legacy numbered orchestrators visible as stale sessions without treating them as the main orchestrator diff --git a/packages/cli/__tests__/commands/start.test.ts b/packages/cli/__tests__/commands/start.test.ts index 1f904b39f..040c87e6b 100644 --- a/packages/cli/__tests__/commands/start.test.ts +++ b/packages/cli/__tests__/commands/start.test.ts @@ -36,6 +36,7 @@ const { get: vi.fn(), spawn: vi.fn(), spawnOrchestrator: vi.fn(), + ensureOrchestrator: vi.fn(), send: vi.fn(), claimPR: vi.fn(), }, @@ -257,7 +258,28 @@ beforeEach(async () => { mockSessionManager.restore.mockReset(); mockSessionManager.restore.mockResolvedValue({ id: "app-orchestrator-restored" }); mockSessionManager.get.mockReset(); + mockSessionManager.get.mockImplementation(async (id: string) => { + const sessions = await mockSessionManager.list("my-app"); + return sessions.find((session: { id: string }) => session.id === id) ?? null; + }); mockSessionManager.spawnOrchestrator.mockReset(); + mockSessionManager.spawnOrchestrator.mockResolvedValue({ id: "app-orchestrator" }); + mockSessionManager.ensureOrchestrator.mockReset(); + mockSessionManager.ensureOrchestrator.mockImplementation(async (args) => { + const existing = await mockSessionManager.get("app-orchestrator"); + if (existing) { + if ( + existing.status === "killed" || + existing.status === "done" || + existing.status === "terminated" || + existing.activity === "exited" + ) { + return mockSessionManager.restore(existing.id); + } + return existing; + } + return mockSessionManager.spawnOrchestrator(args); + }); mockSessionManager.kill.mockReset(); mockExec.mockReset(); mockExecSilent.mockReset(); @@ -818,7 +840,7 @@ describe("start command — browser open waits for port", () => { // a numbered id which must end up in the auto-opened browser URL. mockSessionManager.list.mockResolvedValue([]); mockSessionManager.spawnOrchestrator.mockResolvedValue({ - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -830,7 +852,7 @@ describe("start command — browser open waits for port", () => { // waitForPortAndOpen should have been called with orchestrator URL and AbortSignal expect(mockWaitForPortAndOpen).toHaveBeenCalledTimes(1); const args = mockWaitForPortAndOpen.mock.calls[0]; - expect(args[1]).toContain("/projects/my-app/sessions/app-orchestrator-1"); + expect(args[1]).toContain("/projects/my-app/sessions/app-orchestrator"); expect(args[2]).toBeInstanceOf(AbortSignal); expect(mockEnsureLifecycleWorker).toHaveBeenCalledWith( expect.objectContaining({ configPath: expect.any(String) }), @@ -889,7 +911,7 @@ describe("start command — orchestrator session strategy display", () => { await program.parseAsync(["node", "test", "start", "--no-dashboard"]); const output = getLoggedOutput(); - expect(output).toContain("reused existing session (app-orchestrator)"); + expect(output).toContain("ao session attach app-orchestrator"); expect(output).not.toContain("tmux attach -t tmux-session-1"); }); @@ -955,7 +977,7 @@ describe("start command — orchestrator session strategy display", () => { }, ]); mockSessionManager.spawnOrchestrator.mockResolvedValue({ - id: "app-orchestrator-2", + id: "app-orchestrator", runtimeHandle: { id: "tmux-session-new" }, }); @@ -975,7 +997,7 @@ describe("start command — orchestrator session strategy display", () => { const now = new Date(); mockSessionManager.list.mockResolvedValue([ { - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "killed", activity: "exited", @@ -1009,7 +1031,7 @@ describe("start command — orchestrator session strategy display", () => { }, }, { - id: "app-orchestrator-2", + id: "app-orchestrator", projectId: "my-app", status: "killed", activity: "exited", @@ -1044,15 +1066,15 @@ describe("start command — orchestrator session strategy display", () => { }, ]); mockSessionManager.restore.mockResolvedValue({ - id: "app-orchestrator-2", + id: "app-orchestrator", runtimeHandle: { id: "tmux-restored-2" }, }); await program.parseAsync(["node", "test", "start", "--no-dashboard"]); const output = getLoggedOutput(); - expect(output).toContain("ao session attach app-orchestrator-2"); - expect(mockSessionManager.restore).toHaveBeenCalledWith("app-orchestrator-2"); + expect(output).toContain("ao session attach app-orchestrator"); + expect(mockSessionManager.restore).toHaveBeenCalledWith("app-orchestrator"); expect(mockSessionManager.spawnOrchestrator).not.toHaveBeenCalled(); }); @@ -1085,7 +1107,7 @@ describe("start command — orchestrator session strategy display", () => { }, ]); mockSessionManager.spawnOrchestrator.mockResolvedValue({ - id: "app-orchestrator-2", + id: "app-orchestrator", runtimeHandle: { id: "tmux-session-new" }, }); @@ -1119,7 +1141,7 @@ describe("start command — orchestrator session strategy display", () => { // Return two existing orchestrator sessions mockSessionManager.list.mockResolvedValue([ { - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1128,7 +1150,7 @@ describe("start command — orchestrator session strategy display", () => { runtimeHandle: { id: "tmux-session-1" }, }, { - id: "app-orchestrator-2", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1141,16 +1163,11 @@ describe("start command — orchestrator session strategy display", () => { await program.parseAsync(["node", "test", "start"]); const output = getLoggedOutput(); - // With multiple orchestrators, the CLI auto-selects the most recent and tells the user - // how many other sessions are available so they can pick a different one in the dashboard. - expect(output).toContain("/projects/my-app/sessions/app-orchestrator-2"); - expect(output).toContain("1 other session(s) available"); + expect(output).toContain("/projects/my-app/sessions/app-orchestrator"); - // The browser auto-open should land on the dashboard's orchestrator-selection page - // (not a direct session URL) so the user can pick a different one if desired. expect(mockWaitForPortAndOpen).toHaveBeenCalledTimes(1); const args = mockWaitForPortAndOpen.mock.calls[0]; - expect(args[1]).toContain("/orchestrators?project=my-app"); + expect(args[1]).toContain("/projects/my-app/sessions/app-orchestrator"); // Should NOT spawn a new orchestrator when existing ones exist expect(mockSessionManager.spawnOrchestrator).not.toHaveBeenCalled(); @@ -1160,13 +1177,13 @@ describe("start command — orchestrator session strategy display", () => { // The next block of tests pins down the new lookup contract that runStartup // must follow when deciding whether to reuse, restore, or spawn fresh. - it("restores the most-recently-active killed orchestrator instead of spawning", async () => { + it("creates the canonical orchestrator when only numbered legacy orchestrators exist", async () => { mockConfigRef.current = makeConfig({ "my-app": makeProject({ orchestratorSessionStrategy: "reuse" }), }); - // Only orchestrator on disk is killed (its tmux pane was destroyed) but - // still restorable — runStartup must call sm.restore() and reuse its id. + // Numbered orchestrators are legacy/stale and should not be restored as + // the main orchestrator. mockSessionManager.list.mockResolvedValue([ { id: "app-orchestrator-3", @@ -1179,7 +1196,7 @@ describe("start command — orchestrator session strategy display", () => { }, ]); mockSessionManager.restore.mockResolvedValue({ - id: "app-orchestrator-3", + id: "app-orchestrator", projectId: "my-app", status: "spawning", activity: "active", @@ -1190,14 +1207,12 @@ describe("start command — orchestrator session strategy display", () => { await program.parseAsync(["node", "test", "start", "--no-dashboard"]); - expect(mockSessionManager.restore).toHaveBeenCalledWith("app-orchestrator-3"); - expect(mockSessionManager.spawnOrchestrator).not.toHaveBeenCalled(); + expect(mockSessionManager.restore).not.toHaveBeenCalled(); + expect(mockSessionManager.spawnOrchestrator).toHaveBeenCalledTimes(1); const output = getLoggedOutput(); - // Restored id is preserved (no fresh -N allocation) and the user sees an - // explicit "(restored)" marker so the resurfaced id isn't a surprise. - expect(output).toContain("ao session attach app-orchestrator-3"); - expect(output).toContain("(restored)"); + expect(output).toContain("ao session attach app-orchestrator"); + expect(output).not.toContain("(restored)"); }); it("ignores stale bare {projectId}-orchestrator records that lack role metadata", async () => { @@ -1218,7 +1233,7 @@ describe("start command — orchestrator session strategy display", () => { }, ]); mockSessionManager.spawnOrchestrator.mockResolvedValue({ - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1231,7 +1246,7 @@ describe("start command — orchestrator session strategy display", () => { expect(mockSessionManager.restore).not.toHaveBeenCalled(); const output = getLoggedOutput(); - expect(output).toContain("ao session attach app-orchestrator-1"); + expect(output).toContain("ao session attach app-orchestrator"); expect(output).not.toContain("/sessions/my-app-orchestrator"); }); @@ -1250,7 +1265,7 @@ describe("start command — orchestrator session strategy display", () => { mockSessionManager.list.mockResolvedValue([ // Live but older — this is the one we must pick. { - id: "app-orchestrator-2", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1277,7 +1292,7 @@ describe("start command — orchestrator session strategy display", () => { expect(mockSessionManager.spawnOrchestrator).not.toHaveBeenCalled(); const output = getLoggedOutput(); - expect(output).toContain("/projects/my-app/sessions/app-orchestrator-2"); + expect(output).toContain("/projects/my-app/sessions/app-orchestrator"); expect(output).not.toContain("/projects/my-app/sessions/app-orchestrator-3"); }); @@ -1289,7 +1304,7 @@ describe("start command — orchestrator session strategy display", () => { const now = Date.now(); mockSessionManager.list.mockResolvedValue([ { - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1298,7 +1313,7 @@ describe("start command — orchestrator session strategy display", () => { runtimeHandle: { id: "tmux-1" }, }, { - id: "app-orchestrator-2", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1314,9 +1329,7 @@ describe("start command — orchestrator session strategy display", () => { expect(mockSessionManager.restore).not.toHaveBeenCalled(); const output = getLoggedOutput(); - // The newer one (-2) is auto-selected for attach. - expect(output).toContain("ao session attach app-orchestrator-2"); - expect(output).toContain("1 other session(s) available"); + expect(output).toContain("ao session attach app-orchestrator"); }); it("fails and cleans up dashboard when orchestrator setup throws", async () => { @@ -1394,7 +1407,7 @@ describe("start command — orchestrator session strategy display", () => { // spawnOrchestrator (which would silently allocate a fresh -N). mockSessionManager.list.mockResolvedValue([ { - id: "app-orchestrator-3", + id: "app-orchestrator", projectId: "my-app", status: "killed", activity: "exited", @@ -1411,7 +1424,7 @@ describe("start command — orchestrator session strategy display", () => { .mocked(console.error) .mock.calls.map((c) => c.join(" ")) .join("\n"); - expect(errors).toContain("Failed to restore orchestrator session app-orchestrator-3"); + expect(errors).toContain("Failed to setup orchestrator"); expect(errors).toContain("workspace gone"); expect(mockSessionManager.spawnOrchestrator).not.toHaveBeenCalled(); @@ -1491,7 +1504,7 @@ describe("stop command", () => { const now = Date.now(); mockSessionManager.list.mockResolvedValue([ { - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1500,7 +1513,7 @@ describe("stop command", () => { runtimeHandle: { id: "tmux-1" }, }, { - id: "app-orchestrator-2", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1513,7 +1526,7 @@ describe("stop command", () => { await program.parseAsync(["node", "test", "stop"]); - expect(mockSessionManager.kill).toHaveBeenCalledWith("app-orchestrator-2", { + expect(mockSessionManager.kill).toHaveBeenCalledWith("app-orchestrator", { purgeOpenCode: false, }); }); @@ -1537,7 +1550,7 @@ describe("stop command", () => { mockConfigRef.current = makeConfig({ "my-app": makeProject() }); mockSessionManager.list.mockResolvedValue([ { - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -1551,7 +1564,7 @@ describe("stop command", () => { await program.parseAsync(["node", "test", "stop", "--purge-session"]); - expect(mockSessionManager.kill).toHaveBeenCalledWith("app-orchestrator-1", { + expect(mockSessionManager.kill).toHaveBeenCalledWith("app-orchestrator", { purgeOpenCode: true, }); }); diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 15d29c7fc..817f92c41 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -20,6 +20,7 @@ import { loadConfig, generateOrchestratorPrompt, generateSessionPrefix, + getOrchestratorSessionId, findConfigFile, getGlobalConfigPath, isRepoUrl, @@ -28,10 +29,8 @@ import { isRepoAlreadyCloned, generateConfigFromUrl, configToYaml, - normalizeOrchestratorSessionStrategy, isOrchestratorSession, isTerminalSession, - isRestorable, ConfigNotFoundError, loadLocalProjectConfigDetailed, registerProjectInGlobalConfig, @@ -39,7 +38,6 @@ import { type LocalProjectConfig, type ProjectConfig, type ParsedRepoUrl, - type Session, writeLocalProjectConfig, } from "@aoagents/ao-core"; import { parse as yamlParse, stringify as yamlStringify } from "yaml"; @@ -1060,15 +1058,10 @@ async function runStartup( const shouldStartLifecycle = opts?.dashboard !== false || opts?.orchestrator !== false; let lifecycleStatus: Awaited> | null = null; let port = config.port ?? DEFAULT_PORT; - const orchestratorSessionStrategy = normalizeOrchestratorSessionStrategy( - project.orchestratorSessionStrategy, - ); - console.log(chalk.bold(`\nStarting orchestrator for ${chalk.cyan(project.name)}\n`)); const spinner = ora(); let dashboardProcess: ChildProcess | null = null; - let reused = false; let restored = false; // Start dashboard (unless --no-dashboard) @@ -1129,145 +1122,35 @@ async function runStartup( } } - // Create orchestrator session (unless --no-orchestrator or existing orchestrators found) - let hasMultipleReusable = false; let selectedOrchestratorId: string | null = null; - let otherCandidateCount = 0; if (opts?.orchestrator !== false) { const sm = await getSessionManager(config); - // Check for existing orchestrator sessions for this project. - let allSessions; try { - allSessions = await sm.list(projectId); + spinner.start("Ensuring orchestrator session"); + const systemPrompt = generateOrchestratorPrompt({ config, projectId, project }); + const before = await sm.get(getOrchestratorSessionId(project)); + const session = await sm.ensureOrchestrator({ projectId, systemPrompt }); + selectedOrchestratorId = session.id; + restored = Boolean(session.restoredAt); + if (before && session.id === before.id && !restored) { + spinner.succeed(`Using orchestrator session: ${session.id}`); + } else if (restored) { + spinner.succeed(`Restored orchestrator session: ${session.id}`); + } else { + spinner.succeed(`Orchestrator session ready: ${session.id}`); + } } catch (err) { - spinner.fail("Failed to list sessions"); + spinner.fail("Orchestrator setup failed"); if (dashboardProcess) { dashboardProcess.kill(); } throw new Error( - `Failed to list sessions: ${err instanceof Error ? err.message : String(err)}`, + `Failed to setup orchestrator: ${err instanceof Error ? err.message : String(err)}`, { cause: err }, ); } - const allSessionPrefixes = Object.entries(config.projects).map( - ([, p]) => p.sessionPrefix ?? generateSessionPrefix(p.name ?? ""), - ); - const orchestrators = allSessions.filter((s) => - isOrchestratorSession(s, project.sessionPrefix ?? projectId, allSessionPrefixes), - ); - - // Partition into two reuse buckets so we never spawn a new numbered id when - // an existing one is still usable: - // - live: runtime is still running, attach in place. - // - restorable: status is terminal but the session can be restarted via - // sm.restore() (workspace + branch + handle still on disk). - // Restoring keeps the original numbered id rather than - // allocating a fresh one. - // - // IMPORTANT: live MUST be preferred unconditionally over restorable. A - // previous version sorted both buckets together by `lastActivityAt`, which - // could pick a newer killed record over an older-but-still-running one — - // sm.restore() would then spin up the killed record while the live one - // kept running, leaving two orchestrators alive for the project. Only fall - // back to restorable when the live bucket is empty. - const live = orchestrators.filter((s) => !isTerminalSession(s)); - // isRestorable already requires isTerminalSession internally, so no need - // to repeat that guard here. - const restorable = orchestrators.filter((s) => isRestorable(s)); - type OrchestratorCandidate = { session: Session; mode: "live" | "restore" }; - const byMostRecent = (a: Session, b: Session): number => - (b.lastActivityAt?.getTime() ?? 0) - (a.lastActivityAt?.getTime() ?? 0); - const candidates: OrchestratorCandidate[] = - live.length > 0 - ? [...live] - .sort(byMostRecent) - .map((session) => ({ session, mode: "live" })) - : [...restorable] - .sort(byMostRecent) - .map((session) => ({ session, mode: "restore" })); - - if (candidates.length > 0 && orchestratorSessionStrategy === "reuse") { - const chosen = candidates[0]; - // Multiple candidates → CLI auto-picks the most recent, but the dashboard - // surfaces all of them via the orchestrator-selection page. Only meaningful - // when the dashboard is running. - otherCandidateCount = candidates.length - 1; - if (opts?.dashboard !== false && candidates.length > 1) { - hasMultipleReusable = true; - } - - const otherSuffix = - otherCandidateCount > 0 ? ` (${otherCandidateCount} other session(s) available)` : ""; - - if (chosen.mode === "live") { - selectedOrchestratorId = chosen.session.id; - spinner.succeed(`Using existing orchestrator session: ${chosen.session.id}${otherSuffix}`); - } else { - try { - spinner.start(`Restoring orchestrator session: ${chosen.session.id}`); - const restoredSession = await sm.restore(chosen.session.id); - selectedOrchestratorId = restoredSession.id; - restored = true; - spinner.succeed( - `Restored orchestrator session: ${restoredSession.id}${otherSuffix}`, - ); - } catch (err) { - spinner.fail(`Failed to restore orchestrator session: ${chosen.session.id}`); - if (dashboardProcess) { - dashboardProcess.kill(); - } - throw new Error( - `Failed to restore orchestrator session ${chosen.session.id}: ${ - err instanceof Error ? err.message : String(err) - }`, - { cause: err }, - ); - } - } - } else { - if (orchestratorSessionStrategy === "delete") { - const liveOrchestrators = orchestrators.filter((s) => !isTerminalSession(s)); - for (const orchestrator of liveOrchestrators) { - try { - await sm.kill(orchestrator.id); - } catch (err) { - spinner.fail(`Failed to replace existing orchestrator: ${orchestrator.id}`); - if (dashboardProcess) { - dashboardProcess.kill(); - } - throw new Error( - `Failed to kill existing orchestrator ${orchestrator.id}: ${ - err instanceof Error ? err.message : String(err) - }`, - { cause: err }, - ); - } - } - } - - // No reusable orchestrators — spawn a fresh numbered one. - try { - spinner.start("Creating orchestrator session"); - const systemPrompt = generateOrchestratorPrompt({ config, projectId, project }); - const session = await sm.spawnOrchestrator({ projectId, systemPrompt }); - selectedOrchestratorId = session.id; - reused = - orchestratorSessionStrategy === "reuse" && - session.metadata?.["orchestratorSessionReused"] === "true"; - spinner.succeed(reused ? "Orchestrator session reused" : "Orchestrator session created"); - } catch (err) { - spinner.fail("Orchestrator setup failed"); - if (dashboardProcess) { - dashboardProcess.kill(); - } - throw new Error( - `Failed to setup orchestrator: ${err instanceof Error ? err.message : String(err)}`, - { cause: err }, - ); - } - } } // Print summary @@ -1284,42 +1167,26 @@ async function runStartup( if (opts?.orchestrator !== false && selectedOrchestratorId) { const restoreNote = restored ? " (restored)" : ""; - const otherSummarySuffix = - otherCandidateCount > 0 ? ` — ${otherCandidateCount} other session(s) available` : ""; const target = opts?.dashboard !== false ? projectSessionUrl(port, projectId, selectedOrchestratorId) : `ao session attach ${selectedOrchestratorId}`; - if (reused) { - console.log( - chalk.cyan("Orchestrator:"), - `reused existing session (${selectedOrchestratorId})${otherSummarySuffix}`, - ); - } else { - console.log( - chalk.cyan("Orchestrator:"), - `${target}${restoreNote}${otherSummarySuffix}`, - ); - } + console.log(chalk.cyan("Orchestrator:"), `${target}${restoreNote}`); } console.log(chalk.dim(`Config: ${config.configPath}`)); // Auto-open browser once the server is ready. - // With a single chosen orchestrator (live, restored, or newly spawned), navigate directly to - // its session page. With multiple reusable orchestrators, open the selection page so the user - // can choose or spawn a new one — the dashboard only links one orchestrator per project. + // Navigate directly to the deterministic main orchestrator when one is available. // Polls the port instead of using a fixed delay — deterministic and works regardless of // how long Next.js takes to compile. AbortController cancels polling on early exit. let openAbort: AbortController | undefined; if (opts?.dashboard !== false) { openAbort = new AbortController(); - const orchestratorUrl = hasMultipleReusable - ? `http://localhost:${port}/orchestrators?project=${projectId}` - : selectedOrchestratorId - ? projectSessionUrl(port, projectId, selectedOrchestratorId) - : `http://localhost:${port}`; + const orchestratorUrl = selectedOrchestratorId + ? projectSessionUrl(port, projectId, selectedOrchestratorId) + : `http://localhost:${port}`; void waitForPortAndOpen(port, orchestratorUrl, openAbort.signal); } diff --git a/packages/core/src/__tests__/session-manager/spawn.test.ts b/packages/core/src/__tests__/session-manager/spawn.test.ts index a65d3e452..6964dd48e 100644 --- a/packages/core/src/__tests__/session-manager/spawn.test.ts +++ b/packages/core/src/__tests__/session-manager/spawn.test.ts @@ -4,6 +4,10 @@ import { join } from "node:path"; import { createSessionManager } from "../../session-manager.js"; import { validateConfig } from "../../config.js"; import { getWorkspaceAgentsMdPath } from "../../opencode-agents-md.js"; +import { + buildLifecycleMetadataPatch, + createInitialCanonicalLifecycle, +} from "../../lifecycle-state.js"; import { writeMetadata, readMetadata, @@ -1199,7 +1203,7 @@ describe("spawn", () => { ); // Reserved session metadata should be cleaned up - expect(readMetadataRaw(sessionsDir, "app-orchestrator-1")).toBeNull(); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")).toBeNull(); expect(mockRuntime.create).not.toHaveBeenCalled(); }); @@ -1208,10 +1212,10 @@ describe("spawn", () => { const session = await sm.spawnOrchestrator({ projectId: "my-app" }); - expect(session.id).toBe("app-orchestrator-1"); + expect(session.id).toBe("app-orchestrator"); expect(session.status).toBe("working"); expect(session.projectId).toBe("my-app"); - expect(session.branch).toBe("orchestrator/app-orchestrator-1"); + expect(session.branch).toBe("orchestrator/app-orchestrator"); expect(session.issueId).toBeNull(); expect(session.workspacePath).toBe("/tmp/ws"); }); @@ -1223,8 +1227,8 @@ describe("spawn", () => { expect(mockWorkspace.create).toHaveBeenCalledWith( expect.objectContaining({ - sessionId: "app-orchestrator-1", - branch: "orchestrator/app-orchestrator-1", + sessionId: "app-orchestrator", + branch: "orchestrator/app-orchestrator", projectId: "my-app", }), ); @@ -1234,8 +1238,8 @@ describe("spawn", () => { const worktreePath = join(tmpDir, "orchestrator-ws"); (mockWorkspace.create as ReturnType).mockResolvedValueOnce({ path: worktreePath, - branch: "orchestrator/app-orchestrator-1", - sessionId: "app-orchestrator-1", + branch: "orchestrator/app-orchestrator", + sessionId: "app-orchestrator", projectId: "my-app", }); const sm = createSessionManager({ config, registry: mockRegistry }); @@ -1243,7 +1247,7 @@ describe("spawn", () => { const session = await sm.spawnOrchestrator({ projectId: "my-app" }); expect(session.workspacePath).toBe(worktreePath); - expect(session.branch).toBe("orchestrator/app-orchestrator-1"); + expect(session.branch).toBe("orchestrator/app-orchestrator"); }); it("writes metadata with proper fields", async () => { @@ -1251,11 +1255,11 @@ describe("spawn", () => { await sm.spawnOrchestrator({ projectId: "my-app" }); - const meta = readMetadata(sessionsDir, "app-orchestrator-1"); + const meta = readMetadata(sessionsDir, "app-orchestrator"); expect(meta).not.toBeNull(); expect(meta!.status).toBe("working"); expect(meta!.project).toBe("my-app"); - expect(meta!.branch).toBe("orchestrator/app-orchestrator-1"); + expect(meta!.branch).toBe("orchestrator/app-orchestrator"); expect(meta!.tmuxName).toBeDefined(); expect(meta!.runtimeHandle).toBeDefined(); }); @@ -1265,22 +1269,115 @@ describe("spawn", () => { await sm.spawnOrchestrator({ projectId: "my-app" }); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["role"]).toBe("orchestrator"); - expect(meta?.["branch"]).toBe("orchestrator/app-orchestrator-1"); + expect(meta?.["branch"]).toBe("orchestrator/app-orchestrator"); expect(meta?.["status"]).toBe("working"); expect(meta?.["project"]).toBe("my-app"); }); - it("increments the orchestrator counter for each new session", async () => { + it("ensureOrchestrator returns the canonical session on repeated calls", async () => { const sm = createSessionManager({ config, registry: mockRegistry }); - const s1 = await sm.spawnOrchestrator({ projectId: "my-app" }); - const s2 = await sm.spawnOrchestrator({ projectId: "my-app" }); + const s1 = await sm.ensureOrchestrator({ projectId: "my-app" }); + const s2 = await sm.ensureOrchestrator({ projectId: "my-app" }); - expect(s1.id).toBe("app-orchestrator-1"); - expect(s2.id).toBe("app-orchestrator-2"); - expect(mockWorkspace.create).toHaveBeenCalledTimes(2); + expect(s1.id).toBe("app-orchestrator"); + expect(s2.id).toBe("app-orchestrator"); + expect(mockWorkspace.create).toHaveBeenCalledTimes(1); + }); + + it("ensureOrchestrator coalesces concurrent creation calls", async () => { + const sm = createSessionManager({ config, registry: mockRegistry }); + + const [s1, s2] = await Promise.all([ + sm.ensureOrchestrator({ projectId: "my-app" }), + sm.ensureOrchestrator({ projectId: "my-app" }), + ]); + + expect(s1.id).toBe("app-orchestrator"); + expect(s2.id).toBe("app-orchestrator"); + expect(mockWorkspace.create).toHaveBeenCalledTimes(1); + expect(mockRuntime.create).toHaveBeenCalledTimes(1); + }); + + it("ensureOrchestrator replaces the canonical session for delete strategy", async () => { + const configWithDelete: OrchestratorConfig = { + ...config, + projects: { + ...config.projects, + "my-app": { + ...config.projects["my-app"]!, + orchestratorSessionStrategy: "delete", + }, + }, + }; + writeMetadata(sessionsDir, "app-orchestrator", { + role: "orchestrator", + project: "my-app", + status: "working", + branch: "orchestrator/app-orchestrator", + worktree: join(tmpDir, "old-orchestrator"), + runtimeHandle: JSON.stringify(makeHandle("old-rt")), + }); + const sm = createSessionManager({ config: configWithDelete, registry: mockRegistry }); + + const session = await sm.ensureOrchestrator({ projectId: "my-app" }); + + expect(session.id).toBe("app-orchestrator"); + expect(mockRuntime.destroy).toHaveBeenCalledWith(makeHandle("old-rt")); + expect(mockWorkspace.create).toHaveBeenCalledWith( + expect.objectContaining({ sessionId: "app-orchestrator" }), + ); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")?.["status"]).toBe("working"); + }); + + it("ensureOrchestrator ignores numbered legacy orchestrators and creates the canonical session", async () => { + writeMetadata(sessionsDir, "app-orchestrator-5", { + role: "orchestrator", + project: "my-app", + status: "working", + branch: "orchestrator/app-orchestrator-5", + worktree: join(tmpDir, "legacy-orchestrator"), + }); + const sm = createSessionManager({ config, registry: mockRegistry }); + + const session = await sm.ensureOrchestrator({ projectId: "my-app" }); + + expect(session.id).toBe("app-orchestrator"); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")).not.toBeNull(); + expect(readMetadataRaw(sessionsDir, "app-orchestrator-5")).not.toBeNull(); + expect(mockWorkspace.create).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: "app-orchestrator", + branch: "orchestrator/app-orchestrator", + }), + ); + }); + + it("ensureOrchestrator fails clearly when canonical session is done and non-restorable", async () => { + const lifecycle = createInitialCanonicalLifecycle("orchestrator"); + lifecycle.session.state = "done"; + lifecycle.session.reason = "research_complete"; + lifecycle.session.completedAt = new Date().toISOString(); + lifecycle.runtime.state = "exited"; + lifecycle.runtime.reason = "process_missing"; + const doneWorktree = join(tmpDir, "done-orchestrator"); + mkdirSync(doneWorktree, { recursive: true }); + writeMetadata(sessionsDir, "app-orchestrator", { + role: "orchestrator", + project: "my-app", + status: "done", + branch: "orchestrator/app-orchestrator", + worktree: doneWorktree, + ...buildLifecycleMetadataPatch(lifecycle, "done"), + }); + const sm = createSessionManager({ config, registry: mockRegistry }); + + await expect(sm.ensureOrchestrator({ projectId: "my-app" })).rejects.toThrow( + 'canonical orchestrator session is terminal with status "done"', + ); + expect(mockWorkspace.create).not.toHaveBeenCalled(); }); it("cleans up reserved metadata on workspace creation failure", async () => { @@ -1294,15 +1391,15 @@ describe("spawn", () => { ); // Reserved session file should be cleaned up - expect(readMetadataRaw(sessionsDir, "app-orchestrator-1")).toBeNull(); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")).toBeNull(); }); it("destroys the worktree and metadata when runtime creation fails", async () => { const worktreePath = join(tmpDir, "orchestrator-ws-rt-fail"); (mockWorkspace.create as ReturnType).mockResolvedValueOnce({ path: worktreePath, - branch: "orchestrator/app-orchestrator-1", - sessionId: "app-orchestrator-1", + branch: "orchestrator/app-orchestrator", + sessionId: "app-orchestrator", projectId: "my-app", }); (mockRuntime.create as ReturnType).mockRejectedValueOnce( @@ -1315,15 +1412,15 @@ describe("spawn", () => { ); expect(mockWorkspace.destroy).toHaveBeenCalledWith(worktreePath); - expect(readMetadataRaw(sessionsDir, "app-orchestrator-1")).toBeNull(); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")).toBeNull(); }); it("destroys the worktree when post-launch setup fails", async () => { const worktreePath = join(tmpDir, "orchestrator-ws-postlaunch-fail"); (mockWorkspace.create as ReturnType).mockResolvedValueOnce({ path: worktreePath, - branch: "orchestrator/app-orchestrator-1", - sessionId: "app-orchestrator-1", + branch: "orchestrator/app-orchestrator", + sessionId: "app-orchestrator", projectId: "my-app", }); const postLaunchError = new Error("post-launch setup failed"); @@ -1348,7 +1445,7 @@ describe("spawn", () => { expect(mockRuntime.destroy).toHaveBeenCalled(); expect(mockWorkspace.destroy).toHaveBeenCalledWith(worktreePath); - expect(readMetadataRaw(sessionsDir, "app-orchestrator-1")).toBeNull(); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")).toBeNull(); }); it("deletes previous OpenCode orchestrator sessions before starting", async () => { @@ -1356,8 +1453,8 @@ describe("spawn", () => { const mockBin = installMockOpencode( tmpDir, JSON.stringify([ - { id: "ses_old", title: "AO:app-orchestrator-1", updated: "2025-01-01T00:00:00.000Z" }, - { id: "ses_new", title: "AO:app-orchestrator-1", updated: "2025-01-02T00:00:00.000Z" }, + { id: "ses_old", title: "AO:app-orchestrator", updated: "2025-01-01T00:00:00.000Z" }, + { id: "ses_new", title: "AO:app-orchestrator", updated: "2025-01-02T00:00:00.000Z" }, ]), deleteLogPath, ); @@ -1399,14 +1496,14 @@ describe("spawn", () => { expect(opencodeAgent.getLaunchCommand).toHaveBeenCalledWith( expect.objectContaining({ - sessionId: "app-orchestrator-1", + sessionId: "app-orchestrator", projectConfig: expect.objectContaining({ agentConfig: expect.not.objectContaining({ opencodeSessionId: expect.any(String) }), }), }), ); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["agent"]).toBe("opencode"); expect(meta?.["opencodeSessionId"]).toBeUndefined(); }); @@ -1418,7 +1515,7 @@ describe("spawn", () => { JSON.stringify([ { id: "ses_discovered_orchestrator", - title: "AO:app-orchestrator-1", + title: "AO:app-orchestrator", updated: 1_772_777_000_000, }, ]), @@ -1456,7 +1553,7 @@ describe("spawn", () => { const sm = createSessionManager({ config: configWithReuse, registry: registryWithOpenCode }); await sm.spawnOrchestrator({ projectId: "my-app" }); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["opencodeSessionId"]).toBe("ses_discovered_orchestrator"); }); @@ -1467,7 +1564,7 @@ describe("spawn", () => { JSON.stringify([ { id: "ses_existing", - title: "AO:app-orchestrator-1", + title: "AO:app-orchestrator", updated: 1_772_777_000_000, }, ]), @@ -1512,7 +1609,7 @@ describe("spawn", () => { }), }), ); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["opencodeSessionId"]).toBe("ses_existing"); }); @@ -1521,7 +1618,7 @@ describe("spawn", () => { const mockBin = installMockOpencode( tmpDir, JSON.stringify([ - { id: "ses_existing", title: "AO:app-orchestrator-1", updated: 1_772_777_000_000 }, + { id: "ses_existing", title: "AO:app-orchestrator", updated: 1_772_777_000_000 }, ]), deleteLogPath, ); @@ -1571,7 +1668,7 @@ describe("spawn", () => { const mockBin = installMockOpencode( tmpDir, JSON.stringify([ - { id: "ses_title_match", title: "AO:app-orchestrator-1", updated: 1_772_777_000_000 }, + { id: "ses_title_match", title: "AO:app-orchestrator", updated: 1_772_777_000_000 }, ]), deleteLogPath, ); @@ -1614,7 +1711,7 @@ describe("spawn", () => { }), }), ); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["opencodeSessionId"]).toBe("ses_title_match"); }); @@ -1682,7 +1779,7 @@ describe("spawn", () => { await sm.spawnOrchestrator({ projectId: "my-app" }); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["orchestratorSessionReused"]).toBeUndefined(); }); @@ -1784,7 +1881,7 @@ describe("spawn", () => { expect(mockCodexAgent.getLaunchCommand).toHaveBeenCalled(); expect(mockAgent.getLaunchCommand).not.toHaveBeenCalled(); - expect(readMetadataRaw(sessionsDir, "app-orchestrator-1")?.["agent"]).toBe("codex"); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")?.["agent"]).toBe("codex"); }); it("uses defaults orchestrator agent when project agent is not set", async () => { @@ -1831,7 +1928,7 @@ describe("spawn", () => { await sm.spawnOrchestrator({ projectId: "my-app" }); expect(mockCodexAgent.getLaunchCommand).toHaveBeenCalled(); - expect(readMetadataRaw(sessionsDir, "app-orchestrator-1")?.["agent"]).toBe("codex"); + expect(readMetadataRaw(sessionsDir, "app-orchestrator")?.["agent"]).toBe("codex"); }); it("keeps shared worker permissions when role-specific config only overrides model", async () => { @@ -1933,8 +2030,8 @@ describe("spawn", () => { // Should pass systemPromptFile (not inline systemPrompt) to avoid tmux truncation expect(mockAgent.getLaunchCommand).toHaveBeenCalledWith( expect.objectContaining({ - sessionId: "app-orchestrator-1", - systemPromptFile: expect.stringContaining("orchestrator-prompt-app-orchestrator-1.md"), + sessionId: "app-orchestrator", + systemPromptFile: expect.stringContaining("orchestrator-prompt-app-orchestrator.md"), }), ); @@ -1954,7 +2051,7 @@ describe("spawn", () => { systemPrompt: "Audit test coverage for session-manager and open PRs for gaps", }); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["displayName"]).toBe( "Audit test coverage for session-manager and open PRs for gaps", ); @@ -1965,7 +2062,7 @@ describe("spawn", () => { await sm.spawnOrchestrator({ projectId: "my-app" }); - const meta = readMetadataRaw(sessionsDir, "app-orchestrator-1"); + const meta = readMetadataRaw(sessionsDir, "app-orchestrator"); expect(meta?.["displayName"]).toBeUndefined(); }); diff --git a/packages/core/src/__tests__/test-utils.ts b/packages/core/src/__tests__/test-utils.ts index 7d0a493b0..f3fb79dfd 100644 --- a/packages/core/src/__tests__/test-utils.ts +++ b/packages/core/src/__tests__/test-utils.ts @@ -445,6 +445,7 @@ export function createMockSessionManager(): OpenCodeSessionManager { return { spawn: vi.fn().mockResolvedValue(makeSession()), spawnOrchestrator: vi.fn().mockResolvedValue(makeSession({ id: "app-orchestrator", metadata: { role: "orchestrator" } })), + ensureOrchestrator: vi.fn().mockResolvedValue(makeSession({ id: "app-orchestrator", metadata: { role: "orchestrator" } })), restore: vi.fn().mockResolvedValue(makeSession()), list: vi.fn().mockResolvedValue([]), listCached: vi.fn().mockResolvedValue([]), diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c3f7d5a55..4af159355 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -155,7 +155,10 @@ export { getWorkspaceAgentsMdPath, writeWorkspaceOpenCodeAgentsMd, } from "./opencode-agents-md.js"; -export { normalizeOrchestratorSessionStrategy } from "./orchestrator-session-strategy.js"; +export { + getOrchestratorSessionId, + normalizeOrchestratorSessionStrategy, +} from "./orchestrator-session-strategy.js"; export { resolveSpawnTarget } from "./spawn-target.js"; export type { SpawnTarget } from "./spawn-target.js"; @@ -237,6 +240,7 @@ export { getOriginFilePath, generateSessionName, generateTmuxName, + requireStorageKey, parseTmuxName, expandHome, validateAndStoreOrigin, diff --git a/packages/core/src/orchestrator-session-strategy.ts b/packages/core/src/orchestrator-session-strategy.ts index 0d2c2b490..3aa48b85d 100644 --- a/packages/core/src/orchestrator-session-strategy.ts +++ b/packages/core/src/orchestrator-session-strategy.ts @@ -2,6 +2,10 @@ import type { ProjectConfig } from "./types.js"; export type NormalizedOrchestratorSessionStrategy = "reuse" | "delete" | "ignore"; +export function getOrchestratorSessionId(project: Pick): string { + return `${project.sessionPrefix}-orchestrator`; +} + export function normalizeOrchestratorSessionStrategy( strategy: ProjectConfig["orchestratorSessionStrategy"] | undefined, ): NormalizedOrchestratorSessionStrategy { diff --git a/packages/core/src/paths.ts b/packages/core/src/paths.ts index 9add82d73..35abeffd8 100644 --- a/packages/core/src/paths.ts +++ b/packages/core/src/paths.ts @@ -228,7 +228,7 @@ export function validateAndStoreOrigin(configPath: string, storageKey: string): } } -function requireStorageKey(storageKey: string | undefined): string { +export function requireStorageKey(storageKey: string | undefined): string { if (!storageKey) { throw new Error("storageKey is required"); } diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 7ef8c0c92..f210a6cd2 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -19,6 +19,7 @@ import { promisify } from "node:util"; import { isIssueNotFoundError, isRestorable, + isTerminalSession, SessionNotFoundError, SessionNotRestorableError, WorkspaceMissingError, @@ -69,13 +70,17 @@ import { getWorktreesDir, getProjectBaseDir, generateTmuxName, + requireStorageKey, validateAndStoreOrigin, } from "./paths.js"; import { asValidOpenCodeSessionId } from "./opencode-session-id.js"; import { writeWorkspaceOpenCodeAgentsMd, } from "./opencode-agents-md.js"; -import { normalizeOrchestratorSessionStrategy } from "./orchestrator-session-strategy.js"; +import { + getOrchestratorSessionId, + normalizeOrchestratorSessionStrategy, +} from "./orchestrator-session-strategy.js"; import { sessionFromMetadata } from "./utils/session-from-metadata.js"; import { safeJsonParse, validateStatus } from "./utils/validation.js"; import { isGitBranchNameSafe } from "./utils.js"; @@ -287,11 +292,17 @@ const SEND_CONFIRMATION_POLL_MS = 500; const SEND_CONFIRMATION_OUTPUT_LINES = 20; const SEND_BOOTSTRAP_READY_TIMEOUT_MS = 20_000; const SEND_BOOTSTRAP_STABLE_POLLS = 2; +const ENSURE_ORCHESTRATOR_CONFLICT_WAIT_MS = 20_000; +const ENSURE_ORCHESTRATOR_CONFLICT_POLL_MS = 250; function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +function isFixedOrchestratorReservationError(err: unknown, sessionId: string): boolean { + return err instanceof Error && err.message.includes(`Orchestrator session "${sessionId}" already exists`); +} + async function getTmuxForegroundCommand(sessionName: string): Promise { try { const { stdout } = await execFileAsync( @@ -510,6 +521,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM expiresAt: number; } | null = null; + const ensureOrchestratorPromises = new Map>(); function invalidateCache(): void { sessionCache = null; @@ -853,64 +865,21 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM ); } - /** - * Reserve a unique orchestrator identity ({prefix}-orchestrator-N) for a worktree-based orchestrator. - * Unlike worker sessions, orchestrator IDs are assigned locally without remote branch checks. - */ - function reserveNextOrchestratorIdentity( + function reserveFixedOrchestratorIdentity( project: ProjectConfig, sessionsDir: string, - ): { num: number; sessionId: string; tmuxName: string | undefined } { - const orchestratorPrefix = `${project.sessionPrefix}-orchestrator`; - const usedNumbers = new Set(); - - const orchestratorPattern = new RegExp(`^${escapeRegex(orchestratorPrefix)}-(\\d+)$`); - for (const sessionName of [ - ...listMetadata(sessionsDir), - ...listArchivedSessionIds(sessionsDir), - ]) { - const match = sessionName.match(orchestratorPattern); - if (match) { - const parsed = Number.parseInt(match[1], 10); - if (!Number.isNaN(parsed)) usedNumbers.add(parsed); - } + ): { sessionId: string; tmuxName: string | undefined } { + const sessionId = getOrchestratorSessionId(project); + if (!reserveSessionId(sessionsDir, sessionId)) { + throw new Error( + `Orchestrator session "${sessionId}" already exists. Use ensureOrchestrator() to reuse or restore it.`, + ); } - // Build worker-ID patterns for all other projects. If another project has - // sessionPrefix === orchestratorPrefix (e.g. project B has prefix "app-orchestrator"), - // then its workers are named "app-orchestrator-1", "app-orchestrator-2", etc. — which - // would collide with our orchestrator IDs. Detect this impossible configuration early. - for (const [otherProjectId, otherProject] of Object.entries(config.projects)) { - const otherPrefix = otherProject.sessionPrefix ?? otherProjectId; - if (otherPrefix === project.sessionPrefix) continue; - if (otherPrefix === orchestratorPrefix) { - // Another project's workers are "{otherPrefix}-\d+" which equals "{orchestratorPrefix}-\d+". - // Every candidate ID we would generate collides — fail immediately with a clear message. - throw new Error( - `Cannot spawn orchestrator for project "${project.sessionPrefix}": the orchestrator ID prefix "${orchestratorPrefix}" ` + - `conflicts with the session prefix of project "${otherProjectId}" ("${otherPrefix}"). ` + - `Rename one of the project sessionPrefix values to avoid this overlap.`, - ); - } - } - - let num = 1; - for (let attempts = 0; attempts < 10_000; attempts++) { - if (!usedNumbers.has(num)) { - const sessionId = `${orchestratorPrefix}-${num}`; - const tmuxName = config.configPath - ? generateTmuxName(project.storageKey, orchestratorPrefix, num) - : undefined; - if (reserveSessionId(sessionsDir, sessionId)) { - return { num, sessionId, tmuxName }; - } - } - num += 1; - } - - throw new Error( - `Failed to reserve orchestrator session ID after 10000 attempts (prefix: ${orchestratorPrefix})`, - ); + return { + sessionId, + tmuxName: config.configPath ? `${requireStorageKey(project.storageKey)}-${sessionId}` : undefined, + }; } /** Resolve which plugins to use for a project. */ @@ -1530,13 +1499,11 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM project.orchestratorSessionStrategy, ); - // Reserve a new unique orchestrator identity (e.g. {prefix}-orchestrator-1, -2, ...). - // Each spawnOrchestrator call gets its own numbered session and isolated worktree. - const identity = reserveNextOrchestratorIdentity(project, sessionsDir); + const identity = reserveFixedOrchestratorIdentity(project, sessionsDir); const sessionId = identity.sessionId; const tmuxName = identity.tmuxName; - // Each orchestrator gets an isolated worktree on its own branch. + // The main orchestrator is deterministic, but still uses an isolated worktree. const branch = `orchestrator/${sessionId}`; if (!plugins.workspace) { @@ -1798,6 +1765,85 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM return session; } + async function waitForConcurrentOrchestrator(sessionId: string): Promise { + const deadline = Date.now() + ENSURE_ORCHESTRATOR_CONFLICT_WAIT_MS; + while (Date.now() < deadline) { + const existing = await get(sessionId); + if (existing?.metadata["role"] === "orchestrator") { + return existing; + } + await sleep(ENSURE_ORCHESTRATOR_CONFLICT_POLL_MS); + } + return null; + } + + async function ensureOrchestratorInternal(orchestratorConfig: OrchestratorSpawnConfig): Promise { + const project = config.projects[orchestratorConfig.projectId]; + if (!project) { + throw new Error(`Unknown project: ${orchestratorConfig.projectId}`); + } + + const sessionId = getOrchestratorSessionId(project); + const existing = await get(sessionId); + if (existing) { + const orchestratorSessionStrategy = normalizeOrchestratorSessionStrategy( + project.orchestratorSessionStrategy, + ); + if ( + orchestratorSessionStrategy === "delete" || + orchestratorSessionStrategy === "ignore" + ) { + await kill(sessionId, { purgeOpenCode: orchestratorSessionStrategy === "delete" }); + return spawnOrchestrator(orchestratorConfig); + } + if (existing.lifecycle.session.state === "done") { + throw new SessionNotRestorableError( + sessionId, + `canonical orchestrator session is terminal with status "${existing.status}". Remove or clean up this session before starting a new orchestrator.`, + ); + } + if (isRestorable(existing)) { + return restore(sessionId); + } + if (!isTerminalSession(existing)) { + return existing; + } + throw new SessionNotRestorableError( + sessionId, + `canonical orchestrator session is terminal with status "${existing.status}". Remove or clean up this session before starting a new orchestrator.`, + ); + } + + try { + return await spawnOrchestrator(orchestratorConfig); + } catch (err) { + if (!isFixedOrchestratorReservationError(err, sessionId)) { + throw err; + } + + const concurrent = await waitForConcurrentOrchestrator(sessionId); + if (concurrent) return concurrent; + throw err; + } + } + + async function ensureOrchestrator(orchestratorConfig: OrchestratorSpawnConfig): Promise { + const project = config.projects[orchestratorConfig.projectId]; + if (!project) { + throw new Error(`Unknown project: ${orchestratorConfig.projectId}`); + } + + const sessionId = getOrchestratorSessionId(project); + const existingPromise = ensureOrchestratorPromises.get(sessionId); + if (existingPromise) return existingPromise; + + const promise = ensureOrchestratorInternal(orchestratorConfig).finally(() => { + ensureOrchestratorPromises.delete(sessionId); + }); + ensureOrchestratorPromises.set(sessionId, promise); + return promise; + } + async function list(projectId?: string): Promise { const allSessions = Object.entries(config.projects).flatMap(([entryProjectId, project]) => { if (projectId && entryProjectId !== projectId) return []; @@ -2914,6 +2960,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM return { spawn, spawnOrchestrator, + ensureOrchestrator, restore, list, listCached, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 295a1c478..bb361a30f 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -345,6 +345,9 @@ export function isOrchestratorSession( return false; } const escaped = sessionPrefix.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + if (session.id === `${sessionPrefix}-orchestrator`) { + return true; + } if (!new RegExp(`^${escaped}-orchestrator-\\d+$`).test(session.id)) { return false; } @@ -1723,6 +1726,7 @@ export interface KillOptions { export interface SessionManager { spawn(config: SessionSpawnConfig): Promise; spawnOrchestrator(config: OrchestratorSpawnConfig): Promise; + ensureOrchestrator(config: OrchestratorSpawnConfig): Promise; restore(sessionId: SessionId): Promise; list(projectId?: string): Promise; get(sessionId: SessionId): Promise; diff --git a/packages/web/src/__tests__/api-routes.test.ts b/packages/web/src/__tests__/api-routes.test.ts index d48bad073..6cfecdc9d 100644 --- a/packages/web/src/__tests__/api-routes.test.ts +++ b/packages/web/src/__tests__/api-routes.test.ts @@ -127,6 +127,7 @@ const mockSessionManager: SessionManager = { }), cleanup: vi.fn(async () => ({ killed: [], skipped: [], errors: [] })), spawnOrchestrator: vi.fn(), + ensureOrchestrator: vi.fn(), remap: vi.fn(async () => "ses_mock"), restore: vi.fn(async (id: string) => { const session = testSessions.find((s) => s.id === id); @@ -372,7 +373,7 @@ describe("API Routes", () => { lastActivityAt: new Date("2026-04-19T09:00:00.000Z"), }); const newerLive = makeSession({ - id: "my-app-orchestrator-2", + id: "my-app-orchestrator", projectId: "my-app", metadata: { role: "orchestrator" }, lastActivityAt: new Date("2026-04-19T10:00:00.000Z"), @@ -404,10 +405,10 @@ describe("API Routes", () => { expect(res.status).toBe(200); const data = await res.json(); - expect(data.orchestratorId).toBe("my-app-orchestrator-2"); + expect(data.orchestratorId).toBe("my-app-orchestrator"); expect(data.orchestrators.map((session: { id: string }) => session.id)).toEqual([ + "my-app-orchestrator", "my-app-orchestrator-1", - "my-app-orchestrator-2", ]); expect(data.sessions).toEqual([]); expect(mockSessionManager.listCached).toHaveBeenCalledWith("my-app"); @@ -956,7 +957,7 @@ describe("API Routes", () => { it("returns a guided recovery message for registered orchestrator worktree collisions", async () => { (mockSessionManager.spawnOrchestrator as ReturnType).mockRejectedValueOnce( new Error( - 'Worktree path "/Users/test/.worktrees/my-app/my-app-orchestrator-1" already exists and is still registered with git', + 'Worktree path "/Users/test/.worktrees/my-app/my-app-orchestrator" already exists and is still registered with git', ), ); diff --git a/packages/web/src/__tests__/orchestrators.test.tsx b/packages/web/src/__tests__/orchestrators.test.tsx index a65f707ce..8aaba6d00 100644 --- a/packages/web/src/__tests__/orchestrators.test.tsx +++ b/packages/web/src/__tests__/orchestrators.test.tsx @@ -39,7 +39,7 @@ describe("Orchestrators Page (OrchestratorsRoute)", () => { const mockSessionManager = { list: vi.fn().mockResolvedValue([ { - id: "app-orchestrator-1", + id: "app-orchestrator", projectId: "my-app", status: "working", activity: "active", @@ -66,7 +66,7 @@ describe("Orchestrators Page (OrchestratorsRoute)", () => { render(jsx); expect(screen.getByText("My App")).toBeInTheDocument(); - expect(screen.getByText("app-orchestrator-1")).toBeInTheDocument(); + expect(screen.getByText("app-orchestrator")).toBeInTheDocument(); }); it("shows error when project is missing in searchParams", async () => { diff --git a/packages/web/src/components/ProjectSidebar.tsx b/packages/web/src/components/ProjectSidebar.tsx index e39aefdfd..8164f7c87 100644 --- a/packages/web/src/components/ProjectSidebar.tsx +++ b/packages/web/src/components/ProjectSidebar.tsx @@ -9,6 +9,7 @@ import { getAttentionLevel, type DashboardSession, type AttentionLevel } from "@ import { isOrchestratorSession } from "@aoagents/ao-core/types"; import { getSessionTitle, humanizeBranch } from "@/lib/format"; import { usePopoverClamp } from "@/hooks/usePopoverClamp"; +import { getOrchestratorSessionId } from "@/lib/orchestrator-utils"; import { projectDashboardPath, projectSessionPath } from "@/lib/routes"; import { ThemeToggle } from "./ThemeToggle"; import { AddProjectModal } from "./AddProjectModal"; @@ -389,10 +390,12 @@ function ProjectSidebarInner({ const visibleSessions = workerSessions; const hasActiveSessions = visibleSessions.length > 0; + const projectPrefix = prefixByProject.get(project.id); + const canonicalOrchestratorId = projectPrefix + ? getOrchestratorSessionId({ sessionPrefix: projectPrefix }) + : null; const orchestratorSession = sessions?.find( - (s) => - isOrchestratorSession(s, prefixByProject.get(s.projectId), allPrefixes) && - s.projectId === project.id, + (s) => s.projectId === project.id && s.id === canonicalOrchestratorId, ); return ( diff --git a/packages/web/src/lib/orchestrator-utils.ts b/packages/web/src/lib/orchestrator-utils.ts index f32758e79..b057654c9 100644 --- a/packages/web/src/lib/orchestrator-utils.ts +++ b/packages/web/src/lib/orchestrator-utils.ts @@ -1,7 +1,14 @@ -import type { Session } from "@aoagents/ao-core"; -import { isOrchestratorSession, isTerminalSession } from "@aoagents/ao-core/types"; +import { + isOrchestratorSession, + isTerminalSession, + type Session, +} from "@aoagents/ao-core/types"; import type { Orchestrator } from "@/components/OrchestratorSelector"; +export function getOrchestratorSessionId(project: { sessionPrefix: string }): string { + return `${project.sessionPrefix}-orchestrator`; +} + /** * Filter and map sessions to orchestrator DTOs. * Shared between page.tsx and API route to ensure consistent orchestrator listing. @@ -12,8 +19,14 @@ export function mapSessionsToOrchestrators( projectName: string, allSessionPrefixes?: string[], ): Orchestrator[] { + const canonicalId = getOrchestratorSessionId({ sessionPrefix }); return sessions .filter((s) => isOrchestratorSession(s, sessionPrefix, allSessionPrefixes) && !isTerminalSession(s)) + .sort((a, b) => { + if (a.id === canonicalId) return -1; + if (b.id === canonicalId) return 1; + return (b.lastActivityAt?.getTime() ?? 0) - (a.lastActivityAt?.getTime() ?? 0); + }) .map((s) => ({ id: s.id, projectId: s.projectId,