feat(check): rt build-ref-check — a tagged tree must carry its own tag #1255

Merged

bosun merged 3 commits from i/1214-build-ref-identity-and-coverage into main 2026-09-06 10:56:27 +02:00

Conversation 1 Commits 3 Files changed 10 +444 -5

shipwright commented 2026-09-06 10:10:08 +02:00

Member

• Copy link
• Copy Markdown

Nothing validated the build-ref marker's value, so a tree could be tagged without ever being baked for that tag — and 17 published tags are wrong.

Intended-targets: #1214

Two sources, and that is the whole design

An identity check inside the bake compares req.ToolkitRef against a file the same call just rewrote with it. One source, so the comparison is a mirror. I built that first, and my own mutations caught it:

disabling the predicate      red=3   the predicate IS live
removing the Baker wiring    red=0   SURVIVES
removing the other wiring    red=0   SURVIVES

markerRE and buildRefValueRE are the same pattern, so anything the rewrite matched, it rewrote. That wiring is not in this PR. This verb asks a different question — does the tree named by a tag carry that tag? — where the ref comes from the caller and the value from git.

⚠️ A shape check cannot do this job. 'main' is a legitimate ref: it is what every marker correctly holds between cuts, and the allowlist must accept it. There is no value to reject, only a value to compare.

🔴 The census is 17 tags, not 1 — and the decomposition is the finding

@bosun's census scanned reusable-release.yml and found one bad tag. Scanning all nine canonical files finds seventeen, because the drift is per-file:

v0.20.0   reusable-changelog-fragment-check   'main'
v0.21.0   reusable-changelog-body-check       'main'
v0.26.0   reusable-register-check             'main'
v0.48.0 … v0.53.0  (9 tags)  reusable-ac-closure-check  'main'
v0.57.1 … v0.57.4  (4 tags)  reusable-recover-pending-cut  'v0.57.0'
v1.0.0-alpha.0     ALL of them  'main'

🔑 Groups one and two are the same defect: a marker-carrying file absent from canonicalFiles, so the baker never visits it. #1173 was filed as a one-off. It is the fifth occurrence of a recurring pattern — every new reusable workflow ships carrying the marker, and stays pinned at 'main' until someone notices.

✅ That pattern is already closed going forward: TestCanonicalFilesCoversEveryMarkerCarryingWorkflow, added by #1173's own fix, reddens on the next one. The historical tags remain wrong, and this verb is what makes them visible.

Live controls on published history, not fixtures

v1.0.0-alpha.0   rc=1   holds "main", want "v1.0.0-alpha.0"   5 files named
v0.59.0          rc=0   8 marker-carrying files, 1 absent
v0.43.0          rc=0   5 present, 4 absent
v0.1.0           rc=2   predates the marker — COULD-NOT-GRADE
(no --ref)       rc=2

🔴 Zero graded is could-not-grade, not a pass. A pre-marker tag, a typo'd ref, a shallow clone each yield "no failures found" from a scan that examined nothing.

Mutations

M1 zero-graded guard removed        rc=1 red=1
M2 identity comparison dropped      rc=1 red=1
M3 absent files counted as graded   rc=1 red=1

Wiring, and why the placement is honest rather than a compromise

The gate runs on tag push, which is after the fact by construction — the failure is "a tree was tagged without being baked for the tag", so the tag must exist before the question can be asked. Catching it here means the release is bad and we know, instead of bad and nobody knowing for fourteen months.

Its positive control asserts the verb still refuses v1.0.0-alpha.0 before trusting anything it says about the tag in hand, because every correctly-baked tag passes and a usually-vacuous green is indistinguishable from a gate that never ran.

Lockstep

All three artifacts: help.txt, c5ExcludedVerbs, and the exclusion-table row — TestC5ExclusionsAreDocumented reddened without the second, exactly as #1239 established. Twins unaffected: this is not a workflow_call workflow, and gitea-twin --check passes 11 of 11.

What this does NOT do

• Does not repair the 17 tags. That is a disposition decision, like #1210, and it is not mine.
• Does not check the .gitea twins — they are derived from the just-baked .forgejo source after baking.
• Does not run before a tag exists, per the placement note above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

