Skip to content

ci(migrations): guardrail against table-locking DDL in new migrations (JEF-592) - #149

Merged
thejefflarson merged 1 commit into
mainfrom
thejefflarson/jef-592-ci-guardrail-block-table-locking-ddl-in-migrations
Jul 28, 2026
Merged

ci(migrations): guardrail against table-locking DDL in new migrations (JEF-592)#149
thejefflarson merged 1 commit into
mainfrom
thejefflarson/jef-592-ci-guardrail-block-table-locking-ddl-in-migrations

Conversation

@thejefflarson

Copy link
Copy Markdown
Owner

Summary

Migration 0017 shipped a plain (non-CONCURRENTLY) CREATE INDEX on
metric_series_rollups — at the time a 10.4M-row hot table — with nothing in
CI flagging it as risky. This adds a CI lint that fails on that pattern (and a
couple of other table-locking-DDL shapes) in any newly-added migration.

  • New: scripts/lint-migrations.sh (POSIX sh + awk/grep/sed, no
    new dependency) checks every server/migrations/*.sql file for:
    1. CREATE INDEX / DROP INDEX without CONCURRENTLY.
    2. ALTER TABLE ... ADD COLUMN ... NOT NULL DEFAULT ... (table rewrite).
    3. A multi-statement file that also carries a -- no-transaction comment
      (sqlx only supports one statement per no-transaction migration).
      Escape hatch: a -- lock-ok:<reason> comment anywhere in the file allows it
      (e.g. an index on a table created earlier in the same migration, still
      empty).
  • New: .github/workflows/ci.yml gets a migrations-lint job that runs
    the script on every push/PR.
  • Grandfathering, without editing any existing migration file:
    server/migrations/.locklint-baseline records 17. Migrations at or below
    that version are skipped entirely — sqlx checksums applied migrations, so
    editing 0017 to add a -- lock-ok comment would crashloop prod on next
    boot (the whole reason for this ticket). Only migrations added after the
    baseline get linted; new migrations get no free pass.
  • New: server/migrations/README.md documents the rules, the escape
    hatch, the grandfather mechanism, the one-statement-per--- no-transaction-
    file rule, and points heavy/online DDL against spans/logs/
    metric_series_rollups at the JEF-580 online-DDL lane (ADR 0021) instead of
    a boot migration. CLAUDE.md gets a one-line pointer to it.

How the lint runs / grandfather mechanism

scripts/lint-migrations.sh [migrations-dir] — defaults to server/migrations.
For each *.sql file with a numeric prefix strictly above
.locklint-baseline's value, it strips -- comments, splits on ;, and
checks each statement against the three rules above (a per-file
-- lock-ok: comment short-circuits all three).

Test plan

  • scripts/lint-migrations.sh --selftest — a self-contained fixture suite
    (no real bad migration ever committed) proving: a new plain CREATE INDEX
    is flagged, CONCURRENTLY is accepted, -- lock-ok: escapes a flagged
    statement, ADD COLUMN ... NOT NULL DEFAULT is flagged, a nullable
    ADD COLUMN is not, a 2-statement -- no-transaction file is flagged, a
    1-statement one is not, and a migration at/below the baseline (which would
    otherwise trip rule 1) is skipped. All 9 assertions pass.
  • scripts/lint-migrations.sh server/migrations — passes clean on the
    current tree (0017 grandfathered, unedited).
  • Manually verified (throwaway, not committed) that a synthetic 0018_*.sql
    with a plain CREATE INDEX or an ADD COLUMN ... NOT NULL DEFAULT against
    the current tree's baseline correctly fails with exit 1.
  • shellcheck -s sh scripts/lint-migrations.sh — clean.
  • Verified the script's control flow (the <<EOF $(...) EOF heredoc pattern
    used to avoid a subshell-scoped loop variable) under both bash's /bin/sh
    and dash directly.
  • actionlint .github/workflows/ci.yml — same pre-existing findings as on
    main (unknown custom runner label, shellcheck notes in the unrelated
    image job); no new findings from this change.
  • No Rust or UI files touched, so no cargo/npm gates apply to this change.

Closes JEF-592.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

… (JEF-592)

Migration 0017 shipped a plain (non-CONCURRENTLY) CREATE INDEX on a
10.4M-row hot table with nothing in CI flagging it as risky. Add
scripts/lint-migrations.sh, run as a new CI job, that fails on:
non-CONCURRENTLY CREATE/DROP INDEX, ADD COLUMN ... NOT NULL DEFAULT
(table rewrite), and a multi-statement file that also carries a
`-- no-transaction` comment (sqlx only supports one statement per
no-transaction migration). A `-- lock-ok:<reason>` comment escapes a
genuinely safe case.

Migrations at or below server/migrations/.locklint-baseline (17) are
grandfathered without editing them -- sqlx checksums applied
migrations, so touching 0017 to add an escape comment would crashloop
prod. Only migrations added after the baseline are linted.

Documented the rules and escape hatch in a new
server/migrations/README.md (with a CLAUDE.md pointer), including
where heavy/online DDL against spans/logs/metric_series_rollups
should go instead (the JEF-580 online-DDL lane, ADR 0021).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@thejefflarson
thejefflarson merged commit b647327 into main Jul 28, 2026
5 checks passed
@thejefflarson
thejefflarson deleted the thejefflarson/jef-592-ci-guardrail-block-table-locking-ddl-in-migrations branch July 28, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant