Skip to content

fix: display full error messages instead of truncated table output #72

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: display full error messages instead of truncated table output #72

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
10 changes: 5 additions & 5 deletions app/Commands/CertifyCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use function Laravel\Prompts\error;
use function Laravel\Prompts\info;
use function Laravel\Prompts\spin;
use function Laravel\Prompts\table;

final class CertifyCommand extends Command
{
Expand Down Expand Up @@ -146,10 +145,11 @@ public function handle(): int
error('━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━');

if (! empty($failureRows)) {
table(
headers: ['Check', 'Issue'],
rows: $failureRows
);
$this->newLine();
foreach ($failureRows as $row) {
$this->line(" <fg=red>✗</> <fg=white;options=bold>{$row[0]}</>: {$row[1]}");
}
$this->newLine();
Comment on lines +148 to +152
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find and read the CertifyCommand.php file
find . -name "CertifyCommand.php" -type f

Repository: synapse-sentinel/gate

Length of output: 98

🏁 Script executed:

# Read the CertifyCommand.php file, focusing on the area around lines 148-152
wc -l ./app/Commands/CertifyCommand.php

Repository: synapse-sentinel/gate

Length of output: 102

🏁 Script executed:

cat -n ./app/Commands/CertifyCommand.php | sed -n '130,170p'

Repository: synapse-sentinel/gate

Length of output: 1875

🏁 Script executed:

# Now let me find where $failureRows is populated
cat -n ./app/Commands/CertifyCommand.php | head -150 | tail -50

Repository: synapse-sentinel/gate

Length of output: 2321

🏁 Script executed:

# Search for failureRows initialization/population
grep -n "failureRows" ./app/Commands/CertifyCommand.php

Repository: synapse-sentinel/gate

Length of output: 259

🏁 Script executed:

# Let me see the full context around lines 71 and 94 where failureRows is created/populated
cat -n ./app/Commands/CertifyCommand.php | sed -n '65,100p'

Repository: synapse-sentinel/gate

Length of output: 1567

🏁 Script executed:

# Let me find where CheckResult or similar is defined to understand what $result->details contains
find . -name "*.php" -type f | xargs grep -l "class.*Check\|interface.*Check" | head -10

Repository: synapse-sentinel/gate

Length of output: 375

🏁 Script executed:

# Also, let me look at the runCheck method to understand the result structure
cat -n ./app/Commands/CertifyCommand.php | sed -n '170,234p'

Repository: synapse-sentinel/gate

Length of output: 2297

🏁 Script executed:

# Let me examine the CheckResult class to understand what $details contains
cat -n ./app/Checks/CheckResult.php

Repository: synapse-sentinel/gate

Length of output: 796

🏁 Script executed:

# Let me check the various Check implementations to see what data goes into details
cat -n ./app/Checks/SecurityScanner.php | head -100

Repository: synapse-sentinel/gate

Length of output: 2330

🏁 Script executed:

# Also check TestRunner
cat -n ./app/Checks/TestRunner.php | head -100

Repository: synapse-sentinel/gate

Length of output: 4064

🏁 Script executed:

# Continue reading TestRunner to see parseFailureDetails implementation
cat -n ./app/Checks/TestRunner.php | sed -n '100,180p'

Repository: synapse-sentinel/gate

Length of output: 2103

🏁 Script executed:

# Let me check if OutputFormatter is already used in the codebase
grep -r "OutputFormatter" ./app --include="*.php"

Repository: synapse-sentinel/gate

Length of output: 47

🏁 Script executed:

# Also check what PestOutputParser returns for parseFailures to understand the content better
cat -n ./app/Services/PestOutputParser.php | head -80

Repository: synapse-sentinel/gate

Length of output: 3304

🏁 Script executed:

# Let me check what dependencies are available, particularly Symfony
cat composer.json | grep -A 20 '"require"'

Repository: synapse-sentinel/gate

Length of output: 555

🏁 Script executed:

# Also check if there are any console-related imports in CertifyCommand
head -30 ./app/Commands/CertifyCommand.php

Repository: synapse-sentinel/gate

Length of output: 926

🏁 Script executed:

# Check if symfony/console is available as a transitive dependency
cat composer.lock | grep -A 5 '"name": "symfony/console"'

Repository: synapse-sentinel/gate

Length of output: 328

🏁 Script executed:

# Also verify what base class CertifyCommand extends and where line() comes from
grep -r "class Command" ./app --include="*.php" -A 5

Repository: synapse-sentinel/gate

Length of output: 47

🏁 Script executed:

# Search for escape or sanitize patterns in the codebase
grep -r "escape\|sanitize\|htmlspecialchars" ./app --include="*.php"

Repository: synapse-sentinel/gate

Length of output: 47

Escape dynamic failure text before writing styled console output.

At line 150, $row[0] and $row[1] are injected directly into a formatted console string. These values originate from external sources: check names, package names from composer audit JSON, test failure messages from Pest output, and file paths. If any contain <...> patterns, Symfony's OutputFormatter will parse them as formatting tags, potentially stripping or altering content and breaking error visibility.

Proposed fix 
+use Symfony\Component\Console\Formatter\OutputFormatter;
...
                 $this->newLine();
                 foreach ($failureRows as $row) {
-                    $this->line("  <fg=red>✗</> <fg=white;options=bold>{$row[0]}</>: {$row[1]}");
+                    $checkName = OutputFormatter::escape((string) $row[0]);
+                    $detail = OutputFormatter::escape((string) $row[1]);
+                    $this->line("  <fg=red>✗</> <fg=white;options=bold>{$checkName}</>: {$detail}");
                 }
                 $this->newLine();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$this->newLine();
foreach ($failureRows as $row) {
$this->line(" <fg=red>✗</> <fg=white;options=bold>{$row[0]}</>: {$row[1]}");
}
$this->newLine();
$this->newLine();
foreach ($failureRows as $row) {
$checkName = OutputFormatter::escape((string) $row[0]);
$detail = OutputFormatter::escape((string) $row[1]);
$this->line(" <fg=red>✗</> <fg=white;options=bold>{$checkName}</>: {$detail}");
}
$this->newLine();
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/Commands/CertifyCommand.php` around lines 148 - 152, The console output
is injecting unescaped dynamic values from failureRows ($row[0], $row[1]) into a
formatted string, which lets any "<...>" sequences be interpreted as Symfony
formatting tags; fix it by escaping both values with
Symfony\Component\Console\Formatter\OutputFormatter::escape() (or equivalent
escape helper available in CertifyCommand) before interpolating them into the
styled $this->line call so the red/white styling remains but the content is
printed verbatim.

}
}

Expand Down
55 changes: 55 additions & 0 deletions tests/Unit/Commands/CertifyCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,61 @@
->assertFailed();
});

it('displays full error messages without truncation', function () {
$longDetail = 'Error in src/Services/PaymentGateway.php: Method processTransaction() has cyclomatic complexity of 25 which exceeds the configured maximum of 10. Consider refactoring this method into smaller, more focused methods to improve maintainability and testability.';

$failingCheck = Mockery::mock(CheckInterface::class);
$failingCheck->shouldReceive('name')->andReturn('Tests');
$failingCheck->shouldReceive('run')
->once()
->andReturn(CheckResult::fail('1 test failed', [$longDetail]));

$mock = new MockHandler([
new Response(201),
new Response(201),
]);
$httpClient = new Client(['handler' => HandlerStack::create($mock)]);
$checksClient = new ChecksClient('token', $httpClient, 'owner/repo', 'sha123');

($this->createCommand)([$failingCheck], $checksClient);

$this->artisan('certify')
->expectsOutputToContain($longDetail)
->assertFailed();
});

it('displays multiple long error details from different checks', function () {
$detail1 = 'FAIL Tests\\Unit\\OrderServiceTest > it calculates the total price correctly including tax, shipping, and discount adjustments for international orders';
$detail2 = 'CVE-2024-9999: Critical vulnerability found in vendor/acme/library v2.3.1 - Remote code execution via deserialization of untrusted data in JsonParser::decode()';

$failingCheck1 = Mockery::mock(CheckInterface::class);
$failingCheck1->shouldReceive('name')->andReturn('Tests');
$failingCheck1->shouldReceive('run')
->once()
->andReturn(CheckResult::fail('Tests failed', [$detail1]));

$failingCheck2 = Mockery::mock(CheckInterface::class);
$failingCheck2->shouldReceive('name')->andReturn('Security');
$failingCheck2->shouldReceive('run')
->once()
->andReturn(CheckResult::fail('Security failed', [$detail2]));

$mock = new MockHandler([
new Response(201),
new Response(201),
new Response(201),
]);
$httpClient = new Client(['handler' => HandlerStack::create($mock)]);
$checksClient = new ChecksClient('token', $httpClient, 'owner/repo', 'sha123');

($this->createCommand)([$failingCheck1, $failingCheck2], $checksClient);

$this->artisan('certify')
->expectsOutputToContain($detail1)
->expectsOutputToContain($detail2)
->assertFailed();
});

it('shortens check names in compact output', function () {
// This test covers the shortName() method by using the actual check names
$testsCheck = Mockery::mock(CheckInterface::class);
Expand Down
Loading