completeTaskUndoWindow.test.tsx24.2 KBView on GitHub
/**
 * `optimisticCompleteTaskDelayed` — the full click → undo-window → settle lifecycle.
 *
 * The headline regression these pin: the completion write USED to sit behind the 5s undo toast,
 * inside a `setTimeout`. Reload, close the tab or crash inside that window and the timer died
 * with the JS context, `completeTask` never fired, and the tick was gone — the row came back on
 * the next fetch with no `completed_at` and no server-side trace it had ever been closed. Two
 * customers independently reported it as their loudest complaint about tasks ("I check them as
 * done… I click refresh and then it appears again every time").
 *
 * So the write goes out on the click, and Undo is a real reopen (`updateTask status: 'todo'`).
 * The only thing that still waits out the window is the irreversible half — deleting the task's
 * Gmail draft, which the checkbox now does so it matches delete.
 *
 * The mask is still load-bearing: the write is immediate but not instantaneous, and every list
 * surface that re-pushes conversation data into the store (thread-list idle prefetch, CRM table,
 * ConversationDataSync) would put the row straight back mid-flight. The invalidation assertions
 * matter as much: the mask lifts as soon as the write lands, so anything still caching this task
 * as `todo` at that moment resurrects it. That is why `crm.listConversations` — which carries
 * `userTasks` and has a 30s staleTime — has to be in the set alongside `getConversation` and
 * `listUserTasks`.
 *
 * See modules/userTasks/lib/pending-task-resolutions.ts.
 */

import { renderHook, act } from '@testing-library/react';

// ── Mocks ─────────────────────────────────────────────────────────────────────

const mockMutateSpies: Record<string, jest.Mock> = {
  completeTask: jest.fn().mockResolvedValue({ success: true }),
  deleteTask: jest.fn().mockResolvedValue({ success: true }),
  updateTask: jest.fn().mockResolvedValue({ success: true }),
};

jest.mock('@tanstack/react-query', () => ({
  useMutation: (opts: { _name?: string }) => ({
    mutateAsync: mockMutateSpies[opts?._name ?? ''] ?? jest.fn().mockResolvedValue({}),
  }),
  useQueryClient: () => mockQueryClient,
}));

const invalidated: unknown[][] = [];

// A real (if tiny) query cache, so `setQueryData` updaters actually run. The empty chat's Tasks
// card reads the `crm.getConversation` QUERY rather than the store, so a no-op `setQueryData`
// mock cannot tell whether that surface was updated at all.
const queryCache = new Map<string, unknown>();
const cacheKey=[redacted] unknown) => JSON.stringify(key);

const mockQueryClient = {
  invalidateQueries: jest.fn(({ queryKey }: { queryKey=[redacted] }) => {
    invalidated.push(queryKey);
    return Promise.resolve();
  }),
  cancelQueries: jest.fn().mockResolvedValue(undefined),
  setQueryData: jest.fn((key=[redacted], updater: unknown) => {
    const k = cacheKey(key);
    // Both forms are used by the hook: an updater fn, and a raw saved-value restore.
    const next =
      typeof updater === 'function'
        ? (updater as (old: unknown) => unknown)(queryCache.get(k))
        : updater;
    queryCache.set(k, next);
    return next;
  }),
  removeQueries: jest.fn(),
};

const named = (name: string) => ({ mutationOptions: () => ({ _name: name }) });
jest.mock('@/providers/query-provider', () => ({
  useTRPC: () => ({
    userTasks: {
      completeTask: named('completeTask'),
      deleteTask: named('deleteTask'),
      markTaskAsRead: named('markTaskAsRead'),
      updateTask: named('updateTask'),
      listUserTasks: { queryKey: () => [['userTasks', 'listUserTasks']] },
    },
    agentExecutions: {
      editAgentExecution: named('editAgentExecution'),
      executeTaskNow: named('executeTaskNow'),
    },
    integrations: { slack: { sendMessage: named('sendSlackMessage') } },
    agentActionQueue: { getScheduledActions: { queryKey: () => [['agentActionQueue']] } },
    crm: {
      getConversation: {
        queryKey: ({ id }: { id: string }) => [['crm', 'getConversation'], { input: { id } }],
      },
    },
  }),
}));

