fix(mcp): reject an unsupported flag instead of registering it as the server name - #164
Merged
ralyodio merged 1 commit intoJul 31, 2026
Conversation
… server name An unsupported flag before `--` fell through the parse loop into the positional list, so `mcp add -s user https://mcp.sentry.dev/mcp` registered a server literally named `-s` with the command `user` and demoted the real URL to an argument. The spec is spliced verbatim into each engine's native `mcp add` argv, so Claude received: claude mcp add -s user -s -- user https://mcp.sentry.dev/mcp Reject any token that still starts with `-` once it would become the name or the command, and name the flags mcp does take. Anything after `--` is the user's own command line and is left alone.
Merged
ralyodio
added a commit
that referenced
this pull request
Aug 1, 2026
install.sh resolves releases/latest, so the sixteen commits merged since v0.13.3 have been sitting on main unreachable — including a fix for a page that locks browsers up. The headline is the pit. /pit rendered every ending an account held and a form per name under each, with no bound on either: at 50 endings x 100 names that was 3.1 MiB of HTML and 36,082 DOM elements, and it managed to jam a browser with no script on the page at all (#167). It now draws a window and says what it is not drawing — 173 KiB, 1,926 elements — with a filter box over the top that takes `eggs` as a substring and `def*` as a glob, debounced against the API (#168). The namespace also stopped being the one part of the product a script could not touch: /api/moshpit/* now accepts the same API key /api/me and /api/sessions already did (#169), and /pit/dns finally documents the TronBrowser route for machines whose DNS is not theirs to change (#165). moshcode: foreign keys are enforced, and the licence package.json claims actually ships (#154) cli: help aliases exit 0 (#157), invalid integration commands fail (#160), `--` is honoured (#159), a BOM before a shebang no longer breaks (#158) skills: engines with no skills primitive are reported, not dropped (#166); `--name` requires a value (#156) mcp: an unsupported flag is rejected rather than registered as the server name (#164) pit: the namespace rules are vendored again with a drift test holding them to the published package (#161, #162, #163) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
parseMcpconsumes the flags it knows (--name,-t/--transport,-e/--env,-H/--header) and pushes everything else ontopositional. A flag it does not know therefore survives as a positional and becomes the server name, with its value promoted to the command.The most likely way to hit this is copying the engine-native syntax the PRD itself documents (
claude mcp add -s user …):-suser["https://mcp.sentry.dev/mcp"]Because the spec is spliced verbatim into each engine's native
mcp addargv, the bad name is not just cosmetic — it lands in the argv as a flag:mcpCommandruns that fan-out with no confirmation step, so the mistake propagates to every installed engine at once. There is no error and no warning; the run reportsadded.mcp install --scope user https://…was also wrong, but differently: it fell through toa stdio command server needs an explicit --name, which points the user at the wrong problem entirely.Reproduction
Real CLI, unpatched:
Patched:
The fix
13 lines in
src/integrations.mjs. Once name and target are resolved, a token still starting with-was never consumed as a flag, so it is a typo or an engine-native flag moshcode does not take. Reject it and name the flags that are accepted.Deliberately scoped:
mcp add tools npx -y srvandmcp add tools -- npx -y srv --port 3000are untouched. Both have control tests.--.src/mcp-catalog.mjsstates thatmcp add <name> -- <cmd> …"still takes anything", so the escape hatch is not second-guessed. The check skips the target entirely when it came fromcmdParts.mcp install --scope …now reports the real cause instead of the misleading missing-name error.Tests
New
test/mcp-stray-flag.test.mjs, 16 tests. 8 cover the bug (short flag, long flag, misspelled supported flag, stray in the command position, the misleading install error, the error's contents, nothing reachingplanMcpAdd, and a stray flag in front of a catalog name). 8 are controls asserting the opposite direction, so the fix cannot buy green by over-rejecting: flags after--pass through, flags after the target stay arguments, every supported flag parses byte-identically, the catalog shortcut still resolves, a bare URL install still derives its name, the pre-existing missing-value guards still fire first, an unknown verb still reports an unknown verb, and a stdio install with no name still asks for--name.Fail-before: 8 fail / 8 pass unpatched, 16/16 patched.
Full suite: 615 → 631 tests, 0 failures (pass 493 → 509, skipped 122 unchanged).
Note on the existing coverage
test/mcp.test.mjs:80already exercises flag parsing, but its fixture is["add", "tools", "--", "npx", "-y", "srv"]— the dash tokens sit after--, so they go tocmdPartsand never reach the positional list where the defect lives. It passes unchanged, before and after.