diff --git a/cli/engine/engines/regex/detect-text.mjs b/cli/engine/engines/regex/detect-text.mjs index af37aa17e..ff4db4228 100644 --- a/cli/engine/engines/regex/detect-text.mjs +++ b/cli/engine/engines/regex/detect-text.mjs @@ -416,8 +416,16 @@ function scanInsetStripeCss(rawContent, filePath, lineOffset = 0) { const declaration = match[2].match(/(?:^|;)\s*box-shadow\s*:\s*([^;]+)/i); if (!declaration || !/\binset\b/i.test(declaration[1])) continue; - for (const layer of declaration[1].split(/,(?![^(]*\))/)) { - const shadow = layer.trim().match(/\binset\s+(-?\d*\.?\d+)(px)?\s+(-?\d*\.?\d+)(px)?\s+(-?\d*\.?\d+)(px)?(?:\s+(-?\d*\.?\d+)(px)?)?\s+(.+)$/i); + for (const rawLayer of declaration[1].split(/,(?![^(]*\))/)) { + const layer = rawLayer.trim(); + // `inset` is order-independent inside a box-shadow layer: `inset 4px 0 0 red` + // and `4px 0 0 red inset` paint the same stripe, and requiring it first + // silently missed the second spelling. Strip it only as a standalone + // keyword, so a color token such as var(--inset-accent) survives intact; + // an unchanged layer had no inset keyword and is not our shape. + const body = layer.replace(/(^|\s)inset(?=\s|$)/i, '$1').trim(); + if (body === layer) continue; + const shadow = body.match(/^(-?\d*\.?\d+)(px)?\s+(-?\d*\.?\d+)(px)?\s+(-?\d*\.?\d+)(px)?(?:\s+(-?\d*\.?\d+)(px)?)?\s+(.+)$/i); if (!shadow) continue; const x = Number(shadow[1]); const y = Number(shadow[3]); diff --git a/scripts/benchmark-live-providers.mjs b/scripts/benchmark-live-providers.mjs index 4b99e8956..7d6810019 100644 --- a/scripts/benchmark-live-providers.mjs +++ b/scripts/benchmark-live-providers.mjs @@ -9,7 +9,7 @@ import { fileURLToPath } from 'node:url'; import { bootFixtureSession, FIXTURES_DIR } from '../tests/live-e2e/session.mjs'; import { createFakeAgent } from '../tests/live-e2e/agent.mjs'; -import { parseArgs, positiveIntFlag } from './lib/cli-args.mjs'; +import { boolFlag, parseArgs, positiveIntFlag } from './lib/cli-args.mjs'; import { clickAccept, clickGo, @@ -72,13 +72,14 @@ if (args.requireAll && available.length !== selection.length) { } if (available.length === 0) throw new Error('no provider API keys found; use --dry-run to validate without network calls'); -const needsBrowser = args.pipeline === 'e2e' || args.skipCleanupControl !== true; +const skipCleanupControl = boolFlag(args.skipCleanupControl); +const needsBrowser = args.pipeline === 'e2e' || !skipCleanupControl; const { chromium } = needsBrowser ? await import('playwright') : { chromium: null }; -const browser = chromium ? await chromium.launch({ headless: args.headed !== true }) : null; +const browser = chromium ? await chromium.launch({ headless: !boolFlag(args.headed) }) : null; const results = []; let cleanupControl = { passed: true, skipped: true }; try { - if (args.skipCleanupControl !== true) { + if (!skipCleanupControl) { process.stderr.write('[live-provider-bench] running provider-independent Accept/cleanup control\n'); cleanupControl = await runCleanupControl({ browser, fixture }); } diff --git a/skill/scripts/live-accept.mjs b/skill/scripts/live-accept.mjs index 8bd2e7507..08883c90d 100644 --- a/skill/scripts/live-accept.mjs +++ b/skill/scripts/live-accept.mjs @@ -137,9 +137,27 @@ Output (JSON): if (sourceArtifactManifest) { if (isDiscard) { - removeSourceArtifactSession(id, process.cwd()); + // Take the source lock like every other discard path. The journalled + // discard already fences publication, so this cannot admit a write to a + // discarded session; what it prevents is deleting the preview out from + // under a publisher mid-critical-section, which turns its clean + // stale_generation_epoch into an ENOENT crash. + let result; + try { + result = withSourceLockSync( + sourceArtifactManifest.sourcePath, + 'discard:' + id, + () => { + removeSourceArtifactSession(id, process.cwd()); + return { handled: true }; + }, + { waitMs: ACCEPT_LOCK_WAIT_MS }, + ); + } catch (err) { + result = { handled: false, error: err.message }; + } emitResult({ - handled: true, + ...result, file: sourceArtifactManifest.sourceFile, sourceFile: sourceArtifactManifest.sourceFile, previewMode: sourceArtifactManifest.previewMode, diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs index 7b0902097..a6d9f9892 100644 --- a/tests/detect-antipatterns-fixtures.test.mjs +++ b/tests/detect-antipatterns-fixtures.test.mjs @@ -28,6 +28,11 @@ describe('detectText - Astro structural CSS fixtures', () => { 'Named Red Edge', 'Chromatic Rgb Edge', 'Chromatic Oklch Edge', + // `inset` may follow the offsets/color. Requiring it first missed the same + // stripe written the other legal way. + 'Trailing Inset Edge', + 'Trailing Inset Token Edge', + 'Inset Named Token Edge', ]; const SHOULD_PASS = [ 'Neutral Shadow Token', @@ -49,6 +54,8 @@ describe('detectText - Astro structural CSS fixtures', () => { 'Shorthand Neutral Hex Edge', // Commented-out CSS is not a live rule. 'Commented Out Edge', + // Trailing `inset` still respects the neutral-color exemption. + 'Trailing Inset Neutral Edge', ]; it('Astro style blocks flag unresolved chromatic inset stripes only', () => { diff --git a/tests/fixtures/antipatterns/astro-inset-shadow-stripe.astro b/tests/fixtures/antipatterns/astro-inset-shadow-stripe.astro index 023855c3e..a93c6e396 100644 --- a/tests/fixtures/antipatterns/astro-inset-shadow-stripe.astro +++ b/tests/fixtures/antipatterns/astro-inset-shadow-stripe.astro @@ -14,6 +14,9 @@ const title = 'Astro inset shadow stripe regression';

Named Red Edge

Chromatic Rgb Edge

Chromatic Oklch Edge

+

Trailing Inset Edge

+

Trailing Inset Token Edge

+

Inset Named Token Edge

Should pass

@@ -31,6 +34,7 @@ const title = 'Astro inset shadow stripe regression';

Black Rgb Edge

Shorthand Neutral Hex Edge

Commented Out Edge

+

Trailing Inset Neutral Edge

@@ -60,6 +64,13 @@ const title = 'Astro inset shadow stripe regression'; [data-case="Black Rgb Edge"] { box-shadow: inset 4px 0 0 rgb(0, 0, 0); } [data-case="Shorthand Neutral Hex Edge"] { box-shadow: inset 4px 0 0 #1118; } + /* `inset` is order-independent per spec; these paint the same stripe as above. */ + [data-case="Trailing Inset Edge"] { box-shadow: 4px 0 0 #6366f1 inset; } + [data-case="Trailing Inset Token Edge"] { box-shadow: 4px 0 0 var(--brand-accent) inset; } + /* The keyword must only be stripped standalone: this token merely contains it. */ + [data-case="Inset Named Token Edge"] { box-shadow: inset 4px 0 0 var(--inset-accent); } + [data-case="Trailing Inset Neutral Edge"] { box-shadow: 4px 0 0 #000 inset; } + /* Commented-out rules are not live CSS. [data-case="Commented Out Edge"] { box-shadow: inset 4px 0 0 var(--brand-accent); } */ diff --git a/tests/live-accept.test.mjs b/tests/live-accept.test.mjs index 82df1e730..85eb7d40f 100644 --- a/tests/live-accept.test.mjs +++ b/tests/live-accept.test.mjs @@ -5,12 +5,13 @@ import { describe, it, beforeEach, afterEach } from 'node:test'; import assert from 'node:assert/strict'; -import { existsSync, mkdtempSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, realpathSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; import { execFileSync, spawnSync } from 'node:child_process'; import { scaffoldSourceArtifactSession } from '../skill/scripts/live/source-artifact.mjs'; +import { sourceLockPath } from '../skill/scripts/live/source-lock.mjs'; const __dirname = dirname(fileURLToPath(import.meta.url)); const ACCEPT = resolve(__dirname, '..', 'skill/scripts/live-accept.mjs'); @@ -131,6 +132,33 @@ describe('live-accept — isolated source artifacts', () => { assert.equal(readFileSync(join(tmp, 'page.html'), 'utf-8'), original); assert.equal(existsSync(join(tmp, session.sessionDir)), false); }); + + // Every other discard path (Vue, Svelte, plain wrapper) takes the source lock. + // This one deleted the preview bare, so it could pull the artifact out from + // under an in-flight publisher instead of serializing behind it. + it('serializes the discard behind a publisher holding the source lock', () => { + const { original, session } = scaffold('isolatedlocked'); + // realpath: on macOS mkdtemp hands back /var/... while the child process's + // cwd resolves to /private/var/..., and the lock digest hashes the absolute + // path. Hash the same string the child will. + const realTmp = realpathSync(tmp); + const lockPath = sourceLockPath(join(realTmp, 'page.html'), realTmp); + mkdirSync(dirname(lockPath), { recursive: true }); + // A live holder: process.pid is alive, so the lock is not stale. + writeFileSync(lockPath, JSON.stringify({ + owner: 'generation:isolatedlocked:1', token: 'other', pid: process.pid, at: Date.now(), + }) + '\n'); + + const result = runAccept(tmp, ['--id', 'isolatedlocked', '--discard']); + assert.equal(result.handled, false, JSON.stringify(result)); + assert.equal(result.error, 'source_locked'); + assert.equal( + existsSync(join(tmp, session.sessionDir)), + true, + 'the preview must survive: deleting it under the publisher is the race', + ); + assert.equal(readFileSync(join(tmp, 'page.html'), 'utf-8'), original); + }); }); describe('live-accept — style-element edge cases', () => {