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.
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.
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).
Follow-up to #5538: the recovery card now also renders from _activeCompressionRecoveryPayload() on the final assistant message when a rebuilt/legacy session lacks the per-message _compressionRecovery marker (guarded to last/turn-final assistant only; no double-render; cleared recovery still hidden). Gate: Codex SAFE, Fable SHIP-UX, suite 12179/0, browser clean.
Co-authored-by: Frank Song <franksong2702@gmail.com>
test_same_stream_in_two_tabs_receives_identical_token_sequence started both
SSE-handler threads then immediately put_nowait'd the tokens + stream_end
without waiting for both threads to subscribe. StreamChannel.put_nowait()
buffers events only while there are zero subscribers and clears that buffer
the moment the first subscriber attaches and broadcasts live — so producing
events in the window after tab A subscribes but before tab B does orphans
tab B (subscribes to a cleared buffer, never sees stream_end, hangs ->
thread.join timeout -> 'every tab should finish' fails). ~20% timing flake
on slow CI runners (3.11 shard-4).
Wait until len(stream._subscribers) == len(handlers) (bounded 5s) before
producing events, so delivery is deterministic regardless of thread-start
timing. Test-only; no production change (buffer-then-broadcast is correct for
the real reconnect case).
Verified: original flakes 2/30 under CPU contention; fixed 30/30.
Fixes#5628
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Root fix for the #5619 profile-switch config-cache bug: split the reload gate so a profile-PATH change forces reload unconditionally (was wrongly suppressed by in-memory overrides → stale default-profile config/models/keys), while a file-mtime change still respects overrides. Folded in a test-isolation fix (Codex): the stage302 regression test leaked _cfg_cache/_cfg_mtime/_cfg_path/_cfg_fingerprint/cfg globals (deepcopy snapshot + clear/update restore, since reload_config mutates _cfg_cache in place). Full gate: Codex SAFE (2 rounds), suite 12178/0, 121 profile/provider tests.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
t