Skip to content

Conversation

@brendan-kellam
Copy link
Contributor

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

Similar to #563 and #572, this PR does two things:

  1. Adds a separate ConnectionSyncJob table for job tracking. Also moves to using groupmq.
  2. Improves the connections table with information about connection sync jobs.

Screenshots

Connection list view
image

Connection details view
image

Hovering over a warning icon
image

Hovering over a error icon
image

Info banner in repository page when no repos are configured
image

Notification dot when there is a connection that is syncing for the same time
image

Notification dot in details view
image

Notification dot in repos table to indicate first time indexing
image

TODO:

  • We can probably fully remove BackendError

Summary by CodeRabbit

Release Notes

  • New Features

    • Added connection management interface in settings with detailed sync job history and status tracking.
    • Introduced first-time sync notification indicators for repositories and connections.
    • Enhanced error reporting with detailed sync warnings and diagnostics.
  • Improvements

    • Strengthened GitHub App configuration schema with improved validation for deployment settings and private key handling.
    • Added connection synchronization statistics and monitoring capabilities.

@coderabbitai
Copy link

coderabbitai bot commented Oct 29, 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.

Walkthrough

This PR introduces a declarative configuration management system with a job-based connection synchronization model, unifies repository source return shapes from { validRepos, notFound } to { repos, warnings } across all integrations, refactors the GitHub App configuration schema with new properties, and adds UI pages for monitoring connection sync jobs.

Changes

Cohort / File(s) Summary
Declarative Configuration & Job System
packages/backend/src/configManager.ts, packages/backend/src/index.ts, packages/backend/src/connectionManager.ts
Introduces ConfigManager class for watching config files and syncing connections/search contexts. Refactors ConnectionManager with groupMQ-based job queue, timeout/retry logic, and new createJobs method replacing direct status updates. Wires up lifecycle in entry point.
Database Schema & Migrations
packages/db/prisma/schema.prisma, packages/db/prisma/migrations/20251026194617_*, packages/db/prisma/migrations/20251026194628_*
Adds ConnectionSyncJob table with status enum (PENDING, IN_PROGRESS, COMPLETED, FAILED) and replaces inline sync status fields. Creates default org record (id=1) for single tenancy. Establishes one-to-many relationship from Connection to syncJobs.
Repository Source Integrations
packages/backend/src/github.ts, packages/backend/src/gitlab.ts, packages/backend/src/gitea.ts, packages/backend/src/bitbucket.ts, packages/backend/src/azuredevops.ts
Standardizes return shapes across all repo sources from { validRepos, notFound } to { repos, warnings }. Updates internal error handling to produce warning strings instead of not-found structures. Relocates getTokenFromConfig imports from utils to @sourcebot/crypto.
Compilation Utilities
packages/backend/src/repoCompileUtils.ts, packages/backend/src/connectionUtils.ts
Introduces CompileResult type with repoData and warnings fields. Updates all compile*Config functions to return Promise<CompileResult>. Renames NotFoundResult to WarningResult in utility types.
GitHub App Configuration
packages/schemas/src/v3/app.schema.ts, packages/schemas/src/v3/app.type.ts, packages/schemas/src/v3/githubApp.schema.ts (deleted), packages/schemas/src/v3/githubApp.type.ts (deleted), docs/snippets/schemas/v3/app.schema.mdx, docs/snippets/schemas/v3/index.schema.mdx, schemas/v3/app.json, schemas/v3/githubApp.json (deleted)
Consolidates GitHub App config into main app schema under definitions. Adds deploymentHostname field with hostname format. Expands privateKey to polymorphic object supporting { secret: string } or { env: string }. Updates public types from GithubAppConfig to GitHubAppConfig.
Token Management
packages/backend/src/utils.ts, packages/backend/src/ee/githubAppManager.ts, packages/backend/src/ee/syncSearchContexts.ts
Removes getTokenFromConfig wrapper from utils; all call sites import directly from @sourcebot/crypto without logger parameter. Consolidates imports in EE modules.
Environment & Constants
packages/backend/src/env.ts, packages/backend/src/constants.ts
Adds FORCE_ENABLE_ANONYMOUS_ACCESS boolean env var and makes CONFIG_PATH required. Exports SINGLE_TENANT_ORG_ID constant with value 1.
Connection Settings UI
packages/web/src/app/[domain]/settings/connections/page.tsx, packages/web/src/app/[domain]/settings/connections/[id]/page.tsx, packages/web/src/app/[domain]/settings/connections/layout.tsx, packages/web/src/app/[domain]/settings/connections/components/connectionsTable.tsx, packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx
Adds new connections management pages with layout guard (OWNER role), tables for connections and sync jobs, filtering/sorting capabilities, and detailed sync history per connection.
Navigation & Settings Components
packages/web/src/app/[domain]/components/navigationMenu/index.tsx, packages/web/src/app/[domain]/components/navigationMenu/navigationItems.tsx, packages/web/src/app/[domain]/settings/layout.tsx, packages/web/src/app/[domain]/settings/components/sidebar-nav.tsx, packages/web/src/app/[domain]/settings/components/header.tsx (deleted)
Updates navigation to use isReposButtonNotificationDotVisible and isSettingsButtonNotificationDotVisible booleans. Adds connections link to settings sidebar for owners. Introduces SidebarNavItem type with notification dot support. Removes header component.
Notification Components
packages/web/src/app/[domain]/components/notificationDot.tsx, packages/web/src/app/[domain]/components/backButton.tsx
Adds reusable NotificationDot (green indicator) and BackButton components for UI consistency.
Repository UI Updates
packages/web/src/app/[domain]/repos/page.tsx, packages/web/src/app/[domain]/repos/[id]/page.tsx, packages/web/src/app/[domain]/repos/layout.tsx, packages/web/src/app/[domain]/repos/components/reposTable.tsx
Computes isFirstTimeIndex based on indexedAt and job status. Adds first-time indexing indicator/notification. Reorganizes repo detail page with date displays and scoped org queries. Adds banner for zero repos with connection creation link for owners.
Search Scope & Icon Handling
packages/web/src/app/[domain]/chat/components/demoCards.tsx, packages/web/src/lib/utils.ts, packages/web/src/features/chat/components/searchScopeIcon.tsx
Changes getCodeHostIcon parameter type from string to CodeHostType and return type from nullable to non-nullable. Removes null checks and FolderIcon fallbacks. Adds explicit type casts.
Initialization & Configuration
packages/web/src/initialize.ts
Replaces config-file watching and declarative sync with simplified anonymous-access handling based on entitlements. Removes syncDeclarativeConfig logic; retains guest user pruning.
Actions & Web API
packages/web/src/actions.ts
Adds getConnectionStats action returning connection count and first-time sync jobs in progress. Updates getReposStats to include indexedAt: null filter. Imports ConnectionSyncJobStatus from db.
Other Utilities
packages/backend/src/gerrit.ts, packages/backend/src/git.ts, packages/backend/src/repoIndexManager.ts, packages/web/src/app/[domain]/repos/components/reposTable.tsx (type), packages/web/src/app/[domain]/components/navigationMenu/progressIndicator.tsx, packages/web/src/app/[domain]/components/repositoryCarousel.tsx, packages/web/src/app/api/(server)/stripe/route.ts, packages/web/src/lib/syncStatusMetadataSchema.ts (deleted)
Removes Gerrit hostname extraction. Adds GIT_TERMINAL_PROMPT: '0' to git env. Refactors cleanup job timeout logic. Updates repo carousel link to internal settings. Removes sync status marking from Stripe webhook. Deletes obsolete sync metadata schema.
Dependency Updates
packages/backend/package.json, packages/web/package.json, packages/shared/src/index.server.ts
Adds chokidar ^4.0.3 to backend dependencies for file watching. Removes chokidar from web. Removes syncSearchContexts export from shared server barrel.
Documentation & License
LICENSE.md
Updates licensed directories list, removing packages/shared/src/ee/ while keeping ee/, packages/web/src/ee/, packages/backend/src/ee/.

Sequence Diagram(s)

