fix: normalize cron jobId load path (#62251) (thanks @neeravmakwana)

* Cron: normalize jobId to id when loading jobs.json

* Cron: address review — changelog order, warn on legacy jobId
This commit is contained in:
Neerav Makwana
2026-04-07 03:12:59 -04:00
committed by GitHub
parent 3243c9b5b0
commit 242b2e66f2
6 changed files with 146 additions and 11 deletions

View File

@@ -57,6 +57,7 @@ Docs: https://docs.openclaw.ai
- Agents/subagents: honor `sessions_spawn(lightContext: true)` for spawned subagent runs by preserving lightweight bootstrap context through the gateway and embedded runner instead of silently falling back to full workspace bootstrap injection. (#62264) Thanks @theSamPadilla.
- Slack/media: keep attachment downloads on the SSRF-guarded dispatcher path so Slack media fetching works on Node 22 without dropping pinned transport enforcement. (#62239) Thanks @openperf.
- Docker/plugins: stop forcing bundled plugin discovery to `/app/extensions` in runtime images so packaged installs use compiled `dist/extensions` artifacts again and Node 24 containers do not boot through source-only plugin entry paths. Fixes #62044. (#62316) Thanks @gumadeiras.
- Cron: load `jobId` into `id` when the on-disk store omits `id`, matching doctor migration and fixing `unknown cron job id` for hand-edited `jobs.json`. (#62246) Thanks @neeravmakwana.
## 2026.4.5

View File

@@ -1,3 +1,4 @@
import { normalizeCronJobIdentityFields } from "../cron/normalize-job-identity.js";
import { parseAbsoluteTimeMs } from "../cron/parse.js";
import { coerceFiniteScheduleNumber } from "../cron/schedule.js";
import { inferLegacyName, normalizeOptionalText } from "../cron/service/normalize.js";
@@ -212,19 +213,11 @@ export function normalizeStoredCronJobs(
mutated = true;
}
const rawId = typeof raw.id === "string" ? raw.id.trim() : "";
const legacyJobId = typeof raw.jobId === "string" ? raw.jobId.trim() : "";
if (!rawId && legacyJobId) {
raw.id = legacyJobId;
mutated = true;
trackIssue("jobId");
} else if (rawId && raw.id !== rawId) {
raw.id = rawId;
const idNorm = normalizeCronJobIdentityFields(raw);
if (idNorm.mutated) {
mutated = true;
}
if ("jobId" in raw) {
delete raw.jobId;
mutated = true;
if (idNorm.legacyJobIdIssue) {
trackIssue("jobId");
}

View File

@@ -0,0 +1,53 @@
import { describe, expect, it } from "vitest";
import { normalizeCronJobIdentityFields } from "./normalize-job-identity.js";
describe("normalizeCronJobIdentityFields", () => {
it("copies trimmed jobId into id when id is missing", () => {
const raw: Record<string, unknown> = {
jobId: " stable-slug ",
name: "n",
};
const r = normalizeCronJobIdentityFields(raw);
expect(r.mutated).toBe(true);
expect(r.legacyJobIdIssue).toBe(true);
expect(raw.id).toBe("stable-slug");
expect(raw.jobId).toBeUndefined();
});
it("trims id without reporting a legacy jobId issue when jobId is absent", () => {
const raw: Record<string, unknown> = {
id: " trimmed-id ",
name: "n",
};
const r = normalizeCronJobIdentityFields(raw);
expect(r.mutated).toBe(true);
expect(r.legacyJobIdIssue).toBe(false);
expect(raw.id).toBe("trimmed-id");
});
it("removes redundant jobId while keeping canonical id", () => {
const raw: Record<string, unknown> = {
id: "keep-me",
jobId: "keep-me",
name: "n",
};
const r = normalizeCronJobIdentityFields(raw);
expect(r.mutated).toBe(true);
expect(r.legacyJobIdIssue).toBe(true);
expect(raw.id).toBe("keep-me");
expect(raw.jobId).toBeUndefined();
});
it("ignores non-string jobId", () => {
const raw: Record<string, unknown> = {
id: "x",
jobId: 1,
name: "n",
};
const r = normalizeCronJobIdentityFields(raw);
expect(r.mutated).toBe(true);
expect(r.legacyJobIdIssue).toBe(true);
expect(raw.id).toBe("x");
expect(raw.jobId).toBeUndefined();
});
});

View File

@@ -0,0 +1,18 @@
export function normalizeCronJobIdentityFields(raw: Record<string, unknown>): {
mutated: boolean;
legacyJobIdIssue: boolean;
} {
const rawId = typeof raw.id === "string" ? raw.id.trim() : "";
const legacyJobId = typeof raw.jobId === "string" ? raw.jobId.trim() : "";
const hadJobIdKey = "jobId" in raw;
const normalizedId = rawId || legacyJobId;
const idChanged = Boolean(normalizedId && raw.id !== normalizedId);
if (idChanged) {
raw.id = normalizedId;
}
if (hadJobIdKey) {
delete raw.jobId;
}
return { mutated: idChanged || hadJobIdKey, legacyJobIdIssue: hadJobIdKey };
}

View File

@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
import path from "node:path";
import { describe, expect, it, vi } from "vitest";
import { setupCronServiceSuite } from "../service.test-harness.js";
import { findJobOrThrow } from "./jobs.js";
import { createCronServiceState } from "./state.js";
import { ensureLoaded, persist } from "./store.js";
@@ -89,4 +90,63 @@ describe("cron service store seam coverage", () => {
expect(typeof state.storeFileMtimeMs).toBe("number");
expect((state.storeFileMtimeMs ?? 0) >= (firstMtime ?? 0)).toBe(true);
});
it("normalizes jobId-only jobs in memory so scheduler lookups resolve by stable id", async () => {
const { storePath } = await makeStorePath();
const now = Date.parse("2026-03-23T12:00:00.000Z");
await fs.mkdir(path.dirname(storePath), { recursive: true });
await fs.writeFile(
storePath,
JSON.stringify(
{
version: 1,
jobs: [
{
jobId: "repro-stable-id",
name: "handed",
enabled: true,
createdAtMs: now - 60_000,
updatedAtMs: now - 60_000,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "main",
wakeMode: "now",
payload: { kind: "systemEvent", text: "tick" },
state: {},
},
],
},
null,
2,
),
"utf8",
);
const state = createCronServiceState({
storePath,
cronEnabled: true,
log: logger,
nowMs: () => now,
enqueueSystemEvent: vi.fn(),
requestHeartbeatNow: vi.fn(),
runIsolatedAgentJob: vi.fn(async () => ({ status: "ok" as const })),
});
await ensureLoaded(state);
expect(logger.warn).toHaveBeenCalledWith(
expect.objectContaining({ storePath, jobId: "repro-stable-id" }),
expect.stringContaining("legacy jobId"),
);
const job = findJobOrThrow(state, "repro-stable-id");
expect(job.id).toBe("repro-stable-id");
expect((job as { jobId?: unknown }).jobId).toBeUndefined();
const raw = JSON.parse(await fs.readFile(storePath, "utf8")) as {
jobs: Array<Record<string, unknown>>;
};
expect(raw.jobs[0]?.jobId).toBe("repro-stable-id");
expect(raw.jobs[0]?.id).toBeUndefined();
});
});

View File

@@ -1,4 +1,5 @@
import fs from "node:fs";
import { normalizeCronJobIdentityFields } from "../normalize-job-identity.js";
import { loadCronStore, saveCronStore } from "../store.js";
import type { CronJob } from "../types.js";
import { recomputeNextRuns } from "./jobs.js";
@@ -34,6 +35,15 @@ export async function ensureLoaded(
const loaded = await loadCronStore(state.deps.storePath);
const jobs = (loaded.jobs ?? []) as unknown as CronJob[];
for (const job of jobs) {
const raw = job as unknown as Record<string, unknown>;
const { legacyJobIdIssue } = normalizeCronJobIdentityFields(raw);
if (legacyJobIdIssue) {
const resolvedId = typeof raw.id === "string" ? raw.id : undefined;
state.deps.log.warn(
{ storePath: state.deps.storePath, jobId: resolvedId },
"cron: job used legacy jobId field; normalized id in memory (run openclaw doctor --fix to persist canonical shape)",
);
}
// Persisted legacy jobs may predate the required `enabled` field.
// Keep runtime behavior backward-compatible without rewriting the store.
if (typeof job.enabled !== "boolean") {