Skip to content

Commit fca2978

Browse files
committed
fix: a long command's failure and exit code are what the model sees
The tool result was (output + status).slice(0, 8000): for long output the end — where the failure and the '[exit 1]' line are — was exactly what got cut, so the model could not tell a command had failed. Output that fits is returned byte for byte as before; longer output keeps a quarter from the start and the rest from the end, with the gap said out loud, and the status line is never what is cut. DESKTOP-VISIBLE behaviour change (outputs over 8000 characters only), in its own commit so it can be dropped on its own.
1 parent 972c354 commit fca2978

2 files changed

Lines changed: 126 additions & 1 deletion

File tree

‎extensions/levelcode-ai/agent.js‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,25 @@ function inputPreview(input, redact) {
395395
/** Cheap binary sniff — a NUL byte in the first 8KB means it isn't text we should round-trip as UTF-8. */
396396
function isBinaryFile(abs) { return host.isBinary(abs); }
397397

398+
/**
399+
* What the model is told about a finished command: its output and its status line, in at most `max` characters.
400+
* Output that fits is returned exactly as it always was. Output that does not keeps its START and its END with
401+
* the gap said out loud, and the status line (`[exit 1]`, `[timed out]`, `[stopped by user]`) is never what is cut:
402+
* it used to be, because it sits at the end and the whole thing was sliced from the front.
403+
* @param {string} out
404+
* @param {string} note
405+
* @param {number} [max]
406+
*/
407+
function commandResult(out, note, max) {
408+
max = max || 8000;
409+
if (out.length + note.length <= max) { return out + note; }
410+
const gapText = (n) => '\n… [' + n + ' characters of output omitted] …\n';
411+
const room = max - note.length - gapText(out.length).length;
412+
const head = Math.max(0, Math.floor(room / 4)); // a quarter for the start, the rest for the end — where the failure is
413+
const tail = Math.max(0, room - head);
414+
return out.slice(0, head) + gapText(out.length - head - tail) + out.slice(out.length - tail) + note;
415+
}
416+
398417
// Run a shell command, STREAMING its output: onChunk(text, 'stdout'|'stderr') fires as it runs (live
399418
// terminal in the chat), onExit(code, ms, how) at the end. onStart(child, stop) exposes a stop() that
400419
// kills the whole process GROUP (server + children) — needed because a server is a child of the shell.
@@ -436,7 +455,7 @@ function runCommand(root, command, onChunk, onExit, onStart, timeoutMs, opts) {
436455
const exit = stopped ? 130 : (timedOut ? 124 : (code == null ? -1 : code));
437456
if (onExit) { onExit(exit, ms, how); }
438457
const note = stopped ? '\n[stopped by user]' : timedOut ? '\n[timed out]' : (exit !== 0 ? '\n[exit ' + exit + ']' : '');
439-
resolve((out + note).slice(0, 8000) || '(no output)');
458+
resolve(commandResult(out, note) || '(no output)');
440459
};
441460
child.on('close', (code) => finish(code));
442461
child.on('error', (e) => { cap('\n' + ((e && e.message) || e), 'stderr'); finish(1); });
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* What the model is told when a command ends — run: node test/agentCommandResult.test.js
3+
*
4+
* The bug this locks down: the tool result was `(output + status).slice(0, 8000)`. The status line —
5+
* `[exit 1]`, `[timed out]`, `[stopped by user]` — and the end of the output, which is where a failing
6+
* test run puts its failure, sit at the END, so the longer the output the surer it was to be cut. A
7+
* failing command with 30 000 characters of output came back as its first 8000, with no exit code at
8+
* all, and the model had no way to know it had failed.
9+
*
10+
* Output that fits is unchanged, byte for byte. Output that does not keeps its start and its end.
11+
* The real runAgent → runTool → run_command → runCommand chain runs, with a fake child.
12+
*--------------------------------------------------------------------------------------------*/
13+
// @ts-check
14+
'use strict';
15+
16+
const assert = require('assert');
17+
const fs = require('fs');
18+
const os = require('os');
19+
const path = require('path');
20+
const Module = require('module');
21+
const { EventEmitter } = require('events');
22+
23+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'lc-result-'));
24+
const vscodeMock = { workspace: { workspaceFolders: [{ uri: { fsPath: root }, name: 'app' }] } };
25+
let child;
26+
const childProcessMock = Object.assign({}, require('child_process'), {
27+
// kill() ends the child the way a real one does: it closes.
28+
spawn() { child = Object.assign(new EventEmitter(), { stdout: new EventEmitter(), stderr: new EventEmitter(), kill() { setImmediate(() => child.emit('close', null)); return true; } }); return child; },
29+
});
30+
const origLoad = Module._load;
31+
// @ts-ignore
32+
Module._load = function (request, parent, isMain) {
33+
if (request === 'vscode') { return vscodeMock; }
34+
if (request === 'child_process') { return childProcessMock; }
35+
return origLoad.call(this, request, parent, isMain);
36+
};
37+
const providers = require('../providers/index');
38+
let script = [];
39+
const seen = [];
40+
providers.streamAgentTurn = async (o) => { seen.push(JSON.parse(JSON.stringify(o.messages))); const t = script.shift(); if (!t) { throw new Error('script exhausted'); } return t; };
41+
const { runAgent } = require('../agent');
42+
43+
let n = 0;
44+
async function test(name, fn) { seen.length = 0; await fn(); n++; console.log(' ok - ' + name); }
45+
const settle = () => new Promise((r) => setTimeout(r, 5));
46+
47+
/** Run one command to its end. `drive(child)` plays the process; `ctxOver` can set the timeout or stop it. */
48+
async function result(drive, ctxOver) {
49+
seen.length = 0;
50+
script = [
51+
{ stop_reason: 'tool_use', content: [{ type: 'tool_use', id: 't1', name: 'run_command', input: { command: 'x', explanation: 'x' } }] },
52+
{ stop_reason: 'end_turn', content: [{ type: 'text', text: 'Done: x' }] },
53+
];
54+
const ctx = Object.assign({ messages: [{ role: 'user', content: 'go' }], maxSteps: 4, post() {}, dbg() {}, approve: async () => true, commandRuns: new Map(), commandStops: new Map(), signal: new AbortController().signal }, ctxOver);
55+
const p = runAgent(ctx);
56+
await settle(); await settle();
57+
await drive(child, ctx);
58+
await p;
59+
for (const m of seen) { const u = m[m.length - 1]; if (u.role === 'user' && Array.isArray(u.content)) { const r = u.content.find((c) => c.tool_use_id === 't1'); if (r) { return r.content; } } }
60+
throw new Error('no tool result');
61+
}
62+
const line = (i) => 'line ' + String(i).padStart(4, '0') + ' ' + 'x'.repeat(20) + '\n';
63+
64+
(async () => {
65+
try {
66+
await test('SHORT OUTPUT IS UNCHANGED: output, then the exit note', async () => {
67+
assert.strictEqual(await result((c) => { c.stdout.emit('data', Buffer.from('ok')); c.emit('close', 2); }), 'ok\n[exit 2]');
68+
assert.strictEqual(await result((c) => { c.stdout.emit('data', Buffer.from('fine\n')); c.emit('close', 0); }), 'fine\n');
69+
assert.strictEqual(await result((c) => { c.emit('close', 0); }), '(no output)');
70+
});
71+
72+
await test('LONG OUTPUT: the exit code and the failure at the end are what the model sees', async () => {
73+
const r = await result((c) => {
74+
for (let i = 0; i < 1000; i++) { c.stdout.emit('data', Buffer.from(line(i))); }
75+
c.stderr.emit('data', Buffer.from('FAIL test/foo.test.js: 3 assertions failed\n'));
76+
c.emit('close', 1);
77+
});
78+
assert.ok(r.length <= 8000, 'still bounded: ' + r.length);
79+
assert.ok(r.endsWith('\n[exit 1]'), 'the exit code is last: ' + JSON.stringify(r.slice(-30)));
80+
assert.ok(r.includes('FAIL test/foo.test.js: 3 assertions failed'), 'the failure is there');
81+
assert.ok(r.startsWith('line 0000 '), 'and so is the start');
82+
assert.match(r, /\n… \[\d+ characters of output omitted\] …\n/, 'the gap is said out loud');
83+
const omitted = Number(/\[(\d+) characters of output omitted\]/.exec(r)[1]);
84+
const total = 1000 * line(0).length + 'FAIL test/foo.test.js: 3 assertions failed\n'.length;
85+
assert.strictEqual(r.length - '\n[exit 1]'.length - (r.match(/\n… \[\d+ characters of output omitted\] …\n/)[0].length) + omitted, total, 'nothing is lost silently: kept + omitted = everything');
86+
});
87+
88+
await test('a timeout and a stop are still said, however much came before', async () => {
89+
const noisy = (c) => { for (let i = 0; i < 1000; i++) { c.stdout.emit('data', Buffer.from(line(i))); } };
90+
const t = await result((c) => { noisy(c); }, { commandTimeout: 30 });
91+
assert.ok(t.endsWith('\n[timed out]') && t.length <= 8000, JSON.stringify(t.slice(-40)));
92+
const s = await result((c, ctx) => { noisy(c); for (const [, stop] of ctx.commandStops) { stop(); } });
93+
assert.ok(s.endsWith('\n[stopped by user]') && s.length <= 8000, JSON.stringify(s.slice(-40)));
94+
});
95+
96+
await test('output of exactly the limit is still returned whole', async () => {
97+
const body = 'y'.repeat(8000 - '\n[exit 3]'.length);
98+
assert.strictEqual(await result((c) => { c.stdout.emit('data', Buffer.from(body)); c.emit('close', 3); }), body + '\n[exit 3]');
99+
});
100+
101+
console.log('\nagentCommandResult: ' + n + ' tests passed.');
102+
} finally {
103+
Module._load = origLoad;
104+
fs.rmSync(root, { recursive: true, force: true });
105+
}
106+
})().catch((e) => { console.error(e); process.exit(1); });

0 commit comments

Comments
 (0)