chore: guard against migration version collisions (#16143)
## Description Migrations with hand-written, rounded versions (e.g. `20260925000000`) keep colliding across branches. Git merges two such files cleanly because the filenames differ, and spec CI stays green because it loads the schema instead of running migrations. The failure only shows up on a real `db:migrate`: a `DuplicateMigrationVersionError`, or a database that already recorded one branch's migration silently skipping the other. This adds: - A `Migration Version Check` workflow on PRs that touch `db/migrate`. It fails when a migration added or renamed in the PR has a version ending in `0000`, and when two files in `db/migrate` share a version. Existing migrations are not flagged. - An `AGENTS.md` rule to create migrations with `rails g migration` instead of writing the version by hand. Linear: https://linear.app/chatwoot/issue/INF-118 ## Type of change CI and contributor guidelines only, no app code changes. ## How Has This Been Tested? Ran both steps locally against simulated PR merge commits on top of develop: a generated timestamp passes, `...000000` and `...090000` versions fail, renaming a migration to a rounded version fails, and a duplicate of an existing version fails. Develop as-is passes both checks. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
This commit is contained in:
41
.github/workflows/migration_version_check.yml
vendored
Normal file
41
.github/workflows/migration_version_check.yml
vendored
Normal file
@@ -0,0 +1,41 @@
|
||||
name: Migration Version Check
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
paths:
|
||||
- 'db/migrate/**'
|
||||
|
||||
concurrency:
|
||||
group: pr-${{ github.workflow }}-${{ github.head_ref }}
|
||||
cancel-in-progress: true
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
migration_version_check:
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 2
|
||||
|
||||
- name: Reject hand-written migration versions
|
||||
run: |
|
||||
rounded=$(git diff --name-only --diff-filter=AR HEAD^1 HEAD -- db/migrate | grep -E '^db/migrate/[0-9]{10}0000_' || true)
|
||||
if [ -n "$rounded" ]; then
|
||||
echo "::error::Rounded migration versions collide across branches. Regenerate with 'bundle exec rails g migration':"
|
||||
echo "$rounded"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Reject duplicate migration versions
|
||||
run: |
|
||||
duplicates=$(ls db/migrate | cut -d_ -f1 | sort | uniq -d)
|
||||
if [ -n "$duplicates" ]; then
|
||||
echo "::error::Duplicate migration versions break db:migrate:"
|
||||
echo "$duplicates"
|
||||
exit 1
|
||||
fi
|
||||
@@ -44,6 +44,7 @@
|
||||
## General Guidelines
|
||||
|
||||
- **Database foreign keys**: Prefer Rails associations and callbacks over database foreign-key constraints. Add a database foreign key only when there is a clear need and after checking deletion behavior and production migration/locking risks. Existing foreign keys alone are not a reason to add new ones.
|
||||
- **Migrations**: Create migrations with `bundle exec rails g migration <Name>` so the version is a real timestamp. Never hand-write or round the version (e.g. `20260925000000`); rounded versions collide across branches and break `db:migrate`.
|
||||
- Prefer the smallest production-ready change that solves the current problem.
|
||||
- Build for the expected production path first. Do not add speculative guards, fallbacks, retries, or edge-case handling unless the caller can actually hit that case or production has proven it necessary.
|
||||
- Enforce eligibility and exclusivity rules at the earliest shared entry point. Do not repeat backup guards across downstream jobs, callbacks, services, or writes unless a proven independent path bypasses that point.
|
||||
|
||||
Reference in New Issue
Block a user