da5bf69aee
Some checks failed
Release & Docker / release (push) Has been cancelled
* fix(sidebar): hoist _sessionAttentionState to top-level scope (#3696) _sessionAttentionState was declared inside renderSessionListFromCache() and relied on function hoisting, but the top-level function _sidebarRowHasVisible Messages (reached via renderSessionListFromCache -> _partitionSidebarSessionRows) called it bare. Hoisting is scoped to the enclosing function, so every sidebar cache-render threw 'ReferenceError: _sessionAttentionState is not defined' and the session list went blank. Regressed in #3672 (v0.51.269) when _sidebarRow HasVisibleMessages was extracted to top level. Fix: move _sessionAttentionState to top-level scope (it is pure — only uses its arg plus the i18n global t), so both the visibility predicate and the nested per-row renderer can reach it. Prevention (the durable half): add scripts/scope_undef_gate.py — models the classic-<script> shared global scope (union of all static files' top-level symbols) and runs ESLint no-undef per file, flagging a function defined nested but called from a sibling scope. Wired into CI (.github/workflows/tests.yml lint job) alongside the existing no-const-assign runtime gate, plus an in-suite test (test_static_js_scope_undef.py) and a focused structural regression test (test_issue3696_session_attention_scope.py). RED/GREEN-validated against the broken tree. * fix(streaming): thread source param into stale-stream bailout; tighten scope gate Opus review of #3698 found the new scope_undef_gate's 'source' allowlist entry was masking a real same-class bug: _bailOutOfTerminalEventsFromStaleStream (declared inside attachLiveStream, params activeSid/streamId/uploaded/options) called _closeSource(source) against a 'source' not in its lexical scope. All 5 call sites are inside _wireSSE(source), but JS scope is lexical not dynamic, so the helper would throw ReferenceError: source is not defined on the stale-stream terminal-event path (user back in an active session whose old stream finalizes late). Fix: thread source as an explicit parameter (declaration + all 5 call sites), the same make-the-dependency-explicit fix as #3696 — and REMOVE the 'source' allowlist entry so the gate stays gated against that name (it now passes because the bug is fixed, not because it's allowlisted). Added the documented false-negative classes from Opus's review to the gate docstring (name-collision shadowing, destructuring-regex gap, exposure escape hatches, name-keyed allowlist) and a focused regression test. This is the prevention gate catching a real latent bug on its first outing. --------- Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
59 lines
2.6 KiB
Python
59 lines
2.6 KiB
Python
"""Scope / undefined-reference guard for the static JS bundle (issue #3696).
|
|
|
|
Why this exists: #3696 was a brick-class regression — `_sessionAttentionState`
|
|
was declared *inside* `renderSessionListFromCache()` but called (un-guarded) from
|
|
a separate top-level function `_sidebarRowHasVisibleMessages`. Function hoisting is
|
|
scoped to the enclosing function, so the call threw
|
|
`ReferenceError: _sessionAttentionState is not defined` on every sidebar
|
|
cache-render and the session list went blank (v0.51.269, regressed by #3672).
|
|
|
|
Nothing caught it: `node --check` is a syntax check (a nested function IS valid
|
|
syntax), source-presence tests asserted the strings existed (they did — in the
|
|
wrong scope), and the existing `no-const-assign` runtime gate only covers
|
|
const-reassign / import-assign. This is a DIFFERENT runtime-error class:
|
|
referencing a name that isn't in scope.
|
|
|
|
`scripts/scope_undef_gate.py` models the WebUI's classic-`<script>` shared global
|
|
scope (all top-level symbols across every static file become one namespace), then
|
|
runs ESLint `no-undef` per file. Cross-file globals resolve; a function defined
|
|
only nested and called from a sibling scope is flagged. See that script's header
|
|
for the full design + the verified dynamic-global allowlist.
|
|
|
|
Graceful skip: if node or eslint isn't available the test SKIPS (the release gate
|
|
runs where eslint IS installed — see TESTING.md), so toolchain-free envs aren't
|
|
blocked.
|
|
"""
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO = Path(__file__).resolve().parents[1]
|
|
GATE = REPO / "scripts" / "scope_undef_gate.py"
|
|
|
|
|
|
@pytest.mark.skipif(not GATE.exists(), reason="scope_undef_gate.py missing")
|
|
def test_static_js_has_no_undefined_references():
|
|
if shutil.which("eslint") is None:
|
|
pytest.skip(
|
|
"eslint not installed — install with "
|
|
"`npm install --no-save --before=<48h-ago> eslint` to enforce the "
|
|
"scope/undef guard locally (CI/release env has it). See TESTING.md."
|
|
)
|
|
if shutil.which("node") is None:
|
|
pytest.skip("node not available")
|
|
|
|
proc = subprocess.run(
|
|
[sys.executable, str(GATE), str(REPO)],
|
|
capture_output=True, text=True, timeout=180,
|
|
)
|
|
# Exit 0 = clean or eslint-skip; 1 = real finding; 2 = setup error.
|
|
assert proc.returncode != 1, (
|
|
"scope_undef_gate found undefined reference(s) that throw at runtime in the "
|
|
"browser (brick-class, see #3696):\n" + proc.stdout + proc.stderr
|
|
)
|
|
if proc.returncode == 2:
|
|
pytest.skip(f"scope_undef_gate setup issue (not a code failure): {proc.stdout[:300]}")
|