Repository navigation
fix(daemon): an older daemon that exited no longer blocks worktree delete - #26402
brennanb2025 wants to merge 6 commits into
Conversation
…lete After an upgrade, Orca keeps talking to the previous version's terminal daemon so its terminals survive. Nothing replaces that daemon when it exits, so once it did, every terminal inventory failed with `connect ECONNREFUSED daemon-v<old>.sock` and every worktree delete was refused until Orca restarted. Its inventory now reads as empty when the endpoint refuses and no process answers for any pid recorded for it. A timeout, a live or recycled pid, or an unreadable record still fails closed, and a daemon that can be respawned keeps its own recovery.
|
Status (head Done:
Not done / not verified:
|
The previous commit made an exited older daemon's inventory read as empty, but left its adapter in the router, so every reader still met it and a pane routed to it could not reattach. The adapter now retires itself on first contact that proves the exit (a refused or missing endpoint plus no live process for any known pid). Its holders drop it: the router and degraded provider leave it out of their adapter sets and restarts no longer carry it, and owner resolution forgets its routes without discarding a lookup already in flight. A pane routed to it now reads as not found and cold-restores. Retirement does not dispose the adapter, since that would mark its sessions cleanly ended and suppress the cold restore.
A lookup already in flight could index the answer an older daemon gave just before it retired and record a route to it, which forgetProvider had already run for, stranding the pane on a dead adapter. Indexing now skips any provider retired by the time the lookup lands, and an attach whose owner retired between lookup and spawn reads as not found. invalidateProvider now reuses forgetProvider's route cleanup.
|
Status (head What changed since the last status:
Verified:
Not verified:
|
…-rm-stale-daemon # Conflicts: # src/main/daemon/daemon-pty-router.ts
Main now uses retirement for deliberately shutting an idle daemon down, so an adapter whose daemon already exited is named for that instead.
|
Status (head
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (13)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds evidence checks for legacy daemon exits and reports confirmed exits through provider interfaces and listeners. Session inventory clears tracked sessions for confirmed exits. Router, recovery, and owner-resolution paths filter or forget exited providers, while teardown still includes exited legacy adapters. Tests cover process-liveness evidence and exit handling across inventory, routing, attach-only requests, and owner resolution. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to This change lets worktree deletion succeed when an older terminal daemon has exited, and it keeps the existing fail-closed behavior when the daemon's state is uncertain. No merge-blocking risk was found. Only macOS was exercised live. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
ELI5
When Orca updates, it keeps the previous version's terminal service running so your open terminals survive the update. If that old service later shuts down while Orca is still open, Orca kept trying to ask it "which terminals do you have?" and, getting no answer, refused to delete any workspace at all, until you restarted Orca. Now, the first time Orca sees proof that the old service is gone (its address refuses connections and its process no longer exists), Orca drops it for good, the delete goes ahead, and terminals that were running on it reconnect instead of showing "couldn't safely reconnect".
What Changed
The problem. On a machine that had upgraded across terminal-daemon versions, every workspace delete failed:
The live daemon was v41. The v40 daemon had been preserved at app start (Oct 6 10:38) because it still owned terminals, and its log shows it was stopped by a signal at 20:52 the same evening (
"event":"shutdown","reason":"SIGTERM"). On the way out it removed its own pid and token files, but the socket file stayed.Why it happened. After an upgrade, Orca's terminal router keeps an adapter for each older daemon that was alive at startup (
createLegacyDaemonAdapters). Nothing ever respawns an older daemon, and nothing retires its adapter while the app runs. Before a delete, the worktree teardown asks the router for every terminal process (DaemonPtyRouter.listProcesses), and that call deliberately fails if any adapter can't answer. That is the right rule while a daemon might still be alive. After v40 exited, its adapter could never answer again, so every delete was refused until Orca restarted. The same dead adapter also left the terminal-owner lookup and the memory-registry hydration permanently "incomplete".What is different for the user. Once an older daemon has exited:
--force.Nothing changes while an older daemon is still running, or might be.
Mechanism. An older-version daemon adapter now has a final "daemon exited" state, and every holder drops it. ("Exited", not "retired": main's idle retirement is a different thing, deliberately shutting an idle daemon down.)
Detection, in one place. Every adapter operation connects first. When that connect fails, an adapter with no respawn path marks its daemon exited only if both of these hold:
ECONNREFUSEDorENOENT. Persrc/main/daemon/AGENTS.md, only a refused or missing endpoint proves nothing is serving it; a timeout proves nothing;legacy-daemon-exit-evidence.ts, built on the existinginspectProcessLivenessandmergeProcessLivenessVerdict).Every weaker case still fails closed, as before: a live or recycled pid,
EPERM, an unreadable pid file, or no pid known at all.The mark is final. Nothing respawns an older daemon, so once its process has exited it can never own a terminal again. The adapter clears its session tracking, stops checkpointing, and notifies listeners through
onDaemonExited. A provider-contract method,hasDaemonExited, says so.Holders drop it:
forgetProvider) and ignores any answer it gave before its daemon was found exited, during a lookup that was already in flight.Not disposed. The adapter is deliberately not disposed: disposing would mark its sessions cleanly ended and suppress that restore. Quit-time dispose and disconnect still cover every adapter, as before.
Not changed: "
--forcesaid ok but left the folder." This was suspected as a second bug. Orca's trace log shows every one of those forced deletes went on to succeed in the background, some taking about 35 s.worktree rmreplies as soon as the delete is accepted (removed: true, removing: true), and Git's checkout delete finishes afterwards. The folder was checked at 3 s and 18 s, before the delete had finished. Theremoved: truefield is misleading for scripts, but changing what the CLI waits for is a separate decision and is not in this PR.Why
docs/reference/ssh-execution-boundary.md). A live daemon that is briefly unreachable must still block a delete, so dropping it needs both a refused endpoint and a dead pid.createLegacyDaemonAdaptersalready ignores an older socket that refuses connections at every startup. This applies the same judgement mid-session instead of only at the next restart, with a stricter liveness test:EPERMcounts as alive here.Limitations (labelled):
inspectDaemonProcessIdentity), but on Windows it starts PowerShell for each check, whichdocs/reference/windows-edr-posture.mdwarns against on a recurring path. Failing closed is no worse than today.nohupor tmux) could outlive a force-killed daemon. Restarting Orca already treats such a daemon exactly this way.Connection lostrather than a refused connect) still fails once; the next attempt drops it.Linked Issue
N/A (maintainer; found in the field).
Visual Proof
No UI change. These are terminal transcripts of the reproduction test: a real current daemon, plus a leftover
daemon-v40.sockfile with nothing listening and a v40 pid that no longer exists. Neither run used the live Orca daemon or its files.Before (origin/main
25e0a29), the exact error from the field:After (this PR):
Live check (hidden rig on this laptop, isolated HOME and user data, never the live app or its daemon)
How it was set up:
wt-del,wt-keep) and was force-stopped. Its v42 daemon stayed alive, owning those terminals.Before (main): the exact field error, and the folder stays:
After (this PR): the delete succeeds without
--force(folder and git registration gone within 3 s), and thewt-keeppane reconnects with no error:Not shown by the live check: the reconnected pane displayed a fresh prompt. The earlier lines stayed in its saved snapshot (
checkpoint.json) but were not on screen. Whether the screen should replay them belongs to Orca's general restore path, which this PR doesn't change.Testing
New
src/main/daemon/legacy-daemon-exit-inventory.test.tsruns the real adapter, router, degraded provider andkillAllProcessesForWorktreeagainst a real daemon server and a stale v40 socket file with nothing listening. It covers:The socket-file cases are skipped on Windows, which uses named pipes and leaves no socket file.
A resolver unit test covers a daemon that retires while an owner lookup is in flight: its earlier answer is ignored and no route is recorded.
Ablations, each turning its own guard red: without the router filter; without the degraded filter; without retirement on connect; without the retired-provider handling in attach; with retirement bumping the epoch; without the pid check; without the respawn check; without the in-flight indexing skip.
27 neighbouring files (router, degraded provider, owner resolution, adapter recovery and adoption, restart sequence, teardown, memory hydration, session management, and others): 462 tests pass.
tsc -p config/tsconfig.node.jsonis clean, andorca-ci-checkspasses 22/22.Live check on macOS in an isolated hidden rig (above). Real
~/.codex/config.toml,~/.codex/hooks.json,~/.claude/settings.jsonhashes, the live app's daemon folder listing and its daemon process were identical before and after.Platforms: macOS locally. Linux and Windows follow the same code path but were not run; on Windows the endpoint is a named pipe, which refuses with
ENOENT. SSH is not affected: remote terminals go through the relay, notDaemonPtyAdapter.I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Two fresh Claude review rounds, neither with a P0 or P1 finding.
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
Author: Brennan Benson (X: @BrennanKB5)
Cross-platform considered: macOS and Linux use socket files; Windows uses named pipes and
process.kill(pid, 0). Not relevant to remote SSH, mobile, or backwards compatibility: no wire or persisted-format change. Performance: the pid-file read and signal-0 check run only after a refused connect on an older-daemon adapter, and at most once, because the mark is final.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)