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/core/core-bundle.js | 69 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 2 deletions(-) (limited to 'modules/core/core-bundle.js') 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; -- cgit v1.2.3