refactor(compression): flatten the structured-clone gate to clear the new-code complexity ratchet (#13154)

isStrictlySerializable rejected any undefined value and any non-plain object
(Date/Map/Set/RegExp) before ever checking whether structuredClone (what
worker.postMessage actually uses) would accept it. strategySelector.ts's
runCompressionAsync always builds the 9-key workerOptions object with every
key explicitly present, so provider: undefined alone (the common case when
no provider is resolved yet) rejected almost every real stacked/rtk/standard
compression call, forcing it onto the main event loop instead of a worker
thread.

Widen isStrictlySerializable to accept undefined and the structured-clone-
native Date/Map/Set/RegExp types (copied, not walked, by structuredClone),
while still rejecting functions and Symbols.

Companion fix: compressionWorker.ts's stacked-mode branch called
applyStackedCompression directly on the raw job body, skipping the
adaptBodyForCompression/restore step that the sync in-process path
(strategySelector.ts's runCompression) always applies for Responses
input[] and Kiro conversationState envelopes. Because the over-strict gate
above meant essentially no real stacked call ever reached the worker, this
was dead code until this fix; without also fixing it, widening the gate
would have shipped a live regression that miscompresses Responses/Kiro
bodies whenever they are now correctly routed to the worker.

changelog.d/fixes/13154-compression-worker-clone-gate.md

@@ -0,0 +1 @@
+- fix(sse): widen compression worker eligibility gate to accept structured-clone-safe `undefined`/Date/Map/Set/RegExp values, restoring worker offload for real requests (#13154)

changelog.d/fixes/13963-zcode-win32-shell-shim.md

@@ -1 +0,0 @@
-- fix(providers): use shell:true on win32 for zcode .cmd/.bat shim spawn (#13963)

open-sse/executors/zcodeProtocol.ts

@@ -1,5 +1,4 @@
 import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
-import { shouldUseShellForCommand } from "@/shared/services/cliRuntime";
 
 const HEADER_SIZE = 13;
 const REGULAR_MESSAGE = 1;
@@ -155,21 +154,6 @@ export function encodeZcodeRpcCall(
   return frame;
 }
 
-/**
- * Whether spawn() must go through the shell to launch `command`.
- *
- * On win32, npm installs global CLI wrappers (e.g. the `zcode`/ZCODE_BIN
- * shim) as `.cmd`/`.bat` files. Since Node's CVE-2024-27980 fix, `spawn()`
- * refuses to launch a `.cmd`/`.bat` target without `shell: true`, throwing
- * ENOENT/EINVAL instead (#13963, same class of bug as #8590/Qoder). The
- * bundled ZCODE_SERVER_NODE runtime path spawns a bare `node`/`node.exe`
- * binary (no `.cmd`/`.bat` extension) and must keep `shell: false` even on
- * win32 — `shouldUseShellForCommand()` already encodes that extension check.
- */
-export function shouldUseShellForZcodeCommand(command: string): boolean {
-  return shouldUseShellForCommand(command);
-}
-
 function errorFromPayload(payload: unknown, fallback: string): Error {
   if (payload && typeof payload === "object") {
     const record = payload as JsonRecord;
@@ -228,8 +212,7 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
         cwd: this.cwd,
         env: this.env ? { ...process.env, ...this.env } : process.env,
         stdio: ["pipe", "pipe", "pipe"],
-        // shell:true on win32 for a .cmd/.bat ZCode shim — see #13963/#8590.
-        shell: shouldUseShellForZcodeCommand(this.command),
+        shell: false,
         windowsHide: true,
       });
     } catch (error) {
@@ -277,11 +260,7 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     });
 
     try {
-      await this.withTimeout(
-        readyPromise,
-        this.startupTimeoutMs,
-        "ZCode app-server handshake timed out"
-      );
+      await this.withTimeout(readyPromise, this.startupTimeoutMs, "ZCode app-server handshake timed out");
       this.ready = true;
     } catch (error) {
       await this.disposeChild(child);
@@ -300,10 +279,9 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     this.pendingChunks.push(chunk);
     let total = 0;
     for (const part of this.pendingChunks) total += part.byteLength;
-    const buffer =
-      total === chunk.byteLength && this.pendingChunks.length > 0
-        ? chunk
-        : Buffer.concat(this.pendingChunks);
+    const buffer = total === chunk.byteLength && this.pendingChunks.length > 0
+      ? chunk
+      : Buffer.concat(this.pendingChunks);
     this.pendingChunks = [buffer];
 
     if (!this.handshakeDone) {
@@ -329,13 +307,11 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
       }
       const child = this.child;
       if (!child) return;
-      child.stdin.write(
-        `${JSON.stringify({
-          type: "zcode-hello-ack",
-          version: "omniroute",
-          clientId: `omniroute-${process.pid}`,
-        })}\n`
-      );
+      child.stdin.write(`${JSON.stringify({
+        type: "zcode-hello-ack",
+        version: "omniroute",
+        clientId: `omniroute-${process.pid}`,
+      })}\n`);
       this.handshakeDone = true;
     }
     this.consumeFrames();
@@ -390,12 +366,10 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     if (type === RESPONSE_MESSAGE) {
       request.resolve(payload);
     } else {
-      request.reject(
-        errorFromPayload(
-          payload,
-          type === ERROR_MESSAGE ? "ZCode RPC request failed" : "ZCode RPC request canceled"
-        )
-      );
+      request.reject(errorFromPayload(
+        payload,
+        type === ERROR_MESSAGE ? "ZCode RPC request failed" : "ZCode RPC request canceled"
+      ));
     }
   }
 
@@ -463,11 +437,7 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     }
   }
 
