Skip to content

Fix GitLab::getEvent() field/action parity with Gitea's shape#119

Closed
HarshMN2345 wants to merge 6 commits into
utopia-php:mainfrom
HarshMN2345:fix/gitlab-event-parity
Closed

Fix GitLab::getEvent() field/action parity with Gitea's shape#119
HarshMN2345 wants to merge 6 commits into
utopia-php:mainfrom
HarshMN2345:fix/gitlab-event-parity

Conversation

@HarshMN2345

@HarshMN2345 HarshMN2345 commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

GitLab's getEvent() (added in #118, feat/appwrite-provider-parity) returns push/merge_request event data in a shape that doesn't match Gitea's or GitHub's -- consumers relying on the common field contract (e.g. Appwrite's VCS integration) would silently drop every event.

Verified against real GitLab webhook payload structure:

  • Missing repositoryId entirely -- any DB lookup keyed on it finds nothing.
  • Non-standard key names: name/commitAuthor/commitMessage/commitUrl instead of repositoryName/headCommitAuthorName/headCommitMessage/headCommitUrl (matching Gitea's/GitHub's convention).
  • Merge request action passed through raw GitLab values (open/reopen/update/close) instead of the GitHub/Gitea convention (opened/reopened/synchronize/closed) consumers already branch on. Also maps GitLab's distinct merge action to closed, since a merged MR is done either way.
  • Also adds repositoryUrl, branchUrl, affectedFiles, branch create/delete detection, and cross-project (fork) detection for external MRs -- all present in Gitea's return shape but missing from GitLab's.

While integrating against a real GitLab OAuth app end-to-end (not just mocked HTTP), three further real bugs surfaced -- all fixed here too:

  • Wrong auth header for OAuth2 tokens. The adapter authenticated every request with PRIVATE-TOKEN: <token> (and ?private_token= for presigned URLs), which is only valid for GitLab Personal Access Tokens. Our integration uses OAuth2 access tokens, which GitLab requires as Authorization: Bearer <token>. Every API call was failing with 401 "Bad credentials" regardless of token freshness.
  • searchRepositories() dropped private repos in personal namespaces. The personal-namespace fallback called GET /users/{username}/projects, which GitLab only returns public projects for, even for the authenticated user's own account. Switched to GET /projects?membership=true (every project -- public and private -- the token's user is a member of) with client-side filtering by namespace.path.
  • searchRepositories() return shape didn't match GitHub/Gitea's items/total contract. It returned a bare indexed array instead of ['items' => [...], 'total' => N], breaking any consumer destructuring the common shape. total is now taken from the X-Total response header (or the filtered count, when the personal-namespace client-side filter is in effect, since the header reflects all namespaces in that case).

Test plan

  • Updated testGetEventPush/testGetEventPullRequest to assert the corrected shape
  • New testGetEventPullRequestActionMapping covers all 5 native action values
  • New testGetEventPullRequestDetectsExternal covers fork/cross-project MR detection
  • New testGetEventPushDetectsBranchCreated/testGetEventPushDetectsBranchDeleted
  • Updated testSearchRepositories/testSearchRepositoriesWithSearch to assert against the new items/total shape
  • composer format/composer check (PHPStan level 8) clean
  • Verified live end-to-end against a real GitLab OAuth app and repository (auth header + searchRepositories fixes were only caught this way -- unit tests use mocked HTTP and couldn't have caught either)

Verified against real GitLab webhook payloads: push/MR events were
missing repositoryId (breaking any DB lookup keyed on it), used
non-standard key names (name/commitAuthor/commitMessage/commitUrl
instead of repositoryName/headCommitAuthorName/headCommitMessage/
headCommitUrl), and passed merge_request action through raw
(open/reopen/update/close) instead of normalizing to the GitHub/Gitea
convention (opened/reopened/synchronize/closed) consumers already key
off of. Also adds repositoryUrl, branchUrl, affectedFiles, branch
create/delete detection, and cross-project (fork) detection for MRs,
matching Gitea's return shape field-for-field.
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes GitLab::getEvent() so its push and merge-request return shapes match Gitea's established contract, and simultaneously migrates all API calls from the PRIVATE-TOKEN header to Authorization: Bearer while updating searchRepositories() to return {items, total} (matching the GitHub adapter's shape).

  • getEvent() field parity: Renames name/commitAuthor/commitMessage/commitUrl to repositoryName/headCommitAuthorName/headCommitMessage/headCommitUrl; adds repositoryId, repositoryUrl, branchUrl, affectedFiles, branchCreated, branchDeleted, and external; maps GitLab's native MR actions (open/reopen/update/close/merge) to the GitHub/Gitea verbs consumers already branch on.
  • Auth header migration: All PRIVATE-TOKEN request headers are replaced with Authorization: Bearer, and the archive download URL switches from private_token to access_token.
  • Test coverage: New dedicated tests cover branch-create detection, branch-delete detection, all five MR action mappings, and cross-project (fork) MR detection.

