Fix: GitHub PR attestation misses PRs merged seconds before CI runs

[jumboduck]

Fixes kosli-dev/server#5391.

GitHub's GraphQL associatedPullRequests field has eventual consistency: it can return 0 results for a commit merged only seconds earlier. Both attest pullrequest github and assert pullrequest github called PREvidenceForCommitV2 directly with no fallback, so CI pipelines running immediately after a merge would get a false "found 0 pull request(s)".

Approach

Three slices:

Slice 1 — PREvidenceByPRNumber
New GraphQL query keyed by PR number rather than commit SHA. Querying by number bypasses the associatedPullRequests index so it is immediately consistent. Building block for the hybrid.

Slice 2 — PREvidenceForCommitHybrid
New method on the PRRetriever interface, wired into both call sites.

1. Tries PREvidenceForCommitV2 (original GraphQL by commit SHA) — happy path unchanged
2. If 0 results: calls PullRequestsForCommit (REST, immediately consistent) for PR numbers, then PREvidenceByPRNumber for each — preserves all rich V2 fields (commits, approvers, author, etc.)

We chose this over a plain V2→V1 fallback because V1 lacks the rich fields already in the attestation payload. GitLab, Azure, and Bitbucket implement PREvidenceForCommitHybrid as a pass-through to V2 since they have no GraphQL eventual consistency issue.

Slice 3 — Retry on GraphQL error
Wraps the client.Query call in PREvidenceByPRNumber with a retry loop (10s + 20s + 30s = 60s max). GithubConfig.Sleep is injectable so tests complete instantly.

Testing

All new behaviour is covered by contract tests following the pattern from #807. Retry behaviour is tested with an httptest.Server that fails N times before succeeding.

🤖 Generated with Claude Code

[claude]

Claude finished @jumboduck's task in 4m 22s —— View job

---

PR Review: Fix GitHub PR attestation misses PRs merged seconds before CI runs

• Run static analysis (go build, go vet, go mod tidy)
• Review changed files
• Post review findings

Static Analysis

All checks pass: go build ./..., go vet ./..., and go mod tidy — no issues.

Summary

This is a well-structured PR that solves a real and painful problem (GitHub GraphQL eventual consistency causing false "no PR found" in CI). The three-slice approach is clean and incrementally reviewable.

What's good:

• buildPREvidence extraction — Eliminates the duplication between PREvidenceByPRNumber and PREvidenceForCommitV2 that was flagged in earlier reviews. Single place to change if fields evolve.
• Shared graphqlCommitNode / graphqlReviewNode types — Keeps the two GraphQL queries structurally in sync.
• Contract test pattern — runGitHubContractTests and runPRByNumberContractTests verify that the fake and real implementations satisfy the same contract. The hybrid fallback path is tested independently with PRNumbersByCommit seeding.
• Injectable Sleep — Makes retry tests deterministic without real delays.
• Other providers as pass-throughs — Azure, Bitbucket, and GitLab just delegate to V2, which is the right call since they don't have this consistency issue.

Inline comments posted (all non-blocking):

1. pr == nil guard vs doc comment (github.go:297) — The nil, nil return is unreachable but slightly contradicts the "returns an error" doc. Minor readability nit.
2. Retry loop clarity (github.go:281-290) — Suggested a slightly more explicit pattern that separates initial attempt from retries. Current code is correct.
3. Partial results on multi-PR fallback (github.go:109-124) — Confirming fail-fast is the intended behavior when one PREvidenceByPRNumber call fails after retries.

No blocking issues found. The code compiles, vets clean, the approach is sound, and the testing is thorough. Previously flagged review items (stale doc comment, retry loop restructure) have been addressed in recent commits.

[pbeckham]

We limit to 100 commits here and don't get any more. If the PR has more than 100 commits we would miss this in the evidence