Uh oh!
There was an error while loading. Please reload this page.
fix(cron): bind stale async-job cutoffs through the column encoder - #6327
Conversation
The stale-processing predicate interpolated `Date` values straight into a raw `sql` template. A raw template carries no column context, so drizzle skips `PgTimestamp.mapToDriverValue` (which stringifies via `toISOString`) and postgres-js receives a `Date` it cannot serialize under the pools' `prepare: false` / `fetch_types: false` options. Every run of the job has failed its async-job sweep since the change shipped, leaving stuck jobs unreaped while the surrounding typed `lt(column, date)` sweeps succeeded. Bind both cutoffs with `sql.param(date, column)`, matching the workflow sweep in the same handler. The testing `sql` mock already rejected `sql.param(date)` for this reason but not the interpolated form that shipped, so extend it to cover both. That guard alone fails three existing tests when the fix is reverted, and the full suite shows no other route binding a bare Date this way.
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview The handler now binds both cutoffs with Reviewed by Cursor Bugbot for commit 1715c79. Configure here. |
Greptile SummaryThis PR fixes stale async-job cleanup by binding both
Confidence Score: 5/5The PR appears safe to merge, with no actionable defects identified in the changed binding or regression guard. Both cutoff values are now encoded through the matching timestamp column before reaching the database driver, and the mock detects the unsupported bare-Date form that caused the cleanup sweep to fail.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/cron/cleanup-stale-executions/route.ts | Correctly routes both stale-processing cutoffs through the timestamp column encoder, consistent with the established sibling pattern in the handler. |
| packages/testing/src/mocks/database.mock.ts | Adds a targeted test-time guard against raw Date interpolation that mirrors the production serialization failure addressed by the route fix. |
Reviews (1): Last reviewed commit: "fix(cron): bind stale async-job cutoffs ..." | Re-trigger Greptile
Summary
cleanup-stale-executionsinterpolatedDatevalues directly into a rawsqltemplate. A raw template carries no column context, so drizzle skipsPgTimestamp.mapToDriverValue(which stringifies viatoISOString) and postgres-js receives aDateit cannot serialize under the pools'prepare: false/fetch_types: falseoptionssql.param(date, column), matching the workflow sweep already doing this on line 153 of the same handlersqlmock already rejectedsql.param(date)for exactly this reason but not the interpolated form that shipped — extended it to cover bothEvidence
Every run of the job has failed its async-job sweep since v0.7.59, while the surrounding typed
lt(column, date)sweeps in the same request succeed:CleanupStaleExecutionserrors in 7 days of logs before today; first at 09:00:35 UTC, the first run after the release cut over at 08:42:45Starting stale execution cleanup joblinenow()substituted;async_jobshas zeroprocessingrowsImpact: stuck async jobs are never reaped, so job slots leak and executions stay
runningindefinitely.Type of Change
Testing
bunx vitest run app/api/cron— 15 passing. Reverting the fix with the mock guard in place fails 3 existing tests, so the regression is covered without new test code. Full suite (20,200 tests) shows no other route binding a bareDateinto asqltemplate; the 5 unrelated failures reproduce on cleanorigin/staging. Lint 23/23 and typecheck clean.Checklist
Independent verification after merge found two errors in the description above. The fix itself is correct and confirmed (reproduced locally against the real stack), but the recorded rationale is wrong:
The stated mechanism is wrong. This is not caused by the pools'
prepare: false/fetch_types: falseoptions — a 2×2 matrix test proved those are irrelevant. The actual cause isdrizzle()overwritingclient.options.serializersfor temporal OIDs. Thesql.param(date, column)fix is still the right one; only the explanation was incorrect.The fix is incomplete, and the mock guard does not prove otherwise. Five further live instances of the same bug exist, one introduced in the same commit and release as the site fixed here. The claim that the full suite proved no other instances is false: the
database.mock.tsguard only fires on paths tests actually execute, and 26 test files override the drizzle-orm mock — two of the missed sites have passing tests.Realized production impact was effectively zero. No jobs happened to get stuck during the ~9.5-hour window. The bug was real and deterministic, but caused no measurable harm.
A follow-up PR addresses the remaining instances, corrects the misleading comment in
packages/testing/src/mocks/database.mock.ts, and replaces the mock guard with a repo-wide detector that covers untested and mock-overriding code.