Repository navigation
Conversation
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 36.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: automatic release of settled workers after a two-minute grace period. Description check The description is complete and directly related to the change. It includes the ELI5 summary, user-facing behavior, implementation details, rationale, maintainer issue exception, visual proof, testing… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is complete and directly related to the change. It includes the ELI5 summary, user-facing behavior, implementation details, rationale, maintainer issue exception, visual proof, testing evidence, compatibility notes, limitations, and follow-ups. Minor unchecked checklist items do not prevent approval because the description provides substantial testing and review information.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 36.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 49 functions across 66 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
cc48d6d2-cdfd-43ee-a4ac-31943ce8d665
-
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
-
src/cli/handlers/orchestration/message-send-handler.tssrc/cli/handlers/orchestration/worker-observation-handlers.tssrc/cli/handlers/orchestration/worker-terminal-handlers.tssrc/cli/specs/orchestration-worker-specs.tssrc/main/runtime/orchestration/db/attach-orchestration-db-methods.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.tssrc/main/runtime/orchestration/db/federation/federation-relay-ack.tssrc/main/runtime/orchestration/db/federation/remote-dispatch-attachment-release.tssrc/main/runtime/orchestration/db/orchestration-db-methods.tssrc/main/runtime/orchestration/db/orchestration-db.tssrc/main/runtime/orchestration/db/schema/create-core-tables-sql.tssrc/main/runtime/orchestration/db/schema/migrate-v44.tssrc/main/runtime/orchestration/db/schema/migrate.tssrc/main/runtime/orchestration/db/utc-timestamp.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-archive.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-auto-release.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-user-takeover.tssrc/main/runtime/orchestration/federation-sync.tssrc/main/runtime/orchestration/lifecycle-reconciliation.tssrc/main/runtime/orchestration/orchestration-schema-version-skew.tssrc/main/runtime/orchestration/worker-terminal-auto-release-scheduler.tssrc/main/runtime/orchestration/worker-terminal-ownership.tssrc/main/runtime/rpc/methods/orchestration/federation/federated-worker-release-host.tssrc/main/runtime/rpc/methods/orchestration/federation/federated-worker-release.tssrc/main/runtime/rpc/methods/orchestration/federation/federated-worker-show.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-auto-release-scenarios.test-support.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-control.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-output.test.tssrc/main/runtime/rpc/methods/orchestration/federation/federation-relay.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-auto-release.test.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-control.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-list-projection.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-observation.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-release.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-terminal-resource-presentation.tssrc/main/runtime/workspaces/persisted-surfaces/automation-operations.tssrc/renderer/src/components/tab-bar/KeepWorkerMenuItem.tsxsrc/renderer/src/components/tab-bar/SortableTab.rename-shortcut.test.tsxsrc/renderer/src/components/tab-bar/SortableTabContextMenu.test.tsxsrc/renderer/src/components/tab-bar/SortableTabContextMenu.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/shared/orchestration-fleet-projection.tssrc/shared/orchestration-fleet-worker-projection.tssrc/shared/orchestration-rpc-contract.tssrc/shared/orchestration-runtime-capabilities.tssrc/shared/protocol-version.tssrc/shared/rpc-contract/orchestration-federation-relay-params.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
-
| const lifecycle = 'lifecycle' in value ? value.lifecycle : undefined | ||
| const autoRelease = | ||
| lifecycle && 'autoReleaseAt' in lifecycle && lifecycle.autoReleaseAt | ||
| ? ` (terminal auto-release at ${lifecycle.autoReleaseAt})` | ||
| : '' | ||
| return withWarnings(`Sent ${value.message.id}${autoRelease}`) |
There was a problem hiding this comment.
Include the deadline in federated worker receipts.
A remote worker_done returns the relay variant, so this formatter never prints its deadline. The federated settlement also omits the deadline from the receipt. Add the host deadline to the relay receipt and display it in both text and JSON output. (raw.githubusercontent.com)
| ? ` next=${projection.nextAction.argv.join(' ') || 'none'}` | ||
| : '' | ||
| return `${worker.dispatchId} task=${worker.taskId} [${worker.workerState}${details} terminal=${worker.terminalState ?? 'none'}${next}` | ||
| const autoRelease = worker.resource?.autoReleaseAt |
There was a problem hiding this comment.
Expose remote deadlines in worker-list.
For a federated worker, the deadline is stored on the execution host. worker-list builds resource from the home database, while the remote fleet snapshot contains only liveness. Consequently, even --include-remote omits the deadline, although the terminal will close. Include the host deadline in the fleet snapshot and return it in the worker-list row. (raw.githubusercontent.com)
| const h = createOrchestrationWorkerReleaseHarness() | ||
|
|
||
| afterEach(() => { | ||
| getWorkerTerminalAutoReleaseScheduler(h.runtime).clear() |
There was a problem hiding this comment.
afterEach throws when a test does not call h.setup().
The test 'never arms a chat worker…' (Line 192) opens its own OrchestrationDb and never calls h.setup(). afterEach still calls getWorkerTerminalAutoReleaseScheduler(h.runtime). The harness variable runtime is undefined until setup() runs. In a full run it is the stale runtime from an earlier test, which hides the problem.
Run this test alone, for example with -t or .only. schedulerByRuntime.set(undefined, …) then throws a TypeError because WeakMap keys must be objects. The test fails in the hook.
Track whether setup ran, and clear the scheduler only in that case.
🐛 Proposed fix
-
+ let didSetup = false + afterEach(() => { - getWorkerTerminalAutoReleaseScheduler(h.runtime).clear() + if (didSetup) { + getWorkerTerminalAutoReleaseScheduler(h.runtime).clear() + } + didSetup = false h.cleanup() vi.useRealTimers() }) function setupWithFakeClock(): void { vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) vi.setSystemTime(new Date('2026-10-27T12:00:00Z')) h.setup() + didSetup = true }
…s converge with automatic release - Record worker takeover at the owning runtime when it accepts UI input, before the bytes reach the PTY. - Run remote attachment releases through the same per-resource single-flight as local ones. - Apply the confirmed-release home projection when host observation reports a host-side release, and carry the host deadline in fleet snapshots. - Prove restart recovery with a file-backed database reopened by a fresh runtime.
… counting methods
…remote releases, converge the home - An automatic close checks the terminal's last driving-input time right before closing and keeps a worker someone typed into after worker_done; typing itself does no orchestration database work. - Remote releases share the per-resource single-flight with local release, so a home worker-release joins the host timer's close. - Host fleet snapshots and federationShow report released and the pending deadline; the home applies the existing release projection inside its observation fence. - Real restart tests: a file-backed database reopened by a fresh runtime releases a missed deadline once and finishes interrupted requests.
…, publish archives only through the conditional commit - Each remote release caller records its own request before joining a close under way, and the close decides on the current row, so an explicit release that overlaps the host timer clears the automatic deadline. - Remote capture publishes its archive through commitWorkerTerminalArchiveForRelease, so a retain during capture leaves no stale archive for worker-read. - Remote worker-show projects the fenced host deadline and its next action. - The input guard lives in its own module instead of the scheduler.
…r worker_done - The guide's Completion section, the reuse recipe and the recovery reference now describe the automatic release and its autoReleaseAt deadline: reuse or worker-retain before it; explicit worker-release still works and is required for chat workers and workers without a deadline. - The worker preamble says the terminal stays available until Orca releases it, about 2 minutes after worker_done unless retained. - Regenerated the bundled guides.
There was a problem hiding this comment.
Actionable comments posted: 2
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
71d13ebe-d0c9-4a24-88bd-9fe2824b05ba
-
src/main/runtime/orchestration/__snapshots__/preamble.test.ts.snapis excluded by!**/*.snap
-
skill-guides/orchestration.mdskill-guides/orchestration/references/coordinator-loop.mdskill-guides/orchestration/references/recovery-and-cleanup.mdsrc/cli/bundled-skill-guides.tssrc/main/runtime/orchestration/preamble.test.tssrc/main/runtime/orchestration/preamble.tssrc/main/runtime/rpc/methods/orchestration-worker-start-mode-selection.test.tssrc/main/runtime/rpc/methods/orchestration/runs/tasks-dispatch.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
-
| it with `task-update --status completed`. Enumerate the terminals still owing a | ||
| decision with `worker-list --run <run_id> --terminal-state reclaimable --json`, | ||
| and do not end the coordinator turn until it returns none. | ||
| and do not end the coordinator turn until every row left has an `autoReleaseAt`. |
There was a problem hiding this comment.
Include federated workers in completion accounting.
A federated worker can have no home terminal resource. worker-list --terminal-state reclaimable then excludes it, even when its host has no automatic-release deadline. The coordinator can end the turn without releasing that worker. Enumerate remote workers separately, check their host deadlines, and release those without one. Regenerate the bundled guide. (raw.githubusercontent.com)
| always needs `worker-release`. Do not retain it only to read output; archived | ||
| output remains available through `worker-read`. |
There was a problem hiding this comment.
Do not promise complete output after release.
The archive preserves a bounded snapshot: transcript capture is limited to 50 messages, and terminal fallback is limited to a tail. Earlier output cannot be recovered through the archived worker-read path. Advise coordinators to read needed output before release or retain the terminal when later inspection matters. Regenerate the bundled guide. (raw.githubusercontent.com)
|
Superseded by #36137, which combines the orchestration v3 changes into one PR. |
ELI5
When a worker agent reports that it finished (or failed), Orca now closes its terminal by itself about 2 minutes later. Its output is saved first, so
worker-readstill works. If you or the coordinator want the worker kept, any of these keeps it: reusing it for a follow-up, runningworker-retain, typing into it, or picking Keep Worker Open in the tab menu.What Changed
Before: a finished worker's terminal stayed open until the coordinator ran
worker-release.worker-listshowed it asreclaimableand its next action wasworker-release. Coordinators that forgot this left idle agents open.After:
An accepted
worker_done(succeeded or failed) recordsauto_release_at = now + 2 min, once, in the same settlement transaction. A repeatedworker_donenever moves it.One runtime timer is set for the earliest deadline. It is reset whenever a settlement records a deadline, and again when the database opens, so a restart picks up deadlines it missed. When it fires, it claims the due rows and hands them to the existing release recovery path (
reconcileRequestedWorkerTerminalReleases→completeWorkerTerminalRelease/releaseRemoteAttachment). This PR adds no new archive or close code.The claim (
claimDueWorkerTerminalAutoReleases) is a compare-and-set. It only succeeds if all of these still hold:auto_release_dispatch_id = owner_dispatch_id);ownedandnot_requested;succeededorfailed.Explicit release overrides a retain on purpose, so the timer never calls it.
What wins over the deadline:
worker-start --terminal: the transfer clears the deadline and changes the owner.worker-retain, and the new Keep Worker Open tab-menu item: both record the same durableuser_requestedretain.worker_done, it does not close: the row becomesuser_ownedand retained, using the existing takeover write. Orchestration mail does not count as typing; aterminal sendfrom the CLI does, since someone drove the terminal. Because this is checked only at close time, typing does no orchestration database work, so the fix(orchestration): fence worker release on mobile keystrokes #19337 invariant still holds. An explicitworker-releasestill closes the terminal.Retain and takeover also win after the claim, while output is being captured: the archive commit refuses a row that is no longer
requested.Known gap, accepted: the last-input time is in memory. If the takeover report is lost and Orca restarts within the grace period while the terminal survives, the new runtime has no typing history and the automatic release closes it.
Never released automatically:
start_unknown,stopped, andabandonedworkers;The deadline appears in:
worker-show:autoReleaseAt, andAuto-release:in text output;worker-list:projection.autoReleaseAtfor local and remote workers, plusresource.autoReleaseAtfor local ones, andauto-release=in text output;worker_donesend receipt:lifecycle.autoReleaseAt.While a deadline is pending, the fleet next action is
noneinstead ofworker-release.Remote (federation):
federationAckcarries the home's acceptance of theworker_doneand the newautoRelease: trueflag. A report that is only queued records nothing.worker-retainon the home now goes to the execution host through the neworchestration.federationRetain. Before, it only ran on the home, which owns no terminal.worker-read.WorkerTerminalReleaseSingleFlight, generalized fromcompleteWorkerTerminalRelease). A homeworker-releasethat overlaps the host timer joins the timer's close instead of closing a second time.federationShowreportreleased, and the home applies the existingapplyConfirmedFederatedReleaseHomeProjectioninside its observation fence. Fleet snapshots also carry the host'sautoReleaseAt, so the home'sworker-list --include-remoteshows the pending release and a next action ofnone.Mixed versions: new capability
orchestration.worker-auto-release.v1.autoRelease, so a new host keeps manual release.Schema v44: two nullable columns,
auto_release_atandauto_release_dispatch_id, plus a partial index. The deadline is tied to the Dispatch it was set for, so if an older build transfers a terminal, the stale deadline is ignored.Decisions where the design left room
failed-worker-start-teardown.ts, fix(orchestration): stop retried failed worker starts from piling up live agents #27795). That reason does not apply to a settled chat. A different one does: no draft-edit activity signal exists on desktop or mobile, and closing the chat would throw away a draft the user is writing. So chats keep manual release until draft edits count as user activity.orchestration.workerTerminalUserKeep), shaped like the takeover report. Renderers cannot send the orchestration contract version thatworkerRetainrequires, and naming the pane routes the call to the runtime that owns the terminal. It reuses the same retain write.Why
The release path already archives output and closes only the worker's own terminal, safely. What was missing was a trigger that does not depend on the coordinator remembering to run it. Running the timer through the existing recovery path, behind a fenced claim, keeps one close path. It also means a restart, an unreachable host, or a missing terminal behave exactly like a release that was already requested.
Linked Issue
N/A (maintainer; orchestration simplification idea 5).
Visual Proof
Hidden dev build:
ORCA_BACKGROUND_LAUNCH=1, profile under/tmp/oar, isolated HOME andCLAUDE_CONFIG_DIR, a fake Codex. Three workers reportworker_done:plainis left alone,retainedgetsworker-retain, andtypedgets real keystrokes in its xterm.Before (main): I did not rebuild main for a side-by-side run. On main, nothing records a deadline, so the same flow leaves all three terminals
reclaimableand open until someone runsworker-release.Testing
worker-auto-release.test.ts(on the release harness, fake clock):worker-readafter;worker_donedoes not extend the deadline;worker_donedoes not block the release; an explicitworker-releasestill closes after typing;requestedorreleasingfinishes once after the restart;federation-output.test.ts(new scenarios infederation-auto-release-scenarios.test-support.ts):worker_donekeeps the terminal;worker-releasethat joins the host timer during capture, or after the archive commit, still closes once, even with typing recorded;worker-releasethat overlaps the host timer, with the close held pending, closes once;worker-list --include-remoteand throughworker-show;pnpm check:changed --maxWorkers=2on 99e764c: 27 of 27 checks passed.Review
Agent skill upstream boundary
Notes
setTimeout, unref'd. Deadlines are UTC SQLite timestamps written fromDate.now().worker-showorworker-list --include-remote). A plainworker-list, which does not contact hosts, shows the home's last projection.Follow-ups (out of scope here)
next=orchestration worker-releaseinworker-list. That matches main's existing projection for a retained worker.Guide update
The orchestration guide and its bundled copy now say: terminal workers are released about 2 minutes after
worker_done(autoReleaseAt); runworker-retainfirst to reuse or inspect one later; explicitworker-releasestill works and is required for chat workers and any worker withoutautoReleaseAt. Changed: the Completion section ofskill-guides/orchestration.md, the reuse recipe inreferences/coordinator-loop.md, and the action table and retain/release section inreferences/recovery-and-cleanup.md. The worker preamble now says the terminal stays available until Orca releases it, about 2 minutes afterworker_doneunless the coordinator retains it.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)