From f244e0f9c4b18f63fafa3dd7e59d68436d12c2bc Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 11 Aug 2025 12:33:02 -0700 Subject: [PATCH] Stop reporting errors for non-zero exits --- pkgs/dart_mcp_server/CHANGELOG.md | 1 + .../lib/src/utils/analytics.dart | 1 - .../lib/src/utils/cli_utils.dart | 8 ++- .../test/dart_tooling_mcp_server_test.dart | 64 ++++++++----------- 4 files changed, 34 insertions(+), 40 deletions(-) diff --git a/pkgs/dart_mcp_server/CHANGELOG.md b/pkgs/dart_mcp_server/CHANGELOG.md index 1cb0b997..ebbc5abf 100644 --- a/pkgs/dart_mcp_server/CHANGELOG.md +++ b/pkgs/dart_mcp_server/CHANGELOG.md @@ -13,6 +13,7 @@ based on their text values. * Add an `--exclude-tool` command line flag to exclude tools by name. * Add the abillity to limit the output of `analyze_files` to a set of paths. +* Stop reporting non-zero exit codes from command line tools as tool errors. # 0.1.0 (Dart SDK 3.9.0) diff --git a/pkgs/dart_mcp_server/lib/src/utils/analytics.dart b/pkgs/dart_mcp_server/lib/src/utils/analytics.dart index 4c423ff2..1bc74a38 100644 --- a/pkgs/dart_mcp_server/lib/src/utils/analytics.dart +++ b/pkgs/dart_mcp_server/lib/src/utils/analytics.dart @@ -130,7 +130,6 @@ enum CallToolFailureReason { noRootGiven, noRootsSet, noSuchCommand, - nonZeroExitCode, webSocketException, } diff --git a/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart b/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart index 25220e55..2410fc74 100644 --- a/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart +++ b/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart @@ -238,12 +238,14 @@ Future runCommandInRoot( content: [ TextContent( text: - '$commandDescription failed in ${projectRoot.path}:\n' + '$commandDescription returned a non-zero exit code in ' + '${projectRoot.path}:\n' '$output${errors.isEmpty ? '' : '\nErrors:\n$errors'}', ), + // Returning a non-zero exit code is not considered an "error" in the + // "isError" sense. ], - isError: true, - )..failureReason ??= CallToolFailureReason.nonZeroExitCode; + ); } return CallToolResult( content: [ diff --git a/pkgs/dart_mcp_server/test/dart_tooling_mcp_server_test.dart b/pkgs/dart_mcp_server/test/dart_tooling_mcp_server_test.dart index 2fcfbba5..57777660 100644 --- a/pkgs/dart_mcp_server/test/dart_tooling_mcp_server_test.dart +++ b/pkgs/dart_mcp_server/test/dart_tooling_mcp_server_test.dart @@ -60,43 +60,35 @@ void main() { }); test('sends analytics for failed tool calls', () async { - for (var reason in [null, CallToolFailureReason.nonZeroExitCode]) { - analytics.sentEvents.clear(); + analytics.sentEvents.clear(); - final tool = Tool( - name: 'hello${reason?.name ?? ''}', - inputSchema: Schema.object(), - ); - server.registerTool( - tool, - (_) => - CallToolResult(isError: true, content: []) - ..failureReason = reason, - ); - final result = await testHarness.mcpServerConnection.callTool( - CallToolRequest(name: tool.name), - ); - expect(result.isError, true); - expect( - analytics.sentEvents.single, - isA() - .having((e) => e.eventName, 'eventName', DashEvent.dartMCPEvent) - .having( - (e) => e.eventData, - 'eventData', - equals({ - 'client': server.clientInfo.name, - 'clientVersion': server.clientInfo.version, - 'serverVersion': server.implementation.version, - 'type': AnalyticsEvent.callTool.name, - 'tool': tool.name, - 'success': false, - 'elapsedMilliseconds': isA(), - 'failureReason': ?reason?.name, - }), - ), - ); - } + final tool = Tool(name: 'hello', inputSchema: Schema.object()); + server.registerTool( + tool, + (_) => CallToolResult(isError: true, content: [])..failureReason = null, + ); + final result = await testHarness.mcpServerConnection.callTool( + CallToolRequest(name: tool.name), + ); + expect(result.isError, true); + expect( + analytics.sentEvents.single, + isA() + .having((e) => e.eventName, 'eventName', DashEvent.dartMCPEvent) + .having( + (e) => e.eventData, + 'eventData', + equals({ + 'client': server.clientInfo.name, + 'clientVersion': server.clientInfo.version, + 'serverVersion': server.implementation.version, + 'type': AnalyticsEvent.callTool.name, + 'tool': tool.name, + 'success': false, + 'elapsedMilliseconds': isA(), + }), + ), + ); }); group('are sent for prompts', () {