fix(sse): omit Command Code max_tokens when client sends a non-positive value (#5166) (#5304)

Integrated into release/v3.8.40 (CHANGELOG re-resolved against post-#5294 tip; code identical to the green commit)

CHANGELOG.md

@@ -24,6 +24,7 @@ _In development — bullets added per PR; finalized at release._
 
 ### 🔧 Bug Fixes
 
+- **command-code:** treat a non-positive `max_tokens`/`max_completion_tokens` (e.g. Zoo Code's `-1` "let the server choose") as "no limit" — omit the field instead of forcing it to `1`. `clampMaxTokens` previously did `Math.max(1, …)`, so a client `-1` was sent upstream as `max_tokens: 1`, truncating the response to a single token (the observed `completion_tokens: 1`, `content: null`, `reasoning_content: "The"` with `finish_reason: stop`). Now any value `≤ 0` is dropped so Command Code applies the model's native default; positive values are still floored and clamped to the 200k ceiling. Regression guard: `tests/unit/command-code-maxtokens-negative-5166.test.ts` ([#5166](https://github.com/diegosouzapw/OmniRoute/issues/5166) — thanks @Stazyu)
 - **fix(auth): compare-and-swap guard on the OAuth refresh persist** — under multi-agent load, the per-connection refresh mutex makes `[network refresh + DB write]` atomic for **one** connection, but it does not protect against a **third** writer (a sibling request, a concurrent HealthCheck, or a replica) landing a fresher `refresh_token` rotation on the same `connection_id` between the staleness read and the persist. Overwriting that fresher row reverts the sibling's rotation; the next caller then loads the now-consumed token, Auth0/Anthropic flag it as `refresh_token_reused`, and the whole token family gets revoked (the 1352× claude/`aa5dd5cf` invalidation storm). `getAccessToken` now re-reads the row's current `refresh_token` immediately before persisting (inside the mutex) and **skips the write** when it has rotated past the token the caller presented — the caller still receives the freshly-issued access token, only the DB overwrite is skipped. Opt-in via `runWithCasGuard` (no active guard ⇒ byte-identical behavior); skip/persist counters exposed via `getCasGuardStats()`. Regression guard: `tests/unit/token-refresh-cas-guard-4038.test.ts`. ([#4038](https://github.com/diegosouzapw/OmniRoute/issues/4038) — thanks @KooshaPari for the root-cause diagnosis)
 - **mcp:** break the `schemas/tools.ts ↔ schemas/toolSearch.ts` import cycle introduced when the `tool_search` defs (#5269) were extracted into their own module — `toolSearch.ts` imported `McpToolDefinition` from `tools.ts` while `tools.ts` imported `toolSearchTool` from `toolSearch.ts`, failing `check:cycles` on `release/v3.8.40`. The shared `AuditLevel` + `McpToolDefinition` types now live in a leaf `schemas/toolDefinition.ts` that both import; `tools.ts` re-exports them for backward compatibility.
 - **compression (analytics):** record attempted-but-no-op compression runs so Stacked is no longer invisible when it saves nothing. Previously a `compression_analytics` row was written only on a net-positive saving, so a Stacked (RTK→Caveman) pipeline that ran on already-compact context produced no row — indistinguishable from "never dispatched" (`byMode.stacked.count` stayed flat while Ultra climbed). Such runs are now recorded with `skip_reason` and surfaced as a per-mode `skipped` count plus `totalSkipped`/`bySkipReason` in the analytics summary and the Mode Breakdown; the existing net-saving totals/averages are unchanged (skip rows are excluded from them) (#4268 — thanks @abdulkadirozyurt, @androw)

open-sse/executors/commandCode.ts

@@ -148,14 +148,17 @@ function convertMessages(messages: unknown): { system: string; messages: unknown
 
 // Clamp a client-supplied max_tokens to the endpoint ceiling, mirroring the
 // provider-driven clamp in antigravity.ts: we only intervene when the value is
-// present AND would otherwise be rejected (> 200_000). A valid value is
-// returned floored; anything absent or non-numeric returns undefined so the
-// caller can OMIT the field entirely and let Command Code's upstream apply the
-// model's own native default (rather than us inventing a number).
+// present, positive AND would otherwise be rejected (> 200_000). A valid value
+// is returned floored; anything absent, non-numeric or non-positive returns
+// undefined so the caller can OMIT the field entirely and let Command Code's
+// upstream apply the model's own native default (rather than us inventing a
+// number). A non-positive value such as Zoo Code's max_tokens:-1 ("let the
+// server choose") must be omitted, NOT forced to 1 — the old Math.max(1,...)
+// truncated output to a single token (#5166).
 function clampMaxTokens(value: unknown): number | undefined {
   const numeric = numberValue(value);
-  if (numeric === undefined) return undefined;
-  return Math.max(1, Math.min(Math.floor(numeric), MAX_COMMAND_CODE_TOKENS));
+  if (numeric === undefined || numeric <= 0) return undefined;
+  return Math.min(Math.floor(numeric), MAX_COMMAND_CODE_TOKENS);
 }
 
 // Reasoning/thinking fields that payload rules or clients may inject and that

tests/unit/command-code-maxtokens-negative-5166.test.ts

@@ -0,0 +1,76 @@
+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";
+
+// #5166: Zoo Code sends `max_tokens: -1` to mean "let the server choose". The
+// old clampMaxTokens did `Math.max(1, ...)`, forcing -1 → 1 and truncating
+// output to a single token (the observed `completion_tokens: 1`, `content:null`,
+// `reasoning_content:"The"` symptom). A non-positive limit must be OMITTED so
+// Command Code's upstream applies the model's own native default.
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cc-maxtokens-5166-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { getExecutor } = await import("../../open-sse/executors/index.ts");
+const core = await import("../../src/lib/db/core.ts");
+
+const originalFetch = globalThis.fetch;
+
+type FetchCall = { url: string; init: Record<string, unknown>; body?: any };
+
+function commandCodeStream(lines: unknown[]) {
+  const text = lines.map((line) => `${JSON.stringify(line)}\n`).join("");
+  return new Response(text, { status: 200, headers: { "Content-Type": "application/x-ndjson" } });
+}
+
+test.afterEach(() => {
+  globalThis.fetch = originalFetch;
+});
+
+test.after(() => {
+  globalThis.fetch = originalFetch;
+  core.resetDbInstance();
+});
+
+async function captureParams(body: Record<string, unknown>): Promise<FetchCall> {
+  const calls: FetchCall[] = [];
+  globalThis.fetch = async (url: any, init: any = {}) => {
+    calls.push({ url: String(url), init, body: JSON.parse(String(init.body)) });
+    return commandCodeStream([{ type: "text-delta", text: "ok" }, { type: "finish" }]);
+  };
+  await getExecutor("command-code").execute({
+    model: "deepseek/deepseek-v4-pro",
+    stream: false,
+    credentials: { apiKey: "cc_test_key" },
+    body: { messages: [{ role: "user", content: "Hi" }], ...body },
+  });
+  return calls[0];
+}
+
+test("Command Code omits max_tokens when the client sends max_tokens: -1 (#5166)", async () => {
+  const call = await captureParams({ max_tokens: -1 });
+  assert.ok(
+    !("max_tokens" in call.body.params),
+    `max_tokens:-1 must be omitted, got params.max_tokens=${call.body.params.max_tokens}`
+  );
+});
+
+test("Command Code omits max_tokens when the client sends max_completion_tokens: -1 (#5166)", async () => {
+  const call = await captureParams({ max_completion_tokens: -1 });
+  assert.ok(
+    !("max_tokens" in call.body.params),
+    `max_completion_tokens:-1 must be omitted, got params.max_tokens=${call.body.params.max_tokens}`
+  );
+});
+
+test("Command Code omits max_tokens when the client sends 0 (#5166)", async () => {
+  const call = await captureParams({ max_tokens: 0 });
+  assert.ok(!("max_tokens" in call.body.params), "max_tokens:0 must be omitted");
+});
+
+test("Command Code still honors a positive client max_tokens after the #5166 fix", async () => {
+  const call = await captureParams({ max_tokens: 2048 });
+  assert.equal(call.body.params.max_tokens, 2048);
+});