ROCMAI-83: extract mcp.rs from apps/rocmd/src/lib.rs

[jussielo-amd]

Summary

Sixth PR of Phase 5 (ROCMAI-83, part of the modularization epic ROCMAI-27). Pulls the MCP stdio server, tool schema table, tool dispatch, and the rocm-subprocess capture/argv-building helpers behind the MCP tools into their own mcp.rs module.

• No behavior change. Reaches back into still-crate-root items (build_bridge_snapshot, gather_gpu_snapshot_for_config, bridge_engine_inventory, load_managed_services, stop_managed_service) via crate::, since the modules that will eventually own those (common.rs/persistence.rs/service.rs) are separate, independent PRs not present on this branch (see ROCMAI-83: extract persistence.rs from apps/rocmd/src/lib.rs #477, ROCMAI-83: extract common.rs from apps/rocmd/src/lib.rs #479, ROCMAI-83: extract webhook.rs from apps/rocmd/src/lib.rs #480, ROCMAI-83: extract cli.rs from apps/rocmd/src/lib.rs #481, ROCMAI-83: extract sandbox.rs from apps/rocmd/src/lib.rs #483).
• CommandCapture now lives in mcp.rs (it was mcp.rs-authored originally); sandbox-territory code still inline in lib.rs that takes it as a parameter type is repointed to mcp::CommandCapture.
• 15 tests that exercise this module's own logic (MCP tool-schema/dispatch shape, read-only-verb classification, install_sdk/install_engine/launch_server/watcher_enable argv building, and read_tail_lines) moved into mcp.rs's own #[cfg(test)] mod in this same PR. Tests that share a name prefix or construct these types but actually exercise Cli parsing, run_daemon, or watcher-domain logic stayed in lib.rs for their own later PRs.
• docs/architecture.md updated in this PR.

Independent of #477, #479, #480, #481, #483 — branched separately from main, not stacked, per the one-off PR approach for this phase.

Test plan

• cargo build (rocmd, and rocm+rocmd together to confirm the external API surface is untouched)
• cargo clippy --workspace --all-targets -- -D warnings
• cargo test -p rocmd — 130 tests, same count as before this change (129 passed + 1 pre-existing ignored)
• cargo xtask manifest --check
• cargo fmt / prek hooks clean
• tests/e2e-cucumber (CI)

[r0x0r]

Review (draft): MCP surface motion is clean, but CommandCapture is in the wrong module

The MCP extraction itself is solid and I found nothing wrong with the tool-surface motion. There is one placement decision that collides with #479 and #483, and it is worth settling before this comes out of draft — it is cheaper to move one struct now than to unpick it after either sibling merges.

CommandCapture belongs in common.rs, not mcp.rs

This PR puts pub(crate) struct CommandCapture at mcp.rs:846, on the grounds that it "was mcp.rs-authored originally". #479 puts the same struct at common.rs:204. Both are open against main.

Origin is the wrong test, and the phase already has the right one: common.rs admits anything used by ≥2 of the sandbox/mcp/service/watchers clusters. CommandCapture meets it on main, with the two consumer groups cleanly separated:

Cluster	Consumers on main	Destination PR
sandbox	sandbox_check_updates_value (lib.rs:1194), update_check_status (:1212), sandbox_driver_plan_value (:1252)	#483 → sandbox.rs
MCP	tool_result_from_command (:2332), command_capture_text (:2351), run_rocm_capture (:2373)	#484 → mcp.rs

So it is used by exactly two clusters, which is the rule, and #479's placement is the one that follows it.

The consequence is visible on this branch already: lib.rs:1197, :1215 and :1255 — all three sandbox-cluster functions — now say mcp::CommandCapture. Those three lines are precisely what #483 moves into sandbox.rs. If this lands first, #483's sandbox.rs has to reach for crate::mcp::CommandCapture, giving the sandbox module a hard dependency on the MCP module. That is the cross-cluster coupling Phase 5 exists to remove, and #483's body already assumes otherwise — it lists CommandCapture among the crate-root items it reaches back for, not an mcp:: item.

Worth noting this PR's own doc hunk agrees with #479 and not with its own code: it describes common.rs as holding "CommandCapture/command-timeout plumbing" while the code puts it in mcp.rs.

The fix is small: drop CommandCapture, run_rocm_capture, run_rocm_capture_for_paths and run_command_with_timeout from this PR and let #479 own them; keep tool_result_from_command and command_capture_text here, since those are genuinely MCP result-shaping and have no sandbox consumer.

How this merges with #479 today

Not silently, which is the one piece of good news — but not cleanly either. Both PRs edit the same signatures (sandbox_check_updates_value, update_check_status, sandbox_driver_plan_value) with different module paths, which is a plain text conflict; and #479 edits tool_result_from_command in place while this PR deletes it from lib.rs, which is an edit/delete conflict. Both surface at merge time rather than slipping through. The catch is that resolving them correctly requires knowing which home is right, so whoever hits the conflict inherits this decision at the worst moment. Settling it now turns a judgement call into a mechanical rebase.

The architecture-doc hunk, same as the rest of the phase

The doc text lists six extracted modules; this branch has one (apps/rocmd/src/ here is lib.rs, main.rs, mcp.rs). All six PRs in this phase rewrite the same single line, each branched independently from main, with cumulative text that is only true at merge order 477 → 479 → 480 → 481 → 483 → 484. Any of them merging out of order applies cleanly and leaves main asserting files that do not exist. Scoping each hunk to its own module fixes it at any order and turns the conflicts into trivial appends. Same note as on #479, #480, #481 and #483.

Verified clean

• MCP tool schema is byte-for-byte identical — all 16 tool schemas in rocm_mcp_tools(), every tool name, description, parameter name and required/optional flag, and every readOnlyHint/destructiveHint annotation. A silently renamed tool or dropped required parameter here would be an API break for any MCP client with no compile error to catch it, so it is worth stating that there is none.
• The approval gate is unchanged — mcp_tool_requires_direct_approval has the same six mutating-tool names, and ensure_rocm_command_is_read_only has the same branch arms. I checked set membership specifically: a tool drifting from the mutating set into the read-only set would be a privilege escalation (AGENTS.md §7 requires mutating actions to keep their approval flow), and none did.
• argv builders unchanged — build_install_sdk_args, build_install_engine_args, build_launch_server_args, build_watcher_enable_args; no flag added, dropped or reordered. read_tail_lines bounds arithmetic is identical.
• ensure_direct_mcp_call_allowed widens from private to pub(crate), which the extraction requires since lib.rs:371 calls it. The gate's logic is byte-identical — the visibility change is mechanical, not a loosening.
• Tests — 130 attributes conserved across the branch (115 + 15), matching the claimed 15 moved exactly; no #[ignore] added or dropped.
• Imports — VecDeque and BufRead dropped from lib.rs; I confirmed zero remaining references to either, so nothing fails under -D warnings. All five crate:: reach-backs (build_bridge_snapshot, gather_gpu_snapshot_for_config, bridge_engine_inventory, load_managed_services, stop_managed_service) are private in lib.rs and legally reachable from a child module.
• No TODOs, stubs or half-moved clusters despite the draft status.

---

🤖 by agent-hub on AMD AgentHub