fix(server): refuse worktree removal while another thread references it by Nurozen · Pull Request #9185 · pingdotgg/t3code

Nurozen · 2026-09-02T07:29:19Z

Deleting a thread could remove a git worktree that another thread still uses: the removal path and the checkpoint branch-drift check both consulted only the active shell, so archived threads sharing the worktree were treated as gone, and the legacy sidebar's bulk delete pre-seeded its deleted-set with the whole batch — the first deletion treated still-alive batch mates as deleted and removed a worktree they still pointed at.

The vcsRemoveWorktree handler now refuses while any non-deleted thread (archived included) references the path, branch-drift adoption counts archived siblings, and the legacy sidebar grows its deleted-set only as deletions actually land (matching the current sidebar). Covered by a pure guard unit test, a reactor test proving the archived-sibling case fails on the old logic, and a wire-level server.test.ts case.

Built with Claude Fable 5 on Claude Code.

Note

Medium Risk
Changes worktree removal and branch-drift behavior on shared paths; callers that previously removed worktrees while only archived threads referenced them will now get errors, but the guard reduces risk of breaking concurrent thread workspaces.

Overview
Fixes cases where a git worktree could be removed or branch metadata updated incorrectly because archived threads (and bulk-delete bookkeeping) were ignored.

Server: Adds ProjectionSnapshotQuery.listThreadIdsByWorktreePath to resolve all non-deleted threads at an exact worktree path (archived included). vcsRemoveWorktree now calls this before git worktree remove and fails with a GitCommandError when any thread still references the path; git status refresh runs only after an allowed removal. CheckpointReactor uses the same lookup instead of the active shell snapshot when deciding whether a worktree is shared, so branch-drift adoption does not run on shared paths when the only sibling is archived.

Web: LegacySidebar bulk delete no longer pre-fills deletedThreadKeys with the whole selection; each thread is added only after its delete succeeds, so orphaned-worktree cleanup does not treat still-live batch mates as already gone.

Tests cover the WS guard (including archived owner), reactor behavior with an archived co-tenant, and updated projection mocks.

