Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"require": {
"php": "^8.2|^8.3|^8.4",
"firebase/php-jwt": "^7.0",
"illuminate/contracts": "^11.0||^12.0",
"illuminate/contracts": "^11.0||^12.0||^13.0",
Copy link
Copy Markdown

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

🌐 Web query:

What version of orchestra/testbench is compatible with Laravel 13 (illuminate/* v13), and do spatie/laravel-package-tools v1.16 or v2.0 officially support Laravel 13?

💡 Result:

  • Laravel 13 (illuminate/ v13) + orchestra/testbench:* use orchestra/testbench ^11.0 (11.x) — it requires laravel/framework ^13.0.0. [1]
    (Testbench Core is also on 11.x for Laravel 13.) [2]

  • spatie/laravel-package-tools v1.16.x: does not officially support Laravel 131.16.0 only allows illuminate/contracts ^9.28|^10.0 [3] and 1.16.6 only up to ^11.0. [4]

  • spatie/laravel-package-tools v2.0: there is no 2.x release on Packagist (current releases are 1.x; e.g. latest shown is 1.93.0). [5]
    If you need Laravel 13 support, spatie/laravel-package-tools 1.93.0 explicitly supports illuminate/contracts ^13.0. [6]

Sources: [1] (packagist.org) [2] (packagist.org) [3] (packages.ecosyste.ms) [4] (packages.ecosyste.ms) [5] (packages.ecosyste.ms) [6] (packagist.org)

Citations:


🏁 Script executed:

cat composer.json

Repository: jordanpartridge/github-client

Length of output: 3286


Update orchestra/testbench and spatie/laravel-package-tools to properly support Laravel 13.

Line 31 claims support for illuminate/contracts ^13.0, but the dev and runtime dependency constraints are incomplete:

  • orchestra/testbench (dev): currently ^8.0||^9.0||^10.0||^11.0||^12.0 — testbench 12.x is incompatible with Laravel 13. Add ^13.0 to the constraint.
  • spatie/laravel-package-tools (runtime): currently ^1.16|^2.0 — versions 1.16.0 through 1.92.x do not support Laravel 13 (only 1.93.0+). Tighten to ^1.93|^2.0 (noting that 2.x does not yet exist on Packagist, so effectively ^1.93.0).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@composer.json` at line 31, Update the composer dependency constraints to
properly signal Laravel 13 support: in composer.json amend the
"orchestra/testbench" constraint to include "^13.0" (so it becomes
^8.0||^9.0||^10.0||^11.0||^12.0||^13.0) and tighten
"spatie/laravel-package-tools" to " ^1.93|^2.0" (effectively ^1.93.0 to ensure
versions that support Laravel 13 are required).

"jordanpartridge/conduit-interfaces": "^1.1",
"saloonphp/saloon": "^3.10",
"spatie/laravel-package-tools": "^1.16|^2.0"
Expand Down
21 changes: 21 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
parameters:
ignoreErrors:
-
message: "#^Instantiated class JordanPartridge\\\\GithubClient\\\\Exceptions\\\\ResourceNotFoundException not found\\.$#"
count: 1
path: src/Connectors/GithubConnector.php
Comment on lines +3 to +6
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for ResourceNotFoundException class definition
rg -n 'class ResourceNotFoundException' --type php

Repository: jordanpartridge/github-client

Length of output: 55


Do not suppress instantiation of non-existent ResourceNotFoundException class—fix the underlying bug.

The baseline suppresses a fatal error: "Instantiated class ResourceNotFoundException not found" in GithubConnector.php. This class does not exist in the codebase, so code attempting to instantiate it will fail at runtime.

Fix by either creating the ResourceNotFoundException class or using an existing exception class that matches the intended behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@phpstan-baseline.neon` around lines 3 - 6, The baseline hides a fatal runtime
error caused by instantiating a non-existent ResourceNotFoundException in the
GithubConnector code; fix it by either adding the missing exception class (e.g.,
create class ResourceNotFoundException extends \Exception in the appropriate
namespace) or change the instantiation to use an existing exception type that
conveys "not found" semantics (update the throw/new site in GithubConnector and
its use/import statements to reference the correct exception class, e.g., an
existing NotFoundException or GithubClient\Exceptions\SomeExistingException).
Ensure the chosen exception lives in the namespace imported by GithubConnector
(adjust use statements) and remove the phpstan baseline suppression for that
message.


-
message: "#^Missing parameter \\$message \\(string\\) in call to JordanPartridge\\\\GithubClient\\\\Exceptions\\\\NetworkException constructor\\.$#"
count: 1
path: src/Connectors/GithubConnector.php
Comment on lines +8 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find NetworkException constructor calls with 'reason:' parameter
rg -n -A3 'new NetworkException\(' src/Connectors/GithubConnector.php

Repository: jordanpartridge/github-client

Length of output: 301


🏁 Script executed:

#!/bin/bash
# Find NetworkException class definition
find . -name "*.php" -type f | xargs grep -l "class NetworkException"

Repository: jordanpartridge/github-client

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Check the constructor of NetworkException
rg -A10 "class NetworkException" --type php

Repository: jordanpartridge/github-client

Length of output: 750


🏁 Script executed:

#!/bin/bash
# Find all NetworkException constructor calls
rg -n "new NetworkException\(" --type php -A4

Repository: jordanpartridge/github-client

Length of output: 1114


Critical: Fix mismatched parameter name instead of suppressing the error.

Lines 8-11 and 18-21 in phpstan-baseline.neon suppress errors about incorrect NetworkException constructor calls. The constructor signature is:

public function __construct(
    string $operation,
    string $message,
    int $code = 0,
    ?\Throwable $previous = null,
)

However, src/Connectors/GithubConnector.php:148 calls it with the wrong parameter name:

new NetworkException(
    operation: 'GitHub API request',
    reason: "Server error ({$status}): {$message}",  // ✗ wrong parameter name
    previous: $senderException,
)

This will cause a fatal TypeError at runtime: unknown parameter $reason and missing required parameter $message.

Fix in GithubConnector.php:

Corrected code
 new NetworkException(
     operation: 'GitHub API request',
-    reason: "Server error ({$status}): {$message}",
+    message: "Server error ({$status}): {$message}",
     previous: $senderException,
 )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@phpstan-baseline.neon` around lines 8 - 11, The call to NetworkException in
GithubConnector (the instantiation in src/Connectors/GithubConnector.php) uses
the wrong named parameter "reason" and omits the required "message" parameter;
update the constructor call to use message: "Server error ({$status}):
{$message}" (or equivalent string) instead of reason:, keep operation: 'GitHub
API request' and previous: $senderException so the parameters match
NetworkException::__construct(string $operation, string $message, int $code = 0,
?\Throwable $previous = null).


-
message: "#^Strict comparison using \\=\\=\\= between non\\-falsy\\-string and '' will always evaluate to false\\.$#"
count: 1
path: src/Connectors/GithubConnector.php
Comment on lines +13 to +16
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Verify if the redundant empty string check is intentional.

The baseline suppresses PHPStan's warning about $this->token === '' always evaluating to false after the falsy check. In the code:

if (! $this->token || $this->token === '')

The empty string case is already covered by ! $this->token since empty strings are falsy. The explicit === '' check is redundant.

Options:

  1. If the redundancy is intentional for clarity, document it with a comment and keep the suppression.
  2. Otherwise, simplify to if (! $this->token) and remove this baseline entry.
♻️ Suggested simplification
-if (! $this->token || $this->token === '') {
+if (! $this->token) {
     return null;
 }

Then remove lines 13-16 from this baseline file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@phpstan-baseline.neon` around lines 13 - 16, The baseline suppresses
PHPStan's warning about the redundant check for `$this->token === ''` in the
condition `if (! $this->token || $this->token === '')`; locate the usage of
`$this->token` in the GithubConnector class (e.g., the method containing that
`if` check) and either remove the redundant `=== ''` branch and delete the
corresponding baseline entry (lines referencing
src/Connectors/GithubConnector.php and the Strict comparison message), or if you
intentionally kept the explicit empty-string check for clarity, add an inline
comment next to `if (! $this->token || $this->token === '')` explaining why both
checks are present and keep the baseline suppression; choose one of these two
fixes and apply it consistently.


-
message: "#^Unknown parameter \\$reason in call to JordanPartridge\\\\GithubClient\\\\Exceptions\\\\NetworkException constructor\\.$#"
count: 1
path: src/Connectors/GithubConnector.php
Comment on lines +1 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Reconsider using a baseline for actual bugs.

PHPStan baselines are designed for:

  • Suppressing technical debt tracked for later resolution
  • Ignoring false positives from static analysis
  • Temporary suppressions during large refactorings

However, this baseline suppresses 3 critical bugs that will cause runtime failures:

  1. Instantiating a non-existent exception class (fatal error)
  2. Calling a constructor with wrong parameter names (TypeError)

These should be fixed immediately, not suppressed. Baseline files hide issues from static analysis, giving false confidence that the code is correct.

Recommendation: Fix the critical bugs first, then decide if any remaining suppressions (like the redundant comparison) warrant a baseline entry.

Would you like me to open a separate issue to track fixing these bugs?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@phpstan-baseline.neon` around lines 1 - 21, Remove the three suppressed
PHPStan errors from the baseline and fix the root issues in the GithubConnector:
ensure you reference/instantiate the correct exception class
(JordanPartridge\\GithubClient\\Exceptions\\ResourceNotFoundException) or import
the actual existing exception instead of a non-existent class; update the
NetworkException instantiation to call its actual constructor signature (pass
the required $message string and any valid $code/$previous params, removing the
unknown $reason parameter); and fix the redundant strict comparison (replace the
always-false === check with a proper !== '' or adjust the variable type/logic).
After changes, re-run PHPStan and only keep genuine false-positives in the
baseline.

6 changes: 4 additions & 2 deletions src/Auth/AuthenticationStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace JordanPartridge\GithubClient\Auth;

use JordanPartridge\GithubClient\Exceptions\AuthenticationException;

/**
* Interface for different GitHub authentication strategies.
*/
Expand All @@ -15,7 +17,7 @@ public function getAuthorizationHeader(): string;
/**
* Validate that the authentication credentials are properly configured.
*
* @throws \JordanPartridge\GithubClient\Exceptions\AuthenticationException
* @throws AuthenticationException
*/
public function validate(): void;

Expand All @@ -32,7 +34,7 @@ public function needsRefresh(): bool;
/**
* Refresh the authentication if needed.
*
* @throws \JordanPartridge\GithubClient\Exceptions\AuthenticationException
* @throws AuthenticationException
*/
public function refresh(): void;
}
3 changes: 2 additions & 1 deletion src/Commands/GithubClientCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Illuminate\Console\Command;
use JordanPartridge\GithubClient\Facades\Github;
use JordanPartridge\GithubClient\ValueObjects\Repo;
use Illuminate\Support\Str;

class GithubClientCommand extends Command
{
Expand Down Expand Up @@ -109,7 +110,7 @@ private function showCommits(): int
substr($commit->sha, 0, 8),
$commit->commit->author->name,
$commit->commit->author->date->format('Y-m-d H:i'),
\Illuminate\Support\Str::limit($commit->commit->message, 50),
Str::limit($commit->commit->message, 50),
];
}

Expand Down
6 changes: 6 additions & 0 deletions src/Github.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use JordanPartridge\GithubClient\Exceptions\NetworkException;
use JordanPartridge\GithubClient\Requests\RateLimit\Get;
use JordanPartridge\GithubClient\Resources\ActionsResource;
use JordanPartridge\GithubClient\Resources\ChecksResource;
use JordanPartridge\GithubClient\Resources\CommentsResource;
use JordanPartridge\GithubClient\Resources\CommitResource;
use JordanPartridge\GithubClient\Resources\FileResource;
Expand Down Expand Up @@ -57,6 +58,11 @@ public function actions(): ActionsResource
return new ActionsResource($this);
}

public function checks(): ChecksResource
{
return new ChecksResource($this);
}
Comment on lines +61 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check current facade annotations and verify checks() is missing
echo "=== Current facade annotations ==="
rg -A 10 '@method' src/Facades/Github.php

echo ""
echo "=== Checking if checks() annotation exists ==="
rg 'checks\(\)' src/Facades/Github.php || echo "checks() annotation NOT found - needs to be added"

Repository: jordanpartridge/github-client

Length of output: 932


Add missing @method annotation to the facade.

The new checks() method in src/Github.php requires a corresponding @method annotation in src/Facades/Github.php to enable IDE autocomplete and static analysis for Github::checks() calls.

