Skip to content

Add saved force-delete preference to workspace deletion menu - #26425

Merged
nwparker merged 8 commits into
mainfrom
nwparker/always-delete-inline
Oct 8, 2026
Merged

nwparker merged 8 commits into
mainfrom
nwparker/always-delete-inline

Conversation

@nwparker

@nwparker nwparker commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

When a workspace cannot be deleted normally, Force Delete now has an arrow menu with “Always force delete.” Choosing it deletes the current workspace and remembers the choice for future deletions.

What Changed

The existing ButtonGroup and DropdownMenu primitives form a split button: the main button deletes once, and the arrow opens the saved preference action. Its tooltip says “Applies to all future workspace deletions.” The preference is saved before retrying; a save failure leaves the recovery toast available. The settings handler awaits the existing durable SQLite mutation, rolls back failed writes while preserving unrelated concurrent edits, and notifies other windows only after commit.

The optional alwaysForceDeleteWorktrees user preference defaults to false and uses Orca’s existing persisted profile configuration. It has no Settings control. The shared deletion path applies it for local and SSH workspaces, including terminal-stop verification failures, while preserving host routing, archive-hook handling, lock handling, and metadata-only forget behavior.

Toasts now inherit the app’s tooltip provider. Menu focus stays within the portaled menu instead of triggering Sonner’s toast focus restoration.

Why

The split button keeps one-time deletion direct and puts the persistent choice one click away. Reusing existing controls and configuration avoids a separate preference store or a Settings entry the user does not need.

Linked Issue

Internal maintainer-requested change (author is an organization member); the template’s maintainer exception applies. No existing issue.

Visual Proof

Captured in a hidden, background macOS Electron instance with a disposable dirty worktree, using the visual-proof and electron skills.

Before:

Original recovery toast

After:

Force Delete split button

Menu and tooltip:

Always force delete menu and tooltip

Testing

  • Manually tested in hidden macOS Electron: open menu, hover tooltip, select preference, delete dirty workspace, restart app, verify preference persists, delete another dirty workspace without the failure prompt.
  • Automated tests cover one-time deletion, persistence ordering/failure, portaled menu focus, unsupported errors, local/remote routing, and disconnected metadata-only forget.
  • All 115 tests in the combined deletion, toast, settings, persistence, and SQLite deadline suite pass. Coverage includes awaited disk writes, rejected writes, notification ordering, rollback of existing and absent optional keys, concurrent unrelated edits, and the menu-focus regression.
  • pnpm tc, changed-code quality, localization catalog checks, and Electron build pass locally.
  • The independent Windows SQLite deadline fix was merged in Drain queued SQLite replies before faulting after a main-loop stall #26444. All CI checks pass: static analysis, typecheck, localization, mobile, relay integration, every persistence platform, all ten test shards, Linux compatibility, Linux/Windows packaging, test-selection evidence, and final verification.
  • Windows and SSH behavior covered by existing routing and automated tests; rendered proof captured on macOS only.

Review

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer material copied.

Checklist

  • This PR is small and focused
  • Explained the user-facing change, mechanism, and approach
  • Before/after visual proof attached
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and workspace handling considered
  • Relevant tests, typecheck, changed-code lint, localization checks, and Electron build pass

@nwparker
nwparker requested a review from brennanb2025 as a code owner October 8, 2026 04:08
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b3cf5efd-f51e-4758-967b-5901bf88714c
📥 Commits

Reviewing files that changed from the base of the PR and between cba95f1 and 894416b1190eb4ab8526f9994178ad3e8048e22c.

📒 Files selected for processing (3)
  • src/main/ipc/settings.test.ts
  • src/main/persistence/loading-store/durable-settings-write.test.ts
  • src/main/persistence/loading-store/profile-preferences.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/persistence/loading-store/durable-settings-write.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Adds an alwaysForceDeleteWorktrees setting that defaults to false. When enabled, worktree removal resolves to forced deletion and allows unverified terminal stops, except in forget-local mode. The deletion failure toast can save this preference before force deletion. If saving fails, the toast remains open and reports an error. Tests cover the preference action and local, remote, and metadata-only removal paths.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 89441

The durable-write correction appears ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 17 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the main change: adding a saved force-delete preference to the workspace deletion menu.
Description check ✅ Passed The description follows the required template and explains the user-facing behavior, implementation, rationale, testing, visual proof, compatibility considerations, and checklist completion. The missi…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f6259d60-5f0e-40a1-920b-f417571e1cd9
📥 Commits

Reviewing files that changed from the base of the PR and between 00a8250 and 977e055491597dd0300e2f50f5a31f44d582a9ce.

📒 Files selected for processing (12)
  • src/renderer/src/components/settings/AlwaysForceDeleteWorkspacesSetting.tsx
  • src/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsx
  • src/renderer/src/components/settings/general-search.ts
  • src/renderer/src/components/sidebar/delete-worktree-always-force-preference.test.tsx
  • src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx
  • src/renderer/src/components/sidebar/run-worktree-delete-with-toast.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/store/slices/worktrees-always-force-delete.test.ts
  • src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts
  • src/renderer/src/store/slices/worktrees/teardown/worktree-removal-preference.ts
  • src/shared/default-global-settings.ts
  • src/shared/global-settings-types.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@nwparker
nwparker force-pushed the nwparker/always-delete-inline branch from 977e055 to 27f19e4 Compare October 8, 2026 04:45
@nwparker nwparker changed the title Add Always force delete option to workspace recovery toast Add saved force-delete preference to workspace deletion menu Oct 8, 2026
@nwparker

nwparker commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Reviewed the bot’s remaining pre-merge warnings: this is an internal maintainer-requested change (the author is an organization MEMBER), so the PR template’s maintainer exception applies and no placeholder issue is needed. Blanket docstrings would conflict with AGENTS.md’s requirement for concise, non-obvious comments only; the changed behavior and validation are documented in the PR body. The actionable durable-write finding has been fixed and covered by persistence, IPC, and toast tests.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f435df33-0ed2-4fbc-86e7-c4aeb8fb1c44
📥 Commits

Reviewing files that changed from the base of the PR and between 2dbb2fc and cba95f1.

📒 Files selected for processing (4)
  • src/main/ipc/settings.test.ts
  • src/main/ipc/settings.ts
  • src/main/persistence/loading-store/durable-settings-write.test.ts
  • src/main/persistence/loading-store/profile-preferences.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Comment thread src/main/persistence/loading-store/profile-preferences.ts Outdated
@nwparker
nwparker force-pushed the nwparker/always-delete-inline branch 3 times, most recently from 9c4fdfe to 7f286f1 Compare October 8, 2026 06:05
@nwparker
nwparker force-pushed the nwparker/always-delete-inline branch from 7f286f1 to 141ce66 Compare October 8, 2026 06:10
@nwparker
nwparker merged commit a211489 into main Oct 8, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant