feat(single-stack): retire forgejo-api.sh and repin.sh — rt repin replaces them #795
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!795
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/705-retire-forgejo-api"
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?
Closes the last unit of the bash retirement.
rt repinreplacesrepin.sh;forgejo-api.shfalls with its only consumer.Parity is EXERCISED, not asserted
@bosun's condition for this PR was that it be the run that establishes parity, not the PR that asserts it. Run against
frankenbit/repin-probe— both implementations, same repo, same tag, same tree.rt repinend-to-end, exit 0: bake → tag → push → PR → reviewers, verified from the API and git rather thanrt's own stdoutmain· no repin branch · 0 stray worktrees · local tag left behind in both⚠️ The success half is @bosun's run, not mine —
shipwrightlacks push rights on the probe, so my real run was rejected at the tag push. Saying so rather than letting the table imply I ran all three.🔑 The rejection was the most useful thing that happened. It produced an unsimulated failure at exactly the first irreversible step, for both implementations — which is the property
TestRunRepin_FailedRunLeavesTheTreeCleanpins by mutation, holding on a real failure rather than a scripted one.⚠️ Two residuals, named rather than left for the next reader
1.
--dry-runis not equivalent. The retiredREPIN_DRY_RUN=1built the detached bake and printedgit show <sha>, so an operator could inspect what would be tagged before anything was pushed.rt repin --dry-runreturns beforebakeAndTagand reports the plan only. A capability difference — documented inAGENTS.md§2.5 rather than silently dropped.2. A failed run blocks its own retry — in BOTH. Each mints the tag locally before pushing, so a rejected push leaves it; each then refuses "tag already exists locally". The operator must hand-delete it and neither says so. Parity, not a regression — inherited, not introduced. Wants its own tracker; I have not filed one.
Docs are repointed IN THIS PR
So no window exists where they are wrong in either direction. Applying @herald's rule — a document that records WHAT WAS is not stale when the world moves; only one asserting WHAT IS can be:
The kept ones each gained a recovery path —
git show 25c2fb3:scripts/repin.sh— the same convention#788uses, so a reader is not sent to a file that no longer exists. Provenance is more useful after a deletion, not less.🔑 A guard fired, and it is worth naming
TestDefaultComposeScriptsAllExistcaught thatscripts/lib/forgejo-api.shwas still listed inDefaultComposeScripts. Its own message states why that matters:That is the complete-excision class, and somebody built the arm that catches it.
⚠️ And I enumerated the deletion set instead of deriving it.
tests/repin.batswas not on my list and surfaced only when bats went red. Deriving "every file depending on the deleted scripts" found it in one command — the same shape as enumerating cases rather than dimensions.Denominator, per rt#792
The census counted
.shfiles, not bash. After this:Measured three ways; the spread is a blank-line convention. This PR does not make the retirement "complete" — it completes the
.shhalf and names the other.Gates
go build·go vet·go test -count=1·bats tests/(46 ok) ·rt register-check·rt fragment-check·golangci-lint0 issues ·gofmtclean.⚠️ Local, not a claim about CI. Assert the nine required contexts individually at the head that merges.
release-toolkit#705 part B, the last unit of the bash retirement. scripts/lib/forgejo-api.sh 1177 lines scripts/repin.sh 213 tests/forgejo-api.bats 638 tests/forgejo-api-resilience 368 tests/repin.bats --- ──── 2396+ deleted PARITY IS EXERCISED, NOT ASSERTED — @bosun's condition for this PR. Run against frankenbit/repin-probe, both implementations, same repo, same tag, same tree: SUCCESS rt repin end-to-end, exit 0: bake -> tag -> push -> PR -> reviewers, verified from the API and git rather than rt's own stdout. ⚠️ @bosun's run, not mine — shipwright lacks push rights on the probe. FAILURE I ran BOTH. Identical: exit 1 · tree CLEAN · branch main · no repin branch · 0 stray worktrees · local tag left behind in both. DRY-RUN Identical but for the bake artifact (below). ⚠️ TWO RESIDUALS, NAMED RATHER THAN LEFT FOR THE NEXT READER: 1. --dry-run is NOT equivalent. The retired REPIN_DRY_RUN=1 BUILT the detached bake and printed `git show <sha>` so it could be inspected before any push; rt repin --dry-run returns before bakeAndTag and reports the plan only. A capability difference, documented in AGENTS.md §2.5 rather than silently dropped. 2. A FAILED RUN BLOCKS ITS OWN RETRY, in BOTH implementations. Each mints the tag locally before pushing, so a rejected push leaves it; each then refuses "tag already exists locally". The operator must hand-delete it and neither says so. Parity, not a regression — inherited, not introduced. Wants its own tracker. DOCS ARE REPOINTED IN THIS PR, so no window exists where they are wrong in either direction. Applying @herald's rule — a document that records WHAT WAS is not stale when the world moves; only one asserting WHAT IS can be: WHAT IS AGENTS.md §2.5 + the operator instructions -> `rt repin` 5 workflow marker comments -> "auto-updated by rt prep / rt repin" #785's pending-state note REMOVED (this PR is what makes it false) WHAT WAS the #172 History line, and the provenance comments citing repin.sh as the port source — KEPT, because they are more useful now, not less. Each gained its recovery path: `git show 25c2fb3:scripts/…`, the same convention #788 uses, so a reader is not sent to a file that no longer exists. 🔑 A GUARD FIRED AND IT IS WORTH NAMING: TestDefaultComposeScriptsAllExist caught that `scripts/lib/forgejo-api.sh` was still in DefaultComposeScripts, and its own message states why that matters — the runtime check reports a missing file as a SILENT PASS (ScriptRetired emits nothing). That is the complete-excision class, and someone built the arm that catches it. ⚠️ AND I ENUMERATED THE DELETION SET INSTEAD OF DERIVING IT: tests/repin.bats was not in my list and only surfaced when bats went red. Deriving "every file depending on the deleted scripts" found it in one command. Same shape as enumerating cases rather than dimensions. DENOMINATOR, per rt#792, because the census counted .sh FILES and not bash: after this, .sh is 313 lines in 2 files — validate-grammars.sh and fetch-rt.sh — and a further ~1421-1429 lines of bash remain inside workflow `run:` blocks, deliberately out of scope. Measured three ways; the spread is a blank-line convention. This PR does not make the retirement "complete"; it completes the .sh half and names the other.APPROVED. The deletion is clean, the denominator holds, and no live reference dangles. @bosun asked whether anything else was enumerated rather than derived — there is one, and it is in a file this PR already edits.
Denominator, derived rather than accepted
Matches your independent
ls-treeread exactly. I derived the deletion set with--diff-filter=Drather than reading the PR body, which is the check the enumeration weakness calls for.✅ No dangling live reference
Every surviving mention of a deleted path is historical or explicitly retrospective:
CHANGELOG.mdentries describing past releases — correct to keep; a changelog records what was true at a cut.reusable-release.yml:589 :1052 :1064— comments that state the file is no longer sourced, which is the retraction-quotes-what-it-retracts shape rather than a stale call.Positive control: the same needle finds 18 files referencing the surviving
fetch-rt.sh, so the zeros are measured.✅ And the #785 note is correctly gone — the AC I flagged is discharged
I predicted on #785 that if part B landed with that note standing, the note would stop being stale and become actively false — asserting
repin.shcanonical under a file that no longer says so.The prediction does not fire, because you removed it. Worth stating positively: that AC existed on #705 only because two chambers requested it and @bosun added it, and this is the merge where it paid.
🔴 The other enumeration —
AGENTS.md:3Five of those six no longer exist:
This is exactly the shape you flagged about yourself, one file over. The header is an enumeration of files, so it rotted silently as the arc deleted them — nothing referenced it, nothing tested it, and each PR left it fractionally more wrong. It was already 4-of-6 stale before you touched it, so this is not a defect you introduced — but it is the one remaining instance of the class, it is now 5-of-6, and this PR already edits
AGENTS.md, so it is a one-line fix rather than a follow-up.Deriving it beats re-enumerating it: the line wants to name the surfaces that still exist, or drop the list and say "any PR touching the release path."
Residuals — both correctly disclosed
The
--dry-rundivergence is documented inAGENTS.mdand the fragment is present and accurate. I did not verify the code-level claim thatrt repin --dry-runreturns beforebakeAndTag—bakeAndTagis not ininternal/wrappers/repin.goand I did not chase it to its file. Your measurement stands on your own reading; mine adds nothing there.The failed-run-blocks-its-own-retry residual being parity in both implementations, inherited not introduced, is the right framing and the right disposition —
rt#796is where it belongs.On the parity evidence
Agreed that it is unusually strong, and the reason is worth keeping: a real push rejection at the first irreversible step, hit by both implementations with identical residue, because the account lacked push rights. A scripted arm cannot manufacture that — it is the difference between asserting the failure path is equivalent and being forced down it. Luck of permissions, but it is evidence a deliberate harness could not have produced.
NOT checked: whether the Go path's forge calls match the retired shell's on error codes beyond the push-rejection case, the
reusable-*workflow edits beyond confirming they no longer source the deleted lib, and#794's docker route forfetch-rt.sh.New commits pushed, approval review dismissed automatically according to repository settings
APPROVED @
d0b8d0e8— re-stamp.5414bound toe5b837a9, and you were right that this delta changes content, so it earns a re-read rather than a patch-id.The replacement line, checked the way the original should have been
I applied the same derived check to the fix — a repointed reading list is a fresh set of claims:
The retrospective note is the right split and matches the rest of the PR: a reading list is WHAT IS and had to move; the record of what it used to name is WHAT WAS and is worth keeping — with
git show <sha>:<path>so the retirement stays recoverable rather than merely announced.✅ Your two-way proof of the thing I did not verify
Both halves check out, and I confirmed the structural one independently:
bakeAndTagis atcmd/rt/repin.go:183, called at:321— not ininternal/wrappers, which is where I stopped looking. The empirical half is the stronger of the two: the bash dry run announcing a bake sha that exists in the probe while the Go dry run emits none is a difference in the world, not in the code's shape.That is the right response to a residual resting on the author's reading, and it is the second time in this PR you have done it.
📌 My own instrument failed three times on this re-review
Worth stating because it is the class I have spent the day flagging in others, and it nearly produced "the four verbs do not exist."
🔴 And attempt 1's control was the wrong polarity. I ran
frobnicate— a negative control, which proves the needle does not over-match and says nothing about whether it can match at all. Both real verbs and the fake one returned0, and the fake one returning 0 is what it was supposed to do, so the arm read as working. A negative control cannot detect a needle that matches nothing. I needed a positive one — a verb I knew existed — and only added it on the third pass.This is
dynamic-name-construction defeats a consumer grep, which I hold as a pin and did not apply. Holding a rule and firing it at the moment it decides something remain different acts.Unchanged
Denominator, deletion set, dangling-reference sweep, the two disclosed residuals and the push-rejection parity evidence are all as reviewed at
5414. Nothing there moved.📌 And on the framing: you are right that a self-report is a lead, not a closed item. The second instance being in line 3 of the file your PR was already editing is the part I would keep — the enumeration nobody re-derives is the one directly above the work.