From 0a5f191964a2bd37d3e606a981d7ffe1e4b7f0d5 Mon Sep 17 00:00:00 2001 From: leepokai <109857817+leepokai@users.noreply.github.com> Date: Fri, 7 Aug 2026 22:55:10 +0800 Subject: [PATCH] fix: handle null files field from GraphQL on very large PRs (#1593) GitHub's GraphQL API returns files: null (with no errors entry, and changedFiles misreported as 0) when a PR's diff is too large to compute. The unguarded pullRequest.files.nodes dereference in the fetcher crashed the action with 'TypeError: null is not an object', and the formatter had the same latent crash on prData.files.nodes.length. Widen the GitHubPullRequest type to files | null so the compiler enforces guards, degrade gracefully in the fetcher with a warning, and render the file count as unavailable (not '0 files') in the formatter. Fixes #1587 --- src/github/data/fetcher.ts | 7 +++++- src/github/data/formatter.ts | 2 +- src/github/types.ts | 6 ++++- test/data-fetcher.test.ts | 43 ++++++++++++++++++++++++++++++++++++ test/data-formatter.test.ts | 37 +++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/github/data/fetcher.ts b/src/github/data/fetcher.ts index 8aad4ec4..fb492ea4 100644 --- a/src/github/data/fetcher.ts +++ b/src/github/data/fetcher.ts @@ -424,7 +424,12 @@ export async function fetchGitHubData({ if (prResult.repository.pullRequest) { const pullRequest = prResult.repository.pullRequest; contextData = pullRequest; - changedFiles = pullRequest.files.nodes || []; + if (pullRequest.files === null) { + console.warn( + `GitHub did not return the file list for PR #${prNumber} (diff likely too large); proceeding without file-level context`, + ); + } + changedFiles = pullRequest.files?.nodes ?? []; comments = filterCommentsByActor( filterCommentsToTriggerTime( pullRequest.comments?.nodes || [], diff --git a/src/github/data/formatter.ts b/src/github/data/formatter.ts index 95d56037..82dbe568 100644 --- a/src/github/data/formatter.ts +++ b/src/github/data/formatter.ts @@ -28,7 +28,7 @@ PR Labels: ${formatLabels(prData.labels.nodes)} PR Additions: ${prData.additions} PR Deletions: ${prData.deletions} Total Commits: ${prData.commits.totalCount} -Changed Files: ${prData.files.nodes.length} files`; +Changed Files: ${prData.files ? `${prData.files.nodes.length} files` : "unknown (file list unavailable)"}`; } else { const issueData = contextData as GitHubIssue; const sanitizedTitle = sanitizeContent(issueData.title); diff --git a/src/github/types.ts b/src/github/types.ts index feeb7d73..d5053f42 100644 --- a/src/github/types.ts +++ b/src/github/types.ts @@ -85,9 +85,13 @@ export type GitHubPullRequest = { commit: GitHubCommit; }>; }; + // GitHub's GraphQL `files` field resolves to null when the PR's diff is too + // large for GitHub to compute (very large PRs). `changedFiles` is also + // misreported as 0 in that case, so the null must be guarded and treated as + // "file list unavailable" rather than "no files changed". files: { nodes: GitHubFile[]; - }; + } | null; comments: { nodes: GitHubComment[]; }; diff --git a/test/data-fetcher.test.ts b/test/data-fetcher.test.ts index cc2af464..945eb55e 100644 --- a/test/data-fetcher.test.ts +++ b/test/data-fetcher.test.ts @@ -1775,6 +1775,49 @@ describe("fetchGitHubData integration with time filtering", () => { // Webhook says no body at trigger time — attacker-added GraphQL body must not be used expect(result.contextData.body).toBe(""); }); + + it("should not crash when GraphQL returns null files for a very large PR", async () => { + // GitHub declines to compute the diff for very large PRs: `files` comes + // back as null (with no errors entry) and `changedFiles` is misreported + // as 0. The fetch must degrade gracefully instead of throwing. + const mockOctokits = { + graphql: jest.fn().mockResolvedValue({ + repository: { + pullRequest: { + number: 7912, + title: "Very large PR", + body: "PR body", + author: { login: "author" }, + createdAt: "2024-01-15T10:00:00Z", + state: "OPEN", + labels: { nodes: [] }, + comments: { nodes: [] }, + files: null, + reviews: { nodes: [] }, + }, + }, + user: { login: "trigger-user" }, + }), + rest: { + pulls: { + listFiles: jest.fn().mockResolvedValue({ data: [] }), + }, + }, + }; + + const result = await fetchGitHubData({ + octokits: mockOctokits as any, + repository: "test-owner/test-repo", + prNumber: "7912", + isPR: true, + triggerUsername: "trigger-user", + triggerTime: "2024-01-15T12:00:00Z", + }); + + // No file list is available, so the PR is processed without file-level context. + expect(result.changedFiles).toEqual([]); + expect(result.changedFilesWithSHA).toEqual([]); + }); }); describe("filterCommentsByActor", () => { diff --git a/test/data-formatter.test.ts b/test/data-formatter.test.ts index b3e43629..423bbcda 100644 --- a/test/data-formatter.test.ts +++ b/test/data-formatter.test.ts @@ -108,6 +108,43 @@ Changed Files: 2 files`, ); }); + test("renders an unknown file count when GraphQL returns null files (very large PR)", () => { + // GitHub declines to compute the diff for very large PRs and returns + // `files: null`. `changedFiles` is misreported as 0 in that case, so the + // count must render as unavailable rather than "0 files". + const prData: GitHubPullRequest = { + title: "Very large PR", + body: "PR body", + author: { login: "test-user" }, + baseRefName: "main", + headRefName: "feature/test", + headRefOid: "abc123", + isCrossRepository: false, + headRepository: { owner: { login: "testowner" }, name: "testrepo" }, + createdAt: "2023-01-01T00:00:00Z", + additions: 50, + deletions: 30, + state: "OPEN", + labels: { + nodes: [], + }, + commits: { + totalCount: 3, + nodes: [], + }, + files: null, + comments: { + nodes: [], + }, + reviews: { + nodes: [], + }, + }; + + const result = formatContext(prData, true); + expect(result).toContain("Changed Files: unknown (file list unavailable)"); + }); + test("formats Issue context correctly", () => { const issueData: GitHubIssue = { title: "Test Issue",