fix(server): first-turn session-busy guard, composer draft preservation, test events isolation

packages/webui/server/lib/mcode-acp.js

@@ -12,6 +12,7 @@ import {
   pushStateFor,
   pushAlert,
   getCidsByMcodeSession,
+  updateRunSid,
 } from "./state-bus.js";
 import { applyMavisUsageToCs } from "./mavis-usage.js";
 import { mcodePermissionToWebui } from "./mcode-rpc.js";
@@ -241,6 +242,16 @@ export async function runMcodeAcp(content, opts = {}) {
       } catch (e) {
         console.warn(`[webui] bindDraftToMcodeSid: ${e.message}`);
       }
+      // First-turn session-busy guard: `handleSend` claimed the run with
+      // `beginRun(cid, cs.mcodeSessionId)` BEFORE this turn existed, so on
+      // a session's first turn the claim was registered with `sid: null`
+      // and `runsBySid` never guarded the engine session — a second window
+      // could send to the same brand-new session and get a 200, then lose
+      // its prompt to the engine's "Session already has an active Turn".
+      // The turn's sid is now known: backfill the claim mid-turn (idempotent
+      // when beginRun already carried a real sid; re-points the claim when a
+      // failed session/load fell back to a fresh engine session above).
+      updateRunSid(cid, sid);
     }
     return await streamAcpPrompt(client, sid, content, label, cs, cid, attachments);
   } catch (e) {

packages/webui/server/lib/state-bus.js

@@ -650,6 +650,63 @@ export function endRun(cid) {
   if (entry.sid && runsBySid.get(entry.sid) === key) runsBySid.delete(entry.sid);
 }
 
+/**
+ * Backfill (or re-point) the engine-session claim of a live run.
+ *
+ * Why this exists: during a session's FIRST turn `handleSend` calls
+ * `beginRun(cid, cs.mcodeSessionId)` while `cs.mcodeSessionId` is still
+ * null — the engine session id only comes into existence inside
+ * `runMcodeAcp`'s `session/new`. The run was therefore registered with
+ * `sid: null`, `runsBySid` never guarded that session, and a second
+ * window (a different cid that had already learned the new session id)
+ * could send to it and get a 200 instead of a 409 session-busy — the
+ * engine then rejected the duplicate prompt and the message vanished.
+ *
+ * `runMcodeAcp` calls this the moment the turn's engine session id is
+ * determined (right next to `bindDraftToMcodeSid`, which sets
+ * `cs.mcodeSessionId`), so the registry claim lands mid-turn, not at
+ * finalize. It is also the re-point path: when a stale `session/load`
+ * fails and the turn falls back to a fresh engine session, the entry's
+ * old sid claim is released (only if this cid still owns it — the same
+ * ownership rule `endRun` applies) and the new sid is claimed.
+ *
+ * All checks and mutations are synchronous, so within Node's single
+ * thread a `beginRun` that raced here sees either the pre-backfill or
+ * the post-backfill map — never a half-updated one. A late `beginRun`
+ * carrying this sid from another cid cannot double-register: the
+ * `runsBySid.has(sid)` check inside `beginRun` runs against the same
+ * map this function just filled.
+ *
+ * @param {string} cid client whose live run should claim `sid`
+ * @param {string|null|undefined} sid the engine session id now in use
+ * @returns {boolean} true when the run's sid claim is (already) `sid`
+ */
+export function updateRunSid(cid, sid) {
+  if (!sid) return false;
+  const key = cid || "default";
+  const entry = runsByCid.get(key);
+  // No live run for this cid (endRun already released it, or beginRun
+  // was never this cid's) — nothing to backfill, and no claim may be
+  // created out of thin air.
+  if (!entry) return false;
+  // Idempotent: the run already carries this exact sid (e.g. a turn
+  // that loaded an existing session — beginRun registered it).
+  if (entry.sid === sid) return true;
+  // Never steal another cid's claim. If a different cid is registered
+  // for this sid, the guard missed it earlier; overwriting here would
+  // let `endRun` on this cid drop the OTHER cid's protection.
+  const owner = runsBySid.get(sid);
+  if (owner && owner !== key) return false;
+  // Release the stale claim this run held (load-failure fallback
+  // re-pointed the turn onto a fresh engine session).
+  if (entry.sid && runsBySid.get(entry.sid) === key) {
+    runsBySid.delete(entry.sid);
+  }
+  entry.sid = sid;
+  runsBySid.set(sid, key);
+  return true;
+}
+
 /** Live turn count, for diagnostics and tests. */
 export function activeRunCount() {
   return runsByCid.size;

packages/webui/test/lib/alerts.check.mjs

@@ -17,6 +17,15 @@ import { join } from "node:path";
 import { mkdtempSync, rmSync, readFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 
+// pushAlert audit-writes every unique alert to events.ndjson via the REAL
+// lib/events.js (static import inside alerts.js). Most cases below push
+// dozens of unique "m<i>" alerts; without this redirect each one appended
+// a junk `alert.info` line to the operator's real ~/.mcode-webui/events.ndjson.
+// Same pattern as test/routes/chat-failed-send.check.mjs — env override only,
+// production behavior untouched (events.js resolves the path lazily per append).
+const _tmpAuditDir = mkdtempSync(join(tmpdir(), "webui-alerts-check-"));
+process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpAuditDir, "events.ndjson");
+
 const absPath = (rel) =>
     pathToFileURL(join(import.meta.dirname, "..", "..", "server", rel)).href;
 
@@ -379,4 +388,11 @@ describe("ALERT_LEVELS export", () => {
     test("exposes info / warn / error", () => {
         assert.deepEqual(alerts.ALERT_LEVELS, ["info", "warn", "error"]);
     });
-});
+});
+
+// Remove the redirected audit log after the whole file has run.
+after(() => {
+    try {
+        rmSync(_tmpAuditDir, { recursive: true, force: true });
+    } catch {}
+});

