diff --git a/.gitignore b/.gitignore index d62aa72dc..4ea56d1fb 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ doc/api/ coverage/ .test_optimizer.dart !bricks/test_optimizer/__brick__/test/.test_optimizer.dart +*.vm.json # Android studio and IntelliJ .idea diff --git a/lib/src/mcp/lock.dart b/lib/src/mcp/lock.dart new file mode 100644 index 000000000..f56672626 --- /dev/null +++ b/lib/src/mcp/lock.dart @@ -0,0 +1,25 @@ +import 'dart:async'; + +/// {@template lock} +/// A first-in, first-out asynchronous mutex. +/// +/// [run] executes its body only after every previously enqueued body has +/// completed, so at most one body runs at a time. Unlike a one-deep lock +/// (which only awaits a single pending run), this serializes correctly for any +/// number of concurrent callers — each call chains onto the tail of the queue. +/// {@endtemplate} +class Lock { + /// Tail of the queue: the future that completes when the most recently + /// enqueued run finishes. + Future _tail = Future.value(); + + /// Runs [body] once all previously enqueued runs have completed, and returns + /// its result. A failing [body] does not break the queue — subsequent runs + /// still proceed in order. + Future run(Future Function() body) { + final previous = _tail; + final completer = Completer(); + _tail = completer.future; + return previous.then((_) => body()).whenComplete(completer.complete); + } +} diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index 27ebad9ff..c29a62087 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -1,13 +1,35 @@ import 'dart:async'; -import 'dart:io' show Directory, stderr; +import 'dart:convert'; +import 'dart:io' + show Directory, IOOverrides, IOSink, Stdout, StdoutException, stderr; import 'package:args/command_runner.dart'; import 'package:dart_mcp/server.dart'; import 'package:mason/mason.dart' hide packageVersion; +import 'package:meta/meta.dart'; import 'package:stream_channel/stream_channel.dart'; import 'package:very_good_cli/src/command_runner.dart'; +import 'package:very_good_cli/src/mcp/lock.dart'; import 'package:very_good_cli/src/version.dart'; +/// {@template command_runner_builder} +/// Builds a [VeryGoodCommandRunner] bound to the provided [logger]. +/// +/// A builder (rather than a prebuilt runner) is required so the runner — and +/// therefore its mason [Logger] — can be constructed *inside* the +/// [IOOverrides] zone that redirects `stdout`/`stderr`. mason captures +/// `IOOverrides.current` once, at [Logger] construction, so a runner built +/// outside the zone would still write to the real process stdout and corrupt +/// the MCP JSON-RPC stream. +/// {@endtemplate} +typedef CommandRunnerBuilder = + VeryGoodCommandRunner Function({required Logger logger}); + +/// The default [CommandRunnerBuilder] used when none is injected. +@visibleForTesting +VeryGoodCommandRunner defaultCommandRunnerBuilder({required Logger logger}) => + VeryGoodCommandRunner(logger: logger); + /// {@template very_good_mcp_server} /// MCP Server for Very Good CLI. /// @@ -19,10 +41,9 @@ final class VeryGoodMCPServer extends MCPServer with ToolsSupport { /// {@macro very_good_mcp_server} VeryGoodMCPServer({ required StreamChannel channel, - Logger? logger, - VeryGoodCommandRunner? commandRunner, - }) : _commandRunner = - commandRunner ?? VeryGoodCommandRunner(logger: logger ?? Logger()), + CommandRunnerBuilder? commandRunnerBuilder, + }) : _commandRunnerBuilder = + commandRunnerBuilder ?? defaultCommandRunnerBuilder, super.fromStreamChannel( channel, implementation: Implementation( @@ -34,7 +55,16 @@ final class VeryGoodMCPServer extends MCPServer with ToolsSupport { 'for creating and managing Dart/Flutter projects.', ); - final VeryGoodCommandRunner _commandRunner; + /// {@macro command_runner_builder} + final CommandRunnerBuilder _commandRunnerBuilder; + + /// Serializes tool runs. + /// + /// [_runToolCommand] switches the process-global `Directory.current`, so tool + /// runs must not overlap — the MCP transport can dispatch tool calls + /// concurrently (pipelined requests), and overlapping runs would corrupt each + /// other's working directory. The [Lock] keeps at most one run in flight. + final _lock = Lock(); @override FutureOr initialize(InitializeRequest request) async { @@ -438,7 +468,7 @@ Only one value can be selected. return _runToolCommand( cliArgs, toolName: 'test', - workingDirectory: args['directory'] as String?, + directory: args['directory'] as String?, ); } @@ -448,7 +478,7 @@ Only one value can be selected. return _runToolCommand( cliArgs, toolName: 'packages get', - workingDirectory: args['directory'] as String?, + directory: args['directory'] as String?, ); } @@ -476,71 +506,211 @@ Only one value can be selected. return _runToolCommand( cliArgs, toolName: 'packages check licenses', - workingDirectory: args['directory'] as String?, + directory: 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. + /// Runs a CLI command in-process and returns a [CallToolResult]. + /// + /// The command is executed inside an [IOOverrides] zone that: + /// + /// * redirects `stdout`/`stderr` into a buffer, so the command's mason + /// [Logger] output (test results, compile errors, ...) is captured and + /// returned in the result instead of leaking onto the real process stdout, + /// which is shared with the MCP JSON-RPC stream (the stdio transport + /// requires the server MUST NOT write non-MCP content to stdout); and + /// * sets the current directory to [directory] when provided, so in-process + /// commands that resolve their target from `Directory.current` run in the + /// right package (`directory` is the working directory, not a positional + /// argument). + /// + /// The [Logger] is constructed *inside* the zone on purpose: mason captures + /// `IOOverrides.current` at [Logger] construction time, so building it + /// outside the zone would defeat the redirect. Future _runToolCommand( List 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; + String? directory, + }) { + return _lock.run(() async { + final commandString = 'very_good ${args.join(' ')}'; + final output = StringBuffer(); + + Future runCaptured(Future Function(Logger logger) body) { + final sink = CapturingStdout(output); + // Redirect stdout/stderr so the in-process command's mason Logger + // output is captured into [output] instead of leaking onto the real + // stdout that carries the MCP JSON-RPC stream. The Logger is built + // inside the zone so mason (which pins IOOverrides.current at + // construction) honors it. + return IOOverrides.runZoned( + () => body(Logger()), + stdout: () => sink, + stderr: () => sink, + ); + } - try { - if (workingDirectory != null) { - Directory.current = workingDirectory; + // Appends the captured command output (the real diagnostics) to a + // message, on every result path so partial output emitted before a throw + // is not lost. The buffer is populated whether the run returns or throws. + String withCapturedOutput(String message) { + final captured = sanitizeCommandOutput(output.toString()).trim(); + if (captured.isEmpty) return message; + return '$message\n\nOutput:\n$captured'; } - final exitCode = await _commandRunner.run(args); - if (exitCode == ExitCode.success.code) { + // Builds a failure result from [reason] (the human-readable cause). The + // message is logged once to the real stderr (the stdio transport forbids + // only non-JSON on stdout, so stderr is free for diagnostics) and also + // surfaced — with any captured output — in the tool result, so the same + // text never has to be written twice. [commandString] is appended to keep + // the failure reproducible. + CallToolResult errorResult(String reason, {StackTrace? stackTrace}) { + final message = '"$toolName" $reason\nCommand: $commandString'; + stderr.writeln('[very_good_mcp] ${message.replaceAll('\n', ' ')}'); + if (stackTrace != null) { + stderr.writeln('[very_good_mcp] Stack trace: $stackTrace'); + } return CallToolResult( - content: [TextContent(text: '"$toolName" completed successfully.')], - isError: false, + content: [TextContent(text: withCapturedOutput(message))], + isError: true, ); } - final message = - '"$toolName" failed with exit code $exitCode.\n' - 'Command: $commandString'; - stderr.writeln('[very_good_mcp] $message'); - return CallToolResult( - content: [TextContent(text: message)], - isError: true, - ); - } on UsageException catch (e) { - final message = - '"$toolName" usage error: ${e.message}\n' - 'Command: $commandString'; - stderr.writeln('[very_good_mcp] $message'); - return CallToolResult( - content: [TextContent(text: message)], - isError: true, - ); - } on Exception catch (e, stackTrace) { - final message = - '"$toolName" threw an exception: $e\n' - 'Command: $commandString'; - stderr - ..writeln('[very_good_mcp] $message') - ..writeln('[very_good_mcp] Stack trace: $stackTrace'); - return CallToolResult( - content: [TextContent(text: message)], - isError: true, - ); - } finally { - if (workingDirectory != null) { - Directory.current = previousDirectory; + // Apply [directory] as the real working directory for the duration of + // the run, restoring it afterwards. The underlying commands resolve their + // target package from the process current directory and spawn + // subprocesses with it, so it must be the real cwd (not just an + // IOOverrides override, which subprocesses do not honor). + final previousDirectory = Directory.current; + + try { + if (directory != null) Directory.current = directory; + final exitCode = await runCaptured( + (logger) => _commandRunnerBuilder(logger: logger).run(args), + ); + + if (exitCode == ExitCode.success.code) { + final captured = sanitizeCommandOutput(output.toString()).trim(); + return CallToolResult( + content: [ + TextContent(text: '"$toolName" completed successfully.'), + if (captured.isNotEmpty) TextContent(text: captured), + ], + isError: false, + ); + } + + return errorResult('failed with exit code $exitCode.'); + } on UsageException catch (e) { + return errorResult('usage error: ${e.message}'); + } on Exception catch (e, stackTrace) { + return errorResult('threw an exception: $e', stackTrace: stackTrace); + } finally { + if (directory != null) Directory.current = previousDirectory; } + }); + } +} + +/// A [Stdout] that captures everything written to it into a [StringBuffer] +/// instead of the real process stdout/stderr. +/// +/// Used to redirect a command's in-process [Logger] output (which mason routes +/// through `stdout`/`stderr`, including progress spinners) away from the real +/// stdout shared with the MCP JSON-RPC stream. It reports no terminal so mason +/// emits plain, animation-free lines. +@visibleForTesting +class CapturingStdout implements Stdout { + /// Creates a [CapturingStdout] that appends all writes to [_buffer]. + CapturingStdout(this._buffer); + + final StringBuffer _buffer; + + @override + Encoding encoding = utf8; + + @override + String lineTerminator = '\n'; + + @override + void write(Object? object) => _buffer.write(object ?? 'null'); + + @override + void writeln([Object? object = '']) => _buffer.writeln(object ?? ''); + + @override + void writeAll(Iterable objects, [String separator = '']) => + _buffer.writeAll(objects, separator); + + @override + void writeCharCode(int charCode) => _buffer.writeCharCode(charCode); + + @override + void add(List data) { + try { + _buffer.write(encoding.decode(data)); + } on FormatException { + _buffer.write(String.fromCharCodes(data)); } } + + @override + void addError(Object error, [StackTrace? stackTrace]) {} + + @override + Future addStream(Stream> stream) => stream.forEach(add); + + @override + Future flush() async {} + + @override + Future close() async {} + + @override + Future get done => Future.value(); + + @override + bool get hasTerminal => false; + + @override + bool get supportsAnsiEscapes => false; + + @override + int get terminalColumns => + throw const StdoutException('No terminal attached'); + + @override + int get terminalLines => throw const StdoutException('No terminal attached'); + + @override + IOSink get nonBlocking => this; +} + +/// Matches a CSI ANSI escape sequence (colors, cursor moves, line erases). +final _ansiEscape = RegExp(r'\x1B\[[0-?]*[ -/]*[@-~]'); + +/// Renders raw captured command output as plain text for a tool result. +/// +/// In-process commands (and the test subprocesses they reformat) animate +/// progress with ANSI escape sequences and carriage returns: a spinner redraws +/// a single line in place with `\r` and erases it with `\x1B[2K`. A terminal +/// resolves those to clean lines, but the raw bytes surfaced to an MCP client +/// collapse into one run-on line. This reproduces the terminal's settled view: +/// +/// * strips ANSI escape sequences; +/// * normalizes `\r\n` to `\n`; and +/// * collapses carriage-return redraws to the text after the last `\r` on each +/// line (the final state the user would see), trimming trailing padding. +@visibleForTesting +String sanitizeCommandOutput(String raw) { + return raw + .replaceAll(_ansiEscape, '') + .replaceAll('\r\n', '\n') + .split('\n') + .map( + (line) => + (line.contains('\r') ? line.split('\r').last : line).trimRight(), + ) + .join('\n'); } diff --git a/test/src/mcp/lock_test.dart b/test/src/mcp/lock_test.dart new file mode 100644 index 000000000..a28b0aec8 --- /dev/null +++ b/test/src/mcp/lock_test.dart @@ -0,0 +1,67 @@ +import 'dart:async'; + +import 'package:test/test.dart'; +import 'package:very_good_cli/src/mcp/lock.dart'; + +void main() { + group('Lock', () { + test('returns the body result', () async { + final lock = Lock(); + expect(await lock.run(() async => 42), 42); + }); + + test('serializes any number of concurrent runs', () async { + final lock = Lock(); + var active = 0; + var maxActive = 0; + + Future body() async { + active++; + if (active > maxActive) maxActive = active; + await Future.delayed(const Duration(milliseconds: 5)); + active--; + } + + await Future.wait([ + lock.run(body), + lock.run(body), + lock.run(body), + lock.run(body), + ]); + + expect(maxActive, 1); + }); + + test('runs bodies in FIFO order', () async { + final lock = Lock(); + final order = []; + + final futures = [ + for (var i = 0; i < 4; i++) + lock.run(() async { + await Future.delayed(const Duration(milliseconds: 5)); + order.add(i); + }), + ]; + await Future.wait(futures); + + expect(order, [0, 1, 2, 3]); + }); + + test('a failing body does not break the queue', () async { + final lock = Lock(); + final order = []; + + final failing = lock.run(() async { + order.add('failing'); + throw Exception('boom'); + }); + + final next = lock.run(() async => order.add('next')); + + await expectLater(failing, throwsException); + await next; + expect(order, ['failing', 'next']); + }); + }); +} diff --git a/test/src/mcp/mcp_server_test.dart b/test/src/mcp/mcp_server_test.dart index c1abfdd1e..fce187fb4 100644 --- a/test/src/mcp/mcp_server_test.dart +++ b/test/src/mcp/mcp_server_test.dart @@ -1,6 +1,6 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:io' as io; +import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:dart_mcp/server.dart'; @@ -11,8 +11,6 @@ import 'package:test/test.dart'; import 'package:very_good_cli/src/command_runner.dart'; import 'package:very_good_cli/src/mcp/mcp_server.dart'; -class _MockLogger extends Mock implements Logger {} - class _MockVeryGoodCommandRunner extends Mock implements VeryGoodCommandRunner {} @@ -35,12 +33,17 @@ Map _params(dynamic params) => params as Map; void main() { group('VeryGoodMCPServer', () { - late Logger mockLogger; late VeryGoodCommandRunner mockCommandRunner; + // The mason Logger the server constructs inside the IOOverrides zone and + // passes to the builder. Captured here so tests can drive it directly. + late Logger injectedLogger; late StreamChannelController channelController; // ignore: unused_local_variable Server is not used directly, but needed to keep the channel open late VeryGoodMCPServer server; late Stream> serverResponses; + // A real directory used wherever the `directory` argument is applied as the + // working directory (the server switches the real cwd, which must exist). + late Directory tempDir; setUpAll(() { _idCounter = 1; @@ -71,13 +74,16 @@ void main() { } setUp(() async { - mockLogger = _MockLogger(); mockCommandRunner = _MockVeryGoodCommandRunner(); + tempDir = Directory.systemTemp.createTempSync('vgmcp_test_'); + addTearDown(() => tempDir.deleteSync(recursive: true)); channelController = StreamChannelController(); server = VeryGoodMCPServer( channel: channelController.foreign, - logger: mockLogger, - commandRunner: mockCommandRunner, + commandRunnerBuilder: ({required logger}) { + injectedLogger = logger; + return mockCommandRunner; + }, ); serverResponses = channelController.local.stream @@ -98,10 +104,6 @@ void main() { when( () => mockCommandRunner.run(any()), ).thenAnswer((_) async => ExitCode.success.code); - when(() => mockLogger.info(any())).thenAnswer((_) {}); - when(() => mockLogger.err(any())).thenAnswer((_) {}); - when(() => mockLogger.detail(any())).thenAnswer((_) {}); - when(() => mockLogger.success(any())).thenAnswer((_) {}); // This is the handshake that // MUST happen before any other requests, to fix the timeout. @@ -375,9 +377,6 @@ void main() { }); 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( @@ -394,10 +393,7 @@ void main() { expect(capturedArgs, ['test']); }); - test('does not pass directory positionally with dart flag', () async { - final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_'); - addTearDown(() => tempDir.deleteSync(recursive: true)); - + test('does not pass directory as a positional with dart flag', () async { await sendRequest( CallToolRequest.methodName, _params( @@ -466,7 +462,7 @@ void main() { }); test('handles all arguments (with split "ignore")', () async { - final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_'); + final tempDir = Directory.systemTemp.createTempSync('vgmcp_'); addTearDown(() => tempDir.deleteSync(recursive: true)); await sendRequest( @@ -500,7 +496,7 @@ void main() { group('Tool: packages_check_licenses', () { test('handles basic case (licenses=true)', () async { - final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_'); + final tempDir = Directory.systemTemp.createTempSync('vgmcp_'); addTearDown(() => tempDir.deleteSync(recursive: true)); await sendRequest( @@ -520,7 +516,7 @@ void main() { }); test('defaults to licenses=true if not provided', () async { - final tempDir = io.Directory.systemTemp.createTempSync('vgmcp_'); + final tempDir = Directory.systemTemp.createTempSync('vgmcp_'); addTearDown(() => tempDir.deleteSync(recursive: true)); await sendRequest( @@ -617,15 +613,15 @@ void main() { }); group('directory (working directory)', () { - late io.Directory tempDir; + late Directory tempDir; late String originalCwd; setUp(() { - originalCwd = io.Directory.current.path; - tempDir = io.Directory.systemTemp.createTempSync('vgmcp_cwd_'); + originalCwd = Directory.current.path; + tempDir = Directory.systemTemp.createTempSync('vgmcp_cwd_'); addTearDown(() { // Always restore the cwd so a failure cannot leak into other tests. - io.Directory.current = originalCwd; + Directory.current = originalCwd; if (tempDir.existsSync()) tempDir.deleteSync(recursive: true); }); }); @@ -638,7 +634,7 @@ void main() { test('"$toolName" runs in the requested directory', () async { String? cwdDuringRun; when(() => mockCommandRunner.run(any())).thenAnswer((_) async { - cwdDuringRun = io.Directory.current.resolveSymbolicLinksSync(); + cwdDuringRun = Directory.current.resolveSymbolicLinksSync(); return ExitCode.success.code; }); @@ -658,12 +654,188 @@ void main() { expect(result.isError, isFalse); expect(cwdDuringRun, tempDir.resolveSymbolicLinksSync()); // The working directory is restored after the command completes. - expect(io.Directory.current.path, originalCwd); + expect(Directory.current.path, originalCwd); }); } + }); + + group('captured command output', () { + test('includes captured output in a failure result', () async { + when(() => mockCommandRunner.run(any())).thenAnswer((_) async { + stdout.write('compile error: boom'); + stderr.write('stderr detail'); + return ExitCode.unavailable.code; + }); + + final response = await sendRequest( + CallToolRequest.methodName, + _params(CallToolRequest(name: 'test', arguments: {})), + ); + + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + expect(result.isError, isTrue); + final text = (result.content.first as TextContent).text; + expect(text, contains('"test" failed with exit code 69')); + expect(text, contains('compile error: boom')); + expect(text, contains('stderr detail')); + }); - test('returns an error when the directory does not exist', () async { - final missing = '${tempDir.path}/does-not-exist'; + test('includes captured output in a success result', () async { + when(() => mockCommandRunner.run(any())).thenAnswer((_) async { + stdout.write('All tests passed!'); + return ExitCode.success.code; + }); + + final response = await sendRequest( + CallToolRequest.methodName, + _params(CallToolRequest(name: 'test', arguments: {})), + ); + + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + expect(result.isError, isFalse); + expect(result.content, hasLength(2)); + expect( + (result.content[0] as TextContent).text, + '"test" completed successfully.', + ); + expect((result.content[1] as TextContent).text, 'All tests passed!'); + }); + + test('routes the injected in-zone logger through the capture', () async { + // The load-bearing invariant: a Logger constructed INSIDE the zone + // writes through the capture override. Driving the injected logger + // (not the dart:io globals) is what would fail if the Logger were + // built outside the zone. + when(() => mockCommandRunner.run(any())).thenAnswer((_) async { + injectedLogger + ..info('stdout via logger') + ..err('stderr via logger'); + return ExitCode.unavailable.code; + }); + + final response = await sendRequest( + CallToolRequest.methodName, + _params(CallToolRequest(name: 'test', arguments: {})), + ); + + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + final text = (result.content.first as TextContent).text; + expect(text, contains('stdout via logger')); + expect(text, contains('stderr via logger')); + }); + + test('includes captured output when the run throws', () async { + when(() => mockCommandRunner.run(any())).thenAnswer((_) async { + stdout.write('partial output before crash'); + throw Exception('boom'); + }); + + final response = await sendRequest( + CallToolRequest.methodName, + _params(CallToolRequest(name: 'test', arguments: {})), + ); + + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + expect(result.isError, isTrue); + final text = (result.content.first as TextContent).text; + expect(text, contains('"test" threw an exception')); + expect(text, contains('partial output before crash')); + }); + + test('omits the output block when nothing was captured', () async { + when( + () => mockCommandRunner.run(any()), + ).thenAnswer((_) async => ExitCode.success.code); + + final response = await sendRequest( + CallToolRequest.methodName, + _params(CallToolRequest(name: 'test', arguments: {})), + ); + + final result = CallToolResult.fromMap( + response['result'] as Map, + ); + expect(result.content, hasLength(1)); + expect( + (result.content.first as TextContent).text, + '"test" completed successfully.', + ); + }); + }); + + group('working directory', () { + test( + 'serializes overlapping runs so each keeps its own directory', + () async { + final dirA = Directory.systemTemp.createTempSync('vgmcp_a_'); + final dirB = Directory.systemTemp.createTempSync('vgmcp_b_'); + addTearDown(() { + dirA.deleteSync(recursive: true); + dirB.deleteSync(recursive: true); + }); + + // For each run record (cwd at start, cwd after an async gap). With + // serialization the cwd is stable within a run; without it, a sibling + // run's Directory.current mutation would clobber it across the gap. + final pairs = >[]; + when(() => mockCommandRunner.run(any())).thenAnswer((_) async { + final before = Directory.current.path; + await Future.delayed(const Duration(milliseconds: 20)); + final after = Directory.current.path; + pairs.add([before, after]); + return ExitCode.success.code; + }); + + await Future.wait([ + sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'directory': dirA.path}, + ), + ), + ), + sendRequest( + CallToolRequest.methodName, + _params( + CallToolRequest( + name: 'test', + arguments: {'directory': dirB.path}, + ), + ), + ), + ]); + + expect(pairs, hasLength(2)); + for (final pair in pairs) { + expect( + pair[1], + pair[0], + reason: 'cwd must stay stable for the duration of a single run', + ); + } + expect( + pairs + .map((p) => Directory(p[0]).resolveSymbolicLinksSync()) + .toSet(), + {dirA.resolveSymbolicLinksSync(), dirB.resolveSymbolicLinksSync()}, + ); + }, + ); + + test('errors and restores cwd for a non-existent directory', () async { + final before = Directory.current.path; + final missing = + '${Directory.systemTemp.path}/vgmcp_missing_dir_should_not_exist'; final response = await sendRequest( CallToolRequest.methodName, @@ -676,9 +848,110 @@ void main() { response['result'] as Map, ); expect(result.isError, isTrue); - // The cwd is left unchanged when switching to it fails. - expect(io.Directory.current.path, originalCwd); + // cwd switch failed before the run, so it was never executed... + verifyNever(() => mockCommandRunner.run(any())); + // ...and the process directory is unchanged. + expect(Directory.current.path, before); }); }); }); + + group('defaultCommandRunnerBuilder', () { + test('builds a VeryGoodCommandRunner', () { + expect( + defaultCommandRunnerBuilder(logger: Logger()), + isA(), + ); + }); + }); + + group('CapturingStdout', () { + late StringBuffer buffer; + late CapturingStdout capturing; + + setUp(() { + buffer = StringBuffer(); + capturing = CapturingStdout(buffer); + }); + + test('write/writeln/writeAll/writeCharCode append to the buffer', () { + capturing + ..write('a') + ..write(null) + ..writeln('b') + ..writeln() + ..writeAll(['c', 'd'], '-') + ..writeCharCode(0x65); // 'e' + expect(buffer.toString(), 'anullb\n\nc-de'); + }); + + test('add decodes bytes with the current encoding', () { + capturing.add(utf8.encode('héllo')); + expect(buffer.toString(), 'héllo'); + }); + + test('add falls back to char codes on malformed bytes', () { + capturing.add([0xff, 0xfe]); + expect(buffer.toString(), String.fromCharCodes([0xff, 0xfe])); + }); + + test('addStream forwards all chunks', () async { + await capturing.addStream( + Stream.fromIterable([utf8.encode('x'), utf8.encode('y')]), + ); + expect(buffer.toString(), 'xy'); + }); + + test('reports no terminal and tolerates sink lifecycle calls', () async { + expect(capturing.hasTerminal, isFalse); + expect(capturing.supportsAnsiEscapes, isFalse); + expect(capturing.nonBlocking, same(capturing)); + expect(capturing.encoding, utf8); + expect(capturing.lineTerminator, '\n'); + expect(() => capturing.terminalColumns, throwsA(isA())); + expect(() => capturing.terminalLines, throwsA(isA())); + capturing.addError('ignored'); + await capturing.flush(); + await capturing.close(); + await capturing.done; + }); + }); + + group('sanitizeCommandOutput', () { + test('strips ANSI escape sequences', () { + expect( + sanitizeCommandOutput('\x1B[31mred\x1B[0m and \x1B[1mbold\x1B[0m'), + 'red and bold', + ); + }); + + test('normalizes CRLF to LF', () { + expect(sanitizeCommandOutput('a\r\nb\r\nc'), 'a\nb\nc'); + }); + + test('collapses carriage-return redraws to the settled text', () { + // A spinner redrawing one line in place: erase + rewrite per tick. + expect( + sanitizeCommandOutput( + '\r\x1B[2K00:01 +1\r\x1B[2K00:02 +5\r\x1B[2KAll tests passed!', + ), + 'All tests passed!', + ); + }); + + test('preserves genuine newlines while collapsing redraws per line', () { + expect( + sanitizeCommandOutput('compiling...\rdone\nAll tests passed!'), + 'done\nAll tests passed!', + ); + }); + + test('trims trailing padding left by line erases', () { + expect(sanitizeCommandOutput('result '), 'result'); + }); + + test('leaves plain multi-line output untouched', () { + expect(sanitizeCommandOutput('line 1\nline 2'), 'line 1\nline 2'); + }); + }); }