skills/pbakaus_impeccable
1
0
Fork 0
mirror of https://github.com/pbakaus/impeccable.git synced 2026-09-21 18:47:02 +03:00
Code Issues Packages Projects Releases Wiki Activity

Gate the agent_done marker release to carbonize; hedge the failure toast

Browse Source 
Cursor Bugbot caught a real hole: accept unlocks at the first variant,
so a late generation agent_done for the same session id could arrive
after Accept and close the awaited failure window early, reopening the
exact #384 gap. The SSE broadcast carries no sourceEventType, so only a
carbonize agent_done is provably accept-side; the release is now gated
on it. Copilot's wording point led somewhere real too: a carbonize-phase
failure raises the same error after the source WAS promoted, so the
toast now says "may not have been saved" and normalizes the server
message's terminal punctuation. Regression guard extended to pin both.

Prepared with AI assistance (Claude Code), directed by @pbakaus.

Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-07-25 20:15:05 -07:00
co-authored by Claude Code
parent f9ea2f0de0
commit d0c5558960
2 changed files with 24 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
skill/scripts/live-browser.js
+12 -5
View File
@@ -6490,10 +6490,12 @@
// The deterministic accept has already committed the reviewed DOM
// and fenced generation. Carbonize may continue in the background;
// it must not hold the foreground picker hostage.
// Any agent_done naming the awaited id post-accept is accept-side
// work reporting back (accept ack or carbonize), so the awaited
// failure window closes here too.
if (awaitingAcceptResult?.id && msg.id === awaitingAcceptResult.id) awaitingAcceptResult = null;
// Only a carbonize agent_done is provably accept-side: accept
// unlocks at the first variant, so a late generation agent_done
// for the same session id can still arrive after Accept and must
// not close the awaited failure window early (the SSE broadcast
// carries no sourceEventType to tell the two apart).
if (msg.data?.carbonize === true && awaitingAcceptResult?.id && msg.id === awaitingAcceptResult.id) awaitingAcceptResult = null;
if (msg.data?.carbonize === true && maybeCompleteAcceptedSession(msg)) break;
break;
case 'discarded':
@@ -6518,7 +6520,12 @@
if (awaitingAcceptResult?.id && msg.id === awaitingAcceptResult.id) {
awaitingAcceptResult = null;
console.error('[impeccable] Accept failed after teardown:', msg.message);
showToast('Accept failed: ' + msg.message + ' The variant was not saved; pick the element and generate again.', 8000);
// Hedged on purpose: a carbonize-phase failure raises this same
// error after the source WAS promoted, so "was not saved" would
// overclaim. Normalize the server message's terminal punctuation
// so the two sentences don't run together.
const acceptFailDetail = String(msg.message || 'unknown error').trim().replace(/[.!?]?$/, '.');
showToast('Accept failed: ' + acceptFailDetail + ' The variant may not have been saved. If the change is missing, pick the element and generate again.', 8000);
break;
}
if (maybeCompleteSteer(msg)) break;
tests/live-browser-regression.test.mjs
+12 -2
View File
@@ -318,8 +318,18 @@ describe('live-browser.js regression guards', () => {
assert.ok(errorCase, 'expected an SSE case \'error\' handler in live-browser.js');
assert.match(
errorCase[0],
/if \(awaitingAcceptResult\?\.id && msg\.id === awaitingAcceptResult\.id\) \{[\s\S]{0,400}?awaitingAcceptResult = null;[\s\S]{0,400}?not saved[\s\S]{0,200}?break;/,
'an error naming the awaited accept must clear the marker and tell the user the variant was not saved',
/if \(awaitingAcceptResult\?\.id && msg\.id === awaitingAcceptResult\.id\) \{[\s\S]{0,700}?awaitingAcceptResult = null;[\s\S]{0,700}?may not have been saved[\s\S]{0,300}?break;/,
'an error naming the awaited accept must clear the marker and warn that the variant may not have been saved (hedged: a carbonize-phase failure fires this after the source WAS promoted)',
);
// Accept unlocks at the first variant, so a late generation agent_done
// for the same session id can arrive after Accept; only a carbonize
// agent_done is provably accept-side and may close the window.
const agentDoneCase = SOURCE.match(/case 'agent_done':[\s\S]{0,1200}?break;/);
assert.ok(agentDoneCase, 'expected an SSE case \'agent_done\' handler in live-browser.js');
assert.match(
agentDoneCase[0],
/msg\.data\?\.carbonize === true && awaitingAcceptResult\?\.id && msg\.id === awaitingAcceptResult\.id/,
'agent_done must only release the awaited accept marker for carbonize completions, or a late generation agent_done reopens the #384 hole',
);
const cleanupFn = SOURCE.match(/function cleanupAcceptedSession\(\) \{[\s\S]{0,1200}?\n \}/);
assert.ok(cleanupFn, 'expected cleanupAcceptedSession in live-browser.js');