release: v0.50.250
Bundles 2 PRs: - #1366 fix: guard finalizeThinkingCard with session ID check (with pre-release fix) - #1367 fix(clarify-sse): stale-detector health timer (Opus SHOULD-FIX from v0.50.249) Pre-release fix on #1366: the contributor's guard depends on liveAssistantTurn.dataset.sessionId, but no code in the repo sets that attribute. Without the fix, the guard would always early-return (undefined !== sid is always true), breaking the streaming UI completely — every assistant turn's thinking card would stay open forever. Added per-site stamps at all 3 places that create liveAssistantTurn in static/ui.js, plus a regression test that fails any future creation site that forgets the stamp.
This commit is contained in:
@@ -2,6 +2,12 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.50.250] — 2026-04-30
|
||||
|
||||
### Fixed
|
||||
- **Cross-tab thinking-card cleanup no longer touches the wrong session's DOM** — switching browser tabs while a stream is running could leave `finalizeThinkingCard()` operating on a stale `liveAssistantTurn` node — the thinking card belonged to the stream that started it, not the session currently displayed in the active tab. The guard early-returns when the live turn's `dataset.sessionId` does not match `S.session.session_id`. Per-site stamps were also added: every place that creates `liveAssistantTurn` (3 sites in `static/ui.js`) now writes the current session id onto `dataset.sessionId` so the guard has the data it needs to compare. Without the stamps the guard would always early-return (because `undefined !== "<sid>"` is always true), breaking the streaming UI completely — caught during pre-release review of #1366. Plus a regression test that fails any future `liveAssistantTurn` creation site that forgets the stamp. (`static/ui.js`, `tests/test_pr1366_finalize_thinking_card_guard.py`) @JKJameson — PR #1366
|
||||
- **Clarify SSE health timer is now an actual stale-detector, not an unconditional 60s force-reconnect** — the timer at `static/messages.js:1715` shipped in v0.50.249 / PR #1355 closed and re-opened the EventSource every 60s regardless of activity, with a comment that wrongly claimed it was a "no event in 60s" detector. Effects on healthy connections: one TCP/SSE setup+teardown per minute per active session, plus a `clarify._lock` round-trip and fresh `initial` snapshot push from the server. Now tracks `lastEventAt` on `initial`/`clarify` event arrivals; only reconnects when the gap exceeds 60s. Under healthy conditions (server keepalives every 30s, real events on submit/resolve) the timer never fires. Originally pulled out of the v0.50.249 batch as out-of-scope; brought back per the rule that small correctness-improving fixes ship even when flagged out-of-scope. (`static/messages.js`) — PR #1367 (Opus pre-release review of v0.50.249, SHOULD-FIX #2)
|
||||
|
||||
## [v0.50.249] — 2026-04-30
|
||||
|
||||
### Added
|
||||
|
||||
@@ -3172,6 +3172,11 @@ function renderMessages(){
|
||||
seg.dataset.rawText=String(content).trim();
|
||||
if(m._live){
|
||||
currentAssistantTurn.id='liveAssistantTurn';
|
||||
// Stamp the session id on the live turn so finalizeThinkingCard()
|
||||
// and other late callbacks can verify they're operating on the
|
||||
// right session's DOM (the user may have switched tabs/sessions
|
||||
// while this stream is still streaming). See #1366.
|
||||
if(S.session) currentAssistantTurn.dataset.sessionId=S.session.session_id;
|
||||
seg.setAttribute('data-live-assistant','1');
|
||||
}
|
||||
if(_ERR_MSG_RE.test(String(content||'').trim())) seg.dataset.error='1';
|
||||
@@ -3543,6 +3548,7 @@ function appendLiveToolCard(tc){
|
||||
if(!turn){
|
||||
turn=_createAssistantTurn();
|
||||
turn.id='liveAssistantTurn';
|
||||
if(S.session) turn.dataset.sessionId=S.session.session_id; // see #1366
|
||||
$('msgInner').appendChild(turn);
|
||||
}
|
||||
const inner=_assistantTurnBlocks(turn);
|
||||
@@ -4330,6 +4336,7 @@ function appendThinking(text=''){
|
||||
if(!turn){
|
||||
turn=_createAssistantTurn();
|
||||
turn.id='liveAssistantTurn';
|
||||
if(S.session) turn.dataset.sessionId=S.session.session_id; // see #1366
|
||||
$('msgInner').appendChild(turn);
|
||||
}
|
||||
const blocks=_assistantTurnBlocks(turn);
|
||||
|
||||
87
tests/test_pr1366_finalize_thinking_card_guard.py
Normal file
87
tests/test_pr1366_finalize_thinking_card_guard.py
Normal file
@@ -0,0 +1,87 @@
|
||||
"""Regression tests for the v0.50.250 finalizeThinkingCard cross-tab guard.
|
||||
|
||||
PR #1366 added an early-return guard to finalizeThinkingCard():
|
||||
const _guardTurn = $('liveAssistantTurn');
|
||||
if(_guardTurn && S.session && _guardTurn.dataset.sessionId !== S.session.session_id) return;
|
||||
|
||||
The guard's correctness depends on `liveAssistantTurn.dataset.sessionId`
|
||||
being set whenever the turn is created. If it's never set, the
|
||||
comparison is `undefined !== "<some-id>"` which is always true, and
|
||||
finalizeThinkingCard() always early-returns — breaking the streaming
|
||||
UI completely (every assistant turn's thinking card stays open
|
||||
forever).
|
||||
|
||||
These tests pin both invariants:
|
||||
1. The guard exists in finalizeThinkingCard()
|
||||
2. Every site that creates `liveAssistantTurn` also stamps the
|
||||
dataset.sessionId attribute
|
||||
"""
|
||||
|
||||
import pathlib
|
||||
import re
|
||||
|
||||
REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
|
||||
UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_finalize_thinking_card_guard_exists():
|
||||
"""finalizeThinkingCard() must early-return when displayed session != streaming session."""
|
||||
start = UI_JS.find("function finalizeThinkingCard()")
|
||||
assert start != -1, "finalizeThinkingCard() must exist"
|
||||
end = UI_JS.find("\nfunction ", start + 1)
|
||||
body = UI_JS[start:end if end != -1 else len(UI_JS)]
|
||||
# The guard must read dataset.sessionId from the live turn AND compare
|
||||
# against S.session.session_id. The exact form must early-return.
|
||||
assert "dataset.sessionId" in body, (
|
||||
"finalizeThinkingCard() must read dataset.sessionId from the live turn "
|
||||
"to detect cross-tab/cross-session DOM mismatch."
|
||||
)
|
||||
assert "S.session.session_id" in body, (
|
||||
"finalizeThinkingCard() guard must compare against S.session.session_id."
|
||||
)
|
||||
|
||||
|
||||
def test_live_turn_creation_sites_stamp_session_id():
|
||||
"""Every site that sets `turn.id='liveAssistantTurn'` must also set
|
||||
`turn.dataset.sessionId`. If any site forgets the stamp, the guard in
|
||||
finalizeThinkingCard() always early-returns at that branch (because
|
||||
`undefined !== "<sid>"` is always true), breaking the streaming UI.
|
||||
"""
|
||||
# Find every block that sets the id. Track each occurrence and verify
|
||||
# a dataset.sessionId stamp appears within ~5 lines after it.
|
||||
sites = []
|
||||
for m in re.finditer(r"\.id=['\"]liveAssistantTurn['\"]", UI_JS):
|
||||
# Find what variable name was used (e.g. `turn.id=`, `currentAssistantTurn.id=`)
|
||||
line_start = UI_JS.rfind("\n", 0, m.start()) + 1
|
||||
line = UI_JS[line_start:m.end() + 1]
|
||||
# Get the variable name
|
||||
var_m = re.search(r"(\w+)\.id=['\"]liveAssistantTurn['\"]", line)
|
||||
var_name = var_m.group(1) if var_m else "?"
|
||||
# Look at the next ~500 chars for a dataset.sessionId stamp on the same var
|
||||
# (500 chars accommodates an explanatory comment block before the stamp).
|
||||
window = UI_JS[m.end():m.end() + 500]
|
||||
stamped = bool(re.search(rf"{re.escape(var_name)}\.dataset\.sessionId\s*=", window))
|
||||
sites.append((var_name, m.start(), stamped))
|
||||
|
||||
assert sites, "Expected at least one site setting `<var>.id='liveAssistantTurn'`"
|
||||
unstamped = [(v, p) for v, p, s in sites if not s]
|
||||
assert not unstamped, (
|
||||
f"Found {len(unstamped)} site(s) where `<var>.id='liveAssistantTurn'` "
|
||||
f"is set but `<var>.dataset.sessionId` is NOT stamped within the next "
|
||||
f"500 chars: {unstamped}. Without the stamp, the guard in "
|
||||
f"finalizeThinkingCard() always early-returns at this branch (because "
|
||||
f"undefined !== '<sid>' is always true), breaking the streaming UI. "
|
||||
f"Add `if(S.session) <var>.dataset.sessionId=S.session.session_id;` "
|
||||
f"after the id assignment."
|
||||
)
|
||||
|
||||
|
||||
def test_at_least_three_live_turn_sites():
|
||||
"""Sanity check: there are at least 3 sites that create the live turn.
|
||||
If a future refactor reduces this, the test_live_turn_creation_sites_stamp_session_id
|
||||
test still catches missing stamps, but this catches accidental site removal."""
|
||||
matches = re.findall(r"\.id=['\"]liveAssistantTurn['\"]", UI_JS)
|
||||
assert len(matches) >= 3, (
|
||||
f"Expected at least 3 sites assigning liveAssistantTurn id, found {len(matches)}. "
|
||||
"If sites were intentionally consolidated, this assertion can be relaxed."
|
||||
)
|
||||
Reference in New Issue
Block a user