feat: conductor session + read-only fleet tools (P3) - #124
Merged
Conversation
The conductor becomes a real, reachable session — not just P2 machinery. - role:"conductor" on session.create requests THE per-tenant conductor (singleton, idempotent): the daemon picks its name/workdir/provider, creates it on first request, and returns the existing one after. Persisted (Store + transcript meta) so it self-persists across restarts — one stable session. - codeoid_fleet in-process MCP server, injected only for the conductor: fleet_list / fleet_find (P1 cross-workspace resolution) / fleet_summary (episode digest, never raw scrollback) / fleet_recall / machine_map. Tools close over the manager's live tenant-scoped session view. Read-only by construction — no send/spawn (that's P4). Every call audits under the conductor's WIMSE URI. - Conductor identity wired (P2 -> live): creating the conductor calls registerConductor(ownerSub) and mints its working token by OWNER delegation (RFC 8693). The verified bearer token is retained per-connection (SocketData) as the delegation subject — never logged/persisted. - Per-session provider selection: each Session picks its backend from providerId (config.conductor.provider for the conductor), so any session — the conductor included — can run on claude/gemini/openai or a future open-weight provider. Stateless providers get a SessionProvider adapter. - Claude provider: allowedTools widened with mcp__codeoid_fleet__*; the system-prompt append no longer gates on memory (the conductor contract rides the claude_code preset). - Owner scopes: session:read / session:dispatch added to the CLI + web login scope sets so the owner->conductor delegation has a non-empty intersection. - UX: `codeoid attach conductor` create-or-gets the conductor from any client. - Unknown/future roles fail closed with a clear error (never a silent downgrade to a normal session); the wire frame still parses. Tests: fleet handlers (read surface, find resolution, conductor self-exclusion, memory-off fallback, audit) + conductor session lifecycle (singleton, role + provider persistence, tenancy isolation, resume, fail-closed roles). 1066 unit tests green; typecheck + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a conductor session role, read-only fleet MCP tools, and related provider, session, storage, client, and config support. It also updates scopes, documentation, and tests to cover conductor creation, resume, and fleet access. ChangesConductor Session and Fleet MCP Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
+ Coverage 76.70% 77.10% +0.39%
==========================================
Files 87 89 +2
Lines 13965 14412 +447
==========================================
+ Hits 10712 11112 +400
- Misses 3253 3300 +47
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
docs/conductor-design.md (2)
112-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMalformed markdown italics.
The parenthetical
*(Implemented in P3:* ... *)*uses unbalanced single asterisks (opens italics mid-word, closes oddly at the end), which will likely render incorrectly in most Markdown viewers. Wrap the whole parenthetical in one matched pair, e.g._(Implemented in P3: ...)_.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/conductor-design.md` around lines 112 - 117, The parenthetical markdown in the conductor design doc is malformed because the italics markers are unbalanced around the “Implemented in P3” note. Update the affected prose in the conductor design content so the entire parenthetical is wrapped in one matching emphasis pair, keeping the `Implemented in P3` text and the surrounding clause intact. Locate the malformed emphasis near the conductor/session/fleet server description and replace the stray asterisks with a single consistent markdown style.
119-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDoc now self-contradicts on the fleet tool surface.
This rewrite states the P3 surface is read-only (
fleet_list/fleet_find/fleet_summary/fleet_recall/machine_map) with "No send/spawn/interrupt tool exists yet." That's consistent withsrc/daemon/fleet.ts(FLEET_TOOL_NAMES), but the unchanged §5 table (lines ~191-199) and the "Build phases" list (lines ~321-324) still documentfleet_spawn,fleet_send,fleet_interrupt, andfleet_destroyas part of the current tool surface without qualifying them as future/unimplemented. A reader landing on §5 or §11 after this section will get conflicting information about what's actually available today.Consider adding an explicit "(not yet implemented — P4)" marker next to the write tools in §5, or moving them into a clearly-labeled "planned" sub-table, to keep the doc internally consistent with this new P3 read-only framing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/conductor-design.md` around lines 119 - 129, The documentation in the conductor design section conflicts with later references to fleet write tools, so update the affected tables to match the new read-only P3 framing. In the §5 tool surface table and the Build phases list, mark fleet_spawn, fleet_send, fleet_interrupt, and fleet_destroy as not yet implemented (P4) or move them into a clearly labeled planned section, while keeping the current read-only fleet surface consistent with FLEET_TOOL_NAMES and the conductor description.src/terminal/client.ts (1)
401-405: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRole-fallback match is unreachable dead code.
resp.sessions.find((s) => s.role === nameOrId)can only ever match whennameOrId === "conductor"(the only valueSessionInfo.rolecan hold), but that case is already short-circuited at line 382 and returns before this code runs. As written, this fallback never fires for any input.♻️ Either drop the dead fallback or explain intended future use
const resp = await this.#request({ type: "session.list", id: randomUUID() }); if (resp.type === "session.list.result") { - const match = - resp.sessions.find((s) => s.name === nameOrId) ?? - resp.sessions.find((s) => s.role === nameOrId); + const match = resp.sessions.find((s) => s.name === nameOrId); if (match) return match.id;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/terminal/client.ts` around lines 401 - 405, The role-based fallback in the session lookup is dead code because the only possible SessionInfo.role value is already handled earlier in the flow. Update the matching logic in src/terminal/client.ts around the session list lookup to either remove the unreachable resp.sessions.find((s) => s.role === nameOrId) fallback or replace it with a clearly intended future-use path, and keep the primary nameOrId lookup in the same request-handling block.src/daemon/session.ts (1)
526-535: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
onModelsisn't wired for the gemini/openai branches.Only the Claude branch forwards
onModels, so a gemini/openai session never reports its catalog. Since#currentModelshas no baked-in fallback for non-claude providers,models.listreturns empty for those sessions and the picker stays blank. If that's intended for this phase, ignore; otherwise consider reportinglistModels()after construction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/daemon/session.ts` around lines 526 - 535, The gemini and openai branches in the session provider factory are not forwarding model updates, so those sessions never populate their catalog. Update the session construction logic in the provider switch that creates StatelessSessionProvider with GeminiProvider and OpenAIProvider to wire in the same onModels/listModels reporting flow used by the Claude branch. Make sure the session’s model state is refreshed after construction so models.list is populated for these providers instead of remaining empty.src/daemon/providers/stateless.ts (1)
71-77: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff
teardown()should interrupt the active stateless turn.
Session#teardownProvider()runs on model switch, rotation, and destroy, butStatelessSessionProvider.teardown()is a no-op and never interrupts the currentTurnRun. That leaves in-flight Gemini/OpenAI requests open until they finish. Track the active run and callinterrupt()fromteardown().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/daemon/providers/stateless.ts` around lines 71 - 77, StatelessSessionProvider.teardown() is currently a no-op and does not stop the active TurnRun, so in-flight requests keep running during provider teardown. Update StatelessSessionProvider to track the currently active run started by its turn execution path, and have teardown() call interrupt() on that active run before cleanup. Keep dispose() as final resource cleanup, but ensure teardown() interrupts any ongoing turn on model switch, rotation, or destroy.src/daemon/store.ts (1)
51-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
sessions.role/sessions.providerlook redundant.
Session.toInfo()and transcript meta already carry the conductor role and provider across restarts, butStore.listSessions()/getSession()never surface these columns. If they aren’t needed for migrations or direct DB queries, drop them or wire them into the Store read APIs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/daemon/store.ts` around lines 51 - 55, The `sessions.role` and `sessions.provider` columns in `Store` appear redundant because `Session.toInfo()` and transcript metadata already persist that state, but `Store.listSessions()` and `getSession()` do not read them back. Either remove the `#addColumnIfMissing` calls for these fields in `Store` if they are only migration leftovers, or update the Store read paths and related `Session`/session-info mapping so these columns are actually surfaced through the API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/daemon/session-manager.ts`:
- Around line 983-1040: The conductor creation path in session-manager.ts has a
TOCTOU race: the initial `#conductorFor`(auth.accountId, auth.projectId) check
happens before async identity work, so concurrent session.create calls can both
proceed and create duplicate conductors. Fix this by adding a second synchronous
guard immediately before constructing and registering the new Session (around
the existing Session creation and this.#sessions.set flow), or by memoizing an
in-flight creation promise keyed by account/project so only one conductor can be
created per tenant at a time.
In `@src/terminal/client.ts`:
- Around line 382-395: The `attach conductor` alias can be shadowed because
`TerminalClient` still creates a normal session with the literal name
`conductor`, while `SessionManager` accepts that name in the regular
`session.create` path. Update the `nameOrId === "conductor"` handling in
`TerminalClient` and the `session.create` flow in `SessionManager` so the
singleton conductor session is reserved: either reject the literal name for
user-created sessions or remap it to the special singleton identifier before
creation, and keep the alias resolution separate from regular session names.
---
Nitpick comments:
In `@docs/conductor-design.md`:
- Around line 112-117: The parenthetical markdown in the conductor design doc is
malformed because the italics markers are unbalanced around the “Implemented in
P3” note. Update the affected prose in the conductor design content so the
entire parenthetical is wrapped in one matching emphasis pair, keeping the
`Implemented in P3` text and the surrounding clause intact. Locate the malformed
emphasis near the conductor/session/fleet server description and replace the
stray asterisks with a single consistent markdown style.
- Around line 119-129: The documentation in the conductor design section
conflicts with later references to fleet write tools, so update the affected
tables to match the new read-only P3 framing. In the §5 tool surface table and
the Build phases list, mark fleet_spawn, fleet_send, fleet_interrupt, and
fleet_destroy as not yet implemented (P4) or move them into a clearly labeled
planned section, while keeping the current read-only fleet surface consistent
with FLEET_TOOL_NAMES and the conductor description.
In `@src/daemon/providers/stateless.ts`:
- Around line 71-77: StatelessSessionProvider.teardown() is currently a no-op
and does not stop the active TurnRun, so in-flight requests keep running during
provider teardown. Update StatelessSessionProvider to track the currently active
run started by its turn execution path, and have teardown() call interrupt() on
that active run before cleanup. Keep dispose() as final resource cleanup, but
ensure teardown() interrupts any ongoing turn on model switch, rotation, or
destroy.
In `@src/daemon/session.ts`:
- Around line 526-535: The gemini and openai branches in the session provider
factory are not forwarding model updates, so those sessions never populate their
catalog. Update the session construction logic in the provider switch that
creates StatelessSessionProvider with GeminiProvider and OpenAIProvider to wire
in the same onModels/listModels reporting flow used by the Claude branch. Make
sure the session’s model state is refreshed after construction so models.list is
populated for these providers instead of remaining empty.
In `@src/daemon/store.ts`:
- Around line 51-55: The `sessions.role` and `sessions.provider` columns in
`Store` appear redundant because `Session.toInfo()` and transcript metadata
already persist that state, but `Store.listSessions()` and `getSession()` do not
read them back. Either remove the `#addColumnIfMissing` calls for these fields
in `Store` if they are only migration leftovers, or update the Store read paths
and related `Session`/session-info mapping so these columns are actually
surfaced through the API.
In `@src/terminal/client.ts`:
- Around line 401-405: The role-based fallback in the session lookup is dead
code because the only possible SessionInfo.role value is already handled earlier
in the flow. Update the matching logic in src/terminal/client.ts around the
session list lookup to either remove the unreachable resp.sessions.find((s) =>
s.role === nameOrId) fallback or replace it with a clearly intended future-use
path, and keep the primary nameOrId lookup in the same request-handling block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4052eede-17bb-4bfd-8222-191d570e555a
📒 Files selected for processing (17)
docs/conductor-design.mdpackages/protocol/src/schemas.tspackages/protocol/src/types.tssrc/cli.tssrc/config.tssrc/daemon/fleet.tssrc/daemon/providers/claude/index.tssrc/daemon/providers/stateless.tssrc/daemon/server.tssrc/daemon/session-manager.tssrc/daemon/session.tssrc/daemon/store.tssrc/daemon/transcript.tssrc/terminal/client.tssrc/tests/conductor-session.test.tssrc/tests/fleet.test.tsweb/src/lib/auth.ts
- Close a TOCTOU race in #createConductor: the singleton guard read #conductorFor before awaiting identity registration/token minting, so two concurrent conductor creates for one tenant could both pass and spawn two conductors. Re-check synchronously right before Session construction (the re-check → new Session → #sessions.set runs with no await between), so only one conductor is ever registered per (account, project). Test forces the race with a yielding identity stub; verified it fails without the re-check. - Reserve the conductor's configured display name from the normal session.create path — a regular session named "conductor" would shadow the singleton in session.list. Reject it and point the caller at role:"conductor". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
P3 of the conductor (
docs/conductor-build-plan.md). Builds directly on the P0–P2 foundations merged in #51 — turns the conductor from dormant machinery into a real, reachable session that can see the fleet.Depends on nothing new: the P1 retrieval pipeline and P2 durable identity are already on main.
The conductor session
role: "conductor"onsession.createrequests THE per-tenant conductor — a singleton, idempotent create-or-get. The daemon chooses its name/workdir/provider (fromconfig.conductor); the request's name/workdir are ignored. Persisted (Store columns + transcript meta) so it self-persists across daemon restarts — one stable session, resumed with its role, provider, and fleet tools intact.registerConductor(ownerSub)and mints the conductor's working token by owner delegation (RFC 8693). The verified bearer token is retained per-connection (SocketData.rawToken) purely as the delegation subject — never logged, never persisted.codeoid_fleetMCP server (read-only)Injected only into the conductor session, mirroring
buildMemoryMcpServer:fleet_list— sessions grouped by workspacefleet_find— cross-workspace natural-language resolution (the P1 pipeline): "which session was the authz fix?"fleet_summary— a compressed episode digest of one session, never raw scrollback (the never-OOC guarantee)fleet_recall— cross-fleet episode recallmachine_map— workspaces + git branch/dirty stateRead-only by construction: no send/spawn/interrupt tool exists yet (that's P4), and the conductor identity carries only
session:read/session:dispatch, nevertools:write/tools:execute. Every tool call audits under the conductor's WIMSE URI. AFLEET_TOOL_NAMESguardrail test fails if a send-class tool ever leaks in.Per-session provider selection (your open-weight note)
Each
Sessionnow builds its backend from aproviderId—config.conductor.providerfor the conductor — so any session, the conductor included, can run on a different backend (claude/gemini/openai, or a future open-weight provider once its provider is registered). Stateless providers (Gemini/OpenAI) get a thinSessionProvideradapter. Caveat, logged at create time: MCP tools are only surfaced by the Claude provider today, so a conductor on another provider chats but can't see the fleet yet.Supporting wiring
allowedToolswidened withmcp__codeoid_fleet__*; the system-prompt append no longer gates on memory, so the conductor contract rides theclaude_codepreset.session:read/session:dispatchadded to the CLI + web login scope sets, so the owner→conductor delegation has a non-empty intersection.codeoid attach conductorcreate-or-gets the conductor from any client.Exit criterion
Build plan P3 exit: via
codeoid attach conductor, "which session was the authz fix?" resolves across workspaces; conductor holds only an index, never raw transcripts. The resolution logic (fleet_findcross-workspace), the singleton/attach path, and the digest-not-transcript property are covered by unit tests; the human-in-the-loop attach is a manual/demo step (needs a live Claude turn).Testing
fleet.test.ts(read surface, find resolution, conductor self-exclusion, memory-off fallback, audit, read-only guardrail) andconductor-session.test.ts(singleton, role + provider persistence, tenancy isolation, resume, fail-closed unknown roles).bun run typecheck+bun run lintclean.🤖 Generated with Claude Code
Summary by CodeRabbit