Repository navigation
Conversation
…tup wait Under startupPolicy wait-for-setup the agent cannot become ready before the setup hook exits, so a readiness budget that runs out while setup is still running is the setup gate's timeout. worker-start now reports failedStage setup_wait with an error that says setup was still running after N ms and names --timeout-ms, instead of agent_readiness / timeout. Addresses stablyai#21512
…or-setup readiness timeout The setup receipt stays `running` on a terminal readiness timeout: only a wait that returns updates it, and a failed setup leaves the agent pane's shell alive, so readiness times out then too. Reporting "setup was still running" from that receipt mislabels a failed or finished setup. At the timeout, worker-start now probes the setup terminal (1 s, aborted afterwards) and reports what it saw: still running -> setup_wait with the --timeout-ms hint; exited non-zero -> setup_wait "Setup failed (exit N)" and setup.state failed; exited 0 -> agent_readiness with setup.state succeeded; no setup terminal -> unchanged. A structured worker whose own setup-completion race timed out reports setup_wait. Refs stablyai#21512
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughWorker startup now checks setup state when agent-readiness waiting times out. A running setup can be reported as a Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The change appears mergeable after normal checks. No concrete issue with setup-timeout classification remains identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to This changes how setup-related start failures are reported, not who can launch work or access resources. No new security weakness was established, but interruption and recovery behavior remains partly verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the full 3-file diff at ef8ccb7228 — the new setup-terminal probe for wait-for-setup readiness timeouts, its wiring into startLocalWorker, and the added tests.
worker-setup-gate.ts— newsetupErrorAtReadinessTimeout. Underwait-for-setupwith a running setup receipt, raceswaitForSetupTerminalCompletionagainst a 1 s timer and anAbortController, then maps the outcome to a still-running error, a persistedfailed/succeededreceipt (viapersistWorkerSetupWaitOutcome), ornullto keep the agent-readiness timeout.local-worker-start.ts— stage relabeling. Wraps the readiness wait in try/catch and, on a'timeout'rejection (or a structured gate timeout), calls the probe before rethrowing, settingfailedStage = 'setup_wait'when setup blocked the agent.workers-new-worktree.test.ts— five scenarios plus controls. still-running, exited-nonzero, exited-zero, no-setup-terminal, the structured-gate timeout, and astart-immediatelycontrol, all asserted onfailedStage/lastError/setup.state/effects.
I verified the probe's race, timer, and abort lifecycle (the late rejection after abort.abort() is handled by Promise.race's own reaction — no unhandled rejection or orphaned observer), that both terminal readiness lanes reject with the exact 'timeout' message the catch matches, and that an observed exit is persisted through the same receipt object the failure path returns so setup, effects, and failedStage agree. The --timeout-ms the message names is the real worker-start flag. The exited-zero test genuinely fails on main (setup.state stays running).
ℹ️ Remote-host worker starts still report the old stage
src/main/runtime/rpc/methods/orchestration/federation/federation.ts:220 runs its own readiness wait without the probe, so a federated (remote-host) worker start under wait-for-setup keeps failedStage: agent_readiness / lastError: 'timeout' on a readiness timeout even when setup was still running or had failed. The PR body discloses this as a follow-up; flagging it so remote parity stays tracked, since this change closes #21512 for local workers only.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
This branch has not been deployed
1 participant
ELI5
Some repos run a long setup script (install, build) before Orca lets a worker agent start. When the 60-second start budget ran out,
worker-startalways blamed the agent (agent_readiness/timeout), whatever setup was doing. Now, when the budget runs out underwait-for-setup, worker-start asks the setup terminal what setup actually did and reports that: still running (with the flag that gives it more time), failed with its exit code, or finished (then the agent really was the slow part).What Changed
startupPolicy: wait-for-setup, a readiness timeout always gavefailedStage: agent_readiness,lastError: "timeout", andsetup.state: runningin the same receipt, even after setup had exited. Nothing pointed at--timeout-ms.failedStage: setup_wait,lastError: "Setup was still running after 60000 ms (startup policy wait-for-setup), so the agent could not start. Retry with a larger --timeout-ms.",setup.state: running.failedStage: setup_wait,lastError: "Setup failed (exit N), so the agent could not start (startup policy wait-for-setup).",setup.state: failed. A longer timeout would not help here, so the message does not suggest one.failedStage: agent_readiness,lastError: "timeout"as before, but now withsetup.state: succeeded.agent_readiness/timeout.setup_waitwith the "still running" message instead ofagent_readiness.runningwhen the worktree is created and only changes when a readiness wait returns. A wait that rejects withtimeoutleaves it atrunning. A setup that fails also leaves the agent pane's shell alive (the gate prints "Setup failed; skipping agent startup." and the agent never starts), so the readiness wait times out in that case too. The newsetupErrorAtReadinessTimeoutinworker-setup-gate.tsaskswaitForSetupTerminalCompletion(the same signal the structured gate uses), raced against a 1 s probe and cancelled by anAbortSignalso no observer is left behind. An exit it sees is persisted through the existingpersistWorkerSetupWaitOutcome, so the receipt and the stored dispatch both sayfailedorsucceeded.start-immediately, a blocked agent, and a readiness wait that returnsexitedfor a failed setup.Why
Under
wait-for-setupthe agent's start is held behind the setup hook. Callers readfailedStageandlastErrorto decide what to do. A receipt that blames the agent while setup is still working makes them tear down a healthy worker. A receipt that says "still running, retry with a larger timeout" after setup has failed sends them into a longer retry that fails the same way. The probe gives each case its own answer.Alternatives considered: trusting
setup.stateat the timeout (it is stale on this path, see above); raising the default budget when a setup hook exists, or making the budget follow setup activity (points 2-3 of the issue); or reporting a non-terminal outcome instead offailed(point 1). Points 1-3 change behavior for every repo or change the worker lifecycle and retry rules, so they need a maintainer decision and are out of scope. This PR addresses point 4 (name--timeout-mswhen it would help) and labels the failed stage from what setup actually did.Linked Issue
Addresses #21512
This addresses point 4 of the issue and the stage label. The receipt still says
state: failedfor a setup that is still running, and points 1-3 stay open (see Notes).Visual Proof
N/A. No visual change: this only changes the
failedStage,lastErrorandsetup.statefields of theorchestration worker-startRPC/CLI receipt for readiness timeouts underwait-for-setup. No UI shows them.Testing
In
src/main/runtime/rpc/methods/orchestration/worker/workers-new-worktree.test.ts:reports a readiness timeout while wait-for-setup is still running as a setup wait: setup terminal still running at the timeout givessetup_wait, the "Setup was still running after 1000 ms" text and--timeout-ms, and the setup terminal is asked. Fails onmain.reports a wait-for-setup readiness timeout after setup failed as a setup failure: setup exited 3 givessetup_wait, "Setup failed (exit 3)",setup.state: failed, and no "still running". Fails onmain.keeps a wait-for-setup readiness timeout after setup succeeded at agent readiness: setup exited 0 givesagent_readiness/timeoutwithsetup.state: succeeded. Fails onmain(setup.statestaysrunning).reports a structured worker whose setup gate timed out as a setup wait: a structured worker whose setup-completion race times out givessetup_wait. Fails onmain.keeps a wait-for-setup readiness timeout with no running setup at agent readiness: a skipped setup keepsagent_readinessand the setup terminal is not asked. Passes before and after.keeps a start-immediately readiness timeout at agent readiness: control. Passes before and after.Each guard in the new code was removed once on purpose. Every removal made at least one of these tests fail.
Ran on Linux: the 119 orchestration and worker-start test files (two mail test files timed out under host load and passed 48/48 when rerun alone),
pnpm lint,tsc --noEmitfortsconfig.tc.cli.json, andtsc --noEmitwith thetsconfig.node.jsonsettings over the three changed files. The fullpnpm test,tscfor the wholetsconfig.node.jsonandtsconfig.tc.web.json, andpnpm buildwere not run locally (memory limits on the test machine). Not tested on macOS or Windows. The change is platform-neutral TypeScript.AI Disclosure
Claude Opus 5.5 (claude-opus-5-5), Anthropic. An automation session wrote the change, tests and this description.
Review
Reviewed before submission by an automated reviewer against the change and its tests.
waitForSetupTerminalCompletion, which reads the completion marker that both the POSIX (bash) and the Windows (powershell.exe) observed setup commands print. No paths, shells, shortcuts or OS calls are added.local-worker-start.ts,worker-setup-gate.ts). Federated (remote-host) worker starts infederation.tskeep their current behavior; that can be a follow-up. No RPC param or wire shape changes:failedStagealready usessetup_waitfor a failed setup.wait-for-setuppolicy.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
state: failedwhile setup is still running, and points 1-3 of the issue (a non-terminal outcome, an activity-based budget, a policy-derived default) are left for a maintainer decision. Maintainers may prefer to keep worker-start reports state:failed / agent_readiness while setup.state is still 'running' under wait-for-setup #21512 open for those.agent_readiness/timeoutstays.local-worker-start.ts. Start orchestration workers with their brief on the launch command line (step 4 of 7) #24081 rewrites this readiness block (launch-brief workers wait on a real setup-completion race), so whichever lands second needs a rebase, and the "still running / failed / succeeded" split would move into its setup gate. fix: failed worktree setup left the gated agent terminal silently waiting for two hours #16891 changes the setup gates so a failed setup no longer leaves the agent waiting; with it, more failed setups would reach the existingexitedpath, and this PR's probe still covers the rest. fix(orchestration): cancel timed out setup observers #22259 cancels timed-out observers in the structured gate; the probe here already aborts its own observer.failedStage === 'agent_readiness'for a timeout while setup was running, or after setup failed, now seesetup_wait, the stage that already means "the setup gate did not let the agent start".Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred).pnpm lintand the CLI typecheck config pass locally. The node and web typecheck configs, the full test suite and the build were not run locally; CI covers them.