GitHub node - #79
Merged
Merged
Conversation
…d add auth infrastructure
… schema architecture
…ments, and triggers
Contributor
|
Warning Review limit reached
More reviews will be available in 6 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds modular Prisma schemas + migration, webhook-secret encryption and migration scripts, GitHub node & trigger types, API client and executors, UI components/dialogs, TRPC routers, webhook HTTP handler with HMAC verification and idempotency, and smaller auth/Slack/google-sheets adjustments. ChangesGitHub Integration Feature
Sequence Diagram(s)sequenceDiagram
participant Client as Browser
participant Dialog as GitHubDialog
participant TRPC as TRPC Router
participant DB as Prisma
participant Executor as GitHub Executor
participant GH as GitHub API
Client->>Dialog: Open config / getByNodeId
Dialog->>TRPC: getByNodeId(nodeId)
TRPC->>DB: Load GitHubNode
DB-->>TRPC: Node data
TRPC-->>Dialog: Prefill form
Client->>Dialog: Save -> upsert
Dialog->>TRPC: upsert(nodeId, workflowId, values)
TRPC->>DB: Upsert GitHubNode
DB-->>TRPC: OK
Note over Executor,GH: During execution
Executor->>DB: Load node & credential
DB-->>Executor: node + encrypted credential
Executor->>Executor: decrypt credential
Executor->>GH: API request via GitHubClient
GH-->>Executor: Response
Executor-->>Workflow: return result / publish status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e5fe19016
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,1795 @@ | |||
| -- CreateEnum | |||
| CREATE TYPE "NodeType" AS ENUM ('INITIAL', 'MANUAL_TRIGGER', 'HTTP_REQUEST', 'GOOGLE_FORM_TRIGGER', 'STRIPE_TRIGGER', 'WEBHOOK_TRIGGER', 'SCHEDULE_TRIGGER', 'ANTHROPIC', 'GEMINI', 'OPENAI', 'GROQ', 'XAI', 'DISCORD', 'SLACK', 'DEEPSEEK', 'PERPLEXITY', 'TELEGRAM', 'X', 'WORKDAY', 'IF_ELSE', 'GMAIL', 'SET_VARIABLE', 'GOOGLE_SHEETS', 'GOOGLE_DRIVE', 'CODE', 'WHATSAPP', 'LOOP', 'NOTION', 'RAZORPAY', 'SWITCH', 'WAIT', 'MERGE', 'ERROR_TRIGGER', 'RAZORPAY_TRIGGER', 'WHATSAPP_TRIGGER', 'MSG91', 'SHIPROCKET', 'ZOHO_CRM', 'HUBSPOT', 'FRESHDESK', 'MEDIA_UPLOAD', 'SORT', 'FILTER', 'CASHFREE', 'CASHFREE_TRIGGER', 'AGGREGATE', 'POSTGRES'); | |||
There was a problem hiding this comment.
Because prisma.config.ts now points Prisma at prisma/schema, this new prisma/schema/migrations history is what deploys will use, but the migration is a fresh baseline that starts by creating existing enums/tables instead of applying only the GitHub/webhook-secret deltas. On any database that already applied the old prisma/migrations history, this will fail immediately with existing types/tables; on a fresh database it also leaves the new GitHub feature unusable because this enum omits GITHUB/GITHUB_TRIGGER and the file contains no GitHubNode/GitHubTriggerNode table creation.
Useful? React with 👍 / 👎.
Comment on lines
+98
to
+100
| await sendWorkflowExecution({ | ||
| workflowId: webhookTrigger.workflowId, | ||
| inngestId: idempotencyKey, |
There was a problem hiding this comment.
The trigger dialog persists an events JSON array and tells users the workflow will only process those GitHub events, but this route sends every verified delivery to Inngest without checking webhookTrigger.events against x-github-event. For a repo webhook subscribed to multiple events, configuring ['push'] still executes the workflow for pull requests, issues, etc., so workflows run on events users explicitly tried to filter out.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
scripts/migrate-webhook-secrets.ts-38-43 (1)
38-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the migration’s UPDATE concurrency-safe (guard against stale legacy snapshots).
scripts/migrate-webhook-secrets.tsreads rows wheresecretTokenEncryptedis null andsecretTokenLegacynot null, then updates byidonly (settingsecretTokenEncryptedand clearingsecretTokenLegacy). In this repo, the only other Prisma write towebhookTriggerisprisma.webhookTrigger.create(...)for new rows; there are no other in-repo Prisma/executeRawupdates to these secret columns. The remaining realistic race is multiple concurrent executions of this migration overwritingsecretTokenEncryptedbased on a stalesecretTokenLegacysnapshot.🔧 Suggested fix
- await prisma.webhookTrigger.update({ - where: { id: trigger.id }, - data: { - secretTokenEncrypted: encrypted, - secretTokenLegacy: null, // clear legacy plaintext - }, - }) - migrated++ + const result = await prisma.webhookTrigger.updateMany({ + where: { + id: trigger.id, + secretTokenEncrypted: null, + secretTokenLegacy: { not: null }, + }, + data: { + secretTokenEncrypted: encrypted, + secretTokenLegacy: null, // clear legacy plaintext + }, + }) + if (result.count === 0) { + skipped++ + continue + } + migrated++🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/migrate-webhook-secrets.ts` around lines 38 - 43, The current prisma.webhookTrigger.update(...) is vulnerable to races; instead perform a conditional write using prisma.webhookTrigger.updateMany(...) with a where clause that includes the id plus the expected legacy snapshot (e.g. { id: trigger.id, secretTokenEncrypted: null, secretTokenLegacy: trigger.secretTokenLegacy }) so only a row matching the same legacy value is migrated, then check the returned count === 1 and handle count === 0 (skip/log) to detect concurrent/up-to-date cases; replace the update call with this conditional updateMany pattern in scripts/migrate-webhook-secrets.ts.prisma/schema/nodes_flow_control.prisma-30-41 (1)
30-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
SwitchNodeis missing@@index([workflowId]).All other node models in this file have both
@@index([workflowId])and@@index([nodeId]), butSwitchNodeonly has thenodeIdindex. This will degrade query performance when filtering switch nodes by workflow.Proposed fix
model SwitchNode { id String `@id` `@default`(cuid()) nodeId String `@unique` workflowId String variableName String `@default`("switch") casesJson String `@default`("[]") `@db.Text` createdAt DateTime `@default`(now()) updatedAt DateTime `@updatedAt` workflow workflow `@relation`(fields: [workflowId], references: [id], onDelete: Cascade) @@index([nodeId]) + @@index([workflowId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_flow_control.prisma` around lines 30 - 41, The SwitchNode Prisma model currently only defines @@index([nodeId]) which omits the workflowId index used by other node models; update the SwitchNode model to add @@index([workflowId]) (in addition to the existing @@index([nodeId])) so queries filtering by workflowId use an index and match the other node models; locate the SwitchNode block in the Prisma schema and add the @@index([workflowId]) directive alongside the existing @@index([nodeId]).prisma/schema/nodes_github.prisma-13-14 (1)
13-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
onDelete: SetNullon credential relation.Other node models (e.g.,
GmailNode,GoogleSheetsNode) specifyonDelete: SetNullfor the optional credential relation. This ensures the node remains intact if a credential is deleted. Without it, deleting a credential may cause referential integrity issues or unexpected behavior depending on database defaults.Proposed fix
credentialId String? - credential Credential? `@relation`(fields: [credentialId], references: [id]) + credential Credential? `@relation`(fields: [credentialId], references: [id], onDelete: SetNull)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_github.prisma` around lines 13 - 14, Add an explicit onDelete: SetNull to the optional credential relation so deleting a Credential won't break referential integrity; update the relation declaration that uses credentialId and credential (referencing Credential) to include onDelete: SetNull (mirror other node models like GmailNode/GoogleSheetsNode) so the node's credentialId is set to null when its Credential is removed.src/features/executions/components/github/executors/repository.ts-324-336 (1)
324-336:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame JSON.parse issue exists here for gist files.
Similar to the
clientPayloadissue above, theconfig.options.filesparsing at lines 325-326 and 351-352 should be wrapped in try/catch for better error messages when users provide invalid JSON.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/executions/components/github/executors/repository.ts` around lines 324 - 336, The JSON.parse of config.options.files in the GitHubOperation.GIST_CREATE branch can throw on invalid JSON; wrap the parsing logic for config.options.files (used to produce the files variable before calling client.request) in a try/catch, and on error throw or return a clear, contextual error that includes the invalid input (e.g., mention config.options.files and the operation GitHubOperation.GIST_CREATE) so callers see why parsing failed; ensure you still fall back to an empty object when options.files is undefined and keep using resolveTemplate(config.body || "", context) and client.request(...) unchanged aside from using the safely parsed files variable.src/features/executions/components/github/executors/repository.ts-133-143 (1)
133-143:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for JSON.parse on user-provided clientPayload.
If the resolved
clientPayloadis not valid JSON, this throws a cryptic error. Since this is user input, invalid JSON is a realistic scenario.Suggested fix
case GitHubOperation.REPOSITORY_DISPATCH: { const event_type = resolveTemplate(config.eventType || "", context); - const client_payload = config.clientPayload - ? JSON.parse(resolveTemplate(config.clientPayload, context)) - : {}; + let client_payload = {}; + if (config.clientPayload) { + try { + client_payload = JSON.parse(resolveTemplate(config.clientPayload, context)); + } catch { + throw new NonRetriableError("Invalid JSON in clientPayload field"); + } + } await client.request(`/repos/${owner}/${repo}/dispatches`, {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/executions/components/github/executors/repository.ts` around lines 133 - 143, The code handling GitHubOperation.REPOSITORY_DISPATCH must validate and gracefully handle parse errors for user-provided config.clientPayload: wrap the JSON.parse(resolveTemplate(config.clientPayload, context)) call (used to set client_payload) in a try/catch, and on failure return or throw a clear, actionable error (or set client_payload = {} and log via processLogger or the existing logger) instead of letting a cryptic exception bubble up; keep the rest of the request (client.request(`/repos/${owner}/${repo}/dispatches`, ...)) unchanged and ensure the error mentions the invalid clientPayload and the resolveTemplate/context so the caller can fix their input.src/features/triggers/components/github-trigger/dialog.tsx-75-78 (1)
75-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClipboard copy flow can show success on failure.
navigator.clipboard.writeTextis async and can reject; current code always toasts success.Proposed fix
- const handleCopy = (text: string, name: string) => { - navigator.clipboard.writeText(text) - toast.success(`${name} copied to clipboard`) - } + const handleCopy = async (text: string, name: string) => { + try { + await navigator.clipboard.writeText(text) + toast.success(`${name} copied to clipboard`) + } catch { + toast.error(`Failed to copy ${name}`) + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/triggers/components/github-trigger/dialog.tsx` around lines 75 - 78, handleCopy currently calls navigator.clipboard.writeText(text) without awaiting errors and always shows toast.success; change handleCopy to handle the Promise from navigator.clipboard.writeText (either async/await or .then/.catch) and only call toast.success(`${name} copied to clipboard`) after the write resolves, and call toast.error with a clear message (e.g., `${name} copy failed`) when the write rejects so failures are surfaced to the user.
🧹 Nitpick comments (10)
prisma/schema/nodes_utility.prisma (1)
95-130: ⚡ Quick winMissing database indexes on
SortNode.Other utility nodes (
FilterNode,AggregateNode,PostgresNode) include@@index([workflowId])and@@index([nodeId]), butSortNodeis missing these indexes.Proposed fix
workflow workflow `@relation`(fields: [workflowId], references: [id], onDelete: Cascade) createdAt DateTime `@default`(now()) updatedAt DateTime `@updatedAt` + + @@index([workflowId]) + @@index([nodeId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_utility.prisma` around lines 95 - 130, The SortNode model is missing database indexes for quick lookup; add the same indexes used by other utility nodes by declaring @@index([workflowId]) and @@index([nodeId]) on the SortNode model so queries filtering by workflowId or nodeId are efficient; update the SortNode definition (model SortNode) to include these two @@index declarations.prisma/schema/nodes_messaging.prisma (1)
54-73: ⚡ Quick winMissing database indexes on
WhatsAppNode.
SlackNode,NotionNode, andMsg91Nodeinclude@@index([workflowId])and@@index([nodeId]), butWhatsAppNodeis missing these indexes.Proposed fix
createdAt DateTime `@default`(now()) updatedAt DateTime `@updatedAt` workflow workflow `@relation`(fields: [workflowId], references: [id], onDelete: Cascade) + + @@index([workflowId]) + @@index([nodeId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_messaging.prisma` around lines 54 - 73, Add the missing database indexes to the WhatsAppNode Prisma model: add @@index([workflowId]) and @@index([nodeId]) to the WhatsAppNode model (e.g., append these index directives inside the WhatsAppNode model block, similar to SlackNode/NotionNode/Msg91Node) so queries filtering by workflowId or nodeId are indexed; ensure you reference the existing fields workflowId and nodeId in these index directives.prisma/schema/nodes_google.prisma (1)
89-105: ⚡ Quick winMissing database indexes on
GoogleDriveNode.
GmailNodeandGoogleSheetsNodeinclude@@index([workflowId])and@@index([nodeId]), butGoogleDriveNodeis missing these indexes.Proposed fix
createdAt DateTime `@default`(now()) updatedAt DateTime `@updatedAt` workflow workflow `@relation`(fields: [workflowId], references: [id], onDelete: Cascade) + + @@index([workflowId]) + @@index([nodeId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_google.prisma` around lines 89 - 105, The GoogleDriveNode Prisma model is missing the same database indexes present on GmailNode and GoogleSheetsNode; add index declarations to the GoogleDriveNode model by appending @@index([workflowId]) and @@index([nodeId]) to the model definition so queries filtering by workflowId or nodeId use indexed lookups; modify the GoogleDriveNode model (symbols: GoogleDriveNode, workflowId, nodeId) to include those @@index entries.prisma/schema/triggers.prisma (1)
129-144: ⚡ Quick winMissing
workflowIdandnodeIdindexes onGmailWatcher.Other trigger models include indexes on
workflowIdandnodeId.GmailWatcheronly has a composite index on[email, active]but is missing the workflow lookup index.Proposed fix
workflow workflow `@relation`(fields: [workflowId], references: [id], onDelete: Cascade) @@index([email, active]) + @@index([workflowId]) + @@index([nodeId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/triggers.prisma` around lines 129 - 144, Add the missing indexes to the GmailWatcher model: include an index on workflowId and an index on nodeId by adding @@index([workflowId]) and @@index([nodeId]) to the model (you can keep the existing nodeId uniqueness), ensuring GmailWatcher matches other trigger models' lookup indexes.prisma/schema/nodes_github.prisma (2)
59-75: ⚡ Quick winMissing database indexes on
GitHubTriggerNode.Add indexes on
workflowId,nodeIdfor consistency with other trigger/node models and query performance.Proposed fix
createdAt DateTime `@default`(now()) updatedAt DateTime `@default`(now()) `@updatedAt` + + @@index([workflowId]) + @@index([nodeId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_github.prisma` around lines 59 - 75, Add missing DB indexes to the GitHubTriggerNode model: create an index for workflowId and one for nodeId to match other trigger/node models and improve query performance. Locate the GitHubTriggerNode model and add a field-level or model-level index for workflowId (reference symbol: workflowId) and ensure nodeId (reference symbol: nodeId) also has an index (even if currently `@unique`) using Prisma's `@index` or @@index syntax so queries filtering by workflowId or nodeId are backed by DB indexes.
6-57: ⚡ Quick winMissing database indexes on
GitHubNode.Other node models include
@@index([workflowId])and@@index([nodeId])for query performance.GitHubNodeis missing these indexes.Proposed fix
createdAt DateTime `@default`(now()) updatedAt DateTime `@default`(now()) `@updatedAt` + + @@index([workflowId]) + @@index([nodeId]) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema/nodes_github.prisma` around lines 6 - 57, Add the missing Prisma indexes to the GitHubNode model by adding the model-level index directives for workflowId and nodeId (e.g., add @@index([workflowId]) and @@index([nodeId]) inside the GitHubNode model). Locate the GitHubNode model declaration and append these @@index directives (after the field list, before the closing brace) so queries filtering by workflowId or nodeId use database indexes.src/app/api/webhooks/github/[webhookId]/route.ts (1)
79-80: ⚡ Quick winMissing header validation.
The code falls back to
"unknown"and""for missing GitHub headers. While this prevents crashes, it may allow invalid webhooks to be processed. Consider validating that required headers are present.♻️ Proposed validation
const githubEvent = request.headers.get("x-github-event") || "unknown"; - const githubDelivery = request.headers.get("x-github-delivery") || ""; + const githubDelivery = request.headers.get("x-github-delivery"); + + if (!githubDelivery) { + return NextResponse.json( + { success: false, error: "Missing x-github-delivery header" }, + { status: 400 } + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/webhooks/github/`[webhookId]/route.ts around lines 79 - 80, The code currently defaults missing GitHub headers to "unknown" and "" (githubEvent and githubDelivery); instead validate presence of required headers (at least "x-github-event" and "x-github-delivery") after reading them from request.headers.get and short-circuit with an HTTP 400/422 response or throw an error when they are missing or empty. Update the handler around the githubEvent/githubDelivery retrieval in route.ts to check these values, return a clear bad-request response when invalid, and include which header is missing in the response/log to help debugging.src/features/executions/components/github/types.ts (1)
33-33: ⚡ Quick winReplace
anytype with a more specific type.The
optionsfield is typed asany, which disables TypeScript's type checking and can lead to runtime errors. Consider defining a more specific type or usingRecord<string, unknown>for better type safety.♻️ Proposed fix
- options?: any; + options?: Record<string, unknown>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/executions/components/github/types.ts` at line 33, The options property is currently typed as any which disables TS checking; replace the `options?: any` declaration with a stricter type—either a defined interface/alias that enumerates expected keys (e.g., GitHubActionOptions) or at minimum `Record<string, unknown>`—and update any usages to conform to that type (modify the declaration where `options` is defined and adjust callers to satisfy the new shape).src/features/executions/components/github/api-client.ts (1)
39-46: ⚡ Quick winRate limit check could be more robust.
The rate limit detection only checks if
x-ratelimit-remainingequals"0". Consider also checking thex-ratelimit-resetheader to provide better error messages, and verify the header exists before comparing.♻️ Proposed improvement
if (response.status === 403 || response.status === 429) { const remaining = response.headers.get("x-ratelimit-remaining"); - if (remaining === "0") { + if (remaining && parseInt(remaining) === 0) { const resetTime = response.headers.get("x-ratelimit-reset"); - throw new NonRetriableError( - `GitHub API rate limit exceeded. Resets at ${resetTime}` - ); + const resetDate = resetTime ? new Date(parseInt(resetTime) * 1000).toISOString() : "unknown"; + throw new NonRetriableError( + `GitHub API rate limit exceeded. Resets at ${resetDate}` + ); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/executions/components/github/api-client.ts` around lines 39 - 46, The rate-limit handling in the response.status === 403 || 429 branch is fragile: update the logic around response.headers.get("x-ratelimit-remaining") and "x-ratelimit-reset" so you first verify headers exist (not null), treat missing or non-numeric values safely, and include the reset timestamp in the NonRetriableError message; specifically, in the branch that throws new NonRetriableError, read and parse both remaining and reset (e.g., via response.headers.get("x-ratelimit-reset")), fallback to a sensible message when headers are absent or non-numeric, and construct a clearer error string that contains the reset time or notes that the header was missing—make these changes in the rate limit check around the response.headers.get usage in the API client file.src/features/executions/components/github/executor.ts (1)
57-60: 💤 Low valueAdd error handling for malformed credential JSON.
If the decrypted credential value is not valid JSON,
JSON.parsewill throw a cryptic error. Wrapping this in a try/catch with a descriptive error improves debuggability.Suggested improvement
- const creds = JSON.parse(decrypt(credential.value)) as { - accessToken: string; - baseUrl?: string; - }; + let creds: { accessToken: string; baseUrl?: string }; + try { + creds = JSON.parse(decrypt(credential.value)); + } catch { + await publish(githubChannel().status({ nodeId, status: "error" })); + throw new NonRetriableError("Invalid GitHub credential format. Please re-add your credential."); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/executions/components/github/executor.ts` around lines 57 - 60, Wrap the JSON.parse(decrypt(credential.value)) call in a try/catch inside the executor (around the const creds assignment) to catch malformed JSON; in the catch, log or throw a descriptive error that includes context (e.g., credential identifier or type, and the raw decrypted string length/preview) and the original parse error, then fail gracefully or rethrow a wrapped error so callers of the GitHub executor know the credential was invalid. Ensure you still type the resulting creds as { accessToken: string; baseUrl?: string } after successful parse.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prisma/schema/migrations/20260604130200_schema_refactoring/migration.sql`:
- Around line 1-3: The migration is missing GitHub-related schema objects which
causes schema drift: add the missing enum members "GITHUB" and "GITHUB_TRIGGER"
to the CREATE TYPE "NodeType", add missing members "GITHUB" and "GITHUB_APP" to
the "CredentialType" enum, add a CREATE TYPE "GitHubOperation" per enums.prisma,
and add CREATE TABLE statements for GitHubNode and GitHubTriggerNode (including
all columns, indices and foreign keys that match the Prisma models) and any
associated indices/foreign keys referenced by those tables; ensure the new SQL
exactly matches the Prisma schema (enums.prisma and model definitions) so the
migration fully reflects GitHubNode, GitHubTriggerNode, GitHubOperation, and
CredentialType changes.
In `@prisma/schema/nodes_commerce.prisma`:
- Around line 170-171: RazorpayNode's credential relation uses onDelete: SetNull
but ShiprocketNode and CashfreeNode do not, which risks FK constraint failures
when a Credential is deleted; update the credential relation fields for
ShiprocketNode and CashfreeNode (the relation that uses credential and
credentialId) to include onDelete: SetNull so deleted credentials set the
foreign key to null and remain consistent with RazorpayNode.
In `@prisma/schema/nodes_crm.prisma`:
- Around line 10-11: The credential relations on the CRM node models
(ZohoCrmNode, HubspotNode, FreshdeskNode) are missing onDelete behavior; update
each relation annotation that links credentialId -> Credential (the relation
named credential on these models) to include onDelete: SetNull (matching
RazorpayNode) so that when a Credential is deleted the FK is set to null and FK
constraint errors are avoided.
In `@prisma/schema/nodes_github.prisma`:
- Around line 70-71: Add an encrypted counterpart for the plaintext
webhookSecret on the GitHubTriggerNode model: introduce a nullable String field
named webhookSecretEncrypted and keep the existing webhookSecret as-is during
migration (marking it deprecated in comments if you follow the repo pattern).
Update any code that reads webhook secrets to prefer webhookSecretEncrypted
(decrypting it) and fall back to webhookSecret only while you run a data
migration; add a DB migration that creates webhookSecretEncrypted and a one-off
migration script to encrypt and copy existing webhookSecret values into
webhookSecretEncrypted, then remove/deprecate webhookSecret in a subsequent
release.
In `@src/app/api/webhooks/github/`[webhookId]/route.ts:
- Around line 27-30: The content-length header parsing can yield NaN and bypass
the size check; update the logic around request.headers.get("content-length")
and the contentLength variable to safely parse and validate the numeric value
(use parseInt(..., 10) or Number and then check isFinite/!isNaN) and treat any
missing/invalid/negative value as unacceptable (or as zero depending on policy),
then compare the validated numeric length against 10_000_000 and return
NextResponse.json({ error: "Payload too large" }, { status: 413 }) when it
exceeds the limit or when the header is invalid if you prefer stricter
rejection.
In `@src/features/executions/components/github/api-client.ts`:
- Around line 85-108: The paginate method is bypassing centralized
error/rate-limit handling by calling fetch directly; update paginate to call the
existing request flow instead of fetch — either refactor request() to optionally
return both parsed data and response headers (e.g., return { data, headers }) or
add a private helper like requestWithHeaders(endpoint, options) that enforces
rate-limit/error/204/non-JSON logic and returns headers; then have paginate use
that helper to get parsed data and the Link header, check response.ok/204 before
parsing, validate Array.isArray(data) and break appropriately, and read
linkHeader from the returned headers to drive pagination; keep the paginate
signature and logic (perPage/page loop) but delegate network/error handling to
request/requestWithHeaders so rate limits and errors are handled consistently.
In `@src/features/executions/components/github/components/generic-fields.tsx`:
- Around line 116-125: The controlled Textarea currently uses
JSON.stringify(values.options) and only updates values via setValues when
parsing succeeds, which causes partial edits to be lost; change this by
introducing a local string state (e.g., optionsText) initialized from
JSON.stringify(values.options, null, 2), bind Textarea value to optionsText,
update optionsText on every onChange, attempt JSON.parse(e.target.value) and
only call setValues({ ...values, options: parsed }) when parse succeeds, and add
an effect to sync optionsText when values.options changes externally; update
references in the component (Textarea value, onChange handler, and the parse
logic) accordingly so partial/invalid JSON is preserved while still updating
values on valid JSON.
In `@src/features/executions/components/github/components/workflow-fields.tsx`:
- Around line 174-183: The Textarea for advanced options is treating
values.options as the single source of truth so intermediate/invalid JSON edits
get discarded; change to a local string state (e.g., optionsText) initialized
from JSON.stringify(values.options) and bind the Textarea to that local state so
user typing is preserved, update optionsText on every onChange, attempt
JSON.parse and only call setValues({ ...values, options: parsed }) when parse
succeeds (or alternatively parse and set onBlur), and add a useEffect to resync
optionsText when values.options changes externally; update references of
Textarea value/onChange and keep setValues usage only when valid JSON is
available.
In `@src/features/executions/components/github/dialog.tsx`:
- Around line 154-156: The labels are not bound to their corresponding Select
controls which breaks accessibility; update each <label> to include an htmlFor
that matches an id on the corresponding SelectTrigger (or the native control
inside the Select) — e.g., add htmlFor="credential-select" to the label that
relates to credentialId and set id="credential-select" on the matching
<SelectTrigger> (and do the same for the second label/control pair referenced
around the other Select), ensuring the Select component forwards the id to the
underlying input/button so the label correctly associates with the control.
- Around line 63-95: The useEffect that hydrates the dialog from nodeData
(useEffect) only sets state when nodeData is truthy, so stale form state remains
when opening an empty/new node; update the effect to handle the falsy case by
explicitly resetting local state: call setCredentialId("") and setValues(...)
with the default empty values (operation default, owner:"", repo:"", perPage:30,
draft:false, prerelease:false, options:{} etc.) when nodeData is null/undefined,
ensuring the dialog is cleared before a new/empty node is edited; keep using the
existing setters setCredentialId and setValues to locate the change.
In `@src/features/executions/components/github/executor.ts`:
- Around line 121-122: The executor currently returns only the local result,
dropping upstream variables; update the return in the github executor (the
function containing the publish call with githubChannel().status({ nodeId,
status: "success" })) to merge the incoming context with the result so
accumulated context is preserved for downstream nodes (e.g., return an object
combining the original context and result, ensuring result keys override context
when appropriate). Locate the function that receives the incoming context and
ensure you merge that context with the `result` before returning and after
publishing the success status.
In `@src/features/executions/components/github/executors/issues-prs.ts`:
- Around line 59-61: Replace repeated JSON.parse calls with a small
safeParseJson helper and use it across the issue handlers (ISSUE_CREATE,
ISSUE_UPDATE, ISSUE_ADD_LABELS, ISSUE_ASSIGN, ISSUE_UNASSIGN): implement
function safeParseJson<T=unknown>(value: string, fieldName: string) that tries
JSON.parse and on failure throws a NonRetriableError with a message like
"Invalid JSON in <fieldName>: <error message>", then replace occurrences such as
body.labels = JSON.parse(config.labels) and body.assignees =
JSON.parse(config.assignees) with body.labels = safeParseJson(config.labels,
'labels') and body.assignees = safeParseJson(config.assignees, 'assignees') (add
similar replacements wherever labels/assignees parsing is used); keep existing
behavior for milestone parsing but consider validating the resulting number.
- Around line 27-29: Wrap the parsing of config.labels and config.assignees in
safe JSON parsing with try/catch (or a helper like safeParseJson) and only
assign to body.labels/body.assignees if parsing succeeds; on parse failure
log/return a clear error instead of letting an exception bubble. Similarly
validate config.options?.milestone after parseInt (use Number.parseInt or Number
and then check Number.isFinite/!Number.isNaN) before assigning to
body.milestone, and handle invalid milestone values by logging/returning a
validation error. Update the block that currently reads config.labels,
config.assignees and config.options?.milestone so it uses these guarded parses
and validation.
In `@src/features/executions/components/github/executors/workflows.ts`:
- Around line 27-31: The code in the GitHubOperation.WORKFLOW_DISPATCH branch
calls JSON.parse on resolveTemplate(config.clientPayload, context) without
protection; wrap the parsing in a try/catch around the JSON.parse call that
produces inputs (and use the resolved string from resolveTemplate), and on parse
failure handle it explicitly: either throw a new Error with a descriptive
message including the failing payload and the original error, or log the error
and fall back to an empty object; update the inputs assignment inside that block
accordingly so resolveTemplate, config.clientPayload and inputs are referenced
and the failure is clearly reported/handled.
In `@src/features/triggers/components/github-trigger/executor.ts`:
- Around line 12-25: The status updates are being published to githubChannel()
while the trigger node listens on the webhook-trigger channel; update both
publish calls to use the webhook-trigger realtime channel function (e.g.,
webhookTriggerChannel().status) so the nodeId/status messages go to the same
channel the GitHub trigger subscribes to—modify the two publish(...) invocations
that call githubChannel().status to call the webhook-trigger channel's status
method instead, leaving nodeId and status values and the surrounding
step.run(...) logic unchanged.
- Around line 19-28: The current executor calls step.run("github-trigger", async
() => context) and then publishes a success status, but if step.run throws no
error status is emitted; wrap the call in try/catch around step.run, on success
publish githubChannel().status({ nodeId, status: "success" }) as before, and in
the catch publish githubChannel().status({ nodeId, status: "error" }) (include
any minimal context if available), then rethrow the caught error so callers
still see the failure; update the executor function that uses step.run, publish,
githubChannel(), and nodeId accordingly.
In `@src/inngest/channels/github.ts`:
- Around line 3-10: The global githubChannel (GITHUB_CHANNEL_NAME and
githubChannel created via channel(...).addTopic(...)) exposes a shared "status"
topic across tenants/workflows; change the design to create a scoped channel per
tenant/workflow by deriving the channel name or topic namespace from runtime
identifiers (e.g., tenantId and/or workflowId) when calling channel(...) so
subscribers only receive status events for their own context; update any places
that reference GITHUB_CHANNEL_NAME/githubChannel to use a factory function
(e.g., getGithubChannel(tenantId, workflowId)) that constructs the scoped
channel/topic signature and registers the same "status" topic shape.
In `@src/server/routers/github-trigger.router.ts`:
- Around line 45-54: The PR currently stores and verifies webhookSecret in
plaintext; update the data model and code to persist an encrypted secret using
the existing encryptWebhookSecret helper and decrypt/verify in the webhook
route. Add a webhookSecretEncrypted field to the GitHubTriggerNode model (and
migrate away from plaintext webhookSecret), modify the router logic in
github-trigger.router.ts to call encryptWebhookSecret(webhookSecret) before
saving (use webhookId/webhookSecret variable names already present) and
remove/stop writing the plaintext field, and update
src/app/api/webhooks/github/[webhookId]/route.ts to load the encrypted value and
decrypt/verify signatures against the decrypted secret when validating incoming
requests. Ensure the unused import of encryptWebhookSecret is actually used and
that all references to plaintext webhookSecret are removed or migrated.
---
Minor comments:
In `@prisma/schema/nodes_flow_control.prisma`:
- Around line 30-41: The SwitchNode Prisma model currently only defines
@@index([nodeId]) which omits the workflowId index used by other node models;
update the SwitchNode model to add @@index([workflowId]) (in addition to the
existing @@index([nodeId])) so queries filtering by workflowId use an index and
match the other node models; locate the SwitchNode block in the Prisma schema
and add the @@index([workflowId]) directive alongside the existing
@@index([nodeId]).
In `@prisma/schema/nodes_github.prisma`:
- Around line 13-14: Add an explicit onDelete: SetNull to the optional
credential relation so deleting a Credential won't break referential integrity;
update the relation declaration that uses credentialId and credential
(referencing Credential) to include onDelete: SetNull (mirror other node models
like GmailNode/GoogleSheetsNode) so the node's credentialId is set to null when
its Credential is removed.
In `@scripts/migrate-webhook-secrets.ts`:
- Around line 38-43: The current prisma.webhookTrigger.update(...) is vulnerable
to races; instead perform a conditional write using
prisma.webhookTrigger.updateMany(...) with a where clause that includes the id
plus the expected legacy snapshot (e.g. { id: trigger.id, secretTokenEncrypted:
null, secretTokenLegacy: trigger.secretTokenLegacy }) so only a row matching the
same legacy value is migrated, then check the returned count === 1 and handle
count === 0 (skip/log) to detect concurrent/up-to-date cases; replace the update
call with this conditional updateMany pattern in
scripts/migrate-webhook-secrets.ts.
In `@src/features/executions/components/github/executors/repository.ts`:
- Around line 324-336: The JSON.parse of config.options.files in the
GitHubOperation.GIST_CREATE branch can throw on invalid JSON; wrap the parsing
logic for config.options.files (used to produce the files variable before
calling client.request) in a try/catch, and on error throw or return a clear,
contextual error that includes the invalid input (e.g., mention
config.options.files and the operation GitHubOperation.GIST_CREATE) so callers
see why parsing failed; ensure you still fall back to an empty object when
options.files is undefined and keep using resolveTemplate(config.body || "",
context) and client.request(...) unchanged aside from using the safely parsed
files variable.
- Around line 133-143: The code handling GitHubOperation.REPOSITORY_DISPATCH
must validate and gracefully handle parse errors for user-provided
config.clientPayload: wrap the JSON.parse(resolveTemplate(config.clientPayload,
context)) call (used to set client_payload) in a try/catch, and on failure
return or throw a clear, actionable error (or set client_payload = {} and log
via processLogger or the existing logger) instead of letting a cryptic exception
bubble up; keep the rest of the request
(client.request(`/repos/${owner}/${repo}/dispatches`, ...)) unchanged and ensure
the error mentions the invalid clientPayload and the resolveTemplate/context so
the caller can fix their input.
In `@src/features/triggers/components/github-trigger/dialog.tsx`:
- Around line 75-78: handleCopy currently calls
navigator.clipboard.writeText(text) without awaiting errors and always shows
toast.success; change handleCopy to handle the Promise from
navigator.clipboard.writeText (either async/await or .then/.catch) and only call
toast.success(`${name} copied to clipboard`) after the write resolves, and call
toast.error with a clear message (e.g., `${name} copy failed`) when the write
rejects so failures are surfaced to the user.
---
Nitpick comments:
In `@prisma/schema/nodes_github.prisma`:
- Around line 59-75: Add missing DB indexes to the GitHubTriggerNode model:
create an index for workflowId and one for nodeId to match other trigger/node
models and improve query performance. Locate the GitHubTriggerNode model and add
a field-level or model-level index for workflowId (reference symbol: workflowId)
and ensure nodeId (reference symbol: nodeId) also has an index (even if
currently `@unique`) using Prisma's `@index` or @@index syntax so queries filtering
by workflowId or nodeId are backed by DB indexes.
- Around line 6-57: Add the missing Prisma indexes to the GitHubNode model by
adding the model-level index directives for workflowId and nodeId (e.g., add
@@index([workflowId]) and @@index([nodeId]) inside the GitHubNode model). Locate
the GitHubNode model declaration and append these @@index directives (after the
field list, before the closing brace) so queries filtering by workflowId or
nodeId use database indexes.
In `@prisma/schema/nodes_google.prisma`:
- Around line 89-105: The GoogleDriveNode Prisma model is missing the same
database indexes present on GmailNode and GoogleSheetsNode; add index
declarations to the GoogleDriveNode model by appending @@index([workflowId]) and
@@index([nodeId]) to the model definition so queries filtering by workflowId or
nodeId use indexed lookups; modify the GoogleDriveNode model (symbols:
GoogleDriveNode, workflowId, nodeId) to include those @@index entries.
In `@prisma/schema/nodes_messaging.prisma`:
- Around line 54-73: Add the missing database indexes to the WhatsAppNode Prisma
model: add @@index([workflowId]) and @@index([nodeId]) to the WhatsAppNode model
(e.g., append these index directives inside the WhatsAppNode model block,
similar to SlackNode/NotionNode/Msg91Node) so queries filtering by workflowId or
nodeId are indexed; ensure you reference the existing fields workflowId and
nodeId in these index directives.
In `@prisma/schema/nodes_utility.prisma`:
- Around line 95-130: The SortNode model is missing database indexes for quick
lookup; add the same indexes used by other utility nodes by declaring
@@index([workflowId]) and @@index([nodeId]) on the SortNode model so queries
filtering by workflowId or nodeId are efficient; update the SortNode definition
(model SortNode) to include these two @@index declarations.
In `@prisma/schema/triggers.prisma`:
- Around line 129-144: Add the missing indexes to the GmailWatcher model:
include an index on workflowId and an index on nodeId by adding
@@index([workflowId]) and @@index([nodeId]) to the model (you can keep the
existing nodeId uniqueness), ensuring GmailWatcher matches other trigger models'
lookup indexes.
In `@src/app/api/webhooks/github/`[webhookId]/route.ts:
- Around line 79-80: The code currently defaults missing GitHub headers to
"unknown" and "" (githubEvent and githubDelivery); instead validate presence of
required headers (at least "x-github-event" and "x-github-delivery") after
reading them from request.headers.get and short-circuit with an HTTP 400/422
response or throw an error when they are missing or empty. Update the handler
around the githubEvent/githubDelivery retrieval in route.ts to check these
values, return a clear bad-request response when invalid, and include which
header is missing in the response/log to help debugging.
In `@src/features/executions/components/github/api-client.ts`:
- Around line 39-46: The rate-limit handling in the response.status === 403 ||
429 branch is fragile: update the logic around
response.headers.get("x-ratelimit-remaining") and "x-ratelimit-reset" so you
first verify headers exist (not null), treat missing or non-numeric values
safely, and include the reset timestamp in the NonRetriableError message;
specifically, in the branch that throws new NonRetriableError, read and parse
both remaining and reset (e.g., via response.headers.get("x-ratelimit-reset")),
fallback to a sensible message when headers are absent or non-numeric, and
construct a clearer error string that contains the reset time or notes that the
header was missing—make these changes in the rate limit check around the
response.headers.get usage in the API client file.
In `@src/features/executions/components/github/executor.ts`:
- Around line 57-60: Wrap the JSON.parse(decrypt(credential.value)) call in a
try/catch inside the executor (around the const creds assignment) to catch
malformed JSON; in the catch, log or throw a descriptive error that includes
context (e.g., credential identifier or type, and the raw decrypted string
length/preview) and the original parse error, then fail gracefully or rethrow a
wrapped error so callers of the GitHub executor know the credential was invalid.
Ensure you still type the resulting creds as { accessToken: string; baseUrl?:
string } after successful parse.
In `@src/features/executions/components/github/types.ts`:
- Line 33: The options property is currently typed as any which disables TS
checking; replace the `options?: any` declaration with a stricter type—either a
defined interface/alias that enumerates expected keys (e.g.,
GitHubActionOptions) or at minimum `Record<string, unknown>`—and update any
usages to conform to that type (modify the declaration where `options` is
defined and adjust callers to satisfy the new shape).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96044213-bdbf-4ed6-b628-cd284c0b7c05
📒 Files selected for processing (55)
.gitignorepackage.jsonprisma.config.tsprisma/schema.prisma.bakprisma/schema/auth.prismaprisma/schema/base.prismaprisma/schema/core.prismaprisma/schema/enums.prismaprisma/schema/migrations/20260604130200_schema_refactoring/migration.sqlprisma/schema/migrations/migration_lock.tomlprisma/schema/nodes_commerce.prismaprisma/schema/nodes_crm.prismaprisma/schema/nodes_flow_control.prismaprisma/schema/nodes_github.prismaprisma/schema/nodes_google.prismaprisma/schema/nodes_messaging.prismaprisma/schema/nodes_utility.prismaprisma/schema/triggers.prismascripts/migrate-webhook-secrets.tssrc/app/api/auth/custom-signup/route.tssrc/app/api/webhooks/github/[webhookId]/route.tssrc/features/credentials/components/credentials.tsxsrc/features/executions/components/github/actions.tssrc/features/executions/components/github/api-client.tssrc/features/executions/components/github/components/generic-fields.tsxsrc/features/executions/components/github/components/issue-fields.tsxsrc/features/executions/components/github/components/operation-groups.tssrc/features/executions/components/github/components/repo-fields.tsxsrc/features/executions/components/github/components/workflow-fields.tsxsrc/features/executions/components/github/dialog.tsxsrc/features/executions/components/github/executor.tssrc/features/executions/components/github/executors/issues-prs.tssrc/features/executions/components/github/executors/repository.tssrc/features/executions/components/github/executors/search-misc.tssrc/features/executions/components/github/executors/users-orgs.tssrc/features/executions/components/github/executors/workflows.tssrc/features/executions/components/github/node.tsxsrc/features/executions/components/github/types.tssrc/features/executions/components/google-sheets/dialog.tsxsrc/features/executions/components/slack/dialog.tsxsrc/features/executions/components/slack/executor.tssrc/features/executions/lib/executor-registry.tssrc/features/triggers/components/github-trigger/dialog.tsxsrc/features/triggers/components/github-trigger/executor.tssrc/features/triggers/components/github-trigger/node.tsxsrc/inngest/channels/github.tssrc/inngest/functions.tssrc/lib/auth-client.tssrc/lib/auth.tssrc/lib/webhook-secret.tssrc/server/routers/github-trigger.router.tssrc/server/routers/github.router.tssrc/server/routers/slack.router.tssrc/server/routers/webhook-trigger.router.tssrc/trpc/routers/_app.ts
💤 Files with no reviewable changes (2)
- src/features/executions/components/slack/dialog.tsx
- src/server/routers/slack.router.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 15 file(s) based on 18 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 15 file(s) based on 18 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prisma/schema/migrations/20260604130200_schema_refactoring/migration.sql`:
- Around line 1338-1347: Add a new nullable TEXT column named
webhookSecretEncrypted to the GitHubTriggerNode table in the migration SQL so
the schema matches nodes_github.prisma and the runtime code; update the CREATE
TABLE statement for GitHubTriggerNode to include "webhookSecretEncrypted" TEXT
(nullable) alongside the existing webhookSecret column, and ensure any ALTER
TABLE statements in the same migration (if present) also add this column so
existing databases get the encrypted-field column.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 368934ae-2134-4fd5-95a3-6d9fdba48767
📒 Files selected for processing (15)
prisma/schema/migrations/20260604130200_schema_refactoring/migration.sqlprisma/schema/nodes_commerce.prismaprisma/schema/nodes_crm.prismaprisma/schema/nodes_github.prismasrc/app/api/webhooks/github/[webhookId]/route.tssrc/features/executions/components/github/api-client.tssrc/features/executions/components/github/components/generic-fields.tsxsrc/features/executions/components/github/components/workflow-fields.tsxsrc/features/executions/components/github/dialog.tsxsrc/features/executions/components/github/executor.tssrc/features/executions/components/github/executors/issues-prs.tssrc/features/executions/components/github/executors/workflows.tssrc/features/triggers/components/github-trigger/executor.tssrc/inngest/channels/github.tssrc/server/routers/github-trigger.router.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- prisma/schema/nodes_github.prisma
- src/features/executions/components/github/components/generic-fields.tsx
- prisma/schema/nodes_commerce.prisma
- src/app/api/webhooks/github/[webhookId]/route.ts
- src/features/executions/components/github/executor.ts
- src/features/executions/components/github/dialog.tsx
- prisma/schema/nodes_crm.prisma
- src/features/executions/components/github/executors/issues-prs.ts
- src/server/routers/github-trigger.router.ts
- src/features/executions/components/github/executors/workflows.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/executions/components/github/executor.ts (1)
94-122: 💤 Low valueConsider defining a shared type or mapper instead of repeated
as unknown as GitHubConfigcasts.The same unsafe cast pattern appears five times. While this works, it bypasses type checking between the Prisma model and the
GitHubConfiginterface. If the Prisma schema andGitHubConfigdrift, runtime errors could occur without compile-time warnings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/executions/components/github/executor.ts` around lines 94 - 122, The repeated unsafe casts "as unknown as GitHubConfig" should be replaced by a single, explicit mapper/transform function (e.g., toGitHubConfig(prismaModel): GitHubConfig) that validates and converts the Prisma config shape to the GitHubConfig interface; update all call sites (executeRepositoryOperations, executeIssuesPrsOperations, executeWorkflowOperations, executeUserOrgOperations, executeSearchMiscOperations) to pass the result of toGitHubConfig(config) instead of casting, and ensure the mapper performs any necessary field renaming, defaults, and runtime checks so type drift is caught and handled consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prisma/schema/migrations/20260604130200_schema_refactoring/migration.sql`:
- Around line 1924-1948: Add a unique index on the GitHubTriggerNode.webhookId
column so webhook lookups use the index; update the migration that creates the
"GitHubTriggerNode" table by adding a CREATE UNIQUE INDEX for webhookId (e.g.,
name it "GitHubTriggerNode_webhookId_key") similar to other trigger tables,
ensuring the index targets "webhookId" to optimize gitHubTriggerNode.findFirst({
where: { webhookId } }) queries.
---
Nitpick comments:
In `@src/features/executions/components/github/executor.ts`:
- Around line 94-122: The repeated unsafe casts "as unknown as GitHubConfig"
should be replaced by a single, explicit mapper/transform function (e.g.,
toGitHubConfig(prismaModel): GitHubConfig) that validates and converts the
Prisma config shape to the GitHubConfig interface; update all call sites
(executeRepositoryOperations, executeIssuesPrsOperations,
executeWorkflowOperations, executeUserOrgOperations,
executeSearchMiscOperations) to pass the result of toGitHubConfig(config)
instead of casting, and ensure the mapper performs any necessary field renaming,
defaults, and runtime checks so type drift is caught and handled consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4c81ebc-9dc9-4296-a115-b29fc181cac6
📒 Files selected for processing (24)
prisma/schema/migrations/20260604130200_schema_refactoring/migration.sqlprisma/schema/nodes_flow_control.prismaprisma/schema/nodes_github.prismaprisma/schema/nodes_google.prismaprisma/schema/nodes_messaging.prismaprisma/schema/nodes_utility.prismaprisma/schema/triggers.prismascripts/fix-ts.jsscripts/migrate-github-secrets.tsscripts/migrate-webhook-secrets.tssrc/app/api/webhooks/github/[webhookId]/route.tssrc/features/executions/components/github/api-client.tssrc/features/executions/components/github/components/generic-fields.tsxsrc/features/executions/components/github/components/workflow-fields.tsxsrc/features/executions/components/github/dialog.tsxsrc/features/executions/components/github/executor.tssrc/features/executions/components/github/executors/issues-prs.tssrc/features/executions/components/github/executors/repository.tssrc/features/executions/components/github/executors/search-misc.tssrc/features/executions/components/github/executors/users-orgs.tssrc/features/executions/components/github/executors/workflows.tssrc/features/executions/components/github/types.tssrc/features/triggers/components/github-trigger/dialog.tsxsrc/server/routers/github-trigger.router.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- src/features/executions/components/github/dialog.tsx
- src/server/routers/github-trigger.router.ts
- scripts/migrate-webhook-secrets.ts
- src/app/api/webhooks/github/[webhookId]/route.ts
- src/features/executions/components/github/api-client.ts
- prisma/schema/nodes_messaging.prisma
- src/features/executions/components/github/executors/users-orgs.ts
- src/features/executions/components/github/components/workflow-fields.tsx
- prisma/schema/nodes_google.prisma
- prisma/schema/nodes_github.prisma
- prisma/schema/triggers.prisma
- src/features/executions/components/github/components/generic-fields.tsx
- prisma/schema/nodes_utility.prisma
- src/features/executions/components/github/executors/workflows.ts
- prisma/schema/nodes_flow_control.prisma
- src/features/triggers/components/github-trigger/dialog.tsx
- src/features/executions/components/github/executors/repository.ts
- src/features/executions/components/github/executors/issues-prs.ts
Comment on lines
+1924
to
+1948
| CREATE TABLE "GitHubTriggerNode" ( | ||
| "id" TEXT NOT NULL, | ||
| "nodeId" TEXT NOT NULL, | ||
| "workflowId" TEXT NOT NULL, | ||
| "owner" TEXT NOT NULL DEFAULT '', | ||
| "repo" TEXT NOT NULL DEFAULT '', | ||
| "events" TEXT NOT NULL DEFAULT '["push"]', | ||
| "webhookId" TEXT, | ||
| "webhookSecret" TEXT, | ||
| "webhookSecretEncrypted" TEXT, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
|
|
||
| CONSTRAINT "GitHubTriggerNode_pkey" PRIMARY KEY ("id") | ||
| ); | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "GitHubNode_nodeId_key" ON "GitHubNode"("nodeId"); | ||
| CREATE INDEX "GitHubNode_workflowId_idx" ON "GitHubNode"("workflowId"); | ||
| CREATE INDEX "GitHubNode_nodeId_idx" ON "GitHubNode"("nodeId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "GitHubTriggerNode_nodeId_key" ON "GitHubTriggerNode"("nodeId"); | ||
| CREATE INDEX "GitHubTriggerNode_workflowId_idx" ON "GitHubTriggerNode"("workflowId"); | ||
|
|
Contributor
There was a problem hiding this comment.
Missing index on GitHubTriggerNode.webhookId for webhook lookup performance.
The webhook route (src/app/api/webhooks/github/[webhookId]/route.ts) queries gitHubTriggerNode.findFirst({ where: { webhookId } }), but this migration only creates indexes on nodeId and workflowId. Without an index on webhookId, webhook ingestion will perform a full table scan.
Other trigger tables (RazorpayTrigger, WhatsAppTrigger, WebhookTrigger) all have a unique index on webhookId.
Proposed fix: Add unique index on webhookId
CREATE UNIQUE INDEX "GitHubTriggerNode_nodeId_key" ON "GitHubTriggerNode"("nodeId");
CREATE INDEX "GitHubTriggerNode_workflowId_idx" ON "GitHubTriggerNode"("workflowId");
+CREATE UNIQUE INDEX "GitHubTriggerNode_webhookId_key" ON "GitHubTriggerNode"("webhookId");
+CREATE INDEX "GitHubTriggerNode_webhookId_idx" ON "GitHubTriggerNode"("webhookId");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma/schema/migrations/20260604130200_schema_refactoring/migration.sql`
around lines 1924 - 1948, Add a unique index on the GitHubTriggerNode.webhookId
column so webhook lookups use the index; update the migration that creates the
"GitHubTriggerNode" table by adding a CREATE UNIQUE INDEX for webhookId (e.g.,
name it "GitHubTriggerNode_webhookId_key") similar to other trigger tables,
ensuring the index targets "webhookId" to optimize gitHubTriggerNode.findFirst({
where: { webhookId } }) queries.
… workflow execution management
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit