frankenbit/release-toolkit
frankenbit/release-toolkit
Watch
17
1
Fork
You've already forked release-toolkit
 0
Code Issues 39 Pull requests Projects Releases 112 Packages 1 Wiki Activity Actions

fix(repin): fetch the remote and build the rc tag and re-pin branch from <remote>/main (#1566) #1567

Merged
alex merged 3 commits from claude/1566-repin-fetches-base into main 2026-10-06 15:59:35 +02:00
Conversation 6 Commits 3 Files changed 5 +434 -44
claude commented 2026-10-06 12:27:55 +02:00
Member

Refs #1566

Summary

rt repin now fetches before it builds the rc tag. Until now it baked whatever the operator's local main held. A checkout that had not fetched would push a tag built from old code. The retired repin.sh fetched first, and the Go port dropped that step without saying so.

Smaller fixes come with it:

  • The re-pin branch now starts at the same commit as the tag. It used to start at whatever the operator had checked out.
  • The wrappers are now read at that commit too, so the plan matches what gets pushed. Review 7281 caught this on the first head.
  • AGENTS.md §2.5 now describes the --reviewer flag. It used to name a $REPIN_REVIEWER env var that rt repin never reads.

This is low urgency. The rc re-pin is dormant for toolkit-self (AGENTS.md §2.2). This PR is for the day someone pins toolkit-self to a tag on purpose.

What changes

  • --base now defaults to <remote>/main, resolved after a git fetch <remote>. An explicit --base still wins. A failed fetch refuses before anything is written.
  • The bake worktree and the repin/<rc-tag> branch both start at that base commit.
  • The wrappers are graded at the base, in a throwaway worktree (planWrappers), before the dry run returns and before the bake. The plan, the "nothing to re-pin" refusal and the "could not grade" refusal all come from the base.
  • --dry-run fetches and prints the base commit and the base's wrappers ([repin] base origin/main = d55a7a707113). It still returns before the bake, as §2.5 already discloses.
  • If the forge cannot name the default branch, the PR targets main, not origin/main. The old comment said repin.sh fell back to --base. It fell back to the literal main (script lines 193-195).
  • AGENTS.md §2.5: steps 1 to 3 match the code, and so does the break-glass comment. The dry-run caveat no longer calls itself the only port difference.

Decision: code or doc, per finding (AGENTS.md §7)

1. No fetch, base is the local main.

  • Fix the code when the verb is meant to give the script's guarantee ("The rc must be cut from current remote main") without the operator remembering to fetch. That matters most because the tag push is the first irreversible step, and it comes after the base is chosen.
  • Fix the doc when something relies on baking an unfetched local base, such as offline use or a local-only commit.
  • Nothing found relies on that. The port commit b858061 has a one-line message, with no record of choosing the local base. No workflow invokes rt repin (#1180). --base <local-ref> still covers the case after this PR.
  • Chosen: code.

2. The re-pin branch starts at the operator's HEAD. The same tree applies. Nothing gains from building the PR on a different commit than the tag, and the script built both from the base. Chosen: code.

2b. Where the plan is graded (review 7281).

  • Grade the operator's checkout when the commit is built from the checkout. That was true on c2fad81, and it is what repin.sh did (lines 123-130).
  • Grade the base when the commit is built at the base, which it is after finding 2.
  • The first head of this PR mixed the two. A base-only wrapper was missing from the plan, and an ungradeable one refused only after the tag was pushed.
  • Chosen: the base. This goes past the script. The script also discovered wrappers in the checkout and committed at $BASE_SHA, so by its source a wrapper that only the base held was never bumped.

3. $REPIN_REVIEWER.

  • Read the env var again when something sets it without a human, such as a workflow or a wrapper script.
  • Describe the flag when the env var was only the script's input channel and the verb now has a flag for the same value.
  • git grep finds REPIN_REVIEWER only in AGENTS.md and the v0.26.0 CHANGELOG entry about repin.sh. Every rt repin mention under .forgejo/ and .gitea/ is a comment, not a run: step. Reading it again would add a second input for one value.
  • Chosen: doc. The same holds for REPIN_REMOTE and REPIN_BASE, which became --remote and --base.

Carry-forward from repin.sh (AGENTS.md §1)

Each step of git show 25c2fb3:scripts/repin.sh, against rt repin before and after this PR:

Script step rt repin at c2fad81 After this PR
1. tag-shape check ported, stricter regex unchanged
2. clean tree ported unchanged
3. git fetch "$REMOTE", base ${REMOTE}/main dropped restored
4. tag absent, locally and on the remote ported unchanged
wrapper discovery in the operator's checkout ported at the base (goes past the script)
5. detached bake at the base ported, but at the local main at the fetched base
dry run builds the bake not ported, disclosed in §2.5 unchanged; now prints the base
6. tag push ported without a local tag (#796) unchanged
7. wrapper branch from $BASE_SHA from the operator's HEAD from the base
8. PR base fallback main fell back to --base the base's branch name (main by default)
failed review request is a warning an error unchanged; now disclosed in §2.5
REPIN_* env inputs flags unchanged; §2.5 now says flags

Evidence

Reproduction. TestRunRepin_BuildsFromTheFetchedRemoteMain drives runRepin against real git. The remote's main is one commit ahead of an operator clone that has not fetched. The test reads --base's default from the command, so it grades what an operator gets. On c2fad81 it fails on every count:

bake built at f41ef98c29d0, want the remote's main d2289bbf602f (stale local main is f41ef98c29d0; --base default "main")
rc tag on the remote points at f41ef98c29d0, want d2289bbf602f
re-pin branch is built on f41ef98c29d0, want the remote's main d2289bbf602f (stale local main is f41ef98c29d0)

With an explicit --base origin/main after a manual fetch, c2fad81 still built the branch on the stale commit. That run isolates finding 2.

Review 7281's two shapes. Both new tests run against real git with a stale clone. With b2500f8's repin.go they fail as the review measured:

  • TestRunRepin_PlansTheBasesWrappers: the dry-run plan lacks re-pinned 2 wrapper(s) and second.yml, and the PR body does not list second.yml, although the pushed commit changes it.
  • TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag: the remote holds the rc tag when the refusal comes, the bake has run, and the forge was called.
  • TestRunRepin_RefusesWhenNothingPinned, which now pins a wrapper in the operator's tree and none in the base, gets no refusal.

Mutants. Each was applied byte-exactly to cmd/rt/repin.go at 0e7f37d and graded with go test ./cmd/rt/ -run Repin in golang:1.25. All 12 are killed.

Mutant Killed by
M1 no fetch BuildsFromTheFetchedRemoteMain, PlansTheBasesWrappers, UngradeableBaseWrapper…, RefusesWhenTheFetchFails, DryRunResolvesTheBase
M2 default base is the local main BuildsFromTheFetchedRemoteMain, PlansTheBasesWrappers, UngradeableBaseWrapper…, DryRunResolvesTheBase
M3 --base flag default back to main BuildsFromTheFetchedRemoteMain
M4 re-pin branch without a start point BuildsFromTheFetchedRemoteMain
M5 fetch error ignored RefusesWhenTheFetchFails
M6 PR fallback keeps origin/ PRBaseFallbackIsABranchName
M7 resolve before fetch BuildsFromTheFetchedRemoteMain, PlansTheBasesWrappers, UngradeableBaseWrapper…, RefusesWhenTheFetchFails, DryRunResolvesTheBase
M8 dry run returns before the base and the plan DryRunResolvesTheBase (both subtests), PlansTheBasesWrappers
M9 bake worktree at HEAD BuildsFromTheFetchedRemoteMain
M10 plan from the operator's checkout PlansTheBasesWrappers, UngradeableBaseWrapper…, RefusesWhenNothingPinned
M11 plan worktree at HEAD PlansTheBasesWrappers, UngradeableBaseWrapper…
M12 plan errors ignored UngradeableBaseWrapper…

M8 used to move only the base resolution below the dry-run return. On this head that no longer compiles, because the plan needs the base, so M8 now moves the dry-run return above both.

End to end. The built rt at 0e7f37d ran in a clone one commit behind a local bare remote:

remote main : d55a7a707113
local main  : dc546fc5f1aa  (clone, not fetched)
--- rt repin --dry-run v9.9.9-rc.1
[repin] base origin/main = d55a7a707113
[repin] re-pinned 1 wrapper(s) to v9.9.9-rc.1
[repin]   .forgejo/workflows/release.yml
[repin] --dry-run: fetched, nothing else written; no tag minted, no push, no PR
rc=0
local main after: dc546fc5f1aa
--- rt repin --dry-run --base origin/nope v9.9.9-rc.1
Error: rt repin: resolve base "origin/nope": exit status 128
rc=1

Suite. On an LF clone at 0e7f37d, in golang:1.25:

  • go test ./... gives rc=0, 30 packages ok.
  • golangci-lint in forgejo-ci-go:latest reports 0 issues.
  • rt fragment-check (content mode) passes with no warnings.

Self-bootstrap (AGENTS.md §2)

No re-pin: toolkit-self floats @main (AGENTS.md §2.1). Adopters get the change from the next cut tag, though no adopter invokes rt repin. rt check-self-bootstrap on this head:

[check-self-bootstrap] pinned toolkit ref: main
[check-self-bootstrap] floating-pin (release-toolkit#456): main matches main by construction

Changelog

changelog.d/1566-repin-fetches-base.fixed.md.

Out of scope

  • A test of realBake itself. That is #1563.
  • Three architecture docs still name REPIN_DRY_RUN as a seam rt repin honours: docs/architecture/test-strategy.md §5, and arc42 §2.3 and §8.3. It takes --dry-run. That is the same drift, but outside #1566's criteria.

Review request

@alex, please verify at source (AGENTS.md §6):

  1. Run TestRunRepin_BuildsFromTheFetchedRemoteMain on c2fad81 (red) and on this head (green).
  2. Read resolveBase, planWrappers and the three git worktree add calls in cmd/rt/repin.go. All three must start at baseSHA, and the plan must come before the dry-run return.
  3. Read AGENTS.md §2.5 against rt repin --help.

Head: 0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb. Review 7281 was on b2500f8; the delta is one commit, b2500f8..0e7f37d. Pin it with rt review-submit-check (§6.2).

🤖 Generated with Claude Code

Refs #1566 ## Summary `rt repin` now fetches before it builds the rc tag. Until now it baked whatever the operator's local `main` held. A checkout that had not fetched would push a tag built from old code. The retired `repin.sh` fetched first, and the Go port dropped that step without saying so. Smaller fixes come with it: - The re-pin branch now starts at the same commit as the tag. It used to start at whatever the operator had checked out. - The wrappers are now read at that commit too, so the plan matches what gets pushed. Review 7281 caught this on the first head. - AGENTS.md §2.5 now describes the `--reviewer` flag. It used to name a `$REPIN_REVIEWER` env var that `rt repin` never reads. This is low urgency. The rc re-pin is dormant for toolkit-self (AGENTS.md §2.2). This PR is for the day someone pins toolkit-self to a tag on purpose. ## What changes - `--base` now defaults to `<remote>/main`, resolved after a `git fetch <remote>`. An explicit `--base` still wins. A failed fetch refuses before anything is written. - The bake worktree and the `repin/<rc-tag>` branch both start at that base commit. - The wrappers are graded at the base, in a throwaway worktree (`planWrappers`), before the dry run returns and before the bake. The plan, the "nothing to re-pin" refusal and the "could not grade" refusal all come from the base. - `--dry-run` fetches and prints the base commit and the base's wrappers (`[repin] base origin/main = d55a7a707113`). It still returns before the bake, as §2.5 already discloses. - If the forge cannot name the default branch, the PR targets `main`, not `origin/main`. The old comment said `repin.sh` fell back to `--base`. It fell back to the literal `main` (script lines 193-195). - AGENTS.md §2.5: steps 1 to 3 match the code, and so does the break-glass comment. The dry-run caveat no longer calls itself the only port difference. ## Decision: code or doc, per finding (AGENTS.md §7) **1. No fetch, base is the local `main`.** - *Fix the code* when the verb is meant to give the script's guarantee ("The rc must be cut from current remote main") without the operator remembering to fetch. That matters most because the tag push is the first irreversible step, and it comes after the base is chosen. - *Fix the doc* when something relies on baking an unfetched local base, such as offline use or a local-only commit. - Nothing found relies on that. The port commit `b858061` has a one-line message, with no record of choosing the local base. No workflow invokes `rt repin` (#1180). `--base <local-ref>` still covers the case after this PR. - **Chosen: code.** **2. The re-pin branch starts at the operator's HEAD.** The same tree applies. Nothing gains from building the PR on a different commit than the tag, and the script built both from the base. **Chosen: code.** **2b. Where the plan is graded (review 7281).** - *Grade the operator's checkout* when the commit is built from the checkout. That was true on `c2fad81`, and it is what `repin.sh` did (lines 123-130). - *Grade the base* when the commit is built at the base, which it is after finding 2. - The first head of this PR mixed the two. A base-only wrapper was missing from the plan, and an ungradeable one refused only after the tag was pushed. - **Chosen: the base.** This goes past the script. The script also discovered wrappers in the checkout and committed at `$BASE_SHA`, so by its source a wrapper that only the base held was never bumped. **3. `$REPIN_REVIEWER`.** - *Read the env var again* when something sets it without a human, such as a workflow or a wrapper script. - *Describe the flag* when the env var was only the script's input channel and the verb now has a flag for the same value. - `git grep` finds `REPIN_REVIEWER` only in AGENTS.md and the v0.26.0 CHANGELOG entry about `repin.sh`. Every `rt repin` mention under `.forgejo/` and `.gitea/` is a comment, not a `run:` step. Reading it again would add a second input for one value. - **Chosen: doc.** The same holds for `REPIN_REMOTE` and `REPIN_BASE`, which became `--remote` and `--base`. ## Carry-forward from `repin.sh` (AGENTS.md §1) Each step of `git show 25c2fb3:scripts/repin.sh`, against `rt repin` before and after this PR: | Script step | `rt repin` at `c2fad81` | After this PR | |---|---|---| | 1. tag-shape check | ported, stricter regex | unchanged | | 2. clean tree | ported | unchanged | | 3. `git fetch "$REMOTE"`, base `${REMOTE}/main` | **dropped** | restored | | 4. tag absent, locally and on the remote | ported | unchanged | | wrapper discovery in the operator's checkout | ported | **at the base** (goes past the script) | | 5. detached bake at the base | ported, but at the local `main` | at the fetched base | | dry run builds the bake | not ported, disclosed in §2.5 | unchanged; now prints the base | | 6. tag push | ported without a local tag (#796) | unchanged | | 7. wrapper branch from `$BASE_SHA` | **from the operator's HEAD** | from the base | | 8. PR base fallback `main` | fell back to `--base` | the base's branch name (`main` by default) | | failed review request is a warning | an error | unchanged; now disclosed in §2.5 | | `REPIN_*` env inputs | flags | unchanged; §2.5 now says flags | ## Evidence **Reproduction.** `TestRunRepin_BuildsFromTheFetchedRemoteMain` drives `runRepin` against real git. The remote's `main` is one commit ahead of an operator clone that has not fetched. The test reads `--base`'s default from the command, so it grades what an operator gets. On `c2fad81` it fails on every count: ``` bake built at f41ef98c29d0, want the remote's main d2289bbf602f (stale local main is f41ef98c29d0; --base default "main") rc tag on the remote points at f41ef98c29d0, want d2289bbf602f re-pin branch is built on f41ef98c29d0, want the remote's main d2289bbf602f (stale local main is f41ef98c29d0) ``` With an explicit `--base origin/main` after a manual fetch, `c2fad81` still built the branch on the stale commit. That run isolates finding 2. **Review 7281's two shapes.** Both new tests run against real git with a stale clone. With `b2500f8`'s `repin.go` they fail as the review measured: - `TestRunRepin_PlansTheBasesWrappers`: the dry-run plan lacks `re-pinned 2 wrapper(s)` and `second.yml`, and the PR body does not list `second.yml`, although the pushed commit changes it. - `TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag`: the remote holds the rc tag when the refusal comes, the bake has run, and the forge was called. - `TestRunRepin_RefusesWhenNothingPinned`, which now pins a wrapper in the operator's tree and none in the base, gets no refusal. **Mutants.** Each was applied byte-exactly to `cmd/rt/repin.go` at `0e7f37d` and graded with `go test ./cmd/rt/ -run Repin` in `golang:1.25`. All 12 are killed. | Mutant | Killed by | |---|---| | M1 no fetch | BuildsFromTheFetchedRemoteMain, PlansTheBasesWrappers, UngradeableBaseWrapper…, RefusesWhenTheFetchFails, DryRunResolvesTheBase | | M2 default base is the local `main` | BuildsFromTheFetchedRemoteMain, PlansTheBasesWrappers, UngradeableBaseWrapper…, DryRunResolvesTheBase | | M3 `--base` flag default back to `main` | BuildsFromTheFetchedRemoteMain | | M4 re-pin branch without a start point | BuildsFromTheFetchedRemoteMain | | M5 fetch error ignored | RefusesWhenTheFetchFails | | M6 PR fallback keeps `origin/` | PRBaseFallbackIsABranchName | | M7 resolve before fetch | BuildsFromTheFetchedRemoteMain, PlansTheBasesWrappers, UngradeableBaseWrapper…, RefusesWhenTheFetchFails, DryRunResolvesTheBase | | M8 dry run returns before the base and the plan | DryRunResolvesTheBase (both subtests), PlansTheBasesWrappers | | M9 bake worktree at `HEAD` | BuildsFromTheFetchedRemoteMain | | M10 plan from the operator's checkout | PlansTheBasesWrappers, UngradeableBaseWrapper…, RefusesWhenNothingPinned | | M11 plan worktree at `HEAD` | PlansTheBasesWrappers, UngradeableBaseWrapper… | | M12 plan errors ignored | UngradeableBaseWrapper… | M8 used to move only the base resolution below the dry-run return. On this head that no longer compiles, because the plan needs the base, so M8 now moves the dry-run return above both. **End to end.** The built `rt` at `0e7f37d` ran in a clone one commit behind a local bare remote: ``` remote main : d55a7a707113 local main : dc546fc5f1aa (clone, not fetched) --- rt repin --dry-run v9.9.9-rc.1 [repin] base origin/main = d55a7a707113 [repin] re-pinned 1 wrapper(s) to v9.9.9-rc.1 [repin] .forgejo/workflows/release.yml [repin] --dry-run: fetched, nothing else written; no tag minted, no push, no PR rc=0 local main after: dc546fc5f1aa --- rt repin --dry-run --base origin/nope v9.9.9-rc.1 Error: rt repin: resolve base "origin/nope": exit status 128 rc=1 ``` **Suite.** On an LF clone at `0e7f37d`, in `golang:1.25`: - `go test ./...` gives rc=0, 30 packages ok. - `golangci-lint` in `forgejo-ci-go:latest` reports 0 issues. - `rt fragment-check` (content mode) passes with no warnings. ## Self-bootstrap (AGENTS.md §2) No re-pin: toolkit-self floats `@main` (AGENTS.md §2.1). Adopters get the change from the next cut tag, though no adopter invokes `rt repin`. `rt check-self-bootstrap` on this head: ``` [check-self-bootstrap] pinned toolkit ref: main [check-self-bootstrap] floating-pin (release-toolkit#456): main matches main by construction ``` ## Changelog `changelog.d/1566-repin-fetches-base.fixed.md`. ## Out of scope - A test of `realBake` itself. That is #1563. - Three architecture docs still name `REPIN_DRY_RUN` as a seam `rt repin` honours: `docs/architecture/test-strategy.md` §5, and arc42 §2.3 and §8.3. It takes `--dry-run`. That is the same drift, but outside #1566's criteria. ## Review request @alex, please verify at source (AGENTS.md §6): 1. Run `TestRunRepin_BuildsFromTheFetchedRemoteMain` on `c2fad81` (red) and on this head (green). 2. Read `resolveBase`, `planWrappers` and the three `git worktree add` calls in `cmd/rt/repin.go`. All three must start at `baseSHA`, and the plan must come before the dry-run return. 3. Read AGENTS.md §2.5 against `rt repin --help`. Head: `0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb`. Review 7281 was on `b2500f8`; the delta is one commit, `b2500f8..0e7f37d`. Pin it with `rt review-submit-check` (§6.2). 🤖 Generated with [Claude Code](https://claude.com/claude-code) 
rt repin baked whatever the operator's local main held. It never ran
git fetch, and --base defaulted to the local branch. The retired
repin.sh fetched first and defaulted to ${REMOTE}/main ("The rc must
be cut from current remote main"). The re-pin branch also started at
the operator's HEAD, because `git worktree add -b` had no start point,
so --base moved the tag but not the PR.

- resolveBase fetches --remote, then resolves --base (default
  <remote>/main) to a commit. A failed fetch refuses before anything
  is written. It runs before --dry-run returns, so the plan names the
  base commit.
- The bake worktree and the re-pin branch both start at that commit.
- When the forge cannot name the default branch, the PR base is the
  base's branch name (main), not origin/main. The old comment said
  repin.sh fell back to --base; it fell back to the literal main.

TestRunRepin_BuildsFromTheFetchedRemoteMain drives runRepin against
real git with a remote one commit ahead of an unfetched clone, using
the command's own --base default. It fails on c2fad81.

Refs #1566

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs(agents): section 2.5 describes rt repin's fetch, base and --reviewer flag (#1566)
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 5s
Details
base-divergence-check / check (pull_request) Successful in 6s
Details
go-ci / lint + build + test (pull_request) Successful in 33s
Details
go-ci / record reviewed vs landed commit (pull_request) Has been skipped
Details
pr / plan, and run every gate when combined (pull_request) Successful in 10s
Details
readme-pin-check / mirror (pull_request) Successful in 7s
Details
tests / bats (pull_request) Successful in 38s
Details
tests / checks (pull_request) Successful in 8s
Details
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 8s
Details
go-ci / page landing-tree failure (pull_request) Has been skipped
Details
pr / manifest (pull_request) Has been skipped
Details
pr / fragments (pull_request) Has been skipped
Details
pr / changelog_body (pull_request) Has been skipped
Details
pr / register (pull_request) Has been skipped
Details
pr / ac_closure (pull_request) Has been skipped
Details
pr / workflow_parse (pull_request) Has been skipped
Details
pr / split result (pull_request) Has been skipped
Details
pr / gates (pull_request) Successful in 0s
Details
b2500f8f65 
Step 1 said the bake is built from "current origin/main", which held
for repin.sh but not for rt repin until the previous commit. Step 3
named $REPIN_REVIEWER, which rt repin never reads; it takes a
repeatable --reviewer flag. The dry-run caveat called itself the one
behavioural difference the port did not preserve; #1566 found three
more, now restored, and one that stays (a failed review request is
an error, not a warning).

Refs #1566

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude requested review from alex 2026-10-06 12:27:55 +02:00
claude reviewed 2026-10-06 15:06:33 +02:00
claude left a comment

Verdict: CHANGES NEEDED at b2500f8f65e23d9a58841fcb4a5b1c6cbc93a8b5. The fetch and the base are right and verified. There is one finding: moving the commit to the base leaves the wrapper plan and the pre-push refusals grading a different tree. This is posted as COMMENT because the forge identity is claude, the PR's author.

Refs #1566

Finding: the plan and the #843 refusal read the operator's tree, while the commit is built at the base

runRepin still computes changed with repinWrappers(d.dir, o.rcTag, false), which reads the operator's working tree. After this PR, the commit is built from repinWrappers(tmp, ...) in a worktree at baseSHA. In the case #1566 is about, a checkout that has not fetched, those are two different trees. I measured both consequences with a throwaway probe test on this head: real git, a bare remote, and an operator clone one commit behind.

(a) The plan does not describe the commit. The base gained a second wrapper (release-cut.yml) that the stale clone lacks.

  • The dry run and the real run both print re-pinned 1 wrapper(s), listing only release.yml.
  • The pushed repin/ commit changes both release-cut.yml and release.yml.
  • So the new §2.5 sentence, "rt repin --dry-run prints the base commit and the wrappers it would change", is false in exactly that case.

(b) A refusal now fires after the first irreversible step. The base holds a wrapper the discovery cannot grade (jobs: { r: { uses: ...reusable-release.yml@main } }), and the stale clone does not.

  • The pre-push repinWrappers(d.dir, ...) passes.
  • The rc tag is pushed.
  • Then commitInWorktree's rewrite at the base refuses with COULD NOT GRADE 1 workflow file(s) ... odd.yml.
  • What is left: the rc tag is on the remote (e42a730e...), there is no repin/ branch, and there is no PR. Forge calls: [default-branch] only.

That breaks the verb's own contract: "It refuses before it mutates, and the push is the first irreversible step". It is new in this PR. On c2fad81, the commit worktree started at the operator's HEAD, which is the tree that validateRepin's clean-tree check and the pre-push repinWrappers(d.dir, ...) grade, so the #843 refusal always fired before the push.

repin.sh does not settle this. It also discovered wrappers in the operator's checkout, assuming a clean tree "matches the tip of the tracked ref". It then skipped base-only wrappers with [[ -f "$wf" ]] || continue, so the parity table has no row that covers it.

Suggested fix: grade the base, not d.dir, before anything is pushed.

  • In runRepin, after resolveBase, add a detached worktree at baseSHA and run repinWrappers(<that tree>, o.rcTag, false). That gives the plan, the zero-wrapper refusal and the could-not-grade refusal.
  • Do this in the dry run too, so the plan names base-relative files.
  • Add a test for each of the two probe shapes above: the plan lists the base's wrappers, and an ungradeable base wrapper refuses with no tag on the remote.
  • The §2.5 sentence then holds as written.

Verified at source

  • Your request 1: TestRunRepin_BuildsFromTheFetchedRemoteMain fails on c2fad81 with all three errors (bake, tag, branch on the stale commit) and passes on this head. Both runs were in golang:1.25.
  • Your request 2: resolveBase fetches --remote before it runs rev-parse --verify <ref>^{commit}. bakeCommit and commitInWorktree now both receive baseSHA, and the branch worktree starts there.
  • Your request 3: §2.5 matches the flags. --reviewer is repeatable, and --remote and --base have the defaults described. No .yml, .sh or .go file reads REPIN_REVIEWER, REPIN_REMOTE or REPIN_BASE; the only hit is a comment in repin.go.
  • Script history: at 25c2fb3:scripts/repin.sh, line 88 fetches, line 89 defaults the base to ${REMOTE}/main, line 157 builds the wrapper worktree at $BASE_SHA, lines 193-195 fall back to the literal main, and line 209 makes a failed review request a WARNING.
  • CI: all 18 statuses on the head are success.

Not re-run by me

Your nine mutants and the end-to-end dry run.

Verdict: CHANGES NEEDED at `b2500f8f65e23d9a58841fcb4a5b1c6cbc93a8b5`. The fetch and the base are right and verified. There is one finding: moving the commit to the base leaves the wrapper plan and the pre-push refusals grading a different tree. This is posted as COMMENT because the forge identity is `claude`, the PR's author. Refs #1566 ## Finding: the plan and the #843 refusal read the operator's tree, while the commit is built at the base `runRepin` still computes `changed` with `repinWrappers(d.dir, o.rcTag, false)`, which reads the operator's working tree. After this PR, the commit is built from `repinWrappers(tmp, ...)` in a worktree at `baseSHA`. In the case #1566 is about, a checkout that has not fetched, those are two different trees. I measured both consequences with a throwaway probe test on this head: real git, a bare remote, and an operator clone one commit behind. **(a) The plan does not describe the commit.** The base gained a second wrapper (`release-cut.yml`) that the stale clone lacks. - The dry run and the real run both print `re-pinned 1 wrapper(s)`, listing only `release.yml`. - The pushed `repin/` commit changes **both** `release-cut.yml` and `release.yml`. - So the new §2.5 sentence, "`rt repin --dry-run` prints the base commit and the wrappers it would change", is false in exactly that case. **(b) A refusal now fires after the first irreversible step.** The base holds a wrapper the discovery cannot grade (`jobs: { r: { uses: ...reusable-release.yml@main } }`), and the stale clone does not. - The pre-push `repinWrappers(d.dir, ...)` passes. - The rc tag is pushed. - Then `commitInWorktree`'s rewrite at the base refuses with `COULD NOT GRADE 1 workflow file(s) ... odd.yml`. - What is left: **the rc tag is on the remote** (`e42a730e...`), there is no `repin/` branch, and there is no PR. Forge calls: `[default-branch]` only. That breaks the verb's own contract: "It refuses before it mutates, and the push is the first irreversible step". It is new in this PR. On `c2fad81`, the commit worktree started at the operator's HEAD, which is the tree that `validateRepin`'s clean-tree check and the pre-push `repinWrappers(d.dir, ...)` grade, so the #843 refusal always fired before the push. `repin.sh` does not settle this. It also discovered wrappers in the operator's checkout, assuming a clean tree "matches the tip of the tracked ref". It then skipped base-only wrappers with `[[ -f "$wf" ]] || continue`, so the parity table has no row that covers it. **Suggested fix:** grade the base, not `d.dir`, before anything is pushed. - In `runRepin`, after `resolveBase`, add a detached worktree at `baseSHA` and run `repinWrappers(<that tree>, o.rcTag, false)`. That gives the plan, the zero-wrapper refusal and the could-not-grade refusal. - Do this in the dry run too, so the plan names base-relative files. - Add a test for each of the two probe shapes above: the plan lists the base's wrappers, and an ungradeable base wrapper refuses with no tag on the remote. - The §2.5 sentence then holds as written. ## Verified at source - **Your request 1:** `TestRunRepin_BuildsFromTheFetchedRemoteMain` fails on `c2fad81` with all three errors (bake, tag, branch on the stale commit) and passes on this head. Both runs were in `golang:1.25`. - **Your request 2:** `resolveBase` fetches `--remote` before it runs `rev-parse --verify <ref>^{commit}`. `bakeCommit` and `commitInWorktree` now both receive `baseSHA`, and the branch worktree starts there. - **Your request 3:** §2.5 matches the flags. `--reviewer` is repeatable, and `--remote` and `--base` have the defaults described. No `.yml`, `.sh` or `.go` file reads `REPIN_REVIEWER`, `REPIN_REMOTE` or `REPIN_BASE`; the only hit is a comment in `repin.go`. - **Script history:** at `25c2fb3:scripts/repin.sh`, line 88 fetches, line 89 defaults the base to `${REMOTE}/main`, line 157 builds the wrapper worktree at `$BASE_SHA`, lines 193-195 fall back to the literal `main`, and line 209 makes a failed review request a WARNING. - **CI:** all 18 statuses on the head are success. ## Not re-run by me Your nine mutants and the end-to-end dry run.
claude commented 2026-10-06 15:07:30 +02:00
Author
Member

HOLD. Review 7281's finding is confirmed: the plan, the zero-wrapper refusal and the could-not-grade refusal still read the operator's tree, while the commit is built at the base. A new commit on this branch moves the plan to a detached worktree at the base, before the dry run returns and before the tag push, with a test for each of the two shapes. I will post the new full head SHA when it is pushed. Reviewed head stays b2500f8f65e23d9a58841fcb4a5b1c6cbc93a8b5.

HOLD. Review 7281's finding is confirmed: the plan, the zero-wrapper refusal and the could-not-grade refusal still read the operator's tree, while the commit is built at the base. A new commit on this branch moves the plan to a detached worktree at the base, before the dry run returns and before the tag push, with a test for each of the two shapes. I will post the new full head SHA when it is pushed. Reviewed head stays `b2500f8f65e23d9a58841fcb4a5b1c6cbc93a8b5`.
fix(repin): grade the wrappers at the base, before the dry run returns and before the tag push (#1566)
All checks were successful
base-divergence-check / check (pull_request) Successful in 5s
Details
go-ci / lint + build + test (pull_request) Successful in 33s
Details
go-ci / record reviewed vs landed commit (pull_request) Has been skipped
Details
readme-pin-check / mirror (pull_request) Successful in 7s
Details
tests / bats (pull_request) Successful in 38s
Details
tests / checks (pull_request) Successful in 7s
Details
go-ci / page landing-tree failure (pull_request) Has been skipped
Details
pr / plan, and run every gate when combined (pull_request) Successful in 9s
Details
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
Details
pr / manifest (pull_request) Has been skipped
Details
pr / fragments (pull_request) Has been skipped
Details
pr / changelog_body (pull_request) Has been skipped
Details
pr / register (pull_request) Has been skipped
Details
pr / ac_closure (pull_request) Has been skipped
Details
pr / workflow_parse (pull_request) Has been skipped
Details
pr / split result (pull_request) Has been skipped
Details
pr / gates (pull_request) Successful in 0s
Details
check-self-bootstrap / check (push) Successful in 5s
Details
gitea-twin-check / check (push) Successful in 4s
Details
go-ci / lint + build + test (push) Successful in 34s
Details
go-ci / record reviewed vs landed commit (push) Successful in 2s
Details
prepared-uncut-check / toolkit-self prepared-uncut controls (push) Successful in 5s
Details
prepared-uncut-check / prepared-but-uncut release (push) Successful in 7s
Details
prepared-uncut-check / check (push) Successful in 0s
Details
release / decide + act (push) Successful in 11s
Details
release / release (push) Successful in 0s
Details
tests / bats (push) Successful in 37s
Details
tests / checks (push) Successful in 7s
Details
go-ci / page landing-tree failure (push) Has been skipped
Details
release / fire-cut (push) Has been skipped
Details
0e7f37d5a9 
Review 7281 on PR #1567. The previous commit built the re-pin commit
at the fetched base, but runRepin still computed the plan from the
operator's checkout. Two shapes, measured on b2500f8 with a stale
clone one commit behind:

- The base gains a second wrapper. The dry run and the real run both
  printed 1 wrapper, while the pushed commit changed 2, and the PR body
  listed 1.
- The base holds a wrapper Discover cannot grade. The pre-push check
  passed, the rc tag was pushed, and only then did commitInWorktree's
  rewrite refuse with COULD NOT GRADE. That left a tag with no branch
  and no PR. It was new in this PR: on c2fad81 the commit tree was the
  operator's HEAD, the tree the check graded.

planWrappers now checks the base out in a throwaway detached worktree
and runs repinWrappers there with write=false. The plan, the
nothing-to-re-pin refusal and the could-not-grade refusal all come
from the base, before the dry run returns and before the bake. It
prints repository-relative paths.

This goes past repin.sh, which discovered wrappers in the checkout
too and committed at $BASE_SHA, so a wrapper only the base held was
never bumped.

Tests: TestRunRepin_PlansTheBasesWrappers and
TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag run against
real git, and TestRunRepin_RefusesWhenNothingPinned now puts the
pinned wrapper in the operator's tree and none in the base. All three
fail on b2500f8's repin.go.

Refs #1566

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude commented 2026-10-06 15:16:13 +02:00
Author
Member

Released from HOLD. New head: 0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb, one new commit on top of the reviewed b2500f8f65e23d9a58841fcb4a5b1c6cbc93a8b5 (fast-forward, no rewrite).

It answers review 7281: planWrappers grades the wrappers in a detached worktree at the base, before the dry run returns and before the bake. Both measured shapes have a real-git test, and both are red on b2500f8's repin.go. RefusesWhenNothingPinned now puts the pinned wrapper in the operator's tree and none in the base. All 12 mutants are killed, including three new ones on the plan. go test ./... and golangci-lint are clean on 0e7f37d.

The PR body is updated too: finding 2b in the decision tree, a carry-forward row for wrapper discovery, the new mutant table and the new head. Please take a fresh read and pin 0e7f37d with rt review-submit-check.

Released from HOLD. New head: `0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb`, one new commit on top of the reviewed `b2500f8f65e23d9a58841fcb4a5b1c6cbc93a8b5` (fast-forward, no rewrite). It answers review 7281: `planWrappers` grades the wrappers in a detached worktree at the base, before the dry run returns and before the bake. Both measured shapes have a real-git test, and both are red on `b2500f8`'s `repin.go`. `RefusesWhenNothingPinned` now puts the pinned wrapper in the operator's tree and none in the base. All 12 mutants are killed, including three new ones on the plan. `go test ./...` and `golangci-lint` are clean on `0e7f37d`. The PR body is updated too: finding 2b in the decision tree, a carry-forward row for wrapper discovery, the new mutant table and the new head. Please take a fresh read and pin `0e7f37d` with `rt review-submit-check`.
claude reviewed 2026-10-06 15:18:20 +02:00
claude left a comment

Verdict: APPROVE at 0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb. This replaces review 7281, which was at b2500f8. It is posted as COMMENT because the forge identity is claude, the PR's author, so @alex has to click Approve.

Refs #1566

Delta b2500f8..0e7f37d, verified at source

  • History is intact: b2500f8 is an ancestor of the new head, so nothing was rewritten. The base is still main c2fad81.
  • Where the plan is graded: planWrappers checks baseSHA out in a detached throwaway worktree and runs repinWrappers(tmp, rcTag, false) there. runRepin calls it after resolveBase and before both the dry-run return and the bake. So the plan, the zero-wrapper refusal and the could-not-grade refusal all grade the tree the commit is built from.
  • My two probe shapes from 7281, re-run on this head (real git, a stale clone one commit behind):
    • (a) The base gained a second wrapper. The dry run and the real run now both print re-pinned 2 wrapper(s): .forgejo/workflows/release-cut.yml and .forgejo/workflows/release.yml. The pushed commit changes those same two files.
    • (b) The base holds an ungradeable wrapper. The run refuses with at base <sha>: ... COULD NOT GRADE ... odd.yml. The remote has no rc tag and no repin/ branch, and there were no forge calls.
  • The new tests are not placebos: with b2500f8's repin.go restored, TestRunRepin_PlansTheBasesWrappers, TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag and TestRunRepin_RefusesWhenNothingPinned all fail. On this head they pass.
  • Suite: go test ./... in golang:1.25 passes in every package.
  • AGENTS.md section 2.5: step 1 and the caveat now match the code. The new script claim holds: repin.sh discovered wrappers with rt consumer-wrappers .forgejo/workflows in the operator's checkout, then looped only over that list in the base worktree. A wrapper that only the base held was never bumped.
  • CI: all 18 statuses on 0e7f37d are success.

Nit, non-blocking

Section 2.5 says the dry run "fetches --remote and writes nothing else". It now also creates and removes a throwaway worktree, which leaves transient .git/worktrees metadata that its own defer cleans up. That is accurate enough for an operator, but you may want "writes nothing else to the checkout" if you touch the line again.

Everything verified in 7281 still holds: the fetch-then-resolve order, both worktrees at baseSHA, the --reviewer flag and the script line citations.

Verdict: APPROVE at `0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb`. This replaces review 7281, which was at `b2500f8`. It is posted as COMMENT because the forge identity is `claude`, the PR's author, so @alex has to click Approve. Refs #1566 ## Delta `b2500f8..0e7f37d`, verified at source - **History is intact:** `b2500f8` is an ancestor of the new head, so nothing was rewritten. The base is still main `c2fad81`. - **Where the plan is graded:** `planWrappers` checks `baseSHA` out in a detached throwaway worktree and runs `repinWrappers(tmp, rcTag, false)` there. `runRepin` calls it after `resolveBase` and before both the dry-run return and the bake. So the plan, the zero-wrapper refusal and the could-not-grade refusal all grade the tree the commit is built from. - **My two probe shapes from 7281, re-run on this head** (real git, a stale clone one commit behind): - **(a)** The base gained a second wrapper. The dry run and the real run now both print `re-pinned 2 wrapper(s)`: `.forgejo/workflows/release-cut.yml` and `.forgejo/workflows/release.yml`. The pushed commit changes those same two files. - **(b)** The base holds an ungradeable wrapper. The run refuses with `at base <sha>: ... COULD NOT GRADE ... odd.yml`. The remote has **no rc tag** and **no `repin/` branch**, and there were **no forge calls**. - **The new tests are not placebos:** with `b2500f8`'s `repin.go` restored, `TestRunRepin_PlansTheBasesWrappers`, `TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag` and `TestRunRepin_RefusesWhenNothingPinned` all fail. On this head they pass. - **Suite:** `go test ./...` in `golang:1.25` passes in every package. - **AGENTS.md section 2.5:** step 1 and the caveat now match the code. The new script claim holds: `repin.sh` discovered wrappers with `rt consumer-wrappers .forgejo/workflows` in the operator's checkout, then looped only over that list in the base worktree. A wrapper that only the base held was never bumped. - **CI:** all 18 statuses on `0e7f37d` are success. ## Nit, non-blocking Section 2.5 says the dry run "fetches `--remote` and writes nothing else". It now also creates and removes a throwaway worktree, which leaves transient `.git/worktrees` metadata that its own `defer` cleans up. That is accurate enough for an operator, but you may want "writes nothing else to the checkout" if you touch the line again. Everything verified in 7281 still holds: the fetch-then-resolve order, both worktrees at `baseSHA`, the `--reviewer` flag and the script line citations.
alex approved these changes 2026-10-06 15:59:30 +02:00
alex merged commit 0e7f37d5a9 into main 2026-10-06 15:59:35 +02:00
alex deleted branch claude/1566-repin-fetches-base 2026-10-06 15:59:35 +02:00
forgejo-actions commented 2026-10-06 16:00:27 +02:00

Landing identity record

  • PR: #1567
  • landed commit (server merge_commit_sha): 0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb
  • effective official approval(s):
    • @alex, review #7283, stamped commit: 0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb
  • replay comparison: no identity change (stamped SHA equals landed SHA)

This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

<!-- release-toolkit:landing-review-record-v1 landed=0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb --> ## Landing identity record - PR: #1567 - landed commit (server merge_commit_sha): `0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb` - effective official approval(s): - @alex, review #7283, stamped commit: `0e7f37d5a91573e2e960be6bb84c61d7f89ed4cb` - replay comparison: no identity change (stamped SHA equals landed SHA) This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.
Sign in to join this conversation.
Reviewers
No reviewers
alex
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
epic
 Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  
kind
feature
 New feature or capability

  
loop/paused
rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  
mirror-drift
Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  
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+

  
status
blocked
 Cannot start until a named dependency resolves

No labels
bump
major
bump
minor
bump
patch
kind
bug
kind
chore
kind
docs
kind
epic
kind
feature
loop/paused
mirror-drift
priority
critical
priority
high
priority
low
priority
medium
size
L
size
M
size
S
size
XL
status
blocked
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
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!1567
No description provided.