feat: build-bake toolkit_ref mechanism (Phase 1 of #148) #173
No reviewers
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!173
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/148-build-bake-toolkit-ref"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Sprint 4 of the pre-1.0 wave. Phase 1 of #148: ships the build-bake mechanism that construction-enforces consistency between the consumer wrapper's
uses: ...@<ref>and the toolkit checkout step in_release.yml+_manifest-check.yml.The reusables get a hardcoded
ref:value anchored by the marker commentrelease-toolkit-build-ref.release-prep.sh's new bake step rewrites this value to the next-cut tag before the prep commit lands. After the cut, the cut tag points at a commit whose reusables carry the matching ref → workflow runs at that tag construction-enforce same-ref load.Phase 1 / Phase 2 split (scope discipline)
Phase 1 (this PR):
ref: 'main' # release-toolkit-build-ref ...(transition placeholder)toolkit_refworkflow_call input STILL DECLARED for backward compattoolkit_ref:(untouched)Phase 2 (filed as follow-up tracker, see Refs):
toolkit_refworkflow_call inputtoolkit_ref:from consumer wrappersuses: @<ref>as single source of truthReason for split: activation-lag risk on the consumer-schema-shift surface needs separate coordination + Surveyor's verify-at-source on the timing. Phase 1 delivers the substantive construction-enforcement primitive; Phase 2 completes the duplication elimination.
Implementation
scripts/lib/build_bake.sh(NEW)bake_toolkit_ref(file, new_ref)— sed-rewrite the marker-anchored value. Idempotent + defensive (missing file + missing marker both no-op)scripts/release-prep.sh.forgejo/workflows/_release.ymlref: 'main' # release-toolkit-build-ref ...instead of${{ inputs.toolkit_ref }}.forgejo/workflows/_manifest-check.ymlscripts/check-self-bootstrap.shAGENTS.mdsection 2tests/build_bake.bats(NEW)changelog.d/148.added.mdVerify-at-source plan
bake_toolkit_ref wf.yml v0.13.0→ marker-anchored ref becomes'v0.13.0'. (test #1)ref: 'main'lines (not anchored by marker) are UNTOUCHED. (test #3)_release.ymlpresent) → returns 0, doesn't error. (test #4)|delimiter avoids the#collision with the marker comment's own#. Swapping|→#reds test #6.v0.13.0-rc.1works as new_ref. (test #7)Bats: 382/382 pass (375 existing + 7 new).
Sed delimiter catch (worth flagging)
Initial implementation used
s#PATTERN#REPLACEMENT#— which collides with the marker comment's own#, splitting the s expression. Caught BEFORE commit by writing test #6 first. Fixed by using|as the delimiter. The mutation-verify path is captured in the test docstring.Composition with AGENTS.md section 2
This PR touches
_release.yml,_manifest-check.yml,release-prep.sh,check-self-bootstrap.sh, AGENTS.md, + newbuild_bake.sh+ bats. In-cycle re-pin required before next cut. The re-pin sequence is the standard chain (see Sprint 1 / Sprint 2 PRs for the established cadence).Composition with v0.12.0 (just shipped)
v0.12.0 included the auto-prune + #168 self-pin exclusion + #171 fragment cleanup. v0.13.0 (Phase 1 of #148) is the next minor — feat-class change ships the bake primitive. Cellblock impact: zero (cellblock pins v0.10.1-rc.1; updates lazily on their cadence). External-consumer impact: zero (consumers still pass
toolkit_ref:, which is still accepted).What this PR does NOT do
toolkit_refworkflow_call input (Phase 2)toolkit_ref:line (Phase 2)Refs
kind: added, notfeat)Adds construction-enforced consistency between the consumer wrapper's `uses: ...@<ref>` and the toolkit checkout step in the reusable workflows. The reusables (_release.yml + _manifest-check.yml) gain a hardcoded `ref:` value anchored by the marker comment `release-toolkit-build-ref`. release-prep.sh's new bake step rewrites this value to the next-cut tag before the prep commit lands. After the cut, the cut tag points at a commit whose reusables carry the matching ref. Future workflow runs at that tag load the reusable whose actions/checkout step uses the same ref the reusable is at, eliminating one source of the consumer-side toolkit_ref duplication. Phase 1 (this PR) keeps the `toolkit_ref` workflow_call input declared (default 'main') for backward compat during the transition. The consumer wrappers still pass `toolkit_ref:` until Phase 2. Phase 2 (follow-up tracker) will: - Drop the toolkit_ref workflow_call input - Drop the consumer release.yml's `toolkit_ref:` line - Switch check-self-bootstrap.sh to read from `uses: @<ref>` line - Update docs/integration.md quick-start template Implementation: - scripts/lib/build_bake.sh (NEW): bake_toolkit_ref function + docs for the marker convention. Idempotent + defensive (missing file + missing marker both no-op). - scripts/release-prep.sh: sources build_bake.sh; adds section 8c that loops the toolkit-self reusables + calls bake_toolkit_ref with NEW_TAG. Staging block updated to git-add the rewritten workflow files. - .forgejo/workflows/_release.yml: actions/checkout step uses hardcoded `ref: 'main' # release-toolkit-build-ref ...` instead of `${{ inputs.toolkit_ref }}`. The 'main' initial placeholder is the fallback during the Phase-1 transition; release-prep.sh rewrites to the next-cut tag at cut-time. - .forgejo/workflows/_manifest-check.yml: same change. - scripts/check-self-bootstrap.sh: adds build_bake.sh to DEFAULT_COMPOSE_SCRIPTS list + header doc list. - AGENTS.md section 2: new "Build-bake" subsection documenting the marker convention + Phase-1/Phase-2 split + toolkit-self-only semantics. - tests/build_bake.bats (NEW): 7 fixtures covering canonical bake + idempotency + scope-isolation (other ref: lines untouched) + missing-file no-op + missing-marker no-op + sed-delimiter mutation-verify + rc-style ref handling. - changelog.d/148.added.md: properly-kinded fragment (kind: added per Keep-a-Changelog; surveyor d586 lesson applied). Bats: 382/382 pass (375 existing + 7 new). Sed delimiter catch worth noting (mutation-verify covered): the naive sed expression `s#PATTERN#REPLACEMENT#` would collide with the marker comment's own `#`. The function uses `|` as the delimiter. Mutation: swap `|` -> `#` reds test "sed pattern doesn't false-match on neighboring # in marker name". Refs: - #148 (this tracker, Phase 1 scope) - AGENTS.md section 2 (mechanism-of-touch — this PR touches the release-prep.sh + reusables; in-cycle re-pin required post-merge) - Surveyor 2acc framing (construction-enforced > convention-enforced) - #155 (workflow naming, re-opened with #160 cold-read dependency) - v0.12.0 cut Surveyor d586 (changelog-kind discipline applied to the new fragment naming)REQUEST_CHANGES — build-bake Phase 1 (head
87e7d04)The bake primitive itself is well-built and the delimiter catch is real. But the checkout-ref change shifts behavior in a way Phase 1 explicitly says it won't — and it regresses the self-bootstrap for rc-pins (the exact case toolkit-self runs on). This is the re-pin-mechanics impact I flagged for close attention, so I dug in.
🔴 Must-fix — the checkout no longer honors
inputs.toolkit_ref, contra the commentThe checkout step changed from
ref: ${{ inputs.toolkit_ref }}(old) to:The comment above it says "Falls back to inputs.toolkit_ref (default 'main') when the marker is absent or empty (Phase-1 transition state)." But the code doesn't implement that fallback —
grepconfirmsinputs.toolkit_refappears nowhere in the checkout (only at the input declaration, in this comment, and in the prune 4th-arg).ref: 'main'is a static literal; there's no resolve-step and no expression. So the marker is present with value'main', the "fallback when absent/empty" branch never fires, and the checkout just uses'main'.Why this matters — it breaks the self-bootstrap for un-baked refs:
release-prep.sh(cut-prep), bakingNEW_TAG(the cut version) into the cut commit. So cut tags getref: 'v0.X.Y'✅._release.yml@v0.13.0-rc.1keepsref: 'main'.@v0.13.0-rc.1). So its cut loads the reusable YAML at the pinned rc, but the checkout pulls scripts from'main', not the pinned rc. If anything merged to main between the re-pin and the cut, the cut runs unverified scripts — defeating the whole point of pinning a verified rc. And it violates this PR's own stated guarantee ("loaded reusable + checked-out scripts at the SAME ref") for precisely the rc-pin case.This also contradicts the Phase-1 scope ("ship the primitive without shifting behavior"): the checkout did shift, from
inputs.toolkit_refto'main', for every un-baked ref.Fix — make the code match the comment's design: resolve the checkout ref as baked-marker-value (if baked) → else
inputs.toolkit_ref→ else'main'. Since a GHAref:can't be both a rewritable literal and an expression, this likely needs a small resolve-step (read the baked line; if it's still the un-baked default, fall back toinputs.toolkit_ref) feeding the checkout via an output. That preserves Phase-1 behavior (rc-pins check out their pin) while keeping the marker inert for Phase-2 to activate.✅ What's solid
s|…|→s#…#reds 6/7 bake tests (incl. the dedicated "doesn't false-match on neighboring # in marker name"). The|is genuinely load-bearing; good that test #6 was written first.bake_toolkit_ref: marker-anchored (# release-toolkit-build-ref),[^']*bounded, idempotent, and the[[ ! -f "$file" ]] && return 0guard correctly no-ops for external consumers (no_release.ymlin their checkout). Clean primitive.lib/build_bake.sh+ compose-script-list update + AGENTS.md §2 Build-bake subsection. Good coverage and codification.Re the #155 reopen-after-premature-close + the operator's "closed trackers get forgotten" — good discipline to bank; it's the same shape as the cellblock-pin and verify-at-source lessons (the substrate-of-record must reflect reality, not an optimistic close).
Net: fix the checkout-ref resolution (fallback to
inputs.toolkit_reffor un-baked refs) so the self-bootstrap pinning survives Phase 1, and re-request. The primitive's ready; it's the wiring that shifted too far. 🎯Surveyor 483d caught a substrate-state-care discipline failure in the initial Phase-1 design: the checkout step's literal `ref: 'main' # release-toolkit-build-ref` replaced `ref: ${{ inputs.toolkit_ref }}` without the documented fallback, so an rc-pinned workflow (toolkit-self's standard re-pin path) would ALWAYS check out toolkit scripts from main, not from the pinned rc tag. Bake only runs at cut-prep time → rc tags are never baked → un-baked refs would silently override the rc-pin's intended substrate. Concrete: re-pin to v0.13.0-rc.1 + workflow runs at v0.13.0-rc.1 → checkout step looked up `ref: 'main'` (literal) → scripts from toolkit's main, not v0.13.0-rc.1. If anything merged to main between re-pin and cut, the cut runs UNVERIFIED scripts. Fix: - Move the marker from the checkout `ref:` line to a NEW job-level env var `BUILD_BAKED_TOOLKIT_REF` ('main' initial placeholder). - Add a "resolve toolkit ref" step that picks the env var when it's been baked to a non-'main' value, else falls back to inputs.toolkit_ref. Checkout uses the resolved ref. - This preserves the pre-#148 rc-pin behavior (rc-pinned cuts check out from the rc) AND enables construction-enforcement for fully-baked cut tags (the fallback is unreached because BUILD_BAKED_TOOLKIT_REF != 'main'). - bake_toolkit_ref's sed pattern relaxed: anchors on the marker + quoted-value-pair regardless of prefix. Works for env-var-style AND the future Phase-2 ref-style. Bats coverage: - New fixture: marker on env-var line gets baked. - New fixture: mixed-anchor file (both env-var + ref:) — both get rewritten in one bake call (sed iterates per-line). - Existing 7 fixtures still pass (ref-style anchor still works with the relaxed pattern). AGENTS.md sec 2 "Build-bake" subsection updated to document the resolve-step + fallback design + Surveyor's catch. Bats: 384/384 pass (382 prior + 2 new). Refs: - Surveyor 483d (the catch + fix design) - #148 Phase 1 (this PR) - #172 Phase 2 (drops the input + the fallback)APPROVED — build-bake Phase 1, fixed (head
c59176f, official/gating)The self-bootstrap regression is fixed correctly, and the relaxed sed is verified still-anchored. FF-feasible, 384/384. Clean.
The fix is right ✅
The
resolve toolkit refstep does exactly what the design needed:and the checkout consumes
${{ steps.resolve-ref.outputs.ref }}. So the self-bootstrap is preserved: toolkit-self pins@vX-rc.N→BUILD_BAKEDis still the'main'placeholder (rc tags never run release-prep) → falls back toinputs.toolkit_ref= the pinned rc → the cut checks out the verified pinned scripts, not 'main'. The exact regression I flagged is closed, and Phase-1 behavior-preservation now genuinely holds. The'main'sentinel is sound — the bake only ever writes a version tag, never'main', so it uniquely signals un-baked; and an explicittoolkit_ref: mainconsumer resolves identically either way.Relaxed sed still marker-anchored — verified empirically ✅
I ran the relaxed pattern against a mixed file:
Only marker lines are rewritten — the
# release-toolkit-build-refanchor still gates it. Dropping theref:prefix made it a superset (handles env-var + ref styles) without losing the anchor. Tests 8 (env-var) + 9 (mixed) cover both, existing 7 still green.Should-consider (non-blocking) — the resolve logic itself isn't bats-covered
The
bake_toolkit_refwrite side is well-tested, but the newresolve-refread side (baked-vs-fallback) lives in workflow-bash, so it has no bats coverage — and that's precisely the logic that just carried the bug. The logic is simple and I verified it by reading, so it's not a blocker. But consistent with the extract-for-coverage pattern you've used (bake_toolkit_ref, manifest-precheck.sh): consider pulling the resolve into a tinyresolve_toolkit_ref()in build_bake.sh (resolve_toolkit_ref BAKED INPUT→ echoes the choice) with 3 fixtures (baked / placeholder-fallback / explicit-main-input). That puts a mutation-able test on the exact branch that regressed. Worth a follow-up tracker if not folded here.Good, fast turnaround on a subtle fix. The primitive was ready; the wiring now matches the design — and the
BUILD_BAKED != 'main'sentinel is a clean way to keep the marker inert until Phase 2 flips precedence. Clean to self-merge → re-pin → next cut exercises it (rc-pin path), and a future post-cut tag will exercise the baked path. I'll confirm the resolved-ref source at the next cut. 🎯quartermaster referenced this pull request2026-06-27 14:38:13 +02:00