Skip to content

fix(github): PR watches wait for required checks that have not started yet - #28200

Open
juliusmarminge wants to merge 5 commits into
mainfrom
fix/issue-28183
Open

juliusmarminge wants to merge 5 commits into
mainfrom
fix/issue-28183

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

A PR watch on GitHub said "All N required checks passed" while a required check had not been created on the head yet. T3 learned which GitHub checks are required only from isRequired on the check contexts already on the commit. An aggregate job with needs:, which GitHub creates only when it starts, was missing from that list. An agent that trusted the wake could report the PR done, or merge it, before that check ran. Our own repo hits this: the ruleset on main requires Check, which needs: jobs that are not required, so the other 7 required checks can pass before Check exists.

Change

  • PullRequestCore now also reads the base branch's required checks, on github.com only, behind the same gate as isRequired. Classic branch protection comes from baseRef.refUpdateRule.requiredStatusCheckContexts, rulesets from baseRef.rules. Both can be read with READ permission.
  • On an open PR, a required name that no check on the head carries is added as a pending, required check. This happens in one helper used by both the first-page read and the full walk past 100 checks. Rules are read as they are now, so closed and merged PRs are left alone.
  • Required names are matched against the raw check names, before same-named jobs from different workflows get their <Workflow> / prefix. A same-named check that GitHub marks as not required does not count as the required one having started.
  • A head with no checks at all stays empty, so the watch's "no checks have reported" wake still fires.
  • If GitHub refuses the new fields with FORBIDDEN, the read retries once without them. This follows the existing fallback for refused requested reviewers. Each field has its own include variable, so the retry keeps isRequired.

The watch logic in pullRequestWatch.ts is unchanged; it now gets the right checks. Clients show the added check through the existing pending-check rendering. No contract changes.

Scope and approval

Fixes #28183

Verification

  • Red tests in packages/source-control-github/src/server/gitHubPullRequestJson.test.ts:
    • waits for a check branch protection requires that has not started yet and waits for a check a ruleset requires that has not started yet fail on main: the result has only lint, with no pending Done.
    • counts a required check as started when workflows share its name, waits for a required check that only a check GitHub calls not required shares a name with, and adds no required check to a head no check has reported on each failed before their fix.
  • GitHubPullRequestApi.test.ts:
    • reads the detail without base branch rules refused covers rules refused alone and together with a reviewer refusal. In both cases the retry still asks for isRequired.
    • The >100-check walk test now asserts that the added required check survives.
  • vp test run on the three GitHub test files and apps/server/src/orchestration-v2/pullRequestWatch.test.ts: 217 passed. vp run check:changed: all passed (format, lint, knip, graphql codegen check, typecheck of 11 packages, tests).
  • Runtime transcript: a live PullRequestCore response for fix(web): terminal URLs like redis:// no longer become broken file links #28189, from each side's own generated query, decoded by each side's toPullRequestCore and passed to evaluatePullRequestWatch. To show the window, the required Check context was removed from the head's checks:
=== main 15fe283272 ('Check' removed from the head rollup)
required checks seen: Test Server 1=success, Test Server 2=success, Test Server 3=success, Test=success, Rust=skipped, Mobile Native Static Analysis=skipped, Release Smoke=skipped
checksState: passing
watch changes: [{"kind":"checks-passed","count":7,"required":true}]
- All 7 required checks passed on 67ff6ae.
=== fix 76a8ba0a44 ('Check' removed from the head rollup)
required checks seen: Test Server 1=success, ..., Release Smoke=skipped, Check=pending
checksState: pending
watch changes: []
=== fix 76a8ba0a44, Check present
required checks seen: Test Server 1=success, ..., Release Smoke=skipped, Check=success
checksState: passing
watch changes: [{"kind":"checks-passed","count":8,"required":true}]
- All 8 required checks passed on 67ff6ae.

Not checked:

  • No before/after captures of the PR detail panel, checks popover, thread PR row or mobile check rows with the added pending row. No client code changed, but the row and the pending state are visible there. On web, the primary action can be "enable auto-merge" instead of "merge" while the check is missing, which matches GitHub blocking the merge.
  • The FORBIDDEN fallback is covered by mocked responses only. No GitHub App user token or fine-grained PAT was tried against the new fields; an OAuth token with READ access reads both on microsoft/vscode, vercel/next.js and facebook/react.
  • Rulesets are read up to 100 rules per branch. The most seen live was 17, on vercel/next.js.

Triaged and fixed by Claude Opus 5.5 (claude-opus-5-5) in Claude Code inside T3 Code, with reviews by GPT-6.1 Sol in Codex.

🤖 Generated with Claude Code

juliusmarminge and others added 5 commits October 31, 2026 16:02
These tests fail on main: a required status check from branch protection
or a ruleset that has not started on the head commit is missing from the
PR's checks, so a PR watch reports all required checks passed early.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A PR watch could report every required check passed while a check the
base branch requires was not on the head commit yet: an aggregate job
with `needs:` only appears once it starts, and required checks were
only known from `isRequired` on contexts already present.

