diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index c32101b12..10353bc95 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -6907,6 +6907,27 @@ // MutationObserver for progressive variant reveal // + // A session id can have more than one wrapper in the DOM: the target may sit + // inside a `.map()` callback (the wrapper renders once per item), or the + // agent may have relocated the wrapper out of the shared primitive live-wrap + // scaffolded into. A plain first match can then pin an empty scaffold while + // the real variants sit in a later wrapper, which strands the session at + // 0/N. Prefer a wrapper that actually holds variants. With zero or one match + // this is exactly the querySelector it replaces. + function findVariantsWrapper(sessionId) { + const selector = sessionId + ? '[data-impeccable-variants="' + sessionId + '"]' + : '[data-impeccable-variants]'; + const matches = document.querySelectorAll(selector); + if (matches.length < 2) return matches[0] || null; + for (const candidate of matches) { + if (candidate.querySelector('[data-impeccable-variant]:not([data-impeccable-variant="original"])')) { + return candidate; + } + } + return matches[0]; + } + function startVariantObserver(sessionId) { let updating = false; // re-entrancy guard @@ -6936,7 +6957,7 @@ } if (!dominated) return; - const wrapper = document.querySelector('[data-impeccable-variants="' + sessionId + '"]'); + const wrapper = findVariantsWrapper(sessionId); if (!wrapper) return; const variants = wrapper.querySelectorAll('[data-impeccable-variant]:not([data-impeccable-variant="original"])'); @@ -9059,7 +9080,7 @@ void main() { } function restoreFromActiveSessions(activeSessions, reason) { - const wrapper = document.querySelector('[data-impeccable-variants]'); + const wrapper = findVariantsWrapper(null); if (wrapper && !isFrameworkComponentPreviewMode(wrapper.dataset.impeccablePreview)) return false; if (svelteComponentSession?.sessionId === currentSessionId) return false; return restoreSessionWithoutWrapper(reason || 'sse_connected', activeSessions); @@ -9400,8 +9421,13 @@ void main() { return restoreSessionWithoutWrapper('browser_resumed_over_handled_wrapper'); } - function resumeSession(recoveryRevision = liveInteractionRevision) { - const wrapper = document.querySelector('[data-impeccable-variants]'); + function resumeSession(recoveryRevision = liveInteractionRevision, opts = {}) { + // Which path resumed matters in the journal: an init resume is a fresh + // page load, the deferred-wrapper scout is a mid-page-load arrival. Both + // used to log the same `browser_resumed`, which made issue #719 take a + // DOM reconstruction to diagnose. + const resumeReason = opts.reason || 'browser_resumed'; + const wrapper = findVariantsWrapper(null); const runtimeWrapper = wrapper || document.querySelector('[data-impeccable-carbonize]'); if (restoreSessionSupersedingHandledWrapper(runtimeWrapper)) return true; if (scheduleHandledRuntimeWrapperReload(runtimeWrapper, recoveryRevision)) return false; @@ -9500,16 +9526,38 @@ void main() { showBar(state === 'CYCLING' ? 'cycling' : 'generating'); startScrollTracking(); - // Build the params panel for the restored visible variant. Previously - // this was missed on page-reload resume: showVariantInDOM above fires - // refreshParamsPanel, but state was still IDLE at that moment so it - // hid. Now that state is CYCLING, re-fire. - if (state === 'CYCLING') refreshParamsPanel(); + // A resume can BE the arrival, not just a re-entry after one. The server's + // generation preflight runs live-wrap with --defer-source-write, so the + // wrapper and every variant reach the DOM in one HMR batch, and the + // deferred-wrapper scout (constructed at init) runs before the variant + // MutationObserver (constructed at Go) on that batch. Finish the same + // transition the observer would have finished. Without hideShaderOverlay + // the generating shader stays frozen over the target and the session looks + // stuck at GENERATING while the bar already cycles (issue #719). + if (state === 'CYCLING') { + recoveryWaitingForAnchor = false; + hideShaderOverlay(); + if (isInsert) finalizeInsertSession(); + disableInlineEdit(); + // Build the params panel for the restored visible variant. Previously + // this was missed on page-reload resume: showVariantInDOM above fires + // refreshParamsPanel, but state was still IDLE at that moment so it + // hid. Now that state is CYCLING, re-fire. + refreshParamsPanel(); + } saveSession(); if (arrivedVariants > 0 && arrivedVariants < expectedVariants) { sendCheckpoint('variants_progress'); } else { - queueCheckpoint('browser_resumed'); + queueCheckpoint(resumeReason); + // Only variants_progress and variants_ready count as publication + // progress. When the resume is the arrival, the observer never gets to + // report it (this function disconnects and re-creates it below, which + // drops the records it had already queued for this same batch), so + // without this the server never learns the variants were published. + if (arrivedVariants > 0 && arrivedVariants >= expectedVariants && expectedVariants > 0) { + sendCheckpoint('variants_ready'); + } } // Start observing for more variants AFTER initial setup @@ -12831,7 +12879,7 @@ void main() { const wrapper = document.querySelector('[data-impeccable-variants],[data-impeccable-carbonize]'); if (!wrapper) return; scout.disconnect(); - if (resumeSession(deferredResumeRevision)) { + if (resumeSession(deferredResumeRevision, { reason: 'browser_resumed_deferred_wrapper' })) { console.log('[impeccable] Resumed deferred session ' + currentSessionId + ' (post-hydration).'); } }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index 0617806f8..f2aaa0b46 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -470,7 +470,7 @@ describe('live-browser source contracts', () => { 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\)/, + /function resumeSession\(recoveryRevision = liveInteractionRevision, opts = \{\}\)[\s\S]{0,700}?\[data-impeccable-carbonize\][\s\S]{0,180}?scheduleHandledRuntimeWrapperReload\(runtimeWrapper, recoveryRevision\)/, 'resume must inspect handled carbonize wrappers before clearing handled state', ); assert.match( @@ -526,11 +526,67 @@ describe('live-browser source contracts', () => { ); 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', + /const deferredResumeRevision = liveInteractionRevision;[\s\S]{0,350}?const scout = new MutationObserver[\s\S]{0,400}?resumeSession\(deferredResumeRevision, \{ reason: 'browser_resumed_deferred_wrapper' \}\)/, + 'the deferred-wrapper scout must retain its originating interaction revision and name itself in the journal', ); }); + it('finishes the cycling transition when the resume is the arrival (#719)', () => { + // The server's generation preflight runs live-wrap with + // --defer-source-write, so the wrapper and every variant reach the DOM in + // one HMR batch. The deferred-wrapper scout is constructed at init, the + // variant MutationObserver at Go, and observer callbacks run in + // construction order, so on that batch the scout resumes first and + // resumeSession IS the transition into CYCLING. It has to finish the same + // transition the observer would have: leaving the generating shader up + // paints a frozen capture of the original over a DOM that already holds + // the variants, which is the stuck loader from issue #719. + const resumeStart = SOURCE.indexOf('function resumeSession('); + const resumeEnd = SOURCE.indexOf('\n //', resumeStart); + const resume = SOURCE.slice(resumeStart, resumeEnd); + assert.match( + resume, + /if \(state === 'CYCLING'\) \{[\s\S]{0,200}?hideShaderOverlay\(\);/, + 'a resume into CYCLING must take the generating shader down', + ); + assert.match( + resume, + /if \(state === 'CYCLING'\) \{[\s\S]{0,700}?refreshParamsPanel\(\);/, + 'a resume into CYCLING must still rebuild the params panel', + ); + // Only variants_progress|variants_ready count as publication progress, so + // a resume that already holds every variant has to report one of them or + // the server never learns the generation was published. + assert.match( + resume, + /queueCheckpoint\(resumeReason\);[\s\S]{0,500}?sendCheckpoint\('variants_ready'\)/, + 'a complete resume must report variants_ready, not only browser_resumed', + ); + }); + + it('prefers the wrapper that actually holds variants over the first match (#719)', () => { + // A target inside a `.map()` renders one wrapper per item, and an agent + // that relocates the wrapper out of the shared primitive live-wrap + // scaffolded leaves an empty one behind. First match can then pin a + // scaffold with no variants and strand the session at 0/N. + const start = SOURCE.indexOf('function findVariantsWrapper(sessionId)'); + assert.ok(start > 0, 'findVariantsWrapper must exist'); + const helper = SOURCE.slice(start, SOURCE.indexOf('\n function startVariantObserver(', start)); + assert.match(helper, /if \(matches\.length < 2\) return matches\[0\] \|\| null;/); + assert.match( + helper, + /candidate\.querySelector\('\[data-impeccable-variant\]:not\(\[data-impeccable-variant="original"\]\)'\)[\s\S]{0,80}?return candidate;/, + 'the preferred wrapper is the one holding non-original variants', + ); + assert.match(helper, /return matches\[0\];/, 'with no populated wrapper the old first match still wins'); + for (const caller of [ + 'const wrapper = findVariantsWrapper(sessionId);', + 'const wrapper = findVariantsWrapper(null);', + ]) { + assert.ok(SOURCE.includes(caller), `${caller} should be how the resolvers look a wrapper up`); + } + }); + it('invalidates nullable deferred recovery as soon as a replacement edit starts configuring', () => { assert.match( SOURCE, diff --git a/tests/live-e2e/preactions.mjs b/tests/live-e2e/preactions.mjs index 87ddd76c7..9c7a7af9f 100644 --- a/tests/live-e2e/preactions.mjs +++ b/tests/live-e2e/preactions.mjs @@ -93,7 +93,8 @@ export async function waitForCyclingRobust(page, expectedCount, opts = {}) { try { await waitForCycling(page, expectedCount, { timeout: firstPassTimeoutMs }); return; - } catch { + } catch (preErr) { + if (preErr?.impeccableNoRetry) throw preErr; log(`Cycling not reached in ${firstPassTimeoutMs}ms — retracing preActions`); await runPreActions(page, preActions); } @@ -106,7 +107,7 @@ export async function waitForCyclingRobust(page, expectedCount, opts = {}) { if (process.env.IMPECCABLE_E2E_DEBUG) { firstErr.message += '\n\n--- live UI snapshot ---\n' + JSON.stringify(await liveUiSnapshot(page), null, 2); } - if (agentMode !== 'llm') throw firstErr; + if (agentMode !== 'llm' || firstErr?.impeccableNoRetry) throw firstErr; } log('Cycling not reached after LLM generate — reloading to pick up HMR'); diff --git a/tests/live-e2e/ui.mjs b/tests/live-e2e/ui.mjs index be02bd669..7b2bc259f 100644 --- a/tests/live-e2e/ui.mjs +++ b/tests/live-e2e/ui.mjs @@ -605,6 +605,35 @@ export async function clickGo(page) { throw lastErr || new Error('Go click did not leave configure mode'); } +/** + * The generating shader is the visible loader: a frozen capture of the + * original painted over the target while variants are being written. Every + * transition into CYCLING has to take it down, so a cycling bar with the + * shader still up is the stuck loader from issue #719, not a cosmetic + * detail. The transition is synchronous, so a short window is generous. + */ +async function assertGeneratingShaderCleared(page) { + const stillUp = await page + .waitForFunction( + () => !(window.__impeccableLiveQuery || document.querySelector.bind(document))('#impeccable-live-shader'), + undefined, + { timeout: 5_000 }, + ) + .then(() => false) + .catch(() => true); + if (stillUp) { + const err = new Error( + 'CYCLING reached but the generating shader overlay (#impeccable-live-shader) is still up: ' + + 'the loader never handed off to the variant cycler', + ); + // Cycling was reached, so the recovery paths in waitForCyclingRobust + // (retrace preActions, reload) have nothing to recover and a reload would + // hide the defect by taking the shader down with the page. + err.impeccableNoRetry = true; + throw err; + } +} + /** * Wait for the bar to enter CYCLING state — happens after the agent's * variants land in the DOM via HMR and the MutationObserver counts them. @@ -636,6 +665,7 @@ export async function waitForCycling(page, expectedCount, { timeout = 30_000 } = { barSel: BAR_ID, expected: expectedCount }, { timeout }, ); + await assertGeneratingShaderCleared(page); } catch (err) { if (process.env.IMPECCABLE_E2E_DEBUG) { const snapshot = await page.evaluate((barSel) => {