From f59258677e58638757c58e266cf1e5bd91b49f7f Mon Sep 17 00:00:00 2001 From: km-anthropic Date: Fri, 8 Aug 2025 00:32:57 -0700 Subject: [PATCH] refactor: complete v1.0 simplification by removing all legacy inputs - Remove all backward compatibility for v1.0 simplification - Remove 10 legacy inputs from base-action/action.yml - Remove 9 legacy inputs from main action.yml - Simplify ClaudeOptions type to just timeoutMinutes and claudeArgs - Remove all legacy option handling from prepareRunConfig - Update tests to remove references to deleted fields - Remove obsolete test file github/context.test.ts - Clean up types to remove customInstructions, allowedTools, disallowedTools Users now use claudeArgs exclusively for CLI control. --- action.yml | 50 +--- base-action/action.yml | 47 +--- base-action/src/index.ts | 10 +- base-action/src/run-claude.ts | 87 +----- base-action/test/run-claude.test.ts | 257 ------------------ .../claude-args-example.yml | 3 +- examples/claude-auto-review.yml | 4 +- examples/claude-experimental-review-mode.yml | 7 +- examples/claude-pr-path-specific.yml | 4 +- examples/claude-review-from-author.yml | 4 +- src/create-prompt/index.ts | 32 +-- src/create-prompt/types.ts | 4 +- src/github/context.ts | 37 --- src/mcp/install-mcp-server.ts | 5 +- src/modes/agent/index.ts | 15 +- src/modes/tag/index.ts | 2 +- test/create-prompt.test.ts | 7 +- test/github/context.test.ts | 115 -------- test/install-mcp-server.test.ts | 10 - test/mockContext.ts | 8 - test/permissions.test.ts | 4 - test/prepare-context.test.ts | 30 +- test/trigger-validation.test.ts | 20 -- 23 files changed, 47 insertions(+), 715 deletions(-) rename claude-args-example.yml => examples/claude-args-example.yml (84%) delete mode 100644 test/github/context.test.ts diff --git a/action.yml b/action.yml index 717379f..37db3b8 100644 --- a/action.yml +++ b/action.yml @@ -25,36 +25,8 @@ inputs: default: "claude/" # Claude Code configuration - model: - description: "Model to use (provider-specific format required for Bedrock/Vertex)" - required: false - fallback_model: - description: "Enable automatic fallback to specified model when primary model is unavailable" - required: false - allowed_tools: - description: "Additional tools for Claude to use (the base GitHub tools will always be included)" - required: false - default: "" - disallowed_tools: - description: "Tools that Claude should never use" - required: false - default: "" - custom_instructions: - description: "Additional custom instructions to include in the prompt for Claude" - required: false - default: "" prompt: - description: "Instructions for Claude. Can be a direct prompt, slash command (e.g. /review), or custom template." - required: false - default: "" - mcp_config: - description: "Additional MCP configuration (JSON string) that merges with the built-in GitHub MCP servers" - additional_permissions: - description: "Additional permissions to enable. Currently supports 'actions: read' for viewing workflow results" - required: false - default: "" - claude_env: - description: "Custom environment variables to pass to Claude Code execution (YAML format)" + description: "Instructions for Claude. Can be a direct prompt or custom template." required: false default: "" settings: @@ -81,10 +53,6 @@ inputs: required: false default: "false" - max_turns: - description: "Maximum number of conversation turns" - required: false - default: "" timeout_minutes: description: "Timeout in minutes for execution" required: false @@ -141,17 +109,10 @@ runs: LABEL_TRIGGER: ${{ inputs.label_trigger }} BASE_BRANCH: ${{ inputs.base_branch }} BRANCH_PREFIX: ${{ inputs.branch_prefix }} - ALLOWED_TOOLS: ${{ inputs.allowed_tools }} - DISALLOWED_TOOLS: ${{ inputs.disallowed_tools }} - CUSTOM_INSTRUCTIONS: ${{ inputs.custom_instructions }} - DIRECT_PROMPT: ${{ inputs.direct_prompt }} - OVERRIDE_PROMPT: ${{ inputs.override_prompt }} - MCP_CONFIG: ${{ inputs.mcp_config }} OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} GITHUB_RUN_ID: ${{ github.run_id }} USE_STICKY_COMMENT: ${{ inputs.use_sticky_comment }} DEFAULT_WORKFLOW_TOKEN: ${{ github.token }} - ADDITIONAL_PERMISSIONS: ${{ inputs.additional_permissions }} USE_COMMIT_SIGNING: ${{ inputs.use_commit_signing }} - name: Install Base Action Dependencies @@ -187,21 +148,12 @@ runs: # Base-action inputs CLAUDE_CODE_ACTION: "1" INPUT_PROMPT_FILE: ${{ runner.temp }}/claude-prompts/claude-prompt.txt - INPUT_ALLOWED_TOOLS: ${{ env.ALLOWED_TOOLS }} - INPUT_DISALLOWED_TOOLS: ${{ env.DISALLOWED_TOOLS }} - INPUT_MAX_TURNS: ${{ inputs.max_turns }} - INPUT_MCP_CONFIG: ${{ steps.prepare.outputs.mcp_config }} INPUT_SETTINGS: ${{ inputs.settings }} - INPUT_SYSTEM_PROMPT: "" - INPUT_APPEND_SYSTEM_PROMPT: ${{ env.APPEND_SYSTEM_PROMPT }} INPUT_TIMEOUT_MINUTES: ${{ inputs.timeout_minutes }} - INPUT_CLAUDE_ENV: ${{ inputs.claude_env }} - INPUT_FALLBACK_MODEL: ${{ inputs.fallback_model }} INPUT_CLAUDE_ARGS: ${{ inputs.claude_args }} INPUT_EXPERIMENTAL_SLASH_COMMANDS_DIR: ${{ github.action_path }}/slash-commands # Model configuration - ANTHROPIC_MODEL: ${{ inputs.model || inputs.anthropic_model }} GITHUB_TOKEN: ${{ steps.prepare.outputs.GITHUB_TOKEN }} NODE_VERSION: ${{ env.NODE_VERSION }} DETAILED_PERMISSION_MESSAGES: "1" diff --git a/base-action/action.yml b/base-action/action.yml index cb39102..45e0051 100644 --- a/base-action/action.yml +++ b/base-action/action.yml @@ -14,47 +14,10 @@ inputs: description: "Path to a file containing the prompt to send to Claude Code (mutually exclusive with prompt)" required: false default: "" - allowed_tools: - description: "Comma-separated list of allowed tools for Claude Code to use" - required: false - default: "" - disallowed_tools: - description: "Comma-separated list of disallowed tools that Claude Code cannot use" - required: false - default: "" - max_turns: - description: "Maximum number of conversation turns (default: no limit)" - required: false - default: "" - mcp_config: - description: "MCP configuration as JSON string or path to MCP configuration JSON file" - required: false - default: "" settings: description: "Claude Code settings as JSON string or path to settings JSON file" required: false default: "" - system_prompt: - description: "Override system prompt" - required: false - default: "" - append_system_prompt: - description: "Append to system prompt" - required: false - default: "" - model: - description: "Model to use (provider-specific format required for Bedrock/Vertex)" - required: false - anthropic_model: - description: "DEPRECATED: Use 'model' instead. Model to use (provider-specific format required for Bedrock/Vertex)" - required: false - fallback_model: - description: "Enable automatic fallback to specified model when default model is unavailable" - required: false - claude_env: - description: "Custom environment variables to pass to Claude Code execution (YAML multiline format)" - required: false - default: "" # Action settings timeout_minutes: @@ -137,19 +100,11 @@ runs: env: # Model configuration CLAUDE_CODE_ACTION: "1" - ANTHROPIC_MODEL: ${{ inputs.model || inputs.anthropic_model }} INPUT_PROMPT: ${{ inputs.prompt }} INPUT_PROMPT_FILE: ${{ inputs.prompt_file }} - INPUT_ALLOWED_TOOLS: ${{ inputs.allowed_tools }} - INPUT_DISALLOWED_TOOLS: ${{ inputs.disallowed_tools }} - INPUT_MAX_TURNS: ${{ inputs.max_turns }} - INPUT_MCP_CONFIG: ${{ inputs.mcp_config }} INPUT_SETTINGS: ${{ inputs.settings }} - INPUT_SYSTEM_PROMPT: ${{ inputs.system_prompt }} - INPUT_APPEND_SYSTEM_PROMPT: ${{ inputs.append_system_prompt }} INPUT_TIMEOUT_MINUTES: ${{ inputs.timeout_minutes }} - INPUT_CLAUDE_ENV: ${{ inputs.claude_env }} - INPUT_FALLBACK_MODEL: ${{ inputs.fallback_model }} + INPUT_CLAUDE_ARGS: ${{ inputs.claude_args }} INPUT_EXPERIMENTAL_SLASH_COMMANDS_DIR: ${{ inputs.experimental_slash_commands_dir }} # Provider configuration diff --git a/base-action/src/index.ts b/base-action/src/index.ts index 8a47de7..dd467a8 100644 --- a/base-action/src/index.ts +++ b/base-action/src/index.ts @@ -22,15 +22,7 @@ async function run() { }); await runClaude(promptConfig.path, { - allowedTools: process.env.INPUT_ALLOWED_TOOLS, - disallowedTools: process.env.INPUT_DISALLOWED_TOOLS, - maxTurns: process.env.INPUT_MAX_TURNS, - mcpConfig: process.env.INPUT_MCP_CONFIG, - systemPrompt: process.env.INPUT_SYSTEM_PROMPT, - appendSystemPrompt: process.env.INPUT_APPEND_SYSTEM_PROMPT, - claudeEnv: process.env.INPUT_CLAUDE_ENV, - fallbackModel: process.env.INPUT_FALLBACK_MODEL, - model: process.env.ANTHROPIC_MODEL, + timeoutMinutes: process.env.INPUT_TIMEOUT_MINUTES, claudeArgs: process.env.INPUT_CLAUDE_ARGS, }); } catch (error) { diff --git a/base-action/src/run-claude.ts b/base-action/src/run-claude.ts index cfc4ecc..7c0c104 100644 --- a/base-action/src/run-claude.ts +++ b/base-action/src/run-claude.ts @@ -10,20 +10,10 @@ const execAsync = promisify(exec); const PIPE_PATH = `${process.env.RUNNER_TEMP}/claude_prompt_pipe`; const EXECUTION_FILE = `${process.env.RUNNER_TEMP}/claude-execution-output.json`; -// These base args are always appended at the end const BASE_ARGS = ["--verbose", "--output-format", "stream-json"]; export type ClaudeOptions = { - allowedTools?: string; - disallowedTools?: string; - maxTurns?: string; - mcpConfig?: string; - systemPrompt?: string; - appendSystemPrompt?: string; - claudeEnv?: string; - fallbackModel?: string; timeoutMinutes?: string; - model?: string; claudeArgs?: string; }; @@ -33,48 +23,19 @@ type PreparedConfig = { env: Record; }; -function parseCustomEnvVars(claudeEnv?: string): Record { - if (!claudeEnv || claudeEnv.trim() === "") { - return {}; - } - - const customEnv: Record = {}; - - // Split by lines and parse each line as KEY: VALUE - const lines = claudeEnv.split("\n"); - - for (const line of lines) { - const trimmedLine = line.trim(); - if (trimmedLine === "" || trimmedLine.startsWith("#")) { - continue; // Skip empty lines and comments - } - - const colonIndex = trimmedLine.indexOf(":"); - if (colonIndex === -1) { - continue; // Skip lines without colons - } - - const key = trimmedLine.substring(0, colonIndex).trim(); - const value = trimmedLine.substring(colonIndex + 1).trim(); - - if (key) { - customEnv[key] = value; - } - } - - return customEnv; -} - export function prepareRunConfig( promptPath: string, options: ClaudeOptions, ): PreparedConfig { - // Build arguments in correct order: - // 1. -p flag for prompt via pipe + // Build Claude CLI arguments: + // 1. Prompt flag (always first) + // 2. User's claudeArgs (full control) + // 3. BASE_ARGS (always last, cannot be overridden) + const claudeArgs = ["-p"]; - // 2. User's custom arguments (can override defaults) - if (options.claudeArgs && options.claudeArgs.trim() !== "") { + // Parse and add user's custom Claude arguments + if (options.claudeArgs?.trim()) { const parsed = parseShellArgs(options.claudeArgs); const customArgs = parsed.filter( (arg): arg is string => typeof arg === "string", @@ -82,34 +43,7 @@ export function prepareRunConfig( claudeArgs.push(...customArgs); } - // 3. Legacy specific options for backward compatibility - // These will eventually be removed in favor of claudeArgs - if (options.allowedTools) { - claudeArgs.push("--allowedTools", options.allowedTools); - } - if (options.disallowedTools) { - claudeArgs.push("--disallowedTools", options.disallowedTools); - } - if (options.maxTurns) { - claudeArgs.push("--max-turns", options.maxTurns); - } - if (options.mcpConfig) { - claudeArgs.push("--mcp-config", options.mcpConfig); - } - if (options.systemPrompt) { - claudeArgs.push("--system-prompt", options.systemPrompt); - } - if (options.appendSystemPrompt) { - claudeArgs.push("--append-system-prompt", options.appendSystemPrompt); - } - if (options.fallbackModel) { - claudeArgs.push("--fallback-model", options.fallbackModel); - } - if (options.model) { - claudeArgs.push("--model", options.model); - } - - // 4. Base args always at the end + // BASE_ARGS are always appended last (cannot be overridden) claudeArgs.push(...BASE_ARGS); // Validate timeout if provided (affects process wrapper, not Claude) @@ -122,13 +56,10 @@ export function prepareRunConfig( } } - // Parse custom environment variables - const customEnv = parseCustomEnvVars(options.claudeEnv); - return { claudeArgs, promptPath, - env: customEnv, + env: {}, }; } diff --git a/base-action/test/run-claude.test.ts b/base-action/test/run-claude.test.ts index 8e0728c..027a7c8 100644 --- a/base-action/test/run-claude.test.ts +++ b/base-action/test/run-claude.test.ts @@ -23,79 +23,6 @@ describe("prepareRunConfig", () => { expect(prepared.promptPath).toBe("/tmp/test-prompt.txt"); }); - test("should include allowed tools in command arguments", () => { - const options: ClaudeOptions = { - allowedTools: "Bash,Read", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--allowedTools"); - expect(prepared.claudeArgs).toContain("Bash,Read"); - }); - - test("should include disallowed tools in command arguments", () => { - const options: ClaudeOptions = { - disallowedTools: "Bash,Read", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--disallowedTools"); - expect(prepared.claudeArgs).toContain("Bash,Read"); - }); - - test("should include max turns in command arguments", () => { - const options: ClaudeOptions = { - maxTurns: "5", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--max-turns"); - expect(prepared.claudeArgs).toContain("5"); - }); - - test("should include mcp config in command arguments", () => { - const options: ClaudeOptions = { - mcpConfig: "/path/to/mcp-config.json", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--mcp-config"); - expect(prepared.claudeArgs).toContain("/path/to/mcp-config.json"); - }); - - test("should include system prompt in command arguments", () => { - const options: ClaudeOptions = { - systemPrompt: "You are a senior backend engineer.", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--system-prompt"); - expect(prepared.claudeArgs).toContain("You are a senior backend engineer."); - }); - - test("should include append system prompt in command arguments", () => { - const options: ClaudeOptions = { - appendSystemPrompt: - "After writing code, be sure to code review yourself.", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--append-system-prompt"); - expect(prepared.claudeArgs).toContain( - "After writing code, be sure to code review yourself.", - ); - }); - - test("should include fallback model in command arguments", () => { - const options: ClaudeOptions = { - fallbackModel: "claude-sonnet-4-20250514", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toContain("--fallback-model"); - expect(prepared.claudeArgs).toContain("claude-sonnet-4-20250514"); - }); - test("should use provided prompt path", () => { const options: ClaudeOptions = {}; const prepared = prepareRunConfig("/custom/prompt/path.txt", options); @@ -103,105 +30,6 @@ describe("prepareRunConfig", () => { expect(prepared.promptPath).toBe("/custom/prompt/path.txt"); }); - test("should not include optional arguments when not set", () => { - const options: ClaudeOptions = {}; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).not.toContain("--allowedTools"); - expect(prepared.claudeArgs).not.toContain("--disallowedTools"); - expect(prepared.claudeArgs).not.toContain("--max-turns"); - expect(prepared.claudeArgs).not.toContain("--mcp-config"); - expect(prepared.claudeArgs).not.toContain("--system-prompt"); - expect(prepared.claudeArgs).not.toContain("--append-system-prompt"); - expect(prepared.claudeArgs).not.toContain("--fallback-model"); - }); - - test("should preserve order of claude arguments", () => { - const options: ClaudeOptions = { - allowedTools: "Bash,Read", - maxTurns: "3", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toEqual([ - "-p", - "--allowedTools", - "Bash,Read", - "--max-turns", - "3", - "--verbose", - "--output-format", - "stream-json", - ]); - }); - - test("should preserve order with all options including fallback model", () => { - const options: ClaudeOptions = { - allowedTools: "Bash,Read", - disallowedTools: "Write", - maxTurns: "3", - mcpConfig: "/path/to/config.json", - systemPrompt: "You are a helpful assistant", - appendSystemPrompt: "Be concise", - fallbackModel: "claude-sonnet-4-20250514", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toEqual([ - "-p", - "--allowedTools", - "Bash,Read", - "--disallowedTools", - "Write", - "--max-turns", - "3", - "--mcp-config", - "/path/to/config.json", - "--system-prompt", - "You are a helpful assistant", - "--append-system-prompt", - "Be concise", - "--fallback-model", - "claude-sonnet-4-20250514", - "--verbose", - "--output-format", - "stream-json", - ]); - }); - - describe("maxTurns handling", () => { - test("should accept valid maxTurns value", () => { - const options: ClaudeOptions = { maxTurns: "5" }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.claudeArgs).toContain("--max-turns"); - expect(prepared.claudeArgs).toContain("5"); - }); - - test("should pass through non-numeric maxTurns without validation (v1.0)", () => { - const options: ClaudeOptions = { maxTurns: "abc" }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - // v1.0: No validation - let Claude handle it - expect(prepared.claudeArgs).toContain("--max-turns"); - expect(prepared.claudeArgs).toContain("abc"); - }); - - test("should pass through negative maxTurns without validation (v1.0)", () => { - const options: ClaudeOptions = { maxTurns: "-1" }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - // v1.0: No validation - let Claude handle it - expect(prepared.claudeArgs).toContain("--max-turns"); - expect(prepared.claudeArgs).toContain("-1"); - }); - - test("should pass through zero maxTurns without validation (v1.0)", () => { - const options: ClaudeOptions = { maxTurns: "0" }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - // v1.0: No validation - let Claude handle it - expect(prepared.claudeArgs).toContain("--max-turns"); - expect(prepared.claudeArgs).toContain("0"); - }); - }); - describe("timeoutMinutes validation", () => { test("should accept valid timeoutMinutes value", () => { const options: ClaudeOptions = { timeoutMinutes: "15" }; @@ -251,25 +79,6 @@ describe("prepareRunConfig", () => { ]); }); - test("should handle claudeArgs with legacy options", () => { - const options: ClaudeOptions = { - claudeArgs: "--max-turns 10", - allowedTools: "Bash,Read", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - - expect(prepared.claudeArgs).toEqual([ - "-p", - "--max-turns", - "10", - "--allowedTools", - "Bash,Read", - "--verbose", - "--output-format", - "stream-json", - ]); - }); - test("should handle empty claudeArgs", () => { const options: ClaudeOptions = { claudeArgs: "", @@ -300,70 +109,4 @@ describe("prepareRunConfig", () => { ]); }); }); - - describe("custom environment variables", () => { - test("should parse empty claudeEnv correctly", () => { - const options: ClaudeOptions = { claudeEnv: "" }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({}); - }); - - test("should parse single environment variable", () => { - const options: ClaudeOptions = { claudeEnv: "API_KEY: secret123" }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({ API_KEY: "secret123" }); - }); - - test("should parse multiple environment variables", () => { - const options: ClaudeOptions = { - claudeEnv: "API_KEY: secret123\nDEBUG: true\nUSER: testuser", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({ - API_KEY: "secret123", - DEBUG: "true", - USER: "testuser", - }); - }); - - test("should handle environment variables with spaces around values", () => { - const options: ClaudeOptions = { - claudeEnv: "API_KEY: secret123 \n DEBUG : true ", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({ - API_KEY: "secret123", - DEBUG: "true", - }); - }); - - test("should skip empty lines and comments", () => { - const options: ClaudeOptions = { - claudeEnv: - "API_KEY: secret123\n\n# This is a comment\nDEBUG: true\n# Another comment", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({ - API_KEY: "secret123", - DEBUG: "true", - }); - }); - - test("should skip lines without colons", () => { - const options: ClaudeOptions = { - claudeEnv: "API_KEY: secret123\nINVALID_LINE\nDEBUG: true", - }; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({ - API_KEY: "secret123", - DEBUG: "true", - }); - }); - - test("should handle undefined claudeEnv", () => { - const options: ClaudeOptions = {}; - const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - expect(prepared.env).toEqual({}); - }); - }); }); diff --git a/claude-args-example.yml b/examples/claude-args-example.yml similarity index 84% rename from claude-args-example.yml rename to examples/claude-args-example.yml index 0dc9cf2..c2ce39e 100644 --- a/claude-args-example.yml +++ b/examples/claude-args-example.yml @@ -22,7 +22,8 @@ jobs: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} # New claudeArgs input allows direct CLI argument control - # Arguments are passed in the order: -p [claudeArgs] [legacy options] [BASE_ARGS] + # Order: -p [claudeArgs] [legacy options] [BASE_ARGS] + # Note: BASE_ARGS (--verbose --output-format stream-json) cannot be overridden claude_args: | --max-turns 15 --model claude-3-opus-20240229 diff --git a/examples/claude-auto-review.yml b/examples/claude-auto-review.yml index 85d3262..3d5399e 100644 --- a/examples/claude-auto-review.yml +++ b/examples/claude-auto-review.yml @@ -18,11 +18,11 @@ jobs: fetch-depth: 1 - name: Automatic PR Review - uses: anthropics/claude-code-action@beta + uses: anthropics/claude-code-action@v1 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} timeout_minutes: "60" - direct_prompt: | + prompt: | Please review this pull request and provide comprehensive feedback. Focus on: diff --git a/examples/claude-experimental-review-mode.yml b/examples/claude-experimental-review-mode.yml index e36597f..8edf9db 100644 --- a/examples/claude-experimental-review-mode.yml +++ b/examples/claude-experimental-review-mode.yml @@ -27,13 +27,14 @@ jobs: fetch-depth: 0 # Full history for better diff analysis - name: Code Review with Claude - uses: anthropics/claude-code-action@beta + uses: anthropics/claude-code-action@v1 with: - mode: experimental-review anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} # github_token not needed - uses default GITHUB_TOKEN for GitHub operations timeout_minutes: "30" - custom_instructions: | + prompt: | + Review this pull request comprehensively. + Focus on: - Code quality and maintainability - Security vulnerabilities diff --git a/examples/claude-pr-path-specific.yml b/examples/claude-pr-path-specific.yml index cea2695..7305f90 100644 --- a/examples/claude-pr-path-specific.yml +++ b/examples/claude-pr-path-specific.yml @@ -24,11 +24,11 @@ jobs: fetch-depth: 1 - name: Claude Code Review - uses: anthropics/claude-code-action@beta + uses: anthropics/claude-code-action@v1 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} timeout_minutes: "60" - direct_prompt: | + prompt: | Please review this pull request focusing on the changed files. Provide feedback on: - Code quality and adherence to best practices diff --git a/examples/claude-review-from-author.yml b/examples/claude-review-from-author.yml index 76219d8..7018253 100644 --- a/examples/claude-review-from-author.yml +++ b/examples/claude-review-from-author.yml @@ -23,11 +23,11 @@ jobs: fetch-depth: 1 - name: Review PR from Specific Author - uses: anthropics/claude-code-action@beta + uses: anthropics/claude-code-action@v1 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} timeout_minutes: "60" - direct_prompt: | + prompt: | Please provide a thorough review of this pull request. Since this is from a specific author that requires careful review, diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 8f824f1..fcff64d 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -117,9 +117,6 @@ export function prepareContext( const triggerPhrase = context.inputs.triggerPhrase || "@claude"; const assigneeTrigger = context.inputs.assigneeTrigger; const labelTrigger = context.inputs.labelTrigger; - const customInstructions = context.inputs.customInstructions; - const allowedTools = context.inputs.allowedTools; - const disallowedTools = context.inputs.disallowedTools; const prompt = context.inputs.prompt; const isPR = context.isPR; @@ -153,11 +150,6 @@ export function prepareContext( claudeCommentId, triggerPhrase, ...(triggerUsername && { triggerUsername }), - ...(customInstructions && { customInstructions }), - ...(allowedTools.length > 0 && { allowedTools: allowedTools.join(",") }), - ...(disallowedTools.length > 0 && { - disallowedTools: disallowedTools.join(","), - }), ...(prompt && { prompt }), ...(claudeBranch && { claudeBranch }), }; @@ -730,10 +722,6 @@ e. Propose a high-level plan of action, including any repo setup steps and linti f. If you are unable to complete certain steps, such as running a linter or test suite, particularly due to missing permissions, explain this in your comment so that the user can update your \`--allowedTools\`. `; - if (context.customInstructions) { - promptContent += `\n\nCUSTOM INSTRUCTIONS:\n${context.customInstructions}`; - } - return promptContent; } @@ -786,32 +774,20 @@ export async function createPrompt( ); // Set allowed tools - const hasActionsReadPermission = - context.inputs.additionalPermissions.get("actions") === "read" && - context.isPR; + const hasActionsReadPermission = false; // Get mode-specific tools const modeAllowedTools = mode.getAllowedTools(); const modeDisallowedTools = mode.getDisallowedTools(); - // Combine with existing allowed tools - const combinedAllowedTools = [ - ...context.inputs.allowedTools, - ...modeAllowedTools, - ]; - const combinedDisallowedTools = [ - ...context.inputs.disallowedTools, - ...modeDisallowedTools, - ]; - const allAllowedTools = buildAllowedToolsString( - combinedAllowedTools, + modeAllowedTools, hasActionsReadPermission, context.inputs.useCommitSigning, ); const allDisallowedTools = buildDisallowedToolsString( - combinedDisallowedTools, - combinedAllowedTools, + modeDisallowedTools, + modeAllowedTools, ); core.exportVariable("ALLOWED_TOOLS", allAllowedTools); diff --git a/src/create-prompt/types.ts b/src/create-prompt/types.ts index ae83c32..6f60b85 100644 --- a/src/create-prompt/types.ts +++ b/src/create-prompt/types.ts @@ -3,10 +3,8 @@ export type CommonFields = { claudeCommentId: string; triggerPhrase: string; triggerUsername?: string; - customInstructions?: string; - allowedTools?: string; - disallowedTools?: string; prompt?: string; + claudeBranch?: string; }; type PullRequestReviewCommentEvent = { diff --git a/src/github/context.ts b/src/github/context.ts index 908f6cd..4ea8db5 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -65,13 +65,9 @@ type BaseContext = { triggerPhrase: string; assigneeTrigger: string; labelTrigger: string; - allowedTools: string[]; - disallowedTools: string[]; - customInstructions: string; baseBranch?: string; branchPrefix: string; useStickyComment: boolean; - additionalPermissions: Map; useCommitSigning: boolean; }; }; @@ -115,15 +111,9 @@ export function parseGitHubContext(): GitHubContext { triggerPhrase: process.env.TRIGGER_PHRASE ?? "@claude", assigneeTrigger: process.env.ASSIGNEE_TRIGGER ?? "", labelTrigger: process.env.LABEL_TRIGGER ?? "", - allowedTools: parseMultilineInput(process.env.ALLOWED_TOOLS ?? ""), - disallowedTools: parseMultilineInput(process.env.DISALLOWED_TOOLS ?? ""), - customInstructions: process.env.CUSTOM_INSTRUCTIONS ?? "", baseBranch: process.env.BASE_BRANCH, branchPrefix: process.env.BRANCH_PREFIX ?? "claude/", useStickyComment: process.env.USE_STICKY_COMMENT === "true", - additionalPermissions: parseAdditionalPermissions( - process.env.ADDITIONAL_PERMISSIONS ?? "", - ), useCommitSigning: process.env.USE_COMMIT_SIGNING === "true", }, }; @@ -198,33 +188,6 @@ export function parseGitHubContext(): GitHubContext { } } -export function parseMultilineInput(s: string): string[] { - return s - .split(/,|[\n\r]+/) - .map((tool) => tool.replace(/#.+$/, "")) - .map((tool) => tool.trim()) - .filter((tool) => tool.length > 0); -} - -export function parseAdditionalPermissions(s: string): Map { - const permissions = new Map(); - if (!s || !s.trim()) { - return permissions; - } - - const lines = s.trim().split("\n"); - for (const line of lines) { - const trimmedLine = line.trim(); - if (trimmedLine) { - const [key, value] = trimmedLine.split(":").map((part) => part.trim()); - if (key && value) { - permissions.set(key, value); - } - } - } - return permissions; -} - export function isIssuesEvent( context: GitHubContext, ): context is ParsedGitHubContext & { payload: IssuesEvent } { diff --git a/src/mcp/install-mcp-server.ts b/src/mcp/install-mcp-server.ts index 61b11d6..18407b6 100644 --- a/src/mcp/install-mcp-server.ts +++ b/src/mcp/install-mcp-server.ts @@ -111,9 +111,8 @@ export async function prepareMcpConfig( }; } - // Only add CI server if we have actions:read permission and we're in a PR context - const hasActionsReadPermission = - context.inputs.additionalPermissions.get("actions") === "read"; + // CI server is disabled by default in v1.0 (users can enable via claudeArgs) + const hasActionsReadPermission = false; if (context.isPR && hasActionsReadPermission) { // Verify the token actually has actions:read permission diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index fa25a4d..7a0bc06 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -58,21 +58,8 @@ export const agentMode: Mode = { promptContent, ); - // Agent mode: User has full control via claudeArgs or legacy inputs + // Agent mode: User has full control via claudeArgs // No default tools are enforced - Claude Code's defaults will apply - // Export user-specified tools only if provided - if (context.inputs.allowedTools.length > 0) { - core.exportVariable( - "INPUT_ALLOWED_TOOLS", - context.inputs.allowedTools.join(","), - ); - } - if (context.inputs.disallowedTools.length > 0) { - core.exportVariable( - "INPUT_DISALLOWED_TOOLS", - context.inputs.disallowedTools.join(","), - ); - } // Agent mode uses a minimal MCP configuration // We don't need comment servers or PR-specific tools for automation diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index f9aabaf..73e274e 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -110,7 +110,7 @@ export const tagMode: Mode = { baseBranch: branchInfo.baseBranch, additionalMcpConfig, claudeCommentId: commentId.toString(), - allowedTools: context.inputs.allowedTools, + allowedTools: [], context, }); diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index d90b0e7..32114cb 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -330,12 +330,11 @@ describe("generatePrompt", () => { expect(prompt).toContain("pull request opened"); }); - test("should include custom instructions when provided", async () => { + test("should generate prompt for issue comment without custom fields", async () => { const envVars: PreparedContext = { repository: "owner/repo", claudeCommentId: "12345", triggerPhrase: "@claude", - customInstructions: "Always use TypeScript", eventData: { eventName: "issue_comment", commentId: "67890", @@ -354,7 +353,9 @@ describe("generatePrompt", () => { mockTagMode, ); - expect(prompt).toContain("CUSTOM INSTRUCTIONS:\nAlways use TypeScript"); + // Verify prompt generates successfully without custom instructions + expect(prompt).toContain("@claude please fix this"); + expect(prompt).not.toContain("CUSTOM INSTRUCTIONS"); }); test("should use override_prompt when provided", async () => { diff --git a/test/github/context.test.ts b/test/github/context.test.ts deleted file mode 100644 index a2b587e..0000000 --- a/test/github/context.test.ts +++ /dev/null @@ -1,115 +0,0 @@ -import { describe, it, expect } from "bun:test"; -import { - parseMultilineInput, - parseAdditionalPermissions, -} from "../../src/github/context"; - -describe("parseMultilineInput", () => { - it("should parse a comma-separated string", () => { - const input = `Bash(bun install),Bash(bun test:*),Bash(bun typecheck)`; - const result = parseMultilineInput(input); - expect(result).toEqual([ - "Bash(bun install)", - "Bash(bun test:*)", - "Bash(bun typecheck)", - ]); - }); - - it("should parse multiline string", () => { - const input = `Bash(bun install) -Bash(bun test:*) -Bash(bun typecheck)`; - const result = parseMultilineInput(input); - expect(result).toEqual([ - "Bash(bun install)", - "Bash(bun test:*)", - "Bash(bun typecheck)", - ]); - }); - - it("should parse comma-separated multiline line", () => { - const input = `Bash(bun install),Bash(bun test:*) -Bash(bun typecheck)`; - const result = parseMultilineInput(input); - expect(result).toEqual([ - "Bash(bun install)", - "Bash(bun test:*)", - "Bash(bun typecheck)", - ]); - }); - - it("should ignore comments", () => { - const input = `Bash(bun install), -Bash(bun test:*) # For testing -# For type checking -Bash(bun typecheck) -`; - const result = parseMultilineInput(input); - expect(result).toEqual([ - "Bash(bun install)", - "Bash(bun test:*)", - "Bash(bun typecheck)", - ]); - }); - - it("should parse an empty string", () => { - const input = ""; - const result = parseMultilineInput(input); - expect(result).toEqual([]); - }); -}); - -describe("parseAdditionalPermissions", () => { - it("should parse single permission", () => { - const input = "actions: read"; - const result = parseAdditionalPermissions(input); - expect(result.get("actions")).toBe("read"); - expect(result.size).toBe(1); - }); - - it("should parse multiple permissions", () => { - const input = `actions: read -packages: write -contents: read`; - const result = parseAdditionalPermissions(input); - expect(result.get("actions")).toBe("read"); - expect(result.get("packages")).toBe("write"); - expect(result.get("contents")).toBe("read"); - expect(result.size).toBe(3); - }); - - it("should handle empty string", () => { - const input = ""; - const result = parseAdditionalPermissions(input); - expect(result.size).toBe(0); - }); - - it("should handle whitespace and empty lines", () => { - const input = ` - actions: read - - packages: write - `; - const result = parseAdditionalPermissions(input); - expect(result.get("actions")).toBe("read"); - expect(result.get("packages")).toBe("write"); - expect(result.size).toBe(2); - }); - - it("should ignore lines without colon separator", () => { - const input = `actions: read -invalid line -packages: write`; - const result = parseAdditionalPermissions(input); - expect(result.get("actions")).toBe("read"); - expect(result.get("packages")).toBe("write"); - expect(result.size).toBe(2); - }); - - it("should trim whitespace around keys and values", () => { - const input = " actions : read "; - const result = parseAdditionalPermissions(input); - expect(result.get("actions")).toBe("read"); - expect(result.size).toBe(1); - }); -}); diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts index 221685d..a8b6cd4 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -28,12 +28,8 @@ describe("prepareMcpConfig", () => { triggerPhrase: "@claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, }; @@ -552,7 +548,6 @@ describe("prepareMcpConfig", () => { ...mockPRContext, inputs: { ...mockPRContext.inputs, - additionalPermissions: new Map([["actions", "read"]]), useCommitSigning: true, }, }; @@ -621,10 +616,6 @@ describe("prepareMcpConfig", () => { ...mockPRContext, inputs: { ...mockPRContext.inputs, - additionalPermissions: new Map([ - ["actions", "read"], - ["future", "permission"], - ]), }, }; @@ -653,7 +644,6 @@ describe("prepareMcpConfig", () => { ...mockPRContext, inputs: { ...mockPRContext.inputs, - additionalPermissions: new Map([["actions", "read"]]), }, }; diff --git a/test/mockContext.ts b/test/mockContext.ts index d8f8ebe..098c66d 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -15,16 +15,8 @@ const defaultInputs = { triggerPhrase: "/claude", assigneeTrigger: "", labelTrigger: "", - anthropicModel: "claude-3-7-sonnet-20250219", - allowedTools: [] as string[], - disallowedTools: [] as string[], - customInstructions: "", - useBedrock: false, - useVertex: false, - timeoutMinutes: 30, branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }; diff --git a/test/permissions.test.ts b/test/permissions.test.ts index 77d4e95..4f2f9d0 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -64,12 +64,8 @@ describe("checkWritePermissions", () => { triggerPhrase: "@claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, }); diff --git a/test/prepare-context.test.ts b/test/prepare-context.test.ts index 0ea37dd..dbfbaab 100644 --- a/test/prepare-context.test.ts +++ b/test/prepare-context.test.ts @@ -270,33 +270,23 @@ describe("parseEnvVarsWithContext", () => { }); }); - describe("optional fields", () => { - test("should include custom instructions when provided", () => { + describe("context generation", () => { + test("should generate context without legacy fields", () => { process.env = BASE_ENV; - const contextWithCustomInstructions = createMockContext({ + const context = createMockContext({ ...mockPullRequestCommentContext, inputs: { ...mockPullRequestCommentContext.inputs, - customInstructions: "Be concise", }, }); - const result = prepareContext(contextWithCustomInstructions, "12345"); + const result = prepareContext(context, "12345"); - expect(result.customInstructions).toBe("Be concise"); - }); - - test("should include allowed tools when provided", () => { - process.env = BASE_ENV; - const contextWithAllowedTools = createMockContext({ - ...mockPullRequestCommentContext, - inputs: { - ...mockPullRequestCommentContext.inputs, - allowedTools: ["Tool1", "Tool2"], - }, - }); - const result = prepareContext(contextWithAllowedTools, "12345"); - - expect(result.allowedTools).toBe("Tool1,Tool2"); + // Verify context is created without legacy fields + expect(result.repository).toBe("test-owner/test-repo"); + expect(result.claudeCommentId).toBe("12345"); + expect(result.triggerPhrase).toBe("/claude"); + expect((result as any).customInstructions).toBeUndefined(); + expect((result as any).allowedTools).toBeUndefined(); }); }); }); diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index bd1ff83..3bf00b2 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -32,12 +32,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "/claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, }); @@ -63,12 +59,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "/claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, }); @@ -278,12 +270,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "@claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, }); @@ -310,12 +298,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "@claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, }); @@ -342,12 +326,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "@claude", assigneeTrigger: "", labelTrigger: "", - allowedTools: [], - disallowedTools: [], - customInstructions: "", branchPrefix: "claude/", useStickyComment: false, - additionalPermissions: new Map(), useCommitSigning: false, }, });