fix(schedule-engine): centralize lastScheduleTime cron-prev fallback

External callers of registerNextTaskScheduleInstance — the deploy-time
declarative schedule sync, schedule upsert (cron change / activate),
and recovery — all called with just `{ instanceId }`, no
lastScheduleTime. That replaced the in-flight Redis job's payload with
one having lastScheduleTime: undefined, so the next fire fell back to
instance.lastScheduledTimestamp (a column this PR stops writing). On
every subsequent app deploy, customers would have seen one stale fire
per schedule, in perpetuity — the staleness compounding as the
unmaintained DB column drifted further from reality.

Move the cron-prev derivation inside registerNextTaskScheduleInstance
itself: when the caller doesn't pass lastScheduleTime, derive from the
cron expression's previous slot (guarded against the slot predating
the instance's createdAt — preserves first-fire `undefined` semantics
for brand-new schedules). Internal callers (after-fire, after-skip)
keep passing explicit values so their carry-forward semantics are
unchanged.

Also drops the duplicate cron-prev block from `#recoverTaskScheduleInstance`
— recovery now relies on the centralized fallback inside register.

Two new tests:
- "should derive lastScheduleTime from cron when external callers omit
  it" — exercises the deploy/upsert pattern on a long-running instance.
- "should leave lastScheduleTime undefined for brand-new schedules" —
  preserves the first-run sentinel (`if (!payload.lastTimestamp)`)
  customers rely on.

Per Devin review on PR #3476.

Author: ericallam

internal-packages/schedule-engine/src/engine/index.ts

@@ -198,16 +198,43 @@ export class ScheduleEngine {
           timezone: instance.taskSchedule.timezone,
         });
 
-        // Enqueue the scheduled task. The next job's `lastScheduleTime`
-        // payload is the *actual* previous fire time (passed in by the
-        // caller), not `fromTimestamp` — `fromTimestamp` advances on every
-        // tick (including skips) so it can't be used as the previous-fire
-        // anchor without leaking skipped slots into customer-visible
-        // payload.lastTimestamp.
+        // Determine the lastScheduleTime to embed in the next worker job's
+        // payload. If the caller passed it explicitly (the after-fire path
+        // does this with the just-fired timestamp, the after-skip path
+        // carries the existing value forward), use that. Otherwise — every
+        // external caller (deploy sync, schedule upsert, recovery) — derive
+        // from the cron expression's previous slot.
+        //
+        // Without this fallback, every deploy / cron edit would clobber the
+        // existing in-flight job's lastScheduleTime with `undefined`, and
+        // the next fire would surface a frozen DB-column value to the
+        // customer (since this PR stops writing that column). Pure cron
+        // math, no DB read on top of the existing instance load — the
+        // recovery loop already pays the cost of loading the instance.
+        let lastScheduleTime = params.lastScheduleTime;
+        if (lastScheduleTime === undefined) {
+          try {
+            const cronPrev = previousScheduledTimestamp(
+              instance.taskSchedule.generatorExpression,
+              instance.taskSchedule.timezone
+            );
+            // Guarded against the cron's previous slot predating the
+            // instance itself — for a brand-new schedule, the slot is from
+            // before the schedule existed, so `undefined` is the honest
+            // answer (preserves the `if (!payload.lastTimestamp)` first-run
+            // sentinel customers rely on).
+            if (cronPrev.getTime() > instance.createdAt.getTime()) {
+              lastScheduleTime = cronPrev;
+            }
+          } catch {
+            // Malformed cron — leave undefined.
+          }
+        }
+
         await this.enqueueScheduledTask(
           params.instanceId,
           nextScheduledTimestamp,
-          params.lastScheduleTime
+          lastScheduleTime
         );
 
         // Record metrics
@@ -800,39 +827,16 @@ export class ScheduleEngine {
       return "skipped";
     }
 
-    // Approximate the previous fire from the cron expression itself rather
-    // than reading state from the DB. For a continuously-running schedule
-    // this equals the actual last fire time. For paused-then-resumed
-    // schedules or recently-edited cron expressions the value will be
-    // approximate — same trade-off the dashboard "Last run" cell accepts.
-    // Guarded against the schedule not having existed long enough to have
-    // fired (cron's previous slot before instance creation), and against
-    // cron-parser throwing on malformed expressions. Pure cron math, no DB
-    // read — recovery fan-outs (Redis crash, restart storms) must not add
-    // load to hot tables.
-    let lastScheduleTime: Date | undefined;
-    try {
-      const cronPrev = previousScheduledTimestamp(
-        schedule.generatorExpression,
-        schedule.timezone
-      );
-      if (cronPrev.getTime() > instance.createdAt.getTime()) {
-        lastScheduleTime = cronPrev;
-      }
-    } catch {
-      lastScheduleTime = undefined;
-    }
-
     this.logger.debug("No job found for instance, registering next run", {
       instanceId: instance.id,
       schedule,
-      lastScheduleTime: lastScheduleTime?.toISOString(),
     });
 
