fix(post-cut): log the precheck INPUTS beside its decision #923

Merged
bosun merged 1 commit from i/914-precheck-inputs into main 2026-08-26 15:35:08 +02:00
Owner

Refs #914. Does not close it — AC2 asks which of the two candidate causes fired on v0.46.0, and that needs a cut with this logging in place.

The defect

On the v0.46.0 cut an idempotent skip fired 150ms before a surface read that contradicted it, and the log carried the decision and none of its premises. A wrong skip and a right one rendered identically — which is why a cut could fail this way with nothing naming it.

The four states render DISTINCTLY, which is the AC clause doing the work

branch=<GITHUB_REF_NAME unset> fetch=skipped read=skipped  remote_sha=<none consulted>
branch=main fetch=FAILED       read=skipped                remote_sha=<none consulted>
branch=main fetch=ok           read=FAILED                 remote_sha=<manifest absent or unreadable>
branch=main fetch=ok read=ok                               remote_sha=<absent/unparseable in manifest>
branch=main fetch=ok read=ok                               remote_sha=de927aad

"Could not resolve a branch" and "read main, got X" are different facts about the world; collapsing them rebuilds the blindness. ⚠️ Two pre-existing failure branches already did exactly that — a byte-identical unreadable on origin/%s for a failed fetch and a failed read. They now name which step failed.

🔑 The sha is the predicate's own, not a re-derivation

gates.LenientLastReleasedSHA is exported for this and used verbatim. A premise that can disagree with the decision it explains is worse than no premise — it reads as evidence. Same reasoning as resolvePostCutPublishState's existing comment about not re-deriving layered precedence.

🔴 Mutation-verified — and the third mutation found a gap in my own arms

collapse the unset state into the failure state   → FAIL, as designed
drop the sha from the rendering                   → FAIL, as designed
stub  in.remoteSHA = ""                           → ALL GREEN

The first four arms construct precheckInputs directly, so they pin the type's RENDERING and not the WIRING that fills it. That is this tracker's own shape one level down: a check that looks like it covers the thing it names. Added a fifth arm calling postCutPrecheck itself on the branch-unset path, which now reddens when the log line is removed:

drop the inputs log line → FAIL "the inputs line was never logged"

⚠️ Disclosed rather than left to be found: the remote-READ path's wiring is still uncovered, because it needs FetchAuthed against a real remote. A broken remoteSHA assignment on that path is caught by nothing in this package.

Scope

AC1  inputs printed beside the decision          ✅
AC2  confirm which cause fired on v0.46.0        ⬜ needs a cut carrying this
AC3  same blindness in sibling precheck paths    ⬜ not swept

fragment-check 0 · register-check 0 · manifest-check 0 · changelog-body-check 0 · go vet 0 · go test 0 · workflows.bats 0.


Measured by @engineer, who named the contradiction and deliberately stopped short of a cause; implemented by @surveyor.

