From 2cb4d917386bde3982d9beb9fa19e92f0bc9c63c Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 5 Mar 2026 05:15:10 +0000 Subject: [PATCH] fix: handle ProcessTimedOutException and ProcessSignaledException in SymfonyProcessRunner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap Process::run() in try/catch to gracefully handle timeout and signal exceptions. Returns a failed ProcessResult instead of letting exceptions propagate uncaught through callers. - ProcessTimedOutException → exit code 124 - ProcessSignaledException → exit code from process or 137 - Add tests for both exception paths - Run pint (style fixes in 5 additional files) Closes #50 --- app/Commands/CertifyCommand.php | 3 +-- app/GitHub/ChecksClient.php | 5 +++++ app/Services/PromptAssembler.php | 8 +++---- app/Services/SymfonyProcessRunner.php | 19 ++++++++++++++++- app/Transformers/PhpStanPromptTransformer.php | 2 +- .../TestFailurePromptTransformer.php | 3 +-- .../Services/SymfonyProcessRunnerTest.php | 21 +++++++++++++++++++ 7 files changed, 51 insertions(+), 10 deletions(-) diff --git a/app/Commands/CertifyCommand.php b/app/Commands/CertifyCommand.php index f50d6bb..685018c 100644 --- a/app/Commands/CertifyCommand.php +++ b/app/Commands/CertifyCommand.php @@ -6,7 +6,6 @@ use App\Branding; use App\Checks\CheckInterface; -use App\Checks\CheckResult; use App\Checks\PestSyntaxValidator; use App\Checks\SecurityScanner; use App\Checks\TestRunner; @@ -123,7 +122,7 @@ public function handle(): int $checksClient->postCertificationComment($checkResults); } else { // Post actionable prompt with fix directions on failure - $assembler = new PromptAssembler(); + $assembler = new PromptAssembler; $assembled = $assembler->assemble($rawOutputs); if ($assembled['prompt'] !== '') { diff --git a/app/GitHub/ChecksClient.php b/app/GitHub/ChecksClient.php index 92e2e71..17c5a33 100644 --- a/app/GitHub/ChecksClient.php +++ b/app/GitHub/ChecksClient.php @@ -89,6 +89,7 @@ public function createCheck(string $name, string $status = 'in_progress'): ?int return $data['id'] ?? null; } catch (GuzzleException $e) { $this->logError('createCheck', $e); + return null; } } @@ -119,6 +120,7 @@ public function completeCheck( return true; } catch (GuzzleException $e) { $this->logError('completeCheck', $e); + return false; } } @@ -155,6 +157,7 @@ public function reportCheck( return true; } catch (GuzzleException $e) { $this->logError('reportCheck', $e); + return false; } } @@ -199,6 +202,7 @@ public function postCertificationComment(array $checkResults): bool return true; } catch (GuzzleException $e) { $this->logError('postCertificationComment', $e); + return false; } } @@ -234,6 +238,7 @@ public function postActionablePrompt(string $prompt): bool return true; } catch (GuzzleException $e) { $this->logError('postActionablePrompt', $e); + return false; } } diff --git a/app/Services/PromptAssembler.php b/app/Services/PromptAssembler.php index 58efb7a..bd27ea9 100644 --- a/app/Services/PromptAssembler.php +++ b/app/Services/PromptAssembler.php @@ -22,8 +22,8 @@ final class PromptAssembler public function __construct() { $this->transformers = [ - new PhpStanPromptTransformer(), - new TestFailurePromptTransformer(), + new PhpStanPromptTransformer, + new TestFailurePromptTransformer, ]; } @@ -94,7 +94,7 @@ public function transform(string $checkName, string $output): array private function buildCombinedPrompt(array $failedChecks): string { $count = count($failedChecks); - $prompt = "# 🔧 Synapse Sentinel: {$count} check" . ($count === 1 ? '' : 's') . " need attention\n\n"; + $prompt = "# 🔧 Synapse Sentinel: {$count} check".($count === 1 ? '' : 's')." need attention\n\n"; $prompt .= "The following issues must be resolved before this PR can be merged:\n\n"; foreach ($failedChecks as $checkName => $section) { @@ -117,6 +117,6 @@ private function truncate(string $text, int $maxLength = 2000): string return $text; } - return substr($text, 0, $maxLength) . "\n... (truncated)"; + return substr($text, 0, $maxLength)."\n... (truncated)"; } } diff --git a/app/Services/SymfonyProcessRunner.php b/app/Services/SymfonyProcessRunner.php index 3047209..4ec7ac6 100644 --- a/app/Services/SymfonyProcessRunner.php +++ b/app/Services/SymfonyProcessRunner.php @@ -6,6 +6,8 @@ use App\Contracts\ProcessResult; use App\Contracts\ProcessRunner; +use Symfony\Component\Process\Exception\ProcessSignaledException; +use Symfony\Component\Process\Exception\ProcessTimedOutException; use Symfony\Component\Process\Process; final class SymfonyProcessRunner implements ProcessRunner @@ -13,7 +15,22 @@ final class SymfonyProcessRunner implements ProcessRunner public function run(array $command, string $workingDirectory, int $timeout = 120): ProcessResult { $process = new Process($command, $workingDirectory, timeout: $timeout); - $process->run(); + + try { + $process->run(); + } catch (ProcessTimedOutException $e) { + return new ProcessResult( + successful: false, + output: "Process timed out after {$timeout} seconds: ".$e->getMessage(), + exitCode: 124, + ); + } catch (ProcessSignaledException $e) { + return new ProcessResult( + successful: false, + output: 'Process killed by signal: '.$e->getMessage(), + exitCode: $e->getProcess()->getExitCode() ?? 137, + ); + } return new ProcessResult( successful: $process->isSuccessful(), diff --git a/app/Transformers/PhpStanPromptTransformer.php b/app/Transformers/PhpStanPromptTransformer.php index 2d3ebf5..69fa343 100644 --- a/app/Transformers/PhpStanPromptTransformer.php +++ b/app/Transformers/PhpStanPromptTransformer.php @@ -115,7 +115,7 @@ private function buildPrompt(array $data): array $relativePath = $this->relativePath($filePath); $errorCount = $fileData['errors'] ?? 0; - $prompt .= "### {$relativePath} ({$errorCount} error" . ($errorCount === 1 ? '' : 's') . ")\n\n"; + $prompt .= "### {$relativePath} ({$errorCount} error".($errorCount === 1 ? '' : 's').")\n\n"; foreach ($fileData['messages'] ?? [] as $index => $message) { $prompt .= $this->formatError($index + 1, $message); diff --git a/app/Transformers/TestFailurePromptTransformer.php b/app/Transformers/TestFailurePromptTransformer.php index 379402b..450e847 100644 --- a/app/Transformers/TestFailurePromptTransformer.php +++ b/app/Transformers/TestFailurePromptTransformer.php @@ -95,7 +95,6 @@ private function parseJunitXml(string $xml): array /** * Extract test cases from a test suite. * - * @param \SimpleXMLElement $suite * @param array> $failures * @param array> $errors */ @@ -246,7 +245,7 @@ private function cleanMessage(string $message): string // Truncate if too long if (strlen($clean) > 500) { - $clean = substr($clean, 0, 500) . '... (truncated)'; + $clean = substr($clean, 0, 500).'... (truncated)'; } return trim($clean); diff --git a/tests/Unit/Services/SymfonyProcessRunnerTest.php b/tests/Unit/Services/SymfonyProcessRunnerTest.php index bf0a7ca..f1635b2 100644 --- a/tests/Unit/Services/SymfonyProcessRunnerTest.php +++ b/tests/Unit/Services/SymfonyProcessRunnerTest.php @@ -51,5 +51,26 @@ expect($result->successful)->toBeTrue(); }); + + it('returns failed result with exit code 124 on process timeout', function () { + $runner = new SymfonyProcessRunner; + $result = $runner->run(['sleep', '30'], sys_get_temp_dir(), timeout: 1); + + expect($result)->toBeInstanceOf(ProcessResult::class); + expect($result->successful)->toBeFalse(); + expect($result->exitCode)->toBe(124); + expect($result->output)->toContain('Process timed out after 1 seconds'); + }); + + it('returns failed result when process is killed by signal', function () { + $runner = new SymfonyProcessRunner; + // bash -c that sends SIGKILL to itself + $result = $runner->run(['bash', '-c', 'kill -9 $$'], sys_get_temp_dir()); + + expect($result)->toBeInstanceOf(ProcessResult::class); + expect($result->successful)->toBeFalse(); + expect($result->output)->toContain('Process killed by signal'); + expect($result->exitCode)->toBe(137); + }); }); });