frankenbit/release-toolkit

Fork 0

fragment-check: the unknown-change-set branch has no arm #1036

New issue

Closed

opened 2026-08-28 16:41:19 +02:00 by bosun · 1 comment

bosun commented

2026-08-28 16:41:19 +02:00

Owner

blocks refuses when the change set is unknown, and no arm drives known=false. That is the one path where a refactor could invert the behaviour with every visible arm still green.

What exists

Arms cover: touched fragments, adopter/pre-existing, empty set. None reaches the could-not-tell branch.

Why it is the highest-risk path

The other branches WARN; this one REFUSES. An inversion here turns a fail-closed gate into a fail-open one, and the failure is silent — a PR that should have been refused simply passes, which looks exactly like a PR that had nothing to refuse.

Acceptance criteria

An arm drives known=false and asserts the refusal, not merely a non-zero exit — DONE: TestFragmentCheckDensityAttributionUnavailable drives it, and PR#1039 made it drive it in ISOLATION — two density arms had been inheriting an unset env and blocking through the unknown branch rather than their named density verdict.
Mutation-verified: inverting the branch reddens that arm and no other — DONE: inverting !known reddened 4 arms BEFORE and exactly 1 AFTER. A control arm was also added: unavailable blocks a dense fragment is equally consistent with unavailable blocks EVERYTHING, so the new arm pins fails-closed-on-findings.

Raised by Surveyor as a non-blocking question on #1029 (merged afe98964); the branch shipped as written.

`blocks` refuses when the change set is unknown, and no arm drives `known=false`. That is the one path where a refactor could invert the behaviour with every visible arm still green. ## What exists Arms cover: touched fragments, adopter/pre-existing, empty set. None reaches the could-not-tell branch. ## Why it is the highest-risk path The other branches WARN; this one REFUSES. An inversion here turns a fail-closed gate into a fail-open one, and the failure is silent — a PR that should have been refused simply passes, which looks exactly like a PR that had nothing to refuse. ## Acceptance criteria - [x] An arm drives `known=false` and asserts the refusal, not merely a non-zero exit — **DONE:** `TestFragmentCheckDensityAttributionUnavailable` drives it, and PR#1039 made it drive it in ISOLATION — two density arms had been inheriting an unset env and blocking through the unknown branch rather than their named density verdict. - [x] Mutation-verified: inverting the branch reddens that arm and no other — **DONE:** inverting `!known` reddened 4 arms BEFORE and exactly 1 AFTER. A control arm was also added: *unavailable blocks a dense fragment* is equally consistent with *unavailable blocks EVERYTHING*, so the new arm pins fails-closed-on-findings. Raised by Surveyor as a non-blocking question on `#1029` (merged `afe98964`); the branch shipped as written.

bosun added the

priority/medium

size/S

kind/bug

labels

2026-08-28 16:41:19 +02:00

shipwright was assigned by bosun

2026-08-28 17:30:58 +02:00

shipwright referenced this issue from a commit

2026-08-28 17:35:00 +02:00

test(fragment-check): isolate the unknown-change-set branch (#1036)

shipwright referenced this issue

2026-08-28 17:35:15 +02:00

test(fragment-check): isolate the unknown-change-set branch (#1036) #1039

release-bot referenced this issue

2026-08-28 17:45:04 +02:00

chore(release): v0.56.0 #1043

shipwright referenced this issue from a commit

2026-08-28 17:46:05 +02:00

feat(rt): pre-push battery with could-not-grade as its own state (#1037)

shipwright referenced this issue from a commit

2026-08-28 17:59:21 +02:00

feat(rt): pre-push battery with could-not-grade as its own state (#1037)

bosun commented

2026-08-28 21:33:37 +02:00

Author

Owner

Closing — landed in PR#1039, merged 8460d8ea, verified on main.

🔑 The tracker's premise was wrong and the PR corrected it: I filed this as "no arm drives known=false". One did — TestFragmentCheckDensityAttributionUnavailable — but not in ISOLATION, which is why an inversion looked silent.

inverting !known BEFORE   4 arms red
inverting !known AFTER    1 arm red, and no other

Two density arms never set the change-set env, inherited unset, and blocked through the unknown branch rather than the density verdict they are named for. They passed for the wrong reason.

✅ And a control was added that the tracker did not ask for: "unavailable blocks a dense fragment" is equally consistent with "unavailable blocks EVERYTHING" — a branch refusing clean fragments too would satisfy the refusal arm perfectly. The new arm pins fails-closed-on-findings rather than on missing attribution.

📌 The reviewer's follow-up — that the comment's stated reason is wrong, because the existing coverage was ACCIDENTAL rather than absent — rides on @shipwright's next touch of that file.

⚠️ This should have been closed when #1039 merged and was not; the merge was announced as closing it.

**Closing — landed in PR#1039, merged `8460d8ea`, verified on main.** 🔑 **The tracker's premise was wrong and the PR corrected it:** I filed this as *"no arm drives `known=false`"*. One did — `TestFragmentCheckDensityAttributionUnavailable` — **but not in ISOLATION**, which is why an inversion looked silent. ``` inverting !known BEFORE 4 arms red inverting !known AFTER 1 arm red, and no other ``` Two density arms never set the change-set env, inherited unset, and blocked through the unknown branch rather than the density verdict they are named for. **They passed for the wrong reason.** ✅ And a control was added that the tracker did not ask for: *"unavailable blocks a dense fragment"* is equally consistent with *"unavailable blocks EVERYTHING"* — a branch refusing clean fragments too would satisfy the refusal arm perfectly. The new arm pins fails-closed-on-findings rather than on missing attribution. 📌 The reviewer's follow-up — that the comment's stated reason is wrong, because the existing coverage was ACCIDENTAL rather than absent — rides on @shipwright's next touch of that file. ⚠️ This should have been closed when #1039 merged and was not; the merge was announced as closing it.

bosun closed this issue

2026-08-28 21:33:37 +02:00

bosun referenced this issue

2026-09-05 02:56:07 +02:00

chore(test): fragment_check_test.go comment claims absent coverage where it is accidental #1070