feat(mcp): negotiate the protocol revision (2025-06-18) + bundle-mode dsh converter

[Hmbown]

First half of #6280 — lands the parallel lane's verified work. Not closing #6280 yet: the 2026-07-28 rmcp layer follows.

MCP negotiation (3e4102d46): every surface advertised the original 2024-11-05 revision and the stdio client required an exact match. Now: server + both clients advertise 2025-06-18 and negotiate over MCP_SUPPORTED_PROTOCOL_VERSIONS = [2025-06-18, 2025-03-26, 2024-11-05] — the server echoes the client's revision when supported (latest otherwise), stdio accepts any supported revision, the main client validates the negotiated version, and streamable HTTP sends the required MCP-Protocol-Version header on every post-initialize request. An old Codewhale client against a new server still connects (it hears the version it asked for).

Converter bundle mode (6b7e4ac53): convert-plugin.py --bundle <package-dir> consumes a real dsh npm bundle — patch-list application with applyEntryPatches parity, !!js capture-never-evaluate with the three idiom lowerings, host stdio directory snapshotting, skill import, and named diagnostics in CONVERSION.md for everything unportable.

Verification (parallel lane receipts, this checkout):

• codewhale-mcp 79/79
• codewhale-tui --lib mcp 425/425
• cargo fmt --all -- --check clean
• converter suite 46/46 (6 new bundle-mode tests), proven on the codewhale-cu-plugin/integrations/dsh reference bundle that previously could not round-trip
• ./scripts/sync-changelog.sh + node web/scripts/derive-changelog.mjs run; regenerated files included

Note: includes no changes from the unrelated config/provider/client.rs lane still dirty in the shared tree — only the nine MCP/converter files plus changelog artifacts.

No-Issue: first half of #6280 only — the 2026-07-28 rmcp layer is what closes it, so this PR deliberately does not.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 6m 17s —— View job

---

Claude finished reviewing

Todo list

• Gather context (CLAUDE.md/AGENTS.md, diff vs origin/main, changed files)
• Review MCP negotiation changes (crates/mcp, crates/tui/src/mcp*)
• Review converter bundle-mode changes (scripts/convert-plugin.py, tests)
• Review recommend.rs / plugin_suggestions.rs changes
• Check provider/model route safety invariant
• Post final review

---

Summary

Solid, well-scoped PR overall. The MCP negotiation change is clean and consistent across all four surfaces (mcp crate stdio server, mcp crate stdio client, TUI stdio client, TUI's own server/HTTP client), and each surface has a matching negotiation test. No provider/model-prefix routing logic is touched by this diff. Found one likely-broken new converter test and one real trust-boundary issue in the new bundle-mode path inference.

Correctness

scripts/convert-plugin.py:550-573 — the test_dsh_bundle_relative_entry_and_group_children_convert test looks like it should currently fail.

Trace: for the nested group row {"args": ["server.mjs"], "cwd": "mcp"}, DSH_ENTRY ((?:\./)?[A-Za-z0-9_][A-Za-z0-9_./-]*\.(?:mjs|js|cjs)) fully matches the bare filename "server.mjs" (its character class already allows embedded / and .). So not DSH_ENTRY.fullmatch(arg) at line 552 is False, and the whole cwd-aware root-inference block (lines 563–573, which is the code that would set implicit_roots[name] = bundle / cwd and normalize config["cwd"] = ".") is skipped entirely. root stays None.

stdio_server() (line 283) then immediately raises "Local MCP needs an explicit --stdio-root...", which walk()'s try/except ConversionError (line 598-601) swallows into a skip-note — so "inner" never lands in servers. Since nothing else in that entry list produces a component, files or servers (line ~684) is false too, so convert() raises "No portable components selected...", which propagates out of main() as a non-zero exit (main()'s except (ConversionError, ...) → return 1). The test's self.assertEqual(result.returncode, 0, result.stderr) should then fail with that message in result.stderr.

