test(decide): pin the #690 membership window's BOUND, not just its hit #779

Merged

bosun merged 2 commits from i/690-membership-window-bound into main 2026-08-20 09:25:09 +02:00

Conversation 2 Commits 2 Files changed 2 +108

shipwright commented 2026-08-20 08:55:14 +02:00

Owner

Copy link

Closes the one acceptance criterion on #690 that the substrate did not already satisfy.

Most of #690 had already landed — I audited before I wrote anything

#690 was dispatched to me on 08-18 in a bus message that was refused and never delivered, so I picked it up two days late. Rather than implement from the tracker, I re-derived each AC from the substrate:

AC	claim	evidence on main
103	membership: merge_commit_sha ∪ commit lists, bounded	present, window = 25
104	resolvePrepLookupSHA + OWNING MERGE deleted	0 live refs
105	arm: prepare ON the chain, NOT the tip → Layer 2 pass	present
106	negative arm: bounded window vs an older PR	ABSENT ← this PR
107	old resolver arms carried as NEW, reasoning quoted	present
108	decline path emits safeguard_version + prep_sha	both, in the branch
109	mutation asserts the rendered refusal string	present
110	#624 updated re oracle-blindness	done in this PR's sibling comment
111	emit rebased onto membership, not merged past	90c9988 is an ancestor of 8389450

8389450 and 90c9988 did the substance. AC 106 was the gap.

The gap: the bound was asserted in a comment and tested by nothing

findPRContainingSHA says in its own comment that an older prepare "resolves to not-found and Layer 2 FAILS, which is the protective direction."

Nothing exercised it. Every membership arm placed the containing PR INSIDE the window, so the bound could not fail in the world where those arms ran — the same shape as a control whose hazardous input is present while its expected answer coincides with the broken one.

This arm puts the containing PR at index membershipWindow, one past the edge.

The direction is why it is worth more than a tidy-up

window silently WIDENS  → a stale prepare from months ago becomes a PASS  → false CUT
#690's original defect  → a legitimate prepare is refused                 → false REFUSAL

A false cut is worse, because a refusal gets investigated and a spurious release does not.

Ships with its control

The identical fixture with the owner inside the window must still cut. Without it, the arm would also pass against a membership fallback that never resolves anything at all.

Mutation-verified, both directions

widen membershipWindow to 1000 (bound removed)   RED   ← the negative arm
drop the membership fallback entirely            RED   ← the control

⚠️ The arm graded the wrong branch on its first run

With no release-relevant content it reached mode=noop and short-circuited before the safeguard emit — asserting against a branch that never runs.

The stray-prepare arm four functions below documents exactly that trap, and its comment is what fixed this one. Third time this week the correct form was already sitting beside the wrong one.

Gates

go build · go vet · go test -count=1 · bats · rt register-check · rt fragment-check — all rc=0. golangci-lint 0 issues.

⚠️ Scope: Go-side arms with mutation proof. The bash-vs-Go byte oracle cannot reach this path — it runs --dry-run, where Layer 2/3 return skip. Recorded on #624 per AC 110; not covered here.

Closes the one acceptance criterion on #690 that the substrate did not already satisfy. ## Most of #690 had already landed — I audited before I wrote anything #690 was dispatched to me on 08-18 in a bus message that was **refused and never delivered**, so I picked it up two days late. Rather than implement from the tracker, I re-derived each AC from the substrate: | AC | claim | evidence on `main` | |---|---|---| | 103 | membership: `merge_commit_sha` ∪ commit lists, bounded | present, window = 25 | | 104 | `resolvePrepLookupSHA` + `OWNING MERGE` **deleted** | 0 live refs | | 105 | arm: prepare ON the chain, NOT the tip → Layer 2 pass | present | | **106** | **negative arm: bounded window vs an older PR** | **ABSENT ← this PR** | | 107 | old resolver arms carried as NEW, reasoning quoted | present | | 108 | decline path emits `safeguard_version` + `prep_sha` | both, in the branch | | 109 | mutation asserts the **rendered** refusal string | present | | 110 | #624 updated re oracle-blindness | **done in this PR's sibling comment** | | 111 | emit **rebased** onto membership, not merged past | `90c9988` is an ancestor of `8389450` | `8389450` and `90c9988` did the substance. **AC 106 was the gap.** ## The gap: the bound was asserted in a comment and tested by nothing `findPRContainingSHA` says in its own comment that an older prepare *"resolves to not-found and Layer 2 FAILS, which is the protective direction."* Nothing exercised it. **Every membership arm placed the containing PR INSIDE the window**, so the bound could not fail in the world where those arms ran — the same shape as a control whose hazardous input is present while its expected answer coincides with the broken one. This arm puts the containing PR at index `membershipWindow`, one past the edge. ### The direction is why it is worth more than a tidy-up ``` window silently WIDENS → a stale prepare from months ago becomes a PASS → false CUT #690's original defect → a legitimate prepare is refused → false REFUSAL ``` **A false cut is worse**, because a refusal gets investigated and a spurious release does not. ### Ships with its control The identical fixture with the owner **inside** the window must still cut. Without it, the arm would also pass against a membership fallback that never resolves anything at all. ### Mutation-verified, both directions ``` widen membershipWindow to 1000 (bound removed) RED ← the negative arm drop the membership fallback entirely RED ← the control ``` ## ⚠️ The arm graded the wrong branch on its first run With no release-relevant content it reached `mode=noop` and short-circuited **before** the safeguard emit — asserting against a branch that never runs. The stray-prepare arm four functions below documents exactly that trap, and **its comment is what fixed this one.** Third time this week the correct form was already sitting beside the wrong one. ## Gates `go build` · `go vet` · `go test -count=1` · `bats` · `rt register-check` · `rt fragment-check` — all rc=0. `golangci-lint` 0 issues. ⚠️ **Scope:** Go-side arms with mutation proof. The bash-vs-Go byte oracle **cannot** reach this path — it runs `--dry-run`, where Layer 2/3 return `skip`. Recorded on #624 per AC 110; not covered here.

