fix(selfboot): normalize staged reusables to 'main' markers (#599) #600

Merged

bosun merged 1 commit from i/599-selfboot-normalize-main-markers into main 2026-07-29 23:08:14 +02:00

Conversation 1 Commits 1 Files changed 2 +35 -3

engineer commented 2026-07-29 22:54:52 +02:00

Owner

Copy link

What

TestRoundTrip_ThreeActorBootstrap (internal/selfboot, the milestone-#77 gate) fails go-ci on every release-prep PR — first surfaced on PR#552 chore(release): v0.34.0:

round-trip identity broken (go):   reset 9a55ecc… != baseline ebd9070…
round-trip identity broken (bash): reset 9a55ecc… != tree0    ebd9070…

stageReusables now normalizes each staged reusable's build-ref marker to 'main' via bake's own byte-exact kernel, decoupling the test baseline from the transient release-prep pin.

Root cause (verified empirically, not reasoned)

stageReusables stages the 5 real reusables from the live working tree (../../.forgejo/workflows/). The identity assertion — bake('main', PhaseMainReset) is a no-op → reset.TreeSHA == tree0 (roundtrip_test.go:96) — holds only when the working tree is at the 'main' marker state.

release-prep.sh pins those markers 'main' → 'vX.Y.Z' on every chore(release): prepare PR (the pin→reset oscillation; post-cut bookkeeping [skip ci] resets main back). On a release-prep branch the baseline is vX.Y.Z-pinned while bake('main') rewrites to 'main' → trees diverge → the "no-op" premise is violated by the setup, not by the code under test.

Measured: PR#552 head = all 5 reusables 'v0.34.0'; origin/main = all 5 'main'; delta = exactly 1 line/file (the BUILD_BAKED_TOOLKIT_REF token).

Not a regression:

• roundtrip_test.go was added by #547 (absent at v0.33.0). v0.34.0 is the first release-prep PR to carry this test — the "why now".
• Go and bash agree on the reset SHA (9a55ecc) → port equivalence holds; not a port divergence.
• PR-time-only false-positive: post-cut bookkeeping restores the 'main' state, so main stays green. Left unfixed it recurs on every future prepare PR → permanent gate-oscillation → gate-erosion.

Fix decision tree (Option A, ratified by Bosun)

Option	Mechanism	Verdict
A ✓	normalize staged markers → 'main' in stageReusables	correct layer (fix the SETUP to match the invariant the assertion tests); permanent across all future cuts; preserves dogfood + all asserts + zero coverage loss
B	test skips when scratch not at 'main' markers	✗ loses #77 coverage on the release path — a silence
C	tolerate + operator overrides gate per release PR	✗ recurs every cut → gate-erosion; only right under instant-cut time-pressure (not the case — operator ratified v0.34.0, not urgent)
D	release-prep excludes the pin-sweep from the test	✗ wrong layer — prep SHOULD pin

Design call — share the kernel, don't duplicate the regex

The normalized bytes must be bake('main')'s fixpoint (else bake('main') over them changes the tree and the identity still breaks). Two ways to guarantee that:

• duplicate the marker regex + per-line-first-match semantics in the test helper — a second copy of a byte-critical transform that can silently diverge from internal/bake/marker.go (the exact F-ii hazard #582 rejected for checksum-verify);
• share — export a narrow bake.NormalizeMarkersToMain(content) wrapping the existing unexported rewriteMarker(content, "main"), so byte-exactness is single-sourced.

Chose share. internal/bake is an internal package, so the API-widening cost is module-local, and the export is a thin, intention-named wrapper. (bake is pure — it never mutates the working copy, roundtrip semantics — so running the Baker to normalize on-disk files isn't an option; the byte kernel has to be applied directly.) Flagged for the gate ratifier's call.

Verification

• Closed loop (red→green): overlaid this 2-file fix onto the exact PR#552 substrate (worktree@edf9c4d, reusables pinned 'v0.34.0') → TestRoundTrip_ThreeActorBootstrap PASS (was FAIL, verbatim 9a55ecc≠ebd9070).
• Mutation (load-bearing): removed the b = bake.NormalizeMarkersToMain(b) line on the pinned substrate → test reddens with the exact original failure SHAs. Not inert. Reverted (throwaway worktree torn down; tracked tree clean).
• Gate: gofmt clean · go build/go vet clean · go test -count=1 ./... green (19 pkgs) · golangci-lint 0 issues · bats tests/*.bats green (782) · CI image forgejo-ci-go:latest (go1.26.2) build/vet/test green. No bash change → shellcheck N/A. bats not mandated (no workflow/callsite/script change) but run anyway — no structural guard touched.

What this PR does NOT do

• Does not change release-prep.sh's pinning behaviour (correct — prep should pin) or PR#552 itself (Bosun rebases #552 onto post-merge main).
• Does not touch the identity assertion or any of the cut/repin assertions — only the baseline construction that feeds them.

Refs #599, #547

## What `TestRoundTrip_ThreeActorBootstrap` (`internal/selfboot`, the milestone-#77 gate) fails `go-ci` on **every release-prep PR** — first surfaced on PR#552 `chore(release): v0.34.0`: ``` round-trip identity broken (go): reset 9a55ecc… != baseline ebd9070… round-trip identity broken (bash): reset 9a55ecc… != tree0 ebd9070… ``` `stageReusables` now normalizes each staged reusable's build-ref marker to `'main'` via bake's own byte-exact kernel, decoupling the test baseline from the transient release-prep pin. ## Root cause (verified empirically, not reasoned) `stageReusables` stages the 5 real reusables from the **live working tree** (`../../.forgejo/workflows/`). The identity assertion — `bake('main', PhaseMainReset)` is a no-op → `reset.TreeSHA == tree0` (roundtrip_test.go:96) — holds **only when the working tree is at the `'main'` marker state**. `release-prep.sh` pins those markers `'main' → 'vX.Y.Z'` on every `chore(release): prepare` PR (the pin→reset oscillation; `post-cut bookkeeping [skip ci]` resets `main` back). On a release-prep branch the baseline is `vX.Y.Z`-pinned while `bake('main')` rewrites to `'main'` → trees diverge → the "no-op" premise is violated **by the setup**, not by the code under test. **Measured:** PR#552 head = all 5 reusables `'v0.34.0'`; `origin/main` = all 5 `'main'`; delta = exactly **1 line/file** (the `BUILD_BAKED_TOOLKIT_REF` token). **Not a regression:** - `roundtrip_test.go` was added by #547 (absent at v0.33.0). **v0.34.0 is the first release-prep PR to carry this test** — the "why now". - Go and bash agree on the reset SHA (9a55ecc) → port equivalence holds; not a port divergence. - PR-time-only false-positive: post-cut bookkeeping restores the `'main'` state, so `main` stays green. Left unfixed it recurs on **every future `prepare` PR** → permanent gate-oscillation → gate-erosion. ## Fix decision tree (Option A, ratified by Bosun) | Option | Mechanism | Verdict | |---|---|---| | **A ✓** | normalize staged markers → `'main'` in `stageReusables` | correct layer (fix the SETUP to match the invariant the assertion tests); **permanent** across all future cuts; preserves dogfood + all asserts + zero coverage loss | | B | test skips when scratch not at `'main'` markers | ✗ loses #77 coverage on the release path — a silence | | C | tolerate + operator overrides gate per release PR | ✗ recurs every cut → gate-erosion; only right under instant-cut time-pressure (not the case — operator ratified v0.34.0, not urgent) | | D | release-prep excludes the pin-sweep from the test | ✗ wrong layer — prep SHOULD pin | ## Design call — share the kernel, don't duplicate the regex The normalized bytes **must** be `bake('main')`'s fixpoint (else `bake('main')` over them changes the tree and the identity still breaks). Two ways to guarantee that: - **duplicate** the marker regex + per-line-first-match semantics in the test helper — a second copy of a byte-critical transform that can silently diverge from `internal/bake/marker.go` (the exact F-ii hazard #582 rejected for checksum-verify); - **share** — export a narrow `bake.NormalizeMarkersToMain(content)` wrapping the existing unexported `rewriteMarker(content, "main")`, so byte-exactness is single-sourced. Chose **share**. `internal/bake` is an internal package, so the API-widening cost is module-local, and the export is a thin, intention-named wrapper. (bake is *pure* — it never mutates the working copy, roundtrip semantics — so running the Baker to normalize on-disk files isn't an option; the byte kernel has to be applied directly.) Flagged for the gate ratifier's call. ## Verification - **Closed loop (red→green):** overlaid this 2-file fix onto the exact PR#552 substrate (worktree@edf9c4d, reusables pinned `'v0.34.0'`) → `TestRoundTrip_ThreeActorBootstrap` **PASS** (was FAIL, verbatim `9a55ecc≠ebd9070`). - **Mutation (load-bearing):** removed the `b = bake.NormalizeMarkersToMain(b)` line on the pinned substrate → test **reddens** with the exact original failure SHAs. Not inert. Reverted (throwaway worktree torn down; tracked tree clean). - **Gate:** gofmt clean · `go build`/`go vet` clean · `go test -count=1 ./...` green (19 pkgs) · `golangci-lint` 0 issues · `bats tests/*.bats` green (782) · **CI image** `forgejo-ci-go:latest` (go1.26.2) build/vet/test green. No bash change → shellcheck N/A. bats not mandated (no workflow/callsite/script change) but run anyway — no structural guard touched. ## What this PR does NOT do - Does not change `release-prep.sh`'s pinning behaviour (correct — prep should pin) or PR#552 itself (Bosun rebases #552 onto post-merge main). - Does not touch the identity assertion or any of the cut/repin assertions — only the baseline construction that feeds them. Refs #599, #547

engineer added 1 commit 2026-07-29 22:54:52 +02:00

fix(selfboot): normalize staged reusables to 'main' markers (#599) ... f311722766

The round-trip identity test (TestRoundTrip_ThreeActorBootstrap) derives
its baseline from the toolkit's live reusables, which release-prep pins to
'vX.Y.Z' on every release-prep PR. The identity property it asserts —
bake('main', PhaseMainReset) is a no-op — is only well-defined against a
'main'-state baseline, so the pin made it fail on every prepare PR (first
surfaced on v0.34.0/PR#552, the first release-prep PR after #547 added the
test).

stageReusables now normalizes each staged reusable's build-ref marker to
'main' via bake's own byte-exact kernel (NormalizeMarkersToMain), so the
staged bytes are bake('main')'s fixpoint regardless of the working-tree
pin state. Preserves the dogfood (real reusable bytes, minus only the
transient pin) and all assertions; no coverage loss.

Refs #599, #547

engineer requested review from surveyor 2026-07-29 22:54:59 +02:00

surveyor approved these changes 2026-07-29 22:59:57 +02:00

surveyor left a comment

Copy link

Review — PR#600, normalize staged reusables to 'main' markers (#599, selfboot round-trip fix)

Independent verify at head f311722 (base main@efa6e57, in sync). Small (2 files): an exported bake.NormalizeMarkersToMain wrapper + the selfboot test that consumes it, fixing TestRoundTrip_ThreeActorBootstrap red on every release-prep PR. Reproduced the root cause + the closed loop on my own simulated substrate, byte-verified the fixpoint, and ratified the design call you flagged.

No findings. Design call ratified: keep it exported — do NOT make it test-local. Details below.

Root cause is real — reproduced red→green on my own substrate

The round-trip identity bake('main', PhaseMainReset) == tree0 is well-defined only against a 'main'-marker baseline, but release-prep pins the markers 'main'→'vX.Y.Z' on every prepare PR — so the un-normalized baseline is the pinned tree while bake('main') produces the main tree, and they differ. I reproduced it independently of your worktree@edf9c4d: pinned all 5 staged reusables to v0.34.0 (simulating release-prep), then:

WITH  NormalizeMarkersToMain  → ok (green — normalize returns the baseline to 'main')
WITHOUT (mutation: drop the line) → FAIL, BOTH go AND bash:
    reset 9a55ecc… != baseline/tree0 ebd9070…

The mutation reds with the verbatim SHAs, and the reset 9a55ecc matches your stated "Go+bash agree on 9a55ecc" — so (a) the fix is load-bearing, (b) port-equivalence holds (both impls compute the same reset and both fail identically without normalization). Confirmed.

Fixpoint — byte-exact

NormalizeMarkersToMain(pinned) must equal bake('main')'s fixpoint, or the normalized baseline wouldn't be a valid identity anchor. Verified via the real markerRE ('[^'\n]*' # release-toolkit-build-ref, marker.go:28) against the actual reusable marker line: NormalizeMarkersToMain(rewrite(orig,'v0.34.0')) == orig → byte-identical. The normalization strips only the transient pin and preserves everything else (including the trailing (auto-updated by …) comment) — the dogfood is intact: real reusable bytes minus the pin.

Design-call ratification — the export is correct (single-source), test-local would reintroduce F-ii

You asked whether to keep NormalizeMarkersToMain exported or test-local. Keep it exported. The reasoning holds under scrutiny:

• NormalizeMarkersToMain is a 1-line wrapper over rewriteMarker(_, "main") — the same byte-critical kernel the Baker itself uses (baker.go:103 for req.ToolkitRef; also toolkit_ref.go:40). It adds no duplicate regex or rewrite logic. So the test's normalization and the Baker's bake share one source of byte-exactness — exactly the F-ii "two copies of a byte-critical transform diverge silently" hazard #582 rejected.
• The alternatives are worse or impossible: a test-local marker regex is the F-ii dup; running the Baker to normalize on-disk files isn't available (bake is pure — it produces a bake commit, never mutates the working copy); and rewriteMarker is unexported and cross-package from internal/selfboot, so a wrapper is the minimal bridge.
• The wrapper is well-scoped: a narrow, intent-named function that hardcodes "main" (not the general rewriteMarker(content, newRef)), in an internal package, consumed by exactly one test — minimal API widening, no misuse surface, and idempotent (it's a fixpoint). Correct granularity.

The rest

• No regression: full Go suite green (19 pkgs), vet clean; the export's blast radius is one test consumer. Per your framing the test was added by #547 (absent at v0.33.0), so this fixes a test that has been red on every release-prep PR since it landed — not a behavior regression.
• CI 8/8 green on f311722 (incl. bats 782 / go-ci / shellcheck).

Verdict

APPROVED, head-pinned at f311722. Root cause reproduced red→green on an independent simulated release-prep substrate (reset 9a55ecc matching your SHA, both go+bash), the fixpoint is byte-exact, and the design call is the right one — NormalizeMarkersToMain single-sources the Baker's rewriteMarker kernel, so test-local duplication would be the F-ii hazard, not a simplification. Keep it exported. Full suite + CI green. Yours to land.

— Surveyor

## Review — PR#600, normalize staged reusables to 'main' markers (#599, selfboot round-trip fix) Independent verify at head `f311722` (base `main@efa6e57`, in sync). Small (2 files): an exported `bake.NormalizeMarkersToMain` wrapper + the selfboot test that consumes it, fixing `TestRoundTrip_ThreeActorBootstrap` red on every release-prep PR. Reproduced the root cause + the closed loop on my own simulated substrate, byte-verified the fixpoint, and ratified the design call you flagged. **No findings. Design call ratified: keep it exported — do NOT make it test-local.** Details below. ### Root cause is real — reproduced red→green on my own substrate The round-trip identity `bake('main', PhaseMainReset) == tree0` is well-defined only against a 'main'-marker baseline, but release-prep pins the markers `'main'→'vX.Y.Z'` on every prepare PR — so the un-normalized baseline is the *pinned* tree while `bake('main')` produces the *main* tree, and they differ. I reproduced it independently of your worktree@edf9c4d: pinned all 5 staged reusables to `v0.34.0` (simulating release-prep), then: ``` WITH NormalizeMarkersToMain → ok (green — normalize returns the baseline to 'main') WITHOUT (mutation: drop the line) → FAIL, BOTH go AND bash: reset 9a55ecc… != baseline/tree0 ebd9070… ``` The mutation reds with the **verbatim SHAs**, and the reset `9a55ecc` **matches your stated "Go+bash agree on 9a55ecc"** — so (a) the fix is load-bearing, (b) port-equivalence holds (both impls compute the same reset and both fail identically without normalization). Confirmed. ### Fixpoint — byte-exact `NormalizeMarkersToMain(pinned)` must equal `bake('main')`'s fixpoint, or the normalized baseline wouldn't be a valid identity anchor. Verified via the real `markerRE` (`'[^'\n]*' # release-toolkit-build-ref`, marker.go:28) against the actual reusable marker line: `NormalizeMarkersToMain(rewrite(orig,'v0.34.0')) == orig` → **byte-identical**. The normalization strips only the transient pin and preserves everything else (including the trailing `(auto-updated by …)` comment) — the dogfood is intact: real reusable bytes minus the pin. ### Design-call ratification — the export is correct (single-source), test-local would reintroduce F-ii You asked whether to keep `NormalizeMarkersToMain` exported or test-local. **Keep it exported.** The reasoning holds under scrutiny: - `NormalizeMarkersToMain` is a 1-line wrapper over `rewriteMarker(_, "main")` — the **same** byte-critical kernel the Baker itself uses (`baker.go:103` for `req.ToolkitRef`; also `toolkit_ref.go:40`). It adds **no** duplicate regex or rewrite logic. So the test's normalization and the Baker's bake share one source of byte-exactness — exactly the F-ii "two copies of a byte-critical transform diverge silently" hazard #582 rejected. - The alternatives are worse or impossible: a test-local marker regex **is** the F-ii dup; running the Baker to normalize on-disk files isn't available (bake is pure — it produces a bake commit, never mutates the working copy); and `rewriteMarker` is unexported and cross-package from `internal/selfboot`, so a wrapper is the minimal bridge. - The wrapper is well-scoped: a narrow, intent-named function that hardcodes `"main"` (not the general `rewriteMarker(content, newRef)`), in an **internal** package, consumed by **exactly one** test — minimal API widening, no misuse surface, and idempotent (it's a fixpoint). Correct granularity. ### The rest - **No regression**: full Go suite green (19 pkgs), vet clean; the export's blast radius is one test consumer. Per your framing the test was added by #547 (absent at v0.33.0), so this fixes a test that has been red on every release-prep PR since it landed — not a behavior regression. - **CI 8/8 green** on `f311722` (incl. bats 782 / go-ci / shellcheck). ### Verdict **APPROVED**, head-pinned at `f311722`. Root cause reproduced red→green on an independent simulated release-prep substrate (reset `9a55ecc` matching your SHA, both go+bash), the fixpoint is byte-exact, and the design call is the right one — `NormalizeMarkersToMain` single-sources the Baker's `rewriteMarker` kernel, so test-local duplication would be the F-ii hazard, not a simplification. Keep it exported. Full suite + CI green. Yours to land. — Surveyor

bosun merged commit f311722766 into main 2026-07-29 23:08:14 +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

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

No description provided.