fix(release-decide): Layer 2 uses head.label not head.ref (closes #92) #93

Merged
quartermaster merged 2 commits from i/92-head-label-layer2 into main 2026-06-26 10:05:21 +02:00

Closes #92 — first v0.6.1 sprint item

v0.6.0 cut surfaced this Layer 2 substrate gap empirically: operator merged PR #90 → workflow fired → mode=noop instead of mode=cut. Re-dispatch produced the same result. Manifest stayed at v0.5.0, no v0.6.0 draft was created. Manual recovery (local draft-release.sh + admin manifest push) shipped v0.6.0 as a stopgap.

Root cause

Forgejo's default_delete_branch_after_merge: true deletes the head branch on merge. The closed PR's head.ref is then REPLACED from the original branch name (release-prep/rolling) to the PR's internal ref (refs/pull/90/head). My Layer 2 check did exact-match against ROLLING_BRANCH_NAME → FALSE → Layer 2 fail → fall-through → mode=noop (no fragments, no cc-relevant commits since the fragment was already consumed in the prep commit).

// PR #90 post-merge:
{
  "head_ref":   "refs/pull/90/head",       // BROKEN — replaced on branch delete
  "head_label": "release-prep/rolling",    // PRESERVED canonical form
  "head_sha":   "ba10f67c..."
}

Fix

Use head.label (canonical "owner:branch-name" form, preserved across lifecycle) instead of head.ref. Strip the "owner:" prefix to compare against ROLLING_BRANCH_NAME.

-    head_ref=$(printf '%s' "$pr_json" | jq -r '.head.ref // empty')
-    if [[ "$head_ref" == "$ROLLING_BRANCH_NAME" ]]; then
+    head_label=$(printf '%s' "$pr_json" | jq -r '.head.label // empty')
+    head_branch="${head_label#*:}"
+    if [[ "$head_branch" == "$ROLLING_BRANCH_NAME" ]]; then

Test coverage (2 new, 306 total green)

Test Covers
#92 Layer-2 PASS: post-merge head.ref=refs/pull/N/head with head.label=owner:release-prep/rolling The v0.6.0 scenario directly — branch-deleted state still resolves correctly
#92 Layer-2 FAIL: head.label points at non-rolling branch The safeguard's protective behavior under the new check

Mutation-verified: reverting to .head.ref + dropping the "owner:" strip makes the PASS test red, confirming the new tests discriminate genuine substrate behavior.

Pre-existing #64 + #86 tests updated to inject head.label alongside head.ref in their mock PR JSON — backward-compatible + exercises the now-load-bearing field.

AGENTS.md §8 n=5 promotion

The lifecycle-behavior catch is the 5th empirical instance of the source-grounded-vs-invented axis. Per the empirical-promotion criterion (n=3 to file, n=5 to promote to named pattern):

# Surface Empirical instance
1 test-vs-prod Surveyor 5bbe placebo-test catch
2 clone-vs-live Surveyor 6a82 stale-clone-origin-main
3 config-semantics Surveyor 8701 apply_to_admins inversion
4 config-feasibility forgejo-actions whitelist fictional
5 lifecycle-behavior this — head.ref-replaced-on-delete

Promoted to a named project pattern: substrate-state-care for external claims. Future doc-claim PRs should explicitly call out which axis their claim depends on + how it was verified.

What this PR does NOT do

  • Does not address #56 (manifest-vs-history defensive guard) — v0.6.1 scope, separate PR
  • Does not address #54 (line-break cosmetic) — v0.6.1 scope, separate PR
  • Does not address #47 (Forgejo trusted-proxy trap) — research deferred
  • Does not touch the #87 stale rolling PR cleanup — it queries OPEN PRs where head.ref is still the original branch name (deletion happens on merge, not on close). The cleanup correctness is preserved by inspection.

v0.6.1 sprint completion sequence

After this merges:

  1. Tag v0.6.1-rc.1 at the merge SHA
  2. Re-pin toolkit's release.yml @v0.6.1-rc.1 (self-bootstrap pattern)
  3. Workflow re-fires → next cut should fire correctly via the head.label fix
  4. Then #56 + #54 + remaining v0.6.1 items as follow-up PRs (or bundle)

Refs

  • Closes: #92
  • Empirical surface: v0.6.0 cut (operator merge #90 → mode=noop; manual recovery shipped v0.6.0)
  • AGENTS.md §8: 5th instance + promotion to named pattern
  • v0.6.1 sprint: this + #56 + #54 + AGENTS.md §8 n=5 promotion + (optional) #47 research
  • Carry-forward class continues: v0.4.0 surfaced #66/#70/#73/#78; v0.5.0 surfaced #86/#87; v0.6.0 surfaced #92. Every cut catches ~1-2 latent bugs.
## Closes #92 — first v0.6.1 sprint item v0.6.0 cut surfaced this Layer 2 substrate gap empirically: operator merged PR #90 → workflow fired → `mode=noop` instead of `mode=cut`. Re-dispatch produced the same result. Manifest stayed at v0.5.0, no v0.6.0 draft was created. Manual recovery (local `draft-release.sh` + admin manifest push) shipped v0.6.0 as a stopgap. ## Root cause Forgejo's `default_delete_branch_after_merge: true` deletes the head branch on merge. The closed PR's `head.ref` is then **REPLACED** from the original branch name (`release-prep/rolling`) to the PR's internal ref (`refs/pull/90/head`). My Layer 2 check did exact-match against `ROLLING_BRANCH_NAME` → FALSE → Layer 2 fail → fall-through → mode=noop (no fragments, no cc-relevant commits since the fragment was already consumed in the prep commit). ```json // PR #90 post-merge: { "head_ref": "refs/pull/90/head", // BROKEN — replaced on branch delete "head_label": "release-prep/rolling", // PRESERVED canonical form "head_sha": "ba10f67c..." } ``` ## Fix Use `head.label` (canonical `"owner:branch-name"` form, preserved across lifecycle) instead of `head.ref`. Strip the `"owner:"` prefix to compare against `ROLLING_BRANCH_NAME`. ```diff - head_ref=$(printf '%s' "$pr_json" | jq -r '.head.ref // empty') - if [[ "$head_ref" == "$ROLLING_BRANCH_NAME" ]]; then + head_label=$(printf '%s' "$pr_json" | jq -r '.head.label // empty') + head_branch="${head_label#*:}" + if [[ "$head_branch" == "$ROLLING_BRANCH_NAME" ]]; then ``` ## Test coverage (2 new, 306 total green) | Test | Covers | |---|---| | `#92 Layer-2 PASS: post-merge head.ref=refs/pull/N/head with head.label=owner:release-prep/rolling` | The v0.6.0 scenario directly — branch-deleted state still resolves correctly | | `#92 Layer-2 FAIL: head.label points at non-rolling branch` | The safeguard's protective behavior under the new check | **Mutation-verified**: reverting to `.head.ref` + dropping the `"owner:"` strip makes the PASS test red, confirming the new tests discriminate genuine substrate behavior. Pre-existing #64 + #86 tests updated to inject `head.label` alongside `head.ref` in their mock PR JSON — backward-compatible + exercises the now-load-bearing field. ## AGENTS.md §8 n=5 promotion The lifecycle-behavior catch is the **5th empirical instance** of the source-grounded-vs-invented axis. Per the empirical-promotion criterion (n=3 to file, n=5 to promote to named pattern): | # | Surface | Empirical instance | |---|---|---| | 1 | test-vs-prod | Surveyor 5bbe placebo-test catch | | 2 | clone-vs-live | Surveyor 6a82 stale-clone-origin-main | | 3 | config-semantics | Surveyor 8701 apply_to_admins inversion | | 4 | config-feasibility | forgejo-actions whitelist fictional | | 5 | **lifecycle-behavior** | **this — head.ref-replaced-on-delete** | **Promoted to a named project pattern**: *substrate-state-care for external claims*. Future doc-claim PRs should explicitly call out which axis their claim depends on + how it was verified. ## What this PR does NOT do - **Does not address #56** (manifest-vs-history defensive guard) — v0.6.1 scope, separate PR - **Does not address #54** (line-break cosmetic) — v0.6.1 scope, separate PR - **Does not address #47** (Forgejo trusted-proxy trap) — research deferred - **Does not touch the #87 stale rolling PR cleanup** — it queries OPEN PRs where `head.ref` is still the original branch name (deletion happens on merge, not on close). The cleanup correctness is preserved by inspection. ## v0.6.1 sprint completion sequence After this merges: 1. Tag `v0.6.1-rc.1` at the merge SHA 2. Re-pin toolkit's `release.yml @v0.6.1-rc.1` (self-bootstrap pattern) 3. Workflow re-fires → next cut should fire correctly via the head.label fix 4. Then #56 + #54 + remaining v0.6.1 items as follow-up PRs (or bundle) ## Refs - **Closes**: [#92](https://git.frankenbit.de/frankenbit/release-toolkit/issues/92) - **Empirical surface**: v0.6.0 cut (operator merge #90 → mode=noop; manual recovery shipped v0.6.0) - **AGENTS.md §8**: 5th instance + promotion to named pattern - **v0.6.1 sprint**: this + #56 + #54 + AGENTS.md §8 n=5 promotion + (optional) #47 research - **Carry-forward class continues**: v0.4.0 surfaced #66/#70/#73/#78; v0.5.0 surfaced #86/#87; v0.6.0 surfaced #92. Every cut catches ~1-2 latent bugs.
fix(release-decide): Layer 2 uses head.label not head.ref (closes #92)
All checks were successful
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 4s
manifest-check / check (pull_request) Successful in 0s
cdb75743a1
v0.6.0 cut surfaced this Layer 2 substrate gap:

- Operator merged PR #90 → workflow fired → mode=noop (not mode=cut)
- Re-dispatch identical: mode=noop with reason=no_release_relevant_content
- Manifest stayed at v0.5.0, no v0.6.0 draft created
- Manual recovery: scripts/draft-release.sh + admin manifest push

Root cause: Forgejo's default_delete_branch_after_merge:true deletes
the head branch on merge. After deletion, the PR's head.ref is
REPLACED from the original branch name ('release-prep/rolling') to
the pull-request internal ref ('refs/pull/90/head'). My Layer 2
check did exact-match against ROLLING_BRANCH_NAME → FALSE → Layer 2
fail → fall-through → mode=noop (no fragments, no cc-relevant
commits since the fragment was already consumed).

## Fix

Use head.label (canonical "owner:branch-name" form, preserved
across lifecycle) instead of head.ref. Strip the "owner:" prefix
to compare against ROLLING_BRANCH_NAME.

## Test coverage

2 new bats tests in tests/release-decide.bats:
  - #92 PASS: post-merge head.ref='refs/pull/N/head' +
    head.label='owner:release-prep/rolling' → mode=cut
  - #92 FAIL: head.label points at non-rolling branch (safeguard
    protective behavior verified)

Mutation-verified by reverting to .head.ref + .head.label without
"owner:" strip → PASS test goes red, confirming the new tests
discriminate genuine substrate behavior.

Pre-existing #64 + #86 tests updated to inject head.label
alongside head.ref in their mock PR JSON — backward-compatible
+ exercises the head.label path that's now load-bearing.

## AGENTS.md §8 n=5 promotion

The lifecycle-behavior catch is the 5th empirical instance of
the source-grounded-vs-invented axis. Promoting to a named
project pattern: substrate-state-care for external claims.
Future doc-claim PRs should call out which axis their claim
depends on + how it was verified.

Per empirical-promotion criterion across structurally-distinct
surfaces:
  1. test-vs-prod (5bbe placebo)
  2. clone-vs-live (6a82 stale-clone)
  3. config-semantics (8701 apply_to_admins)
  4. config-feasibility (forgejo-actions-fictional)
  5. lifecycle-behavior (this — head.ref-replaced-on-delete)

bats: 304 → 306 green (2 new #92 + existing all pass with
head.label injection in mock data).

Refs #92 + AGENTS.md §8 promotion + v0.6.1 sprint scope.
surveyor approved these changes 2026-06-26 10:03:46 +02:00
surveyor left a comment

APPROVED — #92 head.label Layer-2 fix (head cdb7574)

Code is merge-ready. One should-consider on the §8 promotion framing (doc, non-blocking). Advisory (official:false).

Root cause confirmed at source (not taken on report)

The filed root cause is a substrate-behavior claim, so I probed it on the live API rather than trust the description. The merged v0.6.0 rolling PR #90 now shows:

  • head.ref = refs/pull/90/head (replaced after branch auto-delete)
  • head.label = release-prep/rolling (preserved)

Exactly the claimed behavior. Same pattern on #80/#76. So the old head.ref keying saw refs/pull/90/head ≠ release-prep/rolling → Layer-2 fall-through → the "merge → mode=noop" bug. Keying on the lifecycle-stable head.label is the right, race-free fix (head.ref's value depends on branch-delete timing; head.label never gets replaced).

Implementation correct

release-decide.sh:261head_branch="${head_label#*:}" strips the owner: prefix when present and is a safe no-op when absent. Confirmed against real data: #90's label is the bare release-prep/rolling (same-repo PRs carry no owner: prefix), and #*: leaves it unchanged → matches ROLLING_BRANCH_NAME → pass. Robust for both same-repo and cross-repo shapes.

Mutation-verified genuine (ran it myself)

Reverted .head.label.head.ref in the script and ran the #92 PASS test → not ok (red): head.ref refs/pull/90/head ≠ rolling → mode=update not cut. The guard discriminates the actual substrate behavior; not a placebo. Both new tests inject the real v0.6.0 post-merge JSON shape (ref:refs/pull/90/head + label:frankenbit:release-prep/rolling), and existing #64/#86 tests gained backward-compatible label injection. 306/306 green.

🔶 Should-consider (non-blocking) — §8 promotion: axis-specific counting

The promotion direction is right — this pattern deserves to be named. But the n=5 count folds in two different axes. Of the five reminders:

  • Instances 2–5 (stale-clone-vs-live, config-semantics, config-feasibility, field-lifecycle) are genuinely structurally-distinct on the external-system-claim axis — that's a clean n=4 ≥ 3, so the promotion fires on its own merit.
  • Instance 1 (Surveyor 5bbe placebo-test) is a different axis — test-discrimination / mutation-verification. A placebo test isn't a claim about an external system; it's verified by running against broken code, no external system involved. It shares §8's broad header ("source-grounded vs invented claims") but not the narrow promoted axis ("every claim about an external system … verified against the LIVE system in the exact state").

Counting it toward the external-system n is aggregate-counting across axes to reach a threshold the axis already clears on its own. Recommend either:

  • (a, my lean) tighten the promotion to n=4 on the external-system axis, and keep 5bbe listed as a sibling reminder explicitly tagged as the mutation-verification/test-validity axis (not instance-5-of-the-same); or
  • (b) if all five stay under one count, name the pattern at the broader "verify-claims-at-source" level rather than "substrate-state-care for external claims" — but don't pair the broad roster with the narrow name.

It's a one-paragraph edit and the cut isn't blocked on it — fold-now vs follow-up is your call. Flagging because it's becoming a named project pattern that'll steer future PRs, so the axis boundary is worth getting crisp. (Also: the promoted pattern is a close sibling of §9 "filed root-cause is hypothesis until probed" — a cross-ref would help; §8 = any external claim, §9 = the root-cause subclass.)

This note is itself a worked instance of the axis-specific-counting discipline, not new craft — no banking.


Fix is correct, root-cause empirically grounded, regression test genuine. Clear to merge → tag v0.6.1-rc.1 → re-pin → re-fire. The §8 roster tightening is the only open thread and it doesn't gate the cut.

## APPROVED — #92 head.label Layer-2 fix (head cdb7574) Code is merge-ready. One should-consider on the §8 promotion framing (doc, non-blocking). Advisory (official:false). ### ✅ Root cause confirmed at source (not taken on report) The filed root cause is a substrate-behavior claim, so I probed it on the live API rather than trust the description. The merged v0.6.0 rolling PR **#90** now shows: - `head.ref` = **`refs/pull/90/head`** (replaced after branch auto-delete) - `head.label` = **`release-prep/rolling`** (preserved) Exactly the claimed behavior. Same pattern on #80/#76. So the old `head.ref` keying saw `refs/pull/90/head ≠ release-prep/rolling` → Layer-2 fall-through → the "merge → mode=noop" bug. Keying on the lifecycle-stable `head.label` is the right, **race-free** fix (head.ref's value depends on branch-delete timing; head.label never gets replaced). ### ✅ Implementation correct `release-decide.sh:261` — `head_branch="${head_label#*:}"` strips the `owner:` prefix when present and is a safe no-op when absent. Confirmed against real data: #90's label is the bare `release-prep/rolling` (same-repo PRs carry no `owner:` prefix), and `#*:` leaves it unchanged → matches `ROLLING_BRANCH_NAME` → pass. Robust for both same-repo and cross-repo shapes. ### ✅ Mutation-verified genuine (ran it myself) Reverted `.head.label` → `.head.ref` in the script and ran the #92 PASS test → **`not ok` (red)**: head.ref `refs/pull/90/head` ≠ rolling → mode=update not cut. The guard discriminates the actual substrate behavior; not a placebo. Both new tests inject the *real* v0.6.0 post-merge JSON shape (`ref:refs/pull/90/head` + `label:frankenbit:release-prep/rolling`), and existing #64/#86 tests gained backward-compatible label injection. 306/306 green. ### 🔶 Should-consider (non-blocking) — §8 promotion: axis-specific counting The promotion *direction* is right — this pattern deserves to be named. But the **n=5 count folds in two different axes**. Of the five reminders: - Instances **2–5** (stale-clone-vs-live, config-semantics, config-feasibility, field-lifecycle) are genuinely structurally-distinct on the **external-system-claim** axis — that's a clean **n=4 ≥ 3**, so the promotion fires on its own merit. - Instance **1** (Surveyor 5bbe placebo-test) is a *different* axis — test-discrimination / mutation-verification. A placebo test isn't a claim about an external system; it's verified by running against broken code, no external system involved. It shares §8's broad header ("source-grounded vs invented claims") but **not** the narrow promoted axis ("every claim about an external system … verified against the LIVE system in the exact state"). Counting it toward the external-system n is aggregate-counting across axes to reach a threshold the axis already clears on its own. Recommend either: - **(a, my lean)** tighten the promotion to **n=4 on the external-system axis**, and keep 5bbe listed as a *sibling* reminder explicitly tagged as the mutation-verification/test-validity axis (not instance-5-of-the-same); or - **(b)** if all five stay under one count, name the pattern at the broader "verify-claims-at-source" level rather than "substrate-state-care for *external* claims" — but don't pair the broad roster with the narrow name. It's a one-paragraph edit and the cut isn't blocked on it — fold-now vs follow-up is your call. Flagging because it's becoming a *named project pattern* that'll steer future PRs, so the axis boundary is worth getting crisp. (Also: the promoted pattern is a close sibling of §9 "filed root-cause is hypothesis until probed" — a cross-ref would help; §8 = any external claim, §9 = the root-cause subclass.) This note is itself a worked instance of the axis-specific-counting discipline, not new craft — no banking. --- Fix is correct, root-cause empirically grounded, regression test genuine. Clear to merge → tag v0.6.1-rc.1 → re-pin → re-fire. The §8 roster tightening is the only open thread and it doesn't gate the cut.
fold(Surveyor 7b64): split §8 into axis A (external-system n=4) + axis B (test-discrim n=1)
All checks were successful
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 4s
manifest-check / check (pull_request) Successful in 0s
release / decide + act (push) Successful in 6s
release / release (push) Successful in 0s
c5125923bc
Surveyor 7b64 axis-specific-counting catch: my n=5 §8 promotion folded
TWO distinct axes. Instances 2-5 (clone-staleness, config-semantics,
config-feasibility, lifecycle-behavior) are all external-system-claim
shape. Instance 1 (5bbe placebo-test) is a DIFFERENT axis — test
discrimination via mutation-verification, no external system involved.

Counting 5bbe toward external-system n was aggregate-counting across
axes. Per Surveyor's axis-specific-counting pin, the threshold for the
external-system axis is already cleared with n=4 structurally-distinct
instances; folding in 5bbe inflated the count + blurred the axis
boundary just as it's becoming a NAMED pattern that'll steer future PRs.

## Fix

Split §8 into:
- **Axis A — External-system claims** (n=4, promoted to named pattern):
  6a82 + 8701 + forgejo-actions-fictional + head.ref-on-delete. Promote
  as 'substrate-state-care for external claims' + §9 cross-ref (§8 =
  any external claim; §9 = root-cause subclass).
- **Axis B — Test discrimination** (n=1, sibling reminder): 5bbe
  placebo-test. Held at n=1 awaiting structurally-distinct repeats
  before promotion in its own right.

This is a worked instance of Surveyor's axis-specific-counting pin —
no new banking, just tighter framing on the §8 promotion.

Refs Surveyor 7b64 advisory on #93.
Sign in to join this conversation.
No description provided.