diff --git a/apps/ade-cli/README.md b/apps/ade-cli/README.md index 49cd2da08..1f339adb5 100644 --- a/apps/ade-cli/README.md +++ b/apps/ade-cli/README.md @@ -230,6 +230,7 @@ ade lanes list --text ade lanes create "fix-checkout-flow" --parent main ade lanes create "lin-123" --linear-issue-json '{"id":"...","identifier":"LIN-123","title":"...","projectId":"...","projectSlug":"...","teamId":"...","teamKey":"...","stateId":"...","stateName":"Todo","stateType":"unstarted","priority":2,"priorityLabel":"high","labels":[],"assigneeId":null,"assigneeName":null,"createdAt":"...","updatedAt":"..."}' ade lanes reparent lane-child --parent lane-parent --stack-base-branch main +ade lanes delete lane-id --force --delete-branch ade lanes create-from-linear --issue-id ENG-431 --start-chat --provider codex --model ade lanes batch-create-from-linear --linear-issues-json '[{"id":"...","identifier":"ENG-431"},{"id":"...","identifier":"ENG-440"}]' ade chat attach-linear-issue --issue-id ENG-431 diff --git a/apps/ade-cli/src/cli.test.ts b/apps/ade-cli/src/cli.test.ts index cf5e85c9f..223c66662 100644 --- a/apps/ade-cli/src/cli.test.ts +++ b/apps/ade-cli/src/cli.test.ts @@ -1362,6 +1362,33 @@ describe("ADE CLI", () => { }); }); + it("maps lane delete flags to the shared lane action", () => { + const plan = buildCliPlan([ + "lanes", + "delete", + "lane-old", + "--force", + "--delete-branch", + "--delete-remote-branch", + ]); + expect(plan.kind).toBe("execute"); + if (plan.kind !== "execute") throw new Error(`expected execute plan, got ${plan.kind}`); + expect(plan.label).toBe("lane delete"); + expect(plan.steps[0]?.params).toEqual({ + name: "run_ade_action", + arguments: { + domain: "lane", + action: "delete", + args: { + laneId: "lane-old", + force: true, + deleteBranch: true, + deleteRemoteBranch: true, + }, + }, + }); + }); + it("forwards PR GitHub snapshot full-history flag to the runtime action", () => { const snapshot = buildCliPlan([ "prs", diff --git a/apps/ade-cli/src/cli.ts b/apps/ade-cli/src/cli.ts index 17a50b844..a8e80db9a 100644 --- a/apps/ade-cli/src/cli.ts +++ b/apps/ade-cli/src/cli.ts @@ -937,6 +937,7 @@ const HELP_BY_COMMAND: Record = { $ ade lanes import --branch Register an existing branch/worktree $ ade lanes archive Archive a lane in ADE $ ade lanes unarchive Restore an archived lane + $ ade lanes delete --force Delete a lane and clean up its worktree $ ade lanes attach --path --name Attach an external worktree $ ade lanes reparent --parent Move lane onto a new parent (runs git rebase) $ ade lanes reparent --parent --stack-base-branch diff --git a/apps/ade-cli/src/tuiClient/__tests__/appInput.test.ts b/apps/ade-cli/src/tuiClient/__tests__/appInput.test.ts index 24199d4cd..2ce1b9958 100644 --- a/apps/ade-cli/src/tuiClient/__tests__/appInput.test.ts +++ b/apps/ade-cli/src/tuiClient/__tests__/appInput.test.ts @@ -1,3 +1,7 @@ +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; import { CLAUDE_TERMINAL_SUBMIT_CONFIRM_DELAY_MS, @@ -21,6 +25,7 @@ import { isPromptLineBackspace, isPromptWordBackspace, isTerminalSessionFastPollActive, + isLaneWorktreeAvailable, isTerminalSessionWorking, isTerminalSessionResumable, shouldToggleLatestFailedLineOnBlankEnter, @@ -310,6 +315,48 @@ describe("lane delete form helpers", () => { }); }); +describe("lane worktree availability", () => { + function laneAt(worktreePath: string): LaneSummary { + return { + id: "lane-1", + name: "Lane one", + laneType: "worktree", + baseRef: "main", + branchRef: "feature/lane-one", + worktreePath, + parentLaneId: null, + childCount: 0, + stackDepth: 0, + parentStatus: null, + isEditProtected: false, + status: { dirty: false, ahead: 0, behind: 0, remoteBehind: 0, rebaseInProgress: false }, + color: null, + icon: null, + tags: [], + createdAt: "2026-05-20T00:00:00.000Z", + }; + } + + it("rejects an existing stale directory that resolves to another git root", () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-tui-stale-lane-")); + try { + const init = spawnSync("git", ["init"], { cwd: repoRoot, encoding: "utf8" }); + const staleLanePath = path.join(repoRoot, ".ade", "worktrees", "stale-lane"); + fs.mkdirSync(staleLanePath, { recursive: true }); + + if (init.status === 0) { + expect(isLaneWorktreeAvailable(laneAt(repoRoot))).toBe(true); + } else { + fs.mkdirSync(path.join(repoRoot, ".git"), { recursive: true }); + expect(isLaneWorktreeAvailable(laneAt(repoRoot))).toBe(true); + } + expect(isLaneWorktreeAvailable(laneAt(staleLanePath))).toBe(false); + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + } + }); +}); + describe("right pane context defaults", () => { function laneForContext(overrides: Partial = {}): LaneSummary { return { diff --git a/apps/ade-cli/src/tuiClient/app.tsx b/apps/ade-cli/src/tuiClient/app.tsx index 78654fe09..070a2fac2 100644 --- a/apps/ade-cli/src/tuiClient/app.tsx +++ b/apps/ade-cli/src/tuiClient/app.tsx @@ -1009,14 +1009,56 @@ function formatGoalBannerLine(goal: CodexThreadGoal | null): string | null { import { subagentSnapshotsFromEvents } from "../../../desktop/src/shared/chatSubagents"; export { subagentSnapshotsFromEvents }; -function isLaneWorktreeAvailable(lane: LaneSummary | null | undefined): boolean { +const LANE_WORKTREE_AVAILABILITY_CACHE_TTL_MS = 2_000; +const laneWorktreeAvailabilityCache = new Map(); + +function cacheLaneWorktreeAvailability(root: string, stat: fs.Stats, checkedAt: number, available: boolean): boolean { + laneWorktreeAvailabilityCache.set(root, { checkedAt, mtimeMs: stat.mtimeMs, available }); + return available; +} + +function normalizeWorktreePath(root: string): string { + const resolved = path.resolve(root); + try { + return fs.realpathSync.native(resolved); + } catch { + return resolved; + } +} + +export function isLaneWorktreeAvailable(lane: LaneSummary | null | undefined): boolean { const root = lane?.worktreePath?.trim(); if (!root) return false; + const resolvedRoot = normalizeWorktreePath(root); + let stat: fs.Stats; try { - return fs.statSync(root).isDirectory(); + stat = fs.statSync(resolvedRoot); + if (!stat.isDirectory()) return false; } catch { return false; } + const cached = laneWorktreeAvailabilityCache.get(resolvedRoot); + const now = Date.now(); + if (cached && cached.mtimeMs === stat.mtimeMs && now - cached.checkedAt < LANE_WORKTREE_AVAILABILITY_CACHE_TTL_MS) { + return cached.available; + } + const markerExists = fs.existsSync(path.join(resolvedRoot, ".git")); + if (!markerExists) { + return cacheLaneWorktreeAvailability(resolvedRoot, stat, now, false); + } + const probe = spawnSync("git", ["rev-parse", "--path-format=absolute", "--show-toplevel"], { + cwd: resolvedRoot, + encoding: "utf8", + timeout: 8_000, + }); + let available: boolean; + if (probe.status === 0) { + const topLevel = probe.stdout.trim(); + available = topLevel ? normalizeWorktreePath(topLevel) === resolvedRoot : true; + } else { + available = false; + } + return cacheLaneWorktreeAvailability(resolvedRoot, stat, now, available); } function laneWorktreeUnavailableMessage(lane: LaneSummary | null | undefined): string | null { diff --git a/apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts b/apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts index 4f558ab12..1f07eb5a4 100644 --- a/apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts +++ b/apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts @@ -5,6 +5,8 @@ describe("buildCodingAgentSystemPrompt", () => { it("returns a prompt containing the cwd", () => { const result = buildCodingAgentSystemPrompt({ cwd: "/my/project" }); expect(result).toContain("/my/project"); + expect(result).toContain("Read-only inspection outside this path is allowed"); + expect(result).toContain("mutating commands only inside this path"); }); it("defaults to coding mode and edit permission mode", () => { diff --git a/apps/desktop/src/main/services/ai/tools/systemPrompt.ts b/apps/desktop/src/main/services/ai/tools/systemPrompt.ts index a7dbe447e..2cec42a09 100644 --- a/apps/desktop/src/main/services/ai/tools/systemPrompt.ts +++ b/apps/desktop/src/main/services/ai/tools/systemPrompt.ts @@ -219,7 +219,7 @@ export function buildCodingAgentSystemPrompt(args: { return [ `You are ADE's software engineering agent working in ${args.cwd}.`, - "This session is bound to that worktree. Read, edit, and run commands only inside this path unless ADE explicitly relaunches you in a different lane.", + "This session is bound to that worktree for writes and mutations. Read-only inspection outside this path is allowed when needed, but edit files and run mutating commands only inside this path unless ADE explicitly relaunches you in a different lane.", ...(orchestrationDirective ? [orchestrationDirective] : []), ...(runtime ? [ diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 8df5ef9e9..2a3e54e6f 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -3200,7 +3200,8 @@ describe("createAgentChatService", () => { })); expect(firstUserContent).toContain("[ADE launch directive]"); expect(firstUserContent).toContain(tmpRoot); - expect(firstUserContent).toContain("only inside that worktree"); + expect(firstUserContent).toContain("Read-only inspection outside that worktree is allowed"); + expect(firstUserContent).toContain("mutating commands only inside that worktree"); expect(firstUserContent).toContain("only normal reason to skip ADE CLI"); expect(firstUserContent).toContain("ade actions list --text"); expect(secondUserContent).not.toContain("[ADE launch directive]"); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index e7542af57..854131749 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -3532,7 +3532,7 @@ function buildLaneWorktreeDirective(args: { laneId: string; laneWorktreePath: st return [ "[ADE launch directive]", `ADE launched this session in lane '${laneId}' at worktree '${laneWorktreePath}'.`, - "Read, edit, and run commands only inside that worktree. Do not switch to project root, another lane, or another repo unless ADE explicitly relaunches you there.", + "Read-only inspection outside that worktree is allowed when needed. Edit files and run mutating commands only inside that worktree unless ADE explicitly relaunches you elsewhere.", ].join("\n"); } @@ -11928,7 +11928,7 @@ export function createAgentChatService(args: { "", "## ADE Workspace", `ADE launched this session in lane worktree: ${managed.laneWorktreePath}.`, - "Read, edit, and run commands only inside that worktree. Do not switch to project root, another lane, or another repo unless ADE explicitly relaunches you there.", + "Read-only inspection outside that worktree is allowed when needed. Edit files and run mutating commands only inside that worktree unless ADE explicitly relaunches you elsewhere.", "", ...slashCommandsSection, "", @@ -15933,7 +15933,7 @@ export function createAgentChatService(args: { "", "## ADE Workspace", `ADE launched this session in lane worktree: ${managed.laneWorktreePath}.`, - "Read, edit, and run commands only inside that worktree. Do not switch to project root, another lane, or another repo unless ADE explicitly relaunches you there.", + "Read-only inspection outside that worktree is allowed when needed. Edit files and run mutating commands only inside that worktree unless ADE explicitly relaunches you elsewhere.", "", ...(linearDirective ? [linearDirective, ""] : []), ...slashCommandsSection, diff --git a/apps/desktop/src/main/services/git/gitOperationsService.test.ts b/apps/desktop/src/main/services/git/gitOperationsService.test.ts index 8d8be664f..b97f0662e 100644 --- a/apps/desktop/src/main/services/git/gitOperationsService.test.ts +++ b/apps/desktop/src/main/services/git/gitOperationsService.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mockGit = vi.hoisted(() => ({ runGit: vi.fn(), @@ -6,6 +6,12 @@ const mockGit = vi.hoisted(() => ({ getHeadSha: vi.fn(), })); +afterEach(() => { + mockGit.runGit.mockReset(); + mockGit.runGitOrThrow.mockReset(); + mockGit.getHeadSha.mockReset(); +}); + vi.mock("./git", () => ({ runGit: (...args: unknown[]) => mockGit.runGit(...args), runGitOrThrow: (...args: unknown[]) => mockGit.runGitOrThrow(...args), @@ -70,6 +76,36 @@ function createTestGitOperationsService( }; } +describe("gitOperationsService lane worktree guard", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + mockGit.runGit.mockReset(); + }); + + it("rejects mutating actions when a stale lane path resolves to the main worktree", async () => { + mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/main\n", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: `unexpected git command: ${args.join(" ")}` }; + }); + const { service, mockStart } = createTestGitOperationsService("feature/stale", { + worktreePath: "/tmp/main/.ade/worktrees/feature-stale-12345678", + }); + + await expect(service.stageAll({ laneId: "lane-1", paths: [".ade/cto/identity.yaml"] })).rejects.toThrow( + "Lane worktree is missing. Restore or recreate the lane worktree at /tmp/main/.ade/worktrees/feature-stale-12345678 before viewing history.", + ); + + expect(mockGit.getHeadSha).not.toHaveBeenCalled(); + expect(mockGit.runGitOrThrow).not.toHaveBeenCalled(); + expect(mockStart).not.toHaveBeenCalled(); + }); +}); + describe("gitOperationsService.stashClear", () => { beforeEach(() => { vi.clearAllMocks(); @@ -187,6 +223,9 @@ describe("gitOperationsService.getSyncStatus", () => { it("marks a configured upstream as missing when the remote branch was deleted", async () => { mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }; + } if (args[0] === "rev-parse" && args[1] === "--abbrev-ref") { return { exitCode: 128, stdout: "", stderr: "upstream gone" }; } @@ -949,11 +988,65 @@ describe("gitOperationsService.generateCommitMessage", () => { vi.clearAllMocks(); }); + it("does not read main diffs when a stale lane path resolves to the main worktree", async () => { + const generateCommitMessage = vi.fn(); + mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/main\n", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: `unexpected git command: ${args.join(" ")}` }; + }); + + const service = createGitOperationsService({ + laneService: { + getLaneBaseAndBranch: () => ({ + baseRef: "main", + branchRef: "feature/stale", + worktreePath: "/tmp/main/.ade/worktrees/feature-stale-12345678", + laneType: "worktree", + }), + } as any, + operationService: { + start: vi.fn(), + finish: vi.fn(), + } as any, + projectConfigService: { + get: () => ({ + effective: { + ai: { + featureModelOverrides: { + commit_messages: "anthropic/claude-haiku-4-5", + }, + }, + }, + }), + } as any, + aiIntegrationService: { + getFeatureFlag: () => true, + getStatus: vi.fn(async () => ({ + availableModelIds: ["anthropic/claude-haiku-4-5"], + })), + generateCommitMessage, + } as any, + logger: makeStubLogger(), + }); + + await expect(service.generateCommitMessage({ laneId: "lane-1" })).rejects.toThrow( + "Lane worktree is missing. Restore or recreate the lane worktree at /tmp/main/.ade/worktrees/feature-stale-12345678 before viewing history.", + ); + + expect(mockGit.runGit).toHaveBeenCalledTimes(1); + expect(generateCommitMessage).not.toHaveBeenCalled(); + }); + it("uses the configured model and sends a lightweight changed-files prompt", async () => { let capturedPrompt = ""; let capturedModel = ""; mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }; + } if (args[0] === "diff") { return { exitCode: 0, @@ -1043,6 +1136,7 @@ describe("gitOperationsService.generateCommitMessage", () => { expect(capturedPrompt).not.toContain("Branch:"); expect(capturedPrompt).toContain("Diff:"); expect(mockGit.runGit.mock.calls.map((call) => call[0])).toEqual([ + ["rev-parse", "--path-format=absolute", "--show-toplevel"], ["diff", "--cached", "--name-status", "--find-renames"], ["show", "--name-status", "--format=", "--find-renames", "HEAD"], ["diff", "--cached", "--no-color", "-U2", "--find-renames"], @@ -1051,6 +1145,9 @@ describe("gitOperationsService.generateCommitMessage", () => { it("prefixes generated commit messages with a Linear reference for linked lanes", async () => { mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }; + } if (args[0] === "diff") { return { exitCode: 0, @@ -1154,6 +1251,9 @@ describe("gitOperationsService cached lane reads", () => { }); mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }; + } if (args[0] === "rev-parse") { await upstreamLookupGate; return { exitCode: 0, stdout: "origin/main\n", stderr: "" }; @@ -1178,14 +1278,19 @@ describe("gitOperationsService cached lane reads", () => { const [firstResult, secondResult] = await Promise.all([first, second]); expect(firstResult).toEqual(secondResult); - expect(mockGit.runGit).toHaveBeenCalledTimes(2); + expect(mockGit.runGit).toHaveBeenCalledTimes(3); expect(mockGit.runGit).toHaveBeenNthCalledWith( 1, - ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"], + ["rev-parse", "--path-format=absolute", "--show-toplevel"], expect.objectContaining({ cwd: "/tmp/ade-lane" }), ); expect(mockGit.runGit).toHaveBeenNthCalledWith( 2, + ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"], + expect.objectContaining({ cwd: "/tmp/ade-lane" }), + ); + expect(mockGit.runGit).toHaveBeenNthCalledWith( + 3, ["rev-list", "--left-right", "--count", "origin/main...HEAD"], expect.objectContaining({ cwd: "/tmp/ade-lane" }), ); @@ -1195,10 +1300,11 @@ describe("gitOperationsService cached lane reads", () => { mockGit.runGitOrThrow.mockResolvedValue( "abc123\u001fab\u001fparent123\u001fArul\u001f2026-04-11T21:00:00.000Z\u001fInitial commit", ); - mockGit.runGit.mockResolvedValue({ - exitCode: 1, - stdout: "", - stderr: "no upstream", + mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: "no upstream" }; }); const { service } = createTestGitOperationsService(); @@ -1208,11 +1314,13 @@ describe("gitOperationsService cached lane reads", () => { expect(first).toEqual(second); expect(mockGit.runGitOrThrow).toHaveBeenCalledTimes(1); - expect(mockGit.runGit).toHaveBeenCalledTimes(1); + expect(mockGit.runGit).toHaveBeenCalledTimes(2); }); it("checks whether a commit is reachable from the lane head", async () => { - mockGit.runGit.mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); + mockGit.runGit + .mockResolvedValueOnce({ exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }) + .mockResolvedValueOnce({ exitCode: 0, stdout: "", stderr: "" }); const { service } = createTestGitOperationsService(); await expect( @@ -1226,7 +1334,9 @@ describe("gitOperationsService cached lane reads", () => { }); it("reports commits outside the lane head history as not reachable", async () => { - mockGit.runGit.mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" }); + mockGit.runGit + .mockResolvedValueOnce({ exitCode: 0, stdout: "/tmp/ade-lane\n", stderr: "" }) + .mockResolvedValueOnce({ exitCode: 1, stdout: "", stderr: "" }); const { service } = createTestGitOperationsService(); await expect( @@ -1246,7 +1356,28 @@ describe("gitOperationsService cached lane reads", () => { await expect(service.listRecentCommits({ laneId: "lane-1", limit: 20 })).rejects.toThrow( "Lane worktree is missing. Restore or recreate the lane worktree at /tmp/missing-lane before viewing history.", ); - expect(mockGit.runGit).not.toHaveBeenCalled(); + expect(mockGit.runGit).toHaveBeenCalledWith( + ["rev-parse", "--path-format=absolute", "--show-toplevel"], + { cwd: "/tmp/missing-lane", timeoutMs: 8_000 }, + ); + }); + + it("does not read main history when a stale lane path resolves to the main worktree", async () => { + mockGit.runGit.mockImplementation(async (args: string[]) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: "/tmp/main\n", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: `unexpected git command: ${args.join(" ")}` }; + }); + + const { service } = createTestGitOperationsService("feature/stale", { + worktreePath: "/tmp/main/.ade/worktrees/feature-stale-12345678", + }); + + await expect(service.listRecentCommits({ laneId: "lane-1", limit: 20 })).rejects.toThrow( + "Lane worktree is missing. Restore or recreate the lane worktree at /tmp/main/.ade/worktrees/feature-stale-12345678 before viewing history.", + ); + expect(mockGit.runGitOrThrow).not.toHaveBeenCalled(); }); it("parses file history entries for modified and renamed files", async () => { diff --git a/apps/desktop/src/main/services/git/gitOperationsService.ts b/apps/desktop/src/main/services/git/gitOperationsService.ts index 9bb1708b3..c9255f20d 100644 --- a/apps/desktop/src/main/services/git/gitOperationsService.ts +++ b/apps/desktop/src/main/services/git/gitOperationsService.ts @@ -64,6 +64,10 @@ type CommitMessagePromptContext = { diffSnippet: string; }; +function normAbs(p: string): string { + return path.resolve(p); +} + type CachedReadEntry = { expiresAt: number; value?: T; @@ -429,7 +433,7 @@ export function createGitOperationsService({ function isMissingWorktreeError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error); - return /git working directory not found:/i.test(message); + return /git working directory not found:|Lane worktree is missing\./i.test(message); } function laneWorktreeMissingError(lane: LaneInfo): Error { @@ -438,6 +442,26 @@ export function createGitOperationsService({ ); } + async function assertLaneWorktreeRoot(lane: LaneInfo): Promise { + try { + const topLevelRes = await runGit( + ["rev-parse", "--path-format=absolute", "--show-toplevel"], + { cwd: lane.worktreePath, timeoutMs: 8_000 }, + ); + if (!topLevelRes || typeof topLevelRes.exitCode !== "number") return; + if (topLevelRes.exitCode !== 0) { + throw laneWorktreeMissingError(lane); + } + const topLevel = topLevelRes.stdout.trim(); + if (topLevel && normAbs(topLevel) !== normAbs(lane.worktreePath)) { + throw laneWorktreeMissingError(lane); + } + } catch (error) { + if (isMissingWorktreeError(error)) throw error; + throw laneWorktreeMissingError(lane); + } + } + const runLaneOperation = async ({ laneId, kind, @@ -453,6 +477,7 @@ export function createGitOperationsService({ }): Promise<{ result: T; action: GitActionResult }> => { invalidateLaneReadCache(laneId); const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const preHeadSha = await getHeadSha(lane.worktreePath); const operation = operationService.start({ laneId, @@ -760,6 +785,7 @@ export function createGitOperationsService({ async generateCommitMessage(args: GitGenerateCommitMessageArgs): Promise { const model = await assertCommitMessageGenerationEnabled(); const lane = laneService.getLaneBaseAndBranch(args.laneId); + await assertLaneWorktreeRoot(lane); const promptContext = await loadCommitMessagePromptContext(lane); if (!promptContext.hasStagedChanges && !args.amend) { throw new Error("Stage changes before generating a commit message."); @@ -787,6 +813,7 @@ export function createGitOperationsService({ const limit = typeof args.limit === "number" ? Math.max(1, Math.min(500, Math.floor(args.limit))) : 30; return readLaneCached(`recent-commits:${laneId}:${limit}`, 2_000, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); let out: string; try { out = await runGitOrThrow( @@ -851,6 +878,7 @@ export function createGitOperationsService({ const commitSha = normalizeCommitShaArg(args.commitSha); return readLaneCached(`commit:${laneId}:${commitSha}`, 2_000, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); let out: string; try { out = await runGitOrThrow( @@ -911,6 +939,7 @@ export function createGitOperationsService({ const commitSha = normalizeCommitShaArg(args.commitSha); return readLaneCached(`commit-in-lane-history:${laneId}:${commitSha}`, 2_000, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const result = await runGit( ["merge-base", "--is-ancestor", commitSha, "HEAD"], { cwd: lane.worktreePath, timeoutMs: 10_000 }, @@ -930,6 +959,7 @@ export function createGitOperationsService({ const limit = typeof args.limit === "number" ? Math.max(1, Math.min(100, Math.floor(args.limit))) : 20; return readLaneCached(`file-history:${laneId}:${relPath}:${limit}`, 2_000, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const out = await runGitOrThrow( [ "log", @@ -1012,6 +1042,7 @@ export function createGitOperationsService({ const laneId = args.laneId.trim(); return readLaneCached(`sync-status:${laneId}:default`, 2_000, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const readConfiguredUpstream = async (): Promise => { const branchName = localBranchNameForConfig(lane.branchRef); if (!branchName) return null; @@ -1120,6 +1151,7 @@ export function createGitOperationsService({ async listCommitFiles(args: GitListCommitFilesArgs): Promise { const lane = laneService.getLaneBaseAndBranch(args.laneId); + await assertLaneWorktreeRoot(lane); const sha = args.commitSha.trim(); if (!sha.length) throw new Error("commitSha is required"); const res = await runGitOrThrow(["show", "--pretty=format:", "--name-only", sha], { @@ -1134,6 +1166,7 @@ export function createGitOperationsService({ async getCommitMessage(args: GitGetCommitMessageArgs): Promise { const lane = laneService.getLaneBaseAndBranch(args.laneId); + await assertLaneWorktreeRoot(lane); const sha = args.commitSha.trim(); if (!sha.length) throw new Error("commitSha is required"); const res = await runGitOrThrow(["show", "-s", "--format=%B", sha], { @@ -1247,6 +1280,7 @@ export function createGitOperationsService({ const laneId = args.laneId.trim(); return readLaneCached(`stashes:${laneId}:default`, 1_500, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); return (await listBranchStashes(lane)).map(toPublicStash); }); }, @@ -1481,6 +1515,7 @@ export function createGitOperationsService({ if (!laneId) throw new Error("laneId is required"); return readLaneCached(`conflict-state:${laneId}:default`, 1_500, async () => { const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const gitDir = await getAbsoluteGitDir(lane.worktreePath); const kind = gitDir ? detectConflictKind(gitDir) : null; @@ -1558,6 +1593,7 @@ export function createGitOperationsService({ async listBranches(args: { laneId: string }): Promise { const lane = laneService.getLaneBaseAndBranch(args.laneId); + await assertLaneWorktreeRoot(lane); // Subject goes last so a tab inside a commit subject doesn't desync the // parser — anything after index 7 is rejoined. const FORMAT = [ @@ -1657,6 +1693,7 @@ export function createGitOperationsService({ async getUserIdentity(args: { laneId: string }): Promise { const lane = laneService.getLaneBaseAndBranch(args.laneId); + await assertLaneWorktreeRoot(lane); const readConfig = async (key: string): Promise => { const result = await runGit(["config", "--get", key], { cwd: lane.worktreePath, @@ -1673,6 +1710,7 @@ export function createGitOperationsService({ const laneId = args.laneId.trim(); if (!laneId) return fallback; const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const [remoteRes, branchRes] = await Promise.all([ runGit(["remote", "get-url", "origin"], { cwd: lane.worktreePath, timeoutMs: 8_000 }).catch(() => null), lane.branchRef?.trim() @@ -1707,6 +1745,7 @@ export function createGitOperationsService({ const laneId = args.laneId.trim(); if (!laneId) return fallback; const lane = laneService.getLaneBaseAndBranch(laneId); + await assertLaneWorktreeRoot(lane); const branch = args.branch?.trim() || lane.branchRef?.trim() || ""; if (!branch) return fallback; diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index a5313911f..aa176bb8c 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -2768,6 +2768,53 @@ describe("laneService updateAppearance color uniqueness", () => { }); }); +describe("laneService stale worktree status", () => { + beforeEach(() => { + vi.mocked(getHeadSha).mockReset(); + vi.mocked(runGit).mockReset(); + vi.mocked(runGitOrThrow).mockReset(); + }); + + it("does not report main checkout status for a stale lane directory inside the repo", async () => { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-stale-status-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-stale-status", repoRoot }); + const childPath = path.join(repoRoot, "child"); + fs.mkdirSync(childPath, { recursive: true }); + + vi.mocked(runGit).mockImplementation(async (args: string[], opts?: { cwd?: string }) => { + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + expect(opts?.cwd).toBe(childPath); + return { exitCode: 0, stdout: `${repoRoot}\n`, stderr: "" } as any; + } + if (args[0] === "status") return { exitCode: 0, stdout: " M main-only-file\n", stderr: "" } as any; + if (args[0] === "rev-list") return { exitCode: 0, stdout: "0\t5\n", stderr: "" } as any; + return { exitCode: 1, stdout: "", stderr: "" } as any; + }); + + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-stale-status", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + + const lanes = await service.list({ includeStatus: true }); + const child = lanes.find((lane) => lane.id === "lane-child"); + expect(child?.status).toEqual({ + dirty: false, + ahead: 0, + behind: 0, + remoteBehind: -1, + rebaseInProgress: false, + }); + expect(vi.mocked(runGit).mock.calls.some(([args, opts]) => + args[0] === "status" && (opts as { cwd?: string } | undefined)?.cwd === childPath + )).toBe(false); + }); +}); + describe("laneService delete teardown + cancellation + streaming", () => { beforeEach(() => { vi.mocked(getHeadSha).mockReset(); @@ -2915,7 +2962,7 @@ describe("laneService delete teardown + cancellation + streaming", () => { expect(fs.existsSync(childPath)).toBe(false); expect(db.get<{ id: string }>("select id from lanes where id = ?", ["lane-child"])).toBeNull(); - expect(vi.mocked(runGitOrThrow).mock.calls.some(([args]) => args[0] === "worktree" && args[1] === "prune")).toBe(true); + expect(vi.mocked(runGit).mock.calls.some(([args]) => args[0] === "worktree" && args[1] === "prune")).toBe(true); const last = events[events.length - 1]; expect(last.progress.steps.find((s: any) => s.name === "git_worktree_remove")?.detail).toContain("removed residual files"); }); @@ -2956,6 +3003,62 @@ 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 () => { + const events: any[] = []; + const fake = makeFakeServices(); + const { service, db, repoRoot } = await setupWithLane({ teardown: fake, events }); + const childPath = path.join(repoRoot, "child"); + 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]) => { + if (path.resolve(String(target)) === childPath) { + const error = new Error(`ENOTEMPTY: directory not empty, rmdir '${childPath}'`) as NodeJS.ErrnoException; + error.code = "ENOTEMPTY"; + throw error; + } + return realRm(target, options); + }); + + vi.mocked(runGit).mockImplementation(async (args: string[], opts?: { cwd?: string }) => { + const laneBranchGitStub = defaultLaneBranchGitStub(args); + if (laneBranchGitStub) return laneBranchGitStub; + if (args[0] === "rev-parse" && args[1] === "--path-format=absolute" && args[2] === "--show-toplevel") { + return { exitCode: 0, stdout: `${repoRoot}\n`, stderr: "" } as any; + } + if (args[0] === "worktree" && args[1] === "remove") { + expect(opts?.cwd).toBe(repoRoot); + return { + exitCode: 128, + stdout: "", + stderr: `fatal: '${childPath}' is not a working tree`, + } as any; + } + if (args[0] === "worktree" && args[1] === "prune") { + return { exitCode: 0, stdout: "", stderr: "" } as any; + } + if (args[0] === "show-ref") return { exitCode: 1, stdout: "", stderr: "" } as any; + return { exitCode: 0, stdout: "", stderr: "" } as any; + }); + vi.mocked(runGitOrThrow).mockImplementation(async (args: string[]) => { + if (args[0] === "worktree" && args[1] === "list") return ""; + return ""; + }); + + try { + await service.delete({ laneId: "lane-child", deleteBranch: false, force: true }); + } finally { + rmSpy.mockRestore(); + } + + expect(db.get<{ id: string }>("select id from lanes where id = ?", ["lane-child"])).toBeNull(); + expect(fs.existsSync(childPath)).toBe(true); + 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"); + }); + it("keeps recent delete progress queryable for remounted renderers", async () => { const events: any[] = []; const fake = makeFakeServices(); diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index 535d7769f..3d92aa795 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -257,6 +257,35 @@ function normAbs(p: string): string { return path.resolve(p); } +const STALE_WORKTREE_ROOT_MESSAGE = "Lane worktree is missing or no longer points at its Git worktree root."; + +async function isExpectedGitWorktreeRoot(worktreePath: string): Promise { + if (!worktreePath) return false; + try { + const topLevelRes = await runGit( + ["rev-parse", "--path-format=absolute", "--show-toplevel"], + { cwd: worktreePath, timeoutMs: 8_000 }, + ); + if (!topLevelRes || typeof topLevelRes.exitCode !== "number") return true; + if (topLevelRes.exitCode !== 0) return false; + const topLevel = topLevelRes.stdout.trim(); + if (!topLevel) return true; + return normAbs(topLevel) === normAbs(worktreePath); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + // Test mocks can throw for unhandled git probes. The real runGit helper + // returns a GitRunResult for normal git failures, so keep those tests on + // their existing path without weakening production validation. + return /^Unexpected git call:/i.test(message); + } +} + +async function assertExpectedGitWorktreeRoot(worktreePath: string): Promise { + if (!(await isExpectedGitWorktreeRoot(worktreePath))) { + throw new Error(STALE_WORKTREE_ROOT_MESSAGE); + } +} + type GitWorktreeInfo = UnregisteredLaneCandidate & { isBare: boolean; }; @@ -500,6 +529,7 @@ function toLaneSummary(args: { } async function detectBranchRef(worktreePath: string, fallback: string): Promise { + if (!(await isExpectedGitWorktreeRoot(worktreePath))) return fallback; const branchRes = await runGit(["rev-parse", "--abbrev-ref", "HEAD"], { cwd: worktreePath, timeoutMs: 8_000 }); if (branchRes.exitCode === 0) { const value = branchRes.stdout.trim(); @@ -509,6 +539,10 @@ async function detectBranchRef(worktreePath: string, fallback: string): Promise< } async function computeLaneStatus(worktreePath: string, baseRef: string, branchRef: string): Promise { + if (!(await isExpectedGitWorktreeRoot(worktreePath))) { + return cloneLaneStatus(DEFAULT_LANE_STATUS); + } + const dirtyRes = await runGit(["status", "--porcelain=v1"], { cwd: worktreePath, timeoutMs: 8_000 }); const dirty = dirtyRes.exitCode === 0 && dirtyRes.stdout.trim().length > 0; @@ -776,6 +810,8 @@ type WorktreeChangeState = { }; async function inspectWorktreeChanges(worktreePath: string): Promise { + await assertExpectedGitWorktreeRoot(worktreePath); + const statusRes = await runGit(["status", "--porcelain=v1"], { cwd: worktreePath, timeoutMs: 8_000 }); if (statusRes.exitCode !== 0) { throw new Error("Unable to inspect lane worktree state."); @@ -805,6 +841,8 @@ type GitStashEntry = { }; async function listGitStashes(worktreePath: string): Promise { + await assertExpectedGitWorktreeRoot(worktreePath); + const stashRes = await runGit(["stash", "list", "--format=%gd%x1f%gs"], { cwd: worktreePath, timeoutMs: 15_000, @@ -4612,6 +4650,10 @@ export function createLaneService({ progress.steps.find((s) => s.name === name); const nonFatalFailures: Array<{ step: LaneDeleteStepName; message: string }> = []; + const recordNonFatalFailure = (step: LaneDeleteStepName, message: string): void => { + nonFatalFailures.push({ step, message }); + logger.warn("lane.delete.non_fatal_step_failed", { laneId, step, error: message }); + }; const runStep = async ( name: LaneDeleteStepName, @@ -4637,8 +4679,7 @@ export function createLaneService({ step.completedAt = new Date().toISOString(); step.durationMs = Date.now() - t0; if (!fatal) { - nonFatalFailures.push({ step: name, message: errorMessage }); - logger.warn("lane.delete.non_fatal_step_failed", { laneId, step: name, error: errorMessage }); + recordNonFatalFailure(name, errorMessage); } broadcastDeleteEvent(progress); if (fatal) throw error; @@ -4669,6 +4710,9 @@ export function createLaneService({ try { if (hasWorktree) { await runStep("git_status", async () => { + if (!(await isExpectedGitWorktreeRoot(row.worktree_path))) { + return { detail: fs.existsSync(row.worktree_path) ? "stale worktree directory" : "missing worktree directory" }; + } const dirtyRes = await runGit(["status", "--porcelain=v1"], { cwd: row.worktree_path, timeoutMs: 8_000 }); const dirty = dirtyRes.exitCode === 0 && dirtyRes.stdout.trim().length > 0; if (dirty && !force) { @@ -4748,17 +4792,46 @@ export function createLaneService({ const removeArgs = ["worktree", "remove"]; if (force) removeArgs.push("--force"); removeArgs.push(row.worktree_path); + const isStillRegisteredWorktree = async (): Promise => { + try { + const worktrees = await listGitWorktrees(); + const target = normAbs(row.worktree_path); + return worktrees.some((wt) => wt.path === target); + } catch { + return fs.existsSync(worktreeMetadataPath); + } + }; + const pruneWorktreesBestEffort = async (): Promise => { + const pruneRes = await runGit(["worktree", "prune"], { cwd: projectRoot, timeoutMs: 30_000 }); + if (pruneRes.exitCode === 0) return null; + return (pruneRes.stderr || pruneRes.stdout || "git worktree prune failed").trim(); + }; const removeResidualDirectory = async (detail: string, failurePrefix?: string) => { try { await removeWorktreeDirectoryWithRecovery(row.worktree_path); } catch (rmError) { - throw new Error( - `${failurePrefix ? `${failurePrefix}; ` : ""}manual cleanup failed: ${ - rmError instanceof Error ? rmError.message : String(rmError) - }` - ); + const cleanupMessage = `${failurePrefix ? `${failurePrefix}; ` : ""}manual cleanup failed: ${ + rmError instanceof Error ? rmError.message : String(rmError) + }`; + const pruneFailure = await pruneWorktreesBestEffort(); + const fullMessage = pruneFailure + ? `${cleanupMessage}; git worktree prune failed: ${pruneFailure}` + : cleanupMessage; + if (await isStillRegisteredWorktree()) { + throw new Error(fullMessage); + } + recordNonFatalFailure("git_worktree_remove", fullMessage); + return { detail: `${detail}; warning: ${fullMessage}` }; + } + const pruneFailure = await pruneWorktreesBestEffort(); + if (pruneFailure) { + const message = `git worktree prune failed: ${pruneFailure}`; + if (await isStillRegisteredWorktree()) { + throw new Error(message); + } + recordNonFatalFailure("git_worktree_remove", message); + return { detail: `${detail}; warning: ${message}` }; } - await runGitOrThrow(["worktree", "prune"], { cwd: projectRoot, timeoutMs: 30_000 }); return { detail }; }; // 60s — large worktrees (e.g. with node_modules) can take longer than 15s @@ -4886,8 +4959,9 @@ export function createLaneService({ const row = getLaneRow(laneId); if (!row) throw new Error(`Lane not found: ${laneId}`); const worktreeExists = Boolean(row.worktree_path) && fs.existsSync(row.worktree_path); + const worktreeUsable = worktreeExists && await isExpectedGitWorktreeRoot(row.worktree_path); let dirty = false; - if (worktreeExists) { + if (worktreeUsable) { const dirtyRes = await runGit(["status", "--porcelain=v1"], { cwd: row.worktree_path, timeoutMs: 6_000 }); dirty = dirtyRes.exitCode === 0 && dirtyRes.stdout.trim().length > 0; } @@ -4899,7 +4973,7 @@ export function createLaneService({ // ref like "origin/feature" must probe "feature"). const riskBranchName = row.branch_ref ? branchNameForDelete(row.branch_ref, "origin") : ""; if (riskBranchName) { - const cwd = worktreeExists ? row.worktree_path : projectRoot; + const cwd = worktreeUsable ? row.worktree_path : projectRoot; const unpushed = await runGit( ["rev-list", "--count", riskBranchName, "--not", "--remotes"], { cwd, timeoutMs: 8_000 } diff --git a/apps/desktop/src/main/services/pty/ptyService.test.ts b/apps/desktop/src/main/services/pty/ptyService.test.ts index 3665ef4be..ba2832643 100644 --- a/apps/desktop/src/main/services/pty/ptyService.test.ts +++ b/apps/desktop/src/main/services/pty/ptyService.test.ts @@ -2152,7 +2152,7 @@ describe("ptyService", () => { exitCode: 0, status: "completed", }); - sessionService.readTranscriptTail.mockResolvedValueOnce([ + sessionService.readTranscriptTail.mockResolvedValue([ "Update available! 0.130.0 -> 0.134.0\n", "Update ran successfully! Please restart Codex.\n", ].join("")); @@ -2165,6 +2165,73 @@ describe("ptyService", () => { expect(loadPty).not.toHaveBeenCalled(); }); + it("sendToSession backfills a Codex storage target before launching resume", async () => { + vi.useFakeTimers(); + try { + const fakeNow = new Date("2026-04-15T22:00:00.000Z"); + vi.setSystemTime(fakeNow); + + const homedir = os.homedir(); + const sessionsBase = path.join(homedir, ".codex", "sessions"); + const dirPath = path.join(sessionsBase, "2026", "04", "15"); + const filePath = path.join(dirPath, "rollout-2026-04-15T21-30-00-thread-storage.jsonl"); + const startedAt = "2026-04-15T21:30:00.000Z"; + const firstLine = JSON.stringify({ + timestamp: startedAt, + type: "session_meta", + payload: { + id: "thread-storage", + timestamp: startedAt, + cwd: "/tmp/worktree", + }, + }); + + mocks.existsSyncResults.set(sessionsBase, true); + mocks.existsSyncResults.set(dirPath, true); + mocks.dirEntries.set(dirPath, [path.basename(filePath)]); + mocks.fileContents.set(filePath, `${firstLine}\n`); + mocks.fileStats.set(filePath, { size: firstLine.length, mtimeMs: fakeNow.getTime() - 30_000, isDirectory: false }); + + const { service, sessionService, loadPty } = createHarness(); + sessionService.readTranscriptTail.mockResolvedValue("OpenAI Codex\nmodel: gpt-5\n› "); + sessionService.create({ + sessionId: "session-codex-storage-send", + laneId: "lane-1", + ptyId: null, + tracked: true, + title: "Codex CLI", + startedAt, + transcriptPath: "/tmp/worktree/.ade/transcripts/session-codex-storage-send.log", + toolType: "codex", + resumeCommand: "codex --no-alt-screen resume", + }); + sessionService.end({ + sessionId: "session-codex-storage-send", + endedAt: "2026-04-15T21:45:00.000Z", + exitCode: 0, + status: "completed", + }); + + await service.sendToSession({ + sessionId: "session-codex-storage-send", + text: "continue", + }); + + expect(sessionService.setResumeCommand).toHaveBeenCalledWith( + "session-codex-storage-send", + "codex resume thread-storage", + ); + const spawn = (loadPty.mock.results[0]?.value as any).spawn; + expect(spawn).toHaveBeenCalledWith( + "/bin/bash", + ["--noprofile", "--norc", "-lc", "codex --no-alt-screen resume thread-storage continue"], + expect.any(Object), + ); + } finally { + vi.useRealTimers(); + } + }); + it("sendToSession resumes an ended tracked CLI session with the message in the launch command", async () => { const { service, sessionService, mockPty, loadPty } = createHarness(); sessionService.create({ @@ -2791,6 +2858,9 @@ describe("ptyService", () => { service.sendToSession({ sessionId: "session-concurrent-send", text: "first" }), service.sendToSession({ sessionId: "session-concurrent-send", text: "second" }), ]); + for (let i = 0; i < 10 && loadPty.mock.calls.length === 0; i += 1) { + await Promise.resolve(); + } await Promise.resolve(); mockPty._emitter.emit("data", "OpenAI Codex\n› "); const [first, second] = await pending; diff --git a/apps/desktop/src/main/services/pty/ptyService.ts b/apps/desktop/src/main/services/pty/ptyService.ts index e0baac7d0..16af1d362 100644 --- a/apps/desktop/src/main/services/pty/ptyService.ts +++ b/apps/desktop/src/main/services/pty/ptyService.ts @@ -3101,10 +3101,10 @@ export function createPtyService({ } }; - const resolveEndedResumeSession = ( + const resolveEndedResumeSession = async ( sessionId: string, session: TerminalSessionSummary | null, - ): { session: TerminalSessionSummary; provider: TerminalResumeProvider } | Promise<{ session: TerminalSessionSummary; provider: TerminalResumeProvider }> => { + ): Promise<{ session: TerminalSessionSummary; provider: TerminalResumeProvider }> => { if (!session) throw new Error(`Terminal session '${sessionId}' was not found.`); const provider = session.resumeMetadata?.provider ?? providerFromTool(session.toolType); @@ -3116,32 +3116,43 @@ export function createPtyService({ `${displayName} exited before ADE could capture a concrete resume target. Start a new ${displayName} session.`, ); }; + const resumeTargetIdFor = (candidate: TerminalSessionSummary): string | null => { + const parsedResumeCommand = parseTrackedCliResumeCommand(candidate.resumeCommand, candidate.toolType); + return sanitizeResumeTargetId(candidate.resumeMetadata?.targetId ?? null) + ?? (parsedResumeCommand?.provider === provider + ? sanitizeResumeTargetId(parsedResumeCommand.targetId ?? null) + : null); + }; - const parsedResumeCommand = parseTrackedCliResumeCommand(session.resumeCommand, session.toolType); - const storedResumeTargetId = sanitizeResumeTargetId(session.resumeMetadata?.targetId ?? null) - ?? (parsedResumeCommand?.provider === provider - ? sanitizeResumeTargetId(parsedResumeCommand.targetId ?? null) - : null); + let resolvedSession = session; + let storedResumeTargetId = resumeTargetIdFor(resolvedSession); + if (!storedResumeTargetId && provider !== "cursor" && isTrackedCliToolType(resolvedSession.toolType)) { + const cwd = inferSessionCwdFromTranscriptPath(resolvedSession.transcriptPath); + const backfilled = await tryBackfillResumeTarget(sessionId, resolvedSession.toolType, "resume-launch", cwd); + const updatedSession = backfilled ? sessionService.get(sessionId) : null; + if (updatedSession) { + resolvedSession = updatedSession; + storedResumeTargetId = resumeTargetIdFor(resolvedSession); + } + } if ( provider === "codex" - && isCodexTrackedCliToolType(session.toolType) + && isCodexTrackedCliToolType(resolvedSession.toolType) && !storedResumeTargetId ) { - return sessionService.readTranscriptTail(session.transcriptPath, 220_000) - .then((transcript) => { - if (isCodexCliUpdateTranscript(transcript)) { - throw new Error( - "Codex updated and exited before ADE could create a resumable thread. Start a new Codex session.", - ); - } - return throwMissingResumeTarget(); - }); + const transcript = await sessionService.readTranscriptTail(resolvedSession.transcriptPath, 220_000); + if (isCodexCliUpdateTranscript(transcript)) { + throw new Error( + "Codex updated and exited before ADE could create a resumable thread. Start a new Codex session.", + ); + } + return throwMissingResumeTarget(); } if (!storedResumeTargetId && provider !== "cursor") { throwMissingResumeTarget(); } - return { session, provider }; + return { session: resolvedSession, provider }; }; const resumeLaunchOverrides = ( @@ -4005,10 +4016,7 @@ export function createPtyService({ ); } - const resolvedResume = resolveEndedResumeSession(sessionId, session); - const { session: resumableSession, provider } = resolvedResume instanceof Promise - ? await resolvedResume - : resolvedResume; + const { session: resumableSession, provider } = await resolveEndedResumeSession(sessionId, session); const overrides = resumeLaunchOverrides(args); const openCodeReplayCommand = provider === "opencode" && resumableSession.resumeMetadata?.provider === "opencode" @@ -4068,10 +4076,7 @@ export function createPtyService({ ); } - const resolvedResume = resolveEndedResumeSession(sessionId, session); - const { session: resumableSession, provider } = resolvedResume instanceof Promise - ? await resolvedResume - : resolvedResume; + const { session: resumableSession, provider } = await resolveEndedResumeSession(sessionId, session); const { command: resumeCommand } = buildResumeCommandForSession(resumableSession, provider, resumeLaunchOverrides(args)); if (!resumeCommand) { throw new Error(`Terminal session '${sessionId}' does not have a resume command.`); diff --git a/docs/features/agents/tool-registration.md b/docs/features/agents/tool-registration.md index f94404e47..b0e0a3aee 100644 --- a/docs/features/agents/tool-registration.md +++ b/docs/features/agents/tool-registration.md @@ -278,6 +278,12 @@ task. `buildAdeCliAgentGuidance()` and `buildAdeCliInlineGuidance()` currently share the same compact guidance body so system prompts, worker launches, and inline Work-tab preambles stay aligned. +Lane launch directives pair this ADE CLI guidance with worktree write +boundaries. Agents may inspect files outside the launched lane when they +need read-only context, but edits and mutating shell commands are only +allowed inside the lane worktree unless ADE relaunches the session in a +different lane. + ## Fragile and tricky wiring - **Identity must come from env or trusted CLI flags.** A rogue client diff --git a/docs/features/chat/README.md b/docs/features/chat/README.md index 60462faf4..8ccfb749f 100644 --- a/docs/features/chat/README.md +++ b/docs/features/chat/README.md @@ -117,7 +117,11 @@ render them, but neither one *runs* them. `acpEventMapper.ts` for terminal lifecycle parity. - **Lane-scoped.** Every session carries `laneId`; lane context (branch, worktree path) is injected into the system prompt, and working-directory - resolution runs through `resolveLaneLaunchContext`. + resolution runs through `resolveLaneLaunchContext`. The injected ADE + workspace directive treats the lane worktree as the boundary for + writes and mutations: read-only inspection outside the worktree is + allowed when needed, but file edits and mutating commands must stay + inside the launched lane unless ADE relaunches the session elsewhere. - **Event stream first.** All transcript content is a JSON-lines stream of `AgentChatEventEnvelope` values. Renderer components derive UI state entirely from this stream. diff --git a/docs/features/chat/agent-routing.md b/docs/features/chat/agent-routing.md index 0180ec307..a075f3bfd 100644 --- a/docs/features/chat/agent-routing.md +++ b/docs/features/chat/agent-routing.md @@ -174,6 +174,13 @@ over a stale persisted pair. Turns use the Codex-native `effort` key (`turn/start({ threadId, input, effort?, serviceTier? })`) instead of the lifecycle `reasoningEffort` name. +For Codex and the other coding-agent paths, ADE's worktree guidance is +write-scoped rather than read-scoped: the launched lane worktree is the +only place the agent should edit files or run mutating commands, while +read-only inspection outside that path is allowed when it needs context. +This wording appears in both the system prompt and the first-turn launch +directive so resumed/continued sessions keep the same boundary. + #### Provider service tiers (Fast Mode) `ModelDescriptor.serviceTiers?: string[]` advertises the optional diff --git a/docs/features/history/README.md b/docs/features/history/README.md index 1ba9cbbc3..5b7913b82 100644 --- a/docs/features/history/README.md +++ b/docs/features/history/README.md @@ -48,7 +48,7 @@ Main process / runtime services: |---|---| | `apps/desktop/src/main/services/history/operationService.ts` | CRUD for `operations` rows; the canonical entry point for `record`, `start`, `finish`, `list`, `get`, and `listHeadChanges`. Same source backs the runtime daemon and the desktop fallback path. | | `apps/desktop/src/main/services/state/kvDb.ts` | Schema for `operations`, `checkpoints`, `pack_events`, `pack_versions`, `pack_heads`, `terminal_sessions`, and orchestration-related tables. | -| `apps/desktop/src/main/services/git/gitOperationsService.ts` | Brackets every git operation with `operationService.start` / `finish`, captures pre/post HEAD SHAs, and owns the per-lane undo/redo head-change pipeline (`undoLastHeadChange`, `redoLastHeadChange`, `createTag`, `resetToCommit`, `pull` with `ff-only` / `rebase` / `merge` modes). Undo selection is branch-aware: it ignores checkout/undo rows, requires the recorded operation's `metadata.branchRef` to match the lane's current branch, and rechecks the branch before running `reset --hard`. | +| `apps/desktop/src/main/services/git/gitOperationsService.ts` | Brackets every git operation with `operationService.start` / `finish`, captures pre/post HEAD SHAs, and owns the per-lane undo/redo head-change pipeline (`undoLastHeadChange`, `redoLastHeadChange`, `createTag`, `resetToCommit`, `pull` with `ff-only` / `rebase` / `merge` modes). Before lane git mutations or lane git reads, it verifies that the saved `worktreePath` is still the Git top-level for that lane; stale paths that resolve to the primary checkout are rejected as missing lane worktrees so History and Git Actions do not read or mutate the wrong branch. Undo selection is branch-aware: it ignores checkout/undo rows, requires the recorded operation's `metadata.branchRef` to match the lane's current branch, and rechecks the branch before running `reset --hard`. | | `apps/desktop/src/main/services/lanes/laneService.ts` | Lane CRUD now accepts `CreateLaneArgs.startPoint`, used by the Commits view's "Create lane here" affordance to fork a new lane from a specific commit. | | `apps/desktop/src/main/services/prs/prService.ts` | Records PR creation as an operation. | | `apps/desktop/src/main/services/conflicts/conflictService.ts` | Records rebase operations. | @@ -324,6 +324,12 @@ Defined in `apps/desktop/src/shared/ipc.ts`, handled in | `ade.git.undoLastHeadChange` | `GitHeadChangeActionArgs` (`{ laneId }`) | Resets HEAD to the previous successful head-changing op's `preHeadSha`. Fails if HEAD has moved since. Records `git_undo_head_change`. | | `ade.git.redoLastHeadChange` | `GitHeadChangeActionArgs` (`{ laneId }`) | Restores HEAD to the most recent undo's `redoHeadSha`. Fails if HEAD has moved since the undo. Records `git_redo_head_change`. | +All lane-scoped `ade.git.*` reads and mutations validate the lane +worktree root before running provider Git commands. A missing or stale +worktree path throws the same "restore or recreate the lane worktree" +error for history reads, commit-message generation, branch/stash/sync +metadata, and mutating Git actions. + The conflict-resume channels (`gitRebaseContinue`, `gitRebaseAbort`, `gitMergeContinue`, `gitMergeAbort`) accept either a bare `string` laneId (legacy) or `{ laneId }` (preferred); the preload normalizes diff --git a/docs/features/lanes/worktree-isolation.md b/docs/features/lanes/worktree-isolation.md index 1f23ec97e..6c5308a8e 100644 --- a/docs/features/lanes/worktree-isolation.md +++ b/docs/features/lanes/worktree-isolation.md @@ -84,8 +84,10 @@ auto-clean it. the directory first: 1. Fetch the row; reject if `is_edit_protected = 1` (primary). -2. Check worktree dirtiness when a managed worktree exists; dirty - lanes require the caller's force acknowledgement. +2. Check worktree dirtiness only when the saved path still resolves to + that exact Git worktree root. If the directory is missing, or a stale + path under the repo now resolves to the primary checkout, ADE treats + the lane as stale instead of reading the primary worktree's status. 3. Cancel auto-rebase and dismiss rebase suggestions for the lane. 4. Stop ADE-managed processes, PTYs, and file watchers for the lane, then run any lane-environment cleanup supplied by the runtime. @@ -93,7 +95,12 @@ the directory first: run `git worktree remove --force `. If Git reports success but residual files remain, ADE removes the directory with `fs.promises.rm` and runs `git worktree prune` before continuing. - If attached: skip. + If Git already considers the path unregistered, ADE still prunes the + worktree registry and attempts manual residual cleanup. If the path + 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. 6. If caller requested `deleteBranch`: `git branch -D `. Optional remote branch cleanup uses `git push --delete ` and is non-fatal. @@ -111,6 +118,12 @@ unrelated process, PTY, watcher, or environment cleanup. A worktree that has been manually removed from disk but still has a row is repaired by `laneService.removeStaleWorktrees()` at startup. +Status/read paths also verify the saved `worktree_path` with +`git rev-parse --path-format=absolute --show-toplevel` before running +lane-local Git reads. When the top-level is missing or differs from +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. ## Per-lane state directories @@ -139,6 +152,13 @@ present. This matters because: - Stashes, rebases, merges, and cherry-picks are worktree-local — nothing bleeds into other lanes. +- Before mutating a lane or reading history/diff metadata for a lane, + `gitOperationsService` validates that `worktree_path` is still the + Git top-level for that lane. A stale path that now resolves to the + primary repo checkout is treated as a missing lane worktree, so ADE + does not stage, commit, generate commit messages, list commits, + inspect branches/stashes/conflicts, or compute sync state from the + wrong worktree. - `git worktree` detects in-progress merge/rebase state via files in the worktree's gitdir (`rebase-apply/`, `rebase-merge/`, `MERGE_HEAD`). `detectConflictKind` in diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index 97aa8d9fc..05e9cae17 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -592,9 +592,11 @@ See `apps/desktop/src/shared/types/sessions.ts` for the full shape. when the user sends text to an ended agent CLI session and the PTY service opens the transcript in append mode. When the runtime is still live, it submits to that PTY directly. When the PTY is gone, it rebuilds - the provider resume command, creates a new PTY bound to the same durable - session id, and includes the new prompt in that launch command when - resume metadata is available. If another send arrives while the resume + the provider resume command, backfills a missing resume target on + demand when possible (including Codex rollout storage during an active + resume launch), creates a new PTY bound to the same durable session id, + and includes the new prompt in that launch command when resume metadata + is available. If another send arrives while the resume flight is already in progress, that later text is serialized and written after the PTY is attached. This keeps identity, lane association, and transcript history intact without killing the resumed @@ -668,8 +670,8 @@ PTY: | Channel | Purpose | |---|---| | `ade.pty.create` | create or reattach; returns `{ ptyId, sessionId, pid }`. Accepts an optional `chatSessionId` to mark the terminal as chat-owned. | -| `ade.pty.resumeSession` | prompt-free tracked CLI relaunch. Args: `{ sessionId, cols?, rows?, model?, reasoningEffort?, permissionMode? }`. Reuses a live PTY when attached; otherwise validates the row, rebuilds the provider resume command, and spawns a continuation PTY in the same `terminal_sessions` row without writing a prompt. Returns `PtyResumeSessionResult` (`{ ptyId, sessionId, pid, session, resumed, reusedExistingRuntime }`). | -| `ade.pty.sendToSession` | send-or-continue. Args: `{ sessionId, text, cols?, rows?, model?, reasoningEffort?, permissionMode? }`. Submits text into the live PTY when one is attached; otherwise validates that the row is a tracked agent CLI session, rebuilds the resume command via `buildTrackedCliResumeCommand` (honouring runtime overrides), spawns the continuation PTY in the same `terminal_sessions` row, and includes the user's text in the launch command when resume metadata is available. Later sends that land after a resume flight has started are serialized through the agent CLI input protocol: line clear, bracketed paste envelope, chunked 64-byte writes with 5 ms inter-chunk delay, then carriage return with a provider-specific submit delay. Returns `PtySendToSessionResult` (`{ ptyId, sessionId, pid, session, resumed, reusedExistingRuntime }`). | +| `ade.pty.resumeSession` | prompt-free tracked CLI relaunch. Args: `{ sessionId, cols?, rows?, model?, reasoningEffort?, permissionMode? }`. Reuses a live PTY when attached; otherwise validates the row, backfills a missing resume target when possible, rebuilds the provider resume command, and spawns a continuation PTY in the same `terminal_sessions` row without writing a prompt. Returns `PtyResumeSessionResult` (`{ ptyId, sessionId, pid, session, resumed, reusedExistingRuntime }`). | +| `ade.pty.sendToSession` | send-or-continue. Args: `{ sessionId, text, cols?, rows?, model?, reasoningEffort?, permissionMode? }`. Submits text into the live PTY when one is attached; otherwise validates that the row is a tracked agent CLI session, backfills a missing resume target when possible, rebuilds the resume command via `buildTrackedCliResumeCommand` (honouring runtime overrides), spawns the continuation PTY in the same `terminal_sessions` row, and includes the user's text in the launch command when resume metadata is available. Later sends that land after a resume flight has started are serialized through the agent CLI input protocol: line clear, bracketed paste envelope, chunked 64-byte writes with 5 ms inter-chunk delay, then carriage return with a provider-specific submit delay. Returns `PtySendToSessionResult` (`{ ptyId, sessionId, pid, session, resumed, reusedExistingRuntime }`). | | `ade.pty.write` | write bytes to PTY | | `ade.pty.resize` | cols/rows resize | | `ade.pty.dispose` | close PTY; optional `sessionId` used for logging | diff --git a/docs/features/terminals-and-sessions/pty-and-processes.md b/docs/features/terminals-and-sessions/pty-and-processes.md index 72e98f3af..af7ff17a4 100644 --- a/docs/features/terminals-and-sessions/pty-and-processes.md +++ b/docs/features/terminals-and-sessions/pty-and-processes.md @@ -395,7 +395,11 @@ write paths into one call: 2. Otherwise require a tracked agent CLI row (Claude, Codex, Cursor, OpenCode, Droid; chats and shells are rejected with a clear error). Resolve the provider from `resumeMetadata.provider`, fall back to - `providerFromTool(toolType)`. + `providerFromTool(toolType)`. If a Claude/Codex/Droid/OpenCode row + has no concrete target yet, the service runs the same on-demand + resume-target backfill used by `ensureResumeTargets`; Codex can scan + rollout storage during this resume-launch path before ADE reports a + missing target. 3. Rebuild the resume command via `buildTrackedCliResumeCommand(metadata, overrides)` — runtime `model` / `reasoningEffort` / `permissionMode` overrides flow into @@ -567,9 +571,9 @@ resolved. Strategies, in order: drift window. The recovered id becomes `opencode --session `. The Droid storage scan and the OpenCode `session list` invocation only -fire on the `close` / `dispose` reasons (and on demand), not on -`session-list` or `resume-launch`, so renderer list refreshes don't -spawn a `spawnSync` or hit external storage on every render. +fire on the `close` / `dispose` reasons and explicit on-demand +continuation paths, not on `session-list`, so renderer list refreshes +don't spawn a `spawnSync` or hit external storage on every render. Any found ID updates the row's `resumeMetadata.targetId` through `sessionService.updateMeta`. A resume command is always written even @@ -583,15 +587,16 @@ single `pty.resume_target_backfill_failed` warn per failing ID; it never throws. The Codex storage scan is gated by the `reason` argument that the -backfill runs under: `"close"` and `"dispose"` (the regular end-of- -session paths) consult `~/.codex/sessions` with the 10-minute drift -window; `"session-list"` and `"resume-launch"` skip the storage -lookup entirely. Lazy hydration over `sessions.list` therefore relies -only on transcript regex matches, which keeps the list-render hot path -off the disk and prevents one renderer's idle list refresh from -adopting another lane's Codex rollout. Live Codex sessions still get -their resume target through the live capture path, which uses the -strict `requiredText: "ADE session guidance"` gate. +backfill runs under: `"close"` and `"dispose"` consult +`~/.codex/sessions` with the 10-minute drift window; `"session-list"` +skips the storage lookup entirely; `"resume-launch"` is allowed to use +the storage lookup because the user is actively trying to continue that +specific session. Lazy hydration over `sessions.list` therefore relies +only on transcript regex matches, keeping the list-render hot path off +the disk and preventing one renderer's idle refresh from adopting +another lane's Codex rollout. Live Codex sessions still get their +resume target through the live capture path, which uses the strict +`requiredText: "ADE session guidance"` gate. ### Live Codex session-id capture