Skip to content

fix(sw): precache the full module graph per generation (skew fix + offline)#309

Merged
Stvad merged 16 commits into
masterfrom
claude/module-version-mismatch-0in3jg
Jul 4, 2026
Merged

fix(sw): precache the full module graph per generation (skew fix + offline)#309
Stvad merged 16 commits into
masterfrom
claude/module-version-mismatch-0in3jg

Conversation

@Stvad

@Stvad Stvad commented Jul 3, 2026

Copy link
Copy Markdown
Owner

The bug

A user hit repeated "An extension failed to load: The requested module '../data/api/blockType.js' does not provide an export named 'INFRASTRUCTURE_TYPE_DISPLAY'" toasts right after the merge of #297 — different builds of the data layer mixed together in one page.

Root cause

The prod build ships every module at an unhashed, stable URL (preserveModules + [name].js) so extensions can import @/… through the page importmap. Cross-build consistency then rests entirely on public/sw.js pinning each deploy to its own km-assets-<id> cache and never grafting foreign-generation bytes (no clients.claim()).

That invariant only held for assets actually in the generation cache. install precached the first-paint HTML graph + Babel — not the lazy tail. src/extensions/api.js (the extension surface, imported only at runtime by user-extension blocks, so never in the bundler's static/first-paint graph) was never precached. So when an extension loaded after a deploy, assetCacheFirst missed the booting generation's cache and its network fallback served the newest generation's api.js — grafting it onto an old page where src/data/api/blockType.js was already resolved from the booting generation. #297 changed blockType.js's export shape (INFRASTRUCTURE_TYPE_DISPLAY) and added a re-export of it to the surface, turning that graft into a hard ES-module binding error. Three toasts = three enabled extension blocks all hitting the same import.

The fix

Precache the entire emitted asset graph into each generation's cache, so assetCacheFirst never has to fall through to the network for a same-origin module. A reachable-closure walk can't work here (user-extension imports aren't in the bundler's static graph), so we enumerate the full dist tree instead:

  • scripts/precache-assets.mjs (new, pure + DI-tested) — mirrors the SW's isCacheableAsset set: every same-origin runtime asset (js/mjs/css/wasm/fonts/images), excluding .map (dev-only, ~30 MB), sw.js, version.json, and the shell HTML/manifest.
  • scripts/inject-sw-build-id.mjs — walks all of dist/ and injects __PRECACHE_REST_ASSETS__ (full graph minus first-paint) instead of the babel-only closure.
  • public/sw.js — installs first-paint with {cache:'default'} and the rest with {cache:'reload'} (unhashed URLs carry per-deploy-varying bytes, so default could copy stale cross-generation bytes and reintroduce the skew), fanned out through a bounded pool.
  • Removes the now-subsumed precache-lazy-assets closure (Babel is covered by the full walk); updates the SW header + design-doc references.

Each generation's cache is now self-contained → no network graft → and, as the design already intended, the app is fully offline-capable (every lazily-imported module is in Cache Storage).

Verification

  • yarn run check green (lint + 4433 tests + sync-config/rpc-projections/no-service-role).
  • New unit tests for the enumeration helper.
  • Against a real production build: the injected first-paint ∪ rest = 1597 URLs = every cacheable file on disk (0 missing), no first-paint/rest overlap, sw.js parses. Critically, src/extensions/api.js — the module in the error — was previously unprecached and is now covered.

Relation to minification (#302)

#302 (turn on OXC minify: true) was closed on a P1 from its Codex review: with preserveModules, minification changes Vite's HTML emission — the unminified build emits one entry script + the full transitive modulepreload graph, but the minified build emits ~206 bare <script> tags and no modulepreloads. Since the SW precache was derived from that HTML graph (collectPrecacheAssets), minification made it omit boot-imported chunks (Codex's example: /src/main.js imports /src/App.js, but /src/App.js wasn't collected) → an offline reload then fails when assetCacheFirst misses an unlisted chunk. The Codex fix suggestion was "make the precache walker follow the emitted import graph."

This PR does exactly that (stronger: it precaches the whole emitted tree, so completeness no longer depends on the HTML graph's shape at all). Verified empirically with a minified build (minify: true, APP_BASE_PATH=/knowledge-medium/): first-paint drops to 206 as Codex described, but the full-dist rest walk brings the union to 1596 = every cacheable file on disk (0 missing), and /src/App.js is covered. So this directly resolves the P1 that blocked #302 — re-enabling minify: true is a sensible follow-up on top of this (its own within-build boot correctness was already verified in #302).

Note on bandwidth

The {cache:'reload'} rest set re-fetches even byte-unchanged assets on each deploy (unhashed URLs give the SW no way to prove a prior generation's entry is identical). The incremental over today is small — first-paint stays {cache:'default'}; only the previously-lazy tail moves to reload. A content-manifest that lets the SW copy unchanged bytes from a retained generation is a possible follow-up to reclaim it.

🤖 Generated with Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-04 18:35 UTC

claude added 4 commits July 3, 2026 23:54
The prod build ships every module at an unhashed, stable URL
(preserveModules + [name].js) so extensions can import `@/…` through the
importmap. Cross-build consistency then rests entirely on the service
worker pinning each deploy to its own `km-assets-<id>` cache and never
grafting foreign-generation bytes. But that only held for assets actually
in the generation cache: install precached first-paint + Babel, not the
lazy tail. `src/extensions/api.js` (the extension surface, imported only at
runtime by user-extension blocks) was never precached, so on a cache miss
after a deploy `assetCacheFirst` fell through to the network and served the
NEWEST generation's copy — grafting it onto an old page where
`src/data/api/blockType.js` was already resolved from the booting
generation. When the export shapes drifted (PR #297 added
`INFRASTRUCTURE_TYPE_DISPLAY` to blockType + a re-export in the surface),
the graft threw `does not provide an export named 'INFRASTRUCTURE_TYPE_DISPLAY'`.

Precache the whole emitted asset graph instead of a reachable closure
(user-extension imports aren't in the bundler's static graph, so a closure
can't cover them): walk dist for every same-origin cacheable asset, split
into first-paint ({cache:'default'}) and the rest ({cache:'reload'} so
unhashed per-deploy-varying bytes aren't copied stale). Each generation's
cache is now self-contained — no network graft — which also makes the app
fully offline. Verified against a real build: union = every cacheable file
on disk (1597/1597), and `src/extensions/api.js` is now covered.

Replaces the babel-only `precache-lazy-assets` closure (now subsumed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
Round 1 review of the precache change surfaced defects, fixed here:

- first-paint used {cache:'default'}, which copies a stale prior-generation
  entry from the browser HTTP cache (Pages serves the unhashed URLs
  max-age=600) into the new generation cache, persistently poisoning it —
  the same export-skew this precache exists to kill. Switch first-paint AND
  rest to {cache:'no-cache'}: it revalidates against the origin's content
  ETag (verified Pages honors If-None-Match → 304), so a stale entry 200s
  with current bytes and an unchanged asset 304s (cheap — also stops a
  redeploy re-downloading unchanged files).
- Storage yield: the OPFS SQLite DB / IndexedDB are sacred and persist() is
  granted (no auto-evict), so gate the heavy full-graph precache on
  storage.estimate() — skip it below a headroom floor so asset caches can't
  consume the room a DB write needs. Degrades to online-only there.
- recordGeneration ran at the START of install; an interrupted (now heavier)
  install left a phantom ledger id that shrank the GC retention window. Move
  it to AFTER the precache completes.
- activate GC ran writeLedger (a cache.put that throws under quota) BEFORE
  the space-freeing deletes, so quota could block GC. Delete first, then
  guard the ledger trim.
- Honesty: the network fallback still grafts a foreign generation on a
  genuine miss (incomplete precache), so correct the "self-heal"/"no graft"
  comments and flag a per-generation guard as the follow-up.
- Exclude only the ROOT sw.js (by path, not basename) so a nested dep sw.js
  stays precached; unwrap collectRestAssets' single-field return; add a
  drift-guard test that ASSET_EXTENSION matches between sw.js and the helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
Turns on rolldown-vite's oxc minifier (`true`, not `'esbuild'` which isn't
installed). ~7 MB / ~10% off each deploy's JS, faster client parse/exec.

Previously blocked (PR #302) because with preserveModules, minification
changes Vite's HTML emission — one entry + the full modulepreload graph
becomes ~200 bare <script> tags with no modulepreloads — and the SW precache
was derived from that HTML graph, so it dropped boot-imported chunks
(offline reload then 404'd on an unlisted import). The full-graph precache
(this branch) walks all of dist/ instead, so completeness no longer depends
on the HTML graph's shape. Verified against a minified build: precache union
= every cacheable file on disk (1596/1596, 0 missing), incl. the chunks the
HTML no longer preloads.

Sourcemaps stay ON — deliberately load-bearing: each carries sourcesContent
(the original TSX), which powers in-system plugin editing and the plugin
materialization / "eject" flow. Minify does not touch that (a minified
module's .js.map still contains the authored TSX).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
…fixes

Round-2 review + maintainer steer ("always have the latest cached for
offline"):

- Remove the storage.estimate() skip-guard. Proactively skipping the
  full-graph precache on low headroom disabled offline for the current
  generation (200 MB floor bit ordinary mobile) — the wrong trade. Always
  precache the whole graph so the latest build is fully offline-capable;
  bound storage by KEEP_GENERATIONS + the activate GC (and the preview
  sweep, next) instead of by dropping offline coverage. The caches are
  ~50 MB bounded and dwarfed by the OPFS DB, so this costs little.
- Move recordGeneration back to the START of install. Recording at the end
  (a round-1 fix for a phantom ledger id) traded a self-cleaning phantom for
  a PERMANENT orphan cache: an interrupted install left an untracked
  km-assets-<id> the scoped GC can never reach on the shared origin. A
  phantom just ages out of the keep-window and gets swept — the lesser evil.
  (A clean fix — provisional ledger entries — rides with the preview sweep.)
- Pool the first-paint fetches too (both lists through the bounded pool,
  first-paint first), so the minified build's ~200 <script> URLs don't fire
  as one unpooled burst whose swallowed failures become precache holes.
- Fix stale {cache:'default'}/{cache:'reload'} rationale comments across
  sw.js, inject-sw-build-id.mjs, and precache-assets.mjs — both lists now
  install with {cache:'no-cache'}; they stay separate for boot ORDERING,
  not cache mode.

Sourcemaps stay excluded (verified: nothing fetches .js.map at runtime —
extensions carry inline maps; DevTools fetches on demand online only), which
also avoids ~90 MB (30 MB × 3 gens) of never-read cache.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@Stvad Stvad force-pushed the claude/module-version-mismatch-0in3jg branch from 7b0a6b8 to aa81315 Compare July 3, 2026 23:55
claude added 5 commits July 4, 2026 00:27
…ble modules

The service worker was a hand-written classic worker in public/sw.js — the one
file with no type-checking (not a .ts, outside `yarn run check`), forced to
duplicate ASSET_EXTENSION into scripts/precache-assets.mjs behind a byte-compare
drift-guard, and untestable without eval'ing the whole worker against mocked
globals. As the SW's logic grows (offline precache, generation GC, the coming
preview-cache sweeper) that shape hurts more.

Move it to a proper TS build:
- src/sw/sw.ts — the worker entry, WebWorker-typed (tsconfig.sw.json).
- src/sw/{assets,preview,ledger}.ts — globals-free pure logic (asset/preview
  predicates, ledger retention math), unit-tested directly and shared with the
  build scripts. scripts/precache-assets.ts now imports the ONE ASSET_EXTENSION
  from src/sw/assets.ts, so the precached set can't drift from what's served —
  the drift-guard test is deleted (the import is the guarantee).
- vite.sw.config.ts builds src/sw/sw.ts into a single self-contained IIFE at
  dist/sw.js (classic worker — registration is not {type:'module'}); the build
  runs after the app build (emptyOutDir:false). scripts/inject-sw-build-id.ts
  (was .mjs, now vite-node so it can import the shared module) then stamps it.

Injection hardening: inject now replaces the whole `JSON.parse("__…__")` call
with a raw JS array literal instead of substituting a token inside a string.
The bundler re-emits the placeholder wrapped in double quotes, and the JSON
payload's own double quotes would break a double-quoted wrapper — a silent,
ships-a-dead-worker failure. It also asserts no placeholder survives and that
the stamped output parses as JS, so any future breakage fails the build.

Verified: full build stamps 206 first-paint + 1390 rest = 1596 assets == every
cacheable file on disk (0 missing), stamped sw.js parses, `yarn run check`
green (4444 tests). Behavior of the worker itself is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
The worker's orchestration (ledger I/O, install precache, activate GC, fetch
routing) was still untested — it reaches for `self`/`caches`/`fetch`, so it
could only be exercised by eval'ing the built worker against mocked globals.

Extract it into src/sw/worker.ts as `createServiceWorker(config, env)`,
parameterized by its globals (caches/fetch/origin) instead of `self`. src/sw/sw.ts
becomes a thin bootstrap that reads the build-injected config + real globals and
wires the factory to the SW events — behavior unchanged.

src/sw/worker.test.ts then drives it directly with an in-memory CacheStorage and
a stub fetch (22 tests):
- ledger I/O: round-trip, absent/corrupt/non-array tolerance, record de-dupe;
- install: records the generation, precaches shell (cache:reload) + both asset
  lists (cache:no-cache) into the right caches, swallows a per-URL failure,
  skips non-ok responses;
- activate GC: deletes only expired generations (keeps recent + vendor/meta),
  no-ops when the ledger fits, and reclaims space even when the ledger-trim
  write throws QuotaExceededError (the free-space-before-trim ordering);
- fetch routing: non-GET / same-origin non-asset / foreign-preview passthrough,
  asset cache hit (no network), shell-cache fallback for icons, asset miss →
  network → cache, Response.error() on miss+offline, esm.sh vendor cache-first;
- shellNetworkFirst: fresh HTML cached under the canonical key, offline → cached
  shell, offline + no cache → reject.

Also refresh the src/sw path in the design docs (plugin-module-url-sw-hybrid,
pr-preview-deployments, domain-onboarding) now that the SW moved out of public/.

`yarn run check` green (4466 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
github.io is one origin for production AND every PR preview, and Cache Storage
is per-origin — so a client accumulates the km-shell-*/km-assets-* caches of
every preview it ever opened. Production's activate GC only touches its OWN
generations (by ledger id), and a merged preview's SW never runs again to clean
up after itself, so those preview caches leak on the shared origin forever.

Add a cross-scope sweep, run from every SW's activate:
- The generation ledger gains a timestamp: {ids, updatedAt} instead of a bare
  array. writeLedger stamps Date.now() (injected); readLedger tolerates the
  legacy bare-array shape (updatedAt undefined). normalizeLedger centralizes it.
- computeReapableCaches (pure, in ledger.ts) reads every scope's ledger out of
  the shared meta cache and returns the caches + ledger entries to drop. Safety
  rails, all unit-tested:
    * preview-only — a scope is reapable only if PREVIEW_SUBTREE matches its
      ledger key path. Production is never a preview scope, so the sweep is
      STRUCTURALLY incapable of reaping prod caches.
    * stale-only — a numeric updatedAt older than 14 days. A legacy untimestamped
      ledger is never reaped (staleness unprovable).
    * shared-sha protection — never delete a generation any surviving scope still
      references (two deploys can share a build sha).
    * self-scope guard — the sweeping SW's own ledger is never reaped (defensive;
      it was just re-stamped on install anyway).
- The impure sweep in worker.ts enumerates the meta cache and applies the plan;
  it's wired into activate() after the own-generation GC, best-effort (a sweep
  failure never breaks activation).

Injected the clock (env.now) so the ledger timestamps + sweep are deterministic
in tests. New coverage: computeReapableCaches/normalizeLedger unit tests + an
activate-level integration suite (stale preview reaped, prod never reaped, fresh
+ legacy previews kept, shared-sha spared, self-scope protected) driven through
the in-memory CacheStorage mock.

Known limitation: previews last deployed BEFORE this ships have untimestamped
ledgers and so are never swept — a bounded, pre-existing set; all NEW previews
write timestamped ledgers and are swept 14 days after they go idle.

`yarn run check` green (4482 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
The provisional-ledger-entries refinement did NOT ship with the preview sweep
(the sweep reaps other scopes' preview caches, not this scope's own phantom
ids). Reword the comment so it doesn't imply otherwise — it's a possible
follow-up, and the phantom is still handled by keep-window aging.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
From a two-agent adversarial review of the sweeper + refactor:

- Sweep now ignores non-ledger km-meta entries. It treated EVERY km-meta entry
  as a scope ledger; km-meta is ledger-only today, but a future entry under a
  /pr-preview/ path with a coincidentally ledger-shaped stale body could have
  been misread as a reapable scope and deleted. Filter to keys ending in
  `/__km_generations__` (the ledger basename). TDD'd: a stray preview-path entry
  is left intact.

- Correct two overclaiming comments the review flagged (docs, no behavior
  change):
  * The own-scope activate GC's "ids don't collide across deploys (distinct
    build shas)" now states WHY that holds (a build id is the built commit's sha;
    a preview HEAD carries the PR's own commits, so no two live scopes build the
    same commit — the fast-forward-merge case tears the preview down) and notes
    that if id derivation ever allowed collisions this path would need the same
    cross-scope shared-id guard the sweep already applies.
  * The precache "can't drift from what's served" claim is now scoped to the
    EXTENSION axis. The SW also serves by request.destination, a superset that's
    empty only because every emitted dist asset carries a recognized extension.

- Document the sweep's snapshot/best-effort nature: a stale preview redeploying
  DURING another client's sweep can drop its just-written ledger entry — harmless
  and self-healing (caches keyed by the new id are untouched; re-recorded next
  deploy).

`yarn run check` green (4483 tests); full build still stamps 1596 precache
assets == on-disk set, stamped sw.js parses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@Stvad

Stvad commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@codex pls review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8afd73045

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/sw/ledger.ts
Codex review (P2): the preview sweep keys off the ledger's updatedAt, but only
install / activate (deploys) wrote it — ordinary fetches/navigations never
refreshed it. So a PR preview that stays open and actively USED for >14 days
without a redeploy looked abandoned and had its live km-shell-*/km-assets-*
caches reaped (breaking its offline reloads), even though a controlled tab was
using it.

Fix: a preview scope re-stamps its OWN ledger updatedAt on use — any fetch its
SW handles proves a controlled tab is alive — throttled to touchIntervalMs (1
day, well under the 14-day stale window) via an in-memory guard, so it's ~one
write per interval of activity, not one per request. Fire-and-forget so it never
delays the response. Production never touches (it's never reaped, and gating on
OWN_SCOPE_IS_PREVIEW keeps its ledger from churning on every request). Now
"stale" means "not USED in 14 days", not "not deployed in 14 days".

Tests: a live preview re-stamps past the interval; a within-interval fetch and a
production fetch both return synchronously and schedule no write.

yarn run check green (4486 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

The SW ships to every user, so minify its bundle like the app's (oxc:
4.68 kB, 2.07 kB gzip). The post-build stamp is now written to survive
minification: oxc rewrites string literals to backticks, so the injector
keys off each placeholder STRING LITERAL (whose contents a minifier can't
alter) rather than the enclosing JSON.parse(...) call, and swaps it for a
correctly-escaped JS string literal of the JSON payload — robust to both the
quote rewrite and any future identifier aliasing.

Extract the substitution into a pure, unit-tested stampSwSource() (mirrors
precache-assets.ts) and pin the minification robustness with a regression
test across all three quote styles + the $/quote/backslash escaping trap +
the failure guards. Verified end-to-end: the minified+stamped worker
executes in a stubbed worker scope, wires its listeners, and its runtime
JSON.parse decodes to the exact 1596-asset on-disk cacheable set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Minifying the SW buys almost nothing: the stamped sw.js is dominated by the
injected precache URL list (data, which the minifier never touches), so
minifying just the ~5 kB of code saves ~10 kB pre-gzip on a file fetched once
per deploy and cached — while costing us a readable worker in DevTools, which
matters for a correctness/security-critical worker (generation cache pinning +
GC, the "stuck load" failure class). App-bundle minification stays on; this
reverts only the SW build.

The injector hardening from the previous commit stays: stampSwSource keys off
each placeholder STRING LITERAL rather than the JSON.parse call, so the stamp
survives minification and flipping this back on later needs no stamp changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

DataLayerError set `this.name = new.target.name`, but production OXC
minification strips class names, so at runtime every data-layer error reported
a mangled identifier (e.g. "q") in logs, error boundaries, and telemetry —
right when a readable name matters most. Pin each error's `name` to a source
string literal instead (a localized block keyed by literal names the minifier
can't touch), and drop the base's new.target.name derivation.

No behavior change beyond the name: all consumers use `instanceof` (identity,
unaffected), verified by the existing suite. Adds a test asserting every
exported DataLayerError reports its export name, so a new subclass that forgets
its entry fails there instead of shipping a garbage name. Verified against the
actual minified build: 20/20 classes report their real name at runtime.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

…ction

Navigation was network-first — the one place the SW contradicted its own
generation-pinning. It served a new deploy's HTML while assets stayed cache-first
pinned to the OLD generation (new-HTML-over-old-assets skew on the first
controlled load after a deploy — the "stuck load"), and shellNetworkFirst even
wrote fresh HTML back into the old generation's shell cache, mutating a cache
meant to be immutable. No rationale for network-first was recorded — it predates
the SW's own origin commit and reads as the conventional SPA default.

Flip navigation to cache-first from this generation's own shell cache (install
already precaches ./index.html), seeding only on a cold miss and never
overwriting a present shell. Every controlled load is now fully
generation-consistent, and offline skips a doomed network round-trip. Trade-off:
a new deploy is applied on the next reload rather than the next navigation, which
matches the existing "open pages keep their generation until they reload" design
and the update-prompt flow.

Since new deploys no longer surface on navigation, add an explicit
app.checkForUpdates global action (command palette) that pokes the SW update
check on demand — the same check the 30-min poll runs — with toast feedback
(up-to-date / new version available / unavailable / error). A found update still
flows through the existing updatefound -> reload-prompt path. Bundled into the
app-update system extension alongside the reload action.

Tests: worker.test.ts navigation cases rewritten for cache-first (cache wins over
a live network; a cold miss seeds the shell); new registerServiceWorker.test
covers the four checkForAppUpdate outcomes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 45bc656356

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/sw/worker.ts Outdated
The touch-on-use write was launched detached (`void (async …)()`), so on a quick
cache-hit response the browser could terminate the worker before the async
cache.put landed — silently dropping the heartbeat that keeps a live preview from
being reaped (Codex P2). Thread the event's waitUntil from the fetch listener
(sw.ts) through handleFetch into maybeTouchOwnLedger, so the ledger write is tied
to the event's lifetime and the worker stays alive until it completes. The
detached path is kept only as a fallback for non-event callers (tests). New test
asserts the touch is handed to waitUntil (and a throttled fetch schedules
nothing).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2cc8714ca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/sw/worker.ts
React is externalized to esm.sh in prod, so its module URLs are absolute +
cross-origin and thus excluded from the dist/-walking same-origin precache.
That left an offline hole: a controlled first load with no network could not
resolve React through the import map. Inject the import map's integrity keys
(the exact known-SRI set the app imports) as a fourth precache list and seed
the shared vendor cache during install, so a controlled offline load resolves
React. Vendor URLs are immutable (version- + SRI-pinned), so install fetches
them with cache:'default'; the vendor cache stays shared + un-namespaced.

Verified end-to-end: the built worker precaches exactly the 12 import-map
integrity keys in dist/index.html (no drift).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SVSyvMRFfb4t6A3SAtqb3
@Stvad Stvad merged commit 0208582 into master Jul 4, 2026
2 checks passed
@Stvad Stvad deleted the claude/module-version-mismatch-0in3jg branch July 4, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants