MemberJunction / MemberJunction/MJ
Gate 5 (CDP): upgrade to 6.1.1 fails in Metadata_Sync migrations on databases that ran `mj sync push` before migrating
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
Found during gate 5 (CDP upgrade matrix) of #4475, certifying 6.1.1. Filed for a cert-owner ruling; see **Ruling needed** at the bottom.
## Summary
Every `*__Metadata_Sync.sql` migration in the 6.1.x set replays its record creations as **unguarded** `spCreate @ID = ''`. Those same GUIDs are hardcoded `primaryKey` values in `metadata/**/*.json`, and `metadata/.mj-sync.json` sets `push.autoCreateMissingRecords: true` — so `mj sync push` creates the identical rows, with the identical IDs, on whatever database it is pointed at.
Two shipped mechanisms therefore both claim to create the same row, and the migration never checks whether the push got there first. On any database where a push ran ahead of the migration chain, the migration dies on a primary-key violation.
CDP stage and CDP production are both in this state. First failure:
```
Migration: migrations/v6/V202608080752__v6.1.x__Metadata_Sync.sql
Failed at batch 1 of 102 (lines 1-35)
Violation of PRIMARY KEY constraint 'PK__Credenti__3214EC27...'.
Cannot insert duplicate key in object '__mj.CredentialType'.
The duplicate key value is (82dff26b-2abb-4a69-8718-1fe550b60816).
```
The row it tries to insert is **content-identical** to the row already present. Nothing is wrong with the data; the migration simply inserts without looking.
No flyway history row is written, so the run stops cleanly and is retryable.
## Reproduction
Fully reproduced from repo artifacts on a private database — no CDP data involved.
```bash
# 1. Build a database from repo migrations only, stopping at the migration
# immediately before the failure (V202608080201).
mj migrate --dir 202608080752 removed>
# 2. CONTROL — apply the failing migration with no push. PASSES.
mj migrate --dir
# 3. TEST — on a clone of the step-1 state, run the documented push first.
mj sync push --dir=metadata --include="credential-types" --ci
# -> "2 created", inserts 82DFF26B-2ABB-4A69-8718-1FE550B60816
# 4. Then migrate. FAILS, byte-identical to CDP:
mj migrate --dir
# -> Failed at batch 1/102 (lines 1-35), PK violation, same key value
```
The control arm is the important half: **the migration chain alone never collides.** Nothing in the v5→v6.1.1 path double-creates this row (I also checked for cross-file duplicate `spCreate` across all of `migrations/` — the only hits are v2-era and not on the upgrade path). The trigger is `mj sync push`, which is documented workflow.
Post-failure state matches CDP exactly: `flyway_schema_history` has no row for `202608080752`, and `CredentialType` is unchanged at 18 rows — batch 1 aborts the file with no partial application.
## Root cause
1. `metadata/credential-types/.credential-types.json` pins `primaryKey.ID = 82DFF26B-…`. `metadata/.mj-sync.json` sets `push.autoCreateMissingRecords: true`. In `packages/MetadataSync/src/services/PushService.ts:1100-1135`, a record whose primary key is absent goes to `NewRecord()` + `Set(pkField, pkValue)` — creating the row **with the canonical GUID**.
2. The release `Metadata_Sync` migration is a *recording* of that same push, captured through `packages/GenericDatabaseProvider/src/SqlLogger.ts` (the `simpleSQLFallback` path, lines 143-147, which is what produces the `(core SP call only)` blocks). It replays the create as an unconditional `spCreate`.
The migration encodes a precondition it never states or verifies — *this row is absent* — true only on the clean DB at the last released version that `metadata/CLAUDE.md` rule 1b mandates for generation. Any database where a push ran ahead of migrations violates it.
**This is not a build-engineer error.** All 8 files carry `-- Description: MetadataSync push operation` with distinct session IDs — genuine tool output, cut one per edge release (edge.1 through edge.7), not hand-authored. **0 of 567** fixed-GUID `spCreate*` calls in `migrations/v6` are guarded. Multiple people generated these across five weeks and every run produced the identical unguarded shape. The emitter has no other output form.
## Scope — who is and is not affected
| Situation | Affected? | Evidence |
|---|---|---|
| Fresh `mj install` | **No** | Gate 4 on 6.1.1: sync push after migration was a clean no-op, 89/89 directories, 14,218 records, both macOS and Windows |
| Gate 2 CI | **No** | That workflow migrates from scratch *then* runs `mj sync push --ci` — correct order |
| Upgrade with no push before migrating | **No** | Control arm above |
| Upgrade where a push ran first | **Yes** | CDP stage + production |
Narrower than #4483, which triggered on a data condition nobody controls (did you ever use Reports). This triggers on **operator sequencing**. But for an already-pushed database there is no ordering workaround left — you cannot un-push, and both CDP environments already hold the rows.
## Blast radius
Measured against the `v6.1.1` tag tree, not `next`. Counting unguarded fixed-GUID `spCreate*` calls whose GUID is a `primaryKey` of a record shipped in `metadata/`:
- **567** fixed-GUID `spCreate*` calls in `migrations/v6`; **0 guarded**
- **190** collide with metadata-shipped primary keys, across **8** migrations
| Migration | Collisions | Status for CDP |
|---|---|---|
| `V202608051834` | 2 | already applied |
| `V202608080752` | 22 | **failing now, at the 1st** |
| `V202608112251` | 15 | ahead |
| `V202608202231` | 22 | ahead |
| `V202608260834` | 1 | ahead |
| `V202609011700` | 16 | ahead |
| `V202609101740` | 11 | ahead |
| `V202609132006` | 101 | ahead |
**188 remain ahead of CDP.** Deleting the one Azure Blob row and re-running produces the same error on the next one, 188 times. Any fix sized to the single reported row is wrong.
Note the count is also a consequence of 8 sync migrations existing where `metadata/CLAUDE.md` rule 1b mandates one consolidated migration per release. The per-edge cadence multiplied the surface — but a single consolidated file would fail identically, at the first pushed row. File count is blast radius, not cause.
## Proposed fix
### Layer 1 — the emitter (root cause, prospective)
Emit every fixed-GUID create as create-or-update:
```sql
IF NOT EXISTS (SELECT 1 FROM [${flyway:defaultSchema}].[CredentialType] WHERE ID = @ID_x)
EXEC [${flyway:defaultSchema}].spCreateCredentialType @ID = @ID_x, ...;
ELSE
EXEC [${flyway:defaultSchema}].spUpdateCredentialType @ID = @ID_x, ...;
```
`spCreate` and `spUpdate` have **byte-identical parameter lists** (verified on `CredentialType`: both take `@ID, @Name, @Description_Clear, @Description, @Category, @FieldSchema, @IconClass_Clear, @IconClass, @ValidationEndpoint_Clear, @ValidationEndpoint`). CodeGen generates both from the same column set, so this holds generally — worth asserting in a test rather than assuming. The table name is the SP-name suffix, and `GetCreateUpdateSPName` in `packages/MJCore/src/generic/databaseProviderBase.ts:1978` already resolves both names from the same `EntityInfo`.
The emit seam is the `simpleSQLFallback` string, substituted at `SqlLogger.ts:143-147` and constructed in the SQLServerDataProvider save path (`SQLServerDataProvider.ts:1884`, `:1912`). Implementer should confirm the exact construction site — I traced the substitution, not the full build.
This converges instead of skipping: a database that pushed older metadata still receives the release's content. It is one change in the emitter, zero per release, and it fixes the class permanently.
### Layer 2 — the 190 already shipped
Constrained by Flyway checksum validation. `flyway_schema_history.checksum` is populated (verified), so **editing an already-shipped migration breaks `validate` on every database that already applied it** — including the gate 4 installs from 6.1.1. Two options:
- **(a) Regenerate the 8 files with guards, plus a repair step.** For CDP that is a one-file repair (`V202608051834` is the only affected file already applied). Fleet-wide it needs release-note guidance. Note `mj migrate` has no `repair` subcommand today — only `convert`, `create`, `rebake` — so that capability needs adding or doing by hand.
- **(b) Leave shipped files alone; one-time reconciliation before migrating**, deleting the push-created rows the pending migrations will create. Riskier: on a CDP that actually uses Azure Blob storage a `Credential` row may reference that `CredentialType`, so this needs a dependent check per row across 188 rows on production.
Recommend (a) — it converges rather than deletes, and avoids reasoning about FK dependents on live data.
### Verification already done
Prototyped the Layer 1 transform against `V202608080752` (47 create batches guarded). On the collided database it applied cleanly, wrote its history row, and the resulting `CredentialType` table hashes **identical** to the clean-install control (`02989E2B9FD3E3FF180730437AD0DEBCCA7887C0A606B2AE377D455E109A9209` on both). Nothing overwritten, nothing lost, rows already correct left correct.
Whatever fix is chosen needs a regression test that CI does not currently have: **migrate partway, `mj sync push`, then finish migrating.** Gate 2 only ever pushes after migrating, which is precisely why this is invisible at build time.
## Ruling needed
Gate 5 cannot pass for CDP on 6.1.1 regardless of label — that is a fact, not a judgment. The open question is whether this clears the post-batch bar, which #4475 sets at **showstopper only** from soak start (batch closed 2026-09-15 04:14 UTC).
Arguments against a 6.1.2 respin: fresh installs are clean, gate 2 CI is clean, the trigger is operator sequencing rather than a data condition, and the Layer 2 fix is migration-history surgery that is safer in 6.2 than in a rushed respin.
Suggested disposition if not ruled a showstopper — **recorded, does not block**, alongside #4495:
1. Layer 1 emitter fix on `next`, ships in 6.2.
2. Document the 6.1 LTS upgrade ordering constraint: *complete migrations before pushing metadata.*
3. One-time reconciliation for CDP stage and production.
4. Add the migrate→push→migrate regression test.
**What would flip this to showstopper:** any other gate 5 owner (Skip, Izzy, MJC, AIDP Next) reporting the same failure. That would prove it is the normal upgrade path rather than CDP-specific sequencing. Their reports are pending, so it may be worth holding the ruling until they land.
Contributor guide
Research direction
Start with packages/MetadataSync/src/services/PushService.ts, packages/GenericDatabaseProvider/src/SqlLogger.ts, and the SQLServerDataProvider save path at the cited locations; then inspect GetCreateUpdateSPName in packages/MJCore/src/generic/databaseProviderBase.ts. Reproduce the migrate→push→migrate sequence and trace how the migration SQL is built. Done means the emitter behavior and the shipped-migration strategy are agreed, with a regression test covering the collision path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- sql, typescript
- Domain
- databases, testing-qa, tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100