diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b040db1b0..ca81dcea1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Scheduler holds pre-review branch updates while checks are in flight + +- `inspect_pr` now decides `wait` instead of `update_branch` when a behind, unreviewed head still has queued or running check runs (`has_in_flight_check_runs`, built on the existing `latest_check_runs`/`running_check_state`). Under a saturated runner queue each PR's own delayed `pull_request_target` scheduler run merged `main` into the head before review dispatch, cancelling every queued check on the old head (22/28 on #1926, 21/30 on #1484) and requeueing the PR at the back, so no head ever completed its checks: 76 of the 77 PRs merged into this repository since 2026-09-04 had 0/12 required contexts satisfied at merge time. The hold has no age cap on purpose -- a check that never finishes keeps the head in place instead of restarting that loop, and the update resumes once every newest check run is terminal. `CLAUDE.md` now describes both update paths. Tracked in #1935. + ### CodeQL scan dispatch matrix serialisation - Serialised the dispatched CodeQL matrix with `toJSON()` in `codeql-scan-dispatch.yml`. `codeql-pr.yml` sends `client_payload.matrix` as an array and the handler assigned it straight into `env:`, where a value must be a scalar, so GitHub rejected the step with "A sequence was not expected" and the dispatched scan never ran -- 0 successes against 136 failures since the handler was added in #1776. The validate step already consumes the value through `jq`, so JSON text is the shape it was written for and no consumer changes. Added a string contract test, because neither `yaml.safe_load` nor `actionlint` 1.7.12 flags this: it is an Actions template rule, so only GitHub's own validator rejects it and no local gate catches the class. diff --git a/CLAUDE.md b/CLAUDE.md index f1b2cbb482..30db1fc23b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,9 +48,10 @@ an actually-executed PoC via `scripts/ci/sandboxed_verify.py` or `scripts/ci/san split `Developer experience:` / `User experience:` sections). Deterministic code may repair only trusted `path:line` bindings on LLM probes that already carry an independent proof and source-line digest; it never invents observed -results. The scheduler updates a PR branch only -when the latest review is approved, no current-head check has failed, and GitHub reports the PR as -behind. The mechanical merge scheduler itself never synthesizes a fix: it gives `DIRTY`/`CONFLICTING` +results. The scheduler updates a PR branch in two cases: after approval, when no current-head check +has failed and GitHub reports the PR as behind; and before review dispatch, when the PR is behind and +no current-head check is still queued or running (an in-flight check is evidence the update would +discard; see #1935). The mechanical merge scheduler itself never synthesizes a fix: it gives `DIRTY`/`CONFLICTING` PRs repair guidance. A separate edit-capable autofix flow (`scripts/ci/pr_review_fix_scheduler.py` → `.github/workflows/pr-review-autofix.yml`) may, for an approved same-repository-head PR, merge the base into the head and resolve the conflict markers; the diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index e97b41074a..c4e9d28ebd 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -1904,6 +1904,11 @@ def opencode_in_progress(pr: dict[str, Any], *, stale_after_minutes: int | None return opencode_progress_state(pr, stale_after_minutes=stale_after) == "running" +def has_in_flight_check_runs(pr: dict[str, Any]) -> bool: + """Return whether any newest current-head check run is still queued or running.""" + return any(running_check_state(node) == "running" for node in latest_check_runs(pr)) + + _STRIX_SUCCESS_CONCLUSIONS = {"SUCCESS"} @@ -4781,6 +4786,19 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio f"current head has no OpenCode approval; branch is outdated before review dispatch, " f"but head repo {head_repo} is not writable by the scheduler credential", ) + if has_in_flight_check_runs(pr): + # Updating now would cancel every queued or running check on the + # current head and requeue the pull request behind them. Under a + # saturated runner queue the PR's own delayed scheduler run does + # this on every execution, so no head ever finishes its checks + # (#1935). Deliberately no age cap: a check that never finishes + # keeps the head where it is instead of restarting that loop. + return decide( + "wait", + "current head has no OpenCode approval; branch is outdated before review dispatch, " + "but current-head checks are still queued or running; holding the update so their " + "evidence is not discarded", + ) if merge_state == "BEHIND": freshness_reason = "current head has no OpenCode approval; branch is outdated before review dispatch" else: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 1e5848aac3..2cbda7f85b 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -10648,3 +10648,50 @@ def test_reconcile_releases_strix_lease_when_no_run_was_created(tmp_path): record = next(iter(load_state_file(gate.state_path).records.values())) assert record.status == "stale" + + +def test_inspect_pr_holds_pre_review_update_while_current_head_checks_run(): + """A behind, unreviewed head keeps its queued checks instead of being updated (#1935). + + Under a saturated queue the PR's own delayed scheduler run used to merge + ``main`` into the head before review dispatch, cancelling every queued + check on the old head and requeueing the PR behind them. The hold has no + age cap on purpose: a check that never finishes keeps the head in place + rather than restarting that loop, and the update resumes as soon as every + newest check run has a terminal status. + """ + + def behind_with(nodes): + return make_pr( + mergeStateStatus="BEHIND", + statusCheckRollup={"contexts": {"nodes": nodes}}, + ) + + held = inspect( + behind_with( + [ + {"__typename": "CheckRun", "name": "trivy-fs", "status": "QUEUED", "conclusion": None}, + {"__typename": "CheckRun", "name": "scan-pr-queue", "status": "IN_PROGRESS", "conclusion": None}, + {"__typename": "CheckRun", "name": "osv-scan", "status": "COMPLETED", "conclusion": "SUCCESS"}, + ] + ) + ) + assert held.action == "wait" + assert "branch is outdated before review dispatch" in held.reason + assert "checks are still queued or running" in held.reason + + resumed = inspect( + behind_with( + [ + {"__typename": "CheckRun", "name": "trivy-fs", "status": "COMPLETED", "conclusion": "SUCCESS"}, + {"__typename": "CheckRun", "name": "scan-pr-queue", "status": "COMPLETED", "conclusion": "SKIPPED"}, + ] + ) + ) + assert resumed.action == "update_branch" + assert resumed.reason.startswith( + "current head has no OpenCode approval; branch is outdated before review dispatch" + ) + assert "checks are still queued or running" not in resumed.reason + + assert sched.has_in_flight_check_runs(behind_with([])) is False