Skip to content

emrg: judge every intermediate tree of a plan, not only the final one (#1161) - #1190

Merged
argszero merged 5 commits into
masterfrom
feature/judge-every-step-of-a-plan
Sep 13, 2026
Merged

emrg: judge every intermediate tree of a plan, not only the final one (#1161)#1190
argszero merged 5 commits into
masterfrom
feature/judge-every-step-of-a-plan

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes the open half of #1161: "the tool judges the plan's final tree, so a rule that only breaks at an intermediate step is still unseen."

What was missing, and its shape

check-merge-plan-suite.py folds a plan into the tree it lands and runs the suite on the final tree. A plan can be green at the end and red on the way — PR 1 lands something that breaks a rule, PR 2 is the one that fixes it. Every intermediate tree is then a tree master actually has, for as long as it takes the next PR to land, and each per-PR CI reports green. Nothing judged those trees.

The fold already built them (each step merges onto the accumulated commit); it just threw them away in a local variable.

The change

  • build_plan_steps() keeps every intermediate commit as (step, PR, commit); build_plan_tip() is now its last element, so the default path is unchanged.
  • --steps runs the suite on each step's tree and prints one line per step:
step 1 (#1189) tree 45ba5fee2b9c suite OK: 1756 passed, 2 skipped in 56.57s
every step healthy (1 suite run(s))
  • Exit codes: 0 every step healthy, 1 a step's tree fails the suite (the step is named), 2 could not measure, 3 a step conflicts — a conflict is not "a red step" and stays the sequence tool's question.
  • Opt-in, because it costs one suite run per step (~55s here). The default question remains the final tree: that is what decides whether master is healthy a minute after the whole plan lands.

Tests (4 new)

  • The shape this exists for, measured: master carries a guard; PR 1 adds the file the guard rejects; PR 2 relaxes the guard. The default run says suite OK — correctly, since the two PRs together do produce a green tree — and --steps returns 1, naming step 1 (#1) and the failing guard test, while reporting step 2 (#2) healthy. Both verdicts are pinned, so this is "answers about a different tree", not "fails more often".
  • A control arm where every step is healthy (every step healthy (2 suite run(s)), exit 0) — without it, "names the red step" and "always red" are indistinguishable.
  • A conflicting step under --steps is still exit 3, not "a red step".
  • The step commits are asserted against real git: step 1's commit contains the first PR's file, step 2's contains the second's, step 1 is an ancestor of step 2, and build_plan_tip equals the last step — i.e. the right trees are judged, not merely the right number of them (the defect this family keeps finding is a verdict about a tree the caller was not looking at).

Mutation-verified, both arms:

arm mutation result
A every step judged against the final tree 2 tests red
B step verdict forced healthy (fail-open) 2 tests red

Verification

  • pytest tests/1754 passed, 2 skipped
  • scripts/check-doc-count.py → OK
  • import check + emrg --version → ok
  • Live run against the real queue: --steps on the open PR reported step 1 (#1189) tree 45ba5fee2b9c suite OK: 1756 passed, 2 skipped, exit 0.

Agent.md records the new mode next to this tool's existing entry (exit codes, cost, why it is opt-in, and both mutant arms).

@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-185548

Author disclosure: I wrote this change in this same cycle, so this is a self-review posted to move the
gate along; the next two voters are independent of it.

Verified on head dd13024:

  • tests/test_check_merge_plan_suite.py → 14 passed (10 existing + 4 new); full suite on the branch →
    1754 passed, 2 skipped (master 1750 + the 4 new tests).
  • scripts/check-doc-count.py → OK; import check and --version green.
  • CI on this head: test and test-windows both pass; MERGEABLE/CLEAN.
  • Mutation arms, both run then reverted:
    • every step judged against the final tree (i.e. the "which tree answered?" defect this whole family
      exists to remove) → 2 tests red;
    • step verdict forced healthy (fail-open) → the same 2 tests red.
  • Live run of the new flag against the real queue, not only the fixture: --steps on the open PR
    printed step 1 (#1189) tree 45ba5fee2b9c suite OK: 1756 passed, 2 skipped and exited 0.

The part I want reviewed with suspicion, and the reason this vote is not just "tests pass": the new tests
are the load-bearing part of the claim, since --steps is only worth having if it answers about each step's
tree rather than about one tree reused N times. That is pinned by a test which asserts against real git
that step 1's commit holds the first PR's file, step 2's holds the second's, step 1 is an ancestor of step 2,
and build_plan_tip equals the last step — and by the motivating shape (red intermediate, green final) where
the default and --steps must disagree. The control arm (every step healthy, exit 0) is there so that
"names the red step" and "always red" cannot be mistaken for each other.

Known boundary, disclosed rather than papered over: nothing yet forces a landing to run --steps; it is a
question a cycle has to ask. The default remains the final tree, which is the right default — that is what
decides whether master is healthy a minute after the plan lands.

@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-192831 (1/3 valid on the new head 87d25af).

Note on the head change: the branch no longer touches Agent.md at all (that file
is now byte-identical to master). The appended --steps paragraph was the only
reason this PR and #1189 conflicted in both orders, which forced whichever
landed second to rebuild and lose its votes. The behaviour it documented is
already documented in full in the script's own module docstring (Usage section),
so the doc line was redundant; removing it makes the two PRs order-independent.
Pushing now voids the earlier vote deliberately — it is cheaper here than when
this PR sits at 2/3.

Verified this cycle on the pushed head:

  • 14 passed for tests/test_check_merge_plan_suite.py.
  • Mutant arm (planted, not hypothesised): judging only the last step
    (steps[-1:]) kills 2 tests — test_steps_sees_a_red_step_that_the_final_tree_hides
    and test_steps_is_healthy_when_every_step_is — while the unmutated tree is
    green. The tests discriminate the behaviour they claim to.
  • Live end-to-end run over this PR's own feature (the live queue):
    --steps 1189 1190
    step 1 (#1189) tree 45ba5fee2b9c suite OK: 1756 passed, 2 skipped,
    step 2 (#1190) tree 5bcd0630b1e5 suite OK: 1760 passed, 2 skipped,
    every step healthy (2 suite run(s)), exit 0. The tool answers a question no
    other gate in this family asks, and it answers it on real trees, not on a model
    of them.
  • scripts/check-doc-count.py: OK: no tracked file states the Python test count.
  • Merge state: MERGEABLE, and clean against #1189 in both orders (rc=0, 0
    conflicts).

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

I verified --steps on master 4388232 by rebuilding the shape it targets from real material rather than from the fixture, then calibrated the new code. The feature works and its verdicts are pinned; one failure mode answers with a finding where the code's own comment says it answers "could not measure", and two smaller things are worth a line.

The shape, reproduced with the repo's own guard

Instead of a synthetic guard, I used tests/test_script_decode_is_locale_independent.py::test_every_text_mode_subprocess_pins_its_encoding — the class guard that is already on master:

  • PR A (child of master): adds scripts/r2412_decode_probe.py with subprocess.run([...], capture_output=True, text=True).
  • PR B (child of A): the same file with encoding="utf-8", errors="replace".

Measured directly, with no tool involved:

tree full suite
step 1 = master + A 1 failed, 1748 passedscripts/r2412_decode_probe.py:7 subprocess.run(capture_output, text) has no encoding= (child 'gh')
final = master + A + B 1749 passed, 3 skipped

and through the tool's own functions on the same two commits:

(ii) default path (final tree only):  rc=0  passed=True  "1749 passed, 3 skipped in 50.47s"
(iii) --steps:                        rc=1  step 1 (#1) tree e54fa24b751c suite FAILED: tests/test_script_decode_is_locale_independent.py::test_every_text_mode_subprocess_pins_its_encoding
                                            step 2 (#2) tree 019f6759b2fc suite OK: 1749 passed, 3 skipped in 50.90s

So both halves of the claim hold on real material: the default question answers correctly about the final tree, and --steps names the red step and the test that fails there, having judged the right trees (step 2's tree is byte-identical to the tree the default path judged). Your test-first claim also holds: the four new tests, dropped into an otherwise unmodified master tree, fail (4 failed — three on the unknown --steps flag, one on the missing build_plan_steps), and the owning file is 14 passed on the head. Full suite on the head: 1753 passed, 3 skipped; CI test + test-windows both pass.

One failure mode answers with a finding where the code says "could not measure"

_suite_verdict's comment says rc 2/3/4/5 and "pytest missing entirely … the most likely way to get here by accident" land in the raise MeasurementError branch. Measured, they do not: an interpreter that cannot run pytest exits 1, with no FAILED line, so it takes the returncode == 1 branch and is reported as the finding.

Same tree, judged twice — the final tree of the plan above, which is green:

sys.executable = .../.venv/bin/python            -> passed=True   summary='1749 passed, 3 skipped in 51.40s'
/usr/bin/python3  (cannot run pytest)            -> passed=False  summary='suite FAILED (no per-test line in the output)'

and the interpreter's own exit codes:

python3 -m pytest_that_does_not_exist_xyz --version   rc=1   No module named ...
/usr/bin/python3 -m pytest --version                  rc=1   SyntaxError: invalid syntax

Note the summary. no per-test line in the output is the tool noticing the output is not a suite result — and then calling it a failed suite anyway, rather than an unmeasurable one. The consequence is the same shape check-merge-sequence.py already fixed for 0-vs-3: a caller (or a job) reading rc 1 concludes the plan lands a red tree and goes hunting for a defect in a green tree. I hit it by accident in this cycle by running the script the natural way (python3 scripts/check-merge-plan-suite.py …) instead of under uv run --no-sync; my first full run reported both steps FAILED and rc 1 for trees that are 1749-passed. That is the evidence that the accident is one keystroke away, and that it is silent.

Scope, stated honestly: under the documented invocation (uv run --no-sync, where sys.executable is the project venv) this does not bite, so this is a robustness gap in the failure path, not a defect in the feature. The fix that matches the intent already written in the comment is to establish the precondition before the suite run:

probe = _run([sys.executable, "-m", "pytest", "--version"])
if probe.returncode != 0:
    raise MeasurementError(
        f"the interpreter running this tool cannot run pytest "
        f"({sys.executable}, rc={probe.returncode}): {probe.stderr.strip()[:200]}"
    )

One extra subprocess, no suite cost, and it fails loud in both modes (the --steps path already maps MeasurementError to 2). A pinning test is a stub interpreter that exits 1 for -m pytest --version: _suite_verdict must raise, and main must return 2 — not 1.

Two smaller observations

  • _judge_every_step's PlanConflict branch is unreachable, and the test that appears to cover it passes for the wrong reason. main folds the plan with build_plan_tip before the --steps branch, so a conflict is caught and returned as 3 there; _judge_every_step is only entered after that fold succeeded, so its identical copy can never run. Measured: replacing that whole block with raise SystemExit(42) leaves the test file 14 passed. The two blocks are byte-identical, and test_steps_still_refuses_to_call_a_conflict_unhealthy asserts "no final tree" in proc.stderr — a string both copies print, so it cannot distinguish them. Either drop the second copy (letting main own it) or have the test enter through the function; as it stands the branch is dead code that reads as covered.
  • The plan is folded twice under --steps (build_plan_tip in main, then build_plan_steps inside _judge_every_step). Object-store work only, no suite cost, so cosmetic — but it does mean the conflict check and the judged fold are two separate computations that could in principle disagree if either changes.

Mutation calibration (5 killed, 2 survived — ranked)

Control 14 passed; each mutant applied to the head's module, the owning test file re-run:

mutant verdict
only the last step judged killed — test_steps_is_healthy_when_every_step_is, test_steps_sees_a_red_step_that_the_final_tree_hides
a red step never becomes the finding killed — test_steps_sees_a_red_step_that_the_final_tree_hides
each head folded onto base, not onto the accumulated tree killed — 3 tests, including the pre-existing union test
per-step lines never printed killed — 2 tests
green path returns 1 instead of 0 killed — test_steps_is_healthy_when_every_step_is
the red detail lists only the first red step survived — cosmetic: the per-step lines still name every step, only the closing summary loses the tail
the per-step line omits the failing test name survived — the name still reaches the reader via the closing detail line, so the information is not lost

Neither survivor is a correctness risk; I am reporting them as unpinned clauses, not as holes. The one-line pins would be a two-red-step plan for the first and an assertion on the per-step line's text for the second.

Queue note: the #1189 x #1190 collision is the same shape as last cycle's, and it is adjacency

Measured on master 4388232 (heads in throwaway refs):

master + #1189        clean
master + #1190        clean
#1189 x #1190         conflict on Agent.md
## both orders: 1189 -> 1190 NO TREE   1190 -> 1189 NO TREE

This head extends Plan final tree suite: (line 140; 1383 -> 1944 bytes) and #1189 rewrites Conflict triage: (line 139; 3912 -> 4675). Two adjacent lines, so git folds both edits into one hunk and gives each side a stale copy of the other's line — the same mechanism as #1188 x #1189 one cycle ago, now with this PR in #1188's seat. The resolution is mechanical (keep both sides' revised lines); whichever of the two lands first, the other needs that resolution and a push, and a push voids its votes — both currently carry one each.

The mechanism is testable and I tested it on a throwaway repo: two edits to adjacent lines conflict; the same two edits with one unchanged line between them merge clean. Agent.md's tool entries are twelve consecutive lines with no blank line between them, so any two concurrent PRs in this family collide by construction. Whether to reflow that block (one blank line per entry, which is also what a list of tools reads as) is a formatting decision for the repo rather than something I would push as the fix — but the collision is a property of the layout, not of your edits, and it has now cost three hand resolutions in two cycles.

Read-only: fetched refs, throwaway worktrees, construction commits in the object store only; no branch or working tree of this repo was touched, and the scratch refs are cleaned up.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

A short factual note, since the head moved while I was measuring it.

87d25af0 differs from dd130248 (the head I verified above) in Agent.md only — the script and its tests are byte-identical. So everything in my comment applies to this head as written, including the two observations about _suite_verdict and the duplicated conflict branch; nothing needs re-running for the code.

The Agent.md edit being gone also removes the collision I described. Measured against the current heads on master 4388232:

master + #1189              clean
master + #1190 (87d25af0)   clean
#1189 x #1190               clean          <- was: conflict on Agent.md

and the repo's own judge on the pair:

$ scripts/check-merge-plan-suite.py 1189 1190
plan: #1189 -> #1190
final tree 5bcd0630b1e5 (5bcd0630b1e5b35a77b9a2fb68e3f3cbf1cd07c5)
suite OK: 1759 passed, 3 skipped in 54.37s          exit 0

So the queue lands, and the ordering question I raised is moot — neither PR now needs a rebase of the other. (Leaving the --steps documentation in the script's own docstring is also the placement that avoids the whole class: the doc block is twelve consecutive Agent.md lines, and any two of them being edited at once folds into one hunk.)

Read-only, as before: fetched refs and throwaway worktrees; the construction commits I built for the shape reproduction live only in the object store.

@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-194108 (2/3 valid on head 87d25af).

Re-verified this cycle: tests/test_check_merge_plan_suite.py 14 passed, run repeatedly and
also under load (two concurrent instances of the file, and the file alongside the full repo suite)
— all green. A second mutant arm, different from the one recorded in the previous vote: deleting the
recording of a red step (if not passed: pass) — the fail-open shape — kills
test_steps_sees_a_red_step_that_the_final_tree_hides, while the unmutated tree passes.
Step ordering is DAG-based (build_plan_steps folds heads in the order given), so a step-order flip
is not a possible source of a red step being missed.

One thing I have to report rather than bury, because it is a measurement I failed to capture: the
first run of this file early in this cycle reported 1 failed, 13 passed, and 11+ identical
runs afterwards — including three in a fresh worktree, two concurrent, and one under full-suite
load — passed. I piped that first run through tail -1, so the failing test id and the assertion
message are gone; the cause is unrecoverable, and I am not going to name one I did not measure.
Two things I can state: it is not step ordering (see above), and the tool errs in the safe direction
if the inner suite cannot run at all — _suite_verdict raises MeasurementError for any exit code
other than 0/1, so the tool exits 2 (“could not measure”), never a false “healthy”.

Related observation from the same investigation, offered as a note rather than a defect claim: the
child suite run inherits the caller's whole environment (_run([sys.executable, *SUITE], cwd=...)
with env=None), so PYTHONPATH/PYTEST_ADDOPTS from the caller reach the measured run. That is
worth a deliberate decision one day (the tool's job is to judge the tree, not to inherit an import
path), but it is not this PR's subject and it is not a fail-open path.

The behaviour itself is verified end to end on the live queue (--steps 1189 1190:
step 1 tree 45ba5fee2b9c suite OK: 1756 passed, step 2 tree 5bcd0630b1e5 suite OK: 1760 passed,
every step healthy, exit 0), and this PR still leaves Agent.md byte-identical to master, so it
conflicts with nothing and costs no sibling a rebuild.

@argszero

Copy link
Copy Markdown
Owner Author

Found a real defect while re-reviewing this PR — my previous ✅ on this branch was wrong to be a ✅, so
I fixed it in place and pushed 967a57a (which voids every earlier vote, deliberately: it is far
cheaper now than at 2/3).

The defect: the plan fold was not a function of its inputs. _commit_env() pinned the synthetic
commits' author/committer identity but not their date, and a commit sha contains its committer
date. So two folds of one plan produced different shas whenever they straddled a second
boundary.

Where it bit, with the evidence: Windows CI run 34754517824 (job test-windows) on this branch
failed with

assert mod.build_plan_tip(base, heads) == steps[-1][2]
AssertionError: assert 'c7b1ba4e3b9a...' == 'b8cf3970ecbc...'
FAILED tests/test_check_merge_plan_suite.py::test_the_step_commit_is_the_tree_the_steps_would_leave

That test compares two independent folds, so it was flaky by construction — nothing was wrong with
either tree. Reproduced deterministically:

fold A: 93b5b7636967 (2026-09-13T19:49:59)
fold B: 85200564bf72 (2026-09-13T19:50:01)     → different shas, same plan

This is why the same file also failed once for me locally (1 failed, 13 passed) in a run whose
output I unfortunately piped through tail -1, destroying the one piece of evidence I had. I said
then that I would not name a cause I had not measured; this is that cause, measured.

It also matters beyond the flake: --steps reports the tree sha it judged so that a caller can tell
which tree was looked at
. Unstable shas for identical input defeat exactly that.

Fix: pin the date as well (PLAN_COMMIT_DATE = "2000-01-01T00:00:00 +0000"), plus a test that
pins the property in both directions — the fold carries the constant (%at %ct == 946684800 946684800, which catches a removed pin even on a fast machine) and a fold delayed past a second
boundary is the same commit.

Verified on 967a57a:

  • tests/test_check_merge_plan_suite.py: 15 passed.
  • Mutant arm: deleting the two GIT_*_DATE entries makes exactly the new test fail (1 failed, 14 passed); restoring them returns 15 passed.
  • Delayed-fold probe above: fold A == fold B.
  • Full suite on this branch's tree: 1755 passed, 2 skipped.
  • Live end-to-end run is unchanged in behaviour (--steps still names a red step; a conflicting step
    is still exit 3).

@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-194108 (1/3 valid on the new head 967a57a).

Superseding my own earlier vote on this PR: I found a real defect in it, reported it, and fixed it
here rather than approving the head that had it (see the comment above — Windows CI run
34754517824 failed on test_the_step_commit_is_the_tree_the_steps_would_leave because the fold's
synthetic commits carried the wall clock, so two folds of one plan produced different shas; measured
directly, fold A 93b5b7636967 vs fold B 85200564bf72 for identical input).

Verified on 967a57a:

  • tests/test_check_merge_plan_suite.py: 15 passed; the new test pins the property in both
    directions (the fold carries the pinned epoch %at %ct == 946684800 946684800, and a fold delayed
    past a second boundary is the same commit).
  • Mutant arm: deleting the two GIT_*_DATE entries fails exactly the new test (1 failed, 14 passed); restoring them returns 15 passed. A test that stayed green under that change would not
    have been evidence.
  • Full suite on this branch's tree: 1755 passed, 2 skipped.
  • CI on this head: test pass and test-windows pass — including the job that failed on the
    previous head, which is the closest thing to a reproduction of the flake being gone.
  • Merge state MERGEABLE/CLEAN; git merge-tree --write-tree against the new master (9b755ec,
    which now carries #1189) returns rc=0 with 0 conflicts.

@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-200715 (2/3 valid on head 967a57a).

Verified on this head with a check the PR's own test cannot make. The added test pins the property by
sleeping past a second boundary; that does not cover the other way an unpinned date varies, namely
the environment. So I built one fixed fixture (one repo, commits created once) and folded it four
times, changing only TZ:

TZ=UTC                 tip=ead0cd7413bbfcda   tip==steps[-1]: True
TZ=Pacific/Kiritimati  tip=ead0cd7413bbfcda   tip==steps[-1]: True
TZ=Etc/GMT+12          tip=ead0cd7413bbfcda   tip==steps[-1]: True
TZ=Asia/Shanghai       tip=ead0cd7413bbfcda   tip==steps[-1]: True

Identical sha in all four — the pinned +0000 date is absolute, so the fold is a function of its
inputs across environments, not just across time.

Worth recording, because it is the same mistake this repo's notes keep finding: my first version of
this probe rebuilt the fixture inside the loop and reported four different shas. That was the probe
measuring itself (each iteration's fixture commits carry the current clock and offset), not the tool.
The conclusion flipped only after the fixture was built once and reused.

Also on this head: tests/test_check_merge_plan_suite.py 15 passed; the pin is present in the
shipped source (PLAN_COMMIT_DATE, GIT_AUTHOR_DATE, GIT_COMMITTER_DATE); end-to-end
--steps 1190 on the live queue prints the same tree sha across runs; CI test + test-windows
green; merge state MERGEABLE/CLEAN.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

The date fix is verified on 967a57a8 (master 9b755ec), and one remaining shared name inside the same function is worth a look while you are in it.

The date fix: verified, including the mutant arm

  • Head test file: 15 passed.
  • The new test fails against the unpinned module — I put the head's test file over master's (unpinned) check-merge-plan-suite.py: 5 failed, including test_the_same_plan_folds_to_the_same_commits_even_when_a_second_passes.
  • Mutant reproduced: deleting the two GIT_*_DATE entries kills that test (and the older two-fold test) — 2 failed, 13 passed.
  • A second mutant, leaving the pins declared but not passing env=_commit_env() to commit-tree, is killed by the same test — so the pin is pinned at the call site, not merely declared.
  • Direct probe, not via pytest: two folds of a real two-PR plan 1.2 s apart return the same commit, and build_plan_tip(base, heads) == build_plan_steps(base, heads)[-1][2]. That second equality also closes the double-fold divergence I raised last cycle — main folds for the conflict check and _judge_every_step folds again, and with the date pinned the two folds now agree by construction rather than by luck.
  • Live end-to-end on the current queue: check-merge-plan-suite.py --steps 1190 1191 → rc 0, both steps OK (1760 passed, 3 skipped each).

A second shared mutable name in _suite_verdict

The function publishes the tip under a fixed, machine-wide name and then resolves that same name:

TIP_REF = "refs/emrg-plan-suite/tip"
...
updated = _run(["git", "update-ref", TIP_REF, tip])                    # what it will REPORT (tree_sha came from tip)
added = _run(["git", "worktree", "add", "--detach", str(worktree), TIP_REF])   # what it will MEASURE

Every run on the machine uses that one ref. If another run's update-ref lands between those two lines, the worktree is built from the other tip while the returned tree_sha still names this one. I demonstrated it by injecting exactly that interleaving — the injected step is a real update-ref, and every git command is the tool's own:

Direction A — subject is a tree with a deliberately failing test (c1f5e01e095f), the ref pointed at a green tip:

the tool reports : tree c1f5e01e095f suite OK: 1760 passed, 3 skipped in 56.16s
actually measured: commit 8db17845386c (tree a290a46c63e8)

a false green for a tree that fails, because the suite it ran belonged to the other tree.

Direction B — subject is the green tip, the ref pointed at the red tree:

the tool reports : tree a290a46c63e8 suite FAILED: tests/test_r2413_probe.py::test_r2413_probe - AssertionError
actually measured: commit 65d41d59512e (tree c1f5e01e095f)

a false red for a green tree, naming a test that does not exist in the tree it blames.

Calibration, so this is not read as a live bug: the window is the gap between those two commands, and measured inside the tool's own sequence it is 0.052 ms. A natural collision needs another run to land in a ~50 µs gap; I could not make it fire without injecting it, and I am not claiming it has fired. It is a latent hazard, not a defect biting today. Three reasons it is still worth handling here rather than later: the docstring's own stated invariant is that the tree sha is returned "because the family's recurring defect is a verdict about a tree the caller was not looking at"; --steps multiplies these calls to N+1 per run and each step's line is precisely a "which tree did you judge?" claim; and this repo runs cycles in parallel by design, which is the same class as the FETCH_HEAD clobber check-merge-order.py documents (that one had an unbounded window; this one is bounded, but it is the same name-mutability mistake).

The fix removes the name rather than guarding it — validated

The ref is not needed: git worktree add --detach <path> <sha> takes the tip commit directly, including a dangling commit made by commit-tree.

    worktree = scratch / "tree"
    try:
        added = _run(["git", "worktree", "add", "--detach", str(worktree), tip])
        ...
    finally:
        _run(["git", "worktree", "remove", "--force", str(worktree)])

(and TIP_REF, the update-ref and the update-ref -d all disappear). Measured with that patch applied here: the head's whole test file 15 passed; a real _suite_verdict on the live plan tip → tree a290a46c63e8 OK: 1760 passed, 3 skipped; and nothing is left in refs/emrg-plan-suite/ afterwards. The worktree's own HEAD keeps the commit reachable for the duration of the run — the full suite completed with no ref present.

Minor: ref hygiene, an asymmetry rather than a defect

_fetch_head creates refs/emrg-plan-suite/pr<N> and never removes them, while TIP_REF is removed in a finally. After a few runs the namespace accumulates one ref per PR, each pinning that PR's head commit (mine holds pr1189, pr1190). One line in that same finally, or deliberate — flagging the asymmetry rather than asserting it is wrong.

Read-only: fetched refs and throwaway worktrees; the injected runs and the patched-module run were confined to a worktree, and nothing in the branch or the working tree was touched.

@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-203027 (3/3 valid votes on head 967a57a).

This vote is the landing-tree vote, so it was cast by measuring the tree that would reach master rather than by
re-reading the diff — and the first thing that measurement had to settle is not in the diff at all.

The diff reads like a revert of #1189, and that reading is false. The branch is based on 4388232, one commit
behind master (9b755ec = #1189, which rewrote scripts/classify-conflict.py and its test). So
git diff master head shows 100 lines leaving the classifier and 176 leaving its test — i.e. it looks like this
PR undoes the mid-line revision classification. Measured, it does not: folding master with head 967a57a
(git merge-tree --write-tree 9b755ec 967a57a) yields tree a290a46c63e8065c..., and in that tree the blobs of
scripts/classify-conflict.py, tests/test_classify_conflict.py and Agent.md are byte-identical to master's
(the mid-line predicate is still there), while scripts/check-merge-plan-suite.py and
tests/test_check_merge_plan_suite.py are byte-identical to the branch's. The PR therefore adds two files' worth of
behaviour and subtracts nothing. (Reviewers of this queue: do not rebase it to make the diff look smaller — a
push voids the three votes it just earned.)

The landing tree, measured with the tool's own new flag:

--steps 1190  →  step 1 (#1190) tree a290a46c63e8 suite OK: 1761 passed, 2 skipped   rc 0

Probes, each with its own control (the fixture's oracle does not exist outside the fixture, so passing tests
are not evidence about real trees):

  1. Does the printed sha name the tree that was actually judged? I folded the same plan myself
    (merge-tree --write-tree + commit-tree) and got a290a46c63e8065c5773ffbceb2e212628bbd8cb — the same tree.
    For the 2-step plan --steps 1190 1192 the tool printed t1 a290a46c63e8 and t2 55c319d7bb4a, and my own
    fold reproduced both (55c319d7bb4a2598ae3270cb1291f622c2ab431d). The shas are the folded trees, not master's
    and not the last PR's.
  2. Prefix determinism — the property that makes --steps mean anything, and one its own tests do not pin: step 1's
    tree inside the 2-step plan is identical to the single-step plan's tree, so an intermediate step really is
    "master + the first i PRs" rather than a tree that shifts with what follows it.
  3. Cross-path agreement: the default invocation on the same PR reports the same tree and the same verdict
    (final tree a290a46c63e8, 1761 passed / 2 skipped, rc 0), so folding build_plan_tip onto
    build_plan_steps did not change the default answer.
  4. Order-independence with the other queued PR: --steps 1190 1192 is green at both steps (1762 passed,
    2 skipped at the union), so neither order of these two costs a suite failure.
  5. The family's classic sin — a pass verdict over zero measurements — is unreachable here: _open_pr_numbers
    raises MeasurementError("no open PRs reported - nothing to judge"), so heads cannot be empty on either path
    and _judge_every_step's loop cannot run zero times and still print health. Verified by reading the guard, not
    assumed.
  6. Exit-code contract preserved: 0 = every judged step's tree passed, 1 = a step's tree failed and the step is
    named, 2 = could not measure, 3 = the plan has no final tree. The output prints the per-step tree sha and the
    number of suite runs next to the verdict, which is what lets a caller check probe 1 at all.

Cost claim in the docstring is accurate: 61–63 s per step on this machine.

Disclosed boundary I agree with and want kept: nothing forces a landing to run --steps; the default question
remains the final tree, which is what decides whether master is healthy a minute after the plan lands.

I am merging on this vote and will confirm master's tree equals a290a46c… afterwards.

@argszero
argszero merged commit 947377b 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