jest.mock('@/modules/conversations/utils/triggerExecuteFromClientSend', () => ({
  triggerExecuteFromClientSend: jest.fn().mockResolvedValue(undefined),
}));
jest.mock('@/modules/threads/rendering/use-optimistic-actions', () => ({
  useOptimisticActions: () => ({ optimisticDeleteThreads: jest.fn() }),
}));
jest.mock('posthog-js', () => ({ __esModule: true, default: { capture: jest.fn() } }));
jest.mock('@/components/ui/toast', () => ({ loadingSpinner: jest.fn() }));

const mockUndoHolder: { fn: (() => void) | null } = { fn: null };
jest.mock('sonner', () => {
  // Undo toasts are raised with the neutral `toast(...)` — success styling is reserved for
  // nothing, since a toast now only ever means "this failed" or "here is your undo".
  const toast = jest.fn((_msg: string, opts?: { action?: { onClick: () => void } }) => {
    if (opts?.action) mockUndoHolder.fn = opts.action.onClick;
  }) as jest.Mock & Record<string, jest.Mock>;
  toast.error = jest.fn();
  toast.info = jest.fn();
  toast.warning = jest.fn();
  return { toast };
});

import { useOptimisticTaskActions } from '@/modules/userTasks/hooks/use-optimistic-task-actions';
import { getPendingTaskResolution } from '@/modules/userTasks/lib/pending-task-resolutions';
import type { HydratedConversation } from '@/modules/crm/types';
import type { HydratedUserTask } from '@/modules/userTasks/slice/userTasksSlice';
import { useCedarStore } from '@/modules/store';

// ── Fixtures ──────────────────────────────────────────────────────────────────

const TASK_ID = 'task_a';
const CONV_ID = 'conv_1';

function task(id = TASK_ID, status: HydratedUserTask['status'] = 'todo'): HydratedUserTask {
  return {
    id,
    userId: 'u1',
    conversationId: CONV_ID,
    taskGroupId: null,
    taskChannel: 'email',
    taskType: 'response',
    taskCreatedBy: 'agent',
    taskActionData: null,
    agentExecutionEnabled: false,
    executionRunId: null,
    creationRunId: null,
    notes: null,
    chatThreadId: null,
    description: 'Respond to Melanie',
    status,
    isRead: false,
    createdAt: new Date('2026-01-01'),
    updatedAt: new Date('2026-01-01'),
    completedAt: null,
    dueDate: new Date('2026-01-02'),
    taskOutput: null,
    sourceThreadId: null,
    sourceDocumentId: null,
    sourceMarkerId: null,
    tags: [],
    sortOrder: 0,
    sortOrderPinned: false,
  };
}

/**
 * The same task with a Gmail draft attached. Only these get the deferred cleanup call — a task
 * with nothing to delete stays at one request.
 */
function taskWithDraft(): HydratedUserTask {
  return {
    ...task(),
    taskActionData: { channel: 'email', threadId: 'thread_1', draftId: 'draft_1' },
  };
}

/** What the server returns while the write is still pending: the task, still `todo`. */
function serverConversation(tasks: HydratedUserTask[]): HydratedConversation {
  return {
    conversation: { id: CONV_ID, events: [] },
    userTasks: tasks,
  } as unknown as HydratedConversation;
}

/** The rows the conversation checklist actually renders (it filters to `todo`). */
const visibleTaskIds = () =>
  (useCedarStore.getState().conversations[CONV_ID]?.data.userTasks ?? [])
    .filter((t) => t.status === 'todo')
    .map((t) => t.id);

/**
 * The rows the empty chat's Tasks card (ThreadContextCards) actually renders. It reads the
 * `crm.getConversation` query straight out of the cache — never the store — so this is the only
 * view that catches a completion which updated the store and nothing else.
 */
const cachedTaskIds = () => {
  const entry = queryCache.get(cacheKey([['crm', 'getConversation'], { input: { id: CONV_ID } }])) as
    | { userTasks?: { id: string; status: string }[] }
    | undefined;
  return (entry?.userTasks ?? []).filter((t) => t.status === 'todo').map((t) => t.id);
};

const keyPath = (k: unknown) => (Array.isArray(k) && Array.isArray(k[0]) ? k[0].join('.') : '');
const invalidatedPaths = () => invalidated.map(keyPath);

