Skip to content

fix: add postActionablePrompt tests and logError type safety #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jordanpartridge wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: add postActionablePrompt tests and logError type safety #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app/Commands/CertifyCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'] !== '') {
Expand Down
8 changes: 7 additions & 1 deletion app/GitHub/ChecksClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use GuzzleHttp\Client;
use GuzzleHttp\Exception\GuzzleException;
use GuzzleHttp\Exception\RequestException;

final class ChecksClient
{
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -119,6 +121,7 @@ public function completeCheck(
return true;
} catch (GuzzleException $e) {
$this->logError('completeCheck', $e);

return false;
}
}
Expand Down Expand Up @@ -155,6 +158,7 @@ public function reportCheck(
return true;
} catch (GuzzleException $e) {
$this->logError('reportCheck', $e);

return false;
}
}
Expand Down Expand Up @@ -199,6 +203,7 @@ public function postCertificationComment(array $checkResults): bool
return true;
} catch (GuzzleException $e) {
$this->logError('postCertificationComment', $e);

return false;
}
}
Expand Down Expand Up @@ -234,6 +239,7 @@ public function postActionablePrompt(string $prompt): bool
return true;
} catch (GuzzleException $e) {
$this->logError('postActionablePrompt', $e);

return false;
}
}
Expand All @@ -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();

Expand Down
8 changes: 4 additions & 4 deletions app/Services/PromptAssembler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ final class PromptAssembler
public function __construct()
{
$this->transformers = [
new PhpStanPromptTransformer(),
new TestFailurePromptTransformer(),
new PhpStanPromptTransformer,
new TestFailurePromptTransformer,
];
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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)";
}
}
2 changes: 1 addition & 1 deletion app/Transformers/PhpStanPromptTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions app/Transformers/TestFailurePromptTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ private function parseJunitXml(string $xml): array
/**
* Extract test cases from a test suite.
*
* @param \SimpleXMLElement $suite
* @param array<array<string, mixed>> $failures
* @param array<array<string, mixed>> $errors
*/
Expand Down Expand Up @@ -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);
Expand Down
126 changes: 126 additions & 0 deletions tests/Unit/GitHub/ChecksClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading