Skip to content

Commit 738e85f

Browse files
ndemiancclaude
andcommitted
fix(ai): sniff the accumulated tail, not the raw chunk (PR #35 review)
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 <noreply@anthropic.com>
1 parent e0c87b4 commit 738e85f

2 files changed

Lines changed: 40 additions & 8 deletions

File tree

extensions/levelcode-ai/agent.js

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ function buildSystem(menu) {
6969
}
7070
const TOOLS_TOKENS_EST = Math.round(JSON.stringify(TOOLS).length / 4);
7171

72+
// How much of a background command's accumulated output the sniffers re-read on each chunk. Generous
73+
// next to any single log line, so a url split across chunk boundaries is still found, yet small enough
74+
// that a noisy watcher which never prints an address costs nothing to keep scanning.
75+
const SNIFF_TAIL = 8192;
76+
7277
const FILE_EXCLUDES = '{**/node_modules/**,**/.git/**,**/out/**,**/dist/**,**/.vscode-test/**,**/*.map}';
7378

7479
// ---- ripgrep search (self-contained) ---------------------------------------
@@ -392,15 +397,27 @@ async function runTool(tu, ctx) {
392397
if (entry) {
393398
entry.ring = (entry.ring + chunk).slice(-100000); // bounded tail for read_command_output
394399
entry.totalBytes += chunk.length;
395-
if (!entry.port) { const p = sniffPort(chunk); if (p) { entry.port = p; ctx.post({ type: 'bgTask', id: runId, port: p }); } }
396-
if (!entry.ready && looksReady(chunk)) { entry.ready = true; ctx.post({ type: 'bgTask', id: runId, ready: true }); }
400+
// Sniff the accumulated TAIL, never the raw chunk: stdout arrives in arbitrary slices, so
401+
// a line can straddle a boundary ("http://local" + "host:5173/") and match neither half.
402+
// A few KB is far more than any single line needs, and bounding it keeps the rescan cheap
403+
// for a chatty server that never prints an address at all. (Applies to all three sniffs —
404+
// port and ready had the same latent gap.)
405+
const tail = entry.ring.slice(-SNIFF_TAIL);
406+
if (!entry.port) { const p = sniffPort(tail); if (p) { entry.port = p; ctx.post({ type: 'bgTask', id: runId, port: p }); } }
407+
if (!entry.ready && looksReady(tail)) { entry.ready = true; ctx.post({ type: 'bgTask', id: runId, ready: true }); }
397408
// Auto-preview: the moment a background command advertises a LOCAL address, offer to show
398409
// it in the built-in browser. Fired at most ONCE per run — if the user closes the tab we
399410
// must not reopen it on the next log line, and a restart-on-save server would otherwise
400411
// spawn a tab per reload. The host decides whether to honour it (setting + dedupe).
401412
if (!entry.previewUrl && typeof ctx.openPreview === 'function') {
402-
const url = sniffPreviewUrl(chunk);
403-
if (url) { entry.previewUrl = url; dbg('preview.detected', { id: runId, url: url }); ctx.openPreview(url); }
413+
const url = sniffPreviewUrl(tail);
414+
if (url) {
415+
entry.previewUrl = url;
416+
dbg('preview.detected', { id: runId, url: url });
417+
// Never let a preview reject inside a live stream handler — showing a browser tab
418+
// must not be able to disturb a running command's output.
419+
Promise.resolve(ctx.openPreview(url)).catch((e) => dbg('preview.rejected', { id: runId, error: String((e && e.message) || e) }));
420+
}
404421
}
405422
}
406423
};

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,28 @@ test('PREVIEW: silence when nothing resembles a server', () => {
7676
}
7777
});
7878

79-
test('PREVIEW: garbage input returns null instead of throwing', () => {
80-
// This runs inside a stdout handler — a throw here would take down a live command stream.
81-
for (const junk of [{}, [], 42, true, Symbol.iterator.toString()]) {
82-
assert.doesNotThrow(() => sniffPreviewUrl(/** @type {any} */(junk)));
79+
test('PREVIEW: garbage input returns null — not merely "does not throw"', () => {
80+
// This runs inside a stdout handler, so not throwing is necessary but nowhere near sufficient:
81+
// returning a non-null url would still pop a browser tab. Assert the value, not just the absence
82+
// of an exception.
83+
for (const junk of [{}, [], 42, true, Symbol.iterator.toString(), NaN, () => {}]) {
84+
let got;
85+
assert.doesNotThrow(() => { got = sniffPreviewUrl(/** @type {any} */(junk)); }, 'threw on ' + String(junk));
86+
assert.strictEqual(got, null, 'opened something from junk input: ' + String(junk));
8387
}
8488
});
8589

90+
test('SPLIT: a url straddling two stdout chunks matches only once reassembled', () => {
91+
// Why agent.js sniffs the accumulated ring TAIL rather than the raw chunk. runCommand streams
92+
// arbitrary slices, so a dev server's address routinely arrives in two pieces — and each piece on
93+
// its own is invisible to the sniffer, which would mean the preview silently never opened.
94+
const first = ' ➜ Local: http://local';
95+
const second = 'host:5173/\n';
96+
assert.strictEqual(sniffPreviewUrl(first), null, 'the leading half must not match on its own');
97+
assert.strictEqual(sniffPreviewUrl(second), null, 'the trailing half must not match on its own');
98+
assert.strictEqual(sniffPreviewUrl(first + second), 'http://localhost:5173/', 'reassembled, it must');
99+
});
100+
86101
// ---- 3. the sniffers the preview builds on (previously untested) ------------------------------
87102

88103
test('PORT: the most specific pattern wins, so later logs cannot masquerade as the server', () => {

0 commit comments

Comments
 (0)