Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExpose auto-save via the environment provider and thread it into FormSettingCard usages; reorganize deployment settings into section groups; initialize default environment settings during onboarding; add silent mode for settings mutations; centralize deploy-step failure reporting and add redeploy/skipped handling; adjust deployments table actions and various UI sizing/navigation tweaks. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/deployments-list.tsx:
- Line 71: Replace the invalid Tailwind spacing token "px-4.5" in the className
on the deployments list container with an arbitrary value "px-[18px]" so it
matches the arbitrary pattern used elsewhere (e.g., "pl-[18px]"); locate the JSX
element that contains className="flex flex-col items-start px-4.5 py-1.5" (in
the DeploymentsList component) and update that token to px-[18px].
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/shared/form-setting-card.tsx:
- Around line 54-62: The form-level onBlur in form-setting-card.tsx currently
calls requestSubmit() when focus leaves the form (guarded by autoSave and
saveState.status), which incorrectly fires when focus moves into portal-mounted
controls like Popover/Combobox because relatedTarget lies outside the form;
update the onBlur handler to ignore transitions into known managed portals or
portal hosts (e.g., detect a data-attribute/class on portal roots or consult an
EnvironmentSettingsProvider flag) before calling requestSubmit, or remove
form-level autosave and instead wire autosave to explicit field-level commit
events (e.g., Combobox selection handlers) so requestSubmit is only invoked from
components that know when a user finished interacting; reference the onBlur
handler, autoSave, saveState.status, relatedTarget, and requestSubmit in your
changes.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsx:
- Around line 24-25: The component currently uses the `sections` prop to only
control expansion but still renders all sections; update rendering so each
section is conditionally rendered based on `sections[sectionName]` (e.g., use
`sections.build` to decide whether to render the Build section) and ensure
`sections.build` is actually referenced where the Build section JSX is produced;
adjust the render logic for "advanced", "sentinel", "runtime", and "build" so
collapsed vs omitted behavior is consistent (use `DeploymentSection` keys to map
visibility and expansion state), and remove any dead/unused `sections.build`
declaration if it becomes redundant.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/new/steps/onboarding-environment-provider.tsx:
- Around line 93-100: The current checks treat numeric 0 as empty and overwrite
valid zeros; update the assignments for draft.port, draft.cpuMillicores, and
draft.memoryMib to use nullish checking (e.g., use the nullish
coalescing/assignment or explicit null/undefined checks) so only null or
undefined values get defaulted to 8080, 256, and 256 respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4377f1cc-9798-4de5-838c-b3924fba8be9
📒 Files selected for processing (23)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/deployments-list.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/dockerfile-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/build-settings/root-directory-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/command.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/cpu.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/healthcheck/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/instances.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/memory.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/port-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/runtime-settings/regions.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/sentinel-settings/keyspaces.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/shared/form-setting-card.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/shared/settings-group.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/environment-provider.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/components/status-indicator.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/new/steps/configure-deployment.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/new/steps/onboarding-environment-provider.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/new/steps/select-repo/language-icon.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/new/steps/select-repo/repo-list-item.tsxweb/apps/dashboard/app/(app)/integrations/github/callback/page.tsxweb/apps/dashboard/lib/collections/deploy/environment-settings.ts
💤 Files with no reviewable changes (1)
- web/apps/dashboard/app/(app)/integrations/github/callback/page.tsx
...spaceSlug]/projects/[projectId]/(overview)/deployments/components/table/deployments-list.tsx
Show resolved
Hide resolved
...kspaceSlug]/projects/[projectId]/(overview)/settings/components/shared/form-setting-card.tsx
Show resolved
Hide resolved
...d/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsx
Show resolved
Hide resolved
...s/dashboard/app/(app)/[workspaceSlug]/projects/new/steps/onboarding-environment-provider.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@svc/ctrl/worker/deploy/deploy_handler.go`:
- Around line 92-103: The defer closure that sets the deployment step failure
message reads the outer err which can be stale due to multiple returns and
shadowed loop errors; change the function to use a single named return error
(e.g., retErr error) and have the defer read retErr (not outer err) so it always
reports the real final error; ensure every return assigns to retErr (e.g.,
retErr = runtimeSettingsErr) before returning, stop reassigning retErr in
cleanup calls like w.updateDeploymentStatus by using a separate local variable
for cleanup errors, and keep the existing call to w.endDeploymentStep
(referenced as currentStep, failureReason, w.endDeploymentStep) so the defer
writes the correct message based on retErr.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-progress.tsx:
- Around line 44-53: The now state is initialized to 0 which makes duration
negative if a failed step has startedAt but no endedAt; change the
initialization of now (useState for now/setNow) to a real timestamp (e.g.,
Date.now()) so the component has a meaningful baseline even when the useEffect
short-circuits on isFailed, keeping the existing setInterval logic in useEffect
unchanged.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx:
- Around line 89-91: The duration display currently uses a truthy check which
hides valid zero durations; update the conditional around the duration rendering
in deployment-step.tsx so it checks for null/undefined instead of falsiness
(e.g., use duration != null or the nullish coalescing pattern) and call
ms(duration) when duration is 0; locate the JSX span that reads "{duration ?
ms(duration) : null}" and change the condition to allow 0 while still guarding
against null/undefined so legitimate short durations render.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 651c12f8-a480-4255-96a0-c14bc0005607
📒 Files selected for processing (4)
svc/ctrl/worker/deploy/deploy_handler.goweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-progress.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/deployment-status-badge.tsx
...ojectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-progress.tsx
Show resolved
Hide resolved
.../[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx (1)
91-91:⚠️ Potential issue | 🟡 MinorRender zero-duration values correctly at Line 91.
{duration ? ...}hides valid0durations. Use a null/undefined check instead.💡 Proposed fix
- <span className="text-gray-10 text-xs">{duration ? ms(duration) : null}</span> + <span className="text-gray-10 text-xs"> + {duration != null ? ms(Math.max(0, duration)) : null} + </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx at line 91, The JSX currently hides valid zero durations by using a truthy check on the duration variable in the span (the expression "{duration ? ms(duration) : null}"); change that check to a null/undefined check so 0 renders (e.g., use duration != null or duration !== null && duration !== undefined) and keep the ms(duration) call and fallback as-is to ensure only null/undefined suppress rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx:
- Around line 70-76: The "Complete" Badge (component Badge in
deployment-step.tsx) is only visually hidden via opacity/scale but remains in
the accessibility tree; update the render so when status !== "completed" the
Badge has aria-hidden={true} (or add the "invisible" utility class) in addition
to the existing opacity/scale classes; modify the Badge props/className
computation (the cn call around Badge) to include aria-hidden={status !==
"completed"} (or conditionally append "invisible") so assistive tech will ignore
the hidden badge.
---
Duplicate comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx:
- Line 91: The JSX currently hides valid zero durations by using a truthy check
on the duration variable in the span (the expression "{duration ? ms(duration) :
null}"); change that check to a null/undefined check so 0 renders (e.g., use
duration != null or duration !== null && duration !== undefined) and keep the
ms(duration) call and fallback as-is to ensure only null/undefined suppress
rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 871d5e99-e8bf-4a97-9a62-3e8010004f75
📒 Files selected for processing (2)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-progress.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx
.../[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-step.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx (1)
94-101:⚠️ Potential issue | 🟠 MajorPass deploymentId to logs URL for consistency with requests filtering.
The "Go to requests..." action filters by deployment via URL parameter (
deploymentId=contains:${selectedDeployment.id}), while "Go to logs..." navigates without pre-filtering. The logs page supports deployment filtering through both its UI filter component and query hook (which acceptsdeploymentId), so the navigation should include this parameter to maintain consistency:/${workspace.slug}/projects/${selectedDeployment.projectId}/logs?deploymentId=${selectedDeployment.id}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx around lines 94 - 101, The "runtime-logs" action currently navigates via router.push without the deployment filter; update the onClick handler for the menu item with id "runtime-logs" to append deploymentId=${selectedDeployment.id} to the logs URL (use router.push to `/${workspace.slug}/projects/${selectedDeployment.projectId}/logs?deploymentId=${selectedDeployment.id}`) so the logs page receives the same deployment filter as the "Go to requests..." action; adjust any URL-building logic around the onClick in this component to include selectedDeployment.id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx:
- Around line 94-101: The "runtime-logs" action currently navigates via
router.push without the deployment filter; update the onClick handler for the
menu item with id "runtime-logs" to append deploymentId=${selectedDeployment.id}
to the logs URL (use router.push to
`/${workspace.slug}/projects/${selectedDeployment.projectId}/logs?deploymentId=${selectedDeployment.id}`)
so the logs page receives the same deployment filter as the "Go to requests..."
action; adjust any URL-building logic around the onClick in this component to
include selectedDeployment.id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 599c29c9-9570-433d-aa54-442bea37e694
📒 Files selected for processing (5)
svc/ctrl/worker/deploy/deploy_handler.goweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/build-steps-table/build-step-logs-expanded.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/build-steps-table/columns.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/deployment-progress.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/components/table/components/actions/deployment-list-table-action.popover.constants.tsx
✅ Files skipped from review due to trivial changes (1)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/(deployment-progress)/build-steps-table/build-step-logs-expanded.tsx
What does this PR do?
This PR addresses:
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated