unbridled-41 opened a new pull request, #5916:
URL: https://github.com/apache/rocketmq-dashboard/pull/5916
### Which Issue(s) This PR Fixes
Fixes #5915
### Problem / Evidence
`rmqctl mcp config` appends `--instance-id` only when it is set, exactly
like the genuinely optional `--config`/`--context`, and exits 0 with an argv
that cannot run:
```
$ rmqctl mcp config
{"mcpServers":{"rocketmq-studio":{"command":"rmqctl","args":["mcp","stdio"]}}}
$ rmqctl mcp stdio
error [INVALID_ARGUMENT]: --instance-id is required
hint: Pass the Studio Instance identifier explicitly, e.g. --instance-id
my-instance; rmqctl never defaults it from the context.
exit=1
```
`mcp stdio` requires the identifier by design (decision 7: no default
injection from the context), so the generated snippet is unusable - and the
docs promise this flow (README: "any external agent pointed at the output of
`rmqctl mcp config`"; `docs/ai-agent-architecture.md`: "直接粘 `rmqctl mcp config`
的输出即可").
### Root cause / Fix
The flag was treated as optional because the other three are, and nothing
validated it even though the sibling `mcp stdio` command refuses to start
without it. Fail closed: without an instance identifier, report
`INVALID_ARGUMENT` and print no snippet. The with-flag output is unchanged,
which the cross-language golden test against the Studio-generated snippet still
verifies.
### Priority and scoring
**PRIORITY 62** — impact 22/40 (a documented onboarding path produces a
configuration that cannot start, and the failure surfaces only in the MCP
client's log), blast radius 14/20 (every user who follows the
README/architecture doc), reproducibility 20/20 (two commands, exact output
above), maintenance value 6/20.
**FIX_CONFIDENCE 88** — fail-closed validation matching the sibling
command's own requirement.
### Tests
`cd rmqctl && go test ./cmd/ -run TestMCPConfig`
| Test | Before | After |
|---|---|---|
| `TestMCPConfigRequiresInstanceID` | FAIL (exit 0, snippet printed) | PASS |
| `TestMCPConfigMatchesStudioGeneratedSnippet` | PASS | PASS (output
unchanged) |
| `TestMCPConfigCarriesExplicitInstanceID` | PASS | PASS |
| `TestMCPConfigEmitsOnlyContextAndConfigReferences` | updated to supply the
identifier (its subject - no credential leakage, no duplicated context fields -
is unchanged) and to assert the snippet carries the supplied instance rather
than an invented one | PASS |
`go test ./...` reports only the pre-existing YAML renderer failures owned
by open PR #5850; `gofmt -l cmd/` is clean.
### Risk
A user who previously generated a snippet without the flag now gets an error
instead of an unusable file; the error names the flag. `--config`, `--context`
and `--timeout` keep their optional behaviour.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