-  private async withTimeout<T>(
-    promise: Promise<T>,
-    timeoutMs: number,
-    message: string
-  ): Promise<T> {
+  private async withTimeout<T>(promise: Promise<T>, timeoutMs: number, message: string): Promise<T> {
     let timer: ReturnType<typeof setTimeout> | undefined;
     try {
       return await Promise.race([

open-sse/services/compression/compressionWorker.ts

@@ -4,10 +4,36 @@ import {
   applyStackedCompression,
   type StackedCompressionStep,
 } from "./strategySelector.ts";
+import { adaptBodyForCompression } from "./bodyAdapter.ts";
 import type {
   CompressionWorkerJob,
   CompressionWorkerMessage,
 } from "./compressionWorkerProtocol.ts";
+import type { CompressionResult } from "./types.ts";
+
+// #13154 follow-up: `applyCompression`'s sync/in-process "stacked" branch runs every body
+// through `adaptBodyForCompression` first (Responses `input[]` and Kiro `conversationState`
+// envelopes get flattened to `messages[]`, then restored after compression) — see
+// strategySelector.ts's `runCompression`. Before the worker-eligibility gate widening in this
+// same fix, essentially no real "stacked" call ever reached the worker (any `undefined`
+// option key rejected it), so this branch calling `applyStackedCompression` directly on the
+// raw body was dead code. Now that eligible calls are actually routed here, it must mirror
+// that same adapt/restore step or Responses/Kiro bodies get miscompressed (wrong shape, and
+// hard-budget post-pass warnings silently lost) only when the worker happens to run them.
+function runStackedJob(
+  job: CompressionWorkerJob,
+  onEngineStep: (step: StackedCompressionStep) => void
+): CompressionResult {
+  const adapter = adaptBodyForCompression(
+    job.body,
+    job.options?.config?.codexResponsesConfig?.preserveToolNames
+  );
+  const result = applyStackedCompression(adapter.body, job.options?.config?.stackedPipeline, {
+    ...job.options,
+    onEngineStep,
+  });
+  return adapter.adapted ? { ...result, body: adapter.restore(result.body) } : result;
+}
 
 if (!parentPort) throw new Error("compressionWorker must run in a worker thread");
 parentPort.on("message", (job: CompressionWorkerJob) => {
@@ -20,10 +46,7 @@ parentPort.on("message", (job: CompressionWorkerJob) => {
       } satisfies CompressionWorkerMessage);
     const result =
       job.mode === "stacked"
-        ? applyStackedCompression(job.body, job.options?.config?.stackedPipeline, {
-            ...job.options,
-            onEngineStep,
-          })
+        ? runStackedJob(job, onEngineStep)
         : applyCompression(job.body, job.mode, job.options);
     parentPort.postMessage({
       id: job.id,

open-sse/services/compression/compressionWorkerProtocol.ts

@@ -33,24 +33,34 @@ function isPlainObject(value: object): value is Record<string, unknown> {
   return prototype === Object.prototype || prototype === null;
 }
 
+// Anything that is not a (non-null) object is either a structured-clone-safe primitive
+// or an unsupported value (e.g. a non-finite number, a function, a symbol). Isolated
+// from `isStrictlySerializable` so the recursive walk below stays flat.
+function isClonablePrimitive(value: unknown): boolean {
+  if (value === null || value === undefined) return true;
+  if (typeof value === "string" || typeof value === "boolean") return true;
+  if (typeof value === "number") return Number.isFinite(value);
+  return false;
+}
+
+// Date/Map/Set/RegExp are copied natively by structuredClone (not walked as plain
+// objects), so they are always structured-clone-safe regardless of their contents.
+const NATIVELY_CLONABLE_CTORS = [Date, Map, Set, RegExp] as const;
+function isNativelyClonable(value: object): boolean {
+  return NATIVELY_CLONABLE_CTORS.some((ctor) => value instanceof ctor);
+}
+
 // `seen` tracks only the current recursion PATH (ancestors), not every node ever visited:
 // add before descending, remove after returning. That way a real cycle (a node reachable
 // from itself) is still rejected, but two sibling branches that happen to reference the
 // SAME non-cyclic sub-object (a false positive with a globally-shared `seen` set) are not.
 export function isStrictlySerializable(value: unknown, seen = new Set<object>()): boolean {
-  if (
-    value === null ||
-    typeof value === "string" ||
-    typeof value === "boolean" ||
-    typeof value === "number"
-  ) {
-    return typeof value !== "number" || Number.isFinite(value);
-  }
-  if (typeof value !== "object") return false;
+  if (value === null || typeof value !== "object") return isClonablePrimitive(value);
   if (seen.has(value)) return false;
   seen.add(value);
   try {
     if (Array.isArray(value)) return value.every((entry) => isStrictlySerializable(entry, seen));
+    if (isNativelyClonable(value)) return true;
     if (!isPlainObject(value)) return false;
     return Object.values(value).every((entry) => isStrictlySerializable(entry, seen));
   } finally {

scripts/check/check-public-creds.mjs

@@ -90,18 +90,18 @@ const ENV_KEY_RE = /(clientId|clientSecret|apiKey)Env\s*:/;
 //   The MiniMax family was extracted from services/usage.ts into services/usage/minimax.ts
 //   (god-file decomposition), so the FP moved with the getMiniMaxUsage signature.
 //
-//   open-sse/executors/zcodeProtocol.ts L336: `clientId: \`omniroute-${process.pid}\``
+//   open-sse/executors/zcodeProtocol.ts L313: `clientId: \`omniroute-${process.pid}\``
 //   is the per-process identifier in the local ZCode app-server handshake. It is
 //   generated from the process PID, is not an upstream OAuth/client credential, and
 //   must remain visible in the wire contract. Frozen by file:line:value key.
 //   NOTE: the key includes the LINE, so any edit that shifts this statement breaks
 //   the gate twice over — a stale-entry error plus a "new violation" for the same
-//   literal. That happened again at L313 -> L336 (#13963, win32 shell:true spawn
-//   fix). Re-point the line; do not remove the entry.
+//   literal. That is what happened here (L302 -> L313). Re-point the line; do not
+//   remove the entry.
 export const KNOWN_LITERAL_CREDS = new Set([
   "open-sse/services/usage/minimax.ts:213:minimax", // TODO(6A.8): pre-existing FP — TS fn-param type, not a credential (getMiniMaxUsage signature)
   "open-sse/services/usage/minimax.ts:213:minimax-cn", // TODO(6A.8): pre-existing FP — TS fn-param type, not a credential (getMiniMaxUsage signature)
-  "open-sse/executors/zcodeProtocol.ts:336:omniroute-${process.pid}", // local per-process ZCode handshake ID, not an upstream credential
+  "open-sse/executors/zcodeProtocol.ts:313:omniroute-${process.pid}", // local per-process ZCode handshake ID, not an upstream credential
 ]);
 
 /**

tests/unit/check-public-creds.test.ts

@@ -68,10 +68,10 @@ test("allowlist freezes a literal by file:line:value key", () => {
 });
 
 test("allowlist preserves the local ZCode handshake client ID without weakening credential detection", () => {
-  // 335 newlines puts the statement on line 336, which is where it lives in
+  // 312 newlines puts the statement on line 313, which is where it lives in
   // zcodeProtocol.ts today. The allowlist key carries the line number, so this
-  // literal has to be kept in step with the source (it moved 302 -> 313 -> 336).
-  const src = `${"\n".repeat(335)}clientId: \`omniroute-\${process.pid}\`,`;
+  // literal has to be kept in step with the source (it moved 302 -> 313).
+  const src = `${"\n".repeat(312)}clientId: \`omniroute-\${process.pid}\`,`;
   assert.deepEqual(
     findLiteralCreds(src, KNOWN_LITERAL_CREDS, "open-sse/executors/zcodeProtocol.ts"),
     []

tests/unit/compression-worker-gate-13154-repro.test.ts

@@ -0,0 +1,69 @@
+import assert from "node:assert/strict";
+import { describe, it } from "node:test";
+import {
+  isCompressionWorkerEligible,
+  isStrictlySerializable,
+} from "../../open-sse/services/compression/compressionWorkerProtocol.ts";
+import type { CompressionConfig } from "../../open-sse/services/compression/types.ts";
+
+const body = {
+  model: "gpt-test",
+  messages: [{ role: "user", content: "hi" }],
+};
+const config = {
+  enabled: true,
+  defaultMode: "stacked",
+  autoTriggerTokens: 1,
+  cacheMinutes: 0,
+  preserveSystemPrompt: true,
+  stackedPipeline: [{ engine: "rtk" }, { engine: "caveman" }],
+} as CompressionConfig;
+
+describe("#13154: compression worker gate rejects structured-cloneable bodies", () => {
+  it("structuredClone accepts a workerOptions shape with an explicit `undefined` key", () => {
+    const workerOptions = {
+      model: "gpt-test",
+      supportsVision: true,
+      providerTransport: "direct" as const,
+      provider: undefined,
+      imageTransportFidelity: "unknown" as const,
+      sourceFormat: "chat" as const,
+      targetFormat: "chat" as const,
+      compressionStage: "pre-translation" as const,
+      config,
+    };
+    assert.doesNotThrow(() => structuredClone({ body, mode: "stacked", options: workerOptions }));
+  });
+
+  it("the gate should accept that same structured-cloneable shape", () => {
+    const workerOptions = {
+      model: "gpt-test",
+      supportsVision: true,
+      providerTransport: "direct" as const,
+      provider: undefined,
+      imageTransportFidelity: "unknown" as const,
+      sourceFormat: "chat" as const,
+      targetFormat: "chat" as const,
+      compressionStage: "pre-translation" as const,
+      config,
+    };
+    assert.equal(isStrictlySerializable({ body, mode: "stacked", options: workerOptions }), true);
+    assert.equal(isCompressionWorkerEligible(body, "stacked", workerOptions), true);
+  });
+
+  it("realistic runCompressionAsync-shaped call (only `provider` unset) should be eligible", () => {
+    const workerOptions = {
+      model: "gpt-test",
+      supportsVision: undefined,
+      providerTransport: undefined,
+      provider: undefined,
+      imageTransportFidelity: undefined,
+      sourceFormat: undefined,
+      targetFormat: undefined,
+      compressionStage: undefined,
+      config,
+    };
+    assert.doesNotThrow(() => structuredClone({ body, mode: "stacked", options: workerOptions }));
+    assert.equal(isCompressionWorkerEligible(body, "stacked", workerOptions), true);
+  });
+});

tests/unit/compression/compression-worker.test.ts

@@ -79,17 +79,8 @@ describe("compression worker eligibility", () => {
     }
   });
 
-  it("rejects functions, symbols, classes, special objects, cycles, and non-finite numbers", () => {
-    for (const value of [
-      () => undefined,
-      Symbol("x"),
-      new Date(),
-      new Map(),
-      new Set(),
-      /x/,
-      NaN,
-      Infinity,
-    ]) {
+  it("rejects functions, symbols, cycles, and non-finite numbers", () => {
+    for (const value of [() => undefined, Symbol("x"), NaN, Infinity]) {
       assert.equal(isStrictlySerializable(value), false);
     }
     const cyclic: Record<string, unknown> = {};
@@ -97,6 +88,34 @@ describe("compression worker eligibility", () => {
     assert.equal(isStrictlySerializable(cyclic), false);
   });
 
+  it("#13154: accepts structured-clone-native Date/Map/Set/RegExp values", () => {
+    for (const value of [new Date(), new Map(), new Set(), /x/]) {
+      assert.equal(isStrictlySerializable(value), true);
+    }
+  });
+
+  it("#13154: accepts `undefined` values instead of rejecting the whole tree", () => {
+    assert.equal(isStrictlySerializable(undefined), true);
+    assert.equal(isStrictlySerializable({ provider: undefined, model: "gpt-test" }), true);
+  });
+
+  it("#13154: accepts strategySelector.ts's exact 9-key workerOptions shape with `provider` unset", () => {
+    // Mirrors runCompressionAsync's workerOptions object: all 9 keys always present,
+    // `provider` commonly unresolved (undefined) at call time.
+    const workerOptions = {
+      model: "gpt-test",
+      supportsVision: undefined,
+      providerTransport: undefined,
+      provider: undefined,
+      imageTransportFidelity: undefined,
+      sourceFormat: undefined,
+      targetFormat: undefined,
+      compressionStage: undefined,
+      config,
+    };
+    assert.equal(isCompressionWorkerEligible(body, "stacked", workerOptions), true);
+  });
+
   it("#13154: does not misread a shared (non-cyclic) sub-object referenced by two sibling branches as a cycle", () => {
     // Original bug: a single `seen` set shared across the whole recursion tree (never
     // backtracked) meant visiting the SAME object twice via two different, non-cyclic

tests/unit/zcode-win32-spawn-13963.test.ts

@@ -1,78 +0,0 @@
-/**
- * Regression test for #13963 — Windows: `zc`/`zcode` provider always fails
- * with `spawn zcode ENOENT` even when ZCODE_BIN is set.
- *
- * Root cause: ZcodeAppServerClient.start() (open-sse/executors/zcodeProtocol.ts)
- * spawns the ZCode CLI with a hardcoded `shell: false`, regardless of
- * process.platform or the file extension of the resolved command. On
- * Windows, npm installs global CLI wrappers as `.cmd`/`.bat` shims, and
- * since Node's CVE-2024-27980 fix, `spawn()` refuses to launch a `.cmd`/
- * `.bat` target without `shell: true`, throwing ENOENT/EINVAL instead. This
- * is the same class of bug as #8590 (Qoder), already fixed elsewhere in
- * this repo (devin-cli.ts, auggie.ts, cliRuntime.ts's
- * shouldUseShellForCommand()).
- *
- * `shouldUseShellForZcodeCommand()` is a small, pure, exported helper so
- * this can be asserted directly without needing to intercept the live ESM
- * `spawn` binding (node:child_process.spawn is unmockable via mock.method()
- * without --experimental-test-module-mocks, which is not enabled in
- * `npm run test:unit` — see tests/unit/windows-hide-child-process-spawns-8131.test.ts).
- */
-
-import test from "node:test";
-import assert from "node:assert/strict";
-
-const { shouldUseShellForZcodeCommand } =
-  await import("@omniroute/open-sse/executors/zcodeProtocol");
-
-/** Temporarily override process.platform for the duration of `fn`. */
-function withPlatform<T>(platform: string, fn: () => T): T {
-  const original = Object.getOwnPropertyDescriptor(process, "platform")!;
-  Object.defineProperty(process, "platform", { value: platform, configurable: true });
-  try {
-    return fn();
-  } finally {
-    Object.defineProperty(process, "platform", original);
-  }
-}
-
-test("shouldUseShellForZcodeCommand returns true on win32 for a .cmd shim", () => {
-  const result = withPlatform("win32", () =>
-    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\AppData\\Roaming\\npm\\zcode.cmd")
-  );
-  assert.equal(
-    result,
-    true,
-    "spawn() must use shell:true on win32 when the resolved ZCode binary is a " +
-      ".cmd/.bat shim, or launching it throws ENOENT/EINVAL (Node CVE-2024-27980 fix) " +
-      "— see #8590 (Qoder) for the same class of bug already fixed elsewhere in this repo"
-  );
-});
-
-test("shouldUseShellForZcodeCommand returns true on win32 for a .bat shim", () => {
-  const result = withPlatform("win32", () =>
-    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\AppData\\Roaming\\npm\\zcode.bat")
-  );
-  assert.equal(result, true);
-});
-
-test("shouldUseShellForZcodeCommand stays false on win32 for the bundled node runtime path", () => {
-  // The ZCODE_SERVER_NODE bundled-runtime path (open-sse/executors/zcode.ts:96-101)
-  // spawns a bare `node`/`node.exe` binary, not a .cmd/.bat shim — must stay shell:false.
-  const result = withPlatform("win32", () =>
-    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\.zcode\\server\\node.exe")
-  );
-  assert.equal(result, false);
-});
-
-test("shouldUseShellForZcodeCommand returns false on linux even for a .cmd-named command", () => {
-  const result = withPlatform("linux", () =>
-    shouldUseShellForZcodeCommand("/usr/local/bin/zcode.cmd")
-  );
-  assert.equal(result, false);
-});
-
-test("shouldUseShellForZcodeCommand returns false on darwin for the plain zcode binary", () => {
-  const result = withPlatform("darwin", () => shouldUseShellForZcodeCommand("zcode"));
-  assert.equal(result, false);
-});