From 6dd1fed8d3cebadf20b1c998e097d6d04e924cd2 Mon Sep 17 00:00:00 2001
From: webtecnica <webtecnica@gmail.com>
Date: Sat, 1 Aug 2026 17:52:30 -0300
Subject: [PATCH] fix(ui): guard all bare _autoScrollFollow reads (#6614)

---
 static/ui.js                                  |  18 ++-
 ...st_issue4006_auto_scroll_follow_default.py | 110 ++++++++++++++++++
 tests/test_sse_recovery_scroll_stranding.py   |   4 +-
 3 files changed, 125 insertions(+), 7 deletions(-)

diff --git a/static/ui.js b/static/ui.js
index 3be12f7c..0b192756 100644
--- a/static/ui.js
+++ b/static/ui.js
@@ -5664,6 +5664,14 @@ window.addEventListener('resize',function(){
 // Any manual scroll up sets a sticky unpinned flag until the user scrolls back
 // to the bottom (near-bottom hysteresis on downward motion) or clicks ↓.
 // Programmatic scrolls are ignored via _programmaticScroll. Fixes #1469 / #1360 / #1731.
+// #6606 ownership: ui.js/messages.js load before boot.js, and boot.js only
+// assigns window._autoScrollFollow after its awaited settings request. Every
+// consumer in this file (scroll listener, settle paths, scrollIfPinned,
+// DOM-replace gate) can run before that assignment — a bare read would throw
+// ReferenceError. Establish the default synchronously here (single owner);
+// boot.js overwrites it with the saved setting after hydration. The typeof
+// guard preserves an explicit saved `false` if boot.js already ran.
+if(typeof window._autoScrollFollow==='undefined'){ window._autoScrollFollow=true; }
 let _scrollPinned=true;
 let _programmaticScroll=false;
 let _programmaticScrollSetAt=0;
@@ -6104,7 +6112,7 @@ if(typeof window!=='undefined'){
         if(nearBottom){
           _nearBottomCount=_nearBottomCount+1;
           if(_nearBottomCount>=2){_scrollPinned=true;_nearBottomCount=0;}
-        }else if(!movedUp && (typeof _autoScrollFollow==='undefined'||_autoScrollFollow) && _scrollPinned){
+        }else if(!movedUp && window._autoScrollFollow && _scrollPinned){
           // Content-grew-beneath-a-pinned-viewport case (NOT a user scroll-away).
           // During streaming on a tall transcript (esp. mobile, where chunks land
           // fast), new content increases scrollHeight under a stationary viewport,
@@ -6779,7 +6787,7 @@ function _shouldFollowMessagesOnDomReplace(){
   // following only for users who are still pinned or effectively at the tail.
   // A broad near-bottom window causes long answers/mobile readers who scroll up
   // a little to read mid-stream to get snapped back to the bottom on completion.
-  return (typeof _autoScrollFollow==='undefined'||_autoScrollFollow) && !_messageUserUnpinned && (_scrollPinned || _isMessagePaneNearBottom(120));
+  return window._autoScrollFollow && !_messageUserUnpinned && (_scrollPinned || _isMessagePaneNearBottom(120));
 }
 function _followMessagesAfterDomReplace(){
   if(_shouldFollowMessagesOnDomReplace()){
@@ -6829,7 +6837,7 @@ function _settleMessageScrollToBottom(force, explicit){
   // active one that may now be in the global _settleRO. (Codex review #3.)
   const ro=new ResizeObserver(()=>{
     if(token!==_bottomSettleToken){ ro.disconnect(); if(_settleRO===ro) _settleRO=null; return; }
-    if((!_autoScrollFollow&&!explicit)||!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){
+    if((!window._autoScrollFollow&&!explicit)||!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){
       ro.disconnect(); if(_settleRO===ro) _settleRO=null;
       _programmaticScroll=false;
       return;
@@ -6868,7 +6876,7 @@ function _settleMessageScrollToBottom(force, explicit){
   _settleFinalTimer=setTimeout(()=>{
     if(token!==_bottomSettleToken) return;
     ro.disconnect(); if(_settleRO===ro) _settleRO=null;
-    if((!_autoScrollFollow&&!explicit)||!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){ _programmaticScroll=false; return; }
+    if((!window._autoScrollFollow&&!explicit)||!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){ _programmaticScroll=false; return; }
     _settleFinalScroll(token);
   },2000);
 }
@@ -6889,7 +6897,7 @@ function _settleFinalScroll(token){
   _deferClearProgrammaticScroll();
 }
 function scrollIfPinned(){
-  if(!_autoScrollFollow) return;
+  if(!window._autoScrollFollow) return;
   if(_messageUserUnpinned){
     // Only scrollToBottom() cleared this flag, so one scroll-up permanently
     // killed auto-follow. Re-pin ONLY when the reader has genuinely returned to
diff --git a/tests/test_issue4006_auto_scroll_follow_default.py b/tests/test_issue4006_auto_scroll_follow_default.py
index da6a905b..113909c0 100644
--- a/tests/test_issue4006_auto_scroll_follow_default.py
+++ b/tests/test_issue4006_auto_scroll_follow_default.py
@@ -7,8 +7,14 @@ future edit can't silently flip it or, worse, default it ON in config.py while
 hydrating it OFF in the browser (the classic default-mismatch bug, where an
 existing user with no saved value sees the feature as disabled).
 """
+import json
 import pathlib
 import re
+import shutil
+import subprocess
+import textwrap
+
+import pytest
 
 ROOT = pathlib.Path(__file__).resolve().parent.parent
 
@@ -72,3 +78,107 @@ def test_follow_gate_references_auto_scroll_follow_and_unpin():
     assert "_autoScrollFollow" in src and "_messageUserUnpinned" in src, (
         "the follow gate must reference both _autoScrollFollow and _messageUserUnpinned"
     )
+
+
+def test_ui_sync_init_defaults_auto_scroll_follow_before_consumers():
+    """ui.js must establish window._autoScrollFollow synchronously at the
+    top-level scroll-state section — before boot.js hydrates it from the awaited
+    settings request. ui.js/messages.js load before boot.js (index.html), so any
+    scroll listener / settle path / scrollIfPinned / DOM-replace gate running in
+    that window reads an undeclared binding and throws ReferenceError (#6606).
+
+    The typeof guard makes the default `true` WITHOUT clobbering an explicit
+    saved `false` if boot.js already hydrated it."""
+    src = _read("static/ui.js")
+    assert "if(typeof window._autoScrollFollow==='undefined'){ window._autoScrollFollow=true; }" in src, (
+        "ui.js must synchronously default window._autoScrollFollow to true before "
+        "boot.js hydration (bare reads otherwise throw ReferenceError pre-boot)"
+    )
+    # The sync init must sit BEFORE every consumer it protects in the file.
+    init_idx = src.index("window._autoScrollFollow=true;")
+    scroll_listener_idx = src.index("!movedUp && window._autoScrollFollow && _scrollPinned")
+    gate_idx = src.index("window._autoScrollFollow && !_messageUserUnpinned")
+    settle_idx = src.index("!window._autoScrollFollow&&!explicit")
+    pinned_idx = src.index("function scrollIfPinned")
+    for label, idx in (
+        ("scroll listener", scroll_listener_idx),
+        ("DOM-replace gate", gate_idx),
+        ("settle guard", settle_idx),
+        ("scrollIfPinned", pinned_idx),
+    ):
+        assert init_idx < idx, f"sync init must precede the {label} consumer"
+
+
+def test_ui_readers_use_explicit_window_owner():
+    """Every _autoScrollFollow reader in ui.js must go through the explicit
+    window._autoScrollFollow owner — no bare undeclared reads that can throw
+    ReferenceError before boot.js assigns the global (#6606)."""
+    src = _read("static/ui.js")
+    # Any bare `_autoScrollFollow` token not preceded by `window.` is a bug.
+    for m in re.finditer(r"(?<![\w.])_autoScrollFollow", src):
+        before = src[max(0, m.start() - 7):m.start()]
+        assert before == "window.", (
+            f"bare _autoScrollFollow read at ui.js offset {m.start()}: "
+            f"...{src[max(0, m.start()-40):m.start()+20]!r}"
+        )
+
+
+@pytest.mark.skipif(shutil.which("node") is None, reason="node required for behavioral test")
+def test_auto_scroll_follow_undefined_true_false_behavior():
+    """Behavioral: extract the real follow expressions from ui.js and evaluate
+    them in Node across undefined / true / false:
+
+    - undefined must NOT throw (property read, not a bare undeclared binding)
+      and must not block an explicit bottom settlement;
+    - false must suppress automatic following (DOM-replace gate, settle guard,
+      scrollIfPinned) WITHOUT suppressing explicit bottom settlement;
+    - true keeps following enabled."""
+    src = _read("static/ui.js")
+    # Extract the exact live expressions (not hand-copied replicas).
+    dom_replace_expr = "window._autoScrollFollow && !_messageUserUnpinned && (_scrollPinned || _isMessagePaneNearBottom(120))"
+    assert dom_replace_expr in src, "DOM-replace follow gate expression changed"
+    settle_expr = "(!window._autoScrollFollow&&!explicit)||!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()"
+    assert settle_expr in src, "settle guard expression changed"
+    pinned_guard_expr = "!window._autoScrollFollow"
+    assert f"if({pinned_guard_expr}) return;" in src, "scrollIfPinned guard changed"
+
+    def run(expr, follow, explicit=False, pinned=True, unpinned=False, near=True):
+        follow_js = "undefined" if follow is None else ("true" if follow else "false")
+        harness = textwrap.dedent(f"""
+            const window = {{ _autoScrollFollow: {follow_js} }};
+            let _scrollPinned = {str(pinned).lower()};
+            let _messageUserUnpinned = {str(unpinned).lower()};
+            let explicit = {str(explicit).lower()};
+            function _isMessagePaneNearBottom(px){{ return {str(near).lower()}; }}
+            function _recentNonMessageScrollIntent(){{ return false; }}
+            let _out;
+            try {{
+              _out = ({expr});
+            }} catch (e) {{
+              console.error('THREW: ' + e);
+              process.exit(2);
+            }}
+            console.log(JSON.stringify(_out === undefined ? '__UNDEF__' : _out));
+        """)
+        res = subprocess.run(["node", "-e", harness], capture_output=True, text=True, timeout=30)
+        assert res.returncode == 0, res.stderr
+        val = json.loads(res.stdout.strip())
+        return None if val == "__UNDEF__" else val
+
+    # undefined: property read → falsy, never throws, explicit settle still allowed.
+    assert not run(dom_replace_expr, None)          # undefined && … → undefined (falsy)
+    assert run(settle_expr, None) is True          # suppress implicit settle
+    assert run(settle_expr, None, explicit=True) is False  # explicit settle proceeds
+    assert run(pinned_guard_expr, None) is True    # scrollIfPinned returns early
+    # true: follow enabled everywhere.
+    assert run(dom_replace_expr, True) is True
+    assert run(settle_expr, True) is False         # implicit settle proceeds
+    assert run(pinned_guard_expr, True) is False   # scrollIfPinned does NOT early-return
+    # false: automatic follow suppressed…
+    assert run(dom_replace_expr, False) is False
+    assert run(settle_expr, False) is True
+    assert run(pinned_guard_expr, False) is True
+    # …but an EXPLICIT bottom settlement still runs (false must not block ↓/open).
+    assert run(settle_expr, False, explicit=True) is False
+    # Unpinned reader is never re-followed, regardless of the setting.
+    assert run(dom_replace_expr, True, unpinned=True) is False
diff --git a/tests/test_sse_recovery_scroll_stranding.py b/tests/test_sse_recovery_scroll_stranding.py
index cc5a5c12..b695b337 100644
--- a/tests/test_sse_recovery_scroll_stranding.py
+++ b/tests/test_sse_recovery_scroll_stranding.py
@@ -50,11 +50,11 @@ def test_content_grew_keeps_pin_and_resnaps_not_unpin():
     # did NOT move up and auto-follow is on, keep the pin and re-snap to bottom
     # rather than unpinning. Assert the new guard + re-snap call are present.
     c = _compact(UI_JS)
-    assert _compact("}else if(!movedUp && _autoScrollFollow && _scrollPinned){") in c, (
+    assert _compact("}else if(!movedUp && window._autoScrollFollow && _scrollPinned){") in c, (
         "content-grew-beneath-pinned guard missing from the scroll listener"
     )
     # The guard body must re-snap to bottom, not set _scrollPinned=false.
-    idx = c.find(_compact("else if(!movedUp && _autoScrollFollow && _scrollPinned){"))
+    idx = c.find(_compact("else if(!movedUp && window._autoScrollFollow && _scrollPinned){"))
     assert idx != -1
     # The re-snap call must be the FIRST scroll action after the guard opens,
     # before the chain reaches any `else{ _scrollPinned=false }` fallthrough.