From 4bc048549b64dc743a406ce143cfd500f7f68d05 Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Mon, 4 May 2026 02:30:40 +1000 Subject: [PATCH] fix: improve proxy validation output --- src/cli/proxy-cli.runtime.test.ts | 67 ++++++++++++++++++++++++++++++- src/cli/proxy-cli.runtime.ts | 65 +++++++++++++++++++++--------- 2 files changed, 112 insertions(+), 20 deletions(-) diff --git a/src/cli/proxy-cli.runtime.test.ts b/src/cli/proxy-cli.runtime.test.ts index f38cee36762..e1784420b83 100644 --- a/src/cli/proxy-cli.runtime.test.ts +++ b/src/cli/proxy-cli.runtime.test.ts @@ -43,6 +43,8 @@ describe("proxy cli runtime", () => { "OPENCLAW_DEBUG_PROXY_CERT_DIR", "OPENCLAW_DEBUG_PROXY_SESSION_ID", "OPENCLAW_DEBUG_PROXY_ENABLED", + "FORCE_COLOR", + "NO_COLOR", ] as const; const savedEnv = Object.fromEntries(envKeys.map((key) => [key, process.env[key]])); let tempDir = ""; @@ -54,6 +56,8 @@ describe("proxy cli runtime", () => { process.env.OPENCLAW_DEBUG_PROXY_CERT_DIR = path.join(tempDir, "certs"); delete process.env.OPENCLAW_DEBUG_PROXY_ENABLED; delete process.env.OPENCLAW_DEBUG_PROXY_SESSION_ID; + delete process.env.FORCE_COLOR; + process.env.NO_COLOR = "1"; getRuntimeConfigMock.mockReset(); getRuntimeConfigMock.mockReturnValue({ proxy: { @@ -311,6 +315,66 @@ describe("proxy cli runtime", () => { ); }); + it("prints check errors on the same line", async () => { + runProxyValidationMock.mockResolvedValueOnce({ + ok: true, + config: { + enabled: true, + proxyUrl: "http://proxy.example:3128", + source: "config", + errors: [], + }, + checks: [ + { + kind: "denied", + url: "http://127.0.0.1:12345/", + ok: true, + error: "fetch failed", + }, + ], + }); + const { runProxyValidateCommand } = await import("./proxy-cli.runtime.js"); + + await runProxyValidateCommand({}); + + expect(process.stdout.write).toHaveBeenCalledWith( + "Proxy validation passed\n\n" + + "Proxy\n" + + " Source: config\n" + + " URL: http://proxy.example:3128/\n\n" + + "Checks\n" + + " ✓ denied http://127.0.0.1:12345/ — fetch failed\n", + ); + }); + + it("applies the terminal color theme when rich output is enabled", async () => { + vi.resetModules(); + vi.doMock("../terminal/theme.js", () => ({ + colorize: (rich: boolean, color: (value: string) => string, value: string) => + rich ? color(value) : value, + isRich: () => true, + theme: { + heading: (value: string) => `${value}`, + success: (value: string) => `${value}`, + error: (value: string) => `${value}`, + muted: (value: string) => `${value}`, + warn: (value: string) => `${value}`, + }, + })); + try { + const { runProxyValidateCommand } = await import("./proxy-cli.runtime.js"); + + await runProxyValidateCommand({}); + + const output = String(vi.mocked(process.stdout.write).mock.calls[0]?.[0] ?? ""); + expect(output).toContain("Proxy validation passed"); + expect(output).toContain("Checks"); + expect(output).toContain("✓"); + } finally { + vi.doUnmock("../terminal/theme.js"); + } + }); + it("prints actionable check failure output", async () => { runProxyValidationMock.mockResolvedValueOnce({ ok: false, @@ -347,8 +411,7 @@ describe("proxy cli runtime", () => { " URL: http://proxy.example:3128/\n\n" + "Checks\n" + " ✓ allowed http://target.example/allowed HTTP 200\n" + - " ✗ denied http://target.example/allowed HTTP 200\n" + - " Denied destination was reachable through the proxy\n\n" + + " ✗ denied http://target.example/allowed HTTP 200 — Denied destination was reachable through the proxy\n\n" + "Next steps\n" + " Update the proxy ACL so denied destinations are blocked, or pass the expected --denied-url values.\n", ); diff --git a/src/cli/proxy-cli.runtime.ts b/src/cli/proxy-cli.runtime.ts index c3ded290d08..a7299385d08 100644 --- a/src/cli/proxy-cli.runtime.ts +++ b/src/cli/proxy-cli.runtime.ts @@ -19,6 +19,7 @@ import { getDebugProxyCaptureStore, } from "../proxy-capture/store.sqlite.js"; import type { CaptureQueryPreset } from "../proxy-capture/types.js"; +import { colorize, isRich, theme } from "../terminal/theme.js"; export async function runDebugProxyStartCommand(opts: { host?: string; port?: number }) { const settings = resolveDebugProxySettings(); @@ -148,11 +149,41 @@ function redactProxyValidationResult(result: ProxyValidationResult): ProxyValida }; } -function formatProxyCheckLine(check: ProxyValidationResult["checks"][number]): string { - const icon = check.ok ? "✓" : "✗"; - const paddedKind = check.kind.padEnd(7, " "); - const status = check.status === undefined ? "" : ` HTTP ${check.status}`; - return ` ${icon} ${paddedKind} ${check.url}${status}`; +type ProxyValidationTextColors = { + heading: (value: string) => string; + success: (value: string) => string; + error: (value: string) => string; + muted: (value: string) => string; + warn: (value: string) => string; +}; + +function getProxyValidationTextColors(): ProxyValidationTextColors { + const rich = isRich(); + const apply = (color: (value: string) => string) => (value: string) => + colorize(rich, color, value); + return { + heading: apply(theme.heading), + success: apply(theme.success), + error: apply(theme.error), + muted: apply(theme.muted), + warn: apply(theme.warn), + }; +} + +function formatProxyCheckLine( + check: ProxyValidationResult["checks"][number], + colors: ProxyValidationTextColors, +): string { + const icon = check.ok ? colors.success("✓") : colors.error("✗"); + const paddedKind = colors.muted(check.kind.padEnd(7, " ")); + const status = + check.status === undefined + ? "" + : ` ${check.ok ? colors.success(`HTTP ${check.status}`) : colors.error(`HTTP ${check.status}`)}`; + const detail = check.error + ? ` — ${check.ok ? colors.muted(check.error) : colors.error(check.error)}` + : ""; + return ` ${icon} ${paddedKind} ${check.url}${status}${detail}`; } function formatProxyValidationNextSteps(result: ProxyValidationResult): string[] { @@ -185,37 +216,35 @@ function formatProxyValidationNextSteps(result: ProxyValidationResult): string[] } function formatProxyValidationText(result: ProxyValidationResult): string { + const colors = getProxyValidationTextColors(); const redactedProxyUrl = redactProxyUrl(result.config.proxyUrl); const lines = [ - `Proxy validation ${result.ok ? "passed" : "failed"}`, + result.ok ? colors.success("Proxy validation passed") : colors.error("Proxy validation failed"), "", - "Proxy", - ` Source: ${result.config.source}`, - ` URL: ${redactedProxyUrl ?? "not configured"}`, + colors.heading("Proxy"), + ` Source: ${colors.muted(result.config.source)}`, + ` URL: ${redactedProxyUrl ?? colors.muted("not configured")}`, ]; if (result.config.errors.length > 0) { - lines.push("", "Problems"); + lines.push("", colors.heading("Problems")); for (const error of result.config.errors) { - lines.push(` - ${error}`); + lines.push(` - ${colors.error(error)}`); } } if (result.checks.length > 0) { - lines.push("", "Checks"); + lines.push("", colors.heading("Checks")); for (const check of result.checks) { - lines.push(formatProxyCheckLine(check)); - if (check.error) { - lines.push(` ${check.error}`); - } + lines.push(formatProxyCheckLine(check, colors)); } } const nextSteps = formatProxyValidationNextSteps(result); if (nextSteps.length > 0) { - lines.push("", "Next steps"); + lines.push("", colors.heading("Next steps")); for (const nextStep of nextSteps) { - lines.push(` ${nextStep}`); + lines.push(` ${colors.warn(nextStep)}`); } }