Skip to content

fix: apply MCP directory argument as the working directory #1600

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.

Sign up for GitHub

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

Open
jmadren wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: apply MCP directory argument as the working directory #1600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 37 additions & 14 deletions lib/src/mcp/mcp_server.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import 'dart:async';
import 'dart:io' show stderr;
import 'dart:io' show Directory, stderr;

import 'package:args/command_runner.dart';
import 'package:dart_mcp/server.dart';
Expand Down Expand Up @@ -322,11 +322,10 @@ Only one value can be selected.
}

List<String> _parseTest(Map<String, Object?> args) {
// NOTE: 'directory' is intentionally not added here. It is applied as the
// working directory in [_runToolCommand], not as a positional test target.
final cliArgs = <String>[if (args['dart'] == true) 'dart', 'test'];

if (args['directory'] != null) {
cliArgs.add(args['directory']! as String);
}
if (args['coverage'] == true) {
cliArgs.add('--coverage');
}
Expand Down Expand Up @@ -389,11 +388,10 @@ Only one value can be selected.
}

List<String> _parsePackagesGet(Map<String, Object?> args) {
// NOTE: 'directory' is applied as the working directory in
// [_runToolCommand], not as a positional argument.
final cliArgs = <String>['packages', 'get'];

if (args['directory'] != null) {
cliArgs.add(args['directory']! as String);
}
if (args['recursive'] == true) {
cliArgs.add('--recursive');
}
Expand All @@ -408,12 +406,10 @@ Only one value can be selected.
}

List<String> _parsePackagesCheck(Map<String, Object?> args) {
// NOTE: 'directory' is applied as the working directory in
// [_runToolCommand], not as a positional argument.
final cliArgs = <String>['packages', 'check', 'licenses'];

if (args['directory'] != null) {
cliArgs.add(args['directory']! as String);
}

return cliArgs;
}

Expand All @@ -426,13 +422,21 @@ Only one value can be selected.
Future<CallToolResult> _handleTest(CallToolRequest request) async {
final args = request.arguments ?? {};
final cliArgs = _parseTest(args);
return _runToolCommand(cliArgs, toolName: 'test');
return _runToolCommand(
cliArgs,
toolName: 'test',
workingDirectory: args['directory'] as String?,
);
}

Future<CallToolResult> _handlePackagesGet(CallToolRequest request) async {
final args = request.arguments ?? {};
final cliArgs = _parsePackagesGet(args);
return _runToolCommand(cliArgs, toolName: 'packages get');
return _runToolCommand(
cliArgs,
toolName: 'packages get',
workingDirectory: args['directory'] as String?,
);
}

Future<CallToolResult> _handlePackagesCheck(CallToolRequest request) async {
Expand All @@ -456,18 +460,33 @@ Only one value can be selected.
}

final cliArgs = _parsePackagesCheck(args);
return _runToolCommand(cliArgs, toolName: 'packages check licenses');
return _runToolCommand(
cliArgs,
toolName: 'packages check licenses',
workingDirectory: args['directory'] as String?,
);
}

/// Runs a CLI command and returns a [CallToolResult] with descriptive
/// error messages including the command that was run and the exit code.
Future<CallToolResult> _runToolCommand(
List<String> args, {
required String toolName,
String? workingDirectory,
}) async {
final commandString = 'very_good ${args.join(' ')}';

// The underlying CLI commands resolve their target package from
// `Directory.current` (and child processes inherit the process cwd), so a
// requested [workingDirectory] is applied by switching the current
// directory for the duration of the run and restoring it afterwards.
// Relative paths are resolved against the server's current directory.
final previousDirectory = Directory.current;

try {
if (workingDirectory != null) {
Directory.current = workingDirectory;
}
final exitCode = await _commandRunner.run(args);

if (exitCode == ExitCode.success.code) {
Expand Down Expand Up @@ -505,6 +524,10 @@ Only one value can be selected.
content: [TextContent(text: message)],
isError: true,
);
} finally {
if (workingDirectory != null) {
Directory.current = previousDirectory;
}
}
}
}
107 changes: 95 additions & 12 deletions test/src/mcp/mcp_server_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:async';
import 'dart:convert';
import 'dart:io' as io;

import 'package:args/command_runner.dart';
import 'package:dart_mcp/server.dart';
Expand Down Expand Up @@ -370,35 +371,44 @@ void main() {
]);
});

