feat: [AI-6948] register mcps slash command in server command list#885
Conversation
Register a new slash command /mcps that lists all configured MCP servers and their current connection status. The webview chat intercepts this command client-side and calls GET /mcp directly, displaying results inline without an LLM call.
|
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. |
d8c851f
into
fix/datamate-stdio-local-transport
dev-punia-altimate
left a comment
There was a problem hiding this comment.
Multi-Persona Review — Verdict: request-changes
The PR registers the /mcps slash command correctly but fails to implement the core user-facing functionality described in AI-6948. Without server-side data retrieval, the command is non-functional as a feature. Additionally, code style and test coverage gaps must be addressed before approval.
14/14 agents completed · 126s · 4 findings (0 critical, 1 high, 2 medium)
High
- [product-manager] Ticket AI-6948 asks for the /mcps command to enable users to list added MCP servers and their connection status, but this PR only registers the command with a template and description — it does not implement the underlying logic to fetch or display server status. The webview interception is handled in a companion PR, but without server-side data retrieval, the command is non-functional as a user-facing feature.
- 💡 Either implement server-side logic to return MCP server status in a follow-up ticket, or clearly document in the ticket that this is a placeholder registration and the full functionality is deferred.
Medium
- [code-reviewer, tech-lead, end-user] Template string uses double quotes instead of backticks, which may cause issues if the string contains embedded expressions or line breaks; inconsistent with codebase style where template literals are preferred for multi-line or dynamic content. →
packages/opencode/src/command/index.ts:150- 💡 Change "List all configured MCP servers and their current connection status." to
List all configured MCP servers and their current connection status.
- 💡 Change "List all configured MCP servers and their current connection status." to
- [devops, end-user, pr-hygiene] New command registration lacks automated test coverage; while the PR includes a test plan, no tests are implemented, risking undetected regressions in command availability or template rendering.
- 💡 Add a unit test in
packages/opencode/src/command/__tests__/index.test.tsthat verifies/mcpsis present in the registered commands and its template/description match expected values.
- 💡 Add a unit test in
Low
- [cto] Adding a server-side fallback template for a webview-intercepted command is a valid pattern, but this PR does not yet standardize it across other similar commands. The QA intelligence suggests this should become a standard pattern, but no follow-up work is tracked.
- 💡 Create a follow-up task to audit and standardize server-side fallback templates for all webview-intercepted commands (e.g., /discover-mcps, /configure-claude) to ensure consistency and reduce future tech debt.
Multi-Persona Review · vllm:qwen3-next-80b (waves) + vllm-fallback (synth) ·
| }, | ||
| [Default.MCPS]: { | ||
| name: Default.MCPS, | ||
| description: "list added MCP servers and their connection status", |
There was a problem hiding this comment.
[MEDIUM · code-reviewer, tech-lead, end-user] Template string uses double quotes instead of backticks, which may cause issues if the string contains embedded expressions or line breaks; inconsistent with codebase style where template literals are preferred for multi-line or dynamic content.
💡 Suggestion: Change "List all configured MCP servers and their current connection status." to List all configured MCP servers and their current connection status.
Confidence: 95/100
…cps command (#893) * feat: datamate stdio local transport + extension single-gateway mode - Add stdio transport support for datamate MCP (vs HTTP-only before) - Single-gateway mode: when .vscode/mcp.json has "datamate" key, always use it as server name — prevents duplicate tool sets from extension - syncDatamateUrlFromVscodeMcp: use updatedAt field as change signal for the "datamate" entry (works for both stdio and HTTP), URL comparison for all other remote entries - Strip ALTIMATE_EXTENSION_RPC from persisted mcp-discover configs to avoid stale socket paths across VS Code sessions - persistMcpEnabled: write enabled/disabled flag to disk on MCP connect/disconnect so it survives session restarts - Add /altimate/mcp/reload-datamate endpoint to re-sync and reconnect without full server restart - MCP.ToolsChanged subscription in prompt loop for traceability - Merge main: preserve trace consumer in serve.ts, restore exports on isAnthropicLikeModel and insertReminders * feat: [AI-6948] register mcps slash command in server command list (#885) Co-authored-by: saravmajestic <saravmajestic@altimate.ai> * fix: address review comments + stale config cache in reload-datamate + improve logging * refactor: replace hardcoded IDE paths with glob scan for mcp.json Instead of checking .vscode/mcp.json, .cursor/mcp.json, and .github/copilot/mcp.json separately, scan all mcp.json files under the project root and use the first one that contains a datamate entry. This is IDE-agnostic and will work with any editor that writes an mcp.json. - Remove IDE_MCP_SOURCES constant - Add findAllMcpJsonFiles() using Glob.scan(**/mcp.json) - Add extractServersMap() helper to try both "servers" and "mcpServers" keys - Update readDatamateTransportFromIde() and syncDatamateUrlFromVscodeMcp() to use the new scan approach * fix(mcps): bypass LLM for /mcps list, enable, disable commands Add direct handler in SessionPrompt.command for the mcps slash command. Instead of routing through the AI template, the handler calls MCP.status(), MCP.connect(), and MCP.disconnect() directly and returns a structured MessageV2.WithParts response, matching the TUI HTTP API behavior. - /mcps (no args): queries MCP.status() and renders a markdown table - /mcps enable <name>: calls MCP.connect(name) and reports result - /mcps disable <name>: calls MCP.disconnect(name) and confirms * fix(mcps): move /mcps bypass handler before Command.get() check The /mcps direct handler was added after the Command.get() call and "Command not found" error, so it never fired — /mcps was not in the registered commands list and would throw before reaching the bypass. Move the handler to run before Command.get() so it short-circuits cleanly for the built-in /mcps, /mcps enable, and /mcps disable commands without requiring a registered command entry. * fix(mcp-discover): strip enabled:false when explicitly adding server to config * fix(mcp-discover): set enabled:true when user explicitly adds server to config When the user asks to add an MCP server via /discover-and-add-mcps, the config entry should have enabled:true so /mcps shows it as connected. Previously the strip of the auto-discovery enabled:false left no enabled field, which caused ambiguity in the UI even though mcp/index.ts treats missing as enabled=true at runtime. * fix(mcp-discover): add updatedAt timestamp and connect MCP immediately after config write - Add updatedAt: new Date().toISOString() when writing MCP server to config so clients can detect config changes without restarting - Call MCP.connect(name) immediately after addMcpToConfig so the server becomes active in the current session — /mcps now shows connected without restart * chore: move isAnthropicLikeModel NOTE docblock back above function * chore: strip altimate_change markers from altimate/ subtree, inline DATAMATE_KEY * fix: address review comments — glob ignore, type guard, persist from disk, dedup respond() * fix: add model param to respond() helper — was undefined in closure causing no-reply on /mcps * fix: [AI-6948] discover MCPs from project root instead of git worktree `/discover-and-add-mcps` found no servers (and auto-discovery added none) for non-git projects: `Instance.worktree` resolves to "/" when there is no `.git`, so the scan looked under the filesystem root and missed the workspace's `.vscode/mcp.json` (datamate). The `add` action also resolved its project-scoped config path against "/". - `mcp-discover.ts`: use `Instance.directory` (project root) instead of `Instance.worktree` for the discovery scan, persisted-name read, and the project-scoped config write target - `config.ts`: same `Instance.worktree` -> `Instance.directory` for the background auto-discovery caller - `discover.ts`: replace the hardcoded IDE paths with a recursive `**/mcp.json` glob scan rooted at the project directory (IDE-agnostic, trying both `servers` and `mcpServers` keys), keeping the non-`mcp.json` sources (`.mcp.json`, `.gemini/settings.json`, `~/.claude.json`) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: [AI-6948] address PR review — preserve MCP config fields on disk writes Resolves the review findings on the `/mcps` enable/disable persistence and datamate transport sync paths. - `mcp/config.ts`: `readMcpEntryFromDisk` now uses `getNodeValue` instead of a manual children walk. `jsonc-parser` only populates `Node.value` for primitives, so the old walk silently dropped `command` arrays and `environment`/`headers` objects — corrupting local stdio (and remote-with- headers) entries on the next write via `persistMcpEnabled`. - `altimate/datamate-transport.ts`: same `getNodeValue` fix for the two sibling walkers; preserve non-transport fields (`enabled`, `timeout`, `oauth`, …) when rebuilding the synced datamate entry; widen the `**/mcp.json` ignore list (`target`, `.next`, `out`, `vendor`, `coverage`, `.venv`, `.turbo`). - `mcp/index.ts`: serialize `persistMcpEnabled` (promise chain) so concurrent enable/disable can't interleave read-modify-write and clobber each other. - `server/server.ts`: `reload-datamate` returns HTTP 500 (not 200) on error so the extension can distinguish failure. - `session/prompt.ts`: `/mcps enable|disable` validates the server name against config and reports clearly when it's unknown (was silent). - `test/mcp/config.test.ts`: add `readMcpEntryFromDisk` round-trip tests, including the persist-enabled flow that previously corrupted local entries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: [AI-6948] preserve IDE source precedence in mcp.json glob scan The `**/mcp.json` glob scan sorted purely alphabetically, which let `.cursor/mcp.json` override `.vscode/mcp.json` for same-named servers — a regression vs the previous fixed-source order (.vscode > .cursor > .github/copilot). Caught by the `discover-adversarial` precedence test. Order globbed files by IDE precedence first, then alphabetically, restoring first-source-wins semantics while staying IDE-agnostic for any other mcp.json. Also align the ignore list with `datamate-transport.ts`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: [AI-6948] balance altimate_change markers (CI Marker Guard + integrity tests) Two pre-existing marker imbalances on the branch failed the "Require-markers regression backstop" job and the `altimate_change marker integrity` tests (origin/main is balanced; these were introduced by earlier merge commits on this branch): - `cli/cmd/serve.ts` (6 start / 4 end): close the trace-import region after its import, and close the branding region after the rebranded log line (the sync-datamate region stays nested inside it). - `session/prompt.ts` (43 start / 44 end): remove a duplicate `altimate_change end` left by a merge — the MCP-ToolsChanged region is already closed before `while (true)`, and the SessionStatus region right after has its own pair. Comment-only changes; no behavior impact. Verified with `bun run script/upstream/analyze.ts --require-markers --strict` (38/38) and the marker-integrity suites (118/0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: saravmajestic <saravmajestic@altimate.ai> Co-authored-by: altimate-harness-bot[bot] <274995457+altimate-harness-bot[bot]@users.noreply.github.com> Co-authored-by: Saravanan <sarav.majestic@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
/mcpsin altimate-code's server command list (command/index.ts) so it appears in the slash autocomplete in the Altimate Code Chat webview/mcpsclient-side (see companion PR) — this server-side registration ensures autocomplete works and provides a fallback template if ever routed through the LLMChanges
packages/opencode/src/command/index.ts— addMCPS: "mcps"toDefaultconstants; registermcpscommand entry with description and minimal templateTest plan
GET /commandreturnsmcpsin the command listtsc --noEmitexits 0 inpackages/opencodeCompanion PR in vscode-altimate-mcp-server: AltimateAI/vscode-altimate-mcp-server#356
Requested by @saravmajestic via harness
Jira: AI-6948
Summary by cubic
Registers
/mcpsinaltimate-codeserver commands so it shows in slash autocomplete and has a server-side fallback template. Addresses AI-6948 by letting users list configured MCP servers and their connection status from chat.MCPS: "mcps"toCommand.Defaultand registered/mcpswith description and minimal template inpackages/opencode/src/command/index.ts./mcpsis routed through the LLM.Written for commit 728bc56. Summary will update on new commits.