Skip to content

fix: preserve server arguments after the -- separator - #98

Open
Dcastroro wants to merge 1 commit into
f:masterfrom
Dcastroro:fix/preserve-server-arguments
Open

Dcastroro wants to merge 1 commit into
f:masterfrom
Dcastroro:fix/preserve-server-arguments

Conversation

@Dcastroro

Copy link
Copy Markdown

When a stdio server uses an option such as -f, --transport, or --auth-header, ProcessFlags consumes it as an MCP Tools option anywhere in the command line. For example, a server's -f settings.yaml is removed and also changes the client's output format.

Honor a standalone -- in ProcessFlags: consume the separator and preserve every following argument verbatim. This applies to tools, resources, prompts, and guard. 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.

mcp tools -f json -- python3 server.py -f settings.yaml --transport stdio

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 unknown shadow nolint warning remains.
  • go build -o /tmp/mcp ./cmd/mcptools — passes.
  • Four executable smoke checks with a local stdio MCP fixture: tools, resources, and prompts receive all server arguments unchanged, including a repeated -- and an empty argument; legacy trailing -f json still works.
  • Reviewed all 14 currently open PRs, including feat: add --timeout/-t flag for initialization timeout #87 and feat: add config source prefix syntax for loading servers from IDE configs #94, before preparing this change; none implements this separator behavior.

Copilot AI balanced review requested due to automatic review settings October 16, 2026 11:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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:

  • ProcessFlags now 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 subsequent ProcessFlags pass 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

-f, --format, --transport and --auth-* are stripped from the server command's own arguments

2 participants