fix: preserve authoritative archived task state (#1826)
This commit is contained in:
@@ -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> = {}): Task {
|
||||
return {
|
||||
@@ -26,8 +32,8 @@ function makeArchivedTaskDetails(overrides: Partial<Task> = {}): 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("");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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: "",
|
||||
|
||||
@@ -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}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 (
|
||||
<header
|
||||
@@ -107,6 +109,7 @@ const TaskTopBar = memo(function TaskTopBar({
|
||||
issueUrl={issueUrl}
|
||||
issueNumber={issueNumber}
|
||||
officeTaskHref={officeTaskHref}
|
||||
onTaskUnarchived={onTaskUnarchived}
|
||||
/>
|
||||
</header>
|
||||
);
|
||||
@@ -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 (
|
||||
<div className="flex items-center justify-self-end gap-2 [&_button]:whitespace-nowrap">
|
||||
<TopbarMetrics activeSessionId={activeSessionId} size="sm" />
|
||||
{isArchived && (
|
||||
<TopbarCluster label="Unarchive task" className="[&_button]:h-7 [&_button]:text-xs">
|
||||
<TaskUnarchiveButton taskId={taskId} />
|
||||
<TaskUnarchiveButton taskId={taskId} onUnarchived={onTaskUnarchived} />
|
||||
</TopbarCluster>
|
||||
)}
|
||||
{officeTaskHref && (
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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}`);
|
||||
});
|
||||
|
||||
@@ -56,11 +56,10 @@ function makeUpdatedMessage(payload: Record<string, unknown>) {
|
||||
} as Parameters<NonNullable<ReturnType<typeof registerTasksHandlers>["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,
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user