Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 39 additions & 14 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,27 +234,15 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
}),
);

it.effect("does not wrap a remove-worktree command failure in a synthetic error", () =>
it.effect("treats removing a path git no longer registers as a worktree as a no-op", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const pathService = yield* Path.Path;
const missingWorktree = pathService.join(cwd, "missing-worktree");
const driver = yield* GitVcsDriver.GitVcsDriver;
yield* driver.initRepo({ cwd });

const error = yield* driver
.removeWorktree({ cwd, path: missingWorktree })
.pipe(Effect.flip);

assert.deepInclude(error, {
_tag: "GitCommandError",
operation: "GitVcsDriver.removeWorktree",
command: "git",
argumentCount: 3,
cwd,
});
assert.notProperty(error, "cause");
assert.notInclude(error.detail, "Git command failed in");
yield* driver.removeWorktree({ cwd, path: missingWorktree });
}),
);
});
Expand Down Expand Up @@ -686,6 +674,43 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
yield* driver.removeWorktree({ cwd, path: worktreePath });
const fileSystem = yield* FileSystem.FileSystem;
assert.equal(yield* fileSystem.exists(worktreePath), false);

// Removing the same worktree again is a no-op, so a stale thread record
// pointing at it cannot fail a delete.
yield* driver.removeWorktree({ cwd, path: worktreePath });
}),
);

it.effect("does not wrap a remove-worktree command failure in a synthetic error", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const { initialBranch } = yield* initRepoWithCommit(cwd);
const pathService = yield* Path.Path;
const worktreePath = pathService.join(
yield* makeTmpDir("git-worktrees-"),
"dirty-worktree",
);
const driver = yield* GitVcsDriver.GitVcsDriver;

yield* driver.createWorktree({
cwd,
path: worktreePath,
refName: initialBranch,
newRefName: "feature/dirty-worktree",
});
yield* writeTextFile(worktreePath, "untracked.txt", "dirty");

const error = yield* driver.removeWorktree({ cwd, path: worktreePath }).pipe(Effect.flip);

assert.deepInclude(error, {
_tag: "GitCommandError",
operation: "GitVcsDriver.removeWorktree",
command: "git",
argumentCount: 3,
cwd,
});
assert.notProperty(error, "cause");
assert.notInclude(error.detail, "Git command failed in");
}),
);
});
Expand Down
24 changes: 21 additions & 3 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ function isNonRepositoryGitStderr(stderr: string): boolean {
return stderr.toLowerCase().includes("not a git repository");
}

function isUnregisteredWorktreeGitStderr(stderr: string): boolean {
return stderr.toLowerCase().includes("is not a working tree");
}

interface Trace2Monitor {
readonly env: NodeJS.ProcessEnv;
readonly flush: Effect.Effect<void, never>;
Expand Down Expand Up @@ -2398,9 +2402,23 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
args.push("--force");
}
args.push(input.path);
yield* executeGit("GitVcsDriver.removeWorktree", input.cwd, args, {
timeoutMs: 15_000,
fallbackErrorDetail: "git worktree remove failed",
const result = yield* executeGitWithStableDiagnostics(
"GitVcsDriver.removeWorktree",
input.cwd,
args,
{ timeoutMs: 15_000, allowNonZeroExit: true },
);
// A path git no longer registers as a worktree is already in the requested
// state — deleting a thread whose worktree_path has drifted must not fail.
if (result.exitCode === 0 || isUnregisteredWorktreeGitStderr(result.stderr)) {
return;
}
return yield* new GitCommandError({
...gitCommandContext({ operation: "GitVcsDriver.removeWorktree", cwd: input.cwd, args }),
detail: "git worktree remove failed",
...(result.exitCode === null ? {} : { exitCode: result.exitCode }),
stdoutLength: result.stdout.length,
stderrLength: result.stderr.length,
});
});

Expand Down
36 changes: 22 additions & 14 deletions apps/web/src/components/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1838,26 +1838,34 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec
if (!confirmed) return;
}

const deletedThreadKeys = new Set(threadKeys);
for (const { threadRef } of selectedThreadEntries) {
// Grown as deletions actually land, never seeded with the whole batch:
// orphaned-worktree detection must only discount threads that are
// really gone, or the first delete would treat still-alive batch mates
// as deleted and remove a worktree they still point at.
const deletedThreadKeys = new Set<string>();
let firstError: unknown = null;
for (const { threadKey, threadRef } of selectedThreadEntries) {
const result = await deleteThread(threadRef, {
deletedThreadKeys,
});
if (result._tag === "Failure") {
if (!isAtomCommandInterrupted(result)) {
const error = squashAtomCommandFailure(result);
toastManager.add(
stackedThreadToast({
type: "error",
title: "Failed to delete threads",
description: error instanceof Error ? error.message : "An error occurred.",
}),
);
}
return;
if (isAtomCommandInterrupted(result)) break;
// One bad thread must not abort the rest of the selection.
firstError ??= squashAtomCommandFailure(result);
continue;
Comment thread
cursor[bot] marked this conversation as resolved.
}
deletedThreadKeys.add(threadKey);
}
if (firstError !== null) {
toastManager.add(
stackedThreadToast({
type: "error",
title: "Failed to delete threads",
description: firstError instanceof Error ? firstError.message : "An error occurred.",
}),
);
}
removeFromSelection(threadKeys);
removeFromSelection([...deletedThreadKeys]);

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.

Stale bulk selection keys persist

Medium Severity

After bulk delete, removeFromSelection only receives keys recorded in deletedThreadKeys, but the loop runs over selectedThreadEntries, which can be a subset of selectedThreadKeys. Selection entries with no resolvable thread shell are never deleted and are no longer cleared, so multi-select can stay active for keys that no longer map to rows.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b6dccf4. Configure here.

},
[
appSettingsConfirmThreadArchive,
Expand Down
27 changes: 15 additions & 12 deletions apps/web/src/components/SidebarV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1921,28 +1921,31 @@ export default function SidebarV2() {
// really gone, or the first delete would treat still-alive batch mates
// as deleted and remove a worktree they still point at.
const deletedThreadKeys = new Set<string>();
let firstError: unknown = null;
for (const threadKey of threadKeys) {
const thread = threadByKeyRef.current.get(threadKey);
if (!thread) continue;
const result = await deleteThread(scopeThreadRef(thread.environmentId, thread.id), {
deletedThreadKeys,
});
if (result._tag === "Failure") {
if (!isAtomCommandInterrupted(result)) {
const error = squashAtomCommandFailure(result);
toastManager.add(
stackedThreadToast({
type: "error",
title: "Failed to delete threads",
description: error instanceof Error ? error.message : "An error occurred.",
}),
);
}
return;
if (isAtomCommandInterrupted(result)) return;
// One bad thread must not abort the rest of the selection.
firstError ??= squashAtomCommandFailure(result);
continue;
}
deletedThreadKeys.add(threadKey);
}
removeFromSelection(threadKeys);
if (firstError !== null) {
toastManager.add(
stackedThreadToast({
type: "error",
title: "Failed to delete threads",
description: firstError instanceof Error ? firstError.message : "An error occurred.",
}),
);
}
removeFromSelection([...deletedThreadKeys]);
},
[
attemptSettle,
Expand Down
Loading