mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-18 07:47:27 +00:00
fix(cron): respect subagents.model in isolated cron sessions (#11474)
* fix(cron): respect subagents.model in isolated cron sessions * fix(cron): enforce model allowlist for subagents.model * Cron: fix isolated subagent model gate regressions --------- Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -79,6 +79,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Cron/Isolated model defaults: resolve isolated cron `subagents.model` (including object-form `primary`) through allowlist-aware model selection so isolated cron runs honor subagent model defaults unless explicitly overridden by job payload model. (#11474) Thanks @AnonO6.
|
||||
- Cron/Isolated sessions list: persist the intended pre-run model/provider on isolated cron session entries so `sessions_list` reflects payload/session model overrides even when runs fail before post-run telemetry persistence. (#21279) Thanks @altaywtf.
|
||||
- Cron/One-shot reliability: retry transient one-shot failures with bounded backoff and configurable retry policy before disabling. (#24435) Thanks .
|
||||
- Gateway/Cron auditability: add gateway info logs for successful cron create, update, and remove operations. (#25090) Thanks .
|
||||
|
||||
@@ -6,8 +6,9 @@ import {
|
||||
inferUniqueProviderFromConfiguredModels,
|
||||
parseModelRef,
|
||||
buildModelAliasIndex,
|
||||
modelKey,
|
||||
normalizeModelSelection,
|
||||
normalizeProviderId,
|
||||
modelKey,
|
||||
resolveAllowedModelRef,
|
||||
resolveConfiguredModelRef,
|
||||
resolveModelRefFromString,
|
||||
@@ -470,3 +471,31 @@ describe("model-selection", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeModelSelection", () => {
|
||||
it("returns trimmed string for string input", () => {
|
||||
expect(normalizeModelSelection("ollama/llama3.2:3b")).toBe("ollama/llama3.2:3b");
|
||||
});
|
||||
|
||||
it("returns undefined for empty/whitespace string", () => {
|
||||
expect(normalizeModelSelection("")).toBeUndefined();
|
||||
expect(normalizeModelSelection(" ")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("extracts primary from object", () => {
|
||||
expect(normalizeModelSelection({ primary: "google/gemini-2.5-flash" })).toBe(
|
||||
"google/gemini-2.5-flash",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns undefined for object without primary", () => {
|
||||
expect(normalizeModelSelection({ fallbacks: ["a"] })).toBeUndefined();
|
||||
expect(normalizeModelSelection({})).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for null/undefined/number", () => {
|
||||
expect(normalizeModelSelection(undefined)).toBeUndefined();
|
||||
expect(normalizeModelSelection(null)).toBeUndefined();
|
||||
expect(normalizeModelSelection(42)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -208,22 +208,6 @@ export function inferUniqueProviderFromConfiguredModels(params: {
|
||||
return providers.values().next().value;
|
||||
}
|
||||
|
||||
export function normalizeModelSelection(value: unknown): string | undefined {
|
||||
if (typeof value === "string") {
|
||||
const trimmed = value.trim();
|
||||
return trimmed || undefined;
|
||||
}
|
||||
if (!value || typeof value !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
const primary = (value as { primary?: unknown }).primary;
|
||||
if (typeof primary === "string") {
|
||||
const trimmed = primary.trim();
|
||||
return trimmed || undefined;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function resolveAllowlistModelKey(raw: string, defaultProvider: string): string | null {
|
||||
const parsed = parseModelRef(raw, defaultProvider);
|
||||
if (!parsed) {
|
||||
@@ -612,3 +596,23 @@ export function resolveHooksGmailModel(params: {
|
||||
|
||||
return resolved?.ref ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize a model selection value (string or `{primary?: string}`) to a
|
||||
* plain trimmed string. Returns `undefined` when the input is empty/missing.
|
||||
* Shared by sessions-spawn and cron isolated-agent model resolution.
|
||||
*/
|
||||
export function normalizeModelSelection(value: unknown): string | undefined {
|
||||
if (typeof value === "string") {
|
||||
const trimmed = value.trim();
|
||||
return trimmed || undefined;
|
||||
}
|
||||
if (!value || typeof value !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
const primary = (value as { primary?: unknown }).primary;
|
||||
if (typeof primary === "string" && primary.trim()) {
|
||||
return primary.trim();
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
215
src/cron/isolated-agent.subagent-model.test.ts
Normal file
215
src/cron/isolated-agent.subagent-model.test.ts
Normal file
@@ -0,0 +1,215 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js";
|
||||
import type { CliDeps } from "../cli/deps.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import type { CronJob } from "./types.js";
|
||||
|
||||
vi.mock("../agents/pi-embedded.js", () => ({
|
||||
abortEmbeddedPiRun: vi.fn().mockReturnValue(false),
|
||||
runEmbeddedPiAgent: vi.fn(),
|
||||
resolveEmbeddedSessionLane: (key: string) => `session:${key.trim() || "main"}`,
|
||||
}));
|
||||
vi.mock("../agents/model-catalog.js", () => ({
|
||||
loadModelCatalog: vi.fn(),
|
||||
}));
|
||||
|
||||
import { loadModelCatalog } from "../agents/model-catalog.js";
|
||||
import { runEmbeddedPiAgent } from "../agents/pi-embedded.js";
|
||||
import { runCronIsolatedAgentTurn } from "./isolated-agent.js";
|
||||
|
||||
async function withTempHome<T>(fn: (home: string) => Promise<T>): Promise<T> {
|
||||
return withTempHomeBase(fn, { prefix: "openclaw-cron-submodel-" });
|
||||
}
|
||||
|
||||
async function writeSessionStore(home: string) {
|
||||
const dir = path.join(home, ".openclaw", "sessions");
|
||||
await fs.mkdir(dir, { recursive: true });
|
||||
const storePath = path.join(dir, "sessions.json");
|
||||
await fs.writeFile(
|
||||
storePath,
|
||||
JSON.stringify(
|
||||
{
|
||||
"agent:main:main": {
|
||||
sessionId: "main-session",
|
||||
updatedAt: Date.now(),
|
||||
lastProvider: "webchat",
|
||||
lastTo: "",
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
"utf-8",
|
||||
);
|
||||
return storePath;
|
||||
}
|
||||
|
||||
function makeCfg(
|
||||
home: string,
|
||||
storePath: string,
|
||||
overrides: Partial<OpenClawConfig> = {},
|
||||
): OpenClawConfig {
|
||||
const base: OpenClawConfig = {
|
||||
agents: {
|
||||
defaults: {
|
||||
model: "anthropic/claude-sonnet-4-5",
|
||||
workspace: path.join(home, "openclaw"),
|
||||
},
|
||||
},
|
||||
session: { store: storePath, mainKey: "main" },
|
||||
} as OpenClawConfig;
|
||||
return { ...base, ...overrides };
|
||||
}
|
||||
|
||||
function makeDeps(): CliDeps {
|
||||
return {
|
||||
sendMessageWhatsApp: vi.fn(),
|
||||
sendMessageTelegram: vi.fn(),
|
||||
sendMessageDiscord: vi.fn(),
|
||||
sendMessageSlack: vi.fn(),
|
||||
sendMessageSignal: vi.fn(),
|
||||
sendMessageIMessage: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
function makeJob(): CronJob {
|
||||
const now = Date.now();
|
||||
return {
|
||||
id: "job-sub",
|
||||
name: "subagent-model-job",
|
||||
enabled: true,
|
||||
createdAtMs: now,
|
||||
updatedAtMs: now,
|
||||
schedule: { kind: "every", everyMs: 60_000 },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "now",
|
||||
payload: { kind: "agentTurn", message: "do work" },
|
||||
state: {},
|
||||
};
|
||||
}
|
||||
|
||||
function mockEmbeddedAgent() {
|
||||
vi.mocked(runEmbeddedPiAgent).mockResolvedValue({
|
||||
payloads: [{ text: "ok" }],
|
||||
meta: {
|
||||
durationMs: 5,
|
||||
agentMeta: { sessionId: "s", provider: "p", model: "m" },
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
describe("runCronIsolatedAgentTurn: subagent model resolution (#11461)", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(runEmbeddedPiAgent).mockReset();
|
||||
vi.mocked(loadModelCatalog).mockResolvedValue([]);
|
||||
});
|
||||
|
||||
it("uses agents.defaults.subagents.model when set", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const storePath = await writeSessionStore(home);
|
||||
mockEmbeddedAgent();
|
||||
|
||||
await runCronIsolatedAgentTurn({
|
||||
cfg: makeCfg(home, storePath, {
|
||||
agents: {
|
||||
defaults: {
|
||||
model: "anthropic/claude-sonnet-4-5",
|
||||
workspace: path.join(home, "openclaw"),
|
||||
subagents: { model: "ollama/llama3.2:3b" },
|
||||
},
|
||||
},
|
||||
}),
|
||||
deps: makeDeps(),
|
||||
job: makeJob(),
|
||||
message: "do work",
|
||||
sessionKey: "cron:job-sub",
|
||||
lane: "cron",
|
||||
});
|
||||
|
||||
const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0];
|
||||
expect(call?.provider).toBe("ollama");
|
||||
expect(call?.model).toBe("llama3.2:3b");
|
||||
});
|
||||
});
|
||||
|
||||
it("explicit job model override takes precedence over subagents.model", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const storePath = await writeSessionStore(home);
|
||||
mockEmbeddedAgent();
|
||||
|
||||
const job = makeJob();
|
||||
job.payload = { kind: "agentTurn", message: "do work", model: "openai/gpt-4o" };
|
||||
|
||||
await runCronIsolatedAgentTurn({
|
||||
cfg: makeCfg(home, storePath, {
|
||||
agents: {
|
||||
defaults: {
|
||||
model: "anthropic/claude-sonnet-4-5",
|
||||
workspace: path.join(home, "openclaw"),
|
||||
subagents: { model: "ollama/llama3.2:3b" },
|
||||
},
|
||||
},
|
||||
}),
|
||||
deps: makeDeps(),
|
||||
job,
|
||||
message: "do work",
|
||||
sessionKey: "cron:job-sub",
|
||||
lane: "cron",
|
||||
});
|
||||
|
||||
const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0];
|
||||
expect(call?.provider).toBe("openai");
|
||||
expect(call?.model).toBe("gpt-4o");
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to main model when subagents.model is unset", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const storePath = await writeSessionStore(home);
|
||||
mockEmbeddedAgent();
|
||||
|
||||
await runCronIsolatedAgentTurn({
|
||||
cfg: makeCfg(home, storePath),
|
||||
deps: makeDeps(),
|
||||
job: makeJob(),
|
||||
message: "do work",
|
||||
sessionKey: "cron:job-sub",
|
||||
lane: "cron",
|
||||
});
|
||||
|
||||
const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0];
|
||||
expect(call?.provider).toBe("anthropic");
|
||||
expect(call?.model).toBe("claude-sonnet-4-5");
|
||||
});
|
||||
});
|
||||
|
||||
it("supports subagents.model with {primary} object format", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const storePath = await writeSessionStore(home);
|
||||
mockEmbeddedAgent();
|
||||
|
||||
await runCronIsolatedAgentTurn({
|
||||
cfg: makeCfg(home, storePath, {
|
||||
agents: {
|
||||
defaults: {
|
||||
model: "anthropic/claude-sonnet-4-5",
|
||||
workspace: path.join(home, "openclaw"),
|
||||
subagents: { model: { primary: "google/gemini-2.5-flash" } },
|
||||
},
|
||||
},
|
||||
}),
|
||||
deps: makeDeps(),
|
||||
job: makeJob(),
|
||||
message: "do work",
|
||||
sessionKey: "cron:job-sub",
|
||||
lane: "cron",
|
||||
});
|
||||
|
||||
const call = vi.mocked(runEmbeddedPiAgent).mock.calls[0]?.[0];
|
||||
expect(call?.provider).toBe("google");
|
||||
expect(call?.model).toBe("gemini-2.5-flash");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -16,6 +16,7 @@ import { runWithModelFallback } from "../../agents/model-fallback.js";
|
||||
import {
|
||||
getModelRefStatus,
|
||||
isCliProvider,
|
||||
normalizeModelSelection,
|
||||
resolveAllowedModelRef,
|
||||
resolveConfiguredModelRef,
|
||||
resolveHooksGmailModel,
|
||||
@@ -160,6 +161,7 @@ export async function runCronIsolatedAgentTurn(params: {
|
||||
});
|
||||
let provider = resolvedDefault.provider;
|
||||
let model = resolvedDefault.model;
|
||||
|
||||
let catalog: Awaited<ReturnType<typeof loadModelCatalog>> | undefined;
|
||||
const loadCatalog = async () => {
|
||||
if (!catalog) {
|
||||
@@ -167,6 +169,24 @@ export async function runCronIsolatedAgentTurn(params: {
|
||||
}
|
||||
return catalog;
|
||||
};
|
||||
// Isolated cron sessions are subagents — prefer subagents.model when set,
|
||||
// but only if it passes the model allowlist. #11461
|
||||
const subagentModelRaw =
|
||||
normalizeModelSelection(agentConfigOverride?.subagents?.model) ??
|
||||
normalizeModelSelection(params.cfg.agents?.defaults?.subagents?.model);
|
||||
if (subagentModelRaw) {
|
||||
const resolvedSubagent = resolveAllowedModelRef({
|
||||
cfg: cfgWithAgentDefaults,
|
||||
catalog: await loadCatalog(),
|
||||
raw: subagentModelRaw,
|
||||
defaultProvider: resolvedDefault.provider,
|
||||
defaultModel: resolvedDefault.model,
|
||||
});
|
||||
if (!("error" in resolvedSubagent)) {
|
||||
provider = resolvedSubagent.ref.provider;
|
||||
model = resolvedSubagent.ref.model;
|
||||
}
|
||||
}
|
||||
// Resolve model - prefer hooks.gmail.model for Gmail hooks.
|
||||
const isGmailHook = baseSessionKey.startsWith("hook:gmail:");
|
||||
let hooksGmailModelApplied = false;
|
||||
|
||||
Reference in New Issue
Block a user