mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 05:12:43 +00:00
fix: cron model fallback to agent defaults when payload.model fails (#26717)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: 06454bd55b
Co-authored-by: Youyou972 <50808411+Youyou972@users.noreply.github.com>
Co-authored-by: shakkernerd <165377636+shakkernerd@users.noreply.github.com>
Reviewed-by: @shakkernerd
This commit is contained in:
@@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
|
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
|
||||||
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
||||||
- Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
|
- Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
|
||||||
|
- Cron/Model overrides: when isolated `payload.model` is no longer allowlisted, fall back to default model selection instead of failing the job, while still returning explicit errors for invalid model strings. (#26717) Thanks @Youyou972.
|
||||||
- Security/Nextcloud Talk: reject unsigned webhook traffic before full body reads, reducing unauthenticated request-body exposure, with auth-order regression coverage. (#26118) Thanks @bmendonca3.
|
- Security/Nextcloud Talk: reject unsigned webhook traffic before full body reads, reducing unauthenticated request-body exposure, with auth-order regression coverage. (#26118) Thanks @bmendonca3.
|
||||||
- Security/Nextcloud Talk: stop treating DM pairing-store entries as group allowlist senders, so group authorization remains bounded to configured group allowlists. (#26116) Thanks @bmendonca3.
|
- Security/Nextcloud Talk: stop treating DM pairing-store entries as group allowlist senders, so group authorization remains bounded to configured group allowlists. (#26116) Thanks @bmendonca3.
|
||||||
- Security/IRC: keep pairing-store approvals DM-only and out of IRC group allowlist authorization, with policy regression tests for allowlist resolution. (#26112) Thanks @bmendonca3.
|
- Security/IRC: keep pairing-store approvals DM-only and out of IRC group allowlist authorization, with policy regression tests for allowlist resolution. (#26112) Thanks @bmendonca3.
|
||||||
|
|||||||
@@ -6,6 +6,13 @@ import { runWithModelFallback } from "../../agents/model-fallback.js";
|
|||||||
const buildWorkspaceSkillSnapshotMock = vi.fn();
|
const buildWorkspaceSkillSnapshotMock = vi.fn();
|
||||||
const resolveAgentConfigMock = vi.fn();
|
const resolveAgentConfigMock = vi.fn();
|
||||||
const resolveAgentSkillsFilterMock = vi.fn();
|
const resolveAgentSkillsFilterMock = vi.fn();
|
||||||
|
const getModelRefStatusMock = vi.fn().mockReturnValue({ allowed: false });
|
||||||
|
const isCliProviderMock = vi.fn().mockReturnValue(false);
|
||||||
|
const resolveAllowedModelRefMock = vi.fn();
|
||||||
|
const resolveConfiguredModelRefMock = vi.fn();
|
||||||
|
const resolveHooksGmailModelMock = vi.fn();
|
||||||
|
const resolveThinkingDefaultMock = vi.fn();
|
||||||
|
const logWarnMock = vi.fn();
|
||||||
|
|
||||||
vi.mock("../../agents/agent-scope.js", () => ({
|
vi.mock("../../agents/agent-scope.js", () => ({
|
||||||
resolveAgentConfig: resolveAgentConfigMock,
|
resolveAgentConfig: resolveAgentConfigMock,
|
||||||
@@ -36,14 +43,12 @@ vi.mock("../../agents/model-selection.js", async (importOriginal) => {
|
|||||||
const actual = await importOriginal<typeof import("../../agents/model-selection.js")>();
|
const actual = await importOriginal<typeof import("../../agents/model-selection.js")>();
|
||||||
return {
|
return {
|
||||||
...actual,
|
...actual,
|
||||||
getModelRefStatus: vi.fn().mockReturnValue({ allowed: false }),
|
getModelRefStatus: getModelRefStatusMock,
|
||||||
isCliProvider: vi.fn().mockReturnValue(false),
|
isCliProvider: isCliProviderMock,
|
||||||
resolveAllowedModelRef: vi
|
resolveAllowedModelRef: resolveAllowedModelRefMock,
|
||||||
.fn()
|
resolveConfiguredModelRef: resolveConfiguredModelRefMock,
|
||||||
.mockReturnValue({ ref: { provider: "openai", model: "gpt-4" } }),
|
resolveHooksGmailModel: resolveHooksGmailModelMock,
|
||||||
resolveConfiguredModelRef: vi.fn().mockReturnValue({ provider: "openai", model: "gpt-4" }),
|
resolveThinkingDefault: resolveThinkingDefaultMock,
|
||||||
resolveHooksGmailModel: vi.fn().mockReturnValue(null),
|
|
||||||
resolveThinkingDefault: vi.fn().mockReturnValue(undefined),
|
|
||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -138,7 +143,7 @@ vi.mock("../../infra/skills-remote.js", () => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("../../logger.js", () => ({
|
vi.mock("../../logger.js", () => ({
|
||||||
logWarn: vi.fn(),
|
logWarn: (...args: unknown[]) => logWarnMock(...args),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("../../security/external-content.js", () => ({
|
vi.mock("../../security/external-content.js", () => ({
|
||||||
@@ -222,6 +227,13 @@ describe("runCronIsolatedAgentTurn — skill filter", () => {
|
|||||||
});
|
});
|
||||||
resolveAgentConfigMock.mockReturnValue(undefined);
|
resolveAgentConfigMock.mockReturnValue(undefined);
|
||||||
resolveAgentSkillsFilterMock.mockReturnValue(undefined);
|
resolveAgentSkillsFilterMock.mockReturnValue(undefined);
|
||||||
|
resolveConfiguredModelRefMock.mockReturnValue({ provider: "openai", model: "gpt-4" });
|
||||||
|
resolveAllowedModelRefMock.mockReturnValue({ ref: { provider: "openai", model: "gpt-4" } });
|
||||||
|
resolveHooksGmailModelMock.mockReturnValue(null);
|
||||||
|
resolveThinkingDefaultMock.mockReturnValue(undefined);
|
||||||
|
getModelRefStatusMock.mockReturnValue({ allowed: false });
|
||||||
|
isCliProviderMock.mockReturnValue(false);
|
||||||
|
logWarnMock.mockReset();
|
||||||
// Fresh session object per test — prevents mutation leaking between tests
|
// Fresh session object per test — prevents mutation leaking between tests
|
||||||
resolveCronSessionMock.mockReturnValue({
|
resolveCronSessionMock.mockReturnValue({
|
||||||
storePath: "/tmp/store.json",
|
storePath: "/tmp/store.json",
|
||||||
@@ -408,5 +420,78 @@ describe("runCronIsolatedAgentTurn — skill filter", () => {
|
|||||||
it("preserves defaults when agent overrides primary in object form", async () => {
|
it("preserves defaults when agent overrides primary in object form", async () => {
|
||||||
await expectPrimaryOverridePreservesDefaults({ primary: "anthropic/claude-sonnet-4-5" });
|
await expectPrimaryOverridePreservesDefaults({ primary: "anthropic/claude-sonnet-4-5" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("applies payload.model override when model is allowed", async () => {
|
||||||
|
resolveAllowedModelRefMock.mockReturnValueOnce({
|
||||||
|
ref: { provider: "anthropic", model: "claude-sonnet-4-6" },
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await runCronIsolatedAgentTurn(
|
||||||
|
makeParams({
|
||||||
|
job: makeJob({
|
||||||
|
payload: { kind: "agentTurn", message: "test", model: "anthropic/claude-sonnet-4-6" },
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result.status).toBe("ok");
|
||||||
|
expect(logWarnMock).not.toHaveBeenCalled();
|
||||||
|
expect(runWithModelFallbackMock).toHaveBeenCalledOnce();
|
||||||
|
const runParams = runWithModelFallbackMock.mock.calls[0][0];
|
||||||
|
expect(runParams.provider).toBe("anthropic");
|
||||||
|
expect(runParams.model).toBe("claude-sonnet-4-6");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to agent defaults when payload.model is not allowed", async () => {
|
||||||
|
resolveAllowedModelRefMock.mockReturnValueOnce({
|
||||||
|
error: "model not allowed: anthropic/claude-sonnet-4-6",
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await runCronIsolatedAgentTurn(
|
||||||
|
makeParams({
|
||||||
|
cfg: {
|
||||||
|
agents: {
|
||||||
|
defaults: {
|
||||||
|
model: { primary: "openai-codex/gpt-5.3-codex", fallbacks: defaultFallbacks },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
job: makeJob({
|
||||||
|
payload: { kind: "agentTurn", message: "test", model: "anthropic/claude-sonnet-4-6" },
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result.status).toBe("ok");
|
||||||
|
expect(logWarnMock).toHaveBeenCalledWith(
|
||||||
|
"cron: payload.model 'anthropic/claude-sonnet-4-6' not allowed, falling back to agent defaults",
|
||||||
|
);
|
||||||
|
expect(runWithModelFallbackMock).toHaveBeenCalledOnce();
|
||||||
|
const callCfg = runWithModelFallbackMock.mock.calls[0][0].cfg;
|
||||||
|
const model = callCfg?.agents?.defaults?.model as
|
||||||
|
| { primary?: string; fallbacks?: string[] }
|
||||||
|
| undefined;
|
||||||
|
expect(model?.primary).toBe("openai-codex/gpt-5.3-codex");
|
||||||
|
expect(model?.fallbacks).toEqual(defaultFallbacks);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns an error when payload.model is invalid", async () => {
|
||||||
|
resolveAllowedModelRefMock.mockReturnValueOnce({
|
||||||
|
error: "invalid model: openai/",
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await runCronIsolatedAgentTurn(
|
||||||
|
makeParams({
|
||||||
|
job: makeJob({
|
||||||
|
payload: { kind: "agentTurn", message: "test", model: "openai/" },
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result.status).toBe("error");
|
||||||
|
expect(result.error).toBe("invalid model: openai/");
|
||||||
|
expect(logWarnMock).not.toHaveBeenCalled();
|
||||||
|
expect(runWithModelFallbackMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -198,10 +198,17 @@ export async function runCronIsolatedAgentTurn(params: {
|
|||||||
defaultModel: resolvedDefault.model,
|
defaultModel: resolvedDefault.model,
|
||||||
});
|
});
|
||||||
if ("error" in resolvedOverride) {
|
if ("error" in resolvedOverride) {
|
||||||
return { status: "error", error: resolvedOverride.error };
|
if (resolvedOverride.error.startsWith("model not allowed:")) {
|
||||||
|
logWarn(
|
||||||
|
`cron: payload.model '${modelOverride}' not allowed, falling back to agent defaults`,
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
return { status: "error", error: resolvedOverride.error };
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
provider = resolvedOverride.ref.provider;
|
||||||
|
model = resolvedOverride.ref.model;
|
||||||
}
|
}
|
||||||
provider = resolvedOverride.ref.provider;
|
|
||||||
model = resolvedOverride.ref.model;
|
|
||||||
}
|
}
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const cronSession = resolveCronSession({
|
const cronSession = resolveCronSession({
|
||||||
|
|||||||
Reference in New Issue
Block a user