Conversation
WalkthroughAdds a new Gitea VCS adapter class (src/VCS/Adapter/Git/Gitea.php) implementing repository and organization creation and many unimplemented stubs; adds tests for Gitea (tests/VCS/Adapter/GiteaTest.php) with most tests skipped; updates adapter abstract signatures to accept optional parameters (appId, accessToken, refreshToken) and propagates those signature changes to GitHub adapter and tests; introduces docker-compose services for gitea and gitea-bootstrap and a shared volume; renames a test abstract method (testgetEvent → testGetEvent) and adjusts GitHub test accordingly. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1cc30ea29
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@docker-compose.yml`:
- Line 50: The command that creates a user via `gitea admin user create`
contains a real-looking email literal `jayesh@gmail.com`; replace it with a
placeholder reserved domain like `jayesh@example.com` in that invocation (the
shell command string using `su git -c 'gitea admin user create --username jayesh
--password jayesh123 --email ... --admin --must-change-password=false'`) so no
personal email is leaked in the repo.
- Around line 48-53: Make the bootstrap command idempotent and remove PII:
ensure the user creation line (the su git -c 'gitea admin user create --username
jayesh ...') uses a non-real email (e.g., jayesh@example.com) and make the
access token logic around su git -c 'gitea admin user generate-access-token
--username jayesh --token-name jayesh-token --scopes all --raw' robust by
handling the token-already-exists case—either delete an existing token first or
fall back to reading an existing token if generation fails (so the echo to
/data/gitea/token.txt always happens), and keep the overall shell pipeline
idempotent instead of letting token generation break the && chain.
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 77-87: createRepository currently ignores the $owner parameter and
always posts to /user/repos; update createRepository to choose the endpoint
based on $owner — if $owner is null/empty or equals the authenticated user use
"/user/repos", otherwise use "/orgs/{$owner}/repos" (i.e., set $url accordingly
before calling $this->call); ensure the Authorization header and payload remain
the same and keep returning $response['body'] ?? [] so callers creating org
repos hit the correct API path.
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 116-119: Rename the test method testgetEvent to testGetEvent to
match the project's PascalCase test naming convention; update the method
declaration in the GiteaTest class (public function testgetEvent(): void) to
public function testGetEvent(): void and adjust any internal references or
annotations that mention testgetEvent so PHPUnit and callers use the new name.
- Around line 33-40: The setupGitea method silently leaves self::$accessToken
empty if /data/gitea/token.txt is missing or unreadable, causing cryptic 401/403
failures; update setupGitea to check file_exists and is_readable and that
trim(file_get_contents($tokenFile)) is non-empty, and if any check fails call
$this->markTestSkipped (or self::markTestSkipped) with a clear message about the
missing/invalid Gitea token file so tests fail fast and informatively while
still setting self::$accessToken when valid.
- Line 44: System::getEnv('GITEA_TEST_OWNER') can return null and is passed to
createRepository(), which expects a string; modify the test to validate or
supply a fallback before calling createRepository(): call
System::getEnv('GITEA_TEST_OWNER'), if it returns null either
assertNonNull/throw a clear exception or mark the test skipped (e.g.,
$this->markTestSkipped) and otherwise cast/assign the value to a string variable
to pass to createRepository(); ensure you reference System::getEnv and
createRepository in the change so the variable passed into createRepository() is
guaranteed to be a string.
🧹 Nitpick comments (4)
src/VCS/Adapter/Git/Gitea.php (2)
52-58: Overloading GitHub-specific parameter names for different semantics is a maintainability concern.The docblock explains the mapping well, but this parameter reuse (
$installationId→ access token,$privateKey→ refresh token,$githubAppId→ URL) will confuse future developers. This is an inherited limitation of the abstract interface, but worth noting for a potential follow-up to make the baseAdapterinterface more VCS-agnostic.
11-31: Remove redundant property and constructor re-declarations inherited from parent.The parent
Gitclass already declares$endpoint,$accessToken,$cache, and$headers, and its constructor already assigns$this->cache. Re-declaring them here is redundant. Only$refreshTokenand$giteaUrlare truly new to this class.Proposed fix
class Gitea extends Git { - protected string $endpoint = 'http://gitea:3000/api/v1'; - - protected string $accessToken; - protected string $refreshToken; protected string $giteaUrl; - protected Cache $cache; - - /** - * Global Headers - * - * `@var` array<string, string> - */ - protected $headers = ['content-type' => 'application/json']; - - public function __construct(Cache $cache) + public function __construct(Cache $cache) { - $this->cache = $cache; + parent::__construct($cache); + $this->endpoint = 'http://gitea:3000/api/v1'; }tests/VCS/Adapter/GiteaTest.php (2)
42-54: Created test repositories are never cleaned up.Each test run creates a new
test-repo-<timestamp>repository that persists in the Gitea instance. WhendeleteRepository()is implemented, add teardown logic. For now, consider adding a// TODO: clean up test repos once deleteRepository is implementedcomment to track this.
22-30:setUp()creates adapter directly instead of usingcreateVCSAdapter().Line 29 instantiates
Giteadirectly, but you already definecreateVCSAdapter()(presumably required byBase). Consider using it to stay consistent with the base class pattern:- $this->vcsAdapter = new Gitea(new Cache(new None())); + $this->vcsAdapter = $this->createVCSAdapter();
- Update Adapter interface with accessToken and refreshToken params - Rename githubAppId to appId for clarity - Add setEndpoint method to Gitea adapter - Use environment variables for Gitea bootstrap credentials - Add .env.example with defaults - Skip all unimplemented tests to pass CI
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/VCS/Adapter.php (2)
54-59:⚠️ Potential issue | 🟡 MinorStale docblock:
$githubAppId→$appId.The
@paramon line 56 still references$githubAppId, but the parameter was renamed to$appId.Proposed fix
* `@param` string $installationId * `@param` string $privateKey - * `@param` string $githubAppId + * `@param` string $appId + * `@param` string $accessToken + * `@param` string $refreshToken * `@return` void
64-68:⚠️ Potential issue | 🟡 MinorRename
$githubAppIdto$appIdingenerateAccessTokenfor consistency withinitializeVariablesand the GitHub adapter implementation.The abstract method uses
$githubAppIdbutinitializeVariablesuses$appId. The GitHub adapter already overrides with$appId, while Gitea still uses$githubAppId. Update the abstract signature in Adapter.php (line 68 and docblock line 65) and the Gitea implementation (line 75 and docblock) to standardize on$appId.Changes needed
In Adapter.php:
- * `@param` string $githubAppId + * `@param` string $appId * `@return` void */ - abstract protected function generateAccessToken(string $privateKey, string $githubAppId): void; + abstract protected function generateAccessToken(string $privateKey, string $appId): void;In Gitea.php:
- protected function generateAccessToken(string $privateKey, string $githubAppId): void + protected function generateAccessToken(string $privateKey, string $appId): void
🤖 Fix all issues with AI agents
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 49-57: The docblock on the Gitea class initialization is stale:
remove the old parameter-overloading notes and replace them with an accurate
description of the current constructor signature (document $accessToken and
$refreshToken as dedicated OAuth2 tokens and the parameter used for the Gitea
instance URL), and keep a short note that Gitea uses OAuth2 rather than GitHub
App installations; update the docblock for the Gitea class/constructor (e.g.,
Gitea::__construct or the Gitea initialization method) to list the exact
parameter names and purposes instead of the outdated
"$installationId/$privateKey/$githubAppId" mapping.
🧹 Nitpick comments (1)
.env.example (1)
1-3: Add a trailing newline and sort keys alphabetically.Static analysis (dotenv-linter) flags missing trailing blank line and unordered keys. Minor, but good practice for consistency.
Proposed fix
-GITEA_ADMIN_USERNAME= -GITEA_ADMIN_PASSWORD= -GITEA_ADMIN_EMAIL= +GITEA_ADMIN_EMAIL= +GITEA_ADMIN_PASSWORD= +GITEA_ADMIN_USERNAME= +
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/VCS/Adapter/GiteaTest.php (1)
22-43: Test setup is well-structured for incremental adapter development.Static properties ensure one-time initialization across test methods. The on-demand organization creation in
setUpavoids reliance on pre-existing Gitea state.One note: if
createOrganizationsilently returns''(as discussed in the Gitea adapter review),self::$ownerwill be an empty string, and the conditionalif (empty(self::$owner))will re-attempt org creation on every test. Consider asserting the result is non-empty.Proposed improvement
if (empty(self::$owner)) { $orgName = 'test-org-' . \uniqid(); self::$owner = $this->vcsAdapter->createOrganization($orgName); + $this->assertNotEmpty(self::$owner, 'Failed to create test organization'); }🤖 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 22 - 43, The test setUp can loop repeatedly if GiteaAdapter::createOrganization returns an empty string; after calling $this->vcsAdapter->createOrganization(...) assign the result to self::$owner and immediately assert it's non-empty (e.g., use $this->assertNotEmpty(self::$owner) or throw a RuntimeException) so failures are explicit rather than silently retrying; update the setUp method around the createOrganization call to validate the returned value from createOrganization and fail the test on empty results.src/VCS/Adapter/Git/Gitea.php (1)
11-19:$giteaUrlproperty is set but never read.
setEndpoint()writes to both$this->giteaUrland$this->endpoint, but$giteaUrlis never referenced elsewhere in the class. If it's reserved for future use (e.g., constructing clone URLs), consider adding a comment; otherwise remove it to keep the surface clean.🤖 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 11 - 19, The class property $giteaUrl is assigned in setEndpoint() but never read elsewhere (dead surface area); remove the unused protected string $giteaUrl declaration from class Gitea (or if it is intended for future use, add an inline comment near setEndpoint() and the property explaining the planned purpose), and update setEndpoint() to only set $this->endpoint (or retain the comment) so there are no unused properties.
🤖 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`:
- Around line 100-110: The createOrganization method currently returns an empty
string when the API call doesn't provide body.name; change it to detect error
responses from the HTTP call (e.g., non-2xx status or missing body.name) inside
createOrganization and throw a descriptive exception (e.g., RuntimeException or
an existing adapter-specific exception) instead of returning ''. Use the
existing call(...) result (inspect its status and body keys) to determine
failure and include the API status/message in the exception so callers like
setUp in GiteaTest (which assigns to self::$owner) cannot receive an empty owner
string.
- Around line 88-98: The endpoint used in createRepository is incorrect: change
the URL built in the createRepository method from "/org/{$owner}/repos" to the
correct plural form "/orgs/{$owner}/repos" so the Gitea API call (using
self::METHOD_POST and $this->call with the access token) targets POST
/orgs/{org}/repos and returns the repository body as before.
In `@src/VCS/Adapter/Git/GitHub.php`:
- Around line 59-65: initializeVariables may pass a null $appId into
generateAccessToken which uses it as the JWT "iss" claim, producing an invalid
token; update initializeVariables (and related callers) to validate $appId
before calling generateAccessToken: if $appId is null or empty, either throw a
clear exception or obtain/derive a valid appId (or refuse to generate a token),
and add a guard that prevents calling generateAccessToken with a null $appId so
the JWT "iss" claim is always a non-null string.
---
Duplicate comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 49-58: Update the stale docblock on the Gitea class/constructor:
remove the outdated parameter-overloading notes and replace them with accurate
descriptions for the current signature (explicitly document $accessToken and
$refreshToken and the actual purpose of $appId, $installationId, $privateKey if
still present). Locate the Gitea class/constructor in Gitea.php (symbols: class
Gitea, __construct) and edit the docblock so each `@param` matches the real
parameter name and purpose instead of the old “used to pass” mapping.
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 45-55: setupGitea currently leaves self::$accessToken empty if
/data/gitea/token.txt is missing which causes initializeVariables to throw a
generic error; update setupGitea to fail-fast: if file_exists($tokenFile) is
false, throw a clear RuntimeException (or use PHPUnit markTestSkipped) with a
descriptive message about the missing Gitea token file, and also throw/handle a
distinct exception when file_get_contents($tokenFile) returns false so
self::$accessToken is never left empty.
---
Nitpick comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 11-19: The class property $giteaUrl is assigned in setEndpoint()
but never read elsewhere (dead surface area); remove the unused protected string
$giteaUrl declaration from class Gitea (or if it is intended for future use, add
an inline comment near setEndpoint() and the property explaining the planned
purpose), and update setEndpoint() to only set $this->endpoint (or retain the
comment) so there are no unused properties.
In `@tests/VCS/Adapter/GiteaTest.php`:
- Around line 22-43: The test setUp can loop repeatedly if
GiteaAdapter::createOrganization returns an empty string; after calling
$this->vcsAdapter->createOrganization(...) assign the result to self::$owner and
immediately assert it's non-empty (e.g., use $this->assertNotEmpty(self::$owner)
or throw a RuntimeException) so failures are explicit rather than silently
retrying; update the setUp method around the createOrganization call to validate
the returned value from createOrganization and fail the test on empty results.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/VCS/Adapter/Git/Gitea.php (2)
11-17:$giteaUrlis redundant and risks an uninitialized-property error.
$giteaUrlis only written and read insidesetEndpoint()as an intermediate to build$endpoint. It's never used elsewhere, making it dead state. If anything ever reads it beforesetEndpoint()is called, PHP 8 will throw a typed-property access error since it has no default.Consider dropping
$giteaUrland inlining the logic:Proposed fix
- protected string $giteaUrl; - ... public function setEndpoint(string $endpoint): void { - $this->giteaUrl = rtrim($endpoint, '/'); - $this->endpoint = $this->giteaUrl . '/api/v1'; + $this->endpoint = rtrim($endpoint, '/') . '/api/v1'; }🤖 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 11 - 17, Remove the dead/unsafe property $giteaUrl and inline its use in setEndpoint(): stop declaring protected string $giteaUrl; and update the setEndpoint() implementation to build and assign the final value directly to $endpoint (using the incoming host/port/path values or their concatenation) so no intermediate property is required; ensure $endpoint remains the single source of truth and that setEndpoint() still validates/normalizes the URL before assigning to protected string $endpoint.
112-224: Consider using\BadMethodCallExceptionfor stubs instead of genericException.All stubs throw
new Exception("Not implemented yet"). PHP's built-in\BadMethodCallExceptionmore precisely communicates that the method exists but isn't supported yet, and allows callers to catch it distinctly from operational errors.🤖 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 112 - 224, The stub methods in Gitea.php (e.g., searchRepositories, getRepository, getRepositoryName, getRepositoryTree, listRepositoryLanguages, getRepositoryContent, listRepositoryContents, deleteRepository, createComment, getComment, updateComment, getUser, getOwnerName, getPullRequest, getPullRequestFromBranch, listBranches, getCommit, getLatestCommit, updateCommitStatus, generateCloneCommand, getEvent, validateWebhookEvent) currently throw new Exception("Not implemented yet"); change these to throw new \BadMethodCallException("Not implemented yet") so callers can distinguish unimplemented API stubs from operational errors; update each method's throw to use the fully-qualified \BadMethodCallException class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 88-98: The createRepository method currently always posts to
/orgs/{$owner}/repos which 404s for personal repos; update createRepository to
choose the endpoint based on owner: if $owner is empty (or indicates the
authenticated user) use '/user/repos', otherwise use "/orgs/{$owner}/repos";
keep the same Authorization header and body payload, and ensure the method
signature and returned $response['body'] handling remain unchanged.
- Around line 100-110: The createOrganization method currently returns an empty
string when the API fails; update createOrganization to detect non-success
responses from $this->call (e.g., check $response['status'] or presence of an
error payload) and surface the failure instead of returning ''. Specifically,
after $response = $this->call(...), if the status is not 2xx or
$response['body']['name'] is missing, throw a descriptive exception (or return
an error object) including the HTTP status and any message from
$response['body'] so callers of createOrganization get actionable failure
information; ensure the thrown exception type is consistent with other VCS
adapter errors and update any tests or callers that expect a string.
- Around line 49-59: The docblock for the Gitea class/method in Gitea.php is
stale and still claims $installationId/$privateKey are overloaded to carry
tokens; update the docblock to match the current method signature by removing or
correcting the lines that say "$installationId is used to pass the access token"
and "$privateKey is used to pass the refresh token" and instead document the
actual parameters (e.g., describe $appId as the Gitea instance URL, and that
$accessToken and $refreshToken are the OAuth tokens). Keep the note that Gitea
uses OAuth2 and ensure parameter descriptions reference the class/method name
(Gitea::__construct or relevant initializer) so future readers find the correct
implementation.
---
Nitpick comments:
In `@src/VCS/Adapter/Git/Gitea.php`:
- Around line 11-17: Remove the dead/unsafe property $giteaUrl and inline its
use in setEndpoint(): stop declaring protected string $giteaUrl; and update the
setEndpoint() implementation to build and assign the final value directly to
$endpoint (using the incoming host/port/path values or their concatenation) so
no intermediate property is required; ensure $endpoint remains the single source
of truth and that setEndpoint() still validates/normalizes the URL before
assigning to protected string $endpoint.
- Around line 112-224: The stub methods in Gitea.php (e.g., searchRepositories,
getRepository, getRepositoryName, getRepositoryTree, listRepositoryLanguages,
getRepositoryContent, listRepositoryContents, deleteRepository, createComment,
getComment, updateComment, getUser, getOwnerName, getPullRequest,
getPullRequestFromBranch, listBranches, getCommit, getLatestCommit,
updateCommitStatus, generateCloneCommand, getEvent, validateWebhookEvent)
currently throw new Exception("Not implemented yet"); change these to throw new
\BadMethodCallException("Not implemented yet") so callers can distinguish
unimplemented API stubs from operational errors; update each method's throw to
use the fully-qualified \BadMethodCallException class.
Add Gitea VCS Adapter - Initial Implementation
Summary
This PR adds initial support for Gitea as a VCS adapter, starting with basic repository creation functionality. This is the first of multiple PRs that will incrementally add full Gitea support.
What's Included
1. Gitea Adapter Class
src/VCS/Adapter/Git/Gitea.phpcreateRepository()method2. Test Suite
tests/VCS/Adapter/GiteaTest.phptestCreateRepository- validates repository creationSummary by CodeRabbit
New Features
Tests