feat(desktop): add teardown script support for workspace deletion by saddlepaddle · Pull Request #178 · superset-sh/superset

saddlepaddle · 2025-11-29T03:33:32Z

Summary

Adds teardown script support that runs when workspaces are deleted
Teardown scripts are configured in .superset/setup.json under the teardown key
Creates superset-teardown.sh which deletes the associated Neon branch
Shows a toast warning if teardown fails (deletion still proceeds)

Changes

Add teardown field to SetupConfig type
Create teardown.ts utility to run teardown scripts synchronously (60s timeout)
Modify delete mutation to run teardown before worktree removal
Add toast notification for teardown failures in DeleteWorkspaceDialog

Test plan

Create a workspace (should create Neon branch)
Delete the workspace (should delete Neon branch)
Verify Neon branch is removed from Neon dashboard
Test with teardown script failing (should show warning toast but still delete workspace)

Notes

Users need to update their local .superset/setup.json to include:

{
  "commands": ["./superset-setup.sh"],
  "teardown": ["./superset-teardown.sh"]
}

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Workspaces now execute custom teardown commands when deleted for automatic resource cleanup
- Teardown error warnings are displayed to users if cleanup fails during workspace deletion

_{✏️ Tip: You can customize this high-level summary in your review settings.}

vercel · 2025-11-29T03:33:37Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
website	Ready	Preview	Comment	Nov 29, 2025 3:54am

coderabbitai · 2025-11-29T03:33:41Z

Warning

Rate limit exceeded

@saddlepaddle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 46 seconds before requesting another review.

⌛ How to resolve this issue?

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

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

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

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

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 164abed and 4734121.

📒 Files selected for processing (9)

.superset/setup.json (1 hunks)
apps/desktop/src/lib/trpc/routers/workspaces/utils/setup.test.ts (2 hunks)
apps/desktop/src/lib/trpc/routers/workspaces/utils/setup.ts (1 hunks)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1 hunks)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (4 hunks)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (2 hunks)
apps/desktop/src/shared/types.ts (1 hunks)
superset-setup.sh (1 hunks)
superset-teardown.sh (1 hunks)

Walkthrough

This PR introduces workspace teardown functionality, enabling automatic cleanup when a workspace is deleted. It adds a new teardown configuration to setup.json, creates a teardown utility that executes shell commands from the setup configuration, integrates teardown into the workspace deletion flow, updates the type system to include teardown commands, and provides a bash script to delete Neon branches.

Changes

Cohort / File(s)	Summary
Configuration `.superset/setup.json`	Added `teardown` field with teardown script path; consolidated `commands` array to single-line format.
Type Definitions `apps/desktop/src/shared/types.ts`	Added optional `teardown: string[]` property to `SetupConfig` interface.
Teardown Utility `apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts`	New module exporting `TeardownResult` interface and `runTeardown()` function; loads setup config, validates teardown commands, executes them via shell with 60-second timeout and workspace environment variables.
Backend Integration `apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts`	Updated workspace `delete` mutation to invoke teardown before removing worktree; return type now includes optional `teardownError` field.
Frontend UI `apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx`	Added logic to display teardown warning toast on error; added finally block to reset loading state.
Teardown Script `superset-teardown.sh`	New Bash script that validates neonctl availability, deletes Neon branch for workspace, handles missing branches gracefully.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant UI as DeleteWorkspaceDialog
    participant Router as Workspaces Router
    participant Utils as Teardown Utility
    participant Shell as Shell / neonctl
    
    User->>UI: Click delete workspace
    UI->>Router: call delete(workspaceName)
    Router->>Utils: runTeardown(mainRepoPath, worktreePath, workspaceName)
    Utils->>Utils: loadSetupConfig(.superset/setup.json)
    alt Teardown commands defined
        Utils->>Shell: execSync(teardown commands, env vars)
        alt Success
            Shell-->>Utils: exit 0
            Utils-->>Router: {success: true}
        else Error
            Shell-->>Utils: error
            Utils-->>Router: {success: false, error: msg}
        end
    else No teardown commands
        Utils-->>Router: {success: true}
    end
    Router->>Router: Remove worktree
    alt Teardown had error
        Router-->>UI: {success: true, teardownError: msg}
        UI->>UI: Display warning toast
    else Teardown succeeded
        Router-->>UI: {success: true}
    end
    UI->>User: Workspace deleted (with optional teardown warning)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring extra attention:

teardown.ts: Review error handling in config loading, command execution timeout behavior, and environment variable passing to ensure teardown commands receive correct context.
workspaces.ts: Verify deletion logic ordering (check worktree exists before teardown), ensure teardown error doesn't prevent workspace removal when appropriate, and confirm error propagation to frontend.
superset-teardown.sh: Validate neonctl error handling, confirm the fixed project-id is correct, and ensure graceful handling of already-deleted branches.
Frontend integration: Confirm toast notification appears correctly and doesn't block the deletion flow completion.

