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).
_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.
Steering (submitting mid-stream) dropped any pending composer attachments. Now the
pending files are uploaded before the /steer payload is sent, the uploaded file
paths are appended to the steer text so the active agent can inspect them with
tools, file chips stay staged if the steer fails (so nothing is lost), and
file-only busy submissions are handled.
Contributor stage; gate-pass. Co-authored-by: ruizanthony <ruizanthony@users.noreply.github.com>
A message whose text contained a lone UTF-16 surrogate (e.g. a truncated emoji
from a partial paste/stream) made encodeURIComponent throw URIError inside
_messageViewportAnchorKeyForMessage, crashing the viewport-anchor computation and
breaking scroll-anchor restore for the whole transcript. New _safeEncodeURIComponent
catches the URIError and rebuilds the string via a UTF-16 code-unit walk that keeps
valid high+low surrogate PAIRS intact (emoji survive) and drops only LONE
surrogates. Uses no regex lookbehind/lookahead so it parses on every browser
engine (older WebViews / Safari <16.4 lack lookbehind and would brick ui.js at
parse time).
Contributor stage; gate-pass. Co-authored-by: rumotoshino <rumotoshino@users.noreply.github.com>
The Mermaid toolbar's "fit" and "fullscreen" icons rendered byte-identical (the
fullscreen path was fit's corner-brackets drawn twice). Replaced the fullscreen
glyph with a distinct corner-frame + diagonal-expand-arrows icon so the two
actions are visually distinguishable at 14px. Still wired to openLightbox; no
dependency on the old path. (The other half of #5525 — mermaid height:0 on
mobile — does not reproduce on current master; verified live at 390x844.)
Self-built (nesquena-hermes); nesquena independent review APPROVED.
An expired-auth 401 on the login page fed the login redirect its own address:
each of the three redirect guards (_safeNextPath in login.js,
_safe_login_redirect_path in routes.py, the api() 401 redirect in workspace.js,
_safe_login_inner_next in auth.py) validated against open-redirect but none
rejected a `next` pointing back at the login page, so every auth bounce wrapped +
re-encoded the previous login URL one level deeper — growing the URL exponentially
until the browser's length limit broke the tab. All guards now collapse/reject a
`next` whose decoded leading PATH resolves to the login route (bounded
percent-decoding up to 8 levels, fail-closed at the cap), while preserving the
existing open-redirect protections AND legitimate non-login paths that merely
carry their own `next=` query key. Length cap as belt-and-suspenders.
Self-built (nesquena-hermes); nesquena independent review APPROVED. Gate (Codex)
found + fixed an over-broad nested-next reject (regressed next-carrying paths) and
a deep-encode decode off-by-one; both root-fixed, login guards verified clean.
A single scroll-up during streaming set _messageUserUnpinned=true and
scrollIfPinned() then permanently stopped auto-follow (only scrollToBottom()
cleared it) — a permanent auto-follow lockout even after the user returned to
the bottom. scrollIfPinned() now re-pins, but ONLY when the reader has genuinely
reached the true-bottom tail (<=80px) AND shows no active scroll intent
(wheel/key/touch/non-message), reusing the listener's _nearBottomCount debounce.
Proximity alone (the ~250px nearBottom band) must never re-pin — that is the
#4295 invariant. Restores the listener's <=80px true-bottom gate too.
Co-authored-by: luperrypf <luperrypf@users.noreply.github.com>
Fable senior review (COMMENT, no blockers) + DRY nit:
- Widen the ResizeObserver from #msg to the whole #composerWrap so growth from the
attachment tray / selection chips (not just textarea rows) also re-pins a bottom-
pinned reader. Fable flagged tray/chip growth as a likely REMAINING #5515 trigger
— same viewport-shrink physics, now covered by the same guarded re-pin at one seam.
#composerWrap contains #msg, so the textarea path is still covered.
- DRY: _repinMessagesAfterComposerResize() now reuses _messageBottomDistance()
instead of re-implementing the bottom-distance expression inline.
Re-verified live (composer grow -> bottomDist stays 0) + 524 scroll/composer
regression tests green + 6 issue tests green.
#5514 (deterministic): with the chat pinned to the bottom, growing the composer
past one row (multi-line typing, Shift+Enter, a multi-line / WisprFlow paste)
shrinks the flex:1 .messages viewport by the same delta. scrollTop is unchanged
but max-scrollTop rose by delta, so the reader is stranded delta-px above the
bottom — the transcript 'scrolls up one row per composer row.' autoResize()
resized the textarea and called updateSendBtn() but never re-pinned the transcript.
Reproduced live: composer 176px -> viewport shrank 745->613 -> transcript stranded
132px above bottom while _scrollPinned stayed true.
#5515 (intermittent 'random scroll up'): the same viewport-shrink from composer
growth via paths that don't route through input->autoResize (paste, draft restore,
programmatic value set, reflow) reads to the user as a random upward jump. Covered
by a ResizeObserver seam (below).
Fix: new _repinMessagesAfterComposerResize() (static/ui.js) re-pins to the bottom
ONLY when the reader is genuinely still pinned (honors _messageUserUnpinned /
_scrollPinned — never yanks a reader who scrolled away; verified live). Called from:
(a) autoResize() after the height write, and
(b) a ResizeObserver on the #msg textarea that re-pins on GROW only, catching
every composer height-change path (not just the typed-input seam).
Verified live on a 40-message seeded session: pinned reader stays at bottomDist=0
after a 4-line composer grow; an unpinned (scrolled-up) reader is NOT moved.
Tests: tests/test_issue5514_composer_grow_scroll_pin.py (3 static wiring + 3 node-vm
behavioral). 524 scroll/composer regression tests across 39 files still green.
NOTE on #5515: this resolves the composer-growth cause (which the reporter's
WisprFlow-paste symptom points at). If a purely non-composer intermittent jump
persists, the remaining suspect is touch-device overflow-anchor:auto on content-
above height change — that path has no repro yet and is tracked as needinfo on
#5515.
Codex gate findings (2 SILENT, both fixed):
- ui.js: incremental prose node now flows through the shared row-decoration
block (data-anchor-scene-row/-row-id/-row-role/-source-event-type) instead of
returning early, so live incremental rows keep the identity attrs the scene
reconciler matches on.
- messages.js: _cancelThrottledSnapshotTimer() + _clearAnchorProseIncrementalNode()
now run on apperror, cancel, _handleStreamError, and _restoreSettledSession
terminal paths (previously only fallback/done/stream_end) — closes the
snapshot-timer/anchor-cache/window-global teardown gap.
- test: extend test_messages_js_stream_perf_cleanup_lifecycle to pin teardown on
all four additional terminal paths.
CORE equivalence (Codex flagged smd != renderMd): reconciled — Opus's 1498-frame
harness against real smd.min.js proves incremental-smd == whole-smd byte-identical;
live-smd-vs-renderMd difference is the already-shipped fidelity model (main live
body streams smd, settles renderMd), and the incremental path is gated on !settled
so settled DOM still comes from renderMd. Not a regression.
In compact-worklog mode the anchor activity scene re-renders every frame, and
its live prose row was rebuilt with renderMd(fullText) each time — re-parsing
the entire growing answer per frame, which is O(n^2) over a long streamed
response (the incremental smd parser that the main body already uses was
bypassed and its segment hidden).
Keep a persistent per-segment smd parser + DOM node (keyed by the prose row's
local_id, using the same safe renderer as the live body) and feed it only the
delta each frame; hand that persistent node back to the scene builder instead
of re-parsing. Turns the per-frame prose cost from O(n) into O(delta), i.e. the
whole answer from O(n^2) to O(n).
Safe by construction: settled rows and any failure (no smd, no stable key,
parser error, text divergence) fall through to the existing renderMd path, so
worst case is identical to today. Cache is bounded (<=32 segments) and self-heals
on non-append text changes, mirroring _smdWrite.
Verified: node --check + ESLint runtime guard clean. Streaming render profiled
in-browser via a per-frame timing probe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex re-gate found 2 SILENT regressions the certifier missed:
- inline lost the 'Fit to screen' toolbar button (PR gated Fit to lightbox-only)
- the new readable-scale/envelope path leaked into lightbox mode (changed zoom-out floor + fit init for wide diagrams)
Fix: toolbar restored to master shape (Fit for all modes, Fullscreen inline-only);
_minScale() returns the flat _MERMAID_VIEWER_MIN_SCALE for lightbox; lightbox init
restored to master's _fitScale()/box*initialFit; the new readable-height sizing is
now scoped strictly to inline mode. Tests updated to the master-consistent contract.
Rebase PR #5162 (rename busy_input_mode -> default_message_mode; flip the
default from 'queue' to 'steer') onto current origin/master WITHOUT dropping
the shipped #5170 localStorage persistence mirror.
Rename the mirror machinery to the new setting name for consistency:
_BUSY_INPUT_MODES -> _DEFAULT_MESSAGE_MODES (values unchanged)
_normalizeBusyInputMode -> _normalizeDefaultMessageMode (fallback now 'steer')
_persistBusyInputMode -> _persistDefaultMessageMode
_readPersistedBusyInputMode -> _readPersistedDefaultMessageMode
window._busyInputMode -> window._defaultMessageMode (+ renamed exports)
localStorage: write the new 'hermes-default-message-mode' key; read it with a
fallback to the legacy 'hermes-busy-input-mode' key so an existing user's
persisted preference survives the rename.
Preserve #5170 behavior at every mirror site under the new names:
- boot success -> window._defaultMessageMode=_persistDefaultMessageMode(...)
- boot FAILURE -> window._defaultMessageMode=_readPersistedDefaultMessageMode()
(NOT a hardcoded 'steer' — a saved 'interrupt'/'queue' must still apply when
the server is unreachable; do not regress #5167/#5132)
- preferences autosave, settings-panel load, and _applySavedSettingsUi all
persist through _persistDefaultMessageMode(...)
Tests updated for the rename while keeping the persistence-behavior assertions
(test_1062, test_5145, test_5167); test_5167 gains explicit guards that the
load-failure path reads the persisted pref and never hardcodes a literal mode,
plus autosave/panel-load mirror-write coverage.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
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
/api/clarify/pending always returns HTTP 200 when present (returns
{"pending": null} for an unknown session — it never 404s). The front-end
clarify poller warned "Clarify endpoint unavailable. Please restart
server." on ANY caught error whose message merely contained "404" or
"not found", so an unrelated stale-session 404 ("Session not found", e.g.
an old-profile session polling briefly after a profile switch) or a
transient error produced a misleading missing-endpoint toast that pointed
operators at the wrong layer.
Clarify polling now branches on the structured HTTP status that api()
attaches to the thrown Error (err.status):
- 404 "Session not found" -> handled as a stale-session poll (stop + hide
card silently), no toast;
- restart-server warning fires only on a genuine route-not-found 404 whose
body is NOT session-scoped;
- poll failures are logged with path, status, polling session id, and
current session id for diagnosis.
Interrupt provenance: cancelStream()/cancelSessionStream() now log a
'[stream] cancel requested' line with the trigger reason
(composer-stop / slash-stop / slash-interrupt / busy-interrupt /
sidebar-stop). Passive UI lifecycle events (session switch, tab hide, page
unload) already tear down only the local SSE transport via
closeLiveStream() and never call /api/chat/cancel — only explicit
Stop/interrupt paths interrupt the backend agent/tool run. This is
confirmed by test_clarify_pending_never_404s locking the handler shape.
Supersedes #5343 (which handled only the profile-switch sub-case and kept
the broad message-scrape). Adds tests/test_issue5345_*.py (9 tests).
Co-authored-by: claw-io <claw-io@users.noreply.github.com>
Co-authored-by: ruizanthony <ruizanthony@users.noreply.github.com>
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.
Rod Boev