feat(prep): rt prep orchestrator (Go port of release-prep.sh) — #556 #561

Merged

bosun merged 4 commits from i/556-rt-prep-port into main 2026-07-27 04:02:04 +02:00

Conversation 2 Commits 4 Files changed 31 +3028 -43

engineer commented 2026-07-27 03:45:58 +02:00

Owner

Copy link

rt prep — the Go port of scripts/release-prep.sh (#556)

Ports the release-prep orchestrator to rt prep. This is the whole of #556 across three commits on the branch:

• Piece 1 (619f402) — the Cutter seam: Request.Composed (B-request) + nil-Manifest skip.
• Piece 2 (ed4714f) — internal/prep compose library (fragments + git-walk CC history + manual [Unreleased] prose → changelog.VersionSection).
• Piece 3 (8f2fce1) — cmd/rt/prep.go orchestrator + git porcelain + hooks + version-files + bake + PR ops + the equivalence harness.

The flow mirrors release-prep.sh section-for-section: validate config → read CHANGELOG state → determine version → compose → seal + delete fragments (via the Cutter) → clear sidecar → bump version files → run post-bump hooks → bake toolkit ref → derive owner/repo + base branch → (prod) branch/commit/push → open/update the release-prep PR → emit the six machine-readable outputs.

Ratified contract (unchanged from framing)

• B-request (039f): rt prep OWNS the richer compose and hands a finished VersionSection to the Cutter via Request.Composed; nil preserves #554's fragment-only path byte-for-byte.
• nil-Manifest-skip (3be5): rt prep passes Deps.Manifest = nil; the manifest write is the post-merge cut's job (#557), keyed on the merge SHA. nil Composer is safe because Composed != nil (Prepare never calls Composer on that path).

Equivalence harness surfaced TWO latent bugs in merged code — both fixed here

internal/prep/equivalence_test.go runs the real scripts/release-prep.sh and the prebuilt rt binary against git-bootstrapped fixtures and byte-compares stdout, exit_code, and git_artifacts (the sealed CHANGELOG, the bumped VERSION, the fragment delete). Both cases (target-version + auto-bump dry-run) are green in the forgejo-ci-go:latest image, not just on the host.

1. changelog.MergeSections emit order was wrong (#532 defect). It used the Keep-a-Changelog order {Added Changed Deprecated Removed Fixed} and dropped Upgrade entirely (a kind absent from the list is skipped), while the oracle changelog_merge_sections uses CHANGELOG_STANDARD_SECTIONS order {Added Changed Fixed Removed Deprecated Upgrade}. Measured directly against the oracle. The two orders diverge only when Fixed, Removed, and Deprecated co-occur — which is why the #532 fixtures never distinguished them. ScaffoldMissingSections is unaffected. Also fixed the sibling has_none divergence: None. is a sentinel, not content — a scaffolded None. meeting real manual [Unreleased] prose for the same kind drops the None., it does not append it. Pinned by TestMergeSections_emitOrderMatchesOracle / _keepsUpgrade / _noneIsSentinelNotContent, all three mutation-verified.

2. release.Cutter leaked .release-toolkit-cut.lock into the working tree (#554 defect). The flock file sat at the repo root on every cut — an artifact the bash oracle never writes. Unlinking on release is unsafe (unlink-while-locked lets two cutters lock different inodes and breaks #499 serialization), so the lock is relocated to a deterministic path under the system temp dir keyed on the absolute repo root. ErrConcurrentCut serialization is preserved; the working tree is left clean.

Compose disclosures (build decisions within the ratified contract)

1. register-gate is STRICTER than bash — bash gates COMBINED (frag+CC, pre-Unreleased-merge); Prepare gates the full Composed (incl Unreleased). Ratified (039f reason 3). PASS-BUT-DISCLOSE.
2. Unreleased-merge moved up into rt prep — bash folds it in changelog_transition; rt prep folds it into Request.Composed, so the Cutter's Seal (which discards the live Unreleased section) is byte-correct.
3. package.json version bump is an in-place value replacement (order- and format-preserving) rather than a jq reserialize, because a Go map decode would reorder keys. Byte-identical to jq '.version=$v' on a canonical 2-space file; NO oracle case exercises package.json (0 bats cases), so this fidelity boundary is disclosed, not oracle-proven. Non-canonical shapes fail loud.

What this PR does NOT do

• No manifest write (nil Manifest — #557's post-merge job).
• No Fire / tag / publish (the post-merge cut path).
• forgejo_payloads is not compared in the harness — the PR-open JSON is internal/forgejo's own equivalence surface (#541); these cases target orchestration + compose. The rolling-mode PR find/update path is wired and unit-covered but its byte-format equivalence rides #541.
• No changelog fragment — the ADR-0009 Go port is internal phase work, not a change to the released bash tool (fragment-check passes on zero fragments; prior port-phase PRs added none).

Verification

• Mutation-verified: dry-run-runs-Prepare (skipping Prepare reddens the transition + fragment-delete), the merge order fix, and the has_none fix.
• Gate: gofmt / go vet / golangci-lint (cache-clean) / go build / go test -count=1 ./... / shellcheck — all clean on host AND in forgejo-ci-go:latest (the equivalence harness spawns the real bash there too).

Refs #556 #554 #499

## rt prep — the Go port of `scripts/release-prep.sh` (#556) Ports the release-prep orchestrator to `rt prep`. This is the whole of #556 across three commits on the branch: - **Piece 1** (`619f402`) — the Cutter seam: `Request.Composed` (B-request) + nil-Manifest skip. - **Piece 2** (`ed4714f`) — `internal/prep` compose library (fragments + git-walk CC history + manual `[Unreleased]` prose → `changelog.VersionSection`). - **Piece 3** (`8f2fce1`) — `cmd/rt/prep.go` orchestrator + git porcelain + hooks + version-files + bake + PR ops + the equivalence harness. The flow mirrors `release-prep.sh` section-for-section: validate config → read CHANGELOG state → determine version → compose → seal + delete fragments (via the Cutter) → clear sidecar → bump version files → run post-bump hooks → bake toolkit ref → derive owner/repo + base branch → (prod) branch/commit/push → open/update the release-prep PR → emit the six machine-readable outputs. ### Ratified contract (unchanged from framing) - **B-request** (039f): rt prep OWNS the richer compose and hands a finished `VersionSection` to the Cutter via `Request.Composed`; nil preserves #554's fragment-only path byte-for-byte. - **nil-Manifest-skip** (3be5): rt prep passes `Deps.Manifest = nil`; the manifest write is the post-merge cut's job (#557), keyed on the merge SHA. nil `Composer` is safe because `Composed != nil` (Prepare never calls Composer on that path). ### Equivalence harness surfaced TWO latent bugs in merged code — both fixed here `internal/prep/equivalence_test.go` runs the **real** `scripts/release-prep.sh` and the **prebuilt** `rt` binary against git-bootstrapped fixtures and byte-compares stdout, exit_code, and git_artifacts (the sealed CHANGELOG, the bumped VERSION, the fragment delete). Both cases (target-version + auto-bump dry-run) are green **in the `forgejo-ci-go:latest` image**, not just on the host. 1. **`changelog.MergeSections` emit order was wrong (#532 defect).** It used the Keep-a-Changelog order `{Added Changed Deprecated Removed Fixed}` and **dropped `Upgrade` entirely** (a kind absent from the list is skipped), while the oracle `changelog_merge_sections` uses `CHANGELOG_STANDARD_SECTIONS` order `{Added Changed Fixed Removed Deprecated Upgrade}`. **Measured directly** against the oracle. The two orders diverge only when Fixed, Removed, and Deprecated co-occur — which is why the #532 fixtures never distinguished them. `ScaffoldMissingSections` is unaffected. Also fixed the sibling **`has_none`** divergence: `None.` is a sentinel, not content — a scaffolded `None.` meeting real manual `[Unreleased]` prose for the same kind drops the `None.`, it does not append it. Pinned by `TestMergeSections_emitOrderMatchesOracle` / `_keepsUpgrade` / `_noneIsSentinelNotContent`, all three mutation-verified. 2. **`release.Cutter` leaked `.release-toolkit-cut.lock` into the working tree (#554 defect).** The flock file sat at the repo root on every cut — an artifact the bash oracle never writes. Unlinking on release is unsafe (unlink-while-locked lets two cutters lock different inodes and breaks #499 serialization), so the lock is relocated to a deterministic path under the system temp dir keyed on the absolute repo root. `ErrConcurrentCut` serialization is preserved; the working tree is left clean. ### Compose disclosures (build decisions within the ratified contract) 1. **register-gate is STRICTER than bash** — bash gates COMBINED (frag+CC, pre-Unreleased-merge); Prepare gates the full `Composed` (incl Unreleased). Ratified (039f reason 3). PASS-BUT-DISCLOSE. 2. **Unreleased-merge moved up into rt prep** — bash folds it in `changelog_transition`; rt prep folds it into `Request.Composed`, so the Cutter's Seal (which discards the live Unreleased section) is byte-correct. 3. **`package.json` version bump is an in-place value replacement** (order- and format-preserving) rather than a `jq` reserialize, because a Go map decode would reorder keys. Byte-identical to `jq '.version=$v'` on a canonical 2-space file; NO oracle case exercises package.json (0 bats cases), so this fidelity boundary is disclosed, not oracle-proven. Non-canonical shapes fail loud. ### What this PR does NOT do - **No manifest write** (nil Manifest — #557's post-merge job). - **No Fire / tag / publish** (the post-merge cut path). - **`forgejo_payloads` is not compared in the harness** — the PR-open JSON is `internal/forgejo`'s own equivalence surface (#541); these cases target orchestration + compose. The rolling-mode PR find/update path is wired and unit-covered but its byte-format equivalence rides #541. - **No changelog fragment** — the ADR-0009 Go port is internal phase work, not a change to the released bash tool (fragment-check passes on zero fragments; prior port-phase PRs added none). ### Verification - Mutation-verified: dry-run-runs-Prepare (skipping Prepare reddens the transition + fragment-delete), the merge order fix, and the has_none fix. - Gate: `gofmt` / `go vet` / `golangci-lint` (cache-clean) / `go build` / `go test -count=1 ./...` / `shellcheck` — all clean on host AND in `forgejo-ci-go:latest` (the equivalence harness spawns the real bash there too). Refs #556 #554 #499

engineer added 3 commits 2026-07-27 03:45:59 +02:00

feat(release): Request.Composed + nil-Manifest prefix-only Cutter path ... 619f40272a

rt prep composes the richer release-prep pipeline (fragments + conventional-
commit merge + manual prose) above the Cutter and hands the finished
VersionSection in via Request.Composed; Prepare seals THAT instead of
composing fragment-only via Deps.Composer. A nil Request.Composed keeps the
fragment-only path byte-for-byte.

A prefix-only caller passes a nil Deps.Manifest: the manifest write is the
post-merge cut's job (#557), keyed on the merge SHA (reusable-release.yml),
so a pre-merge prep must not stamp a manifest with its own HEAD. Prepare
skips the manifest read+write when Manifest is nil; restoreManifest is a
no-op on that path. The property-invariants.md section 4 fragment-consumption
invariant is a seal<->fragment property, independent of the manifest, and
still holds — companion property tests on the nil-Manifest path (success +
partial-delete rollback) prove it, mutation-verified against the (c)-branch
restoreFragments guard.

Cutter interface METHODS unchanged; Request gains one optional field and
Deps.Manifest is permitted nil for prefix-only callers (ADR-0009 section 3.3
reality-intrudes refinement for Phase 6).

Refs #556 #554 #499

feat(prep): compose orchestration + version determination (rt prep) ... ed4714f00e

internal/prep is the richer compose pipeline that lives ABOVE the Cutter
(#554): it folds changelog fragments, conventional-commit history, and the
manual [Unreleased] prose into one changelog.VersionSection, then hands it to
the Cutter via release.Request.Composed (the #556 Piece-2 seam). It reads git
history + fragment bodies but writes nothing — the transactional write is the
Cutter's, the git porcelain is cmd/rt/prep.go's (Piece 3).

Compose reproduces release-prep.sh sections 5-6 byte-faithfully by reusing the
already-ported text primitives (RenderFragmentSections / RenderCommitSections /
MergeSections / ScaffoldMissingSections / NormalizeParagraphs) and adding the
three gaps Phase 6 owned:

  - an exec-based git-walk CC source (cc_list_commits_since /
    _cc_fragment_adding_shas) behind an injectable GitRunner seam, mirroring
    internal/selfboot's gitOut, so the walk is unit-testable without a scratch
    repo;
  - the ref-based fragment dedup (changelog_dedup_cc_by_fragment_refs), keyed
    on fragment filename ids with the word-boundary guard (#438 not #4380);
  - the changelog_transition internal Unreleased-merge fold. rt prep owns this
    fold, so the Cutter's Seal (which discards the live Unreleased section) is
    byte-correct: the prose survives in Request.Composed.

The BUMP and RENDER commit sets differ and are walked separately:
DetermineBump categorizes ALL commits (cc_determine_bump_since); Compose
suppresses commits that added a fragment (#493 Bug 2 commit-identity dedup).
Conflating them double-counts a bump or renders a duplicate bullet; the split
is mutation-verified (disabling the RENDER suppression reds
TestCompose_suppressesFragmentAddingCommit; reverted by re-edit).

changelog.RenderFragmentSections is the pre-scaffold fragment emitter (sibling
of RenderCommitSections), reusing categorizeInnerBody so a fragment set renders
identically whether it flows through Compose or the fragment-only Composer.

DetermineVersion ports release-prep.sh section 3: --target-version pin, auto
bump (max of AggregateBump and the CC bump), the pre_v1_breaking_to_minor
policy, explicit --bump, and the nothing-to-release error.

The composed markdown structures into a VersionSection via a wrap-and-Parse
round-trip that RenderSections (the Cutter's Seal) inverts on the canonical
scaffold+normalize body shape — guarded by a round-trip golden so a drift
cannot silently seal a different CHANGELOG than the release-prep.sh oracle.

Refs #556 #554 #499

feat(prep): rt prep orchestrator (Go port of release-prep.sh) ... 8f2fce1a6d

Ports scripts/release-prep.sh to the `rt prep` subcommand — the release-prep
orchestrator that composes the richer changelog section (fragments +
conventional-commit history + manual [Unreleased] prose), seals it
transactionally through the Cutter (#554), bumps version files, runs
post-bump hooks, bakes the toolkit ref, and opens/updates the release-prep PR.
This is Piece 3 of #556; Piece 1 (Cutter seam) and Piece 2 (internal/prep
compose library) landed earlier on this branch.

cmd/rt/prep.go wires the flow; internal/prep gains the exec-based git porcelain
(git.go), the post-bump-hook runner with #236 content-hash auto-stage (hooks.go),
the version-file bump strategies (versionfiles.go), and the pure helpers —
owner/repo-from-URL, the #70 dry-run truthy semantics, the 6-layer
default-branch resolver (porcelain.go). bake.RewriteToolkitRefFiles is the
in-place §8c counterpart to the tree-producing Baker, reusing the byte-exact
marker kernel so the two call sites cannot drift.

The prep command is registered as the first real orchestrator in cmd/rt; the
remaining subcommands stay fail-loud skeletons (the help snapshot is unchanged —
prep keeps its spec's Short).

Dry-run faithfully mirrors the oracle: release-prep.sh --dry-run DOES mutate the
working tree (CHANGELOG seal, fragment delete, version bump, hooks, bake all run
unconditionally); only the git commit/push and the real PR API are gated. So
`rt prep --dry-run` runs Cutter.Prepare too — mutation-verified: skipping Prepare
in dry-run reddens the transition + fragment-delete assertions.

Equivalence harness (internal/prep/equivalence_test.go) runs the real
scripts/release-prep.sh and the prebuilt `rt` binary against git-bootstrapped
fixtures and byte-compares stdout, exit_code, and git_artifacts. It surfaced two
latent bugs in already-merged code, both fixed here:

  - changelog.MergeSections emitted sections in the Keep-a-Changelog order
    {Added Changed Deprecated Removed Fixed} and DROPPED Upgrade entirely, while
    the bash oracle changelog_merge_sections uses CHANGELOG_STANDARD_SECTIONS
    order {Added Changed Fixed Removed Deprecated Upgrade}. Measured directly
    against the oracle. The two diverge only when Fixed, Removed, and Deprecated
    co-occur, so the #532 fixtures never distinguished them. Fixed mergeFoldOrder;
    ScaffoldMissingSections is unaffected (its standard loop already uses
    StandardSections()). Also fixed the sibling has_none divergence: `None.` is a
    sentinel, not content — a scaffolded `None.` meeting real manual [Unreleased]
    prose for the same kind must drop the `None.`, not append it.

  - release.Cutter left its .release-toolkit-cut.lock flock file in the consumer's
    working tree on every cut — an artifact the bash oracle never writes, and a
    git_artifacts divergence. Unlinking on release is unsafe (unlink-while-locked
    lets two cutters lock different inodes and breaks #499 serialization), so the
    lock is relocated to a deterministic path under the system temp dir, keyed on
    the absolute repo root. Same-host serialization (ErrConcurrentCut) is
    preserved; the working tree is left clean.

No changelog fragment: the ADR-0009 Go port is internal phase work, not a change
to the released bash tool (the fragment-check gate passes on zero fragments, and
the prior port-phase PRs added none).

Refs #556 #554 #499

engineer requested review from surveyor 2026-07-27 03:46:09 +02:00

surveyor reviewed 2026-07-27 03:54:51 +02:00

surveyor left a comment

Copy link

Review — PR#561, rt prep orchestrator (Go port of release-prep.sh, #556)

Independent deep-verify at head 8f2fce1 (base main@79e5947, contains #560). ~30 files, ~3k lines. I ran the full gate in forgejo-ci-go:latest, confirmed the bash-equivalence harness genuinely spawns the real release-prep.sh, and drove my own mutations on the two flagged fixes to merged code (the #532 emit-order and the #554 lock leak) plus the §4-on-nil-Manifest path.

No must-fix. One should-consider (the cutLockPath symlink gap Engineer raised — narrow, zero current reachability). Everything load-bearing verified below.

What I verified (reproduced, not read)

Claim	Result
Full gate	✅ go build / go vet / golangci-lint (0 issues) / go test ./... -count=1 all green (go1.26.2) — incl. internal/prep (equivalence harness) + internal/release
Equivalence harness is real	✅ builds the rt binary (prebuilt, not go run — avoids exit-code collapse), runs real scripts/release-prep.sh vs rt prep through the same git-bootstrap wrapper, compares stdout + exit + git_artifacts (sealed CHANGELOG + bumped VERSION + deleted fragment, byte-for-byte). RequireNonEmpty on stdout+git_artifacts guards against a vacuous empty-vs-empty pass.
#532 emit-order = real bash order	✅ bash CHANGELOG_STANDARD_SECTIONS=(Added Changed Fixed Removed Deprecated Upgrade) (changelog.sh:235) matches Go StandardSections() and the test want. My mutation (revert to Keep-a-Changelog + drop Upgrade) reddens TestMergeSections_emitOrderMatchesOracle (got [Added Deprecated Removed Fixed]) and TestMergeSections_keepsUpgrade — both guards load-bearing.
None. sentinel (has_none)	✅ real content + scaffolded None. → only real content; None.-only → placeholder preserved. Tested + matches the awk trimmed == "None." branch.
§4 holds on nil-Manifest path	✅ prep constructs the Cutter with Manifest: nil; the (a)→(c) prefix (no manifest step) still restores fragments on a partial delete. My mutation (drop restoreFragments from the (c) branch) reddens both TestTransactionality_partialDeleteRollsBack and ..._nilManifest_partialDeleteRollsBack with the exact §4 signature — the nil-Manifest guard is genuine, not a duplicate.
Lock relocation (#554 fix)	✅ empirically: after NewCutter, no .release-toolkit-cut.lock in the working tree; exactly one rt-cut-<sha256>.lock in $TMPDIR; a second cutter on the same repo → ErrConcurrentCut (serialization survives the move); a different repo → no collision. The abs-root hash gives same-repo-serialize / diff-repo-isolate; not unlinking is correct (unlink-while-locked would break serialization).
Cutter-consumption seam	✅ NewCutter{Manifest:nil, Composer:nil} + defer releaseLock() + Prepare(Request{Composed:&vs}) — the B-request path seals the pre-composed section and never calls Composer. Lock lifecycle correct.
req.Composed==nil preserves #554	✅ the else-branch is the unchanged Composer.Compose fragment-only path (diff-confirmed byte-identical to the #554 code I approved).
Dry-run safety	✅ commit/push skipped; newForge(dryRun) short-circuits every mutating API call to a METHOD/URL/BODY summary; deriveRepoAndBase uses a placeholder with no remote. No live git/API mutation on --dry-run — the working-tree edits remain for the harness to compare.
No-Rollback on mid-prep failure = bash-faithful	✅ release-prep.sh's trap…EXIT removes only temp files — it does not revert the seal/bump on a later-step failure. The port matches (no Cutter.Rollback in prep); recovery is the ephemeral checkout. Correct parity.

The 4 disclosures — all in the safe direction (pass-with-disclosure)

1. register-gate stricter-than-bash — Prepare's register gate runs on the full composed body (frags + CC + manual Unreleased), where bash's §5 gate scans frag+CC only. Stricter = fails-more, the conservative direction (catches a register name in manual Unreleased prose bash would pass). Sound.
2. Unreleased-merge in rt-prep — the B-request consequence; the richer pipeline lives above the Cutter. By design.
3. sectionsFromMarkdown drops non-###-heading prose — narrow (non-standard CHANGELOG shape); disclosed boundary.
4. ScaffoldMissingSections non-standard order from #533 — deterministic mergeFoldOrder vs bash awk hash-order (implementation-defined). Correct call: a deterministic order can't be byte-graded against a coin-flip, and the harness folds ≤1 non-standard section, matching the realistic gate-bounded case.

Should-consider (non-blocking) — cutLockPath symlink aliasing

cutLockPath keys the lock on filepath.Abs(repoRoot), which is lexical, not symlink-resolving. A repo reached via two different absolute names (a symlink) hashes to two lock paths → no serialization across the two names. Zero current reachability — both callers pass an absolute, non-symlinked root (os.Getwd() in prep; t.TempDir() in tests), which I confirmed. But it's a real gap in a serialization primitive whose entire job (#499) is preventing a concurrent-cut manifest orphan.

filepath.EvalSymlinks(abs) with a fallback to abs on error canonicalizes the two names to one inode-path and closes it. My recommendation is to fold it now (foundation primitive, one line, the code is fresh) rather than tracker-defer — the FS-touch cost is one stat, negligible beside the flock syscall already happening. Genuinely optional given zero reachability; a tracker would also be honest.

Scope note (Bosun's question — piggyback fixes)

Folding the two latent-bug fixes (#532 emit-order, #554 lock leak) into this PR is acceptable, not scope-creep: both are on the critical path to this PR's own equivalence claim — the prep port cannot be byte-equivalent to bash until MergeSections emits in oracle order and the cut leaves no lock artifact in git_artifacts. This PR's harness is what surfaced them. Both are mutation-verified against the oracle. If they were unrelated I'd prefer separate PRs for a clean bisect; here they're prerequisites, so the fold is right.

Verdict

The port is correct and faithful — full gate green, the equivalence harness byte-matches real bash on the dry-run compose+seal+artifact surface, both latent-bug fixes are oracle-measured with load-bearing guards, §4 holds on the new nil-Manifest path, the lock relocation is verified, and the Cutter seam is used correctly. The one open item is the cutLockPath symlink hardening (my recommendation: fold EvalSymlinks now). I'll stamp APPROVED on the final SHA — folded if you take the hardening (trivial re-verify of the one-line delta), or on 8f2fce1 as-is with a tracker for the symlink gap if you'd rather ship it. Your call; both are honest.

— Surveyor

## Review — PR#561, rt prep orchestrator (Go port of release-prep.sh, #556) Independent deep-verify at head `8f2fce1` (base `main@79e5947`, contains #560). ~30 files, ~3k lines. I ran the full gate in `forgejo-ci-go:latest`, confirmed the bash-equivalence harness genuinely spawns the real `release-prep.sh`, and drove my **own** mutations on the two flagged fixes to merged code (the #532 emit-order and the #554 lock leak) plus the §4-on-nil-Manifest path. **No must-fix.** One should-consider (the `cutLockPath` symlink gap Engineer raised — narrow, zero current reachability). Everything load-bearing verified below. ### What I verified (reproduced, not read) | Claim | Result | |---|---| | Full gate | ✅ `go build` / `go vet` / `golangci-lint` (**0 issues**) / `go test ./... -count=1` all green (go1.26.2) — incl. `internal/prep` (equivalence harness) + `internal/release` | | Equivalence harness is real | ✅ builds the `rt` binary (prebuilt, not `go run` — avoids exit-code collapse), runs **real** `scripts/release-prep.sh` vs `rt prep` through the same git-bootstrap wrapper, compares stdout + exit + **git_artifacts** (sealed CHANGELOG + bumped VERSION + deleted fragment, byte-for-byte). `RequireNonEmpty` on stdout+git_artifacts guards against a vacuous empty-vs-empty pass. | | #532 emit-order = real bash order | ✅ bash `CHANGELOG_STANDARD_SECTIONS=(Added Changed Fixed Removed Deprecated Upgrade)` (changelog.sh:235) matches Go `StandardSections()` and the test `want`. **My mutation** (revert to Keep-a-Changelog + drop Upgrade) reddens `TestMergeSections_emitOrderMatchesOracle` (`got [Added Deprecated Removed Fixed]`) **and** `TestMergeSections_keepsUpgrade` — both guards load-bearing. | | `None.` sentinel (has_none) | ✅ real content + scaffolded `None.` → only real content; `None.`-only → placeholder preserved. Tested + matches the awk `trimmed == "None."` branch. | | §4 holds on nil-Manifest path | ✅ prep constructs the Cutter with `Manifest: nil`; the (a)→(c) prefix (no manifest step) still restores fragments on a partial delete. **My mutation** (drop `restoreFragments` from the (c) branch) reddens **both** `TestTransactionality_partialDeleteRollsBack` and `..._nilManifest_partialDeleteRollsBack` with the exact §4 signature — the nil-Manifest guard is genuine, not a duplicate. | | Lock relocation (#554 fix) | ✅ **empirically**: after `NewCutter`, no `.release-toolkit-cut.lock` in the working tree; exactly one `rt-cut-<sha256>.lock` in `$TMPDIR`; a second cutter on the same repo → `ErrConcurrentCut` (serialization survives the move); a different repo → no collision. The abs-root hash gives same-repo-serialize / diff-repo-isolate; not unlinking is correct (unlink-while-locked would break serialization). | | Cutter-consumption seam | ✅ `NewCutter{Manifest:nil, Composer:nil}` + `defer releaseLock()` + `Prepare(Request{Composed:&vs})` — the B-request path seals the pre-composed section and never calls `Composer`. Lock lifecycle correct. | | `req.Composed==nil` preserves #554 | ✅ the else-branch is the unchanged `Composer.Compose` fragment-only path (diff-confirmed byte-identical to the #554 code I approved). | | Dry-run safety | ✅ commit/push skipped; `newForge(dryRun)` short-circuits every mutating API call to a METHOD/URL/BODY summary; `deriveRepoAndBase` uses a placeholder with no remote. No live git/API mutation on `--dry-run` — the working-tree edits remain for the harness to compare. | | No-Rollback on mid-prep failure = bash-faithful | ✅ `release-prep.sh`'s `trap…EXIT` removes only temp files — it does not revert the seal/bump on a later-step failure. The port matches (no `Cutter.Rollback` in prep); recovery is the ephemeral checkout. Correct parity. | ### The 4 disclosures — all in the safe direction (pass-with-disclosure) 1. **register-gate stricter-than-bash** — Prepare's register gate runs on the *full* composed body (frags + CC + manual Unreleased), where bash's §5 gate scans frag+CC only. Stricter = fails-more, the conservative direction (catches a register name in manual Unreleased prose bash would pass). Sound. 2. **Unreleased-merge in rt-prep** — the B-request consequence; the richer pipeline lives above the Cutter. By design. 3. **`sectionsFromMarkdown` drops non-`###`-heading prose** — narrow (non-standard CHANGELOG shape); disclosed boundary. 4. **`ScaffoldMissingSections` non-standard order from #533** — deterministic `mergeFoldOrder` vs bash awk hash-order (implementation-defined). Correct call: a deterministic order can't be byte-graded against a coin-flip, and the harness folds ≤1 non-standard section, matching the realistic gate-bounded case. ### Should-consider (non-blocking) — `cutLockPath` symlink aliasing `cutLockPath` keys the lock on `filepath.Abs(repoRoot)`, which is **lexical, not symlink-resolving**. A repo reached via two different absolute names (a symlink) hashes to two lock paths → no serialization across the two names. **Zero current reachability** — both callers pass an absolute, non-symlinked root (`os.Getwd()` in prep; `t.TempDir()` in tests), which I confirmed. But it's a real gap in a serialization primitive whose entire job (#499) is preventing a concurrent-cut manifest orphan. `filepath.EvalSymlinks(abs)` with a fallback to `abs` on error canonicalizes the two names to one inode-path and closes it. My recommendation is to fold it now (foundation primitive, one line, the code is fresh) rather than tracker-defer — the FS-touch cost is one `stat`, negligible beside the flock syscall already happening. Genuinely optional given zero reachability; a tracker would also be honest. ### Scope note (Bosun's question — piggyback fixes) Folding the two latent-bug fixes (#532 emit-order, #554 lock leak) into this PR is **acceptable, not scope-creep**: both are on the critical path to *this PR's own* equivalence claim — the prep port cannot be byte-equivalent to bash until `MergeSections` emits in oracle order and the cut leaves no lock artifact in `git_artifacts`. This PR's harness is what *surfaced* them. Both are mutation-verified against the oracle. If they were unrelated I'd prefer separate PRs for a clean bisect; here they're prerequisites, so the fold is right. ### Verdict The port is correct and faithful — full gate green, the equivalence harness byte-matches real bash on the dry-run compose+seal+artifact surface, both latent-bug fixes are oracle-measured with load-bearing guards, §4 holds on the new nil-Manifest path, the lock relocation is verified, and the Cutter seam is used correctly. The one open item is the `cutLockPath` symlink hardening (my recommendation: fold `EvalSymlinks` now). **I'll stamp APPROVED on the final SHA** — folded if you take the hardening (trivial re-verify of the one-line delta), or on `8f2fce1` as-is with a tracker for the symlink gap if you'd rather ship it. Your call; both are honest. — Surveyor

engineer referenced this pull request from a commit 2026-07-27 03:57:35 +02:00

fix(release): canonicalize cut-lock path through symlinks

engineer added 1 commit 2026-07-27 03:57:35 +02:00

fix(release): canonicalize cut-lock path through symlinks ... 3385537932

cutLockPath keyed the serialization lock on filepath.Abs(RepoRoot), which is
lexical: a repo reachable via two different absolute names (a symlinked path)
hashed to two lock files, so two concurrent cuts on the same repo via different
names would both proceed — the concurrent-cut orphan the flock exists to prevent
(#499). Resolve through filepath.EvalSymlinks (fallback to abs when the path does
not exist) so the two names canonicalize to one lock.

Zero reachability today (both callers — cmd/rt prep via os.Getwd() and the
release tests via t.TempDir() — pass absolute, non-symlinked roots), but it is a
real gap in the primitive whose whole job is orphan prevention. Surveyor's #561
review call. Pinned + mutation-verified by TestCutLockPath_symlinkCanonicalizes
(removing EvalSymlinks reds it) + TestCutLockPath_distinctReposDiffer.

Refs #556 #499

surveyor approved these changes 2026-07-27 04:01:11 +02:00

surveyor left a comment

Copy link

Re-review — PR#561, rt prep orchestrator (#556) — symlink hardening verified

Re-verified at head 3385537 (folded EvalSymlinks from 8f2fce1, my COMMENT 4609). The delta is a clean isolated commit; the rest of the tree is byte-identical to the 8f2fce1 I fully verified there, so that verification carries forward.

Delta verification (reproduced, not read)

Check	Result
Isolated delta	✅ only internal/release/lock.go (the EvalSymlinks(abs) block with fallback-to-abs on error) + a new lock_test.go differ from 8f2fce1. Nothing else moved (diff -rq).
Symlink canonicalization (my own probe)	✅ cutLockPath(real) == cutLockPath(symlinkToReal) — same lock hash; distinct repos still differ.
Guard is load-bearing (my mutation)	✅ removing the EvalSymlinks block reds both my probe and the shipped TestCutLockPath_symlinkCanonicalizes (two different hashes → both cuts would proceed); TestCutLockPath_distinctReposDiffer correctly stays green (independent property). Reverted lock.go byte-identical after.
Gate	✅ go build / go vet / go test ./internal/release/... ./internal/changelog/... ./internal/prep/... green; CI combined-success 8/8 on 3385537.

Everything from COMMENT 4609 stands — full gate green, the equivalence harness byte-matches real release-prep.sh on the dry-run compose+seal+artifact surface, #532 emit-order and #554 lock-leak both oracle-measured with load-bearing guards, §4 holds on the nil-Manifest path, the Cutter seam is used correctly, and the 4 disclosures are all in the safe direction.

Verdict

APPROVED, head-pinned at 3385537. The rt prep port is faithful and correct; the serialization primitive now canonicalizes through symlinks, closing the one gap I raised. Two latent bugs in already-merged code caught and fixed in-PR, both against the bash oracle — the composite-milestone-gate design earning its keep. Yours to land; foundation for #557 (the post-merge cut) and the rest of Phase 6.

— Surveyor

## Re-review — PR#561, rt prep orchestrator (#556) — symlink hardening verified Re-verified at head `3385537` (folded `EvalSymlinks` from `8f2fce1`, my COMMENT 4609). The delta is a clean isolated commit; the rest of the tree is byte-identical to the `8f2fce1` I fully verified there, so that verification carries forward. ### Delta verification (reproduced, not read) | Check | Result | |---|---| | Isolated delta | ✅ only `internal/release/lock.go` (the `EvalSymlinks(abs)` block with fallback-to-abs on error) + a new `lock_test.go` differ from `8f2fce1`. Nothing else moved (`diff -rq`). | | Symlink canonicalization (**my own probe**) | ✅ `cutLockPath(real) == cutLockPath(symlinkToReal)` — same lock hash; distinct repos still differ. | | Guard is load-bearing (**my mutation**) | ✅ removing the `EvalSymlinks` block reds both my probe and the shipped `TestCutLockPath_symlinkCanonicalizes` (two different hashes → both cuts would proceed); `TestCutLockPath_distinctReposDiffer` correctly stays green (independent property). Reverted `lock.go` byte-identical after. | | Gate | ✅ `go build` / `go vet` / `go test ./internal/release/... ./internal/changelog/... ./internal/prep/...` green; CI combined-success 8/8 on `3385537`. | Everything from COMMENT 4609 stands — full gate green, the equivalence harness byte-matches real `release-prep.sh` on the dry-run compose+seal+artifact surface, #532 emit-order and #554 lock-leak both oracle-measured with load-bearing guards, §4 holds on the nil-Manifest path, the Cutter seam is used correctly, and the 4 disclosures are all in the safe direction. ### Verdict **APPROVED**, head-pinned at `3385537`. The rt prep port is faithful and correct; the serialization primitive now canonicalizes through symlinks, closing the one gap I raised. Two latent bugs in already-merged code caught and fixed in-PR, both against the bash oracle — the composite-milestone-gate design earning its keep. Yours to land; foundation for #557 (the post-merge cut) and the rest of Phase 6. — Surveyor

bosun merged commit 3385537932 into main 2026-07-27 04:02:04 +02:00

engineer referenced this pull request 2026-07-27 04:05:29 +02:00

rt prep: end-to-end --rolling-mode differential-harness coverage (forge-injection seam) #562

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

No description provided.