Improve MCP failure diagnostics#71
Open
TomProkop wants to merge 3 commits into
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves txc MCP failure diagnostics by keeping the immediate tool failure response concise while attaching a fetchable, structured diagnostics resource (JSON). It also centralizes MCP result construction in a dedicated factory and adds test coverage for the new resource-based failure-details flow.
Changes:
- Introduced
McpToolResultFactoryto build MCPCallToolResultresponses and expose failure diagnostics as MCP resources. - Updated
ToolLogStoreto store structured failure details (exit code, summary, error summary, full log) and serialize them as JSON resources. - Added unit + integration tests validating that failed tool calls return a readable
resource_linkand that the linked resource can be fetched and parsed.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/TALXIS.CLI.Tests/MCP/ToolLogStoreTests.cs | Updated tests for the new StoreFailure API and expanded assertions for stored failure metadata. |
| tests/TALXIS.CLI.Tests/MCP/McpToolResultFactoryTests.cs | Added unit tests verifying failure and exception results attach a readable JSON diagnostics resource. |
| tests/TALXIS.CLI.IntegrationTests/McpTests.cs | Added integration coverage ensuring a failed tool call exposes a fetchable failure-details resource. |
| tests/TALXIS.CLI.IntegrationTests/McpTestClient.cs | Extended test client wrapper with resources list/read APIs. |
| src/TALXIS.CLI.MCP/ToolLogStore.cs | Refactored store entries to represent failure details and added JSON serialization for resource reads. |
| src/TALXIS.CLI.MCP/Program.cs | Switched tool result building + resource handlers to use McpToolResultFactory. |
| src/TALXIS.CLI.MCP/McpToolResultFactory.cs | New factory encapsulating MCP success/failure result assembly and resource exposure. |
| src/TALXIS.CLI.MCP/CliSubprocessRunner.cs | Added an internal constructor to support explicit stdout/stderr-derived diagnostics in tests. |
| global.json | Added rollForward: latestFeature for the pinned .NET SDK version. |
Comments suppressed due to low confidence (1)
src/TALXIS.CLI.MCP/Program.cs:767
- These resource handler comments still mention "execution logs" / "full execution log", but the implementation now lists/reads structured failure-details JSON resources via McpToolResultFactory. Updating the comments will keep the MCP resource contract clear (especially the MIME type/content expectations).
// MCP resource listing — exposes stored tool execution logs
ValueTask<ListResourcesResult> ListResourcesAsync(RequestContext<ListResourcesRequestParams> ctx, CancellationToken ct)
{
return ValueTask.FromResult(new ListResourcesResult { Resources = toolResultFactory.BuildResources() });
}
// MCP resource read — returns the full execution log for a given URI
ValueTask<ReadResourceResult> ReadResourceAsync(RequestContext<ReadResourceRequestParams> ctx, CancellationToken ct)
| @@ -18,6 +18,7 @@ | |||
|
|
|||
| // In-memory store for tool execution logs, exposed as MCP resources | |||
| var store = new ToolLogStore(); | ||
| store.Store("tool_a", "log a", "", isError: false); | ||
| store.Store("tool_b", "log b", "err", isError: true); | ||
| store.StoreFailure("tool_a", 0, "summary a", "", "log a"); |
Comment on lines
+94
to
+109
| public string ToJson() | ||
| { | ||
| var document = new FailureDetailsDocument | ||
| { | ||
| Kind = "tool-failure-details", | ||
| ToolName = ToolName, | ||
| ExitCode = ExitCode, | ||
| Summary = Summary, | ||
| PrimaryText = PrimaryText, | ||
| ErrorSummary = ErrorSummary, | ||
| FullLog = FullLog, | ||
| Timestamp = Timestamp | ||
| }; | ||
|
|
||
| return JsonSerializer.Serialize(document, TxcOutputJsonOptions.Default); | ||
| } |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Validation: