Fix notifications on channel changes

crates/channel/src/channel_store.rs

@@ -940,6 +940,13 @@ impl ChannelStore {
 
                 for channel_id in &payload.delete_channels {
                     let channel_id = *channel_id;
+                    if payload
+                        .channels
+                        .iter()
+                        .any(|channel| channel.id == channel_id)
+                    {
+                        continue;
+                    }
                     if let Some(OpenedModelHandle::Open(buffer)) =
                         self.opened_buffers.remove(&channel_id)
                     {

crates/collab/src/db.rs

@@ -436,6 +436,23 @@ pub struct Channel {
     pub role: ChannelRole,
 }
 
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct ChannelMember {
+    pub role: ChannelRole,
+    pub user_id: UserId,
+    pub kind: proto::channel_member::Kind,
+}
+
+impl ChannelMember {
+    pub fn to_proto(&self) -> proto::ChannelMember {
+        proto::ChannelMember {
+            role: self.role.into(),
+            user_id: self.user_id.to_proto(),
+            kind: self.kind.into(),
+        }
+    }
+}
+
 #[derive(Debug, PartialEq)]
 pub struct ChannelsForUser {
     pub channels: ChannelGraph,

crates/collab/src/db/ids.rs

@@ -114,6 +114,22 @@ impl ChannelRole {
             other
         }
     }
+
+    pub fn can_see_all_descendants(&self) -> bool {
+        use ChannelRole::*;
+        match self {
+            Admin | Member => true,
+            Guest | Banned => false,
+        }
+    }
+
+    pub fn can_only_see_public_descendants(&self) -> bool {
+        use ChannelRole::*;
+        match self {
+            Guest => true,
+            Admin | Member | Banned => false,
+        }
+    }
 }
 
 impl From<proto::ChannelRole> for ChannelRole {

crates/collab/src/db/queries/channels.rs

@@ -633,32 +633,84 @@ impl Database {
             .await
     }
 
-    pub async fn get_channel_members_and_roles(
+    pub async fn participants_to_notify_for_channel_change(
         &self,
-        id: ChannelId,
+        new_parent: ChannelId,
-    ) -> Result<Vec<(UserId, ChannelRole)>> {
+        admin_id: UserId,
+    ) -> Result<Vec<(UserId, ChannelsForUser)>> {
         self.transaction(|tx| async move {
-            #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)]
+            let mut results: Vec<(UserId, ChannelsForUser)> = Vec::new();
-            enum QueryUserIdsAndRoles {
+
-                UserId,
+            let members = self
-                Role,
+                .get_channel_participant_details_internal(new_parent, admin_id, &*tx)
+                .await?;
+
+            dbg!(&members);
+
+            for member in members.iter() {
+                if !member.role.can_see_all_descendants() {
+                    continue;
+                }
+                results.push((
+                    member.user_id,
+                    self.get_user_channels(
+                        member.user_id,
+                        vec![channel_member::Model {
+                            id: Default::default(),
+                            channel_id: new_parent,
+                            user_id: member.user_id,
+                            role: member.role,
+                            accepted: true,
+                        }],
+                        &*tx,
+                    )
+                    .await?,
+                ))
             }
 
-            let ancestor_ids = self.get_channel_ancestors(id, &*tx).await?;
+            let public_parent = self
-            let user_ids_and_roles = channel_member::Entity::find()
+                .public_path_to_channel_internal(new_parent, &*tx)
-                .distinct()
+                .await?
-                .filter(
+                .last()
-                    channel_member::Column::ChannelId
+                .copied();
-                        .is_in(ancestor_ids.iter().copied())
+
-                        .and(channel_member::Column::Accepted.eq(true)),
+            let Some(public_parent) = public_parent else {
+                return Ok(results);
+            };
+
+            // could save some time in the common case by skipping this if the
+            // new channel is not public and has no public descendants.
+            let public_members = if public_parent == new_parent {
+                members
+            } else {
+                self.get_channel_participant_details_internal(public_parent, admin_id, &*tx)
+                    .await?
+            };
+
+            dbg!(&public_members);
+
+            for member in public_members {
+                if !member.role.can_only_see_public_descendants() {
+                    continue;
+                };
+                results.push((
+                    member.user_id,
+                    self.get_user_channels(
+                        member.user_id,
+                        vec![channel_member::Model {
+                            id: Default::default(),
+                            channel_id: public_parent,
+                            user_id: member.user_id,
+                            role: member.role,
+                            accepted: true,
+                        }],
+                        &*tx,
                     )
-                .select_only()
+                    .await?,
-                .column(channel_member::Column::UserId)
+                ))
-                .column(channel_member::Column::Role)
+            }
-                .into_values::<_, QueryUserIdsAndRoles>()
+
-                .all(&*tx)
+            Ok(results)
-                .await?;
-            Ok(user_ids_and_roles)
         })
         .await
     }
@@ -696,12 +748,12 @@ impl Database {
         .await
     }
 
-    pub async fn get_channel_participant_details(
+    pub async fn get_channel_participant_details_internal(
         &self,
         channel_id: ChannelId,
         admin_id: UserId,
-    ) -> Result<Vec<proto::ChannelMember>> {
+        tx: &DatabaseTransaction,
-        self.transaction(|tx| async move {
+    ) -> Result<Vec<ChannelMember>> {
         self.check_user_is_channel_admin(channel_id, admin_id, &*tx)
             .await?;
 
@@ -739,11 +791,7 @@ impl Database {
             .stream(&*tx)
             .await?;
 
-            struct UserDetail {
+        let mut user_details: HashMap<UserId, ChannelMember> = HashMap::default();
-                kind: Kind,
-                channel_role: ChannelRole,
-            }
-            let mut user_details: HashMap<UserId, UserDetail> = HashMap::default();
 
         while let Some(user_membership) = stream.next().await {
             let (user_id, channel_role, is_direct_member, is_invite_accepted, visibility): (
@@ -768,8 +816,8 @@ impl Database {
             }
 
             if let Some(details_mut) = user_details.get_mut(&user_id) {
-                    if channel_role.should_override(details_mut.channel_role) {
+                if channel_role.should_override(details_mut.role) {
-                        details_mut.channel_role = channel_role;
+                    details_mut.role = channel_role;
                 }
                 if kind == Kind::Member {
                     details_mut.kind = kind;
@@ -779,20 +827,40 @@ impl Database {
                     details_mut.kind = kind;
                 }
             } else {
-                    user_details.insert(user_id, UserDetail { kind, channel_role });
+                user_details.insert(
+                    user_id,
+                    ChannelMember {
+                        user_id,
+                        kind,
+                        role: channel_role,
+                    },
+                );
             }
         }
 
         Ok(user_details
             .into_iter()
-                .map(|(user_id, details)| proto::ChannelMember {
+            .map(|(_, details)| details)
-                    user_id: user_id.to_proto(),
-                    kind: details.kind.into(),
-                    role: details.channel_role.into(),
-                })
             .collect())
+    }
+
+    pub async fn get_channel_participant_details(
+        &self,
+        channel_id: ChannelId,
+        admin_id: UserId,
+    ) -> Result<Vec<proto::ChannelMember>> {
+        let members = self
+            .transaction(move |tx| async move {
+                Ok(self
+                    .get_channel_participant_details_internal(channel_id, admin_id, &*tx)
+                    .await?)
             })
-        .await
+            .await?;
+
+        Ok(members
+            .into_iter()
+            .map(|channel_member| channel_member.to_proto())
+            .collect())
     }
 
     pub async fn get_channel_participants_internal(
@@ -883,6 +951,60 @@ impl Database {
         Ok(row)
     }
 
+    // ordered from higher in tree to lower
+    // only considers one path to a channel
+    // includes the channel itself
+    pub async fn path_to_channel(&self, channel_id: ChannelId) -> Result<Vec<ChannelId>> {
+        self.transaction(move |tx| async move {
+            Ok(self.path_to_channel_internal(channel_id, &*tx).await?)
+        })
+        .await
+    }
+
+    pub async fn parent_channel_id(&self, channel_id: ChannelId) -> Result<Option<ChannelId>> {
+        let path = self.path_to_channel(channel_id).await?;
+        if path.len() >= 2 {
+            Ok(Some(path[path.len() - 2]))
+        } else {
+            Ok(None)
+        }
+    }
+
+    pub async fn public_parent_channel_id(
+        &self,
+        channel_id: ChannelId,
+    ) -> Result<Option<ChannelId>> {
+        let path = self.path_to_channel(channel_id).await?;
+        if path.len() >= 2 && path.last().copied() == Some(channel_id) {
+            Ok(Some(path[path.len() - 2]))
+        } else {
+            Ok(path.last().copied())
+        }
+    }
+
+    pub async fn path_to_channel_internal(
+        &self,
+        channel_id: ChannelId,
+        tx: &DatabaseTransaction,
+    ) -> Result<Vec<ChannelId>> {
+        let arbitary_path = channel_path::Entity::find()
+            .filter(channel_path::Column::ChannelId.eq(channel_id))
+            .order_by(channel_path::Column::IdPath, sea_orm::Order::Desc)
+            .one(tx)
+            .await?;
+
+        let Some(path) = arbitary_path else {
+            return Ok(vec![]);
+        };
+
+        Ok(path
+            .id_path
+            .trim_matches('/')
+            .split('/')
+            .map(|id| ChannelId::from_proto(id.parse().unwrap()))
+            .collect())
+    }
+
     // ordered from higher in tree to lower
     // only considers one path to a channel
     // includes the channel itself
@@ -903,22 +1025,7 @@ impl Database {
         channel_id: ChannelId,
         tx: &DatabaseTransaction,
     ) -> Result<Vec<ChannelId>> {
-        let arbitary_path = channel_path::Entity::find()
+        let ancestor_ids = self.path_to_channel_internal(channel_id, &*tx).await?;
-            .filter(channel_path::Column::ChannelId.eq(channel_id))
-            .order_by(channel_path::Column::IdPath, sea_orm::Order::Desc)
-            .one(tx)
-            .await?;
-
-        let Some(path) = arbitary_path else {
-            return Ok(vec![]);
-        };
-
-        let ancestor_ids: Vec<ChannelId> = path
-            .id_path
-            .trim_matches('/')
-            .split('/')
-            .map(|id| ChannelId::from_proto(id.parse().unwrap()))
-            .collect();
 
         let rows = channel::Entity::find()
             .filter(channel::Column::Id.is_in(ancestor_ids.iter().copied()))
@@ -1044,6 +1151,27 @@ impl Database {
         Ok(channel_ids)
     }
 
+    // returns all ids of channels in the tree under this channel_id.
+    pub async fn get_channel_descendant_ids(
+        &self,
+        channel_id: ChannelId,
+    ) -> Result<HashSet<ChannelId>> {
+        self.transaction(|tx| async move {
+            let pairs = self.get_channel_descendants([channel_id], &*tx).await?;
+
+            let mut results: HashSet<ChannelId> = HashSet::default();
+            for ChannelEdge {
+                parent_id: _,
+                channel_id,
+            } in pairs
+            {
+                results.insert(ChannelId::from_proto(channel_id));
+            }
+            Ok(results)
+        })
+        .await
+    }
+
     // Returns the channel desendants as a sorted list of edges for further processing.
     // The edges are sorted such that you will see unknown channel ids as children
     // before you see them as parents.

crates/collab/src/rpc.rs

@@ -2220,26 +2220,13 @@ async fn create_channel(
         return Ok(());
     };
 
-    let members: Vec<proto::ChannelMember> = db
+    let updates = db
-        .get_channel_participant_details(parent_id, session.user_id)
+        .participants_to_notify_for_channel_change(parent_id, session.user_id)
-        .await?
+        .await?;
-        .into_iter()
-        .filter(|member| {
-            member.role() == proto::ChannelRole::Admin
-                || member.role() == proto::ChannelRole::Member
-        })
-        .collect();
-
-    let mut updates: HashMap<UserId, proto::UpdateChannels> = HashMap::default();
-
-    for member in members {
-        let user_id = UserId::from_proto(member.user_id);
-        let channels = db.get_channel_for_user(parent_id, user_id).await?;
-        updates.insert(user_id, build_initial_channels_update(channels, vec![]));
-    }
 
     let connection_pool = session.connection_pool().await;
-    for (user_id, update) in updates {
+    for (user_id, channels) in updates {
+        let update = build_initial_channels_update(channels, vec![]);
         for connection_id in connection_pool.user_connection_ids(user_id) {
             if user_id == session.user_id {
                 continue;
@@ -2353,31 +2340,55 @@ async fn set_channel_visibility(
     let channel_id = ChannelId::from_proto(request.channel_id);
     let visibility = request.visibility().into();
 
-    let channel = db
+    let previous_members = db
-        .set_channel_visibility(channel_id, visibility, session.user_id)
-        .await?;
-
-    let members = db
         .get_channel_participant_details(channel_id, session.user_id)
         .await?;
 
+    db.set_channel_visibility(channel_id, visibility, session.user_id)
+        .await?;
+
+    let mut updates: HashMap<UserId, ChannelsForUser> = db
+        .participants_to_notify_for_channel_change(channel_id, session.user_id)
+        .await?
+        .into_iter()
+        .collect();
+
+    let mut participants_who_lost_access: HashSet<UserId> = HashSet::default();
+    match visibility {
+        ChannelVisibility::Members => {
+            for member in previous_members {
+                if ChannelRole::from(member.role()).can_only_see_public_descendants() {
+                    participants_who_lost_access.insert(UserId::from_proto(member.user_id));
+                }
+            }
+        }
+        ChannelVisibility::Public => {
+            if let Some(public_parent_id) = db.public_parent_channel_id(channel_id).await? {
+                let parent_updates = db
+                    .participants_to_notify_for_channel_change(public_parent_id, session.user_id)
+                    .await?;
+
+                for (user_id, channels) in parent_updates {
+                    updates.insert(user_id, channels);
+                }
+            }
+        }
+    }
+
     let connection_pool = session.connection_pool().await;
-    // TODO: notify people who were guests and are now not allowed.
+    for (user_id, channels) in updates {
-    for member in members {
+        let update = build_initial_channels_update(channels, vec![]);
-        for connection_id in connection_pool.user_connection_ids(UserId::from_proto(member.user_id))
+        for connection_id in connection_pool.user_connection_ids(user_id) {
-        {
+            session.peer.send(connection_id, update.clone())?;
-            session.peer.send(
+        }
-                connection_id,
+    }
-                proto::UpdateChannels {
+    for user_id in participants_who_lost_access {
-                    channels: vec![proto::Channel {
+        let update = proto::UpdateChannels {
-                        id: channel.id.to_proto(),
+            delete_channels: vec![channel_id.to_proto()],
-                        name: channel.name.clone(),
-                        visibility: channel.visibility.into(),
-                        role: member.role.into(),
-                    }],
             ..Default::default()
-                },
+        };
-            )?;
+        for connection_id in connection_pool.user_connection_ids(user_id) {
+            session.peer.send(connection_id, update.clone())?;
         }
     }
 
@@ -2485,42 +2496,20 @@ async fn link_channel(
     // TODO: Remove this restriction once we have symlinks
     db.assert_root_channel(channel_id).await?;
 
-    let channels_to_send = db.link_channel(session.user_id, channel_id, to).await?;
+    db.link_channel(session.user_id, channel_id, to).await?;
-    let members = db.get_channel_members_and_roles(to).await?;
+
+    let member_updates = db
+        .participants_to_notify_for_channel_change(to, session.user_id)
+        .await?;
+
+    dbg!(&member_updates);
+
     let connection_pool = session.connection_pool().await;
 
-    for (member_id, role) in members {
+    for (member_id, channels) in member_updates {
-        let build_channel_proto = |channel: &db::Channel| proto::Channel {
+        let update = build_initial_channels_update(channels, vec![]);
-            id: channel.id.to_proto(),
-            visibility: channel.visibility.into(),
-            name: channel.name.clone(),
-            role: role.into(),
-        };
-
         for connection_id in connection_pool.user_connection_ids(member_id) {
-            let channels: Vec<_> = if role == ChannelRole::Guest {
+            session.peer.send(connection_id, update.clone())?;
-                channels_to_send
-                    .channels
-                    .iter()
-                    .filter(|channel| channel.visibility != ChannelVisibility::Public)
-                    .map(build_channel_proto)
-                    .collect()
-            } else {
-                channels_to_send
-                    .channels
-                    .iter()
-                    .map(build_channel_proto)
-                    .collect()
-            };
-
-            session.peer.send(
-                connection_id,
-                proto::UpdateChannels {
-                    channels,
-                    insert_edge: channels_to_send.edges.clone(),
-                    ..Default::default()
-                },
-            )?;
         }
     }
 
@@ -2548,11 +2537,11 @@ async fn move_channel(
     let from_parent = ChannelId::from_proto(request.from);
     let to = ChannelId::from_proto(request.to);
 
-    let from_public_parent = db
+    let previous_participants = db
-        .public_path_to_channel(from_parent)
+        .get_channel_participant_details(channel_id, session.user_id)
-        .await?
+        .await?;
-        .last()
+
-        .copied();
+    debug_assert_eq!(db.parent_channel_id(channel_id).await?, Some(from_parent));
 
     let channels_to_send = db
         .move_channel(session.user_id, channel_id, from_parent, to)
@@ -2563,68 +2552,42 @@ async fn move_channel(
         return Ok(());
     }
 
-    let to_public_parent = db.public_path_to_channel(to).await?.last().cloned();
+    let updates = db
+        .participants_to_notify_for_channel_change(to, session.user_id)
+        .await?;
 
-    let members_from = db
+    let mut participants_who_lost_access: HashSet<UserId> = HashSet::default();
-        .get_channel_participant_details(from_parent, session.user_id)
+    let mut channels_to_delete = db.get_channel_descendant_ids(channel_id).await?;
-        .await?
+    channels_to_delete.insert(channel_id);
-        .into_iter()
-        .filter(|member| {
-            member.role() == proto::ChannelRole::Admin || member.role() == proto::ChannelRole::Guest
-        });
 
-    let members_to = db
+    for previous_participant in previous_participants.iter() {
-        .get_channel_participant_details(to, session.user_id)
+        let user_id = UserId::from_proto(previous_participant.user_id);
-        .await?
+        if previous_participant.kind() == proto::channel_member::Kind::AncestorMember {
-        .into_iter()
+            participants_who_lost_access.insert(user_id);
-        .filter(|member| {
-            member.role() == proto::ChannelRole::Admin || member.role() == proto::ChannelRole::Guest
-        });
-
-    let mut updates: HashMap<UserId, proto::UpdateChannels> = HashMap::default();
-
-    for member in members_to {
-        let user_id = UserId::from_proto(member.user_id);
-        let channels = db.get_channel_for_user(to, user_id).await?;
-        updates.insert(user_id, build_initial_channels_update(channels, vec![]));
         }
-
-    if let Some(to_public_parent) = to_public_parent {
-        // only notify guests of public channels (admins/members are notified by members_to above, and banned users don't care)
-        let public_members_to = db
-            .get_channel_participant_details(to, session.user_id)
-            .await?
-            .into_iter()
-            .filter(|member| member.role() == proto::ChannelRole::Guest);
-
-        for member in public_members_to {
-            let user_id = UserId::from_proto(member.user_id);
-            if updates.contains_key(&user_id) {
-                continue;
-            }
-            let channels = db.get_channel_for_user(to_public_parent, user_id).await?;
-            updates.insert(user_id, build_initial_channels_update(channels, vec![]));
-        }
-    }
-
-    for member in members_from {
-        let user_id = UserId::from_proto(member.user_id);
-        let update = updates
-            .entry(user_id)
-            .or_insert(proto::UpdateChannels::default());
-        update.delete_edge.push(proto::ChannelEdge {
-            channel_id: channel_id.to_proto(),
-            parent_id: from_parent.to_proto(),
-        })
-    }
-
-    if let Some(_from_public_parent) = from_public_parent {
-        // TODO: for each guest member of the old public parent
-        // delete the edge that they could see (from the from_public_parent down)
     }
 
     let connection_pool = session.connection_pool().await;
-    for (user_id, update) in updates {
+    for (user_id, channels) in updates {
+        let mut update = build_initial_channels_update(channels, vec![]);
+        update.delete_channels = channels_to_delete
+            .iter()
+            .map(|channel_id| channel_id.to_proto())
+            .collect();
+        participants_who_lost_access.remove(&user_id);
+        for connection_id in connection_pool.user_connection_ids(user_id) {
+            session.peer.send(connection_id, update.clone())?;
+        }
+    }
+
+    for user_id in participants_who_lost_access {
+        let update = proto::UpdateChannels {
+            delete_channels: channels_to_delete
+                .iter()
+                .map(|channel_id| channel_id.to_proto())
+                .collect(),
+            ..Default::default()
+        };
         for connection_id in connection_pool.user_connection_ids(user_id) {
             session.peer.send(connection_id, update.clone())?;
         }

crates/collab/src/tests/channel_tests.rs

@@ -5,6 +5,7 @@ use crate::{
 use call::ActiveCall;
 use channel::{ChannelId, ChannelMembership, ChannelStore};
 use client::User;
+use futures::future::try_join_all;
 use gpui::{executor::Deterministic, ModelHandle, TestAppContext};
 use rpc::{
     proto::{self, ChannelRole},
@@ -913,6 +914,210 @@ async fn test_lost_channel_creation(
         ],
     );
 }
+
+#[gpui::test]
+async fn test_channel_link_notifications(
+    deterministic: Arc<Deterministic>,
+    cx_a: &mut TestAppContext,
+    cx_b: &mut TestAppContext,
+    cx_c: &mut TestAppContext,
+) {
+    deterministic.forbid_parking();
+
+    let mut server = TestServer::start(&deterministic).await;
+    let client_a = server.create_client(cx_a, "user_a").await;
+    let client_b = server.create_client(cx_b, "user_b").await;
+    let client_c = server.create_client(cx_c, "user_c").await;
+
+    let user_b = client_b.user_id().unwrap();
+    let user_c = client_c.user_id().unwrap();
+
+    let channels = server
+        .make_channel_tree(&[("zed", None)], (&client_a, cx_a))
+        .await;
+    let zed_channel = channels[0];
+
+    try_join_all(client_a.channel_store().update(cx_a, |channel_store, cx| {
+        [
+            channel_store.set_channel_visibility(zed_channel, proto::ChannelVisibility::Public, cx),
+            channel_store.invite_member(zed_channel, user_b, proto::ChannelRole::Member, cx),
+            channel_store.invite_member(zed_channel, user_c, proto::ChannelRole::Guest, cx),
+        ]
+    }))
+    .await
+    .unwrap();
+
+    deterministic.run_until_parked();
+
+    client_b
+        .channel_store()
+        .update(cx_b, |channel_store, _| {
+            channel_store.respond_to_channel_invite(zed_channel, true)
+        })
+        .await
+        .unwrap();
+
+    client_c
+        .channel_store()
+        .update(cx_c, |channel_store, _| {
+            channel_store.respond_to_channel_invite(zed_channel, true)
+        })
+        .await
+        .unwrap();
+
+    deterministic.run_until_parked();
+
+    // we have an admin (a), member (b) and guest (c) all part of the zed channel.
+
+    // create a new private sub-channel
+    // create a new priate channel, make it public, and move it under the previous one, and verify it shows for b and c
+    let active_channel = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.create_channel("active", Some(zed_channel), cx)
+        })
+        .await
+        .unwrap();
+
+    // the new channel shows for b and not c
+    assert_channels_list_shape(
+        client_a.channel_store(),
+        cx_a,
+        &[(zed_channel, 0), (active_channel, 1)],
+    );
+    assert_channels_list_shape(
+        client_b.channel_store(),
+        cx_b,
+        &[(zed_channel, 0), (active_channel, 1)],
+    );
+    assert_channels_list_shape(client_c.channel_store(), cx_c, &[(zed_channel, 0)]);
+
+    let vim_channel = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.create_channel("vim", None, cx)
+        })
+        .await
+        .unwrap();
+
+    client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.set_channel_visibility(vim_channel, proto::ChannelVisibility::Public, cx)
+        })
+        .await
+        .unwrap();
+
+    client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.link_channel(vim_channel, active_channel, cx)
+        })
+        .await
+        .unwrap();
+
+    deterministic.run_until_parked();
+
+    // the new channel shows for b and c
+    assert_channels_list_shape(
+        client_a.channel_store(),
+        cx_a,
+        &[(zed_channel, 0), (active_channel, 1), (vim_channel, 2)],
+    );
+    assert_channels_list_shape(
+        client_b.channel_store(),
+        cx_b,
+        &[(zed_channel, 0), (active_channel, 1), (vim_channel, 2)],
+    );
+    assert_channels_list_shape(
+        client_c.channel_store(),
+        cx_c,
+        &[(zed_channel, 0), (vim_channel, 1)],
+    );
+
+    let helix_channel = client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.create_channel("helix", None, cx)
+        })
+        .await
+        .unwrap();
+
+    client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.link_channel(helix_channel, vim_channel, cx)
+        })
+        .await
+        .unwrap();
+
+    client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.set_channel_visibility(
+                helix_channel,
+                proto::ChannelVisibility::Public,
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    // the new channel shows for b and c
+    assert_channels_list_shape(
+        client_b.channel_store(),
+        cx_b,
+        &[
+            (zed_channel, 0),
+            (active_channel, 1),
+            (vim_channel, 2),
+            (helix_channel, 3),
+        ],
+    );
+    assert_channels_list_shape(
+        client_c.channel_store(),
+        cx_c,
+        &[(zed_channel, 0), (vim_channel, 1), (helix_channel, 2)],
+    );
+
+    client_a
+        .channel_store()
+        .update(cx_a, |channel_store, cx| {
+            channel_store.set_channel_visibility(vim_channel, proto::ChannelVisibility::Members, cx)
+        })
+        .await
+        .unwrap();
+
+    // the members-only channel is still shown for c, but hidden for b
+    assert_channels_list_shape(
+        client_b.channel_store(),
+        cx_b,
+        &[
+            (zed_channel, 0),
+            (active_channel, 1),
+            (vim_channel, 2),
+            (helix_channel, 3),
+        ],
+    );
+    client_b
+        .channel_store()
+        .read_with(cx_b, |channel_store, _| {
+            assert_eq!(
+                channel_store
+                    .channel_for_id(vim_channel)
+                    .unwrap()
+                    .visibility,
+                proto::ChannelVisibility::Members
+            )
+        });
+
+    assert_channels_list_shape(
+        client_c.channel_store(),
+        cx_c,
+        &[(zed_channel, 0), (helix_channel, 1)],
+    );
+}
+
 #[gpui::test]
 async fn test_guest_access(
     deterministic: Arc<Deterministic>,