Post-mutation refetch can commit SWR-stale bodies (reaction live update flake)

crueber commented

2026-09-04 17:02:47 +00:00

Owner

Follow-up from #31 verification. The thread GET uses ccSWR (private, max-age=0, stale-while-revalidate=60). After a mutation, the explicit reload() refetch and the SSE-frame refetch race: Chrome serves the pre-mutation body with status 200 from disk cache (fromDiskCache=true, observed via CDP), and with no ordering guard in web/src/lib/data.js the stale commit can land last, leaving the timeline stale until the next event. Reproduced ~6/10 runs against the local standalone stack (disk 99% full slows cache reads into losing positions). Evidence: CDP net log [201 reactions, 200 cached=true, 200 cached=false] with DOM stuck pre-mutation; deterministic-fresh with Network.setCacheDisabled. Candidate fix (own vehicle, cross-cutting: comments/patches share the path): bypass HTTP cache on mutation-triggered refetches and/or treat SDK 304 as silent keep-current instead of ReposError(304) into the tray.

crueber referenced this issue

2026-09-04 17:02:51 +00:00

Design Fix for http://192.168.2.48:8080/crueber/walhub/issues/1 #31

crueber referenced this issue from a commit

2026-09-04 19:44:15 +00:00

Fix #41: mutation-refetch ordering guard + HTTP-cache bypass (12_web_ui.md 2.4)

crueber referenced this issue from a pull request that will close it,

2026-09-04 19:44:17 +00:00

Fix #41: mutation refetch ordering guard #52

crueber commented

2026-09-04 19:44:24 +00:00

Author

Owner

Fix PR: #52 (branch fix/issue-41) — ordering/generation guard + cache-bypass on mutation refetches + SDK 304 as silent keep-current. node suite 217/217 green; no merge (awaiting review).

Fix PR: https://git.packden.us/crueber/walhub/pulls/52 (branch fix/issue-41) — ordering/generation guard + cache-bypass on mutation refetches + SDK 304 as silent keep-current. node suite 217/217 green; no merge (awaiting review).

crueber commented

2026-09-04 19:46:03 +00:00

Author

Owner

Reviewed PR #52 (fix/issue-41) in scratch worktree at 2e15899. No fixes needed — no new branch pushed.

VERIFIED (scratch worktree, removed afterward; main worktree untouched, read-only):

