diff --git a/.github/workflows/dart_mcp_server.yaml b/.github/workflows/dart_mcp_server.yaml index b25b8c3a..f0788168 100644 --- a/.github/workflows/dart_mcp_server.yaml +++ b/.github/workflows/dart_mcp_server.yaml @@ -5,17 +5,17 @@ on: # Run CI on all PRs (against any branch) and on pushes to the main branch. pull_request: paths: - - '.github/workflows/dart_mcp_server.yaml' - - 'pkgs/dart_mcp_server/**' - - 'pkgs/dart_mcp/**' + - ".github/workflows/dart_mcp_server.yaml" + - "pkgs/dart_mcp_server/**" + - "pkgs/dart_mcp/**" push: - branches: [ main ] + branches: [main] paths: - - '.github/workflows/dart_mcp_server.yaml' - - 'pkgs/dart_mcp_server/**' - - 'pkgs/dart_mcp/**' + - ".github/workflows/dart_mcp_server.yaml" + - "pkgs/dart_mcp_server/**" + - "pkgs/dart_mcp/**" schedule: - - cron: '0 0 * * 0' # weekly + - cron: "0 0 * * 0" # weekly defaults: run: @@ -64,6 +64,6 @@ jobs: # If this fails, you need to run 'dart tool/update_readme.dart'. - run: dart tool/update_readme.dart - - run: git diff --exit-code README.md + - run: git diff --exit-code README.md || (echo 'README.md needs to be updated. Run "dart tool/update_readme.dart"' && false) - run: dart test diff --git a/pkgs/dart_mcp_server/CHANGELOG.md b/pkgs/dart_mcp_server/CHANGELOG.md index 14ab3549..1cb0b997 100644 --- a/pkgs/dart_mcp_server/CHANGELOG.md +++ b/pkgs/dart_mcp_server/CHANGELOG.md @@ -12,6 +12,7 @@ tends to hide nested text widgets which makes it difficult to find widgets based on their text values. * Add an `--exclude-tool` command line flag to exclude tools by name. +* Add the abillity to limit the output of `analyze_files` to a set of paths. # 0.1.0 (Dart SDK 3.9.0) diff --git a/pkgs/dart_mcp_server/README.md b/pkgs/dart_mcp_server/README.md index bc73d24f..16e85098 100644 --- a/pkgs/dart_mcp_server/README.md +++ b/pkgs/dart_mcp_server/README.md @@ -151,7 +151,7 @@ For more information, see the official VS Code documentation for | `run_tests` | Run tests | Run Dart or Flutter tests with an agent centric UX. ALWAYS use instead of `dart test` or `flutter test` shell commands. | | `create_project` | Create project | Creates a new Dart or Flutter project. | | `pub` | pub | Runs a pub command for the given project roots, like `dart pub get` or `flutter pub add`. | -| `analyze_files` | Analyze projects | Analyzes the entire project for errors. | +| `analyze_files` | Analyze projects | Analyzes specific paths, or the entire project, for errors. | | `resolve_workspace_symbol` | Project search | Look up a symbol or symbols in all workspaces by name. Can be used to validate that a symbol exists or discover small spelling mistakes, since the search is fuzzy. | | `signature_help` | Signature help | Get signature help for an API being used at a given cursor position in a file. | | `hover` | Hover information | Get hover information at a given cursor position in a file. This can include documentation, type information, etc for the text at that position. | diff --git a/pkgs/dart_mcp_server/lib/src/mixins/analyzer.dart b/pkgs/dart_mcp_server/lib/src/mixins/analyzer.dart index 610ae703..b9843e14 100644 --- a/pkgs/dart_mcp_server/lib/src/mixins/analyzer.dart +++ b/pkgs/dart_mcp_server/lib/src/mixins/analyzer.dart @@ -14,7 +14,9 @@ import 'package:meta/meta.dart'; import '../lsp/wire_format.dart'; import '../utils/analytics.dart'; +import '../utils/cli_utils.dart'; import '../utils/constants.dart'; +import '../utils/file_system.dart'; import '../utils/sdk.dart'; /// Mix this in to any MCPServer to add support for analyzing Dart projects. @@ -22,7 +24,7 @@ import '../utils/sdk.dart'; /// The MCPServer must already have the [ToolsSupport] and [LoggingSupport] /// mixins applied. base mixin DartAnalyzerSupport - on ToolsSupport, LoggingSupport, RootsTrackingSupport + on ToolsSupport, LoggingSupport, RootsTrackingSupport, FileSystemSupport implements SdkSupport { /// The LSP server connection for the analysis server. Peer? _lspConnection; @@ -249,8 +251,54 @@ base mixin DartAnalyzerSupport final errorResult = await _ensurePrerequisites(request); if (errorResult != null) return errorResult; + var rootConfigs = (request.arguments?[ParameterNames.roots] as List?) + ?.cast>(); + final allRoots = await roots; + + if (rootConfigs != null && rootConfigs.isEmpty) { + // Have to have at least one root set. + return noRootsSetResponse; + } + + // Default to use the known roots if none were specified. + rootConfigs ??= [ + for (final root in allRoots) {ParameterNames.root: root.uri}, + ]; + + final requestedUris = {}; + for (final rootConfig in rootConfigs) { + final validated = validateRootConfig( + rootConfig, + knownRoots: allRoots, + fileSystem: fileSystem, + ); + + if (validated.errorResult != null) { + return errorResult!; + } + + final rootUri = Uri.parse(validated.root!.uri); + + if (validated.paths != null && validated.paths!.isNotEmpty) { + for (final path in validated.paths!) { + requestedUris.add(rootUri.resolve(path)); + } + } else { + requestedUris.add(rootUri); + } + } + + final entries = diagnostics.entries.where((entry) { + final entryPath = entry.key.toFilePath(); + return requestedUris.any((uri) { + final requestedPath = uri.toFilePath(); + return fileSystem.path.equals(requestedPath, entryPath) || + fileSystem.path.isWithin(requestedPath, entryPath); + }); + }); + final messages = []; - for (var entry in diagnostics.entries) { + for (var entry in entries) { for (var diagnostic in entry.value) { final diagnosticJson = diagnostic.toJson(); diagnosticJson[ParameterNames.uri] = entry.key.toString(); @@ -411,8 +459,10 @@ base mixin DartAnalyzerSupport @visibleForTesting static final analyzeFilesTool = Tool( name: 'analyze_files', - description: 'Analyzes the entire project for errors.', - inputSchema: Schema.object(), + description: 'Analyzes specific paths, or the entire project, for errors.', + inputSchema: Schema.object( + properties: {ParameterNames.roots: rootsSchema(supportsPaths: true)}, + ), annotations: ToolAnnotations(title: 'Analyze projects', readOnlyHint: true), ); diff --git a/pkgs/dart_mcp_server/lib/src/server.dart b/pkgs/dart_mcp_server/lib/src/server.dart index a77f6846..249905a4 100644 --- a/pkgs/dart_mcp_server/lib/src/server.dart +++ b/pkgs/dart_mcp_server/lib/src/server.dart @@ -36,8 +36,8 @@ final class DartMCPServer extends MCPServer ResourcesSupport, RootsTrackingSupport, RootsFallbackSupport, - DartAnalyzerSupport, DashCliSupport, + DartAnalyzerSupport, PubSupport, PubDevSupport, DartToolingDaemonSupport, diff --git a/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart b/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart index 9b595e39..25220e55 100644 --- a/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart +++ b/pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart @@ -252,6 +252,105 @@ Future runCommandInRoot( ); } +/// Validates a root argument given via [rootConfig], ensuring that it falls +/// under one of the [knownRoots], and that all `paths` arguments are also under +/// the given root. +/// +/// Returns a root on success, equal to the given root (but this could be a +/// subdirectory of one of the [knownRoots]), as well as any paths that were +/// validated. +/// +/// If no [ParameterNames.paths] are provided, then the [defaultPaths] will be +/// used, if present. Otherwise no paths are validated or will be returned. +/// +/// On failure, returns a [CallToolResult]. +({Root? root, List? paths, CallToolResult? errorResult}) +validateRootConfig( + Map? rootConfig, { + List? defaultPaths, + required FileSystem fileSystem, + required List knownRoots, +}) { + final rootUriString = rootConfig?[ParameterNames.root] as String?; + if (rootUriString == null) { + // This shouldn't happen based on the schema, but handle defensively. + return ( + root: null, + paths: null, + errorResult: CallToolResult( + content: [ + TextContent(text: 'Invalid root configuration: missing `root` key.'), + ], + isError: true, + )..failureReason ??= CallToolFailureReason.noRootGiven, + ); + } + final rootUri = Uri.parse(rootUriString); + if (rootUri.scheme != 'file') { + return ( + root: null, + paths: null, + errorResult: CallToolResult( + content: [ + TextContent( + text: + 'Only file scheme uris are allowed for roots, but got ' + '$rootUri', + ), + ], + isError: true, + )..failureReason ??= CallToolFailureReason.invalidRootScheme, + ); + } + + final knownRoot = knownRoots.firstWhereOrNull( + (root) => _isUnderRoot(root, rootUriString, fileSystem), + ); + if (knownRoot == null) { + return ( + root: null, + paths: null, + errorResult: CallToolResult( + content: [ + TextContent( + text: + 'Invalid root $rootUriString, must be under one of the ' + 'registered project roots:\n\n${knownRoots.join('\n')}', + ), + ], + isError: true, + )..failureReason ??= CallToolFailureReason.invalidRootPath, + ); + } + final root = Root(uri: rootUriString); + + final paths = + (rootConfig?[ParameterNames.paths] as List?)?.cast() ?? + defaultPaths; + if (paths != null) { + final invalidPaths = paths.where( + (path) => !_isUnderRoot(root, path, fileSystem), + ); + if (invalidPaths.isNotEmpty) { + return ( + root: null, + paths: null, + errorResult: CallToolResult( + content: [ + TextContent( + text: + 'Paths are not allowed to escape their project root:\n' + '${invalidPaths.join('\n')}', + ), + ], + isError: true, + )..failureReason ??= CallToolFailureReason.invalidPath, + ); + } + } + return (root: root, paths: paths, errorResult: null); +} + /// Returns 'dart' or 'flutter' based on the pubspec contents. /// /// Throws an [ArgumentError] if there is no pubspec. diff --git a/pkgs/dart_mcp_server/test/test_harness.dart b/pkgs/dart_mcp_server/test/test_harness.dart index c30364b8..37354939 100644 --- a/pkgs/dart_mcp_server/test/test_harness.dart +++ b/pkgs/dart_mcp_server/test/test_harness.dart @@ -173,7 +173,7 @@ class TestHarness { final result = await mcpServerConnection.callTool(request); expect( result.isError, - expectError ? true : isNot(true), + expectError ? true : isNot(isTrue), reason: result.content.join('\n'), ); return result; @@ -185,11 +185,12 @@ class TestHarness { Future callToolWithRetry( CallToolRequest request, { int maxTries = 5, + bool expectError = false, }) async { var tryCount = 0; while (true) { try { - return await callTool(request); + return await callTool(request, expectError: expectError); } catch (_) { if (tryCount++ >= maxTries) rethrow; } diff --git a/pkgs/dart_mcp_server/test/tools/analyzer_test.dart b/pkgs/dart_mcp_server/test/tools/analyzer_test.dart index f6d7e656..14397c1c 100644 --- a/pkgs/dart_mcp_server/test/tools/analyzer_test.dart +++ b/pkgs/dart_mcp_server/test/tools/analyzer_test.dart @@ -72,6 +72,266 @@ void main() { ); }); + test('can analyze a project with multiple errors (no paths)', () async { + final example = d.dir('example', [ + d.file('main.dart', 'void main() => 1 + "2";'), + d.file('other.dart', 'void other() => foo;'), + ]); + await example.create(); + final exampleRoot = testHarness.rootForPath(example.io.path); + testHarness.mcpClient.addRoot(exampleRoot); + + await pumpEventQueue(); + + final request = CallToolRequest(name: analyzeTool.name); + final result = await testHarness.callToolWithRetry(request); + expect(result.isError, isNot(true)); + expect(result.content, hasLength(2)); + expect( + result.content, + containsAll([ + isA().having( + (t) => t.text, + 'text', + contains("Undefined name 'foo'"), + ), + isA().having( + (t) => t.text, + 'text', + contains( + "The argument type 'String' can't be assigned to the parameter " + "type 'num'.", + ), + ), + ]), + ); + }); + + test('can analyze a specific file', () async { + final example = d.dir('example', [ + d.file('main.dart', 'void main() => 1 + "2";'), + d.file('other.dart', 'void other() => foo;'), + ]); + await example.create(); + final exampleRoot = testHarness.rootForPath(example.io.path); + testHarness.mcpClient.addRoot(exampleRoot); + + await pumpEventQueue(); + + final request = CallToolRequest( + name: analyzeTool.name, + arguments: { + ParameterNames.roots: [ + { + ParameterNames.root: exampleRoot.uri, + ParameterNames.paths: ['main.dart'], + }, + ], + }, + ); + final result = await testHarness.callToolWithRetry(request); + expect(result.isError, isNot(true)); + expect(result.content, hasLength(1)); + expect( + result.content.single, + isA().having( + (t) => t.text, + 'text', + contains( + "The argument type 'String' can't be assigned to the parameter " + "type 'num'.", + ), + ), + ); + }); + + test('can analyze a specific directory', () async { + final example = d.dir('example', [ + d.file('main.dart', 'void main() => 1 + "2";'), + d.dir('sub', [d.file('other.dart', 'void other() => foo;')]), + ]); + await example.create(); + final exampleRoot = testHarness.rootForPath(example.io.path); + testHarness.mcpClient.addRoot(exampleRoot); + + await pumpEventQueue(); + + final request = CallToolRequest( + name: analyzeTool.name, + arguments: { + ParameterNames.roots: [ + { + ParameterNames.root: exampleRoot.uri, + ParameterNames.paths: ['sub'], + }, + ], + }, + ); + final result = await testHarness.callToolWithRetry(request); + expect(result.isError, isNot(true)); + expect(result.content, hasLength(1)); + expect( + result.content.single, + isA().having( + (t) => t.text, + 'text', + contains("Undefined name 'foo'"), + ), + ); + }); + + test('handles a non-existent path', () async { + final example = d.dir('example', [ + d.file('main.dart', 'void main() => 1 + "2";'), + ]); + await example.create(); + final exampleRoot = testHarness.rootForPath(example.io.path); + testHarness.mcpClient.addRoot(exampleRoot); + + await pumpEventQueue(); + + final request = CallToolRequest( + name: analyzeTool.name, + arguments: { + ParameterNames.roots: [ + { + ParameterNames.root: exampleRoot.uri, + ParameterNames.paths: ['not_a_real_file.dart'], + }, + ], + }, + ); + final result = await testHarness.callToolWithRetry(request); + expect(result.isError, isNot(true)); + expect( + result.content.single, + isA().having((t) => t.text, 'text', 'No errors'), + ); + }); + + test('handles an empty paths list for a root', () async { + final example = d.dir('example', [ + d.file('main.dart', 'void main() => 1 + "2";'), + ]); + await example.create(); + final exampleRoot = testHarness.rootForPath(example.io.path); + testHarness.mcpClient.addRoot(exampleRoot); + + await pumpEventQueue(); + + final request = CallToolRequest( + name: analyzeTool.name, + arguments: { + ParameterNames.roots: [ + { + ParameterNames.root: exampleRoot.uri, + ParameterNames.paths: [], // Empty paths + }, + ], + }, + ); + final result = await testHarness.callToolWithRetry(request); + expect(result.isError, isNot(true)); + expect(result.content, hasLength(1)); + expect( + result.content.single, + isA().having( + (t) => t.text, + 'text', + contains( + "The argument type 'String' can't be assigned to the parameter " + "type 'num'.", + ), + ), + ); + }); + + test('handles an empty roots list', () async { + // We still need a root registered with the server so that the + // prerequisites check passes. + final example = d.dir('example', [ + d.file('main.dart', 'void main() => 1;'), + ]); + await example.create(); + final exampleRoot = testHarness.rootForPath(example.io.path); + testHarness.mcpClient.addRoot(exampleRoot); + await pumpEventQueue(); + + final request = CallToolRequest( + name: analyzeTool.name, + arguments: {ParameterNames.roots: []}, + ); + final result = await testHarness.callToolWithRetry( + request, + expectError: true, + ); + expect(result.isError, isTrue); + expect( + result.content.single, + isA().having( + (t) => t.text, + 'text', + 'No roots set. At least one root must be set in order to use this ' + 'tool.', + ), + ); + }); + + test('can analyze files in multiple roots', () async { + final projectA = d.dir('project_a', [ + d.file('main.dart', 'void main() => 1 + "a";'), + ]); + await projectA.create(); + final projectARoot = testHarness.rootForPath(projectA.io.path); + testHarness.mcpClient.addRoot(projectARoot); + + final projectB = d.dir('project_b', [ + d.file('other.dart', 'void other() => foo;'), + ]); + await projectB.create(); + final projectBRoot = testHarness.rootForPath(projectB.io.path); + testHarness.mcpClient.addRoot(projectBRoot); + + await pumpEventQueue(); + + final request = CallToolRequest( + name: analyzeTool.name, + arguments: { + ParameterNames.roots: [ + { + ParameterNames.root: projectARoot.uri, + ParameterNames.paths: ['main.dart'], + }, + { + ParameterNames.root: projectBRoot.uri, + ParameterNames.paths: ['other.dart'], + }, + ], + }, + ); + final result = await testHarness.callToolWithRetry(request); + expect(result.isError, isNot(true)); + expect(result.content, hasLength(2)); + expect( + result.content, + containsAll([ + isA().having( + (t) => t.text, + 'text', + contains( + "The argument type 'String' can't be assigned to the " + "parameter type 'num'.", + ), + ), + isA().having( + (t) => t.text, + 'text', + contains("Undefined name 'foo'"), + ), + ]), + ); + }); + test('can look up symbols in a workspace', () async { final example = d.dir('lib', [ d.file('awesome_class.dart', 'class MyAwesomeClass {}'), diff --git a/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart b/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart index b38f092b..58bd106b 100644 --- a/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart +++ b/pkgs/dart_mcp_server/test/utils/cli_utils_test.dart @@ -279,6 +279,169 @@ void main() { ); }); }); + + group('validateRootConfig', () { + test('succeeds with a valid root and no paths', () { + final result = validateRootConfig( + {ParameterNames.root: 'file:///project/'}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNull); + expect(result.root, isNotNull); + expect(result.root!.uri, 'file:///project/'); + expect(result.paths, isNull); + }); + + test('succeeds with a root that is a subdirectory of a known root', () { + final result = validateRootConfig( + {ParameterNames.root: 'file:///project/sub'}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNull); + expect(result.root, isNotNull); + expect(result.root!.uri, 'file:///project/sub'); + expect(result.paths, isNull); + }); + + test('succeeds with valid paths', () { + final paths = ['./lib', 'lib/src/stuff.dart']; + final result = validateRootConfig( + {ParameterNames.root: 'file:///project/', ParameterNames.paths: paths}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNull); + expect(result.root, isNotNull); + expect(result.root!.uri, 'file:///project/'); + expect(result.paths, paths); + }); + + test('uses default paths when none are provided', () { + final defaultPaths = ['./lib']; + final result = validateRootConfig( + {ParameterNames.root: 'file:///project/'}, + defaultPaths: defaultPaths, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNull); + expect(result.root, isNotNull); + expect(result.root!.uri, 'file:///project/'); + expect(result.paths, defaultPaths); + }); + + test('uses provided paths over default paths', () { + final paths = ['./lib']; + final defaultPaths = ['./test']; + final result = validateRootConfig( + {ParameterNames.root: 'file:///project/', ParameterNames.paths: paths}, + defaultPaths: defaultPaths, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNull); + expect(result.root, isNotNull); + expect(result.root!.uri, 'file:///project/'); + expect(result.paths, paths); + }); + + test('fails if root config is null', () { + final result = validateRootConfig( + null, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNotNull); + expect(result.root, isNull); + expect(result.paths, isNull); + expect(result.errorResult!.isError, isTrue); + expect( + (result.errorResult!.content.single as TextContent).text, + contains('missing `root` key'), + ); + }); + + test('fails if root config is missing root key', () { + final result = validateRootConfig( + {}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNotNull); + expect(result.root, isNull); + expect(result.paths, isNull); + expect(result.errorResult!.isError, isTrue); + expect( + (result.errorResult!.content.single as TextContent).text, + contains('missing `root` key'), + ); + }); + + test('fails if root is outside of known roots', () { + final result = validateRootConfig( + {ParameterNames.root: 'file:///other/'}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNotNull); + expect(result.root, isNull); + expect(result.paths, isNull); + expect(result.errorResult!.isError, isTrue); + expect( + (result.errorResult!.content.single as TextContent).text, + contains('Invalid root file:///other/'), + ); + }); + + test('fails if root has a non-file scheme', () { + final result = validateRootConfig( + {ParameterNames.root: 'http:///project/'}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNotNull); + expect(result.root, isNull); + expect(result.paths, isNull); + expect(result.errorResult!.isError, isTrue); + expect( + (result.errorResult!.content.single as TextContent).text, + contains('Only file scheme uris are allowed'), + ); + }); + + test('fails with paths that escape the root', () { + final paths = ['../outside.dart', '/other/place.dart']; + final result = validateRootConfig( + {ParameterNames.root: 'file:///project/', ParameterNames.paths: paths}, + knownRoots: [Root(uri: 'file:///project/')], + fileSystem: fileSystem, + ); + + expect(result.errorResult, isNotNull); + expect(result.root, isNull); + expect(result.paths, isNull); + expect(result.errorResult!.isError, isTrue); + expect( + (result.errorResult!.content.single as TextContent).text, + allOf( + contains('Paths are not allowed to escape their project root'), + contains('../outside.dart'), + contains('/other/place.dart'), + ), + ); + }); + }); } class FakeProcessManager extends Fake implements ProcessManager {} diff --git a/pkgs/dart_mcp_server/test_fixtures/counter_app/android/local.properties b/pkgs/dart_mcp_server/test_fixtures/counter_app/android/local.properties index be68df01..2115949c 100644 --- a/pkgs/dart_mcp_server/test_fixtures/counter_app/android/local.properties +++ b/pkgs/dart_mcp_server/test_fixtures/counter_app/android/local.properties @@ -1 +1,2 @@ -flutter.sdk=/usr/local/google/home/jakemac/flutter \ No newline at end of file +flutter.sdk=/usr/local/google/home/gspencer/code/flutter +sdk.dir=/usr/local/google/home/gspencer/Android/Sdk \ No newline at end of file