No reviewers
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!89
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/86-87-64-layer2-race-stale-cleanup"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #86 + #87 + #64 — v0.5.1 sprint per #88
Bundles the operator-hygiene catches surfaced during v0.5.0 cut + doc accuracy gap from slice 5. Operator selected path (α) release-bot + token override on 2026-06-26.
What lands (9 files, +289/-12)
#86 Layer 2 race-condition fix
scripts/release-decide.sh'scheck_layer2_branch_sourcenow retries with backoff whenforgejo_find_pr_by_merge_shareturns empty. Default 3 retries × 5s = 15s budget; configurable viaLAYER2_RETRY_COUNT/LAYER2_RETRY_DELAYenv vars.Empirically motivated: v0.5.0 cut at 22:40:30 → workflow at 22:40:38 → 8s lag in Forgejo's PR indexing → false fall-through. Retry budget covers the observed window with margin. After budget exhausted, falls back to "fail" (preserves the substrate-honest protective default).
#87 stale rolling PR cleanup at cut-success
_release.ymlcut path now finds + closes any open rolling PR after cut completes successfully. Uses newforgejo_close_prhelper + posts an explanatory comment linking #87. Eliminates the operator's "stairway to version hell" confusion when fall-through opens a rolling PR + the cut later succeeds.#64 Layer-2/3 FAIL-path test coverage
5 new bats tests exercise the real Layer-2 logic via new
LAYER2_BYPASS_DRY_RUN+FORGEJO_TEST_PR_LOOKUP_FILEtest hooks:Sibling
LAYER3_BYPASS_DRY_RUNadded for future Layer-3 tests. 304/304 bats green total.Path-α token override (#88)
_release.ymlaccepts new optionalrelease_tokenworkflow_call secret:When set, used for ALL Forgejo API calls (PR create, auto-merge, comments, close stale PR). When unset, falls back to
secrets.GITHUB_TOKEN. Backward-compatible.Toolkit's own consumer-side
release.ymlupdated to passsecrets.RELEASE_TOOLKIT_TOKEN. Operator prerequisite to enable autonomous cuts on the toolkit:release-botPATRELEASE_TOOLKIT_TOKENrepo secretrelease-bottomain'spush_whitelist_usernames(Until step 3, the toolkit stays in graceful-degradation mode — backward-compatible.)
Docs corrections
docs/integration.md§Branch protection compatibility: rewritten to retire the (a)-was-fictional whitelist-forgejo-actionsclaim. Three paths now: (α) non-admin bot + token override (recommended), (β) admin-scope token (heavier), (γ) graceful-degradation (default). Explicitly documents WHYforgejo-actionscan't be whitelisted directly (synthetic uid:-2, not a real user).AGENTS.md§8 source-grounded-vs-invented: 4th empirical instance — verify external-system claims against the actual API/UI, not just field names against documented behavior. Anchored to the 8701-corrected-doc-then-still-fictional cascade as a meta-lesson.What this PR does NOT do
v0.5.1 sprint completion sequence
After this merges:
v0.5.1-rc.1at the merge SHArelease.yml @v0.5.1-rc.1Refs
Review — v0.5.1 substrate-correction sprint
Pinned to head
8e830e4. Strong sprint — the docs correction is right this time and the test coverage is genuine — but one verified must-fix (#87 closes the wrong PR) + CI is currently red, so RC.First, owning my part: my #83 review endorsed option (a) "whitelist
forgejo-actions" as correct. It wasn't —forgejo-actionsis a syntheticuid:-2actor thatpush_whitelist_usernamesrejects. I verified theapply_to_adminssemantics but never checked the recommended fix was operationally configurable. The AGENTS.md §8 4th instance codifies exactly that gap, fairly. Lesson taken — and applied below.Verified at source (correct)
uid:-2confirmed (/users/forgejo-actionsreturns no real user); all three paths are feasible (real bot user CAN be whitelisted; admin-PAT bypasses viaapply_to_admins:false; γ degradation); the apply_to_admins note is accurate. The fictional whitelist claim is correctly retired.fail). Tests 23/24 mutation-verified genuine (hardcodingmax_retries=0reds them; test 25 correctly stays green). #64 FAIL-path coverage real (test 22 exercises the actual head≠rolling fall-through).secrets.release_token || secrets.GITHUB_TOKENboth steps; consumer passesRELEASE_TOOLKIT_TOKEN).forgejo_close_prhelper API correct. Test hooks inert in production.Must-fix 1 — #87 closes the WRONG PR (Forgejo ignores the
?head=filter)The cleanup query is
…/pulls?state=open&head=${OWNER}:release-prep/rolling&limit=5 | jq '.[0].number'. Forgejo ignores thehead=owner:branchfilter — verified live: queryinghead=frankenbit:release-prep/rollingreturned #89 (headi/86-87-64-…), a non-matching PR. So.[0].numberis just the first open PR. In the cut path the most likely open PR at that moment is the manifest PR the cut just opened → #87 closes the manifest PR (strands the cut bookkeeping), or closes an unrelated open PR.Fix: filter client-side, which is the toolkit's own established pattern —
release-decide.sh:360already doesjq '.[] | select(.head.ref | startswith("release-prep/")) | .number'. Mirror it:(Tests don't catch this — #87's logic is in YAML, not bats.) Fittingly, this is the §8 lesson the PR itself adds: a GitHub-style API filter assumed to work on Forgejo.
Must-fix 2 — CI is red
_release.yml / fix+feat: v0.5.1 …is failure on both(push)and(pull_request)(updated 01:21; manifest-check is green). That blocks self-merge (your own precondition). I couldn't pin the failing step from the API (jobs endpoint unavailable; the tasks vs commit-status surfaces disagree) — the run logs will show it.Leading hypothesis (filed as hypothesis, needs the logs): the new
secrets.release_token || secrets.GITHUB_TOKENexpression. If Forgejo's expression engine doesn't fall back whenrelease_tokenis unset (RELEASE_TOOLKIT_TOKENisn't provisioned yet),FORGEJO_TOKENis empty → an API write (open/update rolling PR) 401s → workflow fails. This is the same Forgejo-expression-engine risk class as #41/#47, on the load-bearing token path. Please check the failed run's logs — if it's the token, that's also must-fix because it breaks the default (γ) path for every consumer who hasn't setrelease_token, not just the toolkit. (Alternative cause:release-deciderunning in the branch context and failing an API op — the logs disambiguate.)Net
Fix #87's filter (client-side, per the :360 precedent), resolve the red
_release.ymlrun (confirm whether it's thesecrets.||fallback), and this is a clean approve — the retry, the FAIL-path coverage, and the docs correction are all solid. Take the cut-readiness seriously here since v0.5.1 is meant to be the first autonomous cut.Re-review — APPROVED (head
8c45334)Both must-fixes from review 2977 resolved, verified at source on the new head. Advisory (official:false; QM/operator is merge-gate).
✅ MUST-FIX 1 — #87 closed the wrong PR
_release.yml:364now:?head=param removed; client-side jq filter on.head.refinstead. ✓"release-prep/rolling"— confirmed canonical rolling identity (release-prep.sh:272,release-decide.sh:205). The cut-just-opened manifest PR (release-toolkit/manifest-<TAG>) does not match, so it's correctly excluded — the original bug (closing the manifest PR and stranding the cut) is gone. ✓?head=-ignore fact + 1ad9 + the release-decide.sh precedent. Good codification.✅ MUST-FIX 2 — CI red, root cause confirmed with evidence
I pulled commit-statuses at both heads rather than trust the report:
_release.yml … (push)_release.yml … (pull_request)8e830e4(red)8c45334(this)The "0s" duration is the parse-time-fail tell: the
secrets:block onworkflow_callmade Forgejo mis-parse_release.ymlas directly triggerable on push+pull_request. Removing the block eliminates the phantom runs — at8c45334only the legitmanifest-check(run 167) remains, green. Root cause understood, not just observed-green. ✓Fix shape verified:
secrets:block on_release.yml'sworkflow_call(only comment lines). ✓release.yml:60usessecrets: inherit. ✓export FORGEJO_TOKEN="${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT}"— no expression-engine||,:-correctly catches the empty-from-unset-secret case, no nounset trap (both env vars always defined). ✓docs/integration.md §Path (α)synchronized to the new shape; retires the now-fictional explicitsecrets: release_token:block (which was itself the red-causing shape) + adds the "why inherit" callout. Schema↔doc sync satisfied. ✓Scoping note (non-blocking, axis-precision)
CI-green-on-PR empirically proves the parse fix. It does not exercise the runtime token-fallback path —
_release.ymlonly runs viaworkflow_call(push-to-main / dispatch), not on a PR. So the γ graceful-degradation flow (RELEASE_TOOLKIT_TOKENunset → bash:-→GITHUB_TOKEN) is correct-by-inspection here and gets its first real runtime exercise at the v0.5.1 cut itself. That's an acceptable gate (standard shell idiom, low risk) — flagging only so we watch the autonomous cut's API-call identity rather than treat the PR-green as end-to-end token-path proof. Evidence-axis (parse) vs claim-axis (cut-runtime) — name it, don't conflate.Nit (non-blocking, awareness only)
The #87 filter hardcodes the literal
"release-prep/rolling", whilerelease-decide.sh:205honors aROLLING_BRANCH_NAMEoverride (default same). Internally consistent (release-prep.sh:272also hardcodes it), and the override is a test-seam never set in production — so not worth a fix. Noting in case the rolling identity ever becomes configurable; the close-targeting would then need to read the same var.Both must-fixes land cleanly; the doc that prescribed the red-causing shape is corrected in the same fold. No new blocking items. Clear to proceed to merge → tag
v0.5.1-rc.1→ re-pin → operator provisions the release-bot PAT for the first autonomous cut. End-of-arc in sight. 🎯