fix(review): resolve findings from /review-reviews battery (v3.8.21 hardening) (#3618)

Pre-release hardening from the /review-reviews battery — 15 findings resolved (L1-L13,L15) + L14 live-verified WONTFIX, convergence re-review clean. lint/typecheck:core/test:vitest(146)/build green; zero new test:unit failures vs baseline 797de433f.
2026-08-08 00:02:20 +03:00 · 2026-06-11 02:40:33 -03:00
parent 797de433f2
commit c2d90229ca
23 changed files with 815 additions and 67 deletions
--- a/.npmignore
+++ b/.npmignore
@@ -9,6 +9,16 @@ app/vscode-extension/
 **/db.json

 # Source code (pre-built app/ is published instead)
+#
+# NOTE (#3578 / #3821-review): package.json "files" is the source of truth for what
+# ships. It now allowlists the backend source closure the MCP server needs at runtime
+# (open-sse/, src/lib, src/server, ...) and OVERRIDES the broad src/ + open-sse/ excludes
+# below — npm honors files[] over .npmignore for inclusion. These lines are kept only as
+# intent/back-stop: if files[] is ever trimmed back to specific paths, they must NOT be
+# allowed to re-hide the MCP closure (that would silently reintroduce the --mcp
+# ERR_MODULE_NOT_FOUND #3578 fixed). The closure gate in
+# tests/unit/mcp-published-files-closure-3578.test.ts asserts the real `npm pack` output
+# in both directions (closure present + zero test files), catching such a regression.
 src/
 open-sse/
 docs/
@@ -18,6 +28,17 @@ images/
 logs/
 scripts/

+# Co-located tests must never ship even when their parent dir is allowlisted by files[].
+# (Primary guard is the "!**/*.test.*" negations in package.json files[]; this is defense
+# in depth for any nested dir the allowlist pulls in.)
+**/__tests__/
+**/*.test.ts
+**/*.test.tsx
+**/*.test.js
+**/*.test.mjs
+**/*.spec.ts
+**/*.spec.tsx
+
 # Config/dev files
 *.md
 !README.md
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@

 - **refactor(chatCore):** extract the chatCore request phases — idempotency check, semantic cache check, common request sanitization, and memory/skills injection — into dedicated `open-sse/handlers/chatCore/` modules (`idempotency.ts`, `semanticCache.ts`, `sanitization.ts`, `memorySkillsInjection.ts`), slimming the monolithic handler with no behavior change. (Maintainer follow-up: re-derive `idempotencyKey` at the Phase 9.2 save site after the check moved into the module, fixing a `ReferenceError` on successful non-cached responses.) ([#3598](https://github.com/diegosouzapw/OmniRoute/pull/3598) — thanks @oyi77)
 - **docs(opencode-provider):** soft-deprecate `@omniroute/opencode-provider` in favour of `@omniroute/opencode-plugin`. The provider package writes a **static** model list to `opencode.json` that drifts behind the live OmniRoute catalog, whereas the plugin fetches `/v1/models` at OpenCode startup. The package keeps working (no code/behavior change), but its npm description and README now carry a deprecation banner with the one-line migration, and a guard test pins the notice. ([#3419](https://github.com/diegosouzapw/OmniRoute/issues/3419) — thanks @herjarsa)
+- **chore(review):** pre-release hardening from a multi-reviewer `/review-reviews` battery over the v3.8.21 diff (7 Opus reviewers; zero blocker/high). Resolved findings: npm tarball no longer ships co-located test files (`files[]` negations + reconciled `.npmignore`; the #3578 closure gate now asserts the real `npm pack` output in both directions); `getSanitizedCachedProviderLimitsMap` scopes its connection scan to antigravity/agy instead of decrypting every active connection on each dashboard poll; the Antigravity quota-tier remap (`toClientAntigravityQuotaModelId`) is centralized in `antigravityModelAliases.ts` (was an inline if-ladder in `usage.ts`); the chatCore idempotency check returns its resolved key so the save site reuses a single derivation; and new tests pin the chatCore extracted modules, the Antigravity `usage_history` fallback contract, the reasoning-wrapper prefix-preservation heuristic, the Antigravity SSE `markdown` branch, and the upstream-ca/test no-persist guarantee. (Live-verified that agy consumer tokens are accepted by the non-daily `cloudcode-pa` host used by `retrieveUserQuota`, so #3604 is not agy-host-limited.)

 ### 🔧 Bug Fixes

@@ -28,6 +29,8 @@
 - **fix(antigravity):** the Antigravity/agy Gemini 3.5 Flash catalog now exposes clean public tier IDs (`gemini-3.5-flash-low`/`-medium`/`-high`, matching Antigravity 2.0.4's Low/Medium/High selector) and maps them to the live upstream IDs at the executor boundary, instead of the old confusing `-preview`/`-agent` names. Antigravity model-id normalization moved out of the global model resolver into the executor so client-visible IDs are no longer rewritten before account/credential routing and logging. (Maintainer follow-up: kept `gemini-3.5-flash-preview` as a hidden backward-compat alias routing to the High tier so saved combos/configs keep working; live-validated the tier set via the `agy` CLI catalog.) ([#3603](https://github.com/diegosouzapw/OmniRoute/pull/3603) — thanks @dhaern)
 - **fix(usage):** Antigravity/agy Provider Limits now report accurate consumption — `retrieveUserQuota` (live usage) is preferred over the `fetchAvailableModels` catalog view (which keeps reporting full buckets after real usage), with a local `usage_history` fallback for buckets that are only catalog-visible; cached entries are sanitized so retired upstream IDs are not re-exposed, and a deduplicated post-usage refresh keeps the dashboard fresh after each request. (Maintainer follow-up: the post-usage refresh is decoupled through a lightweight `usageEvents` bus so `usageHistory` no longer imports `providerLimits`/the executors graph, keeping the `typecheck:core` surface stable.) ([#3604](https://github.com/diegosouzapw/OmniRoute/pull/3604) — thanks @dhaern)
 - **fix(gemini):** textual reasoning wrappers emitted as assistant prose (`<think>`/`<thinking>`/`<thought>`/`<internal_thought>`, including malformed/open tags like `<thought\n…` before a tool call) are now routed to `reasoning_content` instead of leaking into visible `content`, in both the non-streaming sanitizer and the Gemini streaming translator (with split-chunk buffering so a tag fragmented across SSE chunks stays hidden). Structured tool calls and the existing textual tool-call conversion are preserved. ([#3605](https://github.com/diegosouzapw/OmniRoute/pull/3605) — thanks @dhaern)
+- **fix(gemini):** a signed native `functionCall` arriving while a textual reasoning wrapper opened in an earlier streaming chunk is still buffered now flushes that buffered reasoning to `reasoning_content` before the tool call, instead of silently discarding it. (Pre-release `/review-reviews` finding.)
+- **fix(api):** `/v1/completions` now drops a stale upstream `content-length` on the SSE branch too (the JSON branch already did) — re-serialization changes the byte length, so a buffered SSE body could otherwise advertise the pre-rewrite length and truncate/hang the client. (Pre-release `/review-reviews` finding.)

 ---

--- a/open-sse/config/antigravityModelAliases.ts
+++ b/open-sse/config/antigravityModelAliases.ts
@@ -206,6 +206,43 @@ export function toClientAntigravityModelId(modelId: string): string {
  return ANTIGRAVITY_REVERSE_MODEL_ALIASES[modelId] || modelId;
 }

+// Quota buckets reported by the Antigravity backend are keyed by UPSTREAM model ids — a
+// DIFFERENT namespace from the public/client catalog. In that upstream quota namespace
+// `gemini-3.5-flash-low` denotes the *Medium* tier's bucket (it is the upstream target of
+// the `gemini-3.5-flash-medium` forward alias), even though the same literal is also a
+// public "Low" client id. This remap therefore CANNOT be derived from
+// ANTIGRAVITY_REVERSE_MODEL_ALIASES (which has no `gemini-3.5-flash-low` entry precisely
+// because it is already a valid client id) — it encodes the upstream-bucket → client-tier
+// chain explicitly. Keep it the inverse of the `-low/-medium/-high` rows in
+// ANTIGRAVITY_MODEL_ALIASES above. (#3821-review LEDGER-5 — was duplicated as an inline
+// if-ladder in open-sse/services/usage.ts.)
+const ANTIGRAVITY_QUOTA_BUCKET_TO_CLIENT: AntigravityModelAliasMap = Object.freeze({
+  "gemini-3.5-flash-extra-low": "gemini-3.5-flash-low",
+  "gemini-3.5-flash-low": "gemini-3.5-flash-medium",
+  "gemini-3-flash-agent": "gemini-3.5-flash-high",
+});
+
+// Retired/hidden upstream preview buckets that must be dropped from client-facing usage.
+const ANTIGRAVITY_DROPPED_QUOTA_BUCKETS = new Set<string>([
+  "gemini-3.5-flash-preview",
+  "gemini-3-flash-preview",
+]);
+
+/**
+ * Map an UPSTREAM Antigravity quota-bucket model id to the client-visible tier id used in
+ * usage responses, or `null` if the bucket should be hidden from clients. Operates on the
+ * upstream quota namespace (see ANTIGRAVITY_QUOTA_BUCKET_TO_CLIENT) — do NOT pass client
+ * ids here. Single source of truth shared by the usage service and the provider-limits
+ * cache sanitizer.
+ */
+export function toClientAntigravityQuotaModelId(modelId: string): string | null {
+  if (!modelId) return null;
+  if (ANTIGRAVITY_DROPPED_QUOTA_BUCKETS.has(modelId)) return null;
+  const tierClientId = ANTIGRAVITY_QUOTA_BUCKET_TO_CLIENT[modelId];
+  if (tierClientId) return tierClientId;
+  return toClientAntigravityModelId(modelId);
+}
+
 export function getClientVisibleAntigravityModelName(
  modelId: string,
  fallbackName?: string
--- a/open-sse/executors/antigravity.ts
+++ b/open-sse/executors/antigravity.ts
@@ -328,7 +328,11 @@ function markCreditsExhausted(accountId: string): void {
  creditsExhaustedUntil.set(accountId, Date.now() + CREDITS_EXHAUSTED_TTL_MS);
 }

-function processAntigravitySSEPayload(
+/**
+ * Accumulate one Antigravity SSE `data:` payload into `collected`. Exported for unit
+ * tests (the markdown / candidate-parts extraction branches). @internal
+ */
+export function processAntigravitySSEPayload(
  payload: string,
  collected: AntigravityCollectedStream,
  log?: { debug?: (scope: string, message: string) => void }
--- a/open-sse/handlers/chatCore.ts
+++ b/open-sse/handlers/chatCore.ts
@@ -180,7 +180,7 @@ import {
  isCacheableForRead,
  isCacheableForWrite,
 } from "@/lib/semanticCache";
-import { getIdempotencyKey, saveIdempotency } from "@/lib/idempotencyLayer";
+import { saveIdempotency } from "@/lib/idempotencyLayer";
 import { createProgressTransform, wantsProgress } from "../utils/progressTracker.ts";
 import { createPiiSseTransform } from "@/lib/streamingPiiTransform";
 import { isFeatureFlagEnabled } from "@/shared/utils/featureFlags";
@@ -1867,7 +1867,9 @@ export async function handleChatCore({
  };

  // ── Phase 9.2: Idempotency check ──
-  const idempotencyHit = await checkIdempotencyCache({
+  // Resolve the idempotency key once here and reuse it at the Phase 9.2 save site below,
+  // rather than re-deriving it. (#3821-review LEDGER-6)
+  const { hit: idempotencyHit, idempotencyKey } = await checkIdempotencyCache({
    clientRawRequest,
    provider,
    model,
@@ -5232,10 +5234,8 @@ export async function handleChatCore({
    }

    // ── Phase 9.2: Save for idempotency ──
-    // The idempotency *check* moved into checkIdempotencyCache() during the
-    // chatCore modularization (#3598); re-derive the key here for the save path.
-    // getIdempotencyKey is pure (reads idempotency-key/x-request-id headers).
-    const idempotencyKey = getIdempotencyKey(clientRawRequest?.headers);
+    // Reuse the key resolved by checkIdempotencyCache() above (single derivation per
+    // request). (#3821-review LEDGER-6)
    saveIdempotency(idempotencyKey, translatedResponse, 200);
    reqLogger.logConvertedResponse(translatedResponse);
    persistAttemptLogs({
--- a/open-sse/handlers/chatCore/idempotency.ts
+++ b/open-sse/handlers/chatCore/idempotency.ts
@@ -2,6 +2,12 @@ import { getIdempotencyKey, checkIdempotency } from "@/lib/idempotencyLayer";
 import { calculateCost } from "@/lib/usage/costCalculator";
 import { buildOmniRouteResponseMetaHeaders } from "@/domain/omnirouteResponseMeta";

+/**
+ * Resolve the request's idempotency key once and check the idempotency store. Returns the
+ * resolved `idempotencyKey` alongside the cache `hit` so the caller can reuse the SAME key
+ * for the later save path instead of re-deriving it — eliminating the dual-derivation that
+ * the chatCore modularization (#3598) introduced. (#3821-review LEDGER-6)
+ */
 export async function checkIdempotencyCache({
  clientRawRequest,
  provider,
@@ -16,7 +22,7 @@ export async function checkIdempotencyCache({
  effectiveServiceTier: unknown;
  startTime: number;
  log: unknown;
-}) {
+}): Promise<{ hit: { success: true; response: Response } | null; idempotencyKey: string }> {
  const idempotencyKey = getIdempotencyKey(clientRawRequest?.headers);
  const cachedIdemp = checkIdempotency(idempotencyKey);
  if (cachedIdemp) {
@@ -33,23 +39,26 @@ export async function checkIdempotencyCache({
        })
      : 0;
    return {
-      success: true,
-      response: new Response(JSON.stringify(cachedIdemp.response), {
-        status: cachedIdemp.status,
-        headers: {
-          "Content-Type": "application/json",
-          "X-OmniRoute-Idempotent": "true",
-          ...buildOmniRouteResponseMetaHeaders({
-            provider,
-            model,
-            cacheHit: false,
-            latencyMs: Date.now() - startTime,
-            usage: idempotentUsage,
-            costUsd: idempotentCost,
-          }),
-        },
-      }),
+      idempotencyKey,
+      hit: {
+        success: true,
+        response: new Response(JSON.stringify(cachedIdemp.response), {
+          status: cachedIdemp.status,
+          headers: {
+            "Content-Type": "application/json",
+            "X-OmniRoute-Idempotent": "true",
+            ...buildOmniRouteResponseMetaHeaders({
+              provider,
+              model,
+              cacheHit: false,
+              latencyMs: Date.now() - startTime,
+              usage: idempotentUsage,
+              costUsd: idempotentCost,
+            }),
+          },
+        }),
+      },
    };
  }
-  return null;
+  return { hit: null, idempotencyKey };
 }
--- a/open-sse/handlers/chatCore/semanticCache.ts
+++ b/open-sse/handlers/chatCore/semanticCache.ts
@@ -88,4 +88,4 @@ export async function checkSemanticCache({
    }
  }
  return null;
-}
+}
--- a/open-sse/services/usage.ts
+++ b/open-sse/services/usage.ts
@@ -9,7 +9,7 @@ import {
 } from "../config/antigravityUpstream.ts";
 import {
  isUserCallableAntigravityModelId,
-  toClientAntigravityModelId,
+  toClientAntigravityQuotaModelId,
 } from "../config/antigravityModelAliases.ts";
 import { isUserCallableAgyModelId } from "../config/agyModels.ts";
 import { getGlmQuotaUrl } from "../config/glmProvider.ts";
@@ -1974,26 +1974,9 @@ interface AntigravityUsageOptions {
 const ANTIGRAVITY_LOCAL_USAGE_WINDOW_MS = 5 * 60 * 60 * 1000;
 const ANTIGRAVITY_LOCAL_USAGE_TOKENS_PER_UNIT = 1000;

-const ANTIGRAVITY_QUOTA_MODEL_ALIASES: Record<string, string | null> = {
-  "gemini-3.5-flash-preview": null,
-  "gemini-3-flash-preview": null,
-};
-
-function normalizeAntigravityQuotaModelId(modelId: string): string | null {
-  if (!modelId) return null;
-  return Object.prototype.hasOwnProperty.call(ANTIGRAVITY_QUOTA_MODEL_ALIASES, modelId)
-    ? ANTIGRAVITY_QUOTA_MODEL_ALIASES[modelId]
-    : modelId;
-}
-
-function toClientAntigravityQuotaModelId(modelId: string): string | null {
-  if (!modelId) return null;
-  if (normalizeAntigravityQuotaModelId(modelId) === null) return null;
-  if (modelId === "gemini-3.5-flash-extra-low") return "gemini-3.5-flash-low";
-  if (modelId === "gemini-3.5-flash-low") return "gemini-3.5-flash-medium";
-  if (modelId === "gemini-3-flash-agent") return "gemini-3.5-flash-high";
-  return toClientAntigravityModelId(modelId);
-}
+// `toClientAntigravityQuotaModelId` was an inline if-ladder here; it is now the single
+// source of truth in open-sse/config/antigravityModelAliases.ts (imported above), shared
+// with the provider-limits cache sanitizer. (#3821-review LEDGER-5)

 function getAntigravityLocalUsageUnits(
  provider: "antigravity" | "agy",
--- a/open-sse/translator/response/gemini-to-openai.ts
+++ b/open-sse/translator/response/gemini-to-openai.ts
@@ -387,9 +387,16 @@ export function geminiToOpenAIResponse(chunk, state) {
        }

        if (hasFunctionCall) {
-          state.activeTextualReasoningTag = undefined;
+          // Flush any still-open textual reasoning wrapper as reasoning_content BEFORE
+          // the tool call. A signed native functionCall arriving while a `<thinking>`
+          // (etc.) tag opened in an earlier chunk is still buffered must not silently
+          // drop that buffered reasoning — flushOpenTextualReasoning emits it and clears
+          // the active-tag/content buffers. (LEDGER-4 / #3821-review)
+          flushOpenTextualReasoning(state, results);
+          // Also drop any partial open-tag fragment buffered at a chunk boundary
+          // (flushOpenTextualReasoning early-returns when only this is set), matching the
+          // pre-fix branch which cleared all three buffers. (#3821-review convergence)
          state.textualReasoningTagBuffer = undefined;
-          state.textualReasoningContentBuffer = undefined;
          emitFunctionCallPart(part, state, results);
        }
        continue;
--- a/package.json
+++ b/package.json
@@ -31,7 +31,14 @@
    "scripts/build/native-binary-compat.mjs",
    "scripts/build/build-next-isolated.mjs",
    "README.md",
-    "LICENSE"
+    "LICENSE",
+    "!**/__tests__/**",
+    "!**/*.test.ts",
+    "!**/*.test.tsx",
+    "!**/*.test.js",
+    "!**/*.test.mjs",
+    "!**/*.spec.ts",
+    "!**/*.spec.tsx"
  ],
  "workspaces": [
    "open-sse"
--- a/src/app/api/v1/completions/textCompletionTransform.ts
+++ b/src/app/api/v1/completions/textCompletionTransform.ts
@@ -101,9 +101,14 @@ export async function asTextCompletionResponse(res: Response): Promise<Response>
  const contentType = res.headers.get("content-type") || "";

  if (contentType.includes("text/event-stream") && res.body) {
+    // Re-serialization changes the byte length, so drop any upstream content-length
+    // (a buffered SSE body could otherwise advertise a stale length and truncate/hang
+    // the client), mirroring the JSON branch below. (#3821-review LEDGER-8)
+    const headers = new Headers(res.headers);
+    headers.delete("content-length");
    return new Response(res.body.pipeThrough(createTextCompletionStreamTransformer()), {
      status: res.status,
-      headers: res.headers,
+      headers,
    });
  }

--- a/src/lib/usage/providerLimits.ts
+++ b/src/lib/usage/providerLimits.ts
@@ -604,10 +604,34 @@ export async function getSanitizedCachedProviderLimitsMap(): Promise<
  Record<string, ProviderLimitsCacheEntry>
 > {
  const caches = getAllProviderLimitsCache();
-  const connections = (await getProviderConnections({
-    isActive: true,
-  })) as unknown as ProviderConnectionLike[];
-  const byId = new Map(connections.map((conn) => [conn.id, conn]));
+  // Sanitization only rewrites Antigravity/agy quota keys; every other provider's cache
+  // entry is returned untouched (see sanitizeProviderLimitsCacheForConnection). The
+  // dashboard polls this on an auto-refresh interval, so avoid the unconditional
+  // `SELECT * FROM provider_connections` + per-row credential decryption that the
+  // previous implementation paid on every poll: skip the scan entirely when nothing is
+  // cached, and otherwise fetch ONLY the Antigravity/agy connections. For any other
+  // provider, byId.get(id) is undefined and the entry is returned verbatim — identical
+  // output to scanning every active connection, but without decrypting unrelated keys.
+  // (LEDGER-2 / #3821-review)
+  const connectionIds = Object.keys(caches);
+  if (connectionIds.length === 0) return {};
+
+  const sanitizableConnections = [
+    ...((await getProviderConnections({
+      isActive: true,
+      provider: "antigravity",
+    })) as unknown as ProviderConnectionLike[]),
+    ...((await getProviderConnections({
+      isActive: true,
+      provider: "agy",
+    })) as unknown as ProviderConnectionLike[]),
+  ];
+  if (sanitizableConnections.length === 0) {
+    // No connection can change the cache → return the raw entries unchanged.
+    return { ...caches };
+  }
+
+  const byId = new Map(sanitizableConnections.map((conn) => [conn.id, conn]));
  const sanitized: Record<string, ProviderLimitsCacheEntry> = {};
  for (const [connectionId, entry] of Object.entries(caches)) {
    sanitized[connectionId] =
--- a/src/lib/usage/usageHistory.ts
+++ b/src/lib/usage/usageHistory.ts
@@ -75,7 +75,6 @@ function toNumber(value: unknown): number {
  return 0;
 }

-
 function percentile(sortedValues: number[], p: number): number {
  if (sortedValues.length === 0) return 0;
  if (sortedValues.length === 1) return sortedValues[0];
--- a/tests/unit/antigravity-local-usage-fallback-3821.test.ts
+++ b/tests/unit/antigravity-local-usage-fallback-3821.test.ts
@@ -0,0 +1,119 @@
+/**
+ * LEDGER-3 (#3821-review) — the Antigravity local-usage fallback (#3604) replaces a
+ * stale full `fetchAvailableModels` bucket (used=0) with real consumption summed from
+ * `usage_history`, flipping quotaSource to "localUsageHistory". Every prior #3604 test
+ * mocks only the HTTP layer, so the model-id match against usage_history.model was never
+ * exercised end-to-end. This seeds a real usage_history row keyed by the CLIENT tier id
+ * the fallback queries and asserts the flip — the regression guard for the id contract.
+ *
+ * Contract note: the fallback queries `usage_history WHERE model = <client tier id>`
+ * (e.g. gemini-3.5-flash-high), so the executor MUST log usage under that same client id
+ * for the fallback to fire. This test pins exactly that join.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ag-local-usage-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.API_KEY_SECRET = "test-ag-local-usage-secret";
+
+const core = await import("../../src/lib/db/core.ts");
+// Load usage.ts up-front (its index.ts proxyFetch patch runs at module eval) before mocks.
+const usageModule = await import("../../open-sse/services/usage.ts");
+const { getUsageForProvider } = usageModule;
+
+const originalFetch = globalThis.fetch;
+
+test.after(() => {
+  globalThis.fetch = originalFetch;
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+test("Antigravity fetchAvailableModels(used=0) → localUsageHistory when usage_history has rows", async () => {
+  core.resetDbInstance();
+
+  // resetTime an hour out → the 5h local-usage window is [now-4h, now+1h).
+  const resetTime = new Date(Date.now() + 60 * 60 * 1000).toISOString();
+  const seededTimestamp = new Date(Date.now() - 30 * 60 * 1000).toISOString(); // within window
+
+  // Seed a usage_history row keyed by the CLIENT tier id the fallback queries.
+  const db = core.getDbInstance() as unknown as { prepare: (sql: string) => { run: (...a: unknown[]) => unknown } };
+  db.prepare(
+    `INSERT INTO usage_history (provider, model, connection_id, tokens_input, tokens_output, tokens_reasoning, success, timestamp)
+     VALUES (?, ?, ?, ?, ?, ?, 1, ?)`
+  ).run("antigravity", "gemini-3.5-flash-high", "conn-local-1", 1000, 1500, 500, seededTimestamp);
+  // Total seeded tokens = 3000 → ceil(3000/1000) = 3 units used.
+
+  globalThis.fetch = (async (input: any) => {
+    const url = typeof input === "string" ? input : input?.url || "";
+    // retrieveUserQuota (the live signal) is unavailable → falls back to fetchAvailableModels.
+    if (url.includes("retrieveUserQuota")) {
+      return { ok: false, status: 404, json: async () => ({}) } as Response;
+    }
+    // fetchAvailableModels returns a FULL (stale) bucket: remainingFraction 1.0 + resetTime.
+    return {
+      ok: true,
+      json: async () => ({
+        models: {
+          "gemini-3.5-flash-high": {
+            quotaInfo: { remainingFraction: 1.0, resetTime },
+          },
+        },
+      }),
+    } as Response;
+  }) as typeof fetch;
+
+  const connection = {
+    id: "conn-local-1",
+    provider: "antigravity",
+    accessToken: "fake-token-local-usage-unique",
+    providerSpecificData: {},
+    projectId: undefined,
+  };
+
+  const result = await getUsageForProvider(connection, { forceRefresh: true });
+  assert.ok(result && "quotas" in result, "should return quotas");
+  const quota = (result as any).quotas["gemini-3.5-flash-high"];
+  assert.ok(quota, "should have the gemini-3.5-flash-high quota");
+  assert.equal(quota.quotaSource, "localUsageHistory", "stale full bucket replaced by local usage");
+  assert.equal(quota.used, 3, "3000 seeded tokens → 3 units used");
+});
+
+test("Antigravity stays fetchAvailableModels when usage_history has no matching rows", async () => {
+  core.resetDbInstance();
+
+  const resetTime = new Date(Date.now() + 60 * 60 * 1000).toISOString();
+
+  globalThis.fetch = (async (input: any) => {
+    const url = typeof input === "string" ? input : input?.url || "";
+    if (url.includes("retrieveUserQuota")) {
+      return { ok: false, status: 404, json: async () => ({}) } as Response;
+    }
+    return {
+      ok: true,
+      json: async () => ({
+        models: {
+          "gemini-3.5-flash-high": { quotaInfo: { remainingFraction: 1.0, resetTime } },
+        },
+      }),
+    } as Response;
+  }) as typeof fetch;
+
+  const connection = {
+    id: "conn-local-2",
+    provider: "antigravity",
+    accessToken: "fake-token-local-usage-empty",
+    providerSpecificData: {},
+    projectId: undefined,
+  };
+
+  const result = await getUsageForProvider(connection, { forceRefresh: true });
+  const quota = (result as any).quotas["gemini-3.5-flash-high"];
+  assert.ok(quota, "should have the quota");
+  assert.equal(quota.quotaSource, "fetchAvailableModels", "no local rows → keep the catalog view");
+  assert.equal(quota.used, 0, "full bucket stays at 0 used");
+});
--- a/tests/unit/antigravity-model-aliases.test.ts
+++ b/tests/unit/antigravity-model-aliases.test.ts
@@ -7,6 +7,7 @@ import {
  isUserCallableAntigravityModelId,
  resolveAntigravityModelId,
  toClientAntigravityModelId,
+  toClientAntigravityQuotaModelId,
 } from "../../open-sse/config/antigravityModelAliases.ts";
 import { AntigravityExecutor } from "../../open-sse/executors/antigravity.ts";
 import { openaiToAntigravityRequest } from "../../open-sse/translator/request/openai-to-gemini.ts";
@@ -15,6 +16,24 @@ function getPublicModel(id: string) {
  return ANTIGRAVITY_PUBLIC_MODELS.find((model) => model.id === id) as any;
 }

+// #3821-review LEDGER-5 — the upstream quota-bucket → client-tier remap is now the single
+// source of truth here (was duplicated as an inline if-ladder in usage.ts). It operates on
+// the UPSTREAM quota namespace, where `gemini-3.5-flash-low` is the Medium tier's bucket.
+test("toClientAntigravityQuotaModelId maps upstream quota buckets to client tiers", () => {
+  assert.equal(toClientAntigravityQuotaModelId("gemini-3.5-flash-extra-low"), "gemini-3.5-flash-low");
+  // Dual-meaning id: in the quota namespace this bucket is the Medium tier.
+  assert.equal(toClientAntigravityQuotaModelId("gemini-3.5-flash-low"), "gemini-3.5-flash-medium");
+  assert.equal(toClientAntigravityQuotaModelId("gemini-3-flash-agent"), "gemini-3.5-flash-high");
+  // Non-tier ids fall back to the standard reverse alias map.
+  assert.equal(toClientAntigravityQuotaModelId("gemini-3.1-pro"), "gemini-3-pro-preview");
+  // Always-allowed bucket passes through unchanged.
+  assert.equal(toClientAntigravityQuotaModelId("credits"), "credits");
+  // Retired preview buckets are dropped (hidden from clients).
+  assert.equal(toClientAntigravityQuotaModelId("gemini-3.5-flash-preview"), null);
+  assert.equal(toClientAntigravityQuotaModelId("gemini-3-flash-preview"), null);
+  assert.equal(toClientAntigravityQuotaModelId(""), null);
+});
+
 test("resolveAntigravityModelId maps the documented Antigravity aliases to upstream IDs", () => {
  assert.equal(resolveAntigravityModelId("gemini-3-pro-preview"), "gemini-3.1-pro");
  assert.equal(resolveAntigravityModelId("gemini-3-pro-image-preview"), "gemini-3-pro-image");
@@ -165,11 +184,14 @@ test("AntigravityExecutor.transformRequest maps Gemini 3.5 Flash tiers to live u
  );

  if (result instanceof Response) throw new Error("Unexpected Response from transformRequest");
+  // The "High" tier resolves to the live upstream id; the request body is forwarded
+  // under that id. (Dropped four assertions on modelConfigId/model_config_id — the
+  // executor never sets those fields, so they were vacuously true and gave false
+  // confidence. #3821-review LEDGER-10.)
  assert.equal(result.model, "gemini-3-flash-agent");
-  assert.equal(result.modelConfigId, undefined);
-  assert.equal(result.model_config_id, undefined);
-  assert.equal(result.request.modelConfigId, undefined);
-  assert.equal(result.request.model_config_id, undefined);
+  assert.deepEqual(result.request.contents, [
+    { role: "user", parts: [{ text: "Hello" }] },
+  ]);
 });

 test("AntigravityExecutor.transformRequest sends Claude through Gemini-compatible Cloud Code schema", async () => {
--- a/tests/unit/chatcore-extracted-modules-3821.test.ts
+++ b/tests/unit/chatcore-extracted-modules-3821.test.ts
@@ -0,0 +1,110 @@
+/**
+ * LEDGER-6 (#3821-review) — the chatCore modularization (#3598) relocated ~300 lines into
+ * open-sse/handlers/chatCore/{idempotency,sanitization,semanticCache,memorySkillsInjection}.ts
+ * with no direct tests at the new seam. The extraction shipped a real `ReferenceError`
+ * (idempotencyKey) that no test caught. These tests pin the pure/extractable pieces:
+ *  - sanitizeChatRequestBody (token-field normalization, empty-name stripping, tool filter)
+ *  - checkIdempotencyCache now returns { hit, idempotencyKey } so the save site reuses the
+ *    single derivation (no dual getIdempotencyKey call).
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { sanitizeChatRequestBody } from "../../open-sse/handlers/chatCore/sanitization.ts";
+import { checkIdempotencyCache } from "../../open-sse/handlers/chatCore/idempotency.ts";
+import { FORMATS } from "../../open-sse/translator/formats.ts";
+import { saveIdempotency } from "../../src/lib/idempotencyLayer.ts";
+
+test("sanitizeChatRequestBody: Chat Completions target maps max_output_tokens → max_tokens", () => {
+  const out = sanitizeChatRequestBody({ max_output_tokens: 256 }, FORMATS.OPENAI, FORMATS.OPENAI);
+  assert.equal(out.max_tokens, 256);
+  assert.equal(out.max_output_tokens, undefined);
+});
+
+test("sanitizeChatRequestBody: Responses target maps max_completion_tokens → max_output_tokens", () => {
+  const out = sanitizeChatRequestBody(
+    { max_completion_tokens: 512 },
+    FORMATS.OPENAI,
+    FORMATS.OPENAI_RESPONSES
+  );
+  assert.equal(out.max_output_tokens, 512);
+  assert.equal(out.max_completion_tokens, undefined);
+});
+
+test("sanitizeChatRequestBody: Responses target maps max_tokens → max_output_tokens", () => {
+  const out = sanitizeChatRequestBody({ max_tokens: 128 }, FORMATS.OPENAI_RESPONSES, FORMATS.OPENAI);
+  assert.equal(out.max_output_tokens, 128);
+  assert.equal(out.max_tokens, undefined);
+});
+
+test("sanitizeChatRequestBody: strips empty message name and filters nameless tools", () => {
+  const out = sanitizeChatRequestBody(
+    {
+      messages: [
+        { role: "user", content: "hi", name: "" },
+        { role: "assistant", content: "yo", name: "keepme" },
+      ],
+      tools: [
+        { type: "function", function: { name: "real_tool", parameters: {} } },
+        { type: "function", function: { name: "" } }, // dropped — empty name
+        { type: "function", function: {} }, // dropped — no name
+      ],
+    },
+    FORMATS.OPENAI,
+    FORMATS.OPENAI
+  );
+
+  const messages = out.messages as Array<Record<string, unknown>>;
+  assert.ok(!("name" in messages[0]), "empty name stripped");
+  assert.equal(messages[1].name, "keepme", "non-empty name kept");
+
+  const tools = out.tools as Array<Record<string, unknown>>;
+  assert.equal(tools.length, 1, "only the named tool survives");
+  assert.equal((tools[0].function as Record<string, unknown>).name, "real_tool");
+});
+
+test("checkIdempotencyCache returns { hit:null, idempotencyKey } on a miss", async () => {
+  const headers = new Headers({ "idempotency-key": "idem-miss-3821" });
+  const result = await checkIdempotencyCache({
+    clientRawRequest: { headers },
+    provider: "openai",
+    model: "gpt-4.1",
+    effectiveServiceTier: undefined,
+    startTime: 0,
+    log: undefined,
+  });
+  assert.equal(result.hit, null);
+  assert.equal(result.idempotencyKey, "idem-miss-3821");
+});
+
+test("checkIdempotencyCache returns a hit Response reusing the same key after a save", async () => {
+  const key = "idem-hit-3821";
+  saveIdempotency(key, { object: "chat.completion", choices: [], usage: {} }, 200);
+
+  const headers = new Headers({ "idempotency-key": key });
+  const result = await checkIdempotencyCache({
+    clientRawRequest: { headers },
+    provider: "openai",
+    model: "gpt-4.1",
+    effectiveServiceTier: undefined,
+    startTime: 0,
+    log: undefined,
+  });
+
+  assert.equal(result.idempotencyKey, key, "the resolved key is returned for the save site to reuse");
+  assert.ok(result.hit, "a cached entry produces a hit");
+  assert.equal(result.hit!.response.headers.get("X-OmniRoute-Idempotent"), "true");
+});
+
+test("checkIdempotencyCache resolves a null key when no idempotency headers are present", async () => {
+  const result = await checkIdempotencyCache({
+    clientRawRequest: { headers: new Headers() },
+    provider: "openai",
+    model: "gpt-4.1",
+    effectiveServiceTier: undefined,
+    startTime: 0,
+    log: undefined,
+  });
+  assert.equal(result.hit, null);
+  assert.equal(result.idempotencyKey, null);
+});
--- a/tests/unit/completions-text-format-3571.test.ts
+++ b/tests/unit/completions-text-format-3571.test.ts
@@ -5,6 +5,7 @@ import {
  toTextCompletionObject,
  transformSseData,
  createTextCompletionStreamTransformer,
+  asTextCompletionResponse,
 } from "../../src/app/api/v1/completions/textCompletionTransform.ts";

 // #3571 — /v1/completions (legacy OpenAI Completions API) must return
@@ -104,3 +105,52 @@ test("#3571 stream transformer end-to-end: chat SSE → text SSE", async () => {
  assert.ok(result.includes("data: [DONE]")); // [DONE] preserved
  assert.ok(!result.includes("delta")); // no chat shape leaks
 });
+
+// #3821-review LEDGER-8 — both response branches rewrite the body, so a stale upstream
+// content-length must be dropped (a buffered SSE body with content-length would otherwise
+// advertise the pre-rewrite length and truncate/hang the client).
+test("#3571/#3821 asTextCompletionResponse drops content-length on the SSE branch", async () => {
+  const sseBody =
+    'data: {"object":"chat.completion.chunk","choices":[{"index":0,"delta":{"content":"hi"},"finish_reason":"stop"}]}\n\n';
+  const upstream = new Response(sseBody, {
+    status: 200,
+    headers: {
+      "content-type": "text/event-stream",
+      // A (deliberately wrong) content-length that must NOT survive the rewrite.
+      "content-length": String(sseBody.length),
+    },
+  });
+
+  const out = await asTextCompletionResponse(upstream);
+  assert.equal(out.headers.get("content-length"), null, "content-length must be stripped");
+  assert.match(out.headers.get("content-type") || "", /text\/event-stream/);
+  const text = await out.text();
+  assert.ok(text.includes('"object":"text_completion"'));
+  assert.ok(text.includes('"text":"hi"'));
+});
+
+test("#3571/#3821 asTextCompletionResponse drops content-length on the JSON branch", async () => {
+  const jsonBody = JSON.stringify({
+    object: "chat.completion",
+    choices: [{ index: 0, message: { content: "hi" }, finish_reason: "stop" }],
+  });
+  const upstream = new Response(jsonBody, {
+    status: 200,
+    headers: { "content-type": "application/json", "content-length": String(jsonBody.length) },
+  });
+
+  const out = await asTextCompletionResponse(upstream);
+  assert.equal(out.headers.get("content-length"), null);
+  const obj = await out.json();
+  assert.equal(obj.object, "text_completion");
+  assert.equal(obj.choices[0].text, "hi");
+});
+
+test("#3571/#3821 asTextCompletionResponse passes error responses through untouched", async () => {
+  const upstream = new Response(JSON.stringify({ error: { message: "boom" } }), {
+    status: 500,
+    headers: { "content-type": "application/json" },
+  });
+  const out = await asTextCompletionResponse(upstream);
+  assert.equal(out, upstream, "non-ok responses are returned as-is");
+});
--- a/tests/unit/executor-agy.test.ts
+++ b/tests/unit/executor-agy.test.ts
@@ -2,6 +2,17 @@ import test from "node:test";
 import assert from "node:assert/strict";

 import { getExecutor, AntigravityExecutor } from "../../open-sse/executors/index.ts";
+import { processAntigravitySSEPayload } from "../../open-sse/executors/antigravity.ts";
+
+function emptyCollected(): any {
+  return {
+    textContent: "",
+    finishReason: "",
+    toolCalls: [],
+    usage: null,
+    remainingCredits: null,
+  };
+}

 test("getExecutor('agy') returns AntigravityExecutor (not DefaultExecutor)", () => {
  const executor = getExecutor("agy");
@@ -40,3 +51,27 @@ test("getExecutor('agy') buildHeaders returns Bearer auth", () => {
  const headers = executor.buildHeaders({ accessToken: "test-token" });
  assert.equal(headers.Authorization, "Bearer test-token");
 });
+
+// #3821-review LEDGER-9 — the Antigravity SSE `markdown` extraction branch had no test.
+test("processAntigravitySSEPayload accumulates top-level markdown into textContent", () => {
+  const collected = emptyCollected();
+  processAntigravitySSEPayload(JSON.stringify({ markdown: "Hello " }), collected);
+  processAntigravitySSEPayload(JSON.stringify({ response: { markdown: "world" } }), collected);
+  assert.equal(collected.textContent, "Hello world");
+});
+
+test("processAntigravitySSEPayload uses candidate parts text when no markdown is present", () => {
+  const collected = emptyCollected();
+  processAntigravitySSEPayload(
+    JSON.stringify({ response: { candidates: [{ content: { parts: [{ text: "from parts" }] } }] } }),
+    collected
+  );
+  assert.equal(collected.textContent, "from parts");
+});
+
+test("processAntigravitySSEPayload ignores [DONE] and malformed payloads without throwing", () => {
+  const collected = emptyCollected();
+  processAntigravitySSEPayload("[DONE]", collected);
+  processAntigravitySSEPayload("{not json", collected);
+  assert.equal(collected.textContent, "");
+});
--- a/tests/unit/mcp-published-files-closure-3578.test.ts
+++ b/tests/unit/mcp-published-files-closure-3578.test.ts
@@ -2,6 +2,7 @@ import test from "node:test";
 import assert from "node:assert/strict";
 import fs from "node:fs";
 import path from "node:path";
+import { execFileSync } from "node:child_process";

 // #3578 — `omniroute --mcp` crashed on npm installs with ERR_MODULE_NOT_FOUND for
 // src/lib/combos/steps.ts: the MCP server runs from raw TypeScript source and imports
@@ -100,3 +101,55 @@ test("#3578 every MCP-server source file is covered by package.json files", () =
      uncovered.map((f) => "  - " + f).join("\n")
  );
 });
+
+// #3821-review (LEDGER-1): the static `files` check above only guards UNDER-inclusion
+// (every MCP file is allowlisted). It cannot see that the whole-directory entries
+// (open-sse/, src/lib/, ...) also drag co-located test files into the tarball, nor that
+// a future secret-bearing fixture under a shipped dir would publish. This test asserts
+// the REAL `npm pack --dry-run` output in BOTH directions: the MCP closure is present AND
+// no `__tests__` / `*.test.*` / `*.spec.*` file ships. It is the regression anchor for the
+// `!**/*.test.*` negations in package.json `files`.
+function packedFilePaths(): string[] {
+  // --dry-run writes no tarball; --json emits [{ files: [{ path }] }] on stdout.
+  const out = execFileSync("npm", ["pack", "--dry-run", "--json"], {
+    cwd: ROOT,
+    encoding: "utf8",
+    maxBuffer: 32 * 1024 * 1024,
+    stdio: ["ignore", "pipe", "ignore"],
+  });
+  const parsed = JSON.parse(out) as Array<{ files?: Array<{ path: string }> }>;
+  const entry = parsed[0];
+  assert.ok(entry?.files?.length, "npm pack --dry-run returned no files");
+  return entry.files!.map((f) => f.path);
+}
+
+const TEST_FILE_RE = /(?:^|\/)__tests__\/|\.(?:test|spec)\.[cm]?[jt]sx?$/;
+
+test("#3578/#3821 npm pack ships the MCP closure but no test files", () => {
+  const packed = packedFilePaths();
+  const packedSet = new Set(packed);
+
+  // Direction 1 — under-inclusion: every MCP-reachable source file is actually packed.
+  const closure = computeMcpClosure();
+  const missing = closure.filter((f) => !packedSet.has(f));
+  assert.deepEqual(
+    missing,
+    [],
+    `MCP-reachable source files are missing from the published tarball (would 404 --mcp):\n` +
+      missing.map((f) => "  - " + f).join("\n")
+  );
+  // Spot-check the file from the original bug report.
+  assert.ok(
+    packedSet.has("src/lib/combos/steps.ts"),
+    "src/lib/combos/steps.ts (the #3578 bug file) must be in the tarball"
+  );
+
+  // Direction 2 — over-inclusion: no co-located test / spec file is published.
+  const shippedTests = packed.filter((f) => TEST_FILE_RE.test(f));
+  assert.deepEqual(
+    shippedTests,
+    [],
+    `These test files leaked into the npm tarball — tighten package.json "files" negations:\n` +
+      shippedTests.map((f) => "  - " + f).join("\n")
+  );
+});
--- a/tests/unit/provider-limits-sanitize-scope-3821.test.ts
+++ b/tests/unit/provider-limits-sanitize-scope-3821.test.ts
@@ -0,0 +1,107 @@
+/**
+ * LEDGER-2 (#3821-review) — getSanitizedCachedProviderLimitsMap is polled by the
+ * ProviderLimits dashboard on an auto-refresh interval. It used to run an
+ * unconditional `SELECT * FROM provider_connections` (decrypting every active
+ * connection's credentials) on every poll, even though quota-key sanitization only
+ * ever rewrites Antigravity/agy entries. The fix scopes the connection scan to
+ * antigravity/agy (and skips it entirely for an empty cache).
+ *
+ * These tests pin the BEHAVIOR the optimization must preserve:
+ *   1. empty cache  → {} (no scan needed)
+ *   2. non-Antigravity entry → returned verbatim (a junk quota key survives), proving
+ *      entries whose connection is no longer fetched are still passed through unchanged
+ *   3. Antigravity entry → still sanitized (a non-user-callable quota key is dropped),
+ *      proving the scoped query still feeds the sanitizer
+ *
+ * (2) is the load-bearing case: with the old code the openai connection was fetched and
+ * present in the lookup; with the new code it is NOT fetched at all, yet the output must
+ * be identical — which it is, because sanitizeProviderLimitsCacheForConnection returns
+ * the entry unchanged when no matching connection is supplied.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-plimits-scope-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.API_KEY_SECRET = "test-plimits-scope-secret";
+
+const core = await import("../../src/lib/db/core.ts");
+const providersDb = await import("../../src/lib/db/providers.ts");
+const providerLimitsDb = await import("../../src/lib/db/providerLimits.ts");
+const providerLimits = await import("../../src/lib/usage/providerLimits.ts");
+
+test.beforeEach(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+});
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+function cacheEntry(quotas: Record<string, unknown>) {
+  return {
+    quotas,
+    plan: null,
+    message: null,
+    fetchedAt: new Date(0).toISOString(),
+    source: null,
+  };
+}
+
+test("empty provider-limits cache returns {} without any connection scan", async () => {
+  const out = await providerLimits.getSanitizedCachedProviderLimitsMap();
+  assert.deepEqual(out, {});
+});
+
+test("non-Antigravity cache entry is returned verbatim (junk quota key survives)", async () => {
+  // An active openai connection whose credentials would be decrypted by the old
+  // unconditional scan. Under the fix it is never fetched — the entry must still pass
+  // through unchanged.
+  const conn = await providersDb.createProviderConnection({
+    provider: "openai",
+    authType: "api_key",
+    name: "OpenAI key",
+    apiKey: "sk-test-openai",
+  });
+  const quotas = { "definitely-not-a-real-model": { used: 1, limit: 10 } };
+  providerLimitsDb.setProviderLimitsCache((conn as { id: string }).id, cacheEntry(quotas));
+
+  const out = await providerLimits.getSanitizedCachedProviderLimitsMap();
+  const entry = out[(conn as { id: string }).id];
+  assert.ok(entry, "openai cache entry should be present");
+  // Sanitization is antigravity/agy-only → the junk key is NOT dropped for openai.
+  assert.deepEqual(entry.quotas, quotas);
+});
+
+test("Antigravity cache entry is still sanitized (non-user-callable quota key dropped)", async () => {
+  const conn = await providersDb.createProviderConnection({
+    provider: "antigravity",
+    authType: "oauth",
+    name: "Antigravity acct",
+    email: "antigravity@example.test",
+    accessToken: "ag-access",
+    refreshToken: "ag-refresh",
+    expiresAt: new Date(Date.now() + 3_600_000).toISOString(),
+  });
+  // `credits` is always allowed; the junk model id is not user-callable → dropped.
+  const quotas = {
+    credits: { used: 5, limit: 100 },
+    "definitely-not-a-real-model": { used: 1, limit: 10 },
+  };
+  providerLimitsDb.setProviderLimitsCache((conn as { id: string }).id, cacheEntry(quotas));
+
+  const out = await providerLimits.getSanitizedCachedProviderLimitsMap();
+  const entry = out[(conn as { id: string }).id];
+  assert.ok(entry?.quotas, "antigravity cache entry should be present");
+  assert.ok("credits" in (entry.quotas as Record<string, unknown>), "credits is kept");
+  assert.ok(
+    !("definitely-not-a-real-model" in (entry.quotas as Record<string, unknown>)),
+    "non-user-callable quota key is dropped for antigravity"
+  );
+});
--- a/tests/unit/response-sanitizer.test.ts
+++ b/tests/unit/response-sanitizer.test.ts
@@ -17,6 +17,28 @@ test("extractThinkingFromContent separates think blocks from visible content", (
  assert.equal(parsed.thinking, "reasoning 1\n\nreasoning 2");
 });

+// #3821-review LEDGER-7 — the unclosed-reasoning-tag heuristic (#3605) reclassifies a
+// dangling `<thought`-style tail as reasoning. Pin that a REAL visible prefix before such
+// a tail is preserved as content (only a whitespace/§marker§ prefix collapses to ""), and
+// that a non-reasoning tag like `<thoughtful>` is NOT captured.
+test("extractThinkingFromContent preserves a real prefix before a dangling reasoning tag", () => {
+  const parsed = extractThinkingFromContent("Here is the answer. <thought\nleftover reasoning");
+  assert.equal(parsed.content, "Here is the answer.");
+  assert.equal(parsed.thinking, "leftover reasoning");
+});
+
+test("extractThinkingFromContent: §marker§-only prefix collapses to empty content", () => {
+  const parsed = extractThinkingFromContent("§54§ <thought\ninternal planning");
+  assert.equal(parsed.content, "");
+  assert.equal(parsed.thinking, "internal planning");
+});
+
+test("extractThinkingFromContent does NOT treat <thoughtful> as a reasoning tag", () => {
+  const parsed = extractThinkingFromContent("See the <thoughtful> approach here");
+  assert.equal(parsed.content, "See the <thoughtful> approach here");
+  assert.equal(parsed.thinking, null);
+});
+
 test("sanitizeOpenAIResponse strips non-standard fields and preserves required top-level fields", () => {
  const sanitized = sanitizeOpenAIResponse({
    id: "chatcmpl_existing",
--- a/tests/unit/translator-resp-gemini-to-openai.test.ts
+++ b/tests/unit/translator-resp-gemini-to-openai.test.ts
@@ -1122,3 +1122,112 @@ test("Gemini stream: checks lastParen before lastBracket when identifying partia
  assert.equal(toolCall.function.name, "my_tool");
  assert.equal(toolCall.function.arguments, "{}");
 });
+
+// #3821-review LEDGER-4 — a signed native functionCall arriving while a textual
+// `<thinking>` wrapper opened in an earlier chunk is still buffered must flush that
+// buffered reasoning as reasoning_content, not silently discard it.
+test("Gemini stream: open textual reasoning is flushed before a signed native tool call", () => {
+  const state = createStreamingState();
+
+  // chunk 1: opens a <thinking> wrapper with no close tag → buffered, nothing emitted.
+  const r1 =
+    geminiToOpenAIResponse(
+      {
+        responseId: "resp-flush-reasoning",
+        modelVersion: "gemini-3-flash-agent",
+        candidates: [{ content: { parts: [{ text: "<thinking>deep reasoning here" }] } }],
+      },
+      state
+    ) || [];
+  assert.ok(
+    !r1.some((e: any) => e.choices?.[0]?.delta?.reasoning_content),
+    "reasoning is still buffered (awaiting close tag) — nothing emitted yet"
+  );
+
+  // chunk 2: signed native functionCall while the reasoning wrapper is still open.
+  const r2 =
+    geminiToOpenAIResponse(
+      {
+        responseId: "resp-flush-reasoning",
+        modelVersion: "gemini-3-flash-agent",
+        candidates: [
+          {
+            content: {
+              parts: [
+                {
+                  thoughtSignature: "sig-flush-1",
+                  functionCall: { id: "call-flush-1", name: "do_thing", args: {} },
+                },
+              ],
+            },
+          },
+        ],
+      },
+      state
+    ) || [];
+
+  const reasoningIdx = r2.findIndex((e: any) => e.choices?.[0]?.delta?.reasoning_content);
+  const toolIdx = r2.findIndex((e: any) => e.choices?.[0]?.delta?.tool_calls);
+  assert.equal(
+    r2[reasoningIdx]?.choices[0].delta.reasoning_content,
+    "deep reasoning here",
+    "buffered textual reasoning must be flushed, not dropped, when a tool call arrives"
+  );
+  assert.equal(r2[toolIdx]?.choices[0].delta.tool_calls[0].id, "call-flush-1");
+  assert.ok(reasoningIdx >= 0 && toolIdx > reasoningIdx, "reasoning is emitted before the tool call");
+});
+
+// #3821-review LEDGER-15 — a reasoning-only chunk interrupting a partially-buffered
+// textual "[Tool call: ...]" must not strand the buffer; it resolves once the rest of
+// the tool-call text arrives (or at finishReason).
+test("Gemini stream: partial textual tool call survives a reasoning-only chunk", () => {
+  const state = createStreamingState();
+
+  // chunk 1: partial textual tool call (incomplete JSON) → buffered.
+  geminiToOpenAIResponse(
+    {
+      responseId: "resp-interleave",
+      modelVersion: "gemini-3.5-flash-low",
+      candidates: [
+        { content: { parts: [{ text: '[Tool call: terminal]\nArguments: {"command":"ls' }] } },
+      ],
+    },
+    state
+  );
+
+  // chunk 2: a reasoning-only chunk fully consumed as reasoning_content.
+  const r2 =
+    geminiToOpenAIResponse(
+      {
+        responseId: "resp-interleave",
+        modelVersion: "gemini-3.5-flash-low",
+        candidates: [{ content: { parts: [{ text: "<thinking>pondering</thinking>" }] } }],
+      },
+      state
+    ) || [];
+  assert.equal(
+    r2.find((e: any) => e.choices?.[0]?.delta?.reasoning_content)?.choices[0].delta
+      .reasoning_content,
+    "pondering"
+  );
+  assert.ok(
+    typeof state.textualToolCallBuffer === "string" &&
+      state.textualToolCallBuffer.includes("[Tool call: terminal]"),
+    "the partial tool-call buffer must survive the reasoning-only chunk"
+  );
+
+  // chunk 3: completes the tool-call text + finishReason → resolves to a structured call.
+  const r3 =
+    geminiToOpenAIResponse(
+      {
+        responseId: "resp-interleave",
+        modelVersion: "gemini-3.5-flash-low",
+        candidates: [{ content: { parts: [{ text: '"}' }] }, finishReason: "STOP" }],
+      },
+      state
+    ) || [];
+  const toolCall = r3.find((e: any) => e.choices?.[0]?.delta?.tool_calls)?.choices[0].delta
+    .tool_calls[0];
+  assert.ok(toolCall, "the textual tool call resolves after the reasoning-only interruption");
+  assert.equal(toolCall.function.name, "terminal");
+});
--- a/tests/unit/upstream-ca-test-route-3488.test.ts
+++ b/tests/unit/upstream-ca-test-route-3488.test.ts
@@ -4,7 +4,15 @@ import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";

-import { POST } from "../../src/app/api/tools/agent-bridge/upstream-ca/test/route.ts";
+// Point the data dir at a throwaway location BEFORE importing the route so we can assert
+// the validate-only route never writes the persisted CA-path file. resolveMitmDataDir()
+// reads DATA_DIR at call time, so this also governs the route under test.
+const DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ca-datadir-"));
+process.env.DATA_DIR = DATA_DIR;
+// The persisted path used by the real (persisting) POST /upstream-ca route.
+const PERSISTED_CA_PATH_FILE = path.join(DATA_DIR, "mitm", "upstream-ca.path");
+
+const { POST } = await import("../../src/app/api/tools/agent-bridge/upstream-ca/test/route.ts");

 // #3488 — UpstreamCaField's "Test" button POSTed to /api/tools/agent-bridge/upstream-ca/test,
 // which did not exist (404). The new validate-only route checks the CA file exists and is a
@@ -38,7 +46,10 @@ const nonPemPath = path.join(dir, "not-a-cert.txt");
 fs.writeFileSync(validCaPath, TEST_CA_PEM);
 fs.writeFileSync(nonPemPath, "this is not a certificate");

-test.after(() => fs.rmSync(dir, { recursive: true, force: true }));
+test.after(() => {
+  fs.rmSync(dir, { recursive: true, force: true });
+  fs.rmSync(DATA_DIR, { recursive: true, force: true });
+});

 function postJson(body: unknown): Request {
  return new Request("http://localhost/api/tools/agent-bridge/upstream-ca/test", {
@@ -57,13 +68,24 @@ test("#3488 valid PEM cert → 200 ok with subject", async () => {
 });

 test("#3488 does NOT persist the CA path (validate-only)", async () => {
-  await POST(postJson({ path: validCaPath }));
-  // The persisted path file used by the real POST route must not be created by /test.
-  // We can't import the constant without side effects, so assert the dry-run returns a
-  // shape with no activation marker and leaves no global state — the absence of an
-  // `activated`/`persisted` field is the contract.
+  // Real side-effect guard (#3821-review LEDGER-11): the persisting POST /upstream-ca
+  // route writes <dataDir>/mitm/upstream-ca.path. After a successful /test call that file
+  // must NOT exist — proving the dry-run never persisted/activated the CA.
+  assert.ok(
+    !fs.existsSync(PERSISTED_CA_PATH_FILE),
+    "precondition: persisted CA-path file should not exist before the test"
+  );
+
  const res = await POST(postJson({ path: validCaPath }));
+  assert.equal(res.status, 200);
  const json = await res.json();
+  assert.equal(json.ok, true);
+
+  assert.ok(
+    !fs.existsSync(PERSISTED_CA_PATH_FILE),
+    "validate-only /test route must not write the persisted upstream-ca.path file"
+  );
+  // And it must not advertise activation/persistence in its response shape.
  assert.equal(json.persisted, undefined);
  assert.equal(json.activated, undefined);
 });