Skip to content

Commit 82e6cea

Browse files
committed
fix(web/shell): #107 review — the agent's commands run in a browser on Windows
agent.js handed the shell its folder roots and working directory as `Uri.fsPath`; the runtime mounts and matches by `Uri.path`. On a Mac or Linux they are the same string. In a browser on Windows they are not: VS Code takes the platform from the user agent (src/vs/base/common/platform.ts), and uriToFsPath (src/vs/base/common/uri.ts) turns every `/` into `\` whatever the scheme, so `file:///proj` — a folder opened from the computer — is `\proj`. normalizePath('\proj') is null, and every run_command (and the verify command) answered "the working directory \proj is not inside a workspace folder". The mirror tab was told the same `\proj`, which its links (cwdsOf) and its "in <folder>" line map against `uri.path` mounts. shellSpawnOpts now names everything in the shell's terms: each folder's root is `uri.path`, the working directory is the URI path of the folder runCommand was given (looked up by its fsPath; a folder removed while the command waited for approval is not found, and the shell then says it is not inside a workspace folder), and the commit hook resolves the change paths against those same roots — so a change is mapped back to its folder and readNow (host.readBytes -> host.uriFor, which matches `uri.path`) reads the right file. touched gets those paths too, which is what host.uriFor maps for the verify step. runCommand passes `opts.cwd` when a shell supplied one. The desktop is unaffected: it never has a registered shell (host.shellInfo() is null, connectShell resolves false), so shellSpawnOpts returns {} and the desktop's spawn is given `root` as before — shellSeam's "spawns exactly what it always did" and agentRunCommand.test.js (untouched) pass. The rest of the extension still names files by fsPath (workspaceFolderList, resolveWorkspacePath), so in a browser on Windows the file tools, and with them the review a command's changes go through, still cannot resolve a path: a command's write is reported as not applied there. That is the browser edition's, not the shell's, and is not changed here. Test: web/test/unit/shell-agent-paths.test.js runs the real runAgent -> run_command -> host.spawn -> the real runtime's spawnAgent (fake engine, scripted provider), with fsPath computed as uriToFsPath does on Windows: the command runs in /proj and its output comes back, the mirror tab's cwd maps to a mount, run_command's `folder` picks /lib, a command's changes come back to the review as folder-relative paths read from the right file, and the verify command runs in /proj. All four fail on 187fd21 and pass here; reverting any one of the five changed lines fails at least one of them.
1 parent 356bc47 commit 82e6cea

3 files changed

Lines changed: 204 additions & 12 deletions

File tree

