From 6f2255946a834f2458613450533cd2906eb31e7d Mon Sep 17 00:00:00 2001 From: Agando Date: Tue, 27 Oct 2026 14:59:06 +0100 Subject: fix(newos4): bound one turn's tool context, honest skill-create, quarantine a corrupt memory graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three items the last review and probes left open, each with a test that is red on HEAD. - MAINT-023 (module-aiassist.js): one agentic turn grew without limit — every role:'tool' body was pushed verbatim and only this.context was trimmed BETWEEN turns (measured 6 -> 112 705 tokens across 8 model calls against maxContextTokens 12 000). Observations are now clipped to maxToolResultChars (6 000) with a note saying how much was withheld and to re-read with offset/limit, and _boundToolContext() elides the oldest tool bodies once the running total passes toolContextBudgetChars (24 000), always keeping the newest 4 verbatim. The ceiling is turn-independent: the same ~26 k chars at 8 turns and at 20. - MAINT-024 (modules/ext/skill-pack-core.js): the command printed "✅ Skill created" with fs.writeFile()'s boolean discarded (probe: fresh VFS, success printed, no file), stripped invalid characters instead of rejecting the name (My-Skill -> y-kill, DevOps_Thing -> evpshing), and treated an unreadable /path argument as the skill body. Now: kebab-case is validated (uppercase normalises and the final name is reported; anything else is refused), /skills is created on first use, the write and index results are checked, and an unreadable body path is an error. - MAINT-003 (module-memory-graph.js): a corrupt store looked exactly like a fresh install — fsReadJSON returns null on a parse failure, so 85 nodes could vanish with no warning and no copy. The probe confirmed the loss: seed 6 nodes, truncate nodes.json and export.json, reload -> 1 node, no message, nothing in .trash. _quarantineCorrupt() now keeps the unreadable file as .corrupt, warns, and then loads from whatever remains. Tests: test-tool-context-bound.js (11), test-skill-create.js (16), test-memory-graph-recovery.js (8). RED on HEAD: tool-context crashes on _clipToolResult (exit 1), skill-create 16 assertions fail on the missing file/index, memory-graph B1/B2/C1 fail (no warning, no .corrupt copy). Suite now 13/13 files green; maintain --fix reports "node tests 13/13 pass". Live on v4.6.107: skill-create My-Skill -> ✅ "my-skill"; DevOps_Thing -> Invalid skill name; /does/not/exist.md -> Cannot read skill body; only my-skill in the index. --- modules/test/test-tool-context-bound.js | 93 +++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 modules/test/test-tool-context-bound.js (limited to 'modules/test/test-tool-context-bound.js') diff --git a/modules/test/test-tool-context-bound.js b/modules/test/test-tool-context-bound.js new file mode 100644 index 0000000..8cf2a5e --- /dev/null +++ b/modules/test/test-tool-context-bound.js @@ -0,0 +1,93 @@ +// modules/test/test-tool-context-bound.js — run: node +// MAINT-023: one agentic turn must not grow without limit. Measures the same growth the +// reviewer measured (6 -> 112 705 tokens across 8 model calls) against the new bounds. +const fsMod = require('fs'), path = require('path'); +const ROOT = path.join(__dirname, '..', 'core'); +const store = {}; +global.localStorage = { getItem: k => (k in store ? store[k] : null), setItem: (k, v) => { store[k] = String(v); }, removeItem: k => { delete store[k]; } }; +global.indexedDB = { open() { throw new Error('no idb'); }, deleteDatabase() { throw new Error('no idb'); } }; +global.window = global; + +const AI = new Function(fsMod.readFileSync(path.join(ROOT, 'module-aiassist.js'), 'utf8') + '\nreturn WebX11AIAssistantModule;')(); + +function mk() { + const ai = Object.create(AI.prototype); + ai.terminal = { print: () => {}, log: () => {}, fs: null }; + return ai; +} +const approxTokens = s => Math.ceil(s / 4); + +let fails = 0, passes = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} + +// A. a single oversized observation is clipped with a note +{ + const ai = mk(); + const big = 'x'.repeat(20000); + const clipped = ai._clipToolResult(big); + ok('A1 oversized result is clipped', clipped.length < 7000, 'len=' + clipped.length); + ok('A2 the model is told it was clipped', /clipped:/.test(clipped) && /offset\/limit/.test(clipped), clipped.slice(-90)); + ok('A3 short results pass through untouched', ai._clipToolResult('ok') === 'ok'); + ok('A4 non-string results are stringified', ai._clipToolResult({ a: 1 }) === '{"a":1}'); +} + +// B. the running total of tool bodies stays inside the budget +{ + const ai = mk(); + const messages = [{ role: 'user', content: 'go' }]; + let worst = 0; + for (let turn = 0; turn < 8; turn++) { + messages.push({ role: 'assistant', content: '', tool_calls: [{ id: 'c' + turn }] }); + // the reviewer's shape: each read_file page arrives at 16 000 chars + messages.push({ role: 'tool', tool_call_id: 'c' + turn, content: ai._clipToolResult('y'.repeat(16000)) }); + ai._boundToolContext(messages); + const toolChars = messages.filter(m => m.role === 'tool').reduce((s, m) => s + String(m.content).length, 0); + worst = Math.max(worst, toolChars); + } + // The ceiling is keepVerbatim clipped results plus tiny stubs — it must not grow with turns. + ok('B1 total tool context stays within the ceiling', worst <= 27000, 'worst=' + worst); + ok('B2 8 turns stay far below the old 112k-token blowup', approxTokens(worst) < 8000, 'worst≈' + approxTokens(worst) + ' tokens'); + ok('B3 the newest 4 results are verbatim', messages.filter(m => m.role === 'tool').slice(-4).every(m => !/elided/.test(m.content))); + ok('B4 older results carry an elision stub', messages.filter(m => m.role === 'tool').slice(0, 4).every(m => /elided/.test(m.content))); +} + +// B2. the ceiling is turn-independent: 20 turns cost the same as 8 +{ + const ai = mk(); + const messages = [{ role: 'user', content: 'go' }]; + for (let turn = 0; turn < 20; turn++) { + messages.push({ role: 'assistant', content: '', tool_calls: [{ id: 'c' + turn }] }); + messages.push({ role: 'tool', tool_call_id: 'c' + turn, content: ai._clipToolResult('y'.repeat(16000)) }); + ai._boundToolContext(messages); + } + const toolChars = messages.filter(m => m.role === 'tool').reduce((s, m) => s + String(m.content).length, 0); + ok('B5 20 turns cost the same ceiling as 8', toolChars <= 27000, 'toolChars=' + toolChars); +} + +// C. no tool messages -> nothing is touched +{ + const ai = mk(); + const messages = [{ role: 'user', content: 'hi' }, { role: 'assistant', content: 'hello' }]; + const before = JSON.stringify(messages); + ai._boundToolContext(messages); + ok('C1 non-tool messages are untouched', JSON.stringify(messages) === before); +} + +// D. a small turn is never rewritten +{ + const ai = mk(); + const messages = [{ role: 'user', content: 'go' }, { role: 'tool', tool_call_id: 'c1', content: 'small result' }]; + ai._boundToolContext(messages); + ok('D1 small tool results stay verbatim', messages[1].content === 'small result', messages[1].content); +} + +if (fails === 0) { + console.log('test-tool-context-bound: ' + passes + ' passed, 0 failed'); +} else { + console.log('test-tool-context-bound: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); +} -- cgit v1.2.3