From a70fe639939855713fdf52f25acfad34c4038868 Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Mon, 24 Aug 2026 10:46:30 -0700
Subject: [PATCH] fix(workspace-cleanup): read git for a pinned workspace
 before deleting it (#15370)

`shouldReadWorkspaceCleanupGitEvidence` refused to read git whenever `blockers`
included 'pinned' -- in a clause `forceGitCheck` could not override. So the
confirm-time forced read, which exists precisely to decide whether a removal
needs force, never ran for pinned rows.

This is reachable today, not only after the planned verdict removal. 'pinned' is
not a queue blocker (queue blockers are main-worktree, folder-repo, dismissed),
so a pinned idle workspace is hand-selectable from the row checkbox right now.
Its git evidence stays `clean: null, checkedAt: null`, and
`shouldForceWorkspaceCleanupRemoval` returns true whenever git is unknown -- so
it force-deletes with no evidence ever obtained.

Moved 'pinned' into the cost-skip clause so `forceGitCheck` overrides it:
broad scans still skip it, targeted preflights do not. main-worktree and
folder-repo stay unconditional because both are refused before removal, so
reading git for them is pure cost.

Broad-scan performance is unchanged, and there is now a test pinning that: it
passes before and after. The preflight test fails against the unfixed source.
---
 .../ipc/workspace-cleanup-broad-scan.test.ts  | 38 +++++++++++++++++++
 src/main/ipc/workspace-cleanup-candidate.ts   | 15 +++++---
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/src/main/ipc/workspace-cleanup-broad-scan.test.ts b/src/main/ipc/workspace-cleanup-broad-scan.test.ts
index ab65f6faa22..a5d19d26f7e 100644
--- a/src/main/ipc/workspace-cleanup-broad-scan.test.ts
+++ b/src/main/ipc/workspace-cleanup-broad-scan.test.ts
@@ -132,6 +132,16 @@ function makeStore(repos: Repo[] = [REPO], allMeta: Record<string, WorktreeMeta>
   } as unknown as Store
 }
 
+/** `repo-1::/repo-old` is in the shared fixture map, so pinning it needs a getWorktreeMeta override. */
+function makePinnedStore(): Store {
+  const pinned = makeWorktreeMeta({ isPinned: true, lastActivityAt: NOW - 40 * DAY_MS })
+  return {
+    ...makeStore(),
+    getWorktreeMeta: (worktreeId: string) =>
+      worktreeId === 'repo-1::/repo-old' ? pinned : META_BY_WORKTREE_ID[worktreeId]
+  } as Store
+}
+
 describe('workspace cleanup broad scan opt-in', () => {
   beforeEach(() => {
     vi.useFakeTimers()
@@ -230,6 +240,34 @@ describe('workspace cleanup broad scan opt-in', () => {
     )
   })
 
+  it('skips git for a pinned workspace on a broad scan, which is the cost saving', async () => {
+    const store = makePinnedStore()
+
+    await scanWorkspaceCleanup(store, { includeAllWorkspaces: true })
+
+    expect(getStatusMock).not.toHaveBeenCalledWith(
+      expect.stringContaining('/repo-old'),
+      expect.anything()
+    )
+  })
+
+  it('reads git for a pinned workspace on a targeted preflight, which decides force-removal', async () => {
+    // Why this matters: `pinned` is not a queue blocker, so a pinned idle workspace is
+    // hand-selectable today, and removal forces whenever git is unknown. Skipping the
+    // forced read meant it could be force-deleted with no evidence ever obtained.
+    const worktreeId = 'repo-1::/repo-old'
+
+    const result = await scanWorkspaceCleanup(makePinnedStore(), { worktreeIds: [worktreeId] })
+
+    expect(getStatusMock).toHaveBeenCalledWith(
+      expect.stringContaining('/repo-old'),
+      expect.anything()
+    )
+    const pinned = result.candidates.find((candidate) => candidate.worktreeId === worktreeId)
+    expect(pinned?.blockers).toContain('pinned')
+    expect(pinned?.git.checkedAt).not.toBeNull()
+  })
+
   it('keeps the legacy suggestion-only projection when the flag is absent', async () => {
     const result = await scanWorkspaceCleanup(makeStore())
 
diff --git a/src/main/ipc/workspace-cleanup-candidate.ts b/src/main/ipc/workspace-cleanup-candidate.ts
index 58c56316acc..72f89161690 100644
--- a/src/main/ipc/workspace-cleanup-candidate.ts
+++ b/src/main/ipc/workspace-cleanup-candidate.ts
@@ -192,11 +192,16 @@ function shouldReadWorkspaceCleanupGitEvidence(args: {
   if ((skipGit && !forceGitCheck) || repoIsFolder || worktree.isMainWorktree) {
     return false
   }
-  if (
-    blockers.includes('pinned') ||
-    blockers.includes('main-worktree') ||
-    blockers.includes('folder-repo')
-  ) {
+  // Why pinned sits with the cost skips and not the refusals: a pinned workspace is
+  // still queueable (`pinned` is not a queue blocker), so it can reach removal -- and
+  // removal forces whenever git is unknown. Skipping its git read on a broad scan is a
+  // fair saving; skipping it on the confirm-time forced read meant deleting with no
+  // evidence ever obtained. main-worktree and folder-repo stay unconditional: those are
+  // refused before removal, so reading git for them is pure cost.
+  if (blockers.includes('pinned') && !forceGitCheck) {
+    return false
+  }
+  if (blockers.includes('main-worktree') || blockers.includes('folder-repo')) {
     return false
   }