frankenbit/release-toolkit

Watch

Fork 0

bug(repin): rt repin bakes the rc tag from an unfetched local main, and AGENTS.md section 2.5 names a REPIN_REVIEWER env var it never reads #1566

New issue

Closed

opened 2026-10-06 12:17:45 +02:00 by claude · 1 comment

claude commented

2026-10-06 12:17:45 +02:00

Member

rt repin builds its rc tag from whatever the operator's local main holds. It never runs git fetch. The retired scripts/repin.sh fetched first and built from origin/main, and AGENTS.md §2.5 still describes the script. The same section also names a $REPIN_REVIEWER env var that rt repin does not read.

Measured

All reads are on live origin/main c2fad81, 2026-10-06.

1. No fetch, and the base defaults to the local branch.

cmd/rt/repin.go:194 defaults --base to main, the local branch. bakeCommit resolves it with git rev-parse --verify <base> (:215).
grep -n fetch cmd/rt/repin.go finds nothing.
The retired script (git show 25c2fb3:scripts/repin.sh, lines 86-90) ran git fetch "$REMOTE" and defaulted the base to ${REMOTE}/main. Its comment says "The rc must be cut from current remote main."
AGENTS.md:179 (§2.5 step 1) says the bake commit is built from "current origin/main".

2. The re-pin branch starts at the operator's HEAD, not at the base. Found while reading the script beside the port.

commitInWorktree runs git worktree add -q -b repin/<tag> <tmp> with no start point (cmd/rt/repin.go:280). The branch starts at whatever the operator has checked out.
The script built it from the base: git worktree add -q --detach "$TMPWT" "$BASE_SHA" (line 157).
So --base moves the bake but not the branch.

3. $REPIN_REVIEWER is documented but not read.

AGENTS.md:181 (§2.5 step 3) says review is requested from $REPIN_REVIEWER when it is set.
grep -rn REPIN_REVIEWER cmd internal is empty. rt repin takes a repeatable --reviewer flag (cmd/rt/repin.go:195).
The script's other env inputs became flags the same way: REPIN_REMOTE is --remote, and REPIN_BASE is --base.

4. A comment misstates the script's fallback. cmd/rt/repin.go:340-343 says "repin.sh falls back to the --base flag" when the forge cannot name the default branch. The script fell back to the literal main (lines 193-195). The two agree today only because --base defaults to main.

Reproduction against real git. The setup is a bare remote, an operator clone, and one more commit pushed to the remote's main that the clone has not fetched. runRepin ran with the command's own --base default and a faked forge. An in-test bake recorded the commit it was handed. Results on c2fad81:

Run	Bake built at	rc tag on the remote	Re-pin branch parent
`--base` default (`main`)	stale local main	stale local main	stale local main
`--base origin/main`, clone fetched first	remote main	remote main	stale local main

The second row isolates finding 2. Even a correct base leaves the branch on the operator's HEAD.

Why it matters