sequenceDiagram
    participant ConfigManager
    participant Chokidar as File Watcher
    participant DB as Prisma DB
    participant ConnectionMgr as ConnectionManager
    participant Queue as Job Queue

    ConfigManager->>Chokidar: watch(configPath)
    ConfigManager->>ConfigManager: syncConfig()
    ConfigManager->>DB: query existing connections
    
    loop For each declared connection
        ConfigManager->>DB: find connection by name + orgId
        alt Connection exists
            ConfigManager->>DB: update config
        else New connection
            ConfigManager->>DB: create connection
        end
        ConfigManager->>ConnectionMgr: createJobs([connection])
        ConnectionMgr->>DB: create ConnectionSyncJob(PENDING)
        ConnectionMgr->>Queue: enqueue sync job
    end
    
    Chokidar->>ConfigManager: file changed event
    ConfigManager->>ConfigManager: syncConfig()
    
    Note over Queue: Job Lifecycle
    Queue->>ConnectionMgr: runJob(job)
    ConnectionMgr->>ConnectionMgr: compile*Config()
    ConnectionMgr->>DB: update ConnectionSyncJob(IN_PROGRESS)
    
    alt Success
        ConnectionMgr->>DB: update ConnectionSyncJob(COMPLETED)<br/>mark connection synced
        ConnectionMgr->>Queue: emit telemetry
    else Failure
        ConnectionMgr->>DB: update ConnectionSyncJob(FAILED)<br/>record errorMessage
        alt Retry available
            ConnectionMgr->>Queue: requeue job
        else Max attempts exceeded
            ConnectionMgr->>DB: mark job FAILED
        end
    end
Loading
sequenceDiagram
    participant RepoSource as Repo Source<br/>(GitHub/GitLab/etc)
    participant Compile as Compile*Config
    participant Source as getX*FromConfig
    
    Note over RepoSource,Source: Before: { validRepos, notFound }
    Compile->>Source: fetch repos
    Source->>Source: collect validRepos[]
    Source->>Source: collect notFound{ users, orgs, repos }
    Source-->>Compile: { validRepos, notFound }
    Compile-->>Compile: return { repoData, notFound }
    
    Note over RepoSource,Source: After: { repos, warnings }
    Compile->>Source: fetch repos
    Source->>Source: collect repos[]
    Source->>Source: collect warnings[] (404/errors)
    Source-->>Compile: { repos, warnings }
    Compile-->>Compile: return { repoData, warnings }
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–75 minutes

Areas requiring extra attention:

  • Connection job system refactor (packages/backend/src/connectionManager.ts): New groupMQ-based queue, job lifecycle handlers (onJobCompleted, onJobFailed, onJobStalled), and timeout/retry logic require careful validation of job state transitions and error handling.
  • Declarative configuration manager (packages/backend/src/configManager.ts): File watching, incremental syncs, and connection lifecycle (create/update/delete) need verification for edge cases (concurrent file changes, partial syncs).
  • Repository source return shape unification (packages/backend/src/{github,gitlab,gitea,bitbucket,azuredevops}.ts): Systematic change from notFound structures to warnings arrays across 5+ integrations; ensure all call sites in repoCompileUtils.ts handle the new shapes correctly.
  • Compile result type propagation (packages/backend/src/repoCompileUtils.ts): All 8+ compile*Config functions now return CompileResult; verify downstream consumers in connectionManager.ts extract and use warnings correctly.
  • GitHub App configuration schema consolidation (schema/type files): Moved from separate githubApp.* files into app.* definitions; ensure all imports and schema references are updated (deleted files may break external consumers).
  • UI first-time sync detection logic (packages/web/src/app/[domain]/repos/page.tsx, connectionsTable.tsx): Calculation of isFirstTimeIndex based on indexedAt: null and job status requires testing across various sync states.
  • Settings page access control (packages/web/src/app/[domain]/settings/connections/layout.tsx): OWNER-only guard and org membership validation must be thoroughly tested.

Possibly related PRs

Suggested labels

sourcebot-team

Suggested reviewers

  • msukkari

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.55% 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 "feat(worker,web): Improved connection management" is directly related to the central theme of this pull request. The changeset introduces a comprehensive overhaul of connection management across the worker (backend) and web packages, including a new ConnectionSyncJob table for tracking job status, a ConfigManager for declarative connection synchronization, groupmq-based job queueing, and new UI components for connection management. While the title could be more specific about these technical details (such as "job tracking" or "queue-based synchronization"), it does clearly summarize the primary change: the connection management system has been significantly improved. A teammate reviewing the git history would understand that connection-related functionality has been enhanced.

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 29, 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: 26

Caution

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

⚠️ Outside diff range comments (11)
docs/snippets/schemas/v3/index.schema.mdx (1)

4283-4407: Schema duplication in apps configuration.

The GitHubAppConfig schema is defined twice: once under definitions.GitHubAppConfig (lines 4284-4343) and again inline within the oneOf array (lines 4346-4405). This duplication violates the DRY principle and creates a maintenance burden.

In standard JSON Schema practice, definitions should be referenced using $ref rather than duplicated. The oneOf should reference the definition like this:

"oneOf": [
  {
    "$ref": "#/properties/apps/items/definitions/GitHubAppConfig"
  }
]

Since this is an auto-generated file, the schema generator should be updated to emit $ref references instead of duplicating the schema definition.

packages/schemas/src/v3/app.schema.ts (1)

5-128: Schema duplication between definitions and oneOf.

The GitHubAppConfig schema is fully defined in definitions.GitHubAppConfig (lines 5-65) and then duplicated verbatim in the oneOf array (lines 67-128). This creates a maintenance issue where changes must be made in two places.

The standard JSON Schema pattern uses $ref to reference definitions:

  "oneOf": [
    {
-     "type": "object",
-     "properties": { ... },
-     "required": [...],
-     "additionalProperties": false
+     "$ref": "#/definitions/GitHubAppConfig"
    }
  ]

Since this is an auto-generated file (line 1), the schema generator should be updated to emit proper $ref references instead of duplicating schema definitions.

docs/snippets/schemas/v3/app.schema.mdx (1)

6-129: Schema duplication between definitions and oneOf.

The GitHubAppConfig schema is defined in definitions.GitHubAppConfig (lines 6-66) and then duplicated in the oneOf array (lines 68-129). This duplication creates unnecessary maintenance overhead.

Following JSON Schema best practices, the oneOf should reference the definition using $ref:

"oneOf": [
  {
    "$ref": "#/definitions/GitHubAppConfig"
  }
]

Since this is auto-generated (line 1), the underlying schema generator needs to be updated to produce proper $ref references instead of duplicating schema content.

packages/web/src/app/[domain]/repos/[id]/page.tsx (2)

23-29: Fix Next.js page params typing and dynamic back link (prevents route validation failures).

  • params is not a Promise in App Router; remove await and correct type.
  • Use the dynamic domain from params to build the BackButton href.
