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
61 changes: 58 additions & 3 deletions src/VCS/Adapter/Git/Gitea.php
Original file line number Diff line number Diff line change
Expand Up @@ -364,16 +364,71 @@ public function listBranches(string $owner, string $repositoryName): array
throw new Exception("Not implemented yet");
}

/**
* Get details of a commit using commit hash
*
* @param string $owner Owner name of the repository
* @param string $repositoryName Name of the repository
* @param string $commitHash SHA of the commit
* @return array<mixed> Details of the commit
*/
public function getCommit(string $owner, string $repositoryName, string $commitHash): array
{
throw new Exception("Not implemented yet");
$url = "/repos/{$owner}/{$repositoryName}/git/commits/{$commitHash}";

Choose a reason for hiding this comment

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

P1 Badge Use the repo commit endpoint for getCommit

getCommit calls /git/commits/{sha} but then parses the response as if it were the repository commit schema (body['commit']['author']['name'], body['commit']['message']). In Gitea, the git-data commit payload does not carry those nested fields, so valid commits will silently return fallback values like 'Unknown' and 'No message' instead of real metadata, making this new endpoint functionally incorrect for normal inputs.

Useful? React with 👍 / 👎.


$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "token $this->accessToken"]);

$statusCode = $response['headers']['status-code'] ?? 0;
if ($statusCode >= 400) {
throw new Exception("Commit not found or inaccessible");
}

$body = $response['body'] ?? [];
$commit = $body['commit'] ?? [];
$author = $body['author'] ?? [];

return [
'commitAuthor' => $commit['author']['name'] ?? 'Unknown',
'commitMessage' => $commit['message'] ?? 'No message',
'commitAuthorAvatar' => $author['avatar_url'] ?? '',
'commitAuthorUrl' => $author['html_url'] ?? '',
'commitHash' => $body['sha'] ?? '',
'commitUrl' => $body['html_url'] ?? '',
];
Comment on lines +377 to +397
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

getCommit() is calling the /git/commits/{sha} endpoint but then parses the response as if it came from the /commits/{sha} endpoint (expects commit.*, author.*, and html_url). This can lead to silently returning placeholder values (e.g. Unknown / No message / empty URLs) even when the API call succeeds. Align the endpoint and response parsing with getLatestCommit()/GitHub adapter (or adjust parsing to match the actual /git/commits schema) and fail fast if required fields are missing.

Copilot uses AI. Check for mistakes.
}

/**
* Get latest commit of a branch
*
* @param string $owner Owner name of the repository
* @param string $repositoryName Name of the repository
* @param string $branch Name of the branch
* @return array<mixed> Details of the commit
*/
public function getLatestCommit(string $owner, string $repositoryName, string $branch): array
{
throw new Exception("Not implemented yet");
}
$url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1";
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Encode branch when building the query string.

$branch is interpolated directly into the URL, so query-significant characters can change request semantics. Build the query with encoding to avoid malformed/altered requests.

