From 8febe70f3e344773f8aefce1fc87bd7cb9dd6700 Mon Sep 17 00:00:00 2001 From: Andrii Pasternak <44377756+AndriiPasternak31@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:50:13 +0000 Subject: [PATCH 1/5] test(migrations): every _migrate* function is registered, and every registered function is one (#3395) run_all_migrations runs only what MIGRATIONS lists, so a migration function that is written but never registered ships green today and never runs on an existing install. A data-only migration (an UPDATE backfill) is invisible to any schema diff. test_fan_out_id_migration_present pinned this class for one name; this test pins it for every name, in both directions. The functions are discovered from the loaded module (inspect, filtered to _migrate* defined in db/migrations.py) and compared by identity with the MIGRATIONS captured at import. Non-vacuity: the discovered set must be non-empty and equal the top-level `def _migrate*` names parsed from the file's source, so an empty or partial discovery cannot pass. Mutations, each applied to db/migrations.py from a cp snapshot and restored: - M1, delete ("a2a_caller_identity", _migrate_a2a_caller_identity) from MIGRATIONS: AssertionError: _migrate* functions defined in db/migrations.py but not registered in MIGRATIONS (existing installs never run them): ['_migrate_a2a_caller_identity'] - D1, add an unregistered data-only _migrate_probe_backfill (UPDATE, no DDL): AssertionError: _migrate* functions defined in db/migrations.py but not registered in MIGRATIONS (existing installs never run them): ['_migrate_probe_backfill'] - V1, register probe_backfill, which is not a _migrate* function: AssertionError: MIGRATIONS entries whose function is not a _migrate* function defined in db/migrations.py: ['probe_backfill -> migrations.probe_backfill'] - Vacuity, point the test's file filter at a non-existent split.py: AssertionError: no _migrate* functions discovered in .../src/backend/db/migrations.py M0 (no mutation): green, 170 defined = 170 registered. The plan's size floor (discovered >= len(MIGRATIONS)) was replaced: under V1 it fired first and reported "discovery is broken" instead of naming the foreign entry. The source-parse check is the floor instead. Part of #3395. --- tests/unit/test_migrations.py | 74 +++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/unit/test_migrations.py b/tests/unit/test_migrations.py index 655d397291..0d0d9acc61 100644 --- a/tests/unit/test_migrations.py +++ b/tests/unit/test_migrations.py @@ -8,7 +8,9 @@ Uses an in-memory SQLite database. """ +import ast import importlib.util +import inspect import sqlite3 from datetime import datetime, timezone from pathlib import Path @@ -279,6 +281,78 @@ def test_fan_out_id_migration_present(self): names = [name for name, _ in MIGRATIONS] assert "execution_fan_out_id" in names + def test_every_migrate_function_is_registered_and_vice_versa(self): + """The registry is a bijection with the module's `_migrate*` functions (#3395). + + `run_all_migrations` runs only what `MIGRATIONS` lists, so a `_migrate*` + function that is written but never registered never runs on an existing + install. No schema diff can see the data-only case (an `UPDATE` + backfill), and a column the function adds is masked on a fresh install + whenever `init_schema` creates the table. This generalises + `test_fan_out_id_migration_present` from one name to every name, in both + directions: every `_migrate*` function defined in `db/migrations.py` is + registered, and every registered function is one of them. + + Rule: a helper in `db/migrations.py` that is deliberately not a + migration must not use the `_migrate` prefix. + + The functions are discovered from the loaded module, never from a + hand-written list, and compared by object identity against the + `MIGRATIONS` captured at import (other tests `patch.object` the module + attribute). The non-vacuity floor keeps the test from passing on an + empty or partial discovery, e.g. after `db/migrations.py` is split into + a package: the discovered names must be non-empty and equal the + top-level `def _migrate*` names parsed from the file's source. + + Other shape: none of the delivery brief's six cases (fresh install, + empty data, another origin, flag off, clock ahead, restricted + container) applies to a module-introspection test; the second case is + the converse direction (a registered function that is not a + `_migrate*` function of this module). + """ + module_file = Path(_migrations_mod.__file__).resolve() + defined = { + fn + for name, fn in inspect.getmembers(_migrations_mod, inspect.isfunction) + if name.startswith("_migrate") + and Path(fn.__code__.co_filename).resolve() == module_file + } + registered = {fn for _, fn in MIGRATIONS} + + # Non-vacuity: discovery is non-empty and agrees with a second, + # independent derivation from the same file, so an empty or partial + # discovery cannot make the bijection pass. + source_names = { + node.name + for node in ast.parse(module_file.read_text(encoding="utf-8")).body + if isinstance(node, ast.FunctionDef) and node.name.startswith("_migrate") + } + assert defined, f"no _migrate* functions discovered in {module_file}" + assert registered, "MIGRATIONS is empty" + assert {fn.__name__ for fn in defined} == source_names, ( + f"introspection of {module_file} disagrees with its source: " + f"discovered {len(defined)}, source defines {len(source_names)}" + ) + + problems = [] + unregistered = sorted(fn.__name__ for fn in defined - registered) + if unregistered: + problems.append( + "_migrate* functions defined in db/migrations.py but not registered " + f"in MIGRATIONS (existing installs never run them): {unregistered}" + ) + foreign = sorted( + f"{name} -> {fn.__module__}.{fn.__qualname__}" + for name, fn in MIGRATIONS + if fn not in defined + ) + if foreign: + problems.append( + "MIGRATIONS entries whose function is not a _migrate* function " + f"defined in db/migrations.py: {foreign}" + ) + assert not problems, "\n".join(problems) + # --- helpers --- From 0c00614e062ca13c44ea5a62347b52d0d4e42c5f Mon Sep 17 00:00:00 2001 From: Andrii Pasternak <44377756+AndriiPasternak31@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:51:16 +0000 Subject: [PATCH 2/5] test(registry): index test_migrations.py with the #3395 registry bijection tests/registry.json had no entry for unit/test_migrations.py. The entry sits beside the other migration-runner entry (#1160) rather than at the end of the array, so it does not collide with another slice of #3395 that appends there. Part of #3395. --- tests/registry.json | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/registry.json b/tests/registry.json index cd8c29853d..0010a4d3bb 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1948,6 +1948,19 @@ ], "description": "Migration runner atomicity + cross-process lock (#1160). _atomic_rebuild rename-swap inside an explicit transaction closes the DROP-rebuild data-loss window (crash mid-rebuild rolls back with no data loss; verified for agent_sharing + agent_skills incl. lowercase/NULL-email behavior preservation and index recreation); replay-after-partial-apply completes cleanly; a failed migration is named on its traceback via add_note (original exception type preserved). Cross-process flock (db/migration_lock.py) serialises concurrent boots — multiprocessing tests prove mutual exclusion and a safe concurrent rebuild." }, + { + "file": "unit/test_migrations.py", + "feature": "#3395", + "added": "2026-10-26", + "categories": [ + "backend", + "unit", + "database", + "migrations", + "guard" + ], + "description": "Migration runner unit tests (#236): schema_migrations tracking, idempotency and resume, failure propagation (only `no such table` is a fresh-install skip), and MIGRATIONS sanity. #3395 adds the registry bijection: the `_migrate*` functions defined in db/migrations.py, discovered by introspection and cross-checked against the file's parsed source (non-vacuity), equal the functions registered in MIGRATIONS, compared by identity. An unregistered function (including a data-only backfill no schema diff can see) is named; so is a registered function that is not a `_migrate*` of the module. A deliberately unregistered helper must not use the `_migrate` prefix." + }, { "file": "unit/test_compatibility_checks.py", "feature": "#668", From 25844000f46340fc9855ce9148202456d6ebc768 Mon Sep 17 00:00:00 2001 From: Andrii Pasternak <44377756+AndriiPasternak31@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:52:11 +0000 Subject: [PATCH 3/5] test(migrations): the registry bijection also refuses one function under two entries (#3395) /review finding: the set comparison is a bijection on functions only, so one _migrate* function registered under two MIGRATIONS names passed while running twice on every fresh database. The test now names every function with more than one entry. Mutation, register _migrate_a2a_caller_identity a second time as "a2a_caller_identity_again": AssertionError: functions registered under more than one MIGRATIONS entry: ["_migrate_a2a_caller_identity <- ['a2a_caller_identity', 'a2a_caller_identity_again']"] M0, M1, D1 and V1 re-run after this change: green, then red with the same messages as in the first commit. Part of #3395. --- tests/unit/test_migrations.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_migrations.py b/tests/unit/test_migrations.py index 0d0d9acc61..1a76589279 100644 --- a/tests/unit/test_migrations.py +++ b/tests/unit/test_migrations.py @@ -291,7 +291,7 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): whenever `init_schema` creates the table. This generalises `test_fan_out_id_migration_present` from one name to every name, in both directions: every `_migrate*` function defined in `db/migrations.py` is - registered, and every registered function is one of them. + registered exactly once, and every registered function is one of them. Rule: a helper in `db/migrations.py` that is deliberately not a migration must not use the `_migrate` prefix. @@ -351,6 +351,18 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): "MIGRATIONS entries whose function is not a _migrate* function " f"defined in db/migrations.py: {foreign}" ) + entries_by_fn = {} + for name, fn in MIGRATIONS: + entries_by_fn.setdefault(fn, []).append(name) + repeated = sorted( + f"{fn.__name__} <- {names}" + for fn, names in entries_by_fn.items() + if len(names) > 1 + ) + if repeated: + problems.append( + f"functions registered under more than one MIGRATIONS entry: {repeated}" + ) assert not problems, "\n".join(problems) From e2fecdb56ee7a9da067f6c84e753809803993bca Mon Sep 17 00:00:00 2001 From: Andrii Pasternak <44377756+AndriiPasternak31@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:52:37 +0000 Subject: [PATCH 4/5] test(registry): the test_migrations.py entry names the one-entry-per-function rule (#3395) Follows the /review fix that refuses one function under two MIGRATIONS entries. Part of #3395. --- tests/registry.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/registry.json b/tests/registry.json index 0010a4d3bb..258843ba99 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1959,7 +1959,7 @@ "migrations", "guard" ], - "description": "Migration runner unit tests (#236): schema_migrations tracking, idempotency and resume, failure propagation (only `no such table` is a fresh-install skip), and MIGRATIONS sanity. #3395 adds the registry bijection: the `_migrate*` functions defined in db/migrations.py, discovered by introspection and cross-checked against the file's parsed source (non-vacuity), equal the functions registered in MIGRATIONS, compared by identity. An unregistered function (including a data-only backfill no schema diff can see) is named; so is a registered function that is not a `_migrate*` of the module. A deliberately unregistered helper must not use the `_migrate` prefix." + "description": "Migration runner unit tests (#236): schema_migrations tracking, idempotency and resume, failure propagation (only `no such table` is a fresh-install skip), and MIGRATIONS sanity. #3395 adds the registry bijection: the `_migrate*` functions defined in db/migrations.py, discovered by introspection and cross-checked against the file's parsed source (non-vacuity), equal the functions registered in MIGRATIONS, compared by identity, each registered exactly once. An unregistered function (including a data-only backfill no schema diff can see) is named; so is a registered function that is not a `_migrate*` of the module. A deliberately unregistered helper must not use the `_migrate` prefix." }, { "file": "unit/test_compatibility_checks.py", From 1d8cca27a9b4db1c227ea3f7c1a9d3084d5d10ba Mon Sep 17 00:00:00 2001 From: Andrii Pasternak <44377756+AndriiPasternak31@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:50:42 +0000 Subject: [PATCH 5/5] test(migrations): a decorated or async _migrate* is named, not counted (#3395) The bijection discovered functions by introspection and cross-checked them against the parsed source. A `_migrate*` that is `async def`, or decorated by a wrapper from another module, dropped out of one derivation but not the other, so it failed the non-vacuity check with "discovered N, source defines M" (equal counts when there is one of each) instead of being named as unregistered. A registered decorated migration failed the same way, although it runs. Discovery now reads the source: every top-level `def` or `async def` named `_migrate*`, resolved to the object the module binds, which is the object MIGRATIONS holds. The non-vacuity cross-check compares those names with the `_migrate*` callables the loaded module binds and names the difference. An `async def` migration is refused even when registered: run_all_migrations calls it without awaiting it, so it is recorded as applied and its body never runs. The old mismatch caught that only by accident. --- tests/registry.json | 2 +- tests/unit/test_migrations.py | 158 ++++++++++++++++++++++++++++------ 2 files changed, 131 insertions(+), 29 deletions(-) diff --git a/tests/registry.json b/tests/registry.json index 258843ba99..232432cd44 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1959,7 +1959,7 @@ "migrations", "guard" ], - "description": "Migration runner unit tests (#236): schema_migrations tracking, idempotency and resume, failure propagation (only `no such table` is a fresh-install skip), and MIGRATIONS sanity. #3395 adds the registry bijection: the `_migrate*` functions defined in db/migrations.py, discovered by introspection and cross-checked against the file's parsed source (non-vacuity), equal the functions registered in MIGRATIONS, compared by identity, each registered exactly once. An unregistered function (including a data-only backfill no schema diff can see) is named; so is a registered function that is not a `_migrate*` of the module. A deliberately unregistered helper must not use the `_migrate` prefix." + "description": "Migration runner unit tests (#236): schema_migrations tracking, idempotency and resume, failure propagation (only `no such table` is a fresh-install skip), and MIGRATIONS sanity. #3395 adds the registry bijection: the `_migrate*` functions defined in db/migrations.py, discovered from the file's parsed source (top-level `def` or `async def`, decorated or not) and cross-checked against the callables the loaded module binds (non-vacuity), equal the functions registered in MIGRATIONS, compared by identity, each registered exactly once. An unregistered function (including a data-only backfill no schema diff can see, and a decorated or `async def` one) is named; so is a registered function that is not a `_migrate*` of the module, and an `async def` migration, which the runner would record as applied without running. A deliberately unregistered helper must not use the `_migrate` prefix." }, { "file": "unit/test_compatibility_checks.py", diff --git a/tests/unit/test_migrations.py b/tests/unit/test_migrations.py index 1a76589279..93c295a796 100644 --- a/tests/unit/test_migrations.py +++ b/tests/unit/test_migrations.py @@ -9,9 +9,12 @@ """ import ast +import functools import importlib.util import inspect import sqlite3 +import types +from collections.abc import Callable, Sequence from datetime import datetime, timezone from pathlib import Path from unittest.mock import patch @@ -296,13 +299,19 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): Rule: a helper in `db/migrations.py` that is deliberately not a migration must not use the `_migrate` prefix. - The functions are discovered from the loaded module, never from a - hand-written list, and compared by object identity against the - `MIGRATIONS` captured at import (other tests `patch.object` the module - attribute). The non-vacuity floor keeps the test from passing on an - empty or partial discovery, e.g. after `db/migrations.py` is split into - a package: the discovered names must be non-empty and equal the - top-level `def _migrate*` names parsed from the file's source. + The functions are discovered from the file's source, never from a + hand-written list: every top-level `def` or `async def` named + `_migrate*`, decorated or not. Each name resolves to the object the + loaded module binds, compared by identity against the `MIGRATIONS` + captured at import (other tests `patch.object` the module attribute). + The non-vacuity floor keeps the test from passing on an empty or + partial discovery, e.g. after `db/migrations.py` is split into a + package: the discovered names must be non-empty and equal the + `_migrate*` callables the loaded module binds. + + An `async def` migration is refused even when registered: + `run_all_migrations` calls it without awaiting it, so its body never + runs and the migration is still recorded as applied. Other shape: none of the delivery brief's six cases (fresh install, empty data, another origin, flag off, clock ahead, restricted @@ -310,32 +319,47 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): the converse direction (a registered function that is not a `_migrate*` function of this module). """ - module_file = Path(_migrations_mod.__file__).resolve() - defined = { - fn - for name, fn in inspect.getmembers(_migrations_mod, inspect.isfunction) - if name.startswith("_migrate") - and Path(fn.__code__.co_filename).resolve() == module_file - } - registered = {fn for _, fn in MIGRATIONS} - - # Non-vacuity: discovery is non-empty and agrees with a second, - # independent derivation from the same file, so an empty or partial - # discovery cannot make the bijection pass. + self._assert_registry_bijection(_migrations_mod, MIGRATIONS) + + @staticmethod + def _assert_registry_bijection( + module: types.ModuleType, + migrations: Sequence[tuple[str, Callable[..., object]]], + ) -> None: + assert module.__file__ is not None, f"{module.__name__} has no source file" + module_file = Path(module.__file__).resolve() + # Read from the source, so a decorator or `async def` cannot hide one. source_names = { node.name for node in ast.parse(module_file.read_text(encoding="utf-8")).body - if isinstance(node, ast.FunctionDef) and node.name.startswith("_migrate") + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.name.startswith("_migrate") + } + registered = {fn for _, fn in migrations} + + # Non-vacuity: discovery is non-empty and agrees with a second, + # independent derivation from the loaded module, so an empty or + # partial discovery cannot make the bijection pass. + bound_names = { + name + for name, obj in vars(module).items() + if name.startswith("_migrate") and callable(obj) } - assert defined, f"no _migrate* functions discovered in {module_file}" + assert source_names, f"no _migrate* functions defined in {module_file}" assert registered, "MIGRATIONS is empty" - assert {fn.__name__ for fn in defined} == source_names, ( - f"introspection of {module_file} disagrees with its source: " - f"discovered {len(defined)}, source defines {len(source_names)}" + assert source_names == bound_names, ( + f"the top-level _migrate* defs in {module_file} disagree with the " + "_migrate* callables the loaded module binds: only in source " + f"{sorted(source_names - bound_names)}, only bound " + f"{sorted(bound_names - source_names)}" ) + # The bound object is the one MIGRATIONS holds (the wrapper, if decorated). + defined = {getattr(module, name): name for name in source_names} problems = [] - unregistered = sorted(fn.__name__ for fn in defined - registered) + unregistered = sorted( + name for fn, name in defined.items() if fn not in registered + ) if unregistered: problems.append( "_migrate* functions defined in db/migrations.py but not registered " @@ -343,7 +367,7 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): ) foreign = sorted( f"{name} -> {fn.__module__}.{fn.__qualname__}" - for name, fn in MIGRATIONS + for name, fn in migrations if fn not in defined ) if foreign: @@ -351,8 +375,8 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): "MIGRATIONS entries whose function is not a _migrate* function " f"defined in db/migrations.py: {foreign}" ) - entries_by_fn = {} - for name, fn in MIGRATIONS: + entries_by_fn: dict[Callable[..., object], list[str]] = {} + for name, fn in migrations: entries_by_fn.setdefault(fn, []).append(name) repeated = sorted( f"{fn.__name__} <- {names}" @@ -363,8 +387,62 @@ def test_every_migrate_function_is_registered_and_vice_versa(self): problems.append( f"functions registered under more than one MIGRATIONS entry: {repeated}" ) + never_run = sorted( + name + for fn, name in defined.items() + if inspect.iscoroutinefunction(fn) or inspect.isasyncgenfunction(fn) + ) + if never_run: + problems.append( + "async _migrate* functions: run_all_migrations calls each migration " + "without awaiting it, so the body never runs and the migration is " + f"still recorded as applied: {never_run}" + ) assert not problems, "\n".join(problems) + def test_registry_bijection_names_unregistered_decorated_and_async_migrations( + self, tmp_path: Path + ) -> None: + """An unregistered decorated or `async def` `_migrate*` is named as unregistered (#3395).""" + module = _load_synthetic_migrations( + tmp_path, + "def _migrate_plain(cursor, conn):\n pass\n\n" + "@foreign_decorator\ndef _migrate_decorated(cursor, conn):\n pass\n\n" + "async def _migrate_async(cursor, conn):\n pass\n\n" + "MIGRATIONS = [('plain', _migrate_plain)]\n", + ) + with pytest.raises(AssertionError) as raised: + self._assert_registry_bijection(module, module.MIGRATIONS) + message = str(raised.value) + assert ( + "not registered in MIGRATIONS (existing installs never run them): " + "['_migrate_async', '_migrate_decorated']" + ) in message, message + assert "still recorded as applied: ['_migrate_async']" in message, message + + def test_registry_bijection_accepts_a_registered_decorated_migration( + self, tmp_path: Path + ) -> None: + """A decorator from another module does not hide a registered migration.""" + module = _load_synthetic_migrations( + tmp_path, + "@foreign_decorator\ndef _migrate_decorated(cursor, conn):\n pass\n\n" + "MIGRATIONS = [('decorated', _migrate_decorated)]\n", + ) + self._assert_registry_bijection(module, module.MIGRATIONS) + + def test_registry_bijection_refuses_a_registered_async_migration( + self, tmp_path: Path + ) -> None: + """Registering an `async def` migration does not make it run.""" + module = _load_synthetic_migrations( + tmp_path, + "async def _migrate_async(cursor, conn):\n pass\n\n" + "MIGRATIONS = [('async', _migrate_async)]\n", + ) + with pytest.raises(AssertionError, match=r"never runs.*\['_migrate_async'\]"): + self._assert_registry_bijection(module, module.MIGRATIONS) + # --- helpers --- @@ -373,3 +451,27 @@ def wrapper(cursor, conn): log.append(name) return original_fn(cursor, conn) return wrapper + + +def _foreign_decorator( + fn: Callable[[object, object], None], +) -> Callable[[object, object], None]: + """A decorator defined outside the migrations module, as a shared helper would be.""" + + @functools.wraps(fn) + def wrapper(cursor: object, conn: object) -> None: + fn(cursor, conn) + + return wrapper + + +def _load_synthetic_migrations(tmp_path: Path, source: str) -> types.ModuleType: + """Load `source` as a stand-in `migrations.py`, with `foreign_decorator` in scope.""" + path = tmp_path / "migrations.py" + path.write_text(source, encoding="utf-8") + spec = importlib.util.spec_from_file_location("synthetic_migrations", path) + assert spec is not None and spec.loader is not None, f"cannot load {path}" + module = importlib.util.module_from_spec(spec) + module.__dict__["foreign_decorator"] = _foreign_decorator + spec.loader.exec_module(module) + return module