diff options
| author | Agando <roduccomm@gmail.com> | 2026-10-27 14:13:35 +0100 |
|---|---|---|
| committer | Agando <roduccomm@gmail.com> | 2026-10-27 14:13:35 +0100 |
| commit | abe6cd64666e1fa61cc79ea4392cd78339e581d8 (patch) | |
| tree | b64d0ef1516df11022969d62fffcf64937535a9a | |
| parent | 9a61780447066919964b66cdbaa35b9165629419 (diff) | |
fix(newos4): connect to Ollama on first use, honest write_file, list_files sees /sys
Three defects, each re-probed first and covered by a test that is red on HEAD.
- ollama-manager.js: nothing connects at boot; only the Ollama tab button called connect(),
so a user's first message died with "Ollama not connected. Use ollamaconnect first." even
when the endpoint was healthy. New _ensureConnected() runs from chat(), chatStream() and
chatWithTools(): one attempt, shared by concurrent callers, a "Connecting…" hint, and only
then the error — which now says the endpoint is unreachable instead of telling the user to
run a command they already expected to work.
- module-aiassist.js write_file tool: fs.writeFile()'s false return was dropped and the tool
answered "File written: …" anyway, sending the model on to a read that found nothing
(MAINT-012). It now returns "Write FAILED: … — parent directory missing or target is a
directory; nothing was written".
- module-aiassist.js list_files tool: the hand-rolled walk over root.content never resolved
the virtual mounts, so /sys — a directory the UI and `ls` both show — answered
"Directory not found" (MAINT-011). It now uses fs.readDir(), which resolves mounts and
returns null for a genuinely missing path.
Tests: modules/test/test-ai-fs-tools.js (7 asserts) and
modules/test/test-ollama-autoconnect.js (11 asserts). RED on HEAD: ai tools 4 passed/3 FAILED
(/sys → "Directory not found", write_file → "File written: …"), ollama 2 passed/9 FAILED
("Use ollamaconnect first"). GREEN now 7/7 and 11/11; maintain --fix reports "node tests 9/9 pass".
Live on v4.6.105: connectedBefore=false, _ensureConnected()=true in 41 ms, 37 models.
| -rw-r--r-- | main.html | 6 | ||||
| -rw-r--r-- | modules/core/core-bundle.js | 92 | ||||
| -rw-r--r-- | modules/core/module-aiassist.js | 54 | ||||
| -rw-r--r-- | modules/core/module-loader.js | 2 | ||||
| -rw-r--r-- | modules/core/ollama-manager.js | 46 | ||||
| -rw-r--r-- | modules/test/test-ai-fs-tools.js | 74 | ||||
| -rw-r--r-- | modules/test/test-ollama-autoconnect.js | 98 |
7 files changed, 287 insertions, 85 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.104"></script> +<script src="modules/core/core-bundle.js?v=v4.6.105"></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.104"></script> +<script src="modules/core/module-loader.js?v=v4.6.105"></script> <!-- Critical ext bundle loads AFTER webx11 exists so registerModule calls work --> -<script src="modules/ext/critical-ext-bundle.js?v=v4.6.104"></script> +<script src="modules/ext/critical-ext-bundle.js?v=v4.6.105"></script> <script> // ═══ ONE TRUE VERSION SOURCE ═══ diff --git a/modules/core/core-bundle.js b/modules/core/core-bundle.js index d40988e..0d3a783 100644 --- a/modules/core/core-bundle.js +++ b/modules/core/core-bundle.js @@ -1604,11 +1604,36 @@ class OllamaManager { } } + // ─── Lazy connect ──────────────────────────────────────────────────────── + /** + * Every request path calls this first. Nothing connects at boot, so the user's first message + * used to die with "Use ollamaconnect first" even when the endpoint was healthy + * (reported 2026-10-26/27). One attempt, then the caller prints the real reason. + * Concurrent callers share the same in-flight promise. + */ + async _ensureConnected() { + if (this.connected) return true; + if (this._connecting) { + try { await this._connecting; } catch (e) { /* fall through to the state check */ } + return this.connected; + } + this.terminal.print('Connecting to Ollama…', 'info'); + this._connecting = this.connect(); + try { await this._connecting; } catch (e) { + this.terminal.log('WARN', 'Ollama', 'Auto-connect failed: ' + e.message); + } + this._connecting = null; + if (!this.connected) { + this.terminal.print('Auto-connect failed — check the endpoint in the Ollama tab (or run ollamaconnect).', 'warning'); + } + return this.connected; + } + // ─── Chat (non-streaming) ──────────────────────────────────────────────── async chat(prompt, model = null) { - if (!this.connected) { - this.terminal.print('Ollama not connected. Use ollamaconnect first.', 'error'); + if (!this.connected && !(await this._ensureConnected())) { + this.terminal.print('Ollama not connected: the endpoint is unreachable. Check the Ollama tab or run ollamaconnect.', 'error'); return null; } const useModel = model || this.currentModel; @@ -1775,7 +1800,10 @@ class OllamaManager { // ─── Chat Stream ───────────────────────────────────────────────────────── async chatStream(prompt, onToken, onComplete, model = null) { - if (!this.connected) { this.terminal.print('Ollama not connected.', 'error'); onComplete && onComplete(null); return; } + if (!this.connected && !(await this._ensureConnected())) { + this.terminal.print('Ollama not connected: the endpoint is unreachable.', 'error'); + onComplete && onComplete(null); return; + } const useModel = model || this.currentModel; if (!useModel) { this.terminal.print('No model selected.', 'error'); onComplete && onComplete(null); return; } @@ -1946,8 +1974,8 @@ class OllamaManager { } async chatWithTools(messages, tools, model = null) { - if (!this.connected) { - this.terminal.print('Ollama not connected. Use ollamaconnect first.', 'error'); + if (!this.connected && !(await this._ensureConnected())) { + this.terminal.print('Ollama not connected: the endpoint is unreachable. Check the Ollama tab or run ollamaconnect.', 'error'); return { message: null, tool_calls: null, error: 'Not connected' }; } const useModel = model || this.currentModel; @@ -10814,41 +10842,19 @@ buildSystemPrompt() { switch (name) { case 'list_files': { const originalPath = a.path || ''; - const p = originalPath.replace(/^\//, ''); - const saved = [...this.terminal.fs.currentPath]; - if (originalPath.startsWith('/')) { - this.terminal.fs.currentPath = []; + const p = originalPath.replace(/^\//, '').replace(/\/+$/, ''); + const fsapi = this.terminal.fs; + const saved = [...fsapi.currentPath]; + if (originalPath.startsWith('/')) fsapi.currentPath = []; + // MAINT-011: readDir() resolves the virtual mounts and returns null for a + // path that does not exist. The old hand-rolled walk over root.content could + // not see /sys, so a directory the UI shows answered "Directory not found". + const entries = fsapi.readDir(p || '/'); + fsapi.currentPath = saved; + if (entries === null || entries === undefined) { + return JSON.stringify({ error: `Directory not found: ${p || '/'}` }); } - if (p) { - const parts = p.split('/'); - const dir = this.terminal.fs.root; - let cur = dir; - let found = true; - for (const part of parts) { - if (part === '..') { saved.pop(); continue; } - if (part && cur.content[part] && cur.content[part].type === 'dir') cur = cur.content[part]; - else { found = false; break; } - } - if (!found) { - // Retry from root — cwd may be stale - this.terminal.fs.currentPath = []; - cur = this.terminal.fs.root; - found = true; - for (const part of parts) { - if (part === '..') continue; - if (part && cur.content[part] && cur.content[part].type === 'dir') cur = cur.content[part]; - else { found = false; break; } - } - if (!found) { - this.terminal.fs.currentPath = saved; - return JSON.stringify({ error: `Directory not found: ${p}` }); - } - } - this.terminal.fs.currentPath = saved.concat(parts.filter(x => x !== '..')); - } - const files = this.terminal.fs.ls(); - this.terminal.fs.currentPath = saved; - return JSON.stringify(files.map(f => ({ name: f.name, type: f.type, size: f.size }))); + return JSON.stringify(entries.map(f => ({ name: f.name, type: f.type, size: f.size }))); } case 'read_file': { const rp = a.path || ''; @@ -10888,8 +10894,14 @@ buildSystemPrompt() { if (wp.startsWith('/')) this.terminal.fs.currentPath = []; const path = wp.replace(/^\//, ''); const content = a.content || ''; - this.terminal.fs.writeFile(path, content); + // MAINT-012: writeFile() returns false when the parent directory is missing + // or the target is a directory node. Ignoring it told the model the write + // succeeded, and its follow-up read then found nothing. + const wrote = this.terminal.fs.writeFile(path, content); this.terminal.fs.currentPath = savedW; + if (!wrote) { + return `Write FAILED: ${path} — parent directory missing or target is a directory; nothing was written`; + } return `File written: ${path} (${content.length} chars)`; } case 'make_directory': { diff --git a/modules/core/module-aiassist.js b/modules/core/module-aiassist.js index 43cbb11..303f9c2 100644 --- a/modules/core/module-aiassist.js +++ b/modules/core/module-aiassist.js @@ -2372,41 +2372,19 @@ buildSystemPrompt() { switch (name) { case 'list_files': { const originalPath = a.path || ''; - const p = originalPath.replace(/^\//, ''); - const saved = [...this.terminal.fs.currentPath]; - if (originalPath.startsWith('/')) { - this.terminal.fs.currentPath = []; + const p = originalPath.replace(/^\//, '').replace(/\/+$/, ''); + const fsapi = this.terminal.fs; + const saved = [...fsapi.currentPath]; + if (originalPath.startsWith('/')) fsapi.currentPath = []; + // MAINT-011: readDir() resolves the virtual mounts and returns null for a + // path that does not exist. The old hand-rolled walk over root.content could + // not see /sys, so a directory the UI shows answered "Directory not found". + const entries = fsapi.readDir(p || '/'); + fsapi.currentPath = saved; + if (entries === null || entries === undefined) { + return JSON.stringify({ error: `Directory not found: ${p || '/'}` }); } - if (p) { - const parts = p.split('/'); - const dir = this.terminal.fs.root; - let cur = dir; - let found = true; - for (const part of parts) { - if (part === '..') { saved.pop(); continue; } - if (part && cur.content[part] && cur.content[part].type === 'dir') cur = cur.content[part]; - else { found = false; break; } - } - if (!found) { - // Retry from root — cwd may be stale - this.terminal.fs.currentPath = []; - cur = this.terminal.fs.root; - found = true; - for (const part of parts) { - if (part === '..') continue; - if (part && cur.content[part] && cur.content[part].type === 'dir') cur = cur.content[part]; - else { found = false; break; } - } - if (!found) { - this.terminal.fs.currentPath = saved; - return JSON.stringify({ error: `Directory not found: ${p}` }); - } - } - this.terminal.fs.currentPath = saved.concat(parts.filter(x => x !== '..')); - } - const files = this.terminal.fs.ls(); - this.terminal.fs.currentPath = saved; - return JSON.stringify(files.map(f => ({ name: f.name, type: f.type, size: f.size }))); + return JSON.stringify(entries.map(f => ({ name: f.name, type: f.type, size: f.size }))); } case 'read_file': { const rp = a.path || ''; @@ -2446,8 +2424,14 @@ buildSystemPrompt() { if (wp.startsWith('/')) this.terminal.fs.currentPath = []; const path = wp.replace(/^\//, ''); const content = a.content || ''; - this.terminal.fs.writeFile(path, content); + // MAINT-012: writeFile() returns false when the parent directory is missing + // or the target is a directory node. Ignoring it told the model the write + // succeeded, and its follow-up read then found nothing. + const wrote = this.terminal.fs.writeFile(path, content); this.terminal.fs.currentPath = savedW; + if (!wrote) { + return `Write FAILED: ${path} — parent directory missing or target is a directory; nothing was written`; + } return `File written: ${path} (${content.length} chars)`; } case 'make_directory': { diff --git a/modules/core/module-loader.js b/modules/core/module-loader.js index 0d61710..1f790e3 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.104'; + const LATEST_VER = 'v4.6.105'; const CACHE = '?v=' + LATEST_VER; const stored = localStorage.getItem('webx11_cache_ver'); if (stored !== LATEST_VER) { diff --git a/modules/core/ollama-manager.js b/modules/core/ollama-manager.js index 4f47db2..3d157ba 100644 --- a/modules/core/ollama-manager.js +++ b/modules/core/ollama-manager.js @@ -369,11 +369,36 @@ class OllamaManager { } } + // ─── Lazy connect ──────────────────────────────────────────────────────── + /** + * Every request path calls this first. Nothing connects at boot, so the user's first message + * used to die with "Use ollamaconnect first" even when the endpoint was healthy + * (reported 2026-10-26/27). One attempt, then the caller prints the real reason. + * Concurrent callers share the same in-flight promise. + */ + async _ensureConnected() { + if (this.connected) return true; + if (this._connecting) { + try { await this._connecting; } catch (e) { /* fall through to the state check */ } + return this.connected; + } + this.terminal.print('Connecting to Ollama…', 'info'); + this._connecting = this.connect(); + try { await this._connecting; } catch (e) { + this.terminal.log('WARN', 'Ollama', 'Auto-connect failed: ' + e.message); + } + this._connecting = null; + if (!this.connected) { + this.terminal.print('Auto-connect failed — check the endpoint in the Ollama tab (or run ollamaconnect).', 'warning'); + } + return this.connected; + } + // ─── Chat (non-streaming) ──────────────────────────────────────────────── async chat(prompt, model = null) { - if (!this.connected) { - this.terminal.print('Ollama not connected. Use ollamaconnect first.', 'error'); + if (!this.connected && !(await this._ensureConnected())) { + this.terminal.print('Ollama not connected: the endpoint is unreachable. Check the Ollama tab or run ollamaconnect.', 'error'); return null; } const useModel = model || this.currentModel; @@ -540,7 +565,10 @@ class OllamaManager { // ─── Chat Stream ───────────────────────────────────────────────────────── async chatStream(prompt, onToken, onComplete, model = null) { - if (!this.connected) { this.terminal.print('Ollama not connected.', 'error'); onComplete && onComplete(null); return; } + if (!this.connected && !(await this._ensureConnected())) { + this.terminal.print('Ollama not connected: the endpoint is unreachable.', 'error'); + onComplete && onComplete(null); return; + } const useModel = model || this.currentModel; if (!useModel) { this.terminal.print('No model selected.', 'error'); onComplete && onComplete(null); return; } @@ -711,8 +739,8 @@ class OllamaManager { } async chatWithTools(messages, tools, model = null) { - if (!this.connected) { - this.terminal.print('Ollama not connected. Use ollamaconnect first.', 'error'); + if (!this.connected && !(await this._ensureConnected())) { + this.terminal.print('Ollama not connected: the endpoint is unreachable. Check the Ollama tab or run ollamaconnect.', 'error'); return { message: null, tool_calls: null, error: 'Not connected' }; } const useModel = model || this.currentModel; @@ -893,7 +921,13 @@ class OllamaManager { } return { - message: message ? { role: message.role || 'assistant', content: content } : null, + message: message ? { + role: message.role || 'assistant', + content: content, + // OpenAI-compatible providers require tool_calls INSIDE the assistant message; + // a bare { role:'tool' } with no preceding tool_calls is rejected with HTTP 400. + ...(Array.isArray(toolCalls) && toolCalls.length ? { tool_calls: toolCalls } : {}), + } : null, tool_calls: toolCalls ? toolCalls.map(tc => ({ id: tc.id, type: tc.type || 'function', diff --git a/modules/test/test-ai-fs-tools.js b/modules/test/test-ai-fs-tools.js new file mode 100644 index 0000000..c427783 --- /dev/null +++ b/modules/test/test-ai-fs-tools.js @@ -0,0 +1,74 @@ +// modules/test/test-ai-fs-tools.js — run: node +// Proves MAINT-011 (list_files must see the virtual /sys mount) and MAINT-012 +// (write_file must not report success when the write was refused). +const fsMod = require('fs'), path = require('path'); +const ROOT = path.join(__dirname, '..', 'core'); +const ls = {}; +global.localStorage = { getItem: k => (k in ls ? ls[k] : null), setItem: (k, v) => { ls[k] = String(v); }, removeItem: k => { delete ls[k]; } }; +global.indexedDB = { open() { throw new Error('no idb'); }, deleteDatabase() { throw new Error('no idb'); } }; +global.window = global; + +const load = (file, cls) => new Function(fsMod.readFileSync(path.join(ROOT, file), 'utf8') + '\nreturn ' + cls + ';')(); +const VFS = load('virtual-fs.js', 'VirtualFileSystem'); +const AI = load('module-aiassist.js', 'WebX11AIAssistantModule'); + +function mk() { + const out = []; + const vfs = new VFS(); + vfs.saveToStorage = () => true; + // Bypass the constructor (it touches the DOM) but keep the real prototype methods. + const ai = Object.create(AI.prototype); + ai.terminal = { print: (m, t) => out.push({ text: String(m), type: t || '' }), log: () => {}, fs: vfs }; + ai.checkGuardrail = () => null; + return { ai, vfs, out }; +} + +let fails = 0, passes = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} +const run = async (ai, name, args) => JSON.parse(await ai.runTool(name, args)); + +(async () => { + // A. list_files on the virtual /sys mount + { + const { ai } = mk(); + const res = await run(ai, 'list_files', { path: '/sys' }); + ok('A1 /sys lists entries instead of an error', Array.isArray(res), JSON.stringify(res).slice(0, 120)); + ok('A2 /sys contains the modules mount', Array.isArray(res) && res.some(e => e.name === 'modules'), + JSON.stringify(res).slice(0, 160)); + } + + // B. a genuinely missing directory still reports the error + { + const { ai } = mk(); + const res = await run(ai, 'list_files', { path: '/no-such-dir' }); + ok('B1 missing path reports Directory not found', !!res.error && /Directory not found/.test(res.error), JSON.stringify(res)); + } + + // C. write_file into a missing parent must not claim success + { + const { ai, vfs } = mk(); + const res = await ai.runTool('write_file', { path: '/missing/dir/f.txt', content: 'x' }); + ok('C1 refused write reports failure', typeof res === 'string' && /Write FAILED/.test(res), String(res)); + ok('C2 nothing was created', vfs.readFile('/missing/dir/f.txt') === null); + } + + // D. the same write succeeds once the parent exists + { + const { ai, vfs } = mk(); + vfs.mkdirRecursive('/missing/dir'); + const res = await ai.runTool('write_file', { path: '/missing/dir/f.txt', content: 'x' }); + ok('D1 successful write still reports success', /File written/.test(String(res)), String(res)); + ok('D2 content landed', vfs.readFile('/missing/dir/f.txt') === 'x'); + } + + if (fails === 0) { + console.log('test-ai-fs-tools: ' + passes + ' passed, 0 failed'); + } else { + console.log('test-ai-fs-tools: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); + } +})(); diff --git a/modules/test/test-ollama-autoconnect.js b/modules/test/test-ollama-autoconnect.js new file mode 100644 index 0000000..b9db4ff --- /dev/null +++ b/modules/test/test-ollama-autoconnect.js @@ -0,0 +1,98 @@ +// modules/test/test-ollama-autoconnect.js — run: node +// Proves the first message lazily connects instead of dying with "Use ollamaconnect first", +// and that a genuinely unreachable endpoint still produces the error (after one attempt). +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.window = { addEventListener: () => {}, location: { origin: 'https://webx11.duckdns.org' } }; +global.document = { addEventListener: () => {} }; + +const OllamaManager = new Function( + fsMod.readFileSync(path.join(ROOT, 'ollama-manager.js'), 'utf8') + '\nreturn OllamaManager;')(); + +let fails = 0, passes = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} + +function mk(behaviour) { + const out = []; + const term = { + print: (m, t) => out.push({ text: String(m), type: t || '' }), + log: () => {}, + // chat() prints the model answer through this, even in non-streaming mode + printStreamingResponse: (t) => out.push({ text: String(t), type: 'stream' }), + }; + const om = Object.create(OllamaManager.prototype); + Object.assign(om, { + terminal: term, connected: false, 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, + saveSettings: () => {}, _saveProviders: () => {}, _logUsage: () => {}, _flushUsageLog: () => {}, + }); + om.currentProvider = om.providers[0]; + global.fetch = behaviour; + return { om, out }; +} + +const json = (body, okFlag = true) => Promise.resolve({ ok: okFlag, status: okFlag ? 200 : 500, json: async () => body }); + +(async () => { + // A. healthy endpoint: the first chat() call connects, then answers + { + const seen = []; + const { om, out } = mk((url) => { + seen.push(url); + if (url.includes('/api/tags')) return json({ models: [{ name: 'm1' }] }); + // chat() goes through /api/generate for ollama providers + return json({ response: 'pong', message: { content: 'pong' }, eval_count: 1, prompt_eval_count: 1 }); + }); + 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('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)); + } + + // B. dead endpoint: one attempt, then an actionable error + { + const seen = []; + const { om, out } = mk((url) => { seen.push(url); return Promise.reject(new TypeError('Failed to fetch')); }); + const res = await om.chat('ping'); + const text = out.map(o => o.text).join(' | '); + ok('B1 still attempts the connection', seen.some(u => u.includes('/api/tags')), seen.join(', ')); + ok('B2 returns null', res === null, String(res)); + ok('B3 error names the real cause', /endpoint is unreachable/.test(text), text.slice(0, 240)); + ok('B4 auto-connect failure is reported once', /Auto-connect failed/.test(text), text.slice(0, 240)); + ok('B5 manager stays disconnected', om.connected === false); + } + + // C. concurrent callers share one attempt + { + let tagCalls = 0; + const { om } = mk((url) => { + if (url.includes('/api/tags')) { tagCalls++; return json({ models: [{ name: 'm1' }] }); } + return json({ response: 'ok', message: { content: 'ok' }, eval_count: 1, prompt_eval_count: 1 }); + }); + await Promise.all([om.chat('a'), om.chat('b'), om.chat('c')]); + ok('C1 three concurrent messages probe /api/tags once', tagCalls === 1, 'tagCalls=' + tagCalls); + } + + if (fails === 0) { + console.log('test-ollama-autoconnect: ' + passes + ' passed, 0 failed'); + } else { + console.log('test-ollama-autoconnect: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); + } +})(); |