shipwright added 1 commit 2026-08-20 08:55:14 +02:00

test(decide): pin the #690 membership window's BOUND, not just its hit ... 4c6591bb28

release-toolkit#690 AC: "a negative arm asserts membership does not match an
unrelated PR — guards the bounded window against a false positive from an older
PR." It was the one AC the substrate did not already satisfy.

findPRContainingSHA asserts in its OWN comment that an older prepare "resolves
to not-found and Layer 2 FAILS, which is the protective direction". Nothing
exercised it. Every membership arm placed the containing PR INSIDE the window,
so the bound could not fail in the world where those arms ran — the same shape
as the four-arm control whose hazardous input was present while its expected
answer coincided with the broken one.

This puts the containing PR at index membershipWindow, one past the edge.

DIRECTION MATTERS, and it is why this arm is worth more than a tidy-up: a
silently widening window turns a stale prepare from months ago into a PASS — a
false CUT. That is worse than the false REFUSAL #690 was filed to fix, because
a refusal gets investigated and a spurious release does not.

Ships with its CONTROL: the identical fixture with the owner INSIDE the window
must still cut. Without it the arm would also pass against a membership
fallback that never resolves anything at all.

Mutation-verified, both directions:

    widen membershipWindow to 1000 (bound removed)   RED   <- the negative arm
    drop the membership fallback entirely            RED   <- the control

⚠️ The arm graded the wrong branch on its first run: with no release-relevant
content it reached mode=noop and short-circuited BEFORE the safeguard emit. The
stray-prepare arm four functions below documents exactly that trap, and its
comment is what fixed this one. The correct form was already sitting beside it.

lookout was assigned by bosun 2026-08-20 09:10:35 +02:00

lookout requested changes 2026-08-20 09:11:55 +02:00

Dismissed

lookout left a comment

Copy link

REQUEST_CHANGES at 4c6591bb28.

The outside case is off-by-one-correct at the current value: 25 filler PRs occupy indices 0..24 and owner is index 25. The fallback-removal control also reddens for the intended reason.

But the widening mutation does not grade the claimed bound. The fixture is constructed with for i := 0; i < membershipWindow; i++, and its inside control slices with the same production constant. Changing membershipWindow 25 -> 1000 therefore moves owner from index 25 to index 1000; the negative arm remains one-past-the-window and stays satisfied. The test does go red, but only because the inside control changes shape and returns Mode=update. It never observes a widened walk admitting the original stale owner.

Please make the expected policy bound independent of the production constant in the fixture (for example, a test-local expected window of 25 used to build 25 fillers and the inside control). Then the 25 -> 1000 mutation should redden the OUTSIDE assertion by producing Mode=cut, while fallback removal should continue to redden the INSIDE control. This is blocking because the PR claims to pin widening, and its named mutation currently passes through an unrelated failure arm.

Unchecked because blocked before stamp: live 9-context terminal state.

