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 /modules/test/test-skill-create.js | |
| 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.
Diffstat (limited to 'modules/test/test-skill-create.js')
| -rw-r--r-- | modules/test/test-skill-create.js | 100 |
1 files changed, 100 insertions, 0 deletions
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); +} |