This sandbox's Bash tool blocked all python3/cargo invocations beyond --version in this session, so I could not execute python3 -m unittest scripts.test_convert_plugin.ConversionTests.test_dsh_bundle_relative_entry_and_group_children_convert to confirm at runtime — please run it directly, since the PR claims "converter suite 46/46." If it does fail, the fix is presumably to drop (or narrow) the not DSH_ENTRY.fullmatch(arg) guard so a bare relative filename combined with a non-default cwd still goes through the bundle / cwd resolution branch.

Security / trust boundary

scripts/convert-plugin.py:550-562 — implicit root inference from an untrusted bundle can pull an arbitrary host directory into the generated plugin.

--bundle mode evaluates a dsh package's own patch file and, when an MCP row's args is a single string that isn't a plain relative entry name, treats it as a host path: if absolute and pointing at an existing .js/.mjs/.cjs file, it silently sets implicit_roots[name] = resolved.parent and later (stdio_files, line 319) copies that entire parent directory into the output plugin — no !!js tag is required, a plain literal string in the patch is enough. Previously, the only way to select a directory for copying was the explicit, user-supplied --stdio-root SERVER=DIRECTORY flag — a case where the operator, not the input file, chooses the path. Bundle mode's whole premise is "consumes a real dsh npm bundle" (i.e., potentially third-party/untrusted content per the PR description), so this lets bundle content — not the operator — pick an arbitrary absolute host path to snapshot. stdio_files only excludes hidden files and a short list of named credential files/extensions, not general secrets, so a crafted or compromised bundle could cause the tool to bundle an unrelated project directory (source, configs, etc.) into a plugin the operator might then install or share. The CONVERSION.md note documents this after the fact, but there's no gate before the copy happens.

Suggest either (a) requiring these paths be confirmed via an explicit --stdio-root (as with every other root selection in this tool), or (b) restricting implicit root inference to paths that resolve inside the bundle directory, and refusing (with a note, not a silent copy) when the resolved entry lives outside it.

Nice choices worth calling out

• crates/mcp/src/lib.rs / stdio_client.rs, crates/tui/src/mcp.rs / mcp_server.rs: negotiation logic is consistent everywhere (echo supported revision, else newest) and each surface got a direct unit test (initialize_negotiates_supported_revisions, initialize_result_requires_supported_protocol..., etc.). MCP_PROTOCOL_VERSION/MCP_SUPPORTED_PROTOCOL_VERSIONS are defined once per crate and reused rather than re-declared per call site.
• crates/tui/src/mcp/streamable_http.rs: correctly scopes the new MCP-Protocol-Version header to the Streamable HTTP transport only (via the McpTransport::set_protocol_version default no-op), since legacy SSE has no header channel — avoids overreaching the abstraction.
• crates/tui/src/core/engine.rs / crates/tui/src/plugins/recommend.rs: RecommendedPluginGate's skill-name snapshot correctly reuses the existing discover_for_workspace_and_dir_with_mode_and_plugins (same catalogue the system prompt indexes) rather than re-deriving skill discovery, and the module doc honestly states the known limitation (mid-session skill installs don't retroactively suppress).
• scripts/convert-plugin.py: !!js handling is capture-never-evaluate (JsExpr marker + explicit allow-list of idioms in lower_js), with a test (test_dsh_bundle_never_executes_js_and_records_unlowerable_rows) actually proving a require('node:fs').writeFileSync(...) payload never runs — good adversarial coverage for the "no source code execution" invariant.

Note

crates/tui/src/core/engine.rs, crates/tui/src/plugins/recommend.rs, and crates/tui/src/tui/plugin_suggestions.rs (the RecommendedPluginGate/#6274 changes) are included in this PR's diff via commit b3127a7 but aren't mentioned in the PR description, which frames this as "only the nine MCP/converter files plus changelog artifacts." Worth double-checking that's intentional inclusion rather than an accidental carry-over from a shared branch tip.