feat(agent-adapter,engine,daemon): simulator MCP tools for every MCP-capable agent (CODE-395)#254
feat(agent-adapter,engine,daemon): simulator MCP tools for every MCP-capable agent (CODE-395)#254AprilNEA wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc4d881a07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| z.object({ | ||
| kind: z.literal('simulator.activity'), | ||
| sessionId: SessionIdSchema, | ||
| udid: z.string().min(1).optional(), | ||
| tool: z.string(), | ||
| phase: z.enum(['started', 'settled']), |
There was a problem hiding this comment.
Bump the wire protocol for simulator activity
Adding simulator.activity while retaining protocol version 44 lets an older v44 client complete the handshake but silently reject these broadcasts, so the activity badge never updates and the incompatible peer is not detected. Increment WIRE_PROTOCOL_VERSION whenever introducing this wire variant.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| /** Append the session's simulator MCP endpoint for agents whose SDK can consume it. */ | ||
| private withSimulatorMcp(options: StartOptions, sessionId: SessionId): StartOptions { | ||
| if (!this.simulatorMcp || !MCP_CAPABLE_AGENT_KINDS.has(options.kind)) return options; | ||
| const endpoint = this.simulatorMcp.endpointFor(sessionId); |
There was a problem hiding this comment.
Revoke MCP tokens when session startup fails
When translation resolution or adapter startup fails after this call, the session never reaches normal teardown: SessionOrchestrator.discardFailedStart removes the session without invoking onStopped, and a translation failure occurs before startLive entirely. Consequently the endpoint's token maps grow on every failed attempt, and a partially started adapter that received the URL retains a valid simulator endpoint for a rejected session; release the token on every resolver/start failure as well as normal stops.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR gives MCP-capable agents access to session-scoped simulator tools. The main changes are:
Confidence Score: 4/5The change requires attention before merging. Normal session ownership and token release are implemented consistently, but pre-live failures do not consistently roll back minted tokens. Duplicate endpoint names can silently replace caller-provided MCP configuration, and large or aborted POST bodies can consume daemon memory and connections. packages/host/engine/src/session/start-options-resolver.ts and apps/daemon/src/sim/mcp-endpoint.ts
What T-Rex did
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Engine
participant Resolver
participant Endpoint
participant Agent
participant Simulator
Client->>Engine: Start session
Engine->>Resolver: Resolve options with session ID
Resolver->>Endpoint: Mint tokenized endpoint
Resolver-->>Engine: Options with MCP server
Engine->>Agent: Start with MCP configuration
Agent->>Endpoint: Tool request with token
Endpoint->>Simulator: Operation under session ownership
Simulator-->>Endpoint: Result or ownership error
Endpoint-->>Agent: MCP response
Engine->>Endpoint: Release token on normal stop
Reviews (1): Last reviewed commit: "feat(agent-adapter,engine,daemon): simul..." | Re-trigger Greptile |
| /** Append the session's simulator MCP endpoint for agents whose SDK can consume it. */ | ||
| private withSimulatorMcp(options: StartOptions, sessionId: SessionId): StartOptions { | ||
| if (!this.simulatorMcp || !MCP_CAPABLE_AGENT_KINDS.has(options.kind)) return options; | ||
| const endpoint = this.simulatorMcp.endpointFor(sessionId); |
There was a problem hiding this comment.
Failed Starts Retain MCP Tokens
endpointFor stores a bearer token while options are being resolved, before translator startup, workspace setup, and adapter startup can fail. Those failure paths do not all invoke the normal stop callback, so each failed start can leave a valid endpoint and two map entries until daemon shutdown.
Artifacts
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
- Keeps the command output available without making the summary code-heavy.
| if (!this.simulatorMcp || !MCP_CAPABLE_AGENT_KINDS.has(options.kind)) return options; | ||
| const endpoint = this.simulatorMcp.endpointFor(sessionId); | ||
| if (!endpoint) return options; | ||
| return { ...options, mcpServers: [...(options.mcpServers ?? []), endpoint] }; |
There was a problem hiding this comment.
Injected Name Overwrites User Server
When the caller already supplies an MCP server named linkcode-sim, appending the built-in endpoint creates a duplicate name. Each changed adapter converts this array into keyed configuration with last-entry-wins behavior, so the injected endpoint silently replaces the caller's server and its tools.
Artifacts
Repro: focused resolver and Claude adapter collision test
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: focused Vitest configuration used to execute the test
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
- Keeps the command output available without making the summary code-heavy.
| return new Promise((resolve, reject) => { | ||
| const chunks: Buffer[] = []; | ||
| req.on('data', (chunk: Buffer) => chunks.push(chunk)); | ||
| req.on('end', () => resolve(Buffer.concat(chunks).toString('utf8'))); | ||
| req.on('error', reject); | ||
| }); |
There was a problem hiding this comment.
Request Bodies Grow Without Bound
A local MCP client can stream an arbitrarily large POST, which retains every chunk and then allocates the combined buffer. A client that disconnects before end can also leave the promise pending because aborted and premature close are not handled, allowing faulty or compromised agent processes to exhaust daemon memory or connections.
Summary
Stacked on #251. Agents get the simulator through MCP — the CODE-93 passthrough plus the daemon's built-in endpoint, in one atomic change (the pieces are cross-package-interdependent):
StartOptions.mcpServers(stdio + http) now reaches claude-code (options.mcpServersrecord), codex (thread/startconfigdotted keysmcp_servers.<name>.*; an http entry armsexperimental_use_rmcp_client; http headers have no config key → loud reject), and opencode (createOpencodeconfig.mcp, http→remote/ stdio→local). pi rejects a non-emptymcpServersloudly — and the engine never injects for it.SessionStartOptionsResolverappends the per-session endpoint forMCP_CAPABLE_AGENT_KINDSon every start/resume path;EngineDepsnow takes the daemon-constructedSimulatorService(+SimulatorMcpProvider) so the MCP endpoint and the engine share one claims registry; session stop releases claims and tokens.@modelcontextprotocol/sdk), one session-bound token per session, stateless per-request servers. Tools:sim_list_devices/boot/shutdown/install/launch/terminate/open_url/screenshot— screenshot returns MCP image content, so the capture lands in the agent's context. Ownership/caps enforced via the shared service;simulator.activitybroadcasts (new wire variant, still protocol 44) drive the panel's “agent is driving this device” badge, and MCP-path boot/shutdown pushsimulator.devices.changedlike the wire handler does.Verification
conflict, token revocation).pnpm check:ci+pnpm test(1785) green.sim_list_devices → sim_boot (iPhone 17) → sim_screenshot → sim_shutdownthrough the injected endpoint — activity and devices.changed broadcasts observed client-side. The codex run also live-verifies themcp_servers.*dotted-key + rmcp-client bet. opencode passthrough is SDK-typed and unit-tested; live run pending an opencode install.