fix(run-engine): always validate fast-path skip on the writer

Replica probe stays on `readOnlyPrisma` (when `useReplicaForFastPathRead`
is on) but the full-run read used to construct the returned `existing`
result is now always issued against the writer. The writer-side read
also re-checks `status === DELAYED`, `delayUntil`, and the max-duration
window so replica lag can't cause a trigger to silently collapse onto a
run that has already moved out of DELAYED on the writer.

Also clarifies the `fastPathSkipEnabled` doc to note that trailing-mode
triggers carrying `updateData` always bypass the fast path.

Co-Authored-By: Eric Allam <eallam@icloud.com>

Author: devin-ai-integration[bot]

internal-packages/run-engine/src/engine/systems/debounceSystem.ts

@@ -63,8 +63,11 @@ export type DebounceSystemOptions = {
    */
   fastPathSkipEnabled?: boolean;
   /**
-   * When true, route the unlocked fast-path read through `readOnlyPrisma`
-   * (e.g. an Aurora reader) instead of the writer.
+   * When true, route the cheap probe of the unlocked fast-path through
+   * `readOnlyPrisma` (e.g. an Aurora reader) instead of the writer. The
+   * full-run read used to construct the returned `existing` result still
+   * goes through the writer, so callers never see a run whose status has
+   * already moved out of DELAYED on the writer due to replica lag.
    */
   useReplicaForFastPathRead?: boolean;
 };
@@ -479,12 +482,13 @@ return 0
     tx?: PrismaClientOrTransaction;
   }): Promise<DebounceResult> {
     const prisma = tx ?? this.$.prisma;
-    // Reads that are explicitly best-effort (the fast-path skip) can run on
-    // `readOnlyPrisma` when configured. Replica lag is fine: the monotonic-
-    // forward invariant means a stale read just falls through to the locked
-    // path. Only divert reads when the caller isn't inside a tx (where the
-    // read needs to see the tx's writes).
-    const fastPathReadPrisma =
+    // The cheap probe in the fast-path skip can run on `readOnlyPrisma` when
+    // configured. Replica lag is fine because the probe is best-effort: a
+    // stale view either falls through to the locked path or is rejected by
+    // the writer-validated re-check inside `#tryFastPathSkip`. Only divert
+    // the probe when the caller isn't inside a tx (where the read needs to
+    // see the tx's writes).
+    const probeReadPrisma =
       tx ?? (this.useReplicaForFastPathRead ? this.$.readOnlyPrisma : this.$.prisma);
 
     // Compute the (quantized) target delayUntil up-front, before taking any lock.
@@ -503,7 +507,12 @@ return 0
         existingRunId,
         newDelayUntil,
         debounce,
-        prisma: fastPathReadPrisma,
+        probePrisma: probeReadPrisma,
+        // The full-run read used to construct the returned `existing` result
+        // always goes through the writer, even when the cheap probe is on a
+        // replica. Otherwise replica lag could let us return a run whose
+        // status has already moved out of DELAYED on the writer.
+        validatePrisma: prisma,
       });
       if (fastPathResult) {
         return fastPathResult;
@@ -578,25 +587,34 @@ return 0
    * the lock to apply their data update. Also falls through when the run has
    * already exceeded its max debounce duration so the locked path can return
    * `max_duration_exceeded` and let the caller create a new run.
+   *
+   * The cheap probe (`probePrisma`) may be on a read replica - replica lag is
+   * fine because the monotonic-forward invariant means a stale view just falls
+   * through to the locked path. The full-run read used to construct the
+   * returned `existing` result always goes through `validatePrisma` (the
+   * writer), so callers never receive a run whose status has already moved out
+   * of DELAYED on the writer due to replica lag.
    */
   async #tryFastPathSkip({
     existingRunId,
     newDelayUntil,
     debounce,
-    prisma,
+    probePrisma,
+    validatePrisma,
   }: {
     existingRunId: string;
     newDelayUntil: Date;
     debounce: DebounceOptions;
-    prisma: PrismaClientOrTransaction | PrismaReplicaClient;
+    probePrisma: PrismaClientOrTransaction | PrismaReplicaClient;
+    validatePrisma: PrismaClientOrTransaction;
   }): Promise<DebounceResult | null> {
     // Trailing mode with updateData still needs the lock so the data update is
     // applied; only short-circuit when there's nothing to update.
     if (debounce.mode === "trailing" && debounce.updateData) {
       return null;
     }
 
-    const probe = await prisma.taskRun.findFirst({
+    const probe = await probePrisma.taskRun.findFirst({
       where: { id: existingRunId },
       select: { status: true, delayUntil: true, createdAt: true },
     });
@@ -622,11 +640,20 @@ return 0
       return null;
     }
 
-    const fullRun = await prisma.taskRun.findFirst({
+    // Validate against the writer before returning. Also re-checks delayUntil
+    // and the max-duration window in case the writer has moved on since the
+    // (possibly stale) probe.
+    const fullRun = await validatePrisma.taskRun.findFirst({
       where: { id: existingRunId },
       include: { associatedWaitpoint: true },
     });
-    if (!fullRun || fullRun.status !== "DELAYED") {
+    if (!fullRun || fullRun.status !== "DELAYED" || !fullRun.delayUntil) {
+      return null;
+    }
+    if (newDelayUntil.getTime() > fullRun.delayUntil.getTime()) {
+      return null;
+    }
+    if (newDelayUntil.getTime() > fullRun.createdAt.getTime() + maxDurationMs) {
       return null;
     }
 

internal-packages/run-engine/src/engine/types.ts

@@ -146,16 +146,19 @@ export type RunEngineOptions = {
     /**
      * Whether to read the existing run's `delayUntil` outside of the redlock and
      * short-circuit when the new (quantized) `delayUntil` is not later than the
-     * current one. Drops `updateData` when triggered for trailing mode.
+     * current one. Trailing-mode triggers carrying `updateData` always bypass
+     * this fast path and take the lock so payload/metadata/tag updates still
+     * land on the run.
      *
      * Default: true.
      */
     fastPathSkipEnabled?: boolean;
     /**
-     * Whether to route the unlocked fast-path read of `delayUntil`/`createdAt`
-     * through `readOnlyPrisma` (e.g. an Aurora reader) instead of the writer.
-     * Safe because the read is best-effort and re-checked under the lock by
-     * whichever caller is actually pushing forward; replica lag at worst means
+     * Whether to route the cheap probe of the unlocked fast-path through
+     * `readOnlyPrisma` (e.g. an Aurora reader) instead of the writer. The
+     * full-run read used to construct the returned `existing` result still
+     * goes through the writer, so callers never see a run whose status has
+     * already moved out of DELAYED on the writer. Replica lag at worst means
      * a few extra callers fall through to the lock.
      *
      * Default: false.