simplex-chat/plans/channel_message_bugs_fix_plan.md

Plan: Channel Message Bugs Fix

Table of Contents

1. Executive Summary
2. Bug 1: Delivery Context Flag
3. Bug 2: Reaction Attribution
4. Bug 3: Update Fallback Default
5. Bug 4: Forward API Parameter
6. Bug 5: CLI Forward Hardcode
7. Test Plan
8. Implementation Order

---

Executive Summary

5 bugs identified in channel message handling:

#	Location	Bug	Severity
1	Subscriber.hs:935-945	Events use isChannelOwner instead of item's showGroupAsSender	Critical
2	Subscriber.hs:1818-1842	Reactions allow m_=Nothing and fall back to membership	High
3	Subscriber.hs:1950-1969	Update fallback creates item without correct sendAsGroup flag	Medium
4	Commands.hs:930,944	Forward API ignores _sendAsGroup parameter	High
5	Commands.hs:2191,2196,2201,4633	CLI forward hardcodes False	Medium

---

Bug 1: Delivery Context Flag

Current Code (Subscriber.hs:935-945)

let isChannelOwner = useRelays' gInfo' && memberRole' m'' == GROwner
    showGroupAsSender' = case event of
      XMsgNew mc -> fromMaybe False (asGroup (mcExtMsgContent mc))
      XMsgUpdate {} -> isChannelOwner    -- BUG: should use item's flag
      XMsgDel {} -> isChannelOwner       -- BUG
      XMsgReact {} -> isChannelOwner     -- BUG
      XMsgFileDescr {} -> isChannelOwner -- BUG
      XFileCancel {} -> isChannelOwner   -- BUG
      _ -> False

Problem

Events referencing existing items (update, delete, react, file) compute showGroupAsSender' from current sender role (isChannelOwner) instead of item's stored showGroupAsSender flag.

Fix

Extract showGroupAsSender from the chat item being referenced:

showGroupAsSender' = case event of
  XMsgNew mc -> fromMaybe False (asGroup (mcExtMsgContent mc))
  XMsgUpdate {} -> itemShowGroupAsSender ci  -- from item lookup
  XMsgDel {} -> itemShowGroupAsSender ci
  XMsgReact {} -> itemShowGroupAsSender ci
  XMsgFileDescr {} -> itemShowGroupAsSender ci
  XFileCancel {} -> itemShowGroupAsSender ci
  _ -> False

Note: Use chatDir from ChatItem and pattern match on CIChannelRcv to determine sendAsGroup flag.

Files Modified

• src/Simplex/Chat/Library/Subscriber.hs: Lines 935-945

---

Bug 2: Reaction Attribution

Current Code (Subscriber.hs:1818-1842)

groupMsgReaction :: GroupInfo -> Maybe GroupMember -> SharedMsgId -> Maybe MemberId -> Maybe MsgScope -> MsgReaction -> Bool -> RcvMessage -> UTCTime -> CM (Maybe DeliveryJobScope)
groupMsgReaction g m_ sharedMsgId itemMemberId scope_ reaction add RcvMessage {msgId} brokerTs
  ...
  where
    GroupInfo {membership} = g
    reactor = fromMaybe membership m_  -- BUG (line 1842): uses membership when m_ is Nothing
    ciDir = maybe CIChannelRcv CIGroupRcv m_

Problem

