diff --git a/docs/custom-automations.md b/docs/custom-automations.md index fabb52f..15318f3 100644 --- a/docs/custom-automations.md +++ b/docs/custom-automations.md @@ -54,6 +54,11 @@ on: pull_request: types: [opened, synchronize] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: review-by-author: if: | diff --git a/docs/solutions.md b/docs/solutions.md index 2315064..4185c70 100644 --- a/docs/solutions.md +++ b/docs/solutions.md @@ -27,6 +27,11 @@ on: pull_request: types: [opened, synchronize] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: review: runs-on: ubuntu-latest @@ -81,6 +86,11 @@ on: pull_request: types: [opened, synchronize, ready_for_review, reopened] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: review: runs-on: ubuntu-latest @@ -145,6 +155,11 @@ on: - "src/api/**" - "config/security.yml" +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: security-review: runs-on: ubuntu-latest @@ -202,6 +217,11 @@ on: pull_request: types: [opened, synchronize] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: external-review: if: github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR' @@ -260,6 +280,11 @@ on: pull_request: types: [opened, synchronize] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: checklist-review: runs-on: ubuntu-latest @@ -448,6 +473,11 @@ on: - "src/api/**/*.ts" - "src/routes/**/*.ts" +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: doc-sync: runs-on: ubuntu-latest @@ -504,6 +534,11 @@ on: pull_request: types: [opened, synchronize] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: security: runs-on: ubuntu-latest diff --git a/examples/pr-review-comprehensive.yml b/examples/pr-review-comprehensive.yml index 3002b4d..3ddc0b5 100644 --- a/examples/pr-review-comprehensive.yml +++ b/examples/pr-review-comprehensive.yml @@ -7,6 +7,11 @@ on: pull_request: types: [opened, synchronize, ready_for_review, reopened] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: review-with-tracking: runs-on: ubuntu-latest diff --git a/examples/pr-review-filtered-authors.yml b/examples/pr-review-filtered-authors.yml index 0032720..26d3d93 100644 --- a/examples/pr-review-filtered-authors.yml +++ b/examples/pr-review-filtered-authors.yml @@ -4,6 +4,11 @@ on: pull_request: types: [opened, synchronize] +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: review-by-author: # Only run for PRs from specific authors diff --git a/examples/pr-review-filtered-paths.yml b/examples/pr-review-filtered-paths.yml index f465a4b..932d6f5 100644 --- a/examples/pr-review-filtered-paths.yml +++ b/examples/pr-review-filtered-paths.yml @@ -10,6 +10,11 @@ on: - "api/**/*.py" # You can add more specific patterns as needed +# Prevent multiple reviews from running simultaneously on the same PR +concurrency: + group: claude-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: claude-review-paths: runs-on: ubuntu-latest diff --git a/src/github/validation/actor.ts b/src/github/validation/actor.ts index 2599254..36be2ff 100644 --- a/src/github/validation/actor.ts +++ b/src/github/validation/actor.ts @@ -6,11 +6,11 @@ */ import type { Octokit } from "@octokit/rest"; -import type { ParsedGitHubContext } from "../context"; +import type { GitHubContext } from "../context"; export async function checkHumanActor( octokit: Octokit, - githubContext: ParsedGitHubContext, + githubContext: GitHubContext, ) { // Fetch user information from GitHub API const { data: userData } = await octokit.users.getByUsername({ diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index 4bcd4aa..864f4fe 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -5,6 +5,7 @@ import type { PreparedContext } from "../../create-prompt/types"; import { prepareMcpConfig } from "../../mcp/install-mcp-server"; import { parseAllowedTools } from "./parse-tools"; import { configureGitAuth } from "../../github/operations/git-config"; +import { checkHumanActor } from "../../github/validation/actor"; import type { GitHubContext } from "../../github/context"; import { isEntityContext } from "../../github/context"; @@ -77,7 +78,14 @@ export const agentMode: Mode = { return false; }, - async prepare({ context, githubToken }: ModeOptions): Promise { + async prepare({ + context, + octokit, + githubToken, + }: ModeOptions): Promise { + // Check if actor is human (prevents bot-triggered loops) + await checkHumanActor(octokit.rest, context); + // Configure git authentication for agent mode (same as tag mode) if (!context.inputs.useCommitSigning) { // Use bot_id and bot_name from inputs directly diff --git a/test/modes/agent.test.ts b/test/modes/agent.test.ts index 16e3796..25bf844 100644 --- a/test/modes/agent.test.ts +++ b/test/modes/agent.test.ts @@ -145,12 +145,12 @@ describe("Agent Mode", () => { users: { getAuthenticated: mock(() => Promise.resolve({ - data: { login: "test-user", id: 12345 }, + data: { login: "test-user", id: 12345, type: "User" }, }), ), getByUsername: mock(() => Promise.resolve({ - data: { login: "test-user", id: 12345 }, + data: { login: "test-user", id: 12345, type: "User" }, }), ), }, @@ -187,6 +187,65 @@ describe("Agent Mode", () => { process.env.GITHUB_REF_NAME = originalRefName; }); + test("prepare method rejects bot actors without allowed_bots", async () => { + const contextWithPrompts = createMockAutomationContext({ + eventName: "workflow_dispatch", + }); + contextWithPrompts.actor = "claude[bot]"; + contextWithPrompts.inputs.allowedBots = ""; + + const mockOctokit = { + rest: { + users: { + getByUsername: mock(() => + Promise.resolve({ + data: { login: "claude[bot]", id: 12345, type: "Bot" }, + }), + ), + }, + }, + } as any; + + await expect( + agentMode.prepare({ + context: contextWithPrompts, + octokit: mockOctokit, + githubToken: "test-token", + }), + ).rejects.toThrow( + "Workflow initiated by non-human actor: claude (type: Bot)", + ); + }); + + test("prepare method allows bot actors when in allowed_bots list", async () => { + const contextWithPrompts = createMockAutomationContext({ + eventName: "workflow_dispatch", + }); + contextWithPrompts.actor = "dependabot[bot]"; + contextWithPrompts.inputs.allowedBots = "dependabot"; + + const mockOctokit = { + rest: { + users: { + getByUsername: mock(() => + Promise.resolve({ + data: { login: "dependabot[bot]", id: 12345, type: "Bot" }, + }), + ), + }, + }, + } as any; + + // Should not throw - bot is in allowed list + await expect( + agentMode.prepare({ + context: contextWithPrompts, + octokit: mockOctokit, + githubToken: "test-token", + }), + ).resolves.toBeDefined(); + }); + test("prepare method creates prompt file with correct content", async () => { const contextWithPrompts = createMockAutomationContext({ eventName: "workflow_dispatch", @@ -199,12 +258,12 @@ describe("Agent Mode", () => { users: { getAuthenticated: mock(() => Promise.resolve({ - data: { login: "test-user", id: 12345 }, + data: { login: "test-user", id: 12345, type: "User" }, }), ), getByUsername: mock(() => Promise.resolve({ - data: { login: "test-user", id: 12345 }, + data: { login: "test-user", id: 12345, type: "User" }, }), ), },