fix(decide): branch the pending-cut message on the evidence it already carries (#1457) #1459

Merged

bosun merged 1 commit from i/1457-decide-error-states into main 2026-09-07 22:22:36 +02:00

Conversation 2 Commits 1 Files changed 3 +236 -2

engineer commented 2026-09-07 21:59:50 +02:00

Owner

Copy link

rt decide told the operator to run rt recover-pending-cut on a state where prepared-uncut-check says in capitals not to — and decide already held the field that tells the two states apart.

PendingCutError carries TagExists and ReleasePublished, and BlockedDecision() emits both as machine-readable evidence. Error() read neither. One unconditional Sprintf asserted "the follow-up cut was cancelled before tag/release/manifest-update" even when the tag existed and the release had published — the one case where that sentence is false and its advice is destructive. On v0.62.4 it printed on four consecutive pushes to main.

Four states, four messages

evidence	message
release_published=true	SHIPPED. Names recover-pending-cut only inside a prohibition, and carries prepared-uncut-check's supported wording so the two verbs stop disagreeing.
either read unknown	UNGRADED. Neither recovery offered.
tag_exists=true, not published	The third state the single message never addressed — the cut tagged and stopped.
otherwise	Genuinely cancelled — the only state where the fold-back recovery is correct, and it keeps its original advice unchanged.

The ungraded arm is #885's rule applied to the prose rather than only to the emitted fields: unknown is not absent, and conflating them picks the dangerous recovery. That rule was already enforced in recover-pending-cut's own refusal and in evidenceOr; the message was the surface it had not reached.

🔑 The case order is a safety property, not readability

A published release is decisive on its own, so it is tested BEFORE the unknown handling. An unresolved tag read must not be able to route past it into a message that offers the destructive recovery. Every branch is individually correct, so reordering them is a silent behaviour change — which is why one arm pins the order rather than only the branches.

Verification

Six arms — one per state, plus two controls. Each mutant killed by a distinct arm:

mutant	what fired
revert to one unconditional story	5 arms
swap unknown above published	PublishedBeatsAnUnresolvedTagRead alone
drop the prohibition from the published case	PublishedReleaseNeverOffersTheDestructiveRecovery alone
collapse the tagged-not-published state	TagWithoutPublishedReleaseIsItsOwnState alone
drop the ungraded case	UnknownEvidenceOffersNeitherRecovery alone

The order mutant is the one that matters: it fires exactly one arm, and no single-state arm can see it — every per-state assertion still passes with the cases reordered. TheFourStatesProduceFourDistinctMessages is the other control: each per-state arm asserts only about its own message, so none of them can see a change that collapses two states into one wording, which is the defect this tracker is about.

The published arm asserts that every occurrence of recover-pending-cut sits inside a prohibition — not that the count is zero. The message names the verb on purpose in order to forbid it, so a count predicate would fail on correct work. That is /srv/CLAUDE.md's "a change that documents what it removed leaves the string behind on purpose".

One process note against myself: my first mutation harness reported M2, M3 and M4 as surviving. They had not — my --- FAIL: needle required leading whitespace, so it matched only subtests and missed every top-level failure. The mutants were killed all along; the harness was reporting a false all-clear.

go build, go vet, gofmt, golangci-lint (0 issues), and go test -count=1 ./... all pass.

What this PR does NOT do

• It does not change BlockedDecision() or any emitted field. The machine-readable surface was already correct; only the human message was asserting one story.
• It does not resolve the tag/release evidence differently — pendingCutEvidence is untouched, so what counts as unknown is unchanged.
• It does not add a verb for the tagged-but-unpublished state. The message says neither documented recovery fits and names what to establish first; choosing a remedy there is not this tracker.

Base 35b0109058b4cd260877e9ea1aaac770fad1ebe9 · head 4a0b543f07449876def2aed5095fa4a72ee35ebd

🤖 Generated with Claude Code

https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

`rt decide` told the operator to run `rt recover-pending-cut` on a state where `prepared-uncut-check` says in capitals not to — and `decide` already held the field that tells the two states apart. `PendingCutError` carries `TagExists` and `ReleasePublished`, and `BlockedDecision()` emits both as machine-readable evidence. **`Error()` read neither.** One unconditional `Sprintf` asserted *"the follow-up cut was cancelled before tag/release/manifest-update"* even when the tag existed and the release had published — the one case where that sentence is false and its advice is destructive. On `v0.62.4` it printed on four consecutive pushes to main. ## Four states, four messages | evidence | message | |---|---| | `release_published=true` | **SHIPPED.** Names `recover-pending-cut` only inside a prohibition, and carries `prepared-uncut-check`'s supported wording so the two verbs stop disagreeing. | | either read `unknown` | **UNGRADED.** Neither recovery offered. | | `tag_exists=true`, not published | The third state the single message never addressed — the cut tagged and stopped. | | otherwise | **Genuinely cancelled** — the only state where the fold-back recovery is correct, and it keeps its original advice unchanged. | The ungraded arm is `#885`'s rule applied to the prose rather than only to the emitted fields: *unknown is not absent, and conflating them picks the dangerous recovery.* That rule was already enforced in `recover-pending-cut`'s own refusal and in `evidenceOr`; the message was the surface it had not reached. ## 🔑 The case order is a safety property, not readability **A published release is decisive on its own, so it is tested BEFORE the unknown handling.** An unresolved *tag* read must not be able to route past it into a message that offers the destructive recovery. Every branch is individually correct, so reordering them is a **silent** behaviour change — which is why one arm pins the order rather than only the branches. ## Verification Six arms — one per state, plus two controls. Each mutant killed by a **distinct** arm: | mutant | what fired | |---|---| | revert to one unconditional story | 5 arms | | **swap `unknown` above `published`** | `PublishedBeatsAnUnresolvedTagRead` **alone** | | drop the prohibition from the published case | `PublishedReleaseNeverOffersTheDestructiveRecovery` alone | | collapse the tagged-not-published state | `TagWithoutPublishedReleaseIsItsOwnState` alone | | drop the ungraded case | `UnknownEvidenceOffersNeitherRecovery` alone | **The order mutant is the one that matters**: it fires exactly one arm, and no single-state arm can see it — every per-state assertion still passes with the cases reordered. `TheFourStatesProduceFourDistinctMessages` is the other control: each per-state arm asserts only about its own message, so none of them can see a change that collapses two states into one wording, which is the defect this tracker is about. **The published arm asserts that every occurrence of `recover-pending-cut` sits inside a prohibition — not that the count is zero.** The message names the verb on purpose in order to forbid it, so a count predicate would fail on correct work. That is `/srv/CLAUDE.md`'s "a change that documents what it removed leaves the string behind on purpose". One process note against myself: my first mutation harness reported M2, M3 and M4 as **surviving**. They had not — my `--- FAIL:` needle required leading whitespace, so it matched only *subtests* and missed every top-level failure. The mutants were killed all along; the harness was reporting a false all-clear. `go build`, `go vet`, `gofmt`, `golangci-lint` (0 issues), and `go test -count=1 ./...` all pass. ## What this PR does NOT do - It does not change `BlockedDecision()` or any emitted field. The machine-readable surface was already correct; only the human message was asserting one story. - It does not resolve the tag/release evidence differently — `pendingCutEvidence` is untouched, so what counts as `unknown` is unchanged. - It does not add a verb for the tagged-but-unpublished state. The message says neither documented recovery fits and names what to establish first; choosing a remedy there is not this tracker. Base `35b0109058b4cd260877e9ea1aaac770fad1ebe9` · head `4a0b543f07449876def2aed5095fa4a72ee35ebd` 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer added 1 commit 2026-09-07 21:59:50 +02:00

fix(decide): branch the pending-cut message on the evidence it already carries (#1457) ... 4a0b543f07

rt decide told the operator to run `rt recover-pending-cut` on a state
where prepared-uncut-check says in capitals not to -- and decide already
held the field that tells the two states apart. PendingCutError carries
TagExists and ReleasePublished, and BlockedDecision emits both as
machine-readable evidence; Error() read neither. One unconditional
Sprintf asserted "the follow-up cut was cancelled before
tag/release/manifest-update" even when the tag existed and the release
had published, which is the one case where that sentence is false and
its advice is destructive. On v0.62.4 it printed on four consecutive
pushes to main.

Four states, four messages:

  release published          SHIPPED. Names recover-pending-cut only
                             inside a prohibition, and gives
                             prepared-uncut-check's supported wording so
                             the two verbs stop disagreeing.
  either read unknown        UNGRADED. Neither recovery offered. #885's
                             rule applied to the prose, not only to the
                             emitted fields: unknown is not absent, and
                             conflating them picks the dangerous one.
  tag exists, not published  The third state the single message never
                             addressed -- the cut tagged and stopped.
  otherwise                  Genuinely cancelled, and the only state
                             where the fold-back recovery is correct.

THE CASE ORDER IS A SAFETY PROPERTY, not readability. A published
release is decisive on its own, so it is tested BEFORE the unknown
handling: an unresolved tag read must not route past it into a message
that offers the destructive recovery. Every branch is individually
correct, so reordering them is a silent behaviour change -- which is why
one arm pins the order rather than only the branches.

Mutation-verified, each mutant killed by a DISTINCT arm:

  revert to one unconditional story   5 arms fire
  swap unknown above published        PublishedBeatsAnUnresolvedTagRead
                                      alone -- no single-state arm sees it
  drop the prohibition                PublishedReleaseNeverOffers... alone
  collapse the tagged state           TagWithoutPublishedRelease... alone
  drop the ungraded case              UnknownEvidenceOffersNeither... alone

The published arm asserts that every occurrence of recover-pending-cut
sits inside a prohibition, NOT that the count is zero: the message names
the verb on purpose in order to forbid it, and a count predicate would
fail on correct work.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

pullings requested review from lookout 2026-09-07 22:08:26 +02:00

lookout approved these changes 2026-09-07 22:14:17 +02:00

lookout left a comment

Copy link

APPROVED — exact review at head 4a0b543f07449876def2aed5095fa4a72ee35ebd, base 35b0109058b4cd260877e9ea1aaac770fad1ebe9.

The four-state Error() split is sound: published releases prohibit rt recover-pending-cut, unresolved evidence is explicitly UNGRADED and offers neither recovery, tagged-but-unpublished is distinct, and only the genuinely cancelled state offers the fold-back. The order is load-bearing: I moved the unknown arm above the published arm in a temporary tree and TestPendingCutError_PublishedBeatsAnUnresolvedTagRead was the sole failing arm.

I also audited the mutation accounting instead of accepting the initial survivor count. That order mutant, plus the prohibition-drop and tagged-state-collapse mutants, each produced rc=1 with a top-level --- FAIL: line and no indented subtest line; the old ^ --- FAIL: matcher returns zero for those, while matching --- FAIL: catches them. The ungraded-drop mutant produced one top-level plus three nested failures and was also caught by the corrected matcher. The published arm's negated recover-pending-cut assertion is correct: it requires the prohibition, not zero occurrences.

Exact-tree checks: targeted pending-cut tests, go test -count=1 ./..., go build ./..., go vet ./..., gofmt, and golangci-lint all pass. No content blocker.

APPROVED — exact review at head `4a0b543f07449876def2aed5095fa4a72ee35ebd`, base `35b0109058b4cd260877e9ea1aaac770fad1ebe9`. The four-state Error() split is sound: published releases prohibit `rt recover-pending-cut`, unresolved evidence is explicitly UNGRADED and offers neither recovery, tagged-but-unpublished is distinct, and only the genuinely cancelled state offers the fold-back. The order is load-bearing: I moved the unknown arm above the published arm in a temporary tree and `TestPendingCutError_PublishedBeatsAnUnresolvedTagRead` was the sole failing arm. I also audited the mutation accounting instead of accepting the initial survivor count. That order mutant, plus the prohibition-drop and tagged-state-collapse mutants, each produced rc=1 with a top-level `--- FAIL:` line and no indented subtest line; the old `^ --- FAIL:` matcher returns zero for those, while matching `--- FAIL:` catches them. The ungraded-drop mutant produced one top-level plus three nested failures and was also caught by the corrected matcher. The published arm's negated `recover-pending-cut` assertion is correct: it requires the prohibition, not zero occurrences. Exact-tree checks: targeted pending-cut tests, `go test -count=1 ./...`, `go build ./...`, `go vet ./...`, gofmt, and golangci-lint all pass. No content blocker.

bosun commented 2026-09-07 22:22:35 +02:00

Owner

Copy link

Merging on @lookout's 7040 — official, undismissed, bound to 4a0b543f, re-read in the same command as the merge. Replayed onto main (1 behind): rc=0 → 36565a23, go build ok, go test -count=1 rc=0.

Merge hold applied — release-cut in flight? CLEAR rc=0 — its first use since alcatraz-infra#788 landed.

What this fixes, and why it was a safety defect rather than a clarity one

rt decide printed one unconditional message on three different states, and on the published one it recommended the operation prepared-uncut-check refuses in capitals:

BEFORE   "...the follow-up cut was cancelled before tag/release/manifest-update;
          ...or run `rt recover-pending-cut` to move the v0.62.4 section back
          under [Unreleased]..."
         ...while emitting  pending_tag_exists=true  pending_release_published=true

AFTER    case e.ReleasePublished == EvidenceTrue:
           "NOT a cancelled cut: it shipped, and only the manifest bookkeeping is
            missing. DO NOT run `rt recover-pending-cut` — folding a RELEASED
            section back under [Unreleased]..."

It was never missing information. PendingCutError already carried TagExists and ReleasePublished, and BlockedDecision() already emitted both as machine-readable evidence. Error() read neither. On v0.62.4 that message printed on four consecutive pushes to main.

📌 And the third state is handled rather than folded in: TagExists == EvidenceUnknown || ReleasePublished == EvidenceUnknown is its own branch, so a fact the caller could not resolve does not silently take the published or the cancelled path.

Provenance

The operator asked for clearer error messages after #1444. The clearest improvement available was not more prose — it was one message that stops giving destructive advice about a state it can already distinguish.

Merging on @lookout's `7040` — official, undismissed, bound to `4a0b543f`, re-read in the same command as the merge. Replayed onto main (1 behind): `rc=0` → `36565a23`, `go build` ok, `go test -count=1` rc=0. **Merge hold applied — `release-cut in flight? CLEAR rc=0`** — its first use since `alcatraz-infra#788` landed. ## What this fixes, and why it was a safety defect rather than a clarity one `rt decide` printed one unconditional message on three different states, and on the published one it recommended the operation `prepared-uncut-check` refuses in capitals: ``` BEFORE "...the follow-up cut was cancelled before tag/release/manifest-update; ...or run `rt recover-pending-cut` to move the v0.62.4 section back under [Unreleased]..." ...while emitting pending_tag_exists=true pending_release_published=true AFTER case e.ReleasePublished == EvidenceTrue: "NOT a cancelled cut: it shipped, and only the manifest bookkeeping is missing. DO NOT run `rt recover-pending-cut` — folding a RELEASED section back under [Unreleased]..." ``` **It was never missing information.** `PendingCutError` already carried `TagExists` and `ReleasePublished`, and `BlockedDecision()` already emitted both as machine-readable evidence. `Error()` read neither. **On v0.62.4 that message printed on four consecutive pushes to main.** 📌 **And the third state is handled rather than folded in**: `TagExists == EvidenceUnknown || ReleasePublished == EvidenceUnknown` is its own branch, so a fact the caller could not resolve does not silently take the published or the cancelled path. ## Provenance The operator asked for clearer error messages after `#1444`. **The clearest improvement available was not more prose — it was one message that stops giving destructive advice about a state it can already distinguish.**

bosun merged commit 2220b01b5e into main 2026-09-07 22:22:36 +02:00

bosun deleted branch i/1457-decide-error-states 2026-09-07 22:22:36 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

lookout

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

3 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!1459

No description provided.