Skip to content

fix(migration): backfill workspace.time_used with current timestamp#26582

Closed
kitlangton wants to merge 1 commit into
devfrom
kit/workspace-time-default-now
Closed

fix(migration): backfill workspace.time_used with current timestamp#26582
kitlangton wants to merge 1 commit into
devfrom
kit/workspace-time-default-now

Conversation

@kitlangton
Copy link
Copy Markdown
Contributor

@kitlangton kitlangton commented May 9, 2026

Summary

  • Existing workspace rows currently get time_used = 0 after the v1.14.43 migration shipped in fix(storage): default workspace time migration #26556, so they sort as the oldest "most recently used" rows forever even when they were in active use before the upgrade.
  • Schema declares time_used: integer().notNull().$default(() => Date.now()), so newly inserted rows via Drizzle get a real timestamp — only the migrated rows are wrong.
  • Ship a new migration 20260509205313_backfill_workspace_time_used that runs UPDATE workspace SET time_used = unixepoch() * 1000 WHERE time_used = 0.

Why a new migration (and not edit the existing one)

Drizzle's bun-sqlite migrator filters getMigrationsToRun by name only, not hash:

function getMigrationsToRun({ localMigrations, dbMigrations }) {
  const dbNamesSet = new Set(dbMigrations.map((m) => m.name).filter((n) => n !== null))
  return localMigrations.filter((lm) => !lm.name || !dbNamesSet.has(lm.name))
}

So editing 20260507164347_add_workspace_time/migration.sql in place is a no-op for any user who already applied the prior DEFAULT 0 version (i.e. anyone on v1.14.43). A fresh migration with a new timestamp is the only path that reaches them.

Why WHERE time_used = 0

  • Users on v1.14.41 or v1.14.42 (broken migration crashed): the ALTER migration runs first with DEFAULT 0, every row is 0, the backfill catches them all.
  • Users on v1.14.43 (DEFAULT 0 fix applied): only the new migration runs, every row is still 0 from the prior backfill, the backfill catches them all.
  • Future / dev: the migration is a no-op for any row that already has a non-zero time_used. Idempotent and self-documenting.

Why unixepoch() * 1000 (not unixepoch('subsec') * 1000)

  • Whole-second precision is sufficient for "most recently used" sort ordering.
  • Avoids the float-cast truncation race that unixepoch('subsec') * 1000 introduces vs JS Date.now().
  • Matches the millisecond-integer convention the rest of the codebase uses.

Verification

  • bun test --timeout 5000 test/storage/workspace-time-migration.test.ts — 2 tests, covers both fresh-upgrade and already-DEFAULT-0 paths.
  • bun typecheck

The schema declares time_used with `$default(() => Date.now())`, so new
inserts via Drizzle get a real millisecond timestamp. The shipped ALTER
migration backfilled existing rows with 0, so any user already on
v1.14.43 has workspace rows stuck at 0 — they would sort as the oldest
'most recently used' rows forever, even when actively used pre-upgrade.

Drizzle's bun-sqlite migrator skips by name only (not hash), so editing
the existing migration file is a no-op for those users. Ship the
backfill as a new migration that updates rows where time_used = 0,
which:
  - covers users coming from any prior state in one pass
  - is idempotent (re-running the migration leaves non-zero rows alone)
  - leaves the existing migration file untouched

Add a regression test for both the fresh-upgrade path and the
already-applied-DEFAULT-0 path.
@kitlangton kitlangton force-pushed the kit/workspace-time-default-now branch from 0f56311 to afa7f8c Compare May 9, 2026 20:54
@kitlangton
Copy link
Copy Markdown
Contributor Author

Closing — on reflection, DEFAULT 0 (shipped in #26556) is the better behavior here.

time_used has exactly one reader: the recent-workspaces sort in dialog-workspace-create.tsx. Backfilling pre-existing rows to unixepoch() * 1000 would cluster every migrated workspace at the top of the recent list with identical timestamps, drowning out real recent activity. Leaving them at 0 lets them surface naturally the next time someone opens a session in them (which bumps time_used to Date.now() via projectors.ts).

The original 'default to now instead of nullable' framing was based on a misread of the shipped fix — the column is already NOT NULL DEFAULT 0, not nullable.

No-op.

@kitlangton kitlangton closed this May 9, 2026
@kitlangton kitlangton deleted the kit/workspace-time-default-now branch May 9, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant