Skip to content

feat: deployment plan ui#1008

Merged
adityachoudhari26 merged 2 commits intomainfrom
deployment-plan-ui
Apr 17, 2026
Merged

feat: deployment plan ui#1008
adityachoudhari26 merged 2 commits intomainfrom
deployment-plan-ui

Conversation

@adityachoudhari26
Copy link
Copy Markdown
Member

@adityachoudhari26 adityachoudhari26 commented Apr 17, 2026

fixes #1007

Summary by CodeRabbit

Release Notes

  • New Features
    • Deployment Plans UI added—view and manage plans within each deployment
    • Plan details page displaying results with environment, resource, and agent information
    • Plan status indicators showing computing, completed, or failed states
    • Diff viewer to compare proposed versus current plan changes in split or unified mode

Copilot AI review requested due to automatic review settings April 17, 2026 19:38
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 17, 2026

Warning

Rate limit exceeded

@adityachoudhari26 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 19 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 19 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c9e55e11-4a36-4c4d-9cb2-acc231c76d12

📥 Commits

Reviewing files that changed from the base of the PR and between 574fa28 and cb2b189.

📒 Files selected for processing (4)
  • apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx
  • apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx
  • packages/trpc/src/routes/deployment-plans.ts
  • packages/trpc/src/routes/deployments.ts
📝 Walkthrough

Walkthrough

This PR introduces a deployment plans UI feature by adding two new nested routes under :deploymentId/plans for listing and viewing individual plan details, replacing the "traces" tab with a "plans" tab in the navbar, creating UI components for displaying plan statuses and diffs, implementing two page routes with table displays, and adding tRPC endpoints to fetch plan data, aggregated summaries, and diff information.

Changes

Cohort / File(s) Summary
Route Configuration
apps/web/app/routes.ts, apps/web/app/routes/deployments.ts
Added two new nested routes under :deploymentId/plans for plan list and plan detail pages; wired deploymentPlansRouter into the deploymentsRouter.
Navigation UI
apps/web/app/routes/ws/deployments/_components/DeploymentsNavbarTabs.tsx
Updated DeploymentTab union type to replace "traces" with "plans", updated route-to-tab mapping, and swapped the corresponding navbar tab trigger and link.
Plan UI Components
apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx, apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx
Added PlanStatusBadge component for rendering status labels with color coding; added PlanDiffDialog component with Monaco diff editor for side-by-side or unified YAML diff viewing.
Plan Pages
apps/web/app/routes/ws/deployments/page.$deploymentId.plans.tsx, apps/web/app/routes/ws/deployments/page.$deploymentId.plans.$planId.tsx
Added deployment plans list page with table showing versions, statuses, target counts, GitHub links, and timestamps; added plan details page with nested results table including environment/resource/agent info and conditional diff viewing for each result.
tRPC Plan Routes
packages/trpc/src/routes/deployment-plans.ts
Implemented deploymentPlansRouter with three protected queries: list (fetches plans with computed status and aggregated summaries), results (fetches plan target results joined with environment/resource/agent info), and resultDiff (fetches current/proposed YAML diffs for a specific result).

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Browser as Client
    participant Router as tRPC Router
    participant DB as Database

    User->>Browser: Navigate to deployment plans
    activate Browser
    Browser->>Router: trpc.deployment.plans.list()
    activate Router
    Router->>DB: Query deployment plans
    activate DB
    DB-->>Router: Plan records + target results
    deactivate DB
    Router->>Router: Compute status & aggregate summaries
    Router-->>Browser: Plans list with metadata
    deactivate Router
    Browser->>User: Display plans table
    deactivate Browser

    User->>Browser: Click on specific plan
    activate Browser
    Browser->>Router: trpc.deployment.plans.results()
    activate Router
    Router->>DB: Join plan target results with details
    activate DB
    DB-->>Router: Joined result rows
    deactivate DB
    Router-->>Browser: Results with environment/resource/agent/status
    deactivate Router
    Browser->>User: Display plan details & results
    deactivate Browser

    User->>Browser: Click "View diff" on a result
    activate Browser
    Browser->>Router: trpc.deployment.plans.resultDiff()
    activate Router
    Router->>DB: Fetch current/proposed diff content
    activate DB
    DB-->>Router: Diff strings & metadata
    deactivate DB
    Router-->>Browser: current & proposed YAML content
    deactivate Router
    Browser->>User: Display Monaco diff editor
    deactivate Browser
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #699: Modifies the same DeploymentsNavbarTabs component to update the tab union and route-to-tab mapping, so changes may overlap or need coordination.
  • #785: Updates the same deployment navbar tab logic (DeploymentTab union, useDeploymentTab route mapping, and tab triggers), requiring careful review for conflicts.