When m_ is Nothing, reactor incorrectly falls back to membership (user's own member record). However, reactions should always come from an identifiable member - the m_ parameter should never be Nothing for reactions.

Fix

Reactions can only come from members (including owners), never from channels. XMsgReact handler must be reworked to require GroupMember instead of Maybe GroupMember. The m_ parameter should not be optional for reactions.

Files Modified

• src/Simplex/Chat/Library/Subscriber.hs: Lines 1818-1842

---

Bug 3: Update Fallback Default

Current Code (Subscriber.hs:1950-1969)

updateRcvChatItem `catchCINotFound` \_ -> do
  (chatDir, mentions', scopeInfo) <- case m_ of
    Just m -> ...
    Nothing -> pure (CDChannelRcv gInfo Nothing, M.empty, Nothing)  -- BUG: no sendAsGroup info
  (ci, cInfo) <- saveRcvChatItem' user chatDir msg ...

Problem

When x.msg.update arrives for a locally-deleted item in a channel (m_ is Nothing), the fallback creates a new item with CDChannelRcv gInfo Nothing but doesn't know the original item's sendAsGroup flag.

Fix (Option B: Require sender to include flag in the event)

Add asGroup field to XMsgUpdate message format.

Rationale: We don't know what owner wants otherwise - it may send as channel or it may send as owner, and different members must have the same view (e.g. when multiple relays are used, it would be random).

Files Modified

• src/Simplex/Chat/Library/Subscriber.hs: Lines 1950-1969
• Protocol message format (XMsgUpdate)

---

Bug 4: Forward API Parameter

Current Code (Commands.hs:930,944)

APIForwardChatItems ... _sendAsGroup -> withUser $ \user -> case toCType of
  CTGroup -> do
    ...
    sendGroupContentMessages user gInfo toScope (sendAsGroup' gInfo) False itemTTL cmrs'
    --                                          ^^^^^^^^^^^^^^^^^^^ BUG: ignores _sendAsGroup

Problem

The _sendAsGroup parameter is received but ignored. The function computes its own sendAsGroup' gInfo instead.

Fix

APIForwardChatItems ... sendAsGroup -> withUser $ \user -> case toCType of
  CTGroup -> do
    ...
    sendGroupContentMessages user gInfo toScope sendAsGroup False itemTTL cmrs'

Files Modified

• src/Simplex/Chat/Library/Commands.hs: Line 930 (rename parameter), Line 944 (use parameter)

---

Bug 5: CLI Forward Hardcode

Current Code (Commands.hs)

-- Line 2191
processChatCommand vr nm $ APIForwardChatItems toChatRef (ChatRef CTDirect contactId Nothing) (forwardedItemId :| []) Nothing False

-- Line 2196
processChatCommand vr nm $ APIForwardChatItems toChatRef (ChatRef CTGroup groupId Nothing) (forwardedItemId :| []) Nothing False

-- Line 2201
processChatCommand vr nm $ APIForwardChatItems toChatRef (ChatRef CTLocal folderId Nothing) (forwardedItemId :| []) Nothing False

-- Line 4633
"/_forward " *> (APIForwardChatItems <$> chatRefP <* A.space <*> chatRefP <*> _strP <*> sendMessageTTLP <*> pure False),

Problem

All CLI forward commands hardcode False for sendAsGroup instead of computing based on destination.

Fix

Compute sendAsGroup before calling API based on destination group's channel status:

-- Lines 2191, 2196, 2201: Need to determine sendAsGroup based on toChatRef
-- If toChatRef is a channel and user is owner, sendAsGroup should default to True

-- Line 4633: Parser should accept optional flag (parser cannot know context)

Files Modified

• src/Simplex/Chat/Library/Commands.hs: Lines 2191, 2196, 2201, 4633

---

Test Plan

New Tests (8 total)

Tests 1-4 cover Bug 1 (delivery context flag). Each tests a specific event type where the owner sends as member (sendAsGroup=False). Existing tests already cover the "sends as channel" (sendAsGroup=True) case; these tests verify that the delivery context correctly uses the item's stored sendAsGroup=False flag rather than recomputing from the owner's current role.

Test 1: testChannelOwnerUpdateAsMember

Objective: Verify x.msg.update uses item's sendAsGroup=False, not current role.

Scenario:

1. Owner sends message as member (sendAsGroup=False)
2. Member receives message, verify it shows as from member (not channel)
3. Owner updates message
4. Verify update delivery context uses sendAsGroup=False from the item, not recomputed from owner role

Coverage: Bug 1

---

Test 2: testChannelOwnerDeleteAsMember

Objective: Verify x.msg.del uses item's sendAsGroup=False, not current role.

Scenario:

1. Owner sends message as member (sendAsGroup=False)
2. Member receives message, verify it shows as from member (not channel)
3. Owner deletes message
4. Verify delete delivery context uses sendAsGroup=False from the item, not recomputed from owner role

Coverage: Bug 1

---

Test 3: testChannelOwnerFileTransferAsMember

Objective: Verify file delivery (including x.msg.file.descr) uses item's sendAsGroup=False, not current role.

Scenario:

1. Owner sends file as member (sendAsGroup=False)
2. Member receives file, verify it shows as from member (not channel)
3. Verify file delivery uses sendAsGroup=False from the item, not recomputed from owner role

Note: x.msg.file.descr is part of file delivery, not a separate event to test independently.

Coverage: Bug 1

---

Test 4: testChannelOwnerFileCancelAsMember

Objective: Verify x.file.cancel uses item's sendAsGroup=False, not current role.

Scenario:

1. Owner sends file as member (sendAsGroup=False)
2. Member receives file, verify it shows as from member (not channel)
3. Owner cancels file
4. Verify cancel delivery context uses sendAsGroup=False from the item, not recomputed from owner role

Coverage: Bug 1

---

Test 5: testChannelReactionAttribution

Objective: Verify reactions require a member sender (not optional).

Scenario:

1. Owner sends channel message
2. Owner adds reaction (as member, not as channel)
3. Verify reaction is attributed to owner's member record
4. Member adds reaction to channel message
5. Verify member reaction is attributed correctly
6. Verify channel cannot send reactions (m_ must be Just)

Coverage: Bug 2

---

Test 6: testChannelUpdateFallbackSendAsGroup

Objective: Verify update on deleted item creates correct sendAsGroup from protocol field.

Scenario:

1. Owner sends channel message (sendAsGroup=True)
2. Member receives and locally deletes
3. Owner updates message (XMsgUpdate includes asGroup=True)
4. Verify member's recreated item has sendAsGroup=True
5. Owner sends message as member (sendAsGroup=False)
6. Member receives and locally deletes
7. Owner updates message (XMsgUpdate includes asGroup=False)
8. Verify member's recreated item has sendAsGroup=False

Coverage: Bug 3

---

Test 7: testForwardAPIUsesParameter

Objective: Verify Forward API respects sendAsGroup parameter.

Scenario:

1. Create channel with owner
2. Forward message to channel with sendAsGroup=True
3. Verify message sent as channel
4. Forward message with sendAsGroup=False
5. Verify message sent as member

Coverage: Bug 4

---

Test 8: testForwardCLISendAsGroup

Objective: Verify CLI forward commands compute sendAsGroup correctly.

Scenario:

1. Create channel with owner
2. Use /forward to forward to channel
3. Verify sendAsGroup computed correctly (True for owner in channel)

Coverage: Bug 5

---

Implementation Order

Phase 1: Critical Fix (Bug 1)

1. Fix delivery context in Subscriber.hs
2. Add Tests 1-4 (testChannelOwnerUpdateAsMember, testChannelOwnerDeleteAsMember, testChannelOwnerFileTransferAsMember, testChannelOwnerFileCancelAsMember)

Phase 2: API Fixes (Bugs 4, 5)

1. Fix Forward API parameter usage
2. Fix CLI forward hardcodes
3. Add Tests 7 and 8 (testForwardAPIUsesParameter, testForwardCLISendAsGroup)

Phase 3: Behavior Fixes (Bugs 2, 3)

1. Rework XMsgReact handler to require GroupMember (not Maybe GroupMember)
2. Add asGroup field to XMsgUpdate protocol message
3. Add Tests 5 and 6 (testChannelReactionAttribution, testChannelUpdateFallbackSendAsGroup)

---

Files Summary

File	Changes
src/Simplex/Chat/Library/Subscriber.hs	Lines 935-945 (Bug 1), 1818-1842 (Bug 2), 1950-1969 (Bug 3)
src/Simplex/Chat/Library/Commands.hs	Lines 930,944 (Bug 4), 2191,2196,2201,4633 (Bug 5)
Protocol message types	Add asGroup field to XMsgUpdate (Bug 3)
tests/ChatTests/Groups.hs	Add 8 new tests