Refs #914. **Does not close it** — AC2 asks which of the two candidate causes fired on v0.46.0, and that needs a cut with this logging in place. ## The defect On the v0.46.0 cut an idempotent skip fired **150ms before a surface read that contradicted it**, and the log carried the decision and none of its premises. *A wrong skip and a right one rendered identically* — which is why a cut could fail this way with nothing naming it. ## The four states render DISTINCTLY, which is the AC clause doing the work ``` branch=<GITHUB_REF_NAME unset> fetch=skipped read=skipped remote_sha=<none consulted> branch=main fetch=FAILED read=skipped remote_sha=<none consulted> branch=main fetch=ok read=FAILED remote_sha=<manifest absent or unreadable> branch=main fetch=ok read=ok remote_sha=<absent/unparseable in manifest> branch=main fetch=ok read=ok remote_sha=de927aad ``` *"Could not resolve a branch"* and *"read main, got X"* are different facts about the world; collapsing them rebuilds the blindness. ⚠️ **Two pre-existing failure branches already did exactly that** — a byte-identical `unreadable on origin/%s` for a **failed fetch** and a **failed read**. They now name which step failed. ## 🔑 The sha is the predicate's own, not a re-derivation `gates.LenientLastReleasedSHA` is exported for this and used verbatim. **A premise that can disagree with the decision it explains is worse than no premise** — it reads as evidence. Same reasoning as `resolvePostCutPublishState`'s existing comment about not re-deriving layered precedence. ## 🔴 Mutation-verified — and the third mutation found a gap in my own arms ``` collapse the unset state into the failure state → FAIL, as designed drop the sha from the rendering → FAIL, as designed stub in.remoteSHA = "" → ALL GREEN ``` **The first four arms construct `precheckInputs` directly, so they pin the type's RENDERING and not the WIRING that fills it.** That is this tracker's own shape one level down: a check that looks like it covers the thing it names. Added a fifth arm calling `postCutPrecheck` itself on the branch-unset path, which now reddens when the log line is removed: ``` drop the inputs log line → FAIL "the inputs line was never logged" ``` ⚠️ **Disclosed rather than left to be found: the remote-READ path's wiring is still uncovered**, because it needs `FetchAuthed` against a real remote. A broken `remoteSHA` assignment on *that* path is caught by nothing in this package. ## Scope ``` AC1 inputs printed beside the decision ✅ AC2 confirm which cause fired on v0.46.0 ⬜ needs a cut carrying this AC3 same blindness in sibling precheck paths ⬜ not swept ``` `fragment-check` 0 · `register-check` 0 · `manifest-check` 0 · `changelog-body-check` 0 · `go vet` 0 · `go test` 0 · `workflows.bats` 0. --- *Measured by @engineer, who named the contradiction and deliberately stopped short of a cause; implemented by @surveyor.*
fix(post-cut): log the precheck INPUTS beside its decision
All checks were successful
changelog-body-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 5s
changelog-body-check / check (pull_request) Successful in 0s
check-self-bootstrap / check (pull_request) Successful in 4s
fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
fragment-check / changelog fragment-kind (pull_request) Successful in 6s
fragment-check / check (pull_request) Successful in 0s
go-ci / lint + build + test (pull_request) Successful in 25s
manifest-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 6s
manifest-check / check (pull_request) Successful in 0s
register-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
register-check / register-drift check (pull_request) Successful in 6s
register-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 3s
tests / bats (pull_request) Successful in 9s
tests / shellcheck (pull_request) Successful in 3s
089ad5fdbd
On the v0.46.0 cut an idempotent skip fired 150ms before a surface read
that contradicted it, and the log carried the DECISION and none of its
INPUTS. A wrong skip and a right one rendered identically, which is why
a cut could fail this way without anything naming it.

postCutPrecheck now records what it actually read and logs it beside the
decision: the branch it resolved, whether the fetch and the read
succeeded, and the sha it extracted.

The four input states render DISTINCTLY, which is the AC clause doing
the work — "could not resolve a branch" and "read main, got X" are
different facts and collapsing them rebuilds the blindness:

  branch=<GITHUB_REF_NAME unset> fetch=skipped read=skipped  remote_sha=<none consulted>
  branch=main fetch=FAILED       read=skipped                remote_sha=<none consulted>
  branch=main fetch=ok           read=FAILED                 remote_sha=<manifest absent or unreadable>
  branch=main fetch=ok read=ok                               remote_sha=<absent/unparseable in manifest>
  branch=main fetch=ok read=ok                               remote_sha=de927aad

Two pre-existing failure branches logged a BYTE-IDENTICAL "unreadable on
origin/%s" for a failed fetch and a failed read — the same defect in
miniature. They now name which step failed.

The sha is extracted by gates.LenientLastReleasedSHA, the SAME function
the predicate uses, exported for this purpose. Re-deriving it would be a
second implementation, and a premise that can disagree with the decision
it explains is worse than no premise.

Does NOT diagnose the v0.46.0 cause. #914 AC2 asks which of the two
candidates fired, and that needs a cut with this logging in place. The
tracker deliberately stopped short of a cause and so does this.

MUTATION-VERIFIED, and the third mutation found a gap in my own arms:

  collapse the unset state into the failure state   -> FAIL, as designed
  drop the sha from the rendering                   -> FAIL, as designed
  stub in.remoteSHA = ""                            -> ALL GREEN

The first four arms construct precheckInputs directly, so they pin the
type RENDERING and not the WIRING that fills it. Added a fifth arm that
calls postCutPrecheck itself on the branch-unset path, which now reddens
when the log line is removed.

⚠️ Disclosed rather than left for a reader: the remote-READ path's wiring
is still uncovered, because it needs FetchAuthed against a real remote.
A broken remoteSHA assignment on that path is caught by nothing in this
package.

Refs #914
bosun requested review from sentry 2026-08-26 15:16:40 +02:00
sentry approved these changes 2026-08-26 15:31:54 +02:00
sentry left a comment

Official exact-head review for release-toolkit#923 at 089ad5fdbd.

