bug(decide): the pending-cut message recommends a DESTRUCTIVE recovery on a shipped release, using evidence it already has #1457

New issue

Closed

opened 2026-09-07 21:41:49 +02:00 by bosun · 1 comment

bosun commented

2026-09-07 21:41:49 +02:00

Owner

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

The two messages, on the same tree

rt decide
  "...the follow-up cut was cancelled before tag/release/manifest-update;
   operator action required: complete the pending release + update the manifest,
   or run `rt recover-pending-cut` (#1130) to move the v0.62.4 section back
   under [Unreleased]..."
  ...while emitting:  pending_tag_exists=true   pending_release_published=true

rt prepared-uncut-check
  "🔴 DO NOT cut and DO NOT run `rt recover-pending-cut`. Cutting would re-cut a
   released version; recover-pending-cut would fold a RELEASED section back
   under [Unreleased]. Both are destructive against this state."

Same state, opposite advice, and the destructive one is what decide prints on every push to main until someone repairs it. On v0.62.4 that was four consecutive pushes.

It is not missing information — it is unused information

PendingCutError carries TagExists and ReleasePublished (internal/decide/pending.go:62-63) and BlockedDecision() emits both as machine-readable evidence (:102-103). Error() reads neither. It is one unconditional fmt.Sprintf, so the sentence "the follow-up cut was cancelled before tag/release/manifest-update" is printed even when the tag exists and the release published — which is the case where it is false and its advice is dangerous.

Three states, one message today

tag absent,  release absent      genuinely cancelled — recover-pending-cut is CORRECT
tag present, release PUBLISHED   SHIPPED — record it in the manifest; recover is DESTRUCTIVE
tag present, release not published   a third state, currently unaddressed

AC

Error() branches on TagExists / ReleasePublished rather than asserting one story
The published case does not name recover-pending-cut as an option at all
The published case names what DOES work, matching prepared-uncut-check's wording so two verbs stop disagreeing
An arm per state, since a single message passing three ways is what produced this

Anchor

@bosun, answering the operator's "make the error messages clearer" after #1444. The clearest improvement available is not more prose — it is one message that stops giving destructive advice about a state it can already distinguish.

`rt decide` tells the operator to run `rt recover-pending-cut` on a state where `prepared-uncut-check` says in capitals not to — and decide already holds the field that tells the two states apart. ## The two messages, on the same tree ``` rt decide "...the follow-up cut was cancelled before tag/release/manifest-update; operator action required: complete the pending release + update the manifest, or run `rt recover-pending-cut` (#1130) to move the v0.62.4 section back under [Unreleased]..." ...while emitting: pending_tag_exists=true pending_release_published=true rt prepared-uncut-check "🔴 DO NOT cut and DO NOT run `rt recover-pending-cut`. Cutting would re-cut a released version; recover-pending-cut would fold a RELEASED section back under [Unreleased]. Both are destructive against this state." ``` **Same state, opposite advice, and the destructive one is what `decide` prints on every push to main until someone repairs it.** On v0.62.4 that was four consecutive pushes. ## It is not missing information — it is unused information `PendingCutError` carries `TagExists` and `ReleasePublished` (`internal/decide/pending.go:62-63`) and `BlockedDecision()` emits both as machine-readable evidence (`:102-103`). **`Error()` reads neither.** It is one unconditional `fmt.Sprintf`, so the sentence *"the follow-up cut was cancelled before tag/release/manifest-update"* is printed even when the tag exists and the release published — which is the case where it is false and its advice is dangerous. ## Three states, one message today ``` tag absent, release absent genuinely cancelled — recover-pending-cut is CORRECT tag present, release PUBLISHED SHIPPED — record it in the manifest; recover is DESTRUCTIVE tag present, release not published a third state, currently unaddressed ``` ## AC - [x] `Error()` branches on `TagExists` / `ReleasePublished` rather than asserting one story - [x] The published case does not name `recover-pending-cut` as an option at all - [x] The published case names what DOES work, matching `prepared-uncut-check`'s wording so two verbs stop disagreeing - [x] An arm per state, since a single message passing three ways is what produced this ## Anchor @bosun, answering the operator's *"make the error messages clearer"* after `#1444`. The clearest improvement available is not more prose — it is one message that stops giving destructive advice about a state it can already distinguish.

bosun added the

priority/high

size/S

kind/bug

labels

2026-09-07 21:41:49 +02:00

engineer was assigned by pullings

2026-09-07 21:54:43 +02:00

engineer referenced this issue from a commit

2026-09-07 21:59:25 +02:00

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

engineer referenced this issue

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

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

bosun referenced this issue from a commit

2026-09-07 22:22:38 +02:00

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

release-bot referenced this issue

2026-09-07 22:24:29 +02:00

chore(release): v0.63.0 #1448

bosun commented

2026-09-07 22:26:00 +02:00

Author

Owner

Closing. #1459 merged at 2220b01b. Re-derived by @bosun against origin/main.

AC1  internal/decide/pending.go   `case e.ReleasePublished == EvidenceTrue:`      1
AC2  the published branch:  "DO NOT run `rt recover-pending-cut`"                 1
AC3  `EvidenceUnknown` handled as its OWN branch                                  3 sites
AC4  per-state arms, named for the state they pin:
       TestPendingCutError_PublishedReleaseNeverOffersTheDestructiveRecovery
       TestPendingCutError_UnknownEvidenceOffersNeitherRecovery
       TestPendingCutEvidence

AC4 is the one that matters and it is the strict form: an arm per state, each named for the state, rather than one arm asserting the message changed. A single message passing three ways is what produced this defect.

What this was

rt decide printed one unconditional fmt.Sprintf on three different states, and on the published one it recommended the operation prepared-uncut-check refuses in capitals. 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, telling the operator to run the one command that would have folded a released section back under [Unreleased].

📌 Filed after the operator asked for clearer error messages, and it is the reason the answer was not "add more prose": the clearest improvement available was one message that stops giving destructive advice about a state it can already distinguish. @engineer implemented; @lookout reviewed.

Closing. `#1459` merged at `2220b01b`. Re-derived by @bosun against `origin/main`. ``` AC1 internal/decide/pending.go `case e.ReleasePublished == EvidenceTrue:` 1 AC2 the published branch: "DO NOT run `rt recover-pending-cut`" 1 AC3 `EvidenceUnknown` handled as its OWN branch 3 sites AC4 per-state arms, named for the state they pin: TestPendingCutError_PublishedReleaseNeverOffersTheDestructiveRecovery TestPendingCutError_UnknownEvidenceOffersNeitherRecovery TestPendingCutEvidence ``` **AC4 is the one that matters and it is the strict form**: an arm per state, each named for the state, rather than one arm asserting the message changed. *A single message passing three ways is what produced this defect.* ## What this was `rt decide` printed **one unconditional `fmt.Sprintf`** on three different states, and on the published one it recommended the operation `prepared-uncut-check` refuses in capitals. **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**, telling the operator to run the one command that would have folded a released section back under `[Unreleased]`. 📌 **Filed after the operator asked for clearer error messages, and it is the reason the answer was not "add more prose":** the clearest improvement available was one message that stops giving destructive advice about a state it can already distinguish. @engineer implemented; @lookout reviewed.

bosun closed this issue

2026-09-07 22:26:00 +02:00