Skip to content

ci(migrations): close the parallel-merge gap in duplicate-number detection#3288

Merged
bokelley merged 1 commit into
mainfrom
bokelley/migration-dup-check
Apr 26, 2026
Merged

ci(migrations): close the parallel-merge gap in duplicate-number detection#3288
bokelley merged 1 commit into
mainfrom
bokelley/migration-dup-check

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

The "No duplicate migration numbers" workflow had a known gap: two PRs whose checks ran in parallel against different snapshots of main could both pass legitimately. That's how #3235 + #3244 both landed with migration 433 (and #2793 + #2800 with 419 in February).

Three layered fixes:

  1. merge_group trigger — runs the check on the real merge commit when the GitHub merge queue is enabled. The only way to fully serialize.

  2. Daily scheduled run as a safety net for when merge queue isn't on. If a duplicate slips through, the workflow goes red on main within 24 hours; branch protection then blocks subsequent merges until the duplicate is renumbered.

  3. Filesystem invariant test in server/tests/unit/migrate.test.ts that scans the migrations directory and asserts no duplicate version numbers / malformed filenames. Runs locally on npm test:unit and on every CI run, regardless of workflow timing — catches duplicates the moment a developer's working tree has them.

The error message is also clearer about the rebase-and-renumber recovery procedure, including the IF NOT EXISTS requirement so re-running on systems that already applied the old number is a no-op.

What this doesn't fix

The truly bulletproof fix is branch protection: require branches up-to-date before merging OR enable the merge queue. Both are GitHub configuration, not workflow code. The workflow comments document this so the next person who hits the gap knows to check the config.

Tests

  • New migrations directory invariant test in server/tests/unit/migrate.test.ts (12 → 14 tests). Reads the actual migrations directory at test time and asserts no duplicates.

Test plan

  • npm run typecheck clean
  • 14/14 unit tests pass
  • CI green
  • After merge: turn on the daily-cron job and verify it fires

🤖 Generated with Claude Code

…ction

The pull_request workflow correctly catches a stale-view-of-main case
when a single PR runs against an out-of-date base, but two PRs whose
checks run in parallel against different snapshots can both pass
legitimately. That's how #3235 + #3244 both landed with migration 433
(and #2793 + #2800 with 419 in February).

Three layered fixes:

1. merge_group trigger — runs the check on the real merge commit when
   the GitHub merge queue is enabled. Only way to fully serialize.

2. Daily scheduled run as a safety net. If a duplicate slips through,
   the workflow goes red on main within 24 hours and branch protection
   blocks subsequent merges until the duplicate is renumbered.

3. Filesystem invariant test in server/tests/unit/migrate.test.ts that
   scans the migrations directory and asserts no duplicate version
   numbers / malformed filenames. Runs locally on npm test:unit and on
   every CI run regardless of workflow timing.

Error message also clearer about the rebase-and-renumber recovery
procedure, including the IF NOT EXISTS requirement so re-running on
systems that already applied the old number is a no-op.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit a5da5dc into main Apr 26, 2026
11 of 12 checks passed
@bokelley bokelley deleted the bokelley/migration-dup-check branch April 26, 2026 20:21
bokelley added a commit that referenced this pull request Apr 26, 2026
… 436 → 438 (#3298)

Hotfix — two PRs landed migration 436 in parallel:
  - 436_addie_prompt_telemetry.sql (separate PR)
  - 436_organization_membership_provisioning_source.sql (#3295)

Deploy preflight blocks main because the migrate runner crashes on
duplicate version numbers.

Renumbers mine to 438 (437 is auto_provision_digest_sent_at). The
migration is fully idempotent (IF NOT EXISTS on every ALTER and
CREATE INDEX) so re-running it on systems that already applied as
436 is a no-op.

Same recovery shape as the catalog/auto-provision 433 collision in
the previous cycle. The CI fix landing via #3288 plugs the
parallel-merge gap for future PRs but can't retroactively prevent
this one.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 26, 2026
)

PRs #3282 and #3295 both shipped a `436_*.sql` migration in parallel
before #3288 (the duplicate-detection preflight) landed. The collision
blocks every subsequent deploy at the Preflight check #3288 added.
Main is wedged; prod is currently down because the in-flight
GCP-KMS-eager-init deploy (already reverted in #3297) couldn't be
followed up by the hotfix deploy due to the migration block.

Renames the later-merged of the two duplicates:
- 436_organization_membership_provisioning_source.sql → 438_*

Both migrations use `ADD COLUMN IF NOT EXISTS` / `CREATE INDEX IF NOT
EXISTS` so the rename is safe across:
- prod (release_command never succeeded with the duplicate present;
  applies 438 fresh)
- dev DBs that already pulled the original 436 (re-runs idempotently
  as 438; original schema_migrations row at version=436 stays
  orphaned but harmless)

Updates the matching reference in
.changeset/provisioning-source-attribution.md.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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