The rc re-pin is dormant for toolkit-self (AGENTS.md §2.2, #456). It exists for the day someone pins toolkit-self to a tag on purpose. That day comes after a long dormancy, when a local main is most likely to be stale.

On that day, rt repin would push an rc tag built from old code. The tag push is the first irreversible step, so the rc number is spent before anyone can look.

Nothing reliably catches it afterwards. This is read from AGENTS.md, not run on the forge:

build-ref-check grades whether the tag carries its own name in the marker. A stale tag does, so it passes.
check-self-bootstrap compares only the guarded compose files (§2.3). It goes red on push: main only if one of those changed between the stale base and main. A Go-only difference turns nothing red (§2.2).

This is the carry-forward loss AGENTS.md §1 describes. The fetch was load-bearing setup in the script, and the port dropped it without saying so. §2.5's dry-run caveat calls itself "the one behavioural difference the port did not preserve". This issue adds three more.

Acceptance criteria

rt repin fetches --remote before it resolves the base. If the fetch fails, it refuses before any tag, branch or PR exists.
With no --base, the bake is built from <remote>/main as just fetched, not from the local main.
The repin/<rc-tag> branch starts at the same base commit as the bake.
If the forge cannot name the default branch, the PR base falls back to main, not to origin/main.
rt repin --dry-run prints the base ref and commit it would build from.
A real-git test reproduces both rows above and fails on c2fad81. Each new guard is mutation-verified, and the PR body shows the runs.
AGENTS.md §2.5 matches the code: the fetch and the base, the branch start, and --reviewer instead of $REPIN_REVIEWER. The dry-run caveat no longer calls itself the only port difference.
The PR body decides code or doc for each finding, as a decision tree (AGENTS.md §7).
go test ./... and golangci-lint pass.

Out of scope

A test of realBake itself. That is #1563.
Making rt repin --dry-run build the bake, as the script's dry run did. §2.5 already discloses that difference.
Resolving the base from the forge's default branch instead of main. The script also hard-coded main, and cli-surface.md lists repin as toolkit-self only, whose default branch is main.

`rt repin` builds its rc tag from whatever the operator's local `main` holds. It never runs `git fetch`. The retired `scripts/repin.sh` fetched first and built from `origin/main`, and AGENTS.md §2.5 still describes the script. The same section also names a `$REPIN_REVIEWER` env var that `rt repin` does not read. ## Measured All reads are on live `origin/main` `c2fad81`, 2026-10-06. **1. No fetch, and the base defaults to the local branch.** - `cmd/rt/repin.go:194` defaults `--base` to `main`, the local branch. `bakeCommit` resolves it with `git rev-parse --verify <base>` (`:215`). - `grep -n fetch cmd/rt/repin.go` finds nothing. - The retired script (`git show 25c2fb3:scripts/repin.sh`, lines 86-90) ran `git fetch "$REMOTE"` and defaulted the base to `${REMOTE}/main`. Its comment says "The rc must be cut from current remote main." - AGENTS.md:179 (§2.5 step 1) says the bake commit is built from "current `origin/main`". **2. The re-pin branch starts at the operator's HEAD, not at the base.** Found while reading the script beside the port. - `commitInWorktree` runs `git worktree add -q -b repin/<tag> <tmp>` with no start point (`cmd/rt/repin.go:280`). The branch starts at whatever the operator has checked out. - The script built it from the base: `git worktree add -q --detach "$TMPWT" "$BASE_SHA"` (line 157). - So `--base` moves the bake but not the branch. **3. `$REPIN_REVIEWER` is documented but not read.** - AGENTS.md:181 (§2.5 step 3) says review is requested from `$REPIN_REVIEWER` when it is set. - `grep -rn REPIN_REVIEWER cmd internal` is empty. `rt repin` takes a repeatable `--reviewer` flag (`cmd/rt/repin.go:195`). - The script's other env inputs became flags the same way: `REPIN_REMOTE` is `--remote`, and `REPIN_BASE` is `--base`. **4. A comment misstates the script's fallback.** `cmd/rt/repin.go:340-343` says "repin.sh falls back to the --base flag" when the forge cannot name the default branch. The script fell back to the literal `main` (lines 193-195). The two agree today only because `--base` defaults to `main`. **Reproduction against real git.** The setup is a bare remote, an operator clone, and one more commit pushed to the remote's `main` that the clone has not fetched. `runRepin` ran with the command's own `--base` default and a faked forge. An in-test bake recorded the commit it was handed. Results on `c2fad81`: | Run | Bake built at | rc tag on the remote | Re-pin branch parent | |---|---|---|---| | `--base` default (`main`) | stale local main | stale local main | stale local main | | `--base origin/main`, clone fetched first | remote main | remote main | **stale local main** | The second row isolates finding 2. Even a correct base leaves the branch on the operator's HEAD. ## Why it matters The rc re-pin is dormant for toolkit-self (AGENTS.md §2.2, #456). It exists for the day someone pins toolkit-self to a tag on purpose. That day comes after a long dormancy, when a local `main` is most likely to be stale. On that day, `rt repin` would push an rc tag built from old code. The tag push is the first irreversible step, so the rc number is spent before anyone can look. Nothing reliably catches it afterwards. This is read from AGENTS.md, not run on the forge: - `build-ref-check` grades whether the tag carries its own name in the marker. A stale tag does, so it passes. - `check-self-bootstrap` compares only the guarded compose files (§2.3). It goes red on `push: main` only if one of those changed between the stale base and main. A Go-only difference turns nothing red (§2.2). This is the carry-forward loss AGENTS.md §1 describes. The fetch was load-bearing setup in the script, and the port dropped it without saying so. §2.5's dry-run caveat calls itself "the one behavioural difference the port did not preserve". This issue adds three more. ## Acceptance criteria - [x] `rt repin` fetches `--remote` before it resolves the base. If the fetch fails, it refuses before any tag, branch or PR exists. - [x] With no `--base`, the bake is built from `<remote>/main` as just fetched, not from the local `main`. - [x] The `repin/<rc-tag>` branch starts at the same base commit as the bake. - [x] If the forge cannot name the default branch, the PR base falls back to `main`, not to `origin/main`. - [x] `rt repin --dry-run` prints the base ref and commit it would build from. - [x] A real-git test reproduces both rows above and fails on `c2fad81`. Each new guard is mutation-verified, and the PR body shows the runs. - [x] AGENTS.md §2.5 matches the code: the fetch and the base, the branch start, and `--reviewer` instead of `$REPIN_REVIEWER`. The dry-run caveat no longer calls itself the only port difference. - [x] The PR body decides code or doc for each finding, as a decision tree (AGENTS.md §7). - [x] `go test ./...` and `golangci-lint` pass. ## Out of scope - A test of `realBake` itself. That is #1563. - Making `rt repin --dry-run` build the bake, as the script's dry run did. §2.5 already discloses that difference. - Resolving the base from the forge's default branch instead of `main`. The script also hard-coded `main`, and `cli-surface.md` lists `repin` as toolkit-self only, whose default branch is `main`.

claude added the

labels

2026-10-06 12:17:45 +02:00

claude referenced this issue from a commit

2026-10-06 12:26:23 +02:00

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

claude referenced this issue from a commit

2026-10-06 12:26:23 +02:00

docs(agents): section 2.5 describes rt repin's fetch, base and --reviewer flag (#1566)

claude referenced this issue

2026-10-06 12:27:55 +02:00

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

claude referenced this issue from a commit

2026-10-06 15:14:57 +02:00

fix(repin): grade the wrappers at the base, before the dry run returns and before the tag push (#1566)

release-bot referenced this issue

2026-10-06 16:00:53 +02:00

chore(release): v0.68.0 #1559

quartermaster commented

2026-10-19 08:44:09 +02:00

Owner

Already fixed and left open — verified against `origin/main` `f87c0d3`

Timestamp corrected: this comment first read "2026-10-19 00:40". The reads were taken then, but the comment posted at 08:44 and I stamped it from a stale clock. origin/main is still f87c0d3 at 08:44, so every row below holds — which is the argument for anchoring on the SHA rather than the hour.

b246ab5 (PR #1567, merged 2026-10-06T15:59) landed all four findings. Re-derived here per AC
rather than taken from the PR body, since the question is whether main holds the behaviour now:

AC	Evidence at `f87c0d3`
fetch before resolving the base; refuse before any tag/branch/PR	`resolveBase`, `cmd/rt/repin.go:217` — `git fetch --quiet <remote>`, error returns before anything is written
no `--base` builds from `<remote>/main`	`:198` flag default `""`, documented and resolved as `<remote>/main`
the `repin/<rc-tag>` branch starts at the bake's base	`:345` `git worktree add -q -b <branch> <tmp> <baseSHA>`, with `:342` "Start the branch at the base the bake was built from, as repin.sh did"
PR base falls back to `main`, not `origin/main`	`:410-416` — `strings.TrimPrefix(baseRef, o.remote+"/")`, with the reason recorded: a PR cannot target `origin/main`, and refusing would let a transient forge outage block a break-glass
`--dry-run` prints the base ref and commit	`:383` `[repin] base %s = %s` — emitted before the dry-run return at `:401`, so the dry run gets it
real-git test reproducing both rows	`cmd/rt/repin_base_test.go` — `newStaleRepo` builds a bare remote plus an unfetched clone; `TestRunRepin_BuildsFromTheFetchedRemoteMain`, `TestRunRepin_RefusesWhenTheFetchFails`, `TestRunRepin_PlansTheBasesWrappers`, `TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag`
AGENTS.md §2.5 matches the code	step 1 names the fetch and the refusal; step 2 says "from the same base commit"; step 3 states `rt repin` reads none of `$REPIN_REVIEWER`/`$REPIN_REMOTE`/`$REPIN_BASE` and takes flags instead
the dry-run caveat no longer calls itself the only port difference	§2.5's caveat now reads "This caveat used to call itself the one behavioural difference the port did not preserve. It was not." and enumerates the three
PR body decides code-or-doc per finding	PR #1567, "## Decision: code or doc, per finding (AGENTS.md §7)"
`go test ./...` and `golangci-lint`	re-run here on `f87c0d3`: `go test ./...` rc=0, all packages ok; `golangci-lint run ./...` 0 issues, rc=0

Finding 4 — the comment claiming "repin.sh falls back to the --base flag" — is gone with the
rewrite; :412-415 now states the script's actual fallback and why the port keeps it.

Closing. Nothing was left to implement; the ticket outlived its fix by thirteen days.

## Already fixed and left open — verified against `origin/main` `f87c0d3` *Timestamp corrected: this comment first read "2026-10-19 00:40". The reads were taken then, but the comment posted at 08:44 and I stamped it from a stale clock. `origin/main` is still `f87c0d3` at 08:44, so every row below holds — which is the argument for anchoring on the SHA rather than the hour.* `b246ab5` (PR #1567, merged 2026-10-06T15:59) landed all four findings. Re-derived here per AC rather than taken from the PR body, since the question is whether main holds the behaviour now: | AC | Evidence at `f87c0d3` | |---|---| | fetch before resolving the base; refuse before any tag/branch/PR | `resolveBase`, `cmd/rt/repin.go:217` — `git fetch --quiet <remote>`, error returns before anything is written | | no `--base` builds from `<remote>/main` | `:198` flag default `""`, documented and resolved as `<remote>/main` | | the `repin/<rc-tag>` branch starts at the bake's base | `:345` `git worktree add -q -b <branch> <tmp> <baseSHA>`, with `:342` *"Start the branch at the base the bake was built from, as repin.sh did"* | | PR base falls back to `main`, not `origin/main` | `:410-416` — `strings.TrimPrefix(baseRef, o.remote+"/")`, with the reason recorded: a PR cannot target `origin/main`, and refusing would let a transient forge outage block a break-glass | | `--dry-run` prints the base ref and commit | `:383` `[repin] base %s = %s` — emitted *before* the dry-run return at `:401`, so the dry run gets it | | real-git test reproducing both rows | `cmd/rt/repin_base_test.go` — `newStaleRepo` builds a bare remote plus an unfetched clone; `TestRunRepin_BuildsFromTheFetchedRemoteMain`, `TestRunRepin_RefusesWhenTheFetchFails`, `TestRunRepin_PlansTheBasesWrappers`, `TestRunRepin_UngradeableBaseWrapperRefusesBeforeTheTag` | | AGENTS.md §2.5 matches the code | step 1 names the fetch and the refusal; step 2 says *"from the same base commit"*; step 3 states `rt repin` reads none of `$REPIN_REVIEWER`/`$REPIN_REMOTE`/`$REPIN_BASE` and takes flags instead | | the dry-run caveat no longer calls itself the only port difference | §2.5's caveat now reads *"This caveat used to call itself the one behavioural difference the port did not preserve. It was not."* and enumerates the three | | PR body decides code-or-doc per finding | PR #1567, *"## Decision: code or doc, per finding (AGENTS.md §7)"* | | `go test ./...` and `golangci-lint` | re-run here on `f87c0d3`: `go test ./...` **rc=0**, all packages ok; `golangci-lint run ./...` **0 issues, rc=0** | Finding 4 — the comment claiming *"repin.sh falls back to the --base flag"* — is gone with the rewrite; `:412-415` now states the script's actual fallback and why the port keeps it. Closing. Nothing was left to implement; the ticket outlived its fix by thirteen days.

quartermaster closed this issue

2026-10-19 08:44:10 +02:00