feat(office): make task title and description editable (#2891)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
@@ -0,0 +1,132 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
// AC-29: committing a title on a task carrying a pending agent-generated
|
||||
// title persists the user's title and clears the pending-agent-title
|
||||
// metadata.
|
||||
func TestUpdateTaskTitle_ClearsPendingAgentTitleMetadata(t *testing.T) {
|
||||
svc, bus, repo := newTitleTestService(t)
|
||||
ctx := context.Background()
|
||||
seedPendingTitleTask(t, repo, "task-ac29", "sess-owner", true)
|
||||
|
||||
newTitle := "Fix the login bug"
|
||||
task, err := svc.UpdateTask(ctx, "task-ac29", &UpdateTaskRequest{Title: &newTitle})
|
||||
if err != nil {
|
||||
t.Fatalf("UpdateTask: %v", err)
|
||||
}
|
||||
if task.Title != newTitle {
|
||||
t.Fatalf("title = %q, want %q", task.Title, newTitle)
|
||||
}
|
||||
if models.IsAgentTitlePending(task.Metadata) {
|
||||
t.Fatalf("metadata = %v, want the pending marker cleared", task.Metadata)
|
||||
}
|
||||
if models.AgentTitleOwnerSessionID(task.Metadata) != "" {
|
||||
t.Fatalf("metadata = %v, want the owner claim cleared", task.Metadata)
|
||||
}
|
||||
|
||||
stored, err := repo.GetTask(ctx, "task-ac29")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTask: %v", err)
|
||||
}
|
||||
if stored.Title != newTitle {
|
||||
t.Fatalf("persisted title = %q, want %q", stored.Title, newTitle)
|
||||
}
|
||||
if models.IsAgentTitlePending(stored.Metadata) {
|
||||
t.Fatalf("persisted metadata = %v, want the pending marker cleared", stored.Metadata)
|
||||
}
|
||||
|
||||
if types := eventTypes(bus.GetPublishedEvents()); len(types) != 1 || types[0] != events.TaskUpdated {
|
||||
t.Fatalf("published %v, want exactly one %s", types, events.TaskUpdated)
|
||||
}
|
||||
}
|
||||
|
||||
// AC-48: committing a description-only save on a task carrying a pending
|
||||
// agent-generated title persists the new description, leaves the
|
||||
// pending-agent-title metadata intact, and leaves the stored title
|
||||
// unchanged. This exercises the conditional title statement in
|
||||
// updateTaskTx.
|
||||
func TestUpdateTaskDescriptionOnly_LeavesPendingAgentTitleAndStoredTitleIntact(t *testing.T) {
|
||||
svc, bus, repo := newTitleTestService(t)
|
||||
ctx := context.Background()
|
||||
seedPendingTitleTask(t, repo, "task-ac48", "sess-owner", true)
|
||||
|
||||
newDescription := "Investigate the checkout flow regression"
|
||||
task, err := svc.UpdateTask(ctx, "task-ac48", &UpdateTaskRequest{Description: &newDescription})
|
||||
if err != nil {
|
||||
t.Fatalf("UpdateTask: %v", err)
|
||||
}
|
||||
if task.Description != newDescription {
|
||||
t.Fatalf("description = %q, want %q", task.Description, newDescription)
|
||||
}
|
||||
if task.Title != "Untitled" {
|
||||
t.Fatalf("title = %q, want the pending agent title left untouched", task.Title)
|
||||
}
|
||||
if !models.IsAgentTitlePending(task.Metadata) {
|
||||
t.Fatalf("metadata = %v, want the pending marker to survive a description-only save", task.Metadata)
|
||||
}
|
||||
if models.AgentTitleOwnerSessionID(task.Metadata) != "sess-owner" {
|
||||
t.Fatalf("metadata = %v, want the owner claim to survive a description-only save", task.Metadata)
|
||||
}
|
||||
|
||||
stored, err := repo.GetTask(ctx, "task-ac48")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTask: %v", err)
|
||||
}
|
||||
if stored.Description != newDescription {
|
||||
t.Fatalf("persisted description = %q, want %q", stored.Description, newDescription)
|
||||
}
|
||||
if stored.Title != "Untitled" {
|
||||
t.Fatalf("persisted title = %q, want it unchanged", stored.Title)
|
||||
}
|
||||
if !models.IsAgentTitlePending(stored.Metadata) {
|
||||
t.Fatalf("persisted metadata = %v, want the pending marker intact", stored.Metadata)
|
||||
}
|
||||
|
||||
if types := eventTypes(bus.GetPublishedEvents()); len(types) != 1 || types[0] != events.TaskUpdated {
|
||||
t.Fatalf("published %v, want exactly one %s", types, events.TaskUpdated)
|
||||
}
|
||||
}
|
||||
|
||||
// AC-31: a title or description update publishes task.updated without
|
||||
// assignee_agent_profile_id on the payload, so queueTaskAssignedRun cannot
|
||||
// queue a run as a result of the edit.
|
||||
func TestUpdateTaskTitleOrDescription_OmitsAssigneeAgentProfileIDFromPayload(t *testing.T) {
|
||||
svc, bus, repo := newTitleTestService(t)
|
||||
ctx := context.Background()
|
||||
if err := repo.CreateTask(ctx, &models.Task{
|
||||
ID: "task-ac31", WorkspaceID: "ws-title", WorkflowID: "wf-title", WorkflowStepID: "step-1",
|
||||
Title: "Original title", Description: "Original description", Priority: "medium",
|
||||
AssigneeAgentProfileID: "agent-x",
|
||||
}); err != nil {
|
||||
t.Fatalf("create task: %v", err)
|
||||
}
|
||||
bus.ClearEvents()
|
||||
|
||||
newTitle := "Edited title"
|
||||
if _, err := svc.UpdateTask(ctx, "task-ac31", &UpdateTaskRequest{Title: &newTitle}); err != nil {
|
||||
t.Fatalf("UpdateTask (title): %v", err)
|
||||
}
|
||||
titleData := singlePublishedEventData(t, bus)
|
||||
if _, ok := titleData["assignee_agent_profile_id"]; ok {
|
||||
t.Fatalf("title-only update published assignee_agent_profile_id = %#v, want the key absent",
|
||||
titleData["assignee_agent_profile_id"])
|
||||
}
|
||||
|
||||
bus.ClearEvents()
|
||||
newDescription := "Edited description"
|
||||
if _, err := svc.UpdateTask(ctx, "task-ac31", &UpdateTaskRequest{Description: &newDescription}); err != nil {
|
||||
t.Fatalf("UpdateTask (description): %v", err)
|
||||
}
|
||||
descData := singlePublishedEventData(t, bus)
|
||||
if _, ok := descData["assignee_agent_profile_id"]; ok {
|
||||
t.Fatalf("description-only update published assignee_agent_profile_id = %#v, want the key absent",
|
||||
descData["assignee_agent_profile_id"])
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,468 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, renderHook, waitFor } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { StateProvider, useAppStoreApi } from "@/components/state-provider";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
beginWrite,
|
||||
closeFieldEditor,
|
||||
endWrite,
|
||||
getCanonicalValue,
|
||||
isFieldGuarded,
|
||||
nextTaskSequence,
|
||||
openFieldEditor,
|
||||
recordRefetchCandidate,
|
||||
recordWriteSuccess,
|
||||
seedInitialCanonical,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
import type { OfficeTask } from "@/lib/state/slices/office/types";
|
||||
import type { Task } from "./types";
|
||||
|
||||
const mockGetTask = vi.fn();
|
||||
const mockListActivityForTarget = vi.fn();
|
||||
const mockListComments = vi.fn();
|
||||
const mockListTaskSessions = vi.fn();
|
||||
|
||||
vi.mock("@/lib/api/domains/office-api", () => ({
|
||||
getTask: (...args: unknown[]) => mockGetTask(...args),
|
||||
listActivityForTarget: (...args: unknown[]) => mockListActivityForTarget(...args),
|
||||
listComments: (...args: unknown[]) => mockListComments(...args),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/session-api", () => ({
|
||||
listTaskSessions: (...args: unknown[]) => mockListTaskSessions(...args),
|
||||
}));
|
||||
|
||||
import { useIssueData, useTaskOptimisticHelpers } from "./page";
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
const TS = "2026-05-01T00:00:00Z";
|
||||
const TASK_ID = "t-1";
|
||||
const TITLE_FIELD = "title";
|
||||
const EDITED_TITLE = "Edited While Loading";
|
||||
const ORIGINAL_TITLE = "Original Title";
|
||||
const ORIGINAL_DESCRIPTION = "Original description";
|
||||
|
||||
const officeTaskFixture: OfficeTask = {
|
||||
id: TASK_ID,
|
||||
workspaceId: "ws-1",
|
||||
identifier: "TASK-1",
|
||||
title: ORIGINAL_TITLE,
|
||||
description: ORIGINAL_DESCRIPTION,
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
createdAt: TS,
|
||||
updatedAt: TS,
|
||||
};
|
||||
|
||||
function makeStoreWrapper(initialTasks: OfficeTask[]) {
|
||||
function StoreSeed({ children }: { children: ReactNode }) {
|
||||
const api = useAppStoreApi();
|
||||
if (initialTasks.length > 0) {
|
||||
api.getState().setTasks(initialTasks);
|
||||
}
|
||||
return <>{children}</>;
|
||||
}
|
||||
function Wrapper({ children }: { children: ReactNode }) {
|
||||
return (
|
||||
<StateProvider>
|
||||
<StoreSeed>{children}</StoreSeed>
|
||||
</StateProvider>
|
||||
);
|
||||
}
|
||||
return Wrapper;
|
||||
}
|
||||
|
||||
function taskFixture(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: TASK_ID,
|
||||
workspaceId: "ws-1",
|
||||
identifier: "TASK-1",
|
||||
title: "Live Title",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
labels: [],
|
||||
blockedBy: [],
|
||||
blocking: [],
|
||||
children: [],
|
||||
reviewers: [],
|
||||
approvers: [],
|
||||
decisions: [],
|
||||
createdBy: "user",
|
||||
createdAt: TS,
|
||||
updatedAt: TS,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeTaskRefSetter(initialTask: Task = taskFixture()) {
|
||||
const taskRef: { current: Task | null } = { current: initialTask };
|
||||
const setTask: React.Dispatch<React.SetStateAction<Task | null>> = (updater) => {
|
||||
taskRef.current =
|
||||
typeof updater === "function"
|
||||
? (updater as (prev: Task | null) => Task | null)(taskRef.current)
|
||||
: updater;
|
||||
};
|
||||
return { taskRef, setTask };
|
||||
}
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((res) => {
|
||||
resolve = res;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
describe("useIssueData — initial load guard preservation (AC-27/28/38/39)", () => {
|
||||
it("does not overwrite title/description with a stale GET response that resolves after a mid-flight commit", async () => {
|
||||
mockListActivityForTarget.mockResolvedValue({ activity: [] });
|
||||
mockListComments.mockResolvedValue({ comments: [] });
|
||||
mockListTaskSessions.mockResolvedValue({ sessions: [] });
|
||||
const { promise: getTaskPromise, resolve: resolveGetTask } = deferred<{
|
||||
task: OfficeTask;
|
||||
timeline: [];
|
||||
}>();
|
||||
mockGetTask.mockReturnValue(getTaskPromise);
|
||||
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
const { result } = renderHook(() => useIssueData(TASK_ID), { wrapper: Wrapper });
|
||||
|
||||
// The from-store fast path seeds the displayed task synchronously.
|
||||
await waitFor(() => expect(result.current.task?.title).toBe(ORIGINAL_TITLE));
|
||||
|
||||
// Simulate a title commit landing in full (useCommitTaskTitle's actual
|
||||
// sequence: guard the field, apply optimistically, then record the
|
||||
// write's own success) while the initial GET is still in flight.
|
||||
const writeSequence = nextTaskSequence(TASK_ID);
|
||||
act(() => {
|
||||
beginWrite(TASK_ID, TITLE_FIELD, writeSequence);
|
||||
result.current.applyTaskPatch({ title: EDITED_TITLE });
|
||||
recordWriteSuccess(TASK_ID, TITLE_FIELD, EDITED_TITLE, "2026-05-01T00:00:05Z", writeSequence);
|
||||
endWrite(TASK_ID, TITLE_FIELD, writeSequence);
|
||||
});
|
||||
expect(result.current.task?.title).toBe(EDITED_TITLE);
|
||||
|
||||
// The GET resolves with the DTO as it looked before that commit landed
|
||||
// (an older title, a different status — the merge must only protect
|
||||
// title/description, not block the whole update).
|
||||
await act(async () => {
|
||||
resolveGetTask({
|
||||
task: { ...officeTaskFixture, title: "Server Title", status: "in_progress" },
|
||||
timeline: [],
|
||||
});
|
||||
await getTaskPromise;
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
await waitFor(() => expect(result.current.task?.status).toBe("in_progress"));
|
||||
expect(result.current.task?.title).toBe(EDITED_TITLE);
|
||||
expect(result.current.task?.description).toBe(ORIGINAL_DESCRIPTION);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useIssueData — canonical seeded synchronously from the store cache (COR-001)", () => {
|
||||
it("seeds canonical title/description as soon as the store-cache fast path renders, before the authoritative GET resolves", async () => {
|
||||
mockListActivityForTarget.mockResolvedValue({ activity: [] });
|
||||
mockListComments.mockResolvedValue({ comments: [] });
|
||||
mockListTaskSessions.mockResolvedValue({ sessions: [] });
|
||||
// The GET is left permanently pending for this test: the render gate
|
||||
// makes the page interactive from the store cache alone, and canonical
|
||||
// must already be seeded in that window, not only once this resolves.
|
||||
const { promise: getTaskPromise } = deferred<{ task: OfficeTask; timeline: [] }>();
|
||||
mockGetTask.mockReturnValue(getTaskPromise);
|
||||
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
const { result } = renderHook(() => useIssueData(TASK_ID), { wrapper: Wrapper });
|
||||
|
||||
await waitFor(() => expect(result.current.task?.title).toBe(ORIGINAL_TITLE));
|
||||
|
||||
// AC-9/AC-64: a title commit that fails during this window restores the
|
||||
// field's recorded canonical value. Without a canonical value seeded
|
||||
// here, that restore silently no-ops (getCanonicalValue returns
|
||||
// undefined) and the failed edit is left standing.
|
||||
expect(getCanonicalValue(TASK_ID, "title")).toBe(ORIGINAL_TITLE);
|
||||
expect(getCanonicalValue(TASK_ID, "description")).toBe(ORIGINAL_DESCRIPTION);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useIssueData — store-cache seed does not clobber an existing canonical value (P1 review fix)", () => {
|
||||
it("does not let the store-cache fast path's seed win an equal-timestamp tiebreak against an already-recorded canonical value", async () => {
|
||||
mockListActivityForTarget.mockResolvedValue({ activity: [] });
|
||||
mockListComments.mockResolvedValue({ comments: [] });
|
||||
mockListTaskSessions.mockResolvedValue({ sessions: [] });
|
||||
// Left pending: the seed-vs-clobber window under test is between the
|
||||
// store-cache fast path rendering and this GET resolving.
|
||||
const { promise: getTaskPromise } = deferred<{ task: OfficeTask; timeline: [] }>();
|
||||
mockGetTask.mockReturnValue(getTaskPromise);
|
||||
|
||||
// Simulates a canonical value already recorded on an earlier visit (e.g.
|
||||
// a write still pending when the user last navigated away), at the same
|
||||
// instant as the store-cached task seeded below.
|
||||
seedInitialCanonical(TASK_ID, "title", "Pending Write Title", TS);
|
||||
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
const { result } = renderHook(() => useIssueData(TASK_ID), { wrapper: Wrapper });
|
||||
|
||||
await waitFor(() => expect(result.current.task?.title).toBe(ORIGINAL_TITLE));
|
||||
|
||||
// Without the if-absent guard, the store-cache fast path's own seed call
|
||||
// (issued at a higher sequence, same instant as the pre-existing
|
||||
// canonical) would win the AC-63 tiebreak and clobber it.
|
||||
expect(getCanonicalValue(TASK_ID, "title")).toBe("Pending Write Title");
|
||||
});
|
||||
});
|
||||
|
||||
describe("useIssueData — initial load failure still seeds canonical (AC-59/AC-72)", () => {
|
||||
it("seeds canonical title/description from the store cache when the authoritative GET throws", async () => {
|
||||
mockListActivityForTarget.mockResolvedValue({ activity: [] });
|
||||
mockListComments.mockResolvedValue({ comments: [] });
|
||||
mockListTaskSessions.mockResolvedValue({ sessions: [] });
|
||||
mockGetTask.mockRejectedValue(new Error("network down"));
|
||||
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
const { result } = renderHook(() => useIssueData(TASK_ID), { wrapper: Wrapper });
|
||||
|
||||
// The from-store fast path renders the task interactively; the
|
||||
// authoritative GET then fails outright.
|
||||
await waitFor(() => expect(result.current.task?.title).toBe(ORIGINAL_TITLE));
|
||||
|
||||
// AC-9/AC-64: a later failed title commit restores the recorded
|
||||
// canonical value. If the authoritative load never completes, the page
|
||||
// still renders (and lets the user edit) the store-cached title, so a
|
||||
// canonical value must exist for that restore to have a target.
|
||||
await waitFor(() => expect(getCanonicalValue(TASK_ID, "title")).toBe(ORIGINAL_TITLE));
|
||||
expect(getCanonicalValue(TASK_ID, "description")).toBe(ORIGINAL_DESCRIPTION);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useIssueData — guard release does not spuriously patch an unset description (AC-71)", () => {
|
||||
it("leaves a no-description task's description undefined after a no-op description-editor open/close", async () => {
|
||||
mockListActivityForTarget.mockResolvedValue({ activity: [] });
|
||||
mockListComments.mockResolvedValue({ comments: [] });
|
||||
mockListTaskSessions.mockResolvedValue({ sessions: [] });
|
||||
const taskNoDescription: OfficeTask = { ...officeTaskFixture, description: undefined };
|
||||
mockGetTask.mockResolvedValue({ task: taskNoDescription, timeline: [] });
|
||||
|
||||
const Wrapper = makeStoreWrapper([taskNoDescription]);
|
||||
const { result } = renderHook(() => ({ issue: useIssueData(TASK_ID), api: useAppStoreApi() }), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
await waitFor(() => expect(result.current.issue.task?.title).toBe(ORIGINAL_TITLE));
|
||||
await waitFor(() => expect(result.current.issue.loading).toBe(false));
|
||||
|
||||
// No-op: a user opens the description editor to check whether there's a
|
||||
// description, then cancels without typing anything. The canonical value
|
||||
// is normalised to "" (AC-71), but the store/display never held a
|
||||
// description in the first place, so nothing should be written.
|
||||
act(() => {
|
||||
openFieldEditor(TASK_ID, "description");
|
||||
closeFieldEditor(TASK_ID, "description");
|
||||
});
|
||||
|
||||
expect(result.current.issue.task?.description).toBeUndefined();
|
||||
expect(
|
||||
result.current.api.getState().office.tasks.items.find((t) => t.id === TASK_ID)?.description,
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("useTaskOptimisticHelpers — unmount cleanup order (AC-68)", () => {
|
||||
it("flushes a canonical value recorded while guarded to the store on unmount", () => {
|
||||
openFieldEditor(TASK_ID, TITLE_FIELD);
|
||||
const sequence = nextTaskSequence(TASK_ID);
|
||||
recordRefetchCandidate(TASK_ID, TITLE_FIELD, "Flushed On Release", TS, sequence);
|
||||
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
// Mirrors the real invariant: by the time a listener could fire, the
|
||||
// page has already finished its initial load, so `task` is non-null.
|
||||
const { taskRef, setTask } = makeTaskRefSetter();
|
||||
const setTimeline = vi.fn();
|
||||
|
||||
const { result, unmount } = renderHook(
|
||||
() => ({
|
||||
helpers: useTaskOptimisticHelpers(TASK_ID, setTask, setTimeline),
|
||||
api: useAppStoreApi(),
|
||||
}),
|
||||
{ wrapper: Wrapper },
|
||||
);
|
||||
const storeApi = result.current.api;
|
||||
|
||||
unmount();
|
||||
|
||||
expect(storeApi.getState().office.tasks.items.find((t) => t.id === TASK_ID)?.title).toBe(
|
||||
"Flushed On Release",
|
||||
);
|
||||
expect(taskRef.current?.title).toBe("Flushed On Release");
|
||||
});
|
||||
|
||||
it("reconciles the store once a write still pending at unmount time resolves (P1 review fix)", () => {
|
||||
const sequence = nextTaskSequence(TASK_ID);
|
||||
beginWrite(TASK_ID, TITLE_FIELD, sequence);
|
||||
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
const { taskRef, setTask } = makeTaskRefSetter();
|
||||
const setTimeline = vi.fn();
|
||||
|
||||
const { result, unmount } = renderHook(
|
||||
() => ({
|
||||
helpers: useTaskOptimisticHelpers(TASK_ID, setTask, setTimeline),
|
||||
api: useAppStoreApi(),
|
||||
}),
|
||||
{ wrapper: Wrapper },
|
||||
);
|
||||
const storeApi = result.current.api;
|
||||
|
||||
unmount();
|
||||
|
||||
// The field is still guarded by the pending write, so the listener must
|
||||
// stay registered past unmount instead of unsubscribing unconditionally.
|
||||
expect(isFieldGuarded(TASK_ID, TITLE_FIELD)).toBe(true);
|
||||
|
||||
act(() => {
|
||||
recordWriteSuccess(
|
||||
TASK_ID,
|
||||
TITLE_FIELD,
|
||||
"Reconciled After Unmount",
|
||||
"2026-05-01T00:00:05Z",
|
||||
sequence,
|
||||
);
|
||||
endWrite(TASK_ID, TITLE_FIELD, sequence);
|
||||
});
|
||||
|
||||
expect(isFieldGuarded(TASK_ID, TITLE_FIELD)).toBe(false);
|
||||
expect(taskRef.current?.title).toBe("Reconciled After Unmount");
|
||||
expect(storeApi.getState().office.tasks.items.find((t) => t.id === TASK_ID)?.title).toBe(
|
||||
"Reconciled After Unmount",
|
||||
);
|
||||
});
|
||||
|
||||
it("never restores title/description when rolling back an unrelated picker mutation (AC-61)", () => {
|
||||
const Wrapper = makeStoreWrapper([officeTaskFixture]);
|
||||
const { taskRef, setTask } = makeTaskRefSetter();
|
||||
const setTimeline = vi.fn();
|
||||
|
||||
const { result } = renderHook(() => useTaskOptimisticHelpers(TASK_ID, setTask, setTimeline), {
|
||||
wrapper: Wrapper,
|
||||
});
|
||||
|
||||
// A stale snapshot from before a since-confirmed title edit landed.
|
||||
const staleSnapshot: Task = { ...taskRef.current!, title: "Stale Title", priority: "high" };
|
||||
act(() => {
|
||||
result.current.restoreTask(staleSnapshot);
|
||||
});
|
||||
|
||||
expect(taskRef.current?.title).toBe("Live Title");
|
||||
expect(taskRef.current?.priority).toBe("high");
|
||||
});
|
||||
});
|
||||
|
||||
describe("useTaskOptimisticHelpers — task id changes with pending writes", () => {
|
||||
function renderTaskHelpers() {
|
||||
const oldTask = taskFixture({ id: "t-1", title: "Old title", description: "Old description" });
|
||||
const newTask = taskFixture({ id: "t-2", title: "New title", description: "New description" });
|
||||
const { taskRef, setTask } = makeTaskRefSetter(oldTask);
|
||||
const setTimeline = vi.fn();
|
||||
const Wrapper = makeStoreWrapper([]);
|
||||
const rendered = renderHook(
|
||||
({ taskId }: { taskId: string }) => useTaskOptimisticHelpers(taskId, setTask, setTimeline),
|
||||
{ initialProps: { taskId: "t-1" }, wrapper: Wrapper },
|
||||
);
|
||||
return { ...rendered, oldTask, newTask, taskRef };
|
||||
}
|
||||
|
||||
it("does not apply an old task's deferred title to the replacement task", () => {
|
||||
const sequence = nextTaskSequence("t-1");
|
||||
beginWrite("t-1", TITLE_FIELD, sequence);
|
||||
const { rerender, unmount, newTask, taskRef } = renderTaskHelpers();
|
||||
|
||||
taskRef.current = newTask;
|
||||
rerender({ taskId: "t-2" });
|
||||
|
||||
act(() => {
|
||||
recordWriteSuccess(
|
||||
"t-1",
|
||||
TITLE_FIELD,
|
||||
"Old title from server",
|
||||
"2026-05-01T00:00:05Z",
|
||||
sequence,
|
||||
);
|
||||
endWrite("t-1", TITLE_FIELD, sequence);
|
||||
});
|
||||
|
||||
expect(taskRef.current).toEqual(newTask);
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("does not apply an old task's captured patch or restore to the replacement task", () => {
|
||||
const { result, rerender, unmount, newTask, oldTask, taskRef } = renderTaskHelpers();
|
||||
const oldApplyPatch = result.current.applyTaskPatch;
|
||||
const oldRestoreTask = result.current.restoreTask;
|
||||
|
||||
taskRef.current = newTask;
|
||||
rerender({ taskId: "t-2" });
|
||||
|
||||
act(() => {
|
||||
oldApplyPatch({ title: "Old optimistic title" });
|
||||
oldRestoreTask({ ...oldTask, title: "Old restored title", priority: "high" });
|
||||
});
|
||||
|
||||
expect(taskRef.current).toEqual(newTask);
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
|
||||
describe("useIssueData — initial load task identity", () => {
|
||||
it("does not preserve the previous task's guarded fields after a route change", async () => {
|
||||
mockListActivityForTarget.mockResolvedValue({ activity: [] });
|
||||
mockListComments.mockResolvedValue({ comments: [] });
|
||||
mockListTaskSessions.mockResolvedValue({ sessions: [] });
|
||||
const firstLoad = deferred<{ task: OfficeTask; timeline: [] }>();
|
||||
const secondLoad = deferred<{ task: OfficeTask; timeline: [] }>();
|
||||
mockGetTask.mockImplementation((taskId: string) =>
|
||||
taskId === "t-1" ? firstLoad.promise : secondLoad.promise,
|
||||
);
|
||||
|
||||
const Wrapper = makeStoreWrapper([
|
||||
{ ...officeTaskFixture, title: "First task", description: "First description" },
|
||||
]);
|
||||
const { result, rerender } = renderHook(
|
||||
({ taskId }: { taskId: string }) => useIssueData(taskId),
|
||||
{ initialProps: { taskId: "t-1" }, wrapper: Wrapper },
|
||||
);
|
||||
|
||||
act(() => {
|
||||
firstLoad.resolve({
|
||||
task: { ...officeTaskFixture, title: "First task", description: "First description" },
|
||||
timeline: [],
|
||||
});
|
||||
});
|
||||
await waitFor(() => expect(result.current.task?.id).toBe("t-1"));
|
||||
rerender({ taskId: "t-2" });
|
||||
openFieldEditor("t-2", TITLE_FIELD);
|
||||
await act(async () => {
|
||||
secondLoad.resolve({
|
||||
task: {
|
||||
...officeTaskFixture,
|
||||
id: "t-2",
|
||||
identifier: "TASK-2",
|
||||
title: "Second task",
|
||||
description: "Second description",
|
||||
},
|
||||
timeline: [],
|
||||
});
|
||||
await secondLoad.promise;
|
||||
});
|
||||
await waitFor(() => expect(result.current.task?.id).toBe("t-2"));
|
||||
|
||||
expect(result.current.task?.title).toBe("Second task");
|
||||
expect(result.current.task?.description).toBe("Second description");
|
||||
closeFieldEditor("t-2", TITLE_FIELD);
|
||||
});
|
||||
});
|
||||
@@ -12,7 +12,7 @@ import {
|
||||
type SetStateAction,
|
||||
} from "react";
|
||||
import { useRouter, useSearchParams } from "@/lib/routing/client-router";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { useOfficeRefetch } from "@/hooks/use-office-refetch";
|
||||
import { useLatestOnly } from "@/hooks/use-latest-only";
|
||||
import { TaskOptimisticContextProvider } from "@/hooks/use-optimistic-task-mutation";
|
||||
@@ -23,6 +23,15 @@ import {
|
||||
type TaskCommentResponse,
|
||||
} from "@/lib/api/domains/office-api";
|
||||
import { fetchTask } from "@/lib/api/domains/kanban-api";
|
||||
import {
|
||||
endAllEditorsForTask,
|
||||
getCanonicalValue,
|
||||
isFieldGuarded,
|
||||
nextTaskSequence,
|
||||
recordRefetchCandidate,
|
||||
seedInitialCanonical,
|
||||
subscribeField,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
import { listTaskSessions } from "@/lib/api/domains/session-api";
|
||||
import {
|
||||
liveSessionMetadataFromStore,
|
||||
@@ -237,9 +246,24 @@ function useSessionLiveSync({
|
||||
const taskId = task?.id;
|
||||
useEffect(() => {
|
||||
if (!taskId || !sessionStatesKey) return;
|
||||
// AC-52: the sequence marks when this refetch was *issued*, so a slow
|
||||
// response can still be recognized as stale against a write issued
|
||||
// after it but resolved before it.
|
||||
const sequence = nextTaskSequence(taskId);
|
||||
getTask(taskId)
|
||||
.then((res) => {
|
||||
if (res.task) onTaskRefetch(mapOfficeTaskToTask(res.task), res.timeline ?? []);
|
||||
if (res.task) {
|
||||
const mapped = mapOfficeTaskToTask(res.task);
|
||||
onTaskRefetch(mapped, res.timeline ?? []);
|
||||
recordRefetchCandidate(taskId, "title", mapped.title, mapped.updatedAt, sequence);
|
||||
recordRefetchCandidate(
|
||||
taskId,
|
||||
"description",
|
||||
mapped.description,
|
||||
mapped.updatedAt,
|
||||
sequence,
|
||||
);
|
||||
}
|
||||
})
|
||||
.catch(() => {});
|
||||
void onCommentsRefetch();
|
||||
@@ -252,15 +276,79 @@ function useSessionLiveSync({
|
||||
// Optimistic update helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function useTaskOptimisticHelpers(
|
||||
// Exported (in addition to being used internally by `useIssueData`) so unit
|
||||
// tests can drive the real guard-cleanup and restore behavior directly,
|
||||
// rather than only through a fully-rendered page.
|
||||
export function useTaskOptimisticHelpers(
|
||||
id: string,
|
||||
setTask: React.Dispatch<React.SetStateAction<Task | null>>,
|
||||
setTimeline: React.Dispatch<React.SetStateAction<TimelineEvent[]>>,
|
||||
) {
|
||||
const storeApi = useAppStoreApi();
|
||||
|
||||
const applyTaskPatch = useCallback(
|
||||
(patch: Partial<Task>) => {
|
||||
setTask((prev) => (prev && prev.id === id ? { ...prev, ...patch } : prev));
|
||||
},
|
||||
[id, setTask],
|
||||
);
|
||||
|
||||
// AC-27/49/50/71: apply a field's canonical value to the displayed task
|
||||
// (and the Office task store entry) the instant it becomes unguarded —
|
||||
// either because a candidate was recorded while already unguarded, or
|
||||
// because the last open editor / in-flight write for that field just
|
||||
// cleared. `notifyField` fires unconditionally on every guard release
|
||||
// (office-task-content-sync.ts), so these listeners own the "if the two
|
||||
// differ" comparison against what's currently displayed/stored (AC-71):
|
||||
// without it, a no-op editor open/close on a task with no description
|
||||
// would write "" over an `undefined` description, since the canonical
|
||||
// value is always normalised to "" but the store never held one.
|
||||
useEffect(() => {
|
||||
let unmounted = false;
|
||||
const unsubTitle = subscribeField(id, "title", (value) => {
|
||||
setTask((prev) =>
|
||||
prev && prev.id === id && prev.title !== value ? { ...prev, title: value } : prev,
|
||||
);
|
||||
const current = storeApi.getState().office.tasks.items.find((t) => t.id === id);
|
||||
if (current && current.title !== value) {
|
||||
storeApi.getState().patchTaskInStore(id, { title: value });
|
||||
}
|
||||
if (unmounted && !isFieldGuarded(id, "title")) unsubTitle();
|
||||
});
|
||||
const unsubDescription = subscribeField(id, "description", (value) => {
|
||||
setTask((prev) =>
|
||||
prev && prev.id === id && (prev.description ?? "") !== value
|
||||
? { ...prev, description: value }
|
||||
: prev,
|
||||
);
|
||||
const current = storeApi.getState().office.tasks.items.find((t) => t.id === id);
|
||||
if (current && (current.description ?? "") !== value) {
|
||||
storeApi.getState().patchTaskInStore(id, { description: value });
|
||||
}
|
||||
if (unmounted && !isFieldGuarded(id, "description")) unsubDescription();
|
||||
});
|
||||
return () => {
|
||||
// AC-68/69: ending the open-editor contribution can synchronously flush
|
||||
// a canonical value that was recorded while guarded (deferred-apply),
|
||||
// and that flush needs the listeners above still registered to reach
|
||||
// the Office task store. A write issued before unmount is deliberately
|
||||
// left pending by endAllEditorsForTask and must still be able to reach
|
||||
// the store once it resolves, so a field with a write still in flight
|
||||
// keeps its listener alive past this cleanup; each listener retires
|
||||
// itself once that field's guard finally clears.
|
||||
unmounted = true;
|
||||
endAllEditorsForTask(id);
|
||||
if (!isFieldGuarded(id, "title")) unsubTitle();
|
||||
if (!isFieldGuarded(id, "description")) unsubDescription();
|
||||
};
|
||||
}, [id, setTask, storeApi]);
|
||||
|
||||
// Refetch the canonical task DTO when the backend broadcasts an update
|
||||
// (priority / project / parent / blockers / participants / assignee).
|
||||
// The optimistic patch we applied locally gets reconciled with server
|
||||
// state.
|
||||
// state. Title/description are excluded from this merge — the
|
||||
// subscribeField effect above governs them so a guarded field's
|
||||
// optimistic/in-flight value survives an unrelated refetch (AC-38/39).
|
||||
//
|
||||
// A WS-driven trigger can fire again before a prior GET resolves (two
|
||||
// status changes in quick succession, ordinary network jitter). Without
|
||||
@@ -271,12 +359,19 @@ function useTaskOptimisticHelpers(
|
||||
const { begin, isCurrent } = useLatestOnly();
|
||||
const refetchTask = useCallback(async () => {
|
||||
const token = begin();
|
||||
// AC-52: the sequence marks when this refetch was *issued*.
|
||||
const sequence = nextTaskSequence(id);
|
||||
try {
|
||||
const [res, genericTask] = await Promise.all([getTask(id), fetchTask(id).catch(() => null)]);
|
||||
if (!isCurrent(token)) return;
|
||||
if (res.task) {
|
||||
setTask(mapOfficeTaskToTask(res.task, genericTask ?? undefined));
|
||||
const mapped = mapOfficeTaskToTask(res.task, genericTask ?? undefined);
|
||||
setTask((prev) =>
|
||||
prev ? { ...mapped, title: prev.title, description: prev.description } : mapped,
|
||||
);
|
||||
if (res.timeline) setTimeline(res.timeline);
|
||||
recordRefetchCandidate(id, "title", mapped.title, mapped.updatedAt, sequence);
|
||||
recordRefetchCandidate(id, "description", mapped.description, mapped.updatedAt, sequence);
|
||||
}
|
||||
} catch {
|
||||
/* swallow — next user action will retry */
|
||||
@@ -286,18 +381,22 @@ function useTaskOptimisticHelpers(
|
||||
void refetchTask();
|
||||
});
|
||||
|
||||
const applyTaskPatch = useCallback(
|
||||
(patch: Partial<Task>) => {
|
||||
setTask((prev) => (prev ? { ...prev, ...patch } : prev));
|
||||
},
|
||||
[setTask],
|
||||
);
|
||||
|
||||
// `restoreTask` backs `TaskOptimisticContextValue.restore`, the failure
|
||||
// rollback for the generic picker mutation hook (status/priority/project/
|
||||
// parent/labels/blockedBy/assignee). Title/description are excluded from
|
||||
// the restored snapshot and always kept at their live value — those two
|
||||
// fields are governed exclusively by the guard above, and this generic
|
||||
// rollback never patches them optimistically in the first place (AC-61:
|
||||
// only two writers may touch a guarded field, and this isn't one of them).
|
||||
const restoreTask = useCallback(
|
||||
(snapshot: Task) => {
|
||||
setTask(snapshot);
|
||||
setTask((prev) =>
|
||||
prev && prev.id === id
|
||||
? { ...snapshot, title: prev.title, description: prev.description }
|
||||
: prev,
|
||||
);
|
||||
},
|
||||
[setTask],
|
||||
[id, setTask],
|
||||
);
|
||||
|
||||
return { applyTaskPatch, restoreTask };
|
||||
@@ -318,6 +417,14 @@ function useTaskDetailRefetch(
|
||||
// loaded by the initial detail request until the next full load.
|
||||
statusSummary: updated.statusSummary ?? previous.statusSummary,
|
||||
repositories: updated.repositories ?? previous.repositories,
|
||||
// AC-27/28/38/39/49/50: title/description must not be blindly
|
||||
// overwritten by this whole-task refetch merge — they're
|
||||
// governed exclusively by the subscribeField-driven
|
||||
// deferred-apply effect in useTaskOptimisticHelpers, which
|
||||
// respects the open-editor/in-flight-write guard. Every other
|
||||
// field applies immediately.
|
||||
title: previous.title,
|
||||
description: previous.description,
|
||||
}
|
||||
: updated,
|
||||
);
|
||||
@@ -337,7 +444,48 @@ function useTaskDetailRefetch(
|
||||
// driven refetch in useTaskOptimisticHelpers handles canonical refresh after
|
||||
// a property mutation commits).
|
||||
// `errorKey` remains a catalog key so locale changes do not re-issue the load.
|
||||
function useIssueData(id: string) {
|
||||
|
||||
// AC-59: seeds each field's canonical value from a loaded task.
|
||||
function seedCanonicalTitleAndDescription(taskId: string, task: Task): void {
|
||||
seedInitialCanonical(taskId, "title", task.title, task.updatedAt);
|
||||
seedInitialCanonical(taskId, "description", task.description ?? "", task.updatedAt);
|
||||
}
|
||||
|
||||
// P1 review fix: the store-cache fast path can observe an unpersisted
|
||||
// optimistic value (a title/description write still in flight when the user
|
||||
// last navigated away). Seeding from it unconditionally would let that value
|
||||
// win an equal-timestamp tiebreak against the already-recorded canonical
|
||||
// value on a revisit before the authoritative GET resolves. Only seed a
|
||||
// field from the cache when nothing is recorded for it yet; once a canonical
|
||||
// value exists, only the authoritative GET response may supersede it.
|
||||
function seedCanonicalTitleAndDescriptionIfAbsent(taskId: string, task: Task): void {
|
||||
if (getCanonicalValue(taskId, "title") === undefined) {
|
||||
seedInitialCanonical(taskId, "title", task.title, task.updatedAt);
|
||||
}
|
||||
if (getCanonicalValue(taskId, "description") === undefined) {
|
||||
seedInitialCanonical(taskId, "description", task.description ?? "", task.updatedAt);
|
||||
}
|
||||
}
|
||||
|
||||
// AC-59/AC-72: if the authoritative GET fails but the page is already
|
||||
// rendering this task interactively from the store cache, seed canonical
|
||||
// from it — otherwise a commit issued before the load ever succeeds (or if
|
||||
// it never does) would have no restore target on failure (AC-9/AC-64). In
|
||||
// practice this is now redundant with the synchronous seed in `load()`
|
||||
// below, but is kept as a defensive fallback for the store-miss case.
|
||||
function handleInitialLoadFailure(
|
||||
taskId: string,
|
||||
fromStoreTask: Task | null,
|
||||
setErrorKey: (key: string | null) => void,
|
||||
): void {
|
||||
if (fromStoreTask) {
|
||||
seedCanonicalTitleAndDescription(taskId, fromStoreTask);
|
||||
} else {
|
||||
setErrorKey("office:failedToLoadTask");
|
||||
}
|
||||
}
|
||||
|
||||
export function useIssueData(id: string) {
|
||||
const storeIssues = useAppStore((s) => s.office.tasks.items);
|
||||
const setTaskSessionsForTask = useAppStore((s) => s.setTaskSessionsForTask);
|
||||
const storeIssuesRef = useRef(storeIssues);
|
||||
@@ -370,7 +518,16 @@ function useIssueData(id: string) {
|
||||
setLoading(true);
|
||||
setErrorKey(null);
|
||||
const fromStore = storeIssuesRef.current.find((i) => i.id === id);
|
||||
if (fromStore && !cancelled) setTask(mapOfficeTaskToTask(fromStore));
|
||||
const fromStoreTask = fromStore ? mapOfficeTaskToTask(fromStore) : null;
|
||||
if (fromStoreTask && !cancelled) {
|
||||
setTask(fromStoreTask);
|
||||
// AC-59/AC-9/AC-64: the page is interactive from here, so a commit
|
||||
// issued before the GET below resolves needs a canonical value to
|
||||
// restore to on failure. Seeded only if absent (see the function's
|
||||
// doc comment): the later GET-success seed below is what's allowed
|
||||
// to supersede an existing canonical value.
|
||||
seedCanonicalTitleAndDescriptionIfAbsent(id, fromStoreTask);
|
||||
}
|
||||
|
||||
try {
|
||||
const [res, genericTask] = await Promise.all([
|
||||
@@ -382,13 +539,24 @@ function useIssueData(id: string) {
|
||||
if (!fromStore) setErrorKey("office:taskNotFound");
|
||||
} else {
|
||||
const freshTask = mapOfficeTaskToTask(res.task, genericTask ?? undefined);
|
||||
setTask(freshTask);
|
||||
// AC-27/28/38/39: this GET can resolve after a title/description
|
||||
// commit has already landed (the page renders interactively from
|
||||
// `fromStore` before this request settles), so it must not blindly
|
||||
// overwrite those two fields — same merge pattern as `refetchTask`
|
||||
// and `onTaskRefetch` below.
|
||||
setTask((prev) =>
|
||||
prev && prev.id === id
|
||||
? { ...freshTask, title: prev.title, description: prev.description }
|
||||
: freshTask,
|
||||
);
|
||||
if (res.timeline) setTimeline(res.timeline);
|
||||
// AC-59: the initial load seeds each field's canonical value.
|
||||
seedCanonicalTitleAndDescription(id, freshTask);
|
||||
const detail = await fetchIssueDetailData(freshTask.workspaceId, id);
|
||||
if (!cancelled) applyDetail(detail);
|
||||
}
|
||||
} catch {
|
||||
if (!cancelled && !fromStore) setErrorKey("office:failedToLoadTask");
|
||||
if (!cancelled) handleInitialLoadFailure(id, fromStoreTask, setErrorKey);
|
||||
}
|
||||
|
||||
if (!cancelled) setLoading(false);
|
||||
|
||||
@@ -1,13 +1,30 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { TaskOptimisticContextProvider } from "@/hooks/use-optimistic-task-mutation";
|
||||
import type { Task, TaskSession } from "@/app/office/tasks/[id]/types";
|
||||
import {
|
||||
OfficeTopbarChromeProvider,
|
||||
useOfficeTopbarChrome,
|
||||
} from "@/app/office/components/office-topbar-context";
|
||||
|
||||
vi.mock("@/lib/api/domains/kanban-api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api/domains/kanban-api")>(
|
||||
"@/lib/api/domains/kanban-api",
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
updateTask: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const TITLE_HEADING_TESTID = "office-task-title";
|
||||
const TITLE_INPUT_TESTID = "office-task-title-input";
|
||||
const DESCRIPTION_READ_TESTID = "office-task-description";
|
||||
const DESCRIPTION_TEXTAREA_TESTID = "office-task-description-textarea";
|
||||
const SOME_DESCRIPTION = "Some description";
|
||||
|
||||
const { CHAT_EDITABLE, CHAT_READONLY, CHAT_READONLY_TEST_ID } = vi.hoisted(() => ({
|
||||
CHAT_EDITABLE: "editable",
|
||||
CHAT_READONLY: "readonly",
|
||||
@@ -85,8 +102,14 @@ vi.mock("./chat-activity-tabs", () => ({
|
||||
}));
|
||||
|
||||
import { OfficeSimplePane } from "./OfficeSimplePane";
|
||||
import { updateTask } from "@/lib/api/domains/kanban-api";
|
||||
|
||||
afterEach(() => cleanup());
|
||||
const mockedUpdateTask = vi.mocked(updateTask);
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
const baseTask: Task = {
|
||||
id: "task-1",
|
||||
@@ -136,6 +159,10 @@ const runningSession: TaskSession = {
|
||||
updatedAt: "2026-05-01T10:07:00Z",
|
||||
};
|
||||
|
||||
function optimisticContextFor(task: Task) {
|
||||
return { task, applyPatch: vi.fn(), restore: vi.fn() };
|
||||
}
|
||||
|
||||
function TopbarActions() {
|
||||
const chrome = useOfficeTopbarChrome();
|
||||
return <>{chrome?.actions}</>;
|
||||
@@ -145,8 +172,10 @@ function PaneHarness({ task, sessions }: { task: Task; sessions: TaskSession[] }
|
||||
return (
|
||||
<StateProvider>
|
||||
<OfficeTopbarChromeProvider>
|
||||
<OfficeSimplePane task={task} comments={[]} activity={[]} sessions={sessions} />
|
||||
<TopbarActions />
|
||||
<TaskOptimisticContextProvider value={optimisticContextFor(task)}>
|
||||
<OfficeSimplePane task={task} comments={[]} activity={[]} sessions={sessions} />
|
||||
<TopbarActions />
|
||||
</TaskOptimisticContextProvider>
|
||||
</OfficeTopbarChromeProvider>
|
||||
</StateProvider>
|
||||
);
|
||||
@@ -208,3 +237,73 @@ describe("OfficeSimplePane ExecutionIndicator wiring", () => {
|
||||
expect(screen.getByTestId("execution-indicator").textContent).toBe("in_progress");
|
||||
});
|
||||
});
|
||||
|
||||
describe("OfficeSimplePane editor composition (AC-35, AC-57)", () => {
|
||||
it("commits the title while a dirty description draft stays open with its draft intact (AC-35)", async () => {
|
||||
mockedUpdateTask.mockResolvedValueOnce({
|
||||
title: "New title",
|
||||
updated_at: "2026-05-02T00:00:00Z",
|
||||
} as never);
|
||||
const task = { ...baseTask, description: SOME_DESCRIPTION };
|
||||
renderPane(task, []);
|
||||
|
||||
fireEvent.click(screen.getByTestId(DESCRIPTION_READ_TESTID));
|
||||
const textarea = screen.getByTestId(DESCRIPTION_TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
fireEvent.change(textarea, { target: { value: "Dirty draft" } });
|
||||
|
||||
fireEvent.doubleClick(screen.getByTestId(TITLE_HEADING_TESTID));
|
||||
const titleInput = screen.getByTestId(TITLE_INPUT_TESTID);
|
||||
fireEvent.change(titleInput, { target: { value: "New title" } });
|
||||
await act(async () => {
|
||||
fireEvent.keyDown(titleInput, { key: "Enter" });
|
||||
});
|
||||
|
||||
expect(mockedUpdateTask).toHaveBeenCalledWith(task.id, { title: "New title" });
|
||||
expect(screen.queryByTestId(TITLE_INPUT_TESTID)).toBeNull();
|
||||
expect((screen.getByTestId(DESCRIPTION_TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe(
|
||||
"Dirty draft",
|
||||
);
|
||||
});
|
||||
|
||||
it("closes the title editor and discards its draft when the description editor opens (AC-57)", () => {
|
||||
const task = { ...baseTask, description: SOME_DESCRIPTION };
|
||||
renderPane(task, []);
|
||||
|
||||
fireEvent.doubleClick(screen.getByTestId(TITLE_HEADING_TESTID));
|
||||
const titleInput = screen.getByTestId(TITLE_INPUT_TESTID);
|
||||
fireEvent.change(titleInput, { target: { value: "Uncommitted draft" } });
|
||||
|
||||
// Opening the description editor moves focus off the title input.
|
||||
fireEvent.blur(titleInput);
|
||||
fireEvent.click(screen.getByTestId(DESCRIPTION_READ_TESTID));
|
||||
|
||||
expect(screen.queryByTestId(TITLE_INPUT_TESTID)).toBeNull();
|
||||
expect(screen.getByTestId(TITLE_HEADING_TESTID).textContent).toBe(task.title);
|
||||
expect(screen.getByTestId(DESCRIPTION_TEXTAREA_TESTID)).toBeTruthy();
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("OfficeSimplePane editors have no status/lifecycle/archived gate (AC-30, AC-47)", () => {
|
||||
it("still opens the title and description editors when the task is cancelled with a failed session", () => {
|
||||
const task = { ...baseTask, status: "cancelled" as const, description: SOME_DESCRIPTION };
|
||||
renderPane(task, [completedSession, failedSession]);
|
||||
|
||||
fireEvent.doubleClick(screen.getByTestId(TITLE_HEADING_TESTID));
|
||||
expect(screen.getByTestId(TITLE_INPUT_TESTID)).toBeTruthy();
|
||||
|
||||
fireEvent.click(screen.getByTestId(DESCRIPTION_READ_TESTID));
|
||||
expect(screen.getByTestId(DESCRIPTION_TEXTAREA_TESTID)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("still opens the title and description editors while a session is RUNNING on a done task", () => {
|
||||
const task = { ...baseTask, status: "done" as const, description: SOME_DESCRIPTION };
|
||||
renderPane(task, [runningSession]);
|
||||
|
||||
fireEvent.doubleClick(screen.getByTestId(TITLE_HEADING_TESTID));
|
||||
expect(screen.getByTestId(TITLE_INPUT_TESTID)).toBeTruthy();
|
||||
|
||||
fireEvent.click(screen.getByTestId(DESCRIPTION_READ_TESTID));
|
||||
expect(screen.getByTestId(DESCRIPTION_TEXTAREA_TESTID)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,6 +33,8 @@ import { copyToClipboard } from "@/lib/utils/copy-to-clipboard";
|
||||
import { NewTaskDialog } from "@/app/office/components/new-task-dialog";
|
||||
import { ActiveSessionRefProvider } from "./components/active-session-ref-context";
|
||||
import { TopbarWorkingIndicator } from "./components/topbar-working-indicator";
|
||||
import { TaskTitleEditable } from "./components/task-title-editable";
|
||||
import { TaskDescriptionEditable } from "./components/task-description-editable";
|
||||
import { TreeCancelDialog } from "@/components/task/TreeCancelDialog";
|
||||
import {
|
||||
cancelTaskTree,
|
||||
@@ -507,12 +509,12 @@ export function OfficeSimplePane({
|
||||
executionState={task.executionState}
|
||||
/>
|
||||
)}
|
||||
<h1 className="text-xl font-semibold mt-4">{task.title}</h1>
|
||||
{task.description && (
|
||||
<div className="prose prose-sm mt-4 max-w-none text-sm whitespace-pre-wrap">
|
||||
{task.description}
|
||||
</div>
|
||||
)}
|
||||
<TaskTitleEditable taskId={task.id} title={task.title} />
|
||||
<TaskDescriptionEditable
|
||||
taskId={task.id}
|
||||
description={task.description}
|
||||
sessions={sessions}
|
||||
/>
|
||||
<TaskActionRow
|
||||
task={task}
|
||||
treePreview={treePreview}
|
||||
|
||||
@@ -0,0 +1,510 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { TaskOptimisticContextProvider } from "@/hooks/use-optimistic-task-mutation";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
isFieldGuarded,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
import { TaskDescriptionEditable } from "./task-description-editable";
|
||||
import type { Task, TaskSession } from "@/app/office/tasks/[id]/types";
|
||||
|
||||
vi.mock("@/lib/api/domains/kanban-api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api/domains/kanban-api")>(
|
||||
"@/lib/api/domains/kanban-api",
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
updateTask: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
import { updateTask } from "@/lib/api/domains/kanban-api";
|
||||
|
||||
const mockedUpdateTask = vi.mocked(updateTask);
|
||||
|
||||
const ORIGINAL_DESCRIPTION = "Original description";
|
||||
const CHANGED_DESCRIPTION = "Changed description";
|
||||
const READ_TESTID = "office-task-description";
|
||||
const EMPTY_TESTID = "office-task-description-empty";
|
||||
const TEXTAREA_TESTID = "office-task-description-textarea";
|
||||
const SAVE_TESTID = "office-task-description-save";
|
||||
const CANCEL_TESTID = "office-task-description-cancel";
|
||||
const DISCARD_CONFIRM_TESTID = "office-task-description-discard-confirm";
|
||||
const RUNNING_NOTE_TESTID = "office-task-description-running-note";
|
||||
const UPDATED_AT = "2026-05-02T00:00:00Z";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
const task: Task = {
|
||||
id: "t-1",
|
||||
workspaceId: "ws-1",
|
||||
identifier: "TASK-1",
|
||||
title: "Task",
|
||||
description: ORIGINAL_DESCRIPTION,
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
labels: [],
|
||||
blockedBy: [],
|
||||
blocking: [],
|
||||
children: [],
|
||||
reviewers: [],
|
||||
approvers: [],
|
||||
decisions: [],
|
||||
createdBy: "user",
|
||||
createdAt: "2026-05-01T00:00:00Z",
|
||||
updatedAt: "2026-05-01T00:00:00Z",
|
||||
};
|
||||
|
||||
function Wrapper({ children }: { children: ReactNode }) {
|
||||
const ctx = { task, applyPatch: vi.fn(), restore: vi.fn() };
|
||||
return (
|
||||
<StateProvider>
|
||||
<TaskOptimisticContextProvider value={ctx}>{children}</TaskOptimisticContextProvider>
|
||||
</StateProvider>
|
||||
);
|
||||
}
|
||||
|
||||
function renderEditable(description: string | undefined, sessions: TaskSession[] = []) {
|
||||
return render(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description={description} sessions={sessions} />
|
||||
</Wrapper>,
|
||||
);
|
||||
}
|
||||
|
||||
function openEditor(description = ORIGINAL_DESCRIPTION) {
|
||||
renderEditable(description);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
return screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
}
|
||||
|
||||
function changeDraft(textarea: HTMLTextAreaElement, value: string) {
|
||||
fireEvent.change(textarea, { target: { value } });
|
||||
}
|
||||
|
||||
describe("TaskDescriptionEditable read view", () => {
|
||||
it("renders the empty-state placeholder when there is no description", () => {
|
||||
renderEditable(undefined);
|
||||
expect(screen.getByTestId(EMPTY_TESTID)).toBeTruthy();
|
||||
expect(screen.queryByTestId(READ_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("opens the editor with an empty draft when the empty-state placeholder is clicked (AC-13)", () => {
|
||||
renderEditable(undefined);
|
||||
fireEvent.click(screen.getByTestId(EMPTY_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
expect(textarea.value).toBe("");
|
||||
});
|
||||
|
||||
it("renders the description text and opens the editor on click, focused (AC-12)", () => {
|
||||
const textarea = openEditor();
|
||||
expect(textarea.value).toBe(ORIGINAL_DESCRIPTION);
|
||||
expect(document.activeElement).toBe(textarea);
|
||||
});
|
||||
|
||||
it("opens the editor when the description element has keyboard focus and Enter is pressed (AC-33)", () => {
|
||||
renderEditable(ORIGINAL_DESCRIPTION);
|
||||
fireEvent.keyDown(screen.getByTestId(READ_TESTID), { key: "Enter" });
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
expect(textarea.value).toBe(ORIGINAL_DESCRIPTION);
|
||||
});
|
||||
|
||||
it("renders Markdown syntax as literal plain text rather than formatted output (AC-46)", () => {
|
||||
const markdown = "# Heading\n\n**bold** and _italic_ and [a link](https://example.com)";
|
||||
renderEditable(markdown);
|
||||
const view = screen.getByTestId(READ_TESTID);
|
||||
expect(view.textContent).toBe(markdown);
|
||||
expect(view.querySelector("strong")).toBeNull();
|
||||
expect(view.querySelector("em")).toBeNull();
|
||||
expect(view.querySelector("a")).toBeNull();
|
||||
expect(view.querySelector("h1")).toBeNull();
|
||||
});
|
||||
|
||||
it("does not open the editor when clicking to select text", () => {
|
||||
renderEditable(ORIGINAL_DESCRIPTION);
|
||||
const view = screen.getByTestId(READ_TESTID);
|
||||
const getSelectionSpy = vi.spyOn(window, "getSelection").mockReturnValue({
|
||||
toString: () => "some selected text",
|
||||
} as Selection);
|
||||
fireEvent.click(view);
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
getSelectionSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable save", () => {
|
||||
it("Save is disabled until the draft differs from the baseline", () => {
|
||||
const textarea = openEditor();
|
||||
const save = screen.getByTestId(SAVE_TESTID) as HTMLButtonElement;
|
||||
expect(save.disabled).toBe(true);
|
||||
changeDraft(textarea, CHANGED_DESCRIPTION);
|
||||
expect(save.disabled).toBe(false);
|
||||
});
|
||||
|
||||
it("commits a trimmed draft on Save and closes the editor", async () => {
|
||||
mockedUpdateTask.mockResolvedValue({
|
||||
description: CHANGED_DESCRIPTION,
|
||||
updated_at: UPDATED_AT,
|
||||
} as never);
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, ` ${CHANGED_DESCRIPTION} `);
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
});
|
||||
expect(mockedUpdateTask).toHaveBeenCalledWith("t-1", { description: CHANGED_DESCRIPTION });
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("sends and persists a long description unmodified apart from whitespace trimming (AC-45)", async () => {
|
||||
const longDescription = "Lorem ipsum dolor sit amet. ".repeat(400).trim();
|
||||
mockedUpdateTask.mockResolvedValue({
|
||||
description: longDescription,
|
||||
updated_at: UPDATED_AT,
|
||||
} as never);
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, ` ${longDescription} `);
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
});
|
||||
expect(mockedUpdateTask).toHaveBeenCalledWith("t-1", { description: longDescription });
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable clearing to empty (AC-24)", () => {
|
||||
it("clears the stored description and renders the empty-state placeholder after saving an empty draft", async () => {
|
||||
mockedUpdateTask.mockResolvedValue({
|
||||
description: "",
|
||||
updated_at: UPDATED_AT,
|
||||
} as never);
|
||||
const { rerender } = render(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description={ORIGINAL_DESCRIPTION} sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
changeDraft(textarea, " ");
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
});
|
||||
expect(mockedUpdateTask).toHaveBeenCalledWith("t-1", { description: "" });
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
|
||||
// The parent re-renders with the newly cleared description once the
|
||||
// store/canonical value converges (simulated here via rerender).
|
||||
rerender(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description="" sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
expect(screen.getByTestId(EMPTY_TESTID)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable whitespace-only draft on an empty description (AC-43)", () => {
|
||||
it("treats a whitespace-only draft as equal to an empty baseline, keeping Save disabled and sending no request", () => {
|
||||
renderEditable("");
|
||||
fireEvent.click(screen.getByTestId(EMPTY_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
changeDraft(textarea, " ");
|
||||
expect((screen.getByTestId(SAVE_TESTID) as HTMLButtonElement).disabled).toBe(true);
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable in-flight save state (AC-41)", () => {
|
||||
it("keeps the textarea editable, disables Save and Cancel, and takes no action on Escape while saving", async () => {
|
||||
let resolveUpdate: (value: never) => void = () => {};
|
||||
mockedUpdateTask.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveUpdate = resolve;
|
||||
}),
|
||||
);
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, CHANGED_DESCRIPTION);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
// Yield one microtask so the `isSaving` state commits without waiting
|
||||
// for the still-pending mocked update to resolve.
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(textarea.disabled).toBe(false);
|
||||
expect((screen.getByTestId(SAVE_TESTID) as HTMLButtonElement).disabled).toBe(true);
|
||||
expect((screen.getByTestId(CANCEL_TESTID) as HTMLButtonElement).disabled).toBe(true);
|
||||
|
||||
fireEvent.keyDown(textarea, { key: "Escape" });
|
||||
expect(screen.getByTestId(TEXTAREA_TESTID)).toBeTruthy();
|
||||
|
||||
await act(async () => {
|
||||
resolveUpdate({
|
||||
description: CHANGED_DESCRIPTION,
|
||||
updated_at: UPDATED_AT,
|
||||
} as never);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable draft changed mid-save (AC-42)", () => {
|
||||
it("keeps the editor open with the newer draft, rebases the baseline to the saved value, and re-enables Save", async () => {
|
||||
let resolveUpdate: (value: never) => void = () => {};
|
||||
mockedUpdateTask.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveUpdate = resolve;
|
||||
}),
|
||||
);
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, "First draft");
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
// The user keeps typing while the save for "First draft" is in flight.
|
||||
changeDraft(textarea, "Second draft");
|
||||
|
||||
await act(async () => {
|
||||
resolveUpdate({ description: "First draft", updated_at: UPDATED_AT } as never);
|
||||
});
|
||||
|
||||
expect(screen.getByTestId(TEXTAREA_TESTID)).toBeTruthy();
|
||||
expect((screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe("Second draft");
|
||||
expect((screen.getByTestId(SAVE_TESTID) as HTMLButtonElement).disabled).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable blur (AC-21)", () => {
|
||||
it("keeps the editor open, keeps the draft, and sends no request when the textarea loses focus", () => {
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, "Changed but not saved");
|
||||
fireEvent.blur(textarea);
|
||||
expect(screen.getByTestId(TEXTAREA_TESTID)).toBeTruthy();
|
||||
expect((screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe(
|
||||
"Changed but not saved",
|
||||
);
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable escape and cancel", () => {
|
||||
it("closes the editor on Escape when the draft is clean", () => {
|
||||
const textarea = openEditor();
|
||||
fireEvent.keyDown(textarea, { key: "Escape" });
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("does nothing on Escape when the draft is dirty", () => {
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, "Changed");
|
||||
fireEvent.keyDown(textarea, { key: "Escape" });
|
||||
expect(screen.getByTestId(TEXTAREA_TESTID)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("closes immediately on Cancel when the draft is clean", () => {
|
||||
openEditor();
|
||||
fireEvent.click(screen.getByTestId(CANCEL_TESTID));
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable discard confirmation", () => {
|
||||
it("shows a discard-confirm dialog on Cancel when dirty, and discards on confirm", () => {
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, "Changed");
|
||||
fireEvent.click(screen.getByTestId(CANCEL_TESTID));
|
||||
expect(screen.getByTestId(DISCARD_CONFIRM_TESTID)).toBeTruthy();
|
||||
fireEvent.click(screen.getByRole("button", { name: "Discard" }));
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps the draft when discard is dismissed via 'keep editing'", () => {
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, "Changed");
|
||||
fireEvent.click(screen.getByTestId(CANCEL_TESTID));
|
||||
fireEvent.click(screen.getByRole("button", { name: "Keep editing" }));
|
||||
expect((screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe("Changed");
|
||||
});
|
||||
|
||||
it("keeps the draft when discard is dismissed via Escape rather than the explicit discard choice (AC-36)", () => {
|
||||
const textarea = openEditor();
|
||||
changeDraft(textarea, "Changed");
|
||||
fireEvent.click(screen.getByTestId(CANCEL_TESTID));
|
||||
expect(screen.getByTestId(DISCARD_CONFIRM_TESTID)).toBeTruthy();
|
||||
fireEvent.keyDown(screen.getByTestId(DISCARD_CONFIRM_TESTID), { key: "Escape" });
|
||||
expect(screen.queryByTestId(DISCARD_CONFIRM_TESTID)).toBeNull();
|
||||
expect((screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe("Changed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable frozen baseline (AC-40)", () => {
|
||||
it("keeps Save disabled and Escape closing when a refetch changes the stored description while open", () => {
|
||||
const { rerender } = render(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description={ORIGINAL_DESCRIPTION} sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
|
||||
// A refetch changes the stored description while the editor is open.
|
||||
rerender(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description="Refetched description" sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
// The draft still equals the frozen (original) baseline, not the
|
||||
// refetched value, so Save stays disabled...
|
||||
const save = screen.getByTestId(SAVE_TESTID) as HTMLButtonElement;
|
||||
expect(save.disabled).toBe(true);
|
||||
|
||||
// ...and Escape closes immediately rather than being treated as dirty.
|
||||
fireEvent.keyDown(textarea, { key: "Escape" });
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable unmount cleanup (AC-68, COR-002)", () => {
|
||||
it("releases the field's guard when the component unmounts while the editor is still open", () => {
|
||||
const { unmount } = renderEditable(ORIGINAL_DESCRIPTION);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
expect(isFieldGuarded("t-1", "description")).toBe(true);
|
||||
|
||||
unmount();
|
||||
|
||||
expect(isFieldGuarded("t-1", "description")).toBe(false);
|
||||
});
|
||||
|
||||
it("is a no-op on unmount when the editor is already closed", () => {
|
||||
const { unmount } = renderEditable(ORIGINAL_DESCRIPTION);
|
||||
expect(isFieldGuarded("t-1", "description")).toBe(false);
|
||||
|
||||
unmount();
|
||||
|
||||
expect(isFieldGuarded("t-1", "description")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable taskId change while editing (P1 review fix, COR-002)", () => {
|
||||
it("closes the editor and releases the old task's guard when taskId changes while open", () => {
|
||||
const { rerender } = render(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description={ORIGINAL_DESCRIPTION} sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
changeDraft(textarea, "Changed");
|
||||
expect(isFieldGuarded("t-1", "description")).toBe(true);
|
||||
|
||||
// TaskBody swaps simple/advanced panes on a URL search param, which can
|
||||
// reuse this component instance for a different taskId without
|
||||
// unmounting it (no key={taskId} upstream). Description never closes on
|
||||
// blur (AC-21), so this is the only safety net for a still-open editor.
|
||||
rerender(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-2" description="Other task description" sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
expect(isFieldGuarded("t-1", "description")).toBe(false);
|
||||
expect(isFieldGuarded("t-2", "description")).toBe(false);
|
||||
expect(screen.queryByTestId(TEXTAREA_TESTID)).toBeNull();
|
||||
expect(screen.getByTestId(READ_TESTID).textContent).toBe("Other task description");
|
||||
});
|
||||
|
||||
it("ignores an old save result after the replacement task editor opens", async () => {
|
||||
let resolveUpdate: (value: never) => void = () => {};
|
||||
mockedUpdateTask.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveUpdate = resolve;
|
||||
}),
|
||||
);
|
||||
const { rerender } = render(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-1" description={ORIGINAL_DESCRIPTION} sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
changeDraft(textarea, CHANGED_DESCRIPTION);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(SAVE_TESTID));
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
rerender(
|
||||
<Wrapper>
|
||||
<TaskDescriptionEditable taskId="t-2" description="Other task description" sessions={[]} />
|
||||
</Wrapper>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
expect((screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe(
|
||||
"Other task description",
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
resolveUpdate({ description: CHANGED_DESCRIPTION, updated_at: UPDATED_AT } as never);
|
||||
});
|
||||
|
||||
expect((screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement).value).toBe(
|
||||
"Other task description",
|
||||
);
|
||||
expect((screen.getByTestId(SAVE_TESTID) as HTMLButtonElement).disabled).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskDescriptionEditable running-session note", () => {
|
||||
it("shows the note only while a session is RUNNING", () => {
|
||||
const sessions: TaskSession[] = [
|
||||
{
|
||||
id: "s-1",
|
||||
agentName: "agent",
|
||||
agentRole: "engineer",
|
||||
state: "RUNNING",
|
||||
isPrimary: true,
|
||||
},
|
||||
];
|
||||
renderEditable(ORIGINAL_DESCRIPTION, sessions);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
expect(screen.getByTestId(RUNNING_NOTE_TESTID)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does not show the note when no session is RUNNING", () => {
|
||||
renderEditable(ORIGINAL_DESCRIPTION, []);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
expect(screen.queryByTestId(RUNNING_NOTE_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps Save enabled while a session is RUNNING and the draft is dirty (AC-25)", () => {
|
||||
const sessions: TaskSession[] = [
|
||||
{
|
||||
id: "s-1",
|
||||
agentName: "agent",
|
||||
agentRole: "engineer",
|
||||
state: "RUNNING",
|
||||
isPrimary: true,
|
||||
},
|
||||
];
|
||||
renderEditable(ORIGINAL_DESCRIPTION, sessions);
|
||||
fireEvent.click(screen.getByTestId(READ_TESTID));
|
||||
const textarea = screen.getByTestId(TEXTAREA_TESTID) as HTMLTextAreaElement;
|
||||
changeDraft(textarea, CHANGED_DESCRIPTION);
|
||||
expect(screen.getByTestId(RUNNING_NOTE_TESTID)).toBeTruthy();
|
||||
expect((screen.getByTestId(SAVE_TESTID) as HTMLButtonElement).disabled).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,321 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import type { KeyboardEvent, MouseEvent } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { Textarea } from "@kandev/ui/textarea";
|
||||
import {
|
||||
AlertDialog,
|
||||
AlertDialogAction,
|
||||
AlertDialogCancel,
|
||||
AlertDialogContent,
|
||||
AlertDialogDescription,
|
||||
AlertDialogFooter,
|
||||
AlertDialogHeader,
|
||||
AlertDialogTitle,
|
||||
} from "@kandev/ui/alert-dialog";
|
||||
import { useCommitTaskDescription } from "@/hooks/use-optimistic-task-mutation";
|
||||
import { closeFieldEditor, openFieldEditor } from "@/lib/state/office-task-content-sync";
|
||||
import type { TaskSession } from "@/app/office/tasks/[id]/types";
|
||||
|
||||
type TaskDescriptionEditableProps = {
|
||||
taskId: string;
|
||||
description: string | undefined;
|
||||
sessions: TaskSession[];
|
||||
};
|
||||
|
||||
type DescriptionReadViewProps = {
|
||||
description: string | undefined;
|
||||
onActivate: () => void;
|
||||
};
|
||||
|
||||
function DescriptionReadView({ description, onActivate }: DescriptionReadViewProps) {
|
||||
const { t } = useTranslation();
|
||||
const trimmedDescription = (description ?? "").trim();
|
||||
|
||||
const handleKeyDown = useCallback(
|
||||
(e: KeyboardEvent<HTMLDivElement>) => {
|
||||
if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
onActivate();
|
||||
}
|
||||
},
|
||||
[onActivate],
|
||||
);
|
||||
|
||||
const handleClick = useCallback(() => {
|
||||
const selection = window.getSelection();
|
||||
if (selection && selection.toString().length > 0) return; // AC-34
|
||||
onActivate();
|
||||
}, [onActivate]);
|
||||
|
||||
const stopSelectionMouseDown = useCallback((e: MouseEvent) => {
|
||||
// Allow text selection inside the read view without triggering a click
|
||||
// activation once the mouseup fires with a non-empty selection.
|
||||
e.stopPropagation();
|
||||
}, []);
|
||||
|
||||
if (!trimmedDescription) {
|
||||
return (
|
||||
<div
|
||||
data-testid="office-task-description-empty"
|
||||
tabIndex={0}
|
||||
role="button"
|
||||
className="prose prose-sm mt-4 max-w-none text-sm text-muted-foreground cursor-text rounded-sm outline-none focus-visible:ring-[2px] focus-visible:ring-ring/35"
|
||||
onClick={onActivate}
|
||||
onKeyDown={handleKeyDown}
|
||||
>
|
||||
{t("task:addDescription")}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
data-testid="office-task-description"
|
||||
tabIndex={0}
|
||||
role="button"
|
||||
className="prose prose-sm mt-4 max-w-none text-sm whitespace-pre-wrap cursor-text rounded-sm outline-none focus-visible:ring-[2px] focus-visible:ring-ring/35"
|
||||
onMouseDown={stopSelectionMouseDown}
|
||||
onClick={handleClick}
|
||||
onKeyDown={handleKeyDown}
|
||||
>
|
||||
{description}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
type DescriptionDiscardDialogProps = {
|
||||
open: boolean;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
onConfirm: () => void;
|
||||
};
|
||||
|
||||
function DescriptionDiscardDialog({
|
||||
open,
|
||||
onOpenChange,
|
||||
onConfirm,
|
||||
}: DescriptionDiscardDialogProps) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<AlertDialog open={open} onOpenChange={onOpenChange}>
|
||||
<AlertDialogContent data-testid="office-task-description-discard-confirm">
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>{t("task:discardDescriptionTitle")}</AlertDialogTitle>
|
||||
<AlertDialogDescription>{t("task:discardDescriptionBody")}</AlertDialogDescription>
|
||||
</AlertDialogHeader>
|
||||
<AlertDialogFooter>
|
||||
<AlertDialogCancel className="cursor-pointer">{t("task:keepEditing")}</AlertDialogCancel>
|
||||
<AlertDialogAction className="cursor-pointer" onClick={onConfirm}>
|
||||
{t("task:discard")}
|
||||
</AlertDialogAction>
|
||||
</AlertDialogFooter>
|
||||
</AlertDialogContent>
|
||||
</AlertDialog>
|
||||
);
|
||||
}
|
||||
|
||||
type DescriptionEditViewProps = {
|
||||
draft: string;
|
||||
isDirty: boolean;
|
||||
isSaving: boolean;
|
||||
hasRunningSession: boolean;
|
||||
showDiscardConfirm: boolean;
|
||||
onChange: (value: string) => void;
|
||||
onKeyDown: (e: KeyboardEvent<HTMLTextAreaElement>) => void;
|
||||
onSave: () => void;
|
||||
onCancel: () => void;
|
||||
onDiscardOpenChange: (open: boolean) => void;
|
||||
onDiscardConfirm: () => void;
|
||||
};
|
||||
|
||||
function DescriptionEditView({
|
||||
draft,
|
||||
isDirty,
|
||||
isSaving,
|
||||
hasRunningSession,
|
||||
showDiscardConfirm,
|
||||
onChange,
|
||||
onKeyDown,
|
||||
onSave,
|
||||
onCancel,
|
||||
onDiscardOpenChange,
|
||||
onDiscardConfirm,
|
||||
}: DescriptionEditViewProps) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<div className="mt-4 space-y-2">
|
||||
<Textarea
|
||||
data-testid="office-task-description-textarea"
|
||||
aria-label={t("task:description")}
|
||||
autoFocus
|
||||
rows={8}
|
||||
value={draft}
|
||||
onChange={(e) => onChange(e.target.value)}
|
||||
onKeyDown={onKeyDown}
|
||||
/>
|
||||
{hasRunningSession && (
|
||||
<p
|
||||
data-testid="office-task-description-running-note"
|
||||
className="text-xs text-muted-foreground"
|
||||
>
|
||||
{t("task:descriptionRunningNote")}
|
||||
</p>
|
||||
)}
|
||||
<div className="flex gap-2">
|
||||
<Button
|
||||
data-testid="office-task-description-save"
|
||||
size="sm"
|
||||
className="cursor-pointer"
|
||||
disabled={!isDirty || isSaving}
|
||||
onClick={onSave}
|
||||
>
|
||||
{t("common:save")}
|
||||
</Button>
|
||||
<Button
|
||||
data-testid="office-task-description-cancel"
|
||||
size="sm"
|
||||
variant="outline"
|
||||
className="cursor-pointer"
|
||||
disabled={isSaving}
|
||||
onClick={onCancel}
|
||||
>
|
||||
{t("common:cancel")}
|
||||
</Button>
|
||||
</div>
|
||||
<DescriptionDiscardDialog
|
||||
open={showDiscardConfirm}
|
||||
onOpenChange={onDiscardOpenChange}
|
||||
onConfirm={onDiscardConfirm}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Click-to-edit description (docs/specs/office-task-content-editing/spec.md
|
||||
* §C). No blur-cancel (AC-21): only Escape (when clean), Save, or Cancel
|
||||
* leave the editor. Escape/Cancel with a dirty draft confirms discard first.
|
||||
*/
|
||||
export function TaskDescriptionEditable({
|
||||
taskId,
|
||||
description,
|
||||
sessions,
|
||||
}: TaskDescriptionEditableProps) {
|
||||
const commitDescription = useCommitTaskDescription();
|
||||
const [isEditing, setIsEditing] = useState(false);
|
||||
const [draft, setDraft] = useState("");
|
||||
// Frozen at editor-open time, trimmed (AC-51): dirty/discard comparisons
|
||||
// never re-derive from a live canonical/displayed value while guarded.
|
||||
const [baseline, setBaseline] = useState("");
|
||||
const [isSaving, setIsSaving] = useState(false);
|
||||
const [showDiscardConfirm, setShowDiscardConfirm] = useState(false);
|
||||
// Mirrors `draft` so the async Save handler can read the *live* draft
|
||||
// after its `await` without closing over a stale value typed during the
|
||||
// in-flight request (AC-17 vs AC-42).
|
||||
const draftRef = useRef("");
|
||||
// Invalidated when taskId changes so an old request cannot update the
|
||||
// replacement task's editor state.
|
||||
const saveGenerationRef = useRef(0);
|
||||
|
||||
const isDirty = draft.trim() !== baseline;
|
||||
const hasRunningSession = sessions.some((s) => s.state === "RUNNING");
|
||||
|
||||
// AC-68/COR-002: TaskBody swaps simple/advanced panes on a URL search
|
||||
// param rather than the route id, which unmounts this component without
|
||||
// running any id-keyed cleanup elsewhere. Description never closes on blur
|
||||
// (AC-21), so an open editor at toggle time has no other safety net.
|
||||
// closeFieldEditor is a no-op if the guard was already released via
|
||||
// save/cancel/discard. This component instance can also be reused for a
|
||||
// different taskId without unmounting (no key={taskId} upstream); resetting
|
||||
// the local editor state closes a still-open editor so the previous task's
|
||||
// draft never lingers over the new task.
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
saveGenerationRef.current += 1;
|
||||
closeFieldEditor(taskId, "description");
|
||||
setIsEditing(false);
|
||||
setIsSaving(false);
|
||||
setShowDiscardConfirm(false);
|
||||
};
|
||||
}, [taskId]);
|
||||
|
||||
const closeEditor = useCallback(() => {
|
||||
closeFieldEditor(taskId, "description");
|
||||
setIsEditing(false);
|
||||
setShowDiscardConfirm(false);
|
||||
}, [taskId]);
|
||||
|
||||
const activate = useCallback(() => {
|
||||
const value = description ?? "";
|
||||
setBaseline(value.trim());
|
||||
setDraft(value);
|
||||
draftRef.current = value;
|
||||
setIsEditing(true);
|
||||
openFieldEditor(taskId, "description");
|
||||
}, [description, taskId]);
|
||||
|
||||
const handleChange = useCallback((value: string) => {
|
||||
setDraft(value);
|
||||
draftRef.current = value;
|
||||
}, []);
|
||||
|
||||
const handleCancel = useCallback(() => {
|
||||
if (isSaving) return;
|
||||
if (isDirty) {
|
||||
setShowDiscardConfirm(true);
|
||||
return;
|
||||
}
|
||||
closeEditor();
|
||||
}, [closeEditor, isDirty, isSaving]);
|
||||
|
||||
const handleTextareaKeyDown = useCallback(
|
||||
(e: KeyboardEvent<HTMLTextAreaElement>) => {
|
||||
if (e.key !== "Escape") return;
|
||||
if (isSaving) return;
|
||||
if (isDirty) return; // AC-22: Escape does nothing while dirty
|
||||
e.preventDefault();
|
||||
closeEditor(); // AC-23
|
||||
},
|
||||
[closeEditor, isDirty, isSaving],
|
||||
);
|
||||
|
||||
const handleSave = useCallback(async () => {
|
||||
if (isSaving || !isDirty) return;
|
||||
setIsSaving(true);
|
||||
const trimmed = draft.trim();
|
||||
const saveGeneration = saveGenerationRef.current;
|
||||
const result = await commitDescription(taskId, trimmed);
|
||||
if (saveGeneration !== saveGenerationRef.current) return;
|
||||
setIsSaving(false);
|
||||
if (!result.ok) return; // stays open, draft intact (AC-18)
|
||||
if (draftRef.current.trim() === trimmed) {
|
||||
closeEditor(); // AC-16
|
||||
} else {
|
||||
setBaseline(result.value.trim()); // AC-42: draft changed mid-save
|
||||
}
|
||||
}, [closeEditor, commitDescription, draft, isDirty, isSaving, taskId]);
|
||||
|
||||
if (!isEditing) {
|
||||
return <DescriptionReadView description={description} onActivate={activate} />;
|
||||
}
|
||||
|
||||
return (
|
||||
<DescriptionEditView
|
||||
draft={draft}
|
||||
isDirty={isDirty}
|
||||
isSaving={isSaving}
|
||||
hasRunningSession={hasRunningSession}
|
||||
showDiscardConfirm={showDiscardConfirm}
|
||||
onChange={handleChange}
|
||||
onKeyDown={handleTextareaKeyDown}
|
||||
onSave={handleSave}
|
||||
onCancel={handleCancel}
|
||||
onDiscardOpenChange={(open) => {
|
||||
if (!open) setShowDiscardConfirm(false);
|
||||
}}
|
||||
onDiscardConfirm={closeEditor}
|
||||
/>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,278 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { TaskOptimisticContextProvider } from "@/hooks/use-optimistic-task-mutation";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
isFieldGuarded,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
import { TaskTitleEditable } from "./task-title-editable";
|
||||
import type { Task } from "@/app/office/tasks/[id]/types";
|
||||
|
||||
vi.mock("@/lib/api/domains/kanban-api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api/domains/kanban-api")>(
|
||||
"@/lib/api/domains/kanban-api",
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
updateTask: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
import { updateTask } from "@/lib/api/domains/kanban-api";
|
||||
|
||||
const mockedUpdateTask = vi.mocked(updateTask);
|
||||
|
||||
const ORIGINAL_TITLE = "Original title";
|
||||
const HEADING_TESTID = "office-task-title";
|
||||
const INPUT_TESTID = "office-task-title-input";
|
||||
const ERROR_TESTID = "office-task-title-error";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
const task: Task = {
|
||||
id: "t-1",
|
||||
workspaceId: "ws-1",
|
||||
identifier: "TASK-1",
|
||||
title: ORIGINAL_TITLE,
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
labels: [],
|
||||
blockedBy: [],
|
||||
blocking: [],
|
||||
children: [],
|
||||
reviewers: [],
|
||||
approvers: [],
|
||||
decisions: [],
|
||||
createdBy: "user",
|
||||
createdAt: "2026-05-01T00:00:00Z",
|
||||
updatedAt: "2026-05-01T00:00:00Z",
|
||||
};
|
||||
|
||||
function Wrapper({ children }: { children: ReactNode }) {
|
||||
const ctx = { task, applyPatch: vi.fn(), restore: vi.fn() };
|
||||
return (
|
||||
<StateProvider>
|
||||
<TaskOptimisticContextProvider value={ctx}>{children}</TaskOptimisticContextProvider>
|
||||
</StateProvider>
|
||||
);
|
||||
}
|
||||
|
||||
function renderEditable() {
|
||||
return render(
|
||||
<Wrapper>
|
||||
<TaskTitleEditable taskId="t-1" title={ORIGINAL_TITLE} />
|
||||
</Wrapper>,
|
||||
);
|
||||
}
|
||||
|
||||
function openEditor() {
|
||||
fireEvent.doubleClick(screen.getByTestId(HEADING_TESTID));
|
||||
return screen.getByTestId(INPUT_TESTID);
|
||||
}
|
||||
|
||||
describe("TaskTitleEditable read/activate", () => {
|
||||
it("renders the title as a heading and enters edit mode on double-click, focused with its content selected (AC-1)", () => {
|
||||
renderEditable();
|
||||
const heading = screen.getByTestId(HEADING_TESTID);
|
||||
expect(heading.textContent).toBe(ORIGINAL_TITLE);
|
||||
fireEvent.doubleClick(heading);
|
||||
const input = screen.getByTestId(INPUT_TESTID) as HTMLInputElement;
|
||||
expect(input.value).toBe(ORIGINAL_TITLE);
|
||||
fireEvent.focus(input);
|
||||
expect(document.activeElement).toBe(input);
|
||||
expect(input.selectionStart).toBe(0);
|
||||
expect(input.selectionEnd).toBe(ORIGINAL_TITLE.length);
|
||||
});
|
||||
|
||||
it("enters edit mode on Enter when the heading is focused, seeded with the current title (AC-2)", () => {
|
||||
renderEditable();
|
||||
fireEvent.keyDown(screen.getByTestId(HEADING_TESTID), { key: "Enter" });
|
||||
const input = screen.getByTestId(INPUT_TESTID) as HTMLInputElement;
|
||||
expect(input.value).toBe(ORIGINAL_TITLE);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskTitleEditable commit", () => {
|
||||
it("commits a trimmed, changed title on Enter", async () => {
|
||||
mockedUpdateTask.mockResolvedValue({
|
||||
title: "New title",
|
||||
updated_at: "2026-05-02T00:00:00Z",
|
||||
} as never);
|
||||
renderEditable();
|
||||
const input = openEditor();
|
||||
fireEvent.change(input, { target: { value: " New title " } });
|
||||
await act(async () => {
|
||||
fireEvent.keyDown(input, { key: "Enter" });
|
||||
});
|
||||
expect(mockedUpdateTask).toHaveBeenCalledWith("t-1", { title: "New title" });
|
||||
expect(screen.queryByTestId(INPUT_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("does not commit and closes without a request when Enter matches the baseline", async () => {
|
||||
renderEditable();
|
||||
const input = openEditor();
|
||||
fireEvent.change(input, { target: { value: ORIGINAL_TITLE } });
|
||||
await act(async () => {
|
||||
fireEvent.keyDown(input, { key: "Enter" });
|
||||
});
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
expect(screen.queryByTestId(INPUT_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("shows an error and stays open when the draft is emptied, clearing once fixed (AC-37)", () => {
|
||||
renderEditable();
|
||||
const input = openEditor() as HTMLInputElement;
|
||||
fireEvent.change(input, { target: { value: " " } });
|
||||
expect(input.getAttribute("aria-invalid")).toBe("true");
|
||||
fireEvent.keyDown(input, { key: "Enter" });
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
expect(screen.getByTestId(ERROR_TESTID)).toBeTruthy();
|
||||
expect(screen.getByTestId(INPUT_TESTID)).toBeTruthy();
|
||||
|
||||
fireEvent.change(input, { target: { value: "Fixed title" } });
|
||||
expect(screen.queryByTestId(ERROR_TESTID)).toBeNull();
|
||||
expect(input.getAttribute("aria-invalid")).toBe("false");
|
||||
});
|
||||
|
||||
it("ignores Enter during IME composition", () => {
|
||||
renderEditable();
|
||||
const input = openEditor();
|
||||
fireEvent.change(input, { target: { value: "New title" } });
|
||||
fireEvent.keyDown(input, { key: "Enter", keyCode: 229 });
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
expect(screen.getByTestId(INPUT_TESTID)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskTitleEditable frozen baseline (AC-51)", () => {
|
||||
it("keeps the frozen baseline when a newer canonical title arrives while the editor is open", () => {
|
||||
const { rerender } = render(
|
||||
<Wrapper>
|
||||
<TaskTitleEditable taskId="t-1" title={ORIGINAL_TITLE} />
|
||||
</Wrapper>,
|
||||
);
|
||||
const input = openEditor();
|
||||
|
||||
// A canonical update lands (e.g. via the deferred-apply store patch)
|
||||
// while the editor is still open, changing the prop the parent passes.
|
||||
rerender(
|
||||
<Wrapper>
|
||||
<TaskTitleEditable taskId="t-1" title="Canonical update" />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
// Typing the ORIGINAL (frozen) baseline back in and pressing Enter must
|
||||
// be a no-op close, not a commit compared against the new canonical
|
||||
// value — proving the baseline never re-derived from the live prop.
|
||||
fireEvent.change(input, { target: { value: ORIGINAL_TITLE } });
|
||||
fireEvent.keyDown(input, { key: "Enter" });
|
||||
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
expect(screen.queryByTestId(INPUT_TESTID)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskTitleEditable cancel", () => {
|
||||
it("closes the editor without committing on Escape", () => {
|
||||
renderEditable();
|
||||
const input = openEditor();
|
||||
fireEvent.change(input, { target: { value: "Changed but not saved" } });
|
||||
fireEvent.keyDown(input, { key: "Escape" });
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
expect(screen.queryByTestId(INPUT_TESTID)).toBeNull();
|
||||
});
|
||||
|
||||
it("closes the editor without committing on blur", () => {
|
||||
renderEditable();
|
||||
const input = openEditor();
|
||||
fireEvent.change(input, { target: { value: "Changed but not saved" } });
|
||||
fireEvent.blur(input);
|
||||
expect(mockedUpdateTask).not.toHaveBeenCalled();
|
||||
expect(screen.queryByTestId(INPUT_TESTID)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskTitleEditable clamp (AC-6)", () => {
|
||||
it("retains only the first 60 code points when typed or pasted text would exceed the cap", () => {
|
||||
renderEditable();
|
||||
const input = openEditor() as HTMLInputElement;
|
||||
// fireEvent.change dispatches the same DOM event a paste produces, so
|
||||
// this covers both entry paths named in AC-6.
|
||||
fireEvent.change(input, { target: { value: "x".repeat(90) } });
|
||||
expect(input.value).toHaveLength(60);
|
||||
expect(input.value).toBe("x".repeat(60));
|
||||
});
|
||||
|
||||
it("counts by code point, not UTF-16 code unit, so a surrogate-pair character is not split", () => {
|
||||
renderEditable();
|
||||
const input = openEditor() as HTMLInputElement;
|
||||
// Each emoji below is one code point (two UTF-16 code units): 65 emoji is
|
||||
// 65 code points, so the clamp must keep the first 60 emoji intact
|
||||
// rather than cutting mid-surrogate-pair at 60 code units.
|
||||
const emoji = "😀".repeat(65);
|
||||
fireEvent.change(input, { target: { value: emoji } });
|
||||
expect(Array.from(input.value)).toHaveLength(60);
|
||||
expect(input.value).toBe("😀".repeat(60));
|
||||
});
|
||||
|
||||
it("does not clamp text at or under the 60 code point cap", () => {
|
||||
renderEditable();
|
||||
const input = openEditor() as HTMLInputElement;
|
||||
const atCap = "x".repeat(60);
|
||||
fireEvent.change(input, { target: { value: atCap } });
|
||||
expect(input.value).toBe(atCap);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskTitleEditable unmount cleanup (AC-68, COR-002)", () => {
|
||||
it("releases the field's guard when the component unmounts while the editor is still open", () => {
|
||||
const { unmount } = renderEditable();
|
||||
openEditor();
|
||||
expect(isFieldGuarded("t-1", "title")).toBe(true);
|
||||
|
||||
unmount();
|
||||
|
||||
expect(isFieldGuarded("t-1", "title")).toBe(false);
|
||||
});
|
||||
|
||||
it("is a no-op on unmount when the editor is already closed", () => {
|
||||
const { unmount } = renderEditable();
|
||||
expect(isFieldGuarded("t-1", "title")).toBe(false);
|
||||
|
||||
unmount();
|
||||
|
||||
expect(isFieldGuarded("t-1", "title")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskTitleEditable taskId change while editing (P1 review fix, COR-002)", () => {
|
||||
it("closes the editor and releases the old task's guard when taskId changes while open", () => {
|
||||
const { rerender } = render(
|
||||
<Wrapper>
|
||||
<TaskTitleEditable taskId="t-1" title={ORIGINAL_TITLE} />
|
||||
</Wrapper>,
|
||||
);
|
||||
openEditor();
|
||||
expect(isFieldGuarded("t-1", "title")).toBe(true);
|
||||
|
||||
// TaskBody swaps simple/advanced panes on a URL search param, which can
|
||||
// reuse this component instance for a different taskId without
|
||||
// unmounting it (no key={taskId} upstream).
|
||||
rerender(
|
||||
<Wrapper>
|
||||
<TaskTitleEditable taskId="t-2" title="Other task title" />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
expect(isFieldGuarded("t-1", "title")).toBe(false);
|
||||
expect(isFieldGuarded("t-2", "title")).toBe(false);
|
||||
expect(screen.queryByTestId(INPUT_TESTID)).toBeNull();
|
||||
expect(screen.getByTestId(HEADING_TESTID).textContent).toBe("Other task title");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,144 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import type { KeyboardEvent } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { Input } from "@kandev/ui/input";
|
||||
import { useCommitTaskTitle } from "@/hooks/use-optimistic-task-mutation";
|
||||
import { useTaskTitleSelectionRestore } from "@/hooks/use-task-title-selection-restore";
|
||||
import { closeFieldEditor, openFieldEditor } from "@/lib/state/office-task-content-sync";
|
||||
|
||||
type TaskTitleEditableProps = {
|
||||
taskId: string;
|
||||
title: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Inline title editor (docs/specs/office-task-content-editing/spec.md §B).
|
||||
* Double-click or Enter-on-focus opens a single-line input; Enter commits,
|
||||
* Escape/blur cancel. No archived-task or status gate (AC-30, AC-47).
|
||||
*/
|
||||
export function TaskTitleEditable({ taskId, title }: TaskTitleEditableProps) {
|
||||
const { t } = useTranslation();
|
||||
const commitTitle = useCommitTaskTitle();
|
||||
const [isEditing, setIsEditing] = useState(false);
|
||||
const [draft, setDraft] = useState("");
|
||||
// Frozen at editor-open time (AC-51): the equality test in AC-8 must never
|
||||
// compare against a canonical value that arrives while the field is
|
||||
// guarded, even though nothing else can normally change `title` while
|
||||
// guarded — an unrelated in-flight write's failure restore can (see the
|
||||
// task plan's R5 addendum), so a live read of `title` is not safe here.
|
||||
const [baseline, setBaseline] = useState("");
|
||||
const { inputRef, clampChange } = useTaskTitleSelectionRestore(draft);
|
||||
|
||||
// AC-68/COR-002: TaskBody swaps simple/advanced panes on a URL search
|
||||
// param rather than the route id, which unmounts this component without
|
||||
// running any id-keyed cleanup elsewhere. closeFieldEditor is a no-op if
|
||||
// the guard was already released via commit/cancel/blur. This component
|
||||
// instance can also be reused for a different taskId without unmounting
|
||||
// (no key={taskId} upstream); setIsEditing(false) closes a still-open
|
||||
// editor so the previous task's draft never lingers over the new task.
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
closeFieldEditor(taskId, "title");
|
||||
setIsEditing(false);
|
||||
};
|
||||
}, [taskId]);
|
||||
|
||||
const closeEditor = useCallback(() => {
|
||||
closeFieldEditor(taskId, "title");
|
||||
setIsEditing(false);
|
||||
}, [taskId]);
|
||||
|
||||
const activate = useCallback(() => {
|
||||
setBaseline(title);
|
||||
setDraft(title);
|
||||
setIsEditing(true);
|
||||
openFieldEditor(taskId, "title");
|
||||
}, [taskId, title]);
|
||||
|
||||
const handleHeadingKeyDown = useCallback(
|
||||
(e: KeyboardEvent<HTMLHeadingElement>) => {
|
||||
if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
activate();
|
||||
}
|
||||
},
|
||||
[activate],
|
||||
);
|
||||
|
||||
const commit = useCallback(
|
||||
(trimmed: string) => {
|
||||
// Begin the write's guard contribution before releasing the editor's,
|
||||
// so the field is never observably unguarded between the two (R5-F3
|
||||
// ordering pattern, applied here to avoid a needless intermediate
|
||||
// deferred-apply notification).
|
||||
void commitTitle(taskId, trimmed);
|
||||
closeFieldEditor(taskId, "title");
|
||||
setIsEditing(false);
|
||||
},
|
||||
[commitTitle, taskId],
|
||||
);
|
||||
|
||||
const handleInputKeyDown = useCallback(
|
||||
(e: KeyboardEvent<HTMLInputElement>) => {
|
||||
// IME composition uses Enter to accept a candidate (AC-10).
|
||||
if (e.nativeEvent.isComposing || e.nativeEvent.keyCode === 229) return;
|
||||
if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
const trimmed = draft.trim();
|
||||
if (!trimmed) return; // AC-7: invalid, stays open, no request
|
||||
if (trimmed === baseline) {
|
||||
closeEditor(); // AC-8
|
||||
return;
|
||||
}
|
||||
commit(trimmed); // AC-3
|
||||
} else if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
closeEditor(); // AC-4
|
||||
}
|
||||
},
|
||||
[baseline, closeEditor, commit, draft],
|
||||
);
|
||||
|
||||
if (!isEditing) {
|
||||
return (
|
||||
<h1
|
||||
data-testid="office-task-title"
|
||||
tabIndex={0}
|
||||
className="text-xl font-semibold mt-4 rounded-sm outline-none focus-visible:ring-[2px] focus-visible:ring-ring/35"
|
||||
onDoubleClick={activate}
|
||||
onKeyDown={handleHeadingKeyDown}
|
||||
>
|
||||
{title}
|
||||
</h1>
|
||||
);
|
||||
}
|
||||
|
||||
const trimmedDraft = draft.trim();
|
||||
const isInvalid = trimmedDraft.length === 0;
|
||||
|
||||
return (
|
||||
<div className="mt-4">
|
||||
<Input
|
||||
ref={inputRef}
|
||||
data-testid="office-task-title-input"
|
||||
aria-label={t("task:taskTitle")}
|
||||
aria-invalid={isInvalid}
|
||||
autoFocus
|
||||
value={draft}
|
||||
onChange={(e) => setDraft(clampChange(e))}
|
||||
onFocus={(e) => e.target.select()}
|
||||
onBlur={closeEditor}
|
||||
onKeyDown={handleInputKeyDown}
|
||||
className="text-xl font-semibold h-auto py-1"
|
||||
/>
|
||||
{isInvalid && (
|
||||
<p data-testid="office-task-title-error" className="mt-1 text-xs text-destructive">
|
||||
{t("task:titleRequired")}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,220 @@
|
||||
import type { Browser, BrowserContext, Page } from "@playwright/test";
|
||||
import { test, expect } from "../../fixtures/office-fixture";
|
||||
import { watchWs } from "../../helpers/causal-waits";
|
||||
|
||||
/**
|
||||
* E2E coverage for in-place title and description editing on the Office task
|
||||
* detail page (docs/specs/office-task-content-editing/spec.md).
|
||||
*
|
||||
* Title commits are optimistic (apply + close immediately, roll back on
|
||||
* failure); description saves are not optimistic (editor stays open in a
|
||||
* saving state, and a failure leaves the draft untouched). See
|
||||
* hooks/use-optimistic-task-mutation.ts for both commit paths.
|
||||
*/
|
||||
|
||||
async function gotoTaskPage(testPage: Page, taskId: string, title: string) {
|
||||
await testPage.goto(`/office/tasks/${taskId}`);
|
||||
await expect(testPage.getByRole("heading", { name: title })).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Opens an independent client against the same backend: its own context means
|
||||
* its own store, WebSocket and localStorage, which is what makes it a
|
||||
* meaningful stand-in for a second browser tab observing the same task.
|
||||
*/
|
||||
async function openSecondClient(
|
||||
browser: Browser,
|
||||
frontendUrl: string,
|
||||
backendPort: number,
|
||||
): Promise<{ page: Page; context: BrowserContext }> {
|
||||
const context = await browser.newContext({ baseURL: frontendUrl });
|
||||
const page = await context.newPage();
|
||||
await page.addInitScript(
|
||||
({ port }: { port: number }) => {
|
||||
localStorage.setItem("kandev.onboarding.completed", "true");
|
||||
window.__KANDEV_API_PORT = String(port);
|
||||
window.__KANDEV_E2E_EXPOSE_STORE__ = true;
|
||||
},
|
||||
{ port: backendPort },
|
||||
);
|
||||
return { page, context };
|
||||
}
|
||||
|
||||
async function mockTaskPatchFailure(testPage: Page, taskId: string) {
|
||||
await testPage.route(
|
||||
(url) =>
|
||||
url.pathname === `/api/v1/tasks/${taskId}` || url.pathname.endsWith(`/tasks/${taskId}`),
|
||||
async (route) => {
|
||||
if (route.request().method() === "PATCH") {
|
||||
await route.fulfill({
|
||||
status: 500,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ error: "forced failure" }),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
test.describe("task title and description editing", () => {
|
||||
test("editing the title renders the trimmed value and persists it", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const task = await apiClient.createTask(officeSeed.workspaceId, "Original title task", {
|
||||
workflow_id: officeSeed.workflowId,
|
||||
});
|
||||
await gotoTaskPage(testPage, task.id, "Original title task");
|
||||
|
||||
await testPage.getByTestId("office-task-title").dblclick();
|
||||
const input = testPage.getByTestId("office-task-title-input");
|
||||
await input.fill(" Renamed via inline edit ");
|
||||
await input.press("Enter");
|
||||
|
||||
// AC-3: renders the trimmed draft immediately and closes the editor.
|
||||
await expect(testPage.getByTestId("office-task-title-input")).toHaveCount(0);
|
||||
await expect(testPage.getByTestId("office-task-title")).toHaveText("Renamed via inline edit");
|
||||
|
||||
// AC-26: persisted, and only the title field changed.
|
||||
const persisted = (await apiClient.getTask(task.id)) as unknown as Record<string, unknown>;
|
||||
expect(persisted.title).toBe("Renamed via inline edit");
|
||||
});
|
||||
|
||||
test("editing the description saves it, closes the editor, and persists it", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const task = await apiClient.createTask(officeSeed.workspaceId, "Description edit task", {
|
||||
workflow_id: officeSeed.workflowId,
|
||||
description: "Original description",
|
||||
});
|
||||
await gotoTaskPage(testPage, task.id, "Description edit task");
|
||||
|
||||
await testPage.getByTestId("office-task-description").click();
|
||||
const textarea = testPage.getByTestId("office-task-description-textarea");
|
||||
await expect(textarea).toHaveValue("Original description");
|
||||
await textarea.fill(" Updated description body ");
|
||||
await testPage.getByTestId("office-task-description-save").click();
|
||||
|
||||
// AC-16 + AC-17: save request fires, editor closes, saved value renders.
|
||||
await expect(testPage.getByTestId("office-task-description-textarea")).toHaveCount(0, {
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(testPage.getByTestId("office-task-description")).toHaveText(
|
||||
"Updated description body",
|
||||
);
|
||||
|
||||
// AC-26: persisted, and only the description field changed.
|
||||
const persisted = (await apiClient.getTask(task.id)) as unknown as Record<string, unknown>;
|
||||
expect(persisted.description).toBe("Updated description body");
|
||||
});
|
||||
|
||||
test("a second browser context observes title and description edits without a reload", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
officeSeed,
|
||||
backend,
|
||||
browser,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const task = await apiClient.createTask(officeSeed.workspaceId, "Cross-client edit task", {
|
||||
workflow_id: officeSeed.workflowId,
|
||||
description: "Starting description",
|
||||
});
|
||||
|
||||
await gotoTaskPage(testPage, task.id, "Cross-client edit task");
|
||||
|
||||
const second = await openSecondClient(browser, backend.frontendUrl, backend.port);
|
||||
const secondClientWs = watchWs(second.page);
|
||||
await gotoTaskPage(second.page, task.id, "Cross-client edit task");
|
||||
|
||||
// Device A edits the title; device B must converge without reloading.
|
||||
const titleUpdate = secondClientWs.waitForEvent("office.task.updated");
|
||||
await testPage.getByTestId("office-task-title").dblclick();
|
||||
const titleInput = testPage.getByTestId("office-task-title-input");
|
||||
await titleInput.fill("Renamed on device A");
|
||||
await titleInput.press("Enter");
|
||||
await expect(testPage.getByTestId("office-task-title")).toHaveText("Renamed on device A");
|
||||
|
||||
// AC-27: the other client picks up the change from office.task.updated.
|
||||
await titleUpdate;
|
||||
await expect(second.page.getByTestId("office-task-title")).toHaveText("Renamed on device A");
|
||||
|
||||
// Device A edits the description; device B must converge without reloading.
|
||||
const descriptionUpdate = secondClientWs.waitForEvent("office.task.updated");
|
||||
await testPage.getByTestId("office-task-description").click();
|
||||
const descriptionTextarea = testPage.getByTestId("office-task-description-textarea");
|
||||
await descriptionTextarea.fill("Updated on device A");
|
||||
await testPage.getByTestId("office-task-description-save").click();
|
||||
await expect(testPage.getByTestId("office-task-description-textarea")).toHaveCount(0, {
|
||||
timeout: 10_000,
|
||||
});
|
||||
|
||||
await descriptionUpdate;
|
||||
await expect(second.page.getByTestId("office-task-description")).toHaveText(
|
||||
"Updated on device A",
|
||||
);
|
||||
|
||||
await second.context.close();
|
||||
});
|
||||
|
||||
test("a failed title save rolls back to the previous title and toasts", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const task = await apiClient.createTask(officeSeed.workspaceId, "Rollback title task", {
|
||||
workflow_id: officeSeed.workflowId,
|
||||
});
|
||||
await gotoTaskPage(testPage, task.id, "Rollback title task");
|
||||
await mockTaskPatchFailure(testPage, task.id);
|
||||
|
||||
await testPage.getByTestId("office-task-title").dblclick();
|
||||
const input = testPage.getByTestId("office-task-title-input");
|
||||
await input.fill("Title that will fail to save");
|
||||
await input.press("Enter");
|
||||
|
||||
// AC-9: the optimistic render is reverted once the PATCH fails, and the
|
||||
// stored title is what's restored (the "last word standing" case).
|
||||
await expect(async () => {
|
||||
const heading = testPage.getByTestId("office-task-title");
|
||||
await expect(heading).toHaveText("Rollback title task");
|
||||
}).toPass({ timeout: 5_000 });
|
||||
|
||||
const persisted = (await apiClient.getTask(task.id)) as unknown as Record<string, unknown>;
|
||||
expect(persisted.title).toBe("Rollback title task");
|
||||
});
|
||||
|
||||
test("a failed description save keeps the editor open with the draft intact", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const task = await apiClient.createTask(officeSeed.workspaceId, "Rollback description task", {
|
||||
workflow_id: officeSeed.workflowId,
|
||||
description: "Original description",
|
||||
});
|
||||
await gotoTaskPage(testPage, task.id, "Rollback description task");
|
||||
await mockTaskPatchFailure(testPage, task.id);
|
||||
|
||||
await testPage.getByTestId("office-task-description").click();
|
||||
const textarea = testPage.getByTestId("office-task-description-textarea");
|
||||
await textarea.fill("Draft that fails to save");
|
||||
await testPage.getByTestId("office-task-description-save").click();
|
||||
|
||||
// AC-18: editor stays open, draft is untouched, nothing was persisted.
|
||||
await expect(testPage.getByTestId("office-task-description-textarea")).toBeVisible();
|
||||
await expect(testPage.getByTestId("office-task-description-textarea")).toHaveValue(
|
||||
"Draft that fails to save",
|
||||
);
|
||||
|
||||
const persisted = (await apiClient.getTask(task.id)) as unknown as Record<string, unknown>;
|
||||
expect(persisted.description).toBe("Original description");
|
||||
});
|
||||
});
|
||||
@@ -12,7 +12,7 @@ vi.mock("@/lib/api/domains/office-extended-api", () => ({
|
||||
|
||||
const setTaskQuorum = vi.fn();
|
||||
let byTaskId: Record<string, unknown> = {};
|
||||
let refetchTriggers: Record<string, number> = {};
|
||||
let refetchTrigger: { types: string[]; timestamp: number } | null = null;
|
||||
const WORKSPACE_ID = "workspace-1";
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
@@ -20,7 +20,7 @@ vi.mock("@/components/state-provider", () => ({
|
||||
sel({
|
||||
office: {
|
||||
taskQuorum: { byTaskId },
|
||||
refetchTriggers,
|
||||
refetchTrigger,
|
||||
},
|
||||
setTaskQuorum,
|
||||
}),
|
||||
@@ -32,7 +32,7 @@ describe("useTaskQuorum", () => {
|
||||
mocks.getTaskQuorum.mockReset();
|
||||
mocks.getTaskQuorum.mockResolvedValue({ guards: [], reevaluation_blocked: false });
|
||||
byTaskId = {};
|
||||
refetchTriggers = {};
|
||||
refetchTrigger = null;
|
||||
});
|
||||
|
||||
it("fetches once on mount for a given task", async () => {
|
||||
@@ -69,7 +69,7 @@ describe("useTaskQuorum", () => {
|
||||
await waitFor(() => expect(mocks.getTaskQuorum).toHaveBeenCalledTimes(1));
|
||||
|
||||
rerender({ taskId: null });
|
||||
refetchTriggers = { "task:task-1": 1 };
|
||||
refetchTrigger = { types: ["task:task-1"], timestamp: 1 };
|
||||
rerender({ taskId: "task-1" });
|
||||
await waitFor(() => expect(mocks.getTaskQuorum).toHaveBeenCalledTimes(2));
|
||||
});
|
||||
@@ -78,13 +78,13 @@ describe("useTaskQuorum", () => {
|
||||
const { rerender } = renderHook(() => useTaskQuorum("task-1", WORKSPACE_ID));
|
||||
await waitFor(() => expect(mocks.getTaskQuorum).toHaveBeenCalledTimes(1));
|
||||
|
||||
refetchTriggers = { "task:task-1": 1 };
|
||||
refetchTrigger = { types: ["task:task-1"], timestamp: 1 };
|
||||
rerender();
|
||||
await waitFor(() => expect(mocks.getTaskQuorum).toHaveBeenCalledTimes(2));
|
||||
});
|
||||
|
||||
it("does not refetch on mount just because the trigger map already has an unrelated entry", async () => {
|
||||
refetchTriggers = { "task:task-other": 3 };
|
||||
it("does not refetch on mount just because the trigger already has an unrelated type", async () => {
|
||||
refetchTrigger = { types: ["task:task-other"], timestamp: 3 };
|
||||
renderHook(() => useTaskQuorum("task-1", WORKSPACE_ID));
|
||||
await waitFor(() => expect(mocks.getTaskQuorum).toHaveBeenCalledTimes(1));
|
||||
expect(mocks.getTaskQuorum).toHaveBeenCalledWith("task-1", WORKSPACE_ID);
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useOfficeRefetch } from "@/hooks/use-office-refetch";
|
||||
import { getTaskQuorum } from "@/lib/api/domains/office-extended-api";
|
||||
import type { QuorumResponseDTO } from "@/lib/state/slices/office/quorum-types";
|
||||
import { t } from "@/lib/i18n";
|
||||
@@ -19,13 +20,6 @@ export function useTaskQuorum(
|
||||
): UseTaskQuorumResult {
|
||||
const quorum = useAppStore((s) => (taskId ? s.office.taskQuorum.byTaskId[taskId] : undefined));
|
||||
const setTaskQuorum = useAppStore((s) => s.setTaskQuorum);
|
||||
// office.task.decision_recorded (lib/ws/handlers/office.ts) bumps this
|
||||
// counter for `task:${taskId}` — recording a decision changes the guard's
|
||||
// approve/reject count, so the cached snapshot must be invalidated even
|
||||
// though taskId itself hasn't changed.
|
||||
const decisionRefetchTrigger = useAppStore((s) =>
|
||||
taskId ? s.office.refetchTriggers[`task:${taskId}`] : undefined,
|
||||
);
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [fetchedScope, setFetchedScope] = useState<string | null>(null);
|
||||
@@ -58,10 +52,13 @@ export function useTaskQuorum(
|
||||
void refresh();
|
||||
}, [taskId, workspaceId, fetchedScope, refresh]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!taskId || !workspaceId || decisionRefetchTrigger === undefined) return;
|
||||
void refresh();
|
||||
}, [taskId, workspaceId, decisionRefetchTrigger, refresh]);
|
||||
// office.task.decision_recorded (lib/ws/handlers/office.ts) fires
|
||||
// `task:${taskId}` — recording a decision changes the guard's
|
||||
// approve/reject count, so the cached snapshot must be invalidated even
|
||||
// though taskId itself hasn't changed.
|
||||
useOfficeRefetch(taskId ? `task:${taskId}` : "", () => {
|
||||
if (taskId && workspaceId) void refresh();
|
||||
});
|
||||
|
||||
return { quorum, isLoading, error, refresh };
|
||||
}
|
||||
|
||||
@@ -1,144 +1,62 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { cleanup, renderHook } from "@testing-library/react";
|
||||
|
||||
let storeState: { office: { refetchTriggers: Record<string, number> } } = {
|
||||
office: { refetchTriggers: {} },
|
||||
};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: typeof storeState) => unknown) => selector(storeState),
|
||||
}));
|
||||
|
||||
import { createElement, type ReactNode } from "react";
|
||||
import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { StateProvider, useAppStoreApi } from "@/components/state-provider";
|
||||
import { useOfficeRefetch } from "./use-office-refetch";
|
||||
|
||||
function bump(type: string) {
|
||||
const triggers = storeState.office.refetchTriggers;
|
||||
storeState = {
|
||||
office: { refetchTriggers: { ...triggers, [type]: (triggers[type] ?? 0) + 1 } },
|
||||
};
|
||||
afterEach(() => cleanup());
|
||||
|
||||
function wrapper({ children }: { children: ReactNode }) {
|
||||
return createElement(StateProvider, { children });
|
||||
}
|
||||
|
||||
/**
|
||||
* office.task.updated fires `task:<id>` and `dashboard` synchronously in the
|
||||
* same handler. Both a task-detail listener and a dashboard listener must
|
||||
* observe their own type from that one WS event — regression coverage for
|
||||
* the same-tick batching in setOfficeRefetchTrigger (AC-27).
|
||||
*/
|
||||
describe("useOfficeRefetch", () => {
|
||||
beforeEach(() => {
|
||||
storeState = { office: { refetchTriggers: {} } };
|
||||
});
|
||||
it("notifies every listener whose type was fired in the same tick", async () => {
|
||||
const taskCallback = vi.fn();
|
||||
const dashboardCallback = vi.fn();
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it("does not fire on mount when no trigger has fired for this type", () => {
|
||||
const onRefetch = vi.fn();
|
||||
renderHook(() => useOfficeRefetch("dashboard", onRefetch));
|
||||
expect(onRefetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fires when its exact type is bumped", () => {
|
||||
const onRefetch = vi.fn();
|
||||
const { rerender } = renderHook(() => useOfficeRefetch("dashboard", onRefetch));
|
||||
|
||||
bump("dashboard");
|
||||
rerender();
|
||||
|
||||
expect(onRefetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("fires on a prefix match (comments: matches comments:task-123)", () => {
|
||||
const onRefetch = vi.fn();
|
||||
const { rerender } = renderHook(() => useOfficeRefetch("comments", onRefetch));
|
||||
|
||||
bump("comments:task-123");
|
||||
rerender();
|
||||
|
||||
expect(onRefetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not fire for an unrelated type", () => {
|
||||
const onRefetch = vi.fn();
|
||||
const { rerender } = renderHook(() => useOfficeRefetch("dashboard", onRefetch));
|
||||
|
||||
bump("tasks");
|
||||
rerender();
|
||||
|
||||
expect(onRefetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not re-fire on a render that doesn't bump its type", () => {
|
||||
const onRefetch = vi.fn();
|
||||
const { rerender } = renderHook(() => useOfficeRefetch("dashboard", onRefetch));
|
||||
|
||||
bump("dashboard");
|
||||
rerender();
|
||||
rerender();
|
||||
rerender();
|
||||
|
||||
expect(onRefetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Regression test: a single WS handler often bumps several distinct trigger
|
||||
// types in one synchronous call (e.g. `task:${id}` then `dashboard`), which
|
||||
// React coalesces into a single render. A shared "last trigger" store field
|
||||
// would overwrite itself on every bump, so only the final type's subscriber
|
||||
// would ever observe a change — every other type silently never refetches.
|
||||
// Per-type counters must let every matching subscriber see its own bump
|
||||
// even when several types were bumped before React ever renders.
|
||||
it("lets every subscriber observe its own bump when multiple types are bumped in the same tick", () => {
|
||||
const onTaskRefetch = vi.fn();
|
||||
const onDashboardRefetch = vi.fn();
|
||||
const { rerender: rerenderTask } = renderHook(() =>
|
||||
useOfficeRefetch("task:t-1", onTaskRefetch),
|
||||
);
|
||||
const { rerender: rerenderDashboard } = renderHook(() =>
|
||||
useOfficeRefetch("dashboard", onDashboardRefetch),
|
||||
const { result } = renderHook(
|
||||
() => {
|
||||
useOfficeRefetch("task:t-1", taskCallback);
|
||||
useOfficeRefetch("dashboard", dashboardCallback);
|
||||
return useAppStoreApi();
|
||||
},
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
// Simulate a WS handler firing both triggers before either hook re-renders.
|
||||
bump("task:t-1");
|
||||
bump("dashboard");
|
||||
rerenderTask();
|
||||
rerenderDashboard();
|
||||
|
||||
expect(onTaskRefetch).toHaveBeenCalledTimes(1);
|
||||
expect(onDashboardRefetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Regression test: a prefix subscriber watches several sub-keys under one
|
||||
// triggerType (e.g. "comments" matches "comments:task-A" and
|
||||
// "comments:task-B"). A single scalar "last seen" value would let a bump
|
||||
// on one sub-key mask a later bump on a sibling sub-key whenever the
|
||||
// sibling's counter is lower than the max already observed. Per-key
|
||||
// tracking must let each sub-key's bump fire independently.
|
||||
it("still fires for a sub-key bump after a higher-numbered sibling sub-key already fired", () => {
|
||||
const onRefetch = vi.fn();
|
||||
const { rerender } = renderHook(() => useOfficeRefetch("comments", onRefetch));
|
||||
|
||||
// task-B races ahead first, bumped three times.
|
||||
bump("comments:task-B");
|
||||
bump("comments:task-B");
|
||||
bump("comments:task-B");
|
||||
rerender();
|
||||
expect(onRefetch).toHaveBeenCalledTimes(1);
|
||||
|
||||
// task-A's first bump (counter 1) is lower than task-B's counter (3), so
|
||||
// a scalar "max seen" comparison would swallow it.
|
||||
bump("comments:task-A");
|
||||
rerender();
|
||||
|
||||
expect(onRefetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("uses the latest callback without re-firing when only the callback changes", () => {
|
||||
const first = vi.fn();
|
||||
const second = vi.fn();
|
||||
const { rerender } = renderHook(({ cb }) => useOfficeRefetch("dashboard", cb), {
|
||||
initialProps: { cb: first },
|
||||
act(() => {
|
||||
result.current.getState().setOfficeRefetchTrigger("task:t-1");
|
||||
result.current.getState().setOfficeRefetchTrigger("dashboard");
|
||||
});
|
||||
|
||||
rerender({ cb: second });
|
||||
bump("dashboard");
|
||||
rerender({ cb: second });
|
||||
await waitFor(() => expect(taskCallback).toHaveBeenCalledTimes(1));
|
||||
await waitFor(() => expect(dashboardCallback).toHaveBeenCalledTimes(1));
|
||||
});
|
||||
|
||||
expect(first).not.toHaveBeenCalled();
|
||||
expect(second).toHaveBeenCalledTimes(1);
|
||||
it("prefix-matches a task channel without notifying an unrelated task", async () => {
|
||||
const matchingCallback = vi.fn();
|
||||
const otherCallback = vi.fn();
|
||||
|
||||
const { result } = renderHook(
|
||||
() => {
|
||||
useOfficeRefetch("task:t-1", matchingCallback);
|
||||
useOfficeRefetch("task:t-2", otherCallback);
|
||||
return useAppStoreApi();
|
||||
},
|
||||
{ wrapper },
|
||||
);
|
||||
|
||||
act(() => {
|
||||
result.current.getState().setOfficeRefetchTrigger("task:t-1");
|
||||
});
|
||||
|
||||
await waitFor(() => expect(matchingCallback).toHaveBeenCalledTimes(1));
|
||||
expect(otherCallback).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,48 +1,34 @@
|
||||
import { useEffect, useLayoutEffect, useRef } from "react";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
|
||||
// Any real counter the store can hold is >= 1, so 0 reliably "looks bumped"
|
||||
// on the very first observed value and "looks unbumped" otherwise — see
|
||||
// use-session-commits.ts for the same sentinel pattern.
|
||||
const REFETCH_TRIGGER_INIT = 0;
|
||||
|
||||
/**
|
||||
* Calls `onRefetch` when a matching office refetch trigger fires.
|
||||
* Supports exact match ("dashboard") or prefix match ("comments:" matches "comments:task-123").
|
||||
*
|
||||
* Triggers are per-type counters (`office.refetchTriggers`), not a single
|
||||
* "last trigger" value: a WS handler often bumps several distinct types in
|
||||
* one synchronous call (e.g. `task:${id}` then `dashboard`), and React
|
||||
* coalesces those into one render, so a shared last-value field would only
|
||||
* Triggers are batched into a single `office.refetchTrigger` object per tick
|
||||
* rather than tracked as separate values: a WS handler often bumps several
|
||||
* distinct types in one synchronous call (e.g. `task:${id}` then `dashboard`),
|
||||
* and React coalesces those into one render, so per-type values would only
|
||||
* ever be observable by the final type's subscribers.
|
||||
*
|
||||
* @param triggerType - The trigger type to watch for (e.g. "dashboard", "tasks", "comments")
|
||||
* @param onRefetch - Callback invoked when a matching trigger fires
|
||||
*/
|
||||
export function useOfficeRefetch(triggerType: string, onRefetch: () => void) {
|
||||
const triggers = useAppStore((s) => s.office.refetchTriggers);
|
||||
const trigger = useAppStore((s) => s.office.refetchTrigger);
|
||||
const callbackRef = useRef(onRefetch);
|
||||
// Update ref in a layout effect to avoid mutating during render
|
||||
useLayoutEffect(() => {
|
||||
callbackRef.current = onRefetch;
|
||||
});
|
||||
|
||||
// Tracks the highest counter we've already acted on, per matching key, so
|
||||
// a bump triggers exactly one refetch rather than re-firing on every
|
||||
// render. A single scalar here would let a bump on one sub-key (e.g.
|
||||
// "comments:task-B") mask a later bump on a sibling sub-key ("comments:
|
||||
// task-A") whenever the sibling's counter happened to be lower.
|
||||
const lastSeenRef = useRef<Record<string, number>>({});
|
||||
|
||||
useEffect(() => {
|
||||
let fire = false;
|
||||
for (const [type, seq] of Object.entries(triggers)) {
|
||||
if (type !== triggerType && !type.startsWith(triggerType + ":")) continue;
|
||||
if (seq > (lastSeenRef.current[type] ?? REFETCH_TRIGGER_INIT)) {
|
||||
lastSeenRef.current[type] = seq;
|
||||
fire = true;
|
||||
}
|
||||
if (!trigger) return;
|
||||
const matches = trigger.types.some(
|
||||
(type) => type === triggerType || type.startsWith(triggerType + ":"),
|
||||
);
|
||||
if (matches) {
|
||||
callbackRef.current();
|
||||
}
|
||||
if (fire) callbackRef.current();
|
||||
}, [triggers, triggerType]);
|
||||
}, [trigger, triggerType]);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,228 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, cleanup, render } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { StateProvider, useAppStoreApi } from "@/components/state-provider";
|
||||
import { TaskOptimisticContextProvider, useCommitTaskTitle } from "./use-optimistic-task-mutation";
|
||||
import type { Task } from "@/app/office/tasks/[id]/types";
|
||||
import type { OfficeTask } from "@/lib/state/slices/office/types";
|
||||
import type { Task as HttpTask } from "@/lib/types/http";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
seedInitialCanonical,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
|
||||
vi.mock("sonner", () => ({
|
||||
toast: {
|
||||
error: vi.fn(),
|
||||
success: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/kanban-api", () => ({
|
||||
updateTask: vi.fn(),
|
||||
}));
|
||||
|
||||
import { toast } from "sonner";
|
||||
import { updateTask } from "@/lib/api/domains/kanban-api";
|
||||
|
||||
const mockUpdateTask = vi.mocked(updateTask);
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
const TS = "2026-05-01T00:00:00Z";
|
||||
const ORIGINAL_TITLE = "Original Title";
|
||||
const WRITE_FAILURE_MESSAGE = "boom";
|
||||
|
||||
const baseTask: Task = {
|
||||
id: "t-1",
|
||||
workspaceId: "ws-1",
|
||||
identifier: "TASK-1",
|
||||
title: "First task",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
labels: [],
|
||||
blockedBy: [],
|
||||
blocking: [],
|
||||
children: [],
|
||||
reviewers: [],
|
||||
approvers: [],
|
||||
decisions: [],
|
||||
createdBy: "user",
|
||||
createdAt: TS,
|
||||
updatedAt: TS,
|
||||
};
|
||||
|
||||
const baseOfficeTask: OfficeTask = {
|
||||
id: "t-1",
|
||||
workspaceId: "ws-1",
|
||||
identifier: "TASK-1",
|
||||
title: "First task",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
createdAt: TS,
|
||||
updatedAt: TS,
|
||||
};
|
||||
|
||||
function makeHarness(initialTask: Task, initialOffice: OfficeTask | null) {
|
||||
const state = {
|
||||
task: initialTask,
|
||||
patches: [] as Partial<Task>[],
|
||||
};
|
||||
const ctxValue = {
|
||||
task: initialTask,
|
||||
applyPatch: (patch: Partial<Task>) => {
|
||||
state.patches.push(patch);
|
||||
state.task = { ...state.task, ...patch };
|
||||
},
|
||||
restore: (snapshot: Task) => {
|
||||
state.task = snapshot;
|
||||
},
|
||||
};
|
||||
function StoreSeed({ children }: { children: ReactNode }) {
|
||||
const api = useAppStoreApi();
|
||||
if (initialOffice) {
|
||||
api.getState().setTasks([initialOffice]);
|
||||
}
|
||||
return <>{children}</>;
|
||||
}
|
||||
function Wrapper({ children }: { children: ReactNode }) {
|
||||
return (
|
||||
<StateProvider>
|
||||
<StoreSeed>
|
||||
<TaskOptimisticContextProvider value={ctxValue}>{children}</TaskOptimisticContextProvider>
|
||||
</StoreSeed>
|
||||
</StateProvider>
|
||||
);
|
||||
}
|
||||
return { Wrapper, state };
|
||||
}
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
let reject!: (reason?: unknown) => void;
|
||||
const promise = new Promise<T>((res, rej) => {
|
||||
resolve = res;
|
||||
reject = rej;
|
||||
});
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
|
||||
function TitleHookProbe({
|
||||
onReady,
|
||||
}: {
|
||||
onReady: (
|
||||
commit: ReturnType<typeof useCommitTaskTitle>,
|
||||
storeApi: ReturnType<typeof useAppStoreApi>,
|
||||
) => void;
|
||||
}) {
|
||||
const commit = useCommitTaskTitle();
|
||||
const storeApi = useAppStoreApi();
|
||||
onReady(commit, storeApi);
|
||||
return null;
|
||||
}
|
||||
|
||||
describe("useCommitTaskTitle rollback ordering — AC-65 chained failures", () => {
|
||||
it("chained double failure: restores only once both commits have resolved, regardless of resolution order (AC-65)", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
let storeApi: ReturnType<typeof useAppStoreApi> | null = null;
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c, s) => ([commit, storeApi] = [c, s])} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const first = deferred<HttpTask>();
|
||||
const second = deferred<HttpTask>();
|
||||
mockUpdateTask.mockReturnValueOnce(first.promise);
|
||||
mockUpdateTask.mockReturnValueOnce(second.promise);
|
||||
|
||||
let p1!: Promise<void>;
|
||||
let p2!: Promise<void>;
|
||||
act(() => {
|
||||
p1 = commit!("t-1", "A");
|
||||
p2 = commit!("t-1", "B");
|
||||
});
|
||||
|
||||
// AC-53: the earlier commit (A) fails first, but B is still unresolved,
|
||||
// so no restore happens yet — the displayed title stays at B's draft.
|
||||
await act(async () => {
|
||||
first.reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await p1;
|
||||
});
|
||||
expect(state.task.title).toBe("B");
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe("B");
|
||||
|
||||
// AC-65: the later commit (B) now also fails. Nothing is left pending,
|
||||
// so this is the restore that actually fires — to the recorded canonical
|
||||
// value, not to either commit's draft. Assert both the displayed title
|
||||
// and the Office task store entry, per the spec's explicit "both end at
|
||||
// the canonical title" requirement for this regression test.
|
||||
await act(async () => {
|
||||
second.reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await p2;
|
||||
});
|
||||
expect(state.task.title).toBe(ORIGINAL_TITLE);
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe(ORIGINAL_TITLE);
|
||||
expect(state.patches).toEqual([{ title: "A" }, { title: "B" }, { title: ORIGINAL_TITLE }]);
|
||||
expect(toast.error).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("chained double failure: restores as soon as the later commit resolves, even if it fails first (AC-65)", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
let storeApi: ReturnType<typeof useAppStoreApi> | null = null;
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c, s) => ([commit, storeApi] = [c, s])} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const first = deferred<HttpTask>();
|
||||
const second = deferred<HttpTask>();
|
||||
mockUpdateTask.mockReturnValueOnce(first.promise);
|
||||
mockUpdateTask.mockReturnValueOnce(second.promise);
|
||||
|
||||
let p1!: Promise<void>;
|
||||
let p2!: Promise<void>;
|
||||
act(() => {
|
||||
p1 = commit!("t-1", "A");
|
||||
p2 = commit!("t-1", "B");
|
||||
});
|
||||
|
||||
// The later commit (B) fails first, and nothing later is pending, so it
|
||||
// restores immediately to canonical.
|
||||
await act(async () => {
|
||||
second.reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await p2;
|
||||
});
|
||||
expect(state.task.title).toBe(ORIGINAL_TITLE);
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe(ORIGINAL_TITLE);
|
||||
|
||||
// The earlier commit (A) then also fails. B already resolved, so this
|
||||
// restore fires too (per AC-65's "restore in either resolution order"),
|
||||
// converging on the same canonical value rather than clobbering it —
|
||||
// asserted on both the displayed title and the Office task store entry
|
||||
// so this reaches the same end state as the reverse resolution order
|
||||
// above regardless of which layer is read.
|
||||
await act(async () => {
|
||||
first.reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await p1;
|
||||
});
|
||||
expect(state.task.title).toBe(ORIGINAL_TITLE);
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe(ORIGINAL_TITLE);
|
||||
expect(state.patches).toEqual([
|
||||
{ title: "A" },
|
||||
{ title: "B" },
|
||||
{ title: ORIGINAL_TITLE },
|
||||
{ title: ORIGINAL_TITLE },
|
||||
]);
|
||||
expect(toast.error).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
@@ -4,10 +4,19 @@ import type { ReactNode } from "react";
|
||||
import { StateProvider, useAppStoreApi } from "@/components/state-provider";
|
||||
import {
|
||||
TaskOptimisticContextProvider,
|
||||
useCommitTaskDescription,
|
||||
useCommitTaskTitle,
|
||||
useOptimisticTaskMutation,
|
||||
} from "./use-optimistic-task-mutation";
|
||||
import type { Task } from "@/app/office/tasks/[id]/types";
|
||||
import type { OfficeTask } from "@/lib/state/slices/office/types";
|
||||
import type { Task as HttpTask } from "@/lib/types/http";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
getCanonicalValue,
|
||||
seedInitialCanonical,
|
||||
subscribeField,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
|
||||
vi.mock("sonner", () => ({
|
||||
toast: {
|
||||
@@ -16,11 +25,19 @@ vi.mock("sonner", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/kanban-api", () => ({
|
||||
updateTask: vi.fn(),
|
||||
}));
|
||||
|
||||
import { toast } from "sonner";
|
||||
import { updateTask } from "@/lib/api/domains/kanban-api";
|
||||
|
||||
const mockUpdateTask = vi.mocked(updateTask);
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
const TS = "2026-05-01T00:00:00Z";
|
||||
@@ -154,6 +171,48 @@ describe("useOptimisticTaskMutation", () => {
|
||||
expect(toast.error).toHaveBeenCalledWith("Update failed");
|
||||
});
|
||||
|
||||
it("never restores title/description in the store, even when a confirmed edit lands mid-flight (AC-61)", async () => {
|
||||
const { Wrapper } = makeHarness(baseTask, baseOfficeTask);
|
||||
let mutate: ReturnType<typeof useOptimisticTaskMutation> | null = null;
|
||||
let storeApi: ReturnType<typeof useAppStoreApi> | null = null;
|
||||
function Probe() {
|
||||
const m = useOptimisticTaskMutation();
|
||||
const s = useAppStoreApi();
|
||||
mutate = m;
|
||||
storeApi = s;
|
||||
return null;
|
||||
}
|
||||
render(
|
||||
<Wrapper>
|
||||
<Probe />
|
||||
</Wrapper>,
|
||||
);
|
||||
const { promise, reject } = deferred<unknown>();
|
||||
const apiCall = vi.fn().mockReturnValue(promise);
|
||||
|
||||
let pending!: Promise<void>;
|
||||
act(() => {
|
||||
pending = mutate!("t-1", { priority: "high" }, apiCall);
|
||||
});
|
||||
|
||||
// A title commit succeeds server-side and lands in the store while this
|
||||
// unrelated priority mutation is still in flight (e.g. useCommitTaskTitle
|
||||
// resolving concurrently).
|
||||
act(() => {
|
||||
storeApi!.getState().patchTaskInStore("t-1", { title: "Confirmed New Title" });
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
reject(new Error("nope"));
|
||||
await expect(pending).rejects.toThrow("nope");
|
||||
});
|
||||
|
||||
// The failed priority mutation's rollback must not clobber the
|
||||
// meanwhile-confirmed title back to its pre-mutation snapshot value.
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe("Confirmed New Title");
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.priority).toBe("medium");
|
||||
});
|
||||
|
||||
it("works when the office store has no entry for the task", async () => {
|
||||
const { Wrapper, state } = makeHarness(baseTask, null);
|
||||
let mutate: ReturnType<typeof useOptimisticTaskMutation> | null = null;
|
||||
@@ -170,3 +229,456 @@ describe("useOptimisticTaskMutation", () => {
|
||||
expect(state.task.status).toBe("done");
|
||||
});
|
||||
});
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
let reject!: (reason?: unknown) => void;
|
||||
const promise = new Promise<T>((res, rej) => {
|
||||
resolve = res;
|
||||
reject = rej;
|
||||
});
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
|
||||
function taskResponse(overrides: Partial<HttpTask> & { title: string; description: string }) {
|
||||
return {
|
||||
id: "t-1",
|
||||
workspace_id: "ws-1",
|
||||
workflow_id: "wf-1",
|
||||
workflow_step_id: "step-1",
|
||||
position: 0,
|
||||
state: "TODO",
|
||||
priority: "medium",
|
||||
created_at: TS,
|
||||
updated_at: TS,
|
||||
...overrides,
|
||||
} as unknown as HttpTask;
|
||||
}
|
||||
|
||||
const NEW_TITLE = "New Title";
|
||||
const ORIGINAL_TITLE = "Original Title";
|
||||
const WRITE_FAILURE_MESSAGE = "boom";
|
||||
const WRITE_RESPONSE_TS = "2026-05-02T00:00:00Z";
|
||||
const NEW_DESCRIPTION = "New description";
|
||||
|
||||
function TitleHookProbe({
|
||||
onReady,
|
||||
}: {
|
||||
onReady: (
|
||||
commit: ReturnType<typeof useCommitTaskTitle>,
|
||||
storeApi: ReturnType<typeof useAppStoreApi>,
|
||||
) => void;
|
||||
}) {
|
||||
const commit = useCommitTaskTitle();
|
||||
const storeApi = useAppStoreApi();
|
||||
onReady(commit, storeApi);
|
||||
return null;
|
||||
}
|
||||
|
||||
describe("useCommitTaskTitle", () => {
|
||||
it("patches the local task and store optimistically at issue time, and keeps it on success (AC-3, AC-11)", async () => {
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
let storeApi: ReturnType<typeof useAppStoreApi> | null = null;
|
||||
const { promise, resolve } = deferred<ReturnType<typeof taskResponse>>();
|
||||
mockUpdateTask.mockReturnValueOnce(promise);
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c, s) => ([commit, storeApi] = [c, s])} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
let pending!: Promise<void>;
|
||||
act(() => {
|
||||
pending = commit!("t-1", NEW_TITLE);
|
||||
});
|
||||
expect(state.patches).toEqual([{ title: NEW_TITLE }]);
|
||||
expect(mockUpdateTask).toHaveBeenCalledWith("t-1", { title: NEW_TITLE });
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe(NEW_TITLE);
|
||||
|
||||
await act(async () => {
|
||||
resolve(taskResponse({ title: NEW_TITLE, description: "", updated_at: WRITE_RESPONSE_TS }));
|
||||
await pending;
|
||||
});
|
||||
expect(state.task.title).toBe(NEW_TITLE);
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe(NEW_TITLE);
|
||||
expect(toast.error).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("useCommitTaskTitle write-success canonical (AC-58)", () => {
|
||||
it("records the write's own response as canonical, and a subscribeField listener observes the saved text (not the pre-write baseline) once the guard releases, with no refetch in play", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
mockUpdateTask.mockResolvedValueOnce(
|
||||
taskResponse({ title: NEW_TITLE, description: "", updated_at: WRITE_RESPONSE_TS }),
|
||||
);
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const seen: string[] = [];
|
||||
subscribeField("t-1", "title", (value) => seen.push(value));
|
||||
|
||||
await act(async () => {
|
||||
await commit!("t-1", NEW_TITLE);
|
||||
});
|
||||
|
||||
expect(getCanonicalValue("t-1", "title")).toBe(NEW_TITLE);
|
||||
// The field is guarded (beginWrite) for the whole commit, so the
|
||||
// listener fires exactly once, after endWrite releases the guard, with
|
||||
// the saved value — never with the seeded pre-write baseline.
|
||||
expect(seen).toEqual([NEW_TITLE]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useCommitTaskTitle rollback ordering", () => {
|
||||
it("restores the currently recorded canonical title on failure when no later commit is in play (AC-9, AC-64)", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
mockUpdateTask.mockRejectedValueOnce(new Error(WRITE_FAILURE_MESSAGE));
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await commit!("t-1", NEW_TITLE);
|
||||
});
|
||||
|
||||
expect(state.patches).toEqual([{ title: NEW_TITLE }, { title: ORIGINAL_TITLE }]);
|
||||
expect(state.task.title).toBe(ORIGINAL_TITLE);
|
||||
expect(toast.error).toHaveBeenCalledWith(WRITE_FAILURE_MESSAGE);
|
||||
});
|
||||
|
||||
it("does not restore when a later-sequenced commit already succeeded (AC-44)", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const first = deferred<ReturnType<typeof taskResponse>>();
|
||||
const second = deferred<ReturnType<typeof taskResponse>>();
|
||||
mockUpdateTask.mockReturnValueOnce(first.promise);
|
||||
mockUpdateTask.mockReturnValueOnce(second.promise);
|
||||
|
||||
let p1!: Promise<void>;
|
||||
let p2!: Promise<void>;
|
||||
act(() => {
|
||||
p1 = commit!("t-1", "A");
|
||||
p2 = commit!("t-1", "B");
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
second.resolve(taskResponse({ title: "B", description: "", updated_at: WRITE_RESPONSE_TS }));
|
||||
await p2;
|
||||
first.reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await p1;
|
||||
});
|
||||
|
||||
expect(state.patches).toEqual([{ title: "A" }, { title: "B" }]);
|
||||
expect(state.task.title).toBe("B");
|
||||
expect(toast.error).toHaveBeenCalledWith(WRITE_FAILURE_MESSAGE);
|
||||
});
|
||||
|
||||
it("does not restore when a later-sequenced commit is still unresolved (AC-53)", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const first = deferred<ReturnType<typeof taskResponse>>();
|
||||
const second = deferred<ReturnType<typeof taskResponse>>();
|
||||
mockUpdateTask.mockReturnValueOnce(first.promise);
|
||||
mockUpdateTask.mockReturnValueOnce(second.promise);
|
||||
|
||||
let p1!: Promise<void>;
|
||||
act(() => {
|
||||
p1 = commit!("t-1", "A");
|
||||
commit!("t-1", "B");
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
first.reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await p1;
|
||||
});
|
||||
|
||||
// The later commit (B) is still unresolved, so the earlier failure (A)
|
||||
// must not restore anything yet.
|
||||
expect(state.patches).toEqual([{ title: "A" }, { title: "B" }]);
|
||||
expect(state.task.title).toBe("B");
|
||||
expect(toast.error).toHaveBeenCalledWith(WRITE_FAILURE_MESSAGE);
|
||||
|
||||
await act(async () => {
|
||||
second.resolve(taskResponse({ title: "B", description: "", updated_at: WRITE_RESPONSE_TS }));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// AC-65's chained-double-failure regression tests live in
|
||||
// use-optimistic-task-mutation-rollback-ordering.test.tsx (kept out of this
|
||||
// file to stay under the max-lines limit).
|
||||
|
||||
describe("useCommitTaskTitle rollback field scoping", () => {
|
||||
it("restores only the title, leaving an unrelated store field changed in the meantime untouched (AC-55)", async () => {
|
||||
seedInitialCanonical("t-1", "title", ORIGINAL_TITLE, TS);
|
||||
const { Wrapper } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
let storeApi: ReturnType<typeof useAppStoreApi> | null = null;
|
||||
const { promise, reject } = deferred<ReturnType<typeof taskResponse>>();
|
||||
mockUpdateTask.mockReturnValueOnce(promise);
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c, s) => ([commit, storeApi] = [c, s])} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
let pending!: Promise<void>;
|
||||
act(() => {
|
||||
pending = commit!("t-1", NEW_TITLE);
|
||||
});
|
||||
|
||||
// A second, unrelated store field changes while the title write is
|
||||
// still in flight (e.g. a concurrent priority change from elsewhere).
|
||||
act(() => {
|
||||
storeApi!.getState().patchTaskInStore("t-1", { priority: "high" });
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
reject(new Error(WRITE_FAILURE_MESSAGE));
|
||||
await pending;
|
||||
});
|
||||
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.title).toBe(ORIGINAL_TITLE);
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.priority).toBe("high");
|
||||
});
|
||||
});
|
||||
|
||||
function DescriptionHookProbe({
|
||||
onReady,
|
||||
}: {
|
||||
onReady: (commit: ReturnType<typeof useCommitTaskDescription>) => void;
|
||||
}) {
|
||||
const commit = useCommitTaskDescription();
|
||||
onReady(commit);
|
||||
return null;
|
||||
}
|
||||
|
||||
describe("useCommitTaskDescription", () => {
|
||||
it("does not patch until the save succeeds (AC-56)", async () => {
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskDescription> | null = null;
|
||||
const { promise, resolve } = deferred<ReturnType<typeof taskResponse>>();
|
||||
mockUpdateTask.mockReturnValueOnce(promise);
|
||||
render(
|
||||
<Wrapper>
|
||||
<DescriptionHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
let pending!: Promise<{ ok: true; value: string } | { ok: false }>;
|
||||
act(() => {
|
||||
pending = commit!("t-1", NEW_DESCRIPTION);
|
||||
});
|
||||
expect(state.patches).toEqual([]);
|
||||
expect(mockUpdateTask).toHaveBeenCalledWith("t-1", { description: NEW_DESCRIPTION });
|
||||
|
||||
let result!: { ok: true; value: string } | { ok: false };
|
||||
await act(async () => {
|
||||
resolve(
|
||||
taskResponse({
|
||||
title: "First task",
|
||||
description: NEW_DESCRIPTION,
|
||||
updated_at: WRITE_RESPONSE_TS,
|
||||
}),
|
||||
);
|
||||
result = await pending;
|
||||
});
|
||||
expect(state.patches).toEqual([{ description: NEW_DESCRIPTION }]);
|
||||
expect(result).toEqual({ ok: true, value: NEW_DESCRIPTION });
|
||||
});
|
||||
|
||||
it("leaves the draft state untouched, toasts, and reports failure (AC-18)", async () => {
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskDescription> | null = null;
|
||||
mockUpdateTask.mockRejectedValueOnce(new Error("nope"));
|
||||
render(
|
||||
<Wrapper>
|
||||
<DescriptionHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
let result!: { ok: true; value: string } | { ok: false };
|
||||
await act(async () => {
|
||||
result = await commit!("t-1", NEW_DESCRIPTION);
|
||||
});
|
||||
expect(state.patches).toEqual([]);
|
||||
expect(result).toEqual({ ok: false });
|
||||
expect(toast.error).toHaveBeenCalledWith("nope");
|
||||
});
|
||||
});
|
||||
|
||||
describe("useCommitTaskDescription write-success canonical (AC-58)", () => {
|
||||
it("records the write's own response as canonical, and a subscribeField listener observes the saved text (not the pre-write baseline) once the guard releases, with no refetch in play", async () => {
|
||||
seedInitialCanonical("t-1", "description", "Original description", TS);
|
||||
const { Wrapper } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskDescription> | null = null;
|
||||
mockUpdateTask.mockResolvedValueOnce(
|
||||
taskResponse({
|
||||
title: baseTask.title,
|
||||
description: NEW_DESCRIPTION,
|
||||
updated_at: WRITE_RESPONSE_TS,
|
||||
}),
|
||||
);
|
||||
render(
|
||||
<Wrapper>
|
||||
<DescriptionHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const seen: string[] = [];
|
||||
subscribeField("t-1", "description", (value) => seen.push(value));
|
||||
|
||||
await act(async () => {
|
||||
await commit!("t-1", NEW_DESCRIPTION);
|
||||
});
|
||||
|
||||
expect(getCanonicalValue("t-1", "description")).toBe(NEW_DESCRIPTION);
|
||||
expect(seen).toEqual([NEW_DESCRIPTION]);
|
||||
});
|
||||
});
|
||||
|
||||
function DescriptionHookProbeWithStore({
|
||||
onReady,
|
||||
}: {
|
||||
onReady: (
|
||||
commit: ReturnType<typeof useCommitTaskDescription>,
|
||||
storeApi: ReturnType<typeof useAppStoreApi>,
|
||||
) => void;
|
||||
}) {
|
||||
const commit = useCommitTaskDescription();
|
||||
const storeApi = useAppStoreApi();
|
||||
onReady(commit, storeApi);
|
||||
return null;
|
||||
}
|
||||
|
||||
describe("useCommitTaskDescription stale write response (P1 review fix)", () => {
|
||||
it("does not apply a write response that recordWriteSuccess rejects as stale, once a newer write already superseded it", async () => {
|
||||
seedInitialCanonical("t-1", "description", "Original description", TS);
|
||||
const { Wrapper, state } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskDescription> | null = null;
|
||||
let storeApi: ReturnType<typeof useAppStoreApi> | null = null;
|
||||
render(
|
||||
<Wrapper>
|
||||
<DescriptionHookProbeWithStore onReady={(c, s) => ([commit, storeApi] = [c, s])} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
const first = deferred<ReturnType<typeof taskResponse>>();
|
||||
const second = deferred<ReturnType<typeof taskResponse>>();
|
||||
mockUpdateTask.mockReturnValueOnce(first.promise);
|
||||
mockUpdateTask.mockReturnValueOnce(second.promise);
|
||||
|
||||
let p1!: Promise<{ ok: true; value: string } | { ok: false }>;
|
||||
let p2!: Promise<{ ok: true; value: string } | { ok: false }>;
|
||||
act(() => {
|
||||
p1 = commit!("t-1", "Draft A");
|
||||
p2 = commit!("t-1", "Draft B");
|
||||
});
|
||||
|
||||
let resultB!: { ok: true; value: string } | { ok: false };
|
||||
await act(async () => {
|
||||
second.resolve(
|
||||
taskResponse({
|
||||
title: baseTask.title,
|
||||
description: "Draft B",
|
||||
updated_at: "2026-05-02T00:00:00Z",
|
||||
}),
|
||||
);
|
||||
resultB = await p2;
|
||||
});
|
||||
expect(resultB).toEqual({ ok: true, value: "Draft B" });
|
||||
expect(state.patches).toEqual([{ description: "Draft B" }]);
|
||||
|
||||
let resultA!: { ok: true; value: string } | { ok: false };
|
||||
await act(async () => {
|
||||
first.resolve(
|
||||
taskResponse({
|
||||
title: baseTask.title,
|
||||
description: "Draft A",
|
||||
updated_at: "2026-05-01T12:00:00Z",
|
||||
}),
|
||||
);
|
||||
resultA = await p1;
|
||||
});
|
||||
|
||||
// Draft A's response is older than Draft B's already-recorded canonical
|
||||
// value, so recordWriteSuccess rejects it (AC-60/AC-62): it must not
|
||||
// apply to the local task or the store, and the hook must report the
|
||||
// still-current canonical value rather than the stale one it saved.
|
||||
expect(state.patches).toEqual([{ description: "Draft B" }]);
|
||||
expect(state.task.description).toBe("Draft B");
|
||||
expect(storeApi!.getState().office.tasks.items[0]?.description).toBe("Draft B");
|
||||
expect(resultA).toEqual({ ok: true, value: "Draft B" });
|
||||
expect(getCanonicalValue("t-1", "description")).toBe("Draft B");
|
||||
});
|
||||
});
|
||||
|
||||
describe("single-field request body (AC-26)", () => {
|
||||
it("sends only the title field when committing a title, even though the local task has other properties set", async () => {
|
||||
const { Wrapper } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskTitle> | null = null;
|
||||
mockUpdateTask.mockResolvedValueOnce(
|
||||
taskResponse({ title: NEW_TITLE, description: "", updated_at: WRITE_RESPONSE_TS }),
|
||||
);
|
||||
render(
|
||||
<Wrapper>
|
||||
<TitleHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await commit!("t-1", NEW_TITLE);
|
||||
});
|
||||
|
||||
expect(mockUpdateTask).toHaveBeenCalledTimes(1);
|
||||
expect(mockUpdateTask.mock.calls[0]?.[1]).toEqual({ title: NEW_TITLE });
|
||||
});
|
||||
|
||||
it("sends only the description field when committing a description, even though the local task has other properties set", async () => {
|
||||
const { Wrapper } = makeHarness(baseTask, baseOfficeTask);
|
||||
let commit: ReturnType<typeof useCommitTaskDescription> | null = null;
|
||||
mockUpdateTask.mockResolvedValueOnce(
|
||||
taskResponse({
|
||||
title: baseTask.title,
|
||||
description: NEW_DESCRIPTION,
|
||||
updated_at: WRITE_RESPONSE_TS,
|
||||
}),
|
||||
);
|
||||
render(
|
||||
<Wrapper>
|
||||
<DescriptionHookProbe onReady={(c) => (commit = c)} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await commit!("t-1", NEW_DESCRIPTION);
|
||||
});
|
||||
|
||||
expect(mockUpdateTask).toHaveBeenCalledTimes(1);
|
||||
expect(mockUpdateTask.mock.calls[0]?.[1]).toEqual({ description: NEW_DESCRIPTION });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,15 @@ import { useAppStoreApi } from "@/components/state-provider";
|
||||
import type { Task } from "@/app/office/tasks/[id]/types";
|
||||
import type { OfficeTask } from "@/lib/state/slices/office/types";
|
||||
import { t } from "@/lib/i18n";
|
||||
import { updateTask } from "@/lib/api/domains/kanban-api";
|
||||
import {
|
||||
beginWrite,
|
||||
endWrite,
|
||||
getCanonicalValue,
|
||||
nextTaskSequence,
|
||||
recordWriteSuccess,
|
||||
shouldRestoreAfterFailedWrite,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
|
||||
/**
|
||||
* Context for the local (page-level) task representation. The office store
|
||||
@@ -64,13 +73,21 @@ export function useOptimisticTaskMutation() {
|
||||
try {
|
||||
await apiCall();
|
||||
} catch (err) {
|
||||
// Rollback both layers.
|
||||
// Rollback both layers. This hook never patches title/description
|
||||
// (see `toOfficeTaskPatch` above), so the rollback must not touch
|
||||
// them either — those two fields are governed exclusively by the
|
||||
// per-field guard in office-task-content-sync.ts and have their own
|
||||
// dedicated writers (useCommitTaskTitle/useCommitTaskDescription).
|
||||
// Restoring the full pre-mutation snapshot here would silently
|
||||
// revert a confirmed title/description edit whenever an unrelated
|
||||
// picker mutation fails (AC-61: exactly two writers may touch a
|
||||
// guarded field).
|
||||
ctx.restore(snapshot);
|
||||
if (storeSnapshot) {
|
||||
storeApi.getState().patchTaskInStore(taskId, storeSnapshot);
|
||||
const { title: _title, description: _description, ...storeRollback } = storeSnapshot;
|
||||
storeApi.getState().patchTaskInStore(taskId, storeRollback);
|
||||
}
|
||||
const message = err instanceof Error ? err.message : t("task:updateFailed");
|
||||
toast.error(message);
|
||||
toastUpdateFailure(err);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
@@ -95,3 +112,99 @@ function toOfficeTaskPatch(patch: Partial<Task>): Partial<OfficeTask> {
|
||||
if (patch.blockedBy !== undefined) out.blockedBy = patch.blockedBy;
|
||||
return out;
|
||||
}
|
||||
|
||||
function toastUpdateFailure(err: unknown): void {
|
||||
const message = err instanceof Error ? err.message : t("task:updateFailed");
|
||||
toast.error(message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Commits a title edit (AC-3, AC-9, AC-11, AC-44, AC-53, AC-54, AC-55, AC-64,
|
||||
* AC-65). Optimistic at issue time: the local task and the Office task store
|
||||
* entry are patched with the trimmed draft before the request resolves. On
|
||||
* failure, restores the field's *currently recorded* canonical value — never
|
||||
* a commit-time snapshot — but only when `shouldRestoreAfterFailedWrite`
|
||||
* says this commit is still the "last word standing" for the field.
|
||||
*/
|
||||
export function useCommitTaskTitle() {
|
||||
const ctx = useTaskOptimisticContext();
|
||||
const storeApi = useAppStoreApi();
|
||||
|
||||
return useCallback(
|
||||
async (taskId: string, trimmedTitle: string): Promise<void> => {
|
||||
const sequence = nextTaskSequence(taskId);
|
||||
beginWrite(taskId, "title", sequence);
|
||||
ctx.applyPatch({ title: trimmedTitle });
|
||||
storeApi.getState().patchTaskInStore(taskId, { title: trimmedTitle });
|
||||
|
||||
try {
|
||||
const updated = await updateTask(taskId, { title: trimmedTitle });
|
||||
recordWriteSuccess(taskId, "title", updated.title, updated.updated_at, sequence);
|
||||
endWrite(taskId, "title", sequence);
|
||||
} catch (err) {
|
||||
const shouldRestore = shouldRestoreAfterFailedWrite(taskId, "title", sequence);
|
||||
endWrite(taskId, "title", sequence);
|
||||
if (shouldRestore) {
|
||||
const canonical = getCanonicalValue(taskId, "title");
|
||||
if (canonical !== undefined) {
|
||||
ctx.applyPatch({ title: canonical });
|
||||
storeApi.getState().patchTaskInStore(taskId, { title: canonical });
|
||||
}
|
||||
}
|
||||
toastUpdateFailure(err);
|
||||
}
|
||||
},
|
||||
[ctx, storeApi],
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Commits a description save (AC-16, AC-17, AC-18, AC-42, AC-56). Not
|
||||
* optimistic: neither the local task nor the Office task store entry is
|
||||
* patched until the write succeeds, and a failure needs no rollback because
|
||||
* nothing was applied. The caller (the description editor) is responsible
|
||||
* for AC-17/AC-42's "did the draft change while the save was in flight"
|
||||
* comparison against the returned value.
|
||||
*/
|
||||
export function useCommitTaskDescription() {
|
||||
const ctx = useTaskOptimisticContext();
|
||||
const storeApi = useAppStoreApi();
|
||||
|
||||
return useCallback(
|
||||
async (
|
||||
taskId: string,
|
||||
trimmedDescription: string,
|
||||
): Promise<{ ok: true; value: string } | { ok: false }> => {
|
||||
const sequence = nextTaskSequence(taskId);
|
||||
beginWrite(taskId, "description", sequence);
|
||||
|
||||
try {
|
||||
const updated = await updateTask(taskId, { description: trimmedDescription });
|
||||
const accepted = recordWriteSuccess(
|
||||
taskId,
|
||||
"description",
|
||||
updated.description,
|
||||
updated.updated_at,
|
||||
sequence,
|
||||
);
|
||||
endWrite(taskId, "description", sequence);
|
||||
if (!accepted) {
|
||||
// A newer write or refetch already superseded this response
|
||||
// (AC-60/AC-62): applying it here would show a stale description.
|
||||
// The already-recorded newer value reaches the page/store via the
|
||||
// subscribeField deferred-apply listener once the guard clears.
|
||||
const canonical = getCanonicalValue(taskId, "description") ?? updated.description;
|
||||
return { ok: true, value: canonical };
|
||||
}
|
||||
ctx.applyPatch({ description: updated.description });
|
||||
storeApi.getState().patchTaskInStore(taskId, { description: updated.description });
|
||||
return { ok: true, value: updated.description };
|
||||
} catch (err) {
|
||||
endWrite(taskId, "description", sequence);
|
||||
toastUpdateFailure(err);
|
||||
return { ok: false };
|
||||
}
|
||||
},
|
||||
[ctx, storeApi],
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,532 @@
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
beginWrite,
|
||||
closeFieldEditor,
|
||||
endAllEditorsForTask,
|
||||
endWrite,
|
||||
getCanonicalValue,
|
||||
isFieldGuarded,
|
||||
nextTaskSequence,
|
||||
openFieldEditor,
|
||||
parseUpdatedAt,
|
||||
recordRefetchCandidate,
|
||||
recordWriteSuccess,
|
||||
seedInitialCanonical,
|
||||
shouldRestoreAfterFailedWrite,
|
||||
subscribeField,
|
||||
} from "./office-task-content-sync";
|
||||
|
||||
const TASK = "task-1";
|
||||
const T10 = "2026-08-20T10:00:00Z";
|
||||
const T11 = "2026-08-20T11:00:00Z";
|
||||
const T12 = "2026-08-20T12:00:00Z";
|
||||
|
||||
beforeEach(() => {
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
describe("parseUpdatedAt", () => {
|
||||
it("parses the SQLite space-separated offset-bearing layout to nanosecond precision (AC-66)", () => {
|
||||
const ns = parseUpdatedAt("2026-08-20 14:30:05.123456789+00:00");
|
||||
const wholeSecondMs = Date.parse("2026-08-20T14:30:05+00:00");
|
||||
expect(ns).toBe(BigInt(wholeSecondMs) * BigInt(1_000_000) + BigInt(123456789));
|
||||
});
|
||||
|
||||
it("parses the RFC3339Nano T/Z layout emitted by a write response to nanosecond precision (AC-66)", () => {
|
||||
const ns = parseUpdatedAt("2026-08-20T14:30:05.123456789Z");
|
||||
const wholeSecondMs = Date.parse("2026-08-20T14:30:05Z");
|
||||
expect(ns).toBe(BigInt(wholeSecondMs) * BigInt(1_000_000) + BigInt(123456789));
|
||||
});
|
||||
|
||||
it("treats the space-separated form's offset as authoritative, matching its T/Z equivalent", () => {
|
||||
const spaceForm = parseUpdatedAt("2026-08-20 14:30:05.123456789+00:00");
|
||||
const tForm = parseUpdatedAt("2026-08-20T14:30:05.123456789Z");
|
||||
expect(spaceForm).toBe(tForm);
|
||||
});
|
||||
|
||||
it("resolves the space-separated form via its own embedded offset, not the host machine's local timezone (AC-66)", () => {
|
||||
// Every other space-separated fixture above uses a +00:00 offset, which
|
||||
// coincidentally equals local time on a UTC-TZ test runner and would not
|
||||
// catch a regression to `new Date(rawSpaceSeparatedString)` — the spec
|
||||
// explicitly calls out that browsers disagree on how (or whether) to
|
||||
// parse that non-ISO-8601 form, with some falling back to local time.
|
||||
// A non-zero offset makes the correct answer distinguishable from any
|
||||
// such fallback regardless of what timezone the test happens to run in.
|
||||
const ns = parseUpdatedAt("2026-08-20 14:30:05.123456789+05:30");
|
||||
// 14:30:05+05:30 is 09:00:05Z — computed via an explicit offset, so this
|
||||
// expected value is itself timezone-independent.
|
||||
const wholeSecondMs = Date.parse("2026-08-20T09:00:05+00:00");
|
||||
expect(ns).toBe(BigInt(wholeSecondMs) * BigInt(1_000_000) + BigInt(123456789));
|
||||
});
|
||||
|
||||
it("does not collapse two distinct sub-millisecond instants to the same value", () => {
|
||||
// AC-60/62 requires strict ordering between these two: both round to the
|
||||
// same millisecond under Date.parse (123ms), but differ at the
|
||||
// microsecond digit — a millisecond-truncating parse would wrongly
|
||||
// report them equal.
|
||||
const earlier = parseUpdatedAt("2026-08-20T14:30:05.123400000Z");
|
||||
const later = parseUpdatedAt("2026-08-20T14:30:05.123500000Z");
|
||||
expect(earlier).not.toBe(later);
|
||||
expect(earlier! < later!).toBe(true);
|
||||
});
|
||||
|
||||
it("returns null for an unparseable value (AC-67 candidate)", () => {
|
||||
expect(parseUpdatedAt("not-a-date")).toBeNull();
|
||||
expect(parseUpdatedAt("")).toBeNull();
|
||||
expect(parseUpdatedAt(null)).toBeNull();
|
||||
expect(parseUpdatedAt(undefined)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("seedInitialCanonical (AC-59, AC-72)", () => {
|
||||
it("records the first candidate unconditionally, even if its updated_at is unparseable", () => {
|
||||
seedInitialCanonical(TASK, "title", "Hello", "garbage");
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("Hello");
|
||||
});
|
||||
|
||||
it("a same-session revisit re-seed overwrites a stale canonical with a newer value (AC-59, AC-64)", () => {
|
||||
// A revisit's fresh initial-load seed must be able to supersede a
|
||||
// canonical value recorded by an earlier visit in the same SPA session —
|
||||
// otherwise a later failed commit's restore (AC-9/AC-64, which reads the
|
||||
// canonical value live at restore time) could resurrect a value the
|
||||
// server has already moved past instead of the value most recently
|
||||
// loaded from it.
|
||||
seedInitialCanonical(TASK, "title", "Hello", T10);
|
||||
seedInitialCanonical(TASK, "title", "Newer on revisit", T11);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("Newer on revisit");
|
||||
});
|
||||
|
||||
it("a revisit re-seed does not resurrect a stale value once a newer one is canonical", () => {
|
||||
// The revisit seed goes through the same AC-60/62 ordering as any other
|
||||
// candidate — it is not a blind overwrite, just no longer a no-op.
|
||||
seedInitialCanonical(TASK, "title", "Hello", T11);
|
||||
seedInitialCanonical(TASK, "title", "Stale revisit", T10);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("Hello");
|
||||
});
|
||||
|
||||
it("normalises an absent description to the empty string (AC-71)", () => {
|
||||
seedInitialCanonical(TASK, "description", undefined as unknown as string, T10);
|
||||
expect(getCanonicalValue(TASK, "description")).toBe("");
|
||||
});
|
||||
|
||||
it("records the first candidate unconditionally even when its sequence predates the field's last write (AC-72 overrides AC-52)", () => {
|
||||
// AC-72: until a field has a recorded canonical value, the seed rule
|
||||
// exempts the next candidate from every other rule, including the AC-52
|
||||
// staleness check — that check compares against a write, but there is
|
||||
// nothing yet to be stale relative to. This reproduces the initial page
|
||||
// load's seed call landing after a title commit was already issued
|
||||
// (e.g. the authoritative GET resolves late, or is retried, while the
|
||||
// page already rendered the task interactively from a store cache).
|
||||
const staleSeq = nextTaskSequence(TASK);
|
||||
const writeSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", writeSeq);
|
||||
|
||||
const accepted = recordRefetchCandidate(TASK, "title", "first ever value", T10, staleSeq);
|
||||
|
||||
expect(accepted).toBe(true);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("first ever value");
|
||||
});
|
||||
});
|
||||
|
||||
describe("recordRefetchCandidate ordering (AC-60, AC-62, AC-63, AC-73)", () => {
|
||||
it("discards a strictly older candidate (AC-60)", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const accepted = recordRefetchCandidate(
|
||||
TASK,
|
||||
"title",
|
||||
"older",
|
||||
"2026-08-20T09:00:00Z",
|
||||
nextTaskSequence(TASK),
|
||||
);
|
||||
expect(accepted).toBe(false);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("v1");
|
||||
});
|
||||
|
||||
it("accepts a strictly newer candidate", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const accepted = recordRefetchCandidate(TASK, "title", "newer", T11, nextTaskSequence(TASK));
|
||||
expect(accepted).toBe(true);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("newer");
|
||||
});
|
||||
|
||||
it("breaks an equal-instant tie by request-issue sequence, higher sequence wins (AC-63)", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const loserSeq = nextTaskSequence(TASK);
|
||||
const winnerSeq = nextTaskSequence(TASK);
|
||||
// Winner recorded first, loser arrives after but carries a lower sequence.
|
||||
expect(recordRefetchCandidate(TASK, "title", "winner", T10, winnerSeq)).toBe(true);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("winner");
|
||||
expect(recordRefetchCandidate(TASK, "title", "loser", T10, loserSeq)).toBe(false);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("winner");
|
||||
});
|
||||
|
||||
it("discards a candidate whose updated_at cannot be parsed (AC-67)", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const accepted = recordRefetchCandidate(
|
||||
TASK,
|
||||
"title",
|
||||
"bad",
|
||||
"garbage",
|
||||
nextTaskSequence(TASK),
|
||||
);
|
||||
expect(accepted).toBe(false);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("v1");
|
||||
});
|
||||
|
||||
it("an unparseable seed loses to any later candidate that does parse (AC-73)", () => {
|
||||
seedInitialCanonical(TASK, "title", "seed", "garbage");
|
||||
const accepted = recordRefetchCandidate(
|
||||
TASK,
|
||||
"title",
|
||||
"supersedes",
|
||||
T10,
|
||||
nextTaskSequence(TASK),
|
||||
);
|
||||
expect(accepted).toBe(true);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("supersedes");
|
||||
});
|
||||
|
||||
it("discards a stale SQLite-layout refetch that lands within the same millisecond as an already-recorded RFC3339Nano write (AC-60/62/66)", () => {
|
||||
// Both instants round to the same millisecond, so a millisecond-precision
|
||||
// comparison would wrongly treat them as an equal-instant tie (or worse,
|
||||
// accept the stale one outright) instead of correctly ordering them.
|
||||
seedInitialCanonical(TASK, "title", "seed", T10);
|
||||
const writeSeq = nextTaskSequence(TASK);
|
||||
expect(
|
||||
recordWriteSuccess(TASK, "title", "from write", "2026-08-20T14:30:05.123500000Z", writeSeq),
|
||||
).toBe(true);
|
||||
endWrite(TASK, "title", writeSeq);
|
||||
|
||||
const refetchSeq = nextTaskSequence(TASK);
|
||||
const accepted = recordRefetchCandidate(
|
||||
TASK,
|
||||
"title",
|
||||
"from stale refetch",
|
||||
"2026-08-20 14:30:05.123400000+00:00",
|
||||
refetchSeq,
|
||||
);
|
||||
expect(accepted).toBe(false);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("from write");
|
||||
});
|
||||
|
||||
it("accepts an SQLite-layout refetch that truly postdates an RFC3339Nano write by sub-millisecond microseconds (AC-60/62/66)", () => {
|
||||
seedInitialCanonical(TASK, "title", "seed", T10);
|
||||
const writeSeq = nextTaskSequence(TASK);
|
||||
expect(
|
||||
recordWriteSuccess(TASK, "title", "from write", "2026-08-20T14:30:05.123400000Z", writeSeq),
|
||||
).toBe(true);
|
||||
endWrite(TASK, "title", writeSeq);
|
||||
|
||||
const refetchSeq = nextTaskSequence(TASK);
|
||||
const accepted = recordRefetchCandidate(
|
||||
TASK,
|
||||
"title",
|
||||
"from later refetch",
|
||||
"2026-08-20 14:30:05.123500000+00:00",
|
||||
refetchSeq,
|
||||
);
|
||||
expect(accepted).toBe(true);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("from later refetch");
|
||||
});
|
||||
});
|
||||
|
||||
describe("recordRefetchCandidate staleness vs a committed write (AC-52)", () => {
|
||||
it("discards a refetch whose request predates this client's most recent write to the field", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const refetchSeq = nextTaskSequence(TASK);
|
||||
const writeSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", writeSeq);
|
||||
|
||||
const accepted = recordRefetchCandidate(TASK, "title", "stale", T12, refetchSeq);
|
||||
expect(accepted).toBe(false);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("v1");
|
||||
});
|
||||
|
||||
it("does not apply write-staleness to a write's own response (AC-52 scoped to refetch only)", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const writeSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", writeSeq);
|
||||
// A later write is also issued for the same field, making this write's
|
||||
// sequence no longer "most recent" — its own response must still record.
|
||||
const laterWriteSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", laterWriteSeq);
|
||||
|
||||
const accepted = recordWriteSuccess(TASK, "title", "first-write-result", T11, writeSeq);
|
||||
expect(accepted).toBe(true);
|
||||
});
|
||||
|
||||
it("still records a non-stale refetch while a write to that field is in flight (AC-49)", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const writeSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", writeSeq);
|
||||
const refetchSeq = nextTaskSequence(TASK);
|
||||
|
||||
const accepted = recordRefetchCandidate(TASK, "title", "v2-from-refetch", T11, refetchSeq);
|
||||
|
||||
expect(accepted).toBe(true);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("v2-from-refetch");
|
||||
// The write is still in flight, so the field remains guarded: recording
|
||||
// succeeded, but nothing was applied to the displayed value yet (AC-49
|
||||
// scoped to recording; AC-50 governs the deferred apply on release).
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("guard derivation (AC-28, AC-38, AC-68, AC-69, AC-70)", () => {
|
||||
it("is unguarded by default", () => {
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
});
|
||||
|
||||
it("is guarded while an editor is open", () => {
|
||||
openFieldEditor(TASK, "title");
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(true);
|
||||
closeFieldEditor(TASK, "title");
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
});
|
||||
|
||||
it("is guarded while a write is in flight, independent of editor state", () => {
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seq);
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(true);
|
||||
endWrite(TASK, "title", seq);
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
});
|
||||
|
||||
it("stays guarded until both the editor closes and the write resolves", () => {
|
||||
openFieldEditor(TASK, "title");
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seq);
|
||||
closeFieldEditor(TASK, "title");
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(true);
|
||||
endWrite(TASK, "title", seq);
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
});
|
||||
|
||||
it("endAllEditorsForTask ends only the open-editor contribution, not pending writes (AC-68)", () => {
|
||||
openFieldEditor(TASK, "title");
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "description", seq);
|
||||
openFieldEditor(TASK, "description");
|
||||
|
||||
endAllEditorsForTask(TASK);
|
||||
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
expect(isFieldGuarded(TASK, "description")).toBe(true); // write still pending
|
||||
endWrite(TASK, "description", seq);
|
||||
expect(isFieldGuarded(TASK, "description")).toBe(false);
|
||||
});
|
||||
|
||||
it("a write resolving after unmount still records canonical and clears its guard (AC-69)", () => {
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seq);
|
||||
endAllEditorsForTask(TASK); // simulate unmount; write is still in flight
|
||||
|
||||
const accepted = recordWriteSuccess(TASK, "title", "post-unmount", T12, seq);
|
||||
expect(accepted).toBe(true);
|
||||
endWrite(TASK, "title", seq);
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("post-unmount");
|
||||
});
|
||||
|
||||
it("ends a write's guard contribution the same way on failure as on success (AC-70)", () => {
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seq);
|
||||
// Failure path: caller does NOT call recordWriteSuccess, only endWrite.
|
||||
endWrite(TASK, "title", seq);
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldRestoreAfterFailedWrite (AC-9, AC-44, AC-53, R5-F6)", () => {
|
||||
it("restores when no other commit to the field is pending or already succeeded", () => {
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seq);
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", seq)).toBe(true);
|
||||
});
|
||||
|
||||
it("suppresses restore when a later-sequenced commit already succeeded (AC-44)", () => {
|
||||
const failingSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", failingSeq);
|
||||
const laterSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", laterSeq);
|
||||
recordWriteSuccess(TASK, "title", "later-wins", T11, laterSeq);
|
||||
endWrite(TASK, "title", laterSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", failingSeq)).toBe(false);
|
||||
});
|
||||
|
||||
it("suppresses restore when a later-sequenced commit is still unresolved (AC-53)", () => {
|
||||
const failingSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", failingSeq);
|
||||
const laterSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", laterSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", failingSeq)).toBe(false);
|
||||
});
|
||||
|
||||
it("does not suppress restore because of an earlier-sequenced commit, resolved or not (R5-F6)", () => {
|
||||
const earlierSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", earlierSeq); // still unresolved
|
||||
const failingSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", failingSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", failingSeq)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not suppress restore when a later-sequenced commit also resolves with failure (AC-65)", () => {
|
||||
const failingSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", failingSeq);
|
||||
const laterSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", laterSeq);
|
||||
// The later commit also fails: caller does not call recordWriteSuccess, only endWrite.
|
||||
endWrite(TASK, "title", laterSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", failingSeq)).toBe(true);
|
||||
});
|
||||
|
||||
it("chained double failure: earlier commit resolves first, restore is suppressed then completed by the later failure (AC-65)", () => {
|
||||
// This module never mutates canonical on failure — recordWriteSuccess is
|
||||
// the only writer, and it is deliberately never called here (both
|
||||
// commits fail). The two shouldRestoreAfterFailedWrite return values
|
||||
// below are the actual assertions; the real "does the displayed title
|
||||
// get restored to canonical" behavior is exercised end-to-end (via
|
||||
// useCommitTaskTitle's actual restore action) in
|
||||
// use-optimistic-task-mutation.test.tsx's "chained double failure" tests.
|
||||
seedInitialCanonical(TASK, "title", "original", T10);
|
||||
const earlySeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", earlySeq);
|
||||
const laterSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", laterSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", earlySeq)).toBe(false); // AC-53: later still pending
|
||||
endWrite(TASK, "title", earlySeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", laterSeq)).toBe(true); // nothing left pending
|
||||
endWrite(TASK, "title", laterSeq);
|
||||
});
|
||||
|
||||
it("chained double failure: later commit resolves first, both restores fire, canonical unaffected (AC-65)", () => {
|
||||
// See the comment on the previous test: this only exercises the boolean
|
||||
// query, not an actual restore. The end-to-end restore-to-canonical
|
||||
// behavior lives in use-optimistic-task-mutation.test.tsx.
|
||||
seedInitialCanonical(TASK, "title", "original", T10);
|
||||
const earlySeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", earlySeq);
|
||||
const laterSeq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", laterSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", laterSeq)).toBe(true); // nothing later pending
|
||||
endWrite(TASK, "title", laterSeq);
|
||||
|
||||
expect(shouldRestoreAfterFailedWrite(TASK, "title", earlySeq)).toBe(true); // the later write already resolved
|
||||
endWrite(TASK, "title", earlySeq);
|
||||
});
|
||||
});
|
||||
|
||||
describe("recordWriteSuccess ordering relative to guard release (AC-58)", () => {
|
||||
it("records the canonical value while the write's guard contribution is still held, before the caller releases it", () => {
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
const seq = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seq);
|
||||
|
||||
recordWriteSuccess(TASK, "title", "v2", T11, seq);
|
||||
|
||||
// Canonical must already reflect the persisted value even though the
|
||||
// write's guard contribution has not been released yet (endWrite has not
|
||||
// run) — AC-58 requires recording strictly before the guard releases,
|
||||
// not merely "eventually" once it does.
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("v2");
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(true);
|
||||
|
||||
endWrite(TASK, "title", seq);
|
||||
expect(isFieldGuarded(TASK, "title")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("convergence after overlapping successful title commits (AC-54)", () => {
|
||||
it("converges on the later-persisted title when the earlier commit's response is recorded second", () => {
|
||||
const seen: string[] = [];
|
||||
seedInitialCanonical(TASK, "title", "original", T10);
|
||||
subscribeField(TASK, "title", (value) => seen.push(value));
|
||||
|
||||
const seqA = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seqA);
|
||||
const seqB = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seqB);
|
||||
|
||||
// B was issued after A, but its server response is recorded first while
|
||||
// A is still in flight, so the field stays guarded and nothing notifies yet.
|
||||
recordWriteSuccess(TASK, "title", "B-title", T11, seqB);
|
||||
endWrite(TASK, "title", seqB);
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("B-title");
|
||||
expect(seen).toEqual([]);
|
||||
|
||||
// A's response arrives second, carrying a later updated_at (it was
|
||||
// persisted after B on the server) and releases the guard.
|
||||
recordWriteSuccess(TASK, "title", "A-title", T12, seqA);
|
||||
endWrite(TASK, "title", seqA);
|
||||
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("A-title");
|
||||
expect(seen).toEqual(["A-title"]);
|
||||
});
|
||||
|
||||
it("converges on the same later-persisted title regardless of which response is recorded first", () => {
|
||||
seedInitialCanonical(TASK, "title", "original", T10);
|
||||
|
||||
const seqA = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seqA);
|
||||
const seqB = nextTaskSequence(TASK);
|
||||
beginWrite(TASK, "title", seqB);
|
||||
|
||||
// This time the earlier-issued commit's response, carrying the earlier
|
||||
// updated_at, is recorded first.
|
||||
recordWriteSuccess(TASK, "title", "A-title", T11, seqA);
|
||||
endWrite(TASK, "title", seqA);
|
||||
recordWriteSuccess(TASK, "title", "B-title", T12, seqB);
|
||||
endWrite(TASK, "title", seqB);
|
||||
|
||||
expect(getCanonicalValue(TASK, "title")).toBe("B-title");
|
||||
});
|
||||
});
|
||||
|
||||
describe("deferred-apply notification (AC-27, AC-50)", () => {
|
||||
it("notifies immediately when a candidate is accepted while unguarded", () => {
|
||||
const seen: string[] = [];
|
||||
subscribeField(TASK, "title", (value) => seen.push(value));
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
recordRefetchCandidate(TASK, "title", "v2", T11, nextTaskSequence(TASK));
|
||||
expect(seen).toEqual(["v1", "v2"]);
|
||||
});
|
||||
|
||||
it("defers notification while guarded, then fires once the guard releases", () => {
|
||||
const seen: string[] = [];
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
subscribeField(TASK, "title", (value) => seen.push(value));
|
||||
|
||||
openFieldEditor(TASK, "title");
|
||||
recordRefetchCandidate(TASK, "title", "v2", T11, nextTaskSequence(TASK));
|
||||
expect(seen).toEqual([]); // guarded: no notification yet
|
||||
|
||||
closeFieldEditor(TASK, "title");
|
||||
expect(seen).toEqual(["v2"]);
|
||||
});
|
||||
|
||||
it("unsubscribe stops further notifications", () => {
|
||||
const seen: string[] = [];
|
||||
const unsubscribe = subscribeField(TASK, "title", (value) => seen.push(value));
|
||||
seedInitialCanonical(TASK, "title", "v1", T10);
|
||||
unsubscribe();
|
||||
recordRefetchCandidate(TASK, "title", "v2", T11, nextTaskSequence(TASK));
|
||||
expect(seen).toEqual(["v1"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("multi-task isolation", () => {
|
||||
it("does not leak canonical state between tasks", () => {
|
||||
seedInitialCanonical("task-a", "title", "A", T10);
|
||||
seedInitialCanonical("task-b", "title", "B", T10);
|
||||
expect(getCanonicalValue("task-a", "title")).toBe("A");
|
||||
expect(getCanonicalValue("task-b", "title")).toBe("B");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,326 @@
|
||||
/**
|
||||
* Canonical-value / in-flight-guard concurrency protocol for Office task
|
||||
* title + description editing (docs/specs/office-task-content-editing/spec.md,
|
||||
* section E). Framework-agnostic, module-level singleton state per task.
|
||||
*
|
||||
* Two structures are kept deliberately separate (see the task plan's R5-F4
|
||||
* record): `canonicalByTaskField` never loses a value when a field becomes
|
||||
* unguarded, while `guardByTaskField` entries are garbage-collected as soon
|
||||
* as a field has neither an open editor nor an unresolved write.
|
||||
*/
|
||||
|
||||
export type SyncField = "title" | "description";
|
||||
type CandidateKind = "refetch" | "write";
|
||||
|
||||
type CanonicalRecord = {
|
||||
value: string;
|
||||
updatedAtNs: bigint | null;
|
||||
sequence: number;
|
||||
};
|
||||
|
||||
type GuardState = {
|
||||
editorOpen: boolean;
|
||||
pendingWrites: Set<number>;
|
||||
};
|
||||
|
||||
const FIELDS: SyncField[] = ["title", "description"];
|
||||
|
||||
const canonicalByTaskField = new Map<string, Map<SyncField, CanonicalRecord>>();
|
||||
const guardByTaskField = new Map<string, Map<SyncField, GuardState>>();
|
||||
const sequenceByTask = new Map<string, number>();
|
||||
const lastWriteIssueSequenceByTaskField = new Map<string, Map<SyncField, number>>();
|
||||
const lastSuccessfulWriteSequenceByTaskField = new Map<string, Map<SyncField, number>>();
|
||||
const listenersByTaskField = new Map<string, Map<SyncField, Set<(value: string) => void>>>();
|
||||
|
||||
function fieldMap<T>(store: Map<string, Map<SyncField, T>>, taskId: string): Map<SyncField, T> {
|
||||
let m = store.get(taskId);
|
||||
if (!m) {
|
||||
m = new Map();
|
||||
store.set(taskId, m);
|
||||
}
|
||||
return m;
|
||||
}
|
||||
|
||||
/** Monotonic per-task counter; call at request-issue time (refetch or write). */
|
||||
export function nextTaskSequence(taskId: string): number {
|
||||
const next = (sequenceByTask.get(taskId) ?? 0) + 1;
|
||||
sequenceByTask.set(taskId, next);
|
||||
return next;
|
||||
}
|
||||
|
||||
function normalizeValue(raw: string | null | undefined): string {
|
||||
// AC-71: description normalises an absent value to "". Title's DTO
|
||||
// declares it required, but applying the same fallback to both is harmless.
|
||||
return raw ?? "";
|
||||
}
|
||||
|
||||
const DATETIME_LAYOUT =
|
||||
/^(\d{4})-(\d{2})-(\d{2})[ T](\d{2}):(\d{2}):(\d{2})(\.\d+)?(Z|[+-]\d{2}:\d{2})$/;
|
||||
|
||||
/**
|
||||
* Parses both `updated_at` layouts in play (AC-66): the space-separated,
|
||||
* offset-bearing SQLite bind layout, and RFC3339Nano. Returns nanoseconds
|
||||
* since the Unix epoch as a bigint, not `Date.parse`'s millisecond number —
|
||||
* `Date.parse` truncates any fractional-second precision past milliseconds,
|
||||
* which would silently defeat AC-60/62's strict ordering guarantee for two
|
||||
* candidates issued within the same millisecond (exactly what a refetch
|
||||
* racing a write's own response produces). The fractional-second digits are
|
||||
* parsed directly rather than handed to `Date.parse`, whose input must not
|
||||
* carry the space-separated form as-is since browsers disagree on how (or
|
||||
* whether) to interpret it.
|
||||
*/
|
||||
export function parseUpdatedAt(raw: string | null | undefined): bigint | null {
|
||||
if (!raw) return null;
|
||||
const match = DATETIME_LAYOUT.exec(raw.trim());
|
||||
if (!match) return null;
|
||||
const [, year, month, day, hour, minute, second, frac = "", offset] = match;
|
||||
const wholeSecondMs = Date.parse(`${year}-${month}-${day}T${hour}:${minute}:${second}${offset}`);
|
||||
if (Number.isNaN(wholeSecondMs)) return null;
|
||||
const nanoDigits = `${frac.slice(1)}000000000`.slice(0, 9);
|
||||
return BigInt(wholeSecondMs) * BigInt(1_000_000) + BigInt(nanoDigits);
|
||||
}
|
||||
|
||||
export function isFieldGuarded(taskId: string, field: SyncField): boolean {
|
||||
const g = guardByTaskField.get(taskId)?.get(field);
|
||||
return !!g && (g.editorOpen || g.pendingWrites.size > 0);
|
||||
}
|
||||
|
||||
export function getCanonicalValue(taskId: string, field: SyncField): string | undefined {
|
||||
return canonicalByTaskField.get(taskId)?.get(field)?.value;
|
||||
}
|
||||
|
||||
function notifyField(taskId: string, field: SyncField): void {
|
||||
const value = getCanonicalValue(taskId, field);
|
||||
if (value === undefined) return;
|
||||
const listeners = listenersByTaskField.get(taskId)?.get(field);
|
||||
if (!listeners || listeners.size === 0) return;
|
||||
listeners.forEach((listener) => listener(value));
|
||||
}
|
||||
|
||||
/**
|
||||
* Subscribes to a field's deferred-apply notifications (AC-27/AC-50). Fired
|
||||
* whenever a candidate is accepted while the field is unguarded, and whenever
|
||||
* the field transitions from guarded to unguarded. The listener receives the
|
||||
* current canonical value and is responsible for its own "if the two differ"
|
||||
* comparison against whatever it currently displays.
|
||||
*/
|
||||
export function subscribeField(
|
||||
taskId: string,
|
||||
field: SyncField,
|
||||
listener: (value: string) => void,
|
||||
): () => void {
|
||||
const tm = fieldMap(listenersByTaskField, taskId);
|
||||
const set = tm.get(field) ?? new Set<(value: string) => void>();
|
||||
set.add(listener);
|
||||
tm.set(field, set);
|
||||
return () => {
|
||||
set.delete(listener);
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Records a candidate canonical value for a field. Implements the seed rule
|
||||
* (AC-72), the parse-failure discard (AC-67), the unknown-instant-loses rule
|
||||
* (AC-73), the strict-ordering discard (AC-60/AC-62), the equal-instant
|
||||
* tiebreak by request-issue sequence (AC-63), and — for refetch candidates
|
||||
* only — the write-predates-refetch staleness discard (AC-52). Returns
|
||||
* whether the candidate was accepted. On acceptance, fires a deferred-apply
|
||||
* notification immediately if the field is currently unguarded; otherwise the
|
||||
* eventual guard release will notify once the guard clears (AC-50).
|
||||
*/
|
||||
type Candidate = {
|
||||
rawValue: string | null | undefined;
|
||||
updatedAtRaw: string | null | undefined;
|
||||
sequence: number;
|
||||
kind: CandidateKind;
|
||||
};
|
||||
|
||||
function recordCandidate(taskId: string, field: SyncField, candidate: Candidate): boolean {
|
||||
const { rawValue, updatedAtRaw, sequence, kind } = candidate;
|
||||
const canonicalMap = fieldMap(canonicalByTaskField, taskId);
|
||||
const current = canonicalMap.get(field);
|
||||
const value = normalizeValue(rawValue);
|
||||
const candidateNs = parseUpdatedAt(updatedAtRaw);
|
||||
|
||||
// AC-72: until a field has a recorded canonical value, this is a seed and
|
||||
// every other rule below (including AC-52's staleness check) is exempted —
|
||||
// there is nothing yet to be stale, out of order, or unparseable relative to.
|
||||
if (current && kind === "refetch") {
|
||||
const lastWriteSeq = lastWriteIssueSequenceByTaskField.get(taskId)?.get(field);
|
||||
if (lastWriteSeq !== undefined && sequence < lastWriteSeq) {
|
||||
return false; // AC-52
|
||||
}
|
||||
}
|
||||
|
||||
if (current) {
|
||||
if (candidateNs === null) {
|
||||
return false; // AC-67
|
||||
}
|
||||
if (current.updatedAtNs !== null) {
|
||||
if (candidateNs < current.updatedAtNs) return false; // AC-60 / AC-62
|
||||
if (candidateNs === current.updatedAtNs && sequence <= current.sequence) return false; // AC-63 loser
|
||||
}
|
||||
// Either candidateNs > current.updatedAtNs, or candidateNs === current.updatedAtNs
|
||||
// with a higher sequence, or current.updatedAtNs was unparseable (AC-73).
|
||||
}
|
||||
|
||||
canonicalMap.set(field, { value, updatedAtNs: candidateNs, sequence });
|
||||
if (!isFieldGuarded(taskId, field)) {
|
||||
notifyField(taskId, field);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* AC-59: seeds a field's canonical value on initial page load. The first-ever
|
||||
* call for a (taskId, field) is an unconditional AC-72 seed; a same-session
|
||||
* revisit's call goes through `recordCandidate`'s normal ordering rules like
|
||||
* any other candidate, so a fresh load can supersede a stale canonical value
|
||||
* left over from an earlier visit instead of being silently dropped.
|
||||
*/
|
||||
export function seedInitialCanonical(
|
||||
taskId: string,
|
||||
field: SyncField,
|
||||
value: string,
|
||||
updatedAtRaw: string | null | undefined,
|
||||
): void {
|
||||
recordCandidate(taskId, field, {
|
||||
rawValue: value,
|
||||
updatedAtRaw,
|
||||
sequence: nextTaskSequence(taskId),
|
||||
kind: "refetch",
|
||||
});
|
||||
}
|
||||
|
||||
/** AC-49: records a non-stale refetch's value as canonical for a field. */
|
||||
export function recordRefetchCandidate(
|
||||
taskId: string,
|
||||
field: SyncField,
|
||||
rawValue: string | null | undefined,
|
||||
updatedAtRaw: string | null | undefined,
|
||||
sequence: number,
|
||||
): boolean {
|
||||
return recordCandidate(taskId, field, { rawValue, updatedAtRaw, sequence, kind: "refetch" });
|
||||
}
|
||||
|
||||
/**
|
||||
* AC-58: records a successful write's own response value as canonical.
|
||||
* Must be called before the caller releases the write's guard contribution
|
||||
* via `endWrite` (R5-F3: a failed write must never call this).
|
||||
*/
|
||||
export function recordWriteSuccess(
|
||||
taskId: string,
|
||||
field: SyncField,
|
||||
rawValue: string | null | undefined,
|
||||
updatedAtRaw: string | null | undefined,
|
||||
sequence: number,
|
||||
): boolean {
|
||||
const fm = fieldMap(lastSuccessfulWriteSequenceByTaskField, taskId);
|
||||
const current = fm.get(field);
|
||||
if (current === undefined || sequence > current) {
|
||||
fm.set(field, sequence);
|
||||
}
|
||||
return recordCandidate(taskId, field, { rawValue, updatedAtRaw, sequence, kind: "write" });
|
||||
}
|
||||
|
||||
/**
|
||||
* AC-44/AC-53: decides whether a failed write should restore the field's
|
||||
* canonical value. Must be called before `endWrite` removes this write's own
|
||||
* sequence from the in-flight set. Restore is suppressed when a *later*
|
||||
* (higher-sequence) commit to the same field has already succeeded (AC-44) or
|
||||
* is still unresolved (AC-53) — an earlier-sequenced commit in either state
|
||||
* does not suppress it (R5-F6).
|
||||
*/
|
||||
export function shouldRestoreAfterFailedWrite(
|
||||
taskId: string,
|
||||
field: SyncField,
|
||||
sequence: number,
|
||||
): boolean {
|
||||
const lastSuccess = lastSuccessfulWriteSequenceByTaskField.get(taskId)?.get(field);
|
||||
if (lastSuccess !== undefined && lastSuccess > sequence) return false; // AC-44
|
||||
|
||||
const pending = guardByTaskField.get(taskId)?.get(field)?.pendingWrites;
|
||||
if (pending) {
|
||||
for (const pendingSeq of pending) {
|
||||
if (pendingSeq > sequence) return false; // AC-53
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function releaseIfUnguarded(
|
||||
taskId: string,
|
||||
field: SyncField,
|
||||
gm: Map<SyncField, GuardState>,
|
||||
guard: GuardState,
|
||||
): void {
|
||||
if (!guard.editorOpen && guard.pendingWrites.size === 0) {
|
||||
gm.delete(field);
|
||||
notifyField(taskId, field);
|
||||
}
|
||||
}
|
||||
|
||||
/** AC-28/AC-38: an open editor is one of the two guard facts. */
|
||||
export function openFieldEditor(taskId: string, field: SyncField): void {
|
||||
const gm = fieldMap(guardByTaskField, taskId);
|
||||
const guard = gm.get(field) ?? { editorOpen: false, pendingWrites: new Set<number>() };
|
||||
guard.editorOpen = true;
|
||||
gm.set(field, guard);
|
||||
}
|
||||
|
||||
/** Closing the editor by any route (commit, cancel, blur) ends its guard contribution. */
|
||||
export function closeFieldEditor(taskId: string, field: SyncField): void {
|
||||
const gm = guardByTaskField.get(taskId);
|
||||
const guard = gm?.get(field);
|
||||
if (!gm || !guard) return;
|
||||
guard.editorOpen = false;
|
||||
releaseIfUnguarded(taskId, field, gm, guard);
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers an in-flight write's guard contribution and records this write
|
||||
* as the field's most-recently-issued write for AC-52 staleness comparisons.
|
||||
*/
|
||||
export function beginWrite(taskId: string, field: SyncField, sequence: number): void {
|
||||
const gm = fieldMap(guardByTaskField, taskId);
|
||||
const guard = gm.get(field) ?? { editorOpen: false, pendingWrites: new Set<number>() };
|
||||
guard.pendingWrites.add(sequence);
|
||||
gm.set(field, guard);
|
||||
fieldMap(lastWriteIssueSequenceByTaskField, taskId).set(field, sequence);
|
||||
}
|
||||
|
||||
/** AC-70: ends a write's guard contribution identically whether it succeeded or failed. */
|
||||
export function endWrite(taskId: string, field: SyncField, sequence: number): void {
|
||||
const gm = guardByTaskField.get(taskId);
|
||||
const guard = gm?.get(field);
|
||||
if (!gm || !guard) return;
|
||||
guard.pendingWrites.delete(sequence);
|
||||
releaseIfUnguarded(taskId, field, gm, guard);
|
||||
}
|
||||
|
||||
/**
|
||||
* AC-68: ends the open-editor contribution for both fields on unmount /
|
||||
* route change. Deliberately leaves `pendingWriteCount` untouched — a write
|
||||
* issued before unmount must still resolve and record (AC-69).
|
||||
*/
|
||||
export function endAllEditorsForTask(taskId: string): void {
|
||||
const gm = guardByTaskField.get(taskId);
|
||||
if (!gm) return;
|
||||
for (const field of FIELDS) {
|
||||
const guard = gm.get(field);
|
||||
if (guard?.editorOpen) {
|
||||
guard.editorOpen = false;
|
||||
releaseIfUnguarded(taskId, field, gm, guard);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Test-only: clears all module state between test cases. */
|
||||
export function __resetOfficeTaskContentSyncForTests(): void {
|
||||
canonicalByTaskField.clear();
|
||||
guardByTaskField.clear();
|
||||
sequenceByTask.clear();
|
||||
lastWriteIssueSequenceByTaskField.clear();
|
||||
lastSuccessfulWriteSequenceByTaskField.clear();
|
||||
listenersByTaskField.clear();
|
||||
}
|
||||
@@ -0,0 +1,65 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { create } from "zustand";
|
||||
import { immer } from "zustand/middleware/immer";
|
||||
import { createOfficeSlice } from "./office-slice";
|
||||
import type { OfficeSlice } from "./types";
|
||||
|
||||
function makeStore() {
|
||||
return create<OfficeSlice>()(
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
immer((...a) => ({ ...(createOfficeSlice as any)(...a) })),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* A single WS event routinely fires setOfficeRefetchTrigger more than once in
|
||||
* the same synchronous handler (e.g. `task:<id>` immediately followed by
|
||||
* `dashboard` for office.task.updated). Both calls must survive together in
|
||||
* one trigger object — otherwise React's automatic batching of the resulting
|
||||
* store updates only ever lets a listener observe the last write, silently
|
||||
* starving any listener keyed on the earlier type.
|
||||
*/
|
||||
describe("setOfficeRefetchTrigger — same-tick batching", () => {
|
||||
it("carries every type fired synchronously in the same trigger", async () => {
|
||||
const store = makeStore();
|
||||
|
||||
store.getState().setOfficeRefetchTrigger("task:t-1");
|
||||
store.getState().setOfficeRefetchTrigger("dashboard");
|
||||
|
||||
// The batched flush happens in a microtask.
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
|
||||
expect(store.getState().office.refetchTrigger?.types).toEqual(["task:t-1", "dashboard"]);
|
||||
});
|
||||
|
||||
it("de-dupes a type fired twice in the same tick", async () => {
|
||||
const store = makeStore();
|
||||
|
||||
store.getState().setOfficeRefetchTrigger("dashboard");
|
||||
store.getState().setOfficeRefetchTrigger("dashboard");
|
||||
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
|
||||
expect(store.getState().office.refetchTrigger?.types).toEqual(["dashboard"]);
|
||||
});
|
||||
|
||||
it("starts a fresh trigger object for calls in a later tick", async () => {
|
||||
const store = makeStore();
|
||||
|
||||
store.getState().setOfficeRefetchTrigger("tasks");
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
const first = store.getState().office.refetchTrigger;
|
||||
|
||||
store.getState().setOfficeRefetchTrigger("dashboard");
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
const second = store.getState().office.refetchTrigger;
|
||||
|
||||
expect(first?.types).toEqual(["tasks"]);
|
||||
expect(second?.types).toEqual(["dashboard"]);
|
||||
expect(second).not.toBe(first);
|
||||
});
|
||||
});
|
||||
@@ -42,7 +42,7 @@ export const defaultOfficeState: OfficeSliceState = {
|
||||
},
|
||||
meta: null,
|
||||
isLoading: false,
|
||||
refetchTriggers: {},
|
||||
refetchTrigger: null,
|
||||
routing: {
|
||||
byWorkspace: {},
|
||||
knownProviders: [],
|
||||
@@ -220,6 +220,11 @@ function createTaskActions(set: SetFn) {
|
||||
}
|
||||
|
||||
function createMiscActions(set: SetFn) {
|
||||
// Per-store closure state for batching setOfficeRefetchTrigger calls; see
|
||||
// that action below.
|
||||
let pendingRefetchTypes: string[] = [];
|
||||
let refetchFlushScheduled = false;
|
||||
|
||||
return {
|
||||
setApprovals: (approvals: OfficeSlice["office"]["approvals"]) =>
|
||||
set((draft) => {
|
||||
@@ -271,11 +276,28 @@ function createMiscActions(set: SetFn) {
|
||||
set((draft) => {
|
||||
draft.office.isLoading = loading;
|
||||
}),
|
||||
setOfficeRefetchTrigger: (type: string) =>
|
||||
set((draft) => {
|
||||
const prev = draft.office.refetchTriggers[type] ?? 0;
|
||||
draft.office.refetchTriggers[type] = prev + 1;
|
||||
}),
|
||||
// Same-tick calls (e.g. `task:<id>` immediately followed by `dashboard`
|
||||
// for one WS event) are buffered and flushed as a single trigger object
|
||||
// in a microtask. Without this, React's automatic batching of the
|
||||
// resulting store updates only ever lets a listener observe the *last*
|
||||
// write in the tick — any earlier type in the same burst is silently
|
||||
// dropped for a listener whose effect hasn't run in between the two
|
||||
// `set()` calls.
|
||||
setOfficeRefetchTrigger: (type: string) => {
|
||||
if (!pendingRefetchTypes.includes(type)) {
|
||||
pendingRefetchTypes.push(type);
|
||||
}
|
||||
if (refetchFlushScheduled) return;
|
||||
refetchFlushScheduled = true;
|
||||
queueMicrotask(() => {
|
||||
const types = pendingRefetchTypes;
|
||||
pendingRefetchTypes = [];
|
||||
refetchFlushScheduled = false;
|
||||
set((draft) => {
|
||||
draft.office.refetchTrigger = { types, timestamp: Date.now() };
|
||||
});
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -504,6 +504,18 @@ import type {
|
||||
|
||||
// --- Slice state & actions ---
|
||||
|
||||
/**
|
||||
* One or more refetch types fired together. A single WS event routinely
|
||||
* triggers several types in the same synchronous handler (e.g. `task:<id>`
|
||||
* and `dashboard`); batching them into one trigger object is what lets
|
||||
* `useOfficeRefetch` observe every type instead of only the last write to
|
||||
* survive React's automatic batching of same-tick store updates.
|
||||
*/
|
||||
export type OfficeRefetchTrigger = {
|
||||
types: string[];
|
||||
timestamp: number;
|
||||
};
|
||||
|
||||
/**
|
||||
* Office collections that belong to one workspace, stored per workspace id
|
||||
* rather than as a single current value.
|
||||
@@ -538,12 +550,12 @@ export type OfficeSliceState = {
|
||||
tasks: TasksState;
|
||||
meta: OfficeMeta | null;
|
||||
isLoading: boolean;
|
||||
// Per-type counters rather than one "last trigger" value: a single WS
|
||||
// handler often fires several distinct types in the same synchronous
|
||||
// call (e.g. `task:${id}` then `dashboard`), and React/Zustand coalesce
|
||||
// those into one render, so a shared last-value field would only ever
|
||||
// let the final type's subscribers see a change. See useOfficeRefetch.
|
||||
refetchTriggers: Record<string, number>;
|
||||
// A single WS handler often fires several distinct types in the same
|
||||
// synchronous call (e.g. `task:${id}` then `dashboard`); batching them
|
||||
// into one OfficeRefetchTrigger object per tick is what lets every
|
||||
// matching subscriber observe its type instead of only the last write
|
||||
// surviving React's automatic batching. See useOfficeRefetch.
|
||||
refetchTrigger: OfficeRefetchTrigger | null;
|
||||
routing: RoutingState;
|
||||
providerHealth: ProviderHealthSliceState;
|
||||
runAttempts: RunAttemptsState;
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { StoreApi } from "zustand";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import { registerOfficeHandlers } from "./office";
|
||||
import {
|
||||
__resetOfficeTaskContentSyncForTests,
|
||||
beginWrite,
|
||||
openFieldEditor,
|
||||
} from "@/lib/state/office-task-content-sync";
|
||||
|
||||
/**
|
||||
* Minimal in-memory store for the office WS handler tests.
|
||||
@@ -13,7 +18,15 @@ function makeStore(activeWorkspaceId: string | null) {
|
||||
const upsertProviderHealth = vi.fn();
|
||||
const appendRunAttempt = vi.fn();
|
||||
const setWorkspaceRouting = vi.fn();
|
||||
const patchTaskInStore = vi.fn();
|
||||
// Mirrors the real patchTaskInStore action's field-merge behavior (minus
|
||||
// status normalization, which these handler tests don't exercise) so the
|
||||
// content-editing guard tests can assert on actual post-patch item state,
|
||||
// not just on how the mock was called.
|
||||
const patchTaskInStore = vi.fn((taskId: string, patch: Record<string, unknown>) => {
|
||||
const items = state.office.tasks.items as Array<{ id: string } & Record<string, unknown>>;
|
||||
const idx = items.findIndex((t) => t.id === taskId);
|
||||
if (idx >= 0) Object.assign(items[idx], patch);
|
||||
});
|
||||
let state = {
|
||||
workspaces: { items: [], activeId: activeWorkspaceId },
|
||||
office: { tasks: { items: [] } },
|
||||
@@ -54,6 +67,7 @@ function makeStore(activeWorkspaceId: string | null) {
|
||||
}
|
||||
|
||||
const ACTIVE_WS = "ws-current";
|
||||
const ACTION_TASK_UPDATED = "office.task.updated";
|
||||
|
||||
describe("office WS handler — workspace filter", () => {
|
||||
beforeEach(() => {
|
||||
@@ -142,11 +156,11 @@ describe("office WS handler — task field updates", () => {
|
||||
it("maps the producer state field on office.task.updated", () => {
|
||||
const { store, patchTaskInStore } = makeStore(ACTIVE_WS);
|
||||
const handlers = registerOfficeHandlers(store);
|
||||
const handler = handlers["office.task.updated"]!;
|
||||
const handler = handlers[ACTION_TASK_UPDATED]!;
|
||||
|
||||
handler({
|
||||
type: "notification",
|
||||
action: "office.task.updated",
|
||||
action: ACTION_TASK_UPDATED,
|
||||
payload: { workspace_id: ACTIVE_WS, task_id: "t-1", state: "SCHEDULING" },
|
||||
} as Parameters<typeof handler>[0]);
|
||||
|
||||
@@ -226,6 +240,109 @@ describe("office WS handler — task field updates", () => {
|
||||
});
|
||||
});
|
||||
|
||||
const REMOTE_TITLE = "Remote title";
|
||||
const REMOTE_DESCRIPTION = "Remote description";
|
||||
|
||||
describe("office WS handler — content-editing guard (AC-61)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
__resetOfficeTaskContentSyncForTests();
|
||||
});
|
||||
|
||||
function seedTaskItem(store: StoreApi<AppState>) {
|
||||
store.setState(
|
||||
() =>
|
||||
({
|
||||
office: {
|
||||
tasks: {
|
||||
items: [
|
||||
{ id: "t-1", title: "Old title", description: "Old description", status: "todo" },
|
||||
],
|
||||
},
|
||||
},
|
||||
}) as unknown as Partial<AppState>,
|
||||
);
|
||||
}
|
||||
|
||||
it("leaves a guarded field's store entry unchanged, including when the write originates in this handler", () => {
|
||||
const { store } = makeStore(ACTIVE_WS);
|
||||
seedTaskItem(store);
|
||||
openFieldEditor("t-1", "title");
|
||||
const handlers = registerOfficeHandlers(store);
|
||||
const handler = handlers[ACTION_TASK_UPDATED]!;
|
||||
|
||||
handler({
|
||||
type: "notification",
|
||||
action: ACTION_TASK_UPDATED,
|
||||
payload: {
|
||||
workspace_id: ACTIVE_WS,
|
||||
task_id: "t-1",
|
||||
title: REMOTE_TITLE,
|
||||
description: REMOTE_DESCRIPTION,
|
||||
status: "in_progress",
|
||||
},
|
||||
} as Parameters<typeof handler>[0]);
|
||||
|
||||
const item = store.getState().office.tasks.items[0] as {
|
||||
title: string;
|
||||
description: string;
|
||||
status: string;
|
||||
};
|
||||
expect(item.title).toBe("Old title");
|
||||
expect(item.description).toBe("Remote description");
|
||||
expect(item.status).toBe("in_progress");
|
||||
});
|
||||
|
||||
it("leaves both fields' store entries unchanged while a write is in flight for each", () => {
|
||||
const { store } = makeStore(ACTIVE_WS);
|
||||
seedTaskItem(store);
|
||||
beginWrite("t-1", "title", 1);
|
||||
beginWrite("t-1", "description", 1);
|
||||
const handlers = registerOfficeHandlers(store);
|
||||
const handler = handlers[ACTION_TASK_UPDATED]!;
|
||||
|
||||
handler({
|
||||
type: "notification",
|
||||
action: ACTION_TASK_UPDATED,
|
||||
payload: {
|
||||
workspace_id: ACTIVE_WS,
|
||||
task_id: "t-1",
|
||||
title: REMOTE_TITLE,
|
||||
description: REMOTE_DESCRIPTION,
|
||||
},
|
||||
} as Parameters<typeof handler>[0]);
|
||||
|
||||
const item = store.getState().office.tasks.items[0] as { title: string; description: string };
|
||||
expect(item.title).toBe("Old title");
|
||||
expect(item.description).toBe("Old description");
|
||||
});
|
||||
|
||||
it("applies both fields normally when neither is guarded", () => {
|
||||
const { store } = makeStore(ACTIVE_WS);
|
||||
seedTaskItem(store);
|
||||
const handlers = registerOfficeHandlers(store);
|
||||
const handler = handlers[ACTION_TASK_UPDATED]!;
|
||||
|
||||
handler({
|
||||
type: "notification",
|
||||
action: ACTION_TASK_UPDATED,
|
||||
payload: {
|
||||
workspace_id: ACTIVE_WS,
|
||||
task_id: "t-1",
|
||||
title: REMOTE_TITLE,
|
||||
description: REMOTE_DESCRIPTION,
|
||||
},
|
||||
} as Parameters<typeof handler>[0]);
|
||||
|
||||
const item = store.getState().office.tasks.items[0] as { title: string; description: string };
|
||||
expect(item.title).toBe("Remote title");
|
||||
expect(item.description).toBe("Remote description");
|
||||
});
|
||||
});
|
||||
|
||||
describe("office WS handler — approval flow events", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -7,6 +7,7 @@ import type {
|
||||
ProviderHealth,
|
||||
RouteAttempt,
|
||||
} from "@/lib/state/slices/office/types";
|
||||
import { isFieldGuarded } from "@/lib/state/office-task-content-sync";
|
||||
|
||||
/**
|
||||
* Registers WS handlers for office domain events.
|
||||
@@ -61,7 +62,7 @@ function buildTaskHandlers(
|
||||
if (!isCurrentWorkspace(p)) return;
|
||||
const taskId = (p.task_id ?? p.id) as string | undefined;
|
||||
if (!taskId) return;
|
||||
updateTaskStatus(taskId, normalizeIssueFields(p));
|
||||
updateTaskStatus(taskId, stripGuardedContentFields(taskId, normalizeIssueFields(p)));
|
||||
// Per-task channel — the detail page subscribes to refresh the
|
||||
// server-authoritative task DTO after a property mutation.
|
||||
triggerRefetch(`task:${taskId}`);
|
||||
@@ -298,3 +299,18 @@ function normalizeIssueFields(p: Record<string, unknown>): Record<string, unknow
|
||||
if (p.assignee_agent_profile_id != null) out.assigneeAgentProfileId = p.assignee_agent_profile_id;
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* AC-61: a guarded field (open editor or unresolved write) must not be
|
||||
* overwritten by this unguarded WS fast path, even though this handler is
|
||||
* shared with surfaces (list/board cards) where no editor is ever open.
|
||||
*/
|
||||
function stripGuardedContentFields(
|
||||
taskId: string,
|
||||
fields: Record<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
const out = { ...fields };
|
||||
if (isFieldGuarded(taskId, "title")) delete out.title;
|
||||
if (isFieldGuarded(taskId, "description")) delete out.description;
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"addApprover": "+ Add approver",
|
||||
"addAtLeastOneSource": "Add at least one source.",
|
||||
"addBlocker": "+ Add blocker",
|
||||
"addDescription": "Add a description...",
|
||||
"addDetailsAboutThisChange": "Add details about this change...",
|
||||
"addFolder": "Add folder",
|
||||
"addFolderExecutorUnknown": "Waiting to confirm this task's executor supports folders",
|
||||
@@ -359,6 +360,7 @@
|
||||
"describeYourChanges": "Describe your changes...",
|
||||
"description": "description",
|
||||
"description2": "Description",
|
||||
"descriptionRunningNote": "A session is running for this task. Saving now will not interrupt it.",
|
||||
"destroyTerminalConfirm": "Destroy \"{{label}}\"? This kills the PTY and removes it.",
|
||||
"detach": "Detach",
|
||||
"detachFromParent": "Detach from parent",
|
||||
@@ -379,6 +381,8 @@
|
||||
"discardChanges2": "Discard changes",
|
||||
"discardChangesToFile": "This will permanently discard all changes to <2>{{file}}</2>. This action cannot be undone.",
|
||||
"discardCount": "Discard {{selectionCount}}",
|
||||
"discardDescriptionBody": "Your changes to the description have not been saved. Discard them?",
|
||||
"discardDescriptionTitle": "Discard description changes?",
|
||||
"discardFailed": "Discard failed",
|
||||
"dismiss": "Dismiss",
|
||||
"displayNameOptional": "Display name (optional)",
|
||||
@@ -647,6 +651,7 @@
|
||||
"kandevUpdateTask": "Kandev: Update Task",
|
||||
"kandevUpdateTaskPlan": "Kandev: Update Task Plan",
|
||||
"kandevWriteTaskDocument": "Kandev: Write Task Document",
|
||||
"keepEditing": "Keep editing",
|
||||
"keepOpen": "Keep Open",
|
||||
"keepRunning": "Keep running",
|
||||
"keyAndContentAreRequired": "Key and content are required",
|
||||
@@ -1562,6 +1567,7 @@
|
||||
"title2": "{{longName}} title",
|
||||
"title3": "{{longName}} title...",
|
||||
"titleOptional": "Title (optional)",
|
||||
"titleRequired": "Title can't be empty.",
|
||||
"toFocus": "to focus",
|
||||
"todos": "Todos",
|
||||
"todosCompleted": "{{completed}}/{{total}} completed",
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"addApprover": "+ Àďď àƥƥŕōvēŕ",
|
||||
"addAtLeastOneSource": "Àďď àţ ĺēàśţ ōńē śōũŕćē.",
|
||||
"addBlocker": "+ Àďď ƀĺōćķēŕ",
|
||||
"addDescription": "Àďď à ďēśćŕĩƥţĩōń...",
|
||||
"addDetailsAboutThisChange": "Àďď ďēţàĩĺś àƀōũţ ţĥĩś ćĥàńĝē...",
|
||||
"addFolder": "Àďď ƒōĺďēŕ",
|
||||
"addFolderExecutorUnknown": "Ŵàĩţĩńĝ ţō ćōńƒĩŕḿ ţĥĩś ţàśķ'ś ēxēćũţōŕ śũƥƥōŕţś ƒōĺďēŕś",
|
||||
@@ -359,6 +360,7 @@
|
||||
"describeYourChanges": "Ďēśćŕĩƀē ŷōũŕ ćĥàńĝēś...",
|
||||
"description": "ďēśćŕĩƥţĩōń",
|
||||
"description2": "Ďēśćŕĩƥţĩōń",
|
||||
"descriptionRunningNote": "À śēśśĩōń ĩś ŕũńńĩńĝ ƒōŕ ţĥĩś ţàśķ. Śàvĩńĝ ńōŵ ŵĩĺĺ ńōţ ĩńţēŕŕũƥţ ĩţ.",
|
||||
"destroyTerminalConfirm": "Ďēśţŕōŷ \"{{label}}\"? Ţĥĩś ķĩĺĺś ţĥē ƤŢŶ àńď ŕēḿōvēś ĩţ.",
|
||||
"detach": "Ďēţàćĥ",
|
||||
"detachFromParent": "Ďēţàćĥ ƒŕōḿ ƥàŕēńţ",
|
||||
@@ -379,6 +381,8 @@
|
||||
"discardChanges2": "Ďĩśćàŕď ćĥàńĝēś",
|
||||
"discardChangesToFile": "Ţĥĩś ŵĩĺĺ ƥēŕḿàńēńţĺŷ ďĩśćàŕď àĺĺ ćĥàńĝēś ţō <2>{{file}}</2>. Ţĥĩś àćţĩōń ćàńńōţ ƀē ũńďōńē.",
|
||||
"discardCount": "Ďĩśćàŕď {{selectionCount}}",
|
||||
"discardDescriptionBody": "Ŷōũŕ ćĥàńĝēś ţō ţĥē ďēśćŕĩƥţĩōń ĥàvē ńōţ ƀēēń śàvēď. Ďĩśćàŕď ţĥēḿ?",
|
||||
"discardDescriptionTitle": "Ďĩśćàŕď ďēśćŕĩƥţĩōń ćĥàńĝēś?",
|
||||
"discardFailed": "Ďĩśćàŕď ƒàĩĺēď",
|
||||
"dismiss": "Ďĩśḿĩśś",
|
||||
"displayNameOptional": "Ďĩśƥĺàŷ ńàḿē (ōƥţĩōńàĺ)",
|
||||
@@ -647,6 +651,7 @@
|
||||
"kandevUpdateTask": "Ķàńďēv: Ũƥďàţē Ţàśķ",
|
||||
"kandevUpdateTaskPlan": "Ķàńďēv: Ũƥďàţē Ţàśķ Ƥĺàń",
|
||||
"kandevWriteTaskDocument": "Ķàńďēv: Ŵŕĩţē Ţàśķ Ďōćũḿēńţ",
|
||||
"keepEditing": "Ķēēƥ ēďĩţĩńĝ",
|
||||
"keepOpen": "Ķēēƥ Ōƥēń",
|
||||
"keepRunning": "Ķēēƥ ŕũńńĩńĝ",
|
||||
"keyAndContentAreRequired": "Ķēŷ àńď ćōńţēńţ àŕē ŕēqũĩŕēď",
|
||||
@@ -1562,6 +1567,7 @@
|
||||
"title2": "{{longName}} ţĩţĺē",
|
||||
"title3": "{{longName}} ţĩţĺē...",
|
||||
"titleOptional": "Ţĩţĺē (ōƥţĩōńàĺ)",
|
||||
"titleRequired": "Ţĩţĺē ćàń'ţ ƀē ēḿƥţŷ.",
|
||||
"toFocus": "ţō ƒōćũś",
|
||||
"todos": "Ţōďōś",
|
||||
"todosCompleted": "{{completed}}/{{total}} ćōḿƥĺēţēď",
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"addApprover": "+ Adicionar aprovador",
|
||||
"addAtLeastOneSource": "Adicione pelo menos uma fonte.",
|
||||
"addBlocker": "+ Adicionar bloqueio",
|
||||
"addDescription": "Adicionar uma descrição...",
|
||||
"addDetailsAboutThisChange": "Adicione detalhes sobre esta alteração...",
|
||||
"addFolder": "Adicionar pasta",
|
||||
"addFolderExecutorUnknown": "O executor desta tarefa ainda não foi confirmado",
|
||||
@@ -344,6 +345,7 @@
|
||||
"describeYourChanges": "Descreva as suas alterações...",
|
||||
"description": "descrição",
|
||||
"description2": "Descrição",
|
||||
"descriptionRunningNote": "Há uma sessão em execução para esta tarefa. Guardar agora não a irá interromper.",
|
||||
"destroyTerminalConfirm": "Destruir \"{{label}}\"? Isto termina o PTY e remove-o.",
|
||||
"detach": "Separar",
|
||||
"detachFromParent": "Separar da tarefa-mãe",
|
||||
@@ -364,6 +366,8 @@
|
||||
"discardChanges2": "Descartar as alterações",
|
||||
"discardChangesToFile": "Isto descarta permanentemente todas as alterações a <2>{{file}}</2>. Esta ação não pode ser anulada.",
|
||||
"discardCount": "Descartar {{selectionCount}}",
|
||||
"discardDescriptionBody": "As suas alterações à descrição não foram guardadas. Descartá-las?",
|
||||
"discardDescriptionTitle": "Descartar as alterações à descrição?",
|
||||
"discardFailed": "Não foi possível descartar",
|
||||
"dismiss": "Dispensar",
|
||||
"displayNameOptional": "Nome a apresentar (opcional)",
|
||||
@@ -632,6 +636,7 @@
|
||||
"kandevUpdateTask": "Kandev: Atualizar tarefa",
|
||||
"kandevUpdateTaskPlan": "Kandev: Atualizar plano da tarefa",
|
||||
"kandevWriteTaskDocument": "Kandev: Escrever documento da tarefa",
|
||||
"keepEditing": "Continuar a editar",
|
||||
"keepOpen": "Manter aberto",
|
||||
"keepRunning": "Manter em execução",
|
||||
"keyAndContentAreRequired": "A chave e o conteúdo são obrigatórios",
|
||||
@@ -1517,6 +1522,7 @@
|
||||
"title2": "Título do {{longName}}",
|
||||
"title3": "Título do {{longName}}...",
|
||||
"titleOptional": "Título (opcional)",
|
||||
"titleRequired": "O título não pode estar vazio.",
|
||||
"toFocus": "para focar",
|
||||
"todos": "Tarefas por fazer",
|
||||
"todosCompleted": "{{completed}}/{{total}} concluídas",
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"addApprover": "+ 添加审批人",
|
||||
"addAtLeastOneSource": "请至少添加一个来源。",
|
||||
"addBlocker": "+ 添加阻塞项",
|
||||
"addDescription": "添加描述…",
|
||||
"addDetailsAboutThisChange": "添加此变更的详细说明…",
|
||||
"addFolder": "添加文件夹",
|
||||
"addFolderExecutorUnknown": "尚未确认此任务的执行器",
|
||||
@@ -344,6 +345,7 @@
|
||||
"describeYourChanges": "描述你的变更…",
|
||||
"description": "描述",
|
||||
"description2": "描述",
|
||||
"descriptionRunningNote": "此任务有一个正在运行的会话。现在保存不会中断它。",
|
||||
"destroyTerminalConfirm": "销毁“{{label}}”？这将终止 PTY 并移除它。",
|
||||
"detach": "分离",
|
||||
"detachFromParent": "从父任务分离",
|
||||
@@ -364,6 +366,8 @@
|
||||
"discardChanges2": "丢弃更改",
|
||||
"discardChangesToFile": "将永久丢弃对 <2>{{file}}</2> 的全部更改。此操作无法撤销。",
|
||||
"discardCount": "丢弃 {{selectionCount}}",
|
||||
"discardDescriptionBody": "对描述的更改尚未保存。是否丢弃？",
|
||||
"discardDescriptionTitle": "丢弃描述更改？",
|
||||
"discardFailed": "丢弃失败",
|
||||
"dismiss": "关闭",
|
||||
"displayNameOptional": "显示名称（可选）",
|
||||
@@ -631,6 +635,7 @@
|
||||
"kandevUpdateTask": "Kandev：更新任务",
|
||||
"kandevUpdateTaskPlan": "Kandev：更新任务计划",
|
||||
"kandevWriteTaskDocument": "Kandev：写入任务文档",
|
||||
"keepEditing": "继续编辑",
|
||||
"keepOpen": "保持打开",
|
||||
"keepRunning": "继续运行",
|
||||
"keyAndContentAreRequired": "键与内容为必填",
|
||||
@@ -1516,6 +1521,7 @@
|
||||
"title2": "{{longName}} 标题",
|
||||
"title3": "{{longName}} 标题…",
|
||||
"titleOptional": "标题（可选）",
|
||||
"titleRequired": "标题不能为空。",
|
||||
"toFocus": "以聚焦",
|
||||
"todos": "待办事项",
|
||||
"todosCompleted": "已完成 {{completed}}/{{total}}",
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"addApprover": "+ 新增審批人",
|
||||
"addAtLeastOneSource": "請至少新增一個來源。",
|
||||
"addBlocker": "+ 新增阻塞項",
|
||||
"addDescription": "新增描述…",
|
||||
"addDetailsAboutThisChange": "新增此變更的詳細說明…",
|
||||
"addFolder": "新增資料夾",
|
||||
"addFolderExecutorUnknown": "尚未確認此任務的執行器",
|
||||
@@ -344,6 +345,7 @@
|
||||
"describeYourChanges": "描述你的變更…",
|
||||
"description": "描述",
|
||||
"description2": "描述",
|
||||
"descriptionRunningNote": "此任務有一個正在執行的工作階段。現在儲存不會中斷它。",
|
||||
"destroyTerminalConfirm": "銷燬「{{label}}」？這將終止 PTY 並移除它。",
|
||||
"detach": "分離",
|
||||
"detachFromParent": "從父任務分離",
|
||||
@@ -364,6 +366,8 @@
|
||||
"discardChanges2": "丟棄更改",
|
||||
"discardChangesToFile": "將永久丟棄對 <2>{{file}}</2> 的全部更改。此操作無法撤銷。",
|
||||
"discardCount": "丟棄 {{selectionCount}}",
|
||||
"discardDescriptionBody": "對描述的更改尚未儲存。是否丟棄？",
|
||||
"discardDescriptionTitle": "丟棄描述更改？",
|
||||
"discardFailed": "丟棄失敗",
|
||||
"dismiss": "關閉",
|
||||
"displayNameOptional": "顯示名稱（可選）",
|
||||
@@ -631,6 +635,7 @@
|
||||
"kandevUpdateTask": "Kandev：更新任務",
|
||||
"kandevUpdateTaskPlan": "Kandev：更新任務計劃",
|
||||
"kandevWriteTaskDocument": "Kandev：寫入任務文檔",
|
||||
"keepEditing": "繼續編輯",
|
||||
"keepOpen": "保持開啟",
|
||||
"keepRunning": "繼續執行",
|
||||
"keyAndContentAreRequired": "鍵與內容為必填",
|
||||
@@ -1516,6 +1521,7 @@
|
||||
"title2": "{{longName}} 標題",
|
||||
"title3": "{{longName}} 標題…",
|
||||
"titleOptional": "標題（可選）",
|
||||
"titleRequired": "標題不能為空。",
|
||||
"toFocus": "以聚焦",
|
||||
"todos": "待辦事項",
|
||||
"todosCompleted": "已完成 {{completed}}/{{total}}",
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"addApprover": "+ 新增審批人",
|
||||
"addAtLeastOneSource": "請至少新增一個來源。",
|
||||
"addBlocker": "+ 新增阻塞項",
|
||||
"addDescription": "新增描述…",
|
||||
"addDetailsAboutThisChange": "新增此變更的詳細說明…",
|
||||
"addFolder": "新增資料夾",
|
||||
"addFolderExecutorUnknown": "尚未確認此任務的執行器",
|
||||
@@ -344,6 +345,7 @@
|
||||
"describeYourChanges": "描述你的變更…",
|
||||
"description": "描述",
|
||||
"description2": "描述",
|
||||
"descriptionRunningNote": "此任務有一個正在執行的工作階段。現在儲存不會中斷它。",
|
||||
"destroyTerminalConfirm": "銷燬「{{label}}」？這將終止 PTY 並移除它。",
|
||||
"detach": "分離",
|
||||
"detachFromParent": "從父任務分離",
|
||||
@@ -364,6 +366,8 @@
|
||||
"discardChanges2": "丟棄更改",
|
||||
"discardChangesToFile": "將永久丟棄對 <2>{{file}}</2> 的全部更改。此操作無法撤銷。",
|
||||
"discardCount": "丟棄 {{selectionCount}}",
|
||||
"discardDescriptionBody": "對描述的更改尚未儲存。是否丟棄？",
|
||||
"discardDescriptionTitle": "丟棄描述更改？",
|
||||
"discardFailed": "丟棄失敗",
|
||||
"dismiss": "關閉",
|
||||
"displayNameOptional": "顯示名稱（可選）",
|
||||
@@ -631,6 +635,7 @@
|
||||
"kandevUpdateTask": "Kandev：更新任務",
|
||||
"kandevUpdateTaskPlan": "Kandev：更新任務計劃",
|
||||
"kandevWriteTaskDocument": "Kandev：寫入任務檔案",
|
||||
"keepEditing": "繼續編輯",
|
||||
"keepOpen": "保持開啟",
|
||||
"keepRunning": "繼續執行",
|
||||
"keyAndContentAreRequired": "鍵與內容為必填",
|
||||
@@ -1516,6 +1521,7 @@
|
||||
"title2": "{{longName}} 標題",
|
||||
"title3": "{{longName}} 標題…",
|
||||
"titleOptional": "標題（可選）",
|
||||
"titleRequired": "標題不能為空。",
|
||||
"toFocus": "以聚焦",
|
||||
"todos": "待辦事項",
|
||||
"todosCompleted": "已完成 {{completed}}/{{total}}",
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -70,7 +70,7 @@ This spec consolidates the office task surface: lifecycle, parent/child handoffs
|
||||
|
||||
### D. Inline editable properties
|
||||
|
||||
- Every property row that holds an enumerable, lookup-able, or free-text value is editable inline by clicking the value: Status, Priority, Assignee, Project, Parent task, Blocked by, Reviewers, Approvers, Labels.
|
||||
- Every property row that holds an enumerable, lookup-able, or free-text value is editable inline by clicking the value: Status, Priority, Assignee, Project, Parent task, Blocked by, Reviewers, Approvers, Labels, Title, Description.
|
||||
- Read-only rows stay read-only: Created by, Started, Completed, Created, Updated, Tree cost metrics. They reflect events, not user intent.
|
||||
- Edits are **optimistic**: the new value renders immediately; on API failure the row reverts to the prior value and a toast surfaces the error.
|
||||
- Pickers are searchable when the candidate list could exceed roughly 10 items (Assignee, Project, Parent, Blocked-by). Status and Priority pickers use a fixed list. The threshold for switching to a search input is around 8 candidates.
|
||||
@@ -241,7 +241,7 @@ Multiple rows per `(task, decider, role)` in `office_task_approval_decisions` ar
|
||||
- `POST /tasks/:id/approve { comment?: string }` - caller approves. 403 if caller is not in reviewers or approvers. Returns the created decision row.
|
||||
- `POST /tasks/:id/request-changes { comment: string }` - caller requests changes. Comment required. Same 403 rule. Returns the decision row.
|
||||
- `GET /tasks/:id/decisions` - current (non-superseded) decisions. Also surfaced on the task detail endpoint as a `decisions: TaskDecision[]` field.
|
||||
- `PATCH /tasks/:id` - inline property edits (status, priority, assignee, project, parent, blocked-by, reviewers, approvers, labels). Returns the updated task.
|
||||
- `PATCH /tasks/:id` - inline property edits (status, priority, assignee, project, parent, blocked-by, reviewers, approvers, labels). Returns the updated task. Title and description are not fields of this endpoint; they are edited through `PATCH /api/v1/tasks/:id`.
|
||||
- `POST /tasks/:id/blockers { blocker_task_id }` - returns 400 with a body carrying the cycle path when the addition would create a cycle.
|
||||
- `DELETE /tasks/:id/reviewers/:agentId` / `DELETE /tasks/:id/approvers/:agentId` - removes the participant AND terminates that agent's session for the task.
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
"template": 8192
|
||||
},
|
||||
"legacy_size_exceptions": {
|
||||
"docs/specs/office-task-content-editing/spec.md": 84103,
|
||||
"docs/specs/task-cost-ledger/spec.md": 91931
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user