Skip to content

TASK-143 — check the merged result of every open change, and name the pair that disagrees - #27

Open
ranjiao wants to merge 7 commits into
feat/work-modesfrom
coding/task-143-merge-result-ci
Open

TASK-143 — check the merged result of every open change, and name the pair that disagrees#27
ranjiao wants to merge 7 commits into
feat/work-modesfrom
coding/task-143-merge-result-ci

Conversation

@ranjiao

@ranjiao ranjiao commented Aug 20, 2026

Copy link
Copy Markdown
Owner

What was actually established, before anything was changed

The spec warned not to assume the diagnosis. Two things were measured first.

1. ci.yml already tests the merge result. actions/checkout resolves
refs/pull/N/merge on a pull_request event. PR #16's run that same morning
says so verbatim:

git fetch ... +9e18354...:refs/remotes/pull/16/merge
HEAD is now at 9e18354 Merge 96822a4e... into e9d8c4dc...

So "check out something different" would have been a change that looks right,
passes review, and prevents nothing.

2. For #14 and #15 that workflow never ran at all.

actions/runs?head_sha=e3f8621...     → total_count: 0
actions/runs?head_sha=765b2c1...     → total_count: 0
commits/<sha>/check-runs             → total_count: 0   (both)
gh pr checks 14 / 15                 → "no checks reported on the ... branch"

"Each was green on its own base" was each agent's own local suite run, not a CI
verdict. Both PRs were non-draft and merged cleanly against e9d8c4dc, and
PR #16 — created 33 seconds after #15 — did get a run, so Actions was working.
The cause of the two missing runs is not established; only the fact is.

The two real defects

  1. The merge result is computed against the base as it stood when the event
    fired
    , and GitHub never re-fires pull_request when the base moves. TASK-100 — the two store files are in claims[], so a collision on them is reportable #14
    and TASK-110 — measure what Perry's own state costs, and propose a retention policy #15 were both computed against e9d8c4dc; TASK-100 — the two store files are in claims[], so a collision on them is reportable #14 landed on 02e61fd and
    TASK-110 — measure what Perry's own state costs, and propose a retention policy #15 landed on top of TASK-100 — the two store files are in claims[], so a collision on them is reportable #14. No run ever saw the tree that shipped. This is not
    theoretical: during this one session feat/work-modes moved 45ef9c6
    551274f60916fb3.
  2. Nothing ever tests two open changes together.

The mechanism

