From f3be1c828c6d5a31aa388e778ad9cce3f96537e0 Mon Sep 17 00:00:00 2001 From: 0x4C33 <60883781+haoruilee@users.noreply.github.com> Date: Thu, 12 Mar 2026 22:00:36 +0800 Subject: [PATCH] fix(status): resolve context window by provider-qualified key, prefer max on bare-id collision, solve #35976 (#36389) Merged via squash. Prepared head SHA: f8cf752c59708fb388fd200276115277e8b217d6 Co-authored-by: haoruilee <60883781+haoruilee@users.noreply.github.com> Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com> Reviewed-by: @jalehman --- CHANGELOG.md | 3 + src/agents/context.lookup.test.ts | 236 ++++++++++++++++++++++++++++++ src/agents/context.test.ts | 60 +++++++- src/agents/context.ts | 124 +++++++++++++++- 4 files changed, 413 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 563e7e6b132..f69befffc2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,9 @@ Docs: https://docs.openclaw.ai - Telegram/direct delivery: bridge direct delivery sends to internal `message:sent` hooks so internal hook listeners observe successful Telegram deliveries. (#40185) Thanks @vincentkoc. - Dependencies: refresh workspace dependencies except the pinned Carbon package, and harden ACP session-config writes against non-string SDK values so newer ACP clients fail fast instead of tripping type/runtime mismatches. - Telegram/polling restarts: clear bounded cleanup timeout handles after `runner.stop()` and `bot.stop()` settle so stall recovery no longer leaves stray 15-second timers behind on clean shutdown. (#43188) thanks @kyohwang. +- Gateway/config errors: surface up to three validation issues in top-level `config.set`, `config.patch`, and `config.apply` error messages while preserving structured issue details. (#42664) Thanks @huntharo. +- Hooks/plugin context parity followup: pass `trigger` and `channelId` through embedded `llm_input`, `agent_end`, and `llm_output` hook contexts so plugins receive the same agent metadata across hook phases. (#42362) Thanks @zhoulf1006. +- Status/context windows: normalize provider-qualified override cache keys so `/status` resolves the active provider's configured context window even when `models.providers` keys use mixed case or surrounding whitespace. (#36389) Thanks @haoruilee. ## 2026.3.8 diff --git a/src/agents/context.lookup.test.ts b/src/agents/context.lookup.test.ts index 584f9c27cbb..428d47759bc 100644 --- a/src/agents/context.lookup.test.ts +++ b/src/agents/context.lookup.test.ts @@ -18,6 +18,26 @@ function mockContextModuleDeps(loadConfigImpl: () => unknown) { })); } +// Shared mock setup used by multiple tests. +function mockDiscoveryDeps( + models: Array<{ id: string; contextWindow: number }>, + configModels?: Record }>, +) { + vi.doMock("../config/config.js", () => ({ + loadConfig: () => ({ models: configModels ? { providers: configModels } : {} }), + })); + vi.doMock("./models-config.js", () => ({ + ensureOpenClawModelsJson: vi.fn(async () => {}), + })); + vi.doMock("./agent-paths.js", () => ({ + resolveOpenClawAgentDir: () => "/tmp/openclaw-agent", + })); + vi.doMock("./pi-model-discovery.js", () => ({ + discoverAuthStorage: vi.fn(() => ({})), + discoverModels: vi.fn(() => ({ getAll: () => models })), + })); +} + describe("lookupContextTokens", () => { beforeEach(() => { vi.resetModules(); @@ -87,4 +107,220 @@ describe("lookupContextTokens", () => { vi.useRealTimers(); } }); + + it("returns the smaller window when the same bare model id is discovered under multiple providers", async () => { + mockDiscoveryDeps([ + { id: "gemini-3.1-pro-preview", contextWindow: 1_048_576 }, + { id: "gemini-3.1-pro-preview", contextWindow: 128_000 }, + ]); + + const { lookupContextTokens } = await import("./context.js"); + // Trigger async cache population. + await new Promise((r) => setTimeout(r, 0)); + // Conservative minimum: bare-id cache feeds runtime flush/compaction paths. + expect(lookupContextTokens("gemini-3.1-pro-preview")).toBe(128_000); + }); + + it("resolveContextTokensForModel returns discovery value when provider-qualified entry exists in cache", async () => { + // Registry returns provider-qualified entries (real-world scenario from #35976). + // When no explicit config override exists, the bare cache lookup hits the + // provider-qualified raw discovery entry. + mockDiscoveryDeps([ + { id: "github-copilot/gemini-3.1-pro-preview", contextWindow: 128_000 }, + { 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)); + + // With provider specified and no config override, bare lookup finds the + // provider-qualified discovery entry. + const result = resolveContextTokensForModel({ + provider: "google-gemini-cli", + model: "gemini-3.1-pro-preview", + }); + expect(result).toBe(1_048_576); + }); + + it("resolveContextTokensForModel returns configured override via direct config scan (beats discovery)", async () => { + // Config has an explicit contextWindow; resolveContextTokensForModel should + // return it via direct config scan, preventing collisions with raw discovery + // entries. Real callers (status.summary.ts etc.) always pass cfg. + mockDiscoveryDeps([ + { id: "google-gemini-cli/gemini-3.1-pro-preview", contextWindow: 1_048_576 }, + ]); + + const cfg = { + models: { + providers: { + "google-gemini-cli": { + models: [{ id: "gemini-3.1-pro-preview", contextWindow: 200_000 }], + }, + }, + }, + }; + + const { resolveContextTokensForModel } = await import("./context.js"); + await new Promise((r) => setTimeout(r, 0)); + + const result = resolveContextTokensForModel({ + cfg: cfg as never, + provider: "google-gemini-cli", + model: "gemini-3.1-pro-preview", + }); + expect(result).toBe(200_000); + }); + + it("resolveContextTokensForModel honors configured overrides when provider keys use mixed case", async () => { + mockDiscoveryDeps([{ id: "openrouter/anthropic/claude-sonnet-4-5", contextWindow: 1_048_576 }]); + + const cfg = { + models: { + providers: { + " OpenRouter ": { + models: [{ id: "anthropic/claude-sonnet-4-5", contextWindow: 200_000 }], + }, + }, + }, + }; + + const { resolveContextTokensForModel } = await import("./context.js"); + await new Promise((r) => setTimeout(r, 0)); + + const result = resolveContextTokensForModel({ + cfg: cfg as never, + provider: "openrouter", + model: "anthropic/claude-sonnet-4-5", + }); + expect(result).toBe(200_000); + }); + + it("resolveContextTokensForModel: config direct scan prevents OpenRouter qualified key collision for Google provider", async () => { + // When provider is explicitly "google" and cfg has a Google contextWindow + // override, the config direct scan returns it before any cache lookup — + // so the OpenRouter raw "google/gemini-2.5-pro" qualified entry is never hit. + // Real callers (status.summary.ts) always pass cfg when provider is explicit. + mockDiscoveryDeps([{ id: "google/gemini-2.5-pro", contextWindow: 999_000 }]); + + const cfg = { + models: { + providers: { + google: { models: [{ id: "gemini-2.5-pro", contextWindow: 2_000_000 }] }, + }, + }, + }; + + const { resolveContextTokensForModel } = await import("./context.js"); + await new Promise((r) => setTimeout(r, 0)); + + // Google with explicit cfg: config direct scan wins before any cache lookup. + const googleResult = resolveContextTokensForModel({ + cfg: cfg as never, + provider: "google", + model: "gemini-2.5-pro", + }); + expect(googleResult).toBe(2_000_000); + + // OpenRouter provider with slash model id: bare lookup finds the raw entry. + const openrouterResult = resolveContextTokensForModel({ + provider: "openrouter", + model: "google/gemini-2.5-pro", + }); + expect(openrouterResult).toBe(999_000); + }); + + it("resolveContextTokensForModel prefers exact provider key over alias-normalized match", async () => { + // When both "qwen" and "qwen-portal" exist as config keys (alias pattern), + // resolveConfiguredProviderContextWindow must return the exact-key match first, + // not the first normalized hit — mirroring pi-embedded-runner/model.ts behaviour. + mockDiscoveryDeps([]); + + const cfg = { + models: { + providers: { + "qwen-portal": { models: [{ id: "qwen-max", contextWindow: 32_000 }] }, + qwen: { models: [{ id: "qwen-max", contextWindow: 128_000 }] }, + }, + }, + }; + + const { resolveContextTokensForModel } = await import("./context.js"); + await new Promise((r) => setTimeout(r, 0)); + + // Exact key "qwen" wins over the alias-normalized match "qwen-portal". + const qwenResult = resolveContextTokensForModel({ + cfg: cfg as never, + provider: "qwen", + model: "qwen-max", + }); + expect(qwenResult).toBe(128_000); + + // Exact key "qwen-portal" wins (no alias lookup needed). + const portalResult = resolveContextTokensForModel({ + cfg: cfg as never, + provider: "qwen-portal", + model: "qwen-max", + }); + expect(portalResult).toBe(32_000); + }); + + it("resolveContextTokensForModel(model-only) does not apply config scan for inferred provider", async () => { + // status.ts log-usage fallback calls resolveContextTokensForModel({ model }) + // with no provider. When model = "google/gemini-2.5-pro" (OpenRouter ID), + // resolveProviderModelRef infers provider="google". Without the guard, + // resolveConfiguredProviderContextWindow would return Google's configured + // window and misreport context limits for the OpenRouter session. + mockDiscoveryDeps([{ id: "google/gemini-2.5-pro", contextWindow: 999_000 }]); + + const cfg = { + models: { + providers: { + google: { models: [{ id: "gemini-2.5-pro", contextWindow: 2_000_000 }] }, + }, + }, + }; + + const { resolveContextTokensForModel } = await import("./context.js"); + await new Promise((r) => setTimeout(r, 0)); + + // model-only call (no explicit provider) must NOT apply config direct scan. + // Falls through to bare cache lookup: "google/gemini-2.5-pro" → 999k ✓. + const modelOnlyResult = resolveContextTokensForModel({ + cfg: cfg as never, + model: "google/gemini-2.5-pro", + // no provider + }); + expect(modelOnlyResult).toBe(999_000); + + // Explicit provider still uses config scan ✓. + const explicitResult = resolveContextTokensForModel({ + cfg: cfg as never, + provider: "google", + model: "gemini-2.5-pro", + }); + expect(explicitResult).toBe(2_000_000); + }); + + it("resolveContextTokensForModel: qualified key beats bare min when provider is explicit (original #35976 fix)", async () => { + // Regression: when both "gemini-3.1-pro-preview" (bare, min=128k) AND + // "google-gemini-cli/gemini-3.1-pro-preview" (qualified, 1M) are in cache, + // an explicit-provider call must return the provider-specific qualified value, + // not the collided bare minimum. + mockDiscoveryDeps([ + { id: "github-copilot/gemini-3.1-pro-preview", contextWindow: 128_000 }, + { id: "gemini-3.1-pro-preview", contextWindow: 128_000 }, + { 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)); + + // Qualified "google-gemini-cli/gemini-3.1-pro-preview" → 1M wins over + // bare "gemini-3.1-pro-preview" → 128k (cross-provider minimum). + const result = resolveContextTokensForModel({ + provider: "google-gemini-cli", + model: "gemini-3.1-pro-preview", + }); + expect(result).toBe(1_048_576); + }); }); diff --git a/src/agents/context.test.ts b/src/agents/context.test.ts index 267755a8849..98eb99d7295 100644 --- a/src/agents/context.test.ts +++ b/src/agents/context.test.ts @@ -8,23 +8,44 @@ import { import { createSessionManagerRuntimeRegistry } from "./pi-extensions/session-manager-runtime-registry.js"; describe("applyDiscoveredContextWindows", () => { - it("keeps the smallest context window when duplicate model ids are discovered", () => { + it("keeps the smallest context window when the same bare model id appears under multiple providers", () => { const cache = new Map(); applyDiscoveredContextWindows({ cache, models: [ - { id: "claude-sonnet-4-5", contextWindow: 1_000_000 }, - { id: "claude-sonnet-4-5", contextWindow: 200_000 }, + { id: "gemini-3.1-pro-preview", contextWindow: 128_000 }, + { id: "gemini-3.1-pro-preview", contextWindow: 1_048_576 }, ], }); - expect(cache.get("claude-sonnet-4-5")).toBe(200_000); + // Keep the conservative (minimum) value: this cache feeds runtime paths such + // as flush thresholds and session persistence, not just /status display. + // Callers with a known provider should use resolveContextTokensForModel which + // tries the provider-qualified key first. + expect(cache.get("gemini-3.1-pro-preview")).toBe(128_000); + }); + + it("stores provider-qualified entries independently", () => { + const cache = new Map(); + applyDiscoveredContextWindows({ + cache, + models: [ + { id: "github-copilot/gemini-3.1-pro-preview", contextWindow: 128_000 }, + { id: "google-gemini-cli/gemini-3.1-pro-preview", contextWindow: 1_048_576 }, + ], + }); + + expect(cache.get("github-copilot/gemini-3.1-pro-preview")).toBe(128_000); + expect(cache.get("google-gemini-cli/gemini-3.1-pro-preview")).toBe(1_048_576); }); }); describe("applyConfiguredContextWindows", () => { - it("overrides discovered cache values with explicit models.providers contextWindow", () => { - const cache = new Map([["anthropic/claude-opus-4-6", 1_000_000]]); + it("writes bare model id to cache; does not touch raw provider-qualified discovery entries", () => { + // Discovery stored a provider-qualified entry; config override goes into the + // bare key only. resolveContextTokensForModel now scans config directly, so + // there is no need (and no benefit) to also write a synthetic qualified key. + const cache = new Map([["openrouter/anthropic/claude-opus-4-6", 1_000_000]]); applyConfiguredContextWindows({ cache, modelsConfig: { @@ -37,6 +58,33 @@ describe("applyConfiguredContextWindows", () => { }); expect(cache.get("anthropic/claude-opus-4-6")).toBe(200_000); + // Discovery entry is untouched — no synthetic write that could corrupt + // an unrelated provider's raw slash-containing model ID. + expect(cache.get("openrouter/anthropic/claude-opus-4-6")).toBe(1_000_000); + }); + + it("does not write synthetic provider-qualified keys; only bare model ids go into cache", () => { + // applyConfiguredContextWindows must NOT write "google-gemini-cli/gemini-3.1-pro-preview" + // into the cache — that keyspace is reserved for raw discovery model IDs and + // a synthetic write would overwrite unrelated entries (e.g. OpenRouter's + // "google/gemini-2.5-pro" being clobbered by a Google provider config). + const cache = new Map(); + cache.set("google-gemini-cli/gemini-3.1-pro-preview", 1_048_576); // discovery entry + applyConfiguredContextWindows({ + cache, + modelsConfig: { + providers: { + "google-gemini-cli": { + models: [{ id: "gemini-3.1-pro-preview", contextWindow: 200_000 }], + }, + }, + }, + }); + + // Bare key is written. + expect(cache.get("gemini-3.1-pro-preview")).toBe(200_000); + // Discovery entry is NOT overwritten. + expect(cache.get("google-gemini-cli/gemini-3.1-pro-preview")).toBe(1_048_576); }); it("adds config-only model context windows and ignores invalid entries", () => { diff --git a/src/agents/context.ts b/src/agents/context.ts index bd3aeaf6fc2..d705438bd50 100644 --- a/src/agents/context.ts +++ b/src/agents/context.ts @@ -6,6 +6,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { computeBackoff, type BackoffPolicy } from "../infra/backoff.js"; import { consumeRootOptionToken, FLAG_TERMINATOR } from "../infra/cli-root-options.js"; import { resolveOpenClawAgentDir } from "./agent-paths.js"; +import { normalizeProviderId } from "./model-selection.js"; import { ensureOpenClawModelsJson } from "./models-config.js"; type ModelEntry = { id: string; contextWindow?: number }; @@ -41,8 +42,12 @@ export function applyDiscoveredContextWindows(params: { continue; } const existing = params.cache.get(model.id); - // When multiple providers expose the same model id with different limits, - // prefer the smaller window so token budgeting is fail-safe (no overestimation). + // When the same bare model id appears under multiple providers with different + // limits, keep the smaller window. This cache feeds both display paths and + // runtime paths (flush thresholds, session context-token persistence), so + // overestimating the limit could delay compaction and cause context overflow. + // Callers that know the active provider should use resolveContextTokensForModel, + // which tries the provider-qualified key first and falls back here. if (existing === undefined || contextWindow < existing) { params.cache.set(model.id, contextWindow); } @@ -222,13 +227,15 @@ function resolveProviderModelRef(params: { } const providerRaw = params.provider?.trim(); if (providerRaw) { + // Keep the exact (lowercased) provider key; callers that need the canonical + // alias (e.g. cache key construction) apply normalizeProviderId explicitly. return { provider: providerRaw.toLowerCase(), model: modelRaw }; } const slash = modelRaw.indexOf("/"); if (slash <= 0) { return undefined; } - const provider = modelRaw.slice(0, slash).trim().toLowerCase(); + const provider = normalizeProviderId(modelRaw.slice(0, slash)); const model = modelRaw.slice(slash + 1).trim(); if (!provider || !model) { return undefined; @@ -236,6 +243,58 @@ function resolveProviderModelRef(params: { return { provider, model }; } +// Look up an explicit contextWindow override for a specific provider+model +// directly from config, without going through the shared discovery cache. +// This avoids the cache keyspace collision where "provider/model" synthetic +// keys overlap with raw slash-containing model IDs (e.g. OpenRouter's +// "google/gemini-2.5-pro" stored as a raw catalog entry). +function resolveConfiguredProviderContextWindow( + cfg: OpenClawConfig | undefined, + provider: string, + model: string, +): number | undefined { + const providers = (cfg?.models as ModelsConfig | undefined)?.providers; + if (!providers) { + return undefined; + } + + // Mirror the lookup order in pi-embedded-runner/model.ts: exact key first, + // then normalized fallback. This prevents alias collisions (e.g. when both + // "qwen" and "qwen-portal" exist as config keys) from picking the wrong + // contextWindow based on Object.entries iteration order. + function findContextWindow(matchProviderId: (id: string) => boolean): number | undefined { + for (const [providerId, providerConfig] of Object.entries(providers!)) { + if (!matchProviderId(providerId)) { + continue; + } + if (!Array.isArray(providerConfig?.models)) { + continue; + } + for (const m of providerConfig.models) { + if ( + typeof m?.id === "string" && + m.id === model && + typeof m?.contextWindow === "number" && + m.contextWindow > 0 + ) { + return m.contextWindow; + } + } + } + return undefined; + } + + // 1. Exact match (case-insensitive, no alias expansion). + const exactResult = findContextWindow((id) => id.trim().toLowerCase() === provider.toLowerCase()); + if (exactResult !== undefined) { + return exactResult; + } + + // 2. Normalized fallback: covers alias keys such as "qwen" → "qwen-portal". + const normalizedProvider = normalizeProviderId(provider); + return findContextWindow((id) => normalizeProviderId(id) === normalizedProvider); +} + function isAnthropic1MModel(provider: string, model: string): boolean { if (provider !== "anthropic") { return false; @@ -267,7 +326,64 @@ export function resolveContextTokensForModel(params: { if (modelParams?.context1m === true && isAnthropic1MModel(ref.provider, ref.model)) { return ANTHROPIC_CONTEXT_1M_TOKENS; } + // Only do the config direct scan when the caller explicitly passed a + // provider. When provider is inferred from a slash in the model string + // (e.g. "google/gemini-2.5-pro" → ref.provider = "google"), the model ID + // may belong to a DIFFERENT provider (e.g. an OpenRouter session). Scanning + // cfg.models.providers.google in that case would return Google's configured + // window and misreport context limits for the OpenRouter session. + // See status.ts log-usage fallback which calls with only { model } set. + if (params.provider) { + const configuredWindow = resolveConfiguredProviderContextWindow( + params.cfg, + ref.provider, + ref.model, + ); + if (configuredWindow !== undefined) { + return configuredWindow; + } + } } - return lookupContextTokens(params.model) ?? params.fallbackContextTokens; + // When provider is explicitly given and the model ID is bare (no slash), + // try the provider-qualified cache key BEFORE the bare key. Discovery + // entries are stored under qualified IDs (e.g. "google-gemini-cli/ + // gemini-3.1-pro-preview → 1M"), while the bare key may hold a cross- + // provider minimum (128k). Returning the qualified entry gives the correct + // provider-specific window for /status and session context-token persistence. + // + // Guard: only when params.provider is explicit (not inferred from a slash in + // the model string). For model-only callers (e.g. status.ts log-usage + // fallback with model="google/gemini-2.5-pro"), the inferred provider would + // construct "google/gemini-2.5-pro" as the qualified key which accidentally + // matches OpenRouter's raw discovery entry — the bare lookup is correct there. + if (params.provider && ref && !ref.model.includes("/")) { + const qualifiedResult = lookupContextTokens( + `${normalizeProviderId(ref.provider)}/${ref.model}`, + ); + if (qualifiedResult !== undefined) { + return qualifiedResult; + } + } + + // Bare key fallback. For model-only calls with slash-containing IDs + // (e.g. "google/gemini-2.5-pro") this IS the raw discovery cache key. + const bareResult = lookupContextTokens(params.model); + if (bareResult !== undefined) { + return bareResult; + } + + // When provider is implicit, try qualified as a last resort so inferred + // provider/model pairs (e.g. model="google-gemini-cli/gemini-3.1-pro") + // still find discovery entries stored under that qualified ID. + if (!params.provider && ref && !ref.model.includes("/")) { + const qualifiedResult = lookupContextTokens( + `${normalizeProviderId(ref.provider)}/${ref.model}`, + ); + if (qualifiedResult !== undefined) { + return qualifiedResult; + } + } + + return params.fallbackContextTokens; }