github fakes contract tests

[jumboduck]

Extends the fake/contract test pattern (introduced for AWS Lambda in #763) to the GitHub API integration.

Decisions made

Abstraction level — We considered abstracting at the raw SDK level (as Lambda does) but the GitHub integration uses two different clients: a REST client (google/go-github) and a GraphQL
client (shurcooL/graphql). The GraphQL client's reflection-based Query() API makes SDK-level faking awkward. Instead we abstract at the operation level, using the existing
types.PRRetriever interface which already covers both PREvidenceForCommitV1 and PREvidenceForCommitV2.

V1/V2 behavioural difference — The two API versions have genuinely different contracts for unknown commit SHAs: V2 (GraphQL) returns an empty slice with no error; V1 (REST) returns a 422
error. Rather than paper over this, the contract tests and the fake both document and enforce this split. FakeGitHubClient.PREvidenceForCommitV1 returns an error for unseeded commits;
PREvidenceForCommitV2 returns an empty (non-nil) slice.

nil vs empty slice — PREvidenceForCommitV2 on the fake returns []*types.PREvidence{} (not nil) for unknown commits, matching the real implementation's initialised slice. This matters
because nil serialises to JSON null, which the Kosli server rejects.

ProviderAndLabel() on PRRetriever — The command layer used reflect.TypeOf to determine the git provider name and PR label. To allow fake injection without breaking that logic, we added
ProviderAndLabel() (string, string) to types.PRRetriever and removed the reflection entirely. All four VCS implementations (GitHub, GitLab, Azure, Bitbucket) implement the method.

Command-layer injection — A NewGithubRetrieverFunc package-level factory (mirroring NewLambdaClientFunc) allows tests to swap in FakeGitHubClient. The assertPRGithub and attestPRGithub
command test suites no longer require KOSLI_GITHUB_TOKEN.

CI — The daily aws-contract-tests job is replaced by a Contract Tests job running make test_contract, which covers both AWS and GitHub contract tests. KOSLI_GITHUB_TOKEN is already
available in CI secrets.

Test plan

• make test_integration passes locally
• TestGitHubContract_Fake passes without any credentials
• TestGitHubContract_RealGitHub passes with KOSLI_GITHUB_TOKEN set
• AssertPRGithubCommandTestSuite and AttestGithubPRCommandTestSuite pass without KOSLI_GITHUB_TOKEN
• make test_smoke_github runs the real GitHub contract tests
• make test_contract runs both AWS and GitHub contract tests

[claude]

Claude finished @jumboduck's task in 2m 57s —— View job

---

PR Review: GitHub Fakes Contract Tests

• Phase 1: Static analysis (go build, go vet, go mod tidy)
• Phase 2: Read and review all changed files
• Phase 3: Post inline comments and summary

Static Analysis

All three checks pass cleanly — go build ./..., go vet ./..., and go mod tidy with no issues.

Summary

This is a well-structured PR that cleanly extends the fake/contract test pattern from Lambda to GitHub. The design decisions are sound and well-documented in the PR description.

What's good:

• Clean abstraction level — abstracting at the PRRetriever operation level (rather than raw SDK) is the right call given the REST/GraphQL dual-client situation.
• V1/V2 contract split — documenting and enforcing the behavioural difference (V1 errors on unknown commits, V2 returns empty) is honest and prevents false confidence.
• Reflection removal — replacing reflect.TypeOf-based provider detection with ProviderAndLabel() on the interface is a real improvement. Cleaner, extensible, and fake-friendly.
• Consistent patterns — NewGithubRetrieverFunc / ResetGithubRetrieverFunc mirrors the Lambda factory pattern exactly. Good for maintainability.
• Contract test structure — runGitHubContractTests as a shared suite that both fake and real tests call is clean. Error-injection tests are correctly separated as fake-specific.
• nil vs empty slice — the []*types.PREvidence{} return for V2 unknown commits (instead of nil) is a good catch that prevents JSON serialization issues.
• CI integration — consolidating into a single make test_contract target that covers both AWS and GitHub is sensible.

Minor suggestions (non-blocking):

1. Missing MergeCommit in test fixtures — the command test suites (assertPRGithub_test.go, attestPRGithub_test.go) seed PREvidence without MergeCommit. This isn't a bug today, but the attestPR path serializes MergeCommit into the API payload. Setting it to the commit SHA would be more consistent with real behaviour. (See inline suggestions.)

2. CI silent skip risk — if KOSLI_GITHUB_TOKEN is missing from CI secrets, TestGitHubContract_RealGitHub silently skips rather than failing. The contract tests job would pass without actually testing the real API. Consider adding a step that asserts the env var is set, or at least document this expectation.

No bugs or security issues found. The PR is ready to merge with the minor suggestions above at the author's discretion.