From 6c7f7b5cc001848e1ce32be57be206dc2adcd434 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Tue, 28 Jul 2026 18:17:50 -0700 Subject: [PATCH] fix: scope each keys during restore and fail loudly on an unenterable app root Two review findings: - restoreSvelteMarkup visited an {#each} key with outer scopes only, so a contract prop sharing a loop binding name rewrote the key: with prop name -> user.name and loop context "name", the key (name.id) became (user.name.id) in the accepted route. The key evaluates per item, so it is now visited with the loop context and index bound. Regression test verified failing on the previous code. - enterLiveRoot silently kept the ambient working directory when the resolved appRoot no longer existed or chdir failed, letting a helper derive server, session, and source paths from the wrong project. Both cases now exit with a clear error naming the app root and the --target escape hatch. AI-assisted (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/live/roots.mjs | 18 ++++++++++++++++-- skill/scripts/live/svelte-ast.mjs | 5 ++++- tests/live-svelte-ast.test.mjs | 22 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/skill/scripts/live/roots.mjs b/skill/scripts/live/roots.mjs index a34b68c1e..1e27d9a6c 100644 --- a/skill/scripts/live/roots.mjs +++ b/skill/scripts/live/roots.mjs @@ -487,8 +487,22 @@ export function enterLiveRoot(cwd = process.cwd()) { const resolved = resolveLiveRoots(cwd, targetPath ? { targetPath } : {}); if (!resolved.manifest) return null; const appRoot = resolved.manifest.appRoot; - if (path.resolve(cwd) !== path.resolve(appRoot) && isDir(appRoot)) { - try { process.chdir(appRoot); } catch { /* keep current cwd */ } + if (path.resolve(cwd) !== path.resolve(appRoot)) { + // Failing to land on the resolved appRoot must be fatal: a helper that + // silently keeps its ambient cwd derives server, session, and source + // paths from a different project and mutates the wrong state. A manifest + // pointing at a deleted directory is stale ambient truth, not a reason + // to guess. + if (!isDir(appRoot)) { + console.error(`[impeccable live] resolved app root does not exist: ${appRoot} (stale roots manifest? re-run the live boot, or pass --target )`); + process.exit(1); + } + try { + process.chdir(appRoot); + } catch (err) { + console.error(`[impeccable live] could not enter app root ${appRoot}: ${err.message}`); + process.exit(1); + } } return resolved.manifest; } diff --git a/skill/scripts/live/svelte-ast.mjs b/skill/scripts/live/svelte-ast.mjs index 72733cbfd..06e18b62e 100644 --- a/skill/scripts/live/svelte-ast.mjs +++ b/skill/scripts/live/svelte-ast.mjs @@ -844,10 +844,13 @@ export function restoreSvelteMarkup(markup, contract, parse) { break; case 'EachBlock': { visitExpr(node.expression, nextScopes); - if (node.key) visitExpr(node.key, nextScopes); const bound = new Set(); if (node.context) collectPatternNames(node.context, bound); if (node.index) bound.add(node.index); + // The key evaluates per item, so the loop context and index are in + // scope there. Visiting it with outer scopes only let a contract + // prop that shares a loop binding's name rewrite the key. + if (node.key) visitExpr(node.key, [...nextScopes, bound]); walk(node.body, [...nextScopes, bound]); if (node.fallback) walk(node.fallback, nextScopes); break; diff --git a/tests/live-svelte-ast.test.mjs b/tests/live-svelte-ast.test.mjs index 31f0bd020..736cf5a80 100644 --- a/tests/live-svelte-ast.test.mjs +++ b/tests/live-svelte-ast.test.mjs @@ -294,3 +294,25 @@ describe('review regressions: attribute slots and hydration honesty', () => { assert.match(res.reason, /mixing loop and outer identifiers/); }); }); + +describe('review regressions: each-key restore scoping', () => { + it('leaves a key that reads the loop binding alone when a prop shares its name', () => { + // The corruption shape: prop `name` maps back to `user.name`, and the + // loop context is ALSO called `name`. The key evaluates per item, so its + // `name` is the loop binding, never the prop; restoring it used to write + // `(user.name.id)` into the route. + const contract = [{ prop: 'name', expr: 'user.name', kind: 'text' }]; + const markup = `

{name}

+
    + {#each people as name (name.id)} +
  • {name.first}
  • + {/each} +
`; + const restored = restoreSvelteMarkup(markup, contract, parse); + assert.equal(restored.ok, true, restored.reason); + assert.match(restored.markup, /

\{user\.name\}<\/p>/, 'free usage restores to the expression'); + assert.match(restored.markup, /\(name\.id\)/, 'the key keeps the loop binding'); + assert.doesNotMatch(restored.markup, /\(user\.name\.id\)/); + assert.match(restored.markup, /\{name\.first\}/, 'the body keeps the loop binding'); + }); +});