Skip to content

fix: add migration safety guards#1577

Open
hiddeco wants to merge 1 commit intomainfrom
migration-safety-improvements
Open

fix: add migration safety guards#1577
hiddeco wants to merge 1 commit intomainfrom
migration-safety-improvements

Conversation

@hiddeco
Copy link
Collaborator

@hiddeco hiddeco commented Mar 9, 2026

Gate entrypoint migrations behind RUN_ALEMBIC_MIGRATIONS with a case-insensitive check matching Pydantic's bool coercion. Set a 10s lock_timeout in Alembic's online migration path to fail fast under contention. Add a DDL safety checklist to the migration template so authors consider locking, concurrent indexing, and expand-contract patterns.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/alembic/script.py.mako">

<violation number="1" location="backend/alembic/script.py.mako:18">
P1: Custom agent: **Check for Cursor Rules Drift**

Rule "Check for Cursor Rules Drift" is violated: this PR adds new Alembic migration safety conventions, but the relevant Cursor rule (`.cursor/rules/backend-rules.mdc`) was not updated to include them.</violation>

<violation number="2" location="backend/alembic/script.py.mako:30">
P2: The checklist incorrectly labels `now()` as a volatile default that rewrites the table; use a truly volatile example like `clock_timestamp()`.</violation>
</file>

<file name="backend/entrypoint.dev.sh">

<violation number="1" location="backend/entrypoint.dev.sh:25">
P2: The migration gate only treats `"true"` as enabled, so valid truthy values like `1`, `on`, or `yes` will incorrectly disable migrations.</violation>
</file>

<file name="backend/entrypoint.sh">

<violation number="1" location="backend/entrypoint.sh:50">
P1: The migration gate only accepts `"true"`; other common truthy values (e.g. `1`, `yes`, `on`) will incorrectly skip migrations.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@@ -15,6 +15,25 @@ down_revision = ${repr(down_revision)}
branch_labels = ${repr(branch_labels)}
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Custom agent: Check for Cursor Rules Drift

Rule "Check for Cursor Rules Drift" is violated: this PR adds new Alembic migration safety conventions, but the relevant Cursor rule (.cursor/rules/backend-rules.mdc) was not updated to include them.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/alembic/script.py.mako, line 18:

<comment>Rule "Check for Cursor Rules Drift" is violated: this PR adds new Alembic migration safety conventions, but the relevant Cursor rule (`.cursor/rules/backend-rules.mdc`) was not updated to include them.</comment>

<file context>
@@ -15,6 +15,25 @@ down_revision = ${repr(down_revision)}
 branch_labels = ${repr(branch_labels)}
 depends_on = ${repr(depends_on)}
 
+# --- DDL safety checklist (remove before committing) ---
+#
+# [ ] lock_timeout: env.py sets a 10s default. Override per-statement
</file context>
Fix with Cubic

Gate entrypoint migrations behind
`RUN_ALEMBIC_MIGRATIONS` with a case-insensitive
check matching Pydantic's bool coercion. Set a 10s
`lock_timeout` in Alembic's online migration path to
fail fast under contention. Add a DDL safety checklist
to the migration template so authors consider locking,
concurrent indexing, and expand-contract patterns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:database Database schema, migrations, and data models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant