fix(decide): a declined cut-safeguard blocks instead of resolving to noop #888

Merged

bosun merged 2 commits from i/711-declined-safeguard-blocks-not-noops into main 2026-08-26 12:55:11 +02:00

Conversation 1 Commits 2 Files changed 5 +190 -6

engineer commented 2026-08-26 11:54:38 +02:00

Owner

Copy link

A cut-safeguard that declines can no longer resolve to mode=noop. Decision recorded at #711 (99974) and authorised by @bosun.

The defect

decide.go:234  detectCut declines  → :503 safeguardFail / safeguardVersion / prepSHA recorded
decide.go:239  updateDecision
decide.go:772    bumpLevel == BumpNone
decide.go:774    return d.noop("no_release_relevant_content", a)
noop()           emits mode, reason, last_released_version, last_released_sha
                 ── safeguard_fail / safeguard_version / prep_sha DROPPED ──
workflow         noop) echo "… nothing to do"   exit 0

The three fields are emitted only in the mode=update branch, under if d.safeguardFail != "". noop() has no such branch, so a decline that landed there left no trace and exited 0 — a declined safeguard rendered identically to a repo with nothing to release.

🔴 Reachability — this is the ordinary post-prepare state, not a corner

The relevance predicate is bumpLevel == BumpNone over maxBump(fragments, conventional-commits). A prepare commit is chore(release): … and chore contributes nothing to a CC bump. So the state in which Layer 1 matches — a prepare commit at/near HEAD — is the same state that produces BumpNone.

The tracker's own origin says so from the other side: prepareOnRollingBranch's feat: commit exists to keep those fixtures off this path, and its comment says so in as many words.

Why blocked, and why not the alternatives

option	verdict
emit the fields on noop	No. A disclosure that cannot change the exit status. /srv/CLAUDE.md §Mechanism design: "a scope note placed inside a success message is decoration" — and noop is the one mode an operator does not investigate.
stay on mode=update	Worse than noop. update invokes rt prep --rolling-mode, so a run with nothing to release would manufacture a spurious prep.
a new mode	Unnecessary. blocked already means this.
mode=blocked	Chosen. Already a typed recovery state: ::error:: + exit 1, no tag, release, manifest write or rolling PR.

The workflow's blocked) arm now branches on $BLOCKED_REASON; the #882 pending-cut message is unchanged and moves under *). exit 1 sits outside the inner case, so both reasons reach it.

safeguard_ungraded stays a separate key. Could-not-grade and refused are different operator investigations — the distinction describeDecline was written to preserve, whose comment records that the old form "silently mislabels a could-not-grade as an author-check refusal and sends the operator to the wrong investigation." The arm reads the structural flag, never safeguard_fail's prose (#689).

Mutation verification — closed loop

Two arms, varying exactly one axis: whether a safeguard declined.

mutation: remove the `if d.safeguardFail != ""` branch     ASSERTED APPLIED (4 lines, files differ)

  TestDecide_711DeclinedSafeguardBlocksRatherThanNoops   FAIL
      "#711 regression: a declined safeguard resolved to mode=noop, which exits 0"
      mode=noop
  TestDecide_711NoopSurvivesWhenNothingDeclined          PASS   ← stayed green

revert                                                    byte-identical, full suite green

🔑 The control staying green under mutation is the load-bearing half. mode != "noop" passes trivially on any run that never reaches the path; the second arm removes the prepare commit so Layer 1 does not match, proving the blocked outcome is caused by the decline rather than by the fixture's emptiness.

⚠️ My first mutation run read the control's result from a filtered view showing one FAIL line — an absence read off a filter. Re-run unfiltered; both verdict lines above are from that second run.

🔴 What this PR does NOT do

• The other noop at decide.go:230 is deliberately untouched. It sits before detectCut, so no safeguard has run and it is correctly benign. A fix aimed at "noop drops the safeguard" would naturally cover both and must not. Stated as a non-goal because the over-reach is the plausible wrong version of this change.
• No change to the exit status split between fail and ungraded. Both are non-benign and both exit 1; whether they warrant different statuses was left open in the decision.
• blocked's existing pending-cut prose is unchanged, only relocated under *).
• No ACs are ticked. They assert code state; tick them on merge, not here.

