From 15949959b0da67cc997e7e80e99b8bee6af8d2df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 20 Jul 2026 18:04:11 +0100 Subject: [PATCH] fix: preserve authoritative archived task state (#1826) --- .../task/task-page-content-helpers.test.ts | 48 ++++++++++++++----- .../task/task-page-content-helpers.ts | 31 +++++------- .../web/components/task/task-page-content.tsx | 39 ++++++++++++--- apps/web/components/task/task-page-inner.tsx | 5 ++ apps/web/components/task/task-top-bar.tsx | 7 ++- .../components/task/task-unarchive-button.tsx | 16 +++++-- .../task/unarchive-detail-topbar.spec.ts | 10 ++-- .../lib/ws/handlers/tasks-unarchive.test.ts | 11 ++--- 8 files changed, 111 insertions(+), 56 deletions(-) diff --git a/apps/web/components/task/task-page-content-helpers.test.ts b/apps/web/components/task/task-page-content-helpers.test.ts index d1b477f3fc..e3c4e3fb6d 100644 --- a/apps/web/components/task/task-page-content-helpers.test.ts +++ b/apps/web/components/task/task-page-content-helpers.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it, vi } from "vitest"; -import type { Task } from "@/lib/types/http"; +import { + taskId as toTaskId, + workflowId as toWorkflowId, + workspaceId as toWorkspaceId, + type Task, +} from "@/lib/types/http"; import type { KanbanState } from "@/lib/state/slices"; import { buildArchivedValue, @@ -12,6 +17,7 @@ import { } from "./task-page-content-helpers"; type KanbanTask = KanbanState["tasks"][number]; +const ARCHIVED_AT = "2026-07-19T00:00:00Z"; function makeArchivedTaskDetails(overrides: Partial = {}): Task { return { @@ -26,8 +32,8 @@ function makeArchivedTaskDetails(overrides: Partial = {}): Task { priority: 0, repositories: [], created_at: "", - updated_at: "2026-07-19T00:00:00Z", - archived_at: "2026-07-19T00:00:00Z", + updated_at: ARCHIVED_AT, + archived_at: ARCHIVED_AT, ...overrides, } as Task; } @@ -222,15 +228,19 @@ describe("syncActiveTaskSession", () => { }); describe("resolveEffectiveTask archived state", () => { - // Regression: unarchiving from the detail top bar publishes - // task.updated(archived_at=null) which re-adds the task to the kanban, but - // the one-shot fetchTask details still carry the old archived_at. The - // resolved task must reflect the live kanban entry so the top bar stops - // showing the archived UI / Unarchive button — otherwise the task "never - // comes back" after a successful unarchive. - it("clears a stale archived_at when the task is present in the kanban", () => { + it("keeps fetched archived state when a stale matching kanban card remains", () => { const taskDetails = makeArchivedTaskDetails(); - const kanbanTask = makeKanbanTask(); + const kanbanTask = makeKanbanTask({ updatedAt: "2026-07-18T00:00:00Z" }); + + const resolved = resolveEffectiveTask(taskDetails, null, kanbanTask, "task-1"); + + expect(resolved?.archived_at).toBe(ARCHIVED_AT); + expect(buildArchivedValue(resolved, null).isArchived).toBe(true); + }); + + it("clears fetched archived state when a matching kanban card is newer", () => { + const taskDetails = makeArchivedTaskDetails(); + const kanbanTask = makeKanbanTask({ updatedAt: "2026-07-20T00:00:00Z" }); const resolved = resolveEffectiveTask(taskDetails, null, kanbanTask, "task-1"); @@ -243,7 +253,7 @@ describe("resolveEffectiveTask archived state", () => { const resolved = resolveEffectiveTask(taskDetails, null, null, "task-1"); - expect(resolved?.archived_at).toBe("2026-07-19T00:00:00Z"); + expect(resolved?.archived_at).toBe(ARCHIVED_AT); expect(buildArchivedValue(resolved, null).isArchived).toBe(true); }); @@ -257,4 +267,18 @@ describe("resolveEffectiveTask archived state", () => { expect(resolved?.state).toBe("IN_PROGRESS"); expect(resolved?.workspace_id).toBe("ws-1"); }); + + it("does not copy IDs from rejected task details into a kanban-only placeholder", () => { + const unrelatedTask = makeArchivedTaskDetails({ + id: toTaskId("other-task"), + workspace_id: toWorkspaceId("other-workspace"), + workflow_id: toWorkflowId("other-workflow"), + }); + const kanbanTask = makeKanbanTask({ id: toTaskId("task-1") }); + + const resolved = resolveEffectiveTask(unrelatedTask, null, kanbanTask, "task-1"); + + expect(resolved?.workspace_id).toBe(""); + expect(resolved?.workflow_id).toBe(""); + }); }); diff --git a/apps/web/components/task/task-page-content-helpers.ts b/apps/web/components/task/task-page-content-helpers.ts index e172d97866..d6bf923aac 100644 --- a/apps/web/components/task/task-page-content-helpers.ts +++ b/apps/web/components/task/task-page-content-helpers.ts @@ -118,7 +118,7 @@ export function resolveEffectiveTask( if (!baseTask && !kanbanTask) return null; if (baseTask) return mergeBaseWithKanban(baseTask, kanbanTask); - if (kanbanTask) return buildTaskFromKanban(kanbanTask, taskDetails, initialTask); + if (kanbanTask) return buildTaskFromKanban(kanbanTask); return null; } @@ -127,6 +127,13 @@ export function mergeBaseWithKanban( kanbanTask: KanbanState["tasks"][number] | null, ): Task { if (!kanbanTask) return baseTask; + const kanbanUpdatedAt = Date.parse(kanbanTask.updatedAt ?? ""); + const baseUpdatedAt = Date.parse(baseTask.updated_at ?? ""); + const hasNewerKanbanState = + Boolean(baseTask.archived_at) && + Number.isFinite(kanbanUpdatedAt) && + Number.isFinite(baseUpdatedAt) && + kanbanUpdatedAt > baseUpdatedAt; return { ...baseTask, title: kanbanTask.title ?? baseTask.title, @@ -136,25 +143,11 @@ export function mergeBaseWithKanban( position: kanbanTask.position ?? baseTask.position, state: (kanbanTask.state as Task["state"] | undefined) ?? baseTask.state, repositories: baseTask.repositories, - // A task present in the kanban board is by definition not archived: the - // board never holds archived tasks, and unarchive re-adds it via - // task.updated(archived_at=null). The one-shot `fetchTask` that seeds - // baseTask only refetches on task-id change, so it keeps a stale - // archived_at after an in-place unarchive from the detail top bar — - // trust the live kanban entry instead. Without this the top bar keeps - // showing the Unarchive button / archived banner and the task "never - // comes back" even though the backend restored it. - archived_at: null, + archived_at: hasNewerKanbanState ? null : baseTask.archived_at, }; } -export function buildTaskFromKanban( - kanbanTask: KanbanState["tasks"][number], - taskDetails: Task | null, - initialTask: Task | null, -): Task { - const prevWorkspaceId = taskDetails?.workspace_id ?? initialTask?.workspace_id; - const prevBoardId = taskDetails?.workflow_id ?? initialTask?.workflow_id; +export function buildTaskFromKanban(kanbanTask: KanbanState["tasks"][number]): Task { return { id: toTaskId(kanbanTask.id), title: kanbanTask.title, @@ -162,8 +155,8 @@ export function buildTaskFromKanban( workflow_step_id: kanbanTask.workflowStepId, position: kanbanTask.position, state: kanbanTask.state ?? "CREATED", - workspace_id: prevWorkspaceId ?? toWorkspaceId(""), - workflow_id: prevBoardId ?? toWorkflowId(""), + workspace_id: toWorkspaceId(""), + workflow_id: toWorkflowId(""), priority: 0, repositories: [], created_at: "", diff --git a/apps/web/components/task/task-page-content.tsx b/apps/web/components/task/task-page-content.tsx index 66f9122ab1..f2c787c769 100644 --- a/apps/web/components/task/task-page-content.tsx +++ b/apps/web/components/task/task-page-content.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { IconAlertTriangle } from "@tabler/icons-react"; import type { Repository, RepositoryScript, Task } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; @@ -217,7 +217,18 @@ function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) { setTaskDetails, ]); - return { task, kanbanTask, taskLoadError: hasTaskDetails ? null : taskLoadError }; + const onTaskUnarchived = useCallback((taskId: string) => { + setTaskDetails((current) => + current?.id === taskId ? { ...current, archived_at: null } : current, + ); + }, []); + + return { + task, + kanbanTask, + taskLoadError: hasTaskDetails ? null : taskLoadError, + onTaskUnarchived, + }; } function useTaskPageData( @@ -239,7 +250,7 @@ function useTaskPageData( return session?.task_id === activeTaskId ? sid : null; }); - const { task, taskLoadError } = useTaskDetails(activeTaskId, initialTask); + const { task, taskLoadError, onTaskUnarchived } = useTaskDetails(activeTaskId, initialTask); const agent = useSessionAgent(task); const ensureSession = useEnsureTaskSession(task); @@ -266,7 +277,15 @@ function useTaskPageData( [effectiveRepositories, task?.repositories], ); - return { task, taskLoadError, agent, effectiveSessionId, repository, ensureSession }; + return { + task, + taskLoadError, + agent, + effectiveSessionId, + repository, + ensureSession, + onTaskUnarchived, + }; } export function TaskPageContent({ @@ -285,8 +304,15 @@ export function TaskPageContent({ const { isMobile } = useResponsiveBreakpoint(); const connectionStatus = useAppStore((state) => state.connection.status); - const { task, taskLoadError, agent, effectiveSessionId, repository, ensureSession } = - useTaskPageData(initialTask, initialTaskId, sessionId, initialRepositories); + const { + task, + taskLoadError, + agent, + effectiveSessionId, + repository, + ensureSession, + onTaskUnarchived, + } = useTaskPageData(initialTask, initialTaskId, sessionId, initialRepositories); const workflowSteps = useWorkflowStepsMapped(); const sessionPanel = useSessionPanelState(effectiveSessionId); @@ -334,6 +360,7 @@ export function TaskPageContent({ initialLayout={initialLayout} officeTaskHref={officeTaskHref} ensureSession={ensureSession} + onTaskUnarchived={onTaskUnarchived} /> ); } diff --git a/apps/web/components/task/task-page-inner.tsx b/apps/web/components/task/task-page-inner.tsx index a26c774b89..3d060106ac 100644 --- a/apps/web/components/task/task-page-inner.tsx +++ b/apps/web/components/task/task-page-inner.tsx @@ -50,6 +50,7 @@ export type TaskPageInnerProps = { initialLayout?: string | null; officeTaskHref?: string | null; ensureSession: UseEnsureTaskSessionResult; + onTaskUnarchived: (taskId: string) => void; }; type RemoteExecutorStatus = { @@ -100,6 +101,7 @@ function buildTaskTopBarProps(params: { sessionWorkflowStepId: string | null; agentctlReady: boolean; officeTaskHref?: string | null; + onTaskUnarchived: (taskId: string) => void; }) { const { taskProps, agent, merged, workflowSteps, showDebugOverlay, onToggleDebugOverlay } = params; @@ -124,6 +126,7 @@ function buildTaskTopBarProps(params: { isAgentctlReady: params.agentctlReady, remoteExecutorType: params.remote.remoteExecutorType, officeTaskHref: params.officeTaskHref, + onTaskUnarchived: params.onTaskUnarchived, }; } @@ -212,6 +215,7 @@ export function TaskPageInner({ initialLayout, officeTaskHref, ensureSession, + onTaskUnarchived, }: TaskPageInnerProps) { const taskProps = resolveTaskProps(task, repository); const remote = resolveRemoteExecutor(resumption.sessionStatus as RemoteExecutorStatus | null); @@ -241,6 +245,7 @@ export function TaskPageInner({ sessionWorkflowStepId: sessionPanel.sessionWorkflowStepId, agentctlReady: agentctlStatus.isReady, officeTaskHref, + onTaskUnarchived, }); const layoutProps = buildTaskLayoutProps({ taskProps, diff --git a/apps/web/components/task/task-top-bar.tsx b/apps/web/components/task/task-top-bar.tsx index 0b3c68f3af..38f857d5bb 100644 --- a/apps/web/components/task/task-top-bar.tsx +++ b/apps/web/components/task/task-top-bar.tsx @@ -43,6 +43,7 @@ type TaskTopBarProps = { isAgentctlReady?: boolean; remoteExecutorType?: string | null; officeTaskHref?: string | null; + onTaskUnarchived?: (taskId: string) => void; }; type TopBarLeftProps = { @@ -70,6 +71,7 @@ const TaskTopBar = memo(function TaskTopBar({ issueNumber, remoteExecutorType, officeTaskHref, + onTaskUnarchived, }: TaskTopBarProps) { return (
); @@ -336,6 +339,7 @@ function TopBarRight({ issueUrl, issueNumber, officeTaskHref, + onTaskUnarchived, }: { taskId?: string | null; activeSessionId?: string | null; @@ -349,13 +353,14 @@ function TopBarRight({ issueUrl?: string; issueNumber?: number; officeTaskHref?: string | null; + onTaskUnarchived?: (taskId: string) => void; }) { return (
{isArchived && ( - + )} {officeTaskHref && ( diff --git a/apps/web/components/task/task-unarchive-button.tsx b/apps/web/components/task/task-unarchive-button.tsx index 033c7e3051..9eda61e86e 100644 --- a/apps/web/components/task/task-unarchive-button.tsx +++ b/apps/web/components/task/task-unarchive-button.tsx @@ -7,10 +7,13 @@ import { unarchiveTask } from "@/lib/api"; import { unarchiveToastPayload } from "@/lib/tasks/unarchive-feedback"; import { useToast } from "@/components/toast-provider"; -// Shown in the task top bar when the task is archived. On success the -// backend publishes task.updated with archived_at=null, which restores the -// task in the kanban/store — no manual refetch needed here. -export function TaskUnarchiveButton({ taskId }: { taskId?: string | null }) { +export function TaskUnarchiveButton({ + taskId, + onUnarchived, +}: { + taskId?: string | null; + onUnarchived?: (taskId: string) => void; +}) { const { toast } = useToast(); const [isPending, setIsPending] = useState(false); if (!taskId) return null; @@ -20,6 +23,11 @@ export function TaskUnarchiveButton({ taskId }: { taskId?: string | null }) { try { const result = await unarchiveTask(taskId); toast(unarchiveToastPayload(result)); + if (result.success && result.unarchived_ids.includes(taskId)) { + onUnarchived?.(taskId); + } else if (result.success) { + console.warn("[TaskUnarchiveButton] task missing from successful response", taskId); + } } catch (err) { toast({ title: "Failed to unarchive task", diff --git a/apps/web/e2e/tests/task/unarchive-detail-topbar.spec.ts b/apps/web/e2e/tests/task/unarchive-detail-topbar.spec.ts index a17e937a1b..c6b435be47 100644 --- a/apps/web/e2e/tests/task/unarchive-detail-topbar.spec.ts +++ b/apps/web/e2e/tests/task/unarchive-detail-topbar.spec.ts @@ -2,12 +2,8 @@ import { test, expect } from "../../fixtures/test-base"; // Regression: unarchiving from the task-detail top bar must clear the archived // UI *in place*. The detail view seeds `archived_at` from a one-shot fetchTask -// that only re-fetches when the task id changes. Unarchive publishes -// task.updated(archived_at=null), which re-adds the task to the kanban, and the -// task resolver must trust that live entry over the stale fetch. Before the fix -// the merge kept the stale archived_at, so the Unarchive button stayed and the -// task "never came back" even though the backend restored it and the toast said -// success. +// that only re-fetches when the task id changes, so the successful lifecycle +// request must explicitly clear that local detail state. test("unarchiving from the detail top bar clears the archived UI in place", async ({ testPage, apiClient, @@ -35,5 +31,5 @@ test("unarchiving from the detail top bar clears the archived UI in place", asyn // proves the detail view observed the unarchive without a navigation/refetch. await expect(unarchiveButton).toHaveCount(0); // No redirect away — unarchive keeps the user on the same task route. - await expect(testPage).toHaveURL(new RegExp(`/t/${task.id}`)); + await expect(testPage).toHaveURL((url) => url.pathname === `/t/${task.id}`); }); diff --git a/apps/web/lib/ws/handlers/tasks-unarchive.test.ts b/apps/web/lib/ws/handlers/tasks-unarchive.test.ts index 238f0c4ff1..7db24948f2 100644 --- a/apps/web/lib/ws/handlers/tasks-unarchive.test.ts +++ b/apps/web/lib/ws/handlers/tasks-unarchive.test.ts @@ -56,11 +56,10 @@ function makeUpdatedMessage(payload: Record) { } as Parameters["task.updated"]>>[0]; } -// Unarchive is delivered as task.updated with archived_at back to null. The -// handler must re-add the task to the kanban caches (mirror of the archive -// removal path). +// The backend omits archived_at after unarchive because the field is nil. The +// resulting task.updated event must still re-add the task to the kanban caches. describe("task.updated unarchive restore", () => { - it("re-adds the task to the active kanban when archived_at clears", () => { + it("re-adds the task to the active kanban when archived_at is omitted", () => { const store = makeStore(); const handlers = registerTasksHandlers(store); @@ -72,7 +71,6 @@ describe("task.updated unarchive restore", () => { title: "Restored task", state: "TODO", is_ephemeral: false, - archived_at: null, }), ); @@ -80,7 +78,7 @@ describe("task.updated unarchive restore", () => { expect(state.kanban.tasks.map((t) => t.id)).toContain(TASK_ID); }); - it("re-adds the task to a multi-kanban snapshot when archived_at clears", () => { + it("re-adds the task to a multi-kanban snapshot when archived_at is omitted", () => { const store = makeStore({ kanban: { workflowId: "wf-other", steps: [], tasks: [] } as unknown as AppState["kanban"], kanbanMulti: { @@ -100,7 +98,6 @@ describe("task.updated unarchive restore", () => { title: "Restored task", state: "TODO", is_ephemeral: false, - archived_at: null, }), );