Skip to content

fix(db): timestamp columns store a real ISO value on omit, not the literal string - #656

Merged
JSONbored merged 1 commit into
mainfrom
fix/timestamp-defaults
Jun 13, 2026
Merged

fix(db): timestamp columns store a real ISO value on omit, not the literal string#656
JSONbored merged 1 commit into
mainfrom
fix/timestamp-defaults

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

The bug

Found while investigating the gate hang: prod webhook_events.received_at contained the literal string "CURRENT_TIMESTAMP" for a range of rows, which breaks ordering and time-range queries on that table.

Root cause: drizzle's .default("CURRENT_TIMESTAMP") is a static default — drizzle applies it client-side and injects the literal string "CURRENT_TIMESTAMP" into any insert that omits the column. It never reaches SQLite's CURRENT_TIMESTAMP function (the migration DDL, e.g. migrations/0001_initial.sql, does use a valid DEFAULT CURRENT_TIMESTAMP, but drizzle pre-empts it). So every timestamp column that an insert omits gets the bad literal instead of a timestamp. This pattern is used on 59 columns across the schema.

The fix

Replace every .default("CURRENT_TIMESTAMP") with .$defaultFn(() => nowIso()), so drizzle injects a real ISO-8601 timestamp on omit — consistent with the nowIso() value every explicit insert already uses.

  • App-layer only, no migration. Prod column DDL already has a valid default; this only stops drizzle from injecting the bad literal. Inserts that already set timestamps explicitly are unaffected (explicit values override the default fn).
  • Confirmed nothing in src/test depends on the literal string; there is no drizzle schema-drift check in CI.

Tests

Behavioral tests insert rows omitting receivedAt / createdAt / updatedAt and assert the stored values are ISO-8601 and never "CURRENT_TIMESTAMP". Full suite green; coverage holds above the 97% gate; Workers-runtime tests pass.

Note

Existing historical rows still hold the literal (fixing those needs a backfill/table rebuild on the ~2.8GB prod D1 — out of scope here). This stops new bad writes.

Separate from #652 (AI review/BYOK) and #655 (gate finalize-on-error); all three came out of the same gate-hang investigation.

…he literal string

drizzle's `.default("CURRENT_TIMESTAMP")` is a STATIC default: drizzle applies
it client-side and injects the literal string "CURRENT_TIMESTAMP" into any
insert that omits the column (it never reaches SQLite's CURRENT_TIMESTAMP
function, even though the migration DDL uses the real default). This corrupted
timestamp columns on omit — observed in prod on webhook_events.received_at,
which broke ordering/time-range queries on that table.

Replace all 59 `.default("CURRENT_TIMESTAMP")` with `.$defaultFn(() => nowIso())`
so an omitted timestamp column gets a real ISO-8601 value consistent with every
explicit nowIso() insert. App-layer only — no migration, prod column DDL already
uses a valid default; this just stops drizzle from injecting the bad literal.

Behavioral tests confirm omitted receivedAt/createdAt/updatedAt now store ISO
timestamps, never "CURRENT_TIMESTAMP".
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
This PR replaces static string defaults for timestamp columns with a function that injects a real ISO‑8601 timestamp, fixing previously stored literal "CURRENT_TIMESTAMP" values. It adds a utility import and a focused test suite confirming the behavior.

Suggestions

  • Verify that nowIso returns a UTC ISO string and is exported from the correct path.
  • Consider adding a quick test for one of the other tables to ensure the change is consistent across all timestamp columns.
  • Run the full test suite to confirm no downstream code relied on the literal "CURRENT_TIMESTAMP" values.

Worth double-checking

  • If any existing data relied on the literal "CURRENT_TIMESTAMP" string, queries or migrations may need adjustment.
  • Ensure the new import path does not introduce circular dependencies in the DB layer.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR replaces static string defaults 'CURRENT_TIMESTAMP' with drizzle $defaultFn(() => nowIso()) for all timestamp columns in the database schema. This ensures inserts that omit timestamp columns store real ISO-8601 timestamps instead of the literal string 'CURRENT_TIMESTAMP', fixing a corruption issue. A new test verifies the fix for webhookEvents.receivedAt and repositorySettings timestamps. The change is consistent, well-scoped, and addresses the stated problem without side effects.

No blocking issues spotted.

@ghost ghost added the gittensory-review label Jun 13, 2026
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #656 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 13, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

1 similar comment
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored
JSONbored merged commit cd6e037 into main Jun 13, 2026
10 checks passed
@JSONbored
JSONbored deleted the fix/timestamp-defaults branch June 13, 2026 23:10
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant