diff --git a/docs/agent-lifecycle.md b/docs/agent-lifecycle.md index c0ac0c5586..c0aad974f6 100644 --- a/docs/agent-lifecycle.md +++ b/docs/agent-lifecycle.md @@ -270,6 +270,9 @@ The daemon runs one archive lane per workspace, shared by every connection | an undo runs: archive | waits for the undo, then archives | | an undo runs: undo | joins the running undo | +A request takes its place in the lane before its first `await`. Arrival order decides, so an undo +sent right behind its archive still cancels it. + An aborted archive stops at its next checkpoint and reports `canceled`; the undo unarchives only the agents that archive itself archived. Deleting the worktree directory is the point of no return: an undo after it recreates the worktree through workspace recovery. The client shows or diff --git a/packages/app/e2e/browser/workspace-archive-undo.spec.ts b/packages/app/e2e/browser/workspace-archive-undo.spec.ts index 46c81e504a..4806e42b7c 100644 --- a/packages/app/e2e/browser/workspace-archive-undo.spec.ts +++ b/packages/app/e2e/browser/workspace-archive-undo.spec.ts @@ -1,4 +1,3 @@ -import { existsSync } from "node:fs"; import type { Page } from "@playwright/test"; import { expect, test } from "../support/fixtures"; import { gotoAppShell } from "../support/helpers/app"; @@ -11,29 +10,23 @@ import { import { getServerId } from "../support/helpers/server-id"; import { expectWorkspaceAbsentFromSidebar } from "../support/helpers/sidebar"; import { createTempGitRepo } from "../support/helpers/workspace"; +import { + archiveFromRowButton, + expectDirectoryPresence, + expectHostWorkspaceSettledActive, + outlastUndoWindow, + pressUndoShortcut, + rowArchiveButton, + rowKebab, + undoButton, + workspaceRow, + workspaceTitleWidth, +} from "../support/helpers/workspace-archive-undo"; import { waitForSidebarHydration, waitForWorkspaceInSidebar, } from "../support/helpers/workspace-ui"; -const UNDO_SHORTCUT = process.platform === "darwin" ? "Meta+Z" : "Control+Z"; - -function workspaceRow(page: Page, workspaceId: string) { - return page.getByTestId(`sidebar-workspace-row-${getServerId()}:${workspaceId}`); -} - -function archiveButton(page: Page, workspaceId: string) { - return page.getByTestId(`sidebar-workspace-archive-button-${getServerId()}:${workspaceId}`); -} - -async function titleWidth(page: Page, workspaceId: string): Promise { - const box = await page - .getByTestId(`sidebar-workspace-title-${getServerId()}:${workspaceId}`) - .boundingBox(); - if (!box) throw new Error(`Title of ${workspaceId} is not laid out`); - return box.width; -} - test.describe("Workspace archive undo", () => { let client: Awaited>; let tempRepo: { path: string; cleanup: () => Promise }; @@ -55,11 +48,6 @@ test.describe("Workspace archive undo", () => { await tempRepo?.cleanup().catch(() => undefined); }); - async function activeWorkspaceIds(): Promise { - const { entries } = await client.fetchWorkspaces(); - return entries.map((entry) => entry.id); - } - async function seedWorktree(page: Page, slug: string) { await openProjectViaDaemon(client, tempRepo.path); const worktree = await createWorktreeViaDaemon(client, { @@ -78,23 +66,19 @@ test.describe("Workspace archive undo", () => { test("the hover archive button overlays the title instead of narrowing it", async ({ page }) => { const worktree = await seedWorktree(page, "archive-button-overlay"); - const row = workspaceRow(page, worktree.workspaceId); await page.mouse.move(0, 0); - await expect(archiveButton(page, worktree.workspaceId)).toHaveCount(0); - const restingWidth = await titleWidth(page, worktree.workspaceId); + await expect(rowArchiveButton(page, worktree.workspaceId)).toHaveCount(0); + const restingWidth = await workspaceTitleWidth(page, worktree.workspaceId); - await row.hover(); - await expect(archiveButton(page, worktree.workspaceId)).toBeVisible(); + await workspaceRow(page, worktree.workspaceId).hover(); + await expect(rowArchiveButton(page, worktree.workspaceId)).toBeVisible(); await expect(page.getByTestId("sidebar-workspace-trailing-scrim")).toBeVisible(); - expect(await titleWidth(page, worktree.workspaceId)).toBe(restingWidth); + expect(await workspaceTitleWidth(page, worktree.workspaceId)).toBe(restingWidth); - const kebab = page.getByTestId( - `sidebar-workspace-kebab-${getServerId()}:${worktree.workspaceId}`, - ); const [buttonBox, kebabBox] = await Promise.all([ - archiveButton(page, worktree.workspaceId).boundingBox(), - kebab.boundingBox(), + rowArchiveButton(page, worktree.workspaceId).boundingBox(), + rowKebab(page, worktree.workspaceId).boundingBox(), ]); expect(buttonBox && kebabBox && buttonBox.x + buttonBox.width <= kebabBox.x).toBe(true); }); @@ -102,50 +86,57 @@ test.describe("Workspace archive undo", () => { test("Cmd/Ctrl+Z right after archiving keeps the worktree", async ({ page }) => { const worktree = await seedWorktree(page, "archive-undo-shortcut"); - page.once("dialog", (dialog) => void dialog.accept()); - await workspaceRow(page, worktree.workspaceId).hover(); - await archiveButton(page, worktree.workspaceId).click(); - await expectWorkspaceAbsentFromSidebar(page, worktree.workspaceId); - await expect(page.getByTestId("workspace-archived-toast-undo")).toBeVisible(); - - await page.keyboard.press(UNDO_SHORTCUT); + await archiveFromRowButton(page, worktree.workspaceId); + await pressUndoShortcut(page); await expect(workspaceRow(page, worktree.workspaceId)).toBeVisible(); - await expect(page.getByTestId("workspace-archived-toast-undo")).toHaveCount(0); - await expect.poll(activeWorkspaceIds).toContain(worktree.workspaceId); - expect(existsSync(worktree.workspaceDirectory)).toBe(true); - // A late remove from the canceled archive must not take the row back out. - await page.waitForTimeout(1500); + await expect(undoButton(page)).toHaveCount(0); + await expectHostWorkspaceSettledActive(client, worktree.workspaceId); + // Once the host settled, any remove the canceled archive sent has already arrived. await expect(workspaceRow(page, worktree.workspaceId)).toBeVisible(); + await expectDirectoryPresence(worktree.workspaceDirectory, true); }); test("Undo after the worktree was removed recreates it", async ({ page }) => { const worktree = await seedWorktree(page, "archive-undo-recreate"); - page.once("dialog", (dialog) => void dialog.accept()); - await workspaceRow(page, worktree.workspaceId).hover(); - await archiveButton(page, worktree.workspaceId).click(); - const undo = page.getByTestId("workspace-archived-toast-undo"); + await archiveFromRowButton(page, worktree.workspaceId); // Hovering the toast holds it, and with it the undo, open while the archive finishes. - await undo.hover(); - await expect - .poll(() => existsSync(worktree.workspaceDirectory), { timeout: 30_000 }) - .toBe(false); - - await undo.click(); + await undoButton(page).hover(); + await expectDirectoryPresence(worktree.workspaceDirectory, false); + await undoButton(page).click(); await expect(workspaceRow(page, worktree.workspaceId)).toBeVisible({ timeout: 30_000 }); - await expect - .poll(() => existsSync(worktree.workspaceDirectory), { timeout: 30_000 }) - .toBe(true); + await expectDirectoryPresence(worktree.workspaceDirectory, true); + }); + + test("hovering Undo keeps the toast open past its deadline, and its close ends the undo", async ({ + page, + }) => { + await page.clock.install({ time: Date.now() }); + const worktree = await seedWorktree(page, "archive-undo-hold"); + + await archiveFromRowButton(page, worktree.workspaceId); + await page.getByTestId("workspace-archived-toast").hover({ position: { x: 8, y: 8 } }); + await undoButton(page).hover(); + await outlastUndoWindow(page); + await expect(undoButton(page)).toBeVisible(); + + await page.mouse.move(0, 0); + await outlastUndoWindow(page); + await expect(undoButton(page)).toHaveCount(0); + + // The toast took the undo with it. + await pressUndoShortcut(page); + await expectWorkspaceAbsentFromSidebar(page, worktree.workspaceId); }); test("Cmd/Ctrl+Z without an archive to undo leaves the sidebar alone", async ({ page }) => { const worktree = await seedWorktree(page, "archive-undo-none"); - await page.keyboard.press(UNDO_SHORTCUT); + await pressUndoShortcut(page); - await expect(page.getByTestId("workspace-archived-toast-undo")).toHaveCount(0); + await expect(undoButton(page)).toHaveCount(0); await expect(workspaceRow(page, worktree.workspaceId)).toBeVisible(); }); }); diff --git a/packages/app/e2e/support/helpers/workspace-archive-undo.ts b/packages/app/e2e/support/helpers/workspace-archive-undo.ts new file mode 100644 index 0000000000..febe46d435 --- /dev/null +++ b/packages/app/e2e/support/helpers/workspace-archive-undo.ts @@ -0,0 +1,75 @@ +import { existsSync } from "node:fs"; +import type { Page } from "@playwright/test"; +import { expect } from "../fixtures"; +import type { connectNewWorkspaceDaemonClient } from "./new-workspace"; +import { getServerId } from "./server-id"; +import { expectWorkspaceAbsentFromSidebar } from "./sidebar"; + +type DaemonClient = Awaited>; + +const UNDO_SHORTCUT = process.platform === "darwin" ? "Meta+Z" : "Control+Z"; +/** The archived toast's lifetime, from `WORKSPACE_ARCHIVE_UNDO_WINDOW_MS`. */ +const UNDO_WINDOW_MS = 6000; + +export function workspaceRow(page: Page, workspaceId: string) { + return page.getByTestId(`sidebar-workspace-row-${getServerId()}:${workspaceId}`); +} + +export function rowArchiveButton(page: Page, workspaceId: string) { + return page.getByTestId(`sidebar-workspace-archive-button-${getServerId()}:${workspaceId}`); +} + +export function rowKebab(page: Page, workspaceId: string) { + return page.getByTestId(`sidebar-workspace-kebab-${getServerId()}:${workspaceId}`); +} + +export function undoButton(page: Page) { + return page.getByRole("button", { name: /^Undo/ }); +} + +export async function workspaceTitleWidth(page: Page, workspaceId: string): Promise { + const box = await page + .getByTestId(`sidebar-workspace-title-${getServerId()}:${workspaceId}`) + .boundingBox(); + if (!box) throw new Error(`Title of ${workspaceId} is not laid out`); + return box.width; +} + +/** Archives a workspace with its row's hover button and waits for the undo toast. */ +export async function archiveFromRowButton(page: Page, workspaceId: string): Promise { + page.once("dialog", (dialog) => void dialog.accept()); + await workspaceRow(page, workspaceId).hover(); + await rowArchiveButton(page, workspaceId).click(); + await expectWorkspaceAbsentFromSidebar(page, workspaceId); + await expect(undoButton(page)).toBeVisible(); +} + +export async function pressUndoShortcut(page: Page): Promise { + await page.keyboard.press(UNDO_SHORTCUT); +} + +/** Moves the page clock past the moment an unheld archived toast would have closed. */ +export async function outlastUndoWindow(page: Page): Promise { + await page.clock.fastForward(UNDO_WINDOW_MS + 1000); +} + +/** Waits until the host lists the workspace as active with no archive running on it. */ +export async function expectHostWorkspaceSettledActive( + client: DaemonClient, + workspaceId: string, +): Promise { + await expect + .poll( + async () => { + const { entries } = await client.fetchWorkspaces(); + const entry = entries.find((candidate) => candidate.id === workspaceId); + return entry ? { active: true, archiving: Boolean(entry.archivingAt) } : null; + }, + { timeout: 30_000 }, + ) + .toEqual({ active: true, archiving: false }); +} + +export async function expectDirectoryPresence(directory: string, present: boolean): Promise { + await expect.poll(() => existsSync(directory), { timeout: 30_000 }).toBe(present); +} diff --git a/packages/app/src/components/toast-host.test.tsx b/packages/app/src/components/toast-host.test.tsx index ee8683c5ae..32f1bfb8a7 100644 --- a/packages/app/src/components/toast-host.test.tsx +++ b/packages/app/src/components/toast-host.test.tsx @@ -32,25 +32,4 @@ describe("useToastHost", () => { act(() => dismissUploading()); expect(result.current.toast?.content).toBe("Image uploaded"); }); - - it("tells a toast when it leaves the screen, once, however it leaves", () => { - const { result } = renderHook(() => useToastHost()); - const left: string[] = []; - - let dismissFirst = () => {}; - act(() => { - dismissFirst = result.current.api.show("Archived", { - durationMs: null, - onDismiss: () => left.push("replaced"), - }); - }); - act(() => { - result.current.api.show("Copied", { onDismiss: () => left.push("dismissed") }); - }); - act(() => dismissFirst()); - act(() => result.current.dismiss()); - act(() => result.current.dismiss()); - - expect(left).toEqual(["replaced", "dismissed"]); - }); }); diff --git a/packages/app/src/contexts/session-workspace-upserts.test.ts b/packages/app/src/contexts/session-workspace-upserts.test.ts index cc4991b82a..7e942f234b 100644 --- a/packages/app/src/contexts/session-workspace-upserts.test.ts +++ b/packages/app/src/contexts/session-workspace-upserts.test.ts @@ -2,11 +2,11 @@ import { describe, expect, it } from "vitest"; import type { WorkspaceDescriptor } from "@/stores/session-store"; import { clearWorkspaceArchivePending, - clearWorkspaceArchiveUndoPending, isWorkspaceArchivePending, markWorkspaceArchivePending, markWorkspaceArchiveUndoPending, shouldSuppressWorkspaceForLocalArchive, + settleWorkspaceArchiveIntent, shouldSuppressWorkspaceRemovalForLocalUndo, } from "@/contexts/session-workspace-upserts"; @@ -125,7 +125,7 @@ describe("workspace archive undo intent", () => { it("lets host upserts through and holds host removes while an undo is pending", () => { markWorkspaceArchivePending(target); - markWorkspaceArchiveUndoPending(target); + const undo = markWorkspaceArchiveUndoPending(target); expect({ upsertSuppressed: shouldSuppressWorkspaceForLocalArchive({ @@ -135,20 +135,25 @@ describe("workspace archive undo intent", () => { removeSuppressed: shouldSuppressWorkspaceRemovalForLocalUndo(target), }).toEqual({ upsertSuppressed: false, removeSuppressed: true }); - clearWorkspaceArchiveUndoPending(target); + settleWorkspaceArchiveIntent(undo); expect(shouldSuppressWorkspaceRemovalForLocalUndo(target)).toBe(false); }); it("keeps a newer intent when the request behind an older one settles late", () => { - markWorkspaceArchivePending(target); - markWorkspaceArchiveUndoPending(target); + const firstArchive = markWorkspaceArchivePending(target); + const firstUndo = markWorkspaceArchiveUndoPending(target); + settleWorkspaceArchiveIntent(firstArchive); clearWorkspaceArchivePending(target); expect(shouldSuppressWorkspaceRemovalForLocalUndo(target)).toBe(true); markWorkspaceArchivePending(target); - clearWorkspaceArchiveUndoPending(target); + settleWorkspaceArchiveIntent(firstUndo); expect(isWorkspaceArchivePending(target)).toBe(true); - clearWorkspaceArchivePending(target); + const secondUndo = markWorkspaceArchiveUndoPending(target); + settleWorkspaceArchiveIntent(firstUndo); + expect(shouldSuppressWorkspaceRemovalForLocalUndo(target)).toBe(true); + + settleWorkspaceArchiveIntent(secondUndo); }); }); diff --git a/packages/app/src/contexts/session-workspace-upserts.ts b/packages/app/src/contexts/session-workspace-upserts.ts index b5685fe41b..7c03ecf8e2 100644 --- a/packages/app/src/contexts/session-workspace-upserts.ts +++ b/packages/app/src/contexts/session-workspace-upserts.ts @@ -7,12 +7,18 @@ import { normalizeWorkspaceOpaqueId } from "@/utils/workspace-identity"; * - `archive`: the row is hidden; host upserts for it are dropped until its remove arrives. * - `undo`: the row is back; host removes for it are dropped until the undo response arrives. * - * Only the request that set an intent may clear it, so a late response for an earlier - * request never clears the intent of a newer one. + * Each request gets its own intent. A request settles or checks only the intent it set, so a + * late response for an earlier request never touches the intent of a newer one. */ -type LocalWorkspaceArchiveIntent = "archive" | "undo"; +export interface WorkspaceArchiveIntent { + readonly kind: "archive" | "undo"; +} -const intentsByServer = new Map>(); +const intentsByServer = new Map>(); +const targetByIntent = new WeakMap< + WorkspaceArchiveIntent, + { serverId: string; workspaceId: string } +>(); interface WorkspaceIntentTarget { serverId: string; @@ -28,63 +34,82 @@ function resolveTarget( return { serverId, workspaceId }; } -function readIntent(input: WorkspaceIntentTarget): LocalWorkspaceArchiveIntent | null { +function readIntent(input: WorkspaceIntentTarget): WorkspaceArchiveIntent | null { const target = resolveTarget(input); if (!target) return null; return intentsByServer.get(target.serverId)?.get(target.workspaceId) ?? null; } -function setIntent(input: WorkspaceIntentTarget, intent: LocalWorkspaceArchiveIntent): void { +function setIntent( + input: WorkspaceIntentTarget, + kind: WorkspaceArchiveIntent["kind"], +): WorkspaceArchiveIntent { + const intent: WorkspaceArchiveIntent = { kind }; const target = resolveTarget(input); - if (!target) return; + if (!target) return intent; + targetByIntent.set(intent, target); const intents = intentsByServer.get(target.serverId) ?? new Map(); intents.set(target.workspaceId, intent); intentsByServer.set(target.serverId, intents); + return intent; } -function clearIntent(input: WorkspaceIntentTarget, intent: LocalWorkspaceArchiveIntent): void { - const target = resolveTarget(input); - if (!target) return; +function deleteIntent(target: { serverId: string; workspaceId: string }): void { const intents = intentsByServer.get(target.serverId); - if (intents?.get(target.workspaceId) !== intent) return; + if (!intents) return; intents.delete(target.workspaceId); if (intents.size === 0) intentsByServer.delete(target.serverId); } +function clearIntentOfKind( + input: WorkspaceIntentTarget, + kind: WorkspaceArchiveIntent["kind"], +): void { + const target = resolveTarget(input); + if (!target || readIntent(target)?.kind !== kind) return; + deleteIntent(target); +} + +/** True while `intent` is still the latest thing this client asked for its workspace. */ +export function isWorkspaceArchiveIntentCurrent(intent: WorkspaceArchiveIntent): boolean { + const target = targetByIntent.get(intent); + return target !== undefined && readIntent(target) === intent; +} + +/** The request behind `intent` settled. A newer request's intent stays in place. */ +export function settleWorkspaceArchiveIntent(intent: WorkspaceArchiveIntent): void { + const target = targetByIntent.get(intent); + if (target && readIntent(target) === intent) deleteIntent(target); +} + export function markWorkspaceArchivePending(input: { serverId: string; workspaceId: string; -}): void { - setIntent(input, "archive"); +}): WorkspaceArchiveIntent { + return setIntent(input, "archive"); } +/** The host removed the workspace, which settles whichever archive this client has pending. */ export function clearWorkspaceArchivePending(input: { serverId: string; workspaceId: string; }): void { - clearIntent(input, "archive"); + clearIntentOfKind(input, "archive"); } export function isWorkspaceArchivePending(input: WorkspaceIntentTarget): boolean { - return readIntent(input) === "archive"; + return readIntent(input)?.kind === "archive"; } export function markWorkspaceArchiveUndoPending(input: { serverId: string; workspaceId: string; -}): void { - setIntent(input, "undo"); -} - -export function clearWorkspaceArchiveUndoPending(input: { - serverId: string; - workspaceId: string; -}): void { - clearIntent(input, "undo"); +}): WorkspaceArchiveIntent { + return setIntent(input, "undo"); } export function isWorkspaceArchiveUndoPending(input: WorkspaceIntentTarget): boolean { - return readIntent(input) === "undo"; + return readIntent(input)?.kind === "undo"; } export function shouldSuppressWorkspaceForLocalArchive(input: { diff --git a/packages/app/src/runtime/directory-sync/workspace-replica.test.ts b/packages/app/src/runtime/directory-sync/workspace-replica.test.ts index 874773e5c7..52d3692d59 100644 --- a/packages/app/src/runtime/directory-sync/workspace-replica.test.ts +++ b/packages/app/src/runtime/directory-sync/workspace-replica.test.ts @@ -8,9 +8,9 @@ import { } from "@/stores/session-store"; import { clearWorkspaceArchivePending, - clearWorkspaceArchiveUndoPending, markWorkspaceArchivePending, markWorkspaceArchiveUndoPending, + settleWorkspaceArchiveIntent, } from "@/contexts/session-workspace-upserts"; import { useWorkspaceSetupStore } from "@/stores/workspace-setup-store"; import { WorkspaceDirectoryReplica } from "./workspace-replica"; @@ -388,18 +388,18 @@ it("keeps a workspace whose archive is being undone when the host reports its re const replica = new WorkspaceDirectoryReplica(serverId); const restored = normalizeWorkspaceDescriptor(workspace("restored")); + const undo = markWorkspaceArchiveUndoPending({ serverId, workspaceId: restored.id }); try { replica.acceptWorkspaces([restored]); - markWorkspaceArchiveUndoPending({ serverId, workspaceId: restored.id }); expect(replica.applyDelta({ kind: "remove", id: restored.id })).toEqual([]); expect(useSessionStore.getState().sessions[serverId]?.workspaces.has(restored.id)).toBe(true); - clearWorkspaceArchiveUndoPending({ serverId, workspaceId: restored.id }); + settleWorkspaceArchiveIntent(undo); replica.applyDelta({ kind: "remove", id: restored.id }); expect(useSessionStore.getState().sessions[serverId]?.workspaces.has(restored.id)).toBe(false); } finally { - clearWorkspaceArchiveUndoPending({ serverId, workspaceId: restored.id }); + settleWorkspaceArchiveIntent(undo); store.clearSession(serverId); } }); diff --git a/packages/app/src/workspace/archive-undo/internal/undo-window.test.ts b/packages/app/src/workspace/archive-undo/internal/undo-window.test.ts index 1678ff365d..1e36395f2a 100644 --- a/packages/app/src/workspace/archive-undo/internal/undo-window.test.ts +++ b/packages/app/src/workspace/archive-undo/internal/undo-window.test.ts @@ -8,10 +8,9 @@ import { useSessionStore, type WorkspaceDescriptor } from "@/stores/session-stor import { seedSessionHosts } from "@/test/seed-session"; import { archiveWorkspaceWithUndo, - hasUndoableWorkspaceArchive, undoLatestWorkspaceArchive, type UndoableWorkspaceArchiveClient, -} from "./undo-window"; +} from "@/workspace/archive-undo"; const SERVER_ID = "workspace-archive-undo-window"; @@ -159,9 +158,9 @@ describe("archiveWorkspaceWithUndo", () => { expect(committed).toEqual([]); toast.timeOut(); - expect({ committed, undoable: hasUndoableWorkspaceArchive() }).toEqual({ + expect({ committed, undone: undoLatestWorkspaceArchive() }).toEqual({ committed: ["a"], - undoable: false, + undone: false, }); }); @@ -250,7 +249,7 @@ describe("archiveWorkspaceWithUndo", () => { await archive; undoLatestWorkspaceArchive(); host.failUndo("a", new Error("Branch is gone")); - await expect.poll(() => toast.errors).toEqual(["Branch is gone"]); + await expect.poll(() => toast.errors).toEqual(["Couldn't undo the archive"]); expect(storedWorkspace("a")).toBeUndefined(); }); @@ -277,8 +276,8 @@ describe("archiveWorkspaceWithUndo", () => { await expect(archive).rejects.toThrow("registry unavailable"); expect({ toast: toast.shownTestID, - undoable: hasUndoableWorkspaceArchive(), + undone: undoLatestWorkspaceArchive(), row: storedWorkspace("a"), - }).toEqual({ toast: null, undoable: false, row: workspace("a") }); + }).toEqual({ toast: null, undone: false, row: workspace("a") }); }); }); diff --git a/packages/app/src/workspace/archive-undo/internal/undo-window.tsx b/packages/app/src/workspace/archive-undo/internal/undo-window.tsx index dc6cf992d1..65b5d8419e 100644 --- a/packages/app/src/workspace/archive-undo/internal/undo-window.tsx +++ b/packages/app/src/workspace/archive-undo/internal/undo-window.tsx @@ -72,11 +72,8 @@ class UndoWindow { this.release(); this.dismissToast(); void undoWorkspaceArchiveOptimistically(this.input).catch((error: unknown) => { - this.input.toast.error( - error instanceof Error - ? error.message - : i18n.t("sidebar.workspace.toasts.undoArchiveFailed"), - ); + console.warn("[workspace-archive-undo] undo failed", error); + this.input.toast.error(i18n.t("sidebar.workspace.toasts.undoArchiveFailed")); }); return true; }; diff --git a/packages/app/src/workspace/workspace-archive.test.ts b/packages/app/src/workspace/workspace-archive.test.ts index eb4e8bf5de..e519bcffb3 100644 --- a/packages/app/src/workspace/workspace-archive.test.ts +++ b/packages/app/src/workspace/workspace-archive.test.ts @@ -4,7 +4,6 @@ import type { DaemonClient } from "@getpaseo/client/internal/daemon-client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { clearWorkspaceArchivePending, - clearWorkspaceArchiveUndoPending, isWorkspaceArchivePending, isWorkspaceArchiveUndoPending, } from "@/contexts/session-workspace-upserts"; @@ -105,7 +104,6 @@ afterEach(() => { clearWorkspaceArchivePending({ serverId: SERVER_ID, workspaceId: "workspace-2" }); clearWorkspaceArchivePending({ serverId: SECOND_SERVER_ID, workspaceId: "workspace-1" }); clearWorkspaceArchivePending({ serverId: SECOND_SERVER_ID, workspaceId: "workspace-2" }); - clearWorkspaceArchiveUndoPending({ serverId: SERVER_ID, workspaceId: "workspace-1" }); useSessionStore.setState((state) => ({ ...state, sessions: {} })); }); @@ -314,4 +312,57 @@ describe("undoWorkspaceArchiveOptimistically", () => { expect(storedWorkspace(archived.id)).toBeUndefined(); expect(isWorkspaceArchivePending(target())).toBe(true); }); + it("puts the row back when another client's undo cancels this archive", async () => { + const archived = workspace(); + getHostRuntimeStore().acceptWorkspaceSnapshots(SERVER_ID, [archived]); + const archive = beginOptimisticWorkspaceArchive({ + client: createClient(async () => + archivePayload({ workspaceId: archived.id, canceled: true, error: "undone" }), + ), + workspace: target(), + }); + + await expect(archive.done).resolves.toBe("canceled"); + + expect({ + row: storedWorkspace(archived.id), + archivePending: isWorkspaceArchivePending(target()), + }).toEqual({ row: archived, archivePending: false }); + }); + + it("an earlier undo settling keeps holding the row for a later undo", async () => { + const archived = workspace(); + getHostRuntimeStore().acceptWorkspaceSnapshots(SERVER_ID, [archived]); + const firstUndoResponse = deferred(); + const secondUndoResponse = deferred(); + const first = beginOptimisticWorkspaceArchive({ + client: createClient(async () => archivePayload({ workspaceId: archived.id })), + workspace: target(), + }); + await first.done; + const firstUndo = undoWorkspaceArchiveOptimistically({ + client: { undoWorkspaceArchive: async () => firstUndoResponse.promise }, + workspace: target(), + hidden: first.hidden, + }); + const second = beginOptimisticWorkspaceArchive({ + client: createClient(async () => new Promise(() => {})), + workspace: target(), + }); + const secondUndo = undoWorkspaceArchiveOptimistically({ + client: { undoWorkspaceArchive: async () => secondUndoResponse.promise }, + workspace: target(), + hidden: second.hidden, + }); + + firstUndoResponse.resolve(); + await firstUndo; + + expect({ + row: storedWorkspace(archived.id), + undoPending: isWorkspaceArchiveUndoPending(target()), + }).toEqual({ row: archived, undoPending: true }); + secondUndoResponse.resolve(); + await secondUndo; + }); }); diff --git a/packages/app/src/workspace/workspace-archive.ts b/packages/app/src/workspace/workspace-archive.ts index eeaf928c81..cc2a24db97 100644 --- a/packages/app/src/workspace/workspace-archive.ts +++ b/packages/app/src/workspace/workspace-archive.ts @@ -1,10 +1,10 @@ import { getHostRuntimeStore } from "@/runtime/host-runtime"; import { - clearWorkspaceArchivePending, - clearWorkspaceArchiveUndoPending, - isWorkspaceArchiveUndoPending, + isWorkspaceArchiveIntentCurrent, markWorkspaceArchivePending, markWorkspaceArchiveUndoPending, + settleWorkspaceArchiveIntent, + type WorkspaceArchiveIntent, } from "@/contexts/session-workspace-upserts"; import { useSessionStore, type WorkspaceDescriptor } from "@/stores/session-store"; import { resolveWorkspaceMapKeyByIdentity } from "@/utils/workspace-identity"; @@ -34,6 +34,7 @@ export interface OptimisticWorkspaceArchive { interface OptimisticWorkspaceArchiveSnapshot { workspace: WorkspaceDescriptor | null; + intent: WorkspaceArchiveIntent; } export interface WorkspaceArchiveFailure { @@ -63,23 +64,24 @@ function hideWorkspaceOptimistically( workspaceId: workspace.workspaceId, }); const snapshot = workspaceKey ? (workspaces?.get(workspaceKey) ?? null) : null; - markWorkspaceArchivePending({ + const intent = markWorkspaceArchivePending({ serverId: workspace.serverId, workspaceId: workspace.workspaceId, }); getHostRuntimeStore().removeWorkspaceSnapshot(workspace.serverId, workspace.workspaceId); - return { workspace: snapshot }; + return { workspace: snapshot, intent }; } +/** + * Puts back a row whose archive did not happen. Skipped when a newer request on this client + * (an undo, or another archive) owns the row by now. + */ function restoreOptimisticallyHiddenWorkspace(input: { serverId: string; - workspaceId: string; snapshot: OptimisticWorkspaceArchiveSnapshot; }): void { - clearWorkspaceArchivePending({ - serverId: input.serverId, - workspaceId: input.workspaceId, - }); + if (!isWorkspaceArchiveIntentCurrent(input.snapshot.intent)) return; + settleWorkspaceArchiveIntent(input.snapshot.intent); if (input.snapshot.workspace) { getHostRuntimeStore().acceptWorkspaceSnapshots(input.serverId, [input.snapshot.workspace]); } @@ -99,7 +101,10 @@ async function archiveWorkspaceOrThrow(input: { return "archived"; } -/** Hides the row now and archives it on the host. A failed archive puts the row back. */ +/** + * Hides the row now and archives it on the host. A failed archive puts the row back, and so + * does a canceled one: an undo from another client stopped it, so the workspace stays. + */ export function beginOptimisticWorkspaceArchive(input: { client: WorkspaceArchiveClient; workspace: WorkspaceArchiveTarget; @@ -107,16 +112,16 @@ export function beginOptimisticWorkspaceArchive(input: { const snapshot = hideWorkspaceOptimistically(input.workspace); const done = (async () => { try { - return await archiveWorkspaceOrThrow({ + const outcome = await archiveWorkspaceOrThrow({ client: input.client, workspaceId: input.workspace.workspaceId, }); + if (outcome === "canceled") { + restoreOptimisticallyHiddenWorkspace({ serverId: input.workspace.serverId, snapshot }); + } + return outcome; } catch (error) { - restoreOptimisticallyHiddenWorkspace({ - serverId: input.workspace.serverId, - workspaceId: input.workspace.workspaceId, - snapshot, - }); + restoreOptimisticallyHiddenWorkspace({ serverId: input.workspace.serverId, snapshot }); throw error; } })(); @@ -141,19 +146,19 @@ export async function undoWorkspaceArchiveOptimistically(input: { hidden: WorkspaceDescriptor | null; }): Promise { const { serverId, workspaceId } = input.workspace; - markWorkspaceArchiveUndoPending({ serverId, workspaceId }); + const intent = markWorkspaceArchiveUndoPending({ serverId, workspaceId }); if (input.hidden) { getHostRuntimeStore().acceptWorkspaceSnapshots(serverId, [input.hidden]); } try { await input.client.undoWorkspaceArchive(workspaceId); } catch (error) { - if (isWorkspaceArchiveUndoPending({ serverId, workspaceId })) { + if (isWorkspaceArchiveIntentCurrent(intent)) { getHostRuntimeStore().removeWorkspaceSnapshot(serverId, workspaceId); } throw error; } finally { - clearWorkspaceArchiveUndoPending({ serverId, workspaceId }); + settleWorkspaceArchiveIntent(intent); } } diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 2f600576d0..c652f70908 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -7807,15 +7807,16 @@ export class Session { request: Extract, ): Promise { try { - const existing = await this.workspaceRegistry.get(request.workspaceId); - if (!existing) { - throw new Error(`Workspace not found: ${request.workspaceId}`); - } - + // Claim the archive queue before any await, so an undo sent right behind this request + // queues after it and cancels it instead of running first. const outcome = await this.workspaceArchiveOperations.archive( - existing.workspaceId, - (signal) => - archiveByScope( + request.workspaceId, + async (signal) => { + const existing = await this.workspaceRegistry.get(request.workspaceId); + if (!existing) { + throw new Error(`Workspace not found: ${request.workspaceId}`); + } + return archiveByScope( { paseoHome: this.paseoHome, paseoWorktreesBaseRoot: this.worktreesRoot, @@ -7844,7 +7845,8 @@ export class Session { requestId: request.requestId, signal, }, - ), + ); + }, ); if (outcome.kind === "canceled") { diff --git a/packages/server/src/server/workspace-archive-operations.test.ts b/packages/server/src/server/workspace-archive-operations.test.ts index 7f2b9f168e..c262f58fa3 100644 --- a/packages/server/src/server/workspace-archive-operations.test.ts +++ b/packages/server/src/server/workspace-archive-operations.test.ts @@ -205,6 +205,24 @@ describe("WorkspaceArchiveOperations", () => { expect(restored).toEqual([["agent-a"]]); }); + test("an archive of an already archived workspace keeps the earlier archive's agents for undo", async () => { + const operations = new WorkspaceArchiveOperations(); + await operations.archive("ws", async () => archived(["agent-a"])); + await operations.archive("ws", async () => ({ + archivedAgentIds: ["agent-a"], + newlyArchivedAgentIds: [], + archivedWorkspaceIds: [], + removedDirectory: false, + })); + const restored: string[][] = []; + + await operations.undo("ws", async ({ agentIds }) => { + restored.push(agentIds); + }); + + expect(restored).toEqual([["agent-a"]]); + }); + test("an undo restores the archive's agents once; a repeat undo restores none", async () => { const operations = new WorkspaceArchiveOperations(); await operations.archive("ws", async () => archived(["agent-a"])); diff --git a/packages/server/src/server/workspace-archive-operations.ts b/packages/server/src/server/workspace-archive-operations.ts index ed1d66b5cb..c8bb15a9d2 100644 --- a/packages/server/src/server/workspace-archive-operations.ts +++ b/packages/server/src/server/workspace-archive-operations.ts @@ -31,7 +31,8 @@ const DEFAULT_UNDO_RETENTION_MS = 10 * 60_000; * Each call claims the lane synchronously, so whichever request arrives last decides the * workspace's final state no matter how the earlier work interleaves. * - * The agents an archive took down are kept in memory for the undo window. After a daemon + * The agents an archive took down are kept in memory for the undo window. An archive that took + * nothing down, such as a repeat archive from a stale client, leaves them alone. After a daemon * restart, undo still restores the workspace but its agents stay archived. */ export class WorkspaceArchiveOperations { @@ -58,11 +59,15 @@ export class WorkspaceArchiveOperations { await settled(current?.done); try { const result = await run(controller.signal); - this.retainUndoable(workspaceId, result.newlyArchivedAgentIds); + if (result.archivedWorkspaceIds.length > 0 || result.newlyArchivedAgentIds.length > 0) { + this.retainUndoable(workspaceId, result.newlyArchivedAgentIds); + } return { kind: "archived", result }; } catch (error) { if (!(error instanceof WorkspaceArchiveCanceledError)) throw error; - this.retainUndoable(workspaceId, error.newlyArchivedAgentIds); + if (error.recordArchived || error.newlyArchivedAgentIds.length > 0) { + this.retainUndoable(workspaceId, error.newlyArchivedAgentIds); + } return { kind: "canceled" }; } })(); diff --git a/packages/server/src/server/workspace-archive-service.test.ts b/packages/server/src/server/workspace-archive-service.test.ts index ac2bfbd565..ecf4210ece 100644 --- a/packages/server/src/server/workspace-archive-service.test.ts +++ b/packages/server/src/server/workspace-archive-service.test.ts @@ -1191,6 +1191,34 @@ describe("archiveByScope cancellation", () => { expect(result.archivedAgentIds.sort()).toEqual(["agent-active", "agent-already-archived"]); expect(result.newlyArchivedAgentIds).toEqual(["agent-active"]); }); + + test("an archived agent loaded to read its history is not reported as newly archived", async () => { + const { tempDir, repoDir } = createGitRepo(); + const paseoHome = path.join(tempDir, ".paseo"); + const worktree = await createPaseoOwnedWorktree(repoDir, paseoHome, "loaded-archived"); + const workspaceId = "ws-loaded-archived"; + const deps = createArchiveDeps({ + paseoHome, + activeWorkspaces: [{ workspaceId, cwd: worktree.worktreePath, kind: "worktree" }], + }); + deps.agentStorage = storedAgents(workspaceId, [ + { id: "agent-active", archivedAt: null }, + { id: "agent-history", archivedAt: "2026-01-01T00:00:00.000Z" }, + ]); + deps.agentManager.listAgents = () => + [ + { id: "agent-active", workspaceId }, + { id: "agent-history", workspaceId }, + ] as ManagedAgent[]; + + const result = await archiveByScope(deps, { + scope: { kind: "workspace", workspaceId }, + requestId: "req-loaded-archived", + signal: new AbortController().signal, + }); + + expect(result.newlyArchivedAgentIds).toEqual(["agent-active"]); + }); }); describe("resolveWorkspaceIdAtPath", () => { diff --git a/packages/server/src/server/workspace-archive-service.ts b/packages/server/src/server/workspace-archive-service.ts index 79d77eee54..54d772add1 100644 --- a/packages/server/src/server/workspace-archive-service.ts +++ b/packages/server/src/server/workspace-archive-service.ts @@ -612,11 +612,15 @@ async function archiveWorkspaceContentsWithProgress( ...matchingStoredRecords.filter((record) => !record.archivedAt).map((record) => record.id), ]), ]; - // Internal agents are retired rather than archived: nothing on disk can bring them back. + // Undo brings back only what this archive took down. Internal agents are retired rather than + // archived, and an agent loaded only to read its history was archived before this request. const restorableAgentIds = new Set(agentIdsToArchive); for (const agent of liveAgents) { if (agent.internal) restorableAgentIds.delete(agent.id); } + for (const record of matchingStoredRecords) { + if (record.archivedAt) restorableAgentIds.delete(record.id); + } const [agentResults, [terminalResult]] = await Promise.all([ Promise.allSettled( agentIdsToArchive.map((agentId) => diff --git a/packages/server/src/server/workspace-archive-undo.e2e.test.ts b/packages/server/src/server/workspace-archive-undo.e2e.test.ts index a9399af5de..31b23bf9df 100644 --- a/packages/server/src/server/workspace-archive-undo.e2e.test.ts +++ b/packages/server/src/server/workspace-archive-undo.e2e.test.ts @@ -190,6 +190,36 @@ test.skipIf(process.platform === "win32")( 60000, ); +test.skipIf(process.platform === "win32")( + "an undo sent right behind its archive leaves the workspace active", + async () => { + const repo = createRepoWithGatedTeardown(); + repo.releaseTeardown(); + const { workspaceId, directory, agentId } = await createWorktreeWorkspaceWithAgent( + repo.repoDir, + "archive-then-undo-same-tick", + ); + + const [archiveResponse] = await Promise.all([ + ctx.client.archiveWorkspace(workspaceId), + ctx.client.undoWorkspaceArchive(workspaceId), + ]); + + expect({ + archiveCanceled: archiveResponse.canceled ?? false, + workspaceActive: await isWorkspaceActive(workspaceId), + directoryExists: existsSync(directory), + agentArchived: await isAgentArchived(agentId), + }).toEqual({ + archiveCanceled: true, + workspaceActive: true, + directoryExists: true, + agentArchived: false, + }); + }, + 60000, +); + test("an undo on a workspace that was never archived leaves it as it was", async () => { const repo = createRepoWithGatedTeardown(); repo.releaseTeardown();