fix(ui): bind the shared header controls on explicit init, not at module import

header.js's control bindings (sign-out, the mobile hamburger, the SINGLE New chat button) ran at module import. The Containerfile stage-1 build inlines header.js into every bundle that imports it (the shell's app.js, token-gate.js and the router's lazy views), so the shell page registered the #nav-toggle click handler twice, and two toggle handlers cancel each other — one tap = open + close = the mobile menu dead in the deployed image only. The dev tree's single ESM instance (and every test that runs against it) never showed it; a lazy view load adding a THIRD copy made the menu work again, which is why the failure looked state-dependent (chat cold boot dead, /sources.html alive). - header.js: the three bindings move into an exported bindSharedHeaderControls(), guarded by a marker on <body> (NOT module state — every bundle copy has its own function instance), so later bundle copies and repeated inits (the token gate's mid-page header re-boot) are no-ops; header.js is now side-effect-free at top level, which also lets esbuild tree-shake the dead copies out of the bundles that do not need them (the token-gate bundle no longer carries the binding code at all) - app.js / login.js / shared.js / document.js: call bindSharedHeaderControls() once at module top — import-time parity, unconditional (no async boot path to miss); doc-edit.js ships no header controls and calls nothing - unit: tests/unit/test_header_bindings_once_per_document.py pins the contract — the init export, the document-level idempotency marker, all three bindings inside the init, NO top-level addEventListener remaining, and exactly one module-top call in each header-carrying page script; stale import-time docstrings in the legacy header pins updated to the new contract Verified: full unit + integration suite (1746 passed), the hamburger / pinned-composer / smoke E2E stories green in isolation, ruff + pyright clean. Containerfile-equivalent esbuild 0.25.5 rebuild probed in Chromium: exactly ONE #nav-toggle click listener on chat cold boot, /sources.html and login.html, and a touch tap opens the menu in all three states (pre-fix production: two listeners on cold boot = dead, three on sources = alive).
2026-09-08 22:31:45 -04:00
parent 4d287155c0
commit 1f0e4c6bb9
10 changed files with 379 additions and 110 deletions
@@ -21,8 +21,10 @@ This module pins the source-level contract for tasks 01 and 02:
  block still squeezes the inline nav at 641–900px, and the action pills'
  squeeze rules + the 58px bar height are untouched);
 * the header.js toggle behavior (task 02) is ONE module-owned binding —
-  the same import-time pattern as the sign-out/steering bindings: null-
-  safe lookups of ``#nav-toggle`` + ``#app-nav``, a ``setNavMenu`` that
+  the same explicit-init pattern as the sign-out binding (header.js's
+  ``bindSharedHeaderControls`` — 2026-09-08: import-time binding ran
+  once per bundle copy under the Containerfile build): null-safe
+  lookups of ``#nav-toggle`` + ``#app-nav``, a ``setNavMenu`` that
  syncs BOTH the ``.is-open`` class and ``aria-expanded``, a click
  toggle, a delegated nav-link close, an Esc close that refocuses the
  opener, and a matchMedia resize-back-to-desktop close; the binding
@@ -418,9 +420,10 @@ def _js_clean(code: str) -> str:


 def test_header_js_looks_up_toggle_and_nav_null_safe() -> None:
