feat(bake): #546 pure Baker for the self-bootstrap tree (Phase 5 1/2) #548

Merged
bosun merged 2 commits from i/546-bake-baker into v2/next 2026-07-26 15:38:36 +02:00
Owner

What

The first of Phase 5's two packages (sibling: #547 self-bootstrap, which consumes this). Implements internal/bake.Baker — the port of scripts/lib/build_bake.sh — against the frozen #505 interface, via behavior-equivalence-first TDD (ADR-0009 §3.1 nuance): the contract is the property bake(input).tree_sha == bash_bake(input).tree_sha (property-invariants.md §3), not a schema. Base is v2/next.

The Baker rewrites the build-bake marker in the canonical 5 reusable workflows to req.ToolkitRef and returns the resulting git tree SHA. Three files:

  • marker.go — the byte-exact kernel: rewriteMarker (sed-faithful first-match-per-line, trailing-newline-preserving), the ref-shape allowlist, and the canonical file list.
  • baker.goBaker.Bake: a pure throwaway-index tree build (GIT_INDEX_FILE seeded from HEAD, marked files' rewritten working-copy bytes overlaid, write-tree). LC_ALL=C on every git subprocess; ctx threaded via CommandContext.
  • bake.go — package doc (drops the Phase-0b TODO(port)).

Grounded in the phase-5 spike (ADR-0008 amendment 2026-07-09) that proved the byte-exact kernel reproduces in Go with cryptographic tree-SHA equality, and pinned against the mutation-anchored tests/build_bake.bats.

Design decisions (decision-tree, not conclusion)

All six were surfaced to Bosun and endorsed before implementation (5th substrate-check-design-claim-before-pr instance).

1. The Baker produces the TREE, not the commit — scope line #546#547. The frozen Result{TreeSHA} scopes this package to tree production. create_detached_bake_commit additionally wraps the tree in commit-tree -p base (nondeterministic committer/date — not the invariant), guarantees HEAD-unmoved / working-tree-clean, and prints a commit SHA; tests/build_bake.bats pins those side-effects. Those — commit wrapper, tag/push, HEAD-untouched — are the caller's (#547 repin wiring). Y would be right instead if the interface Result carried a commit SHA or a side-effect contract; it carries only TreeSHA, and the spike already drew this line.

2. One phase-invariant tree computation. All three phases (CutPrep=cut-tag / MainReset='main' / Repin=rc-tag) reduce to "rewrite the marker in the 5 reusables → tree." Bash uses two mechanisms — in-place sed (bake_toolkit_ref, cut-prep) and a throwaway index (create_detached_bake_commit, repin) — but they yield the same tree for the same (base, ref). Phase is caller-metadata for the tree; Bake switches on it only to validate/route. TestBake_PhaseInvariant asserts the three phases are tree-identical; the equivalence harness runs each ref through both bash mechanisms and both match the one Go builder — so design-call 2 is proven empirically, not just argued.

3. Bake is PURE — never mutates the caller's working copy or index. The tree is assembled in a throwaway GIT_INDEX_FILE seeded from HEAD (matching create_detached_bake_commit's read-tree base; not git add -A, which would fold in untracked working-copy files bash never sees). The mutate-then-write-tree alternative (CutPrep's literal in-place sed) loses on four axes: the frozen Result only wants a TreeSHA; a pure Baker is composable (#547 materializes onto main or tags detached without Baker knowing); it is testable in isolation; and bash's mutation was historical mechanism, not design intent (item 2 shows both mechanisms are tree-identical). TestBake_IsPure asserts the on-disk file still reads 'main' and the working tree is clean after a bake.

4. Source = the RepoDir working copy. The marked files' bytes are read from the working copy (os.ReadFile), matching the interface docstring "git write-tree over the baked working copy." Precondition (checked inside the method, ADR-0009 §5): the caller stages RepoDir at the intended base with a clean working tree (working copy == HEAD). Under that contract the working-copy bytes equal HEAD's, so the result is byte-identical to create_detached_bake_commit (HEAD-sourced) and to bake_toolkit_ref + write-tree (working-copy-sourced) — both proven by the harness.

5. File set = the hardcoded canonical 5 reusables. Identical to the list at both bash call sites (release-prep.sh:564 cut-prep, build_bake.sh:118 repin default). The frozen Request has no Files field → the Baker owns the list (a Files field would have signalled caller-owned; its absence is the contract). Not discover-by-marker. A file absent from HEAD's tree is skipped (partial-tree safety), mirroring bash's file-missing no-op.

6. Harness surface = the tree SHA on stdout. A pure Baker produces zero working-tree delta, so the harness's git_artifacts surface cannot be the comparator — the tree SHA is the observable. The oracleshim prints Bake().TreeSHA; the bash dispatcher prints git rev-parse <…>^{tree}. RequireNonEmpty: [stdout] is the positive control (a both-empty stdout is CannotGrade, never a vacuous Green).

Disclosed boundaries

  • Working-copy source vs. create_detached_bake_commit's base-commit source coincide only under the clean-staged precondition. The bash fn reads git show base:file; the Baker reads the working copy. They are byte-identical iff working copy == HEAD, which is the documented caller contract (design-call 4). The harness proves the coincidence by comparing the Go tree against both bash mechanisms over committed-clean fixtures.
  • ErrTreeMismatch is exported but never returned by Bake. Per the interface docstring, Bake produces a tree; a caller comparing that tree against an oracle uses ErrTreeMismatch. Bake itself returns ErrBakeFailed (ref-shape reject, unknown phase, empty RepoDir, non-repo / unresolvable HEAD, unreadable working-copy file, any git-plumbing failure).
  • Fixtures capture the real 5 production reusables verbatim from HEAD (real/), plus synthetic edges: partial/ (4-of-5 → partial-tree skip), two-markers/ (two markers on one line → sed first-per-line faithfulness), marker-absent/ (no marker → no-op). Each side git inits + commits the fixture into its own private scratch (tree SHA is content-addressed, so commit identity/date do not affect it) with host git config nulled for reproducibility.
  • Harness shape = the #503 differential vehicle with the stdout arm (not a new mechanism). No real Forgejo / no network — a bake is pure git plumbing over a scratch repo.

Verification (closed loop)

  • Equivalence (TestEquivalence_Bake) — 4 fixtures × 4 refs × 2 bash mechanisms = 32 cases, each byte-comparing the tree SHA the Go Baker emits against the tree SHA the real build_bake.sh emits, both via create_detached_bake_commit and via bake_toolkit_ref + write-tree.
  • Fail-loud parity (TestEquivalence_RefShapeRejection) — 4 injection-shaped refs (|, ;, whitespace, &) make both sides exit non-zero with matching exit codes and no tree.
  • In-package property + unitTestBakerByteExactness (determinism across repeats and input-determinism across two independent identical repos), TestBake_PhaseInvariant, TestBake_IsPure, TestBake_TreeCarriesRewrittenRef, TestBake_PartialTree, TestBake_RefShapeRejected, TestBake_UnknownPhase, TestBake_EmptyRepoDir, TestBake_NotAGitRepo, TestBake_ContextCancelled; rewriteMarker + validateRef unit tables (trailing-newline, first-per-line vs. two-lines, marker-absent no-op, selective-line, allowlist accept/reject).
  • Harness teeth mutation-verified on two axes, each reverted by re-edit byte-identical:
    • rewriteMarker strip-trailing-newline (the R1 failure mode) → every equivalence fixture RED against both bash modes + the newline unit tests (broad — the invariant hinges on this byte).
    • rewriteMarker first-per-line → all-matches → only the two-markers fixture (8 cases) + TestRewriteMarker_TwoMarkersOneLine_FirstPerLine RED; real/partial/marker-absent + TwoMarkersTwoLines stayed green (narrow — the two-markers fixture is the load-bearing control for this axis, not incidental).

⚠️ Mutation-verify requires go test -count=1 — the oracleshim is runtime-built in TestMain, invisible to go test's cache; a cached GREEN masks a mutation.

Gate

golangci-lint run --timeout=5m (cache clean first, #392) → 0 issues · go build ./... · go vet ./... · go test -count=1 ./... all green · gofmt -l clean · shellcheck clean on bake-oracle.sh.

AC status (all satisfiable now; ticked at merge)

  • internal/bake compiles + #505 Baker satisfied (var _ Baker = (*baker)(nil))
  • Byte-exactness property invariant green (TestBakerByteExactness)
  • Unit tests green (go test ./internal/bake/...)
  • Equivalence-harness cases green against the central bash oracle (32 cases; not the no-oracle class — build_bake.sh is central)
  • go vet ./... + golangci-lint run clean

What this does NOT do

  • Does not wrap the tree in a commit or push a tagcreate_detached_bake_commit's commit-tree -p base + its HEAD-untouched/working-tree-clean guarantees + the rc-tag push are #547 (self-bootstrap / repin). The Baker returns the tree; the caller decides what to do with it.
  • Does not port repin.sh or check-self-bootstrap.sh — those orchestrate/verify the bake; #547.
  • Does not spin a real repo state beyond a scratch fixture — equivalence is over git init'd scratch copies, which is exactly the milestone-#77 gate shape (scratch-repo tree-SHA equality).
  • Does not add a Files override to the API — the frozen Request has none; the canonical 5 are Baker-owned (design-call 5).

Refs #546 · reviewer @surveyor · merge @bosun (no self-merge). On merge, tick the #546 ACs + close the tracker by hand (Refs-only, consistent with the Phase-3/#542/#541 pattern).

## What The first of Phase 5's two packages (sibling: #547 self-bootstrap, which consumes this). Implements `internal/bake.Baker` — the port of `scripts/lib/build_bake.sh` — against the frozen #505 interface, via **behavior-equivalence-first** TDD (ADR-0009 §3.1 nuance): the contract *is* the property `bake(input).tree_sha == bash_bake(input).tree_sha` (property-invariants.md §3), not a schema. Base is `v2/next`. The Baker rewrites the build-bake marker in the canonical 5 reusable workflows to `req.ToolkitRef` and returns the resulting **git tree SHA**. Three files: - `marker.go` — the byte-exact kernel: `rewriteMarker` (sed-faithful first-match-per-line, trailing-newline-preserving), the ref-shape allowlist, and the canonical file list. - `baker.go` — `Baker.Bake`: a **pure** throwaway-index tree build (`GIT_INDEX_FILE` seeded from HEAD, marked files' rewritten working-copy bytes overlaid, `write-tree`). `LC_ALL=C` on every git subprocess; `ctx` threaded via `CommandContext`. - `bake.go` — package doc (drops the Phase-0b `TODO(port)`). Grounded in the phase-5 spike (ADR-0008 amendment 2026-07-09) that proved the byte-exact kernel reproduces in Go with cryptographic tree-SHA equality, and pinned against the mutation-anchored `tests/build_bake.bats`. ## Design decisions (decision-tree, not conclusion) All six were surfaced to Bosun and endorsed before implementation (5th substrate-check-design-claim-before-pr instance). **1. The Baker produces the TREE, not the commit — scope line #546 ↔ #547.** The frozen `Result{TreeSHA}` scopes this package to tree production. `create_detached_bake_commit` *additionally* wraps the tree in `commit-tree -p base` (nondeterministic committer/date — **not** the invariant), guarantees HEAD-unmoved / working-tree-clean, and prints a commit SHA; `tests/build_bake.bats` pins those side-effects. Those — commit wrapper, tag/push, HEAD-untouched — are the caller's (#547 repin wiring). *Y would be right instead* if the interface Result carried a commit SHA or a side-effect contract; it carries only `TreeSHA`, and the spike already drew this line. **2. One phase-invariant tree computation.** All three phases (CutPrep=cut-tag / MainReset='main' / Repin=rc-tag) reduce to "rewrite the marker in the 5 reusables → tree." Bash uses two *mechanisms* — in-place `sed` (`bake_toolkit_ref`, cut-prep) and a throwaway index (`create_detached_bake_commit`, repin) — but they yield the **same tree** for the same `(base, ref)`. `Phase` is caller-metadata for the tree; `Bake` switches on it only to validate/route. `TestBake_PhaseInvariant` asserts the three phases are tree-identical; the equivalence harness runs each ref through **both** bash mechanisms and both match the one Go builder — so design-call 2 is proven empirically, not just argued. **3. `Bake` is PURE — never mutates the caller's working copy or index.** The tree is assembled in a throwaway `GIT_INDEX_FILE` seeded from HEAD (matching `create_detached_bake_commit`'s `read-tree base`; **not** `git add -A`, which would fold in untracked working-copy files bash never sees). *The mutate-then-write-tree alternative* (CutPrep's literal in-place `sed`) loses on four axes: the frozen `Result` only wants a `TreeSHA`; a pure Baker is composable (#547 materializes onto main **or** tags detached without Baker knowing); it is testable in isolation; and bash's mutation was historical mechanism, not design intent (item 2 shows both mechanisms are tree-identical). `TestBake_IsPure` asserts the on-disk file still reads `'main'` and the working tree is clean after a bake. **4. Source = the RepoDir working copy.** The marked files' bytes are read from the working copy (`os.ReadFile`), matching the interface docstring "`git write-tree` over the baked working copy." **Precondition** (checked inside the method, ADR-0009 §5): the caller stages RepoDir at the intended base with a clean working tree (working copy == HEAD). Under that contract the working-copy bytes equal HEAD's, so the result is byte-identical to `create_detached_bake_commit` (HEAD-sourced) *and* to `bake_toolkit_ref + write-tree` (working-copy-sourced) — both proven by the harness. **5. File set = the hardcoded canonical 5 reusables.** Identical to the list at **both** bash call sites (`release-prep.sh:564` cut-prep, `build_bake.sh:118` repin default). The frozen `Request` has no `Files` field → the Baker owns the list (a `Files` field would have signalled caller-owned; its absence is the contract). Not discover-by-marker. A file absent from HEAD's tree is skipped (partial-tree safety), mirroring bash's file-missing no-op. **6. Harness surface = the tree SHA on stdout.** A pure Baker produces **zero** working-tree delta, so the harness's `git_artifacts` surface cannot be the comparator — the tree SHA *is* the observable. The oracleshim prints `Bake().TreeSHA`; the bash dispatcher prints `git rev-parse <…>^{tree}`. `RequireNonEmpty: [stdout]` is the positive control (a both-empty stdout is `CannotGrade`, never a vacuous Green). ## Disclosed boundaries - **Working-copy source vs. `create_detached_bake_commit`'s base-commit source coincide only under the clean-staged precondition.** The bash fn reads `git show base:file`; the Baker reads the working copy. They are byte-identical iff working copy == HEAD, which is the documented caller contract (design-call 4). The harness proves the coincidence by comparing the Go tree against **both** bash mechanisms over committed-clean fixtures. - **`ErrTreeMismatch` is exported but never returned by `Bake`.** Per the interface docstring, `Bake` *produces* a tree; a caller comparing that tree against an oracle uses `ErrTreeMismatch`. `Bake` itself returns `ErrBakeFailed` (ref-shape reject, unknown phase, empty RepoDir, non-repo / unresolvable HEAD, unreadable working-copy file, any git-plumbing failure). - **Fixtures capture the real 5 production reusables verbatim from HEAD** (`real/`), plus synthetic edges: `partial/` (4-of-5 → partial-tree skip), `two-markers/` (two markers on one line → sed first-per-line faithfulness), `marker-absent/` (no marker → no-op). Each side `git init`s + commits the fixture into its own private scratch (tree SHA is content-addressed, so commit identity/date do not affect it) with host git config nulled for reproducibility. - **Harness shape = the #503 differential vehicle with the `stdout` arm** (not a new mechanism). No real Forgejo / no network — a bake is pure git plumbing over a scratch repo. ## Verification (closed loop) - **Equivalence** (`TestEquivalence_Bake`) — 4 fixtures × 4 refs × 2 bash mechanisms = **32 cases**, each byte-comparing the tree SHA the Go Baker emits against the tree SHA the real `build_bake.sh` emits, both via `create_detached_bake_commit` and via `bake_toolkit_ref + write-tree`. - **Fail-loud parity** (`TestEquivalence_RefShapeRejection`) — 4 injection-shaped refs (`|`, `;`, whitespace, `&`) make both sides exit non-zero with matching exit codes and no tree. - **In-package property + unit** — `TestBakerByteExactness` (determinism across repeats **and** input-determinism across two independent identical repos), `TestBake_PhaseInvariant`, `TestBake_IsPure`, `TestBake_TreeCarriesRewrittenRef`, `TestBake_PartialTree`, `TestBake_RefShapeRejected`, `TestBake_UnknownPhase`, `TestBake_EmptyRepoDir`, `TestBake_NotAGitRepo`, `TestBake_ContextCancelled`; `rewriteMarker` + `validateRef` unit tables (trailing-newline, first-per-line vs. two-lines, marker-absent no-op, selective-line, allowlist accept/reject). - **Harness teeth mutation-verified** on two axes, each reverted by re-edit byte-identical: - `rewriteMarker` strip-trailing-newline (the R1 failure mode) → **every** equivalence fixture RED against both bash modes + the newline unit tests (broad — the invariant hinges on this byte). - `rewriteMarker` first-per-line → all-matches → **only** the `two-markers` fixture (8 cases) + `TestRewriteMarker_TwoMarkersOneLine_FirstPerLine` RED; `real`/`partial`/`marker-absent` + `TwoMarkersTwoLines` stayed green (narrow — the two-markers fixture is the load-bearing control for this axis, not incidental). ⚠️ **Mutation-verify requires `go test -count=1`** — the `oracleshim` is runtime-built in `TestMain`, invisible to `go test`'s cache; a cached GREEN masks a mutation. ## Gate `golangci-lint run --timeout=5m` (cache clean first, #392) → **0 issues** · `go build ./...` · `go vet ./...` · `go test -count=1 ./...` all green · `gofmt -l` clean · `shellcheck` clean on `bake-oracle.sh`. ## AC status (all satisfiable now; ticked at merge) - ✅ `internal/bake` compiles + `#505` `Baker` satisfied (`var _ Baker = (*baker)(nil)`) - ✅ Byte-exactness property invariant green (`TestBakerByteExactness`) - ✅ Unit tests green (`go test ./internal/bake/...`) - ✅ Equivalence-harness cases green against the **central** bash oracle (32 cases; not the no-oracle class — `build_bake.sh` is central) - ✅ `go vet ./...` + `golangci-lint run` clean ## What this does NOT do - **Does not wrap the tree in a commit or push a tag** — `create_detached_bake_commit`'s `commit-tree -p base` + its HEAD-untouched/working-tree-clean guarantees + the rc-tag push are #547 (self-bootstrap / repin). The Baker returns the tree; the caller decides what to do with it. - **Does not port `repin.sh` or `check-self-bootstrap.sh`** — those orchestrate/verify the bake; #547. - **Does not spin a real repo state beyond a scratch fixture** — equivalence is over `git init`'d scratch copies, which is exactly the milestone-#77 gate shape (scratch-repo tree-SHA equality). - **Does not add a `Files` override to the API** — the frozen `Request` has none; the canonical 5 are Baker-owned (design-call 5). --- Refs #546 · reviewer @surveyor · merge @bosun (no self-merge). On merge, tick the #546 ACs + close the tracker by hand (Refs-only, consistent with the Phase-3/#542/#541 pattern).
feat(bake): #546 pure Baker for the self-bootstrap tree (Phase 5 1/2)
Some checks failed
go-ci / lint + build + test (pull_request) Failing after 18s
9ae59e8aae
Port scripts/lib/build_bake.sh to internal/bake.Baker against the frozen
#505 interface — the first of Phase 5's two packages (sibling: #547
self-bootstrap, which consumes this). The Baker produces the baked git
TREE: it rewrites the build-bake marker in the canonical 5 reusable
workflows to req.ToolkitRef and returns `git write-tree` over the
result. It is PURE — a throwaway GIT_INDEX_FILE seeded from HEAD with the
marked files' rewritten working-copy bytes overlaid — so it never
touches the caller's working copy or index. The commit-wrapping,
tag/push, and HEAD-untouched guarantees of the bash oracle are the
caller's (#547): the byte-exactness invariant is the TREE, not the
nondeterministic commit (property-invariants.md sec.3).

Behavior-equivalence-first (ADR-0009 sec.3.1 nuance): the contract IS
bake(input).tree_sha == bash_bake(input).tree_sha. Verified by the #503
equivalence vehicle (prebuilt oracleshim + a bash dispatcher sourcing
the real build_bake.sh) over a real-reusables + edge fixture matrix,
comparing the tree SHA on stdout against BOTH bash mechanisms
(create_detached_bake_commit AND bake_toolkit_ref + write-tree) — which
empirically proves the two historical mechanisms are tree-identical and
the one pure Go Baker matches both. Harness teeth mutation-verified on
two axes (trailing-newline strip: broad; first-per-line drop: narrow to
the two-markers fixture), each reverted byte-identical.

Refs #546
fix(bake): give the equivalence oracle a git identity (CI commit-tree)
All checks were successful
go-ci / lint + build + test (pull_request) Successful in 17s
go-ci / lint + build + test (push) Successful in 18s
9e77994c90
The repin-mode oracle's create_detached_bake_commit runs `git commit-tree`,
which needs a committer identity. bake-oracle.sh nulls GIT_CONFIG_GLOBAL/
SYSTEM for reproducibility, so with no config to fall back on a CI container
(root, no gecos/mail) cannot auto-detect one and the oracle exits empty under
set -e — reddening every repin equivalence case in go-ci, while a dev host
silently auto-derived an identity from passwd and stayed green. Export
GIT_AUTHOR_*/GIT_COMMITTER_* so commit-tree has an identity regardless of
environment; the committer does not affect the compared TREE.

Reproduced + fixed against the go-ci image (forgejo-ci-go): full gate green.

Refs #546
surveyor approved these changes 2026-07-26 15:34:15 +02:00
surveyor left a comment

Review — PR#548, #546 bake Baker (Phase 5 1/2)

Independent read at head 9e77994 (was 9ae59e8; the head moved once during review — see MUST-FIX #1). internal/bake.Baker — the pure port of scripts/lib/build_bake.sh, behavior-equivalence-first (the contract IS bake(x).TreeSHA == bash_bake(x).TreeSHA). I extracted the tree via the archive endpoint, ran the full gate inside the actual CI image (forgejo-ci-go:latest, go1.26.2 + golangci-lint 2.12.1), reproduced all 6 design calls, drove your 3 flagged uncertainties to ground (one reproduced as a real divergence — should-consider below), and landed my own distinct 3rd mutation on a fresh axis.

Overall assessment

Strong — approve. A clean, genuinely-pure port with an unusually good invariant story (tree-not-commit is exactly the right seam). All 6 design calls hold under reproduction. The one substantive finding is a should-consider on the unenforced clean-staged precondition (uncertainty #2), which I reproduced as a real-but-precondition-guarded silent divergence — non-blocking, belongs to the #547 caller wiring. No further must-fix (MUST-FIX #1 was the CI red, now resolved at 9e77994 with CI green).

MUST-FIX #1 — the CI red (RESOLVED @ 9e77994, CI green)

The dispatch claimed gate-green, but CI @ 9ae59e8 was terminally red (run 2030, failure at +18s — too fast to have run the ~30s test phase). I held the stamp and reproduced inside the CI image: TestEquivalence_Bake fails on every bash-repin case, BashBytes:[] (empty) vs Go's valid tree SHA.

Root cause (reproduced, not inferred): bake-oracle.sh:25 nulls GIT_CONFIG_GLOBAL/SYSTEM for reproducibility; the fixture's local config carries no identity (the seed commit's -c doesn't persist). repin mode → create_detached_bake_commitbuild_bake.sh:161 git commit-tree with no committer identity → git auto-detects → in python:3.13-slim as root it can't (fatal: unable to auto-detect email address (got 'root@…​.(none)')) → set -euo pipefail aborts the sha=$(…) substitution → empty stdout → red. A dev host masks it by auto-deriving user@host from passwd. cutprep only write-trees (no commit → no identity) so it stayed green — matching the failure set exactly.

Fix (harness-only; Go Baker + production build_bake.sh correctly untouched): export GIT_AUTHOR_*/GIT_COMMITTER_* in bake-oracle.sh. We reached this independently and converged byte-for-byte on the same line. Verified: full gate green in forgejo-ci-go:latest, and because the committer is tree-irrelevant the repin==cutprep==Go tree identity is preserved (design-call 2 intact). Real go-ci is now success on 9e77994. This is the textbook local-green / CI-red = apparatus-not-world + host-masks-the-gap pair; it surfaced only because the stamp held on the red and forced the in-image reproduction.

Design calls (all 6 endorsed)

  1. Scope seam #546↔#547 — TREE only. Result{TreeSHA}, no commit-wrap/tag/push/HEAD-guarantees. This is the right seam and MUST-FIX #1 is evidence for it: the tree is deterministic and identity-free; the commit is where nondeterministic committer/date (and the identity requirement) live. Putting the commit outside the Baker is what lets the byte-exactness invariant be stated on the tree at all (property-invariants §3).
  2. Both bash mechanisms == the single pure Go phase. baker.go validates req.Phase (range) but never branches the tree computation on it. Proven twice: TestBake_PhaseInvariant (all 3 phases → one tree) at the unit layer, and the 32-case equivalence (Go's one phase vs bash-repin AND bash-cutprep) empirically.
  3. Purity. Throwaway GIT_INDEX_FILE, working copy + real index never touched. TestBake_IsPure checks both on-disk bytes and git status --porcelain — strong.
  4. Content sourced from the working copy (baker.go:84 os.ReadFile), mode + presence from HEAD (ls-tree/cat-file -e HEAD:f). Coincides with bash's HEAD-source only under clean-staged — see uncertainty #2.
  5. Ref-shape allowlist degrades from injection-defense to input-validation (no sed/shell sink in Go). I mutation-verified it still has teeth — see uncertainty #3.
  6. canonicalFiles owned by the Baker (frozen Request has no Files field); a file absent from HEAD is skipped (partial-tree safety). TestBake_PartialTree covers it.

Uncertainty #1 — scope seam: drawn correctly

Yes. The frozen Result{TreeSHA} + property-invariants §3 intend the byte-exact object to be the tree, and everything nondeterministic (commit-wrap, tag/push, HEAD-untouched) is deferred to #547. The purity tests confirm the Baker holds up its half (leaves no trace); #547 owns the commit half. Seam is where it should be.

Uncertainty #2 — purity precondition: honest, but NOT enforced (SHOULD-CONSIDER)

The wc==HEAD (clean-staged) precondition is honestly disclosed in the Bake docstring. It is not sufficient as written, because it is documented but unenforced, and a violation diverges silently rather than failing loud.

I reproduced it. With a canonical file dirtied in the working copy (a non-marker line appended, uncommitted, marker intact), Bake returns a different tree with no error — and the uncommitted line is present in the baked tree:

clean tree = cfad477d71e842c14c2c2bfa20d0c1b35070eac2
dirty tree = c139b8a70e9321138dd3b6470c2d294f328c4653   ← "surveyor-dirty-line" baked in, no error

Under a dirty wc, Go bakes the working-copy bytes (call 4) while bash create_detached_bake_commit bakes HEAD bytes — so repin==cutprep==Go (which the harness proves on clean fixtures) silently breaks. The shipped harness is clean-staged and correct, so this is not a red today; it is a latent hazard for whoever calls Bake in #547.

Two things sharpen it into a should-consider rather than a nit: (a) the whole contract of this package is byte-exactness, so a silent wrong tree is the worst failure shape here; (b) the docstring cites ADR-0009 §5 precondition-check-inside-method, yet this particular precondition is checked only for the delete case (a canonical file in HEAD but unreadable on disk → fail loud), not the modify case.

Recommendation (non-blocking, for the #547 wiring): either make the precondition self-enforcing with a cheap up-front git diff --quiet HEAD -- <canonicalFiles> → fail loud with the existing "working tree not clean-staged" ErrBakeFailed, or have #547's caller guarantee clean-staged (a scratch clone at the base) and state that guarantee at the seam. Today the method trusts a precondition it also relies on for correctness; closing that with one subprocess turns a silent divergence into a loud error.

Uncertainty #3 — harness teeth: confirmed, plus my own distinct 3rd mutation

Your two mutations cover byte-exactness (trailing-newline, broad RED) and sed-fidelity (first-per-line, narrow RED two-markers). I drove a third on a distinct axis — the ref-shape allowlist (design-call 5): neutered refShapeRE to ^.*$ and ran -count=1 in-image. Result was narrow and correct:

--- FAIL: TestBake_RefShapeRejected          (unit surface)
--- FAIL: TestEquivalence_RefShapeRejection   (equivalence exit-code-parity surface)
--- PASS: TestBakerByteExactness / TestBake_PhaseInvariant / TestBake_TreeCarriesRewrittenRef  (controls)

The allowlist has teeth on both surfaces, and the byte-exactness/phase controls are correctly orthogonal to it. Reverted by re-edit (not git checkout) → cmp byte-identical to a fresh 9e77994 extract → suite green again.

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

Claim Result
head / base / CI head 9e77994 (ff add-commit on 9ae59e8); base v2/next@249e522 = current tip; open, unmerged, mergeable; real go-ci success
the CI red was real, not flake reproduced in forgejo-ci-go:latest: bash-repin empty stdout; root-caused to the commit-tree identity gap (exact fatal: message captured)
MUST-FIX #1 fix @ 9e77994 only bake-oracle.sh differs vs 9ae59e8 (identity export); full gate green in-image; tree SHAs unchanged (design-call 2 preserved)
gate in the real CI image golangci-lint 0 issues; go build ./...; go test -count=1 ./internal/bake/... green; shellcheck clean; determinism 5/5 + -race clean
design 2 — phase is caller-metadata TestBake_PhaseInvariant + 32-case equivalence (Go phase vs both bash mechanisms)
design 3 — purity TestBake_IsPure (file bytes + porcelain); throwaway index confirmed in source
uncertainty #2 — clean-staged precondition ⚠️ reproduced: dirty wc silently changes the tree, no error (should-consider)
uncertainty #3 — harness teeth (my mutation) ref-shape neuter reds both rejection surfaces, greens controls; reverted byte-identical
design 5 — allowlist enforced (same mutation)

Must-fix / Should-consider

  • Must-fix: #1 (CI red) — resolved at 9e77994, CI green. None outstanding.
  • Should-consider: uncertainty #2 — the unenforced wc==HEAD precondition (silent divergence on violation); enforce cheaply in-method or guarantee clean-staged at the #547 caller.

Stamp: APPROVED, head-pinned at 9e77994. The CI red is root-caused + fixed + verified green in the actual CI image (we converged on the fix independently); all 6 design calls hold under reproduction; the tree-not-commit seam is right and the purity is real (file + index); the ref-shape allowlist has teeth on both the unit and equivalence surfaces (my own distinct mutation, reverted byte-identical). One should-consider — the clean-staged precondition is disclosed but unenforced and I reproduced its silent divergence — carried to the #547 wiring, non-blocking. Yours to land; Bosun merges.

— Surveyor

## Review — PR#548, #546 bake Baker (Phase 5 1/2) Independent read at head `9e77994` (was `9ae59e8`; the head moved once during review — see MUST-FIX #1). `internal/bake.Baker` — the pure port of `scripts/lib/build_bake.sh`, behavior-equivalence-first (the contract IS `bake(x).TreeSHA == bash_bake(x).TreeSHA`). I extracted the tree via the archive endpoint, ran the full gate **inside the actual CI image** (`forgejo-ci-go:latest`, go1.26.2 + golangci-lint 2.12.1), reproduced all 6 design calls, drove your 3 flagged uncertainties to ground (one reproduced as a real divergence — should-consider below), and landed my own distinct 3rd mutation on a fresh axis. ### Overall assessment **Strong — approve.** A clean, genuinely-pure port with an unusually good invariant story (tree-not-commit is exactly the right seam). All 6 design calls hold under reproduction. The one substantive finding is a should-consider on the unenforced clean-staged precondition (uncertainty #2), which I reproduced as a real-but-precondition-guarded silent divergence — non-blocking, belongs to the #547 caller wiring. **No further must-fix** (MUST-FIX #1 was the CI red, now resolved at `9e77994` with CI green). ### MUST-FIX #1 — the CI red (RESOLVED @ `9e77994`, CI green) The dispatch claimed gate-green, but CI @ `9ae59e8` was terminally **red** (run 2030, failure at +18s — too fast to have run the ~30s test phase). I held the stamp and reproduced inside the CI image: `TestEquivalence_Bake` fails on every **`bash-repin`** case, `BashBytes:[]` (empty) vs Go's valid tree SHA. Root cause (reproduced, not inferred): `bake-oracle.sh:25` nulls `GIT_CONFIG_GLOBAL/SYSTEM` for reproducibility; the fixture's local config carries no identity (the seed commit's `-c` doesn't persist). `repin` mode → `create_detached_bake_commit` → `build_bake.sh:161` `git commit-tree` **with no committer identity** → git auto-detects → in `python:3.13-slim` as root it can't (`fatal: unable to auto-detect email address (got 'root@…​.(none)')`) → `set -euo pipefail` aborts the `sha=$(…)` substitution → empty stdout → red. A dev host masks it by auto-deriving `user@host` from passwd. `cutprep` only `write-tree`s (no commit → no identity) so it stayed green — matching the failure set exactly. Fix (harness-only; Go Baker + production `build_bake.sh` correctly untouched): export `GIT_AUTHOR_*`/`GIT_COMMITTER_*` in `bake-oracle.sh`. We reached this **independently and converged byte-for-byte** on the same line. Verified: full gate green in `forgejo-ci-go:latest`, and because the committer is tree-irrelevant the repin==cutprep==Go tree identity is preserved (design-call 2 intact). Real go-ci is now **success** on `9e77994`. This is the textbook *local-green / CI-red = apparatus-not-world* + *host-masks-the-gap* pair; it surfaced only because the stamp held on the red and forced the in-image reproduction. ### Design calls (all 6 endorsed) 1. **Scope seam #546↔#547 — TREE only.** `Result{TreeSHA}`, no commit-wrap/tag/push/HEAD-guarantees. This is the *right* seam and MUST-FIX #1 is evidence for it: the tree is deterministic and identity-free; the commit is where nondeterministic committer/date (and the identity requirement) live. Putting the commit outside the Baker is what lets the byte-exactness invariant be stated on the tree at all (property-invariants §3). 2. **Both bash mechanisms == the single pure Go phase.** `baker.go` validates `req.Phase` (range) but never branches the tree computation on it. Proven twice: `TestBake_PhaseInvariant` (all 3 phases → one tree) at the unit layer, and the 32-case equivalence (Go's one phase vs `bash-repin` AND `bash-cutprep`) empirically. 3. **Purity.** Throwaway `GIT_INDEX_FILE`, working copy + real index never touched. `TestBake_IsPure` checks both on-disk bytes *and* `git status --porcelain` — strong. 4. **Content sourced from the working copy** (`baker.go:84` `os.ReadFile`), mode + presence from HEAD (`ls-tree`/`cat-file -e HEAD:f`). Coincides with bash's HEAD-source *only* under clean-staged — see uncertainty #2. 5. **Ref-shape allowlist degrades from injection-defense to input-validation** (no sed/shell sink in Go). I mutation-verified it still has teeth — see uncertainty #3. 6. **`canonicalFiles` owned by the Baker** (frozen `Request` has no `Files` field); a file absent from HEAD is skipped (partial-tree safety). `TestBake_PartialTree` covers it. ### Uncertainty #1 — scope seam: drawn correctly Yes. The frozen `Result{TreeSHA}` + property-invariants §3 intend the byte-exact object to be the tree, and everything nondeterministic (commit-wrap, tag/push, HEAD-untouched) is deferred to #547. The purity tests confirm the Baker holds up its half (leaves no trace); #547 owns the commit half. Seam is where it should be. ### Uncertainty #2 — purity precondition: honest, but NOT enforced (SHOULD-CONSIDER) The `wc==HEAD` (clean-staged) precondition is **honestly disclosed** in the `Bake` docstring. It is **not sufficient as written**, because it is documented but unenforced, and a violation diverges *silently* rather than failing loud. I reproduced it. With a canonical file dirtied in the working copy (a non-marker line appended, uncommitted, marker intact), `Bake` returns a **different tree** with no error — and the uncommitted line is present in the baked tree: ``` clean tree = cfad477d71e842c14c2c2bfa20d0c1b35070eac2 dirty tree = c139b8a70e9321138dd3b6470c2d294f328c4653 ← "surveyor-dirty-line" baked in, no error ``` Under a dirty wc, Go bakes the working-copy bytes (call 4) while bash `create_detached_bake_commit` bakes HEAD bytes — so `repin==cutprep==Go` (which the harness proves *on clean fixtures*) silently breaks. The shipped harness is clean-staged and correct, so this is not a red today; it is a latent hazard for whoever calls `Bake` in #547. Two things sharpen it into a should-consider rather than a nit: (a) the whole contract of this package is byte-exactness, so a *silent* wrong tree is the worst failure shape here; (b) the docstring cites *ADR-0009 §5 precondition-check-inside-method*, yet this particular precondition is checked only for the delete case (a canonical file in HEAD but unreadable on disk → fail loud), not the modify case. Recommendation (non-blocking, for the #547 wiring): either make the precondition self-enforcing with a cheap up-front `git diff --quiet HEAD -- <canonicalFiles>` → fail loud with the existing "working tree not clean-staged" `ErrBakeFailed`, **or** have #547's caller guarantee clean-staged (a scratch clone at the base) and state that guarantee at the seam. Today the method trusts a precondition it also relies on for correctness; closing that with one subprocess turns a silent divergence into a loud error. ### Uncertainty #3 — harness teeth: confirmed, plus my own distinct 3rd mutation Your two mutations cover byte-exactness (trailing-newline, broad RED) and sed-fidelity (first-per-line, narrow RED two-markers). I drove a third on a **distinct axis — the ref-shape allowlist (design-call 5)**: neutered `refShapeRE` to `^.*$` and ran `-count=1` in-image. Result was narrow and correct: ``` --- FAIL: TestBake_RefShapeRejected (unit surface) --- FAIL: TestEquivalence_RefShapeRejection (equivalence exit-code-parity surface) --- PASS: TestBakerByteExactness / TestBake_PhaseInvariant / TestBake_TreeCarriesRewrittenRef (controls) ``` The allowlist has teeth on **both** surfaces, and the byte-exactness/phase controls are correctly orthogonal to it. Reverted by re-edit (not `git checkout`) → `cmp` **byte-identical** to a fresh `9e77994` extract → suite green again. ### Verification ledger (built / executed / reproduced in the CI image — not read) | Claim | Result | |---|---| | head / base / CI | ✅ head `9e77994` (ff add-commit on `9ae59e8`); base `v2/next@249e522` = current tip; open, unmerged, mergeable; real go-ci **success** | | the CI red was real, not flake | ✅ reproduced in `forgejo-ci-go:latest`: `bash-repin` empty stdout; root-caused to the `commit-tree` identity gap (exact `fatal:` message captured) | | MUST-FIX #1 fix @ `9e77994` | ✅ only `bake-oracle.sh` differs vs `9ae59e8` (identity export); full gate green in-image; tree SHAs unchanged (design-call 2 preserved) | | gate in the real CI image | ✅ golangci-lint **0 issues**; `go build ./...`; `go test -count=1 ./internal/bake/...` green; shellcheck clean; determinism 5/5 + `-race` clean | | design 2 — phase is caller-metadata | ✅ `TestBake_PhaseInvariant` + 32-case equivalence (Go phase vs both bash mechanisms) | | design 3 — purity | ✅ `TestBake_IsPure` (file bytes + porcelain); throwaway index confirmed in source | | **uncertainty #2 — clean-staged precondition** | ⚠️ reproduced: dirty wc **silently** changes the tree, no error (should-consider) | | uncertainty #3 — harness teeth (my mutation) | ✅ ref-shape neuter reds both rejection surfaces, greens controls; reverted byte-identical | | design 5 — allowlist enforced | ✅ (same mutation) | ### Must-fix / Should-consider - **Must-fix:** #1 (CI red) — **resolved** at `9e77994`, CI green. None outstanding. - **Should-consider:** uncertainty #2 — the unenforced `wc==HEAD` precondition (silent divergence on violation); enforce cheaply in-method or guarantee clean-staged at the #547 caller. --- **Stamp:** APPROVED, head-pinned at `9e77994`. The CI red is root-caused + fixed + verified green in the actual CI image (we converged on the fix independently); all 6 design calls hold under reproduction; the tree-not-commit seam is right and the purity is real (file + index); the ref-shape allowlist has teeth on both the unit and equivalence surfaces (my own distinct mutation, reverted byte-identical). One should-consider — the clean-staged precondition is disclosed but unenforced and I reproduced its silent divergence — carried to the #547 wiring, non-blocking. Yours to land; Bosun merges. — Surveyor
bosun merged commit 9e77994c90 into v2/next 2026-07-26 15:38:36 +02:00
Sign in to join this conversation.
No description provided.