From e3164c809123a90b94abdd5c876b0b2b7eea3a2e Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 20 May 2025 21:20:15 +0000 Subject: [PATCH 1/7] only keep the last 10 runtime errors --- pkgs/dart_mcp_server/lib/src/mixins/dtd.dart | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart index efd9524d..96c365f6 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart @@ -109,6 +109,9 @@ base mixin DartToolingDaemonSupport final resource = Resource( uri: '$runtimeErrorsScheme://${debugSession.id}', name: debugSession.name, + description: + 'The last 10 runtime errors seen for debug session ' + '"${debugSession.name}".', ); addResource(resource, (request) async { final errors = errorService.errors; @@ -614,7 +617,7 @@ base mixin DartToolingDaemonSupport static final getRuntimeErrorsTool = Tool( name: 'get_runtime_errors', description: - 'Retrieves the list of runtime errors that have occurred in the active ' + 'Retrieves the list 10 runtime errors that have occurred in the active ' 'Dart or Flutter application. Requires "${connectTool.name}" to be ' 'successfully called first.', annotations: ToolAnnotations( @@ -810,7 +813,12 @@ class _AppErrorsListener { // are added. final errorsController = StreamController.broadcast(); final errors = []; - errorsController.stream.listen(errors.add); + errorsController.stream.listen((e) { + if (errors.length > 10) { + errors.removeAt(0); + } + errors.add(e); + }); // 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 From 744e563fb95e7c4a2d097e32218164726050361c Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 21 May 2025 14:26:18 +0000 Subject: [PATCH 2/7] extract logic into ErrorLog class, add unit tests --- pkgs/dart_mcp_server/lib/src/mixins/dtd.dart | 90 ++++++++++++++----- pkgs/dart_mcp_server/test/tools/dtd_test.dart | 58 ++++++++++++ 2 files changed, 124 insertions(+), 24 deletions(-) diff --git a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart index 96c365f6..caac0232 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart @@ -110,14 +110,14 @@ base mixin DartToolingDaemonSupport uri: '$runtimeErrorsScheme://${debugSession.id}', name: debugSession.name, description: - 'The last 10 runtime errors seen for debug session ' + 'Recent runtime errors seen for debug session ' '"${debugSession.name}".', ); addResource(resource, (request) async { - final errors = errorService.errors; + final errors = errorService.errorLog; return ReadResourceResult( contents: [ - for (var error in errors) + for (var error in errors.errors) TextResourceContents(uri: resource.uri, text: error), ], ); @@ -297,7 +297,7 @@ base mixin DartToolingDaemonSupport (await _AppErrorsListener.forVmService( vmService, this, - )).errors.clear(); + )).errorLog.clear(); } final vm = await vmService.getVM(); @@ -375,9 +375,9 @@ base mixin DartToolingDaemonSupport vmService, this, ); - final errors = errorService.errors; + final errors = errorService.errorLog; - if (errors.isEmpty) { + if (errors.errors.isEmpty) { return CallToolResult( content: [TextContent(text: 'No runtime errors found.')], ); @@ -386,14 +386,14 @@ base mixin DartToolingDaemonSupport content: [ TextContent( text: - 'Found ${errors.length} ' - 'error${errors.length == 1 ? '' : 's'}:\n', + 'Found ${errors.errors.length} ' + 'error${errors.errors.length == 1 ? '' : 's'}:\n', ), - ...errors.map((e) => TextContent(text: e.toString())), + ...errors.errors.map((e) => TextContent(text: e.toString())), ], ); if (request.arguments?['clearRuntimeErrors'] == true) { - errorService.errors.clear(); + errorService.errorLog.clear(); } return result; } catch (e) { @@ -617,9 +617,9 @@ base mixin DartToolingDaemonSupport static final getRuntimeErrorsTool = Tool( name: 'get_runtime_errors', description: - 'Retrieves the list 10 runtime errors that have occurred in the active ' - 'Dart or Flutter application. Requires "${connectTool.name}" to be ' - 'successfully called first.', + 'Retrieves the most recent runtime errors that have occurred in the ' + 'active Dart or Flutter application. Requires "${connectTool.name}" to ' + 'be successfully called first.', annotations: ToolAnnotations( title: 'Get runtime errors', readOnlyHint: true, @@ -770,7 +770,7 @@ base mixin DartToolingDaemonSupport /// Listens on a VM service for errors. class _AppErrorsListener { /// All the errors recorded so far (may be cleared explicitly). - final List errors; + final ErrorLog errorLog; /// A broadcast stream of all errors that come in after you start listening. Stream get errorsStream => _errorsController.stream; @@ -788,7 +788,7 @@ class _AppErrorsListener { final VmService _vmService; _AppErrorsListener._( - this.errors, + this.errorLog, this._errorsController, this._extensionEventsListener, this._stderrEventsListener, @@ -812,13 +812,8 @@ class _AppErrorsListener { // 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((e) { - if (errors.length > 10) { - errors.removeAt(0); - } - errors.add(e); - }); + final errorLog = ErrorLog(); + errorsController.stream.listen(errorLog.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 @@ -857,7 +852,7 @@ class _AppErrorsListener { logger.log(LoggingLevel.error, 'Error subscribing to app errors: $e'); } return _AppErrorsListener._( - errors, + errorLog, errorsController, extensionEvents, stderrEvents, @@ -867,7 +862,7 @@ class _AppErrorsListener { } Future shutdown() async { - errors.clear(); + errorLog.clear(); await _errorsController.close(); await _extensionEventsListener.cancel(); await _stderrEventsListener.cancel(); @@ -966,3 +961,50 @@ extension type DebugSession.fromJson(Map _value) if (vmServiceUri != null) 'vmServiceUri': vmServiceUri, }); } + +@visibleForTesting +/// Manages a log of errors with a maximum size in terms of total characters. +class ErrorLog { + Iterable get errors => _errors; + final List _errors = []; + int _size = 0; + @visibleForTesting + int get size => _size; + + final int _maxSize; + + ErrorLog({ + // One token is ~4 characters. Allow up to 5k tokens by default, so 20k + // characters. + int maxSize = 20000, + }) : _maxSize = maxSize; + + /// Adds a new [error] to the log. + void add(String error) { + if (error.length > _maxSize) { + // If we get a single error over the max size, just trim it and clear + // all other errors. + final trimmed = error.substring(0, _maxSize); + _errors.clear(); + _size = trimmed.length; + _errors.add(trimmed); + } else { + // Otherwise, we append the error and then remove as many errors from the + // front as we need to in order to get under the max size. + _size += error.length; + _errors.add(error); + var removeCount = 0; + while (_size > _maxSize) { + _size -= _errors[removeCount].length; + removeCount++; + } + _errors.removeRange(0, removeCount); + } + } + + /// Clears all errors. + void clear() { + _size = 0; + _errors.clear(); + } +} diff --git a/pkgs/dart_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_mcp_server/test/tools/dtd_test.dart index 83849e30..4edcfc45 100644 --- a/pkgs/dart_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_mcp_server/test/tools/dtd_test.dart @@ -576,6 +576,64 @@ void main() { }); }); }); + + group('ErrorLog', () { + test('adds errors and respects max size', () { + final log = ErrorLog(maxSize: 10); + log.add('abc'); + expect(log.errors, ['abc']); + expect(log.size, 3); + + log.add('defg'); + expect(log.errors, ['abc', 'defg']); + expect(log.size, 7); + + log.add('hijkl'); + expect(log.errors, ['defg', 'hijkl']); + expect(log.size, 9); + + log.add('mnopq'); + expect(log.errors, ['hijkl', 'mnopq']); + expect(log.size, 10); + }); + + test('handles single error larger than max size', () { + final log = ErrorLog(maxSize: 10); + log.add('abcdefghijkl'); + expect(log.errors, ['abcdefghij']); + expect(log.size, 10); + + log.add('mnopqrstuvwxyz'); + expect(log.errors, ['mnopqrstuv']); + expect(log.size, 10); + }); + + test('clear removes all errors', () { + final log = ErrorLog(maxSize: 10); + log + ..add('abc') + ..add('def'); + log.clear(); + expect(log.errors, isEmpty); + expect(log.size, 0); + }); + + test('add, clear,clear and then add again', () { + final log = ErrorLog(maxSize: 10); + log + ..add('abc') + ..add('def'); + log.clear(); + expect(log.errors, isEmpty); + expect(log.size, 0); + log.add('ghi'); + expect(log.errors, ['ghi']); + expect(log.size, 3); + log.add('jklmnopqrstuv'); + expect(log.errors, ['jklmnopqrs']); + expect(log.size, 10); + }); + }); } extension on Iterable { From 4573d717ed3f0473f69d837d031820bfb0573132 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 21 May 2025 14:29:30 +0000 Subject: [PATCH 3/7] minor cleanup --- pkgs/dart_mcp_server/lib/src/mixins/dtd.dart | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart index caac0232..aa429faf 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart @@ -114,10 +114,9 @@ base mixin DartToolingDaemonSupport '"${debugSession.name}".', ); addResource(resource, (request) async { - final errors = errorService.errorLog; return ReadResourceResult( contents: [ - for (var error in errors.errors) + for (var error in errorService.errorLog.errors) TextResourceContents(uri: resource.uri, text: error), ], ); @@ -375,9 +374,9 @@ base mixin DartToolingDaemonSupport vmService, this, ); - final errors = errorService.errorLog; + final errorLog = errorService.errorLog; - if (errors.errors.isEmpty) { + if (errorLog.errors.isEmpty) { return CallToolResult( content: [TextContent(text: 'No runtime errors found.')], ); @@ -386,10 +385,10 @@ base mixin DartToolingDaemonSupport content: [ TextContent( text: - 'Found ${errors.errors.length} ' - 'error${errors.errors.length == 1 ? '' : 's'}:\n', + 'Found ${errorLog.errors.length} ' + 'error${errorLog.errors.length == 1 ? '' : 's'}:\n', ), - ...errors.errors.map((e) => TextContent(text: e.toString())), + ...errorLog.errors.map((e) => TextContent(text: e.toString())), ], ); if (request.arguments?['clearRuntimeErrors'] == true) { @@ -968,6 +967,8 @@ class ErrorLog { Iterable get errors => _errors; final List _errors = []; int _size = 0; + + /// The number of characters used by all errors in the log. @visibleForTesting int get size => _size; From f2f57c3222dfce31ef6f19c3a8aca35d27f43f74 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 21 May 2025 14:37:01 +0000 Subject: [PATCH 4/7] add issues: write permissions to the labeler --- .github/workflows/pull_request_label.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pull_request_label.yml b/.github/workflows/pull_request_label.yml index 54e3df53..1333e86b 100644 --- a/.github/workflows/pull_request_label.yml +++ b/.github/workflows/pull_request_label.yml @@ -13,6 +13,7 @@ on: jobs: label: permissions: + issues: write pull-requests: write runs-on: ubuntu-latest steps: From 0a1c5610d8eb4eec46f12ddd88d5e8903185bad4 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Thu, 22 May 2025 16:37:11 +0000 Subject: [PATCH 5/7] rename to characters --- pkgs/dart_mcp_server/lib/src/mixins/dtd.dart | 14 ++++++------- pkgs/dart_mcp_server/test/tools/dtd_test.dart | 20 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart index aa429faf..24d8f657 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart @@ -966,11 +966,11 @@ extension type DebugSession.fromJson(Map _value) class ErrorLog { Iterable get errors => _errors; final List _errors = []; - int _size = 0; + int _characters = 0; /// The number of characters used by all errors in the log. @visibleForTesting - int get size => _size; + int get characters => _characters; final int _maxSize; @@ -987,16 +987,16 @@ class ErrorLog { // all other errors. final trimmed = error.substring(0, _maxSize); _errors.clear(); - _size = trimmed.length; + _characters = trimmed.length; _errors.add(trimmed); } else { // Otherwise, we append the error and then remove as many errors from the // front as we need to in order to get under the max size. - _size += error.length; + _characters += error.length; _errors.add(error); var removeCount = 0; - while (_size > _maxSize) { - _size -= _errors[removeCount].length; + while (_characters > _maxSize) { + _characters -= _errors[removeCount].length; removeCount++; } _errors.removeRange(0, removeCount); @@ -1005,7 +1005,7 @@ class ErrorLog { /// Clears all errors. void clear() { - _size = 0; + _characters = 0; _errors.clear(); } } diff --git a/pkgs/dart_mcp_server/test/tools/dtd_test.dart b/pkgs/dart_mcp_server/test/tools/dtd_test.dart index 4edcfc45..349a7fe9 100644 --- a/pkgs/dart_mcp_server/test/tools/dtd_test.dart +++ b/pkgs/dart_mcp_server/test/tools/dtd_test.dart @@ -582,30 +582,30 @@ void main() { final log = ErrorLog(maxSize: 10); log.add('abc'); expect(log.errors, ['abc']); - expect(log.size, 3); + expect(log.characters, 3); log.add('defg'); expect(log.errors, ['abc', 'defg']); - expect(log.size, 7); + expect(log.characters, 7); log.add('hijkl'); expect(log.errors, ['defg', 'hijkl']); - expect(log.size, 9); + expect(log.characters, 9); log.add('mnopq'); expect(log.errors, ['hijkl', 'mnopq']); - expect(log.size, 10); + expect(log.characters, 10); }); test('handles single error larger than max size', () { final log = ErrorLog(maxSize: 10); log.add('abcdefghijkl'); expect(log.errors, ['abcdefghij']); - expect(log.size, 10); + expect(log.characters, 10); log.add('mnopqrstuvwxyz'); expect(log.errors, ['mnopqrstuv']); - expect(log.size, 10); + expect(log.characters, 10); }); test('clear removes all errors', () { @@ -615,7 +615,7 @@ void main() { ..add('def'); log.clear(); expect(log.errors, isEmpty); - expect(log.size, 0); + expect(log.characters, 0); }); test('add, clear,clear and then add again', () { @@ -625,13 +625,13 @@ void main() { ..add('def'); log.clear(); expect(log.errors, isEmpty); - expect(log.size, 0); + expect(log.characters, 0); log.add('ghi'); expect(log.errors, ['ghi']); - expect(log.size, 3); + expect(log.characters, 3); log.add('jklmnopqrstuv'); expect(log.errors, ['jklmnopqrs']); - expect(log.size, 10); + expect(log.characters, 10); }); }); } From 1792ecc986608f1a55ab08be33dc0a5edb7dbfb6 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 22 May 2025 09:38:08 -0700 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Nate Bosch --- pkgs/dart_mcp_server/lib/src/mixins/dtd.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart index 24d8f657..929c2923 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart @@ -388,7 +388,8 @@ base mixin DartToolingDaemonSupport 'Found ${errorLog.errors.length} ' 'error${errorLog.errors.length == 1 ? '' : 's'}:\n', ), - ...errorLog.errors.map((e) => TextContent(text: e.toString())), + for (final e in errorLog.errors) + TextContent(text: e.toString()), ], ); if (request.arguments?['clearRuntimeErrors'] == true) { @@ -961,8 +962,9 @@ extension type DebugSession.fromJson(Map _value) }); } -@visibleForTesting + /// Manages a log of errors with a maximum size in terms of total characters. +@visibleForTesting class ErrorLog { Iterable get errors => _errors; final List _errors = []; From a4636efebfd00bf6805b287f823cc1388bf23376 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Thu, 22 May 2025 16:47:50 +0000 Subject: [PATCH 7/7] format --- pkgs/dart_mcp_server/lib/src/mixins/dtd.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart index 929c2923..1dfffe44 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/dtd.dart @@ -388,8 +388,7 @@ base mixin DartToolingDaemonSupport 'Found ${errorLog.errors.length} ' 'error${errorLog.errors.length == 1 ? '' : 's'}:\n', ), - for (final e in errorLog.errors) - TextContent(text: e.toString()), + for (final e in errorLog.errors) TextContent(text: e.toString()), ], ); if (request.arguments?['clearRuntimeErrors'] == true) { @@ -962,7 +961,6 @@ extension type DebugSession.fromJson(Map _value) }); } - /// Manages a log of errors with a maximum size in terms of total characters. @visibleForTesting class ErrorLog {