CyberCoveLLC/Triple-C
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 44 Wiki Activity

Cache notes in one place, and serialise a project's writes

Browse Source 
Implements the design spec's §2 — "notes cached in zustand keyed by
project id" — which the plan substituted with a hook-local `useState`.

Sharing the `NotesPanel` *component* between the Project Home sub-tab and
the dock did not share the *cache*. Both resolve to the same project, so
two panels mount two `useNotes(P)`, each with its own list. Edit a note
in the dock and blur; the tab's copy is still pre-edit, and the tab's
next blur commits `{...staleRecord, title, body}` — the dock's edit gone
from disk with no error and no indicator. That is the feature's own
primary workflow: take notes in the dock while the agent runs, which is
the reason the dock exists, then go back to the tab.

`notesByProject` plus a per-project in-flight flag now hold the list.
Both surfaces render from one array; two panels mounting for one project
make one read; and because the write is keyed by project, a response
that lands after the user has moved on updates the project it belongs to
rather than whichever is on screen. This is also the boundary §8 says a
detached notes window needs.

Three more bugs in the same code, fixed with it:

- Delete-after-edit could resurrect the note. Clicking Delete with the
  textarea focused fires blur first, so `save_note` and `delete_note` go
  out back to back; Rust's `write_lock` stops them interleaving but does
  not order them, and a delete that wins the lock is undone by the
  upsert behind it. A project's mutations now go through one promise
  chain, module-scoped for the reason `useTerminal`'s input queue is.
- An unsaved draft vanished when any other note was saved, because the
  re-read replaced the list with the backend's. "New note" now persists,
  so the backend owns the row from the start — chosen over merging local
  drafts because a local-only row in a *shared* cache would exist in the
  panel that made it and nowhere else.
- The save outcome was reported for the wrong project after a switch:
  the guard covered only the list replacement, so the new project's
  SaveIndicator flashed "Saved ✓" for the old project's write. The
  indicator now resets on a project change and reports only its own.

`NotesPanel` also re-seeds its draft when the *stored* text of the note
it has selected changes, so an edit made in the other surface reaches
the editor and not only the list. It never overwrites something
half-typed; that still blurs into a last-writer-wins save, as any
blur-commit editor does.

