fix(header): ship the tuning toggle hidden — no anonymous flash

#steering-toggle (the header 'Tuning' button) shipped visible in all six pages and was only removed after /api/whoami resolved, so anonymous visitors saw it flash for the whole round-trip (TODO.md L3). It now ships hidden on every page and initSharedHeader unhides it only for admin — the same ship-hidden / reveal-for-admin contract as the admin-only nav links; the anonymous end-state (removed from the DOM, phase-16 'absent, not hidden') is unchanged. Adds the story E2E suite (MutationObserver proves zero visible frames for anonymous on every page, admin reveal + panel + count badge, nav-contract regression) and the source-level unit pins. Also fixes test_steering.py's BASE_SCRIPT_COUNT (2 → 3: brand.js + markdown.js + app.js, since phase 39).
2026-08-27 22:38:58 -04:00
parent 02c76ad328
commit 6f9e033117
11 changed files with 429 additions and 17 deletions
@@ -45,8 +45,9 @@ QUESTION = "How is my Kubernetes cluster set up?"
 MOCK_ANSWER_MARKER = "Deterministic mock answer for E2E"
 NOTE = "STEEER-MARKER be concise"
 XSS_NOTE = "<script>window.__xss = true; alert('xss')</script>"
-#: index.html ships exactly two classic/module script tags.
-BASE_SCRIPT_COUNT = 2
+#: index.html ships exactly three classic/module script tags: the
+#: phase-39 brand.js classic layer + markdown.js + the app.js module.
+BASE_SCRIPT_COUNT = 3


 async def _import_fixtures(mock_port: int) -> ImportSummary:
