fix(cron): restore isolated delivery defaults

This commit is contained in:
Peter Steinberger
2026-03-08 00:18:31 +00:00
parent 8a469a12b2
commit 4e07bdbdfd
7 changed files with 342 additions and 11 deletions

View File

@@ -288,6 +288,9 @@ export async function runServiceRestart(params: {
cfg,
env: process.env,
modeOverride: "local",
// Drift checks should compare the persisted gateway token against the
// service token, not let an exported shell env mask config drift.
localTokenPrecedence: "config-first",
}).token;
const driftIssue = checkTokenDrift({ serviceToken, configToken });
if (driftIssue) {

View File

@@ -0,0 +1,85 @@
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import {
clearFastTestEnv,
loadRunCronIsolatedAgentTurn,
resetRunCronIsolatedAgentTurnHarness,
resolveCronDeliveryPlanMock,
resolveDeliveryTargetMock,
restoreFastTestEnv,
runEmbeddedPiAgentMock,
runWithModelFallbackMock,
} from "./run.test-harness.js";
const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn();
function makeParams() {
return {
cfg: {},
deps: {} as never,
job: {
id: "message-tool-policy",
name: "Message Tool Policy",
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
payload: { kind: "agentTurn", message: "send a message" },
delivery: { mode: "none" },
} as never,
message: "send a message",
sessionKey: "cron:message-tool-policy",
};
}
describe("runCronIsolatedAgentTurn message tool policy", () => {
let previousFastTestEnv: string | undefined;
const mockFallbackPassthrough = () => {
runWithModelFallbackMock.mockImplementation(async ({ provider, model, run }) => {
const result = await run(provider, model);
return { result, provider, model, attempts: [] };
});
};
beforeEach(() => {
previousFastTestEnv = clearFastTestEnv();
resetRunCronIsolatedAgentTurnHarness();
resolveDeliveryTargetMock.mockResolvedValue({
ok: true,
channel: "telegram",
to: "123",
accountId: undefined,
error: undefined,
});
});
afterEach(() => {
restoreFastTestEnv(previousFastTestEnv);
});
it('keeps the message tool enabled when delivery.mode is "none"', async () => {
mockFallbackPassthrough();
resolveCronDeliveryPlanMock.mockReturnValue({
requested: false,
mode: "none",
});
await runCronIsolatedAgentTurn(makeParams());
expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1);
expect(runEmbeddedPiAgentMock.mock.calls[0]?.[0]?.disableMessageTool).toBe(false);
});
it("disables the message tool when cron delivery is active", async () => {
mockFallbackPassthrough();
resolveCronDeliveryPlanMock.mockReturnValue({
requested: true,
mode: "announce",
channel: "telegram",
to: "123",
});
await runCronIsolatedAgentTurn(makeParams());
expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1);
expect(runEmbeddedPiAgentMock.mock.calls[0]?.[0]?.disableMessageTool).toBe(true);
});
});

View File

@@ -0,0 +1,155 @@
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import {
clearFastTestEnv,
loadRunCronIsolatedAgentTurn,
resolveAgentConfigMock,
resetRunCronIsolatedAgentTurnHarness,
restoreFastTestEnv,
runWithModelFallbackMock,
} from "./run.test-harness.js";
const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn();
const { resolveSandboxConfigForAgent } = await import("../../agents/sandbox/config.js");
function makeJob(overrides?: Record<string, unknown>) {
return {
id: "sandbox-test-job",
name: "Sandbox Test",
schedule: { kind: "cron", expr: "0 9 * * *", tz: "UTC" },
sessionTarget: "isolated",
payload: { kind: "agentTurn", message: "test" },
...overrides,
} as never;
}
function makeParams(overrides?: Record<string, unknown>) {
return {
cfg: {
agents: {
defaults: {
sandbox: {
mode: "all" as const,
workspaceAccess: "rw" as const,
docker: {
network: "none",
dangerouslyAllowContainerNamespaceJoin: true,
dangerouslyAllowExternalBindSources: true,
},
browser: {
enabled: true,
autoStart: false,
},
prune: {
maxAgeDays: 7,
},
},
},
},
},
deps: {} as never,
job: makeJob(),
message: "test",
sessionKey: "cron:sandbox-test",
...overrides,
};
}
describe("runCronIsolatedAgentTurn sandbox config preserved", () => {
let previousFastTestEnv: string | undefined;
beforeEach(() => {
previousFastTestEnv = clearFastTestEnv();
resetRunCronIsolatedAgentTurnHarness();
});
afterEach(() => {
restoreFastTestEnv(previousFastTestEnv);
});
it("preserves default sandbox config when agent entry omits sandbox", async () => {
resolveAgentConfigMock.mockReturnValue({
name: "worker",
workspace: "/tmp/custom-workspace",
sandbox: undefined,
heartbeat: undefined,
tools: undefined,
});
await runCronIsolatedAgentTurn(makeParams({ agentId: "worker" }));
expect(runWithModelFallbackMock).toHaveBeenCalledTimes(1);
const runCfg = runWithModelFallbackMock.mock.calls[0]?.[0]?.cfg;
expect(runCfg?.agents?.defaults?.sandbox).toEqual({
mode: "all",
workspaceAccess: "rw",
docker: {
network: "none",
dangerouslyAllowContainerNamespaceJoin: true,
dangerouslyAllowExternalBindSources: true,
},
browser: {
enabled: true,
autoStart: false,
},
prune: {
maxAgeDays: 7,
},
});
});
it("keeps global sandbox defaults when agent override is partial", async () => {
resolveAgentConfigMock.mockReturnValue({
sandbox: {
docker: {
image: "ghcr.io/openclaw/sandbox:custom",
},
browser: {
image: "ghcr.io/openclaw/browser:custom",
},
prune: {
idleHours: 1,
},
},
});
await runCronIsolatedAgentTurn(makeParams({ agentId: "specialist" }));
expect(runWithModelFallbackMock).toHaveBeenCalledTimes(1);
const runCfg = runWithModelFallbackMock.mock.calls[0]?.[0]?.cfg;
const resolvedSandbox = resolveSandboxConfigForAgent(runCfg, "specialist");
expect(runCfg?.agents?.defaults?.sandbox).toEqual({
mode: "all",
workspaceAccess: "rw",
docker: {
network: "none",
dangerouslyAllowContainerNamespaceJoin: true,
dangerouslyAllowExternalBindSources: true,
},
browser: {
enabled: true,
autoStart: false,
},
prune: {
maxAgeDays: 7,
},
});
expect(resolvedSandbox.mode).toBe("all");
expect(resolvedSandbox.workspaceAccess).toBe("rw");
expect(resolvedSandbox.docker).toMatchObject({
image: "ghcr.io/openclaw/sandbox:custom",
network: "none",
dangerouslyAllowContainerNamespaceJoin: true,
dangerouslyAllowExternalBindSources: true,
});
expect(resolvedSandbox.browser).toMatchObject({
enabled: true,
image: "ghcr.io/openclaw/browser:custom",
autoStart: false,
});
expect(resolvedSandbox.prune).toMatchObject({
idleHours: 1,
maxAgeDays: 7,
});
});
});

View File

