SEP-1813: A stripped app whose versions/ directory survives still fails alembic --name sep upgrade heads

[peter-o-addo]

Summary

Widen the orphan-head filter so stripped apps with empty or leftover versions/ dirs still migrate, and align the sync-script advice with that rule.

• _orphan_heads.py: replace missing_version_locations with empty_version_locations, and update fail-closed docs / ERROR / WARNING
• sync_alembic_version_locations.py, alembic.ini: update GENERATED_COMMENT and refusal stderr, then regenerate the committed header
• tests: cover empty-location arming, shapes 3–4 upgrades/refusals, and the generator↔filter handoff

Tested

• Confirm scripts/sync_alembic_version_locations.py and alembic.ini both say a configured location that is absent from disk or contributes no migration scripts arms the filter.
• Run the sync script against a stripped apps tree and confirm it exits 1 with stderr mentioning contributes no migration scripts and upgrade heads.
• Probe empty_version_locations with a present-but-empty versions dir and confirm that path is reported (filter arms).
• Probe empty_version_locations on the real populated alembic.ini tree and confirm it returns empty (fail-closed evidence stays off).

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• All tests pass locally (make test)
• Pre-commit hooks pass (make run-pre-commit)
• Database migrations generated if models changed (make makemigrations)
• User-facing changes documented (README, inline help, UI text)
• Configuration changes documented with examples
• Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)

[Copilot]

Pull request overview

Broadens orphan-head detection to handle stripped apps whose migration directories remain but contain no revisions.

Changes:

• Detect empty as well as absent migration locations.
• Align generated configuration and CLI diagnostics.
• Add unit, integration, and generator-to-filter handoff coverage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
app/sep/migrations/_orphan_heads.py	Detects locations contributing no revisions.
scripts/sync_alembic_version_locations.py	Updates synchronization guidance and diagnostics.
alembic.ini	Regenerates the explanatory header.
tests/app/sep/migrations/test_orphan_heads.py	Tests empty-location detection.
tests/app/sep/migrations/test_alembic_integration.py	Tests upgrades with empty migration directories.
tests/app/sep/migrations/test_strip_handoff.py	Covers generator/filter handoff scenarios.
tests/scripts/test_sync_alembic_version_locations.py	Tests synchronization refusal behavior.

Suppressed comments (8)

tests/app/sep/migrations/test_alembic_integration.py:492

• AC5 is an unreachable ticket reference in the shipped test. Keep the docstring self-contained by stating only the behavior it verifies.

    """Name the empty versions/ location in the skip WARNING (AC5)."""

tests/app/sep/migrations/test_strip_handoff.py:103

• Pin UTF-8 for the synthetic Alembic configuration so this integration test has deterministic file I/O across hosts.

    ini_path.write_text(
        "[alembic]\n"
        "databases = sep\n"
        "\n"
        "[sep]\n"
        "script_location = %(here)s/app/sep/migrations\n"
        "version_path_separator = :\n"
        f"version_locations = {_MAIN_ENTRY}:"
        f"{_ALPHA_ENTRY}\n"
    )

tests/app/sep/migrations/test_strip_handoff.py:90

• Pin UTF-8 rather than relying on the host locale for this text file.

    (alpha / "__init__.py").write_text("")

tests/app/sep/migrations/test_strip_handoff.py:157

• Pin UTF-8 on this text read rather than relying on the test runner’s locale.

    before = ini_path.read_text()

tests/app/sep/migrations/test_strip_handoff.py:165

• Both text reads should pin UTF-8 so the assertions use the same deterministic encoding as the writer.

        assert ini_path.read_text() == before
    else:
        assert _ALPHA_ENTRY in ini_path.read_text()

tests/scripts/test_sync_alembic_version_locations.py:473

• Pin UTF-8 on the newly added text read instead of inheriting the host locale.

        assert ini_path.read_text() == before

tests/scripts/test_sync_alembic_version_locations.py:481

• Pin UTF-8 on the newly added text read instead of inheriting the host locale.

        assert "app/sep/apps/alpha/migrations/versions" in ini_path.read_text()

tests/scripts/test_sync_alembic_version_locations.py:528

• Pin UTF-8 on the newly added text read instead of inheriting the host locale.

        assert ini_path.read_text() == before

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[peter-o-addo]

1. Claim text

scripts/sync_alembic_version_locations.py:57:# from disk or contributes no migration scripts is how the orphan-head
alembic.ini:85:# from disk or contributes no migration scripts is how the orphan-head
.github/instructions/migrations.instructions.md:20: ... absent from disk or contributes no migration scripts ...
sidecar/README.md:133-135: ... contributes no revisions (absent from disk or present and empty) ...

2. Sync refusal on stripped tree

Ran the sync script against a temp ini that still lists alpha while the apps root is empty, so regeneration would drop that entry.

Output:

.../alembic.ini: refusing to remove 1 [sep] version_locations entry(ies): %(here)s/app/sep/apps/alpha/migrations/versions. A configured location that is absent from disk or contributes no migration scripts is how the orphan-head filter recognises a stripped app, so removing it silently would disarm that check. Restore the migration directory; or, on a tree with an app deliberately stripped, skip this script and run `alembic --name sep upgrade heads` directly — leaving the entry in place is what arms the filter. Re-run with `--allow-removals` only when the migration chain is being deleted for good.
exit=1

3. Empty versions dir arms the filter

Called empty_version_locations with a real config plus one present-but-empty versions directory appended to version_locations.

Output:

armed on empty dir: True
empty_version_locations: ('.../versions',)
ok

4. Populated tree stays fail-closed

Called empty_version_locations on the committed alembic.ini tree where every configured location has revisions.

Output:

empty_version_locations: ()
fail-closed evidence empty: ok

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
app/sep
inventory.py
app/sep/migrations
_orphan_heads.py
app/sep/sync/syncers
pmm.py
Project Total

This report was generated by python-coverage-comment-action