fix(assistant): include messageId on canned-response message_complete events (LUM-1902)

[ashleeradka]

Summary

Quick fix for LUM-1902. The reported symptom (3-dot loading indicator stuck after initial hatch) is one instance of a broader class — five paths in conversation-routes.ts that bypass the agent loop and emit message_complete without messageId, hitting a race with the macOS streaming-buffer flush:

• canned first greeting (:1451) — the reported case, hit by every new user on hatch
• inline approval reply (:422)
• slash command output (:1774)
• /compact (:1855)
• /clean (:1855)

All five capture the persisted assistant row id and pass messageId to the broadcast. Centralized into a single emitCannedMessageComplete helper so the temporary patch is one helper + five one-line call sites, easy to grep and remove when #31994 lands (see "Relationship to #31994" below).

Why it's stuck — actual causal chain (race, not deterministic)

The bug only manifests when SSE events are delivered far enough apart for a 50 ms timer to fire between them — heavier daemon work between events (onboarding artifact persistence, telemetry, forceCompact(), forceClean(), etc.) reliably hits that window.

1. User triggers one of the canned paths → isSending = true → 3-dot indicator renders.
2. Server persists messages, returns 202, then broadcasts user_message_echo → assistant_text_delta → message_complete.
3. handleAssistantTextDelta (ChatActionHandler.swift:463):
   • sets isThinking = false (line 484)
   • buffers text into streamingDeltaBuffer and calls scheduleStreamingFlush() — a 50 ms timer (ChatViewModel.swift:916)
   • does not create the streaming bubble yet; currentAssistantMessageId stays nil until the flush fires
4. handleMessageComplete arrives. Filter at ChatActionHandler.swift:507:

   if (complete.messageId == nil || complete.source == "aux") && (vm.currentAssistantMessageId != nil || vm.isThinking) {
       return
   }

   • <50 ms gap (delta → complete): flush hasn't fired → currentAssistantMessageId == nil and isThinking == false → guard's second clause is false → guard does not fire → handler proceeds. ✓
   • ≥50 ms gap: flush has fired → currentAssistantMessageId is set → guard fires → terminal cleanup is skipped → isSending stays true until the 60-s watchdog at ChatViewModel.swift:328. ✗

No recovery on the canned paths: the normal agent loop emits assistant_activity_state{phase:idle} which runs its own cleanup and schedules a 5-s scheduleIdleFallbackCleanup() (ChatActionHandler.swift:1583). The canned paths emit neither, so the 60-s watchdog is the only safety net.

Populating messageId flips the guard's first clause to false regardless of timing, the guard no longer fires, isSending clears.

Scope decisions

Why include the other four sites: Same bug class, same trivial 2-line shape, and /compact's "everything froze right after I compacted a long chat" UX is particularly bad. Inline approval, slash, /clean are lower-frequency but identical-cost fixes. All five live in the same file, so we end up with one helper + five callers.

Why a single helper: Per [reviewer guidance], temporary fixes should be confined to easy-to-rip-out surfaces. emitCannedMessageComplete carries the full LUM-1902 context comment in one place; each call site is a one-liner. When #31994 lands, grep emitCannedMessageComplete finds every site, and the inline-then-delete is mechanical.

Why not the wake-target adapter: Same bug shape but not a quick fix. AgentEvent.message_complete (agent/loop.ts:121-124) carries { type, message } — there's no persisted id to forward at the point of relay. Needs the pre-allocated anchor pattern (same approach #31994 took for the loop path, explicitly deferred for wakes via TODO(PR-2b-followup)). Tracked in LUM-1904.

Relationship to PR #31994 (apollo workstream)

#31994 (draft) "assistant,web: pre-allocate assistant turn anchor id + stamp SSE events (PR 2b)" pre-allocates an assistantTurnId at agent-loop entry and stamps every assistant-channel event (including message_complete) with that anchor id. Its scope explicitly includes the same five conversation-routes.ts emit sites.

When #31994 lands it subsumes this PR entirely. The diff will conflict at the five call sites; resolution is mechanical (delete the helper + each emitCannedMessageComplete(...) call, take their stamped state.assistantTurnId version). I posted a heads-up on #31994.

This PR exists because #31994 is still in draft and LUM-1902 is hitting every new user on hatch — we need something we can ship on the next daemon roll.

Web impact: none

The web client's handleMessageComplete (apps/web/src/domains/chat/utils/stream-handlers/message-handlers.ts:86) has no messageId-based filter — it always calls completeTurn(). Adding messageId to these events is a no-op for web: finalizeMessageComplete (stream-message-updaters.ts:173-210) keeps tail.id when an assistant bubble is already streaming, so event.messageId is unused on these paths.

Recent area history checked

