diff --git a/skill/scripts/live-accept.mjs b/skill/scripts/live-accept.mjs index a3d59f253..8e80ff42e 100644 --- a/skill/scripts/live-accept.mjs +++ b/skill/scripts/live-accept.mjs @@ -55,6 +55,28 @@ function operationFailure(err, extra = {}) { return { handled: false, mode: 'error', error: err.message, ...extra }; } +/** + * Mark an unhandled preview-path result as a real failure. + * + * operationFailure only covers results built from a *thrown* error. The accept + * implementations also return `{handled: false, error}` for their own checks + * (variant missing, template empty, original text ambiguous), and those arrived + * without `mode`, so completion.mjs classified them as agent_done and + * reference/live.md routed the agent to "read file, find markers, edit". + * + * That handoff only makes sense for a plain wrapper session, which is the one + * shape with markers in the user's source to edit. Component and isolated + * artifact previews keep the source clean until Accept, so there is nothing to + * hand-edit and an unhandled result is always a failure. `previewMode` is + * exactly that discriminator: only the preview branches set it. + */ +function markPreviewFailure(result) { + if (result?.handled === false && !result.mode && result.previewMode) { + return { ...result, mode: 'error' }; + } + return result; +} + // --------------------------------------------------------------------------- // CLI // --------------------------------------------------------------------------- @@ -127,7 +149,8 @@ Output (JSON): })); return; } - const emitResult = (result) => { + const emitResult = (rawResult) => { + const result = markPreviewFailure(rawResult); if (result?.handled !== false) { writeAcceptReceipt(process.cwd(), id, { operation: requestedOperation, @@ -252,15 +275,13 @@ Output (JSON): { waitMs: ACCEPT_LOCK_WAIT_MS }, ); } catch (err) { - result = { - handled: false, - error: err.message, + result = operationFailure(err, { file: vueComponentManifest.sourceFile, sourceFile: vueComponentManifest.sourceFile, previewMode: 'vue-component', componentDir: vueComponentManifest.componentDir, carbonize: false, - }; + }); } emitResult(result); return; @@ -306,14 +327,12 @@ Output (JSON): { waitMs: ACCEPT_LOCK_WAIT_MS }, ); } catch (err) { - result = { - handled: false, - error: err.message, + result = operationFailure(err, { file: svelteComponentManifest.sourceFile, sourceFile: svelteComponentManifest.sourceFile, previewMode: 'svelte-component', componentDir: svelteComponentManifest.componentDir, - }; + }); } if (result.carbonize) { result.todo = 'REQUIRED before next poll: carbonize cleanup in ' + result.file + '. See reference/live.md "Required after accept".'; diff --git a/skill/scripts/live/completion.mjs b/skill/scripts/live/completion.mjs index 986773066..6779bdb01 100644 --- a/skill/scripts/live/completion.mjs +++ b/skill/scripts/live/completion.mjs @@ -1,9 +1,21 @@ +// A preview whose variants live outside the user's source: component modules or +// an isolated artifact. These keep the real file untouched until Accept, so a +// failed accept leaves nothing in source for the agent to hand-edit and must be +// reported as a failure rather than reference/live.md's manual-cleanup handoff. +// Previously only `svelte-component` was special-cased here, so the same failure +// on a Vue or isolated-artifact preview was acknowledged as a success. +const PREVIEW_MODES_WITHOUT_SOURCE_MARKERS = new Set([ + 'svelte-component', + 'vue-component', + 'source-artifact', +]); + export function completionTypeForAcceptResult(eventType, acceptResult) { if (eventType === 'discard') return acceptResult?.handled === true ? 'discarded' : 'error'; if (acceptResult?.handled === true && acceptResult?.carbonize === true) return 'agent_done'; if (acceptResult?.handled === true) return 'complete'; if (acceptResult?.mode === 'error') return 'error'; - if (eventType === 'accept' && acceptResult?.previewMode === 'svelte-component') return 'error'; + if (eventType === 'accept' && PREVIEW_MODES_WITHOUT_SOURCE_MARKERS.has(acceptResult?.previewMode)) return 'error'; return 'agent_done'; } diff --git a/tests/live-accept.test.mjs b/tests/live-accept.test.mjs index 85eb7d40f..ef8f20249 100644 --- a/tests/live-accept.test.mjs +++ b/tests/live-accept.test.mjs @@ -133,6 +133,33 @@ describe('live-accept — isolated source artifacts', () => { assert.equal(existsSync(join(tmp, session.sessionDir)), false); }); + // reference/live.md routes on `mode`: without it the agent is told "manual + // cleanup: read file, find markers, edit". There are no markers in source for + // an isolated preview, so every failure here must self-describe as mode:error. + it('marks a failed artifact accept as mode:error, not a manual handoff', () => { + scaffold('isolatedmissing'); + const result = runAccept(tmp, ['--id', 'isolatedmissing', '--variant', '9']); + assert.equal(result.handled, false, JSON.stringify(result)); + assert.equal(result.mode, 'error', 'the agent must not be told to hand-edit a source file with no markers'); + assert.equal(result.previewMode, 'source-artifact'); + }); + + it('marks an artifact accept blocked by the source lock as mode:error', () => { + const { original } = scaffold('isolatedlockacc'); + const realTmp = realpathSync(tmp); + const lockPath = sourceLockPath(join(realTmp, 'page.html'), realTmp); + mkdirSync(dirname(lockPath), { recursive: true }); + writeFileSync(lockPath, JSON.stringify({ + owner: 'generation:isolatedlockacc:1', token: 'other', pid: process.pid, at: Date.now(), + }) + '\n'); + + const result = runAccept(tmp, ['--id', 'isolatedlockacc', '--variant', '1']); + assert.equal(result.handled, false, JSON.stringify(result)); + assert.equal(result.mode, 'error'); + assert.equal(result.error, 'source_locked'); + assert.equal(readFileSync(join(tmp, 'page.html'), 'utf-8'), original, 'source must be untouched'); + }); + // Every other discard path (Vue, Svelte, plain wrapper) takes the source lock. // This one deleted the preview bare, so it could pull the artifact out from // under an in-flight publisher instead of serializing behind it. diff --git a/tests/live-completion.test.mjs b/tests/live-completion.test.mjs index 7fc51a742..3292a894e 100644 --- a/tests/live-completion.test.mjs +++ b/tests/live-completion.test.mjs @@ -53,6 +53,29 @@ describe('live completion type classification', () => { ); }); + // Previews whose variants live outside the user's source leave nothing in the + // file to hand-edit, so a failed accept there is a failure, not live.md's + // "read file, find markers, edit" handoff. Only svelte-component was special + // cased, so the identical failure on a Vue or isolated-artifact preview was + // acknowledged as a success and Live continued past it. + for (const previewMode of ['svelte-component', 'vue-component', 'source-artifact']) { + it(`treats a failed ${previewMode} accept as an error, not a manual handoff`, () => { + assert.equal( + completionTypeForAcceptResult('accept', { handled: false, error: 'source_locked', previewMode }), + 'error', + ); + }); + } + + it('still treats a failed plain-wrapper accept as a manual handoff', () => { + // The one shape with editable markers in source. This must not regress into + // an error, or every hand-editable session starts failing the poll loop. + assert.equal( + completionTypeForAcceptResult('accept', { handled: false, error: 'Markers not found' }), + 'agent_done', + ); + }); + it('classifies handled accept/discard and real failures explicitly', () => { assert.equal(completionTypeForAcceptResult('accept', { handled: true }), 'complete'); assert.equal(completionTypeForAcceptResult('discard', { handled: true }), 'discarded');