feat(webapp/deployments): Vercel improvements & fixes #3037
Conversation
Include build server metadata in progress API and deployment handling, add skipEnqueue flag, and avoid enque builds for GitHub-integrated ments without artifact keys. - Add BuildServerMetadata schema and include it in ProgressDeploymentRequestBody. - Thread buildServerMetadata through API handler into deployment service. - Make artifactKey optional for native builds and add skipEnqueue with default false to request validation. - When progressing deployments, merge incoming buildServerMetadata and validate against existing metadata. - Only enqueue native builds when artifactKey is present and skipEnqueue is false. This prevents enqueue attempts for GitHub integrated builds that lack an artifactKey.
Automatically set the Vercel project when entering project-selection if there is at least one available project and none selected. This prevents an empty selection state and reduces manual clicks for the common case of a single or first project. Disable the Select when there is exactly one available project to make the intent explicit and avoid unnecessary interaction. Add a filter prop to the Select only when there are more than five projects to keep the dropdown performant and avoid enabling filtering for small lists. Adjust the effect deps to trigger selection only on relevant state and project list changes.
Introduce PostHog instrumentation in the VercelOnboardingModal to
capture user progress and actions during the onboarding flow.
Key changes:
- Import posthog and add a reusable trackOnboarding callback that
attaches context (origin, step, organization_slug, project_slug).
- Start session recording and emit a "vercel onboarding started" event
the first time the modal reaches project-selection while open.
- Track key milestones and user decisions:
- "vercel onboarding project selected" when a project is chosen.
- "vercel onboarding abandoned" when the user skips onboarding.
- "vercel onboarding env mapping completed" with skipped flag or
selected staging environment.
- "vercel onboarding build settings completed".
- "vercel onboarding completed" with github_connected flag.
- Ensure tracking callbacks are included in effect and callback deps
to avoid stale closures and double-tracking.
Why:
- Provide visibility into onboarding funnel performance and user
behaviors to improve onboarding UX and conversion.
|
WalkthroughThe pull request introduces build server metadata tracking across deployment services and adds analytics instrumentation to the Vercel onboarding flow. A new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/integrations/VercelOnboardingModal.tsx`:
- Line 50: Replace the direct import of posthog and direct calls to
posthog.capture()/posthog.startSessionRecording() in VercelOnboardingModal by
calling the centralized usePostHog() hook from app/hooks/usePostHog.ts; remove
the import line `import posthog from "posthog-js"`, add `usePostHog()`
invocation at the top of the VercelOnboardingModal component (so initialization
runs on mount) and then use the hook-provided API or ensure posthog calls occur
only after usePostHog() has initialized (e.g., call
capture/startSessionRecording after usePostHog() returns/sets ready), keeping
the original event names/arguments unchanged.
In `@packages/core/src/v3/schemas/api.ts`:
- Around line 595-619: The discriminated union InitializeDeploymentRequestBody
is invalid because the discriminator is defined optional in both
InitializeDeploymentRequestBodyBase (isNativeBuild:
z.boolean().default(false).optional()) and the false branch (isNativeBuild:
z.literal(false).optional()), causing invalid_union_discriminator when the field
is omitted; fix by removing isNativeBuild from
InitializeDeploymentRequestBodyBase entirely, change the false branch to require
isNativeBuild: z.literal(false) (no .optional()), keep the true branch as is
(isNativeBuild: z.literal(true)), and add the missing trailing semicolon to the
export statement for InitializeDeploymentRequestBody.
🧹 Nitpick comments (6)
apps/webapp/app/components/integrations/VercelOnboardingModal.tsx (3)
631-638: Unnecessaryas stringcast; condition is always true in this branch.On Line 633,
state as string !== "completed"castsOnboardingStatetostringneedlessly—"completed"is already a valid member of the union. More importantly, this code only runs whenisLoadingStateis true (Line 629), sostatecan never be"completed"here, making the guard dead code.The non-loading dialog handler on Line 663 already does this correctly without the cast.
Suggested fix
- if (state as string !== "completed") { - trackOnboarding("vercel onboarding abandoned"); - } + trackOnboarding("vercel onboarding abandoned");
468-482:nextUrlis listed in the dependency array but unused in the callback.
handleSkipOnboardingdoes not referencenextUrlin its body, yet it appears in theuseCallbackdependency array on Line 482. This won't cause a bug, but it creates unnecessary re-memoizations whennextUrlchanges.Suggested fix
- }, [actionUrl, fetcher, onClose, nextUrl, fromMarketplaceContext, trackOnboarding]); + }, [actionUrl, fetcher, onClose, fromMarketplaceContext, trackOnboarding]);
176-199:trackOnboardingcloses overstate, causing potential stale-state in some call sites.Because
trackOnboardingdepends onstate(Line 186), each state transition creates a new callback reference. This is fine for effects that listtrackOnboardingin their deps, but handlers likehandleBuildSettingsNext(Line 515) capturetrackOnboardingat memoization time. Ifstatehas transitioned between when the handler was memoized and when it fires, the loggedstepvalue could be stale.For event handlers that fire on user interaction (not in effects), consider passing the step explicitly rather than relying on the closed-over
state:Example approach
const trackOnboarding = useCallback( - (eventName: string, extraProperties?: Record<string, unknown>) => { + (eventName: string, extraProperties?: Record<string, unknown>, stepOverride?: string) => { posthog.capture(eventName, { origin: fromMarketplaceContext ? "marketplace" : "dashboard", - step: state, + step: stepOverride ?? state, organization_slug: organizationSlug, project_slug: projectSlug, ...extraProperties, }); }, [fromMarketplaceContext, state, organizationSlug, projectSlug] );Then at call sites:
trackOnboarding("vercel onboarding build settings completed", {}, "build-settings");packages/core/src/v3/schemas/api.ts (1)
608-614:skipEnqueue: z.boolean().default(false).optional()— same.default().optional()concern.Same as the base field:
.optional()after.default(false)is confusing. If the intent is "default to false when missing," just use.default(false). If the intent is "allow undefined in the output," the.default()is never reached. Pick one.Suggested fix
- skipEnqueue: z.boolean().default(false).optional(), + skipEnqueue: z.boolean().default(false),apps/webapp/app/v3/services/initializeDeployment.server.ts (1)
193-207:BuildServerMetadatatype is used for the value but Zod runtime parsing is not applied here.The
buildServerMetadataobject is constructed manually and passed directly to Prisma. While the type annotation enforces compile-time safety, theBuildServerMetadataZod schema is imported (line 1) but only used as a type. This is fine since the data originates from already-validatedpayload, but note that the constructed object includesskipEnqueue(line 203) which may beundefined— this will be stored as{ skipEnqueue: undefined }in the JSON column rather than absent. Consider whether that's intentional.Optional: strip undefined keys before storing
const buildServerMetadata: BuildServerMetadata | undefined = payload.isNativeBuild || payload.buildId ? { buildId: payload.buildId, ...(payload.isNativeBuild ? { isNativeBuild: payload.isNativeBuild, artifactKey: payload.artifactKey, skipPromotion: payload.skipPromotion, configFilePath: payload.configFilePath, skipEnqueue: payload.skipEnqueue, } : {}), } : undefined;When spreading,
undefinedvalues are included as explicit keys in JSON. If that's undesired,JSON.parse(JSON.stringify(obj))or a utility likelodash/omitBycould strip them.apps/webapp/app/v3/services/deployment.server.ts (1)
73-108: Duplicated merge logic forbuildServerMetadataacross both progress functions.The identical merge pattern (spread existing + spread new) appears in both
progressToInstallingandprogressToBuilding. This is manageable now but could drift if the merge semantics change.Consider extracting a small helper:
Optional extraction
const mergedBuildServerMetadata = newBuildServerMetadata ? { ...(existingBuildServerMetadata ?? {}), ...newBuildServerMetadata, } : undefined;Then use
...(mergedBuildServerMetadata ? { buildServerMetadata: mergedBuildServerMetadata } : {})in both update calls, computingexistingBuildServerMetadataonce from the deployment passed into each function.Also applies to: 110-157
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/v3/services/initializeDeployment.server.tspackages/core/src/v3/schemas/api.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.tspackages/core/src/v3/schemas/api.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.tspackages/core/src/v3/schemas/api.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.tspackages/core/src/v3/schemas/api.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.ts
apps/webapp/app/v3/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Organize services in the webapp following the pattern
app/v3/services/*/*.server.ts
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.tspackages/core/src/v3/schemas/api.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tsapps/webapp/app/components/integrations/VercelOnboardingModal.tsxapps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.tspackages/core/src/v3/schemas/api.ts
{packages,integrations}/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Add a changeset when modifying any public package in
packages/*orintegrations/*usingpnpm run changeset:add
Files:
packages/core/src/v3/schemas/api.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/presenters/v3/BranchesPresenter.server.ts:45-45
Timestamp: 2026-02-03T18:27:05.229Z
Learning: In the Vercel integration feature, the GitHub app is responsible for builds and provides git metadata (using source: "trigger_github_app"). The Vercel integration is only for linking deployments between platforms, not for triggering builds or providing git metadata.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Use build extensions in trigger.config.ts (additionalFiles, additionalPackages, aptGet, prismaExtension, etc.) to customize the build
Applied to files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/services/deployment.server.tspackages/core/src/v3/schemas/api.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` pattern
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `tasks.trigger()` with type-only imports to trigger tasks from backend code without importing the task implementation
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The SDK at packages/trigger-sdk is an isomorphic TypeScript SDK
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `task()` function from `trigger.dev/sdk/v3` to define tasks with id and run properties
Applied to files:
apps/webapp/app/v3/services/deployment.server.ts
📚 Learning: 2026-02-04T16:34:48.876Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/vercel.connect.tsx:13-27
Timestamp: 2026-02-04T16:34:48.876Z
Learning: In apps/webapp/app/routes/vercel.connect.tsx, configurationId may be absent for "dashboard" flows but must be present for "marketplace" flows. Enforce this with a Zod superRefine and pass installationId to repository methods only when configurationId is defined (omit the field otherwise).
Applied to files:
apps/webapp/app/components/integrations/VercelOnboardingModal.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/integrations/VercelOnboardingModal.tsx
📚 Learning: 2025-08-14T12:13:20.455Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2392
File: packages/cli-v3/src/utilities/gitMeta.ts:195-218
Timestamp: 2025-08-14T12:13:20.455Z
Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.
Applied to files:
packages/core/src/v3/schemas/api.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to {packages,integrations}/**/* : Add a changeset when modifying any public package in `packages/*` or `integrations/*` using `pnpm run changeset:add`
Applied to files:
packages/core/src/v3/schemas/api.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure build process in trigger.config.ts using `build` object with external packages, extensions, and JSX settings
Applied to files:
packages/core/src/v3/schemas/api.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
packages/core/src/v3/schemas/api.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use metadata methods (set, del, replace, append, remove, increment, decrement, stream, flush) to update metadata during task execution
Applied to files:
packages/core/src/v3/schemas/api.ts
📚 Learning: 2026-02-03T18:27:05.229Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/presenters/v3/BranchesPresenter.server.ts:45-45
Timestamp: 2026-02-03T18:27:05.229Z
Learning: In the Vercel integration feature, the GitHub app is responsible for builds and provides git metadata (using source: "trigger_github_app"). The Vercel integration is only for linking deployments between platforms, not for triggering builds or providing git metadata.
Applied to files:
packages/core/src/v3/schemas/api.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
packages/core/src/v3/schemas/api.ts
🧬 Code graph analysis (1)
apps/webapp/app/v3/services/deployment.server.ts (1)
packages/core/src/v3/schemas/api.ts (2)
BuildServerMetadata(488-495)BuildServerMetadata(497-497)
⏰ 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). (26)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
🔇 Additional comments (7)
apps/webapp/app/components/integrations/VercelOnboardingModal.tsx (1)
612-616: Auto-selecting the first project is a nice UX improvement.This pairs well with the
disabledprop on Line 692 for single-project lists.packages/core/src/v3/schemas/api.ts (1)
488-497: LGTM — clean schema definition.The
BuildServerMetadataschema is well-structured with all optional fields, consistent with the pattern used elsewhere (e.g.,GitMeta).apps/webapp/app/routes/api.v1.deployments.$deploymentId.progress.ts (1)
46-52: LGTM — straightforward field forwarding.The new
buildServerMetadatafield is correctly extracted from the validated body and passed through to the service, consistent with the existing pattern forcontentHash,gitMeta, andruntime.apps/webapp/app/v3/services/initializeDeployment.server.ts (1)
242-276: LGTM — well-guarded enqueue condition.The additional guards (
payload.artifactKey && !payload.skipEnqueue) correctly prevent enqueueing when the artifact is absent (GitHub integration path) or when explicitly skipped. The comment on line 242 provides helpful context.apps/webapp/app/v3/services/deployment.server.ts (3)
43-53: LGTM — clean signature extension and destructuring.The
updatesparameter is cleanly extended withbuildServerMetadata, and the destructuring on line 53 correctly separates it fromrestUpdatesto prevent passing it raw to Prisma (which would fail since it needs to be merged with existing metadata).
505-510: Good defensive parsing ofbuildServerMetadatafrom the database.Using
BuildServerMetadata.safeParse(...).datagracefully handles cases where the stored JSON doesn't conform to the schema (e.g., after schema evolution), returningundefinedrather than throwing.
5-10:⚠️ Potential issue | 🟡 MinorImport logger from
~/services/logger.serverinstead of@trigger.dev/core/v3.The logger should be sourced from
~/services/logger.server, which provides webapp-specific configuration including Sentry integration, sensitive data filtering, and HTTP context tracking. All other webapp service files (28/30 in the same directory) follow this pattern. Using the logger from@trigger.dev/core/v3bypasses essential logging configuration and context management.⛔ Skipped due to learnings
Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/webapp.mdc:0-0 Timestamp: 2025-11-27T16:26:58.661Z Learning: Applies to apps/webapp/app/v3/services/**/*.server.{ts,tsx} : Organize services in the webapp following the pattern `app/v3/services/*/*.server.ts`Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/webapp.mdc:0-0 Timestamp: 2025-11-27T16:26:58.661Z Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webappLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/webapp.mdc:0-0 Timestamp: 2025-11-27T16:26:58.661Z Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `trigger.dev/core` in the webapp, use subpath exports from the package.json instead of importing from the root pathLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasksLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/webapp.mdc:0-0 Timestamp: 2025-11-27T16:26:58.661Z Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/webapp.mdc:0-0 Timestamp: 2025-11-27T16:26:58.661Z Learning: Applies to apps/webapp/app/services/**/*.server.{ts,tsx} : Separate testable services from configuration files; follow the pattern of `realtimeClient.server.ts` (testable service) and `realtimeClientGlobal.server.ts` (configuration) in the webappLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasksLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger.config.ts : Use build extensions in trigger.config.ts (additionalFiles, additionalPackages, aptGet, prismaExtension, etc.) to customize the buildLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-15T11:50:06.067Z Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` patternLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-27T16:26:37.432Z Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webappLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use metadata methods (set, del, replace, append, remove, increment, decrement, stream, flush) to update metadata during task executionLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functionsLearnt from: myftija Repo: triggerdotdev/trigger.dev PR: 2392 File: packages/cli-v3/src/utilities/gitMeta.ts:195-218 Timestamp: 2025-08-14T12:13:20.455Z Learning: In the GitMeta schema (packages/core/src/v3/schemas/common.ts), all fields are intentionally optional to handle partial data from various deployment contexts (local, GitHub Actions, GitHub App). Functions like getGitHubAppMeta() are designed to work with missing environment variables rather than validate their presence.Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger.config.ts : Configure build process in trigger.config.ts using `build` object with external packages, extensions, and JSX settingsLearnt from: 0ski Repo: triggerdotdev/trigger.dev PR: 2994 File: apps/webapp/app/presenters/v3/BranchesPresenter.server.ts:45-45 Timestamp: 2026-02-03T18:27:05.229Z Learning: In the Vercel integration feature, the GitHub app is responsible for builds and provides git metadata (using source: "trigger_github_app"). The Vercel integration is only for linking deployments between platforms, not for triggering builds or providing git metadata.Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger.config.ts : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced loggingLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-11-27T16:27:35.304Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validationLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-15T11:50:06.067Z Learning: Applies to **/*.{ts,tsx,js,jsx} : Import from `trigger.dev/core` using subpaths only, never import from root
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const InitializeDeploymentRequestBodyBase = z.object({ | ||
| contentHash: z.string(), | ||
| userId: z.string().optional(), | ||
| /** @deprecated This is now determined by the webapp. This is only used to warn users with old CLI versions. */ | ||
| selfHosted: z.boolean().optional(), | ||
| gitMeta: GitMeta.optional(), | ||
| type: z.enum(["MANAGED", "UNMANAGED", "V1"]).optional(), | ||
| runtime: z.string().optional(), | ||
| initialStatus: z.enum(["PENDING", "BUILDING"]).optional(), | ||
| triggeredVia: DeploymentTriggeredVia.optional(), | ||
| buildId: z.string().optional(), | ||
| isNativeBuild: z.boolean().default(false).optional() | ||
| }) | ||
| export const InitializeDeploymentRequestBody = z.discriminatedUnion("isNativeBuild", [ | ||
| InitializeDeploymentRequestBodyBase.extend({ | ||
| isNativeBuild: z.literal(true), | ||
| skipPromotion: z.boolean().optional(), | ||
| artifactKey: z.string().optional(), | ||
| configFilePath: z.string().optional(), | ||
| skipEnqueue: z.boolean().default(false).optional(), | ||
| }), | ||
| InitializeDeploymentRequestBodyBase.extend({ | ||
| isNativeBuild: z.literal(false).optional(), | ||
| }) | ||
| .and( | ||
| z.preprocess( | ||
| (val) => { | ||
| const obj = val as any; | ||
| if (!obj || !obj.isNativeBuild) { | ||
| return { ...obj, isNativeBuild: false }; | ||
| } | ||
| return obj; | ||
| }, | ||
| z.discriminatedUnion("isNativeBuild", [ | ||
| z.object({ | ||
| isNativeBuild: z.literal(true), | ||
| skipPromotion: z.boolean(), | ||
| artifactKey: z.string(), | ||
| configFilePath: z.string().optional(), | ||
| }), | ||
| z.object({ | ||
| isNativeBuild: z.literal(false), | ||
| }), | ||
| ]) | ||
| ) | ||
| ); | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and read the specific section
fd -e ts "api.ts" packages/core/src/v3/schemas/Repository: triggerdotdev/trigger.dev
Length of output: 104
🏁 Script executed:
# Check the actual content of the file around lines 595-619
head -620 packages/core/src/v3/schemas/api.ts | tail -35 | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 1569
🏁 Script executed:
# Check Zod version in the repository
find . -name "package.json" -type f | xargs grep -l "zod" | grep -E "(^package\.json|packages/core/package\.json)" | head -5Repository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
# Get Zod version from packages/core
cat packages/core/package.json | grep -A 2 '"zod"'Repository: triggerdotdev/trigger.dev
Length of output: 153
🌐 Web query:
Zod z.default().optional() chain behavior output type
💡 Result:
In Zod, the order matters because .optional() short-circuits on undefined, while .default() only runs when it sees undefined.
z.string().optional().default("x")
- Runtime: If the input is
undefined/ missing,.optional()allows it through, then.default("x")replaces it with"x". - Types:
z.input<typeof schema>→string | undefinedz.infer<typeof schema>(output) →string(because the default guarantees a value)
This also explains why an object key can become required in the inferred type even though it can be omitted in the input. Zod distinguishes input vs output types; usez.inputwhen you want the “what can I pass in?” type. [1][2]
z.string().default("x").optional()
- Runtime: If the input is
undefined/ missing, the outer.optional()returnsundefinedimmediately, so the inner.default("x")never runs. - Types:
z.input<typeof schema>→string | undefinedz.infer<typeof schema>→string | undefined
- Net effect: the default is effectively not applied for missing/
undefinedinputs (you’ll getundefinedout). (This is the practical implication of optional short-circuiting before default can apply.) [2]
Rule of thumb
If you want “optional on input, but always defined on output”, prefer:
z.string().optional().default("x")…and use z.input<typeof ...> if you want the TypeScript type that allows the key to be omitted. [1][2]
Sources: Zod inference discussion/issue about .optional().default() and required keys in inferred output types [1], and explanation that default changes output vs input typing (use z.input) [2].
🌐 Web query:
Zod discriminatedUnion optional discriminator field z.literal(false).optional()
💡 Result:
z.discriminatedUnion(key, options) only works when every option is an object schema that has the discriminator key present with a single, non-optional literal value (so Zod can read that key and pick the right branch). The docs describe discriminated unions as object schemas that “all share a particular key”, which Zod checks to choose the schema. [1]
So this will not work as a discriminator:
z.literal(false).optional() // type is false | undefinedBecause the discriminator can be omitted (undefined), Zod can’t reliably select a branch; when the key is missing you’ll typically get invalid_union_discriminator (same error code you see for a wrong discriminator value). [2]
What to do instead
A) Make the discriminator required (recommended)
const S = z.discriminatedUnion("enabled", [
z.object({ enabled: z.literal(false), /* ... */ }),
z.object({ enabled: z.literal(true), /* ... */ }),
]);B) If you want to accept missing and treat it as false, preprocess and then discriminated-union
const S = z.preprocess((v) => {
if (v && typeof v === "object" && !Array.isArray(v) && !("enabled" in (v as any))) {
return { ...(v as any), enabled: false };
}
return v;
}, z.discriminatedUnion("enabled", [
z.object({ enabled: z.literal(false), /* ... */ }),
z.object({ enabled: z.literal(true), /* ... */ }),
]));C) If you truly need an optional discriminator, don’t use discriminatedUnion
Use z.union([...]) and add your own refinement/logic, because Zod can’t discriminate without a guaranteed discriminator key. [1]
Sources: [1] [2]
🏁 Script executed:
# Check if there are any TypeScript errors or runtime issues with this pattern
# Look for usage of InitializeDeploymentRequestBody in tests or elsewhere
rg "InitializeDeploymentRequestBody" packages/core --type ts -A 3 -B 1 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 1645
🏁 Script executed:
# Check Zod's type system for default().optional() behavior
# by looking at how the schema is actually used in the codebase
rg "InitializeDeploymentRequestBody" --type ts -lRepository: triggerdotdev/trigger.dev
Length of output: 280
🏁 Script executed:
# Check how InitializeDeploymentRequestBody is actually used
# Look for any validation attempts or error handling
rg "InitializeDeploymentRequestBody" -A 5 -B 1 packages/cli-v3/src/apiClient.ts | head -40Repository: triggerdotdev/trigger.dev
Length of output: 587
🏁 Script executed:
# Check the webapp usage
rg "InitializeDeploymentRequestBody" -A 5 -B 1 apps/webapp/app/v3/services/initializeDeployment.server.ts | head -40Repository: triggerdotdev/trigger.dev
Length of output: 893
🏁 Script executed:
# Look for test cases that might exercise this schema with isNativeBuild as false/undefined
rg -i "initializedeployment.*false|isNativeBuild.*false" --type ts -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 1456
🏁 Script executed:
# Check if there are any integration tests or actual usage that demonstrates the behavior
find . -type f -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "InitializeDeploymentRequestBody" 2>/dev/nullRepository: triggerdotdev/trigger.dev
Length of output: 53
🏁 Script executed:
# Test if the schema actually works with undefined isNativeBuild
# Create a test file to validate the behavior
cat > /tmp/test_schema.js << 'EOF'
const z = require('zod');
const InitializeDeploymentRequestBodyBase = z.object({
contentHash: z.string(),
isNativeBuild: z.boolean().default(false).optional()
});
const InitializeDeploymentRequestBody = z.discriminatedUnion("isNativeBuild", [
InitializeDeploymentRequestBodyBase.extend({
isNativeBuild: z.literal(true),
skipPromotion: z.boolean().optional(),
}),
InitializeDeploymentRequestBodyBase.extend({
isNativeBuild: z.literal(false).optional(),
})
]);
// Test 1: Both branches explicitly set
console.log("Test 1 - isNativeBuild: true");
try {
const result = InitializeDeploymentRequestBody.parse({ contentHash: "abc", isNativeBuild: true });
console.log("✓ Passed:", result);
} catch(e) {
console.log("✗ Failed:", e.message);
}
console.log("\nTest 2 - isNativeBuild: false");
try {
const result = InitializeDeploymentRequestBody.parse({ contentHash: "abc", isNativeBuild: false });
console.log("✓ Passed:", result);
} catch(e) {
console.log("✗ Failed:", e.message);
}
console.log("\nTest 3 - isNativeBuild: undefined (omitted)");
try {
const result = InitializeDeploymentRequestBody.parse({ contentHash: "abc" });
console.log("✓ Passed:", result);
} catch(e) {
console.log("✗ Failed:", e.message);
}
EOF
cd /tmp && node test_schema.jsRepository: triggerdotdev/trigger.dev
Length of output: 931
🏁 Script executed:
# Check if there's any documentation or comments about this pattern
rg "discriminatedUnion|isNativeBuild" packages/core/src/v3/schemas/api.ts -B 3 -A 10Repository: triggerdotdev/trigger.dev
Length of output: 2256
🏁 Script executed:
# Look for any recent changes or discussion around this schema
git log --oneline -20 -- packages/core/src/v3/schemas/api.ts 2>/dev/null | head -10Repository: triggerdotdev/trigger.dev
Length of output: 136
🏁 Script executed:
# Check if there are any type tests or validation tests
find . -path "*/node_modules" -prune -o -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) -print | xargs grep -l "isNativeBuild" 2>/dev/nullRepository: triggerdotdev/trigger.dev
Length of output: 53
Fix critical schema bug: z.discriminatedUnion() does not support optional discriminators.
Line 22 is missing a trailing semicolon (}) should be });).
More critically, the schema has a fundamental flaw: z.discriminatedUnion("isNativeBuild", [...]) requires the discriminator key to be present with a definite literal value. The second branch uses z.literal(false).optional(), which allows isNativeBuild to be undefined. When undefined, Zod cannot discriminate between branches and will fail with invalid_union_discriminator.
The base schema's isNativeBuild: z.boolean().default(false).optional() is also problematic—.optional() after .default() short-circuits before the default runs, so omitted values produce undefined, not false.
Recommended fix:
- Remove
isNativeBuildfrom the base schema entirely (both branches override it anyway). - Make the discriminator required in the second branch:
isNativeBuild: z.literal(false)(not optional). - Add the trailing semicolon.
All current callers explicitly pass isNativeBuild: false, so the bug is masked in practice, but the schema will fail if a caller omits the field.
🤖 Prompt for AI Agents
In `@packages/core/src/v3/schemas/api.ts` around lines 595 - 619, The
discriminated union InitializeDeploymentRequestBody is invalid because the
discriminator is defined optional in both InitializeDeploymentRequestBodyBase
(isNativeBuild: z.boolean().default(false).optional()) and the false branch
(isNativeBuild: z.literal(false).optional()), causing
invalid_union_discriminator when the field is omitted; fix by removing
isNativeBuild from InitializeDeploymentRequestBodyBase entirely, change the
false branch to require isNativeBuild: z.literal(false) (no .optional()), keep
the true branch as is (isNativeBuild: z.literal(true)), and add the missing
trailing semicolon to the export statement for InitializeDeploymentRequestBody.
1 participant
No description provided.