Add this annotation with the other @method declarations:

`@method` static \JordanPartridge\GithubClient\Resources\ChecksResource checks()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Github.php` around lines 61 - 64, Add a matching `@method` annotation to
the Github facade so IDEs and static analyzers recognize the new checks()
method: in src/Facades/Github.php add the annotation "@method static
\JordanPartridge\GithubClient\Resources\ChecksResource checks()" alongside the
existing `@method` declarations for other resource methods, ensuring the facade
correctly documents the new checks() method defined in the Github class.


public function issues(): IssuesResource
{
return new IssuesResource($this);
Expand Down
32 changes: 32 additions & 0 deletions src/Requests/Checks/GetCheckRunsForRef.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace JordanPartridge\GithubClient\Requests\Checks;

use Saloon\Enums\Method;
use Saloon\Http\Request;

class GetCheckRunsForRef extends Request
{
protected Method $method = Method::GET;

public function __construct(
protected string $owner,
protected string $repo,
protected string $ref,
protected ?int $perPage = null,
protected ?int $page = null,
) {}

public function resolveEndpoint(): string
{
return "/repos/{$this->owner}/{$this->repo}/commits/{$this->ref}/check-runs";
}

protected function defaultQuery(): array
{
return array_filter([
'per_page' => $this->perPage,
'page' => $this->page,
], fn ($value) => $value !== null);
}
}
22 changes: 22 additions & 0 deletions src/Requests/Checks/GetCombinedStatus.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

namespace JordanPartridge\GithubClient\Requests\Checks;

use Saloon\Enums\Method;
use Saloon\Http\Request;

class GetCombinedStatus extends Request
{
protected Method $method = Method::GET;

public function __construct(
protected string $owner,
protected string $repo,
protected string $ref,
) {}

public function resolveEndpoint(): string
{
return "/repos/{$this->owner}/{$this->repo}/commits/{$this->ref}/status";
}
}
30 changes: 30 additions & 0 deletions src/Requests/Files/GetContents.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace JordanPartridge\GithubClient\Requests\Files;

use Saloon\Enums\Method;
use Saloon\Http\Request;

class GetContents extends Request
{
protected Method $method = Method::GET;

public function __construct(
protected string $owner,
protected string $repo,
protected string $path,
protected ?string $ref = null,
) {}

public function resolveEndpoint(): string
{
return "/repos/{$this->owner}/{$this->repo}/contents/{$this->path}";
}

protected function defaultQuery(): array
{
return array_filter([
'ref' => $this->ref,
], fn ($value) => $value !== null);
}
}
63 changes: 63 additions & 0 deletions src/Resources/ChecksResource.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

namespace JordanPartridge\GithubClient\Resources;

use JordanPartridge\GithubClient\Requests\Checks\GetCheckRunsForRef;
use JordanPartridge\GithubClient\Requests\Checks\GetCombinedStatus;

readonly class ChecksResource extends BaseResource
{
/**
* Get check runs for a commit reference (SHA, branch, or tag).
*
* @return array{total_count: int, check_runs: array<int, array<string, mixed>>}
*/
public function forRef(string $owner, string $repo, string $ref): array
{
$response = $this->github()->connector()->send(
new GetCheckRunsForRef($owner, $repo, $ref),
);

return $response->json();
}

/**
* Get the combined commit status for a reference.
*
* Returns the overall state (success, failure, pending) and individual statuses.
*
* @return array{state: string, statuses: array<int, array<string, mixed>>, total_count: int}
*/
public function combinedStatus(string $owner, string $repo, string $ref): array
{
$response = $this->github()->connector()->send(
new GetCombinedStatus($owner, $repo, $ref),
);

return $response->json();
}

/**
* Check if all checks pass for a given ref.
*/
public function allPassing(string $owner, string $repo, string $ref): bool
{
$checkRuns = $this->forRef($owner, $repo, $ref);

if (empty($checkRuns['check_runs'])) {
return true;
}

foreach ($checkRuns['check_runs'] as $run) {
if ($run['status'] !== 'completed') {
return false;
}

if ($run['conclusion'] !== 'success' && $run['conclusion'] !== 'skipped') {
return false;
}
}

return true;
}
Comment on lines +43 to +62
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Pagination gap may cause false positives when many check runs exist.

allPassing() only evaluates the first page of results from forRef(). If a repository has more check runs than GitHub's default page size (~30), failing checks on subsequent pages won't be detected, causing the method to incorrectly return true.

Consider either:

  1. Fetching all pages before evaluating, or
  2. At minimum, comparing total_count against the actual check_runs count to detect truncation
🐛 Option 1: Detect pagination truncation and throw
     public function allPassing(string $owner, string $repo, string $ref): bool
     {
         $checkRuns = $this->forRef($owner, $repo, $ref);

+        // Guard against incomplete results due to pagination
+        if ($checkRuns['total_count'] > count($checkRuns['check_runs'])) {
+            throw new \RuntimeException(
+                "Check runs response is paginated ({$checkRuns['total_count']} total, " .
+                count($checkRuns['check_runs']) . " returned). Cannot reliably determine all passing."
+            );
+        }
+
         if (empty($checkRuns['check_runs'])) {
             return true;
         }
🐛 Option 2: Fetch all pages (more complete solution)
     public function allPassing(string $owner, string $repo, string $ref): bool
     {
-        $checkRuns = $this->forRef($owner, $repo, $ref);
-
-        if (empty($checkRuns['check_runs'])) {
-            return true;
-        }
+        $page = 1;
+        $perPage = 100; // Max allowed by GitHub API
+
+        do {
+            $response = $this->forRef($owner, $repo, $ref, $perPage, $page);
+            $checkRuns = $response['check_runs'];
+
+            foreach ($checkRuns as $run) {
+                if ($run['status'] !== 'completed') {
+                    return false;
+                }
+
+                if ($run['conclusion'] !== 'success' && $run['conclusion'] !== 'skipped') {
+                    return false;
+                }
+            }
+
+            $page++;
+            $hasMore = count($checkRuns) === $perPage;
+        } while ($hasMore);

-        foreach ($checkRuns['check_runs'] as $run) {
-            if ($run['status'] !== 'completed') {
-                return false;
-            }
-
-            if ($run['conclusion'] !== 'success' && $run['conclusion'] !== 'skipped') {
-                return false;
-            }
-        }
-
         return true;
     }
📝 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
public function allPassing(string $owner, string $repo, string $ref): bool
{
$checkRuns = $this->forRef($owner, $repo, $ref);
if (empty($checkRuns['check_runs'])) {
return true;
}
foreach ($checkRuns['check_runs'] as $run) {
if ($run['status'] !== 'completed') {
return false;
}
if ($run['conclusion'] !== 'success' && $run['conclusion'] !== 'skipped') {
return false;
}
}
return true;
}
public function allPassing(string $owner, string $repo, string $ref): bool
{
$checkRuns = $this->forRef($owner, $repo, $ref);
// Guard against incomplete results due to pagination
if ($checkRuns['total_count'] > count($checkRuns['check_runs'])) {
throw new \RuntimeException(
"Check runs response is paginated ({$checkRuns['total_count']} total, " .
count($checkRuns['check_runs']) . " returned). Cannot reliably determine all passing."
);
}
if (empty($checkRuns['check_runs'])) {
return true;
}
foreach ($checkRuns['check_runs'] as $run) {
if ($run['status'] !== 'completed') {
return false;
}
if ($run['conclusion'] !== 'success' && $run['conclusion'] !== 'skipped') {
return false;
}
}
return true;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Resources/ChecksResource.php` around lines 43 - 62, allPassing() only
inspects the first page returned by forRef(), causing false positives when
results are paginated; update the logic to detect and handle pagination by
either (a) using the response's total_count vs count(check_runs) to detect
truncation and throw an error, or (b) iterating pages and aggregating all check
runs before evaluating. Concretely, modify either forRef() to fetch all pages
(looping with page/per_page until collected) or change allPassing() to call
forRef() repeatedly (page++/per_page) and merge all check_runs, then run the
existing status/conclusion checks on the full aggregated list; ensure you
reference forRef() and allPassing() when implementing the pagination loop or
truncation check.

}
29 changes: 29 additions & 0 deletions src/Resources/FileResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace JordanPartridge\GithubClient\Resources;

