mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-05 23:02:10 +03:00
This commit is contained in:
committed by
GitHub
parent
76b1b04495
commit
c50a83a94d
@@ -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)
|
||||
|
||||
@@ -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<QoderCliRunResult> {
|
||||
const command = String(options.command || "").trim() || getQoderCliCommand();
|
||||
async function spawnQoderCli(options: SpawnQoderCliOptions): Promise<QoderCliRunResult> {
|
||||
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<QoderCliRunResult
|
||||
env,
|
||||
cwd: options.cwd || undefined,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
...(useShell ? { shell: true } : {}),
|
||||
});
|
||||
} catch (err) {
|
||||
resolve({
|
||||
@@ -944,10 +946,7 @@ export async function validateQoderCliPat({
|
||||
if (run.error && /enoent|not found|no such file|spawn/i.test(run.error)) {
|
||||
return {
|
||||
valid: false,
|
||||
error:
|
||||
`Qoder CLI (qodercli) was not found on the OmniRoute host (${run.error}). ` +
|
||||
"Install it from https://qoder.com or point CLI_QODER_BIN at the binary. " +
|
||||
"PAT auth is driven through the local qodercli binary.",
|
||||
error: buildQoderCliNotFoundHint(run.error),
|
||||
unsupported: false,
|
||||
};
|
||||
}
|
||||
|
||||
111
open-sse/services/qoderCliResolve.ts
Normal file
111
open-sse/services/qoderCliResolve.ts
Normal file
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* qodercli command resolution (#6263).
|
||||
*
|
||||
* Extracted from `qoderCli.ts` (frozen at the file-size baseline) so the
|
||||
* Windows-aware resolution logic can grow without bloating the transport module.
|
||||
*
|
||||
* The bare `"qodercli"` name does not resolve on Windows, where npm installs the
|
||||
* CLI as a `qodercli.cmd` wrapper under `%APPDATA%\npm` (a user-PATH directory)
|
||||
* that `spawn` cannot find with `shell:false` and an unenriched env. OmniRoute
|
||||
* already has a Windows-aware resolver for this exact tool in `cliRuntime.ts`, so
|
||||
* we reuse it: `getCliRuntimeStatus("qoder")` returns an absolute `.cmd`/`.exe`
|
||||
* `commandPath`, and `shouldUseShellForCommand()` tells us whether it needs cmd.exe.
|
||||
*/
|
||||
import {
|
||||
getCliRuntimeStatus,
|
||||
getKnownToolPaths,
|
||||
shouldUseShellForCommand,
|
||||
} from "@/shared/services/cliRuntime";
|
||||
|
||||
export function getQoderCliCommand(): string {
|
||||
const explicit = String(process.env.CLI_QODER_BIN || "").trim();
|
||||
return explicit || "qodercli";
|
||||
}
|
||||
|
||||
export type QoderCliInvocation = { command: string; useShell: boolean };
|
||||
|
||||
// Resolving through cliRuntime does synchronous fs walks plus a `--version`
|
||||
// healthcheck spawn; memoize the result so we don't repeat that on every chat /
|
||||
// quota request. The install location is effectively static for a running host.
|
||||
const QODER_RESOLVE_TTL_MS = 5 * 60 * 1000;
|
||||
// Keyed on the fallback command (which folds in CLI_QODER_BIN) so changing the
|
||||
// override — or a test pointing at a fresh stub — invalidates a stale entry
|
||||
// instead of spawning a since-deleted binary.
|
||||
let qoderInvocationCache: (QoderCliInvocation & { key: string; expiresAt: number }) | null = null;
|
||||
|
||||
/** Test-only: drop the memoized qodercli command resolution. */
|
||||
export function __clearQoderCliInvocationCache(): void {
|
||||
qoderInvocationCache = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the exact command + shell flag to spawn qodercli. `deps` is injectable
|
||||
* for unit tests; production uses the real cliRuntime exports.
|
||||
*/
|
||||
export async function resolveQoderCliInvocation(
|
||||
explicitCommand?: string | null,
|
||||
deps: {
|
||||
getStatus?: typeof getCliRuntimeStatus;
|
||||
shouldUseShell?: typeof shouldUseShellForCommand;
|
||||
} = {}
|
||||
): Promise<QoderCliInvocation> {
|
||||
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."
|
||||
);
|
||||
}
|
||||
@@ -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)
|
||||
);
|
||||
|
||||
125
tests/unit/qodercli-windows-resolve-6263.test.ts
Normal file
125
tests/unit/qodercli-windows-resolve-6263.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user