wip

docs/protocol/channels-protocol.md

@@ -74,13 +74,17 @@ The announce is an optimisation. When it does not reach a subscriber — because
 
 ### Relay rejection
 
-**Trigger.** When a relay operator runs `APILeaveGroup` (`/leave #channel`) on a channel the relay serves, the relay's local `groups` row for the channel transitions `relay_own_status → 'rejected'` in the same transaction that flips its membership to `GSMemLeft`. The row is keyed by the channel's `relay_request_group_link` (a `ShortLinkContact`), which the relay learned when it received the original `x.grp.relay.inv`.
+When a relay operator removes the relay from a channel, the relay marks the channel as rejected and refuses future invitations from the same channel link:
 
-**Signal.** On the next `x.grp.relay.inv` from any owner for a channel link the relay has already marked `'rejected'`, the relay accepts the contact through the agent's normal async-accept path and sends `x.grp.relay.reject` over the owner-relay direct contact channel, then lets the eventual INFO event drive cleanup of its transient bookkeeping. The message is unsigned and is not part of the forwarded group message set. The payload carries a `RelayRejectionReason` — currently only `rejoin_rejected`; older relays or future reasons fall through to `RRRUnknown text` for forward compatibility.
+1. **Leave.** The relay operator runs `/leave #channel`. The relay marks the channel as rejected locally, keyed by the channel's short link.
 
-**Owner handling.** The owner's CONF handler validates that the sender is a relay member (`memberRole == GRRelay`) and that the receiver is the owner. It transitions the corresponding `GroupRelay.relayStatus` atomically `RSInvited → RSRejected`, marks the relay `GroupMember` `GSMemRejected`, and deletes the chat-layer connection. The transition is final on the owner side and is only cleared by the relay operator running `/relay allow <groupId>` (which transitions the relay's own row `'rejected' → 'inactive'` and emits no event back to the owner). The owner's subsequent user-initiated `addRelays` invocation creates a fresh `GroupRelay` row independent of the rejected one and proceeds normally — the relay's lookup will find no `'rejected'` row for the link.
+2. **Refuse.** When the owner later sends `x.grp.relay.inv` for the same channel link — typically from a re-invitation — the relay does not accept the invitation as a relay. Instead it replies with `x.grp.relay.reject` over the owner-relay direct contact channel, carrying a rejection reason. The current reason is `rejoin_rejected`; older relays or future reasons fall through to an unknown reason for forward compatibility.
 
-**Limitations.** (a) An older owner client that does not recognise `x.grp.relay.reject` parses it as `XUnknown` and falls through to the CONF catch-all, logging a parse error and leaving the relay's `GroupRelay` permanently at `RSInvited` — the same UX as a relay that never responds. (b) An older relay binary continues to write `RSInactive` on leave and does not enforce rejection at `xGrpRelayInv`. In a mixed-version deployment where some relays are old, those relays accept fresh invitations after a leave while new-binary relays reject — asymmetric behaviour that the operator can resolve by re-running `/leave` under the new binary.
+3. **Owner handling.** The owner marks the corresponding relay as rejected and notifies the operator UI. The owner's next user-initiated relay addition for the same channel creates a fresh invitation, which the relay rejects again unless the rejection has been cleared.
+
+4. **Clear.** The relay operator runs `/relay allow <groupId>` to clear the rejection for the channel. After the next user-initiated relay addition, the relay accepts the invitation and rejoins as a relay.
+
+An older owner client that does not recognise `x.grp.relay.reject` ignores the message and leaves the relay invitation in an invited state indefinitely — the same end state as a relay that does not respond. An older relay binary does not enforce rejection; in mixed-version deployments the operator can re-run `/leave` under the new binary to re-establish rejection.
 
 ### Subscriber connection
 

src/Simplex/Chat/Controller.hs

@@ -533,6 +533,7 @@ data ChatCommand
   | BlockForAll GroupName ContactName Bool
   | RemoveMembers {groupName :: GroupName, members :: NonEmpty ContactName, withMessages :: Bool}
   | LeaveGroup GroupName
