fix(web): prevent user messages from vanishing on thread switch (#2409)

[serrrfirat]

Summary

• Fixes user messages disappearing when switching threads or on SSE reconnect ([QA] User messages disappear after typing in chat #2409)
• The root cause: loadHistory() clears the DOM and re-fetches from DB, but the agent loop hasn't persisted the user message yet (it persists after safety checks)
• Previous fix attempt (fix(gateway): persist user messages at send time (#2409) #2434) was closed because it persisted before the safety pipeline
• This fix is purely frontend — tracks optimistically-shown messages in a _pendingUserMessages map and re-injects them after loadHistory() if they're not in the DB response yet
• Pending messages are cleared when any SSE event (tool_started, stream_chunk, response) confirms the agent processed the message, or after a 60s TTL

Test plan

• Send message in thread A, switch to thread B, switch back to A — message still visible
• Send message, wait for response, switch away and back — full turn visible (no duplicate)
• Send message, kill SSE connection (dev tools), reconnect — message still visible
• Refresh page after sending — message appears once agent persists (within seconds)
• Send message with images attached — same behavior

Closes #2409

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request implements a mechanism to track and re-inject pending user messages that have not yet been persisted to the database, ensuring they remain visible during the safety-pipeline processing window. The review feedback identifies a critical race condition where clearing the entire pending message queue upon a single server event could lead to message loss if multiple messages are sent in rapid succession. Furthermore, an improvement was suggested for the message re-injection logic to use a frequency map instead of a set, correctly handling scenarios where a user sends multiple messages with identical content.

[gemini-code-assist]

Clearing all pending messages for a thread upon receiving a single agent activity event introduces a race condition. If a user sends multiple messages in quick succession, the first agent event related to the first message will cause all pending messages for that thread to be deleted, including those the agent hasn't processed yet. This can lead to the later messages vanishing if the user switches threads.

A safer approach is to rely on the logic in loadHistory() to filter out already-persisted messages, and the TTL to eventually clean up the _pendingUserMessages map. Please remove this line. The same issue exists in the tool_started and stream_chunk handlers.

[gemini-code-assist]

This line introduces a race condition that can cause message loss if a user sends multiple messages quickly. As mentioned in another comment, please remove this line and rely on the loadHistory logic to handle pending messages correctly.

[gemini-code-assist]

This line introduces a race condition that can cause message loss if a user sends multiple messages quickly. As mentioned in another comment, please remove this line and rely on the loadHistory logic to handle pending messages correctly.

[gemini-code-assist]

The current implementation for re-injecting pending messages has a bug when a user sends the same message content multiple times in quick succession. By using a Set for dbContents, you lose the frequency of each message. If a user sends "ok" and then "ok" again, and loadHistory runs after the first one is persisted, the Set will contain "ok". Both pending messages will then be considered "in the DB", and the second optimistic message will not be re-injected, causing it to vanish.

Additionally, the logic can be refactored to be more DRY by calculating the fresh pending messages once and reusing the result.

Here's a suggested implementation that addresses both points by using a frequency map for DB contents and simplifying the logic:

      // Re-inject pending user messages not yet in DB (#2409)
      const pending = _pendingUserMessages.get(currentThreadId);
      let freshPending = [];
      if (pending && pending.length > 0) {
        const now = Date.now();
        freshPending = pending.filter(p => now - p.timestamp < PENDING_MSG_TTL_MS);
        if (freshPending.length > 0) {
          const dbContentsCounts = data.turns
            .map(t => t.user_input)
            .filter(Boolean)
            .reduce((acc, content) => {
              acc[content] = (acc[content] || 0) + 1;
              return acc;
            }, {});

          for (const p of freshPending) {
            if (dbContentsCounts[p.content] > 0) {
              dbContentsCounts[p.content]--;
            } else {
              addMessage('user', p.content);
            }
          }
          _pendingUserMessages.set(currentThreadId, freshPending);
        } else {
          _pendingUserMessages.delete(currentThreadId);
        }
      }
      container.scrollTop = container.scrollHeight;
      // Show welcome card when history is empty
      if (data.turns.length === 0 && freshPending.length === 0) {
        showWelcomeCard();
      }

[serrrfirat]

Addressed review feedback in 76c62f7:

1. Removed early _pendingUserMessages.delete() from response, tool_started, and stream_chunk SSE handlers — fixes the race condition where the first server event clears all pending messages for a thread, causing later rapid-fire messages to vanish on thread switch.
2. Replaced Set-based dedup with a frequency map in loadHistory() — correctly handles duplicate-content messages (e.g. user sends "ok" twice in quick succession).
3. Simplified welcome-card guard using hoisted freshPending variable instead of re-computing the TTL check inline.

[henrypark133]

Verified finding:

crates/ironclaw_gateway/static/app.js:1245 now pushes every optimistic send into _pendingUserMessages, but the successful-turn path in the response SSE handler (app.js:850-880) never removes the entry after the turn is persisted. That means the pending map stays populated until the 60s TTL expires, so any history reload in that window can still re-inject a stale user message or keep the thread in a fake pending state. The new test_pending_message_cleared_after_response should fail against this head for the same reason.

Suggested fix: clear or decrement the matching pending entry when the turn completes, rather than relying on the TTL sweep during a later loadHistory().

[serrrfirat]

Addressed @henrypark133's review — pending entries are now cleared on turn completion instead of relying solely on TTL:

// Turn complete — remove oldest pending entry for this thread (#2409)
const _pending = _pendingUserMessages.get(data.thread_id);
if (_pending) {
  _pending.shift();
  if (_pending.length === 0) _pendingUserMessages.delete(data.thread_id);
}

Added in the response SSE handler after loadThreads(). Uses .shift() (FIFO) so only the oldest pending entry is removed per response — no race condition if the user sends multiple messages rapidly. test_pending_message_cleared_after_response should now pass.

[serrrfirat]

Medium Severity

The optimistic pending entry is not removed when /api/chat/send fails.

sendMessage() records the message in _pendingUserMessages before the POST has succeeded, but the .catch() path only marks the DOM node as failed and never removes the pending entry. If the request is rejected by rate limiting, auth expiry, or a transient network error, switching threads or reconnecting within the 60s TTL will re-inject the message from _pendingUserMessages as if it had been accepted by the agent, and the failed-send retry styling is lost.

Please track a local pending id/reference and remove that exact entry in the apiFetch(...).catch() path. A regression test should force /api/chat/send to fail, reload history, and assert the failed message is not re-injected as a normal sent message.

[serrrfirat]

Medium Severity

The dedupe counter uses a plain object keyed by raw user message content.

dbContentsCounts is initialized as {} and then indexed by content, which is user-controlled. Values such as __proto__, constructor, or toString collide with object prototype properties and can make dbContentsCounts[p.content] > 0 behave incorrectly. In those cases a message that is already present in history can still be re-injected as a duplicate pending message.

Please use new Map() or Object.create(null) for these counts, and add a regression test with a message such as __proto__ or toString.

[serrrfirat]

Medium Severity

This reconnect regression test bypasses the real send path.

The test manually calls addMessage() and pushes into _pendingUserMessages, so it can still pass if sendMessage() stops registering real optimistic sends correctly. It also calls _wait_for_connected() after connectSSE(), but that helper only checks sseHasConnectedBefore; after the initial connection this flag is already true, so the assertion does not prove a reconnect or history reload actually happened.

Please drive the real send path with a controlled delay or mocked /api/chat/send/history timing, then force loadHistory() or an actual SSE reconnect and wait on an observable reload/reconnect signal before asserting the message remains.

addressed