MotirBuilding in public
MOTIR · moooon
onMotir
You’re viewing a public project. Anyone can view it — no account needed. Sign in to submit, upvote, or comment on requests.View-only — you can’t edit work items
MOTIR-3469

(motir-core) The per-tick KEY — `job_queue.scheduled_for`, a unique on `(job_id, scheduled_for)`, and an insert-if-absent enqueue

Done
Description

Give a scheduled run the identity that makes enqueuing it idempotent, so two workers ticking the same minute produce ONE run. A migration, a Prisma model change, and one repository method — no scheduler yet.

The defect this closes, in the schema's own words

prisma/schema.prisma, model JobQueueRun, on origin/main@a26d500d:

⚠️ eventId is NULLABLE, and deliberately: a CRON run has no triggering event. That is also what makes the (eventId, jobId) unique constraint below the right shape — in Postgres a NULL never equals another NULL, so the constraint dedups event-triggered enqueues without ever constraining scheduled ones.

That is correct and it is exactly the gap. dispatchEventToEngine leans on @@unique([eventId, jobId]) for the property it calls out in its own header — "ENQUEUE IS IDEMPOTENT PER (event, job) … The guarantee is the UNIQUE constraint, not a check-then-insert: a check would be a read-derived write with a race in the middle." A scheduled enqueue has no such constraint available, so the same read-derived write — has this tick already been enqueued? then insert — is a race with nothing under it. fly.toml already runs the app group at min_machines_running = 2; the moment the worker group is at more than one machine, both tick the same minute.

The shape

Add a nullable scheduledFor DateTime? @map("scheduled_for") to JobQueueRun and @@unique([jobId, scheduledFor]). The same NULL-never-equals-NULL property that makes the existing constraint safe for cron runs makes this one safe for event runs: an event-triggered row carries a NULL scheduled_for and is untouched by it. The two constraints are complementary halves of one idea, and the comment on the new one should say so rather than repeating the mechanism.

Then jobQueueRepository.enqueueScheduled(...) — insert, and treat Prisma's P2002 as "this tick is already queued" rather than as an error, which is precisely what the dispatcher already does for its own constraint and for the same reason.

The clock trap this file already documents — read it before writing SQL

lib/repositories/jobQueueRepository.ts opens with a warning that cost a whole test suite once: Prisma maps DateTime to timestamp(3) WITHOUT time zone and writes naive UTC, so every clock expression in that file is (now() AT TIME ZONE 'UTC'), never bare now() — and the bug it prevents is invisible on CI and on Neon (both UTC) and fatal under a non-UTC session. A fire time is written by the scheduler and read by a claim on another machine, so it is exactly the cross-process case the warning names. If this card writes raw SQL, it obeys that rule; if it uses the Prisma builder, it says why that is safe here.

Scope boundary

ENDS at: the column, the constraint, the migration, the repository method, and their unit tests. Nothing calls enqueueScheduled yet — the scheduler card is its first caller. That is deliberate and has a precedent in this very subsystem: registerEngineJob shipped in MOTIR-3421's pass as "PURELY ADDITIVE … this table is written and never read".

Does NOT touch claimDueRuns, reclaimExpiredLeases, the lease logic, or dispatchEventToEngine. An event-triggered enqueue behaves identically before and after this card, and a test asserts that rather than assuming it.

Acceptance criteria

  • prisma migrate dev produces ONE migration adding scheduled_for and the unique index, and prisma migrate diff against the datamodel is empty afterwards — no spurious rename. Follow CLAUDE.md § Migrations: a plain @@unique, never a hand-written PARTIAL index, which is the drift hazard the sibling @@index([state, runAt]) comment already records at length.
  • A second insert with the same (jobId, scheduledFor) raises P2002, and enqueueScheduled returns a discriminated already-queued result rather than throwing — asserted against a real Postgres, because a unique constraint is a database property and a mocked test asserts nothing about it.
  • Two concurrent enqueueScheduled calls for the same (jobId, scheduledFor) produce exactly one row — driven concurrently against a warm pool, not as two sequential calls, for the reason claimDueRuns' header gives about serial tests.
  • An event-triggered enqueue is unaffected: scheduled_for is NULL, @@unique([eventId, jobId]) still dedups, and two scheduled rows for the SAME job at DIFFERENT fire times both insert.
  • The migration replays from empty. tests/jobs/engine-schema.test.ts is the existing home for that assertion and for the EXPLAIN on the claim index; extend it rather than starting a parallel file.
  • RLS on job_queue is unchanged and re-asserted: this is a system.* row with a NULL workspace_id, and tests/jobs/rls.test.ts already covers the table.
  • Every clock expression this card adds reads (now() AT TIME ZONE 'UTC'), or the card states in a comment why the write is bound in JS and safe.

Context refs

  • prisma/schema.prismamodel JobQueueRun, its two @@index comments and the NULL-event note quoted above
  • lib/repositories/jobQueueRepository.ts — the timezone warning, create, and the claim's read-derived-write argument
  • lib/jobs/engine/dispatcher.ts — property (2) in its header: the P2002-as-expected-outcome precedent to mirror
  • CLAUDE.md § Migrations — the partial-index differ trap
  • tests/jobs/engine-schema.test.ts · tests/jobs/rls.test.ts — the suites to extend
  • fly.toml [processes] — why "more than one worker" is the case to design for, not a hypothetical