From cee45dbaf826e6efb3807c9d6e7afefa5efa659d Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Mon, 20 Jul 2026 10:09:56 -0400 Subject: [PATCH 1/2] fix: archive tasks optimistically Remove archived tasks from the UI before host cleanup completes, while preserving rollback on failure and refreshing the authoritative archive record after success. Generated-By: PostHog Code Task-Id: ac916f1c-ce06-4167-b1bb-726a17a3fa02 --- .../src/archive/archiveOrchestration.test.ts | 48 +++++------ .../core/src/archive/archiveOrchestration.ts | 82 +++++++++---------- .../core/src/archive/optimisticArchive.ts | 9 +- .../ui/src/features/archive/useArchiveTask.ts | 7 +- 4 files changed, 69 insertions(+), 77 deletions(-) diff --git a/packages/core/src/archive/archiveOrchestration.test.ts b/packages/core/src/archive/archiveOrchestration.test.ts index 08ae2b9e37..5e91d41c6b 100644 --- a/packages/core/src/archive/archiveOrchestration.test.ts +++ b/packages/core/src/archive/archiveOrchestration.test.ts @@ -33,6 +33,7 @@ class Harness { logError: vi.fn(), cache: { cancelPathFilter: vi.fn().mockResolvedValue(undefined), + invalidateArchiveList: vi.fn(), invalidatePathFilter: vi.fn(), setArchivedTaskIds: (updater) => { this.ids = updater(this.ids); @@ -61,45 +62,34 @@ describe("archiveTask", () => { expect(harness.deps.archive).toHaveBeenCalledWith(TASK_ID); expect(harness.deps.disconnectFromTask).toHaveBeenCalledWith(TASK_ID); expect(harness.deps.clearViewedState).toHaveBeenCalledWith(TASK_ID); - expect(harness.ids).toContain(TASK_ID); - expect(harness.list.some((a) => a.taskId === TASK_ID)).toBe(true); - }); - - it("does not clear read state when the archive request fails", async () => { - harness.deps.archive = vi.fn().mockRejectedValue(new Error("boom")); - - await expect(archiveTask(TASK_ID, harness.deps)).rejects.toThrow("boom"); - - expect(harness.deps.clearViewedState).not.toHaveBeenCalled(); + expect(harness.ids).toEqual([TASK_ID]); + expect(harness.list.map((task) => task.taskId)).toEqual([TASK_ID]); }); - it("with optimistic:false, defers cache writes until archive resolves", async () => { - let idsWhenArchiveCalled: string[] = ["sentinel"]; - harness.deps.archive = vi.fn().mockImplementation(async () => { - // Snapshot the cache at the moment the request is made — the row must - // still be present (not yet marked archived) while it's in flight. - idsWhenArchiveCalled = [...harness.ids]; - }); + it("updates both archive caches before host preparation resolves", async () => { + let resolveCancellation: () => void = () => undefined; + harness.deps.cache.cancelPathFilter = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + resolveCancellation = resolve; + }), + ); - await archiveTask(TASK_ID, harness.deps, { optimistic: false }); + const result = archiveTask(TASK_ID, harness.deps); - expect(idsWhenArchiveCalled).not.toContain(TASK_ID); - // Once the archive resolves, the row is removed from the list. expect(harness.ids).toContain(TASK_ID); - expect(harness.list.some((a) => a.taskId === TASK_ID)).toBe(true); + expect(harness.list.some((task) => task.taskId === TASK_ID)).toBe(true); + + resolveCancellation(); + await result; }); - it("with optimistic:false, leaves caches untouched when archive fails", async () => { - harness.deps.getPinnedTaskIds = vi.fn().mockResolvedValue([TASK_ID]); + it("does not clear read state when the archive request fails", async () => { harness.deps.archive = vi.fn().mockRejectedValue(new Error("boom")); - await expect( - archiveTask(TASK_ID, harness.deps, { optimistic: false }), - ).rejects.toThrow("boom"); + await expect(archiveTask(TASK_ID, harness.deps)).rejects.toThrow("boom"); - expect(harness.ids).not.toContain(TASK_ID); - expect(harness.list).toEqual([]); - expect(harness.deps.togglePin).toHaveBeenCalledWith(TASK_ID); + expect(harness.deps.clearViewedState).not.toHaveBeenCalled(); }); it("rolls back caches and re-pins when archive fails", async () => { diff --git a/packages/core/src/archive/archiveOrchestration.ts b/packages/core/src/archive/archiveOrchestration.ts index 4da7a7690a..831f8f1d4d 100644 --- a/packages/core/src/archive/archiveOrchestration.ts +++ b/packages/core/src/archive/archiveOrchestration.ts @@ -14,6 +14,7 @@ export interface ArchiveWorkspaceInfo extends OptimisticWorkspaceInfo { export interface ArchiveCacheWriter { cancelPathFilter(): Promise; + invalidateArchiveList(): void; invalidatePathFilter(): void; setArchivedTaskIds(updater: (old: string[] | undefined) => string[]): void; setArchiveList( @@ -46,13 +47,6 @@ export interface ArchiveOrchestrationDeps { export interface ArchiveTaskOptions { skipNavigate?: boolean; - /** - * When true (default), the task is removed from the sidebar list immediately - * via an optimistic cache write and rolled back on failure. When false, the - * row stays put until the archive actually succeeds — used by the interactive - * single-archive flow so the row can show a spinner until it's confirmed gone. - */ - optimistic?: boolean; } export async function archiveTask( @@ -60,64 +54,68 @@ export async function archiveTask( deps: ArchiveOrchestrationDeps, options?: ArchiveTaskOptions, ): Promise { - const workspace = await deps.getWorkspace(taskId); - const stopped = await deps.stopCloudRun(taskId); - if (!stopped) { - throw new Error("Couldn't stop the task. Try again in a moment."); - } - - const optimistic = options?.optimistic ?? true; - const pinnedTaskIds = await deps.getPinnedTaskIds(); - const wasPinned = pinnedTaskIds.includes(taskId); - if (!options?.skipNavigate) { deps.navigateAwayFromTaskIfActive(taskId); } const commandCenterSnapshot = deps.snapshotCommandCenter(taskId); - - await deps.unpin(taskId); deps.removeFromCommandCenter(taskId); - await deps.cache.cancelPathFilter(); + const optimisticArchived = buildOptimisticArchivedTask(taskId, null); + deps.cache.setArchivedTaskIds((old) => appendArchivedTaskId(old, taskId)); + deps.cache.setArchiveList((old) => + appendOptimisticArchivedTask(old, optimisticArchived), + ); + + let wasPinned = false; + let didUnpin = false; + + try { + const cancelPathFilter = deps.cache.cancelPathFilter(); + await cancelPathFilter; + const [workspace, pinnedTaskIds, stopped] = await Promise.all([ + deps.getWorkspace(taskId), + deps.getPinnedTaskIds(), + deps.stopCloudRun(taskId), + ]); + if (!stopped) { + throw new Error("Couldn't stop the task. Try again in a moment."); + } - const optimisticArchived = buildOptimisticArchivedTask(taskId, workspace); + wasPinned = pinnedTaskIds.includes(taskId); + await deps.unpin(taskId); + didUnpin = true; - const applyArchivedCacheWrites = () => { - deps.cache.setArchivedTaskIds((old) => appendArchivedTaskId(old, taskId)); deps.cache.setArchiveList((old) => - appendOptimisticArchivedTask(old, optimisticArchived), + appendOptimisticArchivedTask( + old, + buildOptimisticArchivedTask( + taskId, + workspace, + optimisticArchived.archivedAt, + ), + ), ); - }; - - if (optimistic) { - applyArchivedCacheWrites(); - } - if ( - workspace?.worktreePath && - deps.getFocusedWorktreePath() === workspace.worktreePath - ) { - await deps.disableFocus(); - } + if ( + workspace?.worktreePath && + deps.getFocusedWorktreePath() === workspace.worktreePath + ) { + await deps.disableFocus(); + } - try { await deps.disconnectFromTask(taskId); await deps.archive(taskId); deps.clearTerminalStates(taskId); deps.clearViewedState(taskId); - // Non-optimistic flows keep the row visible during the request, then remove - // it the moment the archive succeeds. - if (!optimistic) { - applyArchivedCacheWrites(); - } + deps.cache.invalidateArchiveList(); deps.cache.invalidatePathFilter(); } catch (error) { deps.logError("Failed to archive task", error); deps.cache.setArchivedTaskIds((old) => removeArchivedTaskId(old, taskId)); deps.cache.setArchiveList((old) => removeArchivedTask(old, taskId)); - if (wasPinned) { + if (wasPinned && didUnpin) { await deps.togglePin(taskId); } if (commandCenterSnapshot.index !== -1) { diff --git a/packages/core/src/archive/optimisticArchive.ts b/packages/core/src/archive/optimisticArchive.ts index 2494f9da84..e5fb5fe287 100644 --- a/packages/core/src/archive/optimisticArchive.ts +++ b/packages/core/src/archive/optimisticArchive.ts @@ -27,7 +27,8 @@ export function appendArchivedTaskId( old: string[] | undefined, taskId: string, ): string[] { - return old ? [...old, taskId] : [taskId]; + if (!old) return [taskId]; + return old.includes(taskId) ? old : [...old, taskId]; } export function removeArchivedTaskId( @@ -41,7 +42,11 @@ export function appendOptimisticArchivedTask( old: ArchivedTask[] | undefined, optimistic: ArchivedTask, ): ArchivedTask[] { - return old ? [...old, optimistic] : [optimistic]; + if (!old) return [optimistic]; + return [ + ...old.filter((task) => task.taskId !== optimistic.taskId), + optimistic, + ]; } export function removeArchivedTask( diff --git a/packages/ui/src/features/archive/useArchiveTask.ts b/packages/ui/src/features/archive/useArchiveTask.ts index a83828320d..6c1e106a12 100644 --- a/packages/ui/src/features/archive/useArchiveTask.ts +++ b/packages/ui/src/features/archive/useArchiveTask.ts @@ -59,6 +59,9 @@ function makeCacheWriter( return { cancelPathFilter: () => queryClient.cancelQueries({ queryKey: keys.archivePathFilterKey }), + invalidateArchiveList: () => { + queryClient.invalidateQueries({ queryKey: keys.archiveListQueryKey }); + }, invalidatePathFilter: () => { queryClient.invalidateQueries({ queryKey: keys.archivePathFilterKey }); }, @@ -141,7 +144,6 @@ export async function archiveTaskImperative( keys: ArchiveCacheKeys, options?: { skipNavigate?: boolean; - optimistic?: boolean; navigateSpace?: "code" | "website"; }, ): Promise { @@ -180,10 +182,7 @@ export function useArchiveTask(options?: { const { restore } = useUnarchiveTask(); const archiveTask = async ({ taskId }: { taskId: string }) => { - // Non-optimistic: keep the row in place (with a spinner) until the archive - // is confirmed, rather than removing it instantly and rolling back on error. await archiveTaskImperative(taskId, queryClient, keys, { - optimistic: false, navigateSpace: options?.navigateSpace, }); const toastId = `archive-undo-${taskId}`; From c5bfe0119bb290418e6330307b8e646234aa493c Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Mon, 20 Jul 2026 10:12:58 -0400 Subject: [PATCH 2/2] refactor: use existing optimistic archive option Restore the existing archive orchestration API and enable its optimistic mode explicitly for single-task archives. Generated-By: PostHog Code Task-Id: ac916f1c-ce06-4167-b1bb-726a17a3fa02 --- .../src/archive/archiveOrchestration.test.ts | 48 ++++++----- .../core/src/archive/archiveOrchestration.ts | 82 ++++++++++--------- .../core/src/archive/optimisticArchive.ts | 9 +- .../ui/src/features/archive/useArchiveTask.ts | 5 +- 4 files changed, 75 insertions(+), 69 deletions(-) diff --git a/packages/core/src/archive/archiveOrchestration.test.ts b/packages/core/src/archive/archiveOrchestration.test.ts index 5e91d41c6b..08ae2b9e37 100644 --- a/packages/core/src/archive/archiveOrchestration.test.ts +++ b/packages/core/src/archive/archiveOrchestration.test.ts @@ -33,7 +33,6 @@ class Harness { logError: vi.fn(), cache: { cancelPathFilter: vi.fn().mockResolvedValue(undefined), - invalidateArchiveList: vi.fn(), invalidatePathFilter: vi.fn(), setArchivedTaskIds: (updater) => { this.ids = updater(this.ids); @@ -62,34 +61,45 @@ describe("archiveTask", () => { expect(harness.deps.archive).toHaveBeenCalledWith(TASK_ID); expect(harness.deps.disconnectFromTask).toHaveBeenCalledWith(TASK_ID); expect(harness.deps.clearViewedState).toHaveBeenCalledWith(TASK_ID); - expect(harness.ids).toEqual([TASK_ID]); - expect(harness.list.map((task) => task.taskId)).toEqual([TASK_ID]); + expect(harness.ids).toContain(TASK_ID); + expect(harness.list.some((a) => a.taskId === TASK_ID)).toBe(true); }); - it("updates both archive caches before host preparation resolves", async () => { - let resolveCancellation: () => void = () => undefined; - harness.deps.cache.cancelPathFilter = vi.fn().mockImplementation( - () => - new Promise((resolve) => { - resolveCancellation = resolve; - }), - ); + it("does not clear read state when the archive request fails", async () => { + harness.deps.archive = vi.fn().mockRejectedValue(new Error("boom")); + + await expect(archiveTask(TASK_ID, harness.deps)).rejects.toThrow("boom"); - const result = archiveTask(TASK_ID, harness.deps); + expect(harness.deps.clearViewedState).not.toHaveBeenCalled(); + }); - expect(harness.ids).toContain(TASK_ID); - expect(harness.list.some((task) => task.taskId === TASK_ID)).toBe(true); + it("with optimistic:false, defers cache writes until archive resolves", async () => { + let idsWhenArchiveCalled: string[] = ["sentinel"]; + harness.deps.archive = vi.fn().mockImplementation(async () => { + // Snapshot the cache at the moment the request is made — the row must + // still be present (not yet marked archived) while it's in flight. + idsWhenArchiveCalled = [...harness.ids]; + }); + + await archiveTask(TASK_ID, harness.deps, { optimistic: false }); - resolveCancellation(); - await result; + expect(idsWhenArchiveCalled).not.toContain(TASK_ID); + // Once the archive resolves, the row is removed from the list. + expect(harness.ids).toContain(TASK_ID); + expect(harness.list.some((a) => a.taskId === TASK_ID)).toBe(true); }); - it("does not clear read state when the archive request fails", async () => { + it("with optimistic:false, leaves caches untouched when archive fails", async () => { + harness.deps.getPinnedTaskIds = vi.fn().mockResolvedValue([TASK_ID]); harness.deps.archive = vi.fn().mockRejectedValue(new Error("boom")); - await expect(archiveTask(TASK_ID, harness.deps)).rejects.toThrow("boom"); + await expect( + archiveTask(TASK_ID, harness.deps, { optimistic: false }), + ).rejects.toThrow("boom"); - expect(harness.deps.clearViewedState).not.toHaveBeenCalled(); + expect(harness.ids).not.toContain(TASK_ID); + expect(harness.list).toEqual([]); + expect(harness.deps.togglePin).toHaveBeenCalledWith(TASK_ID); }); it("rolls back caches and re-pins when archive fails", async () => { diff --git a/packages/core/src/archive/archiveOrchestration.ts b/packages/core/src/archive/archiveOrchestration.ts index 831f8f1d4d..4da7a7690a 100644 --- a/packages/core/src/archive/archiveOrchestration.ts +++ b/packages/core/src/archive/archiveOrchestration.ts @@ -14,7 +14,6 @@ export interface ArchiveWorkspaceInfo extends OptimisticWorkspaceInfo { export interface ArchiveCacheWriter { cancelPathFilter(): Promise; - invalidateArchiveList(): void; invalidatePathFilter(): void; setArchivedTaskIds(updater: (old: string[] | undefined) => string[]): void; setArchiveList( @@ -47,6 +46,13 @@ export interface ArchiveOrchestrationDeps { export interface ArchiveTaskOptions { skipNavigate?: boolean; + /** + * When true (default), the task is removed from the sidebar list immediately + * via an optimistic cache write and rolled back on failure. When false, the + * row stays put until the archive actually succeeds — used by the interactive + * single-archive flow so the row can show a spinner until it's confirmed gone. + */ + optimistic?: boolean; } export async function archiveTask( @@ -54,68 +60,64 @@ export async function archiveTask( deps: ArchiveOrchestrationDeps, options?: ArchiveTaskOptions, ): Promise { + const workspace = await deps.getWorkspace(taskId); + const stopped = await deps.stopCloudRun(taskId); + if (!stopped) { + throw new Error("Couldn't stop the task. Try again in a moment."); + } + + const optimistic = options?.optimistic ?? true; + const pinnedTaskIds = await deps.getPinnedTaskIds(); + const wasPinned = pinnedTaskIds.includes(taskId); + if (!options?.skipNavigate) { deps.navigateAwayFromTaskIfActive(taskId); } const commandCenterSnapshot = deps.snapshotCommandCenter(taskId); - deps.removeFromCommandCenter(taskId); - const optimisticArchived = buildOptimisticArchivedTask(taskId, null); - deps.cache.setArchivedTaskIds((old) => appendArchivedTaskId(old, taskId)); - deps.cache.setArchiveList((old) => - appendOptimisticArchivedTask(old, optimisticArchived), - ); - - let wasPinned = false; - let didUnpin = false; + await deps.unpin(taskId); + deps.removeFromCommandCenter(taskId); - try { - const cancelPathFilter = deps.cache.cancelPathFilter(); - await cancelPathFilter; - const [workspace, pinnedTaskIds, stopped] = await Promise.all([ - deps.getWorkspace(taskId), - deps.getPinnedTaskIds(), - deps.stopCloudRun(taskId), - ]); - if (!stopped) { - throw new Error("Couldn't stop the task. Try again in a moment."); - } + await deps.cache.cancelPathFilter(); - wasPinned = pinnedTaskIds.includes(taskId); - await deps.unpin(taskId); - didUnpin = true; + const optimisticArchived = buildOptimisticArchivedTask(taskId, workspace); + const applyArchivedCacheWrites = () => { + deps.cache.setArchivedTaskIds((old) => appendArchivedTaskId(old, taskId)); deps.cache.setArchiveList((old) => - appendOptimisticArchivedTask( - old, - buildOptimisticArchivedTask( - taskId, - workspace, - optimisticArchived.archivedAt, - ), - ), + appendOptimisticArchivedTask(old, optimisticArchived), ); + }; - if ( - workspace?.worktreePath && - deps.getFocusedWorktreePath() === workspace.worktreePath - ) { - await deps.disableFocus(); - } + if (optimistic) { + applyArchivedCacheWrites(); + } + if ( + workspace?.worktreePath && + deps.getFocusedWorktreePath() === workspace.worktreePath + ) { + await deps.disableFocus(); + } + + try { await deps.disconnectFromTask(taskId); await deps.archive(taskId); deps.clearTerminalStates(taskId); deps.clearViewedState(taskId); - deps.cache.invalidateArchiveList(); + // Non-optimistic flows keep the row visible during the request, then remove + // it the moment the archive succeeds. + if (!optimistic) { + applyArchivedCacheWrites(); + } deps.cache.invalidatePathFilter(); } catch (error) { deps.logError("Failed to archive task", error); deps.cache.setArchivedTaskIds((old) => removeArchivedTaskId(old, taskId)); deps.cache.setArchiveList((old) => removeArchivedTask(old, taskId)); - if (wasPinned && didUnpin) { + if (wasPinned) { await deps.togglePin(taskId); } if (commandCenterSnapshot.index !== -1) { diff --git a/packages/core/src/archive/optimisticArchive.ts b/packages/core/src/archive/optimisticArchive.ts index e5fb5fe287..2494f9da84 100644 --- a/packages/core/src/archive/optimisticArchive.ts +++ b/packages/core/src/archive/optimisticArchive.ts @@ -27,8 +27,7 @@ export function appendArchivedTaskId( old: string[] | undefined, taskId: string, ): string[] { - if (!old) return [taskId]; - return old.includes(taskId) ? old : [...old, taskId]; + return old ? [...old, taskId] : [taskId]; } export function removeArchivedTaskId( @@ -42,11 +41,7 @@ export function appendOptimisticArchivedTask( old: ArchivedTask[] | undefined, optimistic: ArchivedTask, ): ArchivedTask[] { - if (!old) return [optimistic]; - return [ - ...old.filter((task) => task.taskId !== optimistic.taskId), - optimistic, - ]; + return old ? [...old, optimistic] : [optimistic]; } export function removeArchivedTask( diff --git a/packages/ui/src/features/archive/useArchiveTask.ts b/packages/ui/src/features/archive/useArchiveTask.ts index 6c1e106a12..910c17788d 100644 --- a/packages/ui/src/features/archive/useArchiveTask.ts +++ b/packages/ui/src/features/archive/useArchiveTask.ts @@ -59,9 +59,6 @@ function makeCacheWriter( return { cancelPathFilter: () => queryClient.cancelQueries({ queryKey: keys.archivePathFilterKey }), - invalidateArchiveList: () => { - queryClient.invalidateQueries({ queryKey: keys.archiveListQueryKey }); - }, invalidatePathFilter: () => { queryClient.invalidateQueries({ queryKey: keys.archivePathFilterKey }); }, @@ -144,6 +141,7 @@ export async function archiveTaskImperative( keys: ArchiveCacheKeys, options?: { skipNavigate?: boolean; + optimistic?: boolean; navigateSpace?: "code" | "website"; }, ): Promise { @@ -183,6 +181,7 @@ export function useArchiveTask(options?: { const archiveTask = async ({ taskId }: { taskId: string }) => { await archiveTaskImperative(taskId, queryClient, keys, { + optimistic: true, navigateSpace: options?.navigateSpace, }); const toastId = `archive-undo-${taskId}`;