mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-01-22 22:44:13 +08:00
fix: accumulate multiple --allowedTools flags for Agent SDK (#719)
* 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<string, string>, 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -11,9 +11,13 @@ export type ParsedSdkOptions = {
|
|||||||
hasJsonSchema: boolean;
|
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
|
* Parse claudeArgs string into extraArgs record for SDK pass-through
|
||||||
* The SDK/CLI will handle --mcp-config, --json-schema, etc.
|
* The SDK/CLI will handle --mcp-config, --json-schema, etc.
|
||||||
|
* For allowedTools and disallowedTools, multiple occurrences are accumulated (comma-joined).
|
||||||
*/
|
*/
|
||||||
function parseClaudeArgsToExtraArgs(
|
function parseClaudeArgsToExtraArgs(
|
||||||
claudeArgs?: string,
|
claudeArgs?: string,
|
||||||
@@ -33,7 +37,12 @@ function parseClaudeArgsToExtraArgs(
|
|||||||
|
|
||||||
// Check if next arg is a value (not another flag)
|
// Check if next arg is a value (not another flag)
|
||||||
if (nextArg && !nextArg.startsWith("--")) {
|
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
|
i++; // Skip the value
|
||||||
} else {
|
} else {
|
||||||
result[flag] = null; // Boolean flag
|
result[flag] = null; // Boolean flag
|
||||||
@@ -59,6 +68,33 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
|
|||||||
// Detect if --json-schema is present (for hasJsonSchema flag)
|
// Detect if --json-schema is present (for hasJsonSchema flag)
|
||||||
const hasJsonSchema = "json-schema" in extraArgs;
|
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
|
// Build custom environment
|
||||||
const env: Record<string, string | undefined> = { ...process.env };
|
const env: Record<string, string | undefined> = { ...process.env };
|
||||||
if (process.env.INPUT_ACTION_INPUTS_PRESENT) {
|
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 = {
|
const sdkOptions: SdkOptions = {
|
||||||
// Direct options from ClaudeOptions inputs
|
// Direct options from ClaudeOptions inputs
|
||||||
model: options.model,
|
model: options.model,
|
||||||
maxTurns: options.maxTurns ? parseInt(options.maxTurns, 10) : undefined,
|
maxTurns: options.maxTurns ? parseInt(options.maxTurns, 10) : undefined,
|
||||||
allowedTools: options.allowedTools
|
allowedTools:
|
||||||
? options.allowedTools.split(",").map((t) => t.trim())
|
mergedAllowedTools.length > 0 ? mergedAllowedTools : undefined,
|
||||||
: undefined,
|
disallowedTools:
|
||||||
disallowedTools: options.disallowedTools
|
mergedDisallowedTools.length > 0 ? mergedDisallowedTools : undefined,
|
||||||
? options.disallowedTools.split(",").map((t) => t.trim())
|
|
||||||
: undefined,
|
|
||||||
systemPrompt,
|
systemPrompt,
|
||||||
fallbackModel: options.fallbackModel,
|
fallbackModel: options.fallbackModel,
|
||||||
pathToClaudeCodeExecutable: options.pathToClaudeCodeExecutable,
|
pathToClaudeCodeExecutable: options.pathToClaudeCodeExecutable,
|
||||||
|
|
||||||
// Pass through claudeArgs as extraArgs - CLI handles --mcp-config, --json-schema, etc.
|
// 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,
|
extraArgs,
|
||||||
env,
|
env,
|
||||||
};
|
};
|
||||||
|
|||||||
163
base-action/test/parse-sdk-options.test.ts
Normal file
163
base-action/test/parse-sdk-options.test.ts
Normal file
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user