use InvalidArgumentException;
use JordanPartridge\GithubClient\Requests\Files\GetContents;
use JordanPartridge\GithubClient\Requests\Files\Index;
use JordanPartridge\GithubClient\ValueObjects\Repo;
use Saloon\Http\Response;
Expand All @@ -19,4 +20,32 @@ public function all(string $repo_name, string $commit_sha): Response

return $this->github()->connector()->send(new Index($repo->fullName(), $commit_sha));
}

/**
* Get the contents of a file at a specific ref (branch, tag, or SHA).
*
* @return array{name: string, path: string, sha: string, size: int, content: string, encoding: string}
*/
public function contents(string $owner, string $repo, string $path, ?string $ref = null): array
{
$response = $this->github()->connector()->send(
new GetContents($owner, $repo, $path, $ref),
);

return $response->json();
}

/**
* Get the decoded contents of a file at a specific ref.
*/
public function getContent(string $owner, string $repo, string $path, ?string $ref = null): string
{
$data = $this->contents($owner, $repo, $path, $ref);

if ($data['encoding'] === 'base64') {
return base64_decode($data['content']);
}

return $data['content'];
}
Comment on lines +41 to +50
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential runtime errors in getContent() method.

Several edge cases could cause issues:

  1. Missing key access: If $data['encoding'] or $data['content'] don't exist (e.g., when the path is a directory or submodule), this will throw an undefined array key error.

  2. base64_decode can return false: On malformed input, base64_decode() returns false, but the method declares a string return type.

  3. Directory responses: GitHub's API returns an array of objects when the path is a directory, not a single file object.