Suggested reviewers

  • zacharyblasczyk

Poem

🐰 A nimble diff viewer hops into view,
Plans laid bare with statuses true,
From list to details, each target aligned,
With badges and diffs, a feature refined! 📋✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: deployment plan ui' directly matches the main objective of adding UI for viewing deployment plan diffs as specified in issue #1007.
Linked Issues check ✅ Passed All code changes implement the UI for viewing deployment plan diffs as required by issue #1007, including routes, components, and backend TRPC endpoints.
Out of Scope Changes check ✅ Passed All changes are directly related to adding the deployment plan UI and diff viewing functionality required by issue #1007; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch deployment-plan-ui

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a deployment plans UI flow so users can browse plan runs and inspect per-target diffs from the web app, backed by new tRPC endpoints under the deployment router.

Changes:

  • Added deployment.plans.* tRPC endpoints to list plans, list plan target results, and fetch a specific result diff.
  • Added web routes/pages for deployment plans list and plan detail, including a Monaco diff dialog.
  • Updated the deployments navbar tabs and web route registry to include the new Plans pages.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/trpc/src/routes/deployments.ts Mounts the new plans sub-router under deployment.
packages/trpc/src/routes/deployment-plans.ts Implements plan list/results/diff queries with auth checks.
apps/web/app/routes/ws/deployments/page.$deploymentId.plans.tsx New plans list page showing plan status/summary and navigation to a plan.
apps/web/app/routes/ws/deployments/page.$deploymentId.plans.$planId.tsx New plan detail page listing target results and opening diff dialog when changes exist.
apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx Introduces a status badge component for plan/plan-result statuses.
apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx Adds a dialog with Monaco DiffEditor for current vs proposed.
apps/web/app/routes/ws/deployments/_components/DeploymentsNavbarTabs.tsx Adds a “Plans” tab (and removes “Traces” from the tab list).
apps/web/app/routes.ts Registers the new :deploymentId/plans and :deploymentId/plans/:planId routes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +30 to +37
export const deploymentPlansRouter = router({
list: protectedProcedure
.meta({
authorizationCheck: ({ canUser, input }) =>
canUser
.perform(Permission.DeploymentGet)
.on({ type: "deployment", id: input.deploymentId }),
})
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says this fixes #1007 (stop posting every plan diff as PR comments and instead post a single link to the UI). This PR adds the UI + APIs, but the workspace-engine GitHub commenter still formats and upserts unified diffs into PR comments (apps/workspace-engine/svc/controllers/deploymentplanresult/github_comment.go). Either update the PR description/scope, or follow up by changing the commenter to post a single link to this new plans UI instead of full diffs.

Copilot uses AI. Check for mistakes.
Comment on lines 7 to 15
type DeploymentTab =
| "environments"
| "resources"
| "settings"
| "release-targets"
| "traces"
| "variables"
| "policies";
| "policies"
| "plans";

Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deployments navbar no longer includes a “Traces” tab, but the deployment traces route still exists (apps/web/app/routes.ts has a :deploymentId/traces route). This makes traces effectively undiscoverable from the deployment UI. Either restore the tab, or remove/redirect the traces route so navigation and routing stay consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +5
export const PlanStatusDisplayName: Record<string, string> = {
computing: "Computing",
completed: "Completed",
failed: "Failed",
};
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PlanStatusBadge only defines display names/colors for "computing", "completed", and "failed", but plan target results use statuses like "errored" and "unsupported" (and the plan detail page renders badges for those result statuses). As a result, badges for errored/unsupported results will fall back to the neutral styling and raw status text. Add mappings for the full set of statuses used by plan results (and consider standardizing on "errored" vs "failed" across the API/UI).

Copilot uses AI. Check for mistakes.
Comment on lines +114 to +120
const status =
summary.computing > 0
? "computing"
: summary.errored > 0
? "failed"
: "completed";
return { ...p, status, summary };
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan-level status returned from list uses "failed" when any target is "errored". That introduces an extra status value that doesn’t exist in the underlying deployment_plan_target_status enum (computing/completed/errored/unsupported) and forces the client to handle two different “error” spellings ("errored" for results vs "failed" for plans). Consider returning "errored" here (and/or explicitly modeling the plan status type) so API status values stay consistent and easier to consume.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +66
<DiffEditor
height="100%"
language="yaml"
theme="vs-dark"
original={diffQuery.data.current}
modified={diffQuery.data.proposed}
options={{
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PlanDiffDialog hardcodes the Monaco theme to "vs-dark". The app already has theme-aware Monaco usage elsewhere (e.g. VersionActionsPanel uses ThemeProvider to switch between "vs" and "vs-dark"), so this dialog will look wrong in light mode. Use the existing ThemeProvider to select the Monaco theme based on the current app theme.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (6)
apps/web/app/routes/ws/deployments/page.$deploymentId.plans.$planId.tsx (2)

167-175: Breadcrumb shows raw planId UUID.

Showing the full UUID is functional but noisy. Since results doesn't return any plan-level metadata (e.g., version tag), consider adding a small trpc.deployment.plans.get (or folding plan summary into results) so the breadcrumb can display the plan's version tag like the list page does.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/routes/ws/deployments/page`.$deploymentId.plans.$planId.tsx
around lines 167 - 175, Breadcrumb currently renders the raw planId UUID in
BreadcrumbPage; fetch the plan summary (e.g., using trpc.deployment.plans.get or
by augmenting the existing results call) to obtain a human-friendly identifier
such as the plan's version tag, and replace the planId display with that tag;
specifically, call trpc.deployment.plans.get(planId) (or include versionTag in
the results response), store the returned plan.versionTag, and render
<BreadcrumbPage className="font-mono">{plan.versionTag ??
planId}</BreadcrumbPage> so it falls back to the UUID when metadata is
unavailable.

134-142: Polling stays active while dialog is open — benign but noisy.

Every 5s the results query refetches and can force the PlanDiffDialog row re-renders (the result object identity changes). Not a correctness issue, but if the diff is open and the underlying result status flips, the dialog won't refresh its resultDiff query automatically. If that's desired, consider either (a) stopping plan polling once status === "completed" for all rows, or (b) also refetching resultDiff on a short interval while open.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/routes/ws/deployments/page`.$deploymentId.plans.$planId.tsx
around lines 134 - 142, The results query in DeploymentPlanDetail is polling
every 5s regardless of whether a diff dialog is open or all rows are completed;
update the trpc.deployment.plans.results.useQuery call (resultsQuery) to disable
or reduce polling when not needed by adding a conditional
enabled/refetchInterval: make enabled false if planId is falsy OR if a
PlanDiffDialog is open (use dialog open state) OR if all returned results have
status === "completed" (stop polling once all completed), and alternatively, if
you want live diffs when the dialog is open, add a short refetchInterval for the
resultDiff query while PlanDiffDialog is open so the dialog explicitly
refetches; adjust the logic around resultsQuery and resultDiff to derive
enabled/refetchInterval from the dialog open state and aggregated statuses.
packages/trpc/src/routes/deployment-plans.ts (1)

102-110: s[row.status] += row.count — confirm type safety.

This relies on the DB returning exactly the four enum values present as keys on PlanSummary. If anything ever drifts (migration, raw insert, new enum member), this silently produces NaN on the summary. A switch or a guarded lookup is marginally safer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/routes/deployment-plans.ts` around lines 102 - 110, The
code uses dynamic indexing s[row.status] in the loop (variables: summaryByPlan,
PlanSummary, emptySummary, counts, row.status), which can produce NaN if
row.status contains an unexpected value; replace the dynamic increment with a
guarded lookup or switch over the allowed statuses (explicitly handle each enum
member and a default/fallback), validate row.status before modifying s, and
ensure the increment only touches known PlanSummary keys so unexpected DB values
are ignored or logged rather than producing NaN.
apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx (1)

16-27: Consider a stronger status type and React.FC per coding guidelines.

status: string forgoes the type safety the backend already provides. Deriving from the tRPC RouterOutputs (or a shared union of the DB enum) would catch future mismatches like the missing statuses above at compile time.

As per coding guidelines: "For React components, use React.FC with explicit typing" and "Prefer interfaces for public APIs".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx`
around lines 16 - 27, The PlanStatusBadge component currently types its prop as
plain string which loses compile-time safety; change it to use a specific union
type derived from your backend types (e.g., the plan status union from tRPC
RouterOutputs or a shared DB enum) and declare the component as a React.FC with
an explicit props interface (e.g., interface PlanStatusBadgeProps { status:
<BackendStatusUnion>; }). Update the function/component signature for
PlanStatusBadge to use React.FC<PlanStatusBadgeProps> and ensure usages match
the new type so TypeScript will catch missing keys in PlanStatusBadgeColor and
PlanStatusDisplayName.
apps/web/app/routes/ws/deployments/page.$deploymentId.plans.tsx (1)

119-158: Row click hijacks text selection / middle-click.

Using onClick on <TableRow> + useNavigate breaks middle-click-to-open, Cmd/Ctrl+click, and text selection in cells. Consider wrapping the row content in a <Link> (or using a real <a> in the first cell) so users can open a plan in a new tab and drag-select values like the version tag.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/routes/ws/deployments/page`.$deploymentId.plans.tsx around lines
119 - 158, The row-level onClick in PlansTableRow is hijacking
middle-click/Cmd/Ctrl+click and text selection; remove the TableRow onClick and
instead make the navigable element a real link so browsers can open in new tabs
and allow selection. Update PlansTableRow to stop attaching onClick to TableRow
and wrap the row content (or at minimum the primary cell like the version tag)
in a proper anchor/Link (e.g., <Link to=... className="contents" or an <a> in
the first TableCell) so the cursor pointer and styles remain but native link
behaviors (middle-click, modifier-click, drag-select) are preserved; keep the
onSelect prop usage in the parent as needed but replace programmatic use of
useNavigate with an href/Link target.
apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx (1)

50-74: isLoading is correct; focus on error handling instead.

The current code using isLoading is the recommended approach for disabled/lazy queries in TanStack React Query v5. The official docs explicitly state it avoids false positives when the query is disabled, making it the right choice here. Do not use isPending, as it would incorrectly show a loading state even when the dialog is closed.

However, error states are not handled explicitly. If the query fails (e.g., server error), it falls into the data == null branch and shows "No diff available" instead of an error message. Consider adding an isError check:

Suggested improvement
         {diffQuery.isLoading ? (
             <div className="flex h-full items-center justify-center text-sm text-muted-foreground">
               Loading diff...
             </div>
+          ) : diffQuery.isError ? (
+            <div className="flex h-full items-center justify-center text-sm text-red-600">
+              Failed to load diff
+            </div>
           ) : diffQuery.data == null ? (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx`
around lines 50 - 74, The dialog currently treats failed queries as missing
data; update PlanDiffDialog to check diffQuery.isError before the data==null
branch and render an error state (e.g., a centered message that includes
diffQuery.error?.message or a generic "Failed to load diff" text) so users see
failures instead of "No diff available"; keep diffQuery.isLoading and the
DiffEditor usage (original/modified props and view handling) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx`:
- Around line 1-14: Add missing 'errored' and 'unsupported' entries to both
PlanStatusDisplayName and PlanStatusBadgeColor: map 'errored' to the same
display string and red styling as 'failed' (so errored renders as an error/red
badge), and add 'unsupported' with a display name like "Unsupported" and a
neutral gray styling (similar format to the other entries, e.g., bg-gray...
text-gray... border-gray...). Update the PlanStatusDisplayName and
PlanStatusBadgeColor objects (symbols: PlanStatusDisplayName,
PlanStatusBadgeColor) so the badge component can render these statuses correctly
instead of falling back to defaults; alternatively normalize statuses
server-side, but implement the entries here to fix the immediate UI mismatch.

In `@packages/trpc/src/routes/deployment-plans.ts`:
- Around line 112-121: The mapping in the plans list uses a non‑canonical status
name ("failed") causing mismatches with the DB enum; update the mapping in the
return block (the plans.map that uses summaryByPlan and emptySummary) so it
emits "errored" instead of "failed", treat a plan with summary.total === 0 as
"computing" (to avoid new plans being marked completed), and optionally surface
"unsupported" at the plan level when summary.unsupported === summary.total;
adjust the status variable calculation in that function accordingly.
- Around line 187-206: The mapping of agent fields from dispatchContext.jobAgent
in the rows.map block repeats logic that also exists where resultDiff returns
agent info; extract a small helper (e.g., mapJobAgent or normalizeAgent) and use
it from both places to produce { id, name, type } with the same defaulting
behavior (handling dispatchContext.jobAgent possibly undefined and using ""
defaults). Locate usages around the rows.map in deployment-plans.ts (the
dispatchContext.jobAgent access) and the resultDiff mapping code, replace the
duplicated inline mapping with calls to the new helper so both endpoints stay in
sync.

---

Nitpick comments:
In `@apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx`:
- Around line 50-74: The dialog currently treats failed queries as missing data;
update PlanDiffDialog to check diffQuery.isError before the data==null branch
and render an error state (e.g., a centered message that includes
diffQuery.error?.message or a generic "Failed to load diff" text) so users see
failures instead of "No diff available"; keep diffQuery.isLoading and the
DiffEditor usage (original/modified props and view handling) unchanged.

In `@apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx`:
- Around line 16-27: The PlanStatusBadge component currently types its prop as
plain string which loses compile-time safety; change it to use a specific union
type derived from your backend types (e.g., the plan status union from tRPC
RouterOutputs or a shared DB enum) and declare the component as a React.FC with
an explicit props interface (e.g., interface PlanStatusBadgeProps { status:
<BackendStatusUnion>; }). Update the function/component signature for
PlanStatusBadge to use React.FC<PlanStatusBadgeProps> and ensure usages match
the new type so TypeScript will catch missing keys in PlanStatusBadgeColor and
PlanStatusDisplayName.

In `@apps/web/app/routes/ws/deployments/page`.$deploymentId.plans.$planId.tsx:
- Around line 167-175: Breadcrumb currently renders the raw planId UUID in
BreadcrumbPage; fetch the plan summary (e.g., using trpc.deployment.plans.get or
by augmenting the existing results call) to obtain a human-friendly identifier
such as the plan's version tag, and replace the planId display with that tag;
specifically, call trpc.deployment.plans.get(planId) (or include versionTag in
the results response), store the returned plan.versionTag, and render
<BreadcrumbPage className="font-mono">{plan.versionTag ??
planId}</BreadcrumbPage> so it falls back to the UUID when metadata is
unavailable.
- Around line 134-142: The results query in DeploymentPlanDetail is polling
every 5s regardless of whether a diff dialog is open or all rows are completed;
update the trpc.deployment.plans.results.useQuery call (resultsQuery) to disable
or reduce polling when not needed by adding a conditional
enabled/refetchInterval: make enabled false if planId is falsy OR if a
PlanDiffDialog is open (use dialog open state) OR if all returned results have
status === "completed" (stop polling once all completed), and alternatively, if
you want live diffs when the dialog is open, add a short refetchInterval for the
resultDiff query while PlanDiffDialog is open so the dialog explicitly
refetches; adjust the logic around resultsQuery and resultDiff to derive
enabled/refetchInterval from the dialog open state and aggregated statuses.

In `@apps/web/app/routes/ws/deployments/page`.$deploymentId.plans.tsx:
- Around line 119-158: The row-level onClick in PlansTableRow is hijacking
middle-click/Cmd/Ctrl+click and text selection; remove the TableRow onClick and
instead make the navigable element a real link so browsers can open in new tabs
and allow selection. Update PlansTableRow to stop attaching onClick to TableRow
and wrap the row content (or at minimum the primary cell like the version tag)
in a proper anchor/Link (e.g., <Link to=... className="contents" or an <a> in
the first TableCell) so the cursor pointer and styles remain but native link
behaviors (middle-click, modifier-click, drag-select) are preserved; keep the
onSelect prop usage in the parent as needed but replace programmatic use of
useNavigate with an href/Link target.

In `@packages/trpc/src/routes/deployment-plans.ts`:
- Around line 102-110: The code uses dynamic indexing s[row.status] in the loop
(variables: summaryByPlan, PlanSummary, emptySummary, counts, row.status), which
can produce NaN if row.status contains an unexpected value; replace the dynamic
increment with a guarded lookup or switch over the allowed statuses (explicitly
handle each enum member and a default/fallback), validate row.status before
modifying s, and ensure the increment only touches known PlanSummary keys so
unexpected DB values are ignored or logged rather than producing NaN.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6f421494-59ac-47da-a530-c99d1bf944bf

📥 Commits

Reviewing files that changed from the base of the PR and between afc753a and 574fa28.

📒 Files selected for processing (8)
  • apps/web/app/routes.ts
  • apps/web/app/routes/ws/deployments/_components/DeploymentsNavbarTabs.tsx
  • apps/web/app/routes/ws/deployments/_components/plans/PlanDiffDialog.tsx
  • apps/web/app/routes/ws/deployments/_components/plans/PlanStatusBadge.tsx
  • apps/web/app/routes/ws/deployments/page.$deploymentId.plans.$planId.tsx
  • apps/web/app/routes/ws/deployments/page.$deploymentId.plans.tsx
  • packages/trpc/src/routes/deployment-plans.ts
  • packages/trpc/src/routes/deployments.ts

Comment thread packages/trpc/src/routes/deployment-plans.ts
Comment on lines +187 to +206
return rows.map((r) => {
const agent = r.dispatchContext.jobAgent ?? {};
return {
resultId: r.resultId,
targetId: r.targetId,
environment: { id: r.environmentId, name: r.environmentName },
resource: { id: r.resourceId, name: r.resourceName },
agent: {
id: (agent.id as string | undefined) ?? "",
name: (agent.name as string | undefined) ?? "",
type: (agent.type as string | undefined) ?? "",
},
status: r.status,
hasChanges: r.hasChanges,
message: r.message,
contentHash: r.contentHash,
startedAt: r.startedAt,
completedAt: r.completedAt,
};
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find where dispatchContext is written to understand the actual shape of jobAgent.
rg -nP -C3 'dispatchContext' --type=ts
rg -nP -C3 'jobAgent\s*[:=]' --type=ts apps/workspace-engine packages/db || true

Repository: ctrlplanedev/ctrlplane

Length of output: 12766


🏁 Script executed:

# Find where deploymentPlanTargetResult.dispatchContext is populated
rg -nP 'deploymentPlanTargetResult' --type=ts -A 5 -B 5 | head -100

# Search for forcedeploy controller context creation
fd -t f -e ts -e tsx -path '*forcedeploy*'

Repository: ctrlplanedev/ctrlplane

Length of output: 8594


🏁 Script executed:

# Look for where dispatchContext is set for deployment plan targets
rg -nP 'dispatchContext.*=' --type=ts packages/trpc packages/db apps/workspace-engine -B 3 -A 3 | head -150

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check the forcedeploy controller to understand context structure
fd -t f -e go -path '*forcedeploy*' apps/workspace-engine

Repository: ctrlplanedev/ctrlplane

Length of output: 234


🏁 Script executed:

# Find insertions/updates of deploymentPlanTargetResult with dispatchContext
rg -n 'deploymentPlanTargetResult' packages/trpc packages/db --type=ts -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 8153


🏁 Script executed:

# Find the Go files in forcedeploy controller
find apps/workspace-engine -name '*forcedeploy*' -type f

Repository: ctrlplanedev/ctrlplane

Length of output: 123


🏁 Script executed:

# Look for where dispatchContext is assigned to deployment plan target result
rg -n 'dispatchContext' apps/workspace-engine --type=go -B 2 -A 2

Repository: ctrlplanedev/ctrlplane

Length of output: 9907


🏁 Script executed:

# Look at the forcedeploy controller implementation
cat apps/workspace-engine/pkg/reconcile/events/forcedeploy.go | head -150

Repository: ctrlplanedev/ctrlplane

Length of output: 120


🏁 Script executed:

# Search for where deployment plan results are created in forcedeploy
rg -n 'InsertResult' apps/workspace-engine/svc/controllers/forcedeploy --type=go -B 5 -A 5

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Look for buildDispatchContext in forcedeploy context
rg -n 'BuildDispatchContext\|buildDispatchContext\|DispatchContext' apps/workspace-engine/svc/controllers/forcedeploy --type=go -B 2 -A 2

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Find the forcedeploy controller files
find apps/workspace-engine/svc/controllers/forcedeploy -type f -name "*.go"

Repository: ctrlplanedev/ctrlplane

Length of output: 589


🏁 Script executed:

# Look at the main controller file
cat apps/workspace-engine/svc/controllers/forcedeploy/releasetarget.go | head -100

Repository: ctrlplanedev/ctrlplane

Length of output: 1216


🏁 Script executed:

# Search for how dispatchContext is built in forcedeploy
rg -n 'dispatchContext\|DispatchContext' apps/workspace-engine/svc/controllers/forcedeploy -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Search for InsertResult in forcedeploy reconcile
rg -n 'InsertResult\|deployment' apps/workspace-engine/svc/controllers/forcedeploy/reconcile.go -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Look at the reconcile.go file to understand the flow
cat apps/workspace-engine/svc/controllers/forcedeploy/reconcile.go | head -200

Repository: ctrlplanedev/ctrlplane

Length of output: 3914


🏁 Script executed:

# Look at deploymentplan controller
cat apps/workspace-engine/svc/controllers/deploymentplan/controller.go | head -200

Repository: ctrlplanedev/ctrlplane

Length of output: 5635


🏁 Script executed:

# Search for where the dispatchContext is built in deploymentplan
rg -n 'dispatchContext\|InsertResult' apps/workspace-engine/svc/controllers/deploymentplan -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check the reconcile file for deploymentplan
cat apps/workspace-engine/svc/controllers/deploymentplan/reconcile.go | head -150

Repository: ctrlplanedev/ctrlplane

Length of output: 163


🏁 Script executed:

# Continue reading the processTarget function from controller.go
cat apps/workspace-engine/svc/controllers/deploymentplan/controller.go | tail -n +180 | head -150

Repository: ctrlplanedev/ctrlplane

Length of output: 2666


🏁 Script executed:

# List the actual files in deploymentplan controller
ls -la apps/workspace-engine/svc/controllers/deploymentplan/

Repository: ctrlplanedev/ctrlplane

Length of output: 582


🏁 Script executed:

# Check if forcedeploy creates deployment plan results
rg -n 'deploymentplan\|InsertTarget\|InsertResult' apps/workspace-engine/svc/controllers/forcedeploy -i

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check if there are other sources of deployment plan results
rg -n 'deploymentPlanTargetResult\|InsertResult' apps/workspace-engine --type=go | grep -v node_modules

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Look at the DispatchContext type definition in Go
rg -n 'type DispatchContext' apps/workspace-engine --type=go -A 10

Repository: ctrlplanedev/ctrlplane

Length of output: 1179


🏁 Script executed:

# Check if resultDiff endpoint exists
rg -n 'resultDiff' packages/trpc --type=ts

Repository: ctrlplanedev/ctrlplane

Length of output: 147


🏁 Script executed:

# Verify the TypeScript schema type for deploymentPlanTargetResult
cat packages/db/src/schema/deployment-plan.ts | head -160

Repository: ctrlplanedev/ctrlplane

Length of output: 4749


🏁 Script executed:

# Check all places where deployment plan results are created
rg -n 'deploymentPlanTargetResult\|deployment_plan_target_result' packages --type=ts -B 2 -A 2

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check if forcedeploy creates deployment plan results or just jobs
rg -n 'CreateJobAndEnqueueDispatch\|setter' apps/workspace-engine/svc/controllers/forcedeploy -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Verify the learning context - check if forcedeploy results lack jobAgent
find apps/workspace-engine/svc/controllers/forcedeploy -name "*.go" -exec grep -l "dispatchContext\|DispatchContext" {} \;

Repository: ctrlplanedev/ctrlplane

Length of output: 135


🏁 Script executed:

# Look at forcedeploy setters
cat apps/workspace-engine/svc/controllers/forcedeploy/setters_postgres.go | head -100

Repository: ctrlplanedev/ctrlplane

Length of output: 2778


🏁 Script executed:

# Check the Setter interface for forcedeploy
cat apps/workspace-engine/svc/controllers/forcedeploy/setters.go

Repository: ctrlplanedev/ctrlplane

Length of output: 291


🏁 Script executed:

# Look for where deployment plan results are updated from forcedeploy
rg -n 'deploymentPlanTargetResult\|deployment_plan_target_result' apps/workspace-engine/svc/controllers/forcedeploy -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 48


The dispatchContext.jobAgent access is safe as written.

dispatchContext comes from the deploymentplan controller, which always includes jobAgent in the dispatch context struct. The defensive code pattern (?? {} and ?? "" defaults) correctly handles the TypeScript side where the shape is Record<string, any>. No change needed here.

That said, the agent mapping logic (lines 188–199) is duplicated if resultDiff returns agent info. Extract this into a small reusable helper to keep both endpoints in sync.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/routes/deployment-plans.ts` around lines 187 - 206, The
mapping of agent fields from dispatchContext.jobAgent in the rows.map block
repeats logic that also exists where resultDiff returns agent info; extract a
small helper (e.g., mapJobAgent or normalizeAgent) and use it from both places
to produce { id, name, type } with the same defaulting behavior (handling
dispatchContext.jobAgent possibly undefined and using "" defaults). Locate
usages around the rows.map in deployment-plans.ts (the dispatchContext.jobAgent
access) and the resultDiff mapping code, replace the duplicated inline mapping
with calls to the new helper so both endpoints stay in sync.

@adityachoudhari26 adityachoudhari26 merged commit 6d8789c into main Apr 17, 2026
10 checks passed
@adityachoudhari26 adityachoudhari26 deleted the deployment-plan-ui branch April 17, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: deployment plans UI

2 participants