fix(relay): thread responses under original message in Slack channels

[PierreLeGuen]

Summary

• Channel mention responses now thread under the user's original message instead of posting as top-level messages
• Root cause: relay channel used event.channel_id (e.g. C088K6C3SQZ) as thread_ts fallback — Slack requires a message timestamp, not a channel ID
• Fix: use event.id (the Slack message ts) as fallback, which is the correct format for threading

Test plan

• Regression test: start_uses_message_ts_as_thread_id_for_mentions
• All 20 relay channel tests pass
• Zero clippy warnings
• Deploy and verify @ironclaw mention in channel produces threaded reply

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the relay channel to use message timestamps as thread IDs for Slack mentions, ensuring responses are correctly threaded. Feedback highlights that the current logic for setting the thread ID may inadvertently overwrite the conversation scope, potentially breaking agent history, and suggests explicitly maintaining the channel ID as the scope. Additionally, it is recommended to handle the thread_id in metadata as an optional value to avoid sending empty strings as invalid timestamps to Slack.

[gemini-code-assist]

There are two issues with this logic:

1. Conversation Continuity: with_thread overwrites the conversation_scope_id. By using event.id as the thread ID for top-level messages, each message effectively starts a new conversation scope, which breaks the agent's memory/history continuity within the channel. For top-level messages, we should thread under the message but keep the channel_id as the conversation scope.
2. Invalid Fallback: The fallback to channel_id at line 210 contradicts the PR's goal of avoiding non-timestamp thread IDs. If no valid message ID is available, we should avoid setting a thread ID entirely while still maintaining the channel scope.

                let msg = if let Some(thread_id) = event.thread_id.as_ref() {
                    msg.with_thread(thread_id)
                } else if !event.id.is_empty() {
                    msg.with_thread(&event.id).with_conversation_scope(&event.channel_id)
                } else {
                    msg.with_conversation_scope(&event.channel_id)
                };

References

1. Use .as_ref().map() on Option fields within a struct to prevent partial moves, making the code more robust against future changes.

[gemini-code-assist]

Using unwrap_or(&event.id) forces a string value in the metadata. If event.id is empty, this results in an empty string "" being stored, which causes the respond method to send an invalid thread_ts to Slack. It's better to keep this as an Option so that it serializes to null when no valid ID is available, allowing the response logic to correctly omit the threading parameter.

Suggested change

	"thread_id": event.thread_id.as_deref().unwrap_or(&event.id),
	"thread_id": event.thread_id.as_ref().map(|s| s.as_str()).or(Some(event.id.as_str()).filter(|s| !s.is_empty())),

References

1. When coercing an empty string to a value based on a JSON schema, convert it to null only if the schema allows null or does not allow string. Otherwise, preserve the empty string as it may be a valid value.
2. Use .as_ref().map() on Option fields within a struct to prevent partial moves, making the code more robust against future changes.

[zmanian]

Review -- APPROVE

Clean, correct bug fix. The root cause is right: channel mentions without an existing thread used channel_id as thread_ts, which Slack silently ignores. The fix uses event.id (message timestamp) as the fallback, which is the correct value for Slack's thread_ts parameter.

Three-way cascade is sound: event.thread_id -> event.id -> event.channel_id. Metadata thread_id updated consistently. Good regression test.

Minor note

The channel_id fallback is now only reachable if event.id is empty (shouldn't happen for valid Slack events). Consider logging a warning on that path.