Skip to content

Make managed Board replacement atomic and truthful - #1100

Merged
jeffhuber merged 8 commits into
mainfrom
codex/1082-board-atomic
Sep 21, 2026
Merged

jeffhuber merged 8 commits into
mainfrom
codex/1082-board-atomic

Conversation

@jeffhuber

@jeffhuber jeffhuber commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

launchctl bootstrap can 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 serve versus board service arguments.

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.py
  • python -m code_mower.docs_lifecycle --json
  • source distribution and wheel build, followed by twine check
  • uv pip check

Closes #1082

@jeffhuber jeffhuber added needs-claude-audit builder:codex Code Mower generated label labels Sep 21, 2026
@jeffhuber

Copy link
Copy Markdown
Contributor Author

Independent exact-head review of 079c9b61ef5f412e427ebea9de998b5f2a647a9e is BLOCKED with two P1s and one P2:

  • P1 — failed delayed health does not roll back. In src/code_mower/board_service.py:2268-2295, once bootstrap() returns success, a failing health window returns delayed_health_failed while leaving the replacement installed/running. A replacement that serves the wrong repo reproduces this. v1.6.0: make managed Board replacement atomic and truthful #1082 and docs/board-service-lifecycle.md:157-164 require verified old-or-new state. Route failed delayed/final validation through rollback and verify the old binding; report unresolved with recovery only when rollback cannot be verified. Add a successful-bootstrap/failed-health fixture.
  • P1 — unreadable prior state is falsely reported restored. At board_service.py:2240-2255, an unreadable prior definition is treated like first install. After failed replacement and deletion, reconciliation says absent/verified and the message says the previous service was restored, though the definition is gone. An explicit takeover cannot back up unreadable prior state; report unresolved (or another truthful discarded-prior state) with exact recovery. Add an unreadable-takeover failure fixture.
  • P2 — rollback verification hardcodes the current version. validate_binding() compares identity versions with the invoking install; rollback therefore rejects a byte-identical restored older Board. Capture pre-mutation identity/version evidence and validate restored-old against it, or use a target-specific prior-binding validator. Add an older-version rollback fixture.

Review validation: 117 focused tests, Ruff, privacy scan, and docs lifecycle passed.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — BLOCKED

Audited commit d7ba94e5717148e048e932b977c854eb177fe911 against c38c137353e0d58ef60b66d47aebd2a73668ab62.

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, git diff --check passes, and the privacy scan passes.

P2 — “restored absence” does not re-read the port

_rollback_and_reconcile() declares a first-install rollback verified when the definition is gone, runtime_state() says the launchd label is absent, and _rollback() returned success (src/code_mower/board_service.py:2126-2145). It never calls port_listener_inventory() in this branch. A Board process can fork or race a detached listener that survives bootout; the code then returns apply_failed with reconciliation: {state: "absent", verified: true} while that process still owns the requested port. That contradicts the PR/issue invariant that the final result re-read the port and leaves the operator without the recovery command emitted for unresolved states.

I reproduced this with the existing FakeHost: bootstrap starts the job but reports failure, binding is forced to fail, rollback successfully removes the launchd job and definition, and a supervised detached listener is added on port 5332 during bootout. The result was apply_failed, state=absent, verified=true, while listeners={5332: 901} and the definition was absent.

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 rollback_failed, state=unresolved, and the exact recovery instruction.

Because this exact head has one P2, I am leaving the audit label state blocked.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — BLOCKED

Audited commit 50908a8f09f7c8cc33c0f4be3ef263c460b80a22 against c38c137353e0d58ef60b66d47aebd2a73668ab62.

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 rollback_failed, state=unresolved, and the recovery instruction. The 122 focused Board service tests and git diff --check pass.

P2 — ambiguous bootstrap of an existing unloaded definition is still reported as failure without reconciliation

restart_service() handles an installed definition whose launchd job is absent by calling provider.bootstrap() at src/code_mower/board_service.py:2724-2737, but any nonzero provider result returns apply_failed immediately at lines 2738-2746. This bypasses the fresh health/final-binding reconciliation added for the same ambiguous launchctl bootstrap outcome in _apply().

