Greptile review follow-ups (no behavior change):
- Remove the captured-but-never-read scrollTopAtCapture field from
_captureMessageViewportAnchor; only scrollHeightAtCapture is consulted.
- Rename _grewAbove -> _grewSinceCapture in the realign guard (the check is
overall scrollHeight growth since capture, not specifically above-viewport),
matching the fallback guard's _grewSinceSnap.
- Add a mutation-checked fallback active-intent test: content grew but the reader
has recent real input intent, so the absolute snapshot.top restore is kept.
A residual mobile scroll jump-back remained after #5638: while the reader is up in
history during a live stream, the anchor captured for a same-frame restore goes stale
as the streaming chunk grows content ABOVE the viewport. The anchor's captured
topOffset (and the absolute snapshot.top) no longer map to the same content, so
realigning to them yanks a still reader backward by a few hundred px per tick.
The existing snapshot.userUnpinned===true fallback skip does not cover it: the
scrollHeight-collapse scroll event re-pins the state machine (flips userUnpinned back to
false) mid-stream, so both the semantic realign (_restoreMessageViewportAnchor) and the
absolute snapshot.top fallback still fire.
Fix: two guards, both keyed on content-growth-since-capture + absence of recent real
input intent, NOT on a scrollTop diff — on an overflow-anchor:auto container the browser
itself writes scrollTop to compensate above-viewport growth, so a still reader's scrollTop
is not stationary; _recentMessage*ScrollIntent instead reflects genuine touch/wheel/key
input, which the browser's anchor layer never sets.
- _captureMessageViewportAnchor records scrollHeightAtCapture (+scrollTopAtCapture).
- _restoreMessageViewportAnchor refuses the realign when content grew since capture, there
is no recent intent, and the delta would move scrollTop >8px.
- the absolute snapshot.top fallback in _restoreMessageScrollSnapshotSameFrame mirrors the
same guard.
An actively scrolling reader (recent intent) keeps the legitimate restore; a fresh anchor
(no growth) and legacy snapshots without the captured geometry are unaffected. Adds
tests/test_issue5637_stale_anchor_guard.py (7 node-harness cases; two are mutation-checked
to fail if either guard is removed).
Two linked causes of the mobile scroll jump-back, both confirmed by the
maintainer against origin/master in #5637.
Primary (CSS): the @media (pointer: coarse) block applied a flat
`content-visibility: auto; contain-intrinsic-size: auto 1px` to every .msg-row.
Off-screen assistant rows can be multi-thousand px (tool-result / long answers),
so a 1px reservation collapses their off-screen height; scrollHeight lurches down
by tens of thousands of px on a long transcript, the browser force-clamps
scrollTop, and the viewport is thrown to the top. A flat intrinsic-size cannot
both reserve tall rows and keep scrollHeight stable for iOS flick momentum (why
1px was chosen), so content-visibility is scoped to the short, size-predictable
user rows; assistant rows keep real rendered height. Mirrors the existing
#liveAssistantTurn opt-out pattern.
Secondary (JS): _restoreMessageScrollSnapshotSameFrame fell back to an absolute
snapshot.top when the semantic anchor restore failed. During streaming the live
activity-scene refresh fires this every tick; for a reader scrolled up into
history (unpinned), snapping to a stale absolute top nudges the viewport backward
by an amount that grows with scrollHeight. Now: hold position (no scrollTop write)
for the unpinned/anchor-failed case and let the browser's own scroll anchoring
keep the reader put. Pinned/near-bottom readers keep the tail-relative restore.
Adds tests/test_issue5637_mobile_scroll_clamp_jumpback.py (mutation-checked:
reverting either fix fails the matching assertion).
Address greptile P2 on the vscroll recycled-anchor test harness: the
brace-counting function extractor desynced on a bare { or } inside a string,
template, regex, or comment within the extracted function body, silently
truncating the extract. Skip those spans so brace matching runs on real code
structure only, and add a regression test with a function whose string/regex/
comment literals contain unbalanced-looking braces.
Verified: the naive extractor fails all four tests (the compensation function
now contains querySelector attribute-selector strings that desync it); the
hardened extractor passes.
_compensateScrollForMeasurementDelta re-found its viewport anchor only by
rawIdx and bailed with a bare 'if(!row) return' when that row was recycled
out of the render window. On long sessions containing multi-thousand-px
turns, a large scroll delta recycles the anchor row, so the estimated->measured
topPad swap (a scrollHeight change of tens of thousands of px) hit scrollTop
uncompensated and threw the viewport to the top -- a mobile scroll jump-back
that did not reproduce on desktop (where the anchor row stays rendered).
Fall back when the rawIdx row is gone: (1) re-find the same row by its stable
data-session-msg-idx and compensate by its measured offset delta; (2) if that
row is also unrendered, compensate by the top-spacer (topPad) height delta
captured before the re-render. _captureMessageViewportAnchor now records
topPadBefore so the fallback has a reference.
Adds tests/test_issue4346_vscroll_recycled_anchor_jumpback.py (Node-harness
behavioral tests). All fail on the pre-fix bare-return and pass on the fix;
existing vscroll suites stay green.
Relates to #4346.
The known-dirs-probe test built its expected path from os.environ['ProgramFiles']
+ os.path.join, but the production code uses os.path.expandvars('%ProgramFiles%..').
On the Linux CI runners expandvars does not expand %VAR% syntax and os.path.join
uses '/', so the expected path diverged from what the code produced and the test
failed (assert None == 'C:\Program Files/Git/cmd/git.exe').
Mock the OS-specific surfaces (winreg, os.path.exists/expandvars/join) so the
Windows code path is exercised deterministically on any host. Verified the two
previously-failing assertions now pass under posix os.path semantics, and a
mutation check (reverting the fix) still makes the three behavior tests fail.
Parallels tests/test_issue5175_macos_launchd_git.py for the Windows case:
verifies _resolve_git_executable() resolves git via the Git-for-Windows
registry InstallPath and known install dirs when it is off PATH, that
_detect_webui_version() recovers a real stamp instead of 'unknown', and
falsifiable guards that the registry path is Windows-only and returns None
when git is genuinely absent.
When the WebUI server is launched from a venv python whose PATH does not
include git (e.g. hermes-agent venv on Windows), shutil.which('git')
returns None and WEBUI_VERSION falls back to 'unknown'. That freezes the
?v=<stamp> static-asset cache key, so browsers keep serving stale cached
ui.js/messages.js even after the server is restarted with fixed code —
frontend fixes silently never reach clients (recurring scroll jump-back).
Add a Windows fallback to _resolve_git_executable(): read the
Git-for-Windows registry InstallPath (HKLM + WOW6432Node + HKCU), then
probe common install dirs. Mirrors the existing darwin /usr/bin/git
fallback. No behavior change on posix or when git is already on PATH.
The Node-executed gate in test_anchor_fallback_ownership.py
(test_render_messages_keeps_anchor_owned_turn_out_of_legacy_activity_rebuilds)
eval()s the real renderMessages(). Commit 7536f6f1 made renderMessages schedule
its post-render pass via _postProcessWithAnchorSuppression(), but the harness
only stubbed postProcessRenderedMessages() — so the eval threw
'ReferenceError: _postProcessWithAnchorSuppression is not defined' and the test
failed on Linux CI (shard 2). It passed locally only because Windows hit the
unrelated WinError 206 command-line-too-long first, masking the real error.
Add a no-op stub for _postProcessWithAnchorSuppression alongside the existing
postProcessRenderedMessages stub. Verified by dumping the generated node script
to a temp .js file and running 'node file.js' (bypassing the Windows -e length
limit): the eval no longer throws and the test's assertions pass.
Commit 7536f6f1 routed the deferred post-render dispatches through
_postProcessWithAnchorSuppression() (holds overflow-anchor suppression across
the async media/layout settle frame, then calls postProcessRenderedMessages).
Six pre-existing tests string-matched the old
'requestAnimationFrame(()=>postProcessRenderedMessages(inner))' literal and
failed on the rename — behavior is preserved (the wrapper still invokes
postProcessRenderedMessages), so this is a test-fix not a code-fix.
Per the gate-cert recommendation, the tests now assert the BEHAVIOR chain
(post-render is scheduled via _postProcessWithAnchorSuppression, and that
wrapper calls postProcessRenderedMessages) rather than the exact rAF literal, so
a future wrapper rename can't re-orphan them.
Files: test_csv_table_rendering, test_excalidraw_inline_embed,
test_issue483_inline_diff_viewer, test_issue484_json_tree_viewer, test_issue347,
test_pdf_html_preview. Verified: the 6 updated assertions pass locally (the only
local failures are the pre-existing Windows-only WinError 206 command-line-too-long
in Node-harness tests, unrelated, green on Linux CI).
The sync-frame guards (_fixMobileScrollJank / _suppressBrowserOverflowAnchor)
only cover the render frame itself. postProcessRenderedMessages() — syntax
highlight, inline diff/csv/pdf/html/excalidraw, katex/mermaid — is scheduled a
FRAME LATER via requestAnimationFrame(), after those guards have released. Each
of those can change the height of rows ABOVE the viewport; on mobile
(overflow-anchor:auto) the browser's native anchor engine then compensates
scrollTop a SECOND time in that unguarded frame, yanking an unpinned reader to
another turn (the residual mobile 往回大跳).
Wrap all three deferred post-process dispatches (fast-path cache branch, main
render tail, live-tool remount) in _postProcessWithAnchorSuppression(), which
routes through the shared _suppressBrowserOverflowAnchor() and holds suppression
one extra frame so late media/layout reflow is covered too. Desktop rests at
overflow-anchor:none so the wrapper is a verified no-op there.
Reproduced on an isolated debug instance with a cloned 1179-message session:
above-viewport +350px during the async settle window jumped scrollTop +350 on
mobile (auto) and 0 with the wrapper; desktop (none) 0 both ways. static/ui.js
only.
Root cause (mobile-only, never reproduces on desktop): .messages CSS resting
overflow-anchor is 'auto' on touch devices but 'none' on hover+fine-pointer
desktops (style.css media query). When _restoreMessageViewportAnchor writes
scrollTop to realign the reader's anchor row AND content height above the
viewport changed in the same frame, a mobile browser's native scroll-anchoring
ALSO shifts scrollTop -- the two compensations stack and yank the reader to an
unrelated earlier turn. Desktop never has the browser layer, which is why this
reproduced only on phones.
Fix: _suppressBrowserOverflowAnchor() sets overflow-anchor:none for the JS
scrollTop write, releases (restores prior value) next frame. Engages ONLY when
computed value is 'auto' (mobile) -- pure no-op on desktop (already none).
Verified on isolated debug instance (mobile-viewport Playwright):
- mobile auto: 800px above-viewport growth compensation 800px -> 0 (browser layer suppressed)
- desktop none: helper returns null, inline value untouched (byte-identical behavior)
- streaming: real turn, mid-read follow, 0 jumps, content held
- scroll-regression suite green
A visible tab can permanently miss a server-initiated turn (self-wake /
cron / restart hook) when its per-session SSE is momentarily down at the
emit instant. server_turn_started is a fire-and-forget broadcast with no
replay buffer; if the turn also finishes before the tab reconnects, the
on-subscribe self-heal finds no live run in ACTIVE_RUNS and replays
nothing, so the transcript stays stale until a hard refresh.
Extend the self-heal: the (re)subscribing tab reports its last-known
message_count (?known_count); when there is no live run to replay, the
handler compares the persisted count and, if the server is ahead, emits a
lightweight session-updated frame. The frontend syncs incrementally via
the #5189 keepStaleUntilLoaded swap-in-place loadSession path (no
clear+refetch, so the #5177/#5189 blank-gap jump is not reintroduced).
Idle-only and pane-scoped; emits only when the server is strictly ahead.
The RED gate-cert (Codex-verified) found the keep-open was NOT the one-frame
transient the PR claimed: _disarmKeepSettledWorklogOpen() only cleared the token
with no follow-up render, so the just-settled worklog stayed force-open and could
be cached into _sessionHtmlCache and persist across session restores.
Two coordinated fixes:
1. Don't cache the forced-open DOM. Add _isKeepSettledWorklogOpenArmed() and gate
the _sessionHtmlCache.set() population on a typeof-safe !_keepOpenArmed local, so
the armed settle render is never cached. The typeof guard also keeps standalone
renderMessages() node harnesses (which don't define the helper) working.
2. Collapse pass after disarm for BOTH pin states. The STREAM_DONE handler disarms,
then runs _renderMessagesWithScrollSnapshot() UNCONDITIONALLY (a scroll-preserving
re-render that collapses the worklog back to its copied live/user disclosure
state), and only THEN scrollToBottom() for followers to re-settle at the tail.
The second RED gate-cert caught that gating the collapse pass to the non-following
path left a pinned follower forced-open, because scrollToBottom() only settles
scroll position and does NOT re-render. Running the collapse render before the
follower re-settle fixes both pin states. The same-frame JS restore absorbs the
collapse height change instead of leaving it to native scroll-anchoring (which
mobile suppresses mid-render), so there is no jump, and this unarmed render is
the one that populates the cache.
Net: keep-open is genuinely one settle render for pinned AND unpinned readers —
neither persists nor caches — and a user-collapsed worklog is preserved. Verified
e2e on an isolated debug instance for both pin states (worklog open-while-armed ->
collapsed-after-pass; pinned follower stays pinned at tail). Regression tests lock
the cache-skip and the collapse-before-scrollToBottom ordering; the scrollToBottom
adjacency keeps the #1360 done-follow test green.
Maintainer review on #5260 flagged that dropping the pin gate means an unpinned
reader who manually collapsed the live worklog now gets a one-frame flash-open at
the settle render before it re-collapses. Document the mechanism in-tree at the
call site (disclosure state preserved + keep-open disarms after the render, so
the next re-render honors the collapsed state) so a future reader doesn't mistake
the single-frame override for a lost preference. Doc-only; no behavior change.
The STREAM_DONE keep-open exception (#4970 round 6/7) was scoped to PINNED
followers only, on the assumption that unpinned readers 'preserve their viewport
normally'. That holds on desktop (overflow-anchor:none + JS snapshot restore) but
NOT on mobile.
Repro (isolated debug instance + CDP touch emulation, faithful to a real phone):
a reader who scrolls UP to read inside the just-settled turn is UNPINNED. At
STREAM_DONE the live worklog/thinking cards collapse into a compact summary,
shrinking the transcript by hundreds of px ABOVE their viewport. On mobile the
CSS resting value is overflow-anchor:auto, but _fixMobileScrollJank() flips an
inline overflow-anchor:none over the settle render — so native scroll-anchoring
is suppressed during exactly the frame the unpinned reader needs it to absorb the
above-viewport shrink. The content then leaps to the top of the latest turn (the
'往回大跳' report). Single-variable measurement: collapse-with-auto shifts the
reader 0px; collapse-with-none shifts -248px.
Fix: keep the just-settled worklog open for BOTH pin states (still one-shot
scoped to the turn that just settled, so historical worklogs still collapse
compact). This removes the shrink entirely, fixing the jump for every
device/anchor-mode instead of fighting the anchor engine. The #4856 guard and the
mobile overflow-anchor:auto CSS are untouched, so Boev's Android DOM-wipe fix and
akrhin's #MOBILESCROLL fix both stay intact.
- Rename _shouldKeepSettledWorklogOpenForPinnedFollow ->
_shouldKeepSettledWorklogOpenForStreamSettle (drops the _scrollPinned /
_messageUserUnpinned gate; keeps the one-shot stream-id token).
- Update the #4970 behavioral regression test to assert keep-open is TRUE for the
armed turn under BOTH pin states, FALSE for historical turns and after disarm.
#5172 (render server-initiated turns in a hidden tab, shipped v0.51.743 via
#5218) has a non-blocking multi-pane edge flagged in review: when a
server-initiated turn arrives for a session that is NOT the active pane, the
hidden-tab status poll called _attachServerInitiatedStream() and could not tell
whether the attach actually happened. _attachServerInitiatedStream returned
undefined in all paths, so the poll had no signal — a stop-on-call would cancel
polling while the turn was never rendered, leaving it invisible until the next
user interaction.
Fix: _attachServerInitiatedStream now returns a bool — true when this tab is
responsible for rendering the stream (attached, or already attached to the same
sid/streamId), false when the attach was not consummated (sid not the current
pane, invalid input, or a thrown error). _startHiddenActiveStreamPoll captures
the bool and only calls _stopHiddenActiveStreamPoll() when true; on false it
keeps polling and re-tries on a later tick (e.g. once the user switches the pane
back to sid, or the turn finishes and status returns active_stream_id=null).
Tests in tests/test_hidden_tab_server_initiated_turn.py: assert all return paths
are explicit bool (no bare return), the non-current-pane path returns false, and
the poll gates _stopHiddenActiveStreamPoll on the attach result.
Two related 'jump back' classes where a reader who is following the live stream
gets stranded mid-transcript, both verified live (OLD stranded the viewport
470-580px from bottom; FIX lands at 1px).
1. Content-grew-beneath-a-pinned-viewport (static/ui.js scroll listener).
While streaming on a tall transcript (especially on mobile, where chunks land
fast), new content increases scrollHeight under a stationary viewport. The
reader never scrolled (top did not move up, _messageUserUnpinned is false), but
bottomDistance crosses the nearBottom threshold, so the code fell through to
_scrollPinned=false, killing auto-follow mid-stream. The follow writer and the
scroll listener then fought frame-by-frame; the viewport stalled while content
kept growing and was progressively stranded. Fix: in the !_messageUserUnpinned
branch, when the viewport did NOT move up and auto-follow is on, keep the pin
and re-snap to the true bottom via _setMessageScrollToBottom() instead of
unpinning. The genuine scroll-away fallthrough is unchanged.
2. SSE-recovery follow-restore (static/messages.js). _handleStreamError (SSE
drop), the Task-cancelled apply + fallback paths, and the reconnect-stream-dead
cleanup all push/replace S.messages then renderMessages({preserveScroll:true}).
preserveScroll's restore path keys on the pre-render snapshot's bottom-distance,
which during a live stream can read large (content grew under a followed
viewport), so it yanked a following reader up to a stale historical position on
a process restart / SSE drop / cancel. Fix: capture follow-intent
(_isMessagePaneNearBottom) BEFORE mutating S.messages, and after the recovery
render, scrollToBottom() if the reader was following — so they see the
interruption/cancellation notice in place. Readers who scrolled up to read
history are left where they were.
Structural source-lock tests in tests/test_sse_recovery_scroll_stranding.py;
behavioral A/B verified via Playwright on a local source build.
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_error_reconnect_path_can_restore_from_journal anchored on the exact string
setComposerStatus('Reconnecting — but the first status was changed to a template
literal `Reconnecting… (1/${_retryDelays.length})` for the staged-probe counter,
so that single-quoted anchor no longer exists (ValueError: substring not found in
CI shard 2). Re-anchor on the block's stable entry point _reconnectAttempted=true;
(unique in source) and widen the window to 1100 chars; the st.active /
st.replay_available / Restoring stream / _runJournalReplayParams() assertions are
unchanged.
- messages.js: declare _retryDelays before the first setComposerStatus and show
'Reconnecting… (1/4)' on the initial probe so the counter progresses
(1/4)->(2/4)->(3/4)->(4/4) instead of jumping from a bare 'Reconnecting…' to
(2/4) (looked like it started at 2).
- test: replace the fixed block_start+1600 char window with a brace-matched
_reconnect_block() helper so the ordering / no-early-clear assertions cannot
silently under-assert if the reconnect block grows. Add a source-lock for the
(1/N) stage-counter start.
After the chat EventSource fires 'error', attachLiveStream made a single 1.5s
reconnect probe and, if /api/chat/stream/status did not yet report active or
replay_available, fell straight through to _handleStreamError(): clearing the
owner INFLIGHT state, nulling S.activeStreamId, pushing a 'Connection
interrupted' message and re-rendering. That wiped the live message DOM even
when the backend was still producing tokens or the run-journal replay file was
a beat away from becoming visible, so the settled response disappeared and then
reappeared from sidecar/replay (or only on refresh).
Replace the single probe with a short staged retry window
(_retryDelays=[1500,3000,5000,8000] ms) driven by a recursive _probeReconnect.
Each stage re-queries stream status and reconnects/replays when the backend is
reachable; the live DOM and S.activeStreamId/INFLIGHT state are kept intact
across the whole window, and _handleStreamError is only reached after every
stage has failed.
Adds tests/test_sse_error_multi_probe_reconnect.py source-locking the staged
shape and the ordering invariant (terminal error only after the retry window is
exhausted; no inflight/activeStreamId clear mid-window).
Maintainer/Codex found one narrow edge in the keyboard intent stamp: because the
listener runs capture-phase and accepts any focused descendant of #messages,
Space/Spacebar on an in-transcript control (tool-card toggles, copy buttons,
role buttons, links/tabs) stamped _lastMessageKeyScrollIntentMs before the
control handler could preventDefault/stopPropagation. That made a button
activation look like a scroll intent and could mask a legitimate post-render
artifact.
Fix by excluding Space/Spacebar when the event target or active element is an
interactive transcript control: button, a[href], select, summary, role=button,
role=tab, role=menuitem, or contenteditable. Keep PageUp/PageDown/arrows/Home/End
stamping unchanged, and keep the existing INPUT/TEXTAREA/contenteditable composer
exclusion.
Add a behavioral node-harness test: Space on a focused transcript button leaves
the key-scroll stamp at -Infinity (serialized null), while PageUp on the pane
still stamps 1234. test_4856 + test_4295 green (20 passed), node --check clean.
Keyboard scrolling of the message pane (PageUp/PageDown, Arrow keys, Space,
Home/End) fires a native scroll event with no wheel/touch/scrollbar/non-message
intent. Inside the 1400ms post-render artifact window the suppression branch
then returned before movedUp could unpin, so a keyboard scroll-up was swallowed
and live-follow snapped the reader back to the bottom.
- Add _lastMessageKeyScrollIntentMs + _recentMessageKeyScrollIntent(), stamped
by a capture-phase keydown listener on the scroll keys, gated to when the
message pane is the scroll target (focused/contains focus/hovered) and not an
editable field (composer/input/contenteditable).
- Gate the post-render suppression on !_recentMessageKeyScrollIntent() (typeof
guard keeps the #4295 node harness inert).
- Clear the stamp in both _resetScrollDirectionTracker() and
_resetStreamScrollFollow() (same stale-state hygiene as the wheel stamp).
Tests: keyboard-scroll-inside-window-still-unpins behavioral regression (harness
extended with injected _recentMessageKeyScrollIntent), plus a source lock for the
helper/keydown-stamp/suppression-gate/both-resets. test_4856 + test_4295 green
(19 passed), node --check clean.
Maintainer dual-gate found two stale-state leaks in the new
_lastMessageWheelIntentMs and one adjacent pre-existing gap; all three fixed:
MUST-FIX 1 — _resetScrollDirectionTracker() (session switch) did not clear
_lastMessageWheelIntentMs, so a gentle wheel in chat A left
_recentMessageWheelIntent() true into chat B's first post-render window,
under-suppressing the artifact and falsely unpinning. Now reset to -Infinity.
MUST-FIX 2 — _resetStreamScrollFollow() (fresh stream) had the same leak: a
gentle upward wheel within 1200ms of a new stream could silently disable live
follow. Now reset to -Infinity.
SHOULD-FIX 3 — the suppression branch ignored _scrollbarDragActive, so a manual
scrollbar-drag upward scroll inside the 1400ms window was swallowed. Gate the
branch on (typeof _scrollbarDragActive==='undefined' || !_scrollbarDragActive);
typeof guard keeps the #4295 node harness inert.
Tests: scrollbar-drag-inside-window-still-unpins behavioral regression (harness
extended with injected _scrollbarDragActive), plus source locks for both resets
and the scrollbar-drag gate. test_4856 + test_4295 green (17 passed),
node --check clean.
Maintainer dual-gate (Codex) found the suppression could swallow a genuine
low-delta trackpad wheel scroll-up for ~1.4s after a render:
_recordNonMessageScrollIntent() only recorded message-pane wheel intent at
deltaY<-30, so a gentle deltaY:-5 left both intent helpers false and the
post-render branch returned before movedUp set _messageUserUnpinned.
- Track recent low-delta upward message-pane wheel intent separately
(_lastMessageWheelIntentMs / _recentMessageWheelIntent), recorded for any
upward wheel (deltaY<0). The decisive deltaY<-30 sticky-unpin is unchanged.
- Require !_recentMessageWheelIntent() before suppressing the artifact, so a
real gentle scroll-up inside the window still unpins.
- Add behavioral node-harness regressions: gentle wheel inside the window
unpins; no-intent artifact inside the window stays suppressed; outside the
window unpins. Plus a source lock that low-delta intent is tracked.
The new movedUp suppression branch called _recentMessageRenderArtifactWindow()
and the intent helpers in an unguarded position. The #4295 unit harness injects
the scroll-listener body via new Function() without those helpers, so the branch
threw ReferenceError on a movedUp sample and crashed the node subprocess.
Reorder the && chain with typeof-function guards first; production evaluates the
real helpers, the harness short-circuits before any call. Behavior unchanged.
The default-only guard corrected context_length to the real per-model cap
(e.g. 1M for claude-opus-4.7-1m) but left threshold_tokens pointing at the
ContextCompressor's stale value (computed from the global 232K cap → 197.2k
@ 85%). UI then showed 'auto-compress at 197.2k / 1M' which is misleading.
Rescale threshold_tokens by the real/orig ratio so the displayed trigger
reflects the actual window (e.g. ~850k @ 1M).
NOTE: this only corrects the SSE display payload. The real auto-compress
trigger lives inside ContextCompressor in hermes-agent (agent_init.py:1446
constructs it with the global cap). A full fix requires a parallel change
upstream — tracked separately.
_live_usage_snapshot() runs on every metering tick (~10x/sec while streaming).
The #3256 default-only guard recomputed get_model_context_length() there on
every tick for non-default models, which does a config read + potential
metadata/network probe — freezing claude-opus-4.7-1m streams while the default
model (4.8) stayed fast (guard not triggered for it). Resolve the real cap at
most once per stream via _real_ctx_cache. Backend-only, no frontend changes.
Re-applies cb0065eb + b34311b3 (context_length default-only guard) which were
dropped by the upgrade reset to v0.51.185. Fixes 4.7-1m context window showing
as stale global cap (232K) instead of real 1M metadata. Backend-only: touches
api/routes.py + api/streaming.py, zero frontend/render changes.
_run_git used subprocess.run(text=True) without an explicit encoding, so on
Chinese Windows (and other non-UTF-8 codepages) git stdout was decoded with the
locale codepage. _dirty_suffix() runs `git diff --binary HEAD`, whose binary
bytes are not valid GBK, raising UnicodeDecodeError in the subprocess reader
thread. That left r.stdout = None, so `r.stdout.strip()` raised AttributeError
during module import of api.updates, crashing server.py before it could bind
its port.
Force UTF-8 decoding with errors=replace and guard against None defensively.
allenliang2022