diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index 3e30e23dc..e5c776e80 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -1943,7 +1943,7 @@ app.whenReady().then(async () => { } }, onDeleteEvent: (event) => emitProjectEvent(projectRoot, IPC.lanesDeleteEvent, event), - onPlacementChanged: (event) => { + onPlacementChanged: async (event) => { // Refresh the VM launch-context cache so subsequent // resolveLaneLaunchContext() calls see the new placement. // TODO(mac-vm-onboarding): emit a renderer-facing IPC event so the @@ -1963,7 +1963,7 @@ app.whenReady().then(async () => { }); } try { - agentChatServiceRef?.handleLanePlacementChanged?.({ + await agentChatServiceRef?.handleLanePlacementChanged?.({ laneId: event.laneId, from: event.from, to: event.to, diff --git a/apps/desktop/src/main/services/ai/tools/orchestrationTools.test.ts b/apps/desktop/src/main/services/ai/tools/orchestrationTools.test.ts index 16b1a868c..b672cf57a 100644 --- a/apps/desktop/src/main/services/ai/tools/orchestrationTools.test.ts +++ b/apps/desktop/src/main/services/ai/tools/orchestrationTools.test.ts @@ -367,6 +367,40 @@ describe("spawnAgent tool", () => { expect(setup.chat.createSession).not.toHaveBeenCalled(); }); + it("does not treat planning phase done as plan approval", async () => { + setup = await setupWithRun("lead"); + const manifest = setup.svc.getManifestForRun(setup.runId)!; + const patched = await setup.svc.manifestPatch( + { + runId: setup.runId, + ifMatchEtag: manifest.etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [ + { + op: "replace", + path: "/phases/{id:planning}/status", + value: "done", + }, + ], + }, + setup.bundlePath, + ); + expect(patched.ok).toBe(false); + + const tools = makeToolSet(setup, "lead", "S-lead"); + const result: any = await tools.spawnAgent!.execute({ + role: "worker", + tag: "backend", + goalSummary: "Implement T-1", + stepId: "T-1", + initialMessage: VALID_BRIEF, + }); + expect(result.ok).toBe(false); + expect(result.error).toBe("plan_not_approved"); + expect(setup.chat.createSession).not.toHaveBeenCalled(); + }); + it.each([ ["codex", { codexSandbox: "danger-full-access", codexApprovalPolicy: "never", codexConfigSource: "flags" }], ["cursor", { cursorModeId: "full-auto" }], @@ -500,6 +534,26 @@ describe("requestPlanApproval and model routing tools", () => { const manifest = setup.svc.getManifestForRun(setup.runId)!; expect(manifest.modelRouting.byRoleTag?.["worker:web-ui"]).toEqual(selection); }); + + it.each([ + ["Not approved — don't start yet", "decline"], + ["No, don't proceed with this plan", "none"], + ["Please revise before we proceed", "decline"], + ] as const)("does not treat rejection text %j as approval when decision is %s", async (answer, decision) => { + setup = await setupWithRun("lead"); + const onAskUser = vi.fn(async () => ({ answer, decision })); + const tools = makeToolSet(setup, "lead", "S-lead", { + universal: { permissionMode: "full-auto", onAskUser }, + }); + const result: any = await tools.requestPlanApproval!.execute({ + planSummary: VALID_APPROVAL_PLAN, + }); + expect(result.ok).toBe(false); + expect(result.error).toBe("plan_rejected"); + const manifest = setup.svc.getManifestForRun(setup.runId)!; + expect(manifest.currentPhase).toBe("planning"); + expect(manifest.leadState.planApprovedAt).toBeUndefined(); + }); }); describe("orchestration heartbeats", () => { diff --git a/apps/desktop/src/main/services/ai/tools/orchestrationTools.ts b/apps/desktop/src/main/services/ai/tools/orchestrationTools.ts index 0946643b3..0efebb7f9 100644 --- a/apps/desktop/src/main/services/ai/tools/orchestrationTools.ts +++ b/apps/desktop/src/main/services/ai/tools/orchestrationTools.ts @@ -744,9 +744,10 @@ function createRequestPlanApprovalTool( const result = typeof response === "string" ? { answer: response, decision: undefined as string | undefined } : response; + // Require an explicit approval decision — do not infer approval from free-text + // answers. Substring regexes false-positive on rejections like "Not approved". const approved = result.decision === "accept" - || result.decision === "accept_for_session" - || /\b(approve|approved|yes|start|proceed)\b/i.test(result.answer ?? ""); + || result.decision === "accept_for_session"; if (!approved) { return { ok: false as const, diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index d2723ea77..e8a4981d0 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -1103,9 +1103,19 @@ function createMockSessionService() { }); }), get: vi.fn((sessionId: string) => sessions.get(sessionId) ?? null), - list: vi.fn((_opts?: any) => - Array.from(sessions.values()), - ), + list: vi.fn((opts?: any) => { + let rows = Array.from(sessions.values()); + if (typeof opts?.laneId === "string") { + rows = rows.filter((row) => row.laneId === opts.laneId); + } + if (typeof opts?.status === "string") { + rows = rows.filter((row) => row.status === opts.status); + } + rows = rows.sort((a, b) => String(b.startedAt ?? "").localeCompare(String(a.startedAt ?? ""))); + if (opts?.limit === null) return rows; + const limit = typeof opts?.limit === "number" ? opts.limit : 200; + return rows.slice(0, limit); + }), reopen: vi.fn((sessionId: string) => { const row = sessions.get(sessionId); if (row) { @@ -2830,6 +2840,199 @@ describe("createAgentChatService", () => { } }); + it("repoints orchestration bundle path when lane placement changes", async () => { + const { orchestrationService, created } = await createLoadedOrchestrationRun("S-lead-placement"); + const movedWorktree = path.join(tmpRoot, "lane-vm-mirror"); + fs.mkdirSync(movedWorktree, { recursive: true }); + try { + const { service, laneService } = createService({ + getOrchestrationService: () => orchestrationService, + }); + const session = await service.createSession({ + laneId: "lane-1", + provider: "claude", + model: "sonnet", + modelId: "anthropic/claude-sonnet-4-6", + interactionMode: "orchestrator-lead", + orchestrationRunId: created.runId, + orchestrationRole: "lead", + orchestrationBundlePath: created.manifest.bundlePath, + }); + + const lanes = await laneService.list(); + const lane1 = lanes.find((entry: { id: string }) => entry.id === "lane-1"); + expect(lane1).toBeTruthy(); + lane1.worktreePath = movedWorktree; + vi.mocked(laneService.getLaneBaseAndBranch).mockImplementation((nextLaneId: string) => { + const lane = lanes.find((entry: { id: string }) => entry.id === nextLaneId); + if (!lane) { + return { + baseRef: "main", + branchRef: "feature/selected", + worktreePath: tmpRoot, + laneType: "feature", + runtimePlacement: "local", + }; + } + return { + baseRef: "main", + branchRef: lane.branchRef, + worktreePath: lane.worktreePath, + laneType: lane.laneType, + runtimePlacement: "local", + }; + }); + + await service.handleLanePlacementChanged({ + laneId: "lane-1", + from: "macos-vm", + to: "local", + }); + + const expectedBundlePath = path.join( + fs.realpathSync(movedWorktree), + ".ade", + "orchestration", + created.runId, + ); + expect(readPersistedChatState(session.id).orchestrationBundlePath).toBe(expectedBundlePath); + expect(orchestrationService.getBundlePathForRun(created.runId)).toBe(expectedBundlePath); + } finally { + await orchestrationService.dispose(); + } + }); + + it("repoints persisted orchestration bundle paths for cold sessions when lane placement changes", async () => { + const { orchestrationService, created } = await createLoadedOrchestrationRun("S-cold-placement"); + const movedWorktree = path.join(tmpRoot, "lane-vm-mirror-cold"); + fs.mkdirSync(movedWorktree, { recursive: true }); + try { + const { service, laneService } = createService({ + getOrchestrationService: () => orchestrationService, + }); + const session = await service.createSession({ + laneId: "lane-1", + provider: "claude", + model: "sonnet", + modelId: "anthropic/claude-sonnet-4-6", + interactionMode: "orchestrator-lead", + orchestrationRunId: created.runId, + orchestrationRole: "lead", + orchestrationBundlePath: created.manifest.bundlePath, + }); + await service.dispose({ sessionId: session.id }); + + const lanes = await laneService.list(); + const lane1 = lanes.find((entry: { id: string }) => entry.id === "lane-1"); + expect(lane1).toBeTruthy(); + lane1.worktreePath = movedWorktree; + vi.mocked(laneService.getLaneBaseAndBranch).mockImplementation((nextLaneId: string) => { + const lane = lanes.find((entry: { id: string }) => entry.id === nextLaneId); + if (!lane) { + return { + baseRef: "main", + branchRef: "feature/selected", + worktreePath: tmpRoot, + laneType: "feature", + runtimePlacement: "local", + }; + } + return { + baseRef: "main", + branchRef: lane.branchRef, + worktreePath: lane.worktreePath, + laneType: lane.laneType, + runtimePlacement: "local", + }; + }); + + await service.handleLanePlacementChanged({ + laneId: "lane-1", + from: "macos-vm", + to: "local", + }); + + const expectedBundlePath = path.join( + fs.realpathSync(movedWorktree), + ".ade", + "orchestration", + created.runId, + ); + expect(readPersistedChatState(session.id).orchestrationBundlePath).toBe(expectedBundlePath); + expect(orchestrationService.getBundlePathForRun(created.runId)).toBe(expectedBundlePath); + } finally { + await orchestrationService.dispose(); + } + }); + + it("repoints cold orchestration bundle paths beyond the newest 500 sessions", async () => { + const { orchestrationService, created } = await createLoadedOrchestrationRun("S-cold-placement-old"); + const movedWorktree = path.join(tmpRoot, "lane-vm-mirror-cold-old"); + fs.mkdirSync(movedWorktree, { recursive: true }); + try { + const { service, laneService } = createService({ + getOrchestrationService: () => orchestrationService, + }); + const session = await service.createSession({ + laneId: "lane-1", + provider: "claude", + model: "sonnet", + modelId: "anthropic/claude-sonnet-4-6", + interactionMode: "orchestrator-lead", + orchestrationRunId: created.runId, + orchestrationRole: "lead", + orchestrationBundlePath: created.manifest.bundlePath, + }); + await service.dispose({ sessionId: session.id }); + const coldRow = mockState.sessions.get(session.id); + if (!coldRow) throw new Error("expected cold session row"); + coldRow.startedAt = "2020-01-01T00:00:00.000Z"; + for (let i = 0; i < 501; i++) { + mockState.sessions.set(`S-newer-${i}`, { + ...coldRow, + id: `S-newer-${i}`, + title: `Newer session ${i}`, + toolType: "codex-chat", + status: "ended", + startedAt: new Date(Date.UTC(2026, 0, 1, 0, 0, i)).toISOString(), + endedAt: new Date(Date.UTC(2026, 0, 1, 0, 1, i)).toISOString(), + }); + } + + const lanes = await laneService.list(); + const lane1 = lanes.find((entry: { id: string }) => entry.id === "lane-1"); + expect(lane1).toBeTruthy(); + lane1.worktreePath = movedWorktree; + vi.mocked(laneService.getLaneBaseAndBranch).mockImplementation((nextLaneId: string) => { + const lane = lanes.find((entry: { id: string }) => entry.id === nextLaneId); + return { + baseRef: "main", + branchRef: lane?.branchRef ?? "feature/selected", + worktreePath: lane?.worktreePath ?? tmpRoot, + laneType: lane?.laneType ?? "feature", + runtimePlacement: "local", + }; + }); + + await service.handleLanePlacementChanged({ + laneId: "lane-1", + from: "macos-vm", + to: "local", + }); + + const expectedBundlePath = path.join( + fs.realpathSync(movedWorktree), + ".ade", + "orchestration", + created.runId, + ); + expect(readPersistedChatState(session.id).orchestrationBundlePath).toBe(expectedBundlePath); + expect(orchestrationService.getBundlePathForRun(created.runId)).toBe(expectedBundlePath); + } finally { + await orchestrationService.dispose(); + } + }); + it("attaches ADE orchestration tools to OpenCode orchestrator sessions through MCP", async () => { vi.mocked(streamText).mockReturnValue({ fullStream: (async function* () { diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index aa88a9ad7..67de1141c 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -7187,6 +7187,26 @@ export function createAgentChatService(args: { return launchContext; }; + const orchestrationBundlePathForRun = (worktreePath: string, runId: string): string => + path.join(worktreePath, ".ade", "orchestration", runId); + + const relocateOrchestrationRunBundle = async ( + runId: string, + bundlePath: string, + sessionId: string, + ): Promise => { + try { + await getOrchestrationService?.()?.relocateRunBundle(runId, bundlePath); + } catch (error) { + logger.warn("agent_chat.orchestration_bundle_relocate_failed", { + runId, + sessionId, + bundlePath, + error: error instanceof Error ? error.message : String(error), + }); + } + }; + const resolvePrimaryIdentityLane = async (): Promise => { await laneService.ensurePrimaryLane?.().catch(() => {}); const lanes = await laneService.list({ includeArchived: false, includeStatus: false }); @@ -23787,13 +23807,26 @@ export function createAgentChatService(args: { * the lane and emits a system notice so the chat banner can flip between * "Running locally" and "Running inside Mac VM" without a reload. */ - const handleLanePlacementChanged = (event: { + const handleLanePlacementChanged = async (event: { laneId: string; from: "macos-vm" | "local" | "none"; to: "macos-vm" | "local"; - }): void => { + }): Promise => { const laneId = String(event?.laneId ?? "").trim(); if (!laneId.length) return; + const relocations: Promise[] = []; + const changedLane = (() => { + try { + return laneService.getLaneBaseAndBranch(laneId); + } catch { + return null; + } + })(); + const changedLaneWorktreePathRaw = trimLine(changedLane?.worktreePath); + const changedLaneWorktreePath = changedLaneWorktreePathRaw + ? safeRealpath(changedLaneWorktreePathRaw) ?? changedLaneWorktreePathRaw + : null; + const touchedSessionIds = new Set(); for (const managed of managedSessions.values()) { const candidateLaneIds = [ managed.session.laneId, @@ -23801,6 +23834,7 @@ export function createAgentChatService(args: { managed.selectedExecutionLaneId, ]; if (!candidateLaneIds.includes(laneId)) continue; + touchedSessionIds.add(managed.session.id); try { refreshManagedLaneLaunchContext(managed, { purpose: "follow lane placement change" }); } catch (error) { @@ -23812,6 +23846,17 @@ export function createAgentChatService(args: { error: error instanceof Error ? error.message : String(error), }); } + const runId = managed.session.orchestrationRunId?.trim(); + const worktree = managed.laneWorktreePath?.trim(); + if (runId && worktree) { + const nextBundlePath = orchestrationBundlePathForRun(worktree, runId); + const currentBundlePath = managed.session.orchestrationBundlePath?.trim(); + if (currentBundlePath !== nextBundlePath) { + managed.session.orchestrationBundlePath = nextBundlePath; + persistChatState(managed); + relocations.push(relocateOrchestrationRunBundle(runId, nextBundlePath, managed.session.id)); + } + } const message = event.to === "local" ? "Lane detached from Mac VM; further turns run locally." : "Lane attached to Mac VM; further turns run inside the VM at /Volumes/My Shared Files."; @@ -23829,6 +23874,38 @@ export function createAgentChatService(args: { }); } } + if (changedLaneWorktreePath) { + const rows = sessionService.list({ laneId, limit: null }); + for (const row of rows) { + if (!isChatToolType(row.toolType) || touchedSessionIds.has(row.id)) continue; + const persisted = readPersistedState(row.id); + if (!persisted) continue; + const runId = persisted.orchestrationRunId?.trim(); + if (!runId) continue; + const executionLaneId = + trimLine(persisted.preferredExecutionLaneId) + ?? trimLine(persisted.selectedExecutionLaneId) + ?? row.laneId; + if (executionLaneId !== laneId) continue; + const nextBundlePath = orchestrationBundlePathForRun(changedLaneWorktreePath, runId); + if (persisted.orchestrationBundlePath === nextBundlePath) continue; + const metadataPath = metadataPathFor(row.id); + try { + const raw = JSON.parse(fs.readFileSync(metadataPath, "utf8")) as Record; + raw.orchestrationBundlePath = nextBundlePath; + raw.updatedAt = nowIso(); + fs.writeFileSync(metadataPath, JSON.stringify(raw, null, 2), "utf8"); + relocations.push(relocateOrchestrationRunBundle(runId, nextBundlePath, row.id)); + } catch (error) { + logger.warn("agent_chat.persisted_orchestration_bundle_repoint_failed", { + laneId, + sessionId: row.id, + error: error instanceof Error ? error.message : String(error), + }); + } + } + } + await Promise.all(relocations); }; const readTranscript = async ( diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index 4745e891e..b099f4ac8 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -878,7 +878,7 @@ export function createLaneService({ onHeadChanged?: (args: { laneId: string; reason: string; preHeadSha: string | null; postHeadSha: string | null }) => void; onRebaseEvent?: (event: RebaseRunEventPayload) => void; onDeleteEvent?: (event: LaneDeleteEvent) => void; - onPlacementChanged?: (event: LanePlacementChangedEvent) => void; + onPlacementChanged?: (event: LanePlacementChangedEvent) => void | Promise; onLinearIssueLinked?: (args: { lane: LaneSummary; issue: LaneLinearIssue; linkedAt: string }) => void | Promise; teardownDeps?: LaneDeleteTeardownDeps; macosVmHooks?: LaneMacosVmHooks | null; @@ -893,10 +893,10 @@ export function createLaneService({ let activeMacosVmHooks: LaneMacosVmHooks | null = macosVmHooks ?? null; - const emitPlacementChanged = (event: LanePlacementChangedEvent): void => { + const emitPlacementChanged = async (event: LanePlacementChangedEvent): Promise => { if (!onPlacementChanged) return; try { - onPlacementChanged(event); + await onPlacementChanged(event); } catch (error) { logger.warn("laneService.placement_changed_emit_failed", { laneId: event.laneId, @@ -2168,7 +2168,7 @@ export function createLaneService({ } if (args.previousPlacement !== "macos-vm") { - emitPlacementChanged({ + await emitPlacementChanged({ type: "lane-placement-changed", laneId, from: args.previousPlacement, @@ -4579,7 +4579,7 @@ export function createLaneService({ }); } - emitPlacementChanged({ + await emitPlacementChanged({ type: "lane-placement-changed", laneId: row.id, from: "macos-vm", diff --git a/apps/desktop/src/main/services/orchestration/orchestrationService.test.ts b/apps/desktop/src/main/services/orchestration/orchestrationService.test.ts index 438db49ac..2491e81f4 100644 --- a/apps/desktop/src/main/services/orchestration/orchestrationService.test.ts +++ b/apps/desktop/src/main/services/orchestration/orchestrationService.test.ts @@ -1,5 +1,5 @@ /* @vitest-environment node */ -import { describe, expect, it, beforeEach, afterEach } from "vitest"; +import { describe, expect, it, beforeEach, afterEach, vi } from "vitest"; import { promises as fsp } from "node:fs"; import path from "node:path"; import os from "node:os"; @@ -238,6 +238,175 @@ describe("orchestrationService", () => { await restarted.dispose(); }); + it("relocates a cached runtime and writes subsequent changes to the moved bundle", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const movedWorktree = path.join(lane, "vm-mirror-worktree"); + const { runId, manifest } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Placement move", + }); + const movedBundlePath = path.join(movedWorktree, ".ade", "orchestration", runId); + await fsp.mkdir(path.dirname(movedBundlePath), { recursive: true }); + await fsp.cp(manifest.bundlePath, movedBundlePath, { recursive: true }); + + expect(svc.getBundlePathForRun(runId)).toBe(manifest.bundlePath); + await svc.subscribe(runId, manifest.bundlePath); + await svc.relocateRunBundle(runId, movedBundlePath); + expect(svc.getBundlePathForRun(runId)).toBe(movedBundlePath); + + const loaded = await svc.bundleRead(runId, movedBundlePath); + const patched = await svc.manifestPatch( + { + runId, + ifMatchEtag: loaded.manifest.etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [{ op: "replace", path: "/title", value: "Moved write" }], + }, + movedBundlePath, + ); + expect(patched.ok).toBe(true); + if (!patched.ok) return; + expect(patched.manifest.bundlePath).toBe(movedBundlePath); + + const movedManifest = JSON.parse( + await fsp.readFile(path.join(movedBundlePath, "manifest.json"), "utf-8"), + ) as { title: string; bundlePath: string }; + const originalManifest = JSON.parse( + await fsp.readFile(path.join(manifest.bundlePath, "manifest.json"), "utf-8"), + ) as { title: string }; + expect(movedManifest.title).toBe("Moved write"); + expect(movedManifest.bundlePath).toBe(movedBundlePath); + expect(originalManifest.title).toBe("Placement move"); + await new Promise((resolve) => setTimeout(resolve, 1_100)); + const externallyUpdated = { + ...movedManifest, + title: "External moved update", + serverGeneration: patched.manifest.serverGeneration + 1, + etag: `g${patched.manifest.serverGeneration + 1}-external`, + }; + await fsp.writeFile( + path.join(movedBundlePath, "manifest.json"), + JSON.stringify(externallyUpdated, null, 2), + ); + for (let attempt = 0; attempt < 20; attempt += 1) { + if (svc.getManifestForRun(runId)?.title === "External moved update") break; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + expect(svc.getManifestForRun(runId)?.title).toBe("External moved update"); + await svc.release(runId); + await svc.dispose(); + }); + + it("does not create a runtime when relocating an unsubscribed run", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + await svc.relocateRunBundle("R-not-loaded", path.join(lane, "moved", ".ade", "orchestration", "R-not-loaded")); + expect(svc.getBundlePathForRun("R-not-loaded")).toBeNull(); + await svc.dispose(); + }); + + it("relocates a cached runtime even when no subscriber is attached", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const { runId, manifest } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Cached placement move", + }); + const movedBundlePath = path.join(lane, "vm-mirror-cached", ".ade", "orchestration", runId); + await fsp.mkdir(path.dirname(movedBundlePath), { recursive: true }); + await fsp.cp(manifest.bundlePath, movedBundlePath, { recursive: true }); + + await svc.relocateRunBundle(runId, movedBundlePath); + + expect(svc.getBundlePathForRun(runId)).toBe(movedBundlePath); + await svc.dispose(); + }); + + it("does not move a relocated runtime back when a stale caller passes the old bundle path", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const { runId, manifest } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Stale path move", + }); + const movedBundlePath = path.join(lane, "vm-mirror-stale-caller", ".ade", "orchestration", runId); + await fsp.mkdir(path.dirname(movedBundlePath), { recursive: true }); + await fsp.cp(manifest.bundlePath, movedBundlePath, { recursive: true }); + await svc.relocateRunBundle(runId, movedBundlePath); + + const loaded = await svc.bundleRead(runId, movedBundlePath); + const patched = await svc.manifestPatch( + { + runId, + ifMatchEtag: loaded.etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [{ op: "replace", path: "/title", value: "Moved write from stale caller" }], + }, + manifest.bundlePath, + ); + + expect(patched.ok).toBe(true); + expect(svc.getBundlePathForRun(runId)).toBe(movedBundlePath); + const movedManifest = JSON.parse( + await fsp.readFile(path.join(movedBundlePath, "manifest.json"), "utf-8"), + ) as { title: string }; + const originalManifest = JSON.parse( + await fsp.readFile(path.join(manifest.bundlePath, "manifest.json"), "utf-8"), + ) as { title: string }; + expect(movedManifest.title).toBe("Moved write from stale caller"); + expect(originalManifest.title).toBe("Stale path move"); + await svc.dispose(); + }); + + it("keeps manifest and generation writes on the same bundle during concurrent relocation", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const { runId, manifest } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Concurrent placement move", + }); + await svc.subscribe(runId, manifest.bundlePath); + const manifestPath = path.join(manifest.bundlePath, "manifest.json"); + const movedBundlePath = path.join(lane, "vm-mirror-worktree-late", ".ade", "orchestration", runId); + const originalRename = fsp.rename.bind(fsp); + let relocatePromise: Promise | null = null; + const renameSpy = vi.spyOn(fsp, "rename").mockImplementation((async (from: any, to: any) => { + await originalRename(from, to); + if (!relocatePromise && path.resolve(String(to)) === path.resolve(manifestPath)) { + relocatePromise = svc.relocateRunBundle(runId, movedBundlePath); + } + }) as any); + try { + const patched = await svc.manifestPatch( + { + runId, + ifMatchEtag: manifest.etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [{ op: "replace", path: "/title", value: "Concurrent write" }], + }, + manifest.bundlePath, + ); + expect(patched.ok).toBe(true); + if (!patched.ok) return; + const originalGen = await fsp.readFile(path.join(manifest.bundlePath, ".gen"), "utf-8"); + expect(Number.parseInt(originalGen.trim(), 10)).toBe(patched.manifest.serverGeneration); + } finally { + renameSpy.mockRestore(); + } + if (relocatePromise) { + await relocatePromise; + } + await svc.release(runId); + await svc.dispose(); + }); + it("rejects mismatched etag on manifestPatch", async () => { const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); const { manifest } = await svc.runCreate({ @@ -1070,6 +1239,161 @@ describe("orchestration watcher resilience", () => { await svc.dispose(); }); + it("blocks manifest writes after an external manifest runId swap", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const { manifest, etag } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Original run", + }); + const manifestPath = path.join(manifest.bundlePath, "manifest.json"); + const foreign = { + ...JSON.parse(await fsp.readFile(manifestPath, "utf-8")), + runId: "R-foreign-checkout", + etag: "etag-foreign", + title: "Foreign branch manifest", + }; + await fsp.writeFile(manifestPath, JSON.stringify(foreign, null, 2)); + + const patch = await svc.manifestPatch( + { + runId: manifest.runId, + ifMatchEtag: etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [{ op: "replace", path: "/title", value: "Stale write attempt" }], + }, + manifest.bundlePath, + ); + expect(patch.ok).toBe(false); + if (patch.ok) return; + expect(patch.error).toBe("validation_failed"); + if (patch.error !== "validation_failed") return; + expect(patch.message).toContain("suspended"); + + const heartbeat = await svc.agentHeartbeat( + { runId: manifest.runId, sessionId: "S-lead" }, + manifest.bundlePath, + ); + expect(heartbeat.ok).toBe(false); + if (!heartbeat.ok) { + expect(heartbeat.reason).toContain("suspended"); + } + + const onDisk = JSON.parse(await fsp.readFile(manifestPath, "utf-8")); + expect(onDisk.runId).toBe("R-foreign-checkout"); + expect(onDisk.title).toBe("Foreign branch manifest"); + await svc.dispose(); + }); + + it("cleans failed temp writes and emits suspension after an in-flight runId swap", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const { manifest, etag } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Original run", + }); + const events: any[] = []; + const off = svc.on("event", (payload) => events.push(payload)); + await svc.subscribe(manifest.runId, manifest.bundlePath); + const manifestPath = path.join(manifest.bundlePath, "manifest.json"); + const originalReadFile = fsp.readFile.bind(fsp); + const foreign = { + ...JSON.parse(await originalReadFile(manifestPath, "utf-8")), + runId: "R-foreign-mid-commit", + etag: "etag-foreign-mid-commit", + title: "Foreign branch manifest", + }; + let manifestReadCount = 0; + let injectedForeignManifest = false; + const readSpy = vi.spyOn(fsp, "readFile").mockImplementation((async (file: any, options?: any) => { + if ( + !injectedForeignManifest + && path.resolve(String(file)) === path.resolve(manifestPath) + && options === "utf-8" + ) { + manifestReadCount++; + if (manifestReadCount === 2) { + injectedForeignManifest = true; + await fsp.writeFile(manifestPath, JSON.stringify(foreign, null, 2)); + } + } + return originalReadFile(file, options); + }) as any); + try { + const patch = await svc.manifestPatch( + { + runId: manifest.runId, + ifMatchEtag: etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [{ op: "replace", path: "/title", value: "Stale write attempt" }], + }, + manifest.bundlePath, + ); + expect(patch.ok).toBe(false); + if (patch.ok) return; + expect(patch.error).toBe("validation_failed"); + } finally { + readSpy.mockRestore(); + } + + await vi.waitFor(() => { + expect(events.some((event) => + event.runId === manifest.runId + && event.kind === "lifecycle" + && event.status === "suspended", + )).toBe(true); + }); + const files = await fsp.readdir(manifest.bundlePath); + expect(files.filter((entry) => entry.endsWith(".tmp"))).toEqual([]); + off(); + await svc.dispose(); + }); + + it("returns etag_conflict instead of overwriting a newer on-disk manifest", async () => { + const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); + const { manifest } = await svc.runCreate({ + laneId: "L-1", + leadSessionId: "S-lead", + bundleRoot: lane, + title: "Initial", + }); + const manifestPath = path.join(manifest.bundlePath, "manifest.json"); + const external = { + ...JSON.parse(await fsp.readFile(manifestPath, "utf-8")), + title: "external-title", + serverGeneration: manifest.serverGeneration + 1, + etag: `g${manifest.serverGeneration + 1}-external`, + }; + await fsp.writeFile(manifestPath, JSON.stringify(external, null, 2)); + + const patchRes = await svc.manifestPatch( + { + runId: manifest.runId, + ifMatchEtag: manifest.etag, + actorRole: "lead", + actorSessionId: "S-lead", + patches: [{ op: "replace", path: "/title", value: "patched-title" }], + }, + manifest.bundlePath, + ); + + expect(patchRes.ok).toBe(false); + if (patchRes.ok) return; + expect(patchRes.error).toBe("etag_conflict"); + + const onDisk = JSON.parse(await fsp.readFile(manifestPath, "utf-8")) as { + title: string; + serverGeneration: number; + }; + expect(onDisk.title).toBe("external-title"); + expect(onDisk.serverGeneration).toBe(manifest.serverGeneration + 1); + await svc.dispose(); + }); + it("planAppend produces an event with the new contents", async () => { const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); const { manifest } = await svc.runCreate({ diff --git a/apps/desktop/src/main/services/orchestration/orchestrationService.ts b/apps/desktop/src/main/services/orchestration/orchestrationService.ts index fcc885085..63707a954 100644 --- a/apps/desktop/src/main/services/orchestration/orchestrationService.ts +++ b/apps/desktop/src/main/services/orchestration/orchestrationService.ts @@ -60,6 +60,15 @@ class AsyncMutex { } } +class OrchestrationPersistConflictError extends Error { + readonly onDisk: OrchestrationManifest; + constructor(onDisk: OrchestrationManifest) { + super("manifest on disk is newer than the in-flight write"); + this.name = "OrchestrationPersistConflictError"; + this.onDisk = onDisk; + } +} + const MANIFEST_FILE = "manifest.json"; const PLAN_FILE = "plan.md"; const GEN_FILE = ".gen"; @@ -71,6 +80,15 @@ const WATCHER_IDLE_CLOSE_MS = 30_000; const ORCHESTRATION_INDEX_VERSION = 1; const RUN_LIST_DEFAULT_LIMIT = 100; const RUN_LIST_MAX_LIMIT = 250; +const RUN_SUSPENDED_MESSAGE = + "orchestration run is suspended (bundle changed externally); re-open the run or restore the correct branch"; + +class OrchestrationRunSuspendedError extends Error { + constructor() { + super(RUN_SUSPENDED_MESSAGE); + this.name = "OrchestrationRunSuspendedError"; + } +} export type OrchestrationServiceEvents = { event: (payload: OrchestrationEventPayload) => void; @@ -166,7 +184,11 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { await atomicWrite(genPath, `${gen}\n`); } - async function atomicWrite(target: string, contents: string): Promise { + async function atomicWrite( + target: string, + contents: string, + options?: { beforeCommit?: () => Promise }, + ): Promise { const dir = path.dirname(target); await fsp.mkdir(dir, { recursive: true }); const tmp = `${target}.${process.pid}.${Date.now()}.${Math.random() @@ -179,7 +201,15 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { } finally { await handle.close(); } - await fsp.rename(tmp, target); + try { + if (options?.beforeCommit) { + await options.beforeCommit(); + } + await fsp.rename(tmp, target); + } catch (err) { + await fsp.unlink(tmp).catch(() => undefined); + throw err; + } } function getLaneIndexMutex(laneId: string): AsyncMutex { @@ -407,6 +437,27 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { .join(","); } + function relocateRuntimeBundlePath(runtime: RunRuntime, bundlePath: string): void { + if (runtime.bundlePath === bundlePath) return; + if (runtime.watcher) { + void runtime.watcher.close().catch(() => undefined); + runtime.watcher = null; + } + if (runtime.watcherDebounceTimer) { + clearTimeout(runtime.watcherDebounceTimer); + runtime.watcherDebounceTimer = null; + } + if (runtime.watcherIdleTimer) { + clearTimeout(runtime.watcherIdleTimer); + runtime.watcherIdleTimer = null; + } + runtime.bundlePath = bundlePath; + runtime.manifest = null; + runtime.planMd = null; + runtime.recentSelfWriteUntil = 0; + runtime.suspended = false; + } + function getOrCreateRuntime( runId: string, bundlePath: string, @@ -427,6 +478,7 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { suspended: false, }; runs.set(runId, runtime); + return runtime; } return runtime; } @@ -444,9 +496,10 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { ); } if (manifest.runId !== runtime.runId) { - throw new Error( - `manifest.runId ${manifest.runId} does not match expected ${runtime.runId}`, - ); + runtime.suspended = true; + runtime.manifest = null; + runtime.planMd = null; + return; } runtime.manifest = normalizeManifestShape(manifest); } catch (err) { @@ -517,10 +570,12 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { } runtime.watcherDebounceTimer = setTimeout(() => { runtime.watcherDebounceTimer = null; - if (Date.now() < runtime.recentSelfWriteUntil) { - return; // suppress self-emitted events - } - void handleExternalChange(runtime, kind); + void runtime.mutex.run(async () => { + if (Date.now() < runtime.recentSelfWriteUntil) { + return; // suppress self-emitted events + } + await handleExternalChange(runtime, kind); + }); }, WATCHER_DEBOUNCE_MS); }; watcher.on("change", (full) => { @@ -560,6 +615,8 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { // file), do not blindly etag-bump; mark suspended and ignore. if (next.runId !== runtime.runId) { runtime.suspended = true; + runtime.manifest = null; + runtime.planMd = null; emit({ runId: runtime.runId, kind: "lifecycle", @@ -603,10 +660,54 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { runtime: RunRuntime, manifest: OrchestrationManifest, ): Promise { - const manifestPath = path.join(runtime.bundlePath, MANIFEST_FILE); + const bundlePath = runtime.bundlePath; + const manifestPath = path.join(bundlePath, MANIFEST_FILE); + manifest.bundlePath = bundlePath; + const expectedBaseGeneration = + runtime.manifest?.serverGeneration ?? manifest.serverGeneration - 1; + const expectedBaseEtag = runtime.manifest?.etag; + const rejectIfDiskAdvanced = async (): Promise => { + try { + const raw = await fsp.readFile(manifestPath, "utf-8"); + const onDisk = normalizeManifestShape(JSON.parse(raw) as OrchestrationManifest); + if (onDisk.runId !== runtime.runId) { + runtime.suspended = true; + runtime.manifest = null; + runtime.planMd = null; + throw new OrchestrationRunSuspendedError(); + } + if ( + onDisk.runId === runtime.runId && + onDisk.serverGeneration > expectedBaseGeneration && + onDisk.etag !== expectedBaseEtag + ) { + runtime.manifest = onDisk; + throw new OrchestrationPersistConflictError(onDisk); + } + } catch (err) { + if ( + err instanceof OrchestrationPersistConflictError + || err instanceof OrchestrationRunSuspendedError + ) { + throw err; + } + const e = err as NodeJS.ErrnoException; + if (e?.code !== "ENOENT") { + throw err; + } + } + }; + await rejectIfDiskAdvanced(); markSelfWrite(runtime); - await atomicWrite(manifestPath, JSON.stringify(manifest, null, 2)); - await writeServerGeneration(runtime.bundlePath, manifest.serverGeneration); + try { + await atomicWrite(manifestPath, JSON.stringify(manifest, null, 2), { + beforeCommit: rejectIfDiskAdvanced, + }); + } catch (err) { + runtime.recentSelfWriteUntil = 0; + throw err; + } + await writeServerGeneration(bundlePath, manifest.serverGeneration); runtime.manifest = manifest; await appendRunIndexEntry( manifest.laneId, @@ -621,6 +722,12 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { runtime.planMd = plan; } + function assertRunWritable(runtime: RunRuntime): void { + if (runtime.suspended) { + throw new OrchestrationRunSuspendedError(); + } + } + // -------------------------------------------------------------------------- // Public API // -------------------------------------------------------------------------- @@ -726,6 +833,13 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + if (runtime.suspended) { + return { + ok: false, + error: "validation_failed", + message: RUN_SUSPENDED_MESSAGE, + }; + } const current = runtime.manifest; if (!current) { return { @@ -825,7 +939,27 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { ]; next.history = ring; - await persistManifest(runtime, next); + try { + await persistManifest(runtime, next); + } catch (err) { + if (err instanceof OrchestrationRunSuspendedError) { + return { + ok: false, + error: "validation_failed", + message: RUN_SUSPENDED_MESSAGE, + }; + } + if (err instanceof OrchestrationPersistConflictError) { + const latest = runtime.manifest ?? err.onDisk; + return { + ok: false, + error: "etag_conflict", + manifest: latest, + etag: latest.etag, + }; + } + throw err; + } emit({ runId: req.runId, kind: "manifest", @@ -844,6 +978,7 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + assertRunWritable(runtime); if (!runtime.manifest) throw new Error(`run ${req.runId} not found`); const prev = runtime.planMd ?? ""; const heading = req.section.startsWith("#") @@ -871,6 +1006,7 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + assertRunWritable(runtime); if (!runtime.manifest) throw new Error(`run ${req.runId} not found`); if (runtime.manifest.etag !== req.ifMatchEtag) { return { error: "etag_conflict", etag: runtime.manifest.etag }; @@ -895,6 +1031,7 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + assertRunWritable(runtime); if (!runtime.manifest) throw new Error(`run ${req.runId} not found`); const id = `A-${runtime.manifest.assets.length + 1}-${shortRand()}`; const asset: OrchestrationAsset = { @@ -930,7 +1067,11 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); - const manifest = runtime.manifest!; + assertRunWritable(runtime); + if (!runtime.manifest) { + throw new Error(`run ${req.runId} not found`); + } + const manifest = runtime.manifest; const registeredAgent = manifest.agents.find( (agent) => agent.sessionId === req.sessionId, ); @@ -1009,6 +1150,7 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + assertRunWritable(runtime); const manifest = runtime.manifest; if (!manifest) throw new Error(`run ${req.runId} not found`); const task = manifest.tasks.find((entry) => entry.id === req.taskId); @@ -1109,6 +1251,13 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + if (runtime.suspended) { + return { + ok: false, + error: "validation_failed", + message: RUN_SUSPENDED_MESSAGE, + }; + } const manifest = runtime.manifest; if (!manifest) { return { @@ -1240,11 +1389,23 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { }, }, ]; - const patchRes = await directPatch( - runtime, - patches, - `record validation ${req.taskId}/${req.stepId} ${req.status}`, - ); + let patchRes: { manifest: OrchestrationManifest; etag: string }; + try { + patchRes = await directPatch( + runtime, + patches, + `record validation ${req.taskId}/${req.stepId} ${req.status}`, + ); + } catch (err) { + if (err instanceof OrchestrationRunSuspendedError) { + return { + ok: false, + error: "validation_failed", + message: RUN_SUSPENDED_MESSAGE, + }; + } + throw err; + } return { ok: true, manifest: patchRes.manifest, @@ -1262,6 +1423,9 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(req.runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + if (runtime.suspended) { + return { ok: false, reason: RUN_SUSPENDED_MESSAGE }; + } const manifest = runtime.manifest; if (!manifest) return { ok: false, reason: `run ${req.runId} not found` }; if (!manifest.agents.some((agent) => agent.sessionId === req.sessionId)) { @@ -1272,7 +1436,21 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { if (agent) agent.lastHeartbeatAt = nowIso(); // Heartbeats are liveness metadata. They must not invalidate the optimistic // concurrency etag that agents use for the next manifest mutation. - await persistManifest(runtime, next); + try { + await persistManifest(runtime, next); + } catch (err) { + if (err instanceof OrchestrationRunSuspendedError) { + return { ok: false, reason: RUN_SUSPENDED_MESSAGE }; + } + if (err instanceof OrchestrationPersistConflictError) { + return { + ok: false, + reason: "etag_conflict", + etag: err.onDisk.etag, + }; + } + throw err; + } emit({ runId: req.runId, kind: "manifest", @@ -1396,6 +1574,7 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { patches: readonly ManifestPatchOp[], summary: string, ): Promise<{ manifest: OrchestrationManifest; etag: string }> { + assertRunWritable(runtime); if (!runtime.manifest) throw new Error("manifest not loaded"); const next = normalizeManifestShape(applyPatches(runtime.manifest, patches)); const updatedAt = nowIso(); @@ -1434,16 +1613,46 @@ export function createOrchestrationService(deps: OrchestrationServiceDeps) { const runtime = getOrCreateRuntime(runId, bundlePath); return runtime.mutex.run(async () => { await loadIntoRuntime(runtime); + if (runtime.suspended) { + return { ok: false, error: "run_suspended", message: RUN_SUSPENDED_MESSAGE }; + } if (!runtime.manifest) { return { ok: false, error: "run_not_found", message: `run ${runId} not found` }; } - const result = await directPatch(runtime, patches, summary); - return { ok: true, manifest: result.manifest, etag: result.etag }; + try { + const result = await directPatch(runtime, patches, summary); + return { ok: true, manifest: result.manifest, etag: result.etag }; + } catch (err) { + if (err instanceof OrchestrationRunSuspendedError) { + return { ok: false, error: "run_suspended", message: RUN_SUSPENDED_MESSAGE }; + } + if (err instanceof OrchestrationPersistConflictError) { + return { + ok: false, + error: "etag_conflict", + message: `manifest on disk advanced to generation ${err.onDisk.serverGeneration}`, + }; + } + throw err; + } + }); + } + + async function relocateRunBundle(runId: string, bundlePath: string): Promise { + const runtime = runs.get(runId); + if (!runtime) return; + await runtime.mutex.run(async () => { + relocateRuntimeBundlePath(runtime, bundlePath); + await loadIntoRuntime(runtime); + if (runtime.manifest) { + await startWatcher(runtime); + } }); } return { runCreate, + relocateRunBundle, bundleRead, manifestReadSection, manifestPatch, diff --git a/apps/desktop/src/main/services/orchestration/patchPolicy.test.ts b/apps/desktop/src/main/services/orchestration/patchPolicy.test.ts index 4e6b1091c..e64d7b2a9 100644 --- a/apps/desktop/src/main/services/orchestration/patchPolicy.test.ts +++ b/apps/desktop/src/main/services/orchestration/patchPolicy.test.ts @@ -112,6 +112,31 @@ describe("patchPolicy", () => { expect(denied.allowed).toBe(false); }); + it("lead cannot self-approve via manifestPatch", () => { + const manifest = makeManifest(); + const deniedPaths = [ + { op: "add", path: "/leadState/planApprovedAt", value: "now" }, + { op: "replace", path: "/leadState", value: { planApprovedAt: "now" } }, + { op: "replace", path: "/leadState/planApprovalSummary", value: "approved" }, + { op: "replace", path: "/phases/{id:planning}/status", value: "done" }, + { op: "replace", path: "/phases/{id:planning}/completedAt", value: "now" }, + { op: "replace", path: "/currentPhase", value: "developing" }, + ] as const; + for (const op of deniedPaths) { + const result = checkPatchOp(op, { actorRole: "lead", manifest }); + expect(result.allowed, op.path).toBe(false); + } + }); + + it("lead may still patch non-approval leadState fields", () => { + const manifest = makeManifest(); + const allowed = checkPatchOp( + { op: "replace", path: "/leadState/lastSnapshotEtag", value: "etag-1" }, + { actorRole: "lead", manifest }, + ); + expect(allowed.allowed).toBe(true); + }); + it("worker may patch its own rows but not another agent row or validationGate", () => { const manifest = makeManifest(); const taskOk = checkPatchOp( diff --git a/apps/desktop/src/main/services/orchestration/patchPolicy.ts b/apps/desktop/src/main/services/orchestration/patchPolicy.ts index ec98a28f5..cc9ccc932 100644 --- a/apps/desktop/src/main/services/orchestration/patchPolicy.ts +++ b/apps/desktop/src/main/services/orchestration/patchPolicy.ts @@ -116,8 +116,12 @@ const LEAD_DENY_PATTERNS = [ "/bundlePath", "/laneId", "/agents/*/sessionId", + "/leadState", "/leadState/planApprovedAt", "/leadState/planApprovedBySessionId", + "/leadState/planApprovalSummary", + "/phases/{id:planning}/status", + "/phases/{id:planning}/completedAt", "/currentPhase", ]; diff --git a/apps/desktop/src/main/services/orchestration/runtimeProfile.ts b/apps/desktop/src/main/services/orchestration/runtimeProfile.ts index e2fce878a..553547764 100644 --- a/apps/desktop/src/main/services/orchestration/runtimeProfile.ts +++ b/apps/desktop/src/main/services/orchestration/runtimeProfile.ts @@ -83,9 +83,6 @@ export function isOrchestrationPlanApproved( manifest: OrchestrationManifest, ): boolean { if (manifest.currentPhase !== "planning") return true; - if (manifest.phases.some((phase) => phase.id === "planning" && phase.status === "done")) { - return true; - } return typeof manifest.leadState.planApprovedAt === "string" && manifest.leadState.planApprovedAt.trim().length > 0; } diff --git a/apps/desktop/src/main/services/sessions/sessionService.test.ts b/apps/desktop/src/main/services/sessions/sessionService.test.ts index 463b3825b..ad1f9ba19 100644 --- a/apps/desktop/src/main/services/sessions/sessionService.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionService.test.ts @@ -400,6 +400,32 @@ describe("sessionService resume metadata", () => { activeDisposers.push(async () => db.close()); }); + it("allows internal callers to opt out of the default session list limit", async () => { + const projectRoot = makeProjectRoot("ade-session-service-"); + const dbPath = path.join(projectRoot, ".ade", "ade.db"); + const db = await openKvDb(dbPath, createLogger() as any); + insertProjectGraph(db); + const service = createSessionService({ db }); + + for (let i = 0; i < 205; i++) { + service.create({ + sessionId: `session-${i}`, + laneId: "lane-1", + ptyId: null, + tracked: true, + title: `Session ${i}`, + startedAt: new Date(Date.UTC(2026, 2, 17, 0, 0, i)).toISOString(), + transcriptPath: `/tmp/session-${i}.log`, + toolType: "codex-chat", + }); + } + + expect(service.list({ laneId: "lane-1" })).toHaveLength(200); + expect(service.list({ laneId: "lane-1", limit: null })).toHaveLength(205); + + activeDisposers.push(async () => db.close()); + }); + it("repairs legacy droid chat rows from their resume command", async () => { const projectRoot = makeProjectRoot("ade-session-service-"); const dbPath = path.join(projectRoot, ".ade", "ade.db"); diff --git a/apps/desktop/src/main/services/sessions/sessionService.ts b/apps/desktop/src/main/services/sessions/sessionService.ts index 683feb3c0..64d19ded5 100644 --- a/apps/desktop/src/main/services/sessions/sessionService.ts +++ b/apps/desktop/src/main/services/sessions/sessionService.ts @@ -10,6 +10,7 @@ import type { TerminalSessionStatus, TerminalSessionSummary, TerminalToolType, + ListSessionsArgs, UpdateSessionMetaArgs } from "../../../shared/types"; import { stripAnsi } from "../../utils/ansiStrip"; @@ -328,7 +329,7 @@ export function createSessionService({ db }: { db: AdeDb }) { updatedAt: row.updatedAt, }); - const list =({ laneId, status, limit }: { laneId?: string; status?: TerminalSessionStatus; limit?: number } = {}) => { + const list =({ laneId, status, limit }: ListSessionsArgs = {}) => { const where: string[] = []; const params: (string | number | null)[] = []; @@ -342,8 +343,8 @@ export function createSessionService({ db }: { db: AdeDb }) { } const whereSql = where.length ? `where ${where.join(" and ")}` : ""; - const limitSql = typeof limit === "number" ? "limit ?" : "limit 200"; - if (typeof limit === "number") params.push(limit); + const limitSql = limit === null ? "" : "limit ?"; + if (limit !== null) params.push(typeof limit === "number" ? limit : 200); const rows = db.all( ` diff --git a/apps/desktop/src/preload/preload.test.ts b/apps/desktop/src/preload/preload.test.ts index 3d640fee0..40820f31c 100644 --- a/apps/desktop/src/preload/preload.test.ts +++ b/apps/desktop/src/preload/preload.test.ts @@ -4642,6 +4642,78 @@ describe("preload OAuth bridge", () => { }); }); +describe("preload openRepo binding", () => { + beforeEach(() => { + vi.resetModules(); + delete (globalThis as any).__adeBridge; + }); + + afterEach(() => { + vi.resetModules(); + vi.doUnmock("electron"); + delete (globalThis as any).__adeBridge; + }); + + it("restores the previous binding when openRepo is cancelled", async () => { + const remoteRuntimeProjects: string[] = []; + const invoke = vi.fn(async (channel: string, arg?: unknown) => { + if (channel === IPC.projectOpenRepo) { + return null; + } + if (channel === IPC.remoteRuntimeCallAction) { + const request = (arg as { projectId?: string; request?: { domain?: string; action?: string } }).request; + remoteRuntimeProjects.push((arg as { projectId?: string }).projectId ?? ""); + return { + ok: true, + domain: request?.domain, + action: request?.action, + result: [], + statusHints: {}, + }; + } + if (channel === IPC.appGetWindowSession) { + return { + windowId: 1, + project: null, + binding: { + kind: "remote", + key: "remote:target-1:project-1", + targetId: "target-1", + runtimeName: "Remote", + projectId: "project-1", + rootPath: "/remote/project", + displayName: "Project", + }, + }; + } + throw new Error(`unexpected IPC: ${channel}`); + }); + const on = vi.fn(); + const removeListener = vi.fn(); + const exposeInMainWorld = vi.fn((_name: string, value: unknown) => { + (globalThis as any).__adeBridge = value; + }); + + vi.doMock("electron", () => ({ + contextBridge: { exposeInMainWorld }, + ipcRenderer: { invoke, on, removeListener }, + webFrame: { + getZoomLevel: vi.fn(() => 0), + setZoomLevel: vi.fn(), + getZoomFactor: vi.fn(() => 1), + }, + })); + + await import("./preload"); + const bridge = (globalThis as any).__adeBridge; + + await expect(bridge.lanes.list()).resolves.toEqual([]); + await expect(bridge.project.openRepo()).resolves.toBeNull(); + await expect(bridge.lanes.list()).resolves.toEqual([]); + expect(remoteRuntimeProjects).toEqual(["project-1", "project-1"]); + }); +}); + describe("preload remote project binding", () => { beforeEach(() => { vi.resetModules(); diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 49aecbaf3..ba5068621 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -3015,15 +3015,25 @@ contextBridge.exposeInMainWorld("ade", { // appProjectBindingChanged listener) and disabled runtime routing / // event pumping until another refresh restored it. Null once up front; // the listener handles the post-action update. + const previousBinding = currentProjectBinding; rememberProjectBinding(null); - return clearAround( - () => { - clearProjectScopedReadCaches(); - }, - () => runProjectRuntimeTransition(() => - ipcRenderer.invoke(IPC.projectOpenRepo, args ?? {}), - ), - ); + try { + const project = await clearAround( + () => { + clearProjectScopedReadCaches(); + }, + () => runProjectRuntimeTransition(() => + ipcRenderer.invoke(IPC.projectOpenRepo, args ?? {}), + ), + ); + if (!project) { + rememberProjectBinding(previousBinding); + } + return project; + } catch (error) { + rememberProjectBinding(previousBinding); + throw error; + } }, chooseDirectory: async ( args: { title?: string; defaultPath?: string } = {}, diff --git a/apps/desktop/src/shared/types/sessions.ts b/apps/desktop/src/shared/types/sessions.ts index 47c2ad245..5203d3171 100644 --- a/apps/desktop/src/shared/types/sessions.ts +++ b/apps/desktop/src/shared/types/sessions.ts @@ -324,7 +324,7 @@ export type TerminalSessionChangedEvent = { export type ListSessionsArgs = { laneId?: string; status?: TerminalSessionStatus; - limit?: number; + limit?: number | null; }; export type DeleteSessionArgs = {