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 <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-07-28 18:18:05 -07:00
co-authored by Claude Code
parent 0c18cbc9ef
commit 6c7f7b5cc0
3 changed files with 42 additions and 3 deletions
+16 -2
View File
@@ -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 <path>)`);
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;
}
+4 -1
View File
@@ -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;
+22
View File
@@ -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 = `<p>{name}</p>
<ul>
{#each people as name (name.id)}
<li>{name.first}</li>
{/each}
</ul>`;
const restored = restoreSvelteMarkup(markup, contract, parse);
assert.equal(restored.ok, true, restored.reason);
assert.match(restored.markup, /<p>\{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');
});
});