From e6fc861d9b6e01dac7f27d0320730b2f5937848e Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 18:55:44 -0800 Subject: [PATCH 1/3] quest: plan judging checks by their latest run on the head quest status, quest wait, and the quest land gate count every check run on a head, so a run that a newer run of the same check replaced still fails them although GitHub merges the head (moq-dev/moq#7325). Plan the fix in m1 and split it from the m0 landing-status quest. Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 2 +- quest/m0/status-landing.md | 22 ++++---- quest/m1/README.md | 1 + quest/m1/latest-check-runs.md | 98 +++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 11 deletions(-) create mode 100644 quest/m1/latest-check-runs.md diff --git a/quest/m0/README.md b/quest/m0/README.md index aa77a9c2..75e82c05 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -85,7 +85,7 @@ ownership-manifest design: - [Land through the merge queue with an async fixer](/quest/m0/queue-watcher/README.md) - GitHub's merge queue lands one PR at a time, and a host watcher fixes and re-enqueues each PR it drops - [Merge through a host gate](/quest/m0/host-land/README.md) - the host's fixer containers, and the frozen batched gate for the repositories not yet switched - [The land fixer resolves every mechanical failure](/quest/m0/land-fixer/README.md) - lockfile merges, protected edits as ready commits, workflow pushes, and the PR's own CI as context -- [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line and ignores superseded cancelled checks +- [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line once the PR is queued again - [Agents don't stall](/quest/m0/agent-stalls.md) - long waits block in the same turn, pre-existing failures don't stop review, and reviewer outages become waits - [Follow-ups as agents report](/quest/m0/follow-ups-as-reports.md) - coordinators plan follow-ups as reports arrive, fewer of them, with standing milestone rules - [Context before decision prompts](/quest/m0/prompt-context.md) - every prompt explains the work first, and maintainer steps say why an agent can't take them diff --git a/quest/m0/status-landing.md b/quest/m0/status-landing.md index 8155fc6e..90c5bc18 100644 --- a/quest/m0/status-landing.md +++ b/quest/m0/status-landing.md @@ -1,11 +1,9 @@ -# [XS] quest status reports landing and checks as they are now +# [XS] quest status reports landing as it is now ## Goal `quest status` stops printing a `landing-stopped` line once the PR has been -queued again, including through `quest merge --land`. A required check whose -newest run passed counts as passed, even if an older run of it was -cancelled. +queued again, including through `quest merge --land`. ## Plan @@ -16,13 +14,17 @@ Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq disable, or ejection comment from the timeline, even if auto-merge was turned on again later. Drop the line when auto-merge's `enabledAt`, or a merge-queue entry, is newer than that event. -- Checks are classified one context at a time in `summarize` (`classifyCheck`). - A cancelled run and a newer passing run of the same check both count, so - the check still shows as failing. Count only the newest run of each check - name. Moved from m2 to m0 on 2026-11-01 (user, /quest-plan of the follow-ups from the 2026-11-01 `/quest-complete` sessions): ✅ m0, beside the land quests · leave it in m2. The moq-dev land gate and the agents picking merges read -`quest status` every day, so a stale `landing-stopped` line or a superseded -cancelled check sends them after PRs that are already queued and passing. +`quest status` every day, so a stale `landing-stopped` line sends them after +PRs that are already queued. + +Its other half, counting only the newest run of each check, moved to +[Checks are judged by their latest run](/quest/m1/latest-check-runs.md) on +2026-11-02, when the maintainer picked m1 for its fuller plan. + +## Related + +- [Checks are judged by their latest run](/quest/m1/latest-check-runs.md) - superseded runs stop failing `quest status`, `quest wait`, and the gate diff --git a/quest/m1/README.md b/quest/m1/README.md index 1c238be9..435ea7b1 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -27,6 +27,7 @@ earlier "Later work" milestone became m2. - [Launch material](/quest/m1/launch.md) - README, quickstart, demo, comparison, and launch-post drafts - [Workers in a 5-minute cache](/quest/m1/worker-cache.md) - re-plan #276's worker/coordinator split and gate tickets, after confirming the cache premise - [Cancelling a gate stops its active phase](/quest/m1/gate-signals.md) - SIGTERM/SIGHUP reach every process the gate owns, so Bash traps and GNU `time` can't defer cancelling a phase; unblocks moq-dev/moq.pro#3137 +- [Checks are judged by their latest run](/quest/m1/latest-check-runs.md) - `quest status`, `quest wait`, and the gate count each check's latest run on the head, as GitHub does, so a superseded cancelled or failed run stops failing them - [quest wait after a merge](/quest/m1/wait-merge-heads.md) - `quest wait` resolves a head that reached the branch by a merge into it - [One PR's collection failure can't stall its fixer job](/quest/m1/fixer-isolate-failure.md) - errors stay on their PR, and a PR that keeps failing is set aside - [Retire Decisions needed](/quest/m1/decisions-needed.md) - only `## Waiting` hands a draft back, once no open draft uses the old heading diff --git a/quest/m1/latest-check-runs.md b/quest/m1/latest-check-runs.md new file mode 100644 index 00000000..c5647fcd --- /dev/null +++ b/quest/m1/latest-check-runs.md @@ -0,0 +1,98 @@ +# [S] Checks are judged by their latest run on the head + +## Goal + +`quest status`, `quest wait`, and the `quest land` gate judge each check on +a PR's head by its latest run, as GitHub's required-check rollup does. An +older run that a newer run of the same check replaced no longer prints, +counts, wakes a wait, or fails the gate, whether it was cancelled, failed, +or passed. `quest status`'s `checks` state follows those latest runs, not the +rollup's own `state`, which counts every run. The Worker's job context for a +PR reports checks the same way. + +## Plan + +Decided 2026-11-02 (maintainer): plan it in m1. Raised by +[moq-dev/moq#7325](https://github.com/moq-dev/moq/pull/7325), release-plz's +release PR. Each refresh pushes and edits the description at once, starting +two `Release gate` runs, and concurrency cancels one. The cancelled run's +gate fails on purpose; the other run on the same head passes, and GitHub's +merge state counted the pass (CLEAN). `quest status` still printed +`check failing required failure … Release gate`, which misleads merge +decisions. Split from [Current landing status](/quest/m0/status-landing.md), +which owned a one-line version of this in m0. The maintainer's newer m1 pick +for the fuller outcome moved it here. + +Evidence on #7325's head `a15652d`, read 2026-11-02: + +- The rollup's `contexts` list both `Release gate` check runs, from the same + app (GitHub Actions), both `isRequired`. Check suite `112631417489` + (run [41564598600](https://github.com/moq-dev/moq/actions/runs/41564598600)) + has the failure, started 02:12:02Z, and a cancelled `Scope`. Check suite + `112631424074` (run [41564601164](https://github.com/moq-dev/moq/actions/runs/41564601164)) + has the success, started 02:34:46Z. +- The rollup's own `state` is `FAILURE`, and its + `checkRunCountsByState` counts the cancelled and failed runs. +- REST `commits/{sha}/check-runs?filter=latest` returns both runs too. It + keeps only the latest run per check suite, so it collapses a job's re-runs, + not separate workflow runs. +- GitHub does not document which run counts. "About protected branches" only + says duplicate job names across workflows give ambiguous results. The + observed rule is that the newer run of the name, from the app, counts. +- `gh pr checks` keeps the run with the latest `startedAt` per check name, + workflow name, and event (`eliminateDuplicates` in cli/cli's + `pkg/cmd/pr/checks/aggregate.go`). + +Where checks are judged today: + +- `summarize` in `src/cli/status.ts` classifies every context with + `classifyCheck` and prints `rollup.state` as the `checks` state. +- `quest wait` (`check` and `checkActivities` in `src/cli/wait.ts`) reads + every context, so a superseded failure can wake it with `checks:failed`. +- The gate's `checks` in `src/cli/land.ts` fails a name when any of its + reports fails: "A name reported twice, as by a rerun, counts only once + every report passes", from + [moq-dev/quest#229](https://github.com/moq-dev/quest/pull/229). Reverse + it. GitHub merges that head, so the stricter rule rejects PRs GitHub + accepts, such as each release PR that hits this race. +- `readTarget` in `cloudflare/src/github/service.ts` gives a job's agent + every run and the rollup's `state`. + +Decisions made while planning, for review in the PR: + +- One helper both builds import keeps the latest contexts. Put it in + `src/github/` (the Worker already imports `src/github/merge.ts`), not in + `src/cli/status.ts`. Status, wait, land, and the Worker filter through it + before classifying. +- Key a check run by its app (`checkSuite.app.databaseId`) and name. That is + the identity a required check has in branch protection and rulesets, and + the one `quest land` already matches a requirement on. Key a commit status + by its context. A check run and a status with the same name stay separate, + since GitHub requires both. Rejected: `gh pr checks`' key of name, + workflow name, and event. It can keep two runs that GitHub's requirement + counts once, so the list would disagree with mergeability. +- The latest check run is the one created last, the highest `databaseId`. + A newer run that is still queued has no `startedAt`, but it already makes + the check pending. The latest status is the one created last. +- The `checks` state is derived from the latest contexts: `failure` if any + fails, else `pending` if any is pending, else `success`. With no rollup it + stays `-`. Keep the line's format; update the README's `checks` row, which + says it is the rollup state. +- Only the latest runs count toward `passed=`, and only they print as + `check failing` or `check pending` lines. + +Tests: + +- A fixture recorded from #7325: a suite with a cancelled `Scope` and a + failed `Release gate`, and a newer suite where both passed. `quest status` + prints `checks success` with no failing line. `quest wait` never reports + `checks:failed` for it and reports `checks:passed` once the head is idle. + The gate counts the head passing. The Worker's job context lists the newer + runs only. +- A newer queued run of a passed check makes it pending. A newer failure + after a pass fails. The same name from two apps counts twice, and a check + run and a status that share a name both count. + +## Related + +- [Current landing status](/quest/m0/status-landing.md) - the other half of the old quest: `quest status` drops a stale `landing-stopped` line From d53b720e939f574e06cb88000da491738709e26d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 19:03:19 -0800 Subject: [PATCH 2/3] quest: move latest-check-runs to m0 The quest land gate fails a check name if any run with that name failed (since moq-dev/quest#229), so it can wrongly refuse release-plz PRs that GitHub would merge. Maintainer decision 2026-11-02: land work, m0. Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 1 + quest/{m1 => m0}/latest-check-runs.md | 9 ++++++--- quest/m0/status-landing.md | 7 ++++--- quest/m1/README.md | 1 - 4 files changed, 11 insertions(+), 7 deletions(-) rename quest/{m1 => m0}/latest-check-runs.md (92%) diff --git a/quest/m0/README.md b/quest/m0/README.md index 75e82c05..09ea9f67 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -85,6 +85,7 @@ ownership-manifest design: - [Land through the merge queue with an async fixer](/quest/m0/queue-watcher/README.md) - GitHub's merge queue lands one PR at a time, and a host watcher fixes and re-enqueues each PR it drops - [Merge through a host gate](/quest/m0/host-land/README.md) - the host's fixer containers, and the frozen batched gate for the repositories not yet switched - [The land fixer resolves every mechanical failure](/quest/m0/land-fixer/README.md) - lockfile merges, protected edits as ready commits, workflow pushes, and the PR's own CI as context +- [Checks are judged by their latest run](/quest/m0/latest-check-runs.md) - `quest status`, `quest wait`, and the gate count each check's latest run on the head, as GitHub does, so a superseded cancelled or failed run stops failing them or refusing release-plz PRs GitHub would merge - [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line once the PR is queued again - [Agents don't stall](/quest/m0/agent-stalls.md) - long waits block in the same turn, pre-existing failures don't stop review, and reviewer outages become waits - [Follow-ups as agents report](/quest/m0/follow-ups-as-reports.md) - coordinators plan follow-ups as reports arrive, fewer of them, with standing milestone rules diff --git a/quest/m1/latest-check-runs.md b/quest/m0/latest-check-runs.md similarity index 92% rename from quest/m1/latest-check-runs.md rename to quest/m0/latest-check-runs.md index c5647fcd..76e5e268 100644 --- a/quest/m1/latest-check-runs.md +++ b/quest/m0/latest-check-runs.md @@ -12,7 +12,11 @@ PR reports checks the same way. ## Plan -Decided 2026-11-02 (maintainer): plan it in m1. Raised by +Decided 2026-11-02 (maintainer): plan it in m1, then, the same day, move +it to m0. Since [moq-dev/quest#229](https://github.com/moq-dev/quest/pull/229) +the `quest land` gate fails a check name if any run with that name failed, so +it can wrongly refuse release-plz PRs that GitHub would merge. That is land +work, which m0 ranks first. Raised by [moq-dev/moq#7325](https://github.com/moq-dev/moq/pull/7325), release-plz's release PR. Each refresh pushes and edits the description at once, starting two `Release gate` runs, and concurrency cancels one. The cancelled run's @@ -20,8 +24,7 @@ gate fails on purpose; the other run on the same head passes, and GitHub's merge state counted the pass (CLEAN). `quest status` still printed `check failing required failure … Release gate`, which misleads merge decisions. Split from [Current landing status](/quest/m0/status-landing.md), -which owned a one-line version of this in m0. The maintainer's newer m1 pick -for the fuller outcome moved it here. +which owned a one-line version of this; the two halves complete separately. Evidence on #7325's head `a15652d`, read 2026-11-02: diff --git a/quest/m0/status-landing.md b/quest/m0/status-landing.md index 90c5bc18..30f651ce 100644 --- a/quest/m0/status-landing.md +++ b/quest/m0/status-landing.md @@ -22,9 +22,10 @@ leave it in m2. The moq-dev land gate and the agents picking merges read PRs that are already queued. Its other half, counting only the newest run of each check, moved to -[Checks are judged by their latest run](/quest/m1/latest-check-runs.md) on -2026-11-02, when the maintainer picked m1 for its fuller plan. +[Checks are judged by their latest run](/quest/m0/latest-check-runs.md) on +2026-11-02 for its fuller plan, which also covers `quest wait`, the gate, and +the Worker. It stays in m0 beside this quest. ## Related -- [Checks are judged by their latest run](/quest/m1/latest-check-runs.md) - superseded runs stop failing `quest status`, `quest wait`, and the gate +- [Checks are judged by their latest run](/quest/m0/latest-check-runs.md) - superseded runs stop failing `quest status`, `quest wait`, and the gate diff --git a/quest/m1/README.md b/quest/m1/README.md index 435ea7b1..1c238be9 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -27,7 +27,6 @@ earlier "Later work" milestone became m2. - [Launch material](/quest/m1/launch.md) - README, quickstart, demo, comparison, and launch-post drafts - [Workers in a 5-minute cache](/quest/m1/worker-cache.md) - re-plan #276's worker/coordinator split and gate tickets, after confirming the cache premise - [Cancelling a gate stops its active phase](/quest/m1/gate-signals.md) - SIGTERM/SIGHUP reach every process the gate owns, so Bash traps and GNU `time` can't defer cancelling a phase; unblocks moq-dev/moq.pro#3137 -- [Checks are judged by their latest run](/quest/m1/latest-check-runs.md) - `quest status`, `quest wait`, and the gate count each check's latest run on the head, as GitHub does, so a superseded cancelled or failed run stops failing them - [quest wait after a merge](/quest/m1/wait-merge-heads.md) - `quest wait` resolves a head that reached the branch by a merge into it - [One PR's collection failure can't stall its fixer job](/quest/m1/fixer-isolate-failure.md) - errors stay on their PR, and a PR that keeps failing is set aside - [Retire Decisions needed](/quest/m1/decisions-needed.md) - only `## Waiting` hands a draft back, once no open draft uses the old heading From c4d08d6d1466b07f8456bf3ae061f567d0e55cd2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 19:06:20 -0800 Subject: [PATCH 3/3] quest: address CodeRabbit findings on latest-check-runs and status-landing Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 2 +- quest/m0/latest-check-runs.md | 13 ++++++++++--- quest/m0/status-landing.md | 8 +++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/quest/m0/README.md b/quest/m0/README.md index 09ea9f67..eaf5ced7 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -86,7 +86,7 @@ ownership-manifest design: - [Merge through a host gate](/quest/m0/host-land/README.md) - the host's fixer containers, and the frozen batched gate for the repositories not yet switched - [The land fixer resolves every mechanical failure](/quest/m0/land-fixer/README.md) - lockfile merges, protected edits as ready commits, workflow pushes, and the PR's own CI as context - [Checks are judged by their latest run](/quest/m0/latest-check-runs.md) - `quest status`, `quest wait`, and the gate count each check's latest run on the head, as GitHub does, so a superseded cancelled or failed run stops failing them or refusing release-plz PRs GitHub would merge -- [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line once the PR is queued again +- [Current landing status](/quest/m0/status-landing.md) - `quest status` clears a stale `landing-stopped` reason once the PR is queued again - [Agents don't stall](/quest/m0/agent-stalls.md) - long waits block in the same turn, pre-existing failures don't stop review, and reviewer outages become waits - [Follow-ups as agents report](/quest/m0/follow-ups-as-reports.md) - coordinators plan follow-ups as reports arrive, fewer of them, with standing milestone rules - [Context before decision prompts](/quest/m0/prompt-context.md) - every prompt explains the work first, and maintainer steps say why an agent can't take them diff --git a/quest/m0/latest-check-runs.md b/quest/m0/latest-check-runs.md index 76e5e268..813f5794 100644 --- a/quest/m0/latest-check-runs.md +++ b/quest/m0/latest-check-runs.md @@ -3,7 +3,8 @@ ## Goal `quest status`, `quest wait`, and the `quest land` gate judge each check on -a PR's head by its latest run, as GitHub's required-check rollup does. An +a PR's head by its latest run, matching the merge state GitHub reported on +[moq-dev/moq#7325](https://github.com/moq-dev/moq/pull/7325). An older run that a newer run of the same check replaced no longer prints, counts, wakes a wait, or fails the gate, whether it was cancelled, failed, or passed. `quest status`'s `checks` state follows those latest runs, not the @@ -76,7 +77,13 @@ Decisions made while planning, for review in the PR: counts once, so the list would disagree with mergeability. - The latest check run is the one created last, the highest `databaseId`. A newer run that is still queued has no `startedAt`, but it already makes - the check pending. The latest status is the one created last. + the check pending. The latest status is the one with the newest + `createdAt`; on a tie, the one listed later in the rollup. +- Every producer fetches the fields the helper needs, since none has them + all today: `CheckRun.databaseId` and `checkSuite { app { databaseId } }` + on check runs, and `createdAt` on statuses. `status.ts` has none of them, + `wait.ts` lacks the check run's id and app, `land.ts` has only the app, + and the Worker's `readTarget` query has none. - The `checks` state is derived from the latest contexts: `failure` if any fails, else `pending` if any is pending, else `success`. With no rollup it stays `-`. Keep the line's format; update the README's `checks` row, which @@ -98,4 +105,4 @@ Tests: ## Related -- [Current landing status](/quest/m0/status-landing.md) - the other half of the old quest: `quest status` drops a stale `landing-stopped` line +- [Current landing status](/quest/m0/status-landing.md) - the other half of the old quest: `quest status` clears a stale `landing-stopped` reason diff --git a/quest/m0/status-landing.md b/quest/m0/status-landing.md index 30f651ce..a2189336 100644 --- a/quest/m0/status-landing.md +++ b/quest/m0/status-landing.md @@ -2,8 +2,9 @@ ## Goal -`quest status` stops printing a `landing-stopped` line once the PR has been -queued again, including through `quest merge --land`. +`quest status` prints `landing-stopped -` once the PR has been queued again, +including through `quest merge --land`, instead of the reason the earlier +entry ended. ## Plan @@ -12,7 +13,8 @@ Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq - `summarize` in `src/cli/status.ts` takes the last removal, auto-merge disable, or ejection comment from the timeline, even if auto-merge was - turned on again later. Drop the line when auto-merge's `enabledAt`, or a + turned on again later. Report no reason (`landing-stopped -`, as + `statusLines` already prints for none) when auto-merge's `enabledAt`, or a merge-queue entry, is newer than that event. Moved from m2 to m0 on 2026-11-01 (user, /quest-plan of the follow-ups from