Repository navigation
feat: orchestrator chats that delegate to worker threads - #14764
BhansaliMadhav wants to merge 1 commit into
Conversation
An orchestrator chat plans work and spawns worker threads in their own git worktrees, or in an existing checkout. When a worker finishes, fails, or needs the user, the server starts a turn in the orchestrator chat with that news, so the orchestrator ends its turn instead of blocking on a wait tool. Generated with Claude Code (Opus 5.5).
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| Predicate.isObject(activity.payload) && typeof activity.payload.requestId === "string" | ||
| ? activity.payload.requestId | ||
| : null; | ||
| const closed = new Set( |
There was a problem hiding this comment.
🟡 Medium orchestration/workerStatus.ts:116
pendingWorkerRequests drops a request when its ID was resolved earlier, even if a later activity reopens that same ID, so get_worker and worker nudges omit the details while the worker is waiting for input. The global closed set is built before requests are examined; process activities in order and remove an ID only when its current request is resolved.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/workerStatus.ts around line 116:
`pendingWorkerRequests` drops a request when its ID was resolved earlier, even if a later activity reopens that same ID, so `get_worker` and worker nudges omit the details while the worker is waiting for input. The global `closed` set is built before requests are examined; process activities in order and remove an ID only when its current request is resolved.
| const start = Effect.fn("WorkerNudgeReactor.start")(function* () { | ||
| // Seed before forking: a worker that changes after this point must still | ||
| // count as a change once its event arrives. | ||
| yield* seed; |
There was a problem hiding this comment.
🟡 Medium orchestration/WorkerNudgeReactor.ts:199
A worker that changes from running to idle or error while seed is running produces no nudge, so its completion/failure is silently lost until a later status transition. Because the hot streamDomainEvents subscription is created only after yield* seed, that event is not replayed and lastStatus remains the seeded running value; coordinate startup so events during seeding are observed and processed against the pre-seed state (or replay events from a captured sequence).
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/WorkerNudgeReactor.ts around line 199:
A worker that changes from `running` to `idle` or `error` while `seed` is running produces no nudge, so its completion/failure is silently lost until a later status transition. Because the hot `streamDomainEvents` subscription is created only after `yield* seed`, that event is not replayed and `lastStatus` remains the seeded `running` value; coordinate startup so events during seeding are observed and processed against the pre-seed state (or replay events from a captured sequence).
| setDraftThreadContext(currentRouteTarget.draftId, pickExplicitWorkspaceOptions(options)); | ||
| } | ||
| if (options?.orchestrator !== undefined) { | ||
| setDraftThreadContext(currentRouteTarget.draftId, { orchestrator: options.orchestrator }); |
There was a problem hiding this comment.
🟡 Medium hooks/useHandleNewThread.ts:368
The active-empty-draft reuse path enables orchestrator even when the target environment does not advertise orchestratorChats. This bypasses the resolved orchestrator value and creates a draft mode the server cannot support; use the resolved value here as in the other paths.
| setDraftThreadContext(currentRouteTarget.draftId, { orchestrator: options.orchestrator }); | |
| setDraftThreadContext(currentRouteTarget.draftId, { orchestrator }); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/hooks/useHandleNewThread.ts around line 368:
The active-empty-draft reuse path enables `orchestrator` even when the target environment does not advertise `orchestratorChats`. This bypasses the resolved `orchestrator` value and creates a draft mode the server cannot support; use the resolved value here as in the other paths.
| [WS_METHODS.orchestrationRemoveWorkerWorkspace]: (input) => | ||
| observeRpcEffect( | ||
| WS_METHODS.orchestrationRemoveWorkerWorkspace, | ||
| workerWorkspace.remove(input.threadId).pipe( |
There was a problem hiding this comment.
🟡 Medium src/ws.ts:3688
orchestrationRemoveWorkerWorkspace deletes the worker worktree but leaves subscribed clients showing the stale worktree/branch status until an unrelated refresh or polling interval occurs. Unlike vcsRemoveWorktree, this handler never calls refreshGitStatus for the affected repository; refresh the affected repository (or repositories) after workerWorkspace.remove completes.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/ws.ts around line 3688:
`orchestrationRemoveWorkerWorkspace` deletes the worker worktree but leaves subscribed clients showing the stale worktree/branch status until an unrelated refresh or polling interval occurs. Unlike `vcsRemoveWorktree`, this handler never calls `refreshGitStatus` for the affected repository; refresh the affected repository (or repositories) after `workerWorkspace.remove` completes.
| const parentKey = scopedThreadKey( | ||
| scopeThreadRef(thread.environmentId, thread.orchestration.parentThreadId), | ||
| ); | ||
| if (orchestratorKeys.has(parentKey)) { |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:2674
Workers from excluded projects are rendered under an in-scope orchestrator, so selecting a project scope still exposes and makes those workers searchable/actionable. The nesting branch checks only orchestratorKeys and bypasses the worker's own project filter; require inScope(thread) before adding it to workersByParent.
| if (orchestratorKeys.has(parentKey)) { | |
| if (orchestratorKeys.has(parentKey) && inScope(thread)) { |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 2674:
Workers from excluded projects are rendered under an in-scope orchestrator, so selecting a project scope still exposes and makes those workers searchable/actionable. The nesting branch checks only `orchestratorKeys` and bypasses the worker's own project filter; require `inScope(thread)` before adding it to `workersByParent`.
| orderedThreads.map( | ||
| // Nested workers resolve for row actions but stay out of the ordered list, | ||
| // so drag and keyboard traversal only walk top-level rows. | ||
| [...orderedThreads, ...[...workersByParentKey.values()].flat()].map( |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:3019
Settling or snoozing an open worker always creates a new draft instead of navigating to the next visible thread. Because worker keys are added to threadByKey but omitted from orderedThreadKeys, planForwardNavigation gets indexOf(workerKey) === -1 and takes the handleNewThread fallback even when other active cards exist. Keep workers out of drag and keyboard traversal, but provide the navigation planner a list that includes nested workers.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 3019:
Settling or snoozing an open worker always creates a new draft instead of navigating to the next visible thread. Because worker keys are added to `threadByKey` but omitted from `orderedThreadKeys`, `planForwardNavigation` gets `indexOf(workerKey) === -1` and takes the `handleNewThread` fallback even when other active cards exist. Keep workers out of drag and keyboard traversal, but provide the navigation planner a list that includes nested workers.
| envMode: | ||
| options.envMode ?? (nextWorktreePath ? "worktree" : (existing.envMode ?? "local")), | ||
| startFromOrigin: nextStartFromOrigin, | ||
| ...((options.orchestrator ?? existing.orchestrator) ? { orchestrator: true } : {}), |
There was a problem hiding this comment.
🟡 Medium src/composerDraftStore.ts:2804
setDraftThreadContext cannot disable orchestrator mode: when an existing draft has orchestrator: true, passing { orchestrator: false } omits the property from nextDraftThread, so the existing true remains in the draft and the submitted thread is still an orchestrator chat. Preserve the explicit false value instead of conditionally spreading only true.
| ...((options.orchestrator ?? existing.orchestrator) ? { orchestrator: true } : {}), | |
| ...((options.orchestrator ?? existing.orchestrator) !== undefined | |
| ? { orchestrator: options.orchestrator ?? existing.orchestrator } | |
| : {}), |
Also found in 2 other location(s)
apps/web/src/components/chat/ChatComposer.tsx:1827
toggleDraftOrchestratorcannot turn an orchestrator draft back into normal mode. It passes{ orchestrator: false }, butsetDraftThreadContextmerges this as(options.orchestrator ?? existing.orchestrator), so an existingtrueremains true. Thus clicking the advertised "normal chat" toggle leaves the draft in orchestrator mode and the subsequently created thread is still an orchestrator.
apps/web/src/components/chat/ChatComposer.tsx:5046
The newly exposed
onToggleOrchestratorinvokes a toggle that passes{ orchestrator: false }when an orchestrator draft is switched back to normal, butsetDraftThreadContextmerges it asoptions.orchestrator ?? existing.orchestrator. Thusfalsefalls back to the existingtruevalue and the draft remains orchestrator; users can enable this mode but cannot disable it through this control.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/composerDraftStore.ts around line 2804:
`setDraftThreadContext` cannot disable orchestrator mode: when an existing draft has `orchestrator: true`, passing `{ orchestrator: false }` omits the property from `nextDraftThread`, so the existing `true` remains in the draft and the submitted thread is still an orchestrator chat. Preserve the explicit `false` value instead of conditionally spreading only `true`.
Also found in 2 other location(s):
- apps/web/src/components/chat/ChatComposer.tsx:1827 -- `toggleDraftOrchestrator` cannot turn an orchestrator draft back into normal mode. It passes `{ orchestrator: false }`, but `setDraftThreadContext` merges this as `(options.orchestrator ?? existing.orchestrator)`, so an existing `true` remains true. Thus clicking the advertised "normal chat" toggle leaves the draft in orchestrator mode and the subsequently created thread is still an orchestrator.
- apps/web/src/components/chat/ChatComposer.tsx:5046 -- The newly exposed `onToggleOrchestrator` invokes a toggle that passes `{ orchestrator: false }` when an orchestrator draft is switched back to normal, but `setDraftThreadContext` merges it as `options.orchestrator ?? existing.orchestrator`. Thus `false` falls back to the existing `true` value and the draft remains orchestrator; users can enable this mode but cannot disable it through this control.
| // collide with upstream's next migration or hide all of upstream's later | ||
| // ones. These steps are idempotent and run after every full migration. | ||
| if (toMigrationInclusive === undefined) { | ||
| yield* ensureProjectionThreadsOrchestration; |
There was a problem hiding this comment.
🟠 High persistence/Migrations.ts:178
Concurrent initialization fails for the second process with a duplicate-column error when adding orchestration_json, so it cannot complete startup despite busy_timeout being configured. ensureProjectionThreadsOrchestration runs as separate autocommit statements after the numbered migrator releases its lock, allowing both processes to observe the column as absent; serialize these steps under the migration lock or make the column addition safely idempotent.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/persistence/Migrations.ts around line 178:
Concurrent initialization fails for the second process with a duplicate-column error when adding `orchestration_json`, so it cannot complete startup despite `busy_timeout` being configured. `ensureProjectionThreadsOrchestration` runs as separate autocommit statements after the numbered migrator releases its lock, allowing both processes to observe the column as absent; serialize these steps under the migration lock or make the column addition safely idempotent.
| .execute({ | ||
| operation: "WorkerWorkspace.listWorktrees", | ||
| cwd: repoRoot, | ||
| args: ["worktree", "list", "--porcelain"], |
There was a problem hiding this comment.
🟠 High orchestration/WorkerWorkspace.ts:369
useExisting rejects valid checkouts whose paths contain newlines, returning not-a-checkout, because listWorktrees parses newline-delimited output and splits that path into separate records. Use git worktree list --porcelain -z and a NUL-aware parser so such paths remain intact.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/WorkerWorkspace.ts around line 369:
`useExisting` rejects valid checkouts whose paths contain newlines, returning `not-a-checkout`, because `listWorktrees` parses newline-delimited output and splits that path into separate records. Use `git worktree list --porcelain -z` and a NUL-aware parser so such paths remain intact.
| Effect.map((thread) => { | ||
| const orchestration = Option.getOrUndefined(thread)?.orchestration; | ||
| return orchestration?.role === "worker" | ||
| ? orchestration.repos.map((repo) => pathService.join(repo.repoRoot, ".git")) |
There was a problem hiding this comment.
🟠 High Layers/ProviderService.ts:962
Workers using an existing linked-worktree checkout cannot commit in workspace-write mode because workerWritableRoots grants only repoRoot/.git, which is a gitdir pointer file rather than the mutable common Git directory. Resolve the checkout's actual common Git directory before adding it to extraWritableRoots.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/ProviderService.ts around line 962:
Workers using an existing linked-worktree checkout cannot commit in `workspace-write` mode because `workerWritableRoots` grants only `repoRoot/.git`, which is a gitdir pointer file rather than the mutable common Git directory. Resolve the checkout's actual common Git directory before adding it to `extraWritableRoots`.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a broad orchestrator/worker feature spanning MCP, orchestration, persistence, provider sandboxes, git workspaces, authentication, and the web UI, while also adding product defaults. Unresolved critical and high-severity findings include deleting active workspaces, migration races, checkout handling, and worker commit permissions. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
Note This comment is posted by Julius' dot Closing this because the new orchestrator chat mode, worker tools and automatic follow-up turns need prior approval of direction and scope. #14762 has no maintainer approval yet. The PR also explicitly omits the required UI evidence. Please continue the discussion about keeping chats usable while workers run. If maintainers approve the scope, add before/after screenshots and a short recording of worker updates and question handling, then request reconsideration. |
2 participants
Hey Theo, big fan! I built this with a coding agent in a few hours. Proposal and context: #14762. I know this needs maintainer approval of the direction first, and I'm happy for it to wait on that discussion, or be split or closed.
Closes discussions
Problem
An agent that hands work to other agents has to block on a wait call (or poll) until they finish. That ties up the chat, so the user can't use it meanwhile.
What this does
ThreadOrchestrationin contracts). Orchestrator threads get a new T3 MCP toolkit:list_projects,spawn_worker,list_workers,get_worker,send_to_worker,interrupt_worker,remove_worker_workspace. Workers are child threads, nested in the sidebar and listed in a Workers panel.WorkerNudgeReactorstarts a turn in the orchestrator chat when a worker finishes, fails, or starts waiting on the user. The update includes the end of the worker's reply or its question. Updates for a busy orchestrator are held, then sent as one turn once it is idle. There is no blocking wait tool, so no provider tool-call timeouts are involved.WorkerWorkspacegives each worker its own worktree, or a folder with one worktree per repo for multi-repo workers. A worker can instead use the project's checkout or an existing worktree of the project's repo, and a server setting picks the default (Settings → General → Where workers run).{ role: "orchestrator" }; workers can only be created by the server.worktreePathis cleared.Verification
WorkerNudgeReactor: idle orchestrator, busy orchestrator, combined updates, questions, startup state, archived orchestrator.WorkerWorkspaceagainst real git: create, rollback, unsafe-path refusal, git failure, existing checkout and worktree, refusal to remove borrowed checkouts.ProviderServicegranting the orchestrator capability only to orchestrator chats.Not done yet:
orchestration_jsoncolumn is added by an idempotent step after the numbered migrations, because this lives in my fork. For upstream it should become a normal numbered migration, and I'm happy to change that.Built with Claude Code (Opus 5.5).