fix(coord): strict-mode loader + guarded shutdown for coord_registry refresh

Two correctness follow-ups from the multi-stage code review on #453.

bug-2 / perf-2 (DB probe was theatre + double scan)
The previous probe defended nothing the loader didn't already swallow
on the next line: ``load_model_registry``'s row-loop catches Exception
internally, so a transient DB error after the probe still degrades to
a config.toml-only registry that ``existing.reload()`` would apply,
silently dropping every DB-sourced alias.  And on the happy path each
CRUD paid for two scans of ``model_definitions``.

Add a ``strict: bool = False`` flag to ``load_model_registry``.  When
strict, the row-loop's except re-raises instead of swallowing.  The
helper passes ``strict=True`` and drops the probe — single DB scan,
real failure isolation, the loader's silent fallback can no longer
mask a partial-result regression.  Default ``strict=False`` so CLI /
lifespan callers keep their boot-with-config-fallback behaviour.

bug-1 (shutdown could escape after a successful reload)
``ModelRegistry.shutdown()`` calls ``client.close()`` unguarded, and the
helper's ``finally`` block ran it outside the try/except.  A raising
close() after a successful ``existing.reload()`` would surface as 500
with the registry already mutated and the audit row already recording
success.  Wrap ``new_registry.shutdown()`` in its own try/except that
matches the helper's belt-and-suspenders error policy elsewhere.

The helper's docstring also drops the obsolete probe paragraph; the
``if existing is None: return`` branch gets a one-line inline comment
about the boot-from-empty case (the multi-paragraph version restated
behaviour the line itself documents).

129 tests pass (test_admin_model_registry_refresh + test_model_registry).

turnstone/console/server.py

@@ -8001,11 +8001,10 @@ def _refresh_console_coord_registry(app_state: Any, storage: Any) -> None:
 
     The console-side coordinator session factory closes over the
     ``coord_registry`` instance built at lifespan startup
-    (see ``console/server.py``'s lifespan setup and
-    ``console/session_factory.py``).  Replacing the attribute would
-    orphan the closure — new sessions would still resolve through the
-    stale object.  Mutating in place via ``ModelRegistry.reload()``
-    preserves identity, so:
+    (see this module's lifespan setup and ``console/session_factory.py``).
+    Replacing the attribute would orphan the closure — new sessions would
+    still resolve through the stale object.  Mutating in place via
+    ``ModelRegistry.reload()`` preserves identity, so:
 
     - new coordinator sessions see the new state at create-time;
     - active coordinator sessions auto-pick up the swap at next ``send()``
@@ -8013,43 +8012,27 @@ def _refresh_console_coord_registry(app_state: Any, storage: Any) -> None:
       check compares ``cfg.model`` against ``self.model`` and re-resolves
       on mismatch).
 
-    No-op when ``coord_registry`` is ``None``.  Lifespan only sets it
-    when DB model rows existed at boot; with no rows the entire coord
-    subsystem stays disabled (no ``coord_mgr``, no ``coord_adapter``,
-    no ``session_factory`` — see the lifespan setup in this module).
-    Bootstrapping that whole stack on the fly is out of scope for this
-    helper, so a console restart remains required after the operator
-    adds the first model row.
-
     Errors are logged + swallowed.  The DB write that triggered this
     refresh has already succeeded, and the explicit reload button
     remains the user-facing recovery path.  Validation failures
     (e.g. admin deleted the alias that ``registry.default`` points at)
     leave the existing registry intact rather than tearing down a
     working coordinator.
-
-    DB read failures are detected by an explicit probe before calling
-    ``load_model_registry``: the loader swallows storage errors
-    internally and would otherwise return a config.toml-only registry,
-    which would silently drop DB-sourced aliases when applied via
-    ``ModelRegistry.reload``.
     """
     from turnstone.core.model_registry import load_model_registry
 
     existing = getattr(app_state, "coord_registry", None)
     if existing is None:
-        return
-    # Strict DB probe — load_model_registry swallows storage errors
-    # internally (logs + continues with config.toml-only models).  Without
-    # this fail-fast, a transient DB outage would let the helper apply a
-    # truncated registry that drops every DB-sourced alias.
-    try:
-        storage.list_model_definitions(enabled_only=True)
-    except Exception:
-        log.warning("console.coord_registry_refresh_db_probe_failed", exc_info=True)
+        # Lifespan didn't build a coord_registry (no DB model rows at boot)
+        # — the entire coord subsystem stayed uninitialized, so a console
+        # restart is required after the operator adds the first row.
         return
     try:
-        new_registry = load_model_registry(storage=storage)
+        # ``strict=True`` so a transient DB read error surfaces here.
+        # Without it, the loader degrades to a config.toml-only registry
+        # and ``existing.reload()`` would silently drop every DB-sourced
+        # alias.
+        new_registry = load_model_registry(storage=storage, strict=True)
     except ValueError:
         # All rows disabled/deleted — ModelRegistry.__init__ rejects an
         # empty model dict.  Leave the existing registry in place so
@@ -8074,9 +8057,14 @@ def _refresh_console_coord_registry(app_state: Any, storage: Any) -> None:
     except Exception:
         log.warning("console.coord_registry_refresh_reload_failed", exc_info=True)
     finally:
-        # Close any clients the throwaway registry created during DB
-        # load, regardless of whether the in-place reload succeeded.
-        new_registry.shutdown()
+        # Close any clients the throwaway registry created during DB load.
+        # An exception here would otherwise escape after a successful
+        # in-place reload, surfacing as 500 with the registry actually
+        # mutated and the audit row recording success.
+        try:
+            new_registry.shutdown()
+        except Exception:
+            log.warning("console.coord_registry_refresh_shutdown_failed", exc_info=True)
 
 
 async def _notify_nodes_model_reload(request: Request) -> dict[str, Any]:

turnstone/core/model_registry.py

@@ -300,6 +300,7 @@ def load_model_registry(
     context_window: int = 32768,
     provider: str = "openai",
     storage: Any | None = None,
+    strict: bool = False,
 ) -> ModelRegistry:
     """Build a ModelRegistry from CLI args, ``config.toml``, and database.
 
@@ -317,6 +318,15 @@ def load_model_registry(
        ``[model].plan_effort``, ``[model].task_effort`` control routing.
        ``plan_model``/``task_model`` override ``agent_model`` per sub-agent
        role; both fall back to it when unset.
+
+    ``strict``: when True, a storage read failure during the DB-rows step
+    re-raises instead of degrading to a config.toml-only registry.
+    Callers that hot-reload an existing registry need this so a transient
+    DB outage doesn't silently drop every DB-sourced alias when the
+    truncated result is applied via ``ModelRegistry.reload``.  Callers
+    that build a fresh registry from scratch (CLI, lifespan startup) want
+    the default behaviour — boot succeeds with a config-only fallback
+    rather than crashing on a flaky DB.
     """
     import json as _json
 
@@ -370,6 +380,8 @@ def load_model_registry(
                     server_compat=row_server_compat,
                 )
         except Exception:
+            if strict:
+                raise
             log.warning("Failed to load model definitions from storage", exc_info=True)
 
     # 2. Build configs from [models.*] sections (overrides DB for same alias)