Repository navigation
feat(desktop): add Electric SQL sync and tasks view with organization switching - #511
Conversation
📝 WalkthroughWalkthroughThis PR introduces Electric SQL integration for local-first data synchronization across the Superset stack. It adds backend proxy routes for Electric, desktop PGlite databases with schema migrations, multi-organization support, a Tasks feature UI with organization switching, and CI/CD infrastructure for Electric deployment on Fly.io. The feature is gated behind the ELECTRIC_TASKS_ACCESS flag. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Desktop/Web Client
participant ProxyRoute as Electric Proxy Route
participant Auth as Auth Module
participant DB as Database
participant Electric as Electric Service
Client->>ProxyRoute: GET /api/electric/[...path]?table=tasks&organizationId=org123
ProxyRoute->>Auth: authenticateRequest(request)
Auth-->>ProxyRoute: userId
ProxyRoute->>DB: getUser(userId)
DB-->>ProxyRoute: user or null
alt User not found
ProxyRoute-->>Client: 401 Unauthorized
end
ProxyRoute->>DB: verifyOrgMembership(userId, organizationId)
alt Not member
ProxyRoute-->>Client: 403 Forbidden
end
ProxyRoute->>DB: buildWhereClause(table, organizationId)
DB-->>ProxyRoute: WhereClause { fragment, params }
ProxyRoute->>Electric: Forward request with where clause
Electric-->>ProxyRoute: Data response
ProxyRoute-->>Client: Proxied response (headers sanitized)
sequenceDiagram
participant User as User Action
participant UI as TasksView UI
participant PGlite as PGlite Database
participant Sync as Electric Sync
participant API as Backend API
User->>UI: Edit task (open dialog)
UI->>UI: Update local form state
User->>UI: Save changes
UI->>PGlite: db.update(tasks).set(...).where(...)
PGlite-->>UI: Optimistic update complete
UI->>UI: Close dialog & refresh useTasks query
Note over PGlite,Sync: Background sync
Sync->>API: POST /api/electric/[table] with update
API->>API: Validate auth & org membership
API-->>Sync: Response
Sync->>PGlite: Merge canonical state
UI->>UI: useLiveDrizzle subscription reflects changes
sequenceDiagram
participant Desktop as Desktop App
participant LocalDB as Local Database
participant PGlite as PGlite Instance
participant Electric as Electric Service
participant API as Backend API
Desktop->>Desktop: Initialize app
Desktop->>LocalDB: getActiveOrganizationId()
LocalDB-->>Desktop: orgId or fallback to first org
Desktop->>PGlite: openOrganizationDatabase(orgId)
PGlite->>PGlite: Create/open idb://superset_{orgId}
PGlite->>PGlite: Run migrations
PGlite-->>Desktop: DatabaseState { pg, db }
Desktop->>Desktop: Subscribe to accessToken
Note over PGlite,Electric: When token & db ready
Desktop->>Electric: startSync(pg, token, orgId)
Electric->>API: GET /api/electric/[...path]?organizationId=orgId
API-->>Electric: Org-scoped data shapes
Electric->>PGlite: Sync data into tables
PGlite-->>Desktop: Live subscriptions emit updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/src/lib/integrations/sync/tasks.ts (1)
28-39: Remove hardcoded ngrok URL - critical production deployment hazard.The hardcoded ngrok URL will cause failures in production environments:
- Deployment blocker: ngrok URLs are temporary and will break when the tunnel closes
- Security risk: Exposes internal development endpoints in production code
- Reliability: Creates unpredictable failures that are hard to diagnose
This must be resolved before merging.
🔎 Recommended fix: Use environment variable with fallback
- // TODO: In production, use env.NEXT_PUBLIC_API_URL - const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app"; + const qstashBaseUrl = env.NEXT_PUBLIC_API_URL; + + if (!qstashBaseUrl) { + throw new Error("NEXT_PUBLIC_API_URL environment variable is required for task sync"); + } const results = await Promise.allSettled( connections.map(async (conn) => {For local development, set
NEXT_PUBLIC_API_URLin your.env.localfile to your ngrok URL.
🧹 Nitpick comments (15)
apps/desktop/src/renderer/index.html (1)
14-14: Consider more restrictive CSP for Electric SQL proxy ports.The CSP allows connections to
http://127.0.0.1:*, which permits any port on localhost. While localhost is relatively safe, this is broader than necessary. The Electric SQL proxy uses specific ports (31417 for production, 31418 for development perconstants.ts).Consider whether specifying the exact ports would be feasible, though the wildcard approach does simplify the configuration for dual dev/prod environments.
Also applies to: 18-18
apps/desktop/src/main/lib/electric/sync-emitter.ts (1)
1-17: Consider adding type safety for event emissions.The implementation is straightforward and follows a common pub/sub pattern. However, Node's
EventEmitterdoesn't provide compile-time type safety for event names and payloads.Consider using a typed event emitter pattern or library (e.g.,
typed-emitter) to ensure events and their payloads are type-checked at compile time, reducing the risk of runtime errors from typos or incorrect payload types.Example typed emitter approach
import { EventEmitter } from "node:events"; interface SyncEvents { "tasks-updated": () => void; "organizations-updated": () => void; "organization-members-updated": () => void; "users-updated": () => void; } class SyncEmitter extends EventEmitter { emit<K extends keyof SyncEvents>(event: K, ...args: Parameters<SyncEvents[K]>): boolean { return super.emit(event, ...args); } on<K extends keyof SyncEvents>(event: K, listener: SyncEvents[K]): this { return super.on(event, listener); } }apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (2)
19-26: Consider scoping cache invalidation and removing debug logs.The
utils.invalidate()call invalidates the entire tRPC cache, which may cause unnecessary refetches. Consider scoping to relevant queries only. Also, the console.log/console.error statements appear to be debug artifacts.🔎 Proposed refinement
const setActiveOrganization = trpc.settings.setActiveOrganizationId.useMutation({ onSuccess: (data) => { - console.log("[org-switcher] Mutation succeeded:", data); - utils.invalidate(); + utils.settings.getActiveOrganizationId.invalidate(); + utils.tasks.invalidate(); }, - onError: (error) => { - console.error("[org-switcher] Mutation failed:", error); - }, + onError: (error) => {}, });
62-67: Remove debug logging.The console.log statement on line 63 should be removed before merging.
🔎 Proposed fix
onSelect={() => { - console.log("[org-switcher] Switching to:", organization.id); setActiveOrganization.mutate({ organizationId: organization.id, }); }}apps/api/src/app/api/electric/[...path]/utils.ts (1)
23-37: Fragile SQL extraction pattern.The regex-based extraction of the WHERE clause from the full query relies on Drizzle's query format remaining consistent. Consider adding a defensive check or using a more robust approach.
🔎 Suggested improvement
function build( table: PgTableWithColumns<any>, column: PgColumn, ids: string[], ): WhereClause { const whereExpr = inArray(sql`${sql.identifier(column.name)}`, ids); const qb = new QueryBuilder(); const { sql: query, params } = qb .select() .from(table) .where(whereExpr) .toSQL(); const fragment = query.replace(/^select .* from .* where\s+/i, ""); + // Sanity check: ensure we actually extracted a WHERE clause + if (fragment === query) { + throw new Error(`Failed to extract WHERE clause from: ${query}`); + } return { fragment, params }; }apps/api/src/app/api/electric/[...path]/route.ts (1)
67-79: Consider adding a timeout to the Electric fetch.The fetch to Electric has no timeout, which could cause the request to hang indefinitely if Electric is unresponsive.
🔎 Proposed improvement
-// Forward to Electric -const response = await fetch(originUrl.toString()); +// Forward to Electric with timeout +const controller = new AbortController(); +const timeoutId = setTimeout(() => controller.abort(), 30000); + +try { + const response = await fetch(originUrl.toString(), { + signal: controller.signal, + }); + clearTimeout(timeoutId); + + // Must remove these headers per Electric docs + const headers = new Headers(response.headers); + headers.delete("content-encoding"); + headers.delete("content-length"); + + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + }); +} catch (error) { + if (error instanceof Error && error.name === "AbortError") { + return new Response("Electric timeout", { status: 504 }); + } + throw error; +}apps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.md (1)
246-259: Verify ifelectric_sync_statetable is needed.The documentation notes a TODO to investigate whether this table is necessary since Electric's ShapeStream may handle offset/resume internally. Consider resolving this before or shortly after merging.
Would you like me to open an issue to track this investigation?
apps/desktop/src/lib/trpc/routers/organizations/index.ts (1)
21-33: Consider extracting duplicated query logic.The membership query is duplicated between
listandonUpdate. A helper function would reduce duplication.🔎 Proposed refactor
+const getOrganizationsForUser = (userId: string) => { + const memberships = localDb + .select({ organization: organizations }) + .from(organizationMembers) + .innerJoin(organizations, eq(organizationMembers.organization_id, organizations.id)) + .where(eq(organizationMembers.user_id, userId)) + .all(); + return memberships.map((m) => m.organization); +}; + export const createOrganizationsRouter = () => { return router({ list: publicProcedure.query(async () => { const user = await apiClient.user.me.query(); if (!user) { return []; } - - const memberships = localDb - .select({ - organization: organizations, - }) - .from(organizationMembers) - .innerJoin( - organizations, - eq(organizationMembers.organization_id, organizations.id), - ) - .where(eq(organizationMembers.user_id, user.id)) - .all(); - - return memberships.map((m) => m.organization); + return getOrganizationsForUser(user.id); }),Also applies to: 46-56
apps/desktop/src/main/lib/electric/sync-service.ts (3)
30-37: Guard against duplicate subscriptions on repeatedstartSync()calls.If
startSync()is called multiple times without callingstopSync()first (e.g., token refresh, re-auth), the existing shapes remain subscribed while new ones are added, causing duplicate event handlers and database writes.Proposed fix
const shapes: Shape[] = []; export async function startSync(): Promise<void> { + // Clean up any existing subscriptions before starting new ones + stopSync(); + const token = await authService.getAccessToken(); if (!token) { console.log("[sync] No auth token, skipping"); return; }
51-61: Wrap delete + inserts in a transaction for atomicity.The current approach deletes all rows then inserts one-by-one. Without a transaction, concurrent reads may observe an empty or partially-populated table. Additionally, if an insert fails mid-loop, the table is left in an inconsistent state.
Proposed fix using transaction
shape.subscribe(({ rows }) => { console.log(`[sync] ${tableName}: ${rows.length} rows`); - localDb.delete(config.table).run(); - for (const row of rows) { - localDb - .insert(config.table) - .values(row as Record<string, unknown>) - .run(); - } + localDb.transaction((tx) => { + tx.delete(config.table).run(); + for (const row of rows) { + tx.insert(config.table) + .values(row as Record<string, unknown>) + .run(); + } + }); syncEmitter.emit(config.event); });
48-62: Add error handling for shape subscription failures.The Electric SQL Shape subscription may encounter network errors, authentication failures, or stream disconnections. Without an error handler, these failures will be silent.
const shape = new Shape(stream); shapes.push(shape); - shape.subscribe(({ rows }) => { + shape.subscribe( + ({ rows }) => { console.log(`[sync] ${tableName}: ${rows.length} rows`); localDb.delete(config.table).run(); for (const row of rows) { localDb .insert(config.table) .values(row as Record<string, unknown>) .run(); } syncEmitter.emit(config.event); - }); + }, + (error) => { + console.error(`[sync] ${tableName} error:`, error); + } + );apps/desktop/src/lib/trpc/routers/tasks/index.ts (1)
10-19: Consider importing the shared schema instead of duplicating.This schema duplicates
updateTaskSchemafrompackages/trpc/src/router/task/schema.tsbut with fewer fields (missingrepositoryId,branch,prUrl,labels). If this is intentional to limit editable fields on desktop, document the reason; otherwise, import and extend/pick from the shared schema to maintain consistency.Option: Import and pick from shared schema
import { updateTaskSchema as baseUpdateTaskSchema } from "@superset/trpc/router/task/schema"; const updateTaskSchema = baseUpdateTaskSchema.pick({ id: true, title: true, description: true, status: true, priority: true, assigneeId: true, estimate: true, dueDate: true, });apps/desktop/src/lib/trpc/routers/settings/index.ts (1)
210-225: Consider validating that the organization exists before persisting.The mutation accepts any UUID without verifying it exists in the
organizationstable. A malformed or stale organization ID could be persisted, causing downstream failures.Proposed validation
setActiveOrganizationId: publicProcedure .input(z.object({ organizationId: z.string().uuid() })) .mutation(({ input }) => { + const org = localDb + .select() + .from(organizations) + .where(eq(organizations.id, input.organizationId)) + .get(); + if (!org) { + throw new Error("Organization not found"); + } + localDb .insert(settings)apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (2)
44-349: Multiple components in single file violates coding guidelines.As per coding guidelines: "Use one component per file - do not combine multiple components in a single file." This file contains 5 components:
TaskEditDialog,TaskCard,TasksList,Sidebar, andTasksViewContent.Consider extracting each to its own file under
components/:
components/TaskEditDialog/TaskEditDialog.tsxcomponents/TaskCard/TaskCard.tsxcomponents/TasksList/TasksList.tsxcomponents/Sidebar/Sidebar.tsx
256-277: Add error handling for the subscription.If the
onUpdatesubscription fails (e.g., connection issues), the UI will remain stuck in the loading state indefinitely. Consider adding anonErrorcallback to display an error state.Proposed fix
function TasksList() { const [tasks, setTasks] = useState<Task[] | null>(null); const [editingTask, setEditingTask] = useState<Task | null>(null); + const [error, setError] = useState<Error | null>(null); trpc.tasks.onUpdate.useSubscription(undefined, { onData: ({ tasks: updatedTasks }) => { setTasks(updatedTasks); }, + onError: (err) => { + setError(err); + }, }); const isLoading = tasks === null; + if (error) { + return ( + <div className="flex items-center justify-center h-64"> + <div className="flex flex-col items-center gap-2 text-destructive"> + <span className="text-sm">Failed to load tasks: {error.message}</span> + </div> + </div> + ); + } + if (isLoading) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
AGENTS.mdapps/api/package.jsonapps/api/src/app/api/electric/[...path]/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/env.tsapps/api/src/lib/auth.tsapps/api/src/trpc/context.tsapps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.mdapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/organizations/index.tsapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/app-environment.tsapps/desktop/src/main/lib/electric/index.tsapps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/main/lib/electric/sync-service.tsapps/desktop/src/main/lib/local-db/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/renderer/index.htmlapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/shared/constants.tspackages/local-db/drizzle/0001_add_synced_tables.sqlpackages/local-db/drizzle/0002_add_active_organization_id.sqlpackages/local-db/drizzle/meta/0001_snapshot.jsonpackages/local-db/drizzle/meta/0002_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.tspackages/trpc/src/lib/integrations/sync/tasks.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/main/lib/app-environment.ts
🧰 Additional context used
📓 Path-based instructions (14)
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/env.tsapps/api/src/app/api/electric/[...path]/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/lib/auth.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/trpc/context.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/api/src/app/api/integrations/linear/callback/route.tsapps/desktop/src/main/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/main/lib/electric/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/api/src/env.tsapps/desktop/src/main/lib/electric/sync-service.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/api/src/app/api/electric/[...path]/route.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/api/src/lib/auth.tsapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/main/lib/local-db/index.tspackages/local-db/src/schema/schema.tsapps/desktop/src/shared/constants.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/api/src/trpc/context.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/organizations/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxpackages/trpc/src/lib/integrations/sync/tasks.tsapps/desktop/src/lib/trpc/routers/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/api/src/app/api/integrations/linear/callback/route.tsapps/desktop/src/main/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/main/lib/electric/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/api/src/env.tsapps/desktop/src/main/lib/electric/sync-service.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/api/src/app/api/electric/[...path]/route.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/api/src/lib/auth.tsapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/main/lib/local-db/index.tspackages/local-db/src/schema/schema.tsapps/desktop/src/shared/constants.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/api/src/trpc/context.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/organizations/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxpackages/trpc/src/lib/integrations/sync/tasks.tsapps/desktop/src/lib/trpc/routers/index.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/main/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/main/lib/electric/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/main/lib/electric/sync-service.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/main/lib/local-db/index.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/index.tsapps/desktop/src/main/lib/electric/index.tsapps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/main/lib/electric/sync-service.tsapps/desktop/src/main/lib/local-db/index.ts
apps/desktop/src/main/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Files:
apps/desktop/src/main/index.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/main/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/main/lib/electric/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/main/lib/electric/sync-service.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/main/lib/local-db/index.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/lib/trpc/routers/organizations/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/lib/trpc/routers/index.ts
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests co-located with components using .test.ts or .test.tsx naming convention
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent responsibilities and interactions in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (16)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/desktop/package.jsonapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/{components,features}/**/[!.]*.tsx : Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsAGENTS.mdapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Applied to files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/screens/main/index.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/renderer/contexts/TRPCProvider.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/shared/constants.tsapps/desktop/src/lib/trpc/routers/tasks/index.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx} : Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/main/lib/electric/sync-emitter.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/api/src/env.tsapps/desktop/src/shared/constants.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
apps/desktop/src/main/lib/local-db/index.tspackages/local-db/src/schema/schema.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsAGENTS.mdpackages/local-db/drizzle/meta/0002_snapshot.json
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
apps/desktop/src/main/lib/local-db/index.tsapps/api/src/app/api/electric/[...path]/utils.tsAGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Create database migrations by modifying Drizzle schema in packages/db/src/schema and running drizzle-kit generate with snake_case migration names
Applied to files:
packages/local-db/drizzle/0001_add_synced_tables.sqlAGENTS.md
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-11-24T21:32:17.800Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:17.800Z
Learning: Applies to AGENTS.md : Document agent responsibilities and interactions in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Keep diffs minimal with targeted edits only - do not make broad refactoring changes
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Follow existing code patterns and match codebase style
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Do not run dev servers in automation
Applied to files:
AGENTS.md
🧬 Code graph analysis (16)
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
apps/api/src/env.ts (1)
env(4-44)
apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (1)
apps/api/src/trpc/context.ts (1)
createContext(4-12)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts (1)
getShellEnvironment(24-78)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(38-78)
apps/api/src/app/api/electric/[...path]/route.ts (4)
apps/api/src/lib/auth.ts (1)
authenticateRequest(35-57)packages/local-db/src/schema/schema.ts (2)
users(145-161)organizationMembers(193-210)apps/api/src/env.ts (1)
env(4-44)apps/api/src/app/api/electric/[...path]/utils.ts (1)
buildWhereClause(39-70)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (1)
packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)
apps/api/src/lib/auth.ts (1)
apps/api/src/env.ts (1)
env(4-44)
apps/desktop/src/shared/constants.ts (6)
apps/api/src/env.ts (1)
env(4-44)packages/trpc/src/env.ts (1)
env(4-24)apps/desktop/src/main/env.main.ts (1)
env(12-44)apps/desktop/src/renderer/env.renderer.ts (1)
env(45-47)apps/web/src/env.ts (1)
env(5-53)apps/desktop/src/shared/env.shared.ts (1)
env(28-30)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
packages/local-db/src/schema/schema.ts (4)
tasks(218-264)organizationMembers(193-210)organizations(169-185)users(145-161)
apps/desktop/src/renderer/screens/main/index.tsx (2)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
TasksView(343-349)apps/desktop/src/renderer/screens/main/components/TasksView/index.ts (1)
TasksView(1-1)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (3)
packages/local-db/src/schema/schema.ts (3)
SelectTask(267-267)TaskPriority(139-139)tasks(218-264)apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (1)
OrganizationSwitcher(11-90)apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (1)
ActiveOrganizationProvider(12-51)
apps/api/src/trpc/context.ts (3)
apps/api/src/lib/auth.ts (1)
authenticateRequest(35-57)packages/trpc/src/trpc.ts (1)
createTRPCContext(20-24)packages/trpc/src/index.ts (1)
createTRPCContext(9-9)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(38-78)
apps/desktop/src/lib/trpc/routers/tasks/index.ts (5)
packages/trpc/src/router/task/schema.ts (1)
updateTaskSchema(19-32)apps/desktop/src/main/lib/local-db/index.ts (1)
localDb(81-81)packages/local-db/src/schema/schema.ts (3)
settings(117-129)tasks(218-264)SelectTask(267-267)apps/desktop/src/main/lib/electric/index.ts (2)
syncEmitter(1-1)SYNC_EVENTS(1-1)apps/desktop/src/main/lib/api-client.ts (1)
apiClient(11-27)
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (4)
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.ts (1)
OrganizationSwitcher(1-1)packages/local-db/src/schema/schema.ts (1)
organizations(169-185)packages/ui/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(249-249)DropdownMenuTrigger(251-251)DropdownMenuContent(252-252)DropdownMenuItem(255-255)packages/ui/src/components/ui/avatar.tsx (3)
Avatar(53-53)AvatarImage(53-53)AvatarFallback(53-53)
apps/desktop/src/lib/trpc/routers/index.ts (2)
apps/desktop/src/lib/trpc/routers/tasks/index.ts (1)
createTasksRouter(21-79)apps/desktop/src/lib/trpc/routers/organizations/index.ts (1)
createOrganizationsRouter(13-77)
🪛 markdownlint-cli2 (0.18.1)
apps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
190-190: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
264-264: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
310-310: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
326-326: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
523-523: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
574-574: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
597-597: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
641-641: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
707-707: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
738-738: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AGENTS.md
149-149: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
149-149: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Web
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Admin
- GitHub Check: Build
🔇 Additional comments (34)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.ts (1)
30-154: LGTM! Tests correctly updated for async teardown.All test functions have been properly updated to use
async/awaitfor the now-asynchronousrunTeardown()function. The changes are consistent and maintain complete test coverage for success paths, error handling, configuration loading, and environment variable passing.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (1)
90-95: LGTM! Correctly simplified to handle only terminalWarning.The removal of
teardownErrorhandling aligns with the backend changes where teardown is now executed asynchronously in a fire-and-forget manner (lines 874-885 in workspaces.ts). The mutation response no longer includesteardownError, so this simplification is correct.apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (2)
38-42: LGTM! Proper async signature for shell environment loading.The function signature correctly changed to async and returns
Promise<TeardownResult>, which allows awaiting the shell environment retrieval for improved PATH and environment variable resolution.
53-63: LGTM! Shell environment integration improves teardown reliability.Using
getShellEnvironment()instead ofprocess.envensures teardown scripts run with the user's shell-configured PATH and environment variables. According to the shell-env.ts implementation, this function handles errors gracefully by falling back toprocess.envif shell spawning fails, so the teardown remains robust.apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (2)
874-885: Fire-and-forget teardown pattern correctly implemented.The teardown is now invoked asynchronously without blocking the workspace deletion. Failures are logged to the console but don't prevent the deletion from succeeding. This is a reasonable design choice since:
- The worktree is already being removed from disk (line 890)
- Teardown script failures shouldn't prevent cleanup
- Users still receive
terminalWarningfor actionable issues
955-955: Response shape correctly updated to exclude teardownError.The deletion mutation now returns only
{ success: true, terminalWarning }, which aligns with the fire-and-forget teardown pattern. Teardown errors are logged but not surfaced to the client, simplifying error handling in the UI (as seen in DeleteWorkspaceDialog.tsx).apps/desktop/src/shared/constants.ts (1)
16-17: LGTM!The ELECTRIC port constant follows the established pattern for environment-aware port configuration, with clear documentation.
apps/api/src/trpc/context.ts (1)
2-2: LGTM! Clean authentication refactor.The centralization of authentication logic into
authenticateRequestsignificantly improves maintainability by removing duplication and following the single responsibility principle. The context creation is now cleaner and easier to understand.Also applies to: 10-11
apps/api/src/env.ts (1)
9-11: LGTM! Appropriate environment variable validation.The Electric SQL configuration variables use appropriate Zod 4 validators:
- URL validation for the endpoint
- UUID validation for the source identifier
- String validation for the secret
This follows security best practices and the established pattern in the file.
apps/desktop/src/renderer/contexts/TRPCProvider.tsx (1)
14-14: No changes needed—error handling fully compensates for disabled retries.Disabling retries combined with
networkMode: "always"is an intentional design choice aligned with Electric SQL's real-time synchronization. The codebase implements comprehensive error handling at the mutation and hook level (onError callbacks with toast notifications, cache invalidation, state management), ensuring transient failures are properly surfaced to users. The pattern is appropriate for IPC-based communication where retry semantics differ from HTTP requests.apps/api/src/lib/auth.ts (1)
35-57: LGTM!The dual authentication flow is well-structured, correctly prioritizing Clerk auth while providing a clean fallback to desktop JWT tokens.
apps/desktop/src/main/lib/local-db/index.ts (1)
74-77: LGTM - Foreign key constraints intentionally disabled.Per the PR description, FK constraints are disabled because the local DB serves as a sync cache where the cloud Postgres is the source of truth. This prevents constraint violations during Electric SQL sync operations.
apps/api/src/app/api/electric/[...path]/route.ts (1)
15-36: LGTM!The authentication and authorization flow is well-implemented with appropriate status codes: 401 for unauthenticated/unknown user, 403 for users without organization memberships.
packages/local-db/src/schema/schema.ts (2)
133-141: LGTM - Well-documented synced tables section.The comment clearly explains the snake_case naming convention for Electric SQL compatibility. The type definitions for TaskPriority and IntegrationProvider provide good type safety.
218-264: LGTM - Tasks table schema is comprehensive.The tasks table includes all necessary fields for Linear integration with appropriate foreign key relationships, indexes on commonly queried columns (organization_id, assignee_id, status, created_at), and proper onDelete behaviors.
apps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.md (1)
1-61: Excellent architecture documentation.This comprehensive documentation clearly explains the Electric SQL data flow, synced vs local tables, and the security model. The ASCII diagrams effectively illustrate the architecture.
packages/local-db/drizzle/0001_add_synced_tables.sql (1)
28-68: LGTM - Tasks table migration.The tasks table includes comprehensive fields for Linear integration with appropriate indexes on commonly queried columns.
apps/desktop/src/renderer/screens/main/index.tsx (1)
23-23: LGTM! Clean integration of TasksView.The import and conditional rendering follow the existing pattern established for other views (settings, workspace). The implementation is type-safe and consistent with the codebase structure.
Also applies to: 193-195
apps/desktop/src/main/index.ts (2)
133-136: LGTM! Proper error handling for sync startup.The sync initialization is correctly placed after authentication and includes appropriate error logging.
154-157: The original concern is unfounded.stopSync()is a synchronous function that returnsvoid, not a Promise. Its implementation simply iterates through shapes and callsunsubscribeAll()on each (synchronous operations), then clears the shapes array. It does not perform any asynchronous operations and is correctly called without awaiting.Likely an incorrect or invalid review comment.
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.ts (1)
1-1: LGTM! Follows component organization guidelines.The barrel export pattern is consistent with the project's component structure guidelines.
Based on learnings, this follows the pattern of one folder per component with index.ts barrel export.
packages/local-db/drizzle/0002_add_active_organization_id.sql (1)
1-1: LGTM! Clean migration for active organization support.The migration adds the necessary column to persist the active organization selection. The nullable text type is appropriate for this use case.
apps/desktop/src/renderer/screens/main/components/TasksView/index.ts (1)
1-1: LGTM! Standard barrel export pattern.packages/local-db/drizzle/meta/_journal.json (1)
11-25: LGTM! Drizzle migration metadata correctly generated.The journal entries properly track the two new migrations (synced tables and active organization ID). This is auto-generated metadata and the structure is correct.
apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx (2)
29-50: LGTM!The Tasks menu item follows the established pattern used for Settings. Clean integration with the
useOpenTaskshook.
106-109: LGTM!Consistent placement and styling with other menu items.
apps/desktop/src/lib/trpc/routers/index.ts (1)
10-41: LGTM!New routers follow the established factory pattern and are correctly wired into the AppRouter.
packages/local-db/drizzle/meta/0001_snapshot.json (1)
1-956: LGTM!The migration snapshot correctly captures the new synced tables schema with appropriate foreign keys, cascade behaviors, and indexes.
apps/desktop/src/lib/trpc/routers/organizations/index.ts (1)
36-74: LGTM!Correctly uses the
observablepattern from@trpc/server/observableas required by trpc-electron. Proper event listener cleanup on teardown. Based on coding guidelines requiring observable pattern for subscriptions.apps/desktop/src/main/lib/electric/index.ts (1)
1-2: LGTM!Clean barrel export consolidating the Electric sync module's public API.
apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (1)
12-51: LGTM! Well-structured context provider with proper error boundaries.The provider correctly handles loading, error, and success states with user-friendly UI. The hook enforces usage within the provider context.
apps/desktop/src/lib/trpc/routers/settings/index.ts (1)
173-208: Potential race condition: membership lookup may fail if sync hasn't completed.
getActiveOrganizationIdqueriesorganizationMembersandorganizationstables that are populated by Electric SQL sync. If this procedure is called before sync completes (e.g., immediately after auth), the membership lookup at lines 184-192 will returnnull, throwing "User has no organizations" even if the user does belong to organizations.Consider adding a fallback or retry mechanism, or ensure sync completes before this is called.
packages/local-db/drizzle/meta/0002_snapshot.json (1)
1-963: LGTM - Auto-generated Drizzle migration snapshot.This is an auto-generated schema snapshot that correctly captures the new tables (
organizations,organization_members,tasks,users) and theactive_organization_idcolumn insettings. Foreign keys and indexes are properly defined.apps/desktop/src/renderer/stores/app-state.ts (1)
4-88: LGTM! Clean Zustand state management for tasks view.The implementation follows existing patterns in the store, correctly uses Zustand as per coding guidelines, and provides atomic state transitions with appropriate convenience hooks.
| ** IMPORTANT ** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first. | ||
|
|
There was a problem hiding this comment.
Fix markdown emphasis formatting.
The static analysis tool flagged spaces inside emphasis markers. Per MD037, remove the spaces around the text within **.
🔎 Proposed fix
-** IMPORTANT ** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first.
+**IMPORTANT** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first.
📝 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.
| ** IMPORTANT ** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first. | |
| **IMPORTANT** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
149-149: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
149-149: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🤖 Prompt for AI Agents
In AGENTS.md around lines 149 to 150, the emphasis markers contain spaces ("**
IMPORTANT **") which violates MD037; remove the spaces so the bold is correctly
formatted (change "** IMPORTANT **" to "**IMPORTANT** - Never touch the
production database..." ), and scan the file for any other instances of spaced
emphasis and correct them similarly.
| // TODO: In production, use env.NEXT_PUBLIC_API_URL | ||
| const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app"; | ||
| try { | ||
| await qstash.publishJSON({ | ||
| url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, | ||
| body: { organizationId, creatorUserId: userId }, | ||
| retries: 3, | ||
| }); | ||
| } catch (error) { | ||
| console.error("Failed to queue initial sync job:", error); | ||
| // Connection saved successfully, just sync failed - redirect with warning | ||
| return Response.redirect( | ||
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?warning=sync_queued_failed`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Hardcoded ngrok URL will break in non-local environments.
This hardcoded URL will cause sync failures in staging/production. The env.NEXT_PUBLIC_API_URL is already available and validated per your env schema. Replace with the environment variable before merging.
🔎 Proposed fix
- // TODO: In production, use env.NEXT_PUBLIC_API_URL
- const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app";
+ const qstashBaseUrl = env.NEXT_PUBLIC_API_URL;
try {
await qstash.publishJSON({
url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`,
📝 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.
| // TODO: In production, use env.NEXT_PUBLIC_API_URL | |
| const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app"; | |
| try { | |
| await qstash.publishJSON({ | |
| url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, | |
| body: { organizationId, creatorUserId: userId }, | |
| retries: 3, | |
| }); | |
| } catch (error) { | |
| console.error("Failed to queue initial sync job:", error); | |
| // Connection saved successfully, just sync failed - redirect with warning | |
| return Response.redirect( | |
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?warning=sync_queued_failed`, | |
| ); | |
| } | |
| const qstashBaseUrl = env.NEXT_PUBLIC_API_URL; | |
| try { | |
| await qstash.publishJSON({ | |
| url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, | |
| body: { organizationId, creatorUserId: userId }, | |
| retries: 3, | |
| }); | |
| } catch (error) { | |
| console.error("Failed to queue initial sync job:", error); | |
| // Connection saved successfully, just sync failed - redirect with warning | |
| return Response.redirect( | |
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?warning=sync_queued_failed`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/callback/route.ts around lines 102
to 116, the code uses a hardcoded ngrok URL for qstash publishing which will
fail outside local dev; replace the hardcoded qstashBaseUrl with the validated
environment variable env.NEXT_PUBLIC_API_URL (remove the TODO), ensure you
normalize/trim any trailing slash before concatenating the path
`/api/integrations/linear/jobs/initial-sync`, and keep the existing try/catch
behavior so errors still redirect with the same warning.
| // TODO: In production, use env.NEXT_PUBLIC_API_URL | ||
| const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app"; | ||
| const isValid = await receiver.verify({ | ||
| body, | ||
| signature, | ||
| url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/linear/jobs/initial-sync`, | ||
| url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, |
There was a problem hiding this comment.
Signature verification will fail in production due to URL mismatch.
QStash signature verification requires the URL to match the actual endpoint. Using a hardcoded ngrok URL means verification will fail when deployed.
🔎 Proposed fix
- // TODO: In production, use env.NEXT_PUBLIC_API_URL
- const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app";
+ const qstashBaseUrl = env.NEXT_PUBLIC_API_URL;
const isValid = await receiver.verify({
body,
signature,
url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`,
});
📝 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.
| // TODO: In production, use env.NEXT_PUBLIC_API_URL | |
| const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app"; | |
| const isValid = await receiver.verify({ | |
| body, | |
| signature, | |
| url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/linear/jobs/initial-sync`, | |
| url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, | |
| const qstashBaseUrl = env.NEXT_PUBLIC_API_URL; | |
| const isValid = await receiver.verify({ | |
| body, | |
| signature, | |
| url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts around
lines 31 to 36, the QStash signature verification currently uses a hardcoded
ngrok URL which will not match in production; replace the hardcoded
qstashBaseUrl with a dynamic origin: read process.env.NEXT_PUBLIC_API_URL (or
another configured env var that contains the deployed origin) and use that for
the verify URL; if the env var is missing, derive the origin from the incoming
request (e.g., request.headers or request.nextUrl.origin) as a fallback, and
ensure the final URL exactly matches the public endpoint path
(/api/integrations/linear/jobs/initial-sync) before calling receiver.verify.
| list: publicProcedure.query(() => { | ||
| const { activeOrganizationId } = localDb.select().from(settings).get()!; | ||
| if (!activeOrganizationId) { | ||
| throw new Error("No active organization set"); | ||
| } |
There was a problem hiding this comment.
Non-null assertion may throw on fresh database.
localDb.select().from(settings).get()! assumes a settings row exists. On a fresh install before any settings are written, this will throw. The settings router has a getSettings() helper that ensures a row exists.
Proposed fix
list: publicProcedure.query(() => {
- const { activeOrganizationId } = localDb.select().from(settings).get()!;
+ const row = localDb.select().from(settings).get();
+ const activeOrganizationId = row?.activeOrganizationId;
if (!activeOrganizationId) {
throw new Error("No active organization set");
}
📝 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.
| list: publicProcedure.query(() => { | |
| const { activeOrganizationId } = localDb.select().from(settings).get()!; | |
| if (!activeOrganizationId) { | |
| throw new Error("No active organization set"); | |
| } | |
| list: publicProcedure.query(() => { | |
| const row = localDb.select().from(settings).get(); | |
| const activeOrganizationId = row?.activeOrganizationId; | |
| if (!activeOrganizationId) { | |
| throw new Error("No active organization set"); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/tasks/index.ts around lines 23 to 27, the
code uses localDb.select().from(settings).get()! which can throw on a fresh
install because the settings row may not exist; replace this direct select and
non-null assertion by calling the existing getSettings() helper (which ensures a
row is present) and read activeOrganizationId from its result, then keep the
same check and error throw if activeOrganizationId is falsy.
| const handler = () => { | ||
| const { activeOrganizationId } = localDb | ||
| .select() | ||
| .from(settings) | ||
| .get()!; | ||
| if (!activeOrganizationId) { | ||
| throw new Error("No active organization set"); | ||
| } |
There was a problem hiding this comment.
Use emit.error() instead of throwing inside the observable handler.
Throwing inside the observable callback will crash the subscription rather than gracefully propagating the error to subscribers. Use emit.error() to properly signal errors through the subscription channel.
Proposed fix
const handler = () => {
- const { activeOrganizationId } = localDb
- .select()
- .from(settings)
- .get()!;
- if (!activeOrganizationId) {
- throw new Error("No active organization set");
- }
+ const row = localDb.select().from(settings).get();
+ const activeOrganizationId = row?.activeOrganizationId;
+ if (!activeOrganizationId) {
+ emit.error(new Error("No active organization set"));
+ return;
+ }
const result = localDb
📝 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 handler = () => { | |
| const { activeOrganizationId } = localDb | |
| .select() | |
| .from(settings) | |
| .get()!; | |
| if (!activeOrganizationId) { | |
| throw new Error("No active organization set"); | |
| } | |
| const handler = () => { | |
| const row = localDb.select().from(settings).get(); | |
| const activeOrganizationId = row?.activeOrganizationId; | |
| if (!activeOrganizationId) { | |
| emit.error(new Error("No active organization set")); | |
| return; | |
| } | |
| const result = localDb |
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/tasks/index.ts around lines 42 to 49, the
observable handler currently throws an Error when no activeOrganizationId is
found which will crash the subscription; change the handler to call
emit.error(new Error("No active organization set")) instead of throwing, and
return immediately after calling emit.error() so execution stops and the error
is propagated to subscribers through the observable channel.
| function TaskEditDialog({ task, open, onOpenChange }: TaskEditDialogProps) { | ||
| const [title, setTitle] = useState(task.title); | ||
| const [description, setDescription] = useState(task.description || ""); | ||
| const [priority, setPriority] = useState(task.priority); | ||
| const [isSaving, setIsSaving] = useState(false); | ||
|
|
||
| const updateTask = trpc.tasks.update.useMutation({ | ||
| onSuccess: () => { | ||
| onOpenChange(false); | ||
| }, | ||
| onSettled: () => { | ||
| setIsSaving(false); | ||
| }, | ||
| }); | ||
|
|
||
| const handleSave = () => { | ||
| setIsSaving(true); | ||
| updateTask.mutate({ | ||
| id: task.id, | ||
| title, | ||
| description: description || null, | ||
| priority: priority as "urgent" | "high" | "medium" | "low" | "none", | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Form state doesn't reset when editing a different task.
useState initializes from task prop only on first render. If the user opens the dialog for Task A, closes it, then opens it for Task B, the form will still show Task A's values.
Proposed fix using key or useEffect
Option 1: Add key={task.id} to TaskEditDialog in the parent (Line 298-302):
{editingTask && (
<TaskEditDialog
+ key={editingTask.id}
task={editingTask}
open={!!editingTask}
onOpenChange={(open) => !open && setEditingTask(null)}
/>
)}
Option 2: Reset state when task changes:
+import { useState, useEffect } from "react";
+
function TaskEditDialog({ task, open, onOpenChange }: TaskEditDialogProps) {
const [title, setTitle] = useState(task.title);
const [description, setDescription] = useState(task.description || "");
const [priority, setPriority] = useState(task.priority);
const [isSaving, setIsSaving] = useState(false);
+
+ useEffect(() => {
+ setTitle(task.title);
+ setDescription(task.description || "");
+ setPriority(task.priority);
+ }, [task.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.
| function TaskEditDialog({ task, open, onOpenChange }: TaskEditDialogProps) { | |
| const [title, setTitle] = useState(task.title); | |
| const [description, setDescription] = useState(task.description || ""); | |
| const [priority, setPriority] = useState(task.priority); | |
| const [isSaving, setIsSaving] = useState(false); | |
| const updateTask = trpc.tasks.update.useMutation({ | |
| onSuccess: () => { | |
| onOpenChange(false); | |
| }, | |
| onSettled: () => { | |
| setIsSaving(false); | |
| }, | |
| }); | |
| const handleSave = () => { | |
| setIsSaving(true); | |
| updateTask.mutate({ | |
| id: task.id, | |
| title, | |
| description: description || null, | |
| priority: priority as "urgent" | "high" | "medium" | "low" | "none", | |
| }); | |
| }; | |
| import { useState, useEffect } from "react"; | |
| function TaskEditDialog({ task, open, onOpenChange }: TaskEditDialogProps) { | |
| const [title, setTitle] = useState(task.title); | |
| const [description, setDescription] = useState(task.description || ""); | |
| const [priority, setPriority] = useState(task.priority); | |
| const [isSaving, setIsSaving] = useState(false); | |
| useEffect(() => { | |
| setTitle(task.title); | |
| setDescription(task.description || ""); | |
| setPriority(task.priority); | |
| }, [task.id]); | |
| const updateTask = trpc.tasks.update.useMutation({ | |
| onSuccess: () => { | |
| onOpenChange(false); | |
| }, | |
| onSettled: () => { | |
| setIsSaving(false); | |
| }, | |
| }); | |
| const handleSave = () => { | |
| setIsSaving(true); | |
| updateTask.mutate({ | |
| id: task.id, | |
| title, | |
| description: description || null, | |
| priority: priority as "urgent" | "high" | "medium" | "low" | "none", | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
around lines 44 to 67, the TaskEditDialog initializes form state from the task
prop only on first render so opening the dialog for a different task retains the
previous values; fix by either (A) resetting local state when the task prop
changes: add a useEffect that runs on task.id (or task) to setTitle(task.title),
setDescription(task.description || ""), setPriority(task.priority), and
optionally setIsSaving(false), or (B) ensure the parent re-mounts the dialog per
task by passing key={task.id} when rendering TaskEditDialog so state is
initialized anew.
| CREATE TABLE `organization_members` ( | ||
| `id` text PRIMARY KEY NOT NULL, | ||
| `organization_id` text NOT NULL, | ||
| `user_id` text NOT NULL, | ||
| `role` text NOT NULL, | ||
| `created_at` text NOT NULL, | ||
| FOREIGN KEY (`organization_id`) REFERENCES `organizations`(`id`) ON UPDATE no action ON DELETE cascade, | ||
| FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON UPDATE no action ON DELETE cascade | ||
| ); |
There was a problem hiding this comment.
Table creation order incorrect for foreign key dependencies.
The organization_members table references organizations and users via foreign keys, but those tables are created later in the migration. This works because foreign_keys = OFF is set, but the migration order should ideally be: users → organizations → organization_members → tasks.
Since foreign keys are disabled for the sync cache, this won't cause runtime issues, but it's worth noting for future reference if FK constraints are ever re-enabled.
🤖 Prompt for AI Agents
In packages/local-db/drizzle/0001_add_synced_tables.sql around lines 1 to 9 the
CREATE TABLE for organization_members declares foreign keys to organizations and
users which are created later in this migration; reorder the statements so users
is created first, then organizations, then organization_members, and then tasks
(move this organization_members CREATE TABLE to after users and organizations)
to preserve FK dependency order in case foreign key checks are enabled in the
future.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
53-60: Previously flagged: emptyuserIdshandling.The concern from the prior review about empty
userIdscausing invalid SQL remains valid. If thebuild()function is updated with the empty array guard suggested above, this case will be handled automatically. Alternatively, add an explicit early return here for clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/app/api/electric/[...path]/route.tsapps/api/src/app/api/electric/[...path]/utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/electric/[...path]/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/electric/[...path]/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/electric/[...path]/route.ts
🧠 Learnings (2)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
apps/api/src/app/api/electric/[...path]/utils.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
apps/api/src/app/api/electric/[...path]/utils.ts
🧬 Code graph analysis (2)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
packages/local-db/src/schema/schema.ts (4)
tasks(218-264)organizationMembers(193-210)organizations(169-185)users(145-161)
apps/api/src/app/api/electric/[...path]/route.ts (4)
apps/api/src/lib/auth.ts (1)
authenticateRequest(35-57)packages/local-db/src/schema/schema.ts (2)
users(145-161)organizationMembers(193-210)apps/api/src/env.ts (1)
env(4-44)apps/api/src/app/api/electric/[...path]/utils.ts (1)
buildWhereClause(35-65)
🔇 Additional comments (3)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
39-51: LGTM - switch case structure is clean.The table-specific WHERE clause construction is well-organized. Assuming the
build()function is updated to handle empty arrays, these cases will work correctly.apps/api/src/app/api/electric/[...path]/route.ts (2)
15-33: LGTM - authentication and authorization flow.The authentication chain is well-structured with appropriate HTTP status codes (401 for unauthenticated, 403 for no memberships). The early returns prevent further processing for unauthorized requests.
58-62: Parameter indexing is correct per Electric SQL specification.The 1-based indexing (
params[${index + 1}]) matches Electric's documented format for the exploded object query syntax, where parameters are specified asparams[n]=valuewith n being 1-based indices that correspond to$1,$2, etc. in the where clause.
| function build(table: PgTable, column: PgColumn, ids: string[]): WhereClause { | ||
| const whereExpr = inArray(sql`${sql.identifier(column.name)}`, ids); | ||
| const qb = new QueryBuilder(); | ||
| const { sql: query, params } = qb | ||
| .select() | ||
| .from(table) | ||
| .where(whereExpr) | ||
| .toSQL(); | ||
| const fragment = query.replace(/^select .* from .* where\s+/i, ""); | ||
| return { fragment, params }; | ||
| } |
There was a problem hiding this comment.
Guard against empty ids array to prevent invalid SQL.
The inArray() function generates invalid SQL (IN ()) when passed an empty array, causing PostgreSQL to reject the query. This issue affects all table cases, not just the users case mentioned in the prior review.
Additionally, the column can be passed directly to inArray() rather than wrapping with sql.identifier().
🔎 Proposed fix
function build(table: PgTable, column: PgColumn, ids: string[]): WhereClause {
+ if (ids.length === 0) {
+ return { fragment: "1 = 0", params: [] };
+ }
- const whereExpr = inArray(sql`${sql.identifier(column.name)}`, ids);
+ const whereExpr = inArray(column, ids);
const qb = new QueryBuilder();
const { sql: query, params } = qb
.select()
.from(table)
.where(whereExpr)
.toSQL();
const fragment = query.replace(/^select .* from .* where\s+/i, "");
return { fragment, params };
}🤖 Prompt for AI Agents
In apps/api/src/app/api/electric/[...path]/utils.ts around lines 23 to 33, guard
against an empty ids array and pass the column directly to inArray: if
ids.length === 0 return a safe falsey WHERE fragment (for example "1=0") with an
empty params array to avoid generating IN (); otherwise call inArray(column,
ids) (do not wrap the column in sql.identifier()) and build the query as before
so the generated fragment and params are valid.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
apps/desktop/package.json (1)
37-41: Verify Electric SQL package versions still match published releases.A previous review flagged that these versions didn't exist on npm:
@electric-sql/client@1.3.1(highest was 1.3.0)@electric-sql/react@1.0.25(highest was 1.0.9)Please verify these versions now exist on npm, or update to published versions.
#!/bin/bash # Description: Verify Electric SQL package versions on npm registry echo "Checking @electric-sql/client versions..." npm view @electric-sql/client versions --json | jq -r '.[]' | grep -E '^1\.3\.[0-9]+$' | tail -5 echo -e "\nChecking @electric-sql/react versions..." npm view @electric-sql/react versions --json | jq -r '.[]' | grep -E '^1\.0\.[0-9]+$' | tail -5 echo -e "\nChecking @electric-sql/pglite latest version..." npm view @electric-sql/pglite version echo -e "\nChecking @electric-sql/pglite-react latest version..." npm view @electric-sql/pglite-react version echo -e "\nChecking @electric-sql/pglite-sync latest version..." npm view @electric-sql/pglite-sync versionapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
47-70: Form state doesn't reset when editing different tasks.Lines 48-51 initialize local state from the
taskprop, but this only happens on mount. If the dialog closes and reopens for a different task, the form will still show the previous task's values.🔎 Proposed fix options from previous review
Option 1: Add
key={editingTask.id}to force remount (in TasksList around lines 283-289):{editingTask && ( <TaskEditDialog + key={editingTask.id} task={editingTask} open={!!editingTask} onOpenChange={(open) => !open && setEditingTask(null)} /> )}Option 2: Reset state when task changes using useEffect:
+import { useState, useEffect } from "react"; + function TaskEditDialog({ task, open, onOpenChange }: TaskEditDialogProps) { const [title, setTitle] = useState(task.title); const [description, setDescription] = useState(task.description || ""); const [priority, setPriority] = useState(task.priority); const [isSaving, setIsSaving] = useState(false); + + useEffect(() => { + setTitle(task.title); + setDescription(task.description || ""); + setPriority(task.priority); + }, [task.id, task.title, task.description, task.priority]);As per previous review comment.
apps/api/src/app/api/electric/[...path]/utils.ts (1)
54-69: Handle emptyuserIdsarray to prevent invalid SQL.If no organization members are found,
userIdswill be empty, causinginArray()to generate invalid SQL syntax (IN ()) which PostgreSQL will reject. Additionally, thesql.identifier()wrapper is unnecessary.🔎 Proposed fix
case "users": { const members = await db.query.organizationMembers.findMany({ where: eq(organizationMembers.organizationId, organizationId), columns: { userId: true }, }); const userIds = [...new Set(members.map((m) => m.userId))]; + if (userIds.length === 0) { + return { fragment: "1 = 0", params: [] }; + } - const whereExpr = inArray(sql`${sql.identifier(users.id.name)}`, userIds); + const whereExpr = inArray(users.id, userIds); const qb = new QueryBuilder(); const { sql: query, params } = qb .select() .from(users) .where(whereExpr) .toSQL(); const fragment = query.replace(/^select .* from .* where\s+/i, ""); return { fragment, params }; }
🧹 Nitpick comments (15)
apps/desktop/docs/PER_ORG_PGLITE.md (1)
19-44: Add language specifier to code block or use alternative Markdown format.The fenced code block containing the architecture diagram is missing a language specifier. Since this is an ASCII diagram rather than executable code, consider either:
- Adding a language tag (e.g.,
```textor```plaintext)- Using Markdown syntax for diagrams if available in your rendering pipeline
This addresses the
MD040(fenced-code-language) lint rule.🔎 Proposed fix
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ Registry (Plain IndexedDB) │.superset/teardown.sh (1)
23-23: Consider checking if the Docker daemon is running, not just if the command exists.The current check only verifies that the
dockerCLI is installed. If the Docker daemon is stopped, the script will fail later with a less clear error message like "Cannot connect to the Docker daemon."💡 Suggested improvement to check daemon status
-command -v docker &> /dev/null || error "Docker not installed. Install from https://docker.com" +command -v docker &> /dev/null || error "Docker not installed. Install from https://docker.com" +docker info &> /dev/null || error "Docker daemon not running. Start Docker and try again."apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts (2)
44-49: Consider removing sync cleanup from DB lifecycle.Lines 46-47 unsubscribe sync in the DB lifecycle cleanup, but this is redundant since the sync lifecycle effect (lines 60-63) also unsubscribes on cleanup. This creates double-cleanup logic that's harder to reason about.
Consider removing sync cleanup here and letting the sync effect handle its own lifecycle:
🔎 Proposed simplification
return () => { cancelled = true; - syncRef.current?.unsubscribe(); - syncRef.current = null; setState(null); };This way:
- DB effect only manages database state
- Sync effect only manages sync subscription
- Cleaner separation of concerns
53-64: Sync effect dependency on organizationId is redundant.The sync effect depends on
[state, accessToken, organizationId], butorganizationIdchanges are already handled through thestatedependency (since changing org resets state to null, then sets it to the new DB). IncludingorganizationIdexplicitly creates redundant effect runs.🔎 Proposed simplification
- }, [state, accessToken, organizationId]); + }, [state, accessToken]);This is safe because:
- When
organizationIdchanges, the DB effect setsstateto null, then to the new DB- The
statechange triggers the sync effect naturally- No need to depend on
organizationIdexplicitlyapps/api/src/env.ts (1)
9-10: Consider stronger minimum length for ELECTRIC_SECRET.The current minimum length of 16 characters for
ELECTRIC_SECRETmeets basic requirements, but production secrets typically use 32+ characters for enhanced security. Since this secret authenticates requests to the Electric SQL proxy, consider increasing the minimum to match the 32-character requirement used forDESKTOP_AUTH_SECRETon Line 12.🔎 Suggested change
- ELECTRIC_SECRET: z.string().min(16), + ELECTRIC_SECRET: z.string().min(32),apps/desktop/src/renderer/lib/pglite/migrate.ts (2)
59-62: Use parameterized query for migration recording.String interpolation in SQL is vulnerable to injection. While the hash is computed from controlled migration content, using a parameterized query is safer and more consistent with best practices.
🔎 Proposed fix
// Record the migration - await pg.exec( - `INSERT INTO "${MIGRATIONS_TABLE}" (hash, created_at) VALUES ('${hash}', ${Date.now()})`, - ); + await pg.query( + `INSERT INTO "${MIGRATIONS_TABLE}" (hash, created_at) VALUES ($1, $2)`, + [hash, Date.now()], + );
44-63: Consider wrapping migration application in a transaction.If a migration fails partway through (after some statements execute but before recording), the database could be left in an inconsistent state. While this is acceptable for a local sync cache that can be rebuilt, wrapping in a transaction would provide atomicity.
🔎 Optional enhancement
async function applyMigration( pg: PGlite, migration: Migration, hash: string, ): Promise<void> { + await pg.exec("BEGIN"); + try { // Split by statement breakpoint and execute each statement const statements = migration.sql.split("--> statement-breakpoint"); for (const statement of statements) { const trimmed = statement.trim(); if (trimmed) { await pg.exec(trimmed); } } // Record the migration await pg.query( `INSERT INTO "${MIGRATIONS_TABLE}" (hash, created_at) VALUES ($1, $2)`, [hash, Date.now()], ); + await pg.exec("COMMIT"); + } catch (error) { + await pg.exec("ROLLBACK"); + throw error; + } }.superset/setup.sh (1)
73-79: Consider pinning the Electric SQL Docker image version.Using
:latestcan lead to unexpected behavior if the upstream image changes. Consider pinning to a specific version for reproducibility across developer environments.🔎 Example
- electricsql/electric:latest + electricsql/electric:1.0.0 # Pin to specific version.github/workflows/deploy-preview.yml (1)
92-95: Quote the sourced file path to prevent word splitting.Static analysis (shellcheck SC2086) flags this line. While unlikely to cause issues here, quoting is safer practice.
🔎 Proposed fix
- name: Load database URL run: | - source database-status.env + source "./database-status.env" echo "DATABASE_URL_UNPOOLED=$DATABASE_URL_UNPOOLED" >> $GITHUB_ENVapps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
23-37: Consider handling query errors.The provider handles loading and empty states but doesn't handle the case where the query fails with an error. Users would see the loading spinner indefinitely.
🔎 Proposed enhancement
export function OrganizationsProvider({ children }: { children: ReactNode }) { - const { data: organizations, isLoading } = + const { data: organizations, isLoading, error } = trpc.user.myOrganizations.useQuery(); if (isLoading) { return ( <div className="flex h-full items-center justify-center"> <div className="h-5 w-5 animate-spin rounded-full border-2 border-muted-foreground border-t-transparent" /> </div> ); } + if (error) { + return ( + <div className="p-4 text-destructive"> + Failed to load organizations: {error.message} + </div> + ); + } + if (!organizations?.length) { return <div className="p-4 text-destructive">No organizations found</div>; }apps/desktop/src/renderer/lib/pglite/hooks.ts (1)
33-42: Consider explicit handling for emptyuserIdsarray for clarity.While drizzle-orm 0.45.1 handles empty arrays in
inArraycorrectly (treating them as "matches nothing"), adding an explicit check aligns with patterns used elsewhere in the codebase for defensive clarity.export function useUsers(userIds: string[]) { const db = useDb(); + + if (userIds.length === 0) { + return useLiveQuery<SelectUser>("SELECT * FROM users WHERE false", []); + } + const query = db .select() .from(users) .where(and(inArray(users.id, userIds), isNull(users.deleted_at))); const { sql, params } = query.toSQL(); return useLiveQuery<SelectUser>(sql, params); }apps/api/src/app/api/electric/[...path]/utils.ts (1)
23-33: Simplify column reference ineq()call.The
sql.identifier(column.name)wrapper is unnecessary when using Drizzle'seq()operator. You can pass the column directly for cleaner, more idiomatic code.🔎 Proposed fix
function build(table: PgTable, column: PgColumn, id: string): WhereClause { - const whereExpr = eq(sql`${sql.identifier(column.name)}`, id); + const whereExpr = eq(column, id); const qb = new QueryBuilder(); const { sql: query, params } = qb .select() .from(table) .where(whereExpr) .toSQL(); const fragment = query.replace(/^select .* from .* where\s+/i, ""); return { fragment, params }; }apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
56-59: Validate organization ID before switching.
switchOrganizationaccepts any string without validating that the organization exists. This could cause the error at lines 43-45 to throw on the next render cycle.🔎 Proposed fix
const switchOrganization = (newOrganizationId: string) => { + const exists = organizations.find((o) => o.id === newOrganizationId); + if (!exists) { + console.error(`Organization not found: ${newOrganizationId}`); + return; + } localStorage.setItem(ACTIVE_ORG_KEY, newOrganizationId); setActiveOrganizationId(newOrganizationId); };apps/desktop/src/renderer/lib/pglite/database.ts (1)
34-46: Consider removing failed promises from cache.If
PGlite.create()ormigrate()throws, the rejected promise remains in the cache. Subsequent calls toopenOrganizationDatabasewill return the same rejected promise instead of retrying.🔎 Proposed fix
const promise = (async () => { - const pg = (await PGlite.create(`idb://superset_${organizationId}`, { - extensions: { live, sync: electricSync() }, - })) as PGliteWithExtensions; - - await migrate(pg); - - return { pg, db: drizzle(pg, { schema }) }; + try { + const pg = (await PGlite.create(`idb://superset_${organizationId}`, { + extensions: { live, sync: electricSync() }, + })) as PGliteWithExtensions; + + await migrate(pg); + + return { pg, db: drizzle(pg, { schema }) }; + } catch (error) { + dbCache.delete(organizationId); + throw error; + } })();apps/desktop/src/renderer/lib/pglite/schema/schema.ts (1)
152-155: Singleton pattern for local settings.Using
default(1)on the primary key creates a singleton row pattern. This works but consider adding a comment explaining the design intent for future maintainers.🔎 Proposed improvement
// ============================================================================= // Local-only tables (not synced via Electric) // ============================================================================= +// Singleton table - only one row with id=1 should exist export const localSettings = pgTable("local_settings", { id: integer("id").primaryKey().default(1), active_organization_id: uuid("active_organization_id"), });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.github/templates/preview-comment.md.github/workflows/cleanup-preview.yml.github/workflows/deploy-preview.yml.github/workflows/deploy-production.yml.gitignore.superset/setup.sh.superset/teardown.shapps/api/src/app/api/electric/[...path]/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/env.tsapps/api/src/proxy.tsapps/desktop/docs/LOCAL_FIRST_SYNC.mdapps/desktop/docs/PER_ORG_PGLITE.mdapps/desktop/drizzle.config.tsapps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/lib/trpc/routers/user/index.tsapps/desktop/src/main/lib/auth/auth-service.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/index.htmlapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/lib/pglite/migrations/0000_init.sqlapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/src/renderer/lib/pglite/migrations/meta/0000_snapshot.jsonapps/desktop/src/renderer/lib/pglite/migrations/meta/_journal.jsonapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxfly.tomlpackages/trpc/src/router/task/task.tspackages/trpc/src/router/user/user.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/lib/trpc/routers/settings/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/lib/pglite/migrations/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/app/api/electric/[...path]/route.ts
- apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
- apps/desktop/src/lib/trpc/routers/index.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/main/lib/auth/auth-service.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/drizzle.config.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/contexts/AppProviders.tsxpackages/trpc/src/router/user/user.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/api/src/env.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/lib/trpc/routers/user/index.tsapps/api/src/proxy.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/main/lib/auth/auth-service.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/drizzle.config.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/contexts/AppProviders.tsxpackages/trpc/src/router/user/user.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/api/src/env.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/lib/trpc/routers/user/index.tsapps/api/src/proxy.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/main/lib/auth/auth-service.tsapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/drizzle.config.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/lib/pglite/database.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/lib/trpc/routers/user/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/lib/pglite/database.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/main/lib/auth/auth-service.tsapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/lib/pglite/database.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/auth/auth-service.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/env.tsapps/api/src/proxy.ts
🧠 Learnings (10)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/drizzle.config.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/drizzle.config.tsapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Create database migrations by modifying Drizzle schema in packages/db/src/schema and running drizzle-kit generate with snake_case migration names
Applied to files:
apps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/drizzle.config.tsapps/desktop/src/renderer/lib/pglite/migrate.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Applied to files:
apps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/electron.vite.config.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/renderer/lib/trpc.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/lib/trpc.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx} : Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/renderer/lib/trpc.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/desktop/electron.vite.config.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/*/app/**/proxy.ts : Never create middleware.ts in Next.js 16 - always use proxy.ts for request interception
Applied to files:
apps/api/src/proxy.ts
🧬 Code graph analysis (18)
packages/trpc/src/router/task/task.ts (1)
packages/db/src/schema/schema.ts (1)
users(28-48)
apps/desktop/src/renderer/lib/pglite/sync.ts (2)
apps/desktop/src/renderer/lib/pglite/database.ts (1)
PGliteWithExtensions(8-11)apps/api/src/env.ts (1)
env(4-43)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (4)
apps/desktop/src/renderer/lib/pglite/schema/schema.ts (3)
SelectTask(143-143)TaskPriority(144-144)tasks(100-140)packages/local-db/src/schema/schema.ts (3)
SelectTask(267-267)TaskPriority(139-139)tasks(218-264)apps/desktop/src/renderer/lib/trpc.ts (1)
trpc(5-5)apps/desktop/src/renderer/lib/pglite/hooks.ts (1)
useTasks(18-28)
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts (2)
apps/desktop/src/renderer/lib/pglite/database.ts (4)
PGliteWithExtensions(8-11)DrizzleDB(13-13)closeOrganizationDatabase(52-61)openOrganizationDatabase(28-46)apps/desktop/src/renderer/lib/pglite/sync.ts (1)
startSync(11-37)
apps/desktop/src/lib/trpc/routers/tasks/index.ts (3)
packages/trpc/src/router/task/schema.ts (1)
updateTaskSchema(19-32)apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)apps/desktop/src/main/lib/api-client.ts (1)
apiClient(11-27)
apps/desktop/src/renderer/lib/pglite/hooks.ts (1)
apps/desktop/src/renderer/lib/pglite/schema/schema.ts (4)
tasks(100-140)SelectTask(143-143)users(36-52)SelectUser(55-55)
apps/desktop/src/renderer/lib/trpc.ts (2)
apps/desktop/src/lib/trpc/index.ts (1)
trpc(49-49)apps/desktop/src/lib/trpc/routers/index.ts (1)
AppRouter(43-43)
.superset/setup.sh (1)
.superset/teardown.sh (2)
error(8-8)success(9-9)
apps/desktop/src/renderer/lib/pglite/migrate.ts (1)
apps/desktop/src/renderer/lib/pglite/migrations/index.ts (2)
Migration(16-20)migrations(22-24)
.superset/teardown.sh (1)
.superset/setup.sh (2)
error(8-8)success(9-9)
apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (3)
apps/desktop/src/renderer/contexts/index.ts (3)
Organization(8-8)useOrganizations(10-10)OrganizationsProvider(9-9)apps/desktop/src/renderer/lib/trpc.ts (2)
RouterOutputs(6-6)trpc(5-5)apps/api/src/trpc/context.ts (1)
createContext(4-12)
apps/desktop/src/renderer/contexts/AppProviders.tsx (3)
apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
OrganizationsProvider(23-44)apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
PGliteProvider(26-75)apps/desktop/src/renderer/contexts/MonacoProvider.tsx (1)
MonacoProvider(73-105)
packages/trpc/src/router/user/user.ts (1)
packages/db/src/schema/schema.ts (2)
users(28-48)organizationMembers(77-98)
apps/desktop/src/renderer/lib/pglite/database.ts (1)
apps/desktop/src/renderer/lib/pglite/migrate.ts (1)
migrate(65-93)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
packages/local-db/src/schema/schema.ts (4)
tasks(218-264)organizationMembers(193-210)organizations(169-185)users(145-161)
apps/desktop/src/lib/trpc/routers/user/index.ts (2)
packages/trpc/src/trpc.ts (1)
publicProcedure(44-44)apps/desktop/src/main/lib/api-client.ts (1)
apiClient(11-27)
apps/api/src/proxy.ts (1)
apps/api/src/env.ts (1)
env(4-43)
apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (3)
apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (2)
Organization(4-4)useOrganizations(14-21)apps/desktop/src/renderer/lib/trpc.ts (1)
trpc(5-5)apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts (1)
useOrganizationDatabase(19-67)
🪛 actionlint (1.7.9)
.github/workflows/deploy-preview.yml
93-93: shellcheck reported issue in this script: SC2086:info:2:56: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 markdownlint-cli2 (0.18.1)
apps/desktop/docs/LOCAL_FIRST_SYNC.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
apps/desktop/docs/PER_ORG_PGLITE.md
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (38)
.gitignore (1)
67-68: Gitignore addition for examples directory is appropriate.Adding
examples/to.gitignoreis a standard practice to prevent example artifacts and demonstration code from being committed. This aligns well with the introduction of new Electric SQL infrastructure and API components in this PR.If you'd like, please verify that the
examples/directory is indeed part of the PR changes or is being created as part of the Electric SQL setup workflow.apps/desktop/docs/PER_ORG_PGLITE.md (1)
1-165: Documentation is clear and well-structured.The design plan effectively outlines the per-org PGlite refactor architecture and provides a logical 7-step migration path. The shift from prop-drilling
organization_idto implicit org context (via active DB selection) aligns well with the stated PR objectives around organization switching and per-org Electric SQL sync.The architecture diagram clearly shows the registry (plain IndexedDB) → per-org DB (PGlite) relationship, and the Bootstrap Flow (Step 7) appropriately addresses the first-load scenario.
Note: This document describes the target design and implementation roadmap. Verify that the actual code changes in this PR correspond to the steps outlined here (particularly Steps 1–6), and ensure that any deviations or phased rollout are documented or tracked separately.
apps/desktop/docs/LOCAL_FIRST_SYNC.md (1)
1-61: Documentation is clear, pragmatic, and well-aligned with PR scope.The Local-First Sync plan is well-structured, provides useful context for the deferred offline-sync feature, and clearly explains why the implementation is being deferred. The architecture, alternatives table, and effort estimates are useful for future decisions. The decision to focus on the read path (Electric SQL syncing) now and defer the write-path queue aligns well with the current PR objectives.
apps/desktop/src/main/lib/auth/auth-service.ts (1)
170-170: LGTM! Clean event emission for token refresh.The event is correctly emitted after successful token refresh and save, enabling downstream subscribers to react to token updates.
.superset/teardown.sh (1)
36-46: LGTM! Container cleanup logic is well-structured.The implementation correctly handles Electric container teardown:
- Uses exact name matching with
^${ELECTRIC_CONTAINER}$to avoid partial matches- Includes stopped containers with
-aflag- Suppresses errors with
|| trueto ensure teardown continues even if the container doesn't exist- Provides clear user feedback with success/warning messages
The error suppression strategy (
&> /dev/null || true) is appropriate for a teardown script where resources may already be cleaned up..github/templates/preview-comment.md (1)
17-20: LGTM!The Electric (Fly.io) preview link entry is correctly structured and positioned in the deployment table.
packages/trpc/src/router/task/task.ts (1)
13-14: LGTM!Moving the alias declarations into function scope improves encapsulation and makes their usage explicit. The aliases are only used within this procedure, so this scoping is appropriate.
apps/desktop/electron.vite.config.ts (1)
197-197: LGTM!Excluding
@electric-sql/pglitefrom optimization is correct for WASM-dependent modules. The explanatory comment is helpful.apps/desktop/src/lib/trpc/routers/user/index.ts (1)
15-21: LGTM!The new
myOrganizationsroute follows the established pattern and correctly proxies to the API client. The implementation aligns with the server-side route added inpackages/trpc/src/router/user/user.ts.apps/desktop/package.json (1)
46-46: LGTM!The additional dependencies align well with the Electric SQL integration:
@superset/dbprovides schema definitionsidbenables IndexedDB utilities for local storagedrizzle-kitprovides migration toolingAll follow proper versioning patterns.
Also applies to: 85-85, 137-137
apps/desktop/src/renderer/index.html (1)
14-18: LGTM!The CSP updates correctly allow connections to the local Electric SQL proxy. Using wildcards for ports (
http://127.0.0.1:*andhttp://localhost:*) is appropriate for dynamic port allocation, and the security implications are minimal since these are localhost-only connections..github/workflows/cleanup-preview.yml (2)
25-34: LGTM!The Electric Fly.io app cleanup is properly integrated:
- Uses
continue-on-errorto handle missing apps gracefully- Follows consistent naming pattern (
superset-electric-pr-{number})- Properly uses
FLY_API_TOKENfrom secrets
42-49: LGTM!The updated cleanup status message appropriately includes Electric app status alongside Neon database cleanup.
packages/trpc/src/router/user/user.ts (1)
41-58: LGTM!The
myOrganizationsprocedure implementation is correct:
- Properly handles missing users by returning an empty array
- Efficiently fetches memberships with eager-loaded organizations using
with- Clean transformation mapping memberships to organizations
- Follows the same pattern as the existing
myOrganization(singular) procedureThe implementation aligns with the desktop router proxy added in
apps/desktop/src/lib/trpc/routers/user/index.ts.apps/desktop/src/renderer/lib/pglite/sync.ts (1)
11-37: LGTM! Sync implementation is well-structured.The sync function correctly:
- Constructs per-organization shape configurations with proper auth headers
- Uses bearer token authentication for secure access
- Passes organizationId as a query parameter for server-side filtering
- Returns the sync subscription for lifecycle management
The hardcoded
primaryKey: ["id"]on Line 28 is acceptable since all current synced tables useidas their primary key.apps/desktop/src/renderer/lib/pglite/migrations/meta/0000_snapshot.json (1)
1-614: Migration snapshot looks consistent.This is a generated Drizzle ORM snapshot file that captures the schema baseline. The structure is consistent with the schema definitions, and the empty
foreignKeyssections (lines 26, 100, 198, 475, 573) are intentional as mentioned in the PR description: "Disable SQLite foreign key constraints for the local sync cache (local DB is not the source of truth)."apps/desktop/src/renderer/lib/pglite/schema/index.ts (1)
1-1: LGTM! Standard barrel export pattern.This follows the project's component organization guidelines by providing a clean public API surface for the schema module.
apps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts (1)
1-1: LGTM! Clean public API export.Exports the provider and hook with clear, explicit named exports following React context provider patterns.
apps/desktop/src/renderer/lib/trpc.ts (1)
2-6: LGTM! Standard TRPC type export pattern.The
RouterOutputstype alias enables type-safe consumption of router outputs throughout the app. This is the recommended TRPC pattern for deriving types from API responses, as seen in the downstream usage whereOrganizationsProviderderivesOrganizationtype fromRouterOutputs["user"]["myOrganizations"][number].apps/desktop/src/renderer/contexts/AppProviders.tsx (1)
18-22: LGTM! Provider hierarchy is correctly ordered.The nested provider structure properly reflects dependencies:
OrganizationsProviderwraps components that need organization context and tRPC access, whilePGliteProviderdepends onuseOrganizations()and must be nested inside. This ensures each provider has access to the contexts it requires..github/workflows/deploy-production.yml (1)
98-99: LGTM! Electric SQL environment variables properly configured.The
ELECTRIC_URLandELECTRIC_SECRETare correctly added to both the build environment and the Vercel deployment command, consistent with the pattern used for other secrets in the workflow.Also applies to: 130-131
apps/desktop/drizzle.config.ts (1)
1-7: LGTM! Drizzle configuration is properly structured.The configuration correctly uses the
postgresqldialect for PGlite compatibility and specifies appropriate paths for the schema and migrations directory.fly.toml (1)
20-25: Health check configuration looks appropriate.The health check polling the
/v1/healthendpoint every 10 seconds with a 20-second grace period provides reasonable monitoring for the Electric SQL service.apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.ts (1)
1-1: LGTM! Clean barrel export.The re-export properly exposes the
useOrganizationDatabasehook as part of the public API.apps/desktop/src/renderer/contexts/index.ts (1)
7-11: LGTM! Consistent barrel export pattern.The new exports for
Organization,OrganizationsProvider, anduseOrganizationsfollow the established pattern and properly use thetypekeyword for the type export.apps/api/src/proxy.ts (1)
6-11: The hardcoded port 5927 matches the desktop app development server configuration defined inapps/desktop/electron.vite.config.tsand used consistently across the codebase. No changes needed.apps/desktop/src/renderer/lib/pglite/migrations/0000_init.sql (1)
1-86: LGTM! Well-structured schema with appropriate indexes.The migration establishes a solid foundation for the local sync cache:
- Indexes align with expected query patterns (organization lookups, task filtering by status/assignee)
- Unique constraints on critical fields prevent duplicates
- Intentional omission of foreign keys is appropriate for a sync cache where the cloud DB is the source of truth
apps/desktop/src/renderer/lib/pglite/index.ts (1)
1-4: LGTM!Clean barrel file consolidating the PGlite public API with proper type vs value export separation.
.superset/setup.sh (1)
84-95: Good health check implementation.The retry loop with timeout and helpful error message pointing to container logs is well-designed for local development experience.
.github/workflows/deploy-preview.yml (2)
78-121: Deploy Electric job is well-structured.The job correctly:
- Depends on
deploy-databasefor the DATABASE_URL- Uses the fly-pr-review-apps action for per-PR deployment
- Handles secrets appropriately via GitHub secrets
- Follows the established artifact pattern for status reporting
194-195: Correctly propagates Electric configuration to API deployment.The ELECTRIC_URL and ELECTRIC_SECRET are properly passed both to the build environment and the Vercel deployment.
Also applies to: 226-227
apps/desktop/src/renderer/lib/pglite/migrations/index.ts (1)
1-24: LGTM! Clean browser-compatible migration bundling.Good approach using Vite's
?rawsuffix to bundle SQL at build time. The documentation clearly explains how to add future migrations.apps/desktop/src/renderer/lib/pglite/hooks.ts (1)
8-28: Good integration of Drizzle with PGlite live queries.The pattern of building type-safe queries with Drizzle, extracting SQL, and passing to
useLiveQueryis sound. The type cast is documented and necessary for library compatibility.apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
1-21: Clean context setup with proper type inference from tRPC.Good use of
RouterOutputsto derive theOrganizationtype, ensuring type safety end-to-end. The hook's error message clearly guides developers to wrap components in the provider.apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
48-50: LGTM on TRPC subscription pattern.The observable subscription pattern is correctly used here for
trpc.auth.onAccessToken.useSubscription, aligning with the coding guidelines for trpc-electron subscriptions.apps/desktop/src/renderer/lib/pglite/database.ts (1)
52-61: LGTM on close logic.The close function correctly awaits the cached promise before closing, ensuring the database is fully initialized before attempting cleanup.
apps/desktop/src/renderer/lib/pglite/schema/schema.ts (2)
78-95: Verify intentional omission of foreign key constraints.The
organizationMemberstable lacks foreign key references tousersandorganizations, unlike the cloud schema inpackages/local-db. Per the PR objectives, this is intentional since the local DB is not the source of truth.Please confirm this aligns with the design decision to disable SQLite foreign key constraints for the local sync cache, ensuring Electric SQL writes don't fail due to FK violations during partial syncs.
100-140: LGTM on tasks table schema.The tasks table is well-structured with appropriate columns, indexes, and enum usage. The intentional omission of FK constraints aligns with the sync cache design.
| ``` | ||
| User action | ||
| ↓ | ||
| PGlite (optimistic write) | ||
| ↓ | ||
| Zustand pending-writes queue (persisted to localStorage) | ||
| ↓ | ||
| Sync worker (processes queue, retries on failure) | ||
| ↓ | ||
| tRPC API | ||
| ↓ | ||
| Database | ||
| ↓ | ||
| Electric syncs canonical state back to PGlite | ||
| ``` |
There was a problem hiding this comment.
Specify a language identifier for the code block.
The fenced code block is missing a language specifier, which triggers a markdown linting error. Since this is an ASCII diagram, use ```plaintext or ```text.
🔎 Proposed fix
-```
+```plaintext
User action
↓
PGlite (optimistic write)
@@ -26,7 +26,7 @@ PGlite (optimistic write)
Electric syncs canonical state back to PGlite
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In apps/desktop/docs/LOCAL_FIRST_SYNC.md around lines 14 to 28, the fenced code
block containing the ASCII sync diagram lacks a language identifier which
triggers markdown linting; update the opening fence to include a plain text
language (e.g., ```plaintext or ```text) so the block is recognized as verbatim
text, leaving the diagram content unchanged and keeping the closing ``` as-is.
| const updateTaskSchema = z.object({ | ||
| id: z.string().uuid(), | ||
| title: z.string().min(1).optional(), | ||
| description: z.string().nullable().optional(), | ||
| status: z.string().optional(), | ||
| priority: z.enum(["urgent", "high", "medium", "low", "none"]).optional(), | ||
| assigneeId: z.string().uuid().nullable().optional(), | ||
| estimate: z.number().nullable().optional(), | ||
| dueDate: z.coerce.date().nullable().optional(), | ||
| }); |
There was a problem hiding this comment.
Schema drift: Desktop schema is incomplete compared to server schema.
The updateTaskSchema is missing several fields present in the canonical schema at packages/trpc/src/router/task/schema.ts:
repositoryId(uuid, optional)branch(string, nullable, optional)prUrl(url, nullable, optional)labels(array of strings, optional)
Additionally:
- Line 10: Priority enum is hardcoded rather than using
taskPriorityValuesfrom the canonical schema - Line 12:
estimateshould be.number().int().positive().nullable().optional()for consistency
This schema drift could prevent users from updating these fields from the desktop app, even though the UI and database support them.
🔎 Proposed fix to align with canonical schema
+import { taskPriorityValues } from "packages/trpc/src/router/task/schema";
+
const updateTaskSchema = z.object({
id: z.string().uuid(),
title: z.string().min(1).optional(),
description: z.string().nullable().optional(),
status: z.string().optional(),
- priority: z.enum(["urgent", "high", "medium", "low", "none"]).optional(),
+ priority: z.enum(taskPriorityValues).optional(),
+ repositoryId: z.string().uuid().optional(),
assigneeId: z.string().uuid().nullable().optional(),
+ branch: z.string().nullable().optional(),
+ prUrl: z.string().url().nullable().optional(),
- estimate: z.number().nullable().optional(),
+ estimate: z.number().int().positive().nullable().optional(),
dueDate: z.coerce.date().nullable().optional(),
+ labels: z.array(z.string()).optional(),
});
Or, even better, import and reuse the canonical schema directly:
-import { z } from "zod";
+import { updateTaskSchema } from "packages/trpc/src/router/task/schema";
import { apiClient } from "main/lib/api-client";
import { publicProcedure, router } from "../..";
-const updateTaskSchema = z.object({
- id: z.string().uuid(),
- title: z.string().min(1).optional(),
- description: z.string().nullable().optional(),
- status: z.string().optional(),
- priority: z.enum(["urgent", "high", "medium", "low", "none"]).optional(),
- assigneeId: z.string().uuid().nullable().optional(),
- estimate: z.number().nullable().optional(),
- dueDate: z.coerce.date().nullable().optional(),
-});
-
export const createTasksRouter = () => {
return router({
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/tasks/index.ts around lines 5-14, the
updateTaskSchema is missing fields and has mismatched types compared to the
canonical schema: add optional repositoryId: uuid, branch:
string().nullable().optional(), prUrl: z.string().url().nullable().optional(),
and labels: z.string().array().optional(); replace the hardcoded priority enum
with the canonical taskPriorityValues (import it and use
z.enum(taskPriorityValues) or reuse the canonical schema directly), and change
estimate to z.number().int().positive().nullable().optional(); update imports as
needed or import the canonical update schema to avoid future drift.
| {task.labels && task.labels.length > 0 && ( | ||
| <div className="flex flex-wrap gap-1 mt-2"> | ||
| {(task.labels as string[]).map((label) => ( | ||
| <Badge key={label} variant="outline" className="text-xs"> | ||
| {label} | ||
| </Badge> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
Type assertion on labels could fail at runtime.
Line 247 uses (task.labels as string[]) assuming labels is always a string array, but the database schema defines it as jsonb which could contain any JSON value. If the data is corrupted or malformed, this could cause runtime errors.
🔎 Proposed defensive check
{task.labels && task.labels.length > 0 && (
<div className="flex flex-wrap gap-1 mt-2">
- {(task.labels as string[]).map((label) => (
+ {Array.isArray(task.labels) && task.labels.map((label) => (
<Badge key={label} variant="outline" className="text-xs">
- {label}
+ {String(label)}
</Badge>
))}
</div>
)}
This adds a runtime check for array type and safely coerces label values to strings.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
around lines 245-252, the code uses a type assertion (task.labels as string[])
which can crash if labels is not an array; replace the assertion with a runtime
guard that checks Array.isArray(task.labels) before rendering, and when mapping
entries coerce each item to a safe string (e.g., String(item) or skip
non-primitive entries) so malformed JSON won’t throw — only render the Badge
list when the guard passes.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (2)
49-72: Form state doesn't reset when editing a different task.This issue from the previous review is still present.
useStateinitializes from thetaskprop only on mount. Opening the dialog for Task A, closing it, then opening for Task B will show Task A's values.🔎 Recommended fix: Add key prop in parent component
In the parent component at lines 285-291, add
key={editingTask.id}to force remount:{editingTask && ( <TaskEditDialog + key={editingTask.id} task={editingTask} open={!!editingTask} onOpenChange={(open) => !open && setEditingTask(null)} /> )}This approach is preferred per coding guidelines ("Do not use effect unless absolutely necessary") and ensures clean state initialization for each task.
247-254: Type assertion on labels could fail at runtime.This issue from the previous review is still present. Line 249 uses
(task.labels as string[])but the database schema defineslabelsasjsonb, which could contain any JSON value. Malformed data could cause runtime errors.🔎 Proposed defensive check
{task.labels && task.labels.length > 0 && ( <div className="flex flex-wrap gap-1 mt-2"> - {(task.labels as string[]).map((label) => ( - <Badge key={label} variant="outline" className="text-xs"> - {label} + {Array.isArray(task.labels) && task.labels.map((label) => ( + <Badge key={String(label)} variant="outline" className="text-xs"> + {String(label)} </Badge> ))} </div> )}This adds a runtime array check and safely coerces label values to strings.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
288-288: Redundant boolean coercion.
!!editingTaskis unnecessary sinceeditingTaskis alreadyTask | nulland theopenprop expects a boolean. TypeScript will coerce the truthy/falsy value automatically, or you can useeditingTask !== nullfor clarity.🔎 Cleaner alternatives
- open={!!editingTask} + open={editingTask !== null}Or simply:
- open={!!editingTask} + open={Boolean(editingTask)}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/lib/posthog.tsapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/lib/posthog.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/lib/posthog.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/lib/posthog.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/lib/posthog.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/lib/posthog.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx
🧬 Code graph analysis (5)
apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(6-6)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (6)
apps/desktop/src/renderer/lib/pglite/schema/schema.ts (3)
SelectTask(143-143)TaskPriority(144-144)tasks(100-140)apps/desktop/src/renderer/lib/trpc.ts (1)
trpc(5-5)apps/desktop/src/renderer/lib/pglite/hooks.ts (1)
useTasks(18-28)apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (1)
OrganizationSwitcher(12-64)apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
OrganizationsProvider(23-44)apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
PGliteProvider(26-75)
apps/desktop/src/renderer/screens/main/index.tsx (2)
packages/shared/src/constants.ts (1)
FEATURE_FLAGS(45-48)apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
TasksView(331-339)
apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx (4)
apps/desktop/src/renderer/screens/main/components/AvatarDropdown/index.ts (1)
AvatarDropdown(1-1)apps/desktop/src/renderer/stores/app-state.ts (2)
useOpenSettings(82-82)useOpenTasks(87-87)packages/shared/src/constants.ts (1)
FEATURE_FLAGS(45-48)packages/ui/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuItem(255-255)
apps/desktop/src/renderer/lib/posthog.ts (2)
apps/desktop/src/renderer/env.renderer.ts (1)
env(45-47)apps/docs/src/env.ts (1)
env(5-36)
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/index.tsx (1)
198-200: Verify fallback behavior when tasks view is requested without access.When
currentView === "tasks"buthasTasksAccessis false, the user is silently redirected toWorkspaceView. Consider whether an explicit "access denied" message or redirect to a valid view would provide better UX.If this silent fallback is intentional (e.g., the tasks option is never shown in the UI when the flag is disabled), then this is fine. Otherwise, consider adding explicit handling:
if (currentView === "tasks") { if (hasTasksAccess) { return <TasksView />; } // User shouldn't reach here if UI is gated properly return <WorkspaceView />; }apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx (1)
110-115: LGTM! Feature flag integration is clean and consistent.The Tasks menu item is properly gated by the feature flag, follows existing patterns in the component, and is positioned logically in the dropdown menu.
apps/desktop/src/renderer/lib/posthog.ts (1)
14-29: Initialization logic is correct.The use of
posthogFull.initwith proper configuration is appropriate. The loaded callback correctly registers app metadata.apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
1-1: The PostHog integration works correctly with the type cast. TheposthogFullinstance from the full module includes all required functionality, and the cast to thePostHogtype fromposthog-js/reactis a legitimate compatibility pattern. PostHog React hooks likeuseFeatureFlagEnabledare actively used in multiple renderer components and function properly with this setup. The methodsposthog.capture(),posthog.identify(), andposthog.reset()all work as expected. No runtime incompatibilities have been identified.Likely an incorrect or invalid review comment.
| Cancel | ||
| </Button> | ||
| <Button onClick={handleSave} disabled={isSaving || !title.trim()}> | ||
| {isSaving ? "Saving..." : "Save & Sync to Linear"} |
There was a problem hiding this comment.
Hardcoded "Linear" reference may not match actual provider.
The button text "Save & Sync to Linear" assumes the external provider is always Linear, but task.external_provider can vary. Consider making this dynamic.
🔎 Suggested improvement
- {isSaving ? "Saving..." : "Save & Sync to Linear"}
+ {isSaving ? "Saving..." : task.external_provider ? `Save & Sync to ${task.external_provider}` : "Save"}
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
around line 129, the button text is hardcoded to "Save & Sync to Linear" which
assumes the external provider; change it to use the task.external_provider value
(with a safe fallback and proper capitalization) so the label reads e.g. `Save &
Sync to {providerName}` or a generic `Save & Sync to external provider` when
missing; ensure null/undefined is handled and consider using existing
i18n/localization helpers if present.
… switching - Add Electric SQL proxy in API with organization-based filtering - Implement sync service for real-time data sync to local SQLite - Add tasks view with organization switcher and task editing - Add organizations and tasks tRPC routers with subscriptions - Disable SQLite FK constraints (sync cache, not source of truth) - Add local-db schema for synced tables (users, orgs, tasks) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Production: pass ELECTRIC_URL and ELECTRIC_SECRET to Vercel API - Preview: per-PR Electric apps on Fly.io using fly-pr-review-apps action - Local: Docker container with auto-assigned port in setup/teardown scripts - Cleanup: delete Electric Fly apps when PRs are closed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add electric-tasks-access feature flag (only satya@superset.sh) - Hide Tasks menu item unless flag is enabled - Gate TasksView rendering behind flag check - Remove unused require-desktop-auth and test-pages-access flags 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use useFeatureFlagEnabled hook instead of synchronous isFeatureEnabled - Add PostHogProvider (PHProvider) wrapper from posthog-js/react - Move PGliteProvider and OrganizationsProvider to wrap only TasksView - Type cast posthog for compatibility between Electron bundle and React 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
6778669 to
74c4d55
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (10)
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
102-116: Critical: Hardcoded ngrok URL will fail outside local development.This hardcoded URL remains unresolved from the previous review and will cause sync failures in staging/production environments. The validated
env.NEXT_PUBLIC_API_URLis already available in your environment schema and should be used here.🔎 Proposed fix
- // TODO: In production, use env.NEXT_PUBLIC_API_URL - const qstashBaseUrl = "https://e7e7cc5a4723.ngrok-free.app"; + const qstashBaseUrl = env.NEXT_PUBLIC_API_URL; try { await qstash.publishJSON({ url: `${qstashBaseUrl}/api/integrations/linear/jobs/initial-sync`, body: { organizationId, creatorUserId: userId }, retries: 3, });apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
42-66: Handle errors in the asyncemitTokenfunction to prevent unhandled rejections.The
emitTokenfunction is async but is called withoutawait(line 54) and used directly as an event handler (lines 57, 59). IfauthService.getAccessToken()throws an error, it will result in an unhandled promise rejection.AGENTS.md (1)
149-150: Fix markdown emphasis formatting.The static analysis tool flagged spaces inside emphasis markers. Per MD037, remove the spaces around the text within
**.🔎 Proposed fix
-** IMPORTANT ** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first. +**IMPORTANT** - Never touch the production database unless explicitly asked to. Even then, confirm with the user first.apps/api/src/lib/auth.ts (1)
16-23: Handle potentially undefinedpayload.sub.The JWT
subclaim is optional per spec. The current type assertion could returnundefinedas a string ifsubis not present, leading to unexpected behavior downstream.🔎 Proposed fix
if (payload.type !== "access") { return null; } - return payload.sub as string; + if (typeof payload.sub !== "string") { + return null; + } + + return payload.sub; } catch { return null; }apps/api/src/app/api/electric/[...path]/utils.ts (1)
54-69: Handle emptyuserIdsarray to prevent invalid SQL.If no organization members are found,
userIdswill be empty, causinginArray()to generate invalid SQL syntax (IN ()) which PostgreSQL will reject.🔎 Proposed fix
// Users don't have organizationId - need to look up via membership case "users": { const members = await db.query.organizationMembers.findMany({ where: eq(organizationMembers.organizationId, organizationId), columns: { userId: true }, }); const userIds = [...new Set(members.map((m) => m.userId))]; + if (userIds.length === 0) { + return { fragment: "1 = 0", params: [] }; + } const whereExpr = inArray(sql`${sql.identifier(users.id.name)}`, userIds);apps/desktop/docs/LOCAL_FIRST_SYNC.md (1)
14-28: Specify a language identifier for the ASCII diagram code block.The fenced code block lacks a language specifier. Use
```plaintextfor ASCII diagrams to satisfy markdown linting.apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (3)
285-291: Form state doesn't reset when editing a different task.The
TaskEditDialogcomponent usesuseStatewhich only initializes from props on first render. When switching between tasks, stale values will persist. Addkey={editingTask.id}to force remount.🔎 Proposed fix
{editingTask && ( <TaskEditDialog + key={editingTask.id} task={editingTask} open={!!editingTask} onOpenChange={(open) => !open && setEditingTask(null)} /> )}
247-254: Type assertion onlabelscould fail at runtime.Line 249 uses
(task.labels as string[])assuming labels is always a string array, but the schema stores it as JSON which could be malformed. Add a runtime guard.🔎 Proposed defensive check
- {task.labels && task.labels.length > 0 && ( + {Array.isArray(task.labels) && task.labels.length > 0 && ( <div className="flex flex-wrap gap-1 mt-2"> - {(task.labels as string[]).map((label) => ( + {task.labels.map((label) => ( <Badge key={label} variant="outline" className="text-xs"> - {label} + {typeof label === "string" ? label : String(label)} </Badge> ))} </div> )}
128-130: Hardcoded "Linear" reference may not match actual provider.The button text assumes the external provider is always Linear, but
task.external_providercan vary. Consider making this dynamic.🔎 Suggested improvement
- {isSaving ? "Saving..." : "Save & Sync to Linear"} + {isSaving ? "Saving..." : task.external_provider ? `Save & Sync to ${task.external_provider}` : "Save"}packages/local-db/drizzle/0001_add_synced_tables.sql (1)
1-83: Table creation order issue already flagged.The
organization_memberstable referencesorganizationsandusersvia foreign keys, but those tables are created later. As noted in the past review, this works because foreign keys are disabled for the local sync cache, but the order should ideally be:users→organizations→organization_members→tasks.
🧹 Nitpick comments (8)
apps/api/src/app/api/electric/[...path]/utils.ts (2)
23-33: Consider simplifying column reference inbuild()function.The
sql.identifier(column.name)wrapping is unnecessary since Drizzle'seq()accepts columns directly. Additionally, the regex-based fragment extraction is fragile if Drizzle's query format changes.🔎 Proposed simplification
function build(table: PgTable, column: PgColumn, id: string): WhereClause { - const whereExpr = eq(sql`${sql.identifier(column.name)}`, id); + const whereExpr = eq(column, id); const qb = new QueryBuilder(); const { sql: query, params } = qb .select() .from(table) .where(whereExpr) .toSQL(); const fragment = query.replace(/^select .* from .* where\s+/i, ""); return { fragment, params }; }
60-60: SimplifyinArraycolumn reference.Similar to the
build()function, pass the column directly toinArray()rather than wrapping withsql.identifier().🔎 Proposed fix
- const whereExpr = inArray(sql`${sql.identifier(users.id.name)}`, userIds); + const whereExpr = inArray(users.id, userIds);apps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.md (1)
258-259: TODO: Investigateelectric_sync_statenecessity.This TODO is worth tracking. Electric's ShapeStream does handle resume/offset internally in many cases.
Would you like me to open an issue to investigate whether
electric_sync_stateis needed, or help research Electric's built-in sync state management?apps/desktop/docs/PER_ORG_PGLITE.md (1)
19-44: Specify a language identifier for the ASCII diagram code block.The fenced code block is missing a language specifier, which triggers a markdown linting error. Use
```plaintextor```textfor ASCII diagrams.🔎 Proposed fix
-``` +```plaintext ┌─────────────────────────────────────────────────────────────┐apps/desktop/src/renderer/lib/pglite/sync.ts (1)
11-37: Consider adding an explicit return type annotation.The function returns
Promise<any>implicitly fromsyncShapesToTables. Adding an explicit return type (even if it's the library's type) would improve type safety and documentation.🔎 Suggested improvement
+import type { SyncShapesToTablesResult } from "@electric-sql/pglite-sync"; + export async function startSync( pg: PGliteWithExtensions, accessToken: string, organizationId: string, -) { +): Promise<SyncShapesToTablesResult> {If the exact type isn't exported, consider using a more descriptive type or adding a JSDoc comment to document the return value structure.
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
49-140: Consider splitting internal components into separate files.This file contains 6 components (
TaskEditDialog,TaskCard,TasksList,Sidebar,TasksViewContent,TasksView). Per coding guidelines, prefer one component per file and nest child components in acomponents/folder.Suggested structure:
TasksView/ ├── TasksView.tsx # Main export ├── index.ts # Barrel export └── components/ ├── TaskEditDialog/ │ ├── TaskEditDialog.tsx │ └── index.ts ├── TaskCard/ │ ├── TaskCard.tsx │ └── index.ts ├── TasksList/ │ ├── TasksList.tsx │ └── index.ts └── Sidebar/ ├── Sidebar.tsx └── index.tsAs per coding guidelines: "Use one component per file - do not combine multiple components in a single file" and "Nest components in parent's components/ folder if used only once".
Also applies to: 142-258, 261-294, 296-313, 315-329
packages/trpc/src/router/user/user.ts (1)
41-58: Implementation looks correct.The new
myOrganizationsprocedure properly fetches all organizations for the authenticated user with eager loading viawith. The empty array return for missing users is a reasonable choice for list endpoints.Consider extracting the user lookup by
clerkIdinto a shared helper to reduce duplication acrossme,myOrganization, andmyOrganizations:// In a shared utility async function getUserByClerkId(clerkId: string) { return db.query.users.findFirst({ where: eq(users.clerkId, clerkId), }); }apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
35-37: Consider whether "No organizations found" is truly an error state.Using
text-destructivestyling suggests an error, but having no organizations might be a valid state for new users. Consider a friendlier empty state or an onboarding prompt.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (81)
.github/templates/preview-comment.md.github/workflows/cleanup-preview.yml.github/workflows/deploy-preview.yml.github/workflows/deploy-production.yml.gitignore.superset/setup.sh.superset/teardown.shAGENTS.mdapps/admin/package.jsonapps/api/package.jsonapps/api/src/app/api/electric/[...path]/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/env.tsapps/api/src/lib/auth.tsapps/api/src/proxy.tsapps/api/src/trpc/context.tsapps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.mdapps/desktop/docs/LOCAL_FIRST_SYNC.mdapps/desktop/docs/PER_ORG_PGLITE.mdapps/desktop/drizzle.config.tsapps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/lib/trpc/routers/user/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/lib/trpc/routers/workspaces/workspaces.tsapps/desktop/src/main/lib/app-environment.tsapps/desktop/src/main/lib/local-db/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/desktop/src/renderer/contexts/TRPCProvider.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/index.htmlapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/lib/pglite/migrations/0000_init.sqlapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/src/renderer/lib/pglite/migrations/meta/0000_snapshot.jsonapps/desktop/src/renderer/lib/pglite/migrations/meta/_journal.jsonapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/lib/posthog.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/shared/constants.tsapps/docs/package.jsonapps/marketing/package.jsonapps/web/package.jsonfly.tomlpackages/local-db/drizzle/0001_add_synced_tables.sqlpackages/local-db/drizzle/0002_add_active_organization_id.sqlpackages/local-db/drizzle/meta/0001_snapshot.jsonpackages/local-db/drizzle/meta/0002_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.tspackages/shared/src/constants.tspackages/trpc/src/lib/integrations/sync/tasks.tspackages/trpc/src/router/task/task.tspackages/trpc/src/router/user/user.ts
💤 Files with no reviewable changes (2)
- apps/desktop/src/main/lib/app-environment.ts
- apps/desktop/src/lib/trpc/routers/settings/index.ts
🚧 Files skipped from review as they are similar to previous changes (42)
- apps/desktop/drizzle.config.ts
- packages/shared/src/constants.ts
- apps/desktop/src/renderer/lib/pglite/schema/index.ts
- apps/desktop/electron.vite.config.ts
- apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
- packages/trpc/src/lib/integrations/sync/tasks.ts
- apps/desktop/src/lib/trpc/routers/tasks/index.ts
- apps/desktop/src/renderer/lib/posthog.ts
- apps/desktop/src/renderer/contexts/PostHogProvider.tsx
- .github/templates/preview-comment.md
- apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts
- apps/api/src/env.ts
- apps/api/package.json
- apps/desktop/src/lib/trpc/routers/index.ts
- apps/desktop/src/renderer/contexts/TRPCProvider.tsx
- apps/desktop/src/renderer/screens/main/index.tsx
- apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
- apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
- apps/desktop/src/renderer/lib/pglite/migrate.ts
- packages/local-db/drizzle/0002_add_active_organization_id.sql
- apps/desktop/src/lib/trpc/routers/user/index.ts
- apps/desktop/src/renderer/lib/pglite/migrations/meta/_journal.json
- apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.ts
- apps/desktop/src/renderer/lib/pglite/index.ts
- apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/index.ts
- apps/desktop/src/renderer/lib/pglite/migrations/0000_init.sql
- apps/desktop/src/main/lib/local-db/index.ts
- .github/workflows/deploy-production.yml
- apps/desktop/src/renderer/lib/pglite/database.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx
- .superset/teardown.sh
- apps/api/src/proxy.ts
- apps/desktop/src/renderer/lib/pglite/hooks.ts
- apps/api/src/app/api/electric/[...path]/route.ts
- packages/local-db/drizzle/meta/0002_snapshot.json
- .gitignore
- fly.toml
- apps/desktop/src/renderer/lib/pglite/migrations/index.ts
- apps/desktop/src/renderer/lib/pglite/migrations/meta/0000_snapshot.json
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxpackages/local-db/src/schema/schema.tspackages/trpc/src/router/user/user.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/trpc/context.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/api/src/lib/auth.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxpackages/local-db/src/schema/schema.tspackages/trpc/src/router/user/user.tsapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/trpc/context.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/api/src/lib/auth.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.tsapps/desktop/src/renderer/stores/app-state.tsapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/shared/constants.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/schema/schema.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/trpc/context.tsapps/api/src/lib/auth.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place tests co-located with components using .test.ts or .test.tsx naming convention
Files:
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.ts
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent responsibilities and interactions in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (17)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Applied to files:
apps/desktop/src/renderer/screens/main/components/TasksView/index.tsapps/desktop/src/renderer/contexts/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/lib/trpc.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/lib/trpc.tsapps/desktop/src/shared/constants.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
packages/local-db/src/schema/schema.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsAGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/marketing/package.jsonapps/web/package.jsonapps/admin/package.json
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx} : Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/renderer/lib/trpc.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/desktop/src/shared/constants.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
apps/api/src/app/api/electric/[...path]/utils.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsAGENTS.md
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2025-11-24T21:32:17.800Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:17.800Z
Learning: Applies to AGENTS.md : Document agent responsibilities and interactions in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Keep diffs minimal with targeted edits only - do not make broad refactoring changes
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Follow existing code patterns and match codebase style
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/{components,features}/**/[!.]*.tsx : Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Do not run dev servers in automation
Applied to files:
AGENTS.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Create database migrations by modifying Drizzle schema in packages/db/src/schema and running drizzle-kit generate with snake_case migration names
Applied to files:
AGENTS.md
🧬 Code graph analysis (15)
.superset/setup.sh (1)
.superset/teardown.sh (2)
error(8-8)success(9-9)
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(6-6)apps/desktop/src/main/lib/analytics/index.ts (1)
posthog(4-4)
apps/desktop/src/lib/trpc/routers/auth/index.ts (2)
packages/trpc/src/trpc.ts (1)
publicProcedure(44-44)apps/desktop/src/main/lib/auth/auth-service.ts (1)
authService(360-360)
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
apps/api/src/env.ts (1)
env(4-43)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (4)
apps/desktop/src/renderer/lib/pglite/schema/schema.ts (3)
SelectTask(143-143)TaskPriority(144-144)tasks(100-140)apps/desktop/src/renderer/lib/pglite/hooks.ts (1)
useTasks(18-28)apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
OrganizationsProvider(23-44)apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
PGliteProvider(26-75)
packages/trpc/src/router/user/user.ts (1)
packages/db/src/schema/schema.ts (2)
users(28-48)organizationMembers(77-98)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
runTeardown(38-78)
apps/desktop/src/renderer/stores/app-state.ts (1)
apps/desktop/src/main/lib/app-state/schemas.ts (1)
AppState(15-18)
apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (3)
apps/desktop/src/renderer/contexts/index.ts (1)
Organization(8-8)apps/desktop/src/renderer/lib/trpc.ts (2)
RouterOutputs(6-6)trpc(5-5)apps/api/src/trpc/context.ts (1)
createContext(4-12)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts (1)
getShellEnvironment(24-78)
apps/desktop/src/renderer/lib/trpc.ts (2)
apps/desktop/src/lib/trpc/index.ts (1)
trpc(49-49)apps/desktop/src/lib/trpc/routers/index.ts (1)
AppRouter(45-45)
apps/desktop/src/renderer/lib/pglite/sync.ts (2)
apps/desktop/src/renderer/lib/pglite/database.ts (1)
PGliteWithExtensions(8-11)apps/api/src/env.ts (1)
env(4-43)
apps/api/src/trpc/context.ts (3)
apps/api/src/lib/auth.ts (1)
authenticateRequest(35-57)packages/trpc/src/trpc.ts (1)
createTRPCContext(20-24)packages/trpc/src/index.ts (1)
createTRPCContext(9-9)
packages/trpc/src/router/task/task.ts (1)
packages/db/src/schema/schema.ts (1)
users(28-48)
apps/api/src/lib/auth.ts (1)
apps/api/src/env.ts (1)
env(4-43)
🪛 actionlint (1.7.9)
.github/workflows/deploy-preview.yml
93-93: shellcheck reported issue in this script: SC2086:info:2:56: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
149-149: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
149-149: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
apps/desktop/docs/LOCAL_FIRST_SYNC.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
apps/desktop/docs/PER_ORG_PGLITE.md
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (38)
packages/trpc/src/router/task/task.ts (1)
13-15: LGTM! Improved scoping.Moving the alias declarations from module scope into the
all()resolver is a good refactoring. It improves code locality by keeping the aliases close to where they're used and reduces module-level state. Since these aliases are only needed for this specific query, function-local scope is more appropriate.apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
14-15: Correct pattern for PostHog feature flags after user identification.Calling
reloadFeatureFlags()immediately afteridentify()ensures the SDK fetches updated feature flags for the newly identified user. This is the recommended approach in posthog-js and the method is fire-and-forget, making it appropriate for this context.apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.test.ts (1)
30-154: LGTM! All tests correctly updated to async/await pattern.All 10 test functions have been properly converted to use
async/awaitto accommodate the now-asynchronousrunTeardownfunction. The conversions are complete and consistent across all test cases.apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (3)
5-5: LGTM! Async conversion properly implemented.The function signature correctly reflects the asynchronous nature with
Promise<TeardownResult>return type, and the import ofgetShellEnvironmentis properly placed.Also applies to: 38-42
53-62: LGTM! Shell environment integration improves PATH resolution.Replacing
process.envwithgetShellEnvironment()ensures that teardown scripts have access to the proper shell environment (including PATH from shell profiles). ThegetShellEnvironment()function includes caching and fallback toprocess.envon failure, making this a safe and efficient change.
63-63: LGTM! Usingstdio: "pipe"is appropriate for programmatic execution.This prevents the child process output from being mixed with the parent process streams, which is the correct behavior for programmatic teardown execution. Error messages are still captured and logged appropriately.
.github/workflows/cleanup-preview.yml (1)
25-49: LGTM! Electric cleanup is properly implemented.The Electric Fly.io app cleanup follows the same pattern as Neon cleanup:
- Uses
continue-on-error: trueto ensure workflow completes even if app doesn't exist- Reports status in the cleanup comment
- Properly references the PR-specific app name
.github/workflows/deploy-preview.yml (4)
20-20: LGTM! Electric URL correctly configured at workflow level.The workflow-level environment variable makes the Electric URL available to all jobs that need it.
194-227: LGTM! Electric environment variables correctly propagated to API.The Electric URL and secret are properly passed through both the build environment and the Vercel deployment command, following the same pattern as other environment variables.
570-630: LGTM! Electric deployment status correctly integrated into comment generation.The deployment comment logic properly handles Electric status:
- Adds
deploy-electricto job dependencies- Initializes failure defaults
- Conditionally sources success status
- Exports variables for template substitution
This follows the same pattern as other deployment steps.
93-95: Shellcheck warning is a false positive.The static analysis tool flagged SC2086 about quoting on line 95, but the variable
$DATABASE_URL_UNPOOLEDis already inside double quotes ("DATABASE_URL_UNPOOLED=$DATABASE_URL_UNPOOLED"), which prevents word splitting and globbing.While using
${DATABASE_URL_UNPOOLED}syntax would be slightly more explicit, the current code is correct and the warning can be safely ignored.apps/web/package.json (1)
35-35: LGTM: PostHog version pinned to latest.Version 1.310.1 exists on npm and was published recently. The exact version pinning (removing caret) ensures consistent builds across environments.
apps/admin/package.json (1)
32-32: LGTM: PostHog version pinned to latest.Version 1.310.1 exists on npm and was published recently. Consistent with the version updates across other packages in this PR.
apps/desktop/src/renderer/lib/trpc.ts (1)
2-6: LGTM: Type-safe router outputs export.The
RouterOutputstype export follows standard TRPC patterns and enables type-safe consumption of router procedure outputs across the application. This is an additive, non-breaking change.apps/marketing/package.json (1)
27-27: LGTM: PostHog version pinned to latest.Version 1.310.1 exists on npm and was published recently. Consistent with the version updates across other packages in this PR.
apps/docs/package.json (1)
27-27: LGTM: PostHog version pinned to latest.Version 1.310.1 exists on npm and was published recently. Consistent with the version updates across other packages in this PR.
apps/desktop/src/shared/constants.ts (1)
14-15: LGTM: Electric SQL proxy port constant added.The addition of the
ELECTRICport constant follows the established pattern for environment-aware port configuration and supports the Electric SQL integration introduced in this PR.apps/desktop/package.json (1)
46-46: LGTM: New dependencies support Electric SQL integration.The addition of @superset/db, idb, drizzle-kit, and the posthog-js version update align with the PR's Electric SQL integration objectives. All versions are valid and available on npm.
Also applies to: 85-85, 95-95, 137-137
apps/api/src/lib/auth.ts (1)
35-57: LGTM!The
authenticateRequestfunction is well-structured with a clean fallback pattern: Clerk session authentication first, then desktop JWT verification. Proper null handling throughout.apps/api/src/trpc/context.ts (1)
1-12: LGTM!Clean refactor that centralizes authentication by delegating to
authenticateRequest. This simplifies the context creation and follows DRY principles.apps/desktop/src/renderer/index.html (1)
14-18: LGTM!The CSP updates appropriately allow connections to the local Electric SQL proxy endpoints. Using wildcard ports for localhost connections is reasonable for development flexibility in an Electron app context.
apps/desktop/docs/ELECTRIC_SQL_IMPLEMENTATION.md (1)
1-61: Excellent documentation!Comprehensive architecture documentation covering the Electric SQL sync flow, data model, relationships, and security considerations. The ASCII diagrams and code examples clearly illustrate the system design.
packages/local-db/src/schema/schema.ts (2)
133-141: LGTM!Good documentation explaining the snake_case naming convention for Electric SQL compatibility. The type definitions (
TaskPriority,IntegrationProvider) are well-defined.
145-267: LGTM!Well-designed synced tables with:
- Appropriate indexes on frequently queried columns (email, clerk_id, slug, organization_id, assignee_id, status)
- Correct foreign key relationships with sensible cascade behaviors (
onDelete: "cascade"for ownership,onDelete: "set null"for assignee)- Comprehensive task schema supporting Linear integration fields
apps/desktop/src/renderer/stores/app-state.ts (2)
4-26: LGTM!Clean extension of the zustand store following the existing patterns for settings. The tasks tab state management (
isTasksTabOpen,openTasks,closeTasks) mirrors the settings tab pattern, maintaining consistency. Per coding guidelines, zustand is the preferred state management solution.
60-66: LGTM!The
openTasksandcloseTasksimplementations correctly manage bothcurrentViewandisTasksTabOpenstate, consistent with the settings tab pattern.apps/desktop/src/renderer/screens/main/components/TasksView/index.ts (1)
1-1: LGTM!Clean barrel export following the project's component organization guidelines.
packages/local-db/drizzle/meta/_journal.json (1)
11-25: LGTM!Migration journal entries are properly structured with sequential indices and consistent versioning.
apps/desktop/src/renderer/contexts/index.ts (1)
7-11: LGTM!Exports properly follow existing patterns and correctly use
typeprefix for type-only exports.packages/local-db/drizzle/meta/0001_snapshot.json (1)
1-956: LGTM!Auto-generated Drizzle schema snapshot with appropriate table structures, foreign key relationships (cascade/set-null), and indexes for the synced tables.
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
1-39: LGTM on imports and type definitions.Imports are well-organized, no Node.js modules are used (complying with renderer guidelines), and the
Tasktype alias provides a clean abstraction overSelectTask..superset/setup.sh (2)
24-24: Good addition of Docker dependency check.Adding this check early ensures a clear error message if Docker is missing, which is essential for the Electric SQL container workflow.
86-95: Health check loop is well structured.The 30-second timeout with clear error messaging is appropriate. One minor observation: the loop index
$icomparison works correctly in bash.apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts (1)
44-49: Database not closed on unmount.The cleanup function sets
statetonulland cleans up sync, but doesn't close the current organization's database. If unmounting means the database should be released, consider callingcloseOrganizationDatabase(organizationId)in cleanup.Is the database intentionally kept open (cached) after unmount for potential re-use? If so, this is fine. If not, the database handle may leak.
packages/local-db/drizzle/0001_add_synced_tables.sql (1)
63-68: Indexes are well designed for expected query patterns.The combination of unique constraint on
slugplus a separate index may be redundant (unique constraints typically create an implicit index), but this is a minor optimization concern.apps/desktop/src/renderer/lib/pglite/schema/schema.ts (3)
78-95: Foreign key references omitted from Drizzle schema.The
organizationMemberstable definesorganization_idanduser_idas plain UUIDs without.references()toorganizationsorusers. This is intentional per the PR objectives (FK constraints disabled for local sync cache), but worth noting that Drizzle's type inference won't enforce these relationships at the schema level.
100-140: Tasks table schema looks comprehensive.Good coverage of task properties including external integration fields, sync metadata, and status workflow columns. The use of
realforstatus_positionallows for fractional ordering.
152-155: Singleton pattern forlocalSettingsis appropriate.Using
id: integer().primaryKey().default(1)enforces a single settings row. This is a valid pattern for app-level configuration that doesn't need multiple instances.
| deploy-electric: | ||
| name: Deploy Electric (Fly.io) | ||
| runs-on: ubuntu-latest | ||
| needs: deploy-database | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Download database info | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: database-status | ||
|
|
||
| - name: Load database URL | ||
| run: | | ||
| source database-status.env | ||
| echo "DATABASE_URL_UNPOOLED=$DATABASE_URL_UNPOOLED" >> $GITHUB_ENV | ||
|
|
||
| - name: Deploy Electric to Fly.io | ||
| uses: superfly/fly-pr-review-apps@1.3.0 | ||
| with: | ||
| name: superset-electric-pr-${{ github.event.number }} | ||
| region: iad | ||
| org: superset-670 | ||
| config: fly.toml | ||
| secrets: | | ||
| DATABASE_URL=${{ env.DATABASE_URL_UNPOOLED }} | ||
| ELECTRIC_SECRET=${{ secrets.ELECTRIC_SECRET_PREVIEW }} | ||
| env: | ||
| FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} | ||
|
|
||
| - name: Save Electric status | ||
| run: | | ||
| cat > electric-status.env << EOF | ||
| ELECTRIC_STATUS="✅" | ||
| ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ github.event.number }}\">View App</a>" | ||
| EOF | ||
|
|
||
| - name: Upload Electric status | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: electric-status | ||
| path: electric-status.env |
There was a problem hiding this comment.
Use consistent PR number reference throughout the workflow.
Lines 100 and 114 use github.event.number while the cleanup workflow (.github/workflows/cleanup-preview.yml line 34) and the ELECTRIC_URL (line 20) use github.event.pull_request.number. Although both work in pull_request events, this inconsistency could cause cleanup failures in edge cases.
For consistency, use the workflow-level PR_NUMBER environment variable defined at line 14.
🔎 Recommended fix
- name: Deploy Electric to Fly.io
uses: superfly/fly-pr-review-apps@1.3.0
with:
- name: superset-electric-pr-${{ github.event.number }}
+ name: superset-electric-pr-${{ env.PR_NUMBER }}
region: iad
org: superset-670
config: fly.toml
secrets: |
DATABASE_URL=${{ env.DATABASE_URL_UNPOOLED }}
ELECTRIC_SECRET=${{ secrets.ELECTRIC_SECRET_PREVIEW }}
env:
FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }}
- name: Save Electric status
run: |
cat > electric-status.env << EOF
ELECTRIC_STATUS="✅"
- ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ github.event.number }}\">View App</a>"
+ ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ env.PR_NUMBER }}\">View App</a>"
EOF
📝 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.
| deploy-electric: | |
| name: Deploy Electric (Fly.io) | |
| runs-on: ubuntu-latest | |
| needs: deploy-database | |
| steps: | |
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| - name: Download database info | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: database-status | |
| - name: Load database URL | |
| run: | | |
| source database-status.env | |
| echo "DATABASE_URL_UNPOOLED=$DATABASE_URL_UNPOOLED" >> $GITHUB_ENV | |
| - name: Deploy Electric to Fly.io | |
| uses: superfly/fly-pr-review-apps@1.3.0 | |
| with: | |
| name: superset-electric-pr-${{ github.event.number }} | |
| region: iad | |
| org: superset-670 | |
| config: fly.toml | |
| secrets: | | |
| DATABASE_URL=${{ env.DATABASE_URL_UNPOOLED }} | |
| ELECTRIC_SECRET=${{ secrets.ELECTRIC_SECRET_PREVIEW }} | |
| env: | |
| FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} | |
| - name: Save Electric status | |
| run: | | |
| cat > electric-status.env << EOF | |
| ELECTRIC_STATUS="✅" | |
| ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ github.event.number }}\">View App</a>" | |
| EOF | |
| - name: Upload Electric status | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: electric-status | |
| path: electric-status.env | |
| deploy-electric: | |
| name: Deploy Electric (Fly.io) | |
| runs-on: ubuntu-latest | |
| needs: deploy-database | |
| steps: | |
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| - name: Download database info | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: database-status | |
| - name: Load database URL | |
| run: | | |
| source database-status.env | |
| echo "DATABASE_URL_UNPOOLED=$DATABASE_URL_UNPOOLED" >> $GITHUB_ENV | |
| - name: Deploy Electric to Fly.io | |
| uses: superfly/fly-pr-review-apps@1.3.0 | |
| with: | |
| name: superset-electric-pr-${{ env.PR_NUMBER }} | |
| region: iad | |
| org: superset-670 | |
| config: fly.toml | |
| secrets: | | |
| DATABASE_URL=${{ env.DATABASE_URL_UNPOOLED }} | |
| ELECTRIC_SECRET=${{ secrets.ELECTRIC_SECRET_PREVIEW }} | |
| env: | |
| FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} | |
| - name: Save Electric status | |
| run: | | |
| cat > electric-status.env << EOF | |
| ELECTRIC_STATUS="✅" | |
| ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ env.PR_NUMBER }}\">View App</a>" | |
| EOF | |
| - name: Upload Electric status | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: electric-status | |
| path: electric-status.env |
🧰 Tools
🪛 actionlint (1.7.9)
93-93: shellcheck reported issue in this script: SC2086:info:2:56: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In .github/workflows/deploy-preview.yml around lines 78 to 121, the job uses
github.event.number in the Fly app name and ELECTRIC_LINK which is inconsistent
with the workflow-level PR_NUMBER env var; replace all occurrences of
github.event.number here with the workflow-level PR_NUMBER variable (use the
already-defined PR_NUMBER environment variable) so the Fly app name, the
ELECTRIC_LINK, and any other PR references in this job use PR_NUMBER for
consistency with cleanup-preview.yml.
| docker run -d \ | ||
| --name "$ELECTRIC_CONTAINER" \ | ||
| -p 3000 \ | ||
| -e DATABASE_URL="$DIRECT_URL" \ | ||
| -e ELECTRIC_SECRET="$ELECTRIC_SECRET" \ | ||
| electricsql/electric:latest |
There was a problem hiding this comment.
Missing error handling for docker run failure.
If docker run fails (e.g., image pull failure, port conflict), the script continues silently. Consider capturing the exit status and failing early.
🔎 Proposed fix
# Start Electric container with auto-assigned port
-docker run -d \
+if ! docker run -d \
--name "$ELECTRIC_CONTAINER" \
-p 3000 \
-e DATABASE_URL="$DIRECT_URL" \
-e ELECTRIC_SECRET="$ELECTRIC_SECRET" \
- electricsql/electric:latest
+ electricsql/electric:latest; then
+ error "Failed to start Electric container"
+fi
📝 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.
| docker run -d \ | |
| --name "$ELECTRIC_CONTAINER" \ | |
| -p 3000 \ | |
| -e DATABASE_URL="$DIRECT_URL" \ | |
| -e ELECTRIC_SECRET="$ELECTRIC_SECRET" \ | |
| electricsql/electric:latest | |
| if ! docker run -d \ | |
| --name "$ELECTRIC_CONTAINER" \ | |
| -p 3000 \ | |
| -e DATABASE_URL="$DIRECT_URL" \ | |
| -e ELECTRIC_SECRET="$ELECTRIC_SECRET" \ | |
| electricsql/electric:latest; then | |
| error "Failed to start Electric container" | |
| fi |
🤖 Prompt for AI Agents
In .superset/setup.sh around lines 74-79, the docker run call does not check for
failure; update the script to capture docker's exit status immediately after the
command, and if non-zero print a descriptive error to stderr (including docker's
exit code or output) and exit with a non-zero status; alternatively enable
strict exit behavior (set -euo pipefail) earlier in the script and/or wrap the
docker run in an if-check to fail fast on errors.
| export function OrganizationsProvider({ children }: { children: ReactNode }) { | ||
| const { data: organizations, isLoading } = | ||
| trpc.user.myOrganizations.useQuery(); | ||
|
|
||
| if (isLoading) { | ||
| return ( | ||
| <div className="flex h-full items-center justify-center"> | ||
| <div className="h-5 w-5 animate-spin rounded-full border-2 border-muted-foreground border-t-transparent" /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (!organizations?.length) { | ||
| return <div className="p-4 text-destructive">No organizations found</div>; | ||
| } | ||
|
|
||
| return ( | ||
| <OrganizationsContext.Provider value={{ organizations }}> | ||
| {children} | ||
| </OrganizationsContext.Provider> | ||
| ); |
There was a problem hiding this comment.
Missing error state handling.
The component handles isLoading and empty data but doesn't handle query failures. If the TRPC query fails, organizations will be undefined and the component will show "No organizations found" which may be misleading.
🔎 Proposed fix
export function OrganizationsProvider({ children }: { children: ReactNode }) {
- const { data: organizations, isLoading } =
+ const { data: organizations, isLoading, isError, error } =
trpc.user.myOrganizations.useQuery();
if (isLoading) {
return (
<div className="flex h-full items-center justify-center">
<div className="h-5 w-5 animate-spin rounded-full border-2 border-muted-foreground border-t-transparent" />
</div>
);
}
+ if (isError) {
+ return (
+ <div className="p-4 text-destructive">
+ Failed to load organizations: {error?.message ?? "Unknown error"}
+ </div>
+ );
+ }
+
if (!organizations?.length) {
return <div className="p-4 text-destructive">No organizations found</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 function OrganizationsProvider({ children }: { children: ReactNode }) { | |
| const { data: organizations, isLoading } = | |
| trpc.user.myOrganizations.useQuery(); | |
| if (isLoading) { | |
| return ( | |
| <div className="flex h-full items-center justify-center"> | |
| <div className="h-5 w-5 animate-spin rounded-full border-2 border-muted-foreground border-t-transparent" /> | |
| </div> | |
| ); | |
| } | |
| if (!organizations?.length) { | |
| return <div className="p-4 text-destructive">No organizations found</div>; | |
| } | |
| return ( | |
| <OrganizationsContext.Provider value={{ organizations }}> | |
| {children} | |
| </OrganizationsContext.Provider> | |
| ); | |
| export function OrganizationsProvider({ children }: { children: ReactNode }) { | |
| const { data: organizations, isLoading, isError, error } = | |
| trpc.user.myOrganizations.useQuery(); | |
| if (isLoading) { | |
| return ( | |
| <div className="flex h-full items-center justify-center"> | |
| <div className="h-5 w-5 animate-spin rounded-full border-2 border-muted-foreground border-t-transparent" /> | |
| </div> | |
| ); | |
| } | |
| if (isError) { | |
| return ( | |
| <div className="p-4 text-destructive"> | |
| Failed to load organizations: {error?.message ?? "Unknown error"} | |
| </div> | |
| ); | |
| } | |
| if (!organizations?.length) { | |
| return <div className="p-4 text-destructive">No organizations found</div>; | |
| } | |
| return ( | |
| <OrganizationsContext.Provider value={{ organizations }}> | |
| {children} | |
| </OrganizationsContext.Provider> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx around lines 23
to 43, the TRPC query result isn't handling errors so failures show "No
organizations found"; update the hook destructure to include isError and error
(e.g., const { data: organizations, isLoading, isError, error } = ...), add an
early return when isError that renders a clear error state (display
error.message or a user-friendly fallback and optionally log the error), and
keep the existing isLoading and empty-data branches unchanged so users see an
explicit error message instead of the misleading "No organizations found".
| openOrganizationDatabase(organizationId).then((result) => { | ||
| if (cancelled) return; | ||
| setState(result); | ||
| }); |
There was a problem hiding this comment.
Missing error handling for openOrganizationDatabase.
If the database fails to open, the promise rejection is silently ignored. This could leave the component in a stuck state with state remaining null.
🔎 Proposed fix
- openOrganizationDatabase(organizationId).then((result) => {
- if (cancelled) return;
- setState(result);
- });
+ openOrganizationDatabase(organizationId)
+ .then((result) => {
+ if (cancelled) return;
+ setState(result);
+ })
+ .catch((err) => {
+ if (cancelled) return;
+ console.error("Failed to open organization database:", err);
+ // Consider exposing error state to consumers
+ });
📝 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.
| openOrganizationDatabase(organizationId).then((result) => { | |
| if (cancelled) return; | |
| setState(result); | |
| }); | |
| openOrganizationDatabase(organizationId) | |
| .then((result) => { | |
| if (cancelled) return; | |
| setState(result); | |
| }) | |
| .catch((err) => { | |
| if (cancelled) return; | |
| console.error("Failed to open organization database:", err); | |
| // Consider exposing error state to consumers | |
| }); |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts
around lines 39-42, the promise returned by openOrganizationDatabase is awaited
without error handling so rejections are silently ignored; wrap the call in a
.catch (or convert to async/await with try/catch) to handle failures: in the
catch handler check the cancelled flag, log the error (or pass it to an error
state setter), and set state/loading appropriately (or set an error state) so
the component doesn't remain stuck; ensure no state updates occur after
cancelled is true.
| startSync(state.pg, accessToken, organizationId).then((sync) => { | ||
| syncRef.current = sync; | ||
| }); |
There was a problem hiding this comment.
Missing error handling for startSync.
Similarly, sync failures are silently ignored. Consider logging or exposing sync errors to the UI.
🔎 Proposed fix
- startSync(state.pg, accessToken, organizationId).then((sync) => {
- syncRef.current = sync;
- });
+ startSync(state.pg, accessToken, organizationId)
+ .then((sync) => {
+ syncRef.current = sync;
+ })
+ .catch((err) => {
+ console.error("Failed to start Electric sync:", err);
+ });
📝 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.
| startSync(state.pg, accessToken, organizationId).then((sync) => { | |
| syncRef.current = sync; | |
| }); | |
| startSync(state.pg, accessToken, organizationId) | |
| .then((sync) => { | |
| syncRef.current = sync; | |
| }) | |
| .catch((err) => { | |
| console.error("Failed to start Electric sync:", err); | |
| }); |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts
around lines 56–58, the promise returned by startSync is not handling rejections
so sync failures are swallowed; update the call to handle errors by attaching a
.catch handler (or use try/await) that logs the error (via the existing logger
or console.error) and clears or nulls syncRef.current on failure, and also
surface the error to the hook consumers by setting an error state (e.g.,
setSyncError) so the UI can display or react to sync errors.
- Replace hardcoded ngrok URLs with env.NEXT_PUBLIC_API_URL - Add NODE_ENV to API env schema with development default - Make CORS localhost conditional on development mode - Add runtime JWT payload.sub validation instead of type assertion - Fix empty userIds edge case in Electric utils (prevents invalid SQL) - Fix unhandled promise rejection in auth subscription - Import canonical updateTaskSchema from @superset/trpc (no drift) - Add useLiveDrizzle hook with automatic type inference from Drizzle - Remove unnecessary comments throughout codebase - Inject API URL into CSP via Vite transform (remove hardcoded localhost) - Pin Electric SQL image to v1.2.9 (was :latest) - Use FLY_ORG variable instead of hardcoded org in deploy workflow - Fix github.event.number → github.event.pull_request.number consistency - Throw TRPC error when user record missing (was silently returning empty) - Remove obsolete documentation files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
62-62: Pass column directly toinArray()without wrapping.The column should be passed directly to
inArray()rather than wrapping it withsql.identifier(). Drizzle'sinArray()function acceptsPgColumndirectly and handles the identifier internally.🔎 Proposed fix
- const whereExpr = inArray(sql`${sql.identifier(users.id.name)}`, userIds); + const whereExpr = inArray(users.id, userIds);apps/api/src/app/api/electric/[...path]/route.ts (1)
69-69: Add error handling and timeout for the Electric fetch.The
fetch()call lacks error handling and timeout, which can cause unhandled exceptions on network failures or indefinite hangs on slow responses.🔎 Proposed fix
- const response = await fetch(originUrl.toString()); + let response: Response; + try { + response = await fetch(originUrl.toString(), { + signal: AbortSignal.timeout(30_000), + }); + } catch (error) { + console.error("Electric proxy error:", error); + return new Response("Upstream service unavailable", { status: 502 }); + }.github/workflows/deploy-preview.yml (1)
100-100: Use consistent PR number reference throughout the workflow.Lines 100 and 114 use
github.event.pull_request.numberwhile the workflow-levelPR_NUMBERenvironment variable is defined at line 14. For consistency with the cleanup workflow and to avoid edge-case issues, use the workflow-level environment variable.🔎 Proposed fix
- name: Deploy Electric to Fly.io uses: superfly/fly-pr-review-apps@1.3.0 with: - name: superset-electric-pr-${{ github.event.pull_request.number }} + name: superset-electric-pr-${{ env.PR_NUMBER }} region: iad org: ${{ vars.FLY_ORG }} config: fly.toml secrets: | DATABASE_URL=${{ env.DATABASE_URL_UNPOOLED }} ELECTRIC_SECRET=${{ secrets.ELECTRIC_SECRET_PREVIEW }} env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} - name: Save Electric status run: | cat > electric-status.env << EOF ELECTRIC_STATUS="✅" - ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ github.event.pull_request.number }}\">View App</a>" + ELECTRIC_LINK="<a href=\"https://fly.io/apps/superset-electric-pr-${{ env.PR_NUMBER }}\">View App</a>" EOFAlso applies to: 114-114
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (3)
52-56: Form state doesn't reset when switching between tasks.The
useStatehooks initialize fromtaskprop only on first mount. When a user edits Task A, closes the dialog, then opens Task B, the form will still show Task A's values.Add
key={editingTask.id}to force remount:{editingTask && ( <TaskEditDialog + key={editingTask.id} task={editingTask} open={!!editingTask} onOpenChange={(open) => !open && setEditingTask(null)} /> )}Also applies to: 295-301
250-257: Type assertion onlabelscould fail at runtime.The database schema defines
labelsasjsonb, which could contain any JSON value. The type assertion(task.labels as string[])will cause runtime errors if data is malformed.🔎 Proposed defensive check
{task.labels && task.labels.length > 0 && ( <div className="flex flex-wrap gap-1 mt-2"> - {(task.labels as string[]).map((label) => ( + {Array.isArray(task.labels) && task.labels.filter((l): l is string => typeof l === 'string').map((label) => ( <Badge key={label} variant="outline" className="text-xs"> {label} </Badge> ))} </div> )}
131-133: Hardcoded "Linear" reference may not match actual provider.The button text assumes the external provider is always Linear, but
task.external_providercan vary.- {isSaving ? "Saving..." : "Save & Sync to Linear"} + {isSaving ? "Saving..." : task.external_provider ? `Save & Sync to ${task.external_provider}` : "Save"}apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
29-35: Guard against emptyorganizationsarray.If
organizationsis empty, accessingorganizations[0].idon line 33 will throw. WhileOrganizationsProviderlikely ensures data is loaded before rendering children, adding a defensive check improves robustness.🔎 Proposed fix
const [activeOrganizationId, setActiveOrganizationId] = useState<string>( () => { + if (organizations.length === 0) { + throw new Error("PGliteProvider requires at least one organization"); + } const stored = localStorage.getItem(ACTIVE_ORG_KEY); const valid = organizations.find((o) => o.id === stored); return valid?.id ?? organizations[0].id; }, );
🧹 Nitpick comments (3)
packages/trpc/src/router/user/user.ts (1)
44-64: Implementation looks correct.The query correctly uses
findManywith eager loading and maps to the organization objects. The error handling is consistent withmyOrganization.Consider extracting the user lookup to reduce duplication across
me,myOrganization, andmyOrganizations:🔎 Optional: Extract user lookup helper
// In a shared utility or at the top of this file async function getUserByClerkId(clerkId: string) { const user = await db.query.users.findFirst({ where: eq(users.clerkId, clerkId), }); if (!user) { throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "User record not found", }); } return user; }Then simplify procedures:
myOrganizations: protectedProcedure.query(async ({ ctx }) => { - const user = await db.query.users.findFirst({ - where: eq(users.clerkId, ctx.userId), - }); - - if (!user) { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "User record not found", - }); - } + const user = await getUserByClerkId(ctx.userId); const memberships = await db.query.organizationMembers.findMany({apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
1-349: Consider splitting into separate component files.This file contains 6 components (TaskEditDialog, TaskCard, TasksList, Sidebar, TasksViewContent, TasksView). As per coding guidelines, each component should be in its own file with a folder structure like:
TasksView/ ├── TasksView.tsx ├── index.ts └── components/ ├── TaskEditDialog/ ├── TaskCard/ ├── TasksList/ └── Sidebar/This improves maintainability and aligns with the project's component organization conventions.
apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx (1)
44-46: Consider handling subscription errors.The subscription doesn't handle errors that might occur (e.g., if the auth service fails to retrieve tokens). Consider adding
onErrorto gracefully handle failures.🔎 Proposed improvement
trpc.auth.onAccessToken.useSubscription(undefined, { onData: ({ accessToken }) => setAccessToken(accessToken), + onError: (err) => { + console.error("[PGliteProvider] Token subscription error:", err); + }, });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.github/workflows/cleanup-preview.yml.github/workflows/deploy-preview.yml.gitignoreapps/api/src/app/api/electric/[...path]/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/env.tsapps/api/src/lib/auth.tsapps/api/src/proxy.tsapps/desktop/electron.vite.config.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.tsapps/desktop/src/lib/trpc/routers/user/index.tsapps/desktop/src/renderer/index.htmlapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxfly.tomlpackages/trpc/src/index.tspackages/trpc/src/lib/integrations/sync/tasks.tspackages/trpc/src/router/user/user.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/trpc/src/lib/integrations/sync/tasks.ts
- apps/api/src/proxy.ts
- apps/desktop/electron.vite.config.ts
- apps/desktop/src/renderer/index.html
- apps/desktop/src/renderer/lib/pglite/hooks.ts
- apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts
- .gitignore
- apps/api/src/lib/auth.ts
- fly.toml
- apps/desktop/src/renderer/lib/pglite/index.ts
- apps/desktop/src/lib/trpc/routers/user/index.ts
🧰 Additional context used
📓 Path-based instructions (10)
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/env.tsapps/api/src/app/api/electric/[...path]/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/api/src/app/api/integrations/linear/callback/route.tspackages/trpc/src/index.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/env.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/api/src/app/api/electric/[...path]/route.tsapps/desktop/src/lib/trpc/routers/tasks/index.tspackages/trpc/src/router/user/user.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/api/src/app/api/integrations/linear/callback/route.tspackages/trpc/src/index.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/electric/[...path]/utils.tsapps/api/src/env.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/api/src/app/api/electric/[...path]/route.tsapps/desktop/src/lib/trpc/routers/tasks/index.tspackages/trpc/src/router/user/user.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.ts
🧠 Learnings (5)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
packages/trpc/src/index.tsapps/api/src/app/api/electric/[...path]/utils.tspackages/trpc/src/router/user/user.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
packages/trpc/src/index.tsapps/desktop/src/lib/trpc/routers/auth/index.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
apps/api/src/app/api/electric/[...path]/utils.tspackages/trpc/src/router/user/user.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/api/src/env.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/tasks/index.ts
🧬 Code graph analysis (5)
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
apps/api/src/env.ts (1)
env(4-49)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (1)
apps/api/src/env.ts (1)
env(4-49)
apps/desktop/src/lib/trpc/routers/auth/index.ts (4)
packages/trpc/src/index.ts (1)
publicProcedure(15-15)packages/trpc/src/trpc.ts (1)
publicProcedure(44-44)apps/desktop/src/main/lib/auth/auth-service.ts (1)
authService(360-360)apps/desktop/src/main/lib/auth/index.ts (1)
authService(1-1)
apps/desktop/src/lib/trpc/routers/tasks/index.ts (3)
apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)packages/trpc/src/index.ts (2)
publicProcedure(15-15)updateTaskSchema(6-6)apps/desktop/src/main/lib/api-client.ts (1)
apiClient(11-27)
packages/trpc/src/router/user/user.ts (1)
packages/db/src/schema/schema.ts (2)
users(28-48)organizationMembers(77-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Web
- GitHub Check: Build
🔇 Additional comments (12)
packages/trpc/src/router/user/user.ts (2)
3-3: LGTM!Clean import addition for
TRPCErrorto support the new error handling pattern.
27-32: LGTM - explicit error handling is an improvement.Throwing
TRPCErrorinstead of returningnullmakes the failure mode explicit and easier for consumers to handle. TheINTERNAL_SERVER_ERRORcode is appropriate since an authenticated user (via Clerk) should always have a corresponding DB record after calling themeendpoint.apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (1)
31-35: Previous hardcoded URL issue resolved.The QStash signature verification now correctly uses
env.NEXT_PUBLIC_API_URLinstead of the hardcoded ngrok URL, which will work correctly across all environments.apps/api/src/env.ts (1)
5-9: LGTM! Environment configuration is properly validated.The new environment variables follow best practices:
NODE_ENVwith enum validation and sensible defaultELECTRIC_URLvalidated as a proper URLELECTRIC_SECRETwith minimum length requirement- Correct runtime environment mapping
Also applies to: 14-15, 40-40
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
102-115: Previous hardcoded URL issue resolved with proper error handling.The code now correctly uses
env.NEXT_PUBLIC_API_URLand includes try/catch error handling that gracefully degrades by redirecting with a warning parameter when the sync job fails to queue.apps/api/src/app/api/electric/[...path]/utils.ts (1)
58-60: LGTM! Empty array guard added.The empty members check correctly prevents invalid SQL generation when no organization members are found.
.github/workflows/cleanup-preview.yml (1)
25-47: LGTM! Electric cleanup integrated properly.The workflow correctly:
- Sets up Fly CLI
- Deletes the Electric app with proper error handling (
continue-on-error: true)- Reports cleanup status for both Neon and Electric resources
.github/workflows/deploy-preview.yml (2)
194-195: LGTM! Electric configuration properly propagated to API deployment.The
ELECTRIC_URLandELECTRIC_SECRETenvironment variables are correctly passed to both the build environment and the Vercel deployment command, enabling the API to connect to the Electric sync service.Also applies to: 226-227
570-570: LGTM! Post-deployment comment logic handles Electric status.The final comment generation correctly:
- Includes
deploy-electricin the job dependencies- Initializes Electric status variables with failure defaults
- Sources the Electric status file when deployment succeeds
- Exports Electric variables for the comment template
Also applies to: 589-590, 606-608, 630-630
apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
42-71: LGTM! Well-structured subscription with proper error handling.The implementation correctly addresses the previous review feedback:
- Try/catch wraps the async operation with
emit.error()for proper error propagationvoid emitToken()explicitly acknowledges fire-and-forget calls- Handler wrapper ensures consistent error handling for event listeners
- Cleanup properly unregisters both event listeners
The observable pattern is correctly used as required for trpc-electron subscriptions.
packages/trpc/src/index.ts (1)
5-6: LGTM! Clean schema exports.Making the canonical task schemas available at the package root is good practice and enables consumers to import them directly, preventing schema drift across the codebase.
apps/desktop/src/lib/trpc/routers/tasks/index.ts (1)
1-3: Good use of canonical schema import.Importing
updateTaskSchemafrom@superset/trpcinstead of defining it locally addresses the schema drift concern from previous reviews. This ensures consistency with the server-side schema definitions.
| export const createTasksRouter = () => { | ||
| return router({ | ||
| update: publicProcedure | ||
| .input(updateTaskSchema) | ||
| .mutation(async ({ input }) => { | ||
| const result = await apiClient.task.update.mutate(input); | ||
| return result; | ||
| }), | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for task subscription implementations in the desktop tRPC routers
rg -nP --type=ts 'subscription.*Task|\.subscription\(' apps/desktop/src/lib/trpc/routers/
Repository: superset-sh/superset
Length of output: 783
🏁 Script executed:
#!/bin/bash
# Search broadly for task subscriptions
rg -nP --type=ts 'subscription.*task|task.*subscription' apps/desktop/
# Check if tasks/index.ts has any subscription commented out or planned
cat -n apps/desktop/src/lib/trpc/routers/tasks/index.ts
# Check one existing subscription implementation to verify observable pattern usage
head -n 30 apps/desktop/src/lib/trpc/routers/notifications.ts
Repository: superset-sh/superset
Length of output: 1597
🏁 Script executed:
#!/bin/bash
# Search for any task subscription implementation across entire desktop app
rg -niP 'task.*subscription|subscription.*task' apps/desktop/src/
# Also check if there's a subscribe procedure in tasks router
rg -A 20 'export const createTasksRouter' apps/desktop/src/lib/trpc/routers/tasks/index.ts
Repository: superset-sh/superset
Length of output: 307
Add subscription procedure for real-time task updates.
The tasks router currently implements only an update mutation but lacks the subscription procedure required by the PR objectives ("subscription-only pattern for real-time task updates when the selected organization changes"). Other routers in the same directory (notifications, auth, ports, menu, auto-update) implement subscriptions using the observable pattern from @trpc/server/observable.
Add a subscription procedure following this established pattern:
subscribe: publicProcedure.subscription(() => {
return observable<TaskUpdate>((emit) => {
// Setup event listener for task updates
// Call emit.next() when organization changes
// Return cleanup function
});
}),
🤖 Prompt for AI Agents
In apps/desktop/src/lib/trpc/routers/tasks/index.ts around lines 5 to 14, add a
new "subscribe" procedure to the returned router that follows the same
observable subscription pattern used by the other routers: import observable
from "@trpc/server/observable" and the TaskUpdate type, then add subscribe:
publicProcedure.subscription(() => observable<TaskUpdate>((emit) => { register
the task-update event listener (using the same event source used by other
routers or the apiClient event emitter), call emit.next(payload) whenever the
selected organization changes, and return a cleanup function that removes the
listener }); ) so the router exposes real-time task updates and cleans up
listeners properly.
| const switchOrganization = (newOrganizationId: string) => { | ||
| localStorage.setItem(ACTIVE_ORG_KEY, newOrganizationId); | ||
| setActiveOrganizationId(newOrganizationId); | ||
| }; |
There was a problem hiding this comment.
Validate organization ID before switching.
switchOrganization doesn't verify that newOrganizationId exists in the organizations list. If called with an invalid ID, it will persist a bad value to localStorage and cause the error on line 41 after re-render.
🔎 Proposed fix
const switchOrganization = (newOrganizationId: string) => {
+ const exists = organizations.some((o) => o.id === newOrganizationId);
+ if (!exists) {
+ console.warn(`[PGliteProvider] Invalid organization ID: ${newOrganizationId}`);
+ return;
+ }
localStorage.setItem(ACTIVE_ORG_KEY, newOrganizationId);
setActiveOrganizationId(newOrganizationId);
};
📝 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 switchOrganization = (newOrganizationId: string) => { | |
| localStorage.setItem(ACTIVE_ORG_KEY, newOrganizationId); | |
| setActiveOrganizationId(newOrganizationId); | |
| }; | |
| const switchOrganization = (newOrganizationId: string) => { | |
| const exists = organizations.some((o) => o.id === newOrganizationId); | |
| if (!exists) { | |
| console.warn(`[PGliteProvider] Invalid organization ID: ${newOrganizationId}`); | |
| return; | |
| } | |
| localStorage.setItem(ACTIVE_ORG_KEY, newOrganizationId); | |
| setActiveOrganizationId(newOrganizationId); | |
| }; |
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.