Revert "fix(scheduler): skip review dispatch for a head whose merge tree cannot materialize (#1973)"

[seonghobae]

Reverts ad0779be (#1973). I wrote that guard yesterday and its justification is false.

The claim I merged

"Materializing the PR merge tree is a hard precondition of coverage-source-tree,
so a conflicting head can only produce a failed dispatch."

The run object does conclude failure. The review is published anyway — the
reviewer reads the pull request diff, not a merge tree. Its own words, from the
body it posted:

Pull request overview

OpenCode reviewed the current-head product diff. Coverage is a separate gate.

coverage-source-tree is the job that needs the merge commit, and the
opencode-review job does not depend on it.

Evidence

On .github#1529, the PR that motivated the guard, the last of its 27 dispatches
published a 2404-character review at 2026-09-05T19:40:58Z. Attribution is not
circumstantial: the review body carries its own run id, 33969161561, and
that run's opencode-review job concluded failure at the receipt gate nine
seconds later.

run 33969161561   created 2026-09-05T13:31:58Z   updated 19:41:08Z   failure
  validate-pr-metadata   success
  coverage-source-tree   failure   ← the merge tree, as expected
  coverage-evidence      failure
  opencode-review        failure   ← published the review, then failed the gate

It is the only OpenCode review that head has (4 reviews total, 1 by
opencode-agent[bot]), and it is why the PR reads as reviewed today. The guard
would have discarded it.

Not unique to #1529: .github#1555 is CONFLICTING right now and its current
head carries a 2019-character CHANGES_REQUESTED review from the same reviewer.
#789 has 10 OpenCode reviews, #1273 has 4.

The cost argument was wrong by two orders of magnitude

I justified the guard as protecting runner capacity. Across all 27 dispatches of
that head, 109 jobs, split by runner_id:

allocated a runner   27 jobs    0.24 h total   ← fourteen minutes
never allocated      82 jobs
queue residency                69.91 h

Blocking the whole chain would have saved fourteen minutes of runner time and
lost the review. I reached "produced nothing" by reading run conclusions, which
are roll-ups that cannot name what a run did.

The repeat is real, and is fixed at its cause instead

Not an abandonment — the purpose is carried by
#1983.
active_review_run_refs matched a run's name exactly against the review
workflow aliases, but the central review workflows define run-name:, so GitHub
sends the rendered title in that field and every dispatch run was filtered out
before already_running could see it. With that repaired, a conflicting head
receives one dispatch and the next is suppressed while it runs — the outcome this
guard reached for, without discarding the review.

A narrowed replacement (one dispatch per head) was written and discarded: its
history lookup would paginate 39,914 completed repository_dispatch runs per
conflicting PR per scheduler pass (4,669 even at a 7-day window), creating the
rate-limit problem the backlog complains about.

What this does not fix, stated plainly

Review publication is currently rare for reasons upstream of this change. A
parallel measurement by another session over 300 dispatch runs (295 with a
parseable subject, not filtered by conclusion):

runs that published a review    2 / 295
conclusions                     failure 285 / cancelled 9 / success 1
both publishing runs concluded  failure, cancelled

Most runs die at the dispatch actor gate (.github#1929, an owner-gated
variable). So this revert reopens a path that is largely blocked one level up:
it is the correct behaviour, but its effect today is small. It is worth doing
now because the guard is wrong on its own terms and would silently discard the
reviews that do get through — including the only one #1529 ever received.

Those two publishing runs also reconfirm the premise independently: both
concluded failure/cancelled and both published.

Evidence

gate         2964 passed, 1 skipped, 21 subtests   (predicted 2968 - 4)
coverage     100%, 0 missing
interrogate  100%
revert       clean, -29 source / -177 tests, no manual edits

Developer experience: the scheduler no longer prints a skip reason that asserts
an outcome contradicted by the runs it cites.
User experience: a conflicting pull request receives a review and repair
guidance, instead of repair guidance alone.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 0dd6eeb9-57cb-47b9-bae8-e85c246f6269

📥 Commits

Reviewing files that changed from the base of the PR and between 49eb9e7 and c3b66b1.

📒 Files selected for processing (2)

• scripts/ci/pr_review_merge_scheduler_core.py
• tests/test_pr_review_merge_scheduler.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

Verified by running it. The premise holds on a wider sample than the one in the description, and the revert is clean. This reverts my own guard, so the premise was the part worth attacking.

The premise, tested where it can fail

The claim is that the reviewer does not need a merge tree, so a conflicting head can still be reviewed. Falsifiable directly: count reviews that exist on conflicting heads.

Across every open non-draft CONFLICTING pull request here:

	count
carry an opencode-agent[bot] review at the current conflicting head	5 of 15
carry one at some head	10 of 15

The five are #939, #1009, #1065, #1529 and #1555. Three of them are based on main, so this is not a property of stacked pull requests. Reviewing a conflicting head demonstrably works, which is exactly what the comment I wrote into dispatch_opencode_review denied.

A sampling trap worth recording, because it nearly halved this evidence. My first poll of the same query returned only 2 conflicting pull requests, and I almost reported "2 of 2". GraphQL computes mergeable lazily: the first request returned UNKNOWN for the 13 it had not computed yet, which silently dropped them from the filter. A second poll returned all 15. Any census filtering on mergeable needs to be polled until the field settles, or it truncates without saying so.

The revert itself

check	result
pytest	2965 passed, 1 skipped, 21 subtests
coverage (scripts/ci)	100%, 0 missed of 13181
interrogate	100%
merge-tree vs main	clean

Arithmetic predicted before running: main measured 2969 with #1980 in it, minus the 4 tests my change added, gives 2965. That is what ran, so this removes exactly what it should and nothing else.

Two things I checked because the raw diff looked alarming, and both were fine:

• merge_conflict still appears six times in the core module. Those are the pre-existing merge_conflict_guidance and merge_conflict_repair paths, untouched. My dispatch_result == "merge_conflict" branches are gone from all six call sites, and the guard block itself is at zero occurrences.
• git diff main HEAD shows 26 deletions in the queue-contract file, which is test(actions): pin cancellation for the two PR-keyed scans that only had presence #1980's new test. That is an artifact of the branch predating test(actions): pin cancellation for the two PR-keyed scans that only had presence #1980; in the merged tree that test is present. Merging this does not undo test(actions): pin cancellation for the two PR-keyed scans that only had presence #1980.

One point for the record

My guard's stated justification was that a conflicting head "can only produce a failed dispatch". That inference came from reading run conclusion and treating it as a statement about output, which is the rollup-field trap I had logged myself the day before. The evidence above is the direct refutation.

🤖 Addressed by Claude Code