From 26dff6a44c849db9db87796b74e508dd8f6821ef Mon Sep 17 00:00:00 2001 From: Elias Reis Date: Wed, 13 May 2026 10:05:14 -0300 Subject: [PATCH 1/4] fix(mcp): add timeout and process kill to test tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flutter tests can hang indefinitely when pumpAndSettle() is called without a timeout argument. The MCP server was waiting on the process forever with no way to recover. Adds a timeout_seconds parameter (default 120) to the test tool. When the timeout fires, sends SIGKILL to flutter_tester via pkill — the only reliable mechanism to terminate a hung Flutter test process — then returns an actionable error to the caller. Co-Authored-By: Claude Sonnet 4.6 --- lib/src/mcp/mcp_server.dart | 47 +++++++++++++++++++++++++++++-- test/src/mcp/mcp_server_test.dart | 38 +++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index 544ade81e..46da8502d 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -1,5 +1,5 @@ import 'dart:async'; -import 'dart:io' show stderr; +import 'dart:io' show Process, stderr; import 'package:args/command_runner.dart'; import 'package:dart_mcp/server.dart'; @@ -221,6 +221,13 @@ Only one value can be selected. '(e.g. // coverage:ignore-line). ' 'Only applies to Dart tests (dart: true).', ), + 'timeout_seconds': IntegerSchema( + description: + 'Maximum seconds to wait for the test run before killing ' + 'the Flutter test process. Flutter tests can hang ' + 'indefinitely when pumpAndSettle() is called without a ' + 'timeout argument (default is 10 minutes). Defaults to 120.', + ), }, ), ), @@ -425,8 +432,44 @@ Only one value can be selected. Future _handleTest(CallToolRequest request) async { final args = request.arguments ?? {}; + final timeoutSeconds = (args['timeout_seconds'] as num?)?.toInt() ?? 120; final cliArgs = _parseTest(args); - return _runToolCommand(cliArgs, toolName: 'test'); + try { + return await _runToolCommand( + cliArgs, + toolName: 'test', + ).timeout(Duration(seconds: timeoutSeconds)); + } on TimeoutException { + stderr.writeln( + '[very_good_mcp] Test run timed out after ${timeoutSeconds}s. ' + 'Killing flutter_tester processes.', + ); + // OS-level SIGKILL is the only reliable way to stop a hung + // flutter_tester. package:test timeouts fire internally but the + // Flutter process keeps running regardless. + unawaited( + Process.run( + 'pkill', + ['-KILL', '-f', 'flutter_tester'], + runInShell: true, + ), + ); + return CallToolResult( + content: [ + TextContent( + text: + 'Test run timed out after ${timeoutSeconds}s. ' + 'A Flutter test was hanging — likely an unbounded ' + 'pumpAndSettle() call (default timeout is 10 minutes). ' + 'The flutter_tester process was killed.\n' + 'Fix: replace tester.pumpAndSettle() with ' + 'tester.pump(Duration(milliseconds: 500)) or ' + 'tester.pumpAndSettle(timeout: Duration(seconds: 5)).', + ), + ], + isError: true, + ); + } } Future _handlePackagesGet(CallToolRequest request) async { diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index a8630348a..d43c605f7 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -420,6 +420,44 @@ void main() { contains('"test" failed with exit code'), ); }); + + test( + 'returns timeout error and kills flutter_tester when test hangs', + () async { + // Simulate a hung test — the future never completes. + final hangCompleter = Completer(); + when( + () => mockCommandRunner.run(any()), + ).thenAnswer((_) => hangCompleter.future); + + final response = await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'timeout_seconds': 1}, + ), + ), + ); + + expect(response['error'], isNull); + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + expect(result.isError, isTrue); + expect( + (result.content.first as TextContent).text, + allOf([ + contains('timed out after 1s'), + contains('pumpAndSettle'), + ]), + ); + + // Resolve the dangling future so it doesn't outlive the test. + hangCompleter.complete(0); + }, + timeout: const Timeout(Duration(seconds: 5)), + ); }); group('Tool: packages_get', () { From 0bfd236decceb82ec82420d69d0e3a0e10f26dd4 Mon Sep 17 00:00:00 2001 From: Elias Reis Date: Wed, 13 May 2026 11:18:05 -0300 Subject: [PATCH 2/4] docs(mcp): document timeout_seconds parameter for test tool Co-Authored-By: Claude Sonnet 4.6 --- doc/mcp.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/mcp.md b/doc/mcp.md index 52e89fda2..af4d5eb7f 100644 --- a/doc/mcp.md +++ b/doc/mcp.md @@ -137,12 +137,13 @@ Runs tests in a Dart or Flutter project. "platform": "chrome | vm | android | ios", "dart-define": "foo=bar", "dart-define-from-file": "config.json", - "test_randomize_ordering_seed": "random" + "test_randomize_ordering_seed": "random", + "timeout_seconds": 120 } } ``` -All parameters are optional. When `optimization` is not specified, `--no-optimization` is applied by default. +All parameters are optional. When `optimization` is not specified, `--no-optimization` is applied by default. When `timeout_seconds` is not specified, the test run is killed after 120 seconds. ### `packages_get` From e46c0b9fc375cb0a53022b8f3f62172b9f3a752c Mon Sep 17 00:00:00 2001 From: Elias Reis Date: Mon, 18 May 2026 08:28:02 -0300 Subject: [PATCH 3/4] fix(test): add --timeout flag to very_good test and delegate from MCP Adds a --timeout option to very_good test. When set, a Timer fires pkill -KILL -f flutter_tester and completes the test run with a non-zero exit code. No timeout is applied when the flag is omitted. The MCP server's own .timeout() + pkill block is removed in favour of passing --timeout directly to the CLI, keeping the kill logic in one place. Co-Authored-By: Claude Sonnet 4.6 --- doc/mcp.md | 2 +- lib/src/cli/flutter_cli.dart | 2 + lib/src/cli/test_cli_runner.dart | 24 ++++++++++++ lib/src/commands/test/test.dart | 20 ++++++++++ lib/src/mcp/mcp_server.dart | 48 +++++------------------- site/docs/commands/test.md | 2 + test/src/commands/test/test_test.dart | 4 ++ test/src/mcp/mcp_server_test.dart | 54 +++++++++------------------ 8 files changed, 80 insertions(+), 76 deletions(-) diff --git a/doc/mcp.md b/doc/mcp.md index af4d5eb7f..aa2a8ebb4 100644 --- a/doc/mcp.md +++ b/doc/mcp.md @@ -143,7 +143,7 @@ Runs tests in a Dart or Flutter project. } ``` -All parameters are optional. When `optimization` is not specified, `--no-optimization` is applied by default. When `timeout_seconds` is not specified, the test run is killed after 120 seconds. +All parameters are optional. When `optimization` is not specified, `--no-optimization` is applied by default. When `timeout_seconds` is not specified, no timeout is applied. ### `packages_get` diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart index 494d15015..e4a336791 100644 --- a/lib/src/cli/flutter_cli.dart +++ b/lib/src/cli/flutter_cli.dart @@ -218,6 +218,7 @@ class Flutter { void Function(String)? stderr, GeneratorBuilder buildGenerator = MasonGenerator.fromBundle, List? reportOn, + Duration? timeout, }) async { return TestCLIRunner.test( logger: logger, @@ -238,6 +239,7 @@ class Flutter { stderr: stderr, buildGenerator: buildGenerator, reportOn: reportOn, + timeout: timeout, ); } } diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 76ed12793..4e5d32826 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -105,6 +105,7 @@ class TestCLIRunner { GeneratorBuilder buildGenerator = MasonGenerator.fromBundle, List? reportOn, bool checkIgnore = false, + Duration? timeout, @visibleForTesting VeryGoodTestRunner? overrideTestRunner, }) async { final initialCwd = cwd; @@ -188,6 +189,7 @@ class TestCLIRunner { ], stdout: stdout ?? noop, stderr: stderr ?? noop, + timeout: timeout, ).whenComplete(() async { if (optimizePerformance) { await _cleanupOptimizerFile(cwd); @@ -457,6 +459,7 @@ Future _testCommand({ String cwd = '.', bool collectCoverage = false, List? arguments, + Duration? timeout, }) { const clearLine = '\u001B[2K\r'; @@ -496,14 +499,34 @@ Future _testCommand({ late final StreamSubscription subscription; late final StreamSubscription sigintWatchSubscription; + Timer? timeoutTimer; sigintWatchSubscription = sigintWatch.listen((_) async { + timeoutTimer?.cancel(); await _cleanupOptimizerFile(cwd); await subscription.cancel(); await sigintWatchSubscription.cancel(); return completer.complete(ExitCode.success.code); }); + if (timeout != null) { + timeoutTimer = Timer(timeout, () async { + if (completer.isCompleted) return; + stderr('${clearLine}Tests timed out after ${timeout.inSeconds}s.'); + unawaited( + Process.run('pkill', [ + '-KILL', + '-f', + 'flutter_tester', + ], runInShell: true), + ); + await subscription.cancel(); + await sigintWatchSubscription.cancel(); + unawaited(timerSubscription.cancel()); + completer.complete(ExitCode.unavailable.code); + }); + } + subscription = testRunner( workingDirectory: cwd, @@ -642,6 +665,7 @@ Future _testCommand({ if (event is ExitTestEvent) { if (completer.isCompleted) return; + timeoutTimer?.cancel(); unawaited(subscription.cancel()); unawaited(sigintWatchSubscription.cancel()); diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 57d086a66..26bbe5eea 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -30,6 +30,7 @@ class FlutterTestOptions { required this.reportOn, required this.runSkipped, required this.flavor, + required this.timeout, required this.rest, }); @@ -68,6 +69,12 @@ class FlutterTestOptions { .toList(); final runSkipped = argResults['run-skipped'] as bool; final flavor = argResults['flavor'] as String?; + final timeoutSeconds = int.tryParse( + argResults['timeout'] as String? ?? '', + ); + final timeout = timeoutSeconds != null + ? Duration(seconds: timeoutSeconds) + : null; final rest = argResults.rest; return FlutterTestOptions._( @@ -90,6 +97,7 @@ class FlutterTestOptions { reportOn: reportOn, runSkipped: runSkipped, flavor: flavor, + timeout: timeout, rest: rest, ); } @@ -153,6 +161,9 @@ class FlutterTestOptions { /// The flavor to build for testing. final String? flavor; + /// Maximum time to let tests run before killing the process. + final Duration? timeout; + /// The remaining arguments passed to the test command. final List rest; } @@ -179,6 +190,7 @@ typedef FlutterTestCommand = void Function(String)? stdout, void Function(String)? stderr, List? reportOn, + Duration? timeout, }); /// {@template test_command} @@ -331,6 +343,13 @@ class TestCommand extends Command { 'Build a custom app flavor as defined by platform-specific build ' 'setup. Supports the use of product flavors in Android Gradle ' 'scripts, and the use of custom Xcode schemes.', + ) + ..addOption( + 'timeout', + help: + 'Maximum seconds to let tests run before killing the process. ' + 'Useful when tests hang due to an unbounded pumpAndSettle() call.', + valueHelp: 'seconds', ); } @@ -411,6 +430,7 @@ This command should be run from the root of your Flutter project.'''); '--no-pub', ...options.rest, ], + timeout: options.timeout, ); if (results.any((code) => code != ExitCode.success.code)) { return ExitCode.unavailable.code; diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index 46da8502d..006ec1986 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -1,5 +1,5 @@ import 'dart:async'; -import 'dart:io' show Process, stderr; +import 'dart:io' show stderr; import 'package:args/command_runner.dart'; import 'package:dart_mcp/server.dart'; @@ -226,7 +226,7 @@ Only one value can be selected. 'Maximum seconds to wait for the test run before killing ' 'the Flutter test process. Flutter tests can hang ' 'indefinitely when pumpAndSettle() is called without a ' - 'timeout argument (default is 10 minutes). Defaults to 120.', + 'timeout argument. When omitted, no timeout is applied.', ), }, ), @@ -391,6 +391,12 @@ Only one value can be selected. if (args['check_ignore'] == true) { cliArgs.add('--check-ignore'); } + if (args['timeout_seconds'] != null) { + cliArgs.addAll([ + '--timeout', + (args['timeout_seconds']! as num).toInt().toString(), + ]); + } return cliArgs; } @@ -432,44 +438,8 @@ Only one value can be selected. Future _handleTest(CallToolRequest request) async { final args = request.arguments ?? {}; - final timeoutSeconds = (args['timeout_seconds'] as num?)?.toInt() ?? 120; final cliArgs = _parseTest(args); - try { - return await _runToolCommand( - cliArgs, - toolName: 'test', - ).timeout(Duration(seconds: timeoutSeconds)); - } on TimeoutException { - stderr.writeln( - '[very_good_mcp] Test run timed out after ${timeoutSeconds}s. ' - 'Killing flutter_tester processes.', - ); - // OS-level SIGKILL is the only reliable way to stop a hung - // flutter_tester. package:test timeouts fire internally but the - // Flutter process keeps running regardless. - unawaited( - Process.run( - 'pkill', - ['-KILL', '-f', 'flutter_tester'], - runInShell: true, - ), - ); - return CallToolResult( - content: [ - TextContent( - text: - 'Test run timed out after ${timeoutSeconds}s. ' - 'A Flutter test was hanging — likely an unbounded ' - 'pumpAndSettle() call (default timeout is 10 minutes). ' - 'The flutter_tester process was killed.\n' - 'Fix: replace tester.pumpAndSettle() with ' - 'tester.pump(Duration(milliseconds: 500)) or ' - 'tester.pumpAndSettle(timeout: Duration(seconds: 5)).', - ), - ], - isError: true, - ); - } + return _runToolCommand(cliArgs, toolName: 'test'); } Future _handlePackagesGet(CallToolRequest request) async { diff --git a/site/docs/commands/test.md b/site/docs/commands/test.md index 2f42a9cf0..80a90e897 100644 --- a/site/docs/commands/test.md +++ b/site/docs/commands/test.md @@ -46,6 +46,8 @@ very_good test [arguments] --run-skipped Run skipped tests instead of skipping them. --flavor Build a custom app flavor as defined by platform-specific build setup. Supports the use of product flavors in Android Gradle scripts, and the use of custom Xcode schemes. --fail-fast Stop running tests after the first failure. + --timeout= Maximum seconds to let tests run before killing the process. + Useful when tests hang due to an unbounded pumpAndSettle() call. Run "very_good help" to see global options. ``` diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index dbace6f34..2f094735d 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -50,6 +50,7 @@ const expectedTestUsage = [ ' --report-on= Optional file paths to report coverage information to. This should be paths relative to the current working directory. Can be passed multiple times.\n' ' --run-skipped Run skipped tests instead of skipping them.\n' ' --flavor Build a custom app flavor as defined by platform-specific build setup. Supports the use of product flavors in Android Gradle scripts, and the use of custom Xcode schemes.\n' + ' --timeout= Maximum seconds to let tests run before killing the process. Useful when tests hang due to an unbounded pumpAndSettle() call.\n' '\n' 'Run "very_good help" to see global options.', ]; @@ -74,6 +75,7 @@ abstract class FlutterTestCommand { void Function(String)? stderr, bool? forceAnsi, List? reportOn, + Duration? timeout, }); } @@ -120,6 +122,7 @@ void main() { stderr: any(named: 'stderr'), forceAnsi: any(named: 'forceAnsi'), reportOn: any(named: 'reportOn'), + timeout: any(named: 'timeout'), ), ).thenAnswer((_) async => [0]); when(() => argResults['concurrency']).thenReturn(concurrency); @@ -133,6 +136,7 @@ void main() { when(() => argResults['report-on']).thenReturn([]); when(() => argResults['run-skipped']).thenReturn(false); when(() => argResults['flavor']).thenReturn(null); + when(() => argResults['timeout']).thenReturn(null); when( () => argResults['collect-coverage-from'], ).thenReturn('imports'); diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index d43c605f7..b121ade10 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -332,6 +332,7 @@ void main() { 'platform': 'chrome', 'run_skipped': true, 'check_ignore': true, + 'timeout_seconds': 60, }, ), ), @@ -367,6 +368,8 @@ void main() { 'chrome', '--run-skipped', '--check-ignore', + '--timeout', + '60', ]); }); @@ -421,43 +424,22 @@ void main() { ); }); - test( - 'returns timeout error and kills flutter_tester when test hangs', - () async { - // Simulate a hung test — the future never completes. - final hangCompleter = Completer(); - when( - () => mockCommandRunner.run(any()), - ).thenAnswer((_) => hangCompleter.future); - - final response = await sendRequest( - CallToolRequest.methodName, - _params( - CallToolRequest( - name: 'test', - arguments: {'timeout_seconds': 1}, - ), + test('passes --timeout when timeout_seconds is provided', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'timeout_seconds': 120}, ), - ); - - expect(response['error'], isNull); - final result = CallToolResult.fromMap( - response['result'] as Map, - ); - expect(result.isError, isTrue); - expect( - (result.content.first as TextContent).text, - allOf([ - contains('timed out after 1s'), - contains('pumpAndSettle'), - ]), - ); - - // Resolve the dangling future so it doesn't outlive the test. - hangCompleter.complete(0); - }, - timeout: const Timeout(Duration(seconds: 5)), - ); + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect(capturedArgs, ['test', '--timeout', '120']); + }); }); group('Tool: packages_get', () { From 4a59c71c2db2c233a807ea71cf99308bad7cb3ad Mon Sep 17 00:00:00 2001 From: Elias Reis Date: Mon, 18 May 2026 08:37:15 -0300 Subject: [PATCH 4/4] test(test): add coverage for timeout parameter Co-Authored-By: Claude Sonnet 4.6 --- test/src/cli/test_cli_runner_test.dart | 85 ++++++++++++++++++++++++++ test/src/commands/test/test_test.dart | 16 +++++ 2 files changed, 101 insertions(+) diff --git a/test/src/cli/test_cli_runner_test.dart b/test/src/cli/test_cli_runner_test.dart index 0b92b446a..0a28a8880 100644 --- a/test/src/cli/test_cli_runner_test.dart +++ b/test/src/cli/test_cli_runner_test.dart @@ -1360,6 +1360,91 @@ void main() { }, ); + group('timeout parameter', () { + test('completes with unavailable when timeout fires', () async { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + File(p.join(tempDirectory.path, 'pubspec.yaml')).createSync(); + Directory(p.join(tempDirectory.path, 'test')).createSync(); + + // Stream that never emits ExitTestEvent — simulates a hung test. + final controller = StreamController(); + addTearDown(controller.close); + + final result = await TestCLIRunner.test( + testType: TestRunType.flutter, + cwd: tempDirectory.path, + logger: logger, + stdout: stdoutLogs.add, + stderr: stderrLogs.add, + timeout: const Duration(milliseconds: 1), + overrideTestRunner: testRunner(controller.stream), + ); + + expect(result, equals([ExitCode.unavailable.code])); + expect( + stderrLogs, + contains(contains('timed out after')), + ); + }); + + test('cancels timer on normal exit', () async { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + File(p.join(tempDirectory.path, 'pubspec.yaml')).createSync(); + Directory(p.join(tempDirectory.path, 'test')).createSync(); + + await expectLater( + TestCLIRunner.test( + testType: TestRunType.flutter, + cwd: tempDirectory.path, + logger: logger, + stdout: stdoutLogs.add, + stderr: stderrLogs.add, + // Long enough that normal exit wins. + timeout: const Duration(seconds: 30), + overrideTestRunner: testRunner( + Stream.fromIterable([ + const DoneTestEvent(success: true, time: 0), + const ExitTestEvent(exitCode: 0, time: 0), + ]), + ), + ), + completion(equals([ExitCode.success.code])), + ); + }); + + test('cancels timer on SIGINT', () async { + final streamController = StreamController(); + await ProcessSignalOverrides.runZoned(() async { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + File(p.join(tempDirectory.path, 'pubspec.yaml')).createSync(); + Directory(p.join(tempDirectory.path, 'test')).createSync(); + + // Stream that never resolves — SIGINT should win. + final controller = StreamController(); + addTearDown(controller.close); + + ProcessSignalOverrides.current?.addSIGINT(); + final result = await TestCLIRunner.test( + testType: TestRunType.flutter, + cwd: tempDirectory.path, + logger: logger, + stdout: stdoutLogs.add, + stderr: stderrLogs.add, + timeout: const Duration(seconds: 30), + overrideTestRunner: testRunner(controller.stream), + ); + + expect(result, equals([ExitCode.success.code])); + }, sigintStream: streamController.stream); + }); + }); + group('collectCoverageFrom parameter', () { test('passes through collectCoverageFrom to test runner', () async { final tempDirectory = Directory.systemTemp.createTempSync(); diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 2f094735d..81d6ddf73 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -587,6 +587,22 @@ void main() { ), ).called(1); }); + + test('completes normally --timeout 30', () async { + when(() => argResults['timeout']).thenReturn('30'); + final result = await testCommand.run(); + expect(result, equals(ExitCode.success.code)); + verify( + () => flutterTest( + optimizePerformance: true, + arguments: defaultArguments, + logger: logger, + stdout: logger.write, + stderr: logger.err, + timeout: const Duration(seconds: 30), + ), + ).called(1); + }); }); group('coverage', () {