mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +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>
106 lines
3.6 KiB
JavaScript
106 lines
3.6 KiB
JavaScript
import fs from 'node:fs';
|
|
import path from 'node:path';
|
|
import { createHash, randomUUID } from 'node:crypto';
|
|
import { getLiveDir, isLiveServerPidReachable } from '../lib/impeccable-paths.mjs';
|
|
|
|
// Only used to retire a lock whose contents we cannot read (empty or truncated
|
|
// by a crash mid-write). A readable lock's fate is decided by its owner's
|
|
// liveness instead, so a slow critical section is never swept.
|
|
const UNREADABLE_LOCK_STALE_MS = 60_000;
|
|
|
|
export function sourceLockPath(file, cwd = process.cwd()) {
|
|
const digest = createHash('sha256').update(path.resolve(cwd, file)).digest('hex').slice(0, 24);
|
|
return path.join(getLiveDir(cwd), 'locks', digest + '.lock');
|
|
}
|
|
|
|
export function withSourceLockSync(file, owner, fn, {
|
|
cwd = process.cwd(),
|
|
waitMs = 0,
|
|
retryMs = 5,
|
|
} = {}) {
|
|
const lockPath = sourceLockPath(file, cwd);
|
|
fs.mkdirSync(path.dirname(lockPath), { recursive: true });
|
|
const deadline = Date.now() + Math.max(0, Number(waitMs) || 0);
|
|
// Identifies this acquisition specifically, so release can tell our own lock
|
|
// from a replacement that some other writer created.
|
|
const token = randomUUID();
|
|
let acquired = false;
|
|
|
|
while (!acquired) {
|
|
clearStaleLock(lockPath);
|
|
let fd;
|
|
try {
|
|
fd = fs.openSync(lockPath, 'wx');
|
|
fs.writeFileSync(fd, JSON.stringify({
|
|
owner,
|
|
token,
|
|
pid: process.pid,
|
|
at: Date.now(),
|
|
file: path.resolve(cwd, file),
|
|
}) + '\n');
|
|
acquired = true;
|
|
} catch (error) {
|
|
if (error?.code !== 'EEXIST') throw error;
|
|
if (Date.now() >= deadline) {
|
|
const locked = new Error('source_locked');
|
|
locked.code = 'SOURCE_LOCKED';
|
|
locked.lockPath = lockPath;
|
|
throw locked;
|
|
}
|
|
sleepSync(Math.max(1, Math.min(Number(retryMs) || 5, deadline - Date.now())));
|
|
} finally {
|
|
try { if (fd !== undefined) fs.closeSync(fd); } catch {}
|
|
}
|
|
}
|
|
|
|
try {
|
|
return fn();
|
|
} finally {
|
|
releaseOwnLock(lockPath, token);
|
|
}
|
|
}
|
|
|
|
function sleepSync(ms) {
|
|
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
|
|
}
|
|
|
|
function readLock(lockPath) {
|
|
try { return JSON.parse(fs.readFileSync(lockPath, 'utf-8')); } catch { return null; }
|
|
}
|
|
|
|
/**
|
|
* Remove the lock only if it is still the one this call created. If a sweeper
|
|
* judged our lock stale and another writer replaced it, unlinking here would
|
|
* end *their* critical section and admit a third writer to the same file.
|
|
*/
|
|
function releaseOwnLock(lockPath, token) {
|
|
const held = readLock(lockPath);
|
|
if (held && held.token !== token) return;
|
|
try { fs.unlinkSync(lockPath); } catch {}
|
|
}
|
|
|
|
/**
|
|
* A lock is stale when its owner is gone, not when it is old.
|
|
*
|
|
* Age alone cuts both ways: it sweeps a live holder whose critical section
|
|
* outran the timeout (a suspended laptop, a stopped process), letting two
|
|
* writers into the same source file, while still making every accept on a
|
|
* crashed holder's file wait out the full timeout. Asking the OS whether the
|
|
* recorded pid is alive answers both correctly: a dead owner releases at once,
|
|
* and a live owner keeps its lock however long it needs.
|
|
*/
|
|
function clearStaleLock(lockPath) {
|
|
const held = readLock(lockPath);
|
|
if (!held) {
|
|
// Unreadable: either a crash truncated it, or we caught the brief window
|
|
// between create and write in a live acquisition. mtime distinguishes them.
|
|
try {
|
|
const stat = fs.statSync(lockPath);
|
|
if (Date.now() - stat.mtimeMs > UNREADABLE_LOCK_STALE_MS) fs.unlinkSync(lockPath);
|
|
} catch { /* gone already */ }
|
|
return;
|
|
}
|
|
if (typeof held.pid === 'number' && isLiveServerPidReachable(held.pid)) return;
|
|
try { fs.unlinkSync(lockPath); } catch {}
|
|
}
|