From 3e16c074280be6a0678374420e96244a15894951 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:43:00 -0400 Subject: [PATCH 1/2] Fix stale lane worktree cleanup --- .../main/services/lanes/laneService.test.ts | 95 +++++- .../src/main/services/lanes/laneService.ts | 25 ++ .../services/lanes/worktreeResidualCleanup.ts | 308 ++++++++++++++++++ apps/desktop/src/main/services/state/kvDb.ts | 21 ++ docs/ARCHITECTURE.md | 3 +- docs/features/lanes/README.md | 10 +- docs/features/lanes/worktree-isolation.md | 32 +- .../sync-and-multi-device/crdt-model.md | 5 +- 8 files changed, 490 insertions(+), 9 deletions(-) create mode 100644 apps/desktop/src/main/services/lanes/worktreeResidualCleanup.ts diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index 17645b2fd..1c76b6a82 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -3166,7 +3166,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { projectRoot: repoRoot, projectId, defaultBaseRef: "main", - worktreesDir: path.join(repoRoot, "worktrees"), + worktreesDir: repoRoot, onDeleteEvent: (event) => opts.events.push(event), teardownDeps: { processService: opts.teardown.processService, @@ -3290,7 +3290,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { expect(last.progress.steps.find((s: any) => s.name === "git_worktree_remove")?.detail).toContain("recovered from stale state"); }); - it("deletes the lane row with a warning when only unregistered residual files remain", async () => { + it("deletes the lane row and records retryable cleanup when only unregistered residual files remain", async () => { const events: any[] = []; const fake = makeFakeServices(); const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); @@ -3331,6 +3331,8 @@ describe("laneService delete teardown + cancellation + streaming", () => { return ""; }); + await service.list({ includeStatus: false }); + try { await service.delete({ laneId: "lane-child", deleteBranch: false, force: true }); } finally { @@ -3339,11 +3341,100 @@ describe("laneService delete teardown + cancellation + streaming", () => { expect(db.get<{ id: string }>("select id from lanes where id = ?", ["lane-child"])).toBeNull(); expect(fs.existsSync(childPath)).toBe(true); + expect( + db.get<{ lane_id: string; worktree_path: string; attempts: number; last_error: string }>( + "select lane_id, worktree_path, attempts, last_error from local_worktree_residual_cleanups where project_id = ? and worktree_path = ?", + ["proj-delete", childPath], + ), + ).toMatchObject({ + lane_id: "lane-child", + worktree_path: childPath, + attempts: 0, + }); const last = events[events.length - 1]; expect(last.progress.overallStatus).toBe("completed_with_warnings"); const wtStep = last.progress.steps.find((s: any) => s.name === "git_worktree_remove"); expect(wtStep?.status).toBe("completed"); expect(wtStep?.detail).toContain("manual cleanup failed"); + + await service.list({ includeStatus: false }); + + expect(fs.existsSync(childPath)).toBe(false); + expect( + db.get<{ lane_id: string }>( + "select lane_id from local_worktree_residual_cleanups where project_id = ? and worktree_path = ?", + ["proj-delete", childPath], + ), + ).toBeNull(); + }); + + it("does not clean up non-git directories still referenced by archived lanes", async () => { + const events: any[] = []; + const fake = makeFakeServices(); + const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); + const archivedPath = path.join(repoRoot, "archived"); + fs.mkdirSync(archivedPath, { recursive: true }); + fs.writeFileSync(path.join(archivedPath, "keep.txt"), "archived lane files\n", "utf8"); + const now = "2026-03-11T12:00:00.000Z"; + db.run( + ` + insert into lanes( + id, project_id, name, description, lane_type, base_ref, branch_ref, worktree_path, + attached_root_path, is_edit_protected, parent_lane_id, color, icon, tags_json, status, created_at, archived_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + ["lane-archived", "proj-delete", "Archived", null, "worktree", "main", "feature/archived", archivedPath, null, 0, null, null, null, null, "archived", now, now], + ); + db.run( + ` + insert into local_worktree_residual_cleanups( + id, project_id, lane_id, branch_ref, worktree_path, reason, attempts, last_error, created_at, updated_at + ) values (?, ?, ?, ?, ?, 'delete_residual', 0, ?, ?, ?) + `, + ["cleanup-archived", "proj-delete", "lane-archived", "feature/archived", archivedPath, "stale cleanup debt", now, now], + ); + + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "list") return ""; + return ""; + }); + + await service.list({ includeStatus: false }); + + expect(fs.existsSync(path.join(archivedPath, "keep.txt"))).toBe(true); + expect( + db.get<{ lane_id: string }>( + "select lane_id from local_worktree_residual_cleanups where project_id = ? and worktree_path = ?", + ["proj-delete", archivedPath], + ), + ).not.toBeNull(); + }); + + it("cleans only empty untracked directories and leaves unknown non-empty directories alone", async () => { + const events: any[] = []; + const fake = makeFakeServices(); + const { service, repoRoot } = await setupWithLane({ teardown: fake, events }); + const emptyPath = path.join(repoRoot, "empty-residual"); + const youngEmptyPath = path.join(repoRoot, "young-empty-residual"); + const nonEmptyPath = path.join(repoRoot, "non-empty-residual"); + fs.mkdirSync(path.join(emptyPath, ".ade"), { recursive: true }); + fs.mkdirSync(path.join(youngEmptyPath, ".ade"), { recursive: true }); + fs.mkdirSync(nonEmptyPath, { recursive: true }); + fs.writeFileSync(path.join(nonEmptyPath, "user-file.txt"), "not ours\n", "utf8"); + const oldEnough = new Date(Date.now() - 15 * 60_000); + fs.utimesSync(path.join(emptyPath, ".ade"), oldEnough, oldEnough); + fs.utimesSync(emptyPath, oldEnough, oldEnough); + + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "list") return ""; + return ""; + }); + + await service.list({ includeStatus: false }); + + expect(fs.existsSync(emptyPath)).toBe(false); + expect(fs.existsSync(youngEmptyPath)).toBe(true); + expect(fs.existsSync(path.join(nonEmptyPath, "user-file.txt"))).toBe(true); }); it("keeps retained delete progress queryable for remounted renderers", async () => { diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index e8a5e61be..38f7719be 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -19,6 +19,7 @@ import { } from "../../../shared/laneLinearIssue"; import type { createOperationService } from "../history/operationService"; import type { Logger } from "../logging/logger"; +import { createWorktreeResidualCleanup } from "./worktreeResidualCleanup"; import type { AdoptAttachedLaneArgs, AttachLaneArgs, @@ -1821,6 +1822,17 @@ export function createLaneService({ return parseGitWorktreePorcelain(worktreeStdout(result)); }; + const residualWorktreeCleanup = createWorktreeResidualCleanup({ + db, + projectId, + projectRoot, + worktreesDir, + logger, + listGitWorktrees, + isPendingWorktreeCreation: (worktreePath) => pendingWorktreeCreationPaths.has(normAbs(worktreePath)), + removeWorktreeDirectory: removeWorktreeDirectoryWithRecovery, + }); + const findGitWorktreeForBranch = async (branchRef: string): Promise => { const normalizedBranch = normalizeBranchKey(branchRef); if (!normalizedBranch) return null; @@ -2346,6 +2358,11 @@ export function createLaneService({ } catch (err) { logger.warn("laneService.repairLegacyPrimaryBaseRootLanes_failed", { error: err instanceof Error ? err.message : String(err) }); } + try { + await residualWorktreeCleanup.retry(); + } catch (err) { + logger.warn("laneService.residualWorktreeCleanup_failed", { error: err instanceof Error ? err.message : String(err) }); + } try { await recoverManagedWorktreeRows(); } catch (err) { @@ -5119,9 +5136,16 @@ export function createLaneService({ if (await isStillRegisteredWorktree()) { throw new Error(fullMessage); } + residualWorktreeCleanup.recordFailure({ + laneId, + branchRef: row.branch_ref, + worktreePath: row.worktree_path, + error: fullMessage, + }); recordNonFatalFailure("git_worktree_remove", fullMessage); return { detail: `${detail}; warning: ${fullMessage}` }; } + residualWorktreeCleanup.deleteRow(row.worktree_path); const pruneFailure = await pruneWorktreesBestEffort(); if (pruneFailure) { const message = `git worktree prune failed: ${pruneFailure}`; @@ -5141,6 +5165,7 @@ export function createLaneService({ if (fs.existsSync(row.worktree_path)) { return removeResidualDirectory(`${row.worktree_path} (removed residual files)`); } + residualWorktreeCleanup.deleteRow(row.worktree_path); return { detail: row.worktree_path }; } // Recovery path: a previous failed delete (or this one's first attempt) diff --git a/apps/desktop/src/main/services/lanes/worktreeResidualCleanup.ts b/apps/desktop/src/main/services/lanes/worktreeResidualCleanup.ts new file mode 100644 index 000000000..b0b19de44 --- /dev/null +++ b/apps/desktop/src/main/services/lanes/worktreeResidualCleanup.ts @@ -0,0 +1,308 @@ +import fs from "node:fs"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; +import type { AdeDb } from "../state/kvDb"; +import type { Logger } from "../logging/logger"; + +type ResidualCleanupGitWorktreeInfo = { + path: string; + isBare: boolean; +}; + +type LocalWorktreeResidualCleanupRow = { + id: string; + project_id: string; + lane_id: string; + branch_ref: string | null; + worktree_path: string; + reason: "delete_residual"; + attempts: number; + last_error: string | null; + created_at: string; + updated_at: string; +}; + +type WorktreeResidualCleanupDeps = { + db: AdeDb; + projectId: string; + projectRoot: string; + worktreesDir: string; + logger: Logger; + listGitWorktrees: () => Promise; + isPendingWorktreeCreation: (worktreePath: string) => boolean; + removeWorktreeDirectory: (worktreePath: string) => Promise; +}; + +const RESIDUAL_WORKTREE_CLEANUP_SWEEP_TTL_MS = 60_000; +const RESIDUAL_WORKTREE_EMPTY_SCAN_ENTRY_LIMIT = 256; +const RESIDUAL_WORKTREE_ERROR_MAX_LENGTH = 2_000; +const UNKNOWN_EMPTY_WORKTREE_MIN_AGE_MS = 10 * 60_000; + +function normAbs(value: string): string { + return path.resolve(value); +} + +export function createWorktreeResidualCleanup({ + db, + projectId, + projectRoot, + worktreesDir, + logger, + listGitWorktrees, + isPendingWorktreeCreation, + removeWorktreeDirectory, +}: WorktreeResidualCleanupDeps) { + const normalizedProjectRoot = normAbs(projectRoot); + const normalizedWorktreesDir = normAbs(worktreesDir); + let lastSweepAt = 0; + let activeSweep: Promise | null = null; + + const isDirectManagedWorktreePath = (worktreePath: string): boolean => { + const normalized = normAbs(worktreePath); + return ( + normalized !== normalizedProjectRoot && + normalized !== normalizedWorktreesDir && + path.dirname(normalized) === normalizedWorktreesDir && + path.basename(normalized).trim().length > 0 + ); + }; + + const normalizeCleanupError = (error: unknown): string => { + const message = error instanceof Error ? error.message : String(error); + return message.length > RESIDUAL_WORKTREE_ERROR_MAX_LENGTH + ? `${message.slice(0, RESIDUAL_WORKTREE_ERROR_MAX_LENGTH)}...` + : message; + }; + + const listRows = (): LocalWorktreeResidualCleanupRow[] => + db.all( + ` + select * + from local_worktree_residual_cleanups + where project_id = ? + order by updated_at asc + `, + [projectId], + ); + + const deleteRow = (worktreePath: string): void => { + db.run( + "delete from local_worktree_residual_cleanups where project_id = ? and worktree_path = ?", + [projectId, normAbs(worktreePath)], + ); + }; + + const recordFailure = (args: { + laneId: string; + branchRef: string | null; + worktreePath: string; + error: unknown; + }): void => { + const worktreePath = normAbs(args.worktreePath); + if (!isDirectManagedWorktreePath(worktreePath)) { + logger.warn("lane.delete.residual_cleanup_not_recorded_unsafe_path", { + laneId: args.laneId, + worktreePath, + }); + return; + } + if (!fs.existsSync(worktreePath)) { + deleteRow(worktreePath); + return; + } + + const now = new Date().toISOString(); + db.run( + ` + insert into local_worktree_residual_cleanups( + id, project_id, lane_id, branch_ref, worktree_path, reason, attempts, last_error, created_at, updated_at + ) + values(?, ?, ?, ?, ?, 'delete_residual', 0, ?, ?, ?) + on conflict(project_id, worktree_path) do update set + lane_id = excluded.lane_id, + branch_ref = excluded.branch_ref, + reason = excluded.reason, + last_error = excluded.last_error, + updated_at = excluded.updated_at + `, + [ + randomUUID(), + projectId, + args.laneId, + args.branchRef, + worktreePath, + normalizeCleanupError(args.error), + now, + now, + ], + ); + lastSweepAt = 0; + }; + + const noteRetryFailure = (row: LocalWorktreeResidualCleanupRow, error: unknown): void => { + db.run( + ` + update local_worktree_residual_cleanups + set attempts = attempts + 1, + last_error = ?, + updated_at = ? + where project_id = ? and worktree_path = ? + `, + [normalizeCleanupError(error), new Date().toISOString(), projectId, normAbs(row.worktree_path)], + ); + }; + + const directoryHasFilesOrSymlinks = async ( + targetPath: string, + state: { visited: number } = { visited: 0 }, + ): Promise => { + if (state.visited > RESIDUAL_WORKTREE_EMPTY_SCAN_ENTRY_LIMIT) return true; + let entries: fs.Dirent[]; + try { + entries = await fs.promises.readdir(targetPath, { withFileTypes: true }); + } catch { + return true; + } + + for (const entry of entries) { + state.visited += 1; + if (state.visited > RESIDUAL_WORKTREE_EMPTY_SCAN_ENTRY_LIMIT) return true; + const childPath = path.join(targetPath, entry.name); + if (entry.isSymbolicLink()) return true; + if (!entry.isDirectory()) return true; + if (await directoryHasFilesOrSymlinks(childPath, state)) return true; + } + return false; + }; + + const isOldEnoughUnknownEmptyResidual = async (targetPath: string, nowMs: number): Promise => { + try { + const stat = await fs.promises.lstat(targetPath); + return nowMs - stat.mtimeMs >= UNKNOWN_EMPTY_WORKTREE_MIN_AGE_MS; + } catch { + return false; + } + }; + + const retry = async (options: { force?: boolean } = {}): Promise => { + const nowMs = Date.now(); + if (!options.force && nowMs - lastSweepAt < RESIDUAL_WORKTREE_CLEANUP_SWEEP_TTL_MS) { + return activeSweep ?? Promise.resolve(); + } + if (activeSweep) return activeSweep; + + activeSweep = (async () => { + lastSweepAt = Date.now(); + if (!fs.existsSync(normalizedWorktreesDir)) return; + + let gitWorktrees: ResidualCleanupGitWorktreeInfo[]; + try { + gitWorktrees = await listGitWorktrees(); + } catch (error) { + logger.warn("lane.residual_worktree_cleanup.git_worktree_list_failed", { + projectRoot, + error: error instanceof Error ? error.message : String(error), + }); + return; + } + + const gitWorktreePaths = new Set(gitWorktrees.filter((wt) => !wt.isBare).map((wt) => normAbs(wt.path))); + const laneWorktreePaths = new Set( + db.all<{ worktree_path: string }>( + "select worktree_path from lanes where project_id = ?", + [projectId], + ).map((row) => normAbs(row.worktree_path)), + ); + + const rowsByPath = new Map(); + for (const row of listRows()) { + const pathKey = normAbs(row.worktree_path); + if (!isDirectManagedWorktreePath(pathKey)) { + deleteRow(pathKey); + logger.warn("lane.residual_worktree_cleanup.dropped_unsafe_record", { + projectRoot, + laneId: row.lane_id, + worktreePath: pathKey, + }); + continue; + } + rowsByPath.set(pathKey, { ...row, worktree_path: pathKey }); + } + + const candidatePaths = new Set(rowsByPath.keys()); + let dirEntries: fs.Dirent[]; + try { + dirEntries = await fs.promises.readdir(normalizedWorktreesDir, { withFileTypes: true }); + } catch (error) { + logger.warn("lane.residual_worktree_cleanup.readdir_failed", { + projectRoot, + worktreesDir: normalizedWorktreesDir, + error: error instanceof Error ? error.message : String(error), + }); + return; + } + + const sweepStartedAt = Date.now(); + for (const entry of dirEntries) { + if (!entry.isDirectory()) continue; + const childPath = normAbs(path.join(normalizedWorktreesDir, entry.name)); + if (rowsByPath.has(childPath)) continue; + if (gitWorktreePaths.has(childPath) || laneWorktreePaths.has(childPath)) continue; + if (isPendingWorktreeCreation(childPath)) continue; + if ( + !(await directoryHasFilesOrSymlinks(childPath)) && + await isOldEnoughUnknownEmptyResidual(childPath, sweepStartedAt) + ) { + candidatePaths.add(childPath); + } + } + + for (const candidatePath of candidatePaths) { + const pathKey = normAbs(candidatePath); + const row = rowsByPath.get(pathKey); + if (!isDirectManagedWorktreePath(pathKey)) { + if (row) deleteRow(pathKey); + continue; + } + if (isPendingWorktreeCreation(pathKey)) continue; + if (gitWorktreePaths.has(pathKey) || laneWorktreePaths.has(pathKey)) continue; + if (!fs.existsSync(pathKey)) { + if (row) deleteRow(pathKey); + continue; + } + if (!row && (await directoryHasFilesOrSymlinks(pathKey))) continue; + + try { + await removeWorktreeDirectory(pathKey); + if (row) deleteRow(pathKey); + logger.info("lane.residual_worktree_cleanup.removed", { + projectRoot, + laneId: row?.lane_id ?? null, + worktreePath: pathKey, + source: row ? "delete_residual" : "empty_untracked_dir", + }); + } catch (error) { + if (row) { + noteRetryFailure(row, error); + } + logger.warn("lane.residual_worktree_cleanup.remove_failed", { + projectRoot, + laneId: row?.lane_id ?? null, + worktreePath: pathKey, + error: error instanceof Error ? error.message : String(error), + }); + } + } + })().finally(() => { + activeSweep = null; + }); + + return activeSweep; + }; + + return { + deleteRow, + recordFailure, + retry, + }; +} diff --git a/apps/desktop/src/main/services/state/kvDb.ts b/apps/desktop/src/main/services/state/kvDb.ts index d5dd8bdff..9fc655327 100644 --- a/apps/desktop/src/main/services/state/kvDb.ts +++ b/apps/desktop/src/main/services/state/kvDb.ts @@ -511,6 +511,7 @@ const LOCAL_ONLY_CRR_EXCLUDED_TABLES = new Set([ "runtime_processes", "stack_buttons", "test_suites", + "local_worktree_residual_cleanups", ]); function listEligibleCrrTables(db: DatabaseSyncType): string[] { @@ -2938,6 +2939,26 @@ function migrate(db: MigrationDb) { db.run("create index if not exists idx_review_suppressions_project on review_suppressions(project_id, created_at desc)"); db.run("create index if not exists idx_review_suppressions_repo on review_suppressions(project_id, repo_key)"); + // Machine-local cleanup debt for lane delete residual directories. Absolute + // worktree paths are local machine state and must not replicate to phones or + // peer desktops. + db.run(` + create table if not exists local_worktree_residual_cleanups ( + id text primary key, + project_id text not null, + lane_id text not null, + branch_ref text, + worktree_path text not null, + reason text not null default 'delete_residual', + attempts integer not null default 0, + last_error text, + created_at text not null, + updated_at text not null + ) + `); + db.run("create unique index if not exists idx_local_worktree_residual_cleanups_path on local_worktree_residual_cleanups(project_id, worktree_path)"); + db.run("create index if not exists idx_local_worktree_residual_cleanups_updated on local_worktree_residual_cleanups(project_id, updated_at)"); + // Machine-local runtime guard for PR automation. This table intentionally // has no PRIMARY KEY so cr-sqlite does not register it as a CRR table. db.run(` diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 22abc93ce..a9f2281d1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -235,6 +235,7 @@ Schema bootstrap in `kvDb.ts` creates ~103 tables. Anchor tables for agents read |-------|---------| | `projects` | One row per opened repo. Keyed by `root_path`. | | `lanes` | Worktree-backed units of work. Types: `primary`, `worktree`, `attached`. Supports parent/child stacks, run binding, color/icon/tags. | +| `local_worktree_residual_cleanups` | Machine-local lane-delete cleanup debt for residual managed worktree directories. Stores absolute paths and is excluded from CRR replication because only the runtime on that machine can safely retry removal. | | `terminal_sessions` | Tracked PTY sessions per lane with transcript path and head SHAs. The `chat_session_id` column (indexed) marks terminals owned by a chat (chat terminal drawer, App Control launch terminal); `ptyService` exposes them through the `ade.terminal.*` IPC and the `terminal` ADE action domain. The `owner_pid` column (indexed) identifies the ADE OS process that owns the live runtime for the row — cross-process reconcile/dispose paths check it before sweeping so concurrent surfaces don't mark each other's live sessions dead. See §3.5. | | `runtime_processes` | Machine-local process-liveness registry. Every ADE process (desktop main, brain process, TUI runtime) inserts a row on boot keyed by the process incarnation (`pid`, `started_at`) and refreshes `last_seen` on a 5 s heartbeat. The table is excluded from CRR replication because PIDs are only meaningful on the current OS; reconcile / dispose paths cross-reference `terminal_sessions.owner_pid` and `owner_process_started_at` against locally known and live rows to tell "row whose local owner crashed" from "row a sibling process is actively managing" without detaching sessions owned by another synced machine. See §3.5. | | `session_deltas` | Post-session diff stats + touched files + failure lines. Input to pack generation. | @@ -546,7 +547,7 @@ Most services described here live under `apps/desktop/src/main/services/ | `runtime/` | `tempCleanupService.ts`, `processRegistryService.ts`, `machineStateMigration.ts` | Runtime temp cleanup. `processRegistryService` is the per-process heartbeat registrar against machine-local `runtime_processes` (see §3.4); reconcile/dispose paths in `sessionService` and `ptyService` consult live and known owner sets before sweeping `terminal_sessions` rows so sibling processes and synced remote-machine owners are preserved. `machineStateMigration` carries one-shot migrations of the per-machine state files under `~/.ade/`. | | `sessions/` | `sessionService.ts`, `sessionDeltaService.ts` | Terminal session CRUD, post-session delta computation. | | `shared/` | `utils.ts`, `imageDimensions.ts`, `queueRebase.ts`, `packLegacyUtils.ts`, `transcriptInsights.ts` | Cross-domain utilities, including shared record guards and PNG/JPEG dimension parsing used by App Control and iOS Simulator capture paths. | -| `state/` | `kvDb.ts`, `crsqliteExtension.ts`, `globalState.ts`, `projectState.ts`, `onConflictAudit.ts` | SQLite schema + open, CRR extension loader, global state file, per-project state init. `globalState.upsertRecentProject` accepts `preserveRecentOrder` so reactivating an already-known project (by app focus, deep link, etc.) refreshes its `lastOpenedAt` in place instead of jumping it to the front of the recents list. Recent projects use stable keys: local rows are keyed by absolute root path, remote rows by `remote::`, so a remote path string never collides with a local project. Pinned rows are retained above normal recency ordering and survive beyond the cap. `model_picker_favorites` and `model_picker_recents` are per-project CRR tables shared by desktop, TUI, and iOS; they are primary-key-only so CRR can convert them, with the recents cap enforced in `modelPickerStore.ts`. `AdeDb.sync.discardUnpublishedChangesForTables(tableNames)` lets a service clear local CRR state for specific tables without leaking those clears to sync peers — it records the cleared tables and `through_db_version` in the local-only `local_crr_change_suppressions` table, and `exportChangesSince` filters local-site rows for those tables at or below that version on the way out. The local-only excluded set (still kept out of replication) includes that suppression table itself, the snapshot caches, `pr_auto_link_ignores`, `pull_request_ai_summaries`, and `runtime_processes`. `crsql_changes` DELETE statements run through a helper that swallows the read-only-table error the cr-sqlite extension raises when a CRR-managed table is wiped, with a `db.crr_changes_cleanup_skipped` warn log instead of failing the migration. | +| `state/` | `kvDb.ts`, `crsqliteExtension.ts`, `globalState.ts`, `projectState.ts`, `onConflictAudit.ts` | SQLite schema + open, CRR extension loader, global state file, per-project state init. `globalState.upsertRecentProject` accepts `preserveRecentOrder` so reactivating an already-known project (by app focus, deep link, etc.) refreshes its `lastOpenedAt` in place instead of jumping it to the front of the recents list. Recent projects use stable keys: local rows are keyed by absolute root path, remote rows by `remote::`, so a remote path string never collides with a local project. Pinned rows are retained above normal recency ordering and survive beyond the cap. `model_picker_favorites` and `model_picker_recents` are per-project CRR tables shared by desktop, TUI, and iOS; they are primary-key-only so CRR can convert them, with the recents cap enforced in `modelPickerStore.ts`. `AdeDb.sync.discardUnpublishedChangesForTables(tableNames)` lets a service clear local CRR state for specific tables without leaking those clears to sync peers — it records the cleared tables and `through_db_version` in the local-only `local_crr_change_suppressions` table, and `exportChangesSince` filters local-site rows for those tables at or below that version on the way out. The local-only excluded set (still kept out of replication) includes that suppression table itself, the snapshot caches, `local_worktree_residual_cleanups`, `pr_auto_link_ignores`, `pull_request_ai_summaries`, and `runtime_processes`. `crsql_changes` DELETE statements run through a helper that swallows the read-only-table error the cr-sqlite extension raises when a CRR-managed table is wiped, with a `db.crr_changes_cleanup_skipped` warn log instead of failing the migration. | | `sync/` | `syncService.ts`, `syncHostService.ts`, `syncPeerService.ts`, `syncRemoteCommandService.ts`, `syncProtocol.ts`, `deviceRegistryService.ts`, `syncPairingStore.ts` | **Thin delegation to the ADE runtime's sync service.** The authoritative sync service now lives in `apps/ade-cli/src/services/sync/`; the desktop main-process instances default to a non-host viewer role for legacy state and tests. The old in-process host is disabled unless `ADE_ENABLE_DESKTOP_SYNC_HOST=1` (diagnostics only). Wire formats — WebSocket envelope, remote command routing, device registry, pairing secrets — are the same across both implementations. Viewer joins clear the local `devices` + `sync_cluster_state` rows and then call `db.sync.discardUnpublishedChangesForTables(["devices", "sync_cluster_state"])` so the resulting DELETE rows do not leak back to other peers; the peer client follows up with `syncPeerService.acknowledgeLocalDbVersion()` to advance the outbound cursor past the suppressed range. | | `tests/` | `testService.ts` | Test-suite execution + run history. | | `updates/` | `autoUpdateService.ts` | Electron auto-update wrapper around `electron-updater`. Owns the renderer-visible `AutoUpdateSnapshot` (`idle \| checking \| downloading \| ready \| installing \| error`), uses `compareUpdateVersions` (SemVer-aware) to dedupe / supersede staged installers and to reconcile `pendingInstallUpdate` against the running version on next boot. Packaged builds schedule startup/periodic checks; source/dev launches construct the service without auto-check timers so missing `app-update.yml` never surfaces as a renderer error. `quitAndInstall()` is async: it re-runs `checkForUpdates({ allowReady: true })` to confirm the staged build is still latest, and only then flips to `installing` and calls `updater.quitAndInstall(false, true)`. | diff --git a/docs/features/lanes/README.md b/docs/features/lanes/README.md index f0f58ace9..e1864b21c 100644 --- a/docs/features/lanes/README.md +++ b/docs/features/lanes/README.md @@ -56,7 +56,8 @@ Desktop fallback services (`apps/desktop/src/main/services/lanes/`): | File | Responsibility | |------|---------------| -| `laneService.ts` | Lane CRUD, worktree creation/removal, status computation, stack chain traversal, rebase runs, reparent, startup repair routines, branch switching, lane + session Linear issue linkage, and the multi-step lane teardown pipeline (`getDeleteRisk`, `delete`, `cancelDelete`) that streams `LaneDeleteProgress` events as it stops processes/PTYs/watchers, cancels auto-rebase, runs `git worktree remove` / `git branch -D` / optional `git push --delete origin`, verifies residual worktree files are gone before DB cleanup, and cleans the pack directory + DB rows. Lane creation is wrapped so that any failure after the worktree is on disk routes through `cleanupCreatedWorktreeLaneAfterCreateFailure`, which removes the orphaned checkout rather than leaving a worktree no lane row references. Independent deletes can progress through teardown concurrently; only the `git_worktree_remove` step enters the shared worktree-mutation guard, so lane creation is not held behind unrelated stop/cleanup steps but still avoids concurrent edits to Git's worktree registry. Deletes run to completion once started, so `cancelDelete` reports that no active delete can be cancelled. `getSummary(laneId, { includeStatus })` is the scoped summary path used by mobile detail commands so opening a lane does not rebuild the full lane list; `refreshSnapshots` honors `includeStatus` for light runtime-bucket refreshes. `reparent` accepts an optional `stackBaseBranchRef` to pick a specific branch to stack onto (resolved in the project repo with `origin/` preferred); when both the parent link and the resolved base branch are unchanged the call short-circuits without touching git. Branch switching rolls git checkout back to the previous branch when the database update fails. **Linear issue linkage:** `linkLinearIssues` / `unlinkLinearIssues` manage lane-scoped links in `lane_linear_issue_links` (never touching the primary `lane_linear_issues` row); `attachLinearIssueToSession` / `detachLinearIssueFromSession` / `listLinearIssuesForSession` / `listLinearIssuesForLaneSessions` manage session-scoped links in `session_linear_issues`. `attachLinearIssueToSession` resolves the session's lane from `claude_sessions` / `terminal_sessions` and mirrors each issue into the lane's `chat_attach` links when a lane exists, without ever promoting the lane's primary issue. See [Linear integration](../linear-integration/README.md#session-scoped-issue-attachment-and-cli-context-injection). | +| `laneService.ts` | Lane CRUD, worktree creation/removal, status computation, stack chain traversal, rebase runs, reparent, startup repair routines, branch switching, lane + session Linear issue linkage, and the multi-step lane teardown pipeline (`getDeleteRisk`, `delete`, `cancelDelete`) that streams `LaneDeleteProgress` events as it stops processes/PTYs/watchers, cancels auto-rebase, runs `git worktree remove` / `git branch -D` / optional `git push --delete origin`, verifies residual worktree files are gone before DB cleanup, records retryable residual-cleanup debt when manual deletion fails, and cleans the pack directory + DB rows. Lane creation is wrapped so that any failure after the worktree is on disk routes through `cleanupCreatedWorktreeLaneAfterCreateFailure`, which removes the orphaned checkout rather than leaving a worktree no lane row references. Independent deletes can progress through teardown concurrently; only the `git_worktree_remove` step enters the shared worktree-mutation guard, so lane creation is not held behind unrelated stop/cleanup steps but still avoids concurrent edits to Git's worktree registry. Deletes run to completion once started, so `cancelDelete` reports that no active delete can be cancelled. `list()` also runs the residual-worktree cleanup retry sweep before duplicate/stale worktree repair so previous delete warnings can self-heal without blocking lane row cleanup. `getSummary(laneId, { includeStatus })` is the scoped summary path used by mobile detail commands so opening a lane does not rebuild the full lane list; `refreshSnapshots` honors `includeStatus` for light runtime-bucket refreshes. `reparent` accepts an optional `stackBaseBranchRef` to pick a specific branch to stack onto (resolved in the project repo with `origin/` preferred); when both the parent link and the resolved base branch are unchanged the call short-circuits without touching git. Branch switching rolls git checkout back to the previous branch when the database update fails. **Linear issue linkage:** `linkLinearIssues` / `unlinkLinearIssues` manage lane-scoped links in `lane_linear_issue_links` (never touching the primary `lane_linear_issues` row); `attachLinearIssueToSession` / `detachLinearIssueFromSession` / `listLinearIssuesForSession` / `listLinearIssuesForLaneSessions` manage session-scoped links in `session_linear_issues`. `attachLinearIssueToSession` resolves the session's lane from `claude_sessions` / `terminal_sessions` and mirrors each issue into the lane's `chat_attach` links when a lane exists, without ever promoting the lane's primary issue. See [Linear integration](../linear-integration/README.md#session-scoped-issue-attachment-and-cli-context-injection). | +| `worktreeResidualCleanup.ts` | Machine-local retry worker for managed worktree directories that survive lane deletion. It stores cleanup debt in `local_worktree_residual_cleanups`, retries during `laneService.list()`, drops unsafe records, skips registered Git worktrees, active lane paths, and pending creations, removes old empty untracked directories under the managed worktrees directory, and leaves unknown non-empty directories alone unless they were explicitly recorded from the delete path. | | `autoRebaseService.ts` | Auto-rebase worker for stacked lanes, attention state, head-change handlers. Consults `resolvePrRebaseMode` to determine whether a lane with a linked PR should auto-rebase (`pr_target` strategy) or only surface manual attention (`lane_base` strategy). `listStatuses({ includeAll: true })` returns stored statuses without recomputing lane git status for PR workflow views. | | `rebaseSuggestionService.ts` | Emits rebase suggestions when a parent lane advances, dismiss/defer lifecycle. Each suggestion may include up to 20 `RebaseTargetCommit` entries showing the behind commits the rebase would pull in. | | `laneEnvironmentService.ts` | Environment init pipeline: env files, docker services, dependencies, mount points, copy paths (Phase 5 W1) | @@ -359,8 +360,11 @@ a lane parented to primary would always show zero behind. filesystem cleanup uses `fs.promises.rm` instead of synchronous `rmSync`, and `git_worktree_remove` checks the managed worktree path after a successful git removal so residual files are removed and - `git worktree prune` runs before the lane row disappears. Multiple - delete calls can progress through non-Git teardown independently; + `git worktree prune` runs before the lane row disappears. If an + unregistered residual directory cannot be removed, the delete still + completes with a warning and records a local retry row so the next + lane-list sweep can remove the directory once the filesystem allows + it. Multiple delete calls can progress through non-Git teardown independently; the shared worktree-mutation guard is held only while `git_worktree_remove` mutates Git's worktree registry, so lane creation can start while another lane is still stopping processes, diff --git a/docs/features/lanes/worktree-isolation.md b/docs/features/lanes/worktree-isolation.md index 96ca8927c..ac89c3263 100644 --- a/docs/features/lanes/worktree-isolation.md +++ b/docs/features/lanes/worktree-isolation.md @@ -100,7 +100,10 @@ the directory first: is no longer registered and manual cleanup or prune fails, the lane delete can complete with warnings so the stale row and lane-owned metadata are still removed; the warning tells the user what residual - directory or registry cleanup remains manual. If attached: skip. + directory or registry cleanup could not be completed immediately. + Failed residual-directory cleanup is recorded in the machine-local + `local_worktree_residual_cleanups` table so later `lanes.list` calls + can retry it. If attached: skip. 6. If caller requested `deleteBranch`: `git branch -D `. Optional remote branch cleanup uses `git push --delete ` and is non-fatal. @@ -125,6 +128,33 @@ the saved path, ADE returns the default clean lane status and avoids probing `git status`, branch detection, stashes, or change inspection from the wrong checkout. +## Residual cleanup retry sweep + +`worktreeResidualCleanup.ts` is the safety net for managed worktree +directories that survive a delete after the lane row and lane-owned +metadata are gone. The cleanup debt is local machine state, not project +state: `local_worktree_residual_cleanups` stores absolute paths, is +excluded from CRR replication, and is only interpreted by the runtime +that owns those paths. + +The sweep runs from `laneService.list()` with a short TTL so normal lane +refreshes can clear previous warnings without a dedicated user action. +Before removing anything it rebuilds three guard sets: + +- registered non-bare paths from `git worktree list --porcelain` +- `lanes.worktree_path` values still present for the current project, + including archived lanes +- paths currently being created by an in-flight lane create + +Only direct children of the managed `.ade/worktrees/` directory are +eligible. Unsafe records are dropped, registered Git worktrees and +active lane paths are skipped, and pending creations are left alone. +Recorded delete failures are retried until they disappear or the row is +cleared. Unknown directories under `.ade/worktrees/` are removed only +when they are empty, contain no files or symlinks, and are old enough +to avoid racing a create; unknown non-empty directories are treated as +user data and left in place. + ## Per-lane state directories Lanes store lane-local artifacts under a few conventions: diff --git a/docs/features/sync-and-multi-device/crdt-model.md b/docs/features/sync-and-multi-device/crdt-model.md index c50b96a91..65d250ce8 100644 --- a/docs/features/sync-and-multi-device/crdt-model.md +++ b/docs/features/sync-and-multi-device/crdt-model.md @@ -261,8 +261,9 @@ should live outside `.ade/ade.db` or be designed so the host owns all writes and controllers only read. The local-only excluded set is enumerated in `kvDb.ts`'s `LOCAL_ONLY_CRR_EXCLUDED_TABLES` and includes `lane_detail_snapshots`, `lane_list_snapshots`, -`local_crr_change_suppressions`, `pr_auto_link_ignores`, -`pull_request_ai_summaries`, and `runtime_processes`. +`local_crr_change_suppressions`, `local_worktree_residual_cleanups`, +`pr_auto_link_ignores`, `pull_request_ai_summaries`, and +`runtime_processes`. ### Local clears that must not propagate From 692b7525a41c5762d9a95f66c915f8022766fcba Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:10:50 -0400 Subject: [PATCH 2/2] ship: iteration 1 - address review fixture --- .../main/services/lanes/laneService.test.ts | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index 1c76b6a82..057e2c403 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -3155,18 +3155,21 @@ describe("laneService delete teardown + cancellation + streaming", () => { async function setupWithLane(opts: { teardown: ReturnType; events: any[]; createWorktree?: boolean }) { const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-delete-")); + const worktreesDir = path.join(repoRoot, "worktrees"); const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); const projectId = "proj-delete"; await seedProjectAndStack(db, { projectId, repoRoot }); + db.run("update lanes set worktree_path = ? where id = ?", [path.join(worktreesDir, "parent"), "lane-parent"]); + db.run("update lanes set worktree_path = ? where id = ?", [path.join(worktreesDir, "child"), "lane-child"]); // Materialize the lane-child worktree dir so the delete flow exercises git_worktree_remove. - const childPath = path.join(repoRoot, "child"); + const childPath = path.join(worktreesDir, "child"); if (opts.createWorktree !== false) fs.mkdirSync(childPath, { recursive: true }); const service = createLaneService({ db, projectRoot: repoRoot, projectId, defaultBaseRef: "main", - worktreesDir: repoRoot, + worktreesDir, onDeleteEvent: (event) => opts.events.push(event), teardownDeps: { processService: opts.teardown.processService, @@ -3178,7 +3181,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { }, }); // First call (lane-child) has no children rows after the seed; we delete lane-child. - return { db, service, repoRoot }; + return { db, service, repoRoot, worktreesDir, childPath }; } it("runs teardown steps before git_worktree_remove and broadcasts per-step progress", async () => { @@ -3228,8 +3231,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("removes residual worktree files before deleting the lane row", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); - const childPath = path.join(repoRoot, "child"); + const { service, db, childPath } = await setupWithLane({ teardown: fake, events }); fs.writeFileSync(path.join(childPath, "residual.log"), "left behind by git\n", "utf8"); vi.mocked(runGit).mockImplementation(async (args: string[]) => { @@ -3257,8 +3259,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("recovers stale worktree directories with unreadable residual folders", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, repoRoot } = await setupWithLane({ teardown: fake, events }); - const childPath = path.join(repoRoot, "child"); + const { service, childPath } = await setupWithLane({ teardown: fake, events }); const guestTrashPath = path.join(childPath, ".Trashes"); fs.mkdirSync(guestTrashPath, { recursive: true }); fs.chmodSync(guestTrashPath, 0o311); @@ -3293,8 +3294,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("deletes the lane row and records retryable cleanup when only unregistered residual files remain", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); - const childPath = path.join(repoRoot, "child"); + const { service, db, repoRoot, childPath } = await setupWithLane({ teardown: fake, events }); fs.writeFileSync(path.join(childPath, "residual.log"), "left behind by git\n", "utf8"); const realRm = fs.promises.rm.bind(fs.promises); const rmSpy = vi.spyOn(fs.promises, "rm").mockImplementation(async (target: fs.PathLike, options?: Parameters[1]) => { @@ -3371,8 +3371,8 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("does not clean up non-git directories still referenced by archived lanes", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); - const archivedPath = path.join(repoRoot, "archived"); + const { service, db, worktreesDir } = await setupWithLane({ teardown: fake, events }); + const archivedPath = path.join(worktreesDir, "archived"); fs.mkdirSync(archivedPath, { recursive: true }); fs.writeFileSync(path.join(archivedPath, "keep.txt"), "archived lane files\n", "utf8"); const now = "2026-03-11T12:00:00.000Z"; @@ -3413,10 +3413,10 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("cleans only empty untracked directories and leaves unknown non-empty directories alone", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, repoRoot } = await setupWithLane({ teardown: fake, events }); - const emptyPath = path.join(repoRoot, "empty-residual"); - const youngEmptyPath = path.join(repoRoot, "young-empty-residual"); - const nonEmptyPath = path.join(repoRoot, "non-empty-residual"); + const { service, worktreesDir } = await setupWithLane({ teardown: fake, events }); + const emptyPath = path.join(worktreesDir, "empty-residual"); + const youngEmptyPath = path.join(worktreesDir, "young-empty-residual"); + const nonEmptyPath = path.join(worktreesDir, "non-empty-residual"); fs.mkdirSync(path.join(emptyPath, ".ade"), { recursive: true }); fs.mkdirSync(path.join(youngEmptyPath, ".ade"), { recursive: true }); fs.mkdirSync(nonEmptyPath, { recursive: true }); @@ -3507,9 +3507,9 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("runs independent lane delete teardown concurrently", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); + const { service, db, worktreesDir } = await setupWithLane({ teardown: fake, events }); const now = "2026-03-11T12:00:00.000Z"; - const siblingPath = path.join(repoRoot, "sibling"); + const siblingPath = path.join(worktreesDir, "sibling"); fs.mkdirSync(siblingPath, { recursive: true }); db.run( ` @@ -3756,7 +3756,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { it("cleans lane-owned database state when deleting a lane", async () => { const events: any[] = []; const fake = makeFakeServices(); - const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events, createWorktree: false }); + const { service, db, repoRoot, childPath } = await setupWithLane({ teardown: fake, events, createWorktree: false }); const projectId = "proj-delete"; const now = "2026-03-11T12:30:00.000Z"; @@ -3789,7 +3789,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { ); db.run( "insert into files_workspaces(id, kind, lane_id, name, root_path, updated_at) values (?, ?, ?, ?, ?, ?)", - ["workspace-child", "lane", "lane-child", "Child", path.join(repoRoot, "child"), now], + ["workspace-child", "lane", "lane-child", "Child", childPath, now], ); db.run( "insert into file_directory_snapshots(workspace_id, parent_path, include_hidden, nodes_json, updated_at) values (?, ?, ?, ?, ?)", @@ -3866,7 +3866,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { created_at, heartbeat_at, expires_at ) values (?, ?, ?, ?, ?, ?, ?, ?, ?) `, - ["child-key", path.join(repoRoot, "child"), "lane-child", "pr", "PR #1", "token", now, now, now], + ["child-key", childPath, "lane-child", "pr", "PR #1", "token", now, now, now], ); db.run(