tests/merge-check merges the base tip with every candidate and runs the suite
on that tree. If red, it re-runs the failing check three more ways — on the base
alone (pre-existing, attributed to nobody), on each candidate alone (that
change's own regression), and on each surviving pair. Green apart, red
together, is the pair.

merge-check.yml is a separate workflow because its triggers differ: it also
runs on push to an integration branch, which is the moment a base moves
under the PRs still open against it.

Happy path is one suite run; attribution only runs when the combined tree is red.

Rejected

Verification

1 — the pair reconstructed. e9d8c4dc + e3f8621 + 765b2c1, full suite,
no module named on the command line:

CONFLICTING PAIR — pr14 × pr15 · test_state_cost.py
    pr14 alone: absent   pr15 alone: green   together: red
    FAIL: test_a_rewritten_path_carries_its_superseded_versions
    FAIL: test_an_unclaimed_file_under_the_state_root_is_still_reported

Exit 1, and those are the two failures the spec describes. The old mechanism
on the same commits:
merge(e9d8c4dc, 765b2c1) runs test_state_cost at
21 tests, all green, and merge(e9d8c4dc, e3f8621) does not contain the
module at all. Green both ways, red merged — the miss reproduced, then caught.

2 — an independent pair is not reported. Two changes to the same tool
(bin/perry-state-cost gains a --rev alias; tests/test_state_cost.py gains
two argument-error tests) — the kind a same-area heuristic would flag:

pre-existing on the base — attributed to nobody:
  · test_diagnose.py
✓ nothing new is red on the merged result

Exit 0.

3 — attribution. The report names change × change × check. Where it cannot
narrow to a pair it says so and names the whole set rather than guessing. In the
run above it also correctly separated a solo breakage from a pair.

Live in CI (run 32391788811, 3m26s), which is also the pre-existing step
earning its place on the first green run:

merge-check · base feat/work-modes @ 60916fb3
  #27  d5bbdc0a  TASK-143 — ...
  … 1 check(s) red — baselining them on the base alone
pre-existing on the base — attributed to nobody:
  · test_host_support.py
✓ nothing new is red on the merged result

Without that step this run would have blamed the only open PR for a failure the
base already had.

The same tree, two verdicts

On commit d5bbdc0, both workflows ran against the same merged tree:

workflow verdict why
tests (ci.yml) failure✗ test_host_support.py red is red; nothing says whether this PR caused it
merge-check success — attributed to nobody same module, checked on the base alone first, found already red there

That is the whole argument for the differential. ci.yml has come back
failure on essentially every pull request since 2026-08-20 05:41, which is
why nobody reads it. A check that cannot be green is not a gate.

Two things a reviewer should not miss

Needs repository admin, not attempted here. Marking merge-check a
required status check in branch protection. Until an admin applies that,
this reports; it cannot refuse a merge. A workflow silently depending on an
unapplied setting would be worse than no change.

test_host_support::test_concurrent_mixed_registers_do_not_exceed_global_cap
is load-flaky
— green 3/3 alone, red under a loaded parallel sweep, and red on
the CI base above. A test that flips under load can satisfy "green alone, red
together" by luck and name two innocent PRs. It is the most likely source of a
false accusation from this mechanism and is worth its own row.

Diff size

feat/work-modes is ahead of origin, so this PR carries those commits until the
branch is pushed. My own code diff is 4 files, +624 lines
tests/merge-check, .github/workflows/merge-check.yml, a comment in ci.yml,
one line in AGENTS.md. git diff -- perry/ is empty.

🤖 Generated with Claude Code

ranjiao and others added 7 commits August 20, 2026 23:38
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… pair

TASK-143. PR #14 put both store files into `claims[]`; PR #15 shipped
`tests/test_state_cost.py` asserting the world before that claim existed. Each
was fine alone, the merged tree had two red tests neither could have seen, and
a human found out by running the suite afterwards.

**The obvious diagnosis is wrong, and this establishes that rather than
assuming it.** `.github/workflows/ci.yml` fires on `pull_request`, and
`actions/checkout` resolves `refs/pull/N/merge` on that event — the merge
result, already. The run for PR #16 that same morning says so verbatim:

    git fetch ... +9e18354...:refs/remotes/pull/16/merge
    HEAD is now at 9e18354 Merge 96822a4... into e9d8c4d...

So "check out something different" fixes nothing; it is what already happens.

Worse, and also measured rather than assumed: **for #14 and #15 that workflow
never ran at all.** `actions/runs?head_sha=e3f8621` and `?head_sha=765b2c1`
both return `total_count: 0`, `commits/<sha>/check-runs` likewise, and
`gh pr checks` says "no checks reported" on both branches. "Green on its own
base" was each agent's own local suite run, never a CI verdict. Every
`pull_request` run on this repository that day except one was `failure`, so the
signal was not one anybody could have read anyway.

Two real defects, both covered here:

1. The merge result is computed against the base **as it stood when the event
   fired**, and GitHub never re-fires `pull_request` when the base branch
   moves. #14 and #15 were both computed against e9d8c4d; #14 landed on
   02e61fd and #15 landed on top of #14. No run ever saw the tree that shipped.
2. Nothing ever tests two OPEN changes together. Each PR is merged with the
   base and nothing else, so a pair that only disagrees with each other stays
   invisible until one of them lands.

`tests/merge-check` merges the base tip with every candidate at once and runs
the suite on that tree. If it is red it re-runs the failing check on the base
alone (pre-existing — attributed to nobody), on each candidate alone (that
candidate's own regression, named on its own), and on each surviving pair.
Green apart, red together, is the pair. That differential is what makes the
signal attributable instead of "the suite is red".

Rejected: filtering candidates by whether their diffs touch the same files.
#14 touched `schema/state-schema.json` and `bin/perry-lint`; #15 touched
`bin/perry-state-cost` and its test — **zero overlap**. Any same-files shortcut
misses exactly the failure this exists for. Also rejected: a `pull_request`-only
job, which cannot fire when the base moves, and re-running everything on every
merge, which reports no pair and so is not a check.

merge-check.yml is a separate workflow because its triggers differ: it also
runs on push to an integration branch, which is precisely the moment a base
moves under the PRs still open against it.

The scratch clone is `--shared`, so nothing is checked out, staged or cleaned
in the repository it runs from — safe from a worktree somebody else is in.

**Needs repository admin and is deliberately not attempted here**: marking
`merge-check` a required status check in branch protection. Until a repo admin
applies that, this reports; it cannot refuse a merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first CI run of `merge-check.yml` failed in 9 seconds:

    python3: can't open file '.../tests/merge-check': No such file or directory

It checked out the BASE branch, and the base does not have `tests/merge-check`
— the script arrives with this very PR. A checker pinned to the base cannot
bootstrap itself, and can never be improved by the PR that improves it.

The checkout now takes the DEFAULT ref, which on a `pull_request` is the merge
result. That tree always has the script, and it matches what GitHub already
does with the workflow file on that event, so the yaml and the script it runs
come from the same commit. The checkout is only the source of the script;
every tree the suite runs on is still built by `merge-check` in its own clone
from the base sha and the fetched PR heads.

That move broke base resolution, because `git clone` maps the source's
`refs/heads/*` and nothing else — a base held only as `refs/remotes/origin/<b>`
has objects but no name in the clone. `Scratch.find` now tries both spellings
and then fetches by explicit refspec, which is the same fallback the PR heads
already used; the two are now one method instead of two.

Verified after the refactor: the reconstructed pair still reports
`CONFLICTING PAIR — pr14 × pr15 · test_state_cost.py` (exit 1), and
`--pr 23 --pr 24` against the live base still reports no pair (exit 0).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second CI failure, at the fetch step:

    ! [new ref] refs/pull/27/head -> pull/27 (unable to update local ref)
    error: some local refs could not be updated

actions/checkout has already written `refs/remotes/pull/27/merge` for the PR
being tested, which makes `refs/remotes/pull/27` a DIRECTORY. A ref cannot also
live at that path. `--force` does nothing about a D/F collision — the
destination has to be a sibling, so it is now `refs/remotes/pull/*/head`.

The script's fallback chain learns the same spelling, and keeps the old one
after it for a caller that fetched the flat layout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Run 32391788811, 3m26s, against base `feat/work-modes @ 60916fb`:

      … merging 1 candidate(s) onto 60916fb
      … running the suite on the merged result
      … 1 check(s) red — baselining them on the base alone
    pre-existing on the base — attributed to nobody:
      · test_host_support.py
    ✓ nothing new is red on the merged result

That is the pre-existing-baseline step earning its place on the first live
run: `test_host_support` was red on the merged tree, and without checking the
base alone this would have blamed the only open PR for a failure the base
already had.

`timeout-minutes: 60` caps the quadratic pair search, which only runs when the
combined tree is red.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant