fix(live): land valid TSX through wrap → preview → accept → carbonize

Closes #114.

Three orthogonal bugs that surfaced together when live mode picked an
element inside a Vite React/TSX component with sibling branches:

1. JSX wrapper insertion produced invalid TSX
   - Replacing a single picked JSX child with [comment, <div>, comment]
     yields three adjacent siblings, which oxc rejects with "Adjacent
     JSX elements must be wrapped in an enclosing tag."
   - A Fragment `<></>` solves the adjacency case but breaks
     `cloneElement`-using parents (Radix `asChild`, Headless UI, etc.)
     with "Invalid prop supplied to React.Fragment."
   - Fix: keep the wrapper `<div data-impeccable-variants="ID">` as the
     single JSX-slot child and tuck both marker comments INSIDE it.
     accept/discard now expands its replacement range to include the
     wrapper's `<div>` open/close lines via div-depth tracking.

2. carbonize produced nested template literals in TSX `<style>`
   - extractCss captured `{` / `` `} `` lines from the agent's existing
     `<style>{`…`}</style>` template, then handleAccept re-wrapped with
     another pair, producing `<style>{`{`@scope…`}`}</style>` which oxc
     rejects with "Expected `}` but found `@`".
   - Fix: extractCss now strips a leading `{` and trailing `` `} ``
     wherever they appear in the captured content (own line OR attached
     to the first/last CSS line), so re-wrapping always yields exactly
     one `{` ` … ` `}` pair.

3. Ambiguous source matching for repeated JSX branches
   - `findElement` returned the first substring match. Multiple
     `<aside className="card">` siblings all matched the same query, so
     wrap silently landed on the first regardless of which one the user
     picked.
   - Fix: live-wrap accepts `--text TEXT` (the picked element's
     textContent), collects ALL candidates via `findAllElements`, and
     narrows by a tag-stripped, JSX-expression-stripped substring match.
     Returns `element_ambiguous + candidates[]` when multiple branches
     match equally; falls back to first-match when source uses dynamic
     content (`<h1>{title}</h1>`) so existing flows aren't broken.
   - The fake e2e agent now forwards `event.element.textContent` to
     wrap, and live.md tells the agent to do the same.

Test coverage:
- New `vite8-react-tsx-repeated-aside` e2e fixture: three identical
  `<aside>` branches, picks the second card's <h1>, runs the full
  wrap → Go → cycle → accept → carbonize cycle on a real Vite + TSX
  dev server, asserts that Hero One and Hero Three survive untouched
  (proving wrap landed on the correct branch).
- Six new unit tests across live-wrap.test.mjs and live-accept.test.mjs
  covering the Fragment-replacement design, both leading/trailing
  template-literal placements, --text disambiguation, the dynamic-
  content fallback, and the element_ambiguous error shape.
- New `runtime.assertSourceContains` fixture hook so other regression
  fixtures can assert sibling-branch survivability cheaply.

