fix(mcp): align allowed-tools parser with SDK option parser (#1373)

parseAllowedTools (used to decide which GitHub MCP servers to install)
hand-rolled a regex parse of claude_args, while the tools actually
granted to Claude are parsed by parseClaudeArgsToExtraArgs in
base-action/src/parse-sdk-options.ts using shell-quote. The two parsers
diverged on two inputs (#1357):

- Multiple values after a single flag: for
  `--allowedTools "Read" "Grep" "mcp__github__get_commit"` the regex
  captured only "Read", so the github MCP server was not installed even
  though mcp__github__get_commit was granted — tool calls then failed.
- Commented-out lines: the regex counted tools on `#`-prefixed lines
  that the SDK parser strips, installing servers that were never used.

Reimplement parseAllowedTools on the same shell-quote tokenizer and the
same "accumulating flag consumes all consecutive non-flag values"
semantics, stripping comment lines first, so the install decision agrees
with the tools that are actually granted. Unquoted glob patterns (e.g.
`mcp__github__*`), which shell-quote yields as glob objects, are
recovered to their literal text to preserve existing behavior.

Closes #1357

Co-authored-by: bymle <229636660+bymle@users.noreply.github.com>
This commit is contained in:
bymle
2026-06-13 22:49:34 -07:00
committed by GitHub
co-authored by bymle
parent a5e5d3b82e
commit 3d9f0dc7dc
2 changed files with 107 additions and 25 deletions
+37 -3
View File
@@ -37,9 +37,43 @@ describe("parseAllowedTools", () => {
test("handles --allowedTools followed by another --allowedTools flag", () => {
const args = "--allowedTools --allowedTools mcp__github__*";
// The second --allowedTools is consumed as a value of the first, then skipped.
// This is an edge case with malformed input - returns empty.
expect(parseAllowedTools(args)).toEqual([]);
// The first --allowedTools has no value (the next token is another flag);
// the second consumes mcp__github__*. This matches how the SDK option
// parser (parse-sdk-options.ts) reads the same input.
expect(parseAllowedTools(args)).toEqual(["mcp__github__*"]);
});
test("captures multiple values after a single --allowedTools flag", () => {
// Regression for #1357: the install-decision parser must capture every
// value, not just the first, so it agrees with the tools actually granted
// to Claude. Previously only "Read" was seen, so the github MCP server was
// not installed even though mcp__github__get_commit was granted.
const args = '--allowedTools "Read" "Grep" "mcp__github__get_commit"';
expect(parseAllowedTools(args)).toEqual([
"Read",
"Grep",
"mcp__github__get_commit",
]);
});
test("captures multiple values spread across lines under one flag", () => {
const args = `--allowedTools
"Read"
"Grep"
"mcp__github__get_commit"`;
expect(parseAllowedTools(args)).toEqual([
"Read",
"Grep",
"mcp__github__get_commit",
]);
});
test("ignores commented-out lines", () => {
// Regression for #1357: a commented-out flag must not be counted, matching
// the SDK parser which strips comment lines before parsing.
const args = `# --allowedTools "mcp__github__get_commit"
--allowedTools "Read"`;
expect(parseAllowedTools(args)).toEqual(["Read"]);
});
test("parses multiple separate --allowed-tools flags", () => {