Skip to content

fix(sidebar): explain a same-path delete refusal in the user's terms - #15178

Closed
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/delete-collision-copy
Closed

brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/delete-collision-copy

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 0 0 0 0
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​43 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​41

Fixes the wording a user sees when Orca refuses to delete a workspace that exists at the same path on two machines.

Before

Failed to delete workspace feature-auth
Workspace identity is ambiguous across hosts. Refresh projects and try again. [View]

Three problems:

  1. "Workspace identity is ambiguous across hosts" is implementation vocabulary — execution-host routing is Orca's problem, not the user's.
  2. "Refresh projects and try again" does not work. Refreshing cannot disambiguate two real checkouts at the same path, so the copy sends the user around a loop that always ends here.
  3. View opens the workspace diff by id — and an id both machines share cannot pick one, so the button either does nothing useful or opens the other machine's changes.

After

final2.png

Failed to delete workspace feature-auth
This workspace exists on more than one machine at the same path, so Orca can't tell which one to remove. Nothing was deleted. We're improving this — for now, delete it directly on the machine you want it gone from.

View is hidden for this case. The toast is now isDestructive: false, so it renders as info rather than a destructive error — nothing was lost.

The remedy is verified, not assumed

Removing the checkout on the machine clears the collision on Orca's next scan, leaving no orphaned row and needing no Force Delete. I tested that before putting it in shipped copy.

Scope

Copy and the View gate only — no routing behaviour changes. The refusal itself is pre-existing on main; routing a colliding delete to the confirmed host is #15013.

Known, not addressed here: after a refusal the row stays greyed out reading "Deleting…" indefinitely (isDeleting is never cleared). That reproduces on main today, and since the toast auto-dismisses after 10s, the stuck row is what the user is ultimately left with. Worth its own fix.

Verification

typecheck 0 · changed-code quality 0 findings · localization coverage passed · sidebar suite 250 files / 2,193 tests green · copy confirmed in a live dev instance (screenshot above).

The refusal said 'Workspace identity is ambiguous across hosts. Refresh
projects and try again.' — implementation vocabulary, and the remedy does
not work: refreshing cannot disambiguate two real checkouts, and the View
action opens the diff by an id both machines share.

Say what happened to their workspace, confirm nothing was lost, and give
the one step that does resolve it. Verified: removing the checkout on the
machine clears the collision on the next scan, with no orphaned row.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Superseded — folded into #14930 (commit fd52aef) instead of shipping separately.

Both changes touch src/shared/worktree/removal.ts, so keeping them apart guaranteed a merge conflict for whichever landed second. #14930 is the PR that produces this refusal, so the copy explaining it belongs with it.

@brennanb2025
brennanb2025 deleted the brennanb2025/delete-collision-copy branch August 17, 2026 22:01
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds detection for worktree removal errors caused by the same workspace path existing on multiple hosts. Deletion toasts now explain that no data was deleted, provide direct-removal guidance, and mark the result as non-destructive. The failure toast hides the “View” action for ambiguous-host errors. English localization adds messages for running terminals and multi-host workspace paths.

Merge Risk: ⚪ Minimal · up to c35f7

The change is limited to clearer delete-refusal messaging and hiding an unusable action; reported typecheck, localization, and sidebar tests pass, and no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change, rationale, scope, visual result, and verification, but it omits the required linked issue and several template sections. Add a valid issue reference after “Fixes #” and complete the required Testing, Review, Notes, Checklist, and Author sections, or mark them not applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: improving sidebar wording for same-path workspace deletion refusals.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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.

🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/delete-worktree-toast.ts (1)

23-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the ambiguous-host branch.

Test getDeleteWorktreeToastCopy with the ambiguous-host marker. Assert isDestructive: false and the localized fallback copy. Also test showDeleteWorktreeFailureToast in src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx to verify toast.info is used and the View action is hidden.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2663a7b2-5346-4541-a75f-606f2044d613

📥 Commits

Reviewing files that changed from the base of the PR and between a3a2c44 and c35f729.

📒 Files selected for processing (4)
  • src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx
  • src/renderer/src/components/sidebar/delete-worktree-toast.ts
  • src/renderer/src/i18n/locales/en.json
  • src/shared/worktree/removal.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 8 remain after this review.

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