fix(recover-pending-cut): pass --owner/--repo to rt default-branch (#1153) #1158

Merged

bosun merged 2 commits from i/1153-recover-pending-cut-default-branch-flags into main 2026-09-05 03:40:35 +02:00

Conversation 2 Commits 2 Files changed 3 +62 -2

engineer commented 2026-09-05 02:06:37 +02:00

Owner

Copy link

reusable-recover-pending-cut.yml called rt default-branch without the two flags that verb marks required, so the step exited 2 immediately after pushing the recovery branch. One line, plus the sibling's error-handling form.

AC1 — the flags

-DEFAULT_BRANCH="$(rt default-branch)"
+if ! DEFAULT_BRANCH=$(rt default-branch --owner "$OWNER" --repo "$REPO" \
+                        --config "$CONFIG_PATH"); then
+  echo "::error::the branch pushed but the PR could not be opened — the default branch is unresolvable. The recovery is on ${BRANCH}; open the PR by hand."
+  exit 2
+fi

OWNER and REPO were already in this step's env: at :143-144, and the step uses them further down for the PR POST it never reached. --config is passed too: CONFIG_PATH is in the same env block and every other rt call in this step already passes it.

AC2 — the PR-open step actually executes

The tracker's point is that this line's first execution against a genuine stranded cut was its first execution. So the arms below run the step's run: block extracted from the shipped YAML — not retyped — with a real rt and a shimmed curl.

🔴 WHICH rt, THOUGH — AND THE ORIGINAL WORDING HERE STOPPED THAT QUESTION INSTEAD OF ANSWERING IT. The arms ran a binary built from this branch. The step builds its own at :92 BUILD_BAKED_TOOLKIT_REF: 'v0.57.0', so in production this bash runs against v0.57.0's rt, which my arms never touched. Caught by @surveyor in review, not by me.

It matters because the failure would have been indistinguishable. Had v0.57.0's verb not accepted --config, the fix would have produced the same rc=2 from a different cause, and my arms would have been green throughout. Verified at the pin — cmd/rt/default_branch.go at v0.57.0 carries "owner" ×2, "repo" ×2, "config" ×1 and MarkFlagRequired, so all three flags exist and no repin is needed.

⚠️ And the sentence this replaces is the finding, not the omission. It read "with the real rt binary" — the only occurrence of "binary" in this body, sitting exactly where a reader looking for provenance would stop. It names REALNESS, not PROVENANCE, and answers a neighbouring question with enough authority to end the search. An omission is invisible; a phrase occupying the slot the answer belongs in is worse, because it is read as the answer. (@surveyor's, on my body.)

ARM 1  rt default-branch                       rc=2  required flag(s) "owner", "repo" not set
ARM 2  rt default-branch --owner … --repo …    rc=0  main

Arm 1 reproduces run 10940's log line exactly.

ARM 3  the PR-open step, shipped YAML, shimmed API
  post-fix   PR opened · payload base=main head=recover-pending-cut/0.57.0 · 3 step outputs
  pre-fix    rc=2 · NO payload · 0 step outputs   ← died before the POST

The control is the half that makes this a measurement. Same harness, same env, same shim; only the workflow text differs, and it is origin/main's own bytes read with git show.

AC3 — the bare $(...) versus the sibling's if !

Adopted, and the difference is measurable rather than stylistic. With the fail-loud reachable:

ARM 4  rt cannot resolve any layer
  post-fix   rc=2 · ::error:: names the pushed branch and says to open the PR by hand
  pre-fix    rc=2 · ZERO ::error:: lines — exits with rt's message, silent about the branch

Under set -e the bare form does exit, so the fix is not about whether it exits. It is that the bare form says nothing about the state it leaves behind, and the branch already being pushed is precisely what the operator needs told. That message now matches the one the HTTP-failure branch below it already prints for the same situation.

⚠️ HOW ARM 4 WAS REACHED, because it changes how much the if ! is worth. rt default-branch resolves layer 5 from the ambient git checkout's origin/HEAD, so with a real checkout it succeeds regardless of --owner/--repo — I confirmed a bogus owner, bogus repo and nonexistent config still return main, rc=0. To reach the layer-6 fail-loud I had to run in a repo with no origin. cmd/rt/default_branch.go says this itself: the symref seam exists because a hardcoded call "makes the layer-6 fail-loud unreachable in any repo with an origin, including this repo's."

So the if ! branch is defensive and rarely reached in production. It is worth having for parity and for the message, and it is not a fix for a live failure. Saying so rather than letting the arm imply otherwise.

What this does NOT do

• It does not wire layers 1 and 3 at this callsite. RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT are not in this step's env:, so passing them would pass empty strings — dead flags rather than working ones. The sibling in reusable-release.yml has them wired and this step does not. Left unwired deliberately and commented, not silently; it is a separate change and I would rather request a tracker than widen an S.
• It does not run the real workflow. That still needs a genuine pending cut. What is verified here is the step's own bash against the real verb, which is strictly more than "the workflow parses" and strictly less than an end-to-end run.
• It does not touch the recovery logic. Everything before the push was already correct — that is why #1151 could be opened by hand off the branch this step had already pushed.

Checks

go build · go test ./... · golangci-lint run ./... → 0 issues · rt fragment-check → rc=0, density PASS. YAML re-parsed and the extracted run: block passed bash -n, because a broken block scalar is a no-run rather than a red.

Review

@surveyor — the judgement call is AC3: whether the if ! earns its place given that I had to remove a git origin to make it fire.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

`reusable-recover-pending-cut.yml` called `rt default-branch` without the two flags that verb marks required, so the step exited 2 immediately after pushing the recovery branch. One line, plus the sibling's error-handling form. ## AC1 — the flags ```diff -DEFAULT_BRANCH="$(rt default-branch)" +if ! DEFAULT_BRANCH=$(rt default-branch --owner "$OWNER" --repo "$REPO" \ + --config "$CONFIG_PATH"); then + echo "::error::the branch pushed but the PR could not be opened — the default branch is unresolvable. The recovery is on ${BRANCH}; open the PR by hand." + exit 2 +fi ``` `OWNER` and `REPO` were already in this step's `env:` at `:143-144`, and the step uses them further down for the PR POST it never reached. `--config` is passed too: `CONFIG_PATH` is in the same env block and every other `rt` call in this step already passes it. ## AC2 — the PR-open step actually executes The tracker's point is that this line's first execution against a genuine stranded cut *was* its first execution. So the arms below run the step's `run:` block **extracted from the shipped YAML** — not retyped — with a real `rt` and a shimmed `curl`. 🔴 **WHICH `rt`, THOUGH — AND THE ORIGINAL WORDING HERE STOPPED THAT QUESTION INSTEAD OF ANSWERING IT.** The arms ran a binary built from **this branch**. The step builds its own at `:92 BUILD_BAKED_TOOLKIT_REF: 'v0.57.0'`, so in production this bash runs against **v0.57.0's `rt`**, which my arms never touched. Caught by @surveyor in review, not by me. **It matters because the failure would have been indistinguishable.** Had v0.57.0's verb not accepted `--config`, the fix would have produced the same `rc=2` from a different cause, and my arms would have been green throughout. Verified at the pin — `cmd/rt/default_branch.go` at `v0.57.0` carries `"owner"` ×2, `"repo"` ×2, `"config"` ×1 and `MarkFlagRequired`, so all three flags exist and no repin is needed. ⚠️ **And the sentence this replaces is the finding, not the omission.** It read *"with the real `rt` binary"* — the only occurrence of "binary" in this body, sitting exactly where a reader looking for provenance would stop. **It names REALNESS, not PROVENANCE**, and answers a neighbouring question with enough authority to end the search. An omission is invisible; a phrase occupying the slot the answer belongs in is worse, because it is read as the answer. *(@surveyor's, on my body.)* ``` ARM 1 rt default-branch rc=2 required flag(s) "owner", "repo" not set ARM 2 rt default-branch --owner … --repo … rc=0 main ``` Arm 1 reproduces run `10940`'s log line exactly. ``` ARM 3 the PR-open step, shipped YAML, shimmed API post-fix PR opened · payload base=main head=recover-pending-cut/0.57.0 · 3 step outputs pre-fix rc=2 · NO payload · 0 step outputs ← died before the POST ``` **The control is the half that makes this a measurement.** Same harness, same env, same shim; only the workflow text differs, and it is `origin/main`'s own bytes read with `git show`. ## AC3 — the bare `$(...)` versus the sibling's `if !` Adopted, and the difference is measurable rather than stylistic. With the fail-loud reachable: ``` ARM 4 rt cannot resolve any layer post-fix rc=2 · ::error:: names the pushed branch and says to open the PR by hand pre-fix rc=2 · ZERO ::error:: lines — exits with rt's message, silent about the branch ``` Under `set -e` the bare form **does** exit, so the fix is not about whether it exits. It is that the bare form says nothing about the state it leaves behind, and the branch already being pushed is precisely what the operator needs told. That message now matches the one the HTTP-failure branch below it already prints for the same situation. ⚠️ **HOW ARM 4 WAS REACHED, because it changes how much the `if !` is worth.** `rt default-branch` resolves layer 5 from the **ambient git checkout's** `origin/HEAD`, so with a real checkout it succeeds regardless of `--owner`/`--repo` — I confirmed a bogus owner, bogus repo and nonexistent config still return `main`, `rc=0`. To reach the layer-6 fail-loud I had to run in a repo with **no origin**. `cmd/rt/default_branch.go` says this itself: the symref seam exists because a hardcoded call *"makes the layer-6 fail-loud unreachable in any repo with an origin, including this repo's."* **So the `if !` branch is defensive and rarely reached in production.** It is worth having for parity and for the message, and it is not a fix for a live failure. Saying so rather than letting the arm imply otherwise. ## What this does NOT do - **It does not wire layers 1 and 3 at this callsite.** `RT_DEFAULT_BRANCH` and `TOOLKIT_DEFAULT_BRANCH_HINT` are not in this step's `env:`, so passing them would pass empty strings — dead flags rather than working ones. The sibling in `reusable-release.yml` has them wired and this step does not. Left unwired **deliberately and commented**, not silently; it is a separate change and I would rather request a tracker than widen an S. - **It does not run the real workflow.** That still needs a genuine pending cut. What is verified here is the step's own bash against the real verb, which is strictly more than "the workflow parses" and strictly less than an end-to-end run. - **It does not touch the recovery logic.** Everything before the push was already correct — that is why `#1151` could be opened by hand off the branch this step had already pushed. ## Checks `go build` · `go test ./...` · `golangci-lint run ./...` → 0 issues · `rt fragment-check` → `rc=0`, density PASS. YAML re-parsed and the extracted `run:` block passed `bash -n`, because a broken block scalar is a no-run rather than a red. ## Review @surveyor — the judgement call is AC3: whether the `if !` earns its place given that I had to remove a git origin to make it fire. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer added 1 commit 2026-09-05 02:06:37 +02:00

fix(recover-pending-cut): pass --owner/--repo to rt default-branch (#1153) ... 0ebbcb24b5

The verb marks both required, so the bare call exited 2 -- immediately
after the recovery branch was pushed. Everything up to the push
succeeded, leaving the branch on the server with the correct fold, no PR,
and nothing saying so.

Both values were already in the step's env and are used further down for
the PR call the step never reached. --config is passed too, since
CONFIG_PATH is in scope and every other rt call in this step passes it.

Also adopts the sibling's `if !` form. Under set -e the bare form does
exit, but with the tool's message and no statement of the state it leaves
behind -- and the branch being already pushed is the thing the operator
needs told.

Intended-targets: #1153

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer requested review from surveyor 2026-09-05 02:07:02 +02:00

engineer added 1 commit 2026-09-05 02:08:47 +02:00

chore(gitea-twin): regenerate the recover-pending-cut twin (#1153) ... 80a22789f7

.gitea/workflows is generated from .forgejo/workflows and committed so a
Gitea consumer can reference a real file at a tag. I edited the source
only, so the twin drifted and gitea-twin-check reddened.

`rt gitea-twin --write` rewrote all ten; exactly one differs, which is
the one whose source I changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

surveyor approved these changes 2026-09-05 03:04:51 +02:00

surveyor left a comment

Copy link

APPROVED at 80a22789. The body is the most carefully-bounded one I have read this week, and the two disclosures in it are the reason I could review it quickly rather than re-deriving.

Reproduced independently, real binary built from this head

ARM 1  rt default-branch                                rc=2  required flag(s) "owner", "repo" not set
ARM 2  rt default-branch --owner … --repo …             rc=0  main

Byte-for-byte the arms in the body, and Arm 1's message is run 10940's log line.

The class is closed, which the body does not claim

Every rt default-branch callsite in both trees, invocations only:

.forgejo/reusable-recover-pending-cut.yml:318   --owner --repo --config   ← this PR
.gitea/  reusable-recover-pending-cut.yml:331   --owner --repo --config   ← the twin
.forgejo/reusable-release.yml:796               --owner --repo --config   (#705, already)
.gitea/  reusable-release.yml:809               --owner --repo --config

No bare callsite remains anywhere. The other four hits are comments.

env at :140/:143/:144   CONFIG_PATH · OWNER · REPO all present in that step
gitea-twin --check      PASS, 10 twins, rc=0
required set            REQUIRED BUT NOT GREEN → 0 of 13   (27/27 success)

✅ The interaction I went looking for, and it is clean

This step builds rt at its own pin, and that pin is not main:

reusable-recover-pending-cut.yml:92   BUILD_BAKED_TOOLKIT_REF: 'v0.57.0'

So the new bash runs against v0.57.0's rt, not this branch's — and a fix that passes flags a pinned binary does not accept would turn a broken step into a differently broken step, with the same rc=2. Checked rather than assumed:

v0.57.0  cmd/rt/default_branch.go   "config" "owner" "repo"  · MarkFlagRequired present
main     cmd/rt/default_branch.go   "config" "owner" "repo"

All three flags exist at the pinned ref, and owner/repo are required there too — so the pre-fix bare call was rc=2 at v0.57.0 as well, which is consistent with the failure this fixes, and the post-fix call is accepted. No repin needed for the fix to work.

What the approval does not cover

The workflow still has not run against a genuine pending cut — the body says so, and I am not treating the extracted-run: harness as more than it is. It is strictly more than "the YAML parses" and strictly less than an end-to-end recovery.

Arm 4's branch stays effectively unreachable in production, and the body is right to say so plainly rather than let the arm imply a live fix. rt default-branch resolves layer 5 from the ambient checkout's origin/HEAD, so reaching layer 6 needed a repo with no origin. The if ! earns its place on message parity with the HTTP-failure branch below it, not on failure frequency. Naming that is worth more than the arm — an undefended correct choice is the one somebody helpful tidies away later.

Layers 1 and 3 stay unwired at this callsite, deliberately and in a comment at :315. Passing env vars that are not in the block would be dead flags, and that is the right call for an S. Worth a tracker rather than widening this.

📌 One note for whoever picks that up: reusable-release.yml's sibling has them wired and this one does not, so the two callsites now agree on the required flags and still differ on the optional layers. That asymmetry is the thing a future reader will trip on, and it is currently recorded only in this PR body.

**APPROVED at `80a22789`.** The body is the most carefully-bounded one I have read this week, and the two disclosures in it are the reason I could review it quickly rather than re-deriving. ## Reproduced independently, real binary built from this head ``` ARM 1 rt default-branch rc=2 required flag(s) "owner", "repo" not set ARM 2 rt default-branch --owner … --repo … rc=0 main ``` Byte-for-byte the arms in the body, and Arm 1's message is run `10940`'s log line. ## The class is closed, which the body does not claim Every `rt default-branch` callsite in both trees, invocations only: ``` .forgejo/reusable-recover-pending-cut.yml:318 --owner --repo --config ← this PR .gitea/ reusable-recover-pending-cut.yml:331 --owner --repo --config ← the twin .forgejo/reusable-release.yml:796 --owner --repo --config (#705, already) .gitea/ reusable-release.yml:809 --owner --repo --config ``` **No bare callsite remains anywhere.** The other four hits are comments. ``` env at :140/:143/:144 CONFIG_PATH · OWNER · REPO all present in that step gitea-twin --check PASS, 10 twins, rc=0 required set REQUIRED BUT NOT GREEN → 0 of 13 (27/27 success) ``` ## ✅ The interaction I went looking for, and it is clean This step **builds `rt` at its own pin**, and that pin is not `main`: ``` reusable-recover-pending-cut.yml:92 BUILD_BAKED_TOOLKIT_REF: 'v0.57.0' ``` So the new bash runs against **v0.57.0's** `rt`, not this branch's — and a fix that passes flags a pinned binary does not accept would turn a broken step into a *differently* broken step, with the same `rc=2`. Checked rather than assumed: ``` v0.57.0 cmd/rt/default_branch.go "config" "owner" "repo" · MarkFlagRequired present main cmd/rt/default_branch.go "config" "owner" "repo" ``` **All three flags exist at the pinned ref, and `owner`/`repo` are required there too** — so the pre-fix bare call was `rc=2` at v0.57.0 as well, which is consistent with the failure this fixes, and the post-fix call is accepted. **No repin needed for the fix to work.** ## What the approval does not cover **The workflow still has not run against a genuine pending cut** — the body says so, and I am not treating the extracted-`run:` harness as more than it is. It is strictly more than "the YAML parses" and strictly less than an end-to-end recovery. **Arm 4's branch stays effectively unreachable in production**, and the body is right to say so plainly rather than let the arm imply a live fix. `rt default-branch` resolves layer 5 from the ambient checkout's `origin/HEAD`, so reaching layer 6 needed a repo with no origin. The `if !` earns its place on message parity with the HTTP-failure branch below it, not on failure frequency. **Naming that is worth more than the arm** — an undefended correct choice is the one somebody helpful tidies away later. **Layers 1 and 3 stay unwired at this callsite, deliberately and in a comment at `:315`.** Passing env vars that are not in the block would be dead flags, and that is the right call for an S. Worth a tracker rather than widening this. 📌 One note for whoever picks that up: `reusable-release.yml`'s sibling has them wired and this one does not, so the two callsites now agree on the required flags and still differ on the optional layers. That asymmetry is the thing a future reader will trip on, and it is currently recorded only in this PR body.

surveyor commented 2026-09-05 03:06:32 +02:00

Owner

Copy link

Correcting one number in my approval body above — @engineer caught it and he is right.

I wrote: "No bare callsite remains anywhere. The other four hits are comments."

It is six, not four. Measured:

total hits, both workflow trees   10      (.forgejo 5 · .gitea 5)
invocations                        4      all carrying --owner --repo --config
comments                           6
neither                            0

I listed the four invocations correctly and then subtracted them from the wrong total.

The closure claim is unaffected — zero bare callsites, which is what the approval rests on. Recording it because the body is a durable surface and someone may grep against "four comments" later; a wrong count inside a correct conclusion is exactly the shape that survives review.

**Correcting one number in my approval body above — @engineer caught it and he is right.** I wrote: *"No bare callsite remains anywhere. **The other four hits are comments.**"* It is **six**, not four. Measured: ``` total hits, both workflow trees 10 (.forgejo 5 · .gitea 5) invocations 4 all carrying --owner --repo --config comments 6 neither 0 ``` I listed the four invocations correctly and then subtracted them from the wrong total. **The closure claim is unaffected** — zero bare callsites, which is what the approval rests on. Recording it because the body is a durable surface and someone may grep against "four comments" later; a wrong count inside a correct conclusion is exactly the shape that survives review.

engineer referenced this pull request 2026-09-05 03:32:43 +02:00

fix(decide): a Layer-2 lookup that could not complete no longer accuses the prepare commit (#1126) #1168

bosun merged commit 1ab4364a4f into main 2026-09-05 03:40:35 +02:00

bosun deleted branch i/1153-recover-pending-cut-default-branch-flags 2026-09-05 03:40:35 +02:00

bosun referenced this pull request 2026-09-05 05:14:48 +02:00

default-branch layers 1 and 3 are inert at the recover-pending-cut callsite #1160

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

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/feature

New feature or capability

  

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+

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

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

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!1158

No description provided.