-    await this.registerNextTaskScheduleInstance({
-      instanceId: instance.id,
-      lastScheduleTime,
-    });
+    // No `lastScheduleTime` passed — `registerNextTaskScheduleInstance`
+    // will derive it from the cron's previous slot (with a createdAt
+    // guard) so the post-recovery fire reports an accurate
+    // `payload.lastTimestamp`.
+    await this.registerNextTaskScheduleInstance({ instanceId: instance.id });
 
     return "recovered";
   }

internal-packages/schedule-engine/test/scheduleRecovery.test.ts

@@ -386,4 +386,194 @@ describe("Schedule Recovery", () => {
       }
     }
   );
+
+  // External-caller backward-compat. Deploy sync (`syncDeclarativeSchedules`)
+  // and schedule upsert both call `registerNextTaskScheduleInstance` with no
+  // `lastScheduleTime`. They run on every app deploy and on every cron edit.
+  // For an existing-and-firing schedule, the call must NOT clobber the
+  // worker payload's `lastScheduleTime` with `undefined` — otherwise the
+  // next fire would surface a stale frozen DB-column value to the customer
+  // (since this PR stops writing that column). The function must derive a
+  // sensible `lastScheduleTime` from the cron expression's previous slot
+  // when the caller doesn't pass one.
+  containerTest(
+    "should derive lastScheduleTime from cron when external callers omit it",
+    { timeout: 30_000 },
+    async ({ prisma, redisOptions }) => {
+      const engine = new ScheduleEngine({
+        prisma,
+        redis: redisOptions,
+        distributionWindow: { seconds: 10 },
+        worker: { concurrency: 1, disabled: true, pollIntervalMs: 1000 },
+        tracer: trace.getTracer("test", "0.0.0"),
+        onTriggerScheduledTask: async () => ({ success: true }),
+        isDevEnvironmentConnectedHandler: vi.fn().mockResolvedValue(true),
+      });
+
+      try {
+        const organization = await prisma.organization.create({
+          data: { title: "External Caller Org", slug: "external-caller-org" },
+        });
+        const project = await prisma.project.create({
+          data: {
+            name: "External Caller Project",
+            slug: "external-caller-project",
+            externalRef: "external-caller-ref",
+            organizationId: organization.id,
+          },
+        });
+        const environment = await prisma.runtimeEnvironment.create({
+          data: {
+            slug: "external-caller-env",
+            type: "PRODUCTION",
+            projectId: project.id,
+            organizationId: organization.id,
+            apiKey: "tr_external_1234",
+            pkApiKey: "pk_external_1234",
+            shortcode: "external-short",
+          },
+        });
+        const taskSchedule = await prisma.taskSchedule.create({
+          data: {
+            friendlyId: "sched_external_caller",
+            taskIdentifier: "external-caller-task",
+            projectId: project.id,
+            deduplicationKey: "external-caller-dedup",
+            userProvidedDeduplicationKey: false,
+            generatorExpression: "*/5 * * * *",
+            generatorDescription: "Every 5 minutes",
+            timezone: "UTC",
+            type: "DECLARATIVE",
+            active: true,
+          },
+        });
+
+        // Backdate the instance so the cron's previous slot postdates
+        // createdAt — this simulates a long-running schedule, the case
+        // Devin flagged (deploy clobbers lastScheduleTime, post-deploy fire
+        // would otherwise read from a frozen DB column).
+        const longAgo = new Date(Date.now() - 24 * 60 * 60 * 1000);
+        const scheduleInstance = await prisma.taskScheduleInstance.create({
+          data: {
+            taskScheduleId: taskSchedule.id,
+            environmentId: environment.id,
+            projectId: project.id,
+            active: true,
+          },
+        });
+        await prisma.taskScheduleInstance.update({
+          where: { id: scheduleInstance.id },
+          data: { createdAt: longAgo },
+        });
+
+        // External-caller pattern — no lastScheduleTime.
+        await engine.registerNextTaskScheduleInstance({
+          instanceId: scheduleInstance.id,
+        });
+
+        const job = await engine.getJob(`scheduled-task-instance:${scheduleInstance.id}`);
+        expect(job).not.toBeNull();
+        // The function should have derived lastScheduleTime from cron,
+        // putting a real timestamp into the worker payload rather than
+        // undefined. The Redis worker stores payloads as JSON, so the value
+        // is a string when read back here — Zod re-coerces it to Date on
+        // dequeue (workerCatalog uses `z.coerce.date()`).
+        const enqueuedLastScheduleTime = (job?.item as { lastScheduleTime?: string })
+          .lastScheduleTime;
+        expect(enqueuedLastScheduleTime).toBeDefined();
+        const derived = new Date(enqueuedLastScheduleTime!);
+        // The derived value should match the cron's previous slot — for
+        // `*/5 * * * *`, a 5-minute boundary in the recent past.
+        expect(derived.getTime()).toBeLessThan(Date.now());
+        expect(derived.getUTCSeconds()).toBe(0);
+        expect(derived.getUTCMinutes() % 5).toBe(0);
+      } finally {
+        await engine.quit();
+      }
+    }
+  );
+
+  // Brand-new schedules must NOT receive a cron-derived lastScheduleTime —
+  // the cron's previous slot predates the instance, so it's not a real
+  // previous fire. The first-run sentinel (`if (!payload.lastTimestamp)`)
+  // must keep working.
+  containerTest(
+    "should leave lastScheduleTime undefined for brand-new schedules",
+    { timeout: 30_000 },
+    async ({ prisma, redisOptions }) => {
+      const engine = new ScheduleEngine({
+        prisma,
+        redis: redisOptions,
+        distributionWindow: { seconds: 10 },
+        worker: { concurrency: 1, disabled: true, pollIntervalMs: 1000 },
+        tracer: trace.getTracer("test", "0.0.0"),
+        onTriggerScheduledTask: async () => ({ success: true }),
+        isDevEnvironmentConnectedHandler: vi.fn().mockResolvedValue(true),
+      });
+
+      try {
+        const organization = await prisma.organization.create({
+          data: { title: "Brand New Org", slug: "brand-new-org" },
+        });
+        const project = await prisma.project.create({
+          data: {
+            name: "Brand New Project",
+            slug: "brand-new-project",
+            externalRef: "brand-new-ref",
+            organizationId: organization.id,
+          },
+        });
+        const environment = await prisma.runtimeEnvironment.create({
+          data: {
+            slug: "brand-new-env",
+            type: "PRODUCTION",
+            projectId: project.id,
+            organizationId: organization.id,
+            apiKey: "tr_brandnew_1234",
+            pkApiKey: "pk_brandnew_1234",
+            shortcode: "brandnew-short",
+          },
+        });
+        const taskSchedule = await prisma.taskSchedule.create({
+          data: {
+            friendlyId: "sched_brand_new",
+            taskIdentifier: "brand-new-task",
+            projectId: project.id,
+            deduplicationKey: "brand-new-dedup",
+            userProvidedDeduplicationKey: false,
+            // Hourly cron — the previous slot is plausibly minutes-to-an-hour
+            // ago, comfortably predating an instance just created.
+            generatorExpression: "0 * * * *",
+            generatorDescription: "Hourly",
+            timezone: "UTC",
+            type: "DECLARATIVE",
+            active: true,
+          },
+        });
+        const scheduleInstance = await prisma.taskScheduleInstance.create({
+          data: {
+            taskScheduleId: taskSchedule.id,
+            environmentId: environment.id,
+            projectId: project.id,
+            active: true,
+          },
+        });
+
+        await engine.registerNextTaskScheduleInstance({
+          instanceId: scheduleInstance.id,
+        });
+
+        const job = await engine.getJob(`scheduled-task-instance:${scheduleInstance.id}`);
+        expect(job).not.toBeNull();
+        const enqueuedLastScheduleTime = (job?.item as { lastScheduleTime?: Date }).lastScheduleTime;
+        // Brand-new schedule: cron's previous slot predates instance.createdAt,
+        // so the function leaves lastScheduleTime undefined — the first fire
+        // will report `payload.lastTimestamp: undefined` and customer first-run
+        // sentinel patterns keep working.
+        expect(enqueuedLastScheduleTime).toBeUndefined();
+      } finally {
+        await engine.quit();
+      }
+    }
+  );
 });