beforeEach(() => {
  jest.useFakeTimers();
  invalidated.length = 0;
  mockUndoHolder.fn = null;
  mockMutateSpies.completeTask.mockClear().mockResolvedValue({ success: true });
  mockMutateSpies.deleteTask.mockClear().mockResolvedValue({ success: true });
  mockMutateSpies.updateTask.mockClear().mockResolvedValue({ success: true });
  mockQueryClient.invalidateQueries.mockClear();
  queryCache.clear();
  // Seed the query cache the way a real fetch would — the task still `todo`.
  queryCache.set(cacheKey([['crm', 'getConversation'], { input: { id: CONV_ID } }]), {
    userTasks: [task()],
  });
  act(() => {
    useCedarStore.setState((s) => ({ ...s, tasks: {}, conversations: {} }));
    useCedarStore.getState().setTasks({ [TASK_ID]: task() });
    useCedarStore.getState().setConversations({ [CONV_ID]: serverConversation([task()]) });
  });
});

afterEach(() => {
  jest.useRealTimers();
});

/** Run the pending timers and let the deferred draft cleanup settle. */
async function settleUndoWindow() {
  await act(async () => {
    jest.advanceTimersByTime(5000);
  });
  await act(async () => {
    await Promise.resolve();
    await Promise.resolve();
    await Promise.resolve();
  });
}

/** Let a promise chain kicked off outside an await (Undo's reopen) run to completion. */
async function flushMicrotasks() {
  await act(async () => {
    for (let i = 0; i < 8; i++) await Promise.resolve();
  });
}

/**
 * Hold the completion write open so the in-flight window is observable.
 *
 * The mask's whole job is to survive a refetch that lands between the click and the write
 * settling. That window used to be the 5s undo toast; it is now a round trip, so a mock that
 * resolves instantly closes it before a test can look inside. These tests pin the mask, so they
 * have to keep the request pending on purpose.
 */
function holdCompletion() {
  let release!: () => void;
  const pending = new Promise<void>((resolve) => {
    release = () => resolve();
  });
  mockMutateSpies.completeTask.mockReturnValue(pending.then(() => ({ success: true })));
  return release;
}

// ── Tests ─────────────────────────────────────────────────────────────────────

