diff options
| author | Agando <roduccomm@gmail.com> | 2026-10-27 14:59:06 +0100 |
|---|---|---|
| committer | Agando <roduccomm@gmail.com> | 2026-10-27 14:59:06 +0100 |
| commit | 6f2255946a834f2458613450533cd2906eb31e7d (patch) | |
| tree | 85677c588783e030606c4f82cc99b6d2ce138d75 | |
| parent | d2ec6fe99cc6f79013e689732fdc7f77089a8ed7 (diff) | |
fix(newos4): bound one turn's tool context, honest skill-create, quarantine a corrupt memory graph
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 <name>.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.
| -rw-r--r-- | main.html | 6 | ||||
| -rw-r--r-- | modules/core/core-bundle.js | 69 | ||||
| -rw-r--r-- | modules/core/module-aiassist.js | 44 | ||||
| -rw-r--r-- | modules/core/module-loader.js | 2 | ||||
| -rw-r--r-- | modules/core/module-memory-graph.js | 25 | ||||
| -rw-r--r-- | modules/ext/skill-pack-core.js | 37 | ||||
| -rw-r--r-- | modules/test/test-memory-graph-recovery.js | 90 | ||||
| -rw-r--r-- | modules/test/test-skill-create.js | 100 | ||||
| -rw-r--r-- | modules/test/test-tool-context-bound.js | 93 |
9 files changed, 450 insertions, 16 deletions
@@ -1349,13 +1349,13 @@ </script> <!-- v4.5.63: A2A scratchpad fixes + agent-register arg fix --> -<script src="modules/core/core-bundle.js?v=v4.6.106"></script> +<script src="modules/core/core-bundle.js?v=v4.6.107"></script> <!-- Async dependency-ordered module loader now only initializes terminal and loads optional ext modules --> -<script src="modules/core/module-loader.js?v=v4.6.106"></script> +<script src="modules/core/module-loader.js?v=v4.6.107"></script> <!-- Critical ext bundle loads AFTER webx11 exists so registerModule calls work --> -<script src="modules/ext/critical-ext-bundle.js?v=v4.6.106"></script> +<script src="modules/ext/critical-ext-bundle.js?v=v4.6.107"></script> <script> // ═══ ONE TRUE VERSION SOURCE ═══ diff --git a/modules/core/core-bundle.js b/modules/core/core-bundle.js index a057332..c06af5f 100644 --- a/modules/core/core-bundle.js +++ b/modules/core/core-bundle.js @@ -9837,6 +9837,41 @@ buildSystemPrompt() { } return result.substring(0, maxLen) + '...(truncated from ' + result.length + ' chars)'; } + // MAINT-023: one agentic turn used to grow without limit — every role:'tool' body was + // pushed verbatim and only this.context was trimmed BETWEEN turns, so a measured turn + // went 6 → 112 705 tokens across 8 model calls (maxContextTokens is 12 000). Two + // limits keep a single turn inside the window: clip each observation as it arrives, + // and elide the oldest ones once the running total passes the budget. + _clipToolResult(result) { + const text = typeof result === 'string' ? result : JSON.stringify(result); + const max = this.maxToolResultChars || 6000; + if (!text || text.length <= max) return text; + return text.slice(0, max) + + '\n…[clipped: ' + (text.length - max) + ' of ' + text.length + + ' chars withheld — re-read with offset/limit if you need the rest]'; + } + _boundToolContext(messages) { + const keepVerbatim = this.toolResultsVerbatim || 4; + const budget = this.toolContextBudgetChars || 24000; + const idx = []; + for (let i = 0; i < messages.length; i++) { + if (messages[i] && messages[i].role === 'tool') idx.push(i); + } + if (!idx.length) return messages; + const recent = new Set(idx.slice(-keepVerbatim)); + let total = 0; + for (const i of idx) total += String(messages[i].content || '').length; + // Oldest first, never the recent few: those are what the model is working on. + for (const i of idx) { + if (total <= budget) break; + if (recent.has(i)) continue; + const len = String(messages[i].content || '').length; + const stub = '[earlier tool result elided — ' + len + ' chars; call the tool again if you still need it]'; + messages[i].content = stub; + total -= len - stub.length; + } + return messages; + } // 2026-10-21: tool results lived only inside the current turn, so the next turn // had no idea what the model had already read or run. Keep a compact per-turn // trace (per agent tag) and persist it into the session context. @@ -10067,6 +10102,8 @@ buildSystemPrompt() { messages.push({ role: 'system', content: finalPrompt }); } const thinkStart = Date.now(); + // MAINT-023: keep the in-flight tool context bounded before every model call. + this._boundToolContext(messages); const res = await this.terminal.ollamaManager.chatWithTools(messages, this.getToolDefinitions()); const thinkTime = Math.round((Date.now() - thinkStart) / 1000); // Store raw response for debug-ai-last @@ -10236,7 +10273,9 @@ buildSystemPrompt() { _eR.lastResult = typeof toolResult === 'string' ? toolResult : JSON.stringify(toolResult); } } catch (_e) {} - messages.push({ role: 'tool', tool_call_id: tc.id, content: toolResult }); + // MAINT-023: clip on the way in — a single read_file page is 16 000 + // chars and several of them used to enter the context verbatim. + messages.push({ role: 'tool', tool_call_id: tc.id, content: this._clipToolResult(toolResult) }); this._noteObservation(fn.name, fn.arguments, toolResult, _agentTag); } // Only block on destructive tools @@ -10311,7 +10350,8 @@ buildSystemPrompt() { _eD.lastResult = typeof toolResult === 'string' ? toolResult : JSON.stringify(toolResult); } } catch (_e) {} - messages.push({ role: 'tool', tool_call_id: tc.id, content: toolResult }); + // MAINT-023: clip on the way in (same rule as the main loop). + messages.push({ role: 'tool', tool_call_id: tc.id, content: this._clipToolResult(toolResult) }); this._noteObservation(fn.name, fn.arguments, toolResult, _agentTag); } } else { @@ -18163,6 +18203,12 @@ class MemoryGraphModule { try { const fs = this.terminal.fs; if (!fs) return; + // MAINT-003: a corrupt store used to look exactly like a fresh one — fsReadJSON + // returns null on a parse failure, so the graph started empty with no warning and no + // copy of what was there. Quarantine anything unreadable first, then load what's left. + this._quarantineCorrupt('export.json'); + this._quarantineCorrupt('nodes.json'); + this._quarantineCorrupt('edges.json'); // Try loading from export first const exportData = fs.fsReadJSON(`${this._indexPath}export.json`); @@ -18215,6 +18261,25 @@ class MemoryGraphModule { } } + // MAINT-003: keep an unreadable store aside instead of starting fresh over it. + _quarantineCorrupt(name) { + try { + const fs = this.terminal.fs; + const p = `${this._indexPath}${name}`; + const raw = fs.readFile(p); + if (typeof raw !== 'string' || !raw.length) return false; + try { JSON.parse(raw); return false; } catch (e) { + const kept = p + '.corrupt'; + fs.writeFile(kept, raw); + this.terminal.print(`⚠️ Memory graph store ${name} is unreadable — kept a copy at ${kept} and starting from what remains`, 'warning'); + if (typeof fs.deleteFile === 'function') fs.deleteFile(p); + return true; + } + } catch (e) { + return false; + } + } + _saveNow() { try { const fs = this.terminal.fs; diff --git a/modules/core/module-aiassist.js b/modules/core/module-aiassist.js index ac8d297..9965e6a 100644 --- a/modules/core/module-aiassist.js +++ b/modules/core/module-aiassist.js @@ -1322,6 +1322,41 @@ buildSystemPrompt() { } return result.substring(0, maxLen) + '...(truncated from ' + result.length + ' chars)'; } + // MAINT-023: one agentic turn used to grow without limit — every role:'tool' body was + // pushed verbatim and only this.context was trimmed BETWEEN turns, so a measured turn + // went 6 → 112 705 tokens across 8 model calls (maxContextTokens is 12 000). Two + // limits keep a single turn inside the window: clip each observation as it arrives, + // and elide the oldest ones once the running total passes the budget. + _clipToolResult(result) { + const text = typeof result === 'string' ? result : JSON.stringify(result); + const max = this.maxToolResultChars || 6000; + if (!text || text.length <= max) return text; + return text.slice(0, max) + + '\n…[clipped: ' + (text.length - max) + ' of ' + text.length + + ' chars withheld — re-read with offset/limit if you need the rest]'; + } + _boundToolContext(messages) { + const keepVerbatim = this.toolResultsVerbatim || 4; + const budget = this.toolContextBudgetChars || 24000; + const idx = []; + for (let i = 0; i < messages.length; i++) { + if (messages[i] && messages[i].role === 'tool') idx.push(i); + } + if (!idx.length) return messages; + const recent = new Set(idx.slice(-keepVerbatim)); + let total = 0; + for (const i of idx) total += String(messages[i].content || '').length; + // Oldest first, never the recent few: those are what the model is working on. + for (const i of idx) { + if (total <= budget) break; + if (recent.has(i)) continue; + const len = String(messages[i].content || '').length; + const stub = '[earlier tool result elided — ' + len + ' chars; call the tool again if you still need it]'; + messages[i].content = stub; + total -= len - stub.length; + } + return messages; + } // 2026-10-21: tool results lived only inside the current turn, so the next turn // had no idea what the model had already read or run. Keep a compact per-turn // trace (per agent tag) and persist it into the session context. @@ -1552,6 +1587,8 @@ buildSystemPrompt() { messages.push({ role: 'system', content: finalPrompt }); } const thinkStart = Date.now(); + // MAINT-023: keep the in-flight tool context bounded before every model call. + this._boundToolContext(messages); const res = await this.terminal.ollamaManager.chatWithTools(messages, this.getToolDefinitions()); const thinkTime = Math.round((Date.now() - thinkStart) / 1000); // Store raw response for debug-ai-last @@ -1721,7 +1758,9 @@ buildSystemPrompt() { _eR.lastResult = typeof toolResult === 'string' ? toolResult : JSON.stringify(toolResult); } } catch (_e) {} - messages.push({ role: 'tool', tool_call_id: tc.id, content: toolResult }); + // MAINT-023: clip on the way in — a single read_file page is 16 000 + // chars and several of them used to enter the context verbatim. + messages.push({ role: 'tool', tool_call_id: tc.id, content: this._clipToolResult(toolResult) }); this._noteObservation(fn.name, fn.arguments, toolResult, _agentTag); } // Only block on destructive tools @@ -1796,7 +1835,8 @@ buildSystemPrompt() { _eD.lastResult = typeof toolResult === 'string' ? toolResult : JSON.stringify(toolResult); } } catch (_e) {} - messages.push({ role: 'tool', tool_call_id: tc.id, content: toolResult }); + // MAINT-023: clip on the way in (same rule as the main loop). + messages.push({ role: 'tool', tool_call_id: tc.id, content: this._clipToolResult(toolResult) }); this._noteObservation(fn.name, fn.arguments, toolResult, _agentTag); } } else { diff --git a/modules/core/module-loader.js b/modules/core/module-loader.js index 365fa55..3204af0 100644 --- a/modules/core/module-loader.js +++ b/modules/core/module-loader.js @@ -4,7 +4,7 @@ // Core module bundle loaded synchronously in main.html. // This loader only handles version cache-busting, splash, terminal init, and optional ext modules. - const LATEST_VER = 'v4.6.106'; + const LATEST_VER = 'v4.6.107'; const CACHE = '?v=' + LATEST_VER; const stored = localStorage.getItem('webx11_cache_ver'); if (stored !== LATEST_VER) { diff --git a/modules/core/module-memory-graph.js b/modules/core/module-memory-graph.js index 1cb0919..e802387 100644 --- a/modules/core/module-memory-graph.js +++ b/modules/core/module-memory-graph.js @@ -594,6 +594,12 @@ class MemoryGraphModule { try { const fs = this.terminal.fs; if (!fs) return; + // MAINT-003: a corrupt store used to look exactly like a fresh one — fsReadJSON + // returns null on a parse failure, so the graph started empty with no warning and no + // copy of what was there. Quarantine anything unreadable first, then load what's left. + this._quarantineCorrupt('export.json'); + this._quarantineCorrupt('nodes.json'); + this._quarantineCorrupt('edges.json'); // Try loading from export first const exportData = fs.fsReadJSON(`${this._indexPath}export.json`); @@ -646,6 +652,25 @@ class MemoryGraphModule { } } + // MAINT-003: keep an unreadable store aside instead of starting fresh over it. + _quarantineCorrupt(name) { + try { + const fs = this.terminal.fs; + const p = `${this._indexPath}${name}`; + const raw = fs.readFile(p); + if (typeof raw !== 'string' || !raw.length) return false; + try { JSON.parse(raw); return false; } catch (e) { + const kept = p + '.corrupt'; + fs.writeFile(kept, raw); + this.terminal.print(`⚠️ Memory graph store ${name} is unreadable — kept a copy at ${kept} and starting from what remains`, 'warning'); + if (typeof fs.deleteFile === 'function') fs.deleteFile(p); + return true; + } + } catch (e) { + return false; + } + } + _saveNow() { try { const fs = this.terminal.fs; diff --git a/modules/ext/skill-pack-core.js b/modules/ext/skill-pack-core.js index c2c95ba..3d3acf7 100644 --- a/modules/ext/skill-pack-core.js +++ b/modules/ext/skill-pack-core.js @@ -729,17 +729,28 @@ after the behaviour itself was fixed in v4.6.58.`), this.terminal.print('Usage: skill-create <name> <description> <path-or-text> [--tags=a,b] [--category=x]', 'warning'); return; } - const name = positional[0].replace(/[^a-z0-9-]/g, '').toLowerCase(); - if (!name) { this.terminal.print('Name must be kebab-case (letters, numbers, hyphens only)', 'error'); return; } + const rawName = positional[0]; + const name = rawName.toLowerCase(); + // MAINT-024: the old code stripped invalid characters instead of rejecting them, so + // "My-Skill" became "y-kill" and "DevOps_Thing" became "evpshing" — a skill created + // under a name nobody asked for, reported as success. + if (!/^[a-z0-9]+(-[a-z0-9]+)*$/.test(name)) { + this.terminal.print(`Invalid skill name "${rawName}": use kebab-case (letters, numbers, single hyphens, e.g. my-skill)`, 'error'); + return; + } const desc = positional[1]; const bodyArg = positional.slice(2).join(' '); const fs0 = this.terminal.fs; let content = bodyArg; if (bodyArg.startsWith('/')) { - try { - const fromFile = fs0.readFile(bodyArg); - if (typeof fromFile === 'string' && fromFile.length) content = fromFile; - } catch (e) { /* not a file: treat the argument as literal text */ } + // A path that cannot be read is an error, not a body: silently using the path text + // as the skill body produced a skill whose content was "/skills/nope.md". + const fromFile = fs0.readFile(bodyArg); + if (typeof fromFile !== 'string' || !fromFile.length) { + this.terminal.print(`Cannot read skill body: ${bodyArg} (missing or empty)`, 'error'); + return; + } + content = fromFile; } const tags = flags.tags; const fs = this.terminal.fs; @@ -751,9 +762,19 @@ after the behaviour itself was fixed in v4.6.58.`), } const now = new Date().toISOString(); const skillContent = `---\nname: ${name}\ndescription: ${desc}\ntags: [${tags.join(', ')}]\ncategory: ${flags.category}\n---\n${content}`; - fs.writeFile(`/skills/${name}.md`, skillContent); + // MAINT-024: create /skills when this is the first skill, and do not print ✅ unless the + // file actually landed — the old code reported success on a failed write. + try { if (!fs.readDir('/skills')) fs.mkdirRecursive('/skills'); } catch (e) { /* write below reports it */ } + const wroteFile = fs.writeFile(`/skills/${name}.md`, skillContent); + if (!wroteFile) { + this.terminal.print(`Failed to write /skills/${name}.md — the skills directory is not writable`, 'error'); + return; + } index[name] = { name, description: desc, tags, category: flags.category, created: now, updated: now }; - fs.fsWriteJSON('/skills/index.json', index); + const wroteIndex = fs.fsWriteJSON('/skills/index.json', index); + if (wroteIndex === false) { + this.terminal.print(`⚠️ Skill "${name}" written, but /skills/index.json could not be updated — it may not appear in skill lists`, 'warning'); + } this.terminal.print(`✅ Skill "${name}" created (${(skillContent.length / 1024).toFixed(1)}KB)`, 'success'); } diff --git a/modules/test/test-memory-graph-recovery.js b/modules/test/test-memory-graph-recovery.js new file mode 100644 index 0000000..533a9e2 --- /dev/null +++ b/modules/test/test-memory-graph-recovery.js @@ -0,0 +1,90 @@ +// modules/test/test-memory-graph-recovery.js — run: node +// MAINT-003: a corrupt memory-graph store must not look like a fresh install. The graph still +// starts from what remains, but the unreadable file is kept aside and the user is told. +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 load = (f, c) => new Function(fsMod.readFileSync(path.join(ROOT, f), 'utf8') + '\nreturn ' + c + ';')(); +const VFS = load('virtual-fs.js', 'VirtualFileSystem'); +const MG = load('module-memory-graph.js', 'MemoryGraphModule'); + +let fails = 0, passes = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} + +function mk() { + const out = []; + const vfs = new VFS(); + vfs.saveToStorage = () => true; + const term = { print: (m, t) => out.push({ text: String(m), type: t || '' }), log: () => {}, fs: vfs }; + const mg = new MG(); + mg.init(term); + return { mg, vfs, out }; +} +const text = out => out.map(o => o.text).join(' | '); + +// A. a healthy store survives a reload +{ + const a = mk(); + for (let i = 0; i < 4; i++) a.mg.addNode('concept', 'keep-' + i, 'keep ' + i); + a.mg._saveNow(); + const seeded = Object.keys(a.mg._nodes).length; + + const out = []; + const mg2 = new MG(); + mg2.init({ print: (m, t) => out.push({ text: String(m), type: t || '' }), log: () => {}, fs: a.vfs }); + ok('A1 nodes survive a reload', Object.keys(mg2._nodes).length === seeded, 'seeded ' + seeded + ' reloaded ' + Object.keys(mg2._nodes).length); + ok('A2 no quarantine warning on a healthy store', !/unreadable/.test(text(out)), text(out)); +} + +// B. a corrupt store is quarantined and reported, not silently discarded +{ + const a = mk(); + for (let i = 0; i < 4; i++) a.mg.addNode('concept', 'lose-' + i, 'lose ' + i); + a.mg._saveNow(); + const original = a.vfs.readFile('/.memory_graph/nodes.json'); + a.vfs.writeFile('/.memory_graph/nodes.json', '{"nodes":['); // truncated write + a.vfs.writeFile('/.memory_graph/export.json', 'not json at all'); + + const out = []; + const mg2 = new MG(); + mg2.init({ print: (m, t) => out.push({ text: String(m), type: t || '' }), log: () => {}, fs: a.vfs }); + + ok('B1 the user is warned', /unreadable/.test(text(out)), text(out).slice(0, 200)); + ok('B2 the corrupt file is kept aside', a.vfs.readFile('/.memory_graph/nodes.json.corrupt') === '{"nodes":[', + String(a.vfs.readFile('/.memory_graph/nodes.json.corrupt'))); + ok('B3 the quarantine copy held the broken text, not the old good one', original !== a.vfs.readFile('/.memory_graph/nodes.json.corrupt')); + ok('B4 the graph still starts', Object.keys(mg2._nodes).length >= 1, String(Object.keys(mg2._nodes).length)); +} + +// C. the export path is quarantined too, and the nodes.json fallback still loads +{ + const a = mk(); + a.mg.addNode('concept', 'fallback', 'fallback node'); + a.mg._saveNow(); + // the loader expects {nodes: [...], edges: [...]} + a.vfs.fsWriteJSON('/.memory_graph/export.json', { nodes: Object.values(a.mg._nodes), edges: [] }); + a.vfs.writeFile('/.memory_graph/nodes.json', 'broken'); // corrupt fallback + const before = Object.keys(a.mg._nodes).length; + + const out = []; + const mg2 = new MG(); + mg2.init({ print: (m, t) => out.push({ text: String(m), type: t || '' }), log: () => {}, fs: a.vfs }); + ok('C1 corrupt nodes.json is quarantined', !!a.vfs.readFile('/.memory_graph/nodes.json.corrupt')); + ok('C2 the valid export still loads the graph', Object.keys(mg2._nodes).length >= before, + 'before ' + before + ' after ' + Object.keys(mg2._nodes).length); +} + +if (fails === 0) { + console.log('test-memory-graph-recovery: ' + passes + ' passed, 0 failed'); +} else { + console.log('test-memory-graph-recovery: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); +} diff --git a/modules/test/test-skill-create.js b/modules/test/test-skill-create.js new file mode 100644 index 0000000..0c59f9d --- /dev/null +++ b/modules/test/test-skill-create.js @@ -0,0 +1,100 @@ +// modules/test/test-skill-create.js — run: node +// MAINT-024: skill-create must not report success when nothing was written, must reject an +// invalid name instead of rewriting it, and must not use an unreadable path as the body. +const fsMod = require('fs'), path = require('path'); +const ROOT = path.join(__dirname, '..', '..'); +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 load = (rel, cls) => new Function(fsMod.readFileSync(path.join(ROOT, rel), 'utf8') + '\nreturn ' + cls + ';')(); +const VFS = load('modules/core/virtual-fs.js', 'VirtualFileSystem'); +const SkillPack = load('modules/ext/skill-pack-core.js', 'SkillPackCoreModule'); + +function mk(opts) { + const out = []; + const vfs = new VFS(); + vfs.saveToStorage = () => true; + if (opts && opts.breakWrite) vfs.writeFile = () => false; + const sp = Object.create(SkillPack.prototype); + sp.terminal = { print: (m, t) => out.push({ text: String(m), type: t || '' }), fs: vfs }; + sp.commands = {}; + return { sp, vfs, out }; +} +const text = out => out.map(o => o.text).join(' | '); + +let fails = 0, passes = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} + +// A. first skill on a VFS with no /skills directory: created, for real +{ + const { sp, vfs, out } = mk(); + sp._cmdSkillCreate(['first-skill', 'does a thing', 'body text here']); + ok('A1 success message printed', /✅ Skill "first-skill" created/.test(text(out)), text(out)); + ok('A2 the file exists', typeof vfs.readFile('/skills/first-skill.md') === 'string'); + ok('A3 the index was written', !!vfs.fsReadJSON('/skills/index.json')['first-skill']); +} + +// B. an invalid name is refused, not silently rewritten +{ + const { sp, vfs, out } = mk(); + // Uppercase is a predictable normalisation and is reported back in the success line. + sp._cmdSkillCreate(['My-Skill', 'desc', 'body']); + ok('B1 uppercase is normalised and reported', /✅ Skill "my-skill" created/.test(text(out)), text(out)); + ok('B2 the old truncated name never appears', !/y-kill/.test(text(out)) && !vfs.readFile('/skills/y-kill.md'), text(out)); +} +{ + const { sp, vfs, out } = mk(); + // An underscore cannot be normalised away, so the name is rejected outright. + sp._cmdSkillCreate(['DevOps_Thing', 'desc', 'body']); + ok('B3 invalid name refused', /Invalid skill name "DevOps_Thing"/.test(text(out)), text(out)); + ok('B4 nothing was created under a guessed name', vfs.readFile('/skills/devops_thing.md') === null && vfs.readFile('/skills/evpshing.md') === null); + ok('B5 no success line', !/✅/.test(text(out)), text(out)); +} + +// C. a path that cannot be read is an error, not a body +{ + const { sp, vfs, out } = mk(); + sp._cmdSkillCreate(['path-body', 'desc', '/skills/does-not-exist.md']); + ok('C1 unreadable body refused', /Cannot read skill body/.test(text(out)), text(out)); + ok('C2 nothing created', vfs.readFile('/skills/path-body.md') === null); +} + +// D. a failed write must not print ✅ +{ + const { sp, out } = mk({ breakWrite: true }); + sp._cmdSkillCreate(['blocked-skill', 'desc', 'body']); + ok('D1 failed write is reported', /Failed to write \/skills\/blocked-skill\.md/.test(text(out)), text(out)); + ok('D2 no success line on a failed write', !/✅/.test(text(out)), text(out)); +} + +// E. a readable path body is used verbatim, comma text preserved +{ + const { sp, vfs, out } = mk(); + vfs.mkdirRecursive('/skills'); + vfs.writeFile('/body.md', 'line one, with a comma\nline two\nline three'); + sp._cmdSkillCreate(['from-file', 'desc', '/body.md', '--tags=a,b', '--category=testing']); + const content = vfs.readFile('/skills/from-file.md') || ''; + ok('E1 path body used verbatim', content.includes('line one, with a comma\nline two\nline three'), content.slice(0, 120)); + ok('E2 tags parsed', content.includes('tags: [a, b]') && content.includes('category: testing'), content.slice(0, 160)); + ok('E3 success printed', /✅ Skill "from-file" created/.test(text(out)), text(out)); +} + +// F. too few arguments still shows usage +{ + const { sp, out } = mk(); + sp._cmdSkillCreate(['only-name', 'desc']); + ok('F1 usage shown', /Usage: skill-create/.test(text(out)), text(out)); +} + +if (fails === 0) { + console.log('test-skill-create: ' + passes + ' passed, 0 failed'); +} else { + console.log('test-skill-create: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); +} 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); +} |