mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-08-22 03:18:54 +08:00
fix(github): match bot actors in comment filters using GraphQL __typename (#1616)
`exclude_comments_by_actor` and `include_comments_by_actor` never matched
any bot. Both the documented `*[bot]` wildcard and exact entries such as
`dependabot[bot]` silently did nothing.
GitHub's GraphQL API returns the bare login for App actors ("dependabot"),
while REST and the GitHub UI append a suffix ("dependabot[bot]"). Filter
patterns are written in the suffixed form, so matching a GraphQL login
against them could never succeed and `actor.endsWith("[bot]")` was dead
code.
Request `__typename` on the Actor-typed author selections and normalize
App actors to their suffixed name via `resolveActorName()` before matching.
Normalizing at the filter boundary fixes the wildcard and exact-match cases
together, and leaves the author names shown in the prompt unchanged.
The existing test mocked `login: "scanner[bot]"`, a payload GraphQL never
produces, which is why the gap was invisible. It now mocks the real shape
(`__typename: "Bot", login: "scanner"`) and fails without this fix.
The commit author selection is left alone: it is a GitCommit, not an Actor.
Fixes #1514
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0a80d21df7
commit
65b50df083
@@ -7,6 +7,7 @@ export const PR_QUERY = `
|
|||||||
title
|
title
|
||||||
body
|
body
|
||||||
author {
|
author {
|
||||||
|
__typename
|
||||||
login
|
login
|
||||||
}
|
}
|
||||||
baseRefName
|
baseRefName
|
||||||
@@ -57,6 +58,7 @@ export const PR_QUERY = `
|
|||||||
databaseId
|
databaseId
|
||||||
body
|
body
|
||||||
author {
|
author {
|
||||||
|
__typename
|
||||||
login
|
login
|
||||||
}
|
}
|
||||||
createdAt
|
createdAt
|
||||||
@@ -70,6 +72,7 @@ export const PR_QUERY = `
|
|||||||
id
|
id
|
||||||
databaseId
|
databaseId
|
||||||
author {
|
author {
|
||||||
|
__typename
|
||||||
login
|
login
|
||||||
}
|
}
|
||||||
body
|
body
|
||||||
@@ -86,6 +89,7 @@ export const PR_QUERY = `
|
|||||||
line
|
line
|
||||||
diffHunk
|
diffHunk
|
||||||
author {
|
author {
|
||||||
|
__typename
|
||||||
login
|
login
|
||||||
}
|
}
|
||||||
createdAt
|
createdAt
|
||||||
@@ -108,6 +112,7 @@ export const ISSUE_QUERY = `
|
|||||||
title
|
title
|
||||||
body
|
body
|
||||||
author {
|
author {
|
||||||
|
__typename
|
||||||
login
|
login
|
||||||
}
|
}
|
||||||
createdAt
|
createdAt
|
||||||
@@ -125,6 +130,7 @@ export const ISSUE_QUERY = `
|
|||||||
databaseId
|
databaseId
|
||||||
body
|
body
|
||||||
author {
|
author {
|
||||||
|
__typename
|
||||||
login
|
login
|
||||||
}
|
}
|
||||||
createdAt
|
createdAt
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import type { CommentWithImages } from "../utils/image-downloader";
|
|||||||
import { downloadCommentImages } from "../utils/image-downloader";
|
import { downloadCommentImages } from "../utils/image-downloader";
|
||||||
import {
|
import {
|
||||||
parseActorFilter,
|
parseActorFilter,
|
||||||
|
resolveActorName,
|
||||||
shouldIncludeCommentByActor,
|
shouldIncludeCommentByActor,
|
||||||
} from "../utils/actor-filter";
|
} from "../utils/actor-filter";
|
||||||
|
|
||||||
@@ -339,7 +340,7 @@ export function isBodySafeToUse(
|
|||||||
* @returns Filtered array of comments
|
* @returns Filtered array of comments
|
||||||
*/
|
*/
|
||||||
export function filterCommentsByActor<
|
export function filterCommentsByActor<
|
||||||
T extends { author: { login: string } | null },
|
T extends { author: { login: string; __typename?: string } | null },
|
||||||
>(comments: T[], includeActors: string = "", excludeActors: string = ""): T[] {
|
>(comments: T[], includeActors: string = "", excludeActors: string = ""): T[] {
|
||||||
const includeParsed = parseActorFilter(includeActors);
|
const includeParsed = parseActorFilter(includeActors);
|
||||||
const excludeParsed = parseActorFilter(excludeActors);
|
const excludeParsed = parseActorFilter(excludeActors);
|
||||||
@@ -351,9 +352,10 @@ export function filterCommentsByActor<
|
|||||||
|
|
||||||
return comments.filter((comment) =>
|
return comments.filter((comment) =>
|
||||||
shouldIncludeCommentByActor(
|
shouldIncludeCommentByActor(
|
||||||
// author is null for comments from deleted ("ghost") accounts; treat them
|
// Normalizes App actors to their "[bot]"-suffixed name, which is the form
|
||||||
// as the "ghost" login so filtering never dereferences null and crashes.
|
// filter patterns are written in. Also maps deleted ("ghost") accounts,
|
||||||
comment.author?.login ?? "ghost",
|
// whose author is null, to "ghost" so filtering never dereferences null.
|
||||||
|
resolveActorName(comment.author),
|
||||||
includeParsed,
|
includeParsed,
|
||||||
excludeParsed,
|
excludeParsed,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -3,9 +3,14 @@
|
|||||||
// GitHub's GraphQL `author`/`actor` fields resolve to null when the underlying
|
// GitHub's GraphQL `author`/`actor` fields resolve to null when the underlying
|
||||||
// account has been deleted (the "ghost" user). Any field typed as
|
// account has been deleted (the "ghost" user). Any field typed as
|
||||||
// `GitHubAuthor | null` can therefore be null at runtime and must be guarded.
|
// `GitHubAuthor | null` can therefore be null at runtime and must be guarded.
|
||||||
|
// `__typename` distinguishes an App/bot actor from a human. GraphQL's
|
||||||
|
// `Actor.login` returns the bare name for bots ("dependabot"), unlike REST which
|
||||||
|
// appends a suffix ("dependabot[bot]"), so the typename is the only reliable bot
|
||||||
|
// signal on this data. See `resolveActorName` in `utils/actor-filter.ts`.
|
||||||
export type GitHubAuthor = {
|
export type GitHubAuthor = {
|
||||||
login: string;
|
login: string;
|
||||||
name?: string;
|
name?: string;
|
||||||
|
__typename?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
export type GitHubComment = {
|
export type GitHubComment = {
|
||||||
|
|||||||
@@ -11,6 +11,31 @@ export function parseActorFilter(filterString: string): string[] {
|
|||||||
.filter((actor) => actor.length > 0);
|
.filter((actor) => actor.length > 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolves the name to match actor filter patterns against.
|
||||||
|
*
|
||||||
|
* GitHub's GraphQL API returns the bare login for App actors ("dependabot"),
|
||||||
|
* whereas REST and the GitHub UI use a "[bot]" suffix ("dependabot[bot]"). Users
|
||||||
|
* write filter patterns in the suffixed form, both the documented "*[bot]"
|
||||||
|
* wildcard and exact entries like "renovate[bot]", so GraphQL bot logins are
|
||||||
|
* normalized to that form before matching. Without this no "[bot]" pattern can
|
||||||
|
* ever match, because the suffix is simply absent from the data.
|
||||||
|
*
|
||||||
|
* @param author - Comment author; null for deleted ("ghost") accounts
|
||||||
|
* @returns Actor name, "[bot]"-suffixed for App actors
|
||||||
|
*/
|
||||||
|
export function resolveActorName(
|
||||||
|
author: { login: string; __typename?: string } | null | undefined,
|
||||||
|
): string {
|
||||||
|
if (!author) return "ghost";
|
||||||
|
|
||||||
|
if (author.__typename === "Bot" && !author.login.endsWith("[bot]")) {
|
||||||
|
return `${author.login}[bot]`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return author.login;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Checks if an actor matches a pattern
|
* Checks if an actor matches a pattern
|
||||||
* Supports wildcards: "*[bot]" matches all bots, "dependabot[bot]" matches specific
|
* Supports wildcards: "*[bot]" matches all bots, "dependabot[bot]" matches specific
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
|
|||||||
import {
|
import {
|
||||||
parseActorFilter,
|
parseActorFilter,
|
||||||
actorMatchesPattern,
|
actorMatchesPattern,
|
||||||
|
resolveActorName,
|
||||||
shouldIncludeCommentByActor,
|
shouldIncludeCommentByActor,
|
||||||
} from "../src/github/utils/actor-filter";
|
} from "../src/github/utils/actor-filter";
|
||||||
|
|
||||||
@@ -170,3 +171,49 @@ describe("shouldIncludeCommentByActor", () => {
|
|||||||
).toBe(false);
|
).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("resolveActorName", () => {
|
||||||
|
test("appends the [bot] suffix to GraphQL App actors", () => {
|
||||||
|
// GraphQL returns the bare login for bots; REST would say "dependabot[bot]".
|
||||||
|
expect(resolveActorName({ __typename: "Bot", login: "dependabot" })).toBe(
|
||||||
|
"dependabot[bot]",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("leaves human logins untouched", () => {
|
||||||
|
expect(resolveActorName({ __typename: "User", login: "octocat" })).toBe(
|
||||||
|
"octocat",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("does not double-suffix a login that already ends with [bot]", () => {
|
||||||
|
expect(
|
||||||
|
resolveActorName({ __typename: "Bot", login: "dependabot[bot]" }),
|
||||||
|
).toBe("dependabot[bot]");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("maps deleted accounts to ghost", () => {
|
||||||
|
expect(resolveActorName(null)).toBe("ghost");
|
||||||
|
expect(resolveActorName(undefined)).toBe("ghost");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("falls back to the login when __typename is absent", () => {
|
||||||
|
expect(resolveActorName({ login: "octocat" })).toBe("octocat");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a bot actor matches the *[bot] wildcard once resolved", () => {
|
||||||
|
const actor = resolveActorName({ __typename: "Bot", login: "renovate" });
|
||||||
|
|
||||||
|
expect(actorMatchesPattern(actor, "*[bot]")).toBe(true);
|
||||||
|
// The raw GraphQL login never matches, which is the bug being fixed.
|
||||||
|
expect(actorMatchesPattern("renovate", "*[bot]")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a bot actor matches an exact [bot] pattern once resolved", () => {
|
||||||
|
const actor = resolveActorName({ __typename: "Bot", login: "dependabot" });
|
||||||
|
|
||||||
|
expect(shouldIncludeCommentByActor(actor, [], ["dependabot[bot]"])).toBe(
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -1215,7 +1215,10 @@ describe("fetchGitHubData integration with time filtering", () => {
|
|||||||
{
|
{
|
||||||
id: "2",
|
id: "2",
|
||||||
databaseId: "2",
|
databaseId: "2",
|
||||||
author: { login: "scanner[bot]" },
|
// GraphQL returns the bare login for App actors plus
|
||||||
|
// __typename: "Bot". It does NOT append a "[bot]" suffix the
|
||||||
|
// way REST does, so this mirrors a real payload.
|
||||||
|
author: { __typename: "Bot", login: "scanner" },
|
||||||
body: "Pre-trigger bot review",
|
body: "Pre-trigger bot review",
|
||||||
state: "COMMENTED",
|
state: "COMMENTED",
|
||||||
submittedAt: "2024-01-15T11:00:00Z",
|
submittedAt: "2024-01-15T11:00:00Z",
|
||||||
|
|||||||
Reference in New Issue
Block a user