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.

3 files changed, 283 insertions, 0 deletions

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);
+}