node --test web/test/unit/*.test.js: 217/217 pass (incl. new data-guard + sdk-nostore-304 suites). Note: worktree needed a node_modules symlink to the main checkout to run; no installs, no main-tree writes.
vite build: OK (103 modules, built in ~1.6s). esbuild SDK bundle: OK (26.8kb).
No browser run (node tests + reasoning per instructions); no docker/compose touched.
Diff touches exactly 7 files: docs/go/12_web_ui.md, web/sdk/src/{core,errors,index}.js, web/src/lib/data.js, 2 new test files. Zero useData call-site/page changes by construction; no package.json/lockfile changes (no new deps — law 1 holds).

REVIEW FINDINGS (all checked, none blocking):

Generation guard (data.js:79-106): CORRECT. Stale bodies (L89) and stale errors (L96) both dropped via generation !== entry.seq; invalidate (L180-187) unconditionally starts a new generation (old !entry.promise skip removed — the actual fix). No live-lock: generations only increment on start/invalidate, fetch completion never schedules new invalidations, startIfStale still single-flights (L121) so background reads coalesce as before. Minor behavior delta (not a bug): invalidate on an entry with no refetch fn is now a silent no-op, whereas old code refetched Promise.resolve(current value) — old path was a valueless self-copy; fine.
withNoStore nesting (core.js:286-293): EXACT. try/finally restores on sync throw (covered by test); nesting counts correctly. Async-fn caveat (counter disarms at sync return) is real but documented in the jsdoc and harmless here: invalidate wraps refetch() synchronously (data.js:184-185), and _request reads depth synchronously before the first await, so the bypass always arms for the intended path. Fail-open direction (cached read) is the safe one.
304 sentinel: CORRECT at both dispatch points (_dispatch L370, _textResponse L389 — checked before !res.ok so 304 never throws). Handled in the only cache path that matters: start() L90 (silent keep-current, freshness bumped, no tray). OBSERVATION (non-blocking): two direct-GET call sites outside the data layer would misread a raw sentinel — Notifications.jsx:16 (res.count ?? 0 → badge 0) and :47-49 (res.notifications ?? [] → blank list). Unreachable in practice (SDK never sends If-None-Match/If-Modified-Since, so no server/proxy should answer 304), and mutations are non-GET so never 304. Suggest a one-line defensive guard there in a follow-up if you want belt-and-braces, but not a merge blocker.
prefetchData seam (data.js:153-159): runtime-neutral. Same ensureEntry→touch/evict→start path; only caller is the new test file (grep-confirmed, no runtime imports). start() with keepRefetch=false sets refetch exactly as the old code did.
Normal-operation preservation: useData/startIfStale/TTL/LRU/tray logic byte-identical apart from the guard; useResolved flows through start() so both steps get the guard + 304 handling, and the sentinel never reaches component signals (intercepted before signal set).
Docs (12_web_ui.md): accurate — withNoStore fallback ('plain refetch when no SDK client wired', data.js:185), 304 keep-current semantics, and storm-coalescing note all match the code.

MERGE RECOMMENDATION: ready to merge.

Reviewed PR #52 (fix/issue-41) in scratch worktree at 2e15899. No fixes needed — no new branch pushed. VERIFIED (scratch worktree, removed afterward; main worktree untouched, read-only): - node --test web/test/unit/*.test.js: 217/217 pass (incl. new data-guard + sdk-nostore-304 suites). Note: worktree needed a node_modules symlink to the main checkout to run; no installs, no main-tree writes. - vite build: OK (103 modules, built in ~1.6s). esbuild SDK bundle: OK (26.8kb). - No browser run (node tests + reasoning per instructions); no docker/compose touched. - Diff touches exactly 7 files: docs/go/12_web_ui.md, web/sdk/src/{core,errors,index}.js, web/src/lib/data.js, 2 new test files. Zero useData call-site/page changes by construction; no package.json/lockfile changes (no new deps — law 1 holds). REVIEW FINDINGS (all checked, none blocking): 1. Generation guard (data.js:79-106): CORRECT. Stale bodies (L89) and stale errors (L96) both dropped via generation !== entry.seq; invalidate (L180-187) unconditionally starts a new generation (old !entry.promise skip removed — the actual fix). No live-lock: generations only increment on start/invalidate, fetch completion never schedules new invalidations, startIfStale still single-flights (L121) so background reads coalesce as before. Minor behavior delta (not a bug): invalidate on an entry with no refetch fn is now a silent no-op, whereas old code refetched Promise.resolve(current value) — old path was a valueless self-copy; fine. 2. withNoStore nesting (core.js:286-293): EXACT. try/finally restores on sync throw (covered by test); nesting counts correctly. Async-fn caveat (counter disarms at sync return) is real but documented in the jsdoc and harmless here: invalidate wraps refetch() synchronously (data.js:184-185), and _request reads depth synchronously before the first await, so the bypass always arms for the intended path. Fail-open direction (cached read) is the safe one. 3. 304 sentinel: CORRECT at both dispatch points (_dispatch L370, _textResponse L389 — checked before !res.ok so 304 never throws). Handled in the only cache path that matters: start() L90 (silent keep-current, freshness bumped, no tray). OBSERVATION (non-blocking): two direct-GET call sites outside the data layer would misread a raw sentinel — Notifications.jsx:16 (res.count ?? 0 → badge 0) and :47-49 (res.notifications ?? [] → blank list). Unreachable in practice (SDK never sends If-None-Match/If-Modified-Since, so no server/proxy should answer 304), and mutations are non-GET so never 304. Suggest a one-line defensive guard there in a follow-up if you want belt-and-braces, but not a merge blocker. 4. prefetchData seam (data.js:153-159): runtime-neutral. Same ensureEntry→touch/evict→start path; only caller is the new test file (grep-confirmed, no runtime imports). start() with keepRefetch=false sets refetch exactly as the old code did. 5. Normal-operation preservation: useData/startIfStale/TTL/LRU/tray logic byte-identical apart from the guard; useResolved flows through start() so both steps get the guard + 304 handling, and the sentinel never reaches component signals (intercepted before signal set). 6. Docs (12_web_ui.md): accurate — withNoStore fallback ('plain refetch when no SDK client wired', data.js:185), 304 keep-current semantics, and storm-coalescing note all match the code. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-04 19:46:35 +00:00

crueber referenced this issue from a commit

2026-09-04 19:46:35 +00:00

Merge pull request 'Fix #41: mutation refetch ordering guard' (#52) from fix/issue-41 into main

crueber commented

2026-09-04 19:46:36 +00:00

Author

Owner

Fixed by PR #52 (review clean; 217/217 node tests), merged. Closing.

crueber referenced this issue

2026-09-04 20:02:13 +00:00