Skip to content

fix(mcp): reject unknown pre-positional flags in add - #3048

Closed
afjcjsbx wants to merge 2 commits into
sipeed:mainfrom
afjcjsbx:fix/mcp-add-inherited-flag-parsing
Closed

fix(mcp): reject unknown pre-positional flags in add#3048
afjcjsbx wants to merge 2 commits into
sipeed:mainfrom
afjcjsbx:fix/mcp-add-inherited-flag-parsing

Conversation

@afjcjsbx

@afjcjsbx afjcjsbx commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

📝 Description

Fixes mcp add argument parsing when root-level persistent flags are passed before the subcommand, such as --no-color.

mcp add uses DisableFlagParsing: true, so inherited root flags could leak into the custom parser and be misinterpreted as positional arguments. In practice this broke http/sse server registration and could also silently save incorrect stdio server names.

This PR:

  • strips inherited root flags before mcp add parses its own arguments
  • returns an explicit error for unknown --prefixed arguments seen before the required positional arguments
  • adds regression tests for the reported repro and the unknown-flag case

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

Fixes #3041

📚 Technical Context (Skip for Docs)

🧪 Test Environment

  • Hardware: Local development machine
  • OS: macOS
  • Model/Provider:
  • Channels: CLI

📸 Evidence (Optional)

Click to view Logs/Screenshots
GOCACHE=/tmp/picoclaw-gocache go test ./cmd/picoclaw/internal/mcp -count=1
ok  	github.com/sipeed/picoclaw/cmd/picoclaw/internal/mcp	1.162s

Regression coverage added for:

  • inherited root flag before mcp add positionals (--no-color)
  • unknown flag before required positionals returns a clear error

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

@afjcjsbx
afjcjsbx requested review from alexhoshina and mengzhuo June 7, 2026 21:01
@github-actions

Copy link
Copy Markdown

This PR has had no activity for 7 days and has been marked as stale. If you are still working on it, please push an update or leave a comment; otherwise it will be closed automatically in 7 days.

@github-actions

Copy link
Copy Markdown

This PR has been closed after 14 days of inactivity. If you would like to continue, feel free to reopen it or submit a new PR.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mcp add mis-parses global flags into positionals (DisableFlagParsing), breaking http/sse adds and silently mis-naming stdio servers

1 participant