Repository navigation
feat(desktop): allow owners to delete hosts - #5566
Conversation
|
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:
📝 WalkthroughWalkthroughHost deletion is added across the v2 host API, automation creation checks, desktop optimistic collections, and host settings. Owners can confirm deletion of remote hosts, related records are cleaned up transactionally, and settings search exposes the deletion action. ChangesHost deletion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Owner
participant HostSettings
participant OptimisticActions
participant ElectricCollection
participant V2HostAPI
participant Database
Owner->>HostSettings: Confirm remote host deletion
HostSettings->>OptimisticActions: deleteHost(hostId)
OptimisticActions->>ElectricCollection: Delete v2 host optimistically
ElectricCollection->>V2HostAPI: Delete using machineId
V2HostAPI->>Database: Validate ownership and remove related records
Database-->>V2HostAPI: Return transaction ID
V2HostAPI-->>ElectricCollection: Return deletion result
ElectricCollection-->>HostSettings: Resolve persistence
HostSettings-->>Owner: Show success toast
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit e6cb628 on Jul 10, 2026 3:57am UTC. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Greptile SummaryThis PR adds an owner-only "Delete host" action to the desktop Host Settings page, backed by a new transactional
Confidence Score: 4/5Safe to merge — the mutation is well-guarded with transactional row locks and multi-layer ownership checks, and the optimistic UI degrades cleanly on failure. The security model is solid: org membership is pre-checked outside the transaction and host ownership is re-verified inside with FOR UPDATE locks, preventing TOCTOU races. The main gap is test coverage for the zero-workspace deletion code path, where a different WHERE clause is generated but never exercised by the test suite. packages/trpc/src/router/v2-host/v2-host.test.ts — the zero-workspace happy-path scenario is missing and should be added to cover the eq(automations.targetHostId) only branch.
|
| Filename | Overview |
|---|---|
| packages/trpc/src/router/v2-host/v2-host.ts | Adds transactional delete mutation with org-membership pre-check, row-level locks for host and ownership, workspace removal, and automation pausing; security model is sound. |
| packages/trpc/src/router/v2-host/v2-host.test.ts | Good coverage of auth/authz edge cases, but the zero-workspace deletion code path is not tested; a bug in that branch would pass undetected. |
| apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/DeleteHostSection/DeleteHostSection.tsx | New danger-zone component with confirmation dialog and local-host guard; setIsDeleting(false) in finally runs after navigation unmounts the component but is harmless in React 18. |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | Adds onDelete handler for the v2Hosts Electric collection; consistent with existing pattern but accesses transaction.mutations[0] without a guard against an empty array. |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useOptimisticCollectionActions/useOptimisticCollectionActions.ts | Adds deleteHost action following the same runHostsMutation pattern as renameHost; straightforward and correct. |
| apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx | Conditionally renders DeleteHostSection only for owners with correct isLocalHost check; clean integration. |
| apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts | Adds HOST_DELETE setting entry with appropriate keywords and shared variant. |
| apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.test.ts | Adds a focused test confirming delete host search returns the new setting ID. |
| apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/DeleteHostSection/index.ts | Barrel export only; no issues. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant U as User
participant UI as DeleteHostSection
participant OCA as useOptimisticCollectionActions
participant EC as Electric Collection (v2Hosts)
participant API as tRPC v2Host.delete
participant DB as Postgres (dbWs)
U->>UI: Click Delete host and confirm
UI->>OCA: deleteHost(hostId)
OCA->>EC: collection.delete(hostId) optimistic
EC-->>OCA: transaction isPersisted.promise
OCA-->>UI: transaction
UI->>UI: setIsOpen(false) navigate to /settings/hosts
Note over UI: Component unmounts
EC->>API: onDelete calls v2Host.delete.mutate
API->>DB: requireActiveOrgMembership pre-check
DB-->>API: organizationId
API->>DB: BEGIN TRANSACTION
DB->>DB: SELECT v2Hosts FOR UPDATE
DB->>DB: SELECT v2UsersHosts FOR UPDATE role check
DB->>DB: SELECT v2Workspaces FOR UPDATE
DB->>DB: UPDATE automations SET enabled false
DB->>DB: DELETE v2Workspaces
DB->>DB: DELETE v2Hosts cascade to v2UsersHosts
DB->>DB: SELECT pg_current_xact_id
DB-->>API: txid
API-->>EC: success true txid
EC-->>UI: isPersisted resolves toast.success
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant U as User
participant UI as DeleteHostSection
participant OCA as useOptimisticCollectionActions
participant EC as Electric Collection (v2Hosts)
participant API as tRPC v2Host.delete
participant DB as Postgres (dbWs)
U->>UI: Click Delete host and confirm
UI->>OCA: deleteHost(hostId)
OCA->>EC: collection.delete(hostId) optimistic
EC-->>OCA: transaction isPersisted.promise
OCA-->>UI: transaction
UI->>UI: setIsOpen(false) navigate to /settings/hosts
Note over UI: Component unmounts
EC->>API: onDelete calls v2Host.delete.mutate
API->>DB: requireActiveOrgMembership pre-check
DB-->>API: organizationId
API->>DB: BEGIN TRANSACTION
DB->>DB: SELECT v2Hosts FOR UPDATE
DB->>DB: SELECT v2UsersHosts FOR UPDATE role check
DB->>DB: SELECT v2Workspaces FOR UPDATE
DB->>DB: UPDATE automations SET enabled false
DB->>DB: DELETE v2Workspaces
DB->>DB: DELETE v2Hosts cascade to v2UsersHosts
DB->>DB: SELECT pg_current_xact_id
DB-->>API: txid
API-->>EC: success true txid
EC-->>UI: isPersisted resolves toast.success
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/trpc/src/router/v2-host/v2-host.test.ts:240-291
**Zero-workspace path is untested**
The single test that exercises the happy path sets up one workspace (`selectResults.push([{ id: WORKSPACE_ID }])`), so the `workspaceIds.length > 0` branch — which produces a different WHERE clause (`eq(automations.targetHostId, ...)` with no `or`/`inArray`) — is never exercised. A host with no synced workspaces skips the `or` entirely; a bug in that branch (e.g. a typo in the column name) would pass the current test suite undetected.
### Issue 2 of 3
apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/DeleteHostSection/DeleteHostSection.tsx:44-55
**`setIsDeleting(false)` called on an unmounted component**
After `await navigate(...)` resolves, the `DeleteHostSection` component is unmounted (the route is `/settings/hosts/$hostId` and we've navigated to `/settings/hosts`). The `finally` block then calls `setIsDeleting(false)` on the now-unmounted component. React 18 silently ignores such updates, but the state reset is a no-op and the state itself is dead — consider omitting it, or using a ref guard to make the intent explicit.
### Issue 3 of 3
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts:381-387
**Non-defensive access to `transaction.mutations[0]`**
`transaction.mutations[0]` is accessed without guarding against an empty `mutations` array; if it is ever empty the handler would throw a `TypeError` rather than a meaningful error. The existing `onUpdate` handler uses the same pattern, so this is consistent with the codebase convention, but a defensive check would fail loudly with a descriptive message rather than a cryptic undefined-access stack trace.
Reviews (1): Last reviewed commit: "feat(desktop): allow owners to delete ho..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }); | ||
|
|
||
| expect(transactionMock).toHaveBeenCalledTimes(1); | ||
| expect(selectForMock).toHaveBeenNthCalledWith(1, "update"); | ||
| expect(selectForMock).toHaveBeenNthCalledWith(2, "update"); | ||
| expect(selectForMock).toHaveBeenNthCalledWith(3, "update"); | ||
| expect(selectFromMock).toHaveBeenNthCalledWith( | ||
| 3, | ||
| realDbSchema.v2Workspaces, | ||
| ); | ||
| expect(deleteMock).toHaveBeenNthCalledWith(1, realDbSchema.v2Workspaces); | ||
| expect(updateMock).toHaveBeenCalledWith(realDbSchema.automations); | ||
| expect(updateSetMock).toHaveBeenCalledWith({ enabled: false }); | ||
| expect(updateSetMock.mock.calls[0]?.[0]).not.toHaveProperty("targetHostId"); | ||
| expect(updateSetMock.mock.calls[0]?.[0]).not.toHaveProperty( | ||
| "v2WorkspaceId", | ||
| ); | ||
| expect(deleteMock).toHaveBeenNthCalledWith(2, realDbSchema.v2Hosts); | ||
| expect(deleteReturningMock).toHaveBeenCalledWith({ | ||
| machineId: realDbSchema.v2Hosts.machineId, | ||
| }); | ||
| expect(executeMock).toHaveBeenCalledTimes(1); | ||
|
|
||
| expect(deleteWhereMock.mock.calls[0]?.[0]).toMatchObject({ | ||
| conditions: [ | ||
| { right: ORGANIZATION_ID, type: "eq" }, | ||
| { right: HOST_ID, type: "eq" }, | ||
| ], | ||
| type: "and", | ||
| }); | ||
| expect(updateWhereMock.mock.calls[0]?.[0]).toMatchObject({ | ||
| conditions: [ | ||
| { right: ORGANIZATION_ID, type: "eq" }, | ||
| { | ||
| conditions: [ | ||
| { right: HOST_ID, type: "eq" }, | ||
| { right: [WORKSPACE_ID], type: "inArray" }, | ||
| ], | ||
| type: "or", | ||
| }, | ||
| ], | ||
| type: "and", | ||
| }); | ||
| expect(deleteWhereMock.mock.calls[1]?.[0]).toMatchObject({ | ||
| conditions: [ | ||
| { right: ORGANIZATION_ID, type: "eq" }, | ||
| { right: HOST_ID, type: "eq" }, | ||
| ], | ||
| type: "and", | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The single test that exercises the happy path sets up one workspace (selectResults.push([{ id: WORKSPACE_ID }])), so the workspaceIds.length > 0 branch — which produces a different WHERE clause (eq(automations.targetHostId, ...) with no or/inArray) — is never exercised. A host with no synced workspaces skips the or entirely; a bug in that branch (e.g. a typo in the column name) would pass the current test suite undetected.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-host/v2-host.test.ts
Line: 240-291
Comment:
**Zero-workspace path is untested**
The single test that exercises the happy path sets up one workspace (`selectResults.push([{ id: WORKSPACE_ID }])`), so the `workspaceIds.length > 0` branch — which produces a different WHERE clause (`eq(automations.targetHostId, ...)` with no `or`/`inArray`) — is never exercised. A host with no synced workspaces skips the `or` entirely; a bug in that branch (e.g. a typo in the column name) would pass the current test suite undetected.
How can I resolve this? If you propose a fix, please make it concise.| setIsOpen(false); | ||
| await navigate({ to: "/settings/hosts", replace: true }); | ||
|
|
||
| try { | ||
| await transaction.isPersisted.promise; | ||
| toast.success(`Deleted "${hostName}"`); | ||
| } catch { | ||
| // The shared mutation runner reports the error, and the collection | ||
| // restores the host without disrupting wherever the user navigated. | ||
| } finally { | ||
| setIsDeleting(false); | ||
| } |
There was a problem hiding this comment.
setIsDeleting(false) called on an unmounted component
After await navigate(...) resolves, the DeleteHostSection component is unmounted (the route is /settings/hosts/$hostId and we've navigated to /settings/hosts). The finally block then calls setIsDeleting(false) on the now-unmounted component. React 18 silently ignores such updates, but the state reset is a no-op and the state itself is dead — consider omitting it, or using a ref guard to make the intent explicit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/DeleteHostSection/DeleteHostSection.tsx
Line: 44-55
Comment:
**`setIsDeleting(false)` called on an unmounted component**
After `await navigate(...)` resolves, the `DeleteHostSection` component is unmounted (the route is `/settings/hosts/$hostId` and we've navigated to `/settings/hosts`). The `finally` block then calls `setIsDeleting(false)` on the now-unmounted component. React 18 silently ignores such updates, but the state reset is a no-op and the state itself is dead — consider omitting it, or using a ref guard to make the intent explicit.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| onDelete: async ({ transaction }) => { | ||
| const { original } = transaction.mutations[0]; | ||
| const result = await apiClient.v2Host.delete.mutate({ | ||
| hostId: original.machineId, | ||
| }); | ||
| return electricTxidMatch(result.txid); | ||
| }, |
There was a problem hiding this comment.
transaction.mutations[0]
transaction.mutations[0] is accessed without guarding against an empty mutations array; if it is ever empty the handler would throw a TypeError rather than a meaningful error. The existing onUpdate handler uses the same pattern, so this is consistent with the codebase convention, but a defensive check would fail loudly with a descriptive message rather than a cryptic undefined-access stack trace.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts
Line: 381-387
Comment:
**Non-defensive access to `transaction.mutations[0]`**
`transaction.mutations[0]` is accessed without guarding against an empty `mutations` array; if it is ever empty the handler would throw a `TypeError` rather than a meaningful error. The existing `onUpdate` handler uses the same pattern, so this is consistent with the codebase convention, but a defensive check would fail loudly with a descriptive message rather than a cryptic undefined-access stack trace.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/trpc/src/router/v2-host/v2-host.test.ts (1)
228-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for the no-workspace deletion path.
The current test only covers the case where the host has workspaces (
workspaceIds.length > 0). TheworkspaceIds.length === 0branch uses a differentWHEREclause (eq(automations.targetHostId, input.hostId)without theor/inArray), which is untested.💡 Suggested additional test
it("pauses only direct-host automations when the host has no workspaces", async () => { selectResults.push( [{ machineId: HOST_ID }], [{ role: "owner" }], [], // no workspaces ); hostDeleteResults = [{ machineId: HOST_ID }]; const caller = createCaller(createContext()); await expect(caller.v2Host.delete({ hostId: HOST_ID })).resolves.toEqual({ success: true, txid: 456, }); expect(updateWhereMock.mock.calls[0]?.[0]).toMatchObject({ conditions: [ { right: ORGANIZATION_ID, type: "eq" }, { right: HOST_ID, type: "eq" }, ], type: "and", }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trpc/src/router/v2-host/v2-host.test.ts` around lines 228 - 290, Add a test alongside the existing host deletion tests covering the no-workspace branch: configure selectResults with an empty workspace result, invoke v2Host.delete, and assert success. Verify updateWhereMock uses an and condition containing ORGANIZATION_ID and HOST_ID directly, without the workspace inArray/or clause.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/trpc/src/router/v2-host/v2-host.test.ts`:
- Around line 228-290: Add a test alongside the existing host deletion tests
covering the no-workspace branch: configure selectResults with an empty
workspace result, invoke v2Host.delete, and assert success. Verify
updateWhereMock uses an and condition containing ORGANIZATION_ID and HOST_ID
directly, without the workspace inArray/or clause.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f514a0c-693d-40df-87fd-59f892c2f317
📒 Files selected for processing (9)
apps/desktop/src/renderer/routes/_authenticated/hooks/useOptimisticCollectionActions/useOptimisticCollectionActions.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/DeleteHostSection/DeleteHostSection.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/components/DeleteHostSection/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.test.tsapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tspackages/trpc/src/router/v2-host/v2-host.test.tspackages/trpc/src/router/v2-host/v2-host.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Architecture diagram
sequenceDiagram
participant User
participant UI as Desktop UI
participant Search as Settings Search
participant Collection as Electric Collection
participant TRPC as tRPC Server
participant DB as PostgreSQL (Transactional)
Note over User,DB: Host Deletion Flow
User->>UI: Click "Delete host" in Danger Zone
activate UI
UI->>UI: Verify not local host
alt Owner with non-local host
UI->>UI: Show confirmation dialog
User->>UI: Confirm deletion
UI->>Collection: delete(hostId) [optimistic remove]
deactivate UI
Collection->>TRPC: onDelete mutation → v2Host.delete({ hostId })
activate TRPC
TRPC->>TRPC: requireActiveOrgMembership(ctx)
TRPC->>TRPC: Verify session and org membership
Note over TRPC,DB: Transactional block
TRPC->>DB: BEGIN TRANSACTION
activate DB
DB->>DB: SELECT FROM v2_hosts WHERE orgId=id AND hostId=id FOR UPDATE
DB-->>TRPC: host record or empty
alt Host not found
TRPC-->>Collection: NOT_FOUND error
Collection->>UI: Restore host in collection
else Host found
DB->>DB: SELECT FROM v2_users_hosts WHERE orgId, userId, hostId FOR UPDATE
DB-->>TRPC: role record or empty
alt Not owner
TRPC-->>Collection: FORBIDDEN error
Collection->>UI: Restore host in collection
else Is owner
DB->>DB: SELECT FROM v2_workspaces WHERE orgId AND hostId FOR UPDATE
DB-->>TRPC: workspace IDs
alt Has workspace IDs
DB->>DB: UPDATE automations SET enabled=false WHERE orgId AND (targetHostId=hostId OR v2WorkspaceId IN workspaceIds)
else No workspace IDs
DB->>DB: UPDATE automations SET enabled=false WHERE orgId AND targetHostId=hostId
end
DB->>DB: DELETE FROM v2_workspaces WHERE orgId AND hostId
DB->>DB: DELETE FROM v2_hosts WHERE orgId AND machineId
DB-->>TRPC: deleted host or empty
alt Nothing deleted
TRPC-->>Collection: NOT_FOUND error
Collection->>UI: Restore host in collection
else Deleted successfully
DB->>DB: SELECT txid()
DB-->>TRPC: txid
DB-->>TRPC: COMMIT TRANSACTION
deactivate DB
TRPC-->>Collection: { success: true, txid }
deactivate TRPC
Note over Collection,UI: Persistence confirmed
Collection-->>UI: transaction.isPersisted resolved
activate UI
UI->>UI: Navigate to /settings/hosts
UI->>UI: Show toast "Deleted host name"
deactivate UI
end
end
end
else Local host or non-owner
UI->>UI: Button disabled with tooltip
end
Note over User,Search: Settings Search
User->>Search: Type "delete host"
Search->>Search: Match SETTING_ITEM_ID.HOST_DELETE
Search-->>User: Show "Delete host" result
User->>UI: Navigate to host settings via search result
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Kitenite
changed the title
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Kitenite
changed the title
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/src/router/v2-host/v2-host.ts">
<violation number="1">
P2: The `delete` mutation no longer performs an explicit workspace guard before issuing the `DELETE`, so the previous clean `TRPCError({ code: "CONFLICT" })` response for hosts that still have workspaces is gone. Because the DB foreign key will block the deletion at the database level, data integrity is preserved, but the endpoint is now likely to surface a raw database error/500 instead of the intended typed conflict. Consider restoring the explicit `v2Workspaces` existence check (or adding a mapped catch for the FK violation) so callers continue to receive a predictable `CONFLICT` error.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -123,6 +123,93 @@ export const v2HostRouter = { | |||
| return { success: true, txid }; | |||
There was a problem hiding this comment.
P2: The delete mutation no longer performs an explicit workspace guard before issuing the DELETE, so the previous clean TRPCError({ code: "CONFLICT" }) response for hosts that still have workspaces is gone. Because the DB foreign key will block the deletion at the database level, data integrity is preserved, but the endpoint is now likely to surface a raw database error/500 instead of the intended typed conflict. Consider restoring the explicit v2Workspaces existence check (or adding a mapped catch for the FK violation) so callers continue to receive a predictable CONFLICT error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/src/router/v2-host/v2-host.ts, line 198:
<comment>The `delete` mutation no longer performs an explicit workspace guard before issuing the `DELETE`, so the previous clean `TRPCError({ code: "CONFLICT" })` response for hosts that still have workspaces is gone. Because the DB foreign key will block the deletion at the database level, data integrity is preserved, but the endpoint is now likely to surface a raw database error/500 instead of the intended typed conflict. Consider restoring the explicit `v2Workspaces` existence check (or adding a mapped catch for the FK violation) so callers continue to receive a predictable `CONFLICT` error.</comment>
<file context>
@@ -1,11 +1,6 @@
import { db, dbWs } from "@superset/db/client";
import { v2UsersHostRoleValues } from "@superset/db/enums";
-import {
- members,
- v2Hosts,
- v2UsersHosts,
- v2Workspaces,
-} from "@superset/db/schema";
+import { members, v2Hosts, v2UsersHosts } from "@superset/db/schema";
</file context>
Summary
User impact
Owners can remove hosts directly from Settings. Workspace, file, conversation, and automation data is never mutated by this flow. Existing database constraints remain authoritative.
Validation
bun run typecheckinpackages/db,packages/trpc, andapps/desktopbunx drizzle-kit checkbun run lintgit diff --check