+  | AllowRelayGroup GroupName
   | DeleteGroup GroupName
   | ClearGroup GroupName
   | ListMembers GroupName

src/Simplex/Chat/Library/Commands.hs

@@ -1246,7 +1246,7 @@ processChatCommand vr nm = \case
       unless canDelete $ throwChatError $ CEGroupUserRole gInfo GROwner
       -- prevent operator from accidentally clearing the rejection record
       when (relayOwnStatus gInfo == Just RSRejected) $
-        throwChatError $ CECommandError "cannot delete a rejected channel; run /_relay allow <groupId> first"
+        throwChatError $ CECommandError "cannot delete a rejected channel; run /relay allow #<channel> first"
       filesInfo <- withFastStore' $ \db -> getGroupFileInfo db user gInfo
       withGroupLock "deleteChat group" chatId $ do
         deleteCIFiles user filesInfo
@@ -2936,11 +2936,13 @@ processChatCommand vr nm = \case
       toView $ CEvtNewChatItems user [AChatItem SCTGroup SMDSnd (GroupChat gInfo' scopeInfo) ci]
       -- TODO delete direct connections that were unused
       deleteGroupLinkIfExists user gInfo'
+      let isRelayLeave = useRelays' gInfo && isRelay membership
       -- member records are not deleted to keep history
-      withFastStore' $ \db -> updateGroupMemberStatus db userId membership GSMemLeft
-      when (useRelays' gInfo && isRelay membership) $
-        withFastStore' $ \db -> updateRelayOwnStatus_ db gInfo RSRejected
-      pure $ CRLeftMemberUser user gInfo' {membership = membership {memberStatus = GSMemLeft}}
+      withFastStore' $ \db -> do
+        updateGroupMemberStatus db userId membership GSMemLeft
+        when isRelayLeave $ updateRelayOwnStatus_ db gInfo RSRejected
+      let relayOwnStatus' = if isRelayLeave then Just RSRejected else relayOwnStatus gInfo
+      pure $ CRLeftMemberUser user gInfo' {membership = membership {memberStatus = GSMemLeft}, relayOwnStatus = relayOwnStatus'}
     where
       -- Relay leaving channel: create delivery job for cursor-based sending and async connection cleanup.
       leaveChannelRelay gInfo = do
@@ -2992,6 +2994,9 @@ processChatCommand vr nm = \case
   LeaveGroup gName -> withUser $ \user -> do
     groupId <- withFastStore $ \db -> getGroupIdByName db user gName
     processChatCommand vr nm $ APILeaveGroup groupId
+  AllowRelayGroup gName -> withUser $ \user -> do
+    groupId <- withFastStore $ \db -> getGroupIdByName db user gName
+    processChatCommand vr nm $ APIAllowRelayGroup groupId
   DeleteGroup gName -> withUser $ \user -> do
     groupId <- withFastStore $ \db -> getGroupIdByName db user gName
     processChatCommand vr nm $ APIDeleteChat (ChatRef CTGroup groupId Nothing) (CDMFull True)
@@ -5040,8 +5045,8 @@ chatCommandP =
       "/xftp" $> GetUserProtoServers (AProtocolType SPXFTP),
       "/_relay test " *> (APITestChatRelay <$> A.decimal <* A.space <*> strP),
       "/relay test " *> (TestChatRelay <$> strP),
-      "/_relay allow " *> (APIAllowRelayGroup <$> A.decimal),
-      "/relay allow " *> (APIAllowRelayGroup <$> A.decimal),
+      "/_relay allow #" *> (APIAllowRelayGroup <$> A.decimal),
+      "/relay allow #" *> (AllowRelayGroup <$> displayNameP),
       "/relays " *> (SetUserChatRelays <$> chatRelaysP),
       "/relays" $> GetUserChatRelays,
       "/_operators" $> APIGetServerOperators,

src/Simplex/Chat/Library/Internal.hs

@@ -1059,10 +1059,6 @@ acceptRelayJoinRequestAsync
       ownerMember' <- getGroupMemberById db vr user groupMemberId
       pure (gInfo', ownerMember')
 
--- Asynchronous rejection of a relay invitation. Mirrors acceptGroupJoinSendRejectAsync:
--- creates a transient groups row tagged RSRejected (so the worker ignores it) with the
--- owner member in GSMemInvited so the eventual INFO arrival drives cleanup, then enqueues
--- agentAcceptContactAsync to send XGrpRelayReject through the agent's normal retry path.
 acceptRelayJoinRequestRejectAsync
   :: User
   -> Int64

src/Simplex/Chat/Library/Subscriber.hs

@@ -775,13 +775,11 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage =
                     relay <- withStore $ \db -> do
                       relay <- getGroupRelayByGMId db (groupMemberId' m)
                       liftIO $ updateRelayStatusFromTo db relay RSInvited RSRejected
-                    -- complete the contact handshake so the relay receives INFO and cleans
-                    -- up its transient bookkeeping (the new INFO arm at GCHostMember +
-                    -- RSRejected handles that), then tear down our chat-layer connection.
+                    -- complete the contact handshake so the relay receives INFO and cleans up its transient bookkeeping
                     allowAgentConnectionAsync user conn' confId XOk
                     toView $ CEvtGroupRelayUpdated user gInfo m relay
                     toViewTE $ TERelayRejected user gInfo reason
-                | otherwise -> messageError "x.grp.relay.reject: only owner can receive relay rejection"
+                | otherwise -> messageError "x.grp.relay.reject: only owner should receive relay rejection"
               _ -> messageError "CONF from invited member must have x.grp.acpt"
           GCHostMember ->
             case chatMsgEvent of
@@ -824,33 +822,20 @@ processAgentMessageConn vr user@User {userId} corrId agentConnId agentMessage =
                 pure ()
             | otherwise -> messageError "x.grp.mem.info: memberId is different from expected"
           -- sent when connecting via group link
-          XInfo _
-            | memberStatus m == GSMemRejected -> do
-                -- TODO Keep rejected member to allow them to appeal against rejection.
-                deleteMemberConnection' m True
-                withStore' $ \db -> deleteGroupMember db user m
-            | cleanupTransientRelayReject -> cleanupRelayRejectRow
-            | otherwise -> pure ()
-          XOk
-            | cleanupTransientRelayReject -> cleanupRelayRejectRow
-            | otherwise -> pure ()
+          XInfo _ ->
+            -- TODO Keep rejected member to allow them to appeal against rejection.
+            when (memberStatus m == GSMemRejected) $ do
+              deleteMemberConnection' m True
+              withStore' $ \db -> deleteGroupMember db user m
+          XOk ->
+            -- transient relay-reject row cleanup after the rejection handshake completes
+            when (memberCategory m == GCHostMember && maybe False (`elem` ([RSRejected, RSInactive] :: [RelayStatus])) (relayOwnStatus gInfo)) $ do
+              deleteMemberConnection' m True
+              withStore' $ \db -> do
+                deleteGroupMember db user m
+                deleteGroup db user gInfo
           _ -> messageError "INFO from member must have x.grp.mem.info, x.info or x.ok"
         pure ()
-        where
-          -- Transient relay-reject row cleanup. The transient row is created with
-          -- RSRejected + GCHostMember owner. The persistent /leave-time row has the
-          -- same combination but its host connection is deleted by DJRelayRemoved
-          -- before any INFO can fire, so this filter only matches transients.
-          -- RSInactive is also matched to handle the case where APIAllowRelayGroup
-          -- flipped the transient row mid-flight.
-          cleanupTransientRelayReject =
-            memberCategory m == GCHostMember
-              && maybe False (`elem` ([RSRejected, RSInactive] :: [RelayStatus])) (relayOwnStatus gInfo)
-          cleanupRelayRejectRow = do
-            deleteMemberConnection' m True
-            withStore' $ \db -> do
-              deleteGroupMember db user m
-              deleteGroup db user gInfo
       CON _pqEnc -> unless (memberStatus m == GSMemRejected || memberStatus membership == GSMemRejected) $ do
         -- TODO [knocking] send pending messages after accepting?
         -- possible improvement: check for each pending message, requires keeping track of connection state

src/Simplex/Chat/Store/Groups.hs

@@ -1526,7 +1526,7 @@ setGroupInProgressDone db GroupInfo {groupId} = do
     (currentTs, groupId)
 
 createRelayRequestGroup :: DB.Connection -> VersionRangeChat -> User -> GroupRelayInvitation -> InvitationId -> VersionRangeChat -> Int64 -> GroupMemberStatus -> RelayStatus -> ExceptT StoreError IO (GroupInfo, GroupMember)
-createRelayRequestGroup db vr user@User {userId} GroupRelayInvitation {fromMember, fromMemberProfile, relayMemberId, groupLink} invId reqChatVRange initialDelay ownerStatus relayStatus = do
+createRelayRequestGroup db vr user@User {userId} GroupRelayInvitation {fromMember, fromMemberProfile, relayMemberId, groupLink} invId reqChatVRange initialDelay memberStatus relayStatus = do
   currentTs <- liftIO getCurrentTime
   -- Create group with placeholder profile
   let Profile {displayName = fromMemberLDN} = fromMemberProfile
@@ -1543,10 +1543,10 @@ createRelayRequestGroup db vr user@User {userId} GroupRelayInvitation {fromMembe
   (groupId, _groupLDN) <- createGroup_ db userId placeholderProfile Nothing Nothing True (Just relayStatus) Nothing currentTs
   -- Store relay request data for recovery
   liftIO $ setRelayRequestData_ groupId currentTs
-  ownerMemberId <- insertOwner_ currentTs groupId ownerStatus
+  ownerMemberId <- insertOwner_ currentTs groupId
   let relayMember = MemberIdRole relayMemberId GRRelay
   -- TODO [member keys] should relays use member keys?
-  _membership <- createContactMemberInv_ db user groupId (Just ownerMemberId) user relayMember GCUserMember GSMemAccepted IBUnknown Nothing Nothing currentTs vr
+  _membership <- createContactMemberInv_ db user groupId (Just ownerMemberId) user relayMember GCUserMember memberStatus IBUnknown Nothing Nothing currentTs vr
   ownerMember <- getGroupMember db vr user groupId ownerMemberId
   g <- getGroupInfo db vr user groupId
   pure (g, ownerMember)
@@ -1565,7 +1565,7 @@ createRelayRequestGroup db vr user@User {userId} GroupRelayInvitation {fromMembe
           WHERE group_id = ?
         |]
         (Binary invId, groupLink, minVersion reqChatVRange, maxVersion reqChatVRange, initialDelay, currentTs, groupId)
-    insertOwner_ currentTs groupId ownerStatus_ = do
+    insertOwner_ currentTs groupId = do
       let MemberIdRole {memberId, memberRole} = fromMember
           VersionRange minV maxV = reqChatVRange
       (localDisplayName, profileId) <- createNewMemberProfile_ db user fromMemberProfile currentTs
@@ -1580,7 +1580,7 @@ createRelayRequestGroup db vr user@User {userId} GroupRelayInvitation {fromMembe
                 peer_chat_min_version, peer_chat_max_version)
             VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?)
           |]
-          ( (groupId, indexInGroup, memberId, memberRole, GCHostMember, ownerStatus_)
+          ( (groupId, indexInGroup, memberId, memberRole, GCHostMember, memberStatus)
               :. (userId, localDisplayName, Nothing :: (Maybe Int64), profileId, currentTs, currentTs)
               :. (minV, maxV)
           )
@@ -1598,11 +1598,8 @@ updateRelayOwnStatus_ db GroupInfo {groupId} relayStatus = do
   let inactiveAt_ = if relayStatus == RSInactive then Just currentTs else Nothing
   DB.execute db "UPDATE groups SET relay_own_status = ?, relay_inactive_at = ?, updated_at = ? WHERE group_id = ?" (relayStatus, inactiveAt_, currentTs, groupId)
 
--- Atomically flip every RSRejected row sharing the targeted group's
--- relay_request_group_link to RSInactive. Returns the refreshed GroupInfo
--- for the targeted groupId (whether it was flipped or not). The subquery
--- resolves the link in the same UPDATE statement so there is no
--- read-then-write race with concurrent xGrpRelayInv handlers.
+-- Flip every RSRejected row sharing the targeted group's relay_request_group_link
+-- to RSInactive in one statement; returns the refreshed GroupInfo for the targeted groupId.
 allowRelayGroup :: DB.Connection -> VersionRangeChat -> User -> GroupId -> ExceptT StoreError IO GroupInfo
 allowRelayGroup db vr user@User {userId} groupId = do
   currentTs <- liftIO getCurrentTime

src/Simplex/Chat/Types/Shared.hs

@@ -119,9 +119,6 @@ instance ToField RelayStatus where toField = toField . textEncode
 
 $(JQ.deriveJSON (enumJSON $ dropPrefix "RS") ''RelayStatus)
 
--- True for relay-own-status values that mean "relay not serving this group".
--- Both RSInactive (relay removed or stopped) and RSRejected (relay rejected the channel)
--- block normal message delivery; DJRelayRemoved is handled in a status-independent branch.
 relayNotServing :: Maybe RelayStatus -> Bool
 relayNotServing = \case
   Just RSInactive -> True

src/Simplex/Chat/View.hs

@@ -1437,11 +1437,7 @@ viewGroupsList gs = map groupSS $ sortOn ldn_ gs
   where
     ldn_ :: GroupInfo -> Text
     ldn_ GroupInfo {localDisplayName} = T.toLower localDisplayName
-    groupSS g@GroupInfo { membership
-                        , chatSettings = ChatSettings {enableNtfs}
-                        , groupSummary = GroupSummary {currentMembers}
-                        , relayOwnStatus
-                        } =
+    groupSS g@GroupInfo {membership, chatSettings = ChatSettings {enableNtfs}, groupSummary = GroupSummary {currentMembers}, relayOwnStatus} =
       case memberStatus membership of
         GSMemInvited -> groupInvitation' g
         s -> membershipIncognito g <> ttyFullGroup g <> viewMemberStatus s <> rejectionSuffix <> alias g

tests/ChatTests/Groups.hs

@@ -9926,7 +9926,6 @@ testChannelRemoveLeftRelay ps =
             DB.query_ db "SELECT local_display_name FROM group_members" :: IO [Only T.Text]
           danMembers2 `shouldMatchList` [Only "dan", Only "alice"]
 
--- | Read the relay_own_status column from a relay's groups table by group_id.
 queryRelayOwnStatus :: TestCC -> Int64 -> IO (Maybe T.Text)
 queryRelayOwnStatus cc gId = do
   rows <- withCCTransaction cc $ \db ->
@@ -9936,7 +9935,6 @@ queryRelayOwnStatus cc gId = do
     [Only s] -> s
     _ -> Nothing
 
--- | All (group_id, relay_own_status) rows on a relay where relay_own_status is set.
 listRelayOwnStatuses :: TestCC -> IO [(Int64, T.Text)]
 listRelayOwnStatuses cc =
   withCCTransaction cc $ \db ->
@@ -10044,7 +10042,7 @@ testRelayAllowAcceptsAgain ps =
         (cath </)
 
         -- /_relay allow flips bob's row from 'rejected' to 'inactive'
-        bob ##> "/_relay allow 1"
+        bob ##> "/relay allow #team"
         bob <## "#team: relay rejection cleared"
         bobClearStatus <- queryRelayOwnStatus bob 1
         bobClearStatus `shouldBe` Just "inactive"
@@ -10209,12 +10207,12 @@ testRelayDeleteRejectedBlocked ps =
         (cath </)
 
         bob ##> "/d #team"
-        bob <## "bad chat command: cannot delete a rejected channel; run /_relay allow <groupId> first"
+        bob <## "bad chat command: cannot delete a rejected channel; run /relay allow #<channel> first"
 
         stillRejected <- queryRelayOwnStatus bob 1
         stillRejected `shouldBe` Just "rejected"
 
-        bob ##> "/_relay allow 1"
+        bob ##> "/relay allow #team"
         bob <## "#team: relay rejection cleared"
 
         bobInactive <- queryRelayOwnStatus bob 1