From a7a020defd51431568d13c980dfccaa47f3a219f Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 5 Mar 2026 05:14:29 +0000 Subject: [PATCH] fix: add postActionablePrompt tests and logError type safety (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 7 tests for postActionablePrompt() covering all paths: not available, no PR number, empty prompt, success, API error, 403, and 429 responses. Fix logError() type safety by adding instanceof RequestException check before calling hasResponse(), which only exists on RequestException, not on the GuzzleException interface. ChecksClient coverage: 89.1% → 100% --- app/Commands/CertifyCommand.php | 3 +- app/GitHub/ChecksClient.php | 8 +- app/Services/PromptAssembler.php | 8 +- app/Transformers/PhpStanPromptTransformer.php | 2 +- .../TestFailurePromptTransformer.php | 3 +- tests/Unit/GitHub/ChecksClientTest.php | 126 ++++++++++++++++++ 6 files changed, 140 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..fac66a9 100644 --- a/app/GitHub/ChecksClient.php +++ b/app/GitHub/ChecksClient.php @@ -6,6 +6,7 @@ use GuzzleHttp\Client; use GuzzleHttp\Exception\GuzzleException; +use GuzzleHttp\Exception\RequestException; final class ChecksClient { @@ -89,6 +90,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 +121,7 @@ public function completeCheck( return true; } catch (GuzzleException $e) { $this->logError('completeCheck', $e); + return false; } } @@ -155,6 +158,7 @@ public function reportCheck( return true; } catch (GuzzleException $e) { $this->logError('reportCheck', $e); + return false; } } @@ -199,6 +203,7 @@ public function postCertificationComment(array $checkResults): bool return true; } catch (GuzzleException $e) { $this->logError('postCertificationComment', $e); + return false; } } @@ -234,6 +239,7 @@ public function postActionablePrompt(string $prompt): bool return true; } catch (GuzzleException $e) { $this->logError('postActionablePrompt', $e); + return false; } } @@ -242,7 +248,7 @@ private function logError(string $method, GuzzleException $e): void { echo "::error::GitHub API error in {$method}: {$e->getMessage()}\n"; - if ($e->hasResponse()) { + if ($e instanceof RequestException && $e->hasResponse()) { $response = $e->getResponse(); $statusCode = $response->getStatusCode(); 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/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/GitHub/ChecksClientTest.php b/tests/Unit/GitHub/ChecksClientTest.php index 8fa0262..e2cadd6 100644 --- a/tests/Unit/GitHub/ChecksClientTest.php +++ b/tests/Unit/GitHub/ChecksClientTest.php @@ -374,6 +374,132 @@ }); }); + describe('postActionablePrompt', function () { + it('returns false when not available', function () { + $client = new ChecksClient(token: null); + + expect($client->postActionablePrompt('Fix this'))->toBeFalse(); + }); + + it('returns false when no PR number', function () { + $client = new ChecksClient( + token: 'test-token', + repo: 'owner/repo', + sha: 'abc123', + prNumber: null, + ); + + expect($client->postActionablePrompt('Fix this'))->toBeFalse(); + }); + + it('returns true for empty prompt', function () { + $client = new ChecksClient( + token: 'test-token', + repo: 'owner/repo', + sha: 'abc123', + prNumber: 42, + ); + + expect($client->postActionablePrompt(''))->toBeTrue(); + }); + + it('returns true on successful API call', function () { + $mock = new MockHandler([ + new Response(201), + ]); + $handlerStack = HandlerStack::create($mock); + $httpClient = new Client(['handler' => $handlerStack]); + + $client = new ChecksClient( + token: 'test-token', + client: $httpClient, + repo: 'owner/repo', + sha: 'abc123', + prNumber: 42, + ); + + expect($client->postActionablePrompt('Fix this code'))->toBeTrue(); + }); + + it('returns false and outputs error on API error', function () { + $mock = new MockHandler([ + new RequestException('API Error', new Request('POST', 'test')), + ]); + $handlerStack = HandlerStack::create($mock); + $httpClient = new Client(['handler' => $handlerStack]); + + $client = new ChecksClient( + token: 'test-token', + client: $httpClient, + repo: 'owner/repo', + sha: 'abc123', + prNumber: 42, + ); + + ob_start(); + $result = $client->postActionablePrompt('Fix this'); + $output = ob_get_clean(); + + expect($result)->toBeFalse(); + expect($output)->toContain('::error::'); + expect($output)->toContain('API Error'); + }); + + it('outputs specific error for 403 permission denied', function () { + $mock = new MockHandler([ + new RequestException( + 'Forbidden', + new Request('POST', 'test'), + new Response(403) + ), + ]); + $handlerStack = HandlerStack::create($mock); + $httpClient = new Client(['handler' => $handlerStack]); + + $client = new ChecksClient( + token: 'test-token', + client: $httpClient, + repo: 'owner/repo', + sha: 'abc123', + prNumber: 42, + ); + + ob_start(); + $client->postActionablePrompt('Fix this'); + $output = ob_get_clean(); + + expect($output)->toContain('::error::'); + expect($output)->toContain('Permission denied'); + }); + + it('outputs specific error for 429 rate limit', function () { + $mock = new MockHandler([ + new RequestException( + 'Too Many Requests', + new Request('POST', 'test'), + new Response(429) + ), + ]); + $handlerStack = HandlerStack::create($mock); + $httpClient = new Client(['handler' => $handlerStack]); + + $client = new ChecksClient( + token: 'test-token', + client: $httpClient, + repo: 'owner/repo', + sha: 'abc123', + prNumber: 42, + ); + + ob_start(); + $client->postActionablePrompt('Fix this'); + $output = ob_get_clean(); + + expect($output)->toContain('::error::'); + expect($output)->toContain('Rate limit exceeded'); + }); + }); + describe('postCertificationComment', function () { it('returns false when not available', function () { $client = new ChecksClient(token: null);