aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAgando <roduccomm@gmail.com>2026-11-02 20:55:56 +0100
committerAgando <roduccomm@gmail.com>2026-11-02 20:55:56 +0100
commitb339bc13199eb9326307905dd635c56974b55f53 (patch)
treeb8b3c893ef6cae270b656e40a47ffbb872310e1b
parentaeded980e3d998640db8a84ac0c8d189387467b2 (diff)
perf(newos4): delegated agent runs no longer serialise; a waiter can't steal a lock
Closes the second half of STEER-20261102-201045-ab08. - chat() gated the main-turn lock on nothing, so orchestrate-parallel's fan-out (each sub-task calling chat() on the same instance with its own agentTag) ran strictly one at a time. Measured 2x300ms delegated runs: 617ms before, 306ms now; the two 200ms user turns still take 400ms (main path stays serialised). - the 10s lock timeout called this._chatLock = false and printed "force released", which cleared a lock the waiter did not own - that released the main turn's lock while it was still running. A waiter now throws and the owner is the only writer. - delegated runs no longer clear the user's cancel flag on entry (an agent start used to reset this._cancelled, disarming a Stop the user had pressed). - tool-log entries carry the run they belong to and _recentToolLog(tag, n) is what feeds the run log line and _postRunReview, so concurrent runs no longer consume each other's entries. Known limit, unchanged: _currentTag is a single field, so under true concurrency an entry can be attributed to the wrong run (telemetry only - the answers are per-session). Namespacing it properly needs per-call state threading. Test: modules/test/test-chat-concurrency.js, 7 assertions, RED before (H1 617ms, H3 force-released, H5 no tags) and GREEN now; maintain 24/24.
-rw-r--r--main.html6
-rw-r--r--modules/core/core-bundle.js41
-rw-r--r--modules/core/module-aiassist.js41
-rw-r--r--modules/core/module-loader.js2
-rw-r--r--modules/test/test-chat-concurrency.js85
5 files changed, 151 insertions, 24 deletions
diff --git a/main.html b/main.html
index 858a43f..743d261 100644
--- a/main.html
+++ b/main.html
@@ -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.138"></script>
+<script src="modules/core/core-bundle.js?v=v4.6.139"></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.138"></script>
+<script src="modules/core/module-loader.js?v=v4.6.139"></script>
<!-- Critical ext bundle loads AFTER webx11 exists so registerModule calls work -->
-<script src="modules/ext/critical-ext-bundle.js?v=v4.6.138"></script>
+<script src="modules/ext/critical-ext-bundle.js?v=v4.6.139"></script>
<script>
// ═══ ONE TRUE VERSION SOURCE ═══
diff --git a/modules/core/core-bundle.js b/modules/core/core-bundle.js
index ef2d425..cf805a9 100644
--- a/modules/core/core-bundle.js
+++ b/modules/core/core-bundle.js
@@ -9699,18 +9699,24 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
if (!userMessage) {
this.terminal.print('Usage: ai <your-question>', 'warning'); return;
}
- this._cancelled = false;
- this.terminal.setAiBusy(true);
+ // A delegated run owns its own session (see _isAgentRun below) and must not queue
+ // behind the main turn: orchestrate-parallel's fan-out used to run strictly one
+ // sub-task at a time, and a waiter that hit the timeout cleared a lock it did not
+ // own — which released the main turn's lock while it was still running.
+ const _agentTag = (opts && opts.agentTag) || null;
+ if (!_agentTag) this._cancelled = false;
const lockStart = Date.now();
- while (this._chatLock) {
+ while (!_agentTag && this._chatLock) {
if (Date.now() - lockStart > this._chatLockTimeoutMs) {
- this._chatLock = false;
- this.terminal.print('⚠️ Chat lock timed out — force released', 'warning');
- break;
+ throw new Error("chat lock busy after " + Math.round(this._chatLockTimeoutMs / 1000)
+ + "s — refusing to release another turn's lock");
}
await new Promise(r => setTimeout(r, 50));
}
- this._chatLock = true;
+ if (!_agentTag) this._chatLock = true;
+ const _prevTag = this._currentTag;
+ this._currentTag = _agentTag || 'main';
+ this.terminal.setAiBusy(true);
const taskStart = Date.now(); // predictor durations exclude chat-lock wait
this.terminal.markDirty();
// Auto-route context based on intent
@@ -9867,7 +9873,7 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
try {
var om = this.terminal ? this.terminal.ollamaManager : null;
var usageData = om && om._usageLog && om._usageLog.length > 0 ? om._usageLog[om._usageLog.length - 1] : null;
- this._postRunReview(loopResult, this._toolCallLog.slice(-(loopResult.toolTurns || 0)), usageData);
+ this._postRunReview(loopResult, this._recentToolLog(_agentTag || 'main', loopResult.toolTurns || 0), usageData);
} catch (e) { /* self-improvement is best-effort */ }
}
// Auto-create skills from repeated errors
@@ -9884,7 +9890,8 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
setTimeout(() => this.terminal.playNotificationSound(), 100);
}
} finally {
- this._chatLock = false;
+ this._currentTag = _prevTag;
+ if (!_agentTag) this._chatLock = false;
this.terminal.setAiBusy(false);
// Persist only the user's conversation — agent sessions are ephemeral.
if (!_isAgentRun) this._saveContext();
@@ -9894,7 +9901,7 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
// `recent` was referenced but never declared: the bare identifier threw
// ReferenceError inside this finally block, rejecting chat() on every
// tool-using turn (shipped in 32608bc, caught by the caller sweep).
- const recent = this._toolCallLog.slice(-(loopResult.toolTurns || 0));
+ const recent = this._recentToolLog(_agentTag || 'main', loopResult.toolTurns || 0);
const toolNames = recent.map(function(e) { return e.name; }).join(', ');
this.terminal.log('INFO', 'AI', 'Used ' + loopResult.toolTurns + ' tool(s): ' + toolNames
+ ' · returned ' + Math.round(recent.reduce((a, e) => a + e.returnedBytes, 0) / 1024) + 'KB'
@@ -10231,6 +10238,19 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
// 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.
+ // A run may only see the tool calls it made: with concurrent delegated runs the
+ // raw ring buffer interleaves, and slicing it by toolTurns handed one agent
+ // another agent's entries.
+ _recentToolLog(tag, n) {
+ const want = tag || 'main';
+ const out = [];
+ const cap = Math.max(0, Number(n) || 0);
+ for (let i = this._toolCallLog.length - 1; i >= 0 && out.length < cap; i--) {
+ const e = this._toolCallLog[i];
+ if ((e.tag || 'main') === want) out.unshift(e);
+ }
+ return out;
+ }
_recordToolCall(entry) {
const r = String(entry.result || '');
let truncated = false, contentChars = null;
@@ -10250,6 +10270,7 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
clippedBytes: Math.max(0, r.length - Number(entry.returnedBytes || r.length)),
truncated: truncated, durationMs: entry.durationMs || 0,
ok: !entry.error, errorKind: entry.errorKind || null,
+ tag: entry.tag || this._currentTag || 'main',
};
this._toolCallLog.push(rec);
if (this._toolCallLog.length > 50) this._toolCallLog.shift();
diff --git a/modules/core/module-aiassist.js b/modules/core/module-aiassist.js
index 417e1b6..db4e691 100644
--- a/modules/core/module-aiassist.js
+++ b/modules/core/module-aiassist.js
@@ -824,18 +824,24 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
if (!userMessage) {
this.terminal.print('Usage: ai <your-question>', 'warning'); return;
}
- this._cancelled = false;
- this.terminal.setAiBusy(true);
+ // A delegated run owns its own session (see _isAgentRun below) and must not queue
+ // behind the main turn: orchestrate-parallel's fan-out used to run strictly one
+ // sub-task at a time, and a waiter that hit the timeout cleared a lock it did not
+ // own — which released the main turn's lock while it was still running.
+ const _agentTag = (opts && opts.agentTag) || null;
+ if (!_agentTag) this._cancelled = false;
const lockStart = Date.now();
- while (this._chatLock) {
+ while (!_agentTag && this._chatLock) {
if (Date.now() - lockStart > this._chatLockTimeoutMs) {
- this._chatLock = false;
- this.terminal.print('⚠️ Chat lock timed out — force released', 'warning');
- break;
+ throw new Error("chat lock busy after " + Math.round(this._chatLockTimeoutMs / 1000)
+ + "s — refusing to release another turn's lock");
}
await new Promise(r => setTimeout(r, 50));
}
- this._chatLock = true;
+ if (!_agentTag) this._chatLock = true;
+ const _prevTag = this._currentTag;
+ this._currentTag = _agentTag || 'main';
+ this.terminal.setAiBusy(true);
const taskStart = Date.now(); // predictor durations exclude chat-lock wait
this.terminal.markDirty();
// Auto-route context based on intent
@@ -992,7 +998,7 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
try {
var om = this.terminal ? this.terminal.ollamaManager : null;
var usageData = om && om._usageLog && om._usageLog.length > 0 ? om._usageLog[om._usageLog.length - 1] : null;
- this._postRunReview(loopResult, this._toolCallLog.slice(-(loopResult.toolTurns || 0)), usageData);
+ this._postRunReview(loopResult, this._recentToolLog(_agentTag || 'main', loopResult.toolTurns || 0), usageData);
} catch (e) { /* self-improvement is best-effort */ }
}
// Auto-create skills from repeated errors
@@ -1009,7 +1015,8 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
setTimeout(() => this.terminal.playNotificationSound(), 100);
}
} finally {
- this._chatLock = false;
+ this._currentTag = _prevTag;
+ if (!_agentTag) this._chatLock = false;
this.terminal.setAiBusy(false);
// Persist only the user's conversation — agent sessions are ephemeral.
if (!_isAgentRun) this._saveContext();
@@ -1019,7 +1026,7 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
// `recent` was referenced but never declared: the bare identifier threw
// ReferenceError inside this finally block, rejecting chat() on every
// tool-using turn (shipped in 32608bc, caught by the caller sweep).
- const recent = this._toolCallLog.slice(-(loopResult.toolTurns || 0));
+ const recent = this._recentToolLog(_agentTag || 'main', loopResult.toolTurns || 0);
const toolNames = recent.map(function(e) { return e.name; }).join(', ');
this.terminal.log('INFO', 'AI', 'Used ' + loopResult.toolTurns + ' tool(s): ' + toolNames
+ ' · returned ' + Math.round(recent.reduce((a, e) => a + e.returnedBytes, 0) / 1024) + 'KB'
@@ -1356,6 +1363,19 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
// 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.
+ // A run may only see the tool calls it made: with concurrent delegated runs the
+ // raw ring buffer interleaves, and slicing it by toolTurns handed one agent
+ // another agent's entries.
+ _recentToolLog(tag, n) {
+ const want = tag || 'main';
+ const out = [];
+ const cap = Math.max(0, Number(n) || 0);
+ for (let i = this._toolCallLog.length - 1; i >= 0 && out.length < cap; i--) {
+ const e = this._toolCallLog[i];
+ if ((e.tag || 'main') === want) out.unshift(e);
+ }
+ return out;
+ }
_recordToolCall(entry) {
const r = String(entry.result || '');
let truncated = false, contentChars = null;
@@ -1375,6 +1395,7 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re
clippedBytes: Math.max(0, r.length - Number(entry.returnedBytes || r.length)),
truncated: truncated, durationMs: entry.durationMs || 0,
ok: !entry.error, errorKind: entry.errorKind || null,
+ tag: entry.tag || this._currentTag || 'main',
};
this._toolCallLog.push(rec);
if (this._toolCallLog.length > 50) this._toolCallLog.shift();
diff --git a/modules/core/module-loader.js b/modules/core/module-loader.js
index eb916d1..7206155 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.138';
+ const LATEST_VER = 'v4.6.139';
const CACHE = '?v=' + LATEST_VER;
// Exposed so ext modules can register their own URL with the same cache-busting version
// instead of an unversioned path the browser may already have cached (shell.js did).
diff --git a/modules/test/test-chat-concurrency.js b/modules/test/test-chat-concurrency.js
new file mode 100644
index 0000000..19b7e45
--- /dev/null
+++ b/modules/test/test-chat-concurrency.js
@@ -0,0 +1,85 @@
+// node modules/test/test-chat-concurrency.js
+// Fan-out safety for chat(): delegated runs must not serialise, the main turn must stay
+// serialised, a waiter must never clear a lock it does not own, and tool-log entries must
+// be attributable to the run that produced them.
+const fs = require('fs'), path = require('path');
+const ROOT = __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;
+global.addEventListener = () => {};
+
+const AI = new Function(fs.readFileSync(path.join(ROOT, 'module-aiassist.js'), 'utf8') + '\nreturn WebX11AIAssistantModule;')();
+
+const stub = {
+ print: () => {}, log: () => {}, setAiBusy: () => {}, markDirty: () => {},
+ setPrompt: () => {}, clearInput: () => {}, scrollToBottom: () => {}, refreshPrompt: () => {},
+ ollamaManager: null,
+ fs: { mkdirRecursive: () => {}, writeFile: () => true, readFile: () => null, exists: () => false, list: () => [] },
+ addEventListener: () => {}, removeEventListener: () => {}
+};
+
+function mk(delay, opts = {}) {
+ const ai = new AI();
+ ai.terminal = stub;
+ ai._saveContext = () => {};
+ ai._archiveSession = () => {};
+ ai._messageCount = 0;
+ if (opts.lockTimeout) ai._chatLockTimeoutMs = opts.lockTimeout;
+ ai.agenticLoop = async (messages, o) => {
+ await new Promise(r => setTimeout(r, delay));
+ ai._recordToolCall({ name: 'read_file', turn: 1, args: {}, durationMs: delay, ok: true,
+ returnedBytes: 12000, clippedBytes: 6000, truncated: true,
+ tag: (o && o.agentTag) || 'main' });
+ return { finalAnswer: 'A-' + ((o && o.agentTag) || 'main'), toolTurns: 1 };
+ };
+ return ai;
+}
+
+let fails = 0, passes = 0;
+const ok = (n, c, d) => { if (c) { passes++; return; } fails++; console.log('FAIL ' + n + (d ? ' — ' + d : '')); };
+const msgs = () => [{ role: 'user', content: 'go' }];
+const time = async fn => { const t = Date.now(); await fn(); return Date.now() - t; };
+
+(async () => {
+ // H1: two delegated runs overlap
+ const ai = mk(300);
+ const t1 = await time(() => Promise.all([
+ ai.chat(msgs(), { agentTag: 'agent-a' }),
+ ai.chat(msgs(), { agentTag: 'agent-b' })
+ ]));
+ ok('H1 delegated runs run concurrently', t1 < 520, t1 + 'ms for 2x300ms');
+
+ // H2: the main conversation still queues (user turns must not interleave)
+ const ai2 = mk(200);
+ const t2 = await time(() => Promise.all([ai2.chat(msgs()), ai2.chat(msgs())]));
+ ok('H2 main-path turns stay serialised', t2 >= 380, t2 + 'ms for 2x200ms');
+
+ // H3: a waiter must not steal/clear a lock it does not own
+ const ai3 = mk(400, { lockTimeout: 80 });
+ const first = ai3.chat(msgs()); // holds the main lock for 400ms
+ await new Promise(r => setTimeout(r, 40));
+ let secondErr = null;
+ try { await ai3.chat(msgs()); } catch (e) { secondErr = e; }
+ ok('H3 waiter fails instead of force-releasing', !!secondErr, secondErr ? secondErr.message : 'resolved anyway');
+ await first.catch(() => {});
+ ok('H4 the owner still released cleanly afterwards', ai3._chatLock === false, String(ai3._chatLock));
+
+ // H5: tool-log entries are attributable
+ const ai4 = mk(120);
+ await Promise.all([ai4.chat(msgs(), { agentTag: 'agent-x' }), ai4.chat(msgs(), { agentTag: 'agent-y' })]);
+ const tags = ai4._toolCallLog.map(e => e.tag);
+ ok('H5 concurrent runs tag their own tool entries', tags.includes('agent-x') && tags.includes('agent-y'), JSON.stringify(tags));
+ ok('H6 per-run slice returns only that run', ai4._recentToolLog('agent-x', 5).every(e => e.tag === 'agent-x'),
+ JSON.stringify(ai4._recentToolLog('agent-x', 5).map(e => e.tag)));
+
+ // H7: an agent run must not clear a cancel that the user pressed for the main turn
+ const ai5 = mk(50);
+ ai5.cancelAi();
+ await ai5.chat(msgs(), { agentTag: 'agent-z' }).catch(() => {});
+ ok('H7 agent run leaves the user cancel flag alone', ai5._cancelled === true, String(ai5._cancelled));
+
+ console.log('test-chat-concurrency: ' + passes + ' passed, ' + fails + ' failed');
+ process.exit(fails ? 1 : 0);
+})();