From 009f494cd94590d4d309d0e1bf5148d4dd6fffb5 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 19 Mar 2026 15:57:22 -0700 Subject: [PATCH] fix(plugin-sdk): stop library import warmup side effects --- src/agents/context.lookup.test.ts | 43 +++++++++++++++++++------------ src/agents/context.ts | 29 ++++++++++++++++----- src/plugin-sdk/root-alias.cjs | 16 ++++++++++++ src/plugin-sdk/root-alias.test.ts | 11 ++++++++ 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/src/agents/context.lookup.test.ts b/src/agents/context.lookup.test.ts index df0e67e6c68..31d702c5420 100644 --- a/src/agents/context.lookup.test.ts +++ b/src/agents/context.lookup.test.ts @@ -6,25 +6,27 @@ function mockContextDeps(params: { loadConfig: () => unknown; discoveredModels?: DiscoveredModel[]; }) { + const ensureOpenClawModelsJson = vi.fn(async () => {}); vi.doMock("../config/config.js", () => ({ loadConfig: params.loadConfig, })); vi.doMock("./models-config.js", () => ({ - ensureOpenClawModelsJson: vi.fn(async () => {}), + ensureOpenClawModelsJson, })); vi.doMock("./agent-paths.js", () => ({ resolveOpenClawAgentDir: () => "/tmp/openclaw-agent", })); - vi.doMock("./pi-model-discovery.js", () => ({ + vi.doMock("./pi-model-discovery-runtime.js", () => ({ discoverAuthStorage: vi.fn(() => ({})), discoverModels: vi.fn(() => ({ getAll: () => params.discoveredModels ?? [], })), })); + return { ensureOpenClawModelsJson }; } function mockContextModuleDeps(loadConfigImpl: () => unknown) { - mockContextDeps({ loadConfig: loadConfigImpl }); + return mockContextDeps({ loadConfig: loadConfigImpl }); } // Shared mock setup used by multiple tests. @@ -80,14 +82,18 @@ describe("lookupContextTokens", () => { expect(lookupContextTokens("openrouter/claude-sonnet")).toBe(321_000); }); - it("only warms eagerly for startup commands that need model metadata", async () => { + it("only warms eagerly for real openclaw startup commands that need model metadata", async () => { const argvSnapshot = process.argv; try { for (const scenario of [ { - argv: ["node", "openclaw", "--profile", "--", "config", "validate"], + argv: ["node", "openclaw", "chat"], expectedCalls: 1, }, + { + argv: ["node", "openclaw", "--profile", "--", "config", "validate"], + expectedCalls: 0, + }, { argv: ["node", "openclaw", "logs", "--limit", "5"], expectedCalls: 0, @@ -97,16 +103,18 @@ describe("lookupContextTokens", () => { expectedCalls: 0, }, { - argv: ["node", "openclaw", "gateway", "status", "--json"], + argv: ["node", "scripts/test-built-plugin-singleton.mjs"], expectedCalls: 0, }, ]) { vi.resetModules(); const loadConfigMock = vi.fn(() => ({ models: {} })); - mockContextModuleDeps(loadConfigMock); + const { ensureOpenClawModelsJson } = mockContextModuleDeps(loadConfigMock); process.argv = scenario.argv; await import("./context.js"); + await flushAsyncWarmup(); expect(loadConfigMock).toHaveBeenCalledTimes(scenario.expectedCalls); + expect(ensureOpenClawModelsJson).toHaveBeenCalledTimes(scenario.expectedCalls); } } finally { process.argv = argvSnapshot; @@ -132,8 +140,6 @@ describe("lookupContextTokens", () => { mockContextModuleDeps(loadConfigMock); - const argvSnapshot = process.argv; - process.argv = ["node", "openclaw", "config", "validate"]; try { const { lookupContextTokens } = await import("./context.js"); expect(lookupContextTokens("openrouter/claude-sonnet")).toBeUndefined(); @@ -144,7 +150,6 @@ describe("lookupContextTokens", () => { expect(lookupContextTokens("openrouter/claude-sonnet")).toBe(654_321); expect(loadConfigMock).toHaveBeenCalledTimes(2); } finally { - process.argv = argvSnapshot; vi.useRealTimers(); } }); @@ -156,7 +161,7 @@ describe("lookupContextTokens", () => { ]); const { lookupContextTokens } = await import("./context.js"); - // Trigger async cache population. + lookupContextTokens("gemini-3.1-pro-preview"); await flushAsyncWarmup(); // Conservative minimum: bare-id cache feeds runtime flush/compaction paths. expect(lookupContextTokens("gemini-3.1-pro-preview")).toBe(128_000); @@ -171,7 +176,8 @@ describe("lookupContextTokens", () => { { id: "google-gemini-cli/gemini-3.1-pro-preview", contextWindow: 1_048_576 }, ]); - const { resolveContextTokensForModel } = await import("./context.js"); + const { lookupContextTokens, resolveContextTokensForModel } = await import("./context.js"); + lookupContextTokens("google-gemini-cli/gemini-3.1-pro-preview"); await flushAsyncWarmup(); // With provider specified and no config override, bare lookup finds the @@ -224,7 +230,9 @@ describe("lookupContextTokens", () => { mockDiscoveryDeps([{ id: "google/gemini-2.5-pro", contextWindow: 999_000 }]); const cfg = createContextOverrideConfig("google", "gemini-2.5-pro", 2_000_000); - const resolveContextTokensForModel = await importResolveContextTokensForModel(); + const { lookupContextTokens, resolveContextTokensForModel } = await import("./context.js"); + lookupContextTokens("google/gemini-2.5-pro"); + await flushAsyncWarmup(); // Google with explicit cfg: config direct scan wins before any cache lookup. const googleResult = resolveContextTokensForModel({ @@ -286,7 +294,9 @@ describe("lookupContextTokens", () => { mockDiscoveryDeps([{ id: "google/gemini-2.5-pro", contextWindow: 999_000 }]); const cfg = createContextOverrideConfig("google", "gemini-2.5-pro", 2_000_000); - const resolveContextTokensForModel = await importResolveContextTokensForModel(); + const { lookupContextTokens, resolveContextTokensForModel } = await import("./context.js"); + lookupContextTokens("google/gemini-2.5-pro"); + await flushAsyncWarmup(); // model-only call (no explicit provider) must NOT apply config direct scan. // Falls through to bare cache lookup: "google/gemini-2.5-pro" → 999k ✓. @@ -317,8 +327,9 @@ describe("lookupContextTokens", () => { { id: "google-gemini-cli/gemini-3.1-pro-preview", contextWindow: 1_048_576 }, ]); - const { resolveContextTokensForModel } = await import("./context.js"); - await new Promise((r) => setTimeout(r, 0)); + const { lookupContextTokens, resolveContextTokensForModel } = await import("./context.js"); + lookupContextTokens("google-gemini-cli/gemini-3.1-pro-preview"); + await flushAsyncWarmup(); // Qualified "google-gemini-cli/gemini-3.1-pro-preview" → 1M wins over // bare "gemini-3.1-pro-preview" → 128k (cross-provider minimum). diff --git a/src/agents/context.ts b/src/agents/context.ts index cfeee26cd60..279433f0c76 100644 --- a/src/agents/context.ts +++ b/src/agents/context.ts @@ -1,6 +1,7 @@ // Lazy-load pi-coding-agent model metadata so we can infer context windows when // the agent reports a model id. This includes custom models.json entries. +import path from "node:path"; import { loadConfig } from "../config/config.js"; import type { OpenClawConfig } from "../config/config.js"; import { computeBackoff, type BackoffPolicy } from "../infra/backoff.js"; @@ -84,6 +85,19 @@ let configuredConfig: OpenClawConfig | undefined; let configLoadFailures = 0; let nextConfigLoadAttemptAtMs = 0; +function isLikelyOpenClawCliProcess(argv: string[] = process.argv): boolean { + const entryBasename = path + .basename(argv[1] ?? "") + .trim() + .toLowerCase(); + return ( + entryBasename === "openclaw" || + entryBasename === "openclaw.mjs" || + entryBasename === "entry.js" || + entryBasename === "entry.mjs" + ); +} + function getCommandPathFromArgv(argv: string[]): string[] { const args = argv.slice(2); const tokens: string[] = []; @@ -125,9 +139,12 @@ const SKIP_EAGER_WARMUP_PRIMARY_COMMANDS = new Set([ "webhooks", ]); -function shouldSkipEagerContextWindowWarmup(argv: string[] = process.argv): boolean { +function shouldEagerWarmContextWindowCache(argv: string[] = process.argv): boolean { + if (!isLikelyOpenClawCliProcess(argv)) { + return false; + } const [primary] = getCommandPathFromArgv(argv); - return primary ? SKIP_EAGER_WARMUP_PRIMARY_COMMANDS.has(primary) : false; + return Boolean(primary) && !SKIP_EAGER_WARMUP_PRIMARY_COMMANDS.has(primary); } function primeConfiguredContextWindows(): OpenClawConfig | undefined { @@ -205,14 +222,14 @@ export function lookupContextTokens(modelId?: string): number | undefined { if (!modelId) { return undefined; } - // Best-effort: kick off loading, but don't block. + // Best-effort: kick off loading on demand, but don't block lookups. void ensureContextWindowCacheLoaded(); return MODEL_CACHE.get(modelId); } -if (!shouldSkipEagerContextWindowWarmup()) { - // Keep prior behavior where model limits begin loading during startup. - // This avoids a cold-start miss on the first context token lookup. +if (shouldEagerWarmContextWindowCache()) { + // Keep startup warmth for the real CLI, but avoid import-time side effects + // when this module is pulled in through library/plugin-sdk surfaces. void ensureContextWindowCacheLoaded(); } diff --git a/src/plugin-sdk/root-alias.cjs b/src/plugin-sdk/root-alias.cjs index d9d742c3070..23e583f8c4d 100644 --- a/src/plugin-sdk/root-alias.cjs +++ b/src/plugin-sdk/root-alias.cjs @@ -113,6 +113,13 @@ const fastExports = { const target = { ...fastExports }; let rootExports = null; +function shouldResolveMonolithic(prop) { + if (typeof prop !== "string") { + return false; + } + return prop !== "then"; +} + function getMonolithicSdk() { const loaded = tryLoadMonolithicSdk(); if (loaded && typeof loaded === "object") { @@ -125,6 +132,9 @@ function getExportValue(prop) { if (Reflect.has(target, prop)) { return Reflect.get(target, prop); } + if (!shouldResolveMonolithic(prop)) { + return undefined; + } const monolithic = getMonolithicSdk(); if (!monolithic) { return undefined; @@ -137,6 +147,9 @@ function getExportDescriptor(prop) { if (ownDescriptor) { return ownDescriptor; } + if (!shouldResolveMonolithic(prop)) { + return undefined; + } const monolithic = getMonolithicSdk(); if (!monolithic) { @@ -166,6 +179,9 @@ rootExports = new Proxy(target, { if (Reflect.has(target, prop)) { return true; } + if (!shouldResolveMonolithic(prop)) { + return false; + } const monolithic = getMonolithicSdk(); return monolithic ? Reflect.has(monolithic, prop) : false; }, diff --git a/src/plugin-sdk/root-alias.test.ts b/src/plugin-sdk/root-alias.test.ts index 95565cab89a..83937c34b44 100644 --- a/src/plugin-sdk/root-alias.test.ts +++ b/src/plugin-sdk/root-alias.test.ts @@ -109,6 +109,17 @@ describe("plugin-sdk root alias", () => { expect(lazyModule.jitiLoadCalls).toBe(0); }); + it("does not load the monolithic sdk for promise-like or symbol reflection probes", () => { + const lazyModule = loadRootAliasWithStubs(); + const lazyRootSdk = lazyModule.moduleExports; + + expect("then" in lazyRootSdk).toBe(false); + expect(Reflect.get(lazyRootSdk, Symbol.toStringTag)).toBeUndefined(); + expect(Object.getOwnPropertyDescriptor(lazyRootSdk, Symbol.toStringTag)).toBeUndefined(); + expect(lazyModule.createJitiCalls).toBe(0); + expect(lazyModule.jitiLoadCalls).toBe(0); + }); + it("loads legacy root exports on demand and preserves reflection", () => { const lazyModule = loadRootAliasWithStubs({ monolithicExports: {