Conversation
|
WalkthroughAdds an Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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.
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx (1)
62-69: Consider extracting shared utilities.The
shuffleArrayfunction is duplicated inconfirm-basic-details.tsxand here. Similarly,safeParseStringArrayandsafeParseNumberArraycould be shared. Consider extracting these to a shared utility file for better maintainability.Also applies to: 195-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug_.projects.new/route.tsx around lines 62 - 69, Extract the duplicated utility functions shuffleArray, safeParseStringArray, and safeParseNumberArray into a single shared module (e.g., a utils file) and export them; then replace the local definitions in both confirm-basic-details.tsx and route.tsx with imports from that shared utility to eliminate duplication and keep behavior identical (preserve generics and return types), update any imports/usages accordingly, and run type checks to ensure nothing breaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug_.projects.new/route.tsx:
- Around line 62-69: Extract the duplicated utility functions shuffleArray,
safeParseStringArray, and safeParseNumberArray into a single shared module
(e.g., a utils file) and export them; then replace the local definitions in both
confirm-basic-details.tsx and route.tsx with imports from that shared utility to
eliminate duplication and keep behavior identical (preserve generics and return
types), update any imports/usages accordingly, and run type checks to ensure
nothing breaks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 405e81ce-fc18-4889-972b-eade0063caaf
📒 Files selected for processing (5)
apps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsx
📜 Review details
⏰ 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). (28)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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}: In TypeScript SDK usage, always import from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or use deprecated client.defineJob
Import from@trigger.dev/coresubpaths only, never from the root
Use the Run Engine 2.0 (@internal/run-engine) and redis-worker for all new work, not legacy V1 MarQS queue or deprecated V1 functions
Files:
apps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
{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/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
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/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
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 webapp
Files:
apps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
apps/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (apps/webapp/, apps/supervisor/, etc.) with no package changes, add a .server-changes/ file instead of a changeset
Files:
apps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 2964
File: apps/webapp/app/components/AskAI.tsx:121-141
Timestamp: 2026-01-28T16:57:47.620Z
Learning: In the trigger.dev webapp codebase, the Button component (apps/webapp/app/components/primitives/Buttons) does not spread unknown props to the underlying <button> element—it only passes specific props (type, disabled, onClick, name, value, ref, form, autoFocus). When using Radix UI's TooltipTrigger with asChild, a span wrapper around the Button is necessary to receive Radix props (aria-describedby, onPointerEnter, onPointerLeave, data-state) while the Button handles its own behavior. Directly making the Button the child of TooltipTrigger with asChild will break accessibility and tooltip functionality.
📚 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/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
📚 Learning: 2026-01-28T16:57:47.620Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 2964
File: apps/webapp/app/components/AskAI.tsx:121-141
Timestamp: 2026-01-28T16:57:47.620Z
Learning: In the trigger.dev webapp codebase, the Button component (apps/webapp/app/components/primitives/Buttons) does not spread unknown props to the underlying <button> element—it only passes specific props (type, disabled, onClick, name, value, ref, form, autoFocus). When using Radix UI's TooltipTrigger with asChild, a span wrapper around the Button is necessary to receive Radix props (aria-describedby, onPointerEnter, onPointerLeave, data-state) while the Button handles its own behavior. Directly making the Button the child of TooltipTrigger with asChild will break accessibility and tooltip functionality.
Applied to files:
apps/webapp/app/components/primitives/Buttons.tsx
📚 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: Use `useRun`, `useRealtimeRun` and other SWR/realtime hooks from `trigger.dev/react-hooks` for data fetching
Applied to files:
apps/webapp/app/components/primitives/Buttons.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/_app.orgs.new/route.tsxapps/webapp/app/routes/confirm-basic-details.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
📚 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/routes/confirm-basic-details.tsx
📚 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/routes/confirm-basic-details.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
📚 Learning: 2025-09-03T14:34:41.781Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:357-371
Timestamp: 2025-09-03T14:34:41.781Z
Learning: When using Zod's safeParse, the .data property is undefined when parsing fails, but TypeScript may still complain about accessing .data without checking .success first. The suggested approach of checking .success before accessing .data improves type safety and code clarity.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
📚 Learning: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: internal-packages/run-engine/src/engine/index.ts:466-467
Timestamp: 2025-08-14T10:09:02.528Z
Learning: In the triggerdotdev/trigger.dev codebase, it's acceptable to pass `string | undefined` types directly to Prisma operations (both create and update). The codebase consistently uses this pattern and the team is comfortable with how Prisma handles undefined values.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
🔇 Additional comments (13)
apps/webapp/app/components/onboarding/TechnologyPicker.tsx (1)
211-228: LGTM! Well-implemented case-insensitive matching logic.The
addCustomValuefunction correctly:
- Rejects empty input early
- Matches typed values case-insensitively against existing
TECHNOLOGY_OPTIONS- Adds matched options to
value(avoiding duplicates)- Falls back to adding unmatched values as custom entries (avoiding duplicates in both
customValuesandvalue)This addresses the PR objective of checking the corresponding list item when a typed custom option matches an existing option.
apps/webapp/app/components/primitives/Buttons.tsx (3)
210-219: LGTM! Good spinner delay implementation to prevent UI flicker.The 200ms delay before showing the spinner is a solid UX pattern that avoids flashing spinners for fast operations. The effect correctly:
- Resets
showSpinnerimmediately whenisLoadingbecomes false- Cleans up the timeout on unmount or when
isLoadingchanges
241-298: LGTM! Clever approach to preserve button dimensions during loading.Using
invisibleclass (instead of removing content) ensures the button maintains its width while the spinner overlays. The absolute positioning of the spinner centers it correctly within the button bounds.
330-348: LGTM! Proper disabled state handling during loading.The
isDisabledderivation correctly combinesdisabledandisLoading, ensuring both the button element and keyboard shortcuts are disabled during loading. This prevents accidental double-submissions.apps/webapp/app/routes/confirm-basic-details.tsx (4)
146-158: LGTM! Position data correctly nested within value checks.The logic ensures
referralSourcePositionandrolePositionare only stored when their corresponding main values (referralSource,role) exist, preventing orphaned position data.
330-338: LGTM! Position tracking hidden inputs correctly implemented.The 1-based position calculation (
indexOf + 1) properly captures where each option appeared in the randomized list, fulfilling the PR objective to persist randomized option positions.Also applies to: 370-374
411-418: LGTM! Correct usage of the newisLoadingprop.The button properly consumes
isSubmittingvia the newisLoadingprop, providing consistent loading feedback across the onboarding flow.
102-103: The current implementation already handles empty strings correctly via the falsy check.When no selection is made, the hidden input value is an empty string
"", whichz.coerce.number()coerces to0(notNaN). The subsequentif (submission.value.referralSourcePosition)andif (submission.value.rolePosition)checks on lines 146 and 156 prevent storing falsy values (including0), so the position is only stored when a valid selection exists. The code works as intended without additional preprocessing.> Likely an incorrect or invalid review comment.apps/webapp/app/routes/_app.orgs.new/route.tsx (1)
223-225: LGTM! Clean adoption of theisLoadingprop.The button correctly uses the existing
isLoadingstate that accounts for both submission and navigation loading states.apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx (4)
205-214: LGTM! Consistent parsing helper implementation.The
safeParseNumberArrayfunction follows the same pattern assafeParseStringArray, providing consistent error handling with graceful fallback toundefined.
219-223: LGTM! Position data correctly associated with parent values.The positions are only stored when their corresponding arrays (
workingOn,goals) have values, ensuring data integrity.Also applies to: 236-240
400-406: LGTM! Hidden inputs correctly serialize position arrays.The position arrays are computed using 1-based indexing and JSON-serialized for form submission. The server-side
safeParseNumberArraycorrectly returnsundefinedfor empty arrays (due to thelength > 0check), so empty selections won't create unnecessary data.Also applies to: 452-458
483-485: LGTM! ConsistentisLoadingprop usage.Aligns with the Button component changes and other routes updated in this PR.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/routes/confirm-basic-details.tsx (1)
142-157: Keep onboarding positions numeric in JSON.The new
String(...)conversions on Lines 147 and 157 make these fields inconsistent with the other onboarding position metadata, which is already stored as numbers / number arrays elsewhere. Persisting numeric indices as numbers here avoids mixed JSON shapes and simpler downstream reads.Suggested consistency fix
- const onboardingData: Record<string, string | undefined> = {}; + const onboardingData: Record<string, string | number | undefined> = {}; @@ - if (submission.value.referralSourcePosition) { - onboardingData.referralSourcePosition = String(submission.value.referralSourcePosition); + if (submission.value.referralSourcePosition && submission.value.referralSourcePosition > 0) { + onboardingData.referralSourcePosition = submission.value.referralSourcePosition; } @@ - if (submission.value.rolePosition) { - onboardingData.rolePosition = String(submission.value.rolePosition); + if (submission.value.rolePosition && submission.value.rolePosition > 0) { + onboardingData.rolePosition = submission.value.rolePosition; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/confirm-basic-details.tsx` around lines 142 - 157, The onboarding position fields are being converted to strings (String(...)) which creates mixed JSON shapes; instead preserve them as numbers and update the onboardingData type so it can hold numbers. Replace the String(...) conversions for submission.value.referralSourcePosition and submission.value.rolePosition with the raw numeric values (e.g., submission.value.referralSourcePosition and submission.value.rolePosition) and adjust the onboardingData type (the onboardingData declaration and any related type annotations) to allow number values (e.g., Record<string, string | number | undefined> or more specific property types) so downstream code sees consistent numeric indices.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/routes/confirm-basic-details.tsx`:
- Around line 102-103: The schema currently uses z.coerce.number().optional()
for referralSourcePosition and rolePosition which turns empty string "" into 0;
change each to use z.preprocess(val => val === "" ? undefined : val,
z.coerce.number().optional()) so blank hidden inputs become undefined instead of
0, then add bounds checks (e.g., .refine(n => n === undefined || (n >= 1 && n <=
<optionsLength>), { message: "position must be between 1 and the number of
options" }) to enforce 1-based indices within the length of the corresponding
shuffled option arrays (validate referralSourcePosition against the referral
source options length and rolePosition against the role options length).
---
Nitpick comments:
In `@apps/webapp/app/routes/confirm-basic-details.tsx`:
- Around line 142-157: The onboarding position fields are being converted to
strings (String(...)) which creates mixed JSON shapes; instead preserve them as
numbers and update the onboardingData type so it can hold numbers. Replace the
String(...) conversions for submission.value.referralSourcePosition and
submission.value.rolePosition with the raw numeric values (e.g.,
submission.value.referralSourcePosition and submission.value.rolePosition) and
adjust the onboardingData type (the onboardingData declaration and any related
type annotations) to allow number values (e.g., Record<string, string | number |
undefined> or more specific property types) so downstream code sees consistent
numeric indices.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 88f4d448-b045-4b4a-8180-712d8d0fca9c
📒 Files selected for processing (1)
apps/webapp/app/routes/confirm-basic-details.tsx
📜 Review details
⏰ 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 / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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}: In TypeScript SDK usage, always import from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or use deprecated client.defineJob
Import from@trigger.dev/coresubpaths only, never from the root
Use the Run Engine 2.0 (@internal/run-engine) and redis-worker for all new work, not legacy V1 MarQS queue or deprecated V1 functions
Files:
apps/webapp/app/routes/confirm-basic-details.tsx
{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/routes/confirm-basic-details.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/confirm-basic-details.tsx
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/routes/confirm-basic-details.tsx
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 webapp
Files:
apps/webapp/app/routes/confirm-basic-details.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/routes/confirm-basic-details.tsx
apps/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (apps/webapp/, apps/supervisor/, etc.) with no package changes, add a .server-changes/ file instead of a changeset
Files:
apps/webapp/app/routes/confirm-basic-details.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 2964
File: apps/webapp/app/components/AskAI.tsx:121-141
Timestamp: 2026-01-28T16:57:47.620Z
Learning: In the trigger.dev webapp codebase, the Button component (apps/webapp/app/components/primitives/Buttons) does not spread unknown props to the underlying <button> element—it only passes specific props (type, disabled, onClick, name, value, ref, form, autoFocus). When using Radix UI's TooltipTrigger with asChild, a span wrapper around the Button is necessary to receive Radix props (aria-describedby, onPointerEnter, onPointerLeave, data-state) while the Button handles its own behavior. Directly making the Button the child of TooltipTrigger with asChild will break accessibility and tooltip functionality.
📚 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/routes/confirm-basic-details.tsx
📚 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/routes/confirm-basic-details.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/confirm-basic-details.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/routes/confirm-basic-details.tsx
🔇 Additional comments (3)
apps/webapp/app/routes/confirm-basic-details.tsx (3)
7-7: LGTM: navigation-driven loading state.This keeps the submit button busy through the submit → redirect path without introducing another route-local request flag.
Also applies to: 212-213
330-338: LGTM: positional metadata follows the rendered order.Computing the hidden indices from
shuffledReferralSourcesandshuffledRoleskeeps the recorded position aligned with what the user actually saw.Also applies to: 370-374
411-416: LGTM: primary CTA is using the shared loading API.Switching this route to
Button.isLoadingcentralizes spinner/disabled behavior and matches the rest of the rollout.
| referralSourcePosition: z.coerce.number().optional(), | ||
| rolePosition: z.coerce.number().optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Zod v3.25 z.coerce.number().optional() empty string behavior and best practices
💡 Result:
What happens in Zod v3.25 with z.coerce.number().optional() and "" (empty string)
z.coerce.number()coerces using JavaScript’sNumber(input). (zod.dev)- In JavaScript,
Number("") === 0, so parsing""becomes0(notundefined), and then passes number validation. (zod.dev) .optional()only makes the schema acceptundefined(i.e., “missing”), not"". This is consistent with Zod’s “optional means undefined” design. (github.com)
So:
z.coerce.number().optional().parse("") // => 0This is often surprising in HTML form scenarios because an “empty” input can incorrectly validate as 0 (especially bad when the field is “required”). (github.com)
Best-practice patterns (pick based on what “empty” should mean)
1) If empty should mean “missing” → convert "" to undefined before number parsing (recommended)
const optionalNumberFromForm = z.preprocess(
(v) => (v === "" ? undefined : v),
z.coerce.number().optional()
);Now "" becomes undefined, and real numbers still coerce.
If you also want to treat whitespace as empty:
const optionalNumberFromForm = z.preprocess(
(v) => (typeof v === "string" && v.trim() === "" ? undefined : v),
z.coerce.number().optional()
);Related ecosystem tooling: Conform’s coerceFormValue applies a similar rule by default (“empty string => undefined”). (conform.guide)
2) If the field is required, and empty should fail as “required” (not silently become 0)
Same preprocessing, but don’t add .optional():
const requiredNumberFromForm = z.preprocess(
(v) => (v === "" ? undefined : v),
z.coerce.number({ required_error: "Required" })
);Because the empty value becomes undefined, Zod can raise your required_error instead of accepting 0. (zod.dev)
3) If you want to forbid coercing "" to 0 even for optional fields, but keep string inputs
Use a string-first approach and enforce non-empty before transforming:
const num = z
.union([
z.coerce.string().min(1).transform((s) => Number(s)),
z.undefined(),
]);This avoids the Number("") === 0 trap by never calling Number() on empty input. (This general approach is discussed as a workaround in the Zod community.) (github.com)
Rule of thumb
- Avoid
z.coerce.number().optional()directly on raw form values if the form can produce"". - Do preprocess
""→undefined(or use a form helper that does it) when""semantically means “not provided”. (conform.guide)
Citations:
- 1: https://zod.dev/api?utm_source=openai
- 2: https://zod.dev/api?utm_source=openai
- 3: Optional email string schema fails on parsing empty string colinhacks/zod#2513
- 4: z.coerce.number() defaults empty strings to 0 colinhacks/zod#2814
- 5: https://conform.guide/api/zod/coerceFormValue?utm_source=openai
- 6: https://zod.dev/api?utm_source=openai
- 7: z.coerce.number() defaults empty strings to 0 colinhacks/zod#2814
- 8: https://conform.guide/api/zod/coerceFormValue?utm_source=openai
🏁 Script executed:
# First, find and examine the file to see the actual implementation
cd /tmp && git -C /tmp/repo rev-parse --show-toplevel 2>/dev/null || echo "no git repo"
# Get the actual file content around the relevant lines
find . -name "confirm-basic-details.tsx" -type f 2>/dev/null | head -5Repository: triggerdotdev/trigger.dev
Length of output: 80
🏁 Script executed:
# Find the repo and inspect the actual file
find / -type d -name "trigger.dev" 2>/dev/null | head -1 | xargs -I {} cat {}/apps/webapp/app/routes/confirm-basic-details.tsx 2>/dev/null | head -200Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Alternative: search for the file in common locations and examine it
if [ -d "apps/webapp/app/routes" ]; then
echo "=== File found in current context ==="
head -150 apps/webapp/app/routes/confirm-basic-details.tsx
echo ""
echo "=== Lines 100-110 context ==="
sed -n '95,115p' apps/webapp/app/routes/confirm-basic-details.tsx
echo ""
echo "=== Lines 325-375 context (hidden inputs) ==="
sed -n '325,375p' apps/webapp/app/routes/confirm-basic-details.tsx
else
find . -name "confirm-basic-details.tsx" 2>/dev/null
fiRepository: triggerdotdev/trigger.dev
Length of output: 8047
Treat blank positional inputs as undefined, not 0.
The hidden inputs submit "" when no value is chosen. Since z.coerce.number() coerces empty strings to 0 (per JavaScript's Number("") === 0), the current schema parses "" to 0 instead of undefined. While the code happens to work—because 0 is falsy and the position is not stored—this is semantically incorrect and relies on implicit falsy behavior. Additionally, there is no validation to ensure submitted position indices are within valid bounds (1-based, within the length of the shuffled options).
Use z.preprocess() to convert empty strings to undefined before coercion, and add bounds validation for the index range.
Suggested validation tightening
- referralSourcePosition: z.coerce.number().optional(),
- rolePosition: z.coerce.number().optional(),
+ referralSourcePosition: z.preprocess(
+ (value) => (value === "" ? undefined : value),
+ z.coerce.number().int().min(1).max(referralSourceOptions.length).optional()
+ ),
+ rolePosition: z.preprocess(
+ (value) => (value === "" ? undefined : value),
+ z.coerce.number().int().min(1).max(roleOptions.length).optional()
+ ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/routes/confirm-basic-details.tsx` around lines 102 - 103, The
schema currently uses z.coerce.number().optional() for referralSourcePosition
and rolePosition which turns empty string "" into 0; change each to use
z.preprocess(val => val === "" ? undefined : val, z.coerce.number().optional())
so blank hidden inputs become undefined instead of 0, then add bounds checks
(e.g., .refine(n => n === undefined || (n >= 1 && n <= <optionsLength>), {
message: "position must be between 1 and the number of options" }) to enforce
1-based indices within the length of the corresponding shuffled option arrays
(validate referralSourcePosition against the referral source options length and
rolePosition against the role options length).
1 participant
Fixes and improvements to the onboarding questions:
This change is worth double checking @matt-aitken
isLoadingthat shows a spinner in the middle of the button (replacing the button text and any icons) and sets it todisabled. It does this nicely by keeping the button width the same so there's no layout shift.Other fixes