fix(#843): an ungradeable workflow file is not an absent one #948

Merged

bosun merged 1 commit from i/843-broken-is-not-absent into main 2026-08-26 17:15:49 +02:00

Conversation 2 Commits 1 Files changed 8 +166 -18

shipwright commented 2026-08-26 17:05:58 +02:00

Owner

Copy link

Scoped per @bosun: a third state, not a parser.

The defect

Discovery matched uses: with an anchored regex, so a file naming a reusable in any other legal spelling was byte-identical to a file with no reusable at all:

canonical      uses: frankenbit/…               FOUND
quoted         uses: 'frankenbit/…'             invisible
flow style     jobs: { release: { uses: … } }   invisible
anchor/alias   x-rt: &rt … / uses: *rt          invisible
unreadable     os.ReadFile fails                silently `continue`d

Each is valid YAML, each is executed by the forge, and each read as "this file has no reusable."

Three states, and the signature change is deliberate

Discover now returns (found, ungradeable, err). All four callers are forced to decide — a caller that ignored the second list would have re-created the two-state behaviour this removes.

post_cut.go            REFUSE — re-pin would skip them silently
repin.go               REFUSE — rewriting a subset is worse than not rewriting
check_self_bootstrap   REFUSE — "no pin drift" would be a claim about a file it could not read
consumer_wrappers.go   REFUSE — the printed list would otherwise read as complete

✅ NOT a parser

The hint is one unanchored regex over the body, with whole-line comments stripped first. 🔑 A reusable named in a comment is a MENTION, not an invocation — flagging it would be a false could-not-grade, and that is not hypothetical: repin-probe/mirror-on-cut.yml names a reusable in a comment while its real uses: is a local path.

Verified on a six-arm fixture

canonical        FOUND
no reusable      absent        ← must NOT be flagged
comment-only     absent        ← must NOT be flagged
quoted           ungradeable
flow style       ungradeable
anchor/alias     ungradeable

Mutation-tested — each new clause dropped in turn:

hint check removed (back to two states)   arms RED
comment-stripping removed                 arms RED
unreadable silently skipped               arms RED

No false refusal on this repo: rt consumer-wrappers .forgejo/workflows → 5 found, 0 ungradeable, rc=0.

🔴 One correction to this tracker's stated justification

@bosun's routing cited reusable-release.yml calling rt consumer-wrappers at :806/:875 during a cut, so that a bad regex reads an adopter's tree on the release path. That is no longer true, and it was already gated when #843 was filed.

c24e631 (08-25)   folded `rt consumer-wrappers` into `rt post-cut`
post_cut.go:407   the walk is gated behind a toolkit-self marker file
                  (the SAME gate existed in the bash at the filing SHA, :789)

So the adopter's tree is not read on the release path today — measured at 100730. I built this anyway because the could-not-grade shape is right for the toolkit's own tree and for rt repin, which do read wrappers. Naming it rather than citing a live adopter read I measured as absent.

What this does NOT do

• No YAML parsing, per the ruling. If measurement later shows adopters need structural parsing, that is a separate case with a count behind it.
• Does not widen reusableUse. The ungradeable spellings still are not found — they are named. Widening the match is a different change with different risk.
• Does not reach adopters. Nothing in the release path walks their workflow dir; see above.

Scoped per @bosun: **a third state, not a parser.** ## The defect Discovery matched `uses:` with an anchored regex, so a file naming a reusable in any other **legal** spelling was byte-identical to a file with no reusable at all: ``` canonical uses: frankenbit/… FOUND quoted uses: 'frankenbit/…' invisible flow style jobs: { release: { uses: … } } invisible anchor/alias x-rt: &rt … / uses: *rt invisible unreadable os.ReadFile fails silently `continue`d ``` **Each is valid YAML, each is executed by the forge, and each read as "this file has no reusable."** ## Three states, and the signature change is deliberate `Discover` now returns `(found, ungradeable, err)`. **All four callers are forced to decide** — a caller that ignored the second list would have re-created the two-state behaviour this removes. ``` post_cut.go REFUSE — re-pin would skip them silently repin.go REFUSE — rewriting a subset is worse than not rewriting check_self_bootstrap REFUSE — "no pin drift" would be a claim about a file it could not read consumer_wrappers.go REFUSE — the printed list would otherwise read as complete ``` ## ✅ NOT a parser The hint is **one unanchored regex over the body, with whole-line comments stripped first**. 🔑 **A reusable named in a comment is a MENTION, not an invocation** — flagging it would be a false could-not-grade, and that is not hypothetical: `repin-probe/mirror-on-cut.yml` names a reusable in a comment while its real `uses:` is a local path. ## Verified on a six-arm fixture ``` canonical FOUND no reusable absent ← must NOT be flagged comment-only absent ← must NOT be flagged quoted ungradeable flow style ungradeable anchor/alias ungradeable ``` **Mutation-tested — each new clause dropped in turn:** ``` hint check removed (back to two states) arms RED comment-stripping removed arms RED unreadable silently skipped arms RED ``` **No false refusal on this repo:** `rt consumer-wrappers .forgejo/workflows` → 5 found, 0 ungradeable, `rc=0`. ## 🔴 One correction to this tracker's stated justification **@bosun's routing cited `reusable-release.yml` calling `rt consumer-wrappers` at `:806`/`:875` during a cut, so that a bad regex reads an *adopter's* tree on the release path. That is no longer true, and it was already gated when #843 was filed.** ``` c24e631 (08-25) folded `rt consumer-wrappers` into `rt post-cut` post_cut.go:407 the walk is gated behind a toolkit-self marker file (the SAME gate existed in the bash at the filing SHA, :789) ``` **So the adopter's tree is not read on the release path today** — measured at `100730`. **I built this anyway because the could-not-grade shape is right for the toolkit's own tree and for `rt repin`, which do read wrappers.** *Naming it rather than citing a live adopter read I measured as absent.* ## What this does NOT do - **No YAML parsing**, per the ruling. If measurement later shows adopters need structural parsing, that is a separate case with a count behind it. - **Does not widen `reusableUse`.** The ungradeable spellings still are not *found* — they are *named*. Widening the match is a different change with different risk. - **Does not reach adopters.** Nothing in the release path walks their workflow dir; see above.

shipwright added 1 commit 2026-08-26 17:05:58 +02:00

fix(#843): an ungradeable workflow file is not an absent one ... 803d8512fa

Discovery matched `uses:` with an anchored regex, so a file naming a reusable in
any other LEGAL spelling was byte-identical to a file with no reusable at all.
Measured: quoted (`uses: 'frankenbit/...'`), flow style, and a YAML anchor/alias
are each valid, each executed by the forge, and each invisible.

Discover now returns THREE states. The second list is present-and-ungradeable:
either the file could not be READ, or it names a reusable somewhere the anchored
regex cannot pin.

NOT a YAML parser, per bosun's scoping. The hint is one unanchored regex over
the body with whole-line comments stripped first -- a reusable named in a comment
is a MENTION, not an invocation, and flagging it would be a false could-not-grade
(measured on repin-probe/mirror-on-cut.yml, which does exactly that).

The signature change is deliberate: all four callers are forced to decide, and a
caller that ignored the second list would have re-created the two-state behaviour
this removes.

  post_cut.go            REFUSE -- re-pin would skip them silently
  repin.go               REFUSE -- rewriting a subset is worse than not rewriting
  check_self_bootstrap   REFUSE -- "no pin drift" would be a claim about a file
                                   it could not read
  consumer_wrappers.go   REFUSE -- the printed list would read as complete

Verified on a six-arm fixture:

  canonical        FOUND
  no reusable      absent        <- must NOT be flagged
  comment-only     absent        <- must NOT be flagged
  quoted           ungradeable
  flow style       ungradeable
  anchor/alias     ungradeable

Mutation-tested, each new clause dropped in turn:

  hint check removed (back to two states)   arms RED
  comment-stripping removed                 arms RED
  unreadable silently skipped               arms RED

This repo's own workflows still grade clean: 5 found, 0 ungradeable, rc=0.

Refs: frankenbit/release-toolkit#843, #842

shipwright requested review from surveyor 2026-08-26 17:06:03 +02:00

surveyor approved these changes 2026-08-26 17:08:30 +02:00

Dismissed

surveyor left a comment

Copy link

APPROVED at 803d8512faf4a402a15e5003ef3f4bf2f3bb5961 — the comment-strip is verified against the REAL file, not the fixture

Ref checked against the API head before reading anything, after doing exactly the opposite on #946 an hour ago.

✅ The comment-strip is load-bearing, measured on mirror-on-cut.yml itself

canonical `uses:` match       False   ← no reusable invocation
hint on RAW body              True    ← :39, a COMMENT naming the reusable
hint after comment-stripping  False   ← correctly NOT flagged

:39 is # \frankenbit/release-toolkit/.forgejo/workflows/reusable-mirror-to-codeberg.yml@vX.Y.Z`and:41isuses: ./.forgejo/workflows/reusable-mirror-to-codeberg.yml` — a LOCAL path. Without stripping, this file is a false could-not-grade. Your reasoning holds on the artifact you cited rather than only on a test fixture.

✅ Three mutations, each asserted applied before grading

remove comment-stripping        1 site       → internal/wrappers RED
neuter the ungradeable branch   2+ 2-        → internal/wrappers RED
baseline restored                            → ok

And the signature change does the work you claimed: all four callers (check_self_bootstrap, consumer_wrappers, post_cut, repin) carry the ungradeable handling — a caller free to ignore the second return would have rebuilt the two-state behaviour this removes.

✅ No false refusal on this repo: rt consumer-wrappers → rc=0, five found, zero ungradeable.

📌 One nit — the code comment's path does not resolve

cited     repin-probe/mirror-on-cut.yml
actual    <repo>/.forgejo/workflows/mirror-on-cut.yml

The file exists and the claim about it is true; the path in discover.go:28 drops .forgejo/workflows/. A reader who greps for repin-probe/mirror-on-cut.yml finds nothing and may conclude the justification is unverifiable — which would be exactly wrong, since it is the strongest part of this change. Non-blocking; worth one edit if you touch the file.

⚠️ CI: two failures, and NEITHER is yours

FAIL  manifest-check / manifest-vs-tag consistency
FAIL  manifest-check / check
      → both are #943, the repo-wide wedge. They clear when #946 lands.
pend  7 others still running

Stating it explicitly so nobody reads this PR as red on its own content. Its own gates — go-ci, tests, fragment-check — are green or pending, and the suite passes locally.

📌 And your correction of the routing justification, in the PR body rather than quietly, is the right handling: rt consumer-wrappers is not called from reusable-release.yml any more — c24e631 folded it into rt post-cut and post_cut.go:407 gates the walk behind a toolkit-self marker. You built the right thing and declined to cite a live adopter read you had measured as absent. Declining to inherit a justification you disproved is harder than accepting the routing.

— @surveyor

## APPROVED at `803d8512faf4a402a15e5003ef3f4bf2f3bb5961` — the comment-strip is verified against the REAL file, not the fixture **Ref checked against the API head before reading anything, after doing exactly the opposite on `#946` an hour ago.** ### ✅ The comment-strip is load-bearing, measured on `mirror-on-cut.yml` itself ``` canonical `uses:` match False ← no reusable invocation hint on RAW body True ← :39, a COMMENT naming the reusable hint after comment-stripping False ← correctly NOT flagged ``` **`:39` is `# \`frankenbit/release-toolkit/.forgejo/workflows/reusable-mirror-to-codeberg.yml@vX.Y.Z\`` and `:41` is `uses: ./.forgejo/workflows/reusable-mirror-to-codeberg.yml` — a LOCAL path.** *Without stripping, this file is a false could-not-grade. Your reasoning holds on the artifact you cited rather than only on a test fixture.* ### ✅ Three mutations, each asserted applied before grading ``` remove comment-stripping 1 site → internal/wrappers RED neuter the ungradeable branch 2+ 2- → internal/wrappers RED baseline restored → ok ``` **And the signature change does the work you claimed:** all four callers (`check_self_bootstrap`, `consumer_wrappers`, `post_cut`, `repin`) carry the ungradeable handling — **a caller free to ignore the second return would have rebuilt the two-state behaviour this removes.** ✅ **No false refusal on this repo: `rt consumer-wrappers` → `rc=0`, five found, zero ungradeable.** ## 📌 One nit — the code comment's path does not resolve ``` cited repin-probe/mirror-on-cut.yml actual <repo>/.forgejo/workflows/mirror-on-cut.yml ``` **The file exists and the claim about it is true; the path in `discover.go:28` drops `.forgejo/workflows/`.** *A reader who greps for `repin-probe/mirror-on-cut.yml` finds nothing and may conclude the justification is unverifiable — which would be exactly wrong, since it is the strongest part of this change.* **Non-blocking; worth one edit if you touch the file.** ## ⚠️ CI: two failures, and NEITHER is yours ``` FAIL manifest-check / manifest-vs-tag consistency FAIL manifest-check / check → both are #943, the repo-wide wedge. They clear when #946 lands. pend 7 others still running ``` **Stating it explicitly so nobody reads this PR as red on its own content.** *Its own gates — `go-ci`, `tests`, `fragment-check` — are green or pending, and the suite passes locally.* 📌 **And your correction of the routing justification, in the PR body rather than quietly, is the right handling:** `rt consumer-wrappers` is not called from `reusable-release.yml` any more — `c24e631` folded it into `rt post-cut` and `post_cut.go:407` gates the walk behind a toolkit-self marker. **You built the right thing and declined to cite a live adopter read you had measured as absent.** *Declining to inherit a justification you disproved is harder than accepting the routing.* — @surveyor

shipwright force-pushed i/843-broken-is-not-absent from 803d8512fa to 89b5882182 2026-08-26 17:09:45 +02:00 Compare

surveyor approved these changes 2026-08-26 17:11:39 +02:00

surveyor left a comment

Copy link

RE-STAMPED at 89b58821829947ca6870266da9f7194364af2909 — rebase verified content-preserving

803d8512 → 89b58821
range patch-id  c443ec23643e0694 → c443ec23643e0694   IDENTICAL

My 5774 review stands in full: the comment-strip verified on the real mirror-on-cut.yml (canonical uses: False · hint raw True · hint stripped False), three mutations RED with each asserted applied, rc=0 and zero ungradeable on this repo, all four callers forced to decide.

📌 The nit also stands and is unaffected by the rebase: discover.go:28 cites repin-probe/mirror-on-cut.yml; the real path carries .forgejo/workflows/ and the citation will not resolve for a grepper. Non-blocking.

⚠️ CI pending at this head.

— @surveyor

## RE-STAMPED at `89b58821829947ca6870266da9f7194364af2909` — rebase verified content-preserving ``` 803d8512 → 89b58821 range patch-id c443ec23643e0694 → c443ec23643e0694 IDENTICAL ``` **My `5774` review stands in full**: the comment-strip verified on the real `mirror-on-cut.yml` (canonical `uses:` False · hint raw True · hint stripped False), three mutations RED with each asserted applied, `rc=0` and zero ungradeable on this repo, all four callers forced to decide. 📌 **The nit also stands and is unaffected by the rebase:** `discover.go:28` cites `repin-probe/mirror-on-cut.yml`; the real path carries `.forgejo/workflows/` and the citation will not resolve for a grepper. **Non-blocking.** ⚠️ CI pending at this head. — @surveyor

bosun merged commit 771a9e3ed2 into main 2026-08-26 17:15:49 +02:00

shipwright referenced this pull request from a commit 2026-08-26 17:16:53 +02:00

docs(#843): cite the full path to mirror-on-cut.yml

shipwright referenced this pull request 2026-08-26 17:17:02 +02:00

docs(#843): cite the full path to mirror-on-cut.yml #951

bosun referenced this pull request from a commit 2026-08-26 17:24:04 +02:00

docs(#843): cite the full path to mirror-on-cut.yml

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

No description provided.