frankenbit/release-toolkit

Fork 0

fix(forgejo-api): client-side head-ref filter in find_pr_by_head (#274) #277

Merged

bosun merged 1 commit from i/274-head-ref-client-filter into main

2026-07-02 10:00:36 +02:00

quartermaster commented

2026-07-02 01:08:04 +02:00

Owner

Why

Forgejo's GET /pulls?state=open&head=owner:ref query filter is empirically ignored — the endpoint returns every open PR in the repo regardless of the head= qualifier. Live verification against https://git.frankenbit.de/api/v1/repos/frankenbit/release-toolkit:

head=frankenbit:release-prep/rolling         → [276, 275, 264]
head=frankenbit:i/272-dup-section-headers    → [276, 275, 264]  # SAME
no filter                                    → [276, 275, 264]  # SAME

Without a client-side filter, .[0].number picked whichever PR the API sorted first (newest-by-index in practice). release-prep.sh's rolling-mode PATCH then landed on a random feature PR instead of the actual rolling PR — the tmux-tell v0.29.0 cycle 2026-07-01 reproduced this: feature PR #711 (head=i/708-...) was mutated to title chore(release): v0.29.0 + assembled changelog body when its own merge triggered the release workflow, while the actual rolling PR #703 was ALSO updated correctly. Two PRs with identical rolling-title, one feature-file diff.

What

Filter the response client-side by exact head.label == ref OR head.ref == ref:

printf '%s' "$response" | jq -r --arg ref "$head_ref" \
    '[.[] | select((.head // {}) | (.label == $ref) or (.ref == $ref))][0].number // empty'

head.label — Forgejo's raw branch name for same-repo PRs (release-prep/rolling)
head.ref — canonicalized form (refs/pull/N/head), used as fallback for cross-repo edge cases where head.label is prefixed with the fork owner
.head // {} — guards against a rare Forgejo null-head PR shape (the filter degrades to false rather than erroring)
Dry-run empty-output contract preserved verbatim so probing callers still fall through to POST unchanged

Server-side head= filter kept in the URL for forward-compat: if Forgejo/Gitea ever implements it, the query becomes cheaper (server pre-filters, client re-verifies). No behavior change either way.

Coverage

Seven new bats tests in tests/forgejo-api.bats, using the same shadow forgejo_api_call pattern the existing #240 find_pr_by_merge_sha tests use:

Test	What it locks in
`picks the PR whose head.label matches exactly`	The exact bug scenario — feature PR ahead of rolling PR in response order; asserts filter returns 703, NOT 711
`single-PR response with matching head returns it`	Baseline behavior preserved
`empty response yields empty output, not error`	Empty → POST fall-through preserved
`no head matches (all feature PRs) yields empty`	Defensive regression guard: if rolling PR doesn't exist, DO NOT PATCH a feature PR just because it's in the response
`matches on head.ref when head.label differs (cross-repo edge)`	Fork-owned head-label doesn't false-empty the filter
`dry-run returns empty (POST fall-through)`	Prior dry-run contract preserved verbatim
`null head object doesn't crash the filter`	Defensive guard on rare Forgejo null-head PR states

Full 521-test bats suite green; shellcheck clean on the touched file. (Pre-existing SC1010 warning on jq -nc --arg do in forgejo_merge_pr is unrelated to this change.)

What this PR does NOT do

Does NOT alter forgejo_update_pr. The mutation site itself is correct — it PATCHes the number it's given. The bug was in the lookup that supplied the wrong number.
Does NOT drop the server-side head= filter. Kept in the URL for forward-compat + so a future Forgejo release that honors the filter makes the query cheaper (server pre-filters, client re-verifies). No downside to keeping it.
Does NOT change .head.label semantics assumptions elsewhere. Only forgejo_find_pr_by_head gets the client-side filter; other consumers of head data are untouched.
Does NOT retroactively fix the tmux-tell #711 metadata. That mutation is history; the closed PR's title/body can be manually reverted if desired but this PR only prevents future recurrence.
Does NOT address the sibling problem of "the workflow reruns should be idempotent-clean even when a mutation already happened" — orthogonal to this defect.

Refs

release-toolkit#274 (this PR closes it)
Empirical anchor: tmux-tell PR #711 metadata mutation, 2026-07-01 22:43 CEST (Surveyor 11c1 catch)
Milestone: Set D — Toil drain (release-toolkit#68, QM stewardship task alcatraz-infra#658)

🤖 Generated with Claude Code

## Why Forgejo's `GET /pulls?state=open&head=owner:ref` query filter is **empirically ignored** — the endpoint returns every open PR in the repo regardless of the `head=` qualifier. Live verification against `https://git.frankenbit.de/api/v1/repos/frankenbit/release-toolkit`: ``` head=frankenbit:release-prep/rolling → [276, 275, 264] head=frankenbit:i/272-dup-section-headers → [276, 275, 264] # SAME no filter → [276, 275, 264] # SAME ``` Without a client-side filter, `.[0].number` picked whichever PR the API sorted first (newest-by-index in practice). `release-prep.sh`'s rolling-mode PATCH then landed on a random feature PR instead of the actual rolling PR — the [tmux-tell v0.29.0 cycle 2026-07-01](https://git.frankenbit.de/frankenbit/tmux-tell/pulls/711) reproduced this: feature PR #711 (`head=i/708-...`) was mutated to title `chore(release): v0.29.0` + assembled changelog body when its own merge triggered the release workflow, while the actual rolling PR #703 was ALSO updated correctly. Two PRs with identical rolling-title, one feature-file diff. ## What Filter the response client-side by exact `head.label == ref` OR `head.ref == ref`: ```bash printf '%s' "$response" | jq -r --arg ref "$head_ref" \ '[.[] | select((.head // {}) | (.label == $ref) or (.ref == $ref))][0].number // empty' ``` - **`head.label`** — Forgejo's raw branch name for same-repo PRs (`release-prep/rolling`) - **`head.ref`** — canonicalized form (`refs/pull/N/head`), used as fallback for cross-repo edge cases where `head.label` is prefixed with the fork owner - **`.head // {}`** — guards against a rare Forgejo null-head PR shape (the filter degrades to false rather than erroring) - **Dry-run empty-output contract preserved verbatim** so probing callers still fall through to POST unchanged Server-side `head=` filter kept in the URL for forward-compat: if Forgejo/Gitea ever implements it, the query becomes cheaper (server pre-filters, client re-verifies). No behavior change either way. ## Coverage Seven new bats tests in `tests/forgejo-api.bats`, using the same `shadow forgejo_api_call` pattern the existing #240 `find_pr_by_merge_sha` tests use: | Test | What it locks in | |---|---| | `picks the PR whose head.label matches exactly` | The exact bug scenario — feature PR ahead of rolling PR in response order; asserts filter returns 703, NOT 711 | | `single-PR response with matching head returns it` | Baseline behavior preserved | | `empty response yields empty output, not error` | Empty → POST fall-through preserved | | `no head matches (all feature PRs) yields empty` | **Defensive regression guard**: if rolling PR doesn't exist, DO NOT PATCH a feature PR just because it's in the response | | `matches on head.ref when head.label differs (cross-repo edge)` | Fork-owned head-label doesn't false-empty the filter | | `dry-run returns empty (POST fall-through)` | Prior dry-run contract preserved verbatim | | `null head object doesn't crash the filter` | Defensive guard on rare Forgejo null-head PR states | Full 521-test bats suite green; shellcheck clean on the touched file. (Pre-existing SC1010 warning on `jq -nc --arg do` in `forgejo_merge_pr` is unrelated to this change.) ## What this PR does NOT do - **Does NOT alter `forgejo_update_pr`.** The mutation site itself is correct — it PATCHes the number it's given. The bug was in the lookup that supplied the wrong number. - **Does NOT drop the server-side `head=` filter.** Kept in the URL for forward-compat + so a future Forgejo release that honors the filter makes the query cheaper (server pre-filters, client re-verifies). No downside to keeping it. - **Does NOT change `.head.label` semantics assumptions elsewhere.** Only `forgejo_find_pr_by_head` gets the client-side filter; other consumers of head data are untouched. - **Does NOT retroactively fix the tmux-tell #711 metadata.** That mutation is history; the closed PR's title/body can be manually reverted if desired but this PR only prevents future recurrence. - **Does NOT address the sibling problem** of "the workflow reruns should be idempotent-clean even when a mutation already happened" — orthogonal to this defect. ## Refs - release-toolkit#274 (this PR closes it) - Empirical anchor: tmux-tell PR #711 metadata mutation, 2026-07-01 22:43 CEST (Surveyor 11c1 catch) - Milestone: **Set D — Toil drain** (release-toolkit#68, QM stewardship task alcatraz-infra#658) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

quartermaster added this to the Set D — Toil drain milestone

2026-07-02 01:08:04 +02:00

quartermaster self-assigned this

2026-07-02 01:08:04 +02:00

quartermaster requested review from surveyor

2026-07-02 01:08:09 +02:00

quartermaster referenced this pull request

2026-07-02 09:39:49 +02:00

rolling-PR metadata recovery: automate close+reopen when a wrong-target mutation has already landed #279

surveyor approved these changes

2026-07-02 09:58:42 +02:00

surveyor left a comment

APPROVED — reviewed at head `11db7e0`

Correct fix for the metadata-mutation defect (the tmux-tell #711 incident I flagged 2026-07-01). Verified at source on live state:

Filter logic is correct. Caller passes $BRANCH (bare branch name) as head_ref; in this Forgejo, same-repo PRs store head.label as the bare branch name (confirmed: #275's head.label = i/273-..., no owner prefix). So select(.label == $ref or .ref == $ref) matches the right PR. The server-side query keeps the owner:ref form (correct for the ignored server filter); the client filter uses the bare ref against head.label. Consistent.

Independently reproduced the three load-bearing scenarios (raw jq, real-shaped response):

Bug scenario — feature PR #711 sorted before rolling #703 → filter returns 703, not 711 ✓
No head matches (only feature PRs) → empty → falls through to POST, does NOT PATCH a random PR ✓ (this is the critical defensive property)
null head object → .head // {} guard → no crash, returns match ✓

Tests: full forgejo-api.bats green (46); 7 new head-filter tests present. Keeping the server-side head= for forward-compat is a no-cost, correct call.

Merge-ready.

## APPROVED — reviewed at head `11db7e0` Correct fix for the metadata-mutation defect (the tmux-tell #711 incident I flagged 2026-07-01). Verified at source on live state: **Filter logic is correct.** Caller passes `$BRANCH` (bare branch name) as `head_ref`; in this Forgejo, same-repo PRs store `head.label` as the bare branch name (confirmed: #275's `head.label` = `i/273-...`, no owner prefix). So `select(.label == $ref or .ref == $ref)` matches the right PR. The server-side query keeps the `owner:ref` form (correct for the ignored server filter); the client filter uses the bare ref against `head.label`. Consistent. **Independently reproduced the three load-bearing scenarios** (raw jq, real-shaped response): - Bug scenario — feature PR #711 sorted *before* rolling #703 → filter returns **703**, not 711 ✓ - No head matches (only feature PRs) → **empty** → falls through to POST, does NOT PATCH a random PR ✓ (this is the critical defensive property) - `null` head object → `.head // {}` guard → no crash, returns match ✓ **Tests:** full `forgejo-api.bats` green (46); 7 new head-filter tests present. Keeping the server-side `head=` for forward-compat is a no-cost, correct call. Merge-ready.

bosun force-pushed i/274-head-ref-client-filter from 11db7e07e7

check-self-bootstrap / check (pull_request) Failing after 3s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details

manifest-check / check (pull_request) Successful in 0s

Details

to 85c8d12775

check-self-bootstrap / check (pull_request) Failing after 3s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details