[Bug]: PR watch reports "All checks passed" while a second run of the same check is still in progress #17207

Activity

1. juliusmarminge commented on Oct 8, 2026

   

   juliusmarminge

   on Oct 8, 2026

   Member

   More actions

   Note

   Grok responding on behalf of Julius.

   Thanks for the detailed report and the timeline. Reading the code on main (12069ee), the cause looks the way you describe it. I traced it in the code but didn't reproduce it at runtime.

   What happens

   • toCheckEntries dates each run by realTimestamp(completedAt) ?? realTimestamp(startedAt) (apps/server/src/pullRequest/gitHubPullRequestJson.ts:1614).
   • dedupeChecks keys runs by `${workflowName} ${name}` (apps/server/src/pullRequest/pullRequestChecks.ts:39) and keeps whichever has the newest at (:42, compared as text in isAtLeastAsNew at :4-7). The detail read only asks for checkSuite { workflowRun { workflow { name } } } (gitHubPullRequestJson.ts:817-820), so nothing else separates the push run from the pull_request run.
   • With your data, the finished run's 04:18:58Z beats the live run's 04:06:13Z, and the live run is dropped. A run that's still queued with no startedAt loses to any dated run (pullRequestChecks.ts:5), so it would be dropped too.
   • evaluatePullRequestWatch builds its gate from those already-deduped detail.checks (apps/server/src/orchestration-v2/pullRequestWatch.ts:64-66). With no required checks, the gate is that one surviving row, and it wakes with checks-passed (:73-74).

   The doc comment on dedupeChecks (pullRequestChecks.ts:12-14) mentions the case where "a second run of it is already live", but the newest-at rule only fits a re-run that replaces an earlier one.

   Other things that read the same deduped list:

   • The reactor's inFlight flag (apps/server/src/orchestration-v2/PullRequestWatchReactor.ts:543) also misses the live run.
   • The listing row's one-word state (rollupChecksState, gitHubPullRequestJson.ts:1634-1645, used at :1746) and the detail checks list (:1763) would show "passing" with a single row during the same window.

   A mirror case (from reading the code, not observed)

   Say one event's run fails, and the other event's run starts after that failure. The pending run now has the newer at, so it hides the failure. Because the list of failed checks is rebuilt on every pass (pullRequestWatch.ts:62), the earlier failure is forgotten. When the pending run succeeds, the watch could report "passed" even though the other run failed. A gate that only holds back on pending runs wouldn't catch this.

   Other providers

   • Forgejo shares dedupeChecks (forgejoPullRequestJson.ts:211-231), but it dedupes commit statuses by context and updated_at. One context is one status that keeps being updated, so keeping the newest is right there. I believe Forgejo/Gitea Actions put the event in the context name (… (push) / … (pull_request)), which would keep the two runs apart, but I didn't verify that.
   • Bitbucket shares it too (bitbucketPullRequestJson.ts:566-581). It keys by status key with no dates, so the last copy wins. That's a different shape and probably not affected.
   • GitLab doesn't use it. It reports head_pipeline as a single "Pipeline" check (gitLabMergeRequestJson.ts:326-339).
   • Azure DevOps doesn't use it either. It currently returns checks: [] (AzureDevOpsPullRequestProvider.ts:380), so it never wakes on "passed" at all.

   As far as I can tell, this is GitHub-only.

   Fix options

   1. Hold "passed" while any raw rollup run of a gate check is pending (your suggestion). It's small and doesn't change the display. Edge cases:
      • Only pending should come from the raw rollup. An older completed attempt, or a run cancelled by concurrency: cancel-in-progress, would otherwise block "passed" forever if treated as a failure.
      • With required checks, it should be scoped to the gate's names.
      • It doesn't fix the mirror case above, the "passing" listing row, or inFlight.
      • A run stuck in queued/waiting would hold "passed" indefinitely. That's arguably correct, and it's already the behavior when nothing is required.
   2. Tell runs apart in the dedupe. GitHub's GraphQL WorkflowRun exposes event, databaseId and runAttempt, so the selection at gitHubPullRequestJson.ts:819 could ask for them.
      • Keying on workflow + event + name would show Checks / check (push) and Checks / check (pull_request) as two rows. That fixes the gate, the mirror case and the display in one place. It still folds together two runs of the same event on one SHA (for example pull_request opened and then labeled), which keeps the ordering problem for that case.
      • Keying on run id separates those too. But a concurrency-cancelled run would then show up as its own cancelled row and count as a failure (pullRequestWatch.ts:33-34) unless it's filtered out.
   3. As you noted, ordering by startedAt alone isn't enough.

   One thing to double-check whichever way this goes: the test fixture at gitHubPullRequestJson.test.ts:400-436 describes a re-run as appearing in the rollup twice, the finished run plus the live one. That doesn't match your observation that the rollup only listed attempt 2. Option 1 is fine either way, because an older attempt is complete, not pending. For option 2 it decides whether attempts of one run still need folding.

   Related, not duplicates

   • [Bug]: watch_pull_request misses "checks passed" after a push on Bitbucket because the head SHA is never reported #16301: Bitbucket head SHA is never reported.
   • [Bug]: watch_pull_request never reports "checks passed" on Forgejo when a job is skipped #16575 / fix(server): Forgejo skipped jobs no longer keep a pull request watch pending #16668: Forgejo skipped jobs keep the watch pending.
   • fix(server): PR watch reports a required check that first appears already passed #15804 (merged): a required check that first appears already passed.
   • [Bug]: watch_pull_request wakes only on failures and all-required-passed, not on other check state changes #16924: wake on every check state change.
   • Discussion watch_pull_request: say that an active watch follows new pushes #16969: agents are told to rely on the watch instead of their own CI loops, which makes a premature "passed" matter more.
   • fix(pull-request): prioritize failed checks #12274 is open and edits dedupeChecks ordering (failed checks first) but doesn't touch this.

   I didn't find a duplicate or an existing fix. Your gh run list --commit <sha> workaround looks right in the meantime.
2. juliusmarminge

   added

   bugSomething is broken or behaving incorrectly.Something is broken or behaving incorrectly.

   via-triageFiled through npx t3 triageFiled through npx t3 triage

   on Oct 8, 2026
3. Yash-Singh1

   closed this as completedin #17344on Oct 10, 2026