fix(#3162): const→let in _ensureMessagesLoaded (brick-class mobile TypeError) + ESLint runtime-error guard
The #3018 carry-forward reassigns msgs but it was declared const, throwing a TypeError that surfaced as 'Failed to load conversation messages' on every mobile message (v0.51.161-166). Change to let. Adds a static JS runtime-error lint guard (eslint.runtime-guard.config.mjs + tests/test_static_js_runtime_lint.py) using no-const-assign/no-import-assign — the exact class node --check and source-presence tests miss. Dev-only dependency; app stays pure Python + vanilla JS.
This commit is contained in:
4
.gitignore
vendored
4
.gitignore
vendored
@@ -59,3 +59,7 @@ graphify-out/
|
||||
.graphify_uncached.txt
|
||||
|
||||
.venv/
|
||||
|
||||
# Dev-only lint tooling (ESLint runtime-error guard) — see TESTING.md
|
||||
node_modules/
|
||||
package-lock.json
|
||||
|
||||
10
CHANGELOG.md
10
CHANGELOG.md
@@ -3,6 +3,16 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.168] — 2026-05-30 — Release EN (stage-batch50 — hotfix: mobile "Failed to load conversation messages")
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed a `TypeError` in `_ensureMessagesLoaded` that surfaced as a "Failed to load conversation messages" toast on mobile after most messages: a `const msgs` binding was reassigned by the #3018 ephemeral-field carry-forward (introduced v0.51.161), which throws at runtime. Changed to `let`. Mobile triggered it most because SSE/visibility events fire the session-reload path more aggressively (#3162).
|
||||
|
||||
### Added
|
||||
|
||||
- Static JS runtime-error lint guard (`eslint.runtime-guard.config.mjs` + `tests/test_static_js_runtime_lint.py`): a curated, zero-false-positive ESLint check (`no-const-assign`, `no-import-assign`) over `static/**/*.js` that catches the brick-class of runtime errors `node --check` and source-presence tests miss. Runs in the test suite when ESLint is present and skips gracefully otherwise. See `TESTING.md` > "Static JS runtime lint".
|
||||
|
||||
## [v0.51.167] — 2026-05-30 — Release EM (stage-batch49 — iOS-style swipe actions for touch devices + session-list FLIP reflow)
|
||||
|
||||
### Added
|
||||
|
||||
31
TESTING.md
31
TESTING.md
@@ -15,6 +15,37 @@
|
||||
|
||||
---
|
||||
|
||||
## Static JS runtime lint (brick-class regression guard)
|
||||
|
||||
Some JS bugs throw a `TypeError`/`ReferenceError` only when a specific function
|
||||
actually runs in the browser — `node --check` (lazy syntax check), source-presence
|
||||
tests, and even executing the file all miss them. Issue **#3162** was exactly this:
|
||||
a `const` binding reassigned inside `_ensureMessagesLoaded` bricked "load conversation
|
||||
messages" on every mobile message (v0.51.161–166).
|
||||
|
||||
The guard is a curated, zero-false-positive ESLint config (`eslint.runtime-guard.config.mjs`)
|
||||
that runs ONLY runtime-error rules (`no-const-assign`, `no-import-assign`) over
|
||||
`static/**/*.js`. It is NOT a style linter and has no formatting rules.
|
||||
|
||||
```bash
|
||||
# one-time dev setup (ESLint is a dev-only tool; the app stays pure Python + vanilla JS):
|
||||
npm install --no-save --before=<a-date-≥48h-ago> eslint # package-age guard
|
||||
# run the guard:
|
||||
npm run lint:runtime
|
||||
# or directly:
|
||||
npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js"
|
||||
```
|
||||
|
||||
`tests/test_static_js_runtime_lint.py` runs this automatically when eslint is present
|
||||
and **skips gracefully** (clear message) when it isn't — so environments without the
|
||||
node toolchain aren't blocked, while the release gate (which installs eslint) enforces it.
|
||||
|
||||
To widen the guard, fix the pre-existing intentional hits first (as of 2026-05-30:
|
||||
`no-dupe-keys` ×92 i18n locale-fallback, `no-func-assign` ×2 panel override,
|
||||
`no-redeclare` ×1) then promote the rule into the config.
|
||||
|
||||
---
|
||||
|
||||
## How to Use This Document
|
||||
|
||||
Each test has:
|
||||
|
||||
35
eslint.runtime-guard.config.mjs
Normal file
35
eslint.runtime-guard.config.mjs
Normal file
@@ -0,0 +1,35 @@
|
||||
// ESLint flat config — runtime-error guard for the static JS bundle.
|
||||
//
|
||||
// Purpose: catch brick-class runtime errors that `node --check`, source-presence
|
||||
// tests, and even executing the file all MISS, because the error only fires when a
|
||||
// specific function actually runs in the browser. Canonical case: #3162 — a `const`
|
||||
// binding reassigned inside `_ensureMessagesLoaded` threw a TypeError that bricked
|
||||
// "load conversation messages" on every mobile message in v0.51.161-166.
|
||||
//
|
||||
// Scope discipline: ONLY rules that flag genuine "throws at runtime" bugs AND have
|
||||
// ZERO hits on the current clean tree (so the gate is green today and only ever
|
||||
// fails on a NEW regression). This is NOT a style linter.
|
||||
//
|
||||
// Deliberately EXCLUDED (verified to have pre-existing intentional hits 2026-05-30):
|
||||
// - no-dupe-keys (92 hits): intentional i18n locale-fallback override pattern
|
||||
// - no-func-assign (2 hits): switchPanel/switchSettingsSection override pattern
|
||||
// - no-redeclare (1 hit): redeclared loop var in panels.js
|
||||
// If those are cleaned up later, they can be promoted into this guard.
|
||||
//
|
||||
// Run: npx eslint -c eslint.runtime-guard.config.mjs "static/**/*.js"
|
||||
// (tests/test_static_js_runtime_lint.py runs this automatically when eslint is present.)
|
||||
|
||||
export default [
|
||||
// Bundled/minified third-party assets are ES modules and not ours to lint.
|
||||
{ ignores: ["**/vendor/**", "**/*.min.js"] },
|
||||
{
|
||||
files: ["**/*.js"],
|
||||
languageOptions: { ecmaVersion: "latest", sourceType: "script" },
|
||||
rules: {
|
||||
// #3162: reassigning a `const` — runtime TypeError, only fires on execution.
|
||||
"no-const-assign": "error",
|
||||
// Assigning to an import binding — runtime TypeError.
|
||||
"no-import-assign": "error",
|
||||
},
|
||||
},
|
||||
];
|
||||
12
package.json
Normal file
12
package.json
Normal file
@@ -0,0 +1,12 @@
|
||||
{
|
||||
"name": "hermes-webui-devtools",
|
||||
"version": "1.0.0",
|
||||
"private": true,
|
||||
"description": "Dev-only tooling for hermes-webui. NOT a build step — the app remains pure Python + vanilla JS with no bundler. The only dependency is ESLint, used solely as a runtime-error guard over static/*.js (catches brick-class bugs like #3162 const-reassignment that node --check and source-presence tests miss). See TESTING.md > 'Static JS runtime lint'.",
|
||||
"scripts": {
|
||||
"lint:runtime": "eslint --no-config-lookup -c eslint.runtime-guard.config.mjs \"static/**/*.js\""
|
||||
},
|
||||
"devDependencies": {
|
||||
"eslint": "^10.4.0"
|
||||
}
|
||||
}
|
||||
@@ -1321,10 +1321,14 @@ async function _ensureMessagesLoaded(sid) {
|
||||
if (!data || !data.session) return;
|
||||
_messagesTruncated = !!data.session._messages_truncated;
|
||||
_oldestIdx = data.session._messages_offset || 0;
|
||||
const msgs = (data.session.messages || []).filter(m => m && m.role);
|
||||
// #3162: `msgs` is reassigned below by the #3018 ephemeral-field carry-forward,
|
||||
// so it must be `let`, not `const`. The `const` form threw a TypeError inside
|
||||
// _ensureMessagesLoaded() that surfaced as a "Failed to load conversation messages"
|
||||
// toast on every mobile message (SSE/visibility events trigger this reload path
|
||||
// more aggressively on mobile).
|
||||
let msgs = (data.session.messages || []).filter(m => m && m.role);
|
||||
// Check for tool-call metadata on messages (for tool-call card rendering)
|
||||
const hasMessageToolMetadata = msgs.some(m => {
|
||||
if (!m || m.role !== 'assistant') return false;
|
||||
const hasTc = Array.isArray(m.tool_calls) && m.tool_calls.length > 0;
|
||||
const hasTu = Array.isArray(m.content) && m.content.some(p => p && p.type === 'tool_use');
|
||||
return hasTc || hasTu;
|
||||
|
||||
40
tests/test_issue3162_ensure_messages_loaded.py
Normal file
40
tests/test_issue3162_ensure_messages_loaded.py
Normal file
@@ -0,0 +1,40 @@
|
||||
"""Regression test pinning the #3162 fix.
|
||||
|
||||
#3162: `_ensureMessagesLoaded` in static/sessions.js declared `const msgs` but the
|
||||
#3018 ephemeral-field carry-forward reassigns it (`msgs = window._carryForwardEphemeralTurnFields(...)`).
|
||||
`const` -> runtime TypeError -> "Failed to load conversation messages" toast on every
|
||||
mobile message (v0.51.161-166). Fix: `const` -> `let`.
|
||||
|
||||
This is the targeted pin; tests/test_static_js_runtime_lint.py is the general guard.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _ensure_messages_loaded_body() -> str:
|
||||
start = SESSIONS_JS.index("async function _ensureMessagesLoaded")
|
||||
return SESSIONS_JS[start: start + 2000]
|
||||
|
||||
|
||||
def test_ensure_messages_loaded_declares_msgs_with_let():
|
||||
body = _ensure_messages_loaded_body()
|
||||
assert "let msgs = (data.session.messages" in body, (
|
||||
"_ensureMessagesLoaded must declare `let msgs` — it is reassigned by the #3018 "
|
||||
"carry-forward, and `const` throws a runtime TypeError on every mobile message (#3162)"
|
||||
)
|
||||
assert "const msgs = (data.session.messages" not in body, (
|
||||
"found `const msgs` in _ensureMessagesLoaded — this is the #3162 brick-class bug; "
|
||||
"must be `let` because msgs is reassigned"
|
||||
)
|
||||
|
||||
|
||||
def test_ensure_messages_loaded_reassignment_still_present():
|
||||
"""Keep this test meaningful: confirm the reassignment that requires `let` exists.
|
||||
If the carry-forward is removed, revisit whether `let` is still needed."""
|
||||
body = _ensure_messages_loaded_body().replace(" ", "")
|
||||
assert "msgs=window._carryForwardEphemeralTurnFields" in body, (
|
||||
"the #3018 carry-forward reassignment of msgs is gone — re-evaluate the let/const "
|
||||
"decision in _ensureMessagesLoaded"
|
||||
)
|
||||
82
tests/test_static_js_runtime_lint.py
Normal file
82
tests/test_static_js_runtime_lint.py
Normal file
@@ -0,0 +1,82 @@
|
||||
"""Runtime-error lint guard for the static JS bundle (issue #3162).
|
||||
|
||||
Why this exists: #3162 was a brick-class regression — a `const` binding reassigned
|
||||
inside `_ensureMessagesLoaded` threw a `TypeError` that broke "load conversation
|
||||
messages" on every mobile message (v0.51.161-166). Nothing caught it:
|
||||
`node --check` is a lazy syntax check (misses const-assign), source-presence tests
|
||||
asserted the strings existed, and even *running* the file doesn't compile an
|
||||
uncalled function body. Only a real scope-aware linter (ESLint `no-const-assign`)
|
||||
or executing the exact function would have flagged it.
|
||||
|
||||
This test runs ESLint with `eslint.runtime-guard.config.mjs` — a curated set of
|
||||
zero-false-positive RUNTIME-error rules (no style rules) — over `static/**/*.js`.
|
||||
|
||||
Graceful skip: if node or a local/global eslint isn't available, the test SKIPS
|
||||
(with a clear reason) rather than failing — so environments without the toolchain
|
||||
aren't blocked. The release gate runs in an environment where eslint IS installed
|
||||
(see TESTING.md), so the guard is enforced there.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
CONFIG = REPO / "eslint.runtime-guard.config.mjs"
|
||||
|
||||
|
||||
def _find_eslint():
|
||||
"""Return a runnable eslint invocation (list of argv) or None."""
|
||||
# 1. project-local install
|
||||
local = REPO / "node_modules" / ".bin" / "eslint"
|
||||
if local.exists():
|
||||
return [str(local)]
|
||||
# 2. on PATH
|
||||
which = shutil.which("eslint")
|
||||
if which:
|
||||
return [which]
|
||||
return None
|
||||
|
||||
|
||||
@pytest.mark.skipif(not CONFIG.exists(), reason="eslint runtime-guard config missing")
|
||||
def test_static_js_has_no_runtime_error_lint():
|
||||
eslint = _find_eslint()
|
||||
if eslint is None:
|
||||
pytest.skip(
|
||||
"eslint not installed — install with "
|
||||
"`npm install --no-save --before=<48h-ago> eslint` to enforce the "
|
||||
"runtime-error guard locally (CI/release env has it). See TESTING.md."
|
||||
)
|
||||
if shutil.which("node") is None:
|
||||
pytest.skip("node not available")
|
||||
|
||||
cmd = eslint + [
|
||||
"--no-config-lookup",
|
||||
"-c", str(CONFIG),
|
||||
"-f", "json",
|
||||
str(REPO / "static"),
|
||||
]
|
||||
proc = subprocess.run(cmd, capture_output=True, text=True, cwd=str(REPO), timeout=120)
|
||||
|
||||
# ESLint exits non-zero on lint errors; parse the JSON either way.
|
||||
try:
|
||||
results = json.loads(proc.stdout or "[]")
|
||||
except json.JSONDecodeError:
|
||||
pytest.skip(f"eslint produced non-JSON output (env issue): {proc.stderr[:200]}")
|
||||
|
||||
offenders = []
|
||||
for file_result in results:
|
||||
for msg in file_result.get("messages", []):
|
||||
if msg.get("severity") == 2: # error
|
||||
offenders.append(
|
||||
f"{Path(file_result['filePath']).name}:{msg.get('line')}:{msg.get('column')} "
|
||||
f"{msg.get('message')} ({msg.get('ruleId')})"
|
||||
)
|
||||
|
||||
assert not offenders, (
|
||||
"Static JS runtime-error lint failed — these throw at runtime in the browser "
|
||||
"(brick-class, see #3162). Fix them (e.g. `const` -> `let` when reassigned):\n "
|
||||
+ "\n ".join(offenders)
|
||||
)
|
||||
Reference in New Issue
Block a user