Fix inset-order detection, the unlocked artifact discard, and stray boolean flags

Three of the four open review findings. The fourth is declined below.

- The inset-stripe scan only matched layers starting with `inset`, but the keyword
  is order-independent: `box-shadow: 4px 0 0 var(--brand-accent) inset` paints the
  same stripe and was silently missed. Strip the keyword wherever it sits, but
  only as a standalone token, so a color like var(--inset-accent) is not mangled
  into `var(-- -accent)` and quietly reclassified as neutral. The fixture now
  covers both orders plus that token, and a trailing-inset neutral still passes.
- The source-artifact discard deleted the preview without the source lock, unlike
  every other discard path. Take the lock. Narrower than reported, though: the
  server journals `discard_requested` as a fenced phase before live-accept runs
  and the publisher checks it three times, so a publish could never land on a
  discarded session. What this actually prevents is deleting the artifact under a
  publisher mid-critical-section, turning a clean stale_generation_epoch into an
  ENOENT crash.
- benchmark-live-providers.mjs still compared `--headed` and `--skip-cleanup-control`
  against a boolean sentinel, so the `=true` spelling silently did nothing. My
  gap: I introduced boolFlag and converted benchmark-live.mjs but not this one.
  skipCleanupControl is now read once rather than twice, so the two call sites
  cannot drift.

Declined: tightening the selector guard that skips `active` / `current` /
`selected` tokens. It does cause false negatives on names like `.selected-feature`,
but the rule's contract makes selection and focus indicators its one exception,
and `.active-tab` / `.current-step` / `.selected-row` are syntactically identical
to `.selected-feature`. No regex separates them, so tightening the guard trades
missed stripes for false positives on exactly the case the rule exempts. The
conservative skip is the intended behavior.

Prepared with AI assistance under maintainer direction.

Co-Authored-By: Claude <noreply@anthropic.com>

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]);

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 });
   }

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,

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', () => {

tests/fixtures/antipatterns/astro-inset-shadow-stripe.astro

@@ -14,6 +14,9 @@ const title = 'Astro inset shadow stripe regression';
     <article data-case="Named Red Edge"><h3>Named Red Edge</h3></article>
     <article data-case="Chromatic Rgb Edge"><h3>Chromatic Rgb Edge</h3></article>
     <article data-case="Chromatic Oklch Edge"><h3>Chromatic Oklch Edge</h3></article>
+    <article data-case="Trailing Inset Edge"><h3>Trailing Inset Edge</h3></article>
+    <article data-case="Trailing Inset Token Edge"><h3>Trailing Inset Token Edge</h3></article>
+    <article data-case="Inset Named Token Edge"><h3>Inset Named Token Edge</h3></article>
   </section>
   <section aria-labelledby="should-pass">
     <h2 id="should-pass">Should pass</h2>
@@ -31,6 +34,7 @@ const title = 'Astro inset shadow stripe regression';
     <article data-case="Black Rgb Edge"><h3>Black Rgb Edge</h3></article>
     <article data-case="Shorthand Neutral Hex Edge"><h3>Shorthand Neutral Hex Edge</h3></article>
     <article data-case="Commented Out Edge"><h3>Commented Out Edge</h3></article>
+    <article data-case="Trailing Inset Neutral Edge"><h3>Trailing Inset Neutral Edge</h3></article>
   </section>
 </main>
 
@@ -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); }
   */

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', () => {