Skip to content

fix(web): keep bulk thread deletion going after failures - #4615

Merged
Yash-Singh1 merged 6 commits into
pingdotgg:mainfrom
m-de-graaff:fix/idempotent-worktree-removal
Sep 5, 2026
Merged

fix(web): keep bulk thread deletion going after failures#4615
Yash-Singh1 merged 6 commits into
pingdotgg:mainfrom
m-de-graaff:fix/idempotent-worktree-removal

Conversation

@m-de-graaff

@m-de-graaff m-de-graaff commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Bulk thread deletion currently stops at the first failure. For example, if the first selected thread has a locked worktree, cleanup fails and the remaining healthy threads are never deleted. Addresses the client-side portion of #4513; the server fix already merged in #8076 is preserved.

Both Sidebar and LegacySidebar now continue after ordinary failures, stop on interruption, and retain failed or unprocessed live threads in the selection. Successful deletions and missing thread rows are deselected. Worktree cleanup and fallback navigation failures report their own errors instead of turning a successful thread deletion into a thread-deletion failure. Navigation failure no longer skips requested worktree cleanup.

Validation: recorded the locked-worktree case before and after in the isolated dev web app for both sidebars, and verified the surviving threads and Git worktrees. Web typecheck and targeted tests pass, including stale selection cleanup and rejected navigation. Formatting passes; targeted lint has an existing ref-access warning outside the changed lines.

Updated with GPT-6-Astra via Codex, building on the original Claude Code contribution.


Note

Medium Risk
Changes Git worktree cleanup and multi-thread delete behavior; incorrect idempotency matching could hide real git errors, but scope is localized to removeWorktree and sidebar delete loops.

Overview
Thread deletes no longer fail when Git has already forgotten a worktree, and bulk delete keeps going instead of stopping on the first bad row.

removeWorktree now runs git worktree remove with stable (LC_ALL=C) diagnostics and treats the “is not a working tree” case as success, so stale or duplicate worktree_path values (shared path, pruned worktree, etc.) do not block thread deletion. Real failures—e.g. a dirty worktree without --force—still surface as GitCommandError.

In Sidebar and SidebarV2, multi-select delete only adds thread keys to deletedThreadKeys after a successful delete (so orphaned-worktree logic does not treat batch mates as already gone). Non-interruption failures are collected, the loop continues, one error toast runs at the end, and selection clears only for threads that actually deleted. User cancellation still aborts the batch.

Reviewed by Cursor Bugbot for commit b6dccf4. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Make worktree removal idempotent and continue bulk delete after per-thread failures

  • Bulk thread deletion in LegacySidebar.tsx and Sidebar.tsx now continues after ordinary per-thread failures, stops on interruption, and reports only the first failure in a single toast.
  • Selection cleanup removes only successfully deleted or missing threads, retaining failed, unprocessed, and still-live selections via the new getThreadKeysToDeselectAfterDelete helper in threadSelectionStore.ts.
  • Post-deletion navigation failures are converted to a toast in navigateAfterThreadDeletion in useThreadActions.ts instead of propagating the error; worktree removal failures no longer turn an already-successful thread deletion into a failure.
  • Risk: useThreadActions delete-thread callers that previously expected a rejected promise on cleanup or navigation failure will now receive a resolved result with a toast instead.

Macroscope summarized 6d4de55.

`git worktree remove` exits 128 with "is not a working tree" whenever the
path is no longer registered with git — which happens whenever a thread's
recorded worktree_path has drifted, or when two threads share one worktree
and the first delete already removed it. That failed the whole thread
deletion even though the thread was gone and the worktree was already
absent, and the sidebar's bulk delete returned on the first failure, so one
stale worktree aborted the rest of the selection.

Treat an unregistered path as success (the requested state already holds)
and run the command under LC_ALL=C so the stderr match is locale-proof. Real
failures — a dirty worktree without --force, for example — still error.

In both sidebars, collect failures instead of returning, report once at the
end, and deselect only the threads that actually deleted.

Fixes pingdotgg#4513

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3f6703ea-c7ee-4b68-a9df-225ce093710d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jul 27, 2026
Comment thread apps/web/src/components/Sidebar.tsx Outdated
@macroscopeapp

macroscopeapp Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 6d4de55

Macroscope's review found this PR approvable — This is a localized fix to existing thread deletion behavior: bulk deletion continues after recoverable failures, selection state stays accurate, and post-deletion cleanup errors are reported separately. It adds no new capability, schema, production infrastructure, product defaults, or static-analysis overrides.

You can add or adjust custom eligibility rules. Learn more.

`deletedThreadKeys` was seeded with every selected thread before the loop
ran, so the first delete saw all its batch mates as gone and removed a
worktree they still pointed at. Now that a failure no longer aborts the
batch, a thread that fails to delete would keep a live reference to a
worktree the loop already removed.

Grow the set as deletions land, matching SidebarV2, and deselect only the
threads that actually deleted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@m-de-graaff

Copy link
Copy Markdown
Contributor Author

Pre-seeded batch keys skew orphan check — deletedThreadKeys is pre-populated with all selected threads before the deletion loop.

Fixed in bd9c32b. The finding is correct, and this PR made it reachable more often.

const deletedThreadKeys = new Set(threadKeys) predates this PR, but it was previously bounded by the loop returning on the first failure. Now that the loop continues, a thread whose delete fails stays alive while every later iteration still counts it as gone — so a batch mate can remove a worktree that live thread still points at.

Sidebar.tsx now grows the set as deletions actually land and deselects only the threads that deleted, which is what SidebarV2.tsx already did (its comment says exactly why). The two loops are now identical in this respect.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 27, 2026

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b6dccf4. Configure here.

Comment thread apps/web/src/components/Sidebar.tsx Outdated
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 5, 2026 10:55

Dismissing prior approval to re-evaluate 85af072

Comment thread apps/web/src/components/LegacySidebar.tsx
@cursor

cursor Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Sep 5, 2026
@Yash-Singh1 Yash-Singh1 changed the title fix(server): make worktree removal idempotent and keep bulk delete going fix(web): keep bulk thread deletion going after failures Sep 5, 2026
@Yash-Singh1
Yash-Singh1 merged commit bd7f7ea into pingdotgg:main Sep 5, 2026
24 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 5, 2026
## What's Changed
* fix(server): keep Homebrew mise shims manual-only by @juliusmarminge in pingdotgg/t3code#10085
* fix(ssh): report remote package installation failures accurately by @juliusmarminge in pingdotgg/t3code#10088
* fix(web): keep bulk thread deletion going after failures by @m-de-graaff in pingdotgg/t3code#4615
* fix(web): scale agent spawn rows with interface font by @juliusmarminge in pingdotgg/t3code#10092
* fix(web): prevent sidebar tooltip title clipping by @UtkarshUsername in pingdotgg/t3code#10086
* fix(web): keep the composer expanded until the thread can scroll by @t3dotgg in pingdotgg/t3code#9965
* fix(web): preserve original mention text in the composer by @juliusmarminge in pingdotgg/t3code#10100

## New Contributors
* @m-de-graaff made their first contribution in pingdotgg/t3code#4615

**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260905.1286...v0.0.39-nightly.20260905.1287

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260905.1287
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants