From 06ec2af4ff5cbe02a038d93ca38271c48fcec3d3 Mon Sep 17 00:00:00 2001 From: quifox <289420841+quifox@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:26:35 +0800 Subject: [PATCH] fix(client): ignore stale detail reads after refold (#106) --- docs/compatibility.md | 1 + src/client/timelineSource.ts | 30 ++++++---- tests/client/timelineSource.spec.ts | 92 ++++++++++++++++++++++++++++- 3 files changed, 109 insertions(+), 14 deletions(-) diff --git a/docs/compatibility.md b/docs/compatibility.md index 3683657..928fd5f 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -66,6 +66,7 @@ The plugin lives off data it does not own: the durable session log (event shapes - **Host-side projection folds are TOTAL.** The harness projection registry drives `apply` straight off the session/event bus with no error boundary of its own: one throwing fold stalls that unit's cells and its `session/projection` push feed, and the browser waits on "loading" forever. Unknown event types return the state unchanged; per-event processing is isolated so a malformed event is dropped whole (all-or-nothing, no partial state); and no `undefined`-valued property is ever materialized into persisted state — the plain-JSON precondition makes one such property fail EVERY projection-cache write for the session, breaking sessions in unrelated, far-away places. - **Client-side parsing degrades visibly.** Delivered projection payloads are sanitized at the boundary (the `timelineOf` pattern: collections re-proved, scalars zeroed, whole-value absence stays `null` → loading screen); per-item work in any fold over join/log data is isolated (per-item guards, or a bounded catch where a hostile object may throw on property access); every async fetch resolves to data or a visible retryable state, never an unhandled rejection that leaves a spinner. - **A session with no pushed `contextTimeline` value never stalls the view.** The projection store's `seed()` clears a key its baseline omits, and a checkpoint row can be missing or version-stale on a cold-observed session — so the Context tab or `/context` modal can sit with no value at all and no push on the way. The source opens the detail channel itself in that case (rev 0): the plugin's own `/api/dsh-context/detail` route folds the durable log on demand and serves the slim head beside the collections, the next pushed head takes the channel back per its own revision, and a settled-without-data read surfaces the retryable failure note (the same `DetailNote` the detail cards use) instead of an eternal spinner. +- **A refold invalidates earlier detail reads.** When the pushed revision decreases, the client resets its detail ledger and ignores any earlier read's data, absence, or failure. Once that read settles, the pending revision is fetched again; collections from the old fold never merge into the new head. - **Every field is re-proved at runtime.** Structural narrowing over blind casts; optional chaining over non-null assertions; elements that fail the shape are skipped instead of throwing. - **Every parser carries hostile fixtures next to its happy path**: wrong types, null/missing fields, null or primitive elements inside arrays, unpaired references, and objects that throw on property access. The repository's 100% per-file coverage bar applies to every guard branch — an untested guard is an unverified promise. diff --git a/src/client/timelineSource.ts b/src/client/timelineSource.ts index 16d2709..6d514d6 100644 --- a/src/client/timelineSource.ts +++ b/src/client/timelineSource.ts @@ -17,9 +17,9 @@ * The no-stale-content guarantees: the store is per session and shared by * the tab and the modal; a refetch is single-flight with a trailing edge * (a rev bumped mid-flight re-reads after settle); responses race-safe by - * revision (latest wins, with the refold exception — a host that refolded - * restarts the revision, and a response matching the requested rev is - * accepted regardless of order); a transport failure keeps the last good + * generation and revision (a host refold invalidates reads started before + * its revisions restarted, and latest wins within a generation); a + * transport failure keeps the last good * detail and backs off; an absent answer (the session left the live set) * stops the trailing until the head moves again. With no detail at all, * failure surfaces as a retryable note on the detail cards instead of an @@ -134,6 +134,7 @@ export class DetailStore { private targetRev = -1 /** The head rev seen last — a DECREASE means the host refolded (revisions restart). */ private lastHeadRev = -1 + private generation = 0 private failed = false private inFlight = false private timer: ReturnType | null = null @@ -161,6 +162,7 @@ export class DetailStore { */ request(rev: number): void { if (rev < this.lastHeadRev) { + this.generation++ this.acceptedRev = -1 this.wantedRev = -1 this.detail = null @@ -206,13 +208,13 @@ export class DetailStore { } this.inFlight = true this.targetRev = this.wantedRev + const generation = this.generation this.emit() try { const d = await this.fetcher() + if (generation !== this.generation) return if (d !== null) { - // Latest-wins, with the refold exception: a response matching the - // requested rev is the current truth even when its number trails a - // pre-refold landing. + // Compare revisions only within the generation that started this read. if (d.rev >= this.acceptedRev || d.rev === this.targetRev) { this.detail = d this.acceptedRev = d.rev @@ -228,14 +230,16 @@ export class DetailStore { this.failures++ } } catch { - this.failed = this.detail === null - this.failures++ + if (generation === this.generation) { + this.failed = this.detail === null + this.failures++ + } + } finally { + this.inFlight = false + // The head moved or refolded while the read settled: trail once more. + if (this.wantedRev > this.acceptedRev) this.schedule() + this.emit() } - this.inFlight = false - // The head moved while the read settled (or an earlier read lost the - // race): trail once more, debounced. - if (this.wantedRev > this.acceptedRev) this.schedule() - this.emit() } private emit(): void { diff --git a/tests/client/timelineSource.spec.ts b/tests/client/timelineSource.spec.ts index 5f2516d..42e0b57 100644 --- a/tests/client/timelineSource.spec.ts +++ b/tests/client/timelineSource.spec.ts @@ -254,6 +254,65 @@ describe('DetailStore', () => { await until(() => store.getSnapshot().detail?.rev === 2, 'the refold reset never refetched') }) + for (const outcome of ['detail', 'absent', 'failure'] as const) { + for (const timerFired of [false, true]) { + test(`a refold ignores an old ${outcome} before a new read (timer fired: ${String(timerFired)})`, async () => { + vi.useFakeTimers() + let resolve!: (value: ContextTimelineDetail | null) => void + let reject!: (reason: Error) => void + let calls = 0 + const first = new Promise((res, rej) => { + resolve = res + reject = rej + }) + const store = new DetailStore(() => { + calls++ + return calls === 1 ? first : Promise.resolve(detail(2)) + }, 10) + store.request(5) + await vi.advanceTimersByTimeAsync(10) + store.request(2) + if (timerFired) await vi.advanceTimersByTimeAsync(10) + assert.equal(calls, 1, 'the reset never stacks a concurrent read') + + if (outcome === 'failure') reject(new Error('offline')) + else resolve(outcome === 'detail' ? detail(5) : null) + await vi.advanceTimersByTimeAsync(0) + assert.equal(store.getSnapshot().detail, null, 'old collections must not land under the new head') + assert.equal(store.getSnapshot().failed, false, 'old absence or failure must not fail the new generation') + assert.equal(store.getSnapshot().pending, true, 'the new generation still needs its read') + + await vi.advanceTimersByTimeAsync(10) + assert.equal(calls, 2) + assert.equal(store.getSnapshot().detail?.rev, 2) + assert.equal(store.getSnapshot().failed, false) + assert.equal(store.getSnapshot().pending, false) + }) + } + } + + test('several refolds during one read converge to the latest generation', async () => { + vi.useFakeTimers() + let resolve!: (value: ContextTimelineDetail | null) => void + let calls = 0 + const first = new Promise(res => { resolve = res }) + const store = new DetailStore(() => { + calls++ + return calls === 1 ? first : Promise.resolve(detail(1)) + }, 10) + store.request(5) + await vi.advanceTimersByTimeAsync(10) + store.request(3) + store.request(1) + await vi.advanceTimersByTimeAsync(10) + resolve(detail(5)) + await vi.advanceTimersByTimeAsync(0) + assert.equal(store.getSnapshot().detail, null) + await vi.advanceTimersByTimeAsync(10) + assert.equal(calls, 2) + assert.equal(store.getSnapshot().detail?.rev, 1) + }) + test('a transport failure with no detail arms the failed state; the backoff trailing recovers on its own', async () => { const { fetcher, calls } = scriptedFetcher([new Error('offline'), detail(1)]) const store = new DetailStore(fetcher, 20) @@ -430,6 +489,38 @@ function probeRead(container: HTMLElement, key: string): string { } describe('useTimelineSource', () => { + test('a refold never merges the previous generation into the pushed head', async () => { + vi.useFakeTimers() + const response = (value: ContextTimelineDetail) => ({ ok: true, json: async () => ({ ok: true, value }) }) + let resolve!: (value: ReturnType) => void + let calls = 0 + const first = new Promise>(res => { resolve = res }) + vi.stubGlobal('fetch', () => { + calls++ + return calls === 1 ? first : Promise.resolve(response(detail(2))) + }) + const ctx = asClientCtx(new TestClientCtx()) + const m = await mount(h(SourceProbe, { ctx, sessionId: 's1', value: slimHead(5) })) + try { + await act(async () => { await vi.advanceTimersByTimeAsync(300) }) + await m.update(h(SourceProbe, { ctx, sessionId: 's1', value: slimHead(2) })) + await act(async () => { await vi.advanceTimersByTimeAsync(300) }) + await act(async () => { + resolve(response(detail(5))) + await vi.advanceTimersByTimeAsync(0) + }) + assert.equal(probeRead(m.container, 'rev'), '2') + assert.equal(probeRead(m.container, 'state'), 'loading') + assert.equal(probeRead(m.container, 'steps'), '0', 'no rows from the discarded generation') + await act(async () => { await vi.advanceTimersByTimeAsync(300) }) + assert.equal(calls, 2) + assert.equal(probeRead(m.container, 'state'), 'ready') + assert.equal(detailStoreOf(ctx, 's1').getSnapshot().detail?.rev, 2) + } finally { + await m.unmount() + } + }) + test('the inline generation passes through untouched and never fetches', async () => { let calls = 0 const ctx = ctxWithCall(() => { @@ -619,4 +710,3 @@ describe('useTimelineSource', () => { await m.unmount() }) }) -