From 2fa0e7d327fd22970cb988db3be842490ae525d1 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 22 Jul 2026 22:46:04 -0700 Subject: [PATCH] Live: gate mid-generation source injection, monotonic bar, resumable disconnect Three browser-side fixes for the same 3.5-to-4.0.1 regression. - Source-preview targets no longer source-inject per variant_progress checkpoint. Immediate injection raced framework (React/Vue) ownership and triggered removeChild errors, which surfaced as static previews. HMR now owns reconciliation while variants stream in; source injection runs only on the final done (its 750ms settle + retry ladder stays for non-HMR harnesses like Cursor). Progress counts still advance from the variant observer, and the svelte-component progressive path is unchanged. - The agent-phase progress bar advances monotonically. A behind/resumed checkpoint re-broadcasts an earlier phase (the server regresses the snapshot phase to generating), which moved the visible bar backward; a phase rank table now blocks a known-lower phase from overwriting a known-higher one. - The server-lost toast now frames the drop as resumable (session saved, reopen or restart live-poll.mjs) instead of "Session ended", which had led agents to rationalize bailing to direct edits. Co-Authored-By: Claude Fable 5 --- skill/scripts/live-browser.js | 67 ++++++++++++++++++++------ tests/live-browser-regression.test.mjs | 42 +++++++++++++++- tests/live-browser-source.test.mjs | 27 +++++++++-- 3 files changed, 114 insertions(+), 22 deletions(-) diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index f5c3bef03..07f09326a 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -127,6 +127,41 @@ let arrivedVariants = 0; let visibleVariant = 0; let generationPhase = null; + // Ascending order of the agent-generation lifecycle. The visible progress bar + // must never regress: a `browser_resumed`/behind checkpoint re-broadcasts an + // earlier phase (the server regresses the snapshot phase to `generating` on a + // behind checkpoint), and without this the bar jumps backward mid-generation. + // Unranked phases (params sidecar flow, unknown values) always pass so we + // never block a phase we do not model. + const PHASE_RANK = { + queued: 0, + picked_up: 1, + scaffolding: 2, + scaffold_fallback: 3, + source_ready: 4, + generation_ready: 5, + generating: 5, + variants_progress: 5, + first_variant_generating: 6, + first_variant_validating: 7, + first_reviewable: 8, + remaining_variants_generating: 9, + remaining_variants_validating: 10, + second_reviewable: 11, + all_variants_ready: 12, + variants_ready: 12, + variant_parameters_generating: 13, + variant_parameters_validating: 14, + parameters_ready: 15, + }; + function shouldAdvancePhase(current, next) { + if (!next || next === current) return false; + const nextRank = PHASE_RANK[next]; + const currentRank = PHASE_RANK[current]; + // Only block a known-lower phase from overwriting a known-higher one. + if (nextRank === undefined || currentRank === undefined) return true; + return nextRank >= currentRank; + } let parameterGenerationState = 'idle'; let parameterReadyAnnouncedSession = null; let svelteComponentSession = null; @@ -6347,7 +6382,10 @@ break; case 'agent_phase': if (msg.id === currentSessionId && (state === 'GENERATING' || state === 'CYCLING')) { - generationPhase = msg.phase || generationPhase; + // Advance the visible phase monotonically. A behind/resumed + // checkpoint may carry an earlier phase for internal bookkeeping, + // but the bar must not move backward. + if (shouldAdvancePhase(generationPhase, msg.phase)) generationPhase = msg.phase; if (msg.phase === 'variant_parameters_generating' || msg.phase === 'variant_parameters_validating') { parameterGenerationState = 'loading'; } @@ -6363,22 +6401,19 @@ if (msg.publicationKind === 'params') parameterGenerationState = 'loading'; rememberSessionFileMeta(msg); if (isFrameworkComponentPreviewMode(msg.previewMode) && msg.previewFile) { + // Component-preview (Svelte/Vue) progressive delivery: the browser + // mounts compiled components, so there is no framework-owned DOM + // to race. Keep streaming each checkpoint into the preview. injectSvelteComponentsFromManifest(msg.previewFile, msg.id); - } else if ((msg.previewMode === 'source' || !msg.previewMode) && (msg.previewFile || msg.file)) { - // Give normal framework HMR the first chance to reconcile its - // own managed tree. Nuxt route-module HMR can skip intermediate - // revisions, so fall back to source injection only when the - // advertised progress still has not appeared after a short - // settle. Immediate injection races React/Vue ownership and can - // trigger removeChild errors on the next HMR commit. - const targetArrived = Number(msg.arrivedVariants) || 1; - setTimeout(() => { - if (msg.id !== currentSessionId) return; - if (state !== 'GENERATING' && state !== 'CYCLING') return; - if (msg.publicationKind !== 'params' && arrivedVariants >= targetArrived) return; - injectVariantsFromSource(msg.previewFile || msg.file, msg.id); - }, 150); } + // Source-preview targets: do NOT source-inject per checkpoint. + // Immediate injection races framework (React/Vue) ownership mid- + // generation and triggers removeChild errors on the next HMR + // commit. Let HMR own reconciliation while variants stream in; + // source injection runs only on the final `done` (which keeps its + // 750ms settle + retry ladder for non-HMR harnesses like Cursor). + // The visible progress count still advances from the variant + // MutationObserver as HMR lands each variant. } break; case 'steer_done': @@ -6489,7 +6524,7 @@ function handleServerLost() { const recoveryState = currentSessionId ? state : 'IDLE'; if (state === 'GENERATING' || state === 'CYCLING' || state === 'SAVING') { - showToast('Live server disconnected. Session ended.', 5000); + showToast('Live server connection lost. Your session is saved; reopen this page or restart live-poll.mjs to continue.', 6000); } hideBar(); hideHighlight(); diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs index e2066e664..406e1e97e 100644 --- a/tests/live-browser-regression.test.mjs +++ b/tests/live-browser-regression.test.mjs @@ -286,6 +286,41 @@ describe('live-browser.js regression guards', () => { ); }); + it('server-lost toast frames the disconnect as resumable, not ended', () => { + assert.doesNotMatch( + SOURCE, + /Live server disconnected\. Session ended\./, + 'the "Session ended" copy made agents rationalize bailing to direct edits; the session is resumable', + ); + assert.match( + SOURCE, + /Live server connection lost\. Your session is saved;[^']*restart live-poll\.mjs to continue\./, + 'server-lost toast should tell the user the session is saved and how to continue', + ); + }); + + it('the agent-phase progress bar advances monotonically', () => { + // A behind/resumed checkpoint must not move the visible bar backward. + assert.doesNotMatch( + SOURCE, + /generationPhase = msg\.phase \|\| generationPhase;/, + 'raw phase assignment lets a behind checkpoint regress the visible bar to an earlier phase', + ); + assert.match( + SOURCE, + /case 'agent_phase':[\s\S]{0,400}?if \(shouldAdvancePhase\(generationPhase, msg\.phase\)\) generationPhase = msg\.phase;/, + 'agent_phase should only advance the phase when it moves forward', + ); + // The rank table must order the lifecycle so scaffolding/source_ready sit + // below generating and the reviewable phases. + assert.match(SOURCE, /function shouldAdvancePhase\(current, next\)/); + assert.match( + SOURCE, + /scaffolding: 2,[\s\S]{0,120}?source_ready: 4,[\s\S]{0,120}?(generation_ready|generating): 5,/, + 'scaffolding and source_ready must rank below generating', + ); + }); + it('source reinjection preserves the visible variant after cycling', () => { assert.doesNotMatch( SOURCE, @@ -920,10 +955,13 @@ describe('live-browser.js regression guards', () => { ); assert.match(SOURCE, /tune\.disabled = true/, 'pending Tune must be visibly loading but non-interactive'); assert.match(SOURCE, /Tune controls are ready\./, 'parameter arrival needs a clear ready indication'); + // Source-mode DOM injection is gated to the `done` branch (it races + // framework ownership mid-generation), but a params-only publication must + // still flip the Tune controls into their loading state on the checkpoint. assert.match( SOURCE, - /msg\.publicationKind !== 'params' && arrivedVariants >= targetArrived/, - 'a params-only publication must refresh even though the variant count is unchanged', + /case 'variant_progress':[\s\S]{0,120}?if \(msg\.publicationKind === 'params'\) parameterGenerationState = 'loading';/, + 'a params-only publication must mark Tune controls loading even though the variant count is unchanged', ); assert.match(SOURCE, /revisionDomain: 'browser'/, 'browser checkpoints must use their own revision domain'); }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index 86669e3a2..f30141c9b 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -421,11 +421,30 @@ describe('live-browser source contracts', () => { ); }); - it('loads progressive source checkpoints through the no-HMR fallback', () => { + it('does not source-inject per variant_progress checkpoint (HMR owns mid-generation reconciliation)', () => { + // Isolate the variant_progress handler body. + const progressCase = SOURCE.match(/case 'variant_progress':[\s\S]*?break;/); + assert.ok(progressCase, 'variant_progress case should exist'); + assert.doesNotMatch( + progressCase[0], + /injectVariantsFromSource\(/, + 'source-mode progress must not source-inject per checkpoint; it races React/Vue ownership and triggers removeChild errors', + ); + // The svelte-component progressive path stays. assert.match( - SOURCE, - /case 'variant_progress':[\s\S]{0,1400}?msg\.previewMode === 'source'[\s\S]{0,1000}?arrivedVariants >= targetArrived[\s\S]{0,260}?injectVariantsFromSource\(msg\.previewFile \|\| msg\.file, msg\.id\)/, - 'source-mode progress should let framework HMR settle before using the no-HMR fallback', + progressCase[0], + /injectSvelteComponentsFromManifest\(msg\.previewFile, msg\.id\)/, + 'component-preview progressive delivery must still stream per checkpoint', + ); + }); + + it('source-injects only on the final done branch, keeping the 750ms settle', () => { + const doneCase = SOURCE.match(/case 'done':[\s\S]*?break;\n {8}case /); + assert.ok(doneCase, 'done case should exist'); + assert.match( + doneCase[0], + /setTimeout\([\s\S]{0,260}?injectVariantsFromSource\(msg\.file, msg\.id, \{ generationCompleted: true \}\)[\s\S]{0,40}?\}, 750\)/, + 'done should source-inject via the 750ms fallback for harnesses without HMR', ); }); });