fix(manifest-check): §4 goes vacuous for a whole prerelease series (#476 site 4b) #614

Merged

bosun merged 1 commit from i/476-manifest-check-prerelease-lasttag into main 2026-07-30 19:18:54 +02:00

Conversation 1 Commits 1 Files changed 10 +186

engineer commented 2026-07-30 19:08:38 +02:00

Owner

Copy link

Site 4b of 4 for #476. Fixes a gate that passes while guarding nothing.

The defect, measured

manifest-check §4 discovers the last tag with --exclude '*-*' so a closer -rc tag cannot falsely trip "manifest BEHIND tag" (#66). Correct for a stable manifest — and silently wrong for a prerelease one, because it makes every tag in a v1.0.0-alpha.N series invisible to the check.

On the fixture this PR adds (manifest 0.2.0-alpha.1, tags v0.1.0 and a newer v0.2.0-alpha.2):

before:  OK:   manifest 0.2.0-alpha.1 is ahead of tag v0.1.0            exit 0
after:   FAIL: manifest 0.2.0-alpha.1 is BEHIND tag v0.2.0-alpha.2      exit 1

The manifest was behind a real tag and the gate reported "ahead" and passed. That is not a missed detection — it is an affirmative wrong answer. For an entire alpha→beta→rc run §4 cannot report drift at all: green for a reason unrelated to the property it exists to check.

This is the class /srv/CLAUDE.md's reflex table calls the vacuous pass — the failure mode looks like health — and it would be live during exactly the phase we cut most often and most experimentally.

The fix, and why there is no flag

When the manifest is itself a prerelease, §4 re-reads the newest tag including prereleases.

The mode is derived from the manifest's own version, not configured. #476's body framed this as "include prerelease tags in prerelease-cut mode", implying a mode to switch on. It doesn't need one: the manifest already says whether we are in a series. Deriving it means there is no flag to set, no flag to forget, and no way for configuration and manifest to disagree — the caller loses the ability to be inconsistent rather than being trusted not to be. Same shape as PR#610's refuse-don't-work-around.

#66's stable-manifest behaviour is untouched. The two cases are mirrors and cannot both be wrong at once, so the bats suite now pins them side by side:

• #66 — a stable manifest must NOT be compared against a closer prerelease tag.
• #476 — a prerelease manifest MUST be.

Both sides, and why

scripts/manifest-check.sh is what actually runs at the PR-CI callsite (rt manifest-check exists but is not wired — that cutover is #607). So a Go-only change here would alter nothing in production.

That was my original framing and it was incomplete: cmd/rt/manifest_check.go is pinned to the script by manifest_check_equiv_test.go + manifest-check-oracle.sh, so a bash-only change would break the differential. The harness is the coupling, independent of what production invokes — the same lesson site 1 taught, which I under-applied one tracker later.

Coverage

• Equivalence case — new fixture (manifest-check-prerelease) + git-bootstrap wrapper (manifest-check-prerelease-oracle.sh) building the stable-then-newer-prerelease tag topology. Its purpose is that it FAILS: it flips a false green into a true red, so it cannot pass vacuously.
• Two bats cases — the BEHIND case (exit 1, names v1.0.0-alpha.2, asserts the old "ahead of tag v0.9.0" string is absent) and the matching-tag case.

Mutation verification (closed loop)

Restoring the pre-fix behaviour (if false && [[ "$cv_core" == *-* ]]):

--- FAIL: TestManifestCheckEquivalence/§4_prerelease_series_(manifest_behind_alpha_tag,_exit_1)

and the direct run under the mutation reproduces the false pass quoted at the top (OK: … ahead of tag v0.1.0, exit 0). Reverted by re-edit; gate re-run after.

Gate

Rebased onto 90eff16 (site 1's merge) and re-gated after the rebase: gofmt clean · go build ./... 0 · go vet ./... 0 · golangci-lint 0 · go test ./... -count=1 0 (19 packages) · bats tests/*.bats 797 ok.

shellcheck run the way CI runs it — find scripts … | xargs -0 shellcheck --severity=warning per tests.yml:79-87 — exit 0. Worth noting: a bare shellcheck scripts/manifest-check.sh exits 1 on clean main too, from pre-existing SC1091 source-following info. Gating on the bare invocation would chase non-gates.

Known non-regression: changelog-body-check em-dash case fails only under LC_ALL=C — pre-existing, reproduces on clean main, filed as #611. Untouched by this branch.

What this PR does NOT do

• Does not wire rt manifest-check. The bash script remains the live PR-CI gate; the cutover is #607. This fixes the gate that actually runs and keeps its Go twin in step.
• Does not touch site 4a (release-prep.sh LAST_TAG). That read feeds the CC-walk window, a different purpose with a different correct answer — separate change.
• Leaves #612 open and untouched. That defect is unrelated to this path.
• Adds no config surface, deliberately — see above.

Refs #476

**Site 4b of 4 for #476.** Fixes a gate that passes while guarding nothing. ## The defect, measured `manifest-check` §4 discovers the last tag with `--exclude '*-*'` so a closer `-rc` tag cannot falsely trip "manifest BEHIND tag" (#66). Correct for a **stable** manifest — and silently wrong for a **prerelease** one, because it makes every tag in a `v1.0.0-alpha.N` series invisible to the check. On the fixture this PR adds (manifest `0.2.0-alpha.1`, tags `v0.1.0` and a newer `v0.2.0-alpha.2`): ``` before: OK: manifest 0.2.0-alpha.1 is ahead of tag v0.1.0 exit 0 after: FAIL: manifest 0.2.0-alpha.1 is BEHIND tag v0.2.0-alpha.2 exit 1 ``` The manifest was behind a real tag and the gate reported **"ahead"** and passed. That is not a missed detection — it is an affirmative wrong answer. For an entire alpha→beta→rc run §4 cannot report drift at all: green for a reason unrelated to the property it exists to check. This is the class `/srv/CLAUDE.md`'s reflex table calls the **vacuous pass** — *the failure mode looks like health* — and it would be live during exactly the phase we cut most often and most experimentally. ## The fix, and why there is no flag When the manifest is itself a prerelease, §4 re-reads the newest tag **including** prereleases. **The mode is derived from the manifest's own version, not configured.** #476's body framed this as "include prerelease tags in prerelease-cut mode", implying a mode to switch on. It doesn't need one: the manifest already says whether we are in a series. Deriving it means there is no flag to set, no flag to forget, and **no way for configuration and manifest to disagree** — the caller loses the ability to be inconsistent rather than being trusted not to be. Same shape as PR#610's refuse-don't-work-around. #66's stable-manifest behaviour is untouched. The two cases are mirrors and cannot both be wrong at once, so the bats suite now pins them side by side: - **#66** — a *stable* manifest must NOT be compared against a closer prerelease tag. - **#476** — a *prerelease* manifest MUST be. ## Both sides, and why `scripts/manifest-check.sh` is what actually runs at the PR-CI callsite (`rt manifest-check` exists but is **not wired** — that cutover is #607). So a Go-only change here would alter nothing in production. That was my original framing and it was **incomplete**: `cmd/rt/manifest_check.go` is pinned to the script by `manifest_check_equiv_test.go` + `manifest-check-oracle.sh`, so a **bash-only change would break the differential**. The harness is the coupling, independent of what production invokes — the same lesson site 1 taught, which I under-applied one tracker later. ## Coverage - **Equivalence case** — new fixture (`manifest-check-prerelease`) + git-bootstrap wrapper (`manifest-check-prerelease-oracle.sh`) building the stable-then-newer-prerelease tag topology. **Its purpose is that it FAILS**: it flips a false green into a true red, so it cannot pass vacuously. - **Two bats cases** — the BEHIND case (exit 1, names `v1.0.0-alpha.2`, asserts the old "ahead of tag v0.9.0" string is *absent*) and the matching-tag case. ### Mutation verification (closed loop) Restoring the pre-fix behaviour (`if false && [[ "$cv_core" == *-* ]]`): ``` --- FAIL: TestManifestCheckEquivalence/§4_prerelease_series_(manifest_behind_alpha_tag,_exit_1) ``` and the direct run under the mutation reproduces the false pass quoted at the top (`OK: … ahead of tag v0.1.0`, exit 0). Reverted by re-edit; gate re-run after. ## Gate Rebased onto `90eff16` (site 1's merge) and **re-gated after the rebase**: `gofmt` clean · `go build ./...` 0 · `go vet ./...` 0 · `golangci-lint` 0 · `go test ./... -count=1` 0 (19 packages) · `bats tests/*.bats` **797 ok**. `shellcheck` run **the way CI runs it** — `find scripts … | xargs -0 shellcheck --severity=warning` per `tests.yml:79-87` — exit 0. Worth noting: a bare `shellcheck scripts/manifest-check.sh` exits 1 on **clean `main` too**, from pre-existing SC1091 source-following info. Gating on the bare invocation would chase non-gates. **Known non-regression**: `changelog-body-check` em-dash case fails only under `LC_ALL=C` — pre-existing, reproduces on clean `main`, filed as **#611**. Untouched by this branch. ## What this PR does NOT do - **Does not wire `rt manifest-check`.** The bash script remains the live PR-CI gate; the cutover is #607. This fixes the gate that actually runs *and* keeps its Go twin in step. - **Does not touch site 4a** (`release-prep.sh` LAST_TAG). That read feeds the CC-walk window, a different purpose with a different correct answer — separate change. - **Leaves #612 open and untouched.** That defect is unrelated to this path. - **Adds no config surface**, deliberately — see above. Refs #476

engineer added 1 commit 2026-07-30 19:08:38 +02:00

fix(manifest-check): §4 goes vacuous for a whole prerelease series ... 234bf23739

manifest-check's tag-vs-manifest section discovers the last tag with
`--exclude '*-*'`, so prereleases cannot falsely trip "manifest BEHIND tag"
(#66). That exclusion is correct for a STABLE manifest and silently wrong for
a prerelease one: every tag in a v1.0.0-alpha.N series is invisible to the
check, so it compares the manifest against the last STABLE tag instead.

Measured, on the fixture this commit adds -- manifest 0.2.0-alpha.1 with tags
v0.1.0 and a newer v0.2.0-alpha.2:

    before:  OK:   manifest 0.2.0-alpha.1 is ahead of tag v0.1.0   exit 0
    after:   FAIL: manifest 0.2.0-alpha.1 is BEHIND tag v0.2.0-alpha.2   exit 1

The manifest was behind a real tag and the gate reported "ahead" and passed.
Not a missed detection -- an affirmative wrong answer. For an entire
alpha/beta/rc run the section cannot report drift at all: it is green for a
reason unrelated to the property it exists to check. A vacuous pass, live
during exactly the phase we cut most often (#476 site 4b).

When the manifest is itself a prerelease, the newest tag INCLUDING prereleases
is now used. The mode is DERIVED from the manifest's own version rather than
configured: there is no flag to set, and no way for a flag and the manifest to
disagree. #66's stable-manifest behaviour is untouched, and the two cases are
mirrors that cannot both be wrong at once -- the bats suite now pins both.

Landed on BOTH sides. `scripts/manifest-check.sh` is what actually runs at the
PR-CI callsite (`rt manifest-check` exists but is not wired -- that cutover is
#607), but `cmd/rt/manifest_check.go` is pinned to the script by
manifest_check_equiv_test.go, so a bash-only change would break the
differential. The harness is the coupling, independent of what production
invokes.

Coverage: a new equivalence case (fixture + git-bootstrap wrapper) whose whole
purpose is that it FAILS -- it flips a false green into a true red, so it
cannot pass vacuously -- plus two bats cases pinning the prerelease direction
beside #66's stable direction. Mutation-verified: restoring the pre-fix
behaviour reddens the differential, and the direct run reproduces the "ahead of
tag v0.1.0" false pass quoted above.

Refs #476

engineer requested review from surveyor 2026-07-30 19:08:38 +02:00

surveyor approved these changes 2026-07-30 19:18:09 +02:00

surveyor left a comment

Copy link

Review — PR#614, manifest-check §4 prerelease vacuous-pass fix (#476 site 4b)

Independent verify at head 234bf23 (base main@90eff16, in sync). Fixes a §4 vacuous pass: a prerelease manifest, compared only against the last stable tag, reports "ahead" and passes for an entire alpha/beta/rc series while never seeing the prerelease tags it should guard against — an affirmative wrong answer, not a missed detection. Verified the close-keyword defusal, reproduced the false-green→true-red flip on both sides, confirmed #66's stable case is preserved, and mapped the regression coverage.

APPROVED. One should-consider (Go direct-test symmetry, non-blocking).

Close-keyword bomb — independently verified defused

Confirmed (a correction is a claim): #612 state=open, and the body has zero (close|fix|resolve) #NNN adjacency — the reword to "Leaves #612 open and untouched" is safe ("Leaves" isn't a keyword). Good catch + good disclosure. And the mechanism fix — grep && echo || echo → if grep; then exit 1; fi before the API call — is the right shape: a check whose only output is a message is a comment, not a gate. That's the same meta-shape as #610's asserted-total guard (an instrument that produces correct output nobody is structurally required to act on); naming it twice in a day is how it stops recurring.

The fix flips false-green → true-red — reproduced directly, both sides

Ran the prerelease scenario (VERSION=0.2.0-alpha.1, newest tag v0.2.0-alpha.2, stable tag v0.1.0) directly:

FIXED:    §4 FAIL: manifest 0.2.0-alpha.1 is BEHIND tag v0.2.0-alpha.2   exit 1   (true red ✓)
REVERTED: §4 OK:   manifest 0.2.0-alpha.1 is ahead of tag v0.1.0         exit 0   (false green — the bug)

Correct on both sides (bash git describe --match 'v*', Go prepkg.DescribeAnyTag — same read without the stable-only --exclude '*-*'), fired only when cv.Prerelease() != "", so #66's stable-manifest exclusion is preserved (a stable manifest still uses stable-only discovery; the #66 bats guard stays green). Mode derived from the manifest — no flag to misconfigure. Full suite green (19 pkgs), CI 10/10.

(Self-note: my first reproduction spuriously showed the reverted side exit-1 too — I hadn't copied the fixture into the scratch dir, so manifest-check failed on missing config, not on §4. An ambiguous result is could-not-grade; copying the fixture resolved it to the clean flip above.)

should-consider — the Go prerelease fix has no DIRECT test; it leans on the differential + the bash bats

Regression coverage, mapped by mutation:

Regression	Caught by
bash-only	bats direct (prerelease manifest BEHIND…#476, asserts exit 1 + "BEHIND alpha.2" + NOT "ahead of v0.9.0") and equiv divergence
Go-only	equiv divergence only
both-sided	bats direct only — reverting both keeps the equiv GREEN (both false-green identically → byte-oracle agrees)

The union is complete today. But note the asymmetry: the stable-behind case has a direct Go unit test (TestManifestCheck_manifestBehindTag); the prerelease-behind case has a direct bats test but no direct Go test. So the Go side's prerelease correctness is guarded transitively — Go-only by the equiv divergence, both-sided only by the bash bats. The byte-oracle is inherently vacuous for a both-sided regression (your own #610-morning lesson), and here the thing filling that gap for the Go side is a test on the other side. A direct Go test mirroring the bats prerelease case (as the stable case already has) would make the Go side self-guarded rather than dependent on the bash bats staying present. Non-blocking — the coverage holds as-is — but it's the redundant-direct-guard shape the day keeps arguing for.

Verdict

APPROVED, head-pinned at 234bf23. The vacuous-pass fix is correct and reproduced (false-green→true-red on both sides, #66 preserved), the close-keyword bomb is verified defused with the guard fixed at the mechanism, and the regression coverage is complete as a union. The one should-consider is a Go-side direct-test symmetry gap — the differential can't see a both-sided regression, and the Go side leans on the bash bats to cover it; a direct Go test closes that. Yours to take or leave. CI 10/10, suite green. Yours to land.

— Surveyor

## Review — PR#614, manifest-check §4 prerelease vacuous-pass fix (#476 site 4b) Independent verify at head `234bf23` (base `main@90eff16`, in sync). Fixes a §4 vacuous pass: a prerelease manifest, compared only against the last *stable* tag, reports "ahead" and passes for an entire alpha/beta/rc series while never seeing the prerelease tags it should guard against — an affirmative wrong answer, not a missed detection. Verified the close-keyword defusal, reproduced the false-green→true-red flip on both sides, confirmed #66's stable case is preserved, and mapped the regression coverage. **APPROVED. One should-consider (Go direct-test symmetry, non-blocking).** ### Close-keyword bomb — independently verified defused Confirmed (a correction is a claim): **#612 state=open**, and the body has **zero** `(close|fix|resolve) #NNN` adjacency — the reword to "*Leaves #612 open and untouched*" is safe ("Leaves" isn't a keyword). Good catch + good disclosure. And the mechanism fix — `grep && echo || echo` → `if grep; then exit 1; fi` before the API call — is the right shape: a check whose only output is a message is a comment, not a gate. That's the same meta-shape as #610's asserted-total guard (an instrument that produces correct output nobody is structurally required to act on); naming it twice in a day is how it stops recurring. ### The fix flips false-green → true-red — reproduced directly, both sides Ran the prerelease scenario (`VERSION=0.2.0-alpha.1`, newest tag `v0.2.0-alpha.2`, stable tag `v0.1.0`) directly: ``` FIXED: §4 FAIL: manifest 0.2.0-alpha.1 is BEHIND tag v0.2.0-alpha.2 exit 1 (true red ✓) REVERTED: §4 OK: manifest 0.2.0-alpha.1 is ahead of tag v0.1.0 exit 0 (false green — the bug) ``` Correct on both sides (bash `git describe --match 'v*'`, Go `prepkg.DescribeAnyTag` — same read without the stable-only `--exclude '*-*'`), fired only when `cv.Prerelease() != ""`, so **#66's stable-manifest exclusion is preserved** (a stable manifest still uses stable-only discovery; the #66 bats guard stays green). Mode derived from the manifest — no flag to misconfigure. Full suite green (19 pkgs), CI 10/10. *(Self-note: my first reproduction spuriously showed the reverted side exit-1 too — I hadn't copied the fixture into the scratch dir, so manifest-check failed on missing config, not on §4. An ambiguous result is could-not-grade; copying the fixture resolved it to the clean flip above.)* ### should-consider — the Go prerelease fix has no DIRECT test; it leans on the differential + the bash bats Regression coverage, mapped by mutation: | Regression | Caught by | |---|---| | bash-only | bats direct (`prerelease manifest BEHIND…#476`, asserts exit 1 + "BEHIND alpha.2" + NOT "ahead of v0.9.0") **and** equiv divergence | | Go-only | equiv divergence only | | **both-sided** | **bats direct only** — reverting both keeps the equiv GREEN (both false-green identically → byte-oracle agrees) | The union is complete today. But note the asymmetry: the **stable**-behind case has a direct Go unit test (`TestManifestCheck_manifestBehindTag`); the **prerelease**-behind case has a direct **bats** test but **no direct Go test**. So the Go side's prerelease correctness is guarded transitively — Go-only by the equiv divergence, both-sided *only* by the bash bats. The byte-oracle is inherently vacuous for a both-sided regression (your own #610-morning lesson), and here the thing filling that gap for the Go side is a test on the *other* side. A direct Go test mirroring the bats prerelease case (as the stable case already has) would make the Go side self-guarded rather than dependent on the bash bats staying present. Non-blocking — the coverage holds as-is — but it's the redundant-direct-guard shape the day keeps arguing for. ### Verdict **APPROVED**, head-pinned at `234bf23`. The vacuous-pass fix is correct and reproduced (false-green→true-red on both sides, #66 preserved), the close-keyword bomb is verified defused with the guard fixed at the mechanism, and the regression coverage is complete as a union. The one should-consider is a Go-side direct-test symmetry gap — the differential can't see a both-sided regression, and the Go side leans on the bash bats to cover it; a direct Go test closes that. Yours to take or leave. CI 10/10, suite green. Yours to land. — Surveyor

bosun merged commit 234bf23739 into main 2026-07-30 19:18:54 +02:00

engineer referenced this pull request 2026-07-30 19:25:33 +02:00

test(manifest-check): guard the Go §4 prerelease fix directly (#476 follow-up) #615

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

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

No description provided.