Skip to content

Commit 3e807ec

Browse files
authored
fix: handle null comment/review author from deleted accounts (#1490)
GitHub's GraphQL author field is null when the account behind a comment, review, PR, or issue has been deleted (the ghost user). The action typed author as non-null and read author.login directly, so a single comment from a deleted account threw and was swallowed into a generic 'Failed to fetch PR/issue data', failing the entire run. Make author nullable on the four affected types and fall back to 'ghost' at each login read. With the type nullable, tsc flags every dereference, so all sites are covered.
1 parent 2988cbe commit 3e807ec

5 files changed

Lines changed: 112 additions & 14 deletions

File tree

src/github/data/fetcher.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,9 @@ export function isBodySafeToUse(
204204
* @param excludeActors - Comma-separated actors to exclude
205205
* @returns Filtered array of comments
206206
*/
207-
export function filterCommentsByActor<T extends { author: { login: string } }>(
208-
comments: T[],
209-
includeActors: string = "",
210-
excludeActors: string = "",
211-
): T[] {
207+
export function filterCommentsByActor<
208+
T extends { author: { login: string } | null },
209+
>(comments: T[], includeActors: string = "", excludeActors: string = ""): T[] {
212210
const includeParsed = parseActorFilter(includeActors);
213211
const excludeParsed = parseActorFilter(excludeActors);
214212

@@ -219,7 +217,9 @@ export function filterCommentsByActor<T extends { author: { login: string } }>(
219217

220218
return comments.filter((comment) =>
221219
shouldIncludeCommentByActor(
222-
comment.author.login,
220+
// author is null for comments from deleted ("ghost") accounts; treat them
221+
// as the "ghost" login so filtering never dereferences null and crashes.
222+
comment.author?.login ?? "ghost",
223223
includeParsed,
224224
excludeParsed,
225225
),

src/github/data/formatter.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export function formatContext(
2121
const prData = contextData as GitHubPullRequest;
2222
const sanitizedTitle = sanitizeContent(prData.title);
2323
return `PR Title: ${sanitizedTitle}
24-
PR Author: ${prData.author.login}
24+
PR Author: ${prData.author?.login ?? "ghost"}
2525
PR Branch: ${prData.headRefName} -> ${prData.baseRefName}
2626
PR State: ${prData.state}
2727
PR Labels: ${formatLabels(prData.labels.nodes)}
@@ -33,7 +33,7 @@ Changed Files: ${prData.files.nodes.length} files`;
3333
const issueData = contextData as GitHubIssue;
3434
const sanitizedTitle = sanitizeContent(issueData.title);
3535
return `Issue Title: ${sanitizedTitle}
36-
Issue Author: ${issueData.author.login}
36+
Issue Author: ${issueData.author?.login ?? "ghost"}
3737
Issue State: ${issueData.state}
3838
Issue Labels: ${formatLabels(issueData.labels.nodes)}`;
3939
}
@@ -71,7 +71,7 @@ export function formatComments(
7171

7272
body = sanitizeContent(body);
7373

74-
return `[${comment.author.login} at ${comment.createdAt}]: ${body}`;
74+
return `[${comment.author?.login ?? "ghost"} at ${comment.createdAt}]: ${body}`;
7575
})
7676
.join("\n\n");
7777
}
@@ -85,7 +85,7 @@ export function formatReviewComments(
8585
}
8686

8787
const formattedReviews = reviewData.nodes.map((review) => {
88-
let reviewOutput = `[Review by ${review.author.login} at ${review.submittedAt}]: ${review.state}`;
88+
let reviewOutput = `[Review by ${review.author?.login ?? "ghost"} at ${review.submittedAt}]: ${review.state}`;
8989

9090
if (review.body && review.body.trim()) {
9191
let body = review.body;

src/github/types.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
// Types for GitHub GraphQL query responses
2+
3+
// GitHub's GraphQL `author`/`actor` fields resolve to null when the underlying
4+
// account has been deleted (the "ghost" user). Any field typed as
5+
// `GitHubAuthor | null` can therefore be null at runtime and must be guarded.
26
export type GitHubAuthor = {
37
login: string;
48
name?: string;
@@ -8,7 +12,7 @@ export type GitHubComment = {
812
id: string;
913
databaseId: string;
1014
body: string;
11-
author: GitHubAuthor;
15+
author: GitHubAuthor | null;
1216
createdAt: string;
1317
updatedAt?: string;
1418
lastEditedAt?: string;
@@ -39,7 +43,7 @@ export type GitHubFile = {
3943
export type GitHubReview = {
4044
id: string;
4145
databaseId: string;
42-
author: GitHubAuthor;
46+
author: GitHubAuthor | null;
4347
body: string;
4448
state: string;
4549
submittedAt: string;
@@ -53,7 +57,7 @@ export type GitHubReview = {
5357
export type GitHubPullRequest = {
5458
title: string;
5559
body: string;
56-
author: GitHubAuthor;
60+
author: GitHubAuthor | null;
5761
baseRefName: string;
5862
headRefName: string;
5963
headRefOid: string;
@@ -95,7 +99,7 @@ export type GitHubPullRequest = {
9599
export type GitHubIssue = {
96100
title: string;
97101
body: string;
98-
author: GitHubAuthor;
102+
author: GitHubAuthor | null;
99103
createdAt: string;
100104
updatedAt?: string;
101105
lastEditedAt?: string;

test/data-fetcher.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,4 +1499,42 @@ describe("filterCommentsByActor", () => {
14991499
const filtered = filterCommentsByActor(comments, "user1", "");
15001500
expect(filtered).toHaveLength(0);
15011501
});
1502+
1503+
test("does not crash on comments from deleted (null-author) accounts", () => {
1504+
// GitHub's GraphQL returns author: null for comments whose account was
1505+
// deleted. With an exclude filter set (the exact `*[bot]` config we
1506+
// recommend), the null author must not throw when dereferenced.
1507+
const comments = [
1508+
{ author: { login: "user1" }, body: "comment1" },
1509+
{ author: null, body: "from a deleted account" },
1510+
{ author: { login: "bot[bot]" }, body: "comment3" },
1511+
];
1512+
1513+
const { filterCommentsByActor } = require("../src/github/data/fetcher");
1514+
const filtered = filterCommentsByActor(comments, "", "*[bot]");
1515+
// ghost comment is retained (it matches no exclude pattern); the bot is dropped.
1516+
expect(filtered).toHaveLength(2);
1517+
expect(filtered.map((c: any) => c.body)).toEqual([
1518+
"comment1",
1519+
"from a deleted account",
1520+
]);
1521+
});
1522+
1523+
test("treats null author as the 'ghost' login for include/exclude", () => {
1524+
const comments = [
1525+
{ author: null, body: "from a deleted account" },
1526+
{ author: { login: "user1" }, body: "comment2" },
1527+
];
1528+
1529+
const { filterCommentsByActor } = require("../src/github/data/fetcher");
1530+
// Excluding "ghost" removes the deleted-account comment.
1531+
expect(filterCommentsByActor(comments, "", "ghost")).toHaveLength(1);
1532+
expect(filterCommentsByActor(comments, "", "ghost")[0].body).toBe(
1533+
"comment2",
1534+
);
1535+
// Including only "ghost" keeps just the deleted-account comment.
1536+
const onlyGhost = filterCommentsByActor(comments, "ghost", "");
1537+
expect(onlyGhost).toHaveLength(1);
1538+
expect(onlyGhost[0].body).toBe("from a deleted account");
1539+
});
15021540
});

test/data-formatter.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,21 @@ Issue State: OPEN
159159
Issue Labels: architecture, agent-sdk, drift:functional`,
160160
);
161161
});
162+
163+
test("renders a deleted (null-author) issue author as 'ghost'", () => {
164+
const issueData: GitHubIssue = {
165+
title: "Test Issue",
166+
body: "Issue body",
167+
author: null,
168+
createdAt: "2023-01-01T00:00:00Z",
169+
state: "OPEN",
170+
labels: { nodes: [] },
171+
comments: { nodes: [] },
172+
};
173+
174+
const result = formatContext(issueData, false);
175+
expect(result).toContain("Issue Author: ghost");
176+
});
162177
});
163178

164179
describe("formatBody", () => {
@@ -252,6 +267,24 @@ describe("formatComments", () => {
252267
);
253268
});
254269

270+
test("renders deleted (null-author) comments as 'ghost'", () => {
271+
// GitHub returns author: null for comments from deleted accounts.
272+
const comments: GitHubComment[] = [
273+
{
274+
id: "1",
275+
databaseId: "100001",
276+
body: "From a deleted account",
277+
author: null,
278+
createdAt: "2023-01-01T00:00:00Z",
279+
},
280+
];
281+
282+
const result = formatComments(comments);
283+
expect(result).toBe(
284+
"[ghost at 2023-01-01T00:00:00Z]: From a deleted account",
285+
);
286+
});
287+
255288
test("returns empty string for empty comments array", () => {
256289
const result = formatComments([]);
257290
expect(result).toBe("");
@@ -494,6 +527,29 @@ describe("formatReviewComments", () => {
494527
);
495528
});
496529

530+
test("renders deleted (null-author) reviews as 'ghost'", () => {
531+
const reviewData = {
532+
nodes: [
533+
{
534+
id: "review1",
535+
databaseId: "300099",
536+
author: null,
537+
body: "Left before deleting the account",
538+
state: "COMMENTED",
539+
submittedAt: "2023-01-01T00:00:00Z",
540+
comments: {
541+
nodes: [],
542+
},
543+
},
544+
],
545+
};
546+
547+
const result = formatReviewComments(reviewData);
548+
expect(result).toBe(
549+
`[Review by ghost at 2023-01-01T00:00:00Z]: COMMENTED\nLeft before deleting the account`,
550+
);
551+
});
552+
497553
test("formats multiple reviews correctly", () => {
498554
const reviewData = {
499555
nodes: [

0 commit comments

Comments
 (0)