diff --git a/src/github/data/fetcher.ts b/src/github/data/fetcher.ts index 4d123884..cea2ea24 100644 --- a/src/github/data/fetcher.ts +++ b/src/github/data/fetcher.ts @@ -204,11 +204,9 @@ export function isBodySafeToUse( * @param excludeActors - Comma-separated actors to exclude * @returns Filtered array of comments */ -export function filterCommentsByActor( - comments: T[], - includeActors: string = "", - excludeActors: string = "", -): T[] { +export function filterCommentsByActor< + T extends { author: { login: string } | null }, +>(comments: T[], includeActors: string = "", excludeActors: string = ""): T[] { const includeParsed = parseActorFilter(includeActors); const excludeParsed = parseActorFilter(excludeActors); @@ -219,7 +217,9 @@ export function filterCommentsByActor( return comments.filter((comment) => shouldIncludeCommentByActor( - comment.author.login, + // author is null for comments from deleted ("ghost") accounts; treat them + // as the "ghost" login so filtering never dereferences null and crashes. + comment.author?.login ?? "ghost", includeParsed, excludeParsed, ), diff --git a/src/github/data/formatter.ts b/src/github/data/formatter.ts index 398cc742..95d56037 100644 --- a/src/github/data/formatter.ts +++ b/src/github/data/formatter.ts @@ -21,7 +21,7 @@ export function formatContext( const prData = contextData as GitHubPullRequest; const sanitizedTitle = sanitizeContent(prData.title); return `PR Title: ${sanitizedTitle} -PR Author: ${prData.author.login} +PR Author: ${prData.author?.login ?? "ghost"} PR Branch: ${prData.headRefName} -> ${prData.baseRefName} PR State: ${prData.state} PR Labels: ${formatLabels(prData.labels.nodes)} @@ -33,7 +33,7 @@ Changed Files: ${prData.files.nodes.length} files`; const issueData = contextData as GitHubIssue; const sanitizedTitle = sanitizeContent(issueData.title); return `Issue Title: ${sanitizedTitle} -Issue Author: ${issueData.author.login} +Issue Author: ${issueData.author?.login ?? "ghost"} Issue State: ${issueData.state} Issue Labels: ${formatLabels(issueData.labels.nodes)}`; } @@ -71,7 +71,7 @@ export function formatComments( body = sanitizeContent(body); - return `[${comment.author.login} at ${comment.createdAt}]: ${body}`; + return `[${comment.author?.login ?? "ghost"} at ${comment.createdAt}]: ${body}`; }) .join("\n\n"); } @@ -85,7 +85,7 @@ export function formatReviewComments( } const formattedReviews = reviewData.nodes.map((review) => { - let reviewOutput = `[Review by ${review.author.login} at ${review.submittedAt}]: ${review.state}`; + let reviewOutput = `[Review by ${review.author?.login ?? "ghost"} at ${review.submittedAt}]: ${review.state}`; if (review.body && review.body.trim()) { let body = review.body; diff --git a/src/github/types.ts b/src/github/types.ts index 6ed41e39..feeb7d73 100644 --- a/src/github/types.ts +++ b/src/github/types.ts @@ -1,4 +1,8 @@ // Types for GitHub GraphQL query responses + +// GitHub's GraphQL `author`/`actor` fields resolve to null when the underlying +// account has been deleted (the "ghost" user). Any field typed as +// `GitHubAuthor | null` can therefore be null at runtime and must be guarded. export type GitHubAuthor = { login: string; name?: string; @@ -8,7 +12,7 @@ export type GitHubComment = { id: string; databaseId: string; body: string; - author: GitHubAuthor; + author: GitHubAuthor | null; createdAt: string; updatedAt?: string; lastEditedAt?: string; @@ -39,7 +43,7 @@ export type GitHubFile = { export type GitHubReview = { id: string; databaseId: string; - author: GitHubAuthor; + author: GitHubAuthor | null; body: string; state: string; submittedAt: string; @@ -53,7 +57,7 @@ export type GitHubReview = { export type GitHubPullRequest = { title: string; body: string; - author: GitHubAuthor; + author: GitHubAuthor | null; baseRefName: string; headRefName: string; headRefOid: string; @@ -95,7 +99,7 @@ export type GitHubPullRequest = { export type GitHubIssue = { title: string; body: string; - author: GitHubAuthor; + author: GitHubAuthor | null; createdAt: string; updatedAt?: string; lastEditedAt?: string; diff --git a/test/data-fetcher.test.ts b/test/data-fetcher.test.ts index f92fc144..c054039b 100644 --- a/test/data-fetcher.test.ts +++ b/test/data-fetcher.test.ts @@ -1499,4 +1499,42 @@ describe("filterCommentsByActor", () => { const filtered = filterCommentsByActor(comments, "user1", ""); expect(filtered).toHaveLength(0); }); + + test("does not crash on comments from deleted (null-author) accounts", () => { + // GitHub's GraphQL returns author: null for comments whose account was + // deleted. With an exclude filter set (the exact `*[bot]` config we + // recommend), the null author must not throw when dereferenced. + const comments = [ + { author: { login: "user1" }, body: "comment1" }, + { author: null, body: "from a deleted account" }, + { author: { login: "bot[bot]" }, body: "comment3" }, + ]; + + const { filterCommentsByActor } = require("../src/github/data/fetcher"); + const filtered = filterCommentsByActor(comments, "", "*[bot]"); + // ghost comment is retained (it matches no exclude pattern); the bot is dropped. + expect(filtered).toHaveLength(2); + expect(filtered.map((c: any) => c.body)).toEqual([ + "comment1", + "from a deleted account", + ]); + }); + + test("treats null author as the 'ghost' login for include/exclude", () => { + const comments = [ + { author: null, body: "from a deleted account" }, + { author: { login: "user1" }, body: "comment2" }, + ]; + + const { filterCommentsByActor } = require("../src/github/data/fetcher"); + // Excluding "ghost" removes the deleted-account comment. + expect(filterCommentsByActor(comments, "", "ghost")).toHaveLength(1); + expect(filterCommentsByActor(comments, "", "ghost")[0].body).toBe( + "comment2", + ); + // Including only "ghost" keeps just the deleted-account comment. + const onlyGhost = filterCommentsByActor(comments, "ghost", ""); + expect(onlyGhost).toHaveLength(1); + expect(onlyGhost[0].body).toBe("from a deleted account"); + }); }); diff --git a/test/data-formatter.test.ts b/test/data-formatter.test.ts index 5f7aad38..b3e43629 100644 --- a/test/data-formatter.test.ts +++ b/test/data-formatter.test.ts @@ -159,6 +159,21 @@ Issue State: OPEN Issue Labels: architecture, agent-sdk, drift:functional`, ); }); + + test("renders a deleted (null-author) issue author as 'ghost'", () => { + const issueData: GitHubIssue = { + title: "Test Issue", + body: "Issue body", + author: null, + createdAt: "2023-01-01T00:00:00Z", + state: "OPEN", + labels: { nodes: [] }, + comments: { nodes: [] }, + }; + + const result = formatContext(issueData, false); + expect(result).toContain("Issue Author: ghost"); + }); }); describe("formatBody", () => { @@ -252,6 +267,24 @@ describe("formatComments", () => { ); }); + test("renders deleted (null-author) comments as 'ghost'", () => { + // GitHub returns author: null for comments from deleted accounts. + const comments: GitHubComment[] = [ + { + id: "1", + databaseId: "100001", + body: "From a deleted account", + author: null, + createdAt: "2023-01-01T00:00:00Z", + }, + ]; + + const result = formatComments(comments); + expect(result).toBe( + "[ghost at 2023-01-01T00:00:00Z]: From a deleted account", + ); + }); + test("returns empty string for empty comments array", () => { const result = formatComments([]); expect(result).toBe(""); @@ -494,6 +527,29 @@ describe("formatReviewComments", () => { ); }); + test("renders deleted (null-author) reviews as 'ghost'", () => { + const reviewData = { + nodes: [ + { + id: "review1", + databaseId: "300099", + author: null, + body: "Left before deleting the account", + state: "COMMENTED", + submittedAt: "2023-01-01T00:00:00Z", + comments: { + nodes: [], + }, + }, + ], + }; + + const result = formatReviewComments(reviewData); + expect(result).toBe( + `[Review by ghost at 2023-01-01T00:00:00Z]: COMMENTED\nLeft before deleting the account`, + ); + }); + test("formats multiple reviews correctly", () => { const reviewData = { nodes: [