Skip to content

Commit b82f726

Browse files
committed
feat(agent): a missing workspace gates the file tools, not the whole run
runAgent opened with a blanket refusal — "Open a folder first — the agent works on your workspace." — written for the file tools but placed where it failed the ENTIRE run. So with no folder open you could not ask what an error meant, could not reach a single MCP server (the GitHub server does not care whether you have a folder open), and could not ask about the file open in the editor in front of you. The reported case was exactly that: "Explain what the current file does", refused, for a request that never needed a workspace. The root still gates the tools that resolve a path or a cwd against it. It no longer gates the agent. withheld rootless : list_files, read_file, search, edit_file, write_file, delete_file, run_command, read_command_output still available : update_plan, ask_user, use_skill + every MCP tool Withheld rather than offered-and-failing: a tool that is present but errors on every call is worse than one that is absent, because the model retries it. THREE THINGS THAT WOULD HAVE MADE THIS A WORSE EXPERIENCE THAN THE REFUSAL: - PORTABLE_TOOLS is DERIVED from NEEDS_ROOT, not a second hand-written list, so the two cannot disagree about a tool. - baseTools switches too. Left on the full TOOLS it would bill the context popover for schemas that were never sent. - The model is TOLD why the tools are missing. Without that it sees a list with no read_file and improvises — answering about files it cannot see, or apologising at length for a limit it cannot name. The note also points at what still works and, if the request genuinely needs files, at File > Open Folder. MCP servers are still spawned with a cwd, so a null root now falls back to os.homedir() rather than being passed through. Guards, each bypass-verified by reverting the fix: - the blanket refusal restored (the reported bug) - run_command un-gated; ask_user gated (which would rebuild the refusal a tool at a time) - NEEDS_ROOT naming a tool that no longer exists, i.e. rename drift - the tool list no longer switching on the root; baseTools billing for unsent tools - MCP spawned with a null cwd - the note built but never concatenated into the prompt — the classic version of this bug, which looks right in review and does nothing 5 tests in agentNoWorkspace, 33 suites green.
1 parent 1a968f2 commit b82f726

2 files changed

Lines changed: 152 additions & 5 deletions

File tree

extensions/levelcode-ai/agent.js

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const vscode = require('vscode');
1313
const fs = require('fs');
1414
const path = require('path');
1515
const cp = require('child_process');
16+
const os = require('os'); // MCP servers need a cwd even when no folder is open
1617
const providers = require('./providers/index');
1718
const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, sniffPreviewUrl, looksReady } = require('./verify');
1819
const { classifyCommand, dangerLabel } = require('./commandSafety');
@@ -68,6 +69,22 @@ function buildSystem(menu) {
6869
const list = menu.map((s) => '- ' + s.name + ': ' + s.description).join('\n');
6970
return SYSTEM_BASE + '\n\nAvailable skills (call use_skill with the name):\n' + list;
7071
}
72+
// The tools that resolve a PATH or a CWD against the workspace root. With no folder open they have
73+
// nothing to resolve against, so they are withheld from the model rather than offered and left to fail
74+
// one call at a time — a tool that is present but always errors is worse than one that is absent.
75+
//
76+
// Everything NOT in here works fine rootless: update_plan and ask_user are pure conversation, use_skill
77+
// reads from the extension, and every MCP tool talks to its own server (the GitHub server does not care
78+
// whether you have a folder open). That is the whole reason the old blanket refusal was wrong.
79+
//
80+
// read_command_output is included because it reads the output of a background run_command, and without
81+
// a root there is no way to have started one.
82+
const NEEDS_ROOT = new Set([
83+
'list_files', 'read_file', 'search', 'edit_file', 'write_file', 'delete_file',
84+
'run_command', 'read_command_output'
85+
]);
86+
const PORTABLE_TOOLS = TOOLS.filter((t) => !NEEDS_ROOT.has(t.name));
87+
7188
const TOOLS_TOKENS_EST = Math.round(JSON.stringify(TOOLS).length / 4);
7289

