Repository navigation
fix(daemon): bind upgrade shutdown to authenticated instance ownership (#351) - #354
Merged
Merged
Conversation
Ancienttwo
marked this pull request as ready for review
October 15, 2026 04:13
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #351.
daemon upgradepreviously treated a persisted live PID as permission to send SIGTERM. If that PID had been reused, it could terminate an unrelated process before starting a replacement writer.Upgrade now verifies the private per-instance credentials through authenticated health, checks the actual peer PID, and rereads the connection. It asks the daemon to stop itself through the existing authenticated shutdown RPC, carrying the expected PID, root, and token-derived instance ID. The server checks that identity against its own memory after reading the request; a refused result never schedules shutdown. There is no external PID signal in upgrade and no command-line-text identity authority.
Replacement waits for the daemon-owned removal of its connection and lock files. A changed connection, foreign live lock, failed identity check, or unprovable live record returns
AC_RUNTIME_VERSION_UNSUPPORTED/daemon-ownership-unprovenbefore recovery or startup. Dead records are rechecked before recovery. The existing exclusive SQLite writer ownership remains the final writer uniqueness boundary.Legacy guidance: PID-only/missing-credential records and unsupported old RPC peers refuse safely. Stop such a daemon with the previous
archctx daemon stop, or verify and stop it in its original owning terminal, then rerun upgrade. Same-schema authenticated daemons remain upgradeable. No RPC aliases or new lifecycle endpoint were added.Validation:
bun run verifypassed in the native Herdr shell on exact HEAD711ea5c6e7bed639eff8b85f64f02dda6405f51e: main suite 3100 pass / 3 skip / 0 fail, followed by packaged smoke, privacy, acceptance, and configured eval gates. Working tree remained clean.Boundary: base
a697adad7bf686f309405d7a9feef8173d8140a5; sole writer w8:p4 Codex.main.tschanges are confined to upgrade and necessary imports. Other production edits are RPC client/protocol/server; tests cover those paths. D5, architecture-debt, docs-anchors, release metadata, and historical evidence/PRs/comments are untouched. Claude's read-only pre-edit plan review recommended the smaller existing-RPC mechanism and it was adopted.Final acceptance (2026-10-15): candidate 1735f15 integrates main 71d8b65 and adds macOS22 as the fifth existing three-shard target in a separately reviewable CI configuration change. Verify 38659902308 completed 43/43 jobs successfully, including all nine OS/Node summaries. Independent source and incremental reviews found no blockers; the seven-file identity-fix delta remains unchanged. Workflow/controller/caps and assertions are unchanged; topology is 15 shards, four full workers, 29 execution jobs, 31 installs and 43 total jobs. Earlier cancelled candidates are historical evidence, not current acceptance. Coordinator acceptance permits merge under the owner's existing green-and-merge instruction. No release or deployment is included.