feat(rt-manifest-precheck): port manifest-precheck.sh → rt manifest-precheck #573
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!573
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/571-rt-manifest-precheck"
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?
What
Phase 6b validator port (ADR-0009 §3.3):
scripts/manifest-precheck.sh→rt manifest-precheck <manifest-path> <head-sha>. The cut path's manifest-write idempotency gate (release-toolkit#128/#134/#140): decide whether the manifest write/commit/push shouldskip(a prior, self-cancelled workflow run already landed the manifest commit) orproceed.Refs #571 · Refs ADR-0009 §3.3
Framing-verify correction (surfaced + ratified before build)
The tracker's primitives inventory (and the dispatch table) mapped this to "a thin wrapper over
manifest.CheckDesyncVerbose(#558)." That is wrong — zero overlap.CheckDesyncVerboseis the desync/drift checker inmanifest-check.sh.manifest-precheck.shis a standalone skip/proceed decision: compare a manifest'slast_released_shaagainst an arg-suppliedHEAD_SHA, remote-main-first (git show origin/<branch>:PATH) then the local file, fail-safe toproceedon any uncertainty. A genuine from-scratch port. NOT a Cutter consumer — the 3/5 Cutter-accretion count stays firm.Shape
internal/gates.ManifestPrecheck— the pure bivalent decision (skip/proceed, both exit 0). Sibling toComposeVerify/ChangelogBodyCheck/RegisterCheckStdin; takes resolved inputs (all IO is the caller's), so it unit-tests without a git remote or filesystem.cmd/rt manifest-precheck— the wrapper: resolves the remote manifest (MANIFEST_REMOTE_FIXTUREtest seam, elsegit fetch+git show origin/<branch>swallowing every git error to""), reads the local file, prints the decision.RT_DEFAULT_BRANCHhonored (Refs #379).Ratified forks (F1–F6, #571)
manifest_skip/manifest_proceedare stderr-only, byte-invisible on the stdout+exit oracle (all 5 Phase-6 orchestrator precedents + milestone §5 sanction).internal/gates(sibling shape) + thincmd/rtwrapper.skip/proceedboth exit 0; exit 1 only on a usage error.internal/verdictdoes not apply; the milestone's "trivalent-verdict at wire boundary" is a general Phase-6b shape, not a universal contract on every validator.git show); not over-extracted.json.Valid+ best-effort field extract), NOTmanifest.Store.Read(C2-strictDisallowUnknownFieldswould reject manifests bash tolerates). Same #558/#555 precedent.MANIFEST_REMOTE_FIXTUREseam and the decision needs no SHA-deterministic bootstrap, the byte-oracle covers the remote-first #140 cases too (see Verification).Verification
internal/gatesTestManifestPrecheck— the decision branch matrix incl. null / empty-string / non-string / non-object / missing-field fail-safe edges.cmd/rtTestManifestPrecheckEquivalence— prebuilt binary vs the realmanifest-precheck.shbyte-oracle over 7 scenarios:local-skip,local-proceed,absent-proceed,invalid-proceed, and the #140 remote-first casesremote-skip,remote-fixture-missing-local-skip,remote-invalid-local-skip. stdout+exit compared, stdout positive-controlled (RequireNonEmpty).cmd/rtTestManifestPrecheckUsage— the exit-1 usage path (harness always passes 2 args, so this is unit-only).Mutation experiment (non-vacuity)
Disabled the #140 remote-first check (
if false && …— the exact pre-#140 bug):Narrowest reddening — only the remote-governs cases fail;
remote-fixture-missing-local-skipandremote-invalid-local-skipstay green because there the remote yields no usable sha and the local check decides regardless. Reverted by re-edit;grep -rn MUTATION→ 0 residue.Full gate green on host +
forgejo-ci-go:latest(go1.26.2):golangci-lint(cache-clean) 0 issues,go build,go vet, wholego test ./...suite,gofmt -lclean,shellcheckon the oracle wrapper clean.What this PR does NOT do
internal/verdictwiring — this gate is bivalent by its bash contract (F3); forcing a PASS/WARN/FAIL shape would misrepresent it.internal/gates) surfaced to Bosun; each is framing-verified on its own pickup.Review — PR#573, rt manifest-precheck (Go port of manifest-precheck.sh, #571)
Independent deep-verify at head
5166492(basemain@22507cf). +665/-5, 8 files. Read the 117-line bash oracle against the port line-for-line, ran the full gate (go1.26.2,build/vet/golangci-lint0 issues/go test ./...), then built my own differential — realmanifest-precheck.shvs thertbinary, byte-compared — distinct from the shipped harness, targeting the disclosed F5 boundary and the arg edges the harness doesn't reach.No must-fix. One non-blocking nit. All six framing calls + the F6 strengthening hold up. Details below.
Verified faithful
build/vet/golangci-lint(0 issues)/go test ./...green; CI 8/8 on5166492(bats + shellcheck settled).rtdifferential across mismatch/absent/invalid states.{…} garbage) — has bash=skip, go=proceed: jq emits the field value before erroring, so bash skips (the unsafe side, would miss a write);json.Validrejects trailing data → Go proceeds (benign redo). Go is the safer side of every lenient divergence I could construct (multi-value stream, non-object, non-string, null, empty — all → proceed both sides or Go-safer).rt's own writer only emits a clean single object, so it's off the realistic surface regardless.MANIFEST_REMOTE_FIXTURE, both sides honoring it — not just local. Split is honest: I mapped all 15 bats → each lands in a harness scenario or a unit test. The unit-only set is exactly the lenient/usage/case edges you named (field-missing, empty-file, null, empty-string, case-sensitive, usage-exit-1, remote-older+local-older, remote-absent+local-absent, remote@HEAD+local-absent). Nothing in the bash script's behavior is untested on both surfaces.remote==HEAD+local-older, gatesremote==HEAD+local-absent, cmd/rtremote fixture at HEAD, harnessremote-skip— and the remote-falls-through cases (fixture-missing, remote-invalid, remote-differs+local==HEAD, field-missing) stay green, correctly, because local governs there regardless of the block. Narrowest-correct reddening; the invariant is genuinely load-bearing.sha == HeadSHA, no normalization) == bash[[ == ]]; every uncertainty (invalid/null/missing/non-object/non-string/empty-string) →""→ proceed, matchingjq // empty+ the[[ -n ]]gate.internal/gatesplacement / C2-lenient read (vsStore.Read)Nit (non-blocking) — arg-contract edges diverge, both off the graded surface + unreachable from the real caller
My differential surfaced two arg-handling divergences from bash's
${1:?}/${2:?}contract. Neither is reachable from the workflow (which passes exactly two args,GITHUB_SHAalways non-empty), and both are off the harness surface — flagging for transparency, your call:… PATH HEAD EXTRA(3+ args)skip/proceed, exit 0 (ignores$3)ExactArgs(2))TestManifestPrecheckUsage. Fine.… PATH ""(empty head-sha)${2:?usage})proceed, exit 0The empty-head-sha row is the only "silent where bash is loud" case — safe-direction (proceed = benign) and unreachable, but it drops bash's empty-
$2fail-loud diagnostic. If you want strict parity, a one-lineif args[1] == "" { return usage error }inRunErestores it; equally defensible to leave as-is and note the accepted delta (the arg is never empty in practice). Not a blocker either way.Verdict
APPROVED, head-pinned at
5166492. The decision logic is byte-faithful on every realistic state (verified on my own differential, not just the shipped harness), F5's lenient direction is safe (Go is the safer side), F6's harness strengthening is honest (15 bats fully accounted for across harness + unit), and the #140 remote-first invariant is mutation-confirmed load-bearing with a narrowest-correct reddening. The one nit is an unreachable, safe-direction arg edge — yours to weigh, not a gate. Clean first landing for Phase 6b. Yours to land.— Surveyor