Tech-debt batch 5: one is_public() for every public-route check; framework hygiene (#593 #594)

CHANGELOG.md

@@ -22,6 +22,17 @@ All notable changes to this project are documented in this file. The format is b
 - `make test-py` / `make test-py-pg` now default to 4 pytest-xdist workers instead of
   `-n auto`, so concurrent runs on many-core machines don't exhaust memory;
   `PYTEST_WORKERS=auto` restores the old behaviour.
+- Hosting: one `simple_module_hosting.public_access.is_public(app, method, path)` answers
+  "is this route public?" for `AuthMiddleware`, maintenance mode, rate limiting and the
+  `SM031` check, so they can no longer disagree. A legacy auth provider whose deprecated
+  `get_public_paths()` raises is now logged everywhere instead of silently treated as not
+  public in route access (#593).
+- Framework: fail-open error handlers in the setup gate, DB listeners, menu providers, plural
+  rules and the invalidation decoder now log a WARNING (once) with the traceback instead of
+  failing silently; outcomes are unchanged. The policy is in `docs/framework-conventions.md`.
+  The test plugin resets the process-wide mail sender, lease backend and Sentry state per test. The
+  `register_event_handlers(app=None)` default and the `requires_framework=None` bypass are
+  tracked for removal in 0.1.0 (#594).
 
 ### Fixed
 - Tenants: deleting a tenant now tears down its data. `TenantDeleted` (now defined in

docs/framework-conventions.md

@@ -739,3 +739,15 @@ The active locale lands on `request.state.locale`. The `<LocaleSwitcher />` comp
 App boot runs `I18nDiagnostics` against every module's declared locale dirs. See codes `SM013`–`SM016` in the table above. Warnings are printed in dev; errors fail the boot in production.
 
 `make doctor` runs the same checks, and it runs them whether or not `SM_I18N_SUPPORTED_LOCALES` is set. With the variable unset each namespace is checked against the locale files it actually ships: SM014/SM015 still hold every translation to the default catalog's key set, while SM013 stays quiet, since a locale nothing promised cannot be missing. Declaring the set turns SM013 back on — that is the check that reports a module which never shipped one of the locales the install claims to support.
+
+## Error handling policy
+
+Unexpected exceptions retained as fallbacks must log at WARNING or higher with
+a traceback (`exc_info=True`). Recurring request/transport failures warn on the
+first occurrence per process; menu providers warn once per provider per registry.
+Deliberate fail-open sites are audit identity inspection (`db/listeners.py`,
+omit the unavailable primary key), menu providers (`core/menu.py`, omit that
+provider's items), plural locale parsing (`core/_i18n_translator.py`, use `other`),
+and invalidation decoding (`core/invalidation.py`, drop the malformed message
+and keep listening). The migration re-check (`hosting/setup_gate.py`) fails
+closed: retain the incomplete boot snapshot and keep the setup gate active.

docs/superpowers/specs/2026-05-27-pluggable-auth-keycloak-design.md

@@ -1,5 +1,9 @@
 # Pluggable Auth: AuthProvider Contract + Keycloak Module
 
+> **Superseded:** Guidance below to implement `get_public_paths()` is obsolete. Use
+> `ModuleBase.register_public_routes` and the method-aware `PublicRouteRegistry`;
+> see [Public routes](../../framework/public-routes.md). The legacy shim is removed in 0.1.0.
+
 **Date:** 2026-05-27
 **Status:** Design — approved for implementation planning
 **Builds on:** [2026-05-21 Auth principal-resolver chain](2026-05-21-auth-principal-resolver-design.md) (must land first)

docs/superpowers/specs/hifi-gap/auth-screens-02-07.md

@@ -1,5 +1,9 @@
 # Hi-Fi deck gap analysis — auth-screens-02-07
 
+> **Superseded:** Guidance below to implement `get_public_paths()` is obsolete. Use
+> `ModuleBase.register_public_routes` and the method-aware `PublicRouteRegistry`;
+> see [Public routes](../../../framework/public-routes.md). The legacy shim is removed in 0.1.0.
+
 Generated 2026-09-02 from the cached deck (fetched 2026-08-19) vs main @ a8ab6bb. Read-only findings; decisions live in ../2026-09-03-hifi-pages-design.md.
 
 # Design-vs-implementation gap analysis: auth screens (02–07)

framework/core/simple_module_core/_i18n_translator.py

@@ -14,6 +14,7 @@
 # Logged under the historical ``simple_module_core.i18n`` name so existing log
 # filters keep matching after the split.
 logger = logging.getLogger("simple_module_core.i18n")
+_plural_failure_warned = False
 
 
 @lru_cache(maxsize=64)
@@ -30,6 +31,10 @@ def _plural_form(locale: str, count: float) -> str:
     try:
         rule = _plural_rule(locale)
     except Exception:
+        global _plural_failure_warned
+        if not _plural_failure_warned:
+            _plural_failure_warned = True
+            logger.warning("Plural locale parsing failed; using other", exc_info=True)
         return "other"
     return rule(count)
 

framework/core/simple_module_core/invalidation.py

@@ -48,6 +48,7 @@
 from typing import Protocol, runtime_checkable
 
 logger = logging.getLogger(__name__)
+_decode_failure_warned = False
 
 __all__ = [
     "WIRE_VERSION",
@@ -108,8 +109,8 @@ def from_wire(cls, message: str | bytes) -> Invalidation | None:
         routinely shared — with another app, with a stray ``redis-cli publish``,
         with the previous version of this code — and a listener loop that dies
         on one malformed payload takes cross-process invalidation down for the
-        life of the process. Logged at debug: on a shared database this is
-        normal traffic, not a fault.
+        life of the process. Decode exceptions warn once per process with a
+        traceback; unsupported shapes remain ordinary shared-database traffic.
         """
         try:
             data = json.loads(message)
@@ -123,7 +124,10 @@ def from_wire(cls, message: str | bytes) -> Invalidation | None:
                 return None
             return cls(channel=channel, key=key, origin=str(data.get("origin") or ""))
         except Exception:
-            logger.debug("Ignoring undecodable invalidation message: %r", message)
+            global _decode_failure_warned
+            if not _decode_failure_warned:
+                _decode_failure_warned = True
+                logger.warning("Ignoring undecodable invalidation message", exc_info=True)
             return None
 
 

framework/core/simple_module_core/leases.py

@@ -43,6 +43,7 @@
     "configure_leases",
     "get_leases",
     "lease",
+    "reset_lease_backend",
     "semaphore",
 ]
 
@@ -153,6 +154,11 @@ def configure_leases(backend: LeaseBackend | None) -> None:
     _backend = backend if backend is not None else InProcessLeaseBackend()
 
 
+def reset_lease_backend() -> None:
+    """Discard process lease configuration and held in-process leases."""
+    configure_leases(None)
+
+
 def get_leases() -> LeaseBackend:
     """The process-wide backend :func:`lease` and :func:`semaphore` use."""
     return _backend

framework/core/simple_module_core/mail.py

@@ -93,6 +93,12 @@ def install_mail_resolver(resolver: MailSenderResolver | None) -> None:
     _resolver = resolver
 
 
+def reset_mail_sender() -> None:
+    """Clear the process sender and resolver between isolated app/test runs."""
+    install_mail_sender(None)
+    install_mail_resolver(None)
+
+
 def mail_transport_installed() -> bool:
     """Whether a transport (sender or resolver) is installed in this process.
 

framework/core/simple_module_core/menu.py

@@ -85,6 +85,7 @@ def __init__(self) -> None:
         self._items: list[MenuItem] = []
         self._sorted: list[MenuItem] | None = None
         self._providers: list[MenuProvider] = []
+        self._failed_providers: set[int] = set()
 
     def _invalidate(self) -> None:
         self._sorted = None
@@ -125,7 +126,11 @@ async def collect_provider_items(self, request: Request) -> list[MenuItem]:
                 # Materialise first: a generator failing midway must add nothing.
                 items.extend(list(result))
             except Exception:
-                logger.exception("Menu provider %r failed; contributing nothing", provider)
+                if id(provider) not in self._failed_providers:
+                    self._failed_providers.add(id(provider))
+                    logger.warning(
+                        "Menu provider %r failed; contributing nothing", provider, exc_info=True
+                    )
         return items
 
     @property

framework/core/simple_module_core/module.py

@@ -25,6 +25,9 @@
     from simple_module_core.spa_mount import SpaMount
 
 
+EVENT_HANDLERS_OPTIONAL_APP_REMOVE_IN = "0.1.0"
+
+
 class ModuleBase(RequestHooksMixin, ABC):
     """Base class for all modules.
 
@@ -75,7 +78,9 @@ def register_feature_flags(self, registry: FeatureFlagRegistry) -> None:
     def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> None:
         """Subscribe to events published by other modules.
 
-        ``app`` is optional for back-compat; pass it through to handlers
+        The optional ``app=None`` compatibility default has
+        ``remove_in="0.1.0"``. Overrides own the call, so the base class cannot
+        reliably warn when callers omit it. Pass ``app`` through to handlers
         that need ``app.state.sm.db.session_factory`` to persist on the
         framework's engine instead of building their own.
         """

framework/core/simple_module_core/module_meta.py

@@ -50,7 +50,9 @@ class ModuleMeta:
 
     Example: ``">=1.0,<2.0"``. When set, the module is rejected at boot if the
     installed ``simple_module_core.FRAMEWORK_API_VERSION`` does not satisfy it.
-    When ``None``, no compatibility check is performed (legacy modules).
+    When ``None``, no compatibility check is performed (legacy modules);
+    this bypass is deprecated with ``remove_in="0.1.0"`` (tracked by the
+    deprecation-expiry test; it does not warn at boot).
     """
     i18n_audience: str = "public"
     """Who this module's locale catalog is shipped to: ``"public"`` or ``"admin"``.

framework/core/simple_module_core/versioning.py

@@ -8,6 +8,7 @@
 from packaging.specifiers import InvalidSpecifier, SpecifierSet
 from packaging.version import Version
 
+from simple_module_core.deprecation import deprecate
 from simple_module_core.exceptions import FrameworkVersionError
 from simple_module_core.module import ModuleBase
 
@@ -28,6 +29,13 @@
 _FRAMEWORK_VERSION = Version(FRAMEWORK_API_VERSION)
 
 
+_UNVERSIONED_MODULE = deprecate(
+    "ModuleMeta.requires_framework=None compatibility bypass",
+    remove_in="0.1.0",
+    use="ModuleMeta(requires_framework=...) with an explicit compatible range",
+)
+
+
 def check_framework_compatibility(
     modules: Sequence[ModuleBase],
     framework_version: str = FRAMEWORK_API_VERSION,
@@ -48,6 +56,9 @@ def check_framework_compatibility(
     for mod in modules:
         spec_str = mod.meta.requires_framework
         if spec_str is None:
+            # Tracked for removal (``_UNVERSIONED_MODULE``) but not warned per
+            # module: every unversioned module, in-repo ones included, would
+            # warn on each boot.
             continue
         try:
             spec = SpecifierSet(spec_str)

framework/db/simple_module_db/listeners.py

@@ -42,6 +42,7 @@
 from simple_module_db.tenancy import TenantIsolationError, current_tenant_id
 
 logger = logging.getLogger(__name__)
+_entity_pk_failure_warned = False
 _db_logger = logging.getLogger("simple_module.db")
 
 # Set by auth middleware on each request
@@ -82,6 +83,10 @@ def _entity_pk(obj: object) -> object:
             return identity[0]
         return identity
     except Exception:
+        global _entity_pk_failure_warned
+        if not _entity_pk_failure_warned:
+            _entity_pk_failure_warned = True
+            logger.warning("Audit identity inspection failed; omitting primary key", exc_info=True)
         return None
 
 

framework/hosting/simple_module_hosting/_body_limit.py

@@ -22,7 +22,7 @@
 from starlette.requests import Request
 from starlette.types import ASGIApp, Message, Receive, Scope, Send
 
-from simple_module_hosting._error_handlers import _wants_json
+from simple_module_hosting._error_negotiation import _wants_json
 from simple_module_hosting._request_guard_responses import send_guard_response
 
 logger = logging.getLogger("simple_module.request_guard")

framework/hosting/simple_module_hosting/_deferred_startup.py

@@ -0,0 +1,64 @@
+"""Settings hydration and deferred startup replay, independent of setup gates."""
+
+from __future__ import annotations
+
+import asyncio
+import logging
+
+from fastapi import FastAPI
+from simple_module_core.module_settings import get_module_settings_registry
+from simple_module_db.stored_settings import SessionOverrideSource, settings_table_exists
+
+from simple_module_hosting._hydrate_step import hydrate_all
+
+logger = logging.getLogger(__name__)
+
+
+async def hydrate_settings_from_db(app: FastAPI) -> None:
+    """Merge DB-stored overrides into every registered settings object.
+
+    Reads the settings table directly (``simple_module_db.stored_settings``),
+    so it works whether or not the ``settings`` plugin is enabled (#500). A
+    missing table is a fresh install: nothing is stored yet, so there is
+    nothing to merge.
+
+    Note this is the *second* settings read of a boot. The first happens in
+    ``_preapp_config`` before ``create_app`` builds anything, because the
+    module list, i18n registry and middleware stack are all constructed from
+    settings and cannot be rebuilt from here.
+    """
+    if get_module_settings_registry(app) is None:
+        return
+
+    async with app.state.sm.db.session_factory() as session:
+        if not await settings_table_exists(session):
+            return
+        await hydrate_all(app, SessionOverrideSource(session), session)
+
+
+async def run_deferred_startup(app: FastAPI) -> None:
+    """Replay the ``on_startup`` hooks that could not run on an unmigrated DB.
+
+    Called from the wizard's migrations action and from the setup gate's
+    schema re-check, so a worker that did not itself run the migrations (or an
+    operator's out-of-band ``make migrate``) still finishes starting its
+    modules. Serialised per app; a hook that fails again is logged and dropped
+    rather than raised, because this runs after the migrations have already
+    committed and a 500 there would help nobody.
+    """
+    deferred = getattr(app.state, "deferred_startup", None)
+    if not deferred:
+        return
+    lock = getattr(app.state, "deferred_startup_lock", None)
+    if lock is None:  # no await between check and set, so this cannot race
+        lock = app.state.deferred_startup_lock = asyncio.Lock()
+    async with lock:
+        if not deferred:
+            return
+        await hydrate_settings_from_db(app)
+        while deferred:
+            mod = deferred.pop(0)
+            try:
+                await mod.on_startup(app)
+            except Exception:
+                logger.exception("Deferred on_startup of %s failed after migrations", mod.meta.name)