Skip to content

Commit 7dd2b66

Browse files
committed
ci(migrations): add migration safety linter
Adds a custom Python AST-based linter that fails CI on PRs introducing the four anti-patterns the migration runtime defaults cannot reliably catch on a quiet hour: * op.create_index (and raw CREATE INDEX) without CONCURRENTLY, or with CONCURRENTLY but outside an autocommit_block(). * op.create_foreign_key (and raw ADD CONSTRAINT FOREIGN KEY) without NOT VALID. * op.execute("UPDATE ...") / DELETE in-band data backfills. * SET / RESET of lock_timeout, statement_timeout, or idle_in_transaction_session_timeout inside a migration file. Each rule maps to a documented anti-pattern in CLAUDE.md. Why custom AST instead of squawk: squawk operates on rendered SQL, which loses the surrounding Alembic context (autocommit_block wrapping, kwarg options on op.create_index/op.create_foreign_key) — exactly the signals needed to distinguish safe and unsafe shapes here. AST inspection of the migration source is tighter, has zero external deps, runs in milliseconds, and lets each rule produce a message that points at the correct fix. Escape hatch: a `# migration-unsafe-ack: <one-line reason>` directive at the top of the migration file suppresses all rules for that file. The GitHub Actions workflow additionally checks for a `migration-unsafe-ack` PR label; when present it runs the linter in --warn-only mode so violations surface in logs without blocking merge. Both signals are required to deploy a migration that intentionally overrides the runner defaults. The CI step lints only files changed in the PR (via `--base-ref origin/<base>`), so existing migrations are not retro-flagged. Includes 12 unit tests covering each rule, the ack-directive escape hatch (including that bare directives without a reason do not suppress), and the autocommit_block detection.
1 parent b3ea63a commit 7dd2b66

6 files changed

Lines changed: 923 additions & 18 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
name: Migration Safety Lint
2+
3+
on:
4+
pull_request:
5+
branches: [main]
6+
paths:
7+
- 'agentex/database/migrations/alembic/versions/**.py'
8+
- 'agentex/scripts/ci_tools/migration_lint.py'
9+
- '.github/workflows/migration-lint.yml'
10+
11+
permissions:
12+
contents: read
13+
14+
jobs:
15+
lint-migrations:
16+
name: 'Lint changed migrations'
17+
runs-on: ubuntu-latest
18+
steps:
19+
- name: Checkout
20+
uses: actions/checkout@v4
21+
with:
22+
# Need history back to the merge base so --base works.
23+
fetch-depth: 0
24+
25+
- name: Set up Python
26+
uses: actions/setup-python@v5
27+
with:
28+
python-version: '3.12'
29+
30+
- name: Detect migration-unsafe-ack label
31+
id: ack_label
32+
env:
33+
LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}
34+
run: |
35+
if echo "$LABELS" | grep -q '"migration-unsafe-ack"'; then
36+
echo "ack=1" >> "$GITHUB_OUTPUT"
37+
echo "::notice title=migration-unsafe-ack label present::Linter will not fail the build, findings will surface as warnings only."
38+
else
39+
echo "ack=0" >> "$GITHUB_OUTPUT"
40+
fi
41+
42+
- name: Lint migrations
43+
env:
44+
MIGRATION_UNSAFE_ACK: ${{ steps.ack_label.outputs.ack }}
45+
BASE_REF: ${{ github.event.pull_request.base.ref }}
46+
run: |
47+
set -euo pipefail
48+
# Make the merge base reachable so --base diff works.
49+
git fetch --no-tags --depth=1 origin "$BASE_REF"
50+
python3 agentex/scripts/ci_tools/migration_lint.py --base "origin/$BASE_REF"

.pre-commit-config.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,9 @@ repos:
2525
files: ^agentex/(src/.*|scripts/generate_openapi_spec\.py)$
2626
pass_filenames: false
2727

28+
- id: migration-safety-lint
29+
name: 'lint Alembic migrations for dangerous Postgres patterns'
30+
language: system
31+
entry: agentex/scripts/git_hooks/migration_safety_lint.sh
32+
files: ^agentex/database/migrations/alembic/versions/.*\.py$
33+

0 commit comments

Comments
 (0)