* 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>
This commit is contained in:
10 files changed
+294
-45
No files matched your search
@@ -9,6 +9,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`GET /api/session?msg_limit=` is now bounded by a server-side ceiling, and the frontend paginates around it without dropping rows.** A client could request `msg_limit=1000000` (or an outline-jump path asked for `9999`) and force the server to assemble and serialize an unbounded message payload. The backend now clamps `?msg_limit=` to `[1, 500]` via a dedicated `_parse_msg_limit()` helper and sets the existing `_messages_truncated` signal when it clamps; the bare no-`msg_limit` path (branch/undo/jump-to-start) still returns the full transcript. The frontend mirrors the ceiling: `_loadOlderMessages` grows its tail window below the ceiling and switches to bounded `msg_before` backward paging once the server would clamp, and the outline jump uses the bare full-transcript path instead of the old `msg_limit=9999` hack. Two silent row-loss regressions found during review were fixed before ship: a raw-row-heavy `msg_before` page that textually repeated the current tail could be misclassified as the cumulative tail and wholesale-replace it (losing older rows), and a same-session refresh above the ceiling could shrink an already-loaded >500-row transcript to the last 500 — both now route through the correct paging/full-transcript path. Thanks @rh-id. (#6152, #6154, #6177)
|
||||
|
||||
- **Background subagents no longer fail to persist when the parent's cached agent is reused mid-turn.** A parent session's `SessionDB` handle is shared by reference with the background subagents it spawns (`delegate_tool`), but the cached-agent reuse path unconditionally closed that handle when the parent got a new turn / server-side wakeup — so every still-running child then failed `append_message` with `'NoneType' object has no attribute 'execute'` (observed 600+ times under one parent). The reuse path now keeps a still-open handle (closing only the unused fresh one, preserving the #1421 FD-leak fix) and only replaces a dead/missing one. The credential self-heal path was hardened the same way: if rebuilding the handle fails it degrades cleanly to a lazy reinit instead of reusing a closed handle. Thanks @carlotestor. (#6143)
|
||||
|
||||
- **The CLI/cron session-list projection cache is now bounded.** `_CLI_SESSIONS_CACHE` was an unbounded plain dict of deep-copied session-list projections whose fingerprint-folded key advances on every streamed message, so orphaned heavy copies accumulated on a long-lived server until the next structural clear. It's now an `OrderedDict` capped at 8 entries with drop-oldest LRU eviction (recency refresh on hit), mirroring the existing `_CLAUDE_CODE_PARSE_CACHE` / `_SIDECAR_METADATA_CACHE` pattern; TTL stays the freshness control. Thanks @rh-id. (#6140)
|
||||
|
||||
+35
-5
@@ -8446,6 +8446,34 @@ def _message_window_for_display(messages, msg_limit=None, msg_before=None, expan
|
||||
|
||||
|
||||
_LIMITED_TOOL_CONTENT_MAX_CHARS = 4096
|
||||
# Server-side ceiling on the ?msg_limit= tail-window size. A client could
|
||||
# otherwise request msg_limit=1000000 and force the server to assemble and
|
||||
# serialize an unbounded message payload (the frontend's own pagination grows
|
||||
# by ~30 at a time, with one outline-jump path asking for 9999). The ceiling is
|
||||
# generous — far above any legitimate visible-row window — so real pagination is
|
||||
# unaffected; it only caps the pathological/oversized request. When the request
|
||||
# exceeds the ceiling the response is silently clamped and _messages_truncated
|
||||
# is set (the existing truncation signal already covers "more rows exist").
|
||||
_MAX_MSG_LIMIT = 500
|
||||
|
||||
|
||||
def _parse_msg_limit(raw):
|
||||
"""Parse and clamp the ``?msg_limit=`` query value.
|
||||
|
||||
Returns a positive int clamped to ``[1, _MAX_MSG_LIMIT]``, or ``None`` when
|
||||
the value is absent/empty/malformed (the bare no-``msg_limit`` path, which
|
||||
intentionally returns the full transcript for callers that need it).
|
||||
Extracted from the handler so the clamp expression has direct test coverage.
|
||||
"""
|
||||
if not raw:
|
||||
return None
|
||||
try:
|
||||
value = int(raw)
|
||||
except (TypeError, ValueError):
|
||||
return None
|
||||
return max(1, min(value, _MAX_MSG_LIMIT))
|
||||
|
||||
|
||||
# Defensive row backstop for the GET /api/session display path's state.db read.
|
||||
# This is NOT a semantic window (the display window counts visible rows
|
||||
# post-reconciliation via _message_window_for_display); it is a safety net so a
|
||||
@@ -12400,11 +12428,13 @@ def handle_get(handler, parsed) -> bool:
|
||||
# transcript rows. Hidden tool-result rows do not consume the budget;
|
||||
# they are included only when they sit inside the selected window and
|
||||
# are bounded before serialization. Older rows load on-demand.
|
||||
_msg_limit = query.get("msg_limit", [None])[0]
|
||||
try:
|
||||
msg_limit = max(1, int(_msg_limit)) if _msg_limit else None
|
||||
except (ValueError, TypeError):
|
||||
msg_limit = None
|
||||
# Clamp to _MAX_MSG_LIMIT so an oversized request (e.g. msg_limit=9999
|
||||
# from an outline jump, or a hostile value) can't force an unbounded
|
||||
# payload; the existing _messages_truncated signal covers the clamped
|
||||
# case (the client sees there are more rows than returned). Parsing +
|
||||
# clamping live in _parse_msg_limit so the expression has direct test
|
||||
# coverage; None means the bare no-msg_limit path (full transcript).
|
||||
msg_limit = _parse_msg_limit(query.get("msg_limit", [None])[0])
|
||||
# ?msg_before=N — 0-based index into the full message array.
|
||||
# Returns messages before this index (for scroll-to-top lazy loading).
|
||||
# Combined with msg_limit for paging.
|
||||
|
||||
+7
-1
@@ -104,10 +104,16 @@ function _jumpToMessage(rawIdx) {
|
||||
}
|
||||
|
||||
// Row is outside the render window — reload the full session and retry.
|
||||
// Use the bare messages=1 path (no msg_limit) so the server returns the
|
||||
// COMPLETE transcript: the target row is addressed by absolute index
|
||||
// (msg-user-<rawIdx>), so a bounded tail window would miss early rows.
|
||||
// (A previous version sent msg_limit=9999 as a "give me everything" hack,
|
||||
// but the server now clamps msg_limit, so the bare path is the correct way
|
||||
// to request the full transcript here.)
|
||||
if (typeof api !== 'function') return;
|
||||
if (S.busy || S.activeStreamId) return;
|
||||
api('/api/session?session_id=' + encodeURIComponent(sid) +
|
||||
'&messages=1&resolve_model=0&msg_limit=9999')
|
||||
'&messages=1&resolve_model=0')
|
||||
.then(function(data) {
|
||||
if (!data || !data.session) return;
|
||||
if (!S.session || S.session.session_id !== sid) return; // session switched
|
||||
|
||||
+74
-26
@@ -2928,6 +2928,19 @@ let _messagesTruncated = false;
|
||||
// server-bounded and do not consume the visible-message budget.
|
||||
// Older messages are loaded on-demand via _loadOlderMessages().
|
||||
const _INITIAL_MSG_LIMIT = 30;
|
||||
// ============================================================================
|
||||
// COUPLED CONSTANT — keep in sync with api/routes.py:_MAX_MSG_LIMIT.
|
||||
// ============================================================================
|
||||
// This is a hand-mirrored copy of the backend's GET /api/session ?msg_limit=
|
||||
// ceiling. _loadOlderMessages grows its msg_limit tail window by
|
||||
// +_INITIAL_MSG_LIMIT each load; once growth would exceed this ceiling the
|
||||
// server clamps and the tail stops growing, so we switch to msg_before paging
|
||||
// (a fixed-size backward page keyed off _oldestIdx) instead. If you change the
|
||||
// backend _MAX_MSG_LIMIT, change this value to match — otherwise load-older
|
||||
// silently stalls again for long sessions. The durable fix (so this mirror
|
||||
// isn't needed) is to expose the ceiling in the /api/session metadata response
|
||||
// and read it here; see tracking issue for that follow-up.
|
||||
const _MSG_LIMIT_MAX = 500;
|
||||
let _sameSessionForceReloadHint = null;
|
||||
|
||||
function _currentLoadedRenderableMessageCount(){
|
||||
@@ -3032,11 +3045,19 @@ async function _ensureMessagesLoaded(sid, opts) {
|
||||
}
|
||||
// Fetch session messages with a tail window for fast initial load.
|
||||
const reloadLimit = _messageReloadLimitForSession(sid); // defaults to _INITIAL_MSG_LIMIT
|
||||
const reloadLimitParam = reloadLimit ? `&msg_limit=${reloadLimit}` : '';
|
||||
// A reload window above the server's msg_limit ceiling would be clamped by
|
||||
// the backend (returning only the last _MSG_LIMIT_MAX rows), which can
|
||||
// silently SHRINK an already-loaded transcript that had more than the ceiling
|
||||
// of rows visible (rows 400–999 replaced by 500–999). When the requested
|
||||
// window exceeds the ceiling, fall back to the bare full-transcript request
|
||||
// (no msg_limit / no expand_renderable) so a same-session refresh never drops
|
||||
// already-loaded older rows (Codex gate #6154, silent row-loss).
|
||||
const boundedReloadLimit = (reloadLimit && reloadLimit <= _MSG_LIMIT_MAX) ? reloadLimit : null;
|
||||
const reloadLimitParam = boundedReloadLimit ? `&msg_limit=${boundedReloadLimit}` : '';
|
||||
// Older frontends used expand_renderable=1 to request visible-row expansion.
|
||||
// The server now counts msg_limit by visible transcript rows by default; keep
|
||||
// the flag for compatibility with mixed-version deployments.
|
||||
const expandParam = reloadLimit ? '&expand_renderable=1' : '';
|
||||
const expandParam = boundedReloadLimit ? '&expand_renderable=1' : '';
|
||||
let data;
|
||||
try {
|
||||
data = await api(
|
||||
@@ -3531,18 +3552,34 @@ async function _loadOlderMessages() {
|
||||
// rebuilt transcript (#1937).
|
||||
const startGeneration = _messagesGeneration;
|
||||
try {
|
||||
// Ask the server for a larger authoritative tail window instead of a
|
||||
// separate msg_before page. The same /api/session contract handles both —
|
||||
// post-#2716 the backend always runs the full append-only merge, so a
|
||||
// larger msg_limit on the same call produces the same merged transcript
|
||||
// we'd get by stitching pages, but without client-side index bookkeeping.
|
||||
// Cumulative growth: each "load more" asks for currentLoaded + 30, and the
|
||||
// newly exposed head is what we expose to the user.
|
||||
// Two strategies, chosen by whether the growing tail window still fits under
|
||||
// the server's msg_limit ceiling (_MSG_LIMIT_MAX, mirroring backend
|
||||
// _MAX_MSG_LIMIT):
|
||||
//
|
||||
// - Below the ceiling: ask for a larger authoritative tail window
|
||||
// (currentLoaded + _INITIAL_MSG_LIMIT). Post-#2716 the backend runs the
|
||||
// full append-only merge, so a larger msg_limit produces the same merged
|
||||
// transcript we'd get by stitching pages, without client-side index
|
||||
// bookkeeping. The newly exposed head is what we expose to the user.
|
||||
//
|
||||
// - At/above the ceiling: the server clamps msg_limit, so the tail window
|
||||
// stops growing and this strategy would stall (the same clamped tail is
|
||||
// returned, olderMsgs -> 0). Switch to msg_before paging — a fixed
|
||||
// _INITIAL_MSG_LIMIT backward page keyed off _oldestIdx — which is
|
||||
// bounded and never hits the ceiling, so the head stays reachable for
|
||||
// arbitrarily long transcripts. (This is the same paging request the
|
||||
// race-fallback below uses, proven correct there.)
|
||||
const requestedLimit = Math.max(_INITIAL_MSG_LIMIT, (S.messages || []).length + _INITIAL_MSG_LIMIT);
|
||||
const data = await api(
|
||||
`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=${requestedLimit}`,
|
||||
{timeoutMs:120000}
|
||||
);
|
||||
const useBeforePaging = requestedLimit >= _MSG_LIMIT_MAX;
|
||||
const data = useBeforePaging
|
||||
? await api(
|
||||
`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`,
|
||||
{timeoutMs:120000}
|
||||
)
|
||||
: await api(
|
||||
`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=${requestedLimit}`,
|
||||
{timeoutMs:120000}
|
||||
);
|
||||
// Guard: api() may have redirected (401) and returned undefined.
|
||||
if (!data || !data.session) { _loadingOlder = false; return; }
|
||||
// - response shape sane
|
||||
@@ -3569,7 +3606,14 @@ async function _loadOlderMessages() {
|
||||
// the server appended new messages (or merge filtered something) while we
|
||||
// were awaiting, the suffix won't line up — fall back to the legacy
|
||||
// msg_before page so we never drop visible older messages on the floor.
|
||||
let tailMatches = expandedMsgs.length >= currentLen;
|
||||
// When useBeforePaging is true, `data` is a bounded msg_before OLDER page,
|
||||
// not a cumulative tail. A raw-row-heavy older page whose visible text
|
||||
// repeats the current tail could otherwise pass the suffix check below and
|
||||
// be wholesale-replaced AS IF it were the full tail — silently discarding
|
||||
// the current (newer) rows and marking history complete. Gate the suffix
|
||||
// heuristic on !useBeforePaging so every msg_before page is always treated
|
||||
// as an older page and prepended (Codex gate #6154, silent row-loss).
|
||||
let tailMatches = !useBeforePaging && expandedMsgs.length >= currentLen;
|
||||
if (tailMatches && currentLen > 0) {
|
||||
const start = expandedMsgs.length - currentLen;
|
||||
for (let i = 0; i < currentLen; i++) {
|
||||
@@ -3583,18 +3627,22 @@ async function _loadOlderMessages() {
|
||||
let olderMsgs = expandedMsgs.slice(0, olderCount);
|
||||
let nextMessages = expandedMsgs;
|
||||
if (!tailMatches) {
|
||||
// Race fallback: keep the legacy index-page request as the
|
||||
// correctness-preserving alternative. Same guards reapplied because
|
||||
// we just awaited again.
|
||||
const fallback = await api(
|
||||
`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`,
|
||||
{timeoutMs:120000}
|
||||
);
|
||||
if (!fallback || !fallback.session) { _loadingOlder = false; return; }
|
||||
if (!S.session || S.session.session_id !== sid) return;
|
||||
if (_loadingSessionId !== null && _loadingSessionId !== sid) return;
|
||||
if (_messagesGeneration !== startGeneration) return;
|
||||
responseSession = fallback.session;
|
||||
// Race fallback (or the over-ceiling msg_before primary path): keep the
|
||||
// legacy index-page request as the correctness-preserving alternative.
|
||||
// When useBeforePaging is true we already fetched a msg_before page as
|
||||
// the primary `data`, so reuse it instead of re-fetching. Same guards
|
||||
// reapplied because we just awaited again (skipped for the reuse case).
|
||||
if (!useBeforePaging) {
|
||||
const fallback = await api(
|
||||
`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`,
|
||||
{timeoutMs:120000}
|
||||
);
|
||||
if (!fallback || !fallback.session) { _loadingOlder = false; return; }
|
||||
if (!S.session || S.session.session_id !== sid) return;
|
||||
if (_loadingSessionId !== null && _loadingSessionId !== sid) return;
|
||||
if (_messagesGeneration !== startGeneration) return;
|
||||
responseSession = fallback.session;
|
||||
}
|
||||
olderMsgs = (responseSession.messages || []).filter(m => m && m.role);
|
||||
nextMessages = [...olderMsgs, ...S.messages];
|
||||
}
|
||||
|
||||
@@ -230,17 +230,16 @@ def test_session_message_loads_keep_explicit_longer_timeouts():
|
||||
" {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(\n"
|
||||
" `/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=${requestedLimit}`,\n"
|
||||
" {timeoutMs:120000}\n"
|
||||
" )"
|
||||
"`/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(\n"
|
||||
" `/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`,\n"
|
||||
" {timeoutMs:120000}\n"
|
||||
" )"
|
||||
"`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=${requestedLimit}`,\n"
|
||||
" {timeoutMs:120000}"
|
||||
) in src
|
||||
|
||||
|
||||
|
||||
@@ -208,6 +208,13 @@ function createEnvironment() {
|
||||
globalThis._oldestIdx = 0;
|
||||
globalThis._messageRenderWindowSize = 0;
|
||||
globalThis._messageReloadLimitForSession = () => 2;
|
||||
// sessions.js module-level const, referenced by _ensureMessagesLoaded's
|
||||
// boundedReloadLimit ceiling check (#6152/#6154). Not one of the extracted
|
||||
// functions, so define it in the harness (matching the real value) or the
|
||||
// reload-width path resolves it as undefined -> boundedReloadLimit=null ->
|
||||
// the fetch URL drops msg_limit/expand_renderable and mismatches the
|
||||
// enqueued buildMessageUrl(), stalling the ordered api() harness.
|
||||
globalThis._MSG_LIMIT_MAX = 500;
|
||||
globalThis._currentMessageRenderWindowSize = () => 1;
|
||||
globalThis._messageRenderableMessageCount = () => 2;
|
||||
|
||||
|
||||
@@ -15,10 +15,25 @@ SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
|
||||
def _ensure_messages_loaded_body() -> str:
|
||||
start = SESSIONS_JS.index("async function _ensureMessagesLoaded")
|
||||
# Window widened (#3326 added reload-width-hint handling inside this function,
|
||||
# pushing the carry-forward reassignment further down; #3790 added the
|
||||
# cold-load expand_renderable param + comment, pushing it further still).
|
||||
return SESSIONS_JS[start: start + 4500]
|
||||
# Extract the ACTUAL function body via brace-balance instead of a fixed
|
||||
# character window. The old fixed 4500-char window kept needing bumps as the
|
||||
# function grew (#3326 reload-width-hint, #3790 cold-load expand_renderable,
|
||||
# #6152/#6154 the msg_limit ceiling boundedReloadLimit path — each pushed the
|
||||
# carry-forward reassignment further down and eventually past the window).
|
||||
# Balancing braces from the opening `{` is robust to any in-function growth.
|
||||
brace = SESSIONS_JS.index("{", start)
|
||||
depth = 0
|
||||
end = brace
|
||||
for i in range(brace, len(SESSIONS_JS)):
|
||||
c = SESSIONS_JS[i]
|
||||
if c == "{":
|
||||
depth += 1
|
||||
elif c == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end = i + 1
|
||||
break
|
||||
return SESSIONS_JS[start:end]
|
||||
|
||||
|
||||
def test_ensure_messages_loaded_declares_msgs_with_let():
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
"""Stopgap drift guard: the frontend ``_MSG_LIMIT_MAX`` mirror must match the
|
||||
backend ``_MAX_MSG_LIMIT`` ceiling.
|
||||
|
||||
PR #6152 introduced the server-side ``_MAX_MSG_LIMIT`` ceiling on
|
||||
``GET /api/session ?msg_limit=``. PR #6154 added a hand-mirrored
|
||||
``_MSG_LIMIT_MAX`` in ``static/sessions.js`` so the frontend knows when to
|
||||
switch from the growing-tail strategy to ``msg_before`` paging. If the two
|
||||
drift, load-older silently stalls at the old value — the exact regression #6154
|
||||
fixed. The durable fix (#6177) exposes the ceiling via ``/api/session`` metadata
|
||||
so the mirror isn't needed; until then, this static-source assertion catches the
|
||||
drift.
|
||||
|
||||
Placement note: this test ships in #6152 (where ``_MAX_MSG_LIMIT`` is defined).
|
||||
The frontend ``_MSG_LIMIT_MAX`` only exists once #6154 lands, so the assertion
|
||||
SKIPS when the JS constant is absent — it activates automatically once both
|
||||
branches are on master, and bites if the two values ever diverge.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def _backend_max_msg_limit() -> int:
|
||||
"""Extract ``_MAX_MSG_LIMIT = <int>`` from api/routes.py."""
|
||||
src = (ROOT / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
m = re.search(r"^_MAX_MSG_LIMIT\s*=\s*(\d+)\s*$", src, re.MULTILINE)
|
||||
assert m, "_MAX_MSG_LIMIT not found in api/routes.py"
|
||||
return int(m.group(1))
|
||||
|
||||
|
||||
def _frontend_msg_limit_max():
|
||||
"""Extract ``_MSG_LIMIT_MAX = <int>`` from static/sessions.js, or return
|
||||
None when the constant is absent (PR #6154 not yet landed)."""
|
||||
sessions = ROOT / "static" / "sessions.js"
|
||||
if not sessions.exists():
|
||||
return None
|
||||
src = sessions.read_text(encoding="utf-8")
|
||||
m = re.search(r"const\s+_MSG_LIMIT_MAX\s*=\s*(\d+)\s*;", src)
|
||||
return int(m.group(1)) if m else None
|
||||
|
||||
|
||||
def test_frontend_msg_limit_max_matches_backend_ceiling():
|
||||
"""When both constants exist (i.e. #6152 + #6154 have both landed), the
|
||||
frontend mirror MUST equal the backend ceiling or load-older silently stalls
|
||||
for long sessions. Skips gracefully when the JS constant is absent (the
|
||||
#6152-only state) so this test ships atomically with the backend constant
|
||||
without depending on the #6154 branch."""
|
||||
backend = _backend_max_msg_limit()
|
||||
frontend = _frontend_msg_limit_max()
|
||||
if frontend is None:
|
||||
# PR #6154 (the frontend mirror) hasn't landed yet — skip rather than
|
||||
# fail. Once both are on master this branch is exercised and bites on
|
||||
# any drift.
|
||||
import pytest
|
||||
pytest.skip("_MSG_LIMIT_MAX not yet present in static/sessions.js "
|
||||
"(lands in #6154); drift guard activates once both are on master")
|
||||
assert frontend == backend, (
|
||||
f"frontend _MSG_LIMIT_MAX ({frontend}) != backend _MAX_MSG_LIMIT ({backend}): "
|
||||
f"load-older will silently stall at the wrong value. Update both together, "
|
||||
f"or land #6177 (metadata-exposure) to retire the mirror."
|
||||
)
|
||||
|
||||
|
||||
def test_backend_max_msg_limit_is_reasonable():
|
||||
"""The backend ceiling must exist and stay in a sane range (the frontend's
|
||||
real pagination grows by ~30, so anything in [100, 2000] is generous)."""
|
||||
backend = _backend_max_msg_limit()
|
||||
assert 100 <= backend <= 2000, f"_MAX_MSG_LIMIT out of expected range: {backend}"
|
||||
@@ -0,0 +1,66 @@
|
||||
"""Tests: server-side ``msg_limit`` ceiling on ``GET /api/session``.
|
||||
|
||||
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 handler now clamps ``msg_limit`` to
|
||||
``_MAX_MSG_LIMIT`` (generous — far above any legitimate visible-row window) so
|
||||
real pagination is unaffected while the pathological/oversized request is
|
||||
bounded. The existing ``_messages_truncated`` signal covers the clamped case.
|
||||
|
||||
The parse+clamp lives in ``_parse_msg_limit`` so the clamping expression has
|
||||
direct test coverage (driving the handler end-to-end would require a live
|
||||
session + state.db; the helper is the unit under test).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from api.routes import _MAX_MSG_LIMIT, _parse_msg_limit
|
||||
|
||||
|
||||
def test_max_msg_limit_constant_is_reasonable():
|
||||
"""The ceiling must exist and be well above legitimate pagination sizes
|
||||
(the frontend grows windows by ~30, initial load is 30) but finite."""
|
||||
assert isinstance(_MAX_MSG_LIMIT, int)
|
||||
assert 100 <= _MAX_MSG_LIMIT <= 2000, _MAX_MSG_LIMIT
|
||||
|
||||
|
||||
def test_parse_msg_limit_none_when_absent_or_empty():
|
||||
"""No value (the bare no-msg_limit path) returns None — intentionally the
|
||||
'full transcript' escape hatch for branch/undo/jump-to-start flows."""
|
||||
assert _parse_msg_limit(None) is None
|
||||
assert _parse_msg_limit("") is None
|
||||
|
||||
|
||||
def test_parse_msg_limit_none_when_malformed():
|
||||
"""A non-numeric value returns None rather than raising (matches the
|
||||
pre-fix behavior where a ValueError fell through to msg_limit=None)."""
|
||||
assert _parse_msg_limit("not-a-number") is None
|
||||
assert _parse_msg_limit("abc") is None
|
||||
|
||||
|
||||
def test_parse_msg_limit_passes_legit_sizes_unchanged():
|
||||
"""Legitimate pagination sizes (5/30/60/100) are returned verbatim — the
|
||||
ceiling only caps oversized requests."""
|
||||
for legit in (1, 5, 30, 60, 100):
|
||||
if legit > _MAX_MSG_LIMIT:
|
||||
continue
|
||||
assert _parse_msg_limit(str(legit)) == legit, f"legit {legit} altered"
|
||||
|
||||
|
||||
def test_parse_msg_limit_clamps_oversized_to_ceiling():
|
||||
"""An over-ceiling request (the outline-jump 9999, or a hostile 1000000) is
|
||||
clamped down to _MAX_MSG_LIMIT — exactly the regression this PR fixes."""
|
||||
assert _parse_msg_limit("9999") == _MAX_MSG_LIMIT
|
||||
assert _parse_msg_limit("1000000") == _MAX_MSG_LIMIT
|
||||
assert _parse_msg_limit(str(_MAX_MSG_LIMIT + 1)) == _MAX_MSG_LIMIT
|
||||
|
||||
|
||||
def test_parse_msg_limit_ceiling_boundary_itself_passes():
|
||||
"""A request exactly at the ceiling is allowed (clamp is inclusive)."""
|
||||
assert _parse_msg_limit(str(_MAX_MSG_LIMIT)) == _MAX_MSG_LIMIT
|
||||
|
||||
|
||||
def test_parse_msg_limit_zero_and_negative_clamp_to_one():
|
||||
"""Non-positive values clamp to the minimum (1), not None — a caller asking
|
||||
for msg_limit=0 gets a 1-row window, not the full transcript."""
|
||||
assert _parse_msg_limit("0") == 1
|
||||
assert _parse_msg_limit("-5") == 1
|
||||
@@ -427,7 +427,12 @@ def test_same_session_force_reload_keeps_loaded_transcript_width_hint():
|
||||
assert "const appendedMessageCount=Math.max(0,currentMessageCount-previousMessageCount);" in SESSIONS_JS
|
||||
assert "return Math.max(_INITIAL_MSG_LIMIT,loadedRenderableCount,loadedMessageCount+appendedMessageCount);" in SESSIONS_JS
|
||||
assert "const reloadLimit = _messageReloadLimitForSession(sid);" in SESSIONS_JS
|
||||
assert "const reloadLimitParam = reloadLimit ? `&msg_limit=${reloadLimit}` : '';" in SESSIONS_JS
|
||||
# The width hint is applied only when it stays within the server msg_limit
|
||||
# ceiling; an over-ceiling hint would be clamped by the backend and could
|
||||
# silently shrink an already-loaded transcript, so it falls back to the bare
|
||||
# full-transcript path (#6152/#6154 ceiling; Codex gate silent row-loss fix).
|
||||
assert "const boundedReloadLimit = (reloadLimit && reloadLimit <= _MSG_LIMIT_MAX) ? reloadLimit : null;" in SESSIONS_JS
|
||||
assert "const reloadLimitParam = boundedReloadLimit ? `&msg_limit=${boundedReloadLimit}` : '';" in SESSIONS_JS
|
||||
assert "if (_ownsLoad()) _clearSameSessionForceReloadHint(sid);" in SESSIONS_JS
|
||||
|
||||
load_start = SESSIONS_JS.index("async function loadSession(sid)")
|
||||
|
||||
Reference in new issue
Block a user