Skip to content

[mxc] - Execute MCP tools through MXC-constrained process executor #1477

Description

@JoshuaRowePhantom

Part of #1471

Dependencies

Summary

Route every stdio MCP server through a Phantom-owned ProcessExecutorBackedClientTransport. The transport always uses #1474: a null policy selects its ordinary-process branch and a compiled policy selects MXC. It adapts executor-owned stdin/stdout to the ModelContextProtocol SDK's public stream transport, owns process lifecycle, drains stderr separately, and fails closed. Remote MCP continues using #1438 routing, but the launch host resolves and compiles the trust profile locally. Constrained HTTP/SSE is rejected because no child process exists to sandbox.

Root Cause

AgentFactory.cs:540-542,1178-1195 resolves an effective profile but discards it after local-execution authorization. McpToolContextProvider.cs:142-178 creates local or remote MCP clients without trust context. McpTransportFactory.cs:494-552 constructs sealed StdioClientTransport, which launches System.Diagnostics.Process internally and has no executor hook. McpConnectionRequest and RemoteMcpHostHandler likewise carry no host-resolvable trust-profile reference.

ModelContextProtocol 1.4.0 exposes the required public seam: IClientTransport has Name and Task<ITransport> ConnectAsync(CancellationToken), while StreamClientTransport(Stream serverInput, Stream serverOutput, ILoggerFactory?) adapts established streams and handles MCP newline-delimited JSON-RPC framing. Therefore Phantom need not reimplement protocol parsing.

Affected Files

File Required Change
Phantom.Workspaces.Llm.Core/AgentFactory.cs / AgentChat.cs Retain an execution trust context for MCP providers.
New Trust/AgentExecutionTrustContext.cs Hold local effective profile and remote source reference/revision; lazily cache compilation once per host/session.
Phantom.Workspaces.Llm.Core/McpToolContextProvider.cs Pass trust context through local and remote paths.
Phantom.Workspaces.Llm.Core/Mcp/McpTransportFactory.cs Use the process-backed transport for every stdio endpoint; reject constrained HTTP/SSE.
New Mcp/ProcessExecutorBackedClientTransport.cs Implement IClientTransport, launch via #1474, adapt streams, and own process lifecycle.
New Mcp/ProcessOwnedMcpTransport.cs Delegate ITransport while coupling disposal/exit diagnostics to the process handle.
Phantom.Workspaces.Transport/Mcp/McpConnectionRequest.cs Carry a trust-profile reference plus expected revision, never compiled MXC policy.
Phantom.Workspaces/Services/RemoteMcpHostHandler.cs Resolve/compose/compile on the remote host and call the same factory.
Composition and tests Register compiler/executor and verify local/remote behavior.

Design / Fix

Session trust context

Introduce AgentExecutionTrustContext (do not reuse the existing AgentSessionTrustProfile, which contains tool delegates). It contains:

  • the locally resolved effective TrustProfile, when this process is the launch host;
  • the selected trust-profile reference and expected entity revision for remote dispatch;
  • a thread-safe lazy TrustProfileProcessPolicyCompilation cache.

Agent construction resolves once. All MCP providers for the session share the context. Compilation occurs once on the machine that launches the process.

For remote MCP, add only the selected profile reference and expected revision to McpConnectionRequest. RemoteMcpHostHandler resolves through its local repository/provider, rejects missing or stale revisions, composes the effective profile, and compiles locally. The wire model has no compiled-policy field; reject unknown attempts to provide one.

Concrete stdio transport

McpTransportFactory parses the existing stdio URI into command, ordered args, cwd, and environment, then always constructs:

new ProcessExecutorBackedClientTransport(
    name,
    processRequest,
    processExecutor,
    loggerFactory)

processRequest.Policy is null when #1475 says containment is unnecessary and is the compiled MxcProcessPolicy otherwise. This removes the split between SDK-owned uncontained launch and Phantom-owned contained launch; #1474 owns both branches consistently.

ProcessExecutorBackedClientTransport : IClientTransport implements:

public string Name { get; }
public Task<ITransport> ConnectAsync(CancellationToken cancellationToken = default);

