mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-09 17:28:51 +00:00
fix(cli): fall back to settings.json when Claude Code binary is unresolvable (#6701)
getCliRuntimeStatus() only ever answered `installed` from binary resolution (known install paths + where/which PATH search), so a stale PATH, moved binary, or uncatalogued install method reported "not found" even when ~/.claude/settings.json proved the CLI was installed and used before — regressing behind upstream 9router's checkClaudeInstalled(), which already falls back to the settings file when where/which fails. withSettingsFallback() (new src/shared/services/cliInstallFallback.ts, kept out of the frozen cliRuntime.ts to respect its file-size ceiling) restores that parity: only when the binary lookup's own reason is "not_found" (never for deliberate security rejections like unsafe/relative env overrides or symlink escapes) and the tool's settings file exists on disk.
This commit is contained in:
parent
f4fb0d310b
commit
d245d5be77
4 changed files with 147 additions and 3 deletions
|
|
@ -49,6 +49,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
|||
- **fix(api):** editing any existing OpenAI Codex provider connection in the dashboard returned "Invalid request" and the edit could never be saved ([#6562](https://github.com/diegosouzapw/OmniRoute/issues/6562)) — `createProviderConnection()` (`src/lib/db/providers.ts`) auto-increments a new connection's `priority` to `MAX(priority)+1` per provider with no upper bound, and OAuth-imported connections (Codex `codex-auth/import` / `import-bulk`, up to 50 accounts per call, callable repeatedly — the standard Codex bulk-account-rotation workflow) never pass through `createProviderSchema`'s Zod validation at all, so nothing ever capped that value; `EditConnectionModal`'s `handleSubmit` always resends the connection's current `priority` unchanged on every save, and `updateProviderConnectionSchema` capped `priority`/`globalPriority` at `max(100)` — a UI-only ceiling the create path never enforced — so the first edit of any connection whose priority had already grown past 100 (routine once a Codex account count exceeds 100) failed validation regardless of which field the user actually changed. Raised the ceiling to `max(100_000)` on both fields — still bounded (a genuinely out-of-range value is still rejected), just wide enough to accept priorities the app itself already produces. Regression guard: `tests/unit/codex-connection-edit-6562.test.ts` (a Codex OAuth connection whose priority already exceeds the old 100 cap now validates + persists on edit; a control payload with a still-genuinely-invalid priority is still rejected with "Invalid request").
|
||||
- **mimocode**: rotate accounts on MiMoCode's rate-limit-style 400s (body-classified) instead of failing on the first account; malformed 400s still fail fast with the real upstream error (#6648 — thanks @pizzav-xyz)
|
||||
- **fix(cli):** compression CLI REST fallback now reads/writes the canonical `defaultMode` field (surfaced as `strategy`) instead of a nonexistent `engine` key, and table output renders nested objects as JSON instead of `[object Object]` (#6571 — thanks @charleszolot)
|
||||
- **fix(cli):** the dashboard's Claude Code CLI card could report "Not detected"/"Not installed" even when Claude Code was genuinely installed and previously used ([#6701](https://github.com/diegosouzapw/OmniRoute/issues/6701)) — `getCliRuntimeStatus()` (`src/shared/services/cliRuntime.ts`) determined `installed` purely from binary resolution (known install paths + a `where`/`which` PATH search), with no fallback when that lookup fails for reasons unrelated to whether the CLI is actually installed (stale PATH inherited by a long-running/background process, the binary having moved, an install method not yet catalogued, etc.) — even though `~/.claude/settings.json` on disk proves the tool was installed and used before. Upstream 9router's equivalent route already has this exact fallback. A new `withSettingsFallback()` (`src/shared/services/cliInstallFallback.ts`) restores 9router parity: when the binary lookup's own reason is `"not_found"` (never for deliberate security rejections like unsafe/relative env overrides or symlink escapes) and the tool's settings file exists on disk, `installed` now reports `true`. Regression guard: `tests/unit/repro-6701-claude-detect-fallback.test.ts`.
|
||||
|
||||
### 📝 Maintenance
|
||||
|
||||
|
|
|
|||
69
src/shared/services/cliInstallFallback.ts
Normal file
69
src/shared/services/cliInstallFallback.ts
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
import fsSync from "fs";
|
||||
|
||||
/**
|
||||
* #6701 — 9router-parity fallback for CLI install detection.
|
||||
*
|
||||
* `getCliRuntimeStatus()` in `cliRuntime.ts` determines `installed` from
|
||||
* binary resolution alone (known install paths + a `where`/`which` PATH
|
||||
* search). If the binary is not currently resolvable — stale PATH inherited
|
||||
* by a long-running/background OmniRoute process, the binary having moved,
|
||||
* or an install method we don't enumerate yet — it used to unconditionally
|
||||
* report `installed:false`, even when the tool's own settings/config file on
|
||||
* disk proves it was installed and used before.
|
||||
*
|
||||
* Upstream 9router's equivalent route
|
||||
* (`src/app/api/cli-tools/claude-settings/route.js::checkClaudeInstalled()`)
|
||||
* has a second-chance fallback: when `where`/`which` fails, it still reports
|
||||
* `installed:true` if the settings file exists. This restores that fallback
|
||||
* for any CLI tool that declares a `settings` config path (currently
|
||||
* `claude` and `droid` — see `CLI_TOOLS` in `cliRuntime.ts`).
|
||||
*
|
||||
* Only applies when the lookup's own reason is "not_found" — i.e. the binary
|
||||
* genuinely couldn't be located on PATH/known install paths. Deliberate
|
||||
* security rejections (unsafe/relative env override paths, symlink escapes,
|
||||
* suspicious file sizes, etc.) must stay `installed:false` regardless of
|
||||
* whether a settings file happens to exist.
|
||||
*/
|
||||
export interface NotInstalledResult {
|
||||
installed: false;
|
||||
runnable: boolean;
|
||||
command: string | null;
|
||||
commandPath: string | null;
|
||||
reason: string;
|
||||
runtimeMode: string;
|
||||
requiresBinary: boolean;
|
||||
}
|
||||
|
||||
export interface SettingsFallbackResult {
|
||||
installed: true;
|
||||
runnable: false;
|
||||
command: string | null;
|
||||
commandPath: null;
|
||||
reason: "settings_found_binary_unresolved";
|
||||
runtimeMode: string;
|
||||
requiresBinary: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Given the resolved settings-file path for a tool (or undefined if the tool
|
||||
* has none) and the "not installed" result the binary lookup already
|
||||
* produced, return a settings-fallback result when the settings file exists
|
||||
* on disk, or the original "not installed" result unchanged otherwise.
|
||||
*/
|
||||
export const withSettingsFallback = (
|
||||
settingsPath: string | undefined,
|
||||
notInstalledResult: NotInstalledResult
|
||||
): NotInstalledResult | SettingsFallbackResult => {
|
||||
if (notInstalledResult.reason !== "not_found") return notInstalledResult;
|
||||
if (!settingsPath || !fsSync.existsSync(settingsPath)) return notInstalledResult;
|
||||
|
||||
return {
|
||||
installed: true,
|
||||
runnable: false,
|
||||
command: notInstalledResult.command,
|
||||
commandPath: null,
|
||||
reason: "settings_found_binary_unresolved",
|
||||
runtimeMode: notInstalledResult.runtimeMode,
|
||||
requiresBinary: notInstalledResult.requiresBinary,
|
||||
};
|
||||
};
|
||||
|
|
@ -5,7 +5,7 @@ import path from "path";
|
|||
import { spawn, execFileSync } from "child_process";
|
||||
import { getHermesHome } from "@/lib/cli-helper/config-generator/hermesHome";
|
||||
import { getCachedLoginShellPath, mergeShellPath } from "./loginShellPath";
|
||||
|
||||
import { withSettingsFallback } from "./cliInstallFallback";
|
||||
const VALID_RUNTIME_MODES = new Set(["auto", "host", "container"]);
|
||||
const FALSE_VALUES = new Set(["0", "false", "no", "off"]);
|
||||
|
||||
|
|
@ -1057,7 +1057,7 @@ export const getCliRuntimeStatus = async (toolId: string) => {
|
|||
const command = located.command;
|
||||
|
||||
if (!located.installed) {
|
||||
return {
|
||||
return withSettingsFallback(getCliConfigPaths(toolId)?.settings, {
|
||||
installed: false,
|
||||
runnable: false,
|
||||
command,
|
||||
|
|
@ -1065,7 +1065,7 @@ export const getCliRuntimeStatus = async (toolId: string) => {
|
|||
reason: located.reason || "not_found",
|
||||
runtimeMode,
|
||||
requiresBinary,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
if (located.reason === "not_executable") {
|
||||
|
|
|
|||
74
tests/unit/repro-6701-claude-detect-fallback.test.ts
Normal file
74
tests/unit/repro-6701-claude-detect-fallback.test.ts
Normal file
|
|
@ -0,0 +1,74 @@
|
|||
/**
|
||||
* Repro for #6701 — Claude Code CLI reported "not found" in OmniRoute's
|
||||
* dashboard even though the user has used it before (settings.json present)
|
||||
* and upstream 9router (same machine, same settings.json) reports it as
|
||||
* "Connected".
|
||||
*
|
||||
* Root cause: `getCliRuntimeStatus()` in src/shared/services/cliRuntime.ts
|
||||
* only ever answers `installed` from binary resolution (known install paths
|
||||
* + PATH lookup via `where.exe`/`command -v`). If the CLI binary is not
|
||||
* currently resolvable (stale PATH inherited by a long-running/background
|
||||
* OmniRoute process, binary moved, etc.) it unconditionally reports
|
||||
* installed:false — even when `~/.claude/settings.json` proves the tool was
|
||||
* installed and used before.
|
||||
*
|
||||
* Upstream 9router's equivalent route (src/app/api/cli-tools/claude-settings/route.js)
|
||||
* has a second-chance fallback: if `where`/`which` fails, it still reports
|
||||
* installed:true when the settings file exists on disk. OmniRoute's rewrite
|
||||
* into cliRuntime.ts dropped that fallback, which is the concrete regression
|
||||
* relative to 9router this issue's screenshots capture.
|
||||
*
|
||||
* This test forces the binary lookup to fail deterministically (CLI_CLAUDE_BIN
|
||||
* pointed at a path that does not exist) while a real settings.json sits under
|
||||
* an isolated CLI_CONFIG_HOME. Expected (post-fix, 9router-parity) behavior:
|
||||
* installed should stay true because the settings file is present. Current
|
||||
* code returns installed:false / reason:"not_found" — this is the RED proof.
|
||||
*/
|
||||
|
||||
import { describe, it, before, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const { getCliRuntimeStatus } = await import("../../src/shared/services/cliRuntime.ts");
|
||||
|
||||
describe("#6701 — claude detection should fall back to settings.json when binary is unresolvable", () => {
|
||||
let configHome: string;
|
||||
const prevBin = process.env.CLI_CLAUDE_BIN;
|
||||
const prevConfigHome = process.env.CLI_CONFIG_HOME;
|
||||
|
||||
before(() => {
|
||||
// Isolated config home *within* os.homedir() (CLI_CONFIG_HOME validation
|
||||
// requires this) so we never touch the real ~/.claude directory.
|
||||
configHome = fs.mkdtempSync(path.join(os.homedir(), ".omniroute-test-6701-"));
|
||||
const claudeDir = path.join(configHome, ".claude");
|
||||
fs.mkdirSync(claudeDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(claudeDir, "settings.json"),
|
||||
JSON.stringify({ env: { ANTHROPIC_BASE_URL: "http://localhost:20128" } }, null, 2)
|
||||
);
|
||||
|
||||
// Force the binary lookup to fail deterministically regardless of host state.
|
||||
process.env.CLI_CLAUDE_BIN = path.join(os.tmpdir(), "definitely-not-a-real-claude-binary-6701");
|
||||
process.env.CLI_CONFIG_HOME = configHome;
|
||||
});
|
||||
|
||||
after(() => {
|
||||
fs.rmSync(configHome, { recursive: true, force: true });
|
||||
if (prevBin === undefined) delete process.env.CLI_CLAUDE_BIN;
|
||||
else process.env.CLI_CLAUDE_BIN = prevBin;
|
||||
if (prevConfigHome === undefined) delete process.env.CLI_CONFIG_HOME;
|
||||
else process.env.CLI_CONFIG_HOME = prevConfigHome;
|
||||
});
|
||||
|
||||
it("reports installed:true when settings.json exists, even if the binary can't be resolved", async () => {
|
||||
const result = await getCliRuntimeStatus("claude");
|
||||
|
||||
assert.equal(
|
||||
result.installed,
|
||||
true,
|
||||
`Expected installed:true (9router-parity settings.json fallback), got installed:${result.installed} reason:${result.reason}`
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue