Skip to content

refactor(mcp): parseAddArgs via stdlib flag.FlagSet#40

Merged
mudler merged 1 commit into
masterfrom
refactor/mcpmanage-flag-parsing
Jul 1, 2026
Merged

refactor(mcp): parseAddArgs via stdlib flag.FlagSet#40
mudler merged 1 commit into
masterfrom
refactor/mcpmanage-flag-parsing

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

Summary

Internal refactor of nib mcp add's flag parsing (cmd/mcpmanage.go's parseAddArgs). No behavior change.

The function grew flag-by-flag across the MCP CLI work and the recent remote-auth work (#39) into ten near-identical switch cases — two per flag (--url, --transport, --env, --token, --header), one for the space-separated form and one for inline --flag=value — plus a manual pre-scan loop to find a standalone -- and split off the stdio command's own argv.

Replaced with stdlib flag.FlagSet, mirroring the existing parseInstallArgs pattern already in cmd/plugin.go (two-pass flag.NewFlagSet(..., flag.ContinueOnError) parse, handling a positional <name> argument that can appear relative to flags). This isn't a new dependency or a foreign idiom — flag is already imported in cmd/mcp.go, cmd/plugin.go, and cmd/skill.go.

  • --url/--transport/--token become fs.StringVar calls.
  • --env/--header (both repeatable KEY=VALUE flags) share one small kvFlag type implementing flag.Value, registered twice.
  • The manual ---boundary scan is deleted entirely: stdlib flag.FlagSet already terminates parsing at a bare --, strips it, and returns everything after it via fs.Args() — exactly the "rest is the stdio command's own argv" behavior the old code hand-rolled.
  • Post-parse validation (transport must be http/sse, exactly one of command/URL) is unchanged.

Error message text changes in a few places (stdlib's own wording for missing-value/unknown-flag cases) — no test asserts exact text, only err == nil/err != nil, and this is a dev CLI with no message-stability contract.

Design spec: docs/superpowers/specs/2026-07-01-mcpmanage-flag-parsing-refactor.md (local-only, gitignored, not part of this diff)
Implementation plan: docs/superpowers/plans/2026-07-01-mcpmanage-flag-parsing-refactor.md (same)

Test plan

  • All 10 existing tests in cmd/mcpmanage_test.go pass unchanged, both before (baseline) and after the refactor — no test file edits
  • go build ./..., go vet ./..., go test ./... all clean across every package
  • Manually traced the two highest-risk equivalence cases: the ---boundary termination (confirmed fs.Args() correctly excludes -- and returns the literal command argv, including a flag-shaped literal like -x), and the kvFlag map-aliasing (confirmed map[string]stringkvFlag conversion shares the backing map, so Set calls correctly populate srv.Env/srv.Headers)

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mudler mudler merged commit 24e94d2 into master Jul 1, 2026
2 checks passed
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.

2 participants