All 186 unit + static-fixture tests pass; all 21 live e2e fixtures
(20 prior + new TSX) pass with no console errors.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-04-28 13:23:20 -07:00
co-authored by Claude Opus 4.7
parent 638af20566
commit 54d9f05ea5
52 changed files with 4089 additions and 300 deletions
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<title>Vite 8 + TSX repeated branches Fixture</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/main.tsx"></script>
</body>
</html>
@@ -0,0 +1,22 @@
{
"name": "vite8-react-tsx-repeated-aside-fixture",
"private": true,
"version": "0.0.0",
"type": "module",
"scripts": {
"dev": "vite --host 127.0.0.1",
"build": "vite build",
"preview": "vite preview"
},
"dependencies": {
"react": "^19.0.0",
"react-dom": "^19.0.0"
},
"devDependencies": {
"@types/react": "^19.0.0",
"@types/react-dom": "^19.0.0",
"@vitejs/plugin-react": "^6.0.0",
"typescript": "^5.6.0",
"vite": "^8.0.0"
}
}
@@ -0,0 +1,18 @@
export default function App() {
return (
<main className="page">
<aside data-testid="card-1" className="card">
<h1 className="hero-title">Hero One</h1>
<p className="hero-hook">First card body copy.</p>
</aside>
<aside data-testid="card-2" className="card">
<h1 className="hero-title">Hero Two</h1>
<p className="hero-hook">Second card body copy.</p>
</aside>
<aside data-testid="card-3" className="card">
<h1 className="hero-title">Hero Three</h1>
<p className="hero-hook">Third card body copy.</p>
</aside>
</main>
);
}
@@ -0,0 +1,10 @@
import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import App from './App.tsx';
import './styles.css';
createRoot(document.getElementById('root')!).render(
<StrictMode>
<App />
</StrictMode>,
);
@@ -0,0 +1,5 @@
body { margin: 0; font-family: system-ui, sans-serif; }
.page { padding: 2rem; display: grid; gap: 1rem; }
.card { padding: 1rem; border: 1px solid #ddd; border-radius: 0.5rem; }
.hero-title { font-size: 1.5rem; margin: 0 0 0.5rem; }
.hero-hook { color: #555; margin: 0; }
@@ -0,0 +1,17 @@
{
"compilerOptions": {
"target": "ES2022",
"lib": ["ES2022", "DOM", "DOM.Iterable"],
"module": "ESNext",
"moduleResolution": "bundler",
"jsx": "react-jsx",
"strict": true,
"skipLibCheck": true,
"isolatedModules": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": true,
"resolveJsonModule": true,
"noEmit": true
},
"include": ["src"]
}
@@ -0,0 +1,7 @@
import { defineConfig } from 'vite';
import react from '@vitejs/plugin-react';
export default defineConfig({
plugins: [react()],
server: { host: '127.0.0.1', strictPort: false },
});
@@ -0,0 +1,30 @@
{
"name": "Vite 8 + React TSX with repeated <aside> branches",
"config": {
"files": ["index.html"],
"insertBefore": "</body>",
"commentSyntax": "html"
},
"sourceFiles": ["index.html", "src/App.tsx", "src/main.tsx", "src/styles.css", "vite.config.ts", "tsconfig.json"],
"generatedFiles": [],
"wrapCases": [
{
"name": "wraps the picked aside (second branch) inside a Fragment so TSX stays valid",
"args": { "classes": "hero-title", "tag": "h1" },
"expectedFile": "src/App.tsx"
}
],
"runtime": {
"styling": "plain-css",
"install": ["npm", "install", "--no-audit", "--no-fund", "--loglevel=error"],
"devCommand": ["npx", "vite", "--host", "127.0.0.1"],
"readyPattern": "Local:\\s+https?://[^:]+:(\\d+)",
"readyTimeoutMs": 120000,
"pickSelector": "[data-testid='card-2'] h1.hero-title",
"assertSourceContains": ["Hero One", "Hero Three", "First card body copy.", "Third card body copy."],
"probe": {
"expectLiveInit": true,
"expectConsoleClean": true
}
}
}
@@ -0,0 +1,4 @@
node_modules/
dist/
.vite/
package-lock.json
+83
View File
@@ -123,6 +123,89 @@ describe('live-accept — style-element edge cases', () => {
assert.ok(after.includes('variant one'), 'variant 1 content kept');
});
// Regression: the agent writes JSX <style>{`…`}</style> and live-accept's
// extractCss used to capture the `{` … `` ` ``}` template-literal punctuation
// as CSS content. handleAccept then re-wrapped with another `{` …
// `` ` ``}`, producing nested template literals (`<style>{`{`@scope…`}`}`)
// that oxc rejects with "Expected `}` but found `@`". extractCss must
// strip the JSX wrap regardless of where the agent placed it.
it('carbonize does not double-wrap when the variants block uses JSX template literals on their own lines', () => {
const tsx = `export default function App() {\n` +
` return (\n` +
` <main>\n` +
` <>\n` +
` {/* impeccable-variants-start TPL */}\n` +
` <div data-impeccable-variants="TPL" data-impeccable-variant-count="2" style={{ display: 'contents' }}>\n` +
` <div data-impeccable-variant="original"><p className="hook">orig</p></div>\n` +
` <style data-impeccable-css="TPL">\n` +
" {`\n" +
` @scope ([data-impeccable-variant="1"]) { .hook { color: red; } }\n` +
` @scope ([data-impeccable-variant="2"]) { .hook { color: green; } }\n` +
" `}\n" +
` </style>\n` +
` <div data-impeccable-variant="1"><p className="hook">variant one</p></div>\n` +
` <div data-impeccable-variant="2" style={{ display: 'none' }}><p className="hook">variant two</p></div>\n` +
` </div>\n` +
` {/* impeccable-variants-end TPL */}\n` +
` </>\n` +
` </main>\n` +
` );\n` +
`}\n`;
writeFileSync(join(tmp, 'App.tsx'), tsx);
const result = runAccept(tmp, ['--id', 'TPL', '--variant', '1']);
assert.equal(result.handled, true, `accept should succeed: ${JSON.stringify(result)}`);
const after = readFileSync(join(tmp, 'App.tsx'), 'utf-8');
// Exactly one `{` opener after the carbonized <style ...> tag — not two.
const carbonStyleMatch = after.match(/<style data-impeccable-css="TPL">([\s\S]*?)<\/style>/);
assert.ok(carbonStyleMatch, 'carbonize <style> block present');
const inner = carbonStyleMatch[1];
// Inner must open with one `{` ... and end with one ` `` ... — no nesting.
const openCount = (inner.match(/\{`/g) || []).length;
const closeCount = (inner.match(/`\}/g) || []).length;
assert.equal(openCount, 1, `expected exactly one {\` opener, got ${openCount}`);
assert.equal(closeCount, 1, `expected exactly one \`} closer, got ${closeCount}`);
// CSS content survived intact.
assert.ok(inner.includes('@scope ([data-impeccable-variant="1"])'), 'variant-1 scope kept');
});
// Same shape, but the agent put `{`` and ``\`}` attached to first/last CSS
// lines instead of on dedicated lines. Tests the inline-strip branch.
it('carbonize does not double-wrap when JSX template-literal punctuation hugs the CSS lines', () => {
const tsx = `export default function App() {\n` +
` return (\n` +
` <main>\n` +
` <>\n` +
` {/* impeccable-variants-start INLINE */}\n` +
` <div data-impeccable-variants="INLINE" data-impeccable-variant-count="2" style={{ display: 'contents' }}>\n` +
` <div data-impeccable-variant="original"><p className="hook">orig</p></div>\n` +
` <style data-impeccable-css="INLINE">\n` +
" {`@scope ([data-impeccable-variant=\"1\"]) { .hook { color: red; } }\n" +
" @scope ([data-impeccable-variant=\"2\"]) { .hook { color: green; } }`}\n" +
` </style>\n` +
` <div data-impeccable-variant="1"><p className="hook">variant one</p></div>\n` +
` <div data-impeccable-variant="2" style={{ display: 'none' }}><p className="hook">variant two</p></div>\n` +
` </div>\n` +
` {/* impeccable-variants-end INLINE */}\n` +
` </>\n` +
` </main>\n` +
` );\n` +
`}\n`;
writeFileSync(join(tmp, 'App.tsx'), tsx);
const result = runAccept(tmp, ['--id', 'INLINE', '--variant', '1']);
assert.equal(result.handled, true, `accept should succeed: ${JSON.stringify(result)}`);
const after = readFileSync(join(tmp, 'App.tsx'), 'utf-8');
const inner = after.match(/<style data-impeccable-css="INLINE">([\s\S]*?)<\/style>/)[1];
const openCount = (inner.match(/\{`/g) || []).length;
const closeCount = (inner.match(/`\}/g) || []).length;
assert.equal(openCount, 1, `expected one {\` opener, got ${openCount}`);
assert.equal(closeCount, 1, `expected one \`} closer, got ${closeCount}`);
assert.ok(inner.includes('@scope ([data-impeccable-variant="1"])'), 'variant-1 scope kept');
});
// Discard must restore the original element after a self-closing <style />,
// proving extractOriginal also survives the style pattern.
it('discard restores the original element after a JSX self-closing <style />', () => {
+13
View File
@@ -269,6 +269,19 @@ for (const { name, fixture } of fixtures) {
'accepted h1 survives with hero-title class',
);
// Optional fixture hook: assert that arbitrary strings survive the
// wrap → accept → carbonize cycle. Used by repeated-branch fixtures
// to prove wrap disambiguated correctly — sibling branches the test
// didn't pick should be untouched.
if (Array.isArray(fixture.runtime.assertSourceContains)) {
for (const needle of fixture.runtime.assertSourceContains) {
assert.ok(
final.includes(needle),
`source still contains ${JSON.stringify(needle)} after accept (sibling branch must not be rewritten)`,
);
}
}
// 9. DOM-side: at least one matching element, none inside any wrapper.
await page.waitForFunction(
(sel) => {
+7 -1
View File
@@ -323,12 +323,17 @@ export async function runAgentLoop({
// sessions: the agent must derive the selector from the picked
// element on the fly).
const target = typeof wrapTarget === 'function' ? wrapTarget(event) : wrapTarget;
// Pull textContent from the picker event so wrap can disambiguate
// when sibling elements share classes/tag (issue #114). Fixtures can
// still override by including `text` in their wrapTarget.
const text = target.text ?? (event.element?.textContent || '').trim();
const wrapInfo = await runWrap({
tmp,
scriptsDir,
id: event.id,
count: event.count,
...target,
text,
});
log(`wrapped: ${wrapInfo.file} insertLine=${wrapInfo.insertLine}`);
@@ -426,11 +431,12 @@ export async function runAgentLoop({
}
}
async function runWrap({ tmp, scriptsDir, id, count, classes, tag, elementId }) {
async function runWrap({ tmp, scriptsDir, id, count, classes, tag, elementId, text }) {
const args = [path.join(scriptsDir, 'live-wrap.mjs'), '--id', id, '--count', String(count)];
if (elementId) args.push('--element-id', elementId);
if (classes) args.push('--classes', classes);
if (tag) args.push('--tag', tag);
if (text) args.push('--text', text);
const { stdout } = await execFileP(process.execPath, args, { cwd: tmp });
const last = stdout.trim().split('\n').filter(Boolean).pop();
return JSON.parse(last);
+157
View File
@@ -441,6 +441,163 @@ describe('live-wrap — JSX / TSX correctness', () => {
assert.ok(!inside.includes('extra-class'), 'decoy not wrapped');
});
it('keeps the JSX wrapper single-rooted by tucking marker comments INSIDE the outer <div>', () => {
// Replacing one JSX element with [comment, <div>, comment] yields three
// adjacent siblings, which Vite's oxc rejects with "Adjacent JSX
// elements must be wrapped in an enclosing tag." A Fragment `<></>`
// would solve adjacency but breaks `cloneElement`-using parents (Radix
// `asChild` etc.) with "Invalid prop supplied to React.Fragment". The
// wrap script's answer is to tuck the markers INSIDE the outer wrapper
// <div>, which IS the single JSX-slot child.
const tsx = `export default function App() {
return (
<main>
<section className="frag-target">
<h1>Hi</h1>
</section>
</main>
);
}`;
writeFileSync(join(tmp, 'App.tsx'), tsx);
execSync(
`node source/skills/impeccable/scripts/live-wrap.mjs --id frag1 --count 3 --classes "frag-target" --tag "section" --file "${join(tmp, 'App.tsx')}"`,
{ cwd: process.cwd(), encoding: 'utf-8' }
);
const modified = readFileSync(join(tmp, 'App.tsx'), 'utf-8');
// No JSX Fragment wrappers (those break asChild/cloneElement parents).
assert.ok(!modified.includes('<>'), 'no Fragment opener emitted');
assert.ok(!modified.includes('</>'), 'no Fragment closer emitted');
// The outer wrapper <div data-impeccable-variants="..."> appears BEFORE
// both marker comments — markers are tucked inside.
const wrapperIdx = modified.indexOf('data-impeccable-variants="frag1"');
const startMarkerIdx = modified.indexOf('impeccable-variants-start frag1');
const endMarkerIdx = modified.indexOf('impeccable-variants-end frag1');
assert.ok(wrapperIdx !== -1 && startMarkerIdx !== -1 && endMarkerIdx !== -1, 'all markers present');
assert.ok(wrapperIdx < startMarkerIdx, 'wrapper opens before start-marker comment');
assert.ok(endMarkerIdx > startMarkerIdx, 'end marker follows start marker');
});
it('HTML wrapper keeps marker comments OUTSIDE the wrapper <div> (existing layout)', () => {
const html = '<main>\n <section class="html-frag">Hi</section>\n</main>';
writeFileSync(join(tmp, 'page.html'), html);
execSync(
`node source/skills/impeccable/scripts/live-wrap.mjs --id htmlFrag --count 3 --classes "html-frag" --tag "section" --file "${join(tmp, 'page.html')}"`,
{ cwd: process.cwd(), encoding: 'utf-8' }
);
const modified = readFileSync(join(tmp, 'page.html'), 'utf-8');
const wrapperIdx = modified.indexOf('data-impeccable-variants="htmlFrag"');
const startMarkerIdx = modified.indexOf('impeccable-variants-start htmlFrag');
assert.ok(startMarkerIdx < wrapperIdx, 'HTML start marker precedes wrapper div');
});
it('disambiguates repeated JSX siblings via --text and lands on the correct branch', () => {
// Three <aside className="card"> elements with identical classes/tag —
// the user picked the SECOND one. Without --text, first-match wraps the
// first. With --text matching the picked element's textContent, wrap
// narrows to the right branch.
const tsx = `export default function Page() {
return (
<main>
<aside className="card">
<h2>Alpha card</h2>
<p>First in the list.</p>
</aside>
<aside className="card">
<h2>Beta card</h2>
<p>Second in the list.</p>
</aside>
<aside className="card">
<h2>Gamma card</h2>
<p>Third in the list.</p>
</aside>
</main>
);
}`;
writeFileSync(join(tmp, 'Page.tsx'), tsx);
execSync(
`node source/skills/impeccable/scripts/live-wrap.mjs --id repeat1 --count 3 --classes "card" --tag "aside" --text "Beta card Second in the list." --file "${join(tmp, 'Page.tsx')}"`,
{ cwd: process.cwd(), encoding: 'utf-8' }
);
const modified = readFileSync(join(tmp, 'Page.tsx'), 'utf-8');
const originalMatch = modified.match(/data-impeccable-variant="original"[\s\S]*?<\/div>/);
assert.ok(originalMatch, 'original wrapper present');
const inside = originalMatch[0];
assert.ok(inside.includes('Beta card'), 'wrapped the Beta card (the picked one)');
assert.ok(!inside.includes('Alpha card'), 'did not wrap Alpha');
assert.ok(!inside.includes('Gamma card'), 'did not wrap Gamma');
});
it('falls back to first-match when --text is not literally present in source (e.g. {title})', () => {
// textContent the browser sends is the rendered text, but the source uses
// a JSX expression. No candidate's source body contains the literal
// textContent — wrap should keep the first-match behavior rather than
// refusing, because failing here would be more annoying than wrong.
const tsx = `export default function Cards({ items }) {
return (
<main>
{items.map(item => (
<aside key={item.id} className="card">
<h2>{item.title}</h2>
</aside>
))}
</main>
);
}`;
writeFileSync(join(tmp, 'Cards.tsx'), tsx);
// Run with --text that won't show up in source verbatim.
execSync(
`node source/skills/impeccable/scripts/live-wrap.mjs --id dyn1 --count 3 --classes "card" --tag "aside" --text "Beta card body text" --file "${join(tmp, 'Cards.tsx')}"`,
{ cwd: process.cwd(), encoding: 'utf-8' }
);
const modified = readFileSync(join(tmp, 'Cards.tsx'), 'utf-8');
assert.ok(modified.includes('data-impeccable-variants="dyn1"'), 'wrapped (first-match fallback)');
});
it('errors with element_ambiguous when --text matches multiple identical branches', () => {
// Two <aside className="card"> with truly identical body text. --text
// can't pick a winner — wrap should refuse rather than silently land.
const tsx = `export default function Page() {
return (
<main>
<aside className="card">
<h2>Same headline</h2>
<p>Identical body copy.</p>
</aside>
<aside className="card">
<h2>Same headline</h2>
<p>Identical body copy.</p>
</aside>
</main>
);
}`;
writeFileSync(join(tmp, 'Dup.tsx'), tsx);
let errPayload;
try {
execSync(
`node source/skills/impeccable/scripts/live-wrap.mjs --id dup1 --count 3 --classes "card" --tag "aside" --text "Same headline Identical body copy." --file "${join(tmp, 'Dup.tsx')}"`,
{ cwd: process.cwd(), encoding: 'utf-8', stdio: 'pipe' }
);
assert.fail('Should have exited with error');
} catch (err) {
assert.ok(err.status !== 0, 'non-zero exit');
errPayload = JSON.parse(err.stderr.toString().trim());
}
assert.equal(errPayload.error, 'element_ambiguous');
assert.equal(errPayload.fallback, 'agent-driven');
assert.ok(Array.isArray(errPayload.candidates) && errPayload.candidates.length === 2,
'two candidate locations reported');
});
it('respects --tag to reject matches inside the wrong element type', () => {
// Two elements, both containing the class. The <div> comes first in source
// order; a tag-agnostic search would wrap it. With --tag section, the