adityamparikh opened a new pull request, #212: URL: https://github.com/apache/solr-mcp/pull/212
### The bug The remediation hints on `search` failures never reach an MCP client. The MCP annotation layer reports the **root cause's** message as the tool error text — `AbstractSyncMcpToolMethodCallback` walks `getCause()` to the root and uses that message for the `CallToolResult`, with `toolCallExceptionClass` defaulting to `Exception`. Attaching the Solr failure as a cause therefore discards the hint. Against `main`, a search with an undefined facet field returns this to the client: ``` Error from server at http://localhost:64505/solr/shows/select: undefined field: "nonexistent_field_xyz" ``` No hint, so the model has no next step and retries blind. ### The fix Carry the hints in the message and drop the cause, at the three sites in `withRemediationHint`. The original Solr message is unchanged, so the client still learns *which* field was wrong. The diagnostic log moves from `debug` to `warn`: with no cause in the chain it is now the only server-side record of the failure. A javadoc line and a comment state the no-cause invariant at the point where a future contributor would reflexively re-add `, e` and silently break the hints again. ### Why the test is at the MCP level Every existing hint test asserts on the exception object at the service layer, which is precisely where the hint still looks correct — none of them cross the MCP boundary where it was lost. `searchFailureIsAnActionableMcpToolError` runs over both transports via `McpClientIntegrationTestBase`, and fails on `main` with the message quoted above. ### Verification Full `./gradlew build`: **423 tests, 0 failures, 0 skipped.** Confirmed from the JUnit XML rather than the build banner, since a Docker outage would skip every container test and still report success. Native images and non-default Solr versions were not exercised. ### Relationship to #195 #195 identified the same root bug. This change is the narrow fix for it, and leaves two orthogonal questions in that PR for separate discussion: replacing Solr's message text with generic guidance (which also removes the field name the model needs, and inverts `SearchServiceTest`'s existing "original Solr message must be preserved" assertion), and filtering blank entries from the optional list arguments. Worth noting for the first of those: Solr's message does disclose the backend URL to the client, which is a real concern worth addressing on its own terms against the threat model. The same cause-unwrapping defect affects 13 further sites in `JsonResponseParser` and the document creators; filed separately, since the fix there differs — the root message is the useful part, so it should be folded into the wrapper's message rather than the wrapper's text preserved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