I reproduced it with the existing FakeHost: install a valid managed Board, boot its launchd job out while leaving the definition installed, then use a provider whose bootstrap() loads the job and returns failure. restart_service() returns a bare apply_failed payload with no reconciliation, while the label is loaded and PID 901 exclusively serves port 5332 with the expected definition, argv, checkout, repository, and version. This is the same launchd behavior the PR correctly treats as authoritative-host-state success on a replacement, and it contradicts the PR statement that ambiguous bootstrap outcomes keep a fully verified binding.

Please route this unchanged-definition bootstrap failure through delayed health plus a fresh final binding read. If the binding verifies, report restarted; if it does not, report the truthful unresolved failure/recovery state. Add a regression fixture for “existing definition, job absent, bootstrap loads then reports failure.” The analogous loaded-job kickstart failure should also be reconciled if that provider result can be ambiguous.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — BLOCKED

Audited commit a0e1d17eb958896c07282c5e8f8c723c78199e32 against c38c137353e0d58ef60b66d47aebd2a73668ab62.

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 git diff --check pass.

P2 — an unsuccessful no-op kickstart is reported as a completed restart

restart_service() now checks that the installed binding is healthy after kickstart, but it does not check that the supervised process actually changed (src/code_mower/board_service.py:2732-2809). A failed kickstart -k can leave the existing healthy process untouched. Because that unchanged process passes delayed health and the final binding read, the function returns status: restarted and says the service was independently verified as restarted, although no restart occurred.

I reproduced this with the existing FakeHost: install a healthy service at PID 900, use a provider whose kickstart() returns False, "Kickstart failed: 1: Operation not permitted" without mutating host state, then call restart_service(). The result is restarted, reconciliation.state=new, and verified=true, while the final PID is still 900. The new positive fixture proves the opposite case only because its provider actually restarts before returning failure.

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.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — PASS

Audited commit 7ce9f78a09cc830611baf683b85a6f9f8df850db against c38c137353e0d58ef60b66d47aebd2a73668ab62.

No P0/P1/P2 findings remain.

I rechecked the complete terminal-state matrix and every earlier finding:

  • a bootstrap that reports failure after loading a fully verified replacement keeps the new binding without attempting rollback;
  • successful bootstrap followed by failed delayed or final validation restores and independently verifies the prior binding;
  • an unreadable prior definition is never described as restored;
  • an older restored Board is checked against version evidence captured before mutation;
  • first-install rollback proves definition absence, launchd-job absence, and an available zero-listener inventory; a detached listener or unavailable inventory stays unresolved with one recovery command;
  • ambiguous bootstrap of an installed but unloaded definition is reconciled through delayed health and a fresh full binding read;
  • ambiguous failed kickstart is accepted only when the final healthy binding has a different supervised PID; a failed no-op kickstart with the original PID is reported as apply_failed, unresolved, with the recovery instruction.

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:

  • PYTHONPATH=src python3.13 -m unittest discover -s tests -p 'test_board_service.py' — 126 tests passed
  • uvx ruff check src/code_mower/board_service.py tests/test_board_service.py — passed
  • PYTHONPATH=src python3.13 scripts/privacy_scan.py — passed
  • uv run --with pyyaml python -m code_mower.docs_lifecycle --json — 140 documents, zero problems
  • git diff --check origin/main...HEAD — passed

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.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 7ce9f78a09cc830611baf683b85a6f9f8df850db
Verdict: PASS
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at c38c137353e0d58ef60b66d47aebd2a73668ab62

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #1099 merged. New exact head: 9470a646e3f63cbce2161cf833dd3d3ebe73aa94.

The complete binary diff is byte-for-byte identical to the previously audited patch (66,888 bytes; SHA-256 7d8485410d5a5e214c0c3d7710faf554ca1b4a2fe8b25e8a4c7bb9750525e16f). Post-rebase validation remains green: 126 Board service tests, Ruff, privacy scan, and diff hygiene. Fresh exact-head Codex and Claude receipts are required before merge.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 7ce9f78a09cc is no longer an ancestor of current head 9470a646e3f6; commits may have been dropped. Only current-head audit verdicts count.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — PASS

