From 26d6345879aa4c11b5272f19d286f6ce19a85af9 Mon Sep 17 00:00:00 2001 From: Hadi Hassan Date: Wed, 5 Aug 2026 05:12:20 +0300 Subject: [PATCH 1/4] feat(mcp): allow the test tool to target specific paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `test` tool exposed no way to run a subset of a suite, so every call ran everything. The CLI already supports positional test targets — the test command forwards `argResults.rest` to the runner — but the MCP tool had no argument that reached them, and `directory` is deliberately applied as the working directory rather than as a target. Adds an optional `paths` array that is appended after every option, so the args land in `rest`. Behaviour matches the CLI, including the existing rule that targeting specific files disables test optimization. --- lib/src/mcp/mcp_server.dart | 15 ++++++++ test/src/mcp/mcp_server_test.dart | 63 +++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index 7d698ac5c..78d6fbc56 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -174,6 +174,14 @@ If is omitted, then core will be selected. 'Target directory path (defaults to current directory). ' 'Can be absolute or relative path to project root.', ), + 'paths': ListSchema( + description: + 'Test files or directories to run, relative to the project ' + "root (e.g. ['test/src/foo_test.dart', 'test/widgets']). " + 'When omitted, the whole suite runs. Note that targeting ' + 'specific paths disables the test optimization step.', + items: StringSchema(), + ), 'dart': BooleanSchema( description: '''Whether to run Dart tests. If not specified, Flutter tests will be run if a Flutter project is detected.''', @@ -452,6 +460,13 @@ Only one value can be selected. ]); } + // Positional test targets go last, after every option, so that they are + // parsed as `rest` rather than as a value for the preceding option. + final paths = args['paths'] as List?; + if (paths != null) { + cliArgs.addAll(paths.cast()); + } + return cliArgs; } diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index 6c36ce9a3..43af7d632 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -502,6 +502,69 @@ void main() { as List; expect(capturedArgs, equals(['test', '--timeout', '120'])); }); + + test('passes paths as positional test targets', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'paths': ['test/src/foo_test.dart', 'test/widgets'], + }, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect( + capturedArgs, + equals(['test', 'test/src/foo_test.dart', 'test/widgets']), + ); + }); + + test('passes paths after options so they are parsed as rest', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'dart': true, + 'concurrency': '8', + 'paths': ['test/src/foo_test.dart'], + }, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect( + capturedArgs, + equals(['dart', 'test', '-j', '8', 'test/src/foo_test.dart']), + ); + }); + + test('adds no positional targets when paths is empty', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'paths': []}, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect(capturedArgs, equals(['test'])); + }); }); group('Tool: packages_get', () { From b6a39b7c633556a2b844076bfffb41f06617f5cc Mon Sep 17 00:00:00 2001 From: Hadi Hassan Date: Mon, 24 Aug 2026 22:03:28 +0300 Subject: [PATCH 2/4] fix(mcp): scope test `paths` to one package and guard `recursive` `paths` was described as relative to "the project root", which reads as though a package path belongs in it. That lands on the #1599/#1600 failure: from a monorepo root with no pubspec.yaml, targeting packages/a/test/a_test.dart exits 66. The schema now says paths are targets inside the package selected by `directory`, and that a package path belongs in `directory` instead. `recursive` and `paths` were also incoherent together. TestCLIRunner spawns one run per package with that package as the working directory and forwards the same positional targets to each, so a relative path resolves in one package and fails to load in the rest. From a shell the cause is visible; through the tool it is not, so the pair is now rejected up front with a message naming the fix. Targets are emitted behind a `--` terminator so a path beginning with `-` is no longer a UsageException. The parser strips the `--` back out of `rest`, so nothing downstream changes. Also documents `paths` in doc/mcp.md, whose example enumerates every argument. --- doc/mcp.md | 5 ++- lib/src/mcp/mcp_server.dart | 53 +++++++++++++++++++----- test/src/mcp/mcp_server_test.dart | 68 +++++++++++++++++++++++++++++-- 3 files changed, 112 insertions(+), 14 deletions(-) diff --git a/doc/mcp.md b/doc/mcp.md index aa2a8ebb4..8d989d622 100644 --- a/doc/mcp.md +++ b/doc/mcp.md @@ -123,9 +123,10 @@ Runs tests in a Dart or Flutter project. "tool": "test", "arguments": { "directory": "./my_app", + "paths": ["test/src/foo_test.dart", "test/widgets"], "dart": false, "coverage": true, - "recursive": true, + "recursive": false, "optimization": true, "concurrency": "4", "min_coverage": "100", @@ -145,6 +146,8 @@ 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, no timeout is applied. +`directory` selects the package to test; `paths` selects test files or directories *within* that package. To run another package's tests, point `directory` at it — putting a package path in `paths` runs the wrong thing, or nothing at all. Passing `paths` disables the test optimization step, and it cannot be combined with `recursive`, since each package in a recursive run gets the same relative path and only one of them can resolve it. + ### `packages_get` Installs or updates Dart/Flutter package dependencies. diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index c8a654ff3..ee9f0040c 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -170,15 +170,20 @@ If is omitted, then core will be selected. properties: { 'directory': StringSchema( description: - 'Target directory path (defaults to current directory). ' - 'Can be absolute or relative path to project root.', + 'The package to test (defaults to current directory). ' + 'Can be absolute or relative path to project root. ' + 'A package path belongs here, not in "paths".', ), 'paths': ListSchema( description: - 'Test files or directories to run, relative to the project ' - "root (e.g. ['test/src/foo_test.dart', 'test/widgets']). " - 'When omitted, the whole suite runs. Note that targeting ' - 'specific paths disables the test optimization step.', + 'Test files or directories to run, relative to the package ' + 'selected by "directory" (e.g. ' + "['test/src/foo_test.dart', 'test/widgets']). These are " + 'targets inside that one package: to test a different ' + 'package, set "directory" to it rather than putting its ' + 'path here. When omitted, the whole suite runs. Cannot be ' + 'combined with "recursive". Note that targeting specific ' + 'paths disables the test optimization step.', items: StringSchema(), ), 'dart': BooleanSchema( @@ -188,7 +193,11 @@ If is omitted, then core will be selected. description: 'Whether to collect coverage information.', ), 'recursive': BooleanSchema( - description: 'Run tests recursively for all nested packages.', + description: + 'Run tests recursively for all nested packages. ' + 'Cannot be combined with "paths", because each package runs ' + 'in its own working directory and a path is only meaningful ' + 'within one of them.', ), 'optimization': BooleanSchema( description: ''' @@ -457,10 +466,13 @@ Only one value can be selected. } // Positional test targets go last, after every option, so that they are - // parsed as `rest` rather than as a value for the preceding option. + // parsed as `rest` rather than as a value for the preceding option. The + // `--` terminator keeps a target that begins with `-` from being read as + // an option; the parser strips it back out of `rest`, so the test command + // sees the paths and nothing else. final paths = args['paths'] as List?; - if (paths != null) { - cliArgs.addAll(paths.cast()); + if (paths != null && paths.isNotEmpty) { + cliArgs.addAll(['--', ...paths.cast()]); } return cliArgs; @@ -500,6 +512,27 @@ Only one value can be selected. Future _handleTest(CallToolRequest request) async { final args = request.arguments ?? {}; + + // A recursive run spawns one test process per package, each with that + // package as its working directory, and forwards the same positional + // targets to all of them. A relative path can therefore only resolve in + // one package and fails to load in the rest. Reject the pair up front + // rather than let it surface as a load error from every other package. + final paths = args['paths'] as List?; + if (args['recursive'] == true && (paths?.isNotEmpty ?? false)) { + return CallToolResult( + content: [ + TextContent( + text: + '"recursive" cannot be combined with "paths". Paths are ' + 'resolved within a single package, so set "directory" to the ' + 'package holding the tests and drop "recursive".', + ), + ], + isError: true, + ); + } + final cliArgs = _parseTest(args); return _runToolCommand( cliArgs, diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index 43af7d632..a9aa729ea 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -503,7 +503,7 @@ void main() { expect(capturedArgs, equals(['test', '--timeout', '120'])); }); - test('passes paths as positional test targets', () async { + test('passes paths as positional test targets behind `--`', () async { await sendRequest( CallToolRequest.methodName, _params( @@ -521,7 +521,7 @@ void main() { as List; expect( capturedArgs, - equals(['test', 'test/src/foo_test.dart', 'test/widgets']), + equals(['test', '--', 'test/src/foo_test.dart', 'test/widgets']), ); }); @@ -545,10 +545,29 @@ void main() { as List; expect( capturedArgs, - equals(['dart', 'test', '-j', '8', 'test/src/foo_test.dart']), + equals(['dart', 'test', '-j', '8', '--', 'test/src/foo_test.dart']), ); }); + test('passes a path beginning with `-` through unparsed', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'paths': ['-weird_test.dart'], + }, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect(capturedArgs, equals(['test', '--', '-weird_test.dart'])); + }); + test('adds no positional targets when paths is empty', () async { await sendRequest( CallToolRequest.methodName, @@ -565,6 +584,49 @@ void main() { as List; expect(capturedArgs, equals(['test'])); }); + + test('returns error when paths is combined with recursive', () async { + final response = await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: { + 'recursive': true, + 'paths': ['test/src/foo_test.dart'], + }, + ), + ), + ); + + expect(response['error'], isNull); + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + expect(result.isError, isTrue); + expect( + (result.content.first as TextContent).text, + contains('"recursive" cannot be combined with "paths"'), + ); + verifyNever(() => mockCommandRunner.run(any())); + }); + + test('allows recursive when paths is empty', () async { + await sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'recursive': true, 'paths': []}, + ), + ), + ); + + final capturedArgs = + verify(() => mockCommandRunner.run(captureAny())).captured.first + as List; + expect(capturedArgs, equals(['test', '-r'])); + }); }); group('Tool: packages_get', () { From 4f81f30cb85a3864154d94f03788e97070cef7c4 Mon Sep 17 00:00:00 2001 From: Hadi Hassan Date: Mon, 24 Aug 2026 22:03:28 +0300 Subject: [PATCH 3/4] docs(cli): correct isTargettingTestFiles option terminator note --- lib/src/cli/test_cli_runner.dart | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 3d6c42c06..cf471621f 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -62,14 +62,19 @@ class MinCoverageNotMet implements Exception { class TestCLIRunner { /// Determines whether the user is targetting test files or not. /// - /// The user can only target test files by using the `--` option terminator. - /// The additional options after the `--` are passed to the test runner which - /// allows the user to target specific test files or directories. + /// [rest] holds the positional arguments left over after option parsing. + /// Trailing options are allowed, so bare targets work + /// (`very_good test test/foo_test.dart`); the `--` option terminator is only + /// needed for a target that begins with `-`, which would otherwise be read + /// as an option. Either way the parser strips the `--` out of [rest], so + /// what arrives here is the targets alone. /// /// The heuristics used to determine if the user is not targetting test files /// are: /// * No [rest] arguments are passed. - /// * All [rest] arguments are options (i.e. they do not start with `-`). + /// * Every [rest] argument is an option (i.e. it starts with `-`), which + /// only happens when the options were passed through a `--` for the + /// underlying test runner. /// /// See also: /// * [What does -- mean in Shell?](https://www.cyberciti.biz/faq/what-does-double-dash-mean-in-ssh-command/) From 6a48be092666925076945ac598bc84bb5eb51edc Mon Sep 17 00:00:00 2001 From: Hadi Hassan Date: Tue, 25 Aug 2026 21:42:39 +0300 Subject: [PATCH 4/4] fix(test): reject positional targets combined with --recursive A recursive run executes once per package with that package as the working directory and forwards the same positional targets to each, so a relative path resolves in one package and fails to load in the rest. Both `very_good test` and `very_good dart test` now exit with a usage code and an explanation. The check sits beside the existing pubspec.yaml guard in each command, where `argResults.rest` is unambiguous. It cannot live in `TestCLIRunner.test`, which receives targets already merged into `arguments` alongside option values whose values do not start with `-`. This replaces the equivalent guard in the MCP test handler, which goes back to building argv and nothing else. The tool schema still documents the exclusion, since that is what an agent reads before making a call. Refs #1704 --- .../dart/commands/dart_test_command.dart | 9 ++++ lib/src/commands/test/test.dart | 9 ++++ lib/src/mcp/mcp_server.dart | 21 --------- .../dart/commands/dart_test_test.dart | 37 ++++++++++++++++ test/src/commands/test/test_test.dart | 37 ++++++++++++++++ test/src/mcp/mcp_server_test.dart | 43 ------------------- 6 files changed, 92 insertions(+), 64 deletions(-) diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index b5dabf9ae..ad42190ea 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -360,6 +360,15 @@ class DartTestCommand extends Command { final pubspec = File(path.join(targetPath, 'pubspec.yaml')); final recursive = _argResults['recursive'] as bool; + if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { + _logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + if (!recursive && !pubspec.existsSync()) { _logger.err(''' Could not find a pubspec.yaml in $targetPath. diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index ca565a4f9..d08fadf23 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -438,6 +438,15 @@ class TestCommand extends Command { final pubspec = File(path.join(targetPath, 'pubspec.yaml')); final recursive = _argResults['recursive'] as bool; + if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { + _logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + if (!recursive && !pubspec.existsSync()) { _logger.err(''' Could not find a pubspec.yaml in $targetPath. diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index ee9f0040c..79d1ebc05 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -512,27 +512,6 @@ Only one value can be selected. Future _handleTest(CallToolRequest request) async { final args = request.arguments ?? {}; - - // A recursive run spawns one test process per package, each with that - // package as its working directory, and forwards the same positional - // targets to all of them. A relative path can therefore only resolve in - // one package and fails to load in the rest. Reject the pair up front - // rather than let it surface as a load error from every other package. - final paths = args['paths'] as List?; - if (args['recursive'] == true && (paths?.isNotEmpty ?? false)) { - return CallToolResult( - content: [ - TextContent( - text: - '"recursive" cannot be combined with "paths". Paths are ' - 'resolved within a single package, so set "directory" to the ' - 'package holding the tests and drop "recursive".', - ), - ], - isError: true, - ); - } - final cliArgs = _parseTest(args); return _runToolCommand( cliArgs, diff --git a/test/src/commands/dart/commands/dart_test_test.dart b/test/src/commands/dart/commands/dart_test_test.dart index 589183c29..47e3be231 100644 --- a/test/src/commands/dart/commands/dart_test_test.dart +++ b/test/src/commands/dart/commands/dart_test_test.dart @@ -760,6 +760,43 @@ void main() { }, ); + test( + 'exits with usage code when targeting test files with --recursive', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['test/my_test.dart']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.usage.code)); + verify( + () => logger.err(any(that: contains('--recursive'))), + ).called(1); + }, + ); + + test( + 'allows --recursive when rest arguements are all options', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['--track-wdiget-creation']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.success.code)); + verify( + () => dartTest( + recursive: true, + optimizePerformance: true, + arguments: [...defaultArguments, '--track-wdiget-creation'], + logger: logger, + stdout: logger.write, + stderr: logger.err, + ), + ).called(1); + }, + ); + test( 'enables optimizePerformance when rest arguement is an option', () async { diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 4dd2070ac..252e9b453 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -539,6 +539,43 @@ void main() { }, ); + test( + 'exits with usage code when targeting test files with --recursive', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['test/my_test.dart']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.usage.code)); + verify( + () => logger.err(any(that: contains('--recursive'))), + ).called(1); + }, + ); + + test( + 'allows --recursive when rest arguements are all options', + () async { + when(() => argResults['recursive']).thenReturn(true); + when(() => argResults.rest).thenReturn(['--track-wdiget-creation']); + + final result = await testCommand.run(); + + expect(result, equals(ExitCode.success.code)); + verify( + () => flutterTest( + recursive: true, + optimizePerformance: true, + arguments: [...defaultArguments, '--track-wdiget-creation'], + logger: logger, + stdout: logger.write, + stderr: logger.err, + ), + ).called(1); + }, + ); + test( 'enables optimizePerformance when rest arguement is an option', () async { diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index a9aa729ea..0ce7b5ddb 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -584,49 +584,6 @@ void main() { as List; expect(capturedArgs, equals(['test'])); }); - - test('returns error when paths is combined with recursive', () async { - final response = await sendRequest( - CallToolRequest.methodName, - _params( - CallToolRequest( - name: 'test', - arguments: { - 'recursive': true, - 'paths': ['test/src/foo_test.dart'], - }, - ), - ), - ); - - expect(response['error'], isNull); - final result = CallToolResult.fromMap( - response['result'] as Map, - ); - expect(result.isError, isTrue); - expect( - (result.content.first as TextContent).text, - contains('"recursive" cannot be combined with "paths"'), - ); - verifyNever(() => mockCommandRunner.run(any())); - }); - - test('allows recursive when paths is empty', () async { - await sendRequest( - CallToolRequest.methodName, - _params( - CallToolRequest( - name: 'test', - arguments: {'recursive': true, 'paths': []}, - ), - ), - ); - - final capturedArgs = - verify(() => mockCommandRunner.run(captureAny())).captured.first - as List; - expect(capturedArgs, equals(['test', '-r'])); - }); }); group('Tool: packages_get', () {