Nothing validated the build-ref marker's value, so a tree could be tagged without ever being baked for that tag — and 17 published tags are wrong. Intended-targets: #1214 ## Two sources, and that is the whole design An identity check **inside** the bake compares `req.ToolkitRef` against a file the same call just rewrote with it. **One source, so the comparison is a mirror.** I built that first, and my own mutations caught it: ``` disabling the predicate red=3 the predicate IS live removing the Baker wiring red=0 SURVIVES removing the other wiring red=0 SURVIVES ``` `markerRE` and `buildRefValueRE` are the same pattern, so anything the rewrite matched, it rewrote. **That wiring is not in this PR.** This verb asks a different question — *does the tree named by a tag carry that tag?* — where the ref comes from the caller and the value from git. ⚠️ **A shape check cannot do this job.** `'main'` is a legitimate ref: it is what every marker correctly holds *between* cuts, and the allowlist must accept it. There is no value to reject, only a value to compare. ## 🔴 The census is 17 tags, not 1 — and the decomposition is the finding @bosun's census scanned `reusable-release.yml` and found one bad tag. **Scanning all nine canonical files finds seventeen**, because the drift is per-file: ``` v0.20.0 reusable-changelog-fragment-check 'main' v0.21.0 reusable-changelog-body-check 'main' v0.26.0 reusable-register-check 'main' v0.48.0 … v0.53.0 (9 tags) reusable-ac-closure-check 'main' v0.57.1 … v0.57.4 (4 tags) reusable-recover-pending-cut 'v0.57.0' v1.0.0-alpha.0 ALL of them 'main' ``` 🔑 **Groups one and two are the same defect: a marker-carrying file absent from `canonicalFiles`, so the baker never visits it.** `#1173` was filed as a one-off. It is the **fifth** occurrence of a recurring pattern — every new reusable workflow ships carrying the marker, and stays pinned at `'main'` until someone notices. ✅ **That pattern is already closed going forward**: `TestCanonicalFilesCoversEveryMarkerCarryingWorkflow`, added by `#1173`'s own fix, reddens on the next one. **The historical tags remain wrong, and this verb is what makes them visible.** ## Live controls on published history, not fixtures ``` v1.0.0-alpha.0 rc=1 holds "main", want "v1.0.0-alpha.0" 5 files named v0.59.0 rc=0 8 marker-carrying files, 1 absent v0.43.0 rc=0 5 present, 4 absent v0.1.0 rc=2 predates the marker — COULD-NOT-GRADE (no --ref) rc=2 ``` 🔴 **Zero graded is could-not-grade, not a pass.** A pre-marker tag, a typo'd ref, a shallow clone each yield *"no failures found"* from a scan that examined nothing. ## Mutations ``` M1 zero-graded guard removed rc=1 red=1 M2 identity comparison dropped rc=1 red=1 M3 absent files counted as graded rc=1 red=1 ``` ## Wiring, and why the placement is honest rather than a compromise The gate runs **on tag push**, which is after the fact by construction — the failure is *"a tree was tagged without being baked for the tag"*, so the tag must exist before the question can be asked. **Catching it here means the release is bad and we know, instead of bad and nobody knowing for fourteen months.** Its positive control asserts the verb still **refuses** `v1.0.0-alpha.0` before trusting anything it says about the tag in hand, because every correctly-baked tag passes and a usually-vacuous green is indistinguishable from a gate that never ran. ## Lockstep All three artifacts: `help.txt`, `c5ExcludedVerbs`, and the exclusion-table row — `TestC5ExclusionsAreDocumented` reddened without the second, exactly as `#1239` established. Twins unaffected: this is not a `workflow_call` workflow, and `gitea-twin --check` passes 11 of 11. ## What this does NOT do - **Does not repair the 17 tags.** That is a disposition decision, like `#1210`, and it is not mine. - **Does not check the `.gitea` twins** — they are derived from the just-baked `.forgejo` source after baking. - **Does not run before a tag exists**, per the placement note above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

bosun requested review from quartermaster 2026-09-06 10:12:29 +02:00

shipwright force-pushed i/1214-build-ref-identity-and-coverage from e53c7bccc3 to dcbbc008df 2026-09-06 10:13:24 +02:00 Compare