‎extensions/levelcode-ai/agent.js‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,20 +298,32 @@ function workspaceFolderList() {
298298
return (vscode.workspace.workspaceFolders || []).map((f) => ({ name: f.name, root: f.uri.fsPath }));
299299
}
300300
/**
301-
* What a simulated shell needs besides the command line (host.spawn): the workspace folders to mount, and — for
302-
* an AGENT run — where to send the files the command changed. Nothing for the desktop's shell, which writes to
303-
* disk itself and has never been asked for either.
301+
* What a simulated shell needs besides the command line (host.spawn): the workspace folders to mount, the folder to
302+
* run in, and — for an AGENT run — where to send the files the command changed. Nothing for the desktop's shell,
303+
* which writes to disk itself and has never been asked for any of it.
304+
*
305+
* All of it in the shell's own terms: a folder is its URI path (`/proj`), which is where the shell mounts it, what the
306+
* paths of the changes it hands back are made of, and what host.uriFor maps back to a folder. Not the fsPath the rest
307+
* of this file uses: in a browser on Windows (VS Code takes the platform from the user agent) fsPath is `\proj` for
308+
* every scheme, and the shell answered every command with "the working directory \proj is not inside a workspace
309+
* folder". On a Mac or Linux the two are the same string.
310+
* @param {string|null} cwd the folder the command runs in, as runCommand is given it: one folder's fsPath
304311
*/
305-
function shellSpawnOpts(ctx, info) {
312+
function shellSpawnOpts(ctx, info, cwd) {
306313
if (!info) { return {}; }
307-
const folders = (vscode.workspace.workspaceFolders || []).map((f) => ({ name: f.name, root: f.uri.fsPath, uri: f.uri.toString() }));
314+
const live = () => vscode.workspace.workspaceFolders || [];
315+
const shellFolders = () => live().map((f) => ({ name: f.name, root: f.uri.path, uri: f.uri.toString() }));
316+
// (A folder removed while the command waited for approval is not found: the shell is then handed the fsPath, and
317+
// says that it is not inside a workspace folder, which is true.)
318+
const at = live().find((f) => f.uri.fsPath === cwd);
308319
return {
309-
folders,
320+
cwd: at ? at.uri.path : cwd,
321+
folders: shellFolders(),
310322
// `control.cancelled()` is the shell's own word that the run was stopped while its changes were being
311323
// checked: nothing is applied then (see commitShellChanges).
312324
commit: async (changes, control) => {
313325
const r = await commitShellChanges(changes, ctx, {
314-
folders: () => workspaceFolderList(),
326+
folders: shellFolders, // the paths of the changes are made of the roots the shell was given
315327
readNow: async (abs) => { try { return await host.readBytes(abs); } catch { return null; } },
316328
cancelled: () => !!(control && typeof control.cancelled === 'function' && control.cancelled())
317329
});
@@ -449,8 +461,9 @@ const START_KEPT = 8000;
449461
// Returns the (capped) full output for the model.
450462
//
451463
// opts.onData(text, stream) sees EVERY chunk, unlike onChunk, which stops at the live view's cap — a
452-
// registry that feeds on the view stops updating when the view fills up. opts.folders / opts.commit are for
453-
// a host whose shell is simulated (host.spawn); the desktop's spawn ignores them.
464+
// registry that feeds on the view stops updating when the view fills up. opts.cwd / opts.folders / opts.commit
465+
// are for a host whose shell is simulated (host.spawn): `root` named in that shell's terms (shellSpawnOpts). The
466+
// desktop never has them, and its spawn is given `root` as ever.
454467
function runCommand(root, command, onChunk, onExit, onStart, timeoutMs, opts) {
455468
return new Promise((resolve) => {
456469
let out = '', streamed = 0, settled = false, timedOut = false, stopped = false;
@@ -476,7 +489,7 @@ function runCommand(root, command, onChunk, onExit, onStart, timeoutMs, opts) {
476489
let child;
477490
// detached:true → the shell becomes its own process-group leader, so process.kill(-pid) kills the
478491
// whole tree (the shell AND the server/children it spawned), not just the shell (which orphans the server).
479-
try { child = host.spawn(command, { cwd: root, folders: opts && opts.folders, commit: opts && opts.commit, explanation: opts && opts.explanation }); }
492+
try { child = host.spawn(command, { cwd: (opts && opts.cwd) || root, folders: opts && opts.folders, commit: opts && opts.commit, explanation: opts && opts.explanation }); }
480493
catch (e) { if (onExit) { onExit(-1, 0, 'error'); } resolve('ERROR: ' + ((e && e.message) || e)); return; }
481494
const killGroup = (sig) => { try { if (child.pid) { process.kill(-child.pid, sig); } else { child.kill(sig); } } catch (e) { try { child.kill(sig); } catch (e2) { /* already gone */ } } };
482495
const stop = () => { if (settled || stopped) { return; } stopped = true; killGroup('SIGTERM'); setTimeout(() => { if (!settled) { killGroup('SIGKILL'); } }, 1500); };
@@ -668,7 +681,7 @@ async function runTool(tu, ctx) {
668681
ctx.post({ type: 'termExit', id: runId, code: code, ms: ms, how: how });
669682
};
670683
const onStart = (child, stop) => { if (stops) { stops.set(runId, stop); } };
671-
const spawnOpts = Object.assign({ onData, explanation: String(input.explanation || '').slice(0, 300) }, shellSpawnOpts(ctx, sh));
684+
const spawnOpts = Object.assign({ onData, explanation: String(input.explanation || '').slice(0, 300) }, shellSpawnOpts(ctx, sh, cwdRoot));
672685
if (bg) {
673686
// Fire-and-forget: keep streaming + tracking, but return NOW so the agent loop isn't blocked.
674687
// No timeout — a background server is meant to run long (Stop / New Chat reap it).
@@ -1139,7 +1152,7 @@ async function runAgent(ctx) {
11391152
(chunk, stream) => ctx.post({ type: 'verifyOutput', id, chunk, stream }),
11401153
(code) => { if (ctx.commandStops) { ctx.commandStops.delete(id); } exitCode = (code == null ? -1 : code); },
11411154
(child, stop) => { if (ctx.commandStops) { ctx.commandStops.set(id, stop); } },
1142-
ctx.commandTimeout, shellSpawnOpts(ctx, sh));
1155+
ctx.commandTimeout, shellSpawnOpts(ctx, sh, root));
11431156
}
11441157
// A command that couldn't even start (missing npm script / command-not-found) is a CONFIG problem,
11451158
// not the agent's code — flag it as unrunnable so we warn the user instead of looping the agent on it.

‎extensions/levelcode-ai/host.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ const SHELL_API_VERSION = 1;
193193
* @typedef {object} SpawnOptions
194194
* @property {string} cwd absolute path of a workspace folder, or inside one
195195
* @property {WorkspaceFolderRef[]} [folders] the workspace folders, live and in order (a shell mounts each at its root)
196+
* For a registered shell `cwd` and each `root` are URI paths (`/proj`), never fsPath, which is `\proj` in a
197+
* browser on Windows (agent.js shellSpawnOpts); the desktop's spawn is given the fsPath it always was.
196198
* @property {Record<string,string>} [env] extra environment
197199
* @property {string} [explanation] the model's own label for the command (at most 300 characters), for a tab that mirrors it
198200
* @property {(changes: Change[], control?: { cancelled(): boolean }) => Promise<{ rest: Change[], notes: string[], failed?: number, refused?: boolean }>} [commit]
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* The agent's run_command over the REAL LevelCode Shell runtime — run: node web/test/unit/shell-agent-paths.test.js
3+
*
4+
* agent.js (extensions/levelcode-ai) has always named workspace files by `Uri.fsPath`. The shell
5+
* (web/shell/src/runtime.js) mounts each folder at its URI path and refuses a working directory that is not inside
6+
* one. On a Mac or Linux the two are the same string. In a browser on Windows they are not: VS Code takes the
7+
* platform from the user agent (src/vs/base/common/platform.ts), and `uriToFsPath` (src/vs/base/common/uri.ts) turns
8+
* every `/` into `\` for any scheme — `file:///proj`, a folder opened from the computer, has the fsPath `\proj`. The
9+
* shell was handed that, and every command answered "the working directory \proj is not inside a workspace folder".
10+
*
11+
* So this runs the real thing end to end — runAgent → run_command → runCommand → host.spawn → the runtime's
12+
* spawnAgent → a fake engine (web/test/helpers/shell-runtime-fakes.js) and back through agent.js's commit hook —
13+
* with the folders' fsPath computed as VS Code computes it on Windows. A scripted provider stands in for the model.
14+
*--------------------------------------------------------------------------------------------*/
15+
// @ts-check
16+
'use strict';
17+
18+
// A promise that never settles ends the process with status 0 and no output: a suite that did not finish fails.
19+
let finished = false;
20+
process.on('exit', (code) => { if (!finished && code === 0) { console.error('not ok - the tests did not finish'); process.exitCode = 1; } });
21+
22+
const assert = require('assert');
23+
const path = require('path');
24+
const Module = require('module');
25+
26+
const { createRig, flush, Uri } = require(path.join(__dirname, '..', 'helpers', 'shell-runtime-fakes.js'));
27+
const { planMounts, toMountPath } = require(path.join(__dirname, '..', '..', 'shell', 'src', 'runtime.js'));
28+
29+
const AI = path.join(__dirname, '..', '..', '..', 'extensions', 'levelcode-ai');
30+
const te = new TextEncoder();
31+
32+
/** The `vscode` agent.js and host.js are given: the rig's workspace, and the few members they read besides. */
33+
const vscodeMock = /** @type {any} */ ({});
34+
const origLoad = Module._load;
35+
// @ts-ignore — test-only loader shim
36+
Module._load = function (request, parent, isMain) {
37+
if (request === 'vscode') { return vscodeMock; }
38+
return origLoad.call(this, request, parent, isMain);
39+
};
40+
const providers = require(path.join(AI, 'providers', 'index'));
41+
/** @type {any[]} */ let script = [];
42+
/** @type {any[]} */ const turns = [];
43+
providers.streamAgentTurn = async (/** @type {any} */ o) => { turns.push({ messages: JSON.parse(JSON.stringify(o.messages)) }); const t = script.shift(); if (!t) { throw new Error('script exhausted'); } return t; };
44+
45+
/**
46+
* Uri.fsPath as VS Code computes it in a browser on Windows: uri.ts uriToFsPath with isWindows true.
47+
* @param {any} u
48+
*/
49+
function windowsFsPath(u) {
50+
let v;
51+
if (u.authority && u.path.length > 1 && u.scheme === 'file') { v = '//' + u.authority + u.path; }
52+
else if (/^\/[A-Za-z]:/.test(u.path)) { v = u.path[1].toLowerCase() + u.path.slice(2); }
53+
else { v = u.path; }
54+
return v.replace(/\//g, '\\');
55+
}
56+
57+
/**
58+
* A workspace in a browser on Windows, the shell runtime over it, and agent.js + host.js loaded fresh against it,
59+
* with the runtime registered as the shell exactly as web/shell/src/extension.js exports it.
60+
* @param {Array<{ name: string, uri: string }>} folders
61+
*/
62+
function setup(folders) {
63+
const rig = createRig({ workspace: { folders } });
64+
for (const f of rig.ws.vscode.workspace.workspaceFolders) { Object.defineProperty(f.uri, 'fsPath', { get() { return windowsFsPath(this); } }); }
65+
const ws = rig.ws.vscode.workspace;
66+
ws.findFiles = async () => [];
67+
ws.asRelativePath = (/** @type {any} */ u) => String(u.path || u);
68+
ws.textDocuments = [];
69+
for (const k of Object.keys(vscodeMock)) { delete vscodeMock[k]; }
70+
Object.assign(vscodeMock, {
71+
Uri: { file: (/** @type {string} */ p) => new Uri('file', '', p), parse: Uri.parse },
72+
FileType: rig.ws.vscode.FileType,
73+
workspace: ws,
74+
env: { appRoot: '' },
75+
extensions: { getExtension: () => undefined },
76+
});
77+
for (const k of Object.keys(require.cache)) { if (k.startsWith(AI + path.sep) && !k.includes(path.sep + 'providers' + path.sep)) { delete require.cache[k]; } }
78+
/** @type {any} */ (globalThis).__LEVELCODE_BROWSER_HOST__ = true;
79+
const agent = require(path.join(AI, 'agent'));
80+
const host = require(path.join(AI, 'host'));
81+
host.registerShell({ info: rig.rt.info, spawn: (/** @type {string} */ c, /** @type {any} */ o) => rig.rt.spawnAgent(c, o), forDisplay: (/** @type {string} */ t) => t });
82+
/** @type {any[]} */ const runs = [];
83+
/** @type {any[]} */ const starts = [];
84+
rig.rt.onAgentRun((e) => { if (e.type === 'start') { starts.push(e); } });
85+
return { rig, agent, host, runs, starts };
86+
}
87+
88+
const tool = (/** @type {string} */ id, /** @type {string} */ name, /** @type {any} */ input) => ({ stop_reason: 'tool_use', content: [{ type: 'tool_use', id, name, input }] });
89+
const say = (/** @type {string} */ text) => ({ stop_reason: 'end_turn', content: [{ type: 'text', text }] });
90+
const resultFor = (/** @type {string} */ id) => { for (const t of turns) { for (const m of t.messages) { if (m.role === 'user' && Array.isArray(m.content)) { const r = m.content.find((/** @type {any} */ c) => c.tool_use_id === id); if (r) { return r.content; } } } } return null; };
91+
/** @param {any} [over] */
92+
function ctxFor(over) {
93+
/** @type {any[]} */ const posted = [];
94+
return Object.assign({
95+
messages: [{ role: 'user', content: 'go' }], maxSteps: 6, post: (/** @type {any} */ m) => posted.push(m), dbg: () => {},
96+
approve: async () => true, commandRuns: new Map(), commandStops: new Map(), signal: new AbortController().signal, posted,
97+
}, over);
98+
}
99+
100+
let n = 0;
101+
async function test(/** @type {string} */ name, /** @type {() => Promise<void>} */ fn) { turns.length = 0; await fn(); n++; console.log(' ok - ' + name); }
102+
103+
(async () => {
104+
try {
105+
await test('a browser on Windows: run_command runs in the folder — the shell is handed /proj, not \\proj — and its output comes back', async () => {
106+
const { rig, agent, runs, starts } = setup([{ name: 'proj', uri: 'file:///proj' }]);
107+
assert.strictEqual(rig.ws.vscode.workspace.workspaceFolders[0].uri.fsPath, '\\proj', 'the premise: the folder\'s fsPath has a backslash');
108+
rig.engines.state.onRun = async (run, w, h) => { runs.push(run); h.out(run.id, 'stdout', 'hi\n'); await flush(); h.done({ exitCode: 0 }); };
109+
script = [tool('t1', 'run_command', { command: 'echo hi', explanation: 'x' }), say('Done: x')];
110+
await agent.runAgent(ctxFor({}));
111+
assert.strictEqual(resultFor('t1'), 'hi\n');
112+
assert.strictEqual(runs.length, 1);
113+
assert.strictEqual(runs[0].cwd, '/proj');
114+
// The mirror tab hears the same folder, in the terms it resolves a line's links in (terminal/index.js cwdsOf).
115+
assert.strictEqual(starts[0].cwd, '/proj');
116+
assert.strictEqual(toMountPath(planMounts(rig.rt.folders()), starts[0].cwd), '/proj');
117+
rig.rt.dispose();
118+
});
119+
120+
await test('a browser on Windows: the folder run_command picks by name is the one it runs in', async () => {
121+
const { rig, agent, runs } = setup([{ name: 'proj', uri: 'file:///proj' }, { name: 'lib', uri: 'file:///lib' }]);
122+
rig.engines.state.onRun = async (run, w, h) => { runs.push(run); h.done({ exitCode: 0 }); };
123+
script = [tool('t1', 'run_command', { command: 'ls', folder: 'lib', explanation: 'x' }), say('Done: x')];
124+
await agent.runAgent(ctxFor({}));
125+
assert.strictEqual(runs.length, 1, String(resultFor('t1')));
126+
assert.strictEqual(runs[0].cwd, '/lib');
127+
rig.rt.dispose();
128+
});
129+
130+
await test('a browser on Windows: what the command changed comes back to the review by folder, read from the right file, and is verified', async () => {
131+
const { rig, agent, runs } = setup([{ name: 'proj', uri: 'file:///proj' }]);
132+
rig.ws.put('/proj/old.txt', 'a\n');
133+
rig.engines.state.onRun = async (run, w, h) => {
134+
runs.push(run);
135+
h.done({ exitCode: 0, changes: [
136+
{ kind: 'write', path: '/proj/old.txt', existed: true, before: te.encode('a\n'), after: te.encode('b\n') },
137+
{ kind: 'write', path: '/proj/new.txt', existed: false, before: null, after: te.encode('hi\n') },
138+
] });
139+
};
140+
/** @type {any[]} */ const edits = [];
141+
const ctx = ctxFor({ applyEdit: async (/** @type {any} */ r) => { edits.push(r); return true; }, applyDelete: async () => true, touched: new Set() });
142+
script = [tool('t1', 'run_command', { command: 'make', explanation: 'x' }), say('Done: x')];
143+
await agent.runAgent(ctx);
144+
// old.txt is compared with what is on disk now (read through host.uriFor): a path the folders do not contain
145+
// would have been "changed while the command ran", or "outside the workspace folders".
146+
assert.deepStrictEqual(edits, [{ path: 'new.txt', exists: false, proposed: 'hi\n' }, { path: 'old.txt', exists: true, proposed: 'b\n' }], String(resultFor('t1')));
147+
assert.match(String(resultFor('t1')), /2 files changed, ready for Keep\/Undo/);
148+
assert.deepStrictEqual([...ctx.touched].sort(), ['/proj/new.txt', '/proj/old.txt'], 'what the verify step looks at, as host.uriFor maps it');
149+
rig.rt.dispose();
150+
});
151+
152+
await test('a browser on Windows: the verify command runs in the folder too', async () => {
153+
const { rig, agent, runs } = setup([{ name: 'proj', uri: 'file:///proj' }]);
154+
rig.engines.state.onRun = async (run, w, h) => {
155+
runs.push(run);
156+
if (run.script === 'make') { h.done({ exitCode: 0, changes: [{ kind: 'write', path: '/proj/new.txt', existed: false, before: null, after: te.encode('hi\n') }] }); }
157+
else { h.done({ exitCode: 0 }); }
158+
};
159+
const ctx = ctxFor({
160+
applyEdit: async () => true, applyDelete: async () => true, touched: new Set(),
161+
verify: { enabled: true, command: 'grep -q hi new.txt', maxRounds: 1, includeWarnings: false },
162+
getNewDiagnostics: async () => ({ text: '', count: 0 }), getTouchedUris: () => [],
163+
});
164+
script = [tool('t1', 'run_command', { command: 'make', explanation: 'x' }), say('Done: x')];
165+
await agent.runAgent(ctx);
166+
assert.deepStrictEqual(runs.map((r) => r.script + ' @ ' + r.cwd), ['make @ /proj', 'grep -q hi new.txt @ /proj']);
167+
assert.ok(ctx.posted.some((/** @type {any} */ m) => m.type === 'verifyDone' && m.ok === true && m.hadCommand === true));
168+
rig.rt.dispose();
169+
});
170+
171+
console.log('\nshell-agent-paths: ' + n + ' tests passed.');
172+
finished = true;
173+
} finally {
174+
Module._load = origLoad;
175+
delete (/** @type {any} */ (globalThis)).__LEVELCODE_BROWSER_HOST__;
176+
}
177+
})().catch((e) => { console.error(e); process.exit(1); });

0 commit comments

Comments
 (0)