Skip to content

fix(daemon): bind upgrade shutdown to authenticated instance ownership (#351) - #354

Merged
Ancienttwo merged 5 commits into
mainfrom
codex/issue-351-daemon-pid-identity
Oct 15, 2026
Merged

Ancienttwo merged 5 commits into
mainfrom
codex/issue-351-daemon-pid-identity

Conversation

@Ancienttwo

@Ancienttwo Ancienttwo commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Fixes #351.

daemon upgrade previously 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-unproven before 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:

  • Before the fix, five meaningful assertions failed: PID-only record, reused unrelated child PID, identity change after health, mismatched expected identity, and external SIGTERM during normal upgrade. The dead-PID regression passed.
  • All six isolated-process regressions now pass. Refusal preserves child liveness, connection/lock bytes, and real SQLite bytes; no replacement daemon log is created. Normal upgrade uses self-stop without any external signal. Only fixture-owned children are used.
  • Related suites: 270 pass / 1 existing platform skip / 0 fail; two real CLI incompatible/stale-entry upgrade cases pass. Typecheck and package-boundary audit pass.
  • Full bun run verify passed in the native Herdr shell on exact HEAD 711ea5c6e7bed639eff8b85f64f02dda6405f51e: main suite 3100 pass / 3 skip / 0 fail, followed by packaged smoke, privacy, acceptance, and configured eval gates. Working tree remained clean.
  • The first API-shell verify attempt failed at Chromium's macOS sandbox launch with Input/output error before the full suite. That environment failure is retained; the native rerun used the same command and flags, with no verifier or browser-flag changes.

Boundary: base a697adad7bf686f309405d7a9feef8173d8140a5; sole writer w8:p4 Codex. main.ts changes 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.

@Ancienttwo
Ancienttwo marked this pull request as ready for review October 15, 2026 04:13
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T04:15:12.755184Z 1735f15 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Ancienttwo
Ancienttwo merged commit 31bbc3b into main Oct 15, 2026
44 checks passed
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.

daemon upgrade trusts a persisted live PID without verifying process identity

1 participant