mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-08-22 03:18:54 +08:00
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
This commit is contained in:
@@ -424,7 +424,12 @@ export async function fetchGitHubData({
|
|||||||
if (prResult.repository.pullRequest) {
|
if (prResult.repository.pullRequest) {
|
||||||
const pullRequest = prResult.repository.pullRequest;
|
const pullRequest = prResult.repository.pullRequest;
|
||||||
contextData = 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(
|
comments = filterCommentsByActor(
|
||||||
filterCommentsToTriggerTime(
|
filterCommentsToTriggerTime(
|
||||||
pullRequest.comments?.nodes || [],
|
pullRequest.comments?.nodes || [],
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ PR Labels: ${formatLabels(prData.labels.nodes)}
|
|||||||
PR Additions: ${prData.additions}
|
PR Additions: ${prData.additions}
|
||||||
PR Deletions: ${prData.deletions}
|
PR Deletions: ${prData.deletions}
|
||||||
Total Commits: ${prData.commits.totalCount}
|
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 {
|
} else {
|
||||||
const issueData = contextData as GitHubIssue;
|
const issueData = contextData as GitHubIssue;
|
||||||
const sanitizedTitle = sanitizeContent(issueData.title);
|
const sanitizedTitle = sanitizeContent(issueData.title);
|
||||||
|
|||||||
+5
-1
@@ -85,9 +85,13 @@ export type GitHubPullRequest = {
|
|||||||
commit: GitHubCommit;
|
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: {
|
files: {
|
||||||
nodes: GitHubFile[];
|
nodes: GitHubFile[];
|
||||||
};
|
} | null;
|
||||||
comments: {
|
comments: {
|
||||||
nodes: GitHubComment[];
|
nodes: GitHubComment[];
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -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
|
// Webhook says no body at trigger time — attacker-added GraphQL body must not be used
|
||||||
expect(result.contextData.body).toBe("");
|
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", () => {
|
describe("filterCommentsByActor", () => {
|
||||||
|
|||||||
@@ -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", () => {
|
test("formats Issue context correctly", () => {
|
||||||
const issueData: GitHubIssue = {
|
const issueData: GitHubIssue = {
|
||||||
title: "Test Issue",
|
title: "Test Issue",
|
||||||
|
|||||||
Reference in New Issue
Block a user