diff --git a/src/tasks/task-registry.maintenance.issue-60299.test.ts b/src/tasks/task-registry.maintenance.issue-60299.test.ts index b06ab795cc6..bd8e039c732 100644 --- a/src/tasks/task-registry.maintenance.issue-60299.test.ts +++ b/src/tasks/task-registry.maintenance.issue-60299.test.ts @@ -152,11 +152,17 @@ function createTaskRegistryMaintenanceHarness(params: { status: patch.status, endedAt: patch.endedAt, lastEventAt: patch.lastEventAt ?? patch.endedAt, - ...(patch.error !== undefined ? { error: patch.error } : {}), ...(patch.terminalSummary !== undefined ? { terminalSummary: patch.terminalSummary ?? undefined } : {}), } satisfies TaskRecord; + if (Object.hasOwn(patch, "error")) { + if (patch.error === undefined) { + delete next.error; + } else { + next.error = patch.error; + } + } currentTasks.set(patch.taskId, next); return next; }, @@ -532,6 +538,134 @@ describe("task-registry maintenance issue #60299", () => { expectTaskStatus(currentTasks, task.taskId, "lost"); }); + it("recovers terminal lost cron tasks from durable run logs", async () => { + const startedAt = Date.now() - GRACE_EXPIRED_MS; + const task = makeStaleTask({ + runtime: "cron", + sourceId: "cron-job-terminal-lost-ok", + runId: `cron:cron-job-terminal-lost-ok:${startedAt}`, + status: "lost", + error: "backing session missing", + startedAt, + endedAt: startedAt + 60_000, + lastEventAt: startedAt + 60_000, + cleanupAfter: Date.now() + 60_000, + }); + + const { currentTasks } = createTaskRegistryMaintenanceHarness({ + tasks: [task], + cronRunLogEntries: { + "cron-job-terminal-lost-ok": [ + { + ts: startedAt + 1250, + jobId: "cron-job-terminal-lost-ok", + action: "finished", + status: "ok", + summary: "done", + runAtMs: startedAt, + durationMs: 1250, + }, + ], + }, + }); + + const reconciled = reconcileInspectableTasks(); + expect(reconciled).toEqual([ + expect.objectContaining({ + taskId: task.taskId, + status: "succeeded", + endedAt: startedAt + 1250, + terminalSummary: "done", + }), + ]); + expect(reconciled[0]).not.toHaveProperty("error"); + expect(previewTaskRegistryMaintenance()).toMatchObject({ reconciled: 0, recovered: 1 }); + expect(await runTaskRegistryMaintenance()).toMatchObject({ reconciled: 0, recovered: 1 }); + const recoveredTask = currentTasks.get(task.taskId); + expect(recoveredTask).toMatchObject({ + status: "succeeded", + endedAt: startedAt + 1250, + terminalSummary: "done", + }); + expect(recoveredTask).not.toHaveProperty("error"); + }); + + it("does not recover terminal lost cron tasks without a backing-session error", async () => { + const startedAt = Date.now() - GRACE_EXPIRED_MS; + const task = makeStaleTask({ + runtime: "cron", + sourceId: "cron-job-terminal-lost-no-error", + runId: `cron:cron-job-terminal-lost-no-error:${startedAt}`, + status: "lost", + startedAt, + endedAt: startedAt + 60_000, + lastEventAt: startedAt + 60_000, + cleanupAfter: Date.now() + 60_000, + }); + + const { currentTasks } = createTaskRegistryMaintenanceHarness({ + tasks: [task], + cronRunLogEntries: { + "cron-job-terminal-lost-no-error": [ + { + ts: startedAt + 1250, + jobId: "cron-job-terminal-lost-no-error", + action: "finished", + status: "ok", + summary: "done", + runAtMs: startedAt, + durationMs: 1250, + }, + ], + }, + }); + + expect(previewTaskRegistryMaintenance()).toMatchObject({ recovered: 0 }); + expect(await runTaskRegistryMaintenance()).toMatchObject({ recovered: 0 }); + expect(currentTasks.get(task.taskId)).toMatchObject({ + status: "lost", + }); + }); + + it("does not recover terminal lost cron tasks with non-backing-session errors", async () => { + const startedAt = Date.now() - GRACE_EXPIRED_MS; + const task = makeStaleTask({ + runtime: "cron", + sourceId: "cron-job-terminal-lost-other-error", + runId: `cron:cron-job-terminal-lost-other-error:${startedAt}`, + status: "lost", + error: "operator marked lost", + startedAt, + endedAt: startedAt + 60_000, + lastEventAt: startedAt + 60_000, + cleanupAfter: Date.now() + 60_000, + }); + + const { currentTasks } = createTaskRegistryMaintenanceHarness({ + tasks: [task], + cronRunLogEntries: { + "cron-job-terminal-lost-other-error": [ + { + ts: startedAt + 1250, + jobId: "cron-job-terminal-lost-other-error", + action: "finished", + status: "ok", + summary: "done", + runAtMs: startedAt, + durationMs: 1250, + }, + ], + }, + }); + + expect(previewTaskRegistryMaintenance()).toMatchObject({ recovered: 0 }); + expect(await runTaskRegistryMaintenance()).toMatchObject({ recovered: 0 }); + expect(currentTasks.get(task.taskId)).toMatchObject({ + status: "lost", + error: "operator marked lost", + }); + }); + it("recovers interrupted cron tasks from durable cron job state when run logs are absent", async () => { const startedAt = Date.now() - GRACE_EXPIRED_MS; const task = makeStaleTask({ diff --git a/src/tasks/task-registry.maintenance.ts b/src/tasks/task-registry.maintenance.ts index 860483dfdd6..1c77edd31af 100644 --- a/src/tasks/task-registry.maintenance.ts +++ b/src/tasks/task-registry.maintenance.ts @@ -357,6 +357,14 @@ function isTimeoutCronError(error: string | undefined): boolean { return error === "cron: job execution timed out"; } +function isRecoverableLostCronTask(task: TaskRecord): boolean { + if (task.status !== "lost") { + return false; + } + const error = task.error?.trim().toLowerCase(); + return Boolean(error?.includes("backing session missing")); +} + function mapCronTerminalStatus(status: unknown, error?: string): CronTerminalRecovery["status"] { if (status === "ok" || status === "skipped") { return "succeeded"; @@ -454,7 +462,7 @@ function resolveDurableCronTaskRecovery( task: TaskRecord, context: CronRecoveryContext, ): CronTerminalRecovery | undefined { - if (task.runtime !== "cron" || !isActiveTask(task)) { + if (task.runtime !== "cron" || (!isActiveTask(task) && !isRecoverableLostCronTask(task))) { return undefined; } const execution = parseCronExecutionId(task); @@ -808,7 +816,7 @@ function markTaskRecovered(task: TaskRecord, recovery: CronTerminalRecovery): Ta status: recovery.status, endedAt: recovery.endedAt, lastEventAt: recovery.lastEventAt, - ...(recovery.error !== undefined ? { error: recovery.error } : {}), + error: recovery.error, ...(recovery.terminalSummary !== undefined ? { terminalSummary: recovery.terminalSummary } : {}), @@ -823,11 +831,14 @@ function projectTaskRecovered(task: TaskRecord, recovery: CronTerminalRecovery): status: recovery.status, endedAt: recovery.endedAt, lastEventAt: recovery.lastEventAt, - ...(recovery.error !== undefined ? { error: recovery.error } : {}), + error: recovery.error, ...(recovery.terminalSummary !== undefined ? { terminalSummary: recovery.terminalSummary } : {}), }; + if (recovery.error === undefined) { + delete projected.error; + } return { ...projected, ...(typeof projected.cleanupAfter === "number" diff --git a/src/tasks/task-registry.test.ts b/src/tasks/task-registry.test.ts index b2c0d5727d1..5f4b9245c19 100644 --- a/src/tasks/task-registry.test.ts +++ b/src/tasks/task-registry.test.ts @@ -616,6 +616,44 @@ describe("task-registry", () => { }); }); + it("clears terminal errors when explicitly updated without an error", async () => { + await withTaskRegistryTempDir(async (root) => { + process.env.OPENCLAW_STATE_DIR = root; + resetTaskRegistryForTests(); + + const task = createTaskRecord({ + runtime: "cron", + ownerKey: "system:cron:test", + scopeKind: "system", + runId: "run-terminal-error-clear", + task: "Recover cron task", + status: "running", + deliveryStatus: "not_applicable", + startedAt: 100, + }); + + markTaskTerminalById({ + taskId: task.taskId, + status: "failed", + endedAt: 200, + error: "backing session missing", + }); + markTaskTerminalById({ + taskId: task.taskId, + status: "succeeded", + endedAt: 250, + error: undefined, + }); + + const recoveredTask = getTaskById(task.taskId); + expect(recoveredTask).toMatchObject({ + status: "succeeded", + endedAt: 250, + }); + expect(recoveredTask).not.toHaveProperty("error"); + }); + }); + it("keeps stronger run-scoped terminal states when a late success arrives", async () => { await withTaskRegistryTempDir(async () => { resetTaskRegistryMemoryForTest(); diff --git a/src/tasks/task-registry.ts b/src/tasks/task-registry.ts index 4e1fa27909b..1db89b54838 100644 --- a/src/tasks/task-registry.ts +++ b/src/tasks/task-registry.ts @@ -1180,6 +1180,9 @@ function updateTask(taskId: string, patch: Partial): TaskRecord | nu return null; } const next = normalizeTaskTimestamps({ ...current, ...patch }); + if (Object.hasOwn(patch, "error") && patch.error === undefined) { + delete next.error; + } if (isTerminalTaskStatus(next.status) && typeof next.cleanupAfter !== "number") { next.cleanupAfter = resolveTaskCleanupAfter({ ...next, @@ -1553,11 +1556,10 @@ export function markTaskTerminalById(params: { terminalOutcome?: TaskTerminalOutcome | null; }): TaskRecord | null { ensureTaskRegistryReady(); - return updateTask(params.taskId, { + const patch: Partial = { status: params.status, endedAt: params.endedAt, lastEventAt: params.lastEventAt ?? params.endedAt, - ...(params.error !== undefined ? { error: params.error } : {}), ...(params.terminalSummary !== undefined ? { terminalSummary: normalizeTaskSummary(params.terminalSummary) } : {}), @@ -1569,7 +1571,11 @@ export function markTaskTerminalById(params: { }), } : {}), - }); + }; + if (Object.hasOwn(params, "error")) { + patch.error = params.error; + } + return updateTask(params.taskId, patch); } export function markTaskLostById(params: {