diff --git a/src/main/ipc/workspace-cleanup-activity.test.ts b/src/main/ipc/workspace-cleanup-activity.test.ts index 334d6cc4b1d..12548b98747 100644 --- a/src/main/ipc/workspace-cleanup-activity.test.ts +++ b/src/main/ipc/workspace-cleanup-activity.test.ts @@ -1,3 +1,5 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import os from 'node:os' import path from 'node:path' import { describe, expect, it, vi } from 'vitest' import type { Repo, Worktree } from '../../shared/types' @@ -139,6 +141,35 @@ describe('resolveWorkspaceCleanupActivityWorktree', () => { expect(worktree.lastActivityAt).toBe(1_700_000_900_000) }) + it('falls back to a full reflog read when the newest entry exceeds the tail window', async () => { + const dir = await mkdtemp(path.join(os.tmpdir(), 'cleanup-activity-')) + try { + const gitDir = path.join(dir, 'gitdir') + await mkdir(path.join(gitDir, 'logs'), { recursive: true }) + const worktreePath = path.join(dir, 'wt') + await mkdir(worktreePath, { recursive: true }) + await writeFile(path.join(worktreePath, '.git'), `gitdir: ${gitDir}\n`) + // Why: a single record longer than the 8192-byte tail window keeps its + // timestamp before the window; the reader must fall back to a full read. + await writeFile( + path.join(gitDir, 'logs', 'HEAD'), + '0000 1111 Dev 1700000000 -0700\tbranch: Created from HEAD\n' + + `1111 2222 Dev 1700000900 -0700\tcommit: ${'x'.repeat(9000)}\n` + ) + const statPath = vi.fn(async () => ({ mtimeMs: 10_000 })) + + const worktree = await resolveWorkspaceCleanupActivityWorktree( + REPO, + makeWorktree({ path: worktreePath }), + statPath + ) + + expect(worktree.lastActivityAt).toBe(1_700_000_900_000) + } finally { + await rm(dir, { recursive: true, force: true }) + } + }) + it('degrades to other probes when the reflog was expired to an empty file', async () => { const gitDirPath = path.join('/repo', '.git', 'worktrees', 'repo-feature') const statPath = vi.fn(async () => ({ mtimeMs: 10_000 })) diff --git a/src/main/ipc/workspace-cleanup-activity.ts b/src/main/ipc/workspace-cleanup-activity.ts index 7c89e0b7aa8..40d84aec760 100644 --- a/src/main/ipc/workspace-cleanup-activity.ts +++ b/src/main/ipc/workspace-cleanup-activity.ts @@ -134,20 +134,31 @@ async function readNewestReflogEntryAt( try { // A tail cut can start mid-line; scanning backward for the last parseable // entry tolerates that. - const lines = (await readTextFile(reflogPath, { tailBytes: REFLOG_TAIL_BYTES })).split('\n') - for (let index = lines.length - 1; index >= 0; index -= 1) { - // The trailing timezone + tab anchors the capture, so no digit-count floor is needed. - const seconds = /\s(\d{1,11})\s[-+]\d{4}\t/.exec(lines[index] ?? '')?.[1] - if (seconds) { - return Number(seconds) * 1000 - } + const tail = await readTextFile(reflogPath, { tailBytes: REFLOG_TAIL_BYTES }) + const newestFromTail = parseNewestReflogEntryAt(tail) + if (newestFromTail !== 0 || tail.length === 0) { + return newestFromTail } - return 0 + // Why: a single record longer than the tail window keeps its timestamp + // before the window starts; fall back to the full read in that rare case. + return parseNewestReflogEntryAt(await readTextFile(reflogPath)) } catch { return 0 } } +function parseNewestReflogEntryAt(reflog: string): number { + const lines = reflog.split('\n') + for (let index = lines.length - 1; index >= 0; index -= 1) { + // The trailing timezone + tab anchors the capture, so no digit-count floor is needed. + const seconds = /\s(\d{1,11})\s[-+]\d{4}\t/.exec(lines[index] ?? '')?.[1] + if (seconds) { + return Number(seconds) * 1000 + } + } + return 0 +} + async function readLocalWorktreeGitDir( worktreePath: string, gitPath: string, diff --git a/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts b/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts index 3cf8288fbc7..8bfbc586959 100644 --- a/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts +++ b/src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts @@ -177,6 +177,31 @@ describe('workspace cleanup snapshot IPC', () => { await expect(scan).rejects.toThrow('cancelled') }) + it('keeps legacy suggestion-only and full broad scans isolated across modes', async () => { + scanWorkspaceCleanupMock.mockImplementation((_store, _args, options) => { + return new Promise((resolve, reject) => { + options.signal?.addEventListener('abort', () => reject(new Error('cancelled')), { + once: true + }) + setTimeout(() => resolve({ scannedAt: NOW, candidates: [], errors: [] }), 5) + }) + }) + registerWorkspaceCleanupHandlers(makeEmptyStore()) + const handler = vi + .mocked(ipcMain.handle) + .mock.calls.find(([channel]) => channel === 'workspaceCleanup:scan')?.[1] + + // Why: different includeAllWorkspaces modes are separate lanes; one must + // not abort the other even from the same renderer. + const [legacy, full] = await Promise.allSettled([ + handler?.(makeScanEvent({ id: 7 }), { includeAllWorkspaces: false }) as Promise, + handler?.(makeScanEvent({ id: 7 }), { includeAllWorkspaces: true }) as Promise + ]) + + expect(legacy?.status).toBe('fulfilled') + expect(full?.status).toBe('fulfilled') + }) + it('supersedes a concurrent broad scan from the same renderer', async () => { const settlements: Promise[] = [] scanWorkspaceCleanupMock.mockImplementation((_store, _args, options) => { diff --git a/src/main/ipc/workspace-cleanup.ts b/src/main/ipc/workspace-cleanup.ts index 5c1ea19d217..e73dd27cc5e 100644 --- a/src/main/ipc/workspace-cleanup.ts +++ b/src/main/ipc/workspace-cleanup.ts @@ -37,7 +37,14 @@ type WorkspaceCleanupHandlerDeps = { // Why: module scope — handler re-registration on a new main window must not // orphan the previous window's controllers in a discarded map. const activeScans = new Map() -const broadScanControllersBySender = new Map() +// Keyed by sender AND scan mode: legacy suggestion-only and full-workspace +// broad scans are separate lanes and must not supersede each other, matching +// the renderer's broad-scan registry. +const broadScanControllersBySenderMode = new Map() + +function getBroadScanModeKey(senderId: number, args: WorkspaceCleanupScanArgs): string { + return `${senderId}\0${args.includeAllWorkspaces === true}` +} export function registerWorkspaceCleanupHandlers( store: Store, @@ -65,12 +72,13 @@ export function registerWorkspaceCleanupHandlers( activeScans.set(scanKey, controller) } const targeted = hasTargetedWorkspaceCleanupScan(scanArgs) + const broadScanKey = getBroadScanModeKey(sender.id, scanArgs) if (!targeted) { - // Why: two broad fleet scans from one renderer can only be a refresh - // race; running both doubles git subprocess and fs load for a result - // the renderer will discard. - broadScanControllersBySender.get(sender.id)?.abort() - broadScanControllersBySender.set(sender.id, controller) + // Why: two same-mode broad fleet scans from one renderer can only be a + // refresh race; running both doubles git subprocess and fs load for a + // result the renderer will discard. + broadScanControllersBySenderMode.get(broadScanKey)?.abort() + broadScanControllersBySenderMode.set(broadScanKey, controller) } // Why: a window close or reload must stop the fleet scan's git and fs // work, not merely mute its progress events. @@ -101,8 +109,8 @@ export function registerWorkspaceCleanupHandlers( if (scanKey && activeScans.get(scanKey) === controller) { activeScans.delete(scanKey) } - if (!targeted && broadScanControllersBySender.get(sender.id) === controller) { - broadScanControllersBySender.delete(sender.id) + if (!targeted && broadScanControllersBySenderMode.get(broadScanKey) === controller) { + broadScanControllersBySenderMode.delete(broadScanKey) } } } diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index ff9e150468a..0b8daf37d41 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -4329,8 +4329,10 @@ export const createWorktreeSlice: StateCreator await window.api.workspaceCleanup?.recordRemovalSnapshotPrune?.({ // Why: a single (unbatched) remote delete must still drop the row // from the local persisted snapshots or it resurrects from cache; - // an unknown batch id degrades to an immediate one-off prune. - batchId: options?.snapshotPruneBatchId ?? `single-removal:${worktreeId}`, + // an unknown batch id degrades to an immediate one-off prune. The + // id must stay bounded — main rejects batch ids over 128 chars, + // so it cannot embed the unbounded worktreeId. + batchId: options?.snapshotPruneBatchId ?? `single-removal:${crypto.randomUUID()}`, worktreeId, ...(hostId ? { executionHostId: hostId } : {}) })