Skip to content

fix(local-edge): find new workers on release checks and stop cache reads recreating caches - #28

Merged
zgq354-code merged 2 commits into
mainfrom
fix/local-edge-reset-and-worker-update
Oct 9, 2026
Merged

zgq354-code merged 2 commits into
mainfrom
fix/local-edge-reset-and-worker-update

Conversation

@zgq354-code

Copy link
Copy Markdown
Contributor

Outcome

Two Local Edge fixes from the FWA backlog. The first is the update lag the owner hit in production, where a worker change needed a manual reset to arrive. The second is a Cache Storage race that can resurrect a deleted release cache.

Behavior change

1. Worker updates arrive without a reset

  • Discovery. Per the Service Worker spec (Handle Fetch), browsers soft-update on every navigation, but on a subresource request only once the registration is more than 86400 s stale. An open SPA document, or a backgrounded iOS WebClip, does not navigate. So a new worker could stay undiscovered for up to a day.
    • Each release check from a controlled document now also calls registration.update(). That covers startup, visibility, online and the interval.
    • The call is best-effort: a failed script fetch does not fail the release check.
  • Activation. A found worker waits until every controlled document closes, and a reload alone cannot activate it, because the reloading document is still a client.
    • applyUpdate() now posts the existing __fwa:takeover message to a waiting worker with the matching script path. It waits until the worker is activated / redundant (3 s cap), then reloads.
    • The takeover path and persisted client pins are unchanged.
  • Kept. There is no forced swap without user action: an open document keeps its worker until the user applies an update or all documents close. Default skipWaiting was considered and not adopted.

2. Reads never recreate a release cache

  • readReleaseAsset / isReleaseComplete used caches.has followed by caches.open. A reset or cleanup deleting the cache between the two calls recreated an empty app-owned cache, which contradicts the offline-shell spec §3.2 (no resurrection).
  • These reads, and the final candidate verification, now use caches.match(path, { cacheName }), which resolves undefined and never creates a cache.
  • The original two-tab observation (DS P4c2, Chromium 3 of 4 runs) did not reproduce on current code. It failed to reproduce 14 times in this repo and 4 times in DS. This change closes the one code path that can produce that state; it is not a confirmed root-cause fix.

Non-goals

  • Self-healing on corrupted or missing cache entries.
  • The reset confirmation 403 on Firefox / WebKit.
  • The diagnostics panel position.
  • A new client state field for a worker-only update. A deploy that changes only the worker, not the release, still activates on the next full close.

Risk

  • applyUpdate() can now swap the controlling worker for other open tabs of the same scope, through clients.claim() in activate. This is the same path the incompatible-kernel takeover already uses. Pins live in IndexedDB metadata, so pinned tabs keep their release.
  • Public contract docs are updated: loader-contract.md (applyUpdate), config-contract.md (worker discovery) and architecture.md.

Verification

  • Unit regressions. Each fails on the previous implementation and passes now:
    • a read after a has → delete race;
    • worker update checked on each release check of a controlled document;
    • a waiting worker takes over before applyUpdate reloads.
  • e2e finds a new worker on a release check and applies it with the update. The test server switches the worker script mid-session; a generation probe tells worker 1 from worker 2.
    • With the loader changes removed, it fails at discovery: no waiting worker.
    • With only the takeover removed, it fails at the end (Expected "2", Received "1").
  • e2e a reset in another tab leaves no release cache behind. This is invariant coverage. It also passes on the previous code, because the race did not reproduce.
  • pnpm run ci passes: lint, typecheck, 215 unit tests, e2e, hosting, pack check and the packed consumer.
  • Consumer check. The packed tarball was installed into a ds-playground worktree (main@6954a53). Local Edge, release-update, crash-recovery and navigation-startup e2e ran on Chromium: 48 passed, 1 launch flake that passes in isolation.
  • Dev preview (heiyu-only) of that consumer build: https://ds-le-batch-devenv.gqlcc.heiyu.space. Production is untouched, and no npm release has been made.

Limits

  • Chromium Playwright only. Safari / iOS WebClip lifecycle is not verified.
  • No package release is cut by this PR; publishing stays tag-driven and owner-approved.

…ads recreating caches