Possibly related PRs

Adjust workspace setup logic to match conductor flow #167: Both PRs modify .superset/setup.json and workspace setup artifacts; this PR extends that with new teardown handling and cleanup scripts.

Poem

🐰 When workspaces fade and their time is through,
A teardown script cleans up the morning dew.
Neon branches pruned with care so neat,
Deletion complete—the circle's sweet! 🍃

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding teardown script support for workspace deletion, which aligns perfectly with the changeset.
Description check	✅ Passed	The description covers the main objectives, changes, test plan, and implementation notes, though it does not strictly follow the provided template structure with all required sections.

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

❤️ Share

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (1)

11-11: Teardown warning toast wiring looks correct; consider also checking success

Using the TRPC result’s teardownError to trigger a toast.warning is a good way to surface non-fatal teardown failures to the user. Import and usage of toast are consistent with the rest of the file.

One thing to consider (can be a follow-up): the delete mutation can return { success: false, error } without throwing; in that case this handler will still close the dialog as if deletion succeeded. You might want to gate onOpenChange(false) and the teardown warning behind result.success and optionally surface an error toast if it’s false.

Also applies to: 41-46

apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (1)

15-15: Delete mutation teardown integration is correct; consider slightly richer error handling

The wiring here looks solid:

You guard teardown behind worktree && project and a positive worktreeExists check, so you avoid chdir failures for missing worktrees.

runTeardown errors are captured into teardownError and do not block removal or DB cleanup, which matches the requirement “warn but still delete”.

You still short‑circuit on removeWorktree failures and avoid DB inconsistency.

A couple of optional improvements:

If worktreeExists throws (e.g., git misconfigured), the whole mutation will reject. If you want failure behavior to mirror canDelete, you could wrap worktreeExists in a try/catch here and return a structured { success: false, error } instead of throwing.

Depending on how you conceptualize teardown, you might want teardown to run even when the git worktree is gone (e.g., to clean remote resources keyed only by workspace name). Right now, missing git worktree skips teardown entirely.

These are non-blocking and can be addressed later if they become an issue.

Also applies to: 299-336, 368-369

superset-teardown.sh (1)

1-26: Teardown failure signaling vs. UI behavior may not match expectations

The script is generally robust (dependency check, safe quoting, non-fatal branch‑missing case), but there’s a subtle interaction with how runTeardown interprets success:

Because the neonctl branches delete call is inside if ...; then ... else ... fi with set -e, most failures (branch missing, auth error, etc.) will just take the else branch and still exit with status 0.

runTeardown only treats non‑zero exit codes as failures, so these conditions won’t populate teardownError and no warning toast will be shown, even though you print a warning line in the script.

Since execSync is invoked with stdio: "pipe", that warning line also isn’t visible to the user by default.

If you want conditions like “branch not found” or other Neon errors to trigger the teardown‑warning toast, you could either:

Make those cases exit non‑zero (e.g., echo "..."; exit 1), or

Keep exit 0 but have runTeardown capture and plumb the script’s stdout/stderr up into teardownError.

Otherwise, this behavior is fine as long as “already deleted / not found” is considered a clean teardown.

apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1)

1-74: Teardown runner is solid and defensive; consider reuse and async execution

This implementation looks correct and reasonably hardened:

loadSetupConfig gracefully handles missing/invalid JSON and enforces that teardown is an array before use.

runTeardown no‑ops when no teardown is configured, injects useful env vars, and returns a clear { success, error? } result.

Errors from the child process are logged and mapped into a string, which is appropriate for surfacing via the UI.

A few non‑blocking refinements you might consider:

There’s now a second loadSetupConfig alongside ./utils/setup. If that other helper also reads .superset/setup.json, it may be worth consolidating to a single shared loader (with teardown validation moved there) to avoid drift.

execSync with a 60s timeout will block the Electron main process while teardown runs. If teardown scripts might be slow, wrapping exec/spawn in a Promise and awaiting it would keep the event loop responsive while still enforcing the timeout.

If you care about stricter validation, you could assert that every teardown entry is a string (not just that it’s an array) before join(" && ").

Overall, the current design is acceptable; these are incremental improvements.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4ae2cd and 164abed.

📒 Files selected for processing (6)

.superset/setup.json (1 hunks)
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts (1 hunks)
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts (3 hunks)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx (2 hunks)
apps/desktop/src/shared/types.ts (1 hunks)
superset-teardown.sh (1 hunks)

🧰 Additional context used

📓 Path-based instructions (7)

apps/desktop/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc

Files:

apps/desktop/src/shared/types.ts
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx

apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

apps/desktop/src/shared/types.ts
apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
apps/desktop/src/lib/trpc/routers/workspaces/utils/teardown.ts
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/DeleteWorkspaceDialog.tsx

**/*.{ts,tsx}