MemberJunction / MemberJunction/MJ
Open App install-failure compensation: the teardown step has never executed real SQL — exercise it when the first app adopts a teardownDirectory
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
`CompensateSchemaOnFailure` (Open App install-failure rollback) runs an app's teardown scripts as its second step. **That step has never executed a single line of SQL** — not in tests, not in a real install.
The reason is simple: no app currently declares a `migrations.teardownDirectory`, so `HandleTeardown` always hits its early "nothing declared" return. Every run to date — including the end-to-end verification on MemberJunction/MJ#3469 — took the warning branch instead.
The unit tests do not close this either. They stub the migration download to return `Files: []`, which proves the teardown path is *entered* and that the correct directory is requested; `HandleTeardown` then returns early on the empty file list without executing anything. So the assertion is "the path is reachable," not "teardown works."
## Why it's worth tracking
Compensation is the *only* safety net for a failed install now that migrations apply per-migration (MemberJunction/MJ#3469) — a set that fails partway leaves earlier files committed and the database will not undo them. Within that safety net, teardown is the step responsible for the rows that nothing else can reach: free-floating core-schema metadata (`Action`, `AIAgent`, `Query`, `Template`, …) with no FK path back to the app's entities.
So the moment the first app adopts a `teardownDirectory`, this code executes for the very first time **during a failure, on a customer's database**. That is the worst possible place for a first run.
What is known about the surrounding behavior, from testing on #3469:
- Teardown's execution model was verified **directly against a real database** — filename ordering, `${mjSchema}` substitution, all files in one transaction, and rollback of the whole teardown on any error. Inverse DELETEs against never-inserted rows commit as no-ops, so a partially-seeded state is safe.
- What was **not** verified is that path running *inside* `CompensateSchemaOnFailure` — i.e. teardown executing real SQL as part of a real failed install.
## Requested
Gate this on the first adopter: when an app declares a `migrations.teardownDirectory`, exercise the compensation path against it end-to-end — force an install failure after the app's seed migration commits, and confirm the declared teardown actually retires the rows it claims to.
Candidate first adopters, each already tracked: bizapps-sonar#51 (~220 rows), BlueCypress/SaaS#101 (~49), bizapps-committees#17 (5). Whichever lands first is the natural trigger.
Two related observations from the same review, recorded here rather than lost:
1. **Rollback depends on a network round-trip.** `HandleTeardown` downloads teardown scripts from GitHub *during* rollback. If the install failed for a network reason, the teardown download fails too (it is reported, and the schema still drops). Reusing the already-downloaded migration tree would remove that dependency.
2. **Teardown-before-drop ordering is a genuine trade-off, not an oversight.** Reproduced against a real database: with teardown first, a core row pinned by an FK from a row in the app's own schema fails with error 547 and — because all teardown files share one transaction — the whole teardown rolls back. With the drop first, a teardown script that reads an app-schema table fails with error 208. Each order fails exactly where the other succeeds, so the fix is to retry rather than to reorder: attempt teardown with the schema intact, and on failure drop the schema and attempt once more. Teardown is idempotent by construction, so the retry is safe.
## Related
- MemberJunction/MJ#3469
- MemberJunction/MJ#3505 (seed `spCreate*` idempotency)
Contributor guide
Research direction
Start at CompensateSchemaOnFailure and HandleTeardown, then use the first adopting app listed in the issue as the integration fixture. Force installation to fail after the seed migration commits and verify that the declared teardown executes real SQL and retires its rows; the end-to-end test is done when compensation is confirmed against the failed install.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github, sql
- Domain
- backend, databases, testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100