fix: palace single-writer guard (T-065), 10M default cap (T-068), unstale model-picker test (T-070) — suite now 607/607

src/main/config.ts

@@ -267,10 +267,13 @@ export interface HarnessConfig {
   /** Default per-worker TOTAL-token cap (input+output+cache) applied to every
    *  god-triggered ephemeral worker; a worker's own spawn-request `tokenCap`
    *  overrides it. When the effective cap is exceeded the worker is reaped (its
-   *  committed work preserved) and god is informed. This is PLUMBING for a later
-   *  budget feature: per the human directive there is NO per-worker cap today, so
-   *  the default is 0 = UNLIMITED — the mechanism is wired but never throttles
-   *  unless someone explicitly sets a positive cap (per request or here). */
+   *  committed work preserved) and god is informed. Defaults to 10_000_000 — a
+   *  runaway worker is throttled by default. 0 = UNLIMITED remains available as
+   *  an EXPLICIT decision; a malformed value (negative/NaN/string) also resolves
+   *  to unlimited rather than to a nonsense cap (see resolveDefaultWorkerTokenCap
+   *  in src/shared/tokenCaps.ts, which every call site goes through).
+   *  NOTE: a config.json that already persists this key keeps its stored value —
+   *  changing this default only reaches installs that never set one. */
   defaultWorkerTokenCap?: number;
   /** Circuit-breaker thresholds (Lane A #6.6b). Unset = conservative defaults. */
   circuitBreaker?: CircuitBreakerConfig;
