mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-08-22 03:18:54 +08:00
`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>
220 lines
6.7 KiB
TypeScript
220 lines
6.7 KiB
TypeScript
import { describe, expect, test } from "bun:test";
|
|
import {
|
|
parseActorFilter,
|
|
actorMatchesPattern,
|
|
resolveActorName,
|
|
shouldIncludeCommentByActor,
|
|
} from "../src/github/utils/actor-filter";
|
|
|
|
describe("parseActorFilter", () => {
|
|
test("parses comma-separated actors", () => {
|
|
expect(parseActorFilter("user1,user2,bot[bot]")).toEqual([
|
|
"user1",
|
|
"user2",
|
|
"bot[bot]",
|
|
]);
|
|
});
|
|
|
|
test("handles empty string", () => {
|
|
expect(parseActorFilter("")).toEqual([]);
|
|
});
|
|
|
|
test("handles whitespace-only string", () => {
|
|
expect(parseActorFilter(" ")).toEqual([]);
|
|
});
|
|
|
|
test("trims whitespace", () => {
|
|
expect(parseActorFilter(" user1 , user2 ")).toEqual(["user1", "user2"]);
|
|
});
|
|
|
|
test("filters out empty entries", () => {
|
|
expect(parseActorFilter("user1,,user2")).toEqual(["user1", "user2"]);
|
|
});
|
|
|
|
test("handles single actor", () => {
|
|
expect(parseActorFilter("user1")).toEqual(["user1"]);
|
|
});
|
|
|
|
test("handles wildcard bot pattern", () => {
|
|
expect(parseActorFilter("*[bot]")).toEqual(["*[bot]"]);
|
|
});
|
|
});
|
|
|
|
describe("actorMatchesPattern", () => {
|
|
test("matches exact username", () => {
|
|
expect(actorMatchesPattern("john-doe", "john-doe")).toBe(true);
|
|
});
|
|
|
|
test("does not match different username", () => {
|
|
expect(actorMatchesPattern("john-doe", "jane-doe")).toBe(false);
|
|
});
|
|
|
|
test("matches wildcard bot pattern", () => {
|
|
expect(actorMatchesPattern("dependabot[bot]", "*[bot]")).toBe(true);
|
|
expect(actorMatchesPattern("renovate[bot]", "*[bot]")).toBe(true);
|
|
expect(actorMatchesPattern("github-actions[bot]", "*[bot]")).toBe(true);
|
|
});
|
|
|
|
test("does not match non-bot with wildcard", () => {
|
|
expect(actorMatchesPattern("john-doe", "*[bot]")).toBe(false);
|
|
expect(actorMatchesPattern("user-bot", "*[bot]")).toBe(false);
|
|
});
|
|
|
|
test("matches specific bot", () => {
|
|
expect(actorMatchesPattern("dependabot[bot]", "dependabot[bot]")).toBe(
|
|
true,
|
|
);
|
|
expect(actorMatchesPattern("renovate[bot]", "renovate[bot]")).toBe(true);
|
|
});
|
|
|
|
test("does not match different specific bot", () => {
|
|
expect(actorMatchesPattern("dependabot[bot]", "renovate[bot]")).toBe(false);
|
|
});
|
|
|
|
test("is case sensitive", () => {
|
|
expect(actorMatchesPattern("User1", "user1")).toBe(false);
|
|
expect(actorMatchesPattern("user1", "User1")).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("shouldIncludeCommentByActor", () => {
|
|
test("includes all when no filters", () => {
|
|
expect(shouldIncludeCommentByActor("user1", [], [])).toBe(true);
|
|
expect(shouldIncludeCommentByActor("bot[bot]", [], [])).toBe(true);
|
|
});
|
|
|
|
test("excludes when in exclude list", () => {
|
|
expect(shouldIncludeCommentByActor("bot[bot]", [], ["*[bot]"])).toBe(false);
|
|
expect(shouldIncludeCommentByActor("user1", [], ["user1"])).toBe(false);
|
|
});
|
|
|
|
test("includes when not in exclude list", () => {
|
|
expect(shouldIncludeCommentByActor("user1", [], ["user2"])).toBe(true);
|
|
expect(shouldIncludeCommentByActor("user1", [], ["*[bot]"])).toBe(true);
|
|
});
|
|
|
|
test("includes when in include list", () => {
|
|
expect(shouldIncludeCommentByActor("user1", ["user1", "user2"], [])).toBe(
|
|
true,
|
|
);
|
|
expect(shouldIncludeCommentByActor("user2", ["user1", "user2"], [])).toBe(
|
|
true,
|
|
);
|
|
});
|
|
|
|
test("excludes when not in include list", () => {
|
|
expect(shouldIncludeCommentByActor("user3", ["user1", "user2"], [])).toBe(
|
|
false,
|
|
);
|
|
});
|
|
|
|
test("exclusion takes priority over inclusion", () => {
|
|
expect(shouldIncludeCommentByActor("user1", ["user1"], ["user1"])).toBe(
|
|
false,
|
|
);
|
|
expect(
|
|
shouldIncludeCommentByActor("bot[bot]", ["*[bot]"], ["*[bot]"]),
|
|
).toBe(false);
|
|
});
|
|
|
|
test("handles wildcard in include list", () => {
|
|
expect(shouldIncludeCommentByActor("dependabot[bot]", ["*[bot]"], [])).toBe(
|
|
true,
|
|
);
|
|
expect(shouldIncludeCommentByActor("renovate[bot]", ["*[bot]"], [])).toBe(
|
|
true,
|
|
);
|
|
expect(shouldIncludeCommentByActor("user1", ["*[bot]"], [])).toBe(false);
|
|
});
|
|
|
|
test("handles wildcard in exclude list", () => {
|
|
expect(shouldIncludeCommentByActor("dependabot[bot]", [], ["*[bot]"])).toBe(
|
|
false,
|
|
);
|
|
expect(shouldIncludeCommentByActor("renovate[bot]", [], ["*[bot]"])).toBe(
|
|
false,
|
|
);
|
|
expect(shouldIncludeCommentByActor("user1", [], ["*[bot]"])).toBe(true);
|
|
});
|
|
|
|
test("handles mixed include and exclude lists", () => {
|
|
// Include user1 and user2, but exclude user2
|
|
expect(
|
|
shouldIncludeCommentByActor("user1", ["user1", "user2"], ["user2"]),
|
|
).toBe(true);
|
|
expect(
|
|
shouldIncludeCommentByActor("user2", ["user1", "user2"], ["user2"]),
|
|
).toBe(false);
|
|
expect(
|
|
shouldIncludeCommentByActor("user3", ["user1", "user2"], ["user2"]),
|
|
).toBe(false);
|
|
});
|
|
|
|
test("handles complex bot filtering", () => {
|
|
// Include all bots but exclude dependabot
|
|
expect(
|
|
shouldIncludeCommentByActor(
|
|
"renovate[bot]",
|
|
["*[bot]"],
|
|
["dependabot[bot]"],
|
|
),
|
|
).toBe(true);
|
|
expect(
|
|
shouldIncludeCommentByActor(
|
|
"dependabot[bot]",
|
|
["*[bot]"],
|
|
["dependabot[bot]"],
|
|
),
|
|
).toBe(false);
|
|
expect(
|
|
shouldIncludeCommentByActor("user1", ["*[bot]"], ["dependabot[bot]"]),
|
|
).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,
|
|
);
|
|
});
|
|
});
|