[THROWAWAY - DO NOT MERGE] prove the migration-graph guard fires and goes red - #612
Closed
WilfordGrimley wants to merge 2 commits into
Closed
[THROWAWAY - DO NOT MERGE] prove the migration-graph guard fires and goes red#612WilfordGrimley wants to merge 2 commits into
WilfordGrimley wants to merge 2 commits into
Conversation
…branch Two branches can each add `0098_<something>.py` depending on `0097`. The filenames differ, so there is no textual conflict, GitHub reports the second PR MERGEABLE/CLEAN, and both branches are individually valid. The moment the second merges, `cardpicker` has two leaf nodes - and pytest-django builds its test database by running `migrate`, so the fork fails at test-database SETUP on EVERY branch in the repo, not just the one that introduced it. This has now happened twice: at 0096 (#568 vs #570) and at 0098 (#573 vs #601). #576 repaired the first fork but prevented nothing, which is why the second arrived within days. Nothing in CI failed either time. WHY THE MERGE RESULT IS THE WHOLE POINT #601's checks were 10/10 green with the collision already live on master - they had run against master BEFORE #573 landed, and GitHub does not re-run a PR's checks when its base moves. A check reading only the PR branch's files sees one leaf and passes; the fork exists only in the merge. So `check_migration_leaves.py --base origin/<base_ref>` unions the worktree's migrations with the base branch's CURRENT tip, resolved at run time, honouring anything the PR deletes (`--no-renames` is load-bearing: a renumber is a delete+add of near-identical content and git otherwise reports it as a rename, which would resurrect the old number and fail a PR that had already fixed itself). HOW IT DECIDES Static `ast` read of every `migrations/` package: filenames are nodes, each file's `dependencies` gives same-app edges, `run_before` gives reversed ones, and a squash's `replaces` removes the nodes it stands in for. Migration modules are never imported or executed, so this needs no settings module, no installed apps, no postgres and no `requirements.txt` - it runs on a bare `actions/setup-python` in about a second. Non-literal dependency entries (`migrations.swappable_dependency(settings.AUTH_USER_MODEL)`, in seven of this repo's migrations) are cross-app by construction and are skipped, not guessed at. Exit code is the finding count, matching docs_lint.py's and check_protected_core_license.py's convention. Findings: more than one leaf per app (the failure), a duplicate NNNN number prefix within an app (the same defect one step earlier, and the actionable instruction), and a dependency naming a migration that does not exist. WHAT IT CANNOT DO, STATED PLAINLY A check run that PASSED before the base moved stays green in GitHub's UI. No CI job can fix that from the inside; branch protection's "Require branches to be up to date before merging" is the setting that closes it, and this makes the forced re-run meaningful. `merge_group` is wired up so a merge queue would close it too. The workflow is its own file rather than another entry in docs-lint.yml, which four open PRs are already editing. Every path glob uses `**`: a single `*` does not match a slash, so `MPCAutofill/cardpicker/*.py` would miss `migrations/` entirely (#588 hit exactly that). Demonstrated red-then-green against the real collision, and kept as permanent regression coverage in `.github/scripts/tests/test_check_migration_leaves.py` (15 tests) rather than as a one-off local run: a scratch repo where master has `0098_card_illustration_consensus_fields` and a feature branch has `0098_rename_printings_count_catalogued`, both on 0097, asserts clean on the branch alone, two leaves against the merge, and clean again once renumbered to 0099 - plus no-finding cases for a normal single-migration PR, a PR touching no migrations, cross-app dependencies, swappable dependencies, squashes and this repo's own tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
Author
|
Purpose served: "One leaf per app (merged with the base branch)" went RED (exit 2), with the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throwaway demonstration for #611. Adds a second
0098depending on0097, exactly reconstructing the #573-vs-#601 collision. Expected: "One leaf per app (merged with the base branch)" FAILS. Proves (a) the**/migrations/**path filter actually fires and (b) the guard goes red on the real thing. Closing immediately after.