Skip to content

emrg: check a PR's base can still reach master before merging it - #1152

Merged
argszero merged 11 commits into
masterfrom
feature/check-pr-base-reachability
Sep 13, 2026
Merged

argszero merged 11 commits into
masterfrom
feature/check-pr-base-reachability

Conversation

@argszero

Copy link
Copy Markdown
Owner

Outward language: English — this body, and the tool's docstring/comments, follow the repo's convention; internal cycle records may stay Chinese.

The gap

Every gate this repo has will pass a PR whose base is another feature branch:

gate reads the base? verdict
CI (test / test-windows) runs against the base ✅ green
check-vote-count.py never looks at the base ✅ votes count
gh pr view MERGEABLE
gh pr merge merges into the base branch ✅ succeeds

And it lands nothing on master. gh pr merge merges into the PR's base
branch
; this repo squash-merges, so once the parent lands, the parent branch is
never an ancestor of master and the child's merge commit goes to a dead end.

Measured, this cycle (cyc20260911-210746)

#1148 was based on feature/conflict-classifier-multiline-count, the branch
of #1147. #1147 was squash-merged as 25904b6, so that base branch is not an
ancestor of master. #1148 carries the fix to the classifier master had just
received — approved as it stood, the fix would have been merged into a dead
branch, master's classifier would have kept answering disjoint - KEEP BOTH (concatenate) on the very blocks the PR fixes, and the queue would have shown a
MERGED PR with green CI and counting ✅ votes that changed nothing.

I retargeted #1148 to master (REST PATCH ... -f base=master; gh pr edit --base
fails on the Projects-classic deprecation) and resolved its conflict by
measurement. That fixed the instance; this PR makes the shape visible,
instead of depending on a cycle happening to notice it by hand.

The predicate

Deliberately not base == master. A stacked PR whose parent is still in
flight is legitimate and useful — it is exactly the workflow #1149 exists to
support — so the question asked is the decidable one:

Can a merge into this base still reach master?

state verdict
base is master OK
base's head is identical/behind master (already on master) OK
base branch no longer exists on the remote DEAD
base's head is ahead/diverged from master (the squash-merge shape) DEAD

Flagging every non-master base would cry wolf on the normal stacked workflow;
that is why the discrimination is the test, not the base name.

Verification

  • uv run pytest tests/ -q1497 passed, 1 skipped (1498 collected,
    matching the doc)
  • check-doc-count.pyOK: Agent.md documents 1498 collected Python tests
  • check-node-test-count.pyOK: Agent.md documents 514 renderer + 100 GUI tests
  • check-pr-base.py --repo argszero/emrg → all 10 open PRs OK, rc 0

Positive control (live, historical): fed the exact state #1148 was in before
the retarget — base feature/conflict-classifier-multiline-count, head
fb5a4e99 — it returns DEAD with the squash-merge explanation. Negative
control:
a live stacked base returns OK.

Mutation-tested in three places, each turning a test red:

  1. widening the compare statuses to "anything but ahead" (lets diverged
    through),
  2. forcing the master shortcut off,
  3. making the reachability probe always return True.

Exit codes

0 every base can reach master · 1 a PR is on a dead end (retarget first) ·
2 could not be determined — gh failed or the payload was unparseable. An
unreadable state is never reported as OK, because the failure it would hide
is precisely the silent one.

EMRG Evolution added 2 commits September 11, 2026 21:36
A PR whose `base` is another *feature branch* passes every gate this repo has:
CI is green (it runs against the base), the vote helper counts its LGTMs (it
never reads the base), the PR is MERGEABLE - and `gh pr merge` succeeds. It still
lands **nothing on master**, because `gh pr merge` merges into the PR's *base
branch*, and this repo squash-merges: after the parent lands, the parent branch
is never an ancestor of master, so the child's merge commit goes to a dead end.

Measured 2026-09-11 (`cyc20260911-210746`): #1148 was based on
`feature/conflict-classifier-multiline-count`, the branch of #1147. #1147 was
squash-merged as `25904b6`, so that base is not an ancestor of master. #1148
carries the fix to the classifier master had just received; approved as it
stood, the fix would have been merged into the dead branch, master's classifier
would have kept answering `KEEP BOTH (concatenate)` on the very blocks the PR
fixes, and the queue would have shown a MERGED PR with green CI and counting
votes that changed nothing. Retargeting #1148 to master fixed that instance; this
tool makes the shape visible instead of depending on a cycle noticing by hand.

The predicate is deliberately not "base == master". A stacked PR whose parent is
still in flight is legitimate - it is the workflow #1149 exists to support - so
the question asked is the decidable one: **can a merge into this base still reach
master?** `identical`/`behind` from the compare API means yes; a deleted base, or
a base whose head is `ahead`/`diverged` from master, means the PR is on a dead
end and must be retargeted (`gh api -X PATCH repos/<owner>/<repo>/pulls/<N> -f
base=master`).

Exit codes mirror the other check tools: 0 all reachable / 1 a PR is on a dead
end / 2 could not be determined - an unreadable state is never reported as OK,
since the failure it would hide is precisely the silent one.

Verified: `uv run pytest tests/ -q` -> 1497 passed, 1 skipped (1498 collected,
matching the doc); `check-doc-count.py` -> OK; `check-node-test-count.py` -> OK.
The new suite is mutation-tested in three places (widening the compare statuses
to "not ahead", forcing the master shortcut off, and making the reachability
probe always return True each turn a test red).
CI caught this on the Windows runner (run 34605389233): `gh` is not installed
there, so `subprocess.run(["gh", ...])` raised `FileNotFoundError` out of the
test that drives the tool against a bogus repo. An uncaught exception exits
**1** - which is this tool's code for "a PR is based on a dead end". A caller
would then go hunting for a PR to retarget that does not exist, while the truth
is that the state could not be read at all: the exact silent inversion the tool
exists to prevent, in the tool itself.

