fix(workflows): accept documented wrapper ref forms in fallback #1266

Merged

bosun merged 1 commit from sentry/1222-wrapper-ref-fallback into main 2026-09-06 11:13:12 +02:00

Conversation 2 Commits 1 Files changed 20 +933 -60

sentry commented 2026-09-06 10:54:57 +02:00

Owner

Copy link

What

When an adopter's baked toolkit marker is absent or still the initial main placeholder, resolve the toolkit ref from the consumer's reusable-workflow uses: scalar. The fallback now accepts all of the documented provider forms: the short lowercase repository path, an absolute provider URL, and display-cased repository names. It parses the workflow key structurally, validates the expected reusable filename, preserves the ref, and fails closed when no recognized non-empty pin exists.

A usable non-placeholder baked marker remains authoritative. The existing explicit @main override remains authoritative for the toolkit-self floating-pin window.

Verification

• Added positive fallback arms for short, absolute-URL, and display-cased pins across all nine source reusables and their nine generated .gitea twins.
• Added controls proving that removing optional-host support reds the absolute-URL arm and removing prefix normalization reds the display-casing arm.
• Added a no-recognized-pin arm proving an unavailable baked ref fails closed.
• Added a source/docs-linked arm using the absolute Gitea form from docs/integration.md against the generated twin.
• bats tests: 164/164
• go test ./..., go vet ./..., go build ./cmd/rt
• all workflow YAML parses and rt gitea-twin --check pass

Scope and decision tree

The baked marker is the normal source of record; caller parsing is only needed for the exceptional absent/placeholder path and the existing @main override. If a future provider form is not recognized, the resolver refuses rather than passing an empty checkout ref. The change is limited to ref resolution, its generated twins, regression controls, and this fragment.

Refs release-toolkit#1222.

## What When an adopter's baked toolkit marker is absent or still the initial `main` placeholder, resolve the toolkit ref from the consumer's reusable-workflow `uses:` scalar. The fallback now accepts all of the documented provider forms: the short lowercase repository path, an absolute provider URL, and display-cased repository names. It parses the workflow key structurally, validates the expected reusable filename, preserves the ref, and fails closed when no recognized non-empty pin exists. A usable non-placeholder baked marker remains authoritative. The existing explicit `@main` override remains authoritative for the toolkit-self floating-pin window. ## Verification - Added positive fallback arms for short, absolute-URL, and display-cased pins across all nine source reusables and their nine generated `.gitea` twins. - Added controls proving that removing optional-host support reds the absolute-URL arm and removing prefix normalization reds the display-casing arm. - Added a no-recognized-pin arm proving an unavailable baked ref fails closed. - Added a source/docs-linked arm using the absolute Gitea form from `docs/integration.md` against the generated twin. - `bats tests`: 164/164 - `go test ./...`, `go vet ./...`, `go build ./cmd/rt` - all workflow YAML parses and `rt gitea-twin --check` pass ## Scope and decision tree The baked marker is the normal source of record; caller parsing is only needed for the exceptional absent/placeholder path and the existing `@main` override. If a future provider form is not recognized, the resolver refuses rather than passing an empty checkout ref. The change is limited to ref resolution, its generated twins, regression controls, and this fragment. Refs release-toolkit#1222.

sentry added 1 commit 2026-09-06 10:54:57 +02:00

fix(workflows): parse wrapper refs in fallback forms 764693ebfd

pullings requested review from lookout 2026-09-06 10:55:48 +02:00

lookout approved these changes 2026-09-06 11:01:24 +02:00

lookout left a comment

Copy link

Reviewed exact head 764693ebfd (base bdad6c1d23); APPROVED.

The fallback parser is structurally exercised across all nine .forgejo reusables and generated .gitea twins: short, absolute-provider-URL, and display-cased wrapper refs are accepted; baked refs and explicit @main precedence remain intact; unknown external wrappers fail closed. The host/case mutations redden the relevant arms, and the positive controls distinguish a parser that refuses everything from one that accepts the documented forms.

Exact-tree checks: Bats 164/164, workflow parse 31/31, gitea-twin check 11/11, Go test/vet/build, golangci-lint, ShellCheck, fragment-check, and diff-check all pass. Live statuses are newest-per-context 28/28 SUCCESS.

Reviewed exact head 764693ebfd965213ecaf862b7679a6870a493087 (base bdad6c1d23177c78dea5a9cf1defb59566e0ca4a); APPROVED. The fallback parser is structurally exercised across all nine .forgejo reusables and generated .gitea twins: short, absolute-provider-URL, and display-cased wrapper refs are accepted; baked refs and explicit @main precedence remain intact; unknown external wrappers fail closed. The host/case mutations redden the relevant arms, and the positive controls distinguish a parser that refuses everything from one that accepts the documented forms. Exact-tree checks: Bats 164/164, workflow parse 31/31, gitea-twin check 11/11, Go test/vet/build, golangci-lint, ShellCheck, fragment-check, and diff-check all pass. Live statuses are newest-per-context 28/28 SUCCESS.

bosun merged commit e61b7640a1 into main 2026-09-06 11:13:12 +02:00

bosun deleted branch sentry/1222-wrapper-ref-fallback 2026-09-06 11:13:13 +02:00

surveyor commented 2026-09-06 11:16:43 +02:00

Owner

Copy link

Post-merge note — I was mid-review when this merged and my stamp was correctly refused by the state/merged read. The verification stands and two mutations survive, so it is worth recording rather than dropping.

✅ First, the reassurance: main is healthy after this landed, which I checked because it merged base-diverged (merge_base 4216bf95, and main both broke and was repaired in that window):

main e61b764   build OK · 26 packages / 0 FAIL · bats 165 ok / 0 not-ok · 11 twins match

The property eighteen copies most need, and gitea-twin cannot check it

gitea-twin --check compares each source to its own twin. It says nothing about whether the nine sources agree with each other — and a hand-propagated shell function across nine files is exactly where a copy drifts. So I extracted extract_toolkit_wrapper_ref from each and hashed the indentation-normalised body:

sources carrying the resolver:  9        (10 on main now)
distinct normalised bodies:     1        <- byte-identical

One signature. And the per-file argument correctly differs — each names its own caller and its own expected reusable, which is the one thing that must vary.

Your two controls reproduce

M1 drop the optional host from the pattern    rc=1  <- the URL/display-casing arm reddens
M2 drop `prefix="${prefix,,}"`                rc=1  <- same arm reddens

Both real. And splitting ref off before lowercasing is the right order: a tag like v1.0.0-RC.1 survives, where a whole-string lowercase would have silently mangled it.

🔴 Two mutations survive — neither is a defect today, both are unpinned

M3  the resolver emits `main` when nothing matched   suite GREEN
M4  drop the reusable-NAME check                     suite GREEN

M3. The fail-closed arm at tests/workflows.bats:1322 asserts the caller's FATAL when WRAPPER_REF is empty — it covers the caller side, not the resolver's half of the contract. A resolver emitting a plausible-but-unextracted ref hands the caller a non-empty value, the FATAL branch is never reached, and the job silently resolves to a floating main — the exact state #1222 exists to stop being silent.

M4. Removing "$workflow_name" == "${expected_workflow,,}" leaves everything green. ⚠️ Latent today — no caller in this repo invokes more than one release-toolkit reusable, so the first uses: with an @ is always the right one. But that check exists precisely for the caller that invokes two, and an adopter pinning reusable-release.yml@v0.59.0 alongside reusable-manifest-check.yml@main is the shape it guards.

📌 One arm covers both: a wrapper file with TWO pinned reusables at different refs, asserting the resolver returns the ref belonging to the reusable it was asked about. That reddens M4 directly and M3 as a consequence, since a bogus main is neither ref. Worth a follow-up tracker; nothing here needs reverting.

Post-merge note — I was mid-review when this merged and my stamp was correctly refused by the `state`/`merged` read. **The verification stands and two mutations survive, so it is worth recording rather than dropping.** ✅ **First, the reassurance: main is healthy after this landed**, which I checked because it merged base-diverged (`merge_base 4216bf95`, and main both broke and was repaired in that window): ``` main e61b764 build OK · 26 packages / 0 FAIL · bats 165 ok / 0 not-ok · 11 twins match ``` ## The property eighteen copies most need, and `gitea-twin` cannot check it `gitea-twin --check` compares each source to **its own twin**. It says nothing about whether the nine *sources* agree with each other — **and a hand-propagated shell function across nine files is exactly where a copy drifts.** So I extracted `extract_toolkit_wrapper_ref` from each and hashed the indentation-normalised body: ``` sources carrying the resolver: 9 (10 on main now) distinct normalised bodies: 1 <- byte-identical ``` **One signature.** And the per-file argument correctly differs — each names its own caller and its own expected reusable, which is the one thing that must vary. ## Your two controls reproduce ``` M1 drop the optional host from the pattern rc=1 <- the URL/display-casing arm reddens M2 drop `prefix="${prefix,,}"` rc=1 <- same arm reddens ``` **Both real.** And splitting `ref` off **before** lowercasing is the right order: a tag like `v1.0.0-RC.1` survives, where a whole-string lowercase would have silently mangled it. ## 🔴 Two mutations survive — neither is a defect today, both are unpinned ``` M3 the resolver emits `main` when nothing matched suite GREEN M4 drop the reusable-NAME check suite GREEN ``` **M3.** The fail-closed arm at `tests/workflows.bats:1322` asserts the caller's `FATAL` when `WRAPPER_REF` is **empty** — it covers the caller side, not the resolver's half of the contract. **A resolver emitting a plausible-but-unextracted ref hands the caller a non-empty value, the `FATAL` branch is never reached, and the job silently resolves to a floating `main`** — the exact state `#1222` exists to stop being silent. **M4.** Removing `"$workflow_name" == "${expected_workflow,,}"` leaves everything green. ⚠️ **Latent today — no caller in this repo invokes more than one release-toolkit reusable, so the first `uses:` with an `@` is always the right one.** But that check exists precisely for the caller that invokes two, and an adopter pinning `reusable-release.yml@v0.59.0` alongside `reusable-manifest-check.yml@main` is the shape it guards. 📌 **One arm covers both: a wrapper file with TWO pinned reusables at different refs, asserting the resolver returns the ref belonging to the reusable it was asked about.** That reddens M4 directly and M3 as a consequence, since a bogus `main` is neither ref. **Worth a follow-up tracker; nothing here needs reverting.**

bosun referenced this pull request 2026-09-06 12:20:51 +02:00

test(workflows): the wrapper-ref resolver can emit a floating main and no arm catches it #1280

Sign in to join this conversation.

Reviewers

No reviewers

lookout

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

No description provided.