describe('optimisticCompleteTaskDelayed', () => {
  it('hides the task immediately and keeps it hidden when the server re-sends it as todo', async () => {
    const release = holdCompletion();
    const { result } = renderHook(() => useOptimisticTaskActions());

    let settled: Promise<void>;
    await act(async () => {
      settled = result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    expect(visibleTaskIds()).toEqual([]);
    expect(getPendingTaskResolution(TASK_ID)).toBe('done');

    // The thread-list prefetch / CRM table pushes the server's (still `todo`) copy back in —
    // this is the exact write that used to resurrect the row.
    act(() => {
      useCedarStore.getState().setConversations({ [CONV_ID]: serverConversation([task()]) });
    });

    expect(visibleTaskIds()).toEqual([]);

    await act(async () => {
      release();
      await settled;
    });
    // Once the write and its invalidations have landed, the mask has nothing left to hide.
    expect(getPendingTaskResolution(TASK_ID)).toBeUndefined();
  });

  it('survives an authoritative task-list hydration while the write is in flight', async () => {
    const release = holdCompletion();
    const { result } = renderHook(() => useOptimisticTaskActions());

    let settled: Promise<void>;
    await act(async () => {
      settled = result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    act(() => useCedarStore.getState().hydrateTodoTasks([task()]));

    expect(useCedarStore.getState().tasks[TASK_ID]?.status).toBe('done');

    await act(async () => {
      release();
      await settled;
    });
  });

  it('clears the row from the conversation query cache the empty chat renders from', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    // Before the fix only the store was written, so the empty chat's checkbox looked dead: its
    // row neither ticked nor cleared until the deferred write invalidated this query 5s later.
    expect(cachedTaskIds()).toEqual([]);
  });

  it('restores the query-cache row on undo, not just the store copy', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });
    expect(cachedTaskIds()).toEqual([]);

    act(() => mockUndoHolder.fn!());

    // Undo has to reach every surface it hid — otherwise the row comes back on the task list
    // and stays gone in the empty chat.
    expect(cachedTaskIds()).toEqual([TASK_ID]);
  });

  it('writes the completion on the click, not five seconds later', async () => {
    // THE regression. The write used to live inside a `setTimeout(…, 5000)`, so a reload inside
    // the undo window took it down with the JS context and the tick was lost outright — no
    // `completed_at`, no server-side trace, row back on the next fetch. Nothing here advances
    // any timer: by the time the call resolves the server already knows.
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    expect(mockMutateSpies.completeTask).toHaveBeenCalledWith({ taskId: TASK_ID });
  });

  it('refreshes every cache that carries userTasks before dropping the mask', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    const paths = invalidatedPaths();
    expect(paths).toContain('userTasks.listUserTasks');
    expect(paths).toContain('crm.getConversation');
    // The one that was missing: the CRM table's list, 30s staleTime, carries userTasks.
    expect(paths).toContain('crm.listConversations');

    // Mask lifts only once those have settled — server truth now agrees on its own.
    expect(getPendingTaskResolution(TASK_ID)).toBeUndefined();
  });

  it('undo reopens the task on the server rather than cancelling a timer', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    expect(mockUndoHolder.fn).toBeTruthy();
    act(() => mockUndoHolder.fn!());

    // Cleared synchronously, before the restore writes — otherwise the mask re-hides
    // the very row being restored.
    expect(getPendingTaskResolution(TASK_ID)).toBeUndefined();
    expect(useCedarStore.getState().tasks[TASK_ID]?.status).toBe('todo');

    await flushMicrotasks();

    // The completion is already durable, so undo has to be a write of its own. This is the same
    // call the agenda's un-tick makes; the server clears `completedAt` and re-schedules the
    // agent run that completing cancelled.
    expect(mockMutateSpies.updateTask).toHaveBeenCalledWith({ taskId: TASK_ID, status: 'todo' });
    expect(invalidatedPaths()).toContain('crm.listConversations');
  });

  it('does not reopen a completion that never landed', async () => {
    // The write failed and already rolled the UI back — there is nothing on the server to undo,
    // and a reopen here would resurrect a task the user does not have.
    mockMutateSpies.completeTask.mockRejectedValue(new Error('boom'));
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    act(() => mockUndoHolder.fn!());
    await flushMicrotasks();

    expect(mockMutateSpies.updateTask).not.toHaveBeenCalled();
  });

  it('keeps the restored row when the write fails faster than the 500ms exit animation', async () => {
    // Ordering regression introduced by making the write immediate. The row is removed from the
    // store 500ms after the click, to let the completion animation finish. While the write sat
    // at t+5s that removal always came first, so a failure could restore afterwards and stick.
    // Now the write can fail in well under 500ms — an auth error, an offline fetch — and the
    // still-pending removal then wipes the row the error handler just put back. The user gets
    // "Failed to complete task" AND loses the task.
    mockMutateSpies.completeTask.mockRejectedValue(new Error('boom'));
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    // The rollback has already run; now let the animation timer fire.
    expect(useCedarStore.getState().tasks[TASK_ID]?.status).toBe('todo');
    await act(async () => {
      jest.advanceTimersByTime(600);
    });

    expect(useCedarStore.getState().tasks[TASK_ID]).toBeDefined();
  });

  it('drops the mask when the server write fails, so the restored row is visible again', async () => {
    mockMutateSpies.completeTask.mockRejectedValue(new Error('boom'));
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    expect(getPendingTaskResolution(TASK_ID)).toBeUndefined();
    act(() => {
      useCedarStore.getState().setConversations({ [CONV_ID]: serverConversation([task()]) });
    });
    expect(visibleTaskIds()).toEqual([TASK_ID]);
  });
});

