mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-16 16:16:32 +03:00
Self-discard orphaned JSX live sessions again (#716)
* Self-discard orphaned JSX live sessions again (#715) #694 stopped the source fallback from fetching and DOMParser-injecting raw JSX, which was painting {expressions} and comment markers into the page. The JSX gate it put in front of the fetch decided everything from the live DOM alone, and an unmounted wrapper looks exactly like a wrapper that was deleted from the file, so it treated both as "wait for mount": the orphan branch counted down its retry budget and then fell out of the function with no terminal action. A resumed CYCLING session whose region had been edited out of source therefore never reached discardOrphanedSession, the durable snapshot stayed out of the discarded phase, and the picker stayed frozen, which is the #439 regression the live-e2e scenario pins. The fix restores the decision without restoring the parse: probeJsxWrapperForOrphan reads the file as plain text and matches the session marker, so no DOM is ever built from JSX. Marker present means the component is simply not mounted and the observer keeps waiting; marker absent after the same retry budget the HTML path uses means the file moved on, and the session self-discards. AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY * Orphan probe: a source read that fails also retries, then discards Review on #716 (Greptile, Copilot): the probe's empty catch swallowed a failed /source read, so a session whose file had been renamed or deleted (404), or that hit a transient fetch failure, neither retried nor reached a terminal action, which is the frozen-picker failure the probe exists to end. A read that cannot answer now shares the retry budget with a read that answers without the marker, and after the budget the session is discarded with a reason that names the failure. Unit test pins that the probe has no empty catch and that the failure path discards. AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY * Orphan probe: only evidence that the wrapper is gone may discard Review on #716 (Greptile, second pass): after the previous change a transient /source failure that outlasted the 3.6 s retry budget discarded a valid session, and a discard is durable. Now a read that answers without the marker, or a 404 (the file renamed or deleted), retries on the budget and then discards; any other failure retries on the budget and then keeps the session, warns, and tells the user it is checked again on the next event. The unit test pins both halves. AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
fcc271c1cb
commit
3f815865ab
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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('<div data-impeccable-variants="ab12cd34">', 'ab12cd34'), true);
|
||||
assert.equal(sourceHasSessionWrapper("<div data-impeccable-variants='ab12cd34'>", 'ab12cd34'), true);
|
||||
assert.equal(sourceHasSessionWrapper('{/* impeccable-variants-start ab12cd34 */}', 'ab12cd34'), true);
|
||||
assert.equal(sourceHasSessionWrapper('<div data-impeccable-variants="99887766">', '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;/);
|
||||
|
||||
Reference in New Issue
Block a user