connections qol improvements #195
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces extensive changes across the project. New GitHub Actions workflows have been added for staging deployment and Docker image publishing. The submodule configuration is updated to fix the branch for vendor/zoekt, and VSCode recommendations now include a Prisma extension. The Dockerfile now features a shared libraries build stage and additional environment variables. The Makefile, README, and entrypoint script have been updated to reflect new build and database initialization steps. Backend modules see refactors with new ConnectionManager and RepoManager classes, while new packages and updated schemas are added for crypto, db, and repository configuration. The web package receives multiple UI, API, authentication, and multitenancy enhancements. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Checkout as Checkout Step
participant Flyctl as Flyctl Setup
participant Deploy as Deploy Step
GH->>Checkout: Checkout repository with submodules
Checkout->>Flyctl: Set up flyctl using action
Flyctl->>Deploy: Change directory to "staging" folder and deploy application
sequenceDiagram
participant Client as Web Client
participant Middleware as API Middleware
participant Auth as Authentication Service
participant DB as Database (Prisma)
Client->>Middleware: Send request with "X-Org-Domain" header
Middleware->>Auth: Validate session & organization membership
Auth->>DB: Query user and organization
DB-->>Auth: Return user/org details
Auth-->>Middleware: Authentication successful
Middleware->>Client: Return requested data
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (71)
packages/web/src/lib/schemas.ts (2)
100-103: 🛠️ Refactor suggestion
Enhance validation rules for secret creation.
The current schema lacks validation rules for the
keyandvalueproperties. Consider adding:
- Length constraints for both fields
- Pattern matching for valid key names
- Non-empty string validation
Apply this diff to add validation rules:
export const secretCreateRequestSchema = z.object({ - key: z.string(), - value: z.string(), + key: z.string() + .min(1, "Key cannot be empty") + .max(255, "Key cannot exceed 255 characters") + .regex(/^[a-zA-Z0-9_-]+$/, "Key must only contain alphanumeric characters, underscores, and hyphens"), + value: z.string() + .min(1, "Value cannot be empty") + .max(4096, "Value cannot exceed 4096 characters"), });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const secretCreateRequestSchema = z.object({ key: z.string() .min(1, "Key cannot be empty") .max(255, "Key cannot exceed 255 characters") .regex(/^[a-zA-Z0-9_-]+$/, "Key must only contain alphanumeric characters, underscores, and hyphens"), value: z.string() .min(1, "Value cannot be empty") .max(4096, "Value cannot exceed 4096 characters"), });
105-107:
⚠️ Potential issueFix typo in schema name.
The schema name contains a typo:
secreteDeleteRequestSchemashould besecretDeleteRequestSchema.Apply this diff to fix the typo:
-export const secreteDeleteRequestSchema = z.object({ +export const secretDeleteRequestSchema = z.object({ key: z.string(), });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const secretDeleteRequestSchema = z.object({ key: z.string(), });packages/web/src/app/onboard/complete/page.tsx (1)
8-14: 🛠️ Refactor suggestion
Clarify optional vs. required params.
The
searchParamsstructure is marked optional, yet lines below treatsession_id,org_name, andorg_domainas required. Consider making this interface more explicit by removing the optional marks for these critical fields, or handle undefined appropriately in the code.packages/backend/src/zoekt.ts (1)
17-17:
⚠️ Potential issueMitigate potential shell injection with user-provided fields.
Since values likerepo.orgIdandctx.indexPathcould originate from external input, consider usingexecFileor sanitizing these fields to prevent shell injection.- const command = `zoekt-git-index -allow_missing_branches -index ${ctx.indexPath} -file_limit ${DEFAULT_SETTINGS.maxFileSize} -branches ${revisions.join(',')} -tenant_id ${repo.orgId} -shard_prefix ${shardPrefix} ${repoPath}`; + // Example approach using execFile or spawn with arguments + import { spawn } from "child_process"; + const args = [ + '-allow_missing_branches', + '-index', ctx.indexPath, + '-file_limit', DEFAULT_SETTINGS.maxFileSize.toString(), + '-branches', revisions.join(','), + '-tenant_id', repo.orgId, + '-shard_prefix', shardPrefix, + repoPath + ]; + const zoektProcess = spawn('zoekt-git-index', args);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Example approach using execFile or spawn with arguments import { spawn } from "child_process"; const args = [ '-allow_missing_branches', '-index', ctx.indexPath, '-file_limit', DEFAULT_SETTINGS.maxFileSize.toString(), '-branches', revisions.join(','), '-tenant_id', repo.orgId, '-shard_prefix', shardPrefix, repoPath ]; const zoektProcess = spawn('zoekt-git-index', args);packages/backend/src/gerrit.ts (1)
67-68: 🛠️ Refactor suggestion
Use
projects.lengthon arrays.
Object.keys(projects).lengthcan be replaced byprojects.lengthbecauseprojectsis an array, which is clearer and avoids confusion.- logger.debug(`Fetched ${Object.keys(projects).length} projects in ${durationMs}ms.`); + logger.debug(`Fetched ${projects.length} projects in ${durationMs}ms.`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.logger.debug(`Fetched ${projects.length} projects in ${durationMs}ms.`); return projects;packages/backend/src/gitea.ts (1)
67-68:
⚠️ Potential issueAvoid overshadowing the
allReposvariable in map callback.Using the same name
allReposin the callback parameter can be confusing and cause potential bugs. Rename the parameter to clarify its intent.- allRepos.map(async (allRepos) => { + allRepos.map(async (repo) => {Committable suggestion skipped: line range outside the PR's diff.
packages/web/src/data/org.ts (1)
4-12: 🛠️ Refactor suggestion
Add error handling and input validation.
While the implementation is correct, it could benefit from error handling and input validation to improve robustness.
Consider applying this diff:
export const getOrgFromDomain = async (domain: string) => { + if (!domain || typeof domain !== 'string') { + throw new Error('Invalid domain parameter'); + } + + try { const org = await prisma.org.findUnique({ where: { domain: domain } }); return org; + } catch (error) { + console.error('Error fetching organization:', error); + throw error; + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const getOrgFromDomain = async (domain: string) => { if (!domain || typeof domain !== 'string') { throw new Error('Invalid domain parameter'); } try { const org = await prisma.org.findUnique({ where: { domain: domain } }); return org; } catch (error) { console.error('Error fetching organization:', error); throw error; } }packages/web/src/lib/stripe.ts (1)
6-12: 🛠️ Refactor suggestion
Add proper error handling and remove non-null assertion.
The singleton pattern is well implemented, but the code could be more robust with proper error handling and type checking.
Consider applying this diff:
let stripeInstance: Stripe | null = null; export const getStripe = () => { + if (!STRIPE_SECRET_KEY) { + throw new Error('STRIPE_SECRET_KEY is not defined'); + } + if (!stripeInstance) { - stripeInstance = new Stripe(STRIPE_SECRET_KEY!); + try { + stripeInstance = new Stripe(STRIPE_SECRET_KEY); + } catch (error) { + console.error('Error initializing Stripe:', error); + throw error; + } } return stripeInstance; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let stripeInstance: Stripe | null = null; export const getStripe = () => { if (!STRIPE_SECRET_KEY) { throw new Error('STRIPE_SECRET_KEY is not defined'); } if (!stripeInstance) { try { stripeInstance = new Stripe(STRIPE_SECRET_KEY); } catch (error) { console.error('Error initializing Stripe:', error); throw error; } } return stripeInstance; }packages/backend/src/constants.ts (1)
9-13: 💡 Verification agent
🧩 Analysis chain
Review polling intervals and consider environment-specific configurations.
The current settings might cause performance issues:
- 1-second polling intervals (
resyncConnectionPollingIntervalMsandreindexRepoPollingIntervalMs) could lead to high server load- Fixed concurrency multiples might not scale well across different environments
Consider:
- Increasing polling intervals for production
- Making settings configurable per environment
- Adding rate limiting
Run this script to check for any existing rate limiting or environment-specific configurations:
🏁 Script executed:
#!/bin/bash # Description: Check for rate limiting and environment configurations # Search for rate limiting implementations echo "Checking for rate limiting..." rg -g '!node_modules' -A 5 'rate.*limit|throttle' # Search for environment-specific configurations echo "Checking for environment configurations..." rg -g '!node_modules' -A 5 'process\.env.*(_INTERVAL|_CONCURRENCY)'Length of output: 2066
Action Required: Adjust Polling Intervals & Expose Environment Configurations
The hardcoded polling intervals in
packages/backend/src/constants.ts(lines 9–13) and the fixed concurrency multiples may lead to performance issues in production. Additionally, while a rate limiting mechanism is referenced inpackages/backend/src/github.ts, there is no environment-specific configuration for these settings. Consider the following actions:
- Increase the polling intervals (currently at 1 second) for production environments.
- Expose both polling intervals and concurrency multiples through environment variables.
- Integrate these configurable settings with the existing rate limiting strategy, ensuring that high-frequency calls do not overwhelm the server.
packages/crypto/src/environment.ts (3)
7-10: 🛠️ Refactor suggestion
Improve dotenv configuration robustness.
The current configuration has several potential issues:
- Hard-coded path might not work in all environments
- No error handling for missing .env file
- Override option might cause unexpected behavior
Consider this enhanced implementation:
-dotenv.config({ - path: './.env.local', - override: true -}); +try { + const path = process.env.NODE_ENV === 'production' + ? '.env' + : '.env.local'; + + const result = dotenv.config({ + path, + override: process.env.NODE_ENV !== 'production' + }); + + if (result.error) { + console.warn(`Warning: ${path} file not found`); + } +} catch (error) { + console.error('Error loading environment variables:', error); +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { const path = process.env.NODE_ENV === 'production' ? '.env' : '.env.local'; const result = dotenv.config({ path, override: process.env.NODE_ENV !== 'production' }); if (result.error) { console.warn(`Warning: ${path} file not found`); } } catch (error) { console.error('Error loading environment variables:', error); }
12-13:
⚠️ Potential issueAdd validation for encryption key.
The encryption key configuration lacks necessary validation for this critical security parameter.
Consider this enhanced implementation:
+const validateEncryptionKey = (key: string) => { + // Validate key length (32 bytes = 64 hex characters) + return /^[0-9a-f]{64}$/i.test(key); +} + // @note: You can use https://generate-random.org/encryption-key-generator to create a new 32 byte key -export const SOURCEBOT_ENCRYPTION_KEY = getEnv(process.env.SOURCEBOT_ENCRYPTION_KEY); +export const SOURCEBOT_ENCRYPTION_KEY = getEnv( + process.env.SOURCEBOT_ENCRYPTION_KEY, + undefined, + { + required: true, + validator: validateEncryptionKey + } +);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const validateEncryptionKey = (key: string) => { // Validate key length (32 bytes = 64 hex characters) return /^[0-9a-f]{64}$/i.test(key); } // @note: You can use https://generate-random.org/encryption-key-generator to create a new 32 byte key export const SOURCEBOT_ENCRYPTION_KEY = getEnv( process.env.SOURCEBOT_ENCRYPTION_KEY, undefined, { required: true, validator: validateEncryptionKey } );
3-5: 🛠️ Refactor suggestion
Enhance environment variable handling with validation.
The current implementation lacks validation and error handling for required environment variables.
Consider this enhanced implementation:
-export const getEnv = (env: string | undefined, defaultValue?: string) => { - return env ?? defaultValue; -} +type EnvConfig = { + required?: boolean; + validator?: (value: string) => boolean; +} + +export const getEnv = ( + env: string | undefined, + defaultValue?: string, + config: EnvConfig = {} +) => { + const value = env ?? defaultValue; + + if (config.required && !value) { + throw new Error(`Required environment variable is missing`); + } + + if (value && config.validator && !config.validator(value)) { + throw new Error(`Environment variable failed validation`); + } + + return value; +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.type EnvConfig = { required?: boolean; validator?: (value: string) => boolean; } export const getEnv = ( env: string | undefined, defaultValue?: string, config: EnvConfig = {} ) => { const value = env ?? defaultValue; if (config.required && !value) { throw new Error(`Required environment variable is missing`); } if (value && config.validator && !config.validator(value)) { throw new Error(`Environment variable failed validation`); } return value; }packages/web/src/components/hooks/use-mobile.tsx (1)
5-19: 🛠️ Refactor suggestion
Improve hook implementation and handle SSR.
The current implementation has several issues:
- Uses both
window.innerWidthandmatchMediaredundantly- Doesn't handle SSR scenarios
- Initializes state as
undefinedbut returnsboolean+const isBrowser = typeof window !== 'undefined'; + export function useIsMobile() { - const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) + const [isMobile, setIsMobile] = React.useState<boolean>( + isBrowser ? window.innerWidth < MOBILE_BREAKPOINT : false + ) React.useEffect(() => { + if (!isBrowser) return; + const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) } mql.addEventListener("change", onChange) - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) return () => mql.removeEventListener("change", onChange) }, []) - return !!isMobile + return isMobile }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const isBrowser = typeof window !== 'undefined'; export function useIsMobile() { const [isMobile, setIsMobile] = React.useState<boolean>( isBrowser ? window.innerWidth < MOBILE_BREAKPOINT : false ) React.useEffect(() => { if (!isBrowser) return; const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) const onChange = () => { setIsMobile(mql.matches) } mql.addEventListener("change", onChange) setIsMobile(mql.matches) return () => mql.removeEventListener("change", onChange) }, []) return isMobile }packages/web/src/app/[domain]/components/payWall/checkoutButton.tsx (2)
17-17:
⚠️ Potential issueRemove non-null assertion to prevent potential runtime errors.
The non-null assertion (
!) onredirectUrlcould lead to runtime errors if the URL is unexpectedly null.-window.location.href = redirectUrl!; +if (redirectUrl) { + window.location.href = redirectUrl; +} else { + toast.error("Invalid checkout URL. Please try again later."); +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (redirectUrl) { window.location.href = redirectUrl; } else { toast.error("Invalid checkout URL. Please try again later."); }
12-15: 🛠️ Refactor suggestion
Enhance error handling for better user experience.
Currently, errors are only logged to the console. Consider displaying an error toast or message to inform users when checkout creation fails.
if (isServiceError(redirectUrl)) { - console.error("Failed to create checkout session") + toast.error("Failed to create checkout session. Please try again later.") return }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (isServiceError(redirectUrl)) { - console.error("Failed to create checkout session") + toast.error("Failed to create checkout session. Please try again later.") return }packages/web/src/data/connection.ts (2)
4-13: 🛠️ Refactor suggestion
Add error handling for non-existent connections.
The function should handle cases where no connection is found. Consider throwing a custom error or returning a typed result.
export const getConnection = async (connectionId: number, orgId: number) => { const connection = await prisma.connection.findUnique({ where: { id: connectionId, orgId: orgId, }, }); + + if (!connection) { + throw new Error(`Connection not found: id=${connectionId}, orgId=${orgId}`); + } return connection; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const getConnection = async (connectionId: number, orgId: number) => { const connection = await prisma.connection.findUnique({ where: { id: connectionId, orgId: orgId, }, }); if (!connection) { throw new Error(`Connection not found: id=${connectionId}, orgId=${orgId}`); } return connection; }
15-29: 🛠️ Refactor suggestion
Add error handling for database operations.
The function should handle potential database errors. Consider wrapping the Prisma call in a try-catch block.
export const getLinkedRepos = async (connectionId: number, orgId: number) => { + try { const linkedRepos = await prisma.repoToConnection.findMany({ where: { connection: { id: connectionId, orgId: orgId, } }, include: { repo: true, } }); return linkedRepos; + } catch (error) { + throw new Error(`Failed to fetch linked repos: ${error.message}`); + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const getLinkedRepos = async (connectionId: number, orgId: number) => { try { const linkedRepos = await prisma.repoToConnection.findMany({ where: { connection: { id: connectionId, orgId: orgId, } }, include: { repo: true, } }); return linkedRepos; } catch (error) { throw new Error(`Failed to fetch linked repos: ${error.message}`); } }packages/web/src/app/[domain]/secrets/page.tsx (1)
6-21: 🛠️ Refactor suggestion
Add error and loading state handling.
The component should display appropriate UI for error states and loading states.
export default async function SecretsPage({ params: { domain } }: { params: { domain: string } }) { const secrets = await getSecrets(domain); return ( <div className="h-screen flex flex-col items-center"> <NavigationMenu domain={domain} /> - { !isServiceError(secrets) && ( + { isServiceError(secrets) ? ( + <div className="flex items-center justify-center p-4"> + <p className="text-red-500">Failed to load secrets. Please try again.</p> + </div> + ) : ( <div className="max-w-[90%]"> <SecretsTable initialSecrets={secrets} /> </div> )} </div> ) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export default async function SecretsPage({ params: { domain } }: { params: { domain: string } }) { const secrets = await getSecrets(domain); return ( <div className="h-screen flex flex-col items-center"> <NavigationMenu domain={domain} /> { isServiceError(secrets) ? ( <div className="flex items-center justify-center p-4"> <p className="text-red-500">Failed to load secrets. Please try again.</p> </div> ) : ( <div className="max-w-[90%]"> <SecretsTable initialSecrets={secrets} /> </div> )} </div> ) }packages/web/src/app/[domain]/components/orgSelector/orgIcon.tsx (1)
30-35: 🛠️ Refactor suggestion
Add required props for Next.js Image optimization.
The Image component requires width and height props. Consider adding priority prop for images above the fold.
<Image src={placeholderAvatar} alt="Organization avatar" + width={20} + height={20} + priority={true} className={cn(iconVariants({ size, className }))} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Image src={placeholderAvatar} alt="Organization avatar" width={20} height={20} priority={true} className={cn(iconVariants({ size, className }))} /> )packages/web/src/lib/environment.client.ts (1)
12-12: 💡 Verification agent
🧩 Analysis chain
Consider adding a default value for Stripe key.
Other environment variables have default values, but NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY doesn't. Consider adding a default value or documenting why it's required.
🏁 Script executed:
#!/bin/bash # Check if Stripe key is required in other files rg "NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY" --type tsLength of output: 463
Action: Add a Default or Documentation for the Stripe Publishable Key
The environment variable
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYinpackages/web/src/lib/environment.client.tsis treated as required—evidenced by its non-null assertion inpackages/web/src/app/onboard/components/trialInfoCard.tsx—yet it lacks a default value, unlike other environment variables. Please either add a default value or clearly document why no fallback is provided.
- File:
packages/web/src/lib/environment.client.ts(line 12)- File:
packages/web/src/app/onboard/components/trialInfoCard.tsx(usage withloadStripe)packages/web/src/app/[domain]/connections/components/statusIcon.tsx (1)
14-24: 🛠️ Refactor suggestion
Add default case to switch statement.
The switch statement should handle unexpected status values with a default case.
const Icon = useMemo(() => { switch (status) { case 'waiting': case 'running': return <FiLoader className={cn('animate-spin-slow', className)} />; case 'succeeded': return <CircleCheckIcon className={cn('text-green-600', className)} />; case 'failed': return <Cross2Icon className={cn(className)} />; + default: + console.warn(`Unexpected status: ${status}`); + return <Cross2Icon className={cn(className)} />; } }, [className, status]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const Icon = useMemo(() => { switch (status) { case 'waiting': case 'running': return <FiLoader className={cn('animate-spin-slow', className)} />; case 'succeeded': return <CircleCheckIcon className={cn('text-green-600', className)} />; case 'failed': return <Cross2Icon className={cn(className)} />; default: console.warn(`Unexpected status: ${status}`); return <Cross2Icon className={cn(className)} />; } }, [className, status]);supervisord.conf (1)
26-26: 💡 Verification agent
🧩 Analysis chain
Verify removal of configPath argument.
The configPath argument has been removed from the backend command. Please ensure this doesn't break any functionality.
🏁 Script executed:
#!/bin/bash # Check if configPath is used in backend code rg "configPath" packages/backend/Length of output: 832
Subject: Verify Handling of Optional
configPathParameterThe supervisord configuration now omits the
--configPathargument, yet our codebase still references it in multiple locations. For example:
packages/backend/src/index.ts:configPath: args.configPath,packages/backend/src/local.ts: Usesctx.configPathboth for path resolution and in error messages.packages/backend/src/types.ts&packages/backend/src/utils.ts: Define and utilizeconfigPathfor resolving relative paths.Please ensure that this removal does not break functionality. If the backend no longer requires an incoming
configPath, consider one of the following:
- Provide a default value in the argument parsing logic.
- Remove or refactor the portions of the code that depend on
configPath.packages/web/src/app/[domain]/components/orgSelector/index.tsx (2)
18-23: 🛠️ Refactor suggestion
Move database query to a data layer function.
Direct Prisma usage in the component violates separation of concerns. Consider moving the database query to a dedicated data layer function, similar to how
getUserOrgsis used.Create a new function in the data layer:
// data/org.ts export const getOrgByDomain = async (domain: string) => { return prisma.org.findUnique({ where: { domain } }); }Then update the component:
- const activeOrg = await prisma.org.findUnique({ - where: { - domain, - } - }); + const activeOrg = await getOrgByDomain(domain);
18-27: 🛠️ Refactor suggestion
Add error handling for database operations.
The component should handle potential database errors to prevent uncaught exceptions from breaking the UI.
- const orgs = await getUserOrgs(session.user.id); - const activeOrg = await prisma.org.findUnique({ - where: { - domain, - } - }); - - if (!activeOrg) { - return null; - } + try { + const orgs = await getUserOrgs(session.user.id); + const activeOrg = await getOrgByDomain(domain); + + if (!activeOrg) { + return null; + } + } catch (error) { + console.error('Failed to fetch organization data:', error); + return null; + }Committable suggestion skipped: line range outside the PR's diff.
packages/backend/src/environment.ts (1)
4-6:
⚠️ Potential issueFix error message for missing environment variable.
The error message includes the
envparameter which is undefined, making the error message less helpful.- throw new Error(`Missing required environment variable: ${env}`); + throw new Error('Required environment variable not provided');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (required && !env && !defaultValue) { throw new Error('Required environment variable not provided'); }packages/web/src/lib/environment.ts (2)
17-18:
⚠️ Potential issueAdd validation for Stripe environment variables.
Similar to auth variables, Stripe credentials should be validated:
- export const STRIPE_SECRET_KEY = getEnv(process.env.STRIPE_SECRET_KEY); - export const STRIPE_PRODUCT_ID = getEnv(process.env.STRIPE_PRODUCT_ID); + export const STRIPE_SECRET_KEY = requireEnv('STRIPE_SECRET_KEY', getEnv(process.env.STRIPE_SECRET_KEY)); + export const STRIPE_PRODUCT_ID = requireEnv('STRIPE_PRODUCT_ID', getEnv(process.env.STRIPE_PRODUCT_ID));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const STRIPE_SECRET_KEY = requireEnv('STRIPE_SECRET_KEY', getEnv(process.env.STRIPE_SECRET_KEY)); export const STRIPE_PRODUCT_ID = requireEnv('STRIPE_PRODUCT_ID', getEnv(process.env.STRIPE_PRODUCT_ID));
10-15:
⚠️ Potential issueEnhance security and validation for sensitive environment variables.
The auth credentials are sensitive and should be properly validated:
- Consider adding runtime validation for required values
- Add documentation for required environment variables
- Use consistent null handling
+ // Required environment variables for authentication: + // - AUTH_SECRET: Generate using `npx auth secret` + // - AUTH_GITHUB_CLIENT_ID: GitHub OAuth App client ID + // - AUTH_GITHUB_CLIENT_SECRET: GitHub OAuth App client secret + // - AUTH_GOOGLE_CLIENT_ID: Google OAuth client ID + // - AUTH_GOOGLE_CLIENT_SECRET: Google OAuth client secret + // - AUTH_URL: Authentication callback URL + + function requireEnv(name: string, value: string | undefined): string { + if (!value) throw new Error(`Missing required environment variable: ${name}`); + return value; + } - export const AUTH_SECRET = getEnv(process.env.AUTH_SECRET); // Generate using `npx auth secret` + export const AUTH_SECRET = requireEnv('AUTH_SECRET', getEnv(process.env.AUTH_SECRET)); - export const AUTH_GITHUB_CLIENT_ID = getEnv(process.env.AUTH_GITHUB_CLIENT_ID); + export const AUTH_GITHUB_CLIENT_ID = requireEnv('AUTH_GITHUB_CLIENT_ID', getEnv(process.env.AUTH_GITHUB_CLIENT_ID));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Required environment variables for authentication: // - AUTH_SECRET: Generate using `npx auth secret` // - AUTH_GITHUB_CLIENT_ID: GitHub OAuth App client ID // - AUTH_GITHUB_CLIENT_SECRET: GitHub OAuth App client secret // - AUTH_GOOGLE_CLIENT_ID: Google OAuth client ID // - AUTH_GOOGLE_CLIENT_SECRET: Google OAuth client secret // - AUTH_URL: Authentication callback URL function requireEnv(name: string, value: string | undefined): string { if (!value) throw new Error(`Missing required environment variable: ${name}`); return value; } export const AUTH_SECRET = requireEnv('AUTH_SECRET', getEnv(process.env.AUTH_SECRET)); export const AUTH_GITHUB_CLIENT_ID = requireEnv('AUTH_GITHUB_CLIENT_ID', getEnv(process.env.AUTH_GITHUB_CLIENT_ID)); export const AUTH_GITHUB_CLIENT_SECRET = getEnv(process.env.AUTH_GITHUB_CLIENT_SECRET); export const AUTH_GOOGLE_CLIENT_ID = getEnv(process.env.AUTH_GOOGLE_CLIENT_ID); export const AUTH_GOOGLE_CLIENT_SECRET = getEnv(process.env.AUTH_GOOGLE_CLIENT_SECRET); export const AUTH_URL = getEnv(process.env.AUTH_URL)!;packages/backend/src/gitlab.test.ts (1)
30-42:
⚠️ Potential issueFix unsafe type assertions and improve mock data.
The double type assertion is unsafe and the empty object for
forked_from_projectmight not represent real-world scenarios accurately.test('shouldExcludeProject returns true when the project is excluded by exclude.forks.', () => { - const project = { - path_with_namespace: 'test/project', - forked_from_project: {} - } as unknown as ProjectSchema; + const project = createMockProject({ + forked_from_project: { + id: 2, + name: 'original', + path_with_namespace: 'test/original' + } + }); expect(shouldExcludeProject({ project, exclude: { forks: true, } })).toBe(true) });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test('shouldExcludeProject returns true when the project is excluded by exclude.forks.', () => { const project = createMockProject({ forked_from_project: { id: 2, name: 'original', path_with_namespace: 'test/original' } }); expect(shouldExcludeProject({ project, exclude: { forks: true, } })).toBe(true) });packages/web/src/middleware.ts (1)
4-24: 🛠️ Refactor suggestion
Add safety checks and make port configurable.
The current implementation has potential safety issues and hardcoded values.
export default auth((request) => { - const host = request.headers.get("host")!; + const host = request.headers.get("host"); + if (!host) { + return new Response("Missing host header", { status: 400 }); + } const searchParams = request.nextUrl.searchParams.toString(); const path = `${request.nextUrl.pathname}${ searchParams.length > 0 ? `?${searchParams}` : "" }`; + const devPort = process.env.NEXT_PUBLIC_DEV_PORT || '3000'; if ( host === process.env.NEXT_PUBLIC_ROOT_DOMAIN || - host === 'localhost:3000' + host === `localhost:${devPort}` ) { if (request.nextUrl.pathname === "/login" && request.auth) { return NextResponse.redirect(new URL("/", request.url)); } return NextResponse.next(); } const subdomain = host.split(".")[0]; return NextResponse.rewrite(new URL(`/${subdomain}${path}`, request.url)); });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export default auth((request) => { const host = request.headers.get("host"); if (!host) { return new Response("Missing host header", { status: 400 }); } const searchParams = request.nextUrl.searchParams.toString(); const path = `${request.nextUrl.pathname}${ searchParams.length > 0 ? `?${searchParams}` : "" }`; const devPort = process.env.NEXT_PUBLIC_DEV_PORT || '3000'; if ( host === process.env.NEXT_PUBLIC_ROOT_DOMAIN || host === `localhost:${devPort}` ) { if (request.nextUrl.pathname === "/login" && request.auth) { return NextResponse.redirect(new URL("/", request.url)); } return NextResponse.next(); } const subdomain = host.split(".")[0]; return NextResponse.rewrite(new URL(`/${subdomain}${path}`, request.url)); });packages/web/src/app/api/(server)/search/route.ts (1)
11-26: 🛠️ Refactor suggestion
Add safety check for org domain header.
The non-null assertion on the org domain header could lead to runtime errors.
export const POST = async (request: NextRequest) => { - const domain = request.headers.get("X-Org-Domain")!; + const domain = request.headers.get("X-Org-Domain"); + if (!domain) { + return serviceErrorResponse({ + code: "BAD_REQUEST", + message: "Missing organization domain" + }); + } const body = await request.json(); const parsed = await searchRequestSchema.safeParseAsync(body); if (!parsed.success) { return serviceErrorResponse( schemaValidationError(parsed.error) ); } const response = await postSearch(parsed.data, domain); if (isServiceError(response)) { return serviceErrorResponse(response); } return Response.json(response); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const POST = async (request: NextRequest) => { const domain = request.headers.get("X-Org-Domain"); if (!domain) { return serviceErrorResponse({ code: "BAD_REQUEST", message: "Missing organization domain" }); } const body = await request.json(); const parsed = await searchRequestSchema.safeParseAsync(body); if (!parsed.success) { return serviceErrorResponse( schemaValidationError(parsed.error) ); } const response = await postSearch(parsed.data, domain); if (isServiceError(response)) { return serviceErrorResponse(response); } return Response.json(response); }packages/crypto/src/index.ts (2)
11-25: 🛠️ Refactor suggestion
Add key length validation and consider using base64 for key encoding.
While the encryption logic is correct, consider these security improvements:
- Validate that the encryption key is exactly 32 bytes (256 bits) for AES-256.
- Consider using base64 encoding for the key instead of ASCII to support a wider character set.
Apply this diff to add key validation:
export function encrypt(text: string): { iv: string; encryptedData: string } { if (!SOURCEBOT_ENCRYPTION_KEY) { throw new Error('Encryption key is not set'); } - const encryptionKey = Buffer.from(SOURCEBOT_ENCRYPTION_KEY, 'ascii'); + const encryptionKey = Buffer.from(SOURCEBOT_ENCRYPTION_KEY, 'base64'); + if (encryptionKey.length !== 32) { + throw new Error('Encryption key must be 32 bytes (256 bits) for AES-256'); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function encrypt(text: string): { iv: string; encryptedData: string } { if (!SOURCEBOT_ENCRYPTION_KEY) { throw new Error('Encryption key is not set'); } const encryptionKey = Buffer.from(SOURCEBOT_ENCRYPTION_KEY, 'base64'); if (encryptionKey.length !== 32) { throw new Error('Encryption key must be 32 bytes (256 bits) for AES-256'); } const iv = generateIV(); const cipher = crypto.createCipheriv(algorithm, encryptionKey, iv); let encrypted = cipher.update(text, 'utf8', 'hex'); encrypted += cipher.final('hex'); return { iv: iv.toString('hex'), encryptedData: encrypted }; }
27-43: 🛠️ Refactor suggestion
Add input validation for IV and encrypted text.
The decryption logic should validate the input parameters:
- Verify IV length is exactly 16 bytes.
- Ensure encrypted text is not empty and has valid hex format.
Apply this diff to add input validation:
export function decrypt(iv: string, encryptedText: string): string { if (!SOURCEBOT_ENCRYPTION_KEY) { throw new Error('Encryption key is not set'); } + if (!iv || !encryptedText) { + throw new Error('IV and encrypted text are required'); + } + const encryptionKey = Buffer.from(SOURCEBOT_ENCRYPTION_KEY, 'ascii'); const ivBuffer = Buffer.from(iv, 'hex'); + if (ivBuffer.length !== 16) { + throw new Error('IV must be 16 bytes'); + } + const encryptedBuffer = Buffer.from(encryptedText, 'hex'); + if (encryptedBuffer.length === 0) { + throw new Error('Encrypted text cannot be empty'); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function decrypt(iv: string, encryptedText: string): string { if (!SOURCEBOT_ENCRYPTION_KEY) { throw new Error('Encryption key is not set'); } if (!iv || !encryptedText) { throw new Error('IV and encrypted text are required'); } const encryptionKey = Buffer.from(SOURCEBOT_ENCRYPTION_KEY, 'ascii'); const ivBuffer = Buffer.from(iv, 'hex'); if (ivBuffer.length !== 16) { throw new Error('IV must be 16 bytes'); } const encryptedBuffer = Buffer.from(encryptedText, 'hex'); if (encryptedBuffer.length === 0) { throw new Error('Encrypted text cannot be empty'); } const decipher = crypto.createDecipheriv(algorithm, encryptionKey, ivBuffer); let decrypted = decipher.update(encryptedBuffer, undefined, 'utf8'); decrypted += decipher.final('utf8'); return decrypted; }packages/web/src/app/[domain]/settings/billing/manageSubscriptionButton.tsx (1)
16-30: 🛠️ Refactor suggestion
Enhance error handling and user feedback.
The current implementation logs errors to the console, which isn't visible to users. Consider using a toast notification system for user-facing error messages.
const redirectToCustomerPortal = async () => { setIsLoading(true) + setError(null) try { const session = await getCustomerPortalSessionLink(domain) if (isServiceError(session)) { - console.log("Failed to create portal session: ", session) + throw new Error(`Failed to create portal session: ${session.message}`) } else { router.push(session) } } catch (error) { - console.error("Error creating portal session:", error) + const errorMessage = error instanceof Error ? error.message : "Failed to create portal session" + setError(errorMessage) + toast({ + title: "Error", + description: errorMessage, + variant: "destructive", + }) } finally { setIsLoading(false) } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const redirectToCustomerPortal = async () => { setIsLoading(true) setError(null) try { const session = await getCustomerPortalSessionLink(domain) if (isServiceError(session)) { throw new Error(`Failed to create portal session: ${session.message}`) } else { router.push(session) } } catch (error) { const errorMessage = error instanceof Error ? error.message : "Failed to create portal session" setError(errorMessage) toast({ title: "Error", description: errorMessage, variant: "destructive", }) } finally { setIsLoading(false) } }packages/schemas/tools/generate.ts (2)
23-31: 🛠️ Refactor suggestion
Consider adding error handling for schema bundling.
The schema bundling process should handle potential errors that may occur during the
$RefParser.bundleoperation.- const schema = JSON.stringify(await $RefParser.bundle(schemaPath), null, 2); + let schema; + try { + schema = JSON.stringify(await $RefParser.bundle(schemaPath), null, 2); + } catch (error) { + console.error(`Failed to bundle schema ${name}:`, error); + throw error; + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Generate schema - const schema = JSON.stringify(await $RefParser.bundle(schemaPath), null, 2); + let schema; + try { + schema = JSON.stringify(await $RefParser.bundle(schemaPath), null, 2); + } catch (error) { + console.error(`Failed to bundle schema ${name}:`, error); + throw error; + } await writeFile( path.join(outDir, `${name}.schema.ts`), BANNER_COMMENT + 'const schema = ' + schema + ` as const;\nexport { schema as ${name}Schema };`, );
12-14: 💡 Verification agent
🧩 Analysis chain
Verify the path resolution logic.
The path resolution assumes a specific directory structure. Ensure that the paths are correctly resolved in different environments.
🏁 Script executed:
#!/bin/bash # Description: Verify the existence of the schemas directory and its structure. # Test: Check if the schemas directory exists at the expected location cd "$(dirname "$0")/../../.." && \ if [ -d "schemas" ]; then echo "✓ schemas directory exists" find schemas -name "*.json" -type f else echo "✗ schemas directory not found" exit 1 fiLength of output: 187
Action Required: Correct Path Resolution for Schemas Directory
It appears that the computed path for the
schemasdirectory inpackages/schemas/tools/generate.tsisn’t resolving as expected. The verification script failed to locate the directory at${cwd}/../../schemas, which suggests that either the project’s directory structure has changed or thecwdvalue isn’t set as anticipated in some environments.
- Check the
cwdvalue: Confirm that the working directory during execution is as expected.- Review the relative path calculation: Ensure that the schema files are indeed located relative to
${cwd}/../../schemas. If the actual directory differs, adjust the path accordingly.- Validate across environments: Make sure this path resolution logic holds true in both local development and deployment setups.
packages/web/src/app/[domain]/secrets/columns.tsx (1)
34-41: 🛠️ Refactor suggestion
Add confirmation dialog before deletion.
Consider adding a confirmation dialog before deleting secrets to prevent accidental deletions.
<Button variant="destructive" + onClick={(e) => { + e.preventDefault(); + if (window.confirm(`Are you sure you want to delete the secret "${secret.key}"?`)) { + handleDelete(secret.key); + } + }} - onClick={() => { - handleDelete(secret.key); - }} > Delete </Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Button variant="destructive" onClick={(e) => { e.preventDefault(); if (window.confirm(`Are you sure you want to delete the secret "${secret.key}"?`)) { handleDelete(secret.key); } }} > Delete </Button>packages/web/src/app/redeem/components/acceptInviteButton.tsx (1)
24-34: 🛠️ Refactor suggestion
Improve error handling and logging.
The error handling could be improved by:
- Adding more specific error messages based on the error type.
- Including error details in the logs for better debugging.
const res = await redeemInvite(invite, userId) if (isServiceError(res)) { - console.log("Failed to redeem invite: ", res) + console.error("Failed to redeem invite:", { + error: res, + invite: invite.id, + userId + }) toast({ title: "Error", - description: "Failed to redeem invite. Please ensure the organization has an active subscription.", + description: `Failed to redeem invite: ${res.message || 'Please ensure the organization has an active subscription.'}`, variant: "destructive", })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const res = await redeemInvite(invite, userId) if (isServiceError(res)) { - console.log("Failed to redeem invite: ", res) + console.error("Failed to redeem invite:", { + error: res, + invite: invite.id, + userId + }) toast({ title: "Error", - description: "Failed to redeem invite. Please ensure the organization has an active subscription.", + description: `Failed to redeem invite: ${res.message || 'Please ensure the organization has an active subscription.'}`, variant: "destructive", }) } else { router.push("/") }packages/web/src/app/api/(client)/client.ts (1)
8-20: 🛠️ Refactor suggestion
Consider implementing request error handling.
The API client functions should handle fetch errors and provide appropriate error responses.
Example implementation for the
searchfunction:export const search = async (body: SearchRequest, domain: string): Promise<SearchResponse> => { const path = resolveServerPath("/api/search"); - const result = await fetch(path, { - method: "POST", - headers: { - "Content-Type": "application/json", - "X-Org-Domain": domain, - }, - body: JSON.stringify(body), - }).then(response => response.json()); + try { + const response = await fetch(path, { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-Org-Domain": domain, + }, + body: JSON.stringify(body), + }); + + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } + + const result = await response.json(); + return searchResponseSchema.parse(result); + } catch (error) { + throw new Error(`Failed to perform search: ${error.message}`); + } - return searchResponseSchema.parse(result); }Also applies to: 22-34, 36-47
packages/web/src/app/[domain]/settings/components/inviteTableColumns.tsx (1)
40-41: 🛠️ Refactor suggestion
Enhance URL construction security.
The current URL construction might be vulnerable to injection if
invite.idcontains special characters. Consider using URL encoding.-const url = createPathWithQueryParams(`${basePath}redeem?invite_id=${invite.id}`); +const url = createPathWithQueryParams(`${basePath}redeem?invite_id=${encodeURIComponent(invite.id)}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const basePath = `${window.location.origin}${resolveServerPath('/')}`; const url = createPathWithQueryParams(`${basePath}redeem?invite_id=${encodeURIComponent(invite.id)}`);packages/web/src/app/[domain]/connections/components/newConnectionCard.tsx (1)
46-50: 🛠️ Refactor suggestion
Consider strengthening the type safety of the
typeprop.The
typeprop should be restricted to valid code host types instead of accepting any string.- type: string; + type: CodeHostType;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.interface CardProps { type: CodeHostType; title: string; subtitle: string; }packages/web/src/app/[domain]/settings/components/memberInviteForm.tsx (1)
16-18: 🛠️ Refactor suggestion
Strengthen email validation.
The current schema only validates length. Consider using Zod's email validator for proper email format validation.
- email: z.string().min(2).max(40), + email: z.string().email("Invalid email format").max(40),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const formSchema = z.object({ email: z.string().email("Invalid email format").max(40), });packages/web/src/app/[domain]/connections/components/connectionList/index.tsx (1)
31-50: 🛠️ Refactor suggestion
Consider adjusting the polling interval for better performance.
The current 1-second polling interval might be too aggressive and could impact performance. Consider:
- Increasing the interval to 5-10 seconds
- Implementing exponential backoff
- Using WebSocket for real-time updates instead of polling
- const intervalId = setInterval(fetchConnections, 1000); + const intervalId = setInterval(fetchConnections, 5000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { const fetchConnections = async () => { try { const result = await getConnections(domain); if (isServiceError(result)) { setError(result.message); } else { setConnections(result); } setLoading(false); } catch (err) { setError(err instanceof Error ? err.message : 'An error occured while fetching connections. If the problem persists, please contact us at team@sourcebot.dev'); setLoading(false); } }; fetchConnections(); - const intervalId = setInterval(fetchConnections, 1000); + const intervalId = setInterval(fetchConnections, 5000); return () => clearInterval(intervalId); }, [domain]);packages/web/src/app/[domain]/settings/page.tsx (1)
66-71:
⚠️ Potential issueAdd null checks for member properties.
The code assumes that
nameand!). This could lead to runtime errors if the data is incomplete.const memberInfo = members.map((member) => ({ id: member.id, - name: member.name!, - email: member.email!, + name: member.name ?? 'Unknown', + email: member.email ?? 'No email', role: member.orgs[0].role, }))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const memberInfo = members.map((member) => ({ id: member.id, name: member.name ?? 'Unknown', email: member.email ?? 'No email', role: member.orgs[0].role, }))packages/web/src/app/[domain]/connections/components/connectionList/connectionListItem.tsx (2)
10-22:
⚠️ Potential issueAdd exhaustive switch case handling.
The
convertSyncStatusfunction should handle all possible enum values explicitly.const convertSyncStatus = (status: ConnectionSyncStatus) => { switch (status) { case ConnectionSyncStatus.SYNC_NEEDED: return 'waiting'; case ConnectionSyncStatus.IN_SYNC_QUEUE: case ConnectionSyncStatus.SYNCING: return 'running'; case ConnectionSyncStatus.SYNCED: return 'succeeded'; case ConnectionSyncStatus.FAILED: return 'failed'; + default: + const exhaustiveCheck: never = status; + throw new Error(`Unhandled status: ${exhaustiveCheck}`); } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const convertSyncStatus = (status: ConnectionSyncStatus) => { switch (status) { case ConnectionSyncStatus.SYNC_NEEDED: return 'waiting'; case ConnectionSyncStatus.IN_SYNC_QUEUE: case ConnectionSyncStatus.SYNCING: return 'running'; case ConnectionSyncStatus.SYNCED: return 'succeeded'; case ConnectionSyncStatus.FAILED: return 'failed'; default: const exhaustiveCheck: never = status; throw new Error(`Unhandled status: ${exhaustiveCheck}`); } }
56-95: 🛠️ Refactor suggestion
Enhance accessibility for interactive elements.
The link and button lack proper ARIA attributes and keyboard navigation support.
- <Link href={`connections/${id}`}> + <Link + href={`connections/${id}`} + role="listitem" + aria-label={`Connection ${name}, status: ${statusDisplayName}`} + > <div className="flex flex-row justify-between items-center border p-4 rounded-lg cursor-pointer bg-background" + tabIndex={0} > {/* ... */} <Button variant="outline" size={"sm"} className="ml-4" + aria-label={`Manage ${name} connection`} > Manage </Button> </div> </Link>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Link href={`connections/${id}`} role="listitem" aria-label={`Connection ${name}, status: ${statusDisplayName}`} > <div className="flex flex-row justify-between items-center border p-4 rounded-lg cursor-pointer bg-background" tabIndex={0} > <div className="flex flex-row items-center gap-3"> <ConnectionIcon type={type} className="w-8 h-8" /> <div className="flex flex-col"> <p className="font-medium">{name}</p> <span className="text-sm text-muted-foreground">{`Edited ${getDisplayTime(editedAt)}`}</span> </div> </div> <div className="flex flex-row items-center"> <StatusIcon status={convertSyncStatus(status)} className="w-4 h-4 mr-1" /> <p className="text-sm"> <span>{statusDisplayName}</span> { ( status === ConnectionSyncStatus.SYNCED || status === ConnectionSyncStatus.FAILED ) && syncedAt && ( <span>{` ${getDisplayTime(syncedAt)}`}</span> ) } </p> <Button variant="outline" size={"sm"} className="ml-4" aria-label={`Manage ${name} connection`} > Manage </Button> </div> </div> </Link>packages/web/src/app/onboard/components/trialInfoCard.tsx (1)
38-44:
⚠️ Potential issueImprove error handling for Stripe integration.
The current error handling could be more informative for users.
options={{ fetchClientSecret: async () => { - const clientSecret = await setupInitialStripeCustomer(orgCreateInfo.name, orgCreateInfo.domain); - if (isServiceError(clientSecret)) { - throw clientSecret; - } - return clientSecret; + try { + const clientSecret = await setupInitialStripeCustomer(orgCreateInfo.name, orgCreateInfo.domain); + if (isServiceError(clientSecret)) { + throw new Error(`Failed to setup Stripe customer: ${clientSecret.message}`); + } + return clientSecret; + } catch (error) { + console.error('Stripe setup failed:', error); + throw error; + } } }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.options={{ fetchClientSecret: async () => { try { const clientSecret = await setupInitialStripeCustomer(orgCreateInfo.name, orgCreateInfo.domain); if (isServiceError(clientSecret)) { throw new Error(`Failed to setup Stripe customer: ${clientSecret.message}`); } return clientSecret; } catch (error) { console.error('Stripe setup failed:', error); throw error; } } }}packages/web/src/app/[domain]/connections/new/[type]/page.tsx (2)
15-38: 🛠️ Refactor suggestion
Add validation for connection type parameter.
The routing logic should validate the connection type parameter against a predefined set of valid types.
+const VALID_CONNECTION_TYPES = ['github', 'gitlab', 'gitea', 'gerrit'] as const; +type ConnectionType = typeof VALID_CONNECTION_TYPES[number]; + +const isValidConnectionType = (type: string): type is ConnectionType => { + return VALID_CONNECTION_TYPES.includes(type as ConnectionType); +} + export default function NewConnectionPage({ params }: { params: { type: string } }) { const { type } = params; const router = useRouter(); + if (!isValidConnectionType(type)) { + router.push('/connections'); + return null; + } + if (type === 'github') { return <GitHubCreationForm />; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const VALID_CONNECTION_TYPES = ['github', 'gitlab', 'gitea', 'gerrit'] as const; type ConnectionType = typeof VALID_CONNECTION_TYPES[number]; const isValidConnectionType = (type: string): type is ConnectionType => { return VALID_CONNECTION_TYPES.includes(type as ConnectionType); } export default function NewConnectionPage({ params }: { params: { type: string } }) { const { type } = params; const router = useRouter(); if (!isValidConnectionType(type)) { router.push('/connections'); return null; } if (type === 'github') { return <GitHubCreationForm />; } if (type === 'gitlab') { return <GitLabCreationForm />; } if (type === 'gitea') { return <GiteaCreationForm />; } if (type === 'gerrit') { return <GerritCreationForm />; } router.push('/connections'); }
40-115: 🛠️ Refactor suggestion
Reduce code duplication in creation form components.
The creation form components share similar structure and could be refactored to reduce duplication.
+interface ConnectionFormConfig<T> { + type: string; + title: string; + defaultConfig: T; + schema: any; + quickActions: any[]; +} + +const createConnectionForm = <T extends { type: string }>({ + type, + title, + defaultConfig, + schema, + quickActions +}: ConnectionFormConfig<T>) => { + return ( + <ConnectionCreationForm<T> + type={type} + title={title} + defaultValues={{ + config: JSON.stringify(defaultConfig, null, 2), + name: `my-${type}-connection`, + }} + schema={schema} + quickActions={quickActions} + /> + ) +} + +const GitLabCreationForm = () => createConnectionForm({ + type: 'gitlab', + title: 'Create a GitLab connection', + defaultConfig: { type: 'gitlab' } as GitlabConnectionConfig, + schema: gitlabSchema, + quickActions: gitlabQuickActions +});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.interface ConnectionFormConfig<T> { type: string; title: string; defaultConfig: T; schema: any; quickActions: any[]; } const createConnectionForm = <T extends { type: string }>({ type, title, defaultConfig, schema, quickActions }: ConnectionFormConfig<T>) => { return ( <ConnectionCreationForm<T> type={type} title={title} defaultValues={{ config: JSON.stringify(defaultConfig, null, 2), name: `my-${type}-connection`, }} schema={schema} quickActions={quickActions} /> ); }; const GitLabCreationForm = () => createConnectionForm({ type: 'gitlab', title: 'Create a GitLab connection', defaultConfig: { type: 'gitlab' } as GitlabConnectionConfig, schema: gitlabSchema, quickActions: gitlabQuickActions, }); const GitHubCreationForm = () => { const defaultConfig: GithubConnectionConfig = { type: 'github', }; return ( <ConnectionCreationForm<GithubConnectionConfig> type="github" title="Create a GitHub connection" defaultValues={{ config: JSON.stringify(defaultConfig, null, 2), name: 'my-github-connection', }} schema={githubSchema} quickActions={githubQuickActions} /> ); }; const GiteaCreationForm = () => { const defaultConfig: GiteaConnectionConfig = { type: 'gitea', }; return ( <ConnectionCreationForm<GiteaConnectionConfig> type="gitea" title="Create a Gitea connection" defaultValues={{ config: JSON.stringify(defaultConfig, null, 2), name: 'my-gitea-connection', }} schema={giteaSchema} quickActions={giteaQuickActions} /> ); }; const GerritCreationForm = () => { const defaultConfig: GerritConnectionConfig = { type: 'gerrit', url: "https://gerrit.example.com", }; return ( <ConnectionCreationForm<GerritConnectionConfig> type="gerrit" title="Create a Gerrit connection" defaultValues={{ config: JSON.stringify(defaultConfig, null, 2), name: 'my-gerrit-connection', }} schema={gerritSchema} quickActions={gerritQuickActions} /> ); };packages/web/src/app/[domain]/connections/[id]/components/deleteConnectionSetting.tsx (1)
36-56: 🛠️ Refactor suggestion
Add error handling for network failures.
The
handleDeletefunction should handle network failures explicitly. Currently, if the network request fails, the error won't be caught and displayed to the user.handleDelete = useCallback(() => { setIsDialogOpen(false); setIsLoading(true); deleteConnection(connectionId, domain) .then((response) => { if (isServiceError(response)) { toast({ description: `❌ Failed to delete connection. Reason: ${response.message}` }); } else { toast({ description: `✅ Connection deleted successfully.` }); router.replace(`/${domain}/connections`); router.refresh(); } }) + .catch((error) => { + toast({ + description: `❌ Network error while deleting connection. Please try again.` + }); + }) .finally(() => { setIsLoading(false); }); }, [connectionId, domain, router, toast]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const handleDelete = useCallback(() => { setIsDialogOpen(false); setIsLoading(true); deleteConnection(connectionId, domain) .then((response) => { if (isServiceError(response)) { toast({ description: `❌ Failed to delete connection. Reason: ${response.message}` }); } else { toast({ description: `✅ Connection deleted successfully.` }); router.replace(`/${domain}/connections`); router.refresh(); } }) .catch((error) => { toast({ description: `❌ Network error while deleting connection. Please try again.` }); }) .finally(() => { setIsLoading(false); }); }, [connectionId, domain, router, toast]);packages/web/src/app/[domain]/connections/[id]/components/displayNameSetting.tsx (1)
41-58: 🛠️ Refactor suggestion
Add error handling for network failures.
Similar to the delete connection component, network failures should be explicitly handled.
onSubmit = useCallback((data: z.infer<typeof formSchema>) => { setIsLoading(true); updateConnectionDisplayName(connectionId, data.name, domain) .then((response) => { if (isServiceError(response)) { toast({ description: `❌ Failed to rename connection. Reason: ${response.message}` }); } else { toast({ description: `✅ Connection renamed successfully.` }); router.refresh(); } }) + .catch((error) => { + toast({ + description: `❌ Network error while renaming connection. Please try again.` + }); + }) .finally(() => { setIsLoading(false); }); }, [connectionId, domain, router, toast]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const onSubmit = useCallback((data: z.infer<typeof formSchema>) => { setIsLoading(true); updateConnectionDisplayName(connectionId, data.name, domain) .then((response) => { if (isServiceError(response)) { toast({ description: `❌ Failed to rename connection. Reason: ${response.message}` }); } else { toast({ description: `✅ Connection renamed successfully.` }); router.refresh(); } }) .catch((error) => { toast({ description: `❌ Network error while renaming connection. Please try again.` }); }) .finally(() => { setIsLoading(false); }); }, [connectionId, domain, router, toast]);packages/web/src/app/[domain]/settings/billing/changeBillingEmailCard.tsx (1)
39-47: 🛠️ Refactor suggestion
Add error feedback for billing email fetch failures.
The
fetchBillingEmailfunction should notify users when the email fetch fails.useEffect(() => { const fetchBillingEmail = async () => { const email = await getSubscriptionBillingEmail(domain) if (!isServiceError(email)) { setBillingEmail(email) + } else { + toast({ + description: "❌ Failed to fetch billing email. Please refresh the page.", + }) } } fetchBillingEmail() }, [domain])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { const fetchBillingEmail = async () => { const email = await getSubscriptionBillingEmail(domain) if (!isServiceError(email)) { setBillingEmail(email) } else { toast({ description: "❌ Failed to fetch billing email. Please refresh the page.", }) } } fetchBillingEmail() }, [domain])packages/web/src/app/[domain]/connections/quickActions.ts (2)
50-54:
⚠️ Potential issueFix the spread operator syntax in GitLab groups.
The spread operator syntax is incorrect. When spreading an optional array, the parentheses should wrap the entire expression.
- groups: [ - ...previous.groups ?? [], - "" - ] + groups: [ + ...(previous.groups ?? []), + "" + ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.groups: [ ...(previous.groups ?? []), "" ] }),
76-80:
⚠️ Potential issueFix the spread operator syntax in GitLab projects.
Similar to the groups issue, the spread operator syntax needs correction.
- projects: [ - ...previous.projects ?? [], - "" - ] + projects: [ + ...(previous.projects ?? []), + "" + ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.projects: [ ...(previous.projects ?? []), "" ] }),packages/web/src/app/[domain]/connections/components/configEditor.tsx (2)
69-93: 🛠️ Refactor suggestion
Consider adding error handling for JSON.stringify failures.
The
onQuickActionfunction handles JSON.parse errors but not potential JSON.stringify failures.const onQuickAction = (action: QuickActionFn<T>) => { let previousConfig: T; try { previousConfig = JSON.parse(value) as T; } catch { return; } const nextConfig = action(previousConfig); - const next = JSON.stringify(nextConfig, null, 2); + let next: string; + try { + next = JSON.stringify(nextConfig, null, 2); + } catch (error) { + console.error('Failed to stringify config:', error); + return; + } const cursorPos = next.lastIndexOf(`""`) + 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const onQuickAction = (action: QuickActionFn<T>) => { let previousConfig: T; try { previousConfig = JSON.parse(value) as T; } catch { return; } const nextConfig = action(previousConfig); let next: string; try { next = JSON.stringify(nextConfig, null, 2); } catch (error) { console.error('Failed to stringify config:', error); return; } const cursorPos = next.lastIndexOf(`""`) + 1; editorRef.current?.view?.focus(); editorRef.current?.view?.dispatch({ changes: { from: 0, to: value.length, insert: next, } }); editorRef.current?.view?.dispatch({ selection: { anchor: cursorPos, head: cursorPos } }); }
141-141: 🛠️ Refactor suggestion
Remove the eslint-disable comment and fix the type.
Instead of disabling the eslint rule, consider fixing the type issue.
-// eslint-disable-next-line @typescript-eslint/no-explicit-any -stateExtensions(schema as any), +stateExtensions(schema as Schema),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.-// eslint-disable-next-line @typescript-eslint/no-explicit-any -stateExtensions(schema as any), +stateExtensions(schema as Schema),packages/web/src/app/[domain]/components/orgSelector/orgSelectorDropdown.tsx (2)
30-30:
⚠️ Potential issueAdd null check for activeOrg.
The non-null assertion operator (!) is used without checking if the org exists.
-const activeOrg = _orgs.find((org) => org.id === activeOrgId)!; +const activeOrg = _orgs.find((org) => org.id === activeOrgId); +if (!activeOrg) { + throw new Error(`No organization found with id ${activeOrgId}`); +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const activeOrg = _orgs.find((org) => org.id === activeOrgId); if (!activeOrg) { throw new Error(`No organization found with id ${activeOrgId}`); }
92-94: 🛠️ Refactor suggestion
Use org.id directly as the key instead of index.
Using array index as key might cause issues with list updates. Since org.id is unique, it should be used as the key.
-key={index} +key={org.id} -value={`${org.name}-${org.id}`} +value={org.id.toString()}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.key={org.id} // Need to include org id to handle duplicates. value={org.id.toString()}packages/web/src/app/[domain]/secrets/secretsTable.tsx (2)
42-44:
⚠️ Potential issueFix useEffect dependency array to prevent infinite loops.
The
fetchSecretKeysfunction is included in the dependency array, which could cause infinite loops since the function is recreated on each render. Consider usinguseCallbackor moving the function inside theuseEffect.- useEffect(() => { - fetchSecretKeys(); - }, [fetchSecretKeys]); + useEffect(() => { + const fetchSecretKeys = async () => { + const keys = await getSecrets(domain); + if ('keys' in keys) { + setSecrets(keys); + } else { + console.error(keys); + } + }; + fetchSecretKeys(); + }, [domain]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { const fetchSecretKeys = async () => { const keys = await getSecrets(domain); if ('keys' in keys) { setSecrets(keys); } else { console.error(keys); } }; fetchSecretKeys(); }, [domain]);
93-97: 🛠️ Refactor suggestion
Use consistent error type checking.
The error type checking is inconsistent with the pattern used earlier in the file. Consider using
isServiceErrorconsistently.- if ('keys' in keys) { + if (isServiceError(keys)) { + console.error(keys); + } else { setSecrets(keys); - } else { - console.error(keys); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (isServiceError(keys)) { console.error(keys); } else { setSecrets(keys); }packages/web/src/app/onboard/components/orgCreateForm.tsx (1)
43-56: 🛠️ Refactor suggestion
Add error handling for network failures.
The domain existence check should handle network failures gracefully.
async function submitOrgInfoForm(data: OnboardingFormValues) { - const res = await checkIfOrgDomainExists(data.domain); + try { + const res = await checkIfOrgDomainExists(data.domain); + if (isServiceError(res)) { + setErrorMessage("An error occurred while checking the domain. Please try clearing your cookies and trying again."); + return; + } + if (res) { + setErrorMessage("Organization domain already exists. Please try a different one."); + return; + } + setOrgCreateData(data); + } catch (error) { + setErrorMessage("Network error. Please check your connection and try again."); + } - if (isServiceError(res)) { - setErrorMessage("An error occurred while checking the domain. Please try clearing your cookies and trying again."); - return; - } - - if (res) { - setErrorMessage("Organization domain already exists. Please try a different one."); - return; - } else { - setOrgCreateData(data); - } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.async function submitOrgInfoForm(data: OnboardingFormValues) { try { const res = await checkIfOrgDomainExists(data.domain); if (isServiceError(res)) { setErrorMessage("An error occurred while checking the domain. Please try clearing your cookies and trying again."); return; } if (res) { setErrorMessage("Organization domain already exists. Please try a different one."); return; } setOrgCreateData(data); } catch (error) { setErrorMessage("Network error. Please check your connection and try again."); } }packages/web/src/app/[domain]/components/navigationMenu.tsx (1)
95-105: 🛠️ Refactor suggestion
Consider adding error handling for trial status calculation.
The trial status calculation assumes
subscription.nextBillingDateis valid. Add error handling to prevent potential runtime errors if the date is invalid or undefined.-{!isServiceError(subscription) && subscription.status === "trialing" && ( +{!isServiceError(subscription) && subscription.status === "trialing" && subscription.nextBillingDate && ( <Link href={`/${domain}/settings/billing`}> <div className="flex items-center gap-2 px-3 py-1.5 bg-yellow-50 dark:bg-yellow-900/20 border border-yellow-200 dark:border-yellow-700 rounded-full text-yellow-700 dark:text-yellow-400 text-xs font-medium hover:bg-yellow-100 dark:hover:bg-yellow-900/30 transition-colors cursor-pointer"> <span className="inline-block w-2 h-2 bg-yellow-400 dark:bg-yellow-500 rounded-full"></span> <span> - {Math.ceil((subscription.nextBillingDate * 1000 - Date.now()) / (1000 * 60 * 60 * 24))} days left in + {Math.max(0, Math.ceil((subscription.nextBillingDate * 1000 - Date.now()) / (1000 * 60 * 60 * 24)))} days left in trial </span> </div> </Link> )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.{!isServiceError(subscription) && subscription.status === "trialing" && subscription.nextBillingDate && ( <Link href={`/${domain}/settings/billing`}> <div className="flex items-center gap-2 px-3 py-1.5 bg-yellow-50 dark:bg-yellow-900/20 border border-yellow-200 dark:border-yellow-700 rounded-full text-yellow-700 dark:text-yellow-400 text-xs font-medium hover:bg-yellow-100 dark:hover:bg-yellow-900/30 transition-colors cursor-pointer"> <span className="inline-block w-2 h-2 bg-yellow-400 dark:bg-yellow-500 rounded-full"></span> <span> {Math.max(0, Math.ceil((subscription.nextBillingDate * 1000 - Date.now()) / (1000 * 60 * 60 * 24)))} days left in trial </span> </div> </Link> )}packages/web/src/app/[domain]/page.tsx (2)
121-123: 🛠️ Refactor suggestion
Improve error handling feedback.
Silent error handling might confuse users. Consider showing an error message or fallback UI.
if (isServiceError(_repos)) { - return null; + return ( + <div className="flex flex-row items-center gap-3 text-red-500"> + <span className="text-sm">Failed to load repositories. Please try again later.</span> + </div> + ); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (isServiceError(_repos)) { return ( <div className="flex flex-row items-center gap-3 text-red-500"> <span className="text-sm">Failed to load repositories. Please try again later.</span> </div> ); }
189-190:
⚠️ Potential issueAdd URL encoding for search queries.
Search queries might contain special characters that need to be properly encoded.
- href={`/search?query=${query}`} + href={`/search?query=${encodeURIComponent(query)}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.href={`/search?query=${encodeURIComponent(query)}`} className="cursor-pointer hover:underline"packages/schemas/src/v3/connection.type.ts (1)
254-273: 🛠️ Refactor suggestion
Rename
GerritConnectionConfig1toLocalConnectionConfig.The interface name
GerritConnectionConfig1is misleading as it represents a local configuration type.-export interface GerritConnectionConfig1 { +export interface LocalConnectionConfig { /** * Local Configuration */ type: "local"; // ... rest of the interface }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export interface LocalConnectionConfig { /** * Local Configuration */ type: "local"; /** * Path to the local directory to sync with. Relative paths are relative to the configuration file's directory. */ path: string; /** * Enables a file watcher that will automatically re-sync when changes are made within `path` (recursively). Defaults to true. */ watch?: boolean; exclude?: { /** * List of paths relative to the provided `path` to exclude from the index. .git, .hg, and .svn are always exluded. */ paths?: string[]; }; }packages/schemas/src/v3/connection.schema.ts (2)
536-537:
⚠️ Potential issueFix duplicate title in schema definition.
The title "GerritConnectionConfig" is duplicated. This appears to be for the Local connection configuration.
- "title": "GerritConnectionConfig", + "title": "LocalConnectionConfig",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."title": "LocalConnectionConfig", "properties": {
536-536:
⚠️ Potential issueFix incorrect title for Local configuration.
The title is incorrectly set to "GerritConnectionConfig" instead of "LocalConnectionConfig".
- "title": "GerritConnectionConfig", + "title": "LocalConnectionConfig",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."title": "LocalConnectionConfig",packages/db/prisma/migrations/20250212061001_remove_active_org_id/migration.sql (1)
1-8: 💡 Verification agent
🧩 Analysis chain
Verify data migration strategy for activeOrgId removal.
The removal of
activeOrgIdwill result in data loss. Before applying this migration:
- Ensure any active organization data is preserved if needed
- Update application logic that previously relied on
activeOrgId- Consider adding a rollback plan
🏁 Script executed:
#!/bin/bash # Description: Check for references to activeOrgId in the codebase # Search for any remaining references to activeOrgId echo "Searching for code references to activeOrgId..." rg "activeOrgId" # Search for potential foreign key constraints echo "Checking for foreign key constraints..." rg "FOREIGN KEY.*activeOrgId" --type sqlLength of output: 1515
Attention: ActiveOrgId Removal Affects Application Logic
The migration file drops the
activeOrgIdcolumn from theUsertable, which will cause irreversible data loss. Our verification revealed that this column is still actively referenced in the front-end code (for example, in the org selector components withinpackages/web/src/app/[domain]/components/orgSelector). Before applying the migration, please:
- Preserve/Migrate Data: Ensure that any critical organization data is either backed up or migrated appropriately.
- Update Application Logic: Refactor any dependencies on
activeOrgIdin the front-end code (e.g., inindex.tsxandorgSelectorDropdown.tsx) to avoid runtime issues.- Plan Rollback: Develop and test a rollback strategy in case unexpected issues arise after the migration.
packages/db/prisma/migrations/20250206180955_add_domain/migration.sql (2)
9-10:
⚠️ Potential issueCritical Migration Check — Missing Default Value:
TheALTER TABLEcommand adds thedomaincolumn asTEXT NOT NULLwithout a default. This migration will fail if the table already contains data. Ensure that either the table is empty, or consider providing a temporary default value to accommodate existing rows.
11-12:
⚠️ Potential issueEnsure Unique Constraint Readiness:
The unique index ondomainwill enforce distinct values, so it’s essential to verify that no duplicate values exist in current data. Double-check the production data or include a data migration step to consolidate duplicates if necessary.schemas/v3/local.json (1)
1-50: 🛠️ Refactor suggestion
⚠️ Potential issueSchema Title Mismatch
The schema’s title is set to"GerritConnectionConfig"even though thetypeproperty is constrained to"local", and the file name (local.json) implies this schema should define a local connection configuration. To avoid confusion, consider renaming the title to"LocalConnectionConfig".- "title": "GerritConnectionConfig", + "title": "LocalConnectionConfig",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.{ "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "title": "LocalConnectionConfig", "properties": { "type": { "const": "local", "description": "Local Configuration" }, "path": { "type": "string", "description": "Path to the local directory to sync with. Relative paths are relative to the configuration file's directory.", "pattern": ".+" }, "watch": { "type": "boolean", "default": true, "description": "Enables a file watcher that will automatically re-sync when changes are made within `path` (recursively). Defaults to true." }, "exclude": { "type": "object", "properties": { "paths": { "type": "array", "items": { "type": "string", "pattern": ".+" }, "description": "List of paths relative to the provided `path` to exclude from the index. .git, .hg, and .svn are always exluded.", "default": [], "examples": [ [ "node_modules", "bin", "dist", "build", "out" ] ] } }, "additionalProperties": false } }, "required": [ "type", "path" ], "additionalProperties": false }.github/workflows/staging-ghcr-public.yml (1)
25-26:
⚠️ Potential issueRunner Label Correction
The runner label"ubuntu-24.04-arm"is not among the officially supported labels (e.g.,"ubuntu-24.04"is available). Please update this label or adjust your self-hosted runner configuration accordingly.🧰 Tools
🪛 actionlint (1.7.4)
25-25: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
msukkari
changed the title
* SQL Database (#157) * point zoekt to v3 branch * bump zoekt version * Add tenant ID concept into web app and backend (#160) * hacked together a example of using zoekt grpc api * provide tenant id to zoekt git indexer * update zoekt version to point to multitenant branch * pipe tenant id through header to zoekt * remove incorrect submodule reference and settings typo * update zoekt commit * remove unused yarn script * remove unused grpc client in web server * remove unneeded deps and improve tenant id log * pass tenant id when creating repo in db * add mt yarn script * add nocheckin comment to tenant id in v2 schema --------- Co-authored-by: bkellam <bshizzle1234@gmail.com> * bump zoekt version * parallelize repo indexing (#163) * hacked together a example of using zoekt grpc api * provide tenant id to zoekt git indexer * update zoekt version to point to multitenant branch * pipe tenant id through header to zoekt * remove incorrect submodule reference and settings typo * update zoekt commit * remove unused yarn script * remove unused grpc client in web server * remove unneeded deps and improve tenant id log * pass tenant id when creating repo in db * add mt yarn script * add pol of bullmq into backend * add better error handling and concurrency setting * spin up redis instance in dockerfile * cleanup transaction logic when adding repos to index queue * add NEW index status fetch condition * move bullmq deps to backend --------- Co-authored-by: bkellam <bshizzle1234@gmail.com> * Authentication (#164) * Add Org table (#167) * Move logout button & profile picture into settings dropdown (#172) * Multi tenancy support in config syncer (#171) * [wip] initial mt support in config syncer * Move logout button & profile picture into settings dropdown (#172) * update sync status properly and fix bug with multiple config in db case * make config path required in single tenant mode NOTE: deleting config/repos is currently not supported in multi tenancy case. Support for this will be added in a future PR --------- Co-authored-by: Brendan Kellam <bshizzle1234@gmail.com> * add tenant mode support in docker container: * Organization switching & active org management (#173) * updated syncedAt date after config sync: * Migrate to postgres (#174) * spin up postgres in docker container * get initial pol of postgres db working in docker image * spin up postgres server in dev case * updated syncedAt date after config sync: * remove unnecessary port expose in docker file * Connection creation form (#175) * fix issue with yarn dev startup * init (#176) * Add `@sourcebot/schemas` package (#177) * Connection management (#178) * add concept of secrets (#180) * add @sourcebot/schemas package * migrate things to use the schemas package * Dockerfile support * add secret table to schema * Add concept of connection manager * Rename Config->Connection * Handle job failures * Add join table between repo and connection * nits * create first version of crypto package * add crypto package as deps to others * forgot to add package changes * add server action for adding and listing secrets, create test page for it * add secrets page to nav menu * add secret to config and support fetching it in backend * reset secret form on successful submission * add toast feedback for secrets form * add instructions for adding encryption key to dev instructions * add encryption key support in docker file * add delete secret button * fix nits from pr review --------- Co-authored-by: bkellam <bshizzle1234@gmail.com> * bump zoekt version * enforce tenancy on search and repo listing endpoints (#181) * enforce tenancy on search and repo listing * remove orgId from request schemas * adds garbage collection for repos (#182) * refactor repo indexing logic into RepoManager * wip cleanup stale repos * add rest of gc logic * set status to indexing properly * add initial logic for staging environment * try to move encryption key env decleration in docker file to fix build issues * switch encryption key as build arg to se if that fixes build issues * add deployment action for staging image * try using mac github action runners instead * switch to using arm64 runners on arm64 build * change workflow names to fix trigger issue * trigger staging actions to see if it works * fix working directory typo and pray it doesnt push to prod * checkout v3 when deploying staging * try to change into the staging dir manuall * dummy commit to trigger v3 workflows to test * update staging deploy script to match new version in main * reference proper image:tag in staging fly config * update staging fly config to point to ghcr * Connection management (#183) * add invite system and google oauth provider (#185) * add settings page with members list * add invite to schema and basic create form * add invite table * add basic invite link copy button * add auth invite accept case * add non auth logic * add google oauth provider * fix reference to header component in connections * add google logo to google oauth * fix web build errors * bump staging resources * change staging cpu to perf * add side bar nav in settings page * improve styling of members page * wip adding stripe checkout button * wip onboarding flow * add stripe subscription id to org * save stripe session id and add manage subscription button in settings * properly block access to pages if user isn't in an org * wip add paywall * Domain support * Domain support (#188) * Update Makefile to include crypto package when doing a make clean * Add default for AUTH_URL in attempt to fix build * attempt 2 * fix attempt #3: Do not require a encrpytion key at build time * Fix generate script race condition * Attempt #4 * add back paywall and also add support for incrememnting seat count on invite redemption * prevent self invite * action button styling in settings and toast on copy * add ability to remove member from org * move stripe product id to env var * add await for blocking loop in backend * add subscription info to billing page * handle trial case in billing info page * add trial duration indicator to nav bar * check if domain starts or ends with dash * remove unused no org component * Generate AUTH_SECRET if not provided (#189) * remove package lock file and fix prisma dep version * revert dep version updates * fix yarn.lock * add auth and membership check to fetchSubscription * properly handle invite redeem with no valid subscription case * change back fetch subscription to not require org membership * add back subscription check in invite redeem page * Add stripe billing logic (#190) * add side bar nav in settings page * improve styling of members page * wip adding stripe checkout button * wip onboarding flow * add stripe subscription id to org * save stripe session id and add manage subscription button in settings * properly block access to pages if user isn't in an org * wip add paywall * Domain support * add back paywall and also add support for incrememnting seat count on invite redemption * prevent self invite * action button styling in settings and toast on copy * add ability to remove member from org * move stripe product id to env var * add await for blocking loop in backend * add subscription info to billing page * handle trial case in billing info page * add trial duration indicator to nav bar * check if domain starts or ends with dash * remove unused no org component * remove package lock file and fix prisma dep version * revert dep version updates * fix yarn.lock * add auth and membership check to fetchSubscription * properly handle invite redeem with no valid subscription case * change back fetch subscription to not require org membership * add back subscription check in invite redeem page --------- Co-authored-by: bkellam <bshizzle1234@gmail.com> * fix nits * remove providers check * fix more nits * change stripe init to be behind function * fix publishible stripe key handling in docker container * enforce owner perms (#191) * add make owner logic, and owner perms for removal, invite, and manage subscription * add change billing email card to billing settings * enforce owner role in action level * remove unused hover card component * cleanup * add back gitlab, gitea, and gerrit support (#184) * add non github config definitions * refactor github config compilation to seperate file * add gitlab config compilation * Connection management (#183) * wip gitlab repo sync support * fix gitlab zoekt metadata * add gitea support * add gerrit support * Connection management (#183) * add gerrit config compilation * Connection management (#183) --------- Co-authored-by: Brendan Kellam <bshizzle1234@gmail.com> * fix apos usage in redeem page * change csrf cookie to secure not host * Credentials provider (#192) * email password functionality * feedback * cleanup org's repos and shards if it's inactive (#194) * add stripe subscription status and webhook * add inactive org repo cleanup logic * mark reactivated org connections for sync * connections qol improvements (#195) * add client side polling to connections list * properly fetch repo image url * add client polling to connection management page, and add ability to sync failed connections * Fix build with suspense boundary * improved fix * add retries for 429 issues (#196) * add connection compile retry and hard repo limit * add more retry checks * cleanup unused change * address feedback * fix build errors and add index concurrency env var * add config upsert timeout env var * Membership settings rework (#198) * Add refined members list * futher progress on members settings polish * Remove old components * feedback * Magic links (#199) * wip on magic link support * Switch to nodemailer / resend for transactional mail * Further cleanup * Add stylized email using react-email * fix * Fix build * db performance improvements and job resilience (#200) * replace upsert with seperate create many and raw update many calls * add bulk repo status update and queue addition with priority * add support for managed redis * add note for changing raw sql on schema change * remove non secret token options * fix token examples in schema * add better visualization for connection/repo errors and warnings (#201) * replace upsert with seperate create many and raw update many calls * add bulk repo status update and queue addition with priority * add support for managed redis * add note for changing raw sql on schema change * add error package and use BackendException in connection manager * handle connection failure display on web app * add warning banner for not found orgs/repos/users * add failure handling for gerrit * add gitea notfound warning support * add warning icon in connections list * style nits * add failed repo vis in connections list * added retry failed repo index buttons * move nav indicators to client with polling * fix indicator flash issue and truncate large list results * display error nav better * truncate failed repo list in connection list item * fix merge error * fix merge bug * add connection util file [wip] * refactor notfound fetch logic and add missing error package to dockerfile * move repeated logic to function and add zod schema for syncStatusMetadata * add orgid unique constraint to repo * revert repo compile update logic to upsert loop * log upsert stats * [temp] disable polling everywhere (#205) * add health check endpoint * Refined onboarding flow (#202) * Redeem UX pass (#204) * add log for health check * fix new connection complete callback route * add cpu split logic and only wait for postgres if we're going to connec to it * Inline secret creation (#207) * use docker scopes to try and improve caching * Dummy change * remove cpu split logic * Add some instrumentation to web * add posthog events on various user actions (#208) * add page view event support * add posthog events * nit: remove unused import * feedback * fix merge error * use staging posthog papik when building staging image * fix other merge error and build warnings * Add invite email (#209) * wrap posthog provider in suspense to fix build error * add grafana alloy config and setup (#210) * add grafana alloy config and setup * add basic repo prom metrics * nits in dockerfile * remove invalid characters when auto filling domain * add login posthog events * remove hard coded sourcebot.app references * make repo garbage collection async (#211) * add gc queue logic * fix missing switch cases for gc status * style org create form better with new staging domain * change repo rm logic to be async * simplify repo for inactive org query * add grace period for garbage collecting repos * make prom scrape interval 500ms * fix typo in trial card * onboarding tweaks * rename some prom metrics and cleanup unused * wipe existing repo if we've picked up a killed job to ensure good state * Connections UX pass + query optimizations (#212) * remove git & local schemas (#213) * skip stripe checkout for trial + fix indexing in progress UI + additional schema validation (#214) * add additional config validation * wip bypass stripe checkout for trial * fix stripe trial checkout bypass * fix indexing in progress ui on home page * add subscription checks, more schema validation, and fix issue with complete page * dont display if no indexed repos * fix skipping onboard complete check * fix build error * add back button in onboard connection creation flow * Add back revision support (#215) * fix build * Fix bug with repository snapshot * fix share links * fix repo rm issue, 502 page, condition on test clock * Make login and onboarding mobile friendly * fix ordering of quick actions * remove error msg dump on failed repo index job, and update indexedAt field * Add mobile unsupported splash screne * cherry pick fix for file links * [Cherry Pick] Syntax reference guide (#169) (#216) * Add .env to db gitignore * fix case where we have repos but they're all failed for repo snapshot * /settings/secrets page (#217) * display domain properly in org create form * Quick action tweaks (#218) * revamp repo page (#220) * wip repo table * new repo page * add indicator for when feedback is applied in repo page * add repo button * fetch connection data in one query * fix styling * fix (#219) * remove / keyboard shortcut hint in search bar * prevent switching to first page on data update and truncate long repo names in repo list * General settings + cleanup (#221) * General settings * Add alert to org domain change * First attempt at sending logs to grafana * logs wip * add alloy logs * wip * [temp] comment out loki for now * update trial card content and add events for code host selection on onboard * reduce scraping interval to 15s * Add prometheus metric for pending repo indexing jobs * switch magic link to invite code (#222) * wip magic link codes * pipe email to email provider properly * remove magic link data cookie after sign in * clean up unused imports * dont remove cookie before we use it * rm package-lock.json * revert yarn files to v3 state * switch email passing from cookie to search param * add comment for settings dropdown auth update * remove unused middleware file * fix build error and warnings * fix build error with useSearchParam not wrapped in suspense * add sentry support to backend and webapp (#223) * add sentry to web app * set sentry environemnt from env var * add sentry env replace logic in docker container * wip add backend sentry * add sentry to backend * move dns to env var * remove test exception * Fix root domain issue on onboarding * add setup sentry cli step to github action * login to sentry * fix sentry login in action * Update grafana loki endpoint * switch source map publish to runtime in entrypoint * catch and rethrow simplegit exceptions * alloy nits * fix alloy * backend logging (#224) * revert grafana loki config * fix login ui nits * fix quick actions * fix typo in secret creation * fix private repo clone issue for gitlab * add repo index timeout logic * add posthog identify call after registeration * various changes to add terms and security info (#225) * add terms and security to footer * add security card * add demo card * fix build error * nit fix: center 'get in touch' on security card * Dark theme improvements (#226) * (fix) Fixed bug with gitlab and gitea not including hostname in the repoName * Switch to using t3-env for env-var management (#230) * Add missing env var * fix build * Centralize to using a single .env.development for development workflows (#231) * Make billing optional (#232) * Massage environment variables from strings to numbers (#234) * Single tenancy & auth modes (#233) * Add docs to this repo * dummy change * Declarative connection configuration (#235) * fix build * upgrade to next 14.2.25 * Improved database DX * migrate to yarn v4 * Use origin from header for baseUrl of emails (instead of AUTH_URL). Also removed reference to hide scrollbars * Remove SOURCEBOT_ENCRYPTION_KEY from build arg * Fix issue with linking default user to org in single tenant + no-auth mode * Fix fallback tokens (#242) * add SECURITY_CARD_ENABLED flag * Add repository weburl (#243) * Random fixes and improvements (#244) * add zoekt max wall time env var * remove empty warning in docs * fix reference in sh docs * add connection manager upsert timeout env var * Declarative connection cleanup + improvements (#245) * change contact us footer in app to point to main contact form * PostHog event pass (#246) * fix typo * Add sourcebot cloud environment prop to staging workflow * Update generated files * remove AUTH_URL since it unused and (likely) unnecessary * Revert "remove AUTH_URL since it unused and (likely) unnecessary" This reverts commit 1f4a5ae. * cleanup GitHub action releases (#252) * remove alloy, change auth defaul to disabled, add settings page in me dropdown * enforce connection management perms to owner (#253) * enforce conneciton management perms to owner * fix formatting * more formatting * naming nits * fix var name error * change empty repo set copy if auth is disabled * add CONTRIBUTING.md file * hide settings in dropdown with auth isnt enabled * handle case where gerrit weburl is just gitiles path * Docs overhall (#251) * remove nocheckin * fix build error * remove v3 trigger from deploy staging * fix build errors round 2 * another error fix --------- Co-authored-by: msukkari <michael.sukkarieh@mail.mcgill.ca>
Summary by CodeRabbit
New UI & Navigation
Enhanced Onboarding & Authentication
Subscription & Organization Management
Improved Search Functionality