mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 23:26:39 +03:00
Addresses the review findings on #371, plus several the bots did not catch. All fixes have regression coverage that fails on the prior code. Source corruption: - Vue accept dropped valueless root attrs (disabled, v-cloak) and, worse, rewrote @click="x" as a literal click="x" DOM attribute, because the attr parser was name-anchored and skipped the sigil. Tokenize the whole Vue attr grammar and normalize shorthands so accept round-trips directives. - --variant was interpolated unescaped into a RegExp, so --variant '.*' matched the original block first and reported a successful accept while silently restoring the original. Validate against the digits pattern the browser and the /events schema already enforce. - --id reached path.join unvalidated, so --id ../../../../etc/evil wrote and read receipts outside the project. Hoist the existing safeSessionId check into impeccable-paths and apply it at every id-to-path sink. Accept/lock correctness: - Plain HTML/JSX accept and discard did not catch SOURCE_LOCKED, so contention exited non-zero with empty stdout and the agent got no JSON to retry on. - Lock staleness was mtime-only and never read the pid it records: a holder whose critical section outran 60s had its live lock swept, admitting a second writer to the same file, while a crashed holder blocked accepts for a full 60s. Decide staleness by owner liveness, and release only our own lock. Detector: - isNeutralColor only parses computed color forms, so routing authored CSS through it reported inset 4px 0 0 #000 / black / #e5e7eb as chromatic side-tab stripes. Add an authored-color neutrality test covering hex and named neutrals; the fixture had no literal-color cases at all. - Rule line numbers were off by one for every rule after the first, and commented-out CSS was scanned as live rules. Server: - An error reply carries no sourceEventType, and inferSourceEventType returned undefined, which acknowledgePendingEvent treats as a wildcard: a stale generate worker's failure consumed the user's queued Accept, which then reached no agent and left the browser in SAVING forever. - The generate preflight spawned live-wrap.mjs synchronously inside the request handler, freezing the single-threaded server for the whole scaffold (~7.6s measured on this repo, 15s ceiling) and stalling Accept/Discard/SSE. Make it async, claiming the lease before the first await so no event double-delivers. - Every browser checkpoint was echoed back as variant_progress, so a Tune slider drag remounted the preview under the user's cursor and latched the *_reviewable phases from the wrong trigger. Gate on the reason. Cleanup: - Collapse four divergent benchmark argv parsers into scripts/lib/cli-args.mjs. Three silently misread flags: --iterations 20 benchmarked 5, --agent llm ran the fake agent, --median-target=0.4 used the default threshold. - Drop a snapshot cache this branch made write-only (it grew per session for the server's lifetime and was never read), a dead exported reconcile helper, and the unused deferReply branch. Prepared with AI assistance under maintainer direction. Co-Authored-By: Claude <noreply@anthropic.com>
138 lines
4.3 KiB
JavaScript
138 lines
4.3 KiB
JavaScript
import assert from 'node:assert/strict';
|
|
import test from 'node:test';
|
|
import path from 'node:path';
|
|
|
|
import {
|
|
buildGenerationPreflight,
|
|
runGenerationPreflight,
|
|
} from '../skill/scripts/live/generation-preflight.mjs';
|
|
|
|
const SCRIPTS_DIR = path.resolve('skill/scripts');
|
|
|
|
test('builds a replace preflight from the picker locator', () => {
|
|
const command = buildGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-1',
|
|
count: 3,
|
|
pageUrl: '/pricing',
|
|
element: {
|
|
id: 'hero',
|
|
classes: ['hero', 'hero--dark'],
|
|
tagName: 'SECTION',
|
|
textContent: 'A faster way to ship',
|
|
},
|
|
}, SCRIPTS_DIR);
|
|
|
|
assert.equal(command.mode, 'replace');
|
|
assert.deepEqual(command.args.slice(1), [
|
|
'--id', 'session-1', '--count', '3',
|
|
'--element-id', 'hero',
|
|
'--classes', 'hero hero--dark',
|
|
'--tag', 'SECTION',
|
|
'--text', 'A faster way to ship',
|
|
'--page-url', '/pricing',
|
|
]);
|
|
});
|
|
|
|
test('can request an isolated source preview for dedicated generation', () => {
|
|
const command = buildGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-isolated',
|
|
count: 3,
|
|
element: { classes: ['hero'], tagName: 'SECTION' },
|
|
}, SCRIPTS_DIR, { isolated: true });
|
|
assert.equal(command.mode, 'replace');
|
|
assert.equal(command.args.includes('--isolated'), true);
|
|
});
|
|
|
|
test('builds an insert preflight from the anchor locator', () => {
|
|
const command = buildGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-2',
|
|
count: 2,
|
|
mode: 'insert',
|
|
insert: {
|
|
position: 'before',
|
|
anchor: { classes: ['card'], tagName: 'ARTICLE', textContent: 'Plan' },
|
|
},
|
|
}, SCRIPTS_DIR);
|
|
|
|
assert.equal(command.mode, 'insert');
|
|
assert.deepEqual(command.args.slice(1), [
|
|
'--id', 'session-2', '--count', '2', '--position', 'before',
|
|
'--classes', 'card', '--tag', 'ARTICLE', '--text', 'Plan',
|
|
]);
|
|
});
|
|
|
|
test('returns scaffold metadata without exposing child-process details', async () => {
|
|
const calls = [];
|
|
const result = await runGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-3',
|
|
count: 1,
|
|
element: { classes: ['hero'] },
|
|
}, {
|
|
scriptsDir: SCRIPTS_DIR,
|
|
cwd: '/tmp/example',
|
|
async execFileImpl(file, args, options) {
|
|
calls.push({ file, args, options });
|
|
return { stdout: '{"file":"src/App.jsx","insertLine":12}\n', stderr: '' };
|
|
},
|
|
});
|
|
|
|
assert.equal(result.ok, true);
|
|
assert.deepEqual(result.scaffold, { file: 'src/App.jsx', insertLine: 12 });
|
|
assert.equal(calls[0].file, process.execPath);
|
|
assert.equal(calls[0].options.cwd, '/tmp/example');
|
|
});
|
|
|
|
test('skips preflight when the picker has no source locator', async () => {
|
|
const result = await runGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-4',
|
|
count: 3,
|
|
element: { tagName: 'DIV' },
|
|
}, { scriptsDir: SCRIPTS_DIR });
|
|
|
|
assert.deepEqual(result, { ok: false, skipped: true, reason: 'insufficient_locator' });
|
|
});
|
|
|
|
test('yields to the event loop instead of blocking on the child process', async () => {
|
|
// The server is single-threaded and leases polls through this call. A
|
|
// synchronous spawn froze every other request (Accept, Discard, SSE) for the
|
|
// scaffold's full duration — measured at ~7.6s on a large repo.
|
|
let tickedDuringPreflight = false;
|
|
const pending = runGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-async',
|
|
count: 1,
|
|
element: { classes: ['hero'] },
|
|
}, {
|
|
scriptsDir: SCRIPTS_DIR,
|
|
execFileImpl: () => new Promise((resolve) => {
|
|
setTimeout(() => resolve({ stdout: '{"file":"src/App.jsx"}\n', stderr: '' }), 25);
|
|
}),
|
|
});
|
|
setTimeout(() => { tickedDuringPreflight = true; }, 5);
|
|
const result = await pending;
|
|
assert.equal(result.ok, true);
|
|
assert.equal(tickedDuringPreflight, true, 'the event loop must stay responsive during preflight');
|
|
});
|
|
|
|
test('reports a child-process failure without leaking internals or throwing', async () => {
|
|
const error = new Error('spawn failed');
|
|
error.stderr = 'live-wrap.mjs: element not found\n';
|
|
const result = await runGenerationPreflight({
|
|
type: 'generate',
|
|
id: 'session-fail',
|
|
count: 1,
|
|
element: { classes: ['hero'] },
|
|
}, {
|
|
scriptsDir: SCRIPTS_DIR,
|
|
execFileImpl: () => Promise.reject(error),
|
|
});
|
|
assert.equal(result.ok, false);
|
|
assert.equal(result.error, 'live-wrap.mjs: element not found');
|
|
assert.ok(typeof result.durationMs === 'number');
|
|
});
|