feat(orchestration): add worker-start --spec to create and dispatch in one call - #13540
feat(orchestration): add worker-start --spec to create and dispatch in one call#13540S-jooyoung wants to merge 3 commits into
Conversation
…n one call Exactly one of --task or --spec is required; --task-title labels the created Task. The Task is created runtime-side inside the same workerStart call, so a failed dispatch surfaces the created taskId in its receipt instead of leaving an untracked orphan ready task. The --task path is unchanged, and --spec is capability-gated (orchestration.worker-start-spec.v1) so older runtimes fail with a clear error instead of stripping the field. Closes stablyai#13360 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e3d57cdd-6812-4b0c-b526-1fa1d5047ad6
📒 Files selected for processing (10)
skill-guides/orchestration.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/orchestration-worker-cli.test.tssrc/cli/handlers/orchestration.tssrc/cli/specs/orchestration-worker-specs.tssrc/main/runtime/rpc/methods/orchestration-worker-start-schema.tssrc/main/runtime/rpc/methods/orchestration-worker-start-spec.test.tssrc/main/runtime/rpc/methods/orchestration-worker-start-validation.tssrc/main/runtime/rpc/methods/orchestration-workers.tssrc/shared/protocol-version.ts
…light A preflight rejection (launch validation, worktree/terminal checks, federation capability probe) used to leave an orphan ready Task with no receipt, and because worker-start's mutation receipt is only recorded at dispatch acceptance, a retried preflight failure re-created the Task on every attempt. --task validation stays first, so existing error ordering is unchanged; the Task is now created immediately before createStartingWorkerDispatch in both the local and federated paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/runtime/rpc/methods/orchestration-worker-start-validation.ts (1)
47-59: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftInline task creation still needs to be atomic with dispatch acceptance.
createStartingWorkerDispatch(...)wraps dispatch and mutation-receipt acceptance transactionally, but it only reads an existingtaskIdand rolls back only the dispatch-side inserts; it does not makecreateTask()part of that same retry-safe boundary. SincecreateWorkerStartTaskFromSpec()creates the task outside this transaction, a crash or duplicate unknown-result retry can still leave a task without a durable pending acceptance.Move inline task creation into an atomic operation or add a durable acceptance record with rollback semantics for pre-dispatch task creation at the inline create path. Add a test that simulates failure after task creation but before dispatch acceptance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bd0db852-4658-44c8-8a02-5ec30ea11059
📒 Files selected for processing (5)
src/main/runtime/rpc/methods/orchestration-federated-worker-start.tssrc/main/runtime/rpc/methods/orchestration-migration-behavior.test.tssrc/main/runtime/rpc/methods/orchestration-worker-start-spec.test.tssrc/main/runtime/rpc/methods/orchestration-worker-start-validation.tssrc/main/runtime/rpc/methods/orchestration-workers.ts
createStartingWorkerDispatch now accepts createTask and creates the inline task inside its BEGIN IMMEDIATE transaction, so a rejected or crashed acceptance rolls the task back together with the dispatch row and mutation receipt instead of leaving a receiptless orphan or a duplicate on unknown-result retry. --retry-of now requires --task: a just-created task can never satisfy retry validation, so the combination only manufactured failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai Addressed the outside-diff caution (inline task creation vs. dispatch acceptance atomicity) in
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline --decorate --no-renames c3a1a8338
git diff c3a1a8338^ c3a1a8338 -- \
src/main/runtime/rpc/methods/orchestration-worker-start* \
src/main/runtime/rpc/methods/orchestration-workers.ts \
src/cli/handlers/orchestration.ts \
src/cli/specs/orchestration-worker-specs.ts
printf '\n--- candidate symbols ---\n'
rg -n -C 4 \
'createStartingWorkerDispatch|buildWorkerStartTaskInput|createTask|retryOf|retry-of|mutation.?receipt|listTasks' \
src/main/runtime/rpc/methods src/cli/handlers/orchestration.ts src/cli/specs/orchestration-worker-specs.ts
printf '\n--- outlines ---\n'
fd -a '^orchestration-(worker-start|workers|federated-worker-start).*\.ts$' src/main/runtime/rpc/methods \
-x ast-grep outline {} --items all --view expandedLength of output: 131459 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- createStartingWorkerDispatch definition and surrounding transaction ---'
rg -n -A180 -B20 \
'createStartingWorkerDispatch\s*\(' \
src/main/runtime/orchestration/db.ts
printf '%s\n' '--- createTask and mutation receipt helpers ---'
rg -n -A100 -B15 \
'createTask\s*\(|beginMutationReceipt\s*\(|BEGIN IMMEDIATE|COMMIT|ROLLBACK' \
src/main/runtime/orchestration/db.ts
printf '%s\n' '--- federated worker dispatch acceptance path ---'
sed -n '70,145p' src/main/runtime/rpc/methods/orchestration-federated-worker-start.ts
printf '%s\n' '--- all createStartingWorkerDispatch call shapes after the change ---'
rg -n -A12 -B3 'createStartingWorkerDispatch\(\{' src --glob '*.ts'Length of output: 235051
Both local and federated paths pass The atomicity concern is addressed. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
Summary
Adds
--spec(and optional--task-title) toorca orchestration worker-start, so a supervised worker can be created and dispatched in one call instead of the two-mutationtask-create→worker-startid-shuttle.Closes #13360.
--spec <text>creates the Task runtime-side inside the sameworkerStartcall, then enters the existing dispatch flow. Exactly one of--taskor--specis required;--task-titleis only valid with--spec.createStartingWorkerDispatchcreates the--specTask inside its existingBEGIN IMMEDIATEtransaction, so a rejected preflight persists nothing, and a rejected or crashed acceptance rolls the Task back together with the dispatch row and mutation receipt — no orphan Task, and no duplicate Task on an unknown-result retry.--retry-ofrequires--task(a just-created Task can never satisfy retry validation, so the combination is rejected up front).--taskpath is unchanged (backward compatible). Tasks needing--deps/--parentkeep usingtask-create.--specalso works with--on <server>— the Task is created on the Run home, exactly as withtask-create.task-create(--spec,--task-title), keeping the CLI surface consistent.Screenshots
No visual change.
Testing
pnpm lintpnpm typecheckpnpm testpnpm buildNew tests:
src/main/runtime/rpc/methods/orchestration-worker-start-spec.test.ts— runtime:--speccreates the Task in the Run and dispatches toready; a failed dispatch (dispatch_input) still returns the createdtaskIdand the Task remains queryable; a rejected local or federated preflight persists no Task; an in-transaction acceptance failure and a mutation-receipt conflict roll the inline Task back; existing--taskpath still resolves; schema rejects--task+--spec, neither,--task-titlewithout--spec, and--retry-ofwith--spec.src/cli/handlers/orchestration-worker-cli.test.ts— CLI: forwardsspec/taskTitleto the RPC, capability-gates--specagainst old runtimes, and rejects the invalid flag combinations client-side before any RPC call.AI Review Report
Reviewed by the AI agent that authored the change, explicitly checking:
spec/taskTitleare new optional RPC fields (safe perdocs/reference/remote-wire-compatibility.md). Because an older host would strip them via zod and then reject the call as missing--task, the CLI gates--specbehind a new static capabilityorchestration.worker-start-spec.v1(same pattern asworker-launch-preferences.v1) and fails with a clear "update Orca" error instead.startFederatedWorker, so federated placement semantics are unchanged; no local-only assumptions added.status.getonly when--specis used (shared with the existing--model/--effortprobe when both apply); task creation reuses the existingdb.createTaskinsert. No hot-path work added.WorkerStartParams.superRefinetoo, so direct RPC callers get the same contract. CodeRabbit then flagged that the first revision created the Task before pre-dispatch validation, and that inline creation needed to be atomic with dispatch acceptance — the Task is now created insidecreateStartingWorkerDispatch's transaction (rolled back with the dispatch row and mutation receipt on failure), with no-orphan tests for preflight rejections, in-transaction acceptance failures, and receipt conflicts.Security Audit
spec/taskTitlego through the same zodOptionalStringvalidation and the same parameterizeddb.createTaskinsert astask-create— no new parsing, no string interpolation into SQL.getOrchestrationDispatchAuthoritypath, identical totask-create.Notes
orchestration.worker-start-spec.v1(advertised unconditionally by hosts that ship this change).--taskpath unchanged). New client + old host: clearincompatible_runtimeerror instead of a confusingMissing --task.skill-guides/orchestration.mdand the CLI usage/notes insrc/cli/specs/orchestration-worker-specs.ts.🤖 Generated with Claude Code