fix: unblock timed cron cancellation

shakkernerd · shakkernerd · commit 9082233a433c · 2026-06-09T01:22:29.000+01:00
diff --git a/src/cron/service/timer.regression.test.ts b/src/cron/service/timer.regression.test.ts
@@ -937,7 +937,7 @@ describe("cron service timer regressions", () => {
     }
   });
 
-  it("keeps timeout cleanup armed after operator cancellation", async () => {
+  it("unwinds timed cron runs immediately after operator cancellation", async () => {
     vi.useFakeTimers();
     try {
       const store = timerRegressionFixtures.makeStorePath();
@@ -973,6 +973,10 @@ describe("cron service timer regressions", () => {
       const timerPromise = onTimer(state);
       const observedAbortSignal = await runnerStarted.promise;
       const runId = `cron:cancel-before-timeout:${scheduledAt}`;
+      let timerSettled = false;
+      void timerPromise.then(() => {
+        timerSettled = true;
+      });
       const cancelled = cancelActiveCronTaskRun({
         runId,
         reason: "Cancelled by operator.",
@@ -981,18 +985,17 @@ describe("cron service timer regressions", () => {
       expect(cancelled).toBe(true);
       expect(observedAbortSignal?.aborted).toBe(true);
 
-      await vi.advanceTimersByTimeAsync(Math.ceil(FAST_TIMEOUT_SECONDS * 1_000) + 10);
+      for (let attempt = 0; attempt < 5 && !timerSettled; attempt += 1) {
+        await vi.advanceTimersByTimeAsync(0);
+        await Promise.resolve();
+      }
+      expect(timerSettled).toBe(true);
       await timerPromise;
 
-      expect(cleanupTimedOutAgentRun).toHaveBeenCalledWith(
-        expect.objectContaining({
-          job: expect.objectContaining({ id: "cancel-before-timeout" }),
-          timeoutMs: FAST_TIMEOUT_SECONDS * 1_000,
-        }),
-      );
+      expect(cleanupTimedOutAgentRun).not.toHaveBeenCalled();
       const job = state.store?.jobs.find((entry) => entry.id === "cancel-before-timeout");
       expect(job?.state.lastStatus).toBe("error");
-      expect(job?.state.lastError).toContain("timed out");
+      expect(job?.state.lastError).toBe("Cancelled by operator.");
     } finally {
       vi.useRealTimers();
     }
diff --git a/src/cron/service/timer.ts b/src/cron/service/timer.ts
@@ -125,6 +125,21 @@ export async function executeJobCoreWithTimeout(
   opts?: { runId?: string },
 ): Promise<Awaited<ReturnType<typeof executeJobCore>>> {
   const runAbortController = new AbortController();
+  const operatorCancellationMarker = Symbol("cron-operator-cancelled");
+  let resolveOperatorCancellation: ((value: typeof operatorCancellationMarker) => void) | undefined;
+  const operatorCancellationPromise = new Promise<typeof operatorCancellationMarker>((resolve) => {
+    resolveOperatorCancellation = resolve;
+  });
+  const createOperatorCancellationOutcome = () => {
+    const error = abortErrorMessage(runAbortController.signal);
+    return {
+      status: "error" as const,
+      error,
+      diagnostics: createCronRunDiagnosticsFromError("cron-setup", error, {
+        nowMs: state.deps.nowMs,
+      }),
+    };
+  };
   // Main-session cron jobs enqueue work into a downstream child session. The
   // cron wrapper does not own that queued run, so it must not expose a task
   // cancellation handle that could make the wrapper row lie about child state.
@@ -133,20 +148,12 @@ export async function executeJobCoreWithTimeout(
       ? registerActiveCronTaskRun({
           runId: opts?.runId,
           controller: runAbortController,
+          onCancel: () => resolveOperatorCancellation?.(operatorCancellationMarker),
         })
       : undefined;
   const jobTimeoutMs = resolveCronJobTimeoutMs(job);
   try {
     if (typeof jobTimeoutMs !== "number") {
-      const cancellationMarker = Symbol("cron-cancelled");
-      const cancellationPromise = new Promise<typeof cancellationMarker>((resolve) => {
-        const resolveCancelled = () => resolve(cancellationMarker);
-        if (runAbortController.signal.aborted) {
-          resolveCancelled();
-          return;
-        }
-        runAbortController.signal.addEventListener("abort", resolveCancelled, { once: true });
-      });
       const corePromise = executeJobCore(state, job, runAbortController.signal);
       void corePromise.catch((err: unknown) => {
         if (runAbortController.signal.aborted) {
@@ -156,18 +163,11 @@ export async function executeJobCoreWithTimeout(
           );
         }
       });
-      const first = await Promise.race([corePromise, cancellationPromise]);
-      if (first !== cancellationMarker) {
+      const first = await Promise.race([corePromise, operatorCancellationPromise]);
+      if (first !== operatorCancellationMarker) {
         return first;
       }
-      const error = abortErrorMessage(runAbortController.signal);
-      return {
-        status: "error",
-        error,
-        diagnostics: createCronRunDiagnosticsFromError("cron-setup", error, {
-          nowMs: state.deps.nowMs,
-        }),
-      };
+      return createOperatorCancellationOutcome();
     }
 
     let timeoutReason: string | undefined;
@@ -207,7 +207,10 @@ export async function executeJobCoreWithTimeout(
       }
     });
     try {
-      const first = await Promise.race([corePromise, timeoutPromise]);
+      const first = await Promise.race([corePromise, timeoutPromise, operatorCancellationPromise]);
+      if (first === operatorCancellationMarker) {
+        return createOperatorCancellationOutcome();
+      }
       if (first !== timeoutMarker) {
         return first;
       }
diff --git a/src/tasks/cron-task-cancel.ts b/src/tasks/cron-task-cancel.ts
@@ -2,19 +2,24 @@
 
 type CronTaskCancelHandle = {
   controller: AbortController;
+  onCancel?: (reason: string) => void;
 };
 
 const activeCronTaskRunsByRunId = new Map<string, CronTaskCancelHandle>();
 
 export function registerActiveCronTaskRun(params: {
   runId: string | undefined;
   controller: AbortController;
+  onCancel?: (reason: string) => void;
 }): (() => void) | undefined {
   const runId = params.runId?.trim();
   if (!runId) {
     return undefined;
   }
-  activeCronTaskRunsByRunId.set(runId, { controller: params.controller });
+  activeCronTaskRunsByRunId.set(runId, {
+    controller: params.controller,
+    onCancel: params.onCancel,
+  });
   return () => {
     if (activeCronTaskRunsByRunId.get(runId)?.controller === params.controller) {
       activeCronTaskRunsByRunId.delete(runId);
@@ -37,7 +42,9 @@ export function cancelActiveCronTaskRun(params: {
   if (handle.controller.signal.aborted) {
     return false;
   }
-  handle.controller.abort(params.reason?.trim() || "Cancelled by operator.");
+  const reason = params.reason?.trim() || "Cancelled by operator.";
+  handle.controller.abort(reason);
+  handle.onCancel?.(reason);
   return true;
 }
 
