From 5d1e5ec791fc76f85c1893639eefe978461d7f14 Mon Sep 17 00:00:00 2001
From: Jonathan Sykes <telemagnadon@gmail.com>
Date: Thu, 8 Oct 2026 00:03:14 +0800
Subject: [PATCH] Verify explicit activation and legacy incremental migration

---
 frontend/asset-worker.test.js |  2 +-
 frontend/sw-update.js         |  8 +++-
 frontend/sw-update.test.js    |  6 +++
 frontend/sw.js                | 10 ++---
 perf/baseline.mjs             |  5 +++
 perf/migration.mjs            | 71 ++++++++++++++++++++++++++++++++---
 perf/proxy.mjs                | 10 ++++-
 7 files changed, 96 insertions(+), 16 deletions(-)

diff --git a/frontend/asset-worker.test.js b/frontend/asset-worker.test.js
index 667c887..193a31a 100644
--- a/frontend/asset-worker.test.js
+++ b/frontend/asset-worker.test.js
@@ -9,4 +9,4 @@ function environment(){
  return {storage,m,skips,dispatch,request};
 }
 test('install does not activate or publish; CACHE_STATUS is honest; activation commits and exact requests self-heal',async()=>{const e=environment();await e.dispatch('install');assert.equal(e.skips.length,0);const cache=await e.storage.open(core.CACHE);assert.equal(await core.state(cache),null);let reply;await e.dispatch('message',{data:{type:'CACHE_STATUS'},ports:[{postMessage:r=>reply=r}]});assert.equal(reply.ready,true);await e.dispatch('activate');assert.equal((await core.state(cache)).current.buildTag,'next');assert.equal((await e.request('/playlist/x','navigate')).headers.get('X-Asset-Hash'),'index');await cache.delete('/app.js?v=app');await e.dispatch('message',{data:{type:'CACHE_STATUS'},ports:[{postMessage:r=>reply=r}]});assert.equal(reply.ready,false);assert.equal(reply.missing,1);assert.equal(reply.version,'next');assert.equal((await e.request('/app.js')).headers.get('X-Asset-Hash'),'app');assert.ok(await cache.match('/app.js?v=app'));assert.equal(await cache.match('/app.js?v=stale'),undefined);assert.equal((await e.request('/app.js?v=stale')).status,409);});
