Files
xarmian 00d650a861 feat(server,web): detect a revocation that writes no item, and evict the cache it left stale (IDEA-2898) (#1261)
* feat(server): fingerprint the caller's visible set on the item doors (IDEA-2898)

The delta stream can only express changes to ROWS. A revocation that writes
no item — the ordinary shape of revocation — therefore produces no signal at
all, and a client's warm local index goes on serving rows for a collection
the caller can no longer see. `ItemPicker` lists those titles.

`computeAccessEpoch` hashes the caller's EFFECTIVE visible set (collection
ids + item grants, canonicalised by sorting, separated by a byte no id can
contain) and `/items-index`, `/items-changes` and the 60s SSE tick all carry
it. The value is opaque, derived only from the caller's own access, and
costs no extra query — both lists are already resolved before the response
is written.

ONE DEFINITION, deliberately. The delta door computes its epoch from the
LIVE grant set rather than from its own include-deleted query set: a caller
holding a grant on a soft-deleted item would otherwise get a different epoch
from each door, forever, and the client would resync to its page cap on
every poll. `handlers_items_access_epoch_test.go` pins the two doors against
each other on exactly that fixture — a grant on a soft-deleted item is the
one input that discriminates, and an earlier version of this test agreed for
the wrong reason because its fixture had no grants at all.

The unrestricted caller gets a SENTINEL ("all") rather than a hash of the
empty set, because the empty set is a real and opposite state: a restricted
member with zero visible collections. Hashing both the same would make the
widest and narrowest access indistinguishable.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8

* feat(web): compare the access fingerprint and evict the cache when it changes (IDEA-2898)

The client half of the signal. `WorkspaceState.accessEpoch` is the baseline
the cache was built under; it is persisted beside the cursor because the
revocation this closes can land while the tab is CLOSED, and a cache that
forgot its scope on reload could not detect that at all.

`ensureAccessScope` is the comparison, and it is ONE function with two call
sites — the page's `/items-changes` poll and bootstrap's own reconcile loop.
The first draft inlined it in the loop and had already diverged: the inline
copy skipped the null-baseline case, which is precisely the
offline-revocation cache the change exists for.

Three properties the tests pin, each of which was wrong at some point:

  - ABSENCE IS NOT A VALUE. A server that does not send the field is a
    mid-deploy older build, not a changed scope; and a snapshot carrying no
    epoch must not erase a baseline we already know, which sent the reconcile
    loop resyncing to its 50-page cap.
  - THE RESYNC MUST TERMINATE. When the snapshot carries no epoch the
    baseline would be unchanged and the next poll would ask again forever, so
    the epoch we were TOLD goes in as the fallback — and again after the
    await, for the case where the resync was JOINED rather than started and
    the fallback was never seen.
  - RAM AND DISK AGREE. `persistReplace` runs inside the resync, so the
    baseline is set before it, not patched after: a durable meta row a
    version behind the in-memory one makes the next warm boot resync for a
    scope that never changed.

SCOPE, stated plainly because it is a real limitation and not a rounding
error: this closes the single-tab case and the offline case. A write from a
tab that has not yet learned the new scope can still reinsert a row into the
durable cache while the cache advertises the current epoch, and nothing
re-fires until the next access change. That is F2, and PLAN-2903 owns it
together with the cross-tab cache-coherence work. Strictly better than the
pre-change state, where no revocation without a row change was detected at
all.

`LOCAL_INDEX_SCHEMA_VERSION` 3 -> 4: a cache written before this has no
baseline, and adopting the incoming epoch for it would be exactly the silent
adopt the change exists to prevent. Two fixtures that hard-coded 3 now read
the constant, so the next bump does not turn them into stale-cache fixtures
by accident.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8

* test(web): close the four seams a mutation run on the reduced tip found open (IDEA-2898)

The gates that ran on the full branch describe a different tree. A fresh
25-mutant run scoped to what this branch actually ships found four seams with
no coverage, three of them the same shape: the baseline being ADOPTED is
untested everywhere the adoption is silent.

That shape is why they survived a suite written for the eviction. Dropping an
adopt line does not stop a revocation being detected — the cache simply has no
baseline, and a null baseline over a populated cache resyncs, which LOOKS like
the change working. The discriminating case is the QUIET one: an unchanged
scope must cost nothing. All three new tests assert the absence of a resync.

  - warm hydrate adopting the PERSISTED epoch (the offline-revocation case;
    needs the mocked persistence module, since jsdom has no IndexedDB and the
    warm branch is otherwise unreachable)
  - the cold snapshot's epoch becoming the first-ever baseline
  - `applyDelta` handing `persistDelta` the baseline its rows were applied
    under, so the durable meta row is not stamped null after every delta

The fourth is the collection route's `ensureAccessScope` call, whose source
pin moved to PLAN-2903 with the pairing-guard argument it also covered. Half
of what it pinned still ships here, and the mutation run proved that half
uncovered — so the pin comes back NARROWED to the one surviving call site.

