diff --git a/go/gen/compass/v1/comms.pb.go b/go/gen/compass/v1/comms.pb.go index 17780df37..1993eca71 100644 --- a/go/gen/compass/v1/comms.pb.go +++ b/go/gen/compass/v1/comms.pb.go @@ -2125,17 +2125,15 @@ func (x *TopicUpserted) GetTopic() *Topic { return nil } -// A channel was created, or its membership changed. On a membership change, a -// member who was removed is no longer in the channel's member_account_ids, so it -// could no longer match the stream's visibility filter — removed_account_ids -// carries exactly those departed accounts so the server can deliver each of them -// this one final event (their removal), which they would otherwise never see. -// Empty on a create or a pure add/subscribe change. +// A channel was created, or its membership, policy, pinned board, or tree +// placement changed. A subscriber the change cut off still receives this +// one final event, with its own id in removed_account_ids. type ChannelChanged struct { state protoimpl.MessageState `protogen:"open.v1"` Channel *Channel `protobuf:"bytes,1,opt,name=channel,proto3" json:"channel,omitempty"` // Accounts removed by this change (empty otherwise). Present so a departing // member receives this one last event before the channel goes silent to them. + // The server trims it per subscriber: only the recipient's own id, or empty. RemovedAccountIds []string `protobuf:"bytes,2,rep,name=removed_account_ids,json=removedAccountIds,proto3" json:"removed_account_ids,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/go/internal/comms/channel_tree_rpc_pgtest_test.go b/go/internal/comms/channel_tree_rpc_pgtest_test.go new file mode 100644 index 000000000..1e79610ed --- /dev/null +++ b/go/internal/comms/channel_tree_rpc_pgtest_test.go @@ -0,0 +1,266 @@ +//go:build pgtest + +package comms + +import ( + "context" + "slices" + "testing" + "time" + + "connectrpc.com/connect" + + "github.com/RigelBuild/compass/go/events" + compassv1 "github.com/RigelBuild/compass/go/gen/compass/v1" + "github.com/RigelBuild/compass/go/internal/store" +) + +func textPost(channelID, text string) *compassv1.PostMessageRequest { + return &compassv1.PostMessageRequest{ + Container: &compassv1.PostMessageRequest_ChannelId{ChannelId: channelID}, + Topic: &compassv1.PostMessageRequest_TopicName{TopicName: "general"}, + CreateTopic: true, + Blocks: []*compassv1.MessageBlock{{Block: &compassv1.MessageBlock_Text{Text: text}}}, + } +} + +// channelEventsBefore counts ChannelChanged events for channelID on the bus +// until the marker channel's event arrives, so the count is bounded by order. +func channelEventsBefore(t *testing.T, live <-chan events.Stamped[*compassv1.SubscribeCommsResponse], channelID, markerID string) int { + t.Helper() + n := 0 + for { + select { + case event := <-live: + switch event.Payload.GetChannelChanged().GetChannel().GetId() { + case channelID: + n++ + case markerID: + return n + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for the marker event") + return n + } + } +} + +func TestCreateChannelUnderAgentEmitsAttachedChannel(t *testing.T) { + h := newStreamHarness(t) + ctx := context.Background() + owner := mustUser(t, h.store, "tree-owner") + anchor := mustAgent(t, h.store, owner.ID, "anchor") + leaf := mustChildAgent(t, h.store, owner.ID, "leaf", anchor.ID) + outsider := mustUser(t, h.store, "tree-outsider") + + events := firstEventAfterBoundary(t, h, owner.ID, &compassv1.SubscribeCommsRequest{SinceSeq: 0}) + sub, err := h.bus.Subscribe(0, 0) + if err != nil { + t.Fatalf("subscribe event bus: %v", err) + } + defer sub.Cancel() + created, err := h.svc.CreateChannel(WithActor(ctx, owner.ID), connect.NewRequest(&compassv1.CreateChannelRequest{ + Name: "tree-room", Kind: compassv1.ChannelKind_CHANNEL_KIND_CHANNEL, + ParentAgentHandle: "anchor", MembershipMode: compassv1.ChannelMembershipMode_CHANNEL_MEMBERSHIP_MODE_TREE, + })) + if err != nil { + t.Fatalf("CreateChannel(TREE under anchor): %v", err) + } + wire := created.Msg.GetChannel() + if wire.GetParentAgentId() != string(anchor.ID) || wire.GetMembershipMode() != compassv1.ChannelMembershipMode_CHANNEL_MEMBERSHIP_MODE_TREE { + t.Fatalf("CreateChannel response parent=%q mode=%v; want %q TREE", wire.GetParentAgentId(), wire.GetMembershipMode(), anchor.ID) + } + changed := awaitFirst(t, events).GetChannelChanged() + if changed.GetChannel().GetId() != wire.GetId() || changed.GetChannel().GetParentAgentId() != string(anchor.ID) { + t.Fatalf("ChannelChanged = %v; want the attached channel %q under %q", changed, wire.GetId(), anchor.ID) + } + marker, err := h.svc.CreateChannel(WithActor(ctx, outsider.ID), connect.NewRequest(&compassv1.CreateChannelRequest{ + Name: "create-marker", Kind: compassv1.ChannelKind_CHANNEL_KIND_CHANNEL, + })) + if err != nil { + t.Fatalf("CreateChannel(marker): %v", err) + } + if n := channelEventsBefore(t, sub.Live, wire.GetId(), marker.Msg.GetChannel().GetId()); n != 1 { + t.Fatalf("ChannelChanged events for the created channel = %d; want exactly one", n) + } + + if _, err := h.svc.PostMessage(WithActor(ctx, leaf.ID), connect.NewRequest(textPost(wire.GetId(), "from the subtree"))); err != nil { + t.Fatalf("PostMessage by subtree agent: %v", err) + } + _, err = h.svc.PostMessage(WithActor(ctx, outsider.ID), connect.NewRequest(textPost(wire.GetId(), "from outside"))) + connectCodeIs(t, err, connect.CodeNotFound, "PostMessage by non-participant") +} + +func TestCreateChannelRejectsUnknownMembershipModeAndForeignAnchor(t *testing.T) { + svc, st := newHandler(t) + ctx := context.Background() + owner := mustUser(t, st, "mode-owner") + mustAgent(t, st, owner.ID, "anchor") + other := mustUser(t, st, "mode-other") + mustAgent(t, st, other.ID, "foreign") + + _, err := svc.CreateChannel(WithActor(ctx, owner.ID), connect.NewRequest(&compassv1.CreateChannelRequest{ + Name: "bad-mode", Kind: compassv1.ChannelKind_CHANNEL_KIND_CHANNEL, + ParentAgentHandle: "anchor", MembershipMode: compassv1.ChannelMembershipMode(7), + })) + connectCodeIs(t, err, connect.CodeInvalidArgument, "unknown membership mode") + + _, err = svc.CreateChannel(WithActor(ctx, owner.ID), connect.NewRequest(&compassv1.CreateChannelRequest{ + Name: "foreign-anchor", Kind: compassv1.ChannelKind_CHANNEL_KIND_CHANNEL, + ParentAgentHandle: "mode-other/foreign", MembershipMode: compassv1.ChannelMembershipMode_CHANNEL_MEMBERSHIP_MODE_TREE, + })) + connectNotFoundFor(t, err, "mode-other/foreign", "foreign anchor") +} + +func TestReparentChannelEmitsOneChannelChangedToAdmittedAccounts(t *testing.T) { + h := newStreamHarness(t) + ctx := context.Background() + owner := mustUser(t, h.store, "move-owner") + from := mustAgent(t, h.store, owner.ID, "from") + to := mustAgent(t, h.store, owner.ID, "to") + outsider := mustUser(t, h.store, "move-outsider") + channel, err := h.store.CreateChannel(ctx, owner.ID, store.NewChannel{ + Name: "moving", Kind: store.ChannelKindChannel, + ParentAgentID: from.ID, MembershipMode: store.ChannelMembershipModeTree, + }) + if err != nil { + t.Fatalf("CreateChannel: %v", err) + } + + ownerEvents := firstEventAfterBoundary(t, h, owner.ID, &compassv1.SubscribeCommsRequest{SinceSeq: 0}) + outsiderEvents := firstEventAfterBoundary(t, h, outsider.ID, &compassv1.SubscribeCommsRequest{SinceSeq: 0}) + sub, err := h.bus.Subscribe(0, 0) + if err != nil { + t.Fatalf("subscribe event bus: %v", err) + } + defer sub.Cancel() + + resp, err := h.svc.ReparentChannel(WithActor(ctx, owner.ID), connect.NewRequest(&compassv1.ReparentChannelRequest{ + ChannelId: string(channel.ID), NewParentAgentHandle: "to", + })) + if err != nil { + t.Fatalf("ReparentChannel: %v", err) + } + if got := resp.Msg.GetChannel().GetParentAgentId(); got != string(to.ID) { + t.Fatalf("ReparentChannel response parent = %q; want %q", got, to.ID) + } + changed := awaitFirst(t, ownerEvents).GetChannelChanged() + if changed.GetChannel().GetId() != string(channel.ID) || changed.GetChannel().GetParentAgentId() != string(to.ID) { + t.Fatalf("owner ChannelChanged = %v; want %q under %q", changed, channel.ID, to.ID) + } + + // The outsider's own create is a marker: arriving first proves the move + // event was filtered rather than merely slow. + marker, err := h.svc.CreateChannel(WithActor(ctx, outsider.ID), connect.NewRequest(&compassv1.CreateChannelRequest{ + Name: "outsider-marker", Kind: compassv1.ChannelKind_CHANNEL_KIND_CHANNEL, + })) + if err != nil { + t.Fatalf("CreateChannel(marker): %v", err) + } + if got := awaitFirst(t, outsiderEvents).GetChannelChanged().GetChannel().GetId(); got != marker.Msg.GetChannel().GetId() { + t.Fatalf("outsider first event channel = %q; want its marker %q, not the moved channel", got, marker.Msg.GetChannel().GetId()) + } + + if moves := channelEventsBefore(t, sub.Live, string(channel.ID), marker.Msg.GetChannel().GetId()); moves != 1 { + t.Fatalf("ChannelChanged events for the moved channel = %d; want exactly one", moves) + } +} + +func TestReparentChannelNonParticipantIsNotFound(t *testing.T) { + svc, st := newHandler(t) + ctx := context.Background() + owner := mustUser(t, st, "np-owner") + anchor := mustAgent(t, st, owner.ID, "anchor") + mustAgent(t, st, owner.ID, "dest") + outsider := mustUser(t, st, "np-outsider") + channel, err := st.CreateChannel(ctx, owner.ID, store.NewChannel{ + Name: "private", Kind: store.ChannelKindChannel, + ParentAgentID: anchor.ID, MembershipMode: store.ChannelMembershipModeTree, + }) + if err != nil { + t.Fatalf("CreateChannel: %v", err) + } + _, err = svc.ReparentChannel(WithActor(ctx, outsider.ID), connect.NewRequest(&compassv1.ReparentChannelRequest{ + ChannelId: string(channel.ID), + })) + connectCodeIs(t, err, connect.CodeNotFound, "non-participant ReparentChannel") + + other := mustUser(t, st, "np-other") + mustAgent(t, st, other.ID, "theirs") + for _, handle := range []string{"np-other/theirs", "ghost"} { + _, err := svc.ReparentChannel(WithActor(ctx, owner.ID), connect.NewRequest(&compassv1.ReparentChannelRequest{ + ChannelId: string(channel.ID), NewParentAgentHandle: handle, + })) + connectNotFoundFor(t, err, handle, "ReparentChannel destination "+handle) + } +} + +// Detaching an EXPLICIT channel removes the anchor owner set's visibility; the +// final ChannelChanged must still reach a same-owner viewer with no member row. +func TestReparentChannelDetachNotifiesViewersWhoLoseVisibility(t *testing.T) { + h := newStreamHarness(t) + ctx := context.Background() + owner := mustUser(t, h.store, "detach-owner") + anchor := mustAgent(t, h.store, owner.ID, "anchor") + viewer := mustAgent(t, h.store, owner.ID, "viewer") + channel, err := h.store.CreateChannel(ctx, anchor.ID, store.NewChannel{ + Name: "detaching", Kind: store.ChannelKindChannel, ParentAgentID: anchor.ID, + }) + if err != nil { + t.Fatalf("CreateChannel(EXPLICIT under anchor): %v", err) + } + if visible, err := h.store.ChannelVisibleTo(ctx, viewer.ID, channel.ID); err != nil || !visible { + t.Fatalf("pre-detach ChannelVisibleTo(viewer) = %v, %v; want visible via the owner set", visible, err) + } + + events := firstEventAfterBoundary(t, h, viewer.ID, &compassv1.SubscribeCommsRequest{SinceSeq: 0}) + if _, err := h.svc.ReparentChannel(WithActor(ctx, anchor.ID), connect.NewRequest(&compassv1.ReparentChannelRequest{ + ChannelId: string(channel.ID), + })); err != nil { + t.Fatalf("ReparentChannel(detach): %v", err) + } + changed := awaitFirst(t, events).GetChannelChanged() + if changed.GetChannel().GetId() != string(channel.ID) || !slices.Contains(changed.GetRemovedAccountIds(), string(viewer.ID)) { + t.Fatalf("viewer ChannelChanged = %v; want the detached channel naming the viewer as removed", changed) + } + if slices.Contains(changed.GetRemovedAccountIds(), string(anchor.ID)) { + t.Fatalf("removed = %v; the anchor is still a member and must not be named", changed.GetRemovedAccountIds()) + } +} + +// Moving a shared EXPLICIT channel from X's agent to Y's agent drops X's +// non-member agents from visibility. They still get the final event, and a +// Y-side member does not learn X's agent ids from removed_account_ids. +func TestReparentChannelCrossOwnerMoveNotifiesOnlyLostViewers(t *testing.T) { + h := newStreamHarness(t) + ctx := context.Background() + ownerX := mustUser(t, h.store, "xowner") + ownerY := mustUser(t, h.store, "yowner") + anchorX := mustAgent(t, h.store, ownerX.ID, "anchorx") + bystanderX := mustAgent(t, h.store, ownerX.ID, "bystanderx") + memberY := mustAgent(t, h.store, ownerY.ID, "membery") + channel, err := h.store.CreateChannel(ctx, anchorX.ID, store.NewChannel{ + Name: "shared", Kind: store.ChannelKindChannel, ParentAgentID: anchorX.ID, + MemberAccountIDs: []store.AccountID{memberY.ID}, + }) + if err != nil { + t.Fatalf("CreateChannel(shared under X): %v", err) + } + + bystanderEvents := firstEventAfterBoundary(t, h, bystanderX.ID, &compassv1.SubscribeCommsRequest{SinceSeq: 0}) + memberEvents := firstEventAfterBoundary(t, h, memberY.ID, &compassv1.SubscribeCommsRequest{SinceSeq: 0}) + if _, err := h.svc.ReparentChannel(WithActor(ctx, memberY.ID), connect.NewRequest(&compassv1.ReparentChannelRequest{ + ChannelId: string(channel.ID), NewParentAgentHandle: "membery", + })); err != nil { + t.Fatalf("ReparentChannel(to Y's agent): %v", err) + } + + lost := awaitFirst(t, bystanderEvents).GetChannelChanged() + if lost.GetChannel().GetId() != string(channel.ID) || !slices.Equal(lost.GetRemovedAccountIds(), []string{string(bystanderX.ID)}) { + t.Fatalf("X bystander ChannelChanged = %v; want the moved channel naming only itself as removed", lost) + } + kept := awaitFirst(t, memberEvents).GetChannelChanged() + if kept.GetChannel().GetId() != string(channel.ID) || len(kept.GetRemovedAccountIds()) != 0 { + t.Fatalf("Y member ChannelChanged = %v; want the moved channel with no removed ids", kept) + } +} diff --git a/go/internal/comms/comms.go b/go/internal/comms/comms.go index f7fe5fce0..5b8536cf4 100644 --- a/go/internal/comms/comms.go +++ b/go/internal/comms/comms.go @@ -233,8 +233,9 @@ func (c *Comms) ListChannels( return connect.NewResponse(&compassv1.ListChannelsResponse{Channels: out}), nil } -// CreateChannel creates a channel within a group, caller-authorized against the -// parent group; emits ChannelChanged (additive RPC, this task). +// CreateChannel creates a channel in a group or under an agent of the caller's +// owner, explicit or tree-membered. A foreign anchor reads as unknown, and the +// ChannelChanged is emitted after commit. func (c *Comms) CreateChannel( ctx context.Context, req *connect.Request[compassv1.CreateChannelRequest], @@ -247,11 +248,21 @@ func (c *Comms) CreateChannel( if err != nil { return nil, edgeError(err) } + mode, err := channelMembershipModeFromWire(req.Msg.GetMembershipMode()) + if err != nil { + return nil, edgeError(err) + } + parentID, err := c.resolveSameOwnerAgent(ctx, caller, req.Msg.GetParentAgentHandle()) + if err != nil { + return nil, edgeError(err) + } ch, err := c.store.CreateChannel(ctx, caller, store.NewChannel{ Name: req.Msg.GetName(), GroupID: store.ChannelGroupID(req.Msg.GetGroupId()), Kind: channelKindFromWire(req.Msg.GetKind()), MemberAccountIDs: members, + ParentAgentID: parentID, + MembershipMode: mode, }) if err != nil { return nil, edgeError(err) @@ -308,28 +319,11 @@ func (c *Comms) ReparentAgent( if err != nil { return nil, edgeError(err) } - // new_parent_handle empty ⇒ promote to root (no parent to resolve). - var newParentID store.AccountID - if h := req.Msg.GetNewParentHandle(); h != "" { - newParentID, err = c.resolveAgentHandle(ctx, caller, h) - if err != nil { - return nil, edgeError(err) - } - // Oracle-safe remap (DL-269), mirroring CreateAgent's parent pre-check: - // resolve the caller's owner and reject a foreign parent HERE, naming the - // SUBMITTED new_parent_handle — otherwise the store's clause-1 error re-keys - // to the AGENT handle and leaks the parent's existence. - owner, err := c.store.ResolveOwner(ctx, caller) - if err != nil { - return nil, edgeError(err) - } - parentOwner, err := c.store.AgentOwner(ctx, newParentID) - if err != nil { - return nil, edgeError(notFoundHandle(err, h)) - } - if parentOwner != owner { - return nil, edgeError(notFoundHandle(store.ErrNotFound, h)) - } + // new_parent_handle empty ⇒ promote to root. A foreign parent is refused + // here, naming the SUBMITTED handle; the store's error would name the agent. + newParentID, err := c.resolveSameOwnerAgent(ctx, caller, req.Msg.GetNewParentHandle()) + if err != nil { + return nil, edgeError(err) } acc, err := c.store.ReparentAgent( ctx, @@ -352,13 +346,25 @@ func (c *Comms) ReparentAgent( return connect.NewResponse(&compassv1.ReparentAgentResponse{Account: accountToWire(acc)}), nil } -// ReparentChannel lands proto-first: the store invariants and handler body -// arrive with the channel-attach store work, so it is Unimplemented until then. +// ReparentChannel attaches a channel under an agent, moves it, or (EXPLICIT +// only) detaches it to the root; emits ChannelChanged post-commit. The store +// gates participation before any shape refusal, so a refusal reveals nothing. func (c *Comms) ReparentChannel( - _ context.Context, - _ *connect.Request[compassv1.ReparentChannelRequest], + ctx context.Context, + req *connect.Request[compassv1.ReparentChannelRequest], ) (*connect.Response[compassv1.ReparentChannelResponse], error) { - return nil, connect.NewError(connect.CodeUnimplemented, errors.New("comms ReparentChannel: not implemented")) + caller := c.actorFromContext(ctx) + parentID, err := c.resolveSameOwnerAgent(ctx, caller, req.Msg.GetNewParentAgentHandle()) + if err != nil { + return nil, edgeError(err) + } + channelID := store.ChannelID(req.Msg.GetChannelId()) + ch, priorParent, err := c.store.ReparentChannel(ctx, caller, channelID, parentID) + if err != nil { + return nil, edgeError(err) + } + c.publishChannelChanged(ch, c.lostAnchorViewers(ctx, priorParent, ch.ParentAgentID, channelID)) + return connect.NewResponse(&compassv1.ReparentChannelResponse{Channel: channelToWire(ch)}), nil } // ---- agent workspace RPC (D5) ---- @@ -764,3 +770,34 @@ func (c *Comms) actorFromContext(ctx context.Context) store.AccountID { } return c.adminID } + +// lostAnchorViewers names the old anchor's owner set that lost sight of the +// channel when it left that owner's tree (a detach, or a move to another +// owner's agent), so the stream still delivers their final ChannelChanged. +// A read failure only costs that event. +func (c *Comms) lostAnchorViewers(ctx context.Context, oldAnchor, newAnchor store.AccountID, channelID store.ChannelID) []store.AccountID { + if oldAnchor == "" || oldAnchor == newAnchor { + return nil + } + oldOwner, err := c.store.AgentOwner(ctx, oldAnchor) + if err != nil { + slog.WarnContext(ctx, "comms: resolve previous anchor owner", "channel", channelID, "err", err) + return nil + } + if newAnchor != "" { + newOwner, err := c.store.AgentOwner(ctx, newAnchor) + if err != nil { + slog.WarnContext(ctx, "comms: resolve new anchor owner", "channel", channelID, "err", err) + return nil + } + if newOwner == oldOwner { + return nil + } + } + lost, err := c.store.OwnerSetLostChannelVisibility(ctx, oldOwner, channelID) + if err != nil { + slog.WarnContext(ctx, "comms: list viewers who lost the channel", "channel", channelID, "err", err) + return nil + } + return lost +} diff --git a/go/internal/comms/mapping.go b/go/internal/comms/mapping.go index c1cd4b120..5af8380d9 100644 --- a/go/internal/comms/mapping.go +++ b/go/internal/comms/mapping.go @@ -2,6 +2,7 @@ package comms import ( "context" + "fmt" "log/slog" "time" @@ -88,6 +89,20 @@ func channelMembershipModeToWire(m store.ChannelMembershipMode) compassv1.Channe } return compassv1.ChannelMembershipMode_CHANNEL_MEMBERSHIP_MODE_EXPLICIT } + +// channelMembershipModeFromWire rejects an unknown mode instead of silently +// creating an EXPLICIT channel the caller did not ask for. +func channelMembershipModeFromWire(m compassv1.ChannelMembershipMode) (store.ChannelMembershipMode, error) { + switch m { + case compassv1.ChannelMembershipMode_CHANNEL_MEMBERSHIP_MODE_EXPLICIT: + return store.ChannelMembershipModeExplicit, nil + case compassv1.ChannelMembershipMode_CHANNEL_MEMBERSHIP_MODE_TREE: + return store.ChannelMembershipModeTree, nil + default: + return 0, fmt.Errorf("%w: unknown membership mode %d", store.ErrInvalidArgument, m) + } +} + func channelKindToWire(k store.ChannelKind) compassv1.ChannelKind { switch k { case store.ChannelKindDM: diff --git a/go/internal/comms/resolve.go b/go/internal/comms/resolve.go index 043b17f83..fdc7d22c6 100644 --- a/go/internal/comms/resolve.go +++ b/go/internal/comms/resolve.go @@ -137,3 +137,28 @@ func notFoundHandle(err error, handle string) error { } return err } + +// resolveSameOwnerAgent resolves an optional agent handle that must share the +// caller's owner. A foreign agent gets the same NOT_FOUND as an unknown one, +// naming the submitted handle, as ReparentAgent does for its parent. +func (c *Comms) resolveSameOwnerAgent(ctx context.Context, caller store.AccountID, handle string) (store.AccountID, error) { + if handle == "" { + return "", nil + } + agentID, err := c.resolveAgentHandle(ctx, caller, handle) + if err != nil { + return "", err + } + owner, err := c.store.ResolveOwner(ctx, caller) + if err != nil { + return "", err + } + agentOwner, err := c.store.AgentOwner(ctx, agentID) + if err != nil { + return "", notFoundHandle(err, handle) + } + if agentOwner != owner { + return "", notFoundHandle(store.ErrNotFound, handle) + } + return agentID, nil +} diff --git a/go/internal/comms/subscribe.go b/go/internal/comms/subscribe.go index 31dc2cf0d..e4e3a1e9e 100644 --- a/go/internal/comms/subscribe.go +++ b/go/internal/comms/subscribe.go @@ -4,6 +4,7 @@ import ( "context" "errors" "log/slog" + "slices" "connectrpc.com/connect" @@ -125,7 +126,7 @@ func forwardComms( // never a fault. Expressed as ok = (send succeeded) so the client-side // end is not a `return nil` on a non-nil error (which is a real hang-up, // deliberately swallowed, not a fault to surface). - return nil, stream.Send(commsToResponse(event)) == nil + return nil, stream.Send(trimRemovedToActor(commsToResponse(event), actor)) == nil } for _, event := range sub.Replay { @@ -180,6 +181,24 @@ func commsToResponse(event events.Stamped[*compassv1.SubscribeCommsResponse]) *c } } +// trimRemovedToActor keeps only the recipient's own id in removed_account_ids: +// the list exists to deliver each departed account its final event, and the +// other ids may name accounts the recipient could never see. +func trimRemovedToActor(resp *compassv1.SubscribeCommsResponse, actor store.AccountID) *compassv1.SubscribeCommsResponse { + cc := resp.GetChannelChanged() + if cc == nil || len(cc.GetRemovedAccountIds()) == 0 { + return resp + } + var own []string + if slices.Contains(cc.GetRemovedAccountIds(), string(actor)) { + own = []string{string(actor)} + } + resp.Payload = &compassv1.SubscribeCommsResponse_ChannelChanged{ + ChannelChanged: &compassv1.ChannelChanged{Channel: cc.GetChannel(), RemovedAccountIds: own}, + } + return resp +} + // commsResyncRequired is the typed resync signal: the last event the server // sends before closing a stream whose cursor it can no longer serve gap-free. func commsResyncRequired(instanceEpoch uint64) *compassv1.SubscribeCommsResponse { diff --git a/go/internal/store/channel_tree_pgtest_test.go b/go/internal/store/channel_tree_pgtest_test.go index 9d68a7176..bef4d5b2d 100644 --- a/go/internal/store/channel_tree_pgtest_test.go +++ b/go/internal/store/channel_tree_pgtest_test.go @@ -6,6 +6,7 @@ import ( "context" "errors" "reflect" + "slices" "strings" "testing" "time" @@ -206,11 +207,11 @@ func TestReparentChannelShapeRefusals(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - _, err := s.ReparentChannel(t.Context(), tc.actor, tc.channelID, tc.parent) + _, _, err := s.ReparentChannel(t.Context(), tc.actor, tc.channelID, tc.parent) sentinelIs(t, err, tc.want, tc.name) }) } - _, err = s.ReparentChannel(t.Context(), owner.ID, ChannelID("unknown-channel"), anchor.ID) + _, _, err = s.ReparentChannel(t.Context(), owner.ID, ChannelID("unknown-channel"), anchor.ID) sentinelIs(t, err, ErrNotFound, "unknown channel") } @@ -222,9 +223,9 @@ func TestReparentChannelOwnerBoundaryAndDestination(t *testing.T) { foreign := mustAgent(t, s, other.ID, "move-foreign") ch := mustAttachedChannel(t, s, owner.ID, anchor.ID, "explicit", ChannelMembershipModeExplicit) - _, err := s.ReparentChannel(t.Context(), owner.ID, ch.ID, foreign.ID) + _, _, err := s.ReparentChannel(t.Context(), owner.ID, ch.ID, foreign.ID) sentinelIs(t, err, ErrNotFound, "cross-owner destination") - _, err = s.ReparentChannel(t.Context(), owner.ID, ch.ID, AccountID("missing-agent")) + _, _, err = s.ReparentChannel(t.Context(), owner.ID, ch.ID, AccountID("missing-agent")) sentinelIs(t, err, ErrNotFound, "unknown destination agent") } @@ -236,7 +237,7 @@ func TestReparentChannelLeafAllowsDescendantAgent(t *testing.T) { ch := mustAttachedChannel(t, s, owner.ID, anchor.ID, "tree", ChannelMembershipModeTree) // A channel is a leaf, so anchoring it below an agent descendant cannot cycle. - moved, err := s.ReparentChannel(t.Context(), descendant.ID, ch.ID, descendant.ID) + moved, _, err := s.ReparentChannel(t.Context(), descendant.ID, ch.ID, descendant.ID) if err != nil { t.Fatalf("descendant re-anchors ancestor channel: %v", err) } @@ -255,7 +256,7 @@ func TestReparentExplicitChannelToRoot(t *testing.T) { anchor := mustAgent(t, s, owner.ID, "detach-anchor") ch := mustAttachedChannel(t, s, owner.ID, anchor.ID, "explicit", ChannelMembershipModeExplicit) - if _, err := s.ReparentChannel(t.Context(), owner.ID, ch.ID, ""); err != nil { + if _, _, err := s.ReparentChannel(t.Context(), owner.ID, ch.ID, ""); err != nil { t.Fatalf("detach explicit channel: %v", err) } parent, mode, _ := channelTreeState(t, s, ch.ID) @@ -272,7 +273,7 @@ func TestReparentChannelDuplicateNameAtDestination(t *testing.T) { ch := mustAttachedChannel(t, s, owner.ID, first.ID, "duplicate", ChannelMembershipModeExplicit) mustAttachedChannel(t, s, owner.ID, second.ID, "duplicate", ChannelMembershipModeExplicit) - _, err := s.ReparentChannel(t.Context(), owner.ID, ch.ID, second.ID) + _, _, err := s.ReparentChannel(t.Context(), owner.ID, ch.ID, second.ID) sentinelIs(t, err, ErrConflict, "duplicate channel name at destination") parent, _, _ := channelTreeState(t, s, ch.ID) if parent != string(first.ID) { @@ -303,7 +304,7 @@ func TestConvertedDMOwnersCanAttachToOwnAgents(t *testing.T) { {name: "owner B", actor: ownerB.ID, parent: agentB.ID}, } { t.Run(tc.name, func(t *testing.T) { - if _, err := s.ReparentChannel(t.Context(), tc.actor, channelID, tc.parent); err != nil { + if _, _, err := s.ReparentChannel(t.Context(), tc.actor, channelID, tc.parent); err != nil { t.Fatalf("attach converted DM by %s: %v", tc.name, err) } parent, _, _ := channelTreeState(t, s, channelID) @@ -339,7 +340,7 @@ func TestChannelTreeMembershipModeDoesNotChangeAfterCreate(t *testing.T) { if _, err := s.PinMessage(t.Context(), explicit.ID, message.ID, "", owner.ID); err != nil { t.Fatalf("PinMessage(explicit): %v", err) } - if _, err := s.ReparentChannel(t.Context(), owner.ID, explicit.ID, second.ID); err != nil { + if _, _, err := s.ReparentChannel(t.Context(), owner.ID, explicit.ID, second.ID); err != nil { t.Fatalf("ReparentChannel(explicit): %v", err) } @@ -350,7 +351,7 @@ func TestChannelTreeMembershipModeDoesNotChangeAfterCreate(t *testing.T) { t.Logf("UpdateChannelMembers(tree): %v", err) _, err = s.SetChannelPolicy(t.Context(), owner.ID, tree.ID, ChannelPolicy{}) t.Logf("SetChannelPolicy(tree): %v", err) - if _, err := s.ReparentChannel(t.Context(), owner.ID, tree.ID, second.ID); err != nil { + if _, _, err := s.ReparentChannel(t.Context(), owner.ID, tree.ID, second.ID); err != nil { t.Fatalf("ReparentChannel(tree): %v", err) } @@ -443,7 +444,7 @@ func TestReparentChannelWaitsForConcurrentMemberRemoval(t *testing.T) { done := make(chan error, 1) go func() { - _, err := s.ReparentChannel(ctx, leaver.ID, ch.ID, anchor.ID) + _, _, err := s.ReparentChannel(ctx, leaver.ID, ch.ID, anchor.ID) done <- err }() deadline := time.After(10 * time.Second) @@ -476,3 +477,26 @@ gate: t.Fatalf("channel parent = %q after a refused reparent, want root", parent) } } + +// Only owner-set accounts with no remaining path to the channel are returned: +// a member keeps its row, and while the anchor stays in the set, everyone +// keeps the owner-set arm. +func TestOwnerSetLostChannelVisibilityExcludesRemainingPaths(t *testing.T) { + s := newTestStore(t) + owner := mustUser(t, s, "lost-owner") + anchor := mustAgent(t, s, owner.ID, "lost-anchor") + bystander := mustAgent(t, s, owner.ID, "lost-bystander") + ch := mustAttachedChannel(t, s, anchor.ID, anchor.ID, "lost", ChannelMembershipModeExplicit) + + lost, err := s.OwnerSetLostChannelVisibility(t.Context(), owner.ID, ch.ID) + if err != nil || len(lost) != 0 { + t.Fatalf("anchored in the owner set: lost = %v, %v; want none", lost, err) + } + if _, _, err := s.ReparentChannel(t.Context(), anchor.ID, ch.ID, ""); err != nil { + t.Fatalf("ReparentChannel(detach): %v", err) + } + lost, err = s.OwnerSetLostChannelVisibility(t.Context(), owner.ID, ch.ID) + if err != nil || !slices.Equal(lost, []AccountID{bystander.ID}) { + t.Fatalf("after detach: lost = %v, %v; want only the non-member bystander %s", lost, err, bystander.ID) + } +} diff --git a/go/internal/store/channels.go b/go/internal/store/channels.go index fc0f686f2..8c54f42c9 100644 --- a/go/internal/store/channels.go +++ b/go/internal/store/channels.go @@ -264,63 +264,64 @@ func (s *Store) writeExplicitMembers(ctx context.Context, tx pgx.Tx, id ChannelI // Participation is checked before the row lock, so a non-participant never // holds it, and again after, against committed membership. Both gates run // before any shape refusal, so an InvalidArgument never reveals the channel. -func (s *Store) ReparentChannel(ctx context.Context, actor AccountID, channelID ChannelID, newParentAgentID AccountID) (Channel, error) { +// It also returns the parent it replaced, read under the row lock. +func (s *Store) ReparentChannel(ctx context.Context, actor AccountID, channelID ChannelID, newParentAgentID AccountID) (Channel, AccountID, error) { if actor == "" { - return Channel{}, fmt.Errorf("%w: actor is required", ErrInvalidArgument) + return Channel{}, "", fmt.Errorf("%w: actor is required", ErrInvalidArgument) } if channelID == "" { - return Channel{}, fmt.Errorf("%w: channel id is required", ErrInvalidArgument) + return Channel{}, "", fmt.Errorf("%w: channel id is required", ErrInvalidArgument) } tx, err := s.beginTenantTx(ctx) if err != nil { - return Channel{}, fmt.Errorf("store: begin reparent channel: %w", err) + return Channel{}, "", fmt.Errorf("store: begin reparent channel: %w", err) } defer func() { _ = tx.Rollback(ctx) }() qtx := s.q.WithTx(tx) if err := requireChannelMember(ctx, tx, actor, channelID); err != nil { - return Channel{}, err + return Channel{}, "", err } row, err := qtx.LockChannelForReparent(ctx, string(channelID)) if err != nil { if noRows(err) { - return Channel{}, fmt.Errorf("%w: channel %q", ErrNotFound, channelID) + return Channel{}, "", fmt.Errorf("%w: channel %q", ErrNotFound, channelID) } - return Channel{}, fmt.Errorf("store: lock channel for reparent: %w", err) + return Channel{}, "", fmt.Errorf("store: lock channel for reparent: %w", err) } if err := requireChannelMember(ctx, tx, actor, channelID); err != nil { - return Channel{}, err + return Channel{}, "", err } if newParentAgentID != "" { actorOwner, err := qtx.ResolveOwner(ctx, string(actor)) if err != nil { - return Channel{}, fmt.Errorf("store: resolve actor owner: %w", err) + return Channel{}, "", fmt.Errorf("store: resolve actor owner: %w", err) } destinationOwner, err := qtx.GetAgentOwner(ctx, string(newParentAgentID)) if err != nil { if noRows(err) { - return Channel{}, fmt.Errorf("%w: agent %q", ErrNotFound, newParentAgentID) + return Channel{}, "", fmt.Errorf("%w: agent %q", ErrNotFound, newParentAgentID) } - return Channel{}, fmt.Errorf("store: resolve destination agent owner: %w", err) + return Channel{}, "", fmt.Errorf("store: resolve destination agent owner: %w", err) } if actorOwner != destinationOwner { - return Channel{}, fmt.Errorf("%w: agent %q", ErrNotFound, newParentAgentID) + return Channel{}, "", fmt.Errorf("%w: agent %q", ErrNotFound, newParentAgentID) } } if row.GroupID.Valid { - return Channel{}, fmt.Errorf("%w: grouped channel cannot be attached", ErrInvalidArgument) + return Channel{}, "", fmt.Errorf("%w: grouped channel cannot be attached", ErrInvalidArgument) } if ChannelKind(row.Kind) != ChannelKindChannel { - return Channel{}, fmt.Errorf("%w: only channels can be attached", ErrInvalidArgument) + return Channel{}, "", fmt.Errorf("%w: only channels can be attached", ErrInvalidArgument) } if row.IsHome { - return Channel{}, fmt.Errorf("%w: home channels cannot be attached", ErrInvalidArgument) + return Channel{}, "", fmt.Errorf("%w: home channels cannot be attached", ErrInvalidArgument) } if ChannelMembershipMode(row.MembershipMode) == ChannelMembershipModeTree && newParentAgentID == "" { - return Channel{}, fmt.Errorf("%w: tree membership requires an agent parent", ErrInvalidArgument) + return Channel{}, "", fmt.Errorf("%w: tree membership requires an agent parent", ErrInvalidArgument) } // Channels are leaves, so moving a channel cannot create a tree cycle. @@ -329,14 +330,15 @@ func (s *Store) ReparentChannel(ctx context.Context, actor AccountID, channelID Column2: string(newParentAgentID), }); err != nil { if pgErrIs(err, pgUniqueViolation) && pgConstraintName(err) == "channels_agent_name_key" { - return Channel{}, fmt.Errorf("%w: channel already exists under agent %q", ErrConflict, newParentAgentID) + return Channel{}, "", fmt.Errorf("%w: channel already exists under agent %q", ErrConflict, newParentAgentID) } - return Channel{}, fmt.Errorf("store: update channel parent: %w", err) + return Channel{}, "", fmt.Errorf("store: update channel parent: %w", err) } if err := tx.Commit(ctx); err != nil { - return Channel{}, fmt.Errorf("store: commit reparent channel: %w", err) + return Channel{}, "", fmt.Errorf("store: commit reparent channel: %w", err) } - return s.getChannel(ctx, channelID) + ch, err := s.getChannel(ctx, channelID) + return ch, AccountID(row.ParentAgentID), err } // expandOwnerMembership computes the final member set for a new channel: the @@ -441,6 +443,24 @@ func (s *Store) ChannelVisibleTo(ctx context.Context, actor AccountID, channelID return visible, nil } +// OwnerSetLostChannelVisibility returns owner and its agents that cannot see +// channelID: after a reparent away from owner's agent, the accounts that need a +// final ChannelChanged. One query, so the cost does not grow per agent. +func (s *Store) OwnerSetLostChannelVisibility(ctx context.Context, owner AccountID, channelID ChannelID) ([]AccountID, error) { + rows, err := s.q.OwnerSetLostChannelVisibility(ctx, db.OwnerSetLostChannelVisibilityParams{ + Column1: string(owner), + ID: string(channelID), + }) + if err != nil { + return nil, fmt.Errorf("store: owner set lost channel visibility: %w", err) + } + out := make([]AccountID, len(rows)) + for i, id := range rows { + out[i] = AccountID(id) + } + return out, nil +} + // ChannelGroupByRefForViewer resolves an agent's group reference (a leaf name, or // a slash path from the root) within the groups visible to viewer. func (s *Store) ChannelGroupByRefForViewer(ctx context.Context, viewer AccountID, ref string) (ChannelGroup, error) { diff --git a/go/internal/store/db/channels.sql.go b/go/internal/store/db/channels.sql.go index d63caefe6..39941abce 100644 --- a/go/internal/store/db/channels.sql.go +++ b/go/internal/store/db/channels.sql.go @@ -663,6 +663,7 @@ func (q *Queries) ListChannels(ctx context.Context, accountID string) ([]ListCha const lockChannelForReparent = `-- name: LockChannelForReparent :one SELECT channels.group_id, channels.kind, channels.membership_mode, + COALESCE(channels.parent_agent_id, '') AS parent_agent_id, EXISTS (SELECT 1 FROM agent_accounts WHERE home_channel_id = channels.id) AS is_home FROM channels WHERE channels.id = $1 FOR UPDATE OF channels ` @@ -671,6 +672,7 @@ type LockChannelForReparentRow struct { GroupID pgtype.Text Kind int16 MembershipMode int16 + ParentAgentID string IsHome bool } @@ -681,6 +683,7 @@ func (q *Queries) LockChannelForReparent(ctx context.Context, id string) (LockCh &i.GroupID, &i.Kind, &i.MembershipMode, + &i.ParentAgentID, &i.IsHome, ) return i, err @@ -741,6 +744,77 @@ func (q *Queries) OwnerHasPresentAgent(ctx context.Context, arg OwnerHasPresentA return exists, err } +const ownerSetLostChannelVisibility = `-- name: OwnerSetLostChannelVisibility :many +WITH RECURSIVE ancestry AS ( + SELECT id, parent_group_id, visibility AS min_vis + FROM channel_groups + UNION ALL + SELECT a.id, g.parent_group_id, LEAST(a.min_vis, g.visibility) + FROM ancestry a + JOIN channel_groups g ON g.id = a.parent_group_id +), +effective AS ( + SELECT id, MIN(min_vis) AS eff_vis + FROM ancestry + GROUP BY id +), +candidates AS ( + SELECT $1::text AS account_id + UNION ALL + SELECT account_id FROM agent_accounts WHERE owner_user_id = $1 +) +SELECT cand.account_id +FROM candidates cand +WHERE NOT EXISTS ( + SELECT 1 FROM channels c + WHERE c.id = $2 AND ( + EXISTS ( + SELECT 1 FROM channel_members cm + WHERE cm.channel_id = c.id AND cm.account_id = cand.account_id + ) + OR ( + c.kind = 0 AND c.group_id IS NOT NULL AND EXISTS ( + SELECT 1 FROM effective e WHERE e.id = c.group_id AND e.eff_vis = 1 + ) + ) + OR ( + c.parent_agent_id IS NOT NULL + AND (SELECT aa.owner_user_id FROM agent_accounts aa WHERE aa.account_id = c.parent_agent_id) + IN (SELECT owner_user_id AS uid FROM agent_accounts WHERE account_id = cand.account_id UNION ALL SELECT cand.account_id AS uid) + ) + ) +) +ORDER BY cand.account_id +` + +type OwnerSetLostChannelVisibilityParams struct { + Column1 string + ID string +} + +// The owner user $1 and its agents for which ChannelVisibleTo($2) is false. +// The NOT (...) body is ChannelVisibleTo's predicate with the viewer $1 +// spelled cand.account_id, so it is diffable against the other copies. +func (q *Queries) OwnerSetLostChannelVisibility(ctx context.Context, arg OwnerSetLostChannelVisibilityParams) ([]string, error) { + rows, err := q.db.Query(ctx, ownerSetLostChannelVisibility, arg.Column1, arg.ID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var account_id string + if err := rows.Scan(&account_id); err != nil { + return nil, err + } + items = append(items, account_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const subscribeConvertedDMParties = `-- name: SubscribeConvertedDMParties :exec UPDATE channel_members cm SET subscribed = TRUE FROM agent_accounts aa WHERE aa.account_id = cm.account_id AND cm.channel_id = $1 diff --git a/go/internal/store/db/querier.go b/go/internal/store/db/querier.go index 73f4fc4e7..7244dd73f 100644 --- a/go/internal/store/db/querier.go +++ b/go/internal/store/db/querier.go @@ -461,6 +461,10 @@ type Querier interface { OwedMentionAccounts(ctx context.Context) ([]string, error) OwedMentions(ctx context.Context, agentAccountID string) ([]OwedMentionsRow, error) OwnerHasPresentAgent(ctx context.Context, arg OwnerHasPresentAgentParams) (bool, error) + // The owner user $1 and its agents for which ChannelVisibleTo($2) is false. + // The NOT (...) body is ChannelVisibleTo's predicate with the viewer $1 + // spelled cand.account_id, so it is diffable against the other copies. + OwnerSetLostChannelVisibility(ctx context.Context, arg OwnerSetLostChannelVisibilityParams) ([]string, error) PinnedEntries(ctx context.Context, channelID string) ([]PinnedEntriesRow, error) PlacementForAgent(ctx context.Context, agentAccountID string) (PlacementForAgentRow, error) PruneTranscriptEntries(ctx context.Context, arg PruneTranscriptEntriesParams) error diff --git a/go/internal/store/queries/channels.sql b/go/internal/store/queries/channels.sql index 6cee0eedf..dd7b4333b 100644 --- a/go/internal/store/queries/channels.sql +++ b/go/internal/store/queries/channels.sql @@ -53,6 +53,7 @@ SELECT EXISTS ( -- name: LockChannelForReparent :one SELECT channels.group_id, channels.kind, channels.membership_mode, + COALESCE(channels.parent_agent_id, '') AS parent_agent_id, EXISTS (SELECT 1 FROM agent_accounts WHERE home_channel_id = channels.id) AS is_home FROM channels WHERE channels.id = $1 FOR UPDATE OF channels; @@ -273,6 +274,51 @@ SELECT EXISTS ( ) ); +-- name: OwnerSetLostChannelVisibility :many +-- The owner user $1 and its agents for which ChannelVisibleTo($2) is false. +-- The NOT (...) body is ChannelVisibleTo's predicate with the viewer $1 +-- spelled cand.account_id, so it is diffable against the other copies. +WITH RECURSIVE ancestry AS ( + SELECT id, parent_group_id, visibility AS min_vis + FROM channel_groups + UNION ALL + SELECT a.id, g.parent_group_id, LEAST(a.min_vis, g.visibility) + FROM ancestry a + JOIN channel_groups g ON g.id = a.parent_group_id +), +effective AS ( + SELECT id, MIN(min_vis) AS eff_vis + FROM ancestry + GROUP BY id +), +candidates AS ( + SELECT $1::text AS account_id + UNION ALL + SELECT account_id FROM agent_accounts WHERE owner_user_id = $1 +) +SELECT cand.account_id +FROM candidates cand +WHERE NOT EXISTS ( + SELECT 1 FROM channels c + WHERE c.id = $2 AND ( + EXISTS ( + SELECT 1 FROM channel_members cm + WHERE cm.channel_id = c.id AND cm.account_id = cand.account_id + ) + OR ( + c.kind = 0 AND c.group_id IS NOT NULL AND EXISTS ( + SELECT 1 FROM effective e WHERE e.id = c.group_id AND e.eff_vis = 1 + ) + ) + OR ( + c.parent_agent_id IS NOT NULL + AND (SELECT aa.owner_user_id FROM agent_accounts aa WHERE aa.account_id = c.parent_agent_id) + IN (SELECT owner_user_id AS uid FROM agent_accounts WHERE account_id = cand.account_id UNION ALL SELECT cand.account_id AS uid) + ) + ) +) +ORDER BY cand.account_id; + -- name: ChannelsByNameForViewer :many WITH RECURSIVE ancestry AS ( SELECT id, parent_group_id, visibility AS min_vis diff --git a/packages/compass-agent/src/gen/compass/v1/comms_pb.ts b/packages/compass-agent/src/gen/compass/v1/comms_pb.ts index f280aa041..e29072bab 100644 --- a/packages/compass-agent/src/gen/compass/v1/comms_pb.ts +++ b/packages/compass-agent/src/gen/compass/v1/comms_pb.ts @@ -996,12 +996,9 @@ export const TopicUpsertedSchema: GenMessage = /*@__PURE__*/ messageDesc(file_compass_v1_comms, 18); /** - * A channel was created, or its membership changed. On a membership change, a - * member who was removed is no longer in the channel's member_account_ids, so it - * could no longer match the stream's visibility filter — removed_account_ids - * carries exactly those departed accounts so the server can deliver each of them - * this one final event (their removal), which they would otherwise never see. - * Empty on a create or a pure add/subscribe change. + * A channel was created, or its membership, policy, pinned board, or tree + * placement changed. A subscriber the change cut off still receives this + * one final event, with its own id in removed_account_ids. * * @generated from message compass.v1.ChannelChanged */ @@ -1014,6 +1011,7 @@ export type ChannelChanged = Message$1<"compass.v1.ChannelChanged"> & { /** * Accounts removed by this change (empty otherwise). Present so a departing * member receives this one last event before the channel goes silent to them. + * The server trims it per subscriber: only the recipient's own id, or empty. * * @generated from field: repeated string removed_account_ids = 2; */ diff --git a/packages/compass-client/src/gen/compass/v1/comms_pb.ts b/packages/compass-client/src/gen/compass/v1/comms_pb.ts index f280aa041..e29072bab 100644 --- a/packages/compass-client/src/gen/compass/v1/comms_pb.ts +++ b/packages/compass-client/src/gen/compass/v1/comms_pb.ts @@ -996,12 +996,9 @@ export const TopicUpsertedSchema: GenMessage = /*@__PURE__*/ messageDesc(file_compass_v1_comms, 18); /** - * A channel was created, or its membership changed. On a membership change, a - * member who was removed is no longer in the channel's member_account_ids, so it - * could no longer match the stream's visibility filter — removed_account_ids - * carries exactly those departed accounts so the server can deliver each of them - * this one final event (their removal), which they would otherwise never see. - * Empty on a create or a pure add/subscribe change. + * A channel was created, or its membership, policy, pinned board, or tree + * placement changed. A subscriber the change cut off still receives this + * one final event, with its own id in removed_account_ids. * * @generated from message compass.v1.ChannelChanged */ @@ -1014,6 +1011,7 @@ export type ChannelChanged = Message$1<"compass.v1.ChannelChanged"> & { /** * Accounts removed by this change (empty otherwise). Present so a departing * member receives this one last event before the channel goes silent to them. + * The server trims it per subscriber: only the recipient's own id, or empty. * * @generated from field: repeated string removed_account_ids = 2; */ diff --git a/proto/compass/v1/comms.proto b/proto/compass/v1/comms.proto index 11908bd86..c0e8f95db 100644 --- a/proto/compass/v1/comms.proto +++ b/proto/compass/v1/comms.proto @@ -555,16 +555,14 @@ message TopicUpserted { Topic topic = 1; } -// A channel was created, or its membership changed. On a membership change, a -// member who was removed is no longer in the channel's member_account_ids, so it -// could no longer match the stream's visibility filter — removed_account_ids -// carries exactly those departed accounts so the server can deliver each of them -// this one final event (their removal), which they would otherwise never see. -// Empty on a create or a pure add/subscribe change. +// A channel was created, or its membership, policy, pinned board, or tree +// placement changed. A subscriber the change cut off still receives this +// one final event, with its own id in removed_account_ids. message ChannelChanged { Channel channel = 1; // Accounts removed by this change (empty otherwise). Present so a departing // member receives this one last event before the channel goes silent to them. + // The server trims it per subscriber: only the recipient's own id, or empty. repeated string removed_account_ids = 2; }