From 57745ed6d49cbca35b9c604b13d53867f5ad30bc Mon Sep 17 00:00:00 2001 From: Jake MacDonald Date: Tue, 29 Apr 2025 08:40:39 -0700 Subject: [PATCH 01/13] checkpoint, abstract error listening and listen for debug session changed events --- .../lib/src/mixins/dtd.dart | 145 ++++++++++++------ 1 file changed, 98 insertions(+), 47 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index c9999ea..7c88551 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -18,7 +18,7 @@ import 'package:vm_service/vm_service_io.dart'; /// https://pub.dev/packages/dtd). /// /// The MCPServer must already have the [ToolsSupport] mixin applied. -base mixin DartToolingDaemonSupport on ToolsSupport { +base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { DartToolingDaemon? _dtd; /// Whether or not the DTD extension to get the active debug sessions is @@ -83,24 +83,31 @@ base mixin DartToolingDaemonSupport on ToolsSupport { final response = await dtd.getDebugSessions(); final debugSessions = response.debugSessions; for (final debugSession in debugSessions) { - if (activeVmServices.containsKey(debugSession.vmServiceUri)) { + if (activeVmServices.containsKey(debugSession.id)) { continue; } - final vmService = await vmServiceConnectUri(debugSession.vmServiceUri); - activeVmServices[debugSession.vmServiceUri] = vmService; - unawaited( - vmService.onDone.then((_) { - activeVmServices.remove(debugSession.vmServiceUri); - vmService.dispose(); - }), - ); + if (debugSession.vmServiceUri case final vmServiceUri?) { + final vmService = await vmServiceConnectUri(vmServiceUri); + activeVmServices[debugSession.id] = vmService; + unawaited( + vmService.onDone.then((_) { + activeVmServices.remove(debugSession.id); + vmService.dispose(); + }), + ); + } } } @override FutureOr initialize(InitializeRequest request) async { registerTool(connectTool, _connect); - registerTool(getRuntimeErrorsTool, runtimeErrors); + // If we support sampling, then don't expose the runtime errors tool, as + // this might confuse the VM and supporting it would require caching all + // errors indefinitely. + if (request.capabilities.sampling != null) { + registerTool(getRuntimeErrorsTool, runtimeErrors); + } // TODO: these tools should only be registered for Flutter applications, or // they should return an error when used against a pure Dart app (or a @@ -174,6 +181,21 @@ base mixin DartToolingDaemonSupport on ToolsSupport { } }); dtd.streamListen('Service'); + + dtd.onEvent('Editor').listen((e) async { + log(LoggingLevel.debug, e.toString()); + switch (e.kind) { + case 'debugSessionStarted': + case 'debugSessionChanged': + await updateActiveVmServices(); + case 'debugSessionStopped': + await activeVmServices + .remove((e.data['debugSession'] as DebugSession).id) + ?.dispose(); + default: + } + }); + dtd.streamListen('Editor'); } /// Takes a screenshot of the currently running app. @@ -299,38 +321,14 @@ base mixin DartToolingDaemonSupport on ToolsSupport { Future runtimeErrors(CallToolRequest request) async { return _callOnVmService( callback: (vmService) async { - final errors = []; - StreamSubscription? extensionEvents; - StreamSubscription? stderrEvents; try { - // We need to listen to streams with history so that we can get errors - // that occurred before this tool call. - // TODO(https://github.com/dart-lang/ai/issues/57): this can result in - // duplicate errors that we need to de-duplicate somehow. - extensionEvents = vmService.onExtensionEventWithHistory.listen(( - Event e, - ) { - if (e.extensionKind == 'Flutter.Error') { - // TODO(https://github.com/dart-lang/ai/issues/57): consider - // pruning this content down to only what is useful for the LLM to - // understand the error and its source. - errors.add(e.json.toString()); - } - }); - stderrEvents = vmService.onStderrEventWithHistory.listen((Event e) { - final message = decodeBase64(e.bytes!); - // TODO(https://github.com/dart-lang/ai/issues/57): consider - // pruning this content down to only what is useful for the LLM to - // understand the error and its source. - errors.add(message); - }); - - await vmService.streamListen(EventStreams.kExtension); - await vmService.streamListen(EventStreams.kStderr); - + final errorService = await _AppErrorsListener.forVmService(vmService); + final errors = []; + errorService.errors.listen(errors.add); // Await a short delay to allow the error events to come over the open // Streams. await Future.delayed(const Duration(seconds: 1)); + await errorService.shutdown(); if (errors.isEmpty) { return CallToolResult( @@ -352,11 +350,6 @@ base mixin DartToolingDaemonSupport on ToolsSupport { isError: true, content: [TextContent(text: 'Failed to get runtime errors: $e')], ); - } finally { - await extensionEvents?.cancel(); - await stderrEvents?.cancel(); - await vmService.streamCancel(EventStreams.kExtension); - await vmService.streamCancel(EventStreams.kStderr); } }, ); @@ -583,6 +576,64 @@ base mixin DartToolingDaemonSupport on ToolsSupport { ); } +/// Listens on a VM service for errors. +class _AppErrorsListener { + Stream get errors => errorsController.stream; + final StreamController errorsController; + final StreamSubscription extensionEventsListener; + final StreamSubscription stderrEventsListener; + final VmService vmService; + + _AppErrorsListener._( + this.errorsController, + this.extensionEventsListener, + this.stderrEventsListener, + this.vmService, + ); + + static Future<_AppErrorsListener> forVmService(VmService vmService) async { + final errorsController = StreamController(); + // We need to listen to streams with history so that we can get errors + // that occurred before this tool call. + // TODO(https://github.com/dart-lang/ai/issues/57): this can result in + // duplicate errors that we need to de-duplicate somehow. + final extensionEvents = vmService.onExtensionEventWithHistory.listen(( + Event e, + ) { + if (e.extensionKind == 'Flutter.Error') { + // TODO(https://github.com/dart-lang/ai/issues/57): consider + // pruning this content down to only what is useful for the LLM to + // understand the error and its source. + errorsController.add(e.json.toString()); + } + }); + final stderrEvents = vmService.onStderrEventWithHistory.listen((Event e) { + final message = decodeBase64(e.bytes!); + // TODO(https://github.com/dart-lang/ai/issues/57): consider + // pruning this content down to only what is useful for the LLM to + // understand the error and its source. + errorsController.add(message); + }); + + await vmService.streamListen(EventStreams.kExtension); + await vmService.streamListen(EventStreams.kStderr); + return _AppErrorsListener._( + errorsController, + extensionEvents, + stderrEvents, + vmService, + ); + } + + Future shutdown() async { + await extensionEventsListener.cancel(); + await stderrEventsListener.cancel(); + await vmService.streamCancel(EventStreams.kExtension); + await vmService.streamCancel(EventStreams.kStderr); + await errorsController.close(); + } +} + /// Adds the [getDebugSessions] method to [DartToolingDaemon], so that calling /// the Editor.getDebugSessions service method can be wrapped nicely behind a /// method call from a given client. @@ -653,19 +704,19 @@ extension type DebugSession.fromJson(Map _value) String get id => _value['id'] as String; String get name => _value['name'] as String; String get projectRootPath => _value['projectRootPath'] as String; - String get vmServiceUri => _value['vmServiceUri'] as String; + String? get vmServiceUri => _value['vmServiceUri'] as String?; factory DebugSession({ required String debuggerType, required String id, required String name, required String projectRootPath, - required String vmServiceUri, + required String? vmServiceUri, }) => DebugSession.fromJson({ 'debuggerType': debuggerType, 'id': id, 'name': name, 'projectRootPath': projectRootPath, - 'vmServiceUri': vmServiceUri, + if (vmServiceUri != null) 'vmServiceUri': vmServiceUri, }); } From 781740e6a4aaf84ba9cf665db28a13a7fcc79cb2 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Apr 2025 18:05:05 +0000 Subject: [PATCH 02/13] checkpoint --- pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index 7c88551..e76037c 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -89,6 +89,18 @@ base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { if (debugSession.vmServiceUri case final vmServiceUri?) { final vmService = await vmServiceConnectUri(vmServiceUri); activeVmServices[debugSession.id] = vmService; + if (clientCapabilities.sampling != null) { + (await _AppErrorsListener.forVmService(vmService)).errors.listen(( + e, + ) async { + final result = await createMessage( + CreateMessageRequest( + messages: [SamplingMessage(role: Role.user, content: TextContent(text: 'Please fix the runtime error: $e'))], + maxTokens: maxTokens, + ), + ); + }); + } unawaited( vmService.onDone.then((_) { activeVmServices.remove(debugSession.id); From c5771729e5dc016be4aad732ef093b855384c565 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Apr 2025 22:38:01 +0000 Subject: [PATCH 03/13] add resource for runtime errors --- .../lib/src/mixins/dtd.dart | 145 ++++++++++-------- .../lib/src/server.dart | 1 + 2 files changed, 78 insertions(+), 68 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index e76037c..c59b46f 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -18,7 +18,8 @@ import 'package:vm_service/vm_service_io.dart'; /// https://pub.dev/packages/dtd). /// /// The MCPServer must already have the [ToolsSupport] mixin applied. -base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { +base mixin DartToolingDaemonSupport + on ToolsSupport, LoggingSupport, ResourcesSupport { DartToolingDaemon? _dtd; /// Whether or not the DTD extension to get the active debug sessions is @@ -89,22 +90,27 @@ base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { if (debugSession.vmServiceUri case final vmServiceUri?) { final vmService = await vmServiceConnectUri(vmServiceUri); activeVmServices[debugSession.id] = vmService; - if (clientCapabilities.sampling != null) { - (await _AppErrorsListener.forVmService(vmService)).errors.listen(( - e, - ) async { - final result = await createMessage( - CreateMessageRequest( - messages: [SamplingMessage(role: Role.user, content: TextContent(text: 'Please fix the runtime error: $e'))], - maxTokens: maxTokens, - ), - ); - }); - } + // Start listening for and collecting errors immediately. + final errorService = await _AppErrorsListener.forVmService(vmService); + final resource = Resource( + uri: 'runtime-errors://${debugSession.id}', + name: debugSession.name, + ); + addResource(resource, (request) async { + final errors = errorService.errors; + return ReadResourceResult( + contents: [ + for (var error in errors) + TextResourceContents(uri: resource.uri, text: error), + ], + ); + }); + errorService.errorsStream.listen((_) => updateResource(resource)); unawaited( vmService.onDone.then((_) { + removeResource(resource.uri); activeVmServices.remove(debugSession.id); - vmService.dispose(); + vmService.dispose(); // This will also shut down the `errorService`. }), ); } @@ -114,12 +120,7 @@ base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { @override FutureOr initialize(InitializeRequest request) async { registerTool(connectTool, _connect); - // If we support sampling, then don't expose the runtime errors tool, as - // this might confuse the VM and supporting it would require caching all - // errors indefinitely. - if (request.capabilities.sampling != null) { - registerTool(getRuntimeErrorsTool, runtimeErrors); - } + registerTool(getRuntimeErrorsTool, runtimeErrors); // TODO: these tools should only be registered for Flutter applications, or // they should return an error when used against a pure Dart app (or a @@ -335,12 +336,7 @@ base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { callback: (vmService) async { try { final errorService = await _AppErrorsListener.forVmService(vmService); - final errors = []; - errorService.errors.listen(errors.add); - // Await a short delay to allow the error events to come over the open - // Streams. - await Future.delayed(const Duration(seconds: 1)); - await errorService.shutdown(); + final errors = errorService.errors; if (errors.isEmpty) { return CallToolResult( @@ -590,59 +586,72 @@ base mixin DartToolingDaemonSupport on ToolsSupport, LoggingSupport { /// Listens on a VM service for errors. class _AppErrorsListener { - Stream get errors => errorsController.stream; - final StreamController errorsController; - final StreamSubscription extensionEventsListener; - final StreamSubscription stderrEventsListener; - final VmService vmService; + final List errors = []; + Stream get errorsStream => _errorsController.stream; + final StreamController _errorsController; + final StreamSubscription _extensionEventsListener; + final StreamSubscription _stderrEventsListener; + final VmService _vmService; _AppErrorsListener._( - this.errorsController, - this.extensionEventsListener, - this.stderrEventsListener, - this.vmService, - ); + this._errorsController, + this._extensionEventsListener, + this._stderrEventsListener, + this._vmService, + ) { + errorsStream.listen(errors.add); + _vmService.onDone.then((_) => shutdown()); + } + /// Maintain a cache of error listeners by [VmService] instance as an + /// [Expando] so we don't have to worry about explicit cleanup. + static final _errorListeners = Expando<_AppErrorsListener>(); + + /// Returns the canonical [_AppErrorsListener] for the [vmService] instance, + /// which may be an already existing instance. static Future<_AppErrorsListener> forVmService(VmService vmService) async { - final errorsController = StreamController(); - // We need to listen to streams with history so that we can get errors - // that occurred before this tool call. - // TODO(https://github.com/dart-lang/ai/issues/57): this can result in - // duplicate errors that we need to de-duplicate somehow. - final extensionEvents = vmService.onExtensionEventWithHistory.listen(( - Event e, - ) { - if (e.extensionKind == 'Flutter.Error') { + return _errorListeners[vmService] ??= await () async { + final errorsController = StreamController(); + // We need to listen to streams with history so that we can get errors + // that occurred before this tool call. + // TODO(https://github.com/dart-lang/ai/issues/57): this can result in + // duplicate errors that we need to de-duplicate somehow. + final extensionEvents = vmService.onExtensionEventWithHistory.listen(( + Event e, + ) { + if (e.extensionKind == 'Flutter.Error') { + // TODO(https://github.com/dart-lang/ai/issues/57): consider + // pruning this content down to only what is useful for the LLM to + // understand the error and its source. + errorsController.add(e.json.toString()); + } + }); + final stderrEvents = vmService.onStderrEventWithHistory.listen((Event e) { + final message = decodeBase64(e.bytes!); // TODO(https://github.com/dart-lang/ai/issues/57): consider // pruning this content down to only what is useful for the LLM to // understand the error and its source. - errorsController.add(e.json.toString()); - } - }); - final stderrEvents = vmService.onStderrEventWithHistory.listen((Event e) { - final message = decodeBase64(e.bytes!); - // TODO(https://github.com/dart-lang/ai/issues/57): consider - // pruning this content down to only what is useful for the LLM to - // understand the error and its source. - errorsController.add(message); - }); + errorsController.add(message); + }); - await vmService.streamListen(EventStreams.kExtension); - await vmService.streamListen(EventStreams.kStderr); - return _AppErrorsListener._( - errorsController, - extensionEvents, - stderrEvents, - vmService, - ); + await vmService.streamListen(EventStreams.kExtension); + await vmService.streamListen(EventStreams.kStderr); + return _AppErrorsListener._( + errorsController, + extensionEvents, + stderrEvents, + vmService, + ); + }(); } Future shutdown() async { - await extensionEventsListener.cancel(); - await stderrEventsListener.cancel(); - await vmService.streamCancel(EventStreams.kExtension); - await vmService.streamCancel(EventStreams.kStderr); - await errorsController.close(); + errors.clear(); + await _errorsController.close(); + await _extensionEventsListener.cancel(); + await _stderrEventsListener.cancel(); + await _vmService.streamCancel(EventStreams.kExtension); + await _vmService.streamCancel(EventStreams.kStderr); } } diff --git a/pkgs/dart_tooling_mcp_server/lib/src/server.dart b/pkgs/dart_tooling_mcp_server/lib/src/server.dart index ce8273a..f0e344e 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/server.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/server.dart @@ -20,6 +20,7 @@ final class DartToolingMCPServer extends MCPServer with LoggingSupport, ToolsSupport, + ResourcesSupport, RootsTrackingSupport, DartAnalyzerSupport, DartCliSupport, From f4d985894bd5d3be7f93c2dc1ad10b57610166c9 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Apr 2025 23:27:50 +0000 Subject: [PATCH 04/13] add tool to clear the runtime errors, as well as an argument to the hotReload tool to clear the errors before a hot reload --- .../lib/src/mixins/dtd.dart | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index c59b46f..cd2ca0a 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -121,6 +121,7 @@ base mixin DartToolingDaemonSupport FutureOr initialize(InitializeRequest request) async { registerTool(connectTool, _connect); registerTool(getRuntimeErrorsTool, runtimeErrors); + registerTool(clearRuntimeErrorsTool, clearRuntimeErrors); // TODO: these tools should only be registered for Flutter applications, or // they should return an error when used against a pure Dart app (or a @@ -260,6 +261,10 @@ base mixin DartToolingDaemonSupport Future hotReload(CallToolRequest request) async { return _callOnVmService( callback: (vmService) async { + if (request.arguments?['clearRuntimeErrors'] == true) { + (await _AppErrorsListener.forVmService(vmService)).errors.clear(); + } + StreamSubscription? serviceStreamSubscription; try { final hotReloadMethodNameCompleter = Completer(); @@ -325,6 +330,29 @@ base mixin DartToolingDaemonSupport ); } + /// Clears runtime errors from the currently running app. + /// + // TODO: support passing a debug session id when there is more than one debug + // session. + Future clearRuntimeErrors(CallToolRequest request) async { + return _callOnVmService( + callback: (vmService) async { + try { + final errorService = await _AppErrorsListener.forVmService(vmService); + errorService.errors.clear(); + return CallToolResult( + content: [TextContent(text: 'Runtime errors cleared.')], + ); + } catch (e) { + return CallToolResult( + isError: true, + content: [TextContent(text: 'Failed to clear runtime errors: $e')], + ); + } + }, + ); + } + /// Retrieves runtime errors from the currently running app. /// /// If more than one debug session is active, then it just uses the first one. @@ -484,6 +512,20 @@ base mixin DartToolingDaemonSupport ), ); + @visibleForTesting + static final clearRuntimeErrorsTool = Tool( + name: 'clear_runtime_errors', + description: + 'Clears the list of runtime errors that have occurred in the active ' + 'Dart or Flutter application. Requires "${connectTool.name}" to be ' + 'successfully called first.', + annotations: ToolAnnotations( + title: 'Clear runtime errors', + destructiveHint: true, + ), + inputSchema: Schema.object(), + ); + @visibleForTesting static final getRuntimeErrorsTool = Tool( name: 'get_runtime_errors', @@ -517,7 +559,17 @@ base mixin DartToolingDaemonSupport 'This is to apply the latest code changes to the running application. ' 'Requires "${connectTool.name}" to be successfully called first.', annotations: ToolAnnotations(title: 'Hot reload', destructiveHint: true), - inputSchema: Schema.object(), + inputSchema: Schema.object( + properties: { + 'clearRuntimeErrors': Schema.bool( + title: 'Whether to clear runtime errors before hot reloading.', + description: + 'This is useful to clear out old errors that may no longer be ' + 'relevant.', + ), + }, + required: [], + ), ); @visibleForTesting From 7fd9df8f2a5910996ab943878eef3ea67ca852d7 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Apr 2025 23:30:32 +0000 Subject: [PATCH 05/13] drop wasSince extension --- pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index ca9668c..5cc43e3 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -814,12 +814,3 @@ extension type DebugSession.fromJson(Map _value) if (vmServiceUri != null) 'vmServiceUri': vmServiceUri, }); } - -extension on Event { - /// Returns `true` if [timestamp] is >= [since]. - /// - /// If we cannot determine this due to either [timestamp] or [since] being - /// null, then we also return `true`. - bool wasSince(int? since) => - since == null || timestamp == null ? true : timestamp! >= since; -} From 735c22ab56725dddf8b6b4bf5ffbf8beabb33154 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Apr 2025 23:55:40 +0000 Subject: [PATCH 06/13] add tests --- .../lib/src/mixins/dtd.dart | 4 +- .../test/tools/dtd_test.dart | 189 +++++++++++++----- 2 files changed, 139 insertions(+), 54 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index 5cc43e3..822a95a 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -93,7 +93,7 @@ base mixin DartToolingDaemonSupport // Start listening for and collecting errors immediately. final errorService = await _AppErrorsListener.forVmService(vmService); final resource = Resource( - uri: 'runtime-errors://${debugSession.id}', + uri: '$runtimeErrorsScheme://${debugSession.id}', name: debugSession.name, ); addResource(resource, (request) async { @@ -655,6 +655,8 @@ base mixin DartToolingDaemonSupport ), ], ); + + static final runtimeErrorsScheme = 'runtime-errors'; } /// Listens on a VM service for errors. diff --git a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart index aede9af..40e0389 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart @@ -4,6 +4,7 @@ import 'dart:convert'; +import 'package:async/async.dart'; import 'package:dart_mcp/server.dart'; import 'package:dart_tooling_mcp_server/src/mixins/dtd.dart'; import 'package:test/test.dart'; @@ -63,63 +64,145 @@ void main() { ]); }); - test('can get runtime errors', () async { - await testHarness.startDebugSession( - counterAppPath, - 'lib/main.dart', - isFlutter: true, - args: ['--dart-define=include_layout_error=true'], - ); - final tools = (await testHarness.mcpServerConnection.listTools()).tools; - final runtimeErrorsTool = tools.singleWhere( - (t) => t.name == DartToolingDaemonSupport.getRuntimeErrorsTool.name, - ); - final runtimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest(name: runtimeErrorsTool.name), - ); - - expect(runtimeErrorsResult.isError, isNot(true)); + group('runtime errors', () { final errorCountRegex = RegExp(r'Found \d+ errors?:'); - expect( - (runtimeErrorsResult.content.first as TextContent).text, - contains(errorCountRegex), - ); - expect( - (runtimeErrorsResult.content[1] as TextContent).text, - contains('A RenderFlex overflowed by'), - ); - final now = DateTime.now().millisecondsSinceEpoch; - final sinceNowResult = await testHarness.callToolWithRetry( - CallToolRequest( - name: runtimeErrorsTool.name, - arguments: {'since': now}, - ), - ); - expect( - (sinceNowResult.content.first as TextContent).text, - contains('No runtime errors found'), - ); + setUp(() async { + await testHarness.startDebugSession( + counterAppPath, + 'lib/main.dart', + isFlutter: true, + args: ['--dart-define=include_layout_error=true'], + ); + }); - // Trigger a hot reload, should see the error again. - await testHarness.callToolWithRetry( - CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), - ); + test('can be read and cleared using the tools', () async { + final tools = + (await testHarness.mcpServerConnection.listTools()).tools; + final runtimeErrorsTool = tools.singleWhere( + (t) => t.name == DartToolingDaemonSupport.getRuntimeErrorsTool.name, + ); + final runtimeErrorsResult = await testHarness.callToolWithRetry( + CallToolRequest(name: runtimeErrorsTool.name), + ); - final finalRuntimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest( - name: runtimeErrorsTool.name, - arguments: {'since': now}, - ), - ); - expect( - (finalRuntimeErrorsResult.content.first as TextContent).text, - contains(errorCountRegex), - ); - expect( - (finalRuntimeErrorsResult.content[1] as TextContent).text, - contains('A RenderFlex overflowed by'), - ); + expect(runtimeErrorsResult.isError, isNot(true)); + expect( + (runtimeErrorsResult.content.first as TextContent).text, + contains(errorCountRegex), + ); + expect( + (runtimeErrorsResult.content[1] as TextContent).text, + contains('A RenderFlex overflowed by'), + ); + + final clearRuntimeErrorsTool = tools.singleWhere( + (t) => + t.name == DartToolingDaemonSupport.clearRuntimeErrorsTool.name, + ); + final clearRuntimeErrorsResult = await testHarness.callToolWithRetry( + CallToolRequest(name: clearRuntimeErrorsTool.name), + ); + expect(clearRuntimeErrorsResult.isError, isNot(true)); + + final nextResult = await testHarness.callToolWithRetry( + CallToolRequest(name: runtimeErrorsTool.name), + ); + expect( + (nextResult.content.first as TextContent).text, + contains('No runtime errors found'), + ); + + // Trigger a hot reload, should see the error again. + await testHarness.callToolWithRetry( + CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), + ); + + final finalRuntimeErrorsResult = await testHarness.callToolWithRetry( + CallToolRequest(name: runtimeErrorsTool.name), + ); + expect( + (finalRuntimeErrorsResult.content.first as TextContent).text, + contains(errorCountRegex), + ); + expect( + (finalRuntimeErrorsResult.content[1] as TextContent).text, + contains('A RenderFlex overflowed by'), + ); + }); + + test('can be read and subscribed to as a resource', () async { + final serverConnection = testHarness.mcpServerConnection; + final resources = + (await serverConnection.listResources( + ListResourcesRequest(), + )).resources; + final resource = resources.singleWhere( + (r) => + r.uri.startsWith(DartToolingDaemonSupport.runtimeErrorsScheme), + ); + final contents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + final overflowMatcher = isA().having( + (c) => c.text, + 'text', + contains('A RenderFlex overflowed by'), + ); + expect(contents.single, overflowMatcher); + + final resourceUpdatedQueue = StreamQueue( + serverConnection.resourceUpdated, + ); + await serverConnection.subscribeResource( + SubscribeRequest(uri: resource.uri), + ); + await pumpEventQueue(); + + await testHarness.callToolWithRetry( + CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), + ); + + expect( + await resourceUpdatedQueue.next, + isA().having( + (n) => n.uri, + 'uri', + resource.uri, + ), + ); + + final newContents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + expect(newContents, [overflowMatcher, overflowMatcher]); + + // Now hot reload but clear previous errors, should see exactly one + // error only (the new one). + await testHarness.callToolWithRetry( + CallToolRequest( + name: DartToolingDaemonSupport.hotReloadTool.name, + arguments: {'clearRuntimeErrors': 'true'}, + ), + ); + + expect( + await resourceUpdatedQueue.next, + isA().having( + (n) => n.uri, + 'uri', + resource.uri, + ), + ); + + final finalContents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + expect(finalContents.single, overflowMatcher); + }); }); test('can get the widget tree', () async { From d737325d699955221698c6d3b844a2fba38fa700 Mon Sep 17 00:00:00 2001 From: Jake MacDonald Date: Wed, 30 Apr 2025 09:39:41 -0700 Subject: [PATCH 07/13] get tests passing --- pkgs/dart_mcp/CHANGELOG.md | 2 + pkgs/dart_mcp/lib/src/shared.dart | 2 +- .../lib/src/mixins/dtd.dart | 47 ++- .../test/test_harness.dart | 57 +++- .../test/tools/dtd_test.dart | 316 ++++++++++-------- 5 files changed, 258 insertions(+), 166 deletions(-) diff --git a/pkgs/dart_mcp/CHANGELOG.md b/pkgs/dart_mcp/CHANGELOG.md index b7ee4eb..e3e6a28 100644 --- a/pkgs/dart_mcp/CHANGELOG.md +++ b/pkgs/dart_mcp/CHANGELOG.md @@ -18,6 +18,8 @@ `ResourceListChangedNotification`s and `ResourceUpdatedNotification`s. The delay can be modified by overriding `ResourcesSupport.resourceUpdateThrottleDelay`. +- Only send notifications if the peer is still connected. Fixes issues where + notifications are delayed due to throttling and the client has since closed. - **Breaking**: Fixed paginated result subtypes to use `nextCursor` instead of `cursor` as the key for the next cursor. - **Breaking**: Change the `ProgressNotification.progress` and diff --git a/pkgs/dart_mcp/lib/src/shared.dart b/pkgs/dart_mcp/lib/src/shared.dart index f613bba..908a591 100644 --- a/pkgs/dart_mcp/lib/src/shared.dart +++ b/pkgs/dart_mcp/lib/src/shared.dart @@ -70,7 +70,7 @@ base class MCPBase { /// Sends a notification to the peer. void sendNotification(String method, [Notification? notification]) => - _peer.sendNotification(method, notification); + _peer.isClosed ? null : _peer.sendNotification(method, notification); /// Notifies the peer of progress towards completing some request. void notifyProgress(ProgressNotification notification) => diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index 822a95a..c9a906c 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -32,7 +32,7 @@ base mixin DartToolingDaemonSupport /// [VmService] objects are automatically removed from the Map when the /// [VmService] shuts down. @visibleForTesting - final activeVmServices = {}; + final activeVmServices = >{}; /// Whether to await the disposal of all [VmService] objects in /// [activeVmServices] upon server shutdown or loss of DTD connection. @@ -61,7 +61,9 @@ base mixin DartToolingDaemonSupport // the Flutter Widget Inspector for each VM service instance. final future = Future.wait( - activeVmServices.values.map((vmService) => vmService.dispose()), + activeVmServices.values.map( + (vmService) => vmService.then((service) => service.dispose()), + ), ); debugAwaitVmServiceDisposal ? await future : unawaited(future); @@ -88,8 +90,10 @@ base mixin DartToolingDaemonSupport continue; } if (debugSession.vmServiceUri case final vmServiceUri?) { - final vmService = await vmServiceConnectUri(vmServiceUri); - activeVmServices[debugSession.id] = vmService; + final vmService = + await (activeVmServices[debugSession.id] = vmServiceConnectUri( + vmServiceUri, + )); // Start listening for and collecting errors immediately. final errorService = await _AppErrorsListener.forVmService(vmService); final resource = Resource( @@ -217,7 +221,7 @@ base mixin DartToolingDaemonSupport case 'debugSessionStopped': await activeVmServices .remove((e.data['debugSession'] as DebugSession).id) - ?.dispose(); + ?.then((service) => service.dispose()); default: } }); @@ -507,7 +511,7 @@ base mixin DartToolingDaemonSupport // TODO: support selecting a VM Service if more than one are available. final vmService = activeVmServices.values.first; - return await callback(vmService); + return await callback(await vmService); } @visibleForTesting @@ -661,20 +665,31 @@ base mixin DartToolingDaemonSupport /// Listens on a VM service for errors. class _AppErrorsListener { - final List errors = []; + /// All the errors recorded so far (may be cleared explicitly). + final List errors; + + /// A broadcast stream of all errors that come in after you start listening. Stream get errorsStream => _errorsController.stream; + + /// Controller for the [errorsStream]. final StreamController _errorsController; + + /// The listener for Flutter.Error vm service extension events. final StreamSubscription _extensionEventsListener; + + /// The stderr listener on the flutter process. final StreamSubscription _stderrEventsListener; + + /// The vm service instance connected to the flutter app. final VmService _vmService; _AppErrorsListener._( + this.errors, this._errorsController, this._extensionEventsListener, this._stderrEventsListener, this._vmService, ) { - errorsStream.listen(errors.add); _vmService.onDone.then((_) => shutdown()); } @@ -686,7 +701,12 @@ class _AppErrorsListener { /// which may be an already existing instance. static Future<_AppErrorsListener> forVmService(VmService vmService) async { return _errorListeners[vmService] ??= await () async { - final errorsController = StreamController(); + // Needs to be a broadcast stream because we use it to add errors to the + // list but also expose it to clients so they can know when new errors + // are added. + final errorsController = StreamController.broadcast(); + final errors = []; + errorsController.stream.listen(errors.add); // We need to listen to streams with history so that we can get errors // that occurred before this tool call. // TODO(https://github.com/dart-lang/ai/issues/57): this can result in @@ -712,6 +732,7 @@ class _AppErrorsListener { await vmService.streamListen(EventStreams.kExtension); await vmService.streamListen(EventStreams.kStderr); return _AppErrorsListener._( + errors, errorsController, extensionEvents, stderrEvents, @@ -725,8 +746,12 @@ class _AppErrorsListener { await _errorsController.close(); await _extensionEventsListener.cancel(); await _stderrEventsListener.cancel(); - await _vmService.streamCancel(EventStreams.kExtension); - await _vmService.streamCancel(EventStreams.kStderr); + try { + await _vmService.streamCancel(EventStreams.kExtension); + await _vmService.streamCancel(EventStreams.kStderr); + } on RPCError catch (_) { + // The vm service might already be disposed in which causes these to fail. + } } } diff --git a/pkgs/dart_tooling_mcp_server/test/test_harness.dart b/pkgs/dart_tooling_mcp_server/test/test_harness.dart index 0dc4a76..ece4b4c 100644 --- a/pkgs/dart_tooling_mcp_server/test/test_harness.dart +++ b/pkgs/dart_tooling_mcp_server/test/test_harness.dart @@ -87,7 +87,7 @@ class TestHarness { isFlutter: isFlutter, args: args, ); - fakeEditorExtension.debugSessions.add(session); + fakeEditorExtension.addDebugSession(session); final root = rootForPath(projectRoot); final roots = (await mcpClient.handleListRoots(ListRootsRequest())).roots; if (!roots.any((r) => r.uri == root.uri)) { @@ -95,7 +95,7 @@ class TestHarness { } unawaited( session.appProcess.exitCode.then((_) { - fakeEditorExtension.debugSessions.remove(session); + fakeEditorExtension.removeDebugSession(session); }), ); return session; @@ -160,6 +160,7 @@ final class AppDebugSession { final String projectRoot; final String vmServiceUri; final bool isFlutter; + final String id; AppDebugSession._({ required this.appProcess, @@ -167,6 +168,7 @@ final class AppDebugSession { required this.projectRoot, required this.appPath, required this.isFlutter, + required this.id, }); static Future _start( @@ -218,6 +220,7 @@ final class AppDebugSession { projectRoot: projectRoot, appPath: appPath, isFlutter: isFlutter, + id: FakeEditorExtension.nextId.toString(), ); } @@ -229,6 +232,16 @@ final class AppDebugSession { } await process.shouldExit(0); } + + /// Returns this as the Editor service representation. + DebugSession asEditorDebugSession({required bool includeVmServiceUri}) => + DebugSession( + debuggerType: isFlutter ? 'Flutter' : 'Dart', + id: id, + name: 'Test app', + projectRootPath: projectRoot, + vmServiceUri: includeVmServiceUri ? vmServiceUri : null, + ); } /// A basic MCP client which is started as a part of the harness. @@ -248,15 +261,17 @@ final class DartToolingMCPClient extends MCPClient with RootsSupport { /// This class registers a similar extension for a normal `flutter run` process, /// without having the normal editor extension in place. class FakeEditorExtension { - final List debugSessions = []; + Iterable get debugSessions => _debugSessions; + final List _debugSessions = []; final TestProcess dtdProcess; final DartToolingDaemon dtd; final String dtdUri; - int get nextId => ++_nextId; - int _nextId = 0; FakeEditorExtension._(this.dtd, this.dtdProcess, this.dtdUri); + static int get nextId => ++_nextId; + static int _nextId = 0; + static Future connect() async { final dtdProcess = await TestProcess.start('dart', ['tooling-daemon']); final dtdUri = await _getDTDUri(dtdProcess); @@ -266,18 +281,36 @@ class FakeEditorExtension { return extension; } + void addDebugSession(AppDebugSession session) async { + _debugSessions.add(session); + await dtd.postEvent( + 'Editor', + 'debugSessionStarted', + session.asEditorDebugSession(includeVmServiceUri: false), + ); + // Fake a delay between session start and session ready (vm service URI is + // known). + await Future.delayed(const Duration(milliseconds: 10)); + await dtd.postEvent( + 'Editor', + 'debugSessionChanged', + session.asEditorDebugSession(includeVmServiceUri: true), + ); + } + + void removeDebugSession(AppDebugSession session) async { + _debugSessions.remove(session); + await dtd.postEvent('Editor', 'debugSessionStopped', { + 'debugSession': session.asEditorDebugSession(includeVmServiceUri: false), + }); + } + Future _registerService() async { await dtd.registerService('Editor', 'getDebugSessions', (request) async { return GetDebugSessionsResponse( debugSessions: [ for (var debugSession in debugSessions) - DebugSession( - debuggerType: debugSession.isFlutter ? 'Flutter' : 'Dart', - id: nextId.toString(), - name: 'Test app', - projectRootPath: debugSession.projectRoot, - vmServiceUri: debugSession.vmServiceUri, - ), + debugSession.asEditorDebugSession(includeVmServiceUri: true), ], ); }); diff --git a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart index 40e0389..7d53779 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart @@ -64,147 +64,6 @@ void main() { ]); }); - group('runtime errors', () { - final errorCountRegex = RegExp(r'Found \d+ errors?:'); - - setUp(() async { - await testHarness.startDebugSession( - counterAppPath, - 'lib/main.dart', - isFlutter: true, - args: ['--dart-define=include_layout_error=true'], - ); - }); - - test('can be read and cleared using the tools', () async { - final tools = - (await testHarness.mcpServerConnection.listTools()).tools; - final runtimeErrorsTool = tools.singleWhere( - (t) => t.name == DartToolingDaemonSupport.getRuntimeErrorsTool.name, - ); - final runtimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest(name: runtimeErrorsTool.name), - ); - - expect(runtimeErrorsResult.isError, isNot(true)); - expect( - (runtimeErrorsResult.content.first as TextContent).text, - contains(errorCountRegex), - ); - expect( - (runtimeErrorsResult.content[1] as TextContent).text, - contains('A RenderFlex overflowed by'), - ); - - final clearRuntimeErrorsTool = tools.singleWhere( - (t) => - t.name == DartToolingDaemonSupport.clearRuntimeErrorsTool.name, - ); - final clearRuntimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest(name: clearRuntimeErrorsTool.name), - ); - expect(clearRuntimeErrorsResult.isError, isNot(true)); - - final nextResult = await testHarness.callToolWithRetry( - CallToolRequest(name: runtimeErrorsTool.name), - ); - expect( - (nextResult.content.first as TextContent).text, - contains('No runtime errors found'), - ); - - // Trigger a hot reload, should see the error again. - await testHarness.callToolWithRetry( - CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), - ); - - final finalRuntimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest(name: runtimeErrorsTool.name), - ); - expect( - (finalRuntimeErrorsResult.content.first as TextContent).text, - contains(errorCountRegex), - ); - expect( - (finalRuntimeErrorsResult.content[1] as TextContent).text, - contains('A RenderFlex overflowed by'), - ); - }); - - test('can be read and subscribed to as a resource', () async { - final serverConnection = testHarness.mcpServerConnection; - final resources = - (await serverConnection.listResources( - ListResourcesRequest(), - )).resources; - final resource = resources.singleWhere( - (r) => - r.uri.startsWith(DartToolingDaemonSupport.runtimeErrorsScheme), - ); - final contents = - (await serverConnection.readResource( - ReadResourceRequest(uri: resource.uri), - )).contents; - final overflowMatcher = isA().having( - (c) => c.text, - 'text', - contains('A RenderFlex overflowed by'), - ); - expect(contents.single, overflowMatcher); - - final resourceUpdatedQueue = StreamQueue( - serverConnection.resourceUpdated, - ); - await serverConnection.subscribeResource( - SubscribeRequest(uri: resource.uri), - ); - await pumpEventQueue(); - - await testHarness.callToolWithRetry( - CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), - ); - - expect( - await resourceUpdatedQueue.next, - isA().having( - (n) => n.uri, - 'uri', - resource.uri, - ), - ); - - final newContents = - (await serverConnection.readResource( - ReadResourceRequest(uri: resource.uri), - )).contents; - expect(newContents, [overflowMatcher, overflowMatcher]); - - // Now hot reload but clear previous errors, should see exactly one - // error only (the new one). - await testHarness.callToolWithRetry( - CallToolRequest( - name: DartToolingDaemonSupport.hotReloadTool.name, - arguments: {'clearRuntimeErrors': 'true'}, - ), - ); - - expect( - await resourceUpdatedQueue.next, - isA().having( - (n) => n.uri, - 'uri', - resource.uri, - ), - ); - - final finalContents = - (await serverConnection.readResource( - ReadResourceRequest(uri: resource.uri), - )).contents; - expect(finalContents.single, overflowMatcher); - }); - }); - test('can get the widget tree', () async { await testHarness.startDebugSession( counterAppPath, @@ -313,7 +172,7 @@ void main() { final children = widgetTree['children'] as List; final firstWidgetId = (children.first as Map)['valueId']; - final appVmService = server.activeVmServices.values.first; + final appVmService = await server.activeVmServices.values.first; final vm = await appVmService.getVM(); await appVmService.callServiceExtension( 'ext.flutter.inspector.setSelectionById', @@ -362,6 +221,179 @@ void main() { ); }); }); + + group('runtime errors', () { + final errorCountRegex = RegExp(r'Found \d+ errors?:'); + + setUp(() async { + await testHarness.startDebugSession( + counterAppPath, + 'lib/main.dart', + isFlutter: true, + args: ['--dart-define=include_layout_error=true'], + ); + }); + + test('can be read and cleared using the tools', () async { + final tools = + (await testHarness.mcpServerConnection.listTools()).tools; + final runtimeErrorsTool = tools.singleWhere( + (t) => t.name == DartToolingDaemonSupport.getRuntimeErrorsTool.name, + ); + late CallToolResult runtimeErrorsResult; + + // Give the errors at most a second to come through. + var count = 0; + while (true) { + runtimeErrorsResult = await testHarness.callToolWithRetry( + CallToolRequest(name: runtimeErrorsTool.name), + ); + expect(runtimeErrorsResult.isError, isNot(true)); + final firstText = + (runtimeErrorsResult.content.first as TextContent).text; + if (errorCountRegex.hasMatch(firstText)) { + break; + } else if (++count > 10) { + fail('No errors found, expected at least one'); + } else { + await Future.delayed(const Duration(milliseconds: 100)); + } + } + expect( + (runtimeErrorsResult.content[1] as TextContent).text, + contains('A RenderFlex overflowed by'), + ); + + final clearRuntimeErrorsTool = tools.singleWhere( + (t) => + t.name == DartToolingDaemonSupport.clearRuntimeErrorsTool.name, + ); + final clearRuntimeErrorsResult = await testHarness.callToolWithRetry( + CallToolRequest(name: clearRuntimeErrorsTool.name), + ); + expect(clearRuntimeErrorsResult.isError, isNot(true)); + + final nextResult = await testHarness.callToolWithRetry( + CallToolRequest(name: runtimeErrorsTool.name), + ); + expect( + (nextResult.content.first as TextContent).text, + contains('No runtime errors found'), + ); + + // Trigger a hot reload, should see the error again. + await testHarness.callToolWithRetry( + CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), + ); + + final finalRuntimeErrorsResult = await testHarness.callToolWithRetry( + CallToolRequest(name: runtimeErrorsTool.name), + ); + expect( + (finalRuntimeErrorsResult.content.first as TextContent).text, + contains(errorCountRegex), + ); + expect( + (finalRuntimeErrorsResult.content[1] as TextContent).text, + contains('A RenderFlex overflowed by'), + ); + }); + + test('can be read and subscribed to as a resource', () async { + final serverConnection = testHarness.mcpServerConnection; + final onResourceListChanged = + serverConnection.resourceListChanged.first; + var resources = + (await serverConnection.listResources( + ListResourcesRequest(), + )).resources; + if (resources.runtimeErrors.isEmpty) { + await onResourceListChanged; + resources = + (await serverConnection.listResources( + ListResourcesRequest(), + )).resources; + } + final resource = resources.runtimeErrors.single; + + final resourceUpdatedQueue = StreamQueue( + serverConnection.resourceUpdated, + ); + await serverConnection.subscribeResource( + SubscribeRequest(uri: resource.uri), + ); + await pumpEventQueue(); + var contents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + final overflowMatcher = isA().having( + (c) => c.text, + 'text', + contains('A RenderFlex overflowed by'), + ); + // If we haven't seen errors initially, then listen for updates and + // re-read the resource. + if (contents.isEmpty) { + await resourceUpdatedQueue.next; + contents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + } + expect(contents.single, overflowMatcher); + await pumpEventQueue(); + + await testHarness.callToolWithRetry( + CallToolRequest(name: DartToolingDaemonSupport.hotReloadTool.name), + ); + + expect( + await resourceUpdatedQueue.next, + isA().having( + (n) => n.uri, + 'uri', + resource.uri, + ), + ); + + final newContents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + expect(newContents, [overflowMatcher, overflowMatcher]); + + // Now hot reload but clear previous errors, should see exactly one + // error only (the new one). + await testHarness.callToolWithRetry( + CallToolRequest( + name: DartToolingDaemonSupport.hotReloadTool.name, + arguments: {'clearRuntimeErrors': true}, + ), + ); + + expect( + await resourceUpdatedQueue.next, + isA().having( + (n) => n.uri, + 'uri', + resource.uri, + ), + ); + + final finalContents = + (await serverConnection.readResource( + ReadResourceRequest(uri: resource.uri), + )).contents; + expect(finalContents.single, overflowMatcher); + }); + }); }); }); } + +extension on Iterable { + Iterable get runtimeErrors => where( + (r) => r.uri.startsWith(DartToolingDaemonSupport.runtimeErrorsScheme), + ); +} From 4aba16f21b753921ccf64d7583ea6008b05553d8 Mon Sep 17 00:00:00 2001 From: Jake MacDonald Date: Wed, 30 Apr 2025 10:32:52 -0700 Subject: [PATCH 08/13] try to ensure the debug session is shut down before shutting down dtd --- pkgs/dart_tooling_mcp_server/test/test_harness.dart | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/test/test_harness.dart b/pkgs/dart_tooling_mcp_server/test/test_harness.dart index ece4b4c..a9c2578 100644 --- a/pkgs/dart_tooling_mcp_server/test/test_harness.dart +++ b/pkgs/dart_tooling_mcp_server/test/test_harness.dart @@ -87,16 +87,16 @@ class TestHarness { isFlutter: isFlutter, args: args, ); - fakeEditorExtension.addDebugSession(session); + await fakeEditorExtension.addDebugSession(session); final root = rootForPath(projectRoot); final roots = (await mcpClient.handleListRoots(ListRootsRequest())).roots; if (!roots.any((r) => r.uri == root.uri)) { mcpClient.addRoot(root); } unawaited( - session.appProcess.exitCode.then((_) { - fakeEditorExtension.removeDebugSession(session); - }), + session.appProcess.exitCode.then( + (_) => fakeEditorExtension.removeDebugSession(session), + ), ); return session; } @@ -281,7 +281,7 @@ class FakeEditorExtension { return extension; } - void addDebugSession(AppDebugSession session) async { + Future addDebugSession(AppDebugSession session) async { _debugSessions.add(session); await dtd.postEvent( 'Editor', @@ -298,7 +298,7 @@ class FakeEditorExtension { ); } - void removeDebugSession(AppDebugSession session) async { + Future removeDebugSession(AppDebugSession session) async { _debugSessions.remove(session); await dtd.postEvent('Editor', 'debugSessionStopped', { 'debugSession': session.asEditorDebugSession(includeVmServiceUri: false), @@ -317,6 +317,7 @@ class FakeEditorExtension { } Future shutdown() async { + await _debugSessions.toList().map(removeDebugSession).wait; await dtdProcess.kill(); await dtd.close(); } From d23a3d3ca9d47e4bf73a4e5cc3da3aa6eb272075 Mon Sep 17 00:00:00 2001 From: Jake MacDonald Date: Wed, 30 Apr 2025 10:42:53 -0700 Subject: [PATCH 09/13] remove debug sessions in stopDebugSession instead of when the app process dies --- .../test/test_harness.dart | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/test/test_harness.dart b/pkgs/dart_tooling_mcp_server/test/test_harness.dart index a9c2578..0d5604f 100644 --- a/pkgs/dart_tooling_mcp_server/test/test_harness.dart +++ b/pkgs/dart_tooling_mcp_server/test/test_harness.dart @@ -93,16 +93,12 @@ class TestHarness { if (!roots.any((r) => r.uri == root.uri)) { mcpClient.addRoot(root); } - unawaited( - session.appProcess.exitCode.then( - (_) => fakeEditorExtension.removeDebugSession(session), - ), - ); return session; } /// Stops an app debug session. Future stopDebugSession(AppDebugSession session) async { + await fakeEditorExtension.removeDebugSession(session); await AppDebugSession.kill(session.appProcess, session.isFlutter); } @@ -299,10 +295,13 @@ class FakeEditorExtension { } Future removeDebugSession(AppDebugSession session) async { - _debugSessions.remove(session); - await dtd.postEvent('Editor', 'debugSessionStopped', { - 'debugSession': session.asEditorDebugSession(includeVmServiceUri: false), - }); + if (_debugSessions.remove(session)) { + await dtd.postEvent('Editor', 'debugSessionStopped', { + 'debugSession': session.asEditorDebugSession( + includeVmServiceUri: false, + ), + }); + } } Future _registerService() async { From 8f3d33d2b199b6e42471e33ca581df18b8648be6 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Thu, 1 May 2025 15:26:24 +0000 Subject: [PATCH 10/13] drop the clear_runtime_errors tool, add option to the get_runtime_errors to clear them as well --- .../lib/src/mixins/dtd.dart | 52 ++++--------------- .../test/tools/dtd_test.dart | 16 ++---- 2 files changed, 14 insertions(+), 54 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index c9a906c..33ea932 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -125,7 +125,6 @@ base mixin DartToolingDaemonSupport FutureOr initialize(InitializeRequest request) async { registerTool(connectTool, _connect); registerTool(getRuntimeErrorsTool, runtimeErrors); - registerTool(clearRuntimeErrorsTool, clearRuntimeErrors); // TODO: these tools should only be registered for Flutter applications, or // they should return an error when used against a pure Dart app (or a @@ -346,29 +345,6 @@ base mixin DartToolingDaemonSupport ); } - /// Clears runtime errors from the currently running app. - /// - // TODO: support passing a debug session id when there is more than one debug - // session. - Future clearRuntimeErrors(CallToolRequest request) async { - return _callOnVmService( - callback: (vmService) async { - try { - final errorService = await _AppErrorsListener.forVmService(vmService); - errorService.errors.clear(); - return CallToolResult( - content: [TextContent(text: 'Runtime errors cleared.')], - ); - } catch (e) { - return CallToolResult( - isError: true, - content: [TextContent(text: 'Failed to clear runtime errors: $e')], - ); - } - }, - ); - } - /// Retrieves runtime errors from the currently running app. /// /// If more than one debug session is active, then it just uses the first one. @@ -387,7 +363,7 @@ base mixin DartToolingDaemonSupport content: [TextContent(text: 'No runtime errors found.')], ); } - return CallToolResult( + final result = CallToolResult( content: [ TextContent( text: @@ -397,6 +373,10 @@ base mixin DartToolingDaemonSupport ...errors.map((e) => TextContent(text: e.toString())), ], ); + if (request.arguments?['clearRuntimeErrors'] == true) { + errorService.errors.clear(); + } + return result; } catch (e) { return CallToolResult( isError: true, @@ -528,20 +508,6 @@ base mixin DartToolingDaemonSupport ), ); - @visibleForTesting - static final clearRuntimeErrorsTool = Tool( - name: 'clear_runtime_errors', - description: - 'Clears the list of runtime errors that have occurred in the active ' - 'Dart or Flutter application. Requires "${connectTool.name}" to be ' - 'successfully called first.', - annotations: ToolAnnotations( - title: 'Clear runtime errors', - destructiveHint: true, - ), - inputSchema: Schema.object(), - ); - @visibleForTesting static final getRuntimeErrorsTool = Tool( name: 'get_runtime_errors', @@ -555,11 +521,11 @@ base mixin DartToolingDaemonSupport ), inputSchema: Schema.object( properties: { - 'since': Schema.int( + 'clearRuntimeErrors': Schema.bool( + title: 'Whether to clear the runtime errors after retrieving them.', description: - 'Only return errors that occurred after this timestamp (in ' - 'milliseconds since epoch). If not provided then all errors will ' - 'be returned.', + 'This is useful to clear out old errors that may no longer be ' + 'relevant before reading them again.', ), }, ), diff --git a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart index 7d53779..49761af 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart @@ -234,7 +234,7 @@ void main() { ); }); - test('can be read and cleared using the tools', () async { + test('can be read and cleared using the tool', () async { final tools = (await testHarness.mcpServerConnection.listTools()).tools; final runtimeErrorsTool = tools.singleWhere( @@ -246,7 +246,10 @@ void main() { var count = 0; while (true) { runtimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest(name: runtimeErrorsTool.name), + CallToolRequest( + name: runtimeErrorsTool.name, + arguments: {'clearRuntimeErrors': true}, + ), ); expect(runtimeErrorsResult.isError, isNot(true)); final firstText = @@ -264,15 +267,6 @@ void main() { contains('A RenderFlex overflowed by'), ); - final clearRuntimeErrorsTool = tools.singleWhere( - (t) => - t.name == DartToolingDaemonSupport.clearRuntimeErrorsTool.name, - ); - final clearRuntimeErrorsResult = await testHarness.callToolWithRetry( - CallToolRequest(name: clearRuntimeErrorsTool.name), - ); - expect(clearRuntimeErrorsResult.isError, isNot(true)); - final nextResult = await testHarness.callToolWithRetry( CallToolRequest(name: runtimeErrorsTool.name), ); From 8ee128c4645bce2d859a75aa3373973eee83c0ec Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Fri, 2 May 2025 16:07:06 +0000 Subject: [PATCH 11/13] code review updates --- pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart | 7 +++---- pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index 33ea932..afb3b45 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -26,11 +26,11 @@ base mixin DartToolingDaemonSupport /// ready to be invoked. bool _getDebugSessionsReady = false; - /// A Map of [VmService] objects by their associated VM Service URI - /// (represented as a String). + /// A Map of [VmService] object [Future]s by their associated + /// [DebugSession.id]. /// /// [VmService] objects are automatically removed from the Map when the - /// [VmService] shuts down. + /// [DebugSession] shuts down. @visibleForTesting final activeVmServices = >{}; @@ -114,7 +114,6 @@ base mixin DartToolingDaemonSupport vmService.onDone.then((_) { removeResource(resource.uri); activeVmServices.remove(debugSession.id); - vmService.dispose(); // This will also shut down the `errorService`. }), ); } diff --git a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart index 49761af..3bd59a7 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart @@ -267,6 +267,7 @@ void main() { contains('A RenderFlex overflowed by'), ); + // We cleared the errors in the previous call, shouldn't see any here. final nextResult = await testHarness.callToolWithRetry( CallToolRequest(name: runtimeErrorsTool.name), ); From 0e759300bd9f149de345711fdd09f5da3e8721d6 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Fri, 2 May 2025 16:18:37 +0000 Subject: [PATCH 12/13] add more constants for map keys --- .../lib/src/mixins/analyzer.dart | 11 ++++++----- pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart | 10 ++++++---- .../lib/src/utils/constants.dart | 2 ++ pkgs/dart_tooling_mcp_server/test/test_harness.dart | 3 ++- .../test/tools/analyzer_test.dart | 5 +++-- pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart | 5 +++-- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/analyzer.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/analyzer.dart index 953fccd..72bc444 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/analyzer.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/analyzer.dart @@ -13,6 +13,7 @@ import 'package:language_server_protocol/protocol_generated.dart' as lsp; import 'package:meta/meta.dart'; import '../lsp/wire_format.dart'; +import '../utils/constants.dart'; /// Mix this in to any MCPServer to add support for analyzing Dart projects. /// @@ -245,7 +246,7 @@ base mixin DartAnalyzerSupport for (var entry in diagnostics.entries) { for (var diagnostic in entry.value) { final diagnosticJson = diagnostic.toJson(); - diagnosticJson['uri'] = entry.key.toString(); + diagnosticJson[ParameterNames.uri] = entry.key.toString(); messages.add(TextContent(text: jsonEncode(diagnosticJson))); } } @@ -263,7 +264,7 @@ base mixin DartAnalyzerSupport final errorResult = await _ensurePrerequisites(request); if (errorResult != null) return errorResult; - final query = request.arguments!['query'] as String; + final query = request.arguments![ParameterNames.query] as String; final result = await _lspConnection.sendRequest( lsp.Method.workspace_symbol.toString(), lsp.WorkspaceSymbolParams(query: query).toJson(), @@ -306,7 +307,7 @@ base mixin DartAnalyzerSupport ); diagnostics[diagnosticParams.uri] = diagnosticParams.diagnostics; log(LoggingLevel.debug, { - 'uri': diagnosticParams.uri, + ParameterNames.uri: diagnosticParams.uri, 'diagnostics': diagnosticParams.diagnostics.map((d) => d.toJson()).toList(), }); @@ -369,14 +370,14 @@ base mixin DartAnalyzerSupport description: 'Look up a symbol or symbols in all workspaces by name.', inputSchema: Schema.object( properties: { - 'query': Schema.string( + ParameterNames.query: Schema.string( description: 'Queries are matched based on a case-insensitive partial name ' 'match, and do not support complex pattern matching, regexes, ' 'or scoped lookups.', ), }, - required: ['query'], + required: [ParameterNames.query], ), annotations: ToolAnnotations(title: 'Project search', readOnlyHint: true), ); diff --git a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart index afb3b45..70ac58b 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/mixins/dtd.dart @@ -13,6 +13,8 @@ import 'package:meta/meta.dart'; import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service_io.dart'; +import '../utils/constants.dart'; + /// Mix this in to any MCPServer to add support for connecting to the Dart /// Tooling Daemon and all of its associated functionality (see /// https://pub.dev/packages/dtd). @@ -149,7 +151,7 @@ base mixin DartToolingDaemonSupport return _dtdAlreadyConnected; } - if (request.arguments?['uri'] == null) { + if (request.arguments?[ParameterNames.uri] == null) { return CallToolResult( isError: true, content: [ @@ -160,7 +162,7 @@ base mixin DartToolingDaemonSupport try { _dtd = await DartToolingDaemon.connect( - Uri.parse(request.arguments!['uri'] as String), + Uri.parse(request.arguments![ParameterNames.uri] as String), ); unawaited(_dtd!.done.then((_) async => await _resetDtd())); @@ -502,8 +504,8 @@ base mixin DartToolingDaemonSupport 'command. Do not just make up a random URI to pass.', annotations: ToolAnnotations(title: 'Connect to DTD', readOnlyHint: true), inputSchema: Schema.object( - properties: {'uri': Schema.string()}, - required: const ['uri'], + properties: {ParameterNames.uri: Schema.string()}, + required: const [ParameterNames.uri], ), ); diff --git a/pkgs/dart_tooling_mcp_server/lib/src/utils/constants.dart b/pkgs/dart_tooling_mcp_server/lib/src/utils/constants.dart index acd5a69..0b2062f 100644 --- a/pkgs/dart_tooling_mcp_server/lib/src/utils/constants.dart +++ b/pkgs/dart_tooling_mcp_server/lib/src/utils/constants.dart @@ -7,6 +7,8 @@ extension ParameterNames on Never { static const command = 'command'; static const packageName = 'packageName'; static const paths = 'paths'; + static const query = 'query'; static const root = 'root'; static const roots = 'roots'; + static const uri = 'uri'; } diff --git a/pkgs/dart_tooling_mcp_server/test/test_harness.dart b/pkgs/dart_tooling_mcp_server/test/test_harness.dart index 0d5604f..d0e44ea 100644 --- a/pkgs/dart_tooling_mcp_server/test/test_harness.dart +++ b/pkgs/dart_tooling_mcp_server/test/test_harness.dart @@ -10,6 +10,7 @@ import 'package:async/async.dart'; import 'package:dart_mcp/client.dart'; import 'package:dart_tooling_mcp_server/src/mixins/dtd.dart'; import 'package:dart_tooling_mcp_server/src/server.dart'; +import 'package:dart_tooling_mcp_server/src/utils/constants.dart'; import 'package:dtd/dtd.dart'; import 'package:path/path.dart' as p; import 'package:process/process.dart'; @@ -116,7 +117,7 @@ class TestHarness { final result = await callToolWithRetry( CallToolRequest( name: connectTool.name, - arguments: {'uri': fakeEditorExtension.dtdUri}, + arguments: {ParameterNames.uri: fakeEditorExtension.dtdUri}, ), ); diff --git a/pkgs/dart_tooling_mcp_server/test/tools/analyzer_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/analyzer_test.dart index 10506bf..08fef38 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/analyzer_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/analyzer_test.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:dart_mcp/server.dart'; import 'package:dart_tooling_mcp_server/src/mixins/analyzer.dart'; +import 'package:dart_tooling_mcp_server/src/utils/constants.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; @@ -96,7 +97,7 @@ void main() { final result = await testHarness.callToolWithRetry( CallToolRequest( name: DartAnalyzerSupport.resolveWorkspaceSymbolTool.name, - arguments: {'query': 'DartAnalyzerSupport'}, + arguments: {ParameterNames.query: 'DartAnalyzerSupport'}, ), ); expect(result.isError, isNot(true)); @@ -130,7 +131,7 @@ void main() { final result = await testHarness.callToolWithRetry( CallToolRequest( name: DartAnalyzerSupport.resolveWorkspaceSymbolTool.name, - arguments: {'query': 'DartAnalyzerSupport'}, + arguments: {ParameterNames.query: 'DartAnalyzerSupport'}, ), expectError: true, ); diff --git a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart index 3bd59a7..ba5704a 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart @@ -7,6 +7,7 @@ import 'dart:convert'; import 'package:async/async.dart'; import 'package:dart_mcp/server.dart'; import 'package:dart_tooling_mcp_server/src/mixins/dtd.dart'; +import 'package:dart_tooling_mcp_server/src/utils/constants.dart'; import 'package:test/test.dart'; import 'package:vm_service/vm_service.dart'; @@ -347,7 +348,7 @@ void main() { await resourceUpdatedQueue.next, isA().having( (n) => n.uri, - 'uri', + ParameterNames.uri, resource.uri, ), ); @@ -371,7 +372,7 @@ void main() { await resourceUpdatedQueue.next, isA().having( (n) => n.uri, - 'uri', + ParameterNames.uri, resource.uri, ), ); From 931ccecefcdcf7753535c2e34050b517c5994c0f Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Fri, 2 May 2025 16:28:10 +0000 Subject: [PATCH 13/13] make errors test not assume exactly one error, just increasing/decreasing numbers of errors --- .../test/tools/dtd_test.dart | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart index ba5704a..b58f100 100644 --- a/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_tooling_mcp_server/test/tools/dtd_test.dart @@ -319,7 +319,7 @@ void main() { SubscribeRequest(uri: resource.uri), ); await pumpEventQueue(); - var contents = + var originalContents = (await serverConnection.readResource( ReadResourceRequest(uri: resource.uri), )).contents; @@ -330,14 +330,15 @@ void main() { ); // If we haven't seen errors initially, then listen for updates and // re-read the resource. - if (contents.isEmpty) { + if (originalContents.isEmpty) { await resourceUpdatedQueue.next; - contents = + originalContents = (await serverConnection.readResource( ReadResourceRequest(uri: resource.uri), )).contents; } - expect(contents.single, overflowMatcher); + // Sometimes we get this error logged multiple times + expect(originalContents.first, overflowMatcher); await pumpEventQueue(); await testHarness.callToolWithRetry( @@ -353,14 +354,16 @@ void main() { ), ); + // We should see additional errors (but the exact number is variable). final newContents = (await serverConnection.readResource( ReadResourceRequest(uri: resource.uri), )).contents; - expect(newContents, [overflowMatcher, overflowMatcher]); + expect(newContents.length, greaterThan(originalContents.length)); + expect(newContents.last, overflowMatcher); - // Now hot reload but clear previous errors, should see exactly one - // error only (the new one). + // Now hot reload but clear previous errors, should see fewer errors + // than before after this. await testHarness.callToolWithRetry( CallToolRequest( name: DartToolingDaemonSupport.hotReloadTool.name, @@ -381,7 +384,8 @@ void main() { (await serverConnection.readResource( ReadResourceRequest(uri: resource.uri), )).contents; - expect(finalContents.single, overflowMatcher); + expect(finalContents.length, lessThan(newContents.length)); + expect(finalContents.last, overflowMatcher); }); }); });