fix: address code review feedback for doctor health check

- Fix TypeScript error: extract host field for getDaemonHost call
- Use async execFile + Promise.all for parallel provider checks
- Load config once in runConfigChecks instead of twice
This commit is contained in:
Zi Makki
2026-03-09 13:08:15 +01:00
parent 15d7763d0b
commit 59e0b2ec62
3 changed files with 58 additions and 48 deletions

View File

@@ -83,7 +83,7 @@ export async function runDoctorCommand(
let report: DoctorReport
if (remote) {
const host = getDaemonHost(options)
const host = getDaemonHost({ host: options.host as string | undefined })
report = await fetchRemoteReport(host)
} else {
report = await runDoctorChecks()

View File

@@ -1,4 +1,4 @@
import { loadPersistedConfig } from "../../persisted-config.js";
import { loadPersistedConfig, type PersistedConfig } from "../../persisted-config.js";
import type { DoctorCheckResult } from "../types.js";
/**
@@ -20,53 +20,58 @@ function isValidListenString(listen: string): boolean {
return Number.isFinite(parseInt(listen, 10));
}
function checkConfigValid(paseoHome: string): DoctorCheckResult {
try {
loadPersistedConfig(paseoHome);
function checkConfigValid(config: PersistedConfig | null, loadError: string | null): DoctorCheckResult {
if (config) {
return {
id: "config.valid",
label: "Config file",
status: "ok",
detail: "Valid",
};
} catch (err) {
return {
id: "config.valid",
label: "Config file",
status: "error",
detail: err instanceof Error ? err.message : String(err),
};
}
return {
id: "config.valid",
label: "Config file",
status: "error",
detail: loadError ?? "Unknown error",
};
}
function checkListenAddress(paseoHome: string): DoctorCheckResult {
try {
const config = loadPersistedConfig(paseoHome);
const listen = config.daemon?.listen ?? "127.0.0.1:6767";
if (!isValidListenString(listen)) {
return {
id: "config.listen",
label: "Listen address",
status: "error",
detail: `Malformed listen address: ${listen}`,
};
}
return {
id: "config.listen",
label: "Listen address",
status: "ok",
detail: listen,
};
} catch (err) {
function checkListenAddress(config: PersistedConfig | null): DoctorCheckResult {
if (!config) {
return {
id: "config.listen",
label: "Listen address",
status: "error",
detail: err instanceof Error ? err.message : String(err),
detail: "Cannot check (config failed to load)",
};
}
const listen = config.daemon?.listen ?? "127.0.0.1:6767";
if (!isValidListenString(listen)) {
return {
id: "config.listen",
label: "Listen address",
status: "error",
detail: `Malformed listen address: ${listen}`,
};
}
return {
id: "config.listen",
label: "Listen address",
status: "ok",
detail: listen,
};
}
export async function runConfigChecks(paseoHome: string): Promise<DoctorCheckResult[]> {
return [checkConfigValid(paseoHome), checkListenAddress(paseoHome)];
let config: PersistedConfig | null = null;
let loadError: string | null = null;
try {
config = loadPersistedConfig(paseoHome);
} catch (err) {
loadError = err instanceof Error ? err.message : String(err);
}
return [checkConfigValid(config, loadError), checkListenAddress(config)];
}

View File

@@ -1,7 +1,10 @@
import { execFileSync } from "node:child_process";
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import type { DoctorCheckResult } from "../types.js";
const execFileAsync = promisify(execFile);
interface ProviderDef {
name: string;
command: string;
@@ -16,18 +19,20 @@ const PROVIDERS: ProviderDef[] = [
const EXEC_TIMEOUT_MS = 5000;
function whichCommand(command: string): string | null {
async function whichCommand(command: string): Promise<string | null> {
const whichBin = process.platform === "win32" ? "where" : "which";
try {
return execFileSync(whichBin, [command], { encoding: "utf8", timeout: EXEC_TIMEOUT_MS }).trim() || null;
const { stdout } = await execFileAsync(whichBin, [command], { encoding: "utf8", timeout: EXEC_TIMEOUT_MS });
return stdout.trim() || null;
} catch {
return null;
}
}
function getVersion(binaryPath: string): string | null {
async function getVersion(binaryPath: string): Promise<string | null> {
try {
return execFileSync(binaryPath, ["--version"], { encoding: "utf8", timeout: EXEC_TIMEOUT_MS }).trim() || null;
const { stdout } = await execFileAsync(binaryPath, ["--version"], { encoding: "utf8", timeout: EXEC_TIMEOUT_MS });
return stdout.trim() || null;
} catch {
return null;
}
@@ -50,7 +55,7 @@ function checkBinary(provider: ProviderDef, binaryPath: string | null): DoctorCh
};
}
function checkVersion(provider: ProviderDef, binaryPath: string | null): DoctorCheckResult {
async function checkVersion(provider: ProviderDef, binaryPath: string | null): Promise<DoctorCheckResult> {
if (!binaryPath) {
return {
id: `provider.${provider.name}.version`,
@@ -60,7 +65,7 @@ function checkVersion(provider: ProviderDef, binaryPath: string | null): DoctorC
};
}
const version = getVersion(binaryPath);
const version = await getVersion(binaryPath);
if (version) {
return {
id: `provider.${provider.name}.version`,
@@ -78,12 +83,12 @@ function checkVersion(provider: ProviderDef, binaryPath: string | null): DoctorC
};
}
export async function runProviderChecks(): Promise<DoctorCheckResult[]> {
const results: DoctorCheckResult[] = [];
for (const provider of PROVIDERS) {
const binaryPath = whichCommand(provider.command);
results.push(checkBinary(provider, binaryPath));
results.push(checkVersion(provider, binaryPath));
}
return results;
async function checkProvider(provider: ProviderDef): Promise<DoctorCheckResult[]> {
const binaryPath = await whichCommand(provider.command);
return [checkBinary(provider, binaryPath), await checkVersion(provider, binaryPath)];
}
export async function runProviderChecks(): Promise<DoctorCheckResult[]> {
const perProvider = await Promise.all(PROVIDERS.map(checkProvider));
return perProvider.flat();
}