feat(reusable-release): migrate 7 callsites bash→rt + 2c-hybrid rt bootstrap #597

Merged

bosun merged 3 commits from i/582-reusable-rt-callsites into main 2026-07-29 14:15:10 +02:00

Conversation 1 Commits 3 Files changed 7 +248 -96

engineer commented 2026-07-29 13:33:52 +02:00

Owner

Copy link

What

Phase 7 (milestone #80) workflow integration — the load-bearing gate that makes the Phase 6/6b rt ports take effect in production. reusable-release.yml now:

1. bootstraps rt onto PATH itself (it previously ran only checked-out bash), and
2. invokes the 7 ported rt <subcommand>s instead of .release-toolkit/scripts/*.sh.

Single PR (S1) because the bootstrap and the callsite swap mutually require each other — you can't rt decide without rt on PATH, and bootstrapping rt is pointless with no callsite using it. Build order is test-in-place: bootstrap → rt --version smoke → flip callsites.

Measured: 7 files, +248/−96 (3 commits: the migration + a review-fold Go preflight + a stale bats-guard fix).

The bootstrap fork — why 2c hybrid (ratified)

Framing-verify surfaced that rt is not on PATH in the reusable: it checks out toolkit source into .release-toolkit/ and runs bash (install deps provides yq/jq/curl, not Go). Getting rt there is a real design fork:

Option	Mechanism	Rejected/chosen because
2a fetch-only	composite/curl fetch for all refs	✗ breaks toolkit-self: release.yml floating-pins the reusable @main (#456) and goreleaser publishes no main asset
2b build-only	go build for all refs	✗ imposes a Go toolchain on every adopter runner — the exact assumption ADR-0008 §4a exists to remove
2c hybrid ✓	fetch for vX.Y.Z, build for main	different classes need different mechanisms: adopters get the verified binary (no toolchain); toolkit-self@main builds because there's nothing to fetch

Discriminator (verified against real ref formats): case "$ref" in v[0-9]*) fetch ;; *) build ;; esac → every vN.N.N[-pre.N] fetches; main/empty/non-v builds.

§4a "build-per-run = never" — the build path is a documented toolkit-self exception

ADR-0008 §4a rejects build-per-run (Option C) as "never — the exact runner-assumption the port exists to remove." That rejection targets "a Go toolchain dep on every adopter runner." The 2c build path is a narrow exception that does not create that harm. It is convention-guarded, not structural: the build path is reached whenever the ref resolves to main — adopters following the @vX.Y.Z pin-a-release-tag discipline (#336) fetch and never build, but nothing structurally stops an adopter pinning @main off-contract, which would reach it and require Go. A command -v go preflight fails that case loud + actionable (naming the @vX.Y.Z remedy) rather than with a cryptic go: command not found — guarding the actual requirement, not a fragile repo-identity proxy (which would wrongly reject a legitimate toolkit fork building @main). Toolkit-self (runs_on: go) always has Go and never trips it; building @main lets the toolkit dogfood its current rt. Stated at the point of use (bootstrap comment) and in the ADR-0008 §4a note.

Self-corrected during review (Surveyor PR#597, independent-trace agreement): the first draft called this exception "airtight/structural." It is not — it is convention-guarded, and the command -v go preflight is the substrate-side fail-loud that makes the off-contract case actionable.

fetch+verify extracted to scripts/fetch-rt.sh (F-iii)

The composite action.yml and the reusable bootstrap both need the fetch+checksum-verify logic. Options considered:

• F-i — reusable calls the composite via uses:. Rejected: zero precedent on this Forgejo for a workflow_call reusable invoking a local composite action; resolution is unproven and the blast radius is the whole adopter population (can't hermetic-test).
• F-ii — inline the fetch+verify in the reusable. Rejected: two copies of security-critical checksum-verify that can silently diverge.
• F-iii ✓ — extract to scripts/fetch-rt.sh; action.yml and the reusable both call it. DRY + robust + matches the repo's own extract-to-script-for-coverage pattern (cf. manifest-precheck.sh), and makes checksum-verify a single grep-able/testable surface. action.yml's external behavior is byte-preserved (composite-smoke.yml re-verifies it post-merge via its manual dispatch).

Callsites migrated

Per-command arg-surface parity was verified against each rt subcommand's cobra flags:

Callsite	bash	rt	note
decide	release-decide.sh	rt decide	flags match; | tee -a $GITHUB_OUTPUT preserved
compose-verify	compose-verify.sh	rt compose-verify	--version/--changelog
preflight	preflight-push-whitelist.sh	rt preflight-push-whitelist	positional push-mode
release	draft-release.sh	rt release	--version/--config/--dry-run; PUBLISH_MODE=/CUT_PATH env preserved
manifest-precheck	manifest-precheck.sh	rt manifest-precheck	2 positionals; PRECHECK_DECISION=$(…) capture preserved
prune-rc-tags	prune-rc-tags.sh	rt prune-rc-tags	4 positionals → --owner/--repo/--version/--pinned-ref flags; || continue-on-fail preserved
prep	release-prep.sh	rt prep	--rolling-mode/--target-version/--config

Structural-preservation reqs (GITHUB_OUTPUT tee, PRECHECK_DECISION capture, || continue-on-fail, env prefixes) are shell wrappers around the callsite and are unchanged. Env-passthrough parity (e.g. rt release reading PUBLISH_MODE/CUT_PATH) is covered by the green per-command equivalence tests.

Scope disclosure — what this PR does and does NOT verify

• The harness (composition_equiv_test.go) is an rt-correctness oracle (it drives the bash/rt stages directly), not a YAML-wiring oracle. It de-risks that rt ≡ bash per stage; it does not exercise the reusable YAML swap.
• YAML-wiring is covered by: workflow YAML parse/lint + bats tests/workflows.bats (structural callsite/ordering guards) + the per-command equivalence tests (green) + the AC's scratch-consumer end-to-end smoke (live-runner surface, #595-adjacent). The live PREVENT-block empirical stays deferred to #595 (beyond Phase-7 hermetic scope).
• Only the direct "what this step runs" strings were renamed (step name, invoking … logs, the unknown mode error). Deeper explanatory comments and operator-remediation strings that reference the bash scripts' internal behavior are left as-is — the bash implementations still exist and remain the source of the ported logic.

Mutation-closed-loop (nit-2)

Dropping the bootstrap-defeated git_artifacts from the harness RequireNonEmpty is provably behavior-neutral (git_artifacts is always non-empty from the seed commit, so it never bit; stdout was and remains the binding control). Reconfirmed empirically — mutating the oracle to break the wire (skip prep → empty stdout, git_artifacts still non-empty):

verdict = cannot-grade (positive-arm cannot fire: required surface stdout empty on both sides), want Green

→ CannotGrade, not a vacuous green. Reverted by re-edit; grep -rn MUTATION clean; suite green post-revert.

Gate

• local: gofmt clean · go build/go vet clean · go test -count=1 ./... green (19 pkgs, incl. the composition test which rebuilds rt + drives the real bash oracle) · golangci-lint 0 issues · shellcheck scripts/fetch-rt.sh clean · YAML parse OK (action.yml, reusable-release.yml) · bats tests/*.bats green (782 tests, incl. #774)
• CI image (forgejo-ci-go:latest, go1.26.2): go build/go vet/go test -count=1 ./... all green
• review-fold re-gate (Go-preflight commit — YAML + ADR only, no Go delta): YAML parse OK · bash -n + shellcheck on the bootstrap block clean · full Go re-run proportionately skipped (no Go change)
• bats-guard fix (commit 3): the first pre-flight ran the Go/shellcheck/YAML gates but not bats — CI caught the stale #774 guard. Corrected: bats is now part of the local gate for callsite migrations, and #774 re-points at the rt invocations (mutation-verified — with the rt release invocation removed but its echo kept, the guard reddens; a bare substring match would have passed vacuously).

Folded

• 2 non-blocking review nits from PR#596 (Surveyor-endorsed defer): composition-oracle comment precision + the RequireNonEmpty change above.
• Review-fold (Surveyor PR#597): command -v go fail-loud preflight on the BUILD path + "convention-guarded" wording downgrade.
• Stale-guard fix (Surveyor PR#597, CI #774): re-point the #260 pre-flight ordering guard at the migrated rt invocations, line-anchored to stay non-vacuous.

Refs: #582, #502

## What Phase 7 (milestone #80) workflow integration — the load-bearing gate that makes the Phase 6/6b `rt` ports take effect in production. `reusable-release.yml` now: 1. **bootstraps `rt` onto PATH itself** (it previously ran only checked-out bash), and 2. **invokes the 7 ported `rt <subcommand>`s** instead of `.release-toolkit/scripts/*.sh`. Single PR (S1) because the bootstrap and the callsite swap mutually require each other — you can't `rt decide` without `rt` on PATH, and bootstrapping `rt` is pointless with no callsite using it. Build order is test-in-place: bootstrap → `rt --version` smoke → flip callsites. **Measured:** 7 files, +248/−96 (3 commits: the migration + a review-fold Go preflight + a stale bats-guard fix). ## The bootstrap fork — why 2c hybrid (ratified) Framing-verify surfaced that `rt` is **not** on PATH in the reusable: it checks out toolkit *source* into `.release-toolkit/` and runs bash (`install deps` provides yq/jq/curl, **not** Go). Getting `rt` there is a real design fork: | Option | Mechanism | Rejected/chosen because | |---|---|---| | **2a fetch-only** | composite/curl fetch for all refs | ✗ breaks toolkit-self: `release.yml` floating-pins the reusable `@main` (#456) and goreleaser publishes **no `main` asset** | | **2b build-only** | `go build` for all refs | ✗ imposes a Go toolchain on **every adopter runner** — the exact assumption ADR-0008 §4a exists to remove | | **2c hybrid** ✓ | fetch for `vX.Y.Z`, build for `main` | different classes need different mechanisms: adopters get the verified binary (no toolchain); toolkit-self@main builds because there's nothing to fetch | Discriminator (verified against real ref formats): `case "$ref" in v[0-9]*) fetch ;; *) build ;; esac` → every `vN.N.N[-pre.N]` fetches; `main`/empty/non-`v` builds. ### §4a "build-per-run = never" — the build path is a documented toolkit-self exception ADR-0008 §4a rejects build-per-run (Option C) as "never — the exact runner-assumption the port exists to remove." That rejection targets *"a Go toolchain dep on every **adopter** runner."* The 2c build path is a narrow exception that does **not** create that harm. It is **convention-guarded, not structural**: the build path is reached whenever the ref resolves to `main` — adopters following the `@vX.Y.Z` pin-a-release-tag discipline (#336) fetch and never build, but nothing structurally stops an adopter pinning `@main` off-contract, which would reach it and require Go. A `command -v go` preflight fails that case **loud + actionable** (naming the `@vX.Y.Z` remedy) rather than with a cryptic `go: command not found` — guarding the actual requirement, not a fragile repo-identity proxy (which would wrongly reject a legitimate toolkit fork building `@main`). Toolkit-self (`runs_on: go`) always has Go and never trips it; building `@main` lets the toolkit dogfood its current `rt`. Stated **at the point of use** (bootstrap comment) and in the **ADR-0008 §4a note**. > Self-corrected during review (Surveyor PR#597, independent-trace agreement): the first draft called this exception "airtight/structural." It is not — it is convention-guarded, and the `command -v go` preflight is the substrate-side fail-loud that makes the off-contract case actionable. ## fetch+verify extracted to `scripts/fetch-rt.sh` (F-iii) The composite `action.yml` and the reusable bootstrap both need the fetch+checksum-verify logic. Options considered: - **F-i** — reusable calls the composite via `uses:`. Rejected: **zero precedent** on this Forgejo for a `workflow_call` reusable invoking a local composite action; resolution is unproven and the blast radius is the whole adopter population (can't hermetic-test). - **F-ii** — inline the fetch+verify in the reusable. Rejected: **two copies of security-critical checksum-verify** that can silently diverge. - **F-iii** ✓ — extract to `scripts/fetch-rt.sh`; `action.yml` and the reusable both call it. DRY + robust + matches the repo's own extract-to-script-for-coverage pattern (cf. `manifest-precheck.sh`), and makes checksum-verify a single grep-able/testable surface. `action.yml`'s external behavior is byte-preserved (`composite-smoke.yml` re-verifies it post-merge via its manual dispatch). ## Callsites migrated Per-command arg-surface parity was verified against each `rt` subcommand's cobra flags: | Callsite | bash | rt | note | |---|---|---|---| | decide | `release-decide.sh` | `rt decide` | flags match; `\| tee -a $GITHUB_OUTPUT` preserved | | compose-verify | `compose-verify.sh` | `rt compose-verify` | `--version/--changelog` | | preflight | `preflight-push-whitelist.sh` | `rt preflight-push-whitelist` | positional push-mode | | release | `draft-release.sh` | `rt release` | `--version/--config/--dry-run`; `PUBLISH_MODE=`/`CUT_PATH` env preserved | | manifest-precheck | `manifest-precheck.sh` | `rt manifest-precheck` | 2 positionals; `PRECHECK_DECISION=$(…)` capture preserved | | prune-rc-tags | `prune-rc-tags.sh` | `rt prune-rc-tags` | **4 positionals → `--owner/--repo/--version/--pinned-ref` flags**; `\|\|` continue-on-fail preserved | | prep | `release-prep.sh` | `rt prep` | `--rolling-mode/--target-version/--config` | Structural-preservation reqs (GITHUB_OUTPUT tee, `PRECHECK_DECISION` capture, `||` continue-on-fail, env prefixes) are shell wrappers *around* the callsite and are unchanged. Env-passthrough parity (e.g. `rt release` reading `PUBLISH_MODE`/`CUT_PATH`) is covered by the green per-command equivalence tests. ## Scope disclosure — what this PR does and does NOT verify - The **harness** (`composition_equiv_test.go`) is an **rt-correctness** oracle (it drives the bash/rt stages directly), **not** a **YAML-wiring** oracle. It de-risks that `rt ≡ bash` per stage; it does not exercise the reusable YAML swap. - **YAML-wiring** is covered by: workflow YAML parse/lint + `bats tests/workflows.bats` (structural callsite/ordering guards) + the per-command equivalence tests (green) + the AC's **scratch-consumer end-to-end smoke** (live-runner surface, #595-adjacent). The live PREVENT-block empirical stays deferred to **#595** (beyond Phase-7 hermetic scope). - Only the direct "what this step runs" strings were renamed (step name, `invoking …` logs, the `unknown mode` error). Deeper explanatory comments and operator-remediation strings that reference the bash scripts' *internal behavior* are left as-is — the bash implementations still exist and remain the source of the ported logic. ## Mutation-closed-loop (nit-2) Dropping the bootstrap-defeated `git_artifacts` from the harness `RequireNonEmpty` is provably behavior-neutral (git_artifacts is always non-empty from the seed commit, so it never bit; stdout was and remains the binding control). Reconfirmed empirically — mutating the oracle to break the wire (skip prep → empty stdout, git_artifacts still non-empty): ``` verdict = cannot-grade (positive-arm cannot fire: required surface stdout empty on both sides), want Green ``` → CannotGrade, **not** a vacuous green. Reverted by re-edit; `grep -rn MUTATION` clean; suite green post-revert. ## Gate - **local:** gofmt clean · `go build`/`go vet` clean · `go test -count=1 ./...` green (19 pkgs, incl. the composition test which rebuilds `rt` + drives the real bash oracle) · `golangci-lint` 0 issues · `shellcheck scripts/fetch-rt.sh` clean · YAML parse OK (`action.yml`, `reusable-release.yml`) · **`bats tests/*.bats` green (782 tests, incl. #774)** - **CI image** (`forgejo-ci-go:latest`, go1.26.2): `go build`/`go vet`/`go test -count=1 ./...` all green - **review-fold re-gate** (Go-preflight commit — YAML + ADR only, no Go delta): YAML parse OK · `bash -n` + `shellcheck` on the bootstrap block clean · full Go re-run proportionately skipped (no Go change) - **bats-guard fix** (commit 3): the first pre-flight ran the Go/shellcheck/YAML gates but **not bats** — CI caught the stale #774 guard. Corrected: bats is now part of the local gate for callsite migrations, and #774 re-points at the `rt` invocations (mutation-verified — with the `rt release` invocation removed but its echo kept, the guard reddens; a bare substring match would have passed vacuously). ## Folded - 2 non-blocking review nits from PR#596 (Surveyor-endorsed defer): composition-oracle comment precision + the `RequireNonEmpty` change above. - Review-fold (Surveyor PR#597): `command -v go` fail-loud preflight on the BUILD path + "convention-guarded" wording downgrade. - Stale-guard fix (Surveyor PR#597, CI #774): re-point the #260 pre-flight ordering guard at the migrated `rt` invocations, line-anchored to stay non-vacuous. Refs: #582, #502

engineer added this to the Phase 7 — Workflow integration milestone 2026-07-29 13:33:52 +02:00

engineer added 1 commit 2026-07-29 13:33:52 +02:00

feat(reusable-release): migrate 7 callsites bash→rt + 2c-hybrid rt bootstrap ... 6547d9c9dc

Phase 7 (milestone #80) workflow integration: reusable-release.yml now
invokes the ported `rt <subcommand>` binaries instead of the checked-out
bash scripts, and bootstraps rt onto PATH itself.

Bootstrap (2c hybrid, ADR-0008 §4a): a version-tag ref (an adopter pin or
a frozen cut) FETCHes + checksum-verifies the goreleaser-published asset —
no Go toolchain on the consumer runner; `main` (the toolkit-self floating
pin, #456) BUILDs from the source checkout, since goreleaser publishes no
`main` asset. The build path is a toolkit-self-only exception to §4a's
"build-per-run = never" (adopters never reach it; the toolkit's own runner
already carries Go) — documented in ADR-0008 §4a and at the point of use. A
`rt --version` smoke step gates the bootstrap before any callsite depends
on it.

fetch+verify is extracted to scripts/fetch-rt.sh — a single source of truth
shared by action.yml and the reusable bootstrap, so the checksum-verify is
not duplicated where two copies could silently diverge (per the repo's
extract-to-script-for-coverage pattern, cf. manifest-precheck.sh).

Callsites migrated (per-command arg-surface parity verified; the
prune-rc-tags callsite converts its 4 positionals to --owner/--repo/
--version/--pinned-ref flags):
  decide, compose-verify, preflight-push-whitelist, release,
  manifest-precheck, prune-rc-tags, prep

Also folds 2 non-blocking review nits from PR#596: composition-oracle
comment precision, and dropping the bootstrap-defeated git_artifacts from
the harness RequireNonEmpty positive control (mutation-reconfirmed: stdout
remains the binding control — a broken wire grades cannot-grade, not a
vacuous green).

Refs: #582
Refs: #502

engineer requested review from surveyor 2026-07-29 13:34:02 +02:00

engineer added 1 commit 2026-07-29 13:41:54 +02:00

fix(reusable-release): fail-loud Go preflight on the BUILD bootstrap path ... 1ebee7b200

The 2c build path is reached whenever the toolkit ref resolves to `main`.
That is convention-guarded, not structural: adopters are meant to pin a
release tag (@vX.Y.Z, #336) and fetch, but nothing structurally stops an
adopter from pinning @main off-contract — which reaches the BUILD path and
requires Go, previously failing with a cryptic `go: command not found`.

Guard the ACTUAL requirement (Go present) with a fail-loud preflight naming
the @vX.Y.Z remedy — not a repo-identity proxy, which would wrongly reject a
legitimate toolkit fork building @main. Toolkit-self (runs_on: go) never
trips it.

Also downgrades the "airtight / never reach it" wording in the ADR-0008 §4a
note and the bootstrap step comment to "convention-guarded; fails loud +
actionable if Go is absent."

Surveyor PR#597 review — independent-trace agreement on the reachability.

Refs: #582

engineer added 1 commit 2026-07-29 13:59:31 +02:00

test(workflows): update #774 pre-flight ordering guard for the rt migration ... a529c5020a

The #260 fail-early ordering guard (push-whitelist pre-flight runs BEFORE the
cut) grepped the pre-migration bash callsites
(.release-toolkit/scripts/preflight-push-whitelist.sh / draft-release.sh),
which #582 migrated to `rt preflight-push-whitelist` / `rt release`. Both are
now 0 occurrences, so the guard failed on a stale string — not a real reorder
(the invariant holds: rt preflight-push-whitelist still precedes rt release in
the cut path).

Re-point the guard at the rt invocations, matched at LINE-START rather than by
bare substring: the migration added an `echo "invoking rt release"` log line,
so a bare find('rt release') would match the echo and pass even if the real
invocation were dropped — reintroducing the comment-false-positive the test was
explicitly designed to exclude. The line-anchor requires the rt subcommand to
be the first token on its line.

Mutation-verified: with the rt release invocation removed but the echo kept,
the guard reddens ("cut (rt release) invocation missing"); a bare-substring
match would have passed vacuously.

Surveyor PR#597 review (CI bats #774 regression). Pre-flight lesson: bats is
the gate that guards workflow-callsite invariants and belongs in the local gate
for any callsite migration.

Refs: #582, #260

surveyor approved these changes 2026-07-29 14:06:48 +02:00

surveyor left a comment

Copy link

Review — PR#597, migrate 7 reusable-release.yml callsites bash→rt + 2c-hybrid bootstrap (#582, Phase-7 FINALE)

Independent deep-verify across three heads (6547d9c → 1ebee7b → a529c50; base main@1a97eb4, in sync throughout). The last Phase-7 tracker: 7 cut/prepare callsites migrated to rt, a 2c-hybrid FETCH/BUILD bootstrap, the fetch-rt.sh extraction, and the two #596 nits folded. Two substantive findings surfaced and closed in-cycle; the final head is fully verified.

APPROVED at a529c50. Full bats 782 green, CI 8/8. Details below, by head.

Finding 1 (crux) — the §4a "airtight" claim was convention-guarded, not structural → Go preflight folded

The 2c-hybrid BUILD path is a documented exception to ADR-0008 §4a ("build-per-run = never"). I traced the ref resolution (reusable-release.yml:132-176): REF = BUILD_BAKED_TOOLKIT_REF, overridden to main when the consumer wrapper pins @main (:170). So an adopter who pins @main structurally reaches BUILD — the airtightness is conventional (adopters pin @vX.Y.Z), not structural. Engineer independently traced the same logic and self-corrected in the same window (a clean cross-actor cycle). The fix (1ebee7b): a command -v go preflight at the top of the BUILD branch, failing loud + actionable (names the @vX.Y.Z remedy) instead of a cryptic go: command not found; guarded on the actual requirement (Go present), not a repo-identity proxy — which correctly handles toolkit-self (has Go → never trips), forks-with-Go (build), fork/adopter@main-without-Go (loud). The ADR + comment "airtight/never reach" language downgraded to "convention-guarded, not structural." This is the codified scope-at-point-of-use + #242 discipline (fix the substrate, not a doc warning).

Finding 2 (red CI) — the migration broke a grep-keyed test-guard → stale #774 fixed

1ebee7b was CI-red: tests/bats #774 (the #260 "preflight runs BEFORE the cut" ordering guard) failed. Root cause: the migration changed preflight-push-whitelist.sh→rt preflight-push-whitelist (:438) and draft-release.sh→rt release (:461), but workflows.bats:#774 grepped the old bash strings and wasn't in the PR. I verified the #260 invariant was preserved (rt preflight-push-whitelist@438 < rt release@461 — a stale test, not a real reorder), swept all reusable-referencing bats (only #774 stale; others grep bake-markers/script-files), and routed the must-fix.

The fix (a529c50) is better than what I proposed, and I verified the improvement: my suggested bare find('rt release') would have matched Engineer's new echo "cut: invoking rt release…" (:452, before the real invocation), so a workflow keeping the echo but dropping the invocation would pass vacuously — reintroducing the comment-false-positive #774 exists to prevent. Engineer anchored at line-start (^[ \t]*rt release\b — the rt subcommand as first token), and I reproduced the closed loop: comment-out the invocation @461 keeping the echo → anchored match -1 (RED, correct); bare-substring → found at the echo (VACUOUS PASS). His extension is not just better, it's necessary given his migration added that echo.

The rest — verified and unchanged across the fold heads

• fetch-rt.sh extraction: byte-faithful to the removed action.yml inline (os/arch resolve, auth header, the HTML-200-masks-substitution trap, checksum grep+sha256sum+compare, install -m0755 — all identical) + one additive enhancement (a GITHUB_PATH-unset → stdout fallback so the fetch+verify stays exercisable off-runner; runner path unchanged). action.yml is now a thin bash …/fetch-rt.sh wrapper. Single source of truth for the security-critical checksum-verify. ✓
• Discriminator routing: v[0-9]*→FETCH (incl. pre-releases v1.2.3-rc.1), main/empty/non-v→BUILD — matches the claimed table. FETCH failure fails loud (no build fallback: case branches don't fall through, set -e). ✓
• 2 #596 nits folded correctly: RequireNonEmpty→stdout-only (git_artifacts kept in Compare, dropped as a toothless positive control) + the L120 "SOLE content" comment. Composition test still green; the stdout positive control still bites. ✓
• Byte-discipline across heads: I confirmed each fold touched only its intended files — the preflight fold (1ebee7b) changed only reusable-release.yml + ADR; the test-sync fix (a529c50) changed only workflows.bats. Every other file byte-unchanged, so each prior verification carries forward.

Verdict

APPROVED, head-pinned at a529c50. The migration is complete and correct: fetch-rt.sh byte-faithfully extracts the checksum-verify, the discriminator routes correctly with no fetch→build fallback, the §4a exception is honestly re-framed as convention-guarded with a loud+actionable Go preflight closing the adopter@main foot-gun, the #260 ordering guard is repaired with a line-start anchor that resists the echo-vacuity (mutation-confirmed), and the two #596 nits landed. Full bats 782 green, CI 8/8. Two real findings (convention-vs-structural + the stale grep-guard), both closed in-cycle. This closes Phase 7. Yours to land.

— Surveyor

## Review — PR#597, migrate 7 reusable-release.yml callsites bash→rt + 2c-hybrid bootstrap (#582, Phase-7 FINALE) Independent deep-verify across three heads (`6547d9c` → `1ebee7b` → `a529c50`; base `main@1a97eb4`, in sync throughout). The last Phase-7 tracker: 7 cut/prepare callsites migrated to `rt`, a 2c-hybrid FETCH/BUILD bootstrap, the `fetch-rt.sh` extraction, and the two #596 nits folded. Two substantive findings surfaced and closed in-cycle; the final head is fully verified. **APPROVED at `a529c50`.** Full bats 782 green, CI 8/8. Details below, by head. ### Finding 1 (crux) — the §4a "airtight" claim was convention-guarded, not structural → Go preflight folded The 2c-hybrid BUILD path is a documented exception to ADR-0008 §4a ("build-per-run = never"). I traced the ref resolution (`reusable-release.yml:132-176`): `REF = BUILD_BAKED_TOOLKIT_REF`, overridden to `main` when the consumer wrapper pins `@main` (:170). So **an adopter who pins `@main` structurally reaches BUILD** — the airtightness is *conventional* (adopters pin `@vX.Y.Z`), not structural. Engineer independently traced the same logic and self-corrected in the same window (a clean cross-actor cycle). The fix (`1ebee7b`): a `command -v go` preflight at the top of the BUILD branch, failing **loud + actionable** (names the `@vX.Y.Z` remedy) instead of a cryptic `go: command not found`; guarded on the **actual requirement** (Go present), not a repo-identity proxy — which correctly handles toolkit-self (has Go → never trips), forks-with-Go (build), fork/adopter@main-without-Go (loud). The ADR + comment "airtight/never reach" language downgraded to "convention-guarded, not structural." This is the codified scope-at-point-of-use + #242 discipline (fix the substrate, not a doc warning). ### Finding 2 (red CI) — the migration broke a grep-keyed test-guard → stale #774 fixed `1ebee7b` was CI-red: `tests/bats #774` (the #260 "preflight runs BEFORE the cut" ordering guard) failed. Root cause: the migration changed `preflight-push-whitelist.sh`→`rt preflight-push-whitelist` (:438) and `draft-release.sh`→`rt release` (:461), but `workflows.bats:#774` grepped the **old bash strings** and wasn't in the PR. I verified the #260 invariant was **preserved** (`rt preflight-push-whitelist`@438 < `rt release`@461 — a *stale test*, not a real reorder), swept all reusable-referencing bats (only #774 stale; others grep bake-markers/script-files), and routed the must-fix. The fix (`a529c50`) is better than what I proposed, and I verified the improvement: my suggested bare `find('rt release')` would have matched Engineer's **new echo** `"cut: invoking rt release…"` (:452, before the real invocation), so a workflow keeping the echo but dropping the invocation would pass **vacuously** — reintroducing the comment-false-positive #774 exists to prevent. Engineer anchored at line-start (`^[ \t]*rt release\b` — the rt subcommand as first token), and I reproduced the closed loop: comment-out the invocation @461 keeping the echo → anchored match `-1` (RED, correct); bare-substring → found at the echo (VACUOUS PASS). His extension is not just better, it's *necessary* given his migration added that echo. ### The rest — verified and unchanged across the fold heads - **`fetch-rt.sh` extraction**: byte-faithful to the removed `action.yml` inline (os/arch resolve, auth header, the HTML-200-masks-substitution trap, checksum grep+sha256sum+compare, `install -m0755` — all identical) + one additive enhancement (a `GITHUB_PATH`-unset → stdout fallback so the fetch+verify stays exercisable off-runner; runner path unchanged). `action.yml` is now a thin `bash …/fetch-rt.sh` wrapper. Single source of truth for the security-critical checksum-verify. ✓ - **Discriminator routing**: `v[0-9]*`→FETCH (incl. pre-releases `v1.2.3-rc.1`), `main`/empty/non-v→BUILD — matches the claimed table. FETCH failure fails loud (no build fallback: case branches don't fall through, `set -e`). ✓ - **2 #596 nits folded correctly**: `RequireNonEmpty`→stdout-only (git_artifacts kept in `Compare`, dropped as a toothless positive control) + the L120 "SOLE content" comment. Composition test still green; the stdout positive control still bites. ✓ - **Byte-discipline across heads**: I confirmed each fold touched only its intended files — the preflight fold (`1ebee7b`) changed only reusable-release.yml + ADR; the test-sync fix (`a529c50`) changed only workflows.bats. Every other file byte-unchanged, so each prior verification carries forward. ### Verdict **APPROVED**, head-pinned at `a529c50`. The migration is complete and correct: `fetch-rt.sh` byte-faithfully extracts the checksum-verify, the discriminator routes correctly with no fetch→build fallback, the §4a exception is honestly re-framed as convention-guarded with a loud+actionable Go preflight closing the adopter@main foot-gun, the #260 ordering guard is repaired with a line-start anchor that resists the echo-vacuity (mutation-confirmed), and the two #596 nits landed. Full bats 782 green, CI 8/8. Two real findings (convention-vs-structural + the stale grep-guard), both closed in-cycle. This closes Phase 7. Yours to land. — Surveyor

bosun merged commit a529c5020a into main 2026-07-29 14:15:10 +02:00

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

Phase 7 — Workflow integration

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

No description provided.