The composer terminal's output EventSource registered handlers for
output/terminal_closed/terminal_error but none for the transport-level 'error'
event, so a session expiry, killed gateway, or network drop left the terminal
frozen with no feedback and no recordClientSSEError telemetry.
Add an 'error' handler behind the existing identity guard: let the browser
auto-reconnect a CONNECTING source (one-time "reconnecting" note, no manual
loop/backoff), and on a permanently CLOSED source surface a "disconnected" line
and dispose it (null TERMINAL_UI.source) so a restart can reconnect. Notify once
per outage to avoid flooding the pane/telemetry on a flapping connection. Emits
recordClientSSEError('terminal', {ready_state, reason}) like the chat/session
consumers.
UI-evidence (screenshot of terminal on gateway-kill) still needed before merge.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex gate CORE: /api/updates/apply + /force ignored the request-body channel
and re-read the saved setting, so a channel switch whose debounced autosave
hadn't landed could apply the OLD channel. The banner now sends the channel the
CHECK reported for each target; the endpoints validate it against the enum and
thread it into apply_update/apply_force_update (None → saved-setting fallback,
preserving prior behavior). Agent stays channel-neutral server-side.
- test_api_timeout: assert the updates/check call keeps timeoutMs:60000 in its
new _checkBody form (functionally identical; still the 60s override)
- panels.js: reword two // comments to drop apostrophes — the smooth-text-fade
brace-matcher test treats ' in a comment as a string delimiter and lost
loadSettingsPanel's closing brace (node --check confirms the JS was always valid)
text-overflow:ellipsis is a no-op on the inline-flex badge; display:block in the
<=768px rule makes the ellipsis actually render for a long experimental describe.
MUST-FIX: /api/updates/check now accepts an explicit 'channel' in the POST body;
the Settings dropdown passes the just-picked value to checkUpdatesNow(channel) so
the immediate re-check can't race the debounced autosave PUT and answer for the
previous channel (verified live: body channel wins over stale saved setting).
SHOULD-FIX:
- copy: 'soaked' -> 'soak-tested'; append 'Applies to WebUI updates only.' to the
channel helper text (all 15 locales, native translations)
- mobile: .settings-version-badge gets max-width/overflow/ellipsis in the <=768px
query so the '· Experimental' suffix can't overflow a 320px viewport
(#5 badge-suffix i18n key deferred — non-blocking fast-follow per Fable, matches
the existing English-only version badge on master.)
- test_update_stash_recovery: add merge-base handler for the force-update
rewind guard probe (origin/master is a descendant of HEAD -> not a rewind)
- test_security_review_fixes: fake_check accepts channel kwarg
- i18n.js: native translations of the 4 update-channel keys for all 14
non-English locales (locale-coverage tests enforce key presence)
Add an update_channel setting (stable|experimental) that selects which git-tag
stream the self-updater tracks on the single linear master line:
stable -> 'v*' promoted, soaked releases (default; unchanged glob)
experimental -> 'exp-v*' every release batch (opt-in testers)
A channel is only a tag glob — no branches, no divergence — so every ff-only
guarantee (#2653/#2846/#3140) is preserved. Channel governs the WebUI repo only;
the Agent repo keeps its historical branch fall-through.
Correctness (advisor-reviewed, Codex + Fable):
- describe uses --match <glob> so a commit tagged both v* and exp-v* resolves
to the channel-correct tag
- stable never falls through to origin/master when HEAD contains the latest
stable tag (the firehose-suppression that makes channels work)
- apply_force_update refuses to reset --hard onto an ancestor ref (rewind guard)
- update cache + in-progress guard keyed by (channel, include_agent)
- channel display badge is a SEPARATE field; WEBUI_VERSION stays channel-neutral
(asset cache-busting / SW cache / stale-client skew all do exact-string equality)
Settings UX: channel dropdown with risk-forward copy + the ff-only asymmetry
helper ('switching back to Stable keeps your current version until Stable catches
up'), channel chip on the version badge, i18n keys, re-check on switch.
Tests: new tests/test_update_channels.py (12 real-git-fixture tests) + existing
update suite updated for the channel-aware signatures.
When transcript virtualization is disabled (the #4325 opt-out,
_virtualizeTranscript===false), renderMessages() renders every row with no
windowing and never runs the virtualized measure pass
(_updateMessageVirtualMeasurements early-returns when !virtualized). Under
@media (pointer: coarse), .msg-row[data-role="user"] carries
content-visibility: auto; contain-intrinsic-size: auto 96px. Every rebuild does
inner.innerHTML='' then recreates rows as fresh elements, so a fresh off-screen
tall user row (a long paste measuring thousands of px) reserves only the flat
estimate instead of its real height. scrollHeight shrinks by (realHeight -
estimate), the browser force-clamps scrollTop, and the viewport jumps backward
(a browser clamp, JS=none, so scrollTop-write compensation cannot catch it).
#5638 fixed this for the virtualized wipe-and-rebuild path but left the
non-virtualized full-rebuild path uncovered.
Fix, three coordinated parts:
- _estimateUserRowIntrinsicHeight weights CJK / full-width characters as ~2
columns (they wrap at ~24 chars/line, not 48), so a CJK paste reserves close
to its real height even before it is ever measured.
- _applyUserRowIntrinsicHeight reserves max(remembered, estimate): a remembered
height can be a partial paint (a row taller than the viewport only paints its
intersecting slice under content-visibility:auto), so the estimate floors it.
- _rememberRenderedUserRowIntrinsicHeights, called pre-wipe inside
renderMessages, reads the still-laid-out rows' real heights and persists them
keyed by session-relative index, only for rows within the viewport (a fully
off-screen never-painted row reports its collapsed reserve and must not poison
the map), floored at the estimate.
Desktop rests at content-visibility:visible so intrinsic-size is inert there;
verified no behavior change with pointer:fine.
Adds tests/test_issue5744_nonvirtual_userrow_collapse_jumpback.py (7
mutation-checked node-harness tests). Existing #5637/#5638 suites and the
render/virtualization suites pass.
When the anchor row is gone and a virtual top-spacer is present but the snapshot
carries no captured topPadBefore (null), Number(null) is 0, so the topPad-delta
branch would add the ENTIRE current spacer height to scrollTop and fling the reader
far from their content. Guard on an actual (non-null) captured topPadBefore; else
keep the raw fallback target. Mutation-checked (drop the null guard -> the reader is
flung 1300px and the test fails).
Round-2 gate-cert proved the round-1 'snapshot.top + delta' + nearest-to-current
arbiter is mathematically unwinnable: a row's rect offset is scroll-relative
(rect.top - containerRect.top = rowContentPos - scrollTop), so a staged absolute
target only equals the true content-hold when scrollTop == snapshot.top — and in
that exact case the arbiter measures |raw - cur| = 0 and picks raw, writing the
stale value (the reported up-jump, unfixed).
Rework to the app's own realign idiom (mirrors _restoreMessageViewportAnchor and
_compensateScrollForMeasurementDelta): when a measurable anchor row exists, write
el.scrollTop += (currentOffset - capturedOffset) from the LIVE scrollTop — a no-op
when already aligned, a heal when not, no arbiter. Restore the per-tier lookup
guard (key -> sessionIdx, never the rawIdx degradation that maps to a different
message after a virtualization re-window). Genuinely-gone anchor: mirror the
topPad-delta idiom the snapshot already carries (topPadBefore), else keep raw.
Gate-cert tests now use scroll-dependent rect mocks (rect.top = rowContentPos -
scrollTop) so certified hold numbers are physically realizable; mutation-checked
(revert to raw -> the hold tests fail; drop topPad-delta -> that test fails).
_steerClearCurrentOwnerDeadRun cleared busy/activeStreamId/inflight but left
the status line, elapsed timer, and optimistic streaming badge lingering after
a recovered dead run. Add the rest of the app's stale-busy cleanup idiom
(setStatus/setComposerStatus/_clearActivityElapsedTimer/clearOptimisticSessionStreaming),
still bypassing setBusy(false) so its queue-drain can't clobber the restored draft.
vv.scale != 1 means the visual-viewport shrinkage reflects zoom, not the
keyboard; on Chromium 'force enable zoom' that produced a large spurious inset
that jittered on pan. Bail out of the inset when |vv.scale-1|>0.05. + test.
On touch-primary devices (iPad Safari) the on-screen keyboard shrinks the
visual viewport but not the layout viewport, so the bottom-pinned composer
sits behind the keyboard. Compute a --keyboard-bottom-inset from visualViewport
and pad the composer up by it; clear it when the keyboard closes / on
non-touch / no-visualViewport. Reuses the existing visualViewport + pageshow
hooks; no-op on desktop/fine-pointer.
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Frontend fix for the model-picker provider-contamination vector of #5567 (the
backend HERMES_HOME vector shipped in #5577). Credit @b3nw for the root-cause trace.
_modelStateForSelect(sel, modelId) resolved the provider from sel.selectedOptions[0]
(the dropdown's CURRENTLY-selected option) instead of the option whose value matches
modelId. During a profile/tab switch or a model-list rebuild the dropdown transiently
still holds the PREVIOUS profile's default option (e.g. an ollama model), so the wrong
provider was stamped onto a model it doesn't own, persisted into the session's
model_provider, and (since _modelProviderForSend reads the stored value first) re-sent
on every turn -> 'Provider ollama...no API key' for a provider the session never used.
Fix: resolve the provider from the option whose VALUE === modelId; trust the
currently-selected option only when its value matches (preserving the user's exact
pick in the same-value/different-provider collision case). A genuinely-absent model
yields a null provider so the backend re-infers.
This is the WRITE-site fix — it stops all NEW contamination (new-session + send paths
route through _modelStateForSelect). Both Codex and Fable reviewed it SAFE.
Repairing sessions ALREADY poisoned before this shipped is deliberately deferred to a
separate follow-up: a 3-round Codex+Fable gate proved a frontend load-time repair that
reads the shared #modelSelect DOM is a cross-session race surface (it re-manufactured
the reverse-contamination it aimed to fix), and it can't reliably heal live-only-model
sessions anyway. The robust place for that repair is the backend chat-start boundary,
where session + profile identity is unambiguous. Already-poisoned sessions self-heal on
their next healthy send / manual re-pick in the meantime.
Tests: node-behavioral resolver scenarios (tab-switch race, same-value collision,
missing-model null, match-by-value, default->null) + the unrelated-selected-option
case from #5727. Partially addresses #5567 (write-vector); follow-up tracks the
already-poisoned-session repair.
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
A valid session:// deep link to a session owned by a DIFFERENT Hermes
profile used to look identical to a deleted session (404 -> frontend
self-heals away). Now GET /api/session returns a structured 409
session_profile_mismatch envelope (error/code/session_id/profile ONLY,
no transcript) ONLY when the owning profile is KNOWN, and loadSession()
catches it, switches to the owning profile, and retries once. Truly
missing/deleted or legacy None-profile sessions keep the 404 self-heal.
Gate fixes applied (Codex + Fable):
- Codex CORE: added a post-await stale-load guard after
_switchProfileForSessionLoad so a navigation during the switch can't
hijack the UI back to the old session.
- Fable Finding 1: only emit 409 when _session_profile is truthy;
a None-profile (missing/legacy) session under a non-default active
profile now keeps 404 instead of a useless profile=null 409 (which
skipped self-heal + spun the SSE reconnect against a dead sid). Both
detail branches. + 2 regression tests.
- Fable Finding 2: _switchProfileForSessionLoad now clears the sidebar
skeleton + re-renders from cache on switch-POST failure (mirrors the
#4671 canonical-switch catch), then rethrows, so a failed switch can't
strand the sidebar on the skeleton.
Reconciled tests/test_issue1611_session_profile_filtering.py (4 tests) to
the 409 contract while preserving the no-leak boundary assertion.
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Add a complete Czech (cs) locale to hermes-webui: full key parity with the
English reference (1642/1642 keys), real Czech translations for all string and
function-valued keys, Slavic plural helpers for tool/worklog summaries, the cs
login-screen locale in api/routes.py, and a dedicated tests/test_czech_locale.py
parity+placeholder+diacritics guard mirroring the other per-locale tests. All
15 locale-count assertions bumped 14->15.
Co-authored-by: ostravajih <ostravajih@users.noreply.github.com>
Address round-2 gate cert (issuecomment-4896321817) on sha:dcec232d:
1. BRICK (Codex strace-verified): v2's fcntl-flock holder check on
.git/index.lock was wrong. Codex straced git 2.43.0 and proved
that git uses O_CREAT|O_EXCL + rename(2), NOT advisory locking,
so an fcntl-flock probe returns False for a lock file a live git
process is holding open. The 'fail-closed' claim was wrong:
auto-delete could still race and corrupt the index.
2. CORE (Codex): if .git/index.lock vanished between _is_lock_held
and os.remove, the FileNotFoundError surfaced as a 'remove-failed'
diagnostic instead of 'already gone' and prevented the apply retry.
Fix: stop deleting lock files from the server entirely. The only thing
that removes a lock is the user, on the host, via the manual command
surfaced in the response. Once the lock is gone, the user re-clicks
Update Now and the normal non-destructive apply path runs.
Implementation:
api/updates.py:
- Removed _is_lock_held (fcntl-flock probe) -- was provably unsafe.
- Removed _try_remove_lock (os.remove path) -- was provably unsafe.
- Removed _GIT_LOCK_FILES_REMOVABLE (no longer enumerable by name).
- Added _inventory_locks(path) -> dict: pure inventory of well-known
+ other lock files under .git/. Never touches anything.
- Rewrote apply_clear_lock to inventory-only + manual-instruction.
When .git/index.lock is absent, runs the normal non-destructive
apply path; when present, returns ok=False with the exact 'rm -f'
command + an explanation of why the server cannot do this safely
(O_CREAT|O_EXCL cannot be detected with advisory probes).
CORE-1 race evaporates: there is no os.remove to race against.
api/routes.py:
- Updated the /api/updates/clear_lock comment to reflect v2.2.
static/ui.js:
- applyClearUpdateLock: when res.lock_held, call new
_renderLockManualInstruction(target, res) which surfaces the exact
'rm -f <path>' command in a copyable code block plus an
'I've removed the lock -- retry update' button that POSTs the
same endpoint again; the second call takes the success branch
and re-runs the normal apply path. Also lists any other lock
files present so the operator can investigate.
tests/test_updates.py:
- Removed all 4 tests for the deleted v2 helpers (_is_lock_held
Returns*, _try_remove_lock*).
- Added test_v2_probe_helpers_removed: regression guard that
fails loud if either helper is ever reintroduced.
- Rewrote test_apply_clear_lock_removes_unheld_lock_and_runs_normal_update
and test_apply_clear_lock_refuses_when_lock_held for the v2.2
contract. The 'refuses' test monkeypatches os.remove to record
any deletion attempt and asserts the call list is empty -- a
strict runtime guard against future regressions to auto-delete.
- Added 7 new tests: test_inventory_locks_* (4), test_apply_clear
lock_with_no_lock_runs_normal_update, test_apply_clear_lock_with
lock_present_returns_manual_instruction, test_apply_clear_lock
listing_includes_other_locks, test_apply_clear_lock_rejects
unknown_target, test_apply_clear_lock_rejects_not_git_repo.
Verification:
./scripts/test.sh tests/test_updates.py -> 73 passed
./scripts/test.sh tests/test_update_banner_fixes.py -> 102 passed
./scripts/test.sh tests/test_api_timeout.py -> 8 passed
python3 scripts/ruff_lint.py --diff upstream/master -> 0 new violations
Full ./scripts/test.sh -> 12137 passed, 18 failed (all 18 pre-existing
flakes in unrelated subsystems; verified not v2.2 regressions)
Address Greptile P1 review (discussion r3530941293) on commit e8f7e28a:
the v2 frontend recovery path was dead at the DOM level. static/ui.js
references $('btnClearUpdateLock') but no matching element existed in
static/index.html, so users hitting a lock conflict never saw a clickable
affordance -- the error panel was the only visible state.
Add the missing button next to btnForceUpdate in the update-banner action
row:
<button class="update-btn" id="btnClearUpdateLock"
style="display:none"
onclick="applyClearUpdateLock(this)">
Clear lock and retry update
</button>
Style mirrors the neutral update-btn (NOT the red --error used by the
destructive Force update) so the recovery affordance doesn't read as a
warning. Hidden by default; revealed by the existing _showUpdateError()
helper when res.lock_conflict is set.
Regression tests in TestClearLockButton (3 tests):
- test_clear_lock_button_exists
- test_clear_lock_button_hidden_by_default
- test_clear_lock_button_calls_applyClearUpdateLock_handler
These lock in the contract that the button element must be present and
correctly bound; future refactors that rename the id or strip the button
will fail loud. Mirrors the existing TestIndexHtmlBanner conventions
for btnForceUpdate.
Verification:
- ./scripts/test.sh tests/test_updates.py -k lock -> 35 passed
- ./scripts/test.sh tests/test_update_banner_fixes.py -> 102 passed
- ./scripts/test.sh tests/test_api_timeout.py -> 8 passed
- ruff_lint --diff upstream/master -> 0 new violations
Address the RED gate cert on PR #5688 (2 BRICK + 1 CORE) and the
incoming Greptile review (1 P1 + 3 P2):
- BRICK-1 (race-safety): drop the mtime heuristic and any age-based
lock removal. Add /api/updates/clear_lock with a fail-closed
fcntl.flock holder probe (refuses if any process still holds the
file, on POSIX). On non-POSIX, fails closed. Touches only
.git/index.lock (the only well-known short-lived lock git creates).
- BRICK-2 (destructive path coupling): remove lock_conflict from the
force-button condition. Add a separate btnClearUpdateLock that calls
/api/updates/clear_lock -- never apply_force_update. The recovery
path never runs checkout/clean/reset --hard.
- CORE-1 (unconditional pre-cleanup): apply_force_update no longer
iterates .git/**/*.lock. Lock cleanup is exclusively the clear_lock
endpoint's job.
- Greptile P1: when a pull-lock error fires after a stash was pushed,
run git stash pop (with apply+drop fallback) so the user's local
modifications aren't stranded in the stash silently. v2 tests
cover the stashed and the clean cases.
- Greptile P2 (broad 'lock file' substring): tightened _GIT_LOCK_SIGNATURES
to specific git error strings ('index.lock': file exists, '.lock':
file exists, 'another git process seems to be running',
'unable to create .git/index.lock') so unrelated ref-transaction
lock-loss messages no longer trigger a lock-conflict response.
- Greptile P2 (redundant git_dir.exists / magic number): both gone as a
side effect of removing the apply_force_update cleanup loop entirely.
Refactor: rebased onto current upstream/master so PR is no longer
'behind' 130 commits.
Tests:
- 14 new v2 tests covering holder probe (true/false/NOGIL probe),
_try_remove_lock (refuse-on-held/success-on-unheld),
apply_clear_lock (success/refused), signature set parameter table
including false-positive class, apply_force_update no-touch
contract, pull-lock stash restore, pull-lock no-stash-when-clean.
- 2 v1 tests removed (they encoded the unsafe behavior).
- 2 v1 parametrize rows dropped (their positive cases relied on the
broad 'lock file' substring).
Closes#5687
- Detect lock file errors (lock_conflict flag) on fetch/status/pull
failures in _apply_update_inner (#1)
- Expose Force Update button on frontend when lock_conflict is
received (#3)
- Proactively clean stale lock files (>30s old) in apply_force_update
using rglob discovery instead of a hardcoded path list (#2)
- Add 10 tests: _is_git_lock_error parametrized unit tests,
_apply_update_inner lock path coverage, and apply_force_update
stale/recent lock behavior
The renderMessages() re-anchor call `queueMicrotask(()=>_reanchorPinnedTailAfterRender(...))`
threw `ReferenceError: _reanchorPinnedTailAfterRender is not defined` in the standalone
renderMessages() node harness (test_anchor_fallback_ownership), which evals the extracted
renderMessages body with only a minimal set of stubbed helpers.
Guard the call with `typeof` checks on both `queueMicrotask` and
`_reanchorPinnedTailAfterRender`, mirroring the existing
`if(typeof _deferClearProgrammaticScroll==='function')` guard a couple of lines below, so
a harness that evals renderMessages without defining these helpers does not throw. In the
browser both are always defined, so the fix path is unchanged.
Add a source-lock asserting the guard so a future refactor can't drop it and re-break the
harness.
A pinned/tail-following reader gets a fast ~1-row back-and-forth bounce on every
mid-stream re-render (tool completion, activity-scene refresh, clarify echo).
renderMessages() rebuilds the transcript (inner.innerHTML='' then re-append). For a
pinned reader the post-render path writes scrollTop=scrollHeight while still INSIDE
the render sync stack, where the browser reports a transient scrollHeight a few px
short of the settled value (layout is batched). scrollToBottom lands scrollTop a
little high (short of the true tail); that intermediate is painted this frame and the
settle rAF corrects it the next frame -> a ~1-row bounce (measured ~82px, confirmed
with a per-frame scrollTop flight recorder and an isolated CSSOM A/B).
Fix: capture the pre-wipe near-tail state (geometry, since the wipe's clamp scroll
event can perturb the closure pin flags) and, at the end of renderMessages, schedule
_reanchorPinnedTailAfterRender in a microtask. The microtask runs after the sync stack
flushes layout to the settled height but before paint, so writing the settled max
lands the tail exactly and the short intermediate never reaches the screen. A
synchronous call reads the same transient short height and is a no-op (verified: sync
leaves the 82px bounce; microtask drives it to 0). Only re-anchors a pre-wipe
tail-follower left short of the settled max, so an unpinned reader parked in history
is never moved (verified 0px displacement); arms the existing _programmaticScroll
latch; idempotent.
Tests: test_pinned_tail_midstream_jitter.py runs the real extracted helper in a Node
harness (re-anchor short tail-follower / never move a history reader / idempotent
no-op / mutation check / source locks on the queueMicrotask call site and settled-max
write). Neighboring scroll suites remain green.
Address greptile review: the new S.activeStreamId restore branch omitted the
_maybeShowNewMessageScrollCue call that the preserveScroll and idle-unpin restore
branches both make. An unpinned reader whose position is restored mid-stream should
still see the new-message cue, so add it for consistency. Update the behavioral test
to assert the cue fires and the test_issue1690 source-lock for the new branch body.
renderMessages() rebuilds the transcript via inner.innerHTML='' then re-appends
rows. That wipe collapses #msgInner scrollHeight toward the empty-table height, so
the browser is forced to clamp #messages.scrollTop down to the new near-zero max --
a browser primitive (no JS writes scrollTop; overflow-anchor does not govern it).
When this rebuild happens mid-stream while a reader has scrolled up into history
(_messageUserUnpinned), _scrollAfterMessageRender took the S.activeStreamId branch
and called scrollIfPinned(), which is a no-op for an unpinned reader -- so the
clamped-to-0 scrollTop was never restored and the reader was stranded at the top.
Fix: in that branch, when the reader is unpinned and a pre-wipe scroll snapshot
exists (renderMessages already captures one whenever _messageUserUnpinned), restore
the snapshot instead of the no-op. Pinned / tail-following readers keep
scrollIfPinned() so live-follow is unchanged. Reuses the existing
_restoreMessageScrollSnapshot helper (same one the preserveScroll and idle-unpin
branches use), so no new core-timing risk.
Tests: test_wipe_rows0_streaming_unpinned_restore.py executes the real extracted
_scrollAfterMessageRender against a fake #messages + instrumented restore stubs and
asserts the unpinned+activeStreamId+snapshot case restores (scrollTop returns to the
pre-clamp value) while pinned / no-snapshot cases fall back to scrollIfPinned();
mutation-checked (reverting to the bare scrollIfPinned() fails the behavioral asserts).
Updates the test_issue1690 source-lock for the new branch structure.
The module-level _userRowIntrinsicHeightBySessionIdx cache is keyed by
session-relative index (_messageSessionIndexForRawIdx = _messageSessionIndexBase()
+ rawIdx). The base is 0 for the common non-offset session, so keys collide across
sessions: after switching from a session with tall user rows to one with short
rows, the new session's off-screen rows at colliding keys inherited the previous
session's remembered heights and inflated scrollHeight until re-measured.
Add _clearUserRowIntrinsicHeightCache() and call it from
_clearMessageVirtualHeightCache — the existing per-session height-cache clear that
_resetMessageRenderWindow invokes on session switch — so the new cache shares the
exact same lifetime as _messageVirtualHeightCache. Keys are deleted in place to
keep the const binding stable. Guard the call with typeof so the node harnesses
that extract _clearMessageVirtualHeightCache without this helper keep running.
Adds a mutation-checked regression (test_cache_cleared_on_session_switch_prevents_
stale_height_bleed): a rebuilt row at a colliding key after the clear must fall
back to the content estimate, not the stale remembered height; making the clear a
no-op fails the test.
Round-2 gate cert (iOS CORE): the touch predicate _isTouchLikeMessageViewport
used matchMedia('(pointer:coarse)'), which is true on BOTH Android and iOS. But
overflow-anchor is inert on iOS WebKit (the repo's own static/style.css mobile
content-visibility block documents this — it deliberately does not set
overflow-anchor:none because it is a no-op on iOS and re-opens the #4856/#5338
jump on Android). So on iOS the stale-anchor refusal fired but its premise (let
the native overflow-anchor layer hold the viewport) is false → a scrolled-up iOS
reader was left unheld after above-viewport growth, the same class as the round-1
desktop regression, one platform over.
Split the platform check: add _isIOSWebKit() (classic iPhone/iPod/iPad UA, plus
iPadOS 13+ which masquerades as MacIntel but has maxTouchPoints>1 unlike a real
Mac) and exclude it from _isTouchLikeMessageViewport. The refusal now fires ONLY
on Android (pointer:coarse AND overflow-anchor actually works); desktop and iOS
both keep the explicit semantic realign / absolute snapshot.top restore.
Tests: 3 new predicate cases (iPhone, iPadOS-as-Mac, Android control), all
mutation-checked — removing the _isIOSWebKit exclusion fails exactly the iOS
cases while Android/desktop stay green; broadening _isIOSWebKit to any touch
device fails the Android control. The Node harness forces the navigator mock via
Object.defineProperty because Node 18+ ships a built-in read-only navigator that
a plain assignment silently ignores.
Note: iOS Safari cannot be exercised by the Node harness (the 'native anchor
holds' postcondition is mocked, never observed), so this needs a real iOS-Safari
recording before merge per the gate cert.
The node test harnesses (e.g. test_anchor_fallback_ownership) extract
renderMessages and _measureMessageVirtualRow by source and run them with every
collaborator stubbed by name. The new _applyUserRowIntrinsicHeight /
_rememberUserRowIntrinsicHeight helpers were not among those stubs, so the
extracted functions threw ReferenceError ("node behavior check failed") on the
CI shard that runs those files. Gate both cross-function call sites on
`typeof helper === 'function'` (same pattern already used across ui.js for
harness-extracted helpers). Production always has the helpers defined, so the
guard is a no-op there; a harness without them cleanly skips the reservation
and keeps prior behavior. No behavior change on the real page.
A virtualization wipe-and-rebuild recreates user rows as fresh DOM elements,
which discards content-visibility:auto's last-remembered size. An off-screen
tall user row (e.g. a long paste) then falls back to the flat
`contain-intrinsic-size: auto 96px` from the stylesheet the instant it is
rebuilt, collapsing scrollHeight by (realHeight - 96px). The browser then either
force-clamps scrollTop (the dTop≈dH jump) or re-anchors to a far row (the dTop≫dH
jump) — both mobile jump-back symptoms trace to this one collapse.
This is layer-scoped to the CSS/content-visibility path and is only active under
`@media (pointer: coarse)` where content-visibility:auto is applied; desktop
rests at content-visibility:visible so the intrinsic-size is ignored (verified:
writing an intrinsic-size on a visible row produces zero scrollHeight/layout
delta), making the change inert on desktop with no platform gate needed.
Fix: remember each user row's measured height keyed by its stable
data-session-msg-idx and re-apply it as an inline contain-intrinsic-size, so a
rebuilt fresh element reserves the real height instead of 96px:
- `_measureMessageVirtualRow` persists + writes the measured height (user rows
only; assistant rows are content-visibility:visible on mobile).
- the user-row build path in `renderMessages` applies the remembered height, or
a content-length estimate (floored at 96px so short rows never regress) before
the row has ever been measured, so even the first fresh-element frame does not
collapse.
Adds tests/test_issue5638_user_row_intrinsic_height_collapse.py (node harness),
mutation-checked: disabling either the measure writeback or the build-time apply
makes the corresponding assertions fail.
Address the two non-blocking observations from re-review of the touch-gate fix:
1. Add a direct unit test for _isTouchLikeMessageViewport itself — the guard-wiring
tests stub the predicate via a boolean, which validates that the _touchHold term
gates the refusal but not the predicate's own mid-realign stability (the claim that
motivated choosing matchMedia over the computed overflow-anchor probe). The new
_predicate_harness exercises the real predicate + _browserOverflowAnchorActive with
mocked matchMedia/getComputedStyle:
- test_predicate_stays_true_on_touch_when_inline_anchor_clobbered_to_none: on a
pointer:coarse device whose inline overflowAnchor was clobbered to 'none' by a
prior realign tick, the predicate must still report touch=true. Mutation-checked:
reverting the predicate to the bare computed probe makes exactly this test fail.
- test_predicate_false_on_desktop_fine_pointer / _falls_back_to_computed_probe_without_matchmedia
cover the desktop and no-matchMedia paths.
2. Note in the predicate comment that the no-matchMedia fallback to the computed probe
is best-effort (matchMedia('(pointer:coarse)') is universally supported in every
targeted browser, so the primary path is what runs).
The two streaming stale-anchor guards added for #5637 refuse a scroll
restore and rely on the browser's native overflow-anchor layer to hold
the viewport. That layer is only active where .messages computes to
overflow-anchor:auto (touch viewports). On hover+fine-pointer desktops
.messages is overflow-anchor:none, so refusing the restore leaves nothing
to hold the reader -> the desktop reader is yanked after above-viewport
growth (the same jump the guards fix on mobile), and the fallback also
latches _messageUserUnpinned=true.
Gate both refusals on _isTouchLikeMessageViewport(container), a
matchMedia('(pointer:coarse)') predicate (falling back to the computed
overflow-anchor probe) so they fire only where native anchoring can
actually hold the viewport. Desktop keeps its semantic scrollTop realign
and absolute snapshot.top fallback. matchMedia is used rather than the
computed-anchor probe alone because the realign temporarily writes inline
overflowAnchor:none for its own scroll write, which a computed probe would
misread mid-realign.
Add two desktop regression tests (touch_like=False) covering the exact
stale-anchor case on a no-native-anchor viewport; both are mutation-checked
(dropping the touch gate makes them fail).
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).
Isla-Liu