FN-8879: preserve local task detail patches
Keep locally authored task-detail updates visible without weakening server snapshot freshness. - Add a local patch helper that preserves lifecycle guards only for explicitly stale patches. - Apply id-less local detail updates in modal, split-detail, and main-panel views while rejecting foreign task ids. - Cover equal-clock, lifecycle, and split-detail patch behavior with dashboard tests. - Add a patch changeset for visible PR and review updates. Files changed: .changeset/fn-8879-detail-task-patch.md | 7 ++ packages/dashboard/app/components/ListView.tsx | 15 +++-- .../app/components/__tests__/ListView.test.tsx | 21 ++++++ .../app/components/dashboard/MainContent.tsx | 6 +- .../MainContent.detail-task-patch.test.tsx | 77 ++++++++++++++++++++++ .../app/hooks/__tests__/useModalManager.test.ts | 42 ++++++++++++ .../__tests__/useTasks-hydration-freshness.test.ts | 61 ++++++++++++++++- packages/dashboard/app/hooks/useModalManager.ts | 15 +++-- packages/dashboard/app/hooks/useTasks.ts | 45 +++++++++++++ 9 files changed, 275 insertions(+), 14 deletions(-) Fusion-Task-Id: FN-8879 Fusion-Task-Lineage: 7166b597-c4c7-491a-a9d8-856fa820865e Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-8879-detail-task-patch.md
Normal file
7
.changeset/fn-8879-detail-task-patch.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Keep PR and review updates visible in open task details.
|
||||
category: fix
|
||||
dev: Adds applyLocalTaskPatch so FN-8796 server-clock arbitration does not reject local detail patches.
|
||||
@@ -19,7 +19,7 @@ import { isTaskStuck } from "../utils/taskStuck";
|
||||
import { hasPendingAutomaticRecovery, isTaskManuallyRetryable } from "../utils/taskRecovery";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
import { useViewportMode } from "../hooks/useViewportMode";
|
||||
import { mergeTaskSnapshot } from "../hooks/useTasks";
|
||||
import { applyLocalTaskPatch, mergeTaskSnapshot } from "../hooks/useTasks";
|
||||
import { getScopedItem, removeScopedItem, setScopedItem } from "../utils/projectStorage";
|
||||
import { ALL_WORKFLOWS_BOARD_VIEW_ID } from "../utils/boardWorkflowSelection";
|
||||
import {
|
||||
@@ -2120,7 +2120,8 @@ export function ListView({
|
||||
try {
|
||||
const updatedTask = await updateTask(task.id, { githubTracking: { enabled: true } }, projectId);
|
||||
onTasksUpdated?.([updatedTask]);
|
||||
setSelectedTaskSnapshot((previous) => previous?.id === updatedTask.id ? mergeTaskSnapshot(previous, updatedTask) : previous);
|
||||
// FNXC:TaskDetailStateStability 2026-08-09-07:13: updateTask returns a full Task with an id, so this PATCH-response sink intentionally keeps strict identity matching while applying local-patch semantics.
|
||||
setSelectedTaskSnapshot((previous) => previous?.id === updatedTask.id ? applyLocalTaskPatch(previous, updatedTask) : previous);
|
||||
addToast(t("taskDetail.githubTracking.issueCreationRequested", "Requested GitHub tracking issue creation"), "info");
|
||||
} catch (err) {
|
||||
addToast(t("taskDetail.updateFailed", "Failed to update {{id}}: {{error}}", { id: task.id, error: getErrorMessage(err) }), "error");
|
||||
@@ -3703,10 +3704,16 @@ export function ListView({
|
||||
onResetTask={onResetTask}
|
||||
onDuplicateTask={onDuplicateTask}
|
||||
onPopOut={onPopOut ? () => onPopOut(selectedTaskSnapshot) : undefined}
|
||||
/*
|
||||
FNXC:TaskDetailStateStability 2026-08-09-07:13:
|
||||
Locally-authored split-detail patches accept an absent id and use applyLocalTaskPatch.
|
||||
Live board, SSE, and fetch snapshots remain on mergeTaskSnapshot so server clock
|
||||
arbitration continues to protect lifecycle state outside this local callback.
|
||||
*/
|
||||
onTaskUpdated={(updatedTask) => {
|
||||
setSelectedTaskSnapshot((previous) => {
|
||||
if (!previous || previous.id !== updatedTask.id) return previous;
|
||||
return mergeTaskSnapshot(previous, updatedTask);
|
||||
if (!previous || (updatedTask.id !== undefined && updatedTask.id !== previous.id)) return previous;
|
||||
return applyLocalTaskPatch(previous, { ...updatedTask, id: previous.id });
|
||||
});
|
||||
}}
|
||||
addToast={addToast}
|
||||
|
||||
@@ -179,14 +179,19 @@ vi.mock("../TaskDetailModal", () => ({
|
||||
task,
|
||||
onOpenDetail,
|
||||
onRequestClose,
|
||||
onTaskUpdated,
|
||||
}: {
|
||||
task: Task | TaskDetail;
|
||||
onOpenDetail?: (task: Task | TaskDetail) => void;
|
||||
onRequestClose?: () => void;
|
||||
onTaskUpdated?: (patch: Partial<TaskDetail>) => void;
|
||||
}) => (
|
||||
<div data-testid="task-detail-content">
|
||||
<span>{task.id}</span>
|
||||
<output data-testid="split-detail-title">{task.title}</output>
|
||||
<button type="button" onClick={() => onRequestClose?.()}>Close detail</button>
|
||||
<button type="button" onClick={() => onTaskUpdated?.({ title: "renamed" })}>Patch split without id</button>
|
||||
<button type="button" onClick={() => onTaskUpdated?.({ id: "FN-FOREIGN", title: "foreign" })}>Patch split foreign id</button>
|
||||
{(task.dependencies ?? []).map((dependencyId) => (
|
||||
<button
|
||||
key={dependencyId}
|
||||
@@ -2212,6 +2217,22 @@ describe("ListView", () => {
|
||||
viewportSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("applies id-less local split-detail patches and ignores foreign ids", async () => {
|
||||
const viewportSpy = mockDesktopViewport();
|
||||
const tasks = [createMockTask({ id: "FN-001", title: "Original split title" })];
|
||||
renderListView({ tasks });
|
||||
|
||||
fireEvent.click(screen.getByText("FN-001").closest("tr")!);
|
||||
expect(await screen.findByTestId("task-detail-content")).toBeInTheDocument();
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Patch split without id" }));
|
||||
expect(screen.getByTestId("split-detail-title")).toHaveTextContent("renamed");
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Patch split foreign id" }));
|
||||
expect(screen.getByTestId("split-detail-title")).toHaveTextContent("renamed");
|
||||
viewportSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("clears the desktop split-detail shell when embedded detail requests close", async () => {
|
||||
const viewportSpy = mockDesktopViewport();
|
||||
const tasks = [createMockTask({ id: "FN-001", title: "Task" })];
|
||||
|
||||
@@ -8,7 +8,7 @@ import { Board } from "../Board";
|
||||
import { TaskCard } from "../TaskCard";
|
||||
import { ListView } from "../ListView";
|
||||
import { TaskDetailContent } from "../TaskDetailModal";
|
||||
import { mergeTaskSnapshot } from "../../hooks/useTasks";
|
||||
import { applyLocalTaskPatch, mergeTaskSnapshot } from "../../hooks/useTasks";
|
||||
import { ProjectOverview } from "../ProjectOverview";
|
||||
import { MissionManager } from "../MissionManager";
|
||||
import { MailboxView } from "../MailboxView";
|
||||
@@ -937,8 +937,8 @@ export function MainContent({
|
||||
onRequestClose={closeTaskDetailMainPanel}
|
||||
onTaskUpdated={(updatedTask) => {
|
||||
setMainPanelDetailTask((previous) => {
|
||||
if (!previous || previous.id !== updatedTask.id) return previous;
|
||||
return mergeTaskSnapshot(previous, updatedTask);
|
||||
if (!previous || (updatedTask.id !== undefined && updatedTask.id !== previous.id)) return previous;
|
||||
return applyLocalTaskPatch(previous, { ...updatedTask, id: previous.id });
|
||||
});
|
||||
}}
|
||||
addToast={addToast}
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { useState } from "react";
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { TaskDetail } from "@fusion/core";
|
||||
import { MainContent } from "../MainContent";
|
||||
import type { MainContentProps } from "../types";
|
||||
|
||||
vi.mock("../../TaskDetailModal", () => ({
|
||||
TaskDetailContent: ({ task, onTaskUpdated }: {
|
||||
task: TaskDetail;
|
||||
onTaskUpdated?: (patch: Partial<TaskDetail>) => void;
|
||||
}) => (
|
||||
<section>
|
||||
<output data-testid="main-detail-title">{task.title}</output>
|
||||
<output data-testid="main-detail-pr">{task.prInfo?.number ?? "none"}</output>
|
||||
<button type="button" onClick={() => onTaskUpdated?.({ title: "renamed" })}>Patch without id</button>
|
||||
<button type="button" onClick={() => onTaskUpdated?.({ id: "FN-FOREIGN", title: "foreign" })}>Patch foreign id</button>
|
||||
<button type="button" onClick={() => onTaskUpdated?.({ ...task, prInfo: { number: 42 } } as Partial<TaskDetail>)}>Patch equal clock</button>
|
||||
</section>
|
||||
),
|
||||
}));
|
||||
|
||||
const detailTask = {
|
||||
id: "FN-MAIN",
|
||||
title: "Original title",
|
||||
description: "detail",
|
||||
column: "todo",
|
||||
status: "pending",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
log: [],
|
||||
createdAt: "2026-08-09T10:00:00.000Z",
|
||||
updatedAt: "2026-08-09T10:00:00.000Z",
|
||||
columnMovedAt: "2026-08-09T10:00:00.000Z",
|
||||
} as TaskDetail;
|
||||
|
||||
function DetailHost(): JSX.Element {
|
||||
const [task, setTask] = useState<TaskDetail>(detailTask);
|
||||
const props = {
|
||||
taskView: "task-detail",
|
||||
mainPanelDetailTask: task,
|
||||
setMainPanelDetailTask: setTask,
|
||||
tasks: [],
|
||||
filteredBoardTasks: [],
|
||||
currentProject: { id: "project-1" },
|
||||
modalManager: {},
|
||||
globalPaused: false,
|
||||
t: (key: string, fallback?: string) => fallback ?? key,
|
||||
closeTaskDetailMainPanel: vi.fn(),
|
||||
popOutTaskDetail: vi.fn(),
|
||||
openTaskDetailInMainPanel: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
retryTask: vi.fn(),
|
||||
pauseTask: vi.fn(),
|
||||
unpauseTask: vi.fn(),
|
||||
resetTask: vi.fn(),
|
||||
duplicateTask: vi.fn(),
|
||||
} as unknown as MainContentProps;
|
||||
return <MainContent {...props} />;
|
||||
}
|
||||
|
||||
describe("MainContent local detail task patches", () => {
|
||||
it("applies id-less and equal-clock patches but ignores a foreign id", () => {
|
||||
render(<DetailHost />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Patch without id" }));
|
||||
expect(screen.getByTestId("main-detail-title")).toHaveTextContent("renamed");
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Patch foreign id" }));
|
||||
expect(screen.getByTestId("main-detail-title")).toHaveTextContent("renamed");
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Patch equal clock" }));
|
||||
expect(screen.getByTestId("main-detail-pr")).toHaveTextContent("42");
|
||||
});
|
||||
});
|
||||
@@ -469,6 +469,48 @@ describe("useModalManager", () => {
|
||||
expect(result.current.detailTask?.title).toBe("renamed");
|
||||
});
|
||||
|
||||
it("applies equal-clock derived and clock-less lifecycle detail patches", () => {
|
||||
const task = createTaskDetail("FN-LOCAL");
|
||||
const { result } = renderHook(() => useModalManager({ projectId: "proj_1", planningSessions: [] }));
|
||||
act(() => { result.current.openDetailTask(task); });
|
||||
|
||||
act(() => {
|
||||
result.current.updateDetailTask({ ...task, prInfo: { number: 42 } } as Partial<TaskDetail>);
|
||||
result.current.updateDetailTask({ column: "done", status: "completed" });
|
||||
});
|
||||
|
||||
expect(result.current.detailTask).toMatchObject({ column: "done", status: "completed", prInfo: { number: 42 } });
|
||||
});
|
||||
|
||||
it("keeps lifecycle state for a present strictly older local patch", () => {
|
||||
const task = { ...createTaskDetail("FN-STALE"), updatedAt: "2026-08-09T10:00:00.000Z", columnMovedAt: "2026-08-09T10:00:00.000Z", status: "executing" };
|
||||
const { result } = renderHook(() => useModalManager({ projectId: "proj_1", planningSessions: [] }));
|
||||
act(() => { result.current.openDetailTask(task); });
|
||||
act(() => {
|
||||
result.current.updateDetailTask({
|
||||
title: "Fresh local metadata",
|
||||
column: "done",
|
||||
columnMovedAt: "2026-08-09T09:00:00.000Z",
|
||||
status: "completed",
|
||||
updatedAt: "2026-08-09T09:00:00.000Z",
|
||||
});
|
||||
});
|
||||
|
||||
expect(result.current.detailTask).toMatchObject({
|
||||
title: "Fresh local metadata",
|
||||
column: "todo",
|
||||
columnMovedAt: task.columnMovedAt,
|
||||
status: "executing",
|
||||
updatedAt: task.updatedAt,
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores a detail patch safely before a task is open", () => {
|
||||
const { result } = renderHook(() => useModalManager({ projectId: "proj_1", planningSessions: [] }));
|
||||
act(() => { result.current.updateDetailTask({ title: "No detail" }); });
|
||||
expect(result.current.detailTask).toBeNull();
|
||||
});
|
||||
|
||||
it("tracks a target workflow id for normal workflow editor opens and resets it on close", () => {
|
||||
const { result } = renderHook(() =>
|
||||
useModalManager({ projectId: "proj_1", planningSessions: [] }),
|
||||
|
||||
@@ -19,7 +19,7 @@ a mocked cache is what let the missing `savedAt` plumbing hide.
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import type { Task } from "@fusion/core";
|
||||
import { mergeTaskSnapshot, useTasks } from "../useTasks";
|
||||
import { applyLocalTaskPatch, mergeTaskSnapshot, useTasks } from "../useTasks";
|
||||
import * as api from "../../api";
|
||||
import { SWR_CACHE_KEYS } from "../../utils/swrCache";
|
||||
import { isTaskStuck, countStuckTasks } from "../../utils/taskStuck";
|
||||
@@ -467,6 +467,65 @@ The rule under test: a single-row live update may advance the clock only AFTER a
|
||||
every row. It must still advance after that, or stuck detection would silently stop firing for the rest
|
||||
of a long SSE session (this hook has no periodic poll).
|
||||
*/
|
||||
describe("applyLocalTaskPatch", () => {
|
||||
const current = {
|
||||
...createInProgressTask("FN-LOCAL", Date.parse("2026-08-09T10:00:00.000Z")),
|
||||
columnMovedAt: "2026-08-09T10:00:00.000Z",
|
||||
prompt: "# Full detail",
|
||||
log: [{ timestamp: "2026-08-09T10:00:00.000Z", action: "loaded" }],
|
||||
} as Task;
|
||||
|
||||
it("accepts an absent id but rejects an explicit foreign id", () => {
|
||||
expect(applyLocalTaskPatch(current, { title: "Local rename" }).title).toBe("Local rename");
|
||||
expect(applyLocalTaskPatch(current, { id: "FN-OTHER", title: "Foreign" })).toBe(current);
|
||||
});
|
||||
|
||||
it("applies clock-less lifecycle patches and equal-clock derived patches", () => {
|
||||
const clockless = applyLocalTaskPatch(current, { column: "done", status: "completed" });
|
||||
const equalClock = applyLocalTaskPatch(current, { ...current, prInfo: { number: 12 } } as Partial<Task>);
|
||||
|
||||
expect(clockless).toMatchObject({ column: "done", status: "completed" });
|
||||
expect(equalClock.prInfo).toMatchObject({ number: 12 });
|
||||
});
|
||||
|
||||
it("preserves lifecycle state only for present strictly older clocks", () => {
|
||||
const stale = applyLocalTaskPatch(current, {
|
||||
title: "Fresh metadata",
|
||||
column: "done",
|
||||
columnMovedAt: "2026-08-09T09:00:00.000Z",
|
||||
status: "completed",
|
||||
updatedAt: "2026-08-09T09:00:00.000Z",
|
||||
});
|
||||
|
||||
expect(stale).toMatchObject({
|
||||
title: "Fresh metadata",
|
||||
column: current.column,
|
||||
columnMovedAt: current.columnMovedAt,
|
||||
status: current.status,
|
||||
updatedAt: current.updatedAt,
|
||||
});
|
||||
});
|
||||
|
||||
it("applies a patch clock when the current row has no clock", () => {
|
||||
const clocklessCurrent = { ...current, updatedAt: undefined, columnMovedAt: undefined } as Task;
|
||||
expect(applyLocalTaskPatch(clocklessCurrent, {
|
||||
column: "done",
|
||||
columnMovedAt: "2026-08-09T11:00:00.000Z",
|
||||
status: "completed",
|
||||
updatedAt: "2026-08-09T11:00:00.000Z",
|
||||
})).toMatchObject({ column: "done", status: "completed" });
|
||||
});
|
||||
|
||||
it("does not erase defined detail fields with undefined or omitted patch fields", () => {
|
||||
const patched = applyLocalTaskPatch(current, { title: undefined, description: "Updated" });
|
||||
expect(patched).toMatchObject({ title: current.title, prompt: current.prompt, log: current.log, description: "Updated" });
|
||||
});
|
||||
|
||||
it("preserves reference identity for a no-op patch", () => {
|
||||
expect(applyLocalTaskPatch(current, { title: current.title })).toBe(current);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useTasks freshness clock vs single-row live updates", () => {
|
||||
const eventCases: [string, (task: Task) => unknown][] = [
|
||||
["task:created", (task) => task],
|
||||
|
||||
@@ -4,7 +4,7 @@ import type { Task, TaskDetail } from "@fusion/core";
|
||||
import type { SectionId } from "../components/SettingsModal";
|
||||
import type { ToastType } from "./useToast";
|
||||
import { removeScopedItem } from "../utils/projectStorage";
|
||||
import { mergeTaskSnapshot } from "./useTasks";
|
||||
import { applyLocalTaskPatch } from "./useTasks";
|
||||
|
||||
/*
|
||||
FNXC:TaskDetailActivity 2026-06-30-22:15:
|
||||
@@ -390,13 +390,16 @@ export function useModalManager(options: UseModalManagerOptions): ModalManager {
|
||||
setDetailTaskInitialAction(null);
|
||||
setDetailTaskOrigin(null);
|
||||
}, []);
|
||||
/*
|
||||
FNXC:TaskDetailStateStability 2026-08-09-07:13:
|
||||
This callback receives locally-authored patches from the open detail view, not competing server
|
||||
snapshots. FN-5148 pins the id rule: reject an explicit foreign id but accept an absent id; FN-8796
|
||||
must not turn absent/equal local clocks into stale evidence. AppModals owns live board/SSE arbitration.
|
||||
*/
|
||||
const updateDetailTask = useCallback((updated: Partial<TaskDetail>) => {
|
||||
setDetailTask((prev) => {
|
||||
if (!prev) return prev;
|
||||
if (updated.id !== undefined && updated.id !== prev.id) {
|
||||
return prev;
|
||||
}
|
||||
return mergeTaskSnapshot(prev, updated as Task);
|
||||
if (!prev || (updated.id !== undefined && updated.id !== prev.id)) return prev;
|
||||
return applyLocalTaskPatch(prev, { ...updated, id: prev.id });
|
||||
});
|
||||
}, []);
|
||||
const closeDetailTask = useCallback(() => {
|
||||
|
||||
@@ -258,6 +258,11 @@ newer than the row and resolves an equal legacy move clock; sparse SSE patches n
|
||||
This helper intentionally merges only defined sparse fields and retains a fetched detail's prompt/log
|
||||
when a slim board row arrives. Every open-detail host and useTasks ingestion uses this one boundary so
|
||||
one provider cannot regress a modal, main panel, split detail, dock, or popup independently.
|
||||
|
||||
FNXC:TaskDetailStateStability 2026-08-09-07:13:
|
||||
`mergeTaskSnapshot` arbitrates server snapshots only. Locally-authored detail patches must use
|
||||
`applyLocalTaskPatch`: FN-5148 requires mismatched ids to be ignored while accepting an absent id, and
|
||||
FN-8796 showed that an absent or equal local clock is not evidence of staleness.
|
||||
*/
|
||||
export interface TaskSnapshotMergeOptions {
|
||||
/** A complete board/detail fetch can resolve an otherwise ambiguous legacy column clock. */
|
||||
@@ -374,6 +379,46 @@ export function mergeTaskSnapshot<T extends Task>(
|
||||
return merged as T;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TaskDetailStateStability 2026-08-09-07:13:
|
||||
Open detail views author sparse patches after a PATCH response or derived PR/review refresh. Unlike
|
||||
server snapshots, these patches are applied by intent: FN-5148 ignores an explicit foreign id but
|
||||
accepts an absent id. FN-8796's stale lifecycle protection remains only when both sides provide a
|
||||
clock and the local patch is strictly older; absent and equal clocks are not stale evidence.
|
||||
*/
|
||||
export function applyLocalTaskPatch<T extends Task>(current: T, patch: Partial<Task>): T {
|
||||
if (patch.id !== undefined && patch.id !== current.id) return current;
|
||||
|
||||
const merged = { ...current } as Record<string, unknown>;
|
||||
for (const [key, value] of Object.entries(patch)) {
|
||||
if (value !== undefined) merged[key] = value;
|
||||
}
|
||||
|
||||
const hasClock = (value: unknown): value is string => typeof value === "string" && value.length > 0;
|
||||
if (
|
||||
hasClock(patch.columnMovedAt)
|
||||
&& hasClock(current.columnMovedAt)
|
||||
&& compareTimestamps(patch.columnMovedAt, current.columnMovedAt) < 0
|
||||
) {
|
||||
merged.column = current.column;
|
||||
merged.columnMovedAt = current.columnMovedAt;
|
||||
}
|
||||
if (
|
||||
hasClock(patch.updatedAt)
|
||||
&& hasClock(current.updatedAt)
|
||||
&& compareTimestamps(patch.updatedAt, current.updatedAt) < 0
|
||||
) {
|
||||
merged.status = current.status;
|
||||
merged.updatedAt = current.updatedAt;
|
||||
}
|
||||
|
||||
const mergedKeys = Object.keys(merged);
|
||||
if (mergedKeys.length === Object.keys(current).length && mergedKeys.every((key) => merged[key] === (current as Record<string, unknown>)[key])) {
|
||||
return current;
|
||||
}
|
||||
return merged as T;
|
||||
}
|
||||
|
||||
function mergeIncomingTask(current: Task, incoming: Task, options?: TaskSnapshotMergeOptions): Task {
|
||||
return mergeTaskSnapshot(current, incoming, options);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user