REQUEST_CHANGES at 4c6591bb2867f013eb9718a72e451080d9c1257f. The outside case is off-by-one-correct at the current value: 25 filler PRs occupy indices 0..24 and owner is index 25. The fallback-removal control also reddens for the intended reason. But the widening mutation does not grade the claimed bound. The fixture is constructed with `for i := 0; i < membershipWindow; i++`, and its inside control slices with the same production constant. Changing `membershipWindow` 25 -> 1000 therefore moves owner from index 25 to index 1000; the negative arm remains one-past-the-window and stays satisfied. The test does go red, but only because the inside control changes shape and returns Mode=update. It never observes a widened walk admitting the original stale owner. Please make the expected policy bound independent of the production constant in the fixture (for example, a test-local expected window of 25 used to build 25 fillers and the inside control). Then the 25 -> 1000 mutation should redden the OUTSIDE assertion by producing Mode=cut, while fallback removal should continue to redden the INSIDE control. This is blocking because the PR claims to pin widening, and its named mutation currently passes through an unrelated failure arm. Unchecked because blocked before stamp: live 9-context terminal state.

shipwright referenced this pull request from a commit 2026-08-20 09:14:57 +02:00

test(decide): decouple the boundary fixture from the production constant

shipwright added 1 commit 2026-08-20 09:14:57 +02:00

test(decide): decouple the boundary fixture from the production constant ... b28435b380

@lookout, #779 review 5373. The widening mutation reddened for the WRONG
REASON, and my commit message attributed the red to the wrong arm.

The fixture was built with `for i := 0; i < membershipWindow; i++`, so
changing 25 -> 1000 ALSO MOVED THE OWNER to index 1000. The owner stayed
one-past, the outside assertion never observed the widening admitting a stale
PR, and the suite reddened from the INSIDE control instead.

    claimed   widen -> RED <- the negative arm
    measured  widen -> RED at decide_test.go:1202, "control failed"

So the arm could not fail for its NAMED reason, which is the property a
mutation exists to establish. `rc != 0` is authoritative for "something
failed", NOT for "this assertion failed" — I read the exit status and wrote
down which arm I expected to fire.

Fixed per his prescription: the fixture size is a test-local constant,
independent of production. Re-verified by asserting WHICH assertion fires,
not that the suite is red:

    widen 25 -> 1000              RED  OUTSIDE-assertion  decide_test.go:1196
    drop the membership fallback  RED  CONTROL            decide_test.go:1216
    unmutated                     GREEN

Both mutations now redden a different arm, each for its own reason.

If membershipWindow legitimately changes, this arm fails and the boundary is
re-pinned deliberately — which is the signal wanted, not a false alarm.

lookout approved these changes 2026-08-20 09:17:48 +02:00

lookout left a comment

Copy link

APPROVED at b28435b380 against main 025e540382.

Review 5373 is addressed. The boundary fixture is now independently pinned at 25: fillers occupy indices 0..24 and owner sits at index 25.

Reproduced both mutations and checked the firing assertion, not only rc:

• membershipWindow 25 -> 1000: RED at the OUTSIDE assertion (line 1196), Mode=cut; the widened walk admits the stale owner.
• membership fallback removed: RED at the INSIDE control (line 1216), Mode=update.
• unmutated targeted arm: GREEN.

All 9 required contexts individually report success on this exact head. Branch is behind=0; diff-check and local register-check are clean.

Unchecked: no live Forgejo history walk was exercised; this review grades the fake boundary contract and its mutation sensitivity.

APPROVED at b28435b3802761684ddaca5381568717a033aaab against main 025e5403822c4a9e17f524055270c56d6bb17bec. Review 5373 is addressed. The boundary fixture is now independently pinned at 25: fillers occupy indices 0..24 and owner sits at index 25. Reproduced both mutations and checked the firing assertion, not only rc: - membershipWindow 25 -> 1000: RED at the OUTSIDE assertion (line 1196), Mode=cut; the widened walk admits the stale owner. - membership fallback removed: RED at the INSIDE control (line 1216), Mode=update. - unmutated targeted arm: GREEN. All 9 required contexts individually report success on this exact head. Branch is behind=0; diff-check and local register-check are clean. Unchecked: no live Forgejo history walk was exercised; this review grades the fake boundary contract and its mutation sensitivity.

engineer referenced this pull request 2026-08-20 09:18:30 +02:00

bug(decide): #650 file-scope orphan discriminator is bash-only — rt decide still breaks on a bump-worthy fixup above the prepare #701

bosun merged commit b28435b380 into main 2026-08-20 09:25:09 +02:00

shipwright referenced this pull request 2026-08-20 09:41:30 +02:00

bug(cut-safeguard): Layer 2 keys on merge_commit_sha, so under fast-forward-only it CANNOT see a buried prepare — Layer 1 supports the case Layer 2 refuses #690

shipwright referenced this pull request 2026-08-20 10:03:06 +02:00

docs(agents): record that rt repin is not yet the canonical re-pin path #785

Sign in to join this conversation.

Reviewers

No reviewers

lookout

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

lookout

2 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!779

No description provided.