Repository navigation
Remove settled-worker automatic resume and hibernation fences - #19544
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change removes the legacy settled-worker automatic-resume fence from runtime recovery, IPC, renderer state, and workspace-session records. Settled workers with replacement PTYs now retain their original terminal resources when identity is unproven. Normal resume and attach paths handle settled sessions. Recovery result types and tests no longer expose blocked-pane data. New tests cover resource retention, settled-worker wake behavior, workspace-session parsing, stable-pane adoption, and restart persistence. Merge Risk: 🔵 Low · up to This change removes settled-worker resume fences and retains replacement terminal resources to preserve worker tabs. Two low-impact follow-ups remain around restart-test portability and settings-search translation lookup, but no material runtime or data-integrity risk is evidenced. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides a detailed explanation of the change, rationale, scope, compatibility behavior, and verification. However, it does not follow the required template structure and omits the required Visual Proof section, explicit Testing checklist, Agent skill upstream boundary checkbox, Notes, Checklist, and AI Disclosure or an explicit reason for omission. The linked issue references also do not use the required Fixes # format. Resolution Update the description to include the required template sections. Add Visual Proof with screenshots/videos or write N/A with a reason. Add the Testing checklist and document platform coverage. Add the Agent skill upstream boundary checkbox, Notes, Checklist, and applicable AI Disclosure. Use an explicit linked issue entry such as Fixes
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 36466476-4776-44bc-9ae4-ff629b003066
📒 Files selected for processing (71)
config/ts-nocheck-baseline.txtsrc/main/ipc/pty/pane/stable-pane-relay-absence-respawn.test.tssrc/main/runtime/orca-runtime-automation-operations.tssrc/main/runtime/orca-runtime-has-exact-persisted-terminal-surface-identity.tssrc/main/runtime/orca-runtime-preserved-branch-cleanup.tssrc/main/runtime/orca-runtime-register-pty.tssrc/main/runtime/orca-runtime-subscribe-to-terminal-resize.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-02.spec.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-03.spec.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-04.spec.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-05.spec.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-06.spec.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-terminal-recovery.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.tssrc/main/runtime/orchestration/orchestration-legacy-worker-terminal-recovery.test.tssrc/main/runtime/orchestration/orchestration-legacy-worker-terminal-recovery.tssrc/main/runtime/orchestration/orchestration-settled-worker-resume-fence-db.test.tssrc/main/runtime/rpc/methods/orchestration.tssrc/main/runtime/rpc/methods/orchestration/messaging/send-point-to-point.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-release.test.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-release.tssrc/main/runtime/rpc/methods/settled-worker-resume-fence-sweep.tssrc/main/runtime/runtime-legacy-worker-terminal-recovery-controller.tssrc/main/runtime/runtime-legacy-worker-terminal-recovery-persistence.tssrc/main/runtime/runtime-legacy-worker-terminal-recovery-runner.tssrc/main/runtime/runtime-legacy-worker-terminal-recovery-types.tssrc/main/runtime/runtime-legacy-worker-terminal-resume-fence.test.tssrc/main/runtime/runtime-notifier-contract.tssrc/main/runtime/settled-worker-process-replacement.test.tssrc/main/startup/main-process-runtime-service.tssrc/main/window/runtime-window-lifecycle.tssrc/preload/api/agent-status-api.tssrc/preload/api/agent-status-bridge.tssrc/renderer/src/components/terminal-pane/pty-connection-agent-session-resume.test.tssrc/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.tssrc/renderer/src/components/terminal-pane/pty-connection/deferred-session-attach.tssrc/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-choice.tssrc/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.tssrc/renderer/src/components/terminal-pane/pty-connection/retained-legacy-pty-attach.tssrc/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.tssrc/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.tssrc/renderer/src/components/terminal-pane/sleeping-record-park-exemption.test.tssrc/renderer/src/components/terminal-pane/sleeping-record-park-exemption.tssrc/renderer/src/components/terminal-pane/terminal-cold-park-subscription-narrowing.react185.test.tsxsrc/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.test.tssrc/renderer/src/hooks/ipc-events/agent-status-listeners.tssrc/renderer/src/hooks/useIpcEvents-agent-status-ssh-authority.test.tssrc/renderer/src/hooks/useIpcEvents-lifecycle.test.tssrc/renderer/src/lib/agent-hibernation-pane-eligibility.tssrc/renderer/src/lib/agent-hibernation-planner.test.tssrc/renderer/src/lib/live-resume-anchor-record.tssrc/renderer/src/lib/resume-sleeping-agent-session-legacy-worker.test.tssrc/renderer/src/lib/resume-sleeping-agent-session.tssrc/renderer/src/lib/settled-worker-wake-policy.test.tssrc/renderer/src/store/slices/agent-pane-authority.test.tssrc/renderer/src/store/slices/agent-status-manual-sleep-capture.test.tssrc/renderer/src/store/slices/agent-status-open-tab-resume-fence.test.tssrc/renderer/src/store/slices/agent-status-provider-session-actions.tssrc/renderer/src/store/slices/agent-status-provider-session.test.tssrc/renderer/src/store/slices/agent-status-recovery-actions.tssrc/renderer/src/store/slices/agent-status-recovery-collection.tssrc/renderer/src/store/slices/agent-status-sleeping-records.tssrc/renderer/src/store/slices/agent-status-slice-contract.tssrc/renderer/src/store/slices/agent-status.tssrc/renderer/src/store/terminals/terminal-pane-hibernation.tssrc/renderer/src/web/preload-api/web-agent-status-api.tssrc/shared/agent-session-resume.tssrc/shared/workspace-session-schema.sleeping-agent.test.tssrc/shared/workspace-session-sleeping-agents.tstests/e2e/helpers/completed-worker-retirement-fixture.tstests/e2e/settled-worker-tab-survives-restart.spec.ts
💤 Files with no reviewable changes (37)
- src/preload/api/agent-status-api.ts
- src/main/window/runtime-window-lifecycle.ts
- src/renderer/src/store/slices/agent-status-recovery-collection.ts
- src/shared/workspace-session-sleeping-agents.ts
- src/renderer/src/hooks/ipc-events/agent-status-listeners.ts
- src/shared/agent-session-resume.ts
- src/main/startup/main-process-runtime-service.ts
- src/renderer/src/store/slices/agent-status-open-tab-resume-fence.test.ts
- src/renderer/src/lib/live-resume-anchor-record.ts
- src/renderer/src/web/preload-api/web-agent-status-api.ts
- src/main/runtime/runtime-legacy-worker-terminal-recovery-runner.ts
- src/main/runtime/orca-runtime-subscribe-to-terminal-resize.ts
- src/main/runtime/runtime-legacy-worker-terminal-recovery-controller.ts
- src/renderer/src/store/slices/agent-status-provider-session-actions.ts
- src/renderer/src/lib/resume-sleeping-agent-session.ts
- src/renderer/src/components/terminal-pane/sleeping-record-park-exemption.test.ts
- src/main/runtime/runtime-legacy-worker-terminal-resume-fence.test.ts
- src/preload/api/agent-status-bridge.ts
- src/main/runtime/rpc/methods/orchestration/messaging/send-point-to-point.ts
- src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts
- src/renderer/src/store/slices/agent-status-provider-session.test.ts
- src/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.ts
- src/renderer/src/store/slices/agent-status.ts
- src/main/runtime/orchestration/orchestration-settled-worker-resume-fence-db.test.ts
- src/renderer/src/store/slices/agent-status-manual-sleep-capture.test.ts
- src/main/runtime/runtime-notifier-contract.ts
- src/main/runtime/runtime-legacy-worker-terminal-recovery-types.ts
- src/renderer/src/lib/agent-hibernation-planner.test.ts
- src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts
- src/renderer/src/components/terminal-pane/pty-connection/retained-legacy-pty-attach.ts
- src/renderer/src/lib/resume-sleeping-agent-session-legacy-worker.test.ts
- src/main/runtime/rpc/methods/settled-worker-resume-fence-sweep.ts
- src/renderer/src/store/slices/agent-status-slice-contract.ts
- src/renderer/src/components/terminal-pane/terminal-cold-park-subscription-narrowing.react185.test.tsx
- src/renderer/src/store/slices/agent-status-recovery-actions.ts
- src/main/runtime/rpc/methods/orchestration/worker/worker-release.ts
- src/renderer/src/store/terminals/terminal-pane-hibernation.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The fence removal is clean and well-tested, but one pre-existing test still asserts the withdrawn behavior and now fails in the main vitest suite — it needs updating before merge.
Reviewed changes
- Withdrawn policy — removes
automaticResumeBlockedByfromSleepingAgentSessionRecord, the shared session schema, and every reader/writer across main, preload, and renderer. - Fence infrastructure deleted — the
sweepingSettledWorkerResumeFencesRPC wrapper, thesetLegacyWorkerTerminalResumeFencenotifier, and the renderer-only retained-worker attach path. - Recovery planning simplified —
planLegacyWorkerTerminalRecoverydropsblockedPanes; settled dispatches no longer surface in recovery rows or candidates. - One addition —
retainReplacedWorkerTerminalResourcesmarks a settled dispatch's terminal resourceretainedwhen its pane re-registers under a new process incarnation. - Tests — regression e2e rewritten to the new policy; fence-only unit tests removed; new process-replacement and wake-policy tests added.
⚠️ Stale test still asserts the removed resume fence and fails
tests/e2e/completed-worker-retirement-resume.unit.test.ts is not among this PR's changed files, but it still sets automaticResumeBlockedBy: 'legacy-orchestration-worker' and asserts that resumeSleepingAgentSessionsForWorktree leaves the record untouched. With the fence skip gone, the state: 'done' record now falls through to the passive-completed branch and is cleared, so the assertion at line 463 fails. I reproduced it locally; this file is part of the main suite (tests/e2e/**/*.unit.test.ts in config/vitest.config.ts), so it breaks pnpm test.
Technical details
# Stale legacy-worker resume-fence test breaks after fence removal
## Affected sites
- `src/renderer/src/lib/resume-sleeping-agent-session.ts:213` — the `automaticResumeBlockedBy === 'legacy-orchestration-worker'` early `continue` was removed (intended).
- `tests/e2e/completed-worker-retirement-resume.unit.test.ts:437-463` — "Case 4: legacy rollback preserves a fenced record" sets the removed field and expects the record to survive the wake sweep untouched.
## Reproduction
`pnpm exec vitest run tests/e2e/completed-worker-retirement-resume.unit.test.ts --config config/vitest.config.ts`
→ `AssertionError: expected undefined to be 'legacy-orchestration-worker'` at line 463.
## Required outcome
- Update or delete this test to match the new policy: a settled worker's sleeping record is ordinary
agent state and no longer carries a resume fence. The record is now cleared by the
`isPassiveCompletedHibernationEvidence` branch when its pane is no longer owned.
- The adjacent loose type in `tests/e2e/orchestration-legacy-worker-restart-recovery.spec.ts:205`
(`automaticResumeBlockedBy?: string`) is harmless but should be dropped for consistency.
## Note
`tests/` is not typechecked (`config/tsconfig.e2e.json` is explicitly not enforced), so the field
access at line 462 does not surface as a compile error and this only manifests at runtime.DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
Case 4 seeded and asserted automaticResumeBlockedBy, which this branch deletes. A rolled-back settled worker is now an ordinary done record that wake clears as passive evidence, same as any finished agent pane.
# Conflicts: # src/main/runtime/orchestration/orchestration-settled-worker-resume-fence-db.test.ts # src/main/runtime/runtime-legacy-worker-terminal-resume-fence.test.ts
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Retirement-rollback test rewritten to the no-fence policy —
tests/e2e/completed-worker-retirement-resume.unit.test.tsCase 4 no longer fabricatesautomaticResumeBlockedBy; it now asserts the settled worker's sleeping record survives rollback as ordinarystate: 'done', lacks the fence field, and is cleared by the wake sweep (resumeSleepingAgentSessionsForWorktreereturns0). - Merged
main— two testcreateTaskcalls gained the now-requiredrunId: 'run_legacy_local', matching thecreateTask-requires-runIdchange that landed onmain.
This delta resolves the stale-test failure flagged in the prior review; the fixed test and the adjacent orchestration recovery test both pass locally.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 45303c62-b839-4b79-a793-1d15495e9c2b
📒 Files selected for processing (3)
src/main/runtime/orca-runtime-subscribe-to-terminal-resize.tssrc/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery-part-04.spec.tssrc/renderer/src/i18n/en-runtime-required.json
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| "cc8c5ca224": "Windows default", | ||
| "d78fc4fdef": "Loading distributions" | ||
| "d78fc4fdef": "Loading distributions", | ||
| "minimumContrast": { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the translation namespace with the search consumer.
Line 1507 adds auto.components.settings.TerminalPane.minimumContrast. However, src/renderer/src/components/settings/terminal-typography-search.ts reads auto.components.settings.terminal.search.minimumContrast.*. The search catalog will not resolve these new entries and will use its inline fallback strings. Add the corresponding settings.terminal.search.minimumContrast entries or update the consumer to use the TerminalPane namespace.
Main's #19544 left orphan TerminalPane.minimumContrast entries in en-runtime-required.json that en.json no longer carries; the localization gates fail on any merge ref that includes it. Regenerated via sync:localization-runtime-catalog on the merged tree.
…e boot wait (#19608) * fix(orchestration): own a worker terminal from creation, not after the boot wait A worker pane is visible on desktop and phone the moment it is created, but the worker_terminal_resources row saying orchestration owns it was written only after the agent TUI went idle (up to 60s). A keystroke into the booting pane found no owned row, markWorkerTerminalUserOwned returned 0, and the takeover was dropped - so a later worker-release closed the pane under the user. Record custody on the branches that create a terminal, right after creation and before the tui-idle wait. The Dispatch capability still waits for the agent to come up. An explicit --terminal reuse is untouched: it transfers at authority. With the row present from creation, the failed-start adoption is dead. What a failed start still needs is the Dispatch-context pane identity release re-proves through, which is now copied from the custody row. * chore(i18n): drop the orphan minimumContrast entries #19544 re-added to the runtime catalog
Replaces #19427, #19344 and #19358. Fixes the Slack report: a finished orchestration worker's tab was torn down on click after an app restart.
Product decision
A worker that has reported done is an ordinary agent terminal from then on. Waking a sleeping workspace, auto-resume after quit, and hibernation treat it exactly like any other agent pane. The special "settled worker must not resume" rule from #11271 is withdrawn, along with everything built to enforce it.
Why the Slack bug happened
The fence made a settled worker's pane take a renderer-only reattach path that skipped main. Main's daemon adapter never learned the session existed, so
pty:hasPtyanswered false on click and the tab was torn down while the daemon still ran the process.What is deleted
automaticResumeBlockedByon sleeping records and every reader and writer.Source: 44 files, +94 / -522 against the branch base. Tests: 26 files, +880 / -1057.
What is kept
Main attaches to an existing PTY owner before spawning for every pane (
spawnForStablePanetoattachStablePaneOwner). That invariant is what prevents duplicatecodex resume, and it is unchanged. Orchestration already tolerates a resumed process: release refuses when the incarnation differs and the row stays retained.One addition
When a settled worker's pane comes back under a new process incarnation, main marks that Dispatch's terminal resource retained at PTY registration. Without it,
worker-liststill called the resumed terminal reclaimable. One bounded SQL update, Task and Dispatch rows untouched, controls for active workers, missing incarnation, other host, and unchanged live owner.Mixed versions
An old client can still submit
automaticResumeBlockedBy; the shared parser strips it. No stream opcode or daemon capability change.Verification
settled-worker-tab-survives-restart.spec.tsrewritten to the new policy. Daemon live: runtime rediscovers the PTY, tab survives click, zero new spawns. Daemon exited: at most onecodex resume, tab survives,worker-listrow retained/succeeded with Task and Dispatch unchanged.hasPtyfalse, tab unavailable on click) in both run orders; this branch passes 4 of 4 in both orders.tc:node,tc:web, changed-code quality (0 findings), reliability gates, max-lines and ts-nocheck ratchets,git diff --check, CLI and e2e builds.