mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 00:26:41 +03:00
Fix: checkpoint annotated sessions after creation
Defer generate_started until the server acknowledges the generate event, including screenshot capture/upload. Guard failed and superseded creation. Cover delayed creation in source-function tests and a slow-upload Svelte browser regression without reload recovery. Prepared with AI assistance under maintainer direction.
This commit is contained in:
@@ -7834,7 +7834,6 @@
|
||||
if (editBadgeEl && editBadgeEl.style.display !== 'none') renderEditBadge('idle-disabled');
|
||||
showBar('generating');
|
||||
saveSession();
|
||||
sendCheckpoint('generate_started');
|
||||
writeScrollY(window.scrollY);
|
||||
if (variantObserver) variantObserver.disconnect();
|
||||
variantObserver = startVariantObserver(currentSessionId);
|
||||
@@ -7916,7 +7915,6 @@
|
||||
showBar('generating');
|
||||
startScrollTracking();
|
||||
saveSession();
|
||||
sendCheckpoint('generate_started');
|
||||
writeScrollY(window.scrollY);
|
||||
if (variantObserver) variantObserver.disconnect();
|
||||
variantObserver = startVariantObserver(currentSessionId);
|
||||
@@ -8238,7 +8236,8 @@
|
||||
// rasterization from delaying the fetch itself.
|
||||
if (!hasAnnotations) {
|
||||
basePayload.clientSentAt = Date.now();
|
||||
await sendEvent(basePayload);
|
||||
const created = await sendEvent(basePayload);
|
||||
if (created?.ok && currentSessionId === basePayload.id) sendCheckpoint('generate_started');
|
||||
}
|
||||
|
||||
let screenshotPath;
|
||||
@@ -8279,7 +8278,10 @@
|
||||
// is semantic input. Plain requests were already dispatched above.
|
||||
if (hasAnnotations) {
|
||||
basePayload.clientSentAt = Date.now();
|
||||
sendEvent(screenshotPath ? { ...basePayload, screenshotPath } : basePayload);
|
||||
const created = await sendEvent(screenshotPath ? { ...basePayload, screenshotPath } : basePayload);
|
||||
// Capture/upload can take seconds. Progress before this acknowledgment
|
||||
// refers to an unknown session and would clear our own active work.
|
||||
if (created?.ok && currentSessionId === basePayload.id) sendCheckpoint('generate_started');
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -57,6 +57,12 @@
|
||||
"readyPattern": "Local:\\s+https?://[^:]+:(\\d+)",
|
||||
"readyTimeoutMs": 120000,
|
||||
"steer": false,
|
||||
"liveChrome": {
|
||||
"annotations": {
|
||||
"selector": "h1.hero-title",
|
||||
"uploadDelayMs": 300
|
||||
}
|
||||
},
|
||||
"pickSelector": "ul.expense-list",
|
||||
"pickPosition": {
|
||||
"x": 10,
|
||||
|
||||
@@ -2,12 +2,53 @@ import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { runInNewContext } from 'node:vm';
|
||||
|
||||
const SOURCE = readFileSync(join(process.cwd(), 'skill/scripts/live-browser.js'), 'utf-8');
|
||||
const PENDING_DOCK_POSITION_SOURCE = SOURCE.match(/function positionPendingDock\(\) \{[\s\S]*?\n \}/)?.[0] || '';
|
||||
const CAPTURE_AND_EMIT_SOURCE = SOURCE.match(/async function captureAndEmit\([\s\S]*?\n \}/)?.[0] || '';
|
||||
|
||||
describe('live-browser source contracts', () => {
|
||||
it('does not checkpoint a generation before captureAndEmit creates its session', () => {
|
||||
for (const name of ['handleGo', 'handleInsertCreate']) {
|
||||
const body = SOURCE.match(new RegExp(`function ${name}\\(\\) \\{[\\s\\S]*?\\n \\}`))?.[0];
|
||||
assert.ok(body);
|
||||
assert.doesNotMatch(body, /sendCheckpoint\('generate_started'\)/);
|
||||
}
|
||||
});
|
||||
|
||||
for (const annotated of [false, true]) {
|
||||
for (const outcome of ['created', 'failed', 'superseded']) {
|
||||
it(`${annotated ? 'annotated' : 'plain'} generation checkpoints only its acknowledged current session (${outcome})`, async () => {
|
||||
const capture = Promise.withResolvers();
|
||||
const creation = Promise.withResolvers();
|
||||
const events = [];
|
||||
const context = {
|
||||
currentSessionId: 'session-a', state: 'GENERATING', PORT: 1234, TOKEN: 'test',
|
||||
console, Date,
|
||||
captureElementToBlob: () => capture.promise,
|
||||
showShaderOverlay() {},
|
||||
fetch: async () => ({ ok: true, json: async () => ({ path: '/annotation.png' }) }),
|
||||
sendEvent: async (payload) => { events.push(payload.type); return creation.promise; },
|
||||
sendCheckpoint: (reason) => events.push(reason),
|
||||
};
|
||||
const emit = runInNewContext(`(${CAPTURE_AND_EMIT_SOURCE})`, context);
|
||||
const pending = emit({}, { type: 'generate', id: 'session-a' }, {
|
||||
comments: annotated ? [{ text: 'change title' }] : [], strokes: [],
|
||||
}, {});
|
||||
await new Promise(resolve => setImmediate(resolve));
|
||||
assert.deepEqual(events, annotated ? [] : ['generate']);
|
||||
capture.resolve({ blob: {}, paper: 'white' });
|
||||
await new Promise(resolve => setImmediate(resolve));
|
||||
assert.deepEqual(events, ['generate'], 'capture/upload must not checkpoint before creation is acknowledged');
|
||||
if (outcome === 'superseded') context.currentSessionId = 'session-b';
|
||||
creation.resolve(outcome === 'failed' ? null : { ok: true });
|
||||
await pending;
|
||||
assert.deepEqual(events, outcome === 'created' ? ['generate', 'generate_started'] : ['generate']);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
it('reports foreground poll connectivity without a background worker dependency', () => {
|
||||
assert.match(
|
||||
SOURCE,
|
||||
@@ -29,7 +70,7 @@ describe('live-browser source contracts', () => {
|
||||
);
|
||||
assert.match(
|
||||
CAPTURE_AND_EMIT_SOURCE,
|
||||
/if \(hasAnnotations\) \{[\s\S]*?basePayload\.clientSentAt = Date\.now\(\);\s*sendEvent\(screenshotPath \? \{ \.\.\.basePayload, screenshotPath \} : basePayload\);\s*\}/,
|
||||
/if \(hasAnnotations\) \{[\s\S]*?basePayload\.clientSentAt = Date\.now\(\);\s*const created = await sendEvent\(screenshotPath \? \{ \.\.\.basePayload, screenshotPath \} : basePayload\);/,
|
||||
'annotated generation should dispatch exactly after capture and upload resolve',
|
||||
);
|
||||
});
|
||||
|
||||
+10
-6
@@ -1283,17 +1283,21 @@ for (const { name, fixture } of fixtures) {
|
||||
const pickSelector = annotation.selector || fixture.runtime.pickSelector || 'h1.hero-title';
|
||||
try {
|
||||
await waitForHandshake(page);
|
||||
if (annotation.uploadDelayMs) {
|
||||
await page.route('**/annotation?*', async (route) => {
|
||||
await new Promise(resolve => setTimeout(resolve, annotation.uploadDelayMs));
|
||||
await route.continue();
|
||||
});
|
||||
}
|
||||
if (fixture.runtime.preActions) await runPreActions(page, fixture.runtime.preActions);
|
||||
await pickElement(page, pickSelector, { resetPickMode: true });
|
||||
await drawAnnotationPinAndStroke(page, {
|
||||
comment: annotation.comment || 'Make this selected element easier to scan',
|
||||
});
|
||||
await clickGo(page);
|
||||
await waitForCyclingRobust(page, 3, {
|
||||
agentMode,
|
||||
preActions: fixture.runtime.preActions,
|
||||
log: (m) => t.diagnostic(m),
|
||||
});
|
||||
// A reload would mask a checkpoint-before-creation race by adopting
|
||||
// the session again. Annotated generation must complete in this tab.
|
||||
await waitForCycling(page, 3, { timeout: agentMode === 'llm' ? 180_000 : 30_000 });
|
||||
|
||||
const generateEvent = recordedGenerateEvents.at(-1);
|
||||
await assertAnnotationUploadEvent(generateEvent);
|
||||
@@ -1302,7 +1306,7 @@ for (const { name, fixture } of fixtures) {
|
||||
|
||||
const sourceFile = await locateSessionFile(session.appRoot);
|
||||
const svelteComponentTarget = svelteComponentTargetFor(sourceFile);
|
||||
await clickNext(page);
|
||||
await cycleToVariant(page, 2, 3);
|
||||
assert.equal(await getVisibleVariant(page), 2, 'variant 2 visible after annotated generate');
|
||||
await clickAccept(page, { expectedVariant: 2 });
|
||||
await waitForBarHidden(page);
|
||||
|
||||
@@ -26,7 +26,6 @@ import {
|
||||
clickEditCopy,
|
||||
clickExitLiveMode,
|
||||
clickGo,
|
||||
clickNext,
|
||||
cycleToVariant,
|
||||
clickSaveEdit,
|
||||
drawAnnotationPinAndStroke,
|
||||
@@ -282,7 +281,7 @@ async function runAnnotationGenerateFlow({ page, tmp, evidence }) {
|
||||
const generateEvent = latestJournalEvent(tmp, (event) => event.type === 'generate' && event.screenshotPath);
|
||||
await assertAnnotationUploadEvent(generateEvent);
|
||||
assert.ok(existsSync(generateEvent.screenshotPath), 'annotation screenshot file exists');
|
||||
await clickNext(page);
|
||||
await cycleTo(page, 2);
|
||||
await assertVariantCounter(page, 2, 3);
|
||||
await evidence.capture('annotation-cycle');
|
||||
await clickDiscard(page);
|
||||
|
||||
Reference in New Issue
Block a user