fix(web/shell): #107 review — a command's change to the second of two folders called proj is not written into the first

The runtime tells same-named folders apart (planMounts mounts /a/proj and /b/proj each at its own root), but a
command's change reaches the review as `name/rel` (reviewPathFor), and the review finds a folder by name:
resolveWorkspacePathAsync takes the FIRST folder called proj. So a change to /b/proj/x.txt was compared with the right
file (readNow reads /b/proj/x.txt) and written into /a/proj/x.txt; a new file was created there too. The same holds
for two folders at one path — the scratch folder and a folder from the computer called scratch
(`levelcode-scratch:/scratch`, `file:///scratch`), whose change paths name neither — and for a folder whose name has a slash in it, which
the resolver reads as a path.

The smallest correct fix is to refuse: the review and the file tools address a file by `name/rel`, and teaching them
to take a folder is a change to the review that edit_file and the desktop share. commitShellChanges now refuses a
change in a folder the review would not find again (folderMixUp): one that shares its root with another folder, whose
name has a slash, or that is not the first folder of its name. The whole set is refused, as for every other refusal,
and the note says why ('another workspace folder is also called "proj", and the review finds a folder by its name: it
would apply this change there'). The first folder of a name is the one the review finds, so its changes go through as
before; folders of distinct names, nested ones included, are unchanged.

Tests: test/shellCommit.test.js "two folders the review cannot tell apart" (same name — a write, a delete and a set
with a change in each folder —, same root, a slash in the name; the first folder of a name, a lone folder with a slash,
and distinct nested names still applied). test/shellSeam.test.js "two workspace folders called proj", through the real
commit hook, with a review stand-in that finds the file exactly as reviewSession.applyEdit does
(resolveWorkspacePathAsync, then host.uriFor): on 187fd21 the two changes to b/proj are written to a/proj/new.txt and
a/proj/x.txt; here nothing is written, and a change to a/proj still lands in a/proj. Both fail before and pass after;
removing any one of the four checks fails one of them. (shellSeam's vscode stand-in gains workspace.fs.stat and
FileType, which the resolver's existence check needs.)

Author: ndemianc

extensions/levelcode-ai/shellCommit.js

@@ -18,6 +18,8 @@
  *
  *  What is refused, and why:
  *    - outside the workspace folders                          (a mount is the boundary)
+ *    - in a folder the review would not find again            (two folders called proj, or at one path: the review
+ *                                                              finds a folder by name and would write the other's)
  *    - repository / tool metadata and instruction files       (a script can hide `rm -rf .git`, or write
  *      (in any letter case: .GIT is .git on most disks)        .vscode/settings.json or AGENTS.md, which later
  *                                                              steer the agent with the user's authority)
@@ -96,6 +98,22 @@ function eolStyle(text) {
 /** A lone CR (old Mac) is not a line break the editor keeps: refuse it with the same reason as mixed. */
 function hasLoneCr(text) { return /\r(?!\n)/.test(text); }
 
+/**
+ * The workspace folder an absolute path is in: the deepest one, as host.uriFor picks it. Null when it is in none.
+ * @template {{name:string, root:string}} F
+ * @param {string} abs
+ * @param {F[]} folders
+ * @returns {F|null}
+ */
+function folderOf(abs, folders) {
+	/** @type {F|null} */ let best = null;
+	for (const f of folders) {
+		const inside = abs === f.root || abs.startsWith(f.root === '/' ? '/' : f.root + '/');
+		if (inside && (!best || f.root.length > best.root.length)) { best = f; }
+	}
+	return best;
+}
+
 /**
  * The path a review call takes for an absolute path: relative to its workspace folder, and — with more than
  * one folder — prefixed with the folder's name, which is how resolveWorkspacePath finds the folder
@@ -105,16 +123,32 @@ function hasLoneCr(text) { return /\r(?!\n)/.test(text); }
  * @returns {{ review: string, rel: string }|null} null when `abs` is in none of them
  */
 function reviewPathFor(abs, folders) {
-	let best = null;
-	for (const f of folders) {
-		const inside = abs === f.root || abs.startsWith(f.root === '/' ? '/' : f.root + '/');
-		if (inside && (!best || f.root.length > best.root.length)) { best = f; }
-	}
+	const best = folderOf(abs, folders);
 	if (!best) { return null; }
 	const rel = abs.slice(best.root === '/' ? 1 : best.root.length + 1);
 	return { rel, review: folders.length > 1 ? best.name + '/' + rel : rel };
 }
 
+/**
+ * Why the review could apply a change in `folder` to ANOTHER folder's file, or null when it cannot. The review finds a
+ * file by the path reviewPathFor makes: the folder by the name the path starts with — the first folder of that name, as
+ * resolveWorkspacePath finds it — then the file by its absolute path (host.uriFor: the first folder at that root). So
+ * with two clones called proj (/a/proj, /b/proj) a change to the second was compared with the right file (readNow)
+ * and written into the first. A runtime that tells same-named folders apart (planMounts) hands back paths, and the
+ * review takes names: rather than teach the review — which the file tools share — to take a folder, the change is
+ * refused. The first folder of a name is the one the review finds, so its changes go through as before.
+ * @param {{name:string, root:string}} folder
+ * @param {Array<{name:string, root:string}>} folders
+ * @returns {string|null}
+ */
+function folderMixUp(folder, folders) {
+	if (folders.some((f) => f !== folder && f.root === folder.root)) { return 'two workspace folders are at ' + folder.root + ', and a path cannot say which of them it is in'; }
+	if (folders.length < 2) { return null; }
+	if (folder.name.split(/[\\/]/)[0] !== folder.name) { return 'the name of its workspace folder, "' + folder.name + '", has a slash in it, and the review would read it as a path'; }
+	if (folders.find((f) => f.name === folder.name) !== folder) { return 'another workspace folder is also called "' + folder.name + '", and the review finds a folder by its name: it would apply this change there'; }
+	return null;
+}
+
 /**
  * A name as a disk that ignores case sees it. The usual folder from the computer is on one (APFS on a Mac, NTFS on
  * Windows), and there `.GIT/config` IS `.git/config`: matched as typed, a command wrote the repository's config by
@@ -164,6 +198,8 @@ async function commitShellChanges(changes, ctx, io) {
 	for (const c of files) {
 		const p = reviewPathFor(c.path, folders);
 		if (p === null) { refusals.push({ c, why: 'it is outside the workspace folders' }); continue; }
+		const mixUp = folderMixUp(/** @type {{name:string, root:string}} */ (folderOf(c.path, folders)), folders);
+		if (mixUp) { refusals.push({ c, why: mixUp }); continue; }
 		if (isProtected(p.rel)) { refusals.push({ c, why: 'repository metadata, tool settings and instruction files are not changed by commands — the user does that' }); continue; }
 		const key = c.path.toLowerCase();
 		if (seenLower.has(key) && seenLower.get(key) !== c.path) { refusals.push({ c, why: 'it differs from ' + seenLower.get(key) + ' only in letter case, which a case-insensitive disk cannot keep apart' }); continue; }

extensions/levelcode-ai/test/shellCommit.test.js

@@ -47,6 +47,43 @@ const ids = (out) => out.failed.map((f) => f.c.path).sort();
 		assert.strictEqual(reviewPathFor('/ws/sub/x', [{ name: 'ws', root: '/ws' }, { name: 'sub', root: '/ws/sub' }]).review, 'sub/x', 'the deepest folder wins, as host.uriFor does');
 	});
 
+	await test('two folders the review cannot tell apart: a change it would apply to the other one is refused, and the whole set with it', async () => {
+		// The review finds a folder by the name a path starts with (the first folder of that name, as resolveWorkspacePath
+		// does), then a file by its absolute path (the first folder at that root, as host.uriFor does).
+		const twins = [{ name: 'proj', root: '/a/proj' }, { name: 'proj', root: '/b/proj' }];
+		let r = rig({ '/b/proj/x.txt': 'old', '/a/proj/y.txt': 'y' }, { folders: twins });
+		let out = await commitShellChanges([W('/b/proj/x.txt', 'new', 'old'), W('/a/proj/y.txt', 'y2', 'y')], r.ctx, r.io);
+		assert.strictEqual(out.refused, true);
+		assert.deepStrictEqual(r.edits, [], 'proj/x.txt would have been written into /a/proj');
+		assert.deepStrictEqual(ids(out), ['/b/proj/x.txt']);
+		assert.match(out.notes[0], /refused: proj\/x\.txt — another workspace folder is also called "proj"/);
+		r = rig({ '/b/proj/gone.txt': 'g' }, { folders: twins });
+		assert.strictEqual((await commitShellChanges([D('/b/proj/gone.txt', 'g')], r.ctx, r.io)).refused, true, 'a delete too');
+		assert.deepStrictEqual(r.deletes, []);
+		// The first folder of the name is the one the review finds: its changes are applied, to it.
+		r = rig({ '/a/proj/y.txt': 'y' }, { folders: twins });
+		out = await commitShellChanges([W('/a/proj/y.txt', 'y2', 'y')], r.ctx, r.io);
+		assert.strictEqual(out.refused, false);
+		assert.deepStrictEqual(r.edits, [{ path: 'proj/y.txt', exists: true, proposed: 'y2' }]);
+		// Two folders at one path (the scratch folder, and a folder from the computer called scratch): a path names neither.
+		r = rig({ '/scratch/x.txt': 'old' }, { folders: [{ name: 'scratch', root: '/scratch' }, { name: 'mine', root: '/scratch' }] });
+		out = await commitShellChanges([W('/scratch/x.txt', 'new', 'old')], r.ctx, r.io);
+		assert.strictEqual(out.refused, true);
+		assert.match(out.notes[0], /two workspace folders are at \/scratch/);
+		// A folder whose name has a slash in it: the review reads the name as a path and finds another folder, or none.
+		r = rig({}, { folders: [{ name: 'web', root: '/web' }, { name: 'a/b', root: '/ab' }] });
+		out = await commitShellChanges([W('/ab/x.txt', 'new')], r.ctx, r.io);
+		assert.strictEqual(out.refused, true);
+		assert.match(out.notes[0], /"a\/b", has a slash in it/);
+		r = rig({}, { folders: [{ name: 'a/b', root: '/ab' }] });
+		assert.strictEqual((await commitShellChanges([W('/ab/x.txt', 'new')], r.ctx, r.io)).refused, false, 'alone, a folder is found without its name');
+		// Folders of distinct names, nested ones included, are as they were.
+		r = rig({}, { folders: [{ name: 'ws', root: '/ws' }, { name: 'sub', root: '/ws/sub' }, { name: 'lib', root: '/lib' }] });
+		out = await commitShellChanges([W('/ws/sub/a.txt', 'a'), W('/lib/b.txt', 'b'), W('/ws/c.txt', 'c')], r.ctx, r.io);
+		assert.strictEqual(out.refused, false);
+		assert.deepStrictEqual(r.edits.map((e) => e.path), ['lib/b.txt', 'ws/c.txt', 'sub/a.txt']);
+	});
+
 	await test('text changes are reviewed; edit and delete counts and ctx.touched follow; the files named in the note', async () => {
 		const r = rig({ '/ws/a.txt': 'old', '/ws/b.txt': 'bye' });
 		const out = await commitShellChanges([W('/ws/new.txt', 'hi\n'), W('/ws/a.txt', 'new', 'old'), D('/ws/b.txt', 'bye')], r.ctx, r.io);

extensions/levelcode-ai/test/shellSeam.test.js

@@ -28,7 +28,9 @@ const folder = (name, p) => ({ name, uri: { fsPath: p, path: p, toString: () =>
 const vscodeMock = {
 	workspace: { workspaceFolders: [folder('app', root)], findFiles: async () => [], asRelativePath: (u) => String(u.fsPath || u),
 		// the browser build reads through vscode.workspace.fs; here that is the temp directory
-		fs: { readFile: async (u) => fs.promises.readFile(u.fsPath) } },
+		fs: { readFile: async (u) => fs.promises.readFile(u.fsPath),
+			stat: async (u) => { const s = await fs.promises.stat(u.fsPath); return { type: s.isDirectory() ? 2 : 1, size: s.size }; } } },
+	FileType: { File: 1, Directory: 2 },
 	env: { appRoot: '' },
 	Uri: { file: (p) => ({ fsPath: p, path: p }) },
 	extensions: { getExtension: () => undefined },
@@ -262,6 +264,42 @@ function ctxFor(over) {
 			await p;
 		});
 
+		await test('BROWSER, two workspace folders called proj: a command\'s change to the second is never written into the first', async () => {
+			const { agent, host } = load(true);
+			const calls = registerFake(host);
+			const enc = (s) => new TextEncoder().encode(s);
+			const a = path.join(root, 'a', 'proj'), b = path.join(root, 'b', 'proj');
+			fs.mkdirSync(a, { recursive: true }); fs.mkdirSync(b, { recursive: true });
+			fs.writeFileSync(path.join(a, 'x.txt'), 'A\n'); fs.writeFileSync(path.join(b, 'x.txt'), 'B\n');
+			const saved = vscodeMock.workspace.workspaceFolders;
+			vscodeMock.workspace.workspaceFolders = [folder('proj', a), folder('proj', b)];
+			try {
+				// What reviewSession.applyEdit / applyDelete do to find the file they change: the same resolver, then host.uriFor.
+				const written = [];
+				const find = async (r, mustExist) => { const abs = await agent.resolveWorkspacePathAsync(r.path, { mustExist }); if (abs) { written.push(host.uriFor(abs).fsPath); } return !!abs; };
+				script = [tool('t1', 'run_command', { command: 'make', explanation: 'x' }), say('Done: x')];
+				const p = agent.runAgent(ctxFor({ applyEdit: (r) => find(r, !!r.exists), applyDelete: (r) => find(r, true) }));
+				await settle(); await settle();
+				const commit = calls[0].opts.commit;
+				// readNow compares b/proj/x.txt with what the command read (B), which is right: the write is what went wrong.
+				const out = await commit([
+					{ kind: 'write', path: b + '/x.txt', existed: true, before: enc('B\n'), after: enc('B2\n') },
+					{ kind: 'write', path: b + '/new.txt', existed: false, before: null, after: enc('n\n') },
+				]);
+				assert.deepStrictEqual(written, [], 'nothing is written — and nothing into ' + a);
+				assert.strictEqual(out.refused, true);
+				assert.match(out.notes.join('\n'), /refused: proj\/x\.txt — another workspace folder is also called "proj"/);
+				// The first folder of the name is the one the review finds: a change to it is applied, there.
+				const ok = await commit([{ kind: 'write', path: a + '/x.txt', existed: true, before: enc('A\n'), after: enc('A2\n') }]);
+				assert.strictEqual(ok.refused, false);
+				assert.deepStrictEqual(written, [a + '/x.txt']);
+				calls[0].child.emit('close', 0);
+				await p;
+			} finally {
+				vscodeMock.workspace.workspaceFolders = saved;
+			}
+		});
+
 		await test('BROWSER, the shell says the run was stopped while its changes were being checked: the commit hook applies nothing', async () => {
 			const { agent, host } = load(true);
 			const calls = registerFake(host);