Preview branches #2086
Preview branches #2086
Conversation
# Conflicts: # internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts # packages/cli-v3/src/commands/deploy.ts # pnpm-lock.yaml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
65-87: 💡 Verification agent🧩 Analysis chain
Verify deprecated function still functions correctly.
The
findEnvironmentByPublicApiKeyfunction is marked as deprecated but still adds thebranchNameparameter. Since this function doesn't implement branch resolution logic likefindEnvironmentByApiKey, it may not work correctly with preview branches.
🏁 Script executed:
#!/bin/bash # Description: Check if the deprecated public API key function is still used in the codebase # Expected: Should find usage patterns to determine if branch support is needed echo "Searching for usage of findEnvironmentByPublicApiKey..." rg "findEnvironmentByPublicApiKey" --type ts --type tsx -A 3 -B 3 echo -e "\nSearching for publicApiKey or pkApiKey usage..." rg "(publicApiKey|pkApiKey)" --type ts --type tsx -A 2 -B 2Length of output: 415
🏁 Script executed:
#!/bin/bash # Re-run search without file-type restrictions to locate any references echo "Searching for references to findEnvironmentByPublicApiKey..." rg "findEnvironmentByPublicApiKey" -n -A 3 -B 3 || true echo -e "\nSearching for references to pkApiKey usage..." rg "pkApiKey" -n -A 3 -B 3 || trueLength of output: 22621
Remove unused
branchNameparameter from deprecatedfindEnvironmentByPublicApiKeyThe
branchNameargument is accepted but never used—this either needs to be implemented (to matchfindEnvironmentByApiKey’s branch resolution) or dropped entirely since the function is deprecated.Please update:
- apps/webapp/app/models/runtimeEnvironment.server.ts
• RemovebranchName: string | undefinedfrom thefindEnvironmentByPublicApiKeysignature (around line 66).- apps/webapp/app/services/apiAuth.server.ts
• RemovefindEnvironmentByPublicApiKeyimport’s second parameter in calls at lines 116 and 192 (and adjust the import if the signature changes).If you intend to continue supporting preview branches via public keys, add branch‐aware lookup logic here; otherwise drop the unused parameter to clean up the deprecated API.
🧹 Nitpick comments (10)
apps/webapp/app/v3/services/replayTaskRun.server.ts (1)
30-32: Good defensive programming for archived environments.The validation correctly prevents replaying task runs on archived environments, maintaining data integrity. The check is appropriately placed early in the method for fail-fast behavior.
Consider making the error message more descriptive:
- throw new Error("Can't replay a run on an archived environment"); + throw new Error(`Cannot replay task run on archived environment: ${authenticatedEnvironment.slug}`);apps/webapp/app/components/V4Badge.tsx (1)
19-26: Consider adding proper accessibility attributes.The
V4Titlecomponent works well for wrapping content with the V4 badge. Consider addingaria-labelortitleattributes to improve accessibility for screen readers.export function V4Title({ children }: { children: React.ReactNode }) { return ( - <> + <span role="group" aria-label="Version 4 feature"> <span>{children}</span> <V4Badge /> - </> + </span> ); }apps/webapp/test/validateGitBranchName.test.ts (1)
78-108: Minor inconsistency: describe block name doesn't match function being tested.The describe block is named "branchNameFromRef" but tests the
sanitizeBranchNamefunction. Consider updating the describe block name for clarity.-describe("branchNameFromRef", () => { +describe("sanitizeBranchName", () => {Otherwise, the test coverage for Git ref parsing is comprehensive and handles various ref formats correctly.
apps/webapp/app/routes/api.v1.projects.$projectRef.branches.ts (1)
81-81: Remove the unusedenvvariable from the destructuring.The
envvariable is extracted from the request body but never used in the handler.- const { branch, env, git } = parsed.data; + const { branch, git } = parsed.data;packages/cli-v3/src/utilities/gitMeta.ts (1)
179-183: Consider removing debug git status output.The git status debug output may clutter production logs without providing significant value in the GitHub Actions context.
- await x("git", ["status"], { - nodeOptions: { - cwd: process.cwd(), - }, - }).then((result) => console.debug(result.stdout));apps/webapp/app/services/upsertBranch.server.ts (1)
170-199: Comprehensive Git branch name validation.The validation correctly implements Git's branch naming rules. Consider adding a comment with the Git documentation reference for future maintainers.
+// Git branch naming rules reference: https://git-scm.com/docs/git-check-ref-format export function isValidGitBranchName(branch: string): boolean {packages/cli-v3/src/commands/deploy.ts (2)
221-227: Optimize branch determination logic.The branch is determined even for non-preview environments. Consider moving this inside the preview-specific logic.
- const branch = - options.env === "preview" ? getBranch({ specified: options.branch, gitMeta }) : undefined; + let branch: string | undefined; + if (options.env === "preview" && !branch) { + branch = getBranch({ specified: options.branch, gitMeta }); + if (!branch) { throw new Error( - "Didn't auto-detect preview branch, so you need to specify one. Pass --branch <branch>." + "Couldn't auto-detect the preview branch from Git. Please specify one using --branch <branch>." ); + } }
726-742: Helpful directory validation with clear error messages.The function provides excellent UX by catching common CLI usage mistakes and offering clear corrections.
Consider adding a check for common typos like "prev" for "preview":
if (dir === "stg") { throw new Error(`To deploy to staging, you need to pass "--env staging", not "stg".`); } + + if (dir === "prev") { + throw new Error(`To deploy to preview, you need to pass "--env preview", not "prev".`); + }apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (2)
92-103: Improve error message for clarity and user experience.The error message when all variables are blacklisted could be more helpful. Consider providing specific guidance about which variables were removed.
- return { - success: false as const, - error: `You must set at least one valid variable.${ - removedDuplicates ? " These were ignored because they're blacklisted." : "" - }`, - }; + return { + success: false as const, + error: `You must set at least one valid variable.${ + removedDuplicates ? " Some variables were ignored because they are reserved system variables (e.g., TRIGGER_SECRET_KEY, TRIGGER_API_URL, OTEL_*)." : "" + }`, + };
844-855: Consider using a Map for better performance.While the current implementation is correct, it has O(n²) complexity due to the
array.some()call inside the loop. For better performance with large variable arrays, consider using a Map.-export function deduplicateVariableArray(variables: EnvironmentVariable[]) { - const result: EnvironmentVariable[] = []; - // Process array in reverse order so later variables override earlier ones - for (const variable of [...variables].reverse()) { - if (!result.some((v) => v.key === variable.key)) { - result.push(variable); - } - } - // Reverse back to maintain original order but with later variables taking precedence - return result.reverse(); -} +export function deduplicateVariableArray(variables: EnvironmentVariable[]) { + const seen = new Map<string, EnvironmentVariable>(); + // Process in normal order - later variables will override earlier ones in the Map + for (const variable of variables) { + seen.set(variable.key, variable); + } + return Array.from(seen.values()); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
references/hello-world/package.jsonis excluded by!references/**references/hello-world/src/trigger/envvars.tsis excluded by!references/**references/hello-world/src/trigger/public-access-tokens.tsis excluded by!references/**references/hello-world/trigger.config.tsis excluded by!references/**
📒 Files selected for processing (107)
.changeset/silly-cows-serve.md(1 hunks)CHANGESETS.md(1 hunks)apps/webapp/app/assets/icons/ArchiveIcon.tsx(1 hunks)apps/webapp/app/assets/icons/EnvironmentIcons.tsx(2 hunks)apps/webapp/app/assets/icons/IntegrationIcon.tsx(0 hunks)apps/webapp/app/components/BlankStatePanels.tsx(3 hunks)apps/webapp/app/components/GitMetadata.tsx(1 hunks)apps/webapp/app/components/V4Badge.tsx(1 hunks)apps/webapp/app/components/environments/EnvironmentLabel.tsx(5 hunks)apps/webapp/app/components/environments/RegenerateApiKeyModal.tsx(1 hunks)apps/webapp/app/components/navigation/EnvironmentBanner.tsx(1 hunks)apps/webapp/app/components/navigation/EnvironmentPausedBanner.tsx(0 hunks)apps/webapp/app/components/navigation/EnvironmentSelector.tsx(3 hunks)apps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsx(2 hunks)apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(4 hunks)apps/webapp/app/components/navigation/SideMenuItem.tsx(3 hunks)apps/webapp/app/components/primitives/Accordion.tsx(1 hunks)apps/webapp/app/components/primitives/Buttons.tsx(1 hunks)apps/webapp/app/components/primitives/PageHeader.tsx(2 hunks)apps/webapp/app/components/primitives/Popover.tsx(2 hunks)apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx(1 hunks)apps/webapp/app/hooks/useEnvironmentSwitcher.ts(3 hunks)apps/webapp/app/models/api-key.server.ts(2 hunks)apps/webapp/app/models/member.server.ts(1 hunks)apps/webapp/app/models/organization.server.ts(3 hunks)apps/webapp/app/models/project.server.ts(1 hunks)apps/webapp/app/models/runtimeEnvironment.server.ts(5 hunks)apps/webapp/app/presenters/OrganizationsPresenter.server.ts(3 hunks)apps/webapp/app/presenters/v3/ApiKeysPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/BranchesPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts(3 hunks)apps/webapp/app/presenters/v3/EditSchedulePresenter.server.ts(2 hunks)apps/webapp/app/presenters/v3/EnvironmentVariablesPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts(3 hunks)apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.apikeys/route.tsx(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables.new/route.tsx(8 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx(9 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx(4 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx(0 hunks)apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.environments.staging.ts(2 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts(4 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.branches.archive.ts(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.branches.ts(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts(2 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts(1 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.ts(3 hunks)apps/webapp/app/routes/resources.branches.archive.tsx(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx(6 hunks)apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts(2 hunks)apps/webapp/app/services/apiAuth.server.ts(17 hunks)apps/webapp/app/services/archiveBranch.server.ts(1 hunks)apps/webapp/app/services/platform.v3.server.ts(1 hunks)apps/webapp/app/services/realtime/jwtAuth.server.ts(1 hunks)apps/webapp/app/services/upsertBranch.server.ts(1 hunks)apps/webapp/app/utils/cn.ts(1 hunks)apps/webapp/app/utils/environmentSort.ts(1 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts(11 hunks)apps/webapp/app/v3/services/checkSchedule.server.ts(2 hunks)apps/webapp/app/v3/services/createBackgroundWorker.server.ts(1 hunks)apps/webapp/app/v3/services/initializeDeployment.server.ts(2 hunks)apps/webapp/app/v3/services/replayTaskRun.server.ts(1 hunks)apps/webapp/app/v3/services/triggerScheduledTask.server.ts(1 hunks)apps/webapp/app/v3/services/upsertTaskSchedule.server.ts(1 hunks)apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts(6 hunks)apps/webapp/package.json(5 hunks)apps/webapp/prisma/seed.ts(1 hunks)apps/webapp/tailwind.config.js(2 hunks)apps/webapp/test/environmentVariableDeduplication.test.ts(1 hunks)apps/webapp/test/environmentVariableRules.test.ts(1 hunks)apps/webapp/test/validateGitBranchName.test.ts(1 hunks)internal-packages/database/prisma/migrations/20250509180155_runtime_environment_branching/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250509180346_runtime_environment_parent_environment_id_index/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250511145836_runtime_environment_add_is_branchable_environment/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250513135201_runtime_environment_add_project_id_index/migration.sql(1 hunks)internal-packages/database/prisma/migrations/20250515134154_worker_deployment_add_git_info/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(4 hunks)internal-packages/run-engine/src/engine/db/worker.ts(3 hunks)internal-packages/run-engine/src/engine/systems/dequeueSystem.ts(1 hunks)internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts(4 hunks)packages/build/src/extensions/core/syncEnvVars.ts(4 hunks)packages/build/src/extensions/core/vercelSyncEnvVars.ts(3 hunks)packages/cli-v3/package.json(2 hunks)packages/cli-v3/src/apiClient.ts(17 hunks)packages/cli-v3/src/build/buildWorker.ts(2 hunks)packages/cli-v3/src/build/bundle.ts(3 hunks)packages/cli-v3/src/cli/index.ts(2 hunks)packages/cli-v3/src/commands/deploy.ts(9 hunks)packages/cli-v3/src/commands/preview.ts(1 hunks)packages/cli-v3/src/commands/promote.ts(3 hunks)packages/cli-v3/src/commands/workers/build.ts(7 hunks)packages/cli-v3/src/deploy/buildImage.ts(12 hunks)packages/cli-v3/src/entryPoints/managed-index-controller.ts(1 hunks)packages/cli-v3/src/utilities/gitMeta.ts(1 hunks)packages/cli-v3/src/utilities/session.ts(3 hunks)packages/core/src/v3/apiClient/getBranch.ts(1 hunks)
⛔ Files not processed due to max files limit (7)
- packages/core/src/v3/apiClient/index.ts
- packages/core/src/v3/apiClientManager/index.ts
- packages/core/src/v3/apiClientManager/types.ts
- packages/core/src/v3/schemas/api.ts
- packages/core/src/v3/schemas/build.ts
- packages/core/src/v3/schemas/common.ts
- packages/react-hooks/src/hooks/useApiClient.ts
💤 Files with no reviewable changes (3)
- apps/webapp/app/components/navigation/EnvironmentPausedBanner.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx
- apps/webapp/app/assets/icons/IntegrationIcon.tsx
🧰 Additional context used
🧬 Code Graph Analysis (28)
apps/webapp/app/components/environments/RegenerateApiKeyModal.tsx (1)
apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(294-329)
apps/webapp/app/models/project.server.ts (1)
apps/webapp/app/models/organization.server.ts (1)
createEnvironment(79-122)
apps/webapp/app/components/primitives/PageHeader.tsx (1)
apps/webapp/app/components/navigation/EnvironmentBanner.tsx (1)
EnvironmentBanner(13-26)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (2)
apps/webapp/app/components/primitives/PageHeader.tsx (1)
PageTitle(41-58)apps/webapp/app/components/V4Badge.tsx (1)
V4Title(19-26)
packages/cli-v3/src/commands/promote.ts (3)
packages/cli-v3/src/utilities/gitMeta.ts (1)
createGitMeta(8-48)packages/core/src/v3/apiClient/getBranch.ts (1)
getBranch(4-33)packages/cli-v3/src/utilities/session.ts (1)
getProjectClient(102-136)
apps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsx (1)
apps/webapp/app/components/primitives/Badge.tsx (1)
Badge(21-27)
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.ts (2)
apps/webapp/app/db.server.ts (1)
prisma(100-100)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
resolveVariablesForEnvironment(820-842)
apps/webapp/app/components/primitives/Popover.tsx (2)
apps/webapp/app/components/primitives/Icon.tsx (1)
RenderIcon(4-4)apps/webapp/app/components/primitives/Buttons.tsx (1)
ButtonContentPropsType(166-181)
apps/webapp/app/components/primitives/Accordion.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(31-33)
apps/webapp/test/environmentVariableDeduplication.test.ts (2)
packages/core/src/v3/workers/taskExecutor.ts (1)
result(1262-1309)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
deduplicateVariableArray(845-855)
apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts (2)
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
displayableEnvironment(257-277)apps/webapp/app/utils/environmentSort.ts (1)
sortEnvironments(15-31)
packages/cli-v3/src/cli/index.ts (1)
packages/cli-v3/src/commands/preview.ts (1)
configurePreviewCommand(32-60)
apps/webapp/app/models/organization.server.ts (3)
apps/webapp/app/db.server.ts (1)
prisma(100-100)apps/webapp/app/models/project.server.ts (1)
Project(8-8)apps/webapp/app/models/runtimeEnvironment.server.ts (1)
RuntimeEnvironment(8-8)
apps/webapp/app/components/navigation/EnvironmentSelector.tsx (12)
apps/webapp/app/components/primitives/Popover.tsx (4)
PopoverMenuItem(209-209)PopoverContent(207-207)Popover(205-205)PopoverTrigger(212-212)apps/webapp/app/components/environments/EnvironmentLabel.tsx (1)
EnvironmentCombo(48-63)apps/webapp/app/utils/pathBuilder.ts (3)
v3BillingPath(426-430)docsPath(445-447)branchesPath(418-424)apps/webapp/app/components/navigation/SideMenu.tsx (1)
SideMenuEnvironment(96-96)apps/webapp/app/hooks/useProject.tsx (1)
useProject(20-24)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment(19-23)apps/webapp/app/hooks/useEnvironmentSwitcher.ts (1)
useEnvironmentSwitcher(9-24)apps/webapp/app/components/primitives/Buttons.tsx (1)
ButtonContent(183-286)apps/webapp/app/components/primitives/Badge.tsx (1)
Badge(21-27)apps/webapp/app/assets/icons/EnvironmentIcons.tsx (1)
BranchEnvironmentIconSmall(159-178)apps/webapp/app/components/V4Badge.tsx (1)
V4Badge(5-17)apps/webapp/app/components/primitives/TextLink.tsx (1)
TextLink(22-46)
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (1)
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts (1)
processGitMetadata(191-243)
apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts (1)
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
displayableEnvironment(257-277)
apps/webapp/app/services/archiveBranch.server.ts (2)
apps/webapp/app/db.server.ts (2)
PrismaClient(228-228)prisma(100-100)apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1)
userId(126-168)
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts (2)
apps/webapp/app/db.server.ts (1)
prisma(100-100)apps/webapp/app/models/runtimeEnvironment.server.ts (1)
RuntimeEnvironment(8-8)
apps/webapp/app/components/V4Badge.tsx (3)
apps/webapp/app/components/primitives/Tooltip.tsx (1)
SimpleTooltip(118-118)apps/webapp/app/components/primitives/Badge.tsx (1)
Badge(21-27)apps/webapp/app/utils/cn.ts (1)
cn(31-33)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (1)
apps/webapp/app/components/environments/EnvironmentLabel.tsx (1)
EnvironmentCombo(48-63)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx (1)
apps/webapp/app/utils/pathBuilder.ts (1)
v3ProjectPath(136-138)
apps/webapp/prisma/seed.ts (1)
apps/webapp/app/models/organization.server.ts (1)
createEnvironment(79-122)
apps/webapp/app/models/member.server.ts (1)
apps/webapp/app/models/organization.server.ts (1)
createEnvironment(79-122)
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts (1)
apps/webapp/app/services/apiAuth.server.ts (1)
branchNameFromRequest(265-267)
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (2)
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
RuntimeEnvironment(8-8)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
resolveVariablesForEnvironment(820-842)
packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
packages/cli-v3/src/apiClient.ts (1)
CliApiClient(49-664)
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx (1)
apps/webapp/app/components/primitives/Badge.tsx (1)
Badge(21-27)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (1)
apps/webapp/app/components/primitives/Spinner.tsx (1)
SpinnerWhite(76-78)
🪛 Biome (1.9.4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
[error] 565-565: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
packages/cli-v3/src/utilities/gitMeta.ts
[error] 77-77: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🪛 GitHub Actions: 🤖 PR Checks
packages/cli-v3/package.json
[error] 1-1: pnpm install failed with frozen-lockfile because pnpm-lock.yaml is not up to date with packages/cli-v3/package.json. The lockfile specifiers do not match the package.json dependencies.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (213)
.changeset/silly-cows-serve.md (1)
1-5: Changeset looks good. The patch-level version bump and description accurately reflect the addition of Preview branches in v4 projects.internal-packages/database/prisma/migrations/20250509180346_runtime_environment_parent_environment_id_index/migration.sql (1)
1-2: Ensure transactional compatibility.CREATE INDEX CONCURRENTLYcannot run inside a transaction block; Prisma migrations wrap statements in transactions by default, which will cause this step to fail. Please verify that this migration is configured to execute outside of a transaction.internal-packages/database/prisma/migrations/20250513135201_runtime_environment_add_project_id_index/migration.sql (1)
1-1: Confirm concurrent index execution context. Similar to theparentEnvironmentIdindex,CREATE INDEX CONCURRENTLYmust be run outside of a transaction. Ensure this migration runs in a non-transactional context to avoid errors.internal-packages/database/prisma/migrations/20250515134154_worker_deployment_add_git_info/migration.sql (1)
1-3: Validate nullability and defaults forgit. The newgitJSONB column is nullable by default. Ensure downstream code handlesNULLgracefully, or if git metadata should always be present, consider addingDEFAULT '{}'and/or aNOT NULLconstraint.packages/cli-v3/src/cli/index.ts (2)
17-17: LGTM! Import follows established pattern.The import statement correctly brings in the preview command configuration function consistent with other command imports.
36-36: LGTM! Command registration follows existing convention.The preview command is properly registered with the CLI program, following the same pattern as other commands. The placement maintains logical grouping with other command registrations.
apps/webapp/app/utils/environmentSort.ts (1)
6-6: LGTM! Improved environment ordering logic.Moving PREVIEW after STAGING creates a more logical deployment pipeline order that better reflects typical workflow progression: Development → Staging → Preview → Production.
apps/webapp/app/v3/services/upsertTaskSchedule.server.ts (1)
32-32: LGTM! Provides necessary environment context for validation.Adding the
schedule.environmentsparameter allows the CheckScheduleService to validate against archived environments and prevent schedule operations on archived branches, which is essential for proper preview branch lifecycle management.apps/webapp/app/utils/cn.ts (1)
13-13: LGTM! Adds support for standard Tailwind text size.Adding "base" to the recognized text size classes ensures proper handling of the
text-baseclass in Tailwind merge operations, supporting consistent styling across the new preview branch UI components.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx (1)
35-35: LGTM: Proper V4Title integrationThe addition of the
V4Titleimport and its usage in thePageTitlecomponent correctly follows the established pattern for marking v4-only features. This aligns with the broader initiative to visually distinguish v4 features across the application.Also applies to: 119-119
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (1)
56-56: LGTM: Clean Git metadata integrationThe addition of Git metadata to the deployments table is well-implemented:
- Proper import of the
GitMetadatacomponent- Consistent table structure with the new "Git" column header
- Appropriate styling with margin adjustment for visual alignment
- Correct prop passing of
deployment.gitto the componentThis enhancement provides valuable Git context for deployments as part of the preview branches feature.
Also applies to: 197-197, 261-265
apps/webapp/app/components/navigation/HelpAndFeedbackPopover.tsx (1)
25-25: LGTM: Consistent badge component usageThe replacement of
MenuCountwith the standardBadgecomponent aligns with the broader component standardization effort across the application. The implementation is correct:
- Proper import and usage of the
Badgecomponent- Appropriate
variant="extra-small"andclassName="uppercase"styling- Consistent with similar badge refactoring seen in other navigation components
Also applies to: 113-115
packages/cli-v3/src/build/buildWorker.ts (2)
31-31: LGTM - Clean optional parameter addition.The addition of the optional
branchparameter toBuildWorkerOptionsis well-implemented and follows TypeScript best practices.
79-79: LGTM - Proper parameter propagation.The
branchparameter is correctly passed through tocreateBuildManifestFromBundle, maintaining the optional nature of the parameter.apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx (2)
24-24: LGTM - Badge component import added.The import is correctly added to support the new Badge component usage.
73-76: LGTM - Improved badge rendering with consistent styling.The conversion from plain string to Badge component enhances UI consistency while preserving the conditional logic. The "extra-small" variant is appropriate for navigation badges.
apps/webapp/app/components/runs/v3/ReplayRunDialog.tsx (1)
134-139: LGTM - Enhanced environment filtering with branch support.The filter configuration properly handles both environment type and branch name filtering. The string normalization (replacing
/and_with spaces) improves user experience, and the optional chaining with fallback forbranchNamecorrectly handles cases where it might be undefined.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (2)
5-5: LGTM - GitMetadata component import added.The import is correctly added to support displaying Git metadata in deployment details.
203-210: LGTM - Git metadata display integration.The new Property.Item for Git metadata follows the established pattern and properly integrates the GitMetadata component. The layout styling appears intentional for proper alignment within the property table.
apps/webapp/app/components/primitives/PageHeader.tsx (2)
8-8: LGTM: Clean component import update.The import correctly updates to use the new consolidated
EnvironmentBannercomponent.
28-28: LGTM: Component usage properly updated.The component replacement correctly maintains the conditional rendering logic while using the new
EnvironmentBannerthat handles both paused and archived environment states.apps/webapp/app/utils/pathBuilder.ts (1)
418-425: LGTM: Well-implemented path builder function.The
branchesPathfunction follows the established patterns in the file and correctly constructs the path by building on the existingv3EnvironmentPathfunction. The parameter types and return type are consistent with other path builder functions.apps/webapp/app/components/primitives/Popover.tsx (1)
62-62: LGTM: Well-implemented className prop enhancement.The addition of the optional
classNameprop toPopoverMenuItemis properly implemented:
- Correctly added to the component's interface as optional
- Properly merged with existing conditional styles using the
cnutility- Maintains backward compatibility while adding styling flexibility
This enhancement follows React best practices for component styling extensibility.
Also applies to: 70-70, 83-85
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts (2)
8-8: LGTM: Correct import addition for branch support.The import of
branchNameFromRequestis properly added to support branch-aware environment authentication.
33-35: LGTM: Branch context properly integrated into environment authentication.The addition of
branchNameFromRequest(request)as the fourth parameter correctly extends the environment authentication to consider branch context. This aligns with the broader branch-aware environment handling introduced across the codebase and maintains backward compatibility since the branch name is optional.apps/webapp/app/components/primitives/Buttons.tsx (1)
302-307: LGTM! Improved shortcut key handling with proper event management.The enhancement correctly prevents unintended side effects by calling
preventDefault()andstopPropagation()after triggering the programmatic button click. This ensures the shortcut key event doesn't bubble up and cause additional unwanted behaviors.apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts (1)
126-130: LGTM! Correctly implements hierarchical environment variable resolution.The addition of
environment.parentEnvironmentId ?? undefinedas the third parameter enables the repository to properly resolve environment variables from both child and parent environments when dealing with branch environments. The nullish coalescing operator ensures clean handling of cases where no parent environment exists.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
526-535: LGTM! Enhanced schedule validation with environment context.The addition of
[environment.id]as the third parameter tocheckSchedule.callcorrectly provides environment context for schedule validation. This enables the service to validate schedules against archived environments and supports the new environment branching functionality.apps/webapp/app/services/realtime/jwtAuth.server.ts (1)
36-39: LGTM! Correctly implements hierarchical JWT validation for branch environments.The modification to use
environment.parentEnvironment?.apiKey ?? environment.apiKeyproperly handles JWT validation for branch environments by validating tokens against the parent environment's API key when available. This ensures consistent authentication behavior across the environment hierarchy while maintaining backward compatibility for standalone environments.apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts (1)
85-86: LGTM! Parent environment context properly integrated.The addition of
environment.parentEnvironmentId ?? undefinedas the third argument correctly enables environment variable inheritance from parent environments in branch hierarchies. The nullish coalescing operator appropriately handles cases where no parent environment exists.packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
29-33: Branch context correctly integrated into CLI operations.The addition of
env.TRIGGER_PREVIEW_BRANCHas the third argument to theCliApiClientconstructor properly enables branch-aware API requests. Based on the relevant code snippets, this will set thex-trigger-branchheader when the branch is specified, allowing the managed index controller to operate within the correct branch context.apps/webapp/app/presenters/v3/ViewSchedulePresenter.server.ts (2)
56-56: Branch name properly added to Prisma selection.The addition of
branchName: trueto the environment select clause correctly fetches branch metadata from the database.
95-98: Branch name correctly integrated into environment data.The modification properly extends the environment object with branch name information. Using
environment.branchName ?? undefinedappropriately handles null values and maintains consistency with TypeScript's undefined convention.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx (1)
2-2: Improved error handling with graceful redirect.The changes enhance user experience by redirecting to the project page when no environment matches the
envParam, instead of throwing a 404. This is a much better approach that keeps users within the application flow.The implementation correctly uses
v3ProjectPathto construct the redirect URL with the organization and project slugs.Also applies to: 8-8, 51-51
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1)
171-173: Well-implemented archival protection.The guard clause correctly prevents queue operations (pause/resume) on archived environments. The placement after environment validation and before operations is appropriate, and the error message clearly communicates the issue to users.
This implementation is consistent with the broader archival feature being introduced across the codebase.
apps/webapp/app/components/environments/RegenerateApiKeyModal.tsx (1)
31-31: Good consolidation of button styling props.The change simplifies the button configuration by using a single
variant="minimal/small"prop instead of multiple styling-related props, while preserving the functionaltextAlignLeftandLeadingIconproperties.This consolidation improves code maintainability and follows a more declarative API pattern.
apps/webapp/app/v3/services/triggerScheduledTask.server.ts (1)
56-64: Excellent implementation of archival protection for scheduled tasks.The guard clause correctly prevents scheduled tasks from executing in archived environments. The implementation follows the established pattern of the existing deletion checks and includes appropriate debug logging for troubleshooting.
This ensures system integrity by preventing unintended task executions in archived branches, which aligns perfectly with the broader preview branches and archival feature implementation.
apps/webapp/tailwind.config.js (2)
152-153: Good color differentiation for environment types.The updated color assignments provide better visual distinction between staging (orange) and preview (yellow) environments, which aligns well with the introduction of preview branches.
313-313: Useful size additions for new UI components.The addition of the 4.5 (1.125rem) size value to width, height, and the new size category provides good support for the new branch-related UI components and icons introduced in this feature.
Also applies to: 317-317, 320-321
apps/webapp/prisma/seed.ts (1)
49-56: Excellent refactoring to object-based parameters.The change from positional arguments to a structured object with named properties significantly improves code readability and maintainability. The explicit
isBranchableEnvironment: falseclearly indicates that staging environments are not branchable, which aligns with the new preview branch functionality.apps/webapp/app/presenters/v3/EditSchedulePresenter.server.ts (2)
53-53: Good addition of branchName field.Adding the
branchNamefield to the environment selection query is essential for supporting the new preview branch functionality and ensuring the UI can display branch information correctly.
89-99: Effective filtering for data integrity.The filtering logic correctly excludes preview environments that lack a branch name, preventing potential UI issues and maintaining data consistency. The transformation also properly handles the optional branchName field by using nullish coalescing.
apps/webapp/app/v3/services/initializeDeployment.server.ts (2)
1-6: Good import optimizations.The conversion to type-only imports where appropriate and the addition of necessary new imports (
WorkerDeploymentType,env,logger) improves the module's efficiency and supports the new functionality.
110-110: Valuable addition of Git metadata tracking.Adding Git metadata to deployment records enables better tracking of deployment history and debugging. The use of nullish coalescing (
payload.gitMeta ?? undefined) properly handles optional Git metadata.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx (2)
3-3: LGTM! Improved type safety with direct icon imports.The change from string literals to direct React component imports improves type safety and ensures better consistency with other icon usage patterns throughout the application.
275-275: Good use of React components for icons.The direct usage of
ExclamationTriangleIconandTrashIconcomponents, along with the conditionalSpinnerWhitefor loading states, follows React best practices and improves the overall UI consistency.Also applies to: 290-290
apps/webapp/app/models/member.server.ts (1)
177-184: LGTM! Correctly implements the updated createEnvironment API.The change properly updates the function call to use the new object parameter syntax and explicitly sets
isBranchableEnvironment: false, which is appropriate for development environments created during user invitation acceptance.apps/webapp/app/components/navigation/SideMenuItem.tsx (2)
1-1: Excellent improvement to badge flexibility.Changing the badge prop type from
stringtoReactNodeprovides much better flexibility for rendering rich badge content, enabling the use of components likeBadgeandV4Badgementioned in the AI summary.Also applies to: 25-25
49-49: Simplified and cleaner badge rendering logic.The removal of the
MenuCountwrapper component and direct rendering of the badge improves code clarity while maintaining the same functionality.packages/build/src/extensions/core/syncEnvVars.ts (2)
14-14: Good addition of optional branch parameter.The optional
branchproperty maintains backward compatibility while enabling branch-aware environment variable synchronization, which aligns perfectly with the preview branches feature.
89-89: Proper threading of branch context through the sync pipeline.The branch parameter is correctly passed through the entire call chain from
manifest.branchto the user-providedsyncEnvVarsFncallback, enabling branch-aware environment variable handling.Also applies to: 143-143, 155-155
packages/core/src/v3/apiClient/getBranch.ts (1)
4-33: Excellent branch detection utility with robust fallback strategy.The function implements a well-thought-out priority hierarchy for branch name resolution, covering manual specification, environment variables (including Vercel integration), and Git metadata fallbacks. The implementation is clean, pure, and handles all edge cases appropriately.
apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts (2)
35-35: Good addition of branchName to support preview branches.Adding
branchNameto the select clause properly supports the branch-aware environment model.
43-43: Appropriate filtering of archived environments.Filtering out archived environments (
archivedAt: null) prevents replay operations on inactive branches, which aligns with the expected behavior.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (3)
10-10: Good import addition for enhanced environment display.Adding
EnvironmentComboimport supports the improved environment visualization with branch information.
118-120: Essential validation to prevent tests on archived environments.This check prevents users from running tests on archived environments, which could lead to confusion or unexpected behavior. The error message is clear and actionable.
343-343: Improved environment display with EnvironmentCombo.Switching from
EnvironmentLabeltoEnvironmentComboprovides better visual representation of environments, including branch information. Thegap-0.5styling maintains proper visual spacing.Also applies to: 543-543
apps/webapp/test/environmentVariableDeduplication.test.ts (1)
1-64: Comprehensive and well-structured test suite.The test suite excellently covers all aspects of the
deduplicateVariableArrayfunction:
- Core functionality: Tests that later variables override earlier ones when duplicates exist
- Order preservation: Ensures the original order of unique variables is maintained
- Complex scenarios: Handles multiple duplicates with proper precedence
- Edge cases: Covers empty arrays and arrays without duplicates
- Clear assertions: Uses specific value checks and appropriate matchers
The test descriptions are clear and the test cases are comprehensive, providing confidence in the deduplication logic that supports the environment variable merging functionality.
apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts (3)
1-5: LGTM! Import changes are correct.The addition of
Prismato the existing import and the new import ofprocessGitMetadatafromBranchesPresenter.serverare appropriate for integrating Git metadata processing into deployment listings.Also applies to: 11-11
110-110: LGTM! Git field integration is consistent.The addition of the
gitfield to both the TypeScript type definition and the SQL query is correctly implemented. ThePrisma.JsonValue | nulltype accurately reflects the database schema for JSON fields.Also applies to: 127-127
174-174: LGTM! Git metadata processing is properly implemented.The use of
processGitMetadata(deployment.git)correctly processes the raw JSON data from the database and handles validation and null cases as seen in the referenced function implementation.apps/webapp/app/models/project.server.ts (2)
91-96: LGTM! Environment creation updated correctly.The refactoring from positional arguments to object syntax with explicit
isBranchableEnvironment: falsefor the PRODUCTION environment is correct and aligns with the updatedcreateEnvironmentfunction signature.
99-106: LGTM! Development environment creation follows the same pattern.The DEVELOPMENT environment creation is consistently updated to use the object syntax with
isBranchableEnvironment: false, which is appropriate since development environments are traditionally not branchable in this architecture.apps/webapp/app/components/navigation/SideMenu.tsx (3)
41-41: LGTM! Import additions are appropriate.The new imports for
branchesPath,BranchEnvironmentIconSmall, andV4Badgeare correctly added to support the preview branches functionality.Also applies to: 88-89
269-269: LGTM! V4Badge correctly added to Tokens.Adding the
V4Badgeto the Tokens menu item appropriately marks it as a version 4 feature, maintaining consistency with other v4-specific functionality.
295-302: LGTM! Preview branches menu item is well-implemented.The new "Preview branches" menu item is properly configured with:
- Appropriate icon (
BranchEnvironmentIconSmall)- Correct color scheme (
text-preview)- Proper routing to
branchesPath- Consistent data attribute for tracking
- V4Badge to indicate version requirement
The placement under the "Manage" section is logical for this feature.
apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts (3)
13-13: LGTM! Import is consistent with other presenters.The import of
processGitMetadatafromBranchesPresenter.serverfollows the same pattern established inDeploymentListPresenter.server.ts.
102-102: LGTM! Git field added to deployment query.Adding
git: trueto the select statement correctly includes the Git metadata from the database for processing.
167-167: LGTM! Git metadata processing matches the pattern.The use of
processGitMetadata(deployment.git)is consistent with the implementation inDeploymentListPresenter.server.tsand properly handles the JSON data validation and transformation.apps/webapp/app/components/V4Badge.tsx (1)
1-17: LGTM! Clean implementation of V4 feature badge.The
V4Badgecomponent is well-designed with proper prop handling and tooltip integration. The implementation correctly uses existing primitives and follows the established patterns.apps/webapp/app/presenters/v3/EnvironmentVariablesPresenter.server.ts (3)
74-75: LGTM! Proper addition of branch metadata fields.The addition of
isBranchableEnvironmentandbranchNamefields to the query correctly extends the data model to support branch information.
86-86: LGTM! Appropriate filtering of archived environments.The
archivedAt: nullfilter correctly excludes archived environments from the query results, which aligns with the expected behavior for active environment variable management.
115-115: LGTM! Consistent inclusion of branch metadata in response.The addition of
branchNameto both the environment variable objects and the environments list ensures consistent branch information is available throughout the response structure.Also applies to: 126-127
internal-packages/database/prisma/migrations/20250509180155_runtime_environment_branching/migration.sql (3)
2-5: LGTM! Well-structured column additions for branch support.The addition of
archivedAt,branchName, andgitcolumns properly supports the branching feature requirements. Using nullable columns ensures backward compatibility.
7-19: LGTM! Safe conditional column addition.The conditional logic to check for existing
parentEnvironmentIdcolumn before adding it is a good practice that prevents migration failures in environments where the column might already exist.
21-22: Verify CASCADE behavior in production.The foreign key constraint with
ON DELETE CASCADE ON UPDATE CASCADEwill automatically delete child environments when parent environments are deleted. Ensure this cascading behavior aligns with your business requirements, especially for production environments.Consider documenting the cascade behavior and ensuring proper backup/archival procedures are in place before applying this migration to production.
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)
201-213: LGTM! Proper handling of archived environment state.The new
RUN_ENVIRONMENT_ARCHIVEDcase correctly handles situations where preview branches have been archived. The implementation follows the established pattern:
- Logs appropriate warning with context
- Acknowledges the message to prevent reprocessing
- Returns null to skip execution
This ensures that runs associated with archived branches are gracefully removed from the queue without causing system errors.
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.ts (4)
1-1: Good practice: Type-only import optimization.Using type-only imports helps with tree-shaking and makes the dependency clearer.
31-31: LGTM: Consistent property access.The change to use
authenticationResult.environment.iddirectly is more explicit and consistent with the variable naming.
41-52: Good enhancement: Explicit environment fetching with parent context.The explicit fetching of the runtime environment with
parentEnvironmentincluded is essential for the branching feature. This ensures that environment variable resolution can access the full environment hierarchy.
54-57: Consistent with new environment hierarchy API.The updated call to
resolveVariablesForEnvironmentcorrectly passes both the environment and its parent (if any), which aligns with the function signature shown in the relevant code snippets.CHANGESETS.md (1)
33-43: Improved documentation clarity.The simplified snapshot instructions are much clearer and more user-friendly. The numbered steps with emphasis on replacing placeholders and undoing the temporary commit help prevent common mistakes.
apps/webapp/app/presenters/v3/ScheduleListPresenter.server.ts (3)
38-38: Good type design: Optional branch name property.Making
branchNameoptional ensures backward compatibility while supporting the new branching feature.
71-71: Consistent database query update.Adding
branchNameto the selection ensures the data is available for the presenter output.
274-277: Proper data mapping with fallback.The spread of
displayableEnvironmentcombined with explicitbranchNamemapping (with undefined fallback) is a clean approach that preserves existing functionality while adding branch support.apps/webapp/app/assets/icons/ArchiveIcon.tsx (2)
1-18: Well-structured icon component.The
ArchiveIconcomponent follows React best practices:
- Accepts optional className for styling flexibility
- Uses
currentColorfor theme integration- Includes proper clipPath for SVG rendering
- Clean, readable structure
20-44: Consistent implementation for unarchive icon.The
UnarchiveIconcomponent maintains consistency with theArchiveIcon:
- Same prop interface and structure
- Uses
currentColorfor theming- Proper SVG elements and clipPath
Both components will integrate well with the archival features introduced in the branching functionality.
internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts (3)
5-5: LGTM: Clean import addition for Git metadata support.The import of
GitMetaschema enables proper validation of Git metadata in the run execution context.
288-294: LGTM: Safe Git metadata parsing implementation.The implementation correctly uses
safeParseto validate Git metadata before including it in the execution context. This ensures type safety and prevents runtime errors from malformed data.
346-347:✅ Verification successful
LGTM: Proper integration of Git metadata in execution environment.
The Git metadata and branch name are correctly included in the execution environment with appropriate null safety checks.
Verify that the
gitproperty exists in the database schema for the environment object:
🏁 Script executed:
#!/bin/bash # Description: Verify that the environment object includes git property in the database schema # Test: Search for git property in environment-related schema definitions rg -A 5 -B 5 "git.*:" --type ts | head -20Length of output: 1721
LGTM: Git metadata property confirmed in execution environment schema
Verified that the
gitproperty is defined asGitMeta.optional()inTaskRunExecutionEnvironment(packages/core/src/v3/schemas/common.ts). The branch name and Git metadata are correctly integrated with null safety checks.apps/webapp/app/services/platform.v3.server.ts (2)
361-367: LGTM: Consistent environment creation for cloud deployments.The cloud deployment logic now properly creates both STAGING and PREVIEW environments when staging is allowed by the plan. The use of named parameters improves code readability and maintainability.
349-356:❓ Verification inconclusive
Verify removal of version check doesn't break existing functionality.
The logic now always creates both STAGING and PREVIEW environments for non-cloud deployments, removing the previous check for
project.version === "V2". Ensure this change is intentional and doesn't negatively impact existing projects.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any references to project version checks in environment creation # Test: Search for version-related environment creation logic rg -A 3 -B 3 "version.*V2|V2.*version" --type tsLength of output: 3594
🏁 Script executed:
#!/bin/bash # Verify any version checks within environment creation logic in platform.v3.server.ts rg -n "project\.version" -A3 -B3 apps/webapp/app/services/platform.v3.server.ts rg -n "V2" -A3 -B3 apps/webapp/app/services/platform.v3.server.tsLength of output: 147
Verify version check removal in platform.v3.server.ts
I couldn’t find any
project.versionor"V2"checks inapps/webapp/app/services/platform.v3.server.ts. Please confirm that always creating both STAGING and PREVIEW environments for all project versions is intentional and won’t break existing workflows.• No occurrences of
project.versionin the file
• No"V2"references in the file
• Ensure downstream logic doesn’t rely on skipping PREVIEW environments for V2 projectsapps/webapp/test/validateGitBranchName.test.ts (1)
1-76: Excellent comprehensive test coverage for Git branch name validation.The test suite thoroughly covers Git branch naming rules including:
- Control characters (ASCII 0-31)
- Forbidden special characters
- Invalid patterns (consecutive slashes, .., @{, etc.)
- Boundary conditions (starting/ending with slash, .lock suffix)
The test coverage ensures robust validation of branch names throughout the system.
apps/webapp/app/hooks/useEnvironmentSwitcher.ts (2)
2-2: LGTM: Improved type safety with specific property selection.The change from a generic
MinimumEnvironmenttype toPick<RuntimeEnvironment, "id" | "slug">makes the function's requirements more explicit and type-safe.Also applies to: 13-13
87-88: LGTM: Enhanced regex pattern supports branch-aware environment slugs.The updated regex pattern allows any characters except "/" in environment slugs, which properly supports branch-specific environment identifiers while maintaining URL path integrity.
apps/webapp/test/environmentVariableRules.test.ts (1)
1-112: Excellent comprehensive test coverage for environment variable blacklisting.The test suite thoroughly covers all the key scenarios for the
removeBlacklistedVariablesfunction including exact matches, prefix filtering, whitelist exceptions, and edge cases. The test structure is clear and follows good testing practices.apps/webapp/app/models/api-key.server.ts (3)
100-102: Consistent function implementation update.The
envSlugfunction correctly returns "preview" for the PREVIEW environment type, maintaining consistency with the type definition.
106-108: Update type guard function correctly.The
isEnvSlugfunction is properly updated to validate "preview" instead of "prev", ensuring type safety is maintained.
87-87:✅ Verification successful
Standardize preview environment slug naming.
Good change to use the full "preview" slug instead of the abbreviated "prev" for better clarity and consistency.
Verify that this slug change from "prev" to "preview" is consistently applied across the entire codebase to avoid breaking changes:
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining usage of "prev" slug that should be updated to "preview" # Search for potential old "prev" slug usage in TypeScript/JavaScript files echo "Searching for remaining 'prev' slug usage..." rg -i "prev.*env|env.*prev" --type ts --type js -A 2 -B 2 # Search for API key patterns with old slug echo "Searching for API key patterns with old slug..." rg "tr_prev_|pk_prev_" --type ts --type js -A 2 -B 2Length of output: 29277
Slug change verified—no remaining “prev” usages
All instances of the old
"prev"slug have been replaced with"preview"across the codebase. No further updates are needed. Approving these changes.apps/webapp/app/models/organization.server.ts (3)
10-11: Improve import organization and type-only imports.Good refactoring to use type-only imports for
PrismaClientOrTransactionand reorganize import statements for better clarity.
119-119: Proper database field integration for branching support.The
isBranchableEnvironmentfield is correctly added to the database creation, enabling the new preview branch functionality.
79-93:✅ Verification successful
Excellent refactoring to named parameters pattern.
The change from positional arguments to a destructured object parameter significantly improves the function's maintainability and readability. The new
isBranchableEnvironmentparameter properly supports the preview branch functionality.Verify that all existing calls to
createEnvironmenthave been updated to use the new object parameter signature:
🏁 Script executed:
#!/bin/bash # Description: Find all calls to createEnvironment function to ensure they use the new signature # Search for function calls to createEnvironment echo "Searching for createEnvironment function calls..." rg "createEnvironment\s*\(" --type ts -A 5 -B 2 # Look for potential old-style positional argument calls echo "Searching for potential old positional argument patterns..." rg "createEnvironment\s*\(\s*[^{]" --type ts -A 3 -B 1Length of output: 5240
All
createEnvironmentinvocations use the new destructured-object signatureI ran a project-wide search and confirmed that every call to
createEnvironmentnow passes a single object (no legacy positional arguments remain).apps/webapp/package.json (4)
53-56: Add internal workspace dependencies for expanded functionality.The addition of
@internal/redis,@internal/tracing, and later@trigger.dev/redis-workeraligns with the preview branch feature requirements for enhanced backend services.
75-75: Add accordion UI component dependency.The
@radix-ui/react-accordiondependency supports new UI components for the preview branch management interface.
199-200: Add internal development dependencies.The
@internal/replicationand@internal/testcontainersdevDependencies support enhanced testing capabilities for the new branching features.
107-109:❓ Verification inconclusive
Update platform version for preview branch support.
The
@trigger.dev/platformversion update from 1.0.14 to 1.0.15 likely includes backend support for the new preview branch functionality.Verify that the new platform version is compatible and includes the expected preview branch features:
🌐 Web query:
What are the changes in @trigger.dev/platform version 1.0.15 compared to 1.0.14?💡 Result:
I couldn't locate specific release notes detailing the changes between versions 1.0.14 and 1.0.15 of
@trigger.dev/platform. However, Trigger.dev has undergone significant updates in recent versions, including a major overhaul to version 3.0.x. This update introduced a new build system with features like default bundling of dependencies, build extensions, improved configuration, enhanced error handling, and better cold start times. (trigger.mintlify.dev)Given these substantial changes, it's advisable to review the official Trigger.dev documentation or contact their support channels for detailed information on the specific updates between versions 1.0.14 and 1.0.15.
Citations:
Please verify that @trigger.dev/platform 1.0.15 includes preview-branch support
I wasn’t able to find specific changelog entries for 1.0.15 vs. 1.0.14. Before merging, please:
• Review the official Trigger.dev release notes or GitHub releases for v1.0.15
• Confirm that backend support for the new preview-branch feature is present
• Ensure compatibility with your existing workflow in apps/webapp/package.json (lines 107–109)internal-packages/run-engine/src/engine/db/worker.ts (3)
35-36: LGTM! Good addition to the failure code union type.The new
"RUN_ENVIRONMENT_ARCHIVED"failure code is properly integrated into the existing union type structure.
73-73: Correct database query enhancement.Adding
archivedAt: trueto the select query is necessary to support the new archived environment check.
93-100: Excellent early return pattern for archived environments.This check prevents unnecessary processing and database queries for runs on archived environments. The early return pattern is efficient and follows the existing error handling structure in the function.
apps/webapp/app/v3/services/checkSchedule.server.ts (2)
17-17: Parameter addition looks good.The new
environmentIdsparameter allows targeted validation of specific environments.
86-88: Good validation for archived environments.The check prevents schedule operations on archived branches, which is the correct behavior for the preview branches feature.
apps/webapp/app/assets/icons/EnvironmentIcons.tsx (2)
78-78: Good theming improvement.Changing from
fill="white"tofill="currentColor"allows the icon to inherit the text color dynamically, improving theming consistency across the application.
155-178: Well-designed new icons for preview branches.The new
PreviewEnvironmentIconSmallandBranchEnvironmentIconSmallcomponents are well-structured and follow the existing icon patterns. The use ofcurrentColorfor fill and stroke ensures consistent theming. The branch icon design effectively represents the branching concept.apps/webapp/app/routes/api.v1.projects.$projectRef.branches.archive.ts (4)
18-20: Proper HTTP method validation.Restricting to POST requests only is correct for this archival operation.
24-27: Good authentication implementation.Using personal access token authentication provides appropriate security for API access.
71-74: Proper archived state validation.The check for already archived branches prevents redundant operations and provides clear error messaging.
76-85: Clean service integration and error handling.The ArchiveBranchService integration is clean, and the success/failure response handling is appropriate with proper HTTP status codes.
packages/build/src/extensions/core/vercelSyncEnvVars.ts (1)
4-9: LGTM! Well-implemented branch support for Vercel sync.The implementation correctly:
- Adds optional branch parameter with proper fallback chain
- Extends environment mapping to include preview
- Conditionally includes gitBranch in the API request
- Maintains backward compatibility
Also applies to: 20-24, 48-48, 60-61
apps/webapp/app/presenters/v3/ApiKeysPresenter.server.ts (2)
53-59: Verify the orgMember filtering logic for non-dev environments.The conditional filtering only applies
orgMemberconstraint for "dev" environments. This means for other environments (staging, production, preview branches), any organization member can access the API keys regardless of theorgMemberassociation.Please confirm this is the intended behavior for security and access control.
68-72: Good implementation of API key inheritance.The fallback logic correctly implements API key inheritance for branch environments, using the parent environment's API key when available.
packages/cli-v3/src/build/bundle.ts (1)
347-347: LGTM! Clean addition of branch parameter.The branch parameter is correctly added as optional to maintain backward compatibility and properly propagated to the build manifest.
Also applies to: 357-357, 366-366
packages/cli-v3/src/commands/promote.ts (5)
15-16: LGTM: Clean import additions for git functionality.The imports are correctly added to support git metadata creation and branch detection functionality.
23-24: LGTM: Schema properly extended for preview environment support.The schema correctly adds "preview" to the environment enum and includes an optional branch parameter, maintaining backward compatibility.
41-44: LGTM: CLI option properly configured.The branch option is well-documented and follows CLI best practices with clear description and fallback behavior.
98-107: LGTM: Robust branch detection logic with proper error handling.The implementation correctly:
- Creates git metadata only when needed
- Applies branch detection only for preview environments
- Provides clear error message when auto-detection fails
- Uses the established
getBranchutility function
114-114: LGTM: Branch parameter correctly passed to project client.The branch is appropriately passed to
getProjectClient, maintaining consistency with the session utility expectations.packages/cli-v3/src/utilities/session.ts (4)
6-6: LGTM: Import addition for git metadata support.The GitMeta import is correctly added to support the new branch functionality.
98-98: LGTM: Backward-compatible type extension.The optional branch parameter maintains backward compatibility while enabling preview branch functionality.
129-129: LGTM: Branch parameter correctly passed to API client.The branch is appropriately passed to the CliApiClient constructor, enabling branch-aware API operations.
138-161: LGTM: Well-structured upsertBranch function with proper error handling.The implementation correctly:
- Defines clear type for options
- Creates API client with proper credentials
- Calls the appropriate API method with preview environment
- Includes proper error logging and handling
- Returns the expected data structure
packages/cli-v3/src/commands/preview.ts (6)
1-22: LGTM: Proper imports and dependencies.All necessary imports are included, following the established patterns from other CLI commands.
23-31: LGTM: Clean schema definition for command options.The PreviewCommandOptions schema properly extends CommonCommandOptions and includes the necessary fields for branch archiving.
32-60: LGTM: Well-structured command configuration.The command configuration follows CLI best practices with:
- Clear description and argument structure
- Appropriate options with helpful descriptions
- Proper integration with telemetry and banner systems
73-101: LGTM: Robust authentication and initialization.The implementation correctly:
- Handles optional update checks
- Verifies project directory
- Performs proper authentication with clear error messages
- Handles connection failures appropriately
103-121: LGTM: Proper branch detection with clear error handling.The branch detection logic correctly:
- Loads configuration with overrides
- Creates git metadata for branch detection
- Uses the established getBranch utility
- Provides clear error message when branch cannot be detected
122-146: LGTM: Clean archiving implementation with user feedback.The archiving process correctly:
- Provides user feedback with spinner
- Calls the appropriate archive function
- Displays success/failure messages appropriately
- Returns boolean result for further processing
apps/webapp/app/services/archiveBranch.server.ts (5)
6-11: LGTM: Clean service class with dependency injection.The service properly uses dependency injection with a default prisma client, following established patterns in the codebase.
15-41: LGTM: Comprehensive user authorization and data fetching.The implementation correctly:
- Verifies user is a member of the environment's organization
- Includes necessary related data (organization and project)
- Uses proper Prisma query patterns with includes
43-48: LGTM: Proper branch validation logic.The check for
parentEnvironmentIdcorrectly identifies whether an environment is a branch, providing a clear error message for non-branch environments.
50-63: LGTM: Robust archiving implementation with conflict prevention.The implementation correctly:
- Generates a unique slug using nanoid to prevent conflicts
- Updates both slug and shortcode consistently
- Sets the archivedAt timestamp
- Returns comprehensive data including branch, organization, and project info
64-71: LGTM: Comprehensive error handling with proper logging.The error handling correctly:
- Catches all exceptions
- Logs errors with context (environmentId and error details)
- Returns a consistent error response structure
- Avoids exposing internal error details to the client
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx (3)
204-208: Well-implemented fuzzy search integration.The fuzzy filter implementation is clean and properly integrated with the component state. Searching on both 'key' and 'value' fields provides good UX for finding environment variables.
264-290: Excellent search UI with good accessibility practices.The search input implementation includes:
- Proper placeholder text
- Auto-focus for better UX
- Clean layout with reveal toggle and add button
- Consistent styling with the design system
376-392: Smart empty state handling.Good differentiation between "no variables exist" vs "no variables match search" states. This provides clear feedback to users about whether they need to add variables or refine their search.
apps/webapp/app/components/navigation/EnvironmentBanner.tsx (2)
13-26: Clean component architecture with proper conditional rendering.The main component properly handles the conditional display of different banner types using AnimatePresence for smooth transitions. The logic correctly prioritizes archived state over paused state.
76-78:❓ Verification inconclusive
Verify branchName is always defined for archived environments.
The code accesses
environment.branchNamedirectly without null checking. While this may be safe based on the environment model, please verify thatbranchNameis guaranteed to be defined for archived environments.
🏁 Script executed:
#!/bin/bash # Description: Check if branchName is properly typed and validated for archived environments # Expected: Find type definitions and validation that ensure branchName exists for archived environments # Search for environment type definitions and branchName usage ast-grep --pattern 'interface $_ { $$$ branchName: $_ $$$ }' # Search for environment model definitions rg -A 5 -B 5 "branchName.*string" # Check for archived environment validation rg -A 3 -B 3 "archivedAt.*branchName"Length of output: 13963
🏁 Script executed:
#!/bin/bash # Show the EnvironmentBanner component signature and prop type rg -n "export function EnvironmentBanner" -A 30 apps/webapp/app/components/navigation/EnvironmentBanner.tsxLength of output: 1289
🏁 Script executed:
#!/bin/bash # 1. Extract ArchivedBranchBanner code rg -n "function ArchivedBranchBanner" -A 40 apps/webapp/app/components/navigation/EnvironmentBanner.tsx # 2. Inspect runtimeEnvironment.server type definitions sed -n '1,200p' apps/webapp/app/models/runtimeEnvironment.server.ts # 3. Search for branchName in prisma schemas rg -n "model RuntimeEnvironment" -A 20 -g "prisma/**/*.prisma" rg -n "branchName" -A 3 -g "prisma/**/*.prisma"Length of output: 5981
Verify
environment.branchNamefor archived environmentsThe
ArchivedBranchBannerdirectly interpolatesenvironment.branchNamewithout any guard. We need to confirm that wheneverenvironment.archivedAtis truthy,branchNamewill always be defined (i.e. archived environments are only branch previews, which require a branchName). If there’s any scenario where an environment can be archived without a branchName, add a null‐check or provide a sensible fallback.• Location:
• File:apps/webapp/app/components/navigation/EnvironmentBanner.tsx
• Component:ArchivedBranchBanner(around lines 75–78)
• Current code:<Paragraph variant="small" className="text-amber-200"> "{environment.branchName}" branch is archived and is read-only… </Paragraph>• Action:
- Confirm that archived environments always carry a non-null/undefined
branchName.- If not guaranteed, update to something like:
<Paragraph …> "{environment.branchName ?? environment.slug}" branch is archived… </Paragraph>apps/webapp/app/routes/resources.branches.archive.tsx (3)
18-26: Good schema design with proper validation structure.The Zod schema properly validates the required fields for archiving. The composition pattern using
.and()cleanly separates the core archiving options from the UI-specific redirect path.
28-51: Robust server action with proper error handling.The action function follows Remix best practices:
- Requires authentication upfront
- Validates form data with the schema
- Provides meaningful error messages
- Uses appropriate redirect patterns for success/error states
98-108: Excellent user communication about archiving consequences.The UI clearly explains the permanent and read-only nature of archiving, helping users understand the implications before proceeding. The progressive disclosure pattern with multiple paragraphs ensures users are fully informed.
apps/webapp/app/components/primitives/Accordion.tsx (3)
10-20: Proper component implementation with forward refs.The AccordionItem correctly forwards refs and merges custom className props with default styling. The group class enables child component hover states, which is a nice touch for the design system.
22-40: Excellent trigger implementation with smooth animations.The AccordionTrigger includes:
- Proper ref forwarding and flex layout
- Smooth chevron rotation animation using CSS transforms
- Hover states that integrate with the group styling
- Consistent spacing and typography
42-55: Well-implemented content with smooth open/close animations.The AccordionContent properly handles:
- Content overflow during animations
- Custom animation classes for smooth expand/collapse
- Consistent border and spacing
- Flexible className composition for customization
apps/webapp/app/components/BlankStatePanels.tsx (3)
440-479: LGTM: Well-structured component for branchable environment states.The
BranchesNoBranchableEnvironmentcomponent properly handles different scenarios based on whether the deployment is managed cloud or self-hosted, providing appropriate guidance and upgrade paths.
481-552: LGTM: Comprehensive handling of branch limits and creation flow.The
BranchesNoBranchescomponent effectively handles different states:
- Branch limit reached (with upgrade or request options)
- Available capacity (with creation dialog)
The integration with the
NewBranchPanelthrough a dialog pattern is clean and follows existing UI conventions.
554-554: Good enhancement: Dynamic title prop improves reusability.Adding the optional
titleprop with a sensible default maintains backward compatibility while enabling customization.apps/webapp/app/components/navigation/EnvironmentSelector.tsx (3)
67-96: LGTM: Robust environment filtering and rendering logic.The filtering logic correctly separates branch environments from parent environments and handles both branchable and non-branchable environments appropriately. The switch statement provides clear logic flow for different environment types.
116-129: Good addition: Preview environment upgrade option.Adding the Preview environment option alongside the existing Staging option maintains consistency in the upgrade flow and provides users with clear paths to unlock additional features.
138-280: Well-implemented branch submenu with proper UX considerations.The
Branchescomponent handles complex interaction patterns effectively:
- Mouse interaction: The 150ms delay on mouse leave prevents accidental menu closure
- State management: Proper categorization of branch states (no-branches, no-active-branches, has-branches)
- Cleanup: useEffect properly clears timeouts on unmount
- Navigation handling: Menu closes automatically on navigation changes
The archived branch handling and conditional rendering based on branch states is well thought out.
apps/webapp/app/components/GitMetadata.tsx (1)
1-87: Excellent implementation: Clean, consistent Git metadata components.This new component follows excellent patterns:
- Modular design: Separate components for each metadata type enable focused responsibilities
- Type safety: Using
Pickutility types ensures components only receive necessary props- Consistent UI: All sub-components use the same LinkButton and SimpleTooltip pattern
- Conditional rendering: Main component gracefully handles missing or null git metadata
- Null safety: GitMetadataPullRequest includes additional null checks for required fields
The component integrates well with the existing design system and provides a clean way to display Git context in deployment views.
apps/webapp/app/models/runtimeEnvironment.server.ts (3)
4-5: Good imports: Adding necessary dependencies for branch functionality.The import of
loggerandsanitizeBranchNamesupports the enhanced error handling and branch name normalization needed for the new functionality.
10-63: Critical enhancement: Branch-aware environment lookup with proper validation.The enhanced
findEnvironmentByApiKeyfunction correctly implements branch resolution:
- Branch filtering: Uses
sanitizeBranchNamefor consistency and excludes archived environments- PREVIEW environment handling: Properly requires branchName and resolves to child environments
- Error logging: Comprehensive logging for debugging authentication issues
- Security: Maintains existing project deletion checks
- Return consistency: Child environment inherits parent's authentication context
The logic correctly handles the case where a branch is specified but no matching child environment exists.
89-114: Good enhancement: Parent environment context for branch environments.Adding
parentEnvironmentwithapiKeyto the return type enables branch environments to access their parent's API key, which is essential for the branch authentication flow.apps/webapp/app/presenters/OrganizationsPresenter.server.ts (3)
1-1: LGTM! Import addition aligns with type usage.The
RuntimeEnvironmentimport is correctly added to support the enhanced type annotation for environment fields.
79-82: LGTM! Environment query expanded with branch metadata.The addition of branch-related fields (
isBranchableEnvironment,branchName,parentEnvironmentId,archivedAt) properly extends the environment data model to support preview branches functionality.
179-193: LGTM! Type annotation correctly reflects expanded data model.The type annotation accurately captures all the fields being selected from the database query, ensuring type safety throughout the presenter.
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (4)
198-202: LGTM! Branches pricing definition follows established pattern.The pricing definition for branches is properly structured with appropriate title and content, consistent with other feature definitions.
502-502: LGTM! Branches component properly integrated across all pricing tiers.The
Branchescomponent is consistently added to all three pricing tiers (Free, Hobby, Pro) in the appropriate position within the feature lists.Also applies to: 617-619, 736-736
949-949: LGTM! Environment labeling updated to reflect preview branch model.The change from "Dev, Staging and Prod" to "Dev, Preview and Prod" correctly reflects the new environment structure with preview branches.
1030-1043: LGTM! Branches component implementation follows established patterns.The
Branchescomponent properly:
- Uses the same structure as other feature components
- Implements conditional checking based on branch limits
- Includes tooltip functionality with appropriate definitions
- Handles both singular and plural cases with canExceed logic
packages/cli-v3/src/deploy/buildImage.ts (5)
38-38: LGTM! Optional branchName property added to build options.The
branchNamefield is correctly added as optional to maintain backward compatibility while enabling branch context support.
69-69: LGTM! Branch name properly propagated through build functions.The
branchNameparameter is correctly destructured and passed through to both self-hosted and depot build implementations.Also applies to: 92-92, 130-130
152-152: LGTM! Interface extensions maintain consistency.Both
DepotBuildImageOptionsandSelfHostedBuildImageOptionsproperly include the optionalbranchNamefield, maintaining interface consistency across build methods.Also applies to: 302-302
210-212: LGTM! Build arguments correctly handle branch context.The
TRIGGER_PREVIEW_BRANCHbuild argument is properly added to both depot and self-hosted builds with appropriate fallback to empty string whenbranchNameis undefined.Also applies to: 340-342
585-585: LGTM! Dockerfile templates properly expose branch environment variable.Both Node and Bun Dockerfiles correctly:
- Declare
ARG TRIGGER_PREVIEW_BRANCHbuild argument- Set
ENV TRIGGER_PREVIEW_BRANCHenvironment variable from the build arg- Follow consistent patterns across runtime environments
Also applies to: 594-594, 691-691, 700-700
apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.environments.staging.ts (3)
1-7: LGTM! Import statement properly expanded for enhanced functionality.The imports are correctly updated to include the additional database types needed for the refactored environment handling logic.
63-72: LGTM! Environment creation refactored to support both staging and preview.The logic properly:
- Creates both STAGING (non-branchable) and PREVIEW (branchable) environments
- Tracks creation status for accurate counting
- Maintains backward compatibility with existing staging environment creation
77-98: LGTM! Well-structured helper function reduces code duplication.The
upsertEnvironmenthelper function effectively:
- Consolidates environment creation/update logic
- Properly handles concurrency limit updates for both scenarios
- Returns clear status indicators for tracking
- Accepts the
isBranchableEnvironmentparameter to differentiate environment typesapps/webapp/app/components/environments/EnvironmentLabel.tsx (1)
1-125: LGTM! The branch support implementation is clean and consistent.The changes correctly extend the environment components to support branch-specific labeling and iconography. The implementation properly prioritizes branch names over environment types when displaying labels and icons, which aligns well with the preview branches feature.
apps/webapp/app/presenters/v3/BranchesPresenter.server.ts (2)
43-189: Well-structured presenter implementation!The
BranchesPresenterclass is well-designed with proper error handling, user permission checks, and graceful handling of missing data. The pagination logic and search/filter functionality are correctly implemented.
191-243: Robust Git metadata processing!The
processGitMetadatafunction handles URL cleaning comprehensively, supporting both SSH and HTTPS formats with proper fallback logic. The validation of required fields and graceful null returns for incomplete data are well implemented.packages/cli-v3/src/apiClient.ts (3)
652-663: Good refactoring to centralize header construction!The
getHeadersmethod effectively reduces code duplication across the client and ensures consistent header handling. The conditional addition of thex-trigger-branchheader is cleanly implemented.
166-183: LGTM!The
upsertBranchmethod follows the established patterns and includes proper access token validation.
185-199: Consistent implementation for branch archival!The
archiveBranchmethod correctly implements the branch archival endpoint with proper authentication and header handling.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables.new/route.tsx (3)
190-244: Well-designed state management for environment selection!The separation of environment IDs and branch ID state, along with the exclusive selection logic between PREVIEW and non-PREVIEW environments, ensures a consistent and predictable user experience.
348-388: Intuitive branch selection UI!The conditional branch selector with "All branches" default and the clear button provides excellent user experience. The hint text clearly explains the purpose of branch selection.
589-612: Improved error message placement!Moving error messages directly below their respective input fields enhances form usability and makes it clearer which field has an error.
packages/cli-v3/src/utilities/gitMeta.ts (1)
8-48: Well-structured Git metadata extraction!The
createGitMetafunction implements a smart fallback strategy, checking GitHub Actions environment first before falling back to local Git commands. The use ofPromise.allSettledfor parallel operations with proper error handling is excellent.internal-packages/database/prisma/schema.prisma (2)
386-398: LGTM! Well-structured schema changes for preview branches.The schema additions for preview branches are properly designed:
- Self-referential relationships with appropriate cascade deletes
- Git metadata as flexible JSON field
- Soft deletion support via
archivedAt- Performance-optimized indexes on frequently queried fields
Also applies to: 466-468
2885-2887: Consistent Git metadata tracking.The
gitfield addition toWorkerDeploymentaligns with theRuntimeEnvironmentmodel changes, enabling Git metadata tracking at the deployment level.apps/webapp/app/services/upsertBranch.server.ts (2)
147-168: Well-implemented branch limit checking.The function correctly counts only active (non-archived) branches and provides clear usage information.
201-213: Clean branch name extraction from Git refs.The function properly handles various Git ref formats and safely rejects unknown patterns.
packages/cli-v3/src/commands/deploy.ts (4)
46-47: Clean integration of preview environment support.The schema and CLI option additions properly extend the deploy command to support preview branches.
Also applies to: 79-82
229-240: Good auto-archiving logic for merged/closed PRs.The automatic archiving of branches when PRs are merged or closed is a nice UX touch that prevents clutter.
242-258: Proper branch upsertion with error handling.The branch upsertion logic correctly passes Git metadata and handles failures appropriately.
265-265: Consistent branch propagation through deployment pipeline.Branch and Git metadata are properly passed through all stages of the deployment process.
Also applies to: 288-288, 325-325, 437-437
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (5)
460-465: LGTM!The Prisma query correctly includes the
parentEnvironmentrelation when creating worker instances.
488-491: LGTM!The type definition correctly extends
RuntimeEnvironmentwith an optionalparentEnvironmentproperty.
502-502: LGTM!The
EnvironmentWithParenttype is consistently applied to both the options interface and class property.Also applies to: 512-512
697-700: LGTM!The query correctly includes the
parentEnvironmentrelation, maintaining consistency with other queries in the file.
706-708: LGTM!The
parentEnvironmentis correctly propagated through thegetEnvVarsmethod toresolveVariablesForEnvironment, ensuring environment variables are properly resolved with parent context.Also applies to: 812-816
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.apikeys/route.tsx (4)
50-58: LGTM!The loader correctly parses the environment parameter and fetches a single environment's data.
73-87: LGTM!Proper error handling for missing environments and correct construction of environment variable block with optional branch support.
111-147: LGTM!The UI correctly displays environment-specific API keys with proper security masking and regeneration functionality.
148-189: LGTM!The UI properly handles branch-specific information and provides clear instructions for environment variable setup.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx (5)
82-106: LGTM!The loader correctly handles authentication, parameter parsing, and error handling with appropriate user feedback.
122-154: LGTM!The action function properly handles branch creation with good validation, error handling, and user feedback for duplicate branches.
434-480: LGTM!The filter component correctly handles search throttling and URL state management with proper pagination reset.
531-590: LGTM!The branch creation form is well-implemented with proper validation and helpful character restriction hints.
🧰 Tools
🪛 Biome (1.9.4)
[error] 565-565: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
363-425: LGTM!Excellent implementation of the usage limits display with clear visual indicators and appropriate calls to action.
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (2)
589-620: Excellent implementation of hierarchical secret merging.The use of Map for merging secrets with child values overriding parent values is the correct approach. The blacklist filtering is appropriately applied after merging to ensure consistency.
1054-1079: Well-designed blacklist filtering implementation.The three-tier rule system (whitelist, exact match, prefix) provides good flexibility while maintaining security. The precedence order (whitelist first) is correct, ensuring that exceptions like
OTEL_LOG_LEVELwork as intended.apps/webapp/app/services/apiAuth.server.ts (2)
400-447: Good separation of branch and non-branch handling logic.The implementation correctly handles both cases:
- Non-branch environments are returned directly
- Branch environments are validated to ensure they have a parent and are augmented with the parent's API key
The error messages are clear and specific to each failure case.
371-379:❌ Incorrect review comment
Verify the branch matching logic.
The condition checks if either the environment slug doesn't match OR the branch name doesn't match. This could incorrectly reject valid requests when:
- A branch environment is accessed with its parent's slug
- A non-branch environment is accessed without specifying a branch
Consider if this should use AND logic or handle branch and non-branch environments separately.
🏁 Script executed:
#!/bin/bash # Search for how branch environments relate to their parent slugs ast-grep --pattern 'slug: $_' rg -A 5 -B 5 'parentEnvironment.*slug'Length of output: 1751
No changes needed to the branch-matching condition
The
&&checkif (auth.result.environment.slug !== slug && auth.result.environment.branchName !== branch) { throw … }correctly throws only when neither the slug nor the branch name match. It allows:
- Branch requests where
slugis the parent’s slug butbranchNamematches.- Non-branch requests where
slugmatches andbranchNameisundefined.You can safely ignore the previous concern about switching to
||.Likely an incorrect or invalid review comment.
...ma/migrations/20250511145836_runtime_environment_add_is_branchable_environment/migration.sql
Show resolved
Hide resolved
| if (!environment) { | ||
| return { | ||
| success: false, | ||
| error: `${env === "staging" ? "Staging" : "Production"} environment not found`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Update the error message to handle the "preview" environment case.
The error message only handles "staging" and defaults to "Production" for all other cases, which would incorrectly show "Production environment not found" for preview environments.
if (!environment) {
return {
success: false,
- error: `${env === "staging" ? "Staging" : "Production"} environment not found`,
+ error: `${env.charAt(0).toUpperCase() + env.slice(1)} environment not found`,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!environment) { | |
| return { | |
| success: false, | |
| error: `${env === "staging" ? "Staging" : "Production"} environment not found`, | |
| }; | |
| } | |
| if (!environment) { | |
| return { | |
| success: false, | |
| error: `${env.charAt(0).toUpperCase() + env.slice(1)} environment not found`, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts around lines 136
to 141, the error message only distinguishes between "staging" and "production"
environments, incorrectly labeling "preview" environments as "Production".
Update the error message logic to explicitly handle the "preview" environment
case, returning a message like "Preview environment not found" when env equals
"preview", while keeping the existing messages for "staging" and "production".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx (1)
366-387: Consider extracting the progress indicator to a reusable component.The SVG circle progress indicator contains complex calculations that could be extracted into a separate component for better maintainability and reusability.
Create a reusable
CircularProgresscomponent:interface CircularProgressProps { value: number; max: number; size?: number; strokeWidth?: number; showDanger?: boolean; } function CircularProgress({ value, max, size = 24, strokeWidth = 4, showDanger }: CircularProgressProps) { const percentage = (value / max) * 100; const radius = (size - strokeWidth) / 2; const circumference = 2 * Math.PI * radius; const strokeDasharray = `${(percentage / 100) * circumference} ${circumference}`; return ( <svg className="h-full w-full -rotate-90 overflow-visible" width={size} height={size}> <circle className="fill-none stroke-grid-bright" strokeWidth={strokeWidth} r={radius} cx={size / 2} cy={size / 2} /> <circle className={`fill-none ${showDanger ? "stroke-error" : "stroke-success"}`} strokeWidth={strokeWidth} r={radius} cx={size / 2} cy={size / 2} strokeDasharray={strokeDasharray} strokeDashoffset="0" strokeLinecap="round" /> </svg> ); }Then use it in the SimpleTooltip:
-<div className="size-6"> - <svg className="h-full w-full -rotate-90 overflow-visible"> - <circle - className="fill-none stroke-grid-bright" - strokeWidth="4" - r="10" - cx="12" - cy="12" - /> - <circle - className={`fill-none ${ - requiresUpgrade ? "stroke-error" : "stroke-success" - }`} - strokeWidth="4" - r="10" - cx="12" - cy="12" - strokeDasharray={`${(limits.used / limits.limit) * 62.8} 62.8`} - strokeDashoffset="0" - strokeLinecap="round" - /> - </svg> -</div> +<div className="size-6"> + <CircularProgress + value={limits.used} + max={limits.limit} + showDanger={requiresUpgrade} + /> +</div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/BlankStatePanels.tsx(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/components/BlankStatePanels.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
[error] 587-587: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx (1)
223-223:✅ Verification successful
Verify the
textAlignLeftprop is supported by the Button component.The
textAlignLeftprop may not be a valid prop for the Button component. This could cause a TypeScript error or runtime warning.Run the following script to check if the Button component supports this prop:
🏁 Script executed:
#!/bin/bash # Description: Check if Button component accepts textAlignLeft prop # Search for Button component definition and its props ast-grep --pattern 'interface ButtonProps { $$$ }' # Also check the Button component implementation fd -e tsx -e ts "Button" --exec grep -l "export.*Button" {} \; | head -5 | xargs cat | grep -A 20 "textAlignLeft"Length of output: 1869
No issues with the
textAlignLeftprop on ButtonVerified that the
ButtonPropsinterface declarestextAlignLeft?: boolean;and the
ButtonContentimplementation correctly handles it, so thevariant="primary/small"usage withtextAlignLeftis valid. No changes required.
...p/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/v3/validGitBranch.ts (1)
6-6: Minor: Simplify regex by removing unnecessary escaping.The regex contains some unnecessary character escaping that can be simplified for better readability.
Apply this diff to remove unnecessary escaping:
- if (/[ \~\^:\?\*\[\\]/.test(branch)) return false; + if (/[ ~^:?*\[\\]/.test(branch)) return false;In character classes,
~,^,?, and*don't need escaping since they don't have special meaning within[].
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/services/upsertBranch.server.ts(1 hunks)apps/webapp/app/v3/validGitBranch.ts(1 hunks)apps/webapp/test/validateGitBranchName.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/test/validateGitBranchName.test.ts
- apps/webapp/app/services/upsertBranch.server.ts
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- 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 (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 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 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/webapp/app/v3/validGitBranch.ts (2)
1-30: Overall implementation looks solid.The function correctly implements the majority of Git branch naming rules and provides comprehensive validation for the preview branches feature. The logic is clear and well-structured with appropriate comments.
1-30: 🛠️ Refactor suggestionAdd missing Git branch name validation rules.
The function implements most Git branch naming rules correctly, but it's missing a few important edge cases that Git disallows:
- Branch names cannot start with '.' (e.g., ".hidden" is invalid)
- Branch names cannot start with '-' (e.g., "-branch" is invalid)
- Branch names cannot be exactly '@' (special Git reference)
Apply this diff to add the missing validation rules:
export function isValidGitBranchName(branch: string): boolean { // Must not be empty if (!branch) return false; + // Cannot start with '.' or '-', or be exactly '@' + if (branch.startsWith(".") || branch.startsWith("-") || branch === "@") return false; + // Disallowed characters: space, ~, ^, :, ?, *, [, \ if (/[ \~\^:\?\*\[\\]/.test(branch)) return false;Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/webapp/app/v3/gitBranch.ts (1)
32-44: Add documentation and consider edge case handling.The sanitization function is well-implemented and handles the major Git reference formats correctly. Consider adding JSDoc documentation for consistency.
Apply this diff to add documentation:
+/** + * Sanitizes a Git reference string to extract the branch name. + * Removes known Git reference prefixes and validates the format. + * @param ref The Git reference string to sanitize + * @returns The sanitized branch name or null if the reference format is unknown + */ export function sanitizeBranchName(ref: string): string | null { if (!ref) return null; if (ref.startsWith("refs/heads/")) return ref.substring("refs/heads/".length); if (ref.startsWith("refs/remotes/")) return ref.substring("refs/remotes/".length); if (ref.startsWith("refs/tags/")) return ref.substring("refs/tags/".length); if (ref.startsWith("refs/pull/")) return ref.substring("refs/pull/".length); if (ref.startsWith("refs/merge/")) return ref.substring("refs/merge/".length); if (ref.startsWith("refs/release/")) return ref.substring("refs/release/".length); //unknown ref format, so reject if (ref.startsWith("refs/")) return null; return ref; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/services/upsertBranch.server.ts(1 hunks)apps/webapp/app/v3/gitBranch.ts(1 hunks)apps/webapp/test/validateGitBranchName.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/test/validateGitBranchName.test.ts
- apps/webapp/app/services/upsertBranch.server.ts
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/v3/environmentVariableRules.server.ts (1)
15-40: Optimize performance by preprocessing blacklist rules.The current implementation performs multiple linear searches for each variable, resulting in O(n²) complexity. Consider optimizing for better performance:
+// Preprocess rules for O(1) lookups +const exactBlacklist = new Set( + blacklistedVariables.filter(r => r.type === "exact").map(r => r.key) +); +const whitelist = new Set( + blacklistedVariables.filter(r => r.type === "whitelist").map(r => r.key) +); +const prefixBlacklist = blacklistedVariables + .filter(r => r.type === "prefix") + .map(r => r.prefix); export function removeBlacklistedVariables( variables: EnvironmentVariable[] ): EnvironmentVariable[] { return variables.filter((v) => { - const whitelisted = blacklistedVariables.find( - (bv) => bv.type === "whitelist" && bv.key === v.key - ); - if (whitelisted) { + if (whitelist.has(v.key)) { return true; } - const exact = blacklistedVariables.find((bv) => bv.type === "exact" && bv.key === v.key); - if (exact) { + if (exactBlacklist.has(v.key)) { return false; } - const prefix = blacklistedVariables.find( - (bv) => bv.type === "prefix" && v.key.startsWith(bv.prefix) - ); - if (prefix) { + if (prefixBlacklist.some(prefix => v.key.startsWith(prefix))) { return false; } return true; }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/v3/environmentVariableRules.server.ts(1 hunks)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts(10 hunks)apps/webapp/test/environmentVariableRules.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/test/environmentVariableRules.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (3)
apps/webapp/app/v3/environmentVariableRules.server.ts (1)
removeBlacklistedVariables(15-40)packages/core/src/v3/schemas/api.ts (3)
EnvironmentVariableWithSecret(835-848)EnvironmentVariableWithSecret(849-849)EnvironmentVariable(827-830)apps/webapp/app/v3/environmentVariables/repository.ts (2)
EnvironmentVariableWithSecret(82-84)EnvironmentVariable(77-80)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (19)
apps/webapp/app/v3/environmentVariableRules.server.ts (2)
3-6: Well-structured type definition.The union type
VariableRuleis well-designed and provides a clear, extensible structure for different filtering rule types.
8-13: Good blacklist configuration with appropriate security measures.The blacklisted variables correctly prevent exposure of sensitive system variables like
TRIGGER_SECRET_KEYandTRIGGER_API_URL, while appropriately blocking OTEL variables with a whitelist exception forOTEL_LOG_LEVEL.apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (17)
1-1: Good use of type-only imports.Using
typeimports for Prisma types follows best practices and reduces bundle size.
9-17: Consistent type import organization.The type imports are well-organized and use the
typemodifier appropriately for better tree-shaking.
18-18: Excellent centralization of blacklist filtering.Importing the centralized
removeBlacklistedVariablesfunction improves maintainability and consistency across the codebase.
93-94: Good integration of centralized filtering.Replacing the hardcoded filtering with the centralized
removeBlacklistedVariablesfunction improves consistency and maintainability.
99-104: Improved error messaging for blacklisted variables.The enhanced error message that mentions blacklisted variables provides better user feedback when variables are filtered out.
515-517: Good API design for hierarchical environments.Adding the optional
parentEnvironmentIdparameter maintains backward compatibility while enabling the new hierarchical functionality.
523-525: Correct secret redaction logic for hierarchical environments.The query correctly includes both child and parent environment IDs when determining which variables should be redacted, ensuring proper security handling in the hierarchical structure.
590-620: Well-implemented hierarchical secret merging with proper precedence.The merging logic correctly prioritizes child environment variables over parent ones using a Map for deduplication. The use of
removeBlacklistedVariablesensures consistent filtering.
811-811: Essential addition for preview branch support.Adding
branchNameto the payload type is necessary for the new preview branch functionality.
822-824: Clean API for parent environment support.The updated function signature with optional
parentEnvironmentparameter maintains backward compatibility while enabling hierarchical resolution.
827-829: Proper integration of hierarchical variable resolution.Passing the parent environment ID to
getEnvironmentVariablescorrectly enables the hierarchical lookup functionality.
836-836: Consistent parent environment propagation.Passing the
parentEnvironmenttoresolveBuiltInProdVariablesensures consistent behavior across the resolution pipeline.
838-843: Excellent deduplication strategy with clear precedence rules.The use of
deduplicateVariableArrayensures that variables are properly deduplicated with the correct precedence order (overridable < project secrets < built-in variables).
846-856: Efficient and well-documented deduplication logic.The deduplication function correctly implements "later variables override earlier ones" semantics using a reverse-process-reverse pattern. The algorithm is efficient and the comments clearly explain the behavior.
933-936: Consistent parent environment support in built-in variables.Adding the optional
parentEnvironmentparameter toresolveBuiltInProdVariablesmaintains consistency with the hierarchical architecture.
940-940: Correct API key inheritance for preview branches.Using the parent environment's API key when available ensures that preview branches inherit the correct authentication credentials from their parent environment.
960-967: Valuable addition for preview branch identification.Adding the
TRIGGER_PREVIEW_BRANCHenvironment variable whenbranchNameis present allows runtime code to identify and handle preview branch contexts appropriately.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/webapp/app/v3/deduplicateVariableArray.server.ts (1)
4-14: LGTM with a minor performance consideration.The deduplication logic is correct and clearly implements the "later variables override earlier ones" semantics. The approach of reversing twice maintains the original order while ensuring proper precedence.
For large arrays, consider this more efficient approach that avoids multiple array reversals:
export function deduplicateVariableArray(variables: EnvironmentVariable[]) { - const result: EnvironmentVariable[] = []; - // Process array in reverse order so later variables override earlier ones - for (const variable of [...variables].reverse()) { - if (!result.some((v) => v.key === variable.key)) { - result.push(variable); - } - } - // Reverse back to maintain original order but with later variables taking precedence - return result.reverse(); + const seen = new Set<string>(); + const result: EnvironmentVariable[] = []; + + // Process from end to start to prioritize later variables + for (let i = variables.length - 1; i >= 0; i--) { + const variable = variables[i]; + if (!seen.has(variable.key)) { + seen.add(variable.key); + result.unshift(variable); + } + } + + return result; }This optimization uses O(n) time complexity instead of O(n²) for the
some()checks and avoids creating multiple array copies.apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (1)
948-955: Consider validation for the TRIGGER_PREVIEW_BRANCH variable.While the logic is correct, consider adding validation to ensure the branch name meets any required format constraints before setting it as an environment variable.
if (runtimeEnvironment.branchName) { + // Validate branch name format if needed + const validBranchName = runtimeEnvironment.branchName.trim(); + if (!validBranchName) { + throw new Error("Invalid branch name for preview environment"); + } result = result.concat([ { key: "TRIGGER_PREVIEW_BRANCH", - value: runtimeEnvironment.branchName, + value: validBranchName, }, ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/v3/deduplicateVariableArray.server.ts(1 hunks)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts(10 hunks)apps/webapp/test/environmentVariableDeduplication.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/test/environmentVariableDeduplication.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- 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 (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts (7)
1-1: Good use of type imports and proper integration of utility functions.The updated imports correctly use the
typekeyword for types and integrate the new utility functions for deduplication and blacklist filtering. This follows TypeScript best practices and maintains clean dependencies.Also applies to: 9-19
94-106: Well-integrated blacklist filtering with informative error messages.The integration of
removeBlacklistedVariablescentralizes the filtering logic and provides clear user feedback when blacklisted variables are removed. The error message properly informs users about why variables were ignored.
514-551: Correct implementation of hierarchical secret redaction.The method properly handles both parent and child environments when determining which variables to redact. The logic correctly queries both environment IDs when a parent is present and maintains the redaction semantics.
582-622: Robust secret merging logic with proper override semantics.The implementation correctly merges parent and child secrets using a Map to ensure child secrets override parent ones. The use of
removeBlacklistedVariablesat the end ensures compliance with security rules.
822-844: Excellent integration of deduplication with hierarchical variable resolution.The function properly combines variables from multiple sources (overridable, project secrets, built-in) and applies deduplication to ensure proper precedence. The parameter changes to support parent environments are well-integrated.
921-955: Well-implemented branch-aware built-in variables.The function correctly uses the parent environment's API key when available and conditionally adds the
TRIGGER_PREVIEW_BRANCHvariable when a branch name is present. This supports the preview branch functionality effectively.
812-813:✅ Verification successful
Verify branchName field exists in database schema.
The addition of
branchNameto the payload selection assumes this field exists in the RuntimeEnvironment model.
🏁 Script executed:
#!/bin/bash # Description: Verify branchName field exists in RuntimeEnvironment schema # Expected: branchName field should be present in the model definition ast-grep --pattern $'model RuntimeEnvironment { $$$ branchName $$$ $$$ }'Length of output: 80
🏁 Script executed:
#!/bin/bash # Locate the RuntimeEnvironment model in Prisma schema files rg --glob '*.prisma' "model RuntimeEnvironment" -n . # Check if the branchName field is defined within any Prisma schema rg --glob '*.prisma' "branchName" -n .Length of output: 351
branchName field verified in RuntimeEnvironment schema
The
branchNamefield is defined asString?in theRuntimeEnvironmentmodel (seeinternal-packages/database/prisma/schema.prisma:388). No further action required.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/services/apiAuth.server.ts(17 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/webapp/app/services/apiAuth.server.ts (3)
apps/webapp/app/models/runtimeEnvironment.server.ts (2)
findEnvironmentByPublicApiKey(66-87)findEnvironmentByApiKey(10-63)apps/webapp/app/db.server.ts (1)
prisma(100-100)apps/webapp/app/v3/gitBranch.ts (1)
sanitizeBranchName(32-44)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- 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 (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- 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 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
apps/webapp/app/services/apiAuth.server.ts (10)
2-2: Good use of type imports for better bundle optimization.Using
typeimports for types that are only used in type annotations helps with tree-shaking and bundle size optimization.Also applies to: 12-12, 15-15
60-60: Consistent branch name extraction across authentication entry points.Both authentication functions now properly extract and utilize the branch name from the request headers.
Also applies to: 79-79
265-267: Simple and effective branch name extraction.The function correctly extracts the branch name from the
x-trigger-branchheader and handles the null case appropriately.
269-277: Well-structured API key and branch extraction.The function properly separates concerns by extracting both the API key and branch name, returning them as a structured object for consistent usage across the codebase.
98-98: Consistent function signature updates for branch support.Both the deprecated
authenticateApiKeyand the newauthenticateApiKeyWithFailurefunctions correctly accept the optionalbranchNameparameter.Also applies to: 165-165
116-116: Proper propagation of branch name to environment lookup functions.The branch name is consistently passed to both public and private environment lookup functions across all authentication paths.
Also applies to: 128-128, 192-192, 207-207
163-163: Good encapsulation by making internal functions private.Making
authenticateApiKeyWithFailure,isPublicApiKey, andisSecretApiKeyinternal functions improves the API surface and prevents external usage of implementation details.Also applies to: 257-257, 261-261
371-371: Enhanced environment validation for branch support.The validation now correctly checks both the environment slug and branch name, allowing proper routing for preview branches.
400-447:✅ Verification successful
Robust personal access token authentication for preview branches.
The implementation correctly handles both regular and branch-specific environment lookup. The validation ensures that:
- Branch environments are not archived
- Preview environments have a parent environment
- The returned environment includes the parent's API key
However, let's verify the branch name sanitization is applied consistently:
🏁 Script executed:
#!/bin/bash # Description: Verify sanitizeBranchName is used consistently across the codebase # Expected: All branch name handling should use sanitization rg "branchName.*sanitize|sanitize.*branchName" -A 2 -B 2Length of output: 1703
Branch name sanitization is applied consistently across the codebase.
I verified that every branch‐name lookup and write uses
sanitizeBranchName:• apps/webapp/app/services/apiAuth.server.ts –
branchName: sanitizeBranchName(branch)
• apps/webapp/app/services/upsertBranch.server.ts –const sanitizedBranchName = sanitizeBranchName(branchName)and used on creation
• apps/webapp/app/models/runtimeEnvironment.server.ts –branchName: sanitizeBranchName(branchName)No further action required.
423-423:✅ Verification successful
Verify sanitizeBranchName behavior with null/undefined values.
The
sanitizeBranchNamefunction is called with a potentially undefinedbranchparameter. Let's ensure it handles null/undefined values gracefully.
🏁 Script executed:
#!/bin/bash # Description: Check if sanitizeBranchName handles null/undefined input properly # Expected: Function should return null for invalid inputs ast-grep --pattern 'function sanitizeBranchName($_) { if (!$_) return null; $$$ }'Length of output: 88
🏁 Script executed:
#!/bin/bash # Locate all references to sanitizeBranchName rg -n "sanitizeBranchName" . # Specifically find its declaration variants rg -n "function sanitizeBranchName" . rg -n "sanitizeBranchName\s*=" .Length of output: 1781
🏁 Script executed:
#!/bin/bash # Display the sanitizeBranchName function implementation sed -n '1,200p' apps/webapp/app/v3/gitBranch.tsLength of output: 1581
No action needed: sanitizeBranchName already handles null/undefined
ThesanitizeBranchNameimplementation begins with:if (!ref) return null;so passing
undefinedornullwill safely returnnullrather than throwing.
The "Preview" environment is a special environment where you can create "branches". Normally you would automatically create a preview branch that matches your git branch when a Pull Request is opened (and then archive it automatically when the PR is merged/closed).
The process to use preview branches looks like this:
There are two main ways to do this:
Changes
OTEL_.Go live steps