Skip to content

Conversation

brendan-kellam
Copy link
Contributor

@brendan-kellam brendan-kellam commented Sep 17, 2025

This PR adds GitHub permission syncing behind a experimental flag. From the docs:

Permission syncing allows you to sync Access Permission Lists (ACLs) from a code host to Sourcebot. When configured, users signed into Sourcebot (via the code host’s OAuth provider) will only be able to access repositories that they have access to on the code host. Practically, this means:

  • Code Search results will only include repositories that the user has access to.
  • Code navigation results will only include repositories that the user has access to.
  • Ask Sourcebot (and the underlying LLM) will only have access to repositories that the user has access to.
  • File browsing is scoped to the repositories that the user has access to.

It can be enabled by setting EXPERIMENT_EE_PERMISSION_SYNC_ENABLED=true. As this is a EE feature, a license key is required.

How it works 🧠 dump

  • In the database, we maintain a UserToRepoPermission join table between User and Repo. This can be thought of as an edge in a graph.
  • If there is an edge between a given User and Repo, then it indicates that the user has access to that given repo.
  • At query time, in prisma.ts we use a Prisma client extension to extend any queries that operate on the Repo table and add an additional where statement that enforces a edge must exist between the repo and current user. This statement looks like:
OR: [
    // Only include repos that are permitted to the user,
    {
        permittedUsers: {
            some: {
                userId,
            }
        }
    },
    // or are public.
    {
        isPublic: true,
    }
]
  • When I refer to "current user", I mean the user that is making the authenticated request for the given action (authentication context). E.g., if the signed in user is bob, and the client calls the getRepos() action, then the current user is bob.
  • This extended Prisma client is exposed on withAuthV2 and withOptionalAuthV2. My mental model here is that the prisma client exposed via this middleware is scoped to the authentication context and can be used "safely" (i.e., we don't need to worry about always having a where clause on all Repo queries).
  • Once we migrate over to v2 (next up), we should mark the global prisma object as __unsafe to explicitly denote that this user auth context scoping is not applied. There will still be parts of the code where this unscoped client will be needed (e.g., initialize code), but most everything else can use the scoped client (all actions, api routes, etc.)
  • <-- how is the UserToRepoPermission table hydrated? -->
  • On the worker side, this PR adds two new schedulers: UserPermissionSyncer and RepoPermissionSyncer. These workers periodically reach out to the code host to determine the repos <--> user mapping. These schedulers both build this mapping in parallel (and overlap), but in different directions:
    • Repo driven: RepoPermissionSyncer fetches a list of all users that can view a given repository. For GitHub, it uses the
      /repos/{owner}/{repo}/collaborators
      api. The authentication token in the config.json is used to do this.
    • User driven: UserPermissionSyncer fetches a list of all repositories that a given user has access to. For GitHub, uses /user/repos. The access token associated with the User's GitHub account (stored in the Account table and obtained when the user signed in with the OAuth GitHub app) is used to authenticate.
  • These workers process these as asynchronous jobs that are queued using Redis and BullMQ (similar to repository indexing and connection syncing).
  • The default sync interval is 24 hours for both syncers (e.g., a given User will have a User Driven permission sync job run once every 24 hours).
  • <-- Why do we need two syncers? Wouldn't it be sufficient to just have a single direction? -->
  • Consider the following scenarios:
    1. Only repo driven: assume all repos have finished syncing now. If a user signs up afterwards, no UserToRepoPermission entries linking the user and the repos will exist, so the user will not see any repos until the next repo driven sync (24 hours).
    2. Only user driven: it's the inverse. Assume all users have finished syncing now. If a repo is added, no UserToRepoPermission entries linking the repo and the users will exist, so the repo will not be discovered until the next user driven sync (24 hours).
  • There are probably other schemes out there to solve this issue, but this was the most straight forward at the time.

Summary by CodeRabbit

  • New Features

    • Experimental permission syncing (Enterprise): sync repository access from GitHub to Sourcebot. Enable via EXPERIMENT_EE_PERMISSION_SYNC_ENABLED; defaults to 24h user/repo sync intervals. Currently supports GitHub.
    • Improved repository visibility handling with explicit public/private flag.
    • Simplified app and API calls for browsing, files, images, and search—no domain headers/params required.
  • Documentation

    • New “Permission syncing” guide, configuration and environment variable updates, navigation additions, reusable experimental warning, and Agents overview update (adds “Review Agent” card).
  • Chores

    • Added dev script: dev:prisma:generate.

Copy link

coderabbitai bot commented Sep 17, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "[experimental] feat(ee): GitHub permission syncing" is concise and accurately describes the primary change: an experimental enterprise feature that adds GitHub permission syncing. It follows conventional-commit style, specifies scope (ee) and intent (permission syncing), and contains no noisy file lists or vague wording.

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 brendan-kellam changed the title [wip][experimental] GitHub permission syncing [experimental] feat(ee): GitHub permission syncing Sep 20, 2025
@brendan-kellam brendan-kellam marked this pull request as ready for review September 20, 2025 05:01

This comment has been minimized.

@brendan-kellam
Copy link
Contributor Author

@coderabbitai review

Copy link

coderabbitai bot commented Sep 20, 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: 28

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
packages/web/src/app/api/(server)/source/route.ts (1)

18-24: Signature update looks good; add JSON-parse guard to avoid 500s on malformed bodies.

request.json() can throw; return a validation error instead of crashing the route.

 export const POST = async (request: NextRequest) => {
-    const body = await request.json();
+    let body: unknown;
+    try {
+        body = await request.json();
+    } catch {
+        return serviceErrorResponse(schemaValidationError("Invalid JSON body"));
+    }
     const parsed = await fileSourceRequestSchema.safeParseAsync(body);
packages/web/src/features/chat/tools.ts (2)

183-191: Domain removal from search is correct; fix limit default mismatch (doc says 100, schema defaults to 10).

Unify the default and usage.

-        limit: z.number().default(10).describe("Maximum number of matches to return (default: 100)"),
+        limit: z.number().default(100).describe("Maximum number of matches to return (default: 100)"),
 ...
-        const response = await search({
+        const response = await search({
             query,
-            matches: limit ?? 100,
+            matches: limit,

111-118: Remove domain argument from remaining getFileSource call sites

Found callers that still pass SINGLE_TENANT_ORG_DOMAIN — update these to the new getFileSource signature (remove the domain arg).

  • packages/web/src/features/chat/tools.ts:40
  • packages/web/src/features/chat/tools.ts:78
packages/backend/src/connectionManager.ts (2)

27-46: Expose scheduler lifecycle; avoid orphaned intervals.

startScheduler() creates an interval but the class doesn’t retain/clear it in dispose(). Either store the handle and clear it on dispose, or document that callers must manage it.

Apply this diff to track and clear the interval:

 export class ConnectionManager {
   private worker: Worker;
   private queue: Queue<JobPayload>;
   private logger = createLogger('connection-manager');
+  private schedulerHandle?: ReturnType<typeof setInterval>;
@@
-    public startScheduler() {
-        this.logger.debug('Starting scheduler');
-        return setInterval(async () => {
+    public startScheduler() {
+        this.logger.debug('Starting scheduler');
+        this.schedulerHandle = setInterval(async () => {
             const thresholdDate = new Date(Date.now() - this.settings.resyncConnectionIntervalMs);
@@
-        }, this.settings.resyncConnectionPollingIntervalMs);
+        }, this.settings.resyncConnectionPollingIntervalMs);
+        return this.schedulerHandle;
     }
@@
     public dispose() {
-        this.worker.close();
-        this.queue.close();
+        if (this.schedulerHandle) clearInterval(this.schedulerHandle);
+        this.worker.close();
+        this.queue.close();
     }

48-70: Idempotent queueing; dedupe by jobId.

Without a stable BullMQ jobId, multi-instance schedulers can enqueue duplicates. Use jobId: String(connection.id) (or a composite id) to dedupe.

-            await this.queue.add('connectionSyncJob', {
+            await this.queue.add('connectionSyncJob', {
                 connectionId: connection.id,
                 connectionName: connection.name,
                 orgId: connection.orgId,
                 config: connectionConfig,
             }, {
+                jobId: String(connection.id),
                 removeOnComplete: env.REDIS_REMOVE_ON_COMPLETE,
                 removeOnFail: env.REDIS_REMOVE_ON_FAIL,
             });
packages/backend/src/repoCompileUtils.ts (1)

658-664: Azure DevOps: cloneUrl should use remoteUrl, not webUrl.

Using the web URL will break cloning.

-            cloneUrl: webUrl,
+            cloneUrl: repo.remoteUrl,
🧹 Nitpick comments (51)
package.json (1)

9-9: Optional: chain generate into dev to avoid stale client.

migrate:dev usually runs generate, but chaining is a cheap guard when only generator/types change.

-    "dev": "yarn dev:prisma:migrate:dev && npm-run-all --print-label --parallel dev:zoekt dev:backend dev:web watch:mcp watch:schemas",
+    "dev": "yarn dev:prisma:migrate:dev && yarn dev:prisma:generate && npm-run-all --print-label --parallel dev:zoekt dev:backend dev:web watch:mcp watch:schemas",
LICENSE.md (2)

5-5: Add trailing period for consistency.

Apply this diff:

- - All content that resides under the "ee/", "packages/web/src/ee/", "packages/backend/src/ee/", and "packages/shared/src/ee/" directories of this repository, if these directories exist, is licensed under the license defined in "ee/LICENSE"
+ - All content that resides under the "ee/", "packages/web/src/ee/", "packages/backend/src/ee/", and "packages/shared/src/ee/" directories of this repository, if these directories exist, is licensed under the license defined in "ee/LICENSE".

5-5: Consider future‑proofing path pattern.

If additional package workspaces add EE code, using a wildcard reduces churn in LICENSE.md.

Apply this diff (optional):

- - All content that resides under the "ee/", "packages/web/src/ee/", "packages/backend/src/ee/", and "packages/shared/src/ee/" directories of this repository, if these directories exist, is licensed under the license defined in "ee/LICENSE".
+ - All content that resides under the "ee/" directory and any "packages/*/src/ee/" subdirectories of this repository, if these directories exist, is licensed under the license defined in "ee/LICENSE".
packages/web/src/app/api/[domain]/repos/[repoId]/image/route.ts (2)

10-10: Domain param now ignored—verify global uniqueness and caching behavior

  • Please confirm repoId is globally unique across domains. If not, validate params.domain against the repo’s canonical domain and 404/redirect on mismatch to avoid cross-tenant ambiguity and cache confusion.
  • If repo images are permission-scoped under the new sync, consider avoiding shared caching and add auth-aware headers.

Suggested header hardening (if images can be private):

 return new Response(result, {
   headers: {
     'Content-Type': 'image/png',
-    'Cache-Control': 'public, max-age=3600',
+    // Avoid shared caching for potentially permission-scoped resources
+    'Cache-Control': 'private, max-age=600',
+    'Vary': 'Authorization, Cookie',
   },
 });

Also applies to: 22-26


10-12: Use radix with parseInt

Prevents edge cases with leading zeros.

-const repoIdNum = parseInt(repoId);
+const repoIdNum = parseInt(repoId, 10);
packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (2)

37-38: Avoid cross-domain collisions in recent-files storage key.

Include domain to prevent mixing histories for identically named repos across providers.

-    const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(`recentlyOpenedFiles-${repoName}`, []);
+    const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(`recentlyOpenedFiles-${domain}-${repoName}`, []);

71-80: Minor perf nit: avoid repeated toLowerCase in the loop.

-        const matches = files
-            .map((file) => {
-                return {
-                    file,
-                    matchIndex: file.path.toLowerCase().indexOf(searchQuery.toLowerCase()),
-                }
-            })
+        const q = searchQuery.toLowerCase();
+        const matches = files
+            .map((file) => {
+                const pathLower = file.path.toLowerCase();
+                return { file, matchIndex: pathLower.indexOf(q) };
+            })
             .filter(({ matchIndex }) => {
                 return matchIndex !== -1;
             });
CHANGELOG.md (1)

11-11: Fix the broken markdown link reference.

The link reference "[Sourcebot EE]" is not properly defined, causing a markdown linting error. Either define the reference at the bottom of the file or use an inline link.

Apply this diff to fix the link reference:

-### Added
-- [Experimental][Sourcebot EE] Added permission syncing repository Access Control Lists (ACLs) between Sourcebot and GitHub. [#508](https://github.com/sourcebot-dev/sourcebot/pull/508)
+### Added
+- [Experimental] [Sourcebot EE](https://sourcebot.dev/pricing) Added permission syncing repository Access Control Lists (ACLs) between Sourcebot and GitHub. [#508](https://github.com/sourcebot-dev/sourcebot/pull/508)

Alternatively, if you prefer to use reference-style links, add the definition at the bottom of the file:

[Sourcebot EE]: https://sourcebot.dev/pricing
packages/backend/src/github.ts (2)

37-42: Simplify the isHttpError helper function.

The null check is redundant since TypeScript's type narrowing with the in operator already handles null values correctly.

 const isHttpError = (error: unknown, status: number): boolean => {
-    return error !== null
-        && typeof error === 'object'
+    return typeof error === 'object'
+        && error !== null
         && 'status' in error
         && error.status === status;
 }

166-172: Validate the AUTH_EE_GITHUB_BASE_URL environment variable.

The function should validate the base URL format to prevent runtime errors.

 export const createOctokitFromOAuthToken = async (token: string | null): Promise<Octokit> => {
-    const apiUrl = env.AUTH_EE_GITHUB_BASE_URL ? `${env.AUTH_EE_GITHUB_BASE_URL}/api/v3` : "https://api.github.com";
+    let apiUrl = "https://api.github.com";
+    if (env.AUTH_EE_GITHUB_BASE_URL) {
+        try {
+            // Validate URL format
+            new URL(env.AUTH_EE_GITHUB_BASE_URL);
+            apiUrl = `${env.AUTH_EE_GITHUB_BASE_URL}/api/v3`;
+        } catch (error) {
+            logger.warn(`Invalid AUTH_EE_GITHUB_BASE_URL: ${env.AUTH_EE_GITHUB_BASE_URL}, falling back to default`);
+        }
+    }
     return new Octokit({
         auth: token,
         baseUrl: apiUrl,
     });
 }
packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts (1)

19-19: Confirm authentication wrapper on getFileSource
getFileSource is now wrapped in withOptionalAuthV2 (packages/web/src/features/search/fileSourceApi.ts:14–15), so auth is injected via context. Verify that allowing unauthenticated access is intended; if file content must be protected, switch to withAuthV2 to enforce authentication.

packages/db/prisma/schema.prisma (4)

90-101: Prevent duplicate in-flight repo jobs; add useful indexes.

Without a guard, multiple PENDING/IN_PROGRESS jobs can be created per repo. Recommend a partial unique index and lookup indexes.

Migration snippet:

-- Fast lookups
CREATE INDEX IF NOT EXISTS "idx_repo_perm_job_repo_status_time"
  ON "RepoPermissionSyncJob" ("repoId", "status", "createdAt" DESC);

-- Concurrency guard: at most one active job per repo
CREATE UNIQUE INDEX IF NOT EXISTS "uniq_active_repo_perm_job"
  ON "RepoPermissionSyncJob" ("repoId")
  WHERE status IN ('PENDING','IN_PROGRESS');

Note: Prisma doesn’t model partial indexes; keep this as a SQL migration.


297-320: User permission fields: LGTM; add helpful index on sync timestamp.

Optional: index permissionSyncedAt if you’ll query “stale” users.

Prisma addition:

@@ model User
   permissionSyncedAt DateTime?
+  @@index([permissionSyncedAt], map: "idx_user_perm_synced_at")

329-340: Prevent duplicate in-flight user jobs; add lookup index.

Mirror the repo job guard for users.

Migration snippet:

CREATE INDEX IF NOT EXISTS "idx_user_perm_job_user_status_time"
  ON "UserPermissionSyncJob" ("userId", "status", "createdAt" DESC);

CREATE UNIQUE INDEX IF NOT EXISTS "uniq_active_user_perm_job"
  ON "UserPermissionSyncJob" ("userId")
  WHERE status IN ('PENDING','IN_PROGRESS');

342-352: Add index for user-centric ACL lookups.

Most queries are “repos accessible by user X”. The composite PK on (repoId, userId) doesn’t index by userId alone; add an index.

Apply in Prisma:

 model UserToRepoPermission {
   createdAt DateTime @default(now())
   repo   Repo @relation(fields: [repoId], references: [id], onDelete: Cascade)
   repoId Int
   user   User   @relation(fields: [userId], references: [id], onDelete: Cascade)
   userId String
   @@id([repoId, userId])
+  @@index([userId], map: "idx_user_to_repo_permission_user")
 }
docs/docs/configuration/environment-variables.mdx (1)

62-62: Clarify gating and prerequisites.

Note that enabling requires an EE license and that only supported code hosts (GitHub in this PR) are affected.

Proposed copy tweak:

-| `EXPERIMENT_EE_PERMISSION_SYNC_ENABLED` | `false` | <p>Enables [permission syncing](/docs/features/permission-syncing).</p> |
+| `EXPERIMENT_EE_PERMISSION_SYNC_ENABLED` | `false` | <p>Enables [permission syncing](/docs/features/permission-syncing) (EE + license). Currently supports GitHub.</p> |
docs/snippets/experimental-feature-warning.mdx (1)

1-4: Grammar nit: article before “issue”.

Change “a issue” to “an issue”.

-This is an experimental feature. Certain functionality may be buggy or incomplete, and breaking changes may ship in non-major releases. Have feedback? Submit a [issue](https://github.com/sourcebot-dev/sourcebot/issues) on GitHub.
+This is an experimental feature. Certain functionality may be buggy or incomplete, and breaking changes may ship in non-major releases. Have feedback? Submit an [issue](https://github.com/sourcebot-dev/sourcebot/issues) on GitHub.
packages/web/src/__mocks__/prisma.ts (1)

6-8: Reset vi mocks between tests to avoid cross-test leakage.

mockReset(prisma) doesn’t clear vi.fn() state. Clear all mocks in beforeEach.

 beforeEach(() => {
   mockReset(prisma);
+  vi.clearAllMocks();
 });
docs/docs/configuration/config-file.mdx (1)

50-52: Document units and gating for the new interval settings.

Clarify they’re milliseconds and that permission syncing is gated by EXPERIMENT_EE_PERMISSION_SYNC_ENABLED and licensing.

Add after the table:

+> Note: All interval values ending in `Ms` are specified in milliseconds.  
+> Permission syncing is an experimental EE feature and only runs when `EXPERIMENT_EE_PERMISSION_SYNC_ENABLED=true` and your license includes the `permission-syncing` entitlement.
schemas/v3/index.json (1)

71-79: Intervals should be integer typed with explicit defaults.

Use integer (ms) and add defaults (24h) to align tooling and docs.

Apply this diff:

-                "experiment_repoDrivenPermissionSyncIntervalMs": {
-                    "type": "number",
-                    "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
-                    "minimum": 1
-                },
+                "experiment_repoDrivenPermissionSyncIntervalMs": {
+                    "type": "integer",
+                    "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
+                    "minimum": 1,
+                    "default": 86400000
+                },
-                "experiment_userDrivenPermissionSyncIntervalMs": {
-                    "type": "number",
-                    "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
-                    "minimum": 1
-                }
+                "experiment_userDrivenPermissionSyncIntervalMs": {
+                    "type": "integer",
+                    "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
+                    "minimum": 1,
+                    "default": 86400000
+                }
packages/backend/src/constants.ts (2)

18-21: Typo in comment (“deprected”).

Fix spelling to “deprecated”.

Apply this diff:

-    enablePublicAccess: false, // deprected, use FORCE_ENABLE_ANONYMOUS_ACCESS instead
+    enablePublicAccess: false, // deprecated, use FORCE_ENABLE_ANONYMOUS_ACCESS instead

23-25: Make supported host list readonly and narrow type.

Mark as readonly to avoid mutation and enable literal types.

Apply this diff:

-export const PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES = [
-    'github',
-];
+export const PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES = [
+    'github',
+] as const;

Optionally add a type alias for consumers:

export type PermissionSyncSupportedHost = typeof PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES[number];
docs/docs/features/permission-syncing.mdx (2)

35-43: Terminology nit: GitHub editions.

Use “GitHub Enterprise Cloud (GHEC) & GitHub Enterprise Server (GHES)”.

Apply this diff:

-| [GitHub (GHEC & GHEC Server)](/docs/features/permission-syncing#github) | ✅ |
+| [GitHub (GHEC & GHES)](/docs/features/permission-syncing#github) | ✅ |

58-61: Document required scopes for repo‑driven sync.

Add the minimum PAT scopes used by the repo‑driven syncer (e.g., repo; include read:org if team/organization lookups are needed).

Proposed addition after the OAuth note:

- For repo‑driven syncing (using the connection token), ensure the token has at least the `repo` scope. If your org requires team/organization visibility for collaborator resolution, include `read:org`.
packages/backend/src/connectionManager.ts (2)

72-108: Harden the scheduler loop.

If any awaited call throws, the unhandled rejection escapes the interval callback. Add a try/catch to log and continue.

-        this.schedulerHandle = setInterval(async () => {
+        this.schedulerHandle = setInterval(async () => {
+            try {
             const thresholdDate = new Date(Date.now() - this.settings.resyncConnectionIntervalMs);
@@
-            for (const connection of connections) {
-                await this.scheduleConnectionSync(connection);
-            }
+            for (const connection of connections) {
+                await this.scheduleConnectionSync(connection);
+            }
+            } catch (err) {
+                this.logger.error(`Scheduler tick failed: ${err}`);
+                Sentry.captureException(err);
+            }
         }, this.settings.resyncConnectionPollingIntervalMs);

268-291: Guard against missing notFound metadata.

If metadata shape changes, notFound may be undefined and .length will throw. Default safely.

-        const { notFound } = syncStatusMetadata as {
+        const { notFound } = syncStatusMetadata as {
             notFound: {
                 users: string[],
                 orgs: string[],
                 repos: string[],
             }
         };
@@
-                syncStatus:
-                    notFound.users.length > 0 ||
-                        notFound.orgs.length > 0 ||
-                        notFound.repos.length > 0 ? ConnectionSyncStatus.SYNCED_WITH_WARNINGS : ConnectionSyncStatus.SYNCED,
+                syncStatus:
+                    (notFound?.users?.length ?? 0) > 0 ||
+                    (notFound?.orgs?.length ?? 0) > 0 ||
+                    (notFound?.repos?.length ?? 0) > 0
+                        ? ConnectionSyncStatus.SYNCED_WITH_WARNINGS
+                        : ConnectionSyncStatus.SYNCED,
packages/schemas/src/v3/index.schema.ts (2)

72-81: Schema defaults missing for new settings.

Descriptions say “Defaults to 24 hours” but no default is specified. Add default: 86400000 and an examples entry to align tooling/validation with docs.

 "experiment_repoDrivenPermissionSyncIntervalMs": {
   "type": "number",
-  "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
+  "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
+  "default": 86400000,
   "minimum": 1
 },
 "experiment_userDrivenPermissionSyncIntervalMs": {
   "type": "number",
-  "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
+  "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
+  "default": 86400000,
   "minimum": 1
 }

208-217: Duplicate: apply defaults in root settings too.

Mirror the same default values here to keep the root and definitions.Settings in sync.

 "experiment_repoDrivenPermissionSyncIntervalMs": {
   "type": "number",
-  "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
+  "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
+  "default": 86400000,
   "minimum": 1
 },
 "experiment_userDrivenPermissionSyncIntervalMs": {
   "type": "number",
-  "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
+  "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
+  "default": 86400000,
   "minimum": 1
 }
packages/web/src/withAuthV2.test.ts (4)

185-193: Avoid brittle equality on optional prisma.

Asserting prisma: undefined ties tests to internal shape. Prefer objectContaining to focus on the contract (user/org/role).

-        expect(authContext).toStrictEqual({
+        expect(authContext).toStrictEqual(expect.objectContaining({
             user: {
                 ...MOCK_USER,
                 id: userId,
             },
             org: MOCK_ORG,
             role: OrgRole.MEMBER,
-            prisma: undefined,
-        });
+        }));

215-223: Same: relax assertion for OWNER case.

-        expect(authContext).toStrictEqual({
+        expect(authContext).toStrictEqual(expect.objectContaining({
             user: {
                 ...MOCK_USER,
                 id: userId,
             },
             org: MOCK_ORG,
             role: OrgRole.OWNER,
-            prisma: undefined,
-        });
+        }));

239-248: Same: relax assertion for GUEST case.

-        expect(authContext).toStrictEqual({
+        expect(authContext).toStrictEqual(expect.objectContaining({
             user: {
                 ...MOCK_USER,
                 id: userId,
             },
             org: MOCK_ORG,
             role: OrgRole.GUEST,
-            prisma: undefined,
-        });
+        }));

258-263: Same: relax assertion for anonymous case.

-        expect(authContext).toStrictEqual({
+        expect(authContext).toStrictEqual(expect.objectContaining({
             user: undefined,
             org: MOCK_ORG,
             role: OrgRole.GUEST,
-            prisma: undefined,
-        });
+        }));
packages/web/src/prisma.ts (1)

19-23: Typo in JSDoc.
“striclty” → “strictly”.

- * Creates a prisma client extension that scopes queries to striclty information
+ * Creates a prisma client extension that scopes queries to strictly information
packages/backend/src/ee/repoPermissionSyncer.ts (3)

45-104: Use configurable polling; gate by env flag.

  • Polling interval is hardcoded to 5s. Use a setting (e.g., reuse resyncConnectionPollingIntervalMs) or introduce an explicit polling setting to avoid tight loops in prod.
  • Consider also checking env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' alongside entitlement to avoid accidental enablement.
-        return setInterval(async () => {
+        return setInterval(async () => {
             // @todo: make this configurable
             const thresholdDate = new Date(Date.now() - this.settings.experiment_repoDrivenPermissionSyncIntervalMs);
@@
-        }, 1000 * 5);
+        }, this.settings.resyncConnectionPollingIntervalMs);

119-129: Idempotent BullMQ enqueue.

Set jobId on each BullMQ job to prevent duplicates across processes.

-            await this.queue.addBulk(jobs.map((job) => ({
+            await this.queue.addBulk(jobs.map((job) => ({
                 name: 'repoPermissionSyncJob',
                 data: {
                     jobId: job.id,
                 },
                 opts: {
+                    jobId: job.id,
                     removeOnComplete: env.REDIS_REMOVE_ON_COMPLETE,
                     removeOnFail: env.REDIS_REMOVE_ON_FAIL,
                 }
             })))

106-110: Await graceful shutdown of BullMQ client/worker.

close() returns a promise. Consider awaiting to ensure clean shutdown in tests and process exit hooks.

-    public dispose() {
-        this.worker.close();
-        this.queue.close();
+    public async dispose() {
+        await this.worker.close();
+        await this.queue.close();
     }
docs/snippets/schemas/v3/index.schema.mdx (3)

72-82: Schema: use integer + declare default (24h).

Intervals are whole milliseconds; the schema should reflect that and advertise the default explicitly.

Apply this diff:

-        "experiment_repoDrivenPermissionSyncIntervalMs": {
-          "type": "number",
-          "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
-          "minimum": 1
-        },
+        "experiment_repoDrivenPermissionSyncIntervalMs": {
+          "type": "integer",
+          "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
+          "minimum": 1,
+          "default": 86400000
+        },
-        "experiment_userDrivenPermissionSyncIntervalMs": {
-          "type": "number",
-          "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
-          "minimum": 1
-        }
+        "experiment_userDrivenPermissionSyncIntervalMs": {
+          "type": "integer",
+          "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
+          "minimum": 1,
+          "default": 86400000
+        }

72-82: Optional: raise minimum to avoid self‑DDOS.

1ms allows extremely aggressive schedules. Consider raising to at least 60_000.

-          "minimum": 1,
+          "minimum": 60000,

208-218: Mirror integer/default change in nested Settings.

-        "experiment_repoDrivenPermissionSyncIntervalMs": {
-          "type": "number",
+        "experiment_repoDrivenPermissionSyncIntervalMs": {
+          "type": "integer",
           "description": "The interval (in milliseconds) at which the repo permission syncer should run. Defaults to 24 hours.",
-          "minimum": 1
+          "minimum": 1,
+          "default": 86400000
         },
-        "experiment_userDrivenPermissionSyncIntervalMs": {
-          "type": "number",
+        "experiment_userDrivenPermissionSyncIntervalMs": {
+          "type": "integer",
           "description": "The interval (in milliseconds) at which the user permission syncer should run. Defaults to 24 hours.",
-          "minimum": 1
+          "minimum": 1,
+          "default": 86400000
         }
packages/backend/src/index.ts (3)

72-90: Instantiate permission syncers lazily (only when enabled + entitled).

Avoid constructing EE components when disabled/not entitled.

-const repoPermissionSyncer = new RepoPermissionSyncer(prisma, settings, redis);
-const userPermissionSyncer = new UserPermissionSyncer(prisma, settings, redis);
+let repoPermissionSyncer: RepoPermissionSyncer | null = null;
+let userPermissionSyncer: UserPermissionSyncer | null = null;
@@
-else if (env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing')) {
-    repoPermissionSyncerInterval = repoPermissionSyncer.startScheduler();
-    userPermissionSyncerInterval = userPermissionSyncer.startScheduler();
+else if (env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing')) {
+    repoPermissionSyncer = new RepoPermissionSyncer(prisma, settings, redis);
+    userPermissionSyncer = new UserPermissionSyncer(prisma, settings, redis);
+    repoPermissionSyncerInterval = repoPermissionSyncer.startScheduler();
+    userPermissionSyncerInterval = userPermissionSyncer.startScheduler();

And in cleanup:

-    repoPermissionSyncer.dispose();
-    userPermissionSyncer.dispose();
+    repoPermissionSyncer?.dispose();
+    userPermissionSyncer?.dispose();

92-94: Fix log typo.

-    logger.info(`Recieved ${signal}, cleaning up...`);
+    logger.info(`Received ${signal}, cleaning up...`);

123-126: Harden unhandledRejection logging.

Log reason stack; avoid stringifying Promise.

-process.on('unhandledRejection', (reason, promise) => {
-    logger.error(`Unhandled rejection at: ${promise}, reason: ${reason}`);
+process.on('unhandledRejection', (reason) => {
+    const msg = reason instanceof Error ? reason.stack ?? reason.message : String(reason);
+    logger.error(`Unhandled rejection: ${msg}`);
     cleanup('unhandledRejection').finally(() => process.exit(1));
 });
packages/backend/src/ee/userPermissionSyncer.ts (3)

109-128: Queue/DB non-atomicity can orphan jobs (PENDING with nothing enqueued).

Enqueuing inside the DB transaction is not atomic with BullMQ; failures can leave rows stuck in PENDING (then excluded by the scheduler). Commit first, then add to queue; on enqueue failure, mark jobs FAILED with error for visibility (or adopt an outbox pattern).

-        await this.db.$transaction(async (tx) => {
-            const jobs = await tx.userPermissionSyncJob.createManyAndReturn({ data: users.map(u => ({ userId: u.id })) });
-            await this.queue.addBulk( ... );
-        });
+        const jobs = await this.db.userPermissionSyncJob.createManyAndReturn({
+            data: users.map(u => ({ userId: u.id })),
+        });
+        try {
+            await this.queue.addBulk(/* as above with jobId */);
+        } catch (e) {
+            await this.db.userPermissionSyncJob.updateMany({
+                where: { id: { in: jobs.map(j => j.id) } },
+                data: { status: UserPermissionSyncJobStatus.FAILED, completedAt: new Date(), errorMessage: String(e) },
+            });
+            throw e;
+        }

1-1: Remove unused import.

Octokit is not used.

-import { Octokit } from "@octokit/rest";

222-251: Capture richer failure context.

Persist stack traces (truncated) to aid debugging.

-                    errorMessage: err.message,
+                    errorMessage: (err.stack ?? err.message).slice(0, 8000),
packages/web/src/features/search/searchApi.ts (2)

163-167: Inline header construction.

Minor cleanup; no need for a temp var.

-        let header: Record<string, string> = {};
-        header = {
-            "X-Tenant-ID": org.id.toString()
-        };
+        const header: Record<string, string> = { "X-Tenant-ID": org.id.toString() };

196-216: Reduce DB round-trips for repo lookup.

One query with OR reduces latency; also skip when no identifiers.

-            const repos = new Map<string | number, Repo>();
-
-            (await prisma.repo.findMany({
-                where: {
-                    id: {
-                        in: Array.from(repoIdentifiers).filter((id) => typeof id === "number"),
-                    },
-                    orgId: org.id,
-                }
-            })).forEach(repo => repos.set(repo.id, repo));
-
-            (await prisma.repo.findMany({
-                where: {
-                    name: {
-                        in: Array.from(repoIdentifiers).filter((id) => typeof id === "string"),
-                    },
-                    orgId: org.id,
-                }
-            })).forEach(repo => repos.set(repo.name, repo));
+            const repos = new Map<string | number, Repo>();
+            const ids = Array.from(repoIdentifiers);
+            if (ids.length > 0) {
+                const numericIds = ids.filter((v): v is number => typeof v === "number");
+                const nameIds = ids.filter((v): v is string => typeof v === "string");
+                (await prisma.repo.findMany({
+                    where: {
+                        orgId: org.id,
+                        OR: [
+                            ...(numericIds.length ? [{ id: { in: numericIds } }] : []),
+                            ...(nameIds.length ? [{ name: { in: nameIds } }] : []),
+                        ],
+                    }
+                })).forEach(repo => { repos.set(repo.id, repo); repos.set(repo.name, repo); });
+            }
packages/db/prisma/migrations/20250919224623_add_permission_sync_tables/migration.sql (3)

13-24: Add indexes for RepoPermissionSyncJob lookups.

Filter-by-status and by repoId will be common; add a composite index.

 );
 
+-- Indexes for RepoPermissionSyncJob
+CREATE INDEX "RepoPermissionSyncJob_repoId_status_completedAt_idx"
+ON "RepoPermissionSyncJob" ("repoId", "status", "completedAt");

26-37: Add indexes for UserPermissionSyncJob.

Scheduler filters by status/completedAt and joins by userId.

 );
 
+-- Indexes for UserPermissionSyncJob
+CREATE INDEX "UserPermissionSyncJob_userId_idx"
+ON "UserPermissionSyncJob" ("userId");
+CREATE INDEX "UserPermissionSyncJob_userId_status_completedAt_idx"
+ON "UserPermissionSyncJob" ("userId", "status", "completedAt");

10-12: Optional: index permissionSyncedAt for scheduler scans.

Improves range scans when computing threshold windows.

 ALTER TABLE "User" ADD COLUMN     "permissionSyncedAt" TIMESTAMP(3);
 
+CREATE INDEX "User_permissionSyncedAt_idx" ON "User" ("permissionSyncedAt");
+CREATE INDEX "Repo_permissionSyncedAt_idx" ON "Repo" ("permissionSyncedAt");
packages/backend/src/repoCompileUtils.ts (1)

590-613: Generic git host (URL): explicitly set isPublic (default false).

Make intent explicit and consistent with other compilers.

     const repo: RepoData = {
         external_codeHostType: 'generic-git-host',
         external_codeHostUrl: remoteUrl.origin,
         external_id: remoteUrl.toString(),
         cloneUrl: remoteUrl.toString(),
         name: repoName,
         displayName: repoName,
+        isPublic: false,
         isFork: false,
         isArchived: false,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a76ae68 and 0209342.

📒 Files selected for processing (53)
  • CHANGELOG.md (1 hunks)
  • LICENSE.md (1 hunks)
  • docs/docs.json (1 hunks)
  • docs/docs/configuration/config-file.mdx (1 hunks)
  • docs/docs/configuration/environment-variables.mdx (1 hunks)
  • docs/docs/connections/github.mdx (1 hunks)
  • docs/docs/features/agents/overview.mdx (1 hunks)
  • docs/docs/features/permission-syncing.mdx (1 hunks)
  • docs/snippets/experimental-feature-warning.mdx (1 hunks)
  • docs/snippets/schemas/v3/index.schema.mdx (2 hunks)
  • package.json (1 hunks)
  • packages/backend/src/connectionManager.ts (2 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/ee/repoPermissionSyncer.ts (1 hunks)
  • packages/backend/src/ee/userPermissionSyncer.ts (1 hunks)
  • packages/backend/src/env.ts (1 hunks)
  • packages/backend/src/github.ts (4 hunks)
  • packages/backend/src/index.ts (2 hunks)
  • packages/backend/src/main.ts (0 hunks)
  • packages/backend/src/repoCompileUtils.ts (14 hunks)
  • packages/backend/src/repoManager.ts (2 hunks)
  • packages/backend/src/types.ts (2 hunks)
  • packages/db/prisma/migrations/20250919224623_add_permission_sync_tables/migration.sql (1 hunks)
  • packages/db/prisma/migrations/20250919230022_add_is_public_column_to_repo_table/migration.sql (1 hunks)
  • packages/db/prisma/schema.prisma (8 hunks)
  • packages/schemas/src/v3/index.schema.ts (2 hunks)
  • packages/schemas/src/v3/index.type.ts (1 hunks)
  • packages/shared/src/entitlements.ts (1 hunks)
  • packages/web/src/__mocks__/prisma.ts (2 hunks)
  • packages/web/src/actions.ts (6 hunks)
  • packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1 hunks)
  • packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (1 hunks)
  • packages/web/src/app/[domain]/browse/[...path]/page.tsx (0 hunks)
  • packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (1 hunks)
  • packages/web/src/app/[domain]/repos/components/addRepositoryDialog.tsx (1 hunks)
  • packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1 hunks)
  • packages/web/src/app/api/(server)/search/route.ts (1 hunks)
  • packages/web/src/app/api/(server)/source/route.ts (1 hunks)
  • packages/web/src/app/api/[domain]/repos/[repoId]/image/route.ts (1 hunks)
  • packages/web/src/ee/features/sso/sso.ts (3 hunks)
  • packages/web/src/env.mjs (1 hunks)
  • packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts (1 hunks)
  • packages/web/src/features/chat/agent.ts (1 hunks)
  • packages/web/src/features/chat/tools.ts (2 hunks)
  • packages/web/src/features/codeNav/actions.ts (2 hunks)
  • packages/web/src/features/fileTree/actions.ts (2 hunks)
  • packages/web/src/features/fileTree/components/fileTreePanel.tsx (1 hunks)
  • packages/web/src/features/search/fileSourceApi.ts (1 hunks)
  • packages/web/src/features/search/searchApi.ts (3 hunks)
  • packages/web/src/prisma.ts (1 hunks)
  • packages/web/src/withAuthV2.test.ts (4 hunks)
  • packages/web/src/withAuthV2.ts (11 hunks)
  • schemas/v3/index.json (1 hunks)
💤 Files with no reviewable changes (2)
  • packages/backend/src/main.ts
  • packages/web/src/app/[domain]/browse/[...path]/page.tsx
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

11-11: Reference links and images should use a label that is defined
Missing link or image reference definition: "sourcebot ee"

(MD052, reference-links-images)

⏰ 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
🔇 Additional comments (38)
package.json (2)

17-17: Good addition: dev-time Prisma client generation.

This mirrors existing prisma tasks and aligns with the monorepo workflow. No functional concerns.


17-17: Verified — @sourcebot/db defines prisma:generate. scripts.prisma:generate = "prisma generate".

LICENSE.md (1)

5-5: Approve — EE license scope expansion looks correct

ee/LICENSE exists and all expected EE directories are present (ee, packages/web/src/ee, packages/backend/src/ee, packages/shared/src/ee); adding packages/backend/src/ee aligns the license with the new EE backend modules.

packages/web/src/app/api/[domain]/repos/[repoId]/image/route.ts (2)

6-6: Rename to _request is good

Accurately signals the arg is unused. No functional risk.


17-18: Signature change: ensure all call sites updated and auth scoping enforced inside getRepoImage

  • Verify getRepoImage now accepts a single arg (repoId) and no callers still pass domain. Ensure getRepoImage enforces repo visibility via the scoped Prisma client (withAuthV2 / withOptionalAuthV2) rather than relying on domain.

File: packages/web/src/app/api/[domain]/repos/[repoId]/image/route.ts
Lines: 17-18

    const result = await getRepoImage(repoIdNum);
    if (isServiceError(result)) {
packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (1)

58-60: getFiles signature no longer requires domain and no callers pass it.

packages/backend/src/types.ts (1)

56-56: LGTM! Well-structured type composition for repository permissions.

The new RepoWithConnections type appropriately extends the Repo type with its associated connections, which aligns well with the permission syncing feature requirements.

packages/backend/src/env.ts (1)

56-57: LGTM! Environment variables properly configured for permission syncing.

The new environment variables are well-structured with appropriate defaults and follow the existing schema patterns.

packages/shared/src/entitlements.ts (1)

42-42: LGTM! Permission syncing entitlement properly integrated.

The new permission-syncing entitlement is correctly added to the enterprise plans, aligning with the experimental EE feature rollout.

Also applies to: 49-50

packages/db/prisma/schema.prisma (7)

83-88: Job status enum looks good.

Statuses cover the expected lifecycle phases.


178-185: AccountRequest relation change: LGTM.

Cascade on requester deletion is sensible.


200-214: Invite link fields: LGTM.

No issues; consistent with existing Invite model.


274-289: Audit indices: LGTM.

The composite indexes match common query shapes.


260-270: Primary key on ApiKey.hash: verify downstream assumptions.

Switching primary key to hash can break joins expecting a separate id. Confirm no FK references or code paths assume a numeric/opaque id.


106-109: Ensure SearchContext repo membership changes trigger reindex or cache invalidation.

  • syncSearchContexts updates DB via connect/disconnect — packages/shared/src/ee/syncSearchContexts.ts.
  • connectionManager calls syncSearchContexts after connection sync — packages/backend/src/connectionManager.ts (onSyncJobCompleted).
  • Repo indexing is driven by repoIndexingStatus and repoManager scheduling — packages/backend/src/repoManager.ts; syncSearchContexts does not set repoIndexingStatus or enqueue index jobs.
  • Frontend caches search contexts via getSearchContexts / React Query key ["searchContexts", domain] — packages/web/src/actions.ts and packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts.

Confirm newly-added repos will be indexed and clients see the change; if not, either set repoIndexingStatus=NEW / enqueue reindex when connecting repos in syncSearchContexts, and/or invalidate the client cache / broadcast an update after syncSearchContexts runs.


61-66: Confirmed: scoped Prisma extension enforces (isPublic OR permittedUsers.some.userId = currentUser) for the user-facing repo reads — with caveats

The web package defines userScopedPrismaClientExtension (packages/web/src/prisma.ts) which injects an OR filter for repo queries; withAuthV2/withOptionalAuthV2 create a prisma via __unsafePrisma.$extends(userScopedPrismaClientExtension(user?.id)) so handlers receive the extended client.

  • Verified user-facing paths use the scoped client:

    • Search: packages/web/src/features/search/searchApi.ts — withOptionalAuthV2 (~line 131); repo.findMany at lines 199, 208.
    • File browsing / code-nav: packages/web/src/features/fileTree/actions.ts — repo.findFirst at lines 31, 88, 160 (getTree/getFolderContents/getFiles) called via withOptionalAuthV2.
    • UI repo endpoints: packages/web/src/actions.ts — getRepos (≈641–646) and getRepoImage (≈2009–2016) use withOptionalAuthV2/withAuth wrappers.
  • Exceptions / unscoped usages:

    • packages/web/src/initialize.ts uses the global prisma directly (prisma.repo.updateMany at line 71) — prisma.ts explicitly notes some exceptions.
    • Backend/tools scripts and other server-side code import PrismaClient directly; those are unscoped by design.
  • Caveats you must verify:

    1. The extension only runs when env.EXPERIMENT_EE_PERMISSION_SYNC_ENABLED === 'true' AND hasEntitlement('permission-syncing') is true — if those are not enabled in your environment, the filter is not applied.
    2. The extension only injects the filter when the query args include a 'where' property.
    3. Current implementation replaces any existing top-level OR on args.where (it sets args.where = { ...args.where, OR: [...] }), which will drop a pre-existing OR — consider changing to args.where = { AND: [args.where, { OR: [...] }] } to preserve existing OR logic.

Action: confirm the env flag + entitlement are enabled in the target environments and audit any repo reads that deliberately use the global prisma to ensure they should be unscoped.

docs/docs/connections/github.mdx (1)

199-204: “See also” addition: LGTM.

Good cross-link to the permission syncing doc.

docs/docs.json (1)

49-49: Nav entry for permission syncing: LGTM.

Placement under Features is appropriate.

packages/web/src/features/codeNav/actions.ts (2)

66-71: Duplicate update in second call: LGTM.

Consistent with references path.


33-38: Refine call‐site detection for the new search API
Use a simpler grep that excludes fuse.search and catches any direct search() call with multiple positional args:

#!/bin/bash
rg -n --type=ts '(^|[^.])search\([^)]*,\s*' packages/web | grep -v 'fuse\.search'

Run this to confirm no legacy signatures remain.

packages/web/src/features/chat/agent.ts (1)

248-255: LGTM: domain param removed from getFileSource.

Matches the new domain-agnostic API surface.

packages/web/src/features/chat/tools.ts (1)

35-41: No change: findSearchBasedSymbol still requires SINGLE_TENANT_ORG_DOMAIN.*

findSearchBasedSymbolReferences/Definitions in packages/web/src/features/codeNav/actions.ts accept a domain parameter; calling them with SINGLE_TENANT_ORG_DOMAIN is correct.

packages/web/src/__mocks__/prisma.ts (1)

38-48: Regenerate Prisma client — ensure @prisma/client's User type includes permissionSyncedAt.

packages/db/prisma/schema.prisma defines permissionSyncedAt (User & Repo) and the migration packages/db/prisma/migrations/20250919224623_add_permission_sync_tables/migration.sql adds the column; mock updated at packages/web/src/mocks/prisma.ts:47. Run prisma generate (or reinstall deps) and commit/verify the regenerated @prisma/client so TypeScript checks don’t fail.

packages/backend/src/repoManager.ts (1)

29-29: Verify removal of IRepoManager and API surface

  • The run of rg -nP '\bIRepoManager\b' produced no matches; absence of output is inconclusive. Manually confirm no remaining imports/usages and that published typings / external consumers don't depend on the interface — check packages/backend/src/repoManager.ts and the package's exported typings/exports.
packages/web/src/app/api/(server)/search/route.ts (1)

18-22: Auth-scoping confirmed — no action required.
search() in packages/web/src/features/search/searchApi.ts is wrapped with withOptionalAuthV2(async ({ org, prisma }) => ...), so Prisma is auth-scoped and results are permission-filtered.

docs/docs/features/agents/overview.mdx (1)

6-9: Confirm snippet import path

Found docs/snippets/experimental-feature-warning.mdx. Build cannot be verified here — confirm the docs pipeline resolves root‑relative imports (/snippets/experimental-feature-warning.mdx) or change the import to a relative path (../../../snippets/experimental-feature-warning.mdx).

packages/schemas/src/v3/index.type.ts (1)

105-112: LGTM on new Settings fields — confirm generator parity

Types present in packages/schemas/src/v3/index.type.ts (lines 108–112). Defaults wired in packages/backend/src/constants.ts (lines 19–20) as 10006060*24 (24h). packages/schemas/src/v3/index.json is missing from the repo; cannot confirm the fields were generated from the JSON schema. Confirm the JSON schema contains these properties and that index.type.ts was generated (not hand‑edited).

packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (1)

15-23: Confirmed: domainless signatures + org-scoped permission enforcement

getRepoInfoByName (packages/web/src/actions.ts) and getFolderContents (packages/web/src/features/fileTree/actions.ts) take no domain, run under withOptionalAuthV2, and restrict repo lookups via prisma where { orgId: org.id }.

packages/web/src/features/search/fileSourceApi.ts (1)

14-62: Anchor and escape query inputs to prevent mismatches/injection.

  • Anchor file: to the exact path and escape branch.
  • Prefer selecting the exact path from results instead of files[0].
 export const getFileSource = async ({ fileName, repository, branch }: FileSourceRequest): Promise<FileSourceResponse | ServiceError> => sew(() =>
     withOptionalAuthV2(async () => {
         const escapedFileName = escapeStringRegexp(fileName);
         const escapedRepository = escapeStringRegexp(repository);
+        const escapedBranch = branch ? escapeStringRegexp(branch) : undefined;

-        let query = `file:${escapedFileName} repo:^${escapedRepository}$`;
+        let query = `file:^${escapedFileName}$ repo:^${escapedRepository}$`;
         if (branch) {
-            query = query.concat(` branch:${branch}`);
+            query = query.concat(` branch:^${escapedBranch}$`);
         }
@@
-        const file = files[0];
+        const file = files.find(f => f.path === fileName) ?? files[0];

Please confirm that searchResponse.files[*].path exists; if not, I can adjust the predicate to the correct field.

packages/backend/src/index.ts (1)

66-67: Consider disposing PromClient on shutdown.

If PromClient holds timers/sockets, add a dispose/close in cleanup.

Do we have a promClient.close()/dispose()? If yes, add it after repoManager.dispose().

packages/web/src/actions.ts (2)

642-670: LGTM: repo listing now scopes via user‑scoped Prisma.


672-730: getRepoInfoByName: domainless flow looks good — callsites updated.
Confirmed imports/calls use the single‑arg API in packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx and packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx.

packages/web/src/withAuthV2.ts (1)

91-98: Confirm extension handles undefined user id.

We extend Prisma with userScopedPrismaClientExtension(user?.id); verify the extension tolerates undefined.

If it doesn’t, guard it:

-const prisma = __unsafePrisma.$extends(userScopedPrismaClientExtension(user?.id)) as PrismaClient;
+const prisma = __unsafePrisma.$extends(
+  user ? userScopedPrismaClientExtension(user.id) : {}
+) as PrismaClient;
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)

15-23: LGTM: domainless calls align with withOptionalAuthV2.

Ensure getFileSource’s signature no longer requires domain:

packages/backend/src/ee/userPermissionSyncer.ts (1)

42-46: Verify feature gating aligns with EXPERIMENT_EE_PERMISSION_SYNC_ENABLED.

Only entitlement is checked here. Confirm env flag/license checks are enforced at entry points to avoid unexpected activation in non-EE environments.

packages/web/src/features/search/searchApi.ts (1)

39-96: Search context expansion looks solid.

DB-backed context -> reposet translation and error mapping read clean.

packages/web/src/features/fileTree/actions.ts (1)

28-41: Auth integration LGTM.

Switch to withOptionalAuthV2 and org-scoped prisma is consistent with the new enforcement model.

packages/backend/src/repoCompileUtils.ts (1)

126-128: Clarify GitLab “internal” visibility handling.

Do we want to treat internal as non-public (current behavior) or public within the instance? Confirm intended semantics.

msukkari
msukkari previously approved these changes Sep 20, 2025
@brendan-kellam brendan-kellam merged commit 5073c7d into main Sep 20, 2025
10 checks passed
@brendan-kellam brendan-kellam deleted the bkellam/permission_sync branch September 20, 2025 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants