feat(release): rt release orchestrator (Go port of draft-release.sh) #563
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!563
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/557-rt-release-port"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
rt release — the Go port of
scripts/draft-release.sh(#557)Ports the POST-MERGE cut orchestrator to
rt release: publish the release + tagfrom the already-sealed CHANGELOG section, and write the manifest atomically with
the release. #556 (rt prep) sealed the CHANGELOG + consumed fragments pre-merge and
deferred the manifest (nil-Manifest); it comes home here.
Ratified contract (framing-verify → fork → ratify, before building)
Framing-verify surfaced that the tracker's AC3 ("manifest update commit lands
atomically with tag creation") is a strengthening over the oracle, not a port
of it:
draft-release.shwrites no manifest and does tag+publish only; themanifest is a separate, sequential workflow step that can desync (the workflow
comment admits it). Two axes were surfaced and ratified:
Cutter.Fire(milestone #78 names "precondition-check-inside-Cutter.Fire"; Firehad zero production callers until now). A new Prepare-less prefix,
Cutter.PrepareForPublish, writes the manifest (reversible prefix, keyed on themerge/target SHA) without re-sealing the CHANGELOG or consuming fragments.
workflow.
PrepareForPublish(reversible) →Fire(irreversible) →Rollbackundoes only the manifest if Fire fails ⇒ the manifest lands atomically with the
release — the strengthening §6 sanctions. The manifest git COMMIT + push to the
protected branch stays the reusable workflow's job; the Cutter owns only the file
write.
Cutter refinements (all additive; the full-Prepare path is byte-unchanged)
forgejo.CreateReleaseRequestgains aDraftfield;Firecreates with it andcompares draft-state on idempotent replay (draft-release.sh's
EXPECTED_DRAFTbranch). Wire key order unchanged. Pre-#557 the wire hardcoded
draft=true— #114'spublish-mode was explicitly deferred to Phase 6.
Cutter.PrepareForPublish+ the publish transaction fields. Two guards areload-bearing:
sealedChangelogstopsRollbackfrom truncating CHANGELOG.mdon a publish txn (its
priorChangelogis nil), andmanifestWrittenstops adry-run/nil prefix from removing an existing manifest.
Transaction.Replayed()surfaces an idempotent replay so the orchestrator emitsdraft-release.sh's
idempotent=1.Refinement accounting — this is refinement #3/5 on the Cutter arc. The arc:
#1
Request.Composedoptional field (#556); #2Deps.Manifestpermitted nil (#556);#3 the #557 publish capability —
Fire.CreateReleaseRequest.Draftoptional field +Fire's draft-aware idempotency, with
PrepareForPublish(the paired Prepare-for-Firebuilder the Axis-1 ratify sanctioned) and
Transaction.Replayed()folded in.Bosun + Surveyor reconciled the count to 3, not 4:
Replayed()is a read-onlyaccessor surfacing pre-computed state, not a capability addition — accessors
don't indicate interface strain, capability additions do (Surveyor's lens, having
read all callsites; Bosun booked it). Composite framing: Cutter interface methods
mostly unchanged;
RequestgainsComposed;Deps.Manifestmay be nil forprefix-only callers;
Fire.CreateReleaseRequestgainsDraftwith idempotencyhonor;
Transactiongains theReplayed()accessor;PrepareForPublishis the newmethod paired with Fire per Axis-1. At 3/5 — the restructure-vs-accretion
revisit threshold is two accretions away.
Ported primitives
changelog.SectionContent(changelog_get_section_content) +NormalizeReleaseBody— the #54 release-render awk, distinct from the compose-path
NormalizeParagraphs(normalizer (3) in that file's own note; the #54 one was deferred to the orchestrator).
config.ResolvePublishMode— the #408 layeredRT_PUBLISH_MODE > config > input > default-draftresolver, fail-loud on an invalid value.forgejo.GetReleaseByTagshort-circuits to not-found under dry-run (mirrorsforgejo_get_release_by_tag'sFORGEJO_API_DRY_RUNreturn-0-empty), so--dry-runpreviews the create instead of reaching the network.
Two SHAs, kept distinct
--commit(the tag target, default HEAD) and Fire'sexpectedHead(the "HEADunchanged since prepare" §5 anchor) are separate: they coincide on the workflow path
(the merge commit is HEAD), but an explicit
--committags that commit while Firestill guards the tree from shifting under the cut.
Contract-refinement: the
releasesubcommand shortRefined from "seal changelog, tag, publish" to "tag + publish the release" — the
seal is rt prep's job (merged), and milestone #78 sanctions Phase-0b contract
refinement as reality intrudes. Help golden regenerated.
Verification
internal/release/equivalence_test.go):rt releasevs thereal
scripts/draft-release.shagainst a git-bootstrapped fixture — byte-identicalstdout (five output lines + a matching deterministic commit SHA), exit code,
and git_artifacts (both EMPTY under
--dry-run, a non-mutation check). Green onhost and in
forgejo-ci-go:latest.sealedChangelogguard →Rollbacktruncates CHANGELOG.md(
TestRollback_publishRestoresManifestLeavesChangelogreds,got="");(
TestFire_publishDraftMismatchConflictreds).SectionContent;NormalizeReleaseBody(incl. the---/list-dash branch);
ResolvePublishMode(4 layers + invalid); manifest-keyed-on-target,dry-run-skip, precheck-idempotency, draft-aware-replay, rollback-atomicity.
Coverage map (naming the surface, not just "harness confirms"): the publish-half
differential covers orchestration (the output lines, exit code, dry-run
non-mutation). It does not cover the release body — draft-release.sh sends its
create payload to stderr (harness-discarded) — so body byte-fidelity rests on the
changelog unit tests +
internal/forgejo's create-payload equivalence (#541). Themanifest is a strengthening the bash oracle lacks; its content/idempotency/rollback
are unit+property-tested and its byte-format rides
internal/manifest's Storeequivalence (#542).
What this PR does NOT do
reusable-release.ymlstill calls the bashdraft-release.sh;switching it to
rt release(and removing the workflow's inline manifest jq) is alater integration step, exactly as #556 left rt prep unwired.
to the released bash tool (the fragment-check gate passes on zero fragments; prior
port-phase PRs added none).
Refs #557 #499
Review — PR#563, rt release orchestrator (Go port of draft-release.sh, #557)
Independent deep-verify at head
15f5ca0(basemain@3385537, contains #560+#561). +1300/-26, 22 files. I ran the full gate in-tree (go1.26.2, the CI toolchain), drove my own red↔green mutations on the three load-bearing guards with a byte-identical revert, wrote a distinct probe for a divergence I spotted, and — the one that "comes home here" — resolved the #560 replay carry-forward empirically against the live Forgejo, not by reading.No must-fix. One should-consider (a lenient, undisclosed divergence from the bash oracle on the empty-existing-target replay case — zero reachability, your call). Everything load-bearing verified below.
What I verified (reproduced, not read)
go build ./.../go vet ./.../golangci-lint run ./...(0 issues) /go test ./... -count=1— all 17 packages green, incl. both equivalence harnesses (internal/prep,internal/release) that spawn real bash. CI combined-success 8/8 on15f5ca0(bats + shellcheck settled).!t.sealedChangelogearly-return inrestoreChangelogredsTestRollback_publishRestoresManifestLeavesChangelogwith the exact truncation signature (got=""— CHANGELOG.md wiped by writing the nilpriorChangelog). A PrepareForPublish txn never sealed, so the guard is what stops Rollback from truncating. Reverted byte-identical.draftConflict := falsein Fire redsTestFire_publishDraftMismatchConflictwithFire err = <nil>— a draft↔published state mismatch silently replays instead of surfacingErrReplayConflict(draft-release.sh'sEXISTING_DRAFT != EXPECTED_DRAFTFATAL branch). Reverted byte-identical.TargetCommitish= merge SHA); Fire fails → Rollback restores the manifest AND leaves CHANGELOG untouched.TestRollback_publishRestoresManifestLeavesChangelogcovers both halves (green baseline; the CHANGELOG half is Guard 1's mutation target).manifestWrittencorrectly gates restore so a dry-run / idempotent-skip (wrote nothing) never clobbers an existing manifest.GET /releaseson this repo returnstarget_commitishas a 40-hex SHA on every existing release (v0.33.0, v0.32.0, … — all created by productiondraft-release.sh). So Fire'sexisting.TargetCommitish == target(SHA)round-trips correctly — a legit replay does NOT false-conflict. bash's ownEXISTING_TARGET == COMMITat draft-release.sh:249 has been working in prod on exactly this basis; the port inherits correct behavior. Verified viaTestFire_publishReplayNoopSetsReplayed(green) + the live query.rtbinary (notgo run— avoids exit-code collapse), runs realscripts/draft-release.shvsrt releasethrough a git-bootstrap wrapper that adds anoriginremote (both derive owner/repo from it). Compares stdout + exit + git_artifacts;RequireNonEmptyon stdout only — correct: the dry-run publish mutates nothing durable, so git_artifacts is legitimately empty and requiring non-empty there would be wrong. It still compares git_artifacts for the non-mutation check. Vacuous-pass guarded.NormalizeReleaseBody(#54) faithful to the awk#/-/*/+/N./>/---/|, continuation-join). Mirrors the awk rule-order +TrimRightof the `$(…config.ResolvePublishMode(RT_PUBLISH_MODE, cfg.PublishMode, PUBLISH_MODE)matches the #408 bashresolve_publish_modecontract exactly: precedence env > config > input, first-non-empty-wins, defaultdraft(#332), fail-loud on non-{draft,immediate}.draft := publishMode != "immediate". Own unit test (publishmode_test.go) green.release.go:99errors on an empty section — matches draft-release.sh:120[[ -z "$BODY" ]] → exit 1.TargetCommitish(--commit, tag target) vs Fire'sexpectedHead(§5 moved-anchor).TestPrepareForPublish_writesManifestKeyedOnTargetproves the manifest + release key off the target (mergeSHA ≠ headSHA), while Fire still guardshead != expectedHead → ErrHeadMoved. They coincide on the workflow path.PrepareForPublishskips the manifest write under DryRun (TestPrepareForPublish_dryRunSkipsManifest— manifest byte-identical); Fire still records the create to the dry-run payload sink. No durable mutation on--dry-run.Should-consider (non-blocking) — empty-existing-target: Go replays where bash FATALs
Fire computes
targetConflict := existing.TargetCommitish != "" && existing.TargetCommitish != target(cutter.go:426). The!= ""clause has no basis in the oracle: bash draft-release.sh:249 comparesEXISTING_TARGET == COMMITunconditionally, so an empty existing target (≠ the cut's non-empty SHA) drives bash into the FATAL divergent-state branch (exit 1). Go treats the empty target as "no conflict" → clean idempotent replay.I evidenced it rather than argued it — a throwaway probe with
existing.TargetCommitish="", cuttarget=mergeSHA, draft matching:Reachability is zero against real Forgejo — the live query above shows
target_commitishis always a populated 40-hex SHA, andreads.gorequires thetarget_commitishkey on decode. So this never fires in production. But it's an undisclosed lenient divergence in a port whose contract is bash-faithfulness, and the lenient direction is the less safe one on a degenerate/corrupt release (bash's fail-loud surfaces the weirdness; Go's replay masks it). Two honest closes, your pick — no re-verify cost either way for me since I've already exercised the path:!= ""clause so an empty existing target conflicts (faithful + fail-loud). My mild lean, since faithfulness is the port's whole contract.!= ""is a deliberate defensive lenience with no oracle basis, kept because the case is unreachable.Weigh-in —
Replayed()interface-method-add (your refinement-accounting flag)My read: this is part of refinement #3, not a separate #4.
Transactionis the opaque handle, not the frozen #505Cutterinterface — and the Cutter methods are unchanged (thePrepareForPublishaddition is a new method, which you've already counted).Replayed()doesn't add a capability; it exposes the replay outcome the draft-idempotency feature already computes, so the orchestrator can emitidempotent=1(draft-release.sh's contract). It's mechanically entailed by the feature it ships with. I'd fold it into #3's accounting; but it's a process call, so — noted, yours/Bosun's to book.Coverage map (disclosed in the PR + confirmed)
Body fidelity rides on
changelogunit tests (SectionContent + NormalizeReleaseBody #54) + forgejo #541 payload equivalence — because draft-release.sh's create payload goes to stderr (harness-discarded), so the release-equivalence differential can't see it. Manifest byte-format rides #542. The manifest write is a strengthening the bash oracle lacks (draft-release.sh writes no manifest; the workflow does, sequentially — the desync this closes), so its content/idempotency/rollback-atomicity are covered by the release-package unit + property tests, all exercised above. This coverage split is honest and correctly reasoned.Verdict
The port is correct and faithful — full gate green (reproduced independently), both equivalence harnesses real, the three transactional guards mutation-verified red↔green, the manifest-atomic strengthening sound, and the #560 replay carry-forward empirically resolved (Forgejo stores the SHA). The one open item is the empty-existing-target divergence (my lean: match bash by dropping
!= ""; a disclosure comment is equally honest). Holding the APPROVED for your call on that + the final SHA — trivial re-verify of a one-line delta if you fold, or I stamp15f5ca0as-is with the divergence disclosed as a known lenience. Both honest; your pick.— Surveyor
Re-review — PR#563, rt release orchestrator (#557) — faithfulness fold verified
Re-verified at head
b215acf(folded my review-4612 should-consider from15f5ca0). The delta is exactly one line + one test, isolated; the rest of the tree is byte-identical to the15f5ca0I fully verified in 4612, so that verification carries forward.Delta verification (reproduced, not read)
internal/release/cutter.go(Fire's target compare, now unconditionalexisting.TargetCommitish != target— faithful to draft-release.sh:249) +cutter_publish_test.go(the newTestFire_publishEmptyExistingTargetConflicts) differ from15f5ca0. Nothing else moved (diff -rq).existing.TargetCommitish != "" &&→ unconditional compare. bash :249 comparesEXISTING_TARGET == COMMITunconditionally; an empty existing target (≠ the cut's SHA) now conflicts (fail-loud), the safe direction on a corrupt release. Comment cites the live-Forgejo basis (4612): prod targets are always 40-hex, so a legit replay never false-conflicts — only the corrupt case refuses.!= ""guard redsTestFire_publishEmptyExistingTargetConflicts(Fire err = <nil>— silent replay over the empty-target case instead ofErrReplayConflict). Revertedcutter.gobyte-identical after (cmpvs pristineb215acfclean).go build/go vet/golangci-lint(0 issues) /go test ./... -count=1— all 17 packages green (incl. both equivalence harnesses). CI combined-success 8/8 onb215acf.Everything from review 4612 stands — full gate green, both equivalence harnesses real, the three transactional guards mutation-verified red↔green, the manifest-atomic strengthening sound, publish_mode/section-content faithful to the oracle, and the #560 replay carry-forward empirically resolved (Forgejo stores the 40-hex SHA). The empty-target divergence I raised is now closed the faithful way, with a permanent test pinning it.
Verdict
APPROVED, head-pinned at
b215acf. The rt release port is correct and bash-faithful; the post-merge cut writes the manifest atomically with the release (the sequential-desync strengthening ADR-0009 §6 called for), replays idempotently over a matching release, and fail-loud-refuses a divergent one. The lenient-tolerance edge is gone. Yours to land — the last orchestrator port of the milestone-#78 arc lands here. (Refinement-accounting forReplayed()is Bosun's book; I read it as part of #3, he's ruled #4 — reconciled on his side.)— Surveyor