test(comms): pin dmChannelName properties and DM two-party read scope (RIG-3535)

go/internal/comms/dm_open_pgtest_test.go

@@ -16,6 +16,7 @@ import (
 
 	"connectrpc.com/connect"
 	compassv1 "github.com/RigelBuild/compass/go/gen/compass/v1"
+	"github.com/RigelBuild/compass/go/internal/store"
 )
 
 // TestOpenDMSameOwnerCreatesDMChannel: an agent opens a DM with a same-owner
@@ -281,3 +282,117 @@ func TestOpenDMResumeEmitsNoChannelChanged(t *testing.T) {
 		t.Fatalf("ChannelChanged events for the DM = %d, want exactly 1 (the create only; a resume must emit nothing)", dmChannelChanges)
 	}
 }
+
+// Each party posts under its own actor, and each read must return BOTH posts —
+// red if DM membership fails to grant a party read on the peer's turn.
+func TestOpenDMPostAndReadBothParties(t *testing.T) {
+	svc, st := newHandler(t)
+	ctx := context.Background()
+	owner := mustUser(t, st, "owner")
+	alice := mustAgent(t, st, owner.ID, "alice")
+	bob := mustAgent(t, st, owner.ID, "bob")
+
+	opened, err := svc.OpenDM(WithActor(ctx, alice.ID), connect.NewRequest(&compassv1.OpenDMRequest{PeerHandle: "bob"}))
+	if err != nil {
+		t.Fatalf("OpenDM(alice->bob) = %v, want success", err)
+	}
+	dmID := opened.Msg.GetChannel().GetId()
+
+	post := func(actor store.AccountID, text string) string {
+		t.Helper()
+		resp, err := svc.PostMessage(WithActor(ctx, actor), connect.NewRequest(&compassv1.PostMessageRequest{
+			Container:   &compassv1.PostMessageRequest_ChannelId{ChannelId: dmID},
+			Topic:       &compassv1.PostMessageRequest_TopicName{TopicName: "general"},
+			CreateTopic: true,
+			Blocks:      textBlocks(text),
+		}))
+		if err != nil {
+			t.Fatalf("PostMessage(%s): %v", text, err)
+		}
+		return resp.Msg.GetMessage().GetId()
+	}
+	aliceMsg := post(alice.ID, "from alice")
+	bobMsg := post(bob.ID, "from bob")
+
+	// Each party's read of the DM must surface BOTH posts — its own and the peer's.
+	readSees := func(reader store.AccountID) map[string]bool {
+		t.Helper()
+		listed, err := svc.ListMessages(WithActor(ctx, reader), connect.NewRequest(&compassv1.ListMessagesRequest{
+			Container: &compassv1.ListMessagesRequest_ChannelId{ChannelId: dmID},
+		}))
+		if err != nil {
+			t.Fatalf("ListMessages(%s): %v", reader, err)
+		}
+		ids := map[string]bool{}
+		for _, m := range listed.Msg.GetMessages() {
+			ids[m.GetId()] = true
+		}
+		return ids
+	}
+	for _, party := range []struct {
+		name string
+		id   store.AccountID
+	}{{"alice", alice.ID}, {"bob", bob.ID}} {
+		got := readSees(party.id)
+		if !got[aliceMsg] || !got[bobMsg] {
+			t.Fatalf("%s reads DM = %v, want both alice %q and bob %q posts", party.name, got, aliceMsg, bobMsg)
+		}
+	}
+}
+
+// Same-owner is not membership: a third agent under the same owner reads
+// nothing from a two-party DM. Canary-ordered, so an undelivered post cannot
+// pass it vacuously. Red if read-scoping falls back to owner scope.
+func TestOpenDMThirdPartySameOwnerCannotSee(t *testing.T) {
+	svc, st := newHandler(t)
+	ctx := context.Background()
+	owner := mustUser(t, st, "owner")
+	alice := mustAgent(t, st, owner.ID, "alice")
+	bob := mustAgent(t, st, owner.ID, "bob")
+	carol := mustAgent(t, st, owner.ID, "carol")
+
+	opened, err := svc.OpenDM(WithActor(ctx, alice.ID), connect.NewRequest(&compassv1.OpenDMRequest{PeerHandle: "bob"}))
+	if err != nil {
+		t.Fatalf("OpenDM(alice->bob) = %v, want success", err)
+	}
+	dmID := opened.Msg.GetChannel().GetId()
+
+	posted, err := svc.PostMessage(WithActor(ctx, alice.ID), connect.NewRequest(&compassv1.PostMessageRequest{
+		Container:   &compassv1.PostMessageRequest_ChannelId{ChannelId: dmID},
+		Topic:       &compassv1.PostMessageRequest_TopicName{TopicName: "general"},
+		CreateTopic: true,
+		Blocks:      textBlocks("private to alice and bob"),
+	}))
+	if err != nil {
+		t.Fatalf("PostMessage(alice): %v", err)
+	}
+	msgID := posted.Msg.GetMessage().GetId()
+
+	list := func(reader store.AccountID) []*compassv1.Message {
+		t.Helper()
+		listed, err := svc.ListMessages(WithActor(ctx, reader), connect.NewRequest(&compassv1.ListMessagesRequest{
+			Container: &compassv1.ListMessagesRequest_ChannelId{ChannelId: dmID},
+		}))
+		if err != nil {
+			t.Fatalf("ListMessages(%s): %v", reader, err)
+		}
+		return listed.Msg.GetMessages()
+	}
+
+	// Canary: bob, a real party, DOES read the post — proves it was delivered,
+	// so the third-party emptiness below is a real negative, not a vacuous one.
+	var bobSees bool
+	for _, m := range list(bob.ID) {
+		if m.GetId() == msgID {
+			bobSees = true
+		}
+	}
+	if !bobSees {
+		t.Fatalf("party bob cannot read the DM post %q — canary failed, negative would be vacuous", msgID)
+	}
+
+	// carol, same owner but not a DM member, reads nothing.
+	if got := list(carol.ID); len(got) != 0 {
+		t.Fatalf("third same-owner agent carol read %d messages from a DM she is not a member of, want 0", len(got))
+	}
+}