-    """The binding follows the module's import-time pattern (like the
-    sign-out binding): look up #nav-toggle and #app-nav with
-    querySelector at import, and guard the whole binding block behind
+    """The binding follows the module's explicit-init pattern (like the
+    sign-out binding, inside bindSharedHeaderControls): look up
+    #nav-toggle and #app-nav with querySelector at init, and guard the
+    whole binding block behind
    ``navToggle && appNav`` — a page lacking either element is a
    complete no-op."""
    js = _js()
@@ -0,0 +1,187 @@
+"""Unit: the shared-header control bindings are EXPLICIT init, once per
+document (2026-09-08 production double-binding fix).
+
+The bug: header.js's control bindings (sign-out, the mobile hamburger,
+the SINGLE New chat button) used to run at MODULE IMPORT. Under native
+ESM that is one instance per document — fine. But the Containerfile
+stage-1 build inlines header.js into every bundle that imports it (the
+shell's app.js, token-gate.js and the router's lazy views each carry a
+copy), and top-level code runs once per copy: the shell page registered
+the #nav-toggle click handler twice (the app.js + token-gate.js
+bundles), and two toggle handlers cancel each other — one tap = open +
+close = a menu dead in the deployed image only. The dev tree (raw ESM,
+single instance) never showed it, and neither did any test that runs
+against the dev tree; once a lazy view load added a THIRD copy the odd
+count made the menu work again, which is why the failure looked
+state-dependent (chat cold boot dead, /sources.html alive).
+
+The contract pinned here:
+
+* header.js owns the three bindings in ONE exported function —
+  ``bindSharedHeaderControls()`` — whose idempotency marker lives on
+  ``<body>`` (NOT in module state: every bundle copy has its own
+  function instance, so only the document can say "the first caller
+  won");
+* NO top-level ``addEventListener`` remains in header.js — the one
+  surviving listener outside the init is the steering per-note delete
+  inside ``renderSteeringPanel`` (a per-node callback factory, not a
+  module binding);
+* the four page scripts that ship header controls (app.js / login.js /
+  shared.js / document.js) each call ``bindSharedHeaderControls()``
+  exactly once as a module-top statement; doc-edit.js ships no header
+  controls and calls nothing.
+"""
+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"
+
+#: The call as a module-top statement (anchored — comment mentions of
+#: the function can never match).
+_CALL = re.compile(r"^\s*bindSharedHeaderControls\(\);\s*$", re.M)
+_IMPORT = re.compile(
+    r"import\s*\{[^}]*bindSharedHeaderControls[^}]*\}\s*from\s*\"./header\.js\"",
+    re.S,
+)
+
+
+def _text(path: Path) -> str:
+    assert path.is_file(), f"missing frontend file: {path.name}"
+    return path.read_text(encoding="utf-8")
+
+
+def _js() -> str:
+    return _text(HEADER_JS)
+
+
+def _strip_comments(code: str) -> str:
+    """Block + line comments stripped (comments may legally carry the
+    keywords the pins below count). header.js carries no string
+    literal containing ``//`` or braces, so the strip is exact."""
+    code = re.sub(r"/\*.*?\*/", "", code, flags=re.S)
+    return re.sub(r"//[^\n]*", "", code)
+
+
+def _function_body(code: str, signature: str) -> str:
+    """The full text from ``signature`` to its brace-matched closing
+    brace (neither body carries an unbalanced brace inside a string, so
+    the count is exact)."""
+    start = code.index(signature)
+    i = code.index("{", start)
+    depth = 0
+    for j in range(i, len(code)):
+        if code[j] == "{":
+            depth += 1
+        elif code[j] == "}":
+            depth -= 1
+            if depth == 0:
+                return code[start : j + 1]
+    raise AssertionError(f"unbalanced braces after {signature!r}")
+
+
+# ---------- the init function: one entry point, document-level guard ----------
+
+
+def test_header_exports_the_explicit_binding_init() -> None:
+    js = _js()
+    assert "export function bindSharedHeaderControls" in js, (
+        "header.js must export bindSharedHeaderControls — the single "
+        "entry point for the shared-header control bindings"
+    )
+
+
+def test_the_idempotency_marker_lives_on_the_document() -> None:
+    """The guard key is read once + written once, on document.body —
+    NOT in module state (every bundle copy has its own function
+    instance; only the document can say the first caller won)."""
+    js = _strip_comments(_js())
+    assert js.count("borHeaderBound") == 2, (
+        f"exactly one read (the guard) + one write (the marker) of the "
+        f"body dataset key (found {js.count('borHeaderBound')})"
+    )
+    assert "document.body" in js, "the marker lives on the document body"
+    assert 'body.dataset.borHeaderBound = "1"' in js, (
+        "the marker must be WRITTEN by the first caller"
+    )
+
+
+# ---------- all three bindings live inside the init ----------
+
+
+def test_all_three_control_bindings_live_inside_the_init() -> None:
+    js = _strip_comments(_js())
+    body = _function_body(js, "export function bindSharedHeaderControls")
+    for needle in (
+        # sign-out (bar copy for desktop + mobile dropdown copy)
+        'querySelectorAll(".sign-out-btn")',
+        'fetch("/api/logout", { method: "POST" })',
+        # the mobile hamburger (phase 46)
+        'querySelector("#nav-toggle")',
+        'querySelector("#app-nav")',
+        "function setNavMenu(open)",
+        'navToggle.addEventListener("click"',
+        # the SINGLE New chat binding (phase 34 task 02)
+        'querySelector("#new-chat-btn")',
+        'new CustomEvent("bor:new-chat")',
+    ):
+        assert needle in body, (
+            f"{needle!r} must live INSIDE bindSharedHeaderControls — "
+            "a binding outside the init runs at module import in every "
+            "bundle copy that inlines header.js"
+        )
+
+
+# ---------- the anti-regression pin: no top-level listener remains ----------
+
+
+def test_no_top_level_add_event_listener_remains() -> None:
+    """The pin for the double-binding class itself: NOTHING outside
+    bindSharedHeaderControls may register a listener at module scope —
+    the Containerfile inlines header.js into every bundle that imports
+    it, and top-level binding code runs once per bundle copy (two
+    #nav-toggle toggle handlers cancel each other = the dead production
+    menu). The ONE surviving addEventListener outside the init is the
+    steering per-note delete INSIDE renderSteeringPanel — a per-node
+    callback factory (one listener per note button created in the DOM),
+    not a module binding."""
+    js = _strip_comments(_js())
+    init_body = _function_body(js, "export function bindSharedHeaderControls")
+    cut = js.index(init_body)
+    rest = js[:cut] + js[cut + len(init_body) :]
+    assert rest.count("addEventListener") == 1, (
+        "exactly one addEventListener may survive outside the init — "
+        f"the steering per-note delete (found {rest.count('addEventListener')})"
+    )
+    panel = _function_body(rest, "function renderSteeringPanel")
+    assert "addEventListener" in panel, (
+        "the surviving listener must be the steering per-note delete "
+        "(renderSteeringPanel)"
+    )
+
+
+# ---------- the page scripts: one explicit call each, at module top ----------
+
+
+def test_each_header_carrying_page_script_calls_the_init_exactly_once() -> None:
+    for name in ("app.js", "login.js", "shared.js", "document.js"):
+        js = _text(ASSETS / name)
+        assert _IMPORT.search(js), (
+            f"{name}: must import bindSharedHeaderControls from "
+            '"./header.js" (relative — the bundler contract)'
+        )
+        calls = _CALL.findall(js)
+        assert len(calls) == 1, (
+            f"{name}: must CALL bindSharedHeaderControls() exactly once "
+            f"as a module-top statement (found {len(calls)})"
+        )
+    # doc-edit.js ships NO header controls (no nav / auth pair / steering
+    # panel — its own header comment says the page carries none), so it
+    # calls nothing: there is nothing to bind.
+    doc_edit = _text(ASSETS / "doc-edit.js")
+    assert "bindSharedHeaderControls" not in doc_edit, (
+        "doc-edit.js ships no header controls — no binding init call"
+    )
@@ -247,9 +247,9 @@ def test_chat_booted_flag_lands_exactly_once_after_boot_settles() -> None:
    layout commit for the restore case (the rAF callback runs before
    that frame's layout). A pre-settle throw leaves the cluster static
    — a degraded boot is already degraded (the gate/header above it),
-    and the hamburger binding lives in header.js's module body, so it
-    is unaffected either way (documented in the house comment, do not
-    "fix")."""
+    and the hamburger binding lives in header.js's explicit init
+    (bindSharedHeaderControls), so it is unaffected either way
+    (documented in the house comment, do not "fix")."""
    js = _app_js()
    assert js.count("chat-booted") == 1, (
        f"app.js must reference chat-booted EXACTLY once — the single "
@@ -148,8 +148,11 @@ def test_clear_chat_storage_removes_the_phase14_key_silently() -> None:

 def test_sign_out_binding_lives_in_the_shared_module() -> None:
    """The sign-out click binding (disable → POST /api/logout → reload)
-    is owned by header.js at module import — exactly one implementation
-    for every page that loads it. It binds to ALL .sign-out-btn
+    is owned by header.js — bound once per document via the explicit
+    bindSharedHeaderControls() init (NOT a module-import side effect:
+    the Containerfile inlines header.js into every bundle that imports
+    it, and import-time binding ran once per copy). It binds to ALL
+    .sign-out-btn
    elements (the bar copy for desktop + the mobile dropdown copy for
    ≤640px, phase 46), so both copies log out."""
    js = _text(HEADER_JS)
@@ -373,10 +376,11 @@ def test_header_module_loads_before_the_page_script() -> None:
    header.js with a direct <script> tag anymore. Each page script
    imports it relatively (`from "./header.js"`) — a hoisted import that
    the browser evaluates BEFORE the page script body runs, and that the
-    image bundler inlines into the page bundle. The sign-out binding and
-    the whoami cache therefore exist when the page script boots, and
-    header.js can never be evaluated twice on a page (a tag + import pair
-    would double-bind the sign-out listener)."""
+    image bundler inlines into the page bundle. The whoami cache
+    therefore exists when the page script boots, and header.js can
+    never be evaluated twice on a page (the single-evaluation design is
+    the contract; a tag + import pair is now additionally harmless —
+    the binding init is idempotent via the body marker)."""
    cases = [
        (INDEX_HTML, "app.js"),
        (DOCUMENT_HTML, "document.js"),
@@ -490,8 +494,9 @@ def test_login_js_uses_the_shared_fetch_is_admin() -> None:

 def test_new_chat_binding_is_single_and_module_owned() -> None:
    """Phase 34 task 02 + owner rework (2026-08-28): header.js owns the
-    SINGLE #new-chat-btn binding (module import, like the sign-out
-    binding). The button now lives ONLY on the chat page (inside
+    SINGLE #new-chat-btn binding (explicit init via
+    bindSharedHeaderControls, like the sign-out binding). The button
+    now lives ONLY on the chat page (inside
    .chat-shell, above #messages — moved from the navbar at owner
    request), so the click ALWAYS dispatches window "bor:new-chat" and
    app.js acts through its own in-flight-turn guard + list reset: no
@@ -244,8 +244,12 @@ def test_boot_order_header_then_public_read() -> None:
    assert "showInvalid()" in branch
    assert "renderSharedChat" not in branch, "no render on a null read"
    # The relative import of the shared header module (no absolute
-    # /assets/ import — the esbuild bundle contract).
-    assert 'import { initSharedHeader } from "./header.js";' in js
+    # /assets/ import — the esbuild bundle contract). The explicit
+    # binding init rides the same import (2026-09-08 double-binding fix).
+    assert (
+        'import { bindSharedHeaderControls, initSharedHeader } from "./header.js";'
+        in js
+    )
    assert '"/assets/header.js"' not in js
    # No cross-page import (the per-page duplication house style).
    import_lines = [