fix: clear stale inflight UI state

[ai-ag2026]

Summary

• Clears stale composer busy state before starting a new send so blocked UI state does not survive failed/incomplete prior turns.
• Drops stale optimistic sidebar rows once canonical session data arrives.
• Preserves canonical idle server rows over stale optimistic local rows.
• Lets /api/chat/start still run if optional optimistic pre-start UI bookkeeping throws, and keeps those warnings console-only.
• Adds regression coverage for the inflight send/start race, stale optimistic sidebar rows, and pre-start UI helper failures.

Closes #2795.
Relates to the state-consistency umbrella #2361.

Test Plan

• git diff --check origin/master...HEAD
• node --check static/messages.js
• node --check static/sessions.js
• python3 -m pytest tests/test_inflight_send_start_race.py -q -o addopts= -> 7 passed
• Added-line hygiene scans: private/local markers 0, secret-like hits 0

Notes

Prepared from fresh nesquena/hermes-webui:master at e091e65d56fba42f350a95f3308d6d43b3627a87; latest head d2f5c9069120a9ac56585eaf44e9621ec84e93a6 adds the pre-start UI survival follow-up.

[nesquena-hermes]

Summary

Read the diff at cron-pr-2796 against origin/master and traced through static/sessions.js:2012-2086 (_isOptimisticFirstTurnSessionRow / _mergeOptimisticFirstTurnSessions) and static/messages.js:228-318 (send() entry path). Two related issues:

1. A stale client-only S.busy=true (without a runtime-side confirmation) was diverting new user turns into the invisible queue path instead of starting a fresh /api/chat/start.
2. After a failed first turn, the locally optimistic sidebar row could persist forever because the merge unconditionally appended local-only rows back into merged and unconditionally Math.max'd the local message_count over the server's confirmed-idle count.

Code reference

The new busy reconciliation guard at static/messages.js:193-216:

function _clearStaleBusyStateBeforeSend({compressionRunning=false}={}){
  if(!S||!S.busy||compressionRunning) return false;
  const session=S.session||{};
  const sid=session.session_id||'';
  const hasRuntimeConfirmation=Boolean(
    S.activeStreamId||
    session.active_stream_id||
    session.pending_user_message||
    session.pending_started_at
  );
  if(hasRuntimeConfirmation) return false;
  ...
  if(typeof setBusy==='function') setBusy(false);
  ...
}

The four hasRuntimeConfirmation conditions are the canonical "the server actually owns a turn" signals, so the guard only clears state that the runtime never confirmed. Good. Compression carve-out is right: isCompressionUiRunning() (called at line 290) is a client-only state machine and would always fail hasRuntimeConfirmation, so an explicit early-return is required.

The sidebar-merge fix at static/sessions.js:2026-2055:

const fetched=merged[idx]||{};
const fetchedIsServerIdle=_isServerIdleSessionRow(fetched);
const keepLocalOptimistic=fetchedIsServerIdle?false:_shouldKeepLocalOnlyOptimisticSessionRow(local);
...
if(!keepLocalOptimistic&&typeof _dropStaleOptimisticSessionRow==='function') _dropStaleOptimisticSessionRow(sid);
merged[idx]={
  ...local,
  ...fetched,
  title:keepLocalOptimistic?(local.title||fetched.title):fetched.title,
  message_count:keepLocalOptimistic?Math.max(localCount,fetchedCount):fetchedCount,
  ...
};

The new _shouldKeepLocalOnlyOptimisticSessionRow gate at line 2026 requires either an in-flight send (_sendInProgress && sid===_sendInProgressSid), an active session with runtime confirmation, or active+busy within 5s of last_message_at. Outside that window the local row is dropped from the merge and _dropStaleOptimisticSessionRow(sid) clears INFLIGHT, _sessionStreamingById, and clearInflightState (defined at static/ui.js:4186).

Diagnosis

This is the correct shape: the merge previously could not distinguish "user just hit send, server hasn't echoed yet" from "send failed silently, optimistic row is now stuck." The fix encodes that distinction via:

• _sendInProgress+_sendInProgressSid covers the < ~500ms window before the /api/chat/start response lands;
• S.busy && hasRuntimeConfirmation covers an active stream;
• 5-second ageMs covers the "send is mid-flight, no runtime signal yet" tail.

After 5s with no runtime confirmation, the row should drop — and it does. The is_streaming field is also gated on keepLocalOptimistic, so a stale row won't keep the sidebar dot spinning.

One subtle correctness question worth confirming: the else branch at line 2079 now drops local rows that aren't in bySid (i.e. server-absent) unless _shouldKeepLocalOnlyOptimisticSessionRow(local) returns true. The pre-fix behavior was to always push them. If a session was just created via /api/session/new but the listing fetch raced and didn't see it yet, this gate would drop it — but the gate would return true for that case because _sendInProgress && sid===_sendInProgressSid would still be true during the same paint cycle. Good.

Test plan

tests/test_inflight_send_start_race.py:51-98 is the right shape:

• test_send_clears_stale_busy_state_before_queue_branch asserts _clearStaleBusyStateBeforeSend runs before the if(S.busy||compressionRunning) branch and before api('/api/chat/start') — i.e. the reconciliation can't be moved underneath the queue gate without breaking the test.
• test_server_absent_optimistic_first_turn_rows_are_not_kept_forever asserts the keep/drop gate exists, the merged.push(...) is inside the keep branch, and _dropStaleOptimisticSessionRow clears INFLIGHT.
• test_server_idle_row_wins_over_stale_optimistic_count asserts a specific ternary shape for message_count and title — these are guard tests against future regressions where someone might restore the unconditional Math.max.

CI green 3.11/3.12/3.13. LGTM.

[nesquena-hermes]

Follow-up review — three new commits since the first pass

Re-reading the latest HEAD (d2f5c906) since my earlier review at 14:55Z. Three new commits landed, all of which address concrete edge cases worth highlighting:

46c3b902 — preserve server idle rows during optimistic merge

static/sessions.js:2062-2078: The previous merge expression keepLocalOptimistic = !fetchedIsServerIdle || _shouldKeepLocalOnlyOptimisticSessionRow(local) had a logic bug — when the server returned an idle row but the local row was still considered "keepable" by the helper (e.g. recent send-in-progress), the OR kept keepLocalOptimistic=true and the merged row inherited the local-only active_stream_id/pending_user_message/is_streaming flags. So a server-confirmed-idle session could still render as busy in the sidebar.

The fix flips to fetchedIsServerIdle ? false : _shouldKeepLocalOnlyOptimisticSessionRow(local) — when the server says idle, idle wins, period. The follow-up changes to the merged-object fields encode that hard precedence:

active_stream_id: fetchedIsServerIdle ? null : (keepLocalOptimistic ? (fetched.active_stream_id || local.active_stream_id || null) : null),
pending_user_message: fetchedIsServerIdle ? null : (keepLocalOptimistic ? (fetched.pending_user_message || local.pending_user_message || null) : null),
pending_started_at: fetchedIsServerIdle ? null : (keepLocalOptimistic ? (fetched.pending_started_at || local.pending_started_at || null) : null),
is_streaming: fetchedIsServerIdle ? false : (keepLocalOptimistic && Boolean(...)),

This is the correct shape. The server is the source of truth for "is this session active?" and the merge should never let a stale local optimistic flag override that signal.

de51d271 — let chat start survive pre-start UI errors

static/messages.js:436-503: This is the genuinely interesting one. The pre-start optimistic UI block (sidebar upsert, INFLIGHT save, title flash, polling kick-off) now lives inside a try { ... } catch(preStartError) { ... } where the catch path guarantees the user message reaches S.messages and INFLIGHT, sets busy state, and falls through to /api/chat/start.

The motivation, from the catch comment:

// 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.

This is exactly the failure mode that the inflight-recovery storage-quota fix (b2477974, v0.51.117) was bumping into. If saveInflightState or upsertActiveSessionForLocalTurn raises (quota exceeded, schema mismatch, etc.), the user expects the message to still send — the optimistic UI bits are decoration, not load-bearing. The wrapper _runOptionalPreStartUiStep at static/messages.js:219-227 swallows individual helper errors silently, and the outer try/catch(preStartError) is the belt-and-suspenders backstop.

One nit: the setBusy(true) inside the catch falls back to a raw S.busy=true assignment if setBusy itself throws (try{setBusy(true);}catch(_){S.busy=true;}). That's reasonable, but means if setBusy was throwing because of a deeper state inconsistency, you're now operating with S.busy=true and no inflight-send button state update. Probably fine for a recovery edge case, but worth knowing about.

d2f5c906 — hide nonfatal pre-start send warnings

static/messages.js:219-227: Adds _runOptionalPreStartUiStep(label, fn) and a guard test asserting the helper does NOT call setStatus('UI warning before send: ...'). The intent is right — flashing a status banner for a recoverable internal warning would scare users into thinking the message failed when /api/chat/start is about to succeed. Routing it through console.warn only is the right call.

Test coverage assessment

tests/test_inflight_send_start_race.py now has 5+ test functions pinning structural invariants:

• test_pre_start_optimistic_ui_helpers_cannot_block_chat_start — confirms the _runOptionalPreStartUiStep wrapper exists and is used before /api/chat/start.
• test_pre_start_optimistic_block_cannot_prevent_chat_start — pins the outer try { ... } catch(preStartError) { ... } and the catch-position-before-chat-start ordering.

The negative-case test asserting setStatus(\UI warning before send:` is not present in the helper body is a smart regression — without it, someone could re-add a status flash and silently re-introduce the "scary error before successful send" UX bug.

Verdict

LGTM on all three follow-up commits. The fault-tolerance shape is right: server-idle wins in sidebar merge, optimistic UI is decoration not load-bearing, and helper failures stay in the console. CI green on 3.11/3.12/3.13 per the PR description.

The compound fix here addresses a meaningfully larger class of "stuck inflight" failures than the original PR scope — between this and PR #2802, the v0.51.119 cycle should resolve a chunk of the long-tail session-state regressions reported over the last sprint.

[nesquena-hermes]

Shipped in v0.51.122 via release/stage-batch4 (#2815). All 5 sub-fixes squashed into one commit with authorship preserved. Closes #2795.