Skip to content

fix(daemon): an older daemon that exited no longer blocks worktree delete - #26402

Open
brennanb2025 wants to merge 6 commits into
mainfrom
brennanb2025/worktree-rm-stale-daemon
Open

brennanb2025 wants to merge 6 commits into
mainfrom
brennanb2025/worktree-rm-stale-daemon

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​268 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​267
Prod 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​223 $\color{#cf222e}{\Huge{\mathbf{−}}}$​44 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​179

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:

orca worktree rm --worktree id:<repo>::<path> --json
runtime_error: Failed to physically stop every PTY for worktree: ... the terminal sweep failed:
connect ECONNREFUSED ~/Library/Application Support/orca/daemon/daemon-v40.sock.
Retry with force delete (--force) to remove it anyway.

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:

  • Deleting a workspace works again without restarting Orca or using --force.
  • A terminal pane that was running on that daemon reconnects to a working terminal. Before, it showed "Orca couldn't safely reconnect this terminal because the host couldn't verify its saved session" until restart. (In the live check below, the reconnected pane showed a fresh prompt; the earlier lines stayed in the session's saved snapshot but were not visible on screen.)

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.)

  1. 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:

    • the connect failed with ECONNREFUSED or ENOENT. Per src/main/daemon/AGENTS.md, only a refused or missing endpoint proves nothing is serving it; a timeout proves nothing;
    • no process answers for any pid known for that daemon: the record read when Orca adopted it, and the pid file on disk now (legacy-daemon-exit-evidence.ts, built on the existing inspectProcessLiveness and mergeProcessLivenessVerdict).

    Every weaker case still fails closed, as before: a live or recycled pid, EPERM, an unreadable pid file, or no pid known at all.

  2. 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.

  3. Holders drop it:

    • The router and the degraded provider leave an exited-daemon adapter out of their adapter sets (main's idle retirement then no longer reads it as "unverifiable" either), so the worktree-delete sweep, terminal-owner lookup, memory hydration, the session manager and startup reconcile all stop asking it.
    • A daemon restart no longer carries it over.
    • Owner lookup forgets its routes (forgetProvider) and ignores any answer it gave before its daemon was found exited, during a lookup that was already in flight.
    • An attach whose owner's daemon turns out to have exited reads as "session not found", which sends the pane down Orca's existing restore path.
  4. 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: "--force said 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 rm replies 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. The removed: true field is misleading for scripts, but changing what the CLI waits for is a separate decision and is not in this PR.

Why

  • Removes the bug class, not one symptom. The root cause was that nothing ever retired an older-daemon adapter while the app ran. Answering "empty" in one reader (this PR's first version) still left the dead adapter in every other fan-out and blocked reattach. Dropping it once, in a single place, fixes every reader at once.
  • Follows the common pattern for a process that exits underneath its owner. The exit is a lifecycle transition that removes it from the live registry and notifies listeners; readers only read the registry and never probe on read. Difference (intended): the common pattern learns of the exit from an exit event or a fresh daemon instance reconnecting. An older Orca daemon never reconnects, so its first refused connect, backed by the pid evidence, plays the role of that exit event.
  • Not reachable is not proof of exit (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.
  • The same evidence Orca already acts on. createLegacyDaemonAdapters already 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: EPERM counts as alive here.
  • Why a daemon exit doesn't bump the owner-lookup epoch. Bumping it, as a daemon identity change does, would discard the very lookup that discovered the exit and report "owner unverified". Skipping exited-daemon providers while indexing a lookup keeps that lookup valid.

Limitations (labelled):

  • Intended: a recycled pid keeps the old behaviour (refused delete), because the pid then reads as alive. A stronger identity check exists (inspectDaemonProcessIdentity), but on Windows it starts PowerShell for each check, which docs/reference/windows-edr-posture.md warns against on a recurring path. Failing closed is no worse than today.
  • Intended: the evidence is about the daemon process, not each terminal's child processes. A child that ignores SIGHUP (for example nohup or tmux) could outlive a force-killed daemon. Restarting Orca already treats such a daemon exactly this way.
  • Intended: the first operation that hits the dead daemon within its disconnect window (Connection lost rather 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.sock file 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:

before.png

After (this PR):

after.png

Live check (hidden rig on this laptop, isolated HOME and user data, never the live app or its daemon)

How it was set up:

  • A dev build of this branch ran with protocol v42, opened terminals in two scratch worktrees (wt-del, wt-keep) and was force-stopped. Its v42 daemon stayed alive, owning those terminals.
  • The same rig was relaunched with the daemon protocol bumped to v43 locally (never committed), so it adopted the v42 daemon as an older daemon and reattached its terminals.
  • The v42 daemon was then sent SIGTERM by its recorded pid, as in the field. Its pid and token files went away; the socket file stayed.
  • Before = main's code; after = this PR.

Before (main): the exact field error, and the folder stays:

before-cli.png

live-before-app.png

After (this PR): the delete succeeds without --force (folder and git registration gone within 3 s), and the wt-keep pane reconnects with no error:

after-cli.png

live-after-app.png

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.ts runs the real adapter, router, degraded provider and killAllProcessesForWorktree against a real daemon server and a stale v40 socket file with nothing listening. It covers:

    • the delete proceeds;
    • the router drops the retired adapter and never asks it again;
    • a pane routed to a v40 daemon that then shut down gets "not found" (restore from history) instead of a connect error;
    • an unrouted session reads as absent in the same lookup that retires the daemon;
    • the degraded provider drops it too;
    • the session manager lists nothing;
    • it is still refused while the old pid is alive or no pid is recorded;
    • a daemon that has a respawn path keeps its own recovery;
    • the liveness verdict.
      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.json is clean, and orca-ci-checks passes 22/22.

  • Live check on macOS in an isolated hidden rig (above). Real ~/.codex/config.toml, ~/.codex/hooks.json, ~/.claude/settings.json hashes, 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, not DaemonPtyAdapter.

  • 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.

  • Round 1, on the first, inventory-only version: its P2s led to this redesign. Reattach is fixed; the recycled-pid and nohup limitations are labelled above.
  • Round 2, on the retirement design: its P2 was that an in-flight lookup could record a route to an adapter that retired mid-lookup. Fixed, with a test. Its P3 about the second spawn attempt is also fixed.
  • Not done: adding the daemon's handshake-reported pid as further liveness evidence (it would only make the check more conservative).

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and 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

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…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.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status (head bf1fddf8775): draft, ready for coordinator review. CI not yet checked.

Done:

  • Cause confirmed from code and the field logs. The v40 daemon was preserved at app start, then SIGTERMed at 20:52 on Oct 6 while Orca ran. Its adapter stayed in the router, and listProcesses fails if any adapter can't answer, so every delete was refused until restart.
  • Fix and reproduction test. Three ablations each turn their guard red; without the fix, the test fails with the exact field error.
  • 18 neighbouring test files pass (358 tests). Node typecheck is clean, and orca-ci-checks passes 22/22.
  • One fresh Claude review round: no P0 or P1. The P2s are in the body under Limitations, and the P3 test name is fixed.

Not done / not verified:

  • No run on Linux or Windows. The socket-file tests skip on Windows.
  • No live-app check (by design, nothing touched the live daemon).
  • The "--force ok but folder left" report is not a failure: the trace log shows those background deletes succeeded, some in about 35 s. Whether the CLI should wait for the delete is a separate decision; see "Not changed" in the body.

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.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status (head c4bb93490ba): rebuilt as retirement. Draft, ready for coordinator review. CI not yet checked.

What changed since the last status:

  • Redesign. An older-version daemon adapter now retires itself on the first connect that proves its daemon exited (a refused or missing endpoint, and no live process under any known pid). The router, the degraded provider, owner lookup and restart then drop it, so no reader asks it again. A pane routed to it reattaches as "not found" and restores from saved history. Before, it failed until restart.
  • Second review round: no P0 or P1. Its P2, an in-flight lookup recording a route to a just-retired adapter, is fixed with a test; its P3 for the second spawn attempt is fixed too.

Verified:

  • 8 ablations each turn their guard red.
  • 27 neighbouring files pass (462 tests).
  • Node typecheck clean; orca-ci-checks 22/22.

Not verified:

  • No Linux or Windows run; the socket-file tests are skipped on Windows.
  • No live-app check (by design, nothing touched the live daemon).
  • The renderer's restore-from-history flow is exercised only up to the "session not found" result.

…-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.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status (head 8c66f2b2368): CI green, live check done. Draft; coordinator to decide on ready.

  • Main merged (180d8a3c7e7). The PR conflicted with main's new idle retirement, so no CI could run until then. Resolved in the router.
  • Renamed the adapter state from "retired" to "daemon exited", so it can't be confused with main's idle retirement. No logic change.
  • CI at 8c66f2b2368: all 21 required checks pass, including all 10 test shards; 20 skipped, none failing.
    • The only red seen on this PR was on the superseded head bf1fddf: local-downloaded-folder-promotion.test.ts in shard 2/10, a file this PR doesn't touch. It passed on the current head. Unrelated.
  • Live check (hidden rig, isolated HOME and user data; the live app, its daemon and real config were untouched, guard hashes identical before and after). An adopted v42 daemon was SIGTERMed, as in the field.
    • Before (main): worktree rm fails with the exact connect ECONNREFUSED .../daemon-v42.sock error, and the pane shows "couldn't safely reconnect".
    • After (this PR): the delete succeeds without --force (folder gone within 3 s), and the pane reconnects with no error. Images are in the body.
  • Not verified:
    • In the live run the reconnected pane showed a fresh prompt, not the earlier lines; those stayed in the saved snapshot.
    • No Linux or Windows run.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 8, 2026 04:56
@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a84ad70d-8933-4e43-bec4-eb5d6907cf11
📥 Commits

Reviewing files that changed from the base of the PR and between 4f4f021 and 8c66f2b.

📒 Files selected for processing (13)
  • src/main/daemon/daemon-endpoint-errors.ts
  • src/main/daemon/daemon-pty-adapter-subscription-fanout.ts
  • src/main/daemon/daemon-pty-connection-lifecycle.ts
  • src/main/daemon/daemon-pty-router.ts
  • src/main/daemon/daemon-pty-session-inventory.ts
  • src/main/daemon/daemon-session-owner-resolution.test.ts
  • src/main/daemon/daemon-session-owner-resolution.ts
  • src/main/daemon/degraded-daemon-owner-recovery.ts
  • src/main/daemon/degraded-daemon-pty-provider.test.ts
  • src/main/daemon/degraded-daemon-pty-provider.ts
  • src/main/daemon/legacy-daemon-exit-evidence.ts
  • src/main/daemon/legacy-daemon-exit-inventory.test.ts
  • src/main/providers/pty-provider-contract.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.


📝 Walkthrough

Walkthrough

The 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 8c66f

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main fix: an exited older daemon no longer blocks worktree deletion.
Description check ✅ Passed The description is detailed and covers the required explanation, motivation, behavior changes, testing, visual proof, limitations, platform considerations, and checklist. Minor template deviations rem…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant