Make managed Board replacement atomic and truthful - #1100
Conversation
|
Independent exact-head review of
Review validation: 117 focused tests, Ruff, privacy scan, and docs lifecycle passed. |
Exact-head Codex audit — BLOCKEDAudited commit The three earlier findings are fixed at this head: failed delayed health rolls back; an unreadable prior definition is never called restored; and an older Board is validated against its captured pre-mutation version. The 120 focused Board service tests pass, P2 — “restored absence” does not re-read the port
I reproduced this with the existing Please make the absence reconciliation require an available listener inventory with no listeners on the target port, and add a regression fixture for a detached/foreign listener surviving rollback. If inventory is unavailable or the port remains occupied, report Because this exact head has one P2, I am leaving the audit label state blocked. |
Exact-head Codex audit — BLOCKEDAudited commit The prior findings are fixed at this head, including the first-install rollback re-read: a surviving detached listener or unavailable listener inventory now yields P2 — ambiguous bootstrap of an existing unloaded definition is still reported as failure without reconciliation
I reproduced it with the existing Please route this unchanged-definition bootstrap failure through delayed health plus a fresh final binding read. If the binding verifies, report |
Exact-head Codex audit — BLOCKEDAudited commit The earlier atomicity findings are fixed at this head: failed delayed/final validation rolls back; an unreadable prior definition is never called restored; older-version rollback uses captured version evidence; first-install rollback re-reads the unfiltered listener inventory; and an ambiguous bootstrap of an unloaded installed definition is reconciled from delayed health plus a fresh binding read. The 125 focused Board service tests, Ruff, privacy scan, docs lifecycle, and P2 — an unsuccessful no-op kickstart is reported as a completed restart
I reproduced this with the existing Please capture the pre-action supervised PID and require a different final PID before treating an ambiguous failed kickstart as a verified restart. If the PID is unchanged, report the truthful failed/unresolved result with the recovery instruction. Add the no-op failed-kickstart regression alongside the existing “restarts then reports failure” fixture. The unloaded-definition bootstrap path already has an absent-to-loaded transition and does not share this ambiguity. |
Exact-head Codex audit — PASSAudited commit No P0/P1/P2 findings remain. I rechecked the complete terminal-state matrix and every earlier finding:
The exact-head implementation re-reads the definition digest, launchd state/PID/argv, process checkout and supervisor, exclusive port owner, repository identity, installed version, and serving version before a terminal result. Replacement, rollback, and unresolved results remain mutually exclusive in both JSON and text output. Validation at this head:
The working tree was clean before and after review. This audit is limited to the immutable head above; subsequent commits require a new exact-head audit. |
Claude audit (merge-authority lane)Head SHA: |
7ce9f78 to
9470a64
Compare
|
Rebased onto The complete binary diff is byte-for-byte identical to the previously audited patch ( |
|
Code Mower notice: previously audited head |
Exact-head Codex audit — PASSAudited commit P0: 0 · P1: 0 · P2: 0 No blocking findings remain. I reviewed the whole rebased patch and rechecked the complete Board terminal-state matrix:
Before every successful terminal claim, the implementation freshly verifies the definition digest, launchd load state/PID/argv, process checkout and supervisor, exclusive target-port ownership, repository identity, installed version, and serving version. Replacement, rollback, restored absence, and unresolved outcomes stay distinct in the returned payload. The rebased binary patch is byte-identical to the previously audited patch: SHA-256 Validation at this exact head:
The working tree was clean before and after review. This verdict applies only to the immutable head above; any subsequent commit requires a new exact-head audit. |
Claude audit (merge-authority lane)Head SHA: |
|
Exact-head writer evidence for This head resolves the two findings from the Claude audit of
The failed-bootout path retains its explicit final binding read because that path never enters the settled-health window. Validation on this exact head:
This is writer evidence only. Fresh independent Codex and Claude exact-head audits remain required before merge. |
|
Code Mower notice: previously audited head |
Exact-head Codex audit — BLOCKEDAudited commit P0: 0 · P1: 0 · P2: 1 The two findings from the Claude audit of P2 — a restored unloaded prior service is falsely reported unresolvedWhen the prior definition is readable but its launchd job was already unloaded, I reproduced the replacement path with the existing Missing pre-mutation version evidence should force unresolved only when a prior process was serving and evidence was expected but unavailable. For a previously unloaded definition, allow the normal post-restore binding/version contract to decide the result. Add regression coverage for both failed replacement and failed-write restoration from a readable, unloaded prior definition. Validation at this exact head:
The working tree was clean before and after review. I am leaving |
Claude audit (merge-authority lane)Head SHA: |
|
Code Mower notice: previously audited head |
Exact-head Codex audit — PASSAudited commit P0: 0 · P1: 0 · P2: 0 No blocking findings remain in the whole patch. The rebase is a byte-for-byte replay of the previously approved I rechecked the complete replacement and rollback terminal-state matrix and every prior finding. The settled-health sample is the terminal evidence for replacement, restart, rollback, and failed-write restoration. A readable definition whose job was positively absent before mutation can be restored only when the complete binding passes. A loaded or unknown prior job with missing pre-mutation version evidence remains fail-closed as Launchd is the only implemented managed-service backend. Unsupported platforms still fail closed before mutation. Validation at this exact head:
A fresh CodeRabbit CLI pass could not run because its free on-demand allowance is rate-limited for 46 minutes. The byte-identical previously approved patch had a zero-finding whole-patch CodeRabbit review; I completed this audit independently from the full diff and tests. This verdict applies only to the immutable head above; any subsequent commit requires a new exact-head audit. |
|
Code Mower notice: previously audited head |
Claude audit (merge-authority lane)Head SHA: |
|
Code Mower notice: previously audited head |
2 similar comments
|
Code Mower notice: previously audited head |
|
Code Mower notice: previously audited head |
|
Code Mower notice: previously audited head |
|
Exact-head writer evidence for 446442d. This head resolves the two substantive findings from the Claude audit of fad91db:
The audit's BINDING_CHECK_IDS finding was a false positive. The constant exists at src/code_mower/board_service.py line 131 and is exercised by focused tests; no change was made for that assertion. Validation on this exact head:
Fresh independent Codex and Claude exact-head audits remain required. This is writer evidence only. |
|
Code Mower notice: previously audited head |
Claude audit (merge-authority lane)Head SHA: |
Exact-head Codex audit — BLOCKEDAudited commit P0: 0 · P1: 1 · P2: 0 The provider-detail fix is correct at the ambiguous bootstrap and kickstart call sites: default JSON/text redacts the provider definition path through the shared recursive chokepoint, and P1 — a persistent runtime-state failure still escapes reconciliation
I reproduced this with the existing Please keep this entire reconciliation path guarded when runtime state remains unavailable, return All earlier #1082 findings remain fixed outside this path: settled binding evidence is reused; unloaded versus loaded/unknown restoration stays correctly distinguished for replacement and failed-write recovery; older-version rollback, unreadable takeover, first-install listener verification, ambiguous restart/PID transition, and shell-quoted recovery remain correct. Validation at this exact head:
I am leaving |
|
Code Mower notice: previously audited head |
1 similar comment
|
Code Mower notice: previously audited head |
|
Code Mower notice: previously audited head |
launchctl bootstrapcan load a replacement Board and still report failure, which previously sent replacement into rollback and could leave the on-disk definition describing a different process than the one serving the port. This change makes the host state authoritative: replacement success and rollback success each require a fresh read of the installed definition, supervised PID and argv, process checkout, exclusive port owner, repository identity, installed version, and serving version.Ambiguous bootstrap outcomes now keep a fully verified new binding without attempting rollback. Failed bootstrap, delayed-health, and final-read outcomes all restore and verify the old binding before reporting rollback, using version evidence captured before mutation so an older Board can be restored truthfully. An unreadable prior definition can never be called restored, and every unresolved partial state reports one recovery command with its required working directory. A failed first install verifies that its definition and job are absent and re-reads the target port; an unavailable inventory or surviving detached listener is unresolved and returns the recovery command. Restarting an unchanged definition also treats bootstrap and kickstart errors as ambiguous until delayed health and a fresh final binding read prove a healthy service or an unresolved state. A failed kickstart additionally requires the final supervised PID to differ from the pre-action PID, so a healthy no-op cannot be reported as a completed restart.
The launchd fixtures cover permission-style half-takeover, failed health after successful bootstrap, ambiguous restoring bootstrap, older-version rollback, unreadable takeover failure, detached listeners and unavailable inventories after first-install rollback, and an unrecoverable partial state. The lifecycle guide also documents managed recording defaults and the supported
board serveversusboard servicearguments.Validation:
PYTHONPATH=src python3.13 -m unittest discover -s tests -p 'test_board_service.py'(126 tests)ruff check .PYTHONPATH=src python3.13 scripts/privacy_scan.pypython -m code_mower.docs_lifecycle --jsontwine checkuv pip checkCloses #1082