-export default async function RepoDetailPage({ params }: { params: Promise<{ id: string }> }) {
-    const { id } = await params
+export default async function RepoDetailPage({
+  params,
+}: { params: { domain: string; id: string } }) {
+  const { id, domain } = params;
@@
-                <BackButton
-                    href={`/${SINGLE_TENANT_ORG_DOMAIN}/repos`}
+                <BackButton
+                    href={`/${domain}/repos`}
                     name="Back to repositories"
                     className="mb-2"
                 />

As per Next.js 15 route typing.

Also applies to: 55-62


183-206: Prisma: findUnique with additional filters is invalid; use findFirst with where on both fields.

Unless you have a composite unique on (id, orgId), this will fail type-check or at runtime.

-const getRepoWithJobs = async (repoId: number) => sew(() =>
-    withOptionalAuthV2(async ({ prisma, org }) => {
-
-        const repo = await prisma.repo.findUnique({
-            where: {
-                id: repoId,
-                orgId: org.id,
-            },
+const getRepoWithJobs = async (repoId: number) => sew(() =>
+  withOptionalAuthV2(async ({ prisma, org }) => {
+    const repo = await prisma.repo.findFirst({
+      where: {
+        id: repoId,
+        orgId: org.id,
+      },
             include: {
                 jobs: {
                     orderBy: { createdAt: 'desc' },
                 }
             },
-        });
+    });

If you do have a composite unique on (id, orgId), switch to that unique alias in findUnique’s where accordingly.

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

15-27: Prisma: findUnique cannot include non-unique filters; switch to findFirst with combined where.

Current query won’t validate if role condition is present.

-  const guestUser = await prisma.userToOrg.findUnique({
-    where: {
-      orgId_userId: {
-        orgId: SINGLE_TENANT_ORG_ID,
-        userId: SOURCEBOT_GUEST_USER_ID,
-      },
-      role: {
-        not: OrgRole.GUEST,
-      }
-    },
-  });
+  const guestUser = await prisma.userToOrg.findFirst({
+    where: {
+      orgId: SINGLE_TENANT_ORG_ID,
+      userId: SOURCEBOT_GUEST_USER_ID,
+      role: { not: OrgRole.GUEST },
+    },
+  });

If you do have a composite unique alias orgId_userId, reserve findUnique for exact matches without extra predicates.

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

78-91: Add a reentrancy guard for cleanup to prevent concurrent shutdowns

cleanup() can be called from SIGINT, SIGTERM, uncaughtException, and unhandledRejection. Guard once to avoid double-dispose:

+let shuttingDown = false
 const cleanup = async (signal: string) => {
+  if (shuttingDown) return
+  shuttingDown = true
   logger.info(`Received ${signal}, cleaning up...`);
   const shutdownTimeout = 30000;
   try {
     await Promise.race([
       Promise.all([
         repoIndexManager.dispose(),
         connectionManager.dispose(),
         repoPermissionSyncer.dispose(),
         userPermissionSyncer.dispose(),
         promClient.dispose(),
         configManager.dispose(),
       ]),
       new Promise((_, reject) =>
         setTimeout(() => reject(new Error('Shutdown timeout')), shutdownTimeout)
       )
     ]);
packages/web/src/app/[domain]/repos/components/reposTable.tsx (1)

321-338: Fix filtering for “No status” (null) and header typo.

  • The Select sets "null" but default filtering won’t match row values that are actually null.
  • Also, header says “Lastest status”.

Apply:

   {
     accessorKey: "latestJobStatus",
     size: 150,
-    header: "Lastest status",
+    header: "Latest status",
     cell: ({ row }) => getStatusBadge(row.getValue("latestJobStatus")),
+    // Ensure filtering handles null (no jobs) and exact matches
+    filterFn: (row, id, filterValue) => {
+      const v = row.getValue(id) as Repo["latestJobStatus"];
+      if (!filterValue) return true;
+      if (filterValue === "null") return v == null;
+      return v === filterValue;
+    },
   },

And update the Select handler:

   <Select
-    value={(table.getColumn("latestJobStatus")?.getFilterValue() as string) ?? "all"}
+    value={(table.getColumn("latestJobStatus")?.getFilterValue() as string) ?? "all"}
     onValueChange={(value) => {
-      table.getColumn("latestJobStatus")?.setFilterValue(value === "all" ? "" : value)
+      table.getColumn("latestJobStatus")?.setFilterValue(value === "all" ? undefined : value)
     }}
   >

Also applies to: 147-151

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

1766-1813: Fix SSRF + token leakage when proxying repo images. Correct implementation bugs in the proposed diff.

The security concern is valid—unvalidated repo.imageUrl + authentication headers = SSRF + potential token leakage to untrusted hosts. The proposed fix's approach (host allowlist, protocol checks, timeout, size limits) is sound, but the implementation has bugs:

  1. (await new Blob(chunks)) – Blob constructor is not async; should be await new Blob(chunks).arrayBuffer()
  2. new URL(repo.imageUrl) can throw TypeError but isn't wrapped in try-catch – will crash instead of returning notFound()

Corrected diff:

         try {
+            // Basic URL validation
+            let urlObj: URL;
+            try {
+                urlObj = new URL(repo.imageUrl);
+            } catch {
+                logger.warn(`Invalid image URL for repo ${repoId}`);
+                return notFound();
+            }
+            if (!['https:', 'http:'].includes(urlObj.protocol)) {
+                logger.warn(`Blocked non-http(s) URL for repo ${repoId}`);
+                return notFound();
+            }
+            // Allow only known code-host domains for this repo's connections
+            const allowedHosts = new Set<string>();
+            for (const { connection } of repo.connections) {
+                if (connection.connectionType === 'github') allowedHosts.add('github.com');
+                if (connection.connectionType === 'gitlab') {
+                    const c = connection.config as unknown as GitlabConnectionConfig;
+                    allowedHosts.add(c.url ? new URL(c.url).hostname : 'gitlab.com');
+                }
+                if (connection.connectionType === 'gitea') {
+                    const c = connection.config as unknown as GiteaConnectionConfig;
+                    if (c.url) allowedHosts.add(new URL(c.url).hostname);
+                }
+            }
+            if (!allowedHosts.has(urlObj.hostname)) {
+                logger.warn(`Blocked image host ${urlObj.hostname} for repo ${repoId}`);
+                return notFound();
+            }
+            // Timeout + size guard
+            const ac = new AbortController();
+            const t = setTimeout(() => ac.abort(), 8000);
             const response = await fetch(repo.imageUrl, {
                 headers: authHeaders,
+                signal: ac.signal,
-            });
+            }).finally(() => clearTimeout(t));
             if (!response.ok) {
                 logger.warn(`Failed to fetch image from ${repo.imageUrl}: ${response.status}`);
                 return notFound();
             }
-            const imageBuffer = await response.arrayBuffer();
+            const reader = response.body?.getReader();
+            if (!reader) return notFound();
+            const chunks: Uint8Array[] = [];
+            let received = 0;
+            const MAX = 2 * 1024 * 1024; // 2MB
+            while (true) {
+                const { done, value } = await reader.read();
+                if (done) break;
+                received += value.byteLength;
+                if (received > MAX) {
+                    logger.warn(`Image too large (${received} bytes) from ${repo.imageUrl}`);
+                    return notFound();
+                }
+                chunks.push(value);
+            }
+            const imageBuffer = await new Blob(chunks).arrayBuffer();
             return imageBuffer;
         } catch (error) {
packages/backend/src/gitea.ts (2)

53-60: Duplicate filtering prevents warning from ever logging

You filter out repos with undefined full_name twice; the first filter removes them, so the subsequent warning never triggers. Merge into a single filter with logging.

-    allRepos = allRepos.filter(repo => repo.full_name !== undefined);
-    allRepos = allRepos.filter(repo => {
-        if (repo.full_name === undefined) {
-            logger.warn(`Repository with undefined full_name found: orgId=${orgId}, repoId=${repo.id}`);
-            return false;
-        }
-        return true;
-    });
+    allRepos = allRepos.filter(repo => {
+        if (repo.full_name == null) {
+            logger.warn(`Repository with undefined full_name found: orgId=${orgId}, repoId=${repo.id}`);
+            return false;
+        }
+        return true;
+    });

207-211: Validate repo identifier format before calling the API

If repo isn’t “owner/name”, owner or name becomes undefined and yields a hard error rather than a friendly warning (Bitbucket path handles this).

-            const [owner, repoName] = repo.split('/');
+            const [owner, repoName] = repo.split('/');
+            if (!owner || !repoName) {
+                const warning = `Invalid repo ${repo}`;
+                logger.warn(warning);
+                return {
+                    type: 'warning' as const,
+                    warning,
+                };
+            }
🧹 Nitpick comments (50)
packages/backend/src/ee/syncSearchContexts.ts (1)

38-70: Consider deduplicating repos when using both include and includeConnections.

When both filtering criteria are specified, the same repo can appear multiple times in newReposInContext after the concatenation on line 68. While Prisma's connect typically handles duplicate IDs gracefully, it's more efficient to deduplicate before the upsert operations.

Apply this diff to deduplicate repos:

             for (const connection of connections) {
                 newReposInContext = newReposInContext.concat(connection.repos.map(repo => repo.repo));
             }
+
+            // Deduplicate repos by ID
+            const uniqueRepoMap = new Map(newReposInContext.map(repo => [repo.id, repo]));
+            newReposInContext = Array.from(uniqueRepoMap.values());
         }
packages/web/src/app/api/(server)/stripe/route.ts (1)

63-96: Consider handling additional subscription event types.

The webhook currently handles only customer.subscription.created and customer.subscription.deleted. Consider handling customer.subscription.updated to capture status changes like trial ending, plan changes, or transitions to past_due, unpaid, or canceled states. This would ensure the organization's subscription status remains synchronized with Stripe throughout the entire subscription lifecycle.

Example implementation for handling subscription updates:

else if (event.type === 'customer.subscription.updated') {
    const subscription = event.data.object as Stripe.Subscription;
    const customerId = subscription.customer as string;

    const org = await prisma.org.findFirst({
        where: {
            stripeCustomerId: customerId
        }
    });

    if (!org) {
        return new Response('Org not found', { status: 404 });
    }

    const status = subscription.status === 'active' 
        ? StripeSubscriptionStatus.ACTIVE 
        : StripeSubscriptionStatus.INACTIVE;

    await prisma.org.update({
        where: {
            id: org.id
        },
        data: {
            stripeSubscriptionStatus: status,
            stripeLastUpdatedAt: new Date()
        }
    });
    
    logger.info(`Org ${org.id} subscription status updated to ${status}`);

    return new Response(JSON.stringify({ received: true }), {
        status: 200
    });
}
packages/schemas/src/v3/index.type.ts (1)

1073-1102: Tighten “id” semantics and fix “token” wording for privateKey.

  • Clarify that id is the GitHub App ID (numeric). Consider enforcing a numeric string (or number) in the schema/types.
  • The generated doc under privateKey mentions “token”; switch wording to “private key”.

Apply via generator (don’t edit this file directly):

 export interface GitHubAppConfig {
@@
-  /**
-   * The ID of the GitHub App.
-   */
-  id: string;
+  /**
+   * The GitHub App ID (numeric).
+   */
+  id: string; // consider constraining to numeric string in schema
@@
-        /**
-         * The name of the secret that contains the token.
-         */
+        /**
+         * The name of the secret that contains the private key.
+         */
@@
-        /**
-         * The name of the environment variable that contains the token. Only supported in declarative connection configs.
-         */
+        /**
+         * The name of the environment variable that contains the private key. Only supported in declarative connection configs.
+         */

Optionally allow id to come from env/secret (string | Token) if desired for config parity.

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

4282-4342: Avoid duplication: reference the definition from oneOf to prevent drift.

You define GitHubAppConfig under definitions then re-inline the same shape in oneOf. This will diverge over time.

Apply via generator (don’t edit this file directly). Replace the inlined oneOf object with a $ref to the local definition:

 "oneOf": [
-  {
-    "type": "object",
-    "properties": {
-      "type": { "const": "githubApp", "description": "GitHub App Configuration" },
-      "deploymentHostname": { "type": "string", "format": "hostname", "default": "github.com", ... },
-      "id": { "type": "string", "description": "The ID of the GitHub App." },
-      "privateKey": { "anyOf": [ { ...secret... }, { ...env... } ], "description": "The private key of the GitHub App." }
-    },
-    "required": ["type", "id", "privateKey"],
-    "additionalProperties": false
-  }
+  {
+    "$ref": "#/properties/apps/items/definitions/GitHubAppConfig"
+  }
 ]

Also consider:

  • Constrain id to a numeric string: "id": { "type": "string", "pattern": "^[0-9]+$", "description": "GitHub App ID (numeric)." }
  • Optional: allow id via env/secret for config parity (anyOf: [ {type:string,pattern:^[0-9]+$}, { $ref: "./shared.json#/definitions/Token"} ]).

Also applies to: 4345-4402

schemas/v3/app.json (1)

4-38: Schema looks solid; add numeric pattern for id and fix privateKey wording.

  • Enforce numeric App ID and correct “token” wording for the private key.
       "id": {
-        "type": "string",
-        "description": "The ID of the GitHub App."
+        "type": "string",
+        "pattern": "^[0-9]+$",
+        "description": "GitHub App ID (numeric)."
       },
       "privateKey": {
-        "$ref": "./shared.json#/definitions/Token",
-        "description": "The private key of the GitHub App."
+        "$ref": "./shared.json#/definitions/Token",
+        "description": "The private key of the GitHub App."
       }

Optional: permit id via env/secret for deployment flexibility:

-      "id": { "type": "string", "pattern": "^[0-9]+$", "description": "GitHub App ID (numeric)." },
+      "id": {
+        "anyOf": [
+          { "type": "string", "pattern": "^[0-9]+$" },
+          { "$ref": "./shared.json#/definitions/Token" }
+        ],
+        "description": "GitHub App ID (numeric)."
+      },
packages/web/src/app/[domain]/chat/components/demoCards.tsx (1)

43-61: Preserve provider icon classes in selected state; avoid brittle cast.

  • Selected path drops provider-specific classes (e.g., rounding/filters) by replacing them with invert-only styles.
  • Casting to CodeHostType trusts upstream data; add a minimal runtime guard or preserve default class to avoid regressions.

Apply:

-            const codeHostIcon = getCodeHostIcon(searchScope.codeHostType as CodeHostType);
-            const selectedIconClass = isSelected
-                ? "invert dark:invert-0"
-                : codeHostIcon.className;
+            const codeHostIcon = getCodeHostIcon(searchScope.codeHostType as CodeHostType);
+            const iconClass = cn(
+              codeHostIcon.className,
+              isSelected && "invert dark:invert-0"
+            );

             return (
                 <Image
                     src={codeHostIcon.src}
                     alt={`${searchScope.codeHostType} icon`}
                     width={size}
                     height={size}
-                    className={cn(sizeClass, selectedIconClass)}
+                    className={cn(sizeClass, iconClass)}
                 />
             );

Please confirm getCodeHostIcon is exhaustive for all possible searchScope.codeHostType values and returns a non-null fallback.

packages/web/src/app/[domain]/repos/page.tsx (1)

15-29: Compute from the single “latest job” instead of filtering.

You already fetch at most one job (take: 1). Simplify and avoid extra allocations.

-    const repos = _repos
-        .map((repo) => ({
-            ...repo,
-            latestJobStatus: repo.jobs.length > 0 ? repo.jobs[0].status : null,
-            isFirstTimeIndex: repo.indexedAt === null && repo.jobs.filter((job) => job.status === RepoIndexingJobStatus.PENDING || job.status === RepoIndexingJobStatus.IN_PROGRESS).length > 0,
-        }))
+    const repos = _repos
+        .map((repo) => {
+            const latest = repo.jobs[0];
+            const latestStatus = latest?.status ?? null;
+            const isFirstTimeIndex =
+                repo.indexedAt === null &&
+                (latestStatus === RepoIndexingJobStatus.PENDING ||
+                 latestStatus === RepoIndexingJobStatus.IN_PROGRESS);
+            return { ...repo, latestJobStatus: latestStatus, isFirstTimeIndex };
+        })
         .sort((a, b) => {
           if (a.isFirstTimeIndex && !b.isFirstTimeIndex) return -1;
           if (!a.isFirstTimeIndex && b.isFirstTimeIndex) return 1;
           return a.name.localeCompare(b.name);
         });
packages/web/src/app/[domain]/repos/[id]/page.tsx (1)

52-53: Defensive parse for repo.metadata.

repo.metadata may be null. Safe default avoids runtime parse errors.

-    const repoMetadata = repoMetadataSchema.parse(repo.metadata);
+    const repoMetadata = repoMetadataSchema.parse(repo.metadata ?? {});

Confirm repo.metadata is always non-null in your schema; if so, ignore.

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

67-94: Sync disable state from config too (not only enable).

Currently, enablePublicAccess=false does nothing if entitlement exists; metadata may remain true unintentionally.

-    if (env.CONFIG_PATH) {
+    if (env.CONFIG_PATH) {
         const config = await loadConfig(env.CONFIG_PATH);
-        const forceEnableAnonymousAccess = config.settings?.enablePublicAccess ?? env.FORCE_ENABLE_ANONYMOUS_ACCESS === 'true';
+        const forceEnableAnonymousAccess =
+          (config.settings?.enablePublicAccess ??
+           (env.FORCE_ENABLE_ANONYMOUS_ACCESS === 'true'));
 
         if (forceEnableAnonymousAccess) {
           if (!hasAnonymousAccessEntitlement) {
             logger.warn(`FORCE_ENABLE_ANONYMOUS_ACCESS env var is set to true but anonymous access entitlement is not available. Setting will be ignored.`);
           } else {
             const org = await getOrgFromDomain(SINGLE_TENANT_ORG_DOMAIN);
             if (org) {
               const currentMetadata = getOrgMetadata(org);
               const mergedMetadata = {
                 ...(currentMetadata ?? {}),
                 anonymousAccessEnabled: true,
               };
 
               await prisma.org.update({
                 where: { id: org.id },
                 data: { metadata: mergedMetadata },
               });
               logger.info(`Anonymous access enabled via FORCE_ENABLE_ANONYMOUS_ACCESS environment variable`);
             }
           }
+        } else {
+          // Explicitly disable when config/env says so
+          const org = await getOrgFromDomain(SINGLE_TENANT_ORG_DOMAIN);
+          if (org) {
+            const currentMetadata = getOrgMetadata(org);
+            const mergedMetadata = { ...(currentMetadata ?? {}), anonymousAccessEnabled: false };
+            await prisma.org.update({
+              where: { id: org.id },
+              data: { metadata: mergedMetadata },
+            });
+            logger.info(`Anonymous access disabled per configuration`);
+          }
         }
     }

Confirm desired precedence between config.settings.enablePublicAccess and FORCE_ENABLE_ANONYMOUS_ACCESS; adjust the coalesce order accordingly.

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

8-14: Apply exhaustive union handling for future-proof type safety.

The downstream API change verification passed—no references to NotFoundResult or notFoundItems exist in the codebase, so callers have already been updated or never relied on those types.

However, the current implementation handles the union via implicit else logic. Applying the exhaustive switch pattern with assertNever is a recommended best practice to prevent runtime errors if new union members are added later:

 type CustomResult<T> = ValidResult<T> | WarningResult;

 export function processPromiseResults<T>(
   results: PromiseSettledResult<CustomResult<T>>[],
 ): {
   validItems: T[];
   warnings: string[];
 } {
   const validItems: T[] = [];
   const warnings: string[] = [];

-  results.forEach(result => {
-    if (result.status === 'fulfilled') {
-      const value = result.value;
-      if (value.type === 'valid') {
-        validItems.push(...value.data);
-      } else {
-        warnings.push(value.warning);
-      }
-    }
-  });
+  results.forEach((result) => {
+    if (result.status !== "fulfilled") return;
+    const value = result.value;
+    switch (value.type) {
+      case "valid":
+        validItems.push(...value.data);
+        break;
+      case "warning":
+        warnings.push(value.warning);
+        break;
+      default:
+        assertNever(value);
+    }
+  });

   return { validItems, warnings };
 }
+
+function assertNever(_x: never): never {
+  throw new Error("Unhandled result type");
+}
packages/db/prisma/schema.prisma (2)

158-170: Add indexes for common queries (by connection + createdAt, and by status).

UI lists jobs per connection sorted by time and filters by status. Add indexes to avoid seq scans.

 model ConnectionSyncJob {
   id          String                      @id @default(cuid())
   status      ConnectionSyncJobStatus @default(PENDING)
   createdAt   DateTime                    @default(now())
   updatedAt   DateTime                    @updatedAt
   completedAt DateTime?

-  warningMessages String[]
+  warningMessages String[]

   errorMessage String?

   connection   Connection @relation(fields: [connectionId], references: [id], onDelete: Cascade)
   connectionId Int
+
+  @@index([connectionId, createdAt], map: "idx_conn_sync_job_conn_created")
+  @@index([status], map: "idx_conn_sync_job_status")
 }

13-20: Remove unused legacy enum ConnectionSyncStatus (and drop DB type in migration).

This appears orphaned after switching to job-based sync. Keeping it invites confusion.

If confirmed unused, remove the enum and add a migration that drops the old enum type from Postgres (after ensuring no remaining columns reference it).

packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx (3)

81-97: Use a single TooltipProvider and add accessible labels for icon triggers.

Reduce provider duplication and improve a11y. Add aria-labels for the icon-only triggers.

-                    {job.errorMessage ? (
-                        <TooltipProvider>
-                            <Tooltip>
+                    {job.errorMessage ? (
+                            <Tooltip>
                                 <TooltipTrigger>
-                                    <AlertCircle className="h-4 w-4 text-destructive" />
+                                    <AlertCircle aria-label="View error details" className="h-4 w-4 text-destructive" />
                                 </TooltipTrigger>
                                 <TooltipContent className="max-w-[750px] max-h-96 overflow-scroll p-4">
                                   {/* … */}
                                 </TooltipContent>
-                            </Tooltip>
-                        </TooltipProvider>
+                            </Tooltip>
                     ) : job.warningMessages.length > 0 ? (
-                        <TooltipProvider>
-                            <Tooltip>
+                            <Tooltip>
                                 <TooltipTrigger>
-                                    <AlertTriangle className="h-4 w-4 text-warning" />
+                                    <AlertTriangle aria-label="View sync warnings" className="h-4 w-4 text-warning" />
                                 </TooltipTrigger>
                                 <TooltipContent className="max-w-[750px] max-h-96 overflow-scroll p-4">
                                   {/* … */}
                                 </TooltipContent>
-                            </Tooltip>
-                        </TooltipProvider>
+                            </Tooltip>
                     ) : null}

Wrap once near the table:

-        <div className="w-full">
+        <TooltipProvider>
+        <div className="w-full">
          {/* …table… */}
-        </div>
+        </div>
+        </TooltipProvider>

Also applies to: 98-119, 256-289


101-113: Verify Tailwind token “text-warning” exists.

ShadCN defaults don’t include text-warning. If not defined in your theme, switch to a semantic token you have (e.g., text-amber-500 dark:text-amber-400) or add the token.


172-178: Clipboard UX: add aria-live toast or label for screen readers.

The copy icon relies on visual affordance only. Consider adding aria-label="Copy job ID" on the button or emitting a success toast with role="status".

packages/backend/package.json (1)

43-43: Confirm Chokidar v4 compatibility and typing; ensure watcher lifecycle hygiene.

  • If using TypeScript, verify v4 ships types or add @types/chokidar.
  • Ensure Node runtime meets chokidar v4 requirements.
  • Close watchers on shutdown and ignore large dirs (e.g., .git, cache) to avoid leaks and CPU churn.
packages/backend/src/constants.ts (1)

4-4: Avoid magic ID; document or make configurable.

Tying single-tenant flows to 1 works with the migration but is brittle. Prefer reading from env (e.g., SINGLE_TENANT_ORG_ID) with a sane default, or clearly document this contract here.

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

57-57: Order-dependent shutdown: avoid racing disposals if there are dependencies

ConfigManager likely watches and mutates connections; disposing it in parallel with ConnectionManager can race. Prefer sequencing (configManager → repoIndexManager/permission syncers → connectionManager → promClient), or ensure disposals are idempotent and independent. Example:

-        await Promise.all([
-          repoIndexManager.dispose(),
-          connectionManager.dispose(),
-          repoPermissionSyncer.dispose(),
-          userPermissionSyncer.dispose(),
-          promClient.dispose(),
-          configManager.dispose(),
-        ])
+        // dispose config first if it depends on connectionManager being alive
+        await configManager.dispose().catch(logger.warn)
+        await Promise.all([
+          repoIndexManager.dispose(),
+          repoPermissionSyncer.dispose(),
+          userPermissionSyncer.dispose(),
+          promClient.dispose(),
+          connectionManager.dispose(),
+        ])
packages/backend/src/utils.ts (2)

139-154: Token resolution calls: OK; consider explicit usernames for GitHub PAT

The new getTokenFromConfig(db) calls are correct. For GitHub PATs, pass a username ("x-access-token") to ensure Basic auth works consistently across providers (we already do this for GitHub App and GitLab):

- cloneUrlWithToken: createGitCloneUrlWithToken(
-   repo.cloneUrl,
-   {
-     password: token,
-   }
- ),
+ cloneUrlWithToken: createGitCloneUrlWithToken(
+   repo.cloneUrl,
+   {
+     username: 'x-access-token',
+     password: token,
+   }
+ ),

GitLab already uses 'oauth2'; Bitbucket sets username from config/user or 'x-token-auth'. Gitea can remain password-only unless we see auth issues.

Also applies to: 155-170, 171-186, 187-206


94-101: Minor: HTTP status 443 is invalid; use 503 for transient server errors

In fetchWithRetry(), e.status === 443 looks like a port number typo. Replace with 503 (or include 500/502/503 as retryable):

- if ((e.status === 403 || e.status === 429 || e.status === 443) && attempts < maxAttempts) {
+ if (([403, 429, 500, 502, 503].includes(e.status)) && attempts < maxAttempts) {
packages/backend/src/ee/githubAppManager.ts (1)

56-66: Avoid hard-coded org id for private key resolution

SINGLE_TENANT_ORG_ID = 1 will break in multi-tenant scenarios. Make it configurable or derive from config/env:

- const SINGLE_TENANT_ORG_ID = 1;
- const privateKey = await getTokenFromConfig(app.privateKey, SINGLE_TENANT_ORG_ID, this.db);
+ const orgIdForKms = Number(process.env.SINGLE_TENANT_ORG_ID ?? 1);
+ const privateKey = await getTokenFromConfig(app.privateKey, orgIdForKms, this.db);
packages/backend/src/repoIndexManager.ts (1)

152-154: Clarify variable naming: this is a Date, not milliseconds

gcGracePeriodMs holds a Date. Rename for clarity:

- const gcGracePeriodMs = new Date(Date.now() - this.settings.repoGarbageCollectionGracePeriodMs);
+ const gcCutoffDate = new Date(Date.now() - this.settings.repoGarbageCollectionGracePeriodMs);
...
- { indexedAt: { lt: gcGracePeriodMs } },
+ { indexedAt: { lt: gcCutoffDate } },

Also applies to: 162-162

packages/web/src/app/[domain]/components/notificationDot.tsx (1)

7-9: Add aria-hidden for decorative indicator (a11y)

If this dot is purely decorative, hide it from screen readers:

-export const NotificationDot = ({ className }: NotificationDotProps) => {
-    return <div className={cn("w-2 h-2 rounded-full bg-green-600", className)} />
-}
+export const NotificationDot = ({ className }: NotificationDotProps) => {
+  return <div aria-hidden="true" className={cn("w-2 h-2 rounded-full bg-green-600", className)} />
+}
packages/backend/src/azuredevops.ts (2)

166-235: Treat 401/403 like 404 to produce warnings instead of hard-failing

Currently only 404 becomes a warning. Adding 401/403 covers “not found or no access” uniformly:

- if (error && typeof error === 'object' && 'statusCode' in error && error.statusCode === 404) {
+ if (error && typeof error === 'object' && 'statusCode' in error && [401,403,404].includes((error as any).statusCode)) {

Apply in getReposForOrganizations, getReposForProjects, and getRepos.

Also applies to: 237-289, 291-344


30-35: Explicit return type to lock the contract

Annotate the return type for getAzureDevOpsReposFromConfig to prevent accidental regressions:

-export const getAzureDevOpsReposFromConfig = async (...) => {
+export const getAzureDevOpsReposFromConfig = async (...): Promise<{ repos: GitRepository[]; warnings: string[] }> => {

Also applies to: 86-101

packages/web/src/app/[domain]/repos/components/reposTable.tsx (3)

104-110: Alt text can be “null logo” when displayName is null.

Use a safe fallback:

- alt={`${repo.displayName} logo`}
+ alt={`${repo.displayName || repo.name} logo`}

199-205: Use for external commit URLs and open in a new tab.

- const HashComponent = commitUrl ? (
-   <Link
-     href={commitUrl}
-     className="font-mono text-sm text-link hover:underline"
-   >
-     {smallHash}
-   </Link>
- ) : (
+ const HashComponent = commitUrl ? (
+   <a
+     href={commitUrl}
+     target="_blank"
+     rel="noopener noreferrer"
+     className="font-mono text-sm text-link hover:underline"
+   >
+     {smallHash}
+   </a>
+ ) : (

396-399: Pluralization should depend on filtered count.

- {table.getFilteredRowModel().rows.length} {data.length > 1 ? 'repositories' : 'repository'} total
+ {table.getFilteredRowModel().rows.length} {table.getFilteredRowModel().rows.length === 1 ? 'repository' : 'repositories'} total
packages/web/src/app/[domain]/settings/components/sidebar-nav.tsx (2)

33-39: Harden regex handling and improve a11y.

Guard against invalid regex strings and set aria-current:

- const isActive = item.hrefRegex ? new RegExp(item.hrefRegex).test(pathname) : pathname === item.href;
+ let isActive = pathname === item.href;
+ if (item.hrefRegex) {
+   try {
+     isActive = new RegExp(item.hrefRegex).test(pathname);
+   } catch {
+     // ignore invalid regex; fall back to exact match
+   }
+ }
...
- <Link
+ <Link
    key={item.href}
    href={item.href}
+   aria-current={isActive ? "page" : undefined}

Also applies to: 46-49


1-1: Filename style

Consider renaming to sidebarNav.tsx to follow camelCase filename guideline, unless this directory already uses kebab-case. As per coding guidelines.

packages/web/src/app/[domain]/settings/connections/page.tsx (2)

21-23: Minor: simplify job checks since only the latest job is loaded

You take: 1; use optional chaining instead of .filter(...).length > 0 and array index for status.

- isFirstTimeSync: connection.syncedAt === null && connection.syncJobs.filter((job) => job.status === ConnectionSyncJobStatus.PENDING || job.status === ConnectionSyncJobStatus.IN_PROGRESS).length > 0,
- latestJobStatus: connection.syncJobs.length > 0 ? connection.syncJobs[0].status : null,
+ isFirstTimeSync: connection.syncedAt === null && ['PENDING', 'IN_PROGRESS'].includes(connection.syncJobs[0]?.status ?? ''),
+ latestJobStatus: connection.syncJobs[0]?.status ?? null,

9-12: Prefer dynamic rendering to avoid stale job status

These statuses change frequently. Explicitly mark the page dynamic in Next.js 15 to prevent accidental static optimization.

 const DOCS_URL = "https://docs.sourcebot.dev/docs/connections/overview";
 
+export const dynamic = "force-dynamic";
+
 export default async function ConnectionsPage({ params }: { params: { domain: string }}) {
packages/web/src/app/[domain]/settings/layout.tsx (2)

66-70: Avoid unnecessary fetch for non‑owners

You only show the “Connections” item to OWNERS. Fetch connectionStats only for owners to save a DB round‑trip.

-const connectionStats = await getConnectionStats();
-if (isServiceError(connectionStats)) {
-  throw new ServiceErrorException(connectionStats);
-}
+const connectionStats = userRoleInOrg === OrgRole.OWNER ? await getConnectionStats() : null;
+if (connectionStats && isServiceError(connectionStats)) {
+  throw new ServiceErrorException(connectionStats);
+}

102-108: Confirm hrefRegex type expectation

If SidebarNav expects a RegExp, passing a string could cause mismatches. Consider supplying a RegExp:

- hrefRegex: `/${domain}/settings/connections(\/[^/]+)?$`,
+ hrefRegex: new RegExp(`^/${domain}/settings/connections(/[^/]+)?$`),

Please confirm SidebarNavItem['hrefRegex'] accepts RegExp. If it already parses strings, ignore.

packages/web/src/app/[domain]/components/navigationMenu/navigationItems.tsx (1)

61-66: Add accessible labels to notification dots

Provide an aria-label or title so screen readers convey why the dot is shown.

-{isReposButtonNotificationDotVisible && <NotificationDot className="absolute -right-0.5 -top-0.5" />}
+{isReposButtonNotificationDotVisible && (
+  <NotificationDot className="absolute -right-0.5 -top-0.5" aria-label="Repository indexing in progress" />
+)}
...
-{isSettingsButtonNotificationDotVisible && <NotificationDot className="absolute -right-0.5 -top-0.5" />}
+{isSettingsButtonNotificationDotVisible && (
+  <NotificationDot className="absolute -right-0.5 -top-0.5" aria-label="Connection sync in progress" />
+)}

Also applies to: 76-79

packages/web/src/app/[domain]/settings/connections/components/connectionsTable.tsx (1)

117-117: Typo: “Lastest status” → “Latest status”

Minor copy fix in the column header.

- header: "Lastest status",
+ header: "Latest status",
packages/backend/src/github.ts (1)

392-412: Case-insensitive topic matching consistency.

You lower-case config topics but not repo topics; GitHub topics can be mixed case. Consider normalizing repo topics too before micromatch.

-const repoTopics = repo.topics ?? [];
+const repoTopics = (repo.topics ?? []).map(t => t.toLowerCase());
packages/backend/src/gitlab.ts (1)

235-256: Normalize topics for matching.

Mirror the GitHub change: lower-case project topics before micromatch to avoid case mismatches.

-const projectTopics = project.topics ?? [];
+const projectTopics = (project.topics ?? []).map(t => t.toLowerCase());
packages/web/src/actions.ts (1)

579-609: Minor: consider counting pending/in-progress INDEX jobs regardless of indexedAt=null.

Edge: a repo could start an INDEX job after being indexed (reindex). If you only want first-time indexing, current filter is correct; else drop indexedAt:null.

packages/backend/src/connectionManager.ts (2)

315-343: Sanitize logged errors to avoid leaking secrets.

job.failedReason may include provider error messages with tokens/URLs. Consider redaction before logging/emitting.


150-156: Pass abortController to all compile* function calls for consistency, or remove it if unused.

The abortController is created at line 151 but only passed to compileGithubConfig (line 175). The other five compile functions—compileGitlabConfig, compileGiteaConfig, compileGerritConfig, compileBitbucketConfig, and compileAzureDevOpsConfig—do not accept an abortController parameter and cannot receive it currently. The suggested diff requires updating function signatures in packages/backend/src/repoCompileUtils.ts to accept the new parameter and updating all call sites in the switch block to pass it consistently.

packages/backend/src/configManager.ts (3)

29-39: Add a simple concurrency guard to coalesce rapid file changes.

Back-to-back “change” events can race. Guard to ensure one in-flight sync and coalesce pending changes.

-        this.watcher.on('change', async () => {
+        let syncing = false, pending = false;
+        this.watcher.on('change', async () => {
             logger.info(`Config file ${configPath} changed. Syncing config.`);
-            try {
-                await this.syncConfig(configPath);
-            } catch (error) {
-                logger.error(`Failed to sync config: ${error}`);
-            }
+            if (syncing) { pending = true; return; }
+            syncing = true;
+            do {
+                pending = false;
+                try {
+                    await this.syncConfig(configPath);
+                } catch (error) {
+                    logger.error(`Failed to sync config: ${error}`);
+                }
+            } while (pending);
+            syncing = false;
         });

66-69: Avoid JSON.stringify for deep equality.

Key order can cause false positives. Prefer a deep-equal or stable-stringify.

- (JSON.stringify(existingConnectionConfig) !== JSON.stringify(newConnectionConfig));
+ (await import('fast-deep-equal')).then(m => !m.default(existingConnectionConfig, newConnectionConfig))

102-121: Use deleteMany for stale declarative connections; log count.

One query is simpler and faster; log the number deleted.

-        const deletedConnections = await this.db.connection.findMany({
+        const deletedConnections = await this.db.connection.findMany({
             where: {
                 isDeclarative: true,
                 name: {
                     notIn: Object.keys(connections ?? {}),
                 },
                 orgId: SINGLE_TENANT_ORG_ID,
             }
         });
-
-        for (const connection of deletedConnections) {
-            logger.info(`Deleting connection with name '${connection.name}'. Connection ID: ${connection.id}`);
-            await this.db.connection.delete({
-                where: {
-                    id: connection.id,
-                }
-            })
-        }
+        const ids = deletedConnections.map(c => c.id);
+        if (ids.length) {
+            await this.db.connection.deleteMany({ where: { id: { in: ids } }});
+            logger.info(`Deleted ${ids.length} declarative connection(s): ${ids.join(', ')}`);
+        }
packages/backend/src/gitea.ts (2)

126-130: Set an explicit page size for user repo pagination (consistency)

Org flow uses limit: 100 but user flow doesn’t. Aligning avoids server defaults variance.

-            const { durationMs, data } = await measure(() =>
-                paginate((page) => api.users.userListRepos(user, {
-                    page,
-                }))
-            );
+            const { durationMs, data } = await measure(() =>
+                paginate((page) => api.users.userListRepos(user, {
+                    page,
+                    limit: 100,
+                }))
+            );

138-146: Avoid sending 404s to Sentry; treat them as warnings only

404s are expected during discovery; capturing them pollutes error monitoring. Only capture non-404.

-        } catch (e: any) {
-            Sentry.captureException(e);
-
-            if (e?.status === 404) {
+        } catch (e: any) {
+            if (e?.status === 404) {
                 const warning = `User ${user} not found or no access`;
                 logger.warn(warning);
                 return {
                     type: 'warning' as const,
                     warning
                 };
             }
+            Sentry.captureException(e);
             throw e;
         }

Repeat the same pattern in getReposForOrgs and getRepos catch blocks.

Also applies to: 179-187, 218-226

packages/backend/src/bitbucket.ts (3)

212-223: Stop capturing expected 404s in Sentry across Bitbucket fetchers

404s during discovery (workspace/project/repo not found) should be warnings, not captured exceptions.

-        } catch (e: any) {
-            Sentry.captureException(e);
-            logger.error(`Failed ...: ${e}`);
-            const status = e?.cause?.response?.status;
-            if (status == 404) {
+        } catch (e: any) {
+            logger.error(`Failed ...: ${e}`);
+            const status = e?.cause?.response?.status;
+            if (status == 404) {
                 const warning = `... not found or invalid access`;
                 logger.warn(warning);
                 return { type: 'warning' as const, warning };
             }
+            Sentry.captureException(e);
             throw e;
         }

Apply in cloudGetReposForWorkspace, cloudGetReposForProjects, cloudGetRepos, serverGetReposForProjects, serverGetRepos.

Also applies to: 275-286, 324-335, 474-485, 523-534


386-392: Don’t send an empty Authorization header for Bitbucket Server

An empty Authorization: header can trigger auth middleware quirks. Omit the header when no creds.

-    const clientOptions: ClientOptions = {
-        baseUrl: url,
-        headers: {
-            Accept: "application/json",
-            Authorization: authorizationString,
-        },
-    };
+    const clientOptions: ClientOptions = {
+        baseUrl: url,
+        headers: {
+            Accept: "application/json",
+            ...(authorizationString ? { Authorization: authorizationString } : {}),
+        },
+    };

352-360: Align exclude.repos semantics with other providers (glob patterns)

Gitea uses micromatch for patterns; here we do exact includes. Consider glob support for parity.

+import micromatch from 'micromatch';
...
-    if (config.exclude?.repos && config.exclude.repos.includes(cloudRepo.full_name!)) {
+    if (config.exclude?.repos && micromatch.isMatch(cloudRepo.full_name!, config.exclude.repos)) {
         return true;
     }

And for server: compare ${projectName}/${repoSlug} with micromatch.

Also applies to: 554-565

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

27-31: Export CompileResult for consumers

Functions export Promise, but the type alias isn’t exported. Export for downstream typing clarity.

-type CompileResult = {
+export type CompileResult = {
     repoData: RepoData[],
     warnings: string[],
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ff88da and 4fd1645.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (61)
  • LICENSE.md (1 hunks)
  • docs/snippets/schemas/v3/app.schema.mdx (2 hunks)
  • docs/snippets/schemas/v3/index.schema.mdx (2 hunks)
  • packages/backend/package.json (1 hunks)
  • packages/backend/src/azuredevops.ts (7 hunks)
  • packages/backend/src/bitbucket.ts (11 hunks)
  • packages/backend/src/configManager.ts (1 hunks)
  • packages/backend/src/connectionManager.ts (4 hunks)
  • packages/backend/src/connectionUtils.ts (1 hunks)
  • packages/backend/src/constants.ts (1 hunks)
  • packages/backend/src/ee/githubAppManager.ts (3 hunks)
  • packages/backend/src/ee/syncSearchContexts.ts (1 hunks)
  • packages/backend/src/env.ts (2 hunks)
  • packages/backend/src/gerrit.ts (0 hunks)
  • packages/backend/src/git.ts (1 hunks)
  • packages/backend/src/gitea.ts (7 hunks)
  • packages/backend/src/github.ts (9 hunks)
  • packages/backend/src/gitlab.ts (8 hunks)
  • packages/backend/src/index.ts (4 hunks)
  • packages/backend/src/repoCompileUtils.ts (12 hunks)
  • packages/backend/src/repoIndexManager.ts (3 hunks)
  • packages/backend/src/utils.ts (6 hunks)
  • packages/db/prisma/migrations/20251026194617_add_connection_job_table/migration.sql (1 hunks)
  • packages/db/prisma/migrations/20251026194628_ensure_single_tenant_org/migration.sql (1 hunks)
  • packages/db/prisma/schema.prisma (1 hunks)
  • packages/schemas/src/v3/app.schema.ts (2 hunks)
  • packages/schemas/src/v3/app.type.ts (1 hunks)
  • packages/schemas/src/v3/githubApp.schema.ts (0 hunks)
  • packages/schemas/src/v3/githubApp.type.ts (0 hunks)
  • packages/schemas/src/v3/index.schema.ts (2 hunks)
  • packages/schemas/src/v3/index.type.ts (2 hunks)
  • packages/shared/src/index.server.ts (0 hunks)
  • packages/web/package.json (0 hunks)
  • packages/web/src/actions.ts (4 hunks)
  • packages/web/src/app/[domain]/chat/components/demoCards.tsx (2 hunks)
  • packages/web/src/app/[domain]/components/backButton.tsx (1 hunks)
  • packages/web/src/app/[domain]/components/navigationMenu/index.tsx (3 hunks)
  • packages/web/src/app/[domain]/components/navigationMenu/navigationItems.tsx (4 hunks)
  • packages/web/src/app/[domain]/components/navigationMenu/progressIndicator.tsx (2 hunks)
  • packages/web/src/app/[domain]/components/notificationDot.tsx (1 hunks)
  • packages/web/src/app/[domain]/components/repositoryCarousel.tsx (1 hunks)
  • packages/web/src/app/[domain]/repos/[id]/page.tsx (7 hunks)
  • packages/web/src/app/[domain]/repos/components/reposTable.tsx (7 hunks)
  • packages/web/src/app/[domain]/repos/layout.tsx (2 hunks)
  • packages/web/src/app/[domain]/repos/page.tsx (3 hunks)
  • packages/web/src/app/[domain]/settings/components/header.tsx (0 hunks)
  • packages/web/src/app/[domain]/settings/components/sidebar-nav.tsx (1 hunks)
  • packages/web/src/app/[domain]/settings/connections/[id]/page.tsx (1 hunks)
  • packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx (1 hunks)
  • packages/web/src/app/[domain]/settings/connections/components/connectionsTable.tsx (1 hunks)
  • packages/web/src/app/[domain]/settings/connections/layout.tsx (1 hunks)
  • packages/web/src/app/[domain]/settings/connections/page.tsx (1 hunks)
  • packages/web/src/app/[domain]/settings/layout.tsx (4 hunks)
  • packages/web/src/app/[domain]/settings/secrets/components/importSecretCard.tsx (1 hunks)
  • packages/web/src/app/api/(server)/stripe/route.ts (1 hunks)
  • packages/web/src/features/chat/components/searchScopeIcon.tsx (2 hunks)
  • packages/web/src/initialize.ts (2 hunks)
  • packages/web/src/lib/syncStatusMetadataSchema.ts (0 hunks)
  • packages/web/src/lib/utils.ts (12 hunks)
  • schemas/v3/app.json (1 hunks)
  • schemas/v3/githubApp.json (0 hunks)
💤 Files with no reviewable changes (8)
  • packages/web/src/app/[domain]/settings/components/header.tsx
  • packages/web/package.json
  • packages/schemas/src/v3/githubApp.schema.ts
  • packages/backend/src/gerrit.ts
  • schemas/v3/githubApp.json
  • packages/web/src/lib/syncStatusMetadataSchema.ts
  • packages/shared/src/index.server.ts
  • packages/schemas/src/v3/githubApp.type.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/syncSearchContexts.ts
  • packages/web/src/app/[domain]/components/repositoryCarousel.tsx
  • packages/backend/src/constants.ts
  • packages/schemas/src/v3/index.type.ts
  • packages/web/src/app/[domain]/settings/connections/components/connectionJobsTable.tsx
  • packages/web/src/app/api/(server)/stripe/route.ts
  • packages/web/src/app/[domain]/settings/components/sidebar-nav.tsx
  • packages/web/src/features/chat/components/searchScopeIcon.tsx
  • packages/backend/src/repoIndexManager.ts
  • packages/web/src/app/[domain]/repos/components/reposTable.tsx
  • packages/web/src/app/[domain]/settings/connections/page.tsx
  • packages/backend/src/utils.ts
  • schemas/v3/app.json
  • LICENSE.md
  • packages/web/src/lib/utils.ts
  • packages/web/src/app/[domain]/components/notificationDot.tsx
  • packages/web/src/app/[domain]/settings/connections/components/connectionsTable.tsx
  • packages/web/src/app/[domain]/components/navigationMenu/navigationItems.tsx
  • packages/web/src/app/[domain]/repos/page.tsx
  • packages/backend/package.json
  • packages/web/src/app/[domain]/repos/[id]/page.tsx
  • packages/backend/src/azuredevops.ts
  • packages/web/src/app/[domain]/settings/connections/[id]/page.tsx
  • packages/backend/src/connectionManager.ts
  • packages/backend/src/configManager.ts
  • packages/db/prisma/migrations/20251026194617_add_connection_job_table/migration.sql
  • packages/backend/src/ee/githubAppManager.ts
  • packages/web/src/app/[domain]/components/navigationMenu/progressIndicator.tsx
  • packages/backend/src/gitlab.ts
  • docs/snippets/schemas/v3/index.schema.mdx
  • packages/schemas/src/v3/app.schema.ts
  • packages/db/prisma/schema.prisma
  • packages/schemas/src/v3/app.type.ts
  • packages/backend/src/connectionUtils.ts
  • packages/backend/src/git.ts
  • packages/web/src/app/[domain]/settings/secrets/components/importSecretCard.tsx
  • packages/web/src/app/[domain]/components/navigationMenu/index.tsx
  • packages/web/src/app/[domain]/components/backButton.tsx
  • packages/web/src/app/[domain]/settings/connections/layout.tsx
  • packages/backend/src/bitbucket.ts
  • packages/db/prisma/migrations/20251026194628_ensure_single_tenant_org/migration.sql
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/index.ts
  • packages/web/src/initialize.ts
  • packages/web/src/app/[domain]/chat/components/demoCards.tsx
  • packages/web/src/app/[domain]/repos/layout.tsx
  • packages/backend/src/github.ts
  • packages/web/src/actions.ts
  • packages/schemas/src/v3/index.schema.ts
  • docs/snippets/schemas/v3/app.schema.mdx
  • packages/backend/src/gitea.ts
  • packages/web/src/app/[domain]/settings/layout.tsx

msukkari
msukkari previously approved these changes Oct 29, 2025
@brendan-kellam brendan-kellam force-pushed the bkellam/connection_management_v2 branch from af104ed to d6783e4 Compare October 29, 2025 03:27
@github-actions

This comment has been minimized.

@brendan-kellam brendan-kellam merged commit a167acc into main Oct 29, 2025
10 checks passed
@brendan-kellam brendan-kellam deleted the bkellam/connection_management_v2 branch October 29, 2025 04:31
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