diff --git a/scripts/lib/cli-args.mjs b/scripts/lib/cli-args.mjs new file mode 100644 index 000000000..8e0a58efd --- /dev/null +++ b/scripts/lib/cli-args.mjs @@ -0,0 +1,87 @@ +/** + * One argv parser for the Live benchmark / judging scripts. + * + * These scripts had four subtly different hand-rolled parsers, and the gaps + * failed silently rather than loudly: a parser without the `argv[i + 1]` + * lookahead turned `--iterations 20` into `iterations: true` and benchmarked + * the default 5 runs; a parser without kebab→camel mapping turned + * `--median-target=0.4` into a key nothing read, so the comparison ran against + * the default threshold. Both produce a clean-looking report of the wrong thing. + * + * Supported forms, per flag: + * --flag → true + * --flag=value → 'value' + * --flag value → 'value' (unless `value` itself starts with `--`) + * + * Keys are camel-cased, so `--simulated-tail-ms` and `--simulatedTailMs` both + * land on `simulatedTailMs`. + */ +export function parseArgs(argv) { + const out = {}; + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (!arg.startsWith('--')) continue; + const body = arg.slice(2); + if (!body) continue; + const equals = body.indexOf('='); + if (equals !== -1) { + out[toCamel(body.slice(0, equals))] = body.slice(equals + 1); + continue; + } + const next = argv[index + 1]; + if (next !== undefined && !next.startsWith('--')) { + out[toCamel(body)] = next; + index += 1; + } else { + out[toCamel(body)] = true; + } + } + return out; +} + +export function toCamel(value) { + return String(value).replace(/-([a-z0-9])/gi, (_, char) => char.toUpperCase()); +} + +/** + * Read a boolean flag. `--headed` and `--headed=true` must mean the same thing; + * comparing the raw value against `true` silently ignores the second form. + */ +export function boolFlag(value, fallback = false) { + if (value === undefined) return fallback; + if (typeof value === 'boolean') return value; + const normalized = String(value).trim().toLowerCase(); + if (['', 'true', '1', 'yes', 'on'].includes(normalized)) return true; + if (['false', '0', 'no', 'off'].includes(normalized)) return false; + return fallback; +} + +/** + * Parse a positive integer flag, falling back when absent. Throws on a value + * that was clearly meant as a number but isn't one, so `--iterations abc` + * fails instead of quietly benchmarking the default. + */ +export function positiveIntFlag(value, fallback) { + if (value === undefined || value === true) return fallback; + const parsed = Number.parseInt(String(value), 10); + if (!Number.isFinite(parsed) || parsed <= 0 || String(parsed) !== String(value).trim()) { + throw new Error(`expected a positive integer, got: ${value}`); + } + return parsed; +} + +/** + * Resolve a flag that must be one of a fixed set. + * + * A silent `x === 'known' ? 'known' : fallback` is the trap this replaces: the + * private evals Live runner passes `--agent=codex`, which fell through to the + * canned fake agent and produced a clean-looking report of a deterministic stub + * labelled as a real harness run. An unrecognized value is a mistake, not a + * request for the default. + */ +export function resolveEnum(value, allowed, fallback, flagName) { + if (value === undefined || value === true) return fallback; + const normalized = String(value).trim().toLowerCase(); + if (allowed.includes(normalized)) return normalized; + throw new Error(`${flagName} must be one of ${allowed.join(', ')}; got: ${value}`); +} diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 38c6b2470..b47bccb01 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -54,6 +54,7 @@ export const SUITES = { runner: 'node', files: [ 'tests/ci-test-plan.test.mjs', + 'tests/cli-args.test.mjs', 'tests/context.test.mjs', 'tests/context-signals.test.mjs', 'tests/critique-storage.test.mjs', @@ -134,16 +135,19 @@ export const SUITES = { 'tests/live-e2e-steer-agent.test.mjs', 'tests/live-e2e/agent-insert.test.mjs', 'tests/live-event-validation.test.mjs', + 'tests/live-generation-preflight.test.mjs', 'tests/live-inject.test.mjs', 'tests/live-insert.test.mjs', 'tests/live-insert-ui.test.mjs', 'tests/live-manual-edits-buffer.test.mjs', 'tests/live-poll.test.mjs', + 'tests/live-poll-lanes.test.mjs', 'tests/live-poll-stream.test.mjs', 'tests/live-recovery-commands.test.mjs', 'tests/live-reference.test.mjs', 'tests/live-server.test.mjs', 'tests/live-session-store.test.mjs', + 'tests/live-source-lock.test.mjs', 'tests/live-target-context.test.mjs', 'tests/live-wrap.test.mjs', 'tests/live-wrap-buffer-aware.test.mjs', diff --git a/skill/reference/live.md b/skill/reference/live.md index c084d6d21..00cb8e52d 100644 --- a/skill/reference/live.md +++ b/skill/reference/live.md @@ -14,21 +14,24 @@ Execute in order. No step skipped, no step reordered. 1. `live.mjs`: boot. If the request names or implies a file, route, or app inside a monorepo, infer the concrete path and run `node {{scripts_path}}/live.mjs --target ` instead; then run the rest of this live session from the returned `projectRoot`. 2. Open the app URL that serves `pageFile` (infer from `package.json`, docs, terminal output, or an open tab). Never use `serverPort`; it's the helper, not the app. **Cursor:** `browser_navigate` to that URL before polling; do not skip. **Other harnesses:** use the available browser tool; if the URL is uncertain, ask the user once. -3. Poll loop with the default long timeout (600000 ms). After every event or `--reply`, run `live-poll.mjs` again immediately. Never pass a short `--timeout=`. +3. Poll loop with the default long timeout (600000 ms). Run `live-poll.mjs` again immediately after every event or `--reply`; Codex runs this one-shot poll in the foreground. Never pass a short `--timeout=`. The global bar **Impeccable mark** dims and shows a pulsing amber dot when no agent is long-polling `/poll`. Hover the mark for the hint; restart `live-poll.mjs` to reconnect. -4. On `generate`: read screenshot if present; load the action's reference; plan three distinct directions; write all variants in one edit; `--reply done`; poll again. +4. On `generate`: reuse `event.scaffold` when present; read the screenshot if present; load the action's reference; deliver variants using the delivery policy below; `--reply done`; poll again. Generate in this thread. You already hold the project's tokens, conventions, and file layout; that context is the job, not overhead. 5. On `steer`: read the message and `pageUrl`; do the work (page edits, navigation help, or a short reply in the `--reply` message); `--reply steer_done`; poll again. No pickup ack. The Steer bar unlocks when `steer_done` arrives over SSE. -6. On `accept` / `discard`: the poll script runs `live-accept.mjs`, acknowledges the delivered event, and prints `_completionAck`. Plain accepts/discards are terminal immediately; carbonize accepts remain recoverable until you finish cleanup, run `live-complete.mjs --id EVENT_ID`, and only then poll again. +6. On `accept` / `discard`: the poll script runs `live-accept.mjs`, acknowledges the delivered event, and prints `_completionAck`. Plain accepts/discards are terminal immediately. Carbonize accepts remain recoverable until the foreground task runs `live-complete.mjs --id EVENT_ID`; finish that cleanup before polling again. 7. If interrupted, run `live-status.mjs` or `live-resume.mjs` before guessing. The durable journal replays unacknowledged work after helper restart. 8. On `exit`: run the cleanup at the bottom. Harness policy: -- **Claude Code**: run the poll as a **background task** (no short timeout). The harness notifies you when it completes, so the main conversation stays free. Do not block the shell. +- **Claude Code**: run the poll as a **background task** (no short timeout). The harness notifies you when it completes, so the main conversation stays free while you generate and publish in it. Do not block the shell. - **Cursor**: run **one-shot** poll in a **background terminal** with notify on `"type":"(steer|generate|accept|discard|exit)"`. After each event the poll exits; handle it, `--reply`, then start `live-poll.mjs` again. Do **not** use `--stream` on Cursor: incremental stdout notify is slower in practice than exit-based notify (~5s vs sub-second in testing). -- **Codex**: run the poll in the **foreground** (blocking shell; not a background task, not a subagent). Codex background exec sessions do not reliably surface poll stdout back into the conversation at the moment events arrive, so a "fire-and-forget" background poll will stall live mode. +- **Codex**: run the default one-shot poll in a **yielded foreground exec session**. Do not suffix it with `&`, use `--stream`, or leave Live without an active foreground poll. Handle every event in the main task; after each handler/reply, restart the foreground poll. - **Other harnesses**: one-shot foreground unless you know stdout reliably returns to this session when a shell exits. +Generation delivery policy: +- **Default (Cursor and other harnesses):** keep the established atomic single-edit delivery. Do not switch a harness to progressive until its poll loop is known not to block on the extra publish calls. This avoids trading model latency for extra tool-call latency on harnesses with different streaming behavior. + Chat is overhead. No recap, no tutorial output, no pasting PRODUCT / DESIGN bodies. Spend tokens on tools and edits; on failure, one or two short sentences. ## Start @@ -96,14 +99,14 @@ Server restart rule: start `live-server.mjs` again, then poll. Startup requeues **Insert mode** (`event.mode === "insert"`): `{id, mode: "insert", count, pageUrl, insert: { position, anchor }, placeholder: { width, height }, freeformPrompt?, screenshotPath?, comments?, strokes?}`. No `action`. Requires a non-empty `freeformPrompt` **or** annotations. Screenshot is sent only when annotations exist (same rule as replace). Use `placeholder` dimensions as a soft size hint for net-new content. -Speed matters; the user is watching a spinner. Minimize tool calls by using the wrap/insert helper and writing all variants in a single edit. +Speed matters; the user is watching the selected element. Reuse server preflight metadata when available, minimize discovery calls, and follow the harness-specific delivery policy above. ### Insert mode branch When `event.mode === "insert"`: 1. Read the screenshot if `event.screenshotPath` is present (annotations only). -2. Run the insert helper instead of wrap: +2. If `event.scaffold` is present, use it as the insert-helper result and do **not** run the helper again. Otherwise run the insert helper instead of wrap: ```bash node {{scripts_path}}/live-insert.mjs --id EVENT_ID --count EVENT_COUNT --position after \ @@ -113,7 +116,7 @@ node {{scripts_path}}/live-insert.mjs --id EVENT_ID --count EVENT_COUNT --positi - `--position` ← `event.insert.position` (`before` | `after`) - Anchor flags ← `event.insert.anchor` (same mapping as wrap: id, classes, tag, text) -The scaffold has **no** `data-impeccable-variant="original"`. Variants are net-new HTML+CSS inserted at `insertLine`. Load `brand.md` or `product.md` (freeform only, no action sub-command). Write all variants in one edit, then `--reply done`. +The scaffold has **no** `data-impeccable-variant="original"`. Variants are net-new HTML+CSS inserted at `insertLine`. Load `brand.md` or `product.md` (freeform only, no action sub-command). Deliver using the harness policy, then `--reply done`. For Svelte/SvelteKit targets, `live-insert.mjs` returns `previewMode: "svelte-component"` with `mode: "insert"`, `file` pointing at a temporary `node_modules/.impeccable-live//manifest.json`, `componentDir` pointing at the variant component files, and `sourceFile` pointing at the real `.svelte` route. Write each inserted variant as a real Svelte component (`v1.svelte`, `v2.svelte`, …) under `componentDir`. Insert variants must be non-empty net-new content with a single top-level root, no `data-impeccable-*` attributes, and CSS in each component's ` diff --git a/tests/live-accept.test.mjs b/tests/live-accept.test.mjs index e96b92dfe..50e4bc484 100644 --- a/tests/live-accept.test.mjs +++ b/tests/live-accept.test.mjs @@ -5,11 +5,12 @@ import { describe, it, beforeEach, afterEach } from 'node:test'; import assert from 'node:assert/strict'; -import { 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 } from 'node:child_process'; +import { execFileSync, spawnSync } from 'node:child_process'; +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'); @@ -29,6 +30,165 @@ function runAccept(cwd, args) { } } +// The failure that broke the first real Claude Code Live run. Progressive +// publication stages `.impeccable/live/artifacts/-r.`, which +// carries the session marker. findSessionFile walks `src`, `app`, `pages`, ... and +// then `.`; a project whose source is not under one of those (this repo's own site +// lives in `site/pages/`) falls through to the `.` walk, where dot-directories sort +// before letters — so the artifact was found before the real file. +describe('live-accept — marker search must ignore Impeccable state', () => { + let tmp; + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-accept-decoy-')); }); + afterEach(() => rmSync(tmp, { recursive: true, force: true })); + + const SOURCE = [ + '
', + '', + '
ORIGINAL
', + '
VARIANT ONE
', + '', + '
', + '', + ].join('\n'); + + function seed({ revisions = 3 } = {}) { + mkdirSync(join(tmp, 'site', 'pages'), { recursive: true }); + mkdirSync(join(tmp, '.impeccable', 'live', 'artifacts'), { recursive: true }); + writeFileSync(join(tmp, 'site', 'pages', 'index.astro'), SOURCE); + for (let r = 1; r <= revisions; r += 1) { + writeFileSync(join(tmp, '.impeccable', 'live', 'artifacts', `ab12cd34-r${r}.astro`), SOURCE); + } + } + + it('accepts into real source when a staged artifact carries the same marker', () => { + seed(); + const result = runAccept(tmp, ['--id', 'ab12cd34', '--variant', '1']); + assert.equal(result.handled, true, JSON.stringify(result)); + assert.equal( + result.file, + 'site/pages/index.astro', + 'accept must resolve the project file, not the .impeccable artifact decoy', + ); + const source = readFileSync(join(tmp, 'site', 'pages', 'index.astro'), 'utf-8'); + assert.match(source, /VARIANT ONE/); + assert.doesNotMatch(source, /impeccable-variants-start/, 'the wrapper must be gone from real source'); + }); + + it('discards into real source with an artifact decoy present', () => { + seed({ revisions: 1 }); + const result = runAccept(tmp, ['--id', 'ab12cd34', '--discard']); + assert.equal(result.handled, true, JSON.stringify(result)); + assert.equal(result.file, 'site/pages/index.astro'); + assert.match(readFileSync(join(tmp, 'site', 'pages', 'index.astro'), 'utf-8'), /ORIGINAL/); + }); +}); + +describe('live-accept — session id validation', () => { + let tmp; + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-accept-id-')); }); + afterEach(() => rmSync(tmp, { recursive: true, force: true })); + + // --id becomes a path segment for the accept receipt. Traversal here wrote + // JSON to arbitrary absolute paths (e.g. `--id ../../../../etc/evil`). + for (const id of ['../../../../etc/evil', 'a/b', '..', 'a\\b', '']) { + it(`refuses --id ${JSON.stringify(id)} without writing a receipt`, () => { + const res = spawnSync('node', [ACCEPT, '--id', id, '--discard'], { + cwd: tmp, + encoding: 'utf-8', + }); + assert.equal(res.status, 1, 'must exit non-zero'); + assert.match(res.stderr, /Invalid --id|Missing --id/); + assert.equal(existsSync(join(tmp, '.impeccable', 'live', 'accept-receipts')), false); + }); + } + + it('still accepts a well-formed id', () => { + const res = spawnSync('node', [ACCEPT, '--id', 'ab12cd34', '--discard'], { + cwd: tmp, + encoding: 'utf-8', + }); + assert.doesNotMatch(res.stderr || '', /Invalid --id/); + }); + + // --variant is interpolated into a RegExp and into the markup written back to + // source. `.*` matched the `original` block first, so the CLI reported a + // successful accept while actually restoring the original. + for (const variant of ['.*', '[12]', 'original', '1e2', '']) { + it(`refuses --variant ${JSON.stringify(variant)} rather than matching by regex`, () => { + writeFileSync(join(tmp, 'page.html'), [ + '', + '
ORIGINAL CONTENT
', + '
VARIANT ONE
', + '', + '', + ].join('\n')); + const res = spawnSync('node', [ACCEPT, '--id', 'ab12cd34', '--variant', variant], { + cwd: tmp, + encoding: 'utf-8', + }); + assert.equal(res.status, 1); + assert.match(res.stderr, /Invalid --variant|Need --discard/); + assert.match( + readFileSync(join(tmp, 'page.html'), 'utf-8'), + /impeccable-variants-start/, + 'a rejected variant must leave the wrapper untouched', + ); + }); + } +}); + +// The plain wrapper is the only non-component preview path now that the isolated +// source-artifact mode is gone, so its lock-contention behaviour is what carries +// these guarantees. +describe('live-accept — plain wrapper under source-lock contention', () => { + let tmp; + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-accept-lock-')); }); + afterEach(() => rmSync(tmp, { recursive: true, force: true })); + + const PAGE = [ + '', + '
ORIGINAL
', + '
VARIANT ONE
', + '', + '', + ].join('\n'); + + function holdLock() { + // realpath: mkdtemp hands back /var/... on macOS while the child's cwd + // resolves to /private/var/..., and the lock digest hashes the absolute path. + const realTmp = realpathSync(tmp); + const lockPath = sourceLockPath(join(realTmp, 'page.html'), realTmp); + mkdirSync(dirname(lockPath), { recursive: true }); + // process.pid is alive, so the lock is a live holder rather than stale. + writeFileSync(lockPath, JSON.stringify({ + owner: 'generation:ab12cd34:1', token: 'other', pid: process.pid, at: Date.now(), + }) + '\n'); + } + + for (const [label, args] of [['accept', ['--variant', '1']], ['discard', ['--discard']]]) { + it(`reports a blocked ${label} as mode:error rather than a manual handoff`, () => { + writeFileSync(join(tmp, 'page.html'), PAGE); + holdLock(); + const result = runAccept(tmp, ['--id', 'ab12cd34', ...args]); + assert.equal(result.handled, false, JSON.stringify(result)); + assert.equal(result.error, 'source_locked'); + // Without mode:error, completion.mjs classifies this as agent_done with an ok + // ack and live.md tells the agent to hand-edit the file — racing the publisher + // that holds the lock. + assert.equal(result.mode, 'error'); + assert.equal(readFileSync(join(tmp, 'page.html'), 'utf-8'), PAGE, 'source must be untouched'); + assert.equal(existsSync(join(tmp, '.impeccable', 'live', 'accept-receipts')), false, 'no receipt for a failed op'); + }); + } + + it('succeeds once the lock is gone', () => { + writeFileSync(join(tmp, 'page.html'), PAGE); + const result = runAccept(tmp, ['--id', 'ab12cd34', '--variant', '1']); + assert.equal(result.handled, true, JSON.stringify(result)); + assert.match(readFileSync(join(tmp, 'page.html'), 'utf-8'), /VARIANT ONE/); + }); +}); + describe('live-accept — style-element edge cases', () => { let tmp; beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), 'impeccable-accept-test-')); }); @@ -74,6 +234,33 @@ describe('live-accept — style-element edge cases', () => { assert.ok(!after.includes('original text'), 'original content dropped'); }); + it('replays a durable receipt when Accept is retried after source was already written', () => { + const html = ` + +
+

original

+ block should also be treated as a // single skipped unit; the line has both open and close tags. it('finds the accepted variant after a single-line block', () => { diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs index 45c84d1dc..79e8d21f1 100644 --- a/tests/live-browser-regression.test.mjs +++ b/tests/live-browser-regression.test.mjs @@ -74,7 +74,7 @@ describe('live-browser.js regression guards', () => { ); }); - it('uses a Svelte-gated painted-ancestor crop proxy for shader capture', () => { + it('uses a framework-component-gated painted-ancestor crop proxy for shader capture', () => { assert.match( SOURCE, /function findShaderProxyCaptureRoot\(el\) \{[\s\S]{0,500}?let node = el\.parentElement;[\s\S]{0,700}?containsElement && paintsShaderProxySurface\(node\)[\s\S]{0,120}?return null;/, @@ -87,8 +87,8 @@ describe('live-browser.js regression guards', () => { ); assert.match( SOURCE, - /function shouldUseAncestorCropShaderProxy\(el\) \{[\s\S]{0,260}?window\.__IMPECCABLE_LIVE_ADAPTER__[\s\S]{0,280}?currentPreviewMode === 'svelte-component' \|\| svelteComponentSession[\s\S]{0,260}?dataset\?\.impeccablePreview === 'svelte-component';/, - 'ancestor crop proxy must be gated to the Svelte adapter / Svelte component previews', + /function shouldUseAncestorCropShaderProxy\(el\) \{[\s\S]{0,260}?window\.__IMPECCABLE_LIVE_ADAPTER__[\s\S]{0,280}?isFrameworkComponentPreviewMode\(currentPreviewMode\) \|\| svelteComponentSession[\s\S]{0,260}?isFrameworkComponentPreviewMode\(wrapper\?\.dataset\?\.impeccablePreview\);/, + 'ancestor crop proxy must be gated to Svelte/Vue component previews', ); assert.match( SOURCE, @@ -141,11 +141,30 @@ describe('live-browser.js regression guards', () => { it('restores unsaved inline edit drafts before hideBar tears editing down', () => { assert.match( SOURCE, - /function hideBar\(\) \{[\s\S]{0,620}?if \(state === 'EDITING'\) restoreInlineEditDrafts\(\);[\s\S]{0,80}?disableInlineEdit\(\);/, + /function hideBar\(instant\) \{[\s\S]{0,720}?if \(state === 'EDITING'\) restoreInlineEditDrafts\(\);[\s\S]{0,80}?disableInlineEdit\(\);/, 'hideBar should not leave unsaved contenteditable drafts in the DOM when an external event hides the bar', ); }); + it('discards variants without hiding the original or animating stale chrome', () => { + assert.match(SOURCE, /function showOriginalDuringDiscard\(sessionId\)[\s\S]{0,900}?data-impeccable-variant="original"/); + assert.match(SOURCE, /function handleDiscard\(\)[\s\S]{0,420}?cleanup\(\{ restoreOriginal: true, instantChrome: true \}\)/); + assert.match(SOURCE, /if \(instant\) barEl\.style\.display = 'none'/); + assert.match( + SOURCE, + /if \(restoreOriginal\) showOriginalDuringDiscard\(cleanupSessionId\);\s*else wrapper\.style\.display = 'none';/, + 'only non-discard cleanup may blank the wrapper while waiting for HMR', + ); + }); + + it('stores live state off the document root and preserves the selected anchor top', () => { + assert.match(SOURCE, /window\.__IMPECCABLE_LIVE_STATE__ = next/); + assert.doesNotMatch(SOURCE, /document\.documentElement\.dataset\.impeccableLiveState/); + assert.match(SOURCE, /pickedAnchorViewportTop: Number\.isFinite\(pickedAnchorViewportTop\)/); + assert.match(SOURCE, /scrollLockAnchorTop = typeof initialAnchorTop === 'number' && isFinite\(initialAnchorTop\)/); + assert.match(SOURCE, /const anchorDelta = anchorTop - scrollLockAnchorTop/); + }); + it('does not autofocus the steering chat while inline editing', () => { assert.match( SOURCE, @@ -443,6 +462,25 @@ describe('live-browser.js regression guards', () => { /function syncAgentPollingUi\(/, 'global bar brand must reflect agent poll connectivity', ); + // The indicator goes quiet both when nobody is polling and when the agent + // holds leased work. Under one-shot foreground polling the second case is + // every normal generation, so a single "run live-poll.mjs to connect" tip + // told users to fix a healthy session. + assert.match( + SOURCE, + /function agentHasWorkInFlight\(\)\s*\{\s*return state === 'GENERATING' \|\| state === 'SAVING';/, + 'agent poll copy must distinguish a busy agent from an absent one', + ); + assert.match( + SOURCE, + /agentHasWorkInFlight\(\) \? AGENT_BUSY_TIP : AGENT_DISCONNECTED_TIP/, + 'a busy agent must not be described as disconnected', + ); + assert.match( + SOURCE, + /tip\.textContent = agentStatusText\(\)/, + 'tooltip copy must be derived at display time, not read from a cache the 5s status poll last wrote', + ); assert.match( SOURCE, /case 'agent_polling':/, @@ -841,6 +879,58 @@ describe('live-browser.js regression guards', () => { ); }); + it('makes every arrived progressive variant immediately actionable', () => { + assert.match( + SOURCE, + /if \(arrivedVariants > 0\) \{[\s\S]{0,180}?setLiveState\('CYCLING'\)/, + 'the first arrived variant should leave the generating-only state', + ); + assert.doesNotMatch( + SOURCE, + /arrivedVariants < expectedVariants\) \{[\s\S]{0,180}?accept\.style\.pointerEvents = 'none'/, + 'Accept must not wait for variants the user did not choose', + ); + assert.doesNotMatch( + SOURCE, + /arrivedVariants < expectedVariants\) \{[\s\S]{0,180}?discard\.style\.pointerEvents = 'none'/, + 'Discard must cancel remaining work immediately', + ); + assert.match( + SOURCE, + /const resumedState = arrivedVariants > 0 \? 'CYCLING' : 'GENERATING'/, + 'reload recovery should preserve a partially delivered review state', + ); + assert.match( + SOURCE, + /arrivedVariants >= expectedVariants && expectedVariants > 0[\s\S]{0,100}?\? 'variants_ready'[\s\S]{0,60}?: 'variants_progress'/, + 'checkpoint timing must distinguish partial review from complete delivery by counts', + ); + }); + + it('keeps deferred Tune controls visible and refreshes params-only publications', () => { + assert.match( + SOURCE, + /const paramsPending = !hasParams && \(parameterGenerationState === 'pending' \|\| parameterGenerationState === 'loading'\)/, + 'the cycling bar must expose Tune while parameter generation is outstanding', + ); + assert.match(SOURCE, /tune\.disabled = true/, 'pending Tune must be visibly loading but non-interactive'); + assert.match(SOURCE, /Tune controls are ready\./, 'parameter arrival needs a clear ready indication'); + assert.match( + SOURCE, + /msg\.publicationKind !== 'params' && arrivedVariants >= targetArrived/, + 'a params-only publication must refresh even though the variant count is unchanged', + ); + assert.match(SOURCE, /revisionDomain: 'browser'/, 'browser checkpoints must use their own revision domain'); + }); + + it('promotes an early-accepted Svelte preview before releasing the picker', () => { + assert.match( + SOURCE, + /function scheduleAcceptCleanup\(accepted\) \{[\s\S]{0,420}?if \(accepted\?\.isSvelteComponent\) \{[\s\S]{0,120}?commitAcceptedSvelteComponentToDom\(accepted\.id\);[\s\S]{0,120}?cleanupAcceptedSession\(\);/, + 'Svelte early accept must tear down its adapter mount before the next picking session starts', + ); + }); + it('variant injection resolves the picked anchor before entering recovery', () => { assert.match( SOURCE, diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index dc62d88bc..86669e3a2 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -5,8 +5,35 @@ import { join } from 'node:path'; const SOURCE = readFileSync(join(process.cwd(), 'skill/scripts/live-browser.js'), 'utf-8'); const PENDING_DOCK_POSITION_SOURCE = SOURCE.match(/function positionPendingDock\(\) \{[\s\S]*?\n \}/)?.[0] || ''; +const CAPTURE_AND_EMIT_SOURCE = SOURCE.match(/async function captureAndEmit\([\s\S]*?\n \}/)?.[0] || ''; describe('live-browser source contracts', () => { + it('reports foreground poll connectivity without a background worker dependency', () => { + assert.match( + SOURCE, + /syncAgentPollingUi\(!!msg\.agentPolling\)/, + 'the initial SSE state should include foreground poll connectivity', + ); + assert.doesNotMatch(SOURCE, /codexWorker|codex-worker|codex_cli_unavailable/); + }); + + it('dispatches plain generation before screenshot capture without bypassing annotated evidence', () => { + const dispatchIndex = CAPTURE_AND_EMIT_SOURCE.indexOf('await sendEvent(basePayload);'); + const captureIndex = CAPTURE_AND_EMIT_SOURCE.indexOf('await captureElementToBlob'); + assert.ok(dispatchIndex >= 0, 'plain generation should dispatch immediately'); + assert.ok(captureIndex > dispatchIndex, 'plain generation dispatch must happen before capture begins'); + assert.match( + CAPTURE_AND_EMIT_SOURCE, + /if \(blob && hasAnnotations\)[\s\S]*?\/annotation\?token=/, + 'annotation screenshots should still upload before annotated generation dispatch', + ); + assert.match( + CAPTURE_AND_EMIT_SOURCE, + /if \(hasAnnotations\) \{[\s\S]*?basePayload\.clientSentAt = Date\.now\(\);\s*sendEvent\(screenshotPath \? \{ \.\.\.basePayload, screenshotPath \} : basePayload\);\s*\}/, + 'annotated generation should dispatch exactly after capture and upload resolve', + ); + }); + it('saves copy edits to the staged buffer with rich AI context', () => { assert.doesNotMatch( SOURCE, @@ -285,7 +312,7 @@ describe('live-browser source contracts', () => { assert.match(SOURCE, /sendEvent\(\{ type: 'discard', id: currentSessionId \}, \{ throwOnError: true \}\)/); }); - it('waits for post-carbonize completion before final accepted DOM cleanup', () => { + it('releases the foreground picker after deterministic accept while carbonize finishes', () => { assert.match( SOURCE, /let pendingAcceptedSession = null;/, @@ -309,8 +336,8 @@ describe('live-browser source contracts', () => { const agentDoneStart = SOURCE.indexOf("case 'agent_done':"); const errorCaseStart = SOURCE.indexOf("case 'error':", agentDoneStart); const agentDoneSource = SOURCE.slice(agentDoneStart, errorCaseStart); - assert.match(agentDoneSource, /Carbonize accepts are not terminal/); - assert.match(agentDoneSource, /break;/); + assert.match(agentDoneSource, /must not hold the foreground picker hostage/); + assert.match(agentDoneSource, /maybeCompleteAcceptedSession\(msg\)/); assert.match( SOURCE, /function handleGo\(\)[\s\S]{0,900}?pendingAcceptedSession = null;[\s\S]{0,80}?currentSessionId = id8\(\);/, @@ -319,15 +346,15 @@ describe('live-browser source contracts', () => { const handleAcceptStart = SOURCE.indexOf('function handleAccept()'); const maybeCompleteStart = SOURCE.indexOf('function maybeCompleteAcceptedSession', handleAcceptStart); const handleAcceptSource = SOURCE.slice(handleAcceptStart, maybeCompleteStart); - assert.doesNotMatch( + assert.match( handleAcceptSource, - /state = 'CONFIRMED'|cleanupAcceptedSession\(|hideBar\(\)/, - 'accept enqueue should not clear or confirm the browser session before source cleanup completes', + /sendEvent\(acceptPayload, \{ throwOnError: true \}\)[\s\S]*?markSessionHandled\(\);[\s\S]*?setLiveState\('CONFIRMED'\);[\s\S]*?scheduleAcceptCleanup\(pending\);/, + 'durable accept intent should release the foreground picker before background source cleanup completes', ); assert.match( SOURCE, - /function scheduleAcceptCleanup\(accepted\)[\s\S]*?acceptedDomAlreadyClean\(accepted\)[\s\S]*?setTimeout\(function\(\) \{[\s\S]*?ensureAcceptedDomClean\(accepted\);[\s\S]*?cleanupAcceptedSession\(\);[\s\S]*?\}, 1800\);/, - 'post-cleanup fallback should give HMR a second chance before mutating React-owned DOM', + /function scheduleAcceptCleanup\(accepted\)[\s\S]*?queueMicrotask\(function\(\) \{[\s\S]*?cleanupAcceptedSession\(\);[\s\S]*?setTimeout\(function\(\) \{[\s\S]*?ensureAcceptedDomClean\(accepted\);[\s\S]*?\}, 1200\);/, + 'foreground cleanup should be immediate while the no-HMR DOM fallback stays deferred', ); assert.match( SOURCE, @@ -393,4 +420,12 @@ describe('live-browser source contracts', () => { 'source fallback should translate simple JSX style objects such as display:none', ); }); + + it('loads progressive source checkpoints through the no-HMR fallback', () => { + assert.match( + SOURCE, + /case 'variant_progress':[\s\S]{0,1400}?msg\.previewMode === 'source'[\s\S]{0,1000}?arrivedVariants >= targetArrived[\s\S]{0,260}?injectVariantsFromSource\(msg\.previewFile \|\| msg\.file, msg\.id\)/, + 'source-mode progress should let framework HMR settle before using the no-HMR fallback', + ); + }); }); diff --git a/tests/live-completion.test.mjs b/tests/live-completion.test.mjs index 7fc51a742..7e8e28a90 100644 --- a/tests/live-completion.test.mjs +++ b/tests/live-completion.test.mjs @@ -53,6 +53,28 @@ describe('live completion type classification', () => { ); }); + // Component previews keep their variants in module files, not in the user's + // source, so a failed accept leaves nothing to hand-edit: that is a failure, not + // live.md's "read file, find markers, edit" handoff. Only svelte-component was + // special cased, so the identical failure on a Vue preview read as success. + for (const previewMode of ['svelte-component']) { + it(`treats a failed ${previewMode} accept as an error, not a manual handoff`, () => { + assert.equal( + completionTypeForAcceptResult('accept', { handled: false, error: 'source_locked', previewMode }), + 'error', + ); + }); + } + + it('still treats a failed plain-wrapper accept as a manual handoff', () => { + // The one shape with editable markers in source. This must not regress into + // an error, or every hand-editable session starts failing the poll loop. + assert.equal( + completionTypeForAcceptResult('accept', { handled: false, error: 'Markers not found' }), + 'agent_done', + ); + }); + it('classifies handled accept/discard and real failures explicitly', () => { assert.equal(completionTypeForAcceptResult('accept', { handled: true }), 'complete'); assert.equal(completionTypeForAcceptResult('discard', { handled: true }), 'discarded'); diff --git a/tests/live-e2e-agent-output.test.mjs b/tests/live-e2e-agent-output.test.mjs index d988c10b9..0e9fc203d 100644 --- a/tests/live-e2e-agent-output.test.mjs +++ b/tests/live-e2e-agent-output.test.mjs @@ -1,8 +1,18 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; -import { htmlToJsx, normalizeVariantOutput } from './live-e2e/agent.mjs'; +import { + htmlToJsx, + isExpectedGenerationCancellation, + normalizeVariantOutput, +} from './live-e2e/agent.mjs'; describe('live-e2e agent output translation', () => { + it('treats a fenced late generation as expected cancellation only', () => { + assert.equal(isExpectedGenerationCancellation(new Error('Source publication prepare failed: stale_generation_epoch')), true); + assert.equal(isExpectedGenerationCancellation(new Error('Source publication failed: stale_source_revision')), false); + assert.equal(isExpectedGenerationCancellation(new Error('provider unavailable')), false); + }); + it('converts HTML class and inline style attributes to JSX syntax', () => { const jsx = htmlToJsx( '

Title

', diff --git a/tests/live-e2e-llm-agent.test.mjs b/tests/live-e2e-llm-agent.test.mjs index 53d8e98eb..3ec82f3b5 100644 --- a/tests/live-e2e-llm-agent.test.mjs +++ b/tests/live-e2e-llm-agent.test.mjs @@ -9,10 +9,13 @@ import { createLlmAgent, parseManualEditResponse, parseVariantResponse, + progressiveVariantGuidance, resolveLlmAgentConfig, validateManualEditCoverage, validateManualEditPlanningCoverage, validateVariantMaterialChange, + validateVariantCount, + validateProgressiveVariantOutput, validateVariantVisibleCopy, } from './live-e2e/agents/llm-agent.mjs'; @@ -1459,6 +1462,19 @@ describe('live-e2e LLM agent manual edit coverage validation', () => { }); describe('live-e2e LLM agent variant prompt', () => { + it('makes progressive phase boundaries and lazy parameters explicit', () => { + const first = progressiveVariantGuidance({ count: 1, progressive: { phase: 'first' } }); + const remaining = progressiveVariantGuidance({ + count: 3, + progressive: { phase: 'remaining', omitFirstVariantCss: true }, + }); + assert.match(first, /params: \[\]/); + assert.match(first, /materially different/); + assert.match(remaining, /complete final set of exactly 3 variants/); + assert.match(remaining, /Keep its innerHtml exactly unchanged/); + assert.match(remaining, /Do not repeat or modify any scopedCss rule/); + }); + it('tells the model not to nest duplicate picked containers', () => { assert.match(VARIANT_SYSTEM_INSTRUCTIONS, /replacement root itself/); assert.match(VARIANT_SYSTEM_INSTRUCTIONS, /do not wrap a duplicate/); @@ -1484,6 +1500,53 @@ describe('live-e2e LLM agent variant prompt', () => { }); describe('live-e2e LLM agent variant copy validation', () => { + it('enforces the exact requested variant count', () => { + const parsed = { scopedCss: '', variants: [{ innerHtml: '

One

', params: [] }] }; + assert.match(validateVariantCount(parsed, { count: 2 }), /expected exactly 2 variants, received 1/); + assert.equal(validateVariantCount(parsed, { count: 1 }), null); + }); + + it('defers progressive params and preserves the visible first variant', () => { + const firstHtml = '

One

'; + assert.match( + validateProgressiveVariantOutput( + { variants: [{ innerHtml: firstHtml, params: [{ id: 'weight' }] }] }, + { progressive: { phase: 'first' } }, + ), + /defer params/, + ); + assert.equal( + validateProgressiveVariantOutput( + { variants: [{ innerHtml: firstHtml, params: [] }] }, + { progressive: { phase: 'remaining', firstVariant: { innerHtml: firstHtml } } }, + ), + null, + ); + assert.match( + validateProgressiveVariantOutput( + { variants: [{ innerHtml: '

Changed

', params: [] }] }, + { progressive: { phase: 'remaining', firstVariant: { innerHtml: firstHtml } } }, + ), + /preserve variant 1/, + ); + assert.match( + validateProgressiveVariantOutput( + { + scopedCss: '@scope ([data-impeccable-variant="1"]) { .hero-title { color: red; } }', + variants: [{ innerHtml: firstHtml, params: [] }], + }, + { + progressive: { + phase: 'remaining', + firstVariant: { innerHtml: firstHtml }, + omitFirstVariantCss: true, + }, + }, + ), + /omit already-published variant 1 CSS/, + ); + }); + it('allows variants that preserve the picked element text', () => { const result = validateVariantVisibleCopy( { diff --git a/tests/live-e2e.test.mjs b/tests/live-e2e.test.mjs index 0d120bc49..a6be62570 100644 --- a/tests/live-e2e.test.mjs +++ b/tests/live-e2e.test.mjs @@ -22,7 +22,7 @@ import { describe, it, before, after } from 'node:test'; import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; -import { cpSync, existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { appendFileSync, cpSync, existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { dirname, join, relative } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -38,6 +38,7 @@ import { clickAccept, clickApplyEdits, clickEditCopy, + clickDiscard, clickSaveEdit, clickGo, clickNext, @@ -45,6 +46,7 @@ import { editTextLeaf, drawAnnotationPinAndStroke, getVisibleVariant, + installLiveQueryHelpers, pickElement, runLiveChromeBottomBarSmoke, waitForApplyDockHidden, @@ -94,6 +96,10 @@ const reloadVariants = process.env.IMPECCABLE_E2E_RELOAD_VARIANTS === '1' const scenarioNames = parseFixtureFilter(process.env.IMPECCABLE_E2E_SCENARIOS); const liveE2eTestTimeoutMs = readPositiveIntEnv('IMPECCABLE_E2E_TEST_TIMEOUT_MS'); const liveE2eTestOptions = liveE2eTestTimeoutMs ? { timeout: liveE2eTestTimeoutMs } : {}; +// Widens the window between the server-side preflight scaffold (which triggers +// a framework HMR reload) and the agent's variant write. Used to reproduce +// races where the browser observes a wrapper with zero variants mid-generation. +const atomicDelayMs = readPositiveIntEnv('IMPECCABLE_E2E_ATOMIC_DELAY_MS') || 0; if (fixtures.length === 0) { describe('live-e2e (no runtime fixtures registered)', () => { @@ -206,6 +212,7 @@ for (const { name, fixture } of fixtures) { browser, agent, wrapTarget: wrapTargetFromPickedElement, + atomicDelayMs, log: (m) => t.diagnostic(m), }); @@ -220,7 +227,7 @@ for (const { name, fixture } of fixtures) { const domSelector = isInsert ? insertDomSelector : pickSelector; - const usesSvelteComponentPreview = fixtureUsesSvelteKitAdapter(fixture); + const usesSvelteComponentPreview = fixtureUsesSvelteKitAdapter(fixture) || name === 'nuxt-vite7'; const variantContentSelector = isInsert ? (usesSvelteComponentPreview ? '.inserted-copy' : '[data-impeccable-variant="2"] .inserted-copy') : usesSvelteComponentPreview @@ -314,10 +321,11 @@ for (const { name, fixture } of fixtures) { const after = readFileSync(sourceFile, 'utf-8'); const svelteComponentSession = svelteComponentTargetFor(sourceFile); if (svelteComponentSession) { - const variantFile = join(tmp, svelteComponentSession.manifest.componentDir, 'v2.svelte'); + const componentExtension = svelteComponentSession.manifest.componentExtension || 'svelte'; + const variantFile = join(tmp, svelteComponentSession.manifest.componentDir, `v2.${componentExtension}`); const variantBody = readFileSync(variantFile, 'utf-8'); const routeBody = readFileSync(join(tmp, svelteComponentSession.manifest.sourceFile), 'utf-8'); - assert.match(after, /"previewMode": "svelte-component"/, 'Svelte component manifest inserted'); + assert.match(after, /"previewMode": "(?:svelte|vue)-component"/, 'framework component manifest inserted'); if (isInsert) { assert.equal(svelteComponentSession.manifest.mode, 'insert', 'Svelte insert manifest marks insert mode'); if (agentMode === 'fake') { @@ -328,9 +336,9 @@ for (const { name, fixture } of fixtures) { assert.match(variantBody, /<([a-z][\w:-]*)\b[\s\S]*<\/\1>|<[a-z][\w:-]*\b[^>]*\/>/i, 'Svelte insert variant component contains a root element'); } } else { - assert.match(variantBody, new RegExp(`<${svelteComponentSession.expectedTag}\\b`), 'Svelte variant component contains target element'); + assert.match(variantBody, new RegExp(`<${svelteComponentSession.expectedTag}\\b`), 'component variant contains target element'); } - assert.doesNotMatch(routeBody, /data-impeccable-variants="/, 'Svelte route source is not edited during generation'); + assert.doesNotMatch(routeBody, /data-impeccable-variants="/, 'route source is not edited during component preview'); } else { assert.match(after, /data-impeccable-variants="/, 'wrapper inserted'); } @@ -349,7 +357,8 @@ for (const { name, fixture } of fixtures) { } } if (svelteComponentSession) { - assert.match(readFileSync(join(tmp, svelteComponentSession.manifest.componentDir, 'v2.svelte'), 'utf-8'), /