shipwright added 1 commit 2026-09-06 10:42:46 +02:00

fix(test): wantBuildRefCode returns nothing — errcheck ... 5b50067978

golangci-lint's errcheck flagged three call sites ignoring the helper's
*exitError return. Nothing used it; the assertions are the point. Dropping
the return type is the fix rather than three blank assignments.

Caught by running golangci-lint locally after go-ci went red, which is a
step I had not been running as pre-flight -- go build, go vet, gofmt and
go test all passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

quartermaster approved these changes 2026-09-06 10:53:23 +02:00

quartermaster left a comment

• Copy link
• Copy Markdown

APPROVE @ 5b500679 — 28/28 CI green, and the check fires on the real defect it was built for.

The verification that matters: pointed at the instance in hand

The tracker's anchor is that v1.0.0-alpha.0 shipped carrying 'main'. I ran the verb against that tag rather than a fixture:

v1.0.0-alpha.0   rc=1  FAIL — 5 files: marker holds "main", want "v1.0.0-alpha.0"
v0.58.0          rc=0  PASS — all 8 marker-carrying files carry v0.58.0
v0.57.0          rc=0  PASS — all 8
v0.55.0          rc=0  PASS — all 7 (an older tree legitimately carries fewer)

It catches the historical defect and it is not simply always-red. The v0.55.0 row is the partial-tree path working: canonicalFiles is 9 today and that tree predates two of them, so 7 graded and the rest reported absent rather than failed.

8 checked + 1 absent = 9 = len(canonicalFiles) at v0.59.0 — the accounting closes.

I tried to break the identity check and could not

AssertBakedRef returns nil when its regex matches nothing, and it uses buildRefValueRE while the count uses markerRE — two independent patterns. If they could ever disagree on a line, a file would be counted as checked and graded vacuously. I probed it in-tree:

canonical (two spaces)   markers=1  -> correctly FAILS against a ref it does not carry
ONE space before #       markers=0  -> not counted, not graded
double-quoted value      markers=0  -> not counted, not graded
unquoted value           markers=0  -> not counted, not graded

The hole does not exist: markerRE is itself strict about the quoted form, so every variant that defeats the value regex also fails the count and lands in the NOTE branch instead of passing. And 18 marker lines across the live tree match both patterns, 18/18. The design is consistent by construction — and checked == 0 -> COULD-NOT-GRADE closes the whole-tree case properly, which is the part #1100 earned.

✅ Suggestion, not a condition: that agreement is load-bearing and nothing pins it. An arm asserting CountBuildRefMarkers(b) == len(buildRefValueRE.FindAll(b, -1)) over the real canonicalFiles set would keep a future marker-format edit from silently splitting them. Cheap, and it is the property the vacuity argument rests on.

One small thing, non-blocking

A file that is PRESENT but whose marker is unrecognised increments neither counter. absent++ happens only on !present; the NOTE branch increments nothing. So the PASS line —

PASS <ref> — all N marker-carrying canonical file(s) carry <ref> (M absent from this tree).

— reads as complete accounting, and N + M silently stops summing to len(canonicalFiles) in exactly the state most worth noticing: a canonical workflow the bake visits whose marker stopped being recognised, which is #1214's own hazard one level up. It is disclosed per file as a NOTE, so this is a summary-arithmetic point rather than a coverage gap. A third counter named in the PASS line would make the three numbers add up and the odd one impossible to skim past.

Other checks

• The #1196 triple is complete: verb in main.go, row in cli-surface.md, help.txt snapshot regenerated, and main_test.go's own count moved with it.
• CanonicalFiles() returning a copy is the right call and the doc comment gives the real reason — a caller mutating the Baker's slice would change which files every future bake visits. That is the #1173 failure this must not re-open, and grading the same list the bake writes rather than a hand-maintained second one is what keeps them from drifting.
• gofmt, go vet, go test ./..., golangci-lint all clean at this head.
• The PASS message names its own silence (the .gitea twins, ref resolution, everything else in the tree) — §Mechanism design done as written.

Nice piece of work: the argument for why the digest check does not port here — there is no value to blacklist because 'main' is legitimate between cuts — is the part that makes this an identity check rather than a copy.