Its measured limit is in its docblock rather than assumed: deleting the call
kills it; short-circuiting the call (`if (false && await ...)`) SURVIVES,
because the text it matches is still on the line. A source pin cannot see
reachability. That survivor is reported, not hidden.

Instrument corrections made before any of this counted, both caught by the
runner's own controls rather than by inspection:

  - `go vet` was in the Go build gate. It flags unreachable code, so the
    positive control — an early `return` — scored BUILD-FAIL while compiling
    perfectly. A vet failure is not a build failure.
  - the web runs passed `--reporter=basic`, which this vitest does not have.
    Every web mutant failed to START and the classifier read that as a
    verdict. Fixed, and the classifier now requires the FULL baseline
    population (50 tests across 7 files) to have run before it will call
    anything killed — a mutant that stops a file loading also prints a
    failing summary.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8

* fix(web): an empty RAM state is not an empty cache, and a joined resync owes a durable epoch (IDEA-2898)

Two real defects from the review round on the reduced tip. Both are the
SILENT ADOPT class the change exists to prevent, arriving in the two places
the change itself created.

**A cache that has not answered yet is not an empty cache.** `bootstrap`
awaits `hydrate` before merging anything, but the SSE-driven `deltaSync` runs
on its own subscription rather than behind that await — so it can reach
`ensureAccessScope` with RAM empty and IDB holding rows from a scope nobody
has checked. The "nothing to evict, adopt silently" branch then stamps the
new epoch onto the durable cache through the delta that follows, and the
stale row hydrates under an epoch that agrees with the server FOREVER. Not
merely wrong once: permanently inert, which is worse than the defect this
change closes.

`cacheRead` makes the distinction the code was eliding. Declining to adopt
costs nothing and fails in the safe direction — the delta persists a null
epoch, and a null baseline over a populated cache resyncs on the next
reconcile.

**A joined resync updates RAM and leaves the disk behind.** Resyncs are
deduplicated per workspace. When `ensureAccessScope` JOINS one, that resync
already ran its own `persistReplace` under its own baseline, so assigning the
told epoch afterwards leaves the meta row recording the old one. The session
converges and every RELOAD hydrates the stale baseline and pays a full resync
for a scope that has not changed since. The code's own comment claimed the
epoch "lands in RAM and IDB together", which was true of the started path and
false of the joined one three lines below it.

`persistAccessEpoch` repairs just the epoch on an existing cache, on BOTH
branches — the first version of the fix had it on one, and no test in the
file could tell them apart until a mutant did.

Two tests changed because a PROPERTY changed, said out loud rather than
quietly rewritten: "adopts silently when there is no baseline and nothing
cached" held for a workspace that had never been bootstrapped, and no longer
does. It is now "…and the cache is known empty", and the unbootstrapped case
is its own test asserting the opposite. The `seedUnder` helper acquires its
baseline through a cold bootstrap, which is how a real session gets one
anyway.

Mutation matrix re-run whole on this tip: 31 real mutants, 30 killed. The
survivor is the page pin's short-circuit case, documented in its docblock.
Six of the mutants target these two fixes; one of them (`persistAccessEpoch`
minting a meta row) had to be rewritten after it survived for the wrong
reason — the naive version put a keyless row that IDB rejects, so the error
path compensated for the defect and the test never had to.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8

* fix(web): a failed cache read is not an empty cache, and the epoch patch is a compare-and-set (IDEA-2898)

Round 2 on the reduced tip, and both findings are the previous round's fixes
being not quite finished.

**`hydrate` returns the same empty payload for a FAILURE as for an empty
cache** — deliberately, since a best-effort cache should not take the app
down. `cacheRead` was set unconditionally after the await, so a transient IDB
failure read as "there is nothing stored" and re-opened the exact silent
adopt round 1 closed: the durable cache may hold rows from a scope nobody
checked, and adopting stamps the new epoch onto them through the next delta.

`HydrateResult.durableRead` carries the difference the payload cannot. True
when the read succeeded, true when IndexedDB is unsupported (nothing durable
can contradict anything later), true when the read found an incompatible
cache and wiped it — false only when a database that might hold rows could
not be opened or read. Failing that way costs a resync and hides nothing.

**`persistAccessEpoch` was a blind read-modify-write on a row other writers
own.** IDB serializes transactions, but a `persistDelta` or `persistReplace`
carrying a NEWER epoch can commit between the resync this caller joined and
the patch — and the overwrite would then stamp the older epoch onto rows
fetched under the newer one, so the next comparison reports a change that
never happened and pays a full resync for it. Now a compare-and-set against
the epoch the caller believes it is repairing.

Both hydrate failure paths are covered, and they needed different fixtures:
a database at a HIGHER format version (open fails outright) and one whose
`items` store is missing (the open succeeds, the read throws). They are one
statement written twice, and a mutant on either is invisible to a test of the
other — which is how the second one was found, by a mutant surviving a test
written for the first.

