* fix(routes): clamp GET /api/session msg_limit to a server-side ceiling Memory: ?msg_limit= had no upper bound — a client could request msg_limit=1000000 (or the frontend's msg_limit=9999 outline-jump path) and force the server to assemble and serialize an unbounded message payload. The frontend's real pagination grows windows by ~30 at a time (initial load 30, load-older +30, msg_before paging 30), so only the outline-jump and pathological/hostile cases ask for huge values. Add _MAX_MSG_LIMIT (500) and clamp at the query-parse site (max(1, min(int(_msg_limit), _MAX_MSG_LIMIT))). The ceiling is generous — far above any legitimate visible-row window — so real pagination is unaffected. The existing _messages_truncated signal already covers the clamped case: when the request exceeds the ceiling, _message_window_for_display returns _messages_offset > 0 and the client learns more rows exist (load-older still works). The bare no-msg_limit path is intentionally unchanged — it is the documented "give me everything" escape hatch used by branch/undo/jump-to-start flows that need the full transcript for correct global indices. Test: tests/test_session_msg_limit_ceiling.py — the ceiling constant is reasonable (100..2000), an over-ceiling request is clamped and signals truncation, and legit sizes (5/30/60/100) are unaltered. Existing window/tail tests (19) unchanged. Contract Routing - Task type: memory-bound fix (unbounded msg_limit request payload). - Touched areas: GET /api/session query parsing (api/routes.py). - Relevant public docs: AGENTS.md, CONTRIBUTING.md, docs/CONTRACTS.md. - State layer mutated: none (read-only endpoint response shaping). - Contract change (additive): ?msg_limit=N is now clamped to _MAX_MSG_LIMIT=500. Compatibility: the only frontend caller above the ceiling is outline.js's msg_limit=9999, which now receives up to 500 rows + _messages_truncated=true (load-older still works). All other callers (30/60) are unaffected. The bare no-msg_limit path is unchanged. Release note: GET /api/session now caps ?msg_limit at 500 rows server-side, bounding the worst-case payload for oversized requests. Model Used: builtin:zai-coding-plan/GLM-5.2 (ZCode agent). * fix(frontend): route msg_limit-heavy paths around the server ceiling PR #6152 clamps GET /api/session ?msg_limit= to a max of 500 server-side. Two frontend paths relied on unbounded msg_limit and silently broke for sessions > 500 visible rows: 1. outline.js _jumpToMessage sent msg_limit=9999 as a "give me everything" hack so it could scroll to any message by absolute index (msg-user-<rawIdx>). Clamped to 500, the target row's DOM id doesn't exist for early messages in long sessions, and the jump silently no-ops (line 119 `if (r)` is falsy). Fix: drop msg_limit=9999 — use the bare messages=1 path, which #6152 left unbounded precisely for callers that need the full transcript. This restores exact pre-#6152 behavior for outline jumps. 2. sessions.js _loadOlderMessages grows msg_limit by +_INITIAL_MSG_LIMIT each load (30 -> 60 -> ... -> 500 -> clamped). Past 500 the server returns the same clamped tail, olderMsgs shrinks to 0, and endless-scroll / "Load earlier" silently stalls — the head becomes unreachable. Fix: once the grown requestedLimit meets/exceeds the new _MSG_LIMIT_MAX mirror (500), switch the primary fetch to msg_before paging (a fixed _INITIAL_MSG_LIMIT backward page keyed off _oldestIdx) — the same bounded paging the existing race-fallback uses, so the head stays reachable for arbitrarily long transcripts. The response is handled by the existing prepend path (tailMatches is naturally false for a page, so the !tailMatches branch reuses the fetched data). Added _MSG_LIMIT_MAX = 500 constant in sessions.js mirroring backend _MAX_MSG_LIMIT, documented as kept-in-sync. Verification: node --check passes on both files; the backend window-contract tests (test_session_tail_payload, test_session_message_window_renderable_tail, 19 tests) still pass. No JS test harness exists in this repo (vanilla JS, manual browser verification per TESTING.md) — manual verification needed: open a > 500-message session, (a) click an outline entry for an early message and confirm it scrolls+flashes, (b) scroll up past 500 loaded rows and confirm "Load earlier" keeps producing older messages without stalling. Contract Routing - Task type: frontend follow-up to a backend memory fix (#6152). - Touched areas: outline jump-to-message (static/outline.js), endless-scroll / load-older (static/sessions.js _loadOlderMessages). - Relevant public docs: AGENTS.md, CONTRIBUTING.md, docs/CONTRACTS.md. - State layer mutated: client-side S.messages (the active-session transcript), same as the existing load-older path. No backend state change. - Scope boundaries: does not weaken the #6152 clamp. The bare messages=1 path (outline) was intentionally left unbounded by #6152; the msg_before paging path (load-older) is bounded by _INITIAL_MSG_LIMIT and never hits the ceiling. Release note: fixes outline jump-to-message and endless scroll silently breaking on sessions longer than 500 messages after the server-side msg_limit ceiling (companion to #6152). Model Used: builtin:zai-coding-plan/GLM-5.2 (ZCode agent). * test(frontend): update static-source assertion for refactored _loadOlderMessages fetch The static-source test test_session_message_loads_keep_explicit_longer_timeouts asserted the exact multi-line shape of the _loadOlderMessages fetch strings. The #6154 refactor wrapped them in a useBeforePaging ternary (tail-growth vs msg_before paging), reindenting them — the contract the test guards (both fetches keep timeoutMs:120000 for large state.db loads) is preserved, but the literal source shape changed, so the assertion failed on shard 0. Updated the assertions to match the new source shape while keeping the test's intent: assert each fetch URL + its {timeoutMs:120000} survive in sessions.js. Model Used: builtin:zai-coding-plan/GLM-5.2 (ZCode agent). * test(msg-limit): stopgap drift guard for _MSG_LIMIT_MAX === _MAX_MSG_LIMIT Reviewer guidance (#6154 round-2): the stopgap drift-test belongs in THIS PR (#6152) because _MAX_MSG_LIMIT is defined here. The frontend _MSG_LIMIT_MAX mirror lives on #6154 (not master yet), so the assertion SKIPS when the JS constant is absent and activates automatically once both branches land on master. The guard: extract _MAX_MSG_LIMIT from api/routes.py and _MSG_LIMIT_MAX from static/sessions.js via regex; assert equality when both present. If they drift, load-older silently stalls at the wrong value (the exact regression #6154 fixed). Retired by #6177 (metadata-exposure), which removes the mirror entirely. Verified: on this branch the cross-check SKIPS (_MSG_LIMIT_MAX absent, as expected — #6154 not landed) and the backend-existence test PASSES. Extraction logic confirmed correct against simulated match (500==500 passes) and drift (999!=500 fails) cases. Model Used: builtin:zai-coding-plan/GLM-5.2 (ZCode agent). * fix(session): prevent silent row-loss in msg_limit ceiling paging (gate follow-up) Adversarial review of #6152/#6154 reproduced two silent row-loss regressions in static/sessions.js; both fixed here, plus a stale source-string assertion update and the CHANGELOG entry: 1. _loadOlderMessages: gate the suffix-continuity heuristic on !useBeforePaging so a raw-row-heavy msg_before cursor page whose visible text repeats the current tail can't be misclassified as the cumulative tail and wholesale-replace it (previously dropped 470 of 940 rows). msg_before pages are always prepended. 2. loadSession same-session refresh: only apply the width-hint msg_limit when it is <= _MSG_LIMIT_MAX; above the ceiling the backend would clamp and silently shrink an already-loaded >500-row transcript to the last 500. Over-ceiling reloads now use the bare full-transcript path (no msg_limit / no expand_renderable). 3. tests/test_webui_external_refresh_frontend.py: update the reloadLimitParam source assertion to the boundedReloadLimit form. Re-gate: Codex adversarial SAFE TO SHIP (940/940 rows retained on cursor paging, 1000/1000 on refresh, no seam duplication, small sessions still use the fast tail window); targeted + sessions/pagination/regression suites green. Co-authored-by: Ruby Hartono <rh-id@users.noreply.github.com> * test: fix two node-harness tests broken by the boundedReloadLimit change - test_issue3162_ensure_messages_loaded: replace the brittle fixed 4500-char window (which the boundedReloadLimit lines pushed the carry-forward past) with robust brace-balance function-body extraction. - test_cross_session_message_load_isolation: define _MSG_LIMIT_MAX=500 in the node harness env; _ensureMessagesLoaded now references it, and an undefined value made boundedReloadLimit=null -> the fetch URL dropped msg_limit/expand_renderable and mismatched the ordered api() harness -> stall/timeout. --------- Co-authored-by: Ruby Hartono <58564005+rh-id@users.noreply.github.com> Co-authored-by: nesquena-hermes <agent@nesquena-hermes> Co-authored-by: Ruby Hartono <rh-id@users.noreply.github.com>
278 lines
13 KiB
Python
278 lines
13 KiB
Python
"""Regression coverage for #2539 client-side api() timeout handling."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import re
|
|
import subprocess
|
|
import textwrap
|
|
from pathlib import Path
|
|
|
|
ROOT = Path(__file__).resolve().parents[1]
|
|
WORKSPACE_JS = ROOT / "static" / "workspace.js"
|
|
SESSIONS_JS = ROOT / "static" / "sessions.js"
|
|
UI_JS = ROOT / "static" / "ui.js"
|
|
PANELS_JS = ROOT / "static" / "panels.js"
|
|
|
|
|
|
def _source(path: Path) -> str:
|
|
return path.read_text(encoding="utf-8")
|
|
|
|
|
|
def _extract_js_function(src: str, name: str) -> str:
|
|
marker = f"async function {name}("
|
|
start = src.find(marker)
|
|
assert start >= 0, f"{name}() function must exist"
|
|
# The api() signature contains a default object literal (`opts={}`), so the
|
|
# function-body brace is the first `{` after the balanced parameter list.
|
|
paren_depth = 0
|
|
close_paren = -1
|
|
for idx in range(start + len(f"async function {name}"), len(src)):
|
|
ch = src[idx]
|
|
if ch == "(":
|
|
paren_depth += 1
|
|
elif ch == ")":
|
|
paren_depth -= 1
|
|
if paren_depth == 0:
|
|
close_paren = idx
|
|
break
|
|
assert close_paren > start, f"{name}() parameter list must close"
|
|
brace = src.find("{", close_paren)
|
|
assert brace > close_paren, f"{name}() function body must start with {{"
|
|
depth = 0
|
|
in_string: str | None = None
|
|
escaped = False
|
|
in_line_comment = False
|
|
in_block_comment = False
|
|
for idx in range(brace, len(src)):
|
|
ch = src[idx]
|
|
nxt = src[idx + 1] if idx + 1 < len(src) else ""
|
|
if in_line_comment:
|
|
if ch == "\n":
|
|
in_line_comment = False
|
|
continue
|
|
if in_block_comment:
|
|
if ch == "*" and nxt == "/":
|
|
in_block_comment = False
|
|
continue
|
|
if in_string:
|
|
if escaped:
|
|
escaped = False
|
|
elif ch == "\\":
|
|
escaped = True
|
|
elif ch == in_string:
|
|
in_string = None
|
|
continue
|
|
if ch == "/" and nxt == "/":
|
|
in_line_comment = True
|
|
continue
|
|
if ch == "/" and nxt == "*":
|
|
in_block_comment = True
|
|
continue
|
|
if ch in ("'", '"', "`"):
|
|
in_string = ch
|
|
continue
|
|
if ch == "{":
|
|
depth += 1
|
|
elif ch == "}":
|
|
depth -= 1
|
|
if depth == 0:
|
|
return src[start : idx + 1]
|
|
raise AssertionError(f"could not extract {name}() body")
|
|
|
|
|
|
def _node_eval(script: str, timeout: float = 2.0) -> subprocess.CompletedProcess[str]:
|
|
return subprocess.run(
|
|
["node", "-e", script],
|
|
cwd=ROOT,
|
|
text=True,
|
|
capture_output=True,
|
|
timeout=timeout,
|
|
check=False,
|
|
)
|
|
|
|
|
|
def test_api_rejects_hung_fetch_with_timeout_and_toast():
|
|
"""A hung fetch must reject quickly and surface a recognizable timeout toast."""
|
|
api_fn = _extract_js_function(_source(WORKSPACE_JS), "api")
|
|
script = textwrap.dedent(
|
|
f"""
|
|
const events=[];
|
|
global.document={{baseURI:'http://example.test/hermes/'}};
|
|
global.location={{href:'http://example.test/hermes/',pathname:'/hermes/',search:''}};
|
|
global.window={{location:global.location}};
|
|
global.showToast=(msg,ms,type)=>events.push({{msg:String(msg),ms,type}});
|
|
global.fetch=(url,opts)=>new Promise(()=>{{
|
|
if(opts&&opts.signal)opts.signal.addEventListener('abort',()=>events.push({{aborted:true}}));
|
|
}});
|
|
{api_fn}
|
|
api('/api/sessions',{{timeoutMs:20}})
|
|
.then(()=>{{console.error('resolved unexpectedly');process.exit(2);}})
|
|
.catch(err=>{{
|
|
console.log(JSON.stringify({{message:String(err&&err.message||err),events}}));
|
|
process.exit(0);
|
|
}});
|
|
setTimeout(()=>{{console.error('api did not reject after timeoutMs');process.exit(3);}},250);
|
|
"""
|
|
)
|
|
result = _node_eval(script, timeout=1.0)
|
|
assert result.returncode == 0, result.stderr or result.stdout
|
|
payload = json.loads(result.stdout.strip())
|
|
assert "timed out" in payload["message"].lower()
|
|
assert any(event.get("aborted") for event in payload["events"]), payload
|
|
assert any("request timed out" in event.get("msg", "").lower() for event in payload["events"]), payload
|
|
assert any(event.get("type") == "error" for event in payload["events"]), payload
|
|
|
|
|
|
def test_api_rejects_stalled_response_body_with_timeout():
|
|
"""The timeout must stay active through JSON/text body consumption, not only headers."""
|
|
api_fn = _extract_js_function(_source(WORKSPACE_JS), "api")
|
|
script = textwrap.dedent(
|
|
f"""
|
|
const events=[];
|
|
global.document={{baseURI:'http://example.test/hermes/'}};
|
|
global.location={{href:'http://example.test/hermes/',pathname:'/hermes/',search:''}};
|
|
global.window={{location:global.location}};
|
|
global.showToast=(msg,ms,type)=>events.push({{msg:String(msg),ms,type}});
|
|
global.fetch=(url,opts)=>Promise.resolve({{
|
|
ok:true,
|
|
headers:{{get:()=> 'application/json'}},
|
|
json:()=>new Promise(()=>{{
|
|
if(opts&&opts.signal)opts.signal.addEventListener('abort',()=>events.push({{aborted:true}}));
|
|
}}),
|
|
text:()=>Promise.resolve('')
|
|
}});
|
|
{api_fn}
|
|
api('/api/sessions',{{timeoutMs:20}})
|
|
.then(()=>{{console.error('resolved unexpectedly');process.exit(2);}})
|
|
.catch(err=>{{
|
|
console.log(JSON.stringify({{message:String(err&&err.message||err),events}}));
|
|
process.exit(0);
|
|
}});
|
|
setTimeout(()=>{{console.error('api body read did not reject after timeoutMs');process.exit(3);}},250);
|
|
"""
|
|
)
|
|
result = _node_eval(script, timeout=1.0)
|
|
assert result.returncode == 0, result.stderr or result.stdout
|
|
payload = json.loads(result.stdout.strip())
|
|
assert "timed out" in payload["message"].lower()
|
|
assert any(event.get("aborted") for event in payload["events"]), payload
|
|
|
|
|
|
def test_api_can_suppress_timeout_toast_for_background_pollers():
|
|
"""Passive pollers need abort/reject cleanup without a user-visible toast."""
|
|
api_fn = _extract_js_function(_source(WORKSPACE_JS), "api")
|
|
script = textwrap.dedent(
|
|
f"""
|
|
const events=[];
|
|
global.document={{baseURI:'http://example.test/hermes/'}};
|
|
global.location={{href:'http://example.test/hermes/',pathname:'/hermes/',search:''}};
|
|
global.window={{location:global.location}};
|
|
global.showToast=(msg,ms,type)=>events.push({{msg:String(msg),ms,type}});
|
|
global.fetch=(url,opts)=>new Promise(()=>{{
|
|
if(opts&&opts.signal)opts.signal.addEventListener('abort',()=>events.push({{aborted:true}}));
|
|
}});
|
|
{api_fn}
|
|
api('/api/sessions',{{timeoutMs:20,timeoutToast:false}})
|
|
.then(()=>{{console.error('resolved unexpectedly');process.exit(2);}})
|
|
.catch(err=>{{
|
|
console.log(JSON.stringify({{message:String(err&&err.message||err),events}}));
|
|
process.exit(0);
|
|
}});
|
|
setTimeout(()=>{{console.error('api did not reject after timeoutMs');process.exit(3);}},250);
|
|
"""
|
|
)
|
|
result = _node_eval(script, timeout=1.0)
|
|
assert result.returncode == 0, result.stderr or result.stdout
|
|
payload = json.loads(result.stdout.strip())
|
|
assert "timed out" in payload["message"].lower()
|
|
assert any(event.get("aborted") for event in payload["events"]), payload
|
|
assert not any("msg" in event for event in payload["events"]), payload
|
|
|
|
|
|
def test_api_has_default_timeout_and_per_call_override_contract():
|
|
src = _source(WORKSPACE_JS)
|
|
body = _extract_js_function(src, "api")
|
|
assert "timeoutMs" in body, "api() must accept opts.timeoutMs as a per-call override"
|
|
assert "timeoutToast" in body, "api() must let passive callers suppress timeout toasts"
|
|
|
|
assert "30000" in body, "api() must default browser API calls to a 30s timeout"
|
|
assert "AbortController" in body, "api() must abort hung fetches with AbortController"
|
|
assert "delete fetchOpts.timeoutMs" in body, "api() must strip timeoutMs before calling fetch()"
|
|
assert "delete fetchOpts.timeoutToast" in body, "api() must strip timeoutToast before calling fetch()"
|
|
|
|
fetch_call = re.search(r"fetch\(url\.href,\{.*?\.\.\.fetchOpts.*?\}\)", body, re.DOTALL)
|
|
assert fetch_call, "api() must call fetch() with sanitized fetchOpts"
|
|
assert "...opts" not in fetch_call.group(0), "api() must not spread raw opts into fetch()"
|
|
assert "timeoutMs" not in fetch_call.group(0), "api() must not forward timeoutMs to fetch()"
|
|
|
|
|
|
def test_update_flows_keep_explicit_longer_timeouts():
|
|
"""Legitimately long update flows should not inherit the generic 30s guard."""
|
|
src = _source(UI_JS)
|
|
panels = _source(PANELS_JS)
|
|
# /api/updates/check builds its body in a _checkBody var (to optionally add
|
|
# an explicit channel), but must still carry the 60s timeout override.
|
|
assert "api('/api/updates/check',{method:'POST',body:JSON.stringify(_checkBody),timeoutMs:60000})" in panels
|
|
assert "api('/api/updates/summary',{method:'POST',body:JSON.stringify({updates:scopedUpdates,target:target||null}),timeoutMs:60000})" in src
|
|
# apply/force now build their body inline to optionally carry the offered
|
|
# channel (Codex debounce-race fix), but MUST still carry the 120s override.
|
|
assert "api('/api/updates/apply',{method:'POST',body:JSON.stringify(_applyBody),timeoutMs:120000})" in src
|
|
assert "api('/api/updates/force',{method:'POST',body:JSON.stringify((()=>{const b={target};const _ch=window._updateData?.[target]?.channel;if(_ch==='stable'||_ch==='experimental')b.channel=_ch;return b;})()),timeoutMs:120000})" in src
|
|
|
|
|
|
def test_session_message_loads_keep_explicit_longer_timeouts():
|
|
"""Large state.db installs can take longer than the generic 30s API timeout."""
|
|
src = _source(SESSIONS_JS)
|
|
assert (
|
|
"api(\n"
|
|
" `/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0${reloadLimitParam}${expandParam}`,\n"
|
|
" {timeoutMs:120000}\n"
|
|
" )"
|
|
) in src
|
|
# _loadOlderMessages now picks between two strategies (tail-growth vs
|
|
# msg_before paging) via a useBeforePaging ternary, but both keep the long
|
|
# timeoutMs:120000. Assert each URL + timeout survives in the source.
|
|
assert (
|
|
"`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`,\n"
|
|
" {timeoutMs:120000}"
|
|
) in src
|
|
assert (
|
|
"`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=${requestedLimit}`,\n"
|
|
" {timeoutMs:120000}"
|
|
) in src
|
|
|
|
|
|
def test_passive_background_polls_suppress_timeout_toasts():
|
|
"""Passive refreshes should be best-effort and not emit generic timeout toasts."""
|
|
workspace = _source(WORKSPACE_JS)
|
|
sessions = _source(SESSIONS_JS)
|
|
messages = _source(ROOT / "static" / "messages.js")
|
|
ui = _source(UI_JS)
|
|
panels = _source(PANELS_JS)
|
|
|
|
assert "api('/api/client-events/log',{method:'POST',body:JSON.stringify(payload),timeoutMs:3000,timeoutToast:false})" in workspace
|
|
assert "const sessionRequestOpts={" in sessions
|
|
assert "timeoutToast:false" in sessions
|
|
assert "api('/api/sessions' + sessionListQS,sessionRequestOpts)" in sessions
|
|
assert "api('/api/projects' + projectQS,{timeoutToast:false})" in sessions
|
|
assert "api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=0&resolve_model=0`,{timeoutToast:false})" in sessions
|
|
assert 'api("/api/approval/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false})' in messages
|
|
assert 'api("/api/clarify/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false})' in messages
|
|
assert "api('/api/dashboard/status',{timeoutToast:false})" in ui
|
|
assert "api('/api/system/health',{timeoutToast:false})" in ui
|
|
assert "api('/api/health/agent',{timeoutToast:false})" in ui
|
|
assert "api(`/api/crons/status?job_id=${encodeURIComponent(jobId)}`,{timeoutToast:false})" in panels
|
|
|
|
|
|
def test_new_session_inflight_cleanup_still_runs_after_api_rejects():
|
|
"""newSession() must keep its finally cleanup path so timeout rejections unpin the UI."""
|
|
src = _source(SESSIONS_JS)
|
|
start = src.find("async function newSession")
|
|
assert start >= 0, "newSession() must exist"
|
|
finally_idx = src.find("}finally{", start)
|
|
assert finally_idx > start, "newSession() must keep a finally cleanup block"
|
|
block = src[finally_idx : src.find("\n}", finally_idx) + 2]
|
|
assert "_newSessionInFlight=null" in block
|
|
assert "_setNewSessionPending(false)" in block
|