mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-18 09:06:53 +03:00
Drop the live generator subagent; fix the artifact decoy that broke accept
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/<id>-r<n>.<source-ext>`, 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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/<id>-r<n>.<source-ext>`, 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 = [
|
||||
'<main>',
|
||||
'<!-- impeccable-variants-start ab12cd34 -->',
|
||||
'<div data-impeccable-variant="original">ORIGINAL</div>',
|
||||
'<div data-impeccable-variant="1">VARIANT ONE</div>',
|
||||
'<!-- impeccable-variants-end ab12cd34 -->',
|
||||
'</main>',
|
||||
'',
|
||||
].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-')); });
|
||||
|
||||
@@ -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/);
|
||||
|
||||
Reference in New Issue
Block a user