Skip to content

Commit c2ac67c

Browse files
ashleyhindleashleyhindle
authored
Merge pull request #217 from laravel/detect-env-changes-130
Feat: Detect env changes by default, fixes 130
2 parents 2415160 + b93d49d commit c2ac67c

File tree

2 files changed

+168
-39
lines changed

2 files changed

+168
-39
lines changed

‎src/Mcp/ToolExecutor.php‎

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace Laravel\Boost\Mcp;
66

7+
use Dotenv\Dotenv;
8+
use Illuminate\Support\Env;
79
use Laravel\Mcp\Server\Tools\ToolResult;
810
use Symfony\Component\Process\Exception\ProcessFailedException;
911
use Symfony\Component\Process\Exception\ProcessTimedOutException;
@@ -13,14 +15,8 @@ class ToolExecutor
1315
{
1416
public function __construct()
1517
{
16-
//
1718
}
1819

19-
/**
20-
* Execute a tool with the given arguments.
21-
*
22-
* @param array<string, mixed> $arguments
23-
*/
2420
public function execute(string $toolClass, array $arguments = []): ToolResult
2521
{
2622
if (! ToolRegistry::isToolAllowed($toolClass)) {
@@ -34,23 +30,23 @@ public function execute(string $toolClass, array $arguments = []): ToolResult
3430
return $this->executeInline($toolClass, $arguments);
3531
}
3632

37-
/**
38-
* Execute tool in a separate process for isolation.
39-
*
40-
* @param array<string, mixed> $arguments
41-
*/
4233
protected function executeInProcess(string $toolClass, array $arguments): ToolResult
4334
{
44-
$command = [
45-
PHP_BINARY,
46-
base_path('artisan'),
47-
'boost:execute-tool',
48-
$toolClass,
49-
base64_encode(json_encode($arguments)),
50-
];
51-
52-
$process = new Process($command);
53-
$process->setTimeout($this->getTimeout());
35+
$command = $this->buildCommand($toolClass, $arguments);
36+
37+
// We need to 'unset' env vars that will be passed from the parent process to the child process, stopping the child process from reading .env and getting updated values
38+
$env = (Dotenv::create(
39+
Env::getRepository(),
40+
app()->environmentPath(),
41+
app()->environmentFile()
42+
))->safeLoad();
43+
$cleanEnv = array_fill_keys(array_keys($env), false);
44+
45+
$process = new Process(
46+
command: $command,
47+
env: $cleanEnv,
48+
timeout: $this->getTimeout()
49+
);
5450

5551
try {
5652
$process->mustRun();
@@ -62,9 +58,7 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe
6258
return ToolResult::error('Invalid JSON output from tool process: '.json_last_error_msg());
6359
}
6460

65-
// Reconstruct ToolResult from the JSON output
6661
return $this->reconstructToolResult($decoded);
67-
6862
} catch (ProcessTimedOutException $e) {
6963
$process->stop();
7064

@@ -77,14 +71,10 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe
7771
}
7872
}
7973

80-
/**
81-
* Execute tool inline (current process).
82-
*
83-
* @param array<string, mixed> $arguments
84-
*/
8574
protected function executeInline(string $toolClass, array $arguments): ToolResult
8675
{
8776
try {
77+
/** @var \Laravel\Mcp\Server\Tool $tool */
8878
$tool = app($toolClass);
8979

9080
return $tool->handle($arguments);
@@ -93,22 +83,15 @@ protected function executeInline(string $toolClass, array $arguments): ToolResul
9383
}
9484
}
9585

96-
/**
97-
* Check if process isolation should be used.
98-
*/
9986
protected function shouldUseProcessIsolation(): bool
10087
{
101-
// Never use process isolation in testing environment
10288
if (app()->environment('testing')) {
10389
return false;
10490
}
10591

106-
return config('boost.process_isolation.enabled', false);
92+
return config('boost.process_isolation.enabled', true);
10793
}
10894

109-
/**
110-
* Get the execution timeout.
111-
*/
11295
protected function getTimeout(): int
11396
{
11497
return config('boost.process_isolation.timeout', 180);
@@ -157,4 +140,22 @@ protected function reconstructToolResult(array $data): ToolResult
157140

158141
return ToolResult::text('');
159142
}
143+
144+
/**
145+
* Build the command array for executing a tool in a subprocess.
146+
*
147+
* @param string $toolClass
148+
* @param array<string, mixed> $arguments
149+
* @return array<string>
150+
*/
151+
protected function buildCommand(string $toolClass, array $arguments): array
152+
{
153+
return [
154+
PHP_BINARY,
155+
base_path('artisan'),
156+
'boost:execute-tool',
157+
$toolClass,
158+
base64_encode(json_encode($arguments)),
159+
];
160+
}
160161
}

