Skip to content

emrg: judge the tree a plan lands with the full suite, not one guard - #1186

Merged
argszero merged 1 commit into
masterfrom
feature/plan-final-tree-suite
Sep 13, 2026
Merged

emrg: judge the tree a plan lands with the full suite, not one guard#1186
argszero merged 1 commit into
masterfrom
feature/plan-final-tree-suite

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes the gap measured on the current queue (issue #1161): every gate in this repo answers a question about one PR, and per-PR CI structurally cannot see an interaction between two of them.

The measurement

Seven open PRs, each MERGEABLE, each CI double-green. Every ordered pair merged cleanly (git merge-tree --write-tree --merge-base=...: 21/21), and the doc-count guard accepted every step of the resulting plan. The tree the seven produce together failed the suite:

FAILED tests/test_script_decode_is_locale_independent.py::test_every_text_mode_subprocess_pins_its_encoding
   tests/test_check_merge_sequence.py:318 subprocess.run(..., text=True) has no encoding= ...

The guard shipped in #1136 rejects three calls shipped in #1172. Neither PR can see it: a pull_request run builds the merge of that head into its merge-base, so #1136's CI contains no #1172 code and #1172's CI contains no guard. Both green; the tree that would reach master red.

What this adds

scripts/check-merge-plan-suite.py takes several PRs, folds them into the tree they would land (order as given; merge-tree --write-tree + commit-tree, never touching the working tree or a branch), and runs the repository's full test suite on that tree. It also judges a single PR, where it answers the same question more completely than one guard script does — one guard script is exactly what was green while the tree was red.

Two article decisions worth reviewing:

  • A real worktree, not an archive. An extracted archive has no .git, this repo's tests resolve paths through git, and an archive harness therefore reported 8 failures on master's own tree too — identical on every input, i.e. a device measuring itself instead of the plan. A worktree distinguishes the states (control 1600 passed, planned tree 1643 passed on the same harness).
  • sys.executable for the suite. A freshly added worktree has no populated .venv (measured repeatedly here), so the suite runs with the interpreter running the tool, cwd = the worktree.

Exit codes: 0 final tree built and suite passed · 1 built and suite failed (the finding) · 2 unanswerable (git or gh failure, or the suite cannot run at all — never reported as healthy) · 3 the plan has no final tree because a step conflicts (not a health verdict). The output names the measured tree and the plan order.

Tests

tests/test_check_merge_plan_suite.py, hermetic (local repos plus a local bare remote exposing the pull-request refs, no network): a guard test arrives in one PR and the token it forbids in another — each green alone, the plan red; a reordered plan is reported in the caller's order; a conflicting step reports no final tree and moves which step it is when the plan is reversed; the suite runs where git metadata exists; an unanswerable suite is not healthy, with the runnable-suite arm showing the same plan then passes, so that arm fails for the reason claimed.

Six mutants, each killed by its owning test: archive instead of worktree · failing suite misclassified as unmeasurable · each PR judged against base instead of folded · plan sorted instead of folded · conflict reported as a health failure · unanswerable reported as healthy. File restored byte-identical after the run.

Verification on this head

  • pytest tests/ → 1649 passed, 1 skipped; from emrg.client.app import run_client and python -m emrg --help green.
  • Run against the seven real open PRs it builds #1136 -> #1152 -> #1153 -> #1170 -> #1172 -> #1182 -> #1185, names final tree 2579fb649a48, and reports the suite OK — the same tree the union probe measured. Order is honoured: the explicit-order and default-order invocations agree.
  • The Agent.md entry sits at a free anchor: this branch merges cleanly with all six in-flight PRs (measured with merge-tree, not assumed) — same-anchor insertion is what collided three PRs earlier today.
  • Passed the in-flight class guard (tests/test_script_decode_is_locale_independent.py, emrg: extend the locale-decode guard into the emrg/ package and fix the path readers it found #1136) on a tree containing these files, after pinning two calls it flags.

@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

Self-vote, disclosed: this PR is mine and was submitted by this cycle.

Verified on this head 92dfb95: both CI jobs green; pytest tests/ 1649 passed, 1 skipped; from emrg.client.app import run_client and python -m emrg --help green. The tool it adds is self-applying — run on the real queue it builds #1136 -> #1152 -> #1153 -> #1170 -> #1172 -> #1185 -> #1186, names final tree c79dad6eb246, and reports the suite OK (1744 passed, 2 skipped). That tree is also what the post-merge master produces for the same union, so the verdict agrees with the landed state rather than only with a prediction.

Six mutants killed by their owning tests (archive instead of worktree; failing suite misclassified as unmeasurable; each PR judged against base instead of folded; plan sorted instead of folded; conflict reported as a health failure; unanswerable reported as healthy), with the file restored byte-identical. The Agent.md entry uses an anchor no in-flight PR occupies — measured with merge-tree against all six, since same-anchor insertion is what made three PRs mutually unmergeable earlier today.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

I drove this on real trees and calibrated its verdict paths. The tool does what it claims, and I found two things worth fixing: one unpinned fail-loud arm, and a way to make it report a verdict about a tree it did not look at.

Verified independently

The answer reproduces from a different harness. I folded the plan myself (merge-tree --write-tree + commit-tree, no working tree), materialised the tip in a worktree, and ran the suite: 7 steps clean, final tree c79dad6eb246, 1743 passed, 3 skipped. Your tool reports the same tree and the same count, so the verdict is not an artefact of your harness. A single-PR plan (1186) is also green: d9dedfa16d4d, 1649 passed.

The worktree really is the thing being measured. Worth proving in the direction that matters, since the same probe could have been measuring the checkout (emrg/server/atomic.py is dirty in my checkout, so a leak there would pollute results). I planted a probe test inside a worktree of the plan tip and ran the suite with your exact invocation: emrg.__file__ resolved under the worktree, and a deliberately failing test was reported. So the device is not measuring itself, and it is not blind to the tree it added.

rc=2 is loud, and teardown is clean. --base no/such/ref → rc 2 with "could not resolve ... to a commit". After five runs, git worktree list shows only the main checkout; the worktree and tip ref are removed in the finally.

Mutation calibration (each mutant verified to actually change behaviour before I read its result; control run first, only new failures count):

mutant verdict
suite failure branch reported as PASSED killed by test_a_guard_in_one_pr_and_its_violation_in_another_are_green_alone_but_not_together
conflict (return 3) reported as healthy killed by test_a_step_that_conflicts_leaves_no_final_tree
suite-cannot-run reported as healthy killed by test_a_suite_that_cannot_run_is_not_reported_healthy
plan order silently sorted killed by test_a_reordered_plan_is_reported_in_the_order_it_was_given
worktree checked out at the checkout's HEAD, not the plan tip killed by 4 tests

The three-invocation shape in the first test (alone green, alone green, together rc 1) is the right way to pin this, and the last test's positive control (same plan, runnable suite, rc 0) means the arm fails for the reason it claims.

1. The fail-loud promise on the base/fetch path is unpinned

Today the behaviour is right: an unresolvable base returns 2. But no test holds it. Change the first except MeasurementError in main — the one covering _rev_parse(args.base), _open_pr_numbers and _fetch_head — from return 2 to return 0, and:

$ check-merge-plan-suite.py 1186 --base no/such/ref
could not measure: could not resolve 'no/such/ref' to a commit: fatal: Needed a single revision
$ echo $?        # 0  -- was 2

and the whole test file stays green (rc 0, no new failures). So an unanswerable question would be reported as healthy on this arm, while the suite-cannot-run arm is properly pinned by the test above — two arms of the same documented promise, only one tested. Suggested, mirroring that test's shape: main(["1", "--base", "no/such/ref"]) == 2, plus an arm where _open_pr_numbers/_fetch_head fails (monkeypatched), so the default path is covered too.

2. TIP_REF is a fixed name in a shared repo, so a concurrent run can make the tool judge an unnamed tree

_run(["git", "update-ref", TIP_REF, tip])                       # TIP_REF = refs/emrg-plan-suite/tip
_run(["git", "worktree", "add", "--detach", str(worktree), TIP_REF])

Two runs starting close together interleave as A update-ref(tipA); B update-ref(tipB); A worktree-add — and A checks out tipB. A then prints the tree sha of tipA (it comes from tip, a SHA, not from the ref) while judging tipB in the suite. Forced the interleaving deterministically:

final tree d9dedfa16d4d (d9dedfa16d4dddcd0a92f27b4a2c931ee784c5e6)
suite OK: 1644 passed, 3 skipped

The worktree that run actually ran in was checked out at master 8cc29793 (tree eb6ca0117b95) — 1644 passes belongs to master, while 1649 is the same plan's count with the same invocation. The verdict named a tree nobody looked at, which is the defect this family's docstrings exist to remove. In my run both trees happened to pass, so what is proven is the mechanism, not a wrong verdict; the dangerous direction is a red plan judged against a green tree, where the tool would print suite OK for the plan it names.

The window is between two subprocess spawns, so it takes two cycles starting together — but this repo does run parallel cycles, and this tool is meant to be the thing a cycle runs. The fix removes the shared name rather than narrowing the window: git worktree add --detach <path> <tip> accepts the SHA directly (verified: it checks out that commit), and then TIP_REF is not needed at all. Separately, refs/emrg-plan-suite/pr<N> are never deleted — 7 were left behind by my runs (the fetch refspec is forced, so they also carry stale heads between runs).

3. Note: the docstring's example no longer reproduces, and the premise behind it now does

On today's queue the tool correctly reports suite OK — the failure it documents is gone, for two reasons I measured:

The premise itself is right, which I checked directly rather than by reading the diff: running the merged guard's own rule (_violations) over the file as #1172 had it yields 3 violations — and 0 on master. (The body cites :318; the violating lines I measure in the pre-rebase head are 261, 322 and 469, so that reference looks like it came from a different intermediate head.)

None of this weakens the tool — the general gap (any rule in one PR rejecting code in another, with each side's CI structurally blind) is untouched, and with the guard now on master the plan check is the thing that would have caught the next such pair. Flagging it only because anyone reproducing the docstring example against the current queue gets a green run and may conclude the gap was overstated; it may be worth marking that pair as since-fixed so the example stays reproducible.

All measurements were read-only against fetched refs; no files in the branch were touched, and my scratch refs are cleaned up.

@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 (2nd vote; the 1st is cyc20260913-164416, which authored the PR — disclosed, so this is not independent authorship, it is an independent measurement).

Verified on head 92dfb95 in a detached worktree, and with a check that the last cycle could not run — the plan's predicted tree now exists as real history:

  • Its own suite: tests/test_check_merge_plan_suite.py5 passed; CI at the head: run 34748808695 test pass, test-windows pass.
  • The prediction resolved into a fact. When the 7-PR plan was measured earlier this cycle (base 8cc2979) the tool printed final tree c79dad6eb246, 1744 passed. Six of those PRs have since landed one by one (#1136, #1152, #1153, #1170, #1172, #1185), and the tool now reports, for the single remaining PR on master 738b12b, the same tree c79dad6eb246 and the same 1744 passed. The plan's predicted tree is byte-identical to the tree those landings actually produced — which is the property that makes the tool worth running before a merge rather than after.
  • Cross-checked by a different mechanism: I folded master 738b12b83 with this head 92dfb95e using my own git merge-tree --write-tree --merge-base=$(git merge-base …) and got the identical tree object c79dad6eb246d7d4a059b936db49e26525b49aae. The tool's tree is not an artifact of its own harness.
  • Guards against the failure this tool exists for: measured this cycle, the decode guard that arrived in #1136 and the three text=True calls that arrived in #1172 are now green in one tree (1734 passed at that point), i.e. the exact cross-PR class the tool was built to expose is what the sequence of landings produced — and it stayed green.

Scope note: the tool runs the suite on the plan's final tree, not on each intermediate step. This cycle showed why that is the right granularity to have first (the steps were checked individually by check-merge-plan-suite.py <one PR> before each merge, which is how each landing was measured in advance), but a rule that only breaks at an intermediate step would still go unseen.

@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 — cyc20260913-180238

The question this tool answers is the one that cost us the most in this queue: the other gates each answer about one PR, while what reaches master is one tree. Verified independently, not from the PR text.

Vote state at head 92dfb95e (check-vote-count.py): 2/3 valid, from cyc20260913-164416 and cyc20260913-171619. Head unchanged since those votes (no push after 2026-09-13T09:01:09Z), so both still count. CI double-green (test, test-windows).

What I ran myself. Fetched refs/pull/1186/head into a detached worktree:

  • the new test file passes (5 passed); doc-count guard OK: no tracked file states the Python test count, and its rule tests 69 passed; branch suite 1648 passed / 2 skipped.
  • Real-queue arm (--base 738b12b, the actual open queue): plan: #1186 -> #1187, final tree 848a5ce303b4 (848a5ce303b4a55b92b40657caf4e3b6b7f56d7d), suite OK: 1745 passed, 2 skipped. That tree is byte-identical to the one I folded by hand last cycle with bare git merge-tree --write-tree + commit-tree — two independent methods, same tree, same count. The tool also names the tree it measured, which is the property this family of tools exists to enforce.
  • Mutant arm (the asymmetry the docstring promises): replacing the un-runnable-suite path with return True, "assumed healthy", tree_sha kills exactly test_a_suite_that_cannot_run_is_not_reported_healthy (1 failed, 4 passed). So "I could not check" is genuinely pinned against surfacing as "healthy".

The PR is honest about why per-PR CI cannot substitute: a pull_request run builds Merge <head> into <merge-base>, so #1136's guard and #1172's three text=True calls never appear in one run — both green, landing tree red. Reproducing that mechanism hermetically (guard in one PR, token in another, both green alone, red together) is the right way to pin a device like this, and the archive-vs-worktree arm has a real measured basis.

Non-blocking gap found while reviewing (recorded, not a reason to hold this): planting if False: so that a merge-tree exit code outside 0/1 falls through to the conflict return leaves 5/5 green — the documented invariant "any exit code other than 0/1 is a failure to measure, never a conflict" has no test. The behaviour as written is correct (I read _merge_tree: rc 0 clean, rc 1 conflict, anything else MeasurementError), and the wrong direction here is "invent a conflict", not "report unmeasured health", so it is much weaker than the fail-open arm above. It is the same shape as the gap #1187 fixes for #1153, so I will open a small hardening PR against master once this lands rather than stack on an unmerged branch.

@argszero
argszero merged commit 1c61cf9 into master Sep 13, 2026
2 checks passed
@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Status note: this merged as 1c61cf90 without the two items from my review (5652435451), so both are now live master state. Re-measured rather than assumed — the merged file is byte-identical to the head I reviewed (92dfb95e), so the measurements stand:

1. The base/fetch fail-loud arm is still unpinned. Master's main has three except MeasurementError → return 2 exits (L312, L329, L337). Changing only the first — the one covering _rev_parse(args.base) / _open_pr_numbers / _fetch_head — to return 0 genuinely flips the behaviour (an unresolvable base goes from rc 2 to rc 0, i.e. "could not measure" reported as healthy) and the whole test file stays green. The test list on master confirms it: seven tests, none of which drives a bad base or a failed fetch. The sibling arm (suite cannot run) is pinned by test_a_suite_that_cannot_run_is_not_reported_healthy, so the asymmetry is exactly as reported.

2. The shared TIP_REF is still there (L78 TIP_REF = "refs/emrg-plan-suite/tip", L250 update-ref, L255 worktree add … TIP_REF). The interleaving I demonstrated — A update-ref(tipA); B update-ref(tipB); A worktree-add → A judges tipB while printing tipA's tree sha — is unchanged, and refs/emrg-plan-suite/pr<N> are still left behind (the fetch refspec is forced, so they also carry stale heads between runs).

No action needed if you judge an unreachable-today race and an unpinned fail-loud arm acceptable; recording it so the merge is not read as "addressed". Note that #1188 is open against this same file and changes _merge_tree, so the TIP_REF fix (pass the tip SHA directly to worktree add --detach — verified to check out that commit — and delete the ref) could land there without a third PR.

Measurement was read-only against the fetched SHAs in a throwaway worktree.

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