NotesPanel.shared.test.tsx is the configuration none of the existing
tests had: two panels, one project, the real hook. Four of its six
assertions fail against the previous implementation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HjL1E2JFNctUqCYotUwqqb
This commit is contained in:
shadowdao
2026-09-01 13:47:01 -07:00
co-authored by Claude Opus 5
parent be47c5edfd
commit 5c47656444
5 changed files with 613 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/hooks/useNotes.test.ts
+173 -10
View File
@@ -1,6 +1,7 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { renderHook, act, waitFor } from "@testing-library/react";
import { useNotes } from "./useNotes";
import { useAppState } from "../store/appState";
import type { Note } from "../lib/types";
const listNotes = vi.fn();
@@ -13,14 +14,6 @@ vi.mock("../lib/tauri-commands", () => ({
deleteNote: (p: string, id: string) => deleteNote(p, id),
}));
const pushToast = vi.fn();
vi.mock("../store/appState", () => ({
useAppState: Object.assign(
(selector: (s: unknown) => unknown) => selector({ pushToast }),
{ getState: () => ({ pushToast }) },
),
}));
const note = (over: Partial<Note> = {}): Note => ({
id: "n1",
title: "Deploy",
@@ -31,8 +24,35 @@ const note = (over: Partial<Note> = {}): Note => ({
...over,
});
/** The toasts the hook pushed. The store is real, so this is what a user sees. */
const toasts = () => useAppState.getState().toasts;
/**
* A stand-in for the Rust store: one list per project, upsert and delete
* applied to it, `list_notes` reading it back. Several of these tests are about
* what the *list* looks like after a sequence of writes, which a per-call
* `mockResolvedValueOnce` cannot express.
*/
function fakeBackend(initial: Record<string, Note[]> = {}) {
const files: Record<string, Note[]> = { ...initial };
listNotes.mockImplementation(async (p: string) => [...(files[p] ?? [])]);
saveNote.mockImplementation(async (p: string, n: Note) => {
const list = files[p] ?? (files[p] = []);
const at = list.findIndex((x) => x.id === n.id);
if (at === -1) list.unshift(n);
else list[at] = n;
return n;
});
deleteNote.mockImplementation(async (p: string, id: string) => {
files[p] = (files[p] ?? []).filter((x) => x.id !== id);
});
return files;
}
beforeEach(() => {
vi.clearAllMocks();
// The cache is shared app state now, so it has to be reset like any other.
useAppState.setState({ notesByProject: {}, notesLoading: {}, toasts: [] });
listNotes.mockResolvedValue([note()]);
saveNote.mockImplementation(async (_p: string, n: Note) => n);
deleteNote.mockResolvedValue(undefined);
@@ -60,7 +80,7 @@ describe("useNotes", () => {
expect(ok).toBe(false);
expect(result.current.saveState.status).toBe("failed");
expect(pushToast).toHaveBeenCalled();
expect(toasts()).toHaveLength(1);
});
it("replaces the saved note in place rather than appending", async () => {
@@ -128,7 +148,7 @@ describe("useNotes", () => {
await waitFor(() => expect(result.current.loading).toBe(false));
expect(result.current.notes).toHaveLength(0);
expect(pushToast).toHaveBeenCalled();
expect(toasts()).toHaveLength(1);
});
it("ends with the list the backend returned when saving a new note", async () => {
@@ -206,4 +226,147 @@ describe("useNotes", () => {
expect(result.current.notes).toHaveLength(1);
expect(result.current.notes[0].id).toBe("n2");
});
it("keeps the notes already on screen when a refresh fails", async () => {
// The second surface mounting for a project is a refresh behind a list the
// user is already reading. One shared cache means a failed refresh would
// otherwise blank both panels.
fakeBackend({ p1: [note()] });
const tab = renderHook(() => useNotes("p1"));
await waitFor(() => expect(tab.result.current.loading).toBe(false));
listNotes.mockRejectedValueOnce(new Error("read failed"));
const dock = renderHook(() => useNotes("p1"));
await waitFor(() => expect(toasts()).toHaveLength(1));
expect(tab.result.current.notes).toHaveLength(1);
expect(dock.result.current.notes).toHaveLength(1);
});
it("does not report the old project's save on the new project's indicator", async () => {
// The indicator is per-panel and reads "Saved ✓". Firing it after a switch
// tells the user their *current* project was written when it was not.
const { result, rerender } = renderHook(
({ projectId }: { projectId: string }) => useNotes(projectId),
{ initialProps: { projectId: "p1" } },
);
await waitFor(() => expect(result.current.loading).toBe(false));
let resolveSave: ((n: Note) => void) | undefined;
saveNote.mockImplementationOnce(
() => new Promise((resolve) => (resolveSave = resolve)),
);
let savePromise: Promise<boolean> | undefined;
await act(async () => {
savePromise = result.current.saveNote(note({ body: "edited" }));
});
rerender({ projectId: "p2" });
await waitFor(() => expect(result.current.loading).toBe(false));
await act(async () => {
resolveSave?.(note({ body: "edited" }));
await savePromise;
});
expect(result.current.saveState.status).toBe("idle");
});
it("still reports a save on the indicator of the project it was made for", async () => {
const { result } = renderHook(() => useNotes("p1"));
await waitFor(() => expect(result.current.loading).toBe(false));
await act(async () => {
await result.current.saveNote(note({ body: "edited" }));
});
expect(result.current.saveState.status).toBe("saved");
});
it("serialises a project's writes so an edit cannot be re-inserted after its delete", async () => {
// Clicking Delete while the textarea has focus fires blur first, so a save
// and a delete go out back to back. The Rust write lock stops them
// interleaving but does not order them: a delete that wins the lock is
// undone by the upsert behind it, and the note comes back on next load.
const files = fakeBackend({ p1: [note()] });
const { result } = renderHook(() => useNotes("p1"));
await waitFor(() => expect(result.current.loading).toBe(false));
const order: string[] = [];
saveNote.mockImplementationOnce(async (p: string, n: Note) => {
await new Promise((r) => setTimeout(r, 20));
order.push("save");
files[p] = [n];
return n;
});
deleteNote.mockImplementationOnce(async (p: string, id: string) => {
order.push("delete");
files[p] = (files[p] ?? []).filter((x) => x.id !== id);
});
await act(async () => {
const save = result.current.saveNote(note({ body: "typo fixed" }));
const del = result.current.deleteNote("n1");
await Promise.all([save, del]);
});
expect(order).toEqual(["save", "delete"]);
expect(files.p1).toHaveLength(0);
expect(result.current.notes).toHaveLength(0);
});
it("keeps a new note when another note is saved right after it", async () => {
// A purely local draft used to be wiped by the next re-read: two clicks of
// "New note", type in the second, blur, and the first row was gone.
const files = fakeBackend({ p1: [note()] });
const { result } = renderHook(() => useNotes("p1"));
await waitFor(() => expect(result.current.loading).toBe(false));
let first: Note | null = null;
await act(async () => {
first = await result.current.createNote();
await result.current.createNote();
});
expect(result.current.notes).toHaveLength(3);
await act(async () => {
await result.current.saveNote(note({ body: "edited" }));
});
expect(result.current.notes).toHaveLength(3);
expect(result.current.notes.some((n) => n.id === first!.id)).toBe(true);
expect(files.p1).toHaveLength(3);
});
it("shares one cache between every hook watching the same project", async () => {
// The Project Home sub-tab and the dock both mount a panel for the same
// project. Two caches meant an edit in one was invisible to the other, and
// the other's next blur wrote its stale copy back over it.
fakeBackend({ p1: [note()] });
const tab = renderHook(() => useNotes("p1"));
const dock = renderHook(() => useNotes("p1"));
await waitFor(() => expect(tab.result.current.loading).toBe(false));
await waitFor(() => expect(dock.result.current.loading).toBe(false));
// One read for both — the in-flight flag is per project, not per hook.
expect(listNotes).toHaveBeenCalledTimes(1);
await act(async () => {
await dock.result.current.saveNote(note({ body: "written in the dock" }));
});
expect(tab.result.current.notes[0].body).toBe("written in the dock");
expect(tab.result.current.notes).toBe(dock.result.current.notes);
});
it("does not blank an already-loaded list when a second panel mounts", async () => {
fakeBackend({ p1: [note()] });
const tab = renderHook(() => useNotes("p1"));
await waitFor(() => expect(tab.result.current.loading).toBe(false));
const dock = renderHook(() => useNotes("p1"));
// No "Loading notes…" flash on the second surface.
expect(dock.result.current.loading).toBe(false);
expect(dock.result.current.notes).toHaveLength(1);
});
});
app/src/hooks/useNotes.ts
+198 -82
View File
@@ -8,8 +8,8 @@ import { useAppState } from "../store/appState";
function draft(): Note {
const now = new Date().toISOString();
return {
// The backend owns the real id; this one only has to be unique enough to
// key the list until the first save returns.
// The backend keeps whatever id it is handed for a note it has not seen,
// so this one is the note's real id from the first save onward.
id: crypto.randomUUID(),
title: "",
body: "",
@@ -19,41 +19,108 @@ function draft(): Note {
};
}
/** Stable empty list, so a project with nothing cached does not re-render on identity. */
const NO_NOTES: Note[] = [];
/**
* Per-project mutation chain.
*
* A project's writes are serialised so that two of them cannot be in flight at
* once. The Rust `write_lock` stops an upsert and a delete *interleaving*; it
* does not order them, and the order is the part that matters here. Clicking
* Delete while the textarea has focus fires `blur` first, so `save_note` and
* `delete_note` are issued back to back — and if the delete wins the lock, the
* upsert behind it re-inserts the note and it comes back on the next load.
* "Fix a typo, decide the note is useless, delete it" is an ordinary sequence.
*
* Module scope, not hook scope, for the reason `useTerminal`'s input queue is:
* several components call `useNotes` for the same project (the Project Home
* tab and the dock), and a per-hook chain would give each its own ordering and
* leave them racing each other — which is the bug, not the fix.
*/
const mutationChains = new Map<string, Promise<unknown>>();
function enqueueMutation<T>(projectId: string, run: () => Promise<T>): Promise<T> {
const previous = mutationChains.get(projectId) ?? Promise.resolve();
// `run` on both arms: a failed mutation must not stall every later one.
const result = previous.then(run, run);
const tail = result.then(
() => {},
() => {},
);
mutationChains.set(projectId, tail);
void tail.then(() => {
// Drop the entry once idle, so closed projects do not accumulate.
if (mutationChains.get(projectId) === tail) mutationChains.delete(projectId);
});
return result;
}
/**
* Re-read the canonical list into the shared cache.
*
* A successful save stamps a new `updated_at` and the backend sorts on it, so
* the record's position has changed and positional patching would disagree
* with what a reload would show. The backend owns the order; the webview never
* sorts. A failed re-read leaves the cache alone rather than clearing it.
*/
async function refresh(projectId: string): Promise<boolean> {
try {
const reloaded = await commands.listNotes(projectId);
useAppState.getState().setProjectNotes(projectId, reloaded);
return true;
} catch {
return false;
}
}
/**
* A project's notes, cached from the backend.
*
* The backend is the source of truth and this is a cache — every mutation goes
* through a command and the returned record replaces the local one, so the
* list can never drift from the file. `saveState` mirrors `useProjectSave` so
* `ui/SaveIndicator` can report the outcome: a save that fails silently is a
* user staring at text they believe is stored.
* The backend is the source of truth and the zustand slice is the cache —
* every mutation goes through a command and the returned list replaces the
* cached one, so the list can never drift from the file. The cache lives in
* the store rather than in this hook because two surfaces show the same
* project's notes at once; see `notesByProject`.
*
* `saveState` is deliberately *not* shared: it is this panel's report of this
* panel's write, and `ui/SaveIndicator` is per-panel. A save that fails
* silently is a user staring at text they believe is stored.
*/
export function useNotes(projectId: string) {
const [notes, setNotes] = useState<Note[]>([]);
const [loading, setLoading] = useState(true);
const [saveState, setSaveState] = useState<SaveState>({ status: "idle", error: null });
const cached = useAppState((s) => s.notesByProject[projectId]);
const pushToast = useAppState((s) => s.pushToast);
const [saveState, setSaveState] = useState<SaveState>({ status: "idle", error: null });
const resetTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
const currentProjectId = useRef(projectId);
currentProjectId.current = projectId;
useEffect(() => {
if (!projectId) {
setNotes([]);
setLoading(false);
return;
}
let cancelled = false;
setLoading(true);
setNotes([]);
if (!projectId) return;
// Read through `getState` rather than through subscribed values: the
// effect must fire once per project, not again every time the flag it sets
// changes. Two panels mounting for the same project therefore make one
// read, and the second renders from the cache with no loading flash.
const store = useAppState.getState();
if (store.notesLoading[projectId]) return;
store.setNotesLoading(projectId, true);
commands
.listNotes(projectId)
.then((loaded) => {
if (!cancelled) setNotes(loaded);
useAppState.getState().setProjectNotes(projectId, loaded);
})
.catch((e) => {
if (cancelled) return;
setNotes([]);
// A project that has never been read caches the empty list, so a panel
// does not sit on "Loading notes…" forever. One that *has* been read
// keeps what it has: this load is a refresh behind a list already on
// screen — the second surface mounting, say — and a failed refresh
// must not blank both of them. Same rule as `refresh()`. Neither
// branch has a stale-project hazard, because the write is keyed by the
// project it belongs to.
const store = useAppState.getState();
if (store.notesByProject[projectId] === undefined) {
store.setProjectNotes(projectId, []);
}
pushToast({
kind: "error",
message: "Could not load notes for this project",
@@ -61,11 +128,8 @@ export function useNotes(projectId: string) {
});
})
.finally(() => {
if (!cancelled) setLoading(false);
useAppState.getState().setNotesLoading(projectId, false);
});
return () => {
cancelled = true;
};
}, [projectId, pushToast]);
useEffect(
@@ -75,6 +139,27 @@ export function useNotes(projectId: string) {
[],
);
// The indicator belongs to whatever project this panel is showing *now*.
// Without this, switching project mid-save leaves the new project's
// SaveIndicator stuck on the old project's "Saving…" — the same wrong-project
// report as flashing its "Saved ✓", just in the other direction.
useEffect(() => {
if (resetTimer.current) clearTimeout(resetTimer.current);
setSaveState({ status: "idle", error: null });
}, [projectId]);
/**
* Whether this hook is still looking at the project a queued mutation was
* issued for. Only the *reporting* is gated on it — the cache write is not,
* because it is keyed by project and belongs to that project either way.
* Without this, the new project's SaveIndicator flashes "Saved ✓" for the
* old project's write.
*/
const isCurrent = useCallback(
() => currentProjectId.current === projectId,
[projectId],
);
const succeeded = useCallback(() => {
setSaveState({ status: "saved", error: null });
if (resetTimer.current) clearTimeout(resetTimer.current);
@@ -85,68 +170,99 @@ export function useNotes(projectId: string) {
}, []);
const saveNote = useCallback(
async (note: Note) => {
if (resetTimer.current) clearTimeout(resetTimer.current);
setSaveState({ status: "saving", error: null });
try {
await commands.saveNote(projectId, note);
// Re-read the canonical list from the backend. A successful save stamps a new
// `updated_at`, and the backend sorts unpinned notes by `updated_at` descending,
// so the record's position has changed and positional patching would disagree with
// what a reload would show.
try {
const reloaded = await commands.listNotes(projectId);
// Guard against a stale callback: if the project changed while the save was in
// flight, don't overwrite the new project's list with the old one. The save itself
// still succeeded, so succeeded() still fires; only the list replacement is skipped.
if (currentProjectId.current === projectId) {
setNotes(reloaded);
}
} catch {
// Keep the save reported as successful (it was) and leave the existing list alone
// rather than clearing it if the re-read fails.
(note: Note) =>
enqueueMutation(projectId, async () => {
if (isCurrent()) {
if (resetTimer.current) clearTimeout(resetTimer.current);
setSaveState({ status: "saving", error: null });
}
succeeded();
return true;
} catch (e) {
const message = String(e);
setSaveState({ status: "failed", error: message });
pushToast({ kind: "error", message: "Could not save note", detail: message });
return false;
}
},
[projectId, pushToast, succeeded],
try {
await commands.saveNote(projectId, note);
await refresh(projectId);
if (isCurrent()) succeeded();
return true;
} catch (e) {
const message = String(e);
if (isCurrent()) setSaveState({ status: "failed", error: message });
// The toast is not project-scoped — it names the failure and stays
// readable after a switch — so it fires either way.
pushToast({ kind: "error", message: "Could not save note", detail: message });
return false;
}
}),
[projectId, pushToast, succeeded, isCurrent],
);
const createNote = useCallback(async () => {
const note = draft();
// Held locally first so the editor can focus it immediately; the save
// happens on blur like every other edit. The note does not exist backend-side,
// so no re-read can place it and no backend ordering applies to it yet. Prepending
// puts it at the top where the user can see it immediately, and on the first save
// its canonical position is established.
setNotes((current) => [note, ...current]);
return note;
}, []);
const deleteNote = useCallback(
async (noteId: string) => {
try {
await commands.deleteNote(projectId, noteId);
// Guard against a stale callback: if the project changed while the delete was in
// flight, don't modify the list. Note ids are UUIDs so a stale filter cannot match
// another project's note, but applying the same guard for consistency.
if (currentProjectId.current === projectId) {
setNotes((current) => current.filter((n) => n.id !== noteId));
/**
* Create a note by persisting it, rather than holding it locally until the
* first blur.
*
* The draft used to live only in the list, which meant any *other* note
* being saved replaced the list with the backend's and the unsaved draft
* silently vanished — click "New note" twice, type in the second, blur, and
* the first row is gone. Sharing one cache between two surfaces makes that
* worse rather than better: a local-only row would exist in whichever panel
* created it and nowhere else. Letting the backend own the row from the
* start removes the whole class: there is no such thing as a note in the
* list that the file does not have.
*/
const createNote = useCallback(
() =>
enqueueMutation(projectId, async () => {
const note = draft();
try {
const saved = await commands.saveNote(projectId, note);
if (!(await refresh(projectId))) {
// The note exists; only the re-read failed. Show it rather than
// leaving the user with a button that did nothing visible.
const store = useAppState.getState();
store.setProjectNotes(projectId, [
saved,
...(store.notesByProject[projectId] ?? []),
]);
}
return saved;
} catch (e) {
pushToast({
kind: "error",
message: "Could not create note",
detail: String(e),
});
return null;
}
return true;
} catch (e) {
pushToast({ kind: "error", message: "Could not delete note", detail: String(e) });
return false;
}
},
}),
[projectId, pushToast],
);
return { notes, loading, saveState, createNote, saveNote, deleteNote };
const deleteNote = useCallback(
(noteId: string) =>
enqueueMutation(projectId, async () => {
try {
await commands.deleteNote(projectId, noteId);
const store = useAppState.getState();
store.setProjectNotes(
projectId,
(store.notesByProject[projectId] ?? []).filter((n) => n.id !== noteId),
);
return true;
} catch (e) {
pushToast({ kind: "error", message: "Could not delete note", detail: String(e) });
return false;
}
}),
[projectId, pushToast],
);
return {
notes: cached ?? NO_NOTES,
// Only "loading" before the project has ever been read — never on a
// refresh behind a list that is already on screen, and never on the second
// panel to mount for a project the first one already fetched. A failed
// load caches the empty list, so this cannot latch on.
loading: Boolean(projectId) && cached === undefined,
saveState,
createNote,
saveNote,
deleteNote,
};
}