🛡️ Proposed fix with defensive checks
     public function getContent(string $owner, string $repo, string $path, ?string $ref = null): string
     {
         $data = $this->contents($owner, $repo, $path, $ref);
 
+        if (!isset($data['content'], $data['encoding'])) {
+            throw new \RuntimeException('Path does not point to a file or content is unavailable');
+        }
+
         if ($data['encoding'] === 'base64') {
-            return base64_decode($data['content']);
+            $decoded = base64_decode($data['content'], true);
+            if ($decoded === false) {
+                throw new \RuntimeException('Failed to decode base64 content');
+            }
+            return $decoded;
         }
 
         return $data['content'];
     }
📝 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
public function getContent(string $owner, string $repo, string $path, ?string $ref = null): string
{
$data = $this->contents($owner, $repo, $path, $ref);
if ($data['encoding'] === 'base64') {
return base64_decode($data['content']);
}
return $data['content'];
}
public function getContent(string $owner, string $repo, string $path, ?string $ref = null): string
{
$data = $this->contents($owner, $repo, $path, $ref);
if (!isset($data['content'], $data['encoding'])) {
throw new \RuntimeException('Path does not point to a file or content is unavailable');
}
if ($data['encoding'] === 'base64') {
$decoded = base64_decode($data['content'], true);
if ($decoded === false) {
throw new \RuntimeException('Failed to decode base64 content');
}
return $decoded;
}
return $data['content'];
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Resources/FileResource.php` around lines 41 - 50, The getContent method
can fail when contents(...) returns a directory array, is missing
'encoding'/'content' keys, or when base64_decode returns false; update
getContent to validate the response from contents($owner,$repo,$path,$ref): if
the return is an array of objects (directory) or doesn't contain string
'content'/'encoding' keys, throw or return a clear error/empty string, and when
'encoding' === 'base64' call base64_decode and check for false—if decoding
fails, throw a descriptive exception (or fallback to an empty string) so the
method always satisfies its string return type; reference the getContent method
and the contents(...) call to locate and implement these defensive checks and
error handling.

}
3 changes: 2 additions & 1 deletion src/Resources/PullRequestResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use JordanPartridge\GithubClient\Requests\Pulls\Reviews;
use JordanPartridge\GithubClient\Requests\Pulls\Update;
use JordanPartridge\GithubClient\Requests\Pulls\UpdateComment;
use Illuminate\Support\Collection;

readonly class PullRequestResource extends BaseResource
{
Expand Down Expand Up @@ -99,7 +100,7 @@ public function reviews(
string $owner,
string $repo,
int $number,
): \Illuminate\Support\Collection {
): Collection {
$response = $this->github()->connector()->send(new Reviews("{$owner}/{$repo}", $number));

return $response->dto();
Expand Down
3 changes: 2 additions & 1 deletion tests/ArchTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php

use JordanPartridge\GithubClient\Resources\BaseResource;
use Saloon\Http\Request;

describe('General', function () {
it('will not use debugging functions', function () {
Expand All @@ -15,6 +16,6 @@

describe('Requests', function () {
arch('requests extend the base resource', function () {
expect('JordanPartridge\GithubClient\Requests')->toExtend(\Saloon\Http\Request::class);
expect('JordanPartridge\GithubClient\Requests')->toExtend(Request::class);
});
});
5 changes: 3 additions & 2 deletions tests/AuthenticationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use JordanPartridge\GithubClient\Facades\Github;
use Saloon\Http\Faking\MockClient;
use Saloon\Http\Faking\MockResponse;
use JordanPartridge\GithubClient\Data\Repos\RepoData;

describe('Authentication improvements', function () {
it('allows unauthenticated requests for public repositories', function () {
Expand Down Expand Up @@ -168,12 +169,12 @@
$connector->withMockClient($mockClient);

// Create Github instance with unauthenticated connector
$github = new \JordanPartridge\GithubClient\Github($connector);
$github = new JordanPartridge\GithubClient\Github($connector);

// Should be able to get public repo without auth
$repo = $github->getRepo('owner/public-repo');

expect($repo)->toBeInstanceOf(\JordanPartridge\GithubClient\Data\Repos\RepoData::class)
expect($repo)->toBeInstanceOf(RepoData::class)
->and($repo->name)->toBe('public-repo')
->and($repo->private)->toBeFalse();
});
Expand Down
2 changes: 1 addition & 1 deletion tests/CommitResourceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
]);

Github::connector()->withMockClient($mockClient);
$this->resource = new CommitResource(app(\JordanPartridge\GithubClient\Github::class));
$this->resource = new CommitResource(app(JordanPartridge\GithubClient\Github::class));
});

it('can fetch all commits for a repository', function () {
Expand Down
3 changes: 2 additions & 1 deletion tests/DTOPatternTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use JordanPartridge\GithubClient\Data\Pulls\PullRequestDetailDTO;
use JordanPartridge\GithubClient\Data\Pulls\PullRequestDTOFactory;
use JordanPartridge\GithubClient\Data\Pulls\PullRequestSummaryDTO;
use JordanPartridge\GithubClient\Data\Pulls\PullRequestDTO;

describe('DTO Pattern: Summary vs Detail DTOs', function () {
beforeEach(function () {
Expand Down Expand Up @@ -175,7 +176,7 @@
describe('Backward Compatibility', function () {
it('maintains compatibility with existing PullRequestDTO usage', function () {
// The original PullRequestDTO still works exactly as before
$originalDto = \JordanPartridge\GithubClient\Data\Pulls\PullRequestDTO::fromApiResponse($this->detailResponseData);
$originalDto = PullRequestDTO::fromApiResponse($this->detailResponseData);

expect($originalDto->number)->toBe(47)
->and($originalDto->comments)->toBe(1)
Expand Down
3 changes: 2 additions & 1 deletion tests/PullRequestsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use JordanPartridge\GithubClient\Facades\Github;
use Saloon\Http\Faking\MockClient;
use Saloon\Http\Faking\MockResponse;
use Illuminate\Support\Collection;

beforeEach(function () {
config(['github-client.token' => 'fake-token']);
Expand Down Expand Up @@ -162,7 +163,7 @@
$reviews = Github::pullRequests()->reviews('test', 'repo', 1);

expect($reviews)
->toBeInstanceOf(\Illuminate\Support\Collection::class)
->toBeInstanceOf(Collection::class)
->and($reviews->isEmpty())->toBeFalse()
->and($reviews->first())
->toBeInstanceOf(PullRequestReviewDTO::class)
Expand Down
Loading
Loading