fix(cron): finalize removed direct runs (#124457)

* fix(cron): finalize removed direct runs

* refactor(cron): clarify terminal outcome ownership

* docs(qa): record cron receipt audit

* chore(qa): remove product-branch audit report
This commit is contained in:
Peter Steinberger
2026-08-15 23:51:40 -07:00
committed by GitHub
parent bccbb8a0c7
commit 507e8b985e
2 changed files with 48 additions and 20 deletions
@@ -6,10 +6,13 @@ import {
setCommandLaneConcurrency,
} from "../process/command-queue.js";
import { CommandLane } from "../process/lanes.js";
import { openOpenClawStateDatabase } from "../state/openclaw-state-db.js";
import { CronService } from "./service.js";
import { setupCronServiceSuite } from "./service.test-harness.js";
import type { CronEvent, CronServiceDeps } from "./service/state.js";
import { loadCronStore, saveCronStore } from "./store.js";
import { cronStoreKey } from "./store/key.js";
import { readCronTaskRunHistoryPage } from "./task-run-history.js";
import type { CronJob } from "./types.js";
const { logger, makeStorePath } = setupCronServiceSuite({
@@ -167,19 +170,44 @@ describe("cron one-shot schedule ownership", () => {
expect(replacement?.state.runningAtMs).toBeUndefined();
}
if (mode === "queued") {
// Removal aborts the in-flight run: the original run must end as the
// operator's explicit stop, never finalize "ok" into the replacement.
expect(
events.filter((event) => event.action === "finished" && event.jobId === original.id),
).toEqual([
expect.objectContaining({
status: "error",
error: "Cron job removed by operator.",
job: expect.objectContaining({ name: "removed original one-shot" }),
}),
]);
}
// Removal aborts the in-flight run: both direct and queued callers need
// one durable, visible terminal result for the original run.
const finishedEvents = events.filter(
(event) => event.action === "finished" && event.jobId === original.id,
);
expect(finishedEvents).toEqual([
expect.objectContaining({
status: "error",
error: "Cron job removed by operator.",
job: expect.objectContaining({ name: "removed original one-shot" }),
}),
]);
const history = readCronTaskRunHistoryPage({
storeKey: cronStoreKey(store.storePath),
jobId: original.id,
});
expect(history.entries).toEqual([
expect.objectContaining({
status: "error",
error: "Cron job removed by operator.",
runId: finishedEvents[0]?.runId,
}),
]);
const receipts = openOpenClawStateDatabase()
.db.prepare(
"SELECT receipt_id AS receiptId, status, error_text AS error FROM cron_run_receipts WHERE store_key = ? AND job_id = ?",
)
.all(cronStoreKey(store.storePath), original.id) as Array<{
receiptId: string;
status: string;
error: string | null;
}>;
expect(receipts).toEqual([
expect.objectContaining({
status: "error",
error: "Cron job removed by operator.",
}),
]);
} finally {
release.resolve({ status: "ok", summary: "original run finished" });
cron.stop();
+7 -7
View File
@@ -182,17 +182,17 @@ async function finishPreparedManualRun(
}
const endedAt = state.deps.nowMs();
const triggerSkipped = coreResult.status === "ok" && coreResult.triggerEval?.fired === false;
const emitMissingQueuedTerminal = () => {
const emitMissingTerminal = (required = false) => {
const tracker = prepared.terminalTracker;
if (!tracker || tracker.emitted) {
if ((!tracker && !required) || tracker?.emitted) {
return;
}
const job =
prepared.activeJobMarker?.jobRemoved === true
? executionJob
: state.store?.jobs.find((entry) => entry.id === jobId);
// enqueueRun acknowledges a concrete run id, so every accepted request
// needs one terminal event even if the job or service owner changes mid-run.
// Queued calls carry a tracker for dedupe. A removed direct run has no
// tracker, but still needs one durable terminal event/history/task outcome.
emitCronRunFinished(
state,
{
@@ -236,11 +236,11 @@ async function finishPreparedManualRun(
error: coreResult.error,
});
finalized = true;
emitMissingQueuedTerminal();
emitMissingTerminal(true);
return;
}
if (!isCronActiveJobMarkerCurrent(prepared.activeJobMarker)) {
emitMissingQueuedTerminal();
emitMissingTerminal();
return;
}
@@ -424,7 +424,7 @@ async function finishPreparedManualRun(
if (finalized) {
armTimer(state);
}
emitMissingQueuedTerminal();
emitMissingTerminal();
} finally {
// Terminal receipt persistence is fallible; local liveness and admission
// ownership must still retire or this process permanently self-fences the job.