fix(schedules): locking schedules to prevent double runs

[icecrasher321]

Summary

Schedules queuing and execution should be in a locked section.

Type of Change

• Bug fix

Testing

Tested manually by running cron

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
docs	Skipped			Nov 8, 2025 3:39am

[greptile-apps]

Greptile Overview

Greptile Summary

This PR implements optimistic locking for schedule execution to prevent double runs. The approach uses a new lastQueuedAt timestamp column that gets set when schedules are queued (line 30 in route.ts), and the query condition ensures only schedules where lastQueuedAt < nextRunAt or lastQueuedAt IS NULL are selected (lines 38-40).

Critical Issue Found:

• The lock (lastQueuedAt) is never released after execution completes, which will cause all schedules to stop running after their first execution
• Missing lastQueuedAt: null in three places: successful execution (line 650), failed execution (line 682), and error catch block (line 727)

Positive Changes:

• Major code refactoring improves maintainability with well-named helper functions
• Proper use of UPDATE...RETURNING pattern provides atomic locking at database level
• Schema change is clean and appropriate

Confidence Score: 0/5

• Critical bug will break all schedule functionality after first execution
• The locking mechanism is incomplete - lastQueuedAt is set during queuing but never cleared after execution completes. This means every schedule will run exactly once and then become permanently locked, breaking the entire scheduling system. This is a production-breaking bug that must be fixed before merge.
• apps/sim/background/schedule-execution.ts requires immediate attention - three missing lastQueuedAt: null updates will break all schedules

Important Files Changed

File Analysis

Filename	Score	Overview
apps/sim/app/api/schedules/execute/route.ts	3/5	Implements optimistic locking via UPDATE with RETURNING for schedule queueing, but has a critical issue where lock is never released in execution phase
apps/sim/background/schedule-execution.ts	2/5	Major refactor adds helper functions and lock release logic, but missing lock release on successful execution path creates deadlock potential
packages/db/schema.ts	5/5	Added lastQueuedAt column to support optimistic locking mechanism for schedule execution

Sequence Diagram

sequenceDiagram
    participant Cron
    participant RouteAPI as Execute Route
    participant DB as Database
    participant Executor as Schedule Executor
    
    Cron->>RouteAPI: GET /api/schedules/execute
    RouteAPI->>DB: UPDATE workflow_schedule<br/>SET lastQueuedAt = now<br/>WHERE nextRunAt <= now<br/>AND (lastQueuedAt IS NULL OR lastQueuedAt < nextRunAt)<br/>RETURNING *
    DB-->>RouteAPI: Return locked schedules
    
    Note over RouteAPI,DB: Optimistic locking:<br/>Only schedules not already queued
    
    loop For each schedule
        RouteAPI->>Executor: Queue execution job
        Executor->>Executor: Check rate limits & usage
        Executor->>Executor: Load workflow & execute
        
        alt Success
            Executor->>DB: UPDATE schedule<br/>SET lastRanAt = now,<br/>nextRunAt = calculated,<br/>failedCount = 0<br/>❌ MISSING: lastQueuedAt = null
            Note over Executor,DB: BUG: Lock never released!
        else Failure
            Executor->>DB: UPDATE schedule<br/>SET failedCount++,<br/>nextRunAt = calculated<br/>❌ MISSING: lastQueuedAt = null
        else Error
            Executor->>DB: UPDATE schedule<br/>SET failedCount++<br/>❌ MISSING: lastQueuedAt = null
        end
    end
    
    RouteAPI-->>Cron: Return success

Loading

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}