From 140ef39cc69f56a2def232c5a2354d4e49dd60f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Sat, 12 Sep 2026 16:54:56 +0200 Subject: [PATCH] fix(threads): remove archived chats immediately (#3625) --- .../app/threads/threads-page-archive.test.tsx | 384 ++++++++++++++++++ .../app/threads/threads-page-client.test.tsx | 6 +- apps/web/app/threads/threads-page-client.tsx | 77 ++-- apps/web/components/threads/AGENTS.md | 23 +- .../threads/thread-column-activation.test.tsx | 20 + .../components/threads/threads-board.test.tsx | 76 +++- apps/web/components/threads/threads-board.tsx | 37 +- .../threads/use-thread-column-activation.ts | 15 +- .../threads/use-thread-focus-request.test.ts | 46 +++ .../threads/use-thread-focus-request.ts | 24 ++ .../task/mobile-threads-task-actions.spec.ts | 14 + .../task/threads-pending-archive-helpers.ts | 145 +++++++ .../task/threads-task-actions-edge-helpers.ts | 3 + .../task/threads-task-actions-helpers.ts | 3 +- .../tests/task/threads-task-actions.spec.ts | 13 + .../web/lib/threads/thread-view-query.test.ts | 60 +++ apps/web/lib/threads/thread-view-query.ts | 6 +- docs/plans/threads-immediate-archive/plan.md | 232 +++++++++++ .../task-01-remove-pending-archives.md | 277 +++++++++++++ docs/public/sessions-and-review.md | 5 + .../requirements/threads-task-actions.md | 20 +- .../system-design/threads-task-actions.md | 85 +++- 22 files changed, 1492 insertions(+), 79 deletions(-) create mode 100644 apps/web/app/threads/threads-page-archive.test.tsx create mode 100644 apps/web/components/threads/use-thread-focus-request.test.ts create mode 100644 apps/web/components/threads/use-thread-focus-request.ts create mode 100644 apps/web/e2e/tests/task/threads-pending-archive-helpers.ts create mode 100644 docs/plans/threads-immediate-archive/plan.md create mode 100644 docs/plans/threads-immediate-archive/task-01-remove-pending-archives.md diff --git a/apps/web/app/threads/threads-page-archive.test.tsx b/apps/web/app/threads/threads-page-archive.test.tsx new file mode 100644 index 0000000000..021df8a7c4 --- /dev/null +++ b/apps/web/app/threads/threads-page-archive.test.tsx @@ -0,0 +1,384 @@ +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { useEffect } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { StateProvider, useAppStore, useAppStoreApi } from "@/components/state-provider"; +import { createAppStore, type HydrationState } from "@/lib/state/store"; +import type { KanbanState } from "@/lib/state/slices/kanban/types"; +import { sessionId as toSessionId, taskId as toTaskId, type TaskSession } from "@/lib/types/http"; +import { useTaskMenuActions } from "@/hooks/use-task-menu-actions"; +import { registerTasksHandlers } from "@/lib/ws/handlers/tasks"; +import { DEFAULT_THREAD_VIEW } from "@/lib/state/slices/ui/thread-view-builtins"; +import { ThreadsPageClient } from "./threads-page-client"; + +const transport = vi.hoisted(() => ({ archive: vi.fn(), remove: vi.fn(), toast: vi.fn() })); +const route = vi.hoisted(() => ({ search: "", push: vi.fn(), replace: vi.fn() })); +const setView = vi.hoisted(() => vi.fn()); +const activeChats = new Set(); + +vi.mock("@/lib/api", async (original) => ({ + ...(await original()), + archiveTask: transport.archive, + deleteTask: transport.remove, +})); +vi.mock("@/lib/routing/client-router", async (original) => ({ + ...(await original()), + useRouter: () => route, + useSearchParams: () => new URLSearchParams(route.search), +})); +vi.mock("@/src/kanban-route", () => ({ useKanbanRouteBootstrap: () => {} })); +vi.mock("@/hooks/domains/kanban/use-all-workflow-snapshots", () => ({ + useAllWorkflowSnapshots: () => {}, +})); +vi.mock("@/hooks/use-task-listing-view", () => ({ + useTaskListingView: () => ({ setView }), +})); +vi.mock("@/hooks/use-kanban-display-settings", () => ({ + useKanbanDisplaySettings: () => ({ + activeWorkspaceId: useAppStore((state) => state.workspaces.activeId), + workspaces: useAppStore((state) => state.workspaces.items), + workflows: useAppStore((state) => state.workflows.items), + repositories: [], + }), +})); +vi.mock("@/components/kanban/kanban-header", () => ({ KanbanHeader: () => null })); +vi.mock("@/components/toast-provider", () => ({ useToast: () => ({ toast: transport.toast }) })); +vi.mock("@/components/threads/thread-conversation", () => ({ + ThreadConversation: ({ taskId }: { taskId: string }) => { + useEffect(() => { + activeChats.add(taskId); + return () => { + activeChats.delete(taskId); + }; + }, [taskId]); + return null; + }, +})); + +let store: ReturnType; +let actions: ReturnType; + +function Harness() { + store = useAppStoreApi(); + actions = useTaskMenuActions({ stayOnListing: true }); + return ; +} + +function task(id: string, parentTaskId?: string): KanbanState["tasks"][number] { + return { + id, + title: `Task ${id}`, + workspaceId: "workspace", + workflowId: "workflow", + workflowStepId: "step", + position: 0, + state: "IN_PROGRESS", + parentTaskId, + primarySessionId: `session-${id}`, + primarySessionState: "RUNNING", + }; +} + +function renderThreads(tasks = [task("a"), task("b"), task("c")]) { + const base = createAppStore().getState(); + const sessions = tasks.map( + ({ id }): TaskSession => ({ + id: toSessionId(`session-${id}`), + task_id: toTaskId(id), + state: "RUNNING", + is_primary: true, + started_at: "2026-09-12T08:00:00Z", + updated_at: "2026-09-12T08:00:00Z", + }), + ); + const initialState: HydrationState = { + workspaces: { ...base.workspaces, activeId: "workspace" }, + workflows: { + activeId: null, + items: [{ id: "workflow", workspaceId: "workspace", name: "Delivery" }], + }, + kanban: { ...base.kanban, workflowId: "workflow", tasks }, + kanbanMulti: { + ...base.kanbanMulti, + snapshots: { + workflow: { workflowId: "workflow", workflowName: "Delivery", steps: [], tasks }, + }, + }, + taskSessions: { + ...base.taskSessions, + items: Object.fromEntries(sessions.map((s) => [s.id, s])), + }, + taskSessionsByTask: { + ...base.taskSessionsByTask, + itemsByTaskId: Object.fromEntries(sessions.map((s) => [s.task_id, [s]])), + loadedByTaskId: Object.fromEntries(tasks.map(({ id }) => [id, true])), + }, + }; + return render( + + + , + ); +} + +function deferred() { + let resolve!: () => void; + let reject!: (reason: Error) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} + +function visibleIds() { + return screen.queryAllByTestId(/^thread-column-/).map((column) => column.dataset.threadColumnId); +} + +function publishArchive(id: string) { + registerTasksHandlers(store)["task.updated"]!({ + id: `archived-${id}`, + type: "notification", + action: "task.updated", + payload: { + task_id: id, + workspace_id: "workspace", + workflow_id: "workflow", + workflow_step_id: "step", + title: `Task ${id}`, + state: "IN_PROGRESS", + is_ephemeral: false, + archived_at: "2026-09-12T09:00:00Z", + }, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + route.search = ""; + window.history.replaceState({}, "", "/threads"); +}); +afterEach(() => { + cleanup(); + activeChats.clear(); +}); + +// @covers AC-TASKS-THREADS-ACTIONS-003.7, AC-TASKS-THREADS-ACTIONS-003.8 +describe("Threads pending archive lifecycle", () => { + it("removes an archived column before the request settles", async () => { + const pending = deferred(); + transport.archive.mockReturnValueOnce(pending.promise); + renderThreads(); + expect(activeChats.has("a")).toBe(true); + let result!: Promise; + act(() => { + result = actions.runArchive("a"); + }); + try { + expect(visibleIds()).toEqual(["b", "c"]); + expect(activeChats.has("a")).toBe(false); + expect(store.getState().kanbanMulti.snapshots.workflow.tasks.map(({ id }) => id)).toEqual([ + "a", + "b", + "c", + ]); + expect(transport.toast).not.toHaveBeenCalled(); + } finally { + await act(async () => { + pending.resolve(); + await result; + }); + } + expect(visibleIds()).toEqual(["b", "c"]); + expect(transport.toast).toHaveBeenCalledWith(expect.objectContaining({ variant: "success" })); + }); + + // @covers AC-TASKS-THREADS-ACTIONS-003.4, AC-TASKS-THREADS-ACTIONS-004.6 + it("readmits a rejected archive without reviving its consumed deep link", async () => { + route.search = "taskId=a&sessionId=session-a"; + const pending = deferred(); + transport.archive.mockReturnValueOnce(pending.promise); + renderThreads(); + fireEvent.pointerDown(screen.getByTestId("thread-column-a")); + let result!: Promise; + act(() => { + result = actions.runArchive("a"); + }); + try { + expect(visibleIds()).toEqual(["b", "c"]); + fireEvent.pointerDown(screen.getByTestId("thread-column-c")); + } finally { + await act(async () => { + pending.reject(new Error("archive rejected")); + await result; + }); + } + expect(visibleIds()).toEqual(["b", "c", "a"]); + expect(screen.getByTestId("thread-column-a").getAttribute("data-focused")).toBeNull(); + expect(route.replace).not.toHaveBeenCalled(); + expect(route.push).not.toHaveBeenCalled(); + expect(transport.toast).toHaveBeenCalledWith(expect.objectContaining({ variant: "error" })); + }); + + it.each(["event-first", "response-first"])( + "does not reopen successful archives with %s delivery", + async (order) => { + const pending = deferred(); + transport.archive.mockReturnValueOnce(pending.promise); + renderThreads([task("a")]); + let result!: Promise; + act(() => { + result = actions.runArchive("a"); + }); + try { + expect(screen.queryByTestId("threads-empty-state")).not.toBeNull(); + if (order === "event-first") act(() => publishArchive("a")); + } finally { + await act(async () => { + pending.resolve(); + await result; + }); + } + if (order === "response-first") act(() => publishArchive("a")); + expect(visibleIds()).toEqual([]); + expect(activeChats.has("a")).toBe(false); + expect(screen.queryByTestId("threads-empty-state")).not.toBeNull(); + expect(store.getState().taskRemoval.operationsByToken).toEqual({}); + }, + ); +}); + +describe("Threads independent removal intent", () => { + it("releases only the failed archive when two different tasks are pending", async () => { + const first = deferred(); + const second = deferred(); + transport.archive.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + renderThreads(); + let firstResult!: Promise; + let secondResult!: Promise; + act(() => { + firstResult = actions.runArchive("a"); + secondResult = actions.runArchive("b"); + }); + try { + expect(visibleIds()).toEqual(["c"]); + await act(async () => { + first.reject(new Error("archive rejected")); + await firstResult; + }); + expect(visibleIds()).toEqual(["c", "a"]); + expect(Object.values(store.getState().taskRemoval.operationsByToken)).toHaveLength(1); + } finally { + await act(async () => { + first.resolve(); + second.resolve(); + await Promise.all([firstResult, secondResult]); + }); + } + expect(visibleIds()).toEqual(["c", "a"]); + }); + + it("excludes captured descendants while leaving a pending delete visible", async () => { + const archive = deferred(); + const remove = deferred(); + transport.archive.mockReturnValueOnce(archive.promise); + transport.remove.mockReturnValueOnce(remove.promise); + route.search = "taskId=child"; + renderThreads([task("parent"), task("child", "parent"), task("b")]); + let archiveResult!: Promise; + let deleteResult!: Promise; + act(() => { + archiveResult = actions.runArchive("parent", { cascade: true }); + deleteResult = actions.runDelete("b"); + }); + try { + expect(visibleIds()).toEqual(["b"]); + expect(transport.archive).toHaveBeenCalledWith("parent", { cascade: true }); + } finally { + await act(async () => { + archive.resolve(); + remove.resolve(); + await Promise.all([archiveResult, deleteResult]); + }); + } + expect(visibleIds()).toEqual([]); + }); +}); + +describe("Threads archive rejection reconciliation", () => { + it.each(["workspace", "view"])("does not undo a newer %s choice on rejection", async (choice) => { + const pending = deferred(); + transport.archive.mockReturnValueOnce(pending.promise); + renderThreads(); + let result!: Promise; + act(() => { + result = actions.runArchive("a"); + }); + try { + act(() => + store.setState((state) => + choice === "workspace" + ? { + workspaces: { ...state.workspaces, activeId: "another-workspace" }, + } + : { + threadViews: { + ...state.threadViews, + draft: { + ...DEFAULT_THREAD_VIEW, + baseViewId: DEFAULT_THREAD_VIEW.id, + taskScope: { mode: "selected", taskIds: ["b"] }, + }, + }, + }, + ), + ); + } finally { + await act(async () => { + pending.reject(new Error("archive rejected")); + await result; + }); + } + expect(visibleIds()).toEqual(choice === "workspace" ? [] : ["b"]); + expect(route.push).not.toHaveBeenCalled(); + expect(route.replace).not.toHaveBeenCalled(); + }); + + it.each(["archive", "delete"])( + "keeps an authoritative %s removed after the request rejects", + async (event) => { + const pending = deferred(); + transport.archive.mockReturnValueOnce(pending.promise); + renderThreads([task("a")]); + let result!: Promise; + act(() => { + result = actions.runArchive("a"); + }); + try { + act(() => { + if (event === "archive") publishArchive("a"); + else + registerTasksHandlers(store)["task.deleted"]!({ + id: "deleted-a", + type: "notification", + action: "task.deleted", + payload: { + task_id: "a", + workspace_id: "workspace", + workflow_id: "workflow", + workflow_step_id: "step", + title: "Task a", + is_ephemeral: false, + }, + }); + }); + } finally { + await act(async () => { + pending.reject(new Error("response lost")); + await result; + }); + } + expect(visibleIds()).toEqual([]); + expect(screen.queryByTestId("threads-empty-state")).not.toBeNull(); + }, + ); +}); diff --git a/apps/web/app/threads/threads-page-client.test.tsx b/apps/web/app/threads/threads-page-client.test.tsx index 2fc38855f7..aa39051085 100644 --- a/apps/web/app/threads/threads-page-client.test.tsx +++ b/apps/web/app/threads/threads-page-client.test.tsx @@ -1,5 +1,6 @@ import { cleanup, render } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createTaskRemovalState } from "@/lib/state/task-removal"; const bootstrapMock = vi.hoisted(() => vi.fn()); const searchMock = vi.hoisted(() => ({ value: "" })); @@ -29,7 +30,10 @@ vi.mock("@/hooks/use-task-listing-view", () => ({ })); vi.mock("@/components/state-provider", () => ({ useAppStore: (selector: (state: unknown) => unknown) => - selector({ kanbanMulti: { snapshots: {}, isLoading: false } }), + selector({ + kanbanMulti: { snapshots: {}, isLoading: false }, + taskRemoval: createTaskRemovalState(), + }), })); import { scopeSnapshotsToWorkspace, ThreadsPageClient } from "./threads-page-client"; diff --git a/apps/web/app/threads/threads-page-client.tsx b/apps/web/app/threads/threads-page-client.tsx index f942dfb1c5..affc427540 100644 --- a/apps/web/app/threads/threads-page-client.tsx +++ b/apps/web/app/threads/threads-page-client.tsx @@ -77,6 +77,49 @@ export function scopeSnapshotsToWorkspace( ); } +function useThreadDeck( + snapshots: Record, + workspaceId: string | null | undefined, + requestedTaskId: string | null, +) { + const removals = useAppStore((state) => state.taskRemoval.operationsByToken); + const storedThreadViews = useAppStore((state) => state.threadViews); + const threadViews = storedThreadViews ?? { + views: [DEFAULT_THREAD_VIEW], + activeViewId: DEFAULT_THREAD_VIEW.id, + draft: null, + syncError: null, + orderResetGeneration: 0, + }; + const activeThreadView = + threadViews.views.find((view) => view.id === threadViews.activeViewId) ?? DEFAULT_THREAD_VIEW; + const query = useMemo( + () => + queryThreadView(snapshots, activeThreadView, { + workspaceId, + requestedTaskId, + draft: threadViews.draft, + excludedTaskIds: new Set( + Object.values(removals) + .filter((operation) => operation.action === "archive") + .flatMap((operation) => operation.taskIds), + ), + }), + [snapshots, workspaceId, activeThreadView, requestedTaskId, threadViews.draft, removals], + ); + // Ranking decides where a column first appears; after that the slot is the + // reader's, so replying to a thread cannot slide it across the deck. + const threads = useStableThreadOrder( + query.stableCandidates, + `${query.fingerprint}:${requestedTaskId ?? ""}:${threadViews.orderResetGeneration}`, + { + resetThreads: query.admittedCandidates, + maxItems: query.effectiveView.maxColumns, + }, + ); + return { query, threads }; +} + /** * The Threads page: every live agent conversation in the workspace, side by * side. It reads the workflow snapshots the board already keeps in the store, @@ -101,14 +144,6 @@ export function ThreadsPageClient() { const { setView } = useTaskListingView(); const snapshots = useAppStore((state) => state.kanbanMulti.snapshots); const isLoading = useAppStore((state) => state.kanbanMulti.isLoading); - const storedThreadViews = useAppStore((state) => state.threadViews); - const threadViews = storedThreadViews ?? { - views: [DEFAULT_THREAD_VIEW], - activeViewId: DEFAULT_THREAD_VIEW.id, - draft: null, - syncError: null, - orderResetGeneration: 0, - }; // Keep a valid deep-link workspace during the bootstrap transition, but use // the resolved active workspace when a stale or invalid link is supplied. @@ -130,28 +165,9 @@ export function ThreadsPageClient() { setView("threads"); }, [setView]); - const activeThreadView = - threadViews.views.find((view) => view.id === threadViews.activeViewId) ?? DEFAULT_THREAD_VIEW; const requestedTaskId = searchParams.get("taskId"); - const query = useMemo( - () => - queryThreadView(scopedSnapshots, activeThreadView, { - workspaceId: scopedWorkspaceId, - requestedTaskId, - draft: threadViews.draft, - }), - [scopedSnapshots, scopedWorkspaceId, activeThreadView, requestedTaskId, threadViews.draft], - ); - // Ranking decides where a column first appears; after that the slot is the - // reader's, so replying to a thread cannot slide it across the deck. - const threads = useStableThreadOrder( - query.stableCandidates, - `${query.fingerprint}:${requestedTaskId ?? ""}:${threadViews.orderResetGeneration}`, - { - resetThreads: query.admittedCandidates, - maxItems: query.effectiveView.maxColumns, - }, - ); + const requestedSessionId = searchParams.get("sessionId"); + const { query, threads } = useThreadDeck(scopedSnapshots, scopedWorkspaceId, requestedTaskId); const handleOpenTask = useCallback((taskId: string) => router.push(linkToTask(taskId)), [router]); const handleInvalidRequestedSession = useCallback( @@ -171,7 +187,7 @@ export function ThreadsPageClient() { // requested thread may have settled between the link being offered and // followed, and a focus id no column matches would ring nothing. const focusedTaskId = resolveFocusedThreadId(threads, searchParams.get("taskId")); - const focusedSessionId = focusedTaskId ? searchParams.get("sessionId") : null; + const focusedSessionId = focusedTaskId ? requestedSessionId : null; return (
@@ -180,6 +196,7 @@ export function ThreadsPageClient() { isLoading={isLoading} focusedTaskId={focusedTaskId} focusedSessionId={focusedSessionId} + focusRequestKey={JSON.stringify([scopedWorkspaceId, requestedTaskId, requestedSessionId])} onInvalidRequestedSession={handleInvalidRequestedSession} onOpenTask={handleOpenTask} renderHeader={(activeMobileTaskId) => ( diff --git a/apps/web/components/threads/AGENTS.md b/apps/web/components/threads/AGENTS.md index 9d459bdd9a..ab200c33c3 100644 --- a/apps/web/components/threads/AGENTS.md +++ b/apps/web/components/threads/AGENTS.md @@ -53,6 +53,10 @@ order without copying selection state. Do not derive pagination from loaded chat or visibility-ID membership alone: both adjacent columns can stay intersecting across a swipe midpoint. Position changes also refresh the nearest visible detail calculation, retaining the one-phone-transcript limit. +Callback refs reconcile column additions/removals on the existing observer. +Do not rebuild it on every membership change: clearing surviving visibility +briefly unmounts readers' chats and loses editor focus. Recreate observation +only when an empty/nonempty transition replaces the board element. `ThreadTaskActionsProvider` owns one task-action surface above removable columns. Headers pass explicit task IDs; `useTaskManagementFlow` captures the @@ -62,6 +66,15 @@ workspace/task identity and resolves current eligibility from shared snapshots. cleanup with the sidebar without task-detail navigation. Do not add task API calls or mutation policy to Threads. +The page excludes task IDs from pending `archive` operations in +`taskRemoval.operationsByToken` before `queryThreadView` applies scope, filters, +limits or temporary deep-link admission. This unmounts outgoing chats at +acceptance and keeps counts, the phone picker and pagination consistent. Do not +mutate shared snapshots optimistically or include removal intent in the query +fingerprint/stable-order reset key. Successful reconciliation prunes snapshots +before releasing intent; failure readmits only currently eligible tasks in +normal arrival order. Pending deletion retains its existing timing. + Only noninteractive desktop task-header regions handle context menus. The visible `TaskMenuButton` is also the keyboard/touch entry; phone and coarse pointers use one `TaskManagementDrawer` with nested pages. Keep chat, editor, @@ -93,8 +106,14 @@ column re-rendering with new messages does not yank the deck back. The mark retires on the first pointer or focus interaction with the deck, since it only ever answered "where is the column I asked for". Dismissal is keyed to -the requested id, so a later deep link earns a fresh mark rather than being -swallowed by an earlier dismissal. +the raw workspace/task/session request identity, independently of its currently +resolved column. Temporary exclusion and failed archive readmission must not +revive a consumed mark; an actual new deep link still earns a fresh mark. +`useThreadFocusRequest` keeps the initial activation fallback alive after mark +dismissal until its target leaves the deck. Early interaction must not unmount +that chat before the first visibility callback, and failed readmission must +not restore a consumed fallback. URL-driven callers pass `focusRequestKey`; +the optional task-ID default deliberately retains legacy dismissal semantics. `OpenInThreadsButton` is the other half, living in the chat status row. It has two gates, and both matter: diff --git a/apps/web/components/threads/thread-column-activation.test.tsx b/apps/web/components/threads/thread-column-activation.test.tsx index 9083dc08f7..a00d6e299b 100644 --- a/apps/web/components/threads/thread-column-activation.test.tsx +++ b/apps/web/components/threads/thread-column-activation.test.tsx @@ -259,6 +259,26 @@ describe("useThreadColumnActivation", () => { expect(ids(PRELOAD_IDS)).toEqual([TASK_B, TASK_C, TASK_D]); }); + // @covers AC-TASKS-THREADS-ACTIONS-003.5, AC-TASKS-THREADS-ACTIONS-003.6 + it.each([ + { change: "removal", nextIds: [TASK_A, TASK_B] }, + { change: "readmission", nextIds: [TASK_A, TASK_B, TASK_C, TASK_D] }, + ])("retains the surviving visible chat through $change", ({ nextIds }) => { + const view = render(); + observers[0].instance.emit({ + target: screen.getByTestId(`column-${TASK_B}`), + isIntersecting: true, + }); + expect(ids(DETAIL_IDS)).toEqual([TASK_B]); + + view.rerender(); + + expect(ids(DETAIL_IDS)).toEqual([TASK_B]); + expect(observers[0].instance.observed).toEqual( + new Set(nextIds.map((id) => screen.getByTestId(`column-${id}`))), + ); + }); + it("gives phone only the nearest visible snap column as detail owner", () => { responsiveMocks.useResponsiveBreakpoint.mockReturnValue(mobileBreakpoint()); render(); diff --git a/apps/web/components/threads/threads-board.test.tsx b/apps/web/components/threads/threads-board.test.tsx index 38efe9549e..bc1fdfa53b 100644 --- a/apps/web/components/threads/threads-board.test.tsx +++ b/apps/web/components/threads/threads-board.test.tsx @@ -75,6 +75,7 @@ afterEach(() => { const COLUMN_A = "thread-column-a"; const FOCUSED_ATTR = "data-focused"; const COLUMN_B = "thread-column-b"; +const CONVERSATION_A = "thread-conversation-session-a"; const TASK_A = "a"; const PRIMARY_SESSION_A = "session-a-primary"; const BUILDER_SESSION_A = "session-a-builder"; @@ -140,7 +141,7 @@ describe("ThreadsBoard — basic layout", () => { it("mounts the live conversation inside each column", () => { render( {}} />); - expect(screen.getByTestId("thread-conversation-session-a")).not.toBeNull(); + expect(screen.getByTestId(CONVERSATION_A)).not.toBeNull(); }); it("keeps thirty task shells mounted without mounting thirty conversations", () => { @@ -200,7 +201,7 @@ describe("ThreadsBoard — session list loading", () => { expect(screen.getByTestId("thread-session-list-error")).not.toBeNull(); expect(screen.getByRole("button", { name: /retry/i })).not.toBeNull(); - expect(screen.queryByTestId("thread-conversation-session-a")).toBeNull(); + expect(screen.queryByTestId(CONVERSATION_A)).toBeNull(); fireEvent.click(screen.getByRole("button", { name: /retry/i })); await act(async () => { @@ -210,7 +211,7 @@ describe("ThreadsBoard — session list loading", () => { await waitFor(() => { expect(screen.queryByTestId("thread-session-list-error")).toBeNull(); - expect(screen.getByTestId("thread-conversation-session-a")).not.toBeNull(); + expect(screen.getByTestId(CONVERSATION_A)).not.toBeNull(); }); }); }); @@ -479,7 +480,76 @@ describe("ThreadsBoard — focusing a column from a deep link", () => { }); }); +// @covers AC-TASKS-THREADS-ACTIONS-003.4 +describe("ThreadsBoard initial activation", () => { + it.each(["pointerDown", "focusIn"] as const)( + "keeps the requested conversation mounted when %s retires its mark before visibility is ready", + (interaction) => { + render( + {}} + />, + ); + const conversation = screen.getByTestId("thread-conversation-session-b"); + + fireEvent[interaction](screen.getByTestId(COLUMN_B)); + + expect(screen.getByTestId(COLUMN_B).getAttribute(FOCUSED_ATTR)).toBeNull(); + expect(screen.queryByTestId("thread-conversation-session-b")).toBe(conversation); + }, + ); +}); + describe("ThreadsBoard — retiring the deep-link mark", () => { + // @covers AC-TASKS-THREADS-ACTIONS-003.4, AC-TASKS-THREADS-ACTIONS-003.5 + it("keeps a consumed URL request retired when its excluded column returns", () => { + const props = { focusRequestKey: "workspace:b:session-b", onOpenTask: () => {} }; + const allThreads = [thread({ taskId: "a" }), thread({ taskId: "b" })]; + const view = render(); + fireEvent.pointerDown(screen.getByTestId(COLUMN_A)); + view.rerender( + , + ); + view.rerender(); + + expect(screen.getByTestId(COLUMN_B).getAttribute(FOCUSED_ATTR)).toBeNull(); + expect(screen.queryByTestId(CONVERSATION_A)).not.toBeNull(); + expect(screen.queryByTestId("thread-conversation-session-b")).toBeNull(); + }); + + it("honors a new URL request after a consumed request loses its resolved column", () => { + const props = { onOpenTask: () => {} }; + const view = render( + , + ); + fireEvent.pointerDown(screen.getByTestId(COLUMN_A)); + view.rerender( + , + ); + view.rerender( + , + ); + expect(screen.getByTestId(COLUMN_A).getAttribute(FOCUSED_ATTR)).toBe("true"); + }); + it("drops the mark once the reader touches the deck", () => { render( setRetired(true), - }; -} - function focusThreadPicker(event: Event, column: Element | undefined) { const trigger = column?.querySelector('[data-testid="thread-picker-trigger"]'); if (!trigger) return; @@ -97,19 +82,23 @@ export function ThreadsBoard({ threads, isLoading = false, focusedTaskId = null, + focusRequestKey = focusedTaskId, focusedSessionId = null, onInvalidRequestedSession, onOpenTask, renderHeader, }: ThreadsBoardProps) { - const { markedTaskId, retire } = useRetiringFocusMark(focusedTaskId); + const { markedTaskId, activationTaskId, retire } = useThreadFocusRequest( + focusedTaskId, + focusRequestKey, + ); const { isMobile } = useResponsiveBreakpoint(); const [pickerOpen, setPickerOpen] = useState(false); if (!isMobile && pickerOpen) setPickerOpen(false); const returnFocusTaskId = useRef(null); const orderedIds = useMemo(() => threads.map((thread) => thread.taskId), [threads]); const { boardRef, registerColumn, preloadTaskIds, detailTaskIds, mobileTaskId } = - useThreadColumnActivation(orderedIds, focusedTaskId); + useThreadColumnActivation(orderedIds, activationTaskId); const rememberThread = useThreadSelectionRecovery(orderedIds, boardRef, isMobile); function taskColumn(taskId: string | null) { diff --git a/apps/web/components/threads/use-thread-column-activation.ts b/apps/web/components/threads/use-thread-column-activation.ts index 535ebf81e7..b2ae5418d4 100644 --- a/apps/web/components/threads/use-thread-column-activation.ts +++ b/apps/web/components/threads/use-thread-column-activation.ts @@ -92,6 +92,7 @@ export function useThreadColumnActivation( const [visibleIds, setVisibleIds] = useState>(() => new Set()); const [observerReady, setObserverReady] = useState(false); const idsKey = orderedIds.join("\u0000"); + const hasColumns = orderedIds.length > 0; const updateVisibleIds = useCallback(() => { const next = new Set(); @@ -119,13 +120,6 @@ export function useThreadColumnActivation( useEffect(() => { const board = boardRef.current; - const orderedSet = new Set(orderedIds); - for (const id of elementsRef.current.keys()) { - if (!orderedSet.has(id)) { - elementsRef.current.delete(id); - visibilityRef.current.delete(id); - } - } observerRef.current?.disconnect(); observerRef.current = null; visibilityRef.current.clear(); @@ -157,10 +151,9 @@ export function useThreadColumnActivation( observer.disconnect(); if (observerRef.current === observer) observerRef.current = null; }; - // The keyed dependency keeps ref registration stable while still - // rebuilding observation when the shell order or membership changes. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [idsKey, updateVisibleIds]); + // Callback refs reconcile membership without dropping surviving visibility. + // Only an empty/nonempty transition replaces the board's DOM element. + }, [hasColumns, updateVisibleIds]); const fallbackTaskId = useMemo(() => { if (focusedTaskId && orderedIds.includes(focusedTaskId)) return focusedTaskId; diff --git a/apps/web/components/threads/use-thread-focus-request.test.ts b/apps/web/components/threads/use-thread-focus-request.test.ts new file mode 100644 index 0000000000..e1d246c837 --- /dev/null +++ b/apps/web/components/threads/use-thread-focus-request.test.ts @@ -0,0 +1,46 @@ +import { act, renderHook } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { useThreadFocusRequest } from "./use-thread-focus-request"; + +// @covers AC-TASKS-THREADS-ACTIONS-003.4 +describe("thread focus request lifetime", () => { + it("activates a requested target that resolves after hydration", () => { + const { result, rerender } = renderHook( + ({ target }: { target: string | null }) => useThreadFocusRequest(target, "request-b"), + { initialProps: { target: null as string | null } }, + ); + + rerender({ target: "b" }); + + expect(result.current).toMatchObject({ markedTaskId: "b", activationTaskId: "b" }); + }); + + it("does not activate a late target after the reader has consumed its request", () => { + const { result, rerender } = renderHook( + ({ target }: { target: string | null }) => useThreadFocusRequest(target, "request-b"), + { initialProps: { target: null as string | null } }, + ); + act(() => result.current.retire()); + + rerender({ target: "b" }); + + expect(result.current).toMatchObject({ markedTaskId: null, activationTaskId: null }); + }); + + it("rearms only a new request after its consumed target departs and returns", () => { + const { result, rerender } = renderHook( + ({ target, request }: { target: string | null; request: string }) => + useThreadFocusRequest(target, request), + { initialProps: { target: "b" as string | null, request: "request-b" } }, + ); + act(() => result.current.retire()); + expect(result.current).toMatchObject({ markedTaskId: null, activationTaskId: "b" }); + rerender({ target: null, request: "request-b" }); + rerender({ target: "b", request: "request-b" }); + expect(result.current).toMatchObject({ markedTaskId: null, activationTaskId: null }); + + rerender({ target: "b", request: "new-request-b" }); + + expect(result.current).toMatchObject({ markedTaskId: "b", activationTaskId: "b" }); + }); +}); diff --git a/apps/web/components/threads/use-thread-focus-request.ts b/apps/web/components/threads/use-thread-focus-request.ts new file mode 100644 index 0000000000..933c200c32 --- /dev/null +++ b/apps/web/components/threads/use-thread-focus-request.ts @@ -0,0 +1,24 @@ +import { useState } from "react"; + +/** Keeps visual dismissal separate from the initial conversation's mount lifetime. */ +export function useThreadFocusRequest( + focusedTaskId: string | null, + focusRequestKey: string | null, +) { + const [retired, setRetired] = useState(false); + const [requested, setRequested] = useState(focusRequestKey); + const [activationTaskId, setActivationTaskId] = useState(focusedTaskId); + if (requested !== focusRequestKey) { + setRequested(focusRequestKey); + setRetired(false); + setActivationTaskId(focusedTaskId); + } else if ((!retired || !focusedTaskId) && activationTaskId !== focusedTaskId) { + // A consumed request loses its fallback only when its target leaves the deck. + setActivationTaskId(focusedTaskId); + } + return { + markedTaskId: retired ? null : focusedTaskId, + activationTaskId, + retire: () => setRetired(true), + }; +} diff --git a/apps/web/e2e/tests/task/mobile-threads-task-actions.spec.ts b/apps/web/e2e/tests/task/mobile-threads-task-actions.spec.ts index 6e884dcb2f..92a8436ed4 100644 --- a/apps/web/e2e/tests/task/mobile-threads-task-actions.spec.ts +++ b/apps/web/e2e/tests/task/mobile-threads-task-actions.spec.ts @@ -12,6 +12,20 @@ import { filteredArchiveOutcome, lateArchiveOutcome, } from "./threads-task-actions-edge-helpers"; +import { pendingArchiveRecovery, pendingLastArchive } from "./threads-pending-archive-helpers"; + +for (const [name, outcome] of [ + ["pending archive recovers without taking focus", pendingArchiveRecovery], + ["pending last archive stays empty through settlement", pendingLastArchive], +] as const) { + test(name, async ({ testPage, apiClient, seedData }, testInfo) => { + test.setTimeout(120_000); + await testPage.setViewportSize({ width: 360, height: 780 }); + await withTaskActionSettings(apiClient, () => + outcome(testPage, apiClient, seedData, true, testInfo), + ); + }); +} for (const [name, outcome] of [ ["failures and retry", failedActionOutcomes], diff --git a/apps/web/e2e/tests/task/threads-pending-archive-helpers.ts b/apps/web/e2e/tests/task/threads-pending-archive-helpers.ts new file mode 100644 index 0000000000..f433ea4495 --- /dev/null +++ b/apps/web/e2e/tests/task/threads-pending-archive-helpers.ts @@ -0,0 +1,145 @@ +import { expect, type Page, type Route, type TestInfo } from "@playwright/test"; +import type { ApiClient } from "../../helpers/api-client"; +import type { SeedData } from "../../fixtures/test-base"; +import { seedActionThreads, ThreadActionsPage } from "./threads-task-actions-helpers"; +import { holdMutationResponse } from "./removal-transition-helpers"; +import { swipeDeckLeft } from "./mobile-threads-swipe-helpers"; + +function watchColumnDeparture(page: Page, taskId: string) { + return page.evaluateHandle((id) => { + const selector = `[data-thread-column-id="${id}"]`; + const containsColumn = (node: Node) => + node instanceof Element && (node.matches(selector) || node.querySelector(selector)); + const state = { departed: false, reappeared: false, disconnect: () => observer.disconnect() }; + const observer = new MutationObserver((records) => { + for (const record of records) { + if ([...record.removedNodes].some(containsColumn)) state.departed = true; + if (state.departed && [...record.addedNodes].some(containsColumn)) state.reappeared = true; + } + }); + observer.observe(document.body, { subtree: true, childList: true }); + return state; + }, taskId); +} + +// @covers AC-TASKS-THREADS-ACTIONS-003.4, AC-TASKS-THREADS-ACTIONS-003.7, AC-TASKS-THREADS-ACTIONS-003.8, AC-TASKS-THREADS-ACTIONS-004.6 +export async function pendingArchiveRecovery( + page: Page, + api: ApiClient, + seed: SeedData, + mobile: boolean, + testInfo: TestInfo, +) { + const { a, b } = await seedActionThreads(api, seed); + const ui = new ThreadActionsPage(page, mobile); + await api.saveUserSettings({ confirm_task_archive: true }); + const path = `**/api/v1/tasks/${a.id}/archive`; + let pending: Route | null = null; + const handler = (route: Route) => { + pending = route; + }; + await page.route(path, handler); + try { + await page.goto(`/threads?workspace=${seed.workspaceId}&taskId=${a.id}`); + await ui.open(a.id); + await ui.pick("Archive"); + await expect(page.getByTestId("archive-task-confirm")).toBeVisible(); + await expect(ui.column(a.id)).toHaveCount(1); + await ui.press(page.getByRole("button", { name: "Cancel", exact: true })); + await expect(ui.column(a.id)).toHaveCount(1); + expect(pending).toBeNull(); + + await ui.open(a.id); + await ui.pick("Archive"); + await ui.press(page.getByTestId("archive-task-confirm")); + await expect.poll(() => pending !== null).toBe(true); + await expect(ui.column(a.id)).toHaveCount(0); + await expect(ui.trigger(b.id)).toBeInViewport(); + if (mobile) { + await expect(page.getByTestId("thread-swipe-cue")).toHaveCount(0); + await ui.press(ui.column(b.id).getByTestId("thread-picker-trigger")); + await expect(page.getByTestId(`thread-picker-row-${a.id}`)).toHaveCount(0); + await expect(page.getByTestId(`thread-picker-row-${b.id}`)).toHaveCount(1); + await ui.press(page.getByTestId(`thread-picker-row-${b.id}`)); + } + const editor = ui.column(b.id).locator(".tiptap.ProseMirror"); + await ui.press(editor); + await editor.fill("Keep reading and drafting while archive finishes"); + await expect(editor).toBeFocused(); + await page.screenshot({ path: testInfo.outputPath("archive-pending.png") }); + + await pending!.fulfill({ + status: 503, + contentType: "application/json", + body: JSON.stringify({ error: "Archive temporarily unavailable" }), + }); + pending = null; + await expect(page.getByText("Failed to archive task", { exact: true })).toBeVisible(); + await expect(ui.column(a.id)).toHaveCount(1); + await expect(ui.column(a.id)).not.toHaveAttribute("data-focused", "true"); + await expect(editor).toHaveText("Keep reading and drafting while archive finishes"); + await expect(editor).toBeFocused(); + await expect(page).toHaveURL( + new RegExp(`/threads\\?workspace=${seed.workspaceId}&taskId=${a.id}$`), + ); + if (mobile) { + await expect(page.getByTestId("thread-swipe-cue")).toHaveText("1/2"); + await swipeDeckLeft(page, async () => { + await expect(page.getByTestId("thread-swipe-cue")).toHaveText("2/2"); + }); + await expect(ui.trigger(a.id)).toBeInViewport(); + } + await page.screenshot({ path: testInfo.outputPath("archive-rejected.png") }); + } finally { + if (pending) await (pending as Route).abort(); + await page.unroute(path, handler); + } +} + +// @covers AC-TASKS-THREADS-ACTIONS-003.3, AC-TASKS-THREADS-ACTIONS-003.7 +export async function pendingLastArchive( + page: Page, + api: ApiClient, + seed: SeedData, + mobile: boolean, + testInfo: TestInfo, +) { + const { a, b } = await seedActionThreads(api, seed); + expect((await api.rawRequest("POST", `/api/v1/tasks/${b.id}/archive`, {})).ok).toBe(true); + await api.saveUserSettings({ confirm_task_archive: false }); + const ui = new ThreadActionsPage(page, mobile); + await page.goto(`/threads?workspace=${seed.workspaceId}&taskId=${a.id}`); + await expect(ui.column(a.id).locator(".tiptap.ProseMirror")).toBeVisible(); + if (mobile) await expect(page.getByTestId("threads-mobile-topbar")).toBeInViewport(); + const held = await holdMutationResponse(page, `**/api/v1/tasks/${a.id}/archive`, "POST"); + const departure = await watchColumnDeparture(page, a.id); + try { + await ui.open(a.id); + await ui.pick("Archive"); + const empty = page.getByTestId("threads-empty-state"); + await expect(empty).toBeVisible(); + await held.backendResponseReady; + expect(held.requestCount()).toBe(1); + await expect(ui.column(a.id)).toHaveCount(0); + await expect(page.getByTestId("thread-swipe-cue")).toHaveCount(0); + await expect(page.getByText("Archived 1 task.", { exact: true })).toHaveCount(0); + if (mobile) await expect(page.getByTestId("threads-mobile-topbar")).toBeInViewport(); + await page.screenshot({ path: testInfo.outputPath("last-archive-pending.png") }); + held.release(); + await expect(page.getByText("Archived 1 task.", { exact: true })).toBeVisible(); + await expect(empty).toBeVisible(); + await expect(empty).toBeFocused(); + await expect(ui.column(a.id)).toHaveCount(0); + expect( + await departure.evaluate(({ departed, reappeared }) => ({ departed, reappeared })), + ).toEqual({ departed: true, reappeared: false }); + await departure.evaluate((state) => state.disconnect()); + await page.reload(); + await expect(empty).toBeVisible(); + } finally { + held.release(); + await held.dispose(); + await departure.evaluate((state) => state.disconnect()).catch(() => undefined); + await departure.dispose(); + } +} diff --git a/apps/web/e2e/tests/task/threads-task-actions-edge-helpers.ts b/apps/web/e2e/tests/task/threads-task-actions-edge-helpers.ts index ec17f3754a..a57d4e7b55 100644 --- a/apps/web/e2e/tests/task/threads-task-actions-edge-helpers.ts +++ b/apps/web/e2e/tests/task/threads-task-actions-edge-helpers.ts @@ -134,9 +134,11 @@ export async function lateArchiveOutcome( await ui.pick("Archive"); await expect.poll(() => pending !== null).toBe(true); await expect(page.getByTestId("archive-task-confirm")).toHaveCount(0); + await expect(ui.column(a.id)).toHaveCount(0); await ui.open(b.id); await expect(ui.choice("Delete")).toBeDisabled(); await pending!.continue(); + pending = null; await expect(ui.column(a.id)).toHaveCount(0); await expect(ui.choice("Delete")).toBeEnabled(); await ui.nested("Priority"); @@ -145,6 +147,7 @@ export async function lateArchiveOutcome( expect((await api.getTask(a.id)).priority).not.toBe("critical"); await expect(ui.trigger(b.id)).toBeFocused(); } finally { + if (pending) await (pending as Route).abort(); await page.unroute(`**/api/v1/tasks/${a.id}/archive`); await api.saveUserSettings({ confirm_task_archive: true }); } diff --git a/apps/web/e2e/tests/task/threads-task-actions-helpers.ts b/apps/web/e2e/tests/task/threads-task-actions-helpers.ts index 508458437b..e9bc48c4df 100644 --- a/apps/web/e2e/tests/task/threads-task-actions-helpers.ts +++ b/apps/web/e2e/tests/task/threads-task-actions-helpers.ts @@ -272,8 +272,7 @@ export async function allTaskActionOutcomes( await ui.press(page.getByTestId("archive-task-confirm")); await expect(ui.column(a.id)).toHaveCount(0); await expect(ui.trigger(b.id)).toBeFocused(); - const archived = await api.rawRequest("GET", `/api/v1/tasks/${a.id}`); - expect((await archived.json()).archived_at).toBeTruthy(); + await expect.poll(async () => (await api.getTask(a.id)).archived_at).toBeTruthy(); await ui.delete(b.id); await expect(page.getByTestId("threads-empty-state")).toBeVisible(); await expect(page.getByTestId("threads-empty-state")).toBeFocused(); diff --git a/apps/web/e2e/tests/task/threads-task-actions.spec.ts b/apps/web/e2e/tests/task/threads-task-actions.spec.ts index 5999cedcef..ff63b9decc 100644 --- a/apps/web/e2e/tests/task/threads-task-actions.spec.ts +++ b/apps/web/e2e/tests/task/threads-task-actions.spec.ts @@ -11,6 +11,19 @@ import { filteredArchiveOutcome, lateArchiveOutcome, } from "./threads-task-actions-edge-helpers"; +import { pendingArchiveRecovery, pendingLastArchive } from "./threads-pending-archive-helpers"; + +for (const [name, outcome] of [ + ["pending archive recovers without taking focus", pendingArchiveRecovery], + ["pending last archive stays empty through settlement", pendingLastArchive], +] as const) { + test(name, async ({ testPage, apiClient, seedData }, testInfo) => { + test.setTimeout(120_000); + await withTaskActionSettings(apiClient, () => + outcome(testPage, apiClient, seedData, false, testInfo), + ); + }); +} for (const [name, outcome] of [ ["failures and retry", failedActionOutcomes], diff --git a/apps/web/lib/threads/thread-view-query.test.ts b/apps/web/lib/threads/thread-view-query.test.ts index dbd587bf8b..6c60548af9 100644 --- a/apps/web/lib/threads/thread-view-query.test.ts +++ b/apps/web/lib/threads/thread-view-query.test.ts @@ -443,3 +443,63 @@ describe("thread view fingerprint", () => { expect(first.fingerprint).not.toBe(changed.fingerprint); }); }); + +// @covers AC-TASKS-THREADS-ACTIONS-003.7, AC-TASKS-THREADS-ACTIONS-003.8 +describe("pending archive exclusion", () => { + it("excludes pending tasks before scopes, counts and column limits", () => { + const snapshots = { + "workflow-1": snapshot([task({ id: "a" }), task({ id: "b" }), task({ id: "c" })]), + }; + const activeView = view({ + taskScope: { mode: "selected", taskIds: ["a", "b", "c"] }, + maxColumns: 1, + }); + const baseline = queryThreadView(snapshots, activeView); + const result = queryThreadView(snapshots, activeView, { + requestedTaskId: "a", + excludedTaskIds: new Set(["a"]), + }); + + for (const candidates of [ + result.candidates, + result.matchingCandidates, + result.stableCandidates, + ]) { + expect(candidates.map(({ taskId }) => taskId)).toEqual(["b", "c"]); + } + expect(result.admittedCandidates.map(({ taskId }) => taskId)).toEqual(["b"]); + expect(result).toMatchObject({ matchingCount: 2, temporaryAdmissionCount: 0, hiddenCount: 1 }); + expect(result.fingerprint).toBe(baseline.fingerprint); + expect(result.effectiveView).toEqual(activeView); + expect(snapshots["workflow-1"].tasks.map(({ id }) => id)).toEqual(["a", "b", "c"]); + }); + + it("does not temporarily admit an excluded deep link outside the saved filter", () => { + const result = queryThreadView( + { "workflow-1": snapshot([task({ id: "a" }), task({ id: "b" })]) }, + view({ filters: [clause("titleMatch", "Task b", "matches")], maxColumns: 1 }), + { requestedTaskId: "a", excludedTaskIds: new Set(["a"]) }, + ); + + expect(result.admittedCandidates.map(({ taskId }) => taskId)).toEqual(["b"]); + expect(result).toMatchObject({ matchingCount: 1, temporaryAdmissionCount: 0, hiddenCount: 0 }); + }); + + it("excludes every covered descendant and returns an empty query when none survive", () => { + const result = queryThreadView( + { + "workflow-1": snapshot([ + task({ id: "parent" }), + task({ id: "child", parentTaskId: "parent" }), + ]), + }, + view(), + { excludedTaskIds: new Set(["parent", "child"]) }, + ); + + expect(result.candidates).toEqual([]); + expect(result.stableCandidates).toEqual([]); + expect(result.admittedCandidates).toEqual([]); + expect(result).toMatchObject({ matchingCount: 0, temporaryAdmissionCount: 0, hiddenCount: 0 }); + }); +}); diff --git a/apps/web/lib/threads/thread-view-query.ts b/apps/web/lib/threads/thread-view-query.ts index 952500528d..cf730ce57e 100644 --- a/apps/web/lib/threads/thread-view-query.ts +++ b/apps/web/lib/threads/thread-view-query.ts @@ -47,6 +47,8 @@ export type ThreadViewQueryOptions = { workspaceId?: string | null; requestedTaskId?: string | null; draft?: ThreadViewDraft | null; + /** Transient removal intent applies before every admission path, including deep links. */ + excludedTaskIds?: ReadonlySet; }; export type ThreadViewQueryResult = { @@ -460,7 +462,9 @@ export function queryThreadView( options: ThreadViewQueryOptions = {}, ): ThreadViewQueryResult { const effectiveView = cloneViewWithDraft(view, options.draft); - const candidates = selectThreadCandidates(snapshots, options); + const candidates = selectThreadCandidates(snapshots, options).filter( + (candidate) => !options.excludedTaskIds?.has(candidate.taskId), + ); const scoped = applyTaskScope(candidates, effectiveView.taskScope); const matchingCandidates = scoped .filter((candidate) => candidateMatches(candidate, effectiveView.filters)) diff --git a/docs/plans/threads-immediate-archive/plan.md b/docs/plans/threads-immediate-archive/plan.md new file mode 100644 index 0000000000..cbf15b10f9 --- /dev/null +++ b/docs/plans/threads-immediate-archive/plan.md @@ -0,0 +1,232 @@ +--- +created: 2026-09-11 +status: done +requirements: + - REQ-TASKS-THREADS-ACTIONS-001 + - REQ-TASKS-THREADS-ACTIONS-002 + - REQ-TASKS-THREADS-ACTIONS-003 + - REQ-TASKS-THREADS-ACTIONS-004 +system_design: + - ../../specs/tasks/system-design/threads-task-actions.md +legacy_specs: [] +--- + +# Implementation Plan: Immediate Threads Archive Removal + +## Overview + +Remove an archived task's conversation as soon as the user accepts Archive. +Use the existing task-removal intent to exclude its column while cleanup runs, +then let the current deck recovery select a survivor or show the empty state. +One work order owns the correction, targeted tests, and matching usage docs. + +The task system owns this outcome because archive intent, mutation settlement +and visible recovery belong to the existing +[Threads task actions requirement](../../specs/tasks/requirements/threads-task-actions.md). +The [paired design](../../specs/tasks/system-design/threads-task-actions.md) +reuses the deck's ordering and viewport contracts. No new ADR is needed for +this use of the existing removal and presentation boundaries. + +## Evidence and requirement conformance + +Read-only trace at `20efe4855`: + +1. `useTaskManagementFlow` and `useTaskMenuActions({ stayOnListing: true })` + initiate the existing shared archive operation. +2. `hooks/task-removal-coordinator.ts:coordinateTaskRemovalBatch` calls + `beginOperation` before invoking the mutation. The operation records the + task IDs, including captured cascade descendants, in `taskRemoval`. +3. `app/threads/threads-page-client.tsx:ThreadsPageClient` subscribes to + workflow snapshots and saved views, but not task-removal intent. + `queryThreadView` has no pending exclusion input. Therefore an unchanged + snapshot still yields the same column while archive is pending. +4. `ThreadColumn` continues to mount `ThreadConversation` and `TaskChatPanel`. + Successful snapshot pruning or an authoritative archive event eventually + removes it. Until then, chat can expose intermediate cleanup state. +5. `lateArchiveOutcome` already holds an archive request before forwarding it, + but asserts column absence only after `pending.continue()`. Moving the + absence assertion before that release is the smallest browser regression. + +This confirms the missing presentation boundary through source, not a captured +browser replay. The user-reported blocked composer and warning are the symptom; +their precise runtime event sequence was not recorded in this design turn. + +The previous contract specified eventual recovery but omitted immediate archive +departure. `AC-TASKS-THREADS-ACTIONS-003.7` and `.8` define that timing and +membership. `AC-TASKS-THREADS-ACTIONS-003.4` now permits provisional departure and requires +failed archives to restore eligible rows without claiming success or stealing +newer selection. The prior prohibition on treating failure as success remains. +This changes visible timing, not archive persistence or cleanup semantics. + +The original [Threads task actions package](../threads-task-actions/plan.md) +remains a completed historical delivery. Its results are not this repair's +verification. The existing [task-removal coordinator design](../../specs/tasks/system-design/removal-navigation.md) +is reused; none of its other entry points is being redesigned. + +## Scope + +In scope: immediate pending archive exclusion; known consented cascade targets; +query counts and limit refill; consumed deep-link focus; surviving/empty +recovery; failed-request readmission; desktop/mobile proof and usage text. + +Out of scope: delete timing, new archive APIs, cleanup scheduling, worktree +recovery, new persisted state, saved-view semantics, task-detail/Office/preview +navigation, new confirmation rules, UI geometry changes, and broad refactors. + +## Technical approach + +- In `ThreadsPageClient`, derive pending archive task IDs from the existing + `taskRemoval.operationsByToken` subscription. Use operation lifetime through + reconciliation; a successful response alone is not a reason to reopen chat. +- Add optional `excludedTaskIds` to `ThreadViewQueryOptions`; exclude before + all query admission paths, including explicit scopes, column limits and + temporary deep links. Keep exclusion out of `queryFingerprint` and the + stable-order reset key. Other query callers retain their defaults. +- Keep the board's action provider mounted and feed one consistent candidate + projection to columns, header counts, phone picker and pagination. Existing + `useThreadSelectionRecovery` owns successor/predecessor/empty behavior. +- Make `useThreadFocusRequest` distinguish a URL request's identity from its + resolved visible target. A temporarily excluded task returning after failure + must not reactivate an already consumed deep link. Add a narrow optional + board prop for the request identity if needed; preserve current callers. +- Reuse coordinator success/failure cleanup and localized toasts. Do not + optimistically mutate shared task snapshots or introduce API calls in Threads. +- Update the pending-exclusion note in `components/threads/AGENTS.md` and the + Threads paragraph in `docs/public/sessions-and-review.md` with implementation. + The required public documentation update is complete. + +Source paths above are relative to `apps/web/` unless prefixed with `docs/`. + +## ASCII UI preview + +UI-01: Accepted archive of B. Opening/cancelling confirmation keeps the before +state. The current chat persistence during cleanup is established by source. + +```text +Desktop before / pending today Desktop immediately after acceptance ++-------+-------+-------+ +-----------+-----------+ +| A | B ... | C | | A | C ... | +| chat | chat | chat | ---> | chat | chat | +| input | input | input | | input | input | ++-------+-------+-------+ +-----------+-----------+ + Archive B B column is absent + +Phone before Phone immediately after acceptance ++----------------------+ +----------------------+ +| Threads / view 2/3 | | Threads / view 2/2 | +| B Open ... | | C Open ... | +| Conversation B | ---> | Conversation C | +| Composer B | | Composer C | ++----------------------+ +----------------------+ + +Last column archived: [Threads / view] + [existing empty state] +Archive rejected: [eligible task returns] + [existing error toast] +``` + +Structural requirements: remove the column/chat subtree, retain survivor order, +keep one phone conversation and the existing header, and show no outgoing +composer or archive warning during pending cleanup. Widths/copy in the sketch +are illustrative. Returning failed tasks use normal arrival order and do not +take focus from the current surviving reader. + +The phone entry is the existing visible overflow and inset task-actions drawer; +`mobile-thread-column-header.tsx` and `task-management-drawer.tsx` are the +closest shipped exemplars. The confirmation closes into the snapped deck, +which is the primary content surface. Existing horizontal deck scrolling, +internal transcript scrolling, dynamic viewport sizing, safe areas and touch +targets remain in place. No new scroll container or action is introduced. + +## Tests + +All numeric AC suffixes below expand to `AC-TASKS-THREADS-ACTIONS-*`. + +| Evidence | Coverage | +| --- | --- | +| `app/threads/threads-page-archive.test.tsx`: `removes an archived column before the request settles` with the real store/deferred archive; archive/delete distinction; response/event ordering cannot remount success | `001.5`, `002.5`, `003.7` | +| `lib/threads/thread-view-query.test.ts`: pending target excluded from every candidate list/count, explicit scope/deep link, capped refill, captured descendants | `003.2`, `003.3`, `003.8` | +| Page/board tests: independent A/B operations, rejection readmission, foreign workspace/view changes, already archived/deleted rejection, consumed deep link plus newer menu/selection | `002.5`, `003.1`, `003.4`, `003.5`, `003.8`, `004.6` | +| Existing `stable-order`, `thread-selection-fallback`, board and column-activation tests: survivor position, selected sibling/draft preservation, empty focus and one phone detail | `003.1`-`003.6`, `004.6` | +| Existing `hooks/use-task-menu-actions.test.ts`: pending intent, listing route, duplicate guards and request settlement continue working | `002.5`, `003.4`, `003.7` | + +Use a narrowly mocked heavy chat boundary to observe mount/unmount in a rendered +page test, while retaining the real store, query and archive coordinator. Do +not use only final-state assertions or mock the whole query being tested. + +## E2E tests + +Extend the existing `threads-task-actions.spec.ts` (`chromium`) and +`mobile-threads-task-actions.spec.ts` (`mobile-chrome`). Share stimulus helpers +in `threads-task-actions-edge-helpers.ts` and `threads-pending-archive-helpers.ts`; +reuse `ThreadActionsPage`, settings cleanup and the production mock-agent seed path. + +Both projects cover: request held before backend dispatch; real archive +lifecycle events with HTTP response held; last-column empty state; rejection +and retry; deep-linked target; independent survivor/newer menu; confirmation +cancellation and confirmation-disabled acceptance. Unit tests cover the full +cascade/limit/workspace matrix. Assert pending absence before releasing each +barrier. Arm a scoped DOM observer before acceptance to detect outgoing chat +reappearance, and release all held routes in `finally`. + +Reuse `holdMutationResponse` for the post-backend barrier. Assert the task is +archived after success and remains absent after reload. On phone, use actual +touch entry, check pagination and picker membership, and swipe the surviving +deck. Inspect desktop/phone pending screenshots during the focused run. + +## Work orders + +- [x] [Task 01: Remove pending archives from Threads](task-01-remove-pending-archives.md) + +Sequential, one work order; no delegation or additional worktree prerequisite. +Its verification block contains the exact runnable commands. + +## Verification results + +Design checkpoint (2026-09-11): + +- `python3 scripts/lint-spec-files.test.py`: 36 tests passed. +- `python3 scripts/lint-spec-files.py --all`: all specification files passed. +- `git diff --check`: passed for tracked changes; `git diff --no-index --check` + against `/dev/null` passed for both new plan files. +- `git status --short -- docs/specs docs/plans/threads-immediate-archive`: + exactly two modified specifications and the new plan directory. +- Read-only package validation: four documents, 22 relative links, all 15 + referenced requirement/acceptance IDs, and explicit source/test paths resolve. + +At the design checkpoint, production and permanent tests were unchanged, +dependencies were not installed, and product/browser tests were not run. +Implementation was subsequently authorized by "go for it" on 2026-09-12. + +Initial implementation completed on 2026-09-12 in the primary session, without +delegation or additional platform tasks/sessions. The +[work order](task-01-remove-pending-archives.md#results) records behavioral +RED/GREEN evidence, corrected test assumptions, exact environment overrides, +and inspected screenshots. + +- Focused frontend suite: 117 tests passed across eight files. +- Fresh managed browser runs: 8 Chromium tests and 10 mobile-chrome tests + passed with one worker and retries disabled, including native swiping. +- Typecheck, changed-file ESLint with zero warnings, Prettier and the i18n + new-code ratchet passed. +- Public-doc validator test passed; all 46 published pages validated. + Specification linter tests passed (36), all specs validated, and whitespace + checks passed. +- Public how-to documentation now describes immediate departure and failed + readmission. Scoped Threads guidance records query exclusion, focus request + identity and retention of surviving viewport observations. + +PR review remediation is recorded in the +[work order](task-01-remove-pending-archives.md#pr-review-remediation): early +pointer/keyboard interaction now retires the mark without unmounting the +requested chat before visibility is ready. Local verification passed 122 tests +across nine files, the explicit Prettier command, typecheck, lint, i18n and docs +checks, and all eight desktop/ten phone browser regressions with fresh captures. +The legacy focus-key contract is documented, the stale public-doc deferral is +removed, and CI/review completion remains an external post-commit gate. + +## Risks + +- Filtering after column admission leaves incorrect counts or prevents refill. +- Including pending state in the reset key reranks surviving columns. +- Restoring a task can re-trigger a consumed deep link and steal focus. +- Releasing exclusion before snapshot cleanup can flash the archived chat. +- A test that checks only after releasing HTTP can pass with the defect intact. diff --git a/docs/plans/threads-immediate-archive/task-01-remove-pending-archives.md b/docs/plans/threads-immediate-archive/task-01-remove-pending-archives.md new file mode 100644 index 0000000000..5bc9138135 --- /dev/null +++ b/docs/plans/threads-immediate-archive/task-01-remove-pending-archives.md @@ -0,0 +1,277 @@ +--- +id: "01-remove-pending-archives" +title: "Remove pending archives from Threads" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-THREADS-ACTIONS-001 + - REQ-TASKS-THREADS-ACTIONS-002 + - REQ-TASKS-THREADS-ACTIONS-003 + - REQ-TASKS-THREADS-ACTIONS-004 +acceptance_criteria: + - AC-TASKS-THREADS-ACTIONS-001.5 + - AC-TASKS-THREADS-ACTIONS-002.5 + - AC-TASKS-THREADS-ACTIONS-003.1 + - AC-TASKS-THREADS-ACTIONS-003.2 + - AC-TASKS-THREADS-ACTIONS-003.3 + - AC-TASKS-THREADS-ACTIONS-003.4 + - AC-TASKS-THREADS-ACTIONS-003.5 + - AC-TASKS-THREADS-ACTIONS-003.6 + - AC-TASKS-THREADS-ACTIONS-003.7 + - AC-TASKS-THREADS-ACTIONS-003.8 + - AC-TASKS-THREADS-ACTIONS-004.6 +system_design: + - ../../specs/tasks/system-design/threads-task-actions.md +--- + +# Task 01: Remove Pending Archives from Threads + +## Summary + +Connect existing pending archive intent to Threads query admission, so the +outgoing column unmounts at acceptance. Preserve existing recovery and handle +failed readmission without reactivating a consumed deep link. + +## In scope + +- Render-time archive exclusion and consistent query counts/limit refill. +- Real-store regression tests for pending, failure, cascade, deep links, + independent operations, selection and event/response ordering. +- Desktop and phone E2E coverage, rendered inspection, scoped engineering + guidance and the existing public Threads usage paragraph. + +## Out of scope + +Backend behavior, deletion timing, shared removal coordinator redesign, new +state persistence, session management, saved-view preference changes, new copy +or geometry, and unrelated task surfaces. + +## Acceptance + +1. An accepted archive removes every covered admitted column before HTTP or + cleanup completes. Confirm-cancel and pending-delete controls retain current + behavior. Query scope, deep links, counts and caps honor the exclusion. +2. Success stays removed through either event/response order; rejection restores + only currently eligible tasks. Surviving columns, newer menus/selections, + consumed focus requests, workspace and saved view remain coherent. +3. Targeted unit and desktop/mobile browser checks pass, with real pending + interval assertions, phone picker/pagination/swipe evidence, and inspected + desktop/phone renders. Usage docs describe the implemented timing and failure. + +## ASCII UI preview + +UI-01, excerpt from the [combined preview](plan.md#ascii-ui-preview). +Coverage: `003.1`-`003.8` and `004.6` under the AC prefix in frontmatter. + +```text +Accept Archive B +Desktop: [A chat] [B chat] [C chat] -> [A chat] [C chat] +Phone: [B chat, 2/3] -> [C chat, 2/2] +Last: [B chat, no other tasks] -> [existing empty state] +Failure: [surviving deck] -> [eligible B returns + error] +``` + +The outgoing subtree must be absent. The phone keeps one snapped conversation, +the existing fixed header, transcript scroll owner, safe-area behavior and +touch controls. Failed readmission uses normal arrival ordering and does not +steal focus. Sketch spacing and labels are illustrative. + +## Verification + +Run from repository root. Install once before the first frontend command in +this fresh worktree. Use `/tdd`; first reproduce with an unchanged production +query and a deferred archive request. Existing `lateArchiveOutcome` provides +the smallest browser stimulus: assert absence before `pending.continue()`. + +```bash +(cd apps && pnpm install --frozen-lockfile) +(cd apps/web && pnpm exec vitest run app/threads/threads-page-client.test.tsx app/threads/threads-page-archive.test.tsx lib/threads/thread-view-query.test.ts lib/threads/stable-order.test.ts lib/threads/thread-selection-fallback.test.ts components/threads/threads-board.test.tsx components/threads/thread-column-activation.test.tsx components/threads/use-thread-focus-request.test.ts hooks/use-task-menu-actions.test.ts) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm exec eslint app/threads/threads-page-client.tsx app/threads/threads-page-client.test.tsx app/threads/threads-page-archive.test.tsx lib/threads/thread-view-query.ts lib/threads/thread-view-query.test.ts components/threads/threads-board.tsx components/threads/threads-board.test.tsx components/threads/use-thread-column-activation.ts components/threads/thread-column-activation.test.tsx components/threads/use-thread-focus-request.ts components/threads/use-thread-focus-request.test.ts e2e/tests/task/threads-task-actions.spec.ts e2e/tests/task/mobile-threads-task-actions.spec.ts e2e/tests/task/threads-task-actions-helpers.ts e2e/tests/task/threads-task-actions-edge-helpers.ts e2e/tests/task/threads-pending-archive-helpers.ts --max-warnings 0) +(cd apps/web && pnpm exec prettier --check app/threads/threads-page-client.tsx app/threads/threads-page-client.test.tsx app/threads/threads-page-archive.test.tsx lib/threads/thread-view-query.ts lib/threads/thread-view-query.test.ts components/threads/threads-board.tsx components/threads/threads-board.test.tsx components/threads/use-thread-column-activation.ts components/threads/thread-column-activation.test.tsx components/threads/use-thread-focus-request.ts components/threads/use-thread-focus-request.test.ts e2e/tests/task/threads-task-actions.spec.ts e2e/tests/task/mobile-threads-task-actions.spec.ts e2e/tests/task/threads-task-actions-helpers.ts e2e/tests/task/threads-task-actions-edge-helpers.ts e2e/tests/task/threads-pending-archive-helpers.ts) +(cd apps/web && pnpm run i18n:ratchet) +(cd apps/web && pnpm e2e:run --project chromium tests/task/threads-task-actions.spec.ts -- --retries=0) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-threads-task-actions.spec.ts tests/task/mobile-threads-swipe.spec.ts -- --retries=0) +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +python3 scripts/lint-spec-files.test.py +python3 scripts/lint-spec-files.py --all +git diff --check +``` + +Run browser commands sequentially; each managed invocation builds current +production assets. Use test-controlled request/response barriers and existing +causal waits, no arbitrary sleeps. Restore settings and release held routes on +failure. Record RED/GREEN separately, actual discovered/passed counts and +screenshots. If implementation adds another test or changes a helper's scope, +update the exact command list before marking the work complete. + +## Files likely touched + +Under `apps/web/`: + +- `app/threads/threads-page-client.tsx` and `.test.tsx`. +- `app/threads/threads-page-archive.test.tsx` (real-store/coordinator integration). +- `lib/threads/thread-view-query.ts` and `.test.ts`. +- `components/threads/threads-board.tsx` and `.test.tsx` (focus request lifetime). +- `components/threads/use-thread-focus-request.ts` and `.test.ts` (visual + dismissal versus initial activation lifetime and late hydration). +- `components/threads/use-thread-column-activation.ts` and `thread-column-activation.test.tsx` + (retain surviving visibility and editor mounts across removal/readmission). +- `e2e/tests/task/threads-task-actions-edge-helpers.ts`. +- `e2e/tests/task/threads-task-actions-helpers.ts` (wait for persisted archive + independently of immediate column removal). +- `e2e/tests/task/threads-pending-archive-helpers.ts` (desktop/phone pending scenarios). +- `e2e/tests/task/threads-task-actions.spec.ts` and `mobile-threads-task-actions.spec.ts`. +- `components/threads/AGENTS.md`. + +Documentation: `docs/public/sessions-and-review.md`, the paired design's status, +this work order and `plan.md` results. The owning index already links the +requirement/design pair; no duplicate capability documents are needed. + +## Dependencies + +None. Existing task-action and task-removal implementations are present at the +recorded source revision; preserve their behavior and historical plan results. + +## Risks + +Premature exclusion release, stale deep-link refocus, surviving-column rerank, +and tests that miss the interval before cleanup. See the plan's test matrix. + +## Parallelism + +`sequential` + +## Inputs + +- [Requirements](../../specs/tasks/requirements/threads-task-actions.md), + especially deterministic recovery and archive acceptance. +- [System design](../../specs/tasks/system-design/threads-task-actions.md), + especially pending archive exclusion and failure behavior. +- `apps/web/AGENTS.md`, `components/threads/AGENTS.md`, `/mobile-parity`, `/e2e`. +- Existing query/order tests, `use-task-menu-actions.test.ts`, `ThreadActionsPage`, + `lateArchiveOutcome`, `holdMutationResponse`, and mobile swipe helpers. + +## Results + +Implemented and verified on 2026-09-12 after the user's "go for it". + +### Implementation + +The page derives archive-only exclusions from the existing removal coordinator, +and the query applies them before every admission/count path. No optimistic +snapshot mutation, new store, API, copy, confirmation rule or delete timing +was added. Query derivation is a local hook to keep the page within the +function-length limit. + +The board retains consumed URL request identity across temporary absence and +does not use a retired request as its detail fallback. The viewport observer +now follows the board element's lifetime: existing callback refs reconcile +membership without clearing surviving visibility or remounting editors. + +### RED evidence + +- Before production changes, the focused query/board run failed four new + assertions (59 controls passed), and all four new real-store lifecycle cases + failed because outgoing columns stayed mounted. +- A stronger consumed-link assertion exposed the old detail fallback replacing + the surviving chat. Two additional observer tests reproduced replacement of + visible B with fallback A during removal and readmission. +- A fresh browser build with the archive exclusion disabled reproduced both + held-request regressions: expected zero outgoing columns, observed one. +- The first full desktop run passed six cases and exposed two outdated test + assumptions. The success label is `Archived 1 task.`, and immediate DOM + departure no longer proves persisted archival. The existing all-actions + helper now polls backend persistence independently of column absence. + +### Final checks + +The initial implementation checks passed (extended fixup results appear below): + +- Frozen-lockfile dependency installation succeeded. +- Vitest: 117 passed across eight files, including real-store/coordinator/WS + integration, query exclusions, focus recovery and activation budgets. +- Typecheck, changed-file ESLint (`--max-warnings 0`), Prettier and i18n ratchet: + passed. +- Chromium: 8 discovered and 8 passed, retries disabled (1.8 minutes). +- Mobile-chrome: 10 discovered and 10 passed across the task-action and swipe + specs, retries disabled (2.4 minutes). Coverage includes touch confirmation, + picker/pagination exclusion, rejection recovery, native swiping and the + existing 320/360/700/820-pixel geometry checks. +- The successful-archive browser probe observed departure without any DOM + reappearance through lifecycle events and HTTP settlement. Both projects + also verified the empty state after reload. +- Public-doc validator test passed and 46 published pages validated. + All 36 specification linter tests and the all-spec check passed. + Whitespace checks passed. + +The sandbox denied the default Go cache and local socket creation. Final E2E +runs used the approved isolated runner outside that sandbox, prefixed with +`KANDEV_SERVER_HOST=127.0.0.1 GOCACHE=/tmp/threads-archive-go-cache.pom5F8`. +Both invocations rebuilt production assets and ran sequentially with one +worker. The commands additionally used +`--output /tmp/threads-archive-evidence.4vbQSF/desktop` and +`--output /tmp/threads-archive-evidence.4vbQSF/mobile`, respectively. Fixtures +owned and cleaned their temporary backends, worktrees and data; no developer +instance was mutated. + +Inspected both projects' `archive-pending.png`, `archive-rejected.png` and +`last-archive-pending.png` beneath those output roots. Pending chat removal, +survivor drafts, restored-column ordering, phone pagination and empty-state +composition are correct. An additional phone header assertion passed before +and after the last archive, with its refreshed empty-state screenshot inspected +under `/tmp/threads-archive-evidence.4vbQSF/mobile-header`. That test-only check +reused the same fresh production build: + +```bash +KANDEV_SERVER_HOST=127.0.0.1 GOCACHE=/tmp/threads-archive-go-cache.pom5F8 pnpm e2e:run --no-build --project mobile-chrome tests/task/mobile-threads-task-actions.spec.ts -- --grep 'pending last archive' --retries=0 --output /tmp/threads-archive-evidence.4vbQSF/mobile-header +``` + +One test was discovered and passed (12 seconds); the modified helper's ESLint +and Prettier checks also passed. The build retained existing bundle-size/dynamic-import +warnings and optional macOS-sidecar signing warnings on Linux. + +Public docs updated: `docs/public/sessions-and-review.md` (how-to guide). +The owning design is current and the work package is complete. At the +implementation handoff, no commit, push or PR had been requested or performed. + +### PR review remediation + +Greptile identified a pre-observer activation race. Two board regressions +reproduced it: pointer and keyboard focus retired the visual mark and unmounted +the requested non-first conversation (28 existing cases passed). The extracted +`useThreadFocusRequest` now keeps that activation fallback until its consumed +target departs, without reviving it on failed-archive readmission. The regressions +assert the same conversation node stays mounted. Three additional hook controls +cover late hydration, interaction before hydration, and a genuinely new request. + +Claude's compatibility suggestion is addressed by documenting the explicit +URL-key requirement and legacy optional default. CodeRabbit's aggregate +suggestions are addressed by removing the stale public-doc deferral and adding +the exact Prettier command above. No further public-doc change is needed: +the existing how-to already promises uninterrupted conversation use and focus +preservation; this repair enforces that contract. + +Final local remediation verification on 2026-09-12: + +- The updated nine-file Vitest command passed all 122 tests. +- Typecheck, changed-source ESLint with zero warnings, the explicit 16-file + Prettier check, and i18n ratchet passed. +- Fresh managed Chromium run: all eight archive/action regressions plus one + disposable PR capture passed (nine tests, 1.8 minutes). +- Mobile-chrome reused that fresh build: all ten archive/action/swipe + regressions plus one disposable PR capture passed (11 tests, 2.3 minutes). + Both runs used one worker, retries disabled, `--host`, `CAPTURE_PR_ASSETS=1` + and the previously recorded loopback/cache overrides. Capture specs were + removed afterward; screenshot publication uses a separate orphan media ref, + never the feature branch. +- All 19 harness-validator tests, all 196 harness files, all 36 spec-validator + tests, the all-spec check, the public-doc validator test and all 46 public + pages passed. Scoped guidance remains within its line budget. + +Current-head CI, review-thread resolution, the minimum five-minute new-comment +watch, and current-base integration validation are tracked on the live PR/task +after the remediation commit. Local checks do not substitute for those gates. diff --git a/docs/public/sessions-and-review.md b/docs/public/sessions-and-review.md index fd1fe413d5..84c6978d66 100644 --- a/docs/public/sessions-and-review.md +++ b/docs/public/sessions-and-review.md @@ -113,6 +113,11 @@ changes. Canceling a confirmation leaves the task unchanged. See [archive and deletion behavior](tasks-and-workflows.md#archive-unarchive-and-delete) for confirmation preferences and cleanup consequences. +Once you confirm **Archive**, its conversation disappears immediately while +cleanup continues. With archive confirmation disabled, choosing **Archive** is +enough. If the request fails, the task returns when your current filters and +column limit allow, without taking focus from the thread you are using. + If an action or view filter removes your current thread, Threads selects the next remaining thread, otherwise the previous one. If neither survives from the previous view, it selects the first thread in the new view. The empty view diff --git a/docs/specs/tasks/requirements/threads-task-actions.md b/docs/specs/tasks/requirements/threads-task-actions.md index 7a4caab08c..2df59c0e87 100644 --- a/docs/specs/tasks/requirements/threads-task-actions.md +++ b/docs/specs/tasks/requirements/threads-task-actions.md @@ -2,6 +2,7 @@ status: active system: tasks created: 2026-09-10 +updated: 2026-09-11 owners: - kandev --- @@ -117,7 +118,10 @@ without losing the deck's position and session behavior. loading, it shall retain the existing loading state. Pagination, focus, and conversation activation shall not reference a removed column. - **AC-TASKS-THREADS-ACTIONS-003.4:** A failure shall not permanently remove a - task from the deck or advance selection as if the operation succeeded. + task from the deck or be reported as success. A task provisionally hidden for + an archive shall return when its current task state and view still admit it, + without resetting surviving column order or overriding a newer reader + selection. An authoritative removal shall take precedence over restoration. Server events arriving before or after the response shall produce the same final result without repeated jumps. - **AC-TASKS-THREADS-ACTIONS-003.5:** Recovery shall keep the Threads route, @@ -127,6 +131,19 @@ without losing the deck's position and session behavior. - **AC-TASKS-THREADS-ACTIONS-003.6:** Recovery and menu use shall preserve native horizontal swiping, stable task order, conversation drafts, session switching, and the existing viewport-based transcript and subscription budget. +- **AC-TASKS-THREADS-ACTIONS-003.7:** Once the user accepts an archive from + Threads, its task column and conversation shall disappear in the next + rendered state, before waiting for the archive response or cleanup events. + The outgoing chat shall not remain as a blocked composer or show a + workspace-archived warning during that pending operation. Merely opening or + cancelling confirmation shall not remove the column. With confirmation + disabled, choosing Archive shall be the acceptance point. +- **AC-TASKS-THREADS-ACTIONS-003.8:** Every admitted task included in an accepted + archive, including known descendants covered by cascade consent, shall be + excluded from the pending deck, thread picker, and pagination. A deep link + or saved explicit task scope shall not readmit it. Other eligible tasks + shall continue to fill available column slots under the existing limit and + recovery rules. Independent pending archives shall settle independently. ### REQ-TASKS-THREADS-ACTIONS-004: Responsive accessible interaction @@ -181,3 +198,4 @@ without losing the deck's position and session behavior. - [System design](../system-design/threads-task-actions.md) - [Implementation plan](../../../plans/threads-task-actions/plan.md) +- [Immediate archive removal follow-up](../../../plans/threads-immediate-archive/plan.md) diff --git a/docs/specs/tasks/system-design/threads-task-actions.md b/docs/specs/tasks/system-design/threads-task-actions.md index 4c60c8e639..bce3dd6cea 100644 --- a/docs/specs/tasks/system-design/threads-task-actions.md +++ b/docs/specs/tasks/system-design/threads-task-actions.md @@ -1,6 +1,7 @@ --- status: current system: tasks +updated: 2026-09-12 requirements: - REQ-TASKS-THREADS-ACTIONS-001 - REQ-TASKS-THREADS-ACTIONS-002 @@ -10,6 +11,11 @@ requirements: # Threads Task Actions System Design +The task-action surfaces and pending archive exclusion below are implemented. +The [immediate archive removal plan](../../../plans/threads-immediate-archive/plan.md) +records the extension's implementation and targeted verification separately +from the original task-action delivery. + ## Ownership and dependencies The task system owns the action target and delegates execution to the same @@ -30,7 +36,7 @@ commit, merge, work orders and verification results. | --- | --- | | `REQ-TASKS-THREADS-ACTIONS-001` | Shared task operations; menu composition | | `REQ-TASKS-THREADS-ACTIONS-002` | Target lifetime; shared task operations; failure behavior | -| `REQ-TASKS-THREADS-ACTIONS-003` | Admission and selection recovery; state delivery | +| `REQ-TASKS-THREADS-ACTIONS-003` | Pending archive exclusion; admission and selection recovery; state delivery | | `REQ-TASKS-THREADS-ACTIONS-004` | Header entry points; phone composition; dismissal and focus | ## Existing implementation boundaries @@ -201,12 +207,72 @@ compact. The drawer owns vertical drag/scroll only while open. No new gesture listener, pointer capture, touch cancellation, or `touch-action: none` is added to the native horizontal swiper. +## Pending archive exclusion + +`coordinateTaskRemovalBatch` publishes `taskRemoval.operationsByToken` before +calling an archive mutation. Each operation retains its action, workspace, +target IDs and captured cascade descendants until request reconciliation +finishes. `stayOnListing` suppresses detail navigation, but retains this +operation state. Reuse that intent as the Threads presentation boundary. + +`ThreadsPageClient` subscribes to the existing operation map and derives the +union of `taskIds` for operations whose action is `archive`. Pass that set to +an optional `excludedTaskIds` input on `queryThreadView`. Apply the exclusion +before task scope, filters, sorting, limits, counts and temporary deep-link +admission. All query candidate collections must agree on the exclusion. +Workspace scoping remains in the existing snapshot/candidate path. Pending +delete operations retain their current behavior in this archive-only repair. + +Feed the resulting candidates to `useStableThreadOrder` and `ThreadsBoard`. +Removing membership unmounts the outgoing column and its `TaskChatPanel`, +releasing detail activity through the existing viewport owner. A CSS-hidden +chat, disabled composer, or per-column placeholder does not satisfy this +boundary. The board-level task-action owner remains mounted, even when the +last column leaves, so its captured request and error reporting can finish. +Keep the viewport observer attached across membership changes; callback refs +register and unregister shells without resetting surviving visibility. Rebuild +observation when an empty/nonempty transition replaces the board element. +This preserves surviving editor mounts without changing activation budgets. + +Pending intent is transient and is not part of the saved view or query +fingerprint. Do not change snapshot task state, fabricate archive events, or +reset stable ordering to hide a column. Existing WebSocket handling continues +to reconcile all authoritative task/session data. Successful cleanup removes +task rows before the coordinator releases its operation, so release cannot +briefly readmit a successfully archived task. A rejected request releases only +its own exclusion; eligible rows return through the ordinary query and stable +order, while confirmed removals stay absent. Returning rows follow existing +arrival ordering; surviving rows keep their positions. + +A previously consumed task/session deep link must not become a fresh focus +request when pending exclusion temporarily changes its resolved target to +null. Keep the focus-request identity tied to the actual URL request, separate +from its currently rendered resolution, in the page/board focus adapter. +Retain dismissal across temporary absence. New explicit URL requests still +receive normal focus behavior; do not clear unrelated URL parameters or reset +order during bookkeeping. A failed archive cannot close a newer menu or +scroll back to its restored column. `useThreadFocusRequest` tracks the visual +mark separately from the viewport owner's initial detail fallback. Interaction +retires the mark without unmounting the requested conversation before the +first visibility callback. Once a consumed request's target leaves the deck, +its fallback is cleared and readmission cannot replace the surviving chat. +An unconsumed request can still activate a target that resolves after hydration. +URL-driven board callers supply an explicit stable request key; the optional +task-ID default retains legacy dismissal semantics for non-URL callers. + +Desktop uses the remaining columns and their existing scroll-offset recovery. +Phone uses the next snapped conversation, or the existing empty state, after +the archive drawer/confirmation closes. The compact topbar, visible overflow +entry, one transcript budget, picker, scroll owners and safe-area geometry +remain the shipped composition. No new controls, copy or overlays are needed. + ## Admission and selection recovery Let `queryThreadView` and `useStableThreadOrder` process confirmed shared state -as today. Changes to priority can affect existing saved-view filters; moves, -archive, delete, external updates, and manual filter changes use the same -membership reconciliation. Do not invent a parallel admitted list or sort. +with the pending archive exclusion above. Changes to priority can affect +existing saved-view filters; moves, archive, delete, external updates, and +manual filter changes use the same membership reconciliation. Do not invent a +parallel admitted list or sort. `resolveRemainingThreadId(previousOrder, nextOrder, currentTaskId)` implements the requirement's successor/predecessor rule. `useThreadSelectionRecovery` @@ -256,6 +322,9 @@ Shared removal cleanup is idempotent if a task event beats the HTTP response. Failure paths preserve confirmed values, release pending state, and display the shared localized error. Add generic error feedback at the shared action boundary only where the reused API currently propagates an error without visible UI. +Pending archive recovery restores query eligibility, never task persistence or +an archived workspace. An in-flight view/workspace change remains authoritative +when a failed operation releases its exclusion. Server authorization remains final. Re-check availability for the captured task before dispatch, and exercise rejection and provider disappearance in @@ -264,6 +333,14 @@ operation are introduced. ## Verification design +The archive extension needs deferred-request regressions before any server +archive event, plus early lifecycle events while the HTTP response remains +held. Assert column/chat unmounting throughout that interval, query counts and +limit refill, cascade targets, last-column recovery, independent operations, +and rejection with a consumed deep link and newer reader interaction. Keep +controls for cancelled confirmation and pending deletion. The follow-up plan +owns exact commands and desktop/mobile scenario mapping. + Focused TDD covers target changes during menus/confirmations, live eligibility, shared mutation success/failure, pending-operation guards, and deterministic recovery. Browser tests use real isolated task fixtures and perform all six