-
Notifications
You must be signed in to change notification settings - Fork 790
fix: prevent ao start from spawning duplicate orchestrators #909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0b7b315
71bdf75
2a1b513
2a0f27a
e0b5aa0
d54ca02
f9abcdf
4e60238
25b3f31
5255f99
a21b976
5e19249
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ import { | |
| generateConfigFromUrl, | ||
| configToYaml, | ||
| normalizeOrchestratorSessionStrategy, | ||
| isOrchestratorSession, | ||
| ConfigNotFoundError, | ||
| type OrchestratorConfig, | ||
| type ProjectConfig, | ||
|
|
@@ -987,32 +988,82 @@ async function runStartup( | |
| } | ||
| } | ||
|
|
||
| // Create orchestrator session (unless --no-orchestrator or already exists) | ||
| // Create orchestrator session (unless --no-orchestrator or existing orchestrators found) | ||
| let tmuxTarget = sessionId; | ||
| let hasExistingOrchestrators = false; | ||
| let selectedOrchestratorId: string | null = null; | ||
|
|
||
| if (opts?.orchestrator !== false) { | ||
| const sm = await getSessionManager(config); | ||
|
|
||
| // Check for existing orchestrator sessions for this project | ||
| let allSessions; | ||
| try { | ||
| spinner.start("Creating orchestrator session"); | ||
| const systemPrompt = generateOrchestratorPrompt({ config, projectId, project }); | ||
| const session = await sm.spawnOrchestrator({ projectId, systemPrompt }); | ||
| if (session.runtimeHandle?.id) { | ||
| tmuxTarget = session.runtimeHandle.id; | ||
| } | ||
| reused = | ||
| orchestratorSessionStrategy === "reuse" && | ||
| session.metadata?.["orchestratorSessionReused"] === "true"; | ||
| spinner.succeed(reused ? "Orchestrator session reused" : "Orchestrator session created"); | ||
| allSessions = await sm.list(projectId); | ||
| } catch (err) { | ||
| spinner.fail("Orchestrator setup failed"); | ||
| spinner.fail("Failed to list sessions"); | ||
| if (dashboardProcess) { | ||
| dashboardProcess.kill(); | ||
| } | ||
| throw new Error( | ||
| `Failed to setup orchestrator: ${err instanceof Error ? err.message : String(err)}`, | ||
| `Failed to list sessions: ${err instanceof Error ? err.message : String(err)}`, | ||
| { cause: err }, | ||
| ); | ||
| } | ||
| const existingOrchestrators = allSessions.filter((s) => | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing allSessionPrefixes parameter. |
||
| isOrchestratorSession(s, project.sessionPrefix ?? projectId), | ||
| ); | ||
harshitsinghbhandari marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (existingOrchestrators.length > 0) { | ||
| // Existing orchestrators found | ||
| if (opts?.dashboard === false) { | ||
| // No dashboard — auto-select the most recently active orchestrator | ||
| const sortedOrchestrators = [...existingOrchestrators].sort( | ||
| (a, b) => b.lastActivityAt.getTime() - a.lastActivityAt.getTime(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lastActivityAt.getTime() will crash on null. Sessions with no activity yet have lastActivityAt as null. |
||
| ); | ||
| const selected = sortedOrchestrators[0]; | ||
| selectedOrchestratorId = selected.id; | ||
| // Use runtimeHandle.id if available, otherwise fall back to the session ID | ||
| tmuxTarget = selected.runtimeHandle?.id ?? selected.id; | ||
| spinner.succeed( | ||
| `Using existing orchestrator session: ${selected.id}` + | ||
| (existingOrchestrators.length > 1 | ||
| ? ` (${existingOrchestrators.length - 1} other session(s) available)` | ||
| : ""), | ||
| ); | ||
| } else { | ||
| // Dashboard available — let the user select | ||
| hasExistingOrchestrators = true; | ||
| spinner.info( | ||
| `Found ${existingOrchestrators.length} existing orchestrator session(s). ` + | ||
| `Open the dashboard to select or start a new one.`, | ||
| ); | ||
| } | ||
| } else { | ||
harshitsinghbhandari marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // No existing orchestrators — spawn a new one | ||
| try { | ||
| spinner.start("Creating orchestrator session"); | ||
| const systemPrompt = generateOrchestratorPrompt({ config, projectId, project }); | ||
| const session = await sm.spawnOrchestrator({ projectId, systemPrompt }); | ||
| selectedOrchestratorId = session.id; | ||
| if (session.runtimeHandle?.id) { | ||
| tmuxTarget = session.runtimeHandle.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 | ||
|
|
@@ -1030,29 +1081,39 @@ async function runStartup( | |
| console.log(chalk.cyan("Lifecycle:"), lifecycleTarget); | ||
| } | ||
|
|
||
| if (opts?.orchestrator !== false && !reused) { | ||
| if (hasExistingOrchestrators) { | ||
| console.log( | ||
| chalk.cyan("Orchestrator:"), | ||
| "existing sessions found — select one in the dashboard", | ||
| ); | ||
| } else if (opts?.orchestrator !== false && !reused) { | ||
| console.log(chalk.cyan("Orchestrator:"), `tmux attach -t ${tmuxTarget}`); | ||
| } else if (reused) { | ||
| console.log(chalk.cyan("Orchestrator:"), `reused existing session (${sessionId})`); | ||
| } | ||
|
|
||
| console.log(chalk.dim(`Config: ${config.configPath}`)); | ||
|
|
||
| // Show next step hint | ||
| const projectIds = Object.keys(config.projects); | ||
| if (projectIds.length > 0) { | ||
| console.log(chalk.bold("\nNext step:\n")); | ||
| console.log(` Spawn an agent session:`); | ||
| console.log(chalk.cyan(` ao spawn <issue-number>\n`)); | ||
| // Show next step hint (only if no existing orchestrators requiring selection) | ||
| if (!hasExistingOrchestrators) { | ||
| const projectIds = Object.keys(config.projects); | ||
| if (projectIds.length > 0) { | ||
| console.log(chalk.bold("\nNext step:\n")); | ||
| console.log(` Spawn an agent session:`); | ||
| console.log(chalk.cyan(` ao spawn <issue-number>\n`)); | ||
| } | ||
| } | ||
|
|
||
| // Auto-open browser to orchestrator session page once the server is accepting connections. | ||
| // Auto-open browser to orchestrator session page (or selection page) once the server is ready. | ||
| // 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 = `http://localhost:${port}/sessions/${sessionId}`; | ||
| // If existing orchestrators found, open the selection page; otherwise open the session page | ||
| const orchestratorUrl = hasExistingOrchestrators | ||
| ? `http://localhost:${port}/orchestrators?project=${projectId}` | ||
| : `http://localhost:${port}/sessions/${selectedOrchestratorId ?? sessionId}`; | ||
| void waitForPortAndOpen(port, orchestratorUrl, openAbort.signal); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existence check only filters by isOrchestratorSession(), but sessionManager.list() returns terminal sessions too (killed, terminated). With --no-dashboard, the code auto-selects the "most recent" orchestrator at line 1021 and prints tmux attach -t ..., which can now point at a dead tmux target instead of spawning a fresh one.
A proper e2e test manually should have definitely caught it, I encourage you to try it out.