^{Reviewed by Cursor Bugbot for commit 896ed9a. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Refuse worktree removal and branch adoption when another thread references the path

Adds a ProjectionSnapshotQuery lookup that returns all non-deleted thread ids (including archived threads) matching an exact worktree path, backed by a new SQL query and request schema.
The vcsRemoveWorktree RPC in ws.ts now calls this lookup before removing a worktree; if any thread references the path, it fails with a count-based GitCommandError and skips the Git removal workflow.
The branch-drift handler in CheckpointReactor.ts replaces its shell-snapshot scan with the same lookup and skips branch adoption when any returned thread id differs from the current thread.
Fixes batch thread deletion in LegacySidebar.tsx so the deleted-key set starts empty and grows only after each individual delete succeeds, preventing later deletions from discounting still-alive batch members.
Behavioral Change: ProjectionSnapshotQueryShape gains a new required method; all in-tree mocks and fixtures are updated, but out-of-tree implementations of the service interface will need to add it.

^{Macroscope summarized 896ed9a.}

macroscopeapp · 2026-09-02T07:31:44Z

+export const worktreeRemovalBlockedError = (input: {
+  readonly cwd: string;
+  readonly path: string;
+  readonly referencingThreadIds: ReadonlyArray<ThreadId>;
+}): GitCommandError | null =>
+  input.referencingThreadIds.length === 0
+    ? null
+    : new GitCommandError({
+        operation: "ws.vcsRemoveWorktree",
+        command: "git",
+        cwd: input.cwd,
+        detail: `refused to remove worktree ${input.path}: ${input.referencingThreadIds.length} thread(s) still reference it`,
+      });
+
+/**
+ * Wraps a failure of the thread-reference lookup so the handler stays within
+ * the vcsRemoveWorktree wire error union.
+ */
+export const worktreeOwnershipQueryError = (
+  input: { readonly cwd: string; readonly path: string },
+  cause: unknown,
+): GitCommandError =>
+  new GitCommandError({
+    operation: "ws.vcsRemoveWorktree",
+    command: "git",
+    cwd: input.cwd,
+    detail: `failed to check which threads reference worktree ${input.path}`,
+    cause,
+  });


Both exports here only construct a GitCommandError from their arguments, and each has exactly one call site in ws.ts (worktreeOwnershipQueryError is used once inside Effect.mapError). Per the error conventions, a helper whose only behavior is (...args) => new SomeError({ ...args }) — including a one-off alias used with mapError — should not be introduced; construct the error at the failure boundary so its attributes and cause stay visible, and drop this module (and its test) once the construction lives in the handler.

If you want the sharing check itself to remain unit-testable, keep a pure predicate/selector (e.g. returning the referencing thread ids or a boolean) and build the GitCommandError inline in the vcsRemoveWorktree handler, rather than exporting a pre-built error factory.

^{Posted via Macroscope — Effect Service Conventions}

Fixed in 896ed9a — the helper module is dropped and both GitCommandError constructions now live inline at the vcsRemoveWorktree failure boundary (the lookup failure keeps its cause). The sharing decision reduced to referencingThreadIds.length !== 0, so no separate predicate was needed.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

macroscopeapp · 2026-09-02T07:31:44Z

+describe("worktreeRemovalBlockedError", () => {
+  it("allows removal when no non-deleted thread references the path", () => {
+    expect(
+      worktreeRemovalBlockedError({
+        cwd: "/repo",
+        path: "/wt",
+        referencingThreadIds: [],
+      }),
+    ).toBeNull();
+  });
+
+  it("refuses removal while any thread, archived included, references the path", () => {
+    const error = worktreeRemovalBlockedError({
+      cwd: "/repo",
+      path: "/wt",
+      referencingThreadIds: [ThreadId.make("thread-archived"), ThreadId.make("thread-live")],
+    });
+    expect(error?._tag).toBe("GitCommandError");
+    expect(error?.detail).toContain("2 thread(s) still reference it");
+  });
+});
+
+describe("worktreeOwnershipQueryError", () => {
+  it("wraps lookup failures as a wire-compatible git command error", () => {
+    const cause = new Error("db unavailable");
+    const error = worktreeOwnershipQueryError({ cwd: "/repo", path: "/wt" }, cause);
+    expect(error._tag).toBe("GitCommandError");
+    expect(error.cause).toBe(cause);
+  });


These tests only assert that two thin constructors return a GitCommandError; the behavior this PR actually changes is untested: the vcsRemoveWorktree handler refusing removal when an archived thread still references input.path (and allowing it otherwise), and followWorktreeBranchDrift now treating an archived sibling as a shared worktree. Consider replacing these with focused tests at those boundaries — e.g. a ws.ts handler test with a stub ProjectionSnapshotQuery whose archived snapshot contains a thread on the same worktreePath, and a CheckpointReactor test asserting no thread.meta.update dispatch in that case.

^{Posted via Macroscope — Effect Service Conventions}

Fixed in 896ed9a — the constructor-only tests are gone with the module; the PR now tests the real boundaries instead: a wire-level server test asserting vcsRemoveWorktree refuses (and never invokes the git driver) while a thread still references the path, and a CheckpointReactor test asserting branch-drift adoption is skipped when an archived sibling shares the worktree (that case fails against the pre-fix logic).

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

macroscopeapp · 2026-09-02T07:37:49Z

Approvability

Verdict: Approved at 896ed9a

Macroscope's review found this PR approvable — This is a focused bug fix that prevents worktree deletion or branch adoption when non-deleted archived threads still reference the path, while preserving the normal unshared path. The production logic is small and boundary-tested; the other changes are test fixtures or narrowly scoped bulk-delete bookkeeping.

^{You can add or adjust custom eligibility rules. Learn more.}

Worktree removal and shared-worktree detection only consulted the active shell, so archived threads sharing a worktree were treated as gone: deleting a sibling could remove a worktree an archived thread still points at, and checkpoint branch-drift adoption ignored archived co-owners. Removal now refuses while any non-deleted thread references the path — resolved by a single atomic query so a thread can never fall between separate active and archived reads — drift adoption counts archived siblings, and the legacy sidebar's bulk delete no longer pre-seeds its deleted-set with the whole batch (which made the first deletion treat still-alive batch mates as gone). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Mnigos

Ran the tests locally and sanity checked the claims. The archived-sibling reactor test is a real regression test: with just the CheckpointReactor change reverted it fails, and it passes again with the fix. I also grepped for callers of vcsRemoveWorktree and the only one is the thread deletion flow in useThreadActions, so the new guard shouldn't block any legitimate removal.

One small thing I noticed, not a blocker: the guard compares worktree paths exactly, so on macOS a /tmp vs /private/tmp mismatch would slip past it. The old shell snapshot check worked the same way, so nothing new here, just worth knowing.

github-actions Bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Sep 2, 2026

Nurozen mentioned this pull request Sep 2, 2026

feat: fork a conversation from any completed point #9187

Closed

github-actions Bot added the size:M 30-99 changed lines (additions + deletions). label Sep 2, 2026

macroscopeapp Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread apps/server/src/ws.ts Outdated

macroscopeapp Bot reviewed Sep 2, 2026

View reviewed changes

Nurozen force-pushed the fix/worktree-removal-shared-threads branch from 2b78d76 to 896ed9a Compare September 2, 2026 07:48

macroscopeapp Bot approved these changes Sep 2, 2026

View reviewed changes

Mnigos approved these changes Sep 2, 2026

View reviewed changes

Mnigos mentioned this pull request Sep 2, 2026

Worktree path comparisons use exact string equality, so symlinked paths split identity on macOS #9245

Open

2 tasks

shivamhwp mentioned this pull request Sep 10, 2026

fix(server): treat symlinked worktree paths as the same directory #9247

Closed

maria-rcks mentioned this pull request Sep 13, 2026

feat(settings): add automatic storage cleanup per machine and project #11598

Merged

Navigation Menu

fix(server): refuse worktree removal while another thread references it - #9185

fix(server): refuse worktree removal while another thread references it#9185
Nurozen wants to merge 1 commit into
pingdotgg:mainfrom
Nurozen:fix/worktree-removal-shared-threads

Nurozen commented Sep 2, 2026 •

edited by macroscopeapp Bot

Loading

Uh oh!

Uh oh!

macroscopeapp Bot Sep 2, 2026

Uh oh!

Nurozen Sep 2, 2026

Uh oh!

macroscopeapp Bot Sep 2, 2026

Uh oh!

macroscopeapp Bot Sep 2, 2026

Uh oh!

Nurozen Sep 2, 2026

Uh oh!

macroscopeapp Bot Sep 2, 2026

Uh oh!

macroscopeapp Bot commented Sep 2, 2026 •

edited

Loading

Uh oh!

Mnigos left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Nurozen commented Sep 2, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refuse worktree removal and branch adoption when another thread references the path

Uh oh!

Uh oh!

macroscopeapp Bot Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Nurozen Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Nurozen Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approvability

Uh oh!

Mnigos left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Nurozen commented Sep 2, 2026 •

edited by macroscopeapp Bot

Loading

macroscopeapp Bot commented Sep 2, 2026 •

edited

Loading