mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(cli): refresh runtime detection accurately (#7552)
Co-authored-by: nguyenha935 <208228297+nguyenha935@users.noreply.github.com>
This commit is contained in:
1
changelog.d/fixes/pending-cli-service-detection.md
Normal file
1
changelog.d/fixes/pending-cli-service-detection.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(cli):** CLI detection now refreshes stale cached results, reports discovered versions, and checks the Continue `cn` binary instead of assuming it is installed.
|
||||
@@ -92,6 +92,7 @@ export async function GET(request: Request): Promise<Response> {
|
||||
if (authError) return authError;
|
||||
|
||||
try {
|
||||
const forceRefresh = new URL(request.url).searchParams.get("refresh") === "true";
|
||||
const toolIds = Object.keys(CLI_TOOLS);
|
||||
const statuses: ToolBatchStatusMap = {};
|
||||
|
||||
@@ -117,7 +118,7 @@ export async function GET(request: Request): Promise<Response> {
|
||||
await Promise.allSettled(
|
||||
toolIds.map(async (toolId) => {
|
||||
const mtimeMs = mtimesMap[toolId] ?? 0;
|
||||
const cached = getCached(toolId, mtimeMs);
|
||||
const cached = forceRefresh ? null : getCached(toolId, mtimeMs);
|
||||
|
||||
if (cached) {
|
||||
statuses[toolId] = cached;
|
||||
@@ -153,14 +154,13 @@ export async function GET(request: Request): Promise<Response> {
|
||||
|
||||
// Try to extract endpoint from config file
|
||||
const configPath = getCliPrimaryConfigPath(toolId);
|
||||
const endpoint = configPath
|
||||
? await extractEndpointFromConfig(toolId, configPath)
|
||||
: null;
|
||||
const endpoint = configPath ? await extractEndpointFromConfig(toolId, configPath) : null;
|
||||
|
||||
const result: ToolBatchStatus = {
|
||||
detection: {
|
||||
installed: runtime.installed,
|
||||
runnable: runtime.runnable,
|
||||
version: (runtime as Record<string, unknown>).version as string | undefined,
|
||||
command: runtime.command ?? undefined,
|
||||
commandPath: (runtime as Record<string, unknown>).commandPath as string | undefined,
|
||||
reason: runtime.reason ?? undefined,
|
||||
@@ -203,8 +203,11 @@ export async function GET(request: Request): Promise<Response> {
|
||||
return NextResponse.json(statuses);
|
||||
} catch (err) {
|
||||
logger.error({ err }, "Unexpected error in /api/cli-tools/all-statuses");
|
||||
return NextResponse.json(buildErrorBody(500, err instanceof Error ? err.message : String(err)), {
|
||||
status: 500,
|
||||
});
|
||||
return NextResponse.json(
|
||||
buildErrorBody(500, err instanceof Error ? err.message : String(err)),
|
||||
{
|
||||
status: 500,
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,14 +1,17 @@
|
||||
// DRY: shared between /api/cli-tools/status and /api/cli-tools/all-statuses (plan 14 F2)
|
||||
// In-memory mtime-based cache for batch CLI tool status results.
|
||||
// Cache invalidated when mtime changes. Lives until server restart (no TTL).
|
||||
// Cache invalidated when mtime changes or its short TTL expires.
|
||||
|
||||
import type { ToolBatchStatus } from "@/shared/types/cliBatchStatus";
|
||||
|
||||
export interface CacheEntry {
|
||||
mtimeMs: number;
|
||||
cachedAt: number;
|
||||
result: ToolBatchStatus;
|
||||
}
|
||||
|
||||
const CACHE_TTL_MS = 30_000;
|
||||
|
||||
/** Singleton in-memory cache: toolId → { mtimeMs, result } */
|
||||
const _cache = new Map<string, CacheEntry>();
|
||||
|
||||
@@ -18,18 +21,31 @@ const _cache = new Map<string, CacheEntry>();
|
||||
* - entry doesn't exist
|
||||
* - stored mtimeMs !== provided mtimeMs (config file changed)
|
||||
*/
|
||||
export function getCached(toolId: string, mtimeMs: number): ToolBatchStatus | null {
|
||||
export function getCached(
|
||||
toolId: string,
|
||||
mtimeMs: number,
|
||||
now: number = Date.now()
|
||||
): ToolBatchStatus | null {
|
||||
const entry = _cache.get(toolId);
|
||||
if (!entry) return null;
|
||||
if (entry.mtimeMs !== mtimeMs) return null;
|
||||
if (now - entry.cachedAt >= CACHE_TTL_MS) {
|
||||
_cache.delete(toolId);
|
||||
return null;
|
||||
}
|
||||
return entry.result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Store a result in the cache for a toolId with its mtime.
|
||||
*/
|
||||
export function setCached(toolId: string, mtimeMs: number, result: ToolBatchStatus): void {
|
||||
_cache.set(toolId, { mtimeMs, result });
|
||||
export function setCached(
|
||||
toolId: string,
|
||||
mtimeMs: number,
|
||||
result: ToolBatchStatus,
|
||||
now: number = Date.now()
|
||||
): void {
|
||||
_cache.set(toolId, { mtimeMs, cachedAt: now, result });
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -15,11 +15,12 @@ export function useToolBatchStatuses(): UseToolBatchStatusesResult {
|
||||
const [loading, setLoading] = useState<boolean>(true);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
const fetchStatuses = useCallback(async () => {
|
||||
const fetchStatuses = useCallback(async (forceRefresh = false) => {
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
try {
|
||||
const res = await fetch("/api/cli-tools/all-statuses");
|
||||
const url = `/api/cli-tools/all-statuses${forceRefresh ? "?refresh=true" : ""}`;
|
||||
const res = await fetch(url);
|
||||
if (!res.ok) {
|
||||
const text = await res.text().catch(() => String(res.status));
|
||||
setError(`HTTP ${res.status}: ${text.slice(0, 200)}`);
|
||||
@@ -37,6 +38,10 @@ export function useToolBatchStatuses(): UseToolBatchStatusesResult {
|
||||
}
|
||||
}, []);
|
||||
|
||||
const refetch = useCallback(() => {
|
||||
void fetchStatuses(true);
|
||||
}, [fetchStatuses]);
|
||||
|
||||
useEffect(() => {
|
||||
void fetchStatuses();
|
||||
|
||||
@@ -50,5 +55,5 @@ export function useToolBatchStatuses(): UseToolBatchStatusesResult {
|
||||
};
|
||||
}, [fetchStatuses]);
|
||||
|
||||
return { statuses, loading, error, refetch: fetchStatuses };
|
||||
return { statuses, loading, error, refetch };
|
||||
}
|
||||
|
||||
@@ -114,13 +114,13 @@ const CLI_TOOLS: Record<string, any> = {
|
||||
},
|
||||
},
|
||||
continue: {
|
||||
defaultCommand: null,
|
||||
defaultCommand: "cn",
|
||||
envBinKey: "CLI_CONTINUE_BIN",
|
||||
requiresBinary: false,
|
||||
requiresBinary: true,
|
||||
// opencode and continue may take up to 15s on first run / cold start on VPS
|
||||
healthcheckTimeoutMs: 15000,
|
||||
paths: {
|
||||
settings: ".continue/config.json",
|
||||
settings: ".continue/config.yaml",
|
||||
},
|
||||
},
|
||||
opencode: {
|
||||
@@ -571,6 +571,7 @@ export const getKnownToolPaths = (toolId: string): string[] => {
|
||||
],
|
||||
cline: [["cline.cmd", "cline"]],
|
||||
kilo: [["kilocode.cmd", "kilocode"]],
|
||||
continue: [["cn.cmd", "cn"]],
|
||||
opencode: [["opencode.cmd", "opencode"]],
|
||||
qoder: [
|
||||
["qodercli.cmd", "qodercli"],
|
||||
@@ -1112,6 +1113,7 @@ export const getCliRuntimeStatus = async (toolId: string) => {
|
||||
return {
|
||||
installed: true,
|
||||
runnable: healthcheck.runnable,
|
||||
version: healthcheck.version,
|
||||
command,
|
||||
commandPath: located.commandPath,
|
||||
reason: healthcheck.reason,
|
||||
|
||||
@@ -27,12 +27,11 @@ const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
|
||||
const { clearCache, setCached } = await import("../../src/lib/cliTools/batchStatusCache.ts");
|
||||
|
||||
// Import the route under test
|
||||
const allStatusesRoute = await import(
|
||||
"../../src/app/api/cli-tools/all-statuses/route.ts"
|
||||
);
|
||||
const allStatusesRoute = await import("../../src/app/api/cli-tools/all-statuses/route.ts");
|
||||
|
||||
// Import CLI_TOOLS to know how many tools exist
|
||||
const { CLI_TOOLS } = await import("../../src/shared/constants/cliTools.ts");
|
||||
const { getCliPrimaryConfigPath } = await import("../../src/shared/services/cliRuntime.ts");
|
||||
|
||||
const TOOL_COUNT = Object.keys(CLI_TOOLS).length;
|
||||
|
||||
@@ -105,14 +104,8 @@ test("happy path: returns status map covering all tools in CLI_TOOLS", async ()
|
||||
// Each returned entry should have detection and config fields
|
||||
for (const [toolId, entry] of Object.entries(body)) {
|
||||
const e = entry as Record<string, unknown>;
|
||||
assert.ok(
|
||||
"detection" in e,
|
||||
`tool ${toolId} missing detection field`
|
||||
);
|
||||
assert.ok(
|
||||
"config" in e,
|
||||
`tool ${toolId} missing config field`
|
||||
);
|
||||
assert.ok("detection" in e, `tool ${toolId} missing detection field`);
|
||||
assert.ok("config" in e, `tool ${toolId} missing config field`);
|
||||
}
|
||||
} else {
|
||||
// If 500 (e.g., runtime detection fails in CI), error body must be sanitized
|
||||
@@ -190,14 +183,8 @@ test("timeout in 1 tool: others succeed + slot has error field (no full request
|
||||
typeof entry.error === "string",
|
||||
`tool ${toolId} error should be a string, got ${typeof entry.error}`
|
||||
);
|
||||
assert.ok(
|
||||
"detection" in entry,
|
||||
`tool ${toolId} with error should still have detection`
|
||||
);
|
||||
assert.ok(
|
||||
"config" in entry,
|
||||
`tool ${toolId} with error should still have config`
|
||||
);
|
||||
assert.ok("detection" in entry, `tool ${toolId} with error should still have detection`);
|
||||
assert.ok("config" in entry, `tool ${toolId} with error should still have config`);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -248,3 +235,22 @@ test("cache miss: different mtime forces re-execution (cache not used)", async (
|
||||
// The entry should exist — fresh execution was performed (no crash)
|
||||
assert.ok(toolId in body, `expected ${toolId} after cache miss re-execution`);
|
||||
});
|
||||
|
||||
test("refresh=true bypasses a matching cached CLI result", async () => {
|
||||
const toolId = "codex";
|
||||
const cachedVersion = "stale-codex-version-from-cache";
|
||||
const configPath = getCliPrimaryConfigPath(toolId);
|
||||
const mtimeMs = configPath && fs.existsSync(configPath) ? fs.statSync(configPath).mtimeMs : 0;
|
||||
setCached(toolId, mtimeMs, {
|
||||
detection: { installed: true, runnable: true, version: cachedVersion },
|
||||
config: { status: "configured" },
|
||||
});
|
||||
|
||||
const response = await allStatusesRoute.GET(
|
||||
new Request("http://localhost/api/cli-tools/all-statuses?refresh=true")
|
||||
);
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
const body = (await response.json()) as Record<string, { detection?: { version?: string } }>;
|
||||
assert.notEqual(body[toolId]?.detection?.version, cachedVersion);
|
||||
});
|
||||
|
||||
@@ -59,6 +59,14 @@ test("getCached returns entry when mtime is 0 and stored mtime is also 0", () =>
|
||||
assert.deepEqual(result, status);
|
||||
});
|
||||
|
||||
test("getCached expires unchanged negative results after 30 seconds", () => {
|
||||
const status = makeStatus(false);
|
||||
setCached("codex", 0, status, 1_000);
|
||||
|
||||
assert.deepEqual(getCached("codex", 0, 30_999), status);
|
||||
assert.equal(getCached("codex", 0, 31_000), null);
|
||||
});
|
||||
|
||||
// ── invalidate ────────────────────────────────────────────────────────────────
|
||||
|
||||
test("invalidate removes entry from cache", () => {
|
||||
|
||||
@@ -170,6 +170,7 @@ describe("Healthcheck — checkRunnable", () => {
|
||||
assert.ok(result.installed, `Expected installed=true, got reason=${result.reason}`);
|
||||
if (result.runnable) {
|
||||
assert.ok(result.reason === null, `Expected no reason, got ${result.reason}`);
|
||||
assert.equal(result.version, "1.0.0");
|
||||
}
|
||||
} finally {
|
||||
if (prev !== undefined) process.env.CLI_CLINE_BIN = prev;
|
||||
@@ -177,6 +178,27 @@ describe("Healthcheck — checkRunnable", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("should detect Claude through an explicit read-only executable path", async () => {
|
||||
const previousOverride = process.env.CLI_CLAUDE_BIN;
|
||||
const script =
|
||||
process.platform === "win32"
|
||||
? createFile(tmpDir, "claude.cmd", "@echo off\necho 2.1.211 (Claude Code)\n")
|
||||
: createFile(tmpDir, "claude", "#!/bin/sh\necho '2.1.211 (Claude Code)'\n");
|
||||
if (process.platform !== "win32") fs.chmodSync(script, 0o555);
|
||||
process.env.CLI_CLAUDE_BIN = script;
|
||||
|
||||
try {
|
||||
const result = await getCliRuntimeStatus("claude");
|
||||
assert.equal(result.installed, true);
|
||||
assert.equal(result.runnable, true);
|
||||
assert.equal(result.commandPath, script);
|
||||
assert.equal(result.version, "2.1.211 (Claude Code)");
|
||||
} finally {
|
||||
if (previousOverride === undefined) delete process.env.CLI_CLAUDE_BIN;
|
||||
else process.env.CLI_CLAUDE_BIN = previousOverride;
|
||||
}
|
||||
});
|
||||
|
||||
it("should detect qodercli via env override and mark it runnable", async () => {
|
||||
const prev = process.env.CLI_QODER_BIN;
|
||||
const script =
|
||||
@@ -208,13 +230,35 @@ describe("Unknown tool", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── continue tool (requiresBinary: false) ────────────────────
|
||||
// ─── Continue CLI (`cn`) ──────────────────────────────────────
|
||||
|
||||
describe("continue tool — no binary required", () => {
|
||||
it("should report installed=true without checking binary", async () => {
|
||||
const result = await getCliRuntimeStatus("continue");
|
||||
assert.equal(result.installed, true);
|
||||
assert.equal(result.reason, "not_required");
|
||||
describe("Continue CLI detection", () => {
|
||||
it("should not report Continue as installed when the cn binary is absent", async () => {
|
||||
const previousPath = process.env.PATH;
|
||||
const previousOverride = process.env.CLI_CONTINUE_BIN;
|
||||
process.env.PATH = "";
|
||||
delete process.env.CLI_CONTINUE_BIN;
|
||||
|
||||
try {
|
||||
const result = await getCliRuntimeStatus("continue");
|
||||
assert.equal(result.installed, false);
|
||||
assert.equal(result.runnable, false);
|
||||
assert.equal(result.reason, "not_found");
|
||||
assert.equal(result.requiresBinary, true);
|
||||
} finally {
|
||||
if (previousPath === undefined) delete process.env.PATH;
|
||||
else process.env.PATH = previousPath;
|
||||
if (previousOverride === undefined) delete process.env.CLI_CONTINUE_BIN;
|
||||
else process.env.CLI_CONTINUE_BIN = previousOverride;
|
||||
}
|
||||
});
|
||||
|
||||
it("should enumerate cn in Continue's known installation paths", () => {
|
||||
const knownPaths = getKnownToolPaths("continue");
|
||||
assert.ok(
|
||||
knownPaths.some((knownPath) => /^cn(?:\.cmd)?$/i.test(path.basename(knownPath))),
|
||||
"Continue detection should search for the cn executable"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -87,8 +87,9 @@ async function mountHook(): Promise<{
|
||||
// ── Lifecycle ─────────────────────────────────────────────────────────────────
|
||||
|
||||
beforeEach(() => {
|
||||
(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT =
|
||||
true;
|
||||
(
|
||||
globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }
|
||||
).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -165,7 +166,11 @@ describe("useToolBatchStatuses", () => {
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
});
|
||||
|
||||
expect((mockFetch as ReturnType<typeof vi.fn>).mock.calls.length).toBeGreaterThan(callsAfterMount);
|
||||
expect((mockFetch as ReturnType<typeof vi.fn>).mock.calls.length).toBeGreaterThan(
|
||||
callsAfterMount
|
||||
);
|
||||
const calls = (mockFetch as ReturnType<typeof vi.fn>).mock.calls;
|
||||
expect(calls.at(-1)?.[0]).toBe("/api/cli-tools/all-statuses?refresh=true");
|
||||
});
|
||||
|
||||
it("registers focus event listener on mount", async () => {
|
||||
|
||||
Reference in New Issue
Block a user