packages/webui/test/lib/authorize.check.mjs

@@ -17,16 +17,29 @@
 // into a captured `_sseFrames` array, letting tests assert what was
 // emitted to the client.
 
-import { test, describe, before, beforeEach } from "node:test";
+import { test, describe, before, beforeEach, after } from "node:test";
 import assert from "node:assert/strict";
 import { Readable } from "node:stream";
-import { dirname, resolve } from "node:path";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { dirname, join, resolve } from "node:path";
 import { fileURLToPath, pathToFileURL } from "node:url";
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const SERVER_DIR = resolve(__dirname, "..", "..", "server");
 const absPath = (rel) => pathToFileURL(resolve(SERVER_DIR, rel)).href;
 
+// Audit hygiene: authorize.js audit-writes every auth.pending / auth.approve /
+// auth.reject / auth.timeout / auth.cancelled decision to events.ndjson via
+// the REAL lib/events.js (static import inside authorize.js). The dozens of
+// synthetic flows below would otherwise append junk to the operator's real
+// ~/.mcode-webui/events.ndjson on every run. Redirect to a per-run tmp file —
+// events.js resolves the path lazily per append, so the env override set
+// here covers every append this file performs (same pattern as
+// test/lib/alerts.check.mjs).
+const _tmpAuditDir = mkdtempSync(join(tmpdir(), "webui-authorize-check-"));
+process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpAuditDir, "events.ndjson");
+
 // ----- state-bus mock state (read by the registered module mock) -----
 let _sseFrames = [];
 let _subscribers = new Map(); // cid -> Set<fakeRes>
@@ -419,4 +432,11 @@ describe("pushAuthRequest / pushAuthDecision — SSE contracts", () => {
     assert.equal(f.ctx.cid, "");
     _resetForTests();
   });
+});
+
+// Remove the redirected audit log after the whole file has run.
+after(() => {
+  try {
+    rmSync(_tmpAuditDir, { recursive: true, force: true });
+  } catch {}
 });

packages/webui/test/lib/mcode-session-delete.test.js

@@ -22,6 +22,17 @@ import { pathToFileURL } from "node:url";
 
 const absPath = (rel) => pathToFileURL(join(import.meta.dirname, "..", "..", "server", rel)).href;
 
+// Audit hygiene: deleteMcodeSessionFromDb appends `session.delete` /
+// `session.delete.intent` lines to events.ndjson via the REAL lib/events.js
+// (static import inside mcode-session-delete.js). The fixture deletes below
+// (e.g. mvs_deadbeef…) would otherwise append junk to the operator's real
+// ~/.mcode-webui/events.ndjson on every run. Redirect to a per-run tmp file —
+// events.js resolves the path lazily per append, so the env override set
+// here covers every append this file performs (same pattern as
+// test/lib/alerts.check.mjs).
+const _tmpAuditDir = mkdtempSync(join(tmpdir(), "webui-session-delete-test-"));
+process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpAuditDir, "events.ndjson");
+
 // Find sqlite3 binary. On this host: C:\Users\<you>\anaconda3\Library\bin\sqlite3.exe
 // On CI: system PATH
 const SQLITE3_BIN = process.env.SQLITE3_BIN || "sqlite3";
@@ -222,3 +233,10 @@ describe("deleteMcodeSessionFromDb — table-missing case (does not throw)", { s
     assert.equal(r.log.length, 0, `log should be empty, got: ${r.log.join(",")}`);
   });
 });
+
+// Remove the redirected audit log after the whole file has run.
+after(() => {
+  try {
+    rmSync(_tmpAuditDir, { recursive: true, force: true });
+  } catch {}
+});

packages/webui/test/routes/alerts.check.mjs

@@ -12,15 +12,24 @@
 // pure. We synthesize a fake req/res with a write hook and exercise
 // the live broadcast path.
 
-import { test, describe, beforeEach } from "node:test";
+import { test, describe, beforeEach, after } from "node:test";
 import assert from "node:assert/strict";
 import { EventEmitter } from "node:events";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
 import { pathToFileURL } from "node:url";
 import { join } from "node:path";
 
 const absPath = (rel) =>
     pathToFileURL(join(import.meta.dirname, "..", "..", "server", rel)).href;
 
+// Audit hygiene: the pushAlert calls below are audit-written to events.ndjson
+// via the REAL lib/events.js (static import inside alerts.js). Redirect to a
+// per-run tmp file so the operator's real ~/.mcode-webui/events.ndjson stays
+// clean (same pattern as test/lib/alerts.check.mjs).
+const _tmpAuditDir = mkdtempSync(join(tmpdir(), "webui-routes-alerts-check-"));
+process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpAuditDir, "events.ndjson");
+
 const alertsRoute = await import(absPath("routes/alerts.js"));
 const alertsLib = await import(absPath("lib/alerts.js"));
 
@@ -133,4 +142,11 @@ describe("handleAlerts — /api/alerts", () => {
         // cleanup
         req.emit("close");
     });
+});
+
+// Remove the redirected audit log after the whole file has run.
+after(() => {
+    try {
+        rmSync(_tmpAuditDir, { recursive: true, force: true });
+    } catch {}
 });