🔧 Proposed fix
-        $url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1";
+        $query = http_build_query([
+            'sha' => $branch,
+            'limit' => 1,
+        ]);
+        $url = "/repos/{$owner}/{$repositoryName}/commits?{$query}";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/VCS/Adapter/Git/Gitea.php` at line 410, The branch value is interpolated
directly into the commit URL and can contain query-significant characters;
update the URL construction in Gitea.php so the branch is URL-encoded (e.g.,
replace "{$branch}" with rawurlencode($branch)) or build the query with
http_build_query(['sha'=>$branch,'limit'=>1]) and append that to
"/repos/{$owner}/{$repositoryName}/commits" so $url is safe; modify the
assignment to $url accordingly (referencing the $url variable and $branch).

Choose a reason for hiding this comment

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

P2 Badge Encode branch names before building commit query

The branch is interpolated directly into the query string, so valid ref names containing query-significant characters (for example & or +) will be misparsed by HTTP query decoding and request the wrong SHA filter. This causes getLatestCommit to fail or return commits from an unintended ref for those branch names.

Useful? React with 👍 / 👎.

Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

getLatestCommit() interpolates $branch directly into the query string. If branch names contain characters like &, ?, or #, this can break the request (or allow query parameter injection). URL-encode the branch name (or pass it as a GET param to call() so it’s encoded via http_build_query).

Suggested change
$url = "/repos/{$owner}/{$repositoryName}/commits?sha={$branch}&limit=1";
$encodedBranch = rawurlencode($branch);
$url = "/repos/{$owner}/{$repositoryName}/commits?sha={$encodedBranch}&limit=1";

Copilot uses AI. Check for mistakes.

$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "token $this->accessToken"]);

$statusCode = $response['headers']['status-code'] ?? 0;
if ($statusCode >= 400 || empty($response['body'][0])) {
throw new Exception("Latest commit response is missing required information.");
}

$body = $response['body'][0];
$commit = $body['commit'] ?? [];
$author = $body['author'] ?? [];

return [
'commitAuthor' => $commit['author']['name'] ?? 'Unknown',
'commitMessage' => $commit['message'] ?? 'No message',
'commitHash' => $body['sha'] ?? '',
'commitUrl' => $body['html_url'] ?? '',
'commitAuthorAvatar' => $author['avatar_url'] ?? '',
'commitAuthorUrl' => $author['html_url'] ?? '',
];
}
public function updateCommitStatus(string $repositoryName, string $commitHash, string $owner, string $state, string $description = '', string $target_url = '', string $context = ''): void
{
throw new Exception("Not implemented yet");
Expand Down
83 changes: 81 additions & 2 deletions tests/VCS/Adapter/GiteaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,91 @@ public function testUpdateComment(): void

public function testGetCommit(): void
{
$this->markTestSkipped('Will be implemented in follow-up PR');
$repositoryName = 'test-get-commit-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);

// Create a file to generate a commit
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test Commit');

// Get the latest commit to get its SHA
$latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main');
$commitHash = $latestCommit['commitHash'];

// Now test getCommit with that SHA
$result = $this->vcsAdapter->getCommit(self::$owner, $repositoryName, $commitHash);

$this->assertIsArray($result);
$this->assertArrayHasKey('commitHash', $result);
$this->assertArrayHasKey('commitMessage', $result);
$this->assertArrayHasKey('commitAuthor', $result);
$this->assertArrayHasKey('commitUrl', $result);
$this->assertArrayHasKey('commitAuthorAvatar', $result);
$this->assertArrayHasKey('commitAuthorUrl', $result);

$this->assertSame($commitHash, $result['commitHash']);
$this->assertNotEmpty($result['commitMessage']);

Comment on lines +389 to +391
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The assertions in testGetCommit() are currently satisfied by the adapter’s placeholder defaults (e.g. commitMessage being No message). To ensure the endpoint actually returns real commit data, assert that commitMessage contains the commit message used in createFile() (and/or that commitAuthor is not Unknown, and commitUrl is non-empty).

Copilot uses AI. Check for mistakes.
$this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
}

public function testGetLatestCommit(): void
{
$this->markTestSkipped('Will be implemented in follow-up PR');
$repositoryName = 'test-get-latest-commit-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);

// Create files to generate commits
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'test.txt', 'test content');

$result = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main');

$this->assertIsArray($result);
$this->assertArrayHasKey('commitHash', $result);
$this->assertArrayHasKey('commitMessage', $result);
$this->assertArrayHasKey('commitAuthor', $result);
$this->assertArrayHasKey('commitUrl', $result);
$this->assertArrayHasKey('commitAuthorAvatar', $result);
$this->assertArrayHasKey('commitAuthorUrl', $result);

$this->assertNotEmpty($result['commitHash']);
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

In testGetLatestCommit(), assertNotEmpty($result['commitAuthor']) will still pass if the adapter returns the fallback value Unknown. Consider asserting on the expected commit message (e.g. default Add file or a provided custom message) and/or that commitUrl is non-empty to validate that parsing is correct.

Suggested change
$this->assertNotEmpty($result['commitHash']);
$this->assertNotEmpty($result['commitHash']);
$this->assertNotEmpty($result['commitMessage']);
$this->assertNotEmpty($result['commitUrl']);

Copilot uses AI. Check for mistakes.
$this->assertNotEmpty($result['commitAuthor']);

$this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
}

public function testGetCommitWithInvalidSha(): void
{
$repositoryName = 'test-get-commit-invalid-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');

$this->expectException(\Exception::class);
$this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345');
}
Comment on lines +421 to +428
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

testGetCommitWithInvalidSha() creates a repository but never deletes it because the test exits via the expected exception. This can leak repositories across test runs and cause flakiness. Wrap the call in a try/finally (or equivalent) so deleteRepository() always runs.

Copilot uses AI. Check for mistakes.

public function testGetLatestCommitWithInvalidBranch(): void
{
$repositoryName = 'test-get-latest-commit-invalid-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');

$this->expectException(\Exception::class);
$this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch');
}
Comment on lines +420 to +438
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Ensure cleanup runs in exception-path tests.

Both negative tests create repositories but never delete them after the expected exception. Wrap assertions in try/finally so cleanup always executes.

🧹 Proposed fix
     public function testGetCommitWithInvalidSha(): void
     {
         $repositoryName = 'test-get-commit-invalid-' . \uniqid();
         $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
         $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');

-        $this->expectException(\Exception::class);
-        $this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345');
+        try {
+            $this->expectException(\Exception::class);
+            $this->vcsAdapter->getCommit(self::$owner, $repositoryName, 'invalid-sha-12345');
+        } finally {
+            $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+        }
     }

     public function testGetLatestCommitWithInvalidBranch(): void
     {
         $repositoryName = 'test-get-latest-commit-invalid-' . \uniqid();
         $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
         $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test');

-        $this->expectException(\Exception::class);
-        $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch');
+        try {
+            $this->expectException(\Exception::class);
+            $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'non-existing-branch');
+        } finally {
+            $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` around lines 420 - 438, The two tests
testGetCommitWithInvalidSha and testGetLatestCommitWithInvalidBranch create
repositories via $this->vcsAdapter->createRepository(...) but never delete them
on the exception path; wrap the assertions that call
$this->vcsAdapter->getCommit(...) and $this->vcsAdapter->getLatestCommit(...) in
try/finally blocks that always call the cleanup method (e.g.
$this->vcsAdapter->deleteRepository(self::$owner, $repositoryName)) in the
finally so the repositoryName created in each test is removed regardless of the
thrown exception.

Comment on lines +431 to +438
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

testGetLatestCommitWithInvalidBranch() creates a repository but never deletes it because the test exits via the expected exception. Ensure repository cleanup runs even when the exception is thrown (e.g., try/finally around the call).

Copilot uses AI. Check for mistakes.

public function testGetCommitVerifyMessageContent(): void
{
$repositoryName = 'test-commit-message-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);

$customMessage = 'Custom commit message';
$this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test', $customMessage);

$latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main');

$this->assertStringContainsString($customMessage, $latestCommit['commitMessage']);

$this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
}

public function testGetEvent(): void
Expand Down