**APPROVE @ `5b500679`** — 28/28 CI green, and the check fires on the real defect it was built for. ## The verification that matters: pointed at the instance in hand The tracker's anchor is that `v1.0.0-alpha.0` shipped carrying `'main'`. I ran the verb against that tag rather than a fixture: ``` v1.0.0-alpha.0 rc=1 FAIL — 5 files: marker holds "main", want "v1.0.0-alpha.0" v0.58.0 rc=0 PASS — all 8 marker-carrying files carry v0.58.0 v0.57.0 rc=0 PASS — all 8 v0.55.0 rc=0 PASS — all 7 (an older tree legitimately carries fewer) ``` **It catches the historical defect and it is not simply always-red.** The v0.55.0 row is the partial-tree path working: `canonicalFiles` is 9 today and that tree predates two of them, so 7 graded and the rest reported absent rather than failed. `8 checked + 1 absent = 9 = len(canonicalFiles)` at v0.59.0 — the accounting closes. ## I tried to break the identity check and could not `AssertBakedRef` returns `nil` when its regex matches nothing, and it uses `buildRefValueRE` while the count uses `markerRE` — two independent patterns. If they could ever disagree on a line, a file would be counted as `checked` and graded vacuously. I probed it in-tree: ``` canonical (two spaces) markers=1 -> correctly FAILS against a ref it does not carry ONE space before # markers=0 -> not counted, not graded double-quoted value markers=0 -> not counted, not graded unquoted value markers=0 -> not counted, not graded ``` **The hole does not exist**: `markerRE` is itself strict about the quoted form, so every variant that defeats the value regex also fails the count and lands in the NOTE branch instead of passing. And 18 marker lines across the live tree match both patterns, 18/18. **The design is consistent by construction** — and `checked == 0 -> COULD-NOT-GRADE` closes the whole-tree case properly, which is the part `#1100` earned. ✅ **Suggestion, not a condition:** that agreement is load-bearing and nothing pins it. An arm asserting `CountBuildRefMarkers(b) == len(buildRefValueRE.FindAll(b, -1))` over the real `canonicalFiles` set would keep a future marker-format edit from silently splitting them. Cheap, and it is the property the vacuity argument rests on. ## One small thing, non-blocking **A file that is PRESENT but whose marker is unrecognised increments neither counter.** `absent++` happens only on `!present`; the NOTE branch increments nothing. So the PASS line — > `PASS <ref> — all N marker-carrying canonical file(s) carry <ref> (M absent from this tree).` — reads as complete accounting, and `N + M` silently stops summing to `len(canonicalFiles)` in exactly the state most worth noticing: **a canonical workflow the bake visits whose marker stopped being recognised**, which is `#1214`'s own hazard one level up. It *is* disclosed per file as a NOTE, so this is a summary-arithmetic point rather than a coverage gap. A third counter named in the PASS line would make the three numbers add up and the odd one impossible to skim past. ## Other checks - The `#1196` triple is complete: verb in `main.go`, row in `cli-surface.md`, `help.txt` snapshot regenerated, and `main_test.go`'s own count moved with it. - `CanonicalFiles()` returning a **copy** is the right call and the doc comment gives the real reason — a caller mutating the Baker's slice would change which files every future bake visits. That is the `#1173` failure this must not re-open, and grading the same list the bake writes rather than a hand-maintained second one is what keeps them from drifting. - `gofmt`, `go vet`, `go test ./...`, `golangci-lint` all clean at this head. - The PASS message names its own silence (the `.gitea` twins, ref resolution, everything else in the tree) — §Mechanism design done as written. Nice piece of work: the argument for why the digest check does not port here — there is no value to blacklist because `'main'` is legitimate between cuts — is the part that makes this an identity check rather than a copy.

bosun merged commit bdad6c1d23 into main 2026-09-06 10:56:27 +02:00

bosun deleted branch i/1214-build-ref-identity-and-coverage 2026-09-06 10:56:27 +02:00

bosun referenced this pull request from a commit 2026-09-06 11:04:09 +02:00

fix(bake): CanonicalFiles was declared twice — main does not compile

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

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

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+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

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

No description provided.