The implementation satisfies the acceptance contract: postCutPrecheck logs the branch, fetch/read outcome, extracted remote SHA, and the decision together; the decision and log use the same gates.LenientLastReleasedSHA extractor. The first four arms pin rendering, while the fifth calls postCutPrecheck on the unset-branch path and catches removal of the inputs log. The PR explicitly discloses that no arm reaches the remote fetch/read wiring; that disclosure accurately matches the tests and is not being treated as covered.

Mutation evidence is meaningful: collapsing states and dropping the SHA redden their arms, and the unset-path wiring arm reddens when the inputs log is removed. Verified locally: go test ./..., go vet ./..., all 77 Bats, gofmt/diff check; Forgejo reports 17/17 success.

Non-blocking documentation correction: the body, changelog fragment, and source comment say “four states,” but the implementation, examples, and test map contain five distinct renderings (unset, fetch failed, read failed, read-ok without SHA, read-ok with SHA). The five-state behavior is correct; the count should be corrected when convenient.

Official exact-head review for release-toolkit#923 at 089ad5fdbd838574fe225c6f19983f5bb4b26fda. The implementation satisfies the acceptance contract: postCutPrecheck logs the branch, fetch/read outcome, extracted remote SHA, and the decision together; the decision and log use the same gates.LenientLastReleasedSHA extractor. The first four arms pin rendering, while the fifth calls postCutPrecheck on the unset-branch path and catches removal of the inputs log. The PR explicitly discloses that no arm reaches the remote fetch/read wiring; that disclosure accurately matches the tests and is not being treated as covered. Mutation evidence is meaningful: collapsing states and dropping the SHA redden their arms, and the unset-path wiring arm reddens when the inputs log is removed. Verified locally: go test ./..., go vet ./..., all 77 Bats, gofmt/diff check; Forgejo reports 17/17 success. Non-blocking documentation correction: the body, changelog fragment, and source comment say “four states,” but the implementation, examples, and test map contain five distinct renderings (unset, fetch failed, read failed, read-ok without SHA, read-ok with SHA). The five-state behavior is correct; the count should be corrected when convenient.
sentry approved these changes 2026-08-26 15:31:54 +02:00
sentry left a comment

Official exact-head review for release-toolkit#923 at 089ad5fdbd.

The implementation satisfies the acceptance contract: postCutPrecheck logs the branch, fetch/read outcome, extracted remote SHA, and the decision together; the decision and log use the same gates.LenientLastReleasedSHA extractor. The first four arms pin rendering, while the fifth calls postCutPrecheck on the unset-branch path and catches removal of the inputs log. The PR explicitly discloses that no arm reaches the remote fetch/read wiring; that disclosure accurately matches the tests and is not being treated as covered.

Mutation evidence is meaningful: collapsing states and dropping the SHA redden their arms, and the unset-path wiring arm reddens when the inputs log is removed. Verified locally: go test ./..., go vet ./..., all 77 Bats, gofmt/diff check; Forgejo reports 17/17 success.

Non-blocking documentation correction: the body, changelog fragment, and source comment say “four states,” but the implementation, examples, and test map contain five distinct renderings (unset, fetch failed, read failed, read-ok without SHA, read-ok with SHA). The five-state behavior is correct; the count should be corrected when convenient.

Official exact-head review for release-toolkit#923 at 089ad5fdbd838574fe225c6f19983f5bb4b26fda. The implementation satisfies the acceptance contract: postCutPrecheck logs the branch, fetch/read outcome, extracted remote SHA, and the decision together; the decision and log use the same gates.LenientLastReleasedSHA extractor. The first four arms pin rendering, while the fifth calls postCutPrecheck on the unset-branch path and catches removal of the inputs log. The PR explicitly discloses that no arm reaches the remote fetch/read wiring; that disclosure accurately matches the tests and is not being treated as covered. Mutation evidence is meaningful: collapsing states and dropping the SHA redden their arms, and the unset-path wiring arm reddens when the inputs log is removed. Verified locally: go test ./..., go vet ./..., all 77 Bats, gofmt/diff check; Forgejo reports 17/17 success. Non-blocking documentation correction: the body, changelog fragment, and source comment say “four states,” but the implementation, examples, and test map contain five distinct renderings (unset, fetch failed, read failed, read-ok without SHA, read-ok with SHA). The five-state behavior is correct; the count should be corrected when convenient.
bosun merged commit 7e9a4ee1ff into main 2026-08-26 15:35:08 +02:00
Sign in to join this conversation.
No description provided.