Worker updates:
- An open document never navigates, so browsers checked for a new worker
  only after the registration was 24 hours stale. Each release check from a
  controlled document now also calls registration.update() (best-effort).
- A reload cannot activate a waiting worker. applyUpdate() now lets a
  waiting worker take over (bounded to 3 seconds) before reloading, so a
  user-applied update reaches the new worker as well as the new release.
- No change without user action: an open document keeps its worker.

Cache Storage:
- Release reads used caches.has then caches.open; a delete between the two
  recreated an empty app-owned cache. Reads and final candidate
  verification now use caches.match with a cacheName, which never creates.

Tests: unit regressions for both (each fails on the previous code), an e2e
that switches the worker script mid-session, and a two-tab reset e2e that
checks no release cache remains.
@zgq354-code
zgq354-code requested a review from a team as a code owner October 9, 2026 01:45
…ctivated

- Check the waiting worker's state once after attaching the listener, so an update applied in another tab does not wait out the 3 s cap.
- Cover the already-activated and other-script branches.
- Scope the no-recreation guarantee in architecture.md to kernel reads; the diagnostics panel still opens caches by name.
@zgq354-code

Copy link
Copy Markdown
Contributor Author

Independent review (Claude Opus 5.5, read-only ba claude-ask, plan mode)

  • f353e19 — VERDICT GO. No P0–P2. Five P3s:
    1. A user-applied takeover can make the new worker's activate cleanup delete another tab's in-flight candidate cache. That tab's install then fails and retries on the next check. The outcome is safe and bounded, but newly reachable. Recorded, not changed.
    2. The diagnostics panel still uses caches.keys() + caches.open(). Fixed in 7ec3df3: the architecture.md guarantee is scoped to kernel reads.
    3. activateWaitingWorker could miss a worker that another tab already activated and wait out the 3 s cap. Fixed in 7ec3df3: the state is checked once after attaching the listener, with a regression test that fails without the fix.
    4. If a waiting worker is incompatible, a background tab's non-startup snapshot read can stay in error until reload. Low likelihood (non-package or rolled-back worker). Recorded, not changed.
    5. Test gaps: the 3 s timeout branch is untested, and the two-tab reset e2e is invariant-only. Timeout branch recorded; a script-mismatch coverage test was added.
  • 7ec3df3 (increment) — VERDICT: GO.

Verification on 7ec3df3:

  • GitHub verify ×2 green. Local unit tests 70/70 for the loader.
  • One local pnpm run ci run had a single e2e failure: replaces a legacy worker without entering a reload loop stuck in registering (20 s timeout). It then passed 6/6 in isolation and in 2 full-suite reruns (41/41 each). The stuck step is waiting for the replacement worker to activate on a page that never reaches a release check, so the new registration.update() call does not run there. I treat it as a flake, but I have not compared against main, so a pre-existing flake is not excluded.

@zgq354 zgq354 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved on 7ec3df3: Opus independent review GO (two rounds), CI green, owner walkthrough of the dev preview passed.

@zgq354-code
zgq354-code merged commit dd37f39 into main Oct 9, 2026
2 checks passed
@zgq354-code
zgq354-code deleted the fix/local-edge-reset-and-worker-update branch October 9, 2026 02:38
@zgq354-code

Copy link
Copy Markdown
Contributor Author

Correction to the "did not reproduce" note above (2026-10-18). The empty release cache after reset does reproduce on beta.9, in base-app, at about 1 in 6–9 runs. The setup is a beta.5 install, a beta.9 deploy on the same origin, apply update, close and reopen, then reset right away.

Instrumentation of the beta.9 worker (open / match / delete logged with timestamps):

  • reset deletes fwa-local-edge:base-app:release:<active>, and caches.delete resolves true;
  • two lazy-chunk caches.match reads on that cache finish in the same millisecond, about 4 ms before the delete;
  • after the delete, the worker makes no further caches.open / match / delete call, yet caches.keys() lists the cache again with 0 entries.

So the has → open race fixed in this PR is real, but it is not what produces this residue. The recreation happens outside Local Edge's Cache Storage calls. A Chromium-side effect of responses still streaming from the deleted cache is a hypothesis only, not verified. Impact is unchanged: release metadata is deleted, so the empty cache is never used as a release. Tracked in the Base FWA backlog with the reproduction steps.

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