diff --git a/static/messages.js b/static/messages.js index 21b68b4a..49670a37 100644 --- a/static/messages.js +++ b/static/messages.js @@ -436,39 +436,68 @@ async function send(){ const userMsg={role:'user',content:displayText,attachments:uploaded.length?uploadedNames:undefined,_ts:Date.now()/1000}; S.toolCalls=[]; // clear tool calls from previous turn clearLiveToolCards(); // clear any leftover live cards from last turn - S.messages.push(userMsg);renderMessages();appendThinking('',{pending:true});setBusy(true); - // First optimistic pass: make the local user turn visible before /api/chat/start - // can save pending state on the server. - if(typeof upsertActiveSessionForLocalTurn==='function'){ - upsertActiveSessionForLocalTurn({title:displayText.slice(0,64),messageCount:S.messages.length,timestampMs:Date.now()}); - } - const optimisticMessages=[...S.messages]; - INFLIGHT[activeSid]={messages:optimisticMessages,uploaded:uploadedNames,toolCalls:[]}; - if(typeof saveInflightState==='function'){ - saveInflightState(activeSid,{streamId:null,messages:INFLIGHT[activeSid].messages,uploaded:uploadedNames,toolCalls:[]}); - } - if(typeof renderSessionListFromCache==='function') renderSessionListFromCache(); - startApprovalPolling(activeSid); - startClarifyPolling(activeSid); - _fetchYoloState(activeSid); // sync YOLO pill with backend state - S.activeStreamId = null; // will be set after stream starts - if(typeof updateSendBtn==='function') updateSendBtn(); - - // Set provisional title from user message immediately so session appears - // in the sidebar right away with a meaningful name. /api/chat/start persists - // the server-side provisional title and may refine this optimistic text. - if(S.session&&(S.session.title==='Untitled'||!S.session.title)){ - const provisionalTitle=displayText.slice(0,64); - applySessionTitleUpdate(activeSid, provisionalTitle, {force:true, rememberProvisional:true}); - if(typeof upsertActiveSessionForLocalTurn==='function'){ - // Second optimistic pass: carry the provisional title into the cached row - // without re-fetching /api/sessions before pending state exists server-side. - upsertActiveSessionForLocalTurn({title:provisionalTitle,messageCount:S.messages.length,timestampMs:Date.now()}); + let optimisticMessages; + try{ + S.messages.push(userMsg);renderMessages();appendThinking('',{pending:true});setBusy(true); + // First optimistic pass: make the local user turn visible before /api/chat/start + // can save pending state on the server. + _runOptionalPreStartUiStep('upsertActiveSessionForLocalTurn.initial', ()=>{ + if(typeof upsertActiveSessionForLocalTurn==='function'){ + upsertActiveSessionForLocalTurn({title:displayText.slice(0,64),messageCount:S.messages.length,timestampMs:Date.now()}); + } + }); + optimisticMessages=[...S.messages]; + INFLIGHT[activeSid]={messages:optimisticMessages,uploaded:uploadedNames,toolCalls:[]}; + if(typeof saveInflightState==='function'){ + saveInflightState(activeSid,{streamId:null,messages:INFLIGHT[activeSid].messages,uploaded:uploadedNames,toolCalls:[]}); } - } else if(typeof upsertActiveSessionForLocalTurn==='function'){ - upsertActiveSessionForLocalTurn({title:S.session&&S.session.title||displayText.slice(0,64),messageCount:S.messages.length,timestampMs:Date.now()}); - } else { - renderSessionListFromCache(); // ensure it's visible even if already titled + _runOptionalPreStartUiStep('renderSessionListFromCache.initial', ()=>{ + if(typeof renderSessionListFromCache==='function') renderSessionListFromCache(); + }); + _runOptionalPreStartUiStep('startApprovalPolling.prestart', ()=>startApprovalPolling(activeSid)); + _runOptionalPreStartUiStep('startClarifyPolling.prestart', ()=>startClarifyPolling(activeSid)); + _runOptionalPreStartUiStep('fetchYoloState.prestart', ()=>_fetchYoloState(activeSid)); // sync YOLO pill with backend state + S.activeStreamId = null; // will be set after stream starts + _runOptionalPreStartUiStep('updateSendBtn.prestart', ()=>{ + if(typeof updateSendBtn==='function') updateSendBtn(); + }); + + // Set provisional title from user message immediately so session appears + // in the sidebar right away with a meaningful name. /api/chat/start persists + // the server-side provisional title and may refine this optimistic text. + if(S.session&&(S.session.title==='Untitled'||!S.session.title)){ + const provisionalTitle=displayText.slice(0,64); + _runOptionalPreStartUiStep('applySessionTitleUpdate.provisional', ()=>{ + applySessionTitleUpdate(activeSid, provisionalTitle, {force:true, rememberProvisional:true}); + }); + _runOptionalPreStartUiStep('upsertActiveSessionForLocalTurn.provisional', ()=>{ + if(typeof upsertActiveSessionForLocalTurn==='function'){ + // Second optimistic pass: carry the provisional title into the cached row + // without re-fetching /api/sessions before pending state exists server-side. + upsertActiveSessionForLocalTurn({title:provisionalTitle,messageCount:S.messages.length,timestampMs:Date.now()}); + } + }); + } else if(typeof upsertActiveSessionForLocalTurn==='function'){ + _runOptionalPreStartUiStep('upsertActiveSessionForLocalTurn.titled', ()=>{ + upsertActiveSessionForLocalTurn({title:S.session&&S.session.title||displayText.slice(0,64),messageCount:S.messages.length,timestampMs:Date.now()}); + }); + } else { + _runOptionalPreStartUiStep('renderSessionListFromCache.prestart', ()=>{ + renderSessionListFromCache(); // ensure it's visible even if already titled + }); + } + }catch(preStartError){ + // The user turn must reach /api/chat/start even if local optimistic UI + // bookkeeping (render cache, storage quota, sidebar reconciliation, etc.) + // throws. Otherwise the pane can show a user bubble + spinner while the + // backend never receives the turn. + const message=preStartError&&preStartError.message?preStartError.message:String(preStartError||'unknown error'); + try{console.warn('[webui] pre-start optimistic UI failed; continuing to /api/chat/start', message);}catch(_){ } + if(!S.messages.includes(userMsg)) S.messages.push(userMsg); + optimisticMessages=[...S.messages]; + INFLIGHT[activeSid]={messages:optimisticMessages,uploaded:uploadedNames,toolCalls:[]}; + try{setBusy(true);}catch(_){S.busy=true;} + S.activeStreamId=null; } // Start the agent via POST, get a stream_id back diff --git a/tests/test_inflight_send_start_race.py b/tests/test_inflight_send_start_race.py index fcedbe40..731d6d2d 100644 --- a/tests/test_inflight_send_start_race.py +++ b/tests/test_inflight_send_start_race.py @@ -24,7 +24,7 @@ def _function_body(src: str, name: str) -> str: def test_send_preserves_optimistic_messages_across_chat_start_await(): """send() must not dereference INFLIGHT[activeSid] after await without a fallback.""" body = _function_body(MESSAGES_JS, "send") - setup_idx = body.index("const optimisticMessages=[...S.messages];") + setup_idx = body.index("optimisticMessages=[...S.messages];") inflight_idx = body.index("INFLIGHT[activeSid]={messages:optimisticMessages") await_idx = body.index("const startData=await api('/api/chat/start'") save_idx = body.index("saveInflightState(activeSid,{streamId", await_idx) @@ -66,6 +66,42 @@ def test_send_clears_stale_busy_state_before_queue_branch(): ) +def test_pre_start_optimistic_ui_helpers_cannot_block_chat_start(): + """Optional optimistic UI helpers must not strand a local bubble before /api/chat/start.""" + body = _function_body(MESSAGES_JS, "send") + helper_body = _function_body(MESSAGES_JS, "_runOptionalPreStartUiStep") + + optimistic_idx = body.index("S.messages.push(userMsg);renderMessages();appendThinking('',{pending:true});setBusy(true);") + chat_start_idx = body.index("api('/api/chat/start'") + pre_start = body[optimistic_idx:chat_start_idx] + + assert "try" in helper_body and "catch" in helper_body, ( + "optional pre-start UI helper wrapper must catch errors before /api/chat/start" + ) + assert "_runOptionalPreStartUiStep" in pre_start, ( + "send() should wrap optimistic sidebar/title/polling helpers before /api/chat/start" + ) + assert "upsertActiveSessionForLocalTurn" in pre_start and "applySessionTitleUpdate" in pre_start + + +def test_pre_start_optimistic_block_cannot_prevent_chat_start(): + """Any pre-start UI/storage exception must still fall through to /api/chat/start.""" + body = _function_body(MESSAGES_JS, "send") + optimistic_idx = body.index("S.messages.push(userMsg);renderMessages();appendThinking('',{pending:true});setBusy(true);") + chat_start_idx = body.index("api('/api/chat/start'") + pre_start = body[optimistic_idx:chat_start_idx] + + assert "}catch(preStartError){" in pre_start, ( + "The whole optimistic pre-start block needs a catch, not only individual optional helpers" + ) + assert "continuing to /api/chat/start" in pre_start, ( + "The recovery path should document that chat/start must still execute" + ) + assert pre_start.rindex("}catch(preStartError){") < chat_start_idx, ( + "pre-start catch must be before the /api/chat/start call" + ) + + 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")