Refactor orchestrator-v2 developer APIs#51
Conversation
… and runner augmentation. Unify mutable data registries, replace runOrchestrator with Orchestrator + work-item mappers, add script/task agent registration via augment and loadAgent in agent-3-task, and wire the lvl3 example.
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR replaces ChangesOrchestration and registration API refactor
Sequence Diagram(s)sequenceDiagram
participant App as Example App
participant Orchestrator
participant WorkItemSource as BifrostWorkItemSource
participant Mapper as WorkItemMapper
participant Peer
App->>Orchestrator: new Orchestrator()
App->>Orchestrator: registerWorkItemSource(source)
App->>Orchestrator: addWorkItemMapper("task", mapper)
App->>Orchestrator: start({ identity, authorizedRunners, scheduler })
Orchestrator->>Peer: create peer, subscribe connect/disconnect
Orchestrator->>WorkItemSource: watchWorkItems()
WorkItemSource-->>Orchestrator: raw work item
Orchestrator->>Mapper: transform(rawWorkItem)
Mapper-->>Orchestrator: mapped WorkItem
Orchestrator->>Peer: dispatch(mapped WorkItem)
sequenceDiagram
participant App as Example App
participant Runner
participant LoadAgent as loadAgent
participant Parser as parseAgentDefinition
participant DataRegistry
App->>Runner: new Runner({ data })
App->>Runner: registerEngine("cursor", engine)
App->>LoadAgent: loadAgent("./agents/cowsay/AGENT.md")
LoadAgent->>Parser: parseAgentDefinition(fileContent)
Parser-->>LoadAgent: AgentDefinition | null
LoadAgent-->>App: AgentDefinition
App->>Runner: registerTaskAgent("cowsay", agentDefinition)
Runner->>DataRegistry: get("agentDefinition").register("cowsay", agentDefinition)
App->>Runner: registerScriptAgent("doSomething", fn)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
orchestrator-v2/packages/runner/README.md (1)
39-40: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument
createScriptAgentandScriptFnin the "Lower-level exports" section.These new public exports from
index.ts(line 4) are not listed in the README, whilecreateDataRegistryandRegistry— which are exported at the same level — are documented.📖 Proposed addition
- `createDataRegistry(guards)` — create a typed data registry up front +- `createScriptAgent(fn, name)` — create a script agent work item handler +- `ScriptFn` — script agent function type - `loadRunnerConfig(configPath)` — parse and validate YAML config🤖 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 `@orchestrator-v2/packages/runner/README.md` around lines 39 - 40, The README’s “Lower-level exports” section is missing the newly public exports, so add entries for createScriptAgent and ScriptFn alongside the existing items. Update the documentation in the same style as createDataRegistry and Registry, using the symbols from index.ts so readers can find the new API exports easily.orchestrator-v2/docs/orchestrator.md (1)
82-93: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winConfiguration docs don't reflect the new class-based API.
The Configuration section still shows
OrchestratorOptionswithworkItemSourceas a field, butstart()now takesOrchestratorStartOptionswhich omitsworkItemSource(registered separately viaregisterWorkItemSource) and addsabortSignal. A user following these docs would passworkItemSourcetostart()and get a type error. The docs also don't mention the class-based API (new Orchestrator(),registerWorkItemSource,addWorkItemMapper,start) orabortSignalfor graceful shutdown.📝 Suggested doc update
### Configuration ```typescript +// Step 1: Register the work item source +const orchestrator = new Orchestrator(); +orchestrator.registerWorkItemSource(workItemSource); + +// Step 2: Optionally register per-kind mappers +orchestrator.addWorkItemMapper("task", (workItem) => { + return { ...workItem, state: { ...workItem.metadata } }; +}); + +// Step 3: Start with OrchestratorStartOptions (no workItemSource) type OrchestratorStartOptions = { identity: PeerIdentity; authorizedRunners: ReadonlyMap<string, KeyObject>; scheduler: Scheduler; host?: string; port?: number; heartbeatTimeoutMs?: number; // default 30000 maxInFlightPerPeer?: number; // default 1 abortSignal?: AbortSignal; }; + +const handle = await orchestrator.start(options);</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@orchestrator-v2/docs/orchestrator.mdaround lines 82 - 93, Update the
Configuration section to match the class-based Orchestrator API: replace the
outdated OrchestratorOptions example that includes workItemSource with the
current OrchestratorStartOptions shape, which uses abortSignal and omits
workItemSource. Add a concise usage example showing new Orchestrator(),
registerWorkItemSource(), addWorkItemMapper(), and start() so readers see the
correct flow and where each symbol belongs. Make sure the docs clearly
distinguish registration-time setup from the start() options to avoid type
errors from passing workItemSource into start().</details> <!-- cr-comment:v1:7b27e6c9349301e548c2326b --> </blockquote></details> </blockquote></details>🤖 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 `@orchestrator-v2/examples/lvl3/runner.ts`: - Line 11: The agent loading call in runner.ts is using a path relative to process.cwd(), so it can break when the example is launched from another directory. Update the registerTaskAgent invocation by resolving the AGENT.md location from the module itself in runner.ts, using import.meta.url (and the related path/file URL helpers) to build an absolute path before calling loadAgent. Keep the fix localized to the runner setup so the cowsay agent is loaded reliably regardless of the current working directory. In `@orchestrator-v2/packages/orchestrator/src/orchestrator.ts`: - Around line 102-119: `done` can hang in `orchestrator()` because `await registry.waitForAvailablePeer()` is not cancelable when the abort signal fires. Update the work-item loop to race `registry.waitForAvailablePeer()` against the abort signal (or add a `PeerRegistry.cancelWaiters()` path invoked from `cleanup()`), so the await unblocks immediately on abort and the loop can exit cleanly before `dispatchWorkItem()` runs. Keep the fix centered around `waitForAvailablePeer`, `cleanup`, and the `done` async block. --- Outside diff comments: In `@orchestrator-v2/docs/orchestrator.md`: - Around line 82-93: Update the Configuration section to match the class-based Orchestrator API: replace the outdated OrchestratorOptions example that includes workItemSource with the current OrchestratorStartOptions shape, which uses abortSignal and omits workItemSource. Add a concise usage example showing new Orchestrator(), registerWorkItemSource(), addWorkItemMapper(), and start() so readers see the correct flow and where each symbol belongs. Make sure the docs clearly distinguish registration-time setup from the start() options to avoid type errors from passing workItemSource into start(). In `@orchestrator-v2/packages/runner/README.md`: - Around line 39-40: The README’s “Lower-level exports” section is missing the newly public exports, so add entries for createScriptAgent and ScriptFn alongside the existing items. Update the documentation in the same style as createDataRegistry and Registry, using the symbols from index.ts so readers can find the new API exports easily.🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID:
aca116eb-e13f-46e8-b53e-cda5a59b8081⛔ Files ignored due to path filters (1)
orchestrator-v2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml📒 Files selected for processing (33)
orchestrator-v2/README.mdorchestrator-v2/docs/orchestrator.mdorchestrator-v2/docs/runner.mdorchestrator-v2/examples/lvl3/agents/cowsay/AGENT.mdorchestrator-v2/examples/lvl3/doSomething.tsorchestrator-v2/examples/lvl3/orchestrator.tsorchestrator-v2/examples/lvl3/package.jsonorchestrator-v2/examples/lvl3/runner.tsorchestrator-v2/packages/agent-3-task/package.jsonorchestrator-v2/packages/agent-3-task/src/agent-parser.tsorchestrator-v2/packages/agent-3-task/src/augment.spec.tsorchestrator-v2/packages/agent-3-task/src/augment.tsorchestrator-v2/packages/agent-3-task/src/create-task-agent.tsorchestrator-v2/packages/agent-3-task/src/enroll-task-agent.tsorchestrator-v2/packages/agent-3-task/src/index.tsorchestrator-v2/packages/agent-3-task/src/load-agent.tsorchestrator-v2/packages/agent-3-task/src/run-task-agent.spec.tsorchestrator-v2/packages/interfaces-work/src/index.tsorchestrator-v2/packages/interfaces-work/src/types.tsorchestrator-v2/packages/orchestrator/src/index.tsorchestrator-v2/packages/orchestrator/src/orchestrator.tsorchestrator-v2/packages/orchestrator/src/test-helpers.tsorchestrator-v2/packages/runner/README.mdorchestrator-v2/packages/runner/src/data-registry.spec.tsorchestrator-v2/packages/runner/src/data-registry.tsorchestrator-v2/packages/runner/src/index.tsorchestrator-v2/packages/runner/src/runner.tsorchestrator-v2/packages/runner/src/script-agent.tsorchestrator-v2/packages/runner/src/types.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.spec.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.tsorchestrator-v2/packages/work-item-source-bifrost/src/index.tsorchestrator-v2/pnpm-workspace.yaml💤 Files with no reviewable changes (2)
- orchestrator-v2/packages/interfaces-work/src/index.ts
- orchestrator-v2/packages/agent-3-task/src/enroll-task-agent.ts
1 participant
Summary
runOrchestratorwith anOrchestratorclass that registers work item sources and kind-based mappers before dispatchDataRegistry/Registrymodel (no readonly split, no lazyensure)registerScriptAgent) and task agent augment (registerTaskAgent(name, agent),registerEngine,loadAgentin agent-3-task)RuneDetailfromBifrostWorkItemSourcemetadata and wire the lvl3 exampleTest plan
vp checkpassesvp test— 112 tests passvp run -r buildsucceeds across all packages