refactor(openclaw): DRY up duplicate code across plugin

- Extract shared defaultLogger constant, use in both ProxyManager and
  HeadroomContextEngine (was duplicated inline in both constructors)
- Extract parseProxyUrl() helper, reused by normalizeAndValidateProxyUrl
  and withDefaultPort (eliminates redundant URL parsing)
- Add test helpers: stubProbeSuccess/NonHeadroom/Unreachable to replace
  6 identical inline fetch mock constructions
- Remove duplicate probe tests from engine.test.ts (already covered in
  proxy-manager.test.ts)
- Clean up unused probeHeadroomProxy import from engine.test.ts

Net: -26 lines, same coverage (28 tests)
This commit is contained in:
JerrettDavis 2026-04-03 10:28:18 -05:00
parent 89f3e5f01f
commit 3e368f13c8
5 changed files with 51 additions and 77 deletions

View file

@ -8,7 +8,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { compress } from "headroom-ai";
import { ProxyManager, type ProxyManagerConfig, type ProxyManagerLogger } from "./proxy-manager.js";
import { ProxyManager, defaultLogger, type ProxyManagerConfig, type ProxyManagerLogger } from "./proxy-manager.js";
import { agentToOpenAI, openAIToAgent } from "./convert.js";
export interface HeadroomEngineConfig extends ProxyManagerConfig {
@ -36,12 +36,7 @@ export class HeadroomContextEngine {
constructor(config: HeadroomEngineConfig = {}, logger?: ProxyManagerLogger) {
this.config = config;
this.logger = logger ?? {
info: (m) => console.log(`[headroom] ${m}`),
warn: (m) => console.warn(`[headroom] ${m}`),
error: (m) => console.error(`[headroom] ${m}`),
debug: () => {},
};
this.logger = logger ?? defaultLogger;
this.proxyManager = new ProxyManager(config, this.logger);
}

View file

@ -1,5 +1,5 @@
export { default } from "./plugin/index.js";
export { HeadroomContextEngine } from "./engine.js";
export { ProxyManager, normalizeAndValidateProxyUrl, isLocalProxyUrl, probeHeadroomProxy } from "./proxy-manager.js";
export { ProxyManager, normalizeAndValidateProxyUrl, isLocalProxyUrl, defaultLogger, probeHeadroomProxy } from "./proxy-manager.js";
export { agentToOpenAI, openAIToAgent } from "./convert.js";
export { createHeadroomRetrieveTool } from "./tools/headroom-retrieve.js";

View file

@ -27,6 +27,14 @@ export interface ProxyManagerLogger {
debug(message: string): void;
}
/** Default logger that prefixes all messages with `[headroom]`. */
export const defaultLogger: ProxyManagerLogger = {
info: (m) => console.log(`[headroom] ${m}`),
warn: (m) => console.warn(`[headroom] ${m}`),
error: (m) => console.error(`[headroom] ${m}`),
debug: () => {},
};
export interface ProxyProbeResult {
reachable: boolean;
isHeadroom: boolean;
@ -41,13 +49,6 @@ interface LaunchSpec {
checkArgs: string[];
}
const defaultLogger: ProxyManagerLogger = {
info: (m) => console.log(`[headroom] ${m}`),
warn: (m) => console.warn(`[headroom] ${m}`),
error: (m) => console.error(`[headroom] ${m}`),
debug: () => {},
};
export class ProxyManager {
private config: ProxyManagerConfig;
private logger: ProxyManagerLogger;
@ -309,13 +310,17 @@ export class ProxyManager {
}
}
export function normalizeAndValidateProxyUrl(proxyUrl: string): string {
let parsed: URL;
/** Parse a URL, returning the parsed object or throwing a descriptive error. */
function parseProxyUrl(proxyUrl: string): URL {
try {
parsed = new URL(proxyUrl);
return new URL(proxyUrl);
} catch {
throw new Error(`Invalid proxyUrl: "${proxyUrl}"`);
}
}
export function normalizeAndValidateProxyUrl(proxyUrl: string): string {
const parsed = parseProxyUrl(proxyUrl);
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") {
throw new Error("proxyUrl must use http:// or https://");
@ -339,7 +344,7 @@ export function isLocalProxyUrl(proxyUrl: string): boolean {
}
function withDefaultPort(proxyUrl: string, defaultPort: number): string {
const parsed = new URL(proxyUrl);
const parsed = parseProxyUrl(proxyUrl);
if (!parsed.port) {
parsed.port = String(defaultPort);
}

View file

@ -10,7 +10,7 @@
import { describe, it, expect, beforeAll, afterAll, vi, afterEach } from "vitest";
import { HeadroomContextEngine } from "../src/engine.js";
import { agentToOpenAI, openAIToAgent } from "../src/convert.js";
import { ProxyManager, probeHeadroomProxy } from "../src/proxy-manager.js";
import { ProxyManager } from "../src/proxy-manager.js";
const RUN = process.env.HEADROOM_INTEGRATION === "1";
const PROXY_URL = process.env.HEADROOM_PROXY_URL ?? "http://127.0.0.1:8787";
@ -19,27 +19,7 @@ afterEach(() => {
vi.restoreAllMocks();
});
describe("Proxy probing", () => {
it("detects running Headroom proxy", async () => {
const fetchMock = vi.fn()
.mockResolvedValueOnce({ ok: true, status: 200 })
.mockResolvedValueOnce({ ok: true, status: 200 });
vi.stubGlobal("fetch", fetchMock);
const result = await probeHeadroomProxy("http://127.0.0.1:8787");
expect(result).toEqual({ reachable: true, isHeadroom: true });
});
it("flags non-headroom service at configured URL", async () => {
const fetchMock = vi.fn()
.mockResolvedValueOnce({ ok: true, status: 200 })
.mockResolvedValueOnce({ ok: false, status: 404 });
vi.stubGlobal("fetch", fetchMock);
const manager = new ProxyManager({ proxyUrl: "http://127.0.0.1:8787" });
await expect(manager.start()).rejects.toThrow(/does not appear to be a Headroom proxy/);
});
});
// Proxy probing and ProxyManager.start tests live in proxy-manager.test.ts
describe("AgentMessage conversion", () => {
it("converts user message", () => {

View file

@ -10,6 +10,29 @@ afterEach(() => {
vi.restoreAllMocks();
});
/** Stub fetch with a sequence of health/retrieve probe outcomes. */
function stubProbeSuccess() {
const mock = vi.fn()
.mockResolvedValueOnce({ ok: true, status: 200 }) // /health
.mockResolvedValueOnce({ ok: true, status: 200 }); // /v1/retrieve/stats
vi.stubGlobal("fetch", mock);
return mock;
}
function stubProbeNonHeadroom() {
const mock = vi.fn()
.mockResolvedValueOnce({ ok: true, status: 200 }) // /health OK
.mockResolvedValueOnce({ ok: false, status: 404 }); // /v1/retrieve/stats 404
vi.stubGlobal("fetch", mock);
return mock;
}
function stubProbeUnreachable() {
const mock = vi.fn().mockRejectedValue(new Error("ECONNREFUSED"));
vi.stubGlobal("fetch", mock);
return mock;
}
describe("normalizeAndValidateProxyUrl", () => {
it("accepts localhost origins", () => {
expect(normalizeAndValidateProxyUrl("http://127.0.0.1:8787")).toBe("http://127.0.0.1:8787");
@ -50,23 +73,13 @@ describe("isLocalProxyUrl", () => {
describe("probeHeadroomProxy", () => {
it("returns reachable+isHeadroom when both endpoints succeed", async () => {
const fetchMock = vi
.fn()
.mockResolvedValueOnce({ ok: true, status: 200 })
.mockResolvedValueOnce({ ok: true, status: 200 });
vi.stubGlobal("fetch", fetchMock);
stubProbeSuccess();
const result = await probeHeadroomProxy("http://127.0.0.1:8787");
expect(result).toEqual({ reachable: true, isHeadroom: true });
});
it("returns reachable but non-headroom when retrieve endpoint fails", async () => {
const fetchMock = vi
.fn()
.mockResolvedValueOnce({ ok: true, status: 200 })
.mockResolvedValueOnce({ ok: false, status: 404 });
vi.stubGlobal("fetch", fetchMock);
stubProbeNonHeadroom();
const result = await probeHeadroomProxy("http://127.0.0.1:8787");
expect(result.reachable).toBe(true);
expect(result.isHeadroom).toBe(false);
@ -74,7 +87,7 @@ describe("probeHeadroomProxy", () => {
});
it("returns unreachable when health check fails", async () => {
vi.stubGlobal("fetch", vi.fn().mockRejectedValue(new Error("boom")));
stubProbeUnreachable();
const result = await probeHeadroomProxy("http://127.0.0.1:8787");
expect(result.reachable).toBe(false);
expect(result.isHeadroom).toBe(false);
@ -115,12 +128,7 @@ describe("ProxyManager.start", () => {
it("fails when explicit URL is reachable but not a headroom proxy", async () => {
const manager = new ProxyManager({ proxyUrl: "http://127.0.0.1:8787" });
const fetchMock = vi
.fn()
.mockResolvedValueOnce({ ok: true, status: 200 })
.mockResolvedValueOnce({ ok: false, status: 404 });
vi.stubGlobal("fetch", fetchMock);
stubProbeNonHeadroom();
await expect(manager.start()).rejects.toThrow(/does not appear to be a Headroom proxy/);
});
@ -143,13 +151,7 @@ describe("ProxyManager.start", () => {
it("connects to remote proxy without auto-start", async () => {
const manager = new ProxyManager({ proxyUrl: "http://headroom.remote.example:8787", autoStart: true });
const startSpy = vi.spyOn(manager as any, "startHeadroomProxy").mockResolvedValue(undefined);
// Remote probe succeeds
const fetchMock = vi
.fn()
.mockResolvedValueOnce({ ok: true, status: 200 }) // /health
.mockResolvedValueOnce({ ok: true, status: 200 }); // /v1/retrieve/stats
vi.stubGlobal("fetch", fetchMock);
stubProbeSuccess();
const url = await manager.start();
expect(url).toBe("http://headroom.remote.example:8787");
@ -157,14 +159,8 @@ describe("ProxyManager.start", () => {
});
it("does not apply proxyPort default to remote URLs", async () => {
// Remote URL without port should use protocol default, not proxyPort
const manager = new ProxyManager({ proxyUrl: "https://headroom.remote.example", proxyPort: 9999 });
const fetchMock = vi
.fn()
.mockResolvedValueOnce({ ok: true, status: 200 })
.mockResolvedValueOnce({ ok: true, status: 200 });
vi.stubGlobal("fetch", fetchMock);
stubProbeSuccess();
const url = await manager.start();
expect(url).toBe("https://headroom.remote.example");
@ -173,9 +169,7 @@ describe("ProxyManager.start", () => {
it("fails fast for unreachable remote proxy without attempting auto-start", async () => {
const manager = new ProxyManager({ proxyUrl: "https://headroom.remote.example:8787", autoStart: true });
const startSpy = vi.spyOn(manager as any, "startHeadroomProxy").mockResolvedValue(undefined);
const fetchMock = vi.fn().mockRejectedValue(new Error("ECONNREFUSED"));
vi.stubGlobal("fetch", fetchMock);
stubProbeUnreachable();
await expect(manager.start()).rejects.toThrow(/Remote Headroom proxy not reachable/);
expect(startSpy).not.toHaveBeenCalled();