Repository navigation
LOCAL: drain a stale-bundle daemon instead of preserving it (opt-in) - #25
Conversation
A daemon launched from an older app bundle is only replaced when it owns no live session, so a host that is never idle never runs new daemon code. With ORCA_DAEMON_DRAIN_STALE_BUNDLE=1, the first launch instead gives the stale daemon a drain name of its own (link), copies its token and moves its PID record there, and launches a fresh daemon with --handed-over-endpoint <dev>:<ino>. The fresh daemon replaces that exact live entry in one rename; the old one sees its name lost, keeps attaching its sessions and retires when the last ends. Daemon init finds drained daemons beside legacy-protocol ones, the router only attaches to them, and Manage Sessions routes by owning adapter rather than protocol version. Off by default and never on Windows. AGENTS.md records the exception to the endpoint-ownership invariant and its limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review complete. No issues found — approved ✅. Adds the sanctioned stale-daemon drain hand-over ( Reviewed commit: 1a813da |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a813da5c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A launch killed after the PID record moved but before the fresh daemon took the name left a drain slot aliasing the still-current daemon. The next start would register both names, see every session twice and settle no owner, so attach-only restore failed. Every launch now undoes such a slot first (record back, alias removed), and discovery skips any name aliasing the current endpoint or a slot it already registered. Also replaces the drain's type assertions with checked types, keeping two test doubles behind line-specific SAFETY comments, for the changed-code casting gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review complete. 🟡 1 medium 💬 Inline comments (1)
Reviewed commit: 372588a |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 372588af91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A newer Tenki review found issues on this pull request.
ELI5
When Orca updates, the program that keeps your terminals running is left on the old version for as long as any terminal is open. On a machine that is never idle, it never updates. With this change, a new copy starts next to the old one and takes all new terminals. The old copy keeps the terminals it already has and shuts down once the last of them closes. Nothing gets cut off. It's off unless the host turns it on.
What Changed
Before: a daemon launched from an older app bundle (
isDaemonStaleForCurrentBundle) is replaced only at zero live sessions. Otherwise it is preserved (daemon-replacement-preflight.ts). On minidev-alex1 the daemon still runs…202609281942while the app runs…202609282117.After, with
ORCA_DAEMON_DRAIN_STALE_BUNDLE=1: on the first launch after an update, a stale daemon that has live sessions is drained instead of preserved.daemon-drain.ts,daemon-out-of-process-launcher.ts):drain-v<N>-<hex>.sock, by exclusivelink). It checks that the new name is the same inode and that the daemon answers its token through it.--handed-over-endpoint <dev>:<ino>.daemon-endpoint-ownership.ts): onEEXIST, if the canonical entry is still exactly that inode, it takes the name with onerename, so the name is never absent. Every other case falls through to the unchanged proven-dead path.attachOnly. It refuses to create and retires once its last session ends. So this also drains daemons built before this PR, including the one running on minidev-alex1 today.daemon-legacy-adapters.ts): daemon init finds drain slots beside the legacy-protocol endpoints and buildsdrainingadapters that never respawn. A slot is removed only when nothing is serving it and its recorded process is dead. A slot that can't be classified stays.daemon-pty-router.ts): a create-or-attach routed to a draining adapter is sent asattachOnly. Once the session is proven absent, it is created on the current daemon. An owner that cannot be established still throws, so a session is never started twice.pty-management.ts): kill-one and kill-all route by owning adapter, not by protocol version, because a drained daemon shares the current version.It is first launch only: daemon init discovers drained daemons once, and a respawn under a live router would strand the drained sessions. It is never on Windows, because named pipes have no second name.
src/main/daemon/AGENTS.mdrecords this as the one sanctioned exception to "only replace an entry proven dead", with its limits.Why
Options considered for picking up new daemon code on a busy host:
A drain is the only option that loses nothing. It reuses two mechanisms the fork already has: the daemon's retire-once-sessions-end on ownership loss, and the router's multi-daemon routing for protocol upgrades. The app never mutates the canonical endpoint; only the publishing daemon does, with the same one-
renameand two-syscall residual risk as the proven-dead path.Linked Issue
No fork issue. This is item 2 of the SwapLabs host-updater hardening workstream (goal: "Orca fork updates fully automated"). The devvms setting that turns it on per host follows in a separate SwapLabsInc/SwapLabs PR.
Visual Proof
N/A — no UI change; terminals stay attached across an update, verified by the end-to-end test below.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
daemon-drain.test.ts, end to end with two realDaemonServers on Linux:Checked the end-to-end test catches a broken hand-over: disabling the fresh daemon's takeover fails 2 tests.
daemon-init-drain.test.ts, launcher level:--handed-over-endpoint;daemon-pty-router-drain.test.ts: attach-only routing, fallback to current once absent, no create when ownership is unsettled.daemon-endpoint-publish.test.ts: takes over the handed-over live entry, leaves any other live entry in place.daemon-entry.test.ts: argument parsing, including malformed values.pty-management.test.ts: kill-one routes to a same-version drained daemon.pnpm lintandtypecheck:nodepass.vitest src/main/daemon src/main/ipc: all pass except 9 tests in 4 files that fail identically onswaplabs/mainin this environment (bash DEBUG-trap, ephemeral-VM ref and real-git worktree tests).Not yet run: a live drain on a host. That needs a build with this change, and the devvms setting. Platforms: Linux tested; macOS is the same code path and wasn't run; Windows is disabled by design.
AI Disclosure
Written with Claude Opus 5.5 (Claude Code).
Review
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
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Deferred review findings
daemon-out-of-process-launcher.ts). Its drained sessions stay alive but can't be reached until the next launch discovers the slot. Fix: install a degraded provider around the drained adapter, or republish a current daemon, on that failure path.recoverUncommittedDaemonDrains). Recovery could dismantle an in-flight drain slot. The per-profile single-instance lock rules this out outside diagnostic bypasses. Optional hardening: recover only slots whose PID record has moved, and re-check the slot inode after the move.