Fix post-start attach resilience for #4481
This commit is contained in:
1 parent
ccae754c84
commit
36e992415b
4 files changed
+130
-68
No files matched your search
@@ -3,6 +3,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Post-start streaming errors in the browser no longer hide active streams after a successful `/api/chat/start` (#4481).** If browser-side bookkeeping after stream start throws, it is now logged as recoverable and `attachLiveStream(activeSid, streamId, uploadedNames)` still runs when `stream_id` is available, so a transient UI exception cannot clear the send state or prevent live tokens from attaching.
|
||||
|
||||
## [v0.51.513] — 2026-06-19 — Release RX (credential-pool quota status for all pooled providers)
|
||||
|
||||
### Added
|
||||
|
||||
+80
-57
@@ -898,6 +898,16 @@ function _runOptionalPreStartUiStep(label, fn){
|
||||
}
|
||||
}
|
||||
|
||||
function _runOptionalPostStartUiStep(label, fn){
|
||||
try{
|
||||
return typeof fn==='function'?fn():undefined;
|
||||
}catch(e){
|
||||
const message=e&&e.message?e.message:String(e||'unknown error');
|
||||
try{console.warn('[webui] optional post-start UI step failed', label, message);}catch(_){ }
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function _sessionTitleLooksDefaultOrProvisional(titleText, provisionalText){
|
||||
const title=String(titleText||'').replace(/\s+/g,' ').trim();
|
||||
if(!title||title==='Untitled'||title==='New Chat')return true;
|
||||
@@ -1248,8 +1258,12 @@ async function send(){
|
||||
|
||||
// Start the agent via POST, get a stream_id back
|
||||
let streamId;
|
||||
let postStartData;
|
||||
let modelStateForPostStart;
|
||||
let explicitPickForPostStart;
|
||||
try{
|
||||
const _modelState=_chatPayloadModelState();
|
||||
modelStateForPostStart=_modelState;
|
||||
const _pendingPick=(typeof _readPendingSessionModel==='function')
|
||||
? _readPendingSessionModel(activeSid)
|
||||
: null;
|
||||
@@ -1262,6 +1276,7 @@ async function send(){
|
||||
// once read so a later send of an unchanged dropdown isn't treated as an explicit
|
||||
// pick. (#3739/#3737, Codex catch)
|
||||
if(_explicitPick && typeof _clearPendingSessionModel==='function') _clearPendingSessionModel(activeSid);
|
||||
explicitPickForPostStart=_explicitPick;
|
||||
const startData=await api('/api/chat/start',{method:'POST',body:JSON.stringify({
|
||||
session_id:activeSid,message:msgText,
|
||||
// S.session.model remains authoritative; the helper only resolves a
|
||||
@@ -1272,63 +1287,7 @@ async function send(){
|
||||
explicit_model_pick:_explicitPick||undefined,
|
||||
attachments:uploaded.length?uploaded:undefined
|
||||
})});
|
||||
|
||||
if(startData.title) applySessionTitleUpdate(activeSid, startData.title, {provisionalText:displayText.slice(0,64), rememberProvisional:true});
|
||||
|
||||
if(startData.effective_model && S.session){
|
||||
const _sentModel=_modelState.model;
|
||||
if(_explicitPick && _sentModel && startData.effective_model!==_sentModel && typeof showToast==='function'){
|
||||
showToast('Model '+_sentModel+' changed to '+startData.effective_model+' — profile provider mismatch', 5000);
|
||||
}
|
||||
S.session.model=startData.effective_model;
|
||||
S.session.model_provider=startData.effective_model_provider||S.session.model_provider||null;
|
||||
localStorage.setItem('hermes-webui-model', startData.effective_model);
|
||||
if(typeof _writePersistedModelState==='function') _writePersistedModelState(startData.effective_model,S.session.model_provider||null);
|
||||
if($('modelSelect')) _applyModelToDropdown(startData.effective_model, $('modelSelect'),S.session.model_provider||null);
|
||||
if(typeof syncTopbar==='function') syncTopbar();
|
||||
}else if(startData.effective_model_provider && S.session){
|
||||
S.session.model_provider=startData.effective_model_provider;
|
||||
if(typeof _writePersistedModelState==='function') _writePersistedModelState(S.session.model||'',S.session.model_provider||null);
|
||||
if($('modelSelect')&&typeof _applyModelToDropdown==='function') _applyModelToDropdown(S.session.model||'', $('modelSelect'), S.session.model_provider||null);
|
||||
if(typeof syncModelChip==='function') syncModelChip();
|
||||
if(typeof syncTopbar==='function') syncTopbar();
|
||||
}
|
||||
streamId=startData.stream_id;
|
||||
S.activeStreamId = streamId;
|
||||
if(typeof appendThinking==='function') appendThinking('',{pending:true});
|
||||
// setBusy(true) already ran with activeStreamId=null; refresh now that we
|
||||
// have a stream id so the primary button can switch to Stop (see getComposerPrimaryAction).
|
||||
if(typeof updateSendBtn==='function') updateSendBtn();
|
||||
if(S.session&&typeof startData.pending_started_at==='number'){
|
||||
S.session.pending_started_at=startData.pending_started_at;
|
||||
}
|
||||
if(S.session&&S.session.session_id===activeSid){
|
||||
S.session.active_stream_id = streamId;
|
||||
}
|
||||
if(S.session&&S.session.session_id===activeSid&&typeof showLiveRunStatus==='function'){
|
||||
const _startedAt=typeof startData.pending_started_at==='number'
|
||||
? startData.pending_started_at
|
||||
: (S.session.pending_started_at||Date.now()/1000);
|
||||
showLiveRunStatus(activeSid,{startedAt:_startedAt});
|
||||
}
|
||||
if(typeof upsertActiveSessionForLocalTurn==='function'){
|
||||
// Third optimistic pass: stream_id is now known, so the row can reconcile
|
||||
// against real active-stream metadata before the background refresh lands.
|
||||
upsertActiveSessionForLocalTurn({title:S.session&&S.session.title||displayText.slice(0,64),messageCount:S.messages.length,timestampMs:Date.now()});
|
||||
}
|
||||
if(!INFLIGHT[activeSid]){
|
||||
INFLIGHT[activeSid]={messages:optimisticMessages,uploaded:uploadedNames,toolCalls:[]};
|
||||
}
|
||||
const currentInflight=INFLIGHT[activeSid];
|
||||
markInflight(activeSid, streamId);
|
||||
if(typeof saveInflightState==='function'){
|
||||
saveInflightState(activeSid,{streamId,messages:currentInflight.messages||optimisticMessages,uploaded:uploadedNames,toolCalls:currentInflight.toolCalls||[]});
|
||||
}
|
||||
// Refresh session list so background streaming indicators appear immediately for the
|
||||
// session that was just started and any others that may already be running.
|
||||
if(typeof renderSessionList === 'function') {
|
||||
void renderSessionList();
|
||||
}
|
||||
postStartData = startData;
|
||||
}catch(e){
|
||||
const errMsg=String((e&&e.message)||'');
|
||||
// If /api/chat/start returns 404, the session was deleted server-side
|
||||
@@ -1392,6 +1351,70 @@ async function send(){
|
||||
return;
|
||||
}
|
||||
|
||||
const startData = postStartData || {};
|
||||
streamId = postStartData ? postStartData.stream_id : null;
|
||||
S.activeStreamId = streamId;
|
||||
// setBusy(true) already ran with activeStreamId=null; refresh now that we
|
||||
// have a stream id so the primary button can switch to Stop (see
|
||||
// getComposerPrimaryAction).
|
||||
if(typeof updateSendBtn==='function') updateSendBtn();
|
||||
_runOptionalPostStartUiStep('post-start ui/bookkeeping', ()=>{
|
||||
const _modelState=modelStateForPostStart || _chatPayloadModelState();
|
||||
const _explicitPick=explicitPickForPostStart;
|
||||
if(startData&&startData.title) applySessionTitleUpdate(activeSid, startData.title, {provisionalText:displayText.slice(0,64), rememberProvisional:true});
|
||||
|
||||
if(startData&&startData.effective_model && S.session){
|
||||
const _sentModel=_modelState&&_modelState.model;
|
||||
if(_explicitPick && _sentModel && startData.effective_model!==_sentModel && typeof showToast==='function'){
|
||||
showToast('Model '+_sentModel+' changed to '+startData.effective_model+' — profile provider mismatch', 5000);
|
||||
}
|
||||
S.session.model=startData.effective_model;
|
||||
S.session.model_provider=startData.effective_model_provider||S.session.model_provider||null;
|
||||
localStorage.setItem('hermes-webui-model', startData.effective_model);
|
||||
if(typeof _writePersistedModelState==='function') _writePersistedModelState(startData.effective_model,S.session.model_provider||null);
|
||||
if($('modelSelect')) _applyModelToDropdown(startData.effective_model, $('modelSelect'),S.session.model_provider||null);
|
||||
if(typeof syncTopbar==='function') syncTopbar();
|
||||
}else if(startData&&startData.effective_model_provider && S.session){
|
||||
S.session.model_provider=startData.effective_model_provider;
|
||||
if(typeof _writePersistedModelState==='function') _writePersistedModelState(S.session.model||'',S.session.model_provider||null);
|
||||
if($('modelSelect')&&typeof _applyModelToDropdown==='function') _applyModelToDropdown(S.session.model||'', $('modelSelect'), S.session.model_provider||null);
|
||||
if(typeof syncModelChip==='function') syncModelChip();
|
||||
if(typeof syncTopbar==='function') syncTopbar();
|
||||
}
|
||||
|
||||
if(typeof appendThinking==='function') appendThinking('',{pending:true});
|
||||
if(S.session&&typeof startData.pending_started_at==='number'){
|
||||
S.session.pending_started_at=startData.pending_started_at;
|
||||
}
|
||||
if(S.session&&S.session.session_id===activeSid){
|
||||
S.session.active_stream_id = streamId;
|
||||
}
|
||||
if(S.session&&S.session.session_id===activeSid&&typeof showLiveRunStatus==='function'){
|
||||
const _startedAt=typeof startData?.pending_started_at==='number'
|
||||
? startData.pending_started_at
|
||||
: (S.session.pending_started_at||Date.now()/1000);
|
||||
showLiveRunStatus(activeSid,{startedAt:_startedAt});
|
||||
}
|
||||
if(typeof upsertActiveSessionForLocalTurn==='function'){
|
||||
// Third optimistic pass: stream_id is now known, so the row can reconcile
|
||||
// against real active-stream metadata before the background refresh lands.
|
||||
upsertActiveSessionForLocalTurn({title:S.session&&S.session.title||displayText.slice(0,64),messageCount:S.messages.length,timestampMs:Date.now()});
|
||||
}
|
||||
if(!INFLIGHT[activeSid]){
|
||||
INFLIGHT[activeSid]={messages:optimisticMessages,uploaded:uploadedNames,toolCalls:[]};
|
||||
}
|
||||
const currentInflight=INFLIGHT[activeSid];
|
||||
markInflight(activeSid, streamId);
|
||||
if(typeof saveInflightState==='function'){
|
||||
saveInflightState(activeSid,{streamId,messages:currentInflight.messages||optimisticMessages,uploaded:uploadedNames,toolCalls:currentInflight.toolCalls||[]});
|
||||
}
|
||||
// Refresh session list so background streaming indicators appear immediately for the
|
||||
// session that was just started and any others that may already be running.
|
||||
if(typeof renderSessionList === 'function') {
|
||||
void renderSessionList();
|
||||
}
|
||||
});
|
||||
|
||||
// Open SSE stream and render tokens live
|
||||
attachLiveStream(activeSid, streamId, uploadedNames);
|
||||
|
||||
|
||||
@@ -229,27 +229,41 @@ class TestBusySendButton:
|
||||
)
|
||||
|
||||
def test_send_refreshes_primary_button_after_chat_start_stream_id(self):
|
||||
"""send() must call updateSendBtn in the chat/start try block after assigning streamId.
|
||||
"""send() must call updateSendBtn after streamId is assigned and before attachLiveStream.
|
||||
|
||||
setBusy(true) already ran updateSendBtn while activeStreamId was still null, so the
|
||||
Stop affordance did not appear until something else (e.g. typing) called
|
||||
updateSendBtn again.
|
||||
setBusy(true) runs before the API request with activeStreamId still null. The
|
||||
Stop affordance must be refreshed as soon as we have streamId so it cannot be
|
||||
skipped by optional post-start UI failures.
|
||||
"""
|
||||
send_start = MESSAGES_JS.find("async function send(")
|
||||
assert send_start >= 0, "send() not found in messages.js"
|
||||
send_end = MESSAGES_JS.find("const LIVE_STREAMS={}", send_start)
|
||||
assert send_end > send_start, "could not find end of send() body"
|
||||
send_body = MESSAGES_JS[send_start:send_end]
|
||||
api_idx = send_body.find("const startData=await api('/api/chat/start'")
|
||||
assert api_idx >= 0, "send() should issue /api/chat/start"
|
||||
catch_idx = send_body.find("}catch(e){", api_idx)
|
||||
assert catch_idx >= 0, "send() should have API error catch after /api/chat/start"
|
||||
assign = "S.activeStreamId = streamId;"
|
||||
apos = send_body.find(assign)
|
||||
assert apos >= 0, "send() must assign S.activeStreamId from startData"
|
||||
after_assign = send_body[apos:]
|
||||
end_try = after_assign.find(" }catch(e){")
|
||||
assert end_try > 0, "send() outer try/catch not found after stream id assign"
|
||||
try_after_assign = after_assign[:end_try]
|
||||
assert "updateSendBtn" in try_after_assign, (
|
||||
"send() must call updateSendBtn() in the chat/start try block after assigning "
|
||||
"streamId so the primary button switches to Stop without waiting for composer input"
|
||||
assert apos > catch_idx, (
|
||||
"send() must assign S.activeStreamId only after /api/chat/start succeeds"
|
||||
)
|
||||
update_idx = send_body.find("updateSendBtn();", apos)
|
||||
assert update_idx >= 0, "send() must call updateSendBtn() after assigning streamId"
|
||||
assert update_idx > apos, (
|
||||
"send() should call updateSendBtn() after S.activeStreamId is assigned"
|
||||
)
|
||||
attach_idx = send_body.find("attachLiveStream(activeSid, streamId, uploadedNames);")
|
||||
assert attach_idx > update_idx, (
|
||||
"send() should refresh primary button before opening SSE stream attach"
|
||||
)
|
||||
optional_idx = send_body.find("_runOptionalPostStartUiStep('post-start ui/bookkeeping'", apos)
|
||||
assert optional_idx >= 0, "send() should run optional post-start UI/bookkeeping"
|
||||
assert optional_idx > update_idx, (
|
||||
"send() must call updateSendBtn() before entering optional post-start helper so optional failures "
|
||||
"cannot skip it."
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -106,6 +106,27 @@ def test_pre_start_optimistic_block_cannot_prevent_chat_start():
|
||||
)
|
||||
|
||||
|
||||
def test_post_start_bookkeeping_errors_cannot_block_live_attach():
|
||||
"""Any optional post-start UI/bookkeeping failure should be recoverable once stream_id exists."""
|
||||
body = _function_body(MESSAGES_JS, "send")
|
||||
helper_body = _function_body(MESSAGES_JS, "_runOptionalPostStartUiStep")
|
||||
assert "optional post-start UI step failed" in helper_body, (
|
||||
"post-start optional helper failures should stay in warning logs, not user-facing error bubbles"
|
||||
)
|
||||
|
||||
chat_start_idx = body.index("const startData=await api('/api/chat/start'")
|
||||
catch_idx = body.index("}catch(e){", chat_start_idx)
|
||||
optional_idx = body.index("_runOptionalPostStartUiStep('post-start ui/bookkeeping'", catch_idx)
|
||||
stream_id_idx = body.index("streamId = postStartData ? postStartData.stream_id : null;", catch_idx)
|
||||
attach_idx = body.index("attachLiveStream(activeSid, streamId, uploadedNames);")
|
||||
assert catch_idx < stream_id_idx < optional_idx < attach_idx, (
|
||||
"stream-id setup, post-start UI/bookkeeping, and attach must run after successful API catch"
|
||||
)
|
||||
assert "S.messages.push({role:'assistant',content:`**Error:**" not in body[optional_idx : attach_idx], (
|
||||
"post-start optional failures should not append assistant error messages before stream attach"
|
||||
)
|
||||
|
||||
|
||||
def test_server_absent_optimistic_first_turn_rows_are_not_kept_forever():
|
||||
"""A local first-turn sidebar row must expire when /api/chat/start never persisted it."""
|
||||
body = _function_body(SESSIONS_JS, "_mergeOptimisticFirstTurnSessions")
|
||||
|
||||
Reference in new issue
Block a user