fix(cron): recover flat params when LLM omits job wrapper (#12124)

* fix(cron): recover flat params when LLM omits job wrapper (#11310)

Non-frontier models (e.g. Grok) flatten job properties to the top level
alongside `action` instead of nesting them inside the `job` parameter.
The opaque schema (`Type.Object({}, { additionalProperties: true })`)
gives these models no structural hint, so they put name, schedule,
payload, etc. as siblings of action.

Add a flat-params recovery step in the cron add handler: when
`params.job` is missing or an empty object, scan for recognised job
property names on params and construct a synthetic job object before
passing to `normalizeCronJobCreate`. Recovery requires at least one
meaningful signal field (schedule, payload, message, or text) to avoid
false positives.

Added tests:
- Flat params with no job wrapper → recovered
- Empty job object + flat params → recovered
- Message shorthand at top level → inferred as agentTurn
- No meaningful fields → still throws 'job required'
- Non-empty job takes precedence over flat params

* fix(cron): floor nowMs to second boundary before croner lookback

Cron expressions operate at second granularity. When nowMs falls
mid-second (e.g. 12:00:00.500) and the pattern targets that exact
second (like '0 0 12 * * *'), a 1ms lookback still lands inside the
matching second.  Croner interprets this as 'already past' and skips
to the next occurrence (e.g. the following day).

Fix: floor nowMs to the start of the current second before applying
the 1ms lookback.  This ensures the reference always falls in the
*previous* second, so croner correctly identifies the current match.

Also compare the result against the floored nowSecondMs (not raw nowMs)
so that a match at the start of the current second is not rejected by
the >= guard when nowMs has sub-second offset.

Adds regression tests for 6-field cron patterns with specific seconds.

* fix: add changelog entries for cron fixes (#12124) (thanks @tyler6204)

* test: stabilize warning filter emit assertion (#12124) (thanks @tyler6204)
This commit is contained in:
Tyler Yust
2026-02-08 23:10:09 -08:00
committed by GitHub
parent fb8e4489a3
commit 07375a65d8
6 changed files with 202 additions and 8 deletions

View File

@@ -25,6 +25,8 @@ Docs: https://docs.openclaw.ai
- Gateway: stabilize chat routing by canonicalizing node session keys for node-originated chat methods. (#11755) Thanks @mbelinky.
- Web UI: make chat refresh smoothly scroll to the latest messages and suppress new-messages badge flash during manual refresh.
- Cron: route text-only isolated agent announces through the shared subagent announce flow; add exponential backoff for repeated errors; preserve future `nextRunAtMs` on restart; include current-boundary schedule matches; prevent stale threadId reuse across targets; and add per-job execution timeout. (#11641) Thanks @tyler6204.
- Cron tool: recover flat params when LLM omits the `job` wrapper for add requests. (#11310, #12124) Thanks @tyler6204.
- Cron scheduler: fix `nextRun` skipping the current occurrence when computed mid-second. (#12124) Thanks @tyler6204.
- Subagents: stabilize announce timing, preserve compaction metrics across retries, clamp overflow-prone long timeouts, and cap impossible context usage token totals. (#11551) Thanks @tyler6204.
- Agents: recover from context overflow caused by oversized tool results (pre-emptive capping + fallback truncation). (#11579) Thanks @tyler6204.
- Gateway: no more post-compaction amnesia; injected transcript writes now preserve Pi session `parentId` chain so agents can remember again. Thanks @Takhoffman 🦞.

View File

@@ -321,6 +321,109 @@ describe("cron tool", () => {
});
});
// ── Flat-params recovery (issue #11310) ──────────────────────────────
it("recovers flat params when job is missing", async () => {
callGatewayMock.mockResolvedValueOnce({ ok: true });
const tool = createCronTool();
await tool.execute("call-flat", {
action: "add",
name: "flat-job",
schedule: { kind: "at", at: new Date(123).toISOString() },
sessionTarget: "isolated",
payload: { kind: "agentTurn", message: "do stuff" },
});
expect(callGatewayMock).toHaveBeenCalledTimes(1);
const call = callGatewayMock.mock.calls[0]?.[0] as {
method?: string;
params?: { name?: string; sessionTarget?: string; payload?: { kind?: string } };
};
expect(call.method).toBe("cron.add");
expect(call.params?.name).toBe("flat-job");
expect(call.params?.sessionTarget).toBe("isolated");
expect(call.params?.payload?.kind).toBe("agentTurn");
});
it("recovers flat params when job is empty object", async () => {
callGatewayMock.mockResolvedValueOnce({ ok: true });
const tool = createCronTool();
await tool.execute("call-empty-job", {
action: "add",
job: {},
name: "empty-job",
schedule: { kind: "cron", expr: "0 9 * * *" },
sessionTarget: "main",
payload: { kind: "systemEvent", text: "wake up" },
});
expect(callGatewayMock).toHaveBeenCalledTimes(1);
const call = callGatewayMock.mock.calls[0]?.[0] as {
method?: string;
params?: { name?: string; sessionTarget?: string; payload?: { text?: string } };
};
expect(call.method).toBe("cron.add");
expect(call.params?.name).toBe("empty-job");
expect(call.params?.sessionTarget).toBe("main");
expect(call.params?.payload?.text).toBe("wake up");
});
it("recovers flat message shorthand as agentTurn payload", async () => {
callGatewayMock.mockResolvedValueOnce({ ok: true });
const tool = createCronTool();
await tool.execute("call-msg-shorthand", {
action: "add",
schedule: { kind: "at", at: new Date(456).toISOString() },
message: "do stuff",
});
expect(callGatewayMock).toHaveBeenCalledTimes(1);
const call = callGatewayMock.mock.calls[0]?.[0] as {
method?: string;
params?: { payload?: { kind?: string; message?: string }; sessionTarget?: string };
};
expect(call.method).toBe("cron.add");
// normalizeCronJobCreate infers agentTurn from message and isolated from agentTurn
expect(call.params?.payload?.kind).toBe("agentTurn");
expect(call.params?.payload?.message).toBe("do stuff");
expect(call.params?.sessionTarget).toBe("isolated");
});
it("does not recover flat params when no meaningful job field is present", async () => {
const tool = createCronTool();
await expect(
tool.execute("call-no-signal", {
action: "add",
name: "orphan-name",
enabled: true,
}),
).rejects.toThrow("job required");
});
it("prefers existing non-empty job over flat params", async () => {
callGatewayMock.mockResolvedValueOnce({ ok: true });
const tool = createCronTool();
await tool.execute("call-nested-wins", {
action: "add",
job: {
name: "nested-job",
schedule: { kind: "at", at: new Date(123).toISOString() },
payload: { kind: "systemEvent", text: "from nested" },
},
name: "flat-name-should-be-ignored",
});
const call = callGatewayMock.mock.calls[0]?.[0] as {
params?: { name?: string; payload?: { text?: string } };
};
expect(call?.params?.name).toBe("nested-job");
expect(call?.params?.payload?.text).toBe("from nested");
});
it("does not infer delivery when mode is none", async () => {
callGatewayMock.mockResolvedValueOnce({ ok: true });

View File

@@ -301,6 +301,57 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con
}),
);
case "add": {
// Flat-params recovery: non-frontier models (e.g. Grok) sometimes flatten
// job properties to the top level alongside `action` instead of nesting
// them inside `job`. When `params.job` is missing or empty, reconstruct
// a synthetic job object from any recognised top-level job fields.
// See: https://github.com/openclaw/openclaw/issues/11310
if (
!params.job ||
(typeof params.job === "object" &&
params.job !== null &&
Object.keys(params.job as Record<string, unknown>).length === 0)
) {
const JOB_KEYS: ReadonlySet<string> = new Set([
"name",
"schedule",
"sessionTarget",
"wakeMode",
"payload",
"delivery",
"enabled",
"description",
"deleteAfterRun",
"agentId",
"message",
"text",
"model",
"thinking",
"timeoutSeconds",
"allowUnsafeExternalContent",
]);
const synthetic: Record<string, unknown> = {};
let found = false;
for (const key of Object.keys(params)) {
if (JOB_KEYS.has(key) && params[key] !== undefined) {
synthetic[key] = params[key];
found = true;
}
}
// Only use the synthetic job if at least one meaningful field is present
// (schedule, payload, message, or text are the minimum signals that the
// LLM intended to create a job).
if (
found &&
(synthetic.schedule !== undefined ||
synthetic.payload !== undefined ||
synthetic.message !== undefined ||
synthetic.text !== undefined)
) {
params.job = synthetic;
}
}
if (!params.job || typeof params.job !== "object") {
throw new Error("job required");
}

View File

@@ -33,4 +33,38 @@ describe("cron schedule", () => {
const next = computeNextRunAtMs({ kind: "every", everyMs: 30_000, anchorMs: anchor }, anchor);
expect(next).toBe(anchor + 30_000);
});
describe("cron with specific seconds (6-field pattern)", () => {
// Pattern: fire at exactly second 0 of minute 0 of hour 12 every day
const dailyNoon = { kind: "cron" as const, expr: "0 0 12 * * *", tz: "UTC" };
const noonMs = Date.parse("2026-02-08T12:00:00.000Z");
it("returns current occurrence when nowMs is exactly at the match", () => {
const next = computeNextRunAtMs(dailyNoon, noonMs);
expect(next).toBe(noonMs);
});
it("returns current occurrence when nowMs is mid-second (.500) within the match", () => {
// This is the core regression: without the second-floor fix, a 1ms
// lookback from 12:00:00.499 still lands inside the matching second,
// causing croner to skip to the *next day*.
const next = computeNextRunAtMs(dailyNoon, noonMs + 500);
expect(next).toBe(noonMs);
});
it("returns current occurrence when nowMs is late in the matching second (.999)", () => {
const next = computeNextRunAtMs(dailyNoon, noonMs + 999);
expect(next).toBe(noonMs);
});
it("advances to next day once the matching second is fully past", () => {
const next = computeNextRunAtMs(dailyNoon, noonMs + 1000);
expect(next).toBe(noonMs + 86_400_000); // next day
});
it("returns today when nowMs is before the match", () => {
const next = computeNextRunAtMs(dailyNoon, noonMs - 500);
expect(next).toBe(noonMs);
});
});
});

View File

@@ -49,13 +49,17 @@ export function computeNextRunAtMs(schedule: CronSchedule, nowMs: number): numbe
timezone: resolveCronTimezone(schedule.tz),
catch: false,
});
// Use a tiny lookback (1ms) so croner doesn't skip the current second
// boundary. Without this, a job updated at exactly its cron time would
// be scheduled for the *next* matching time (e.g. 24h later for daily).
const next = cron.nextRun(new Date(nowMs - 1));
// Cron operates at second granularity, so floor nowMs to the start of the
// current second. This prevents the lookback from landing inside a matching
// second — if nowMs is e.g. 12:00:00.500 and the pattern fires at second 0,
// a 1ms lookback (12:00:00.499) is still *within* that second, causing
// croner to skip ahead to the next occurrence (e.g. the following day).
// Flooring first ensures the lookback always falls in the *previous* second.
const nowSecondMs = Math.floor(nowMs / 1000) * 1000;
const next = cron.nextRun(new Date(nowSecondMs - 1));
if (!next) {
return undefined;
}
const nextMs = next.getTime();
return Number.isFinite(nextMs) && nextMs >= nowMs ? nextMs : undefined;
return Number.isFinite(nextMs) && nextMs >= nowSecondMs ? nextMs : undefined;
}

View File

@@ -56,7 +56,7 @@ describe("warning filter", () => {
});
it("installs once and suppresses known warnings at emit time", async () => {
const writeSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true);
const baseEmitSpy = vi.spyOn(process, "emitWarning").mockImplementation(() => undefined);
installProcessWarningFilter();
installProcessWarningFilter();
@@ -74,10 +74,10 @@ describe("warning filter", () => {
code: "DEP0060",
});
await new Promise((resolve) => setImmediate(resolve));
expect(writeSpy).not.toHaveBeenCalled();
expect(baseEmitSpy).not.toHaveBeenCalled();
emitWarning("Visible warning", { type: "Warning", code: "OPENCLAW_TEST_WARNING" });
await new Promise((resolve) => setImmediate(resolve));
expect(writeSpy).toHaveBeenCalled();
expect(baseEmitSpy).toHaveBeenCalledTimes(1);
});
});