The mutation matrix is 36 real mutants on this tip, 35 killed; the survivor
remains the page pin's short-circuit case, documented in that file. One
mutant is worth naming because it survived for the WRONG reason twice: the
naive removal of `persistAccessEpoch`'s "no meta row" guard cannot be
detected, because without it the code dereferences `undefined` and the catch
swallows the throw, so the outcome is identical. The faithful version — a
guard replaced by code that actually mints a valid row — is killed. A mutant
has to be the defect, not a crash that happens to look like it.

Also corrected: `accessEpoch`'s doc claimed null exists only before the first
response of a session. Two things falsify that now — a server that sends no
epoch, and the pre-hydration guard.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8

* docs,test(web): the epoch patch's expected value is asserted, and null is not unreachable (IDEA-2898)

Round 3, two P3s and no behavioural findings.

The `HydrateResult.accessEpoch` doc said the version bump to 4 made a null
epoch unreachable for caches this build writes. It does not: a server that
predates `access_epoch` omits it, and `persistDelta`/`persistReplace` record
that absence honestly rather than inventing a value — a case the tests
already cover. What the bump actually rules out is a PRE-IDEA-2898 cache
being READ as though it had a baseline. Comment corrected to say the thing
that is true.

The two joined-resync tests asserted the epoch `persistAccessEpoch` is given
but not the value it expects to be REPLACING. The patch is a compare-and-set,
so a wrong `expectedPrevious` makes it a silent no-op that still looks right
in RAM, and the disagreement surfaces only as a resync on the next reload —
invisible to a single-session test. Both call sites now have their expected
value asserted, and two mutants (each call site given the wrong expectation)
are killed by them.

Claude-Session: https://claude.ai/code/session_01Xk9M5UVPdc84xL5E1mZkm8
2026-09-06 13:36:22 -04:00

75 lines
3.3 KiB
Go

package server
import (
"crypto/sha256"
"encoding/hex"
"sort"
)
// accessEpochUnrestricted is the epoch for a caller with no collection
// filtering at all (owner, editor with "all" access, cookie-session admin).
//
// It is a SENTINEL rather than a hash of the empty set, because the empty set
// is a real and opposite state: a restricted member with zero visible
// collections. Hashing both to the same value would make the widest and the
// narrowest access indistinguishable — a caller whose access was revoked down
// to nothing would carry the same epoch as an owner, and a promotion from
// nothing to everything would signal no change at all.
const accessEpochUnrestricted = "all"
// computeAccessEpoch fingerprints the caller's EFFECTIVE visible resource set
// so a client can tell that the set changed without being told what it is.
//
// IDEA-2898. The delta stream (/items-changes) can only express changes to
// ROWS. A revocation that writes no item — the ordinary shape of revocation —
// therefore produces no signal at all, and a client's warm local index goes on
// serving rows for a collection the caller can no longer see. This value is
// that missing signal: the client compares it against the one it stored and,
// on a change, runs the authoritative resync it already implements.
//
// Inputs are the two lists the item doors already resolve before writing their
// response, so this costs no additional query:
//
// - visibleCollectionIDs: nil means "no filtering" (see the sentinel above);
// a non-nil slice, possibly empty, is the caller's collection-level set.
// - grantedItemIDs: the caller's item-level grants, empty for most callers.
//
// The hash is over a CANONICAL form — both lists sorted, and the two lists
// separated by a byte that cannot occur in an ID — so a set that has not
// changed cannot produce a different epoch just because the store returned its
// rows in a different order. An epoch that flapped on ordering alone would
// trigger a full resync per poll, which is the one failure mode that would
// make this worse than the defect it closes.
//
// The value is derived only from the caller's OWN access and is opaque: it
// names no collection and no item, and two callers with different access
// cannot learn anything about each other from it.
func computeAccessEpoch(visibleCollectionIDs, grantedItemIDs []string) string {
if visibleCollectionIDs == nil {
// Grants only narrow within a filtered set; an unrestricted caller
// cannot also be item-filtered, and the doors never pass both.
return accessEpochUnrestricted
}
h := sha256.New()
writeSortedIDs(h, visibleCollectionIDs)
// Separator: a newline cannot appear in a UUID, so ["a","b"]+[] and
// ["a"]+["b"] cannot collide.
_, _ = h.Write([]byte("\n--\n"))
writeSortedIDs(h, grantedItemIDs)
return hex.EncodeToString(h.Sum(nil))[:16]
}
// writeSortedIDs feeds ids to h in sorted order, one per line. The input slice
// is COPIED before sorting: the callers pass slices they go on to use for the
// query itself, and reordering those under them would be an invisible
// side effect of computing a fingerprint.
func writeSortedIDs(h interface{ Write([]byte) (int, error) }, ids []string) {
sorted := make([]string, len(ids))
copy(sorted, ids)
sort.Strings(sorted)
for _, id := range sorted {
_, _ = h.Write([]byte(id))
_, _ = h.Write([]byte("\n"))
}
}