Order the notes cache by sequence, not by who resolves last
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 2m48s
Build App (Preview) / build-linux (pull_request) Successful in 6m2s
Build App (Preview) / build-windows (pull_request) Successful in 6m4s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 2m48s
Build App (Preview) / build-linux (pull_request) Successful in 6m2s
Build App (Preview) / build-windows (pull_request) Successful in 6m4s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
One gesture puts two requests in flight. With the tab already loaded, clicking the dock toggle while the textarea has focus fires `blur` -> `saveNote` and the dock's mount -> `list_notes` in the same tick. The save finishes and its re-read writes the post-save list; the mount's read -- issued earlier, still out -- then lands its pre-save snapshot on top, and both panels show stale text until something else refreshes. `mutationChains` could not have caught this: it orders a project's writes against each other and the mount load is a read outside it. Putting the read on the chain would work, but it buys correctness with latency the user feels -- a panel mount waiting behind `save_note`'s double-fsync write -- and leaves a "mutation chain" holding reads. The two requests are not competing for a resource; the loser's result is simply older. So every write into `notesByProject[p]` now claims a per-project sequence when the request behind it is issued, and `commitNotes` drops one whose sequence predates what is already cached. Reads take their sequence at issue time, since being ordered by resolution is the bug. Local patches -- the filter behind a confirmed delete, the prepend behind a failed re-read -- take a fresh one at commit time, because they are authoritative then rather than derived from an earlier read, and anything still in flight behind them is genuinely stale. A failed read commits under its *own* sequence, not a fresh one, so its empty list cannot beat a later read that has the real answer. `isCurrent()` stays, and is not folded in. It guards `setSaveState`, not the cache: it asks whether this *panel* is still showing the project a save was made for, which a per-project counter cannot answer -- two panels on one project share every sequence value. Ordering and panel identity are two questions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HjL1E2JFNctUqCYotUwqqb
This commit is contained in:
@@ -369,4 +369,78 @@ describe("useNotes", () => {
|
||||
expect(dock.result.current.loading).toBe(false);
|
||||
expect(dock.result.current.notes).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("does not let a slow mount read overwrite a fresher post-save refresh", async () => {
|
||||
// One gesture, two requests. The tab is already loaded; the user clicks the
|
||||
// dock toggle with the textarea focused, so `blur` → `saveNote` and the
|
||||
// dock's mount → `list_notes` are issued in the same tick. The save's
|
||||
// re-read writes the post-save list; the mount's read — issued earlier,
|
||||
// still in flight — must not then land its pre-save snapshot on top of it.
|
||||
const files = fakeBackend({ p1: [note({ body: "before" })] });
|
||||
const tab = renderHook(() => useNotes("p1"));
|
||||
await waitFor(() => expect(tab.result.current.loading).toBe(false));
|
||||
expect(tab.result.current.notes[0].body).toBe("before");
|
||||
|
||||
// The dock's mount read: it snapshots the list as it is *now* (pre-save)
|
||||
// and hangs, standing in for a plain read that is slower than
|
||||
// `save_note`'s double-fsync write plus the re-read behind it.
|
||||
let releaseMountRead: (() => void) | undefined;
|
||||
listNotes.mockImplementationOnce(async (p: string) => {
|
||||
const preSave = [...(files[p] ?? [])];
|
||||
await new Promise<void>((resolve) => {
|
||||
releaseMountRead = resolve;
|
||||
});
|
||||
return preSave;
|
||||
});
|
||||
const dock = renderHook(() => useNotes("p1"));
|
||||
expect(releaseMountRead).toBeDefined();
|
||||
|
||||
// The save and its re-read complete while that read is still out.
|
||||
await act(async () => {
|
||||
await tab.result.current.saveNote(note({ body: "after" }));
|
||||
});
|
||||
expect(tab.result.current.notes[0].body).toBe("after");
|
||||
|
||||
// Now the stale read lands.
|
||||
await act(async () => {
|
||||
releaseMountRead!();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(tab.result.current.notes[0].body).toBe("after");
|
||||
expect(dock.result.current.notes[0].body).toBe("after");
|
||||
});
|
||||
|
||||
it("does not let a slow mount read resurrect a note deleted while it was in flight", async () => {
|
||||
// The other half of the same ordering rule: a confirmed delete is newer
|
||||
// than any read issued before it finished, so the read's pre-delete list
|
||||
// must not be written back over the shortened one.
|
||||
const files = fakeBackend({ p1: [note()] });
|
||||
const tab = renderHook(() => useNotes("p1"));
|
||||
await waitFor(() => expect(tab.result.current.loading).toBe(false));
|
||||
|
||||
let releaseMountRead: (() => void) | undefined;
|
||||
listNotes.mockImplementationOnce(async (p: string) => {
|
||||
const preDelete = [...(files[p] ?? [])];
|
||||
await new Promise<void>((resolve) => {
|
||||
releaseMountRead = resolve;
|
||||
});
|
||||
return preDelete;
|
||||
});
|
||||
const dock = renderHook(() => useNotes("p1"));
|
||||
expect(releaseMountRead).toBeDefined();
|
||||
|
||||
await act(async () => {
|
||||
await tab.result.current.deleteNote("n1");
|
||||
});
|
||||
expect(tab.result.current.notes).toHaveLength(0);
|
||||
|
||||
await act(async () => {
|
||||
releaseMountRead!();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(tab.result.current.notes).toHaveLength(0);
|
||||
expect(dock.result.current.notes).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user