ConnectAsync may be called once. It launches the process, starts stderr draining and exit monitoring, creates:

var streamTransport = new StreamClientTransport(
    process.StandardInput,
    process.StandardOutput,
    loggerFactory);
var inner = await streamTransport.ConnectAsync(cancellationToken);

and returns ProcessOwnedMcpTransport, which delegates SessionId, MessageReader, and SendMessageAsync to inner. On disposal it disposes the inner MCP transport, closes stdin, disposes the executor handle (killing a still-running tree), and awaits stderr/exit tasks. If setup fails after launch, dispose the handle before rethrowing. No MXC failure retries without policy.

Do not parse JSON-RPC in Phantom. StreamClientTransport owns UTF-8 newline-delimited framing. Child stdout is protocol-only. Drain child stderr concurrently with a no-BOM UTF-8 line reader into the existing logger and a bounded rolling diagnostic buffer; stderr never enters StreamClientTransport. A nonzero premature exit completes/faults the transport with exit code plus sanitized rolling stderr. Cancellation of ConnectAsync disposes any partially launched process.

Windows command resolution

Preserve commands such as npx without making all launches shell-based. Add a resolver that searches PATH/PATHEXT. Launch .exe directly. For resolved .cmd/.bat, launch %ComSpec% with /d /s /c and one command line built by the tested Windows quoting helper from #1474. Preserve ordered arguments, cwd, and the existing inherited/overridden environment semantics. Resolution failure is explicit before launch.

HTTP/SSE and authorization

If an effective profile requires process containment, reject HTTP/SSE MCP endpoints with an unsupported-policy error: Phantom does not own the remote server process and sandboxing the client connection would require containing the whole application. Never report HTTP/SSE as MXC-protected. Unconstrained HTTP/SSE remains unchanged, including OAuth and #1438 routing.

Tool-call JSON-schema authorization remains independent and applies in addition to process containment. DACL mutation is permitted.

Expected Tests

Test Name Class What It Verifies
CreateStdioTransport_UnconstrainedProfile_UsesExecutorWithNullPolicy McpTransportFactoryTests Every stdio launch uses #1474 and null selects ordinary execution.
CreateStdioTransport_ConstrainedProfile_UsesExecutorWithMxcPolicy McpTransportFactoryTests A constrained profile supplies compiled policy to the same transport.
ConnectAsync_SecondCall_ThrowsInvalidOperation ProcessExecutorBackedClientTransportTests A transport instance cannot launch duplicate server processes.
ConnectAsync_Streams_McpHandshakeCompletes ProcessExecutorBackedClientTransportTests Executor streams work through SDK StreamClientTransport framing.
ConnectAsync_StderrOutput_DoesNotEnterProtocolStream ProcessExecutorBackedClientTransportTests Stderr is drained/logged separately from JSON-RPC stdout.
DisposeAsync_RunningServer_KillsProcessTreeAndDrainsTasks ProcessExecutorBackedClientTransportTests Disposal owns the complete process lifecycle.
ConnectAsync_MxcLaunchFails_DoesNotRetryWithoutPolicy ProcessExecutorBackedClientTransportTests Required containment fails closed.
ResolveCommand_CmdShim_UsesComSpecWithQuotedArguments StdioCommandResolverTests Windows npm-style shims retain argv semantics.
CreateHttpTransport_ConstrainedProfile_ReturnsUnsupportedPolicy McpTransportFactoryTests Constrained HTTP/SSE is explicitly rejected.
RemoteStdio_ProfileReference_ResolvesAndCompilesOnLaunchHost RemoteMcpHostHandlerTests The remote host performs authoritative policy compilation.
RemoteStdio_StaleTrustProfileRevision_RejectsLaunch RemoteMcpHostHandlerTests A changed profile cannot silently launch under divergent policy.
RemoteRequest_CompiledPolicyProperty_IsNotAccepted McpConnectionRequestTests The transport contract cannot inject compiled MXC policy.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't workingdiagnosedRoot cause identifiedneeds-slow-testsRequires full test suite including slow Git tests at checkinverified-locallyImplementation has been verified locally

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions