From 3bda3df7299049096ddb1ebd1d9cd689f5f74cb0 Mon Sep 17 00:00:00 2001 From: Jessy LANGE <89694096+jessy2027@users.noreply.github.com> Date: Sat, 14 Feb 2026 00:44:04 +0100 Subject: [PATCH] fix(browser): hot-reload profiles added after gateway start (#4841) (#8816) * fix(browser): hot-reload profiles added after gateway start (#4841) * style: format files with oxfmt * Fix hot-reload stale config fields bug in forProfile * Fix test order-dependency in hot-reload profiles test * Fix mock reset order to prevent stale cfgProfiles * Fix config cache blocking hot-reload by clearing cache before loadConfig * test: improve hot-reload test to properly exercise config cache - Add simulated cache behavior in mock - Prime cache before mutating config - Verify stale value without clearConfigCache - Verify fresh value after hot-reload Addresses review comment about test not exercising cache * test: add hot-reload tests for browser profiles in server context. * fix(browser): optimize profile hot-reload to avoid global cache clear * fix(browser): remove unused loadConfig import * fix(test): execute resetModules before test setup * feat: implement browser server context with profile hot-reloading and tab management. * fix(browser): harden profile hot-reload and shutdown cleanup * test(browser): use toSorted in known-profile names test --------- Co-authored-by: Peter Steinberger --- src/browser/control-service.ts | 10 +- ...server-context.hot-reload-profiles.test.ts | 214 ++++++++++++++++++ ...r-context.list-known-profile-names.test.ts | 40 ++++ src/browser/server-context.ts | 60 ++++- src/browser/server-context.types.ts | 1 + src/browser/server.ts | 10 +- src/config/config.ts | 1 + src/config/io.ts | 2 +- 8 files changed, 331 insertions(+), 7 deletions(-) create mode 100644 src/browser/server-context.hot-reload-profiles.test.ts create mode 100644 src/browser/server-context.list-known-profile-names.test.ts diff --git a/src/browser/control-service.ts b/src/browser/control-service.ts index 93bb89e93..55445fce6 100644 --- a/src/browser/control-service.ts +++ b/src/browser/control-service.ts @@ -3,7 +3,11 @@ import { createSubsystemLogger } from "../logging/subsystem.js"; import { resolveBrowserConfig, resolveProfile } from "./config.js"; import { ensureBrowserControlAuth } from "./control-auth.js"; import { ensureChromeExtensionRelayServer } from "./extension-relay.js"; -import { type BrowserServerState, createBrowserRouteContext } from "./server-context.js"; +import { + type BrowserServerState, + createBrowserRouteContext, + listKnownProfileNames, +} from "./server-context.js"; let state: BrowserServerState | null = null; const log = createSubsystemLogger("browser"); @@ -16,6 +20,7 @@ export function getBrowserControlState(): BrowserServerState | null { export function createBrowserControlContext() { return createBrowserRouteContext({ getState: () => state, + refreshConfigFromDisk: true, }); } @@ -71,10 +76,11 @@ export async function stopBrowserControlService(): Promise { const ctx = createBrowserRouteContext({ getState: () => state, + refreshConfigFromDisk: true, }); try { - for (const name of Object.keys(current.resolved.profiles)) { + for (const name of listKnownProfileNames(current)) { try { await ctx.forProfile(name).stopRunningBrowser(); } catch { diff --git a/src/browser/server-context.hot-reload-profiles.test.ts b/src/browser/server-context.hot-reload-profiles.test.ts new file mode 100644 index 000000000..0ff64c234 --- /dev/null +++ b/src/browser/server-context.hot-reload-profiles.test.ts @@ -0,0 +1,214 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +let cfgProfiles: Record = {}; + +// Simulate module-level cache behavior +let cachedConfig: ReturnType | null = null; + +function buildConfig() { + return { + browser: { + enabled: true, + color: "#FF4500", + headless: true, + defaultProfile: "openclaw", + profiles: { ...cfgProfiles }, + }, + }; +} + +vi.mock("../config/config.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createConfigIO: () => ({ + loadConfig: () => { + // Always return fresh config for createConfigIO to simulate fresh disk read + return buildConfig(); + }, + }), + loadConfig: () => { + // simulate stale loadConfig that doesn't see updates unless cache cleared + if (!cachedConfig) { + cachedConfig = buildConfig(); + } + 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", () => { + beforeEach(() => { + vi.resetModules(); + cfgProfiles = { + openclaw: { cdpPort: 18800, color: "#FF4500" }, + }; + cachedConfig = null; // Clear simulated cache + }); + + it("forProfile hot-reloads newly added profiles from config", async () => { + // Start with only openclaw profile + const { createBrowserRouteContext } = await import("./server-context.js"); + const { resolveBrowserConfig } = await import("./config.js"); + const { loadConfig } = await import("../config/config.js"); + + // 1. Prime the cache by calling loadConfig() first + const cfg = loadConfig(); + const resolved = resolveBrowserConfig(cfg.browser, cfg); + + // Verify cache is primed (without desktop) + expect(cfg.browser.profiles.desktop).toBeUndefined(); + const state = { + server: null, + port: 18791, + resolved, + profiles: new Map(), + }; + + const ctx = createBrowserRouteContext({ + getState: () => state, + refreshConfigFromDisk: true, + }); + + // Initially, "desktop" profile should not exist + expect(() => ctx.forProfile("desktop")).toThrow(/not found/); + + // 2. Simulate adding a new profile to config (like user editing openclaw.json) + cfgProfiles.desktop = { cdpUrl: "http://127.0.0.1:9222", color: "#0066CC" }; + + // 3. Verify without clearConfigCache, loadConfig() still returns stale cached value + 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"); + + // 5. Verify the new profile was merged into the cached state + expect(state.resolved.profiles.desktop).toBeDefined(); + + // 6. Verify GLOBAL cache was NOT cleared - subsequent simple loadConfig() still sees STALE value + // 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 + const { clearConfigCache } = await import("../config/config.js"); + expect(clearConfigCache).not.toHaveBeenCalled(); + }); + + it("forProfile still throws for profiles that don't exist in fresh config", async () => { + const { createBrowserRouteContext } = await import("./server-context.js"); + const { resolveBrowserConfig } = await import("./config.js"); + const { loadConfig } = await import("../config/config.js"); + + const cfg = loadConfig(); + const resolved = resolveBrowserConfig(cfg.browser, cfg); + const state = { + server: null, + port: 18791, + resolved, + 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/); + }); + + it("forProfile refreshes existing profile config after loadConfig cache updates", async () => { + const { createBrowserRouteContext } = await import("./server-context.js"); + const { resolveBrowserConfig } = await import("./config.js"); + const { loadConfig } = await import("../config/config.js"); + + const cfg = loadConfig(); + const resolved = resolveBrowserConfig(cfg.browser, cfg); + const state = { + server: null, + port: 18791, + resolved, + 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); + expect(state.resolved.profiles.openclaw?.cdpPort).toBe(19999); + }); + + it("listProfiles refreshes config before enumerating profiles", async () => { + const { createBrowserRouteContext } = await import("./server-context.js"); + const { resolveBrowserConfig } = await import("./config.js"); + const { loadConfig } = await import("../config/config.js"); + + const cfg = loadConfig(); + const resolved = resolveBrowserConfig(cfg.browser, cfg); + const state = { + server: null, + port: 18791, + resolved, + 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); + }); +}); diff --git a/src/browser/server-context.list-known-profile-names.test.ts b/src/browser/server-context.list-known-profile-names.test.ts new file mode 100644 index 000000000..04c897563 --- /dev/null +++ b/src/browser/server-context.list-known-profile-names.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "vitest"; +import type { BrowserServerState } from "./server-context.js"; +import { resolveBrowserConfig, resolveProfile } from "./config.js"; +import { listKnownProfileNames } from "./server-context.js"; + +describe("browser server-context listKnownProfileNames", () => { + it("includes configured and runtime-only profile names", () => { + const resolved = resolveBrowserConfig({ + defaultProfile: "openclaw", + profiles: { + openclaw: { cdpPort: 18800, color: "#FF4500" }, + }, + }); + const openclaw = resolveProfile(resolved, "openclaw"); + if (!openclaw) { + throw new Error("expected openclaw profile"); + } + + const state: BrowserServerState = { + server: null as unknown as BrowserServerState["server"], + port: 18791, + resolved, + profiles: new Map([ + [ + "stale-removed", + { + profile: { ...openclaw, name: "stale-removed" }, + running: null, + }, + ], + ]), + }; + + expect(listKnownProfileNames(state).toSorted()).toEqual([ + "chrome", + "openclaw", + "stale-removed", + ]); + }); +}); diff --git a/src/browser/server-context.ts b/src/browser/server-context.ts index d6e0e8f04..658e75b3d 100644 --- a/src/browser/server-context.ts +++ b/src/browser/server-context.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import type { ResolvedBrowserProfile } from "./config.js"; import type { PwAiModule } from "./pw-ai-module.js"; import type { + BrowserServerState, BrowserRouteContext, BrowserTab, ContextOptions, @@ -9,6 +10,7 @@ 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, @@ -17,7 +19,7 @@ import { resolveOpenClawUserDataDir, stopOpenClawChrome, } from "./chrome.js"; -import { resolveProfile } from "./config.js"; +import { resolveBrowserConfig, resolveProfile } from "./config.js"; import { ensureChromeExtensionRelayServer, stopChromeExtensionRelayServer, @@ -35,6 +37,14 @@ export type { ProfileStatus, } from "./server-context.types.js"; +export function listKnownProfileNames(state: BrowserServerState): string[] { + const names = new Set(Object.keys(state.resolved.profiles)); + for (const name of state.profiles.keys()) { + names.add(name); + } + return [...names]; +} + /** * Normalize a CDP WebSocket URL to use the correct base URL. */ @@ -559,6 +569,8 @@ function createProfileContext( } export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteContext { + const refreshConfigFromDisk = opts.refreshConfigFromDisk === true; + const state = () => { const current = opts.getState(); if (!current) { @@ -567,10 +579,53 @@ 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; - const profile = resolveProfile(current.resolved, name); + let profile = resolveProfile(current.resolved, name); + + // Hot-reload: try fresh config if profile not found + if (!profile) { + refreshResolvedConfigFresh(current); + profile = resolveProfile(current.resolved, name); + } + if (!profile) { const available = Object.keys(current.resolved.profiles).join(", "); throw new Error(`Profile "${name}" not found. Available profiles: ${available || "(none)"}`); @@ -580,6 +635,7 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon const listProfiles = async (): Promise => { const current = state(); + refreshResolvedConfig(current); const result: ProfileStatus[] = []; for (const name of Object.keys(current.resolved.profiles)) { diff --git a/src/browser/server-context.types.ts b/src/browser/server-context.types.ts index 62a8ae028..d9360b849 100644 --- a/src/browser/server-context.types.ts +++ b/src/browser/server-context.types.ts @@ -72,4 +72,5 @@ export type ProfileStatus = { export type ContextOptions = { getState: () => BrowserServerState | null; onEnsureAttachTarget?: (profile: ResolvedBrowserProfile) => Promise; + refreshConfigFromDisk?: boolean; }; diff --git a/src/browser/server.ts b/src/browser/server.ts index 419bdbfdf..03f084f16 100644 --- a/src/browser/server.ts +++ b/src/browser/server.ts @@ -9,7 +9,11 @@ import { ensureBrowserControlAuth, resolveBrowserControlAuth } from "./control-a import { ensureChromeExtensionRelayServer } from "./extension-relay.js"; import { isPwAiLoaded } from "./pw-ai-state.js"; import { registerBrowserRoutes } from "./routes/index.js"; -import { type BrowserServerState, createBrowserRouteContext } from "./server-context.js"; +import { + type BrowserServerState, + createBrowserRouteContext, + listKnownProfileNames, +} from "./server-context.js"; let state: BrowserServerState | null = null; const log = createSubsystemLogger("browser"); @@ -125,6 +129,7 @@ export async function startBrowserControlServerFromConfig(): Promise state, + refreshConfigFromDisk: true, }); registerBrowserRoutes(app as unknown as BrowserRouteRegistrar, ctx); @@ -173,12 +178,13 @@ export async function stopBrowserControlServer(): Promise { const ctx = createBrowserRouteContext({ getState: () => state, + refreshConfigFromDisk: true, }); try { const current = state; if (current) { - for (const name of Object.keys(current.resolved.profiles)) { + for (const name of listKnownProfileNames(current)) { try { await ctx.forProfile(name).stopRunningBrowser(); } catch { diff --git a/src/config/config.ts b/src/config/config.ts index 4761b7b21..db3091c5f 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -1,4 +1,5 @@ export { + clearConfigCache, createConfigIO, loadConfig, parseConfigJson5, diff --git a/src/config/io.ts b/src/config/io.ts index 26d812d14..64434a5a1 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -820,7 +820,7 @@ function shouldUseConfigCache(env: NodeJS.ProcessEnv): boolean { return resolveConfigCacheMs(env) > 0; } -function clearConfigCache(): void { +export function clearConfigCache(): void { configCache = null; }