Skip to content

Commit 63249cb

Browse files
committed
fix(agent): a markdown "Done:" line ends the run — the finished answer was being nudged
The agent finished, and then finished again. Its answer opened with **Done:** created a styled, renderable workflow diagram of how a chat message becomes a reply. and the loop replied "Stop planning. You have read enough. Make your next edit_file (or write_file) call NOW …". The model answered with a second "Done:", and that is what the user saw. A turn with no tool call ends the run when one of its lines starts with "Done:" — the ending SYSTEM_BASE asks for. The test read the text literally, /(^|\n)\s*done\s*:/, and the `**` sits between the line start and the word, so the line did not count. The turn fell through to the stall check, where /\bnext[, ]/ takes any "next " for a promise of more work; this answer quoted a label from the diagram, "Your next message updates context". Run on develop's loop, that text makes it ask the model for a second turn. The test is now a function, saysDone(), and it looks for the line with the markdown marks stripped: quote, heading and list marks at line starts, then emphasis marks. So **Done:**, __Done:__, `Done:`, "## Done:", "> Done:", "- Done:" and "1. Done:" all count. "Not done: two tests fail" does not, and neither does a turn that announces an edit and makes none. The second half of the test — text whose last word is "done" — still reads the text as written. With the marks stripped, a turn that ends in a fenced shell loop ends in the word too: Let me run this to rename them: ```bash for f in *.txt; do mv "$f" "${f%.txt}.md" done ``` That is a stall, a command pasted where a tool call belonged, and it stays nudged. The price: "**All done.**" is not recognised, as it is not on develop. Nothing else moves — the auto-verify call, the stall check, the nudge's wording and its counter are as they were. One side effect of counting list items. A turn that holds a "- Done: parser" bullet in a progress list and also promises more work was nudged on develop, and now ends the run. Without the bullet mark the same text ended it already. Not fixed here: - A Done line behind an emoji ("✅ **Done:** …") is still not recognised, so with a "next " in the answer it is still nudged. - A finished answer with no Done line that says "Next, run it against staging" is still taken for a promise. That is the stall check's question, and it is left alone. test/agentDone.test.js, 7 cases. The verdicts, called directly; and the real loop on scripted turns, with vscode and the provider replaced the way agentRunCommand.test.js replaces them, because a verdict the loop does not consult fixes nothing. The reported answer ends the run in one turn; the announced edit is nudged; the pasted loop is nudged. The suite awaits runAgent and nothing else — no timers, no waiting on the event loop. It fails with develop's test inside the function (2 cases), with the function fixed and the loop not calling it (1), and with the last-word test moved onto the stripped text (2). 50 suites, 969 cases, 0 failing (scripts/test-extensions.sh) on this commit's tree, on macOS with Node 24 and in a Linux container with Node 20. With the checkout's tsc, agent.js reports the same 94 diagnostics as on develop. Not run in the editor against a live model: the loop was driven by scripted turns.
1 parent f8b6193 commit 63249cb

2 files changed

Lines changed: 208 additions & 2 deletions

File tree

‎extensions/levelcode-ai/agent.js‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,28 @@ async function setupMcp(ctx, wsFolders, dbg) {
715715
}
716716
}
717717