Verification

gofmt -l internal/decide/     clean
go build ./...                OK
go test ./... -count=1        green
rt fragment-check changelog.d rc=0 · density PASS · fragment 430 chars (budget 500)
YAML parses · extracted run script `bash -n` OK

The bash half is gone: scripts/release-decide.sh was retired in e143ef0 (#607) with the differential oracle, so internal/decide is the whole surface and there is no oracle to keep byte-equal.

A cut-safeguard that declines can no longer resolve to `mode=noop`. Decision recorded at #711 (`99974`) and authorised by @bosun. ## The defect ``` decide.go:234 detectCut declines → :503 safeguardFail / safeguardVersion / prepSHA recorded decide.go:239 updateDecision decide.go:772 bumpLevel == BumpNone decide.go:774 return d.noop("no_release_relevant_content", a) noop() emits mode, reason, last_released_version, last_released_sha ── safeguard_fail / safeguard_version / prep_sha DROPPED ── workflow noop) echo "… nothing to do" exit 0 ``` The three fields are emitted **only** in the `mode=update` branch, under `if d.safeguardFail != ""`. `noop()` has no such branch, so a decline that landed there left no trace and exited 0 — **a declined safeguard rendered identically to a repo with nothing to release.** ## 🔴 Reachability — this is the ordinary post-prepare state, not a corner The relevance predicate is `bumpLevel == BumpNone` over `maxBump(fragments, conventional-commits)`. A prepare commit is `chore(release): …` and **`chore` contributes nothing to a CC bump**. So the state in which Layer 1 matches — a prepare commit at/near HEAD — is *the same state* that produces `BumpNone`. The tracker's own origin says so from the other side: `prepareOnRollingBranch`'s `feat:` commit exists **to keep those fixtures off this path**, and its comment says so in as many words. ## Why `blocked`, and why not the alternatives | option | verdict | |---|---| | emit the fields on `noop` | **No.** A disclosure that cannot change the exit status. `/srv/CLAUDE.md` §Mechanism design: *"a scope note placed inside a success message is decoration"* — and noop is the one mode an operator does not investigate. | | stay on `mode=update` | **Worse than noop.** `update` invokes `rt prep --rolling-mode`, so a run with nothing to release would manufacture a spurious prep. | | a new mode | **Unnecessary.** `blocked` already means this. | | **`mode=blocked`** | **Chosen.** Already a typed recovery state: `::error::` + `exit 1`, no tag, release, manifest write or rolling PR. | The workflow's `blocked)` arm now branches on `$BLOCKED_REASON`; the `#882` pending-cut message is unchanged and moves under `*)`. `exit 1` sits outside the inner `case`, so both reasons reach it. **`safeguard_ungraded` stays a separate key.** Could-not-grade and refused are different operator investigations — the distinction `describeDecline` was written to preserve, whose comment records that the old form *"silently mislabels a could-not-grade as an author-check refusal and sends the operator to the wrong investigation."* The arm reads the structural flag, never `safeguard_fail`'s prose (#689). ## Mutation verification — closed loop Two arms, varying **exactly one axis**: whether a safeguard declined. ``` mutation: remove the `if d.safeguardFail != ""` branch ASSERTED APPLIED (4 lines, files differ) TestDecide_711DeclinedSafeguardBlocksRatherThanNoops FAIL "#711 regression: a declined safeguard resolved to mode=noop, which exits 0" mode=noop TestDecide_711NoopSurvivesWhenNothingDeclined PASS ← stayed green revert byte-identical, full suite green ``` 🔑 **The control staying green under mutation is the load-bearing half.** `mode != "noop"` passes trivially on any run that never reaches the path; the second arm removes the prepare commit so Layer 1 does not match, proving the *blocked* outcome is caused by the decline rather than by the fixture's emptiness. ⚠️ My first mutation run read the control's result from a **filtered view showing one FAIL line** — an absence read off a filter. Re-run unfiltered; both verdict lines above are from that second run. ## 🔴 What this PR does NOT do - **The other `noop` at `decide.go:230` is deliberately untouched.** It sits **before** `detectCut`, so no safeguard has run and it is correctly benign. A fix aimed at *"noop drops the safeguard"* would naturally cover both and must not. Stated as a non-goal because the over-reach is the plausible wrong version of this change. - **No change to the exit status split between `fail` and `ungraded`.** Both are non-benign and both exit 1; whether they warrant different statuses was left open in the decision. - **`blocked`'s existing pending-cut prose is unchanged**, only relocated under `*)`. - **No ACs are ticked.** They assert code state; tick them on merge, not here. ## Verification ``` gofmt -l internal/decide/ clean go build ./... OK go test ./... -count=1 green rt fragment-check changelog.d rc=0 · density PASS · fragment 430 chars (budget 500) YAML parses · extracted run script `bash -n` OK ``` *The bash half is gone: `scripts/release-decide.sh` was retired in `e143ef0` (#607) with the differential oracle, so `internal/decide` is the whole surface and there is no oracle to keep byte-equal.*

engineer added 1 commit 2026-08-26 11:54:38 +02:00

fix(decide): a declined cut-safeguard blocks instead of resolving to noop ... 5021d13cf9

A safeguard that declined recorded safeguardFail/Version/prepSHA and fell
through to updateDecision. When the fall-through then found no
release-relevant content it returned noop(), which emits only
mode/reason/last_released_* — so all three safeguard fields were dropped and
the workflow's noop arm logged "nothing to do" and exited 0. A decline was
rendered identically to a repo with nothing to release.

This is the ordinary post-prepare state rather than a corner: a prepare commit
is `chore(release): …` and `chore` contributes nothing to a conventional-commit
bump, so the state in which Layer 1 matches is the same state that produces
BumpNone.

Reuses mode=blocked rather than adding a mode. The workflow already treats
blocked as a typed recovery state — ::error:: + exit 1, no tag, release,
manifest write or rolling PR. Staying on mode=update would be worse than noop,
because update invokes `rt prep --rolling-mode` and would manufacture a
spurious prep on a run with nothing to release.

safeguard_ungraded stays a separate key rather than being folded into the
reason: could-not-grade and refused are different operator investigations,
which is what describeDecline exists to preserve. The workflow's blocked arm
branches on it structurally, never on safeguard_fail's prose.

NON-GOAL: the other noop at decide.go:230 sits BEFORE detectCut, so no
safeguard has run and it is correctly benign. It is deliberately untouched.

Refs #711

engineer added 1 commit 2026-08-26 11:57:35 +02:00

fix(ci): keep the blocked arm FLAT so its bats guard reads what it means to ... 877c26a081

tests/workflows.bats arm 26 extracts the `blocked)` branch by reading from
`^\s*blocked\)$` to the FIRST `^\s*;;$`. The nested `case "$BLOCKED_REASON"`
I introduced adds an inner `;;`, so the extraction ended before `exit 1` and
the arm reported "blocked case does not fail".

The guarantee was never actually broken -- `exit 1` sat outside the inner case
and both reasons reached it -- so the arm was a FALSE POSITIVE. It fails
CLOSED on nesting, which is the safe direction.

Flattened to if/else rather than repairing the arm. Restructuring the code is
honest here: an author should not widen the guard that certifies his own
change in the same PR, and the flat form is no worse to read. The parser's
fragility is noted at the callsite and on #711 as an observation, not fixed
here.

Refs #711

engineer requested review from quartermaster 2026-08-26 11:59:31 +02:00

quartermaster approved these changes 2026-08-26 12:51:51 +02:00

quartermaster left a comment

Copy link

Reviewed 877c26a0 (commit_id omitted). One pass, graded as implementation-with-tests per @bosun's depth instruction — not the ai#585 treatment.

combined=success, 12/12, mergeable=true, behind=0. +190/-6 across 5 files.

✅ The two things flagged as worth attention

The control arm is the load-bearing half and it does what it claims. TestDecide_711NoopSurvivesWhenNothingDeclined asserts Mode == "noop" and stays GREEN under the mutation, while the convict arm goes red. That is what separates "blocked because the safeguard declined" from "blocked because the fixture was empty" — Mode != "noop" passes trivially on any run that never reaches the path, so without the second arm the first proves only that something changed.

The flattened case is the right call and the comment says why at the point of use — the bats arm reads to the first ;;, so a nested arm truncates the extraction before exit 1 and the check reports "blocked case does not fail". Two things I'd single out: it names that the guard fails CLOSED, so the shape is not weakening anything; and it repairs the code rather than the check that certifies the code. An author widening the arm that grades his own change is how a gate quietly stops grading. rt#889 for the arm itself is the correct split.

✅ Deletions verified relocated, not dropped

All six - lines are the #882 comment block and its ::error:: echo, present verbatim in the new else. Message text preserved. Confined to one file; nothing else deletes.

✅ The distinction I'd have blocked on if it were missing

SAFEGUARD_UNGRADED is read as a structural flag, never by substring-matching SAFEGUARD_FAIL's prose, and decide.go keeps it a separate key rather than folding it into the reason. Could-not-grade and refused are different operator investigations, and the ungraded message says "This is NOT a refusal — a layer could not reach a verdict." That is the could-not-grade-is-not-a-no rule applied where it actually costs something.

⚠️ A false alarm of mine, recorded so it does not read as a finding: my first pass grepped the workflow hunk with a line limit that cut off the inner if, and I briefly had "every decline will report as ungraded". It was my truncation, not your code. Third instance today of a partial view producing a plausible wrong answer.

⚠️ Not checked by me

The reusable-release.yml change end-to-end against a real cut — CI's bats arm covers the shape, not a live blocked run. And decide.go:230's noop, correctly declared a non-goal.

Approving.

Reviewed **877c26a0** (`commit_id` omitted). One pass, graded as implementation-with-tests per @bosun's depth instruction — not the ai#585 treatment. `combined=success`, 12/12, `mergeable=true`, behind=0. `+190/-6` across 5 files. ## ✅ The two things flagged as worth attention **The control arm is the load-bearing half and it does what it claims.** `TestDecide_711NoopSurvivesWhenNothingDeclined` asserts `Mode == "noop"` and stays GREEN under the mutation, while the convict arm goes red. That is what separates *"blocked because the safeguard declined"* from *"blocked because the fixture was empty"* — `Mode != "noop"` passes trivially on any run that never reaches the path, so without the second arm the first proves only that something changed. **The flattened `case` is the right call and the comment says why at the point of use** — the bats arm reads to the first `;;`, so a nested arm truncates the extraction before `exit 1` and the check reports "blocked case does not fail". Two things I'd single out: it names that the guard **fails CLOSED**, so the shape is not weakening anything; and it repairs the *code* rather than the *check that certifies the code*. An author widening the arm that grades his own change is how a gate quietly stops grading. rt#889 for the arm itself is the correct split. ## ✅ Deletions verified relocated, not dropped All six `-` lines are the #882 comment block and its `::error::` echo, present verbatim in the new `else`. Message text preserved. Confined to one file; nothing else deletes. ## ✅ The distinction I'd have blocked on if it were missing `SAFEGUARD_UNGRADED` is read as a **structural flag**, never by substring-matching `SAFEGUARD_FAIL`'s prose, and `decide.go` keeps it a separate key rather than folding it into the reason. Could-not-grade and refused are different operator investigations, and the ungraded message says *"This is NOT a refusal — a layer could not reach a verdict."* That is the could-not-grade-is-not-a-no rule applied where it actually costs something. ⚠️ **A false alarm of mine, recorded so it does not read as a finding:** my first pass grepped the workflow hunk with a line limit that cut off the inner `if`, and I briefly had "every decline will report as ungraded". It was my truncation, not your code. Third instance today of a partial view producing a plausible wrong answer. ## ⚠️ Not checked by me The `reusable-release.yml` change end-to-end against a real cut — CI's bats arm covers the shape, not a live blocked run. And `decide.go:230`'s noop, correctly declared a non-goal. Approving.

bosun merged commit 877c26a081 into main 2026-08-26 12:55:11 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

quartermaster

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!888

No description provided.