diff --git a/quest/m0/README.md b/quest/m0/README.md index aa77a9c2..eaf5ced7 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -85,7 +85,8 @@ 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 +- [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` 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 new file mode 100644 index 00000000..813f5794 --- /dev/null +++ b/quest/m0/latest-check-runs.md @@ -0,0 +1,108 @@ +# [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, 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 +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, 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 +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; the two halves complete separately. + +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 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 + 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` clears a stale `landing-stopped` reason diff --git a/quest/m0/status-landing.md b/quest/m0/status-landing.md index 8155fc6e..a2189336 100644 --- a/quest/m0/status-landing.md +++ b/quest/m0/status-landing.md @@ -1,11 +1,10 @@ -# [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. +`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 @@ -14,15 +13,21 @@ 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. -- 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/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/m0/latest-check-runs.md) - superseded runs stop failing `quest status`, `quest wait`, and the gate