From 69456364b288ab5cf526a1c2432aa1eb732bcfc2 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Tue, 28 Jul 2026 15:57:33 -0700 Subject: [PATCH] fix: self-discard orphaned variant sessions instead of freezing the picker Fixes #439. When a cycling session is abandoned and the wrapped region is then edited or regenerated out of the source file, the resumed page used to sit in GENERATING forever with the picker disarmed; the only recovery was a manual live-complete --discarded. Now a resumed CYCLING session whose wrapper cannot be found in source retries the read a few times (HMR or an agent write may be mid-flight), then discards itself, clears local state, and re-arms the picker with a toast. GENERATING restores are exempt: deferred-wrapper flows legitimately have no wrapper in source until the agent's write lands. The browser tags the discard event orphaned:true; the server terminalizes that session directly (phase discarded) and keeps the event out of the agent poll queue, since there is no source cleanup left to perform and the normal discard flow would just fail against the missing scaffolding. New e2e scenario on vite8-react-plain drives the full repro: cycle, revert source externally, reload, assert self-discard, terminal durable phase, and a working picker afterward. AI-assisted (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/live-browser.js | 44 ++++++++++++- skill/scripts/live-server.mjs | 14 +++- .../vite8-react-plain/fixture.json | 45 +++++++++++-- tests/live-e2e.test.mjs | 66 +++++++++++++++++++ 4 files changed, 160 insertions(+), 9 deletions(-) diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index b32a4fac2..2a4d50def 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -6169,6 +6169,22 @@ const COMPLETED_SOURCE_FALLBACK_RETRIES = 3; const COMPLETED_SOURCE_FALLBACK_RETRY_MS = 1200; + /** + * Terminal recovery for a session whose source-side scaffolding no longer + * exists. The discard event is best-effort: with no agent polling it parks + * the durable session in discard_requested, which no resume path adopts; + * with an agent attached it triggers the normal discard finalization. + */ + function discardOrphanedSession(reason) { + const sessionId = currentSessionId; + if (!sessionId) return; + console.warn('[impeccable] Discarding orphaned session ' + sessionId + ': ' + reason); + sendEvent({ type: 'discard', id: sessionId, orphaned: true }).catch(() => {}); + markSessionHandled(); + cleanup({ instantChrome: true }); + showToast('The previous live session no longer matches the source file, so it was discarded. Pick an element to start fresh.', 6000); + } + /** * No-HMR fallback: fetch the raw source file from the live server, * parse it, extract the variant wrapper, and inject it into the live DOM. @@ -6205,6 +6221,25 @@ srcWrapper = doc.querySelector('[data-impeccable-variants="' + sessionId + '"]'); if (!srcWrapper) { console.warn('[impeccable] Variant wrapper not found in source file.'); + // A resumed cycling session whose wrapper is gone from source is an + // ORPHAN: the file was edited or regenerated out from under it, so + // no reload, HMR push, or server restart can ever complete it, and + // the frozen picker it leaves behind used to need a manual + // live-complete --discarded. Retry a few reads first (an agent + // rewrite or HMR patch may be mid-flight), then self-discard and + // hand the surface back to the picker. + if (opts.orphanDiscard && sessionId === currentSessionId) { + const attempt = opts._orphanAttempt || 0; + if (attempt < COMPLETED_SOURCE_FALLBACK_RETRIES) { + setTimeout(() => { + if (sessionId !== currentSessionId) return; + if (state !== 'GENERATING' && state !== 'CYCLING') return; + injectVariantsFromSource(filePath, sessionId, { ...opts, _orphanAttempt: attempt + 1 }); + }, COMPLETED_SOURCE_FALLBACK_RETRY_MS); + } else { + discardOrphanedSession('variant wrapper missing from source'); + } + } return; } @@ -8802,7 +8837,14 @@ void main() { ? currentPreviewFile : (currentSourceFile || currentPreviewFile); if (restoreFile) { - injectVariantsFromSource(restoreFile, currentSessionId); + // A restored CYCLING session promises variants already written into + // source; if they are not there (after retries), the session is an + // orphan and must self-discard instead of freezing the picker (#439). + // GENERATING restores make no such promise: deferred-wrapper flows + // legitimately have no wrapper in source until the agent's write lands. + injectVariantsFromSource(restoreFile, currentSessionId, { + orphanDiscard: savedState === 'CYCLING' && !isFrameworkComponentPreviewMode(currentPreviewMode), + }); return true; } diff --git a/skill/scripts/live-server.mjs b/skill/scripts/live-server.mjs index 62929ed90..4f0381fbc 100644 --- a/skill/scripts/live-server.mjs +++ b/skill/scripts/live-server.mjs @@ -1038,13 +1038,25 @@ function createRequestHandler({ detectScript, liveScriptParts }) { if (msg.type === 'exit') { cleanupSvelteComponentSessionsBeforeExit(); } + // An ORPHANED discard is the browser reporting that the session's + // wrapper no longer exists in source (edited or regenerated away). + // There is no cleanup for an agent to perform, and asking one to run + // the normal discard flow would just fail against the missing + // scaffolding, so the server terminalizes the session itself and the + // event stays out of the poll queue. + const orphanedDiscard = msg.type === 'discard' && msg.orphaned === true; + if (orphanedDiscard && state.sessionStore && msg.id) { + try { + state.sessionStore.appendEvent({ type: 'discarded', id: msg.id, orphaned: true }); + } catch { /* the discard_requested phase already left the resumable set */ } + } // `variant_mounted` is the happy path: it is journaled above so the // snapshot carries render truth, but there is nothing for the agent to // do about it, so it stays out of the poll queue and off the SSE bus. // `variant_mount_failed` is the opposite: the agent published something // the browser could not render, and only the agent can fix it, so it // goes to the queue as a first-class event. - if (msg.type !== 'checkpoint' && msg.type !== 'variant_mounted') { + if (msg.type !== 'checkpoint' && msg.type !== 'variant_mounted' && !orphanedDiscard) { enqueueEvent(msg); } res.writeHead(200, { 'Content-Type': 'application/json' }); diff --git a/tests/framework-fixtures/vite8-react-plain/fixture.json b/tests/framework-fixtures/vite8-react-plain/fixture.json index 69074a9f1..e2bda75de 100644 --- a/tests/framework-fixtures/vite8-react-plain/fixture.json +++ b/tests/framework-fixtures/vite8-react-plain/fixture.json @@ -1,23 +1,45 @@ { "name": "Vite 8 + React + plain CSS", "config": { - "files": ["index.html"], + "files": [ + "index.html" + ], "insertBefore": "", "commentSyntax": "html" }, - "sourceFiles": ["index.html", "src/App.jsx", "src/main.jsx", "src/styles.css", "vite.config.js"], + "sourceFiles": [ + "index.html", + "src/App.jsx", + "src/main.jsx", + "src/styles.css", + "vite.config.js" + ], "generatedFiles": [], "wrapCases": [ { "name": "wraps hero title in source JSX", - "args": { "classes": "hero-title", "tag": "h1" }, + "args": { + "classes": "hero-title", + "tag": "h1" + }, "expectedFile": "src/App.jsx" } ], "runtime": { "styling": "plain-css", - "install": ["npm", "install", "--no-audit", "--no-fund", "--loglevel=error"], - "devCommand": ["npx", "vite", "--host", "127.0.0.1"], + "install": [ + "npm", + "install", + "--no-audit", + "--no-fund", + "--loglevel=error" + ], + "devCommand": [ + "npx", + "vite", + "--host", + "127.0.0.1" + ], "readyPattern": "Local:\\s+https?://[^:]+:(\\d+)", "readyTimeoutMs": 120000, "probe": { @@ -33,7 +55,13 @@ "manualEditScenarios": [ { "name": "React headless manual Apply hard batch", - "element": { "selector": "main.page", "position": { "x": 4, "y": 4 } }, + "element": { + "selector": "main.page", + "position": { + "x": 4, + "y": 4 + } + }, "applyTimeoutMs": 300000, "refreshAfterApply": true, "expectApplyLoading": true, @@ -148,6 +176,9 @@ ], "expectedStashCount": 13 } - ] + ], + "orphanedWrapperScenario": { + "sourceFile": "src/App.jsx" + } } } diff --git a/tests/live-e2e.test.mjs b/tests/live-e2e.test.mjs index 24e0f0c12..65320db81 100644 --- a/tests/live-e2e.test.mjs +++ b/tests/live-e2e.test.mjs @@ -1058,6 +1058,72 @@ for (const { name, fixture } of fixtures) { }); } + if (shouldRunScenario('orphan') && fixture.runtime.orphanedWrapperScenario) { + it('self-discards an orphaned session when its wrapper is edited out of source', liveE2eTestOptions, async (t) => { + if (manualOnly || process.env.IMPECCABLE_E2E_MANUAL_SCENARIO) { + t.skip('manual scenario filter is active'); + return; + } + // Repro of issue #439: a cycling session is abandoned (no Accept or + // Discard), the wrapped region is edited out of source, and the page + // reloads. The resumed session used to freeze the picker forever; + // recovery required a manual live-complete --discarded. It must now + // self-discard and hand the surface back to the picker. + const agent = createFakeAgent(); + const session = await bootFixtureSession({ + name, + fixture, + browser, + agent, + wrapTarget: wrapTargetFromPickedElement, + log: (m) => t.diagnostic(m), + }); + const { page, appRoot, teardown } = session; + const cfg = fixture.runtime.orphanedWrapperScenario; + const pickSelector = fixture.runtime.pickSelector || 'h1.hero-title'; + try { + await waitForHandshake(page); + const sourceFile = join(appRoot, cfg.sourceFile); + const pristine = readFileSync(sourceFile, 'utf-8'); + + await pickElement(page, pickSelector, { position: fixture.runtime.pickPosition }); + await clickGo(page); + await waitForCyclingRobust(page, 3, { timeout: 60_000, log: (m) => t.diagnostic(m) }); + const saved = await readLiveSessionStorage(page); + assert.ok(saved?.id, 'cycling session persisted to local storage'); + + t.diagnostic('Restoring pristine source (simulated external edit that removes the wrapper)'); + writeFileSync(sourceFile, pristine); + await page.reload({ waitUntil: 'domcontentloaded' }); + await waitForHandshake(page); + + // Resume adopts the cycling session, the source read finds no + // wrapper, retries, then self-discards: local session cleared. + const deadline = Date.now() + 30_000; + for (;;) { + const current = await readLiveSessionStorage(page); + if (!current?.id) break; + if (Date.now() > deadline) throw new Error('orphaned session was never discarded'); + await new Promise((resolve) => setTimeout(resolve, 500)); + } + t.diagnostic('Orphaned session discarded; verifying durable phase + picker rearm'); + + const snapshotPath = join(appRoot, '.impeccable/live/sessions', `${saved.id}.snapshot.json`); + const snapshot = JSON.parse(readFileSync(snapshotPath, 'utf-8')); + assert.equal( + snapshot.phase, + 'discarded', + 'an orphaned discard is terminalized server-side without agent involvement', + ); + + // The regression that mattered: the picker must arm again. + await pickElement(page, pickSelector, { position: fixture.runtime.pickPosition }); + } finally { + await teardownAndResetBrowser(teardown); + } + }); + } + if (shouldRunScenario('manual') && Array.isArray(fixture.runtime.manualEditScenarios) && fixture.runtime.manualEditScenarios.length > 0) { const manualScenarioFilter = process.env.IMPECCABLE_E2E_MANUAL_SCENARIO || ''; for (const scenario of fixture.runtime.manualEditScenarios) {