feat: forge doctor subcommand with sandbox/verify/mcp checks#5
Conversation
doanbactam
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add --prompt alias to forge exec for CI compatibility. - Fix attach_mcp_servers to skip failed connections instead of aborting. - Update EventLoop::with_mcp_client signature to &mut self. - Refactor ExecResult to track verify_requested for correct CI exit codes.
| while let Some(line) = reader.next_line().await? { | ||
| if line.trim().is_empty() { | ||
| continue; | ||
| } | ||
| let response = match serde_json::from_str::<JsonRpcRequest>(&line) { | ||
| Ok(request) => handle_request(&mut server, &request).await?, | ||
| Err(e) => JsonRpcResponse { | ||
| jsonrpc: "2.0".into(), | ||
| id: serde_json::Value::Null, | ||
| result: None, | ||
| error: Some(JsonRpcError::parse_error_with(&e.to_string())), | ||
| }, | ||
| }; | ||
| let out = serde_json::to_string(&response)?; | ||
| stdout.write_all(out.as_bytes()).await?; | ||
| stdout.write_all(b"\n").await?; | ||
| stdout.flush().await?; | ||
| } |
There was a problem hiding this comment.
🔴 MCP stdio server sends spurious responses for JSON-RPC notifications, desynchronizing the protocol
The serve function parses every stdin line as a JsonRpcRequest (which requires an id field) and always writes a response to stdout. JSON-RPC 2.0 notifications — like notifications/initialized, which every MCP-compliant client sends after the init handshake — have no id field. This causes deserialization to fail, producing a spurious error response on stdout. Since the client's send_notification (ext/src/mcp/server_process.rs:51-58) does not consume a response, the spurious error sits in the stdout buffer. The next recv_response call (e.g. during list_tools()) reads the error instead of the real response, breaking the protocol.
Trace of failure with Forge's own McpClient
- Client sends
initializerequest (with id) → server responds correctly - Client sends
notifications/initializednotification (noidfield) viasend_notification - Server fails to parse as
JsonRpcRequest→ writes parse-error response to stdout - Client calls
list_tools()→send_and_recvreads the spurious error from step 3 list_toolsfails with "No result" because the response haserrorand noresult
Prompt for agents
The serve function in forge-cli/src/mcp.rs always sends a response for every line, but JSON-RPC 2.0 notifications (messages without an id field) must NOT receive a response. The fix should: (1) attempt to parse the line as a generic serde_json::Value first, (2) check whether the id field is present, (3) if no id is present, treat it as a notification — dispatch it to handle_request (or a notification handler) but do NOT write any response to stdout, (4) only write a response when the incoming message is a proper request (has an id). This matches the JSON-RPC 2.0 spec and prevents desynchronizing the request/response stream. The JsonRpcNotification type already exists in ext/src/mcp/protocol.rs and can be used for deserialization of notification messages.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for (label, server) in &mcp.servers { | ||
| if let Err(e) = event_loop | ||
| .with_mcp_client(server.command.clone(), server.args.clone()) | ||
| .await | ||
| { | ||
| tracing::warn!( | ||
| "Failed to connect MCP server '{}': {} — skipping", | ||
| label, | ||
| e | ||
| ); | ||
| } else { | ||
| tracing::info!("Connected MCP server '{}'", label); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Multiple MCP server connections are silently overwritten - only last server's tools survive
attach_mcp_servers in forge-cli/src/exec.rs:244 iterates over all configured [mcp.servers.*] entries and calls event_loop.with_mcp_client() for each one. However, with_mcp_client at forge-core/src/event_loop.rs:422-423 unconditionally assigns self.mcp_tools = tools and self.mcp_client = Some(...), replacing any previously connected server. This means only the last successfully connected server's tools and client are retained; all earlier connections are silently dropped.
When the agent later encounters a tool from an earlier server (via the _ match arm at line 646), it dispatches to self.mcp_client which now points to the wrong server, causing the tool call to fail or target the wrong server.
Affected code path
In exec.rs:
for (label, server) in &mcp.servers {
event_loop.with_mcp_client(server.command.clone(), server.args.clone()).await
}In event_loop.rs:
self.mcp_tools = tools; // replaces all previous tools
self.mcp_client = Some(...); // replaces previous client
Was this helpful? React with 👍 or 👎 to provide feedback.
Forge PR Review/home/runner/work/_temp/1660e80d-8bc2-4676-aeb6-98f5911536da.sh: line 5391: ./target/release/forge: Argument list too long |
| { | ||
| "sessionID": "ses_13136a1baffe6TZR026eK7VUx7", | ||
| "updatedAt": "2026-06-16T05:03:56.792Z", | ||
| "sources": { | ||
| "background-task": { | ||
| "state": "active", | ||
| "reason": "3 background task(s) active", | ||
| "updatedAt": "2026-06-16T05:03:56.792Z" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🚩 Accidental commit of .omo session tracking file
The file .omo/run-continuation/ses_13136a1baffe6TZR026eK7VUx7.json is a development tooling artifact (session state tracking) committed to the repository. It contains no secrets but shouldn't be version-controlled. Consider adding .omo/ to .gitignore.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
error: unexpected argument '--prompt' found
tip: to pass '--prompt' as a value, use '-- --prompt'
Usage: forge exec [OPTIONS]
For more information, try '--help'.
Description generation failed