From abdceedaf6608601f5f350689573e96a46f62eb7 Mon Sep 17 00:00:00 2001 From: Kyle Tse Date: Thu, 12 Feb 2026 22:12:15 +0000 Subject: [PATCH] fix: respect session model override in agent runtime (#14783) (#14983) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: ec47d1a7bf4e97a5db77281567318c1565d319b5 Co-authored-by: shtse8 <8020099+shtse8@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + ...model-selection.override-respected.test.ts | 132 ++++++++++++++++ ....uses-last-non-empty-agent-text-as.test.ts | 149 +++++++++++++++++- src/cron/isolated-agent/run.ts | 24 +++ src/cron/isolated-agent/session.test.ts | 73 +++++++++ src/cron/isolated-agent/session.ts | 2 + 6 files changed, 380 insertions(+), 1 deletion(-) create mode 100644 src/auto-reply/reply/model-selection.override-respected.test.ts create mode 100644 src/cron/isolated-agent/session.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ae99434e1d..c8c984ab53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai - Cron: isolate scheduler errors so one bad job does not break all jobs. (#14385) Thanks @MarvinDontPanic. - Cron: prevent one-shot `at` jobs from re-firing on restart after skipped/errored runs. (#13878) Thanks @lailoo. - Heartbeat: prevent scheduler stalls on unexpected run errors and avoid immediate rerun loops after `requests-in-flight` skips. (#14901) Thanks @joeykrug. +- Cron: honor stored session model overrides for isolated-agent runs while preserving `hooks.gmail.model` precedence for Gmail hook sessions. (#14983) Thanks @shtse8. - Logging/Browser: fall back to `os.tmpdir()/openclaw` for default log, browser trace, and browser download temp paths when `/tmp/openclaw` is unavailable. - WhatsApp: convert Markdown bold/strikethrough to WhatsApp formatting. (#14285) Thanks @Raikan10. - WhatsApp: allow media-only sends and normalize leading blank payloads. (#14408) Thanks @karimnaguib. diff --git a/src/auto-reply/reply/model-selection.override-respected.test.ts b/src/auto-reply/reply/model-selection.override-respected.test.ts new file mode 100644 index 0000000000..b3457fc559 --- /dev/null +++ b/src/auto-reply/reply/model-selection.override-respected.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { createModelSelectionState } from "./model-selection.js"; + +vi.mock("../../agents/model-catalog.js", () => ({ + loadModelCatalog: vi.fn(async () => [ + { provider: "inferencer", id: "deepseek-v3-4bit-mlx", name: "DeepSeek V3" }, + { provider: "kimi-coding", id: "k2p5", name: "Kimi K2.5" }, + { provider: "anthropic", id: "claude-opus-4-5", name: "Claude Opus 4.5" }, + ]), +})); + +const defaultProvider = "inferencer"; +const defaultModel = "deepseek-v3-4bit-mlx"; + +const makeEntry = (overrides: Record = {}) => ({ + sessionId: "session-id", + updatedAt: Date.now(), + ...overrides, +}); + +describe("createModelSelectionState respects session model override", () => { + it("applies session modelOverride when set", async () => { + const cfg = {} as OpenClawConfig; + const sessionKey = "agent:main:main"; + const sessionEntry = makeEntry({ + providerOverride: "kimi-coding", + modelOverride: "k2p5", + }); + const sessionStore = { [sessionKey]: sessionEntry }; + + const state = await createModelSelectionState({ + cfg, + agentCfg: undefined, + sessionEntry, + sessionStore, + sessionKey, + defaultProvider, + defaultModel, + provider: defaultProvider, + model: defaultModel, + hasModelDirective: false, + }); + + expect(state.provider).toBe("kimi-coding"); + expect(state.model).toBe("k2p5"); + }); + + it("falls back to default when no modelOverride is set", async () => { + const cfg = {} as OpenClawConfig; + const sessionKey = "agent:main:main"; + const sessionEntry = makeEntry(); + const sessionStore = { [sessionKey]: sessionEntry }; + + const state = await createModelSelectionState({ + cfg, + agentCfg: undefined, + sessionEntry, + sessionStore, + sessionKey, + defaultProvider, + defaultModel, + provider: defaultProvider, + model: defaultModel, + hasModelDirective: false, + }); + + expect(state.provider).toBe(defaultProvider); + expect(state.model).toBe(defaultModel); + }); + + it("respects modelOverride even when session model field differs", async () => { + // This tests the scenario from issue #14783: user switches model via /model, + // the override is stored, but session.model still reflects the last-used + // fallback model. The override should take precedence. + const cfg = {} as OpenClawConfig; + const sessionKey = "agent:main:main"; + const sessionEntry = makeEntry({ + // Last-used model (from fallback) - should NOT be used for selection + model: "k2p5", + modelProvider: "kimi-coding", + contextTokens: 262_000, + // User's explicit override - SHOULD be used + providerOverride: "anthropic", + modelOverride: "claude-opus-4-5", + }); + const sessionStore = { [sessionKey]: sessionEntry }; + + const state = await createModelSelectionState({ + cfg, + agentCfg: undefined, + sessionEntry, + sessionStore, + sessionKey, + defaultProvider, + defaultModel, + provider: defaultProvider, + model: defaultModel, + hasModelDirective: false, + }); + + // Should use the override, not the last-used model + expect(state.provider).toBe("anthropic"); + expect(state.model).toBe("claude-opus-4-5"); + }); + + it("uses default provider when providerOverride is not set but modelOverride is", async () => { + const cfg = {} as OpenClawConfig; + const sessionKey = "agent:main:main"; + const sessionEntry = makeEntry({ + modelOverride: "deepseek-v3-4bit-mlx", + // no providerOverride + }); + const sessionStore = { [sessionKey]: sessionEntry }; + + const state = await createModelSelectionState({ + cfg, + agentCfg: undefined, + sessionEntry, + sessionStore, + sessionKey, + defaultProvider, + defaultModel, + provider: defaultProvider, + model: defaultModel, + hasModelDirective: false, + }); + + expect(state.provider).toBe(defaultProvider); + expect(state.model).toBe("deepseek-v3-4bit-mlx"); + }); +}); diff --git a/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts b/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts index 3ec1c935b0..09b3f0361f 100644 --- a/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts +++ b/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts @@ -23,7 +23,10 @@ async function withTempHome(fn: (home: string) => Promise): Promise { return withTempHomeBase(fn, { prefix: "openclaw-cron-" }); } -async function writeSessionStore(home: string) { +async function writeSessionStore( + home: string, + entries: Record> = {}, +) { const dir = path.join(home, ".openclaw", "sessions"); await fs.mkdir(dir, { recursive: true }); const storePath = path.join(dir, "sessions.json"); @@ -37,6 +40,7 @@ async function writeSessionStore(home: string) { lastProvider: "webchat", lastTo: "", }, + ...entries, }, null, 2, @@ -294,6 +298,99 @@ describe("runCronIsolatedAgentTurn", () => { }); }); + it("uses stored session override when no job model override is provided", async () => { + await withTempHome(async (home) => { + const storePath = await writeSessionStore(home, { + "agent:main:cron:job-1": { + sessionId: "existing-cron-session", + updatedAt: Date.now(), + providerOverride: "openai", + modelOverride: "gpt-4.1-mini", + }, + }); + const deps: CliDeps = { + sendMessageWhatsApp: vi.fn(), + sendMessageTelegram: vi.fn(), + sendMessageDiscord: vi.fn(), + sendMessageSignal: vi.fn(), + sendMessageIMessage: vi.fn(), + }; + vi.mocked(runEmbeddedPiAgent).mockResolvedValue({ + payloads: [{ text: "ok" }], + meta: { + durationMs: 5, + agentMeta: { sessionId: "s", provider: "p", model: "m" }, + }, + }); + + const res = await runCronIsolatedAgentTurn({ + cfg: makeCfg(home, storePath), + deps, + job: makeJob({ kind: "agentTurn", message: "do it", deliver: false }), + message: "do it", + sessionKey: "cron:job-1", + lane: "cron", + }); + + expect(res.status).toBe("ok"); + const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0] as { + provider?: string; + model?: string; + }; + expect(call?.provider).toBe("openai"); + expect(call?.model).toBe("gpt-4.1-mini"); + }); + }); + + it("prefers job model override over stored session override", async () => { + await withTempHome(async (home) => { + const storePath = await writeSessionStore(home, { + "agent:main:cron:job-1": { + sessionId: "existing-cron-session", + updatedAt: Date.now(), + providerOverride: "openai", + modelOverride: "gpt-4.1-mini", + }, + }); + const deps: CliDeps = { + sendMessageWhatsApp: vi.fn(), + sendMessageTelegram: vi.fn(), + sendMessageDiscord: vi.fn(), + sendMessageSignal: vi.fn(), + sendMessageIMessage: vi.fn(), + }; + vi.mocked(runEmbeddedPiAgent).mockResolvedValue({ + payloads: [{ text: "ok" }], + meta: { + durationMs: 5, + agentMeta: { sessionId: "s", provider: "p", model: "m" }, + }, + }); + + const res = await runCronIsolatedAgentTurn({ + cfg: makeCfg(home, storePath), + deps, + job: makeJob({ + kind: "agentTurn", + message: "do it", + model: "anthropic/claude-opus-4-5", + deliver: false, + }), + message: "do it", + sessionKey: "cron:job-1", + lane: "cron", + }); + + expect(res.status).toBe("ok"); + const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0] as { + provider?: string; + model?: string; + }; + expect(call?.provider).toBe("anthropic"); + expect(call?.model).toBe("claude-opus-4-5"); + }); + }); + it("uses hooks.gmail.model for Gmail hook sessions", async () => { await withTempHome(async (home) => { const storePath = await writeSessionStore(home); @@ -337,6 +434,56 @@ describe("runCronIsolatedAgentTurn", () => { }); }); + it("keeps hooks.gmail.model precedence over stored session override", async () => { + await withTempHome(async (home) => { + const storePath = await writeSessionStore(home, { + "agent:main:hook:gmail:msg-1": { + sessionId: "existing-gmail-session", + updatedAt: Date.now(), + providerOverride: "anthropic", + modelOverride: "claude-opus-4-5", + }, + }); + const deps: CliDeps = { + sendMessageWhatsApp: vi.fn(), + sendMessageTelegram: vi.fn(), + sendMessageDiscord: vi.fn(), + sendMessageSignal: vi.fn(), + sendMessageIMessage: vi.fn(), + }; + vi.mocked(runEmbeddedPiAgent).mockResolvedValue({ + payloads: [{ text: "ok" }], + meta: { + durationMs: 5, + agentMeta: { sessionId: "s", provider: "p", model: "m" }, + }, + }); + + const res = await runCronIsolatedAgentTurn({ + cfg: makeCfg(home, storePath, { + hooks: { + gmail: { + model: "openrouter/meta-llama/llama-3.3-70b:free", + }, + }, + }), + deps, + job: makeJob({ kind: "agentTurn", message: "do it", deliver: false }), + message: "do it", + sessionKey: "hook:gmail:msg-1", + lane: "cron", + }); + + expect(res.status).toBe("ok"); + const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0] as { + provider?: string; + model?: string; + }; + expect(call?.provider).toBe("openrouter"); + expect(call?.model).toBe("meta-llama/llama-3.3-70b:free"); + }); + }); + it("wraps external hook content by default", async () => { await withTempHome(async (home) => { const storePath = await writeSessionStore(home); diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index b52e9594aa..015ee6d511 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -173,6 +173,7 @@ export async function runCronIsolatedAgentTurn(params: { }; // Resolve model - prefer hooks.gmail.model for Gmail hooks. const isGmailHook = baseSessionKey.startsWith("hook:gmail:"); + let hooksGmailModelApplied = false; const hooksGmailModelRef = isGmailHook ? resolveHooksGmailModel({ cfg: params.cfg, @@ -190,6 +191,7 @@ export async function runCronIsolatedAgentTurn(params: { if (status.allowed) { provider = hooksGmailModelRef.provider; model = hooksGmailModelRef.model; + hooksGmailModelApplied = true; } } const modelOverrideRaw = @@ -247,6 +249,28 @@ export async function runCronIsolatedAgentTurn(params: { cronSession.sessionEntry.label = `Cron: ${labelSuffix}`; } + // Respect session model override — check session.modelOverride before falling + // back to the default config model. This ensures /model changes are honoured + // by cron and isolated agent runs. + if (!modelOverride && !hooksGmailModelApplied) { + const sessionModelOverride = cronSession.sessionEntry.modelOverride?.trim(); + if (sessionModelOverride) { + const sessionProviderOverride = + cronSession.sessionEntry.providerOverride?.trim() || resolvedDefault.provider; + const resolvedSessionOverride = resolveAllowedModelRef({ + cfg: cfgWithAgentDefaults, + catalog: await loadCatalog(), + raw: `${sessionProviderOverride}/${sessionModelOverride}`, + defaultProvider: resolvedDefault.provider, + defaultModel: resolvedDefault.model, + }); + if (!("error" in resolvedSessionOverride)) { + provider = resolvedSessionOverride.ref.provider; + model = resolvedSessionOverride.ref.model; + } + } + } + // Resolve thinking level - job thinking > hooks.gmail.thinking > agent default const hooksGmailThinking = isGmailHook ? normalizeThinkLevel(params.cfg.hooks?.gmail?.thinking) diff --git a/src/cron/isolated-agent/session.test.ts b/src/cron/isolated-agent/session.test.ts new file mode 100644 index 0000000000..e9089dafb6 --- /dev/null +++ b/src/cron/isolated-agent/session.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; + +vi.mock("../../config/sessions.js", () => ({ + loadSessionStore: vi.fn(), + resolveStorePath: vi.fn().mockReturnValue("/tmp/test-store.json"), +})); + +import { loadSessionStore } from "../../config/sessions.js"; +import { resolveCronSession } from "./session.js"; + +describe("resolveCronSession", () => { + it("preserves modelOverride and providerOverride from existing session entry", () => { + vi.mocked(loadSessionStore).mockReturnValue({ + "agent:main:cron:test-job": { + sessionId: "old-session-id", + updatedAt: 1000, + modelOverride: "deepseek-v3-4bit-mlx", + providerOverride: "inferencer", + thinkingLevel: "high", + model: "k2p5", + }, + }); + + const result = resolveCronSession({ + cfg: {} as OpenClawConfig, + sessionKey: "agent:main:cron:test-job", + agentId: "main", + nowMs: Date.now(), + }); + + expect(result.sessionEntry.modelOverride).toBe("deepseek-v3-4bit-mlx"); + expect(result.sessionEntry.providerOverride).toBe("inferencer"); + expect(result.sessionEntry.thinkingLevel).toBe("high"); + // The model field (last-used model) should also be preserved + expect(result.sessionEntry.model).toBe("k2p5"); + }); + + it("handles missing modelOverride gracefully", () => { + vi.mocked(loadSessionStore).mockReturnValue({ + "agent:main:cron:test-job": { + sessionId: "old-session-id", + updatedAt: 1000, + model: "claude-opus-4-5", + }, + }); + + const result = resolveCronSession({ + cfg: {} as OpenClawConfig, + sessionKey: "agent:main:cron:test-job", + agentId: "main", + nowMs: Date.now(), + }); + + expect(result.sessionEntry.modelOverride).toBeUndefined(); + expect(result.sessionEntry.providerOverride).toBeUndefined(); + }); + + it("handles no existing session entry", () => { + vi.mocked(loadSessionStore).mockReturnValue({}); + + const result = resolveCronSession({ + cfg: {} as OpenClawConfig, + sessionKey: "agent:main:cron:new-job", + agentId: "main", + nowMs: Date.now(), + }); + + expect(result.sessionEntry.modelOverride).toBeUndefined(); + expect(result.sessionEntry.providerOverride).toBeUndefined(); + expect(result.sessionEntry.model).toBeUndefined(); + }); +}); diff --git a/src/cron/isolated-agent/session.ts b/src/cron/isolated-agent/session.ts index c31a35465c..cd66a442f7 100644 --- a/src/cron/isolated-agent/session.ts +++ b/src/cron/isolated-agent/session.ts @@ -23,6 +23,8 @@ export function resolveCronSession(params: { thinkingLevel: entry?.thinkingLevel, verboseLevel: entry?.verboseLevel, model: entry?.model, + modelOverride: entry?.modelOverride, + providerOverride: entry?.providerOverride, contextTokens: entry?.contextTokens, sendPolicy: entry?.sendPolicy, lastChannel: entry?.lastChannel,