From b438b6c41f22f3d527d968b7f215e41034b8c702 Mon Sep 17 00:00:00 2001 From: Sergii Demianchuk Date: Thu, 23 Jul 2026 20:33:04 -0400 Subject: [PATCH 1/4] feat(ai): auto-open the built-in browser when the agent starts a web server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LevelCode has shipped VS Code's Simple Browser all along — but you had to know the command name to find it, so nobody did. This makes it appear on its own: the moment a background command advertises a local address, the site opens beside the chat, and the agent keeps working while you watch the page update. That pairs with how edits already land: the agent writes to disk immediately (apply-then-review), so the dev server's watcher fires HMR and the preview updates before you ever click Keep. SECURITY — the reason sniffPreviewUrl is not just "find a URL". Its input is a command's stdout, i.e. whatever a repo's dev script chose to print. Opening any URL found there would let a hostile repo aim the editor's browser anywhere just by logging a line ("Local: http://evil.example.com"). So the host must be LOCAL; remote addresses are ignored, never opened. 0.0.0.0 and [::] are bind addresses rather than destinations, so they're rewritten to localhost, which resolves. Three UX choices worth stating: - preserveFocus — a server coming up must never steal the caret from whoever is typing, and one usually comes up mid-run. - ViewColumn.Beside — the site sits next to the work instead of replacing it. - dedupe by URL, once per session — so CLOSING the tab is final. Without it, a chatty server's next log line would reopen what the user just dismissed, and a restart-on-save server would stack a tab per reload. Failure is swallowed: a preview must never be able to fail an agent run. Off via levelcode.ai.preview.autoOpen. Also fixes two things found on the way: - verify.js shipped a RAW NUL byte in diagKey's separator, so `file` called it "data" and grep/diff skipped the entire module in silence — the same defect review caught in the MCP modules. Replaced with the backslash-u0000 escape (identical runtime value) and pinned by a byte-level test. - verify.js had NO tests at all, despite the plan claiming it was unit-tested. It now has a suite, since auto-open depends on its sniffers. Verified: 23 suites, 0 failures (verify.test.js is new: 10 cases). Both new behaviours mutation-checked — dropping the local-host check opens "http://evil.example.com:3000/" and fails the suite; not rewriting 0.0.0.0 fails too. Simple Browser's api.open signature confirmed against the shipped extension source, not assumed. Co-Authored-By: Claude Opus 4.8 --- extensions/levelcode-ai/agent.js | 12 +- extensions/levelcode-ai/extension.js | 33 ++++++ extensions/levelcode-ai/package.json | 5 + extensions/levelcode-ai/test/verify.test.js | 119 ++++++++++++++++++++ extensions/levelcode-ai/verify.js | Bin 6029 -> 7847 bytes 5 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 extensions/levelcode-ai/test/verify.test.js diff --git a/extensions/levelcode-ai/agent.js b/extensions/levelcode-ai/agent.js index f9fbed2..2059142 100644 --- a/extensions/levelcode-ai/agent.js +++ b/extensions/levelcode-ai/agent.js @@ -14,7 +14,7 @@ const fs = require('fs'); const path = require('path'); const cp = require('child_process'); const providers = require('./providers/index'); -const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, looksReady } = require('./verify'); +const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, sniffPreviewUrl, looksReady } = require('./verify'); const { classifyCommand, dangerLabel } = require('./commandSafety'); const { loadProjectRules } = require('./projectRules'); const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal } = require('./mcpConfig'); @@ -385,7 +385,7 @@ async function runTool(tu, ctx) { const stops = ctx.commandStops; // shared registry so the Stop button / ■ can kill the process group // Only BACKGROUND commands get a registry entry (read_command_output reads it). Foreground // one-shots keep their old behavior + don't accumulate — the model already gets their output. - const entry = bg ? { command: cmd, status: 'running', code: null, how: null, port: null, ready: false, ring: '', totalBytes: 0, lastReadOffset: 0, startedAt: Date.now() } : null; + const entry = bg ? { command: cmd, status: 'running', code: null, how: null, port: null, ready: false, previewUrl: null, ring: '', totalBytes: 0, lastReadOffset: 0, startedAt: Date.now() } : null; if (entry && ctx.commandRuns) { ctx.commandRuns.set(runId, entry); } const onChunk = (chunk, stream) => { ctx.post({ type: 'termOutput', id: runId, chunk: chunk, stream: stream }); @@ -394,6 +394,14 @@ async function runTool(tu, ctx) { entry.totalBytes += chunk.length; if (!entry.port) { const p = sniffPort(chunk); if (p) { entry.port = p; ctx.post({ type: 'bgTask', id: runId, port: p }); } } if (!entry.ready && looksReady(chunk)) { entry.ready = true; ctx.post({ type: 'bgTask', id: runId, ready: true }); } + // Auto-preview: the moment a background command advertises a LOCAL address, offer to show + // it in the built-in browser. Fired at most ONCE per run — if the user closes the tab we + // must not reopen it on the next log line, and a restart-on-save server would otherwise + // spawn a tab per reload. The host decides whether to honour it (setting + dedupe). + if (!entry.previewUrl && typeof ctx.openPreview === 'function') { + const url = sniffPreviewUrl(chunk); + if (url) { entry.previewUrl = url; dbg('preview.detected', { id: runId, url: url }); ctx.openPreview(url); } + } } }; const onExit = (code, ms, how) => { diff --git a/extensions/levelcode-ai/extension.js b/extensions/levelcode-ai/extension.js index 5825b61..cb41cc8 100644 --- a/extensions/levelcode-ai/extension.js +++ b/extensions/levelcode-ai/extension.js @@ -665,6 +665,38 @@ function reapCommands() { for (const [, stop] of commandStops) { try { stop(); } catch (e) { /* already gone */ } } commandStops.clear(); bgRuns.clear(); + previewedUrls.clear(); // the servers are gone; a fresh session may legitimately preview again +} + +// Auto-preview (see openPreview): URLs already shown this session, so a chatty server can't stack tabs +// and — more importantly — so closing the tab is RESPECTED rather than undone by the next log line. +const previewedUrls = new Set(); + +/** + * Show a locally-served URL in the built-in Simple Browser, beside the chat. + * + * This exists because the browser was already there and nobody knew: LevelCode ships VS Code's + * simple-browser, but you had to know the command name to find it. Opening it automatically the moment + * the agent brings a site up turns an invisible feature into the obvious one. + * + * Three deliberate choices: `preserveFocus` so a server coming up never steals the caret from whoever + * is typing; `Beside` so the site sits next to the work rather than replacing it; and dedupe-by-URL so + * the user closing the tab is final. Never throws — a preview must not be able to fail a run. + */ +async function openPreview(url) { + if (!aiConfig().get('preview.autoOpen', true)) { return; } + if (!url || previewedUrls.has(url)) { return; } + previewedUrls.add(url); + try { + await vscode.commands.executeCommand('simpleBrowser.api.open', vscode.Uri.parse(url), { + preserveFocus: true, + viewColumn: vscode.ViewColumn.Beside + }); + post({ type: 'agentTool', icon: 'globe', text: '🌐 preview · ' + url }); + } catch (e) { + // simple-browser disabled or the id moved upstream — log, never surface as a run failure. + dbg('preview.failed', { url: url, error: String((e && e.message) || e) }); + } } // Workspace checkpoints: a per-user-turn stack of file pre-images so the user can roll the workspace back @@ -987,6 +1019,7 @@ async function agentFlow(text) { toolPolicy: userScopedSetting(cfg.inspect('mcp.toolPolicy'), {}) }, contextLimit: contextLimitFor(req.providerId, capsModel(req.model)), // Auto → flagship window; the model SENT stays req.model + openPreview: openPreview, // background server advertised a local URL → show it in-editor commandStops: commandStops, // runId → stop() (process-group kill); used by Stop button / ■ commandRuns: bgRuns, // runId → background-process registry (read_command_output reads it) commandTimeout: cfg.get('commandTimeout', 120000), // backstop before a command is force-killed (0 = off) diff --git a/extensions/levelcode-ai/package.json b/extensions/levelcode-ai/package.json index b29debf..cccd6b7 100644 --- a/extensions/levelcode-ai/package.json +++ b/extensions/levelcode-ai/package.json @@ -436,6 +436,11 @@ "default": false, "description": "Treat editor warnings (not just errors) as verification failures the agent must fix. Off by default — only errors block." }, + "levelcode.ai.preview.autoOpen": { + "type": "boolean", + "default": true, + "markdownDescription": "When the agent starts a web server in the background, automatically open the site in LevelCode's built-in browser, beside the chat.\n\nOnly **local** addresses are ever opened (`localhost` / `127.0.0.1`), and each address opens at most once per session — so closing the tab is respected. Your focus is never taken. Turn this off to open previews yourself with **Simple Browser: Show**." + }, "levelcode.ai.mcp.servers": { "type": "object", "default": {}, diff --git a/extensions/levelcode-ai/test/verify.test.js b/extensions/levelcode-ai/test/verify.test.js new file mode 100644 index 0000000..efd532d --- /dev/null +++ b/extensions/levelcode-ai/test/verify.test.js @@ -0,0 +1,119 @@ +/*--------------------------------------------------------------------------------------------- + * Unit tests for verify.js's output sniffers — run: node test/verify.test.js + * + * These read a background command's STDOUT, which is whatever a repo's dev script chose to print — + * i.e. untrusted text. One direction is load-bearing: + * + * sniffPreviewUrl decides what the editor's built-in browser is pointed at, automatically. If it + * ever returns a REMOTE url, a hostile repo can navigate the user's editor anywhere just by logging + * a line. Every "remote" case below must return null (or the local url found elsewhere in the text). + * Erring toward opening nothing is always safe; erring toward opening is not. + *--------------------------------------------------------------------------------------------*/ +// @ts-check +'use strict'; + +const assert = require('assert'); +const { sniffPreviewUrl, sniffPort, looksReady } = require('../verify'); + +let n = 0; +function test(name, fn) { fn(); n++; console.log(' ok - ' + name); } + +// ---- 1. the security bound: only LOCAL addresses may ever be opened --------------------------- + +// Real-looking lines a hostile or merely misconfigured repo could print. None may be opened. +const REMOTE = [ + 'Local: http://evil.example.com:3000/', + ' ➜ Network: https://attacker.test:8080/pwn', + 'Server started at http://169.254.169.254:80/latest/meta-data', // cloud metadata endpoint + 'listening: http://10.0.0.5:3000', + 'open http://sub.domain.co.uk:4200 to view', + 'http://localhost.evil.com:3000', // suffix trick — the host is NOT localhost + 'http://127.0.0.1.evil.com:3000' // same trick with the loopback literal +]; + +test('SECURITY: a remote url in command output is never opened', () => { + for (const line of REMOTE) { + const got = sniffPreviewUrl(line); + assert.ok( + got === null || /^https?:\/\/(?:localhost|127\.0\.0\.1)(?::|\/|$)/.test(got), + 'opened a non-local address from ' + JSON.stringify(line) + ' -> ' + JSON.stringify(got) + ); + } +}); + +test('SECURITY: a local url still wins when a remote one is printed alongside it', () => { + // Vite prints both; we must take the Local line and ignore Network, whatever its host. + const out = sniffPreviewUrl(' ➜ Local: http://localhost:5173/\n ➜ Network: http://192.168.1.14:5173/'); + assert.strictEqual(out, 'http://localhost:5173/'); +}); + +// ---- 2. finding the address the user actually wants ------------------------------------------- + +test('PREVIEW: prefers the full printed url, keeping scheme, port and base path', () => { + assert.strictEqual(sniffPreviewUrl(' ➜ Local: http://localhost:5173/'), 'http://localhost:5173/'); + assert.strictEqual(sniffPreviewUrl('ready - started server on http://localhost:3000/app'), 'http://localhost:3000/app'); + assert.strictEqual(sniffPreviewUrl('https://localhost:8443/'), 'https://localhost:8443/'); + assert.strictEqual(sniffPreviewUrl('running at http://127.0.0.1:4200'), 'http://127.0.0.1:4200'); +}); + +test('PREVIEW: bind addresses are rewritten to something a browser can actually resolve', () => { + // 0.0.0.0 / [::] mean "every interface", not a destination — opening them literally often fails. + assert.strictEqual(sniffPreviewUrl('Listening on http://0.0.0.0:8000'), 'http://localhost:8000'); + assert.strictEqual(sniffPreviewUrl('serving on http://[::]:9000/'), 'http://localhost:9000/'); +}); + +test('PREVIEW: falls back to localhost when only a port is announced', () => { + // Plenty of servers never print a url — the Express boilerplate is exactly this line. + assert.strictEqual(sniffPreviewUrl('Server running on port 3000'), 'http://localhost:3000'); + assert.strictEqual(sniffPreviewUrl('listening on :8080'), 'http://localhost:8080'); +}); + +test('PREVIEW: silence when nothing resembles a server', () => { + for (const quiet of ['', ' ', 'building...', 'Compiled 42 modules', 'error TS2304: cannot find name', null, undefined]) { + assert.strictEqual(sniffPreviewUrl(quiet), null, JSON.stringify(quiet) + ' should not open anything'); + } +}); + +test('PREVIEW: garbage input returns null instead of throwing', () => { + // This runs inside a stdout handler — a throw here would take down a live command stream. + for (const junk of [{}, [], 42, true, Symbol.iterator.toString()]) { + assert.doesNotThrow(() => sniffPreviewUrl(/** @type {any} */(junk))); + } +}); + +// ---- 3. the sniffers the preview builds on (previously untested) ------------------------------ + +test('PORT: the most specific pattern wins, so later logs cannot masquerade as the server', () => { + assert.strictEqual(sniffPort('http://localhost:5173/'), '5173'); + assert.strictEqual(sniffPort('Server running on port 3000'), '3000'); + assert.strictEqual(sniffPort('no port here'), null); + // A url earlier in the text takes precedence over a bare "port N" mentioned later. + assert.strictEqual(sniffPort('http://localhost:5173/ ... connected to db on port 5432'), '5173'); +}); + +test('READY: recognises the common "it is up" lines, and nothing else', () => { + for (const up of ['compiled successfully', 'Listening on :3000', 'server is running', 'ready in 412 ms', ' Local: http://x']) { + assert.strictEqual(looksReady(up), true, JSON.stringify(up)); + } + for (const notUp of ['', 'building...', 'error: failed to compile']) { + assert.strictEqual(looksReady(notUp), false, JSON.stringify(notUp)); + } +}); + +// ---- 4. source hygiene ------------------------------------------------------------------------ + +test('SOURCE: verify.js contains no raw control bytes', () => { + // It shipped with a raw NUL in the diagKey separator, which made `file` report "data" and made grep + // and diff skip the whole module in silence — you could not search your own source, and a reviewer + // saw only "Binary file matches". The runtime value of an escape is identical, so nothing else + // catches this. (Same defect was caught by review in the MCP modules.) + const buf = require('fs').readFileSync(require('path').join(__dirname, '..', 'verify.js')); + const bad = []; + for (let i = 0; i < buf.length; i++) { + const b = buf[i]; + if (b < 9 || (b > 13 && b < 32)) { bad.push(i); } + } + assert.deepStrictEqual(bad, [], 'raw control bytes at ' + bad.slice(0, 5).join(', ') + ' — use an escape'); +}); + +console.log('\nverify.js: ' + n + ' tests passed.'); diff --git a/extensions/levelcode-ai/verify.js b/extensions/levelcode-ai/verify.js index 74e2718a1f05f9c42997a97e89cf2199e248325a..63c5d3b47a44f3fb7f97741afa74efdd0bc573e6 100644 GIT binary patch delta 1782 zcmZuyO^@3|7#6C8v^nx|=52%&J5B6m+a5?*Rc{frQpBn%LuHs6AuNj5iIt zt8n6g#D(%7xFYq&oj=mQ!G-6YWLH&163KBq^S;m5y!kEtbN{cad+%o5E?%#$?;XCk zeqx+=w3M_|bxo~N+LK>Mnsuu7W2I?k&C*Fr!Y6bpB{{9C>RT%>m0X_NnqL3>2R0~2 zwQ`=0PQz{|i_R*)qM7M*nb0up6-MLv@zcXEj(>Ra^z`hM_9=b#^4AZB>ny*{E+&&K zk3UY;wAV|A!yXOk3^2laT6o_^E+$cyRys$5N_j~@bqQcUcBm^#eD%u7azs{&az)`G=M&Ere|_yBh0M!*IBVER{DFJ??$UH?_&3{HJq^qn16dg@%Qn}>vaOoAZ|Nkon}j!|gyiHT zfa=SmZ@#*Xh;(EHcvz4Tn){RzWVP@LIUJD-AXZ`D+R7!u>e*rgMj*i8Mi5*T3&z08 zlPG*AYS2amO?@fdt?M`pm9%VJg&=KI=DZjPg3ZAVI1njVB&=1y#!+&`LN;4cyknWr*&1MY2e0Qwg#h-V5DroW!j*!$*d!2OJzmP$rcBi5BQG`kkEfyZ<3|to z`oOc1t_Z`0$#y(KSP(B}+ylR@sHhRxDXkmj92dc9skf&Znuj=Y1zsFNiK?asRw;aE zp&U$vrqV)tZfu0Qzf^Ek`%VSJFz zF~xRj>8ZgLQUG)mHE3WZPJ=8k%VN5}zfU}Q0+RKPswleu#&}FT0vk0KdpXu#;dE-m zw}V_aM3kl|6;8?PUw#i`#dJZ1pg?bD%Zy_j#%y#j+0Bye=Z#@|*JPc?bU=fWv>EqsQoA4A+*a;NBLli29hm#=Nw0F~c_4kMWyz|jp|41W*+dOwC;a}ip G+WQXy`B57H delta 34 pcmZ2(+pE8!nwgPdb0f2}(B=~|2N?Ns^7FHcgHjVyDivz2xd79#3&8*Y From 81d7614625442685c77e3fa8a888b794ee6306f6 Mon Sep 17 00:00:00 2001 From: Sergii Demianchuk Date: Thu, 23 Jul 2026 20:56:07 -0400 Subject: [PATCH 2/4] fix(ai): a failed preview open must stay retryable (PR #35 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit openPreview added the URL to the "already previewed" set BEFORE attempting to open it. If simpleBrowser.api.open threw — Simple Browser disabled, or briefly unavailable — the URL stayed marked, so no later run in that session would retry and the preview silently never appeared, with nothing for the user to point at. The marking wasn't arbitrary, though: opening is async, so two runs advertising the same address could otherwise both pass the check and stack two tabs. So the fix keeps BOTH properties instead of just moving the line — three states rather than one Set: never-tried, in-flight, shown. begin() -> in-flight, blocks a concurrent duplicate succeeded() -> shown; never reopen, so closing the tab stays final failed() -> forget the attempt entirely; a later run may retry Extracted as createPreviewGate() in verify.js rather than left inline, because extension.js requires vscode and cannot be unit-tested — this is precisely the kind of state-machine bug that needs a test, and inline it would have had none. Verified: 23 suites, 0 failures (verify.test.js now 15 cases, 5 new). The fix is mutation-checked against the original bug: re-adding the URL to `shown` inside begin() fails with "a failed open must not suppress later attempts". Co-Authored-By: Claude Opus 4.8 --- extensions/levelcode-ai/extension.js | 24 ++++++---- extensions/levelcode-ai/test/verify.test.js | 50 ++++++++++++++++++++- extensions/levelcode-ai/verify.js | 29 +++++++++++- 3 files changed, 93 insertions(+), 10 deletions(-) diff --git a/extensions/levelcode-ai/extension.js b/extensions/levelcode-ai/extension.js index cb41cc8..08fc1c6 100644 --- a/extensions/levelcode-ai/extension.js +++ b/extensions/levelcode-ai/extension.js @@ -22,7 +22,7 @@ const { registerInlineComplete } = require('./inlineComplete'); const { runAgent } = require('./agent'); const { findCompactionCut, estimateMsgTokens } = require('./agentMemory'); const { registerReview } = require('./reviewSession'); -const { formatDiagnosticLines, diagKey } = require('./verify'); +const { formatDiagnosticLines, diagKey, createPreviewGate } = require('./verify'); const { loadSkills, skillsMenu, getSkillBody } = require('./skills'); const { openCustomize } = require('./customize'); const { importFromVscode } = require('./importVscode'); @@ -665,12 +665,13 @@ function reapCommands() { for (const [, stop] of commandStops) { try { stop(); } catch (e) { /* already gone */ } } commandStops.clear(); bgRuns.clear(); - previewedUrls.clear(); // the servers are gone; a fresh session may legitimately preview again + previewGate.clear(); // the servers are gone; a fresh session may legitimately preview again } -// Auto-preview (see openPreview): URLs already shown this session, so a chatty server can't stack tabs -// and — more importantly — so closing the tab is RESPECTED rather than undone by the next log line. -const previewedUrls = new Set(); +// Auto-preview (see openPreview): tracks which addresses have actually been SHOWN this session, so a +// chatty server can't stack tabs and — more importantly — so closing the tab is RESPECTED rather than +// undone by the next log line. A FAILED open stays retryable; see createPreviewGate. +const previewGate = createPreviewGate(); /** * Show a locally-served URL in the built-in Simple Browser, beside the chat. @@ -682,19 +683,26 @@ const previewedUrls = new Set(); * Three deliberate choices: `preserveFocus` so a server coming up never steals the caret from whoever * is typing; `Beside` so the site sits next to the work rather than replacing it; and dedupe-by-URL so * the user closing the tab is final. Never throws — a preview must not be able to fail a run. + * + * The gate records a URL as shown only AFTER the open succeeds (PR #35 review): marking it up-front + * meant one transient failure — Simple Browser disabled for a moment — blacklisted that address for the + * whole session, so the preview silently never appeared again. See createPreviewGate. */ async function openPreview(url) { if (!aiConfig().get('preview.autoOpen', true)) { return; } - if (!url || previewedUrls.has(url)) { return; } - previewedUrls.add(url); + if (!previewGate.shouldOpen(url)) { return; } + previewGate.begin(url); try { await vscode.commands.executeCommand('simpleBrowser.api.open', vscode.Uri.parse(url), { preserveFocus: true, viewColumn: vscode.ViewColumn.Beside }); + previewGate.succeeded(url); // it really appeared → never reopen, so closing the tab is final post({ type: 'agentTool', icon: 'globe', text: '🌐 preview · ' + url }); } catch (e) { - // simple-browser disabled or the id moved upstream — log, never surface as a run failure. + // simple-browser disabled or the id moved upstream — log, never surface as a run failure. The + // URL stays retryable: nothing was shown, so a later run should be free to try again. + previewGate.failed(url); dbg('preview.failed', { url: url, error: String((e && e.message) || e) }); } } diff --git a/extensions/levelcode-ai/test/verify.test.js b/extensions/levelcode-ai/test/verify.test.js index efd532d..ebf0262 100644 --- a/extensions/levelcode-ai/test/verify.test.js +++ b/extensions/levelcode-ai/test/verify.test.js @@ -13,7 +13,7 @@ 'use strict'; const assert = require('assert'); -const { sniffPreviewUrl, sniffPort, looksReady } = require('../verify'); +const { sniffPreviewUrl, sniffPort, looksReady, createPreviewGate } = require('../verify'); let n = 0; function test(name, fn) { fn(); n++; console.log(' ok - ' + name); } @@ -100,6 +100,54 @@ test('READY: recognises the common "it is up" lines, and nothing else', () => { } }); +// ---- 3b. the preview gate: shown-once, but a FAILED open stays retryable ---------------------- + +test('GATE: a successful open is final — closing the tab is never undone', () => { + const g = createPreviewGate(); + const url = 'http://localhost:3000'; + assert.strictEqual(g.shouldOpen(url), true); + g.begin(url); + g.succeeded(url); + // The user may now close that tab. Nothing — no later run, no chatty log line — may reopen it. + assert.strictEqual(g.shouldOpen(url), false); +}); + +test('GATE: a FAILED open is retryable — the bug this gate exists for', () => { + // PR #35 review: the first version marked the URL as previewed BEFORE attempting to open it, so a + // single transient failure (Simple Browser disabled for a moment) blacklisted that address for the + // rest of the session — the preview then silently never appeared, with nothing to point at. + const g = createPreviewGate(); + const url = 'http://localhost:5173'; + g.begin(url); + g.failed(url); + assert.strictEqual(g.shouldOpen(url), true, 'a failed open must not suppress later attempts'); + // …and a later attempt that works still closes the door exactly once. + g.begin(url); g.succeeded(url); + assert.strictEqual(g.shouldOpen(url), false); +}); + +test('GATE: an in-flight open blocks a concurrent duplicate', () => { + // Opening is async, so two runs advertising the same address could both pass the check and stack + // two tabs. This is why "have we shown it" alone is not enough state. + const g = createPreviewGate(); + const url = 'http://localhost:8080'; + g.begin(url); + assert.strictEqual(g.shouldOpen(url), false, 'must not open the same URL twice concurrently'); +}); + +test('GATE: distinct URLs are independent, and clear() resets everything', () => { + const g = createPreviewGate(); + g.begin('http://localhost:3000'); g.succeeded('http://localhost:3000'); + assert.strictEqual(g.shouldOpen('http://localhost:4000'), true, 'a different port is a different site'); + g.clear(); + assert.strictEqual(g.shouldOpen('http://localhost:3000'), true, 'New Chat may legitimately preview again'); +}); + +test('GATE: junk urls are never openable', () => { + const g = createPreviewGate(); + for (const bad of [null, undefined, '']) { assert.strictEqual(g.shouldOpen(/** @type {any} */(bad)), false); } +}); + // ---- 4. source hygiene ------------------------------------------------------------------------ test('SOURCE: verify.js contains no raw control bytes', () => { diff --git a/extensions/levelcode-ai/verify.js b/extensions/levelcode-ai/verify.js index 63c5d3b..811c0ce 100644 --- a/extensions/levelcode-ai/verify.js +++ b/extensions/levelcode-ai/verify.js @@ -128,9 +128,36 @@ function sniffPreviewUrl(text) { return port ? 'http://localhost:' + port : null; } +/** + * Tracks which preview URLs have actually been SHOWN, so the auto-preview opens each address once and + * never fights the user who closed the tab. + * + * The distinction that matters (PR #35 review): a URL counts as shown only when the open SUCCEEDED. An + * attempt that throws — Simple Browser disabled, or unavailable for a moment — must stay retryable, or + * one transient failure silently suppresses the preview for the rest of the session and the feature + * looks broken with nothing to point at. Hence three states rather than a single Set: never-tried, + * in-flight, shown. In-flight exists because opening is async, so two runs advertising the same address + * could otherwise both pass the check and stack two tabs. + */ +function createPreviewGate() { + const shown = new Set(); + const inFlight = new Set(); + return { + /** May we try to open this URL right now? */ + shouldOpen(url) { return !!url && !shown.has(url) && !inFlight.has(url); }, + begin(url) { inFlight.add(url); }, + /** It really appeared — never open it again, so closing the tab is final. */ + succeeded(url) { inFlight.delete(url); shown.add(url); }, + /** It did NOT appear — forget the attempt entirely so a later run can retry. */ + failed(url) { inFlight.delete(url); }, + clear() { shown.clear(); inFlight.clear(); }, + get size() { return shown.size; } + }; +} + /** Does this output line signal the server/watcher is up and ready? (Used even when no port is found.) */ function looksReady(text) { return /\bcompiled successfully\b|\blistening on\b|\bserver (?:is )?(?:running|started|ready)\b|\bwatching for file changes\b|\bready in\b|\bLocal:\s/i.test(String(text == null ? '' : text)); } -module.exports = { SEV, diagKey, formatDiagnosticLines, verifyOutcome, formatVerifyFeedback, looksUnrunnable, sniffPort, sniffPreviewUrl, looksReady }; +module.exports = { SEV, diagKey, formatDiagnosticLines, verifyOutcome, formatVerifyFeedback, looksUnrunnable, sniffPort, sniffPreviewUrl, createPreviewGate, looksReady }; From e0c87b4feea1766f246ca04b3cdaf9f0bc2ce4f8 Mon Sep 17 00:00:00 2001 From: Sergii Demianchuk Date: Thu, 23 Jul 2026 21:09:00 -0400 Subject: [PATCH 3/4] fix(ai): remote-only output must open nothing; document the real local bound (PR #35 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. The SECURITY test allowed sniffPreviewUrl to return a localhost URL for the remote-only fixtures, so it was weaker than the sentence above it claimed — and it was permitting a real leak, not a theoretical one. sniffPort's first pattern matches ANY http(s) URL, so after refusing a remote host the fallback re-extracted the port out of that same refused address: "Local: http://evil.example.com:3000/" -> "http://localhost:3000" A preview conjured entirely out of a line we had deliberately ignored. The fallback now blanks remote URLs first, so remote-only output yields null. The assertion is strict equality against null, matching its stated contract. A bare port with no host ("running on port 3000") still works — that is the Express-boilerplate case the fallback exists for — and mixed Local/Network output still picks the local one. 2. The setting description understated the bound: it said localhost / 127.0.0.1, while the implementation also accepts the IPv6 loopback [::1] and treats the bind addresses 0.0.0.0 and [::] as local (rewriting them to localhost, which a browser can actually resolve). Now says so. Verified: 23 suites, 0 failures. Mutation-checked — restoring the raw-text fallback reproduces the leak above and fails the suite. Co-Authored-By: Claude Opus 4.8 --- extensions/levelcode-ai/package.json | 2 +- extensions/levelcode-ai/test/verify.test.js | 14 ++++++++------ extensions/levelcode-ai/verify.js | 8 +++++++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/extensions/levelcode-ai/package.json b/extensions/levelcode-ai/package.json index cccd6b7..ffdba4f 100644 --- a/extensions/levelcode-ai/package.json +++ b/extensions/levelcode-ai/package.json @@ -439,7 +439,7 @@ "levelcode.ai.preview.autoOpen": { "type": "boolean", "default": true, - "markdownDescription": "When the agent starts a web server in the background, automatically open the site in LevelCode's built-in browser, beside the chat.\n\nOnly **local** addresses are ever opened (`localhost` / `127.0.0.1`), and each address opens at most once per session — so closing the tab is respected. Your focus is never taken. Turn this off to open previews yourself with **Simple Browser: Show**." + "markdownDescription": "When the agent starts a web server in the background, automatically open the site in LevelCode's built-in browser, beside the chat.\n\nOnly **local** addresses are ever opened — `localhost`, `127.0.0.1`, or the IPv6 loopback `[::1]`. The bind addresses `0.0.0.0` and `[::]` count as local and are shown as `localhost`, since a browser can't resolve them. A remote URL printed by a dev script is ignored, never opened.\n\nEach address opens at most once per session, so closing the tab is respected, and your focus is never taken. Turn this off to open previews yourself with **Simple Browser: Show**." }, "levelcode.ai.mcp.servers": { "type": "object", diff --git a/extensions/levelcode-ai/test/verify.test.js b/extensions/levelcode-ai/test/verify.test.js index ebf0262..39da9aa 100644 --- a/extensions/levelcode-ai/test/verify.test.js +++ b/extensions/levelcode-ai/test/verify.test.js @@ -31,13 +31,15 @@ const REMOTE = [ 'http://127.0.0.1.evil.com:3000' // same trick with the loopback literal ]; -test('SECURITY: a remote url in command output is never opened', () => { +test('SECURITY: remote-only output opens NOTHING', () => { + // Strictly null, not "null or some localhost url". An earlier version of this assertion allowed a + // localhost fallback, which quietly permitted the thing it was meant to forbid: the port would be + // re-extracted from the refused remote address and we'd open localhost: — a preview + // conjured entirely out of a line we ignored. If the fixture is remote-only, the answer is nothing. for (const line of REMOTE) { - const got = sniffPreviewUrl(line); - assert.ok( - got === null || /^https?:\/\/(?:localhost|127\.0\.0\.1)(?::|\/|$)/.test(got), - 'opened a non-local address from ' + JSON.stringify(line) + ' -> ' + JSON.stringify(got) - ); + assert.strictEqual(sniffPreviewUrl(line), null, + 'remote-only output must open nothing, got ' + JSON.stringify(sniffPreviewUrl(line)) + + ' from ' + JSON.stringify(line)); } }); diff --git a/extensions/levelcode-ai/verify.js b/extensions/levelcode-ai/verify.js index 811c0ce..5d30c9a 100644 --- a/extensions/levelcode-ai/verify.js +++ b/extensions/levelcode-ai/verify.js @@ -124,7 +124,13 @@ function sniffPreviewUrl(text) { const host = /^(?:0\.0\.0\.0|\[::\])$/i.test(m[2]) ? 'localhost' : m[2]; return m[1].toLowerCase() + '://' + host + ':' + m[3] + (m[4] || ''); } - const port = sniffPort(s); + // Nothing local was printed as a URL. Before falling back to a bare port, blank out any REMOTE one: + // sniffPort's first pattern matches any http(s) URL, so it would re-extract the port out of the very + // address we just refused and we'd open localhost: — a preview inferred entirely from a + // line we deliberately ignored. A port with no host attached ("running on port 3000") still counts. + const localOnly = s.replace(/(https?):\/\/([a-z0-9.\-]+|\[[0-9a-f:]*\]):(\d{2,5})(\/[^\s"'<>)\]]*)?/gi, + (full, _scheme, host) => (LOCAL_HOSTS.test(host) ? full : ' ')); + const port = sniffPort(localOnly); return port ? 'http://localhost:' + port : null; } From 738e85f2a6a19d16e5f793cb2fb632c000eaaf51 Mon Sep 17 00:00:00 2001 From: Sergii Demianchuk Date: Thu, 23 Jul 2026 21:16:21 -0400 Subject: [PATCH 4/4] fix(ai): sniff the accumulated tail, not the raw chunk (PR #35 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. runCommand streams arbitrary stdout slices, so a dev server's address routinely arrives split across a boundary — "http://local" + "host:5173/" — and neither half matches. The preview would then silently never open, which is the worst failure mode for this feature because there is nothing to point at. Simulated against the real streaming path: per-chunk (old): null tail (new): "http://localhost:5173/" The sniffs now read a bounded tail of entry.ring, which was already being maintained for read_command_output. 8 KB is far more than any single log line needs, and bounding it keeps the rescan cheap for a noisy watcher that never prints an address at all. Applied to ALL THREE sniffs, not just the preview: port and ready had the same latent gap, and fixing only the new one would have left the identical bug in place next to it. 2. ctx.openPreview is async and was called bare. Now wrapped so a rejection can never escape into a live stream handler — showing a browser tab must not be able to disturb a running command's output. 3. The junk-input test asserted only doesNotThrow. For a stdout handler that is necessary but not sufficient: a non-null return would still pop a browser tab. It now asserts the value is null, over a wider set of junk. Verified: 23 suites, 0 failures; verify.test.js is 16 cases. The split-chunk case is pinned with the real sniffer — each half must not match, the join must. Co-Authored-By: Claude Opus 4.8 --- extensions/levelcode-ai/agent.js | 25 +++++++++++++++++---- extensions/levelcode-ai/test/verify.test.js | 23 +++++++++++++++---- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/extensions/levelcode-ai/agent.js b/extensions/levelcode-ai/agent.js index 2059142..f57fd33 100644 --- a/extensions/levelcode-ai/agent.js +++ b/extensions/levelcode-ai/agent.js @@ -69,6 +69,11 @@ function buildSystem(menu) { } const TOOLS_TOKENS_EST = Math.round(JSON.stringify(TOOLS).length / 4); +// How much of a background command's accumulated output the sniffers re-read on each chunk. Generous +// next to any single log line, so a url split across chunk boundaries is still found, yet small enough +// that a noisy watcher which never prints an address costs nothing to keep scanning. +const SNIFF_TAIL = 8192; + const FILE_EXCLUDES = '{**/node_modules/**,**/.git/**,**/out/**,**/dist/**,**/.vscode-test/**,**/*.map}'; // ---- ripgrep search (self-contained) --------------------------------------- @@ -392,15 +397,27 @@ async function runTool(tu, ctx) { if (entry) { entry.ring = (entry.ring + chunk).slice(-100000); // bounded tail for read_command_output entry.totalBytes += chunk.length; - if (!entry.port) { const p = sniffPort(chunk); if (p) { entry.port = p; ctx.post({ type: 'bgTask', id: runId, port: p }); } } - if (!entry.ready && looksReady(chunk)) { entry.ready = true; ctx.post({ type: 'bgTask', id: runId, ready: true }); } + // Sniff the accumulated TAIL, never the raw chunk: stdout arrives in arbitrary slices, so + // a line can straddle a boundary ("http://local" + "host:5173/") and match neither half. + // A few KB is far more than any single line needs, and bounding it keeps the rescan cheap + // for a chatty server that never prints an address at all. (Applies to all three sniffs — + // port and ready had the same latent gap.) + const tail = entry.ring.slice(-SNIFF_TAIL); + if (!entry.port) { const p = sniffPort(tail); if (p) { entry.port = p; ctx.post({ type: 'bgTask', id: runId, port: p }); } } + if (!entry.ready && looksReady(tail)) { entry.ready = true; ctx.post({ type: 'bgTask', id: runId, ready: true }); } // Auto-preview: the moment a background command advertises a LOCAL address, offer to show // it in the built-in browser. Fired at most ONCE per run — if the user closes the tab we // must not reopen it on the next log line, and a restart-on-save server would otherwise // spawn a tab per reload. The host decides whether to honour it (setting + dedupe). if (!entry.previewUrl && typeof ctx.openPreview === 'function') { - const url = sniffPreviewUrl(chunk); - if (url) { entry.previewUrl = url; dbg('preview.detected', { id: runId, url: url }); ctx.openPreview(url); } + const url = sniffPreviewUrl(tail); + if (url) { + entry.previewUrl = url; + dbg('preview.detected', { id: runId, url: url }); + // Never let a preview reject inside a live stream handler — showing a browser tab + // must not be able to disturb a running command's output. + Promise.resolve(ctx.openPreview(url)).catch((e) => dbg('preview.rejected', { id: runId, error: String((e && e.message) || e) })); + } } } }; diff --git a/extensions/levelcode-ai/test/verify.test.js b/extensions/levelcode-ai/test/verify.test.js index 39da9aa..fa71c48 100644 --- a/extensions/levelcode-ai/test/verify.test.js +++ b/extensions/levelcode-ai/test/verify.test.js @@ -76,13 +76,28 @@ test('PREVIEW: silence when nothing resembles a server', () => { } }); -test('PREVIEW: garbage input returns null instead of throwing', () => { - // This runs inside a stdout handler — a throw here would take down a live command stream. - for (const junk of [{}, [], 42, true, Symbol.iterator.toString()]) { - assert.doesNotThrow(() => sniffPreviewUrl(/** @type {any} */(junk))); +test('PREVIEW: garbage input returns null — not merely "does not throw"', () => { + // This runs inside a stdout handler, so not throwing is necessary but nowhere near sufficient: + // returning a non-null url would still pop a browser tab. Assert the value, not just the absence + // of an exception. + for (const junk of [{}, [], 42, true, Symbol.iterator.toString(), NaN, () => {}]) { + let got; + assert.doesNotThrow(() => { got = sniffPreviewUrl(/** @type {any} */(junk)); }, 'threw on ' + String(junk)); + assert.strictEqual(got, null, 'opened something from junk input: ' + String(junk)); } }); +test('SPLIT: a url straddling two stdout chunks matches only once reassembled', () => { + // Why agent.js sniffs the accumulated ring TAIL rather than the raw chunk. runCommand streams + // arbitrary slices, so a dev server's address routinely arrives in two pieces — and each piece on + // its own is invisible to the sniffer, which would mean the preview silently never opened. + const first = ' ➜ Local: http://local'; + const second = 'host:5173/\n'; + assert.strictEqual(sniffPreviewUrl(first), null, 'the leading half must not match on its own'); + assert.strictEqual(sniffPreviewUrl(second), null, 'the trailing half must not match on its own'); + assert.strictEqual(sniffPreviewUrl(first + second), 'http://localhost:5173/', 'reassembled, it must'); +}); + // ---- 3. the sniffers the preview builds on (previously untested) ------------------------------ test('PORT: the most specific pattern wins, so later logs cannot masquerade as the server', () => {