MemberJunction / MemberJunction/MJ

Migration gates in changes.yml are forward-only: anything merged to next before the gate existed is unscanned until a release PR

Open
#4,384 0 comments 0 reactions 1 assignee Claimed by @SDesai-BC View on GitHub
bug priority: high
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

Found while cutting v6.1.0-edge.6. It cost a release-day fix (#4378) that could have been caught a week earlier.

## What happened

`changes.yml` resolves its diff base from the PR's own base branch:

```yaml
git fetch --no-tags --depth=1 origin ${{ github.event.pull_request.base.ref }}
CHANGED_FILES=$(git diff --name-only ${{ steps.base.outputs.sha }} ${{ github.sha }})
```

That is correct for attribution — a PR is only blamed for what it adds. But it produces a blind spot:

- **PR into `next`** → base is `next`. A migration already on `next` is on the base, so it is **not in the diff and never scanned**.
- **PR into `main`** → base is `main`. The gate sees **every** migration in the release at once, including ones merged weeks ago.

So a migration gate added on day N **never inspects anything merged before day N** — until a release PR, where it fails for the first time, on release day, on someone else's code.

## The concrete instance

`migrations/v6/V202609031400__v6.1.x__Conversation_Scoped_Skill_Activation.sql` carried 12 `EntityField` INSERTs with a literal `Sequence`.

| | |
|---|---|
| Migration merged to `next` | **2026-09-03** |
| `check-migration-entityfield-sequence.mjs` added (#4292) | **2026-09-08** |
| First time any gate saw it | while preparing the edge.6 release PR |

Five days apart. No PR into `next` could ever have flagged it. I only found it because I ran the gate locally against `origin/main...origin/next` **before** cutting — otherwise the release PR would have gone red on first contact.

Fixed in #4378.

## Why this will recur

A second gate landed the same week — `check-migration-no-prune.mjs` (#4361), wired into the same job. It has exactly the same blind spot for anything merged before **2026-09-10**. I ran it against `main...next` and it passes today, so there is no backlog *for that one* right now — but the structural problem stands, and the next gate added will inherit it.

## What to do

**1. Sweep the existing backlog (one-off).** Run every `changes.yml` migration gate across all of `migrations/` on `next`, not just a PR diff, and fix or accept what turns up. I checked the sequence gate for the edge.6 range and `V202609031400` was the only file — but that only covers `main..next`, not the full tree.

**2. Add a scheduled full-tree run (ongoing).** The gates are fast (the sequence one is seconds; `check-codegen-tail.mjs` is ~2s over 660+ migrations) and need no database. A nightly or weekly run over the whole `migrations/` tree would surface this class the day a gate lands rather than on release day. It should report, not block, so it never wedges unrelated PRs.

**3. Until then, add to DEPLOYMENT.md Step 9** — before cutting the release branch, run the `changes.yml` migration gates locally against `origin/main...origin/next`:

```bash
BASE=$(git rev-parse origin/main); HEAD=$(git rev-parse origin/next)
node .github/scripts/check-migration-entityfield-sequence.mjs "$BASE" "$HEAD"
node .github/scripts/check-migration-no-prune.mjs "$BASE" "$HEAD"
node .github/scripts/check-codegen-tail.mjs
```

> Check the real exit code — piping these to `tail` masks it, which briefly fooled me: `EXIT=$?` after a pipe is `tail`'s status, not the script's.

Step 9 already warns *"A migration authored weeks ago can therefore fail here for the first time — do not assume a failure here is yours."* This makes that warning actionable instead of just a heads-up.

## Acceptance

- Existing migrations on `next` have been swept by every current gate
- A new migration gate surfaces pre-existing violations without waiting for a release PR
- DEPLOYMENT.md Step 9 tells the build engineer to pre-clear the gates before cutting

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.