go/internal/comms/dm_test.go

@@ -0,0 +1,35 @@
+package comms
+
+import "testing"
+
+// dmChannelName must be order-independent and injective. The `:` separator is
+// load-bearing: a hyphen would derive one name for the distinct pairs
+// {a, b--c} and {a--b, c}, cross-adding members into a same-owner private DM.
+func TestDmChannelName(t *testing.T) {
+	cases := []struct {
+		name   string
+		h1, h2 string
+		want   string
+	}{
+		// lo > hi: exercises the swap branch (bob sorts after alice).
+		{name: "unsorted input swaps to sorted name", h1: "bob", h2: "alice", want: "dm:alice:bob"},
+		// lo <= hi: exercises the no-swap branch.
+		{name: "already-sorted input keeps order", h1: "alice", h2: "bob", want: "dm:alice:bob"},
+		{name: "double-hyphen handle, first pair", h1: "a", h2: "b--c", want: "dm:a:b--c"},
+		{name: "double-hyphen handle, second pair", h1: "a--b", h2: "c", want: "dm:a--b:c"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := dmChannelName(tc.h1, tc.h2); got != tc.want {
+				t.Fatalf("dmChannelName(%q, %q) = %q, want %q", tc.h1, tc.h2, got, tc.want)
+			}
+			// Order independence: the reversed pair must map to the same name.
+			if rev := dmChannelName(tc.h2, tc.h1); rev != tc.want {
+				t.Fatalf("dmChannelName(%q, %q) = %q, want %q (order-independent)", tc.h2, tc.h1, rev, tc.want)
+			}
+		})
+	}
+
+	// The two `--` rows above pin distinct names, which is the injectivity
+	// property; a hyphen delimiter would collide them onto `dm--a--b--c`.
+}