Confidence Score: 5/5

Safe to merge — the changes are well-scoped field renames and additions with matching test coverage for every new code path.

The getEvent() return shapes align precisely with Gitea's adapter, the action-mapping constant is correct, the zero-SHA sentinel for branch create/delete is the standard GitLab convention, and searchRepositories() now matches the GitHub adapter's {items, total} structure. No logic paths were left untested by the new test methods.

No files require special attention.

Important Files Changed

Filename Overview
src/VCS/Adapter/Git/GitLab.php Reworks getEvent() to produce a return shape that matches Gitea's adapter exactly (repositoryId, repositoryName, repositoryUrl, headCommit* keys, affectedFiles, branchCreated/branchDeleted, action mapping); also migrates all API calls from PRIVATE-TOKEN header to Authorization: Bearer, updates archive URL parameter to access_token, and changes searchRepositories() to return {items, total} matching GitHub's shape.
tests/VCS/Adapter/GitLabTest.php Updates getEvent push/MR tests to assert the corrected field names; adds testGetEventPushDetectsBranchCreated, testGetEventPushDetectsBranchDeleted, testGetEventPullRequestActionMapping, and testGetEventPullRequestDetectsExternal; updates searchRepositories assertions for the new {items, total} return shape.

Reviews (5): Last reviewed commit: "Fix searchRepositories() return shape to..." | Re-trigger Greptile

Comment thread tests/VCS/Adapter/GitLabTest.php
Comment thread src/VCS/Adapter/Git/GitLab.php
Comment thread src/VCS/Adapter/Git/GitLab.php
HarshMN2345 added a commit to appwrite/appwrite that referenced this pull request Jul 16, 2026
GitLab sends x-gitlab-event as 'Push Hook'/'Merge Request Hook', not
'push'/'pull_request' -- every real webhook was silently falling
through to the default no-op. Caught by Greptile review.

Blocked on utopia-php/vcs#119 (the field/action-name parity fix for
GitLab::getEvent()) merging and releasing before this integration is
fully correct end-to-end -- verified against the patched library
locally, but composer.json still points at the published 4.4.2 since
a personal-fork dependency can't ship in this PR.
…l default

- Added dedicated tests for branchCreated/branchDeleted sentinel paths.
- Documented the external=false fallback as an intentional safe default.
- Reverted a pullRequestNumber strval() cast: Gitea/GitHub both return
  the raw int-or-empty-string from getEvent() too, so casting only
  GitLab would trade one inconsistency for another. Leaving all three
  adapters consistent with each other for now; a real fix needs to
  touch all three, which is a separate, broader change.
…spaces

GET /users/:id/projects only ever returns *public* projects, even for
the authenticated user's own account -- confirmed live against a real
GitLab account with private repos, which came back empty. Falls back
to GET /projects?membership=true instead (every project, public and
private, the token's user is a member of), filtered client-side to
the requested owner's namespace since that endpoint isn't scoped to
one owner. The group-namespace path is untouched, since it was
already correctly scoped by GitLab itself.

Note: the existing testSearchRepositories/testSearchRepositoriesWithSearch
tests only cover a public repo, which is exactly why this didn't get
caught -- worth adding a private-repo case as a follow-up.
…th2 tokens

Every API call used the PRIVATE-TOKEN header (and ?private_token= for
the presigned archive URL), which GitLab only honors for Personal
Access Tokens. Our entire integration is OAuth2-based (authorization
code flow), which GitLab requires via Authorization: Bearer <token>
(or ?access_token= for URL-embedded tokens) -- confirmed live: a
freshly-issued OAuth2 access token was rejected with 401 'Bad
credentials' on every single request, immediately after being minted,
which ruled out expiry/staleness and pointed at the auth mechanism
itself.
…otal contract

GitLab's version returned a bare indexed array; GitHub and Gitea both
return ['items' => ..., 'total' => ...] (confirmed by reading both).
Consumers destructure that shape directly (e.g. Appwrite's
Installations/Repositories/XList.php), so this crashed with a
TypeError the moment a real request reached it. Total count comes
from GitLab's X-Total response header for the successfully-scoped
group/personal-namespace fetch; falls back to the filtered count when
client-side namespace filtering is in effect, since X-Total in that
case reflects every namespace the token can see, not just the
requested owner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant