Always-on process isolation: eliminate conditional complexity

[andreilungeanu]

This PR simplifies the MCP tool execution architecture by removing all conditional logic around process isolation. Instead of having dual execution paths (executeInline() vs executeInProcess()), we now always use subprocess execution with unified timeout handling. This eliminates the dangerous Windows crash scenario where set_time_limit() combined with infinite loops would cause uncatchable fatal errors that kill the entire MCP server process.

All timeout management is now centralized in the ToolExecutor with per-call timeout arguments (clamped between 1-600 seconds) instead of global configuration. This provides consistent, safe behavior across all platforms while making the codebase significantly more maintainable and testable. Users get the same reliable subprocess isolation that was already working well, but now it's the only execution path with no configuration complexity.

A new PR on the same subject after further brainstorming. The issue is not related to the timeout value.
Note: in some cases 30s might still be short, but that’s a separate concern.

## Problem
The Tinker tool was crashing the entire MCP server when user code exceeded set_time_limit() timeouts, causing uncatchable fatal errors. Windows systems were particularly vulnerable due to lack of PCNTL signal alternatives for timeout handling.

## Root Cause
- Windows limitation: set_time_limit() + infinite loops = fatal error crash (cannot be caught)
- PCNTL unavailable: Windows doesn't support PCNTL signals for timeout handling
- No graceful recovery: Server process dies completely, affecting all connected clients

## Solution
Implemented process isolation by default with platform-aware fallbacks:
Fix for #129 // also, isolation will Fix #194

### Changes

1. Process Isolation Config (config/boost.php)

   'process_isolation' => [
       'enabled' => env('BOOST_PROCESS_ISOLATION', true),  // ON by default
       'timeout' => env('BOOST_PROCESS_TIMEOUT', 180),
   ]

2. **Enhanced Tinker Tool** (`src/Mcp/Tools/Tinker.php`) - **Primary**: Relies on ToolExecutor process isolation (Windows-safe) - **Fallback**: Uses `set_time_limit()` only when isolation disabled (Unix systems) - **PCNTL support**: Additional protection on Unix systems - **Proper cleanup**: Finally blocks ensure resource cleanup 3. **Platform-Aware Tests** (`tests/Feature/Mcp/Tools/TinkerTest.php`) - Automatically skips timeout tests on Windows (PCNTL unavailable) - Tests both isolation enabled/disabled scenarios

Configuration

# Default (recommended for Windows)
BOOST_PROCESS_ISOLATION=true

# Performance mode (Unix only - risky on Windows)
BOOST_PROCESS_ISOLATION=false

## 📊 Platform Behavior

| Platform | Process Isolation | Timeout Method | Risk Level |
|----------|------------------|----------------|------------|
| Windows | ✅ Enabled (required) | ToolExecutor isolation | ✅ Safe |
| Windows | ❌ Disabled | set_time_limit() | ⚠️ CRASHES |
| Unix/Linux | ✅ Enabled | ToolExecutor isolation | ✅ Safe |
| Unix/Linux | ❌ Disabled | set_time_limit() + PCNTL | ✅ Safe |

Bottom line: Windows users can now use Tinker without fear of crashing the MCP server.

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[ahosker]

Thanks for this, it realy does keep crashing for me on windows with timeout messages.

[andreilungeanu]

Thanks for this, it realy does keep crashing for me on windows with timeout messages.

After applying this PR, turn on process isolation in boost.php, ever since I did that, no more crashes from the timeout issue on Windows in VSCode. (if you had the boost.php config published before this change, remove it or add manually new options)

More recently, I also set isolation to be enabled by default, even if your old config didn’t include that option. 75d215f

[ashleyhindle]

Seems sensible to me 👌

If you could remove the config changes, and update the branch, we're good to go 👍

[andreilungeanu]

Since parts of this PR were already implemented in feat: default process isolation to true, things are much simpler now. With the config section removed as requested and the fallback changed from false to true, the code relies on config('boost.process_isolation.enabled', true) which always returns true.

Windows users get safe process isolation by default, and this PR now simply focuses on platform-aware timeout handling and test coverage.

[ashleyhindle]

Since parts of this PR were already implemented in feat: default process isolation to true, things are much simpler now. With the config section removed as requested and the fallback changed from false to true, the code relies on config('boost.process_isolation.enabled', true) which always returns true.

Windows users get safe process isolation by default, and this PR now simply focuses on platform-aware timeout handling and test coverage.

🤔 What do you think about us removing the process isolation configuration entirely, so we can always rely on the subprocess timeout management?

Then in ToolExecutor use $arguments['timeout'] ?? 180 as the timeout?

Thinking
Originally the config was there in case the process isolation had issues, but it's been working really well for me and I don't see why it should ever be disabled.

Right now, as you documented, if somebody disables process isolation, then we use set_time_limit in Tinker and their code takes too long, the entire MCP process dies.

Two options
Fundamentally the Tinker tool should always run the eval'ed code in a fork or subprocess to protected against that, so we could add that into the Tinker tool additionally, or just only work with process isolation because it's great and works?

I recognise this is a departure from your original plan, but it solves the same issue and simplifies the Tinker tool a lot.

[andreilungeanu]

@ashleyhindle you're right - that was actually my first instinct too, but I second-guessed myself thinking the changes might be too brutal for the initial PR. The current approach of always using subprocess with getTimeout() method that extracts $arguments['timeout'] ?? 180 and clamps it between 1-600 seconds eliminates the crash risk completely while dramatically simplifying the architecture.

One thing I'd also suggest is reconsidering the hardcoded ini_set('memory_limit', '128M') in Tinker - for large projects this might be too restrictive. It should probably be configurable or at least doubled to 256M to ensure it works properly across different project sizes without running into memory issues during code execution.

[ashleyhindle]

Great change, thank you @andreilungeanu, and appreciate you going the extra mile with the latest refactor 🙌