@@ -437,7 +440,13 @@ const DEFAULTS: HarnessConfig = {
   maxConcurrentWorkers: 4,
   workerIdleTimeoutMinutes: 20,
   integrations: [],
-  defaultWorkerTokenCap: 0, // 0 = unlimited (human directive: NO per-worker cap)
+  // 10M total tokens (input+output+cache) per god-triggered ephemeral worker.
+  // Human directive 2026-08-24 (Aaron): "Without a tokenCap I think 10M is a
+  // sensible default." This SUPERSEDES the earlier "NO per-worker cap" directive,
+  // which was written when a cap could only arrive on a spawn-request, so 0 meant
+  // "not throttling yet". Now an UNCAPPED worker is a decision, not an omission:
+  // set this to 0 to get unlimited back explicitly.
+  defaultWorkerTokenCap: 10_000_000,
   semanticMemory: true,
   embeddingModel: 'minilm',
   missions: [OPS_STANDUP_MISSION],

src/main/index.ts

@@ -82,6 +82,7 @@ import {
 import { buildMissingCliScript, chooseInstallRung } from './cliInstall';
 import { detectNodeVersion, nodeIsUsable, resolveNodeInstaller } from './nodeInstall';
 import { toolCatalog, type ToolStatus } from '../shared/toolCatalog';
+import { effectiveWorkerTokenCap } from '../shared/tokenCaps';
 import { listLocalSkills, loadCatalog, installSkill, uninstallSkill, type LocalSkill } from './skills';
 import { loadHero } from './hero';
 import {
@@ -4805,10 +4806,10 @@ async function ephemeralWorkerTick(): Promise<void> {
     const cfg = readConfig();
     const maxWorkers = Math.max(1, cfg.maxConcurrentWorkers ?? 4);
     const idleTimeoutMs = Math.max(1, cfg.workerIdleTimeoutMinutes ?? 20) * 60_000;
-    // Per-worker token cap. 0 = UNLIMITED (the default — wired but never throttles
-    // unless a positive cap is set per-request or via defaultWorkerTokenCap).
-    const defaultTokenCap = typeof cfg.defaultWorkerTokenCap === 'number' && cfg.defaultWorkerTokenCap > 0
-      ? cfg.defaultWorkerTokenCap : 0;
+    // Per-worker token cap. A worker's own spawn-request cap wins; otherwise the
+    // config default applies (10M unless someone set defaultWorkerTokenCap).
+    // 0 there = UNLIMITED, the deliberate escape hatch.
+    const defaultTokenCap = cfg.defaultWorkerTokenCap;
 
     // (1) Finish or reap. Each release calls teardownPty EXPLICITLY after the
     //     kill, like every other kill site: ptyManager.kill() deletes the session
@@ -4830,7 +4831,7 @@ async function ephemeralWorkerTick(): Promise<void> {
       }
       // Token-cap reap (default-off plumbing). An effective cap > 0 → reap when the
       // worker's cumulative token use exceeds it; its committed work is preserved.
-      const tokenCap = (rec.tokenCap && rec.tokenCap > 0) ? rec.tokenCap : defaultTokenCap;
+      const tokenCap = effectiveWorkerTokenCap(rec.tokenCap, defaultTokenCap);
       if (tokenCap > 0) {
         const used = workerTokensUsed(workerId);
         if (used > tokenCap) {
@@ -4935,12 +4936,11 @@ interface PreservedSnapshot {
 /** List live ephemeral workers (+ preserved worktrees awaiting GC) for the tab. */
 ipcMain.handle('workers:list', (): { live: WorkerSnapshot[]; preserved: PreservedSnapshot[]; maxWorkers: number } => {
   const cfg = readConfig();
-  const defaultCap = typeof cfg.defaultWorkerTokenCap === 'number' && cfg.defaultWorkerTokenCap > 0
-    ? cfg.defaultWorkerTokenCap : 0;
+  const defaultCap = cfg.defaultWorkerTokenCap;
   const now = Date.now();
   const live: WorkerSnapshot[] = [...liveWorkers.values()].map((rec) => {
     const idle = ptyManager.idleFor(rec.workerId);
-    const effCap = (rec.tokenCap && rec.tokenCap > 0) ? rec.tokenCap : (defaultCap > 0 ? defaultCap : 0);
+    const effCap = effectiveWorkerTokenCap(rec.tokenCap, defaultCap);
     return {
       workerId: rec.workerId,
       reqId: rec.reqId,

src/main/memory.ts

@@ -132,8 +132,13 @@ export class MemoryManager {
    *  quarantined again. A count would be useless: the reaper deletes them. */
   private lastQuarantineTs = 0;
   private initStarted = false;
-  /** True while a mineNow() pass is in flight — serializes palace writers. */
+  /** True while a mine pass is in flight — serializes palace writers. */
   private mining = false;
+  /** Wake-ups for writers QUEUED behind the current holder. mineNow drops its
+   *  turn (the next tick is 10 min away), but retainWorker cannot: returning
+   *  false defers a scratch deletion to the next sweep, and on a busy floor
+   *  "the loop is mining" is true often enough that it would never converge. */
+  private mineWaiters: Array<() => void> = [];
   /** agentId → memory.md mtimeMs at last successful mine (skip unchanged). */
   private lastMined = new Map<string, number>();
 
@@ -294,6 +299,28 @@ export class MemoryManager {
 
   // — mining (store) —
 
+  /** Take the palace's single-writer lock, WAITING for any in-flight holder.
+   *  Every path that spawns `mempalace mine` must go through here or through
+   *  `mining` directly — a writer that skips the guard makes the other one fail
+   *  with "held by another writer", and on the retain path the loser is a reaped
+   *  worker's memory. Callers that must not block check `mining` and bail first. */
+  private async acquireMineLock(): Promise<void> {
+    while (this.mining) {
+      await new Promise<void>((resolve) => { this.mineWaiters.push(resolve); });
+    }
+    this.mining = true;
+  }
+
+  /** Release the lock and wake everyone queued. Exactly one of them wins the
+   *  re-check in `acquireMineLock`; the rest queue again. MUST run in a
+   *  `finally` — a lock left held silently stops every future mine. */
+  private releaseMineLock(): void {
+    this.mining = false;
+    const waiting = this.mineWaiters;
+    this.mineWaiters = [];
+    for (const wake of waiting) wake();
+  }
+
   /** Mine every agent whose memory changed since last time, one at a time.
    *  The palace permits a single writer, so mines MUST be serialized — firing
    *  them concurrently makes all but one fail with "held by another writer".
@@ -320,7 +347,7 @@ export class MemoryManager {
         await this.mineAgent(agentDir, id); // one writer at a time
       }
     } finally {
-      this.mining = false;
+      this.releaseMineLock();
     }
     // Every pass above may have left another copy behind, and whether it did
     // decides how long we wait before the next one.
@@ -400,15 +427,33 @@ export class MemoryManager {
       // The race gives us the TIMEOUT half; the boolean gives us the FAILED-MINE
       // half. A non-zero exit is far likelier than a 30s hang, so reading the
       // return value is the part that matters most here.
+      // T-065: the mine loop and this GC-sweep path are two writers to a palace
+      // that permits ONE. Take the same lock mineNow takes — but WAIT for it
+      // rather than early-returning: a false here defers the scratch deletion,
+      // and a floor busy enough to keep the loop mining would defer it forever.
+      // The wait is inside the race, so the 30 s cap bounds lock + mine together.
+      let abandoned = false;
       const mined = await Promise.race([
-        this.mineAgent(agentDir, workerId),
+        (async () => {
+          await this.acquireMineLock();
+          try {
+            // The race already settled on the timeout; hold nothing, mine nothing.
+            if (abandoned) return false;
+            return await this.mineAgent(agentDir, workerId);
+          } finally {
+            this.releaseMineLock();
+          }
+        })(),
         new Promise<never>((_, reject) => {
           // Intentionally NOT unreffed: retainWorker runs inside the GC sweep,
           // so the event loop is already kept alive by the worker tick. Unreffing
           // here would let the loop drain in isolated unit tests before the timer
           // fires, causing a spurious "Promise still pending" cancellation.
           setTimeout(
-            () => reject(new Error(`retainWorker ${workerId}: timed out after ${timeoutMs}ms`)),
+            () => {
+              abandoned = true; // so a lock acquired after this point is released, not leaked
+              reject(new Error(`retainWorker ${workerId}: timed out after ${timeoutMs}ms`));
+            },
             timeoutMs
           );
         })

src/shared/tokenCaps.ts

@@ -1,2 +1,19 @@
 /** Highest accepted per-agent token ceiling across manifests and config IPC. */
 export const MAX_AGENT_TOKEN_CAP = 10_000_000_000;
+
+/** Config-level default worker cap, sanitised. Only a positive finite number is a
+ *  cap; ANYTHING else — 0, negative, NaN, Infinity, a string from a hand-edited
+ *  config.json — resolves to 0, which downstream means UNLIMITED (never throttle).
+ *  0 is the deliberate escape hatch: running a worker uncapped stays available as
+ *  an explicit decision. */
+export function resolveDefaultWorkerTokenCap(raw: unknown): number {
+  return typeof raw === 'number' && Number.isFinite(raw) && raw > 0 ? raw : 0;
+}
+
+/** Effective TOTAL-token cap for one ephemeral worker. A worker's own positive cap
+ *  always wins — the config default must never override a deliberate per-worker
+ *  choice — otherwise the sanitised config default applies. 0 = unlimited. */
+export function effectiveWorkerTokenCap(workerCap: unknown, configDefault: unknown): number {
+  const own = resolveDefaultWorkerTokenCap(workerCap);
+  return own > 0 ? own : resolveDefaultWorkerTokenCap(configDefault);
+}

test/memory-mine-lock-release.test.cjs

@@ -0,0 +1,183 @@
+'use strict';
+
+/**
+ * T-065 QA — the two palace single-writer paths that fail SILENTLY.
+ *
+ * memory-mine-serialization.test.cjs proves the lock SERIALIZES two writers.
+ * It never proves the lock is RELEASED when a mine goes wrong, and it never
+ * exercises more than a pair. Both gaps fail without an error:
+ *
+ * (1) RELEASE ON THROW. `mineAgent` is documented never to reject, but both
+ *     call sites nonetheless release in a `finally`. If that finally ever went
+ *     away, nothing would crash: `acquireMineLock` would simply wait forever,
+ *     and every future palace write — the background loop AND every reaped
+ *     worker's retain — would stop. The palace would quietly stop learning.
+ *
+ * (2) THE QUEUE BEYOND A PAIR. releaseMineLock's contract is that exactly ONE
+ *     woken waiter wins the re-check and "the rest queue again". With two
+ *     writers there is only ever ONE waiter parked, so the whole re-check is
+ *     vacuous: a release that wakes only the head of the queue, or an
+ *     acquireMineLock that re-checks with `if` rather than `while`, passes the
+ *     existing suite 3-of-3 while dropping or trampling every writer past the
+ *     second. Verified — both mutations are green there and red here.
+ *
+ * Both suites assert ORDER — start/end events — never merely that the calls
+ * came back. A concurrency test that only checks completion proves nothing.
+ */
+
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const os = require('node:os');
+const path = require('node:path');
+const loadTs = require('./load-ts.cjs');
+
+const { MemoryManager } = loadTs('src/main/memory.ts');
+
+/** A manager over a temp home holding one hive agent with a memory.md to mine. */
+function makeManager(t, agentId = 'worker-a') {
+  const home = fs.mkdtempSync(path.join(os.tmpdir(), 'md-lockrel-home-'));
+  const agentDir = path.join(home, 'hive', 'agents', agentId);
+  fs.mkdirSync(agentDir, { recursive: true });
+  fs.writeFileSync(path.join(agentDir, 'memory.md'), '## Notes\nSomething worth keeping.\n');
+
+  const memory = new MemoryManager(() => home, () => ({ enabled: true, model: 'minilm' }));
+  memory.bin = () => '/fake/bin/mempalace'; // never probe the real PATH
+  t.after(() => {
+    memory.stop();
+    try { fs.rmSync(home, { recursive: true, force: true }); } catch { /* already gone */ }
+  });
+  return { memory, home, agentDir, agentId };
+}
+
+/** Let every already-queued microtask AND timer callback run, so a "did it
+ *  start?" assertion cannot pass merely because the scheduler was behind. */
+const settle = () => new Promise((r) => setTimeout(r, 20));
+
+// ── (1) the lock must survive a mine that throws ────────────────────────────
+
+test('a mineNow pass whose mine THROWS still leaves the lock free', async (t) => {
+  const { memory } = makeManager(t);
+
+  memory.mineAgent = async () => { throw new Error('mempalace exploded mid-pass'); };
+
+  await assert.rejects(
+    memory.mineNow(),
+    /mempalace exploded mid-pass/,
+    'the throw must surface, not be swallowed'
+  );
+
+  // THE PROPERTY: the next writer gets straight through. A leaked lock makes
+  // mineNow early-return and retainWorker wait out its whole timeout — both
+  // silent, and between them that is every palace write on the floor.
+  const events = [];
+  memory.mineAgent = async (dir, id) => { events.push(`start:${id}`); events.push(`end:${id}`); return true; };
+  memory.lastMined.clear();
+
+  await memory.mineNow();
+  assert.deepEqual(
+    events, ['start:worker-a', 'end:worker-a'],
+    'the guard stayed held after a throwing mine — the background loop is dead'
+  );
+
+  const { agentDir } = makeManager(t, 'worker-b');
+  assert.equal(
+    await memory.retainWorker('worker-reaped', agentDir, 500), true,
+    'a waiting writer never got the lock back after a throwing mineNow pass'
+  );
+  assert.deepEqual(
+    events,
+    ['start:worker-a', 'end:worker-a', 'start:worker-reaped', 'end:worker-reaped'],
+    'the retain must have actually mined, strictly after the loop pass'
+  );
+});
+
+test('a retainWorker whose mine THROWS still leaves the lock free', async (t) => {
+  const { memory, agentDir } = makeManager(t);
+
+  memory.mineAgent = async () => { throw new Error('mempalace exploded mid-retain'); };
+
+  // retainWorker converts a throw into false — the caller keeps the scratch dir.
+  assert.equal(
+    await memory.retainWorker('worker-doomed', agentDir, 5000), false,
+    'a mine that throws must report failure so the scratch dir is preserved'
+  );
+
+  const events = [];
+  memory.mineAgent = async (dir, id) => { events.push(`start:${id}`); events.push(`end:${id}`); return true; };
+  memory.lastMined.clear();
+
+  // Both kinds of consumer must get the lock back: mineNow, which BAILS when it
+  // is held, and retainWorker, which WAITS. A leak is invisible to each of them
+  // in a different way, so assert both.
+  await memory.mineNow();
+  assert.deepEqual(
+    events, ['start:worker-a', 'end:worker-a'],
+    'the guard stayed held after a throwing retain — mineNow now bails forever'
+  );
+
+  assert.equal(
+    await memory.retainWorker('worker-next', agentDir, 500), true,
+    'the next retain waited out its timeout — the guard was never released'
+  );
+  assert.deepEqual(
+    events,
+    ['start:worker-a', 'end:worker-a', 'start:worker-next', 'end:worker-next'],
+    'the following retain must have mined, strictly after the loop pass'
+  );
+});
+
+// ── (2) the waiter queue past a pair ────────────────────────────────────────
+
+test('four queued retainWorkers each run alone, and every one of them runs', async (t) => {
+  // The existing suite only ever has ONE waiter parked, so it cannot see:
+  //   - a release that wakes just the head (waiters 2..N lost forever, each
+  //     caller timing out and refusing to delete its scratch dir), nor
+  //   - an acquire whose re-check is an `if` instead of a `while`, which lets
+  //     every woken waiter barge straight in together — two writers in the
+  //     palace at once, the exact failure this lock exists to prevent.
+  // Both are invisible with two writers and caught here. (A release that fails
+  // to CLEAR the queue is by contrast benign: the stale resolvers it re-calls
+  // are already settled, and settling twice is a no-op — so this test does not
+  // claim to catch that, and does not.)
+  const { memory, agentDir } = makeManager(t);
+
+  const events = [];
+  let live = 0;
+  memory.mineAgent = async (dir, id) => {
+    live += 1;
+    assert.equal(live, 1, `two palace writers were live at once (${id} joined an open mine)`);
+    events.push(`start:${id}`);
+    await settle();      // hold the lock open long enough for the others to queue
+    events.push(`end:${id}`);
+    live -= 1;
+    return true;
+  };
+
+  const ids = ['reap-1', 'reap-2', 'reap-3', 'reap-4'];
+  // Timeout is generous enough for four serialized mines but finite, so a LOST
+  // waiter comes back false instead of hanging the suite.
+  const results = await Promise.all(ids.map((id) => memory.retainWorker(id, agentDir, 3000)));
+
+  assert.deepEqual(
+    results, [true, true, true, true],
+    'a queued retain never got the lock — its worker memory is lost and its scratch dir stays forever'
+  );
+
+  // THE PROPERTY: strict start/end pairing. Any overlap, any writer that ran
+  // twice, and any writer that never ran shows up here.
+  assert.equal(events.length, ids.length * 2, `expected ${ids.length} serialized mines, got: ${events.join(' ')}`);
+  const ran = [];
+  for (let i = 0; i < events.length; i += 2) {
+    const [startTag, id] = events[i].split(':');
+    assert.equal(startTag, 'start', `writes interleaved: ${events.join(' ')}`);
+    assert.equal(events[i + 1], `end:${id}`, `${id}'s mine was interrupted by another writer: ${events.join(' ')}`);
+    ran.push(id);
+  }
+  assert.deepEqual([...ran].sort(), [...ids].sort(), `every queued retain must mine exactly once, got: ${ran.join(' ')}`);
+
+  // And the queue must be empty and the lock free once they have all drained.
+  memory.lastMined.clear();
+  await memory.mineNow();
+  assert.equal(events[events.length - 1], 'end:worker-a', 'the lock was not free after the queue drained');
+});