Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is a small, additive, POSIX-consistent separator fix correctly scoped to the four affected commands and backed by targeted regression tests that cover the key edge cases.
0 open findings
What changed in this PR
This PR fixes a CLI argument-parsing bug where ProcessFlags consumed MCP Tools options (-f/--format, --transport, --server-logs, --auth-user, --auth-header) anywhere in the command line, including when those tokens actually belonged to the downstream stdio MCP server (e.g. a server's own -f settings.yaml). The fix introduces POSIX-style -- separator handling so everything after the first standalone -- is forwarded to the server verbatim, while preserving the existing trailing-option behavior when no separator is present. This applies to the tools, resources, prompts, and guard commands, all of which use DisableFlagParsing: true and route args through ProcessFlags (and, for guard, extractPatterns first).
Changes:
ProcessFlagsnow treats a standalone--as end-of-options and returns all following arguments unchanged.extractPatterns(guard) stops allow/deny parsing at--and preserves the separator so the subsequentProcessFlagspass consumes exactly one separator.- Added README documentation and 14 regression test cases covering the conflicting global flags, repeated/trailing separators, empty arguments, the three list commands via the root command, and guard's two parsing passes.
| File | Description |
|---|---|
cmd/mcptools/commands/utils.go |
Adds the -- end-of-options case to ProcessFlags; safe args[i+1:] slice even when -- is the final token. |
cmd/mcptools/commands/guard.go |
Adds the -- case to extractPatterns, retaining the separator (args[i:]) for the following ProcessFlags pass. |
README.md |
Documents the -- separator usage with examples for the four affected commands; branding and scope match the implementation. |
cmd/mcptools/commands/flag_separator_test.go |
New regression tests validating ProcessFlags, the list commands end-to-end via the root command, and guard's two-pass parsing. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
When a stdio server uses an option such as
-f,--transport, or--auth-header,ProcessFlagsconsumes it as an MCP Tools option anywhere in the command line. For example, a server's-f settings.yamlis removed and also changes the client's output format.Honor a standalone
--inProcessFlags: consume the separator and preserve every following argument verbatim. This applies totools,resources,prompts, andguard. Stop guard's earlier allow/deny parsing at the same boundary while retaining the separator for the second parsing pass. Existing option placement without a separator remains supported.Add README examples and 14 regression cases covering all conflicting global flags, repeated/trailing separators, empty arguments, the three list commands through the root command, and guard's two parsing passes. All 14 fail against the original implementation and pass with the fix. Authentication flag cases use inert option labels; the tests do not require credentials or external services.
Fixes #97.
Validation on Ubuntu 24.04 with Go 1.24.1:
go test -race -json ./...— all 144 leaf test cases pass, no failures.golangci-lint run --timeout=5m ./...(v2.0.2) — zero issues. The repository's existing unknownshadownolint warning remains.go build -o /tmp/mcp ./cmd/mcptools— passes.tools,resources, andpromptsreceive all server arguments unchanged, including a repeated--and an empty argument; legacy trailing-f jsonstill works.