test: fix flaky/order-dependent tests (#855 #860 #861) and drop LEGACY_FLAT

CLAUDE.md

@@ -135,9 +135,10 @@ The Python package name (`feature_flags`) keys the i18n namespace,
 `app.state.<package>` and table prefixes. Don't swap them: `keys.<ns>` breaks if
 the namespace is the meta name.
 
-**Module layout.** New modules use the area layout below; existing modules
-migrate one by one. See [Module layout](docs/module-layout.md) for root lookup consumers,
-tests, compatibility shims and the migration checklist.
+**Module layout.** Every module uses the area layout below
+(`host/tests/test_module_layout.py` enforces it). See [Module layout](docs/module-layout.md)
+for root lookup consumers, tests, compatibility shims and the checklist for moving a
+downstream flat module.
 
 ```text
 modules/<name>/<name>/
@@ -351,7 +352,7 @@ development and testing (`session_cookie_secure`).
 - **No magic strings in non-test Python** (`scripts/check_hardcoded_strings.py`):
   `RequiresPermission(...)`, `registry.map_role(...)`, `registry.add_group(...)`
   items, `inertia.render(...)` keys and `depends_on=[...]` entries must be
-  constants from `core/constants.py` (legacy modules: `constants.py`):
+  constants from `core/constants.py`:
   `PERM_*`, role names, `_PAGE_*`, `_MODULE_*`.
 - **300-line cap** on `.py`/`.ts`/`.tsx`, tests included, untracked files
   included (`scripts/check_file_size.py`). Exempt: shadcn components under

docs/framework-conventions.md

@@ -25,9 +25,8 @@ modules/<name>/
     └── web/crud/test_<name>_crud.py
 ```
 
-New scaffolded modules use `core/` and `web/`; existing modules migrate one at a
-time. See [Module layout](module-layout.md) for the root lookup table, area
-selection, test naming, and compatibility policy. Area initializers contain
+Modules use `core/` and `web/`. See [Module layout](module-layout.md) for the
+root lookup table, area selection, test naming, and compatibility policy. Area initializers contain
 only a docstring. Keep private helpers in the area they serve.
 
 Scaffold with `smpy create-module <name> --dest modules/<name>` (or

docs/guide/project-structure.md

@@ -57,8 +57,8 @@ See [Bundled modules](/modules/) for details on each.
 
 ## Anatomy of a module
 
-New scaffolded modules use the area layout. Existing bundled modules migrate
-one by one; [Module layout](/module-layout) tracks their status and the full contract.
+Every bundled module and every scaffolded module uses the area layout;
+[Module layout](/module-layout) has the full contract.
 
 ```text
 modules/orders/

docs/module-authoring.md

@@ -29,7 +29,7 @@ modules/<name>/
     └── web/crud/test_<name>_crud.py
 ```
 
-New modules use this layout. Existing bundled modules migrate one at a time;
+Every module uses this layout;
 see [Module layout](module-layout.md) for the exact root contract and shim policy.
 Area initializers contain only a docstring; private helpers live in their area.
 Tests have no `__init__.py`, use globally unique `test_<name>_*.py` basenames,

docs/module-layout.md

@@ -1,7 +1,8 @@
 # Module layout
 
-New modules use this layout. Existing modules under `modules/` migrate in
-separate PRs, one module at a time. No existing module moves in PR 0.
+Every module under `modules/` uses this layout; the last one moved in #862.
+A downstream module still on the flat layout moves as described in
+[Moving an existing module](#moving-an-existing-module).
 The downstream `smpy_modules` layout is the reference for areas; this repo
 keeps its own pytest naming and collection conventions.
 
@@ -72,7 +73,6 @@ The 300-line cap still applies, including tests; split by responsibility.
 
 The root ratchet permits `core/`, `web/`, `ui/` and the lookup paths above.
 Declare a new multi-feature domain area in its `DOMAIN_AREAS` map when needed.
-Do not expand `LEGACY_FLAT` to accommodate new code.
 
 ## Frontend
 
@@ -118,8 +118,7 @@ Mirror package ownership, using `tests/<area>/<feature>/test_<module>_*.py`.
 Keep module-wide `conftest.py` at `tests/` root. Do not add `__init__.py` in test
 directories. Basenames must be globally unique: default pytest import mode
 loads non-package tests by basename, even when they live in different folders.
-Existing flat test names are grandfathered only while their module remains
-in `LEGACY_FLAT`; migration renames them with the module prefix.
+The root-layout test refuses a `test_*.py` directly under `tests/`.
 
 A helper shared by several test folders stays at `tests/` root as
 `_<module>_support.py`. The root `conftest.py` puts that directory on
@@ -163,14 +162,12 @@ sys.modules[__name__] = importlib.import_module("orders.web.service")
 The alias makes both paths the same module object, so mocks and
 `monkeypatch` strings keep working. Files containing both `deprecated_module(`
 and `sys.modules[__name__]` are auto-discovered by the expiry test and ignored
-by the root-layout test, so the module can leave `LEGACY_FLAT` while they
-remain. When the old path was inside a package that moved (for example
-`orders.endpoints.api`), keep that package with a docstring-only `__init__.py`
+by the root-layout test while they remain. When the old path was inside a
+package that moved (for example `orders.endpoints.api`), keep that package with a docstring-only `__init__.py`
 and alias each module in it; a package holding only aliases is ignored by the
 root-layout test too. A shim that is a plain re-export instead (not an alias) must be added
-by hand to the expiry test's `_SHIM_MODULES` tuple and `_EXPECTED` set and kept
-in the module's frozen legacy entries until removed. A completely migrated
-module leaves `LEGACY_FLAT` once its Python tests are nested.
+by hand to the expiry test's `_SHIM_MODULES` tuple and `_EXPECTED` set and to
+the root-layout test's `DEPRECATED_ROOT` map until removed.
 
 ## Moving an existing module
 
@@ -194,7 +191,7 @@ One module per PR; no logic changes in a move PR.
    JS tests remain in `tests-js/`.
 4. Add external-import shims with removal release `0.0.46`, alias shims are
    auto-discovered by the expiry test (re-export shims are registered by hand),
-   shrink the exact frozen root exceptions, and update the
+   drop the shim's `DEPRECATED_ROOT` entry once it expires, and update the
    status table below. Domain areas must be named in `DOMAIN_AREAS`.
 5. Run `python -c "import <pkg>.<mod>"` in a **fresh interpreter for every
    module**, not one interpreter importing all modules in sequence.

host/tests/test_module_layout.py

@@ -52,9 +52,6 @@
     "settings": frozenset({"hydrate.py", "module_registry.py", "registration.py"}),
 }
 
-# Frozen inventory, not a computed baseline: moves must remove their old entries.
-LEGACY_FLAT: dict[str, frozenset[str]] = {}
-
 
 def _is_layout_shim(path: Path) -> bool:
     """A moved module's old path: a deprecated alias, allowed until its remove_in."""
@@ -93,23 +90,12 @@ def test_module_root_layout(module: str) -> None:
         f"{module}: remove expired DEPRECATED_ROOT entries"
     )
     allowed = ALLOWED_ROOT | DOMAIN_AREAS.get(module, frozenset()) | deprecated
-    legacy = LEGACY_FLAT.get(module, frozenset())
-    if module in LEGACY_FLAT:
-        assert legacy, f"{module}: remove the empty LEGACY_FLAT entry"
-    assert not entries - allowed - legacy, (
-        f"{module}: unexpected roots {entries - allowed - legacy}"
-    )
-    assert legacy <= entries, f"{module}: remove stale LEGACY_FLAT entries {legacy - entries}"
-    assert not legacy & allowed, f"{module}: remove allowed entries from LEGACY_FLAT"
+    assert not entries - allowed, f"{module}: unexpected roots {entries - allowed}"
     if (package / "cli.py").is_file():
         project = tomllib.loads((MODULES / module / "pyproject.toml").read_text())["project"]
         targets = [
             value for group in project.get("entry-points", {}).values() for value in group.values()
         ]
         assert any(value.split(":")[0] == f"{module}.cli" for value in targets)
-    if module not in LEGACY_FLAT:
-        assert not list((MODULES / module / "tests").glob("test_*.py")), module
-
-
-def test_legacy_modules_still_exist() -> None:
-    assert all((MODULES / name / name).is_dir() for name in LEGACY_FLAT)
+    # Tests mirror the package areas: tests/<area>/<feature>/test_<module>_*.py.
+    assert not list((MODULES / module / "tests").glob("test_*.py")), module

modules/background_tasks/background_tasks/executions/tenant_context.py

@@ -174,7 +174,11 @@ def restore_tenant(*, task_id: str | None, task: Any) -> None:
     # context, so without headers it keeps the caller's tenant and owner.
     eager = bool(getattr(getattr(task, "request", None), "is_eager", False))
     tenant = _tenant_of(task) or (current_tenant_id.get() if eager else _default_tenant)
-    owner = message_owner(task=task) or (_task_owner.get() if eager else "")  # "": no owner
+    # ``""`` (not ``None``) means "no owner" and stops :func:`stamp_owner`
+    # falling through to an ambient ``current_user_id``. A worker prerun sets it
+    # afresh, so a stranded binding never reaches the next task's messages (an
+    # eager run deliberately inherits its caller's).
+    owner = message_owner(task=task) or (_task_owner.get() if eager else "")
     owner_token = _task_owner.set(owner)
     tenant_token = current_tenant_id.set(tenant)
     _tokens.set((*_tokens.get(), (task_id, tenant_token, owner_token)))

modules/background_tasks/tests/conftest.py

@@ -11,6 +11,7 @@
 import pytest
 from background_tasks.core import sync_db
 from background_tasks.core.constants import PERM_VIEW, TaskStatus
+from background_tasks.executions import tenant_context
 from background_tasks.models import TaskExecution
 from sqlalchemy import select
 
@@ -27,6 +28,20 @@ def _no_worker_audit_sink(monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setattr(sync_db, "discover_audit_callback", lambda: None)
 
 
+@pytest.fixture(autouse=True)
+def _unwind_stranded_task_bindings() -> Iterator[None]:
+    """Leave no task tenant/owner binding behind for the next test (#861).
+
+    A test that runs a prerun without its postrun would otherwise leave the
+    owner pinned to "no owner" for every later test in this context.
+    """
+    yield
+    # Loop: release_tenant unwinds to the *latest* entry for an id, so a
+    # repeated id (nested eager run) can leave entries below it.
+    while stack := tenant_context._tokens.get():
+        tenant_context.release_tenant(task_id=stack[0][0])
+
+
 @pytest.fixture
 async def _stub_celery(app) -> None:
     """Replace the real Celery instance with a MagicMock.

modules/background_tasks/tests/worker/tenancy/test_background_tasks_tenant_context.py

@@ -6,9 +6,11 @@
 
 import pytest
 from background_tasks.executions.tenant_context import (
+    OWNER_HEADER,
     TENANT_HEADER,
     release_tenant,
     restore_tenant,
+    stamp_owner,
     stamp_tenant,
 )
 from simple_module_db import TenantIsolationError, current_tenant_id, tenant_context
@@ -68,3 +70,32 @@ def test_task_without_tenant_runs_unscoped():
     restore_tenant(task_id="t3", task=SimpleNamespace(request=SimpleNamespace()))
     assert current_tenant_id.get() is None
     release_tenant(task_id="t3")  # no token recorded — must be a no-op
+
+
+def test_headerless_worker_task_enqueues_without_an_owner():
+    restore_tenant(task_id="t4", task=SimpleNamespace(request=SimpleNamespace()))
+    try:
+        headers: dict = {}
+        stamp_owner(headers)
+        assert OWNER_HEADER not in headers
+    finally:
+        release_tenant(task_id="t4")
+
+
+@pytest.mark.parametrize("next_owner", [None, "next-user"])
+def test_stranded_prerun_never_reaches_the_next_task(next_owner):
+    """#861: a prerun without its postrun must not decide the next task's owner."""
+    stale = SimpleNamespace(request=SimpleNamespace(**{OWNER_HEADER: "stale-user"}))
+    restore_tenant(task_id="t5", task=stale)  # its postrun never runs
+    request = SimpleNamespace(**({OWNER_HEADER: next_owner} if next_owner else {}))
+    restore_tenant(task_id="t6", task=SimpleNamespace(request=request))
+    try:
+        headers: dict = {}
+        stamp_owner(headers)
+        assert headers.get(OWNER_HEADER) == next_owner
+    finally:
+        release_tenant(task_id="t6")
+        release_tenant(task_id="t5")  # unwinds the stranded binding too
+    headers = {}
+    stamp_owner(headers)
+    assert OWNER_HEADER not in headers

modules/tenants/tests/core/service/test_tenants_owner_race.py

@@ -126,7 +126,13 @@ async def invite() -> str:
                     return exc.code
 
         outcomes = await asyncio.gather(*(invite() for _ in range(5)))
-        assert sorted(outcomes) == ["already_invited"] * 4 + ["ok"]
+        assert outcomes.count("ok") == 1
+        # A loser on SQLite can also get SQLITE_BUSY on the writer lock under load
+        # (the retryable ``admission_conflict``): still a refusal (#855).
+        refusals = {"already_invited"} | (
+            {"admission_conflict"} if backend != "postgres" else set()
+        )
+        assert {o for o in outcomes if o != "ok"} <= refusals, outcomes
         async with state.session_factory() as db:
             pending = await InvitationService(TenantService(db), ttl_hours=1).list_pending(
                 tenant.id

modules/users/tests/core/password_hashing/test_users_password_offload.py

@@ -29,14 +29,18 @@ async def test_crypto_runs_in_worker_thread(users_app, monkeypatch, operation):
         token = await manager.generate_reset_password_token(user)
         loop_thread = threading.get_ident()
         crypto_threads = []
+        # Patch the class, not the shared ``PASSWORD_HELPER`` instance: undoing an
+        # instance patch leaves a bound method in its ``__dict__`` that shadows
+        # every later class-level patch (#860).
+        helper_cls = type(manager.password_helper)
         for name in ("hash", "verify_and_update"):
-            original = getattr(manager.password_helper, name)
+            original = getattr(helper_cls, name)
 
-            def record(*args, _original=original):
+            def record(self, *args, _original=original):
                 crypto_threads.append(threading.get_ident())
-                return _original(*args)
+                return _original(self, *args)
 
-            monkeypatch.setattr(manager.password_helper, name, record)
+            monkeypatch.setattr(helper_cls, name, record)
         original_execute = db.execute
 
         async def execute(*args, **kwargs):
@@ -63,6 +67,13 @@ async def execute(*args, **kwargs):
         assert loop_thread not in crypto_threads
 
 
+def test_shared_helper_carries_no_instance_patches():
+    """#860: tests must not leave instance attributes on the shared helper."""
+    from users.core.password_hashing import PASSWORD_HELPER
+
+    assert not {"hash", "verify_and_update"} & vars(PASSWORD_HELPER).keys()
+
+
 async def test_managers_share_password_helper(users_app):
     async with users_app.state.sm.db.session_factory() as db:
         assert (

skills/simple-module-conventions/SKILL.md

@@ -39,7 +39,7 @@ Treat 300 lines as the design pressure on every `.py`, `.ts`, and `.tsx` file (t
 New module settings and their app-state container live in `core/settings.py`
 and `core/services.py`; HTTP dependencies and services live in `web/`. Keep
 root lookup files in place and put non-page frontend code in `ui/`. See
-[Module layout](../../docs/module-layout.md); existing modules migrate one at a time.
+[Module layout](../../docs/module-layout.md).
 
 ### 3. Per-module settings: `register_module_settings` → `app.state.<module>`
 

skills/simple-module-creating/SKILL.md

@@ -62,7 +62,7 @@ root `pyproject.toml` (type-check + test paths) and the `publish-pypi` matrix in
 test` without further wiring. Commit the `uv.lock`, `package-lock.json` and
 regenerated i18n key files it leaves behind.
 
-New modules use this area layout; existing modules migrate one at a time.
+Modules use this area layout.
 Area initializers hold only a docstring. Private helpers stay in their area.
 Tests have no `__init__.py`, keep `conftest.py` at their root and use globally
 unique `test_orders_*.py` basenames; JS tests stay in `tests-js/`.