From cd8c8c4d50a058e7551c3f25e2cc520108435a24 Mon Sep 17 00:00:00 2001 From: Kiet <31864905+Kitenite@users.noreply.github.com> Date: Tue, 14 Apr 2026 17:33:40 -0700 Subject: [PATCH] feat(v2): unify workspace delete through host-service (#3443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Delete * plans(workspace-delete): refine design per review - Drop unsupported claim about v2 branch ephemerality; deleteBranch defaults off, no persisted preference - Add Host ownership section: v2Workspaces.hostId is 1:1 so destroy is host-local; FORBIDDEN from other hosts - Replace "remote-host cleanup" with "abandoned-host cleanup" as the real follow-up (cross-device delete isn't a thing under the schema) * feat(host-service): workspaceCleanup.destroy Adds a single unified delete path for v2 workspaces. Sequences: terminals → teardown → worktree → branch → cloud → host sqlite. - runTeardown silently executes .superset/teardown.sh with a 60s timeout, SIGKILL on timeout, 4KB output tail capture; returns status + exitCode + tail. - disposeSessionsByWorkspaceId kills all active PTYs for a workspace (releases file locks before worktree remove / teardown). - Typed errors: CONFLICT for dirty worktrees (prompt force: true), INTERNAL_SERVER_ERROR with cause.kind=TEARDOWN_FAILED surfaced to the client via errorFormatter so the renderer can show outputTail. - deleteBranch opts in (default false); force also upgrades `git branch -d` to `-D`. - Cloud failure is swallowed as a warning — disk is already clean, cloud self-heals on next sync. * feat(desktop): wire v2 sidebar delete through workspaceCleanup.destroy - useDestroyWorkspace hook resolves the workspace's host-service URL and calls workspaceCleanup.destroy, normalizing TRPC errors into a typed union (conflict / teardown-failed / unknown) for the dialog. - DashboardSidebarDeleteDialog becomes self-contained: owns the mutation, the "delete local branch" checkbox (off by default), and renders the force-retry UI for dirty worktrees and teardown failures (with outputTail preview). - useDashboardSidebarWorkspaceItemActions drops the direct apiTrpcClient.v2Workspace.delete call; post-success cleanup moves to an onDeleted callback (sidebar removal + focus navigation). Path B (renderer → cloud v2Workspace.delete) is now unused in desktop. * fix(workspace-delete): review follow-ups - Run teardown script via createTerminalSessionInternal (same PTY primitive v2 uses for interactive terminals) so the script inherits the user's shell environment — login rcfiles, PATH, nvm/rbenv, etc. Silent: session is transient and never surfaced as a visible pane. Output captured via pty.onData; timeout via pty.kill() so behavior is cross-platform (no process-group SIGKILL on Windows). - Guard the timeout callback with `if (settled) return;` to prevent the event-loop race where a successful 60s exit is misreported as a timeout. - disposeSession now always marks the DB row disposed even when the in-memory session is missing, so zombie `active` rows left from crashes get reconciled during bulk workspace cleanup. - Export TEARDOWN_TIMEOUT_MS from host-service; renderer uses it in the dialog copy instead of a literal `60`. Renderer also runs the output tail through strip-ansi (host-service returns raw PTY bytes; sanitizing is a presentation concern). - TeardownFailureCause.signal is now `number | null` (Unix signal number from node-pty) rather than a bogus NodeJS.Signals cast. - JSDoc: INTERNAL_ERROR → INTERNAL_SERVER_ERROR on the workspace- cleanup error contract. - Tag the audit doc's ASCII-diagram fence as `text` for markdownlint. - biome lint:fix. * Refactor * fix(workspace-delete): optimistic-close UX (no in-dialog wait) Mirrors v1's pattern: the destroy runs in the background under a toast.loading → success/error, the dialog closes immediately on confirm, and only re-opens when the user has a decision to make. - Confirm/force-retry close the dialog optimistically; mutation continues with a toast for feedback. No more frozen "Deleting..." for the up-to-60s teardown duration. - CONFLICT and TEARDOWN_FAILED reopen the dialog in the matching error pane so the user can force-retry with full context. The branch opt-in is preserved across the reopen. - Unknown errors fall through to toast.error — no reopen. - isPending state and "Deleting..." button copy dropped from all panes (never visible since the dialog closes optimistically). * fix(workspace-delete): move TEARDOWN_TIMEOUT_MS to @superset/shared The renderer used to value-import TEARDOWN_TIMEOUT_MS from @superset/host-service, which dragged node-pty (and transitively @parcel/watcher's native .node binary) into the renderer bundle — esbuild had no loader for .node and failed the build. @superset/shared is renderer-safe. Put the constant there so both host-service and the renderer import the same single source of truth without crossing the bundler boundary. * fix(workspace-delete): auth + tolerate missing host-sqlite row Two bugs unblocking the v2 delete flow: 1. v2Workspace.delete cloud procedure used protectedProcedure (session), so host-service's JWT auth returned 401. Switch to jwtProcedure (mirrors v2Workspace.create) with an org-membership check derived from the workspace's organizationId. Returns alreadyGone: true idempotently when the row is missing. 2. workspaceCleanup.destroy threw NOT_FOUND when the host-sqlite row was missing, even though the workspace exists in the cloud and belongs to this host. That state happens when the workspace was created via a flow that didn't register it locally or the host DB was reset — the user has no way to delete without this fix. Now each disk step is gated on having the local row + a resolvable project; missing rows surface as warnings and cloud cleanup still proceeds. Also makes ctx.api absence a warning instead of an upfront PRECONDITION_FAILED, so local-only cleanup still completes. * plans(workspace-delete): cloud-as-commit-point redesign Reshape destroy as a linear preflight → teardown → cloud → local- cleanup saga. No tombstones, no reconciler, no persistent state. - Any failure before the cloud step leaves the workspace untouched. - Any failure after is a warning (local orphans are cheap). - Force skips preflight + teardown; cleanup is always --force past the commit point. - Three phases cleanly separated so future changes (auto-retry, cross-device reconcile) land at a single seam. Also updates the teardown contract to reflect the PTY-via- createTerminalSessionInternal approach (env parity w/ v2 setup; cross-platform timeout via pty.kill) and pins TEARDOWN_TIMEOUT_MS to @superset/shared so the renderer doesn't drag node-pty into its bundle. * fix(workspace-delete): cloud is the commit point Reorders workspaceCleanup.destroy into three phases so the failure semantics match the plan doc: 0. Preflight (dirty worktree) — throws CONFLICT if !force 1. Teardown script — throws TEARDOWN_FAILED if !force 2. Cloud delete ← commit point; throws passthrough on failure 3. Local cleanup (PTYs, worktree, branch, host sqlite) — warnings only Any failure in phases 0–2 leaves the workspace fully intact. The user retries when they've addressed the cause (committed work, fixed teardown, reconnected cloud, re-authed). Phase 3 is best-effort; local orphans are cheap and surface as warnings. No tombstones, no reconciler, no persistent state. Step 3b always uses --force since we're past the commit point regardless of the input flag. * fix(workspace-delete): preserve TeardownFailureCause fields over wire tRPC's `new TRPCError({ cause })` runs non-Error causes through getCauseFromUnknown() which wraps them in a synthetic UnknownCauseError (Error subclass) while copying fields as own properties. Superjson's transformer recognises it as Error and serialises only { name, message, stack } — our { kind, exitCode, signal, timedOut, outputTail } fields were being dropped on the wire, so the renderer saw an Error with no teardown metadata and TeardownFailedPane crashed on stripAnsi(undefined). The errorFormatter now rebuilds a plain object from the cause fields so superjson serialises it as an object and the renderer gets the full TeardownFailureCause. Also defensive outputTail ?? "" in the pane so stripAnsi never sees undefined. * feat(ui/alert-dialog): make content text selectable Desktop globals set user-select: none on html/body for a native feel. Alert dialogs carry user-facing messages (error details, teardown output tails, descriptions) that users need to copy — e.g. paste an error into chat or grep a log snippet. Applied at the component level so every AlertDialog in the app benefits, not just the workspace-delete error panes. * chore(workspace-delete): tighten stale comments + copy - TeardownFailedPane: drop the defensive-round-trip note now that the errorFormatter reliably serializes the cause as a plain object; keep just the `?? ""` coalesce and the WHY of stripAnsi. - ConflictPane: was phrased as "uncommitted or unlocked work" when CONFLICT is now only thrown by the preflight dirty check (locked- worktree cases fall to warnings). Rewrites copy + JSDoc to match. - teardown.ts: timeout message said SIGKILL, but `pty.kill()` with no args sends SIGHUP on Unix. Drop the false specificity. * chore(workspace-delete): dev-only hook to simulate cloud failure SUPERSET_DEBUG_FAIL_CLOUD_DELETE makes workspaceCleanup.destroy throw at phase 2 (cloud delete) without needing to stop the cloud API dev server. Used to verify the destroy saga's passthrough-on-cloud-fail behavior end-to-end. No-op when unset; guarded solely by env-var presence (truthy). * Revert "chore(workspace-delete): dev-only hook to simulate cloud failure" This reverts commit 5c88d88faea46cbf2698242bfffcc15b766493b1. * fix(workspace-delete): review sweep Addresses open PR comments: - branchDeleted (P1, cubic): track via local flag inside the try block instead of computing from preconditions, so the field reports whether git branch actually succeeded. - Teardown timeout can hang (coderabbit major): if pty.kill() doesn't cause onExit to fire (zombie PTY), settle the promise directly after a 2s grace window so workspaceCleanup.destroy never blocks forever. - formatTeardownReason signal-terminated (coderabbit minor): handle `exitCode === null && signal !== null && !timedOut` with a dedicated "terminated by signal N" branch instead of falling through to "failed to start". - Duplicate run calls (coderabbit major): in-flight ref guard in useDestroyDialogState.run so a rapid second click (same pane or re-opened error pane) can't fire the mutation twice before the first resolves. - Global checkbox id (coderabbit minor): DestroyConfirmPane uses useId() so the "Also delete local branch" label targeting doesn't collide across dialog instances. - Audit doc tense (cubic + coderabbit): v1-v2-delete-patterns-audit.md now explicitly labeled as a pre-unification snapshot. - Plan doc delivered-vs-follow-up split (coderabbit): the UI section and work order clearly mark what landed in this PR vs. what's follow-up (hotkey, EmptyTabView, V2WorkspaceRow affordance). --- .../host-service/useDestroyWorkspace/index.ts | 7 + .../useDestroyWorkspace.ts | 87 ++++++ .../DashboardSidebarDeleteDialog.tsx | 118 +++++--- .../components/ConflictPane/ConflictPane.tsx | 57 ++++ .../components/ConflictPane/index.ts | 1 + .../DestroyConfirmPane/DestroyConfirmPane.tsx | 87 ++++++ .../components/DestroyConfirmPane/index.ts | 1 + .../TeardownFailedPane/TeardownFailedPane.tsx | 68 +++++ .../formatTeardownReason.ts | 16 + .../components/TeardownFailedPane/index.ts | 1 + .../UnknownErrorPane/UnknownErrorPane.tsx | 56 ++++ .../components/UnknownErrorPane/index.ts | 1 + .../hooks/useDestroyDialogState/index.ts | 1 + .../useDestroyDialogState.ts | 96 ++++++ .../DashboardSidebarWorkspaceItem.tsx | 14 +- ...useDashboardSidebarWorkspaceItemActions.ts | 37 +-- packages/host-service/src/index.ts | 1 + .../src/runtime/teardown/index.ts | 6 + .../src/runtime/teardown/teardown.ts | 143 +++++++++ .../host-service/src/terminal/terminal.ts | 66 ++++- packages/host-service/src/trpc/error-types.ts | 24 ++ packages/host-service/src/trpc/index.ts | 35 ++- .../host-service/src/trpc/router/router.ts | 2 + .../trpc/router/workspace-cleanup/index.ts | 1 + .../workspace-cleanup/workspace-cleanup.ts | 177 +++++++++++ packages/shared/src/constants.ts | 3 + .../src/router/v2-workspace/v2-workspace.ts | 37 ++- .../ui/src/components/ui/alert-dialog.tsx | 6 +- plans/v1-v2-delete-patterns-audit.md | 135 +++++++++ plans/workspace-delete-unification.md | 280 ++++++++++++++++++ 30 files changed, 1457 insertions(+), 107 deletions(-) create mode 100644 apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/index.ts create mode 100644 apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/useDestroyWorkspace.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/index.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/TeardownFailedPane.tsx create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/formatTeardownReason.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/index.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/index.ts create mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts create mode 100644 packages/host-service/src/runtime/teardown/index.ts create mode 100644 packages/host-service/src/runtime/teardown/teardown.ts create mode 100644 packages/host-service/src/trpc/error-types.ts create mode 100644 packages/host-service/src/trpc/router/workspace-cleanup/index.ts create mode 100644 packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts create mode 100644 plans/v1-v2-delete-patterns-audit.md create mode 100644 plans/workspace-delete-unification.md diff --git a/apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/index.ts b/apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/index.ts new file mode 100644 index 00000000000..18470f70f61 --- /dev/null +++ b/apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/index.ts @@ -0,0 +1,7 @@ +export { + type DestroyWorkspaceError, + type DestroyWorkspaceInput, + type DestroyWorkspaceSuccess, + type UseDestroyWorkspace, + useDestroyWorkspace, +} from "./useDestroyWorkspace"; diff --git a/apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/useDestroyWorkspace.ts b/apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/useDestroyWorkspace.ts new file mode 100644 index 00000000000..83e2681d7e8 --- /dev/null +++ b/apps/desktop/src/renderer/hooks/host-service/useDestroyWorkspace/useDestroyWorkspace.ts @@ -0,0 +1,87 @@ +import type { TeardownFailureCause } from "@superset/host-service"; +import { TRPCClientError } from "@trpc/client"; +import { useCallback } from "react"; +import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; +import { useWorkspaceHostUrl } from "../useWorkspaceHostUrl"; + +export interface DestroyWorkspaceInput { + deleteBranch?: boolean; + force?: boolean; +} + +export interface DestroyWorkspaceSuccess { + success: boolean; + worktreeRemoved: boolean; + branchDeleted: boolean; + cloudDeleted: boolean; + warnings: string[]; +} + +export type DestroyWorkspaceError = + | { kind: "conflict"; message: string } + | { kind: "teardown-failed"; cause: TeardownFailureCause } + | { kind: "unknown"; message: string }; + +export interface UseDestroyWorkspace { + destroy: (input?: DestroyWorkspaceInput) => Promise; +} + +/** + * Calls `workspaceCleanup.destroy` on the workspace's owning host-service. + * Translates TRPC errors into a typed discriminated union so callers can + * prompt for `force: true` on conflict or teardown failure. + * + * Throws a DestroyWorkspaceError (not a TRPCClientError) for easier handling. + */ +export function useDestroyWorkspace(workspaceId: string): UseDestroyWorkspace { + const hostUrl = useWorkspaceHostUrl(workspaceId); + + const destroy = useCallback( + async ( + input: DestroyWorkspaceInput = {}, + ): Promise => { + if (!hostUrl) { + throw { + kind: "unknown", + message: "Host unavailable", + } satisfies DestroyWorkspaceError; + } + + const client = getHostServiceClientByUrl(hostUrl); + try { + const result = await client.workspaceCleanup.destroy.mutate({ + workspaceId, + deleteBranch: input.deleteBranch ?? false, + force: input.force ?? false, + }); + return result; + } catch (err) { + throw normalizeError(err); + } + }, + [hostUrl, workspaceId], + ); + + return { destroy }; +} + +function normalizeError(err: unknown): DestroyWorkspaceError { + if (err instanceof TRPCClientError) { + const code = err.data?.code as string | undefined; + const teardownFailure = ( + err.data as { teardownFailure?: TeardownFailureCause } + )?.teardownFailure; + + if (teardownFailure) { + return { kind: "teardown-failed", cause: teardownFailure }; + } + if (code === "CONFLICT") { + return { kind: "conflict", message: err.message }; + } + return { kind: "unknown", message: err.message }; + } + return { + kind: "unknown", + message: err instanceof Error ? err.message : String(err), + }; +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx index 06d0d815f48..2dea5a47ecc 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx @@ -1,61 +1,85 @@ -import { - AlertDialog, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, -} from "@superset/ui/alert-dialog"; -import { Button } from "@superset/ui/button"; +import { ConflictPane } from "./components/ConflictPane"; +import { DestroyConfirmPane } from "./components/DestroyConfirmPane"; +import { TeardownFailedPane } from "./components/TeardownFailedPane"; +import { UnknownErrorPane } from "./components/UnknownErrorPane"; +import { useDestroyDialogState } from "./hooks/useDestroyDialogState"; interface DashboardSidebarDeleteDialogProps { + workspaceId: string; + workspaceName: string; open: boolean; onOpenChange: (open: boolean) => void; - onConfirm: () => void; - title: string; - description: string; - isPending?: boolean; + /** Fires after a successful destroy (any warnings reported via toast). */ + onDeleted?: () => void; } +/** + * Dispatches between confirm / conflict / teardown-failed / unknown-error + * panes based on the error returned by `workspaceCleanup.destroy`. The + * destroy itself runs in the background under a toast — this dialog is + * only on screen when the user has a decision to make. + */ export function DashboardSidebarDeleteDialog({ + workspaceId, + workspaceName, open, onOpenChange, - onConfirm, - title, - description, - isPending = false, + onDeleted, }: DashboardSidebarDeleteDialogProps) { + const { + deleteBranch, + setDeleteBranch, + error, + clearError, + handleOpenChange, + run, + } = useDestroyDialogState({ + workspaceId, + workspaceName, + onOpenChange, + onDeleted, + }); + + if (error?.kind === "conflict") { + return ( + run(true)} + /> + ); + } + + if (error?.kind === "teardown-failed") { + return ( + run(true)} + /> + ); + } + + if (error?.kind === "unknown") { + return ( + + ); + } + return ( - - - - {title} - {description} - - - - - - - + onOpenChange={handleOpenChange} + workspaceName={workspaceName} + deleteBranch={deleteBranch} + onDeleteBranchChange={setDeleteBranch} + onConfirm={() => run(false)} + /> ); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx new file mode 100644 index 00000000000..3a96e54dca6 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx @@ -0,0 +1,57 @@ +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@superset/ui/alert-dialog"; +import { Button } from "@superset/ui/button"; + +interface ConflictPaneProps { + open: boolean; + onOpenChange: (open: boolean) => void; + /** Re-runs destroy with `force: true`. */ + onForceDelete: () => void; +} + +/** Shown when the preflight dirty-worktree check blocks destroy. */ +export function ConflictPane({ + open, + onOpenChange, + onForceDelete, +}: ConflictPaneProps) { + return ( + + + + + Uncommitted changes in worktree + + + The worktree has uncommitted changes. Delete anyway will discard + them. + + + + + + + + + ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts new file mode 100644 index 00000000000..76a14754291 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts @@ -0,0 +1 @@ +export { ConflictPane } from "./ConflictPane"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx new file mode 100644 index 00000000000..441816c2ba2 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx @@ -0,0 +1,87 @@ +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@superset/ui/alert-dialog"; +import { Button } from "@superset/ui/button"; +import { Checkbox } from "@superset/ui/checkbox"; +import { Label } from "@superset/ui/label"; +import { useId } from "react"; + +interface DestroyConfirmPaneProps { + open: boolean; + onOpenChange: (open: boolean) => void; + workspaceName: string; + deleteBranch: boolean; + onDeleteBranchChange: (next: boolean) => void; + onConfirm: () => void; +} + +/** + * Default pane: the first click on "Delete". Offers the branch opt-in. + * Confirm hands off to the parent which closes the dialog and runs the + * destroy under a toast — no in-dialog pending state. + */ +export function DestroyConfirmPane({ + open, + onOpenChange, + workspaceName, + deleteBranch, + onDeleteBranchChange, + onConfirm, +}: DestroyConfirmPaneProps) { + const checkboxId = useId(); + return ( + + + + + Delete workspace "{workspaceName}"? + + + This removes the worktree from disk. The cloud workspace record will + also be removed. + + +
+
+ + onDeleteBranchChange(checked === true) + } + /> + +
+
+ + + + +
+
+ ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/index.ts new file mode 100644 index 00000000000..14fdb370d38 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/index.ts @@ -0,0 +1 @@ +export { DestroyConfirmPane } from "./DestroyConfirmPane"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/TeardownFailedPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/TeardownFailedPane.tsx new file mode 100644 index 00000000000..90b3555ba7d --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/TeardownFailedPane.tsx @@ -0,0 +1,68 @@ +import type { TeardownFailureCause } from "@superset/host-service"; +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@superset/ui/alert-dialog"; +import { Button } from "@superset/ui/button"; +import stripAnsi from "strip-ansi"; +import { formatTeardownReason } from "./formatTeardownReason"; + +interface TeardownFailedPaneProps { + open: boolean; + onOpenChange: (open: boolean) => void; + cause: TeardownFailureCause; + /** Re-runs destroy with `force: true` — skips teardown entirely. */ + onForceDelete: () => void; +} + +/** Shown when `.superset/teardown.sh` exited non-zero or timed out. */ +export function TeardownFailedPane({ + open, + onOpenChange, + cause, + onForceDelete, +}: TeardownFailedPaneProps) { + const reason = formatTeardownReason(cause); + // Strip ANSI so raw PTY bytes render readably in the
.
+	const cleanTail = stripAnsi(cause.outputTail ?? "");
+
+	return (
+		
+			
+				
+					{reason}
+					
+						Delete anyway will skip the teardown script entirely.
+					
+				
+				{cleanTail && (
+					
+						{cleanTail}
+					
+ )} + + + + +
+
+ ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/formatTeardownReason.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/formatTeardownReason.ts new file mode 100644 index 00000000000..8c0f002c4aa --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/formatTeardownReason.ts @@ -0,0 +1,16 @@ +import type { TeardownFailureCause } from "@superset/host-service"; +import { TEARDOWN_TIMEOUT_MS } from "@superset/shared/constants"; + +/** Human-readable one-liner for the dialog title when teardown fails. */ +export function formatTeardownReason(cause: TeardownFailureCause): string { + if (cause.timedOut) { + return `Teardown timed out after ${Math.round(TEARDOWN_TIMEOUT_MS / 1000)}s`; + } + if (cause.exitCode != null) { + return `Teardown exited with code ${cause.exitCode}`; + } + if (cause.signal != null) { + return `Teardown terminated by signal ${cause.signal}`; + } + return "Teardown failed to start"; +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/index.ts new file mode 100644 index 00000000000..5f3624f7146 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/TeardownFailedPane/index.ts @@ -0,0 +1 @@ +export { TeardownFailedPane } from "./TeardownFailedPane"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx new file mode 100644 index 00000000000..60c3e9ae216 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx @@ -0,0 +1,56 @@ +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@superset/ui/alert-dialog"; +import { Button } from "@superset/ui/button"; + +interface UnknownErrorPaneProps { + open: boolean; + onOpenChange: (open: boolean) => void; + message: string; + /** Clears the error and returns to the default confirm pane. */ + onRetry: () => void; +} + +/** Fallback for TRPC/network errors that aren't CONFLICT or TEARDOWN_FAILED. */ +export function UnknownErrorPane({ + open, + onOpenChange, + message, + onRetry, +}: UnknownErrorPaneProps) { + return ( + + + + + Delete failed + + {message} + + + + + + + + ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts new file mode 100644 index 00000000000..967bb538ef0 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts @@ -0,0 +1 @@ +export { UnknownErrorPane } from "./UnknownErrorPane"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/index.ts new file mode 100644 index 00000000000..3a79f75e96c --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/index.ts @@ -0,0 +1 @@ +export { useDestroyDialogState } from "./useDestroyDialogState"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts new file mode 100644 index 00000000000..a8a19b15a17 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts @@ -0,0 +1,96 @@ +import { toast } from "@superset/ui/sonner"; +import { useCallback, useRef, useState } from "react"; +import { + type DestroyWorkspaceError, + useDestroyWorkspace, +} from "renderer/hooks/host-service/useDestroyWorkspace"; + +interface UseDestroyDialogStateOptions { + workspaceId: string; + workspaceName: string; + onOpenChange: (open: boolean) => void; + onDeleted?: () => void; +} + +/** + * Drives the delete flow for `DashboardSidebarDeleteDialog`. + * + * UX pattern (mirrors v1's deleteWithToast): + * - On confirm, close the dialog immediately and run the destroy + * in the background under a toast.loading → success/error. + * - For decision-required errors (CONFLICT, TEARDOWN_FAILED) we + * reopen the dialog in the matching error pane so the user can + * force-retry with full context. The branch opt-in is preserved. + * - For unknown errors we just toast.error — no reopen. + */ +export function useDestroyDialogState({ + workspaceId, + workspaceName, + onOpenChange, + onDeleted, +}: UseDestroyDialogStateOptions) { + const { destroy } = useDestroyWorkspace(workspaceId); + + const [deleteBranch, setDeleteBranch] = useState(false); + const [error, setError] = useState(null); + const inFlight = useRef(false); + + const handleOpenChange = useCallback( + (next: boolean) => { + if (!next) { + setDeleteBranch(false); + setError(null); + } + onOpenChange(next); + }, + [onOpenChange], + ); + + const clearError = useCallback(() => setError(null), []); + + const run = useCallback( + async (force: boolean) => { + // Guard against double-submit: optimistic close + async mutate means + // a rapid second click (from the same pane or a re-opened error pane) + // could fire destroy twice before the first resolves. + if (inFlight.current) return; + inFlight.current = true; + + // Optimistic close. State (deleteBranch) preserved in case we re-open + // on a decision-required error. + setError(null); + onOpenChange(false); + + const loadingId = toast.loading(`Deleting ${workspaceName}...`); + + try { + const result = await destroy({ deleteBranch, force }); + toast.success(`Deleted ${workspaceName}`, { id: loadingId }); + for (const warning of result.warnings) toast.warning(warning); + setDeleteBranch(false); + onDeleted?.(); + } catch (err) { + const e = err as DestroyWorkspaceError; + if (e.kind === "conflict" || e.kind === "teardown-failed") { + toast.dismiss(loadingId); + setError(e); + onOpenChange(true); + } else { + toast.error(`Failed to delete: ${e.message}`, { id: loadingId }); + } + } finally { + inFlight.current = false; + } + }, + [destroy, deleteBranch, workspaceName, onOpenChange, onDeleted], + ); + + return { + deleteBranch, + setDeleteBranch, + error, + clearError, + handleOpenChange, + run, + }; +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx index 0f3d5435f2e..0278602c92e 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx @@ -38,7 +38,7 @@ export function DashboardSidebarWorkspaceItem({ handleClick, handleCopyPath, handleCreateSection, - handleDelete, + handleDeleted, handleOpenInFinder, isActive, isDeleteDialogOpen, @@ -124,11 +124,11 @@ export function DashboardSidebarWorkspaceItem({ {!isPending && ( )} @@ -186,11 +186,11 @@ export function DashboardSidebarWorkspaceItem({ {!isPending && ( )} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts index 8e41962d19b..eedc58a5daa 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts @@ -75,7 +75,12 @@ export function useDashboardSidebarWorkspaceItemActions({ } }; - const handleDelete = () => { + /** + * Runs after `workspaceCleanup.destroy` succeeds. Removes the row from + * the sidebar and, if we were viewing the deleted workspace, navigates + * to the next sibling or home. + */ + const handleDeleted = () => { const focusTargetId = isActive ? getDeleteFocusTargetWorkspaceId( getFlattenedV2WorkspaceIds(collections), @@ -83,28 +88,14 @@ export function useDashboardSidebarWorkspaceItemActions({ ) : null; - setIsDeleteDialogOpen(false); + removeWorkspaceFromSidebar(workspaceId); - const deletePromise = (async () => { - await apiTrpcClient.v2Workspace.delete.mutate({ id: workspaceId }); - removeWorkspaceFromSidebar(workspaceId); - })(); - - toast.promise(deletePromise, { - loading: "Deleting workspace...", - success: "Workspace deleted", - error: (error) => - `Failed to delete: ${error instanceof Error ? error.message : "Unknown error"}`, - }); - - void deletePromise.then(() => { - if (!isActive) return; - if (focusTargetId) { - void navigateToV2Workspace(focusTargetId, navigate); - } else { - void navigate({ to: "/" }); - } - }); + if (!isActive) return; + if (focusTargetId) { + void navigateToV2Workspace(focusTargetId, navigate); + } else { + void navigate({ to: "/" }); + } }; const handleCreateSection = () => { @@ -157,7 +148,7 @@ export function useDashboardSidebarWorkspaceItemActions({ handleClick, handleCopyPath, handleCreateSection, - handleDelete, + handleDeleted, handleOpenInFinder, isActive, isDeleteDialogOpen, diff --git a/packages/host-service/src/index.ts b/packages/host-service/src/index.ts index 8809e9ec86c..e50521cf1ee 100644 --- a/packages/host-service/src/index.ts +++ b/packages/host-service/src/index.ts @@ -19,5 +19,6 @@ export { LocalModelProvider, } from "./providers/model-providers"; export type { GitCredentialProvider, GitFactory } from "./runtime/git"; +export type { TeardownFailureCause } from "./trpc/error-types"; export type { AppRouter } from "./trpc/router"; export type { ApiClient, HostServiceContext } from "./types"; diff --git a/packages/host-service/src/runtime/teardown/index.ts b/packages/host-service/src/runtime/teardown/index.ts new file mode 100644 index 00000000000..9552c1f61c2 --- /dev/null +++ b/packages/host-service/src/runtime/teardown/index.ts @@ -0,0 +1,6 @@ +export { + runTeardown, + TEARDOWN_SCRIPT_REL_PATH, + TEARDOWN_TIMEOUT_MS, + type TeardownResult, +} from "./teardown"; diff --git a/packages/host-service/src/runtime/teardown/teardown.ts b/packages/host-service/src/runtime/teardown/teardown.ts new file mode 100644 index 00000000000..fae078d8818 --- /dev/null +++ b/packages/host-service/src/runtime/teardown/teardown.ts @@ -0,0 +1,143 @@ +import { randomUUID } from "node:crypto"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { TEARDOWN_TIMEOUT_MS } from "@superset/shared/constants"; +import type { HostDb } from "../../db"; +import { + createTerminalSessionInternal, + disposeSession, +} from "../../terminal/terminal"; + +export { TEARDOWN_TIMEOUT_MS }; + +export const TEARDOWN_SCRIPT_REL_PATH = ".superset/teardown.sh"; +const OUTPUT_TAIL_BYTES = 4096; +const KILL_GRACE_MS = 2_000; + +export type TeardownResult = + | { status: "ok"; output?: string } + | { status: "skipped" } + | { + status: "failed"; + exitCode: number | null; + /** Unix signal number, or null on normal exit. */ + signal: number | null; + timedOut: boolean; + /** Raw PTY bytes — shell output including ANSI. Renderer strips for display. */ + outputTail: string; + }; + +interface RunTeardownOptions { + db: HostDb; + workspaceId: string; + worktreePath: string; + timeoutMs?: number; +} + +/** + * Runs `.superset/teardown.sh` inside the workspace, reusing the same + * terminal primitive v2 uses for interactive sessions. This gives the + * script full environment parity with the user's terminals (login shell + * rcfiles, PATH, nvm/rbenv, etc.), matching how setup.sh runs. + * + * Silent by design — the PTY session is transient and not surfaced as a + * visible pane. The renderer only sees the output tail on failure. + */ +export async function runTeardown({ + db, + workspaceId, + worktreePath, + timeoutMs = TEARDOWN_TIMEOUT_MS, +}: RunTeardownOptions): Promise { + const scriptPath = join(worktreePath, TEARDOWN_SCRIPT_REL_PATH); + if (!existsSync(scriptPath)) return { status: "skipped" }; + + const terminalId = randomUUID(); + // Single-quoted so no shell interpolation is possible on the path. + const initialCommand = `bash ${singleQuote(scriptPath)} ; exit $?`; + + const session = createTerminalSessionInternal({ + terminalId, + workspaceId, + db, + initialCommand, + }); + if ("error" in session) { + return { + status: "failed", + exitCode: null, + signal: null, + timedOut: false, + outputTail: `Failed to start teardown session: ${session.error}`, + }; + } + + let tail = ""; + const appendTail = (chunk: string) => { + tail += chunk; + if (tail.length > OUTPUT_TAIL_BYTES) { + tail = tail.slice(-OUTPUT_TAIL_BYTES); + } + }; + const dataDisposer = session.pty.onData(appendTail); + + return new Promise((resolve) => { + let settled = false; + let timedOut = false; + + const settle = (result: TeardownResult) => { + if (settled) return; + settled = true; + clearTimeout(timer); + try { + dataDisposer.dispose(); + } catch { + // already disposed + } + disposeSession(terminalId, db); + resolve(result); + }; + + session.pty.onExit(({ exitCode, signal }) => { + if (exitCode === 0 && !timedOut) { + settle({ status: "ok", output: tail || undefined }); + return; + } + settle({ + status: "failed", + exitCode: exitCode ?? null, + signal: signal ?? null, + timedOut, + outputTail: tail, + }); + }); + + const timer = setTimeout(() => { + if (settled) return; + timedOut = true; + appendTail(`\n[teardown timed out after ${timeoutMs}ms]\n`); + try { + session.pty.kill(); + } catch { + // PTY may already be dead + } + // Hard-stop: if onExit doesn't fire shortly after kill (zombie PTY), + // settle the promise directly so workspaceCleanup.destroy never hangs. + setTimeout(() => { + settle({ + status: "failed", + exitCode: null, + signal: null, + timedOut: true, + outputTail: tail, + }); + }, KILL_GRACE_MS).unref(); + }, timeoutMs); + timer.unref(); + }); +} + +/** POSIX single-quote escape: safe for any byte sequence in a path. */ +function singleQuote(s: string): string { + return `'${s.replaceAll("'", "'\\''")}'`; +} diff --git a/packages/host-service/src/terminal/terminal.ts b/packages/host-service/src/terminal/terminal.ts index 219b189245c..fc8b859ecb7 100644 --- a/packages/host-service/src/terminal/terminal.ts +++ b/packages/host-service/src/terminal/terminal.ts @@ -7,7 +7,7 @@ import { type ShellReadyScanState, scanForShellReady, } from "@superset/shared/shell-ready-scanner"; -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import type { Hono } from "hono"; import { type IPty, spawn } from "node-pty"; import type { HostDb } from "../db"; @@ -152,23 +152,29 @@ function resolveShellReady( } } -function disposeSession(terminalId: string, db: HostDb) { +/** + * Kills the PTY (if live) and marks the DB row disposed. Safe to call even + * when there's no in-memory session — e.g. for zombie `active` rows left + * over from a prior crash. Exported so workspaceCleanup can dispose the + * transient teardown session. + */ +export function disposeSession(terminalId: string, db: HostDb) { const session = sessions.get(terminalId); - if (!session) return; - - if (session.shellReadyTimeoutId) { - clearTimeout(session.shellReadyTimeoutId); - session.shellReadyTimeoutId = null; - } - if (!session.exited) { - try { - session.pty.kill(); - } catch { - // PTY may already be dead + if (session) { + if (session.shellReadyTimeoutId) { + clearTimeout(session.shellReadyTimeoutId); + session.shellReadyTimeoutId = null; } + if (!session.exited) { + try { + session.pty.kill(); + } catch { + // PTY may already be dead + } + } + sessions.delete(terminalId); } - sessions.delete(terminalId); db.update(terminalSessions) .set({ status: "disposed", endedAt: Date.now() }) @@ -176,6 +182,38 @@ function disposeSession(terminalId: string, db: HostDb) { .run(); } +/** + * Dispose every active session belonging to the given workspace. + * Returns counts so callers (e.g. workspaceCleanup.destroy) can surface warnings. + */ +export function disposeSessionsByWorkspaceId( + workspaceId: string, + db: HostDb, +): { terminated: number; failed: number } { + const rows = db + .select({ id: terminalSessions.id }) + .from(terminalSessions) + .where( + and( + eq(terminalSessions.originWorkspaceId, workspaceId), + eq(terminalSessions.status, "active"), + ), + ) + .all(); + + let terminated = 0; + let failed = 0; + for (const row of rows) { + try { + disposeSession(row.id, db); + terminated += 1; + } catch { + failed += 1; + } + } + return { terminated, failed }; +} + interface CreateTerminalSessionOptions { terminalId: string; workspaceId: string; diff --git a/packages/host-service/src/trpc/error-types.ts b/packages/host-service/src/trpc/error-types.ts new file mode 100644 index 00000000000..094a3f50d5f --- /dev/null +++ b/packages/host-service/src/trpc/error-types.ts @@ -0,0 +1,24 @@ +/** + * Cross-cutting error shapes surfaced via the tRPC error formatter. + * Lives here (not in a router) to avoid circular imports with `trpc/index.ts`. + */ + +export interface TeardownFailureCause { + kind: "TEARDOWN_FAILED"; + exitCode: number | null; + /** Signal number (Unix). null when the process exited normally. */ + signal: number | null; + timedOut: boolean; + outputTail: string; +} + +export function isTeardownFailureCause( + value: unknown, +): value is TeardownFailureCause { + return ( + !!value && + typeof value === "object" && + "kind" in value && + (value as { kind: unknown }).kind === "TEARDOWN_FAILED" + ); +} diff --git a/packages/host-service/src/trpc/index.ts b/packages/host-service/src/trpc/index.ts index b4324e8d292..7d7aee6ddf9 100644 --- a/packages/host-service/src/trpc/index.ts +++ b/packages/host-service/src/trpc/index.ts @@ -1,10 +1,38 @@ import { initTRPC, TRPCError } from "@trpc/server"; import superjson from "superjson"; import type { HostServiceContext } from "../types"; +import { + isTeardownFailureCause, + type TeardownFailureCause, +} from "./error-types"; -const t = initTRPC - .context() - .create({ transformer: superjson }); +const t = initTRPC.context().create({ + transformer: superjson, + errorFormatter({ shape, error }) { + // tRPC wraps non-Error `cause` values via getCauseFromUnknown() into a + // synthetic UnknownCauseError that carries the original fields as own + // properties. Superjson then serializes it as an Error (message/stack + // only) and drops our fields. Re-build a plain object so the wire + // format keeps `kind`, `exitCode`, `outputTail`, etc. + const teardownFailure: TeardownFailureCause | undefined = + isTeardownFailureCause(error.cause) + ? { + kind: "TEARDOWN_FAILED", + exitCode: error.cause.exitCode, + signal: error.cause.signal, + timedOut: error.cause.timedOut, + outputTail: error.cause.outputTail, + } + : undefined; + return { + ...shape, + data: { + ...shape.data, + teardownFailure, + }, + }; + }, +}); export const router = t.router; export const publicProcedure = t.procedure; @@ -19,4 +47,5 @@ export const protectedProcedure = t.procedure.use(async ({ ctx, next }) => { return next({ ctx }); }); +export type { TeardownFailureCause } from "./error-types"; export type { AppRouter } from "./router"; diff --git a/packages/host-service/src/trpc/router/router.ts b/packages/host-service/src/trpc/router/router.ts index 614284c3b00..494ef0ecde5 100644 --- a/packages/host-service/src/trpc/router/router.ts +++ b/packages/host-service/src/trpc/router/router.ts @@ -10,6 +10,7 @@ import { projectRouter } from "./project"; import { pullRequestsRouter } from "./pull-requests"; import { terminalRouter } from "./terminal"; import { workspaceRouter } from "./workspace"; +import { workspaceCleanupRouter } from "./workspace-cleanup"; import { workspaceCreationRouter } from "./workspace-creation"; export const appRouter = router({ @@ -24,6 +25,7 @@ export const appRouter = router({ project: projectRouter, terminal: terminalRouter, workspace: workspaceRouter, + workspaceCleanup: workspaceCleanupRouter, workspaceCreation: workspaceCreationRouter, }); diff --git a/packages/host-service/src/trpc/router/workspace-cleanup/index.ts b/packages/host-service/src/trpc/router/workspace-cleanup/index.ts new file mode 100644 index 00000000000..c07ad35514d --- /dev/null +++ b/packages/host-service/src/trpc/router/workspace-cleanup/index.ts @@ -0,0 +1 @@ +export { workspaceCleanupRouter } from "./workspace-cleanup"; diff --git a/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts b/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts new file mode 100644 index 00000000000..350aefb5a15 --- /dev/null +++ b/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts @@ -0,0 +1,177 @@ +import { TRPCError } from "@trpc/server"; +import { eq } from "drizzle-orm"; +import { z } from "zod"; +import { projects, workspaces } from "../../../db/schema"; +import { runTeardown, type TeardownResult } from "../../../runtime/teardown"; +import { disposeSessionsByWorkspaceId } from "../../../terminal/terminal"; +import type { TeardownFailureCause } from "../../error-types"; +import { protectedProcedure, router } from "../../index"; + +export const workspaceCleanupRouter = router({ + /** + * Destroy a workspace in three phases: + * + * 0. Preflight — dirty-worktree check (skip if force) + * 1. Teardown — run .superset/teardown.sh (skip if force) + * 2. Cloud delete ← COMMIT POINT — throws if it fails + * 3. Local cleanup — PTYs, worktree, branch, host sqlite (best-effort) + * + * Any failure in phases 0–2 leaves the workspace fully intact. Failures + * in phase 3 become warnings — local orphans are cheap, and the user + * has a toast telling them what was left behind. + * + * Force semantics: + * - skips preflight (step 0) + * - skips teardown (step 1) + * - upgrades `git branch -d` to `-D` in step 3c + * - step 3b always uses `--force` (we're past the commit point) + * + * Typed errors for the renderer: + * - CONFLICT → dirty worktree; prompt force-retry + * - INTERNAL_SERVER_ERROR with `data.teardownFailure` → teardown + * script failed; prompt force-retry + * - PRECONDITION_FAILED → no cloud API configured + * - pass-through → cloud auth / network failure + */ + destroy: protectedProcedure + .input( + z.object({ + workspaceId: z.string(), + deleteBranch: z.boolean().default(false), + force: z.boolean().default(false), + }), + ) + .mutation(async ({ ctx, input }) => { + const warnings: string[] = []; + + const local = ctx.db.query.workspaces + .findFirst({ where: eq(workspaces.id, input.workspaceId) }) + .sync(); + const project = local + ? ctx.db.query.projects + .findFirst({ where: eq(projects.id, local.projectId) }) + .sync() + : undefined; + + // ─── Step 0: Preflight ───────────────────────────────────────── + // Block only on dirty worktree (the common "I forgot to commit" + // case). Anything else the local-cleanup phase handles as warning. + if (!input.force && local && project) { + try { + const git = await ctx.git(local.worktreePath); + const status = await git.status(); + if (!status.isClean()) { + throw new TRPCError({ + code: "CONFLICT", + message: "Worktree has uncommitted changes", + }); + } + } catch (err) { + if (err instanceof TRPCError) throw err; + // Can't read status (missing worktree dir, etc.) — not a + // conflict. Continue; step 3b will skip idempotently. + } + } + + // ─── Step 1: Teardown ────────────────────────────────────────── + // Script is the user's last chance to stop services / flush state + // before the workspace goes away. Failure here is recoverable + // via force-retry, which skips this step. + if (!input.force && local && project) { + const teardown: TeardownResult = await runTeardown({ + db: ctx.db, + workspaceId: input.workspaceId, + worktreePath: local.worktreePath, + }); + if (teardown.status === "failed") { + const cause: TeardownFailureCause = { + kind: "TEARDOWN_FAILED", + exitCode: teardown.exitCode, + signal: teardown.signal, + timedOut: teardown.timedOut, + outputTail: teardown.outputTail, + }; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Teardown script failed", + cause, + }); + } + } + + // ─── Step 2: Cloud delete (commit point) ─────────────────────── + // Past this line, the workspace is gone from the user's perspective + // (sidebar will reflect the cloud state). Local artifacts become + // cleanup debris — never a source of truth. + if (!ctx.api) { + throw new TRPCError({ + code: "PRECONDITION_FAILED", + message: "Cloud API not configured", + }); + } + await ctx.api.v2Workspace.delete.mutate({ id: input.workspaceId }); + + // ─── Step 3: Local cleanup (best-effort) ─────────────────────── + // Every failure in this phase is captured as a warning; the + // caller always sees success. + + // 3a. PTYs + const killed = disposeSessionsByWorkspaceId(input.workspaceId, ctx.db); + if (killed.failed > 0) { + warnings.push(`${killed.failed} terminal(s) may still be running`); + } + + // 3b. Worktree (always --force: we're past the commit point) + // 3c. Optional branch delete + let worktreeRemoved = false; + let branchDeleted = false; + if (local && project) { + const git = await ctx.git(project.repoPath); + try { + await git.raw(["worktree", "remove", "--force", local.worktreePath]); + worktreeRemoved = true; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if ( + message.includes("is not a working tree") || + message.includes("No such file or directory") || + message.includes("ENOENT") + ) { + worktreeRemoved = true; + } else { + warnings.push( + `Failed to remove worktree at ${local.worktreePath}: ${message}`, + ); + } + } + + if (input.deleteBranch && local.branch) { + try { + await git.raw(["branch", input.force ? "-D" : "-d", local.branch]); + branchDeleted = true; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + warnings.push( + `Failed to delete branch ${local.branch}: ${message}`, + ); + } + } + } + + // 3d. Host sqlite row + if (local) { + ctx.db + .delete(workspaces) + .where(eq(workspaces.id, input.workspaceId)) + .run(); + } + + return { + success: true, + cloudDeleted: true, + worktreeRemoved, + branchDeleted, + warnings, + }; + }), +}); diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index a61690d62e8..91315376c94 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -45,6 +45,9 @@ export const TOKEN_CONFIG = { REFRESH_THRESHOLD: 5 * 60, } as const; +// Workspace teardown +export const TEARDOWN_TIMEOUT_MS = 60_000; + // PostHog export const POSTHOG_COOKIE_NAME = "superset"; diff --git a/packages/trpc/src/router/v2-workspace/v2-workspace.ts b/packages/trpc/src/router/v2-workspace/v2-workspace.ts index 6514e47095f..6e52441bfe7 100644 --- a/packages/trpc/src/router/v2-workspace/v2-workspace.ts +++ b/packages/trpc/src/router/v2-workspace/v2-workspace.ts @@ -5,10 +5,7 @@ import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import { z } from "zod"; import { jwtProcedure, protectedProcedure } from "../../trpc"; -import { - requireActiveOrgId, - requireActiveOrgMembership, -} from "../utils/active-org"; +import { requireActiveOrgId } from "../utils/active-org"; import { requireOrgResourceAccess, requireOrgScopedResource, @@ -50,7 +47,10 @@ async function getScopedHost(organizationId: string, hostId: string) { ); } -async function getScopedWorkspace(organizationId: string, workspaceId: string) { +async function _getScopedWorkspace( + organizationId: string, + workspaceId: string, +) { return requireOrgScopedResource( () => dbWs.query.v2Workspaces.findFirst({ @@ -187,15 +187,28 @@ export const v2WorkspaceRouter = { return updated; }), - delete: protectedProcedure + // JWT-authed so host-service can orchestrate the full delete saga + // (terminals → teardown → worktree → branch → cloud → host sqlite) via + // its own JWT auth provider. The session-backed protectedProcedure + // would reject host-service callers with 401. + delete: jwtProcedure .input(z.object({ id: z.string().uuid() })) .mutation(async ({ ctx, input }) => { - const organizationId = await requireActiveOrgMembership( - ctx.session, - "No active organization", - ); - const workspace = await getScopedWorkspace(organizationId, input.id); + const workspace = await dbWs.query.v2Workspaces.findFirst({ + columns: { id: true, organizationId: true }, + where: eq(v2Workspaces.id, input.id), + }); + if (!workspace) { + // Already gone in the cloud; idempotent success. + return { success: true, alreadyGone: true as const }; + } + if (!ctx.organizationIds.includes(workspace.organizationId)) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "Not a member of this organization", + }); + } await dbWs.delete(v2Workspaces).where(eq(v2Workspaces.id, workspace.id)); - return { success: true }; + return { success: true, alreadyGone: false as const }; }), } satisfies TRPCRouterRecord; diff --git a/packages/ui/src/components/ui/alert-dialog.tsx b/packages/ui/src/components/ui/alert-dialog.tsx index 28bcedd6728..b8af505e319 100644 --- a/packages/ui/src/components/ui/alert-dialog.tsx +++ b/packages/ui/src/components/ui/alert-dialog.tsx @@ -8,8 +8,12 @@ import { focusEnterEnabledAlertDialogPrimaryAction } from "../../lib/focus-enter import { cn } from "../../lib/utils"; import { buttonVariants } from "./button"; +// `select-text` opts dialog content back into text selection. The desktop +// renderer sets `user-select: none` globally on `html, body` for a native +// feel; alert dialogs carry user-facing messages (errors, output tails, +// descriptions) that should be selectable/copyable. const alertDialogContentClassName = - "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 sm:max-w-lg"; + "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 select-text sm:max-w-lg"; function AlertDialog({ ...props diff --git a/plans/v1-v2-delete-patterns-audit.md b/plans/v1-v2-delete-patterns-audit.md new file mode 100644 index 00000000000..df618bc17b4 --- /dev/null +++ b/plans/v1-v2-delete-patterns-audit.md @@ -0,0 +1,135 @@ +# Workspace v1 vs v2 — Delete Patterns Audit + +> **Status: pre-unification snapshot.** Describes the state of the three +> disjoint delete implementations before the `workspaceCleanup.destroy` +> redesign landed. Kept for historical context. For the current design +> see [`workspace-delete-unification.md`](./workspace-delete-unification.md). + +Audit of user-triggered delete/remove actions across workspace v1 (`apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`) and v2 (`.../v2-workspace/`) in the desktop app. + +## Architecture + +- **v1 workspace route** has no entity deletes in its own tree — only tab *close* (`workspace/$workspaceId/page.tsx:223-227`). All v1 deletes live in the shared dashboard sidebar (`DashboardSidebar*`). +- **v2 workspace route** owns its deletes internally (session selector, file tree) because v2 ships its own sidebar and pane registry. +- **Workspace-level delete** (Hide + Delete with git-safety) is shared chrome: `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/DeleteWorkspaceDialog/DeleteWorkspaceDialog.tsx`, backed by `apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts` (`canDelete`, `delete`, `close`, `canDeleteWorktree`, `deleteWorktree`). + +## Entity comparison + +| Entity | v1 | v2 | Gap | +|---|---|---|---| +| Workspace | Shared `DeleteWorkspaceDialog.tsx:34-328` — Hide (secondary) + Delete (destructive), Enter-key, uncommitted-changes warning (lines 258-268), optional `deleteLocalBranch` checkbox (lines 270-288) | Same shared dialog. But v2 list row `V2WorkspaceRow.tsx:125-147` exposes only Add/Remove-from-sidebar — no delete entry point from the list view | **v2 list has no direct delete affordance**; only reachable via shared sidebar context | +| Chat session | `DashboardSidebarDeleteDialog.tsx` — AlertDialog confirm | `SessionSelectorItem.tsx:37-56` — AlertDialog confirm + toast | Parity | +| File | Not applicable in v1 sidebar | `FileContextMenu.tsx:56` destructive item → parent `FilesTab.tsx:461-493` `handleDelete` → `alert()` confirm + `toast.promise` + `workspaceTrpc.filesystem.deletePath.useMutation()` | v2-only (v1 sidebar has no file tree) | +| Folder | Not applicable in v1 sidebar | `FolderContextMenu.tsx:65` destructive item → same `FilesTab.handleDelete` path | v2-only | +| Chat message | Absent | Absent (`ChatPaneInterface.tsx` has edit/restart, no delete) | Missing in both | +| Diff entry | Absent | `DiffFileEntry.tsx:19,70-75` shows deleted-state UI but offers no delete trigger | **Dead surface in v2** | +| Tab / Pane | Close only (`workspace/$workspaceId/page.tsx:223-227`) | Close only | By design | +| Project (sidebar) | `DashboardSidebarProjectContextMenu.tsx` | Shared sidebar (same) | Shared | +| Section (sidebar) | `DashboardSidebarSectionContextMenu.tsx` | Shared sidebar (same) | Shared | +| Task | `TaskContextMenu.tsx`, `TaskDetailHeader.tsx`, `TaskActionMenu.tsx` | Shared (tasks route is shared) | Shared | +| Checkpoint / snapshot / agent / plan | None found | None found | Not implemented either side | + +## Net gaps + +1. **v2 workspace list row has no delete entry point.** `V2WorkspaceRow.tsx:125-147` only toggles sidebar membership. v1 users reach delete via the sidebar context menu; v2 list users have no parallel path. +2. **No message deletion anywhere.** Parity gap on both sides, but notable given v2's otherwise richer chat CRUD (edit, restart). +3. **`DiffFileEntry` deleted-state without action.** v2-only dead UI surface — displays "deleted" state but offers no user-initiated delete. +4. **Inconsistent confirm UX copy.** File/folder uses `alert()` with "This action cannot be undone." + action-first order (Delete, Cancel). Session uses `alert()` with "Are you sure..." + Cancel-first order. Both confirm, but inconsistently. + +## Confirmed parity (not gaps) + +- Shared `DeleteWorkspaceDialog` two-path pattern (Hide = non-destructive close, Delete = destructive with git-safety). +- Chat session delete: both confirm. +- Sidebar project/section/task deletes: same code paths (shared dashboard sidebar). +- Tab close vs entity delete: intentional semantic difference. + +## Workspace delete call chain — cloud vs host vs local + +There are **three parallel delete backends** in this repo, and they do not share a code path: + +```text +┌──────────────────────────────────────────────────────────────────────────┐ +│ PATH A — LOCAL WORKTREE (v1 chrome; also reachable from v2 via hotkey) │ +│ │ +│ DeleteWorkspaceDialog.tsx │ +│ │ │ +│ ├─ handleDelete → deleteWithToast({deleteFn, forceDeleteFn}) │ +│ │ → useDeleteWorkspace() (optimistic cache update, rollback) │ +│ │ → electronTrpc.workspaces.delete.useMutation │ +│ │ (IPC: renderer → electron main) │ +│ │ │ +│ └─ handleClose → useCloseWorkspace() │ +│ → electronTrpc.workspaces.close.useMutation │ +│ │ +│ apps/desktop/src/lib/trpc/routers/workspaces/procedures/delete.ts │ +│ canDelete :42-162 git status, terminal count, untracked guard │ +│ delete :164-348 markDeleting → cancel init → kill terminals │ +│ → runTeardown → safety check vs git │ +│ → removeWorktreeFromDisk │ +│ → deleteLocalBranch (optional) │ +│ → localDb delete workspace + worktree row │ +│ → analytics `workspace_deleted` │ +│ close :350-375 kill terminals, delete local row only │ +│ deleteWorktree:462-568 same as delete but by worktreeId │ +│ │ +│ Touches: local SQLite (packages/local-db), disk (git worktree), │ +│ child processes (teardown shell), terminal PTYs. │ +│ Does NOT touch: cloud Postgres, host-service, v2Workspaces table. │ +└──────────────────────────────────────────────────────────────────────────┘ + +┌──────────────────────────────────────────────────────────────────────────┐ +│ PATH B — CLOUD v2 WORKSPACE (v2 sidebar context menu) │ +│ │ +│ DashboardSidebarWorkspaceItem context menu │ +│ → useDashboardSidebarWorkspaceItemActions.ts:72-102 handleDelete │ +│ → apiTrpcClient.v2Workspace.delete.mutate({id}) (HTTP → web) │ +│ → removeWorkspaceFromSidebar(workspaceId) (local Electric sync) │ +│ → navigate away if active │ +│ │ +│ packages/trpc/src/router/v2-workspace/v2-workspace.ts:190-200 │ +│ delete: requireActiveOrgMembership │ +│ → getScopedWorkspace(orgId, id) │ +│ → dbWs.delete(v2Workspaces) where id = ... │ +│ → { success: true } │ +│ │ +│ Touches: cloud Postgres (v2Workspaces row) only. │ +│ Does NOT touch: git worktree, local terminals, teardown, local-db. │ +└──────────────────────────────────────────────────────────────────────────┘ + +┌──────────────────────────────────────────────────────────────────────────┐ +│ PATH C — HOST-SERVICE (daemon; unused by the desktop UI today) │ +│ │ +│ packages/host-service/src/trpc/router/workspace/workspace.ts:164-202 │ +│ delete: requires ctx.api (cloud) configured │ +│ → ctx.api.v2Workspace.delete.mutate({id}) ← calls Path B │ +│ → git worktree remove localWorkspace.worktreePath │ +│ → ctx.db.delete(workspaces) (host-service local sqlite) │ +│ │ +│ Zero call sites from apps/desktop/src/renderer for workspace delete. │ +│ (Desktop uses host-service for git-status/diff/events/terminals, but │ +│ not for workspace lifecycle.) │ +└──────────────────────────────────────────────────────────────────────────┘ +``` + +### What a v2 user actually triggers today + +| Entry point | Path taken | Cloud row deleted? | Worktree removed? | Teardown? | Terminals killed? | `canDelete` checks? | +|---|---|---|---|---|---|---| +| v2 sidebar context menu `Delete` | **B** only | ✅ | ❌ | ❌ | ❌ | ❌ | +| `CLOSE_WORKSPACE` hotkey (`layout.tsx:77-89`) | **A** only | ❌ | ✅ | ✅ | ✅ | ✅ | +| `EmptyTabView` dialog | **A** only | ❌ | ✅ | ✅ | ✅ | ✅ | +| v2-workspaces list row | — (no delete affordance) | — | — | — | — | — | + +### Gaps in the delete path + +1. **Path A and Path B never meet.** Deleting via the v2 sidebar removes the cloud `v2Workspaces` row but leaves the worktree on disk, terminals alive, and teardown unrun. Deleting via the dialog (hotkey or `EmptyTabView`) cleans the worktree but leaves the cloud row orphaned. Users will see ghosts on one side or the other depending on entry point. +2. **No `canDelete` on Path B.** Cloud delete has no git-safety guard, no uncommitted-changes warning, no terminal-count check. Users can nuke a cloud workspace while its worktree has unpushed work. +3. **No optimistic UX on Path B.** Path A has full optimistic rollback (`useDeleteWorkspace.onMutate/onError`); Path B is just `toast.promise` with no cache snapshot — UI reconciles via Electric sync after the fact. +4. **Host-service `workspace.delete` (Path C) is the only unified implementation and is unreachable from the UI.** It already composes cloud + worktree + local cleanup in the right order. Desktop never calls it; it only calls host-service for git-status/diff/terminals (`hooks/host-service/*`). +5. **v2-workspaces list page has no delete at all.** `V2WorkspaceRow.tsx:125-147` only toggles sidebar membership. Users browsing the list must pin to sidebar first, then delete — UX dead end. +6. **Analytics divergence.** Path A emits `workspace_deleted`; Path B emits nothing; Path C would emit via whichever layer fires first. Product metrics will undercount v2 deletes. +7. **Teardown never runs for cloud-originated deletes.** Any `SUPERSET_WORKSPACE_NAME`-dependent cleanup scripts silently skip when users delete via the v2 sidebar. + +## Backend safety (shared, for reference) + +`delete.ts:42-162` — `canDelete` returns `{ canDelete, reason?, activeTerminalCount, hasChanges, hasUnpushedCommits }`. Branch workspaces always deletable (lines 78-88). Worktrees check `hasUncommittedChanges()` and `hasUnpushedCommits()` (lines 127-130). `delete.ts:164-348` — untracked-worktree guard (lines 268-308) prevents removing worktrees not tracked in DB; parallel terminal-kill + teardown (line 239-242); two-step retry with `force` (lines 244-262); analytics `workspace_deleted` (line 343) vs `workspace_closed` (line 372). diff --git a/plans/workspace-delete-unification.md b/plans/workspace-delete-unification.md new file mode 100644 index 00000000000..9eea917bf38 --- /dev/null +++ b/plans/workspace-delete-unification.md @@ -0,0 +1,280 @@ +# Workspace Delete Unification — Design + +v2 deletes go through host-service only. v2 is unlaunched, so we cut over directly. + +## Principle + +**Cloud delete is the commit point.** The saga does everything reversible (preflight checks, teardown) before it touches any authoritative state. Only *after* the cloud row is gone do we touch local disk. If anything up to the cloud delete fails, the workspace is exactly as it was before the user clicked. After the cloud delete, local cleanup is best-effort — orphaned disk artifacts are cheap and recoverable. + +This keeps the decision tree linear and stateless: no pending flags, no tombstones, no reconcilers. The procedure runs top to bottom; either it succeeds or it throws at a specific step the caller can understand and act on. + +The ordering is designed to be **easy to revisit later**. The three phases (preflight, commit, local cleanup) are cleanly separated in the code so a future change (auto-retry, cross-device reconcile, schema tombstone) can be introduced at a single seam without refactoring the happy path. + +## Problem + +- **Path A** `electronTrpc.workspaces.delete` — v1 only. Leave alone; dies with v1. +- **Path B** `apiTrpcClient.v2Workspace.delete` — cloud-only, called from renderer. Orphans worktree, PTYs, host row. **Remove.** +- **Path C** `host-service workspace.delete` — partial composition, unreachable from UI. **Rewrite and wire up as `workspaceCleanup.destroy`.** + +## Sequence + +``` +0. Preflight (if !force) + - git status clean? If dirty → throw CONFLICT. + - Skipped when we have no local row (nothing to check). + +1. Teardown (if !force) + - Run .superset/teardown.sh inside the workspace. + - Fail / timeout → throw TEARDOWN_FAILED with output tail. + - Workspace is fully intact on failure. + +2. Cloud delete ← COMMIT POINT + - ctx.api.v2Workspace.delete.mutate({ id }). + - Missing ctx.api, auth failure, network → throw back to the renderer. + - Workspace is fully intact on failure. + +3. Local cleanup (best-effort; every failure is a warning) + - 3a. Kill PTYs owned by this workspace. + - 3b. git worktree remove --force (we're past the commit point). + - 3c. git branch -d / -D if deleteBranch. + - 3d. Host sqlite delete row. + - Any failure adds a warning to the response; the saga continues. + - Orphans from step 3 are cheap: some disk, some sqlite rows, optional + git branch. User (or a future sweeper) can clean them up later. +``` + +## Procedure + +```ts +workspaceCleanup.destroy: protectedProcedure + .input(z.object({ + workspaceId: z.string(), + deleteBranch: z.boolean().default(false), + force: z.boolean().default(false), + })) + .mutation(async ({ ctx, input }) => { + const warnings: string[] = []; + const local = ctx.db.query.workspaces + .findFirst({ where: eq(workspaces.id, input.workspaceId) }).sync(); + const project = local + ? ctx.db.query.projects + .findFirst({ where: eq(projects.id, local.projectId) }).sync() + : undefined; + + // ─── Preflight ───────────────────────────────────────────────────── + if (!input.force && local && project) { + const status = await (await ctx.git(local.worktreePath)).status(); + if (!status.isClean()) { + throw new TRPCError({ + code: "CONFLICT", + message: "Worktree has uncommitted changes", + }); + } + } + + // ─── Teardown ────────────────────────────────────────────────────── + if (!input.force && local && project) { + const teardown = await runTeardown({ + db: ctx.db, + workspaceId: input.workspaceId, + worktreePath: local.worktreePath, + }); + if (teardown.status === "failed") { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Teardown script failed", + cause: { kind: "TEARDOWN_FAILED", ...teardown }, + }); + } + } + + // ─── Cloud (commit point) ────────────────────────────────────────── + if (!ctx.api) { + throw new TRPCError({ + code: "PRECONDITION_FAILED", + message: "Cloud API not configured", + }); + } + await ctx.api.v2Workspace.delete.mutate({ id: input.workspaceId }); + + // ─── Local cleanup (best-effort) ─────────────────────────────────── + const killed = disposeSessionsByWorkspaceId(input.workspaceId, ctx.db); + if (killed.failed > 0) { + warnings.push(`${killed.failed} terminal(s) may still be running`); + } + + let worktreeRemoved = false; + if (local && project) { + const git = await ctx.git(project.repoPath); + try { + await git.raw(["worktree", "remove", "--force", local.worktreePath]); + worktreeRemoved = true; + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + if (/is not a working tree|No such file or directory|ENOENT/.test(msg)) { + worktreeRemoved = true; + } else { + warnings.push( + `Failed to remove worktree at ${local.worktreePath}: ${msg}`, + ); + } + } + + if (input.deleteBranch && local.branch) { + try { + await git.raw(["branch", input.force ? "-D" : "-d", local.branch]); + } catch (err) { + warnings.push( + `Failed to delete branch ${local.branch}: ${(err as Error).message}`, + ); + } + } + } + + if (local) { + ctx.db.delete(workspaces) + .where(eq(workspaces.id, input.workspaceId)).run(); + } + + return { + success: true, + cloudDeleted: true, + worktreeRemoved, + branchDeleted: Boolean(input.deleteBranch && worktreeRemoved && local), + warnings, + }; + }); +``` + +No preflight `canDelete` endpoint, no `deletingAt` tombstone, no reconciliation loop. The renderer reads git's own errors (via typed TRPC) for the "dirty" decision; cloud failure propagates as a real error and is the user's cue to retry. + +## Failure matrix + +| Step fails | Workspace state | Thrown | Renderer UX | +|---|---|---|---| +| 0 (dirty worktree) | Fully intact | `CONFLICT` | Reopen in ConflictPane → "Delete anyway" → `force: true` | +| 1 (teardown script) | Fully intact | `INTERNAL_SERVER_ERROR` + `cause.kind === "TEARDOWN_FAILED"` + output tail | Reopen in TeardownFailedPane → "Delete anyway" → `force: true` (skips 0+1) | +| 1 (teardown 60s timeout) | Fully intact | Same as above with `timedOut: true` | Same | +| 2 (cloud auth) | Fully intact | `UNAUTHORIZED` | toast "Please sign in again" | +| 2 (cloud network / 5xx) | Fully intact | Passed-through error | toast with message; user retries when reachable | +| 2 (ctx.api missing) | Fully intact | `PRECONDITION_FAILED` | toast with message | +| 3a (PTY kill) | Cloud gone; zombie process | warning | User kills stray process | +| 3b (worktree removal) | Cloud gone; worktree on disk | warning w/ path | `rm -rf ` manually | +| 3c (branch delete) | Cloud gone; branch lingers | warning | `git branch -D` manually | +| 3d (sqlite) | Cloud gone; host row lingers (practically never) | warning | Next launch sweep or manual | + +Everything up to step 2 leaves the workspace untouched — the user can fix the underlying issue and retry. Everything in step 3 is a cheap orphan with a clear warning the user can read in the toast. + +## Renderer flow + +```ts +const destroy = useDestroyWorkspace(); + +async function run(force: boolean, deleteBranch: boolean) { + try { + const result = await destroy({ workspaceId, deleteBranch, force }); + for (const w of result.warnings) toast.warning(w); + toast.success(`Deleted ${name}`); + } catch (err) { + switch (err.kind) { + case "conflict": reopenInConflictPane(); break; + case "teardown-failed": reopenInTeardownPane(err.cause); break; + default: toast.error(err.message); + } + } +} +``` + +Fast path: one click for a clean worktree, no branch delete. Confirm dialog only reappears when the user has a decision to make. + +## UX decisions + +- **Delete branch by default?** No. Checkbox off by default; opt-in per delete, no persisted preference. +- **Always confirm?** Only on CONFLICT or TEARDOWN_FAILED. +- **Split `force` into worktree-force and branch-force?** No. One "I acknowledge this might destroy work" signal. + +## Force semantics + +`force: true` is the single "skip safety gates" flag: + +- Skips step 0 (preflight dirty check). +- Skips step 1 (teardown — don't re-run a known-broken script). +- Upgrades `git branch -d` to `-D` in step 3c. +- Step 3b always uses `--force` (we're past the commit point regardless). + +## UI + +**Delivered in this PR:** +- v2 sidebar context menu uses the shared `DashboardSidebarDeleteDialog` backed by `useDestroyWorkspace` → `workspaceCleanup.destroy`. +- Direct `apiTrpcClient.v2Workspace.delete.mutate` call at `useDashboardSidebarWorkspaceItemActions.ts` removed. +- v1 `DeleteWorkspaceDialog` + `useDeleteWorkspace` left unchanged. + +**Follow-up (out of this PR):** +- Wire the `DELETE_WORKSPACE` hotkey (currently `CLOSE_WORKSPACE`) to the same flow. +- Wire `EmptyTabView` to the same flow. +- Add a delete affordance on `V2WorkspaceRow.tsx`. + +## Host ownership + +`v2Workspaces.hostId` ties each workspace to exactly one host. `workspaceCleanup.destroy` is only callable from the host that owns the workspace (via `useWorkspaceHostUrl` routing on the renderer). Cross-device delete is not a supported operation. + +## Teardown contract (step 1 detail) + +Reuses `createTerminalSessionInternal` — the same PTY primitive v2 setup uses — so the teardown script inherits the user's login shell env (rcfiles, PATH, nvm/rbenv) without duplicating the shell-launch plumbing. + +```ts +runTeardown({ db, workspaceId, worktreePath }) + → { status: "ok"; output?: string } + | { status: "skipped" } // .superset/teardown.sh missing + | { status: "failed"; exitCode: number | null; + signal: number | null; timedOut: boolean; outputTail: string } +``` + +- **Script location**: `/.superset/teardown.sh` (mirrors v2 setup's `.superset/setup.sh`). +- **Execution**: PTY session via `createTerminalSessionInternal` with `initialCommand: "bash '' ; exit $?"`. Session is transient, not surfaced as a visible pane; captured via `pty.onData` and torn down via `disposeSession` when the script settles. +- **Capture**: raw PTY bytes into a 4KB ring buffer; the renderer runs `stripAnsi` for display. +- **Exit handling**: `pty.onExit` → exit code / signal. +- **Timeout**: 60s → `pty.kill()` → `status: "failed"` with `timedOut: true` (cross-platform; no process-group SIGKILL). +- **Missing script**: fast-return `{ status: "skipped" }`. +- **`force` in destroy**: skips this step entirely. +- **`TEARDOWN_TIMEOUT_MS` lives in `@superset/shared/constants`** so the renderer can format the timeout reason without value-importing host-service (which would drag node-pty into the renderer bundle). + +## Why no tombstone / reconciler? + +Rejected in favor of linearity. The tombstone alternative would add: +- A `cloudDeletePending` column on host-sqlite `workspaces`. +- A boot-time + periodic sweeper that retries pending cloud deletes. +- A renderer "pending" warning state the user might not understand. + +It was rejected because: +- Cloud failure in the new design is **before** any local state changes, so there is no orphan to clean up — the user just retries. +- Local orphans (post-commit) are cheap and plainly surfaced as warnings. +- Adding persistent state makes future changes (cross-device reconcile, auto-retry) harder, not easier — the plumbing has to either interact with the flag or ignore it. + +If we later want transient-error auto-retry, the seam is clean: step 2's `catch` is the only place that needs to grow. + +## Out of scope + +- **Abandoned-host cleanup**: retired machines leave zombie hosts + workspaces. Separate "Remove this device" settings flow. +- **Visible-pane teardown**: rejected. v2 setup is visible because users need to see it succeed; teardown has nothing actionable once it starts and the pane evaporates with the workspace. +- **Bulk delete / trash bin**: future flow composing `destroy` calls. + +## Work order + +1. Host-service: new `workspaceCleanup` router with `destroy` (linear sequence above). ✅ +2. Renderer: `useDestroyWorkspace` hook + confirm dialog (dirty + branch opt-in). ✅ +3. Switch `v2Workspace.delete` cloud procedure to `jwtProcedure` so host-service can call it. ✅ +4. Reorder the destroy saga to the linear preflight → teardown → cloud → local-cleanup shape. ✅ +5. Swap v2 delete call sites: + - Sidebar context menu. ✅ + - EmptyTabView, `CLOSE_WORKSPACE` hotkey, `V2WorkspaceRow` list affordance — **follow-up**. +6. Delete Path B call path. ✅ + +## Acceptance + +- v2 renderer has one delete target: `hostServiceClient.workspaceCleanup.destroy`. +- `v2Workspace.delete` is only reachable via host-service's JWT auth. +- Clean worktree + no branch delete = one click. +- CONFLICT and TEARDOWN_FAILED offer force-retry with full context. +- Any failure before the cloud step leaves the workspace untouched. +- Any failure after the cloud step surfaces as a warning — never as a phantom workspace row.