From 6bc4f242c79b3b380f962c3733dd112097cbd8b9 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Tue, 1 Sep 2026 17:09:25 -0400 Subject: [PATCH] Fix live cleanup races with framework HMR (#695) Guard delayed accept and discard DOM fallbacks when framework/HMR ownership is present, while preserving static-page cleanup. Add unit/source regressions for both paths and refresh stale Setup wording assertions from #689. AI-assisted: prepared with Codex under @pbakaus direction. --- skill/scripts/live-browser-dom.js | 21 ++ skill/scripts/live-browser-session.js | 29 ++- skill/scripts/live-browser.js | 315 +++++++++++++++++++++---- tests/live-browser-dom.test.mjs | 36 +++ tests/live-browser-regression.test.mjs | 2 +- tests/live-browser-session.test.mjs | 20 ++ tests/live-browser-source.test.mjs | 171 +++++++++++++- tests/live-e2e/agent.mjs | 15 +- tests/live-reference.test.mjs | 6 +- 9 files changed, 563 insertions(+), 52 deletions(-) diff --git a/skill/scripts/live-browser-dom.js b/skill/scripts/live-browser-dom.js index ad6a794b3..e85c95c43 100644 --- a/skill/scripts/live-browser-dom.js +++ b/skill/scripts/live-browser-dom.js @@ -64,6 +64,26 @@ }; } + function hasFrameworkHmrOwnership(el) { + for (let node = el; node; node = node.parentElement) { + let keys = []; + try { keys = Object.getOwnPropertyNames(node); } catch {} + if (keys.some((key) => ( + key.startsWith('__reactFiber$') + || key.startsWith('__reactProps$') + || key.startsWith('__reactContainer$') + || key === '_reactRootContainer' + || key === '__vueParentComponent' + || key === '__vue_app__' + || key === '__vnode' + || key === '__svelte_meta' + ))) { + return true; + } + } + return false; + } + function id8() { if (crypto?.randomUUID) return crypto.randomUUID().replace(/-/g, '').slice(0, 8); return (Math.random().toString(16).slice(2) + Date.now().toString(16)).slice(0, 8); @@ -128,6 +148,7 @@ desc, rectIsUsableAnchor, makeFrozenAnchor, + hasFrameworkHmrOwnership, id8, cssId, liveUiRoot, diff --git a/skill/scripts/live-browser-session.js b/skill/scripts/live-browser-session.js index 0e362d6be..1514bf472 100644 --- a/skill/scripts/live-browser-session.js +++ b/skill/scripts/live-browser-session.js @@ -71,17 +71,38 @@ return checkpointRevision; } + function readHandledIds() { + const raw = safeRead(handledKey); + if (!raw) return []; + try { + const parsed = JSON.parse(raw); + if (Array.isArray(parsed)) { + return parsed.filter(id => typeof id === 'string' && id); + } + if (typeof parsed === 'string' && parsed) return [parsed]; + } catch { /* legacy values were stored as a plain session id */ } + return [raw]; + } + function markHandled(id) { if (!id) return; - safeWrite(handledKey, id); + const ids = readHandledIds().filter(existing => existing !== id); + ids.push(id); + safeWrite(handledKey, JSON.stringify(ids.slice(-8))); } function isHandled(id) { - return !!id && safeRead(handledKey) === id; + return !!id && readHandledIds().includes(id); } - function clearHandled() { - safeRemove(handledKey); + function clearHandled(id) { + if (!id) { + safeRemove(handledKey); + return; + } + const remaining = readHandledIds().filter(existing => existing !== id); + if (remaining.length > 0) safeWrite(handledKey, JSON.stringify(remaining)); + else safeRemove(handledKey); } function writeScrollY(y) { diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index 2b38ec62e..1f373a894 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -121,6 +121,10 @@ let hoveredElement = null; let selectedElement = null; let currentSessionId = null; + // Advances when the user begins configuring a fresh edit, before that edit + // has a server session id. Deferred recovery captures this revision so an + // older accept/discard can never reload over a replacement configuration. + let liveInteractionRevision = 0; let expectedVariants = 0; let arrivedVariants = 0; let visibleVariant = 0; @@ -188,6 +192,9 @@ // when the real accept result arrives or a new session starts. let awaitingAcceptResult = null; let variantObserver = null; + const discardedFrameworkWrapperWatchers = new Map(); + const handledRuntimeWrapperWatchers = new Map(); + const handledRuntimeWrapperReloadSessions = new Set(); let variantSelectionInFlight = false; let variantSelectionPromise = null; let recoveringEmptyCycling = false; @@ -208,6 +215,7 @@ const SCROLL_ANCHOR_LOCK_ID = 'impeccable-scroll-anchor-lock'; const VARIANT_STATE_STYLE_ID = 'impeccable-variant-state'; const DISCARD_STATE_STYLE_ID = 'impeccable-discard-state'; + const HANDLED_WRAPPER_RELOAD_KEY = PREFIX + '-handled-wrapper-reload'; // Dedicated key for scroll position - SEPARATE from LS_KEY so that // saveSession's state updates don't clobber a carefully-captured scrollY. @@ -270,6 +278,7 @@ desc, rectIsUsableAnchor, makeFrozenAnchor, + hasFrameworkHmrOwnership, id8, cssId, liveUiRoot, @@ -2034,6 +2043,16 @@ syncSteerQueueHint(); } + function beginNewLiveConfiguration() { + liveInteractionRevision += 1; + setLiveState('CONFIGURING'); + } + + function deferredRecoverySuperseded(sessionId, recoveryRevision) { + return liveInteractionRevision !== recoveryRevision + || !!(currentSessionId && currentSessionId !== sessionId); + } + /** Element used to position the floating bar / shader during a session. */ function resolveBarAnchor() { if (svelteComponentSession?.sessionId === currentSessionId && (state === 'GENERATING' || state === 'CYCLING')) { @@ -5036,7 +5055,7 @@ && el.parentElement && document.body.contains(el) && !own(el) - && !el.closest?.('[data-impeccable-variants]'); + && !el.closest?.('[data-impeccable-variants],[data-impeccable-carbonize]'); } function elementMatchesOriginalMarkup(liveEl, origContent) { @@ -6608,19 +6627,78 @@ document.getElementById(VARIANT_STATE_STYLE_ID)?.remove(); } + function discardStateStyleId(sessionId) { + return DISCARD_STATE_STYLE_ID + '-' + sessionId; + } + function showOriginalDuringDiscard(sessionId) { if (!sessionId) return; - let styleEl = document.getElementById(DISCARD_STATE_STYLE_ID); + let styleEl = document.getElementById(discardStateStyleId(sessionId)); if (!styleEl) { styleEl = document.createElement('style'); - styleEl.id = DISCARD_STATE_STYLE_ID; + styleEl.id = discardStateStyleId(sessionId); (document.head || document.documentElement).appendChild(styleEl); } + styleEl.dataset.impeccableDiscardSession = sessionId; const wrapper = '[data-impeccable-variants="' + sessionId + '"]'; styleEl.textContent = wrapper + ' > [data-impeccable-variant]:not([data-impeccable-variant="original"]) { display:none !important; }\n' + wrapper + ' > [data-impeccable-variant="original"] { display:block !important; }'; } + function removeDiscardStateStylesheet(sessionId) { + if (!sessionId) return; + document.getElementById(discardStateStyleId(sessionId))?.remove(); + } + + function releaseDiscardedStaticWrapper(wrapper, sessionId) { + removeDiscardStateStylesheet(sessionId); + if (!wrapper) return; + const orig = wrapper.querySelector('[data-impeccable-variant="original"]'); + const content = orig?.firstElementChild; + if (content && wrapper.parentElement) { + wrapper.parentElement.replaceChild(content, wrapper); + return; + } + wrapper.remove(); + } + + function watchForDiscardedFrameworkWrapperRemoval(sessionId) { + if (!sessionId || !document.body) return; + if (discardedFrameworkWrapperWatchers.has(sessionId)) return; + const selector = '[data-impeccable-variants="' + sessionId + '"]'; + let observer = null; + let timer = null; + const stopWatching = function() { + observer?.disconnect(); + if (timer) clearTimeout(timer); + discardedFrameworkWrapperWatchers.delete(sessionId); + }; + const finishIfGone = function() { + if (document.querySelector(selector)) return false; + removeDiscardStateStylesheet(sessionId); + stopWatching(); + return true; + }; + if (finishIfGone()) return; + observer = new MutationObserver(finishIfGone); + observer.observe(document.body, { childList: true, subtree: true }); + const resolveStillMounted = function() { + if (finishIfGone()) return; + const replacementActive = !!currentSessionId + || (state !== 'IDLE' && state !== 'PICKING'); + if (replacementActive) { + timer = setTimeout(resolveStillMounted, 12000); + discardedFrameworkWrapperWatchers.get(sessionId).timer = timer; + return; + } + removeDiscardStateStylesheet(sessionId); + stopWatching(); + location.reload(); + }; + timer = setTimeout(resolveStillMounted, 12000); + discardedFrameworkWrapperWatchers.set(sessionId, { observer, timer }); + } + function resolveScrollLockAnchorTop() { const anchor = resolveBarAnchor(); if (!anchor?.isConnected) return null; @@ -7335,7 +7413,7 @@ hideInsertLine(); configureKind = 'insert'; selectedElement = placeholder; - setLiveState('CONFIGURING'); + beginNewLiveConfiguration(); hideHighlight(); clearAnnotations(); showAnnotOverlay(placeholder); @@ -7353,7 +7431,7 @@ e.preventDefault(); e.stopPropagation(); selectedElement = hoveredElement; - setLiveState('CONFIGURING'); + beginNewLiveConfiguration(); showHighlight(selectedElement); clearAnnotations(); showAnnotOverlay(selectedElement); @@ -7530,7 +7608,7 @@ } else if (e.key === 'Enter') { e.preventDefault(); selectedElement = hoveredElement; - setLiveState('CONFIGURING'); + beginNewLiveConfiguration(); showHighlight(selectedElement); clearAnnotations(); showAnnotOverlay(selectedElement); @@ -8535,6 +8613,7 @@ void main() { } function scheduleAcceptCleanup(accepted) { + const recoveryRevision = liveInteractionRevision; queueMicrotask(function() { if (pendingAcceptedSession?.id !== accepted?.id) return; // Svelte previews live in an adapter-owned mount rather than in source @@ -8551,8 +8630,10 @@ void main() { // races. Static servers still need a fallback, but it must not keep Live // in SAVING or block the user's next pick. if (!accepted?.isSvelteComponent) { + watchForHandledRuntimeWrapper(accepted?.id, recoveryRevision); setTimeout(function() { - if (!acceptedDomAlreadyClean(accepted)) ensureAcceptedDomClean(accepted); + if (deferredRecoverySuperseded(accepted?.id, recoveryRevision)) return; + if (!acceptedDomAlreadyClean(accepted)) ensureAcceptedDomClean(accepted, recoveryRevision); }, 1200); } } @@ -8585,13 +8666,27 @@ void main() { && matches.every((el) => !el.closest('[data-impeccable-variants],[data-impeccable-variant],[data-impeccable-carbonize]')); } - function ensureAcceptedDomClean(pending) { + function ensureAcceptedDomClean(pending, recoveryRevision) { + // Background cleanup for an accepted session must never mutate or reload + // a newer comparison the user has already started. + if (deferredRecoverySuperseded(pending?.id, recoveryRevision)) return; if (acceptedDomAlreadyClean(pending)) return; const sessionId = pending?.id; const variantId = pending?.variant; const wrappers = findAcceptedRuntimeWrappers(sessionId); + if (hasFrameworkHmrOwnership(wrappers[0] || pending?.parentElement)) { + // Vite can coalesce rapid scaffold/carbonize writes and leave the last + // framework-owned preview tree mounted even though source is clean. Give + // HMR another grace window, then reload from clean source rather than + // violating reconciler ownership with a manual DOM mutation. + setTimeout(function() { + if (deferredRecoverySuperseded(pending?.id, recoveryRevision)) return; + if (!acceptedDomAlreadyClean(pending)) location.reload(); + }, 2000); + return; + } if (wrappers.length === 0) { - restoreAcceptedDomFromSnapshot(pending); + restoreAcceptedDomFromSnapshot(pending, recoveryRevision); return; } for (const wrapper of wrappers) { @@ -8608,7 +8703,7 @@ void main() { } wrapper.remove(); } - if (!acceptedDomAlreadyClean(pending)) restoreAcceptedDomFromSnapshot(pending); + if (!acceptedDomAlreadyClean(pending)) restoreAcceptedDomFromSnapshot(pending, recoveryRevision); } function findAcceptedRuntimeWrappers(sessionId) { @@ -8619,17 +8714,17 @@ void main() { ])]; } - function restoreAcceptedDomFromSnapshot(pending) { + function restoreAcceptedDomFromSnapshot(pending, recoveryRevision) { if (acceptedDomAlreadyClean(pending)) return; if (!pending?.acceptedHtml) { - reloadAfterMissingAcceptedDom(pending); + reloadAfterMissingAcceptedDom(pending, recoveryRevision); return; } const parent = pending.parentElement?.isConnected ? pending.parentElement : (pending.parentSelector ? document.querySelector(pending.parentSelector) : null); if (!parent) { - reloadAfterMissingAcceptedDom(pending); + reloadAfterMissingAcceptedDom(pending, recoveryRevision); return; } const template = document.createElement('template'); @@ -8638,10 +8733,11 @@ void main() { ? pending.nextSibling : null; parent.insertBefore(template.content, anchor); - if (!acceptedDomAlreadyClean(pending)) reloadAfterMissingAcceptedDom(pending); + if (!acceptedDomAlreadyClean(pending)) reloadAfterMissingAcceptedDom(pending, recoveryRevision); } - function reloadAfterMissingAcceptedDom(pending) { + function reloadAfterMissingAcceptedDom(pending, recoveryRevision) { + if (deferredRecoverySuperseded(pending?.id, recoveryRevision)) return; if (acceptedDomAlreadyClean(pending)) return; if (pending?.id && document.querySelector('[data-impeccable-variants="' + pending.id + '"]')) return; location.reload(); @@ -8991,14 +9087,15 @@ void main() { return sessionState.isHandled(id); } - function clearHandled() { - sessionState.clearHandled(); + function clearHandled(sessionId) { + sessionState.clearHandled(sessionId); } function cleanup(options) { const restoreOriginal = options?.restoreOriginal === true; const instantChrome = options?.instantChrome === true; const cleanupSessionId = currentSessionId; + const cleanupRevision = liveInteractionRevision; clearMountErrorCard(); lastReportedMountFailure = null; if (svelteComponentSession?.sessionId === cleanupSessionId) { @@ -9016,19 +9113,41 @@ void main() { else wrapper.style.display = 'none'; } setTimeout(function() { - document.getElementById(DISCARD_STATE_STYLE_ID)?.remove(); - if (!cleanupSessionId) return; - const lateWrapper = document.querySelector('[data-impeccable-variants="' + cleanupSessionId + '"]'); - if (!lateWrapper) return; - const orig = lateWrapper.querySelector('[data-impeccable-variant="original"]'); - if (orig) { - const content = orig.firstElementChild; - if (content) { - lateWrapper.parentElement.replaceChild(content, lateWrapper); - return; - } + const recoverySuperseded = deferredRecoverySuperseded(cleanupSessionId, cleanupRevision); + if (!cleanupSessionId) { + removeDiscardStateStylesheet(); + return; } - lateWrapper.remove(); + const lateWrapper = document.querySelector('[data-impeccable-variants="' + cleanupSessionId + '"]'); + if (!lateWrapper) { + removeDiscardStateStylesheet(cleanupSessionId); + return; + } + if (recoverySuperseded) { + if (hasFrameworkHmrOwnership(lateWrapper)) { + watchForDiscardedFrameworkWrapperRemoval(cleanupSessionId); + } else { + releaseDiscardedStaticWrapper(lateWrapper, cleanupSessionId); + } + return; + } + if (hasFrameworkHmrOwnership(lateWrapper)) { + // As on accept, never restructure framework-owned DOM. If HMR missed + // the final source rewrite, reload once after a grace window so the + // discarded source becomes authoritative without a reconciler race. + setTimeout(function() { + const staleWrapper = document.querySelector('[data-impeccable-variants="' + cleanupSessionId + '"]'); + if (deferredRecoverySuperseded(cleanupSessionId, cleanupRevision)) { + if (!staleWrapper) removeDiscardStateStylesheet(cleanupSessionId); + else watchForDiscardedFrameworkWrapperRemoval(cleanupSessionId); + return; + } + removeDiscardStateStylesheet(cleanupSessionId); + if (staleWrapper) location.reload(); + }, 2000); + return; + } + releaseDiscardedStaticWrapper(lateWrapper, cleanupSessionId); }, 2000); } hideBar(instantChrome); @@ -9111,12 +9230,122 @@ void main() { // Resume an active variant session after HMR/page reload. // If a [data-impeccable-variants] wrapper exists in the DOM, the agent wrote // variants before HMR fired. Pick up where we left off. - function resumeSession() { + function handledWrapperReloadKey(sessionId) { + return HANDLED_WRAPPER_RELOAD_KEY + ':' + sessionId; + } + + function clearHandledWrapperReloadStamp(sessionId) { + try { + if (sessionId) { + sessionStorage.removeItem(handledWrapperReloadKey(sessionId)); + const legacy = sessionStorage.getItem(HANDLED_WRAPPER_RELOAD_KEY) || ''; + if (legacy === sessionId || legacy.startsWith(sessionId + ':')) { + sessionStorage.removeItem(HANDLED_WRAPPER_RELOAD_KEY); + } + return; + } + sessionStorage.removeItem(HANDLED_WRAPPER_RELOAD_KEY); + for (let i = sessionStorage.length - 1; i >= 0; i--) { + const key = sessionStorage.key(i); + if (key?.startsWith(HANDLED_WRAPPER_RELOAD_KEY + ':')) sessionStorage.removeItem(key); + } + } catch {} + } + + function scheduleHandledRuntimeWrapperReload(wrapper, recoveryRevision = liveInteractionRevision) { + const sessionId = wrapper?.dataset?.impeccableVariants + || wrapper?.dataset?.impeccableCarbonize; + if (!sessionId || !isSessionHandled(sessionId)) return false; + if (deferredRecoverySuperseded(sessionId, recoveryRevision)) return true; + + if (handledRuntimeWrapperReloadSessions.has(sessionId)) return true; + let reloadAttempts = 0; + try { + reloadAttempts = Number(sessionStorage.getItem(handledWrapperReloadKey(sessionId))) || 0; + if (reloadAttempts >= 2) return true; + sessionStorage.setItem(handledWrapperReloadKey(sessionId), String(reloadAttempts + 1)); + } catch {} + handledRuntimeWrapperReloadSessions.add(sessionId); + + // A framework refresh can replace the variants tree with an intermediate + // carbonize tree and reload the page, cancelling the original accept timer. + // Let the file-side cleanup settle, then reload once from authoritative + // source. The sessionStorage stamp prevents a stale dev-server response + // from turning this recovery into a reload loop. + setTimeout(function() { + if (deferredRecoverySuperseded(sessionId, recoveryRevision)) { + clearHandledWrapperReloadStamp(sessionId); + handledRuntimeWrapperReloadSessions.delete(sessionId); + return; + } + const staleWrapper = document.querySelector( + '[data-impeccable-variants="' + sessionId + '"],' + + '[data-impeccable-carbonize="' + sessionId + '"]', + ); + if (staleWrapper) location.reload(); + else { + clearHandledWrapperReloadStamp(sessionId); + handledRuntimeWrapperReloadSessions.delete(sessionId); + } + }, 3000); + return true; + } + + function watchForHandledRuntimeWrapper(sessionId, recoveryRevision = liveInteractionRevision) { + if (!sessionId || !document.body) return; + const existing = handledRuntimeWrapperWatchers.get(sessionId); + existing?.observer.disconnect(); + if (existing?.timer) clearTimeout(existing.timer); + + const findHandledWrapper = function() { + const wrapper = document.querySelector( + '[data-impeccable-variants="' + sessionId + '"],' + + '[data-impeccable-carbonize="' + sessionId + '"]', + ); + if (wrapper) scheduleHandledRuntimeWrapperReload(wrapper, recoveryRevision); + }; + + // Vite can briefly render the clean accepted tree, then apply a delayed + // carbonize refresh after the one-shot accept fallback has already passed. + // Keep a bounded scout alive through that refresh window so a late stale + // framework tree still reloads from the now-authoritative source. + const observer = new MutationObserver(findHandledWrapper); + observer.observe(document.body, { childList: true, subtree: true }); + const timer = setTimeout(function() { + if (handledRuntimeWrapperWatchers.get(sessionId)?.observer !== observer) return; + observer.disconnect(); + handledRuntimeWrapperWatchers.delete(sessionId); + }, 12000); + handledRuntimeWrapperWatchers.set(sessionId, { observer, timer }); + findHandledWrapper(); + } + + function restoreSessionSupersedingHandledWrapper(runtimeWrapper) { + const handledSessionId = runtimeWrapper?.dataset?.impeccableVariants + || runtimeWrapper?.dataset?.impeccableCarbonize; + if (!handledSessionId || !isSessionHandled(handledSessionId)) return false; + + // Accept releases the picker before carbonize finishes, so a replacement + // generation can already be durable while the prior handled wrapper is + // still mounted. Restore that newer session before the stale-wrapper + // recovery path gets a chance to reload or consume its retry budget. + const saved = loadSession(); + if (!saved?.id || saved.id === handledSessionId || isSessionHandled(saved.id)) return false; + if (currentSessionId === saved.id) return true; + return restoreSessionWithoutWrapper('browser_resumed_over_handled_wrapper'); + } + + function resumeSession(recoveryRevision = liveInteractionRevision) { const wrapper = document.querySelector('[data-impeccable-variants]'); + const runtimeWrapper = wrapper || document.querySelector('[data-impeccable-carbonize]'); + if (restoreSessionSupersedingHandledWrapper(runtimeWrapper)) return true; + if (scheduleHandledRuntimeWrapperReload(runtimeWrapper, recoveryRevision)) return false; if (!wrapper) { if (restoreSessionWithoutWrapper('browser_resumed_without_wrapper')) return true; clearSession(); - clearHandled(); + // Keep the bounded handled-id history durable. A framework can hydrate a + // completed wrapper well after initialization, and a later reload must + // still recognize that wrapper as recovery work rather than resume it. return false; } @@ -9136,7 +9365,7 @@ void main() { wrapper.remove(); if (restoreSessionWithoutWrapper('browser_resumed_svelte_orphan_wrapper')) return true; clearSession(); - clearHandled(); + clearHandled(sessionId); return false; } @@ -12520,22 +12749,28 @@ void main() { connectSSE(); // Check for an active session to resume (variant wrapper already in DOM after HMR) - if (!resumeSession()) { + const resumed = resumeSession(); + if (!resumed) { console.log('[impeccable] Live variant mode ready. Hover over elements to pick one.'); - // SvelteKit (and any framework that hydrates after HTML parse) may add - // the variant wrapper AFTER init runs. Watch for it and retry resume - // once it appears. Disconnect on first hit. + } else { + console.log('[impeccable] Resumed active variant session ' + currentSessionId + ' (' + arrivedVariants + '/' + expectedVariants + ' variants).'); + } + + // SvelteKit, React, and other frameworks may restore a durable session + // before hydration adds its variant wrapper. Keep a deferred-wrapper scout + // whenever init did not see a runtime wrapper, even if local/server state + // was already restored successfully. Disconnect on the first wrapper hit. + if (!resumed || !document.querySelector('[data-impeccable-variants],[data-impeccable-carbonize]')) { + const deferredResumeRevision = liveInteractionRevision; const scout = new MutationObserver(() => { - const wrapper = document.querySelector('[data-impeccable-variants]'); + const wrapper = document.querySelector('[data-impeccable-variants],[data-impeccable-carbonize]'); if (!wrapper) return; scout.disconnect(); - if (resumeSession()) { + if (resumeSession(deferredResumeRevision)) { console.log('[impeccable] Resumed deferred session ' + currentSessionId + ' (post-hydration).'); } }); scout.observe(document.body, { childList: true, subtree: true }); - } else { - console.log('[impeccable] Resumed active variant session ' + currentSessionId + ' (' + arrivedVariants + '/' + expectedVariants + ' variants).'); } if (state === 'IDLE' && (pickActive || insertActive)) setLiveState('PICKING'); diff --git a/tests/live-browser-dom.test.mjs b/tests/live-browser-dom.test.mjs index f537df2ab..597801108 100644 --- a/tests/live-browser-dom.test.mjs +++ b/tests/live-browser-dom.test.mjs @@ -53,6 +53,7 @@ function createDocument() { activeElement: null, elementsById, getElementById(id) { return elementsById.get(id) || null; }, + querySelectorAll() { return []; }, }; } @@ -155,4 +156,39 @@ describe('live-browser-dom helpers', () => { root.listeners.focusin({ stopPropagation: () => { stopped += 1; } }); assert.equal(stopped, 3); }); + + it('detects framework HMR ownership while leaving static DOM eligible for cleanup', () => { + const staticDoc = createDocument(); + const { context, createHelpers } = loadFactory(staticDoc); + const helpers = createHelpers({ prefix: 'impeccable-live', document: staticDoc }); + const staticWrapper = createElement(); + + assert.equal(helpers.hasFrameworkHmrOwnership(staticWrapper), false); + + staticDoc.querySelectorAll = () => [{ getAttribute: () => '/app/@vite/client' }]; + assert.equal(helpers.hasFrameworkHmrOwnership(staticWrapper), false); + + staticDoc.querySelectorAll = () => []; + context.$RefreshReg$ = () => {}; + assert.equal(helpers.hasFrameworkHmrOwnership(staticWrapper), false); + delete context.$RefreshReg$; + + context.__VUE_HMR_RUNTIME__ = {}; + assert.equal(helpers.hasFrameworkHmrOwnership(staticWrapper), false); + delete context.__VUE_HMR_RUNTIME__; + + const reactWrapper = createElement(); + reactWrapper.__reactFiber$impeccable = {}; + assert.equal(helpers.hasFrameworkHmrOwnership(reactWrapper), true); + + const vueParent = createElement(); + vueParent.__vueParentComponent = {}; + const nestedWrapper = createElement(); + nestedWrapper.parentElement = vueParent; + assert.equal(helpers.hasFrameworkHmrOwnership(nestedWrapper), true); + + const svelteWrapper = createElement(); + svelteWrapper.__svelte_meta = {}; + assert.equal(helpers.hasFrameworkHmrOwnership(svelteWrapper), true); + }); }); diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs index 7796404e3..ae583eb32 100644 --- a/tests/live-browser-regression.test.mjs +++ b/tests/live-browser-regression.test.mjs @@ -1057,7 +1057,7 @@ describe('live-browser.js regression guards', () => { it('promotes an early-accepted Svelte preview before releasing the picker', () => { assert.match( SOURCE, - /function scheduleAcceptCleanup\(accepted\) \{[\s\S]{0,420}?if \(accepted\?\.isSvelteComponent\) \{[\s\S]{0,120}?commitAcceptedSvelteComponentToDom\(accepted\.id\);[\s\S]{0,120}?cleanupAcceptedSession\(\);/, + /function scheduleAcceptCleanup\(accepted\) \{[\s\S]{0,650}?if \(accepted\?\.isSvelteComponent\) \{[\s\S]{0,120}?commitAcceptedSvelteComponentToDom\(accepted\.id\);[\s\S]{0,120}?cleanupAcceptedSession\(\);/, 'Svelte early accept must tear down its adapter mount before the next picking session starts', ); }); diff --git a/tests/live-browser-session.test.mjs b/tests/live-browser-session.test.mjs index 1b712de08..8a7c2e8d1 100644 --- a/tests/live-browser-session.test.mjs +++ b/tests/live-browser-session.test.mjs @@ -61,4 +61,24 @@ describe('live-browser-session state helper', () => { 'event=live_browser_session.revision_resume actor=browser operation=reload_checkpoint risk=durable_store_ignores_stale_checkpoint expected=3 actual=' + second.currentCheckpointRevision(), ); }); + + it('retains overlapping handled sessions and reads the legacy single-id format', () => { + const createState = loadFactory(); + const storage = createMemoryStorage(); + const first = createState({ prefix: 'impeccable-live', storage, idFactory: () => 'owner-a' }); + + first.markHandled('session-a'); + first.markHandled('session-b'); + assert.equal(first.isHandled('session-a'), true); + assert.equal(first.isHandled('session-b'), true); + + const second = createState({ prefix: 'impeccable-live', storage, idFactory: () => 'owner-b' }); + assert.equal(second.isHandled('session-a'), true, 'handled sessions survive reload-equivalent helpers'); + second.clearHandled('session-a'); + assert.equal(second.isHandled('session-a'), false); + assert.equal(second.isHandled('session-b'), true, 'clearing one recovery must preserve overlapping sessions'); + + storage.setItem(second.handledKey, 'legacy-session'); + assert.equal(second.isHandled('legacy-session'), true); + }); }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index af03f023d..235bd866d 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -353,12 +353,12 @@ describe('live-browser source contracts', () => { ); assert.match( SOURCE, - /function scheduleAcceptCleanup\(accepted\)[\s\S]*?queueMicrotask\(function\(\) \{[\s\S]*?cleanupAcceptedSession\(\);[\s\S]*?setTimeout\(function\(\) \{[\s\S]*?ensureAcceptedDomClean\(accepted\);[\s\S]*?\}, 1200\);/, + /function scheduleAcceptCleanup\(accepted\)[\s\S]*?queueMicrotask\(function\(\) \{[\s\S]*?cleanupAcceptedSession\(\);[\s\S]*?setTimeout\(function\(\) \{[\s\S]*?ensureAcceptedDomClean\(accepted, recoveryRevision\);[\s\S]*?\}, 1200\);/, 'foreground cleanup should be immediate while the no-HMR DOM fallback stays deferred', ); assert.match( SOURCE, - /function ensureAcceptedDomClean\(pending\)[\s\S]*?acceptedDomAlreadyClean\(pending\)[\s\S]*?findAcceptedRuntimeWrappers\(sessionId\)[\s\S]*?for \(const wrapper of wrappers\)[\s\S]*?parent\.insertBefore\(accepted\.firstChild, wrapper\);[\s\S]*?wrapper\.remove\(\);[\s\S]*?acceptedDomAlreadyClean\(pending\)/, + /function ensureAcceptedDomClean\(pending, recoveryRevision\)[\s\S]*?acceptedDomAlreadyClean\(pending\)[\s\S]*?findAcceptedRuntimeWrappers\(sessionId\)[\s\S]*?for \(const wrapper of wrappers\)[\s\S]*?parent\.insertBefore\(accepted\.firstChild, wrapper\);[\s\S]*?wrapper\.remove\(\);[\s\S]*?acceptedDomAlreadyClean\(pending\)/, 'post-cleanup fallback should unwrap the accepted variant instead of preserving live runtime wrappers', ); assert.match( @@ -383,9 +383,174 @@ describe('live-browser source contracts', () => { ); assert.match( SOURCE, - /function reloadAfterMissingAcceptedDom\(pending\)[\s\S]*?location\.reload\(\);/, + /function reloadAfterMissingAcceptedDom\(pending, recoveryRevision\)[\s\S]*?location\.reload\(\);/, 'missing accepted DOM after clean source should recover by reloading the clean page', ); + assert.match( + SOURCE, + /restoreAcceptedDomFromSnapshot\(pending, recoveryRevision\)[\s\S]*?function restoreAcceptedDomFromSnapshot\(pending, recoveryRevision\)[\s\S]*?reloadAfterMissingAcceptedDom\(pending, recoveryRevision\)/, + 'snapshot restoration must carry the originating recovery revision into its reload fallback', + ); + assert.match( + SOURCE, + /function ensureAcceptedDomClean\(pending, recoveryRevision\) \{[\s\S]{0,250}?deferredRecoverySuperseded\(pending\?\.id, recoveryRevision\)[\s\S]*?setTimeout\(function\(\) \{[\s\S]{0,180}?deferredRecoverySuperseded\(pending\?\.id, recoveryRevision\)[\s\S]{0,180}?location\.reload\(\);/, + 'accepted-session cleanup and its reload fallback must yield to a newer Live session', + ); + }); + + it('never runs accept or discard structural fallbacks inside framework-owned HMR DOM', () => { + assert.match( + SOURCE, + /if \(hasFrameworkHmrOwnership\(wrappers\[0\] \|\| pending\?\.parentElement\)\) \{[\s\S]{0,500}?acceptedDomAlreadyClean\(pending\)[\s\S]{0,80}?location\.reload\(\);[\s\S]{0,80}?return;[\s\S]{0,120}?if \(wrappers\.length === 0\)/, + 'accept cleanup must use a reload grace fallback before any framework-owned structural mutation', + ); + assert.match( + SOURCE, + /if \(hasFrameworkHmrOwnership\(lateWrapper\)\) \{[\s\S]{0,900}?location\.reload\(\);[\s\S]{0,100}?return;[\s\S]{0,150}?releaseDiscardedStaticWrapper\(lateWrapper, cleanupSessionId\)/, + 'discard cleanup must use a reload grace fallback before replacing a framework-owned wrapper', + ); + assert.match( + SOURCE, + /function releaseDiscardedStaticWrapper\(wrapper, sessionId\)[\s\S]{0,400}?replaceChild\(content, wrapper\)/, + 'only the static-wrapper release helper may structurally restore discarded DOM', + ); + assert.match( + SOURCE, + /if \(hasFrameworkHmrOwnership\(lateWrapper\)\) \{[\s\S]{0,700}?removeDiscardStateStylesheet\(cleanupSessionId\);[\s\S]{0,120}?location\.reload\(\);/, + 'discard must keep its original-visibility stylesheet until the HMR grace window ends', + ); + assert.match( + SOURCE, + /const recoverySuperseded = deferredRecoverySuperseded\(cleanupSessionId, cleanupRevision\);[\s\S]{0,500}?if \(recoverySuperseded\) \{[\s\S]{0,250}?watchForDiscardedFrameworkWrapperRemoval\(cleanupSessionId\)[\s\S]{0,150}?releaseDiscardedStaticWrapper\(lateWrapper, cleanupSessionId\)[\s\S]{0,80}?return;/, + 'discard cleanup and its reload grace callback must yield to a newer Live session', + ); + assert.match( + SOURCE, + /function discardStateStyleId\(sessionId\)[\s\S]{0,100}?DISCARD_STATE_STYLE_ID \+ '-' \+ sessionId[\s\S]{0,400}?getElementById\(discardStateStyleId\(sessionId\)\)/, + 'concurrent discard sessions must retain independent visibility stylesheets', + ); + assert.match( + SOURCE, + /function removeDiscardStateStylesheet\(sessionId\)[\s\S]{0,100}?if \(!sessionId\) return;[\s\S]{0,100}?getElementById\(discardStateStyleId\(sessionId\)\)\?\.remove\(\);/, + 'an older discard callback must remove only its own session stylesheet', + ); + assert.match( + SOURCE, + /setTimeout\(function\(\) \{[\s\S]{0,300}?const staleWrapper = document\.querySelector[\s\S]{0,250}?deferredRecoverySuperseded\(cleanupSessionId, cleanupRevision\)[\s\S]{0,250}?watchForDiscardedFrameworkWrapperRemoval\(cleanupSessionId\)[\s\S]{0,100}?return;[\s\S]{0,100}?removeDiscardStateStylesheet\(cleanupSessionId\);[\s\S]{0,100}?location\.reload\(\);/, + 'framework discard recovery may observe safe HMR cleanup but must not reload replacement work', + ); + assert.match( + SOURCE, + /const discardedFrameworkWrapperWatchers = new Map\(\);[\s\S]*?function watchForDiscardedFrameworkWrapperRemoval\(sessionId\)[\s\S]{0,1500}?const replacementActive = !!currentSessionId[\s\S]{0,180}?state !== 'IDLE' && state !== 'PICKING'[\s\S]{0,300}?setTimeout\(resolveStillMounted, 12000\)[\s\S]{0,300}?location\.reload\(\);/, + 'discard recovery must keep observing during replacement work and reload stale framework DOM once Live is idle', + ); + }); + + it('recovers a handled variant or carbonize wrapper after HMR cancels the original cleanup timer', () => { + const start = SOURCE.indexOf('function scheduleHandledRuntimeWrapperReload(wrapper,'); + const end = SOURCE.indexOf('\n function resumeSession(', start); + const recovery = SOURCE.slice(start, end); + assert.match(recovery, /impeccableCarbonize/); + assert.match(recovery, /sessionStorage\.getItem\(handledWrapperReloadKey\(sessionId\)\)/); + assert.match(recovery, /sessionStorage\.setItem\(handledWrapperReloadKey\(sessionId\), String\(reloadAttempts \+ 1\)\)/); + assert.match(recovery, /if \(reloadAttempts >= 2\) return true;/); + assert.match(recovery, /handledRuntimeWrapperReloadSessions\.has\(sessionId\)/); + assert.match(recovery, /handledRuntimeWrapperReloadSessions\.add\(sessionId\)/); + assert.match( + recovery, + /deferredRecoverySuperseded\(sessionId, recoveryRevision\)[\s\S]*?return true;[\s\S]*?setTimeout\(function\(\) \{[\s\S]{0,180}?deferredRecoverySuperseded\(sessionId, recoveryRevision\)[\s\S]{0,220}?handledRuntimeWrapperReloadSessions\.delete\(sessionId\);[\s\S]{0,80}?return;/, + 'handled-wrapper recovery must never reload a newer Live session', + ); + assert.match( + SOURCE, + /const handledRuntimeWrapperReloadSessions = new Set\(\);[\s\S]*?function handledWrapperReloadKey\(sessionId\)[\s\S]{0,100}?HANDLED_WRAPPER_RELOAD_KEY \+ ':' \+ sessionId/, + 'overlapping handled sessions must have independent timers and retry budgets', + ); + assert.match(recovery, /\[data-impeccable-variants=.+\[data-impeccable-carbonize=/s); + assert.match(recovery, /if \(staleWrapper\) location\.reload\(\);/); + assert.match( + SOURCE, + /function resumeSession\(recoveryRevision = liveInteractionRevision\)[\s\S]{0,250}?\[data-impeccable-carbonize\][\s\S]{0,180}?scheduleHandledRuntimeWrapperReload\(runtimeWrapper, recoveryRevision\)/, + 'resume must inspect handled carbonize wrappers before clearing handled state', + ); + assert.match( + SOURCE, + /function restoreSessionSupersedingHandledWrapper\(runtimeWrapper\)[\s\S]{0,900}?saved\.id === handledSessionId[\s\S]{0,300}?restoreSessionWithoutWrapper\('browser_resumed_over_handled_wrapper'\)/, + 'handled-wrapper recovery must recognize a different durable session as newer work', + ); + assert.match( + SOURCE, + /function isUsableInjectionAnchor\(el\)[\s\S]{0,250}?closest\?\.\('\[data-impeccable-variants\],\[data-impeccable-carbonize\]'\)/, + 'a newer restored session must wait for a real page anchor outside stale handled wrappers', + ); + const resumeStart = SOURCE.indexOf('function resumeSession('); + const resumeEnd = SOURCE.indexOf('\n //', resumeStart); + const resume = SOURCE.slice(resumeStart, resumeEnd); + assert.ok( + resume.indexOf('restoreSessionSupersedingHandledWrapper(runtimeWrapper)') + < resume.indexOf('scheduleHandledRuntimeWrapperReload(runtimeWrapper, recoveryRevision)'), + 'a newer durable session must restore before a stale handled wrapper can schedule another reload', + ); + assert.doesNotMatch( + resume, + /clearHandled\(\);/, + 'bounded handled state must survive arbitrarily late wrapper hydration and later reloads', + ); + assert.match( + resume, + /browser_resumed_svelte_orphan_wrapper[\s\S]{0,150}?clearHandled\(sessionId\);/, + 'orphan cleanup must clear only its own handled id', + ); + assert.match( + SOURCE, + /if \(!accepted\?\.isSvelteComponent\) \{[\s\S]{0,100}?watchForHandledRuntimeWrapper\(accepted\?\.id, recoveryRevision\);/, + 'accept cleanup should watch for a carbonize wrapper mounted by a delayed framework refresh', + ); + assert.match( + recovery, + /function watchForHandledRuntimeWrapper\(sessionId, recoveryRevision = liveInteractionRevision\)[\s\S]*?handledRuntimeWrapperWatchers\.get\(sessionId\)[\s\S]*?observer\.observe\(document\.body, \{ childList: true, subtree: true \}\)[\s\S]*?handledRuntimeWrapperWatchers\.set\(sessionId, \{ observer, timer \}\);/, + 'late handled-wrapper recovery should remain bounded while covering slow HMR updates', + ); + assert.match( + SOURCE, + /const handledRuntimeWrapperWatchers = new Map\(\);[\s\S]*?handledRuntimeWrapperWatchers\.delete\(sessionId\);/, + 'overlapping handled-wrapper scouts must retain independent observer state per session', + ); + }); + + it('keeps watching for a framework wrapper when session restore wins the hydration race', () => { + assert.match( + SOURCE, + /const resumed = resumeSession\(\);[\s\S]{0,1200}?if \(!resumed \|\| !document\.querySelector\('\[data-impeccable-variants\],\[data-impeccable-carbonize\]'\)\) \{[\s\S]{0,500}?const scout = new MutationObserver/, + 'restoring durable session state before hydration must still install the deferred-wrapper scout', + ); + assert.match( + SOURCE, + /const deferredResumeRevision = liveInteractionRevision;[\s\S]{0,350}?const scout = new MutationObserver[\s\S]{0,350}?resumeSession\(deferredResumeRevision\)/, + 'the deferred-wrapper scout must retain its originating interaction revision', + ); + }); + + it('invalidates nullable deferred recovery as soon as a replacement edit starts configuring', () => { + assert.match( + SOURCE, + /function beginNewLiveConfiguration\(\) \{[\s\S]{0,100}?liveInteractionRevision \+= 1;[\s\S]{0,80}?setLiveState\('CONFIGURING'\);/, + ); + assert.equal( + SOURCE.match(/beginNewLiveConfiguration\(\);/g)?.length || 0, + 3, + 'mouse replace, mouse insert, and keyboard configuration must all supersede older recovery timers', + ); + assert.match( + SOURCE, + /function deferredRecoverySuperseded\(sessionId, recoveryRevision\) \{[\s\S]{0,160}?liveInteractionRevision !== recoveryRevision[\s\S]{0,100}?currentSessionId !== sessionId/, + 'recovery must be fenced before a replacement configuration has a non-null session id', + ); + assert.match( + SOURCE, + /function scheduleAcceptCleanup\(accepted\) \{[\s\S]{0,100}?const recoveryRevision = liveInteractionRevision;[\s\S]*?watchForHandledRuntimeWrapper\(accepted\?\.id, recoveryRevision\)/, + 'accept and handled-wrapper recovery must share the originating interaction revision', + ); }); it('normalizes generated JSX source before source-fallback DOM parsing', () => { diff --git a/tests/live-e2e/agent.mjs b/tests/live-e2e/agent.mjs index aac83e985..ca2f2d4c6 100644 --- a/tests/live-e2e/agent.mjs +++ b/tests/live-e2e/agent.mjs @@ -29,6 +29,7 @@ import { promisify } from 'node:util'; import { completionTypeForAcceptResult } from '../../skill/scripts/live/completion.mjs'; const execFileP = promisify(execFile); +const CARBONIZE_HMR_BOUNDARY_MS = 250; export const STEER_MARKER_ATTR = 'data-impeccable-steer'; export const STEER_MARKER_VALUE = 'e2e'; @@ -2172,6 +2173,13 @@ export async function runAgentLoop({ const post = await fs.readFile(path.join(tmp, acceptResult.file), 'utf-8'); log(`--- post-accept (pre-carbonize) ---\n${post}`); } + // live-accept writes the intermediate carbonize tree, then a real + // agent reads its cleanup instructions before writing the final + // source. The deterministic agent otherwise collapses both writes + // into the same filesystem watcher tick, so Vite can observe the + // carbonize state but miss the clean state entirely. Preserve the + // real protocol boundary instead of relying on browser DOM cleanup. + await new Promise((resolve) => setTimeout(resolve, CARBONIZE_HMR_BOUNDARY_MS)); await runCarbonizeCleanup({ tmp, file: acceptResult.file, sessionId: event.id, variant: event.variantId }); log(`carbonize cleanup done on ${acceptResult.file}`); } @@ -2455,7 +2463,12 @@ async function runCarbonizeCleanup({ tmp, file, sessionId /* , variant */ }) { // element is now dead weight. body = body.replace(/\s+data-impeccable-hoist-id="[^"]*"/g, ''); - await fs.writeFile(filePath, body, 'utf-8'); + // Publish the clean source atomically. A direct write briefly exposes a + // zero-byte file, which can make the harness (and Vite) observe cleanup as + // complete before the accepted source has actually landed. + const temporaryPath = `${filePath}.impeccable-carbonize-${sessionId}.tmp`; + await fs.writeFile(temporaryPath, body, 'utf-8'); + await fs.rename(temporaryPath, filePath); } function unwrapDivAttributeWrapper(body, attrName, { expandSingleLineContainer = false } = {}) { diff --git a/tests/live-reference.test.mjs b/tests/live-reference.test.mjs index e9df0e261..25fc58719 100644 --- a/tests/live-reference.test.mjs +++ b/tests/live-reference.test.mjs @@ -11,8 +11,8 @@ describe('live reference authoring contract', () => { const skillSrc = readFileSync(join(ROOT, 'skill/SKILL.src.md'), 'utf-8'); const liveMd = readFileSync(join(ROOT, 'skill/reference/live.md'), 'utf-8'); - assert.match(skillSrc, /load the one playbook that owns the request/); - assert.match(skillSrc, /Commands table's reference for an explicit or clearly implied sub-command/); + assert.match(skillSrc, /Load the request's playbook/); + assert.match(skillSrc, /Commands-table reference for an explicit\/implied sub-command/); assert.doesNotMatch(skillSrc, /Use this same scripts directory for all Impeccable helper commands/); assert.doesNotMatch(skillSrc, /walk upward for the nearest project `\.agents`, `\.claude`, or `\.cursor` skill/); assert.doesNotMatch(skillSrc, /## Context diagnostics/); @@ -23,7 +23,7 @@ describe('live reference authoring contract', () => { const skillSrc = readFileSync(join(ROOT, 'skill/SKILL.src.md'), 'utf-8'); const liveMd = readFileSync(join(ROOT, 'skill/reference/live.md'), 'utf-8'); - assert.match(skillSrc, /load the one playbook that owns the request/); + assert.match(skillSrc, /Load the request's playbook/); assert.doesNotMatch(skillSrc, /TARGET_SELECTION_REQUIRED/); assert.doesNotMatch(skillSrc, /productStatus/); assert.doesNotMatch(skillSrc, /designStatus/);