From c7b67b38324f34a7438db64edcbabaed53f6c7d0 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Fri, 17 Jul 2026 18:56:23 -0700 Subject: [PATCH] Drop the live generator subagent; fix the artifact decoy that broke accept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real Claude Code Live run failed, and the subagent was not the cause. Root cause: progressive publication stages each revision as `.impeccable/live/artifacts/-r.`, nothing ever deleted them, and findSessionFile's walker skipped only node_modules/.git/dist/build. It searches src, app, pages, ... 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 accept found the artifact instead of the real file. Two outcomes, both reproduced: where isGeneratedFile returns true it declines with mode: 'fallback' (what the run hit, after which the agent hand-carbonized several hundred lines across three stylesheets, including unrequested drive-by edits); where it returns false, accept writes the variant into the throwaway artifact and reports handled: true while real source never changes. The E2E suite could not have caught this. Every fixture puts source under `src/`, which is searched before the `.` walk can reach `.impeccable`. Five framework fixtures and three progressive scenarios pass because of fixture layout, not because the path works. I read that as evidence and shouldn't have. - Never search `.impeccable`: it is Impeccable's own state, never project source. - Retire a session's staged artifacts on accept/discard, so they cannot outlive the session and become a decoy for anything else that walks the tree. - Regression tests use a site/pages layout with artifacts present. All three fail against the previous code. Generator subagent removed, on both harnesses: The parent must hand-compress the design system into the handoff, and compression is lossy. Measured on the real run: a 6,826-char handoff carrying exactly one token reference, after the parent had itself read kinpaku-tokens.css. The subagent then spent 3 of its first 9 turns hunting DESIGN.md, gave up, and emitted 0 var(--token) uses and 22 raw oklch literals — violating its own spec's "Never invent raw colors when tokens exist" — including a 1:1 gold-on-gold contrast bug. Isolation is not a benefit here; knowing the design system is the job. Generation stays in the main thread, which already holds the tokens and writes them from the first byte, so carbonize is a move rather than a translation. Copy edits keep their subagent: applying a known set of ops to a named file is self-contained, so an isolated context costs nothing. That is the line. Progressive delivery stays for Codex and Claude Code, main-thread driven. Claude Code keeps the full benefit because its poll is a background task. Codex's poll blocks the foreground, so with no subagent the user sees variant 1 early via HMR but cannot accept it until the trio finishes; that is the cost of the simplification and it is worth naming. Prepared with AI assistance under maintainer direction. Co-Authored-By: Claude --- skill/agents/impeccable-live-generator.md | 54 ------------------ skill/reference/live.md | 6 +- skill/scripts/live-accept.mjs | 24 +++++++- skill/scripts/live/generation-publisher.mjs | 24 +++++++- tests/live-accept.test.mjs | 63 +++++++++++++++++++++ tests/live-reference.test.mjs | 33 ++++++----- 6 files changed, 129 insertions(+), 75 deletions(-) delete mode 100644 skill/agents/impeccable-live-generator.md diff --git a/skill/agents/impeccable-live-generator.md b/skill/agents/impeccable-live-generator.md deleted file mode 100644 index 38cbf30ff..000000000 --- a/skill/agents/impeccable-live-generator.md +++ /dev/null @@ -1,54 +0,0 @@ ---- -name: impeccable-live-generator -codex-name: impeccable_live_generator -description: Generates and transactionally publishes one Impeccable Live variant request while the parent keeps polling. -tools: Read, Write, Edit, Bash, Glob, Grep -model: inherit -effort: low -max-turns: 16 -nickname-candidates: - - Variant Producer - - Live Composer - - Direction Maker ---- - -# Impeccable Live Generator - -You own one leased Impeccable Live `generate` event. The parent thread owns browser control and the poll loop. Never poll, Accept, Discard, commit, stage, or edit generated provider output. - -## Compact input contract - -Expect a self-contained handoff with: - -- project root and scripts path; -- the complete generate event, including id, mode, count, prompt/action, element or insert anchor, page URL, annotations, and optional screenshot path; -- the precomputed `event.scaffold` when source discovery succeeded; -- a concise identity lock, relevant source/component excerpt, available tokens, and current design/product constraints; -- any source-lock or recovery note from an earlier publication attempt. - -Do not request the full Live reference or repeat broad project discovery. Use the scaffold and compact handoff. Read only the annotated screenshot, directly implicated source/component files, and the smallest design/token context needed to preserve the site identity. - -## Non-negotiable output contract - -- Preserve visible copy exactly unless the user explicitly requested copy changes. -- Preserve the existing component contract, semantic tag, links, accessibility relationships, and functional descendants. -- Reuse existing components, CSS custom properties, typography, spacing, radii, and color roles. Never invent raw colors or foreign fonts when tokens exist. -- Do not add gradients, blur, glow, glass, neon, decorative shadows, emoji, or unrelated content unless the explicit user direction requires it. -- Never decorate a card, label, row, tab, or container with a colored stripe on only one edge. This includes borders, inset box-shadows, gradients, and pseudo-elements; selection and focus indicators are the only exception. -- Produce the requested number of materially different directions through hierarchy, layout, density, or existing color-role allocation. CSS-only no-ops and source-identical variants are invalid. -- Keep temporary Live markers and preview CSS out of accepted project truth; the publisher/Accept pipeline owns cleanup. - -## Workflow - -1. Trust `event.scaffold` when present. Do not rerun source discovery or wrapping. If it is absent, run the correct wrap/insert helper once. -2. If annotations exist, read the screenshot before designing. Treat pins and strokes as semantic constraints. -3. Name all directions and their parameter axes before writing so the set stays coherent. Parameters are lazy: revision 1 carries no parameter manifest. -4. Prepare revision 1 with `live-publish.mjs --prepare --id EVENT_ID --file SOURCE_FILE`. Edit only the returned artifact (or isolated component directory), never live project source. -5. Write one complete, valid first variant plus only its CSS. Run `detect.mjs --json` on the staged artifact before publishing. Fix genuine findings; when inspection shows a contextual false positive, use judgment and continue without changing persistent detector configuration. The detector is a review signal, not an automatic publication veto. Publish immediately with the returned epoch, artifact path, expected source hash, `--arrived 1`, and the requested `--expected` count. -6. Prepare again from the published prefix, add the remaining validated directions, attach parameter manifests only with the complete set, and publish the largest ready prefix. Preserve every already-published variant byte-for-byte. -7. On `stale_generation_epoch`, `source_changed`, or another fence rejection, stop. Do not retry against stale source or leave direct edits behind. -8. Verify the final artifact/source parses and run the detector again before the final publication. Apply the same genuine-finding versus contextual-false-positive judgment. Reply exactly once with `live-poll.mjs --reply EVENT_ID done --file RELATIVE_PATH`. On a real failure, reply once with `error` and a short reason. - -For Svelte or Vue component preview, write only `vN.svelte` / `vN.vue` in the isolated `componentDir` returned by prepare and update the isolated manifest. Never edit the live component directory. For JSX/TSX source previews, preserve JSX attribute syntax and wrap preview CSS as required by `scaffold.cssAuthoring`. - -Speed matters because the user is waiting. Publish the first reviewable result before exploring tunables, writing explanations, or polishing later variants. Return no recap: tool work and the protocol reply are the result. diff --git a/skill/reference/live.md b/skill/reference/live.md index 4a7970703..87e0826c7 100644 --- a/skill/reference/live.md +++ b/skill/reference/live.md @@ -17,16 +17,16 @@ Execute in order. No step skipped, no step reordered. 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`: reuse `event.scaffold` when present; read the screenshot if present; load the action's reference; deliver variants using the harness policy below; `--reply done`; poll again. In Codex and Claude Code, delegate the complete event to `impeccable_live_generator` / `impeccable-live-generator` and resume polling immediately; the generator owns publication and the reply. +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 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. When `generate` arrives and native subagents are available, delegate the complete event to `impeccable-live-generator` with a compact handoff, then start the next poll immediately while that subagent publishes and replies; if unavailable, generate inline with the same contract. Do not paste this full reference into the handoff. The subagent's context is separate from yours, which is the point: a long session's screenshots, references, and variant CSS stay out of the main conversation. Handle Steer, Accept/Discard, manual Apply, carbonize, and Exit in the main conversation. +- **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 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. When `generate` arrives, delegate to the low-effort `impeccable_live_generator` agent with a compact handoff, then immediately start the next foreground poll while that agent publishes and replies. Do not paste this full reference into the handoff. Handle Steer, Accept/Discard, manual Apply, carbonize, and Exit in the main task; after each handler/reply, restart the foreground poll. +- **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: diff --git a/skill/scripts/live-accept.mjs b/skill/scripts/live-accept.mjs index 8e80ff42e..b3a058b91 100644 --- a/skill/scripts/live-accept.mjs +++ b/skill/scripts/live-accept.mjs @@ -16,7 +16,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { isGeneratedFile } from './lib/is-generated.mjs'; -import { getLiveDir, safeSessionId } from './lib/impeccable-paths.mjs'; +import { IMPECCABLE_DIR, getLiveDir, safeSessionId } from './lib/impeccable-paths.mjs'; import { readBuffer as readManualEditsBuffer, writeBuffer as writeManualEditsBuffer } from './live/manual-edits-buffer.mjs'; import { withSourceLockSync } from './live/source-lock.mjs'; import { @@ -34,6 +34,7 @@ import { findSourceArtifactManifest, removeSourceArtifactSession, } from './live/source-artifact.mjs'; +import { removeGenerationArtifacts } from './live/generation-publisher.mjs'; const EXTENSIONS = ['.html', '.jsx', '.tsx', '.vue', '.svelte', '.astro']; const ACCEPT_LOCK_WAIT_MS = 1_000; @@ -157,6 +158,10 @@ Output (JSON): variantId: isDiscard ? null : String(variantNum), result, }); + // The session is over: drop its staged revision artifacts. Leaving them + // behind is what let a later marker search find a decoy instead of real + // source. Only on success, so a failed accept can still be retried. + removeGenerationArtifacts(id, process.cwd()); } console.log(JSON.stringify(result)); }; @@ -1037,6 +1042,21 @@ function detectCommentSyntax(filePath) { // File search (find the file containing session markers) // --------------------------------------------------------------------------- +/** + * `.impeccable` is the critical entry, and it is not cosmetic. + * + * Progressive publication stages each revision as `.impeccable/live/artifacts/ + * -r.`, and those artifacts carry the very marker this search + * looks for. The walk reaches `.` for any project whose source is not under one + * of the privileged dirs above (this repo's own site lives in `site/pages/`), and + * dot-directories sort before letters, so the artifact was found *before* the + * real file. isGeneratedFile then declined the accept, and the agent fell back to + * carbonizing several hundred lines of stylesheet by hand. + * + * Impeccable's own state directory is never project source. Never search it. + */ +const SEARCH_SKIP_DIRS = new Set(['node_modules', '.git', 'dist', 'build', IMPECCABLE_DIR]); + function findSessionFile(id, cwd) { const marker = 'impeccable-variants-start ' + id; const searchDirs = ['src', 'app', 'pages', 'components', 'public', 'views', 'templates', '.']; @@ -1077,7 +1097,7 @@ function searchDir(dir, query, seen, depth) { for (const entry of entries) { if (!entry.isDirectory()) continue; - if (['node_modules', '.git', 'dist', 'build'].includes(entry.name)) continue; + if (SEARCH_SKIP_DIRS.has(entry.name)) continue; const result = searchDir(path.join(dir, entry.name), query, seen, depth + 1); if (result) return result; } diff --git a/skill/scripts/live/generation-publisher.mjs b/skill/scripts/live/generation-publisher.mjs index 2b64895f2..334cc26ba 100644 --- a/skill/scripts/live/generation-publisher.mjs +++ b/skill/scripts/live/generation-publisher.mjs @@ -3,7 +3,7 @@ import path from 'node:path'; import { createHash } from 'node:crypto'; import { createLiveSessionStore } from './session-store.mjs'; import { withSourceLockSync } from './source-lock.mjs'; -import { getLiveDir } from '../lib/impeccable-paths.mjs'; +import { getLiveDir, safeSessionId } from '../lib/impeccable-paths.mjs'; import { SOURCE_ARTIFACT_PREVIEW_MODE, findSourceArtifactManifest, @@ -13,6 +13,28 @@ export function sha256(value) { return createHash('sha256').update(value).digest('hex'); } +/** + * Delete a session's staged revision artifacts. + * + * Nothing used to remove these, and they are the reason a Live accept could + * resolve to the wrong file: `-r.` carries the session marker, + * so it is a decoy for any marker search that walks the project. live-accept no + * longer searches `.impeccable`, but the artifacts should not outlive the session + * they belong to either. Called on accept and discard. + */ +export function removeGenerationArtifacts(id, cwd = process.cwd()) { + let removed = 0; + try { safeSessionId(id); } catch { return removed; } + const artifactDir = path.join(getLiveDir(cwd), 'artifacts'); + let entries; + try { entries = fs.readdirSync(artifactDir); } catch { return removed; } + for (const name of entries) { + if (!name.startsWith(id + '-r')) continue; + try { fs.rmSync(path.join(artifactDir, name), { force: true }); removed += 1; } catch { /* best effort */ } + } + return removed; +} + export function prepareGenerationArtifact({ id, sourceFile, cwd = process.cwd() } = {}) { if (!id) return failure('missing_session_id'); if (!sourceFile) return failure('missing_file'); diff --git a/tests/live-accept.test.mjs b/tests/live-accept.test.mjs index ef8f20249..3d0b05c7a 100644 --- a/tests/live-accept.test.mjs +++ b/tests/live-accept.test.mjs @@ -31,6 +31,69 @@ 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('retires the session’s staged artifacts and leaves other sessions alone', () => { + seed(); + const dir = join(tmp, '.impeccable', 'live', 'artifacts'); + writeFileSync(join(dir, 'ffff0000-r1.astro'), SOURCE); + runAccept(tmp, ['--id', 'ab12cd34', '--variant', '1']); + assert.equal(existsSync(join(dir, 'ab12cd34-r1.astro')), false, 'own artifacts must not outlive the session'); + assert.equal(existsSync(join(dir, 'ab12cd34-r3.astro')), false); + assert.equal(existsSync(join(dir, 'ffff0000-r1.astro')), true, 'another session’s artifacts must survive'); + }); + + 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-')); }); diff --git a/tests/live-reference.test.mjs b/tests/live-reference.test.mjs index 92b2392af..2506a67a2 100644 --- a/tests/live-reference.test.mjs +++ b/tests/live-reference.test.mjs @@ -1,6 +1,6 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; -import { readFileSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { compileProviderBlocks } from '../scripts/lib/utils.js'; import { PROVIDERS } from '../scripts/lib/transformers/providers.js'; @@ -37,7 +37,6 @@ describe('live reference authoring contract', () => { it('keeps the live prompt focused on the foreground poll loop', () => { const liveMd = readFileSync(join(ROOT, 'skill/reference/live.md'), 'utf-8'); - const generationAgentMd = readFileSync(join(ROOT, 'skill/agents/impeccable-live-generator.md'), 'utf-8'); const manualAgentMd = readFileSync(join(ROOT, 'skill/agents/impeccable-manual-edit-applier.md'), 'utf-8'); const openingContract = liveMd.split('\n').slice(0, 60).join('\n'); @@ -65,21 +64,25 @@ describe('live reference authoring contract', () => { assert.match(liveMd, /delegate source edits to `impeccable_manual_edit_applier`/); assert.match(liveMd, /The subagent must not poll or reply/); assert.match(liveMd, /parent live thread keeps the foreground poll loop/); - assert.match(liveMd, /delegate to the low-effort `impeccable_live_generator` agent/); - assert.match(liveMd, /Do not paste this full reference into the handoff/); - assert.match(generationAgentMd, /codex-name: impeccable_live_generator/); - assert.match(generationAgentMd, /effort: low/); - // The generator ships to every harness with an agent format, not just Codex: - // Codex delegates to unblock its foreground poll, Claude Code delegates to - // keep a long session's screenshots and variant CSS out of the main context. + // Generation stays in the main thread on every harness. The generator subagent + // was removed after the first real Claude Code run: the parent has to + // hand-compress the design system into the handoff, and compression is lossy. + // It shipped 0 `var(--token)` uses and 22 raw oklch literals, violating its own + // "never invent raw colors" rule, then needed hundreds of lines of hand + // carbonize to repair. The parent's context is the job, not overhead. assert.doesNotMatch( - generationAgentMd, - /^providers:/m, - 'the live generator must not be gated to one harness', + liveMd, + /impeccable[-_]live[-_]generator/, + 'live generation must not be delegated to a subagent', ); - assert.match(generationAgentMd, /Never poll, Accept, Discard/); - assert.match(generationAgentMd, /Publish the first reviewable result/); - assert.match(generationAgentMd, /preserve every already-published variant byte-for-byte/i); + assert.equal( + existsSync(join(ROOT, 'skill/agents/impeccable-live-generator.md')), + false, + 'the live generator agent must not come back without the context problem being solved', + ); + // Copy edits keep their subagent: applying a known set of ops to a named file + // is self-contained work, so an isolated context costs nothing. + assert.match(manualAgentMd, /codex-name: impeccable_manual_edit_applier/); assert.match(liveMd, /live-accept\.mjs --page-url PAGE_URL/); assert.match(liveMd, /If `repair` is present/); assert.match(liveMd, /Fix the current source/);