feat: Add Gitea commit endpoints (getCommit, getLatestCommit)#66
Merged
Meldiron merged 7 commits intoutopia-php:mainfrom Mar 5, 2026
Merged
Conversation
WalkthroughTwo public methods were added to the Gitea VCS adapter: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- 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).
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d54667d93b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Adds commit-retrieval support to the Gitea VCS adapter to align with the existing GitHub adapter, along with new integration tests that exercise commit fetching and error paths.
Changes:
- Implement
getCommit(owner, repo, commitHash)in the Gitea adapter. - Implement
getLatestCommit(owner, repo, branch)in the Gitea adapter. - Add PHPUnit integration tests covering commit retrieval and invalid SHA/branch handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/VCS/Adapter/Git/Gitea.php |
Adds Gitea implementations for getCommit and getLatestCommit. |
tests/VCS/Adapter/GiteaTest.php |
Adds integration tests validating commit retrieval and error handling scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/VCS/Adapter/GiteaTest.php (1)
366-390: Always guarantee repository cleanup in happy-path commit tests.These tests delete repositories only at the end. If an assertion or API call fails earlier, cleanup is skipped and can leak test resources across runs. Wrap test bodies in
try/finallyand keepdeleteRepository(...)infinally.♻️ Suggested pattern
public function testGetCommit(): void { $repositoryName = 'test-get-commit-' . \uniqid(); $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false); - $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test Commit'); - - $latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main'); - $commitHash = $latestCommit['commitHash']; - - $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']); - - $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName); + try { + $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test Commit'); + $latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main'); + $commitHash = $latestCommit['commitHash']; + $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']); + } finally { + $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName); + } }Also applies to: 392-414, 444-457
🤖 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 366 - 390, Wrap the body of the testGetCommit method in a try/finally so deleteRepository(self::$owner, $repositoryName) always runs; specifically, after creating $repositoryName and before any assertions/calls (i.e., around calls to createFile, getLatestCommit, getCommit and assertions), place the assertions and API calls in the try block and move the existing deleteRepository call into the finally block to guarantee cleanup even if an assertion or API call throws. Apply the same try/finally pattern to the other tests referenced (lines noted: the tests around 392-414 and 444-457) that currently call deleteRepository only at the end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 366-390: Wrap the body of the testGetCommit method in a
try/finally so deleteRepository(self::$owner, $repositoryName) always runs;
specifically, after creating $repositoryName and before any assertions/calls
(i.e., around calls to createFile, getLatestCommit, getCommit and assertions),
place the assertions and API calls in the try block and move the existing
deleteRepository call into the finally block to guarantee cleanup even if an
assertion or API call throws. Apply the same try/finally pattern to the other
tests referenced (lines noted: the tests around 392-414 and 444-457) that
currently call deleteRepository only at the end.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/VCS/Adapter/Git/Gitea.phptests/VCS/Adapter/GiteaTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/VCS/Adapter/Git/Gitea.php
Meldiron
added
test
Meldiron
reviewed
Mar 3, 2026
| $this->assertArrayHasKey('commitAuthorUrl', $result); | ||
|
|
||
| $this->assertNotEmpty($result['commitHash']); | ||
| $this->assertNotEmpty($result['commitAuthor']); |
Contributor
There was a problem hiding this comment.
Same comment as above - assertHasKey and assertEmpty are most minimal assertions - everytime we can assert value, we should.
For example here I would assertSame for commit hash, message, and author (author only if possible, not sure)
Meldiron
reviewed
Mar 3, 2026
tests/VCS/Adapter/GiteaTest.php
Outdated
Comment on lines
+444
to
+456
| 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); |
Contributor
There was a problem hiding this comment.
I think we can remove this test, and ensure message (as well as author maybe) in other tests.
Also assertStringContainsString is a bit loose, I would go for assertSame.
--
I think you did this after our discussion about separating tests. Over-separation can be problem too - what helps me decide in most scenarios is to always look from perspective of action I am doing in the test, not from perspective of assertions.
Here, action in same as one in testGetLatestCommit - preparing a file with a message in commit. So I would keep it as part of same test.
If there was some extra setup (like more repositories, or more commits), or the flow had multiple steps (like ensuring it's 2nd createFile, not 1st), thats when separate test would make sense to me.
Open to discuss this further if you have strong opinion on this. I am going off of my experience with testing, so I might be missing something too
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/VCS/Adapter/Git/Gitea.php (2)
422-424: Split HTTP-failure vs payload-shape errors.Line 422 currently throws the same message for both transport/status failures and malformed/empty payloads, which makes debugging harder.
🛠️ Proposed improvement
- if ($statusCode >= 400 || empty($response['body'][0])) { - throw new Exception("Latest commit response is missing required information."); - } + if ($statusCode >= 400) { + throw new Exception("Failed to fetch latest commit for branch '{$branch}': HTTP {$statusCode}"); + } + if (empty($response['body'][0])) { + throw new Exception("Latest commit response is missing required information."); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/VCS/Adapter/Git/Gitea.php` around lines 422 - 424, The current check conflates HTTP transport/status errors and malformed payloads; update the logic in Gitea (the method using $statusCode and $response) to throw two distinct exceptions: if ($statusCode >= 400) throw an Exception that includes the HTTP status code and response body/details (e.g. "Failed to fetch latest commit: HTTP {status} - {body}"), otherwise if (empty($response['body'][0])) throw a different Exception clearly stating the payload is missing required commit info (e.g. "Latest commit response missing required information"), so callers can distinguish transport errors from payload-shape errors.
391-391: Remove unused local$committer.Line 391 assigns
$committerbut it is never used in the return statement, which adds noise and triggers static-analysis warnings.♻️ Proposed cleanup
- $committer = $body['committer'] ?? [];🤖 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 391, The local variable $committer in src/VCS/Adapter/Git/Gitea.php is assigned but never used; remove the unnecessary assignment "$committer = $body['committer'] ?? [];" (or, if committer data is actually required, include it in the return structure where other $body fields are returned) to eliminate dead code and static-analysis warnings—look for the assignment in the Git/Gitea adapter function that processes the incoming $body and either delete that line or wire $committer into the returned payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 422-424: The current check conflates HTTP transport/status errors
and malformed payloads; update the logic in Gitea (the method using $statusCode
and $response) to throw two distinct exceptions: if ($statusCode >= 400) throw
an Exception that includes the HTTP status code and response body/details (e.g.
"Failed to fetch latest commit: HTTP {status} - {body}"), otherwise if
(empty($response['body'][0])) throw a different Exception clearly stating the
payload is missing required commit info (e.g. "Latest commit response missing
required information"), so callers can distinguish transport errors from
payload-shape errors.
- Line 391: The local variable $committer in src/VCS/Adapter/Git/Gitea.php is
assigned but never used; remove the unnecessary assignment "$committer =
$body['committer'] ?? [];" (or, if committer data is actually required, include
it in the return structure where other $body fields are returned) to eliminate
dead code and static-analysis warnings—look for the assignment in the Git/Gitea
adapter function that processes the incoming $body and either delete that line
or wire $committer into the returned payload.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/VCS/Adapter/Git/Gitea.phptests/VCS/Adapter/GiteaTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/VCS/Adapter/GiteaTest.php
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 368-393: Wrap repository creation and test logic in a try/finally
(or implement per-test tearDown) so deleteRepository(self::$owner,
$repositoryName) always runs even if assertions fail; update testGetCommit and
testGetLatestCommit to save $repositoryName after createRepository and perform
deleteRepository in the finally block (or register a cleanup callback) to ensure
cleanup on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6da621a8-272b-4331-8ae4-0160c7dacaa1
📒 Files selected for processing (2)
src/VCS/Adapter/Git/Gitea.phptests/VCS/Adapter/GiteaTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/VCS/Adapter/Git/Gitea.php
Comment on lines
+368
to
393
| $repositoryName = 'test-get-commit-' . \uniqid(); | ||
| $this->vcsAdapter->createRepository(self::$owner, $repositoryName, false); | ||
|
|
||
| $customMessage = 'Test commit message'; | ||
| $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test Commit', $customMessage); | ||
|
|
||
| $latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main'); | ||
| $commitHash = $latestCommit['commitHash']; | ||
|
|
||
| $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->assertStringContainsString($customMessage, $result['commitMessage']); | ||
| $this->assertNotEmpty($result['commitAuthor']); | ||
| $this->assertNotEmpty($result['commitUrl']); | ||
|
|
||
| $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName); | ||
| } |
There was a problem hiding this comment.
Ensure repo cleanup runs on failing happy-path tests too.
In testGetCommit and testGetLatestCommit, cleanup only runs at the end. If any assertion/API call fails earlier, repositories are left behind and can pollute later runs.
Proposed resilient cleanup pattern
public function testGetCommit(): void
{
$repositoryName = 'test-get-commit-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
-
- $customMessage = 'Test commit message';
- $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test Commit', $customMessage);
-
- $latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main');
- $commitHash = $latestCommit['commitHash'];
-
- $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->assertStringContainsString($customMessage, $result['commitMessage']);
- $this->assertNotEmpty($result['commitAuthor']);
- $this->assertNotEmpty($result['commitUrl']);
-
- $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+ try {
+ $customMessage = 'Test commit message';
+ $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test Commit', $customMessage);
+
+ $latestCommit = $this->vcsAdapter->getLatestCommit(self::$owner, $repositoryName, 'main');
+ $commitHash = $latestCommit['commitHash'];
+
+ $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->assertStringContainsString($customMessage, $result['commitMessage']);
+ $this->assertNotEmpty($result['commitAuthor']);
+ $this->assertNotEmpty($result['commitUrl']);
+ } finally {
+ $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+ }
}
public function testGetLatestCommit(): void
{
$repositoryName = 'test-get-latest-commit-' . \uniqid();
$this->vcsAdapter->createRepository(self::$owner, $repositoryName, false);
-
- $firstMessage = 'First commit';
- $secondMessage = 'Second commit';
- $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test', $firstMessage);
- $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'test.txt', 'test content', $secondMessage);
-
- $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']);
- $this->assertStringContainsString($secondMessage, $result['commitMessage']);
- $this->assertNotEmpty($result['commitAuthor']);
- $this->assertNotEmpty($result['commitUrl']);
-
- $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+ try {
+ $firstMessage = 'First commit';
+ $secondMessage = 'Second commit';
+ $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'README.md', '# Test', $firstMessage);
+ $this->vcsAdapter->createFile(self::$owner, $repositoryName, 'test.txt', 'test content', $secondMessage);
+
+ $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']);
+ $this->assertStringContainsString($secondMessage, $result['commitMessage']);
+ $this->assertNotEmpty($result['commitAuthor']);
+ $this->assertNotEmpty($result['commitUrl']);
+ } finally {
+ $this->vcsAdapter->deleteRepository(self::$owner, $repositoryName);
+ }
}Also applies to: 397-421
🤖 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 368 - 393, Wrap repository
creation and test logic in a try/finally (or implement per-test tearDown) so
deleteRepository(self::$owner, $repositoryName) always runs even if assertions
fail; update testGetCommit and testGetLatestCommit to save $repositoryName after
createRepository and perform deleteRepository in the finally block (or register
a cleanup callback) to ensure cleanup on failure.
Meldiron
approved these changes
Mar 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Implements commit-related endpoints for Gitea adapter to match GitHub functionality.
Methods Implemented
getCommit(owner, repo, commitHash)- Fetches details of a specific commit by SHAgetLatestCommit(owner, repo, branch)- Fetches the most recent commit of a branchTests Added
testGetCommit- Verifies commit details retrievaltestGetLatestCommit- Verifies latest commit fetchingtestGetCommitWithInvalidSha- Tests error handling for invalid commit SHAtestGetLatestCommitWithInvalidBranch- Tests error handling for non-existent branchtestGetCommitVerifyMessageContent- Verifies custom commit messages are preservedAll tests follow the established pattern with proper setup/teardown and edge case coverage.
Summary by CodeRabbit
New Features
Tests