From 05c95aed7977cd1f47dce86cb5b617c56d8fd529 Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Thu, 4 Dec 2025 10:25:54 -0800 Subject: [PATCH] fix: accumulate multiple --allowedTools flags for Agent SDK (#719) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: merge allowedTools from claudeArgs when using Agent SDK When USE_AGENT_SDK=true, the allowedTools from claudeArgs (which contains tag mode's required tools like mcp__github_comment__update_claude_comment) were being lost because parseClaudeArgsToExtraArgs converts args to a Record, and the SDK was using sdkOptions.allowedTools (from direct options) instead of merging with extraArgs.allowedTools. This fix: - Extracts allowedTools/disallowedTools from extraArgs after parsing - Merges them with any direct options.allowedTools/disallowedTools - Removes them from extraArgs to prevent duplicate CLI flags - Passes the merged list as sdkOptions.allowedTools 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix: accumulate multiple --allowedTools flags in claudeArgs When tag mode adds its --allowedTools (with MCP tools) and the user also provides --allowedTools in their claude_args, the parseClaudeArgsToExtraArgs function was only keeping the last value. This caused tag mode's required tools like mcp__github_comment__update_claude_comment to be lost. Now allowedTools and disallowedTools flags accumulate their values when they appear multiple times in claudeArgs, so both tag mode's tools and user's tools are preserved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- base-action/src/parse-sdk-options.ts | 51 ++++++- base-action/test/parse-sdk-options.test.ts | 163 +++++++++++++++++++++ 2 files changed, 206 insertions(+), 8 deletions(-) create mode 100644 base-action/test/parse-sdk-options.test.ts diff --git a/base-action/src/parse-sdk-options.ts b/base-action/src/parse-sdk-options.ts index cc07366..f101383 100644 --- a/base-action/src/parse-sdk-options.ts +++ b/base-action/src/parse-sdk-options.ts @@ -11,9 +11,13 @@ export type ParsedSdkOptions = { hasJsonSchema: boolean; }; +// Flags that should accumulate multiple values instead of overwriting +const ACCUMULATING_FLAGS = new Set(["allowedTools", "disallowedTools"]); + /** * Parse claudeArgs string into extraArgs record for SDK pass-through * The SDK/CLI will handle --mcp-config, --json-schema, etc. + * For allowedTools and disallowedTools, multiple occurrences are accumulated (comma-joined). */ function parseClaudeArgsToExtraArgs( claudeArgs?: string, @@ -33,7 +37,12 @@ function parseClaudeArgsToExtraArgs( // Check if next arg is a value (not another flag) if (nextArg && !nextArg.startsWith("--")) { - result[flag] = nextArg; + // For accumulating flags, join multiple values with commas + if (ACCUMULATING_FLAGS.has(flag) && result[flag]) { + result[flag] = `${result[flag]},${nextArg}`; + } else { + result[flag] = nextArg; + } i++; // Skip the value } else { result[flag] = null; // Boolean flag @@ -59,6 +68,33 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions { // Detect if --json-schema is present (for hasJsonSchema flag) const hasJsonSchema = "json-schema" in extraArgs; + // Extract and merge allowedTools from both sources: + // 1. From extraArgs (parsed from claudeArgs - contains tag mode's tools) + // 2. From options.allowedTools (direct input - may be undefined) + // This prevents duplicate flags being overwritten when claudeArgs contains --allowedTools + const extraArgsAllowedTools = extraArgs["allowedTools"] + ? extraArgs["allowedTools"].split(",").map((t) => t.trim()) + : []; + const directAllowedTools = options.allowedTools + ? options.allowedTools.split(",").map((t) => t.trim()) + : []; + const mergedAllowedTools = [ + ...new Set([...extraArgsAllowedTools, ...directAllowedTools]), + ]; + delete extraArgs["allowedTools"]; + + // Same for disallowedTools + const extraArgsDisallowedTools = extraArgs["disallowedTools"] + ? extraArgs["disallowedTools"].split(",").map((t) => t.trim()) + : []; + const directDisallowedTools = options.disallowedTools + ? options.disallowedTools.split(",").map((t) => t.trim()) + : []; + const mergedDisallowedTools = [ + ...new Set([...extraArgsDisallowedTools, ...directDisallowedTools]), + ]; + delete extraArgs["disallowedTools"]; + // Build custom environment const env: Record = { ...process.env }; if (process.env.INPUT_ACTION_INPUTS_PRESENT) { @@ -77,22 +113,21 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions { }; } - // Build SDK options - use direct options for explicit inputs, extraArgs for claudeArgs pass-through + // Build SDK options - use merged tools from both direct options and claudeArgs const sdkOptions: SdkOptions = { // Direct options from ClaudeOptions inputs model: options.model, maxTurns: options.maxTurns ? parseInt(options.maxTurns, 10) : undefined, - allowedTools: options.allowedTools - ? options.allowedTools.split(",").map((t) => t.trim()) - : undefined, - disallowedTools: options.disallowedTools - ? options.disallowedTools.split(",").map((t) => t.trim()) - : undefined, + allowedTools: + mergedAllowedTools.length > 0 ? mergedAllowedTools : undefined, + disallowedTools: + mergedDisallowedTools.length > 0 ? mergedDisallowedTools : undefined, systemPrompt, fallbackModel: options.fallbackModel, pathToClaudeCodeExecutable: options.pathToClaudeCodeExecutable, // Pass through claudeArgs as extraArgs - CLI handles --mcp-config, --json-schema, etc. + // Note: allowedTools and disallowedTools have been removed from extraArgs to prevent duplicates extraArgs, env, }; diff --git a/base-action/test/parse-sdk-options.test.ts b/base-action/test/parse-sdk-options.test.ts new file mode 100644 index 0000000..0174c41 --- /dev/null +++ b/base-action/test/parse-sdk-options.test.ts @@ -0,0 +1,163 @@ +#!/usr/bin/env bun + +import { describe, test, expect } from "bun:test"; +import { parseSdkOptions } from "../src/parse-sdk-options"; +import type { ClaudeOptions } from "../src/run-claude"; + +describe("parseSdkOptions", () => { + describe("allowedTools merging", () => { + test("should extract allowedTools from claudeArgs", () => { + const options: ClaudeOptions = { + claudeArgs: '--allowedTools "Edit,Read,Write"', + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toEqual(["Edit", "Read", "Write"]); + expect(result.sdkOptions.extraArgs?.["allowedTools"]).toBeUndefined(); + }); + + test("should extract allowedTools from claudeArgs with MCP tools", () => { + const options: ClaudeOptions = { + claudeArgs: + '--allowedTools "Edit,Read,mcp__github_comment__update_claude_comment"', + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toEqual([ + "Edit", + "Read", + "mcp__github_comment__update_claude_comment", + ]); + }); + + test("should accumulate multiple --allowedTools flags from claudeArgs", () => { + // This simulates tag mode adding its tools, then user adding their own + const options: ClaudeOptions = { + claudeArgs: + '--allowedTools "Edit,Read,mcp__github_comment__update_claude_comment" --model "claude-3" --allowedTools "Bash(npm install),mcp__github__get_issue"', + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toEqual([ + "Edit", + "Read", + "mcp__github_comment__update_claude_comment", + "Bash(npm install)", + "mcp__github__get_issue", + ]); + }); + + test("should merge allowedTools from both claudeArgs and direct options", () => { + const options: ClaudeOptions = { + claudeArgs: '--allowedTools "Edit,Read"', + allowedTools: "Write,Glob", + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toEqual([ + "Edit", + "Read", + "Write", + "Glob", + ]); + }); + + test("should deduplicate allowedTools when merging", () => { + const options: ClaudeOptions = { + claudeArgs: '--allowedTools "Edit,Read"', + allowedTools: "Edit,Write", + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toEqual(["Edit", "Read", "Write"]); + }); + + test("should use only direct options when claudeArgs has no allowedTools", () => { + const options: ClaudeOptions = { + claudeArgs: '--model "claude-3-5-sonnet"', + allowedTools: "Edit,Read", + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toEqual(["Edit", "Read"]); + }); + + test("should return undefined allowedTools when neither source has it", () => { + const options: ClaudeOptions = { + claudeArgs: '--model "claude-3-5-sonnet"', + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.allowedTools).toBeUndefined(); + }); + + test("should remove allowedTools from extraArgs after extraction", () => { + const options: ClaudeOptions = { + claudeArgs: '--allowedTools "Edit,Read" --model "claude-3-5-sonnet"', + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.extraArgs?.["allowedTools"]).toBeUndefined(); + expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet"); + }); + }); + + describe("disallowedTools merging", () => { + test("should extract disallowedTools from claudeArgs", () => { + const options: ClaudeOptions = { + claudeArgs: '--disallowedTools "Bash,Write"', + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.disallowedTools).toEqual(["Bash", "Write"]); + expect(result.sdkOptions.extraArgs?.["disallowedTools"]).toBeUndefined(); + }); + + test("should merge disallowedTools from both sources", () => { + const options: ClaudeOptions = { + claudeArgs: '--disallowedTools "Bash"', + disallowedTools: "Write", + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.disallowedTools).toEqual(["Bash", "Write"]); + }); + }); + + describe("other extraArgs passthrough", () => { + test("should pass through mcp-config in extraArgs", () => { + const options: ClaudeOptions = { + claudeArgs: `--mcp-config '{"mcpServers":{}}' --allowedTools "Edit"`, + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.extraArgs?.["mcp-config"]).toBe( + '{"mcpServers":{}}', + ); + }); + + test("should pass through json-schema in extraArgs", () => { + const options: ClaudeOptions = { + claudeArgs: `--json-schema '{"type":"object"}'`, + }; + + const result = parseSdkOptions(options); + + expect(result.sdkOptions.extraArgs?.["json-schema"]).toBe( + '{"type":"object"}', + ); + expect(result.hasJsonSchema).toBe(true); + }); + }); +});