Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 8aae29e

Browse files
Merge pull request #184 from andreilungeanu/main
Always-on process isolation: eliminate conditional complexity
2 parents 0b7ad74 + 64d594e commit 8aae29e

File tree

4 files changed

+50
-116
lines changed

4 files changed

+50
-116
lines changed

‎src/Mcp/ToolExecutor.php

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,10 @@ public function execute(string $toolClass, array $arguments = []): ToolResult
2323
return ToolResult::error("Tool not registered or not allowed: {$toolClass}");
2424
}
2525

26-
if ($this->shouldUseProcessIsolation()) {
27-
return $this->executeInProcess($toolClass, $arguments);
28-
}
29-
30-
return $this->executeInline($toolClass, $arguments);
26+
return $this->executeInSubprocess($toolClass, $arguments);
3127
}
3228

33-
protected function executeInProcess(string $toolClass, array $arguments): ToolResult
29+
protected function executeInSubprocess(string $toolClass, array $arguments): ToolResult
3430
{
3531
$command = $this->buildCommand($toolClass, $arguments);
3632

@@ -45,7 +41,7 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe
4541
$process = new Process(
4642
command: $command,
4743
env: $cleanEnv,
48-
timeout: $this->getTimeout()
44+
timeout: $this->getTimeout($arguments)
4945
);
5046

5147
try {
@@ -62,7 +58,7 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe
6258
} catch (ProcessTimedOutException $e) {
6359
$process->stop();
6460

65-
return ToolResult::error("Tool execution timed out after {$this->getTimeout()} seconds");
61+
return ToolResult::error("Tool execution timed out after {$this->getTimeout($arguments)} seconds");
6662

6763
} catch (ProcessFailedException $e) {
6864
$errorOutput = $process->getErrorOutput().$process->getOutput();
@@ -71,30 +67,11 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe
7167
}
7268
}
7369

74-
protected function executeInline(string $toolClass, array $arguments): ToolResult
75-
{
76-
try {
77-
/** @var \Laravel\Mcp\Server\Tool $tool */
78-
$tool = app($toolClass);
79-
80-
return $tool->handle($arguments);
81-
} catch (\Throwable $e) {
82-
return ToolResult::error("Inline tool execution failed: {$e->getMessage()}");
83-
}
84-
}
85-
86-
protected function shouldUseProcessIsolation(): bool
70+
protected function getTimeout(array $arguments): int
8771
{
88-
if (app()->environment('testing')) {
89-
return false;
90-
}
72+
$timeout = (int) ($arguments['timeout'] ?? 180);
9173

92-
return config('boost.process_isolation.enabled', true);
93-
}
94-
95-
protected function getTimeout(): int
96-
{
97-
return config('boost.process_isolation.timeout', 180);
74+
return max(1, min(600, $timeout));
9875
}
9976

