[dangrondahl]

Fixes #1274.

Since v2.48.0 the commit queries in PREvidenceByPRNumber and PREvidenceForCommitV2 ask for signature { signer { login } }. GitHub types signer as User, and for some commits created by a GitHub App (e.g. Renovate) it answers with signer: null plus an entry in errors. shurcooL/graphql turns any errors entry into a failed query, so the whole PR attestation fails.

signer now leaves the commit queries and signerLogins looks it up on its own with nodes(ids:), only for signed commits, 100 per request. The client decodes the data before returning the error, so an error that comes with data is accepted: that commit keeps verified, signature_state and signed_by_platform, with an empty signer_username, and the error is logged under --debug. A reply with no data is retried like the other queries in this flow, and fails the attestation once retries run out.

Cost: one extra GraphQL request per PR with signed commits.

Confirmed against a real failing response. It carried "type": "NOT_FOUND", "message": "Not Found" and a path ending in signature.signer, with the rest of the data present, which matches the test fixture. The commit had wasSignedByGitHub: true, so GitHub's own key signed it and the signer still couldn't be resolved. TestPREvidenceForCommitV2_RecordsACommitSignedByAnApp now uses that shape.

Checklist

• Docs are autogenerated from CLI help — make sure the help text reflects your changes (no help text change)
• Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
• Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

[claude]

Claude finished @dangrondahl's task in 1m 29s —— View job

---

PR review: fix: record PR evidence when a commit is signed by a GitHub App

• Static checks: go build ./... ✅, go vet ✅, go mod tidy leaves go.mod/go.sum unchanged ✅
• Read the diff (github.go, pagination.go, tests)
• Checked it against the two earlier review threads
• Unit tests: not run. The sandbox needs approval for go test, so I only read the tests.

Earlier review threads: both fixed

1. Transient failures. signerLogins now goes through the new c.retry helper. Inside an attempt, an error that comes with Nodes is accepted. An error with no data (502, data: null) goes through the same 0/10/20/30s backoff as everything else, as bug: PR attestation silently truncates the commit list on GitHub, GitLab and Azure #1082 requires. q = signerQuery{} resets the result on each attempt, so a failed attempt can't leave partial data behind. Good catch.
2. Errors that come with data are dropped silently. They now go to stderr under --debug with the existing [debug-github] prefix. The godoc says why the error can't be narrowed: the client drops path. That's what I asked for.

Findings

I found no blocking issues. Some small notes, none of which need to change before merge:

• Redundant debug output (nit). With --debug on, debugTransport already logs the whole response body, errors included. So the new line in signerLogins (github.go:329) repeats it. It's still worth keeping as a short marker that the error was ignored, so you don't need to change anything.
• retry has no doc comment (pagination.go:72). It's unexported, so CLAUDE.md doesn't require one. Since the doc text for queryWithRetry above it is now really about the retry ladder, you could move that text onto retry and give queryWithRetry a one-line comment.
• Any partial-data error is accepted. For example, a node ID that doesn't resolve comes back as a zero-value element, so Nodes still isn't empty, and the commit just gets an empty signer_username. This is the trade-off we discussed and it's now visible under --debug. I'm noting it here only so it's recorded.
• Deterministic errors with no data are still retried (e.g. a schema or validation error), adding up to 60s before the attempt fails. All the other queries in this flow behave the same way, so this is consistent.

What's done well

• Moving signer out of the commit page queries and adding TestCommitQueriesDoNotAskForTheSigner guards against the regression coming back.
• The fixture follows the real failing response (NOT_FOUND, path ending in signature.signer, data present). There is also a case where wasSignedByGitHub: true must keep signed_by_platform.
• Edge cases are covered: batches of 100 (with an exact 100/1 split), a 500 then success, no data until retries run out (5 requests), a field error that isn't retried, and the debug log.
• PRs with no signed commits make no extra request (slices.Chunk over an empty slice), so the extra cost is limited to the PRs that need it.
• Results are keyed by Oid, not by position, so a reply in a different order can't attach a signer to the wrong commit.

Summary

The fix is well scoped and well tested, and both earlier review points are handled cleanly. Looks good to merge once CI is green.