feat(webapp): expose project git settings #2464
Conversation
|
WalkthroughAdds end-to-end GitHub integration: a new ConnectedGithubRepository Prisma model and migration; database schema and relation updates for Project and GithubRepository; backend services and presenters (ProjectSettingsService, ProjectSettingsPresenter, DeploymentListPresenter) to connect/disconnect repos, manage branch-tracking and preview settings, and validate branches via checkGitHubBranchExists; Zod BranchTrackingConfigSchema and getTrackedBranchForEnvironment utilities; new route actions/loaders and UI changes for deployments and project settings; GitHub App install path helper; cookie-backed success messaging; and a neverthrow dependency. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
myftija
force-pushed
the
project-git-settings
branch
from
5e66deb to
3ca1a58
Compare
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
myftija
force-pushed
the
gh-app-installation-flow
branch
from
8c5ebd1 to
2809981
Compare
myftija
force-pushed
the
project-git-settings
branch
from
3ca1a58 to
953b201
Compare
myftija
force-pushed
the
gh-app-installation-flow
branch
from
441d01a to
d07e605
Compare
myftija
force-pushed
the
project-git-settings
branch
from
953b201 to
704e68f
Compare
myftija
force-pushed
the
project-git-settings
branch
from
ff5a0b7 to
86a98e2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
apps/webapp/app/v3/github.ts (1)
1-27: Make schema resilient to partially-populated JSON and trim branch names.DB JSON may miss prod/staging keys; current schema requires both. Default and optional fields prevent parse failures, and trim avoids whitespace-only values.
-export const BranchTrackingConfigSchema = z.object({ - prod: z.object({ - branch: z.string().optional(), - }), - staging: z.object({ - branch: z.string().optional(), - }), -}); +export const BranchTrackingConfigSchema = z + .object({ + prod: z + .object({ + branch: z.string().trim().min(1).optional(), + }) + .optional() + .default({}), + staging: z + .object({ + branch: z.string().trim().min(1).optional(), + }) + .optional() + .default({}), + }) + .default({ prod: {}, staging: {} });internal-packages/database/prisma/migrations/20250902135000_add_gh_connected_repo_schema/migration.sql (1)
1-20: Consider a default for branchTracking and clarify updatedAt behavior.
- Without a default, inserts must always supply branchTracking. A JSON default reduces footguns.
- Ensure Prisma model marks updatedAt with @updatedat so it auto-updates.
Suggested migration follow-up:
ALTER TABLE "public"."ConnectedGithubRepository" ALTER COLUMN "branchTracking" SET DEFAULT '{"prod":{},"staging":{}}'::jsonb;Please confirm the Prisma schema uses @updatedat on updatedAt. If not, add it.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
129-138: Minor: memoize derived branch.useMemo avoids re-running safeParse and mapping on every render.
- const branchTrackingOrError = - connectedGithubRepository && - BranchTrackingConfigSchema.safeParse(connectedGithubRepository.branchTracking); - const environmentGitHubBranch = - branchTrackingOrError && branchTrackingOrError.success - ? getTrackedBranchForEnvironment(branchTrackingOrError.data, environment.type) - : undefined; + const environmentGitHubBranch = useMemo(() => { + if (!connectedGithubRepository) return undefined; + const parsed = BranchTrackingConfigSchema.safeParse(connectedGithubRepository.branchTracking); + return parsed.success + ? getTrackedBranchForEnvironment(parsed.data, environment.type) + : undefined; + }, [connectedGithubRepository, environment.type]);apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (3)
351-353: Guard against BigInt→Number precision loss for installation IDs.GitHub IDs can exceed 2^53-1 in theory. Either pass a string/BigInt through, or assert it’s within safe range before Number().
- const installationId = Number(existingConnection.repository.installation.appInstallationId); + const rawInstallationId = existingConnection.repository.installation.appInstallationId; + const installationId = + typeof rawInstallationId === "bigint" ? Number(rawInstallationId) : rawInstallationId; + if (typeof rawInstallationId === "bigint" && rawInstallationId > BigInt(Number.MAX_SAFE_INTEGER)) { + return redirectBackWithErrorMessage(request, "Unsupported GitHub installation id size"); + }Alternatively, update checkGitHubBranchExists to accept a string ID.
707-712: Optional: close modal via redirect instead of action JSON.If you adopt redirect-on-success, this effect is no longer needed.
- useEffect(() => { - if (lastSubmission && "success" in lastSubmission && lastSubmission.success === true) { - setIsModalOpen(false); - } - }, [lastSubmission]); + // No-op if action redirects on success; modal will close on navigation.
734-735: Controlled Select: drop defaultValue when value is provided.Avoid mixing controlled and uncontrolled props.
- value={selectedInstallation?.id} - defaultValue={gitHubAppInstallations.at(0)?.id} + value={selectedInstallation?.id}- value={selectedRepository ? selectedRepository.id : undefined} + value={selectedRepository?.id}Also applies to: 785-786
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(5 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(8 hunks)apps/webapp/app/services/gitHub.server.ts(2 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)apps/webapp/app/v3/github.ts(1 hunks)internal-packages/database/prisma/migrations/20250902135000_add_gh_connected_repo_schema/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/v3/github.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/services/gitHub.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/v3/github.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/services/gitHub.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/v3/github.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/services/gitHub.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/v3/github.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/services/gitHub.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/services/gitHub.server.ts
🧠 Learnings (3)
📚 Learning: 2025-09-02T11:18:06.562Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.562Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
Applied to files:
apps/webapp/app/utils/pathBuilder.ts
📚 Learning: 2025-07-18T17:49:24.468Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL
Applied to files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts
📚 Learning: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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/presenters/v3/DeploymentListPresenter.server.ts
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 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 (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 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 (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (3)
2-2: Type-only Prisma import: LGTM.Reduces bundle size and matches usage.
59-69: Selecting connectedGithubRepository fields: LGTM.The minimal shape (branchTracking, repository htmlUrl/fullName) is appropriate for UI.
157-157: API shape change—verify downstream usage.Adding connectedGithubRepository to the return type may require loader/UI updates. Ensure all consumers are aligned.
You can grep for DeploymentListPresenter.call consumers and check for destructuring assumptions.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
129-138: Solid, defensive parsing for branch tracking.Safe-parse with zod and environment mapping look good and won’t explode on bad JSON.
internal-packages/database/prisma/schema.prisma (3)
375-397: One-to-one link looks correct.connectedGithubRepository? on Project with @@unique([projectId]) enforces the intended cardinality.
2286-2289: Relation back from GithubRepository is fine.Bidirectional relation enables efficient lookups.
2294-2321: Model shape LGTM; confirm multi-project-to-same-repo policy.Storing branchTracking as JSON is pragmatic given current needs. Verify whether you want to allow the same repository to be connected to multiple projects (current schema permits it); if not, add a unique constraint on repositoryId.
Would you like to enforce “one project per repository” at the DB layer?
I can draft the migration if you confirm the policy.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
389-399: Confirm intended merge semantics for branchTracking updates.Current write replaces missing fields with {} (effectively clearing them). If you want to preserve existing values when inputs are left untouched, merge with previous JSON instead of overwriting.
I can provide a merge patch that preserves existing prod/staging branches unless explicitly changed.
...outes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
Outdated
Show resolved
Hide resolved
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
318-333: Add rel="noreferrer noopener" to external link.Prevents tabnabbing and referrer leakage.
- <a + <a href={connectedGithubRepository.repository.htmlUrl} target="_blank" + rel="noreferrer noopener" className="max-w-52 truncate text-sm text-text-dimmed underline transition-colors hover:text-text-bright" >apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (4)
18-19: Import LoaderFunctionArgs from server runtime.-import { type LoaderFunctionArgs } from "@remix-run/router"; +import { type LoaderFunctionArgs } from "@remix-run/server-runtime";
80-81: Use subpath import for tryCatch per webapp guideline.-import { tryCatch } from "@trigger.dev/core"; +import { tryCatch } from "@trigger.dev/core/utils";
408-458: Redirect after connect and validate repository against selected installation.Prevents stale UI and ensures repo belongs to the chosen installation.
- case "connect-repo": { - const { repositoryId } = submission.value; + case "connect-repo": { + const { repositoryId, installationId } = submission.value; @@ - const [repository, existingConnection] = await Promise.all([ + const [repository, existingConnection] = await Promise.all([ prisma.githubRepository.findFirst({ where: { id: repositoryId, + installationId, // must belong to selected installation installation: { organizationId: project.organizationId, }, }, select: { id: true, name: true, defaultBranch: true, }, }), @@ await prisma.connectedGithubRepository.create({ data: { projectId: project.id, repositoryId: repositoryId, branchTracking: { prod: { branch: repository.defaultBranch }, staging: { branch: repository.defaultBranch }, } satisfies BranchTrackingConfig, previewDeploymentsEnabled: true, }, }); - - return json({ - ...submission, - success: true, - }); + return redirectBackWithSuccessMessage(request, "Repository connected successfully"); }
959-966: Add rel="noreferrer noopener" to external repo link.<a href={connectedGitHubRepo.repository.htmlUrl} target="_blank" + rel="noreferrer noopener" className="max-w-52 truncate text-sm text-text-bright hover:underline" >
🧹 Nitpick comments (3)
apps/webapp/app/v3/github.ts (1)
3-10: Harden schema to tolerate partially-populated JSON.If legacy rows are missing prod/staging keys, safeParse fails. Make keys optional with defaults so downstream code can still work.
-export const BranchTrackingConfigSchema = z.object({ - prod: z.object({ - branch: z.string().optional(), - }), - staging: z.object({ - branch: z.string().optional(), - }), -}); +export const BranchTrackingConfigSchema = z + .object({ + prod: z.object({ branch: z.string().optional() }).default({}), + staging: z.object({ branch: z.string().optional() }).default({}), + }) + .partial() + .transform((v) => ({ prod: v.prod ?? {}, staging: v.staging ?? {} }));apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (2)
136-149: Compute environment branch even if branchTracking parsing fails.This lets PREVIEW work when branchTracking JSON is absent/malformed (after fixing getTrackedBranchForEnvironment).
- const branchTrackingOrError = - connectedGithubRepository && - BranchTrackingConfigSchema.safeParse(connectedGithubRepository.branchTracking); - const environmentGitHubBranch = - branchTrackingOrError && branchTrackingOrError.success - ? getTrackedBranchForEnvironment( - branchTrackingOrError.data, - connectedGithubRepository.previewDeploymentsEnabled, - { - type: environment.type, - branchName: environment.branchName ?? undefined, - } - ) - : undefined; + const branchTrackingOrError = connectedGithubRepository + ? BranchTrackingConfigSchema.safeParse(connectedGithubRepository.branchTracking) + : undefined; + const environmentGitHubBranch = connectedGithubRepository + ? getTrackedBranchForEnvironment( + branchTrackingOrError?.success ? branchTrackingOrError.data : undefined, + connectedGithubRepository.previewDeploymentsEnabled, + { + type: environment.type, + branchName: environment.branchName ?? undefined, + } + ) + : undefined;
157-165: Complete useEffect deps to avoid stale values.Include pathname, currentPage, and navigate.
- }, [selectedDeployment, deploymentParam, location.search]); + }, [selectedDeployment, deploymentParam, location.search, location.pathname, currentPage, navigate]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(5 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(8 hunks)apps/webapp/app/v3/github.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/v3/github.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/v3/github.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/v3/github.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/v3/github.ts
🧠 Learnings (2)
📚 Learning: 2025-08-29T10:06:49.283Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-08-29T10:06:49.283Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from trigger.dev/core’s package.json
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
52-55: Don’t log the OAuthstatetoken (redactcallbackData)
stateis sensitive. Log only non-sensitive fields.Apply these diffs:
logger.error("GitHub App callback with invalid session", { - callbackData, + action: callbackData.setup_action, + installationId: + "installation_id" in callbackData ? callbackData.installation_id : undefined, error: sessionResult.error, });-logger.info("GitHub App installation requested, awaiting approval", { - callbackData, -}); +logger.info("GitHub App installation requested, awaiting approval", { + action: callbackData.setup_action, +});Also applies to: 128-131
♻️ Duplicate comments (6)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (6)
18-19: Good fix: server runtime types for loader/action.Importing LoaderFunctionArgs and ActionFunction from @remix-run/server-runtime aligns with Remix server context.
80-80: Correct subpath import for tryCatch.Using @trigger.dev/core/utils follows the webapp guideline to avoid root imports.
833-840: Good hardening: rel="noreferrer noopener" on external GitHub link.Prevents tabnabbing.
978-986: Good hardening: rel="noreferrer noopener" on repo link.External anchor safely opened in new tab.
363-376: Fix type error: safeParse data accessed without success guard.Accessing
.dataon a failed parse is a TS error and can mask logic issues.Apply this diff:
- const existingBranchTracking = BranchTrackingConfigSchema.safeParse( - existingConnection.branchTracking - ); - - const [error, branchValidationsOrFail] = await tryCatch( + const existingBranchTracking = BranchTrackingConfigSchema.safeParse( + existingConnection.branchTracking + ); + const existingProdBranch = existingBranchTracking.success + ? existingBranchTracking.data.prod?.branch + : undefined; + const existingStagingBranch = existingBranchTracking.success + ? existingBranchTracking.data.staging?.branch + : undefined; + + const [error, branchValidationsOrFail] = await tryCatch( Promise.all([ - productionBranch && existingBranchTracking.data?.prod?.branch !== productionBranch + productionBranch && existingProdBranch !== productionBranch ? checkGitHubBranchExists(installationId, owner, repo, productionBranch) : Promise.resolve(true), - stagingBranch && existingBranchTracking.data?.staging?.branch !== stagingBranch + stagingBranch && existingStagingBranch !== stagingBranch ? checkGitHubBranchExists(installationId, owner, repo, stagingBranch) : Promise.resolve(true), ]) );
461-465: Return a redirect after connecting to refresh loader data and UI.JSON leaves the page stale; a redirect revalidates and swaps to the “connected” view.
Apply this diff:
- return json({ - ...submission, - success: true, - }); + return redirectBackWithSuccessMessage(request, "Repository connected successfully");
🧹 Nitpick comments (8)
apps/webapp/app/routes/_app.github.callback/route.tsx (7)
6-10: Consistent message handling: consider aligning error pathsYou’ve moved success paths to request-scoped session +
commitSessionwhile errors still useredirectWithErrorMessage. For consistency and to enable multiple Set-Cookie headers when needed, consider migrating error paths to the same pattern.
16-31: Validatestateis non-emptyHarden the callback by requiring a non-empty
state.Apply this diff:
const QuerySchema = z.discriminatedUnion("setup_action", [ z.object({ setup_action: z.literal("install"), installation_id: z.coerce.number(), - state: z.string(), + state: z.string().min(1), }), z.object({ setup_action: z.literal("update"), installation_id: z.coerce.number(), - state: z.string(), + state: z.string().min(1), }), z.object({ setup_action: z.literal("request"), - state: z.string(), + state: z.string().min(1), }), ]);
64-70: UnnecessaryorderBywith exactidfilter
orderByis redundant when filtering by a uniqueid. Removing it simplifies the query.const org = await $replica.organization.findFirst({ where: { id: organizationId, members: { some: { userId: user.id } }, deletedAt: null }, - orderBy: { createdAt: "desc" }, select: { id: true, }, });
95-102: UseHeaders.appendso you can set multiple cookiesFuture-proof the response (e.g., if you later clear the install-session cookie). Also aligns with learnings from the previous PR where cookie clearing was constrained by headers; this removes that constraint.
- return redirect(redirectTo, { - headers: { - "Set-Cookie": await commitSession(session), - }, - }); + const headers = new Headers(); + headers.append("Set-Cookie", await commitSession(session)); + return redirect(redirectTo, { headers });
115-122: Same multi-cookie improvement for the update pathMirror the success case pattern using
Headers.- return redirect(redirectTo, { - headers: { - "Set-Cookie": await commitSession(session), - }, - }); + const headers = new Headers(); + headers.append("Set-Cookie", await commitSession(session)); + return redirect(redirectTo, { headers });
132-138: Same multi-cookie improvement for the request pathKeeps all success paths consistent and ready for multiple cookies.
- return redirect(redirectTo, { - headers: { - "Set-Cookie": await commitSession(session), - }, - }); + const headers = new Headers(); + headers.append("Set-Cookie", await commitSession(session)); + return redirect(redirectTo, { headers });
40-45: Optional: unify error redirectsIf you later need to clear the install-session cookie on failures, migrate these to
setRequest...Message+commitSession+redirectwithHeaders. Otherwise, keepingredirectWithErrorMessageis fine.Also applies to: 57-58, 79-80, 92-93, 112-113, 143-144
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
725-731: Optional: remove modal-closing effect; redirect makes it unnecessary.After switching to a redirect, the loader revalidates and the modal is not rendered anymore.
Apply this diff:
- useEffect(() => { - if (lastSubmission && "success" in lastSubmission && lastSubmission.success === true) { - setIsModalOpen(false); - } - }, [lastSubmission]); + // No-op: redirect after connect handles closing via route reload.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/webapp/app/routes/_app.github.callback/route.tsx(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(5 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(8 hunks)apps/webapp/app/services/gitHub.server.ts(2 hunks)apps/webapp/app/v3/github.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
- apps/webapp/app/services/gitHub.server.ts
- apps/webapp/app/v3/github.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/routes/_app.github.callback/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/_app.github.callback/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.github.callback/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧠 Learnings (5)
📚 Learning: 2025-09-02T11:27:36.319Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.319Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Applied to files:
apps/webapp/app/routes/_app.github.callback/route.tsx
📚 Learning: 2025-09-02T11:18:06.562Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.562Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
Applied to files:
apps/webapp/app/routes/_app.github.callback/route.tsx
📚 Learning: 2025-08-29T10:06:49.283Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-08-29T10:06:49.283Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from trigger.dev/core’s package.json
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-09-03T14:34:41.730Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2464
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:357-371
Timestamp: 2025-09-03T14:34:41.730Z
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.$projectParam.env.$envParam.settings/route.tsx
🧬 Code graph analysis (2)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
apps/webapp/app/models/message.server.ts (1)
setRequestSuccessMessage(66-76)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (7)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
loader(33-145)apps/webapp/app/utils/pathBuilder.ts (3)
EnvironmentParamSchema(26-28)githubAppInstallPath(144-146)v3ProjectSettingsPath(416-422)apps/webapp/app/models/project.server.ts (1)
findProjectBySlug(109-120)apps/webapp/app/v3/github.ts (2)
BranchTrackingConfigSchema(3-10)BranchTrackingConfig(12-12)apps/webapp/app/models/message.server.ts (2)
redirectBackWithSuccessMessage(209-216)redirectBackWithErrorMessage(200-207)apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization(39-43)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment(19-23)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 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 (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- 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: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
1-1: Good switch to RemixredirectEnables attaching cookie headers on success paths.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
329-334: Unique constraint confirmed on ConnectedGithubRepository.projectId. The Prisma schema defines@@unique([projectId])onConnectedGithubRepositoryand the migration creates a unique index onprojectId, so usingdelete({ where: { projectId } })is safe.
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
378-391: Fix unsafe access to safeParse.data; check success firstAccessing existingBranchTracking.data without guarding on .success can fail typecheck and mask logic errors.
- const existingBranchTracking = BranchTrackingConfigSchema.safeParse( - existingConnection.branchTracking - ); - - const [error, branchValidationsOrFail] = await tryCatch( + const existingBranchTracking = BranchTrackingConfigSchema.safeParse( + existingConnection.branchTracking + ); + const existingProdBranch = existingBranchTracking.success + ? existingBranchTracking.data.prod?.branch + : undefined; + const existingStagingBranch = existingBranchTracking.success + ? existingBranchTracking.data.staging?.branch + : undefined; + + const [error, branchValidationsOrFail] = await tryCatch( Promise.all([ - productionBranch && existingBranchTracking.data?.prod?.branch !== productionBranch + productionBranch && existingProdBranch !== productionBranch ? checkGitHubBranchExists(installationId, owner, repo, productionBranch) : Promise.resolve(true), - stagingBranch && existingBranchTracking.data?.staging?.branch !== stagingBranch + stagingBranch && existingStagingBranch !== stagingBranch ? checkGitHubBranchExists(installationId, owner, repo, stagingBranch) : Promise.resolve(true), ]) );
🧹 Nitpick comments (5)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (5)
265-269: Parse params with the shared schema for consistency and validationUse EnvironmentParamSchema in the action like the loader does to avoid ad-hoc null checks and keep param handling consistent.
- const { organizationSlug, projectParam } = params; - if (!organizationSlug || !projectParam) { - return json({ errors: { body: "organizationSlug is required" } }, { status: 400 }); - } + const { organizationSlug, projectParam } = EnvironmentParamSchema.parse(params);
283-303: DRY project lookup by reusing findProjectBySlugAction duplicates the membership-scoped project lookup that the loader already centralizes via findProjectBySlug.
- const project = await prisma.project.findFirst({ - where: { - slug: projectParam, - organization: { - members: { - some: { - userId, - }, - }, - }, - }, - select: { - id: true, - organizationId: true, - }, - }); - - if (!project) { - return json({ errors: { body: "project not found" } }, { status: 404 }); - } + const project = await findProjectBySlug(organizationSlug, projectParam, userId); + if (!project) { + return json({ errors: { body: "project not found" } }, { status: 404 }); + }
344-353: Make disconnect idempotentUse deleteMany to avoid throwing when no connection exists (race-safe and idempotent).
- await prisma.connectedGithubRepository.delete({ - where: { - projectId: project.id, - }, - }); + await prisma.connectedGithubRepository.deleteMany({ + where: { projectId: project.id }, + });
740-745: Remove modal-close effect; rely on redirect flowWith a redirect on success, this effect becomes dead code and can be removed.
- const lastSubmission = useActionData() as any; @@ - useEffect(() => { - if (lastSubmission && "success" in lastSubmission && lastSubmission.success === true) { - setIsModalOpen(false); - } - }, [lastSubmission]);
508-509: Avoid casting to any in formsYou can type lastSubmission using Conform’s inferred submission type to preserve safety.
Example:
const [renameForm, { projectName }] = useForm<typeof createSchema>({ // ... lastSubmission: lastSubmission as Submission<typeof createSchema> | undefined, });Also applies to: 524-525
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(8 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/utils/pathBuilder.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧠 Learnings (5)
📚 Learning: 2025-08-29T10:06:49.283Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-08-29T10:06:49.283Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from trigger.dev/core’s package.json
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-09-03T14:34:41.730Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2464
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:357-371
Timestamp: 2025-09-03T14:34:41.730Z
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.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-09-02T11:27:36.319Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.319Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-09-02T11:18:06.562Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.562Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (9)
apps/webapp/app/services/session.server.ts (1)
requireUserId(25-35)apps/webapp/app/utils/pathBuilder.ts (3)
EnvironmentParamSchema(26-28)githubAppInstallPath(144-148)v3ProjectSettingsPath(418-424)apps/webapp/app/models/project.server.ts (1)
findProjectBySlug(109-120)apps/webapp/app/v3/github.ts (2)
BranchTrackingConfigSchema(3-10)BranchTrackingConfig(12-12)apps/webapp/app/models/message.server.ts (2)
redirectBackWithSuccessMessage(209-216)redirectBackWithErrorMessage(200-207)apps/webapp/app/services/gitHub.server.ts (1)
checkGitHubBranchExists(141-180)apps/webapp/app/hooks/useProject.tsx (1)
useProject(20-24)apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization(39-43)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment(19-23)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (3)
18-18: Correct server runtime import — LGTMUsing LoaderFunctionArgs and ActionFunction from @remix-run/server-runtime is correct for server modules.
96-112: Flash session is correctly consumed and committed in the loader — LGTMYou read the flash, commit the session, and propagate Set-Cookie on all loader responses.
846-856: External links hardened — LGTMAdding rel="noreferrer noopener" for target="_blank" links prevents tabnabbing.
Also applies to: 993-997
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
Outdated
Show resolved
Hide resolved
...outes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
341-378: Redirect on successful connect to revalidate loader and update UIReturning JSON leaves the page stale (prompt remains visible). Redirecting triggers loader revalidation so the connected state renders immediately.
Apply:
- return json({ - ...submission, - success: true, - }); + return redirectBackWithSuccessMessage( + request, + "GitHub repository connected successfully" + );
🧹 Nitpick comments (5)
apps/webapp/app/services/projectSettingsPresenter.server.ts (1)
15-26: Add an explicit return type to stabilize this presenter’s API.Locking the ResultAsync shape prevents accidental widening during future refactors and improves call-site DX.
-import { err, fromPromise, ok, okAsync } from "neverthrow"; +import { err, fromPromise, ok, okAsync, type ResultAsync } from "neverthrow"; + +type GitHubAppSettings = + | { + enabled: false; + connectedRepository: undefined; + installations: undefined; + } + | { + enabled: true; + connectedRepository?: + | { + branchTracking?: import("~/v3/github").BranchTrackingConfig; + previewDeploymentsEnabled: boolean; + createdAt: Date; + repository: { + id: string; + name: string; + fullName: string; + htmlUrl: string; + private: boolean; + }; + } + | undefined; + installations?: + | Array<{ + id: string; + accountHandle: string | null; + targetType: string; + appInstallationId: number; + repositories: Array<{ + id: string; + name: string; + fullName: string; + htmlUrl: string; + private: boolean; + }>; + }> + | undefined; + }; + +type GetProjectSettingsResult = ResultAsync< + { gitHubApp: GitHubAppSettings }, + | { type: "project_not_found" } + | { type: "other"; cause?: unknown } +>; export class ProjectSettingsPresenter { @@ - getProjectSettings(organizationSlug: string, projectSlug: string, userId: string) { + getProjectSettings( + organizationSlug: string, + projectSlug: string, + userId: string + ): GetProjectSettingsResult {Also applies to: 120-145
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (4)
837-842: createdAt should be a string (JSON-serialized Date)typedjson/json serializes Date to ISO string. Update the prop type to avoid type drift during hydration.
Apply:
type ConnectedGitHubRepo = { branchTracking: BranchTrackingConfig | undefined; previewDeploymentsEnabled: boolean; - createdAt: Date; + createdAt: string; repository: GitHubRepository; };
636-641: Remove modal-close via action data; rely on redirect insteadOnce the connect action redirects, this effect is unnecessary and can cause odd UX if other actions run.
Apply:
- useEffect(() => { - if (lastSubmission && "success" in lastSubmission && lastSubmission.success === true) { - setIsModalOpen(false); - } - }, [lastSubmission]);
625-634: Select is controlled; drop defaultValue to avoid mixed modesYou're already controlling value via state. Remove defaultValue to prevent React’s controlled/uncontrolled warnings.
Apply:
- const [form, { installationId, repositoryId }] = useForm({ + const [form, { installationId, repositoryId }] = useForm({ id: "connect-repo", lastSubmission: lastSubmission, shouldRevalidate: "onSubmit", onValidate({ formData }) { return parse(formData, { schema: ConnectGitHubRepoFormSchema, }); }, }); @@ <Select name={installationId.name} id={installationId.id} value={selectedInstallation?.id} - defaultValue={gitHubAppInstallations.at(0)?.id} setValue={(value) => {Also applies to: 660-676
757-769: Disable “Connect repository” until a repo is selectedPrevents avoidable round-trips and form errors.
Apply:
<FormButtons confirmButton={ <Button type="submit" name="action" value="connect-repo" variant="primary/medium" LeadingIcon={isConnectRepositoryLoading ? SpinnerWhite : undefined} leadingIconClassName="text-white" - disabled={isConnectRepositoryLoading} + disabled={isConnectRepositoryLoading || !selectedRepository} > Connect repository </Button> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(7 hunks)apps/webapp/app/services/gitHub.server.ts(2 hunks)apps/webapp/app/services/projectSettings.server.ts(1 hunks)apps/webapp/app/services/projectSettingsPresenter.server.ts(1 hunks)apps/webapp/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/webapp/package.json
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/services/projectSettingsPresenter.server.tsapps/webapp/app/services/projectSettings.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/services/gitHub.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/services/projectSettingsPresenter.server.tsapps/webapp/app/services/projectSettings.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/services/gitHub.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/services/projectSettingsPresenter.server.tsapps/webapp/app/services/projectSettings.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/services/gitHub.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/services/projectSettingsPresenter.server.tsapps/webapp/app/services/projectSettings.server.tsapps/webapp/app/services/gitHub.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/services/projectSettingsPresenter.server.tsapps/webapp/app/services/projectSettings.server.tsapps/webapp/app/services/gitHub.server.ts
🧠 Learnings (5)
📚 Learning: 2025-08-29T10:06:49.293Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-08-29T10:06:49.293Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from trigger.dev/core’s package.json
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/services/gitHub.server.ts
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsxapps/webapp/app/services/gitHub.server.ts
📚 Learning: 2025-09-03T14:34:41.781Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#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.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-09-02T11:27:36.336Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.336Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
📚 Learning: 2025-09-02T11:18:06.602Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.602Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
🧬 Code graph analysis (4)
apps/webapp/app/services/projectSettingsPresenter.server.ts (2)
apps/webapp/app/models/project.server.ts (1)
findProjectBySlug(109-120)apps/webapp/app/v3/github.ts (1)
BranchTrackingConfigSchema(3-10)
apps/webapp/app/services/projectSettings.server.ts (3)
apps/webapp/app/services/deleteProject.server.ts (1)
DeleteProjectService(10-81)apps/webapp/app/v3/github.ts (2)
BranchTrackingConfig(12-12)BranchTrackingConfigSchema(3-10)apps/webapp/app/services/gitHub.server.ts (1)
checkGitHubBranchExists(141-190)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (8)
apps/webapp/app/routes/_app.github.callback/route.tsx (1)
loader(33-145)apps/webapp/app/utils/pathBuilder.ts (5)
EnvironmentParamSchema(26-28)v3ProjectPath(140-142)organizationPath(92-94)githubAppInstallPath(144-148)v3ProjectSettingsPath(418-424)apps/webapp/app/services/projectSettingsPresenter.server.ts (1)
ProjectSettingsPresenter(8-146)apps/webapp/app/services/projectSettings.server.ts (1)
ProjectSettingsService(8-281)apps/webapp/app/models/message.server.ts (4)
redirectWithSuccessMessage(162-179)redirectWithErrorMessage(181-198)redirectBackWithErrorMessage(200-207)redirectBackWithSuccessMessage(209-216)apps/webapp/app/hooks/useProject.tsx (1)
useProject(20-24)apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization(39-43)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment(19-23)
apps/webapp/app/services/gitHub.server.ts (1)
packages/core/src/v3/apiClient/getBranch.ts (1)
getBranch(4-33)
⏰ 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). (23)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 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 (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (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 (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/webapp/app/services/projectSettings.server.ts (1)
41-52: Remove type-change suggestion
installationIdinconnectGitHubRepocorresponds to theString-typed relation field in the PrismaGitHubRepositorymodel, whereasappInstallationIdis a separateBigIntonGithubAppInstallation(hence theNumber(...)conversion). The existinginstallationId: stringsignature matches the schema—no changes required.Likely an incorrect or invalid review comment.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (5)
18-20: Good: server-side types import is correctImporting ActionFunction/LoaderFunctionArgs from "@remix-run/server-runtime" matches Remix server context.
125-139: Flash handling: session is committed and header returnedReading the flash key and committing the session via Set-Cookie avoids the modal re-opening across requests.
142-156: Zod schemas look solid
- Discriminated union keeps actions clean.
- Checkbox coercion via transform(val === "on") is correct.
Also applies to: 188-194
217-229: Membership verification before mutating is in the right placeEarly guard prevents leaking action behavior to non-members.
889-896: External repo link is safe and concisetarget="_blank" paired with rel="noreferrer noopener" is correct; metadata display looks good.
Also applies to: 900-909
| type GitHubAppInstallation = { | ||
| id: string; | ||
| appInstallationId: bigint; | ||
| targetType: string; | ||
| accountHandle: string; | ||
| repositories: GitHubRepository[]; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
BigInt in loader data will break JSON serialization
If appInstallationId is a bigint from Prisma, typedjson/json cannot serialize it. Use a string (or number) API shape instead, and ensure the presenter/service converts BigInt → string before returning.
Apply:
-type GitHubAppInstallation = {
+type GitHubAppInstallation = {
id: string;
- appInstallationId: bigint;
+ appInstallationId: string;
targetType: string;
accountHandle: string;
repositories: GitHubRepository[];
-};
+};To confirm upstream serialization, run:
🏁 Script executed:
#!/bin/bash
# Inspect where installations are assembled and ensure BigInt ➜ string conversion.
rg -n -C3 'appInstallationId' apps/webapp/app/services | cat
rg -n -C2 'BigInt|bigint' apps/webapp/app/services internal-packages -g '!**/node_modules/**' | catLength of output: 106104
Convert BigInt to number for appInstallationId
typedjson/json cannot serialize BigInt. In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx, change GitHubAppInstallation.appInstallationId from bigint to number. In apps/webapp/app/services/projectSettingsPresenter.server.ts (around the Prisma select on appInstallationId), wrap the raw BigInt in Number(...) so the loader returns a JS number.
🤖 Prompt for AI Agents
In
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx
around lines 587-593, change the GitHubAppInstallation type so appInstallationId
is a number (not bigint); then in
apps/webapp/app/services/projectSettingsPresenter.server.ts (near the Prisma
select that projects appInstallationId) wrap the raw BigInt in Number(...) so
the loader returns a JS number (e.g. select or map step should convert the
BigInt to Number before returning the object to the route). Ensure all
references and usages expect a number thereafter.
@ericallam yes, it is behind the |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (1)
147-149: Use LEFT JOIN to avoid dropping deployments without a triggering user.INNER JOIN excludes GitHub/system-triggered deployments (null triggeredById), skewing lists and pagination.
Apply:
-FROM - ${sqlDatabaseSchema}."WorkerDeployment" as wd -INNER JOIN - ${sqlDatabaseSchema}."User" as u ON wd."triggeredById" = u."id" +FROM + ${sqlDatabaseSchema}."WorkerDeployment" as wd +LEFT JOIN + ${sqlDatabaseSchema}."User" as u ON wd."triggeredById" = u."id"
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
318-325: Thanks for adding rel="noreferrer noopener".This addresses the earlier security comment on the external link; all good.
🧹 Nitpick comments (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (2)
148-155: Add missing dependencies to useEffect.Prevents eslint-react-hooks warnings and stale reads of navigate/path/page.
Apply:
- }, [selectedDeployment, deploymentParam, location.search]); + }, [selectedDeployment, deploymentParam, location.search, navigate, location.pathname, currentPage]);
295-300: colSpan likely off-by-one.There are 9 columns (including the actions column). Blank row uses colSpan=8, which can misalign.
Apply:
- <TableBlankRow colSpan={8}> + <TableBlankRow colSpan={9}>apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (1)
178-186: Pre-index labels to avoid O(n·m) lookups.Minor perf win and simpler reads.
Apply:
- return { + const labelMap = new Map(labeledDeployments.map((l) => [l.deploymentId, l.label])); + return { currentPage: page, totalPages: Math.ceil(totalCount / pageSize), connectedGithubRepository: project.connectedGithubRepository ?? undefined, environmentGitHubBranch, - deployments: deployments.map((deployment, index) => { - const label = labeledDeployments.find( - (labeledDeployment) => labeledDeployment.deploymentId === deployment.id - ); + deployments: deployments.map((deployment, index) => { + const label = labelMap.get(deployment.id);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts
🧠 Learnings (2)
📚 Learning: 2025-07-18T17:49:24.468Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL
Applied to files:
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts
📚 Learning: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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/presenters/v3/DeploymentListPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (1)
apps/webapp/app/v3/github.ts (2)
BranchTrackingConfigSchema(3-10)getTrackedBranchForEnvironment(14-35)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (5)
apps/webapp/app/components/primitives/Table.tsx (1)
Table(41-59)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton(337-403)apps/webapp/app/utils/pathBuilder.ts (1)
v3ProjectSettingsPath(418-424)apps/webapp/app/components/primitives/Pagination.tsx (1)
PaginationControls(7-93)
⏰ 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). (15)
- GitHub Check: units / packages / 📊 Merge Reports
- 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 (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 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 (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
🔇 Additional comments (6)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (4)
4-4: Imports LGTM.New icon, utility, and path imports are appropriate and used correctly.
Also applies to: 10-10, 55-61
133-140: Loader data destructuring LGTM.Matches the presenter’s returned shape.
177-179: Table layout change LGTM.grow and border-t-0 make sense within the resizable panel.
303-334: Bottom bar UX is clear.Nice integrated status + pagination.
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (2)
60-71: Project select LGTM.Selecting branchTracking, previewDeploymentsEnabled, and repo metadata is sufficient for the UI.
2-2: Type-only imports LGTM.Keeps bundle lean and typings clear.
Also applies to: 12-12
This PR enables connecting a GitHub repo to a project. The settings page now
contains a section to configure tracking branches to automatically deploy
commits on the corresponding environment.
We're also trying out
neverthrowfor error handling in this PR. Should help us make error handling more explicit and make exhaustive checking of error types easier. Interfacing with parts of the codebase that don't useneverthrowis not too problematic either. The usage in this PR is rather isolated, so if we decide not to continue with it, it won't be a big deal to remove it.