10077
/**

‎src/Mcp/Tools/Tinker.php

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function schema(ToolInputSchema $schema): ToolInputSchema
3030
->description('PHP code to execute (without opening <?php tags)')
3131
->required()
3232
->integer('timeout')
33-
->description('Maximum execution time in seconds (default: 30)');
33+
->description('Maximum execution time in seconds (default: 180)');
3434
}
3535

3636
/**
@@ -42,30 +42,14 @@ public function handle(array $arguments): ToolResult
4242
{
4343
$code = str_replace(['<?php', '?>'], '', (string) Arr::get($arguments, 'code'));
4444

45-
$timeout = min(180, (int) (Arr::get($arguments, 'timeout', 30)));
46-
set_time_limit($timeout);
47-
ini_set('memory_limit', '128M');
48-
49-
// Use PCNTL alarm for additional timeout control if available (Unix only)
50-
if (function_exists('pcntl_async_signals') && function_exists('pcntl_signal')) {
51-
pcntl_async_signals(true);
52-
pcntl_signal(SIGALRM, function () {
53-
throw new Exception('Code execution timed out');
54-
});
55-
pcntl_alarm($timeout);
56-
}
45+
ini_set('memory_limit', '256M');
5746

5847
ob_start();
5948

6049
try {
6150
$result = eval($code);
6251

63-
if (function_exists('pcntl_alarm')) {
64-
pcntl_alarm(0);
65-
}
66-
6752
$output = ob_get_contents();
68-
ob_end_clean();
6953

7054
$response = [
7155
'result' => $result,
@@ -79,20 +63,16 @@ public function handle(array $arguments): ToolResult
7963
}
8064

8165
return ToolResult::json($response);
82-
8366
} catch (Throwable $e) {
84-
if (function_exists('pcntl_alarm')) {
85-
pcntl_alarm(0);
86-
}
87-
88-
ob_end_clean();
89-
9067
return ToolResult::json([
9168
'error' => $e->getMessage(),
9269
'type' => get_class($e),
9370
'file' => $e->getFile(),
9471
'line' => $e->getLine(),
9572
]);
73+
74+
} finally {
75+
ob_end_clean();
9676
}
9777
}
9878
}

‎tests/Feature/Mcp/ToolExecutorTest.php

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,7 @@
66
use Laravel\Boost\Mcp\Tools\Tinker;
77
use Laravel\Mcp\Server\Tools\ToolResult;
88

9-
test('can execute tool inline', function () {
10-
// Disable process isolation for this test
11-
config(['boost.process_isolation.enabled' => false]);
12-
13-
$executor = app(ToolExecutor::class);
14-
$result = $executor->execute(ApplicationInfo::class, []);
15-
16-
expect($result)->toBeInstanceOf(ToolResult::class);
17-
});
18-
19-
test('can execute tool with process isolation', function () {
20-
// Enable process isolation for this test
21-
config(['boost.process_isolation.enabled' => true]);
22-
9+
test('can execute tool in subprocess', function () {
2310
// Create a mock that overrides buildCommand to work with testbench
2411
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
2512
->shouldAllowMockingProtectedMethods();
@@ -54,8 +41,6 @@
5441
});
5542

5643
test('subprocess proves fresh process isolation', function () {
57-
config(['boost.process_isolation.enabled' => true]);
58-
5944
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
6045
->shouldAllowMockingProtectedMethods();
6146
$executor->shouldReceive('buildCommand')
@@ -76,8 +61,6 @@
7661
});
7762

7863
test('subprocess sees modified autoloaded code changes', function () {
79-
config(['boost.process_isolation.enabled' => true]);
80-
8164
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
8265
->shouldAllowMockingProtectedMethods();
8366
$executor->shouldReceive('buildCommand')
@@ -149,3 +132,40 @@ function buildSubprocessCommand(string $toolClass, array $arguments): array
149132

150133
return [PHP_BINARY, '-r', $testScript];
151134
}
135+
136+
test('respects custom timeout parameter', function () {
137+
$executor = Mockery::mock(ToolExecutor::class)->makePartial()
138+
->shouldAllowMockingProtectedMethods();
139+
140+
$executor->shouldReceive('buildCommand')
141+
->andReturnUsing(fn ($toolClass, $arguments) => buildSubprocessCommand($toolClass, $arguments));
142+
143+
// Test with custom timeout - should succeed with fast code
144+
$result = $executor->execute(Tinker::class, [
145+
'code' => 'return "timeout test";',
146+
'timeout' => 30
147+
]);
148+
149+
expect($result->isError)->toBeFalse();
150+
});
151+
152+
test('clamps timeout values correctly', function () {
153+
$executor = new ToolExecutor();
154+
155+
// Test timeout clamping using reflection to access protected method
156+
$reflection = new ReflectionClass($executor);
157+
$method = $reflection->getMethod('getTimeout');
158+
$method->setAccessible(true);
159+
160+
// Test default
161+
expect($method->invoke($executor, []))->toBe(180);
162+
163+
// Test custom value
164+
expect($method->invoke($executor, ['timeout' => 60]))->toBe(60);
165+
166+
// Test minimum clamp
167+
expect($method->invoke($executor, ['timeout' => 0]))->toBe(1);
168+
169+
// Test maximum clamp
170+
expect($method->invoke($executor, ['timeout' => 1000]))->toBe(600);
171+
});

‎tests/Feature/Mcp/Tools/TinkerTest.php

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -140,46 +140,3 @@
140140

141141
expect($tool->shouldRegister())->toBeTrue();
142142
});
143-
144-
test('uses custom timeout parameter', function () {
145-
$tool = new Tinker;
146-
$result = $tool->handle(['code' => 'return 2 + 2;', 'timeout' => 10]);
147-
148-
expect($result)->isToolResult()
149-
->toolJsonContentToMatchArray([
150-
'result' => 4,
151-
'type' => 'integer',
152-
]);
153-
});
154-
155-
test('uses default timeout when not specified', function () {
156-
$tool = new Tinker;
157-
$result = $tool->handle(['code' => 'return 2 + 2;']);
158-
159-
expect($result)->isToolResult()
160-
->toolJsonContentToMatchArray([
161-
'result' => 4,
162-
'type' => 'integer',
163-
]);
164-
});
165-
166-
test('times out when code takes too long', function () {
167-
$tool = new Tinker;
168-
169-
// Code that will take more than 1 second to execute
170-
$slowCode = '
171-
$start = microtime(true);
172-
while (microtime(true) - $start < 1.2) {
173-
usleep(50000); // Don\'t waste entire CPU
174-
}
175-
return "should not reach here";
176-
';
177-
178-
$result = $tool->handle(['code' => $slowCode, 'timeout' => 1]);
179-
180-
expect($result)->isToolResult()
181-
->toolJsonContent(function ($data) {
182-
expect($data)->toHaveKey('error')
183-
->and($data['error'])->toMatch('/(Maximum execution time|Code execution timed out)/');
184-
});
185-
});

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /