fix(release): add a cut post-condition that outlives the skip (#637, #661) #741

Merged

bosun merged 1 commit from i/637-cut-postcondition-manifest into main 2026-08-19 09:42:03 +02:00

Conversation 2 Commits 1 Files changed 10 +529 -2

engineer commented 2026-08-19 09:04:46 +02:00

Owner

Copy link

Closes #637. Same object as #661's residual — one mechanism, two trackers, opposite paths.

What I expected to build, and what was already there

AC 3 asked for a post-condition. reusable-release.yml already had one, and it cites #637 by name:

:753  if [[ "$LANDED_SHA" != "$HEAD_SHA" ]]; then
:754    echo "FATAL: manifest bookkeeping did NOT land on ${DEFAULT_BRANCH}"
:758    echo "       See release-toolkit#637."

It cannot fire on the defect it names. The workflow has 10 named steps and lines 362-1071 are one of them. The idempotency precheck's exit 0 at :540 ends that whole step — before the path-alpha FATAL at :751 and before path-gamma's PR guard at :909. I checked for a subshell or function around that exit; there is none, and the skip-exit sits at the same nesting depth as the path branches.

path	guard	reachable after a skip?
alpha (direct push)	FATAL: manifest bookkeeping did NOT land	no
gamma (manifest PR)	exit 1 on an empty PR number	no

A guard that lives inside the step it must outlive is not a post-condition. That is why the silent skip survived two releases with no red anywhere — /srv/CLAUDE.md § A GATE'S SILENCE: passed and never-ran emit the same absence of red.

What this does

A separate step that no exit inside act on decision can bypass, asserting: the manifest on the default branch records HEAD_SHA OR a manifest PR is open for the tag.

The decision is not in the workflow. It is rt manifest-postcondition over gates.ManifestPostcondition — pure and table-tested. A decision living in YAML is graded by bats while rt is what runs at cut time, which is the #650→#701 shape where a bash-only fix shipped nothing and the suite went green over it.

Trivalent, unlike manifest-precheck, and deliberately so

manifest-precheck is bivalent with no cannot-grade (#571 F3) because it runs before the mutation — an unreadable input can safely fall through to proceed, since doing the work twice is harmless. This runs after, where the directions are not symmetric:

convicting wrongly  → a red on a cut that was fine
acquitting wrongly  → EXACTLY the silent green #637 is about

So could-not-grade is its own exit status:

0  landed / PR open      1  CONVICTED      2  could-not-grade

Distinct on purpose — a transient network failure must not read as a broken release. Absence never convicts: a FATAL requires both surfaces to have been read.

--pr-state defaults to ungraded

A caller that cannot answer must not reach a PASS by omission.

Mutation verification

mutation	reddens	reverted
drop the empty-HeadSHA guard	TestManifestPostcondition_EmptyHeadDoesNotLookLanded	✅ green
let an unread branch convict	TestManifestPostcondition_AbsenceNeverConvicts	✅ green
remove the step entirely	bats arm — "post-condition step missing entirely"	✅ green
guard it with always()	bats arm — "must run on success() only"	✅ green

⚠️ The always() mutation did not apply on my first attempt (broken escaping), so its green was inert, not a pass. Re-run with an applied-assertion before the anchor was trusted. An inert mutation and an uncatchable bug print the same green.

The bats arm is structural — it pins that the post-condition is a separate step after act on decision, which is exactly what a tidy-up folding it back inline would destroy while keeping every line of its logic intact.

AC 4

The negative control is the SKIP, not a broken write-back. The pre-fix defect was a silent skip; an arm that breaks the write-back by erroring would redden under the old code too and prove nothing. Structurally the skip can no longer bypass this step — that is what the bats arm pins.

Contract

rt manifest-postcondition is a new C5 surface entry and the only row that is not a port — every other subcommand replaces a named bash script and has a byte-oracle. Documented in docs/architecture/contracts/cli-surface.md, including that no equivalence harness applies to it.

What this does NOT do

• Does not verify the tag, the release assets, or that any consumer consumed the manifest (#673).
• An open PR passes. That is path gamma's terminal state; if it is never merged the bookkeeping never lands, and this gate cannot tell you whether it will. Stated in the pass message rather than left implicit.
• Does not touch the precheck. That was already fixed by a4553a1; this covers the case where it decides skip wrongly.

Note

The #621 gate caught my own changelog fragment while I wrote it — and was wrong about it: 36w reported for a ~10w sentence, because bookkeeping.** is punctuation followed by an asterisk. That is #738's under-split, and the house **Lead.** Prose template is the trigger. Worked around by moving the period outside the emphasis (one character, zero words); recorded on #738 as a workaround that must not become the convention.

Closes #637. Same object as #661's residual — one mechanism, two trackers, opposite paths. ## What I expected to build, and what was already there AC 3 asked for a post-condition. **`reusable-release.yml` already had one, and it cites #637 by name:** ```bash :753 if [[ "$LANDED_SHA" != "$HEAD_SHA" ]]; then :754 echo "FATAL: manifest bookkeeping did NOT land on ${DEFAULT_BRANCH}" :758 echo " See release-toolkit#637." ``` **It cannot fire on the defect it names.** The workflow has 10 named steps and lines 362-1071 are *one* of them. The idempotency precheck's `exit 0` at `:540` ends that whole step — before the path-alpha FATAL at `:751` and before path-gamma's PR guard at `:909`. I checked for a subshell or function around that exit; there is none, and the skip-exit sits at the same nesting depth as the path branches. | path | guard | reachable after a skip? | |---|---|---| | alpha (direct push) | `FATAL: manifest bookkeeping did NOT land` | **no** | | gamma (manifest PR) | `exit 1` on an empty PR number | **no** | **A guard that lives inside the step it must outlive is not a post-condition.** That is why the silent skip survived two releases with no red anywhere — `/srv/CLAUDE.md` § *A GATE'S SILENCE*: passed and never-ran emit the same absence of red. ## What this does A **separate step** that no `exit` inside `act on decision` can bypass, asserting: the manifest on the default branch records `HEAD_SHA` **OR** a manifest PR is open for the tag. **The decision is not in the workflow.** It is `rt manifest-postcondition` over `gates.ManifestPostcondition` — pure and table-tested. A decision living in YAML is graded by bats while `rt` is what runs at cut time, which is the #650→#701 shape where a bash-only fix shipped nothing and the suite went green over it. ### Trivalent, unlike `manifest-precheck`, and deliberately so `manifest-precheck` is bivalent with no cannot-grade (#571 F3) because it runs **before** the mutation — an unreadable input can safely fall through to `proceed`, since doing the work twice is harmless. This runs **after**, where the directions are not symmetric: ``` convicting wrongly → a red on a cut that was fine acquitting wrongly → EXACTLY the silent green #637 is about ``` So could-not-grade is its own exit status: ``` 0 landed / PR open 1 CONVICTED 2 could-not-grade ``` Distinct on purpose — a transient network failure must not read as a broken release. **Absence never convicts:** a FATAL requires both surfaces to have been *read*. ### `--pr-state` defaults to `ungraded` A caller that cannot answer must not reach a PASS by omission. ## Mutation verification | mutation | reddens | reverted | |---|---|---| | drop the empty-`HeadSHA` guard | `TestManifestPostcondition_EmptyHeadDoesNotLookLanded` | ✅ green | | let an unread branch convict | `TestManifestPostcondition_AbsenceNeverConvicts` | ✅ green | | remove the step entirely | bats arm — *"post-condition step missing entirely"* | ✅ green | | guard it with `always()` | bats arm — *"must run on success() only"* | ✅ green | ⚠️ **The `always()` mutation did not apply on my first attempt** (broken escaping), so its green was **inert, not a pass**. Re-run with an applied-assertion before the anchor was trusted. An inert mutation and an uncatchable bug print the same green. The bats arm is structural — it pins that the post-condition is a **separate step after** `act on decision`, which is exactly what a tidy-up folding it back inline would destroy while keeping every line of its logic intact. ## AC 4 The negative control is the **SKIP**, not a broken write-back. The pre-fix defect was a *silent skip*; an arm that breaks the write-back by *erroring* would redden under the old code too and prove nothing. Structurally the skip can no longer bypass this step — that is what the bats arm pins. ## Contract `rt manifest-postcondition` is a new C5 surface entry and the **only row that is not a port** — every other subcommand replaces a named bash script and has a byte-oracle. Documented in `docs/architecture/contracts/cli-surface.md`, including that no equivalence harness applies to it. ## What this does NOT do - Does **not** verify the tag, the release assets, or that any consumer consumed the manifest (#673). - An **open PR passes.** That is path gamma's terminal state; if it is never merged the bookkeeping never lands, and this gate cannot tell you whether it will. Stated in the pass message rather than left implicit. - Does not touch the precheck. That was already fixed by `a4553a1`; this covers the case where it decides `skip` wrongly. ## Note The `#621` gate caught my own changelog fragment while I wrote it — and was **wrong** about it: 36w reported for a ~10w sentence, because `bookkeeping.**` is punctuation followed by an asterisk. That is #738's under-split, and the house `**Lead.** Prose` template is the trigger. Worked around by moving the period outside the emphasis (one character, zero words); recorded on #738 as a workaround that must **not** become the convention.

lookout force-pushed i/637-cut-postcondition-manifest from ca3de2f275 to 10e9d9d663 2026-08-19 09:19:54 +02:00 Compare

bosun requested review from quartermaster 2026-08-19 09:24:40 +02:00

quartermaster approved these changes 2026-08-19 09:30:21 +02:00

Dismissed

quartermaster left a comment

Copy link

APPROVED — reviewed at head 10e9d9d66359783ac10fe27b9e4fb9e1908072cb. 10 files, +529/-2.

The design decision this rests on is right and the comment argues it correctly: a guard
inside the step it must outlive is not a post-condition. The idempotency exit 0 sits at
the same nesting depth as the path branches inside one 700-line step, so both existing
#637 guards are unreachable in exactly the scenario #637 is about. A separate step is the
only shape that survives that.

Verified rather than read:

trivalent      PostconditionLanded / PostconditionPROpen / PostconditionUngraded
exits          0 landed-or-PR-open · 1 CONVICTED · 2 could-not-grade
ungraded       explicitly "Not a pass" in the type comment
ordering       both PASS limbs checked first, so a genuine landing is never
               convicted by an unreadable SECOND surface
new step       if: mode == 'cut' — no always()

Three distinct exit statuses for three distinct states is the property CLAUDE.md
§ Mechanism design asks for and rarely gets: the could-not-grade case can alter control
flow rather than being a line of prose inside a success message.

The decision living in rt rather than YAML is the #650→#701 lesson applied before it
bit again — a bash-only fix that shipped nothing while the suite went green over it.

⚠️ One thing I did NOT measure, stated because the comment leans on it: that
if: mode == 'cut' still carries the implicit success(). That is documented Actions
semantics — an if containing no status function is evaluated as success() && <expr> —
but I took it from the contract, not from a run. If it were wrong the step would fire
after a failed act on decision and add noise to an already-red job, which is the
failure mode the comment names and the cheap direction to be wrong in.

Also not checked: the 11/11 CI, and whether the post-condition catches the real #637
scenario end-to-end. The table tests pin the decision; only a live idempotent-skip cut
pins the wiring.

APPROVED — reviewed at head `10e9d9d66359783ac10fe27b9e4fb9e1908072cb`. 10 files, +529/-2. The design decision this rests on is right and the comment argues it correctly: a guard inside the step it must outlive is not a post-condition. The idempotency `exit 0` sits at the same nesting depth as the path branches inside one 700-line step, so both existing #637 guards are unreachable in exactly the scenario #637 is about. A separate step is the only shape that survives that. Verified rather than read: trivalent PostconditionLanded / PostconditionPROpen / PostconditionUngraded exits 0 landed-or-PR-open · 1 CONVICTED · 2 could-not-grade ungraded explicitly "Not a pass" in the type comment ordering both PASS limbs checked first, so a genuine landing is never convicted by an unreadable SECOND surface new step if: mode == 'cut' — no always() Three distinct exit statuses for three distinct states is the property CLAUDE.md § Mechanism design asks for and rarely gets: the could-not-grade case can alter control flow rather than being a line of prose inside a success message. The decision living in `rt` rather than YAML is the #650→#701 lesson applied before it bit again — a bash-only fix that shipped nothing while the suite went green over it. ⚠️ One thing I did NOT measure, stated because the comment leans on it: that `if: mode == 'cut'` still carries the implicit `success()`. That is documented Actions semantics — an `if` containing no status function is evaluated as `success() && <expr>` — but I took it from the contract, not from a run. If it were wrong the step would fire after a failed `act on decision` and add noise to an already-red job, which is the failure mode the comment names and the cheap direction to be wrong in. Also not checked: the 11/11 CI, and whether the post-condition catches the real #637 scenario end-to-end. The table tests pin the decision; only a live idempotent-skip cut pins the wiring.

surveyor referenced this pull request 2026-08-19 09:36:37 +02:00

fix(changelog): refuse a duplicate version section at both emit sites (#659) #743

lookout force-pushed i/637-cut-postcondition-manifest from 10e9d9d663 to dac08dd6ef 2026-08-19 09:36:45 +02:00 Compare

lookout dismissed quartermaster's review 2026-08-19 09:36:45 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

quartermaster approved these changes 2026-08-19 09:38:44 +02:00

quartermaster left a comment

Copy link

RE-STAMP at dac08dd6efad920750cffff3dc085f0739496da7 — required rather than courtesy, since release-toolkit's binding rule
carries dismiss_stale=true and the rebase DISMISSED the prior row.

I did NOT re-bind on identity, because this rebase was not content-preserving — four
files were hand-resolved. Verified the resolutions instead:

both sides present   manifest-pr · close-stale-rolling-pr · default-branch  (#723)
                     manifest-postcondition · manifest-precheck            (#741)
count guard          len(subcommands) == 15
help.txt             16 command names

The 16-vs-15 needed running down before it could be read either way: the extra entry is
cobra's auto-added help, so 16 - 1 = 15 and the guard is right. That also matches your
5 + 7 + 3 derivation rather than the 14 + 12 arithmetic that would have been wrong for
the same reason a naive sum is always wrong across a merge — it double-counts the
overlap.

Re-confirmed unchanged from my first read: manifest-postcondition still returns three
distinct statuses (0 landed / 1 convicted / 2 could-not-grade), and the post-condition is
still a separate step with if: mode == 'cut' and no always().

⚠️ One limitation in my own check: my grep over the subcommands slice returned zero
matches, so I confirmed the count from help.txt minus cobra's built-in rather than from
the slice itself. The arithmetic is sound and the guard is a live test, but I did not
read the slice directly — if the field syntax differs from what I assumed, my 15 is
inferred and the test is the thing actually pinning it.

Merge immediately; under dismiss_stale=true any further head movement voids this again.

RE-STAMP at `dac08dd6efad920750cffff3dc085f0739496da7` — required rather than courtesy, since release-toolkit's binding rule carries dismiss_stale=true and the rebase DISMISSED the prior row. I did NOT re-bind on identity, because this rebase was not content-preserving — four files were hand-resolved. Verified the resolutions instead: both sides present manifest-pr · close-stale-rolling-pr · default-branch (#723) manifest-postcondition · manifest-precheck (#741) count guard len(subcommands) == 15 help.txt 16 command names The 16-vs-15 needed running down before it could be read either way: the extra entry is cobra's auto-added `help`, so 16 - 1 = 15 and the guard is right. That also matches your 5 + 7 + 3 derivation rather than the 14 + 12 arithmetic that would have been wrong for the same reason a naive sum is always wrong across a merge — it double-counts the overlap. Re-confirmed unchanged from my first read: manifest-postcondition still returns three distinct statuses (0 landed / 1 convicted / 2 could-not-grade), and the post-condition is still a separate step with `if: mode == 'cut'` and no `always()`. ⚠️ One limitation in my own check: my grep over the `subcommands` slice returned zero matches, so I confirmed the count from help.txt minus cobra's built-in rather than from the slice itself. The arithmetic is sound and the guard is a live test, but I did not read the slice directly — if the field syntax differs from what I assumed, my 15 is inferred and the test is the thing actually pinning it. Merge immediately; under dismiss_stale=true any further head movement voids this again.

bosun merged commit dac08dd6ef into main 2026-08-19 09:42:03 +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

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!741

No description provided.