diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index d22f2fbff..ccd42b754 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -679,6 +679,121 @@ describe("laneService create", () => { } }); + it("does not let a concurrent list adopt a half-created worktree as a duplicate lane", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-create-race-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const now = "2026-03-11T12:00:00.000Z"; + + // Hoisted so the finally can always release the gate and drain the create, + // even if an assertion throws mid-test. + let releaseWorktreeAdd: () => void = () => {}; + let createPromise: Promise = Promise.resolve(); + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + ["proj-create-race", repoRoot, "demo", "main", now, now], + ); + 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-main", "proj-create-race", "Main", null, "primary", "main", "main", repoRoot, null, 1, null, null, null, null, "active", now, null], + ); + + // `git worktree list` reports a new worktree the moment `worktree add` + // registers it, before checkout completes. Park the add on a gate so a + // list() can run inside that window. + let pendingWorktree: { path: string; branch: string } | null = null; + const worktreeAddGate = new Promise((resolve) => { + releaseWorktreeAdd = resolve; + }); + + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "add") { + pendingWorktree = { branch: args[3], path: args[4] }; + await worktreeAddGate; + return { exitCode: 0, stdout: "", stderr: "" } as any; + } + if (args[0] === "worktree" && args[1] === "list") { + const blocks = [`worktree ${repoRoot}\nbranch refs/heads/main`]; + if (pendingWorktree) { + blocks.push(`worktree ${pendingWorktree.path}\nbranch refs/heads/${pendingWorktree.branch}`); + } + return { exitCode: 0, stdout: `${blocks.join("\n\n")}\n`, stderr: "" } as any; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + const laneBranchGitStub = defaultLaneBranchGitStub(args); + if (laneBranchGitStub) return laneBranchGitStub; + if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "HEAD") { + return { exitCode: 0, stdout: "main\n", stderr: "" }; + } + if (args[0] === "rev-parse" && args[1] === "main") { + return { exitCode: 0, stdout: "sha-main\n", stderr: "" }; + } + if (args[0] === "push" && args[1] === "-u") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "rev-list" && args[1] === "--left-right" && args[2] === "--count") { + return { exitCode: 0, stdout: "0\t0\n", stderr: "" }; + } + if ( + args[0] === "rev-parse" + && args[1] === "--abbrev-ref" + && args[2] === "--symbolic-full-name" + && args[3] === "@{upstream}" + ) { + return { exitCode: 1, stdout: "", stderr: "fatal: no upstream configured" }; + } + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--git-dir") { + return { exitCode: 1, stdout: "", stderr: "fatal: no git dir" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-create-race", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + + createPromise = service.create({ name: "Race lane", baseBranch: "main" }); + await vi.waitFor(() => { + expect(pendingWorktree).not.toBeNull(); + }); + + // List while the create's checkout is still running: recovery must not + // adopt the pending worktree. + const lanesDuringCreate = await service.list({ includeStatus: false }); + expect(lanesDuringCreate.some((lane) => lane.worktreePath === pendingWorktree!.path)).toBe(false); + + releaseWorktreeAdd(); + const lane = (await createPromise) as { id: string; worktreePath: string }; + + const lanesAfterCreate = await service.list({ includeStatus: false }); + const lanesOnPath = lanesAfterCreate.filter((entry) => entry.worktreePath === lane.worktreePath); + expect(lanesOnPath.map((entry) => entry.id)).toEqual([lane.id]); + } finally { + // Always release the gate and drain the create so an assertion failure + // before line 777 cannot leave `createPromise` blocked and hang CI. + releaseWorktreeAdd(); + await Promise.resolve(createPromise).catch(() => undefined); + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); + it("cleans up the row, worktree, and branch when VM lane wiring fails", async () => { const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-create-vm-fail-")); const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); @@ -770,6 +885,119 @@ describe("laneService create", () => { } }); + it("preserves absorbed raced lane data when VM lane wiring fails after commit", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-create-vm-absorb-fail-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const now = "2026-03-11T12:00:00.000Z"; + const projectId = "proj-create-vm-absorb-fail"; + const racedLaneId = "raced-adoption-row"; + const worktreesDir = path.join(repoRoot, "worktrees"); + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + [projectId, repoRoot, "demo", "main", now, now], + ); + 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-main", projectId, "Main", null, "primary", "main", "main", repoRoot, null, 0, null, null, null, null, "active", now, null], + ); + + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "add") { + const branchRef = args[3]; + const worktreePath = args[4]; + 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 (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + [racedLaneId, projectId, "Recovered race", null, "worktree", "main", branchRef, worktreePath, null, 0, null, null, null, null, "active", now, null], + ); + db.run( + ` + insert into terminal_sessions(id, lane_id, title, started_at, transcript_path, status) + values (?, ?, ?, ?, ?, ?) + `, + ["session-on-raced-row", racedLaneId, "shell", now, "/tmp/transcript.jsonl", "running"], + ); + db.run( + ` + insert into lane_linear_issue_links( + id, project_id, lane_id, issue_id, issue_json, role, source, created_at, updated_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + ["link-on-raced-row", projectId, racedLaneId, "ISS-101", "{}", "worked", "chat_attach", now, now], + ); + return { exitCode: 0, stdout: "", stderr: "" } as any; + } + if (args[0] === "worktree" && args[1] === "remove") { + return { exitCode: 0, stdout: "", stderr: "" } as any; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + const laneBranchGitStub = defaultLaneBranchGitStub(args); + if (laneBranchGitStub) return laneBranchGitStub; + if (args[0] === "rev-parse" && args[1] === "main") { + return { exitCode: 0, stdout: "sha-main\n", stderr: "" }; + } + if (args[0] === "branch" && args[1] === "-D") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId, + defaultBaseRef: "main", + worktreesDir, + macosVmHooks: { + linkLaneToCurrentVm: vi.fn(async () => { + throw new Error("vm link failed"); + }), + } as any, + }); + + await expect( + service.create({ + name: "VM absorbed lane", + baseBranch: "main", + runtimePlacement: "macos-vm", + }), + ).rejects.toThrow("vm link failed"); + + const worktreeRows = db.all<{ id: string; runtime_placement: string }>( + "select id, runtime_placement from lanes where project_id = ? and lane_type = 'worktree'", + [projectId], + ); + expect(worktreeRows).toHaveLength(1); + expect(worktreeRows[0]?.id).not.toBe(racedLaneId); + expect(worktreeRows[0]?.runtime_placement).toBe("local"); + expect( + db.get<{ lane_id: string }>("select lane_id from terminal_sessions where id = ?", ["session-on-raced-row"])?.lane_id, + ).toBe(worktreeRows[0]?.id); + expect( + db.get<{ lane_id: string }>("select lane_id from lane_linear_issue_links where id = ?", ["link-on-raced-row"])?.lane_id, + ).toBe(worktreeRows[0]?.id); + expect(vi.mocked(runGitOrThrow).mock.calls.some(([args]) => args[0] === "worktree" && args[1] === "remove")).toBe(false); + expect(vi.mocked(runGit).mock.calls.some(([args]) => args[0] === "branch" && args[1] === "-D")).toBe(false); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); + it("does not emit placement changed when re-attaching a lane that is already on the VM", async () => { const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-reattach-vm-")); const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); @@ -1136,6 +1364,126 @@ describe("laneService list repairs", () => { fs.rmSync(repoRoot, { recursive: true, force: true }); } }); + + it("dedupes duplicate lane rows sharing a managed worktree, keeping the creator row", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-repair-dup-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const worktreesDir = path.join(repoRoot, "worktrees"); + const sharedWorktreePath = path.join(worktreesDir, "claude-pre-launch-review-ba241a46"); + // The creator row's id matches the suffix embedded in the worktree dir name. + const keeperId = "ba241a46-0000-4000-8000-000000000001"; + // The adoption artifact raced in first, so created_at alone would keep the wrong row. + const artifactId = "f2862e51-0000-4000-8000-000000000002"; + + try { + db.run( + "insert into projects(id, root_path, display_name, default_base_ref, created_at, last_opened_at) values (?, ?, ?, ?, ?, ?)", + ["proj-repair-dup", repoRoot, "demo", "main", "2026-06-11T17:41:00.000Z", "2026-06-11T17:41:00.000Z"], + ); + const insertLane = ` + 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 (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `; + db.run(insertLane, ["lane-main", "proj-repair-dup", "Main", null, "primary", "main", "main", repoRoot, null, 1, null, null, null, null, "active", "2026-06-11T17:41:00.000Z", null]); + db.run(insertLane, [artifactId, "proj-repair-dup", "claude pre launch review", null, "worktree", "main", "ade/claude-pre-launch-review-ba241a46", sharedWorktreePath, null, 0, null, null, null, null, "active", "2026-06-11T17:41:33.000Z", null]); + db.run(insertLane, [keeperId, "proj-repair-dup", "claude pre-launch review", null, "worktree", "main", "ade/claude-pre-launch-review-ba241a46", sharedWorktreePath, null, 0, null, null, null, null, "active", "2026-06-11T17:41:33.933Z", null]); + // Work that landed on the duplicate must survive the dedupe. + db.run(insertLane, ["lane-dup-child", "proj-repair-dup", "Child of duplicate", null, "worktree", "ade/claude-pre-launch-review-ba241a46", "ade/dup-child", path.join(worktreesDir, "dup-child-11112222"), null, 0, artifactId, null, null, null, "active", "2026-06-11T18:00:00.000Z", null]); + db.run( + ` + insert into terminal_sessions(id, lane_id, title, started_at, transcript_path, status) + values (?, ?, ?, ?, ?, ?) + `, + ["session-on-dup", artifactId, "shell", "2026-06-11T17:50:00.000Z", "/tmp/transcript.jsonl", "running"], + ); + // A Linear issue link the user attached while the duplicate (artifact) + // row was the one surfaced in the Lanes tab must survive onto the keeper. + // `referenced` is a different role than the keeper's `worked` for the same + // issue, so it must be preserved (role-aware dedupe), while a same-role + // collision is dropped. + const insertLink = ` + insert into lane_linear_issue_links( + id, project_id, lane_id, issue_id, issue_json, role, source, created_at, updated_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?) + `; + db.run(insertLink, ["link-keeper-worked", "proj-repair-dup", keeperId, "ISS-99", "{}", "worked", "chat_attach", "2026-06-11T17:54:00.000Z", "2026-06-11T17:54:00.000Z"]); + db.run(insertLink, ["link-dup-referenced", "proj-repair-dup", artifactId, "ISS-99", "{}", "referenced", "chat_attach", "2026-06-11T17:55:00.000Z", "2026-06-11T17:55:00.000Z"]); + db.run(insertLink, ["link-dup-worked-dupe", "proj-repair-dup", artifactId, "ISS-99", "{}", "worked", "chat_attach", "2026-06-11T17:56:00.000Z", "2026-06-11T17:56:00.000Z"]); + // A session-scoped Linear issue attached directly to the duplicate lane + // must move to the keeper, not be cascade-deleted. + db.run( + ` + insert into session_linear_issues( + id, project_id, session_id, lane_id, issue_id, issue_json, role, source, created_at, updated_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + ["sess-link-on-dup", "proj-repair-dup", "session-on-dup", artifactId, "ISS-77", "{}", "worked", "chat_attach", "2026-06-11T17:57:00.000Z", "2026-06-11T17:57:00.000Z"], + ); + db.run( + ` + insert into lane_branch_profiles( + id, project_id, lane_id, branch_ref, normalized_branch_ref, base_ref, + parent_lane_id, source_branch_ref, created_at, updated_at, last_checked_out_at + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + ["profile-on-dup", "proj-repair-dup", artifactId, "ade/dup-switched", "ade/dup-switched", "main", null, null, "2026-06-11T17:58:00.000Z", "2026-06-11T17:58:00.000Z", "2026-06-11T17:58:00.000Z"], + ); + + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "list") { + return { exitCode: 0, stdout: "", stderr: "" } as any; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + const laneBranchGitStub = defaultLaneBranchGitStub(args); + if (laneBranchGitStub) return laneBranchGitStub; + if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "HEAD") { + return { exitCode: 0, stdout: "main\n", stderr: "" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-repair-dup", + defaultBaseRef: "main", + worktreesDir, + }); + + const lanes = await service.list({ includeStatus: false }); + + const sharedPathLanes = lanes.filter((lane) => lane.worktreePath === sharedWorktreePath); + expect(sharedPathLanes.map((lane) => lane.id)).toEqual([keeperId]); + expect(lanes.some((lane) => lane.id === artifactId)).toBe(false); + expect(lanes.find((lane) => lane.id === "lane-dup-child")?.parentLaneId).toBe(keeperId); + expect( + db.get<{ lane_id: string }>("select lane_id from terminal_sessions where id = ?", ["session-on-dup"])?.lane_id, + ).toBe(keeperId); + // The differently-roled link survives on the keeper; the same-role + // collision is dropped (keeper's own `worked` wins). The keeper ends up + // with exactly one `worked` and one `referenced` row for the issue. + const keeperLinks = db.all<{ id: string; role: string }>( + "select id, role from lane_linear_issue_links where lane_id = ? and issue_id = ? order by role", + [keeperId, "ISS-99"], + ); + expect(keeperLinks.map((r) => r.role)).toEqual(["referenced", "worked"]); + expect(db.get<{ id: string }>("select id from lane_linear_issue_links where id = ?", ["link-dup-worked-dupe"])).toBeNull(); + // The session-scoped Linear issue moved to the keeper instead of being deleted. + expect( + db.get<{ lane_id: string }>("select lane_id from session_linear_issues where id = ?", ["sess-link-on-dup"])?.lane_id, + ).toBe(keeperId); + expect( + db.get<{ lane_id: string }>("select lane_id from lane_branch_profiles where id = ?", ["profile-on-dup"])?.lane_id, + ).toBe(keeperId); + } finally { + db.close(); + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); }); describe("laneService importBranch", () => { @@ -1474,6 +1822,87 @@ describe("laneService importBranch", () => { expect(vi.mocked(runGitOrThrow).mock.calls.some(([args]) => args[0] === "branch" && args[1] === "--track")).toBe(false); }); + it("absorbs raced recovered rows while importing a branch", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-import-race-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + const projectId = "proj-import-race"; + const racedLaneId = "raced-import-row"; + const now = "2026-03-11T12:05:00.000Z"; + await seedProjectAndStack(db, { projectId, repoRoot }); + + vi.mocked(runGit).mockImplementation(async (args: string[]) => { + const laneBranchGitStub = defaultLaneBranchGitStub(args); + if (laneBranchGitStub) return laneBranchGitStub; + if (args[0] === "show-ref" && args[1] === "--verify" && args[3] === "refs/heads/feature/import-race") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "status" && args[1] === "--porcelain=v1") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args[0] === "rev-list" && args[1] === "--left-right" && args[2] === "--count") { + return { exitCode: 0, stdout: "0\t0\n", stderr: "" }; + } + if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "--symbolic-full-name" && args[3] === "@{upstream}") { + return { exitCode: 0, stdout: "origin/feature/import-race\n", stderr: "" }; + } + if (args[0] === "rev-list" && args[1] === "HEAD..@{upstream}" && args[2] === "--count") { + return { exitCode: 0, stdout: "0\n", stderr: "" }; + } + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--git-dir") { + return { exitCode: 1, stdout: "", stderr: "fatal: no git dir" }; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "list") { + return [`worktree ${repoRoot}`, "HEAD 1111111", "branch refs/heads/main", ""].join("\n") as any; + } + if (args[0] === "worktree" && args[1] === "add") { + const worktreePath = args[2]; + const branchRef = args[3]; + 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 (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + [racedLaneId, projectId, "Recovered import race", null, "worktree", "main", branchRef, worktreePath, null, 0, null, null, null, null, "active", now, null], + ); + db.run( + ` + insert into terminal_sessions(id, lane_id, title, started_at, transcript_path, status) + values (?, ?, ?, ?, ?, ?) + `, + ["session-on-import-race", racedLaneId, "shell", now, "/tmp/transcript.jsonl", "running"], + ); + return { exitCode: 0, stdout: "", stderr: "" } as any; + } + throw new Error(`Unexpected git call: ${args.join(" ")}`); + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId, + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + + const result = await service.importBranch({ branchRef: "feature/import-race", name: "Imported race" }); + + const worktreeRows = db.all<{ id: string }>( + "select id from lanes where project_id = ? and lane_type = 'worktree' and branch_ref = ?", + [projectId, "feature/import-race"], + ); + expect(worktreeRows.map((row) => row.id)).toEqual([result.id]); + expect(result.id).not.toBe(racedLaneId); + expect( + db.get<{ lane_id: string }>("select lane_id from terminal_sessions where id = ?", ["session-on-import-race"])?.lane_id, + ).toBe(result.id); + }); + it("removes a created tracking branch when worktree setup fails during import", async () => { const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-service-import-cleanup-")); const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index 6b2089ab8..b9c847035 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -324,6 +324,18 @@ function parseGitWorktreePorcelain(stdout: string): GitWorktreeInfo[] { return worktrees; } +/** + * The trailing segment of a managed worktree directory name is the 8-char + * prefix of the lane id that created it (`createWorktreeLane` builds the path + * as `${slug}-${laneId.slice(0, 8)}`). This is the canonical "which lane owns + * this worktree" signal used to pick the survivor when duplicate rows share a + * path. Returns "" for paths without a hyphen suffix (e.g. attached lanes). + */ +function worktreeDirSuffix(worktreePath: string): string { + const basename = path.basename(worktreePath); + return basename.includes("-") ? basename.split("-").pop() ?? "" : ""; +} + function inferLaneNameFromManagedWorktree(candidate: UnregisteredLaneCandidate): string { const basename = path.basename(candidate.path).trim(); const branchSlug = candidate.branch.trim().replace(/^ade\//, ""); @@ -1818,6 +1830,28 @@ export function createLaneService({ const normalizedProjectRoot = normAbs(projectRoot); const normalizedWorktreesDir = normAbs(worktreesDir); + // Worktree paths / branch refs with an in-flight `git worktree add`. A new + // worktree is visible to `git worktree list` before its lane row is + // inserted, so recovery/adoption surfaces must skip these or a concurrent + // lanes.list adopts the half-created worktree as a duplicate lane. + const pendingWorktreeCreationPaths = new Set(); + const pendingWorktreeCreationBranches = new Set(); + + const trackPendingWorktreeCreation = (worktreePath: string, branchRef: string): (() => void) => { + const pathKey = normAbs(worktreePath); + const branchKey = normalizeBranchKey(branchRef); + pendingWorktreeCreationPaths.add(pathKey); + if (branchKey) pendingWorktreeCreationBranches.add(branchKey); + return () => { + pendingWorktreeCreationPaths.delete(pathKey); + if (branchKey) pendingWorktreeCreationBranches.delete(branchKey); + }; + }; + + const isPendingWorktreeCreation = (worktreePath: string, branchRef: string): boolean => + pendingWorktreeCreationPaths.has(normAbs(worktreePath)) || + (normalizeBranchKey(branchRef).length > 0 && pendingWorktreeCreationBranches.has(normalizeBranchKey(branchRef))); + const getGitTopLevel = async (cwd: string): Promise => { const top = await runGitOrThrow(["rev-parse", "--path-format=absolute", "--show-toplevel"], { cwd, timeoutMs: 10_000 }); return normAbs(top.trim()); @@ -1853,7 +1887,11 @@ export function createLaneService({ ); return worktrees.filter( - (wt) => !wt.isBare && wt.path !== normalizedProjectRoot && !registeredPaths.has(wt.path) + (wt) => + !wt.isBare && + wt.path !== normalizedProjectRoot && + !registeredPaths.has(wt.path) && + !isPendingWorktreeCreation(wt.path, wt.branch) ); }; @@ -1866,6 +1904,8 @@ export function createLaneService({ const branchRef = candidate.branch.trim(); if (!branchRef) continue; if (path.dirname(worktreePath) !== normalizedWorktreesDir) continue; + // A create may have started after the candidate snapshot was taken. + if (isPendingWorktreeCreation(worktreePath, branchRef)) continue; const existingPath = db.get<{ id: string }>( "select id from lanes where project_id = ? and worktree_path = ? limit 1", @@ -1915,6 +1955,158 @@ export function createLaneService({ return recoveredCount; }; + /** + * Fold a duplicate lane's user-visible data onto the keeper, then cascade the + * duplicate row away. Sessions (and their session-scoped Linear links), child + * lanes, branch profiles, the lane's primary Linear issue, and additional + * Linear issue links are all re-pointed first so nothing the user attached to + * the soon-to-be-deleted row is lost. The caller MUST run this inside a + * transaction (both call sites already hold `begin immediate`) and the keeper + * row must already exist. Shared by the create/recover dedupe repair and the + * create-path raced-adoption absorb. + */ + const mergeDuplicateLaneInto = (keeperId: string, duplicateId: string): void => { + // Sessions and their session-scoped Linear links follow the lane. + db.run("update claude_sessions set lane_id = ? where lane_id = ?", [keeperId, duplicateId]); + db.run("update terminal_sessions set lane_id = ? where lane_id = ?", [keeperId, duplicateId]); + db.run("update session_linear_issues set lane_id = ? where lane_id = ? and project_id = ?", [ + keeperId, + duplicateId, + projectId, + ]); + // Child lanes / branch profiles re-parent onto the keeper. + db.run("update lanes set parent_lane_id = ? where parent_lane_id = ? and project_id = ?", [ + keeperId, + duplicateId, + projectId, + ]); + db.run("update lane_branch_profiles set parent_lane_id = ? where parent_lane_id = ? and project_id = ?", [ + keeperId, + duplicateId, + projectId, + ]); + db.run( + `delete from lane_branch_profiles + where lane_id = ? and project_id = ? + and exists ( + select 1 from lane_branch_profiles keeper + where keeper.lane_id = ? and keeper.project_id = ? + and keeper.normalized_branch_ref = lane_branch_profiles.normalized_branch_ref + )`, + [duplicateId, projectId, keeperId, projectId], + ); + db.run("update lane_branch_profiles set lane_id = ? where lane_id = ? and project_id = ?", [ + keeperId, + duplicateId, + projectId, + ]); + // Primary Linear issue: a lane holds at most one (`lane_linear_issues` is + // keyed by lane). Keep the keeper's if it already has one; otherwise adopt + // the duplicate's. + const keeperHasPrimaryIssue = db.get<{ one: number }>( + "select 1 as one from lane_linear_issues where lane_id = ? and project_id = ? limit 1", + [keeperId, projectId], + ); + if (keeperHasPrimaryIssue) { + db.run("delete from lane_linear_issues where lane_id = ? and project_id = ?", [duplicateId, projectId]); + } else { + db.run("update lane_linear_issues set lane_id = ? where lane_id = ? and project_id = ?", [ + keeperId, + duplicateId, + projectId, + ]); + } + // Additional Linear links are unique per (issue, role), so dedupe on both — + // dropping only the duplicate's links the keeper already carries for the + // same issue AND role, and migrating the rest. + db.run( + `delete from lane_linear_issue_links + where lane_id = ? and project_id = ? + and exists ( + select 1 from lane_linear_issue_links keeper + where keeper.lane_id = ? and keeper.project_id = ? + and keeper.issue_id = lane_linear_issue_links.issue_id + and keeper.role = lane_linear_issue_links.role + )`, + [duplicateId, projectId, keeperId, projectId], + ); + db.run("update lane_linear_issue_links set lane_id = ? where lane_id = ? and project_id = ?", [ + keeperId, + duplicateId, + projectId, + ]); + // Everything else lane-scoped on the duplicate cascades away. + cleanupLaneDatabaseRows(duplicateId); + }; + + /** + * Remove duplicate lane rows that share a managed worktree path. These are + * artifacts of the create/recover race: a lanes.list adopted a half-created + * worktree before the creating call inserted its own row. The keeper is the + * row whose id matches the suffix embedded in the worktree directory name + * (the lane that actually created the worktree — see `worktreeDirSuffix`), + * falling back to the oldest row. Each duplicate is folded into the keeper + * via `mergeDuplicateLaneInto`, so no user-visible data is lost. + */ + const repairDuplicateManagedWorktreeLanes = (): void => { + const rows = db.all<{ id: string; worktree_path: string; created_at: string }>( + ` + select id, worktree_path, created_at + from lanes + where project_id = ? and lane_type = 'worktree' and status != 'archived' + `, + [projectId] + ); + + const groups = new Map(); + for (const row of rows) { + const pathKey = normAbs(row.worktree_path); + if (!pathKey.length) continue; + const group = groups.get(pathKey) ?? []; + group.push(row); + groups.set(pathKey, group); + } + + let removedCount = 0; + for (const [pathKey, group] of groups) { + if (group.length < 2) continue; + // Never dedupe under an in-flight create; the row set is still settling. + if (pendingWorktreeCreationPaths.has(pathKey)) continue; + + const sorted = [...group].sort( + (a, b) => a.created_at.localeCompare(b.created_at) || a.id.localeCompare(b.id) + ); + const dirSuffix = worktreeDirSuffix(pathKey); + const keeper = + sorted.find((row) => dirSuffix.length > 0 && row.id.startsWith(dirSuffix)) ?? sorted[0]; + + for (const duplicate of sorted) { + if (duplicate.id === keeper.id) continue; + db.run("begin immediate"); + try { + mergeDuplicateLaneInto(keeper.id, duplicate.id); + db.run("commit"); + removedCount += 1; + } catch (error) { + try { + db.run("rollback"); + } catch { + // surface the original error below + } + throw error; + } + } + } + + if (removedCount > 0) { + invalidateLaneListCache(); + logger.info("laneService.repaired_duplicate_worktree_lanes", { + projectRoot, + count: removedCount, + }); + } + }; + const ensureAttachableWorktreeRoot = async (candidatePath: string): Promise => { const resolvedPath = normAbs(candidatePath); let worktreeRoot = ""; @@ -2120,6 +2312,11 @@ export function createLaneService({ } catch (err) { logger.warn("laneService.recoverManagedWorktreeRows_failed", { error: err instanceof Error ? err.message : String(err) }); } + try { + repairDuplicateManagedWorktreeLanes(); + } catch (err) { + logger.warn("laneService.repairDuplicateManagedWorktreeLanes_failed", { error: err instanceof Error ? err.message : String(err) }); + } try { backfillLaneBranchProfiles(); } catch (err) { @@ -2343,63 +2540,119 @@ export function createLaneService({ const runtimePlacement = normalizeRuntimePlacement(args.runtimePlacement); const worktreePath = path.join(worktreesDir, `${slug}-${suffix}`); - await runGitWorktreeMutation(() => - runGitOrThrow(["worktree", "add", "-b", branchRef, worktreePath, args.startPoint], { - cwd: projectRoot, - timeoutMs: 60_000 - }) - ); - - // From this point the worktree exists on disk. Any failure persisting the lane - // row (or its dependent inserts / VM wiring) must remove the worktree, otherwise - // we orphan a checkout that no lane row references. + // The worktree is visible to `git worktree list` the moment `worktree add` + // registers it, but the lane row only lands after checkout completes. Hold + // the pending marker across that window so a concurrent lanes.list cannot + // adopt the half-created worktree as a duplicate lane. + const releasePendingWorktreeCreation = trackPendingWorktreeCreation(worktreePath, branchRef); let linearIssue: LaneLinearIssue | null = null; + let absorbedRacedAdoptionRows = 0; try { - linkExistingDependencyInstalls(worktreePath); + await runGitWorktreeMutation(() => + runGitOrThrow(["worktree", "add", "-b", branchRef, worktreePath, args.startPoint], { + cwd: projectRoot, + timeoutMs: 60_000 + }) + ); - const laneColor = allocateLaneColorForProject(); - 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, folder, runtime_placement, status, created_at, archived_at - ) - values(?, ?, ?, ?, 'worktree', ?, ?, ?, null, 0, ?, ?, null, null, ?, ?, 'active', ?, null) - `, - [ + // From this point the worktree exists on disk. Any failure persisting the lane + // row (or its dependent inserts / VM wiring) must remove the worktree, otherwise + // we orphan a checkout that no lane row references. + try { + linkExistingDependencyInstalls(worktreePath); + + // Absorb any row another process raced onto this path/branch while the + // checkout ran (the path and branch embed this lane's fresh id suffix, + // and `worktree add -b` proved the branch was free, so a matching row + // can only be a recovery artifact adopted from the half-created + // worktree) and insert the canonical row in one transaction, so a crash + // can never leave the worktree with the artifact deleted but no keeper. + const laneColor = allocateLaneColorForProject(); + db.run("begin immediate"); + let absorbedRowsInTransaction = 0; + try { + // Insert the canonical row first, then fold any raced recovery + // artifact into it (so its sessions / Linear links migrate rather + // than being deleted). Archived lanes are excluded: an old archived + // lane may legitimately share this branch name after its Git ref was + // deleted, and it is not the raced artifact. + 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, folder, runtime_placement, status, created_at, archived_at + ) + values(?, ?, ?, ?, 'worktree', ?, ?, ?, null, 0, ?, ?, null, null, ?, ?, 'active', ?, null) + `, + [ + laneId, + projectId, + args.name, + args.description ?? null, + args.baseRef, + branchRef, + worktreePath, + args.parentLaneId, + laneColor, + args.folder ?? null, + runtimePlacement, + now + ] + ); + + const racedAdoptionRows = db.all<{ id: string }>( + ` + select id from lanes + where project_id = ? + and id != ? + and lane_type = 'worktree' + and status != 'archived' + and is_edit_protected = 0 + and (worktree_path = ? or branch_ref = ?) + `, + [projectId, laneId, worktreePath, branchRef] + ); + for (const raced of racedAdoptionRows) { + logger.info("laneService.absorbed_raced_adoption_row", { laneId, racedLaneId: raced.id }); + mergeDuplicateLaneInto(laneId, raced.id); + absorbedRowsInTransaction += 1; + } + + db.run("commit"); + absorbedRacedAdoptionRows = absorbedRowsInTransaction; + } catch (txError) { + try { + db.run("rollback"); + } catch { + // surface the original error below + } + throw txError; + } + // Linear issue upsert manages its own transaction, so it must run + // after the absorb+insert commit (SQLite has no nested transactions). + linearIssue = args.linearIssue + ? upsertLaneLinearIssue(laneId, args.linearIssue, branchRef) + : null; + invalidateLaneListCache(); + + if (runtimePlacement === "macos-vm") { + await wireMacosVmLanePlacement({ + laneId, + previousPlacement: "none", + rollbackPlacementOnLinkFailure: true, + }); + } + } catch (error) { + await cleanupCreatedWorktreeLaneAfterCreateFailure({ laneId, - projectId, - args.name, - args.description ?? null, - args.baseRef, branchRef, worktreePath, - args.parentLaneId, - laneColor, - args.folder ?? null, - runtimePlacement, - now - ] - ); - linearIssue = args.linearIssue - ? upsertLaneLinearIssue(laneId, args.linearIssue, branchRef) - : null; - invalidateLaneListCache(); - - if (runtimePlacement === "macos-vm") { - await wireMacosVmLanePlacement({ - laneId, - previousPlacement: "none", - rollbackPlacementOnLinkFailure: true, + cause: error, + preserveCreatedLane: absorbedRacedAdoptionRows > 0, }); } - } catch (error) { - await cleanupCreatedWorktreeLaneAfterCreateFailure({ - laneId, - branchRef, - worktreePath, - cause: error, - }); + } finally { + releasePendingWorktreeCreation(); } // Best-effort initial push to establish upstream tracking @@ -2679,10 +2932,22 @@ export function createLaneService({ branchRef: string; worktreePath: string; cause: unknown; + preserveCreatedLane?: boolean; }): Promise { const cleanupErrors: string[] = []; const originalMessage = args.cause instanceof Error ? args.cause.message : String(args.cause); + if (args.preserveCreatedLane) { + invalidateLaneListCache(); + logger.warn("laneService.lane_create_cleanup_skipped_after_absorb", { + laneId: args.laneId, + branchRef: args.branchRef, + worktreePath: args.worktreePath, + error: originalMessage, + }); + throw args.cause instanceof Error ? args.cause : new Error(originalMessage); + } + try { cleanupLaneDatabaseRows(args.laneId); invalidateLaneListCache(); @@ -3421,6 +3686,9 @@ export function createLaneService({ const suffix = laneId.slice(0, 8); const worktreePath = path.join(worktreesDir, `${slug}-${suffix}`); + // Same create/recover race as createWorktreeLane: keep recovery away + // from this path/branch until the lane row exists. + const releasePendingWorktreeCreation = trackPendingWorktreeCreation(worktreePath, branchRef); try { if (remoteRefToTrack) { await runGitOrThrow(["branch", "--track", branchRef, remoteRefToTrack], { cwd: projectRoot, timeoutMs: 15_000 }); @@ -3448,17 +3716,46 @@ export function createLaneService({ const baseRef = args.baseBranch?.trim() || defaultBaseRef; const laneColor = allocateLaneColorForProject(); - 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(?, ?, ?, ?, 'worktree', ?, ?, ?, null, 0, ?, ?, null, null, 'active', ?, null) - `, - [laneId, projectId, displayName, args.description ?? null, baseRef, branchRef, worktreePath, parentLaneId, laneColor, now] - ); - laneInserted = true; + db.run("begin immediate"); + try { + 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(?, ?, ?, ?, 'worktree', ?, ?, ?, null, 0, ?, ?, null, null, 'active', ?, null) + `, + [laneId, projectId, displayName, args.description ?? null, baseRef, branchRef, worktreePath, parentLaneId, laneColor, now] + ); + + const racedAdoptionRows = db.all<{ id: string }>( + ` + select id from lanes + where project_id = ? + and id != ? + and lane_type = 'worktree' + and status != 'archived' + and is_edit_protected = 0 + and (worktree_path = ? or branch_ref = ?) + `, + [projectId, laneId, worktreePath, branchRef] + ); + for (const raced of racedAdoptionRows) { + logger.info("laneService.import_absorbed_raced_adoption_row", { laneId, racedLaneId: raced.id }); + mergeDuplicateLaneInto(laneId, raced.id); + } + + db.run("commit"); + laneInserted = true; + } catch (txError) { + try { + db.run("rollback"); + } catch { + // surface the original error below + } + throw txError; + } invalidateLaneListCache(); // Best-effort push to establish upstream if not already tracking a remote @@ -3577,6 +3874,8 @@ export function createLaneService({ throw new Error(`${error instanceof Error ? error.message : String(error)} Cleanup failed: ${cleanupErrors.join(" ")}`); } throw error; + } finally { + releasePendingWorktreeCreation(); } }, diff --git a/docs/features/lanes/README.md b/docs/features/lanes/README.md index 0687da222..aa966f7a2 100644 --- a/docs/features/lanes/README.md +++ b/docs/features/lanes/README.md @@ -174,7 +174,7 @@ The `LaneType` column on the `lanes` table is one of: Primary lanes are created by `laneService.ensurePrimaryLane()` on project open and never rebuilt. Their `is_edit_protected = 1` flag prevents delete -and reparent operations. Two startup repair routines normalize older data: +and reparent operations. A set of repair routines normalize older data: - `repairPrimaryParentedRootLanes` — detaches non-primary lanes whose `parent_lane_id` was mistakenly set to the primary lane and resets @@ -182,8 +182,18 @@ and reparent operations. Two startup repair routines normalize older data: - `repairLegacyPrimaryBaseRootLanes` — normalizes `base_ref` on root worktree lanes that still point to a stale or non-default branch. Lanes with open PRs are excluded from repair. - -Both routines run at `createLaneService()` time. +- `repairDuplicateManagedWorktreeLanes` — removes duplicate lane rows + that share one managed worktree path (artifacts of the historical + create/recover race, see gotchas). Keeps the row whose id matches the + 8-char suffix embedded in the worktree directory name (the lane that + created the worktree), falling back to the oldest row; sessions, child + lanes, and Linear issue links on the duplicate are re-pointed to the + keeper before the duplicate cascades away, so no user-visible data is + lost. Runs in the `list()` repair block alongside + `recoverManagedWorktreeRows`. + +These routines run inside `listLanes` (i.e. on every `lanes.list`), not at +`createLaneService()` construction time. ## Lane status @@ -516,6 +526,19 @@ open lanes; primary lanes render with a home icon. `parent_lane_id`. - **Startup repair runs every boot.** If you introduce a new lane field that can drift, handle it in the repair routines too. +- **Half-created worktrees must stay invisible to recovery.** A new + worktree is visible to `git worktree list` the moment `worktree add` + registers it, but its lane row only lands after checkout completes. + `recoverManagedWorktreeRows` (run by every `lanes.list`) would adopt + that half-created worktree as a duplicate lane, so `createWorktreeLane` + and `importBranch` hold a pending-creation marker + (`trackPendingWorktreeCreation`) across the add→insert window, and + recovery/unregistered-worktree listings skip pending paths/branches. + Any new code path that runs `git worktree add` under `worktreesDir` + and inserts a lane row afterwards must do the same. The lanes table is + a cr-sqlite CRR, so a unique index cannot enforce this at the DB + layer; `repairDuplicateManagedWorktreeLanes` dedupes any rows that + slip through (e.g. from a second process). - **Lane list cache.** `LANE_LIST_CACHE_TTL_MS = 10_000`. Services that need fresh status after a git operation must call `laneService.list({ refresh: true })` or mutate through the