fix(mitm): remove duplicated 'Command failed:' error prefix (#3641) (#3662)

Closes #3641

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: <cmd>"` 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.

src/mitm/systemCommands.ts

@@ -20,7 +20,12 @@ export function execFileText(command: string, args: string[]): Promise<string> {
   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: <cmd>"
+        // (for non-zero exit) or "spawn <cmd> 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);

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: <command>" 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;
+    },
+  );
+});