diff options
| author | Agando <roduccomm@gmail.com> | 2026-10-27 14:41:20 +0100 |
|---|---|---|
| committer | Agando <roduccomm@gmail.com> | 2026-10-27 14:41:20 +0100 |
| commit | 0ccd2cbc4cc15e0a362b08d76a8bd814abfb9ed9 (patch) | |
| tree | 50020b322d79651c4e70662550b793905708a9ee /modules/test | |
| parent | ea3acceac673b3cb6d84d38f968f057082d8ccb8 (diff) | |
fix(newos4): name the real Ollama failure, bound auto-connect, fix the review findings
Second pass over v4.6.105, driven by an independent review that reproduced each item.
- ollama-manager.js: chat() now reports the captured cause ("Ollama error: Failed to fetch",
"…: HTTP 500") instead of the generic "No provider responded"; _lastChatError is reset per
call so an old reason is never replayed. chatWithTools carries the reason in its error string.
- ollama-manager.js: _ensureConnected() bounds connect() with a deadline (connectTimeoutMs,
default 10 s). connect() uses a bare fetch(), so a blackholed endpoint previously left the
shared in-flight promise pending forever and hung every later message — the old code failed
fast. The attempt is now try/finally, so a throwing terminal.log() can no longer poison
_connecting and suppress the retry on the next message.
- ollama-manager.js: an automatic connect no longer silently rewrites the user's saved model;
the previous choice is restored in memory and in localStorage, and the user is told.
- ollama-manager.js: the DeepSeek empty-content branch returned top-level tool_calls with
content '', so the caller pushed an assistant message with no tool_calls and then a
role:'tool' reply — an orphan tool message, which is the HTTP 400 that branch exists to
prevent. tool_calls now live inside the message as well.
- module-aiassist.js: a no-argument list_files returned the ROOT instead of the cwd (readDir('/')
instead of ls()); it now means the current directory again. Error text keeps the path as the
caller wrote it, and write_file distinguishes a read-only virtual path (/sys/…) from a missing
parent instead of blaming the parent.
Tests: test-ollama-error-text.js (new, 14 asserts, RED 9/5 before the edit), test-ai-fs-tools.js
7 → 16 asserts (write onto a directory, /sys write, list a file, '/', no-arg cwd, relative path),
test-ollama-autoconnect.js 11 → 19 (blackholed endpoint settles + retries, saved model survives;
the old "connecting hint" assertion is now falsifiable — the reviewer showed it also matched
connect()'s pre-existing line). Suite: 10/10 files green. Live on v4.6.106: healthy auto-connect
42 ms with the hint; blackholed endpoint settles in 1242 ms with connectTimeoutMs=1200, prints
"timed out" and "unreachable", _connecting cleared.
Diffstat (limited to 'modules/test')
| -rw-r--r-- | modules/test/test-ai-fs-tools.js | 52 | ||||
| -rw-r--r-- | modules/test/test-ollama-autoconnect.js | 34 | ||||
| -rw-r--r-- | modules/test/test-ollama-error-text.js | 114 |
3 files changed, 199 insertions, 1 deletions
diff --git a/modules/test/test-ai-fs-tools.js b/modules/test/test-ai-fs-tools.js index c427783..3968abe 100644 --- a/modules/test/test-ai-fs-tools.js +++ b/modules/test/test-ai-fs-tools.js @@ -65,6 +65,58 @@ const run = async (ai, name, args) => JSON.parse(await ai.runTool(name, args)); ok('D2 content landed', vfs.readFile('/missing/dir/f.txt') === 'x'); } + // E. write onto a directory node is refused, and the directory survives + { + const { ai, vfs } = mk(); + vfs.mkdirRecursive('/adir'); + vfs.writeFile('/adir/keep.txt', 'k'); + const res = await ai.runTool('write_file', { path: '/adir', content: 'x' }); + ok('E1 write onto a directory is refused', /Write FAILED/.test(String(res)), String(res)); + ok('E2 directory contents intact', vfs.readFile('/adir/keep.txt') === 'k'); + } + + // F. a read-only virtual path names that as the reason + { + const { ai } = mk(); + const res = await ai.runTool('write_file', { path: '/sys/info', content: 'x' }); + ok('F1 virtual path reports read-only', /Write FAILED: \/sys\/info — read-only virtual path/.test(String(res)), String(res)); + } + + // G. list_files on a FILE is an error, on '/' is the root listing + { + const { ai, vfs } = mk(); + vfs.writeFile('/file.txt', 'x'); + const onFile = await run(ai, 'list_files', { path: '/file.txt' }); + ok('G1 listing a file reports Directory not found', !!onFile.error && /\/file\.txt/.test(onFile.error), JSON.stringify(onFile)); + const onRoot = await run(ai, 'list_files', { path: '/' }); + ok('G2 root lists the virtual sys mount', Array.isArray(onRoot) && onRoot.some(e => e.name === 'sys'), JSON.stringify(onRoot).slice(0, 120)); + } + + // H. no argument means the CURRENT directory, not the root + { + const { ai, vfs } = mk(); + vfs.mkdirRecursive('/work'); + vfs.writeFile('/work/inside.txt', 'x'); + vfs.writeFile('/root-only.txt', 'y'); + vfs.currentPath = ['work']; + const cwd = await run(ai, 'list_files', {}); + const names = (cwd.error ? [] : cwd.map(e => e.name)); + ok('H1 no-arg lists the cwd', names.includes('inside.txt') && !names.includes('root-only.txt'), JSON.stringify(cwd).slice(0, 140)); + ok('H2 cwd is restored after the call', JSON.stringify(vfs.currentPath) === '["work"]', JSON.stringify(vfs.currentPath)); + } + + // I. a relative path resolves from the cwd + { + const { ai, vfs } = mk(); + vfs.mkdirRecursive('/docs'); + vfs.writeFile('/docs/sub.txt', 'x'); + vfs.currentPath = ['docs']; + const res = await run(ai, 'list_files', { path: 'sub.txt' }); + ok('I1 a relative file path is resolved, not guessed', !!res.error, JSON.stringify(res)); + const dirRes = await run(ai, 'list_files', { path: '.' }); + ok('I2 "." lists the cwd', Array.isArray(dirRes) && dirRes.some(e => e.name === 'sub.txt'), JSON.stringify(dirRes).slice(0, 120)); + } + if (fails === 0) { console.log('test-ai-fs-tools: ' + passes + ' passed, 0 failed'); } else { diff --git a/modules/test/test-ollama-autoconnect.js b/modules/test/test-ollama-autoconnect.js index b9db4ff..12b0786 100644 --- a/modules/test/test-ollama-autoconnect.js +++ b/modules/test/test-ollama-autoconnect.js @@ -59,7 +59,7 @@ const json = (body, okFlag = true) => Promise.resolve({ ok: okFlag, status: okFl const res = await om.chat('ping'); const text = out.map(o => o.text).join(' | '); ok('A1 connect is attempted lazily', seen.some(u => u.includes('/api/tags')), seen.join(', ')); - ok('A2 the user sees a connecting hint', /Connecting to Ollama/.test(text), text.slice(0, 160)); + ok('A2 the user sees a first-use hint', /connecting on first use/.test(text), text.slice(0, 160)); ok('A3 manager is connected after the attempt', om.connected === true); ok('A4 the message is answered', String(res).includes('pong'), String(res).slice(0, 120)); ok('A5 no "ollamaconnect first" error', !/ollamaconnect first/.test(text), text.slice(0, 200)); @@ -89,6 +89,38 @@ const json = (body, okFlag = true) => Promise.resolve({ ok: okFlag, status: okFl ok('C1 three concurrent messages probe /api/tags once', tagCalls === 1, 'tagCalls=' + tagCalls); } + // D. a blackholed endpoint must not hang every later message + { + const { om, out } = mk(() => new Promise(() => {})); // never settles + om.connectTimeoutMs = 100; + const t0 = Date.now(); + const res = await om.chat('ping'); + const ms = Date.now() - t0; + const text = out.map(o => o.text).join(' | '); + ok('D1 chat settles on a silent endpoint', ms < 3000, ms + 'ms'); + ok('D2 returns null', res === null, String(res)); + ok('D3 the timeout is reported', /timed out/.test(text), text.slice(0, 220)); + ok('D4 _connecting is cleared', om._connecting === null, String(om._connecting)); + const before = out.length; + await om.chat('again'); + ok('D5 the next message retries the connect', out.length > before, 'lines ' + before + ' -> ' + out.length); + } + + // E. an automatic connect must not rewrite the model the user saved + { + store['webx11_ollama_model'] = 'my-saved-model'; + const { om, out } = mk((url) => url.includes('/api/tags') + ? json({ models: [{ name: 'other-model' }] }) + : json({ response: 'x', eval_count: 1, prompt_eval_count: 1 })); + om.currentModel = 'my-saved-model'; + await om._ensureConnected(); + const text = out.map(o => o.text).join(' | '); + ok('E1 the saved model survives an auto-connect', om.currentModel === 'my-saved-model', om.currentModel); + ok('E2 the stored preference is restored', store['webx11_ollama_model'] === 'my-saved-model', String(store['webx11_ollama_model'])); + ok('E3 the user is told why', /Keeping your saved model/.test(text), text.slice(0, 200)); + delete store['webx11_ollama_model']; + } + if (fails === 0) { console.log('test-ollama-autoconnect: ' + passes + ' passed, 0 failed'); } else { diff --git a/modules/test/test-ollama-error-text.js b/modules/test/test-ollama-error-text.js new file mode 100644 index 0000000..01a17b4 --- /dev/null +++ b/modules/test/test-ollama-error-text.js @@ -0,0 +1,114 @@ +// modules/test/test-ollama-error-text.js — run: node +// Proves a failed Ollama chat names the REAL cause ("Failed to fetch", "HTTP 500") +// instead of the generic "No provider responded", and that a stale reason captured by +// an earlier, unrelated failure is never replayed on a later call. +const fsMod = require('fs'), path = require('path'); +const CORE = 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.window = { addEventListener: () => {}, location: { origin: 'https://webx11.duckdns.org' } }; +global.document = { addEventListener: () => {} }; + +const OllamaManager = new Function( + fsMod.readFileSync(path.join(CORE, 'ollama-manager.js'), 'utf8') + '\nreturn OllamaManager;')(); + +let passes = 0, fails = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} + +// A manager that is already "connected" so chat() goes straight to the provider loop: +// this test is about provider failure text, not about the lazy-connect gate. +function mk() { + const out = []; + const term = { + print: (m, t) => out.push({ text: String(m), type: t || '' }), + log: () => {}, + printStreamingResponse: t => out.push({ text: String(t), type: 'stream' }), + }; + const om = Object.create(OllamaManager.prototype); + Object.assign(om, { + terminal: term, + connected: true, models: [], currentModel: 'm1', + endpoint: 'https://webx11.duckdns.org/ollama', fallbackEndpoints: [], + providers: [{ name: 'Ollama', type: 'ollama', endpoint: 'https://webx11.duckdns.org/ollama', model: 'm1', priority: 1, enabled: true }], + currentProvider: null, fallbackCount: 0, responseHooks: [], _usageLog: [], _usageBuffer: [], + _abortController: null, _connecting: null, streamEnabled: false, + saveSettings: () => {}, _saveProviders: () => {}, _logUsage: () => {}, _flushUsageLog: () => {}, + }); + om.currentProvider = om.providers[0]; + return { om, out }; +} + +const json = (body, okFlag = true) => Promise.resolve({ + ok: okFlag, status: okFlag ? 200 : 500, json: async () => body, +}); +const textOf = out => out.map(o => o.text).join(' | '); +const errorLines = out => out.filter(o => /Ollama error/.test(o.text)).map(o => o.text).join(' | '); + +(async () => { + // A. every provider endpoint rejects: the user must see the transport cause + { + const { om, out } = mk(); + global.fetch = () => Promise.reject(new TypeError('Failed to fetch')); + const res = await om.chat('ping'); + const text = textOf(out), line = errorLines(out); + ok('A1 chat() returns null', res === null, String(res)); + ok('A2 error line mentions "Ollama error"', /Ollama error/.test(line), 'line=' + JSON.stringify(line)); + ok('A3 error line names the real cause "Failed to fetch"', /Failed to fetch/.test(line), 'line=' + JSON.stringify(line)); + ok('A4 generic "No provider responded" is not printed', !/No provider responded/.test(text), text.slice(0, 240)); + } + + // B. provider answers HTTP 500: the status code must survive into the message + { + const { om, out } = mk(); + global.fetch = () => json({ error: 'kaboom' }, false); + const res = await om.chat('ping'); + const line = errorLines(out); + ok('B1 chat() returns null', res === null, String(res)); + ok('B2 error line carries "Ollama error"', /Ollama error/.test(line), 'line=' + JSON.stringify(line)); + ok('B3 error line carries the HTTP status "500"', /500/.test(line), 'line=' + JSON.stringify(line)); + ok('B4 generic "No provider responded" is not printed', !/No provider responded/.test(textOf(out)), textOf(out).slice(0, 240)); + } + + // C. the reason is per-call state: a later call must not replay it + { + const { om, out } = mk(); + + // C1: a genuine failure captures a reason + global.fetch = () => Promise.reject(new TypeError('Failed to fetch')); + const first = await om.chat('first'); + + // C2: a success in between + global.fetch = () => json({ response: 'pong', eval_count: 1, prompt_eval_count: 1 }); + const second = await om.chat('second'); + const failMentionsAfterSuccess = out.filter(o => /Failed to fetch/.test(o.text)).length; + + // C3: a provider that fails WITHOUT throwing (empty body) — no reason to report, + // so the fallback wording must come back instead of the stale transport error. + global.fetch = () => json({ response: '', eval_count: 1, prompt_eval_count: 1 }); + const third = await om.chat('third'); + const lastLine = out.filter(o => /Ollama error/.test(o.text)).map(o => o.text).pop() || ''; + + ok('C1 first call returns null', first === null, String(first)); + ok('C2 successful call still answers', String(second).includes('pong'), String(second).slice(0, 120)); + ok('C3 the captured reason is logged exactly once', failMentionsAfterSuccess === 1, 'count=' + failMentionsAfterSuccess); + ok('C4 third call returns null', third === null, String(third)); + ok('C5 no-reason failure falls back to the generic wording', /No provider responded/.test(lastLine), 'last=' + JSON.stringify(lastLine)); + ok('C6 the stale "Failed to fetch" reason is not replayed', !/Failed to fetch/.test(lastLine), 'last=' + JSON.stringify(lastLine)); + } + + if (fails === 0) { + console.log('test-ollama-error-text: ' + passes + ' passed, 0 failed'); + } else { + console.log('test-ollama-error-text: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); + } +})(); |