From ad5ab5822f77dc24f6985283228dbb44dab9e9da Mon Sep 17 00:00:00 2001 From: Andrei Lungeanu Date: 2025年8月21日 21:19:50 +0300 Subject: [PATCH 1/4] Add smart timeout protection for Tinker that works on all platforms The MCP server was crashing when Tinker code ran too long or got stuck in infinite loops. This adds process isolation by default to prevent crashes, but also makes it work differently on Windows vs Linux/Mac since they handle timeouts differently. What changed: - Added process isolation config (on by default for safety) - Windows: Uses process isolation to avoid fatal crashes - Linux/Mac: Can disable isolation for speed since PCNTL handles timeouts gracefully - Added tests that skip appropriately on each platform --- src/Mcp/Tools/Tinker.php | 30 +++++++++++++++----------- tests/Feature/Mcp/Tools/TinkerTest.php | 28 +++++++++++++++++++++++- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/Mcp/Tools/Tinker.php b/src/Mcp/Tools/Tinker.php index 73a4fa51..b799cb5e 100644 --- a/src/Mcp/Tools/Tinker.php +++ b/src/Mcp/Tools/Tinker.php @@ -43,10 +43,17 @@ public function handle(array $arguments): ToolResult $code = str_replace([''], '', (string) Arr::get($arguments, 'code')); $timeout = min(180, (int) (Arr::get($arguments, 'timeout', 30))); - set_time_limit($timeout); + + // When process isolation is enabled, the ToolExecutor handles timeouts + if (! config('boost.process_isolation.enabled', false)) { + // On Windows: set_time_limit causes uncatchable fatal errors that crash the MCP server + // On Unix: set_time_limit works alongside PCNTL for redundant timeout protection + set_time_limit($timeout); + } + ini_set('memory_limit', '128M'); - // Use PCNTL alarm for additional timeout control if available (Unix only) + // Use PCNTL alarm for timeout control if available (Unix only) if (function_exists('pcntl_async_signals') && function_exists('pcntl_signal')) { pcntl_async_signals(true); pcntl_signal(SIGALRM, function () { @@ -60,12 +67,7 @@ public function handle(array $arguments): ToolResult try { $result = eval($code); - if (function_exists('pcntl_alarm')) { - pcntl_alarm(0); - } - $output = ob_get_contents(); - ob_end_clean(); $response = [ 'result' => $result, @@ -81,11 +83,6 @@ public function handle(array $arguments): ToolResult return ToolResult::json($response); } catch (Throwable $e) { - if (function_exists('pcntl_alarm')) { - pcntl_alarm(0); - } - - ob_end_clean(); return ToolResult::json([ 'error' => $e->getMessage(), @@ -93,6 +90,15 @@ public function handle(array $arguments): ToolResult 'file' => $e->getFile(), 'line' => $e->getLine(), ]); + + } finally { + + ob_end_clean(); + + // Clean up PCNTL alarm + if (function_exists('pcntl_alarm')) { + pcntl_alarm(0); + } } } } diff --git a/tests/Feature/Mcp/Tools/TinkerTest.php b/tests/Feature/Mcp/Tools/TinkerTest.php index ddaa0e34..a7bc70f2 100644 --- a/tests/Feature/Mcp/Tools/TinkerTest.php +++ b/tests/Feature/Mcp/Tools/TinkerTest.php @@ -164,6 +164,14 @@ }); test('times out when code takes too long', function () { + // Skip if PCNTL functions are not available + if (!function_exists('pcntl_async_signals') || !function_exists('pcntl_signal')) { + $this->markTestSkipped('PCNTL functions not available for timeout testing'); + } + + // Disable process isolation for this test + config(['boost.process_isolation.enabled' => false]); + $tool = new Tinker; // Code that will take more than 1 second to execute @@ -180,6 +188,24 @@ expect($result)->isToolResult() ->toolJsonContent(function ($data) { expect($data)->toHaveKey('error') - ->and($data['error'])->toMatch('/(Maximum execution time|Code execution timed out)/'); + ->and($data['error'])->toMatch('/(Maximum execution time|Code execution timed out|Fatal error)/'); + }); +}); + +test('relies on process isolation for timeout protection on Windows', function () { + // This test documents that Windows relies on process isolation for safe timeout handling + if (PHP_OS_FAMILY !== 'Windows') { + $this->markTestSkipped('Windows-specific timeout behavior test'); + } + + // Process isolation enabled (recommended for Windows) + config(['boost.process_isolation.enabled' => true]); + + $tool = new Tinker; + $result = $tool->handle(['code' => 'return "Windows: process isolation provides safe timeout protection";']); + + expect($result)->isToolResult() + ->toolJsonContent(function ($data) { + expect($data['result'])->toBe('Windows: process isolation provides safe timeout protection'); }); }); From 197e9630869f7a576bad1765ecef63410aa8065e Mon Sep 17 00:00:00 2001 From: Andrei Lungeanu Date: Wed, 3 Sep 2025 22:14:13 +0300 Subject: [PATCH 2/4] Refactor ToolExecutor to use subprocess execution and enhance timeout handling; update Tinker tool to reflect new timeout defaults and remove inline execution tests. --- src/Mcp/ToolExecutor.php | 37 +++----------- src/Mcp/Tools/Tinker.php | 25 +--------- tests/Feature/Mcp/ToolExecutorTest.php | 56 ++++++++++++++------- tests/Feature/Mcp/Tools/TinkerTest.php | 69 -------------------------- 4 files changed, 46 insertions(+), 141 deletions(-) diff --git a/src/Mcp/ToolExecutor.php b/src/Mcp/ToolExecutor.php index 43cf68f5..104f0de0 100644 --- a/src/Mcp/ToolExecutor.php +++ b/src/Mcp/ToolExecutor.php @@ -23,14 +23,10 @@ public function execute(string $toolClass, array $arguments = []): ToolResult return ToolResult::error("Tool not registered or not allowed: {$toolClass}"); } - if ($this->shouldUseProcessIsolation()) { - return $this->executeInProcess($toolClass, $arguments); - } - - return $this->executeInline($toolClass, $arguments); + return $this->executeInSubprocess($toolClass, $arguments); } - protected function executeInProcess(string $toolClass, array $arguments): ToolResult + protected function executeInSubprocess(string $toolClass, array $arguments): ToolResult { $command = $this->buildCommand($toolClass, $arguments); @@ -45,7 +41,7 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe $process = new Process( command: $command, env: $cleanEnv, - timeout: $this->getTimeout() + timeout: $this->getTimeout($arguments) ); try { @@ -62,7 +58,7 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe } catch (ProcessTimedOutException $e) { $process->stop(); - return ToolResult::error("Tool execution timed out after {$this->getTimeout()} seconds"); + return ToolResult::error("Tool execution timed out after {$this->getTimeout($arguments)} seconds"); } catch (ProcessFailedException $e) { $errorOutput = $process->getErrorOutput().$process->getOutput(); @@ -71,30 +67,11 @@ protected function executeInProcess(string $toolClass, array $arguments): ToolRe } } - protected function executeInline(string $toolClass, array $arguments): ToolResult - { - try { - /** @var \Laravel\Mcp\Server\Tool $tool */ - $tool = app($toolClass); - - return $tool->handle($arguments); - } catch (\Throwable $e) { - return ToolResult::error("Inline tool execution failed: {$e->getMessage()}"); - } - } - - protected function shouldUseProcessIsolation(): bool + protected function getTimeout(array $arguments): int { - if (app()->environment('testing')) { - return false; - } + $timeout = (int) ($arguments['timeout'] ?? 180); - return config('boost.process_isolation.enabled', true); - } - - protected function getTimeout(): int - { - return config('boost.process_isolation.timeout', 180); + return max(1, min(600, $timeout)); } /** diff --git a/src/Mcp/Tools/Tinker.php b/src/Mcp/Tools/Tinker.php index b799cb5e..effbe59c 100644 --- a/src/Mcp/Tools/Tinker.php +++ b/src/Mcp/Tools/Tinker.php @@ -30,7 +30,7 @@ public function schema(ToolInputSchema $schema): ToolInputSchema ->description('PHP code to execute (without opening required() ->integer('timeout') - ->description('Maximum execution time in seconds (default: 30)'); + ->description('Maximum execution time in seconds (default: 180)'); } /** @@ -42,26 +42,8 @@ public function handle(array $arguments): ToolResult { $code = str_replace([''], '', (string) Arr::get($arguments, 'code')); - $timeout = min(180, (int) (Arr::get($arguments, 'timeout', 30))); - - // When process isolation is enabled, the ToolExecutor handles timeouts - if (! config('boost.process_isolation.enabled', false)) { - // On Windows: set_time_limit causes uncatchable fatal errors that crash the MCP server - // On Unix: set_time_limit works alongside PCNTL for redundant timeout protection - set_time_limit($timeout); - } - ini_set('memory_limit', '128M'); - // Use PCNTL alarm for timeout control if available (Unix only) - if (function_exists('pcntl_async_signals') && function_exists('pcntl_signal')) { - pcntl_async_signals(true); - pcntl_signal(SIGALRM, function () { - throw new Exception('Code execution timed out'); - }); - pcntl_alarm($timeout); - } - ob_start(); try { @@ -94,11 +76,6 @@ public function handle(array $arguments): ToolResult } finally { ob_end_clean(); - - // Clean up PCNTL alarm - if (function_exists('pcntl_alarm')) { - pcntl_alarm(0); - } } } } diff --git a/tests/Feature/Mcp/ToolExecutorTest.php b/tests/Feature/Mcp/ToolExecutorTest.php index b226b520..9941443a 100644 --- a/tests/Feature/Mcp/ToolExecutorTest.php +++ b/tests/Feature/Mcp/ToolExecutorTest.php @@ -6,20 +6,7 @@ use Laravel\Boost\Mcp\Tools\Tinker; use Laravel\Mcp\Server\Tools\ToolResult; -test('can execute tool inline', function () { - // Disable process isolation for this test - config(['boost.process_isolation.enabled' => false]); - - $executor = app(ToolExecutor::class); - $result = $executor->execute(ApplicationInfo::class, []); - - expect($result)->toBeInstanceOf(ToolResult::class); -}); - -test('can execute tool with process isolation', function () { - // Enable process isolation for this test - config(['boost.process_isolation.enabled' => true]); - +test('can execute tool in subprocess', function () { // Create a mock that overrides buildCommand to work with testbench $executor = Mockery::mock(ToolExecutor::class)->makePartial() ->shouldAllowMockingProtectedMethods(); @@ -54,8 +41,6 @@ }); test('subprocess proves fresh process isolation', function () { - config(['boost.process_isolation.enabled' => true]); - $executor = Mockery::mock(ToolExecutor::class)->makePartial() ->shouldAllowMockingProtectedMethods(); $executor->shouldReceive('buildCommand') @@ -76,8 +61,6 @@ }); test('subprocess sees modified autoloaded code changes', function () { - config(['boost.process_isolation.enabled' => true]); - $executor = Mockery::mock(ToolExecutor::class)->makePartial() ->shouldAllowMockingProtectedMethods(); $executor->shouldReceive('buildCommand') @@ -149,3 +132,40 @@ function buildSubprocessCommand(string $toolClass, array $arguments): array return [PHP_BINARY, '-r', $testScript]; } + +test('respects custom timeout parameter', function () { + $executor = Mockery::mock(ToolExecutor::class)->makePartial() + ->shouldAllowMockingProtectedMethods(); + + $executor->shouldReceive('buildCommand') + ->andReturnUsing(fn ($toolClass, $arguments) => buildSubprocessCommand($toolClass, $arguments)); + + // Test with custom timeout - should succeed with fast code + $result = $executor->execute(Tinker::class, [ + 'code' => 'return "timeout test";', + 'timeout' => 30 + ]); + + expect($result->isError)->toBeFalse(); +}); + +test('clamps timeout values correctly', function () { + $executor = new ToolExecutor(); + + // Test timeout clamping using reflection to access protected method + $reflection = new ReflectionClass($executor); + $method = $reflection->getMethod('getTimeout'); + $method->setAccessible(true); + + // Test default + expect($method->invoke($executor, []))->toBe(180); + + // Test custom value + expect($method->invoke($executor, ['timeout' => 60]))->toBe(60); + + // Test minimum clamp + expect($method->invoke($executor, ['timeout' => 0]))->toBe(1); + + // Test maximum clamp + expect($method->invoke($executor, ['timeout' => 1000]))->toBe(600); +}); diff --git a/tests/Feature/Mcp/Tools/TinkerTest.php b/tests/Feature/Mcp/Tools/TinkerTest.php index a7bc70f2..8444067f 100644 --- a/tests/Feature/Mcp/Tools/TinkerTest.php +++ b/tests/Feature/Mcp/Tools/TinkerTest.php @@ -140,72 +140,3 @@ expect($tool->shouldRegister())->toBeTrue(); }); - -test('uses custom timeout parameter', function () { - $tool = new Tinker; - $result = $tool->handle(['code' => 'return 2 + 2;', 'timeout' => 10]); - - expect($result)->isToolResult() - ->toolJsonContentToMatchArray([ - 'result' => 4, - 'type' => 'integer', - ]); -}); - -test('uses default timeout when not specified', function () { - $tool = new Tinker; - $result = $tool->handle(['code' => 'return 2 + 2;']); - - expect($result)->isToolResult() - ->toolJsonContentToMatchArray([ - 'result' => 4, - 'type' => 'integer', - ]); -}); - -test('times out when code takes too long', function () { - // Skip if PCNTL functions are not available - if (!function_exists('pcntl_async_signals') || !function_exists('pcntl_signal')) { - $this->markTestSkipped('PCNTL functions not available for timeout testing'); - } - - // Disable process isolation for this test - config(['boost.process_isolation.enabled' => false]); - - $tool = new Tinker; - - // Code that will take more than 1 second to execute - $slowCode = ' - $start = microtime(true); - while (microtime(true) - $start < 1.2) { - usleep(50000); // Don\'t waste entire CPU - } - return "should not reach here"; - '; - - $result = $tool->handle(['code' => $slowCode, 'timeout' => 1]); - - expect($result)->isToolResult() - ->toolJsonContent(function ($data) { - expect($data)->toHaveKey('error') - ->and($data['error'])->toMatch('/(Maximum execution time|Code execution timed out|Fatal error)/'); - }); -}); - -test('relies on process isolation for timeout protection on Windows', function () { - // This test documents that Windows relies on process isolation for safe timeout handling - if (PHP_OS_FAMILY !== 'Windows') { - $this->markTestSkipped('Windows-specific timeout behavior test'); - } - - // Process isolation enabled (recommended for Windows) - config(['boost.process_isolation.enabled' => true]); - - $tool = new Tinker; - $result = $tool->handle(['code' => 'return "Windows: process isolation provides safe timeout protection";']); - - expect($result)->isToolResult() - ->toolJsonContent(function ($data) { - expect($data['result'])->toBe('Windows: process isolation provides safe timeout protection'); - }); -}); From cfe55a07697e35c77b2f5099e1518221f8616e0d Mon Sep 17 00:00:00 2001 From: Ashley Hindle Date: Thu, 4 Sep 2025 07:51:52 +0100 Subject: [PATCH 3/4] feat: change tinker's default memory limit to 256M --- src/Mcp/Tools/Tinker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mcp/Tools/Tinker.php b/src/Mcp/Tools/Tinker.php index effbe59c..80404dbf 100644 --- a/src/Mcp/Tools/Tinker.php +++ b/src/Mcp/Tools/Tinker.php @@ -42,7 +42,7 @@ public function handle(array $arguments): ToolResult { $code = str_replace([''], '', (string) Arr::get($arguments, 'code')); - ini_set('memory_limit', '128M'); + ini_set('memory_limit', '256M'); ob_start(); From 64d594e064e38d860edff948bccfd78929f012d7 Mon Sep 17 00:00:00 2001 From: Ashley Hindle Date: Thu, 4 Sep 2025 07:53:42 +0100 Subject: [PATCH 4/4] style: remove spacer lines --- src/Mcp/Tools/Tinker.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Mcp/Tools/Tinker.php b/src/Mcp/Tools/Tinker.php index 80404dbf..52d9004e 100644 --- a/src/Mcp/Tools/Tinker.php +++ b/src/Mcp/Tools/Tinker.php @@ -63,9 +63,7 @@ public function handle(array $arguments): ToolResult } return ToolResult::json($response); - } catch (Throwable $e) { - return ToolResult::json([ 'error' => $e->getMessage(), 'type' => get_class($e), @@ -74,7 +72,6 @@ public function handle(array $arguments): ToolResult ]); } finally { - ob_end_clean(); } }

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