diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e725b1ba4..68c6fbc6e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ ### 🐛 Bug Fixes +- **fix(providers):** qodercli PAT auth no longer fails with `spawn qodercli ENOENT` on Windows ([#6263](https://github.com/diegosouzapw/OmniRoute/issues/6263)) — `spawnQoderCli` spawned the bare `qodercli` name with `shell:false` and an unenriched env, so the npm `.cmd` wrapper under `%APPDATA%\npm` (a user-PATH directory) was never resolved. It now resolves the absolute `.cmd`/`.exe` path through the existing `getCliRuntimeStatus("qoder")` resolver in `src/shared/services/cliRuntime.ts` (memoized), spawns with `shell` when the target is a `.cmd`/`.bat`, and uses the cliRuntime-enriched env (PATH + PATHEXT + APPDATA); the ENOENT error now lists the searched paths plus the `CLI_QODER_BIN` override. End-to-end spawn on a real Windows host is host-only (Hard Rule #18); the path-resolution logic is unit-tested. Regression guard: `tests/unit/qodercli-windows-resolve-6263.test.ts`. (thanks @chirag127) + - **fix(sse):** the reasoning-token buffer no longer inflates **probe-sized `max_tokens`** ([#6274](https://github.com/diegosouzapw/OmniRoute/issues/6274)) — Claude Code's `/model` capability check sends `max_tokens: 1`, but for a thinking-capable model with a large output cap (e.g. `glm-5.2`) the #3587 headroom heuristic (`max(current + 1000, ceil(current * 1.5))`) rewrote it to `1001` and forwarded that upstream, wasting tokens on a request that was never a genuine reasoning budget. `resolveReasoningBufferedMaxTokens()` (`open-sse/services/reasoningTokenBuffer.ts`) now short-circuits and returns the caller's value verbatim when it is below the new `REASONING_BUFFER_MIN_TRIGGER` (256) threshold — a tiny explicit limit is a probe, not a reasoning request. Real budgets still receive the #3587 headroom unchanged, and the guard runs after the existing capability checks so unknown / non-reasoning models keep returning `null`. Regression guard: `tests/unit/reasoning-token-buffer-6274.test.ts`. (thanks @brightfiscalband) - **fix(cli):** `omniroute reset-password` now works as a real subcommand, and password resets over piped (non-TTY) stdin actually apply ([#6261](https://github.com/diegosouzapw/OmniRoute/issues/6261), [#6258](https://github.com/diegosouzapw/OmniRoute/issues/6258)). Two coupled defects: (1) **#6261** — `bin/omniroute.mjs` routed everything through Commander with only two pre-Commander bypasses (`--mcp`, `reset-encrypted-columns`), so `omniroute reset-password` was rejected as an unknown command; only the separate `omniroute-reset-password` bin worked, while the docs falsely advertised the subcommand (incl. a bogus "legacy alias still works"). A pre-Commander bypass mirroring `reset-encrypted-columns` now dynamically imports `bin/reset-password.mjs` (which self-executes) before Commander parses; the three doc lines were corrected. (2) **#6258** — `bin/reset-password.mjs` issued two sequential `rl.question` prompts; under piped stdin the second read never settled at EOF, so `main()` never reached `resetManagementPassword` and the reset was a silent no-op (both prompts printed, no success, password unchanged). The CLI now detects non-TTY stdin and reads it once (first line = password, second line = confirm if present, else reused), adds a `--password-stdin` flag (entire stdin is the password, no confirmation), and exits `0` explicitly so the success line always flushes; interactive TTY behavior is unchanged. Regression guard: `tests/unit/reset-password-cli-6261-6258.test.ts` (3). (thanks @chirag127) diff --git a/open-sse/services/qoderCli.ts b/open-sse/services/qoderCli.ts index 3aa110eee9..bccd50de6d 100644 --- a/open-sse/services/qoderCli.ts +++ b/open-sse/services/qoderCli.ts @@ -3,6 +3,9 @@ import crypto from "crypto"; import fs from "fs"; import os from "os"; import path from "path"; +import { getLookupEnv } from "@/shared/services/cliRuntime"; +import { buildQoderCliNotFoundHint, resolveQoderCliInvocation } from "./qoderCliResolve"; +export { getQoderCliCommand } from "./qoderCliResolve"; // #6263 public entry point const DEFAULT_TIMEOUT_MS = 45_000; const DEFAULT_MODELS_TIMEOUT_MS = 20_000; @@ -63,11 +66,6 @@ function getString(value: unknown): string { return typeof value === "string" ? value : ""; } -export function getQoderCliCommand(): string { - const explicit = String(process.env.CLI_QODER_BIN || "").trim(); - return explicit || "qodercli"; -} - export function getQoderCliWorkspace(): string { const explicit = String( process.env.QODER_CLI_WORKSPACE || process.env.OMNIROUTE_QODER_WORKSPACE || "" @@ -124,10 +122,13 @@ type SpawnQoderCliOptions = { * honors for headless PAT auth — and the prompt is piped through stdin so no * untrusted value is ever interpolated into a shell command (Hard Rule #13). */ -function spawnQoderCli(options: SpawnQoderCliOptions): Promise { - const command = String(options.command || "").trim() || getQoderCliCommand(); +async function spawnQoderCli(options: SpawnQoderCliOptions): Promise { const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; - const env: NodeJS.ProcessEnv = { ...process.env }; + // #6263: resolve the real qodercli command (absolute .cmd/.exe on Windows) and + // whether it needs a shell, then spawn with the cliRuntime-enriched env (PATH + + // PATHEXT + APPDATA) so the npm `.cmd` wrapper under %APPDATA%\npm is found. + const { command, useShell } = await resolveQoderCliInvocation(options.command); + const env: NodeJS.ProcessEnv = { ...getLookupEnv() }; const token = String(options.token || "").trim(); if (token) env.QODER_PERSONAL_ACCESS_TOKEN = token; @@ -143,6 +144,7 @@ function spawnQoderCli(options: SpawnQoderCliOptions): Promise { + const explicit = String(explicitCommand || "").trim(); + const getStatus = deps.getStatus || getCliRuntimeStatus; + const shouldUseShell = deps.shouldUseShell || shouldUseShellForCommand; + // Only the default path is cached; an explicit per-call command or an injected + // resolver (tests) always resolves fresh and never touches the shared cache. + const cacheable = !explicit && !deps.getStatus && !deps.shouldUseShell; + const fallback = explicit || getQoderCliCommand(); + + if ( + cacheable && + qoderInvocationCache && + qoderInvocationCache.key === fallback && + qoderInvocationCache.expiresAt > Date.now() + ) { + return { command: qoderInvocationCache.command, useShell: qoderInvocationCache.useShell }; + } + + let command = fallback; + try { + const status = await getStatus("qoder"); + if (status && status.installed && status.commandPath) { + command = status.commandPath; + } + } catch { + /* fall back to the bare/explicit command — spawn will surface a real ENOENT */ + } + + const invocation: QoderCliInvocation = { command, useShell: shouldUseShell(command) }; + if (cacheable) { + qoderInvocationCache = { + ...invocation, + key: fallback, + expiresAt: Date.now() + QODER_RESOLVE_TTL_MS, + }; + } + return invocation; +} + +/** + * Build the operator-facing "qodercli not found" error, listing the paths the + * resolver searched plus the `CLI_QODER_BIN` override hint (#6263). + */ +export function buildQoderCliNotFoundHint(runError: string): string { + let searchedHint = ""; + try { + const candidates = getKnownToolPaths("qoder"); + if (candidates.length > 0) { + searchedHint = ` Searched: ${candidates.slice(0, 6).join(", ")}.`; + } + } catch { + /* best-effort — the path list is only advisory for the error message */ + } + return ( + `Qoder CLI (qodercli) was not found on the OmniRoute host (${runError}).` + + searchedHint + + " Install it from https://qoder.com, or set CLI_QODER_BIN to the absolute path " + + "of the qodercli binary (e.g. %APPDATA%\\npm\\qodercli.cmd on Windows). " + + "PAT auth is driven through the local qodercli binary." + ); +} diff --git a/src/shared/services/cliRuntime.ts b/src/shared/services/cliRuntime.ts index d2a37a1f53..737d7639c4 100644 --- a/src/shared/services/cliRuntime.ts +++ b/src/shared/services/cliRuntime.ts @@ -533,7 +533,7 @@ const getExtraPaths = () => * Checks npm global prefix, NVM locations, standalone installer paths. * Works on all platforms — Windows checks .cmd wrappers, Linux/macOS checks bare names. */ -const getKnownToolPaths = (toolId: string): string[] => { +export const getKnownToolPaths = (toolId: string): string[] => { const home = os.homedir(); const paths: string[] = []; @@ -658,7 +658,7 @@ const getNvmNodePath = (): string | null => { return null; }; -const getLookupEnv = () => { +export const getLookupEnv = () => { const env = { ...process.env }; const extraPaths = getExtraPaths(); const basePath = env.PATH || env.Path || ""; @@ -1081,7 +1081,7 @@ export const getCliRuntimeStatus = async (toolId: string) => { } const healthcheck = await checkRunnable( - located.commandPath, + located.commandPath || command || "", // located + executable ⇒ commandPath set env, Number(tool.healthcheckTimeoutMs || 4000) ); diff --git a/tests/unit/qodercli-windows-resolve-6263.test.ts b/tests/unit/qodercli-windows-resolve-6263.test.ts new file mode 100644 index 0000000000..e9826fc301 --- /dev/null +++ b/tests/unit/qodercli-windows-resolve-6263.test.ts @@ -0,0 +1,125 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import os from "node:os"; +import path from "node:path"; + +// #6263 — On Windows, Qoder PAT auth failed with `spawn qodercli ENOENT` even +// though `qodercli.cmd` was installed under `%APPDATA%\npm` and worked from a +// shell. Root cause: `spawnQoderCli` spawned the bare `"qodercli"` name with +// `shell:false` and an unenriched env, so the npm `.cmd` wrapper on the user PATH +// was never resolved. The fix routes command resolution through the already +// Windows-aware `src/shared/services/cliRuntime.ts` (which enumerates +// `qodercli.cmd`/`.exe` under npm-global + `%APPDATA%\npm`, resolves an absolute +// `commandPath`, and flags `.cmd`/`.bat` as needing a shell). +// +// This suite exercises the *pure* resolution logic with mocks; the end-to-end +// spawn of a real `qodercli.cmd` can only be validated on a real Windows host +// (fs realpath/stat security checks + the frozen expected-parent list). + +const qoderResolve = await import("../../open-sse/services/qoderCliResolve.ts"); +const cliRuntime = await import("../../src/shared/services/cliRuntime.ts"); + +const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); +const originalAppData = process.env.APPDATA; +const originalQoderBin = process.env.CLI_QODER_BIN; + +function setPlatform(value: string) { + Object.defineProperty(process, "platform", { configurable: true, value }); +} + +test.afterEach(() => { + if (originalPlatformDescriptor) { + Object.defineProperty(process, "platform", originalPlatformDescriptor); + } + if (originalAppData === undefined) delete process.env.APPDATA; + else process.env.APPDATA = originalAppData; + if (originalQoderBin === undefined) delete process.env.CLI_QODER_BIN; + else process.env.CLI_QODER_BIN = originalQoderBin; + qoderResolve.__clearQoderCliInvocationCache(); +}); + +test("cliRuntime enumerates qodercli.cmd under %APPDATA%\\npm on Windows", () => { + setPlatform("win32"); + // APPDATA must live inside the home dir to pass cliRuntime's env-path validation. + const appData = path.join(os.homedir(), "AppData", "Roaming"); + process.env.APPDATA = appData; + + const candidates = cliRuntime.getKnownToolPaths("qoder"); + const expected = path.join(appData, "npm", "qodercli.cmd"); + + assert.ok( + candidates.includes(expected), + `expected getKnownToolPaths("qoder") to include ${expected}, got: ${candidates.join(", ")}` + ); +}); + +test("shouldUseShellForCommand: true for a .cmd wrapper, false for the bare name (Windows)", () => { + setPlatform("win32"); + const cmdPath = path.join(os.homedir(), "AppData", "Roaming", "npm", "qodercli.cmd"); + assert.equal(cliRuntime.shouldUseShellForCommand(cmdPath), true); + assert.equal(cliRuntime.shouldUseShellForCommand("qodercli"), false); +}); + +test("resolveQoderCliInvocation picks the absolute .cmd path + shell when cliRuntime finds it", async () => { + setPlatform("win32"); + const cmdPath = path.join(os.homedir(), "AppData", "Roaming", "npm", "qodercli.cmd"); + + const invocation = await qoderResolve.resolveQoderCliInvocation(null, { + getStatus: async () => ({ + installed: true, + runnable: true, + command: "qodercli", + commandPath: cmdPath, + reason: null, + runtimeMode: "auto", + requiresBinary: true, + }), + }); + + // The bug was spawning the bare "qodercli"; the fix must select the resolved + // absolute .cmd path and mark it as needing a shell (cmd.exe). + assert.equal(invocation.command, cmdPath); + assert.equal(invocation.useShell, true); + assert.notEqual(invocation.command, "qodercli"); +}); + +test("resolveQoderCliInvocation falls back to the bare command when cliRuntime finds nothing", async () => { + setPlatform("win32"); + delete process.env.CLI_QODER_BIN; + + const invocation = await qoderResolve.resolveQoderCliInvocation(null, { + getStatus: async () => ({ + installed: false, + runnable: false, + command: "qodercli", + commandPath: null, + reason: "not_found", + runtimeMode: "auto", + requiresBinary: true, + }), + }); + + assert.equal(invocation.command, "qodercli"); + // A bare name is not a .cmd/.bat, so no shell is requested. + assert.equal(invocation.useShell, false); +}); + +test("resolveQoderCliInvocation is inert on non-Windows (no shell, resolved path honored)", async () => { + setPlatform("linux"); + const posixPath = "/usr/local/bin/qodercli"; + + const invocation = await qoderResolve.resolveQoderCliInvocation(null, { + getStatus: async () => ({ + installed: true, + runnable: true, + command: "qodercli", + commandPath: posixPath, + reason: null, + runtimeMode: "auto", + requiresBinary: true, + }), + }); + + assert.equal(invocation.command, posixPath); + assert.equal(invocation.useShell, false); +});