Skip to content

Conversation

@brendan-kellam
Copy link
Contributor

@brendan-kellam brendan-kellam commented Oct 30, 2025

This PR adds support for GitLab permission syncing.

Summary by CodeRabbit

  • New Features
    • Added GitLab support for repository and user permission synchronization
    • Introduced GitLab environment configuration settings
    • Enhanced permission sync job logging for improved traceability
    • Conditional OAuth scopes for GitLab SSO when permission syncing is enabled

@github-actions

This comment has been minimized.

@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Walkthrough

This PR extends permission syncing to support GitLab alongside GitHub. It adds GitLab client factory functions, integrates GitLab API calls into repo and user permission syncers with per-job logging, adds a GitLab base URL environment variable, updates OAuth scope handling for GitLab SSO to conditionally include read_api, and refactors job scheduling from transactional to non-transactional creation.

Changes

Cohort / File(s) Summary
Configuration & constants
packages/backend/src/constants.ts, packages/backend/src/env.ts
Added 'gitlab' to PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES constant; added AUTH_EE_GITLAB_BASE_URL environment variable with default "https://gitlab.com"
GitLab integration utilities
packages/backend/src/gitlab.ts
Added createGitLabFromPersonalAccessToken and createGitLabFromOAuthToken factory functions; added getProjectMembers and getProjectsForAuthenticatedUser utility functions with retry and error handling; refactored getGitLabReposFromConfig to use the new factory
Permission syncers
packages/backend/src/ee/repoPermissionSyncer.ts, packages/backend/src/ee/userPermissionSyncer.ts
Added GitLab branch logic to fetch project members/projects and map to user IDs; introduced per-job logger factory for improved traceability; changed job scheduling from transactional to non-transactional creation followed by enqueueing; enhanced error handling and logging in job completion/failure callbacks
SSO scope handling
packages/web/src/ee/features/sso/sso.ts
Updated GitLab OAuth scope to conditionally include "read_api" when permission-syncing experiments are enabled and entitlement is present
Documentation cleanup
packages/backend/src/repoCompileUtils.ts
Removed todo comment about 'internal' visibility consideration

Sequence Diagram(s)

sequenceDiagram
    participant Job
    participant Syncer
    participant GitLabAPI
    participant DB
    participant Logger

    rect rgba(100, 150, 200, 0.2)
    Note over Syncer: GitLab Permission Sync Flow
    Job->>Syncer: runJob(repoId, userId)
    Syncer->>Logger: createJobLogger(jobId)
    Syncer->>GitLabAPI: createGitLabFromOAuthToken(token)
    GitLabAPI-->>Syncer: Gitlab client
    Syncer->>GitLabAPI: getProjectMembers(projectId)
    GitLabAPI-->>Syncer: members array
    Syncer->>DB: mapMembersToUserIds()
    DB-->>Syncer: userId[]
    Syncer-->>Job: return mapped IDs
    end

    rect rgba(150, 200, 100, 0.2)
    Note over Syncer: GitHub Permission Sync Flow (existing)
    Job->>Syncer: runJob(repoId, userId)
    Syncer->>GitLabAPI: Fetch via GitHub API
    GitLabAPI-->>Syncer: collaborators
    Syncer-->>Job: return mapped IDs
    end

    rect rgba(200, 100, 100, 0.2)
    Note over Syncer: Job Scheduling (changed)
    Job->>DB: createJob() [non-transactional]
    DB-->>Job: jobRecord
    Job->>Syncer: enqueueJob(jobRecord)
    Syncer-->>Job: success
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • repoPermissionSyncer.ts and userPermissionSyncer.ts: Heterogeneous changes including new GitLab integration logic, per-job logger refactoring, transaction removal from scheduling, and error handling updates across multiple code paths—each requires separate reasoning
  • gitlab.ts: New factory functions and utility methods with retry patterns and error handling need verification for consistency and correctness
  • Transactional scheduling change: The removal of transactional job creation requires careful review of race condition handling and consistency guarantees
  • Per-job logging pattern: Verify consistent implementation across both syncers and proper logger lifecycle management

Possibly related PRs

Suggested labels

sourcebot-team

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "feat(ee): GitLab permission syncing" directly aligns with the primary objective of the changeset, which is to add GitLab permission syncing support to the system. The changes across multiple files—including updates to permission syncers, new GitLab utilities, environment variables, and SSO scopes—all serve this central feature. The title is concise, uses conventional commit formatting, and clearly communicates the main change in a way that would be readily understood by someone scanning the git history. The title is neither vague nor misleading.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bkellam/gitlab_permission_syncing

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@brendan-kellam
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 384aa9e and 05c3d13.

📒 Files selected for processing (7)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/ee/repoPermissionSyncer.ts (6 hunks)
  • packages/backend/src/ee/userPermissionSyncer.ts (6 hunks)
  • packages/backend/src/env.ts (1 hunks)
  • packages/backend/src/gitlab.ts (3 hunks)
  • packages/backend/src/repoCompileUtils.ts (0 hunks)
  • packages/web/src/ee/features/sso/sso.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/backend/src/repoCompileUtils.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (.cursor/rules/style.mdc)

Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.

Files:

  • packages/backend/src/env.ts
  • packages/backend/src/ee/repoPermissionSyncer.ts
  • packages/web/src/ee/features/sso/sso.ts
  • packages/backend/src/constants.ts
  • packages/backend/src/ee/userPermissionSyncer.ts
  • packages/backend/src/gitlab.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

msukkari
msukkari previously approved these changes Oct 30, 2025
@brendan-kellam brendan-kellam merged commit 4899c9f into main Oct 30, 2025
8 checks passed
@brendan-kellam brendan-kellam deleted the bkellam/gitlab_permission_syncing branch October 30, 2025 18:08
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.

3 participants