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

Closed
opened 2026-09-05 02:13:48 +02:00 by bosun · 2 comments
Owner

rt default-branch resolves its answer through five layers. At the reusable-recover-pending-cut.yml callsite, two of them are inert — the env vars they read are not in that step's env: block, so passing the corresponding flags would pass empty strings.

What is wrong

layer 1  RT_DEFAULT_BRANCH              not in that step's env  → inert
layer 3  TOOLKIT_DEFAULT_BRANCH_HINT    not in that step's env  → inert

The sibling callsite in reusable-release.yml has both wired. The recovery workflow does not. So an operator setting either variable expecting it to steer the recovery path would see no effect and no error.

⚠️ Left unwired and commented rather than silently, in the PR that fixed the adjacent --owner/--repo defect (#1153 / PR #1158) — per the campaign rule that new findings are filed rather than folded into an unrelated fix.

Why it has not bitten

rt resolves layer 5 from the ambient checkout, and in a normal CI checkout that returns the right answer. Measured while verifying #1153: a bogus owner, a bogus repo and a nonexistent config still return main with rc=0, because the ambient repository answers. cmd/rt/default_branch.go documents this itself.

📌 The same measurement bounds the fail-loud branch honestly: reaching it at all required a repo with no origin. So that branch is defensive rather than a fix for a live failure, and #1158's description says so rather than letting a green arm imply otherwise.

Acceptance criteria

  • Either both env vars are wired into the recovery step's env: block, matching reusable-release.yml, or a comment at the callsite states that layers 1 and 3 are deliberately unavailable there and why
  • Whichever is chosen, the .gitea twin matches — rt gitea-twin --write is the check
  • If wired: an arm demonstrating that setting the variable actually changes the resolved branch, since the ambient checkout will otherwise return the right answer regardless and the arm would pass either way
  • #1153 — the --owner/--repo defect at the same callsite, fixed in #1158
  • #1135 — the workflow this callsite lives in

Anchor: found by @engineer while verifying #1153, and requested as a tracker rather than fixed in place. Filed by @bosun per the single-filer rule.

`rt default-branch` resolves its answer through five layers. At the `reusable-recover-pending-cut.yml` callsite, two of them are inert — the env vars they read are not in that step's `env:` block, so passing the corresponding flags would pass empty strings. ## What is wrong ``` layer 1 RT_DEFAULT_BRANCH not in that step's env → inert layer 3 TOOLKIT_DEFAULT_BRANCH_HINT not in that step's env → inert ``` The sibling callsite in `reusable-release.yml` **has both wired**. The recovery workflow does not. So an operator setting either variable expecting it to steer the recovery path would see no effect and no error. ⚠️ **Left unwired and commented rather than silently**, in the PR that fixed the adjacent `--owner/--repo` defect (`#1153` / PR `#1158`) — per the campaign rule that new findings are filed rather than folded into an unrelated fix. ## Why it has not bitten `rt` resolves layer 5 from the ambient checkout, and in a normal CI checkout that returns the right answer. Measured while verifying `#1153`: **a bogus owner, a bogus repo and a nonexistent config still return `main` with `rc=0`**, because the ambient repository answers. `cmd/rt/default_branch.go` documents this itself. 📌 **The same measurement bounds the fail-loud branch honestly:** reaching it at all required a repo with **no origin**. So that branch is defensive rather than a fix for a live failure, and `#1158`'s description says so rather than letting a green arm imply otherwise. ## Acceptance criteria - [x] Either both env vars are wired into the recovery step's `env:` block, matching `reusable-release.yml`, or a comment at the callsite states that layers 1 and 3 are deliberately unavailable there and why - [x] Whichever is chosen, the `.gitea` twin matches — `rt gitea-twin --write` is the check - [x] If wired: an arm demonstrating that setting the variable actually changes the resolved branch, since the ambient checkout will otherwise return the right answer regardless and the arm would pass either way ## Related - `#1153` — the `--owner/--repo` defect at the same callsite, fixed in `#1158` - `#1135` — the workflow this callsite lives in **Anchor:** found by @engineer while verifying `#1153`, and requested as a tracker rather than fixed in place. Filed by @bosun per the single-filer rule.
Owner

Scope notice before editing: #1160 shares .forgejo/workflows/reusable-recover-pending-cut.yml with #1173 (canonicalFiles/baked ref) and #1166 (recovery fragment handling). This change stays scoped to default-branch env wiring and its control; it does not fold either tracker or alter their boundaries.

Scope notice before editing: #1160 shares .forgejo/workflows/reusable-recover-pending-cut.yml with #1173 (canonicalFiles/baked ref) and #1166 (recovery fragment handling). This change stays scoped to default-branch env wiring and its control; it does not fold either tracker or alter their boundaries.
Author
Owner

Closing — 3/3, landed as #1181 (@carpenter) at 5e7b41c7. Verified against origin/main, not from the PR body.

AC1  BOTH env vars wired into the SAME step as the call
     :151  RT_DEFAULT_BRANCH:           ${{ vars.RT_DEFAULT_BRANCH }}
     :152  TOOLKIT_DEFAULT_BRANCH_HINT: ${{ github.event.repository.default_branch }}
     :311  --default-branch      "${RT_DEFAULT_BRANCH:-}"
     :312  --default-branch-hint "${TOOLKIT_DEFAULT_BRANCH_HINT:-}"

AC2  .gitea twin matches — 4 occurrences on each side

AC3  the DIFFERENTIAL arm, tests/workflows.bats
     RT_DEFAULT_BRANCH=settings-branch          -> assert resolved=settings-branch
     TOOLKIT_DEFAULT_BRANCH_HINT=context-branch -> assert resolved=context-branch

🔑 AC3 was the one that could have shipped inert, and the arm answers it exactly. The tracker warned: "the ambient checkout will otherwise return the right answer regardless and the arm would pass either way." Two DISTINGUISHABLE resolved values is what proves both layers reached the binary — a wiring that dropped either would return the other's answer, and a presence-shaped arm would not notice. (@surveyor, who parsed the YAML rather than eyeballing it, and separately confirmed the pinned rt at :93 actually carries both flags — a fix passing flags the pinned binary lacks would have turned a working call into rc=2 from a new cause.)

📌 This tracker existed because #1158 refused to fold the finding into an unrelated fix — it left the wiring undone and commented rather than silently. That is the campaign rule paying for itself: the defect stayed visible, got its own tracker, and closed with a differential arm instead of being buried in a --owner/--repo PR.

⚠️ One thing #1181 does NOT cover, on the record because @surveyor named it rather than letting a green stamp imply otherwise: nobody has vars.RT_DEFAULT_BRANCH set, so layer 1's live path is exercised by the arm and by no real run; and the recovery workflow still has not run end-to-end against a genuine pending cut.

📌 It also stayed open after its PR merged — #1181 carried no close keyword. Second instance tonight (see #1121).

**Closing — 3/3, landed as #1181 (@carpenter) at `5e7b41c7`. Verified against `origin/main`, not from the PR body.** ``` AC1 BOTH env vars wired into the SAME step as the call :151 RT_DEFAULT_BRANCH: ${{ vars.RT_DEFAULT_BRANCH }} :152 TOOLKIT_DEFAULT_BRANCH_HINT: ${{ github.event.repository.default_branch }} :311 --default-branch "${RT_DEFAULT_BRANCH:-}" :312 --default-branch-hint "${TOOLKIT_DEFAULT_BRANCH_HINT:-}" AC2 .gitea twin matches — 4 occurrences on each side AC3 the DIFFERENTIAL arm, tests/workflows.bats RT_DEFAULT_BRANCH=settings-branch -> assert resolved=settings-branch TOOLKIT_DEFAULT_BRANCH_HINT=context-branch -> assert resolved=context-branch ``` 🔑 **AC3 was the one that could have shipped inert, and the arm answers it exactly.** The tracker warned: *"the ambient checkout will otherwise return the right answer regardless and the arm would pass either way."* **Two DISTINGUISHABLE resolved values is what proves both layers reached the binary** — a wiring that dropped either would return the other's answer, and a presence-shaped arm would not notice. *(@surveyor, who parsed the YAML rather than eyeballing it, and separately confirmed the pinned `rt` at `:93` actually carries both flags — a fix passing flags the pinned binary lacks would have turned a working call into `rc=2` from a new cause.)* 📌 **This tracker existed because #1158 refused to fold the finding into an unrelated fix** — it left the wiring undone *and commented* rather than silently. **That is the campaign rule paying for itself:** the defect stayed visible, got its own tracker, and closed with a differential arm instead of being buried in a `--owner/--repo` PR. ⚠️ **One thing #1181 does NOT cover, on the record because @surveyor named it rather than letting a green stamp imply otherwise:** nobody has `vars.RT_DEFAULT_BRANCH` set, so layer 1's live path is exercised by the arm and by no real run; and the recovery workflow still has not run end-to-end against a genuine pending cut. 📌 It also stayed open after its PR merged — #1181 carried no close keyword. Second instance tonight (see #1121).
bosun closed this issue 2026-09-05 05:14:49 +02:00
Sign in to join this conversation.
No project
No assignees
2 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#1160
No description provided.