From 64aff2d0caa07cce46b08edc7271049a1cc799c3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Feb 2026 05:21:13 +0000 Subject: [PATCH] perf(browser): isolate profile hot-reload config refresh --- src/browser/resolved-config-refresh.ts | 58 +++++++ ...server-context.hot-reload-profiles.test.ts | 145 ++++++------------ src/browser/server-context.ts | 61 ++------ 3 files changed, 117 insertions(+), 147 deletions(-) create mode 100644 src/browser/resolved-config-refresh.ts diff --git a/src/browser/resolved-config-refresh.ts b/src/browser/resolved-config-refresh.ts new file mode 100644 index 000000000..1c4a59735 --- /dev/null +++ b/src/browser/resolved-config-refresh.ts @@ -0,0 +1,58 @@ +import type { BrowserServerState } from "./server-context.types.js"; +import { createConfigIO, loadConfig } from "../config/config.js"; +import { resolveBrowserConfig, resolveProfile, type ResolvedBrowserProfile } from "./config.js"; + +function applyResolvedConfig( + current: BrowserServerState, + freshResolved: BrowserServerState["resolved"], +) { + current.resolved = freshResolved; + for (const [name, runtime] of current.profiles) { + const nextProfile = resolveProfile(freshResolved, name); + if (nextProfile) { + runtime.profile = nextProfile; + continue; + } + if (!runtime.running) { + current.profiles.delete(name); + } + } +} + +export function refreshResolvedBrowserConfigFromDisk(params: { + current: BrowserServerState; + refreshConfigFromDisk: boolean; + mode: "cached" | "fresh"; +}) { + if (!params.refreshConfigFromDisk) { + return; + } + const cfg = params.mode === "fresh" ? createConfigIO().loadConfig() : loadConfig(); + const freshResolved = resolveBrowserConfig(cfg.browser, cfg); + applyResolvedConfig(params.current, freshResolved); +} + +export function resolveBrowserProfileWithHotReload(params: { + current: BrowserServerState; + refreshConfigFromDisk: boolean; + name: string; +}): ResolvedBrowserProfile | null { + refreshResolvedBrowserConfigFromDisk({ + current: params.current, + refreshConfigFromDisk: params.refreshConfigFromDisk, + mode: "cached", + }); + let profile = resolveProfile(params.current.resolved, params.name); + if (profile) { + return profile; + } + + // Hot-reload: profile missing; retry with a fresh disk read without flushing the global cache. + refreshResolvedBrowserConfigFromDisk({ + current: params.current, + refreshConfigFromDisk: params.refreshConfigFromDisk, + mode: "fresh", + }); + profile = resolveProfile(params.current.resolved, params.name); + return profile; +} diff --git a/src/browser/server-context.hot-reload-profiles.test.ts b/src/browser/server-context.hot-reload-profiles.test.ts index c6af1a80e..b448a872f 100644 --- a/src/browser/server-context.hot-reload-profiles.test.ts +++ b/src/browser/server-context.hot-reload-profiles.test.ts @@ -1,4 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveBrowserConfig } from "./config.js"; +import { + refreshResolvedBrowserConfigFromDisk, + resolveBrowserProfileWithHotReload, +} from "./resolved-config-refresh.js"; let cfgProfiles: Record = {}; @@ -31,69 +36,10 @@ vi.mock("../config/config.js", () => ({ } return cachedConfig; }, - clearConfigCache: vi.fn(() => { - // Clear the simulated cache - cachedConfig = null; - }), writeConfigFile: vi.fn(async () => {}), })); -vi.mock("./chrome.js", () => ({ - isChromeCdpReady: vi.fn(async () => false), - isChromeReachable: vi.fn(async () => false), - launchOpenClawChrome: vi.fn(async () => { - throw new Error("launch disabled"); - }), - resolveOpenClawUserDataDir: vi.fn(() => "/tmp/openclaw"), - stopOpenClawChrome: vi.fn(async () => {}), -})); - -vi.mock("./cdp.js", () => ({ - createTargetViaCdp: vi.fn(async () => { - throw new Error("cdp disabled"); - }), - normalizeCdpWsUrl: vi.fn((wsUrl: string) => wsUrl), - snapshotAria: vi.fn(async () => ({ nodes: [] })), - getHeadersWithAuth: vi.fn(() => ({})), - appendCdpPath: vi.fn((cdpUrl: string, path: string) => `${cdpUrl}${path}`), -})); - -vi.mock("./pw-ai.js", () => ({ - closePlaywrightBrowserConnection: vi.fn(async () => {}), -})); - -vi.mock("../media/store.js", () => ({ - ensureMediaDir: vi.fn(async () => {}), - saveMediaBuffer: vi.fn(async () => ({ path: "/tmp/fake.png" })), -})); - describe("server-context hot-reload profiles", () => { - let modulesPromise: Promise<{ - createBrowserRouteContext: typeof import("./server-context.js").createBrowserRouteContext; - resolveBrowserConfig: typeof import("./config.js").resolveBrowserConfig; - loadConfig: typeof import("../config/config.js").loadConfig; - clearConfigCache: typeof import("../config/config.js").clearConfigCache; - }> | null = null; - - const getModules = async () => { - if (!modulesPromise) { - modulesPromise = (async () => { - // Avoid parallel imports here; Vitest mock factories use async importOriginal - // and parallel loading can observe partially-initialized modules. - const configMod = await import("../config/config.js"); - const config = await import("./config.js"); - const serverContext = await import("./server-context.js"); - return { - createBrowserRouteContext: serverContext.createBrowserRouteContext, - resolveBrowserConfig: config.resolveBrowserConfig, - loadConfig: configMod.loadConfig, - clearConfigCache: configMod.clearConfigCache, - }; - })(); - } - return await modulesPromise; - }; - beforeEach(() => { vi.clearAllMocks(); cfgProfiles = { @@ -103,8 +49,7 @@ describe("server-context hot-reload profiles", () => { }); it("forProfile hot-reloads newly added profiles from config", async () => { - const { createBrowserRouteContext, resolveBrowserConfig, loadConfig, clearConfigCache } = - await getModules(); + const { loadConfig } = await import("../config/config.js"); // Start with only openclaw profile // 1. Prime the cache by calling loadConfig() first @@ -120,13 +65,14 @@ describe("server-context hot-reload profiles", () => { profiles: new Map(), }; - const ctx = createBrowserRouteContext({ - getState: () => state, - refreshConfigFromDisk: true, - }); - // Initially, "desktop" profile should not exist - expect(() => ctx.forProfile("desktop")).toThrow(/not found/); + expect( + resolveBrowserProfileWithHotReload({ + current: state, + refreshConfigFromDisk: true, + name: "desktop", + }), + ).toBeNull(); // 2. Simulate adding a new profile to config (like user editing openclaw.json) cfgProfiles.desktop = { cdpUrl: "http://127.0.0.1:9222", color: "#0066CC" }; @@ -135,11 +81,15 @@ describe("server-context hot-reload profiles", () => { const staleCfg = loadConfig(); expect(staleCfg.browser.profiles.desktop).toBeUndefined(); // Cache is stale! - // 4. Now forProfile should hot-reload (calls createConfigIO().loadConfig() internally) - // It should NOT clear the global cache - const profileCtx = ctx.forProfile("desktop"); - expect(profileCtx.profile.name).toBe("desktop"); - expect(profileCtx.profile.cdpUrl).toBe("http://127.0.0.1:9222"); + // 4. Hot-reload should read fresh config for the lookup (createConfigIO().loadConfig()), + // without flushing the global loadConfig cache. + const profile = resolveBrowserProfileWithHotReload({ + current: state, + refreshConfigFromDisk: true, + name: "desktop", + }); + expect(profile?.name).toBe("desktop"); + expect(profile?.cdpUrl).toBe("http://127.0.0.1:9222"); // 5. Verify the new profile was merged into the cached state expect(state.resolved.profiles.desktop).toBeDefined(); @@ -148,13 +98,10 @@ describe("server-context hot-reload profiles", () => { // This confirms the fix: we read fresh config for the specific profile lookup without flushing the global cache const stillStaleCfg = loadConfig(); expect(stillStaleCfg.browser.profiles.desktop).toBeUndefined(); - - // Verify clearConfigCache was not called - expect(clearConfigCache).not.toHaveBeenCalled(); }); it("forProfile still throws for profiles that don't exist in fresh config", async () => { - const { createBrowserRouteContext, resolveBrowserConfig, loadConfig } = await getModules(); + const { loadConfig } = await import("../config/config.js"); const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -165,17 +112,18 @@ describe("server-context hot-reload profiles", () => { profiles: new Map(), }; - const ctx = createBrowserRouteContext({ - getState: () => state, - refreshConfigFromDisk: true, - }); - // Profile that doesn't exist anywhere should still throw - expect(() => ctx.forProfile("nonexistent")).toThrow(/not found/); + expect( + resolveBrowserProfileWithHotReload({ + current: state, + refreshConfigFromDisk: true, + name: "nonexistent", + }), + ).toBeNull(); }); it("forProfile refreshes existing profile config after loadConfig cache updates", async () => { - const { createBrowserRouteContext, resolveBrowserConfig, loadConfig } = await getModules(); + const { loadConfig } = await import("../config/config.js"); const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -186,24 +134,20 @@ describe("server-context hot-reload profiles", () => { profiles: new Map(), }; - const ctx = createBrowserRouteContext({ - getState: () => state, - refreshConfigFromDisk: true, - }); - - const before = ctx.forProfile("openclaw"); - expect(before.profile.cdpPort).toBe(18800); - cfgProfiles.openclaw = { cdpPort: 19999, color: "#FF4500" }; cachedConfig = null; - const after = ctx.forProfile("openclaw"); - expect(after.profile.cdpPort).toBe(19999); + const after = resolveBrowserProfileWithHotReload({ + current: state, + refreshConfigFromDisk: true, + name: "openclaw", + }); + expect(after?.cdpPort).toBe(19999); expect(state.resolved.profiles.openclaw?.cdpPort).toBe(19999); }); it("listProfiles refreshes config before enumerating profiles", async () => { - const { createBrowserRouteContext, resolveBrowserConfig, loadConfig } = await getModules(); + const { loadConfig } = await import("../config/config.js"); const cfg = loadConfig(); const resolved = resolveBrowserConfig(cfg.browser, cfg); @@ -214,15 +158,14 @@ describe("server-context hot-reload profiles", () => { profiles: new Map(), }; - const ctx = createBrowserRouteContext({ - getState: () => state, - refreshConfigFromDisk: true, - }); - cfgProfiles.desktop = { cdpPort: 19999, color: "#0066CC" }; cachedConfig = null; - const profiles = await ctx.listProfiles(); - expect(profiles.some((p) => p.name === "desktop")).toBe(true); + refreshResolvedBrowserConfigFromDisk({ + current: state, + refreshConfigFromDisk: true, + mode: "cached", + }); + expect(Object.keys(state.resolved.profiles)).toContain("desktop"); }); }); diff --git a/src/browser/server-context.ts b/src/browser/server-context.ts index 658e75b3d..61698f6e7 100644 --- a/src/browser/server-context.ts +++ b/src/browser/server-context.ts @@ -10,7 +10,6 @@ import type { ProfileRuntimeState, ProfileStatus, } from "./server-context.types.js"; -import { createConfigIO, loadConfig } from "../config/config.js"; import { appendCdpPath, createTargetViaCdp, getHeadersWithAuth, normalizeCdpWsUrl } from "./cdp.js"; import { isChromeCdpReady, @@ -19,12 +18,16 @@ import { resolveOpenClawUserDataDir, stopOpenClawChrome, } from "./chrome.js"; -import { resolveBrowserConfig, resolveProfile } from "./config.js"; +import { resolveProfile } from "./config.js"; import { ensureChromeExtensionRelayServer, stopChromeExtensionRelayServer, } from "./extension-relay.js"; import { getPwAiModule } from "./pw-ai-module.js"; +import { + refreshResolvedBrowserConfigFromDisk, + resolveBrowserProfileWithHotReload, +} from "./resolved-config-refresh.js"; import { resolveTargetIdFromTabs } from "./target-id.js"; import { movePathToTrash } from "./trash.js"; @@ -579,52 +582,14 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon return current; }; - const applyResolvedConfig = ( - current: BrowserServerState, - freshResolved: BrowserServerState["resolved"], - ) => { - current.resolved = freshResolved; - for (const [name, runtime] of current.profiles) { - const nextProfile = resolveProfile(freshResolved, name); - if (nextProfile) { - runtime.profile = nextProfile; - continue; - } - if (!runtime.running) { - current.profiles.delete(name); - } - } - }; - - const refreshResolvedConfig = (current: BrowserServerState) => { - if (!refreshConfigFromDisk) { - return; - } - const cfg = loadConfig(); - const freshResolved = resolveBrowserConfig(cfg.browser, cfg); - applyResolvedConfig(current, freshResolved); - }; - - const refreshResolvedConfigFresh = (current: BrowserServerState) => { - if (!refreshConfigFromDisk) { - return; - } - const freshCfg = createConfigIO().loadConfig(); - const freshResolved = resolveBrowserConfig(freshCfg.browser, freshCfg); - applyResolvedConfig(current, freshResolved); - }; - const forProfile = (profileName?: string): ProfileContext => { const current = state(); - refreshResolvedConfig(current); const name = profileName ?? current.resolved.defaultProfile; - let profile = resolveProfile(current.resolved, name); - - // Hot-reload: try fresh config if profile not found - if (!profile) { - refreshResolvedConfigFresh(current); - profile = resolveProfile(current.resolved, name); - } + const profile = resolveBrowserProfileWithHotReload({ + current, + refreshConfigFromDisk, + name, + }); if (!profile) { const available = Object.keys(current.resolved.profiles).join(", "); @@ -635,7 +600,11 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon const listProfiles = async (): Promise => { const current = state(); - refreshResolvedConfig(current); + refreshResolvedBrowserConfigFromDisk({ + current, + refreshConfigFromDisk, + mode: "cached", + }); const result: ProfileStatus[] = []; for (const name of Object.keys(current.resolved.profiles)) {