diff --git a/skill/scripts/live/svelte-component.mjs b/skill/scripts/live/svelte-component.mjs index 350e66b95..4993453a7 100644 --- a/skill/scripts/live/svelte-component.mjs +++ b/skill/scripts/live/svelte-component.mjs @@ -207,16 +207,18 @@ export function scaffoldSvelteComponentSession({ fs.mkdirSync(dir, { recursive: true }); const contract = analysis.contract; - const seededCss = extractMatchingSourceCss( + const seeded = extractMatchingSourceCss( safeReadSource(path.resolve(cwd, sourceFile)), originalMarkup, ); + const seededCss = seeded.css; // The preview compiles in isolation, so NONE of these source rules applied // to what the user approved. Accept enforces that preview truth: any of // them the variant does not re-declare is superseded and removed, instead // of re-attaching to the accepted markup through kept class names (the - // ".decisions grid grabs the new board" failure). - const seededSelectors = [...collectAllSelectors(seededCss)]; + // ".decisions grid grabs the new board" failure). Only the CLASS-matched + // selectors are candidates; tag rules style shared route elements. + const seededSelectors = [...seeded.supersedable]; const manifest = { id, @@ -266,15 +268,26 @@ function safeReadSource(filePath) { try { return fs.readFileSync(filePath, 'utf-8'); } catch { return ''; } } +function escapeSelectorToken(token) { + return String(token).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + /** * Seed variant stubs with the source component's rules that already style the * selected markup, so variants start from the real cascade (a detached * preview inherits none of the route's compile-scoped CSS) instead of * reimplementing it blind. + * + * Returns { css, supersedable }. `css` is every matching rule (class OR tag + * matched). `supersedable` holds only the CLASS-matched selectors: those are + * the accept-time removal candidates. Tag selectors (h1, a, p) style shared + * elements across the whole route, so they seed the preview but are never + * candidates for removal. */ export function extractMatchingSourceCss(routeSource, originalMarkup) { + const empty = { css: '', supersedable: new Set() }; const styleMatch = String(routeSource || '').match(/]*>([\s\S]*?)<\/style\s*>/i); - if (!styleMatch) return ''; + if (!styleMatch) return empty; const classNames = new Set(); const classRe = /class\s*=\s*(["'])(.*?)\1/g; let m; @@ -284,17 +297,35 @@ export function extractMatchingSourceCss(routeSource, originalMarkup) { const tagRe = /<([a-z][a-z0-9-]*)/gi; const tags = new Set(); while ((m = tagRe.exec(originalMarkup))) tags.add(m[1].toLowerCase()); - if (classNames.size === 0 && tags.size === 0) return ''; + if (classNames.size === 0 && tags.size === 0) return empty; - const selectorMatches = (prelude) => splitSelectorList(prelude).some((selector) => { - for (const cls of classNames) if (selector.includes(`.${cls}`)) return true; - return false; - }); + // Token-boundary matching, never substring: `.btn` must not match + // `.btn-primary`, and `.stage` must not match `.stages`. A substring hit + // seeds a rule that never styled the pick, and a falsely seeded selector + // becomes an accept-time DELETION of a hand-written rule. + const classRes = [...classNames].map((cls) => new RegExp('\\.' + escapeSelectorToken(cls) + '(?![A-Za-z0-9_-])')); + const tagRes = [...tags].map((tag) => new RegExp('(^|[\\s>+~,(])' + escapeSelectorToken(tag) + '(?![A-Za-z0-9_-])', 'i')); + const classMatches = (selector) => classRes.some((re) => re.test(selector)); + const tagMatches = (selector) => tagRes.some((re) => re.test(selector)); + + const supersedable = new Set(); + const ruleMatches = (prelude) => { + let matched = false; + for (const selector of splitSelectorList(prelude)) { + if (classMatches(selector)) { + matched = true; + supersedable.add(normalizeSelector(selector)); + } else if (tagMatches(selector)) { + matched = true; + } + } + return matched; + }; const pick = (nodes) => { const kept = []; for (const node of nodes) { - if (node.type === 'rule' && selectorMatches(node.prelude)) kept.push(node); + if (node.type === 'rule' && ruleMatches(node.prelude)) kept.push(node); else if (node.type === 'at' && node.children) { const children = pick(node.children); if (children.length) kept.push({ ...node, children }); @@ -302,7 +333,7 @@ export function extractMatchingSourceCss(routeSource, originalMarkup) { } return kept; }; - return serializeNodes(pick(parseStylesheet(styleMatch[1]))); + return { css: serializeNodes(pick(parseStylesheet(styleMatch[1]))), supersedable }; } function buildVariantStubV2(variantNum, markupWithProps, contract, seededCss) { @@ -713,10 +744,37 @@ export function inlineSvelteComponentAccept(manifest, variantNum, paramValues = // them. Any seeded selector the variant did not re-declare is superseded; // left in place it re-attaches through kept class names (the accepted root // keeps its original classes) and re-layouts markup it no longer owns. + // + // Removal is bounded by ownership: a selector whose classes are still used + // by route markup OUTSIDE the replaced region does not belong to the pick + // alone, and removing it would strip styling from markup this accept never + // touched. Keeping it risks a visible re-attachment quirk on the accepted + // region; deleting it breaks the rest of the route. Keep it. + const outsideMarkup = [...sourceLines.slice(0, start), ...sourceLines.slice(end + 1)] + .join('\n') + .replace(/]*>[\s\S]*?<\/style\s*>/gi, ''); + const outsideClasses = new Set(); + { + const attrRe = /class\s*=\s*(["'])(.*?)\1/g; + let cm; + while ((cm = attrRe.exec(outsideMarkup))) { + for (const cls of cm[2].split(/\s+/)) if (cls && !cls.includes('{')) outsideClasses.add(cls); + } + const directiveRe = /class:([A-Za-z0-9_-]+)/g; + while ((cm = directiveRe.exec(outsideMarkup))) outsideClasses.add(cm[1]); + } + const usedOutsideReplacedRegion = (selector) => { + const classTokenRe = /\.([A-Za-z0-9_-]+)/g; + let tm; + while ((tm = classTokenRe.exec(selector))) { + if (outsideClasses.has(tm[1])) return true; + } + return false; + }; const incomingSelectors = collectAllSelectors(bakedCss); const superseded = (manifest.seededSelectors || []) .map((selector) => normalizeSelector(selector)) - .filter((selector) => selector && !incomingSelectors.has(selector)); + .filter((selector) => selector && !incomingSelectors.has(selector) && !usedOutsideReplacedRegion(selector)); if (superseded.length > 0) { const scrubbed = removeSelectorsFromSvelteSource(finalText, new Set(superseded)); finalText = scrubbed.text; diff --git a/tests/live-svelte-component-accept.test.mjs b/tests/live-svelte-component-accept.test.mjs index 21156a245..c0c47988c 100644 --- a/tests/live-svelte-component-accept.test.mjs +++ b/tests/live-svelte-component-accept.test.mjs @@ -226,11 +226,43 @@ describe('svelte component scaffold + accept pipeline', () => { }); it('extractMatchingSourceCss picks only rules that style the selection', () => { - const css = extractMatchingSourceCss(ROUTE_SOURCE, '
  1. x
'); + const { css } = extractMatchingSourceCss(ROUTE_SOURCE, '
  1. x
'); assert.match(css, /\.pit-board/); assert.match(css, /\.stage/); assert.doesNotMatch(css, /\.footer/); }); + + it('class matching is token-bounded, never substring', () => { + // The field hazard: a falsely seeded selector becomes an accept-time + // DELETION of a hand-written rule the pick never used. + const route = ``; + const { css, supersedable } = extractMatchingSourceCss(route, ''); + assert.match(css, /\.btn \{/); + assert.match(css, /\.stage \{/); + assert.doesNotMatch(css, /btn-primary/, '.btn must not seed .btn-primary'); + assert.doesNotMatch(css, /\.stages/, '.stage must not seed .stages'); + assert.deepEqual([...supersedable].sort(), ['.btn', '.stage']); + }); + + it('tag rules seed the preview but are never supersedable', () => { + const route = ``; + const { css, supersedable } = extractMatchingSourceCss(route, '

Title

'); + assert.match(css, /h1 \{ font-size/, 'bare tag rules that style the pick are seeded'); + assert.match(css, /h1\.hero/, 'class rules still seed'); + assert.doesNotMatch(css, /^p \{/m, 'unrelated tags are not seeded'); + assert.doesNotMatch(css, /\.sidebar/); + assert.deepEqual([...supersedable], ['h1.hero'], 'only class-matched selectors may be removed on accept'); + }); }); describe('review regressions: preview-truth supersession (the Pitch mangle)', () => { @@ -421,3 +453,88 @@ describe('review regressions: publish-time compile gate', () => { } }); }); + +describe('review regressions: shared-class supersession guard', () => { + const SHARED_SOURCE = ` + +
+
Intro copy stays here.
+
    + {#each items as item} +
  • {item.name}
    • + {/each} +
+
+ + +`; + + it('keeps a superseded selector whose class is still used outside the replaced region', () => { + const tmp4 = realpathSync(mkdtempSync(join(tmpdir(), 'impeccable-shared-class-'))); + try { + mkdirSync(join(tmp4, 'node_modules'), { recursive: true }); + try { + symlinkSync(join(REPO_NODE_MODULES, 'svelte'), join(tmp4, 'node_modules', 'svelte'), 'dir'); + } catch { + cpSync(join(REPO_NODE_MODULES, 'svelte'), join(tmp4, 'node_modules', 'svelte'), { recursive: true }); + } + write(tmp4, 'package.json', JSON.stringify({ name: 'app' })); + write(tmp4, 'src/lib/Shared.svelte', SHARED_SOURCE); + + // Pick the ') + 1; + const originalLines = lines.slice(startLine - 1, endLine); + + const session = scaffoldSvelteComponentSession({ + id: 'shared01', + count: 1, + sourceFile: 'src/lib/Shared.svelte', + sourceStartLine: startLine, + sourceEndLine: endLine, + originalLines, + cwd: tmp4, + }); + assert.equal(session.fallback, undefined, session.reason); + assert.equal(session.manifest.seededSelectors.includes('.card'), true, 'the pick uses .card, so it seeds'); + + // The variant re-declares .list but NOT .card. + write(tmp4, join(session.componentDir, 'v1.svelte'), ` + + + + +`); + const manifest = findSvelteComponentManifest('shared01', tmp4); + const result = inlineSvelteComponentAccept(manifest, 1, null, tmp4); + assert.equal(result.handled, true, result.error); + const out = readFileSync(join(tmp4, 'src/lib/Shared.svelte'), 'utf-8'); + + // .card is shared with the intro div outside the replaced region: + // removing it would strip styling from markup this accept never + // touched, so it must survive despite not being re-declared. + assert.match(out, /\.card \{ border: 1px solid #999; border-radius: 8px; \}/); + assert.equal(result.css.superseded.includes('.card'), false); + // The re-declared .list took the variant's shape. + assert.match(out, /\.list \{ display: flex/); + } finally { + rmSync(tmp4, { recursive: true, force: true }); + } + }); +});