Audited commit 9470a646e3f63cbce2161cf833dd3d3ebe73aa94 against current main 052d49452605eaf668fb07f385673fe5e25b303f.

P0: 0 · P1: 0 · P2: 0

No blocking findings remain. I reviewed the whole rebased patch and rechecked the complete Board terminal-state matrix:

  • an ambiguous bootstrap that leaves a fully verified replacement keeps that new binding;
  • a successful bootstrap followed by failed delayed or final validation restores and independently verifies the prior binding;
  • an unreadable prior definition is never described as restored and yields an unresolved result with recovery guidance when takeover fails;
  • an older restored Board is checked against version evidence captured before mutation;
  • first-install rollback verifies definition absence, launchd-job absence, and an available zero-listener inventory; surviving listeners or unavailable inventory remain unresolved;
  • ambiguous bootstrap of an installed but unloaded definition is reconciled from delayed health and a fresh full binding read;
  • ambiguous failed kickstart is accepted only when the final healthy binding has a different supervised PID; a failed no-op remains apply_failed, unresolved, with a recovery command;
  • removal never claims a released port when listener inventory is unavailable or a listener survives.

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 7d8485410d5a5e214c0c3d7710faf554ca1b4a2fe8b25e8a4c7bb9750525e16f. The intervening main change touches only doctor taxonomy files and has no overlap with this patch.

Validation at this exact head:

  • PYTHONPATH=src python3.13 -m unittest discover -s tests -p 'test_board_service.py' — 126 tests passed
  • uvx ruff check src/code_mower/board_service.py tests/test_board_service.py — passed
  • PYTHONPATH=src python3.13 scripts/privacy_scan.py — passed
  • uv run --with pyyaml python -m code_mower.docs_lifecycle --json — 140 documents, zero problems
  • git diff --check origin/main...HEAD — passed

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.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 9470a646e3f63cbce2161cf833dd3d3ebe73aa94
Verdict: BLOCKED
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 052d49452605eaf668fb07f385673fe5e25b303f

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head writer evidence for 89e1df404c7e05f4777b484777fdbb5d46b34ced.

This head resolves the two findings from the Claude audit of 9470a646e3f63cbce2161cf833dd3d3ebe73aa94:

  • The successful replacement, ambiguous-bootstrap, restart, and rollback-restoration paths now use the last complete validate_binding sample from the bounded settled-health window as their terminal binding evidence. They no longer perform an extra unretired single-shot read after health has already settled, so a transient later probe cannot spuriously roll back a healthy service.
  • The recovery command now shell-quotes the repository and host values with shlex.quote.

The failed-bootout path retains its explicit final binding read because that path never enters the settled-health window.

Validation on this exact head:

  • 128 focused Board lifecycle tests passed, including regressions proving that a transient probe after settled replacement health cannot trigger rollback and that restart uses the settled-health terminal sample.
  • Ruff passed.
  • Privacy scan passed.
  • Branch diff checks passed.

This is writer evidence only. Fresh independent Codex and Claude exact-head audits remain required before merge.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 7ce9f78a09cc is no longer an ancestor of current head 89e1df404c7e; commits may have been dropped. Only current-head audit verdicts count.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — BLOCKED

Audited commit 89e1df404c7e05f4777b484777fdbb5d46b34ced against current main 052d49452605eaf668fb07f385673fe5e25b303f.

P0: 0 · P1: 0 · P2: 1

The two findings from the Claude audit of 9470a646e3f63cbce2161cf833dd3d3ebe73aa94 are fixed at this head. Replacement, restart, failed-write restoration, and rollback restoration all reuse the final complete binding sample from their bounded delayed-health window, so an extra transient probe cannot overturn a settled success. The recovery command shell-quotes its variable repository and host tokens, and adversarial tokenization checks pass.

P2 — a restored unloaded prior service is falsely reported unresolved

When the prior definition is readable but its launchd job was already unloaded, _previous_version_evidence() necessarily returns None because no old process is serving. If replacement then fails and rollback successfully restores and starts that exact prior definition, _rollback_and_reconcile() obtains a settled full binding with status: pass and no failing checks, but src/code_mower/board_service.py:2088-2094 unconditionally overwrites the result to state: unresolved, verified: false. The analogous failed-write restoration path does the same at src/code_mower/board_service.py:2358-2364.

