Repository navigation
feat(lifecycle): add standalone Rust supervisor with crash recovery - #802
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
Codex Review Summary: ✅ CompletedThis 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. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8a9b67316
ℹ️ 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".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Ready for lead: review handoff at second-family review: pending, lead Claude became unavailable and the explicitly requested Muse replacement could not start (quota exhausted). Earlier Claude receipts and current diff are preserved for the lead. Latest normal merge incorporates Validation: Linux source package tests passed (4 unit, 31 integration), Clippy with warnings denied, and earlier model/registry checks passed. Native code is unchanged by the latest base merge. Bot threads are resolved; review-status is CLEAR at this head, but Codex reviewed the pre-merge feature head and a current-head review is requested. Remaining: exact-head full CI, including macOS arm64 and Intel supervisor jobs. These Mac jobs are justified by native process/identity code. Full root suite has no completed local pass; Apple cross-compilation is not runtime evidence. Lead merges; this comment does not claim readiness past those gates. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…isor # Conflicts: # docs/reference/v5-model-coverage.md # scripts/lib/tla-runtime-models.ts
|
@codex review |
|
Ready for lead: eb47cc8 A normal merge of What I ran (Linux 6.8 x86_64): Hosted CI: started at this head; at note time 18 pass, 12 pending (queued macOS jobs and shards), 0 failed. I did not wait for them, as you asked. The previous head 60d737b was 32/32 green. Review: the persistent Codex reviewer is CLEAN at this head. The Codex GitHub bot was re-requested. Recovery-group loops: unchanged since 60d737b (200/200 under parallel load on Linux). The Mac loop command is in the previous note. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Mac result at 60d737b (crate identical at eb47cc8): not clean yet.
Questions to answer from the code while I capture: (1) at the moment the test freezes the blocked guard, can the worker's guard still be waiting on anything the blocked guard or the project supervisor holds: the launch/stop lease on startup fd 2, the log flock, the instance lock, a census step before authored spawn? 🤖 Generated with Claude Code |
|
Diagnosis of the Mac loop failure (9/100, Correction first: at 60d737b, So the forked child did create its file. What never arrived is the worker guard's log write. (1) Yes, there is a shared resource, and it is a product property. The other candidates are clear. The startup fd-2 lease is released ( Deterministic repro on Linux (scratch test, not committed): the test holds (2) Nothing else is shared between fixtures in one test process. Each fixture has its own root. The census filters by session ID and never waits on other tests' processes. (3) It is a product property: one guard stopped (SIGSTOP, a debugger, or a hung write) while it holds the log lock makes every sibling guard unstoppable and blind to exits. It is not merely a delayed start. Proposed fix (recommended default; I will implement it in #802 on your go-ahead):
Alternatives that change the log contract: per-guard log files with no shared lock, or capping record size at PIPE_BUF so O_APPEND writes are atomic without a lock. I would not do either in this slice: #811's readers expect one TSV log at the existing path. Please say whether to proceed with the recommended fix in #802 (it changes the macOS-compiled code; round 5 again). If you would rather defer it, the test-sync part alone would make the loop pass, but the product property would stay. |
|
Cause of the Mac failure (captured run): a frozen guard holding the shared log lock stalls every sibling guard. Correction first: at this head line 1095 is the second wait in Process listing 12 s and 18 s into the failing test (only this fixture was left running):
Stack sample of the worker guard (888 of 888 samples): So: the test froze the blocked guard while that guard held the shared log's flock (it had just written its own first lines). From then on the worker guard cannot flush any output line: it blocks in This is a product fix, in #802:
🤖 Generated with Claude Code |
…on a sibling Every guard took a blocking flock on the shared lifecycle.log around each record. A guard stopped (SIGSTOP, a debugger, a hung write) while holding it stalled every sibling guard's loop in flock: no output, no exit detection, no stop handling. Captured on macOS as the 9/100 failure of expired_target_does_not_abandon_other_recovery (worker guard blocked in Lines::flush -> flock while the frozen blocked guard held the lock). Each record is now one write on the O_APPEND descriptor with no lock. The kernel places each append atomically, so whole records from different guards never interleave on a local file system; only a short write (for example a full disk) can split one. The unused flock retry helper and its unit test are removed. Regressions: held_log_lock_never_stalls_a_guard holds the log flock from outside for the whole run and requires output to reach the log and stop to answer stopped within the normal bound (fails with the old lock); concurrent_guards_append_whole_records_without_a_lock checks two guards bursting 64 KiB records leave only whole, unmixed records. Docs state the log's ordering and integrity and list every lock, who can hold it across a blocking call, and the bound. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hort-write causes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Ready for lead: 15bb1d0 The round-6 delta is Fix (your option a): no lock for log appends. What the log promises now ( Regressions (fail or check on Linux too):
Your item 3, the lock inventory: the "Locks and waits" table in the docs lists each lock, who can hold it across a blocking call, and the bound:
So a guard frozen at an arbitrary instruction can no longer block a sibling guard's command or output. It can delay Loop counts (recovery group What I ran at this head: fmt; Clippy with warnings denied on Linux, aarch64-apple-darwin and x86_64-apple-darwin; 13 unit and 53 fake-caller tests; Review: the persistent Codex reviewer is CLEAN at this head. Its one finding was that the table wrongly promised a framed reply for launches; fixed. It confirmed the append-atomicity claim and found no reader depending on the lock. The Codex GitHub bot was re-requested. For your Mac rerun: the same loop, 100 runs. #811 and #821 are waiting for this, as you said; I will merge this head into them once you've reviewed it. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Second-family review, round 6 (Claude, lead-supplied): CLEAN at 15bb1d0 VERDICT: CLEAN Scope: This review is by reading only. No build, test or Clippy run was made in this round. Kernel facts were checked against Linux v6.8 ( Findings (none blocking)1. Low (tests):
The owner's note already says the test cannot force overlapping or short writes. Suggested fix: make the fixture wait for a start file that the test creates after both guards are ready (or raise the volume), assert that at least one 2. Low (docs): the short-write sentence describes the good case only ( The document says a short write "is completed by a second write and can split one record". From the code:
Also not stated for readers: a reader that reads while a record is being appended can see the head of the record before its tail (the kernel extends the file page by page inside one write). This was the same with the lock, because no reader took it. Suggested fix: say that a final line without LF is incomplete and must be held back, that after a failed write a line can contain a damaged head followed by a whole record, and that a failed log write ends that guard's command. 3. Low (docs): a guard frozen before it detaches makes Each wait bound in the table holds (see "Answers", item 4). The total effect is larger than the intro sentence suggests:
This is the intended lease design from earlier rounds and not a regression. Suggested fix: add one or two sentences: the per-root effect, the failure-path hold, and the recovery step (continue or kill the unpublished guard, which is a session leader started by the supervisor). Nits
Answers to the six review questions1. The append path: correct.
2. Readers: none relied on the lock.
3. The removed helper: nothing else used it.
4. The "Locks and waits" table: each row matches the code.
5. The two new tests.
6. Round-5 low findings.
Round-5 nits: Questions
Checked and fine
Status of earlier open findingsRound 4 (as carried in round 5), code untouched by this delta:
Round 5: see item 6 above; all four low findings are unchanged. Not run by this review: any build, test, Clippy or TLC run; any Linux, Intel or Mac loop. Files outside the crate and its document were read only where the questions pointed to them (the TypeScript log reader and writer on the base branch, the workflow file). Lead: round 6 is clean. The three low findings go to the follow-up PR with the earlier ones: tighten 🤖 Generated with Claude Code |
|
@codex review |
|
Ready for lead: b2b8d53 This is a conflict-free normal merge of What I ran (Linux 6.8 x86_64):
Hosted CI: 13 pass and 17 pending at note time; I did not wait for the queued macOS jobs. Reviews: the persistent Codex reviewer is CLEAN at this head. I re-requested the Codex GitHub bot. Loop counts: the same as the round-6 note (200/200 on Linux, unchanged code). Your Mac loop result decides #802. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Mac result at 15bb1d0 (crate identical at b2b8d53): clean.
With the round-6 review clean, #802 goes into the next combined Mac batch at b2b8d53 and merges if that passes. Then #811 needs a merge of 🤖 Generated with Claude Code |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Resolve conflicts with #802: scripts/lib/tla-runtime-models.ts and docs/reference/v5-model-coverage.md keep both the autostart-start-stop and lifecycle-supervisor entries (the coverage doc has no generator). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…econcile Brings in next at 34b5053 (#802 Rust lifecycle supervisor, #860 read-only update check, #818 desktop Env tab). Conflicts: - src/lib/machine-output.ts and src/cli/json-contracts.ts: both sides added schemas and variants; kept updateCheck / update --check --observe and the four daemon command schemas. - src/commands/update.ts: kept both the observeUpdate import and handleObserve from #860 and this branch's post-update reconcile. --observe returns before anything else, and plain --check returns before the install, so the read-only check never reaches reconcileDaemonAfterUpdate. Generated files (CLI reference, machine-output table, error-code reference) were regenerated and match; E_DAEMON_SERVICE kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # apps/desktop/README.md # apps/desktop/app/src/screenshots.rs # apps/desktop/app/src/view.rs # apps/desktop/core/src/source.rs
# Conflicts: # apps/desktop/README.md # apps/desktop/app/src/main.rs # apps/desktop/app/src/screenshots.rs # apps/desktop/app/src/view.rs # apps/desktop/core/src/fixtures.rs
Add `lifecycle.runner: "supervisor"` for Compose host lifecycle processes; omitted settings keep `mux`. Persistent hooks retain their order, logs keep the existing private TSV path, and down/restart follow the recorded runner. Depends on #802 and its pending native recovery/resource fixes. Packaging is a separate slice. Before hooks or ownership publication, the CLI verifies the sibling executable's permissions and exact version/capability handshake. Compiled installations ignore the executable override; source development and tests may select an explicit binary. Doctor reports its resolved path and version.  Ownership and recovery: - Environment travels only over framed stdin, never argv or payload files. Native IPC lives outside checkout mounts and checks host peers. - Canonical project paths key native roots; ownership records their absolute location. A changed HOME or symlink spelling does not relocate running work. A missing root after dispatch is unconfirmed and retains ownership; it never proves processes stopped. - A durable launch-not-attempted marker permits down/doctor cleanup without the binary only when no PID or process was published. An uncertain launch retains ownership. Adoption advances the cleanup epoch; doctor holds the controller lock throughout retirement. - Initial and later serve requests wait for retiring owners within bounds. Lost incremental replies publish verified late commands before reporting failure. Default mux compatibility changes are explicit: state mutations serialize on `state.lock` and publish atomically at mode 0600. Empty or unparsable JSON from older non-atomic mux writes is preserved in a timestamped sibling, explained once and treated as empty. After supervisor ownership has been published, a durable marker disables this fallback. Unsafe links/foreign ownership and semantically invalid records continue to refuse; global observers keep that project unknown and continue. Run `hack down` before downgrading or switching CLI builds, since older parsers cannot preserve supervisor ownership. Validation on Linux: - 50 Rust tests pass; Clippy with warnings denied and formatting pass. - 56 focused Bun tests pass; an additional definition-replacement regression passes with all 14 real-native controller cases. CLI typecheck/check, changed-file lint, privacy scan and actionlint pass. - Compiled CLI recovery and real-Docker host-process scenarios pass under both runners, no skips or fixture networks left. The compiled missing-companion regression verifies no ownership is published and down remains usable; a production override cannot redirect it. - The controller model remains 21 states; its coverage document distinguishes the finite ownership abstraction from executable, filesystem and platform regressions. - Hosted exact-head CI and second-family review remain pending. No completed local full-repository suite pass is claimed. The native source prerequisite still has lead-required fixes in progress; this PR is not ready to merge. macOS arm64 and Intel jobs are intentional: native identity, Unix sockets, descriptor handling and peer credentials require real Mac execution. Workflow filters now include dependent CLI sources and every executed test; controller checks use the built release executable. Docker shard 4 runs both lifecycle scenarios under the supervisor runner. Release intent: optional capability (`feat`), with the state compatibility corrections above. No lifecycle default change, zellij removal, PTY, log rotation or support for deliberately escaping sessions in this slice. --------- Co-authored-by: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Lifecycle commands currently depend on a mux-hosted shell/Python wrapper. This slice adds a standalone Rust supervisor with framed stdin delivery, private lifecycle TSV logs, durable ownership before launch, and TERM/KILL cleanup after a five-second grace. Resident per-command guards retain session ownership and logging if the project supervisor is killed; recovery refuses duplicates and unsafe signals.
This is the standalone/fake-caller slice of the lifecycle supervisor work. It changes no CLI runner default or configuration and installs no new artifact. CLI opt-in follows #789; distribution packaging is a separate PR. The docs compare artifact choices, list packaging changes, and record the later PTY/attach/provider scope.
Validation:
cargo check --tests; the new workflow runs native contracts on Linux x86_64, macOS arm64 and macOS x86_64. Native macOS evidence is pending.Limits: no PTY; commands deliberately creating another session are unsupported; separately killing a guard with live members produces an unknown-ownership refusal. Darwin lacks pidfds, and numeric process-group signaling retains a small observation/effect race. CLI/e2e parity under both runners and installed-artifact qualification remain later gates. Release intent: optional internal capability (
feat), no default migration in this slice.Linux
pidfd_openerrno (added Nov 15, lead decision): a recovery stop opens a pidfd for each captured member before it signals. With zero flags, a member whose process has exited is reported as:EINVALwhen the PID survives only as a process-group or session IDubuntu-latest(Ubuntu 26.04 image 20261111.196.1; the log does not print the kernel version)ESRCHBefore the fix,
EINVALwas treated as an error, so on 6.8 recovery returned before it sent KILL to a TERM-ignoring survivor, and stop reportedunknown. Four fake-caller tests failed on 6.8 (orphan_recovery_keeps_observed_child_after_term_ends_witness,guard_lost_mid_stop_keeps_admission_snapshot,guard_lost_late_in_stop_gets_full_recovery_deadline,expired_target_does_not_abandon_other_recovery). Both errnos now mean absent. That can only withhold a signal; identity-checked liveness still decides the outcome. A test-only seam injects the errno, so unit tests pin the behaviour on every kernel:EINVALorESRCHskips that member and KILL still reaches the survivor, and any other errno refuses without signalling. Linux-only: the Darwin-compiled code is unchanged from 98d0b0a.Round 5 (Nov 15):
stopandstatusrequest ends with exactly one framed receipt. A failed request exits nonzero with outcomerefused(invalid-request) orunknownand areason:stop-lock-wait-expired,stop-lock-failed,foreign-live-process,receipt-read-failed,retire-failed,stop-incomplete,retired-owner-liveorio-error. The table is indocs/lifecycle-supervisor.md..stop.lock, longer than the two TERM-to-KILL bounds of a recovering stop (about 14 s). An expired wait acts on nothing.expired_target_does_not_abandon_other_recoveryon the Mac,unresponsive_guard_stop_remains_unknownon hosted Intel).Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.