The detail read now also asks the base ref, on github.com only like
`isRequired`, for branch protection's required contexts and the
required status checks of active rulesets. Any required name no check
on the head carries is appended as a pending required check, and the
rollup reads pending unless something already failed. The >100-context
path reuses the same helper, so it keeps those checks too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…se rules

A required check run by same-named jobs in several workflows was renamed
"<Workflow> / <name>" before the presence check, so it got a phantom pending
placeholder and the watch never passed. Presence now comes from the raw
context names.

Credentials refused baseRef.refUpdateRule or baseRef.rules would fail the
whole detail read. Those selections get their own include variable, and a
FORBIDDEN refusal on them (alone or with the requested-reviewer refusal) is
retried once without the refused selections.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…still fires

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…quired check

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 31, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 76a8ba0

Macroscope's review found this PR approvable — PR watches now account for required checks that have not started, preventing premature completion. The change is limited to read-only status reporting and retains permission fallbacks and existing failure handling.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.6 KiB 21.6 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 15fe283 · PR result: 76a8ba0 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed labels Oct 31, 2026
@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 15fe283 and 76a8ba0.

Files selected for processing: 8
  • packages/source-control-github/schema/github.graphql.ts
  • packages/source-control-github/src/server/GitHubPullRequestApi.test.ts
  • packages/source-control-github/src/server/GitHubPullRequestApi.ts
  • packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts
  • packages/source-control-github/src/server/gitHubPullRequestJson.test.ts
  • packages/source-control-github/src/server/gitHubPullRequestJson.ts
  • packages/source-control-github/src/server/pullRequestReads.graphql
  • packages/source-control-github/src/server/pullRequestReads.graphql.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Walkthrough

The pull-request detail query now reads required status checks from branch protection and repository rulesets. The conversion adds configured checks that have not started as pending required checks. The query also retries when GitHub refuses optional reviewer or required-rule fields.

Changes: GitHub required checks

Layer / File(s) Summary
Required-rule query and data shapes
packages/source-control-github/schema/github.graphql.ts, packages/source-control-github/src/server/pullRequestReads.graphql, packages/source-control-github/src/server/pullRequestReads.graphql.ts
PullRequestCore conditionally reads required status-check contexts from base-ref update rules and repository rulesets. Its schemas include the corresponding rule data.
Permission-aware query fallback
packages/source-control-github/src/server/GitHubPullRequestApi.ts, packages/source-control-github/src/server/GitHubPullRequestApi.test.ts
The detail read retries with refused optional fields disabled. Tests cover refusal of required rules, requested reviewers, and both fields together.
Required-check conversion
packages/source-control-github/src/server/gitHubPullRequestJson.ts, packages/source-control-github/src/server/gitHubPullRequestJson.test.ts, packages/source-control-github/src/server/GitHubPullRequestApi.ts, packages/source-control-github/src/server/GitHubPullRequestApi.test.ts, packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts
The conversion collects unique required check names and adds unmatched names as pending required checks when check contexts exist. Tests cover matching contexts, missing checks, empty rollups, and query pagination.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant GitHubPullRequestApi
  participant GitHubGraphQL
  participant toPullRequestCore
  participant pullRequestChecksFromContexts
  GitHubPullRequestApi->>GitHubGraphQL: Request PullRequestCore with requiredRules
  GitHubGraphQL-->>GitHubPullRequestApi: Return base-ref rules and check contexts
  GitHubPullRequestApi->>toPullRequestCore: Pass pull-request response
  toPullRequestCore->>pullRequestChecksFromContexts: Pass contexts and requiredCheckNames
Loading

Suggested reviewers: scottn-pv, bil0000

Merge Risk: Low · up to 76a8b

The change is mergeable with awareness that PR watches may still report required checks as passed too early on branches with more than 100 applicable rules.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: GitHub PR watches now wait for required checks that have not started.
Description check Passed The description includes all required sections. It explains the problem, implementation, scope and linked issue, focused verification, observed results, limitations, and agent details.
Linked Issues check Passed Issue #28183 requires PR watches to remain pending when a required GitHub check has not started. The change reads required check names from baseRef.refUpdateRule.requiredStatusCheckContexts and rule…
Out of Scope Changes check Passed The changes stay within issue #28183. Query schema updates support reading branch protection and ruleset requirements. Decoder changes create the required pending checks. API changes control the githu…
Full details: Linked Issues check

Explanation

Issue #28183 requires PR watches to remain pending when a required GitHub check has not started. The change reads required check names from baseRef.refUpdateRule.requiredStatusCheckContexts and ruleset parameters on github.com. pullRequestChecksFromContexts adds pending required checks for missing names, matches raw names, ignores isRequired: false names, and preserves the empty-rollup behavior. The API applies the same helper after pagination and retains isRequired during the FORBIDDEN fallback. The added tests cover branch protection, rulesets, shared names, empty rollups, pagination, and fallback behavior.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #28183. Query schema updates support reading branch protection and ruleset requirements. Decoder changes create the required pending checks. API changes control the github.com gate and optional-field fallback. Tests validate the implementation. The generated Typename.RuleParameters type supports the added GraphQL union selection. No unrelated product behavior or client contract change is shown.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR watch says "All required checks passed" while a required GitHub check hasn't started yet

1 participant