718+
/**
719+
* Did the model say it is finished — a line starting with "Done:" (what SYSTEM_BASE asks for), or
720+
* text that simply ends in "done"?
721+
*
722+
* The LINE is looked for with markdown marks stripped. A model that writes it as `**Done:** …`,
723+
* `## Done: …` or `- Done: …` has still said it; read literally, the marks sit between the line start
724+
* and the word, so a finished run was taken for an unfinished one and — when the answer also read as
725+
* a promise of more work, for which one "next " is enough — nudged until the model said Done a second
726+
* time.
727+
* @param {string} text the text of a turn that called no tool
728+
* @returns {boolean}
729+
*/
730+
function saysDone(text) {
731+
// Strip quote, heading and list marks at line starts, then emphasis marks, before the line test.
732+
const plain = text
733+
.replace(/^[ \t]*(?:[>#]+|[-*+]|\d+\.)[ \t]+/gm, '')
734+
.replace(/[*_`~]+/g, '');
735+
// The last-word test reads the text AS WRITTEN. With the backticks gone, a turn that ends in a fenced
736+
// shell loop ends in "done" as well — and a command pasted where a tool call belonged is a stall.
737+
return /(^|\n)\s*done\s*:/i.test(plain) || /\bdone\.?\s*$/i.test(text.trim());
738+
}
739+
718740
async function runAgent(ctx) {
719741
// No workspace is no longer a refusal. It used to fail the whole run here, which meant a question
720742
// that never needed a folder — "what does this error mean?", anything through an MCP server, a
@@ -1021,7 +1043,7 @@ async function runAgent(ctx) {
10211043
}
10221044
reason = 'limit'; break;
10231045
}
1024-
if (/(^|\n)\s*done\s*:/i.test(text) || /\bdone\.?\s*$/i.test(text.trim())) {
1046+
if (saysDone(text)) {
10251047
if (await attemptVerify()) { continue; } // verification failed → fix feedback pushed, keep going
10261048
reason = 'done'; break;
10271049
}
@@ -1070,4 +1092,4 @@ async function runAgent(ctx) {
10701092
}
10711093
}
10721094

1073-
module.exports = { resetContextAnnounce, runAgent, makeDiff, resolveWorkspacePath };
1095+
module.exports = { resetContextAnnounce, runAgent, makeDiff, resolveWorkspacePath, saysDone };
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* The agent recognises a "Done:" line written in markdown — run: node test/agentDone.test.js
3+
*
4+
* The bug this locks down: a FINISHED run was pushed to keep going. The loop takes a turn with no
5+
* tool call for the end of the run when a line starts with "Done:", and it read the text literally —
6+
* so the answer
7+
*
8+
* **Done:** created a styled, renderable workflow diagram of how a chat message becomes a reply.
9+
*
10+
* did not count: the `**` sits between the line start and the word. The turn then fell through to the
11+
* stall check, where a "next " anywhere in the answer reads as a promise of more work, and the model
12+
* was sent "Stop planning. … Make your next edit_file (or write_file) call NOW". It replied with a
13+
* second "Done:", and the user watched the agent finish twice.
14+
*
15+
* Two halves, both asserted. saysDone() is the verdict, and is called directly. That runAgent ASKS it
16+
* is a separate fact — a verdict the loop does not consult fixes nothing — so the real loop runs here
17+
* on scripted turns, with `vscode` and the provider replaced as agentRunCommand.test.js replaces them.
18+
* No scripted turn calls a tool, so no process is started; and runAgent resolves only once the run is
19+
* over, so there is nothing left to wait for when it returns.
20+
*--------------------------------------------------------------------------------------------*/
21+
// @ts-check
22+
'use strict';
23+
24+
const assert = require('assert');
25+
const fs = require('fs');
26+
const os = require('os');
27+
const path = require('path');
28+
const Module = require('module');
29+
30+
// ---- the outside world, replaced --------------------------------------------------------------
31+
32+
// vscode: one EMPTY workspace folder — no rules file and no .levelcode/mcp.json, so the run has nothing
33+
// to load and no MCP server to start.
34+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'lc-done-'));
35+
const vscodeMock = { workspace: { workspaceFolders: [{ uri: { fsPath: root }, name: 'app' }] } };
36+
37+
const origLoad = Module._load;
38+
// @ts-ignore — test-only loader shim
39+
Module._load = function (request, parent, isMain) {
40+
if (request === 'vscode') { return vscodeMock; }
41+
return origLoad.call(this, request, parent, isMain);
42+
};
43+
44+
// The provider: scripted turns instead of a network call. Patched on the module object agent.js shares,
45+
// and BEFORE agent.js loads, so it holds however agent.js chooses to import it. `asked` counts the
46+
// turns the loop requested — one more than a finished run needs is the whole bug.
47+
const providers = require('../providers/index');
48+
let script = [];
49+
let asked = 0;
50+
providers.streamAgentTurn = async () => {
51+
asked++;
52+
const turn = script.shift();
53+
if (!turn) { throw new Error('the agent asked for a turn the script does not have'); }
54+
return turn;
55+
};
56+
57+
const { runAgent, saysDone } = require('../agent');
58+
59+
// ---- harness ----------------------------------------------------------------------------------
60+
61+
let n = 0;
62+
function test(name, fn) { fn(); n++; console.log(' ok - ' + name); }
63+
async function testAsync(name, fn) { await fn(); n++; console.log(' ok - ' + name); }
64+
65+
/**
66+
* Run one goal to the end of the RUN. Every scripted turn is text with no tool call, which is the
67+
* only kind of turn the Done test and the stall check ever see.
68+
* @param {string[]} texts what the model says, one entry per turn it is asked for
69+
*/
70+
async function run(texts) {
71+
const posted = [], logged = [];
72+
script = texts.map((text) => ({ stop_reason: 'end_turn', content: [{ type: 'text', text }] }));
73+
asked = 0;
74+
const messages = [{ role: 'user', content: 'draw how a chat message becomes a reply' }];
75+
await runAgent({
76+
messages,
77+
maxSteps: 5,
78+
post: (m) => posted.push(m),
79+
dbg: (label, data) => logged.push({ label, data }),
80+
signal: new AbortController().signal
81+
});
82+
return {
83+
asked,
84+
roles: messages.map((m) => m.role),
85+
nudges: logged.filter((l) => l.label === 'nudge').length,
86+
end: posted.filter((m) => m.type === 'agentError' || m.type === 'agentDone').map((m) => m.reason || m.message)
87+
};
88+
}
89+
90+
// The answer from the bug report. It is a Done line AND it holds a "next " — the second is what turned
91+
// an unrecognised ending into a nudge.
92+
const REPORTED = '**Done:** created a styled, renderable workflow diagram of how a chat message becomes a reply.\n'
93+
+ '\n'
94+
+ 'The last box reads:\n'
95+
+ '“Your next message updates context—not model weights”';
96+
97+
// A real stall: an edit announced, and no tool call to make it.
98+
const PROMISE = 'I found the validator. Now I’ll update it to cover subdomains:';
99+
100+
// Another stall: a command pasted where a run_command call belonged. Its last word is the shell's own
101+
// "done", behind a closing fence.
102+
const PASTED_LOOP = 'Let me run this to rename them:\n'
103+
+ '```bash\n'
104+
+ 'for f in *.txt; do\n'
105+
+ ' mv "$f" "${f%.txt}.md"\n'
106+
+ 'done\n'
107+
+ '```';
108+
109+
(async () => {
110+
try {
111+
// ---- 1. the verdict --------------------------------------------------------------------------
112+
113+
test('VERDICT: a Done line counts, however markdown dresses it', () => {
114+
const done = [
115+
REPORTED,
116+
'Done: created the workflow diagram in docs/flow.html.',
117+
'## Done: shipped',
118+
'> Done: quoted',
119+
'- Done: in a list',
120+
'1. Done: numbered',
121+
'__Done:__ underscore',
122+
'`Done:` code'
123+
];
124+
for (const text of done) {
125+
assert.strictEqual(saysDone(text), true, 'not read as finished: ' + JSON.stringify(text));
126+
}
127+
});
128+
129+
test('VERDICT: the word alone, or no word at all, is not a Done line', () => {
130+
// Stripping marks must not turn "done" somewhere in a sentence into the line the prompt asks for.
131+
const notDone = [
132+
'Not done: two tests fail',
133+
'The validator runs before save. It lives in app/models/link.rb.',
134+
PROMISE
135+
];
136+
for (const text of notDone) {
137+
assert.strictEqual(saysDone(text), false, 'read as finished: ' + JSON.stringify(text));
138+
}
139+
});
140+
141+
test('VERDICT: an answer that simply ends in "done" still counts, as it did before', () => {
142+
// The second clause of the test, older than this fix: plain text whose last word is "done".
143+
assert.strictEqual(saysDone('The diagram is in docs/flow.html and it renders. All done.'), true);
144+
});
145+
146+
test('VERDICT: the "done" that closes a shell loop is not the model saying it', () => {
147+
// Why that second clause reads the text as written, and only the Done LINE is looked for with the
148+
// marks stripped: take the fence's backticks away and this turn ends in the word.
149+
assert.strictEqual(saysDone(PASTED_LOOP), false);
150+
});
151+
152+
// ---- 2. the loop asks it ---------------------------------------------------------------------
153+
154+
await testAsync('LOOP: the reported answer ends the run — the model is not asked again', async () => {
155+
// Scripted the way the bug played out: had the loop asked again, the model's second "Done:" is
156+
// waiting for it. It must never be requested.
157+
const r = await run([REPORTED, 'Done: created the workflow diagram.']);
158+
assert.strictEqual(r.asked, 1, 'a finished run was asked to keep going');
159+
assert.strictEqual(r.nudges, 0);
160+
assert.deepStrictEqual(r.roles, ['user', 'assistant'], 'something was sent to the model after it had finished');
161+
assert.deepStrictEqual(r.end, ['done']);
162+
});
163+
164+
await testAsync('LOOP: a promise with no tool call is still nudged', async () => {
165+
// The stall check is deliberately untouched, and this is the case it exists for. If the fix ever
166+
// reads this text as finished, the edit it announces is never made.
167+
const r = await run([PROMISE, 'Done: the validator covers subdomains.']);
168+
assert.strictEqual(r.nudges, 1, 'the stalled turn was not nudged');
169+
assert.deepStrictEqual(r.roles, ['user', 'assistant', 'user', 'assistant'], 'the nudge never reached the transcript');
170+
assert.strictEqual(r.asked, 2);
171+
assert.deepStrictEqual(r.end, ['done']);
172+
});
173+
174+
await testAsync('LOOP: a command pasted in a fence is still nudged, though its last word is "done"', async () => {
175+
const r = await run([PASTED_LOOP, 'Done: renamed the files.']);
176+
assert.strictEqual(r.nudges, 1, 'a pasted command passed for a finished run');
177+
assert.strictEqual(r.asked, 2);
178+
assert.deepStrictEqual(r.end, ['done']);
179+
});
180+
} finally {
181+
fs.rmSync(root, { recursive: true, force: true });
182+
}
183+
console.log('\nagentDone: ' + n + ' tests passed.');
184+
})().catch((e) => { console.error(e); process.exit(1); });

0 commit comments

Comments
 (0)