I reproduced the replacement path with the existing FakeHost: install a healthy definition, boot its job out while keeping the definition, attempt restart --replace with a different repository, make the replacement bootstrap fail before loading, then let rollback restore and bootstrap the prior definition. The final host runs the exact prior definition and repository, and the settled rollback binding passes every check, but the payload returns rollback_failed, says rollback failed to reach a verified state, and emits a recovery command for the replacement.

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:

  • PYTHONPATH=src python3.13 -m unittest discover -s tests -p 'test_board_service.py' — 128 tests passed
  • rollback and failed-write adversarial probe-count checks — passed
  • recovery-command shell tokenization checks, including hostile synthetic values — passed
  • two adjacent Board lifecycle/data-contract tests — passed
  • uvx ruff check src/code_mower/board_service.py tests/test_board_service.py — passed
  • PYTHONPATH=src python3.13 scripts/privacy_scan.py — passed
  • docs lifecycle — 140 documents, zero problems
  • git diff --check origin/main...HEAD — passed
  • all current GitHub CI jobs — passed

The working tree was clean before and after review. I am leaving needs-codex-audit in place because this exact head remains blocked. Any subsequent commit requires a new exact-head audit.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 89e1df404c7e05f4777b484777fdbb5d46b34ced
Verdict: PASS
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 052d49452605eaf668fb07f385673fe5e25b303f

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 7ce9f78a09cc is no longer an ancestor of current head fad91dbd36ea; commits may have been dropped. Only current-head audit verdicts count.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — PASS

Audited commit fad91dbd36ea8808a3d318678e370ec5f488b76b against current main 4a6148cc94e0909d98cc20074c52118dcc270d72.

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 fab64dc1d34b3d1929d85635f08004d8c96d8c23 patch. The old and new binary diffs have the same SHA-256, ec47fdc1274cb43ffeaaec7fb77ba89a9743c097ba570aae116c94e3ec75a2a6, and git range-diff maps all seven commits one-for-one. The intervening main change from PR #1102 does not overlap this patch's three files.

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 rollback_failed and unresolved, including failed-write restoration. Older-version rollback, unreadable takeover, first-install port absence, ambiguous bootstrap, ambiguous kickstart/PID transition, and shell-quoted single recovery-command behavior remain correct.

Launchd is the only implemented managed-service backend. Unsupported platforms still fail closed before mutation.

Validation at this exact head:

  • PYTHONPATH=src python3.13 -m unittest discover -s tests -p test_board_service.py — 131 passed
  • uv run --with packaging python -m unittest discover -s tests -p test_board_qualification.py — 9 passed
  • independent loaded/unknown failed-write restoration checks — passed
  • hostile recovery-command shell-tokenization check — passed
  • uvx ruff check src/code_mower/board_service.py tests/test_board_service.py — passed
  • PYTHONPATH=src python3.13 scripts/privacy_scan.py — passed
  • docs lifecycle — 140 documents, zero problems
  • git diff --check 4a6148cc...HEAD — passed

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.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 9470a646e3f6 is no longer an ancestor of current head fad91dbd36ea; commits may have been dropped. Only current-head audit verdicts count.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: fad91dbd36ea8808a3d318678e370ec5f488b76b
Verdict: BLOCKED
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 4a6148cc94e0909d98cc20074c52118dcc270d72

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 89e1df404c7e is no longer an ancestor of current head fad91dbd36ea; commits may have been dropped. Only current-head audit verdicts count.

2 similar comments
@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 89e1df404c7e is no longer an ancestor of current head fad91dbd36ea; commits may have been dropped. Only current-head audit verdicts count.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 89e1df404c7e is no longer an ancestor of current head fad91dbd36ea; commits may have been dropped. Only current-head audit verdicts count.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 7ce9f78a09cc is no longer an ancestor of current head 446442d3c228; commits may have been dropped. Only current-head audit verdicts count.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head writer evidence for 446442d.

