diff --git a/src/features/board/hooks/useInvalidateBoard.ts b/src/features/board/hooks/useInvalidateBoard.ts new file mode 100644 index 00000000..a204a2e4 --- /dev/null +++ b/src/features/board/hooks/useInvalidateBoard.ts @@ -0,0 +1,29 @@ +import { useCallback } from "react"; +import { useQueryClient } from "@tanstack/react-query"; +import { queryKeys } from "../../../services/queryKeys"; + +/** + * Marks the cached board stale after a write made outside the board itself — a checklist kept + * from a reply, a card authored from a selection, a confirmed buddy action. + * + * These writes land on a surface the hire is usually not looking at, so nothing else would ever + * re-read the board: an open board behind the dock keeps serving its previous read, and a visit + * inside the `staleTime` window is served the pre-write copy. Marking is what makes whichever + * comes next — the open board, or the next visit — show the card. Call it only once the write has + * really happened: a failed save must not cost the board its cache. + * + * @param projectId The project the card went to. Omit it only for writes whose project the + * backend resolved server-side (buddy actions) and never told the client — that marks every + * cached board rather than guessing one. + * @returns A stable callback; call it after a successful write to invalidate the board cache. + */ +export function useInvalidateBoard(projectId?: string) { + const queryClient = useQueryClient(); + + return useCallback(() => { + void queryClient.invalidateQueries({ + queryKey: + projectId === undefined ? queryKeys.board.all() : queryKeys.board.byProject(projectId), + }); + }, [queryClient, projectId]); +} diff --git a/src/features/board/save/SaveToBoard.tsx b/src/features/board/save/SaveToBoard.tsx index cd581838..96b34f62 100644 --- a/src/features/board/save/SaveToBoard.tsx +++ b/src/features/board/save/SaveToBoard.tsx @@ -4,6 +4,7 @@ import { Button, type ButtonSize } from "../../../components/ui/Button"; import { useToast } from "../../../context/useToast"; import { boardService } from "../../../services/boardService"; import { useProjectContext } from "../../projects/useProjectContext"; +import { useInvalidateBoard } from "../hooks/useInvalidateBoard"; import { rememberOrigin, type CardOrigin } from "../layout/cardOrigins"; import type { AuthoredCardRequest } from "../types"; @@ -76,6 +77,7 @@ export function SaveToBoard({ }: SaveToBoardProps) { const { selectedProjectId } = useProjectContext(); const toast = useToast(); + const invalidateBoard = useInvalidateBoard(selectedProjectId); const [saving, setSaving] = useState(false); const [saved, setSaved] = useState(false); @@ -90,6 +92,9 @@ export function SaveToBoard({ try { const created = await boardService.addCard(selectedProjectId, card); + // This button is often pressed far away from the board — see `useInvalidateBoard`. + invalidateBoard(); + const where = origin?.(); // Never allowed to fail the save: the card is what was asked for, the trail back is extra. if (where) rememberOrigin(selectedProjectId, created.id, where); diff --git a/src/features/board/selection/SelectionActions.tsx b/src/features/board/selection/SelectionActions.tsx index 70c7197d..60788d82 100644 --- a/src/features/board/selection/SelectionActions.tsx +++ b/src/features/board/selection/SelectionActions.tsx @@ -7,6 +7,7 @@ import { useFocusMode } from "../../../context/useFocusMode"; import { useProjectContext } from "../../projects/useProjectContext"; import { ChatContext } from "../../../context/ChatContext"; import { boardService } from "../../../services/boardService"; +import { useInvalidateBoard } from "../hooks/useInvalidateBoard"; import { rememberOrigin } from "../layout/cardOrigins"; import { useCardMarks } from "../marks/useCardMarks"; import { DEFAULT_HIGHLIGHT } from "../marks/highlightColors"; @@ -48,6 +49,7 @@ export function SelectionActions() { const quoteSelection = chatContext?.quoteSelection; const [saving, setSaving] = useState(false); const toast = useToast(); + const invalidateBoard = useInvalidateBoard(selectedProjectId); const navigate = useNavigate(); const add = useCallback(async () => { @@ -65,6 +67,8 @@ export function SelectionActions() { url: selection.origin, label: selection.source ?? "where you were", }); + // Found anywhere in the app, so the board is almost never on screen — see `useInvalidateBoard`. + invalidateBoard(); toast.success( request.kind === "LINK" ? "Link saved to your board" : "Note saved to your board", { @@ -78,7 +82,7 @@ export function SelectionActions() { } finally { setSaving(false); } - }, [selection, selectedProjectId, toast, navigate, clear]); + }, [selection, selectedProjectId, toast, navigate, clear, invalidateBoard]); /** * Hands the selection to the buddy as a quote, unsent. diff --git a/src/features/buddy/components/SaveReplyToBoard.tsx b/src/features/buddy/components/SaveReplyToBoard.tsx index 368f469b..14d054b9 100644 --- a/src/features/buddy/components/SaveReplyToBoard.tsx +++ b/src/features/buddy/components/SaveReplyToBoard.tsx @@ -4,6 +4,7 @@ import { Button } from "../../../components/ui/Button"; import { useToast } from "../../../context/useToast"; import { boardService } from "../../../services/boardService"; import { extractChecklist, toChecklistRequest } from "../../board/generation/checklistFromMarkdown"; +import { useInvalidateBoard } from "../../board/hooks/useInvalidateBoard"; import { useProjectContext } from "../../projects/useProjectContext"; type SaveReplyToBoardProps = { @@ -29,6 +30,7 @@ type SaveReplyToBoardProps = { export function SaveReplyToBoard({ content }: SaveReplyToBoardProps) { const { selectedProjectId } = useProjectContext(); const toast = useToast(); + const invalidateBoard = useInvalidateBoard(selectedProjectId); const [saving, setSaving] = useState(false); const [saved, setSaved] = useState(false); @@ -42,6 +44,8 @@ export function SaveReplyToBoard({ content }: SaveReplyToBoardProps) { setSaving(true); try { await boardService.addCard(selectedProjectId, toChecklistRequest(checklist)); + // The board this list joined may be cached right behind this dock — see `useInvalidateBoard`. + invalidateBoard(); setSaved(true); toast.success("Kept on your board", { description: `"${checklist.title}" — ${checklist.items.length} things to tick off.`, diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index 8be71944..2b9c8953 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -12,6 +12,15 @@ import { type BuddyOpeningAction, } from "../../../services/buddyService"; import { useAuth } from "../../../context/useAuth"; +import { useInvalidateBoard } from "../../board/hooks/useInvalidateBoard"; +import { + BUDDY_ACTION_AMEND_CHECKLIST, + BUDDY_ACTION_CLAIM_GOAL, + BUDDY_ACTION_PLACE_CHECKLIST, + BUDDY_ACTION_PLACE_NOTE, + BUDDY_ACTION_REWORD_CHECKLIST, + BUDDY_ACTION_TICK_CHECKLIST, +} from "../types"; import type { ActionPatch, BuddyMessageView, ProposedAction } from "../types"; /** @@ -44,6 +53,29 @@ function isNotFound(e: unknown): boolean { return e instanceof Error && "status" in e && (e as { status: number }).status === 404; } +/** + * The hire-confirmed buddy actions that write to the board, by their wire names: the five + * `BuddyBoardWriteActions` handles (placing, amending, ticking and rewording a checklist, and + * the note), plus `claim_goal`, which pins the claimed task as the board's CURRENT_TASK card — + * its own outcome line says so ("It's on your board too"). Every other confirm changes something + * else — a task claim, an attestation request, a flag, a username — and needs no board sync. + */ +const BUDDY_BOARD_ACTIONS = new Set([ + BUDDY_ACTION_PLACE_CHECKLIST, + BUDDY_ACTION_AMEND_CHECKLIST, + BUDDY_ACTION_TICK_CHECKLIST, + BUDDY_ACTION_REWORD_CHECKLIST, + BUDDY_ACTION_PLACE_NOTE, + BUDDY_ACTION_CLAIM_GOAL, +]); + +/** + * The mid-answer tool that puts a card on the board. `place_card` is deliberately not a confirmed + * action — it applies the moment the mentor runs it — so a turn that ran it is the only signal + * the client ever gets that the board moved. Acted on by `sendMessage` when the turn ends. + */ +const BUDDY_BOARD_TOOLS = new Set(["place_card"]); + /** * Where "is the buddy in team mode" lives between reloads — scoped to the signed-in user, the * way the project selection is. A shared browser must not hand one manager's team conversation @@ -87,6 +119,7 @@ export function useBuddyConversation( const [messages, setMessages] = useState([]); const [isThinking, setIsThinking] = useState(false); const [isStreaming, setIsStreaming] = useState(false); + const invalidateBoard = useInvalidateBoard(); // The tool the buddy is running right now, if any -- drives "Checking your progress…" // in place of a generic spinner. Cleared as soon as the answer starts streaming. const [activeTool, setActiveTool] = useState(null); @@ -557,12 +590,27 @@ export function useBuddyConversation( // Once the hire says anything, the opener's one-click suggestion has served its purpose. setOpenerAction(null); + // Whether this turn ran a tool that puts a card on the board. A property rather than a + // `let`, so the reads below see the writes made in the stream callbacks — see `greet`. + const touched = { board: false }; + + /** + * `place_card` is the one board write that is not confirmed: its `tool_use` event is the + * whole signal the client gets, and it cannot say whether the tool wrote a card or was + * refused. So the board is marked stale when the turn ends — the failing paths included. + * See `useInvalidateBoard` for why the mark is what makes this visible. + */ + const syncBoardIfTouched = () => { + if (touched.board) invalidateBoard(); + }; + try { await streamMessage( text, { onToolUse: (name) => { setActiveTool(name); + if (BUDDY_BOARD_TOOLS.has(name)) touched.board = true; }, onToken: (token) => { @@ -655,15 +703,20 @@ export function useBuddyConversation( // Read at call time: a turn speaks to whichever conversation is current when it starts. teamProjectIdRef.current ?? undefined, ); + + // The turn is over — the first moment a card placed mid-answer is certainly on the board. + syncBoardIfTouched(); } catch (e) { console.error(e); setIsStreaming(false); setIsThinking(false); setActiveTool(null); failReply(assistantId); + // Same reason as above: a turn that placed a card and then broke still wrote it. + syncBoardIfTouched(); } }, - [failReply], + [failReply, invalidateBoard], ); /** Patches one proposed action in place, keyed by its message and action id. */ @@ -752,6 +805,13 @@ export function useBuddyConversation( ok: result.ok, outcome: result.message, }); + + // `board.all()`, not a project key: the backend re-resolves the project server-side + // (the caller's single onboarding project) and never tells the client which board — + // see `useInvalidateBoard`. + if (result.ok && "action" in action && BUDDY_BOARD_ACTIONS.has(action.action)) { + invalidateBoard(); + } } catch (e) { console.error(e); // A settled proposal does NOT come back 404 — the backend answers 200 with ok: false @@ -775,7 +835,7 @@ export function useBuddyConversation( } })(); }, - [beginDecision, endDecision, patchAction], + [beginDecision, endDecision, patchAction, invalidateBoard], ); /** diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index a5c932b7..1da87ef9 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -53,6 +53,23 @@ export const BUDDY_ACTION_TICK_CHECKLIST = "tick_checklist_items"; /** The backend's `reword_checklist_item` action: one line replaced, shown before and after. */ export const BUDDY_ACTION_REWORD_CHECKLIST = "reword_checklist_item"; +/** + * The backend's `place_note` action: an explanation the mentor offered to keep as a note. + * + * Named like the checklist actions above because the same reader needs it: the confirm path has + * to recognise which actions write to the board (see `BUDDY_BOARD_ACTIONS` in `useBuddyConversation`). + */ +export const BUDDY_ACTION_PLACE_NOTE = "place_note"; + +/** + * The backend's `claim_goal` action: the hire starting to work toward a task. + * + * Confirming this writes twice — the goal claim itself, and the CURRENT_TASK card pinned onto + * the board ("It's on your board too") — which is why the board-syncing set includes it despite + * its not being one of the board tools. + */ +export const BUDDY_ACTION_CLAIM_GOAL = "claim_goal"; + /** * An action proposed in hire mode: the buddy offers to do something *for this hire*, and the * confirm echoes the offer's own payload back verbatim. What gets written is what was shown on diff --git a/tests/unit/features/board/SaveToBoard.test.tsx b/tests/unit/features/board/SaveToBoard.test.tsx new file mode 100644 index 00000000..39fc1665 --- /dev/null +++ b/tests/unit/features/board/SaveToBoard.test.tsx @@ -0,0 +1,104 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { BookmarkPlus } from "lucide-react"; +import type { ReactElement } from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { SaveToBoard } from "../../../../src/features/board/save/SaveToBoard"; +import { boardService } from "../../../../src/services/boardService"; +import { queryKeys } from "../../../../src/services/queryKeys"; +import type { AuthoredCardRequest } from "../../../../src/features/board/types"; + +let selectedProjectId = "p1"; +vi.mock("../../../../src/features/projects/useProjectContext", () => ({ + useProjectContext: () => ({ selectedProjectId }), +})); + +const toast = { success: vi.fn(), error: vi.fn() }; +vi.mock("../../../../src/context/useToast", () => ({ useToast: () => toast })); + +const request = (): AuthoredCardRequest => ({ kind: "NOTE", text: "A reply, frozen." }); + +/** Renders under a client whose cache already holds this project's board, read a moment ago. */ +function renderSave(ui: ReactElement) { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.board.byProject("p1"), { + boardId: "b1", + projectId: "p1", + cards: [], + }); + render({ui}); + return { client }; +} + +const boardIsStale = (client: QueryClient) => + client.getQueryState(queryKeys.board.byProject("p1"))?.isInvalidated ?? false; + +function props(overrides: Partial[0]> = {}) { + return { + request, + label: "Keep on my board", + icon: , + ...overrides, + }; +} + +describe("SaveToBoard", () => { + beforeEach(() => { + vi.clearAllMocks(); + selectedProjectId = "p1"; + // jsdom on this Node has no Storage backing (CI's Node does); clearing is hygiene when present. + window.localStorage?.clear(); + }); + + /** The one button every "keep this" offer is built on — chat, buddy replies, task cards. */ + it("puts the built card on the selected project's board", async () => { + const addCard = vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + renderSave(); + + await userEvent.click(screen.getByRole("button", { name: "Keep on my board" })); + + await waitFor(() => expect(addCard).toHaveBeenCalledOnce()); + expect(addCard.mock.calls[0][0]).toBe("p1"); + expect(addCard.mock.calls[0][1]).toEqual({ kind: "NOTE", text: "A reply, frozen." }); + }); + + it("marks the board stale once the card is really on it", async () => { + vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + const { client } = renderSave(); + + await userEvent.click(screen.getByRole("button", { name: "Keep on my board" })); + + await waitFor(() => expect(boardIsStale(client)).toBe(true)); + }); + + /** The board-surface callers still get their callback — for the affordances that are theirs. */ + it("tells the caller the card landed", async () => { + vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + const onSaved = vi.fn(); + renderSave(); + + await userEvent.click(screen.getByRole("button", { name: "Break this into a checklist" })); + + await waitFor(() => expect(onSaved).toHaveBeenCalledOnce()); + }); + + it("leaves the board and the caller alone when the write failed", async () => { + vi.spyOn(boardService, "addCard").mockRejectedValue(new Error("nope")); + const onSaved = vi.fn(); + const { client } = renderSave(); + + await userEvent.click(screen.getByRole("button", { name: "Keep on my board" })); + + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(onSaved).not.toHaveBeenCalled(); + expect(boardIsStale(client)).toBe(false); + }); + + it("offers nothing when no project is selected", () => { + selectedProjectId = ""; + renderSave(); + + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + }); +}); diff --git a/tests/unit/features/board/SelectionActions.test.tsx b/tests/unit/features/board/SelectionActions.test.tsx index 3a29adad..6b103d2a 100644 --- a/tests/unit/features/board/SelectionActions.test.tsx +++ b/tests/unit/features/board/SelectionActions.test.tsx @@ -1,9 +1,11 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { SelectionActions } from "../../../../src/features/board/selection/SelectionActions"; import { boardService } from "../../../../src/services/boardService"; +import { queryKeys } from "../../../../src/services/queryKeys"; import { ChatContext, type ChatContextValue } from "../../../../src/context/ChatContext"; import { openAiBuddy } from "../../../../src/features/buddy/aiBuddyBus"; import { FocusModeContext } from "../../../../src/context/FocusModeContext"; @@ -58,8 +60,11 @@ describe("SelectionActions", () => { document.dispatchEvent(new Event("selectionchange")); } - function renderToolbar({ focused = false }: { focused?: boolean } = {}) { - return render( + function renderToolbar({ + focused = false, + client, + }: { focused?: boolean; client?: QueryClient } = {}) { + const tree = ( {}, toggleFocused: () => {} }} @@ -68,8 +73,14 @@ describe("SelectionActions", () => { - , + ); + + // A client of its own only where a test reads the cache back; every other render keeps the + // one the harness hands it. + return client + ? render({tree}) + : render(tree); } it("offers nothing until something is selected", () => { @@ -97,6 +108,42 @@ describe("SelectionActions", () => { expect(addCard.mock.calls[0][1]).toMatchObject({ kind: "NOTE" }); }); + it("marks the board stale once the selection is really on it", async () => { + vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.board.byProject("p1"), { + boardId: "b1", + projectId: "p1", + cards: [], + }); + renderToolbar({ client }); + highlight("Run the migration first."); + + await userEvent.click(await screen.findByRole("button", { name: /add to board/i })); + + await waitFor(() => + expect(client.getQueryState(queryKeys.board.byProject("p1"))?.isInvalidated).toBe(true), + ); + }); + + /** The other half of the stale-marking rule: a write that never landed must not cost the board. */ + it("leaves the board alone when the write failed", async () => { + vi.spyOn(boardService, "addCard").mockRejectedValue(new Error("nope")); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.board.byProject("p1"), { + boardId: "b1", + projectId: "p1", + cards: [], + }); + renderToolbar({ client }); + highlight("Run the migration first."); + + await userEvent.click(await screen.findByRole("button", { name: /add to board/i })); + + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(client.getQueryState(queryKeys.board.byProject("p1"))?.isInvalidated).toBe(false); + }); + /** * Being pulled to the board to confirm something landed is the interruption this feature exists * to avoid. The toast carries the way there for whoever wants it. diff --git a/tests/unit/features/board/useInvalidateBoard.test.tsx b/tests/unit/features/board/useInvalidateBoard.test.tsx new file mode 100644 index 00000000..c97906f9 --- /dev/null +++ b/tests/unit/features/board/useInvalidateBoard.test.tsx @@ -0,0 +1,53 @@ +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactNode } from "react"; +import { describe, it, expect } from "vitest"; +import { useInvalidateBoard } from "../../../../src/features/board/hooks/useInvalidateBoard"; +import { queryKeys } from "../../../../src/services/queryKeys"; + +/** A client holding two projects' boards, read a moment ago. */ +function boardContext() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.board.byProject("p1"), { + boardId: "b1", + projectId: "p1", + cards: [], + }); + client.setQueryData(queryKeys.board.byProject("p2"), { + boardId: "b2", + projectId: "p2", + cards: [], + }); + + function Wrapper({ children }: { children: ReactNode }) { + return {children}; + } + + return { client, Wrapper }; +} + +const boardIsStale = (client: QueryClient, projectId: string) => + client.getQueryState(queryKeys.board.byProject(projectId))?.isInvalidated ?? false; + +describe("useInvalidateBoard", () => { + it("marks only the board that was written to", async () => { + const { client, Wrapper } = boardContext(); + const { result } = renderHook(() => useInvalidateBoard("p1"), { wrapper: Wrapper }); + + result.current(); + + await waitFor(() => expect(boardIsStale(client, "p1")).toBe(true)); + // The other project's board is untouched — the writer knew where the card went. + expect(boardIsStale(client, "p2")).toBe(false); + }); + + it("marks every cached board when the writer never learned the project", async () => { + const { client, Wrapper } = boardContext(); + const { result } = renderHook(() => useInvalidateBoard(), { wrapper: Wrapper }); + + result.current(); + + await waitFor(() => expect(boardIsStale(client, "p1")).toBe(true)); + expect(boardIsStale(client, "p2")).toBe(true); + }); +}); diff --git a/tests/unit/features/buddy/SaveReplyToBoard.test.tsx b/tests/unit/features/buddy/SaveReplyToBoard.test.tsx new file mode 100644 index 00000000..97eaa38d --- /dev/null +++ b/tests/unit/features/buddy/SaveReplyToBoard.test.tsx @@ -0,0 +1,127 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider, useQuery } from "@tanstack/react-query"; +import type { ReactElement } from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { SaveReplyToBoard } from "../../../../src/features/buddy/components/SaveReplyToBoard"; +import { boardService } from "../../../../src/services/boardService"; +import { queryKeys } from "../../../../src/services/queryKeys"; + +let selectedProjectId = "p1"; +vi.mock("../../../../src/features/projects/useProjectContext", () => ({ + useProjectContext: () => ({ selectedProjectId }), +})); + +const toast = { success: vi.fn(), error: vi.fn() }; +vi.mock("../../../../src/context/useToast", () => ({ useToast: () => toast })); + +/** A reply holding a list — the shape that makes the offer appear at all. */ +const REPLY = "Here is how to start:\n\n## Getting started\n\n- Run it locally\n- Open a PR\n"; + +/** + * Renders under a client whose cache already holds this project's board, read a moment ago — the + * exact state that used to keep serving a board without the card that was just kept. + */ +function renderReply(ui: ReactElement) { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.board.byProject("p1"), { + boardId: "b1", + projectId: "p1", + cards: [], + }); + render({ui}); + return { client }; +} + +const boardIsStale = (client: QueryClient) => + client.getQueryState(queryKeys.board.byProject("p1"))?.isInvalidated ?? false; + +describe("SaveReplyToBoard", () => { + beforeEach(() => { + vi.clearAllMocks(); + selectedProjectId = "p1"; + }); + + it("offers nothing when the reply holds no list", () => { + renderReply(); + + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + }); + + it("keeps the reply's list as a checklist card on the selected project", async () => { + const addCard = vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + renderReply(); + + await userEvent.click(screen.getByRole("button", { name: /keep as checklist/i })); + + await waitFor(() => expect(addCard).toHaveBeenCalledOnce()); + expect(addCard.mock.calls[0][0]).toBe("p1"); + expect(addCard.mock.calls[0][1]).toMatchObject({ + kind: "CHECKLIST", + title: "Getting started", + items: [ + { text: "Run it locally", done: false }, + { text: "Open a PR", done: false }, + ], + }); + }); + + /** + * Issue #233: the write landed, but the board this dock can be floating over kept serving its + * cached copy for up to `staleTime`. The card existing is what the button says; the cache is + * what the hire actually sees. + */ + it("marks the board stale once the list is really on it", async () => { + vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + const { client } = renderReply(); + + await userEvent.click(screen.getByRole("button", { name: /keep as checklist/i })); + + await waitFor(() => expect(boardIsStale(client)).toBe(true)); + // And the button acknowledges, as it always has. + expect(screen.getByRole("button", { name: /checklist on your board/i })).toBeInTheDocument(); + }); + + /** + * The mechanical promise of #233 in one test: a board mounted behind the dock is an *active + * reader* of the key the keep invalidates, and invalidation alone would be a comment if it + * never reached one. The flag assertions above are about the cache; this one is about the + * refetch the mounted board actually gets. + */ + it("refetches a mounted board after the keep", async () => { + vi.spyOn(boardService, "addCard").mockResolvedValue({ id: "c1" } as never); + const fetchBoard = vi + .fn<() => Promise>() + .mockResolvedValue({ boardId: "b1", projectId: "p1", cards: [] }); + + function MountedBoard() { + useQuery({ queryKey: queryKeys.board.byProject("p1"), queryFn: () => fetchBoard() }); + return null; + } + + render( + + + + , + ); + await waitFor(() => expect(fetchBoard).toHaveBeenCalledOnce()); + + await userEvent.click(screen.getByRole("button", { name: /keep as checklist/i })); + + // The initial read plus the one the invalidation triggered. + await waitFor(() => expect(fetchBoard.mock.calls.length).toBeGreaterThanOrEqual(2)); + }); + + it("leaves the board alone when the save failed", async () => { + vi.spyOn(boardService, "addCard").mockRejectedValue(new Error("nope")); + const { client } = renderReply(); + + await userEvent.click(screen.getByRole("button", { name: /keep as checklist/i })); + + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(boardIsStale(client)).toBe(false); + }); +}); diff --git a/tests/unit/features/buddy/useBuddy.test.tsx b/tests/unit/features/buddy/useBuddy.test.tsx index e3664c65..55c482ab 100644 --- a/tests/unit/features/buddy/useBuddy.test.tsx +++ b/tests/unit/features/buddy/useBuddy.test.tsx @@ -1,10 +1,13 @@ import { renderHook, act, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactNode } from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { useBuddy } from "../../../../src/features/buddy/hooks/useBuddy"; import { BuddyProviderWithStubs } from "./buddyTestHarness"; import { openAiBuddy } from "../../../../src/features/buddy/aiBuddyBus"; -import { http, HttpResponse } from "msw"; +import { delay, http, HttpResponse } from "msw"; import { server } from "../../setup/vitest.setup"; +import { queryKeys } from "../../../../src/services/queryKeys"; /** * A greeting that opens the visit and writes nothing. @@ -455,4 +458,250 @@ describe("useBuddy", () => { expect(result.current.messages[1].actions?.[0].status).toBe("resolved"); }); }); + + /** + * The board a confirmed action writes to is a cache entry nobody here is looking at: the dock + * can float over the board page, and a visit within `staleTime` serves the board as it was read. + * These pin which confirms mark it stale — and which correctly do not. + */ + describe("board synchronisation", () => { + /** A client already holding this hire's board, and a wrapper that puts it under the session. */ + function boardContext() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.board.byProject("p1"), { + boardId: "b1", + projectId: "p1", + cards: [], + }); + + function Wrapper({ children }: { children: ReactNode }) { + return ( + + {children} + + ); + } + + return { client, Wrapper }; + } + + const boardIsStale = (client: QueryClient) => + client.getQueryState(queryKeys.board.byProject("p1"))?.isInvalidated ?? false; + + function stream(events: string[]) { + const encoder = new TextEncoder(); + return new HttpResponse( + new ReadableStream({ + start(controller) { + for (const event of events) controller.enqueue(encoder.encode(`data: ${event}\n\n`)); + controller.close(); + }, + }), + { headers: { "Content-Type": "text/event-stream" } }, + ); + } + + /** Opens the session, sends a question whose reply proposes `proposalEvent`, and confirms it. */ + async function confirmProposal( + proposalEvent: string, + outcome: { ok: boolean; message: string }, + ) { + const { client, Wrapper } = boardContext(); + server.use( + http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([])), + http.post("/api/v1/onboarding/me/buddy/open/stream", () => silentGreeting()), + http.post("/api/v1/onboarding/me/buddy/messages", () => stream([proposalEvent])), + http.post("/api/v1/onboarding/me/buddy/actions", () => HttpResponse.json(outcome)), + ); + + const { result } = renderHook(() => useBuddy(), { wrapper: Wrapper }); + act(() => { + result.current.toggleOpen(); + }); + await waitFor(() => expect(result.current.messages).toHaveLength(0)); + act(() => { + result.current.setDraft("how do I start?"); + }); + act(() => { + result.current.handleSubmit({ preventDefault: vi.fn() } as unknown as React.FormEvent); + }); + await waitFor(() => expect(result.current.messages[1]?.actions?.[0]).toBeDefined()); + + act(() => { + result.current.confirmAction( + result.current.messages[1].id, + result.current.messages[1].actions![0], + ); + }); + await waitFor(() => expect(result.current.messages[1].actions?.[0].status).toBe("resolved")); + + return { client }; + } + + const PLACE_CHECKLIST = + '{"type":"action_proposal","action":"place_checklist","label":"Keep this as a checklist",' + + '"checklist_title":"Getting started","checklist_items":["Run it locally","Open a PR"]}'; + + /** + * Every confirmed action that writes to the board, by its wire name: the five board-write + * handles plus `claim_goal`, which writes twice — the claim, and the CURRENT_TASK card it + * pins ("it's on your board too"). A table beats six near-identical tests drifting apart one + * rename at a time. + */ + const BOARD_WRITING_PROPOSALS: [string, string][] = [ + ["place_checklist", PLACE_CHECKLIST], + [ + "amend_checklist", + '{"type":"action_proposal","action":"amend_checklist","label":"Add that step",' + + '"card_id":"c-1","checklist_title":"Getting started",' + + '"checklist_items":["Run it locally","Open a PR","Ping the PM"]}', + ], + [ + "tick_checklist_items", + '{"type":"action_proposal","action":"tick_checklist_items","label":"Tick those off",' + + '"card_id":"c-1","checklist_items":["Run it locally"]}', + ], + [ + "reword_checklist_item", + '{"type":"action_proposal","action":"reword_checklist_item","label":"Reword that step",' + + '"card_id":"c-1","line_before":"Run it locally","line_after":"Run the app locally"}', + ], + [ + "place_note", + '{"type":"action_proposal","action":"place_note","label":"Keep this as a note",' + + '"note_text":"Deploys need the VPN."}', + ], + [ + "claim_goal", + '{"type":"action_proposal","action":"claim_goal","label":"Work toward this task","task_id":"t-1"}', + ], + ]; + + it.each(BOARD_WRITING_PROPOSALS)( + "marks the board stale when a confirmed %s wrote a card", + async (_action, proposal) => { + const { client } = await confirmProposal(proposal, { ok: true, message: "Done." }); + + expect(boardIsStale(client)).toBe(true); + }, + ); + + it("leaves the board alone when the action changed nothing", async () => { + const { client } = await confirmProposal(PLACE_CHECKLIST, { + ok: false, + message: "I couldn't keep that just now.", + }); + + expect(boardIsStale(client)).toBe(false); + }); + + it("leaves the board alone for an action that never touches it", async () => { + const { client } = await confirmProposal( + '{"type":"action_proposal","action":"request_attestation","label":"Ask them to confirm this","title":"the auth fix","attester_id":"u-9"}', + { ok: true, message: "Asked them to confirm it." }, + ); + + expect(boardIsStale(client)).toBe(false); + }); + + /** Opens the session; each question asked answers with the next entry of `answers`. */ + async function openSession(answers: string[][]) { + const { client, Wrapper } = boardContext(); + let answered = 0; + server.use( + http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([])), + http.post("/api/v1/onboarding/me/buddy/open/stream", () => silentGreeting()), + http.post("/api/v1/onboarding/me/buddy/messages", () => stream(answers[answered++])), + ); + + const { result } = renderHook(() => useBuddy(), { wrapper: Wrapper }); + act(() => { + result.current.toggleOpen(); + }); + await waitFor(() => expect(result.current.messages).toHaveLength(0)); + + return { client, result }; + } + + /** Sends one question and waits for its turn to end. */ + async function ask(result: { current: ReturnType }, text: string) { + act(() => { + result.current.setDraft(text); + }); + act(() => { + result.current.handleSubmit({ preventDefault: vi.fn() } as unknown as React.FormEvent); + }); + await waitFor(() => expect(result.current.isThinking).toBe(false)); + } + + const PLACE_CARD_TURN = [ + '{"type":"tool_use","name":"place_card"}', + '{"type":"token","content":"It is on your board."}', + '{"type":"done"}', + ]; + const METRICS_TURN = [ + '{"type":"tool_use","name":"get_my_metrics"}', + '{"type":"token","content":"You are on track."}', + '{"type":"done"}', + ]; + + /** + * `place_card` is the one board write that is not confirmed — it applies the moment the + * mentor runs it — so its `tool_use` event during the turn is the whole signal the client + * gets that the board may have moved. + */ + it("marks the board stale when the turn placed a card on it", async () => { + const { client, result } = await openSession([PLACE_CARD_TURN]); + + await ask(result, "put the PR review task on my board"); + + await waitFor(() => expect(boardIsStale(client)).toBe(true)); + }); + + it("leaves the board alone when the turn ran other tools", async () => { + const { client, result } = await openSession([METRICS_TURN, PLACE_CARD_TURN]); + const invalidations = vi.spyOn(client, "invalidateQueries"); + + await ask(result, "how am I doing?"); + await ask(result, "put the PR review task on my board"); + + // Waiting for the *second* turn's sync closes the window: whatever the board-silent first + // turn was going to do has happened by now, so a single invalidation proves the metrics + // read marked nothing on its own. + await waitFor(() => expect(boardIsStale(client)).toBe(true)); + expect(invalidations).toHaveBeenCalledTimes(1); + }); + + /** A reply that delivers its events and then drops — the failure lands after `place_card` ran. */ + function streamThenBreak(events: string[]) { + const encoder = new TextEncoder(); + return new HttpResponse( + new ReadableStream({ + async start(controller) { + for (const event of events) controller.enqueue(encoder.encode(`data: ${event}\n\n`)); + await delay(20); + controller.error(new Error("connection lost")); + }, + }), + { headers: { "Content-Type": "text/event-stream" } }, + ); + } + + /** + * The turn can fail after the tool already ran — the card is on the board either way, so the + * sync must not wait for a clean finish. This is the "failing paths included" claim, pinned. + */ + it("still marks the board stale when the turn broke after placing a card", async () => { + const { client, result } = await openSession([]); + server.use( + http.post("/api/v1/onboarding/me/buddy/messages", () => + streamThenBreak(['{"type":"tool_use","name":"place_card"}']), + ), + ); + + await ask(result, "put the PR review task on my board"); + + await waitFor(() => expect(boardIsStale(client)).toBe(true)); + }); + }); }); diff --git a/tests/unit/setup/msw-handlers.ts b/tests/unit/setup/msw-handlers.ts index ff95a7f4..34ec5d8e 100644 --- a/tests/unit/setup/msw-handlers.ts +++ b/tests/unit/setup/msw-handlers.ts @@ -656,4 +656,8 @@ export const handlers = [ http.get("/api/v1/connectors/confluence/sources", () => HttpResponse.json({ connectorId: "confluence", sources: [] }), ), + // Every surface that opens the buddy dock asks for its suggestion chips. A default empty list + // keeps that request handled for the many tests that open the dock without being about the + // chips; `useBuddySuggestions`' own suite mocks the service directly and never sees this. + http.get("/api/v1/onboarding/me/buddy/suggestions", () => HttpResponse.json([])), ];