fix: adversarial review fixes — fail-closed surface resolution, usage hygiene, wire-safe warnings

Twelve fixes from the cross-model (Codex + Claude) adversarial ship review:

- resolveEffectiveSurface holds the last successfully read default surface
  per process, so a transient config outage can't silently widen a
  NULL-surface client to the ceiling; stale never-throws comment rewritten.
- readClientOpUsage counts only success/success_with_warnings rows — denial
  and error traffic can no longer "use" its way into starter derivation or
  advisor fit findings.
- think/index.ts pushes closed warning codes (QUESTION_EMBED_FAILED /
  CALIBRATION_FETCH_FAILED / TRAJECTORY_INJECTION_FAILED) on the wire; raw
  exception text goes to stderr only (D6).
- enforceTokenBudget's minKeep failsafe slices the title too, so used <=
  budget holds unconditionally; the failsafe now stamps a distinct
  budget_truncated stage (additive vocab) while budget_dropped_all is
  reserved for genuinely-empty strict returns.
- advisor drift arm excludes localOnly ops from starter recommendations
  (mirrors derive-starter-ops).
- legacy bearer transport routes tools/call statuses through
  requestLogStatusForResult — denied_after_list / success_with_warnings
  now feed the amendment-33 metric on both HTTP transports.
- request_tools rejects {surface, tools} together as invalid_params; a
  race-lost persist (0-row UPDATE under a concurrent operator pin) refunds
  its rate-limit token (new RateLimiter.refund, capped at limit).
- health-indicators error rate: surface_change is an OPERATION value, not a
  status — audit rows now excluded from numerator AND denominator via the
  operation column.
- expansion_failed carries reason 'timeout' when the expander timed out.
- resolveStrictParamsMode holds the last-known-good DB mode so a transient
  config outage on a reject-mode server can't re-open the warn grace period
  (+ reset seam for tests).
- get_agent_job caps error_text at 2000 chars (unbounded worker field).

Regression tests: usage status filter, denied_after_list on the legacy
transport (DB-plane-pinned gate), strict-mode last-known-good, both-params
reject, limiter refund semantics, title-slice used<=budget pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/commands/serve-http.ts

