Update the older #1432 static regression to assert the extracted reusable-empty helper owns the message-count and in-flight checks, while both New Chat entry points call the helper. This keeps the test aligned with the shared guard introduced in PR #5206.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Adds the optional regression test the gate recommended: pins that
_redact_fn_cached stays byte-identical to _redact_fn_uncached for sensitive
input, is idempotent, and that >16KB inputs which bypass the lru_cache are
still redacted identically (no cache-skip secret leak). Production code
unchanged.
The combined credential redactor (~15 regex passes per string) runs on the
full message history of every /api/session response. Under concurrent
dashboard polling of unchanged sessions this became the dominant CPU cost
and, behind the GIL in the single-process server, serialized requests to
10-44s each — surfacing as repeated 'connection lost' in the browser as
the proxy/browser timed out and the frontend retried, piling on threads.
The redactor is pure and deterministic (force=True, fixed masking), so
identical strings always map to identical output and are safe to memoize
without invalidation. Wrap it in an lru_cache(maxsize=4096), gated to
strings <=16KB so a few giant tool-output dumps can't evict the many small
recurring strings or balloon RSS. Repeated polls become O(1) cache hits
(measured 3.7ms -> 0.002ms per 3KB marker string).
_redact_fn_cached is preserved as the call-site name so the existing
monkeypatch-based redaction tests bypass the cache unchanged (69/69 pass).
Adds the test the project's contribution rules require for non-trivial logic
(AGENTS.md). Covers: posix passthrough (1 call, no sleep), Windows
retry-then-succeed, reraise after exhausting 5 attempts (no silent success on a
save path), no-retry on non-PermissionError, and the 50/100/200/400ms backoff
sequence. Production _safe_replace unchanged.
Windows locks session JSON files briefly during antivirus scans or
browser polling, causing os.replace() to raise WinError 5
(ERROR_ACCESS_DENIED). Add _safe_replace() helper that retries with
exponential backoff (50ms to 800ms, max 5 attempts) on PermissionError.
No-op on non-Windows platforms.
Replaces all 5 os.replace() call sites in models.py:
- Session.save() (main file + backup)
- _write_session_index() (full rebuild + fast path)
- prune_session_from_index()
The source-string assertion windows were too short: the Ctrl+K branch window
(900) cut off before newSession()/closeMobileSidebar() (@992/@1155) and the
Ctrl+B forward window (+120) cut off isContentEditable (@119+). Widen to
1500 / +300 so the guards are actually verified (non-vacuous). Codex-recommended
headroom. Production guard unchanged.
A turn that streamed only prose (a long plain-text answer, or a degeneration
burst that floods the body with repeated tokens) still projected an anchor
activity scene whose activity_rows were all prose/terminal — zero tool/thinking
rows. The settle path promoted it to a collapsed worklog anyway (the gate only
checked activity_rows.length), hiding the whole answer and shrinking the
transcript by the full streamed height at STREAM_DONE, so the browser clamped a
bottom-pinned viewport back to the top (the 'jump back' report).
Add a worklog-worthiness predicate at both gates: the generation gate
(_anchorSceneHasWorklogWorthyRows in messages.js, decides whether to attach a
scene at all) and the render gate (_anchorSceneSceneHasWorklogWorthyRows in
ui.js, defense-in-depth for already-persisted all-prose scenes). A scene is
worklog-worthy only if it has >=1 tool/thinking row or a compression lifecycle
card; pure prose is not, so the turn renders as normal visible prose and the
viewport stays pinned at the bottom.
Behavioral + structural tests in tests/test_pure_prose_turn_not_worklog.py.
Address maintainer must-fix: _shouldKeepSettledWorklogOpenForPinnedFollow()
keyed only on global pin state, so every historical settled worklog re-expanded
on any pinned re-render — defeating the compact default. Gate the keep-open
exception on a one-shot token = the just-settled stream id (armed at STREAM_DONE
renderMessages, disarmed right after), threaded as _shouldKeep...(streamId).
Unpinned path unchanged. Replace source-string test with a behavioral Node test
that drives a second pinned turn and asserts it collapses.
Follow-up to maintainer review on #5058: make the pinned-follower helper comment
explicit that avoiding the visible STREAM_DONE shrink jump intentionally takes
precedence over a transient user-collapsed live worklog while the reader remains
pinned. Add a source lock so that intent stays documented.
Focused scroll locks remain green (22 passed), node --check clean.
A new post-#4970 scroll-jump class surfaced after Release YH: when a streamed
assistant turn with a large live worklog/tool trace settles, the settled compact
worklog collapses hundreds of pixels at STREAM_DONE. The reader is still pinned
and scroll state is correct, but the net scrollHeight shrink makes the browser
clamp scrollTop down by the same amount, which is visible as a large backward
jump.
Fix by keeping the just-settled activity worklog open for pinned followers
(_scrollPinned && !_messageUserUnpinned), so the live->settled DOM swap is
height-stable. Unpinned readers still get compact settled worklogs and keep their
viewport normally. Also make _anchorSceneWorklogGroup respect opts.collapsed;
previously it hard-coded collapsed: !live so the caller could not request an open
settled group.
Regression locks assert the pinned-follow helper, that settled rendering passes
collapsed:!keepSettledWorklogOpen, and that _anchorSceneWorklogGroup honors an
explicit opts.collapsed. Verified locally with a Playwright repro: before the
fix a large tool-worklog answer produced scrollHeight -367/-422px and scrollTop
-367/-422px at STREAM_DONE; after the fix nShrinks=0 and nBack=0. Local focused
scroll locks pass: 22 passed.
After a hidden interval that added persisted messages (post-turn bg-review
writes, sibling-tab writes, the just-finished main turn flushing), switching
the tab back caused refreshActiveSessionIfExternallyUpdated('visible'|'focus')
to hit remoteCount !== localCount and call loadSession(sid, {force:true}),
which synchronously did S.messages = [] before awaiting the metadata +
messages fetches. The visible result was the entire conversation transcript
blanking for the round-trip and reappearing — '对话突然消失，重刷才回来'.
sessions.js itself warns about this 'disappear and reappear' tradeoff at the
top of the remoteCount/remoteLast guard, but only short-circuits the
metadata-only branch (#5061). #5122's 4-probe staged reconnect covers a
different path (SSE error with ready_state=2 on a visible tab) and does not
apply when the SSE error arrives while visibility_state='hidden' — that
bottoms out through _deferStreamErrorIfPageHidden before the reconnect block
ever runs.
Fix:
- refreshActiveSessionIfExternallyUpdated maps the visibility/focus recovery
reasons ({visible, focus}) to a new keepStaleUntilLoaded option, forwarded
to loadSession. The post-stream idle reconcile and the poll/external paths
keep their existing behaviour.
- loadSession ANDs opts.keepStaleUntilLoaded with sameSessionForceReload (so
cross-session switches still clear synchronously — leaving a prior
session's transcript on screen during a navigation is the original bug
the clear was written for) and, on that path, skips the synchronous
S.messages/S.toolCalls/_messagesTruncated/_oldestIdx clear. The new
transcript is SWAPPED into S.messages by _ensureMessagesLoaded(sid,
{force:true}), so the user sees old DOM directly replaced by new DOM in a
single render frame.
- _ensureMessagesLoaded grows an opts.force escape hatch so its 'messages
already populated' early-return cannot skip the swap when stale messages
are still in place.
Verified live on a source 8701 build:
- before: minHtmlObserved 65874, minKidsObserved 11 — 372 samples at 16ms,
DOM html length NEVER dipped below the pre-call value during the reload.
- loadSession was called with force:true keepStale:true reason:'visible',
outcome 'reloaded', sMsgs 21 → 1037, after kids 11 → 134 (single-frame
swap).
Tests:
- tests/test_issue5177_hidden_tab_blank_gap.py (new, 7 cases): source-locks
the keep-stale guard shape, the recovery-reason map, the
_ensureMessagesLoaded opts.force escape hatch, and that the synchronous
clear is wrapped in if(!_keepStaleUntilLoaded).
- Updated the two existing source-lock tests whose anchor strings included
the loadSession call signature
(tests/test_webui_external_refresh_frontend.py +
tests/test_tars_scroll_reset_regressions.py) to match the new
loadSession(... keepStaleUntilLoaded:_keepStaleUntilLoaded) form. No
behavioural assertions changed.
Local: node --check static/sessions.js OK. 122 passed across
test_issue5177_hidden_tab_blank_gap.py + the full adjacent set
(external-refresh / journal-frontend / scroll-reset / reconnect-chronology /
inflight-stream-reuse / sse-error-multi-probe / issue4295 / issue4856).
A turn started SERVER-SIDE (self-wake, cron, restart hook) fans a
`server_turn_started` frame onto the per-session live-view SSE channel so an
open tab renders it live. But while a tab is HIDDEN the WebUI deliberately does
NOT hold that persistent SSE open (connection-pool budget — #3992 / #4151), so
`get_session_channel()` returns None at fan-out time and the turn is dropped for
that tab. The user only saw the turn after a manual interaction (refresh / send)
re-subscribed the channel and replayed it.
Bridge the gap with a lightweight poll of `/api/session/status` (one short GET
per ~6s tick — NOT a held connection, so the connection-pool budget the hidden
guard protects is preserved) that attaches the existing live renderer when it
sees a *live* active stream.
Backend (`api/session_ops.py`):
- `session_status()` now exposes `active_stream_id`, derived through a new
`_live_active_stream_id()` helper that only returns the id when the stream is
genuinely live (present in STREAMS / ACTIVE_RUNS). A stale id left by a
crashed/restarted run surfaces as None, so the poller never attaches a
renderer to a dead stream. Additive field — existing consumers ignore it.
Frontend (`static/messages.js`):
- `_startHiddenActiveStreamPoll` / `_stopHiddenActiveStreamPoll` /
`_attachServerInitiatedStream` implement the poll lifecycle. The poll fires
one immediate tick on hide (so an in-flight turn is caught without waiting a
full interval), attaches mid-flight turns via the reconnecting/replay path,
and stops on re-show (the real SSE takes over), session switch, or once a
stream is rendering.
- Started on BOTH hidden paths: the `visibilitychange` hook's hidden branch (a
visible tab going to background) and `startSessionStream`'s hidden
early-return skip (a session opened while already hidden). The visible path
and `stopSessionStream` clear it.
Tests:
- `tests/test_hidden_tab_server_initiated_turn.py` — backend live-validation
(stale id → None; STREAMS/ACTIVE_RUNS id returned) and frontend lifecycle /
both-hidden-paths / replay-attach source locks.
- Widened the brittle fixed-width source-window slices in
`tests/test_issue3996_sse_visibility.py` (1600/1700 → 2400) so the existing
`visibilitychange` / hidden-skip assertions still find their markers after the
poll-start lines were inserted into `startSessionStream`.
test_skills_stats_cache asserted that adding a skill bumps the skills-dir
mtime so the cheap per-call probe recomputes counts on the next call. Two
_write_skill() calls inside the same coarse filesystem mtime tick could leave
the skills-dir mtime byte-identical to the cached probe value, so the probe
saw no change and served the stale count of 1 (assert enabled==2 failed). This
is an order-dependent flake: passes in isolation, fails intermittently under
full-suite ordering (observed across multiple unrelated PRs).
Force the skills-dir mtime strictly forward after the second write so the
probe is guaranteed to detect a genuine mtime change. The assertion stays
honest (it still proves recompute-on-mtime-change); only the race is removed.
Two cross-file isolation leaks that a finer shard partition exposes are fixed at
root so every shard passes regardless of execution order:
- test_auth_session_persistence.py: bind api.auth STATE_DIR/_SESSIONS_FILE/
_LOGIN_ATTEMPTS_FILE in setUp + restore in tearDown (was relying on a sibling
reload test to rebind them).
- test_request_diagnostics_cache.py: pin the session-list cache source stamp via
an autouse fixture (matches test_session_sidebar_cache.py) so a leaked state.db
WAL sidecar can't cause a spurious cache-miss.
Verified all shards green under 3-, 4-, and 5-way partitions. CI infra + test
harness only; no application behavior change.
