From 39df25ee5a60498b5bb81dfe5d8da2443523d19a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Mon, 27 Jul 2026 16:22:09 -0700 Subject: [PATCH] fix: fourth review round (mount-failure truth, toggle baking, root ambiguity) cursor[bot]: - enqueueEvent dedupes variant_mount_failed per variant, so a second broken variant is no longer swallowed while the first is queued. - Every component (re)injection resets the mount-failure dedupe, so a republish that is still broken at the same URL reports again instead of silently convincing the agent the repair landed. - Toggle baking now mirrors preview truth exactly: the runtime sets data-p-="on" or removes the attribute, so presence and "on" forms survive only while on, and any other valued branch (never matched at preview) is dropped in either state. greptile-apps[bot] (both P1 repros): - When several apps qualify at the same resolution tier (two live servers, or two stopped apps with interrupted sessions), the choice stays deterministic but is now loud: a stderr warning names the chosen app, the alternatives, and how to target a specific app. Silent wrong-app routing was the failure in both repro harnesses. This work was produced with AI assistance (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/live-browser.js | 4 ++++ skill/scripts/live-server.mjs | 11 ++++++++++- skill/scripts/live/accept-css.mjs | 10 +++++++--- skill/scripts/live/roots.mjs | 21 ++++++++++++++++++--- tests/live-accept-css.test.mjs | 12 ++++++++++++ 5 files changed, 51 insertions(+), 7 deletions(-) diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index b95d8b5a8..887ea882c 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -5696,6 +5696,10 @@ } async function injectSvelteComponentsFromManifest(manifestPath, sessionId) { + // Every (re)injection is a fresh attempt: reset the failure dedupe so a + // republish that is STILL broken at the same URL reports again instead of + // being swallowed while the agent believes the repair landed. + lastReportedMountFailure = null; const url = 'http://localhost:' + PORT + '/source?token=' + TOKEN + '&path=' + encodeURIComponent(manifestPath); try { const res = await fetch(url); diff --git a/skill/scripts/live-server.mjs b/skill/scripts/live-server.mjs index 14a207508..29cb32a4b 100644 --- a/skill/scripts/live-server.mjs +++ b/skill/scripts/live-server.mjs @@ -175,7 +175,16 @@ function chatAgentLikelyActive() { const MAX_ANNOTATION_BYTES = 10 * 1024 * 1024; function enqueueEvent(event) { - if (!event || (event.id && state.pendingEvents.some((entry) => entry.event?.id === event.id && entry.event?.type === event.type))) return; + if (!event) return; + // Dedupe by (session, type), except mount failures, which are per-variant: + // variant 2 failing must not be swallowed because variant 1's failure is + // still queued. + const duplicate = event.id && state.pendingEvents.some((entry) => ( + entry.event?.id === event.id + && entry.event?.type === event.type + && (event.type !== 'variant_mount_failed' || entry.event?.variant === event.variant) + )); + if (duplicate) return; state.pendingEvents.push({ event, leaseUntil: 0, seq: state.nextEventSeq++ }); flushPendingPolls(); } diff --git a/skill/scripts/live/accept-css.mjs b/skill/scripts/live/accept-css.mjs index 4f7f1b2fa..74350bc0e 100644 --- a/skill/scripts/live/accept-css.mjs +++ b/skill/scripts/live/accept-css.mjs @@ -334,12 +334,16 @@ export function stripParamSelector(selector, id, kind, chosenValue) { drop = true; return ''; } - // toggle: attribute presence means "on". - if (expected != null && String(expected) !== String(chosenValue) && !isToggleOn(chosenValue)) { + // toggle: the runtime sets data-p-="on" when on and removes the + // attribute when off. A branch survives baking only if it actually + // matched at preview time with the chosen state: the presence form and + // the literal "on" form match while on; every other valued form + // (["false"], ["0"], ...) never matched and is dead regardless of state. + if (expected != null && expected !== 'on') { drop = true; return ''; } - if (expected == null && !isToggleOn(chosenValue)) { + if (!isToggleOn(chosenValue)) { drop = true; return ''; } diff --git a/skill/scripts/live/roots.mjs b/skill/scripts/live/roots.mjs index d324881e4..192367df3 100644 --- a/skill/scripts/live/roots.mjs +++ b/skill/scripts/live/roots.mjs @@ -366,9 +366,24 @@ export function resolveLiveRoots(cwd = process.cwd(), { targetPath = null } = {} .map((entry) => readManifestAt(entry.appRoot)) .filter(Boolean); if (candidates.length > 0) { - const live = candidates.find((manifest) => hasLiveServer(manifest.appRoot)); - const recovering = live || candidates.find((manifest) => hasActiveDurableSession(manifest.appRoot)); - return { manifest: recovering || candidates[0], source: 'pointer' }; + const liveApps = candidates.filter((manifest) => hasLiveServer(manifest.appRoot)); + const recoveringApps = liveApps.length > 0 + ? liveApps + : candidates.filter((manifest) => hasActiveDurableSession(manifest.appRoot)); + const tier = recoveringApps.length > 0 ? recoveringApps : candidates; + // Multiple apps qualifying at the same tier is inherent ambiguity: + // intent is unknowable from the repo root. The choice stays + // deterministic (most recent boot first), but it must be LOUD, not + // silent, so the agent can re-anchor when it meant the other app. + if (tier.length > 1) { + const chosen = tier[0].appRoot; + const others = tier.slice(1).map((manifest) => manifest.appRoot).join(', '); + process.stderr.write( + `[impeccable live] Multiple apps in this repo have live state; using ${chosen}. ` + + `Other candidate(s): ${others}. Run from the app directory (or pass --target) to address a specific app.\n`, + ); + } + return { manifest: tier[0], source: 'pointer' }; } } } diff --git a/tests/live-accept-css.test.mjs b/tests/live-accept-css.test.mjs index 80beaffba..ef2a59539 100644 --- a/tests/live-accept-css.test.mjs +++ b/tests/live-accept-css.test.mjs @@ -197,3 +197,15 @@ describe('review regressions: parser boundaries', () => { assert.deepEqual([...selectors].sort(), ['.a', '.b', '.c']); }); }); + +describe('review regressions: toggle branch truth', () => { + it('drops valued toggle branches that never matched at preview, keeps on-forms only while on', () => { + assert.equal(stripParamSelector('[data-p-flag="false"] .a', 'flag', 'toggle', true), null); + assert.equal(stripParamSelector('[data-p-flag="false"] .a', 'flag', 'toggle', false), null); + assert.equal(stripParamSelector('[data-p-flag="0"] .a', 'flag', 'toggle', false), null); + assert.equal(stripParamSelector('[data-p-flag="on"] .a', 'flag', 'toggle', true), '.a'); + assert.equal(stripParamSelector('[data-p-flag="on"] .a', 'flag', 'toggle', false), null); + assert.equal(stripParamSelector('[data-p-flag] .a', 'flag', 'toggle', true), '.a'); + assert.equal(stripParamSelector('[data-p-flag] .a', 'flag', 'toggle', false), null); + }); +});