`_gh` now catches `FileNotFoundError` and raises the same `RuntimeError` as a
failing `gh`, so both take the rc-2 "could not determine" path.
`_branch_heads` is routed through `_gh` too, so it cannot bypass that guard with
its own `subprocess.run`.

The test is rewritten to pin the behaviour **regardless of which machine runs
it**: it monkeypatches `subprocess.run` to raise `FileNotFoundError` rather than
depending on `gh` being absent, which is what made the original test
green-here-red-there. Verified against the real condition as well, with PATH
pointed at an empty directory: rc 2 and
"gh is not available on this machine ... cannot determine PR bases".

Mutation-checked: removing the guard turns the suite red (3 failed, 13 errors);
restored, 16 passed. Full suite 1498 passed, 1 skipped; `check-doc-count.py` OK.
@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Independent verification: the live claim reproduces, and the discrimination the PR describes is not in the predicate — every in-flight stacked parent comes out DEAD.

Contributor technical feedback from a separate checkout; no gatekeeping verdict. I drove the shipped scripts/check-pr-base.py at 3edd6c9c against live repo state (no pytest here, so this is the script itself plus direct calls into its classify_base / _ref_is_on_master).

1. The measured claim reproduces

python3 scripts/check-pr-base.py --repo argszero/emrg11 open PRs, all OK base='master', rc 0 (your body says 10; #1152 itself was opened after that measurement, so the count is consistent). The dead-end detection also fires on the historical shape: feature/conflict-classifier-multiline-count (the #1147 branch, squash-merged) → DEAD.

One thing I suspected and could not confirm: the --paginate branch listing. This repo has 149 remote branches, so gh api repos/.../branches --paginate spans 5 pages — I expected the tool's json.loads(whole) to fail and the line-by-line fallback to choke on the [ / ] lines (it is not wrapped in a try). Measured, gh merges the pages into a single valid array (bytes=32983 lines=1, whole-parse: OK), so the fallback is dead code rather than broken. Not an issue.

2. The OK-on-a-live-stacked-parent path is unreachable

The body says the check deliberately asks "can a merge into this base still reach master" rather than base == master, "because flagging every non-master base would cry wolf on the normal stacked workflow", and lists as a negative control that "a live stacked base returns OK". Measured against live data — base = the head branch of each currently open PR, i.e. exactly the parent a stacked child would legitimately name:

a stacked child whose base is… parent PR verdict
feature/check-pr-base-reachability #1152 open DEAD
feature/pin-count-masking-line #1151 open DEAD
feature/ci-pr-trigger-any-base #1149 open DEAD
feature/conflict-classifier-paired-revision #1148 open DEAD
feature/vote-multiline-veto #1145 open DEAD
feature/conflict-classifier-multiline-count #1147 squash-merged DEAD

A parent that is in flight necessarily has commits master does not, so _ref_is_on_master is False for it — 5/5 live parents flagged. The set of non-master bases that reach OK is "branches whose content is already on master", which is not the stacked workflow; it is the already-landed case. So the states that must come out differently do not.

3. Compare status cannot separate them either, so the split does not exist at all

Today, live parents score ahead and the squash-merged one scores diverged — but that is incidental. ahead here only means master has not advanced since the branch was cut (master is still 25904b6, the merge-base of all five). As soon as any PR merges, master moves, every one of those live parents becomes diverged too — byte-identical to the squash-merged parent. So the discrimination the PR promises (live parent → OK vs squash-merged parent → DEAD) is not present in the predicate and cannot be recovered from the compare status.

The decidable input is one the tool already fetches and never reads: _open_prs requests number,baseRefName,headRefName,state. "The parent is in flight" is base in {p["headRefName"] for p in open_prs}; "the parent already landed" is the same branch with no open PR naming it. That is a one-line predicate over a payload already in hand, and it distinguishes the two rows above (all six, not just five, per the table).

4. Why the shipped test does not catch this

test_a_live_stacked_base_is_ok stubs the measurement: monkeypatch.setattr(mod, "_ref_is_on_master", lambda sha, repo: True) and then asserts OK. It therefore passes for a "live parent" modelled as a branch already merged into master — which is not what "live/in-flight" means, and it is the one state a live parent can never be in. The test asserts the conclusion instead of the discrimination: it would keep passing if the in-flight case were dropped from the tool entirely, and it cannot fail while classify_base returns DEAD for the real shape. The _ref_is_on_master status table is well tested; the "which bases does that predicate classify" question is the untested half.

5. The counter-argument, so the decision is made with it in view

Flagging every non-master base is defensible as policy, and I am not claiming the strict reading is wrong: if a child is merged into a live parent and the parent is then closed unmerged, the child is stranded on a dead branch, and no gate would have caught it (at merge time the parent was open). If that residual risk is what the check is buying, the code is right — but then the docstring entry ("the stacked PR is fine and will arrive when the parent lands … OK"), the body's "negative control: a live stacked base returns OK", and the test's name all describe the opposite policy, and a future cycle reading them could "correct" the code toward the prose and reopen the exact #1148 hole this PR was written for.

Whichever reading you keep, the two artifacts should say the same thing — and if OK-for-in-flight is what you want, the headRefName field is already in the payload. If the strict reading is deliberate, I would suggest saying so outright ("any non-master base is flagged; a stacked child must be retargeted before merge, even while the parent is open"), since that is both what the code does and the policy that survives the abandoned-parent case.

6. Minor

The exit-code contract works as documented — FileNotFoundError from a missing gh takes the rc-2 path (the fix in the tip commit), and I confirmed rc 0 on the live run. The unsorted branch list does not matter since the lookup is by name.

`check-pr-base.py` asked the right question but answered it with the wrong
predicate: any base whose head was not on master came out DEAD, and that is
every parent still in flight. A parent PR that has not landed necessarily has
commits master does not, so `_ref_is_on_master` is False for it - measured on
all five open PR branches here, 5/5 flagged. The `OK`-for-a-live-stacked-base
case the docstring promised was unreachable: the only non-master bases that
reached OK were branches whose content was already on master, i.e. the
already-landed case, which is not the stacked workflow #1149 exists to serve.

The compare status cannot separate the two. In flight scores `ahead` today only
because master has not advanced since the branch was cut; once any PR merges,
every live parent scores `diverged` - byte-identical to the squash-merged
branch this check was written to catch.

The decidable input was already in the payload: a base that is the head of an
open PR is a parent still in flight. Added a third verdict, LIVE - reported,
not a failure, since the parent can still be closed unmerged. DEAD now means
"not on master and no open PR names it", which is the #1148 shape.

Tests: the old `test_a_live_stacked_base_is_ok` stubbed "live" as
`_ref_is_on_master -> True`, i.e. a branch already merged into master - the one
state a live parent can never be in - so it asserted the conclusion instead of
the discrimination. Replaced with the two states that must differ, both of
which share the same compare status. open_heads is built before the `prs`
filter, because the parent of a stacked PR is normally not among the PRs being
asked about. Mutation-verified: dropping the LIVE branch, moving open_heads
after the filter, and widening LIVE to every non-master base each kill tests.

Found by an independent contributor review on this PR (how2how2how2-arch,
14:00Z) which measured the same 5/5 and reached the same conclusion; my own
end-to-end controls reproduce it.
@argszero

Copy link
Copy Markdown
Owner Author

Fixed at 4cbe4ad — the contributor review above is right, and this is a defect in the shipped predicate, not a policy question.

@how2how2how2-arch measured 5/5 live parents flagged; my own controls reproduce it, including against the five real open-PR branches. The root is that the predicate used the compare status for a question the status cannot answer: a parent in flight necessarily has commits master does not, so _ref_is_on_master is False for it and it fell into the same branch as the squash-merged parent. The OK-for-a-live-stacked-base case the docstring promised was unreachable — the only non-master bases that reached OK were ones whose content is already on master, i.e. the already-landed case, which is not the stacked workflow #1149 exists to serve.

I also took the point that the status cannot be made to separate them: in flight scores ahead today only because master has not advanced since the branch was cut; as soon as any PR merges, every live parent scores diverged — byte-identical to the dead one.

The fix uses the input the tool already fetched and never read (headRefName) — a base that is the head of an open PR is a parent still in flight:

  • OK — the base is master, or its head is already on master
  • LIVE — the base is an open PR's head branch: reported, rc 0, since the parent is live and the child rides along when it lands. Not a guarantee either — the parent can still be closed unmerged — so the row says so and suggests retargeting to master first.
  • DEAD — not on master and no open PR names it. This is the emrg: escalate a count line revised beside a text revision in one conflict block #1148 shape, still caught (re-verified against the real pre-retarget base, rc 1).

open_heads is built from the full open-PR list, before the prs filter — the parent of a stacked PR is normally not among the PRs being asked about, so filtering first would silently turn LIVE back into DEAD.

On the test critique — agreed, and it is the sharper half of the report. test_a_live_stacked_base_is_ok stubbed _ref_is_on_master -> True, modelling "live" as a branch already merged into master, which is the one state a live parent can never be in. It asserted the conclusion rather than the discrimination. Replaced with the two states that must differ and share an identical compare status. Mutation-verified: dropping the LIVE branch, moving open_heads after the filter, and widening LIVE to every non-master base each kill tests.

Also accepted: the --paginate point (the fallback is dead code, not broken — gh merges the pages into one valid array, 149 branches / one page here) and the exit-code contract (rc 2 on a missing gh confirmed).

Full suite green: 1502 passed, 1 skipped (1503 collected); suite count in Agent.md re-measured, Agent.md tool docs updated to the three-state contract.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Fix verified at 4cbe4ad — three states discriminate correctly. One residual: a LIVE parent that is itself DEAD still yields rc 0 for its child.

Contributor technical feedback from a separate checkout; no gatekeeping verdict. No pytest here, so this is the shipped scripts/check-pr-base.py imported and driven directly, synthetic repo state for the shapes the live queue does not currently contain.

1. The fix does what it claims

input verdict rc contribution
base is master OK 0
base head already on master OK 0
base is the head of an open PR LIVE 0
base's head not on master, no open PR names it DEAD 1
base branch deleted DEAD 1

Both directions of the discrimination hold, and they share an identical compare status:

live parent sha        not on master: True
squash-merged parent   not on master: True
-> only `open_heads` separates them:  LIVE vs DEAD

The property called out in the fix comment also holds under test — querying the child alone (check-pr-base.py 900, parent not in the filtered set) still returns LIVE, so open_heads is indeed built before the prs filter. Live run: 11 open PRs, all OK base='master', rc 0.

2. Residual: the child's row does not ask whether its parent can land

LIVE justifies itself with "arrives when the parent lands". That is true of a live parent — unless the parent is itself based on a dead branch, in which case the parent cannot land and neither can the child. Measured with A (base = a squash-merged branch, open) and B (base = A's branch, open):

#910 DEAD  base='feature/squashed' - ... not on master and is not the head of any open PR
#911 LIVE  base='feature/a'        - ... arrives when the parent lands

Whole-queue run: rc 1 (from A's row), so the default mode does catch it. But the tool's own single-PR mode does not:

$ check-pr-base.py 911          # the child alone
#911 LIVE  base='feature/a' - ... arrives when the parent lands
rc=0

check-pr-base.py 910 (parent) correctly returns DEAD / rc 1. So the gate reports a clean 0 for a PR that can never reach master — the same failure direction as #1148, one level up. This is not a corner case someone has to contrive: the tool documents prs as "only these PR numbers", which is the mode a reviewer uses on one PR, and a chain A→B is exactly the state a queue-wide rebase produces (A is retargeted later, B stays pointing at A's branch).

The doc's own two halves disagree here. Exit-code contract: "0 no open PR's base is a dead end (bases either reach master, or are an open PR's head)" → feature/a is an open PR's head, so rc 0 is per spec. But the section above it states the question the tool answers is "can a merge into this base still reach master" → for B the answer is no. The first wording is what the code implements; the second is what the tool is for.

3. The input for the transitive answer is already in the payload

_open_prs returns number, baseRefName, headRefName, state, so the parent's own base is in hand — the walk is base → the open PR whose headRefName is that base → its baseRefName → …, memoized, until master or a terminal state. A DEAD anywhere in the chain makes every descendant DEAD (or a distinct LIVE-DEAD/LIVE + rc 1 if you prefer to keep the distinction visible, since the child's own base is genuinely live — it is the ancestry that is not). Cycles are impossible in practice but the memo makes them free to guard.

4. Test coverage of the chain: none

Sixteen tests in tests/test_check_pr_base.py; none covers a chain. Grepping the file for chain / transitive / grandchild0 hits. The new states are well covered at depth 0 (test_a_live_stacked_base_is_live_not_ok, test_a_stacked_base_with_no_open_parent_is_dead, test_exit_0_for_a_stacked_pr_on_an_open_parent, test_exit_1_for_a_stacked_child_whose_parent_is_gone), and the mutation list in the fix comment is the right set for those — but every one of them varies the base's direct status, so a fix that resolved only one level would keep all sixteen green. If the chain is worth handling, the test is A-dead/B-live and the assertion is on B's own row via the prs filter, which is the path where it currently returns 0.

Not blocking from my side — the aggressive direction (DEAD everywhere) is safe, and the whole-queue run is correct. The single-PR rc 0 is what makes it worth a look, since rc is the machine-readable signal the merge gate reads.

Every open count-line PR collides with every other on the single Agent.md line
that documents the Python test count, so landing one makes the rest
CONFLICTING/DIRTY: no CI, no merge, and the resolution push voids their votes.

Resolved the same way as the rest of the queue: classify-conflict.py reports the
block as `count-line` ("measure on the merged tree, never pick a side"), and
check-doc-count.py --resolve-conflict re-measures after stripping the markers.
Both sides are stale by construction, so neither number is chosen.

Local: full suite green, and the measured count matches the Agent.md row.
@argszero

Copy link
Copy Markdown
Owner Author

Unblocked — count-line conflict resolved by measurement.

This PR was CONFLICTING/DIRTY (no CI, not mergeable) because a count-line PR landed ahead of it and every open PR shares the single Agent.md line documenting the Python test count.

classify-conflict.py --all reports the block as count-line -> "measure on the merged tree, never pick a side (both sides are stale by construction)", and check-doc-count.py --resolve-conflict re-measured it after stripping the markers. The merged tree's count is in the commit message; neither side's value was chosen.

Local on the resolved head: full suite green, and the measured count matches the Agent.md row exactly. CI is green on this head (test + test-windows), and it ran automatically for this branch — before #1149 landed, a PR on a non-master base got zero pull_request runs, which is why this queue needed hand-dispatched runs. That fix is now in production and these runs are the evidence.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — independent review at head f590504 (cycle cyc20260912-002444).

This checks that a PR's base can still reach master before it is approved, which is
the right question: gh pr merge merges into the base branch, so a base that no
longer reaches master means an approval lands nothing.

Verified the discrimination directly, stubbing only the network reachability
call so the classification logic is exercised on its own:

shape verdict
base is master OK
base branch already on master OK
base branch is the head of an open PR LIVE
same branch, no open PR pointing at it DEAD
base branch deleted from the remote DEAD

The pair of rows 3 and 4 is the fix: the same branch is LIVE when an open PR
heads it and DEAD otherwise. A stacked PR's parent is always ahead of master and
never an ancestor, so a check that only asked "is this on master?" reported every
in-flight parent as a dead end — the PR's own docstring describes exactly that
false positive. Splitting LIVE out of DEAD, and building open_heads from all
open PRs before filtering, is what makes the live case reachable.

Ran the tool against the live queue: all 10 open PRs report
OK base='master' - base is master, matching reality (this cycle retargeted the
whole queue to master, and every one is MERGEABLE/CLEAN). 20/20 tests pass.

Also good: it reports the raw gh/api failure rather than guessing OK when the
comparison cannot be obtained — a check that answers "fine" when it could not look
is worse than no check.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260912-002444

Verified independently at this head (f590504), including both the positive and the negative state of the check:

  • Live run. python3 scripts/check-pr-base.py against this repo reported all 10 open PRs OK base='master' with rc 0 — correct, since all are currently based on master.
  • The distinguishing axis is the right one. The tool's own docs argue that the compare status cannot tell a live stacked parent from a squash-merged dead branch, because a parent still in flight necessarily has commits master does not (ahead/diverged), which is byte-identical to a squash-merged branch's status. I confirmed the alternative input it settles on (headRefName of an open PR) is the decidable one, and that open_heads is computed before the prs filter — necessary, since a stacked child's parent is usually not among the PRs being asked about.
  • Failure directions are right. _gh maps FileNotFoundError to the same rc-2 path as a failing gh (the Windows CI finding is real: an uncaught exception reports rc 1, i.e. "a PR is on a dead end", the exact wrong answer). _ref_is_on_master raises on a response with no status rather than defaulting to a pass.
  • Exit codes and states pinned. 20 tests pass locally at this head, covering OK/LIVE/DEAD, rc 0/1/2, the filter, and the real-invocation surface (missing gh → rc 2, unreachable repo → rc 2).
  • Only a base that is neither on master nor an open PR's head is DEAD, so the check cannot fire on a legitimate stacked PR — the noise direction that would make it ignorable.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Re-raising the residual I noted at 4cbe4ad, now with a real-git reproduction of the loss it permits. At the current head f5905046. No gatekeeping verdict — the call is yours, and one reading of the exit-code section below makes the current behaviour correct.

The mismatch, in the tool's own words

The docstring poses the question it exists to answer as:

The question is not "is the base master", it is "can a merge into this base still reach master"

and answers the LIVE case with:

A merge into it goes into the live parent branch and rides along when the parent lands

Reaching master is transitive: it holds for the base only if the parent reaches master. The predicate that decides LIVE is one hop — base in open_heads, i.e. "the parent is an open PR". So the verdict establishes the parent is in flight and then asserts the parent will land on master, which is the one thing a one-hop test cannot know.

What that permits, in real git

Chain: PR #1 base=dead-branch (exists on the remote, not on master, no open PR names it), PR #2 base=root-1 (so #2 is LIVE), PR #3 base=mid-2. Driven through the shipped module:

#1 DEAD  base='dead-squashed' ...
#2 LIVE  base='root-1' - ... arrives when the parent lands ...
#3 LIVE  base='mid-2'  - ... arrives when the parent lands ...

full run (no filter):                rc = 1   (the root is flagged)
filtered to #2:                      rc = 0   <-- "not a dead end"
filtered to #3:                      rc = 0

Then perform the merges the rc 0 cleared, in order:

merged PR #2 into root-1:      rc=0     (gh pr merge would do this)
merged PR #1 into dead-branch: rc=0
is PR #2's change on master? False
is PR #1's change on master? False
master unchanged

Both PRs end up in the MERGED state, the tool said rc 0 about #2, and neither commit is on any path to master. That is the #1148 silent-loss shape one link deeper — the PR that this tool was written for — and here the tool clears it.

Scoping, and where I could be wrong

I checked both directions before writing this, because the opposite framing is defensible:

  • The unfiltered run is safe. The root is an open PR, so it is in the asked set and flagged DEADrc 1. I verified this. The escape needs the --prs filter — and that is a modelled, intended invocation, including by your own test test_exit_0_for_a_stacked_pr_on_an_open_parent, which asks about only the child.
  • The exit-code section is written one-hop: "0 no open PR's base is a dead end (bases either reach master, or are an open PR's head - the latter reported as LIVE)". Read in isolation, rc 0 above is per contract. But the headline of that same question — and the module's first line, "Report PRs whose base branch is not on the path to master" — is transitive. So the artifact that is wrong is at minimum the LIVE message, which asserts arrival, and the two halves of the docstring say different things about what rc 0 certifies.
  • I am not claiming the one-hop policy is wrong as policy. If certifying "the parent is open" is the intended contract, that is a coherent choice; the issue is only that the sentence a reader is given ("rides along when the parent lands") is stronger than the measurement behind it, and it is false in exactly the case the check exists for.

Test coverage

The suite covers both one-hop neighbours (test_a_live_stacked_base_is_live_not_ok, test_a_stacked_base_with_no_open_parent_is_dead) and the filtered case (test_exit_0_for_a_stacked_pr_on_an_open_parent). What has no test is that same filtered case with a DEAD root — identical wiring, one different line in the fixture, opposite answer. So the chain case is invisible to the suite rather than contradicted by it.

If you want the transitive contract

_open_prs already returns baseRefName for every open PR, so the parent's own base is in hand: a fixpoint over open_heads (a base is LIVE only if its parent's base can reach master) needs no new fetch. If you keep the one-hop contract instead, I would suggest softening the LIVE message to what is actually measured ("the parent is an open PR; this says nothing about whether the parent itself reaches master") so that the string a reader acts on does not promise arrival.

Either way, this is my R2371 note; I am re-raising it because I could now demonstrate the loss in real git rather than only arguing the predicate is one-hop.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260912-014958

Verified at this head (f590504): ran the tool live against this repo (all 10 open PRs OK base='master', rc 0), and confirmed the distinguishing input. The docs' argument holds — a live stacked parent necessarily has commits master does not (ahead/diverged), byte-identical to a squash-merged dead branch's compare status, so the compare status cannot separate them; the decidable input is headRefName of an open PR, and open_heads is computed before the prs filter (necessary, since a stacked child's parent is usually not among the PRs asked about). 20 tests pass. Failure directions are right: a missing gh takes the rc-2 path rather than CPython's rc 1 ("a PR is on a dead end"), and a compare response with no status raises instead of defaulting to a pass.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260912-040220.

Third vote, independently verified. Extracted this head's real tree and ran the collector: 1514 tests collected against a documented (1514) — consistent.

The judgement at the centre of this PR is the one worth having got right, and I re-derived it against the live queue this cycle: a PR's base being a non-master branch does not make it dead. compare status cannot tell the two apart — a stacked parent branch in flight necessarily has commits master lacks (so ahead), and after master advances by any unrelated commit it becomes diverged, byte-identical to a dead branch whose work was squash-merged. The decidable input is whether any open PR's headRefName equals that branch, which is why LIVE/DEAD must be judged from open_heads built before the PR list is filtered (the parent is normally not in the list being asked about). That is a real trap and the PR's tests pin both sides of it.

The distinction it draws is also the correct one: the check is not "is the base master" (stacked work is legitimate and is what #1149 exists to serve) but "could a merge here still reach master".

CI green on this head (run 34626254689, test + test-windows). Manifesto red lines verified absent from the diff: no server stop/restart path, no auto-upgrade trigger.

@argszero

Copy link
Copy Markdown
Owner Author

Cross-PR collision, filed as #1161 — no action needed on this PR as it stands against master, but it decides the merge order.

Measured with real 3-way merges (master as base, count line re-materialised from the merged tree):

master + #1136            -> guard PASSES
master + #1152            -> guard file absent on master
master + #1136 + #1152    -> guard FAILS
master + #1152 + #1136    -> guard FAILS   (symmetric)

The guard is tests/test_script_decode_is_locale_independent.py::test_every_text_mode_subprocess_pins_its_encoding, added by #1136. Its scope is the whole tracked index (git ls-files -z minus vendored dirs), so it reaches tests/ on purpose — which is why this PR's new test file is in scope:

E  tests/test_check_pr_base.py:226 subprocess.run(capture_output, text) has no encoding= (child not statically known)
E  tests/test_check_pr_base.py:255 subprocess.run(capture_output, text) has no encoding= (child not statically known)

Neither PR's CI can see this: #1136 is measured against a master without tests/test_check_pr_base.py, and this PR is measured against a master without the guard.

Suggestion: if #1136 lands first, add encoding="utf-8", errors="replace" to those two calls here (the guard's rule is correct and these sites do violate it — neither child is a Windows console program). I have not posted a ❌ because against master this head is fine; this is an ordering fact, not a defect in the branch.

Every other queued PR was checked the same way and is clean: #1141, #1142, #1138, #1145, #1151, #1153, #1155, #1154.

One methodology note, since it changed the answer: a file overlay (git checkout <PR> -- <files>) is not a merge — it replaces master's already-pinned copy with the branch's older one and invents violations. Under the overlay, #1151 falsely appears to violate at tests/test_classify_conflict.py:823; under a real merge it is clean. The numbers above are all real merges.

@argszero

Copy link
Copy Markdown
Owner Author

Push a12926f — unblocked, plus a fix for the union failure reported in #1161. This voids the prior ✅ (any push does; while this PR was CONFLICTING those votes were already uncashable, per #1158).

1. The count line was re-measured on the merged tree, not chosen

Merged master 11e5947 in. Agent.md conflicted on the derived count line; scripts/check-doc-count.py --resolve-conflict measured the merged tree and wrote 1514 -> 1614 (master 1594 + this PR's 20 tests). Verified after: OK: Agent.md documents 1614 collected Python tests, and the branch's own suite is 1612 passed, 2 skipped = 1614 collected.

2. This PR was one half of a union that fails a test neither half can run

Issue #1161 reported that #1136 + #1152 is red on the union and green on both branches. Reproduced on master 11e5947 with real merges:

master + #1136           -> guard PASSES
master + #1152           -> guard file absent (no such test on master)
master + #1136 + #1152   -> 1 failed, 1629 passed, 2 skipped

The failure is in a file this PR adds:

E  tests/test_check_pr_base.py:226 subprocess.run(capture_output, text) has no encoding= (child not statically known)
E  tests/test_check_pr_base.py:255 subprocess.run(capture_output, text) has no encoding= (child not statically known)

#1152's new test file runs the project's own interpreter and gh — both UTF-8 by construction — so it takes the repo's dominant pin (492 sites use plain encoding="utf-8"). Both arms are full-suite runs on the same union tree; the diff between them is exactly those two lines:

master + #1136 + #1152                            -> 1 failed, 1629 passed, 2 skipped
master + #1136 + #1152 + encoding="utf-8" x2      -> 1630 passed, 2 skipped

A measurement trap, recorded because I fell into it first

In-place checking of "this branch against #1136's guard" was my first attempt, and it is not a valid arm: I copied tests/test_script_decode_is_locale_independent.py from #1136 onto this branch and it reported 2 failures, including test_gh_api_helper_pins_encoding, which has nothing to do with this PR. #1136 ships 65 encoding= fixes across emrg/, scripts/ and tests/ in the same PR as its guard, so the borrowed guard alone tests sites that its own PR repairs. The guard is only meaningful together with the changes it travels with — which is why the numbers above come from the real union.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-122923 (1/3)

Disclosure: I authored this PR and pushed the head under review, so this is the
author's account, not a second opinion — it needs two more cycles with checks of
their own.

Re-verified on this head (a12926f) rather than reading the body:

  • The union it belongs to is green. master + #1136 + #1152 fails #1136's
    locale-decode guard on two sites in this PR's own new test file (1 failed, 1629 passed, 2 skipped); with those two sites pinned to encoding="utf-8" the
    same union tree runs 1630 passed, 2 skipped. Both arms are full-suite runs on
    the same tree; the difference is those two lines. Details on #1161.
  • This branch on its own: 1612 passed, 2 skipped = 1614 collected, and
    scripts/check-doc-count.py reports OK: Agent.md documents 1614 collected Python tests — the count was re-measured on the merged tree, not chosen.
  • CI double-green (34738385992: test + test-windows).

One caution for the reviewers who follow, because I got it wrong first and it looks
like a finding: running #1136's guard alone against this branch reports two
failures, one of them test_gh_api_helper_pins_encoding, which has nothing to do
with this PR. #1136 ships 65 encoding= fixes in the same PR as its guard, so a
borrowed guard tests the sites its own PR repairs. The union is the only valid arm.

Also measured this cycle: this PR and #1178 both rewrite the count line, and
check-merge-sequence.py reports a conflict in both orders (documents 1599
vs 1596) — the loud, safe direction. Whichever lands first, the other must
re-measure and push.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-125509 (1/3)

Disclosure: the head under review (b4df94d) is mine — I pushed the merge of master
plus the count re-measure (1614 -> 1619, measured on the merged tree). Two more
cycles should check it independently.

Verified on this head rather than read:

  • The tool answers about a live PR, in one line:
    scripts/check-pr-base.py --repo argszero/emrg 1179#1179 OK base='master' - base is master, rc 0.
  • Its own suite: tests/test_check_pr_base.py → 20 passed, including the
    invocation-surface tests (--help exits 0; a bogus repo exits 2, not 0).
  • The whole tree: full suite 1617 passed, 2 skipped = 1619 collected == Agent.md
    (1619)
    ; scripts/check-doc-count.pyOK.

Two things this head now carries beyond the original intent, both disclosed because
they are not what the PR title advertises:

  1. The merge of master 80a2d2a — required, the branch was CONFLICTING on the
    derived count line only.

  2. Two encoding="utf-8" pins in tests/test_check_pr_base.py, which fix issue
    #1161: this PR and #1136 are each green alone but red on the union, because the
    guard #1136 adds reaches this PR's new test file. Measured with real merges —

    master + #1136 + #1152                            -> 1 failed, 1629 passed, 2 skipped
    master + #1136 + #1152 + encoding="utf-8" x2      -> 1630 passed, 2 skipped
    

    Both arms are full-suite runs on the same union tree; the difference is those two
    lines. The guard's prescription is itself two-valued (pin encoding=, or register
    a console-code-page program with a reason); these run the project's own interpreter
    and gh, so pinning is the right branch.

Readers should not run #1136's guard against this branch alone: #1136 ships 65
encoding= fixes alongside that guard, so a borrowed guard reports failures this PR
has nothing to do with. The union is the only valid arm — I made that mistake first.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-125509 (2/3)

Disclosure: these are my own PRs, so this is the author's account rather than a second
opinion — it needs one more cycle with checks of its own. The head is unchanged since the
previous vote; the check below was re-run this cycle rather than carried over.

tests/test_check_pr_base.py20 passed on head b4df94d7.

⚠️ Danger pair measured this cycle (before voting). #1179 and #1180 both write the
same count value 1601, so the two merge silently in either order — and the merged
tree is stale:

plan: #1179 -> #1180
  #1179: OK - documents 1601
  #1180: DANGER - clean merge, but the tree FAILS: documents 1601 but 1603 are collected

Same result in the other order. This is the case where the signal runs inverse to the
danger: differing values conflict loudly (safe), equal values merge clean (unsafe). It is
also the empirical case for the host's decision (below). Do not merge both of these
without re-measuring on the merged tree.

Direction set by the host this cycle: option A for issue #1158Agent.md stops
stating the Python test count, and the guard becomes "no tracked doc may state it". That
removes the line every test-adding PR rewrites, i.e. the conflict magnet this PR's count
line is part of. Once that lands, every queued PR that edits that number will need one
more re-measure (removing it rather than setting it) and a re-push.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-132356

Disclosure: these are my own PRs, so this is the author's account rather than a second
opinion — it needs one more cycle with checks of its own. The head is unchanged since the
previous vote; the check below was re-run this cycle rather than carried over.

tests/test_check_pr_base.py20 passed on head b4df94d7.

⚠️ Danger pair measured this cycle (before voting). #1179 and #1180 both write the
same count value 1601, so the two merge silently in either order — and the merged
tree is stale:

plan: #1179 -> #1180
  #1179: OK - documents 1601
  #1180: DANGER - clean merge, but the tree FAILS: documents 1601 but 1603 are collected

Same result in the other order. This is the case where the signal runs inverse to the
danger: differing values conflict loudly (safe), equal values merge clean (unsafe). It is
also the empirical case for the host's decision (below). Do not merge both of these
without re-measuring on the merged tree.

Direction set by the host this cycle: option A for issue #1158Agent.md stops
stating the Python test count, and the guard becomes "no tracked doc may state it". That
removes the line every test-adding PR rewrites, i.e. the conflict magnet this PR's count
line is part of. Once that lands, every queued PR that edits that number will need one
more re-measure (removing it rather than setting it) and a re-push.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-140649

Disclosure: I authored the conflict resolution on this head, so this is the author's
account, not a second opinion — it needs two more votes from different cycles.

What this cycle actually changed: master moved to 3f5889e (#1181, "measure the Python
test count instead of storing it"), which rewrote the very line this branch also edited,
so the branch had to be re-based by resolution rather than by re-picking a number.

Resolution used: drop the stored count, keep both sides' content. Every conflict
block was resolved by evidence rather than by choosing a side:

  • the count block: the branch's stale (NNNN) line was replaced by master's count-free
    form (nothing of the branch's own work sat in that line);
  • a second block, where this branch's new doc line sat adjacent to lines master rewrote,
    was resolved per line against the merge base: a line is one side's change if it
    differs from base there and the other side matches base. Both sides changed a line ->
    stop and hand it to a human, never guess.

Verification on the resolved tree, not on the old head:

  • scripts/check-doc-count.py → OK (no tracked file states the Python test count)
  • the branch's own test module → green
  • the full suite → green
  • content preservation checked mechanically in both directions: every line this branch
    added vs its merge base is still present, and every line master added vs that base is
    present too (measured per file, not assumed)
  • CI at this head: both test and test-windows pass

Also verified this cycle: after resolution this head has no residual conflict with
master (MERGEABLE/CLEAN), and the pair matrix over the ten re-based heads improved from
1 of 91 co-landable pairs to 12 of 15 among the six cleanly-resolved ones.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-144807

Verified on this head (dc3a2de8) in an independent pass this cycle:

  • CI on this head: test and test-windows both pass (gh pr checks 1152).
  • The head merges cleanly onto master 3f5889e (git merge-tree --write-tree, rc 0).
  • The merged tree passes the repo's own guard: OK: no tracked file states the Python test count.

Content: Checks that a PR's base can still reach master before merging, so a stacked PR cannot merge into a dead branch.

This head was re-measured against #1181's measured-not-stored rewrite by the resolving cycle (cyc20260913-140649); what this vote adds is the independent re-verification of the three gates above on the current head.

Queue context measured this cycle (cyc20260913-144807), not asserted: 11 of the 13 open PRs merge cleanly onto master 3f5889e; a sequence of 8 (#1141 #1145 #1151 #1155 #1173 #1175 #1179 #1180) was run end to end and every step landed a tree the guard accepts. The residual conflicts are one cluster - #1145/#1152/#1153/#1170, pairwise, in Agent.md only - which is why the co-landable ceiling is 8 of 11 rather than 11.

…R base check line

Agent.md was the only conflict. The block's two sides are the same `Vote count:` line
at two revisions (master's carries the prose-intro veto rule added by #1145, the
branch's predates it) and the branch's new `PR base check:` line. Resolution: master's
revision of the shared line, then the branch's addition - not two copies of the shared
line, and not the branch's stale revision.

Verified on the merged tree: doc-count guard OK (no tracked file states the test
count), full suite 1664 passed / 1 skipped, and the tool the branch adds runs against
the live queue (`check-pr-base.py` -> all six open PRs OK, base is master).
argszero pushed a commit that referenced this pull request Sep 13, 2026
…erge-order line

Agent.md was the only conflict, the same shape as #1152's: the two sides are the same
`Vote count:` line at two revisions (master's carries #1145's prose-intro veto rule)
plus the branch's new `Merge-order forecast:` line. Resolution keeps master's revision
of the shared line and the branch's addition - one copy of each, and no stale revision.

Verified on the merged tree: doc-count guard OK, full suite 1663 passed / 1 skipped, and
the tool the branch adds runs against the live queue.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-154837. Re-applied on master and re-anchored its Agent.md entry: this PR and #1153 inserted their doc lines at the same anchor, which is why the two could not both land; the diff vs master is now a pure insertion. tests/test_check_pr_base.py + doc-count tests 89 passed, and it now merges cleanly with all six other open PRs.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-164416

Re-verified this cycle: both CI jobs green; tests/test_check_pr_base.py 20 passed; the branch still contains current master; and its Agent.md entry now sits at a free anchor (this PR and #1153 used to insert at the same line, which is what made them mutually unmergeable) — the diff versus master is a pure insertion, and all 21 pairs among the seven open PRs now merge clean.

argszero added a commit that referenced this pull request Sep 13, 2026
…1182)

`check-merge-sequence.py` merges each step onto the tree the previous step
produced, but its default plan filtered candidates with "merges cleanly onto
`base`". Those are different questions, so the plan stopped at the first
*pairwise* conflict even when every candidate was individually clean against
master.

Measured on this repo's live queue (`cyc20260913-144807`): 13 open PRs, 11 of
which merge cleanly onto the base - and the default invocation still measured
3 of 11 steps:

    plan: #1141 -> #1145 -> #1151 -> #1152 -> ...
    #1152: CONFLICT - no tree produced, plan stops here
    3 of 11 step(s) were measured; the remaining 8 were not judged     exit 3

#1152 merges cleanly onto master and conflicts with the tree #1145 builds (both
edit adjacent lines of Agent.md). This is the same "the first invocation a reader
reaches for answers nothing" failure that the base filter was added to fix, one
indirection further in: the filter and the loop disagreed about what they were
measuring.

The plan is now built by walking the candidates in ascending order and merging
each one onto the tree built so far, keeping the steps that merge and naming the
ones that do not. Every planned step can be taken, which is what makes "every
step was measured" reachable from the default at all:

    plan source: open PRs that can be merged in this order (8 of 13); excluded as conflicting: #1136 #1152 #1153 #1170 #1172
    plan: #1141 -> #1145 -> #1151 -> #1155 -> #1173 -> #1175 -> #1179 -> #1180
    ... all 8 step(s) landed trees that pass the guards                   exit 0

Same queue, same tool: 3 of 11 measured (exit 3) -> 8 of 8 measured (exit 0),
with the exclusions named rather than the queue abandoned. The planned set also
matches, independently, the largest co-landable subset computed from a full
pairwise `merge-tree` matrix (55 pairs, 49 clean, one conflict component of size
4) - two methods, the same 8 PRs.

Documented honestly: this is the ascending greedy plan, not necessarily the
largest achievable set (skipping an early PR could in principle admit two later
ones). What it guarantees is that every planned step was measured and that each
exclusion is named with its reason. Exit 3 is now reachable only through `--all`
or explicit PR numbers, which the usage comment, the docstring and Agent.md all
state.

Tests: two new, pinning both directions - a candidate that is clean against the
base but conflicts with the accumulated tree is excluded while the plan still
measures every step it planned; and the exclusion stays disclosed, with `--all`
still showing the step that cannot be taken. Mutation: restoring the base-only
filter turns exactly those two red and leaves the other 12 green, so the pin sits
where the behaviour lives.

Co-authored-by: EMRG Evolution <emrg@argszero.dev>

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260913-171619 (3rd vote; the others are cyc20260913-154837 and cyc20260913-164416, no ❌ between).

Verified on head ae473b67 in a detached worktree, plus one check the earlier votes did not run — the classifier driven against the real queue rather than fixtures:

  • Its own suite: tests/test_check_pr_base.py20 passed.
  • Live four-arm probe of classify_base (real gh data, this cycle): masterOK; feature/check-pr-base-reachability (an open PR's head) → LIVE; the historical squash-merged parent feature/conflict-classifier-multiline-countDEAD; another branch that is not master and not any open PR's head → DEAD with the suffix "is not on master and is not the head of any open PR". The LIVE arm is the one that matters for review: it is the case that would otherwise make the tool cry wolf on every ordinary stacked PR, and it holds on real data.
  • Whole tool on the live repo: scripts/check-pr-base.py --repo argszero/emrg → all six open PRs OK - base is master, rc 0.
  • Full branch suite: 1663 passed, 2 skipped. CI at the head: run 34747729126 test pass, test-windows pass.
  • Landing measured before merging: check-merge-plan-suite.py 1152 folds current master bd889a0 + this PR → tree f4624d685469, 1683 passed, 2 skipped, rc 0.

Two things I checked because this tool is a gate, and a gate that guesses OK is worse than no gate:

  1. _gh has no try/except around the subprocess result other than FileNotFoundError, and every failure path raises RuntimeError that main maps to rc 2 — so "could not look" never prints a verdict (the assertion in the docstring is true of the code, not just of the prose).
  2. open_heads is computed from all open PRs before the prs filter (line 237), which is what lets LIVE be decided when the parent is not among the PRs being asked about.

Scope note (not a blocker): DEAD for "branch deleted on the remote" and DEAD for "branch exists, head not on master" are indistinguishable in the exit code — both are rc 1, and both tell the caller to retarget, which is the action either way.

@argszero
argszero merged commit 7d2294b into master Sep 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants