diff --git a/global.json b/global.json index 0370976..998e2e4 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,6 @@ { "sdk": { - "version": "10.0.105" + "version": "10.0.105", + "rollForward": "latestFeature" } } diff --git a/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs b/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs index 9225755..08ea9c5 100644 --- a/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs +++ b/src/TALXIS.CLI.MCP/CliSubprocessRunner.cs @@ -208,6 +208,15 @@ public CliSubprocessResult(int exitCode, string output) Output = output; } + /// Creates a result with explicit stdout/stderr-derived diagnostic fields. + internal CliSubprocessResult(int exitCode, string output, string lastErrors, string fullLog) + { + ExitCode = exitCode; + Output = output; + LastErrors = lastErrors; + FullLog = fullLog; + } + /// Creates a result from a streaming output handler (uses stdout content as output). public CliSubprocessResult(int exitCode, ISubprocessOutputHandler handler) { diff --git a/src/TALXIS.CLI.MCP/McpToolResultFactory.cs b/src/TALXIS.CLI.MCP/McpToolResultFactory.cs new file mode 100644 index 0000000..416666f --- /dev/null +++ b/src/TALXIS.CLI.MCP/McpToolResultFactory.cs @@ -0,0 +1,127 @@ +using ModelContextProtocol; +using ModelContextProtocol.Protocol; +using TALXIS.CLI.Logging; + +namespace TALXIS.CLI.MCP; + +/// +/// Builds MCP tool results and exposes failed tool diagnostics as fetchable resources. +/// +internal sealed class McpToolResultFactory +{ + private readonly ToolLogStore _toolLogStore; + + public McpToolResultFactory(ToolLogStore toolLogStore) + { + _toolLogStore = toolLogStore; + } + + public CallToolResult Build(string toolName, CliSubprocessResult result) + { + if (result.ExitCode == 0) + { + return new CallToolResult + { + Content = [new TextContentBlock { Text = result.Output }] + }; + } + + string summary = BuildFailureSummary(toolName, result.Output, result.LastErrors, result.ExitCode); + string diagnosticsUri = _toolLogStore.StoreFailure( + toolName, + result.ExitCode, + summary, + result.LastErrors, + result.FullLog); + + return BuildFailureResult(toolName, summary, diagnosticsUri); + } + + public CallToolResult BuildExceptionResult(string toolName, Exception exception) + { + string summary = string.IsNullOrWhiteSpace(exception.Message) + ? $"Tool '{toolName}' failed before execution completed." + : LogRedactionFilter.Redact(exception.Message); + string diagnosticsUri = _toolLogStore.StoreFailure( + toolName, + -1, + summary, + summary, + LogRedactionFilter.Redact(exception.ToString())); + + return BuildFailureResult(toolName, summary, diagnosticsUri); + } + + public List BuildResources() + { + return _toolLogStore.ListAll().Select(e => new Resource + { + Uri = e.Uri, + Name = $"Failure details: {e.Entry.ToolName}", + Description = BuildResourceDescription(e.Entry), + MimeType = "application/json" + }).ToList(); + } + + public ReadResourceResult ReadResource(string uri) + { + if (!_toolLogStore.TryGet(uri, out var entry) || entry is null) + { + throw new McpException($"Resource not found: {uri}"); + } + + return new ReadResourceResult + { + Contents = [new TextResourceContents { Uri = uri, MimeType = "application/json", Text = entry.ToJson() }] + }; + } + + private static CallToolResult BuildFailureResult(string toolName, string summary, string diagnosticsUri) + { + return new CallToolResult + { + IsError = true, + Content = + [ + new TextContentBlock { Text = summary }, + new ResourceLinkBlock + { + Uri = diagnosticsUri, + Name = $"Failure details for {toolName}", + Description = "Fetch structured diagnostics for this failed tool call via resources/read.", + MimeType = "application/json" + } + ] + }; + } + + private static string BuildFailureSummary(string toolName, string output, string lastErrors, int exitCode) + { + if (!string.IsNullOrWhiteSpace(output)) + { + return output.Trim(); + } + + var firstErrorLine = lastErrors + .Split([Environment.NewLine, "\n"], StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .FirstOrDefault(); + + return !string.IsNullOrWhiteSpace(firstErrorLine) + ? firstErrorLine + : $"Tool '{toolName}' failed with exit code {exitCode}."; + } + + private static string BuildResourceDescription(ToolLogStore.LogEntry entry) + { + var singleLineSummary = entry.Summary + .ReplaceLineEndings(" ") + .Trim(); + + if (singleLineSummary.Length > 160) + { + singleLineSummary = singleLineSummary[..157] + "..."; + } + + return $"{singleLineSummary} Use resources/read to retrieve detailed diagnostics."; + } +} diff --git a/src/TALXIS.CLI.MCP/Program.cs b/src/TALXIS.CLI.MCP/Program.cs index c7be0aa..46c038d 100644 --- a/src/TALXIS.CLI.MCP/Program.cs +++ b/src/TALXIS.CLI.MCP/Program.cs @@ -16,8 +16,9 @@ RootsService? rootsService = null; IHostApplicationLifetime? appLifetime = null; -// In-memory store for tool execution logs, exposed as MCP resources +// In-memory store for structured failure details, exposed as MCP resources var toolLogStore = new ToolLogStore(); +var toolResultFactory = new McpToolResultFactory(toolLogStore); // Per-output-path lock for workspace_component_create to prevent concurrent writes to the same project var workspaceOutputLocks = new System.Collections.Concurrent.ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); @@ -340,7 +341,7 @@ async Task ExecuteCliToolAsync( mcpLogger.LogInformation("Tool completed: {ToolName} (exit code {ExitCode})", toolName, result.ExitCode); - return BuildToolResult(toolName, result); + return toolResultFactory.Build(toolName, result); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -348,7 +349,7 @@ async Task ExecuteCliToolAsync( } catch (Exception ex) { - return new CallToolResult { Content = [new TextContentBlock { Text = LogRedactionFilter.Redact(ex.ToString()) }], IsError = true }; + return toolResultFactory.BuildExceptionResult(toolName, ex); } finally { @@ -423,7 +424,7 @@ async ValueTask ExecuteAsTaskAsync( mcpLogger.LogInformation("Task completed: {ToolName} (exit code {ExitCode}, taskId: {TaskId})", toolName, result.ExitCode, mcpTask.TaskId); - var callToolResult = BuildToolResult(toolName, result); + var callToolResult = toolResultFactory.Build(toolName, result); var finalStatus = result.ExitCode != 0 ? McpTaskStatus.Failed : McpTaskStatus.Completed; var resultElement = System.Text.Json.JsonSerializer.SerializeToElement(callToolResult); @@ -446,11 +447,7 @@ async ValueTask ExecuteAsTaskAsync( { try { - var errorResult = new CallToolResult - { - IsError = true, - Content = [new TextContentBlock { Text = $"Task execution failed: {LogRedactionFilter.Redact(ex.ToString())}" }] - }; + var errorResult = toolResultFactory.BuildExceptionResult(toolName, ex); var errorElement = System.Text.Json.JsonSerializer.SerializeToElement(errorResult); var failedTask = await taskStore.StoreTaskResultAsync( mcpTask.TaskId, McpTaskStatus.Failed, errorElement, sessionId, CancellationToken.None); @@ -760,75 +757,18 @@ async Task ExecuteMcpSpecificToolWithCapturedOutputAsync(Type co } } -// Build a CallToolResult, including a resource_link to the full log on failure -CallToolResult BuildToolResult(string toolName, CliSubprocessResult result) -{ - var content = new List(); - - if (result.ExitCode == 0) - { - // Success: return stdout content - content.Add(new TextContentBlock { Text = result.Output }); - } - else - { - // Failure: prefer stdout (contains structured error envelope from - // OutputFormatter.WriteResult), fall back to stderr error lines. - var errorText = !string.IsNullOrWhiteSpace(result.Output) - ? result.Output - : !string.IsNullOrWhiteSpace(result.LastErrors) - ? result.LastErrors - : $"Tool '{toolName}' failed with exit code {result.ExitCode}."; - content.Add(new TextContentBlock { Text = errorText }); - - // Store the full execution log and add a resource_link so the client can fetch details - if (!string.IsNullOrWhiteSpace(result.FullLog)) - { - var logUri = toolLogStore.Store(toolName, result.FullLog, result.LastErrors, isError: true); - content.Add(new ResourceLinkBlock - { - Uri = logUri, - Name = $"Full execution log for {toolName}", - Description = "Complete stderr log from the subprocess. Use resources/read to retrieve.", - MimeType = "text/plain" - }); - } - } - - return new CallToolResult - { - Content = content, - IsError = result.ExitCode != 0 - }; -} - -// MCP resource listing — exposes stored tool execution logs +// MCP resource listing — exposes stored failure-detail resources ValueTask ListResourcesAsync(RequestContext ctx, CancellationToken ct) { - var entries = toolLogStore.ListAll(); - var resources = entries.Select(e => new Resource - { - Uri = e.Uri, - Name = $"Execution log: {e.Entry.ToolName}", - Description = $"Full stderr log from {e.Entry.ToolName} at {e.Entry.Timestamp:u}", - MimeType = "text/plain" - }).ToList(); - - return ValueTask.FromResult(new ListResourcesResult { Resources = resources }); + return ValueTask.FromResult(new ListResourcesResult { Resources = toolResultFactory.BuildResources() }); } -// MCP resource read — returns the full execution log for a given URI +// MCP resource read — returns structured failure details for a given URI ValueTask ReadResourceAsync(RequestContext ctx, CancellationToken ct) { var uri = ctx.Params?.Uri ?? throw new McpException("Resource URI is required."); - if (!toolLogStore.TryGet(uri, out var entry) || entry is null) - throw new McpException($"Resource not found: {uri}"); - - return ValueTask.FromResult(new ReadResourceResult - { - Contents = [new TextResourceContents { Uri = uri, MimeType = "text/plain", Text = entry.FullLog }] - }); + return ValueTask.FromResult(toolResultFactory.ReadResource(uri)); } // Helper method to convert JsonElement to the target property type diff --git a/src/TALXIS.CLI.MCP/ToolLogStore.cs b/src/TALXIS.CLI.MCP/ToolLogStore.cs index 28e33de..390413e 100644 --- a/src/TALXIS.CLI.MCP/ToolLogStore.cs +++ b/src/TALXIS.CLI.MCP/ToolLogStore.cs @@ -1,8 +1,12 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using TALXIS.CLI.Core; + namespace TALXIS.CLI.MCP; /// -/// In-memory store for tool execution logs, exposed as MCP resources. -/// Logs are keyed by a unique run ID and evicted FIFO when the store exceeds capacity. +/// In-memory store for failed tool diagnostics, exposed as MCP resources. +/// Entries are keyed by a unique run ID and evicted FIFO when the store exceeds capacity. /// internal sealed class ToolLogStore { @@ -11,7 +15,7 @@ internal sealed class ToolLogStore private readonly Queue _order = []; private readonly int _maxEntries; - /// URI scheme prefix for tool log resources. + /// URI scheme prefix for failure-detail resources. internal const string UriScheme = "txc://logs/"; public ToolLogStore(int maxEntries = 50) @@ -21,13 +25,19 @@ public ToolLogStore(int maxEntries = 50) } /// - /// Stores a tool execution log and returns the resource URI. + /// Stores failed tool diagnostics and returns the resource URI. /// - public string Store(string toolName, string fullLog, string errorSummary, bool isError) + public string StoreFailure(string toolName, int exitCode, string? primaryText, string? errorSummary, string? fullLog) { var runId = Guid.NewGuid().ToString("N")[..12]; var uri = $"{UriScheme}{toolName}/{runId}"; - var entry = new LogEntry(toolName, fullLog, errorSummary, isError, DateTimeOffset.UtcNow); + var entry = new LogEntry( + ToolName: toolName, + ExitCode: exitCode, + PrimaryText: Normalize(primaryText), + ErrorSummary: Normalize(errorSummary), + FullLog: Normalize(fullLog), + Timestamp: DateTimeOffset.UtcNow); lock (_sync) { @@ -66,10 +76,40 @@ public bool TryGet(string uri, out LogEntry? entry) } } + private static string? Normalize(string? value) + => string.IsNullOrWhiteSpace(value) ? null : value.Trim(); + internal sealed record LogEntry( string ToolName, - string FullLog, - string ErrorSummary, - bool IsError, - DateTimeOffset Timestamp); + int ExitCode, + string? PrimaryText, + string? ErrorSummary, + string? FullLog, + DateTimeOffset Timestamp) + { + public string Kind => "tool-failure-details"; + + public string Summary => FirstNonEmpty( + PrimaryText, + ErrorSummary, + $"Tool '{ToolName}' failed with exit code {ExitCode}.")!; + + [JsonIgnore] + public bool HasFullLog => !string.IsNullOrWhiteSpace(FullLog); + + public string ToJson() => JsonSerializer.Serialize(this, TxcOutputJsonOptions.Default); + + private static string? FirstNonEmpty(params string?[] values) + { + foreach (var value in values) + { + if (!string.IsNullOrWhiteSpace(value)) + { + return value; + } + } + + return null; + } + } } diff --git a/tests/TALXIS.CLI.IntegrationTests/McpTestClient.cs b/tests/TALXIS.CLI.IntegrationTests/McpTestClient.cs index 208a4f0..be9a5f9 100644 --- a/tests/TALXIS.CLI.IntegrationTests/McpTestClient.cs +++ b/tests/TALXIS.CLI.IntegrationTests/McpTestClient.cs @@ -49,6 +49,16 @@ public async Task> ListToolsAsync() return await _client.ListToolsAsync(); } + public async Task> ListResourcesAsync() + { + return await _client.ListResourcesAsync(); + } + + public async Task ReadResourceAsync(string uri) + { + return await _client.ReadResourceAsync(uri); + } + public async ValueTask DisposeAsync() { await _client.DisposeAsync(); diff --git a/tests/TALXIS.CLI.IntegrationTests/McpTests.cs b/tests/TALXIS.CLI.IntegrationTests/McpTests.cs index 602adde..d89f805 100644 --- a/tests/TALXIS.CLI.IntegrationTests/McpTests.cs +++ b/tests/TALXIS.CLI.IntegrationTests/McpTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.Json; using System.Threading.Tasks; using ModelContextProtocol.Protocol; using Xunit; @@ -77,4 +78,33 @@ public async Task ExecuteOperation_WorkspaceComponentExplain_ReturnsComponentDet Assert.Contains("pp-entity", textBlock.Text); } } + + [Fact] + public async Task ExecuteOperation_FailedTool_ExposesReadableFailureResource() + { + var client = await McpTestClient.InstanceAsync; + var args = new Dictionary + { + { "operation", "workspace_validate" }, + { "arguments", new Dictionary { { "Path", "definitely-not-a-real-workspace-path" } } } + }; + + var result = await client.CallToolAsync("execute_operation", args); + + Assert.True(result.IsError); + Assert.NotNull(result.Content); + Assert.True(result.Content.Count >= 2); + + var resourceLink = Assert.IsType(result.Content[1]); + Assert.Equal("application/json", resourceLink.MimeType); + + var readResult = await client.ReadResourceAsync(resourceLink.Uri); + var resource = Assert.IsType(Assert.Single(readResult.Contents)); + Assert.Equal("application/json", resource.MimeType); + + using var document = JsonDocument.Parse(resource.Text); + Assert.Equal("workspace_validate", document.RootElement.GetProperty("toolName").GetString()); + Assert.True(document.RootElement.TryGetProperty("summary", out var summary)); + Assert.False(string.IsNullOrWhiteSpace(summary.GetString())); + } } diff --git a/tests/TALXIS.CLI.Tests/MCP/McpToolResultFactoryTests.cs b/tests/TALXIS.CLI.Tests/MCP/McpToolResultFactoryTests.cs new file mode 100644 index 0000000..f32f41b --- /dev/null +++ b/tests/TALXIS.CLI.Tests/MCP/McpToolResultFactoryTests.cs @@ -0,0 +1,62 @@ +using System.Text.Json; +using ModelContextProtocol.Protocol; +using TALXIS.CLI.MCP; +using Xunit; + +namespace TALXIS.CLI.Tests.MCP; + +public class McpToolResultFactoryTests +{ + [Fact] + public void Build_FailedResultWithSummary_AttachesReadableDiagnosticsResource() + { + var store = new ToolLogStore(); + var factory = new McpToolResultFactory(store); + var result = new CliSubprocessResult( + exitCode: 1, + output: "{\"status\":\"failed\",\"message\":\"Validation complete: 3 error(s), 7 warning(s)\"}", + lastErrors: "file1.xml(1,1): schema error", + fullLog: "2026-05-04T10:00:00Z [Error] [WorkspaceValidateCliCommand] file1.xml(1,1): schema error"); + + var toolResult = factory.Build("workspace_validate", result); + + Assert.True(toolResult.IsError); + var text = Assert.IsType(toolResult.Content[0]); + Assert.Contains("Validation complete", text.Text); + + var link = Assert.IsType(toolResult.Content[1]); + Assert.Equal("application/json", link.MimeType); + + var resource = factory.ReadResource(link.Uri); + var contents = Assert.IsType(Assert.Single(resource.Contents)); + Assert.Equal("application/json", contents.MimeType); + + using var document = JsonDocument.Parse(contents.Text); + Assert.Equal("tool-failure-details", document.RootElement.GetProperty("kind").GetString()); + Assert.Equal("workspace_validate", document.RootElement.GetProperty("toolName").GetString()); + Assert.Equal(1, document.RootElement.GetProperty("exitCode").GetInt32()); + Assert.Contains("Validation complete", document.RootElement.GetProperty("summary").GetString()); + Assert.Contains("schema error", document.RootElement.GetProperty("fullLog").GetString()); + } + + [Fact] + public void BuildExceptionResult_AttachesDiagnosticsResource() + { + var store = new ToolLogStore(); + var factory = new McpToolResultFactory(store); + + var toolResult = factory.BuildExceptionResult("workspace_validate", new InvalidOperationException("Boom")); + + Assert.True(toolResult.IsError); + var text = Assert.IsType(toolResult.Content[0]); + Assert.Equal("Boom", text.Text); + + var link = Assert.IsType(toolResult.Content[1]); + var resource = factory.ReadResource(link.Uri); + var contents = Assert.IsType(Assert.Single(resource.Contents)); + + using var document = JsonDocument.Parse(contents.Text); + Assert.Equal(-1, document.RootElement.GetProperty("exitCode").GetInt32()); + Assert.Equal("Boom", document.RootElement.GetProperty("summary").GetString()); + } +} diff --git a/tests/TALXIS.CLI.Tests/MCP/ToolLogStoreTests.cs b/tests/TALXIS.CLI.Tests/MCP/ToolLogStoreTests.cs index 71ab964..ddec4cb 100644 --- a/tests/TALXIS.CLI.Tests/MCP/ToolLogStoreTests.cs +++ b/tests/TALXIS.CLI.Tests/MCP/ToolLogStoreTests.cs @@ -9,7 +9,7 @@ public class ToolLogStoreTests public void Store_ReturnsUriWithToolName() { var store = new ToolLogStore(); - var uri = store.Store("data_package_import", "full log", "error summary", isError: true); + var uri = store.StoreFailure("data_package_import", 1, "summary", "error summary", "full log"); Assert.StartsWith(ToolLogStore.UriScheme + "data_package_import/", uri); } @@ -18,14 +18,15 @@ public void Store_ReturnsUriWithToolName() public void TryGet_ReturnsStoredEntry() { var store = new ToolLogStore(); - var uri = store.Store("my_tool", "full log content", "errors here", isError: true); + var uri = store.StoreFailure("my_tool", 1, "summary", "errors here", "full log content"); Assert.True(store.TryGet(uri, out var entry)); Assert.NotNull(entry); Assert.Equal("my_tool", entry.ToolName); + Assert.Equal(1, entry.ExitCode); + Assert.Equal("summary", entry.PrimaryText); Assert.Equal("full log content", entry.FullLog); Assert.Equal("errors here", entry.ErrorSummary); - Assert.True(entry.IsError); } [Fact] @@ -39,8 +40,8 @@ public void TryGet_ReturnsFalseForUnknownUri() public void ListAll_ReturnsAllEntries() { 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", 1, "summary a", "", "log a"); + store.StoreFailure("tool_b", 1, "summary b", "err", "log b"); var all = store.ListAll(); Assert.Equal(2, all.Count); @@ -50,10 +51,10 @@ public void ListAll_ReturnsAllEntries() public void Store_EvictsOldestWhenOverCapacity() { var store = new ToolLogStore(maxEntries: 3); - var uri1 = store.Store("tool", "log1", "", isError: false); - store.Store("tool", "log2", "", isError: false); - store.Store("tool", "log3", "", isError: false); - store.Store("tool", "log4", "", isError: false); // Should evict uri1 + var uri1 = store.StoreFailure("tool", 1, "summary1", "", "log1"); + store.StoreFailure("tool", 1, "summary2", "", "log2"); + store.StoreFailure("tool", 1, "summary3", "", "log3"); + store.StoreFailure("tool", 1, "summary4", "", "log4"); // Should evict uri1 Assert.False(store.TryGet(uri1, out _)); Assert.Equal(3, store.ListAll().Count); @@ -63,8 +64,8 @@ public void Store_EvictsOldestWhenOverCapacity() public void Store_GeneratesUniqueUris() { var store = new ToolLogStore(); - var uri1 = store.Store("tool", "log1", "", isError: false); - var uri2 = store.Store("tool", "log2", "", isError: false); + var uri1 = store.StoreFailure("tool", 1, "summary1", "", "log1"); + var uri2 = store.StoreFailure("tool", 1, "summary2", "", "log2"); Assert.NotEqual(uri1, uri2); }