Skip to content

Fix normalizeCommand() splitting absolute paths containing spaces #553

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

Merged
pushpak1300 merged 2 commits into from
Feb 9, 2026
Merged

Fix normalizeCommand() splitting absolute paths containing spaces #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
753cc9e
Fix normalizeCommand() splitting absolute paths containing spaces
digitall-it Feb 9, 2026
93fec4f
Refactoring
pushpak1300 Feb 9, 2026
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
10 changes: 10 additions & 0 deletions src/Install/Agents/Agent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,21 @@ protected function installFileMcp(string $key, string $command, array $args = []
/**
* Normalize command by splitting space-separated commands into command + args.
*
* Absolute paths (starting with / on Unix or a drive letter on Windows)
* are never split, as they may contain spaces (e.g. macOS "Application Support").
*
* @param array<int, string> $args
* @return array{command: string, args: array<int, string>}
*/
protected function normalizeCommand(string $command, array $args = []): array
{
if (str_starts_with($command, '/') || preg_match('#^[a-zA-Z]:[/\\\\]#', $command)) {
return [
'command' => $command,
'args' => $args,
];
}

$parts = str($command)->explode(' ');

return [
Expand Down
159 changes: 159 additions & 0 deletions tests/Unit/Install/Agents/AgentTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,17 @@ public function mcpConfigPath(): string
]);
});

test('splits herd php into command and arguments', function (): void {
$environment = new TestAgent($this->strategyFactory);

$result = $environment->testNormalizeCommand('herd php', ['artisan', 'boost:mcp']);

expect($result)->toBe([
'command' => 'herd',
'args' => ['php', 'artisan', 'boost:mcp'],
]);
});

test('splits docker exec commands into parts', function (): void {
$environment = new TestAgent($this->strategyFactory);

Expand Down Expand Up @@ -439,6 +450,32 @@ public function mcpConfigPath(): string
expect($result)->toBe(true);
});

test('shell installation handles herd php commands', function (): void {
$environment = Mockery::mock(TestAgent::class)->makePartial();
$environment->shouldAllowMockingProtectedMethods();

$environment->shouldReceive('shellMcpCommand')
->andReturn('install {key} {command} {args}');

$environment->shouldReceive('mcpInstallationStrategy')
->andReturn(McpInstallationStrategy::SHELL);

$mockResult = Mockery::mock();
$mockResult->shouldReceive('successful')->andReturn(true);
$mockResult->shouldReceive('errorOutput')->andReturn('');

Process::shouldReceive('run')
->once()
->with(Mockery::on(fn ($command): bool => str_contains((string) $command, 'install test-key herd') &&
str_contains((string) $command, '"php"') &&
str_contains((string) $command, '"artisan"')))
->andReturn($mockResult);

$result = $environment->installMcp('test-key', 'herd php', ['artisan', 'boost:mcp']);

expect($result)->toBe(true);
});

test('file installation handles valet php commands', function (): void {
$environment = Mockery::mock(TestSupportsMcp::class)->makePartial();
$environment->shouldAllowMockingProtectedMethods();
Expand Down Expand Up @@ -477,6 +514,44 @@ public function mcpConfigPath(): string
]);
});

test('file installation handles herd php commands', function (): void {
$environment = Mockery::mock(TestSupportsMcp::class)->makePartial();
$environment->shouldAllowMockingProtectedMethods();

$capturedContent = '';

$environment->shouldReceive('mcpInstallationStrategy')
->andReturn(McpInstallationStrategy::FILE);

File::shouldReceive('ensureDirectoryExists')
->once()
->with('.test');

File::shouldReceive('exists')
->once()
->with('.test/mcp.json')
->andReturn(false);

File::shouldReceive('put')
->once()
->with(Mockery::any(), Mockery::capture($capturedContent))
->andReturn(true);

$result = $environment->installMcp('test-key', 'herd php', ['artisan', 'boost:mcp']);

expect($result)->toBe(true)
->and($capturedContent)
->json()
->toMatchArray([
'mcpServers' => [
'test-key' => [
'command' => 'herd',
'args' => ['php', 'artisan', 'boost:mcp'],
],
],
]);
});

test('file installation handles docker exec commands', function (): void {
$environment = Mockery::mock(TestSupportsMcp::class)->makePartial();
$environment->shouldAllowMockingProtectedMethods();
Expand Down Expand Up @@ -514,3 +589,87 @@ public function mcpConfigPath(): string
],
]);
});

test('preserves absolute unix paths with spaces without splitting', function (): void {
$environment = new TestAgent($this->strategyFactory);

$result = $environment->testNormalizeCommand(
'/Users/dev/Library/Application Support/Herd/bin/php83',
['artisan', 'boost:mcp']
);

expect($result)->toBe([
'command' => '/Users/dev/Library/Application Support/Herd/bin/php83',
'args' => ['artisan', 'boost:mcp'],
]);
});

test('preserves absolute unix paths without spaces without splitting', function (): void {
$environment = new TestAgent($this->strategyFactory);

$result = $environment->testNormalizeCommand(
'/usr/local/bin/php',
['artisan', 'boost:mcp']
);

expect($result)->toBe([
'command' => '/usr/local/bin/php',
'args' => ['artisan', 'boost:mcp'],
]);
});

test('preserves absolute windows paths with spaces without splitting', function (): void {
$environment = new TestAgent($this->strategyFactory);

$result = $environment->testNormalizeCommand(
'C:\\Program Files\\PHP\\php.exe',
['artisan', 'boost:mcp']
);

expect($result)->toBe([
'command' => 'C:\\Program Files\\PHP\\php.exe',
'args' => ['artisan', 'boost:mcp'],
]);
});

test('file installation handles absolute paths with spaces correctly', function (): void {
$environment = Mockery::mock(TestSupportsMcp::class)->makePartial();
$environment->shouldAllowMockingProtectedMethods();

$capturedContent = '';

$environment->shouldReceive('mcpInstallationStrategy')
->andReturn(McpInstallationStrategy::FILE);

File::shouldReceive('ensureDirectoryExists')
->once()
->with('.test');

File::shouldReceive('exists')
->once()
->with('.test/mcp.json')
->andReturn(false);

File::shouldReceive('put')
->once()
->with(Mockery::any(), Mockery::capture($capturedContent))
->andReturn(true);

$result = $environment->installMcp(
'test-key',
'/Users/dev/Library/Application Support/Herd/bin/php83',
['artisan', 'boost:mcp']
);

expect($result)->toBe(true)
->and($capturedContent)
->json()
->toMatchArray([
'mcpServers' => [
'test-key' => [
'command' => '/Users/dev/Library/Application Support/Herd/bin/php83',
'args' => ['artisan', 'boost:mcp'],
],
],
]);
});