fix(web/shell): #107 review — a refused commit's "NONE … applied" line is always shown in a terminal

The bug: both tabs showed only the first 8 notes of a run. A refusal is eight notes on its own (six
reasons, "and N more refused", then "NONE of this command's file changes were applied"), from the
terminal's commit (web/shell/src/commit.js) and from levelcode-ai's review (shellCommit.js) alike, and
the runtime puts the notes the run already had in front of it: a moved folder (said on every run), an
emptied /tmp or /home/user, the engine's own notes, variables not kept. With one of those the NONE line
was the ninth note and was cut, so the visitor saw six refusals and never read that nothing was applied.

The fix: notesShown() in output-sink.js, used by pty.js and agent-tab.js. Still at most 8 notes in their
order; when there are more, a note that says the changes were NOT applied ("NONE of …", "… were/was NOT
applied") takes the place of the last of the others. The agent's tab drops non-string notes before the
cap instead of letting them use a place. README section 7 and terminal/NOTES.md say so.

Tests: shell-terminal-pty ("a refused commit's NONE … line is shown behind a note the run already had")
feeds the terminal commit's real refusal notes behind an emptied-/tmp note; shell-terminal-agent-tab
("a refused review's NONE … line …") feeds levelcode-ai's real refusal notes behind one and three notes;
shell-terminal-sink pins notesShown() itself (the runtime's own NOT applied endings, a plain cut without
one, a file name with "none" in it). All three fail on the unfixed code (the NONE line missing; notesShown
not a function) and pass now.

Author: ndemianc

web/shell/README.md

@@ -322,6 +322,7 @@ The agent's commands are mirrored into one read-only tab "LevelCode Agent" witho
 | completion | – | 200 names, 2 s |
 | what a terminal remembers for links | – | 400 lines of 4,000 characters |
 | an `explanation` or a note shown in the agent's tab | the first 4,800 characters scanned, 300 shown, at most 8 notes | – |
+| notes shown after a run (either tab) | at most 8; one that says the changes were NOT applied is always among them (`notesShown`) | same |
 
 The engine's own table (`engine/limits.js`) has 45 keys; `LIMITS.engine` above is the ones the shell sets, the engine's
 own values the rest.

web/shell/src/terminal/NOTES.md

@@ -182,8 +182,10 @@ One reusable read-only tab, appended to with a header (explanation dim, `$ comma
 a result line (`✓ exit 0 · 1.4s`, `✗ exit N`, `■ stopped`, `■ timed out`, `■ stopped: a limit of the shell was reached`).
 The command text, explanation, cwd and notes go through `forTerminal`, the output through the filter. Caps: 1 MiB for
 the output; the first line of a command 300 characters; the explanation and each note 300 characters (cut on what is
-shown, with `…`), at most 8 notes. The explanation is written first and **counts in the run's 1 MiB**; the notes come
-after the output, so counting them would change nothing, and they are bounded by 8 x 300 instead. Only the first 4,800
+shown, with `…`), at most 8 notes - a note that says the changes were NOT applied is always one of them, in the place
+of the last of the others (`notesShown` in `output-sink.js`; the visitor's terminal shows its notes the same way). The
+explanation is written first and **counts in the run's 1 MiB**; the notes come after the output, so counting them would
+change nothing, and they are bounded by 8 x 300 instead. Only the first 4,800
 characters of an explanation or a note are looked at (blank lines past that are not read through), and the join of its
 lines is linear.
 

web/shell/src/terminal/agent-tab.js

@@ -13,7 +13,7 @@
 'use strict';
 
 const { createInputParser } = require('./input-parser');
-const { createWriter, createRunOutput, OUTPUT_LIMIT } = require('./output-sink');
+const { createWriter, createRunOutput, notesShown, OUTPUT_LIMIT } = require('./output-sink');
 const { cwdLabel } = require('./prompt');
 const { createLineLog } = require('./line-log');
 
@@ -30,7 +30,7 @@ const MAX_TEXT_SHOWN = 300;
  * The cut is made before anything is scanned, so a megabyte of blanks costs a slice.
  */
 const MAX_TEXT_SCAN = 16 * MAX_TEXT_SHOWN;
-/** At most this many notes of a run are shown. */
+/** At most this many notes of a run are shown (one that says its changes were NOT applied always among them: notesShown). */
 const MAX_NOTES = 8;
 
 const ESC = '\x1b';
@@ -250,8 +250,8 @@ function createAgentTab(deps) {
 		const run = current;
 		if (!run) { return; }
 		let s = run.output.end();
-		const notes = Array.isArray(e.notes) ? e.notes.slice(0, MAX_NOTES) : [];
-		for (const n of notes) { if (typeof n === 'string') { s += DIM + clip(n) + RESET + '\r\n'; } }
+		const notes = Array.isArray(e.notes) ? notesShown(e.notes.filter((/** @type {unknown} */ n) => typeof n === 'string'), MAX_NOTES) : [];
+		for (const n of notes) { s += DIM + clip(n) + RESET + '\r\n'; }
 		const took = duration(now() - run.startedAt);
 		const code = Number.isFinite(e.exitCode) ? Math.trunc(e.exitCode) : 1;
 		switch (e.how) {

web/shell/src/terminal/output-sink.js

@@ -138,4 +138,27 @@ function createRunOutput(o) {
 	};
 }
 
-module.exports = { createWriter, createRunOutput, CHUNK, OUTPUT_LIMIT, RESET_TAIL };
+/**
+ * A note that says a command's file changes were NOT applied - "NONE of this command's file changes were applied",
+ * "Any file changes it had made were NOT applied", "Its 3 file changes were NOT applied". The shell and levelcode-ai's
+ * review write it in capitals, and a cap on the notes must never be what drops it.
+ */
+const NOT_APPLIED = /\bNONE of\b|\b(?:was|were) NOT applied\b/;
+
+/**
+ * The notes of a run that a terminal shows after it: at most `max` of them, in their order. When there are more, a
+ * note that says the changes were NOT applied is still among them, in the place of the last of the others: a refusal
+ * ends with it (six reasons, the count of the rest, then that line), and the notes the run already had - a moved
+ * folder, an emptied /tmp, the engine's own - come in front of it, so it was the one a plain cut dropped.
+ * @param {string[]} notes @param {number} max @returns {string[]}
+ */
+function notesShown(notes, max) {
+	if (notes.length <= max) { return notes.slice(); }
+	/** @type {number[]} */ const said = [];
+	for (let i = 0; i < notes.length; i++) { if (NOT_APPLIED.test(notes[i])) { said.push(i); } }
+	const keep = new Set(said.slice(-max));
+	for (let i = 0; i < notes.length && keep.size < max; i++) { keep.add(i); }
+	return notes.filter((_, i) => keep.has(i));
+}
+
+module.exports = { createWriter, createRunOutput, notesShown, CHUNK, OUTPUT_LIMIT, RESET_TAIL };

web/shell/src/terminal/pty.js

@@ -16,7 +16,7 @@ const { LineEditor } = require('./line-editor');
 const { createInputParser, DEFAULT_MAX_PASTE } = require('./input-parser');
 const { isComplete } = require('./completeness');
 const { createCompleter } = require('./completion');
-const { createWriter, createRunOutput, OUTPUT_LIMIT } = require('./output-sink');
+const { createWriter, createRunOutput, notesShown, OUTPUT_LIMIT } = require('./output-sink');
 const { strWidth, stripEscapes } = require('./width');
 const { createLineLog } = require('./line-log');
 const { promptFor } = require('./prompt');
@@ -392,7 +392,7 @@ class ShellPty {
 	 */
 	_epilogue(code, tail, notes, how) {
 		let s = tail;
-		const lines = notes.slice(0, 8).map((n) => this.sanitize.forTerminal(n, { crlf: false }).replace(/\n/g, ' '));
+		const lines = notesShown(notes, 8).map((n) => this.sanitize.forTerminal(n, { crlf: false }).replace(/\n/g, ' '));
 		const own = how && how !== 'exit' && how !== 'aborted' && lines.length === 0 ? outcomeLine(how, code) : '';
 		for (const l of lines) { s += DIM + l + RESET + '\r\n'; }
 		if (own) { s += YELLOW + '[' + own + ']' + RESET + '\r\n'; }

web/test/unit/shell-terminal-agent-tab.test.js

@@ -322,6 +322,33 @@ test('at most 8 notes of a run are shown', () => {
 	assert.ok(!s.includes('note 9'));
 });
 
+test('a refused review\'s "NONE of this command\'s file changes were applied" is shown behind the notes the run already had; still at most 8', async () => {
+	// What levelcode-ai's review says when it refuses an agent's set (six reasons, the count of the rest, then the NONE
+	// line), behind what the runtime says on every run of a folder it had to move. The NONE line was the ninth note.
+	const { commitShellChanges } = require(path.join(__dirname, '..', '..', '..', 'extensions', 'levelcode-ai', 'shellCommit.js'));
+	const outside = Array.from({ length: 9 }, (_, i) => ({ kind: 'write', path: '/elsewhere/f' + i, existed: false, before: null, after: new Uint8Array([120]) }));
+	const refused = await commitShellChanges(outside, {}, { folders: () => [{ name: 'scratch', root: '/scratch' }], readNow: async () => null });
+	assert.strictEqual(refused.refused, true);
+	assert.strictEqual(refused.notes.length, 8, 'six reasons, "…and 3 more refused", and the NONE line');
+	const none = refused.notes[7];
+	assert.match(none, /NONE of this command's file changes were applied/);
+	const lead = '[LevelCode Shell: The folder "home" is mounted at /mnt/home, not at /home: /home is the shell\'s own.]';
+	for (const leads of [[lead], [lead, '[LevelCode Shell: an engine note]', '[LevelCode Shell: another]']]) {
+		const t = setup({ mode: 'never' });
+		t.emit(run(1, 'mv src/* lib/'));
+		const a = t.attach();
+		const notes = leads.concat(refused.notes);
+		t.emit({ type: 'end', id: 1, exitCode: 1, how: 'exit', notes });
+		t.timers.drain();
+		const s = a.screen().join('\n');
+		assert.ok(s.includes('NONE of this command\'s file changes were applied'), leads.length + ' note(s) in front: the NONE line is shown:\n' + s);
+		const shown = notes.filter((n) => s.includes(n.slice(0, 60)));
+		assert.strictEqual(shown.length, 8, 'still at most 8: ' + JSON.stringify(shown));
+		assert.strictEqual(shown[shown.length - 1], none, 'the NONE line takes the last place, after the reasons');
+		assert.deepStrictEqual(shown.slice(0, 7), notes.slice(0, 7), 'the others in their order');
+	}
+});
+
 test('an explanation and every note are cut at 300 characters, whatever they are made of; the explanation counts in the run\'s budget', () => {
 	const t = setup({ mode: 'never', outputLimit: 2000 });
 	const huge = 'e'.repeat(5000000);

web/test/unit/shell-terminal-pty.test.js

@@ -670,6 +670,31 @@ test('at most 8 of the runtime\'s notes are shown after a command', async () =>
 	assert.ok(!s.includes('[note 9]'));
 });
 
+test('a refused commit\'s "NONE of this command\'s file changes were applied" is shown behind a note the run already had; still at most 8', async () => {
+	// What the terminal's own commit says when it refuses a set (six reasons, the count of the rest, then the line that
+	// says nothing was applied: eight notes), behind a note the runtime puts in front of every ending (a /tmp that was
+	// emptied, a moved folder, the engine's own). The ninth note used to be cut, and with it the only line that says
+	// the files were left as they were.
+	const { commitChanges } = require(path.join(TERMINAL, '..', 'commit.js'));
+	const outside = Array.from({ length: 9 }, (_, i) => ({ kind: /** @type {const} */ ('write'), path: '/elsewhere/f' + i, existed: false, before: null, after: new Uint8Array([120]) }));
+	const refused = await commitChanges(outside, /** @type {any} */ ({ resolve: () => null }), { writeFile: 1000, writeRun: 5000, entries: 100 });
+	assert.strictEqual(refused.notes.length, 8, 'six reasons, "and 3 more refused", and the NONE line');
+	const none = refused.notes[7];
+	assert.match(none, /NONE of this command's file changes were applied/);
+	const lead = '[LevelCode Shell: /tmp was emptied when the shell restarted (it held 2 files)]';
+	const h = makePty({ cols: 200 });
+	h.type('x', '\r');
+	h.session.finish({ exitCode: 1, notes: [lead].concat(refused.notes) });
+	await settle(); h.timers.drain();
+	const s = h.screen().join('\n');
+	assert.ok(s.includes(none), 'the NONE line is shown:\n' + s);
+	assert.ok(s.includes(lead), 'the note in front is shown');
+	const shown = [lead].concat(refused.notes).filter((n) => s.includes(n));
+	assert.strictEqual(shown.length, 8, 'still at most 8: ' + JSON.stringify(shown));
+	assert.deepStrictEqual(shown, [lead].concat(refused.notes.slice(0, 6), [none]), 'in their order; the NONE line takes the last place');
+	assert.ok(s.indexOf(none) > s.indexOf(refused.notes[5]), 'and stays where it was, after the reasons');
+});
+
 test('dimensions below 2 are never taken (a hidden panel reports 0): the previous ones stay and are what a command is told', async () => {
 	const h = makePty({ cols: 60, rows: 12 });
 	for (const bad of [{ columns: 1, rows: 1 }, { columns: 0, rows: 0 }, { columns: NaN, rows: undefined }, { columns: -5, rows: 9 }]) {

web/test/unit/shell-terminal-sink.test.js

@@ -15,7 +15,7 @@ const path = require('path');
 
 const dir = path.join(__dirname, '..', '..', 'shell', 'src', 'terminal');
 const si = require(path.join(dir, 'shell-integration.js'));
-const { createWriter, createRunOutput, CHUNK, OUTPUT_LIMIT, RESET_TAIL } = require(path.join(dir, 'output-sink.js'));
+const { createWriter, createRunOutput, notesShown, CHUNK, OUTPUT_LIMIT, RESET_TAIL } = require(path.join(dir, 'output-sink.js'));
 
 const queue = [];
 const test = (name, fn) => queue.push([name, fn]);
@@ -174,6 +174,29 @@ test('charge(): text shown outside the filters counts against the run\'s cap', (
 	assert.ok(/\[output limit reached: 20 more characters were not shown\]/.test(r.end()), 'the output had 40 characters left, not 100');
 });
 
+test('notesShown(): at most max notes in their order, and the one that says the changes were NOT applied is never the one cut', () => {
+	const lead = Array.from({ length: 9 }, (_, i) => '[LevelCode Shell: The folder "f' + i + '" is mounted at /mnt/f' + i + ', not at /home/f' + i + ']');
+	const few = lead.slice(0, 3);
+	const copy = notesShown(few, 8);
+	assert.deepStrictEqual(copy, few);
+	assert.notStrictEqual(copy, few, 'a copy');
+	// the runtime's own endings, behind more notes than the cap: a Stop, and a limit that discarded the changes
+	for (const end of [
+		'[LevelCode Shell: stopped. Any file changes the command had made were NOT applied.]',
+		'[LevelCode Shell: a limit of this shell stopped the command. Its 3 file changes were NOT applied.]',
+		'[LevelCode Shell: a limit of this shell stopped the command. Its 1 file change was NOT applied.]',
+		'[LevelCode Shell: NONE of the remaining 2 changes were applied (3 of 5 had already been).]',
+	]) {
+		assert.deepStrictEqual(notesShown(lead.concat(end), 8), lead.slice(0, 7).concat(end), end);
+	}
+	// without such a note it is the plain cut it was; "none" in a file name is not that note
+	const plain = lead.concat('[LevelCode Shell: refused: /w/none of these.txt is outside the workspace folders]');
+	assert.deepStrictEqual(notesShown(plain, 8), plain.slice(0, 8));
+	// more of them than the cap: the last ones (the run's own ending is last)
+	const many = Array.from({ length: 10 }, (_, i) => 'NONE of ' + i);
+	assert.deepStrictEqual(notesShown(many, 8), many.slice(2));
+});
+
 let finished = false;
 // a test that awaits something that never settles leaves nothing to keep the process alive: it would exit 0, silently
 process.on('exit', (code) => { if (code === 0 && !finished) { console.error('the tests did not finish: one of them waits for something that never happens'); process.exitCode = 1; } });