Repository navigation
Backport #4757: fix(world-postgres): store timestamps as timestamp with time zone - #4892
Open
github-actions[bot] wants to merge 1 commit into
Open
github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
) `@workflow/world-postgres` stores every timestamp column except `hooks.token_retention_until` as `timestamp without time zone`. On a Postgres server (or session) that is not in UTC, every `createdAt` the World returns is off by the UTC offset. This PR moves all timestamp columns to `timestamp with time zone`, with migration `0027` converting existing databases and repairing the values that were stored wrong. - `created_at` comes from `DEFAULT now()`, which Postgres casts to the session's local wall time for a naive column. drizzle reads naive columns back as UTC (`new Date(value + "+0000")`). - JS-written columns (`started_at`, `completed_at`, `retry_after`, ...) go in as ISO strings whose `Z` Postgres ignores, so they hold UTC wall time and read back correctly. - Result, with the server in `America/Los_Angeles`: every run's and step's `createdAt` is 7 h **before** its `startedAt`. `Asia/Kolkata`: 5.5 h in the future. A pool with `-c TimeZone=Asia/Tokyo` on a UTC server: 9 h in the future. - It is more than cosmetic: `event.createdAt` drives the deterministic workflow clock (`advanceClock(+event.createdAt)` in both runtimes), so `Date.now()` inside workflow code stays pinned at run start for hours on UTC-negative servers and jumps ahead on UTC-positive ones. Found while building a durable agent harness on top of Workflow whose CI checks what `workflow inspect` shows for runs on each World: on world-postgres 5.0.2 with the database in `America/Los_Angeles`, `createdAt` was 7 h before `startedAt` for every run and step, and correct with the server in UTC. Present on `main`; no existing issue or PR covers it. - **`src/drizzle/schema.ts`**: every timestamp column is `timestamp with time zone`, via a small `timestamptz` custom type with a comment on why. No storage code changes: ISO `Z` strings and `now()` both land as the right instant. TS types stay `Date`. The reader parses Postgres' offset-suffixed output as is, and reads a value with no offset (only an unconverted column prints one) as UTC, as earlier versions did; drizzle's own `withTimezone` reader would parse it in the Node process's local zone. - **Migration `0027_timestamps_with_time_zone.sql`** (one `DO` block; restores the session TimeZone it changes): 1. Sets the session to UTC and converts the 21 columns with no `USING` clause. Postgres 12+ skips the table rewrite for `timestamp` -> `timestamptz` when TimeZone is UTC, and stored values are taken as UTC, which is already right for every JS-written column. 2. If the zone the defaults were written in is not a UTC alias, moves values that only ever came from `now()` from that zone's wall time to the instant they recorded: `created_at` on runs, events, steps, hooks, waits, invocations and stream chunks; `updated_at` on runs/steps/waits only where `updated_at = created_at` (never updated, so both came from the same `now()`); `invocations.responded_at`. Left alone: `invocations.expired_at` (mixes `now()` and JS dates, only null-checked) and `snapshots.created_at` (always written from JS). 3. The legacy zone is the migration session's TimeZone, overridable with `workflow.legacy_timezone` (e.g. `?options=-c%20workflow.legacy_timezone%3DEurope%2FBerlin`) for apps whose pool set a different session zone than `bootstrap` uses. `ALTER DATABASE/ROLE ... SET workflow.legacy_timezone` works too, for poolers that drop startup `options`. 4. Runs once. The drizzle migrator takes no lock, so two `bootstrap` processes started together both apply 0027, and unlike earlier migrations a second application would not fail: it would move the repaired values by the offset again. The block locks the 8 tables first and returns without changes when `workflow_runs.created_at` is already `timestamptz`. It waits at most 10 s for those locks (unless the session set its own `lock_timeout`), so a long transaction fails `bootstrap` instead of queueing every query behind it. - **`meta/_journal.json`**: entry `idx 27`, `when` one day after main's `0026_add_seq_in_band`. - **Package README**: a "Timestamps and time zones" section on the migration and the override. - **Docs** (`docs/content/worlds/v5/postgres.mdx`): a warning callout under Installation. The page says migrations can run as a post-deployment script; on non-UTC servers 0027 needs the upgrade order below. - **Changeset**: `@workflow/world-postgres` patch. - **Test hygiene**: the new test's pools ignore `57P01` ("terminating connection due to administrator command"). `pool.end()` resolves before its sockets close, so stopping the container can still hand that error to a closing client; pg-pool re-emits it on the pool, and with no listener vitest counts an uncaught exception and fails the job even though every test passed. Other errors are still thrown. - **UTC servers** (the default on Neon, Supabase, RDS and the Docker image): catalog-only ALTER, milliseconds under ACCESS EXCLUSIVE on the 8 tables, plus a rebuild of the small `workflow_invocations_pending` partial index. No data changes. Postgres 11 and older rewrite the tables on the ALTER regardless (still correct, slower). - **Non-UTC servers**: the repair UPDATE rewrites every `created_at` row (events and stream chunks can be large) while holding those locks, once. - **Upgrade window.** New code against an unmigrated DB reads naive timestamps as UTC, as old code did, in any Node time zone. Old code against a migrated DB reads correctly in a UTC session and shifted by the session's offset in a non-UTC one. A shifted read moves more than display: `step_started` compares `retryAfter` with `Date.now()`, so a retry backoff can be skipped or stretched by hours. On a non-UTC server: stop the previous version's workers, run `bootstrap`, then start the upgraded app (the README and the new docs callout say this). For the same reason there is no rollback past 0027 on a non-UTC server. - **Assumed zone.** The repair assumes old rows were written in the migration session's zone; it is wrong if the server zone changed over time or the app's pool used another zone (hence `workflow.legacy_timezone`). Wall times in a DST fall-back hour are ambiguous and can land 1 h off. - Consumers of `@workflow/world-postgres/schema` see the column SQL type change; the TS types stay `Date`. New `test/timezone.test.ts` (testcontainers `postgres:15-alpine`, skipped on Windows like the other container tests; per-database time zones are set with `ALTER DATABASE ... SET timezone`): - **Fresh schema** with the database in `America/Los_Angeles` and `Asia/Kolkata`, and a UTC server with a pool setting `TimeZone=Asia/Tokyo`: drive run/step/hook/wait creation through the storage API and assert every `createdAt`/`startedAt` (write results, `runs.get`, `steps.get`, `hooks.get`, `events.list`) is the current instant. - **Schema check**: every timestamp column in `workflow` is `timestamp with time zone` and the `created_at` defaults stay `now()`. - **Upgrade path**: migrate to 0026 (the last naive migration, current `main`) only, write rows the way 5.0.x did (defaults plus JS ISO updates), run all migrations, read back through the drizzle schema. In `America/Los_Angeles`, `created_at` and never-updated `updated_at` equal the transaction's real `now()` (and a `created_at` from January is moved by the standard-time offset, not today's), while JS-written `started_at`/`updated_at`/`resume_at`/snapshot `created_at` are unchanged. On a UTC server the same holds and `pg_relation_filenode` of all 8 tables and their indexes is unchanged except `workflow_invocations_pending`, whose predicate reads `responded_at` (no table rewrite). With rows written in a `TimeZone=Asia/Tokyo` session, `workflow.legacy_timezone=Asia/Tokyo` repairs them (without it they stay 9 h off), set either in the connection's `options` or with `ALTER DATABASE`. - **Runs once**: two `runMigrations` in parallel from the 0025 state (both insert a 0027 row in the migrations table), and the 0027 SQL executed again on a converted database, leave the repaired values where one run put them. Without the lock-and-skip guard both fail by 25,200,000 ms. - **Lock timeout**: with another transaction holding a lock on `workflow_events`, `bootstrap` fails with `55P03` within the 10 s timeout, and succeeds once that transaction ends. - **Reader before the migration**: on an unconverted UTC database with `process.env.TZ = 'America/Los_Angeles'`, `retryAfter`/`startedAt`/`createdAt` read back as the instants written (with drizzle's stock reader `retryAfter` comes back 7 h late). - **Journal order**: every entry's `idx` matches its file number and its `when` is later than the one before, so a duplicate `when` like this branch's pre-merge 0026 fails CI. Results: - Latest head (after merging `main` at ecaa047): `test/timezone.test.ts` 13/13 and the whole package 388/388 on `postgres:15-alpine`; typecheck and Biome clean on changed files; `node scripts/check-changesets.mjs` OK. Breaking the reader alone fails only the pre-migration reader test; restoring the duplicate `when` fails the journal test and every upgrade test. - Earlier heads: on `main`'s schema 8 of the 9 tests fail (off by 25,200,000 ms, 19,800,000 ms and 32,400,000 ms; the UTC no-rewrite test is a guard and passes on both). With this PR: 9/9 on `postgres:15-alpine`, `postgres:12-alpine` and `postgres:18-alpine` (the image the `e2e-local-postgres` job uses), and 9/9 on Homebrew PostgreSQL 14.20 after merging the latest `main`. - After merging `main`: `storage.test.ts`, `retention.test.ts` and `run-creation.test.ts` 185/185 on PostgreSQL 14.20; `pnpm vitest run src` 71/71; `pnpm build` and `pnpm typecheck` clean; Biome clean on changed files; `node scripts/check-changesets.mjs` OK. - CI: the `Unit Tests (ubuntu-latest)` job failed twice on this branch with 330/330 tests passing and two unhandled `57P01` errors attributed to `test/timezone.test.ts` (the teardown race above, now handled in this file and reproduced in isolation with `pg_terminate_backend` on an idle pooled client). `main` currently fails the same job the same way from `test/invoke.test.ts` (e.g. on b5c1bc8), so the job can still go red from the older test files until they get the same listener; that is pre-existing and not touched here. On the latest head the job failed earlier, in `packages/core` (`fanout-dispatch.test.ts` "publishes every queued branch ... in chunks of at most 16": a wall-clock bound, 270 ms against `< 260`), which this PR does not touch; it needs a rerun to show the world-postgres result. - Earlier full-package runs (`pnpm build && pnpm test`, testcontainers): 330/330 on three runs and 329/330 on one. Each failure was a single pre-existing timing test that passes in isolation and is unrelated to this diff: `storage.test.ts` "should release a retained hook after its deadline" (writes a host `Date.now() - 1` and expects the container's `now()` to be past it, on the already-`timestamptz` `token_retention_until`) and `invoke.test.ts` "slow fallback" (3 s budget under parallel load). - A scratch test (not committed) wrote rows with `main`'s storage code on servers in `America/Los_Angeles`, `Asia/Kolkata`, `Europe/London`, `America/St_Johns` (-02:30) and UTC, ran the migrations, then continued the same run with the new code: every run/step/hook/wait/event timestamp is the instant it was written, the event log's `createdAt` stays monotonic across the upgrade, and `main`'s code reading the migrated database from a UTC session gets the same instants. | Page | Preview | | --- | --- | | Postgres World, Installation (migration 0027 callout) | [/worlds/postgres#installation](https://workflow-docs-git-pgp-world-postgres-utc-timestamps.vercel.sh/worlds/postgres#installation) | 1. **`timestamptz` or the keep-`timestamp` fallback?** The fallback: keep `timestamp`, change the defaults to `timezone('utc', now())`, fix the raw `now()` writes in `invocations.ts`, and keep the same repair UPDATE. It stays compatible with old readers in any session zone but leaves the trap for any future `now()` write. This PR prefers the type change. 2. **Repair under lock.** On non-UTC servers the repair UPDATE runs inside the migration while holding ACCESS EXCLUSIVE on the 8 tables, about one rewrite of each table (now documented so operators can plan a window). Acceptable for a one-time migration, or should the repair be batched outside the lock? 3. **Changeset level.** Kept as `patch`, per AGENTS.md (`patch` for bug fixes); the review suggested `minor` given the data-rewriting migration. - `main` shipped its own `0026_add_seq_in_band` (#4821) with the journal `when` this branch's 0026 had, so this migration is now **`0027`**. Without the renumber, databases already at main's 0026 would skip the conversion silently. - **#4761** (`[world-postgres] Scope startup recovery to the World's jobPrefix`) also adds `0027_add_run_job_prefix` with the same `when` as this PR's 0027, and does not have main's 0026 yet. Whichever of the two merges second has to take `0028` and a later `when`; the journal-order test in this PR fails on the clash. - #4756 and #4759 touch world-postgres without migrations and are independent of this one. - [x] 📦 `pnpm changeset` was run to create a changelog for this PR - [x] 🔒 DCO sign-off passes (run `git commit --signoff` on your commits) - [ ] 📝 Ping `@vercel/workflow` in a comment once the PR is ready, and the above checklist is complete 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
2 of 3 tasks
🦋 Changeset detectedLatest commit: 5228a9f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
karthikscale3
approved these changes
Oct 15, 2026
Contributor
Author
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (105 failed)redis (20 failed):
turso (85 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated backport of #4757 to
stable(backport job run).AI recommendation: This is a correctness fix for a bug that also exists on
stable: I verifiedpackages/world-postgres/src/drizzle/schema.tsonorigin/stablestill declares every column with the naivetimestamp(...), so non-UTC servers returncreatedAtvalues off by the UTC offset, which skews the deterministic workflow clock and retry backoffs. The change is apatchchangeset confined to the world-postgres schema, a new migration, tests, and docs/README corrections, with no new feature surface. Note that the migration must be renumbered forstable's journal (which ends well before 0027) and thedocs/content/worlds/path does not exist onstable, so that hunk should be dropped during the cherry-pick.Merge conflicts were resolved by AI (opencode with
anthropic/claude-opus-5). Please review the conflict resolution carefully before merging.