From bf34e22e43b8fe91d55ebd8bb6dceac2663d0726 Mon Sep 17 00:00:00 2001 From: Yuku Kotani Date: Mon, 21 Jul 2025 16:10:35 +0900 Subject: [PATCH] refactor: move allowedBots parameter to context object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move allowedBots from function parameter to context.inputs to maintain consistency with other input handling throughout the codebase. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/entrypoints/prepare.ts | 3 +-- src/github/context.ts | 2 ++ src/github/validation/actor.ts | 3 ++- test/actor.test.ts | 16 +++++++++------- test/install-mcp-server.test.ts | 1 + test/mockContext.ts | 1 + test/trigger-validation.test.ts | 5 +++++ 7 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index 0b1c551..d5e968f 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -48,8 +48,7 @@ async function run() { } // Step 5: Check if actor is human - const allowedBots = process.env.ALLOWED_BOTS || ""; - await checkHumanActor(octokit.rest, context, allowedBots); + await checkHumanActor(octokit.rest, context); // Step 6: Create initial tracking comment const commentData = await createInitialComment(octokit.rest, context); diff --git a/src/github/context.ts b/src/github/context.ts index c156b54..c43f2d6 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -39,6 +39,7 @@ export type ParsedGitHubContext = { useStickyComment: boolean; additionalPermissions: Map; useCommitSigning: boolean; + allowedBots: string; }; }; @@ -70,6 +71,7 @@ export function parseGitHubContext(): ParsedGitHubContext { process.env.ADDITIONAL_PERMISSIONS ?? "", ), useCommitSigning: process.env.USE_COMMIT_SIGNING === "true", + allowedBots: process.env.ALLOWED_BOTS ?? "", }, }; diff --git a/src/github/validation/actor.ts b/src/github/validation/actor.ts index 4d430af..4fe2bcb 100644 --- a/src/github/validation/actor.ts +++ b/src/github/validation/actor.ts @@ -11,7 +11,6 @@ import type { ParsedGitHubContext } from "../context"; export async function checkHumanActor( octokit: Octokit, githubContext: ParsedGitHubContext, - allowedBots: string, ) { // Fetch user information from GitHub API const { data: userData } = await octokit.users.getByUsername({ @@ -24,6 +23,8 @@ export async function checkHumanActor( // Check bot permissions if actor is not a User if (actorType !== "User") { + const allowedBots = githubContext.inputs.allowedBots; + // Parse allowed bots list const allowedBotsList = allowedBots .split(",") diff --git a/test/actor.test.ts b/test/actor.test.ts index bdb13f7..677689d 100644 --- a/test/actor.test.ts +++ b/test/actor.test.ts @@ -24,7 +24,7 @@ describe("checkHumanActor", () => { context.actor = "human-user"; await expect( - checkHumanActor(mockOctokit, context, ""), + checkHumanActor(mockOctokit, context), ).resolves.toBeUndefined(); }); @@ -32,8 +32,9 @@ describe("checkHumanActor", () => { const mockOctokit = createMockOctokit("Bot"); const context = createMockContext(); context.actor = "test-bot"; + context.inputs.allowedBots = ""; - await expect(checkHumanActor(mockOctokit, context, "")).rejects.toThrow( + await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow( "Workflow initiated by non-human actor: test-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.", ); }); @@ -42,9 +43,10 @@ describe("checkHumanActor", () => { const mockOctokit = createMockOctokit("Bot"); const context = createMockContext(); context.actor = "test-bot"; + context.inputs.allowedBots = "*"; await expect( - checkHumanActor(mockOctokit, context, "*"), + checkHumanActor(mockOctokit, context), ).resolves.toBeUndefined(); }); @@ -52,9 +54,10 @@ describe("checkHumanActor", () => { const mockOctokit = createMockOctokit("Bot"); const context = createMockContext(); context.actor = "dependabot"; + context.inputs.allowedBots = "dependabot,renovate"; await expect( - checkHumanActor(mockOctokit, context, "dependabot,renovate"), + checkHumanActor(mockOctokit, context), ).resolves.toBeUndefined(); }); @@ -62,10 +65,9 @@ describe("checkHumanActor", () => { const mockOctokit = createMockOctokit("Bot"); const context = createMockContext(); context.actor = "other-bot"; + context.inputs.allowedBots = "dependabot,renovate"; - await expect( - checkHumanActor(mockOctokit, context, "dependabot,renovate"), - ).rejects.toThrow( + await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow( "Workflow initiated by non-human actor: other-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.", ); }); diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts index 3f14a6e..79ef74b 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -35,6 +35,7 @@ describe("prepareMcpConfig", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }; diff --git a/test/mockContext.ts b/test/mockContext.ts index d035afc..76f7127 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -23,6 +23,7 @@ const defaultInputs = { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }; const defaultRepository = { diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index eaaf834..16f6244 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -39,6 +39,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -70,6 +71,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(false); @@ -285,6 +287,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -317,6 +320,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -349,6 +353,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(false);