Keep Board rollback reconciliation fail closed

[jeffhuber]

PR #1100's exact-head Codex review found one persistent failure path after its merge: when every post-bootstrap runtime_state() query raises, first-install rollback reconciliation re-entered that query through read_service() and escaped with PermissionError instead of returning the promised unresolved result.

This follow-up uses the provider's fail-closed definition-existence probe during absence reconciliation, so an unreadable runtime remains JOB_UNKNOWN, the definition stays discoverable, and the command returns rollback_failed with recovery guidance. It adds a regression in which every post-bootstrap runtime query fails.

Validation:

• 136 Board service tests and 11 subtests pass
• 9 Board qualification tests and 51 subtests pass
• Ruff passes
• privacy scan passes
• documentation lifecycle passes for 140 documents with zero problems
• diff hygiene passes

Finding: #1100 (comment)

[gitar-bot]

Note

Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by October 1. Add seats for more headroom.
Learn more

Code Review ✅ No issues found

🟡 Medium risk · Rollback reconciliation now returns unresolved on persistent runtime-query failures.

Fixes a rollback reconciliation failure path where an unreadable runtime state would escape with PermissionError instead of returning the promised unresolved result. Now uses the provider's fail-closed probe during absence reconciliation so an unreadable runtime remains JOB_UNKNOWN and the command returns rollback_failed with recovery guidance. All tests pass with no issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[jeffhuber]

Exact-head writer evidence for ef848fa43df74a7123a1c9de91b664419da5e04e:

Persistent post-bootstrap runtime-state failures now remain fail closed during first-install rollback reconciliation. Absence proof uses the provider definition-existence probe rather than re-entering read_service() and its runtime query; the command returns rollback_failed, JOB_UNKNOWN, an unresolved reconciliation, and recovery guidance while retaining the definition for discoverability.

Validation:

• 136 Board service tests and 11 subtests passed
• 9 Board qualification tests and 51 subtests passed
• Ruff passed
• privacy scan passed
• documentation lifecycle passed for 140 documents with zero problems
• diff hygiene passed

Fresh independent Codex and Claude exact-head audits remain required.

[github-actions]

Codex audit (merge-authority lane)

Head SHA: ef848fa43df74a7123a1c9de91b664419da5e04e
Verdict: PASS
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 2b3e4a4632119b919701a962726ff5e428711257

[jeffhuber]

Exact-head Codex audit — PASS

Audited commit ef848fa43df74a7123a1c9de91b664419da5e04e against current main 2b3e4a4632119b919701a962726ff5e428711257.

P0: 0 · P1: 0 · P2: 0

No blocking findings remain in the whole patch.

The persistent runtime-state failure found after PR #1100 is resolved. First-install rollback reconciliation now obtains the guarded job state once and checks only whether the definition still exists; it no longer re-enters LaunchdProvider.read_service(), whose runtime attachment could raise on a second provider query. If every post-bootstrap runtime query remains unavailable, the job stays JOB_UNKNOWN, job absence remains unproved, the surviving definition remains discoverable, and the operation returns rollback_failed with state: unresolved plus the existing recovery instruction. It does not raise or falsely certify restored absence.

The new regression exercises the actual adversarial sequence: bootstrap registers the first-install job and reports an ambiguous failure, every later runtime_state() call raises PermissionError, rollback refuses destructive deletion while load state is unconfirmed, and reconciliation returns the fail-closed payload while retaining the definition. The definition-existence probe is the appropriate narrower authority here: LaunchdProvider.definition_exists() treats filesystem read errors as present, and providers without that capability default to present, so uncertainty cannot become absence.

Validation at this exact head:

• tests/test_board_service.py and tests/test_board_qualification.py — 145 passed, 62 subtests passed
• all Board test modules — 487 passed, 405 subtests passed
• Ruff on the changed source and test — passed
• privacy scan — passed
• documentation lifecycle — 140 documents, zero problems
• base-range diff hygiene — passed
• working tree — clean

This verdict applies only to the immutable head above; any subsequent commit requires a new exact-head audit.

[github-actions]

Claude audit (merge-authority lane)

Head SHA: ef848fa43df74a7123a1c9de91b664419da5e04e
Verdict: PASS
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 2b3e4a4632119b919701a962726ff5e428711257