Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion static/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -5941,10 +5941,17 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
// render also populates the cache with the correctly-collapsed DOM, and
// the same-frame JS restore absorbs the collapse so there is no jump.
// (#5260 gate-cert: keep-open must be transient + uncached for everyone.)
// #6385: capture the scroll snapshot from the LIVE DOM before arming
// keep-open, so the collapse render below anchors to the content the
// reader was actually viewing — not to a stale intermediate state where
// the worklog was temporarily expanded.
const _doneLiveScrollSnapshot=typeof _captureMessageScrollSnapshot==='function'
? _captureMessageScrollSnapshot()
: null;
if(typeof _armKeepSettledWorklogOpen==='function') _armKeepSettledWorklogOpen(_settledStreamId);
syncTopbar();renderMessages({preserveScroll:true});
if(typeof _disarmKeepSettledWorklogOpen==='function') _disarmKeepSettledWorklogOpen();
if(typeof _renderMessagesWithScrollSnapshot==='function') _renderMessagesWithScrollSnapshot();
if(typeof _renderMessagesWithScrollSnapshot==='function') _renderMessagesWithScrollSnapshot({_prescrollSnapshot:_doneLiveScrollSnapshot});
else renderMessages({preserveScroll:true});
if(shouldFollowOnDone&&typeof scrollToBottom==='function') scrollToBottom();
if(typeof noteWorkspaceMutationsFromToolCalls==='function') noteWorkspaceMutationsFromToolCalls(S.toolCalls);
Expand Down
8 changes: 7 additions & 1 deletion static/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -15086,7 +15086,13 @@ function _restoreMessageScrollSnapshotSameFrame(snapshot){
}
}
function _renderMessagesWithScrollSnapshot(options){
const scrollSnapshot=_captureMessageScrollSnapshot();
// Accept an optional pre-captured scroll snapshot via _prescrollSnapshot.
// When provided, it is used INSTEAD of capturing a fresh one from the current
// DOM state — essential for the STREAM_DONE collapse render: the caller has
// already captured the snapshot from the LIVE DOM (before keep-open was armed),
// and re-capturing from the intermediate expanded-worklog state would capture
// stale anchors that no longer exist after the worklog collapses. (#6385)
const scrollSnapshot=(options&&options._prescrollSnapshot)||_captureMessageScrollSnapshot();
renderMessages({...(options||{}),preserveScroll:true});
_restoreMessageScrollSnapshotSameFrame(scrollSnapshot);
}
Expand Down
107 changes: 102 additions & 5 deletions tests/test_issue4970_stream_done_shrink_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,18 +171,32 @@ def test_stream_done_runs_scroll_preserving_collapse_pass_after_disarm():
# UNCONDITIONALLY right after disarm (covers both pin states), THEN scrollToBottom()
# only for followers to re-settle at the tail. This makes keep-open genuinely
# one-frame for everyone.
#
# Round 10 (#6385): capture the scroll snapshot from the LIVE DOM before arming
# keep-open, so the collapse render below anchors to the content the reader was
# actually viewing — not to a stale intermediate state where the worklog was
# temporarily expanded. The pre-capture variable must be defined before arm and
# threaded into the collapse pass as `_prescrollSnapshot`.
pre_capture_idx = MESSAGES_JS.index("_doneLiveScrollSnapshot")
arm_idx = MESSAGES_JS.index("_armKeepSettledWorklogOpen")
assert pre_capture_idx < arm_idx, (
"_doneLiveScrollSnapshot must be captured before _armKeepSettledWorklogOpen "
"to snapshot the live (not expanded-worklog) DOM positions."
)
disarm_idx = MESSAGES_JS.index("_disarmKeepSettledWorklogOpen()")
after = MESSAGES_JS[disarm_idx : disarm_idx + 700]
# The collapse pass must run after disarm for BOTH pin states.
assert "_renderMessagesWithScrollSnapshot()" in after, (
# The collapse pass must run after disarm for BOTH pin states, and it must
# carry the pre-captured live snapshot.
assert "_renderMessagesWithScrollSnapshot({_prescrollSnapshot:_doneLiveScrollSnapshot})" in after, (
"after _disarmKeepSettledWorklogOpen() the STREAM_DONE handler must run a "
"scroll-preserving collapse pass (_renderMessagesWithScrollSnapshot) so the "
"forced-open worklog collapses back to the user/live state without the jump."
"scroll-preserving collapse pass (_renderMessagesWithScrollSnapshot) with the "
"pre-captured live snapshot so the forced-open worklog collapses back to "
"the user/live state without the jump."
)
# The follower re-settle (scrollToBottom) must come AFTER the collapse render —
# otherwise a pinned follower keeps the forced-open DOM (scrollToBottom does not
# re-render). This is the exact pinned-path bug the second RED gate-cert caught.
collapse_pos = after.index("_renderMessagesWithScrollSnapshot()")
collapse_pos = after.index("_renderMessagesWithScrollSnapshot")
follow_pos = after.index("shouldFollowOnDone")
assert collapse_pos < follow_pos, (
"the collapse render must run BEFORE the shouldFollowOnDone scrollToBottom() "
Expand All @@ -200,3 +214,86 @@ def test_stream_done_runs_scroll_preserving_collapse_pass_after_disarm():
assert "_captureMessageScrollSnapshot()" in wrapper
assert "_restoreMessageScrollSnapshotSameFrame" in wrapper


@pytest.mark.skipif(shutil.which("node") is None, reason="node required for behavioral test")
def test_prescroll_snapshot_bypasses_capture_no_option_fallback_still_captures():
"""Sentinel _prescrollSnapshot reaches restore without re-capture.

Behavioral harness for _renderMessagesWithScrollSnapshot:

1. Supply a sentinel snapshot via _prescrollSnapshot, stub
_captureMessageScrollSnapshot with a counter — proves the
sentinel reaches _restoreMessageScrollSnapshotSameFrame
without a second capture.

2. Call with no options — proves the no-option fallback still
calls _captureMessageScrollSnapshot() normally (the contract
for callers at static/ui.js:9565, 14416, 14424).

3. Call with empty options {} — same fallback proof.
"""
wrapper = _extract("_renderMessagesWithScrollSnapshot")
harness = textwrap.dedent(f"""
let captureCount = 0;
let restoredSnapshot = null;
let renderedOptions = null;
globalThis._captureMessageScrollSnapshot = () => {{
captureCount++;
return {{_sentinel: 'fresh-capture', bottom: 0, pinned: true}};
}};
globalThis.renderMessages = (opts) => {{ renderedOptions = opts; }};
globalThis._restoreMessageScrollSnapshotSameFrame = (snap) => {{ restoredSnapshot = snap; }};
{wrapper}
// Test 1: supplied _prescrollSnapshot -> use it, do NOT capture
const sentinel = {{_sentinel: 'pre-captured', bottom: 42, pinned: true}};
_renderMessagesWithScrollSnapshot({{_prescrollSnapshot: sentinel}});
const test1_usedPrescroll = restoredSnapshot === sentinel;
const test1_noCapture = captureCount === 0;
const test1_preservedArgs = renderedOptions && renderedOptions._prescrollSnapshot === sentinel;
// Reset
captureCount = 0; restoredSnapshot = null; renderedOptions = null;
// Test 2: no options -> fall back to _captureMessageScrollSnapshot()
_renderMessagesWithScrollSnapshot();
const test2_captured = captureCount === 1;
const test2_usedCapture = restoredSnapshot && restoredSnapshot._sentinel === 'fresh-capture';
// Reset
captureCount = 0; restoredSnapshot = null; renderedOptions = null;
// Test 3: empty options object -> still fall back to capture
_renderMessagesWithScrollSnapshot({{}});
const test3_captured = captureCount === 1;
const test3_usedCapture = restoredSnapshot && restoredSnapshot._sentinel === 'fresh-capture';
console.log(JSON.stringify({{
test1_usedPrescroll,
test1_noCapture,
test1_preservedArgs,
test2_captured,
test2_usedCapture,
test3_captured,
test3_usedCapture
}}));
""")
res = subprocess.run(["node", "-e", harness], capture_output=True, text=True, timeout=30)
assert res.returncode == 0, res.stderr
out = json.loads(res.stdout.strip())
assert out["test1_usedPrescroll"] is True, (
"supplied _prescrollSnapshot must reach _restoreMessageScrollSnapshotSameFrame"
)
assert out["test1_noCapture"] is True, (
"supplied _prescrollSnapshot must bypass _captureMessageScrollSnapshot()"
)
assert out["test1_preservedArgs"] is True, (
"options passed to _renderMessagesWithScrollSnapshot must be forwarded to renderMessages"
)
assert out["test2_captured"] is True, (
"no-option call must fall back to _captureMessageScrollSnapshot()"
)
assert out["test2_usedCapture"] is True, (
"no-option call's captured snapshot must reach _restoreMessageScrollSnapshotSameFrame"
)
assert out["test3_captured"] is True, (
"empty-options call must fall back to _captureMessageScrollSnapshot()"
)
assert out["test3_usedCapture"] is True, (
"empty-options call's captured snapshot must reach _restoreMessageScrollSnapshotSameFrame"
)

Loading