Skip to content

don't bail early when running in multiple roots #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkgs/dart_mcp_server/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# 0.1.0 (Dart SDK 3.8.0) - WP
# 0.1.1 (Dart SDK 3.10.0) - WIP

* Change tools that accept multiple roots to not return immediately on the first
failure.

# 0.1.0 (Dart SDK 3.9.0)

* Add documentation/homepage/repository links to pub results.
* Handle relative paths under roots without trailing slashes.
Expand Down
2 changes: 1 addition & 1 deletion pkgs/dart_mcp_server/lib/src/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ final class DartMCPServer extends MCPServer
}) : super.fromStreamChannel(
implementation: Implementation(
name: 'dart and flutter tooling',
version: '0.1.0',
version: '0.1.1',
),
instructions:
'This server helps to connect Dart and Flutter developers to '
Expand Down
7 changes: 4 additions & 3 deletions pkgs/dart_mcp_server/lib/src/utils/cli_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ Future<CallToolResult> runCommandInRoots(
}

final outputs = <Content>[];
var isError = false;
for (var rootConfig in rootConfigs) {
final result = await runCommandInRoot(
request,
Expand All @@ -109,10 +110,10 @@ Future<CallToolResult> runCommandInRoots(
defaultPaths: defaultPaths,
sdk: sdk,
);
if (result.isError == true) return result;
isError = isError || result.isError == true;
outputs.addAll(result.content);
}
return CallToolResult(content: outputs);
return CallToolResult(content: outputs, isError: isError);
}

/// Runs [commandForRoot] in a single project root specified in the
Expand Down Expand Up @@ -233,7 +234,7 @@ Future<CallToolResult> runCommandInRoot(
TextContent(
text:
'$commandDescription failed in ${projectRoot.path}:\n'
'$output\n\nErrors\n$errors',
'$output${errors.isEmpty ? '' : '\nErrors:\n$errors'}',
),
],
isError: true,
Expand Down
68 changes: 39 additions & 29 deletions pkgs/dart_mcp_server/test/utils/cli_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -195,39 +195,49 @@ void main() {
});

group('cannot run commands', () {
final processManager = FakeProcessManager();

test('with roots outside of known roots', () async {
for (var invalidRoot in ['file:///bar/', 'file:///foo/../bar/']) {
final result = await runCommandInRoots(
CallToolRequest(
name: 'foo',
arguments: {
ParameterNames.roots: [
{ParameterNames.root: invalidRoot},
],
},
),
commandForRoot: (_, _, _) => 'fake',
commandDescription: '',
processManager: processManager,
knownRoots: [Root(uri: 'file:///foo/')],
fileSystem: fileSystem,
sdk: Sdk(),
);
expect(result.isError, isTrue);
expect(
result.content.single,
isA<TextContent>().having(
(t) => t.text,
'text',
allOf(contains('Invalid root $invalidRoot')),
),
);
}
final processManager = TestProcessManager();
final invalidRoots = ['file:///bar/', 'file:///foo/../bar/'];
final allRoots = ['file:///foo/', ...invalidRoots];
final result = await runCommandInRoots(
CallToolRequest(
name: 'foo',
arguments: {
ParameterNames.roots: [
for (var root in allRoots) {ParameterNames.root: root},
],
},
),
commandForRoot: (_, _, _) => 'testProcess',
commandDescription: 'Test process',
processManager: processManager,
knownRoots: [Root(uri: 'file:///foo/')],
fileSystem: fileSystem,
sdk: Sdk(),
);
expect(result.isError, isTrue);
expect(
result.content,
unorderedEquals([
for (var root in invalidRoots)
isA<TextContent>().having(
(t) => t.text,
'text',
contains('Invalid root $root'),
),
for (var root in allRoots)
if (!invalidRoots.contains(root))
isA<TextContent>().having(
(t) => t.text,
'text',
allOf(contains('Test process'), contains(Uri.parse(root).path)),
),
]),
);
});

test('with paths outside of known roots', () async {
final processManager = FakeProcessManager();
final result = await runCommandInRoots(
CallToolRequest(
name: 'foo',
Expand Down