‎tests/Feature/Mcp/ToolExecutorTest.php‎

Lines changed: 131 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
use Laravel\Boost\Mcp\ToolExecutor;
44
use Laravel\Boost\Mcp\Tools\ApplicationInfo;
5+
use Laravel\Boost\Mcp\Tools\GetConfig;
6+
use Laravel\Boost\Mcp\Tools\Tinker;
57
use Laravel\Mcp\Server\Tools\ToolResult;
68

79
test('can execute tool inline', function () {
@@ -14,10 +16,136 @@
1416
expect($result)->toBeInstanceOf(ToolResult::class);
1517
});
1618

19+
test('can execute tool with process isolation', function () {
20+
// Enable process isolation for this test
21+
config(['boost.process_isolation.enabled' => true]);
22+
23+
// Create a mock that overrides buildCommand to work with testbench
24+
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
25+
->shouldAllowMockingProtectedMethods();
26+
$executor->shouldReceive('buildCommand')
27+
->once()
28+
->andReturnUsing(fn ($toolClass, $arguments) => buildSubprocessCommand($toolClass, $arguments));
29+
30+
$result = $executor->execute(GetConfig::class, ['key' => 'app.name']);
31+
32+
expect($result)->toBeInstanceOf(ToolResult::class);
33+
34+
// If there's an error, extract the text content properly
35+
if ($result->isError) {
36+
$errorText = $result->content[0]->text ?? 'Unknown error';
37+
expect(false)->toBeTrue("Tool execution failed with error: {$errorText}");
38+
}
39+
40+
expect($result->isError)->toBeFalse();
41+
expect($result->content)->toBeArray();
42+
43+
// The content should contain the app name (which should be "Laravel" in testbench)
44+
$textContent = $result->content[0]->text ?? '';
45+
expect($textContent)->toContain('Laravel');
46+
});
47+
1748
test('rejects unregistered tools', function () {
1849
$executor = app(ToolExecutor::class);
19-
$result = $executor->execute('NonExistentToolClass', []);
50+
$result = $executor->execute('NonExistentToolClass');
2051

21-
expect($result)->toBeInstanceOf(ToolResult::class);
22-
expect($result->isError)->toBeTrue();
52+
expect($result)->toBeInstanceOf(ToolResult::class)
53+
->and($result->isError)->toBeTrue();
2354
});
55+
56+
test('subprocess proves fresh process isolation', function () {
57+
config(['boost.process_isolation.enabled' => true]);
58+
59+
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
60+
->shouldAllowMockingProtectedMethods();
61+
$executor->shouldReceive('buildCommand')
62+
->andReturnUsing(fn ($toolClass, $arguments) => buildSubprocessCommand($toolClass, $arguments));
63+
64+
$result1 = $executor->execute(Tinker::class, ['code' => 'return getmypid();']);
65+
$result2 = $executor->execute(Tinker::class, ['code' => 'return getmypid();']);
66+
67+
expect($result1->isError)->toBeFalse();
68+
expect($result2->isError)->toBeFalse();
69+
70+
$pid1 = json_decode($result1->content[0]->text, true)['result'];
71+
$pid2 = json_decode($result2->content[0]->text, true)['result'];
72+
73+
expect($pid1)->toBeInt()->not->toBe(getmypid());
74+
expect($pid2)->toBeInt()->not->toBe(getmypid());
75+
expect($pid1)->not()->toBe($pid2);
76+
});
77+
78+
test('subprocess sees modified autoloaded code changes', function () {
79+
config(['boost.process_isolation.enabled' => true]);
80+
81+
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
82+
->shouldAllowMockingProtectedMethods();
83+
$executor->shouldReceive('buildCommand')
84+
->andReturnUsing(fn ($toolClass, $arguments) => buildSubprocessCommand($toolClass, $arguments));
85+
86+
// Path to the GetConfig tool that we'll temporarily modify
87+
// TODO: Improve for parallelisation
88+
$toolPath = dirname(__DIR__, 3).'/src/Mcp/Tools/GetConfig.php';
89+
$originalContent = file_get_contents($toolPath);
90+
91+
$cleanup = function () use ($toolPath, $originalContent) {
92+
file_put_contents($toolPath, $originalContent);
93+
};
94+
95+
try {
96+
$result1 = $executor->execute(GetConfig::class, ['key' => 'app.name']);
97+
98+
expect($result1->isError)->toBeFalse();
99+
$response1 = json_decode($result1->content[0]->text, true);
100+
expect($response1['value'])->toBe('Laravel'); // Normal testbench app name
101+
102+
// Modify GetConfig.php to return a different hardcoded value
103+
$modifiedContent = str_replace(
104+
"'value' => Config::get(\$key),",
105+
"'value' => 'MODIFIED_BY_TEST',",
106+
$originalContent
107+
);
108+
file_put_contents($toolPath, $modifiedContent);
109+
110+
$result2 = $executor->execute(GetConfig::class, ['key' => 'app.name']);
111+
$response2 = json_decode($result2->content[0]->text, true);
112+
113+
expect($result2->isError)->toBeFalse();
114+
expect($response2['value'])->toBe('MODIFIED_BY_TEST'); // Using updated code, not cached
115+
} finally {
116+
$cleanup();
117+
}
118+
});
119+
120+
/**
121+
* Build a subprocess command that bootstraps testbench and executes an MCP tool via artisan.
122+
*/
123+
function buildSubprocessCommand(string $toolClass, array $arguments): array
124+
{
125+
$argumentsEncoded = base64_encode(json_encode($arguments));
126+
$testScript = sprintf(
127+
'require_once "%s/vendor/autoload.php"; '.
128+
'use Orchestra\Testbench\Foundation\Application as Testbench; '.
129+
'use Orchestra\Testbench\Foundation\Config as TestbenchConfig; '.
130+
'use Illuminate\Support\Facades\Artisan; '.
131+
'use Symfony\Component\Console\Output\BufferedOutput; '.
132+
// Bootstrap testbench like all.php does
133+
'$app = Testbench::createFromConfig(new TestbenchConfig([]), options: ["enables_package_discoveries" => false]); '.
134+
'Illuminate\Container\Container::setInstance($app); '.
135+
'$kernel = $app->make("Illuminate\Contracts\Console\Kernel"); '.
136+
'$kernel->bootstrap(); '.
137+
// Register the ExecuteToolCommand
138+
'$kernel->registerCommand(new \Laravel\Boost\Console\ExecuteToolCommand()); '.
139+
'$output = new BufferedOutput(); '.
140+
'$result = Artisan::call("boost:execute-tool", ['.
141+
' "tool" => "%s", '.
142+
' "arguments" => "%s" '.
143+
'], $output); '.
144+
'echo $output->fetch();',
145+
dirname(__DIR__, 3), // Go up from tests/Feature/Mcp to project root
146+
addslashes($toolClass),
147+
$argumentsEncoded
148+
);
149+
150+
return [PHP_BINARY, '-r', $testScript];
151+
}

0 commit comments

Comments
 (0)