This head resolves the two substantive findings from the Claude audit of fad91db:

  • Ambiguous bootstrap and kickstart provider_detail now pass through the same recursive diagnostic redaction chokepoint with the provider definition path included in known_paths. Default text/JSON hides the path; show-local-paths deliberately restores it.
  • Ambiguous bootstrap and first-install rollback absence verification now use one guarded runtime-state reader. OSError and subprocess failures become JOB_UNKNOWN and reconciliation stays fail closed instead of raising or proving absence.
  • The adjacent _job_load_state helper now uses the same guarded reader, preserving its previous fail-closed semantics.

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:

  • 135 focused Board service tests passed, including four new provider-detail and runtime-state families.
  • 9 adjacent Board qualification tests passed.
  • Ruff passed.
  • Privacy scan passed.
  • Documentation lifecycle passed for 140 documents with zero problems.
  • Base-range and worktree diff hygiene passed.

Fresh independent Codex and Claude exact-head audits remain required. This is writer evidence only.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 9470a646e3f6 is no longer an ancestor of current head 446442d3c228; commits may have been dropped. Only current-head audit verdicts count.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 446442d3c22864dd88fb1da2325bb66bf593e668
Verdict: PASS
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 4a6148cc94e0909d98cc20074c52118dcc270d72

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Exact-head Codex audit — BLOCKED

Audited commit 446442d3c22864dd88fb1da2325bb66bf593e668 against current main 4a6148cc94e0909d98cc20074c52118dcc270d72.

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 --show-local-paths deliberately restores it. BINDING_CHECK_IDS also exists at src/code_mower/board_service.py:131 and is exercised by the focused binding-coverage test, so that reported finding is a false positive.

P1 — a persistent runtime-state failure still escapes reconciliation

_runtime_state_or_unknown() correctly guards the direct runtime reads, but the first-install rollback branch immediately calls provider.read_service() at src/code_mower/board_service.py:2160. For LaunchdProvider, a present definition makes read_service() enter with_runtime(), whose call to self.runtime_state(label) at line 811 is unguarded. If runtime-state reads remain unavailable after an ambiguous bootstrap, that second path raises and bypasses the unresolved result and recovery command.

I reproduced this with the existing FakeHost: a provider bootstraps the first-install definition, reports failure, and then raises PermissionError from every subsequent runtime_state() call. The initial guarded read returns JOB_UNKNOWN; rollback refuses to delete the still-unconfirmed job, as intended; then _rollback_and_reconcile() calls read_service() and the command exits with PermissionError: launchd state permanently unreadable. The replacement definition and loaded job remain on the host, but no operation payload or recovery instruction is emitted.

Please keep this entire reconciliation path guarded when runtime state remains unavailable, return rollback_failed with state: unresolved, retain the definition for discoverability, and add a regression where runtime_state() raises on every post-bootstrap read rather than only once.

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:

  • 135 focused Board service tests passed
  • 9 adjacent Board qualification tests passed
  • all six new targeted provider-detail/runtime-state tests passed
  • the executable BINDING_CHECK_IDS coverage test passed
  • persistent runtime-state adversarial reproducer failed with the uncaught PermissionError above
  • Ruff, privacy scan, docs lifecycle (140 documents, zero problems), and base-range diff hygiene passed

I am leaving needs-codex-audit in place. This verdict applies only to the immutable head above; any subsequent commit requires a new exact-head audit.

@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 89e1df404c7e is no longer an ancestor of current head 446442d3c228; commits may have been dropped. Only current-head audit verdicts count.

1 similar comment
@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 89e1df404c7e is no longer an ancestor of current head 446442d3c228; commits may have been dropped. Only current-head audit verdicts count.

@jeffhuber
jeffhuber enabled auto-merge (squash) September 21, 2026 17:33
@github-actions

Copy link
Copy Markdown

Code Mower notice: previously audited head 89e1df404c7e is no longer an ancestor of current head 446442d3c228; commits may have been dropped. Only current-head audit verdicts count.

@jeffhuber
jeffhuber merged commit 2b3e4a4 into main Sep 21, 2026
21 checks passed
@jeffhuber
jeffhuber deleted the codex/1082-board-atomic branch September 21, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.6.0: make managed Board replacement atomic and truthful

1 participant