mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-01-22 14:24:13 +08:00
fix: merge multiple --mcp-config flags and support --allowed-tools parsing (#748)
* fix: merge multiple --mcp-config flags instead of overwriting When users provide their own --mcp-config in claude_args, the action's built-in MCP servers (github_comment, github_ci, etc.) were being lost because multiple --mcp-config flags were overwriting each other. This fix: - Adds mcp-config to ACCUMULATING_FLAGS to collect all values - Changes delimiter to null character to avoid conflicts with JSON - Adds mergeMcpConfigs() to combine mcpServers objects from multiple configs - Merges inline JSON configs while preserving file path configs Fixes #745 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Ashwin Bhat <ashwin-ant@users.noreply.github.com> * fix: support hyphenated --allowed-tools flag and multiple values The --allowed-tools flag was not being parsed correctly when: 1. Using the hyphenated form (--allowed-tools) instead of camelCase (--allowedTools) 2. Passing multiple space-separated values after a single flag (e.g., --allowed-tools "Tool1" "Tool2" "Tool3") This fix: - Adds hyphenated variants (allowed-tools, disallowed-tools) to ACCUMULATING_FLAGS - Updates parsing to consume all consecutive non-flag values for accumulating flags - Merges values from both camelCase and hyphenated variants Fixes #746 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Ashwin Bhat <ashwin-ant@users.noreply.github.com>
This commit is contained in:
Ashwin Bhat
@@ -12,12 +12,79 @@ export type ParsedSdkOptions = {
|
||||
};
|
||||
|
||||
// Flags that should accumulate multiple values instead of overwriting
|
||||
const ACCUMULATING_FLAGS = new Set(["allowedTools", "disallowedTools"]);
|
||||
// Include both camelCase and hyphenated variants for CLI compatibility
|
||||
const ACCUMULATING_FLAGS = new Set([
|
||||
"allowedTools",
|
||||
"allowed-tools",
|
||||
"disallowedTools",
|
||||
"disallowed-tools",
|
||||
"mcp-config",
|
||||
]);
|
||||
|
||||
// Delimiter used to join accumulated flag values
|
||||
const ACCUMULATE_DELIMITER = "\x00";
|
||||
|
||||
type McpConfig = {
|
||||
mcpServers?: Record<string, unknown>;
|
||||
};
|
||||
|
||||
/**
|
||||
* Merge multiple MCP config values into a single config.
|
||||
* Each config can be a JSON string or a file path.
|
||||
* For JSON strings, mcpServers objects are merged.
|
||||
* For file paths, they are kept as-is (user's file takes precedence and is used last).
|
||||
*/
|
||||
function mergeMcpConfigs(configValues: string[]): string {
|
||||
const merged: McpConfig = { mcpServers: {} };
|
||||
let lastFilePath: string | null = null;
|
||||
|
||||
for (const config of configValues) {
|
||||
const trimmed = config.trim();
|
||||
if (!trimmed) continue;
|
||||
|
||||
// Check if it's a JSON string (starts with {) or a file path
|
||||
if (trimmed.startsWith("{")) {
|
||||
try {
|
||||
const parsed = JSON.parse(trimmed) as McpConfig;
|
||||
if (parsed.mcpServers) {
|
||||
Object.assign(merged.mcpServers!, parsed.mcpServers);
|
||||
}
|
||||
} catch {
|
||||
// If JSON parsing fails, treat as file path
|
||||
lastFilePath = trimmed;
|
||||
}
|
||||
} else {
|
||||
// It's a file path - store it to handle separately
|
||||
lastFilePath = trimmed;
|
||||
}
|
||||
}
|
||||
|
||||
// If we have file paths, we need to keep the merged JSON and let the file
|
||||
// be handled separately. Since we can only return one value, merge what we can.
|
||||
// If there's a file path, we need a different approach - read the file at runtime.
|
||||
// For now, if there's a file path, we'll stringify the merged config.
|
||||
// The action prepends its config as JSON, so we can safely merge inline JSON configs.
|
||||
|
||||
// If no inline configs were found (all file paths), return the last file path
|
||||
if (Object.keys(merged.mcpServers!).length === 0 && lastFilePath) {
|
||||
return lastFilePath;
|
||||
}
|
||||
|
||||
// Note: If user passes a file path, we cannot merge it at parse time since
|
||||
// we don't have access to the file system here. The action's built-in MCP
|
||||
// servers are always passed as inline JSON, so they will be merged.
|
||||
// If user also passes inline JSON, it will be merged.
|
||||
// If user passes a file path, they should ensure it includes all needed servers.
|
||||
|
||||
return JSON.stringify(merged);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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).
|
||||
* For allowedTools and disallowedTools, multiple occurrences are accumulated (null-char joined).
|
||||
* Accumulating flags also consume all consecutive non-flag values
|
||||
* (e.g., --allowed-tools "Tool1" "Tool2" "Tool3" captures all three).
|
||||
*/
|
||||
function parseClaudeArgsToExtraArgs(
|
||||
claudeArgs?: string,
|
||||
@@ -37,13 +104,25 @@ function parseClaudeArgsToExtraArgs(
|
||||
|
||||
// Check if next arg is a value (not another flag)
|
||||
if (nextArg && !nextArg.startsWith("--")) {
|
||||
// For accumulating flags, join multiple values with commas
|
||||
if (ACCUMULATING_FLAGS.has(flag) && result[flag]) {
|
||||
result[flag] = `${result[flag]},${nextArg}`;
|
||||
// For accumulating flags, consume all consecutive non-flag values
|
||||
// This handles: --allowed-tools "Tool1" "Tool2" "Tool3"
|
||||
if (ACCUMULATING_FLAGS.has(flag)) {
|
||||
const values: string[] = [];
|
||||
while (i + 1 < args.length && !args[i + 1]?.startsWith("--")) {
|
||||
i++;
|
||||
values.push(args[i]!);
|
||||
}
|
||||
const joinedValues = values.join(ACCUMULATE_DELIMITER);
|
||||
if (result[flag]) {
|
||||
result[flag] =
|
||||
`${result[flag]}${ACCUMULATE_DELIMITER}${joinedValues}`;
|
||||
} else {
|
||||
result[flag] = joinedValues;
|
||||
}
|
||||
} else {
|
||||
result[flag] = nextArg;
|
||||
i++; // Skip the value
|
||||
}
|
||||
i++; // Skip the value
|
||||
} else {
|
||||
result[flag] = null; // Boolean flag
|
||||
}
|
||||
@@ -68,12 +147,23 @@ 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:
|
||||
// Extract and merge allowedTools from all sources:
|
||||
// 1. From extraArgs (parsed from claudeArgs - contains tag mode's tools)
|
||||
// - Check both camelCase (--allowedTools) and hyphenated (--allowed-tools) variants
|
||||
// 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 allowedToolsValues = [
|
||||
extraArgs["allowedTools"],
|
||||
extraArgs["allowed-tools"],
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(ACCUMULATE_DELIMITER);
|
||||
const extraArgsAllowedTools = allowedToolsValues
|
||||
? allowedToolsValues
|
||||
.split(ACCUMULATE_DELIMITER)
|
||||
.flatMap((v) => v.split(","))
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean)
|
||||
: [];
|
||||
const directAllowedTools = options.allowedTools
|
||||
? options.allowedTools.split(",").map((t) => t.trim())
|
||||
@@ -82,10 +172,21 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
|
||||
...new Set([...extraArgsAllowedTools, ...directAllowedTools]),
|
||||
];
|
||||
delete extraArgs["allowedTools"];
|
||||
delete extraArgs["allowed-tools"];
|
||||
|
||||
// Same for disallowedTools
|
||||
const extraArgsDisallowedTools = extraArgs["disallowedTools"]
|
||||
? extraArgs["disallowedTools"].split(",").map((t) => t.trim())
|
||||
// Same for disallowedTools - check both camelCase and hyphenated variants
|
||||
const disallowedToolsValues = [
|
||||
extraArgs["disallowedTools"],
|
||||
extraArgs["disallowed-tools"],
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(ACCUMULATE_DELIMITER);
|
||||
const extraArgsDisallowedTools = disallowedToolsValues
|
||||
? disallowedToolsValues
|
||||
.split(ACCUMULATE_DELIMITER)
|
||||
.flatMap((v) => v.split(","))
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean)
|
||||
: [];
|
||||
const directDisallowedTools = options.disallowedTools
|
||||
? options.disallowedTools.split(",").map((t) => t.trim())
|
||||
@@ -94,6 +195,17 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
|
||||
...new Set([...extraArgsDisallowedTools, ...directDisallowedTools]),
|
||||
];
|
||||
delete extraArgs["disallowedTools"];
|
||||
delete extraArgs["disallowed-tools"];
|
||||
|
||||
// Merge multiple --mcp-config values by combining their mcpServers objects
|
||||
// The action prepends its config (github_comment, github_ci, etc.) as inline JSON,
|
||||
// and users may provide their own config as inline JSON or file path
|
||||
if (extraArgs["mcp-config"]) {
|
||||
const mcpConfigValues = extraArgs["mcp-config"].split(ACCUMULATE_DELIMITER);
|
||||
if (mcpConfigValues.length > 1) {
|
||||
extraArgs["mcp-config"] = mergeMcpConfigs(mcpConfigValues);
|
||||
}
|
||||
}
|
||||
|
||||
// Build custom environment
|
||||
const env: Record<string, string | undefined> = { ...process.env };
|
||||
|
||||
@@ -108,6 +108,48 @@ describe("parseSdkOptions", () => {
|
||||
expect(result.sdkOptions.extraArgs?.["allowedTools"]).toBeUndefined();
|
||||
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
|
||||
});
|
||||
|
||||
test("should handle hyphenated --allowed-tools flag", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: '--allowed-tools "Edit,Read,Write"',
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
expect(result.sdkOptions.allowedTools).toEqual(["Edit", "Read", "Write"]);
|
||||
expect(result.sdkOptions.extraArgs?.["allowed-tools"]).toBeUndefined();
|
||||
});
|
||||
|
||||
test("should accumulate multiple --allowed-tools flags (hyphenated)", () => {
|
||||
// This is the exact scenario from issue #746
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs:
|
||||
'--allowed-tools "Bash(git log:*)" "Bash(git diff:*)" "Bash(git fetch:*)" "Bash(gh pr:*)"',
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
expect(result.sdkOptions.allowedTools).toEqual([
|
||||
"Bash(git log:*)",
|
||||
"Bash(git diff:*)",
|
||||
"Bash(git fetch:*)",
|
||||
"Bash(gh pr:*)",
|
||||
]);
|
||||
});
|
||||
|
||||
test("should handle mixed camelCase and hyphenated allowedTools flags", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: '--allowedTools "Edit,Read" --allowed-tools "Write,Glob"',
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
// Both should be merged - note: order depends on which key is found first
|
||||
expect(result.sdkOptions.allowedTools).toContain("Edit");
|
||||
expect(result.sdkOptions.allowedTools).toContain("Read");
|
||||
expect(result.sdkOptions.allowedTools).toContain("Write");
|
||||
expect(result.sdkOptions.allowedTools).toContain("Glob");
|
||||
});
|
||||
});
|
||||
|
||||
describe("disallowedTools merging", () => {
|
||||
@@ -134,19 +176,129 @@ describe("parseSdkOptions", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("other extraArgs passthrough", () => {
|
||||
test("should pass through mcp-config in extraArgs", () => {
|
||||
describe("mcp-config merging", () => {
|
||||
test("should pass through single mcp-config in extraArgs", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config '{"mcpServers":{}}' --allowedTools "Edit"`,
|
||||
claudeArgs: `--mcp-config '{"mcpServers":{"server1":{"command":"cmd1"}}}'`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
expect(result.sdkOptions.extraArgs?.["mcp-config"]).toBe(
|
||||
'{"mcpServers":{}}',
|
||||
'{"mcpServers":{"server1":{"command":"cmd1"}}}',
|
||||
);
|
||||
});
|
||||
|
||||
test("should merge multiple mcp-config flags with inline JSON", () => {
|
||||
// Simulates action prepending its config, then user providing their own
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config '{"mcpServers":{"github_comment":{"command":"node","args":["server.js"]}}}' --mcp-config '{"mcpServers":{"user_server":{"command":"custom","args":["run"]}}}'`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
const mcpConfig = JSON.parse(
|
||||
result.sdkOptions.extraArgs?.["mcp-config"] as string,
|
||||
);
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("github_comment");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("user_server");
|
||||
expect(mcpConfig.mcpServers.github_comment.command).toBe("node");
|
||||
expect(mcpConfig.mcpServers.user_server.command).toBe("custom");
|
||||
});
|
||||
|
||||
test("should merge three mcp-config flags", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config '{"mcpServers":{"server1":{"command":"cmd1"}}}' --mcp-config '{"mcpServers":{"server2":{"command":"cmd2"}}}' --mcp-config '{"mcpServers":{"server3":{"command":"cmd3"}}}'`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
const mcpConfig = JSON.parse(
|
||||
result.sdkOptions.extraArgs?.["mcp-config"] as string,
|
||||
);
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("server1");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("server2");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("server3");
|
||||
});
|
||||
|
||||
test("should handle mcp-config file path when no inline JSON exists", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config /tmp/user-mcp-config.json`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
expect(result.sdkOptions.extraArgs?.["mcp-config"]).toBe(
|
||||
"/tmp/user-mcp-config.json",
|
||||
);
|
||||
});
|
||||
|
||||
test("should merge inline JSON configs when file path is also present", () => {
|
||||
// When action provides inline JSON and user provides a file path,
|
||||
// the inline JSON configs should be merged (file paths cannot be merged at parse time)
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config '{"mcpServers":{"github_comment":{"command":"node"}}}' --mcp-config '{"mcpServers":{"github_ci":{"command":"node"}}}' --mcp-config /tmp/user-config.json`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
// The inline JSON configs should be merged
|
||||
const mcpConfig = JSON.parse(
|
||||
result.sdkOptions.extraArgs?.["mcp-config"] as string,
|
||||
);
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("github_comment");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("github_ci");
|
||||
});
|
||||
|
||||
test("should handle mcp-config with other flags", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config '{"mcpServers":{"server1":{}}}' --model claude-3-5-sonnet --mcp-config '{"mcpServers":{"server2":{}}}'`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
const mcpConfig = JSON.parse(
|
||||
result.sdkOptions.extraArgs?.["mcp-config"] as string,
|
||||
);
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("server1");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("server2");
|
||||
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
|
||||
});
|
||||
|
||||
test("should handle real-world scenario: action config + user config", () => {
|
||||
// This is the exact scenario from the bug report
|
||||
const actionConfig = JSON.stringify({
|
||||
mcpServers: {
|
||||
github_comment: {
|
||||
command: "node",
|
||||
args: ["github-comment-server.js"],
|
||||
},
|
||||
github_ci: { command: "node", args: ["github-ci-server.js"] },
|
||||
},
|
||||
});
|
||||
const userConfig = JSON.stringify({
|
||||
mcpServers: {
|
||||
my_custom_server: { command: "python", args: ["server.py"] },
|
||||
},
|
||||
});
|
||||
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--mcp-config '${actionConfig}' --mcp-config '${userConfig}'`,
|
||||
};
|
||||
|
||||
const result = parseSdkOptions(options);
|
||||
|
||||
const mcpConfig = JSON.parse(
|
||||
result.sdkOptions.extraArgs?.["mcp-config"] as string,
|
||||
);
|
||||
// All servers should be present
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("github_comment");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("github_ci");
|
||||
expect(mcpConfig.mcpServers).toHaveProperty("my_custom_server");
|
||||
});
|
||||
});
|
||||
|
||||
describe("other extraArgs passthrough", () => {
|
||||
test("should pass through json-schema in extraArgs", () => {
|
||||
const options: ClaudeOptions = {
|
||||
claudeArgs: `--json-schema '{"type":"object"}'`,
|
||||
|
||||
Reference in New Issue
Block a user