From eaebf9b8b6de243d793dedda726edce6b6a10898 Mon Sep 17 00:00:00 2001 From: vidhu bala Date: Wed, 15 Jul 2026 11:12:34 +0100 Subject: [PATCH 1/5] fix(github): fall back to "unknown" when PR creator account is deleted When the GitHub account of a PR creator is deleted, the GraphQL API returns null for the Author node, resulting in an empty login string. PREvidence.Author has omitempty, so "" is omitted from the JSON payload, causing the server's FoundPullRequestV2.author required-field validation to fail with "input should be a string extra inputs are not permitted". Guard added in buildPREvidence: if author == "", set it to "unknown". Both call sites (PREvidenceForCommitV2 and PREvidenceByPRNumber) pass through this function, so both are covered. Co-Authored-By: Claude Sonnet 4.6 --- internal/github/build_pr_evidence_test.go | 22 ++++++++++++++++++++++ internal/github/github.go | 7 +++++++ 2 files changed, 29 insertions(+) diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go index dcb295a69..b2d2b7ec9 100644 --- a/internal/github/build_pr_evidence_test.go +++ b/internal/github/build_pr_evidence_test.go @@ -152,6 +152,28 @@ func TestBuildPREvidence_RecordsCommitSignature(t *testing.T) { require.Equal(t, "VALID", *c.SignatureState) } +// TestBuildPREvidence_EmptyAuthorFallsBackToUnknown verifies that when the PR +// creator's GitHub account has been deleted (Author.Login = ""), the author +// field falls back to "unknown" so the server's required FoundPullRequestV2.author +// field is satisfied and the attestation does not fail validation. +func TestBuildPREvidence_EmptyAuthorFallsBackToUnknown(t *testing.T) { + evidence, err := buildPREvidence( + "https://github.com/kosli-dev/cli/pull/671", + "0e723254516c841126e81f76100be57258ff1386", + "MERGED", + "", // empty author — PR creator account deleted + "2026-03-01T09:00:00Z", + "2026-03-01T12:00:00Z", + "Fix something", + "fix-branch", + "main", + nil, nil, + ) + require.NoError(t, err) + require.Equal(t, "unknown", evidence.Author, + "empty PR author login must fall back to 'unknown' to satisfy FoundPullRequestV2.author required field") +} + // TestBuildPREvidence_UnsignedCommitHasNoSignatureFields verifies an unsigned // commit (no signature node) leaves verified/signature_state nil, so "unsigned" // stays distinct from "present-but-invalid" (verified=false). diff --git a/internal/github/github.go b/internal/github/github.go index 858b3eab7..c61375c2c 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -309,6 +309,13 @@ func buildPREvidence( mergedAt = mergedAtTime.Unix() } + // author is empty when the PR creator's GitHub account has been deleted. + // FoundPullRequestV2.author is a required field on the server; since + // PREvidence.Author has omitempty, sending "" would omit it and fail validation. + if author == "" { + author = "unknown" + } + evidence := &types.PREvidence{ URL: url, MergeCommit: mergeCommit, From 03f7af57a2e98211d945c00190aac57e82e89f86 Mon Sep 17 00:00:00 2001 From: vidhu bala Date: Wed, 15 Jul 2026 18:56:38 +0100 Subject: [PATCH 2/5] fix(github): send empty PR author as "" instead of falling back to "unknown" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the omitempty tag from PREvidence.Author so an empty string is serialised in the JSON payload rather than omitted. Drop the "unknown" fallback in buildPREvidence — the server's FoundPullRequestV2.author accepts an empty string directly. Co-Authored-By: Claude Sonnet 4.6 --- internal/github/build_pr_evidence_test.go | 14 +++++++------- internal/github/github.go | 7 ------- internal/types/types.go | 2 +- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go index b2d2b7ec9..ad95c0d69 100644 --- a/internal/github/build_pr_evidence_test.go +++ b/internal/github/build_pr_evidence_test.go @@ -152,11 +152,11 @@ func TestBuildPREvidence_RecordsCommitSignature(t *testing.T) { require.Equal(t, "VALID", *c.SignatureState) } -// TestBuildPREvidence_EmptyAuthorFallsBackToUnknown verifies that when the PR -// creator's GitHub account has been deleted (Author.Login = ""), the author -// field falls back to "unknown" so the server's required FoundPullRequestV2.author -// field is satisfied and the attestation does not fail validation. -func TestBuildPREvidence_EmptyAuthorFallsBackToUnknown(t *testing.T) { +// TestBuildPREvidence_EmptyAuthorIsPreserved verifies that when the PR +// creator's GitHub account has been deleted (Author.Login = ""), the empty +// string is preserved and serialised as "" rather than omitted, allowing the +// server to accept it directly. +func TestBuildPREvidence_EmptyAuthorIsPreserved(t *testing.T) { evidence, err := buildPREvidence( "https://github.com/kosli-dev/cli/pull/671", "0e723254516c841126e81f76100be57258ff1386", @@ -170,8 +170,8 @@ func TestBuildPREvidence_EmptyAuthorFallsBackToUnknown(t *testing.T) { nil, nil, ) require.NoError(t, err) - require.Equal(t, "unknown", evidence.Author, - "empty PR author login must fall back to 'unknown' to satisfy FoundPullRequestV2.author required field") + require.Equal(t, "", evidence.Author, + "empty PR author login must be preserved so it is serialised as an empty string, not omitted") } // TestBuildPREvidence_UnsignedCommitHasNoSignatureFields verifies an unsigned diff --git a/internal/github/github.go b/internal/github/github.go index c61375c2c..858b3eab7 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -309,13 +309,6 @@ func buildPREvidence( mergedAt = mergedAtTime.Unix() } - // author is empty when the PR creator's GitHub account has been deleted. - // FoundPullRequestV2.author is a required field on the server; since - // PREvidence.Author has omitempty, sending "" would omit it and fail validation. - if author == "" { - author = "unknown" - } - evidence := &types.PREvidence{ URL: url, MergeCommit: mergeCommit, diff --git a/internal/types/types.go b/internal/types/types.go index 8e427a3ea..05a6b82ea 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -5,7 +5,7 @@ type PREvidence struct { URL string `json:"url"` State string `json:"state"` Approvers []any `json:"approvers"` - Author string `json:"author,omitempty"` + Author string `json:"author"` CreatedAt int64 `json:"created_at,omitempty"` MergedAt int64 `json:"merged_at,omitempty"` Title string `json:"title,omitempty"` From fab00a9d960551e10cbd86fb20f52fd3c9c9d469 Mon Sep 17 00:00:00 2001 From: vidhu bala Date: Thu, 16 Jul 2026 11:42:30 +0100 Subject: [PATCH 3/5] test(github): update fake PR fixtures to full V2 shape The minimal {URL, State} fixtures were being accepted by the server as FoundPullRequestV1. Removing omitempty from PREvidence.Author means author:"" is now always sent, which FoundPullRequestV1 rejects (extra="forbid"). Update both fixtures to include all required V2 fields, matching what buildPREvidence actually produces. Co-Authored-By: Claude Sonnet 4.6 --- cmd/kosli/assertPRGithub_test.go | 11 ++++++++++- cmd/kosli/attestPRGithub_test.go | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/cmd/kosli/assertPRGithub_test.go b/cmd/kosli/assertPRGithub_test.go index 2bd2a412c..1872243d8 100644 --- a/cmd/kosli/assertPRGithub_test.go +++ b/cmd/kosli/assertPRGithub_test.go @@ -23,7 +23,16 @@ func (suite *AssertPRGithubCommandTestSuite) SetupTest() { ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string, debug bool) types.PRRetriever { return &ghUtils.FakeGitHubClient{ PRsByCommit: map[string][]*types.PREvidence{ - suite.commitWithPR: {{URL: "https://github.com/kosli-dev/cli/pull/1", State: "MERGED"}}, + suite.commitWithPR: {{ + URL: "https://github.com/kosli-dev/cli/pull/1", + State: "MERGED", + Author: "test-user", + Title: "test PR", + CreatedAt: 1234567890, + HeadRef: "test-branch", + MergeCommit: suite.commitWithPR, + Commits: []types.Commit{}, + }}, }, } } diff --git a/cmd/kosli/attestPRGithub_test.go b/cmd/kosli/attestPRGithub_test.go index c888a35f1..7d10d6227 100644 --- a/cmd/kosli/attestPRGithub_test.go +++ b/cmd/kosli/attestPRGithub_test.go @@ -36,7 +36,16 @@ func (suite *AttestGithubPRCommandTestSuite) SetupTest() { ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string, debug bool) types.PRRetriever { return &ghUtils.FakeGitHubClient{ PRsByCommit: map[string][]*types.PREvidence{ - suite.commitWithPR: {{URL: "https://github.com/kosli-dev/cli/pull/1", State: "MERGED"}}, + suite.commitWithPR: {{ + URL: "https://github.com/kosli-dev/cli/pull/1", + State: "MERGED", + Author: "test-user", + Title: "test PR", + CreatedAt: 1234567890, + HeadRef: "test-branch", + MergeCommit: suite.commitWithPR, + Commits: []types.Commit{}, + }}, }, } } From ecf2172f23df7f8eac66d42ee6a71c116ecdb5af Mon Sep 17 00:00:00 2001 From: vidhu bala Date: Thu, 16 Jul 2026 13:38:33 +0100 Subject: [PATCH 4/5] test(github): add commit entry to fake PR fixtures Empty Commits slice is omitted by omitempty, causing FoundPullRequestV2 validation to fail with "commits: Field required". Add a minimal commit so the slice is non-empty and serialised correctly. Co-Authored-By: Claude Sonnet 4.6 --- cmd/kosli/assertPRGithub_test.go | 8 +++++++- cmd/kosli/attestPRGithub_test.go | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cmd/kosli/assertPRGithub_test.go b/cmd/kosli/assertPRGithub_test.go index 1872243d8..dd1b263bc 100644 --- a/cmd/kosli/assertPRGithub_test.go +++ b/cmd/kosli/assertPRGithub_test.go @@ -31,7 +31,13 @@ func (suite *AssertPRGithubCommandTestSuite) SetupTest() { CreatedAt: 1234567890, HeadRef: "test-branch", MergeCommit: suite.commitWithPR, - Commits: []types.Commit{}, + Commits: []types.Commit{{ + SHA: suite.commitWithPR, + Message: "test commit", + Author: "Test User ", + Timestamp: 1234567890, + Branch: "test-branch", + }}, }}, }, } diff --git a/cmd/kosli/attestPRGithub_test.go b/cmd/kosli/attestPRGithub_test.go index 7d10d6227..b345ccd42 100644 --- a/cmd/kosli/attestPRGithub_test.go +++ b/cmd/kosli/attestPRGithub_test.go @@ -44,7 +44,13 @@ func (suite *AttestGithubPRCommandTestSuite) SetupTest() { CreatedAt: 1234567890, HeadRef: "test-branch", MergeCommit: suite.commitWithPR, - Commits: []types.Commit{}, + Commits: []types.Commit{{ + SHA: suite.commitWithPR, + Message: "test commit", + Author: "Test User ", + Timestamp: 1234567890, + Branch: "test-branch", + }}, }}, }, } From 3b2f4da08f97efcc0c3680f8050857b15edf4550 Mon Sep 17 00:00:00 2001 From: vidhu bala Date: Thu, 16 Jul 2026 13:57:20 +0100 Subject: [PATCH 5/5] test(github): assert empty author is serialised in JSON, not omitted The struct-field assertion passes regardless of the omitempty tag. Adding a JSON marshal check pins the actual wire behaviour: if omitempty were re-added to PREvidence.Author the test would fail. Co-Authored-By: Claude Sonnet 4.6 --- internal/github/build_pr_evidence_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/github/build_pr_evidence_test.go b/internal/github/build_pr_evidence_test.go index ad95c0d69..c18935bae 100644 --- a/internal/github/build_pr_evidence_test.go +++ b/internal/github/build_pr_evidence_test.go @@ -1,6 +1,7 @@ package github import ( + "encoding/json" "testing" "time" @@ -172,6 +173,11 @@ func TestBuildPREvidence_EmptyAuthorIsPreserved(t *testing.T) { require.NoError(t, err) require.Equal(t, "", evidence.Author, "empty PR author login must be preserved so it is serialised as an empty string, not omitted") + + b, err := json.Marshal(evidence) + require.NoError(t, err) + require.Contains(t, string(b), `"author":""`, + "empty author must be serialised, not omitted") } // TestBuildPREvidence_UnsignedCommitHasNoSignatureFields verifies an unsigned