Be more flexible about roots by comparing canonicalized paths (#148)

Instead of using isWithin and also checking for exact equality, we compare the scheme/authority, and then compare the canonicalized paths.

Author: jakemac53

pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:collection/collection.dart';
 import 'package:dart_mcp/server.dart';
 import 'package:file/file.dart';
 import 'package:path/path.dart' as p;
@@ -159,7 +160,9 @@ Future<CallToolResult> runCommandInRoot(
     );
   }
 
-  final root = _findRoot(rootUriString, knownRoots);
+  final root = knownRoots.firstWhereOrNull(
+    (root) => _isUnderRoot(root, rootUriString),
+  );
   if (root == null) {
     return CallToolResult(
       content: [
@@ -195,11 +198,7 @@ Future<CallToolResult> runCommandInRoot(
   final paths =
       (rootConfig?[ParameterNames.paths] as List?)?.cast<String>() ??
       defaultPaths;
-  final invalidPaths = paths.where((path) {
-    final resolvedPath = rootUri.resolve(path).toString();
-    return rootUriString != resolvedPath &&
-        !p.isWithin(rootUriString, resolvedPath);
-  });
+  final invalidPaths = paths.where((path) => !_isUnderRoot(root, path));
   if (invalidPaths.isNotEmpty) {
     return CallToolResult(
       content: [
@@ -263,17 +262,24 @@ Future<String> defaultCommandForRoot(
     ),
 };
 
-/// Returns whether or not [rootUri] is an allowed root, either exactly matching
-/// or under on of the [knownRoots].
-Root? _findRoot(String rootUri, List<Root> knownRoots) {
-  for (final root in knownRoots) {
-    final knownRootUri = Uri.parse(root.uri);
-    final resolvedRoot = knownRootUri.resolve(rootUri).toString();
-    if (root.uri == resolvedRoot || p.isWithin(root.uri, resolvedRoot)) {
-      return root;
-    }
+/// Returns whether [uri] is under or exactly equal to [root].
+///
+/// Relative uris will always be under [root] unless they escape it with `../`.
+bool _isUnderRoot(Root root, String uri) {
+  final rootUri = Uri.parse(root.uri);
+  final resolvedUri = rootUri.resolve(uri);
+  // We don't care about queries or fragments, but the scheme/authority must
+  // match.
+  if (rootUri.scheme != resolvedUri.scheme ||
+      rootUri.authority != resolvedUri.authority) {
+    return false;
   }
-  return null;
+  // Canonicalizing the paths handles any `../` segments and also deals with
+  // trailing slashes versus no trailing slashes.
+  final canonicalRootPath = p.canonicalize(rootUri.path);
+  final canonicalUriPath = p.canonicalize(resolvedUri.path);
+  return canonicalRootPath == canonicalUriPath ||
+      canonicalUriPath.startsWith(canonicalRootPath);
 }
 
 /// The schema for the `roots` parameter for any tool that accepts it.

pkgs/dart_mcp_server/pubspec.yaml

@@ -15,6 +15,7 @@ executables:
 dependencies:
   args: ^2.7.0
   async: ^2.13.0
+  collection: ^1.19.1
   dart_mcp: ^0.2.1
   dds_service_extensions: ^2.0.1
   devtools_shared: ^11.2.0

pkgs/dart_mcp_server/test/utils/cli_utils_test.dart

@@ -83,6 +83,102 @@ void main() {
         expect(fileSystem.directory('/bar/baz').existsSync(), true);
       },
     );
+
+    test('can run commands with missing trailing slashes for roots', () async {
+      final result = await runCommandInRoots(
+        CallToolRequest(
+          name: 'foo',
+          arguments: {
+            ParameterNames.roots: [
+              {ParameterNames.root: 'file:///bar'},
+            ],
+          },
+        ),
+        commandForRoot: (_, _, _) => 'testCommand',
+        arguments: ['a', 'b'],
+        commandDescription: '',
+        processManager: processManager,
+        knownRoots: [Root(uri: 'file:///bar/')],
+        fileSystem: fileSystem,
+        sdk: Sdk(),
+      );
+      expect(result.isError, isNot(true));
+      expect(processManager.commandsRan, [
+        equalsCommand((
+          command: ['testCommand', 'a', 'b'],
+          workingDirectory: '/bar',
+        )),
+      ]);
+    });
+
+    test('can run commands with extra trailing slashes for roots', () async {
+      final result = await runCommandInRoots(
+        CallToolRequest(
+          name: 'foo',
+          arguments: {
+            ParameterNames.roots: [
+              {ParameterNames.root: 'file:///bar/'},
+            ],
+          },
+        ),
+        commandForRoot: (_, _, _) => 'testCommand',
+        arguments: ['a', 'b'],
+        commandDescription: '',
+        processManager: processManager,
+        knownRoots: [Root(uri: 'file:///bar')],
+        fileSystem: fileSystem,
+        sdk: Sdk(),
+      );
+      expect(result.isError, isNot(true));
+      expect(processManager.commandsRan, [
+        equalsCommand((
+          command: ['testCommand', 'a', 'b'],
+          workingDirectory: '/bar/',
+        )),
+      ]);
+    });
+
+    test('with paths inside of known roots', () async {
+      final paths = ['file:///foo/', 'file:///foo', './', '.'];
+      final result = await runCommandInRoots(
+        CallToolRequest(
+          name: 'foo',
+          arguments: {
+            ParameterNames.roots: [
+              {ParameterNames.root: 'file:///foo', ParameterNames.paths: paths},
+              {
+                ParameterNames.root: 'file:///foo/',
+                ParameterNames.paths: paths,
+              },
+            ],
+          },
+        ),
+        commandForRoot: (_, _, _) => 'fake',
+        commandDescription: '',
+        processManager: processManager,
+        knownRoots: [Root(uri: 'file:///foo/')],
+        fileSystem: fileSystem,
+        sdk: Sdk(),
+      );
+      expect(
+        result.isError,
+        isNot(true),
+        reason: result.content.map((c) => (c as TextContent).text).join('\n'),
+      );
+      expect(
+        processManager.commandsRan,
+        unorderedEquals([
+          equalsCommand((
+            command: ['fake', ...paths],
+            workingDirectory: '/foo/',
+          )),
+          equalsCommand((
+            command: ['fake', ...paths],
+            workingDirectory: '/foo',
+          )),
+        ]),
+      );
+    });
   });
 
   group('cannot run commands', () {