mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 08:36:25 +03:00
fix: stop cross-project live session leakage and stale-adapter 401s
Field session on a nested SvelteKit app surfaced a self-reinforcing leak: localStorage is per-origin, two projects reused 127.0.0.1:5174, and a React project's leftover cycling session was resumed inside the Svelte project. Its checkpoints then materialized a ghost session in the new project's durable store that kept reattaching after every discard, and a stale adapter module 401'd on live.js, hiding the picker. Four fixes: - Server: only session-creating events (generate, steer) may mint a journal. Progress events (checkpoints, mount acks, accept/discard) for unknown ids are refused with 404 unknown_session and never enqueued, so foreign browser state cannot create ghost sessions. Browser sends are gated so progress never overtakes its own creating POST (the Go-time checkpoint and generate are concurrent fetches; the first sweep caught the out-of-order arrival breaking every SvelteKit flow). Steer checkpoints now follow the steer event for the same reason. - Browser: saved sessions carry the server's appRoot; a session stamped by another project is dropped at load time. Unstamped legacy state is caught by the unknown_session refusal, which clears local state and re-arms the picker with an explanatory toast. - SvelteKit adapter: the layout import carries a token-derived revision query so a helper restart changes the module specifier and no Vite client/SSR cache can serve an adapter with a rotated-out token; live-inject --port reads the running helper's token from server.json instead of writing an unauthenticated live.js URL; script load failures log an actionable console error; and adapter removal is byte-exact (the old regex swallowed the next line's indentation). - live.mjs resolves surface briefs from appRoot, then contextRoot, then repoRoot, matching context.mjs in nested-app repos. Tests: server unknown-session rejection units, adapter revision/ byte-exact-removal units, and a foreign-session e2e scenario that seeds another project's localStorage state and asserts it is cleared, no ghost journal materializes, and picking still works. AI-assisted (Claude Code). Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
69456364b2
commit
a83d767cf9
@@ -21,6 +21,7 @@
|
||||
|
||||
const TOKEN = window.__IMPECCABLE_TOKEN__;
|
||||
const PORT = window.__IMPECCABLE_PORT__;
|
||||
const APP_ROOT = window.__IMPECCABLE_APP_ROOT__ || null;
|
||||
if (!TOKEN || !PORT) {
|
||||
window.__IMPECCABLE_LIVE_INIT__ = false; // reset so the real load can init
|
||||
return;
|
||||
@@ -7132,6 +7133,13 @@
|
||||
if (currentSessionId) saveSession();
|
||||
}
|
||||
|
||||
// Progress events must never overtake the event that CREATES their session:
|
||||
// the Go-time checkpoint and the generate POST are concurrent fetches, and
|
||||
// when the checkpoint lands first the server rightly refuses it as
|
||||
// unknown_session — which must mean "foreign leftovers", not "you raced
|
||||
// your own Go click". The gate serializes creation before progress.
|
||||
let sessionCreationGate = Promise.resolve();
|
||||
|
||||
function sendEvent(msg, opts) {
|
||||
msg.token = TOKEN;
|
||||
function handleFailure(err) {
|
||||
@@ -7142,15 +7150,42 @@
|
||||
console.debug('[impeccable] Dropped optional live event:', err);
|
||||
return null;
|
||||
}
|
||||
return fetch('http://localhost:' + PORT + '/events', {
|
||||
const doSend = () => fetch('http://localhost:' + PORT + '/events', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(msg),
|
||||
}).then(async res => {
|
||||
if (res.ok) return res;
|
||||
const body = await res.json().catch(() => ({}));
|
||||
// The server refused to journal progress for a session it has never
|
||||
// seen: this browser is carrying state from another project or a
|
||||
// wiped store (two apps sharing a localhost port). Continuing to
|
||||
// report it would freeze the picker behind a session that can never
|
||||
// complete, so drop the local state and hand the surface back.
|
||||
if (body.error === 'unknown_session' && msg.type === 'checkpoint'
|
||||
&& msg.id && msg.id === currentSessionId) {
|
||||
abandonForeignSession(msg.id);
|
||||
return null;
|
||||
}
|
||||
return handleFailure(new Error(body.error || ('HTTP ' + res.status + ' ' + res.statusText)));
|
||||
}).catch(handleFailure);
|
||||
|
||||
if (msg.type === 'generate' || msg.type === 'steer') {
|
||||
const creation = doSend();
|
||||
sessionCreationGate = creation.then(() => {}, () => {});
|
||||
return creation;
|
||||
}
|
||||
return sessionCreationGate.then(doSend);
|
||||
}
|
||||
|
||||
let abandonedForeignSessionId = null;
|
||||
function abandonForeignSession(sessionId) {
|
||||
if (abandonedForeignSessionId === sessionId || sessionId !== currentSessionId) return;
|
||||
abandonedForeignSessionId = sessionId;
|
||||
console.warn('[impeccable] The live server has no record of session ' + sessionId + '; clearing stale local state.');
|
||||
markSessionHandled();
|
||||
cleanup({ instantChrome: true });
|
||||
showToast('A saved live session belonged to a different project, so it was cleared. Pick an element to start fresh.', 6000);
|
||||
}
|
||||
|
||||
function checkpointPayload(reason) {
|
||||
@@ -8894,6 +8929,7 @@ void main() {
|
||||
// it here would overwrite the Go-time value every time state changes.
|
||||
sessionState.saveSession({
|
||||
id: currentSessionId,
|
||||
appRoot: APP_ROOT || undefined,
|
||||
state,
|
||||
action: selectedAction,
|
||||
count: selectedCount,
|
||||
@@ -8915,7 +8951,17 @@ void main() {
|
||||
}
|
||||
|
||||
function loadSession() {
|
||||
return sessionState.loadSession();
|
||||
const saved = sessionState.loadSession();
|
||||
// localStorage is per-origin, and two projects routinely reuse the same
|
||||
// localhost port. A saved session stamped with another project's appRoot
|
||||
// is that project's leftover, never a session this server can complete;
|
||||
// resuming it freezes the picker behind an unfinishable banner.
|
||||
if (saved?.appRoot && APP_ROOT && saved.appRoot !== APP_ROOT) {
|
||||
console.warn('[impeccable] Ignoring saved live session from another project (' + saved.appRoot + ').');
|
||||
sessionState.clearSession();
|
||||
return null;
|
||||
}
|
||||
return saved;
|
||||
}
|
||||
|
||||
function clearSession() {
|
||||
@@ -10205,10 +10251,11 @@ void main() {
|
||||
const id = id8();
|
||||
steerRequestId = id;
|
||||
steerPendingMessage = text;
|
||||
if (steerInputWasFocused) sendSteerCheckpoint(id, 'steer_input_focused', { focused: true });
|
||||
lockSteerChat();
|
||||
scheduleSteerAwaitTimeout(id);
|
||||
sendSteerCheckpoint(id, 'steer_submitted', { message: text, pageUrl: location.href });
|
||||
// Checkpoints follow the steer event, never precede it: the steer event
|
||||
// is what creates the session journal server-side, and a checkpoint for
|
||||
// a not-yet-created session is rejected as unknown_session.
|
||||
sendEvent({
|
||||
type: 'steer',
|
||||
id,
|
||||
@@ -10216,9 +10263,11 @@ void main() {
|
||||
pageUrl: location.href,
|
||||
}).then((res) => {
|
||||
if (!res) {
|
||||
sendSteerCheckpoint(id, 'steer_send_failed', { message: text });
|
||||
unlockSteerChat({ error: 'Could not reach live server', restoreMessage: text });
|
||||
return;
|
||||
}
|
||||
if (steerInputWasFocused) sendSteerCheckpoint(id, 'steer_input_focused', { focused: true });
|
||||
sendSteerCheckpoint(id, 'steer_submitted', { message: text, pageUrl: location.href });
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -205,9 +205,19 @@ Output (JSON):
|
||||
process.exit(1);
|
||||
}
|
||||
// Optional server token: appended to the /live.js src so the token-gated
|
||||
// /live.js handler authorizes the browser fetch. `live.mjs` always passes it.
|
||||
// /live.js handler authorizes the browser fetch. `live.mjs` always passes
|
||||
// it; a manual `--port`-only invocation reads the running helper's token
|
||||
// from server.json instead of writing an unauthenticated URL that 401s.
|
||||
const tokenIdx = args.indexOf('--token');
|
||||
const token = tokenIdx !== -1 ? args[tokenIdx + 1] : undefined;
|
||||
let token = tokenIdx !== -1 ? args[tokenIdx + 1] : undefined;
|
||||
if (!token) {
|
||||
try {
|
||||
const info = JSON.parse(fs.readFileSync(path.join(cwd, '.impeccable', 'live', 'server.json'), 'utf-8'));
|
||||
// A record for a DIFFERENT port is a stale or foreign helper; its token
|
||||
// would 401 just the same, so only adopt a matching one.
|
||||
if (info?.token && Number(info.port) === port) token = info.token;
|
||||
} catch { /* no running helper recorded; keep legacy tokenless behavior */ }
|
||||
}
|
||||
|
||||
// Reconcile before writing anything. Artifacts this run is about to own are
|
||||
// kept (so a repeat inject stays byte-idempotent); artifacts left behind by
|
||||
|
||||
@@ -91,6 +91,12 @@ function resolveProjectContext() {
|
||||
}
|
||||
const DEFAULT_POLL_TIMEOUT = 600_000; // 10 min — agent re-polls on timeout anyway
|
||||
const SSE_HEARTBEAT_INTERVAL = 30_000; // keepalive ping every 30s
|
||||
|
||||
// The browser events allowed to mint a NEW session journal. `generate` starts
|
||||
// a variant session at Go; `steer` mints its own request id. Every other
|
||||
// id-carrying event must land on an existing session (see the unknown_session
|
||||
// gate in the /events handler).
|
||||
const SESSION_CREATING_EVENT_TYPES = new Set(['generate', 'steer']);
|
||||
// The browser checkpoints for several unrelated reasons (see checkpointPayload
|
||||
// in live-browser.js). Only these two report that variant availability changed,
|
||||
// and only they may drive variant_progress / the *_reviewable phases.
|
||||
@@ -730,6 +736,7 @@ function createRequestHandler({ detectScript, liveScriptParts }) {
|
||||
port: state.port,
|
||||
vocabulary: LIVE_COMMANDS,
|
||||
commandPrefix: IMPECCABLE_COMMAND_PREFIX,
|
||||
appRoot: process.cwd(),
|
||||
parts,
|
||||
});
|
||||
res.writeHead(200, {
|
||||
@@ -1020,6 +1027,20 @@ function createRequestHandler({ detectScript, liveScriptParts }) {
|
||||
res.end(JSON.stringify({ ok: true }));
|
||||
return;
|
||||
}
|
||||
// Only the events that START a session may create its journal.
|
||||
// Everything else (checkpoints, mount acks, accept/discard) must
|
||||
// reference a session THIS store already knows: appendEvent creates a
|
||||
// journal for any id it is handed, so without this gate a browser
|
||||
// resuming another project's session from per-origin storage (two
|
||||
// apps sharing a localhost port) materializes a ghost session here
|
||||
// that keeps reattaching after every discard.
|
||||
if (msg.id && state.sessionStore
|
||||
&& !SESSION_CREATING_EVENT_TYPES.has(msg.type)
|
||||
&& !state.sessionStore.has(msg.id)) {
|
||||
res.writeHead(404, { 'Content-Type': 'application/json' });
|
||||
res.end(JSON.stringify({ error: 'unknown_session', id: msg.id }));
|
||||
return;
|
||||
}
|
||||
const missedCompletion = detectMissedGenerationCompletion(msg);
|
||||
if (state.sessionStore && msg.id) {
|
||||
try {
|
||||
|
||||
+10
-2
@@ -169,12 +169,20 @@ The agent should then:
|
||||
let surfaceBrief = null;
|
||||
let surfaceBriefPath = null;
|
||||
try {
|
||||
const resolvedBrief = resolveSurfaceBrief(roots.appRoot, liveTarget.absoluteTargetPath || null);
|
||||
if (resolvedBrief?.brief) {
|
||||
// Briefs live under .impeccable/surfaces, which in a nested-app repo sits
|
||||
// at the CONTEXT or repo root, not the app root; context.mjs already finds
|
||||
// them there, and live must not report "no brief" for the same project.
|
||||
const briefRoots = [roots.appRoot, roots.contextRoot, roots.repoRoot]
|
||||
.filter(Boolean)
|
||||
.filter((dir, i, arr) => arr.findIndex((other) => path.resolve(other) === path.resolve(dir)) === i);
|
||||
for (const briefRoot of briefRoots) {
|
||||
const resolvedBrief = resolveSurfaceBrief(briefRoot, liveTarget.absoluteTargetPath || null);
|
||||
if (!resolvedBrief?.brief) continue;
|
||||
surfaceBrief = resolvedBrief.brief.text ?? safeRead(resolvedBrief.brief.path);
|
||||
surfaceBriefPath = resolvedBrief.brief.path
|
||||
? path.relative(liveTarget.originalCwd, resolvedBrief.brief.path)
|
||||
: null;
|
||||
break;
|
||||
}
|
||||
} catch { /* briefs are optional context */ }
|
||||
console.log(JSON.stringify({
|
||||
|
||||
@@ -32,10 +32,15 @@ export function readLiveBrowserScriptParts(parts, readFile = (filePath) => fs.re
|
||||
}));
|
||||
}
|
||||
|
||||
export function assembleLiveBrowserScript({ token, port, vocabulary, commandPrefix = '/', parts }) {
|
||||
export function assembleLiveBrowserScript({ token, port, vocabulary, commandPrefix = '/', appRoot = null, parts }) {
|
||||
const prelude =
|
||||
`window.__IMPECCABLE_TOKEN__ = '${token}';\n` +
|
||||
`window.__IMPECCABLE_PORT__ = ${port};\n` +
|
||||
// Project identity for browser-side session storage. localStorage is
|
||||
// keyed by ORIGIN, and two projects routinely share a localhost port
|
||||
// across time; saved sessions carry this value so a resume can tell a
|
||||
// foreign project's leftovers from its own.
|
||||
`window.__IMPECCABLE_APP_ROOT__ = ${JSON.stringify(appRoot)};\n` +
|
||||
`window.__IMPECCABLE_COMMAND_PREFIX__ = ${JSON.stringify(commandPrefix)};\n` +
|
||||
// Canonical command vocabulary (values + labels + icons). live-browser.js
|
||||
// builds its action picker from this instead of an inline copy.
|
||||
|
||||
@@ -119,6 +119,18 @@ export function createLiveSessionStore({ cwd = process.cwd(), sessionId } = {})
|
||||
persist(normalized.id, next, prior.nextSeq + 1);
|
||||
return next;
|
||||
},
|
||||
/**
|
||||
* True when a journal exists for the id in either root. appendEvent
|
||||
* CREATES a journal for any id it is handed, so callers that should only
|
||||
* ever touch existing sessions (browser checkpoints, mount acks) check
|
||||
* here first — otherwise a stale id from another project's browser
|
||||
* storage materializes a ghost session in this store.
|
||||
*/
|
||||
has(id) {
|
||||
if (!id || typeof id !== 'string') return false;
|
||||
return fs.existsSync(getJournalPath(rootDir, id))
|
||||
|| fs.existsSync(getJournalPath(legacyRootDir, id));
|
||||
},
|
||||
/**
|
||||
* Read-only. `live-status` and `live-resume` call this against a session a
|
||||
* running server owns; writing the snapshot file here made every read a
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
* actual live UI remains the shared plain-DOM browser chrome.
|
||||
*/
|
||||
|
||||
import crypto from 'node:crypto';
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
@@ -14,6 +15,28 @@ export const SVELTE_LIVE_ROOT_COMPONENT = 'src/lib/impeccable/ImpeccableLiveRoot
|
||||
export const SVELTE_LAYOUT_MARKER_OPEN = '<!-- impeccable-live-svelte-start -->';
|
||||
export const SVELTE_LAYOUT_MARKER_CLOSE = '<!-- impeccable-live-svelte-end -->';
|
||||
export const SVELTE_ROOT_IMPORT = "import ImpeccableLiveRoot from '$lib/impeccable/ImpeccableLiveRoot.svelte';";
|
||||
// Matches the import at ANY revision (or none). [ \t]* bounds only, never
|
||||
// \s*: a greedy \s* after the statement swallowed the next line's
|
||||
// indentation on removal, leaving a formatting scar in user layouts.
|
||||
const SVELTE_ROOT_IMPORT_LINE_RE = /^[ \t]*import ImpeccableLiveRoot from '\$lib\/impeccable\/ImpeccableLiveRoot\.svelte(?:\?[^']*)?';[ \t]*\r?\n?/gm;
|
||||
|
||||
/**
|
||||
* The import specifier carries a token-derived revision query. The adapter
|
||||
* component embeds the helper token, and Vite (client AND SSR) can keep
|
||||
* serving a stale compiled module after the file is rewritten on a helper
|
||||
* restart; the browser then requests /live.js with a rotated-out token and
|
||||
* gets a 401 with no picker. A changed specifier is a different module id,
|
||||
* which no cache survives.
|
||||
*/
|
||||
export function svelteRootImportLine(rev) {
|
||||
if (!rev) return SVELTE_ROOT_IMPORT;
|
||||
return "import ImpeccableLiveRoot from '$lib/impeccable/ImpeccableLiveRoot.svelte?impeccable-live=" + rev + "';";
|
||||
}
|
||||
|
||||
export function svelteAdapterRev(token) {
|
||||
if (!token) return null;
|
||||
return crypto.createHash('sha256').update(String(token)).digest('hex').slice(0, 8);
|
||||
}
|
||||
|
||||
export function detectSvelteKitProject(cwd = process.cwd(), config = null) {
|
||||
const appHtml = findSvelteKitAppHtml(cwd, config);
|
||||
@@ -50,7 +73,7 @@ export function applySvelteKitLiveAdapter({ cwd = process.cwd(), port, token, co
|
||||
fs.mkdirSync(path.dirname(layoutAbs), { recursive: true });
|
||||
const layoutExisted = fs.existsSync(layoutAbs);
|
||||
const before = layoutExisted ? fs.readFileSync(layoutAbs, 'utf-8') : defaultSvelteLayout();
|
||||
const after = patchSvelteLayout(before);
|
||||
const after = patchSvelteLayout(before, { rev: svelteAdapterRev(token) });
|
||||
fs.writeFileSync(layoutAbs, after, 'utf-8');
|
||||
|
||||
return {
|
||||
@@ -94,15 +117,27 @@ export function removeSvelteKitLiveAdapter({ cwd = process.cwd(), config = null
|
||||
};
|
||||
}
|
||||
|
||||
export function patchSvelteLayout(content) {
|
||||
export function patchSvelteLayout(content, { rev = null } = {}) {
|
||||
let out = String(content || '');
|
||||
if (!out.includes(SVELTE_ROOT_IMPORT)) {
|
||||
const scriptMatch = out.match(/<script(?:\s[^>]*)?>/i);
|
||||
if (scriptMatch) {
|
||||
const insertAt = scriptMatch.index + scriptMatch[0].length;
|
||||
out = out.slice(0, insertAt) + '\n ' + SVELTE_ROOT_IMPORT + out.slice(insertAt);
|
||||
} else {
|
||||
out = `<script>\n ${SVELTE_ROOT_IMPORT}\n</script>\n\n` + out;
|
||||
const importLine = svelteRootImportLine(rev);
|
||||
if (!out.includes(importLine)) {
|
||||
// An import at an older revision is replaced in place, keeping its
|
||||
// indentation; only a layout with no impeccable import gets an insert.
|
||||
let replaced = false;
|
||||
out = out.replace(SVELTE_ROOT_IMPORT_LINE_RE, (line) => {
|
||||
if (replaced) return '';
|
||||
replaced = true;
|
||||
const indent = (line.match(/^[ \t]*/) || [''])[0];
|
||||
return indent + importLine + '\n';
|
||||
});
|
||||
if (!replaced) {
|
||||
const scriptMatch = out.match(/<script(?:\s[^>]*)?>/i);
|
||||
if (scriptMatch) {
|
||||
const insertAt = scriptMatch.index + scriptMatch[0].length;
|
||||
out = out.slice(0, insertAt) + '\n ' + importLine + out.slice(insertAt);
|
||||
} else {
|
||||
out = `<script>\n ${importLine}\n</script>\n\n` + out;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -131,8 +166,8 @@ export function unpatchSvelteLayout(content) {
|
||||
'g',
|
||||
);
|
||||
out = out.replace(blockRe, '$1');
|
||||
out = out.replace(new RegExp('^\\s*' + escapeRegExp(SVELTE_ROOT_IMPORT) + '\\s*\\n?', 'gm'), '');
|
||||
out = out.replace(/<script>\s*<\/script>\s*\n?/g, '');
|
||||
out = out.replace(SVELTE_ROOT_IMPORT_LINE_RE, '');
|
||||
out = out.replace(/<script>\s*<\/script>[ \t]*\r?\n?/g, '');
|
||||
return out.replace(/\n{3,}/g, '\n\n');
|
||||
}
|
||||
|
||||
@@ -193,6 +228,11 @@ export function buildSvelteLiveRootComponent(port, token) {
|
||||
script.src = LIVE_URL;
|
||||
script.async = true;
|
||||
script.dataset.impeccableLiveScript = 'true';
|
||||
script.onerror = () => console.error(
|
||||
'[impeccable] live.js failed to load from ' + LIVE_URL
|
||||
+ ' (helper down, or the token rotated while a stale adapter module was cached).'
|
||||
+ ' Re-run the live boot, then reload this page.'
|
||||
);
|
||||
document.head.appendChild(script);
|
||||
|
||||
return () => {
|
||||
|
||||
Reference in New Issue
Block a user