Skip to content

fix: include messageId in notifier message_complete events#23048

Merged
siddseethepalli merged 1 commit into
mainfrom
do/fix-notifier-message-ids
Apr 2, 2026
Merged

fix: include messageId in notifier message_complete events#23048
siddseethepalli merged 1 commit into
mainfrom
do/fix-notifier-message-ids

Conversation

@siddseethepalli
Copy link
Copy Markdown
Contributor

@siddseethepalli siddseethepalli commented Apr 2, 2026

Summary

  • Watch commentary, watch completion, and call question notifiers now include messageId in their message_complete SSE events
  • Fixes a race condition where the LLM Context Inspector shows "No LLM calls yet" during a live session but works after app restart
  • The msg.id from addMessage() was already available in all three paths — just not threaded through to the event

Original prompt

these 2 PRs

🤖 Generated with Claude Code


Open with Devin

Watch handler (commentary/summary) and call question notifier message_complete
events were emitted without messageId. When these fired mid-stream for the same
conversation, the client's handleMessageComplete would clear currentAssistantMessageId
without setting daemonMessageId, causing the LLM Context Inspector to show 0 calls
until the app was restarted and history reconstruction assigned the correct ID.

Thread the already-available msg.id from addMessage() into the message_complete
events for all three notifier paths that persist messages.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@siddseethepalli siddseethepalli merged commit 8b7b4c3 into main Apr 2, 2026
@siddseethepalli siddseethepalli deleted the do/fix-notifier-message-ids branch April 2, 2026 01:11
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Incomplete messageId propagation across broader codebase

This PR adds messageId to message_complete events only within conversation-notifiers.ts. There are many other message_complete emissions in conversation-process.ts:415,501,763,871,933 and handlers/recording.ts:493,546,574,607,674,714,839,971,1005 that also omit messageId. Some of those do persist messages (e.g. slash command handlers in conversation-process.ts that call addMessage). If the intent is for clients to reliably receive messageId on all persisted-message completions, those sites would need the same treatment. However, since messageId is optional on MessageComplete (message-types/messages.ts:197) and clients handle its absence gracefully, this is not a bug — just a potential follow-up for consistency.

Open in Devin Review

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant