* feat: expose msg_limit ceiling via /api/session metadata, frontend reads dynamically (#6177) Backend exposes _MAX_MSG_LIMIT as _msg_limit_max in every /api/session response. Frontend reads it dynamically, falling back to _MSG_LIMIT_MAX for older servers. This removes the hand-mirrored coupling between the two layers. Removed test_msg_limit_ceiling_drift.py since the mirror pattern that required the drift guard is replaced by dynamic metadata. This is the standalone metadata-decoupling piece from #6206, without the clamp/paging that already shipped in exp-v0.52.98 via #6152/#6154. * fix(session): declare _msgLimitMax at module scope + CHANGELOG + gate fixes (#6214 follow-up) The submitted PR used _msgLimitMax at two read sites (boundedReloadLimit in _ensureMessagesLoaded, useBeforePaging in _loadOlderMessages) but never declared it and read it before assignment -> undefined on cold load -> full-transcript fetch every load (regression) + implicit global. Declared `let _msgLimitMax = _MSG_LIMIT_MAX;` at module scope so the reload-width paths always read a defined value (the static fallback) until the server's _msg_limit_max lands. Also: defined the ceiling globals in the two inline node-harness tests that copy _ensureMessagesLoaded's body (test_cross_session_message_load_isolation, test_session_unread_dot_on_visit), updated the source-string assertion in test_webui_external_refresh_frontend, and added 3 decoupling tests (backend field present, module-scope declaration + fallback, both paths read the live ceiling) replacing the deleted drift-guard. Gate: Codex adversarial SAFE TO SHIP (executable probes: cold-load fallback, mixed-version omission, live update, cross-session isolation, over-ceiling bare refresh, msg_before row preservation). Full sharded suite green. Co-authored-by: webtecnica <webtecnica@users.noreply.github.com> --------- Co-authored-by: webtecnica <webtecnica@users.noreply.github.com> Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
This commit is contained in:
8 files changed
+58
-79
No files matched your search
@@ -9,6 +9,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **The frontend now reads the `msg_limit` ceiling from `/api/session` metadata instead of hand-mirroring the constant.** Following the ceiling clamp shipped in exp-v0.52.98, the backend advertises its `_MAX_MSG_LIMIT` as `_msg_limit_max` in every `/api/session` response, and the frontend reads it dynamically (`_msgLimitMax`, a module-scope value defaulting to the static fallback), so the two can no longer drift and the mirrored constant's drift-guard test is no longer needed. Older backends that omit the field fall back to the built-in default, so mixed-version deployments are unaffected. Thanks @webtecnica. (#6214, #6177)
|
||||
|
||||
- **`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)
|
||||
|
||||
@@ -12769,6 +12769,7 @@ def handle_get(handler, parsed) -> bool:
|
||||
_truncated = load_messages and msg_limit is not None and _messages_offset > 0
|
||||
raw["_messages_truncated"] = _truncated
|
||||
raw["_messages_offset"] = _messages_offset
|
||||
raw["_msg_limit_max"] = _MAX_MSG_LIMIT
|
||||
_t4 = _time.monotonic()
|
||||
if _diag: _diag.stage("t4_after_compact_and_merge")
|
||||
if effective_model:
|
||||
|
||||
+13
-7
@@ -2935,12 +2935,17 @@ const _INITIAL_MSG_LIMIT = 30;
|
||||
// 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.
|
||||
// (a fixed-size backward page keyed off _oldestIdx) instead. This const is the
|
||||
// static FALLBACK default only — the live ceiling is read from the /api/session
|
||||
// `_msg_limit_max` metadata into _msgLimitMax below (#6177), so the two can no
|
||||
// longer drift. Keep this fallback value roughly in sync with the backend
|
||||
// _MAX_MSG_LIMIT for the mixed-version case where the server omits the field.
|
||||
const _MSG_LIMIT_MAX = 500;
|
||||
// Live server-advertised msg_limit ceiling. Declared at module scope with the
|
||||
// static fallback so the reload-width paths (_ensureMessagesLoaded /
|
||||
// _loadOlderMessages) always read a defined value even before the first
|
||||
// /api/session response lands; refreshed from `_msg_limit_max` on each load.
|
||||
let _msgLimitMax = _MSG_LIMIT_MAX;
|
||||
let _sameSessionForceReloadHint = null;
|
||||
|
||||
function _currentLoadedRenderableMessageCount(){
|
||||
@@ -3052,7 +3057,7 @@ async function _ensureMessagesLoaded(sid, opts) {
|
||||
// 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 boundedReloadLimit = (reloadLimit && reloadLimit <= _msgLimitMax) ? 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
|
||||
@@ -3072,6 +3077,7 @@ async function _ensureMessagesLoaded(sid, opts) {
|
||||
if (!data || !data.session) return;
|
||||
_messagesTruncated = !!data.session._messages_truncated;
|
||||
_oldestIdx = data.session._messages_offset || 0;
|
||||
_msgLimitMax = data.session._msg_limit_max || _MSG_LIMIT_MAX;
|
||||
// #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"
|
||||
@@ -3570,7 +3576,7 @@ async function _loadOlderMessages() {
|
||||
// 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 useBeforePaging = requestedLimit >= _MSG_LIMIT_MAX;
|
||||
const useBeforePaging = requestedLimit >= _msgLimitMax;
|
||||
const data = useBeforePaging
|
||||
? await api(
|
||||
`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`,
|
||||
|
||||
@@ -215,6 +215,12 @@ function createEnvironment() {
|
||||
// the fetch URL drops msg_limit/expand_renderable and mismatches the
|
||||
// enqueued buildMessageUrl(), stalling the ordered api() harness.
|
||||
globalThis._MSG_LIMIT_MAX = 500;
|
||||
// #6177: _msgLimitMax is a module-scope `let` (live server-advertised ceiling,
|
||||
// defaulting to _MSG_LIMIT_MAX). It's read by _ensureMessagesLoaded's
|
||||
// boundedReloadLimit and _loadOlderMessages's useBeforePaging; the harness
|
||||
// injects only the extracted functions, not module-level lets, so define it
|
||||
// here or those reads resolve undefined -> wrong fetch URL -> ordered-api stall.
|
||||
globalThis._msgLimitMax = 500;
|
||||
globalThis._currentMessageRenderWindowSize = () => 1;
|
||||
globalThis._messageRenderableMessageCount = () => 2;
|
||||
|
||||
|
||||
@@ -1,71 +0,0 @@
|
||||
"""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}"
|
||||
@@ -64,3 +64,34 @@ def test_parse_msg_limit_zero_and_negative_clamp_to_one():
|
||||
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
|
||||
|
||||
|
||||
# ── #6177: metadata-decoupling — the frontend reads the ceiling from the
|
||||
# /api/session `_msg_limit_max` field instead of a hand-mirrored constant. ──
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
_ROUTES_SRC = (Path(__file__).resolve().parents[1] / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
_SESSIONS_JS = (Path(__file__).resolve().parents[1] / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_backend_exposes_msg_limit_max_in_session_response():
|
||||
"""The /api/session handler advertises the ceiling as `_msg_limit_max` so the
|
||||
frontend never has to hand-mirror _MAX_MSG_LIMIT (#6177 decoupling)."""
|
||||
assert 'raw["_msg_limit_max"] = _MAX_MSG_LIMIT' in _ROUTES_SRC
|
||||
|
||||
|
||||
def test_frontend_declares_live_ceiling_at_module_scope_with_fallback():
|
||||
"""`_msgLimitMax` MUST be declared at module scope (not an implicit global)
|
||||
with the static fallback, so the reload-width paths read a DEFINED value
|
||||
before the first /api/session response lands — otherwise a cold load reads
|
||||
`undefined`, drops msg_limit, and full-loads every session."""
|
||||
assert "let _msgLimitMax = _MSG_LIMIT_MAX;" in _SESSIONS_JS
|
||||
# refreshed from the response metadata, falling back when the server omits it
|
||||
assert "_msgLimitMax = data.session._msg_limit_max || _MSG_LIMIT_MAX;" in _SESSIONS_JS
|
||||
|
||||
|
||||
def test_frontend_reload_width_paths_read_the_live_ceiling():
|
||||
"""Both reload-width decisions read the live `_msgLimitMax`, not the mirror."""
|
||||
assert "reloadLimit <= _msgLimitMax" in _SESSIONS_JS # _ensureMessagesLoaded
|
||||
assert "requestedLimit >= _msgLimitMax" in _SESSIONS_JS # _loadOlderMessages
|
||||
@@ -343,6 +343,8 @@ let _sessionCompletionUnread = null;
|
||||
let _messagesTruncated = false;
|
||||
let _oldestIdx = 0;
|
||||
let _messageRenderWindowSize = 0;
|
||||
const _MSG_LIMIT_MAX = 500;
|
||||
let _msgLimitMax = _MSG_LIMIT_MAX;
|
||||
let _pendingCarryForwardSnapshot = null;
|
||||
let _loadingSessionId = 'open';
|
||||
let _loadSessionGeneration = 0;
|
||||
|
||||
@@ -431,7 +431,9 @@ def test_same_session_force_reload_keeps_loaded_transcript_width_hint():
|
||||
# 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
|
||||
# #6177: the ceiling is now read from /api/session metadata into _msgLimitMax
|
||||
# (module-scope let, default _MSG_LIMIT_MAX) instead of the mirrored const.
|
||||
assert "const boundedReloadLimit = (reloadLimit && reloadLimit <= _msgLimitMax) ? reloadLimit : null;" in SESSIONS_JS
|
||||
assert "const reloadLimitParam = boundedReloadLimit ? `&msg_limit=${boundedReloadLimit}` : '';" in SESSIONS_JS
|
||||
assert "if (_ownsLoad()) _clearSameSessionForceReloadHint(sid);" in SESSIONS_JS
|
||||
|
||||
|
||||
Reference in new issue
Block a user