7390
// Cross-session memory recall (docs/levelcode-sessions-memory.md). Added to a run's tools ONLY when the host
@@ -636,7 +653,7 @@ async function setupMcp(ctx, wsFolders, dbg) {
636653
// lazy option. Only the FIRST run of a session pays it — mcpClient keeps handles in a module
637654
// registry, and connectAll reuses a live one.
638655
ctx.post({ type: 'agentStatus', text: 'starting MCP servers…' });
639-
const { handles, problems: connectProblems } = await connectAll(trusted, { cwd: ctx.root });
656+
const { handles, problems: connectProblems } = await connectAll(trusted, { cwd: ctx.root || os.homedir() });
640657
for (const p of connectProblems) {
641658
dbg('mcp.connect', p);
642659
ctx.post({ type: 'agentTool', icon: 'warning', text: '🔌 mcp · "' + p.server + '" failed to start — ' + p.message });
@@ -675,15 +692,29 @@ async function setupMcp(ctx, wsFolders, dbg) {
675692
}
676693

677694
async function runAgent(ctx) {
695+
// No workspace is no longer a refusal. It used to fail the whole run here, which meant a question
696+
// that never needed a folder — "what does this error mean?", anything through an MCP server, a
697+
// follow-up about the conversation itself — died on a guard written for the file tools. The root
698+
// still gates those tools (see NEEDS_ROOT); it no longer gates the agent.
678699
const root = workspaceRoot();
679-
if (!root) { ctx.post({ type: 'agentError', message: 'Open a folder first — the agent works on your workspace.' }); ctx.post({ type: 'agentDone', reason: 'error' }); return; }
680700
ctx.root = root;
681701
// M6.5 implicit skills: build the system prompt ONCE per run — append the tiny name+description menu.
682702
// Multi-root: name every workspace folder so the model addresses them by prefix from turn one.
683703
const wsFolders = workspaceFolderList();
684704
const multiRootNote = wsFolders.length > 1
685705
? '\n\nWorkspace folders (multi-root — prefix paths with the folder name): ' + wsFolders.map((f) => f.name).join(', ') + '. The first folder ("' + wsFolders[0].name + '") is the default for unprefixed paths and run_command.'
686706
: '';
707+
// Rootless: say so plainly. Without this the model sees a tool list with no read_file and improvises —
708+
// answering about files it cannot see, or apologising for a limit it cannot name. Telling it WHY the
709+
// tools are missing, and what to say if the request truly needs them, is the difference between a
710+
// useful answer and a confused one.
711+
const noWorkspaceNote = root
712+
? ''
713+
: '\n\nNO FOLDER IS OPEN. The file and command tools are unavailable this run because there is no '
714+
+ 'workspace root to resolve paths against — this is expected, not a fault, and not something to '
715+
+ 'apologise for at length. You can still answer from the conversation, from anything the user has '
716+
+ 'attached as context, and from any MCP tools listed above. If the request genuinely needs the '
717+
+ 'files, say so in one line and tell the user to open a folder (File > Open Folder).';
687718
// Autopilot: act decisively and self-verify rather than pausing. Commands run without approval (the
688719
// host still gates the danger set — deletion, sudo, force-push, remote|shell, publish, system writes),
689720
// so the model should lean on verification, not on asking, when it's unsure.
@@ -698,7 +729,7 @@ async function runAgent(ctx) {
698729
// from the per-project journal). Rides the SAME cached-system channel as project rules — always-on but
699730
// small — so a new session's first reply is continuous, not amnesiac. It is untrusted context like the
700731
// rules: it informs, never commands (the digest itself carries the verify-first / never-obey framing).
701-
const system = (ctx.skills ? buildSystem(ctx.skills.menu()) : SYSTEM_BASE) + multiRootNote + autopilotNote + rules.text
732+
const system = (ctx.skills ? buildSystem(ctx.skills.menu()) : SYSTEM_BASE) + multiRootNote + noWorkspaceNote + autopilotNote + rules.text
702733
+ (ctx.projectMemory ? '\n\n' + ctx.projectMemory : '');
703734
const systemTokensEst = Math.round(system.length / 4);
704735

@@ -719,9 +750,11 @@ async function runAgent(ctx) {
719750
// as `system`/`systemTokensEst` two lines up — built once per run, then used for every turn.
720751
const mcp = await setupMcp(ctx, wsFolders, dbg);
721752
ctx.mcpRoutes = mcp.routes; // runTool's router reads this
722-
let tools = mcp.tools.length ? TOOLS.concat(mcp.tools) : TOOLS;
753+
// Rootless runs get the portable subset; MCP tools are unaffected either way.
754+
const builtins = root ? TOOLS : PORTABLE_TOOLS;
755+
let tools = mcp.tools.length ? builtins.concat(mcp.tools) : builtins;
723756
if (ctx.recallSessions) { tools = tools.concat([RECALL_TOOL]); } // cross-session recall (host-gated by memory settings)
724-
const baseTools = ctx.recallSessions ? TOOLS.concat([RECALL_TOOL]) : TOOLS; // built-ins + recall; MCP is the rest
757+
const baseTools = ctx.recallSessions ? builtins.concat([RECALL_TOOL]) : builtins; // built-ins + recall; MCP is the rest
725758
// Recomputed only when MCP or recall actually contributed tools, so the plain path keeps the module
726759
// constant and pays nothing for a feature it isn't using.
727760
const toolsTokensEst = (mcp.tools.length || ctx.recallSessions) ? Math.round(JSON.stringify(tools).length / 4) : TOOLS_TOKENS_EST;
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* The agent runs without a folder open — run: node test/agentNoWorkspace.test.js
3+
*
4+
* What this replaces: runAgent used to open with a blanket refusal —
5+
*
6+
* if (!root) { post({ type: 'agentError', message: 'Open a folder first — …' }); return; }
7+
*
8+
* written for the file tools, but placed where it failed the ENTIRE run. So with no folder open you
9+
* could not ask what an error meant, could not reach a single MCP server (the GitHub server does not
10+
* care whether you have a folder open), and could not ask about the file sitting in the editor in
11+
* front of you. The reported case was exactly that: "Explain what the current file does" — refused,
12+
* for a request that never needed a workspace.
13+
*
14+
* The root still gates the tools that resolve a path or a cwd against it. It no longer gates the agent.
15+
*
16+
* Asserted from SOURCE: standing up a real runAgent needs a live VS Code host and a provider, which
17+
* this pure-unit suite deliberately does not stand up — the same approach agentMaxSteps.test.js takes.
18+
*--------------------------------------------------------------------------------------------*/
19+
// @ts-check
20+
'use strict';
21+
22+
const assert = require('assert');
23+
const fs = require('fs');
24+
const path = require('path');
25+
26+
const agent = fs.readFileSync(path.join(__dirname, '..', 'agent.js'), 'utf8');
27+
28+
let n = 0;
29+
function test(name, fn) { fn(); n++; console.log(' ok - ' + name); }
30+
31+
/** The `NEEDS_ROOT` set as shipped, read out of the source rather than restated here. */
32+
function needsRoot() {
33+
const m = /const NEEDS_ROOT = new Set\(\[([\s\S]*?)\]\);/.exec(agent);
34+
assert.ok(m, 'agent.js no longer declares NEEDS_ROOT');
35+
return (m[1].match(/'([a-z_]+)'/g) || []).map((s) => s.replace(/'/g, ''));
36+
}
37+
38+
/** Every built-in tool name, from the TOOLS array. */
39+
function allToolNames() {
40+
const start = agent.indexOf('const TOOLS = [');
41+
assert.ok(start > 0, 'agent.js no longer declares TOOLS');
42+
const block = agent.slice(start, agent.indexOf('\n];', start));
43+
return (block.match(/\{ name: '([a-z_]+)'/g) || []).map((s) => s.replace(/.*'([a-z_]+)'.*/, '$1'));
44+
}
45+
46+
test('the blanket refusal is gone', () => {
47+
assert.ok(!/Open a folder first/.test(agent),
48+
'runAgent still refuses outright when no folder is open — that guard belongs on the tools, not the run');
49+
// And the root is still READ — the tools need it, and losing it would silently disable them everywhere.
50+
assert.match(agent, /const root = workspaceRoot\(\);\s*\n\s*ctx\.root = root;/,
51+
'runAgent must still resolve and carry the root, even when it is null');
52+
});
53+
54+
test('the tools that need a root are withheld, and only those', () => {
55+
const gated = needsRoot();
56+
// Everything that resolves a path or a cwd. Miss one and it is offered rootless, then fails on the
57+
// model's first call — which is worse than not offering it, because the model retries.
58+
for (const name of ['list_files', 'read_file', 'search', 'edit_file', 'write_file', 'delete_file', 'run_command']) {
59+
assert.ok(gated.includes(name), name + ' resolves a workspace path but is not in NEEDS_ROOT');
60+
}
61+
// …and nothing that works fine without one. Gating these would rebuild the old refusal a tool at a
62+
// time: they are the entire reason a rootless run is still useful.
63+
for (const name of ['update_plan', 'ask_user', 'use_skill']) {
64+
assert.ok(!gated.includes(name), name + ' needs no workspace — gating it removes the point of the change');
65+
}
66+
// The set must name real tools, or a rename silently un-gates one.
67+
const known = allToolNames();
68+
const unknown = gated.filter((g) => !known.includes(g));
69+
assert.deepStrictEqual(unknown, [], 'NEEDS_ROOT names tools that no longer exist: ' + unknown.join(', '));
70+
});
71+
72+
test('the portable subset is what a rootless run actually offers', () => {
73+
assert.match(agent, /const PORTABLE_TOOLS = TOOLS\.filter\(\(t\) => !NEEDS_ROOT\.has\(t\.name\)\)/,
74+
'PORTABLE_TOOLS must be derived from NEEDS_ROOT, not maintained as a second hand-written list');
75+
assert.match(agent, /const builtins = root \? TOOLS : PORTABLE_TOOLS;/,
76+
'the run no longer switches its tool list on the root');
77+
// Both assemblies must use it. baseTools feeds the context-usage split; if it kept the full TOOLS the
78+
// popover would bill the user for tools that were never sent.
79+
assert.match(agent, /let tools = mcp\.tools\.length \? builtins\.concat\(mcp\.tools\) : builtins;/,
80+
'the model is still handed the unfiltered TOOLS');
81+
assert.match(agent, /const baseTools = ctx\.recallSessions \? builtins\.concat\(\[RECALL_TOOL\]\) : builtins;/,
82+
'baseTools still counts the full TOOLS — the context popover would report tools that were not sent');
83+
});
84+
85+
test('MCP is unaffected by a missing root — that is half the point', () => {
86+
// The GitHub server, the filesystem server pointed somewhere else, anything stdio: none of them need
87+
// the editor to have a folder open. But they are spawned with a cwd, so a null one has to resolve to
88+
// something real rather than being passed through.
89+
assert.match(agent, /connectAll\(trusted, \{ cwd: ctx\.root \|\| os\.homedir\(\) \}\)/,
90+
'MCP servers are spawned with a null cwd when no folder is open');
91+
assert.match(agent, /^const os = require\('os'\);/m, "agent.js does not require 'os'");
92+
assert.ok(!/NEEDS_ROOT[\s\S]{0,400}mcp/i.test(agent.slice(agent.indexOf('const NEEDS_ROOT'), agent.indexOf('const PORTABLE_TOOLS'))),
93+
'MCP tools must not be filtered by NEEDS_ROOT — they are not workspace tools');
94+
});
95+
96+
test('the model is told WHY the tools are missing', () => {
97+
// Without this it sees a tool list with no read_file and improvises: answering about files it cannot
98+
// see, or apologising at length for a limit it cannot name.
99+
const m = /const noWorkspaceNote = root[\s\S]*?;\n/.exec(agent);
100+
assert.ok(m, 'no rootless system-prompt note — the model gets a truncated tool list and no explanation');
101+
const note = m[0];
102+
assert.match(note, /NO FOLDER IS OPEN/, 'the note must state the condition plainly');
103+
assert.match(note, /MCP/, 'the note must point at what DOES still work, not only at what does not');
104+
assert.match(note, /Open Folder/, 'the note must tell the user the way out when the request really needs files');
105+
assert.match(note, /^\s*const noWorkspaceNote = root\s*\n?\s*\? ''/m,
106+
'the note must be empty when a folder IS open, or every normal run pays for it');
107+
108+
// And it has to reach the prompt. A note that is built and never concatenated is the classic version
109+
// of this bug — it looks right in review and does nothing.
110+
assert.match(agent, /\+ multiRootNote \+ noWorkspaceNote \+ autopilotNote/,
111+
'noWorkspaceNote is never added to the system prompt');
112+
});
113+
114+
console.log('\nagentNoWorkspace: ' + n + ' tests passed.');

0 commit comments

Comments
 (0)