test('passes directory as positional argument', () async {
test('does not pass directory as a positional argument', () async {
final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_');
addTearDown(() => tempDir.deleteSync(recursive: true));

await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(name: 'test', arguments: {'directory': 'my_dir'}),
CallToolRequest(
name: 'test',
arguments: {'directory': tempDir.path},
),
),
);

final capturedArgs =
verify(() => mockCommandRunner.run(captureAny())).captured.first
as List<String>;
expect(capturedArgs, ['test', 'my_dir']);
expect(capturedArgs, ['test']);
});

test('passes directory as positional argument with dart flag', () async {
test('does not pass directory positionally with dart flag', () async {
final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_');
addTearDown(() => tempDir.deleteSync(recursive: true));

await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(
name: 'test',
arguments: {'directory': 'my_dir', 'dart': true},
arguments: {'directory': tempDir.path, 'dart': true},
),
),
);

final capturedArgs =
verify(() => mockCommandRunner.run(captureAny())).captured.first
as List<String>;
expect(capturedArgs, ['dart', 'test', 'my_dir']);
expect(capturedArgs, ['dart', 'test']);
});

test('handles command failure', () async {
Expand Down Expand Up @@ -436,13 +446,16 @@ void main() {
});

test('handles all arguments (with split "ignore")', () async {
final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_');
addTearDown(() => tempDir.deleteSync(recursive: true));

await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(
name: 'packages_get',
arguments: {
'directory': 'my_dir',
'directory': tempDir.path,
'recursive': true,
'ignore': 'pkg1, pkg2',
},
Expand All @@ -456,7 +469,6 @@ void main() {
expect(capturedArgs, [
'packages',
'get',
'my_dir',
'--recursive',
'--ignore',
'pkg1',
Expand All @@ -468,37 +480,43 @@ void main() {

group('Tool: packages_check_licenses', () {
test('handles basic case (licenses=true)', () async {
final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_');
addTearDown(() => tempDir.deleteSync(recursive: true));

await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(
name: 'packages_check_licenses',
arguments: {'licenses': true, 'directory': 'my_dir'},
arguments: {'licenses': true, 'directory': tempDir.path},
),
),
);

final capturedArgs =
verify(() => mockCommandRunner.run(captureAny())).captured.first
as List<String>;
expect(capturedArgs, ['packages', 'check', 'licenses', 'my_dir']);
expect(capturedArgs, ['packages', 'check', 'licenses']);
});

test('defaults to licenses=true if not provided', () async {
final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_');
addTearDown(() => tempDir.deleteSync(recursive: true));

await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(
name: 'packages_check_licenses',
arguments: {'directory': 'my_dir'},
arguments: {'directory': tempDir.path},
),
),
);

final capturedArgs =
verify(() => mockCommandRunner.run(captureAny())).captured.first
as List<String>;
expect(capturedArgs, ['packages', 'check', 'licenses', 'my_dir']);
expect(capturedArgs, ['packages', 'check', 'licenses']);
});

test('returns error if licenses=false', () async {
Expand Down Expand Up @@ -577,5 +595,70 @@ void main() {
expect(text, contains('Command: very_good'));
});
});

group('directory (working directory)', () {
late io.Directory tempDir;
late String originalCwd;

setUp(() {
originalCwd = io.Directory.current.path;
tempDir = io.Directory.systemTemp.createTempSync('vgmcp_cwd_');
addTearDown(() {
// Always restore the cwd so a failure cannot leak into other tests.
io.Directory.current = originalCwd;
if (tempDir.existsSync()) tempDir.deleteSync(recursive: true);
});
});

for (final toolName in const [
'test',
'packages_get',
'packages_check_licenses',
]) {
test('"$toolName" runs in the requested directory', () async {
String? cwdDuringRun;
when(() => mockCommandRunner.run(any())).thenAnswer((_) async {
cwdDuringRun = io.Directory.current.resolveSymbolicLinksSync();
return ExitCode.success.code;
});

final response = await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(
name: toolName,
arguments: {'directory': tempDir.path},
),
),
);

final result = CallToolResult.fromMap(
response['result'] as Map<String, Object?>,
);
expect(result.isError, isFalse);
expect(cwdDuringRun, tempDir.resolveSymbolicLinksSync());
// The working directory is restored after the command completes.
expect(io.Directory.current.path, originalCwd);
});
}

test('returns an error when the directory does not exist', () async {
final missing = '${tempDir.path}/does-not-exist';

final response = await sendRequest(
CallToolRequest.methodName,
_params(
CallToolRequest(name: 'test', arguments: {'directory': missing}),
),
);

final result = CallToolResult.fromMap(
response['result'] as Map<String, Object?>,
);
expect(result.isError, isTrue);
// The cwd is left unchanged when switching to it fails.
expect(io.Directory.current.path, originalCwd);
});
});
});
}
Loading