Skip to content

fix(live): scan JSX comment variant markers; never whole-file-inject raw JSX (#454) #455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kticoder wants to merge 2 commits into
base: main
Choose a base branch
from
+79 −8
Draft

fix(live): scan JSX comment variant markers; never whole-file-inject raw JSX (#454) #455

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
832b374
fix(live): find JSX comment variant markers; never inject whole raw J…
kadirilgin321 Jul 30, 2026
88d501c
fix(live): adopt live DOM wrapper for JSX resumes; synthesize wrapper…
kadirilgin321 Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 79 additions & 8 deletions skill/scripts/live-browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6203,21 +6203,89 @@
return;
}
rememberSessionFileMeta({ file: filePath });
// JSX sources: if the framework already re-rendered the wrapper (Vite full
// reload / Fast Refresh), adopt the live DOM tree instead of replacing it
// with a DOMParser clone — a clone renders JSX {expressions} as literal
// text and detaches React from its own nodes (#454).
const jsxDomWrapper = /\.[cm]?[jt]sx$/i.test(String(filePath || ''))
? document.querySelector('[data-impeccable-variants="' + sessionId + '"]')
: null;
if (jsxDomWrapper) {
const wrapper = jsxDomWrapper;
recoveryWaitingForAnchor = false;
if (pendingVariantAnchorRetryObserver) {
pendingVariantAnchorRetryObserver.disconnect();
pendingVariantAnchorRetryObserver = null;
}
const variants = wrapper.querySelectorAll('[data-impeccable-variant]:not([data-impeccable-variant="original"])');
arrivedVariants = variants.length;
expectedVariants = parseInt(wrapper.dataset.impeccableVariantCount || arrivedVariants);
if (arrivedVariants <= 0) {
if (state === 'GENERATING' && !opts.generationCompleted) return;
recoverEmptyCycling('jsx-dom-adopt-empty');
return;

@cursor cursor Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty JSX adopt skips retries

High Severity

When a JSX live wrapper is present but still has zero variants after generationCompleted, the adopt path calls recoverEmptyCycling immediately. The source-fallback path below retries several times for the same stale-scaffold case, so a wrap that HMR’d before variants can tear the session down instead of waiting for Fast Refresh.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 88d501c. Configure here.

Comment thread
greptile-apps[bot] marked this conversation as resolved.
}
const previousVisibleVariant = currentSessionId === sessionId ? visibleVariant : 0;
const saved = loadSession();
const savedVisibleVariant = saved && saved.id === sessionId ? saved.visible : 0;
visibleVariant = previousVisibleVariant > 0 && previousVisibleVariant <= arrivedVariants
? previousVisibleVariant
: (savedVisibleVariant > 0 && savedVisibleVariant <= arrivedVariants ? savedVisibleVariant : 1);
showVariantInDOM(sessionId, visibleVariant);
selectedElement = pickVariantContent(wrapper, visibleVariant) || wrapper.parentElement;
setLiveState('CYCLING');
hideShaderOverlay();
showOrUpdateCyclingBar();
disableInlineEdit();
refreshParamsPanel();
positionBar();
saveSession();
if (parameterGenerationState === 'loading') completeParameterPublication();
console.log('[impeccable] Adopted ' + arrivedVariants + ' variants from live DOM (JSX).');
return;
}
const url = 'http://localhost:' + PORT + '/source?token=' + TOKEN + '&path=' + encodeURIComponent(filePath);
fetch(url)
.then(r => { if (!r.ok) throw new Error(r.status); return r.text(); })
.then(html => {
const parser = new DOMParser();
let srcWrapper = null;

// Full-file parse works for HTML/JSX; Astro/Vue sources need marker extraction.
const startMark = '<!-- impeccable-variants-start ' + sessionId + ' -->';
const endMark = '<!-- impeccable-variants-end ' + sessionId + ' -->';
const startIdx = html.indexOf(startMark);
const endIdx = html.indexOf(endMark);
const block = startIdx !== -1 && endIdx !== -1 && endIdx > startIdx
? html.slice(startIdx + startMark.length, endIdx).trim()
: html;
// Marker comments differ by source syntax: HTML/Astro/Vue wrappers use
// <!-- ... -->, while JSX/TSX wrappers must use {/* ... */} (an HTML
// comment is invalid inside a JSX element tree), so scan for both.
// Never fall back to whole-file injection for JSX: raw JSX renders
// {expressions} and marker text as literal page content (#454).
const markerPairs = [
{ open: '<!--', close: '-->' },
{ open: '{/*', close: '*/}' },
];
Comment on lines +6259 to +6262

@greptile-apps greptile-apps Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Source-fallback branches lack regression coverage

The new HTML/JSX marker extraction, markerless JSX/TSX skip, markerless HTML fallback, and JSX-comment normalization paths have no behavioral regression coverage. The focused suite remains green after each behavior is removed, so a future change can restore raw JSX injection or corrupt fallback previews without detection. Add focused inputs for both marker syntaxes, markerless .jsx and .tsx files, markerless HTML, and JSX comments before parsing.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Implementation diff without matching test changes

  • Compared the parent and PR revisions for the changed browser source and focused test file; it shows the new marker/fallback/comment-removal code and no test-file change, confirming the coverage gap.

Focused live-browser source suite passing on the PR revision

  • Ran `node --test tests/live-browser-source.test.mjs` in `/home/user/repo`; all 8 tests passed, confirming the existing narrow suite is green but does not establish coverage of the new branches.

Focused suite passes after each changed fallback behavior is removed

  • Created temporary copies outside the repository, removed JSX marker extraction, markerless JSX skip, markerless HTML fallback, and JSX comment stripping one at a time, and ran the focused suite; every mutation passed 8/8, proving these behaviors lack regression detection.

Parent revision leaves a JSX comment in normalized source

  • Executed the focused normalization harness against `832b3742^`; JSX comment text remained in the output, establishing the pre-change behavior.

PR revision removes a JSX comment while preserving HTML input

  • Executed the focused normalization harness against the PR revision; JSX comment text was removed and equivalent HTML was unchanged, demonstrating the changed behavior.

Search results for direct regression coverage of changed fallback paths

  • Searched all test files for the new helper, marker-pair parsing, markerless JSX warning, and JSX marker syntax; results contain only existing generic source-contract references, not representative behavioral cases.

View artifacts

T-Rex Ran code and verified through T-Rex

Fix in Codex Fix in Claude Code

let block = null;
let jsxPairMatched = false;
for (const pair of markerPairs) {
const startMark = pair.open + ' impeccable-variants-start ' + sessionId + ' ' + pair.close;
const endMark = pair.open + ' impeccable-variants-end ' + sessionId + ' ' + pair.close;
const startIdx = html.indexOf(startMark);
const endIdx = html.indexOf(endMark);
if (startIdx !== -1 && endIdx > startIdx) {
block = html.slice(startIdx + startMark.length, endIdx).trim();
jsxPairMatched = pair.open === '{/*';
break;
}
Comment thread
cursor[bot] marked this conversation as resolved.
}
if (jsxPairMatched && block) {
// JSX wrappers carry their markers INSIDE the wrapper div, so the
// extracted block lacks the wrapper element; synthesize it (#454).
block = '<div data-impeccable-variants="' + sessionId + '" style="display: contents">' + block + '</div>';
}

@cursor cursor Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synthesized wrapper drops attributes

Medium Severity

When JSX markers match, the rebuilt wrapper only sets data-impeccable-variants and style. The real open tag outside those markers also carries data-impeccable-variant-count and, for insert sessions, data-impeccable-mode="insert", so source-fallback injection loses planned count and insert-mode behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 88d501c. Configure here.

if (block == null) {
if (/\.[cm]?[jt]sx$/i.test(String(filePath || ''))) {
console.warn('[impeccable] JSX source has no variant markers; skipping raw-source injection (#454).');
block = '';
} else {
block = html;
}
}

@cursor cursor Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fetch path still replaces React

High Severity

The new JSX adopt guard only runs synchronously before fetch. After the source download, an existingWrapper that appeared via HMR is still replaceChild’d with a DOMParser clone—the same React detach / literal {expression} failure #454 describes.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 88d501c. Configure here.

@cursor cursor Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSX no-marker skips recovery

Medium Severity

For .jsx/.tsx with no markers, block is set to '' so the wrapper lookup fails. That branch only discards when orphanDiscard is set; the generationCompleted done-fallback caller just returns, leaving the tab stuck in GENERATING instead of recovering.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 88d501c. Configure here.

const doc = parser.parseFromString(normalizeSourceFallbackBlock(block, filePath), 'text/html');
srcWrapper = doc.querySelector('[data-impeccable-variants="' + sessionId + '"]');
if (!srcWrapper) {
Expand Down Expand Up @@ -6341,6 +6409,9 @@
function normalizeSourceFallbackBlock(block, filePath) {
if (!/\.[cm]?[jt]sx$/i.test(String(filePath || ''))) return block;
return String(block)
// JSX comments ({/* ... */}) are not HTML comments; strip them so they
// don't render as literal text in the DOMParser preview (#454).
.replace(/\{\s*\/\*[\s\S]*?\*\/\s*\}/g, '')
.replace(
/<style\b([^>]*)>\s*\{\s*`([\s\S]*?)`\s*\}\s*<\/style>/g,
(_match, attrs, css) => '<style' + attrs + '>' + css + '</style>',
Expand Down