• fix(web): isSending honors activeTurnId so thinking indicator survives stale terminal events #32057 (0c1695a) fix(web): isSending honors activeTurnId so thinking indicator survives stale terminal events — adjacent web turn-store fix, doesn't touch server or macOS.
• refactor(chat): stop post-turn reconciliation; trust the anchor invariant #32024 (871b0d6) chat refactor that extracted ChatActionHandler.swift; carried the filter forward unchanged.
• [SUPERSEDED] assistant,web: pre-allocate assistant turn anchor id + stamp SSE events (PR 2b) #31994 (apollo PR 2b, draft) — broader anchor-id refactor that will subsume this fix at all five sites when it lands.
• Filter provenance: PR fix: narrow message_complete guard to only filter mid-stream auxiliary events #23075 narrowed the guard from isSending to currentAssistantMessageId; fix: extend message_complete guard to cover thinking phase #23145 extended it to cover the thinking phase; fix(macos): filter aux message_complete from streaming guard, not just sound gate #25539 added the source == "aux" clause. The messageId == nil heuristic is a load-bearing artifact from that evolution.

Risk

• Adds a previously-omitted optional field to one event type at five call sites. Both Swift (MessageComplete.messageId: String?) and TS schemas already model messageId?. Zero compatibility risk.
• No client release needed — ships on next daemon update.

Out of scope

• Wake-target adapter (wake-target-adapter.ts:130) — needs anchor-id pattern, not a quick fix. Tracked in LUM-1904.
• Removing the brittle messageId == nil heuristic from the macOS Swift filter — tracked in LUM-1904 (becomes moot post Electron cutover feat(macos): scaffold Electron desktop wrapper for apps/web #32081).
• LUM-1902 feedback bundle feat: add platform terraform for GKE deployment #2 (empty-draft "creating new chat") — confirmed mechanically correct; UX-perception issue worth a separate ticket.

Test plan

• Manual: fresh hatch → initial greeting completes → 3-dot indicator disappears immediately (not after 60-s watchdog).
• Manual: /compact on a long chat → indicator clears immediately on completion.
• Manual: regression — normal LLM turn after a canned-response path still terminates correctly.
• Manual: regression — aux events (call notifier completions) still don't stomp main turn state.
• bunx tsc --noEmit passes for assistant/.

https://claude.ai/code/session_01TtNewUqyRWwaE9eRo1QMZk

[linear]

LUM-1902

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🚩 Same missing-messageId pattern exists in other code paths

The PR fixes the missing messageId on message_complete for the canned greeting path, but the same pattern exists in at least three other code paths in this file:

• Slash command path (conversation-routes.ts:1782-1785): broadcastMessage({ type: "message_complete", conversationId }) — no messageId, and conversation.processing is still true when it fires (set to false at line 1787). The addMessage return at line 1727 is also not captured.
• Compact command path (conversation-routes.ts:1863): same pattern, processing is true until line 1878.
• Clean command path (conversation-routes.ts:1943): same pattern.

Per the PR's own comment, the macOS client filters message_complete events lacking a messageId while a turn is in flight. These paths all have conversation.processing = true when the event fires, so they could exhibit the same stuck-indicator behavior on macOS. Worth a follow-up to apply the same fix consistently.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[vex-assistant-bot]

✦ APPROVE

Value: Fixes LUM-1902 — the 3-dot stuck indicator that every new user hits on their first hatch. The race is real and the fix covers all five affected paths in one pass.

What this does: Five message_complete events emitted by canned-response paths (bypass the agent loop, so they never got an assistantTurnId) were going out without messageId. The macOS ChatActionHandler.swift:507 filter drops events that arrive after the 50ms streaming-buffer flush window — when daemon onboarding work between events crosses that threshold, isSending latches for the full 60s watchdog. Adding messageId: persistedAssistant.id gives the filter what it needs to resolve the turn.

---

Code review

Helper design is solid — one function, N one-line callers, grep anchor for when #31994 lands. The comment is thorough: explains the root cause, names the five sites, and gives a concrete removal instruction. Exactly the right shape for a temporary patch.

Each call site correctly captures addMessage's return value:

• tryConsumeCanonicalGuardianReply (:422) ✅
• canned first greeting (:1451) ✅
• slash command output (:1774) ✅
• /compact (:1855) ✅
• /clean (:1855) ✅

Type Check ✅ confirms persistedAssistant.id is type-safe across all five sites — addMessage returns a typed row with .id: string, and each capture is inside the success path (the catch block handles any throw from addMessage itself). No null-dereference risk.

No anti-pattern hits — daemon TypeScript, no React/Zustand/hooks patterns in scope.

---

CI: Type Check ✅, FlexFrame ✅, OpenAPI ✅. Test + Lint still running at review time — no red flags in the diff that would cause either to fail.

Devin's review is on pre-rebase commit e42419cb63 (no longer in the branch history) — not applicable to HEAD. Consider triggering @devin review this PR + @codex review on the current HEAD for a clean second approval.

Merge gate: CI finishing + second bot approval needed.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".