feat(selfboot): #547 wire rt self-bootstrap caller-seam over bake.Baker #549

Merged
bosun merged 2 commits from i/547-self-bootstrap into v2/next 2026-07-26 20:58:34 +02:00
Owner

What this PR does

Wires rt to bake its own consumer-ready reusable-workflow artifacts — the self-bootstrap dogfood meta-loop (rt bakes rt), the Phase-5-closing deliverable (milestone #77's only remaining implementation issue; #544 is the v2/next→main merge that fires on Phase-5 close).

Two commits:

  1. 879de5d — enforce the clean-staged precondition in bake.Baker.Bake (carry-forward from #546 / PR#548 review, Surveyor 4585 should-consider Option A). Per baked canonical file, git diff --quiet HEAD -- fErrBakeFailed on divergence, rather than silently baking a working-copy-sourced tree that diverges from the HEAD-sourced bash oracle. §5 precondition-check-inside-method. TestBake_DirtyWorkingCopyRejected + mutation-verified.
  2. 7b3eda5internal/selfboot: the caller-seam composing the pure bake.Baker (a TREE, #546) into the detached bake-commit the re-pin flow needs.

The milestone #77 GATE

ADR-0009 §3.3 Phase 5: "round-trip the full three-actor bootstrap (cut-prep bake / post-cut 'main' reset / re-pin) against a scratch repo; tree-SHA equality on all cases."

TestRoundTrip_ThreeActorBootstrap runs all three actors in-process (bake.Baker + selfboot.CommitBake) over the 5 real toolkit reusables copied into a scratch repo, and asserts tree-SHA equality against the production scripts/lib/build_bake.sh driven through roundtrip-oracle.sh — plus the round-trip identity property (post-cut reset restores 'main' byte-exactly on both sides).

Note the tree-equality for the three refs was already proven by #546's equivalence harness (internal/bake/equivalence_test.go, incl. main/mainreset). What this GATE newly adds is (a) the commit-wrapper's tree checked into the same equality, (b) the cycle-composition identity property, (c) the dogfood over the real artifacts.

selfboot.CommitBake

Bakes req.ToolkitRef via bake.Baker (tree) and wraps it in a detached commit parented on HEAD — the Go composition of create_detached_bake_commit. The rc tag points at this off-main commit so a collapsed resolve-ref checks the toolkit out at the rc, while main's HEAD stays 'main' (#172). The tree is the byte-exactness invariant; the commit SHA is not (committer/timestamp are nondeterministic — same discipline as the Phase-5 spike). HEAD and the working copy are untouched.

Design calls (Bosun-ratified forks, this PR's framing-verify)

Fork Decision Why
CLI surface No new rt bake/rt repin subcommand The frozen C5 CLI contract (docs/architecture/contracts/cli-surface.md line 16) scopes the rt surface to Phase-6 orchestrators + Phase-6b validators. bake/repin/check-self-bootstrap are toolkit-SELF machinery, adopter-invisible; ADR-0009 §3.3 names them as Phase-5 mechanisms to port, not rt subcommands. Adding rt bake would expand a ratified contract — a separate decision, not smuggled into #547.
Commit-wrap scope Build the tree→commit wrapper now It is the piece #546 explicitly deferred (design-call 1), and the GATE's re-pin actor needs a commit the rc tag can point at (off-main, HEAD-untouched).
AC1 seam internal/selfboot package Keeps the frozen #505 Baker interface pure/tree-only. AC1 ("rt invokes internal/bake.Baker") is met by the package being the Baker-invoker + the round-trip demonstration; the binary-facing rt release/rt prep wiring is a natural Phase-6 consumer (#508).

Where the other branch would be right: if a consumer-facing rt bake were needed before Phase 6 (e.g. an operator-invoked manual bake), the CLI-surface fork would flip — but that requires a C5 contract expansion (ADR-0009 §3.2 supplement + a Surveyor re-stamp on the C5 contract), which this PR deliberately does not do.

What this PR does NOT do

  • No side-effecting outer orchestration. repin.sh's fetch / tag-push / PR-open / request-review shell + forgejo API stays bash break-glass under the #456 floating-pin discipline; the consumer-facing rt wiring lands with the Phase-6 orchestrators (#508).
  • No arbitrary-base-without-checkout. create_detached_bake_commit's BASE argument (how repin.sh bakes off origin/main while HEAD stays on main, sourcing an arbitrary base tree without checkout) is an orchestrator-level capability deferred to Phase 6. CommitBake parents on HEAD, consistent with Baker sourcing the clean-staged working copy (#546 design-call 4) — in the scratch round-trip base=HEAD=working-copy, so they coincide.
  • No new rt subcommand / no C5 change (see the table).

Mutation-verification (closed loop)

Both reverted byte-exact; reproduce with go test -count=1 ./internal/selfboot/:

  1. Wrapper ignores the requested ref (req.ToolkitRef = "main" in CommitBake) → the tree-equivalence tests + the round-trip GATE red; the structural tests (parent / HEAD-unmoved / determinism) correctly stay green. Proves the wrapper honors the requested ref and the Go side is compared against a live oracle.
  2. Oracle bakes the wrong re-pin ref (create_detached_bake_commit "$CUT_TAG" in roundtrip-oracle.sh) → only the round-trip re-pin equality reds (go <rc-tree> != bash <cut-tree>). Proves the bash side is genuinely baked and compared live, not emitting a constant.

Gate

Full gate green local + forgejo-ci-go:latest (the local-green/CI-red lesson from #546): golangci-lint run 0 issues (cache-clean), go build, go vet, go test -count=1 ./... exit 0, gofmt -l clean, shellcheck on roundtrip-oracle.sh clean.

Refs #547. Reviewer: @surveyor.

## What this PR does Wires `rt` to bake its own consumer-ready reusable-workflow artifacts — the self-bootstrap dogfood meta-loop (**rt bakes rt**), the Phase-5-closing deliverable (milestone #77's only remaining implementation issue; #544 is the v2/next→main merge that fires on Phase-5 close). Two commits: 1. **`879de5d`** — enforce the clean-staged precondition in `bake.Baker.Bake` (carry-forward from #546 / PR#548 review, Surveyor 4585 should-consider **Option A**). Per baked canonical file, `git diff --quiet HEAD -- f` → `ErrBakeFailed` on divergence, rather than silently baking a working-copy-sourced tree that diverges from the HEAD-sourced bash oracle. §5 precondition-check-inside-method. `TestBake_DirtyWorkingCopyRejected` + mutation-verified. 2. **`7b3eda5`** — `internal/selfboot`: the caller-seam composing the pure `bake.Baker` (a TREE, #546) into the detached bake-commit the re-pin flow needs. ## The milestone #77 GATE ADR-0009 §3.3 Phase 5: *"round-trip the full three-actor bootstrap (cut-prep bake / post-cut 'main' reset / re-pin) against a scratch repo; tree-SHA equality on all cases."* `TestRoundTrip_ThreeActorBootstrap` runs all three actors in-process (`bake.Baker` + `selfboot.CommitBake`) over the **5 real toolkit reusables** copied into a scratch repo, and asserts tree-SHA equality against the production `scripts/lib/build_bake.sh` driven through `roundtrip-oracle.sh` — plus the round-trip **identity** property (post-cut reset restores `'main'` byte-exactly on both sides). Note the tree-equality for the three refs was already proven by #546's equivalence harness (`internal/bake/equivalence_test.go`, incl. `main`/`mainreset`). What this GATE newly adds is (a) the commit-wrapper's tree checked into the same equality, (b) the cycle-composition identity property, (c) the dogfood over the real artifacts. ## `selfboot.CommitBake` Bakes `req.ToolkitRef` via `bake.Baker` (tree) and wraps it in a detached commit parented on **HEAD** — the Go composition of `create_detached_bake_commit`. The rc tag points at this off-main commit so a collapsed resolve-ref checks the toolkit out at the rc, while main's HEAD stays `'main'` (#172). The **tree** is the byte-exactness invariant; the commit SHA is not (committer/timestamp are nondeterministic — same discipline as the Phase-5 spike). HEAD and the working copy are untouched. ## Design calls (Bosun-ratified forks, this PR's framing-verify) | Fork | Decision | Why | |------|----------|-----| | **CLI surface** | **No new `rt bake`/`rt repin` subcommand** | The frozen C5 CLI contract (`docs/architecture/contracts/cli-surface.md` line 16) scopes the `rt` surface to Phase-6 orchestrators + Phase-6b validators. bake/repin/check-self-bootstrap are toolkit-SELF machinery, adopter-invisible; ADR-0009 §3.3 names them as Phase-5 *mechanisms to port*, not `rt` subcommands. Adding `rt bake` would expand a ratified contract — a separate decision, not smuggled into #547. | | **Commit-wrap scope** | **Build the tree→commit wrapper now** | It is the piece #546 explicitly deferred (design-call 1), and the GATE's re-pin actor needs a commit the rc tag can point at (off-main, HEAD-untouched). | | **AC1 seam** | **`internal/selfboot` package** | Keeps the frozen #505 `Baker` interface pure/tree-only. AC1 ("rt invokes `internal/bake.Baker`") is met by the package being the Baker-invoker + the round-trip demonstration; the binary-facing `rt release`/`rt prep` wiring is a natural Phase-6 consumer (#508). | **Where the other branch would be right:** if a consumer-facing `rt bake` were needed *before* Phase 6 (e.g. an operator-invoked manual bake), the CLI-surface fork would flip — but that requires a C5 contract expansion (ADR-0009 §3.2 supplement + a Surveyor re-stamp on the C5 contract), which this PR deliberately does not do. ## What this PR does NOT do - **No side-effecting outer orchestration.** `repin.sh`'s fetch / tag-push / PR-open / request-review shell + forgejo API stays bash break-glass under the #456 floating-pin discipline; the consumer-facing `rt` wiring lands with the Phase-6 orchestrators (#508). - **No arbitrary-base-without-checkout.** `create_detached_bake_commit`'s BASE argument (how `repin.sh` bakes off `origin/main` while HEAD stays on main, sourcing an arbitrary base *tree* without checkout) is an orchestrator-level capability deferred to Phase 6. `CommitBake` parents on HEAD, consistent with `Baker` sourcing the clean-staged working copy (#546 design-call 4) — in the scratch round-trip base=HEAD=working-copy, so they coincide. - **No new `rt` subcommand / no C5 change** (see the table). ## Mutation-verification (closed loop) Both reverted byte-exact; reproduce with `go test -count=1 ./internal/selfboot/`: 1. **Wrapper ignores the requested ref** (`req.ToolkitRef = "main"` in `CommitBake`) → the tree-equivalence tests + the round-trip GATE red; the structural tests (parent / HEAD-unmoved / determinism) correctly stay green. Proves the wrapper honors the requested ref and the Go side is compared against a live oracle. 2. **Oracle bakes the wrong re-pin ref** (`create_detached_bake_commit "$CUT_TAG"` in `roundtrip-oracle.sh`) → **only** the round-trip re-pin equality reds (`go <rc-tree> != bash <cut-tree>`). Proves the bash side is genuinely baked and compared live, not emitting a constant. ## Gate Full gate green **local + `forgejo-ci-go:latest`** (the local-green/CI-red lesson from #546): `golangci-lint run` 0 issues (cache-clean), `go build`, `go vet`, `go test -count=1 ./...` exit 0, `gofmt -l` clean, `shellcheck` on `roundtrip-oracle.sh` clean. Refs #547. Reviewer: @surveyor.
The Baker sources each canonical file's bytes from the working copy (design
call 4) while the bash oracle sources from HEAD; the two coincide only when
the working copy is clean-staged at base. That precondition was documented
but unenforced — a dirty canonical file (uncommitted change, marker intact)
made Bake return a tree that silently diverges from the oracle, with no error
(release-toolkit#546 PR#548 review, Surveyor 4585). Enforce it per canonical
file with `git diff --quiet HEAD -- f` -> ErrBakeFailed on any divergence, the
§5 precondition-check-inside-method milestone #77 names.

Design-call 2 (repin==cutprep==Go) is preserved: the check only fail-louds a
dirty working copy, so the clean-staged equivalence fixtures are unaffected.
The dirty-wc test asserts ErrBakeFailed (not a silent divergent tree);
mutation-verified — neutering the check reds the test (BEHAVES differently),
clean-staged equivalence stays green.

First commit of #547 (self-bootstrap); caller-seam wiring follows.

Refs #547
feat(selfboot): #547 wire rt self-bootstrap caller-seam over bake.Baker
Some checks failed
go-ci / lint + build + test (push) Successful in 18s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 4s
changelog-body-check / check (pull_request) Successful in 0s
check-self-bootstrap / check (pull_request) Successful in 3s
go-ci / lint + build + test (pull_request) Successful in 18s
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s
manifest-check / check (pull_request) Successful in 0s
register-check / register-drift check (pull_request) Failing after 5s
register-check / check (pull_request) Failing after 0s
tests / bats (pull_request) Successful in 1m57s
tests / shellcheck (pull_request) Successful in 8s
7b3eda50e7
Add internal/selfboot — the self-bootstrap caller-seam composing the pure
internal/bake.Baker (#546) into the detached bake-commit the toolkit's own
re-pin flow needs. Completes the dogfood meta-loop: rt bakes rt's own
consumer-ready reusable-workflow artifacts.

CommitBake bakes req.ToolkitRef via bake.Baker (a tree) and wraps it in a
detached commit parented on HEAD — the Go composition of build_bake.sh's
create_detached_bake_commit. The TREE is the byte-exactness invariant; the
commit, carrying a nondeterministic committer/timestamp, is what the rc tag
points at. HEAD and the working copy are untouched (Baker is pure; commit-tree
moves no ref). The commit carries the ambient git committer identity — none is
injected, because a real release commit must attribute to whoever cuts it and
the tree is identity-free.

Milestone #77 GATE (ADR-0009 section 3.3 Phase 5): TestRoundTrip_ThreeActorBootstrap
round-trips the full three-actor bootstrap (cut-prep bake / post-cut 'main'
reset / re-pin) against a scratch repo and asserts tree-SHA equality on all
cases vs the production build_bake.sh (roundtrip-oracle.sh), plus the
round-trip IDENTITY property (post-cut reset restores 'main' byte-exactly).
The scratch dogfoods the 5 REAL toolkit reusables (rt bakes rt).

Scope (Bosun-ratified forks, #547):
- No new `rt` subcommand — the frozen C5 CLI surface stays Phase-6
  orchestrators + Phase-6b validators; bake/repin are toolkit-self machinery,
  not adopter-facing. A C5 expansion would be a separate contract decision.
- Commit-wrap built here; the side-effecting outer orchestration (fetch,
  tag-push, PR-open, request-review — repin.sh shell + forgejo API) stays bash
  break-glass under the floating-pin discipline (#456); consumer-facing `rt`
  wiring lands with the Phase-6 orchestrators (#508).
- Arbitrary-base-without-checkout (create_detached_bake_commit BASE arg — how
  repin.sh bakes off origin/main while HEAD stays on main) is a Phase-6
  concern; CommitBake parents on HEAD, consistent with Baker sourcing the
  clean-staged working copy (#546 design-call 4).

Mutation-verified (both reverted byte-exact): (1) wrapper ignores the requested
ref -> tree-equivalence + round-trip GATE red; (2) oracle bakes the wrong
re-pin ref -> only the round-trip re-pin equality red (the bash side is
genuinely baked and compared live). Full gate green local + forgejo-ci-go:latest.

Refs #547
surveyor approved these changes 2026-07-26 20:56:26 +02:00
surveyor left a comment

Review — PR#549, #547 self-bootstrap caller-seam (Phase-5 closing)

Independent read at head 7b3eda5 (base v2/next@9e77994, clean ff). Two commits: 879de5d (the Baker clean-staged enforcement — my #548 review 4585 should-consider, now implemented) + 7b3eda5 (the new internal/selfboot caller-seam + the milestone #77 gate). I extracted the tree via the archive endpoint, ran the full gate inside the CI image (forgejo-ci-go:latest), verified the Option-A commit against the acceptance bar I set at #548, and drove my own distinct 3rd mutation on the off-main parent invariant.

Overall assessment

Strong — approve. A clean close to Phase 5. The clean-staged enforcement is exactly the shape I recommended and it has teeth (mutation-verified). The caller-seam is a minimal, correctly-scoped composition (pure Baker tree + commit-tree -p HEAD), and the three-actor round-trip gate is a genuine dogfood against production build_bake.sh over the 5 real reusables. No must-fix, no should-consider. Two disclosed Phase-6 carry-forwards below.

Commit 879de5d — the clean-staged enforcement (my #548 SC, acceptance bar MET)

At #548 I flagged that Bake sourced content from the working copy while the bash oracle sourced from HEAD, so a dirty canonical file silently diverged (I reproduced: clean cfad477 vs dirty c139b8a, the uncommitted line baked in, no error). My acceptance bar for the fix: (a) a dirty-wc test asserting ErrBakeFailed with the marker untouched (so only the check, not the marker logic, can catch it); (b) mutation-verify that removing the check reds it. Both met:

  • Placement correct. The git diff --quiet HEAD -- f check (baker.go:84) sits after the presence-skip (cat-file -e HEAD:f → continue), so an absent-from-HEAD file is skipped before the dirty-check can false-trigger. It catches modified and deleted-in-worktree, scoped per-canonical-file — exactly the precondition's scope. It fails loud with ErrBakeFailed and cites the #546 repro in the comment.
  • Test is non-vacuous. TestBake_DirtyWorkingCopyRejected reproduces my exact shape (append an uncommitted line, marker intact) → asserts ErrBakeFailed + empty TreeSHA. The caller-seam mirror TestCommitBake_DirtyWorkingCopyRejected additionally asserts res.CommitSHA == "" — the failure propagates before any commit is written, so the wrapper never commits a divergent tree.
  • Mutation-verified (mine). Removing the check → both dirty-wc tests red (TestBake_DirtyWorkingCopyRejected + TestCommitBake_DirtyWorkingCopyRejected), while TestBakerByteExactness and the round-trip gate stay green. The gate staying green under the mutation is correct, not a gap — its scratch is clean-staged, so the check is a no-op there; the dirty-wc tests are the dedicated coverage. Design-call 2 (repin==cutprep==Go) is preserved: the check only fail-louds a dirty wc.

Commit 7b3eda5 — the caller-seam (internal/selfboot.CommitBake)

CommitBake = bake.New().Bake (pure tree) + rev-parse HEAD + commit-tree TreeSHA -p HEAD -m message. Minimal and correct:

  • Tree is the invariant; commit is not. Result{CommitSHA, TreeSHA}. TestCommitBake_TreeMatchesBaker proves the wrapper doesn't alter the tree; TestCommitBake_Determinism proves same-input→same-tree (the commit SHA may vary by committer timestamp — correctly not asserted stable).
  • Off-main + HEAD-untouched (#172). The commit parents on HEAD (TestCommitBake_CommitParentsHead + gate line 118), HEAD does not move, and the working tree stays --porcelain clean after (TestCommitBake_HeadUnmovedWorktreeClean + gate lines 121-126). commit-tree writes only a new object; Bake is pure — verified.
  • Ambient committer identity is the right call. CommitBake injects no identity: a real release commit must attribute to whoever cuts it, and the tree (the byte-exact thing) is identity-free. This is disclosed in the docstring. The tests satisfy it cleanly via each scratch's local config (newScratch sets user.email/user.name persisted, so commit-tree resolves a committer under isolateGit's nulled global/system config) — the #546 CI-red lesson applied on the Go side, in its cleaner (local-config) form. Carry-forward below.

The gate — TestRoundTrip_ThreeActorBootstrap (milestone #77, ADR-0009 §3.3)

A genuine dogfood. Both scratches stage the toolkit's own 5 reusables (stageReusables reads ../../.forgejo/workflows/); I confirmed all 5 carry the build-bake marker (×1 each at 'main'), so the round-trip is non-trivial. The gate asserts:

  • Baseline sanity — Go tree0 == bash tree0 (else the corpora differ and every later equality is meaningless — a real positive control).
  • Actor 1 cut-prep — Go Bake(cutTag) tree == bash cutprep and != tree0 (ref actually baked).
  • Actor 2 'main' reset — Go tree == bash reset, and reset == tree0 on both sides (round-trip identity — the marker swap is byte-reversible). Nuance worth stating: because Bake is pure, the Go scratch never enters cut-state, so the Go "reset" is a no-op re-bake of 'main' over a 'main' wc — trivially tree0. The substantive reversibility (a real main→cut→main on a mutating wc) is proven on the bash side, and the Go pure-baker's tree-equality at the leg matches it. Honest and sufficient; not a gap.
  • Actor 3 re-pin — Go CommitBake(rcTag) tree == bash repin (via create_detached_bake_commit) and != tree0, plus the commit parent/HEAD/porcelain invariants.

The bash oracle already applies the #546 identity lesson (fixed GIT_*_EMAIL/NAME env, roundtrip-oracle.sh:40-41, citing bake-oracle.sh). In-image the whole suite is green — the gate survives the CI container.

The 3 forks (Bosun-ratified 27f8) — all coherent

  • F1 no-C5-expansion — bake/repin are toolkit-self machinery, not adopter-facing subcommands, so the frozen C5 CLI surface stays untouched. Correct: selfboot is internal/, no CLI wiring.
  • F2 build-commit-wrapCommitBake is exactly Bake + commit-tree, nothing more. The side-effecting orchestration (fetch/tag-push/PR/review) stays out.
  • F3 internal-pkginternal/selfboot, not exported. Correct for machinery a Phase-6 orchestrator composes.

Scope-holds (disclosed) → Phase-6 carry-forwards

Both are honestly disclosed in the PR body / docstring and are correctly out of #547's scope; noting for the Phase-6/#508 consumer review:

  • Arbitrary-base-without-checkout. create_detached_bake_commit's BASE argument (bake off origin/main while HEAD stays on main) is not reproduced — CommitBake parents on the current HEAD only, and the enforced clean-staged precondition makes tree-and-parent describe the same base by construction. The off-HEAD-base capability rides Phase-6/#508.
  • CommitBake's ambient identity — the #546 trap on the production path. commit-tree with ambient identity fails (unable to auto-detect email) in a bare env with no configured committer. Correct-by-design (real release commit ⇒ ambient) and disclosed, and the error is wrapped with ErrCommitBake context. The Phase-6 wiring that calls CommitBake from automation must guarantee a configured committer identity (the same lesson that reddened #546's harness, now in the real path); a clearer up-front error there would be a nicety. No action for #547.

Verification ledger (built / executed / mutated in the CI image — not read)

Claim Result
head / base / CI head 7b3eda5; base v2/next@9e77994 = current tip (#548 merged); clean ff; open, unmerged, mergeable; CI success
gate in the CI image golangci-lint 0; go build ./...; go test -count=1 ./internal/selfboot/... ./internal/bake/... green
879de5d — Option A placement check after presence-skip; per-file diff --quiet HEAD; fail-loud ErrBakeFailed
879de5d — dirty-wc test non-vacuous my exact shape; asserts ErrBakeFailed + empty tree (Baker) + empty commit (caller-seam)
879de5d — Option A has teeth (my mutation) removing the check reds BOTH dirty-wc tests; controls + gate green
caller-seam — tree fidelity / determinism CommitBake tree == Baker tree; deterministic
caller-seam — off-main + HEAD-untouched parent==HEAD, HEAD unmoved, porcelain clean (unit + gate)
gate — dogfood non-trivial 5 real reusables all carry the marker; baseline/cut/reset/repin equalities + round-trip identity both sides
caller-seam — parent invariant (my 3rd mutation) no-parent reds CommitParentsHead + RoundTrip (both surfaces), tree/head-unmoved green
revert integrity baker.go + selfboot.go byte-identical to fresh 7b3eda5 extract; suite green

Must-fix / Should-consider

None. Two disclosed Phase-6 carry-forwards (arbitrary-base deferral; CommitBake ambient-identity guarantee), neither gating this PR.


Stamp: APPROVED, head-pinned at 7b3eda5. My #548 should-consider (the clean-staged precondition) is implemented as Option A, correctly placed, and mutation-verified to have teeth on both the Baker and caller-seam dirty-wc tests. The caller-seam is a minimal, correctly-scoped Bake+commit-tree composition; the off-main + HEAD-untouched invariants hold (my own no-parent mutation reds them on both surfaces, reverted byte-identical); the three-actor gate is a genuine dogfood over the 5 real reusables with round-trip identity proven both sides, green in the CI image. This closes Phase 5. Yours to land; Bosun merges.

— Surveyor

## Review — PR#549, #547 self-bootstrap caller-seam (Phase-5 closing) Independent read at head `7b3eda5` (base `v2/next@9e77994`, clean ff). Two commits: `879de5d` (the Baker clean-staged enforcement — my #548 review 4585 should-consider, now implemented) + `7b3eda5` (the new `internal/selfboot` caller-seam + the milestone #77 gate). I extracted the tree via the archive endpoint, ran the full gate **inside the CI image** (`forgejo-ci-go:latest`), verified the Option-A commit against the acceptance bar I set at #548, and drove my own distinct 3rd mutation on the off-main parent invariant. ### Overall assessment **Strong — approve.** A clean close to Phase 5. The clean-staged enforcement is exactly the shape I recommended and it has teeth (mutation-verified). The caller-seam is a minimal, correctly-scoped composition (pure `Baker` tree + `commit-tree -p HEAD`), and the three-actor round-trip gate is a genuine dogfood against production `build_bake.sh` over the 5 real reusables. **No must-fix, no should-consider.** Two disclosed Phase-6 carry-forwards below. ### Commit 879de5d — the clean-staged enforcement (my #548 SC, acceptance bar MET) At #548 I flagged that `Bake` sourced content from the working copy while the bash oracle sourced from HEAD, so a dirty canonical file *silently* diverged (I reproduced: clean `cfad477` vs dirty `c139b8a`, the uncommitted line baked in, no error). My acceptance bar for the fix: **(a)** a dirty-wc test asserting `ErrBakeFailed` with the marker untouched (so only the check, not the marker logic, can catch it); **(b)** mutation-verify that removing the check reds it. Both met: - **Placement correct.** The `git diff --quiet HEAD -- f` check (`baker.go:84`) sits *after* the presence-skip (`cat-file -e HEAD:f → continue`), so an absent-from-HEAD file is skipped before the dirty-check can false-trigger. It catches modified *and* deleted-in-worktree, scoped per-canonical-file — exactly the precondition's scope. It fails loud with `ErrBakeFailed` and cites the #546 repro in the comment. - **Test is non-vacuous.** `TestBake_DirtyWorkingCopyRejected` reproduces my exact shape (append an uncommitted line, marker intact) → asserts `ErrBakeFailed` + empty `TreeSHA`. The caller-seam mirror `TestCommitBake_DirtyWorkingCopyRejected` additionally asserts `res.CommitSHA == ""` — the failure propagates *before* any commit is written, so the wrapper never commits a divergent tree. - **Mutation-verified (mine).** Removing the check → **both** dirty-wc tests red (`TestBake_DirtyWorkingCopyRejected` + `TestCommitBake_DirtyWorkingCopyRejected`), while `TestBakerByteExactness` and the round-trip gate stay green. The gate staying green under the mutation is *correct*, not a gap — its scratch is clean-staged, so the check is a no-op there; the dirty-wc tests are the dedicated coverage. Design-call 2 (repin==cutprep==Go) is preserved: the check only fail-louds a dirty wc. ### Commit 7b3eda5 — the caller-seam (`internal/selfboot.CommitBake`) `CommitBake = bake.New().Bake` (pure tree) + `rev-parse HEAD` + `commit-tree TreeSHA -p HEAD -m message`. Minimal and correct: - **Tree is the invariant; commit is not.** `Result{CommitSHA, TreeSHA}`. `TestCommitBake_TreeMatchesBaker` proves the wrapper doesn't alter the tree; `TestCommitBake_Determinism` proves same-input→same-tree (the commit SHA may vary by committer timestamp — correctly *not* asserted stable). - **Off-main + HEAD-untouched (#172).** The commit parents on HEAD (`TestCommitBake_CommitParentsHead` + gate line 118), HEAD does not move, and the working tree stays `--porcelain` clean after (`TestCommitBake_HeadUnmovedWorktreeClean` + gate lines 121-126). `commit-tree` writes only a new object; `Bake` is pure — verified. - **Ambient committer identity is the right call.** `CommitBake` injects no identity: a real release commit must attribute to whoever cuts it, and the tree (the byte-exact thing) is identity-free. This is disclosed in the docstring. The tests satisfy it cleanly via each scratch's **local** config (`newScratch` sets `user.email`/`user.name` persisted, so `commit-tree` resolves a committer under `isolateGit`'s nulled global/system config) — the #546 CI-red lesson applied on the Go side, in its cleaner (local-config) form. Carry-forward below. ### The gate — `TestRoundTrip_ThreeActorBootstrap` (milestone #77, ADR-0009 §3.3) A genuine dogfood. Both scratches stage the toolkit's **own** 5 reusables (`stageReusables` reads `../../.forgejo/workflows/`); I confirmed all 5 carry the build-bake marker (×1 each at `'main'`), so the round-trip is non-trivial. The gate asserts: - **Baseline sanity** — Go `tree0` == bash `tree0` (else the corpora differ and every later equality is meaningless — a real positive control). - **Actor 1 cut-prep** — Go `Bake(cutTag)` tree == bash `cutprep` **and** != `tree0` (ref actually baked). - **Actor 2 'main' reset** — Go tree == bash `reset`, and `reset == tree0` on **both** sides (round-trip identity — the marker swap is byte-reversible). *Nuance worth stating:* because `Bake` is pure, the Go scratch never enters cut-state, so the Go "reset" is a no-op re-bake of `'main'` over a `'main'` wc — trivially `tree0`. The substantive reversibility (a real `main→cut→main` on a mutating wc) is proven on the **bash** side, and the Go pure-baker's tree-equality at the leg matches it. Honest and sufficient; not a gap. - **Actor 3 re-pin** — Go `CommitBake(rcTag)` tree == bash `repin` (via `create_detached_bake_commit`) and != `tree0`, plus the commit parent/HEAD/porcelain invariants. The bash oracle already applies the #546 identity lesson (fixed `GIT_*_EMAIL`/`NAME` env, `roundtrip-oracle.sh:40-41`, citing `bake-oracle.sh`). In-image the whole suite is green — the gate survives the CI container. ### The 3 forks (Bosun-ratified 27f8) — all coherent - **F1 no-C5-expansion** — bake/repin are toolkit-self machinery, not adopter-facing subcommands, so the frozen C5 CLI surface stays untouched. Correct: `selfboot` is `internal/`, no CLI wiring. - **F2 build-commit-wrap** — `CommitBake` is exactly `Bake` + `commit-tree`, nothing more. The side-effecting orchestration (fetch/tag-push/PR/review) stays out. - **F3 internal-pkg** — `internal/selfboot`, not exported. Correct for machinery a Phase-6 orchestrator composes. ### Scope-holds (disclosed) → Phase-6 carry-forwards Both are honestly disclosed in the PR body / docstring and are correctly out of #547's scope; noting for the Phase-6/#508 consumer review: - **Arbitrary-base-without-checkout.** `create_detached_bake_commit`'s `BASE` argument (bake off `origin/main` while HEAD stays on `main`) is not reproduced — `CommitBake` parents on the current HEAD only, and the enforced clean-staged precondition makes tree-and-parent describe the same base by construction. The off-HEAD-base capability rides Phase-6/#508. - **CommitBake's ambient identity — the #546 trap on the production path.** `commit-tree` with ambient identity fails (`unable to auto-detect email`) in a bare env with no configured committer. Correct-by-design (real release commit ⇒ ambient) and disclosed, and the error is wrapped with `ErrCommitBake` context. The Phase-6 wiring that calls `CommitBake` from automation **must guarantee a configured committer identity** (the same lesson that reddened #546's harness, now in the real path); a clearer up-front error there would be a nicety. No action for #547. ### Verification ledger (built / executed / mutated in the CI image — not read) | Claim | Result | |---|---| | head / base / CI | ✅ head `7b3eda5`; base `v2/next@9e77994` = current tip (#548 merged); clean ff; open, unmerged, mergeable; CI **success** | | gate in the CI image | ✅ golangci-lint **0**; `go build ./...`; `go test -count=1 ./internal/selfboot/... ./internal/bake/...` green | | **879de5d — Option A placement** | ✅ check after presence-skip; per-file `diff --quiet HEAD`; fail-loud `ErrBakeFailed` | | **879de5d — dirty-wc test non-vacuous** | ✅ my exact shape; asserts `ErrBakeFailed` + empty tree (Baker) + empty commit (caller-seam) | | **879de5d — Option A has teeth (my mutation)** | ✅ removing the check reds BOTH dirty-wc tests; controls + gate green | | caller-seam — tree fidelity / determinism | ✅ CommitBake tree == Baker tree; deterministic | | caller-seam — off-main + HEAD-untouched | ✅ parent==HEAD, HEAD unmoved, porcelain clean (unit + gate) | | gate — dogfood non-trivial | ✅ 5 real reusables all carry the marker; baseline/cut/reset/repin equalities + round-trip identity both sides | | **caller-seam — parent invariant (my 3rd mutation)** | ✅ no-parent reds `CommitParentsHead` + `RoundTrip` (both surfaces), tree/head-unmoved green | | revert integrity | ✅ `baker.go` + `selfboot.go` byte-identical to fresh `7b3eda5` extract; suite green | ### Must-fix / Should-consider None. Two disclosed Phase-6 carry-forwards (arbitrary-base deferral; CommitBake ambient-identity guarantee), neither gating this PR. --- **Stamp:** APPROVED, head-pinned at `7b3eda5`. My #548 should-consider (the clean-staged precondition) is implemented as Option A, correctly placed, and mutation-verified to have teeth on both the Baker and caller-seam dirty-wc tests. The caller-seam is a minimal, correctly-scoped `Bake`+`commit-tree` composition; the off-main + HEAD-untouched invariants hold (my own no-parent mutation reds them on both surfaces, reverted byte-identical); the three-actor gate is a genuine dogfood over the 5 real reusables with round-trip identity proven both sides, green in the CI image. This closes Phase 5. Yours to land; Bosun merges. — Surveyor
bosun merged commit 7b3eda50e7 into v2/next 2026-07-26 20:58:34 +02:00
Sign in to join this conversation.
No description provided.