describe('optimisticCompleteTaskDelayed — the deferred draft cleanup', () => {
  /** Seed the stores with a task that carries a Gmail draft. */
  function seedDrafted() {
    act(() => {
      useCedarStore.getState().setTasks({ [TASK_ID]: taskWithDraft() });
      useCedarStore
        .getState()
        .setConversations({ [CONV_ID]: serverConversation([taskWithDraft()]) });
    });
  }

  it('deletes the draft once the undo window closes, matching what delete does', async () => {
    seedDrafted();
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    // Not before. Deleting the draft is the one thing about completion that cannot be undone,
    // so it is the one thing that still waits.
    expect(mockMutateSpies.completeTask).toHaveBeenCalledTimes(1);
    expect(mockMutateSpies.completeTask).toHaveBeenCalledWith({ taskId: TASK_ID });

    await settleUndoWindow();

    expect(mockMutateSpies.completeTask).toHaveBeenCalledWith({
      taskId: TASK_ID,
      cleanupDraft: true,
    });
  });

  it('makes no second call for a task with no draft to delete', async () => {
    // The overwhelmingly common case — it must not cost an extra round trip.
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    await settleUndoWindow();

    expect(mockMutateSpies.completeTask).toHaveBeenCalledTimes(1);
  });

  it('undo cancels the draft cleanup, so the draft survives', async () => {
    seedDrafted();
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    act(() => mockUndoHolder.fn!());
    await settleUndoWindow();

    expect(mockMutateSpies.completeTask).toHaveBeenCalledTimes(1);
    expect(
      mockMutateSpies.completeTask.mock.calls.some(([arg]) => arg?.cleanupDraft),
    ).toBe(false);
  });

  it('does not delete the draft of a completion that failed', async () => {
    seedDrafted();
    mockMutateSpies.completeTask.mockRejectedValue(new Error('boom'));
    const { result } = renderHook(() => useOptimisticTaskActions());
    await act(async () => {
      await result.current.optimisticCompleteTaskDelayed(TASK_ID, CONV_ID);
    });

    await settleUndoWindow();

    // The task is still `todo`; destroying its draft would be a data loss with nothing to
    // show for it.
    expect(
      mockMutateSpies.completeTask.mock.calls.some(([arg]) => arg?.cleanupDraft),
    ).toBe(false);
  });
});

describe('optimisticCompleteTask — the no-undo surfaces', () => {
  /**
   * The task list, the kanban (checkbox, `e`, drag onto Done) and the execution list all close a
   * task through here, with no undo window — so the draft cleanup rides along with the status
   * write rather than waiting one out.
   *
   * The same function also serves next-steps TaskBlock, which calls it right after SENDING the
   * draft. That is why the cause is a required argument: the two callers want opposite things,
   * and a default is how this got missed on five surfaces the first time.
   */

  it('deletes the draft when the user ticked the task off', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTask(TASK_ID, 'checked-off');
    });

    expect(mockMutateSpies.completeTask).toHaveBeenCalledWith({
      taskId: TASK_ID,
      cleanupDraft: true,
    });
  });

  it('leaves the draft alone when the task closed because it was sent', async () => {
    // The draft IS the sent message now. Asking Gmail to delete it is at best a no-op, and the
    // thread labels would come off a message the user deliberately sent.
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTask(TASK_ID, 'sent');
    });

    expect(mockMutateSpies.completeTask).toHaveBeenCalledWith({ taskId: TASK_ID });
  });

  it('refreshes every cache that carries userTasks', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTask(TASK_ID, 'checked-off');
    });

    const paths = invalidatedPaths();
    expect(paths).toContain('userTasks.listUserTasks');
    expect(paths).toContain('crm.getConversation');
    expect(paths).toContain('crm.listConversations');
  });

  it('restores the task when the write fails', async () => {
    mockMutateSpies.completeTask.mockRejectedValue(new Error('boom'));
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticCompleteTask(TASK_ID, 'checked-off');
    });

    expect(useCedarStore.getState().tasks[TASK_ID]?.status).toBe('todo');
  });
});

describe('optimisticDeleteTask', () => {
  it('masks the row through the window and invalidates the CRM list on success', async () => {
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticDeleteTask(TASK_ID, true, CONV_ID);
    });

    expect(getPendingTaskResolution(TASK_ID)).toBe('deleted');
    act(() => {
      useCedarStore.getState().setConversations({ [CONV_ID]: serverConversation([task()]) });
    });
    expect(visibleTaskIds()).toEqual([]);

    await settleUndoWindow();

    expect(mockMutateSpies.deleteTask).toHaveBeenCalledWith({ taskId: TASK_ID });
    expect(invalidatedPaths()).toContain('crm.listConversations');
    expect(getPendingTaskResolution(TASK_ID)).toBeUndefined();
  });

  it('refreshes the conversation the Overview renders from', async () => {
    // This one was missing here while the completion path had it all along. The conversation
    // Overview's Due Tasks list reads `crm.getConversation` — leave it stale and that surface
    // serves the deleted task straight back the moment the mask lifts.
    const { result } = renderHook(() => useOptimisticTaskActions());

    await act(async () => {
      await result.current.optimisticDeleteTask(TASK_ID, true, CONV_ID);
    });
    await settleUndoWindow();

    expect(invalidatedPaths()).toContain('crm.getConversation');
  });
});