Uh oh!
There was an error while loading. Please reload this page.
fix: add an MCP status command and report discovered-config drift - #1160
fix: add an MCP status command and report discovered-config drift#1160sahrizvi wants to merge 1 commit into
Conversation
`status` is the name people reach for when a server will not connect, and it was the one name that did not exist. The gap was narrower than it looks: `mcp list` already probed live and already printed the failure reason, so `status` is registered as a sibling sharing that handler rather than a second view to keep in sync. It is a distinct command rather than an alias because an alias widens yargs' alias column enough to rewrap unrelated sibling rows in the help output. MCP discovery is first-source-wins, so a server already present in the user's config was skipped outright and a changed `.vscode/mcp.json` — a new port, a moved command — was never mentioned. `driftFields()` now reports which fields disagree, naming nested keys individually (`environment.ALTIMATE_EXTENSION_RPC`) so the message points at the thing to fix. The configured value still wins; silently overwriting a user's own config would be worse than the silence it replaces. Closes#790Closes#878 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fJ3X7pcGT4R9yzjsJnqsV
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1. No
mcp statuscommand (#790)statusis the name people reach for when a server will not connect, and it was the one name that did not exist.Worth being straight about the size of this: the gap was much narrower than the issue implies.
mcp listalready probed live —mcp.status()initialises the MCP service — and already printed the failure reason for failed and needs-client-registration servers. The missing piece was the entry point, not the view. Sostatusis registered as a sibling sharing the same handler, rather than a second implementation to keep in sync.Why a command and not an alias. It was first written as
aliases: ["ls", "status"]. That widened yargs' alias column enough to rewrap an unrelated sibling row —mcp auth listlost its[aliases: ls]onto a second line. As its own command the help output stays additive: the diff against the committed snapshot is exactly one added line.2. Discovery skips config drift (#878)
Discovery is first-source-wins, so a server already present in the user's config is skipped outright. When
.vscode/mcp.jsonchanges — a newALTIMATE_EXTENSION_RPCport, a moved command — the difference was never mentioned. The reporter saw theirdatamateserver keep a stale value with no indication why.driftFields()reports which fields disagree, naming nested keys individually (environment.ALTIMATE_EXTENSION_RPC) so the message points at the thing to fix rather than justenvironment.enabledis excluded, since discovery sets it for its own reasons.The configured value still wins. The command reports the disagreement and where to look, and leaves the decision to the user. Silently overwriting someone's own config would be worse than the silence it replaces — which is why this reports rather than reconciles, one of the three options the issue offered.
Testing
opencodesuite: 11,495+ pass, no regressions.HOMEplus a temp project containing a drifted.vscode/mcp.json, asserting both the drifted field is named and that an agreeing config stays silent.statusregistration, each fails exactly one test.On flaky tests
The subprocess-heavy suites (
test/pty,test/cli/run) fail intermittently under parallel load on this branch — different tests each run, all passing in isolation across repeated runs. These flakes pre-date this PR, but this PR does make them more likely to surface: it adds two more e2e files that each spawn real CLI subprocesses, increasing contention. Flagging it rather than leaving it to look like noise.Test-harness fix worth knowing about
The e2e harness pattern copied from
mcp-add.test.tsusesbun run --cwd <pkg>, which makes the CLI's working directory the repo package — so it read the repo's own.opencodeconfig and never saw the temp project, meaning discovery never ran and the drift assertion silently could not fire. Fixed here by setting the spawn cwd to the project instead.mcp-add.test.tsis unaffected in practice because it passes--global, so it is left alone.🤖 Generated with Claude Code
https://claude.ai/code/session_018fJ3X7pcGT4R9yzjsJnqsV
Summary by cubic
Adds
mcp statusso a server that will not connect has the command name people reach for. It reuses themcp listhandler rather than duplicating a view that already probes live and prints failure reasons; it's a distinct command instead of an alias because an alias widens yargs' help column and rewraps unrelated sibling rows.Reports discovered-config drift. Discovery is first-source-wins, so a server already in
altimate-code.jsonis skipped and a changed.vscode/mcp.json(new port, moved command) was never mentioned.mcp statusandmcp listnow name the differing fields — nested keys individually, e.g.environment.ALTIMATE_EXTENSION_RPC. The configured value still wins;enabledis excluded since discovery sets it for its own reasons.Closes#790 and #878.
Written for commit 6776a8a. Summary will update on new commits.