diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index da026e255..c32101b12 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -6216,6 +6216,71 @@ return /\.[cm]?[jt]sx$/i.test(String(filePath || '')); } + function sourceHasSessionWrapper(text, sessionId) { + const src = String(text || ''); + return src.indexOf('data-impeccable-variants="' + sessionId + '"') !== -1 + || src.indexOf("data-impeccable-variants='" + sessionId + "'") !== -1 + || src.indexOf('impeccable-variants-start ' + sessionId) !== -1; + } + + /** + * Orphan probe for JSX targets (#439 + #454). An unmounted wrapper and a + * wrapper deleted from source look identical in the DOM, and only the second + * is an orphan, so the DOM alone cannot decide. #454 forbids parsing or + * injecting raw JSX; reading the file as plain text and matching the session + * marker honors that, because no DOM is ever built from what comes back. + * Marker present means the component is simply not mounted right now (a + * closed modal, another route) and the variant observer keeps waiting. + * Marker absent after the same retry budget the HTML path uses means the + * file was edited out from under the session, which no reload, HMR push, or + * server restart can repair, so the session self-discards and hands the + * surface back to the picker. + */ + function probeJsxWrapperForOrphan(filePath, sessionId, opts) { + const attempt = opts._orphanAttempt || 0; + const url = 'http://localhost:' + PORT + '/source?token=' + TOKEN + '&path=' + encodeURIComponent(filePath); + const stillActive = () => sessionId === currentSessionId && (state === 'GENERATING' || state === 'CYCLING'); + const retryLater = () => { + setTimeout(() => { + if (!stillActive()) return; + injectVariantsFromSource(filePath, sessionId, { ...opts, _orphanAttempt: attempt + 1 }); + }, COMPLETED_SOURCE_FALLBACK_RETRY_MS); + }; + // Discarding is durable (the session moves to the discarded phase and the + // picker replaces it), so it needs evidence that the wrapper is gone: a + // read that answers without the marker, or a 404 (the file itself was + // renamed or deleted). Either kind retries on the shared budget first. + // A read that fails for any other reason (the server briefly away, a + // transient fetch error) says nothing about the wrapper; after the budget + // the session is kept, the user told, and the next event retries. + const onNoWrapper = (reason) => { + if (!stillActive()) return; + if (attempt < COMPLETED_SOURCE_FALLBACK_RETRIES) { retryLater(); return; } + discardOrphanedSession(reason); + }; + const onUnreadable = (detail) => { + if (!stillActive()) return; + if (attempt < COMPLETED_SOURCE_FALLBACK_RETRIES) { retryLater(); return; } + console.warn('[impeccable] Could not read source to check the variant wrapper; keeping the session: ' + detail); + showToast('Could not read the source file to check this session; it stays open and is checked again on the next event.', 5500); + }; + fetch(url) + .then(r => { if (!r.ok) throw new Error('source read failed: ' + r.status); return r.text(); }) + .then(text => { + if (!stillActive()) return; + if (sourceHasSessionWrapper(text, sessionId)) return; + onNoWrapper('variant wrapper missing from source'); + }) + .catch(err => { + const detail = err && err.message ? err.message : 'fetch failed'; + if (/source read failed: 404$/.test(detail)) { + onNoWrapper('source file missing (404) while checking for the variant wrapper'); + return; + } + onUnreadable(detail); + }); + } + function completeSourceInjection(wrapper, sessionId, opts) { recoveryWaitingForAnchor = false; if (pendingVariantAnchorRetryObserver) { @@ -6326,14 +6391,7 @@ return; } if (opts.orphanDiscard && !liveWrapper && 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); - } + probeJsxWrapperForOrphan(filePath, sessionId, opts); } return; } diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index e93dbff86..0617806f8 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -555,7 +555,7 @@ describe('live-browser source contracts', () => { it('never DOMParser-injects JSX source (#454)', () => { const isJsxStart = SOURCE.indexOf('function isJsxSourceFile('); - const isJsxEnd = SOURCE.indexOf('function completeSourceInjection', isJsxStart); + const isJsxEnd = SOURCE.indexOf('function sourceHasSessionWrapper(', isJsxStart); const isJsxSourceFile = new Function( SOURCE.slice(isJsxStart, isJsxEnd) + '\nreturn isJsxSourceFile;', )(); @@ -580,7 +580,12 @@ describe('live-browser source contracts', () => { assert.doesNotMatch( jsxGate, /discardOrphanedSession/, - 'a missing JSX wrap must wait for mount, not discard as an orphan', + 'a missing JSX wrap must not discard from the DOM alone; only the source probe may decide', + ); + assert.match( + jsxGate, + /if \(opts\.orphanDiscard && !liveWrapper && sessionId === currentSessionId\) \{\s*probeJsxWrapperForOrphan\(filePath, sessionId, opts\);/, + 'a resumed CYCLING session with no mounted JSX wrapper must run the source orphan probe (#439)', ); assert.match( jsxGate, @@ -606,6 +611,64 @@ describe('live-browser source contracts', () => { ); }); + it('self-discards a JSX session whose wrapper left the source file (#439)', () => { + const probeStart = SOURCE.indexOf('function probeJsxWrapperForOrphan('); + assert.ok(probeStart !== -1, 'the JSX orphan probe must exist'); + const probeEnd = SOURCE.indexOf('function completeSourceInjection', probeStart); + const probe = SOURCE.slice(probeStart, probeEnd); + + // #454 stands: the probe reads the file as text and never builds a DOM + // from it, so raw JSX can never reach the page through this path. + for (const forbidden of ['DOMParser', 'parseFromString', 'replaceChild', 'innerHTML']) { + assert.ok(!probe.includes(forbidden), 'the orphan probe must not ' + forbidden + ' JSX source'); + } + assert.match(probe, /\.then\(r => \{ if \(!r\.ok\) throw new Error\('source read failed: ' \+ r\.status\); return r\.text\(\); \}\)/); + assert.match( + probe, + /if \(sourceHasSessionWrapper\(text, sessionId\)\) return;/, + 'a wrapper still in source is an unmounted component, not an orphan', + ); + assert.match( + probe, + /const onNoWrapper = \(reason\) => \{[\s\S]*?if \(attempt < COMPLETED_SOURCE_FALLBACK_RETRIES\) \{ retryLater\(\); return; \}\s*discardOrphanedSession\(reason\);/, + 'the probe must exhaust the shared retry budget before discarding', + ); + assert.match( + probe, + /onNoWrapper\('variant wrapper missing from source'\)/, + 'a read without the marker retries on the budget, then discards', + ); + // A read that cannot answer must not strand the session (no empty catch), + // and only evidence that the wrapper is gone may discard: a 404 (the file + // renamed or deleted) counts, a transient failure does not. + assert.doesNotMatch(probe, /\.catch\(\(\) => \{\}\)/, 'the probe must not swallow source read failures'); + assert.match( + probe, + /source read failed: 404\$\/\.test\(detail\)\) \{\s*onNoWrapper\('source file missing \(404\)/, + 'a 404 is evidence the file is gone: retry on the budget, then discard', + ); + assert.match( + probe, + /const onUnreadable = \(detail\) => \{[\s\S]*?retryLater\(\); return; \}[\s\S]*?showToast\(/, + 'a transient failure retries on the budget and then keeps the session, telling the user', + ); + assert.doesNotMatch( + probe.slice(probe.indexOf('const onUnreadable')), + /discardOrphanedSession/, + 'a transient failure must never discard a session', + ); + + const matchStart = SOURCE.indexOf('function sourceHasSessionWrapper('); + const sourceHasSessionWrapper = new Function( + SOURCE.slice(matchStart, probeStart) + '\nreturn sourceHasSessionWrapper;', + )(); + assert.equal(sourceHasSessionWrapper('
', 'ab12cd34'), true); + assert.equal(sourceHasSessionWrapper("
", 'ab12cd34'), true); + assert.equal(sourceHasSessionWrapper('{/* impeccable-variants-start ab12cd34 */}', 'ab12cd34'), true); + assert.equal(sourceHasSessionWrapper('
', 'ab12cd34'), false); + assert.equal(sourceHasSessionWrapper('', 'ab12cd34'), false); + }); + 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;/);