feat(sync): fail fast with a modal when a model is unavailable

TODO.md L4: with a dead model endpoint the sync discovered it only mid-import, after slow clones — and a tooltip on the button is not a readable error. - app/rag/llm.py: ModelUnavailableError + check_models(llm) — a tiny pre-sync probe (one short embedding + one 1-token-scale completion) that fails naming the unavailable model (embed first, then the summary model); the sync sanitizer still masks credentials. - app/api/sync.py: the probe is step 1 of _run_sync — before source resolution and before any clone_or_pull; a model failure is just another 'failed' state (no new endpoint, A10/A12 untouched). - frontend/assets/header.js: applySyncFailure now also opens the module-owned error modal (every page carrying #sync-btn, zero page-markup changes): lazily built backdrop + role=alertdialog panel, error text via textContent, close via button / Esc / backdrop, focus in-and-out to #sync-btn (with a body→#sync-btn fallback — the run's disabled button drops focus to <body>). - frontend/assets/styles.css: the modal on the phase-08 error palette (z-index above the header, .is-open open/close, reduced-motion stilling, 44px close target). - Tests: probe unit tests (both up / embed down / summary down / custom model names), sync integration (fail-fast before any clone, probe-before-effective_sources ordering, credential masking, healthy regression), the phase-41 source pins, and the story E2E (two module apps on distinct ports — dead endpoint on a closed loopback port vs session mock: ≤10 s fail-fast + modal contract, all three dismissal paths with focus out to #sync-btn, button title/.is-error + Sources banner untouched, healthy phase-32 lifecycle regression to 'Synced HH:MM'). E2E (isolation): test_sync_model_down.py 4/4, test_sync_button.py 3/3, test_git_sources_admin.py 6/6, test_local_directory_sources.py 3/3; unit+integration 721 passed, app/ coverage 99%; ruff + pyright clean.
2026-08-27 23:44:35 -04:00
parent 6f9e033117
commit 6cf1df9bf2
9 changed files with 1203 additions and 12 deletions
@@ -327,6 +327,104 @@ def test_header_js_emits_bor_sync_status_on_state_changes() -> None:
    assert "emitSyncStatus(status)" in _body(js, "initSyncButton")


+# ---------- header.js: the sync failure modal (phase 41, TODO.md L4) ----------
+
+
+def test_header_js_owns_a_lazily_created_sync_modal() -> None:
+    """The modal is module-owned and created lazily ONCE (module-level
+    `let syncModal = null`): a .sync-modal-backdrop holding a .sync-modal
+    panel with role="alertdialog" + aria-modal + labelled/described ids
+    + the close button, appended to document.body — no page-markup
+    changes, so every page carrying #sync-btn gets it. The module
+    docstring's sync bullet records the modal (phase 41)."""
+    js = _text(HEADER_JS)
+    assert "let syncModal = null" in js, "module-level once-only modal ref"
+    assert 'role="alertdialog"' in js
+    assert 'aria-modal="true"' in js
+    assert 'aria-labelledby="sync-modal-title"' in js
+    assert 'aria-describedby="sync-modal-error"' in js
+    assert "module-owned error modal" in js, "the docstring sync bullet"
+    create = _body(js, "createSyncModal")
+    assert "sync-modal-backdrop" in create
+    assert 'document.body.appendChild(backdrop)' in create
+    assert "return backdrop" in create, "the module ref must hold the created element"
+    assert "Sync failed" in create, "the dialog title"
+    assert 'class="sync-modal-close"' in create
+    assert 'aria-label="Close error dialog"' in create
+
+
+def test_sync_modal_error_is_rendered_via_text_content() -> None:
+    """The error text is ALWAYS set via textContent (XSS-safe — no
+    innerHTML with user data in the open path), and it is set BEFORE
+    the already-open check, so a second failure while open updates the
+    text IN PLACE (no stacking, no focus jump)."""
+    js = _text(HEADER_JS)
+    body = _body(js, "showSyncModal")
+    assert 'querySelector("#sync-modal-error").textContent' in body
+    assert "innerHTML" not in body, "the open path never touches innerHTML"
+    text = body.find("textContent")
+    open_check = body.find('contains("is-open")')
+    assert text != -1 and open_check != -1 and text < open_check, (
+        "the in-place update happens while the modal is already open"
+    )
+
+
+def test_sync_modal_focus_goes_in_and_out_to_sync_btn() -> None:
+    """On open: document.activeElement is remembered and focus moves
+    to the close button — with a fallback to #sync-btn when the active
+    element is <body> (the run's disabled button dropped focus there;
+    the close must still land on the control that started the run);
+    on close: focus returns to the remembered element (guarded by
+    document.contains — a detached target is a no-op)."""
+    js = _text(HEADER_JS)
+    open_body = _body(js, "showSyncModal")
+    assert "const active = document.activeElement" in open_body
+    assert "active !== document.body" in open_body, (
+        "the disabled-button window leaves focus on <body> — the fallback"
+    )
+    assert "? active : syncBtn" in open_body, ("the fallback remembers #sync-btn")
+    assert 'querySelector(".sync-modal-close").focus()' in open_body
+    close_body = _body(js, "closeSyncModal")
+    assert "document.contains(target)" in close_body
+    assert "target.focus()" in close_body
+    assert 'contains("is-open")' in close_body, "closing a closed modal is a no-op"
+
+
+def test_sync_modal_closes_via_button_esc_and_backdrop() -> None:
+    """All three dismissal paths call the SAME close function: the
+    close button, Esc (ONE document keydown binding, acting only while
+    the modal is open), and a click on the backdrop element itself —
+    the event.target check keeps clicks bubbling from the panel from
+    closing it."""
+    js = _text(HEADER_JS)
+    create = _body(js, "createSyncModal")
+    assert 'addEventListener("click", closeSyncModal)' in create
+    assert 'e.key === "Escape"' in create
+    assert 'addEventListener("keydown"' in create
+    assert 'backdrop.classList.contains("is-open")' in create
+    assert "e.target === backdrop" in create
+
+
+def test_apply_sync_failure_opens_the_modal_after_the_event() -> None:
+    """applySyncFailure opens the modal with the SANITIZED error, and
+    the call comes AFTER the existing button title/aria/.is-error
+    lines + emitSyncStatus (the bor:sync-status event the Sources
+    banner renders off — those lines stay byte-identical; the modal is
+    additive)."""
+    js = _text(HEADER_JS)
+    body = _body(js, "applySyncFailure")
+    call = body.find("showSyncModal(")
+    emit = body.find("emitSyncStatus(status)")
+    assert call != -1, "the modal must open from the failure path"
+    assert emit != -1 and call > emit, "the event still emits first (byte-identical)"
+    assert "showSyncModal(error)" in body, "the sanitized error goes to the modal"
+    # The pre-existing affordances stay (the Sources banner contract).
+    assert "syncBtn.title = error" in body
+    assert 'syncBtn.setAttribute("aria-label", error)' in body
+    assert 'syncBtn.classList.add("is-error")' in body
+    assert "emitSyncStatus(status)" in body
+
+
 # ---------- sources.js: the event-driven result line + banner ----------


@@ -415,3 +513,61 @@ def test_sync_result_is_styled() -> None:
    block = re.search(r"\.sync-result\s*\{([^}]*)\}", css)
    assert block, "styles.css must define .sync-result"
    assert "var(--ink-soft)" in block.group(1)
+
+
+# ---------- styles.css: the sync failure modal (phase 41, TODO.md L4) ----------
+
+
+def test_sync_modal_css_error_palette_and_stacking() -> None:
+    """.sync-modal-backdrop: fixed, full-viewport, rgba dim, z-index
+    above the sticky header; .sync-modal: the centered ≈28rem panel on
+    the phase-08 error palette (panel on --err-bg, 1px --err-line
+    border, --err-ink error text, --ink title — all computed ≥4.5:1);
+    open/close via .is-open (visibility/opacity)."""
+    css = _text(STYLES_CSS)
+    backdrop = re.search(r"\.sync-modal-backdrop\s*\{([^}]*)\}", css)
+    assert backdrop, "styles.css must define .sync-modal-backdrop"
+    b = backdrop.group(1)
+    assert "position: fixed" in b
+    assert "inset: 0" in b
+    assert "z-index: 1000" in b, "above the sticky header (20) + skip-link (100)"
+    assert "rgba(" in b, "the dim over the page"
+    open_state = re.search(r"\.sync-modal-backdrop\.is-open\s*\{([^}]*)\}", css)
+    assert open_state, ".is-open must be the open state"
+    assert "visibility: visible" in open_state.group(1)
+    assert "opacity: 1" in open_state.group(1)
+    panel = re.search(r"\.sync-modal\s*\{([^}]*)\}", css)
+    assert panel, "styles.css must define .sync-modal"
+    p = panel.group(1)
+    assert "max-width: 28rem" in p
+    assert "var(--err-bg)" in p
+    assert "var(--err-line)" in p
+    title = re.search(r"#sync-modal-title\s*\{([^}]*)\}", css)
+    assert title, "the modal title must be styled"
+    assert "var(--ink)" in title.group(1)
+    error = re.search(r"#sync-modal-error\s*\{([^}]*)\}", css)
+    assert error, "the modal error line must be styled"
+    assert "var(--err-ink)" in error.group(1)
+
+
+def test_sync_modal_close_button_touch_floor() -> None:
+    """The close button keeps the 44px touch floor at every width (the
+    global 3px :focus-visible outline applies — no per-button
+    override)."""
+    css = _text(STYLES_CSS)
+    block = re.search(r"\.sync-modal-close\s*\{([^}]*)\}", css)
+    assert block, "styles.css must define .sync-modal-close"
+    assert "min-width: 44px" in block.group(1)
+    assert "min-height: 44px" in block.group(1)
+
+
+def test_sync_modal_respects_reduced_motion() -> None:
+    """The open/close fade is stilled under prefers-reduced-motion —
+    the phase-25 / doc-modal opt-out pattern."""
+    css = _text(STYLES_CSS)
+    reduced = re.search(
+        r"@media \(prefers-reduced-motion: reduce\)\s*\{\s*\.sync-modal-backdrop\s*\{([^}]*)\}",
+        css,
+    )
+    assert reduced, "the backdrop fade must opt out under prefers-reduced-motion"
+    assert "transition: none" in reduced.group(1)
@@ -0,0 +1,199 @@
+"""Unit tests: the phase-41 pre-sync model probe (``check_models``).
+
+The probe verifies both models a sync needs — ``embed`` (one short
+embedding) and the summary model ``lite`` (one 1-token-scale
+completion) — before any expensive work, and fails with a
+: class:`ModelUnavailableError` that **names the dead model**. The fake
+client is duck-typed (``embed_one`` / ``chat`` + ``settings``), same
+shape as :class:`tests.fakes.FakeEmbedder` — no network, no httpx
+transport.
+
+Covered paths (keep ``app/`` >90%):
+- both models up → returns, both methods called exactly once;
+- embedding probe raises → ``ModelUnavailableError`` naming the
+  **embedding model**, and the summary probe is **never** reached;
+- summary probe raises → ``ModelUnavailableError`` naming the
+  **summary model**;
+- message content: model name present, "not available" wording, the
+  original exception chained as ``__cause__``;
+- the error is an ``LLMError`` (the sync's generic failure catch).
+"""
+from __future__ import annotations
+
+import asyncio
+from typing import Any, cast
+
+import pytest
+
+from app.config import Settings
+from app.rag.llm import (
+    EmbeddingError,
+    LLMClient,
+    LLMError,
+    ModelUnavailableError,
+    check_models,
+)
+
+
+class _ProbeClient:
+    """Duck-typed :class:`app.rag.llm.LLMClient` stand-in for the probe."""
+
+    def __init__(
+        self,
+        embed_error: Exception | None = None,
+        chat_error: Exception | None = None,
+        embed_model: str = "embed",
+        summary_model: str = "lite",
+    ) -> None:
+        kwargs: dict[str, Any] = {
+            "_env_file": None,
+            "llm_embed_model": embed_model,
+            "llm_summary_model": summary_model,
+        }
+        self.settings = Settings(**kwargs)  # pyright: ignore[reportCallIssue]
+        self.embed_error = embed_error
+        self.chat_error = chat_error
+        self.embed_texts: list[str] = []
+        self.chat_messages: list[list[dict[str, str]]] = []
+
+    async def embed_one(self, text: str) -> list[float]:
+        self.embed_texts.append(text)
+        if self.embed_error is not None:
+            raise self.embed_error
+        return [0.0] * 768
+
+    async def chat(self, messages: list[dict[str, str]], model: str | None = None) -> str:
+        self.chat_messages.append(list(messages))
+        if self.chat_error is not None:
+            raise self.chat_error
+        return "pong"
+
+
+def _run_check(llm: _ProbeClient) -> Any:
+    # The probe is duck-typed at the ``embed_one`` / ``chat`` surface — the
+    # cast keeps pyright honest about the narrow stand-in.
+    return asyncio.run(check_models(cast("LLMClient", llm)))
+
+
+# --- both models up --------------------------------------------------------
+
+
+def test_both_models_up_returns_and_calls_both() -> None:
+    llm = _ProbeClient()
+
+    assert _run_check(llm) is None
+    assert llm.embed_texts == ["sync model check"]  # the tiny probe input
+    assert llm.chat_messages == [[{"role": "user", "content": "ping"}]]
+
+
+def test_chat_probe_defaults_to_summary_model_setting() -> None:
+    """The probe calls ``chat`` without a model override — the client's
+    ``llm_summary_model`` default (``lite``) is what gets pinged."""
+    llm = _ProbeClient()
+
+    _run_check(llm)
+    # ``chat`` received exactly the probe messages; the model default is
+    # resolved inside LLMClient.chat from settings.llm_summary_model.
+    assert llm.settings.llm_summary_model == "lite"
+    assert llm.chat_messages == [[{"role": "user", "content": "ping"}]]
+
+
+# --- embedding model down --------------------------------------------------
+
+
+def test_embed_down_names_embed_model_and_never_reaches_chat() -> None:
+    boom = EmbeddingError(
+        "embeddings request to https://aipi.reeseapps.com/v1 failed: "
+        "connection refused"
+    )
+    llm = _ProbeClient(embed_error=boom)
+
+    with pytest.raises(ModelUnavailableError) as excinfo:
+        _run_check(llm)
+
+    message = str(excinfo.value)
+    assert "'embed'" in message  # the embedding model is named
+    assert "not available" in message
+    assert excinfo.value.__cause__ is boom  # the original failure is chained
+    assert llm.chat_messages == []  # the summary probe is never reached
+    assert isinstance(excinfo.value, LLMError)  # the sync's generic catch
+
+
+def test_embed_down_wraps_any_exception_not_just_embedding_error() -> None:
+    boom = RuntimeError("transport exploded")
+    llm = _ProbeClient(embed_error=boom)
+
+    with pytest.raises(ModelUnavailableError) as excinfo:
+        _run_check(llm)
+
+    assert "The embedding model ('embed') is not available" in str(excinfo.value)
+    assert excinfo.value.__cause__ is boom
+    assert llm.chat_messages == []
+
+
+# --- summary model down ----------------------------------------------------
+
+
+def test_chat_down_names_summary_model() -> None:
+    boom = LLMError(
+        "chat completion from https://aipi.reeseapps.com/v1 failed: "
+        "connection refused"
+    )
+    llm = _ProbeClient(chat_error=boom)
+
+    with pytest.raises(ModelUnavailableError) as excinfo:
+        _run_check(llm)
+
+    message = str(excinfo.value)
+    assert "'lite'" in message  # the summary model is named
+    assert "not available" in message
+    assert excinfo.value.__cause__ is boom
+    assert llm.embed_texts == ["sync model check"]  # the embed probe passed
+    assert isinstance(excinfo.value, LLMError)
+
+
+def test_chat_down_wraps_any_exception() -> None:
+    llm = _ProbeClient(chat_error=ValueError("stream died"))
+
+    with pytest.raises(ModelUnavailableError) as excinfo:
+        _run_check(llm)
+
+    assert "The summary model ('lite') is not available" in str(excinfo.value)
+
+
+# --- message content (modal-readable wording) -------------------------------
+
+
+@pytest.mark.parametrize(
+    ("client", "model"),
+    [
+        (_ProbeClient(embed_error=EmbeddingError("down")), "embed"),
+        (_ProbeClient(chat_error=LLMError("down")), "lite"),
+    ],
+    ids=["embed-down", "summary-down"],
+)
+def test_message_content_mentions_model_and_availability(
+    client: _ProbeClient, model: str
+) -> None:
+    with pytest.raises(ModelUnavailableError) as excinfo:
+        _run_check(client)
+
+    message = str(excinfo.value)
+    assert f"('{model}')" in message  # the model name, quoted
+    assert "not available" in message  # the modal-readable wording
+    assert "check the model endpoint" in message  # actionable hint
+
+
+def test_custom_model_names_are_surfaced_verbatim() -> None:
+    """The probe names whatever the settings configure, not hardcoded
+    model strings."""
+    llm = _ProbeClient(
+        embed_error=EmbeddingError("down"),
+        embed_model="bge-m3",
+        summary_model="hermes-lite",
+    )
+
+    with pytest.raises(ModelUnavailableError) as excinfo:
+        _run_check(llm)
+
+    assert "'bge-m3'" in str(excinfo.value)