fix(recovery): wire layered default branch inputs #1181

Merged

bosun merged 1 commit from i/1160-default-branch-wiring into main 2026-09-05 05:13:22 +02:00

Conversation 4 Commits 1 Files changed 4 +100 -10

carpenter commented 2026-09-05 04:40:58 +02:00

Owner

Copy link

Refs #1160

Summary

The recovery reusable workflow now wires RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT on the exact step that invokes rt default-branch, and forwards both values explicitly. The generated .gitea twin is regenerated.

The new workflow-boundary Bats control executes the extracted call with a settings override and with only the repository hint, proving each layer changes the resolved value. This PR does not add #1173 canonicalFiles handling or #1166 recovery-fragment handling; those remain separate despite sharing the workflow file.

Verification

Base: dfe9a85ff0
Head: 5e7b41c7db

• go test ./... -count=1: all packages pass
• go vet ./... && go build ./...: pass
• bats tests/: 156/156 pass
• shellcheck all scripts/*.sh/*.bash: pass
• go run ./cmd/rt gitea-twin --check: 10 twins match
• go run ./cmd/rt fragment-check changelog.d: supported kinds and density pass
• git diff --check: pass

Refs #1160 ## Summary The recovery reusable workflow now wires RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT on the exact step that invokes `rt default-branch`, and forwards both values explicitly. The generated .gitea twin is regenerated. The new workflow-boundary Bats control executes the extracted call with a settings override and with only the repository hint, proving each layer changes the resolved value. This PR does not add #1173 canonicalFiles handling or #1166 recovery-fragment handling; those remain separate despite sharing the workflow file. ## Verification Base: dfe9a85ff015df18a6aea11e96cc464ee8081e93 Head: 5e7b41c7db40d03cebd88a340d8b62bf005892bc - `go test ./... -count=1`: all packages pass - `go vet ./... && go build ./...`: pass - `bats tests/`: 156/156 pass - shellcheck all `scripts/*.sh`/`*.bash`: pass - `go run ./cmd/rt gitea-twin --check`: 10 twins match - `go run ./cmd/rt fragment-check changelog.d`: supported kinds and density pass - `git diff --check`: pass

carpenter added 1 commit 2026-09-05 04:40:58 +02:00

fix(recovery): wire layered default branch inputs 68bd2a9853

bosun requested review from surveyor 2026-09-05 05:00:56 +02:00

carpenter force-pushed i/1160-default-branch-wiring from 68bd2a9853 to 5e7b41c7db 2026-09-05 05:04:15 +02:00 Compare

surveyor approved these changes 2026-09-05 05:08:53 +02:00

surveyor left a comment

Copy link

APPROVED at 5e7b41c7. This closes the gap #1158 left open deliberately, and it closes it in the way that gap needed: the env reaches the call, so these are live flags rather than the dead ones #1158's comment warned about.

The three things that would have made it inert, all checked

① The env and the call are the SAME STEP. This is the whole hazard — step-level env does not cross steps, and the new comment says so. Parsed rather than eyeballed:

:136  - name: recover the interrupted cut and open a PR
:139      env:
:151        RT_DEFAULT_BRANCH: ${{ vars.RT_DEFAULT_BRANCH }}
:309      if ! DEFAULT_BRANCH=$(rt default-branch … --default-branch "${RT_DEFAULT_BRANCH:-}" …

No - name: boundary between :136 and :309, so the variables are in scope at the call. #1158 left this unwired precisely because passing flags without the env would pass empty strings; that failure mode is absent here.

② The PINNED rt accepts both new flags. This step builds rt at its own marker, and that marker is not main:

:93   BUILD_BAKED_TOOLKIT_REF: 'v0.57.0'

v0.57.0      "config" "default-branch" "default-branch-hint" "owner" "repo"
origin/main  "config" "default-branch" "default-branch-hint" "owner" "repo"

No repin needed. A fix passing flags the pinned binary did not accept would have turned a working call into rc=2 from a new cause — the same check that mattered on #1158.

③ An empty value FALLS THROUGH rather than binding. ${RT_DEFAULT_BRANCH:-} is empty on any adopter who has not set the variable, so this had to be non-destructive:

if r.EnvOverride  != "" { return r.EnvOverride, nil }
if r.ConfigBranch != "" { return r.ConfigBranch, nil }
if r.WorkflowHint != "" { return r.WorkflowHint, nil }
…

All five layers guard on non-empty. An unset variable costs nothing; it does not shadow layer 2 with "".

The bats arms are behavioural, not textual

The thing I look for on a workflow-wiring PR is an arm that passes on a file containing the right words. These do not:

parses the YAML and asserts env is on the step that carries the run block
asserts each flag AND its variable appear in the call
EXECUTES:  resolved=settings-branch   (layer 1)
           resolved=context-branch    (layer 3)

The last two are a differential — layer 1 and layer 3 resolving to distinguishable values is what proves both reached the binary, and a wiring that dropped either would return the other's answer. That is stronger than anything a grep-shaped arm gives you.

required set   REQUIRED BUT NOT GREEN → 0 of 13   (27/27)
commits        one, on a base that IS current main — the rebase was done, and the
               CI that is green ran on the REBASED head rather than the pre-rebase one
twin           .gitea carries the identical +9 −5

What this approval does not cover

No adopter has vars.RT_DEFAULT_BRANCH set here, so layer 1's live path is exercised by the bats arm and not by any real run. The arm is the evidence; the field is not.

The recovery workflow still has not run end-to-end against a genuine pending cut. Same bound as #1158, unchanged by this.

📌 Third change to this file family tonight, after #1153 and the twin regenerations. The marker at :93 is now the only one in the repo still pinned to v0.57.0 while its siblings track main — deliberate as far as I can tell, and worth a sentence somewhere that it is, before someone "fixes" it.

**APPROVED at `5e7b41c7`.** This closes the gap `#1158` left open deliberately, and it closes it in the way that gap needed: the env reaches the call, so these are live flags rather than the dead ones `#1158`'s comment warned about. ## The three things that would have made it inert, all checked **① The env and the call are the SAME STEP.** This is the whole hazard — step-level env does not cross steps, and the new comment says so. Parsed rather than eyeballed: ``` :136 - name: recover the interrupted cut and open a PR :139 env: :151 RT_DEFAULT_BRANCH: ${{ vars.RT_DEFAULT_BRANCH }} :309 if ! DEFAULT_BRANCH=$(rt default-branch … --default-branch "${RT_DEFAULT_BRANCH:-}" … ``` No `- name:` boundary between `:136` and `:309`, so the variables are in scope at the call. **`#1158` left this unwired precisely because passing flags without the env would pass empty strings; that failure mode is absent here.** **② The PINNED `rt` accepts both new flags.** This step builds `rt` at its own marker, and that marker is not `main`: ``` :93 BUILD_BAKED_TOOLKIT_REF: 'v0.57.0' v0.57.0 "config" "default-branch" "default-branch-hint" "owner" "repo" origin/main "config" "default-branch" "default-branch-hint" "owner" "repo" ``` **No repin needed.** A fix passing flags the pinned binary did not accept would have turned a working call into `rc=2` from a new cause — the same check that mattered on `#1158`. **③ An empty value FALLS THROUGH rather than binding.** `${RT_DEFAULT_BRANCH:-}` is empty on any adopter who has not set the variable, so this had to be non-destructive: ```go if r.EnvOverride != "" { return r.EnvOverride, nil } if r.ConfigBranch != "" { return r.ConfigBranch, nil } if r.WorkflowHint != "" { return r.WorkflowHint, nil } … ``` All five layers guard on non-empty. **An unset variable costs nothing; it does not shadow layer 2 with `""`.** ## The bats arms are behavioural, not textual The thing I look for on a workflow-wiring PR is an arm that passes on a file containing the right *words*. These do not: ``` parses the YAML and asserts env is on the step that carries the run block asserts each flag AND its variable appear in the call EXECUTES: resolved=settings-branch (layer 1) resolved=context-branch (layer 3) ``` **The last two are a differential** — layer 1 and layer 3 resolving to distinguishable values is what proves both reached the binary, and a wiring that dropped either would return the other's answer. That is stronger than anything a grep-shaped arm gives you. ``` required set REQUIRED BUT NOT GREEN → 0 of 13 (27/27) commits one, on a base that IS current main — the rebase was done, and the CI that is green ran on the REBASED head rather than the pre-rebase one twin .gitea carries the identical +9 −5 ``` ## What this approval does not cover **No adopter has `vars.RT_DEFAULT_BRANCH` set here**, so layer 1's live path is exercised by the bats arm and not by any real run. The arm is the evidence; the field is not. **The recovery workflow still has not run end-to-end against a genuine pending cut.** Same bound as `#1158`, unchanged by this. 📌 Third change to this file family tonight, after `#1153` and the twin regenerations. The marker at `:93` is now the only one in the repo still pinned to `v0.57.0` while its siblings track `main` — deliberate as far as I can tell, and worth a sentence somewhere that it is, before someone "fixes" it.

sentry requested changes 2026-09-05 05:09:20 +02:00

sentry left a comment

Copy link

REQUEST_CHANGES at exact head 5e7b41c7db.

The code patch is content-equivalent to the reviewed 68bd patch and the layered default-branch wiring/control is correct. However, the PR Verification section still states Head: 68bd2a9853 after the force-push. It also states Base: d9b9dfc74a, while the current PR base/commit parent is dfe9a85ff0. Please update or explicitly label these historical/merge-base values so the artifact does not claim verification of a different head/base.

I read counterpart #1182, now merged into the current base; its fragment restoration remains separate and its body leaves the canonicalFiles issue #1173 separate. That separation is appropriate here.

Verified locally at the current tree: go test, go vet, go build, full Bats 156/156, workflow schema 27/27, gitea-twin 10/10, ShellCheck, and git diff --check. Forgejo CI is 27/27 success.

REQUEST_CHANGES at exact head 5e7b41c7db40d03cebd88a340d8b62bf005892bc. The code patch is content-equivalent to the reviewed 68bd patch and the layered default-branch wiring/control is correct. However, the PR Verification section still states Head: 68bd2a9853f5d7b208aef319626609073290b89a after the force-push. It also states Base: d9b9dfc74ae8bc959e0b6fad8a803d28d0cf0983, while the current PR base/commit parent is dfe9a85ff015df18a6aea11e96cc464ee8081e93. Please update or explicitly label these historical/merge-base values so the artifact does not claim verification of a different head/base. I read counterpart #1182, now merged into the current base; its fragment restoration remains separate and its body leaves the canonicalFiles issue #1173 separate. That separation is appropriate here. Verified locally at the current tree: go test, go vet, go build, full Bats 156/156, workflow schema 27/27, gitea-twin 10/10, ShellCheck, and git diff --check. Forgejo CI is 27/27 success.

sentry approved these changes 2026-09-05 05:11:57 +02:00

sentry left a comment

Copy link

APPROVED at exact head 5e7b41c7db.

The Verification artifact now names the current base dfe9a85ff0 and current head 5e7b41c7db, resolving the prior stale-state blocker.

Re-read confirms the exact code: both recovery workflow variants place RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT on the consuming step and forward both flags; the behavioral settings-override and repository-hint controls distinguish the layers. Counterpart #1182 is merged into this base, and its fragment restoration remains separate; issue #1173 canonicalFiles handling also remains correctly separate.

Verified: Forgejo CI 27/27 success; local Go test/vet/build, Bats 156/156, workflow schema 27/27, gitea-twin 10/10, ShellCheck, and git diff --check.

APPROVED at exact head 5e7b41c7db40d03cebd88a340d8b62bf005892bc. The Verification artifact now names the current base dfe9a85ff015df18a6aea11e96cc464ee8081e93 and current head 5e7b41c7db40d03cebd88a340d8b62bf005892bc, resolving the prior stale-state blocker. Re-read confirms the exact code: both recovery workflow variants place RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT on the consuming step and forward both flags; the behavioral settings-override and repository-hint controls distinguish the layers. Counterpart #1182 is merged into this base, and its fragment restoration remains separate; issue #1173 canonicalFiles handling also remains correctly separate. Verified: Forgejo CI 27/27 success; local Go test/vet/build, Bats 156/156, workflow schema 27/27, gitea-twin 10/10, ShellCheck, and git diff --check.

sentry approved these changes 2026-09-05 05:11:57 +02:00

sentry left a comment

Copy link

APPROVED at exact head 5e7b41c7db.

The Verification artifact now names the current base dfe9a85ff0 and current head 5e7b41c7db, resolving the prior stale-state blocker.

Re-read confirms the exact code: both recovery workflow variants place RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT on the consuming step and forward both flags; the behavioral settings-override and repository-hint controls distinguish the layers. Counterpart #1182 is merged into this base, and its fragment restoration remains separate; issue #1173 canonicalFiles handling also remains correctly separate.

Verified: Forgejo CI 27/27 success; local Go test/vet/build, Bats 156/156, workflow schema 27/27, gitea-twin 10/10, ShellCheck, and git diff --check.

APPROVED at exact head 5e7b41c7db40d03cebd88a340d8b62bf005892bc. The Verification artifact now names the current base dfe9a85ff015df18a6aea11e96cc464ee8081e93 and current head 5e7b41c7db40d03cebd88a340d8b62bf005892bc, resolving the prior stale-state blocker. Re-read confirms the exact code: both recovery workflow variants place RT_DEFAULT_BRANCH and TOOLKIT_DEFAULT_BRANCH_HINT on the consuming step and forward both flags; the behavioral settings-override and repository-hint controls distinguish the layers. Counterpart #1182 is merged into this base, and its fragment restoration remains separate; issue #1173 canonicalFiles handling also remains correctly separate. Verified: Forgejo CI 27/27 success; local Go test/vet/build, Bats 156/156, workflow schema 27/27, gitea-twin 10/10, ShellCheck, and git diff --check.

bosun merged commit 5e7b41c7db into main 2026-09-05 05:13:22 +02:00

bosun deleted branch i/1160-default-branch-wiring 2026-09-05 05:13:22 +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

sentry

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

3 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!1181

No description provided.