feat: widget auto-updater + faster dashboard startup#12
Conversation
urbanlama
left a comment
There was a problem hiding this comment.
Thanks for the PR. I reviewed the diff locally and ran the relevant checks. I think this should not be merged yet because there are two correctness regressions in the caching changes.
Findings:
- Store read cache can hide events written by another process
appendEvents() writes store-v1.json from the current in-memory state.events. If another TokenBBQ process wrote events after this process loaded its state, this process can later append its own event and overwrite the cache with an incomplete event list. The cache metadata still matches the full file set, so a subsequent loadStore() can return stale/incomplete data.
I reproduced this locally with two loaded store states: append a, load another state and append b, then append c from the first stale state. A fresh loadStore() returned a,c, missing b.
Relevant area: src/store.ts around appendEvents() / writeStoreCache(...).
- Gemini and Pi dedupe moved from global to per-file
The loader cache refactor moved seen into each per-file parse callback for Gemini and Pi. Before this PR, dedupe was global across all files in one load. Now duplicated sessions/messages that appear in multiple files are emitted more than once.
I reproduced this with temporary test data for both loaders: two files containing the same logical Gemini/Pi event returned 2 events instead of 1.
Relevant areas: src/loaders/gemini.ts and src/loaders/pi.ts.
- Local
npm run widget:buildnow requires signing secrets
Because createUpdaterArtifacts is enabled, the full widget build reaches installer generation and then fails locally without TAURI_SIGNING_PRIVATE_KEY. That is probably fine for release CI, where the secret is configured, but it means the existing local build command is no longer generally usable unless documented or split into a signed release build path.
Verification I ran:
npm testpassed: 58 tests, 0 failuresnpm run lintpassednpm run --prefix widget lintpassednpm run buildpassednpm run --prefix widget buildpassedcargo checkpassed afternpm run build:sidecarnpm run widget:buildbuilt the app/installers but failed at updater signing becauseTAURI_SIGNING_PRIVATE_KEYwas not set
Recommendation: fix findings 1 and 2 before merging. Finding 3 can be handled by documenting the signing requirement or splitting local unsigned build vs release signed build.
|
@ Finding 1 — store read-cache race: confirmed, fixed. Finding 2 — gemini/pi per-file dedupe: confirmed, fixed. Finding 3 — local widget build needs signing key: confirmed, split. Verification: |
… build Finding 1: appendEvents() paired a fresh on-disk file snapshot with this process's stale in-memory event list, persisting a read-cache that looked complete (sameFileSet matched) but dropped events another process had appended. Stop refreshing the read-cache in appendEvents(); the append already invalidates the cache via the changed file mtime/size, so the next loadStore() does a correct full rescan and rewrites it. Finding 2: the loader-cache refactor moved gemini/pi dedupe into the per-file parse callback, so duplicate sessions/messages spanning files were emitted more than once. Mirror the claude.ts pattern: carry a dedupeKey out of the per-file parse and dedupe globally after the cache merge. Finding 3: split the widget build. widget:build now builds locally with updater artifacts disabled (widget/src-tauri/tauri.dev.conf.json), so no signing key is required. widget:build:release is the signed path (TAURI_SIGNING_PRIVATE_KEY), matching release CI tauri-action. Adds regression tests for findings 1 and 2 (store, gemini, pi). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second-opinion review (Codex) surfaced a gap in the Finding 1 fix: a store-v1.json written by the OLD buggy appendEvents() (a fresh file snapshot paired with an incomplete event list) is still accepted after upgrade whenever the file set is unchanged, so incomplete data can persist until the first new event is captured. Bumping the shared CURRENT_VERSION would have been wrong — it also versions the on-disk ndjson event lines, so older tokenbbq builds would skip v2 lines as "future". Introduce a separate STORE_CACHE_VERSION (2) used only by the store read-cache, and move the cache file to store-v2.json. Any pre-fix v1 cache is now ignored (different filename and version field) and rebuilt correctly on the next load. Regression test added: a poisoned cache at the old path and at the new path with the old version field is ignored; loadStore() returns the true on-disk union. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6fea793 to
fa539db
Compare
|
Follow-up: ran a second-opinion pass (Codex, read-only) over the fix commit. It surfaced one residual gap in the Finding 1 fix worth closing now: A Codex otherwise concurred: gemini fix matches pre-cache behavior; pi fix correctly restores the pre-loader-cache global dedupe (the timestamp+token-sum key without sessionId is a pre-existing over-dedupe risk, not introduced here — flagging it for a separate decision, out of scope for this PR); widget split is cross-platform sound and no platform-specific tauri configs exist to conflict with the Heads up: I rewrote this branch's two fix commits to repair malformed commit messages (a stray delimiter had leaked in), so the SHAs changed and this was a force-push. Content/trees are unchanged. Tests: 62/62, lint clean. |
Ready for re-review — all three findings resolvedConsolidated status (head: Finding 1 — store read-cache race ✅
Finding 2 — gemini/pi per-file dedupe ✅Both now follow the existing
Finding 3 — local widget build needs signing key ✅ (verified end-to-end)
Proven, not just plausible: ran the full Verification
Note: the two fix commits were rewritten once to repair malformed commit messages (force-push); trees/content unchanged. |
urbanlama
left a comment
There was a problem hiding this comment.
Re-reviewed the current head fa539db after the follow-up fixes.
The previous blocking findings look resolved:
appendEvents()no longer refreshes the store read-cache from a stale in-memory state, and the separateSTORE_CACHE_VERSION/store-v2.jsoninvalidates poisoned pre-fix read caches without changing the event-line schema.- Gemini and Pi now carry per-file cache records through the loader cache and dedupe globally after the cache merge, restoring the pre-cache behavior.
- Local widget builds are split from signed release builds via
widget:build+tauri.dev.conf.json, while release CI keeps the signed updater artifact path.
I also did not find any new blocking issues in the loader cache, store cache, or updater wiring.
Local verification on this head:
npm test-> 62/62 passnpm run lint-> cleannpm run --prefix widget lint-> cleannpm run build-> exit 0
Approving.
Summary
widget/src/update.tsupdate flow wired into the Tauri shell (updater plugin inCargo.toml/tauri.conf.json, capability + release-workflow artifact), with UI inindex.html/styles.css.src/loaders/cache.ts+store.tschanges) so loaders (claude, amp, codex, gemini, opencode, pi) reuse parsed results instead of re-reading everything on each launch.AGENTS.mdand minor.gitignorehousekeeping.Test Plan
npm run test— 58/58 pass (incl. newsrc/loaders/cache.test.ts)npm run lint— clean (tsc --noEmit)🤖 Generated with Claude Code