diff --git a/neode-ui/src/composables/useCachedResource.ts b/neode-ui/src/composables/useCachedResource.ts index a093e90b..804984d7 100644 --- a/neode-ui/src/composables/useCachedResource.ts +++ b/neode-ui/src/composables/useCachedResource.ts @@ -126,6 +126,9 @@ export function useCachedResource(opts: CachedResourceOptions): CachedReso ageMs: computed(() => (entry.fetchedAt === null ? null : Date.now() - entry.fetchedAt)), refresh, invalidate: () => store.invalidate(opts.key), - optimistic: (update) => store.optimistic(opts.key, update), + // Pass this resource's own already-decided `persist` through explicitly + // (WR-04) — store.optimistic() requires it rather than defaulting, so + // the entry's persist decision can never silently diverge by omission. + optimistic: (update) => store.optimistic(opts.key, update, persist), } } diff --git a/neode-ui/src/stores/__tests__/resources.test.ts b/neode-ui/src/stores/__tests__/resources.test.ts index d96efb23..32275a57 100644 --- a/neode-ui/src/stores/__tests__/resources.test.ts +++ b/neode-ui/src/stores/__tests__/resources.test.ts @@ -16,7 +16,7 @@ describe('resources store — stale-while-revalidate semantics', () => { it('first fetch goes idle → loading → ready with data', async () => { const store = useResourcesStore() - const e = store.entry('k1') + const e = store.entry('k1', true) expect(e.loadState).toBe('idle') const p = store.refresh('k1', async () => 'hello') expect(e.loadState).toBe('loading') @@ -29,7 +29,7 @@ describe('resources store — stale-while-revalidate semantics', () => { it('sticky-ready: refresh never regresses ready → loading', async () => { const store = useResourcesStore() await store.refresh('k2', async () => 1) - const e = store.entry('k2') + const e = store.entry('k2', true) const p = store.refresh('k2', async () => 2) expect(e.loadState).toBe('refreshing') await p @@ -40,7 +40,7 @@ describe('resources store — stale-while-revalidate semantics', () => { it('keeps last-known data on refresh error (ready + error set)', async () => { const store = useResourcesStore() await store.refresh('k3', async () => 'good') - const e = store.entry('k3') + const e = store.entry('k3', true) await store.refresh('k3', async () => { throw new Error('boom') }) @@ -54,7 +54,7 @@ describe('resources store — stale-while-revalidate semantics', () => { await store.refresh('k4', async () => { throw new Error('down') }) - const e = store.entry('k4') + const e = store.entry('k4', true) expect(e.loadState).toBe('error') expect(e.data).toBeNull() }) @@ -74,7 +74,7 @@ describe('resources store — stale-while-revalidate semantics', () => { // Fresh pinia = fresh memory cache, same sessionStorage. setActivePinia(createPinia()) const store2 = useResourcesStore() - const e = store2.entry<{ n: number }>('k6') + const e = store2.entry<{ n: number }>('k6', true) expect(e.loadState).toBe('ready') expect(e.data).toEqual({ n: 42 }) }) @@ -82,8 +82,8 @@ describe('resources store — stale-while-revalidate semantics', () => { it('optimistic update applies immediately and rollback restores', async () => { const store = useResourcesStore() await store.refresh('k7', async () => ['a']) - const e = store.entry('k7') - const rollback = store.optimistic('k7', (cur) => [...(cur ?? []), 'b']) + const e = store.entry('k7', true) + const rollback = store.optimistic('k7', (cur) => [...(cur ?? []), 'b'], true) expect(e.data).toEqual(['a', 'b']) rollback() expect(e.data).toEqual(['a']) @@ -95,7 +95,7 @@ describe('resources store — stale-while-revalidate semantics', () => { const revalidate = vi.fn() store.subscribe('k8', revalidate) store.invalidate('k8') - expect(store.entry('k8').fetchedAt).toBeNull() + expect(store.entry('k8', true).fetchedAt).toBeNull() expect(revalidate).not.toHaveBeenCalled() vi.advanceTimersByTime(900) expect(revalidate).toHaveBeenCalledTimes(1) diff --git a/neode-ui/src/stores/__tests__/resourcesClear.test.ts b/neode-ui/src/stores/__tests__/resourcesClear.test.ts index 7bda3863..78fb3fd0 100644 --- a/neode-ui/src/stores/__tests__/resourcesClear.test.ts +++ b/neode-ui/src/stores/__tests__/resourcesClear.test.ts @@ -73,7 +73,7 @@ describe('resources store — clearAll (T-02-02 logout purge)', () => { // The old fetch resolved after clearAll's generation bump — its result // must be dropped, not written into a fresh entry or sessionStorage. expect(store.entries.has('k2')).toBe(false) - const e = store.entry('k2') + const e = store.entry('k2', true) expect(e.data).toBeNull() expect(sessionStorage.getItem('resource:k2')).toBeNull() }) diff --git a/neode-ui/src/stores/resources.ts b/neode-ui/src/stores/resources.ts index 533d1b7c..f7fe2ac1 100644 --- a/neode-ui/src/stores/resources.ts +++ b/neode-ui/src/stores/resources.ts @@ -56,6 +56,11 @@ export const useResourcesStore = defineStore('resources', () => { const inflight = new Map>() const revalidators = new Map void>>() const invalidateTimers = new Map>() + // Records each key's persist decision at creation time (WR-04) — reused to + // detect (and warn on, in dev) a later call for the same key disagreeing + // about persist, since that almost always means two call sites disagree + // about whether a cache key is safe to write to sessionStorage (T-02-01). + const entryPersist = new Map() // Bumped by clearAll() so an in-flight fetch from a just-ended session that // resolves afterward can detect it and skip writing its result — without // this guard the resolving promise would still call writeSnapshot() and @@ -64,8 +69,12 @@ export const useResourcesStore = defineStore('resources', () => { /** Get (or create) the reactive entry for a key, hydrating from the * sessionStorage snapshot on first sight so revisits after a reload paint - * before any RPC completes. Pass `persist: false` to skip snapshots. */ - function entry(key: string, persist = true): ResourceEntry { + * before any RPC completes. `persist` is REQUIRED (no default) so a call + * site can never silently opt a cache key into sessionStorage by omission + * (WR-04/T-02-01) — it only takes effect on the very first call for a + * given key; every later call for the same key must keep passing the SAME + * decision, asserted (dev-only warning) rather than silently reused. */ + function entry(key: string, persist: boolean): ResourceEntry { let e = entries.get(key) if (!e) { const snap = persist ? readSnapshot(key) : null @@ -76,6 +85,12 @@ export const useResourcesStore = defineStore('resources', () => { error: null, }) entries.set(key, e) + entryPersist.set(key, persist) + } else if (import.meta.env.DEV && entryPersist.get(key) !== persist) { + console.warn( + `[resources] entry("${key}") called with persist=${persist}, but this key was already created with persist=${entryPersist.get(key)}. ` + + 'The original decision wins for the lifetime of this entry — persist is decided once per key, never per call (T-02-01).', + ) } return e as ResourceEntry } @@ -149,9 +164,14 @@ export const useResourcesStore = defineStore('resources', () => { } /** Optimistically apply `update` to the cached value; returns a rollback. - * Pattern: rollback on RPC failure (generalized TransportPrefsCard). */ - function optimistic(key: string, update: (current: T | null) => T): () => void { - const e = entry(key) + * Pattern: rollback on RPC failure (generalized TransportPrefsCard). + * `persist` is REQUIRED (no default) for the same reason as entry() — + * silently falling back to persist:true here was the exact footgun WR-04 + * flagged: a caller that runs before any useCachedResource({persist:false}) + * has created the entry would otherwise start writing to sessionStorage + * with no indication anything is wrong. */ + function optimistic(key: string, update: (current: T | null) => T, persist: boolean): () => void { + const e = entry(key, persist) const before = e.data const beforeState = e.loadState e.data = update(before) diff --git a/neode-ui/src/views/Cloud.vue b/neode-ui/src/views/Cloud.vue index f62b344e..e06894b9 100644 --- a/neode-ui/src/views/Cloud.vue +++ b/neode-ui/src/views/Cloud.vue @@ -827,7 +827,10 @@ async function runBrowsePeerPool(targets: PeerNode[]): Promise { } function peerBrowseEntry(onion: string) { - return resources.entry(peerBrowseKey(onion)) + // persist:true matches this key's existing resources.refresh() calls + // above, which never pass `{ persist: false }` either — WR-04 just makes + // that pre-existing decision explicit instead of relying on a default. + return resources.entry(peerBrowseKey(onion), true) } /** Transport badge data for a peer (null until its first browse resolves). */ diff --git a/neode-ui/src/views/PeerFiles.vue b/neode-ui/src/views/PeerFiles.vue index 9154d47a..3cf92d8e 100644 --- a/neode-ui/src/views/PeerFiles.vue +++ b/neode-ui/src/views/PeerFiles.vue @@ -643,7 +643,11 @@ interface PeerBrowse { } const peerOnion = computed(() => props.peerId || currentPeer.value?.onion || '') function browseEntry() { - return resources.entry(`cloud.peer-browse:${peerOnion.value}`) + // persist:true matches this key's existing resources.refresh() call in + // loadCatalog() below, which never passes `{ persist: false }` either — + // WR-04 just makes that pre-existing decision explicit instead of relying + // on a default. + return resources.entry(`cloud.peer-browse:${peerOnion.value}`, true) } const catalogItems = computed(() => browseEntry().data?.items ?? []) const catalogError = computed(() => browseEntry().error ?? '')