fix(agents): warn clearly on unresolved model ids (#39215, thanks @ademczuk)
Co-authored-by: ademczuk <andrew.demczuk@gmail.com>
This commit is contained in:
@@ -296,6 +296,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Telegram error-surface resilience: return a user-visible fallback reply when dispatch/debounce processing fails instead of going silent, while preserving draft-stream cleanup and best-effort thread-scoped fallback delivery. (#39209) Thanks @riftzen-bit.
|
||||
- Gateway/password auth startup diagnostics: detect unresolved provider-reference objects in `gateway.auth.password` and fail with a specific bootstrap-secrets error message instead of generic misconfiguration output. (#39230) Thanks @ademczuk.
|
||||
- Agents/OpenAI-responses compatibility: strip unsupported `store` payload fields when `supportsStore=false` (including OpenAI-compatible non-OpenAI providers) while preserving server-compaction payload behavior. (#39219) Thanks @ademczuk.
|
||||
- Agents/model fallback visibility: warn when configured model IDs cannot be resolved and fallback is applied, with log-safe sanitization of model text to prevent control-sequence injection in warning output. (#39215) Thanks @ademczuk.
|
||||
|
||||
## 2026.3.2
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { resetLogger, setLoggerOverride } from "../logging/logger.js";
|
||||
import type { AuthProfileStore } from "./auth-profiles.js";
|
||||
import { saveAuthProfileStore } from "./auth-profiles.js";
|
||||
import { AUTH_STORE_VERSION } from "./auth-profiles/constants.js";
|
||||
@@ -489,6 +490,34 @@ describe("runWithModelFallback", () => {
|
||||
expect(run.mock.calls[1]?.[1]).toBe("claude-haiku-3-5");
|
||||
});
|
||||
|
||||
it("warns when falling back due to model_not_found", async () => {
|
||||
setLoggerOverride({ level: "silent", consoleLevel: "warn" });
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
try {
|
||||
const cfg = makeCfg();
|
||||
const run = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error("Model not found: openai/gpt-6"))
|
||||
.mockResolvedValueOnce("ok");
|
||||
|
||||
const result = await runWithModelFallback({
|
||||
cfg,
|
||||
provider: "openai",
|
||||
model: "gpt-6",
|
||||
run,
|
||||
});
|
||||
|
||||
expect(result.result).toBe("ok");
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Model "openai/gpt-6" not found'),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
setLoggerOverride(null);
|
||||
resetLogger();
|
||||
}
|
||||
});
|
||||
|
||||
it("skips providers when all profiles are in cooldown", async () => {
|
||||
await expectSkippedUnavailableProvider({
|
||||
providerPrefix: "cooldown-test",
|
||||
|
||||
@@ -3,6 +3,8 @@ import {
|
||||
resolveAgentModelFallbackValues,
|
||||
resolveAgentModelPrimaryValue,
|
||||
} from "../config/model-input.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { sanitizeForLog } from "../terminal/ansi.js";
|
||||
import {
|
||||
ensureAuthProfileStore,
|
||||
getSoonestCooldownExpiry,
|
||||
@@ -28,6 +30,8 @@ import {
|
||||
import type { FailoverReason } from "./pi-embedded-helpers.js";
|
||||
import { isLikelyContextOverflowError } from "./pi-embedded-helpers.js";
|
||||
|
||||
const log = createSubsystemLogger("model-fallback");
|
||||
|
||||
type ModelCandidate = {
|
||||
provider: string;
|
||||
model: string;
|
||||
@@ -527,6 +531,13 @@ export async function runWithModelFallback<T>(params: {
|
||||
options: runOptions,
|
||||
});
|
||||
if ("success" in attemptRun) {
|
||||
const notFoundAttempt =
|
||||
i > 0 ? attempts.find((a) => a.reason === "model_not_found") : undefined;
|
||||
if (notFoundAttempt) {
|
||||
log.warn(
|
||||
`Model "${sanitizeForLog(notFoundAttempt.provider)}/${sanitizeForLog(notFoundAttempt.model)}" not found. Fell back to "${sanitizeForLog(candidate.provider)}/${sanitizeForLog(candidate.model)}".`,
|
||||
);
|
||||
}
|
||||
return attemptRun.success;
|
||||
}
|
||||
const err = attemptRun.error;
|
||||
|
||||
@@ -558,6 +558,35 @@ describe("model-selection", () => {
|
||||
});
|
||||
expect(result).toEqual({ provider: "anthropic", model: "claude-opus-4-6" });
|
||||
});
|
||||
|
||||
it("should warn when specified model cannot be resolved and falls back to default", () => {
|
||||
setLoggerOverride({ level: "silent", consoleLevel: "warn" });
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
try {
|
||||
const cfg: Partial<OpenClawConfig> = {
|
||||
agents: {
|
||||
defaults: {
|
||||
model: { primary: "openai/" },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const result = resolveConfiguredModelRef({
|
||||
cfg: cfg as OpenClawConfig,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-opus-4-6",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ provider: "anthropic", model: "claude-opus-4-6" });
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Falling back to default "anthropic/claude-opus-4-6"'),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
setLoggerOverride(null);
|
||||
resetLogger();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveThinkingDefault", () => {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { resolveAgentModelPrimaryValue, toAgentModelListLike } from "../config/model-input.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { sanitizeForLog } from "../terminal/ansi.js";
|
||||
import { resolveAgentConfig, resolveAgentEffectiveModelPrimary } from "./agent-scope.js";
|
||||
import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "./defaults.js";
|
||||
import type { ModelCatalogEntry } from "./model-catalog.js";
|
||||
@@ -302,8 +303,9 @@ export function resolveConfiguredModelRef(params: {
|
||||
}
|
||||
|
||||
// Default to anthropic if no provider is specified, but warn as this is deprecated.
|
||||
const safeTrimmed = sanitizeForLog(trimmed);
|
||||
log.warn(
|
||||
`Model "${trimmed}" specified without provider. Falling back to "anthropic/${trimmed}". Please use "anthropic/${trimmed}" in your config.`,
|
||||
`Model "${safeTrimmed}" specified without provider. Falling back to "anthropic/${safeTrimmed}". Please use "anthropic/${safeTrimmed}" in your config.`,
|
||||
);
|
||||
return { provider: "anthropic", model: trimmed };
|
||||
}
|
||||
@@ -316,6 +318,11 @@ export function resolveConfiguredModelRef(params: {
|
||||
if (resolved) {
|
||||
return resolved.ref;
|
||||
}
|
||||
|
||||
// User specified a model but it could not be resolved — warn before falling back.
|
||||
const safe = sanitizeForLog(trimmed);
|
||||
const safeFallback = sanitizeForLog(`${params.defaultProvider}/${params.defaultModel}`);
|
||||
log.warn(`Model "${safe}" could not be resolved. Falling back to default "${safeFallback}".`);
|
||||
}
|
||||
// Before falling back to the hardcoded default, check if the default provider
|
||||
// is actually available. If it isn't but other providers are configured, prefer
|
||||
|
||||
14
src/terminal/ansi.test.ts
Normal file
14
src/terminal/ansi.test.ts
Normal file
@@ -0,0 +1,14 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { sanitizeForLog, stripAnsi } from "./ansi.js";
|
||||
|
||||
describe("terminal ansi helpers", () => {
|
||||
it("strips ANSI and OSC8 sequences", () => {
|
||||
expect(stripAnsi("\u001B[31mred\u001B[0m")).toBe("red");
|
||||
expect(stripAnsi("\u001B]8;;https://openclaw.ai\u001B\\link\u001B]8;;\u001B\\")).toBe("link");
|
||||
});
|
||||
|
||||
it("sanitizes control characters for log-safe interpolation", () => {
|
||||
const input = "\u001B[31mwarn\u001B[0m\r\nnext\u0000line\u007f";
|
||||
expect(sanitizeForLog(input)).toBe("warnnextline");
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,19 @@ export function stripAnsi(input: string): string {
|
||||
return input.replace(OSC8_REGEX, "").replace(ANSI_REGEX, "");
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a value for safe interpolation into log messages.
|
||||
* Strips ANSI escape sequences, C0 control characters (U+0000–U+001F),
|
||||
* and DEL (U+007F) to prevent log forging / terminal escape injection (CWE-117).
|
||||
*/
|
||||
export function sanitizeForLog(v: string): string {
|
||||
let out = stripAnsi(v);
|
||||
for (let c = 0; c <= 0x1f; c++) {
|
||||
out = out.replaceAll(String.fromCharCode(c), "");
|
||||
}
|
||||
return out.replaceAll(String.fromCharCode(0x7f), "");
|
||||
}
|
||||
|
||||
export function visibleWidth(input: string): number {
|
||||
return Array.from(stripAnsi(input)).length;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user