diff --git a/CHANGELOG.md b/CHANGELOG.md index 5890bc4d29..d9d8bc60f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ ### 🔧 Bug Fixes +- **MITM/cert**: remove the duplicated "Command failed:" prefix in system-command error messages ([#3641](https://github.com/diegosouzapw/OmniRoute/issues/3641)): `execFileText` was prepending its own `"Command failed: "` prefix on top of Node's `execFile` error message, which already begins with `"Command failed: "` for non-zero exits. The error message now surfaces Node's message directly (no double prefix), with stderr appended only when non-empty. + - **fix(reasoning): replay `reasoning_content` on plain DeepSeek turns** ([#3632] — thanks @adivekar-utexas): the reasoning-replay gate previously only fired when an assistant message already carried `reasoning_content`. Plain (non-tool-call) turns whose `reasoning_content` was stripped by the client (e.g. Cursor) were forwarded without it, so DeepSeek V4+ rejected the request with 400 "the reasoning_content in the thinking mode must be passed back". The gate now also covers missing/empty `reasoning_content` on DeepSeek replay targets, injecting the cached reasoning (or the non-Anthropic placeholder) so multi-turn text conversations no longer 400. Fixes #1682. 2 regression tests. - **fix(kiro): route enterprise IAM Identity Center accounts to their regional endpoint** ([#3631] — thanks @artickc): Kiro/CodeWhisperer access tokens and Q Developer profile ARNs are region-bound, so enterprise IAM Identity Center accounts outside `us-east-1` (e.g. `eu-central-1`) were rejected by the default host. Adds `resolveKiroRegion` (stored region → profileArn region → `us-east-1`) and `kiroRuntimeHost` (regional `q.{region}.amazonaws.com`, legacy `codewhisperer.us-east-1` for the default), routes chat + usage to the regional endpoint, and discovers the region-matched `profileArn` via `ListAvailableProfiles` in a best-effort `postExchange` hook. 9 tests. - **fix(combo): skip same-provider/connection targets on connection-level errors** ([#3637] — thanks @herjarsa): on connection-level upstream errors (408/500/502/503/504/524), remaining same-`provider:connection` targets in a combo request are now skipped to avoid hammering a known-bad connection, in both the priority and round-robin paths. Adjusted in review to **exclude OmniRoute circuit-breaker-open responses** (503 + `X-OmniRoute-Provider-Breaker` / `provider_circuit_open`) from this skip, preserving the invariant that a breaker-open is an ordinary target failure (the next same-provider target is still tried). Co-authored with @herjarsa. diff --git a/src/mitm/systemCommands.ts b/src/mitm/systemCommands.ts index bbf3327d64..45e1eac4ff 100644 --- a/src/mitm/systemCommands.ts +++ b/src/mitm/systemCommands.ts @@ -20,7 +20,12 @@ export function execFileText(command: string, args: string[]): Promise { return new Promise((resolve, reject) => { execFile(command, args, { encoding: "utf8" }, (error, stdout, stderr) => { if (error) { - reject(new Error(`Command failed: ${getErrorMessage(error)}\n${stderr}`)); + // Node's execFile already sets error.message to "Command failed: " + // (for non-zero exit) or "spawn ENOENT" (for missing binary). + // Re-prefixing with "Command failed: " would double the prefix for the + // non-zero exit case. Surface Node's message directly and only append + // stderr when it contains additional context. (#3641) + reject(new Error(getErrorMessage(error) + (stderr ? `\n${stderr}` : ""))); return; } resolve(stdout); diff --git a/tests/unit/mitm-system-commands-prefix-3641.test.ts b/tests/unit/mitm-system-commands-prefix-3641.test.ts new file mode 100644 index 0000000000..1b32a7a437 --- /dev/null +++ b/tests/unit/mitm-system-commands-prefix-3641.test.ts @@ -0,0 +1,55 @@ +/** + * Regression test for #3641 — `execFileText` produces a doubled + * "Command failed: Command failed: ..." prefix in error messages. + * + * Node's `execFile` already sets `error.message` to + * "Command failed: " when the child process exits non-zero. + * `execFileText` was prepending its own "Command failed: " prefix on top of + * that, producing the doubled string. + * + * The fix: surface `getErrorMessage(error)` directly (Node's message already + * contains the command), only appending stderr when it is non-empty. + * + * Uses `/bin/false` (always exits 1) as the deterministic failing command on + * Linux/macOS. This is the only case that triggers "Command failed: ..." in + * Node's error.message (ENOENT from a missing binary uses "spawn ... ENOENT" + * instead, so it never doubles). + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFileText } from "../../src/mitm/systemCommands.ts"; + +// `/bin/false` exits with code 1 — guaranteed to produce a "Command failed:" +// error.message from Node's execFile, which is exactly the case that was +// being doubled by the bug. +const FALSE_CMD = "/bin/false"; + +test("execFileText: error message does NOT contain a doubled 'Command failed:' prefix", async () => { + await assert.rejects( + () => execFileText(FALSE_CMD, []), + (err: unknown) => { + assert.ok(err instanceof Error, "expected an Error"); + const msg = err.message; + assert.ok( + !msg.includes("Command failed: Command failed:"), + `Error message contains doubled prefix: ${JSON.stringify(msg)}`, + ); + return true; + }, + ); +}); + +test("execFileText: error message for a non-zero exit still contains 'Command failed:'", async () => { + await assert.rejects( + () => execFileText(FALSE_CMD, []), + (err: unknown) => { + assert.ok(err instanceof Error, "expected an Error"); + // The message should still contain the Node-generated prefix once. + assert.ok( + err.message.includes("Command failed:"), + `Error message should still contain "Command failed:": ${JSON.stringify(err.message)}`, + ); + return true; + }, + ); +});