feat(integrations): add Linear integration with bidirectional sync#503
feat(integrations): add Linear integration with bidirectional sync#503saddlepaddle merged 14 commits intomainfrom
Conversation
- Add OAuth flow for connecting Linear workspace - Add webhook handler for real-time sync from Linear - Add QStash job queue for async sync-back to Linear - Add integration connections table for storing OAuth tokens - Add task status metadata fields (color, position, type) - Auto-sync task changes to Linear on create/update - Add test pages at /test/linear (gated to @superset.sh users) - Add PostHog feature flag for test page access 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@saddlepaddle has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds Linear integration with bidirectional task synchronization via OAuth, webhook handling, and async job queues. It includes database schema extensions for integration connections and webhook events, API routes for OAuth flows and webhook processing, TRPC procedures for managing integrations, and a web UI dashboard for configuring integrations. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Web as Web App
participant API as API Server
participant Linear as Linear OAuth
participant DB as Database
participant QStash as QStash
User->>Web: Click "Connect Linear"
Web->>API: GET /integrations/linear/connect?organizationId=...
API->>API: Validate user org membership
API->>API: Encode state (orgId, userId) to base64url
API->>Linear: Redirect to OAuth authorization
User->>Linear: Authorize app
Linear->>API: Redirect to /integrations/linear/callback?code=...&state=...
API->>API: Decode & validate state
API->>Linear: Exchange code for access token
Linear-->>API: Return accessToken
API->>Linear: Create LinearClient, fetch org
Linear-->>API: Organization details
API->>DB: Upsert integrationConnections<br/>(orgId, provider="linear", token, orgName)
DB-->>API: Success
API->>QStash: Publish initial-sync job<br/>(organizationId, creatorUserId)
QStash-->>API: Job queued
API->>Web: Redirect to /integrations/linear
Web-->>User: Show "Connected" status
sequenceDiagram
participant QStash as QStash<br/>(Job Queue)
participant API as API Server
participant Linear as Linear API
participant DB as Database
QStash->>API: POST /integrations/linear/jobs/initial-sync<br/>(organizationId, creatorUserId)
API->>API: Verify QStash signature
API->>DB: Fetch integrationConnections<br/>(orgId, provider="linear")
DB-->>API: Connection + accessToken
API->>Linear: Query all issues via GraphQL
Linear-->>API: Issues[]
API->>DB: Fetch users by email (assignees)
DB-->>API: userByEmail map
API->>API: Map issues to tasks<br/>mapIssueToTask()
API->>DB: Batch insert tasks with<br/>onConflictDoUpdate<br/>(externalProvider, externalId)
DB-->>API: Inserted/updated tasks
API-->>QStash: 200 OK { success: true }
sequenceDiagram
participant Linear as Linear<br/>(Webhook)
participant API as API Server
participant DB as Database
participant QStash as QStash
Linear->>API: POST /integrations/linear/webhook<br/>(signature, payload)
API->>API: Verify signature via<br/>LinearWebhookClient
API->>DB: Store webhookEvent<br/>(status="pending")
API->>DB: Fetch integrationConnections<br/>(organizationId, "linear")
DB-->>API: Connection + config
alt Issue Create/Update
API->>API: mapPriorityFromLinear()<br/>mapIssueToTask()
API->>DB: Fetch assignee user by email
API->>DB: Insert/upsert task<br/>onConflictDoUpdate<br/>(externalProvider, externalId)
else Issue Delete
API->>DB: Soft-delete task<br/>(set deletedAt)
end
API->>DB: Mark webhookEvent as processed
API-->>Linear: 200 OK
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly Related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (15)
apps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsx (1)
38-41: Consider using a semantic color constant for the fallback.The hardcoded
#888fallback forstatusColorworks but could be replaced with a semantic CSS variable or constant for better maintainability.🔎 Suggested refactor
<div className="size-3 rounded-full" - style={{ backgroundColor: statusColor ?? "#888" }} + style={{ backgroundColor: statusColor ?? "hsl(var(--muted-foreground))" }} />Or define a constant at the top of the file:
+const DEFAULT_STATUS_COLOR = "#888"; + interface StatusGroupProps { ... } export function StatusGroup({ ... }) { ... <div className="size-3 rounded-full" - style={{ backgroundColor: statusColor ?? "#888" }} + style={{ backgroundColor: statusColor ?? DEFAULT_STATUS_COLOR }} />apps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsx (2)
7-26: Consider standardizing the date type.The
createdAt: Date | stringunion is flexible but could lead to inconsistencies. Consider standardizing on one type (preferablyDate) and converting at the data layer.However, if the data comes from TRPC/API as strings (which is common with JSON serialization), the current approach is pragmatic and the
formatDatehelper handles both correctly.
71-76: Add safety checks to initials calculation.The initials calculation could fail with edge cases:
- Multiple consecutive spaces in the name
- Empty string parts after split
- Names with special characters
🔎 Proposed fix
const initials = task.assignee?.name - ?.split(" ") - .map((n) => n[0]) + ?.split(/\s+/) + .filter((n) => n.length > 0) + .map((n) => n[0]) .join("") .toUpperCase() .slice(0, 2);This handles multiple spaces and filters out empty parts.
apps/web/src/app/test/linear/page.tsx (2)
18-19: Hardcoded TEST_ORG_ID limits test page functionality.The hardcoded organization ID restricts testing to a single organization. Since this is under
/test/*routes, consider:
- Making it dynamic (fetch from user context or URL params)
- Adding a UI control to input/select organization ID
- Documenting that this is a known limitation for the test interface
Do you want me to generate code to make the organization ID dynamic, perhaps by fetching it from the user's organizations or adding a selector UI?
75-88: Consider awaiting refetch calls for consistent state updates.The
refetch()calls on lines 77 and 86 are not awaited, which could lead to timing issues where the UI updates before the fresh data is loaded.🔎 Suggested improvement
const handleDisconnect = async () => { await disconnectMutation.mutateAsync({ organizationId: TEST_ORG_ID }); - connectionQuery.refetch(); + await connectionQuery.refetch(); }; const handleSync = async () => { const result = await syncIssuesMutation.mutateAsync({ organizationId: TEST_ORG_ID, teamId: selectedTeam ?? undefined, }); // Refetch sync status after sync - syncStatusQuery.refetch(); + await syncStatusQuery.refetch(); return result; };This ensures the refetch completes before the function returns, though the current implementation may be acceptable if you want non-blocking refetches.
packages/db/drizzle/0003_set_up_external_integrations.sql (1)
17-34: Consider encrypting sensitive credentials at rest.The
access_token,refresh_token, andwebhook_secretcolumns store sensitive credentials as plain text. While database-level security provides some protection, encrypting these values at the application layer would add defense-in-depth against data breaches.This is a recommended improvement for a production system handling OAuth tokens.
packages/db/src/schema/ingest.ts (3)
20-20: Consider using an enum for the provider field.The
providerfield is currently a plain text field with expected values like "linear" and "github" mentioned in the comment. Using a Postgres enum would provide:
- Type safety at the database level
- Better query optimization
- Clear documentation of allowed values
🔎 Recommended refactor using Drizzle enum
First, create the enum in
packages/db/src/schema/enums.ts:export const integrationProviderValues = ["linear", "github"] as const; export const integrationProviderEnum = z.enum(integrationProviderValues); export type IntegrationProvider = z.infer<typeof integrationProviderEnum>;Then update the schema:
import { index, integer, jsonb, + pgEnum, pgSchema, text, timestamp, uuid, } from "drizzle-orm/pg-core"; +const providerEnum = pgEnum("integration_provider", ["linear", "github"]); + export const ingestSchema = pgSchema("ingest"); export const webhookEvents = ingestSchema.table( "webhook_events", { id: uuid().primaryKey().defaultRandom(), // Source - provider: text().notNull(), + provider: providerEnum().notNull(),
28-28: Consider using an enum for the status field.The
statusfield currently uses text with expected values "pending", "processed", "failed", and "skipped" (per the comment). A Postgres enum would provide type safety and clearer semantics.🔎 Recommended refactor
import { index, integer, jsonb, + pgEnum, pgSchema, text, timestamp, uuid, } from "drizzle-orm/pg-core"; +const webhookStatusEnum = pgEnum("webhook_event_status", [ + "pending", + "processed", + "failed", + "skipped", +]); + export const ingestSchema = pgSchema("ingest"); export const webhookEvents = ingestSchema.table( "webhook_events", { // ... other fields - status: text().notNull().default("pending"), + status: webhookStatusEnum().notNull().default("pending"),
31-31: Consider adding an upper bound to retryCount.The
retryCountfield has no maximum value, which could lead to infinite retry loops if the processing logic has a bug. Consider adding application-level or database-level constraints.💡 Optional enhancement
You could add a check constraint at the database level:
retryCount: integer("retry_count").notNull().default(0), + // Or in the table definition callback, add: + // check("retry_count_limit", sql`retry_count <= 10`)Alternatively, handle this in your application logic by checking the retry count before incrementing and marking the event as failed if the limit is exceeded.
apps/api/src/app/api/integrations/linear/connect/route.ts (1)
22-30: Consider validating UUID format for organizationId.The organizationId parameter is retrieved from the query string but not validated as a proper UUID before being used in database queries. This could lead to unnecessary database queries with malformed UUIDs.
🔎 Optional validation enhancement
const url = new URL(request.url); const organizationId = url.searchParams.get("organizationId"); if (!organizationId) { return Response.json( { error: "Missing organizationId parameter" }, { status: 400 }, ); } + +// Validate UUID format +const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +if (!uuidRegex.test(organizationId)) { + return Response.json( + { error: "Invalid organizationId format" }, + { status: 400 }, + ); +}apps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsx (1)
17-31: Consider making color mode configurable.The component hardcodes
data-color-mode="light"(Line 20), which forces light theme. For better flexibility, especially if the app supports dark mode, consider making this configurable through props or using the system/user preference.🔎 Optional enhancement for theme support
export interface MarkdownEditorProps extends Omit<MDEditorProps, "onChange" | "value"> { value?: string; onChange?: (value: string) => void; className?: string; + colorMode?: "light" | "dark"; /** Preview mode: "edit", "live" (default, side-by-side), "preview" (read-only) */ previewMode?: "edit" | "live" | "preview"; } const MarkdownEditor = forwardRef<HTMLDivElement, MarkdownEditorProps>( - ({ value, onChange, className, previewMode = "live", ...props }, ref) => { + ({ value, onChange, className, colorMode = "light", previewMode = "live", ...props }, ref) => { return ( - <div ref={ref} data-color-mode="light" className={cn(className)}> + <div ref={ref} data-color-mode={colorMode} className={cn(className)}> <MDEditor value={value} onChange={(val) => onChange?.(val ?? "")} preview={previewMode} hideToolbar={false} {...props} /> </div> ); }, );apps/api/src/app/api/webhooks/linear/route.ts (2)
88-103: Duplicate priority mapping function.This
mapLinearPriorityfunction duplicatesmapPriorityFromLinearinpackages/trpc/src/lib/integrations/linear/linear.ts(lines 33-48). Consider importing from the shared module to maintain a single source of truth.🔎 Proposed fix
+import { mapPriorityFromLinear } from "@superset/trpc/lib/integrations/linear"; // ... other imports -/** - * Map Linear priority (0-4) to our priority enum - */ -function mapLinearPriority( - linearPriority: number, -): "urgent" | "high" | "medium" | "low" | "none" { - switch (linearPriority) { - case 1: - return "urgent"; - case 2: - return "high"; - case 3: - return "medium"; - case 4: - return "low"; - default: - return "none"; - } -} // Then use mapPriorityFromLinear instead of mapLinearPriority in processIssueEvent
127-142: Consider adding uniqueness handling foreventId.The
eventIdis composed oforganizationId-webhookTimestamp. If Linear sends multiple events in rapid succession (same millisecond), this could cause duplicate key conflicts. Consider adding the webhook event type or a unique identifier from the payload.🔎 Proposed fix
const [webhookEvent] = await db .insert(webhookEvents) .values({ provider: "linear", - eventId: `${payload.organizationId}-${payload.webhookTimestamp}`, + eventId: `${payload.organizationId}-${payload.webhookTimestamp}-${payload.type}-${payload.action}`, eventType: `${payload.type}.${payload.action}`, payload: payload as unknown as Record<string, unknown>, status: "pending", }) .returning();packages/trpc/src/router/integration/integration.ts (1)
22-60: Consider extracting user/membership verification helper.The user lookup and membership verification pattern is repeated across all procedures in this router (lines 26-43, 75-92, 112-123, 146-157, 168-180, 192-204). The
linear.tsfile already hasverifyOrgMembershipandverifyOrgAdminhelpers that could be shared.🔎 Suggested approach
Extract the verification helpers to a shared location (e.g.,
packages/trpc/src/lib/auth.ts) and reuse them across both routers:// packages/trpc/src/lib/auth.ts export async function verifyOrgMembership(clerkUserId: string, organizationId: string) { // ... implementation from linear.ts } export async function verifyOrgAdmin(clerkUserId: string, organizationId: string) { // ... implementation from linear.ts }Then import and use in both
integration.tsandlinear.ts.packages/trpc/src/router/integration/linear.ts (1)
242-250: Slug generation may cause collisions.Using
Date.now()for slug suffix could cause collisions if multiple issues are inserted in the same millisecond (within the same transaction). Consider using a more unique identifier.🔎 Proposed fix
-const slug = `${issue.identifier.toLowerCase()}-${Date.now()}`; +const slug = `${issue.identifier.toLowerCase()}-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`;Or use the issue's Linear ID which is guaranteed unique:
-const slug = `${issue.identifier.toLowerCase()}-${Date.now()}`; +const slug = `${issue.identifier.toLowerCase()}-${issue.id.slice(-8)}`;
📜 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 (41)
apps/api/package.jsonapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/connect/route.tsapps/api/src/app/api/jobs/integrations/linear/sync-task/route.tsapps/api/src/app/api/webhooks/linear/route.tsapps/api/src/env.tsapps/web/package.jsonapps/web/src/app/test/layout.tsxapps/web/src/app/test/linear/page.tsxapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/web/src/app/test/linear/tasks/components/CreateTaskForm/index.tsapps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxapps/web/src/app/test/linear/tasks/components/EditTaskDialog/index.tsapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/index.tsapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/index.tsapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/TaskRow/index.tsapps/web/src/app/test/linear/tasks/page.tsxpackages/db/drizzle/0003_set_up_external_integrations.sqlpackages/db/drizzle/0004_add_task_status_metadata.sqlpackages/db/drizzle/meta/0003_snapshot.jsonpackages/db/drizzle/meta/0004_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/env.tspackages/db/src/schema/enums.tspackages/db/src/schema/index.tspackages/db/src/schema/ingest.tspackages/db/src/schema/relations.tspackages/db/src/schema/schema.tspackages/shared/src/constants.tspackages/trpc/package.jsonpackages/trpc/src/env.tspackages/trpc/src/lib/integrations/linear/index.tspackages/trpc/src/lib/integrations/linear/linear.tspackages/trpc/src/root.tspackages/trpc/src/router/integration/index.tspackages/trpc/src/router/integration/integration.tspackages/trpc/src/router/integration/linear.tspackages/trpc/src/router/task/task.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/web/src/app/test/linear/tasks/components/CreateTaskForm/index.tspackages/trpc/src/root.tsapps/web/src/app/test/linear/tasks/components/TaskRow/index.tspackages/db/src/schema/enums.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/jobs/integrations/linear/sync-task/route.tsapps/api/src/app/api/integrations/linear/connect/route.tsapps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxpackages/db/src/schema/ingest.tspackages/trpc/src/router/integration/index.tsapps/web/src/app/test/linear/page.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxpackages/trpc/src/router/integration/integration.tsapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/api/src/app/api/webhooks/linear/route.tspackages/trpc/src/router/integration/linear.tsapps/web/src/app/test/linear/tasks/components/StatusGroup/index.tsapps/web/src/app/test/linear/tasks/components/MarkdownEditor/index.tsapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsxapps/web/src/app/test/linear/tasks/components/EditTaskDialog/index.tsapps/web/src/app/test/linear/tasks/page.tsxpackages/trpc/src/lib/integrations/linear/linear.tspackages/trpc/src/lib/integrations/linear/index.tspackages/shared/src/constants.tsapps/web/src/app/test/layout.tsxpackages/db/src/schema/index.tspackages/db/src/schema/schema.tsapps/api/src/env.tspackages/trpc/src/env.tspackages/db/src/schema/relations.tspackages/trpc/src/router/task/task.tspackages/db/src/env.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/web/src/app/test/linear/tasks/components/CreateTaskForm/index.tspackages/trpc/src/root.tsapps/web/src/app/test/linear/tasks/components/TaskRow/index.tspackages/db/src/schema/enums.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/jobs/integrations/linear/sync-task/route.tsapps/api/src/app/api/integrations/linear/connect/route.tsapps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxpackages/db/src/schema/ingest.tspackages/trpc/src/router/integration/index.tsapps/web/src/app/test/linear/page.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxpackages/trpc/src/router/integration/integration.tsapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/api/src/app/api/webhooks/linear/route.tspackages/trpc/src/router/integration/linear.tsapps/web/src/app/test/linear/tasks/components/StatusGroup/index.tsapps/web/src/app/test/linear/tasks/components/MarkdownEditor/index.tsapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsxapps/web/src/app/test/linear/tasks/components/EditTaskDialog/index.tsapps/web/src/app/test/linear/tasks/page.tsxpackages/trpc/src/lib/integrations/linear/linear.tspackages/trpc/src/lib/integrations/linear/index.tspackages/shared/src/constants.tsapps/web/src/app/test/layout.tsxpackages/db/src/schema/index.tspackages/db/src/schema/schema.tsapps/api/src/env.tspackages/trpc/src/env.tspackages/db/src/schema/relations.tspackages/trpc/src/router/task/task.tspackages/db/src/env.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/web/src/app/test/linear/tasks/components/CreateTaskForm/index.tsapps/web/src/app/test/linear/tasks/components/TaskRow/index.tsapps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/index.tsapps/web/src/app/test/linear/tasks/components/MarkdownEditor/index.tsapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsxapps/web/src/app/test/linear/tasks/components/EditTaskDialog/index.ts
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Files:
packages/db/src/schema/enums.tspackages/db/src/schema/ingest.tspackages/db/src/schema/index.tspackages/db/src/schema/schema.tspackages/db/src/schema/relations.tspackages/db/src/env.ts
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/jobs/integrations/linear/sync-task/route.tsapps/api/src/app/api/integrations/linear/connect/route.tsapps/api/src/app/api/webhooks/linear/route.tsapps/api/src/env.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/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxapps/web/src/app/test/linear/page.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsxapps/web/src/app/test/linear/tasks/page.tsxapps/web/src/app/test/layout.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/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsxapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsxapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsx
🧠 Learnings (12)
📚 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/web/src/app/test/linear/tasks/components/CreateTaskForm/index.tsapps/web/src/app/test/linear/tasks/components/TaskRow/index.tspackages/trpc/package.jsonapps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsxapps/web/src/app/test/linear/tasks/components/StatusGroup/index.tsapps/web/src/app/test/linear/tasks/components/MarkdownEditor/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 Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
packages/trpc/src/root.tspackages/trpc/package.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 **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/web/package.jsonapps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.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/web/src/app/test/linear/tasks/components/TaskRow/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 **/*.test.{ts,tsx} : Place tests co-located with components using .test.ts or .test.tsx naming convention
Applied to files:
packages/trpc/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} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
packages/trpc/package.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 **/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
packages/trpc/package.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 **/*.{ts,tsx} : Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Applied to files:
packages/trpc/package.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 packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
packages/db/src/schema/ingest.tspackages/db/src/schema/index.tspackages/db/src/schema/schema.tspackages/db/src/schema/relations.tspackages/db/src/env.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:
packages/db/src/schema/ingest.tspackages/db/src/schema/schema.tspackages/db/src/schema/relations.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 src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/ directories
Applied to files:
apps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.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/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.tspackages/db/src/env.ts
🧬 Code graph analysis (14)
packages/trpc/src/root.ts (2)
packages/trpc/src/router/integration/index.ts (1)
integrationRouter(1-1)packages/trpc/src/router/integration/integration.ts (1)
integrationRouter(13-208)
apps/api/src/app/api/integrations/linear/callback/route.ts (3)
apps/api/src/app/api/integrations/linear/connect/route.ts (1)
GET(15-74)apps/api/src/env.ts (1)
env(4-41)packages/db/src/schema/schema.ts (1)
integrationConnections(200-244)
apps/api/src/app/api/integrations/linear/connect/route.ts (3)
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
GET(27-143)packages/db/src/schema/schema.ts (2)
users(20-40)organizationMembers(69-90)apps/api/src/env.ts (1)
env(4-41)
apps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsx (6)
packages/ui/src/components/ui/dialog.tsx (5)
Dialog(134-134)DialogContent(136-136)DialogHeader(139-139)DialogTitle(142-142)DialogFooter(138-138)packages/ui/src/components/ui/label.tsx (1)
Label(24-24)packages/ui/src/components/ui/input.tsx (1)
Input(29-29)apps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsx (1)
MarkdownEditor(35-35)packages/ui/src/components/ui/select.tsx (5)
Select(177-177)SelectTrigger(185-185)SelectValue(186-186)SelectContent(178-178)SelectItem(180-180)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)
apps/web/src/app/test/linear/page.tsx (4)
apps/api/src/env.ts (1)
env(4-41)packages/trpc/src/env.ts (1)
env(4-18)packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)CardDescription(90-90)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)
apps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsx (3)
apps/web/src/app/test/linear/tasks/components/StatusGroup/index.ts (1)
StatusGroup(1-1)packages/ui/src/components/ui/collapsible.tsx (3)
Collapsible(33-33)CollapsibleTrigger(33-33)CollapsibleContent(33-33)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsx (3)
packages/ui/src/components/ui/button.tsx (1)
Button(60-60)packages/ui/src/components/ui/input.tsx (1)
Input(29-29)packages/ui/src/components/ui/select.tsx (5)
Select(177-177)SelectTrigger(185-185)SelectValue(186-186)SelectContent(178-178)SelectItem(180-180)
packages/trpc/src/router/integration/linear.ts (2)
packages/db/src/schema/schema.ts (3)
users(20-40)organizationMembers(69-90)tasks(124-194)packages/trpc/src/lib/integrations/linear/linear.ts (6)
getLinearConnection(257-273)getLinearTeams(278-296)setDefaultLinearTeam(301-324)disconnectLinear(329-360)getLinearClient(53-71)mapPriorityFromLinear(33-48)
apps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsx (4)
packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/web/src/app/test/linear/tasks/components/TaskRow/index.ts (1)
TaskRow(1-1)packages/ui/src/components/ui/badge.tsx (1)
Badge(46-46)packages/ui/src/components/ui/avatar.tsx (3)
Avatar(53-53)AvatarImage(53-53)AvatarFallback(53-53)
apps/web/src/app/test/linear/tasks/components/MarkdownEditor/MarkdownEditor.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/test/linear/tasks/page.tsx (5)
packages/ui/src/components/ai-elements/task.tsx (1)
Task(40-46)apps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsx (1)
CreateTaskForm(21-115)apps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsx (1)
StatusGroup(20-48)apps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsx (1)
TaskRow(70-141)apps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsx (1)
EditTaskDialog(52-188)
apps/web/src/app/test/layout.tsx (1)
packages/shared/src/constants.ts (1)
COMPANY(12-23)
packages/db/src/schema/schema.ts (1)
packages/db/src/schema/enums.ts (2)
taskStatusEnumValues(3-12)taskPriorityValues(17-23)
packages/trpc/src/router/task/task.ts (2)
packages/db/src/schema/enums.ts (1)
taskPriorityValues(17-23)packages/trpc/src/lib/integrations/linear/index.ts (2)
hasLinearConnection(7-7)queueTaskSync(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (55)
packages/trpc/package.json (1)
11-13: Wildcard export pattern is correctly configured with existing directory structure.The export pattern
"./integrations/*"is properly set up: thepackages/trpc/src/lib/integrations/directory exists with the expectedlinear/module containingindex.ts. If testing TypeScript resolution with this pattern, verify that imports likeimport { ... } from "@superset/trpc/integrations/linear"resolve correctly in your tooling.packages/db/drizzle/meta/_journal.json (1)
25-39: LGTM! Migration journal entries are properly structured.The two new migration entries for external integrations setup and task status metadata follow the correct format and naming conventions.
packages/shared/src/constants.ts (1)
47-48: LGTM! Feature flag addition is well-documented.The
TEST_PAGES_ACCESSflag is properly added with a clear description. This aligns with the access control implementation inapps/web/src/app/test/layout.tsx.apps/web/src/app/test/linear/tasks/components/StatusGroup/StatusGroup.tsx (1)
1-48: LGTM! Component structure follows best practices.The
StatusGroupcomponent is well-implemented with:
- Proper "use client" directive for state management
- Clean separation of concerns
- Appropriate use of Collapsible primitives from @superset/ui
- Good default prop handling
The component adheres to the coding guidelines for React + TailwindCSS components.
apps/web/src/app/test/layout.tsx (1)
11-38: LGTM! Layout implementation is clean and focused.The
TestLayoutserver component properly gates access to test pages with clear authentication and authorization checks. The implementation follows Next.js 15 patterns and is well-documented.apps/web/src/app/test/linear/tasks/components/TaskRow/TaskRow.tsx (3)
28-63: LGTM! PriorityIcon is well-implemented.The priority visualization with colored bars is clear and effective. The implementation correctly maps priority levels to visual indicators with appropriate colors.
65-68: LGTM! Date formatting helper is appropriate.The
formatDatehelper correctly handles bothDateandstringinputs. For invalid date strings, it would return "Invalid Date" which could be improved with error handling, but this is acceptable for an internal test UI.
78-141: LGTM! TaskRow component is well-structured.The component provides a comprehensive task display with:
- Clear visual hierarchy
- Proper use of UI primitives from @superset/ui
- Good responsive layout with flex utilities
- Semantic HTML with proper button type
- Appropriate conditional rendering
The implementation follows React 19 and Next.js 15 best practices.
apps/web/src/app/test/linear/tasks/components/TaskRow/index.ts (1)
1-1: LGTM! Barrel export follows project conventions.This follows the coding guideline: "Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export."
Based on coding guidelines, this is the correct pattern for exposing the TaskRow component.
apps/web/src/app/test/linear/page.tsx (3)
21-60: LGTM! Query and mutation setup follows best practices.The component properly:
- Uses conditional query enabling to prevent unnecessary fetches
- Separates queries and mutations appropriately
- Manages local state for selected team
- Follows React Query and TRPC patterns correctly
112-226: LGTM! Connection and teams UI is well-implemented.The sections properly handle:
- Loading states with spinners
- Connection status display
- Conditional connect/disconnect actions
- Team selection with visual feedback
- Error states
The UI provides clear feedback at each step of the integration flow.
228-331: LGTM! Sync and debug sections provide comprehensive testing interface.The implementation includes:
- Clear sync status and statistics
- Appropriate button states and loading indicators
- Detailed sync results with error handling
- Helpful debug panel for development
Since this is an internal test page (gated to @superset.sh users), the debug panel exposure is acceptable.
apps/api/src/env.ts (2)
15-20: LGTM!Server-side environment variables for Linear and QStash integrations are properly validated with
z.string().min(1), ensuring they cannot be empty strings. Secrets are correctly kept in theserversection, preventing client-side exposure.
24-24: LGTM!The
NEXT_PUBLIC_API_URLis properly validated as a URL and correctly propagated toexperimental__runtimeEnvfor client-side access.Also applies to: 33-33
packages/db/drizzle/0003_set_up_external_integrations.sql (1)
54-61: LGTM!Good index design:
webhook_events_provider_status_idxoptimizes webhook processing querieswebhook_events_event_id_idxenables efficient idempotency checkstasks_external_uniqueconstraint prevents duplicate synced tasksThe composite indexes align well with expected query patterns for webhook processing and task synchronization.
packages/db/drizzle/meta/0003_snapshot.json (1)
1-1167: LGTM!This is an auto-generated Drizzle ORM snapshot file that correctly reflects the schema state after migration 0003. The snapshot is consistent with the corresponding SQL migration.
packages/trpc/src/root.ts (1)
5-5: LGTM!The
integrationRouteris cleanly imported and registered following the established pattern. The alphabetical ordering of router registrations is maintained.Also applies to: 15-15
apps/web/src/app/test/linear/tasks/components/StatusGroup/index.ts (1)
1-1: LGTM!Barrel export follows the established component organization pattern. Based on learnings, this aligns with the project convention of one folder per component with an
index.tsbarrel export.apps/web/src/app/test/linear/tasks/components/EditTaskDialog/index.ts (1)
1-1: LGTM!Consistent barrel export pattern for component organization.
packages/db/drizzle/0004_add_task_status_metadata.sql (1)
1-5: LGTM!The new columns appropriately extend task metadata for Linear integration:
status_color,status_type,status_positioncapture Linear's workflow state representationstarted_at,completed_atenable task lifecycle tracking- Using
realforstatus_positionallows flexible ordering with insertions between itemsConsider adding indexes on
started_atandcompleted_atif you plan to filter or sort tasks by these timestamps frequently.packages/db/src/schema/index.ts (1)
1-1: LGTM!The ingest schema export follows the established pattern and correctly exposes the new webhook events functionality.
packages/trpc/src/router/integration/index.ts (1)
1-1: LGTM!Clean barrel export following the established pattern for router modules.
apps/web/src/app/test/linear/tasks/components/CreateTaskForm/index.ts (1)
1-1: LGTM!Component export follows the established pattern: one folder per component with barrel export via index.ts.
As per coding guidelines.
apps/web/src/app/test/linear/tasks/components/MarkdownEditor/index.ts (1)
1-2: LGTM!Proper type and component exports following the established pattern. Exporting the props type separately is good practice for consumers.
As per coding guidelines.
packages/db/src/schema/ingest.ts (1)
11-43: Schema design follows Drizzle ORM best practices.The webhook events table structure is well-designed with appropriate indexes for common query patterns and proper use of Drizzle ORM schema primitives.
Based on learnings.
packages/db/src/schema/enums.ts (1)
16-25: Priority enum implementation is correct and well-structured.The enum values match Linear's official priority levels (No priority, Low, Medium, High, Urgent) and the implementation follows the existing status enum pattern. However, the comment about the 0-4 mapping should be clarified or removed—while Linear integrations do use numeric codes for priorities, the specific mapping (0=No priority, 1=Urgent, 2=High, 3=Medium, 4=Low) is not universally standardized and the code itself doesn't implement this mapping. If the mapping is relevant to the codebase, consider documenting it explicitly where it's actually used.
apps/api/src/app/api/integrations/linear/connect/route.ts (2)
32-54: LGTM! Proper authorization flow.The implementation correctly verifies both user existence and organization membership before proceeding with the OAuth flow. This prevents unauthorized access to the integration connection process.
64-73: OAuth scope is valid per Linear documentation.The scope
"read,write,issues:create"matches Linear's supported scopes. Ensure the registered Linear OAuth app in your workspace is configured with these scopes enabled during deployment.apps/api/src/app/api/jobs/integrations/linear/sync-task/route.ts (3)
26-53: LGTM! Proper QStash signature verification.The handler correctly verifies the QStash signature before processing any payload, preventing unauthorized job execution. Error handling for both missing signatures and invalid JSON is appropriate.
73-82: Consider retry behavior for missing team configuration.When no team ID is configured (Line 81), the handler returns 200 with
skipped: true, which prevents QStash retries. If the team configuration is expected to be added later, this might be too permissive. Consider whether this should return 500 to trigger retries, or if the current behavior is intentional.Is the missing team configuration a permanent state (organization never configured Linear) or a transient condition that might be resolved?
84-105: LGTM! Correct retry behavior for sync failures.The handler properly returns 500 status for sync failures (Line 90), which will trigger QStash to retry the job. Success cases return 200 with the external identifiers for tracking.
apps/web/src/app/test/linear/tasks/page.tsx (4)
12-13: Test organization ID is appropriate for demo page.The hardcoded all-zeros UUID for
TEST_ORG_IDis acceptable for this test/demo page under/test/linear. The comment clearly indicates this is intentional and matches other test pages.
41-44: Polling interval appropriate for test page.The 5-second refetch interval (Line 43) is suitable for a test/demo page to demonstrate real-time Linear synchronization. For production use, consider implementing WebSocket-based updates or a longer polling interval to reduce server load.
46-73: LGTM! Efficient task grouping with proper memoization.The grouping logic correctly organizes tasks by status, preserves status metadata (color, position), and uses
useMemoto prevent unnecessary recalculations. The fallback position of 999 (Line 64) appropriately places statuses without a position at the end.
93-143: Well-structured UI with good UX patterns.The page provides clear loading/error states, helpful empty state messaging (Lines 108-115), and clean component composition. The edit dialog pattern (Lines 137-141) properly manages selected task state.
apps/api/src/app/api/integrations/linear/callback/route.ts (3)
47-66: LGTM! Proper CSRF protection via state validation.The callback correctly validates the state parameter (Lines 47-58), which provides CSRF protection for the OAuth flow. The base64url decoding and payload validation ensure the state originated from the connect endpoint.
68-93: Correct OAuth 2.0 token exchange implementation.The token exchange follows OAuth 2.0 standards with proper content type (Line 74) and all required parameters (Lines 76-81). Error handling includes logging for debugging (Line 87).
109-137: LGTM! Idempotent upsert handles reconnections properly.The upsert pattern (Lines 112-137) correctly handles both new connections and reconnections. The unique constraint on
(organizationId, provider)(Lines 125-128) ensures only one Linear connection per organization, and all relevant fields are updated on reconnection.apps/web/src/app/test/linear/tasks/components/EditTaskDialog/EditTaskDialog.tsx (3)
67-85: LGTM! Proper form initialization and cache management.The
useEffect(Lines 67-74) correctly initializes form state when the task prop changes. The mutation (Lines 76-85) properly invalidates the organization task list cache and closes the dialog on success.
87-98: Good validation and data normalization.The submission handler (Lines 87-98) properly validates required fields, trims whitespace from the title, and normalizes empty descriptions to
null(Line 94) for database consistency.
100-187: Well-structured dialog with good UX patterns.The dialog implementation properly controls visibility via the task prop (Line 101), displays appropriate loading states (Lines 176-182), and provides a clean two-column layout for status and priority fields (Line 131).
apps/web/src/app/test/linear/tasks/components/CreateTaskForm/CreateTaskForm.tsx (1)
72-114: LGTM! Well-designed inline form with good UX.The expanded form provides excellent UX with autofocus (Line 75), clear priority options (Lines 81-97), loading states (Lines 98-104), and easy cancellation (Lines 105-112). As per coding guidelines, this follows React + TailwindCSS patterns correctly.
packages/db/drizzle/meta/0004_snapshot.json (2)
142-300: Database schema properly supports integration requirements.The
integration_connectionstable structure (Lines 142-300) correctly implements the unique constraint on(organization_id, provider)(Lines 287-296), which aligns with the upsert logic in the callback route. Cascade deletes on foreign keys (Lines 269, 282) ensure referential integrity.
655-1031: Task table structure supports bidirectional sync requirements.The tasks table includes:
- Status metadata fields (Lines 690-707) for UI rendering and grouping
- External integration fields (Lines 771-806) for tracking Linear sync state
- Unique constraint on
(external_provider, external_id)(Lines 1019-1026) preventing duplicate importsThis structure properly supports the bidirectional sync functionality described in the PR.
packages/db/src/schema/relations.ts (1)
72-84: LGTM!The new
integrationConnectionsRelationscorrectly defines the one-to-many relationships matching the foreign keys in theintegrationConnectionstable. The bidirectional relations added tousersRelationsandorganizationsRelationsare consistent with existing patterns.apps/api/src/app/api/webhooks/linear/route.ts (1)
219-297: LGTM!The
processIssueEventfunction handles all three action types appropriately. The idempotency check for create, comprehensive field mapping for update, and the intentional skip of task deletion on remove align with the MVP plan described in comments.packages/trpc/src/router/integration/integration.ts (1)
107-140: Legacy endpoint correctly marked for removal.The TODO comment appropriately flags this for cleanup once the test page migrates to
integration.linear.*. The implementation is correct for backwards compatibility.packages/trpc/src/router/task/task.ts (3)
96-105: Asymmetric sync trigger logic between create and update.The
createmutation queues sync ifhasLinearConnectionreturns true (regardless of whether the task is linked to Linear), whileupdateandupdateStatusonly sync iftask.externalProvider === "linear". This means newly created tasks will always sync to Linear if connected, but subsequent updates only sync if the initial sync succeeded. This may be intentional, but consider whether failed initial syncs should be retried on update.
65-108: LGTM!The create mutation correctly:
- Uses Zod validation for all new fields (priority, estimate, dueDate, labels)
- Defaults labels to empty array
- Implements fire-and-forget sync with proper error handling that doesn't block the mutation response
110-142: LGTM!The update mutation properly handles nullable fields and maintains consistent fire-and-forget sync behavior for Linear-linked tasks.
packages/trpc/src/router/integration/linear.ts (1)
61-106: LGTM!The router endpoints correctly implement authorization checks and delegate to the shared Linear integration utilities. The separation between query (getConnection, getTeams, getSyncStatus) and mutation (setDefaultTeam, disconnect, syncIssues) procedures is appropriate.
packages/trpc/src/lib/integrations/linear/index.ts (1)
1-13: LGTM!Clean barrel export that consolidates all Linear integration utilities. The exports match the public API surface defined in
linear.ts.packages/db/src/schema/schema.ts (1)
124-194: LGTM!The tasks table schema extension is well-designed:
- Status as flexible text accommodates external provider states
- Priority enum ensures type safety for internal values
- External sync fields with unique constraint prevent duplicate syncs
- Optional repositoryId correctly handles Linear-only tasks
- Proper indexes on frequently queried columns
packages/trpc/src/lib/integrations/linear/linear.ts (2)
126-236: LGTM!The
syncTaskToLinearfunction properly:
- Handles both create and update flows based on
externalProvider/externalId- Updates sync metadata on success
- Records sync errors without throwing
- Returns structured results for caller handling
362-382: LGTM!The QStash integration is clean:
- Module-level client instantiation is appropriate for serverless
queueTaskSynccorrectly usespublishJSONwith retries- URL construction from environment variable is standard practice
| }, | ||
| "dependencies": { | ||
| "@clerk/nextjs": "^6.36.2", | ||
| "@uiw/react-md-editor": "^4.0.11", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for the latest version and any security advisories for @uiw/react-md-editor
echo "=== Checking npm package info for @uiw/react-md-editor ==="
npm view @uiw/react-md-editor version 2>/dev/null || echo "npm view failed"
echo ""
echo "=== Checking for vulnerabilities ==="
npm audit --package-lock-only 2>/dev/null | grep -i "@uiw" || echo "No @uiw packages with known vulnerabilities in audit"
echo ""
echo "=== Checking package.json in the repo ==="
if [ -f "apps/web/package.json" ]; then
cat apps/web/package.json | grep -A 2 -B 2 "@uiw/react-md-editor" || echo "Dependency not found in package.json"
else
echo "apps/web/package.json not found"
fiRepository: superset-sh/superset
Length of output: 184
🌐 Web query:
@uiw/react-md-editor latest version npm package
💡 Result:
Latest version: 4.0.8
Install: npm i @uiw/react-md-editor. [1][2]
Sources:
[1] npm package page for @uiw/react-md-editor.
[2] uiwjs/react-md-editor GitHub releases.
🌐 Web query:
@uiw/react-md-editor version 4.0.11 vulnerabilities security
💡 Result:
Short answer: There are no public CVEs or confirmed high‑severity advisories for @uiw/react‑md‑editor v4.0.11, but there have been reported XSS concerns in the project’s issue tracker — so treat untrusted input cautiously and sanitize/output-escape in your app. [1][2][3]
Details/sources:
- Snyk/npm vulnerability scans list no direct vulnerabilities for the package (latest noted as 4.0.11). [1][2]
- Official npm package page (package info / recent versions). [3]
- GitHub issue reporting XSS behavior (open discussion about HTML/script being accepted). Review and mitigation there; consider sanitizing (e.g., DOMPurify) and disabling HTML preview options for untrusted content. [4]
If you want, I can:
- run an npm audit for your project and list any indirect/dependency findings, or
- suggest concrete sanitization/config settings or a minimal patch/workaround.
Address XSS handling for untrusted input before using this component.
The @uiw/react-md-editor dependency addition is appropriate, but the package has reported XSS concerns when handling untrusted markdown input. The component will render HTML/scripts if not properly sanitized. Before use, ensure that:
- Untrusted markdown input is sanitized (e.g., using DOMPurify)
- HTML preview mode is disabled for user-generated content
- Output is properly escaped in your application
Reference the package's GitHub issue tracker for mitigation strategies.
🤖 Prompt for AI Agents
In apps/web/package.json around line 15, adding "@uiw/react-md-editor"
introduces XSS risk when rendering untrusted markdown; before using the
component sanitize all untrusted markdown (e.g., run input through DOMPurify or
equivalent on the server/client), ensure HTML preview mode is disabled for
user-generated content in component props/config, and always escape or sanitize
rendered output where the editor content is inserted; update relevant editor
wrapper/components to apply DOMPurify to incoming markdown, set the editor's
preview/html rendering flags to prevent raw HTML execution, and document the
sanitizer usage in the component tests.
| // Check if user has a @superset.sh email | ||
| const primaryEmail = user.emailAddresses.find( | ||
| (email) => email.id === user.primaryEmailAddressId, | ||
| ); | ||
|
|
||
| const hasInternalEmail = primaryEmail?.emailAddress?.endsWith( | ||
| COMPANY.EMAIL_DOMAIN, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/app/test/layout.tsx | head -50Repository: superset-sh/superset
Length of output: 1185
🏁 Script executed:
rg -A 5 'currentUser\(\)' --type ts --type tsx | head -80Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg -A 3 'primaryEmailAddress' --type ts --type tsx | head -50Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg 'currentUser\(' -A 5 | head -100Repository: superset-sh/superset
Length of output: 365
🏁 Script executed:
rg 'primaryEmailAddress' -A 3 | head -50Repository: superset-sh/superset
Length of output: 1363
🏁 Script executed:
rg 'emailAddresses\.find' -B 2 -A 5Repository: superset-sh/superset
Length of output: 1279
Improve type safety by explicitly checking for primaryEmail following the codebase pattern.
The current logic handles edge cases safely through optional chaining and falsy checks. However, packages/trpc/src/router/user/utils/sync-user-from-clerk.ts demonstrates a more explicit pattern used elsewhere in the codebase:
const primaryEmail = user.emailAddresses.find(
(email) => email.id === user.primaryEmailAddressId,
)?.emailAddress;
if (!primaryEmail) {
redirect("/");
}
const hasInternalEmail = primaryEmail.endsWith(COMPANY.EMAIL_DOMAIN);This approach:
- Extracts
.emailAddressdirectly in the find operation - Explicitly checks for null/undefined with a guard clause
- Improves type safety (emailAddress is guaranteed to exist after the check)
- Aligns with TypeScript best practices per your coding guidelines
🤖 Prompt for AI Agents
In apps/web/src/app/test/layout.tsx around lines 23 to 30, change the current
optional-chaining pattern to extract emailAddress directly from the find call
and add an explicit guard for missing primaryEmail: use find(...)? .emailAddress
to get primaryEmail, then if (!primaryEmail) perform the same
redirect/early-exit used elsewhere (e.g. redirect("/") or return) so that after
the check hasInternalEmail can call primaryEmail.endsWith(COMPANY.EMAIL_DOMAIN)
with proper type safety.
| QSTASH_TOKEN: z.string().min(1), | ||
| NEXT_PUBLIC_API_URL: z.string().url(), |
There was a problem hiding this comment.
Configuration mismatch: NEXT_PUBLIC_API_URL should be in client block.
The NEXT_PUBLIC_API_URL variable uses the NEXT_PUBLIC_ prefix, which in Next.js 15 indicates a client-accessible environment variable. However, it's only defined in the server block. This prevents the variable from being accessed on the client side.
🔎 Recommended fix
export const env = createEnv({
server: {
CLERK_SECRET_KEY: z.string().min(1),
BLOB_READ_WRITE_TOKEN: z.string().min(1),
POSTHOG_API_KEY: z.string(),
POSTHOG_PROJECT_ID: z.string(),
QSTASH_TOKEN: z.string().min(1),
- NEXT_PUBLIC_API_URL: z.string().url(),
},
clientPrefix: "PUBLIC_",
- client: {},
+ client: {
+ NEXT_PUBLIC_API_URL: z.string().url(),
+ },
runtimeEnv: process.env,
emptyStringAsUndefined: true,
skipValidation: !!process.env.SKIP_ENV_VALIDATION,
});Note: If this variable needs to be accessible at runtime in the browser, you may also need to add it to experimental__runtimeEnv or use Next.js's built-in public runtime config, depending on your setup.
📝 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.
| QSTASH_TOKEN: z.string().min(1), | |
| NEXT_PUBLIC_API_URL: z.string().url(), | |
| export const env = createEnv({ | |
| server: { | |
| CLERK_SECRET_KEY: z.string().min(1), | |
| BLOB_READ_WRITE_TOKEN: z.string().min(1), | |
| POSTHOG_API_KEY: z.string(), | |
| POSTHOG_PROJECT_ID: z.string(), | |
| QSTASH_TOKEN: z.string().min(1), | |
| }, | |
| clientPrefix: "PUBLIC_", | |
| client: { | |
| NEXT_PUBLIC_API_URL: z.string().url(), | |
| }, | |
| runtimeEnv: process.env, | |
| emptyStringAsUndefined: true, | |
| skipValidation: !!process.env.SKIP_ENV_VALIDATION, | |
| }); |
🤖 Prompt for AI Agents
In packages/trpc/src/env.ts around lines 10-11, NEXT_PUBLIC_API_URL is declared
under the server block but uses the NEXT_PUBLIC_ prefix and must be available to
client code; move the NEXT_PUBLIC_API_URL z.string().url() definition from the
server schema into the client schema (or add an equivalent entry under the
client block) so it’s validated and exposed to the browser, and if the variable
must be available at runtime in the browser also ensure it’s added to your
Next.js runtime public config (e.g., experimental__runtimeEnv or the appropriate
Next.js public runtime mechanism).
| async function findLinearState( | ||
| client: LinearClient, | ||
| teamId: string, | ||
| statusName: string, | ||
| ): Promise<string | undefined> { | ||
| const team = await client.team(teamId); | ||
| const states = await team.states(); | ||
|
|
||
| // Try exact match first | ||
| const exactMatch = states.nodes.find( | ||
| (s) => s.name.toLowerCase() === statusName.toLowerCase(), | ||
| ); | ||
| if (exactMatch) { | ||
| return exactMatch.id; | ||
| } | ||
|
|
||
| // Try partial match | ||
| const partialMatch = states.nodes.find((s) => | ||
| s.name.toLowerCase().includes(statusName.toLowerCase()), | ||
| ); | ||
| return partialMatch?.id; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Linear API createIssue updateIssue stateId required undefined
💡 Result:
Short answer: stateId is optional. When creating an issue, if you omit stateId Linear assigns the team's backlog (or Triage, if enabled) state. When updating, don't send a JS value of undefined — either omit the stateId key from the input object or send a valid workflow state id string; sending undefined can serialize into null/invalid input and cause errors. See Linear docs and examples for creating/updating issues and listing workflow states to get a valid state id. [1][2]
Sources:
- Linear GraphQL docs — creating/editing issues (explains default state when stateId omitted). [1]
- Example of workflow / state IDs (how to fetch valid state ids to use in stateId). [2]
🏁 Script executed:
cd packages/trpc/src/lib/integrations/linear && \
rg -B 5 -A 5 "findLinearState" --type tsRepository: superset-sh/superset
Length of output: 862
🏁 Script executed:
find . -name "*.ts" -o -name "*.tsx" | xargs rg -l "findLinearState" | head -20Repository: superset-sh/superset
Length of output: 117
🏁 Script executed:
cat -n packages/trpc/src/lib/integrations/linear/linear.ts | sed -n '85,140p'Repository: superset-sh/superset
Length of output: 1686
🏁 Script executed:
cat -n packages/trpc/src/lib/integrations/linear/linear.ts | sed -n '140,180p'Repository: superset-sh/superset
Length of output: 1387
🏁 Script executed:
cat -n packages/trpc/src/lib/integrations/linear/linear.ts | sed -n '180,230p'Repository: superset-sh/superset
Length of output: 1560
Passing undefined stateId to updateIssue will cause serialization errors.
The findLinearState function can return undefined when no matching state is found. This value is passed directly to both updateIssue (line 152) and createIssue (line 189). While createIssue safely defaults to the team's backlog state when stateId is omitted, updateIssue will fail if stateId is undefined — it serializes into invalid input and causes errors.
Conditionally include stateId in the updateIssue call only when defined, or provide fallback logic to handle missing state matches.
| export async function hasLinearConnection( | ||
| organizationId: string, | ||
| ): Promise<boolean> { | ||
| const connection = await db.query.integrationConnections.findFirst({ | ||
| where: and( | ||
| eq(integrationConnections.organizationId, organizationId), | ||
| eq(integrationConnections.provider, "linear"), | ||
| ), | ||
| }); | ||
|
|
||
| return !!connection; | ||
| } |
There was a problem hiding this comment.
Inconsistent syncEnabled check between functions.
getLinearClient (line 60) requires syncEnabled: true, but hasLinearConnection does not check this flag. This could cause hasLinearConnection to return true while getLinearClient returns null, leading to confusing behavior in the task creation flow.
🔎 Proposed fix
Either align both functions or add a parameter:
export async function hasLinearConnection(
organizationId: string,
+ requireSyncEnabled = false,
): Promise<boolean> {
- const connection = await db.query.integrationConnections.findFirst({
- where: and(
+ const whereConditions = [
eq(integrationConnections.organizationId, organizationId),
eq(integrationConnections.provider, "linear"),
- ),
- });
+ ];
+ if (requireSyncEnabled) {
+ whereConditions.push(eq(integrationConnections.syncEnabled, true));
+ }
+ const connection = await db.query.integrationConnections.findFirst({
+ where: and(...whereConditions),
+ });
return !!connection;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function hasLinearConnection( | |
| organizationId: string, | |
| ): Promise<boolean> { | |
| const connection = await db.query.integrationConnections.findFirst({ | |
| where: and( | |
| eq(integrationConnections.organizationId, organizationId), | |
| eq(integrationConnections.provider, "linear"), | |
| ), | |
| }); | |
| return !!connection; | |
| } | |
| export async function hasLinearConnection( | |
| organizationId: string, | |
| requireSyncEnabled = false, | |
| ): Promise<boolean> { | |
| const whereConditions = [ | |
| eq(integrationConnections.organizationId, organizationId), | |
| eq(integrationConnections.provider, "linear"), | |
| ]; | |
| if (requireSyncEnabled) { | |
| whereConditions.push(eq(integrationConnections.syncEnabled, true)); | |
| } | |
| const connection = await db.query.integrationConnections.findFirst({ | |
| where: and(...whereConditions), | |
| }); | |
| return !!connection; | |
| } |
🤖 Prompt for AI Agents
In packages/trpc/src/lib/integrations/linear/linear.ts around lines 241 to 252,
hasLinearConnection currently checks for any linear integration connection but
getLinearClient requires syncEnabled: true, causing inconsistent results; update
hasLinearConnection to either (A) include eq(integrationConnections.syncEnabled,
true) in the query so it only returns true when sync is enabled, or (B) add a
boolean parameter (e.g., requireSyncEnabled = true) and use it to conditionally
include the syncEnabled check in the where clause, keeping the default behavior
aligned with getLinearClient.
| const [issues, states] = await Promise.all([ | ||
| team.issues({ | ||
| first: 100, | ||
| filter: { state: { type: { nin: ["canceled", "completed"] } } }, | ||
| }), | ||
| team.states(), | ||
| ]); |
There was a problem hiding this comment.
Pagination not handled for large teams.
The sync fetches only the first 100 issues. Teams with more than 100 active issues will have incomplete syncs. Consider adding pagination or documenting this limitation.
🔎 Suggested enhancement
// Option 1: Add pagination loop
let hasNextPage = true;
let endCursor: string | undefined;
const allIssues = [];
while (hasNextPage) {
const page = await team.issues({
first: 100,
after: endCursor,
filter: { state: { type: { nin: ["canceled", "completed"] } } },
});
allIssues.push(...page.nodes);
hasNextPage = page.pageInfo.hasNextPage;
endCursor = page.pageInfo.endCursor;
}
// Option 2: Document limitation and add total count to response
return { success: true, ...results, total: issues.nodes.length, hasMore: issues.pageInfo.hasNextPage };🤖 Prompt for AI Agents
In packages/trpc/src/router/integration/linear.ts around lines 141 to 147, the
code only fetches the first 100 issues from team.issues, causing incomplete
syncs for teams with >100 active issues; implement proper pagination by looping
requests using the GraphQL pageInfo (hasNextPage and endCursor) and accumulate
nodes until no next page, or alternatively surface pageInfo and a total/hasMore
flag in the response; ensure each paged request reuses the same filter and
merges results consistently before downstream processing.
Add environment variables for Linear OAuth and QStash job queue to both preview and production deployment workflows for the API app. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Vercel-style integrations page with card grid layout - Create Linear integration detail page with OAuth connection flow - Add SSR for dashboard layout and Linear page - Add sidebar navigation with active state for nested routes - Add confirmation dialog for disconnecting integrations - Add error handling with toast notifications for OAuth errors - Clean up unused tRPC routes (getSyncStatus, getTeams) - Simplify getConnection to return boolean - Export Linear utilities via package.json path - Fix type errors in sync-task route 🤖 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: 7
♻️ Duplicate comments (3)
packages/trpc/src/env.ts (1)
14-15: Configuration mismatch already flagged: NEXT_PUBLIC_API_URL should be in client block.This issue was already identified in a previous review. The
NEXT_PUBLIC_prefix indicates a client-accessible variable in Next.js 15, but it's currently only defined in theserverblock, preventing client-side access.Refer to the existing comment above for the recommended fix.
apps/api/src/app/api/integrations/linear/webhook/route.ts (1)
68-80:timingSafeEqualthrows if buffer lengths differ.This issue was previously flagged.
crypto.timingSafeEqualwill throw aRangeErrorif the signature buffer length differs from expected (e.g., different encoding or truncated input). Add a length check before comparison.🔎 Proposed fix
function verifyWebhookSignature( body: string, signature: string, secret: string, ): boolean { const hmac = crypto.createHmac("sha256", secret); hmac.update(body); const expectedSignature = hmac.digest("hex"); + const signatureBuffer = Buffer.from(signature); + const expectedBuffer = Buffer.from(expectedSignature); + if (signatureBuffer.length !== expectedBuffer.length) { + return false; + } return crypto.timingSafeEqual( - Buffer.from(signature), - Buffer.from(expectedSignature), + signatureBuffer, + expectedBuffer, ); }packages/db/src/schema/schema.ts (1)
214-217: Access tokens stored in plain text.This was flagged in a previous review. While the database should be secured, encrypting sensitive tokens at rest provides defense-in-depth against database breaches. Consider implementing application-level encryption for
accessTokenandrefreshToken.
🧹 Nitpick comments (19)
apps/web/src/app/(dashboard)/layout.tsx (1)
11-13: Add error handling for the TRPC call.The
myOrganization.query()call could fail, causing the entire page to error. Consider wrapping this in a try-catch block or using a safe fallback pattern to ensure the dashboard remains accessible even if the organization data fetch fails.🔎 Proposed error handling approach
const trpc = await api(); - const organization = await trpc.user.myOrganization.query(); + let organization; + try { + organization = await trpc.user.myOrganization.query(); + } catch (error) { + console.error("Failed to fetch organization:", error); + organization = null; + } const displayName = organization?.name ?? "Superset";apps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsx (4)
30-55: Consider extracting internal components to separate files.The
DemoVideocomponent is defined in the same file asProductDemo, which violates the coding guideline: "Use one component per file - do not combine multiple components in a single file."Since
DemoVideois only used withinProductDemo, it should be moved to a separate file in a nested components folder:ProductDemo/components/DemoVideo/DemoVideo.tsx.Based on coding guidelines, components should be organized with one component per file and nested in the parent's components/ folder if used only once.
57-84: Consider extracting internal components to separate files.The
SelectorPillcomponent is defined in the same file asProductDemo, which violates the coding guideline: "Use one component per file - do not combine multiple components in a single file."Since
SelectorPillis only used withinProductDemo, it should be moved to a separate file in a nested components folder:ProductDemo/components/SelectorPill/SelectorPill.tsx.Based on coding guidelines, components should be organized with one component per file and nested in the parent's components/ folder if used only once.
7-28: Consider co-locating constants with the component.The
DEMO_OPTIONSconstant could be extracted to a separate constants file (ProductDemo/constants.ts) to follow the guideline for co-locating component dependencies (constants, config, etc.) next to the file using them.Based on coding guidelines, co-locate component dependencies like constants next to the file using them.
87-89: Simplify optional chaining for guaranteed array access.The expression
DEMO_OPTIONS[0]?.label ?? ""uses optional chaining and nullish coalescing, butDEMO_OPTIONS[0]will always exist since the array is statically defined with 4 elements. This defensive pattern adds unnecessary complexity.🔎 Simplified version
- const [activeOption, setActiveOption] = useState( - DEMO_OPTIONS[0]?.label ?? "", - ); + const [activeOption, setActiveOption] = useState(DEMO_OPTIONS[0].label);packages/trpc/src/lib/integrations/sync/tasks.ts (2)
28-43: Consider adding error handling and provider filtering.The current implementation has two areas for improvement:
No error logging:
Promise.allSettledcatches rejected promises, but the function doesn't log failures. Users won't know if a sync attempt failed.No provider filtering: The function syncs to ALL integration connections regardless of the task's
externalProvider. This could cause unnecessary sync loops (e.g., a task from Linear syncing back to Linear).🔎 Suggested improvements
const results = await Promise.allSettled( connections.map(async (conn) => { + // Skip syncing back to the source provider + if (conn.provider === task.externalProvider) { + return { provider: conn.provider, skipped: true, reason: 'source' }; + } + const endpoint = PROVIDER_ENDPOINTS[conn.provider]; if (!endpoint) { - return { provider: conn.provider, skipped: true }; + return { provider: conn.provider, skipped: true, reason: 'no-endpoint' }; } - await qstash.publishJSON({ - url: `${env.NEXT_PUBLIC_API_URL}${endpoint}`, - body: { taskId }, - retries: 3, - }); + try { + await qstash.publishJSON({ + url: `${env.NEXT_PUBLIC_API_URL}${endpoint}`, + body: { taskId }, + retries: 3, + }); + return { provider: conn.provider, queued: true }; + } catch (error) { + console.error(`Failed to queue sync for provider ${conn.provider}:`, error); + return { provider: conn.provider, failed: true, error: String(error) }; + } - - return { provider: conn.provider, queued: true }; }), );
9-11: Consider externalizing provider endpoint configuration.The provider endpoint mapping is hardcoded. As more integrations are added, consider moving this to a configuration file or database table for better maintainability.
apps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsx (1)
14-26: LGTM with a minor note.The error handling logic is solid. Consider extracting the hardcoded path
"/integrations/linear"to a constant or usingwindow.location.pathnameto avoid maintenance burden if the route changes.packages/trpc/src/router/integration/linear/utils.ts (2)
44-68: Consider using TRPCError for proper HTTP status codes.Throwing plain
Errorobjects may not propagate the correct HTTP status to clients. UsingTRPCErrorallows specifying appropriate codes likeUNAUTHORIZEDorFORBIDDEN.🔎 Proposed fix
+import { TRPCError } from "@trpc/server"; export async function verifyOrgMembership( clerkUserId: string, organizationId: string, ) { const user = await db.query.users.findFirst({ where: eq(users.clerkId, clerkUserId), }); if (!user) { - throw new Error("User not found"); + throw new TRPCError({ code: "NOT_FOUND", message: "User not found" }); } const membership = await db.query.organizationMembers.findFirst({ where: and( eq(organizationMembers.organizationId, organizationId), eq(organizationMembers.userId, user.id), ), }); if (!membership) { - throw new Error("Not a member of this organization"); + throw new TRPCError({ code: "FORBIDDEN", message: "Not a member of this organization" }); } return { user, membership }; }
70-84: Same TRPCError suggestion applies here.🔎 Proposed fix
export async function verifyOrgAdmin( clerkUserId: string, organizationId: string, ) { const { user, membership } = await verifyOrgMembership( clerkUserId, organizationId, ); if (membership.role !== "admin") { - throw new Error("Admin access required"); + throw new TRPCError({ code: "FORBIDDEN", message: "Admin access required" }); } return { user, membership }; }apps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsx (1)
47-49: Consider URL-encoding the organizationId.While UUIDs typically don't contain special characters, defensively encoding the parameter prevents potential issues if the format changes.
🔎 Proposed fix
const handleConnect = () => { - window.location.href = `${env.NEXT_PUBLIC_API_URL}/api/integrations/linear/connect?organizationId=${organizationId}`; + window.location.href = `${env.NEXT_PUBLIC_API_URL}/api/integrations/linear/connect?organizationId=${encodeURIComponent(organizationId)}`; };packages/db/drizzle/0006_add_tasks_deleted_at.sql (1)
1-1: Consider adding an index ondeleted_atfor query performance.If task queries frequently filter by
deleted_at IS NULL(as indicated in the task router), an index would improve performance. This is consistent with theusers_deleted_at_idxpattern in the existing schema.🔎 Proposed addition
ALTER TABLE "tasks" ADD COLUMN "deleted_at" timestamp; +CREATE INDEX "tasks_deleted_at_idx" ON "tasks" ("deleted_at");packages/trpc/src/router/integration/integration.ts (2)
22-23: Consider using TRPCError for proper HTTP status codes.Throwing a plain
Errorreturns a 500 status. For "User not found", a 404 or 401 would be more appropriate. UsingTRPCErrorprovides proper error handling.🔎 Proposed fix
+import { TRPCError } from "@trpc/server"; + // In the procedure: if (!user) { - throw new Error("User not found"); + throw new TRPCError({ code: "NOT_FOUND", message: "User not found" }); }
32-33: Use TRPCError with FORBIDDEN code for authorization failures.A membership check failure is an authorization issue (403), not a generic server error (500).
🔎 Proposed fix
if (!membership) { - throw new Error("Not a member of this organization"); + throw new TRPCError({ code: "FORBIDDEN", message: "Not a member of this organization" }); }packages/trpc/src/router/task/schema.ts (1)
23-24: Consider validating status against the enum like priority.
priorityis validated withz.enum(taskPriorityValues), butstatusaccepts any string. For consistency and type safety, consider using the status enum.🔎 Proposed fix
+import { taskPriorityValues, taskStatusEnumValues } from "@superset/db/enums"; + // In updateTaskSchema: - status: z.string().optional(), + status: z.enum(taskStatusEnumValues).optional(),apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (1)
38-39: Unsafe type assertion on config.The
configfield from JSONB could contain any structure. Consider validating with Zod instead of type assertion to prevent runtime errors.🔎 Proposed fix
+const configSchema = z.object({ defaultTeamId: z.string().optional() }).passthrough(); + async function getDefaultTeamId(organizationId: string): Promise<string | null> { // ... fetch connection ... if (!connection?.config) { return null; } - const config = connection.config as { defaultTeamId?: string }; - return config.defaultTeamId ?? null; + const parsed = configSchema.safeParse(connection.config); + if (!parsed.success) { + return null; + } + return parsed.data.defaultTeamId ?? null; }apps/web/src/app/(dashboard)/integrations/linear/page.tsx (1)
16-32: Consider error handling for TRPC calls.The TRPC queries can fail (network issues, auth errors). Currently, errors propagate to Next.js error boundaries. If more granular error handling or fallback UI is needed, consider wrapping in try-catch.
This is acceptable if an error boundary exists at a higher level. Verify error boundaries are configured for this route segment if not already.
apps/api/src/app/api/integrations/linear/callback/route.ts (1)
64-65: Consider validating token response structure.The
tokenDatatype is asserted without validation. If Linear's API returns an unexpected structure, this could cause runtime errors when accessingaccess_token.🔎 Suggested approach
+const tokenDataSchema = z.object({ + access_token: z.string(), + expires_in: z.number().optional(), +}); + - const tokenData: { access_token: string; expires_in?: number } = - await tokenResponse.json(); + const tokenDataResult = tokenDataSchema.safeParse(await tokenResponse.json()); + if (!tokenDataResult.success) { + return Response.redirect( + `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=token_exchange_failed`, + ); + } + const tokenData = tokenDataResult.data;apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (1)
127-160: Consider batching task inserts for better performance.With up to 100 issues per team across multiple teams, sequential inserts could be slow. Drizzle supports batch inserts which would be more efficient.
🔎 Suggested approach
+ const tasksToInsert = []; for (const { issue, assignee, labels, state } of allIssueData) { if (existingByExternalId.has(issue.id)) { continue; } const assigneeId = assignee?.email ? (userByEmail.get(assignee.email) ?? null) : null; - await db.insert(tasks).values({ + tasksToInsert.push({ slug: issue.identifier, title: issue.title, // ... rest of fields - }); + }); } + + if (tasksToInsert.length > 0) { + await db.insert(tasks).values(tasksToInsert); + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
apps/web/public/hero/agents.mp4is excluded by!**/*.mp4apps/web/public/hero/changes.mp4is excluded by!**/*.mp4apps/web/public/hero/open-in.mp4is excluded by!**/*.mp4apps/web/public/hero/tabs.mp4is excluded by!**/*.mp4apps/web/public/hero/worktrees.mp4is excluded by!**/*.mp4apps/web/public/integrations/linear.svgis excluded by!**/*.svgbun.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
apps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/connect/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/app/api/integrations/linear/webhook/route.tsapps/api/src/lib/integrations/linear/utils.tsapps/web/package.jsonapps/web/src/app/(dashboard)/components/Footer/Footer.tsxapps/web/src/app/(dashboard)/components/Footer/index.tsapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Header/index.tsapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/components/ProductDemo/index.tsapps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/web/src/app/(dashboard)/components/SidebarNav/index.tsapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/index.tsapps/web/src/app/(dashboard)/integrations/linear/page.tsxapps/web/src/app/(dashboard)/integrations/page.tsxapps/web/src/app/(dashboard)/integrations/vercel.htmlapps/web/src/app/(dashboard)/layout.tsxapps/web/src/app/(dashboard)/page.tsxapps/web/src/app/download/page.tsxapps/web/src/app/page.tsxapps/web/src/types/stripe-gradient.d.tspackages/db/drizzle/0005_remove_integration_sync_webhook_fields.sqlpackages/db/drizzle/0006_add_tasks_deleted_at.sqlpackages/db/drizzle/meta/0005_snapshot.jsonpackages/db/drizzle/meta/0006_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/schema.tspackages/trpc/src/env.tspackages/trpc/src/lib/integrations/linear/index.tspackages/trpc/src/lib/integrations/sync/index.tspackages/trpc/src/lib/integrations/sync/tasks.tspackages/trpc/src/lib/posthog-client.tspackages/trpc/src/router/integration/integration.tspackages/trpc/src/router/integration/linear/index.tspackages/trpc/src/router/integration/linear/linear.tspackages/trpc/src/router/integration/linear/utils.tspackages/trpc/src/router/task/schema.tspackages/trpc/src/router/task/task.tspackages/trpc/src/router/user/user.ts
💤 Files with no reviewable changes (2)
- apps/web/src/app/download/page.tsx
- apps/web/src/app/page.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/app/(dashboard)/integrations/components/IntegrationCard/index.ts
- packages/db/drizzle/meta/0006_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/src/app/api/integrations/linear/connect/route.ts
- packages/trpc/src/lib/integrations/linear/index.ts
- apps/web/package.json
- packages/trpc/src/router/task/task.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/index.tsapps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/api/src/lib/integrations/linear/utils.tsapps/web/src/app/(dashboard)/components/SidebarNav/index.tsapps/api/src/app/api/integrations/linear/callback/route.tspackages/trpc/src/router/integration/linear/linear.tsapps/web/src/app/(dashboard)/components/Header/index.tsapps/web/src/app/(dashboard)/components/ProductDemo/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/page.tsxapps/web/src/types/stripe-gradient.d.tsapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxpackages/db/src/schema/enums.tspackages/trpc/src/router/user/user.tsapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxpackages/trpc/src/router/integration/linear/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tspackages/trpc/src/lib/integrations/sync/tasks.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/web/src/app/(dashboard)/components/Footer/index.tspackages/trpc/src/router/integration/linear/index.tsapps/web/src/app/(dashboard)/integrations/page.tsxapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Footer/Footer.tsxapps/web/src/app/(dashboard)/layout.tsxpackages/trpc/src/lib/posthog-client.tspackages/db/src/schema/schema.tspackages/trpc/src/lib/integrations/sync/index.tspackages/trpc/src/router/integration/integration.tsapps/web/src/app/(dashboard)/integrations/linear/page.tsxpackages/trpc/src/env.tspackages/trpc/src/router/task/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/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/index.tsapps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/api/src/lib/integrations/linear/utils.tsapps/web/src/app/(dashboard)/components/SidebarNav/index.tsapps/api/src/app/api/integrations/linear/callback/route.tspackages/trpc/src/router/integration/linear/linear.tsapps/web/src/app/(dashboard)/components/Header/index.tsapps/web/src/app/(dashboard)/components/ProductDemo/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/page.tsxapps/web/src/types/stripe-gradient.d.tsapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxpackages/db/src/schema/enums.tspackages/trpc/src/router/user/user.tsapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxpackages/trpc/src/router/integration/linear/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tspackages/trpc/src/lib/integrations/sync/tasks.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/web/src/app/(dashboard)/components/Footer/index.tspackages/trpc/src/router/integration/linear/index.tsapps/web/src/app/(dashboard)/integrations/page.tsxapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Footer/Footer.tsxapps/web/src/app/(dashboard)/layout.tsxpackages/trpc/src/lib/posthog-client.tspackages/db/src/schema/schema.tspackages/trpc/src/lib/integrations/sync/index.tspackages/trpc/src/router/integration/integration.tsapps/web/src/app/(dashboard)/integrations/linear/page.tsxpackages/trpc/src/env.tspackages/trpc/src/router/task/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/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/index.tsapps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/web/src/app/(dashboard)/components/SidebarNav/index.tsapps/web/src/app/(dashboard)/components/Header/index.tsapps/web/src/app/(dashboard)/components/ProductDemo/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/index.tsapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxapps/web/src/app/(dashboard)/components/Footer/index.tsapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Footer/Footer.tsx
**/{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/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Footer/Footer.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/page.tsxapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxapps/web/src/app/(dashboard)/integrations/page.tsxapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Footer/Footer.tsxapps/web/src/app/(dashboard)/layout.tsxapps/web/src/app/(dashboard)/integrations/linear/page.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/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsxapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsxapps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsxapps/web/src/app/(dashboard)/components/Header/Header.tsxapps/web/src/app/(dashboard)/components/Footer/Footer.tsx
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/lib/integrations/linear/utils.tsapps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/webhook/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Files:
packages/db/src/schema/enums.tspackages/db/src/schema/schema.ts
🧠 Learnings (4)
📚 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/web/src/app/(dashboard)/components/SidebarNav/index.tsapps/web/src/app/(dashboard)/components/ProductDemo/index.tsapps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsxapps/web/src/app/(dashboard)/components/Footer/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 packages/db/src/**/*.{ts,tsx} : Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Applied to files:
packages/db/drizzle/meta/0005_snapshot.jsonpackages/trpc/src/router/user/user.tspackages/db/src/schema/schema.tspackages/trpc/src/router/task/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 apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
packages/trpc/src/router/user/user.tspackages/db/src/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 **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/web/src/app/(dashboard)/components/Footer/Footer.tsx
🧬 Code graph analysis (18)
apps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsx (2)
apps/web/src/app/(dashboard)/components/SidebarNav/index.ts (1)
SidebarNav(1-1)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/api/src/lib/integrations/linear/utils.ts (1)
packages/db/src/schema/schema.ts (1)
SelectTask(197-197)
packages/trpc/src/router/integration/linear/linear.ts (2)
packages/trpc/src/router/integration/linear/utils.ts (2)
verifyOrgMembership(44-68)verifyOrgAdmin(70-84)packages/db/src/schema/schema.ts (1)
integrationConnections(200-239)
apps/web/src/app/(dashboard)/page.tsx (2)
packages/shared/src/constants.ts (2)
DOWNLOAD_URL_MAC_ARM64(29-29)COMPANY(12-23)apps/web/src/app/(dashboard)/components/ProductDemo/ProductDemo.tsx (1)
ProductDemo(86-142)
packages/trpc/src/router/user/user.ts (1)
packages/db/src/schema/schema.ts (2)
users(19-39)organizationMembers(68-89)
apps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsx (2)
packages/trpc/src/env.ts (1)
env(4-24)packages/ui/src/components/ui/alert-dialog.tsx (9)
AlertDialog(146-146)AlertDialogTrigger(149-149)AlertDialogContent(150-150)AlertDialogHeader(151-151)AlertDialogTitle(153-153)AlertDialogDescription(154-154)AlertDialogFooter(152-152)AlertDialogCancel(156-156)AlertDialogAction(155-155)
apps/api/src/app/api/integrations/linear/webhook/route.ts (3)
packages/db/src/schema/ingest.ts (1)
webhookEvents(14-43)packages/db/src/schema/schema.ts (4)
integrationConnections(200-239)SelectIntegrationConnection(243-244)tasks(123-194)users(19-39)apps/api/src/lib/integrations/linear/utils.ts (1)
mapLinearPriority(5-18)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (3)
packages/trpc/src/env.ts (1)
env(4-24)packages/db/src/schema/schema.ts (3)
integrationConnections(200-239)users(19-39)tasks(123-194)apps/api/src/lib/integrations/linear/utils.ts (1)
mapLinearPriority(5-18)
packages/trpc/src/lib/integrations/sync/tasks.ts (2)
packages/trpc/src/env.ts (1)
env(4-24)packages/db/src/schema/schema.ts (2)
tasks(123-194)integrationConnections(200-239)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (5)
packages/trpc/src/env.ts (1)
env(4-24)packages/db/src/schema/schema.ts (3)
integrationConnections(200-239)SelectTask(197-197)tasks(123-194)packages/trpc/src/lib/integrations/linear/index.ts (2)
getLinearClient(2-2)mapPriorityToLinear(3-3)packages/trpc/src/router/integration/linear/utils.ts (2)
getLinearClient(27-42)mapPriorityToLinear(10-25)apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (1)
POST(20-60)
apps/web/src/app/(dashboard)/integrations/page.tsx (3)
apps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsx (2)
IntegrationCardProps(7-15)IntegrationCard(17-106)apps/web/src/app/(dashboard)/integrations/components/IntegrationCard/index.ts (2)
IntegrationCardProps(1-1)IntegrationCard(1-1)packages/ui/src/components/ai-elements/image.tsx (1)
Image(9-24)
apps/web/src/app/(dashboard)/components/Footer/Footer.tsx (2)
apps/web/src/app/(dashboard)/components/Footer/index.ts (1)
Footer(1-1)packages/trpc/src/env.ts (1)
env(4-24)
apps/web/src/app/(dashboard)/layout.tsx (3)
apps/web/src/app/(dashboard)/components/Header/Header.tsx (1)
Header(16-60)apps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsx (1)
SidebarNav(12-37)apps/web/src/app/(dashboard)/components/Footer/Footer.tsx (1)
Footer(3-32)
packages/trpc/src/lib/posthog-client.ts (1)
packages/trpc/src/env.ts (1)
env(4-24)
packages/db/src/schema/schema.ts (1)
packages/db/src/schema/enums.ts (2)
taskStatusEnumValues(3-12)taskPriorityValues(16-22)
packages/trpc/src/router/integration/integration.ts (1)
packages/db/src/schema/schema.ts (3)
users(19-39)organizationMembers(68-89)integrationConnections(200-239)
apps/web/src/app/(dashboard)/integrations/linear/page.tsx (4)
apps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/ErrorHandler.tsx (1)
ErrorHandler(14-26)packages/ui/src/components/ai-elements/image.tsx (1)
Image(9-24)packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)apps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsx (1)
ConnectionControls(26-84)
packages/trpc/src/router/task/schema.ts (1)
packages/db/src/schema/enums.ts (1)
taskPriorityValues(16-22)
⏰ 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 Web
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (38)
apps/web/src/types/stripe-gradient.d.ts (1)
1-18: Type declaration looks good.The module declaration is well-structured with appropriate optional typing for the nested
uniformsconfiguration. Type safety is maintained throughout.One minor consideration: if the library supports resuming the animation after
pause(), you may want to add aplay()orresume()method signature for completeness. Otherwise, this minimal declaration covering the used API surface is appropriate.apps/web/src/app/(dashboard)/components/SidebarNav/index.ts (1)
1-1: LGTM!The barrel export follows the project's component organization guidelines correctly.
apps/web/src/app/(dashboard)/components/ProductDemo/index.ts (1)
1-1: LGTM!The barrel export follows the project's component organization pattern.
apps/web/src/app/(dashboard)/components/Header/index.ts (1)
1-1: LGTM!Consistent barrel export pattern.
apps/web/src/app/(dashboard)/components/Footer/index.ts (1)
1-1: LGTM!Consistent barrel export pattern.
apps/web/src/app/(dashboard)/page.tsx (1)
7-41: LGTM!The homepage component is well-structured with:
- Clean two-column responsive layout
- Proper use of shared constants for URLs
- Secure external link attributes (
target="_blank"withrel="noopener noreferrer")- Good use of the
asChildpattern for the Button componentapps/web/src/app/(dashboard)/components/Footer/Footer.tsx (2)
3-32: Component structure looks good otherwise.The Footer component follows React and Tailwind best practices with:
- Semantic
<footer>element- Proper external link attributes
- Accessible separator with
aria-hidden- Clean responsive layout
Pending resolution of the environment variable issue above.
10-10: No action required —NEXT_PUBLIC_MARKETING_URLis properly defined in the environment schema.The variable is defined in
apps/web/src/env.ts(notpackages/trpc/src/env.ts) in the client section asz.string().url(). Footer.tsx correctly imports from@/env, which resolves to the web app's schema. No TypeScript or runtime error will occur.Likely an incorrect or invalid review comment.
apps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsx (1)
12-37: LGTM!The SidebarNav component is well-implemented with:
- Correct
"use client"directive for client-side interactivity- Proper active state logic that handles the root path specially (exact match for "/" to avoid false positives on sub-routes)
- Clean use of Next.js
LinkandusePathname- Accessible navigation structure
- Consistent styling with conditional opacity and underline
apps/web/src/app/(dashboard)/integrations/page.tsx (2)
38-55: LGTM!The integrations page is well-structured with:
- Proper client component directive
- Type-safe integrations array using
IntegrationCardProps[]- Responsive grid layout that adapts to different screen sizes
- Clean separation of data and presentation
17-25: Good use of Next.js Image for SVG assets.The Linear integration correctly uses Next.js
Imagecomponent with:
- Empty
alt=""for decorative icon (appropriate in this context)- Explicit
widthandheightprops- Proper sizing classes
apps/web/src/app/(dashboard)/components/Header/Header.tsx (1)
16-60: LGTM!The Header component is well-structured with proper optional chaining for user data, good accessibility attributes (aria-label on the home link, proper button type), and clean integration with Clerk's authentication system. The fallback initials and avatar handling are appropriate.
apps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsx (2)
42-48: LGTM! The biome-ignore is appropriately used.The spotlight effect is purely visual and decorative, so the biome-ignore for a11y is justified. When the card is enabled, it's wrapped in a Link (line 105), which provides proper keyboard navigation. The mouse events are only for the visual spotlight effect and don't affect functionality.
The implementation correctly handles both enabled and disabled states with appropriate cursor styling and opacity.
17-106: Well-structured component with good accessibility considerations.The component properly:
- Wraps interactive cards in Link for keyboard navigation
- Disables mouse events and link wrapping for disabled cards
- Uses semantic HTML with appropriate ARIA considerations
- Applies defensive programming with optional chaining and type safety
packages/trpc/src/lib/integrations/sync/index.ts (1)
1-1: LGTM!Clean barrel export pattern for the syncTask function.
packages/trpc/src/router/user/user.ts (1)
22-39: LGTM!The
myOrganizationprocedure correctly fetches the user's organization through the membership relationship. Proper null handling and follows Drizzle ORM patterns as per coding guidelines.packages/trpc/src/lib/posthog-client.ts (1)
5-6: LGTM!The refactor from hardcoded configuration to environment-based values improves flexibility and follows best practices. The changes correctly use the newly added environment variables (
NODE_ENV,KV_REST_API_URL,KV_REST_API_TOKEN,POSTHOG_API_HOST) for runtime configuration.Also applies to: 163-163
apps/web/src/app/(dashboard)/integrations/linear/components/ErrorHandler/index.ts (1)
1-1: LGTM!Clean barrel export following the project's component organization pattern.
packages/db/drizzle/meta/_journal.json (1)
25-53: LGTM!Migration journal entries are properly sequenced with valid timestamps and consistent formatting.
apps/api/src/lib/integrations/linear/utils.ts (1)
1-18: LGTM!Type-safe priority mapping with proper derivation from the DB schema. The mapping is consistent with the inverse
mapPriorityToLinearfunction in the TRPC utils.packages/db/drizzle/meta/0005_snapshot.json (1)
1-1178: LGTM!Auto-generated Drizzle snapshot with well-structured schema design. Notable design choices:
- Unique constraint on
(external_provider, external_id)prevents duplicate synced tasks- Proper cascade delete behaviors on foreign keys
- Appropriate indexes for query patterns
packages/trpc/src/router/integration/linear/utils.ts (2)
10-25: LGTM!Priority mapping is consistent with the inverse
mapLinearPriorityfunction. The switch exhaustively handles all cases with a sensible default.
27-42: LGTM!Clean implementation using Drizzle ORM per coding guidelines. Returns
nullfor missing connections, allowing callers to handle the case appropriately.apps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/ConnectionControls.tsx (1)
26-84: LGTM!Well-structured component with proper separation of connected/disconnected states, confirmation dialog for destructive action, and appropriate loading state handling during the disconnect mutation.
packages/trpc/src/router/integration/linear/index.ts (1)
1-1: LGTM!Clean barrel export following standard module organization patterns.
apps/web/src/app/(dashboard)/integrations/linear/components/ConnectionControls/index.ts (1)
1-1: LGTM!Follows the co-location pattern as per coding guidelines.
packages/db/src/schema/enums.ts (1)
15-24: LGTM!The new
taskPriorityValuesenum follows the established pattern fromtaskStatusEnumValuesand correctly uses Zod for schema validation and type inference. The priority values align with Linear's priority model for bidirectional sync.packages/trpc/src/router/integration/integration.ts (1)
36-47: Good security practice: sensitive fields excluded.The query correctly excludes
accessToken,refreshToken, andtokenExpiresAtfrom the response, preventing credential leakage to clients.apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (1)
63-165: Good error handling pattern with syncError recording.The
syncTaskToLinearfunction properly records sync errors to the database and clears them on success, enabling visibility into sync failures and supporting retry/debugging workflows.apps/web/src/app/(dashboard)/integrations/linear/page.tsx (1)
34-89: LGTM!Clean server component implementation with proper separation of concerns. The UI correctly reflects connection state with appropriate visual feedback (badges), and the
ConnectionControlscomponent handles the interactive connect/disconnect flow client-side.packages/db/drizzle/0005_remove_integration_sync_webhook_fields.sql (1)
1-3: No code references the dropped columns — migration is safe to proceed.Verification confirms that
webhook_id,webhook_secret, andsync_enabledare not referenced anywhere in the application code. The migration can safely remove these columns from theintegration_connectionstable.apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (2)
52-54: Return appropriate status code when connection is missing.Returning a 200 status with
skipped: truemay cause QStash to consider the job successful and not retry. If this is intentional (job should be discarded when no connection exists), this is fine. Otherwise, consider returning a 4xx status.
62-72: LGTM!Good use of parallel fetching with
Promise.allfor teams and issues, and efficient batch queries for user/task lookups before processing.packages/trpc/src/router/integration/linear/linear.ts (2)
9-22: LGTM!The
getConnectionprocedure properly verifies organization membership before querying and efficiently returns only the existence check by selecting just theidcolumn.
24-44: LGTM!The
disconnectprocedure correctly requires admin access and uses.returning()to verify the deletion occurred, providing appropriate feedback when no connection exists.apps/api/src/app/api/integrations/linear/webhook/route.ts (1)
82-163: LGTM on the overall webhook processing flow.Good pattern of storing the webhook event first for audit/debugging purposes, then processing with proper status updates on success/failure. The retry tracking via
retryCountis also useful for monitoring.packages/db/src/schema/schema.ts (2)
123-194: LGTM on the tasks table schema expansion.Good schema design with:
- Flexible status fields (text + metadata) to accommodate external providers
- Proper indexing on commonly queried fields
- Unique constraint on
(externalProvider, externalId)to prevent duplicate synced tasks- Soft delete support via
deletedAt
199-239: LGTM on the integrationConnections table structure.The table design properly models the integration relationship with:
- Unique constraint on
(organizationId, provider)ensuring one connection per provider per org- Proper foreign key cascades for cleanup
- Flexible
configjsonb for provider-specific settingsAs per coding guidelines, this correctly uses Drizzle ORM for schema management.
| const parsed = stateSchema.safeParse( | ||
| JSON.parse(Buffer.from(state, "base64url").toString("utf-8")), | ||
| ); | ||
|
|
||
| if (!parsed.success) { | ||
| return Response.redirect( | ||
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Wrap JSON parsing in try-catch for malformed state.
If the state parameter is valid base64url but contains invalid JSON, JSON.parse will throw an unhandled exception, resulting in a 500 error instead of a graceful redirect to the error page.
🔎 Proposed fix
+ let decodedState: unknown;
+ try {
+ decodedState = JSON.parse(
+ Buffer.from(state, "base64url").toString("utf-8"),
+ );
+ } catch {
+ return Response.redirect(
+ `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state`,
+ );
+ }
+
const parsed = stateSchema.safeParse(
- JSON.parse(Buffer.from(state, "base64url").toString("utf-8")),
+ decodedState,
);📝 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 parsed = stateSchema.safeParse( | |
| JSON.parse(Buffer.from(state, "base64url").toString("utf-8")), | |
| ); | |
| if (!parsed.success) { | |
| return Response.redirect( | |
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state`, | |
| ); | |
| } | |
| let decodedState: unknown; | |
| try { | |
| decodedState = JSON.parse( | |
| Buffer.from(state, "base64url").toString("utf-8"), | |
| ); | |
| } catch { | |
| return Response.redirect( | |
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state`, | |
| ); | |
| } | |
| const parsed = stateSchema.safeParse( | |
| decodedState, | |
| ); | |
| if (!parsed.success) { | |
| return Response.redirect( | |
| `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/callback/route.ts around lines 33 to
41, the code calls JSON.parse directly on the decoded state which can throw for
malformed JSON; wrap the decode+JSON.parse in a try-catch, and on any exception
return the same Response.redirect to
`${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state` so
malformed JSON results in a graceful redirect instead of an unhandled 500; keep
the subsequent schema.safeParse usage (or run safeParse on the parsed object)
inside the try block and handle parse/schema failures with the existing
redirect.
| const parsed = payloadSchema.safeParse(JSON.parse(body)); | ||
| if (!parsed.success) { | ||
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | ||
| } |
There was a problem hiding this comment.
Wrap JSON.parse in try-catch to handle malformed payloads.
If body is not valid JSON, JSON.parse(body) will throw, bypassing the safeParse validation and returning a 500 error instead of a 400.
🔎 Proposed fix
- const parsed = payloadSchema.safeParse(JSON.parse(body));
- if (!parsed.success) {
- return Response.json({ error: "Invalid payload" }, { status: 400 });
- }
+ let jsonBody: unknown;
+ try {
+ jsonBody = JSON.parse(body);
+ } catch {
+ return Response.json({ error: "Invalid JSON" }, { status: 400 });
+ }
+
+ const parsed = payloadSchema.safeParse(jsonBody);
+ if (!parsed.success) {
+ return Response.json({ error: "Invalid payload" }, { status: 400 });
+ }📝 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 parsed = payloadSchema.safeParse(JSON.parse(body)); | |
| if (!parsed.success) { | |
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | |
| } | |
| let jsonBody: unknown; | |
| try { | |
| jsonBody = JSON.parse(body); | |
| } catch { | |
| return Response.json({ error: "Invalid JSON" }, { status: 400 }); | |
| } | |
| const parsed = payloadSchema.safeParse(jsonBody); | |
| if (!parsed.success) { | |
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts around lines
185 to 188, JSON.parse(body) is called directly and will throw on malformed JSON
causing a 500; wrap the parse in a try-catch, return a 400 JSON response when
parsing fails (e.g., { error: "Invalid JSON payload" }), and only call
payloadSchema.safeParse on the successfully parsed object so malformed JSON
returns 400 instead of an unhandled exception.
| if (!task) { | ||
| return Response.json({ error: "Task not found", skipped: true }); | ||
| } | ||
|
|
||
| const resolvedTeamId = | ||
| teamId ?? (await getDefaultTeamId(task.organizationId)); | ||
| if (!resolvedTeamId) { | ||
| return Response.json({ error: "No team configured", skipped: true }); | ||
| } |
There was a problem hiding this comment.
Add appropriate HTTP status codes for error responses.
Returning error responses without status codes defaults to 200 OK, which is misleading for clients expecting to detect failures.
🔎 Proposed fix
if (!task) {
- return Response.json({ error: "Task not found", skipped: true });
+ return Response.json({ error: "Task not found", skipped: true }, { status: 404 });
}
const resolvedTeamId = teamId ?? (await getDefaultTeamId(task.organizationId));
if (!resolvedTeamId) {
- return Response.json({ error: "No team configured", skipped: true });
+ return Response.json({ error: "No team configured", skipped: true }, { status: 400 });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!task) { | |
| return Response.json({ error: "Task not found", skipped: true }); | |
| } | |
| const resolvedTeamId = | |
| teamId ?? (await getDefaultTeamId(task.organizationId)); | |
| if (!resolvedTeamId) { | |
| return Response.json({ error: "No team configured", skipped: true }); | |
| } | |
| if (!task) { | |
| return Response.json({ error: "Task not found", skipped: true }, { status: 404 }); | |
| } | |
| const resolvedTeamId = | |
| teamId ?? (await getDefaultTeamId(task.organizationId)); | |
| if (!resolvedTeamId) { | |
| return Response.json({ error: "No team configured", skipped: true }, { status: 400 }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts around lines
196-204, the error JSON responses are returned without HTTP status codes
(defaulting to 200); change the "Task not found" response to return a 404 status
and change the "No team configured" response to return a 422 (or 400 if you
prefer) status by passing the appropriate status option to Response.json while
keeping the existing body (error message and skipped flag).
| const task = await db.query.tasks.findFirst({ | ||
| where: eq(tasks.id, taskId), | ||
| columns: { organizationId: true, externalProvider: true }, | ||
| }); | ||
|
|
||
| if (!task) { | ||
| throw new Error("Task not found"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's search for externalProvider usage in integration sync files
rg -nP --type=ts -C3 'externalProvider' packages/trpc/src/lib/integrations/Repository: superset-sh/superset
Length of output: 635
🏁 Script executed:
#!/bin/bash
# Let's also examine the full context of the tasks.ts file to see the complete function
wc -l packages/trpc/src/lib/integrations/sync/tasks.tsRepository: superset-sh/superset
Length of output: 115
🏁 Script executed:
#!/bin/bash
# View the file content to understand the full function context
cat -n packages/trpc/src/lib/integrations/sync/tasks.tsRepository: superset-sh/superset
Length of output: 1628
🏁 Script executed:
#!/bin/bash
# Search for externalProvider usage across the entire codebase
rg -nP 'externalProvider' --type=ts --type=tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for externalProvider usage across the entire codebase
rg -nP 'externalProvider' --type=tsRepository: superset-sh/superset
Length of output: 1314
Remove unused externalProvider field from task query.
The externalProvider field is selected at line 16 but never referenced within the syncTask function. While other parts of the codebase (webhook and sync-task endpoint handlers) do use this field, the dispatcher function here doesn't need it since it queues sync jobs to all providers without filtering by source provider.
🤖 Prompt for AI Agents
In packages/trpc/src/lib/integrations/sync/tasks.ts around lines 14 to 21, the
task query unnecessarily selects externalProvider which is not used in syncTask;
remove externalProvider from the columns selection so the query only requests
organizationId (or otherwise only the fields actually used), and adjust any
ensuing types if needed to reflect the reduced result shape.
| slug: z.string().min(1), | ||
| title: z.string().min(1), | ||
| description: z.string().optional(), | ||
| status: z.string().min(1).default("Backlog"), |
There was a problem hiding this comment.
Status default case mismatch with enum values.
The default "Backlog" uses PascalCase, but taskStatusEnumValues in packages/db/src/schema/enums.ts defines "backlog" in lowercase. This mismatch could cause validation or comparison issues downstream.
🔎 Proposed fix
- status: z.string().min(1).default("Backlog"),
+ status: z.string().min(1).default("backlog"),📝 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.
| status: z.string().min(1).default("Backlog"), | |
| status: z.string().min(1).default("backlog"), |
🤖 Prompt for AI Agents
In packages/trpc/src/router/task/schema.ts around line 8, the status default is
set to "Backlog" (PascalCase) which mismatches the taskStatusEnumValues
("backlog") defined in packages/db/src/schema/enums.ts; change the default to
the lowercase "backlog" to match the enum, or preferably import and use the enum
constant/value from packages/db/src/schema/enums.ts as the default so they
remain consistent.
- Add TeamSelector component to configure which Linear team new tasks sync to - Add `getTeams`, `updateConfig` tRPC routes - Merge `getConfig` into `getConnection` route - Use discriminated union pattern for integration config types - Create pgEnum for integration_provider column - Add migration to convert provider column to enum type - Use react-icons/si for Linear icon instead of local SVG 🤖 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: 3
♻️ Duplicate comments (3)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (2)
185-188: Wrap JSON.parse in try-catch to handle malformed payloads.This issue was previously identified. If
bodyis not valid JSON,JSON.parse(body)will throw, bypassing the safeParse validation and returning a 500 error instead of a 400.
196-204: Add appropriate HTTP status codes for error responses.This issue was previously identified. Returning error responses without status codes defaults to 200 OK, which is misleading for clients expecting to detect failures.
🔎 Proposed fix
if (!task) { - return Response.json({ error: "Task not found", skipped: true }); + return Response.json({ error: "Task not found", skipped: true }, { status: 404 }); } const resolvedTeamId = teamId ?? (await getNewTasksTeamId(task.organizationId)); if (!resolvedTeamId) { - return Response.json({ error: "No team configured", skipped: true }); + return Response.json({ error: "No team configured", skipped: true }, { status: 422 }); }packages/db/src/schema/schema.ts (1)
222-225: Access tokens stored in plain text.This issue was previously identified. The
accessToken(andrefreshToken) are stored as plain text. While the database should be secured, encrypting sensitive tokens at rest provides defense-in-depth against database breaches. Consider using application-level encryption.
🧹 Nitpick comments (6)
apps/web/src/app/(dashboard)/integrations/page.tsx (1)
1-1: Consider making this a server component.The page doesn't use client-side hooks or browser APIs. Since only
IntegrationCardrequires client-side interactivity (mouse events, state), you can remove the"use client"directive here and let Next.js automatically handle the client/server boundary, reducing the client-side JavaScript bundle.🔎 Suggested change
-"use client"; - import { FaGithub } from "react-icons/fa";apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (2)
34-39: Consider validating config structure at runtime.The type assertion
as LinearConfigtrusts that the JSONB data matches the expected shape. While low-risk for internal usage, adding a Zod schema for runtime validation would provide defense against corrupted data.🔎 Optional validation approach
import { z } from "zod"; const linearConfigSchema = z.object({ provider: z.literal("linear"), newTasksTeamId: z.string().optional(), }); // In getNewTasksTeamId: const parsed = linearConfigSchema.safeParse(connection.config); if (!parsed.success) { return null; } return parsed.data.newTasksTeamId ?? null;
42-61: Partial state matching may produce unexpected results.If multiple workflow states contain the search term (e.g., "In Progress" and "Progress Review" when searching for "progress"), the first match wins non-deterministically based on API response order. Consider logging when multiple partial matches exist or returning
undefinedto force explicit mapping.packages/db/drizzle/0003_set_up_linear_integration.sql (1)
1-65: Migration is well-structured and follows best practices.The migration correctly:
- Creates schema and types before dependent tables
- Adds foreign key constraints after tables exist
- Creates indexes for query optimization
- Uses appropriate cascade behavior for referential integrity
Consider adding a rollback script for production safety if the deployment process doesn't auto-generate one.
apps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsx (2)
14-16: Consider exporting TeamSelectorProps.While the interface works as-is, exporting it would allow consumers to type-check props and improve IDE autocomplete support.
🔎 Proposed change
-interface TeamSelectorProps { +export interface TeamSelectorProps { organizationId: string; }
65-65: Consider adding visual feedback during mutation.While the Select is disabled during the update, there's no visual indication that a save operation is in progress. Adding a loading indicator would improve user experience.
🔎 Suggested enhancement
Add a loading indicator near the Select:
+ {updateMutation.isPending && ( + <span className="text-xs text-muted-foreground ml-2"> + Saving... + </span> + )} <Select value={currentTeamId} onValueChange={handleChange} disabled={updateMutation.isPending} >Or use a Spinner component if available in
@superset/ui.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsxapps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/index.tsapps/web/src/app/(dashboard)/integrations/linear/page.tsxapps/web/src/app/(dashboard)/integrations/page.tsxpackages/db/drizzle/0003_set_up_linear_integration.sqlpackages/db/drizzle/meta/0003_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/enums.tspackages/db/src/schema/index.tspackages/db/src/schema/schema.tspackages/db/src/schema/types.tspackages/trpc/src/router/integration/linear/linear.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/db/src/schema/index.ts
- packages/trpc/src/router/integration/linear/linear.ts
- apps/web/src/app/(dashboard)/integrations/linear/page.tsx
- packages/db/src/schema/enums.ts
- packages/db/drizzle/meta/_journal.json
🧰 Additional context used
📓 Path-based instructions (8)
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/jobs/sync-task/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/jobs/sync-task/route.tsapps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsxpackages/db/src/schema/types.tspackages/db/src/schema/schema.tsapps/web/src/app/(dashboard)/integrations/page.tsx
**/*.{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/jobs/sync-task/route.tsapps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsxpackages/db/src/schema/types.tspackages/db/src/schema/schema.tsapps/web/src/app/(dashboard)/integrations/page.tsx
**/{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/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.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/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsxapps/web/src/app/(dashboard)/integrations/page.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/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsx
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Files:
packages/db/src/schema/types.tspackages/db/src/schema/schema.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:
packages/db/src/schema/types.tspackages/db/src/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 apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
packages/db/src/schema/schema.ts
🧬 Code graph analysis (2)
packages/db/src/schema/schema.ts (2)
packages/db/src/schema/enums.ts (3)
taskStatusEnumValues(3-12)taskPriorityValues(16-22)integrationProviderValues(26-26)packages/db/src/schema/types.ts (1)
IntegrationConfig(6-6)
apps/web/src/app/(dashboard)/integrations/page.tsx (1)
apps/web/src/app/(dashboard)/integrations/components/IntegrationCard/IntegrationCard.tsx (2)
IntegrationCardProps(7-15)IntegrationCard(17-106)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (11)
apps/web/src/app/(dashboard)/integrations/page.tsx (1)
30-47: LGTM!The component structure is clean with proper responsive grid layout, correct key usage, and type-safe prop spreading. The implementation follows React and TailwindCSS best practices.
packages/db/src/schema/types.ts (1)
1-6: Well-structured discriminated union pattern for integration configs.The
providerdiscriminant field enables type-safe handling when more integration types are added. When adding a new provider (e.g., GitHub), extendIntegrationConfigas a union:export type IntegrationConfig = LinearConfig | GitHubConfig;packages/db/drizzle/meta/0003_snapshot.json (1)
1-1192: Auto-generated Drizzle snapshot - schema state looks consistent.This snapshot correctly captures the database state after the Linear integration migration, including the new
integration_connectionstable, extendedtasksfields, and new enums. The foreign keys, indexes, and unique constraints align with the schema definitions.apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (2)
14-22: LGTM - Signature verification and payload schema are well-defined.The Receiver is correctly configured with both current and next signing keys for key rotation. The Zod schema provides appropriate validation for the job payload.
63-165: Sync logic is well-structured with proper error handling.The function correctly:
- Handles both update (existing Linear issue) and create paths
- Updates local task metadata on success
- Records sync errors for debugging
- Uses proper date formatting for Linear's due date field
packages/db/src/schema/schema.ts (3)
14-26: Enum definitions are well-structured and properly exported.The enums are correctly imported from the centralized
enums.tsfile and exposed as Postgres enums. This maintains consistency between TypeScript types and database constraints.
132-206: Tasks table schema is comprehensive and well-indexed.The expanded task model appropriately handles:
- Flexible status with metadata (color, type, position) for Linear compatibility
- External provider sync fields with composite unique constraint
- Optional repositoryId for Linear-only tasks
- Proper indexes for common query patterns
238-244: Index and unique constraint are appropriate for the access pattern.The composite unique constraint on
(organizationId, provider)correctly enforces one connection per provider per organization. The organization index supports efficient lookups.packages/db/drizzle/0003_set_up_linear_integration.sql (1)
34-36: Status and repository_id changes are consistent with application code, but validate existing data before deploying.The migration safely changes
statusfrom enum to text and makesrepository_idnullable. Application code already handlesrepositoryIdas optional (Zod schemas define it asz.string().uuid().optional(), and the db schema has no.notNull()). However, ensure existing task records have validstatusvalues before running this migration, as the column becomesNOT NULLwithout a default.apps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsx (2)
1-12: LGTM!The imports and client directive are appropriate for a React 19 client component using React Query and TRPC.
46-59: Loading and empty states look good.The loading skeleton and empty state message provide good user feedback. The data extraction with fallback to empty array is safe.
| const teamsQuery = useQuery( | ||
| trpc.integration.linear.getTeams.queryOptions({ organizationId }), | ||
| ); | ||
|
|
||
| const connectionQuery = useQuery( | ||
| trpc.integration.linear.getConnection.queryOptions({ organizationId }), | ||
| ); | ||
|
|
||
| const updateMutation = useMutation( | ||
| trpc.integration.linear.updateConfig.mutationOptions({ | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: trpc.integration.linear.getConnection.queryKey({ | ||
| organizationId, | ||
| }), | ||
| }); | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| const handleChange = (teamId: string) => { | ||
| updateMutation.mutate({ organizationId, newTasksTeamId: teamId }); | ||
| }; |
There was a problem hiding this comment.
Add error handling for queries and mutation.
The component doesn't handle error states from teamsQuery, connectionQuery, or updateMutation. If any operation fails, users will see indefinite loading or no feedback at all.
🔎 Proposed fix
Add error checks after the loading state check:
if (teamsQuery.isLoading || connectionQuery.isLoading) {
return <Skeleton className="h-9 w-48" />;
}
+
+ if (teamsQuery.isError || connectionQuery.isError) {
+ return (
+ <p className="text-sm text-destructive">
+ Failed to load teams. Please try again.
+ </p>
+ );
+ }
+
+ // Optionally show mutation errors
+ {updateMutation.isError && (
+ <p className="text-xs text-destructive mt-1">
+ Failed to update team selection.
+ </p>
+ )}Committable suggestion skipped: line range outside the PR's diff.
| return ( | ||
| <Select | ||
| value={currentTeamId} | ||
| onValueChange={handleChange} | ||
| disabled={updateMutation.isPending} | ||
| > | ||
| <SelectTrigger className="w-48"> | ||
| <SelectValue placeholder="Select a team" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {teams.map((team) => ( | ||
| <SelectItem key={team.id} value={team.id}> | ||
| {team.name} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the TeamSelector.tsx file to see the actual implementation
cat -n apps/web/src/app/\(dashboard\)/integrations/linear/components/TeamSelector/TeamSelector.tsxRepository: superset-sh/superset
Length of output: 2473
🏁 Script executed:
# Find the @superset/ui/select component
find . -path "*/ui/select*" -type f \( -name "*.ts" -o -name "*.tsx" \) 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 106
🏁 Script executed:
# Read the select component to understand how it handles value prop
cat -n ./packages/ui/src/components/ui/select.tsxRepository: superset-sh/superset
Length of output: 7362
🏁 Script executed:
# Search for other uses of Select component with value prop to see patterns
rg -A 2 'value=\{' --glob '**/select*.tsx' --glob '**/*Selector*.tsx' | grep -A 2 'Select' | head -40Repository: superset-sh/superset
Length of output: 1846
🏁 Script executed:
# Check BaseBranchSelector to see how it handles the value prop
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsx | head -100Repository: superset-sh/superset
Length of output: 2378
🏁 Script executed:
# Check Radix UI documentation or search for any issues with undefined Select values
# Let's look for TypeScript errors or type definitions
rg 'SelectPrimitive.Root' --type ts -A 3 -B 3 | head -30Repository: superset-sh/superset
Length of output: 566
🏁 Script executed:
# Search for any TypeScript or runtime issues with Select and undefined values
rg 'Select.*value.*undefined\|undefined.*Select' --type ts --type tsx
# Also check if there are any GitHub issues or comments about this
rg 'Select.*undefined\|undefined.*placeholder' --type ts --type tsx | head -20Repository: superset-sh/superset
Length of output: 119
🏁 Script executed:
# Let's check Radix UI Select types more directly
# Look for any type definitions or comments about value handling
rg 'value.*string|string.*value' packages/ui/src/components/ui/select.tsxRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there are any existing TypeScript errors or type issues with Select
# Look at package.json to see Radix UI version
rg '@radix-ui/react-select' package.jsonRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Find package files that mention radix-ui
find . -name "package*.json" -type f -exec grep -l '@radix-ui/react-select' {} \;Repository: superset-sh/superset
Length of output: 90
🏁 Script executed:
# Check Radix UI version and dependencies
cat ./packages/ui/package.json | grep -A 2 -B 2 '@radix-ui/react-select'Repository: superset-sh/superset
Length of output: 267
🏁 Script executed:
# Let's verify by looking at how Radix UI Select handles value types
# Check if there are any runtime errors or TypeScript type issues reported
rg 'value.*undefined' apps/ --type ts | grep -i select | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Let's check if there are any SelectItems with empty value
rg 'value=""' apps/web/src/app/\(dashboard\)/integrations --type ts --type tsx
# Also check the team structure to see if id can be empty
rg 'team.id' apps/web/src/app/\(dashboard\)/integrations --type ts --type tsx -B 2 -A 2 | head -30Repository: superset-sh/superset
Length of output: 119
🏁 Script executed:
# Check team structure and if id can be empty
rg 'team.id' apps/web/src/app/\(dashboard\)/integrations -B 2 -A 2 | head -30Repository: superset-sh/superset
Length of output: 644
Use a default value for the Select component's controlled value.
Line 63 passes value={currentTeamId} where currentTeamId can be undefined (line 51). Controlled Radix UI Select components should have a defined value; while the placeholder will display when undefined, it's not type-safe or idiomatic.
Use value={currentTeamId ?? ""} to ensure a consistent string value:
<Select
- value={currentTeamId}
+ value={currentTeamId ?? ""}
onValueChange={handleChange}
disabled={updateMutation.isPending}
>📝 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.
| return ( | |
| <Select | |
| value={currentTeamId} | |
| onValueChange={handleChange} | |
| disabled={updateMutation.isPending} | |
| > | |
| <SelectTrigger className="w-48"> | |
| <SelectValue placeholder="Select a team" /> | |
| </SelectTrigger> | |
| <SelectContent> | |
| {teams.map((team) => ( | |
| <SelectItem key={team.id} value={team.id}> | |
| {team.name} | |
| </SelectItem> | |
| ))} | |
| </SelectContent> | |
| </Select> | |
| ); | |
| return ( | |
| <Select | |
| value={currentTeamId ?? ""} | |
| onValueChange={handleChange} | |
| disabled={updateMutation.isPending} | |
| > | |
| <SelectTrigger className="w-48"> | |
| <SelectValue placeholder="Select a team" /> | |
| </SelectTrigger> | |
| <SelectContent> | |
| {teams.map((team) => ( | |
| <SelectItem key={team.id} value={team.id}> | |
| {team.name} | |
| </SelectItem> | |
| ))} | |
| </SelectContent> | |
| </Select> | |
| ); |
🤖 Prompt for AI Agents
In
apps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsx
around lines 61 to 78, the Select is controlled with value={currentTeamId} which
can be undefined; change the prop to pass a defined string by using the nullish
fallback (e.g. currentTeamId ?? "") so the Select always receives a string
value, keeping the placeholder behavior and satisfying type-safety.
| { | ||
| id: "github", | ||
| name: "GitHub", | ||
| description: "Connect repos and sync pull requests.", | ||
| category: "Version Control", | ||
| accentColor: "#FFFFFF", | ||
| disabled: true, | ||
| icon: <FaGithub className="size-8" />, | ||
| }, |
There was a problem hiding this comment.
GitHub accent color has poor contrast and incorrect branding.
The white accent color (#FFFFFF) will create nearly invisible borders and glow effects on light backgrounds. GitHub's official brand color is dark gray/black (e.g., #181717 or #24292e), which would provide better visual contrast and match their branding.
🔎 Suggested fix
{
id: "github",
name: "GitHub",
description: "Connect repos and sync pull requests.",
category: "Version Control",
- accentColor: "#FFFFFF",
+ accentColor: "#181717",
disabled: true,
icon: <FaGithub className="size-8" />,
},📝 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.
| { | |
| id: "github", | |
| name: "GitHub", | |
| description: "Connect repos and sync pull requests.", | |
| category: "Version Control", | |
| accentColor: "#FFFFFF", | |
| disabled: true, | |
| icon: <FaGithub className="size-8" />, | |
| }, | |
| { | |
| id: "github", | |
| name: "GitHub", | |
| description: "Connect repos and sync pull requests.", | |
| category: "Version Control", | |
| accentColor: "#181717", | |
| disabled: true, | |
| icon: <FaGithub className="size-8" />, | |
| }, |
🤖 Prompt for AI Agents
In apps/web/src/app/(dashboard)/integrations/page.tsx around lines 19 to 27, the
GitHub integration uses an inappropriate white accent color (#FFFFFF) which
lacks contrast and doesn't match GitHub branding; update the accentColor to
GitHub's official dark value (e.g., "#181717" or "#24292e") so borders/glows are
visible on light backgrounds and branding is correct.
- Update @linear/sdk to v68.1.0, @upstash/qstash to v2.8.4 - Use Linear SDK webhook types (LinearWebhookClient, parseData) - Refactor webhook handler to use upsert for create/update - Change tasks.externalProvider from text to enum - Add toast on team selection error, green checkmark on success - Update dashboard layout: stacked hero, bottom padding
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/db/src/schema/schema.ts (1)
222-225: Access tokens should be encrypted at rest.The
accessToken(and potentiallyrefreshToken) are stored as plain text, which presents a security risk if the database is compromised. While database security is important, encrypting sensitive tokens at rest provides defense-in-depth.This concern was raised in previous reviews and remains unaddressed.
As suggested in the previous review, consider implementing application-level encryption using AES-256-GCM with a key from environment variables before storing these tokens.
🧹 Nitpick comments (4)
apps/web/src/app/(dashboard)/layout.tsx (3)
21-27: Consider responsive width for sidebar on mobile.The fixed
w-80(320px) width may be too wide on smaller mobile screens (e.g., 375px phones), leaving minimal space for margins. Consider using a responsive width or making the sidebar full-width on mobile.🔎 Proposed fix for responsive sidebar width
- <aside className="w-80 shrink-0"> + <aside className="w-full shrink-0 md:w-80"> <div className="sticky top-24"> <h1 className="text-2xl font-medium leading-none"> {displayName} </h1> <SidebarNav /> </div> </aside>
19-19: Clarify magic number in min-height calculation.The
13remvalue inmin-h-[calc(100svh-13rem)]is unclear. Consider extracting this as a CSS variable or adding a comment explaining what it represents (presumably Header + Footer heights).
11-12: Consider adding try-catch for organization query to improve UX.While the app has a global error handler (
global-error.tsx) that will catch and log errors to Sentry, the TRPC query failure will result in an error page. For a better user experience, consider wrapping the query in a try-catch block to gracefully degrade with the fallback "Superset" display name instead.packages/db/src/schema/schema.ts (1)
87-87: Consider using an enum for role if custom roles aren't needed.The
rolefield is currently defined astext(), which provides flexibility for custom roles but sacrifices type safety. If the set of roles is fixed ("admin", "member"), consider using a pgEnum instead for stricter validation at the database level.🔎 Proposed enum-based approach
If custom roles aren't required, define a role enum:
+export const organizationRole = pgEnum("organization_role", ["admin", "member"]);Then update the field definition:
-role: text().notNull().default("member"), // "admin" | "member" | custom roles +role: organizationRole().notNull().default("member"),
📜 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 (10)
apps/api/package.jsonapps/api/src/app/api/integrations/linear/webhook/route.tsapps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsxapps/web/src/app/(dashboard)/layout.tsxapps/web/src/app/(dashboard)/page.tsxpackages/db/drizzle/0003_set_up_linear_integration.sqlpackages/db/drizzle/meta/0003_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/schema.tspackages/trpc/package.json
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/app/(dashboard)/page.tsx
- packages/trpc/package.json
- apps/api/src/app/api/integrations/linear/webhook/route.ts
- packages/db/drizzle/meta/_journal.json
- apps/api/package.json
- apps/web/src/app/(dashboard)/integrations/linear/components/TeamSelector/TeamSelector.tsx
- packages/db/drizzle/meta/0003_snapshot.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
packages/db/src/schema/schema.tsapps/web/src/app/(dashboard)/layout.tsx
**/*.{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:
packages/db/src/schema/schema.tsapps/web/src/app/(dashboard)/layout.tsx
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Files:
packages/db/src/schema/schema.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/web/src/app/(dashboard)/layout.tsx
🧠 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:
packages/db/src/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 apps/api/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations
Applied to files:
packages/db/src/schema/schema.ts
🧬 Code graph analysis (2)
packages/db/src/schema/schema.ts (2)
packages/db/src/schema/enums.ts (3)
taskStatusEnumValues(3-12)taskPriorityValues(16-22)integrationProviderValues(26-26)packages/db/src/schema/types.ts (1)
IntegrationConfig(6-6)
apps/web/src/app/(dashboard)/layout.tsx (3)
apps/web/src/app/(dashboard)/components/Header/Header.tsx (1)
Header(16-60)apps/web/src/app/(dashboard)/components/SidebarNav/SidebarNav.tsx (1)
SidebarNav(12-37)apps/web/src/app/(dashboard)/components/Footer/Footer.tsx (1)
Footer(3-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (14)
packages/db/src/schema/schema.ts (8)
14-26: LGTM!The enum definitions and imports follow Drizzle ORM best practices and maintain type safety. The new
taskPriorityandintegrationProviderenums are properly backed by const arrays imported from the enums module.
57-57: LGTM!The
clerkOrgIdfield and corresponding index are correctly defined. The nullable field with unique constraint appropriately handles the "null until synced to Clerk" scenario mentioned in the comment.Also applies to: 68-71
151-153: LGTM!Making
repositoryIdoptional with cascade delete correctly handles the requirement that Linear tasks won't have an associated repository. The comment clearly documents this design decision.
171-176: LGTM!The external sync fields are well-designed:
externalProviderandexternalIdcombination with unique constraint (lines 198-201) prevents duplicate sync issuesexternalKeyfor display identifiers like "SUPER-172"lastSyncedAtandsyncErrorfor monitoring sync health- All fields nullable for local-only tasks
192-192: LGTM!The indexes are well-chosen:
tasks_repository_id_idxfor foreign key lookupstasks_external_provider_idxfor filtering external tasks- Composite unique constraint
tasks_external_uniqueon(externalProvider, externalId)prevents duplicate synced tasksAlso applies to: 197-201
209-245: LGTM on table structure!The
integrationConnectionstable is well-designed:
- Proper foreign key constraints with cascade delete
- Unique constraint preventing duplicate provider connections per organization
- Appropriate indexes for queries
- Typed config field using
IntegrationConfigThe security concerns about token encryption are flagged separately.
230-230: Theconfigfield storingIntegrationConfigdoes not contain sensitive data. The type is defined asLinearConfigwith onlyprovider("linear") and an optionalnewTasksTeamIdmetadata field—no API keys, secrets, or authentication tokens are stored here.Likely an incorrect or invalid review comment.
162-164: No action needed — the labels field syntax is correct.The Drizzle ORM syntax
labels: jsonb().$type<string[]>().default([])correctly generates the SQLDEFAULT '[]'::jsonb, as confirmed by the migration file packages/db/drizzle/0003_set_up_linear_integration.sql. The default value is properly configured.Likely an incorrect or invalid review comment.
packages/db/drizzle/0003_set_up_linear_integration.sql (6)
18-32: LGTM!The
integration_connectionstable definition matches the TypeScript schema fromschema.ts. The inline unique constraint on(organization_id, provider)correctly prevents duplicate provider connections per organization.
37-55: LGTM on column additions!All new column additions are migration-safe:
- New columns are either nullable or have appropriate defaults
- The
labelsfield correctly usesDEFAULT '[]'::jsonbfor JSONB array default- The
priorityenum field with default 'none' is safe for existing rows- External sync and timestamp columns being nullable is appropriate
56-57: LGTM!The foreign key constraints with
ON DELETE CASCADEare appropriate:
- Deleting an organization should remove its integration connections
- Deleting the user who connected the integration should remove the connection
This matches the TypeScript schema definitions.
58-65: LGTM!All indexes and unique constraints are correctly defined and match the TypeScript schema:
- Composite indexes on
webhook_eventsoptimize common query patterns (filtering by provider/status, looking up by event_id)- The
tasks_external_uniqueconstraint on(external_provider, external_id)prevents duplicate synced tasks- Single-column indexes on foreign keys and lookup fields improve query performance
1-16: LGTM on ingest schema and webhook_events table.The separate
ingestschema provides good organizational separation for webhook event processing. Thewebhook_eventstable structure with status tracking, retry count, and error handling is well-designed for reliable webhook ingestion. The TypeScript schema definition exists inpackages/db/src/schema/ingest.tswith proper indexes and type exports.
34-36: No action required - status is already treated as text throughout the codebase.The migration correctly reflects the current implementation. The
statuscolumn is already defined astext()in the schema (line 141 of schema.ts) and validated as a string in application code (schema.ts usesz.string()), not as an enum. The unusedtaskStatuspgEnum andtaskStatusEnumZod definitions are dead code that don't affect the migration or application behavior. Existing enum values will be safely preserved as text.
| <div className="mx-auto min-h-[calc(100svh-13rem)] w-[95vw] max-w-screen-2xl pb-8 pt-16"> | ||
| <div className="flex flex-col gap-8 md:flex-row"> | ||
| <aside className="w-80 shrink-0"> | ||
| <div className="sticky top-24"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the layout file to see the sticky element and Header usage
cat -n apps/web/src/app/\(dashboard\)/layout.tsx | head -30Repository: superset-sh/superset
Length of output: 1186
🏁 Script executed:
# Find and examine the Header component
find apps/web -name "Header*" -type fRepository: superset-sh/superset
Length of output: 121
🏁 Script executed:
# Search for Header component styling
rg -n "className" apps/web/src/app/\(dashboard\)/components/Header/ -A 3Repository: superset-sh/superset
Length of output: 1957
Change top-24 to top-16 to align sticky positioning with Header height.
The Header's total height is 4rem (py-4 padding + min-h-8 content = 64px). The sticky element uses top-24 (6rem/96px), creating a 32px gap below the header and risking layout misalignment. Use top-16 to match the actual header height.
🤖 Prompt for AI Agents
In apps/web/src/app/(dashboard)/layout.tsx around line 22, the sticky container
uses class "top-24" which sets a 6rem offset leaving a 32px gap below the 4rem
(64px) Header; change the class to "top-16" so the sticky element's top offset
matches the Header height and removes the gap, i.e., replace "top-24" with
"top-16" on that div.
| status: text().notNull(), // Flexible text - stores any status name | ||
| statusColor: text("status_color"), | ||
| statusType: text("status_type"), | ||
| statusPosition: real("status_position"), | ||
| priority: taskPriority().notNull().default("none"), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the schema file around lines 141-145
head -n 160 packages/db/src/schema/schema.ts | tail -n 30Repository: superset-sh/superset
Length of output: 974
🏁 Script executed:
# Search for status validation in the codebase
rg -n -C 3 'status.*validation|validateStatus|isValidStatus' --type tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Also check for status-related constants or enums
rg -n 'status|Status' packages/db/src/schema/schema.ts -C 2 | head -n 50Repository: superset-sh/superset
Length of output: 1040
🏁 Script executed:
# Look for any validation directories or files
fd -e ts -e tsx 'validation|validator' packages/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the enums file to see what taskStatusEnumValues contains
cat packages/db/src/schema/enums.ts | head -n 50Repository: superset-sh/superset
Length of output: 797
🏁 Script executed:
# Search for validation in services or handlers
rg -n 'status' packages --type ts -g '*/services/*' -g '*/handlers/*' -g '*/validators/*' | head -n 30Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for Zod or valibot schemas that might validate status
rg -n 'z\.enum|v\.picklist.*status|status.*schema' --type ts | head -n 40Repository: superset-sh/superset
Length of output: 1976
🏁 Script executed:
# Look for any validation or type definitions for tasks
fd -e ts 'validation|validator|schema|type' packages/ | xargs grep -l 'status' 2>/dev/null | head -n 10Repository: superset-sh/superset
Length of output: 175
🏁 Script executed:
# Check the tRPC task schema to see if status is validated there
cat packages/trpc/src/router/task/schema.tsRepository: superset-sh/superset
Length of output: 1253
Status field changed from enum to text, but application-level validation not applied in tRPC schemas.
The status field is intentionally changed from enum to text().notNull() to support external integrations like Linear with varied status models. While a taskStatusEnum is defined in packages/db/src/schema/enums.ts with predefined values, it's not enforced in the API validation layer. The tRPC schemas (packages/trpc/src/router/task/schema.ts) use z.string().min(1) for createTaskSchema and z.string().optional() for updateTaskSchema, accepting any string value and bypassing the enum constraint.
Use taskStatusEnum in both tRPC schemas to enforce the predefined status values and maintain consistency between the defined enum and actual API validation.
🤖 Prompt for AI Agents
In packages/trpc/src/router/task/schema.ts (replace createTaskSchema &
updateTaskSchema), the API currently accepts arbitrary strings for status;
change createTaskSchema's status validator from z.string().min(1) to
z.nativeEnum(taskStatusEnum) and change updateTaskSchema's status from
z.string().optional() to z.nativeEnum(taskStatusEnum).optional(); add an import
for taskStatusEnum from packages/db/src/schema/enums.ts, update any related
types if needed, and run typechecks/build to ensure the enum is enforced at the
tRPC validation layer.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (2)
177-180: Wrap JSON.parse in try-catch to handle malformed payloads.The
JSON.parse(body)call will throw on malformed JSON, resulting in a 500 error instead of a 400.🔎 Proposed fix
- const parsed = payloadSchema.safeParse(JSON.parse(body)); - if (!parsed.success) { - return Response.json({ error: "Invalid payload" }, { status: 400 }); - } + let jsonBody: unknown; + try { + jsonBody = JSON.parse(body); + } catch { + return Response.json({ error: "Invalid JSON" }, { status: 400 }); + } + + const parsed = payloadSchema.safeParse(jsonBody); + if (!parsed.success) { + return Response.json({ error: "Invalid payload" }, { status: 400 }); + }
188-196: Add appropriate HTTP status codes for error responses.These error responses default to 200 OK, which is misleading for clients expecting to detect failures via status codes.
🔎 Proposed fix
if (!task) { - return Response.json({ error: "Task not found", skipped: true }); + return Response.json({ error: "Task not found", skipped: true }, { status: 404 }); } const resolvedTeamId = teamId ?? (await getNewTasksTeamId(task.organizationId)); if (!resolvedTeamId) { - return Response.json({ error: "No team configured", skipped: true }); + return Response.json({ error: "No team configured", skipped: true }, { status: 400 }); }apps/api/src/app/api/integrations/linear/callback/route.ts (1)
33-41: Wrap JSON.parse in try-catch for malformed state.If the
stateparameter is valid base64url but contains invalid JSON,JSON.parsewill throw an unhandled exception, resulting in a 500 error instead of a graceful redirect.🔎 Proposed fix
+ let decodedState: unknown; + try { + decodedState = JSON.parse( + Buffer.from(state, "base64url").toString("utf-8"), + ); + } catch { + return Response.redirect( + `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=invalid_state`, + ); + } + const parsed = stateSchema.safeParse( - JSON.parse(Buffer.from(state, "base64url").toString("utf-8")), + decodedState, );
🧹 Nitpick comments (5)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (1)
34-39: Consider validating config shape before type assertion.The type assertion
as LinearConfigassumes the stored config matches the expected shape. If the config was stored incorrectly or the schema evolved, this could cause runtime errors when accessingconfig.newTasksTeamId.🔎 Proposed fix using Zod validation
+const linearConfigSchema = z.object({ + provider: z.literal("linear"), + newTasksTeamId: z.string().optional(), +}); + async function getNewTasksTeamId( organizationId: string, ): Promise<string | null> { const connection = await db.query.integrationConnections.findFirst({ where: and( eq(integrationConnections.organizationId, organizationId), eq(integrationConnections.provider, "linear"), ), }); if (!connection?.config) { return null; } - const config = connection.config as LinearConfig; - return config.newTasksTeamId ?? null; + const parsed = linearConfigSchema.safeParse(connection.config); + if (!parsed.success) { + return null; + } + return parsed.data.newTasksTeamId ?? null; }apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (3)
74-96: Initial sync is limited to 100 issues per team - consider pagination for large workspaces.The
first: 100limit means organizations with more than 100 active issues per team will have an incomplete initial sync. Consider documenting this limitation or implementing pagination.🔎 Example pagination approach
const allIssueData = ( await Promise.all( teams.map(async (team) => { - const issuesResponse = await team.issues({ - first: 100, - filter: { state: { type: { nin: ["canceled", "completed"] } } }, - }); + const allIssues = []; + let hasMore = true; + let cursor: string | undefined; + + while (hasMore) { + const issuesResponse = await team.issues({ + first: 100, + after: cursor, + filter: { state: { type: { nin: ["canceled", "completed"] } } }, + }); + allIssues.push(...issuesResponse.nodes); + hasMore = issuesResponse.pageInfo.hasNextPage; + cursor = issuesResponse.pageInfo.endCursor; + } const issueData = await Promise.all( - issuesResponse.nodes.map(async (issue) => { + allIssues.map(async (issue) => { // ... rest of mapping }), );
115-152: Sequential upserts may be slow for large issue counts.Processing issues one-by-one in a loop could be slow for organizations with many issues. Consider batching inserts.
Note: This is a tradeoff - batching is faster but the current sequential approach is simpler and provides clearer error isolation. For an initial implementation, this is acceptable.
56-59: Consider handling performInitialSync errors.If
performInitialSyncthrows, the endpoint returns a 500 without context. The caller (QStash) will retry, but capturing and logging the error would aid debugging.🔎 Proposed fix
const client = new LinearClient({ accessToken: connection.accessToken }); - await performInitialSync(client, organizationId, creatorUserId); + try { + await performInitialSync(client, organizationId, creatorUserId); + } catch (error) { + console.error("Initial sync failed:", error); + return Response.json( + { error: error instanceof Error ? error.message : "Sync failed" }, + { status: 500 } + ); + } return Response.json({ success: true });apps/api/src/app/api/integrations/linear/callback/route.ts (1)
63-64: Consider validating token response JSON structure.The
tokenResponse.json()call could throw on non-JSON responses, and the type assertion assumes the response shape matches expectations. Consider adding validation.🔎 Proposed fix with Zod validation
+const tokenResponseSchema = z.object({ + access_token: z.string(), + expires_in: z.number().optional(), +}); - const tokenData: { access_token: string; expires_in?: number } = - await tokenResponse.json(); + let tokenJson: unknown; + try { + tokenJson = await tokenResponse.json(); + } catch { + return Response.redirect( + `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=token_exchange_failed`, + ); + } + + const tokenParsed = tokenResponseSchema.safeParse(tokenJson); + if (!tokenParsed.success) { + return Response.redirect( + `${env.NEXT_PUBLIC_WEB_URL}/integrations/linear?error=token_exchange_failed`, + ); + } + const tokenData = tokenParsed.data;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/src/app/api/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/connect/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.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/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/connect/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.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/connect/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/integrations/linear/callback/route.tsapps/api/src/app/api/integrations/linear/jobs/sync-task/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/connect/route.ts
🧬 Code graph analysis (4)
apps/api/src/app/api/integrations/linear/callback/route.ts (2)
apps/api/src/env.ts (1)
env(4-41)packages/db/src/schema/schema.ts (1)
integrationConnections(209-245)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (3)
packages/db/src/schema/schema.ts (3)
integrationConnections(209-245)SelectTask(206-206)tasks(132-203)packages/db/src/schema/types.ts (1)
LinearConfig(1-4)packages/trpc/src/lib/integrations/linear/index.ts (2)
getLinearClient(2-2)mapPriorityToLinear(3-3)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (4)
packages/trpc/src/env.ts (1)
env(4-24)apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (1)
POST(159-209)packages/db/src/schema/schema.ts (3)
integrationConnections(209-245)users(28-48)tasks(132-203)apps/api/src/lib/integrations/linear/utils.ts (1)
mapLinearPriority(5-18)
apps/api/src/app/api/integrations/linear/connect/route.ts (3)
packages/db/src/schema/schema.ts (2)
users(28-48)organizationMembers(77-98)apps/api/src/env.ts (1)
env(4-41)packages/db/src/env.ts (1)
env(9-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (8)
apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts (3)
55-107: LGTM - Update path handles sync correctly.The update flow properly:
- Checks for existing Linear issue
- Updates via the Linear SDK
- Records
lastSyncedAtand clearssyncErroron success- Returns external identifiers for the caller
109-145: LGTM - Create path with proper DB persistence.The create flow correctly:
- Creates a new Linear issue with mapped fields
- Updates the task with external provider metadata
- Sets
lastSyncedAtand clearssyncError
146-156: Good error handling - persists sync errors for visibility.Catching errors and storing them in
syncErrorallows debugging failed syncs without losing error context.apps/api/src/app/api/integrations/linear/connect/route.ts (2)
7-44: LGTM - Proper authentication and authorization checks.The route correctly:
- Authenticates via Clerk
- Validates the organizationId parameter
- Verifies user exists in the database
- Confirms organization membership before proceeding
This prevents unauthorized users from initiating OAuth flows for organizations they don't belong to.
46-60: LGTM - OAuth flow initiation is well-structured.The state payload encoding and OAuth URL construction follow best practices:
- State contains necessary context (organizationId, userId) for the callback
- Uses base64url encoding for URL-safe transmission
- OAuth parameters are properly set
apps/api/src/app/api/integrations/linear/callback/route.ts (3)
52-52: Hardcoded URL issue resolved.The
redirect_urinow correctly usesenv.NEXT_PUBLIC_API_URLinstead of the hardcoded ngrok URL from the previous review.
76-100: LGTM - Integration connection upsert is well-structured.The upsert correctly:
- Inserts new connections with all required fields
- Updates existing connections on conflict (org + provider)
- Preserves the user who last connected
102-106: LGTM - Initial sync job is queued appropriately.Publishing the initial-sync job via QStash with retries ensures the sync will complete even if the first attempt fails.
| const parsed = payloadSchema.safeParse(JSON.parse(body)); | ||
| if (!parsed.success) { | ||
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | ||
| } |
There was a problem hiding this comment.
Wrap JSON.parse in try-catch to handle malformed payloads.
Same issue as in sync-task route - JSON.parse(body) will throw on malformed JSON, resulting in a 500 instead of a 400.
🔎 Proposed fix
- const parsed = payloadSchema.safeParse(JSON.parse(body));
- if (!parsed.success) {
- return Response.json({ error: "Invalid payload" }, { status: 400 });
- }
+ let jsonBody: unknown;
+ try {
+ jsonBody = JSON.parse(body);
+ } catch {
+ return Response.json({ error: "Invalid JSON" }, { status: 400 });
+ }
+
+ const parsed = payloadSchema.safeParse(jsonBody);
+ if (!parsed.success) {
+ return Response.json({ error: "Invalid payload" }, { status: 400 });
+ }📝 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 parsed = payloadSchema.safeParse(JSON.parse(body)); | |
| if (!parsed.success) { | |
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | |
| } | |
| let jsonBody: unknown; | |
| try { | |
| jsonBody = JSON.parse(body); | |
| } catch { | |
| return Response.json({ error: "Invalid JSON" }, { status: 400 }); | |
| } | |
| const parsed = payloadSchema.safeParse(jsonBody); | |
| if (!parsed.success) { | |
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts around
lines 38 to 41, JSON.parse(body) can throw on malformed JSON causing a 500; wrap
the JSON.parse call in a try-catch and if parsing fails return Response.json({
error: "Invalid JSON payload" }, { status: 400 }); then continue to call
payloadSchema.safeParse on the parsed object and return the existing 400
response for schema validation failures.
| if (!connection) { | ||
| return Response.json({ error: "No connection found", skipped: true }); | ||
| } |
There was a problem hiding this comment.
Add status code for "No connection found" response.
The error response defaults to 200 OK, which is misleading.
🔎 Proposed fix
if (!connection) {
- return Response.json({ error: "No connection found", skipped: true });
+ return Response.json({ error: "No connection found", skipped: true }, { status: 404 });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!connection) { | |
| return Response.json({ error: "No connection found", skipped: true }); | |
| } | |
| if (!connection) { | |
| return Response.json({ error: "No connection found", skipped: true }, { status: 404 }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts around
lines 52 to 54, the handler returns a JSON error when no connection is found but
doesn't set an HTTP status code (defaults to 200). Update the return to include
an appropriate status (e.g., 404) by returning Response.json({ error: "No
connection found", skipped: true }, { status: 404 }) so clients receive a proper
Not Found status.
…inserts - Use raw GraphQL query to fetch issues with all relations in one request - Batch database inserts with lodash.chunk to prevent DB overload - Colocate LinearIssue types and fetchAllIssues with initial-sync - Add buildConflictUpdateColumns helper to @superset/db package 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename mapLinearPriority to mapPriorityFromLinear for consistency - Move mapPriorityFromLinear to @superset/trpc/integrations/linear - Remove duplicate utils.ts in API routes 🤖 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 (2)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (2)
41-44: WrapJSON.parsein try-catch to handle malformed payloads.
JSON.parse(body)will throw on malformed JSON, resulting in a 500 instead of a 400.🔎 Proposed fix
- const parsed = payloadSchema.safeParse(JSON.parse(body)); - if (!parsed.success) { - return Response.json({ error: "Invalid payload" }, { status: 400 }); - } + let jsonBody: unknown; + try { + jsonBody = JSON.parse(body); + } catch { + return Response.json({ error: "Invalid JSON" }, { status: 400 }); + } + + const parsed = payloadSchema.safeParse(jsonBody); + if (!parsed.success) { + return Response.json({ error: "Invalid payload" }, { status: 400 }); + }
55-57: Add status code for "No connection found" response.The error response defaults to 200 OK, which is misleading for an error condition.
🔎 Proposed fix
if (!connection) { - return Response.json({ error: "No connection found", skipped: true }); + return Response.json({ error: "No connection found", skipped: true }, { status: 404 }); }
🧹 Nitpick comments (5)
packages/db/src/utils/sql.ts (1)
10-13: Potential runtime issue: silent production of invalid SQL if column is undefined.If
cls[column as string]returnsundefined(e.g., runtime type bypass or schema mismatch),col?.nameevaluates toundefined, producing invalid SQL likeexcluded.undefined. Consider failing fast instead of silently generating broken SQL.🔎 Proposed fix with defensive check
return columns.reduce( (acc, column) => { const col = cls[column as string]; + if (!col) { + throw new Error(`Column "${String(column)}" not found in table`); + } - acc[column] = sql.raw(`excluded.${col?.name}`); + acc[column] = sql.raw(`excluded.${col.name}`); return acc; }, {} as Record<Q, SQL>, );package.json (1)
48-50: Consider movinglodash.chunkto the consuming package.Adding a runtime dependency at the monorepo root level is unusual. Since
lodash.chunkis used inapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts, consider moving this dependency toapps/api/package.jsoninstead for clearer ownership and to follow monorepo best practices.🔎 Suggested change
In
apps/api/package.json, add:"dependencies": { "lodash.chunk": "^4.2.0" }And remove the
dependenciesblock from the rootpackage.json.apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts (2)
66-89: Consider adding error handling for API failures.The pagination loop doesn't handle potential API errors. If the Linear API returns an error or rate-limits the request, the function will throw an unhandled exception.
🔎 Proposed improvement
export async function fetchAllIssues( client: LinearClient, ): Promise<LinearIssue[]> { const allIssues: LinearIssue[] = []; let cursor: string | undefined; do { - const response = await client.client.request< - IssuesQueryResponse, - { first: number; after?: string; filter: object } - >(ISSUES_QUERY, { - first: 100, - after: cursor, - filter: { state: { type: { nin: ["canceled", "completed"] } } }, - }); + let response: IssuesQueryResponse; + try { + response = await client.client.request< + IssuesQueryResponse, + { first: number; after?: string; filter: object } + >(ISSUES_QUERY, { + first: 100, + after: cursor, + filter: { state: { type: { nin: ["canceled", "completed"] } } }, + }); + } catch (error) { + console.error("Failed to fetch issues from Linear:", error); + throw new Error(`Linear API request failed: ${error instanceof Error ? error.message : "Unknown error"}`); + } allIssues.push(...response.issues.nodes); cursor = response.issues.pageInfo.hasNextPage && response.issues.pageInfo.endCursor ? response.issues.pageInfo.endCursor : undefined; } while (cursor); return allIssues; }
91-123: Consider adding an explicit return type annotation.The
mapIssueToTaskfunction returns an object literal without an explicit type. Adding a return type would improve maintainability and catch schema mismatches at compile time.🔎 Proposed improvement
+import type { InsertTask } from "@superset/db/schema"; + +type NewTaskFromLinear = Omit<InsertTask, "id" | "createdAt" | "updatedAt">; + export function mapIssueToTask( issue: LinearIssue, organizationId: string, creatorId: string, userByEmail: Map<string, string>, -) { +): NewTaskFromLinear {apps/api/src/app/api/integrations/linear/webhook/route.ts (1)
60-66: Consider logging unhandled event types.Only
Issueevents are processed; other event types silently succeed. Adding a log statement would help debug missing sync scenarios.🔎 Proposed improvement
try { if (payload.type === "Issue") { await processIssueEvent( payload as EntityWebhookPayloadWithIssueData, connection, ); + } else { + console.log(`Skipping unhandled webhook event type: ${payload.type}`); }
📜 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 (6)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.tspackage.jsonpackages/db/src/utils/sql.ts
🧰 Additional context used
📓 Path-based instructions (4)
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/utils.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/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/utils.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.tspackages/db/src/utils/sql.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/utils.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/route.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.tspackages/db/src/utils/sql.ts
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Store database schema in packages/db/src/ and use Drizzle ORM for schema management
Files:
packages/db/src/utils/sql.ts
🧠 Learnings (1)
📚 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/db/src/utils/sql.ts
🧬 Code graph analysis (4)
apps/api/src/app/api/integrations/linear/utils.ts (1)
packages/db/src/schema/schema.ts (1)
SelectTask(206-206)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (4)
packages/trpc/src/env.ts (1)
env(4-24)packages/db/src/schema/schema.ts (2)
users(28-48)tasks(132-203)apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts (2)
fetchAllIssues(66-89)mapIssueToTask(91-123)packages/db/src/utils/sql.ts (1)
buildConflictUpdateColumns(4-17)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts (1)
apps/api/src/app/api/integrations/linear/utils.ts (1)
mapLinearPriority(5-18)
apps/api/src/app/api/integrations/linear/webhook/route.ts (3)
packages/db/src/schema/ingest.ts (1)
webhookEvents(14-43)packages/db/src/schema/schema.ts (4)
integrationConnections(209-245)SelectIntegrationConnection(249-250)users(28-48)tasks(132-203)apps/api/src/app/api/integrations/linear/utils.ts (1)
mapLinearPriority(5-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (6)
packages/db/src/utils/sql.ts (1)
1-2: LGTM!Clean imports with proper use of
typefor type-only imports. Based on learnings, this is correctly located inpackages/db/src/for Drizzle ORM utilities.apps/api/src/app/api/integrations/linear/utils.ts (1)
1-18: LGTM!Clean implementation with proper type derivation from the database schema. The switch statement correctly maps Linear's priority values (1=urgent, 2=high, 3=medium, 4=low) with a sensible default of "none" for priority 0 or any unexpected values.
apps/api/src/app/api/integrations/linear/jobs/initial-sync/route.ts (1)
65-125: Solid batch insert implementation.The
performInitialSyncfunction correctly:
- Handles empty issues array early
- Deduplicates assignee emails efficiently with
Set- Uses batched inserts to avoid DB overload
- Leverages
buildConflictUpdateColumnsfor clean upsert logic- Resets
syncErroron successful syncapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts (1)
4-18: LGTM on the interface definition.The
LinearIssueinterface accurately models the GraphQL query response shape and includes all necessary fields for task synchronization.apps/api/src/app/api/integrations/linear/webhook/route.ts (2)
20-86: Well-structured webhook handler with proper event tracking.The handler correctly:
- Validates signature presence before processing
- Stores all webhook events for audit/debugging
- Updates event status on success/failure
- Includes retry count tracking for failed events
- Returns appropriate HTTP status codes
88-145: Solid issue event processing with proper upsert handling.The
processIssueEventfunction correctly:
- Handles create, update, and remove actions
- Resolves assignees by email lookup
- Uses
onConflictDoUpdatefor idempotent upserts- Soft-deletes on remove action
- Clears
syncErroron successful updates
| return Response.json({ error: "Missing signature" }, { status: 401 }); | ||
| } | ||
|
|
||
| const payload = webhookClient.parseData(Buffer.from(body), signature); |
There was a problem hiding this comment.
Wrap parseData in try-catch for invalid signatures.
webhookClient.parseData() throws when signature verification fails. This will result in an unhandled exception and 500 response instead of a proper 401.
🔎 Proposed fix
- const payload = webhookClient.parseData(Buffer.from(body), signature);
+ let payload: ReturnType<typeof webhookClient.parseData>;
+ try {
+ payload = webhookClient.parseData(Buffer.from(body), signature);
+ } catch {
+ return Response.json({ error: "Invalid signature" }, { status: 401 });
+ }🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/webhook/route.ts around line 28,
parseData is called without error handling so a bad/invalid signature will throw
and cause a 500; wrap webhookClient.parseData(Buffer.from(body), signature) in a
try-catch, catch the signature/verification error, log or record the error if
desired, and return an HTTP 401 (Unauthorized) response with an appropriate
message for invalid signatures; only proceed to process the payload when
parseData succeeds.
| assigneeId, | ||
| estimate: issue.estimate ?? null, | ||
| dueDate: issue.dueDate ? new Date(issue.dueDate) : null, | ||
| labels: issue.labels.map((l) => l.name), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the webhook route file
find . -name "route.ts" -path "*/integrations/linear/webhook/*" | head -5
# Check for EntityWebhookPayloadWithIssueData type
rg -n "EntityWebhookPayloadWithIssueData" --type ts -A 10
# Look for webhook payload type definitions
rg -n "webhook.*payload" --type ts -i | head -20
# Search for LinearIssue interface
rg -n "interface LinearIssue" --type ts -A 10Repository: superset-sh/superset
Length of output: 4491
🏁 Script executed:
#!/bin/bash
# Read the webhook route to understand the labels usage
cat -n apps/api/src/app/api/integrations/linear/webhook/route.ts | head -130 | tail -40Repository: superset-sh/superset
Length of output: 1545
🏁 Script executed:
#!/bin/bash
# Check initial-sync/utils.ts for LinearIssue interface
find . -name "utils.ts" -path "*/initial-sync/*" -type f
# Look for labels type definition in linear-related files
rg -n "labels.*nodes" --type ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 1299
🏁 Script executed:
#!/bin/bash
# Search for any type definitions or type stubs for @linear/sdk
find . -name "*.ts" -type f | xargs grep -l "@linear/sdk" | head -10
# Look for any comments or documentation about the webhook payload structure
rg -n "labels" apps/api/src/app/api/integrations/linear/webhook/route.ts -B 2 -A 2
# Check if there's a type error or any indication of the payload structure
ast-grep --pattern 'issue.labels' -A 2Repository: superset-sh/superset
Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Search for how issue is typed in the webhook handler
rg -n "const issue = payload.data" apps/api/src/app/api/integrations/linear/webhook/route.ts -A 50 | head -60Repository: superset-sh/superset
Length of output: 1795
🏁 Script executed:
#!/bin/bash
# Look for package.json to see the Linear SDK version
rg -n "@linear/sdk" --type json -A 2 | head -20
# Check if there are any type stubs or overrides
find . -name "*.d.ts" | xargs grep -l "EntityWebhookPayloadWithIssueData" 2>/dev/null
# Look for any error handling or runtime checks around labels
rg -n "labels" apps/api/src/app/api/integrations/linear/webhook/route.ts -B 5 -A 5Repository: superset-sh/superset
Length of output: 940
🏁 Script executed:
#!/bin/bash
# Compare the two usages side by side
echo "=== Webhook route (line 114) ==="
sed -n '110,120p' apps/api/src/app/api/integrations/linear/webhook/route.ts
echo ""
echo "=== Initial-sync utils (line 114) ==="
sed -n '110,120p' apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts
echo ""
echo "=== LinearIssue interface ==="
sed -n '4,20p' apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsRepository: superset-sh/superset
Length of output: 1547
🏁 Script executed:
#!/bin/bash
# Search for any other webhook/payload type definitions
rg -n "data: {" apps/api/src/app/api/integrations/linear/webhook/route.ts -B 10 | head -30Repository: superset-sh/superset
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Search for the issue type annotation in the webhook handler
sed -n '85,95p' apps/api/src/app/api/integrations/linear/webhook/route.ts
# Check the node_modules or type definitions if available
find . -path "*/node_modules/@linear/sdk/webhooks*" -type f 2>/dev/null | head -5
# Look for TypeScript errors or any validation
rg -n "as const" apps/api/src/app/api/integrations/linear/webhook/route.ts
# Check the full function signature
sed -n '88,92p' apps/api/src/app/api/integrations/linear/webhook/route.tsRepository: superset-sh/superset
Length of output: 529
🏁 Script executed:
#!/bin/bash
# Verify the two different label access patterns one more time
echo "Pattern 1 - Webhook (direct map on labels):"
grep -n "labels: issue.labels.map" apps/api/src/app/api/integrations/linear/webhook/route.ts
echo ""
echo "Pattern 2 - Initial-sync (map on labels.nodes):"
grep -n "labels: issue.labels.nodes.map" apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts
echo ""
echo "LinearIssue type definition:"
grep -n "labels:" apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsRepository: superset-sh/superset
Length of output: 400
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or examples for the webhook
find . -name "*webhook*" -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | head -5
# Look for any other files that use EntityWebhookPayloadWithIssueData
rg -n "EntityWebhookPayloadWithIssueData" --type ts
# Check if there are any runtime errors or type complaints in the codebase
rg -n "Object.is\|Array.isArray" apps/api/src/app/api/integrations/linear/webhook/route.ts
# Look for any optional chaining or defensive checks around labels
rg -n "labels\?" apps/api/src/app/api/integrations/linear/Repository: superset-sh/superset
Length of output: 419
🏁 Script executed:
#!/bin/bash
# Check the Linear SDK version and look for any type mismatch handling
npm list @linear/sdk 2>/dev/null || grep -r "@linear/sdk" packages/trpc/package.json apps/api/package.json
# Look for the actual structure in the Linear callback file which might give us hints
sed -n '1,50p' apps/api/src/app/api/integrations/linear/callback/route.ts | grep -A 10 -B 10 "labels" || echo "No labels reference in callback"
# Check the sync-task route
sed -n '1,100p' apps/api/src/app/api/integrations/linear/jobs/sync-task/route.ts | grep -A 5 -B 5 "labels" || echo "No labels reference in sync-task"Repository: superset-sh/superset
Length of output: 286
🏁 Script executed:
#!/bin/bash
# Look at the complete webhook route file to understand context
wc -l apps/api/src/app/api/integrations/linear/webhook/route.ts
# Check if there's any README or documentation about Linear integration
find . -path "*/linear*" -name "README*" -o -path "*/linear*" -name "*.md"
# Look for any tests that might show the actual payload structure
find . -path "**/linear/**" -name "*.test.ts" -o -name "*.spec.ts"
# Check if there's a differences in how payload.data is structured
rg -n "payload.data" apps/api/src/app/api/integrations/linear/ --type ts -A 2Repository: superset-sh/superset
Length of output: 405
🏁 Script executed:
#!/bin/bash
# Let's check if there's type safety being bypassed or if the types are different
# Look at the issue variable and how it's accessed in webhook
sed -n '85,125p' apps/api/src/app/api/integrations/linear/webhook/route.ts
# Compare with the LinearIssue interface
sed -n '4,30p' apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsRepository: superset-sh/superset
Length of output: 1997
🏁 Script executed:
#!/bin/bash
# Let's verify the type of issue.labels in the webhook context
# Check if there's any type override or casting
sed -n '85,95p' apps/api/src/app/api/integrations/linear/webhook/route.ts
# Search for any Linear SDK documentation or usage patterns online
# First, let's check if there's any indication in the codebase about webhook vs GraphQL differences
rg -n "webhook\|graphql" apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts -i
# Look at how the initial-sync fetches data (GraphQL) vs webhook
grep -n "query\|mutation" apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts | head -10Repository: superset-sh/superset
Length of output: 403
🏁 Script executed:
#!/bin/bash
# Check if issue variable could be typed differently
# Look for any type casting or as statements around issue
sed -n '88,100p' apps/api/src/app/api/integrations/linear/webhook/route.ts | grep -E "as |type |interface"
# More importantly, let's see if EntityWebhookPayloadWithIssueData is compatible with LinearIssue
# by checking if the webhook issue.data would have labels as array or object
python3 << 'PYTHON'
# The webhook route imports EntityWebhookPayloadWithIssueData from @linear/sdk/webhooks
# The issue is: payload.data is used directly without conversion
# The webhook assumes labels is an array
# But LinearIssue (for GraphQL) has labels as { nodes: [...] }
# These are likely TWO DIFFERENT structures:
# 1. Webhook payload from @linear/sdk - likely has labels as direct array
# 2. GraphQL response - has labels as { nodes: [...] }
# The code accesses them differently:
# Webhook: issue.labels.map()
# Initial-sync (GraphQL): issue.labels.nodes.map()
print("STRUCTURAL MISMATCH CONFIRMED:")
print("- Webhook (line 114): labels: issue.labels.map((l) => l.name)")
print("- Initial-sync (line 114): labels: issue.labels.nodes.map((l) => l.name)")
print("")
print("The webhook and GraphQL responses have DIFFERENT label structures.")
print("Webhook likely has: labels: Array<{ name: string, id: string, ... }>")
print("GraphQL has: labels: { nodes: Array<{ name: string, id: string }> }")
PYTHONRepository: superset-sh/superset
Length of output: 150
🌐 Web query:
Linear SDK EntityWebhookPayloadWithIssueData labels structure
💡 Result:
Short answer — types and reality
-
The SDK types declare EntityWebhookPayloadWithIssueData.data as an Issue (the GraphQL Issue model). In the GraphQL schema Issue.labels is a connection (IssueLabelConnection) with nodes of IssueLabel. [1][2]
-
In practice Linear webhook payloads often replace nested objects with id fields (e.g., team -> teamId). Webhook payloads therefore may not include full Label/Team objects; label references are typically represented as IDs (labelIds / labelId-like fields) or minimal label data rather than the full IssueLabel GraphQL object. See the SDK webhook typings and the GitHub discussion about webhooks returning id fields (teamId) instead of nested objects. [1][3]
Sources
- SDK webhook typings (EntityWebhookPayloadWithIssueData / Issue typing). [tessl/@linear/sdk docs]. [1]
- Linear Developers — GraphQL / SDK overview (Issue/labels are GraphQL fields / connections). [2]
- GitHub issue noting webhook payloads use id-fields (teamId) instead of nested objects. [3]
🏁 Script executed:
#!/bin/bash
# Let's check if there are any type definition files or JSDoc comments
# that might indicate the webhook payload structure
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "EntityWebhookPayloadWithIssueData" | head -5
# Check for any comments or documentation in the webhook route
head -30 apps/api/src/app/api/integrations/linear/webhook/route.ts
# Look for any error handling that might prevent runtime errors
sed -n '110,120p' apps/api/src/app/api/integrations/linear/webhook/route.tsRepository: superset-sh/superset
Length of output: 1525
Fix webhook labels access to match type definition structure.
Line 114 treats issue.labels as a direct array with .map((l) => l.name), but the type EntityWebhookPayloadWithIssueData defines labels as a GraphQL connection: { nodes: Array<{ id: string; name: string }> }. This will cause a runtime error. Align with the initial-sync implementation: change to issue.labels.nodes.map((l) => l.name).
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/webhook/route.ts around line 114,
the code treats issue.labels as an array and calls issue.labels.map(...), but
the type defines labels as a GraphQL connection { nodes: Array<{ id: string;
name: string }> }; update the access to use issue.labels.nodes.map((l) =>
l.name) (or guard for undefined nodes if necessary) so it matches the type and
avoids runtime errors, mirroring the initial-sync implementation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/api/src/app/api/integrations/linear/webhook/route.ts (2)
28-28: WrapparseDatain try-catch to handle signature verification failures.When
parseDataencounters an invalid signature, it throws an exception that will result in an unhandled 500 error instead of a proper 401 response.🔎 Proposed fix
- const payload = webhookClient.parseData(Buffer.from(body), signature); + let payload: EntityWebhookPayloadWithIssueData; + try { + payload = webhookClient.parseData(Buffer.from(body), signature); + } catch { + return Response.json({ error: "Invalid signature" }, { status: 401 }); + }
114-114: Fix labels access to use GraphQL connection structure.Line 114 accesses
issue.labels.map((l) => l.name)but theEntityWebhookPayloadWithIssueDatatype defines labels as a GraphQL connection:{ nodes: Array<{ id: string; name: string }> }. This will cause a runtime error when the webhook is invoked.🔎 Proposed fix
- labels: issue.labels.map((l) => l.name), + labels: issue.labels.nodes.map((l) => l.name),
🧹 Nitpick comments (1)
packages/trpc/src/router/integration/linear/utils.ts (1)
10-10: Consider centralizing the Priority type definition.The Priority type is defined locally but may be used across multiple files in the Linear integration. Consider moving it to a shared types file to avoid duplication and ensure consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.tspackages/trpc/src/lib/integrations/linear/index.tspackages/trpc/src/router/integration/linear/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trpc/src/lib/integrations/linear/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
packages/trpc/src/router/integration/linear/utils.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/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:
packages/trpc/src/router/integration/linear/utils.tsapps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.ts
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/jobs/initial-sync/utils.tsapps/api/src/app/api/integrations/linear/webhook/route.ts
🧬 Code graph analysis (1)
apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts (2)
packages/trpc/src/lib/integrations/linear/index.ts (1)
mapPriorityFromLinear(3-3)packages/trpc/src/router/integration/linear/utils.ts (1)
mapPriorityFromLinear(27-40)
🔇 Additional comments (14)
apps/api/src/app/api/integrations/linear/webhook/route.ts (5)
1-18: LGTM! Clean imports and initialization.The imports are well-organized and properly typed. The webhook client initialization correctly uses the environment variable for the secret.
30-43: Event storage logic is correct, type cast is acceptable for JSON field.The double type cast on line 36 (
payload as unknown as Record<string, unknown>) is necessary to store the Linear webhook payload in the JSON field of the database schema.
45-58: LGTM! Proper connection lookup with good error handling.The connection lookup correctly uses Drizzle ORM and appropriately handles missing connections by updating the webhook event status for audit purposes before returning 404.
60-86: LGTM! Robust event processing with proper error handling.The event processing flow correctly updates webhook event status (processed/failed) and includes retry tracking. Error handling properly captures error messages for debugging.
135-146: LGTM! Proper soft-delete implementation.The remove action correctly implements soft-delete using
deletedAttimestamp, which is appropriate for maintaining data history and audit trails.packages/trpc/src/router/integration/linear/utils.ts (5)
12-25: LGTM!The priority mapping logic is clear and correctly handles all priority levels.
27-40: LGTM!The inverse priority mapping correctly mirrors the to-Linear mapping.
59-83: LGTM!The membership verification logic correctly validates both user existence and organization membership.
85-99: LGTM!The admin verification correctly extends membership verification with role validation. Good code reuse.
42-57: The code is safe as written. TheaccessTokenfield in the database schema has an explicit.notNull()constraint (packages/db/src/schema/schema.ts line 223), guaranteeing it cannot be null. The function already safely handles the case where no connection exists by returningnullbefore attempting to instantiateLinearClient. No additional validation is needed.apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts (4)
1-2: LGTM! Imports align with refactoring objectives.The imports correctly use the relocated
mapPriorityFromLinearfunction from the TRPC package, as mentioned in the PR objectives.
4-18: LGTM! Interface structure is well-typed.The
LinearIssueinterface properly captures the shape of Linear issues with appropriate nullable types for optional fields.
20-25: LGTM! Standard GraphQL pagination response structure.
27-64: LGTM! GraphQL query structure aligns with the LinearIssue interface.
| export async function fetchAllIssues( | ||
| client: LinearClient, | ||
| ): Promise<LinearIssue[]> { | ||
| const allIssues: LinearIssue[] = []; | ||
| let cursor: string | undefined; | ||
|
|
||
| do { | ||
| const response = await client.client.request< | ||
| IssuesQueryResponse, | ||
| { first: number; after?: string; filter: object } | ||
| >(ISSUES_QUERY, { | ||
| first: 100, | ||
| after: cursor, | ||
| filter: { state: { type: { nin: ["canceled", "completed"] } } }, | ||
| }); | ||
| allIssues.push(...response.issues.nodes); | ||
| cursor = | ||
| response.issues.pageInfo.hasNextPage && response.issues.pageInfo.endCursor | ||
| ? response.issues.pageInfo.endCursor | ||
| : undefined; | ||
| } while (cursor); | ||
|
|
||
| return allIssues; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts | head -100Repository: superset-sh/superset
Length of output: 2994
🏁 Script executed:
# Check if there's a package.json to find Linear SDK version
fd package.json | head -5 | xargs grep -l "@linear/sdk" 2>/dev/null || echo "Checking alternative locations..."Repository: superset-sh/superset
Length of output: 85
🏁 Script executed:
# Find Linear SDK in dependencies
rg "@linear/sdk" --type json | head -20Repository: superset-sh/superset
Length of output: 168
🏁 Script executed:
# Check if Linear SDK types are installed in node_modules
fd -e d.ts | rg "linear" | head -20Repository: superset-sh/superset
Length of output: 46
🌐 Web query:
Linear SDK @linear/sdk 68.1.0 IssueFilter type TypeScript documentation
💡 Result:
The IssueFilter type (used to filter issues in the @linear/sdk TypeScript client) is the auto-generated GraphQL input with many per-field comparator filters and logical combinators. Key fields include (non-exhaustive):
- id, title, description, number, identifier, priority, estimate, url (string / numeric comparators)
- state, assignee, creator, team, project, cycle, labels (nested nullable/collection filters)
- createdAt, updatedAt, dueDate, completedAt, accumulatedStateUpdatedAt (Date/time comparators)
- archived, trashed, trashedAt, isDraft, isCanceled, snoozedUntil (nullable/boolean/date comparators)
- and, or (IssueFilter[] — combine sub-filters)
Example (shape) from generated typings:
{
id?: StringFilter;
title?: StringFilter;
description?: NullableStringFilter;
number?: IntFilter;
identifier?: StringFilter;
priority?: IntFilter;
estimate?: NullableIntFilter;
state?: NullableWorkflowStateFilter;
assignee?: NullableUserFilter;
team?: NullableTeamFilter;
project?: NullableProjectFilter;
cycle?: NullableCycleFilter;
labels?: IssueLabelCollectionFilter;
createdAt?: DateTimeFilter;
updatedAt?: DateTimeFilter;
dueDate?: NullableDateTimeFilter;
completedAt?: NullableDateTimeFilter;
accumulatedStateUpdatedAt?: DateTimeFilter;
archived?: NullableBooleanFilter;
trashed?: NullableBooleanFilter;
and?: IssueFilter[];
or?: IssueFilter[];
...
}
For the full, exact TypeScript definition for the @linear/sdk package (v68.1.0) refer to the generated SDK/type files or the SDK docs/source:
- Linear SDK docs / getting started (SDK repo & docs). [1]
- Example generated typings showing IssueFilter shape (registry / generated docs). [2]
References
[1] https://linear.app/developers/sdk
[2] Example generated typings / registry snapshot showing IssueFilter fields.
🏁 Script executed:
# Check what's imported from Linear SDK
rg "from.*@linear/sdk" apps/api/src/app/api/integrations/linear/ -A 3Repository: superset-sh/superset
Length of output: 2695
🏁 Script executed:
# Check if IssueFilter is used elsewhere
rg "IssueFilter" apps/api/src/Repository: superset-sh/superset
Length of output: 200
🏁 Script executed:
# Look for other Linear integration patterns with error handling
rg "client\.request" apps/api/src/app/api/integrations/linear/ -B 2 -A 5Repository: superset-sh/superset
Length of output: 792
Strengthen type safety and add error handling.
Two issues need attention:
-
Line 75: The
filterparameter is typed asobject, which is too generic and bypasses TypeScript's type checking. The Linear SDK provides a properIssueFiltertype (referenced in the GraphQL query on line 28) that should be used instead. -
Missing error handling: The async function has no try-catch block. If the Linear API request fails, the error will propagate unhandled, potentially causing the initial sync to fail silently.
Additionally, consider extracting the hardcoded page size 100 (line 77) as a named constant for maintainability.
🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts around
lines 66 to 89, the fetchAllIssues function uses a too-generic "filter: object"
type, lacks error handling, and hardcodes the page size; replace the filter type
with the appropriate IssueFilter type from the Linear SDK (import it if needed),
wrap the request loop in a try-catch that logs or wraps and rethrows the error
with context (e.g., "fetchAllIssues failed"), and extract the page size (100)
into a named constant (e.g., PAGE_SIZE) used in the request variables for
clarity and maintainability.
| export function mapIssueToTask( | ||
| issue: LinearIssue, | ||
| organizationId: string, | ||
| creatorId: string, | ||
| userByEmail: Map<string, string>, | ||
| ) { | ||
| const assigneeId = issue.assignee?.email | ||
| ? (userByEmail.get(issue.assignee.email) ?? null) | ||
| : null; | ||
|
|
||
| return { | ||
| organizationId, | ||
| creatorId, | ||
| slug: issue.identifier, | ||
| title: issue.title, | ||
| description: issue.description, | ||
| status: issue.state.name, | ||
| statusColor: issue.state.color, | ||
| statusType: issue.state.type, | ||
| priority: mapPriorityFromLinear(issue.priority), | ||
| assigneeId, | ||
| estimate: issue.estimate, | ||
| dueDate: issue.dueDate ? new Date(issue.dueDate) : null, | ||
| labels: issue.labels.nodes.map((l) => l.name), | ||
| startedAt: issue.startedAt ? new Date(issue.startedAt) : null, | ||
| completedAt: issue.completedAt ? new Date(issue.completedAt) : null, | ||
| externalProvider: "linear" as const, | ||
| externalId: issue.id, | ||
| externalKey: issue.identifier, | ||
| externalUrl: issue.url, | ||
| lastSyncedAt: new Date(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add date parsing validation to prevent Invalid Date objects.
Lines 113, 115–116 parse date strings using new Date() without validation. If Linear returns malformed date strings, this will create Invalid Date objects that could cause issues in downstream code or database inserts.
🔎 Suggested date validation
+function parseNullableDate(dateString: string | null): Date | null {
+ if (!dateString) return null;
+ const date = new Date(dateString);
+ if (Number.isNaN(date.getTime())) {
+ throw new Error(`Invalid date string: ${dateString}`);
+ }
+ return date;
+}
+
export function mapIssueToTask(
issue: LinearIssue,
organizationId: string,
creatorId: string,
userByEmail: Map<string, string>,
) {
const assigneeId = issue.assignee?.email
? (userByEmail.get(issue.assignee.email) ?? null)
: null;
return {
organizationId,
creatorId,
slug: issue.identifier,
title: issue.title,
description: issue.description,
status: issue.state.name,
statusColor: issue.state.color,
statusType: issue.state.type,
priority: mapPriorityFromLinear(issue.priority),
assigneeId,
estimate: issue.estimate,
- dueDate: issue.dueDate ? new Date(issue.dueDate) : null,
+ dueDate: parseNullableDate(issue.dueDate),
labels: issue.labels.nodes.map((l) => l.name),
- startedAt: issue.startedAt ? new Date(issue.startedAt) : null,
- completedAt: issue.completedAt ? new Date(issue.completedAt) : null,
+ startedAt: parseNullableDate(issue.startedAt),
+ completedAt: parseNullableDate(issue.completedAt),
externalProvider: "linear" as const,
externalId: issue.id,
externalKey: issue.identifier,
externalUrl: issue.url,
lastSyncedAt: new Date(),
};
}🤖 Prompt for AI Agents
In apps/api/src/app/api/integrations/linear/jobs/initial-sync/utils.ts around
lines 91 to 123, the code uses new Date(...) for dueDate, startedAt, and
completedAt without validating the parsed Date, which can produce Invalid Date
objects; add a small utility (e.g., parseDateSafe) that accepts a date string,
returns null for falsy inputs, constructs a Date, and returns the Date only if
!isNaN(date.getTime()), otherwise returns null, then replace the three new
Date(...) usages with calls to this helper so malformed Linear date strings
become null instead of Invalid Date.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change provider column from text to integration_provider enum - Make eventId not null - Replace regular index with unique index on (provider, eventId) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
/test/linearfor testing the integration (gated to @superset.sh users via PostHog feature flag)Changes
integrationConnectionstable, task status metadata fieldsintegrationrouter, auto-sync on task create/updateTest plan
/test/linear🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.