@@ -1428,11 +1428,13 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
     try {
       const now = Math.floor(Date.now() / 1000);
       const [expiring] = await sql`SELECT count(*)::int as count FROM oauth_tokens WHERE token_type = 'access' AND expires_at BETWEEN ${now} AND ${now + 86400}`;
-      // Excluded from the error numerator: success, success_with_warnings (a
-      // warn-mode success), and surface_change audit rows; denied_after_list
-      // stays counted — a denied call IS a failure signal.
-      const [errors] = await sql`SELECT count(*)::int as count FROM mcp_request_log WHERE status NOT IN ('success', 'success_with_warnings', 'surface_change') AND created_at > now() - interval '24 hours'`;
-      const [total] = await sql`SELECT count(*)::int as count FROM mcp_request_log WHERE created_at > now() - interval '24 hours'`;
+      // Excluded from the error numerator: success and success_with_warnings
+      // (a warn-mode success); denied_after_list stays counted — a denied
+      // call IS a failure signal. surface_change is an OPERATION value (audit
+      // rows carry status='success'), so audit rows are excluded from BOTH
+      // counts — they are records of operator/self actions, not traffic.
+      const [errors] = await sql`SELECT count(*)::int as count FROM mcp_request_log WHERE status NOT IN ('success', 'success_with_warnings') AND operation != 'surface_change' AND created_at > now() - interval '24 hours'`;
+      const [total] = await sql`SELECT count(*)::int as count FROM mcp_request_log WHERE operation != 'surface_change' AND created_at > now() - interval '24 hours'`;
       const errorRate = (total as any).count > 0 ? ((errors as any).count / (total as any).count * 100).toFixed(1) : '0';
       res.json({
         expiring_soon: (expiring as any).count,
@@ -1968,21 +1970,26 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
    * dual-plane ONLY when the client row carries no usable surface — the
    * common full-surface path pays no extra config read. Unknown row values
    * are ignored with a warn-once per client (amendment 18). Never throws:
-   * surface resolution must not take a request down — any resolver failure
-   * falls back to the ceiling (which is exactly the pre-WP4 behavior).
+   * surface resolution must not take a request down. On a default-surface
+   * read failure the LAST successfully read default (per process) still
+   * applies, so a transient config outage cannot silently widen a client
+   * that normally resolves narrower than the ceiling; with no prior read,
+   * the ceiling is the only floor available (pre-WP4 behavior).
    */
+  let lastKnownDefaultSurface: McpSurface | null = null;
   async function resolveEffectiveSurface(authInfo: AuthInfo): Promise<{ ceiling: McpSurface; effective: McpSurface }> {
     const ceiling = clampSurface(serverSurfaceCeiling);
     // min() can never go below the narrowest surface: a 'verbs' ceiling makes
     // the row/default resolution a no-op, so skip the awaited config read.
     if (ceiling === 'verbs') return { ceiling, effective: ceiling };
+    const rowSurface = resolveClientRowSurface(authInfo.surface, authInfo.clientId);
+    if (rowSurface !== null) return { ceiling, effective: minSurface(ceiling, rowSurface) };
     try {
-      const rowSurface = resolveClientRowSurface(authInfo.surface, authInfo.clientId);
-      if (rowSurface !== null) return { ceiling, effective: minSurface(ceiling, rowSurface) };
       const dflt = await resolveDefaultClientSurface(engine, config);
+      lastKnownDefaultSurface = dflt ?? null;
       return { ceiling, effective: minSurface(ceiling, dflt ?? ceiling) };
     } catch {
-      return { ceiling, effective: ceiling };
+      return { ceiling, effective: minSurface(ceiling, lastKnownDefaultSurface ?? ceiling) };
     }
   }
 

src/core/advisor/collect-mcp-client-fit.ts

@@ -41,6 +41,7 @@
  */
 
 import { gbrainPath } from '../config.ts';
+import { operations } from '../operations.ts';
 import { readClientOpUsage, type ClientOpUsage } from '../mcp-usage.ts';
 import {
   STARTER_OPS,
@@ -201,7 +202,11 @@ export const collectMcpClientFit: AdvisorCollector = {
         .sort((a, b) => b[1] - a[1] || (a[0] < b[0] ? -1 : 1))
         .slice(0, DRIFT_TOP_N)
         .map(([op]) => op);
-      const missingFromStarter = topOps.filter((op) => !STARTER_OPS.has(op));
+      // localOnly ops are never proposable for a network surface (mirrors
+      // derive-starter-ops); a logged localOnly name (old rows, CLI-actor
+      // audit exception) must not produce an unactionable recommendation.
+      const localOnlyOps = new Set(operations.filter((o) => o.localOnly).map((o) => o.name));
+      const missingFromStarter = topOps.filter((op) => !STARTER_OPS.has(op) && !localOnlyOps.has(op));
 
       const usage90 = await readClientOpUsage(ctx.engine, { days: DRIFT_UNUSED_WINDOW_DAYS });
       const seen90 = new Set<string>();

src/core/mcp-usage.ts

@@ -14,6 +14,10 @@
  *     'tools/call:<name>' — the prefix is stripped and the row counts as <name>.
  *   - 'surface_change' audit rows (ENG-8, src/core/surface-audit.ts) are
  *     bookkeeping, not usage — dropped.
+ *   - Only successful calls count ('success' / 'success_with_warnings').
+ *     Denied and errored rows are not usage: a client repeatedly bouncing off
+ *     a gated op must not "use" its way into starter-set derivation or an
+ *     advisor fit finding (that would let denial traffic curate the catalog).
  *
  * Windowed on `created_at` so the query rides `idx_mcp_log_time_agent`
  * (created_at, token_name). Plain SQL through `engine.executeRaw` — works on
@@ -125,6 +129,7 @@ export async function readClientOpUsage(
        FROM mcp_request_log
       WHERE created_at > now() - ($1::int * interval '1 day')
         AND token_name IS NOT NULL
+        AND status IN ('success', 'success_with_warnings')
       GROUP BY token_name, operation`,
     [days],
   );

src/core/operations.ts

@@ -4014,7 +4014,9 @@ const get_agent_job: Operation = {
       created_at: iso(row.created_at),
       started_at: iso(row.started_at),
       finished_at: iso(row.finished_at),
-      error_text: row.error_text ?? null,
+      // Cap: error_text is an unbounded worker-written field (stack traces,
+      // provider dumps); a remote polling view shouldn't ship megabytes.
+      error_text: row.error_text ? row.error_text.slice(0, 2000) : null,
       result,
       ...(row.status === 'waiting' && row.queue_position !== null
         ? { queue_position: Number(row.queue_position) }
@@ -7119,6 +7121,16 @@ const request_tools: Operation = {
     // callers were never surface-bounded.
     const ceiling = ctx.surfaceCeiling ?? 'full';
 
+    // D5: the three branches are mutually exclusive. {surface, tools}
+    // together is ambiguous (persist vs descriptor fetch) — reject loudly
+    // rather than silently persisting and ignoring the tools list.
+    if (p.surface !== undefined && p.tools !== undefined) {
+      throw new OperationError(
+        'invalid_params',
+        'pass either {surface} (persist) or {tools} (descriptor fetch), not both.',
+      );
+    }
+
     // ── persist branch (D5: accepts ONLY {surface}) ─────────────────────
     if (p.surface !== undefined) {
       const requested = p.surface as string;
@@ -7191,7 +7203,12 @@ const request_tools: Operation = {
          RETURNING client_id`,
         [requested, clientId],
       );
-      if (updated.length === 0) throw operatorLocked();
+      if (updated.length === 0) {
+        // Concurrent operator pin won the race: the denial must not consume
+        // the client's persist budget (the limiter meters actual writes).
+        requestToolsPersistLimiter.refund(clientId);
+        throw operatorLocked();
+      }
       await writeSurfaceChangeAudit(ctx.engine, {
         actor: clientId,
         client_id: clientId,

src/core/search/hybrid.ts

@@ -953,13 +953,14 @@ function pushDegraded(
 }
 
 /**
- * WP2/T3 — budget-stage stamp shared by the enforceTokenBudget call sites:
- * budget_dropped_all fires exactly when the strict packer would have
- * returned [] — either it DID (kept 0 with drops, under
- * GBRAIN_SEARCH_SALVAGE=off) or the minKeep failsafe kept one truncated copy.
+ * WP2/T3 — budget-stage stamp shared by the enforceTokenBudget call sites.
+ * Two distinct stages so consumers can tell "empty" from "clipped":
+ * budget_truncated when the minKeep failsafe kept one truncated copy
+ * (results non-empty); budget_dropped_all when the strict packer returned
+ * [] (kept 0 with drops, under GBRAIN_SEARCH_SALVAGE=off).
  */
 function stampBudgetStage(list: DegradedStageEntry[], meta: TokenBudgetMeta): void {
-  if (meta.truncated) pushDegraded(list, 'budget_dropped_all', 'first_result_truncated');
+  if (meta.truncated) pushDegraded(list, 'budget_truncated', 'first_result_truncated');
   else if (meta.kept === 0 && meta.dropped > 0) pushDegraded(list, 'budget_dropped_all');
 }
 
@@ -1382,10 +1383,10 @@ export async function hybridSearch(
       if (queries.length === 0) queries = [query];
       // "Applied" = produced variants beyond the original, not just called.
       expansionApplied = queries.length > 1;
-    } catch {
+    } catch (err) {
       // Expansion failure is non-fatal — original query proceeds alone,
       // stamped so the consumer knows the multi-query recall arm was lost.
-      pushDegraded(degraded, 'expansion_failed', 'provider_error');
+      pushDegraded(degraded, 'expansion_failed', isTimeoutError(err) ? 'timeout' : 'provider_error');
     }
   }
 

src/core/search/token-budget.ts

@@ -144,8 +144,9 @@ export function packToBudget<T>(
  * exceeds the budget (packToBudget's strict [] edge), keep one result with
  * chunk_text truncated to fit — on a COPY, never mutating the shared
  * SearchResult (it flows on to cache write + eval capture). A budget below
- * even the title-only cost keeps a title-only copy (chunk_text: ''). The
- * failsafe lives HERE, not in packToBudget, because packToBudget also
+ * even the title-only cost truncates the TITLE too (chunk_text: ''), so
+ * `used <= budget` holds unconditionally — a hard cap that can be exceeded
+ * is not a cap. The failsafe lives HERE, not in packToBudget, because it also
  * feeds the frozen memory-verb paths (recall/entity/context_pack) whose
  * strict-cap contract must not drift. `GBRAIN_SEARCH_SALVAGE=off`
  * restores the strict [] behavior (ENG-7).
@@ -158,10 +159,12 @@ export function enforceTokenBudget(
   if (items.length === 0 && results.length > 0 && meta.budget > 0 && searchSalvageEnabled()) {
     const first = results[0];
     // Chars that keep resultTokens(copy) <= budget under the char/4 model:
-    // ceil(4*(budget - titleCost)/4) = budget - titleCost. Clamped at 0 so a
-    // sub-title-cost budget degrades to the title-only copy.
-    const chunkChars = Math.max(0, (meta.budget - estimateTokens(first.title)) * 4);
-    const copy: SearchResult = { ...first, chunk_text: first.chunk_text.slice(0, chunkChars) };
+    // ceil(4*(budget - titleCost)/4) = budget - titleCost. A sub-title-cost
+    // budget slices the title itself (budget*4 chars costs exactly budget
+    // tokens under ceil(len/4)), so used <= budget holds unconditionally.
+    const title = (first.title ?? '').slice(0, meta.budget * 4);
+    const chunkChars = Math.max(0, (meta.budget - estimateTokens(title)) * 4);
+    const copy: SearchResult = { ...first, title, chunk_text: first.chunk_text.slice(0, chunkChars) };
     return {
       results: [copy],
       meta: {

src/core/think/index.ts

@@ -388,7 +388,9 @@ export async function runThink(
       const e = await opts.embedQuestion(opts.question);
       if (e) questionEmbedding = e;
     } catch (e) {
-      warnings.push(`QUESTION_EMBED_FAILED: ${(e as Error).message}`);
+      // D6: code-only on the wire; raw exception text goes to server logs.
+      warnings.push('QUESTION_EMBED_FAILED');
+      process.stderr.write(`[think] question embed failed: ${e instanceof Error ? e.message : String(e)}\n`);
     }
   }
 
@@ -439,9 +441,9 @@ export async function runThink(
         warnings.push('NO_CALIBRATION_PROFILE');
       }
     } catch (err) {
-      warnings.push(
-        `CALIBRATION_FETCH_FAILED: ${err instanceof Error ? err.message : 'unknown'}`,
-      );
+      // D6: code-only on the wire; raw exception text goes to server logs.
+      warnings.push('CALIBRATION_FETCH_FAILED');
+      process.stderr.write(`[think] calibration fetch failed: ${err instanceof Error ? err.message : String(err)}\n`);
     }
   }
 
@@ -521,9 +523,9 @@ export async function runThink(
       // Defensive: trajectory injection is best-effort. Any unexpected
       // error degrades to "no trajectory block" + a warning. The think
       // call itself never fails because of trajectory wiring.
-      warnings.push(
-        `TRAJECTORY_INJECTION_FAILED: ${err instanceof Error ? err.message : 'unknown'}`,
-      );
+      // D6: code-only on the wire; raw exception text goes to server logs.
+      warnings.push('TRAJECTORY_INJECTION_FAILED');
+      process.stderr.write(`[think] trajectory injection failed: ${err instanceof Error ? err.message : String(err)}\n`);
     }
   }
   if (trajectoryPointsCount > 0) {

src/core/types.ts

@@ -1662,7 +1662,11 @@ export interface EvalCaptureFailure {
  *   vector_arm_failed  — an engine.searchVector arm threw; surviving arms
  *                        (or keyword) carried the result
  *   budget_dropped_all — the first result alone exceeded the token budget
- *                        (pre-minKeep this returned []; now 1 truncated copy)
+ *                        and NOTHING was returned (GBRAIN_SEARCH_SALVAGE=off
+ *                        strict path — the result set is empty)
+ *   budget_truncated   — the minKeep failsafe kept ONE result truncated to
+ *                        fit the budget (results non-empty but cut; distinct
+ *                        stage so consumers can tell "empty" from "clipped")
  *   keyword_zero       — the keyword arm returned zero rows on a path where
  *                        it was the primary recall arm (vector unavailable)
  *   cache_prestamp     — served from a cache row written before the
@@ -1676,6 +1680,7 @@ export const DEGRADED_STAGES = [
   'rescore_skipped',
   'vector_arm_failed',
   'budget_dropped_all',
+  'budget_truncated',
   'keyword_zero',
   'cache_prestamp',
 ] as const;

src/mcp/http-transport.ts

@@ -31,7 +31,7 @@ import { buildToolDefs } from './tool-defs.ts';
 import { operations } from '../core/operations.ts';
 import type { AuthInfo } from '../core/operations.ts';
 import { VERSION } from '../version.ts';
-import { dispatchToolCall } from './dispatch.ts';
+import { dispatchToolCall, requestLogStatusForResult } from './dispatch.ts';
 import { parseStrictParamsMode } from './validate-params.ts';
 import { filterOpsForSurface, clampSurface, type McpSurface } from './surface.ts';
 import { disabledOpsForPublishGates } from './publish-gates.ts';
@@ -467,7 +467,9 @@ export async function startHttpTransport(opts: HttpTransportOptions) {
           // IS the ceiling request_tools bounds catalog + persist by.
           surfaceCeiling: surface,
         });
-        const status = result.isError ? 'error' : 'success';
+        // Same status taxonomy as the OAuth transport (denied_after_list /
+        // success_with_warnings feed the amendment-33 metric + E4 usage).
+        const status = requestLogStatusForResult(result);
         logRequest(auth.tokenName!, `tools/call:${toolName}`, status, Date.now() - startedMs);
         return Response.json(
           { result, jsonrpc: '2.0', id },

src/mcp/rate-limit.ts

@@ -117,6 +117,19 @@ export class RateLimiter {
     }
   }
 
+  /**
+   * Return one token to a key's bucket (capped at the limit). For callers
+   * that meter an action's SUCCESS, not its attempt: check() before the
+   * action, refund() when the action turns out to be a no-op (e.g. a
+   * concurrent-lock loser whose UPDATE affected 0 rows) so denials don't
+   * consume the caller's budget. No-op for unknown keys.
+   */
+  refund(key: string): void {
+    const bucket = this.buckets.get(key);
+    if (!bucket) return;
+    bucket.tokens = Math.min(this.opts.limit, bucket.tokens + 1);
+  }
+
   /** Test helper: current key count. */
   get size(): number {
     return this.buckets.size;

src/mcp/validate-params.ts

@@ -154,22 +154,34 @@ export function parseStrictParamsMode(v: unknown): StrictParamsMode | null {
  * Dual-plane resolution for `mcp.strict_params`: DB plane (`engine.getConfig`)
  * wins, file plane (`config.mcp.strict_params`) is the fallback, absent or
  * unparseable on both = 'warn' (the grace-period default). A failed DB read
- * also falls to the file plane — validation posture must never take a tool
- * call down. Resolved once per dispatch (amendment 12); serve-http's
- * tools/list also reads it per request so `gbrain config set
+ * holds the LAST successfully resolved DB mode (per process) before falling
+ * to the file plane — a transient config outage on a reject-mode server must
+ * not silently re-open the warn-mode grace period. Validation posture never
+ * takes a tool call down. Resolved once per dispatch (amendment 12);
+ * serve-http's tools/list also reads it per request so `gbrain config set
  * mcp.strict_params reject` flips the advertised schema without a restart
  * (stdio + the legacy bearer transport resolve once at startup — restart to
  * flip there, deliberate).
  */
+let lastKnownDbStrictMode: StrictParamsMode | null = null;
+
+/** Test seam: clear the last-known-good DB mode between cases. */
+export function resetStrictParamsModeCache(): void {
+  lastKnownDbStrictMode = null;
+}
+
 export async function resolveStrictParamsMode(
   engine: BrainEngine,
   config: GBrainConfig | null | undefined,
 ): Promise<StrictParamsMode> {
   try {
     const dbMode = parseStrictParamsMode(await engine.getConfig('mcp.strict_params'));
+    lastKnownDbStrictMode = dbMode;
     if (dbMode) return dbMode;
   } catch {
-    // Engine without a config table / transient error → file plane decides.
+    // Engine without a config table / transient error: last-known-good DB
+    // mode wins; with no prior successful read, the file plane decides.
+    if (lastKnownDbStrictMode) return lastKnownDbStrictMode;
   }
   return parseStrictParamsMode(config?.mcp?.strict_params) ?? 'warn';
 }

test/http-transport.test.ts

@@ -658,6 +658,26 @@ describe('http-transport: mcp_request_log audit', () => {
     } finally { srv.stop(); }
   });
 
+  test('21b. gated-op call → denied_after_list status (amendment 33 metric parity with OAuth transport)', async () => {
+    const TOK = 'audit-tok-denied';
+    const srv = await startTest({ validTokens: new Map([[hash(TOK), { id: 'a-2', name: 'audit-denied' }]]) });
+    try {
+      // Pin the gate OFF on the DB plane (which wins over the file plane) so
+      // the developer's real ~/.gbrain/config.json can't flip this test —
+      // the call-time backstop then denies with detail config_key=…
+      (srv.engine as { getConfig?: (k: string) => Promise<string | null> }).getConfig =
+        async (key: string) => (key === 'mcp.publish_skills' ? 'false' : null);
+      const r = await fetch(`${srv.url}/mcp`, { method: 'POST', headers: { 'Authorization': `Bearer ${TOK}`, 'Content-Type': 'application/json' }, body: rpc('tools/call', { name: 'list_skills', arguments: {} }) });
+      const body = await r.json();
+      expect(body.result.isError).toBe(true);
+      expect(body.result.content[0].text).toContain('permission_denied');
+      await new Promise(res => setTimeout(res, 10));
+      const row = srv.engine.audit[srv.engine.audit.length - 1];
+      expect(row.operation).toBe('tools/call:list_skills');
+      expect(row.status).toBe('denied_after_list');
+    } finally { srv.stop(); }
+  });
+
   test('22. failed auth → audit row with null token_name + auth_failed status', async () => {
     const srv = await startTest({});
     try {

test/mcp-usage.test.ts

@@ -23,12 +23,12 @@ import {
 
 let engine: PGLiteEngine;
 
-async function seed(token: string | null, operation: string, count: number, daysAgo = 0): Promise<void> {
+async function seed(token: string | null, operation: string, count: number, daysAgo = 0, status = 'success'): Promise<void> {
   for (let i = 0; i < count; i++) {
     await engine.executeRaw(
       `INSERT INTO mcp_request_log (token_name, agent_name, operation, latency_ms, status, created_at)
-       VALUES ($1, $2, $3, 5, 'success', now() - ($4::int * interval '1 day'))`,
-      [token, token ?? 'anon', operation, daysAgo],
+       VALUES ($1, $2, $3, 5, $5, now() - ($4::int * interval '1 day'))`,
+      [token, token ?? 'anon', operation, daysAgo, status],
     );
   }
 }
@@ -62,6 +62,13 @@ beforeAll(async () => {
 
   // NULL token_name rows never attribute to a client.
   await seed(null, 'query', 1);
+
+  // client-denied: denial/error traffic must not count as usage — a client
+  // bouncing off a gated op can't "use" its way into starter derivation.
+  await seed('client-denied', 'advisor', 20, 0, 'denied_after_list');
+  await seed('client-denied', 'query', 3, 0, 'error');
+  // warn-mode successes DO count.
+  await seed('client-a', 'query', 1, 0, 'success_with_warnings');
 }, 120_000);
 
 afterAll(async () => {
@@ -100,8 +107,9 @@ describe('readClientOpUsage', () => {
     const usage = await readClientOpUsage(engine);
     const a = usage.find((u) => u.token_name === 'client-a');
     expect(a).toBeDefined();
-    expect(a!.ops).toEqual({ query: 3, search: 2 });
-    expect(a!.total_calls).toBe(5);
+    // 3 plain successes + 1 success_with_warnings (warn-mode successes count).
+    expect(a!.ops).toEqual({ query: 4, search: 2 });
+    expect(a!.total_calls).toBe(6);
     expect(a!.distinct_ops).toEqual(['query', 'search']);
     expect(a!.likely_automation).toBe(false);
     expect(new Date(a!.last_seen).getTime()).toBeGreaterThan(0);
@@ -125,6 +133,12 @@ describe('readClientOpUsage', () => {
     expect(c.likely_automation).toBe(true);
   });
 
+  test('denied and errored rows are not usage — only success statuses count', async () => {
+    const usage = await readClientOpUsage(engine);
+    // client-denied has ONLY denied_after_list + error rows → no usage entry.
+    expect(usage.find((u) => u.token_name === 'client-denied')).toBeUndefined();
+  });
+
   test('window excludes old rows by default; wider window includes them', async () => {
     const usage30 = await readClientOpUsage(engine, { days: MCP_USAGE_DEFAULT_WINDOW_DAYS });
     expect(usage30.find((u) => u.token_name === 'client-old')).toBeUndefined();

test/request-tools.test.ts

@@ -276,6 +276,43 @@ describe('request_tools {surface} persist branch', () => {
     expect(parsed(other).persisted).toBe(true);
   });
 
+  test('{surface, tools} together → invalid_params, never a persist (D5 branch exclusivity)', async () => {
+    await seedClient('cl-both');
+    const res = await dispatchToolCall(engine, 'request_tools', { surface: 'starter', tools: ['query'] }, {
+      ...HTTP, surfaceCeiling: 'full', auth: authFor('cl-both', ['read']),
+    });
+    expect(res.isError).toBe(true);
+    const body = parsed(res);
+    expect(body.error).toBe('invalid_params');
+    expect(body.message).toContain('not both');
+    const rows = await engine.executeRaw(
+      `SELECT surface FROM oauth_clients WHERE client_id = $1`, ['cl-both'],
+    );
+    expect(rows[0].surface).toBe(null);
+  });
+
+  test('RateLimiter.refund returns a token so a race-lost persist never burns budget (adversarial fix)', async () => {
+    // The handler refunds when the atomic UPDATE affects 0 rows (a concurrent
+    // operator pin between SELECT and UPDATE); pin the primitive it leans on.
+    const { RateLimiter } = await import('../src/mcp/rate-limit.ts');
+    const now = 0;
+    const lim = new RateLimiter({ limit: 2, windowMs: 60_000, lruCap: 10 }, () => now);
+    expect(lim.check('k').allowed).toBe(true);
+    expect(lim.check('k').allowed).toBe(true);
+    expect(lim.check('k').allowed).toBe(false); // exhausted
+    lim.refund('k');
+    expect(lim.check('k').allowed).toBe(true); // refund restored exactly one token
+    expect(lim.check('k').allowed).toBe(false);
+    lim.refund('unknown-key'); // no-op, never throws
+    // Refund never overfills past the limit.
+    const lim2 = new RateLimiter({ limit: 1, windowMs: 60_000, lruCap: 10 }, () => now);
+    expect(lim2.check('k').allowed).toBe(true);
+    lim2.refund('k');
+    lim2.refund('k');
+    expect(lim2.check('k').allowed).toBe(true);
+    expect(lim2.check('k').allowed).toBe(false);
+  });
+
   test('invalid surface value → invalid_params naming the valid set (D10: never a persist)', async () => {
     await seedClient('cl-garbage');
     const res = await dispatchToolCall(engine, 'request_tools', { surface: 'garbage' }, {

test/token-budget.test.ts

@@ -149,16 +149,18 @@ describe('enforceTokenBudget', () => {
     expect(kept[0].chunk_text.length).toBeLessThan(big.chunk_text.length);
   });
 
-  test('budget below title-only cost \u2192 keeps a title-only copy', () => {
+  test('budget below title-only cost \u2192 slices the title so used <= budget', () => {
     // Title alone costs 25 tokens (100 chars); budget 5 can't even fit it.
+    // A hard cap that can be exceeded is not a cap: the title is sliced to
+    // budget*4 chars (costs exactly `budget` under ceil(len/4)).
     const big = makeResult({ slug: 'big', title: 'T'.repeat(100), chunk_text: 'x'.repeat(1000) });
     const { results: kept, meta } = enforceTokenBudget([big], 5);
     expect(kept).toHaveLength(1);
     expect(kept[0].chunk_text).toBe('');
-    expect(kept[0].title).toBe('T'.repeat(100));
+    expect(kept[0].title).toBe('T'.repeat(20));
     expect(meta.truncated).toBe(true);
-    // `used` reports the honest title-only cost even though it exceeds budget.
-    expect(meta.used).toBe(25);
+    expect(meta.used).toBe(5);
+    expect(meta.used).toBeLessThanOrEqual(meta.budget);
   });
 
   test('truncated flag absent on ordinary cuts and no-op passes', () => {

test/validate-params.test.ts

@@ -16,7 +16,7 @@
  *      candidate set).
  */
 
-import { describe, expect, test } from 'bun:test';
+import { beforeEach, describe, expect, test } from 'bun:test';
 import { dispatchToolCall } from '../src/mcp/dispatch.ts';
 import {
   normalizeOptionalParams,
@@ -24,6 +24,7 @@ import {
   findUnknownParams,
   buildUnknownParamWarnBlock,
   resolveStrictParamsMode,
+  resetStrictParamsModeCache,
   parseStrictParamsMode,
   UNKNOWN_PARAM_ALLOWLIST,
 } from '../src/mcp/validate-params.ts';
@@ -319,6 +320,20 @@ describe('resolveStrictParamsMode (DB > file > warn)', () => {
     getConfig: async () => { throw new Error('no config table'); },
   } as unknown as BrainEngine;
 
+  // The resolver holds the last successfully read DB mode per process
+  // (adversarial fix: a transient outage must not re-open warn mode) —
+  // isolate each case from its neighbors.
+  beforeEach(() => resetStrictParamsModeCache());
+
+  test('a failed DB read holds the last-known-good DB mode (reject survives an outage)', async () => {
+    expect(await resolveStrictParamsMode(engineWith('reject'), null)).toBe('reject');
+    // DB now unreadable + file plane says warn → cached reject still wins.
+    expect(await resolveStrictParamsMode(throwingEngine, { engine: 'pglite', mcp: { strict_params: 'warn' } } as never)).toBe('reject');
+    // A later successful read of "unset" clears the cache → file plane decides.
+    expect(await resolveStrictParamsMode(engineWith(null), { engine: 'pglite', mcp: { strict_params: 'warn' } } as never)).toBe('warn');
+    expect(await resolveStrictParamsMode(throwingEngine, null)).toBe('warn');
+  });
+
   test('DB plane wins over the file plane', async () => {
     expect(await resolveStrictParamsMode(engineWith('reject'), { engine: 'pglite', mcp: { strict_params: 'warn' } } as never)).toBe('reject');
   });