@@ -0,0 +1,253 @@
+"""Phase 40 E2E (Playwright): the anonymous "Tuning" flash is gone.
+
+The owner report (``TODO.md`` L3): loading a page briefly showed the
+header "Tuning" button (``#steering-toggle``, the steering-notes
+toggle) to ANONYMOUS visitors — it shipped VISIBLE in all six pages'
+markup and ``assets/header.js`` removed it only after ``/api/whoami``
+resolved, so the button flashed for the whole whoami round-trip.
+
+The fix mirrors the admin-only nav links (phase 19/29/35 contract):
+the toggle now SHIPS ``hidden`` in every page and ``initSharedHeader``
+unhides it only when whoami says admin; the anonymous end-state is
+unchanged (toggle + panel REMOVED from the DOM — phase 16 "absent,
+not hidden"). This suite proves the browser-level contract:
+
+* a MutationObserver (installed via ``add_init_script`` before any
+  page code runs) records every frame in which ``#steering-toggle``
+  is both in the DOM and visible (``offsetParent !== null`` or
+  ``!hidden``) — an anonymous load records ZERO such frames, on
+  every page, from first paint to the settled state;
+* after the whoami round-trip the toggle is ABSENT from the DOM for
+  anonymous visitors (removed, not hidden);
+* the admin UX is untouched (phase 15/34 behavior): the toggle is
+  revealed, clicking opens ``#steering-panel``
+  (``aria-expanded="true"``) and the count badge matches the note
+  list — including a self-check that the observer records the admin
+  reveal, so the zero-frame anonymous assertions are not vacuous;
+* the ship-hidden nav-link contract this phase relies on
+  (``#nav-sources`` / ``#nav-git-sources`` / ``#nav-tuning``) is
+  intact: hidden for anonymous, revealed for admin.
+
+Story: ``.agent/user_stories/tuning-toggle-flash.md``
+Run in isolation (DB must be up: ``podman compose up -d db``):
+
+    uv run pytest tests/e2e/test_tuning_toggle_flash.py -v --no-cov
+
+Test → story mapping (Playwright Mapping Rule):
+1. ``test_anonymous_never_sees_toggle``
+2. ``test_anonymous_other_pages_never_flash``
+3. ``test_admin_toggle_revealed_and_working``
+4. ``test_nav_contract_regression``
+"""
+from __future__ import annotations
+
+from playwright.sync_api import Page, expect
+from sqlalchemy import text
+
+from app.db import SessionLocal
+from e2e.auth_helpers import login
+
+#: The pages the contract must hold on besides the chat page (mapping
+#: rule 2). document.html / git-sources.html are covered by the
+#: source-level unit pins (tests/unit/test_steering_toggle_visibility.py)
+#: — the four pages here are the ones an anonymous visitor actually
+#: lands on.
+OTHER_PAGES = ("/sources.html", "/tuning.html", "/login.html")
+
+ADMIN_NOTE = "PHASE40-E2E note — badge check"
+
+#: Admin-only nav links — the ship-hidden / reveal-for-admin family the
+#: steering toggle now belongs to (phase 19/29/35 contract).
+NAV_IDS = ("#nav-sources", "#nav-git-sources", "#nav-tuning")
+
+#: Runs in every new document BEFORE any page script (addInitScript):
+#: arms a MutationObserver over the whole DOM and records every frame
+#: in which #steering-toggle is attached AND visible — visible meaning
+#: rendered (offsetParent !== null) OR carrying no [hidden] attribute
+#: (!el.hidden). A shipped-VISIBLE toggle (the old bug) is recorded the
+#: moment the parser inserts it; a shipped-hidden toggle that is later
+#: revealed is recorded at the reveal mutation. The array is fresh per
+#: document, so each navigation asserts its own frames.
+VISIBILITY_OBSERVER_JS = """
+window.__tuningVisibleFrames = [];
+(() => {
+  const visible = (el) =>
+    !!el && el.isConnected && (el.offsetParent !== null || !el.hidden);
+  const check = () => {
+    if (visible(document.getElementById("steering-toggle"))) {
+      window.__tuningVisibleFrames.push({
+        at: Math.round(performance.now()),
+        href: location.pathname,
+      });
+    }
+  };
+  const start = () => {
+    check();
+    new MutationObserver(check).observe(document.documentElement, {
+      childList: true,
+      subtree: true,
+      attributes: true,
+      attributeFilter: ["hidden"],
+    });
+  };
+  if (document.documentElement) start();
+  else document.addEventListener("DOMContentLoaded", start);
+})();
+"""
+
+
+def install_visibility_observer(page: Page) -> None:
+    """Arm the never-visible frame counter on every document this page
+    creates — the initial load, the post-login redirect, re-gotos."""
+    page.add_init_script(VISIBILITY_OBSERVER_JS)
+
+
+def visible_frames(page: Page) -> list[dict[str, object]]:
+    """The frames the observer recorded in the CURRENT document."""
+    return page.evaluate("() => window.__tuningVisibleFrames || []")
+
+
+def _assert_no_flash(page: Page, path: str) -> None:
+    """Zero visible frames + the phase-16 absent end-state, for one
+    anonymously loaded page."""
+    frames = visible_frames(page)
+    assert frames == [], f"{path}: the toggle was visible {len(frames)}x: {frames!r}"
+    assert page.locator("#steering-toggle").count() == 0, (
+        f"{path}: #steering-toggle must be REMOVED from the DOM for "
+        "anonymous (phase 16 'absent, not hidden')"
+    )
+    assert page.locator("#steering-panel").count() == 0, (
+        f"{path}: #steering-panel must be removed together with the toggle"
+    )
+
+
+def _wait_header_settled_anonymous(page: Page) -> None:
+    """Whoami resolved on the current document: the anonymous state
+    reveals the Sign in link and keeps Sign out hidden (the pair is
+    decided by the SAME initSharedHeader pass that removes the
+    toggle)."""
+    page.wait_for_load_state("networkidle")
+    expect(page.locator("#sign-in-link")).to_be_visible(timeout=15_000)
+    expect(page.locator("#sign-out-btn")).to_be_hidden()
+
+
+# ---------------------------------------------------------------------------
+# 1. Anonymous chat load: zero visible frames, toggle absent afterwards
+# ---------------------------------------------------------------------------
+
+
+def test_anonymous_never_sees_toggle(
+    page: Page, app_url: str, mock_llm: int, db_ready: None
+) -> None:
+    page.set_default_timeout(30_000)
+    install_visibility_observer(page)
+    page.goto(app_url + "/")
+    _wait_header_settled_anonymous(page)
+    _assert_no_flash(page, "/")
+
+
+# ---------------------------------------------------------------------------
+# 2. Anonymous loads of the other pages: same zero-flash contract
+# ---------------------------------------------------------------------------
+
+
+def test_anonymous_other_pages_never_flash(
+    page: Page, app_url: str, mock_llm: int, db_ready: None
+) -> None:
+    page.set_default_timeout(30_000)
+    install_visibility_observer(page)
+    for path in OTHER_PAGES:
+        page.goto(app_url + path)
+        _wait_header_settled_anonymous(page)
+        _assert_no_flash(page, path)
+
+
+# ---------------------------------------------------------------------------
+# 3. Admin: revealed, clickable, count badge matches the list
+# ---------------------------------------------------------------------------
+
+
+def test_admin_toggle_revealed_and_working(
+    page: Page, app_url: str, mock_llm: int, db_ready: None
+) -> None:
+    page.set_default_timeout(30_000)
+    with SessionLocal() as db:
+        db.execute(text("TRUNCATE steering_notes"))
+        db.commit()
+
+    install_visibility_observer(page)
+    login(page, app_url, next="/")
+    page.wait_for_load_state("networkidle")
+    expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
+
+    toggle = page.locator("#steering-toggle")
+    panel = page.locator("#steering-panel")
+
+    # Revealed — the [hidden] attribute is gone and the button renders.
+    expect(toggle).to_be_visible()
+    assert toggle.get_attribute("hidden") is None
+
+    # Observer self-check: the reveal IS a recorded visible frame, so
+    # the zero-frame anonymous assertions above cannot be vacuous.
+    assert visible_frames(page) != [], "the admin reveal was not observed"
+
+    # Click: the panel opens, aria-expanded tracks it (phase 15/34).
+    toggle.click()
+    expect(panel).to_be_visible()
+    expect(toggle).to_have_attribute("aria-expanded", "true")
+
+    # Count badge matches the list — first the empty state…
+    expect(page.locator("#steering-count")).to_have_text("0")
+    expect(page.locator("#steering-list .steering-note")).to_have_count(0)
+    expect(page.locator("#steering-empty")).to_be_visible()
+
+    # …then with one note created through the real admin API.
+    page.evaluate(
+        """async (note) => {
+            const r = await fetch("/api/steering", {
+                method: "POST",
+                headers: {"Content-Type": "application/json"},
+                body: JSON.stringify({note}),
+            });
+            if (!r.ok) throw new Error("steering POST failed: " + r.status);
+        }""",
+        ADMIN_NOTE,
+    )
+    toggle.click()  # close
+    toggle.click()  # re-open (refreshes the list)
+    expect(panel).to_be_visible()
+    expect(toggle).to_have_attribute("aria-expanded", "true")
+    expect(page.locator("#steering-count")).to_have_text("1")
+    expect(page.locator("#steering-list .steering-note")).to_have_count(1)
+    expect(page.locator("#steering-list .steering-note-text")).to_have_text(ADMIN_NOTE)
+
+
+# ---------------------------------------------------------------------------
+# 4. Nav-contract regression (phase 19/34): the ship-hidden family the
+#    toggle now belongs to is intact
+# ---------------------------------------------------------------------------
+
+
+def test_nav_contract_regression(
+    page: Page, app_url: str, mock_llm: int, db_ready: None
+) -> None:
+    page.set_default_timeout(30_000)
+
+    # Anonymous: every admin-only nav link stays hidden on the chat
+    # page (ships [hidden], never revealed).
+    page.goto(app_url + "/")
+    _wait_header_settled_anonymous(page)
+    for nav in NAV_IDS:
+        assert page.locator(nav).count() == 1, f"{nav} missing from the chat header"
+        expect(page.locator(nav)).to_be_hidden()
+        assert page.locator(nav + "[hidden]").count() == 1, (
+            f"{nav} must stay [hidden] for anonymous"
+        )
+
+    # Admin: the same links are revealed — the exact contract the
+    # steering toggle now mirrors.
+    login(page, app_url, next="/")
+    page.wait_for_load_state("networkidle")
+    expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
+    for nav in NAV_IDS:
+        expect(page.locator(nav)).to_be_visible()
@@ -0,0 +1,150 @@
+"""Unit: the tuning toggle ships hidden — the anonymous flash fix (phase 40).
+
+The "Tuning" steering toggle (``#steering-toggle``) used to ship VISIBLE
+in every page's shared header and was removed by ``assets/header.js``
+only after ``/api/whoami`` resolved — so an anonymous user briefly saw
+the button on every page load (``TODO.md`` L3). The fix mirrors the
+admin-only NAV LINKS (phase 19/29/35 contract): the toggle now SHIPS
+with the ``hidden`` attribute in all six pages and ``initSharedHeader``
+unhides it only when whoami says admin. The browser behavior is
+E2E-covered (``tests/e2e/test_tuning_toggle_flash.py``); here we pin
+the source-level wiring — the ship-hidden markup on every page, the
+admin unhide line inside ``initSharedHeader``, the intact anonymous
+remove-from-DOM path (phase 16 "absent, not hidden"), and the
+``#nav-tuning`` hidden contract this phase relies on — so a silent
+regression is caught without a browser.
+"""
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
+ASSETS = FRONTEND / "assets"
+
+HEADER_JS = ASSETS / "header.js"
+
+#: All six pages carry the shared header block (phase 34's five pages +
+#: phase 35's git-sources page).
+PAGES = (
+    FRONTEND / "index.html",
+    FRONTEND / "sources.html",
+    FRONTEND / "document.html",
+    FRONTEND / "git-sources.html",
+    FRONTEND / "login.html",
+    FRONTEND / "tuning.html",
+)
+
+
+def _text(path: Path) -> str:
+    assert path.is_file(), f"missing frontend file: {path}"
+    return path.read_text(encoding="utf-8")
+
+
+def _toggle_tag(html: Path) -> str:
+    tag = re.search(r"<button[^>]*id=\"steering-toggle\"[^>]*>", _text(html))
+    assert tag, f"{html.name}: missing the #steering-toggle button"
+    return tag.group(0)
+
+
+def _toggle_body(html: Path) -> str:
+    """The full <button>…</button> block, for the icon/label/badge pins."""
+    text = _text(html)
+    start = text.find('id="steering-toggle"')
+    assert start != -1, f"{html.name}: missing the #steering-toggle button"
+    return text[start : text.find("</button>", start)]
+
+
+def _init_body(js: str) -> str:
+    """The source of initSharedHeader in header.js."""
+    fn = js.find("function initSharedHeader")
+    assert fn != -1, "initSharedHeader must be defined"
+    return js[fn : js.find("\n}", fn)]
+
+
+# ---------- ship-hidden markup: zero flash for anonymous ----------
+
+
+def test_steering_toggle_ships_hidden_on_all_six_pages() -> None:
+    """#steering-toggle carries the ``hidden`` attribute in ALL SIX
+    pages — the exact ship-hidden contract the admin-only nav links
+    use, so an anonymous user never sees the "Tuning" button for a
+    single frame, on any page."""
+    for html in PAGES:
+        tag = _toggle_tag(html)
+        assert re.search(r"\bhidden\b", tag), (
+            f"{html.name}: #steering-toggle must ship hidden"
+        )
+
+
+def test_steering_toggle_keeps_its_existing_markup() -> None:
+    """Only the ``hidden`` attribute was added: type, class, the
+    aria-expanded / aria-controls wiring, the decorative icon, the
+    "Tuning" label, and the #steering-count badge stay byte-identical
+    on every page — the admin UX is unchanged."""
+    for html in PAGES:
+        tag = _toggle_tag(html)
+        assert 'type="button"' in tag
+        assert 'class="steering-toggle"' in tag
+        assert 'aria-expanded="false"' in tag
+        assert 'aria-controls="steering-panel"' in tag
+        body = _toggle_body(html)
+        assert '<svg aria-hidden="true"' in body, f"{html.name}: icon is gone"
+        assert '<span class="steering-label">Tuning</span>' in body, (
+            f"{html.name}: label markup changed"
+        )
+        assert '<span class="steering-count" id="steering-count">0</span>' in body, (
+            f"{html.name}: count badge markup changed"
+        )
+
+
+def test_steering_panel_still_ships_hidden_on_all_six_pages() -> None:
+    """The #steering-panel section already shipped hidden and stays
+    that way (this phase never touches the panel)."""
+    for html in PAGES:
+        tag = re.search(r'<section[^>]*id="steering-panel"[^>]*>', _text(html))
+        assert tag, f"{html.name}: missing the #steering-panel section"
+        assert re.search(r"\bhidden\b", tag.group(0)), "the panel ships hidden"
+
+
+# ---------- header.js: reveal-for-admin, anonymous removal intact ----------
+
+
+def test_header_js_unhides_the_toggle_for_admin() -> None:
+    """Inside initSharedHeader, the admin branch unhides the toggle —
+    and that line sits BEFORE the refreshSteering() call (the count
+    badge is right before the panel is ever opened)."""
+    body = _init_body(_text(HEADER_JS))
+    assert "if (steeringToggle) steeringToggle.hidden = false;" in body, (
+        "the admin unhide is missing from initSharedHeader"
+    )
+    admin = body.find("if (admin)")
+    unhide = body.find("steeringToggle.hidden = false")
+    refresh = body.find("if (steeringPanel) refreshSteering();")
+    assert -1 < admin < unhide, "the unhide must live in the admin branch"
+    assert unhide < refresh, "the unhide must precede the refreshSteering() call"
+
+
+def test_anonymous_removal_path_is_intact() -> None:
+    """The phase-16 "absent, not hidden" contract is preserved: the
+    anonymous branch still REMOVES the toggle + panel from the DOM —
+    the new hidden attribute only closes the pre-whoami flash window,
+    the end state (absent) is unchanged."""
+    body = _init_body(_text(HEADER_JS))
+    assert "steeringToggle?.remove();" in body
+    assert "steeringPanel?.remove();" in body
+
+
+# ---------- the nav contract this phase relies on ----------
+
+
+def test_nav_tuning_still_ships_hidden_on_all_six_pages() -> None:
+    """The admin-only Tuning NAV LINK (#nav-tuning) — the contract the
+    toggle now mirrors — still ships hidden on every page: one
+    ship-hidden / reveal-for-admin family, nav link and toggle alike."""
+    for html in PAGES:
+        tag = re.search(r'<a[^>]*id="nav-tuning"[^>]*>', _text(html))
+        assert tag, f"{html.name}: missing the #nav-tuning nav link"
+        assert re.search(r"\bhidden\b", tag.group(0)), (
+            f"{html.name}: #nav-tuning must ship hidden"
+        )