Commit Graph
5 Commits
Author SHA1 Message Date
Paul BakausandClaude Opus 4.7 8660d3aa22 fix(live): wrap shape-of-output bugs from second Bugbot review
Two more Cursor Bugbot findings on commit 11dfad81:

1. `filterByText` short-text returned the wrong sentinel value.
   When the trimmed snippet was shorter than 8 chars, the function
   returned `candidates.slice()` (all candidates). The caller then
   sees `filtered.length > 1` and fires `element_ambiguous` — exactly
   the opposite of the documented short-text fallback ("caller falls
   back to first-match," which corresponds to `filtered.length === 0`).
   So any picker event with a short textContent on a page with multiple
   matching siblings spuriously errored.
   Fix: return `[]` for short text, matching the JSDoc.

2. `endLine` in the wrap output was wrong for multi-line picked elements.
   `wrapperLines.length` counts ARRAY elements, but one element is a
   `\n`-joined multi-line string (originalIndented). The actual
   wrapper-region row count is `wrapperLines.length + (originalLines.length
   - 1)`. Reporting `endLine = startLine + wrapperLines.length` placed
   the boundary inside the wrapper for any multi-line pick, giving
   downstream agents an incorrect range.
   Fix: add the originalLines offset (matching what `insertLine` already
   does after the prior commit).

Test coverage:
- `short --text falls back to first-match instead of erroneously firing
  element_ambiguous` covers fix #1.
- `returns endLine that includes the multi-line original content offset`
  covers fix #2 by wrapping a 5-line <section> in a real HTML file and
  asserting the reported endLine points at the variants-end marker (and
  the next line is </main>, proving no rows were missed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-28 16:52:02 -07:00
Paul BakausandClaude Opus 4.7 9ec904302b fix(live): textContent disambiguation handles missing inter-element whitespace
While driving the new live loop end-to-end against the repeated-aside
fixture, --text disambiguation silently fell back to first-match instead
of landing on the picked card.

Root cause: `el.textContent` concatenates child text nodes without
inserting whitespace, so `<h1>Hero Two</h1><p>Second card body copy.</p>`
reads as "Hero TwoSecond card body copy." — but the source has whitespace
between </h1> and <p>. The single-space normalization on both sides
missed the join boundary; substring comparison failed; filterByText
returned [] and the caller fell through to first-match.

Fix: filterByText now compares both single-space AND no-whitespace
normalizations on each side, accepting the candidate if EITHER matches.
Bumped the minimum-target-length threshold from 6 to 8 to compensate
for the slightly looser comparison.

Plus two doc clarifications surfaced during the same session:

- live.md now warns that variant CSS using bare `:scope { ... }` styles
  the variant wrapper div, not the picked element. Always use a
  descendant combinator (`:scope > .card`, `:scope .hero-title`, etc.) —
  the fake test agent's CSS is the canonical template.
- live.md documents the agent-side abort path. Aborting an in-flight
  generate via `live-accept --discard` only mutates source — the browser
  bar stays in GENERATING forever. Use `live-poll --reply EVENT_ID error
  "msg"` instead so the browser receives the error SSE and resets.

Test coverage:
- New unit test in live-wrap.test.mjs covering the textContent-without-
  inter-element-whitespace shape (three identical <aside> branches each
  with <h1> + <p>, picks the second by --text).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-28 15:03:49 -07:00
Paul BakausandClaude Opus 4.7 54d9f05ea5 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>
2026-04-28 13:23:20 -07:00
Paul BakausandClaude Opus 4.7 a4832adf2f fix(live-wrap): JSX/TSX correctness — multi-line tags, className, tag narrowing
Five related bugs that surfaced in a real Next.js App Router project
(EAC) all rooted in live-wrap.mjs treating source as line-anchored HTML:

1. findElement matched on raw substring anywhere, so it landed on a
   className continuation line of a multi-line JSX tag whose class
   happened to collide with a later target. The wrong tag got wrapped
   (really, its attribute line got wrapped, producing broken JSX).
2. findClosingLine's opener regex required whitespace or `>` after the
   tag name, so a bare `<section\n  className="..."\n>` opener was
   unrecognised; it returned `start` silently, capturing only one line.
3. buildSearchQueries only emitted `class="..."`, missing React's
   `className="..."`. The full-combo query never fired in JSX, so
   search silently degraded to single-class substring matching.
4. Wrapper output used `style="display: contents"` unconditionally,
   which is invalid JSX (type error in strict setups, parser hazard
   in production transforms).
5. --tag was ignored during the primary class search. Ambiguous class
   hits inside the wrong element type weren't filtered out.

## Fixes

- New OPENER_RE `/<([A-Za-z][A-Za-z0-9]*)(?=[\s/>]|$)/` recognises
  tag openers at end-of-line too.
- New findOpenerLine(lines, matchLine, tag): walks up to 10 lines
  backward to the enclosing opener when the match lands on a
  continuation line. Aborts the walk if it hits a different tag.
- findElement now iterates all matches (not just the first), takes
  a tag parameter, and routes through findOpenerLine; wrapCli passes
  --tag through.
- buildSearchQueries emits both `class="..."` and `className="..."`
  for multi-class queries, and both `<tag class="..."` /
  `<tag className="..."` for tag+class combos.
- Wrapper builder emits `style={{ display: "contents" }}` when
  commentSyntax is JSX and `style="display: contents"` otherwise.
- findClosingLine uses the same OPENER_RE so its tag-name extraction
  works on multi-line openers too.

## Tests

Five new regression tests in tests/live-wrap.test.mjs, all failing
before the fix, all passing after:

- wraps the correct <section> when a class collides with a multi-line
  tag elsewhere
- emits JSX-safe style attribute ({{ }}) in .tsx files
- finds elements via className= (React) when the exact class combo is
  unique there
- respects --tag to reject matches inside the wrong element type
- findClosingLine recognises an opener line where the tag sits at
  end-of-line (multi-line JSX)

31/31 in tests/live-wrap.test.mjs and 54/54 in
tests/framework-fixtures.test.mjs pass.

Credit: precise bug report from the other agent in the EAC session
made diagnosis and test design straightforward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 00:19:32 -07:00
Paul BakausandClaude Opus 4.6 455fe39155 Add 41 tests for live-wrap and live-server, add auto-execute guards
Tests:
- tests/live-wrap.test.mjs (26 tests): unit tests for buildSearchQueries,
  findElement, findClosingLine, detectCommentSyntax (20 pure function
  tests) + integration tests for the full wrapCli on HTML and JSX
  fixtures with temp dirs (6 tests covering wrapping, ID/class lookup,
  error handling, content preservation).

- tests/live-server.test.mjs (15 tests): integration tests that start a
  real server on port 8499, then test /health, /live.js, /detect.js,
  /poll (timeout + auth), /events POST (validation + auth), browser→agent
  event flow (POST event → poll receives it), agent→browser SSE flow
  (POST reply → SSE stream delivers it), /source (read, path traversal
  rejection, auth, 404).

Also:
- Added auto-execute guards to live-wrap.mjs and live-poll.mjs so they
  work when run directly with `node live-wrap.mjs ...` (needed for both
  skill instructions and integration tests).
- Exported buildSearchQueries, findElement, findClosingLine,
  detectCommentSyntax from live-wrap.mjs for unit testing.
- Updated package.json test script to include the new test files.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-12 19:35:26 -07:00