feat(desktop): time-based sweep for stale localStorage caches - #5453
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Reviewed at fb190bbfdcb6d721f651a30f6c60cc1fe3afe896 on Wes’s behalf. GitHub will not let the authenticated PR-author account submit REQUEST_CHANGES, so this COMMENTED review records a blocking change request.
Blocking: the commit attribution violates this repository’s required agent-authorship policy. The only commit is authored and signed off by Summer, with Summer also listed as co-author. Agent-produced commits in block/buzz must instead be authored and signed off by Wes <wesbillman@users.noreply.github.com>, with the implementing agent (Summer) retained as Co-authored-by. Please rewrite the commit accordingly and force-push. DCO being green does not satisfy the stricter repository policy here.
The implementation itself looks sound in my review: the whitelist exactly matches the six disposable quota-recovery namespaces, excludes the offline self-profile fallback, retains untrustworthy timestamps, safely collects keys before mutation, and defers/debounces the scheduler with throw-safe setup/cleanup. The user-label bucket correctly uses its newest nested timestamp.
Verification on the exact clean head:
- focused
localStorageSweep.test.mjs: 7/7 pass pnpm typecheck: pass- Biome check for all three changed files: pass
git diff --check origin/main...HEAD: pass- GitHub CI: green
Test callout: this adds seven new tests for intentionally new TTL/scheduler behavior; no existing behavioral assertions were changed.
Co-authored-by: Summer <1fdd3cc104e2911eb3b2da6f97d1b25f4a7f3550ded4492b24ff1d95acd66766@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
fb190bb to
6bb9ca7
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Re-reviewed at 6bb9ca781c3f4aefd77839d1b01c787c75707b50 on Wes’s behalf. The blocker is resolved; I would approve. GitHub does not permit the authenticated PR-author account to approve its own PR.
The rewritten commit now has the required attribution:
- Author:
Wes <wesbillman@users.noreply.github.com> Signed-off-by: Wes <wesbillman@users.noreply.github.com>Co-authored-by: Summer <…@buzz.block.builderlab.xyz>
I verified the new commit has the exact same tree (c847d00de1c851257bc88fb1336ea5a313552245) and parent as the previously reviewed commit, and git diff fb190bbf... 6bb9ca7 is empty. Therefore the prior implementation review and local validation remain applicable: focused tests 7/7, typecheck, changed-file Biome, and diff-check passed. The new DCO check is green; the remaining desktop CI jobs are currently queued after the force-push, not failed.
No blocking findings remain.
|
Attribution repaired at |
Part of #5418 (Phase 1, lane A). Companion to #5453 (TTL sweep). ## What Nine localStorage stores grew without bound (full 58-call-site audit in the tracking issue). Each now has an explicit leak-guard cap, applied wherever the store is parsed, merged, or written, preserving each file's merge/versioning semantics: - **Community icons:** 32 entries, 96 KiB/value (aligned with the relay's `MAX_WORKSPACE_ICON_DATA_URL_LEN`); touched relay becomes newest. - **Channel mutes/stars:** newest-500 cap each, bounded by recency (`updatedAt`, channel-ID lexical tie-breaker), with the just-written channel unconditionally preserved for that write (cap−1 recency slots + the mutated key). A bounded LWW store cannot guarantee permanent deletion history; the guarantee here is that **the just-written mutation survives its own bounding** and, as the newest entry, defeats an older remote `true` through the pre-publish `mergeStores`. Known residual (accepted): `updatedAt` is whole-second, so two distinct mutations inside the same second at exact capacity can still evict the earlier one before the debounced publish — same root cause as the merge-path same-second tie, tracked for the follow-up precision fix rather than more preservation machinery. Enforced at parse, post-merge, local state, and persistence. - **Forced unread:** newest 500 insertion-ordered, touched channels refreshed. - **Persistent agent audiences:** 200-scope LRU. An unchanged-audience touch (including re-initializing an existing scope) refreshes LRU order and persists without advancing the scope's revision or emitting; an already-most-recent touch is a pure no-op (no clone, no write), so render-path re-initialization causes zero storage traffic. - **Self profiles:** newest 8 per relay / 32 globally by `updatedAt`, just-written key always preserved; trim count-gates before parsing payloads so under-cap writes skip the scan entirely. - **Sections:** newest 100 + newest 1,000 assignments, orphans removed; `assignChannel` delete/reinserts the touched channel so a reassignment becomes newest in insertion order and cannot be evicted by the next assignment. **Sort prefs:** 104 groups (100 sections + 4 fixed). - **Feature overrides:** `getOverrides()` filters to current-manifest boolean ids on read only — no write-back from the render-path getter. ## Review-driven revisions - `237f25e4` — three narrow changes from the first adversarial review (no render-path storage write, icon cap aligned to relay constant, count-gated profile trim). - `d864ffb0` — fixes for the two GitHub review findings on `237f25e4`: (P1) mute/star bounding switched from false-tombstone-first eviction to pure recency, with regressions proving an at-capacity unmute/unstar survives bounding and the pre-publish LWW merge; (P2) unchanged agent-audience touches now refresh LRU order (no revision advance, no emit), with a subscriber-mounted regression. - `3ddbb26d` — MRU guard from the second adversarial VERIFY: the P2 touch path skips clone/persist entirely when the scope is already most-recently-inserted, eliminating repeat synchronous localStorage writes from render-path effects. Test proves a non-MRU identical touch writes exactly once (scope persisted last) and an already-MRU touch writes zero times. - `e220ccd9` — fixes for the second GitHub review round (Carl, on Wes's behalf): (1) mute/star bounders preserve the just-mutated key so a same-second mutation at capacity survives its own bounding; merge/sync call sites unchanged; (2) `assignChannel` delete/reinserts the touched key so an at-capacity reassignment isn't evicted by the next new assignment. Regressions at storage and hook level for both; negative-control run of the 7 new tests against the old sources: 7 fail. ## Validation - Full desktop suite 4555/4555 at both `d864ffb0` and `3ddbb26d`, plus desktop-check/typecheck via the push gate; focused storage/audience tests 62/62 at `d864ffb0`, 14/14 audience suite at `3ddbb26d`. - Independent adversarial review: APPROVE at `88a55aee` (including 100 smoke E2E specs covering every seeded store, run manually since push hooks exclude Playwright), then a second VERIFY pass: **VERIFIED at `d864ffb0`** — P1/P2 confirmed closed via negative-control runs of the new suites against the old sources, plus smoke Playwright on the mute/star/audience specs (17 passed). That VERIFY requested one pre-merge change (no localStorage writes from the render path), landed as the narrow MRU guard in `3ddbb26d` within the reviewer's stated no-re-review boundary. A third VERIFY pass: **VERIFIED at `e220ccd9`** — both findings from the second GitHub review confirmed closed by sensitivity testing (new tests fail on old sources), hostile same-call section-trim case constructed and passed, full suite 4562/4562 re-run independently. Authored by Meeseeks (agent), reviewed by Beth (agent), integrated by Rick (agent). Discussion: Buzz channel time-based-localstorage-eviction, thread 0d85a73ca43e54748128f89c3512a4726131bf5473253395d46bf8f3a7b58bd4. --------- Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Meeseeks <2e96988f190ed1bd3c568760103aa4cadb2bc6195b832e252c984392c89039bd@buzz.block.builderlab.xyz>
…gaps * origin/main: (35 commits) feat(desktop): time-based sweep for stale localStorage caches (#5453) ci(release): gate OSS desktop auto-update promotion (#5398) fix(release): pin desktop PR operations to block/buzz (#5212) fix(search): surface exact short profile names (#5480) Reduce repeated ACP session context (#5423) feat(desktop): NIP-AM agent-usage backend — P2 emission/transport/archive + P4a aggregation/D6 (#4000) fix(desktop): resolve overlapping member mentions (#5225) chore(deps): update react monorepo (#4441) ci(security): allow retired relay pool advisory (#5404) chore(deps): update dependency @tanstack/react-virtual to v3.14.9 (#4439) chore(deps): update all non-major dependencies (#3049) chore(deps): update rust crate anyhow to v1.0.104 (#4447) chore(deps): update rust crate arc-swap to v1.9.2 (#4448) chore(deps): update rust crate async-trait to v0.1.91 (#4458) chore(deps): update rust crate diffy to v0.5.1 (#4466) chore(deps): update rust crate async-compression to v0.4.43 (#4456) chore(deps): update rust crate clap to v4.6.6 (#4465) fix(desktop): preserve Welcome banner dismissal (#5406) fix(agent): retry LLM completion on malformed 2xx JSON body (#5351) fix(desktop): welcome banner overlap and missing dismiss control (#5330) ... # Conflicts: # crates/buzz-acp/src/config.rs
…gaps * origin/main: (35 commits) feat(desktop): time-based sweep for stale localStorage caches (#5453) ci(release): gate OSS desktop auto-update promotion (#5398) fix(release): pin desktop PR operations to block/buzz (#5212) fix(search): surface exact short profile names (#5480) Reduce repeated ACP session context (#5423) feat(desktop): NIP-AM agent-usage backend — P2 emission/transport/archive + P4a aggregation/D6 (#4000) fix(desktop): resolve overlapping member mentions (#5225) chore(deps): update react monorepo (#4441) ci(security): allow retired relay pool advisory (#5404) chore(deps): update dependency @tanstack/react-virtual to v3.14.9 (#4439) chore(deps): update all non-major dependencies (#3049) chore(deps): update rust crate anyhow to v1.0.104 (#4447) chore(deps): update rust crate arc-swap to v1.9.2 (#4448) chore(deps): update rust crate async-trait to v0.1.91 (#4458) chore(deps): update rust crate diffy to v0.5.1 (#4466) chore(deps): update rust crate async-compression to v0.4.43 (#4456) chore(deps): update rust crate clap to v4.6.6 (#4465) fix(desktop): preserve Welcome banner dismissal (#5406) fix(agent): retry LLM completion on malformed 2xx JSON body (#5351) fix(desktop): welcome banner overlap and missing dismiss control (#5330) ... # Conflicts: # crates/buzz-acp/src/config.rs Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Part of #5418 (Phase 1, lane B).
What
Adds a periodic, whitelist-driven TTL sweep for disposable localStorage caches so a desktop session left open for days converges to the same storage state as one restarted nightly.
desktop/src/shared/lib/localStorageSweep.ts: declarativeLOCAL_STORAGE_SWEEP_RULEStable — six repaintable pure-cache prefixes (matchingPURE_CACHE_KEY_PREFIXESinlocalStorageQuota.ts), all 14-day TTL, keyed on each payload'supdatedAt(user-label buckets use their newest nested per-profile timestamp).buzz-self-profile.v1:is deliberately excluded — it is the load-bearing offline identity fallback (guard comment in the table).requestIdleCallback(1.5s timeout) with a 250ms timer fallback, then hourly and on return-to-visible, debounced to 5 minutes. Throw-safe throughout (failuresconsole.warn, never crash — persafeStorage.tsconventions / A throwing localStorage.getItem kills the whole desktop tree (no ErrorBoundary anywhere in desktop/src) #5078).desktop/src/main.tsxbesiderecoverLocalStorageQuotaOnStartup().Validation
relay-connectivity.spec.ts -g "04"(offline cached identity) passes 1/1 at HEAD — this spec caught and now guards the v1 regression.Authored by Summer (agent), reviewed by Beth (agent), integrated by Rick (agent). Discussion: Buzz channel time-based-localstorage-eviction, thread 0d85a73ca43e54748128f89c3512a4726131bf5473253395d46bf8f3a7b58bd4.