@@ -43,6 +43,8 @@ export const logWarnMock = createMock();
export const countActiveDescendantRunsMock = createMock();
export const listDescendantRunsForRequesterMock = createMock();
export const pickLastNonEmptyTextFromPayloadsMock = createMock();
export const resolveCronDeliveryPlanMock = createMock();
export const resolveDeliveryTargetMock = createMock();
vi.mock("../../agents/agent-scope.js", () => ({
resolveAgentConfig: resolveAgentConfigMock,
@@ -177,16 +179,11 @@ vi.mock("../../security/external-content.js", () => ({
}));
vi.mock("../delivery.js", () => ({
resolveCronDeliveryPlan: vi.fn().mockReturnValue({ requested: false }),
resolveCronDeliveryPlan: resolveCronDeliveryPlanMock,
}));
vi.mock("./delivery-target.js", () => ({
resolveDeliveryTarget: vi.fn().mockResolvedValue({
channel: "discord",
to: undefined,
accountId: undefined,
error: undefined,
}),
resolveDeliveryTarget: resolveDeliveryTargetMock,
}));
vi.mock("./helpers.js", () => ({
@@ -286,6 +283,15 @@ export function resetRunCronIsolatedAgentTurnHarness(): void {
listDescendantRunsForRequesterMock.mockReturnValue([]);
pickLastNonEmptyTextFromPayloadsMock.mockReset();
pickLastNonEmptyTextFromPayloadsMock.mockReturnValue("test output");
resolveCronDeliveryPlanMock.mockReset();
resolveCronDeliveryPlanMock.mockReturnValue({ requested: false, mode: "none" });
resolveDeliveryTargetMock.mockReset();
resolveDeliveryTargetMock.mockResolvedValue({
channel: "discord",
to: undefined,
accountId: undefined,
error: undefined,
});
logWarnMock.mockReset();
}

View File

@@ -125,15 +125,26 @@ export async function runCronIsolatedAgentTurn(params: {
const agentConfigOverride = normalizedRequested
? resolveAgentConfig(params.cfg, normalizedRequested)
: undefined;
const { model: overrideModel, ...agentOverrideRest } = agentConfigOverride ?? {};
const {
model: overrideModel,
sandbox: _agentSandboxOverride,
...agentOverrideRest
} = agentConfigOverride ?? {};
// Use the requested agentId even when there is no explicit agent config entry.
// This ensures auth-profiles, workspace, and agentDir all resolve to the
// correct per-agent paths (e.g. ~/.openclaw/agents/<agentId>/agent/).
const agentId = normalizedRequested ?? defaultAgentId;
// Keep sandbox overrides out of `agents.defaults` here. Sandbox resolution
// already merges global defaults with per-agent overrides using `agentId`;
// copying the agent sandbox into defaults clobbers global defaults and can
// double-apply nested agent overrides during isolated cron runs.
const definedOverrides = Object.fromEntries(
Object.entries(agentOverrideRest).filter(([, value]) => value !== undefined),
);
const agentCfg: AgentDefaultsConfig = Object.assign(
{},
params.cfg.agents?.defaults,
agentOverrideRest as Partial<AgentDefaultsConfig>,
definedOverrides as Partial<AgentDefaultsConfig>,
);
// Merge agent model override with defaults instead of replacing, so that
// `fallbacks` from `agents.defaults.model` are preserved when the agent
@@ -533,7 +544,7 @@ export async function runCronIsolatedAgentTurn(params: {
// was successfully resolved. When resolution fails the agent should not
// be blocked by a target it cannot satisfy (#27898).
requireExplicitMessageTarget: deliveryRequested && resolvedDelivery.ok,
disableMessageTool: deliveryRequested || deliveryPlan.mode === "none",
disableMessageTool: deliveryRequested,
allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe,
abortSignal,
bootstrapPromptWarningSignaturesSeen,

View File

@@ -558,3 +558,60 @@ describe("cron stagger defaults", () => {
}
});
});
describe("createJob delivery defaults", () => {
const now = Date.parse("2026-02-28T12:00:00.000Z");
it('defaults delivery to { mode: "announce" } for isolated agentTurn jobs without explicit delivery', () => {
const state = createMockState(now);
const job = createJob(state, {
name: "isolated-no-delivery",
enabled: true,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "now",
payload: { kind: "agentTurn", message: "hello" },
});
expect(job.delivery).toEqual({ mode: "announce" });
});
it("preserves explicit delivery for isolated agentTurn jobs", () => {
const state = createMockState(now);
const job = createJob(state, {
name: "isolated-explicit-delivery",
enabled: true,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "now",
payload: { kind: "agentTurn", message: "hello" },
delivery: { mode: "none" },
});
expect(job.delivery).toEqual({ mode: "none" });
});
it("preserves legacy payload deliver=false when explicit delivery is omitted", () => {
const state = createMockState(now);
const job = createJob(state, {
name: "isolated-legacy-no-deliver",
enabled: true,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "now",
payload: { kind: "agentTurn", message: "hello", deliver: false } as never,
});
expect(job.delivery).toEqual({ mode: "none" });
});
it("does not set delivery for main systemEvent jobs without explicit delivery", () => {
const state = createMockState(now, { defaultAgentId: "main" });
const job = createJob(state, {
name: "main-no-delivery",
enabled: true,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "main",
wakeMode: "now",
payload: { kind: "systemEvent", text: "ping" },
});
expect(job.delivery).toBeUndefined();
});
});

View File

@@ -1,5 +1,6 @@
import crypto from "node:crypto";
import { normalizeAgentId } from "../../routing/session-key.js";
import { buildDeliveryFromLegacyPayload, hasLegacyDeliveryHints } from "../legacy-delivery.js";
import { parseAbsoluteTimeMs } from "../parse.js";
import {
coerceFiniteScheduleNumber,
@@ -530,6 +531,14 @@ export function createJob(state: CronServiceState, input: CronJobCreate): CronJo
? true
: undefined;
const enabled = typeof input.enabled === "boolean" ? input.enabled : true;
const payloadRecord =
input.payload && typeof input.payload === "object"
? (input.payload as Record<string, unknown>)
: undefined;
const legacyDelivery =
payloadRecord && hasLegacyDeliveryHints(payloadRecord)
? (buildDeliveryFromLegacyPayload(payloadRecord) as CronDelivery)
: undefined;
const job: CronJob = {
id,
agentId: normalizeOptionalAgentId(input.agentId),
@@ -544,7 +553,12 @@ export function createJob(state: CronServiceState, input: CronJobCreate): CronJo
sessionTarget: input.sessionTarget,
wakeMode: input.wakeMode,
payload: input.payload,
delivery: input.delivery,
delivery:
input.delivery ??
legacyDelivery ??
(input.sessionTarget === "isolated" && input.payload.kind === "agentTurn"
? { mode: "announce" }
: undefined),
failureAlert: input.failureAlert,
state: {
...input.state,