-test('reported playback defers explicit activation until pause; install never calls skipWaiting',async()=>{const e=environment(),source={id:'p'};await e.dispatch('message',{source,data:{type:'PLAYING',value:true}});await e.dispatch('message',{source,data:{type:'SKIP_WAITING'}});assert.equal(e.skips.length,0);await e.dispatch('message',{source,data:{type:'PLAYING',value:false}});assert.equal(e.skips.length,1);});
+test('reported playback defers explicit activation until pause; install never calls skipWaiting',async()=>{const e=environment(),source={id:'p'};await e.dispatch('message',{data:{type:'SKIP_WAITING'}});assert.equal(e.skips.length,0);await e.dispatch('message',{source,data:{type:'PLAYING',value:true}});await e.dispatch('message',{source,data:{type:'SKIP_WAITING'}});assert.equal(e.skips.length,0);await e.dispatch('message',{source,data:{type:'PLAYING',value:false}});assert.equal(e.skips.length,1);});
diff --git a/frontend/sw-update.js b/frontend/sw-update.js
index e4b195e..d40de07 100644
--- a/frontend/sw-update.js
+++ b/frontend/sw-update.js
@@ -1,7 +1,7 @@
 /* ============================================================================
  * sw-update — makes "Refresh UI" actually land the new build.
  *
- * Framework-free and dependency-free on purpose (same pattern as
+ * Framework-free, with a shared sync core and a legacy fallback (same pattern as
  * async-guard.js):
  *   • Loads as a plain <script> under CSP `script-src 'self'` (browser
  *     global `window.SwUpdate`).
@@ -17,7 +17,7 @@
  * already current (a failed install leaves a partial cache that the
  * activate handler then mistook for a previous deploy).
  *
- * The fix no longer depends on the service-worker install lifecycle:
+ * Legacy fallback (retained for ASSET_SYNC=0):
  *   1. refreshShellInPlace() downloads a fresh copy of every file the shell
  *      caches hold (cache-busted, so even an old worker's cache-first
  *      handler can't answer with the stale copy) — ALL of them or nothing —
@@ -25,6 +25,10 @@
  *      serves the next load, it serves the new build.
  *   2. Only then is a waiting worker (if any) activated, and the page
  *      reloaded once.
+ * Incremental mode uses AssetSyncCore to fetch only missing verified hash URLs
+ * and publishes a complete blocking set. Playback guards cover activation and
+ * reload callbacks; PLAYING reports provide an additional worker guard.
+ *
  * The banner itself only opens when the build the page is running differs
  * from the server's (see app.js maybeShowUpdateBanner), so no lifecycle
  * event can re-open it once the page is current.
diff --git a/frontend/sw-update.test.js b/frontend/sw-update.test.js
index 53e336f..7bb6f6e 100644
--- a/frontend/sw-update.test.js
+++ b/frontend/sw-update.test.js
@@ -356,3 +356,9 @@ test('playback guard blocks activation and reload unless explicit override is ac
  assert.equal(messages.length,0);assert.equal(reloads,0);
  let playing=false;const container=fakeContainer();await applyUpdate({reg,container,reload:()=>reloads++,isPlaying:()=>playing,askStatus:async()=>({ready:true}),setTimeout:()=>{}});playing=true;container.fireControllerChange();assert.equal(reloads,0);
 });
+test('explicit playback override clears the worker guard and reloads once',async()=>{
+ const messages=[];let reloads=0;const container=fakeContainer();
+ await applyUpdate({reg:{waiting:{postMessage:m=>messages.push(m)}},container,reload:()=>reloads++,isPlaying:()=>true,confirmOverride:()=>true,askStatus:async()=>({ready:true}),setTimeout:()=>{}});
+ assert.deepStrictEqual(messages,[{type:'PLAYING',value:false},{type:'SKIP_WAITING'}]);
+ container.fireControllerChange();container.fireControllerChange();assert.equal(reloads,1);
+});
diff --git a/frontend/sw.js b/frontend/sw.js
index 43e7d6b..266ef71 100644
--- a/frontend/sw.js
+++ b/frontend/sw.js
@@ -2,7 +2,7 @@
  * sw.js — YT Player Service Worker
  *
  * Strategy:
- *   App shell (HTML/CSS/JS)    → cache-first, versioned cache
+ *   App shell (HTML/CSS/JS)    → cache-first, exact per-file hash keys
  *   /api/* requests            → network-only (never cache yt-dlp results)
  *   /api/download/*            → network-only (streamed binary, never cache)
  *   YouTube thumbnails (i.ytimg.com) → stale-while-revalidate
@@ -14,9 +14,9 @@
  *   2. The client (app.js) shows the "Update available" banner only when the
  *      build it is running differs from the server's /api/version buildTag —
  *      a waiting worker or a broadcast is merely a prompt to re-check.
- *   3. "Refresh UI" (sw-update.js) re-downloads every shell file into every
- *      versioned shell cache (all-or-nothing), activates a waiting worker if
- *      there is one (SKIP_WAITING), and reloads once.
+ *   3. Install syncs missing assets into ytplayer-assets. "Refresh UI" activates
+ *      the verified waiting worker (SKIP_WAITING) and reloads once. The legacy
+ *      full-shell path remains available through ASSET_SYNC=0.
  *   4. activate: if an older *versioned shell cache* is found (i.e. this
  *      activation is genuinely replacing a previous deploy, not just the
  *      first-ever install of a freshly (re)registered worker), broadcast
@@ -292,7 +292,7 @@ self.addEventListener('message', (e) => {
     playingClients.set(e.source.id, !!e.data.value);
     if (!e.data.value && deferredClients.delete(e.source.id)) self.skipWaiting();
   }
-  if (e.data && e.data.type === 'SKIP_WAITING') {
+  if (e.data && e.data.type === 'SKIP_WAITING' && e.source && e.source.id) {
     const id = e.source && e.source.id;
     if (playingClients.get(id)) deferredClients.add(id);
     else self.skipWaiting();
diff --git a/perf/baseline.mjs b/perf/baseline.mjs
index 7a08081..b4c893c 100755
--- a/perf/baseline.mjs
+++ b/perf/baseline.mjs
@@ -392,6 +392,7 @@ async function runColdScenario({ browserType, server, proxy, profile }) {
     domContentLoaded: pageMetrics.domContentLoaded,
     bootDone: pageMetrics.bootDone,
     longTaskTotal: pageMetrics.longTaskTotal,
+    requests: stats.requests,
     swInstallDuration,
     swInstallBytes: stats.totalWireBytes,
     storageEstimate: storage,
@@ -630,6 +631,7 @@ async function runUpdateScenario({ browserType, server, proxy, profile, updateKi
 
   return {
     updateKind,
+    requests: stats.requests,
     duration,
     requestCount: stats.requestCount,
     wireBytes: stats.totalWireBytes,
@@ -726,6 +728,7 @@ async function main() {
             mediaReady: summarizeList(coldRuns.map((r) => r.mediaReady)),
             autoplayBlocked: coldRuns.some((r) => r.autoplayBlocked),
             wireBytesByType: coldRuns[0]?.wireBytesByType || {},
+            requests: coldRuns[0]?.requests || [],
           };
         }
 
@@ -796,6 +799,7 @@ async function main() {
             wireBytes: summarizeList(updJsRuns.map((r) => r.wireBytes)),
             duration: summarizeList(updJsRuns.map((r) => r.duration)),
             wireBytesByType: updJsRuns[0]?.wireBytesByType || {},
+            requests: updJsRuns[0]?.requests || [],
           };
         }
 
@@ -822,6 +826,7 @@ async function main() {
             wireBytes: summarizeList(updCssRuns.map((r) => r.wireBytes)),
             duration: summarizeList(updCssRuns.map((r) => r.duration)),
             wireBytesByType: updCssRuns[0]?.wireBytesByType || {},
+            requests: updCssRuns[0]?.requests || [],
           };
         }
 
diff --git a/perf/migration.mjs b/perf/migration.mjs
index bec737e..1a455b0 100644
--- a/perf/migration.mjs
+++ b/perf/migration.mjs
@@ -12,6 +12,7 @@ import { chromium, webkit } from 'playwright';
 const repo = resolve(fileURLToPath(new URL('..', import.meta.url)));
 const commit = 'b77938a', oldTag = '1175f1a1d2c1';
 const args = process.argv.slice(2);
+const profile = args.includes('--profile') ? args[args.indexOf('--profile')+1] : 'lossy';
 const browserName = args.includes('--browser') ? args[args.indexOf('--browser') + 1] : 'all';
 assert.ok(['all', 'chromium', 'webkit'].includes(browserName));
 execFileSync('bash', [join(repo, 'perf/make-shell-fixture.sh'), commit, oldTag], { cwd: repo, stdio: 'inherit' });
@@ -25,6 +26,14 @@ symlinkSync(join(repo, 'server/node_modules'), join(scratch, 'server/node_module
 symlinkSync(join(repo, `perf/fixtures/shell-${oldTag}`), join(scratch, 'server/public'));
 const shell = [...readFileSync(join(repo, 'frontend/sw.js'), 'utf8').match(/const SHELL = \[([\s\S]*?)\n\];/)[1].matchAll(/['"](\/[^'"]+)['"]/g)].map(m => m[1]);
 const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));
+async function waitAsync(page,predicate,arg,timeout=90000) {
+  const deadline=Date.now()+timeout;
+  while(Date.now()<deadline) {
+    if(await page.evaluate(predicate,arg)) return;
+    await sleep(100);
+  }
+  throw new Error('Timed out waiting for asynchronous migration condition');
+}
 async function freePort() {
   const server = createServer();
   await new Promise(resolve => server.listen(0, '127.0.0.1', resolve));
@@ -64,7 +73,7 @@ async function run(name) {
   try {
     await start(true);
     assert.equal((await (await fetch(`http://127.0.0.1:${port}/api/version`)).json()).buildTag, oldTag);
-    proxy = createThrottleProxy({ targetPort: port, profile: 'lossy' });
+    proxy = createThrottleProxy({ targetPort: port, profile });
     const proxyPort = await proxy.listen();
     const origin = `http://127.0.0.1:${proxyPort}`;
     browser = await ({ chromium, webkit }[name]).launch({ headless: true });
@@ -114,10 +123,27 @@ async function run(name) {
       await checkBuildTag();
       await (await navigator.serviceWorker.getRegistration()).update();
     });
-    await page.waitForFunction(async () => !!(await navigator.serviceWorker.getRegistration())?.waiting, { timeout: 90000 });
+    await waitAsync(page,async () => !!(await navigator.serviceWorker.getRegistration())?.waiting);
     await page.waitForFunction(() => /Update available/.test(document.querySelector('#modalTitle')?.textContent || '') && !document.querySelector('#modal').classList.contains('hidden'));
     assert.equal(banners, 1, 'exactly one offer before refresh');
     const before = navigations;
+    await sleep(4500);
+    assert.equal(navigations,before,'legacy installation does not reload without Refresh UI');
+    // A legacy startup update can still be in flight through the stall proxy.
+    // Require the target candidate, not a transient earlier waiting worker.
+    await waitAsync(page,async tag => {
+      const reg=await navigator.serviceWorker.getRegistration();
+      const cache=await caches.open('ytplayer-assets');
+      return !reg.installing && reg.waiting?.state==='installed' &&
+        !!await cache.match('/__ytp_candidate/'+tag);
+    }, manifest.buildTag);
+    const status = await page.evaluate(async()=>SwUpdate.askCacheStatus((await navigator.serviceWorker.getRegistration()).waiting,{timeoutMs:4000}));
+    assert.equal(status?.ready,true,'new worker confirms persistent cache to old helper');
+    assert.equal(status.version,manifest.buildTag,'status belongs to the target build');
+    const oldTab = await context.newPage();
+    await oldTab.goto(origin,{waitUntil:'load'});
+    assert.equal(await oldTab.locator('meta[name="ytp-build"]').getAttribute('content'),oldTag);
+    const oldBytes = await oldTab.evaluate(async()=>await (await fetch('/app.js')).text());
     await page.locator('#modal').getByRole('button', { name: 'Refresh UI', exact: true }).click();
     await page.waitForFunction(tag => document.querySelector('meta[name="ytp-build"]')?.content === tag && typeof checkBuildTag === 'function', manifest.buildTag, { timeout: 90000 });
     await page.waitForFunction(() => !sessionStorage.getItem('ytpUpdateAttempt'));
@@ -125,11 +151,14 @@ async function run(name) {
     await sleep(2000);
     assert.equal(navigations - before, 1, 'one reload, no update loop');
     assert.equal(banners, 1, 'no banner after applying the current build');
+    assert.equal(await oldTab.evaluate(async()=>await (await fetch('/app.js')).text()),oldBytes,'open legacy tab keeps N-1 plain assets');
+    await oldTab.close();
     const cacheResult = await page.evaluate(async ({ tag, shell, files }) => {
-      const cache = await caches.open(`ytplayer-${tag}`);
+      const cache = await caches.open('ytplayer-assets');
       const missing = [], incorrect = [];
       for (const path of shell) {
-        const response = await cache.match(path);
+        const p = path === '/' ? '/index.html' : path;
+        const response = await cache.match(p+'?v='+files[p].h);
         if (!response) { missing.push(path); continue; }
         if (response.headers.get('x-asset-hash') !== files[path === '/' ? '/index.html' : path]?.h) incorrect.push(path);
       }
@@ -139,8 +168,38 @@ async function run(name) {
     assert.deepEqual(cacheResult.incorrect, [], 'every cached shell asset belongs to the new build');
     proxy.stopTracking();
     const stats = proxy.getStats();
+    assert.equal(stats.requests.filter(r=>r.url.includes('__ytpfresh')).length,0,'legacy helper skips its full download after ready:true');
     const durationMs = Date.now() - started;
     assert.deepEqual(errors, [], 'no script failures during installation and update');
+    // Exercise the new helper's actual lexical Player guard, without starting
+    // media or changing the production update/outcome code.
+    let guardText='';
+    page.once('dialog',async dialog=>{guardText=dialog.message();await dialog.dismiss();});
+    const guard=await page.evaluate(async()=>{
+      const saved=Player._wantsPlaying;Player._wantsPlaying=true;let reloads=0;
+      try { await SwUpdate.applyUpdate({reg:null,container:navigator.serviceWorker,reload:()=>reloads++});return {reloads,blocked:false}; }
+      catch(e){return {reloads,blocked:/Playing/.test(e.message)};}
+      finally{Player._wantsPlaying=saved;}
+    });
+    assert.match(guardText,/Playing - update after this song \/ when paused/);
+    assert.deepEqual(guard,{reloads:0,blocked:true});
+    const refresh=await page.evaluate(async files=>{
+      const cache=await caches.open('ytplayer-assets');
+      const key='/async-guard.js?v='+files['/async-guard.js'].h;
+      await cache.delete(key);
+      const requests=[];
+      const result=await SwUpdate.refreshShellInPlace({cachesApi:caches,
+        fetchFn:(url,init)=>{requests.push(url);return fetch(url,init)},isShellCache:()=>true});
+      if(!await cache.match(key)) throw Error('Refresh did not repair missing asset');
+      // Independently test the worker's plain-URL eviction repair.
+      const other='/visible-timer.js?v='+files['/visible-timer.js'].h;
+      await cache.delete(other);
+      const r=await fetch('/visible-timer.js');
+      if(r.headers.get('X-Asset-Hash')!==files['/visible-timer.js'].h || !await cache.match(other)) throw Error('Evicted asset did not self-heal');
+      return {result,requests};
+    },manifest.files);
+    assert.equal(refresh.result.refreshed,1,'thin refresh downloads only its missing asset');
+    assert.deepEqual(refresh.requests,['/api/manifest','/async-guard.js?v='+manifest.files['/async-guard.js'].h]);
     disconnected = true;
     // Match baseline.mjs: Linux WebKit's setOffline breaks even SW navigation.
     // Cut the proxy AND stop Bun so offline boot cannot use network bytes.
@@ -160,7 +219,7 @@ async function run(name) {
       assert.ok(error.message === 'TypeError: Load failed' || /\/api\/(?:recommendations\?[^\s]*|version)\.$/.test(error.message), `unexpected offline error: ${error.message}`);
     }
     if (errors.length) assert.ok(failedRequests.some(url => /\/api\/(?:recommendations|version)/.test(url)), 'transport errors correspond to disconnected API requests');
-    const result = { browser: name, oldTag, newTag: manifest.buildTag, banners, updateReloads: 1, cachedFiles: cacheResult.count, durationMs, wireBytes: stats.totalWireBytes, offline: true, legacyURLs: 21, profile: 'lossy', offlineTransportErrors: errors.length };
+    const result = { browser: name, oldTag, newTag: manifest.buildTag, banners, updateReloads: 1, cachedFiles: cacheResult.count, durationMs, wireBytes: stats.totalWireBytes, offline: true, legacyURLs: 21, profile, userInitiatedOnly: true, playbackGuard: true, evictionRepair: true, incrementalRefresh: true, offlineTransportErrors: errors.length };
     console.log(JSON.stringify(result));
     return result;
   } finally {
@@ -172,5 +231,5 @@ async function run(name) {
 try {
   const results = [];
   for (const name of browserName === 'all' ? ['chromium', 'webkit'] : [browserName]) results.push(await run(name));
-  writeFileSync(join(repo, 'perf/results/phase1-migration-2026-10-07.json'), JSON.stringify({ commit, results }, null, 2) + '\n');
+  writeFileSync(join(repo, `perf/results/phase2-migration-${profile}-2026-10-07.json`), JSON.stringify({ commit, results }, null, 2) + '\n');
 } finally { rmSync(scratch, { recursive: true, force: true }); }
diff --git a/perf/proxy.mjs b/perf/proxy.mjs
index 42ec6b0..c185fe1 100644
--- a/perf/proxy.mjs
+++ b/perf/proxy.mjs
@@ -48,6 +48,7 @@ export function categorizeContentType(contentType, urlPath) {
 export function createThrottleProxy({ targetPort, profile = 'lte' }) {
   let activeTracking = false;
   let requestCount = 0;
+  let requests = [];
   let totalWireBytes = 0;
   let bytesByType = {
     html: 0,
@@ -110,8 +111,11 @@ export function createThrottleProxy({ targetPort, profile = 'lte' }) {
         const ct = upRes.headers['content-type'] || '';
         const cat = categorizeContentType(ct, pathname);
 
+        let recorded;
         if (activeTracking) {
           requestCount++;
+          recorded={url:clientReq.url,status:upRes.statusCode,bytes:0};
+          requests.push(recorded);
         }
 
         clientRes.writeHead(upRes.statusCode, upRes.headers);
@@ -120,6 +124,7 @@ export function createThrottleProxy({ targetPort, profile = 'lte' }) {
           upRes.on('data', (chunk) => {
             if (activeTracking) {
               totalWireBytes += chunk.length;
+              if(recorded) recorded.bytes += chunk.length;
               bytesByType[cat] = (bytesByType[cat] || 0) + chunk.length;
             }
             clientRes.write(chunk);
@@ -131,6 +136,7 @@ export function createThrottleProxy({ targetPort, profile = 'lte' }) {
           throttler.on('data', (chunk) => {
             if (activeTracking) {
               totalWireBytes += chunk.length;
+              if(recorded) recorded.bytes += chunk.length;
               bytesByType[cat] = (bytesByType[cat] || 0) + chunk.length;
             }
           });
@@ -176,7 +182,7 @@ export function createThrottleProxy({ targetPort, profile = 'lte' }) {
       activeTracking = false;
     },
     resetStats: () => {
-      requestCount = 0;
+      requestCount = 0; requests = [];
       totalWireBytes = 0;
       bytesByType = {
         html: 0,
@@ -193,7 +199,7 @@ export function createThrottleProxy({ targetPort, profile = 'lte' }) {
       isOfflineMode = !!val;
     },
     getStats: () => ({
-      requestCount,
+      requestCount, requests,
       totalWireBytes,
       bytesByType: { ...bytesByType },
     }),