fix: eliminate GetText race in ApplyEdits, add coverage, remove dead ApplyEdit

internal/collaboration/hub.go

···
 	r.broadcast <- &broadcastMsg{data: data, except: except}
 }
 
-// ApplyEdit applies an operation via the OT engine and returns the broadcast message.
-func (r *Room) ApplyEdit(op Operation, sender *Client) {
-	newText, _ := r.ot.ApplyWithVersion(op)
-
-	type editMsg struct {
-		Type    string    `json:"type"`
-		Delta   Operation `json:"delta"`
-		Author  string    `json:"author"`
-		Content string    `json:"content"`
-	}
-	msg := editMsg{
-		Type:    "edit",
-		Delta:   op,
-		Author:  sender.DID,
-		Content: newText,
-	}
-	data, err := json.Marshal(msg)
-	if err != nil {
-		log.Printf("ApplyEdit: marshal: %v", err)
-		return
-	}
-	r.BroadcastExcept(data, sender)
-}
-
 // ApplyEdits applies a sequence of operations in order and broadcasts one
 // combined message to all other clients. Each op is applied to the text
 // resulting from the previous op, so positions in each op must be relative
···
 		return
 	}
 
+	// Capture the text returned by the final ApplyWithVersion so we don't
+	// race against another goroutine calling GetText() separately.
+	var finalText string
 	for i := range ops {
-		r.ot.ApplyWithVersion(ops[i])
+		finalText, _ = r.ot.ApplyWithVersion(ops[i])
 	}
 
 	// Include the full document text so receivers can detect and recover from
 	// divergence without a reconnect.
-	finalText := r.ot.GetText()
 	type editsMsg struct {
 		Type    string      `json:"type"`
 		Deltas  []Operation `json:"deltas"`

internal/collaboration/hub_test.go

···
+package collaboration
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+)
+
+// stubClient creates a Client with a real send channel but no WebSocket conn.
+// It is usable for hub/room tests that don't exercise the network layer.
+func stubClient(hub *Hub, did, name, color, roomKey string) *Client {
+	return &Client{
+		hub:     hub,
+		conn:    nil, // not used in hub logic
+		send:    make(chan []byte, 256),
+		DID:     did,
+		Name:    name,
+		Color:   color,
+		roomKey: roomKey,
+	}
+}
+
+// drain reads all messages from a client's send channel within timeout.
+func drain(c *Client, timeout time.Duration) [][]byte {
+	deadline := time.After(timeout)
+	var msgs [][]byte
+	for {
+		select {
+		case msg, ok := <-c.send:
+			if !ok {
+				return msgs
+			}
+			msgs = append(msgs, msg)
+		case <-deadline:
+			return msgs
+		}
+	}
+}
+
+// waitForMessages blocks until n messages arrive on c.send or timeout.
+func waitForMessages(c *Client, n int, timeout time.Duration) [][]byte {
+	deadline := time.After(timeout)
+	var msgs [][]byte
+	for len(msgs) < n {
+		select {
+		case msg, ok := <-c.send:
+			if !ok {
+				return msgs
+			}
+			msgs = append(msgs, msg)
+		case <-deadline:
+			return msgs
+		}
+	}
+	return msgs
+}
+
+// --- Hub tests ---
+
+func TestHub_GetOrCreateRoom_CreatesNew(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc1")
+	if room == nil {
+		t.Fatal("expected non-nil room")
+	}
+}
+
+func TestHub_GetOrCreateRoom_ReturnsSame(t *testing.T) {
+	hub := NewHub()
+	r1 := hub.GetOrCreateRoom("doc1")
+	r2 := hub.GetOrCreateRoom("doc1")
+	if r1 != r2 {
+		t.Error("expected same room instance for same rkey")
+	}
+}
+
+func TestHub_GetOrCreateRoom_DifferentRooms(t *testing.T) {
+	hub := NewHub()
+	r1 := hub.GetOrCreateRoom("doc1")
+	r2 := hub.GetOrCreateRoom("doc2")
+	if r1 == r2 {
+		t.Error("expected different rooms for different rkeys")
+	}
+}
+
+func TestHub_GetRoom_NilForUnknown(t *testing.T) {
+	hub := NewHub()
+	if hub.GetRoom("nonexistent") != nil {
+		t.Error("expected nil for unknown room")
+	}
+}
+
+func TestHub_GetRoom_AfterCreate(t *testing.T) {
+	hub := NewHub()
+	hub.GetOrCreateRoom("doc1")
+	if hub.GetRoom("doc1") == nil {
+		t.Error("expected room to be retrievable after creation")
+	}
+}
+
+// --- Room presence tests ---
+
+func TestRoom_RegisterClient_AppearsInPresence(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-presence")
+	c := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-presence")
+
+	room.RegisterClient(c)
+
+	// Wait for presence broadcast (register → broadcastPresence)
+	msgs := waitForMessages(c, 1, 2*time.Second)
+	if len(msgs) == 0 {
+		t.Fatal("expected presence message after register, got none")
+	}
+
+	var pres PresenceMessage
+	if err := json.Unmarshal(msgs[0], &pres); err != nil {
+		t.Fatalf("unmarshal presence: %v", err)
+	}
+	if pres.Type != "presence" {
+		t.Errorf("expected type=presence, got %q", pres.Type)
+	}
+	if len(pres.Users) != 1 {
+		t.Fatalf("expected 1 user in presence, got %d", len(pres.Users))
+	}
+	if pres.Users[0].DID != "did:plc:alice" {
+		t.Errorf("expected DID did:plc:alice, got %q", pres.Users[0].DID)
+	}
+}
+
+func TestRoom_MultipleClients_PresenceContainsAll(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-multi")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-multi")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-multi")
+
+	room.RegisterClient(alice)
+	// Drain alice's initial presence (just herself)
+	waitForMessages(alice, 1, time.Second)
+
+	room.RegisterClient(bob)
+	// Both get a new presence broadcast; wait for 1 more on each
+	aliceMsgs := waitForMessages(alice, 1, 2*time.Second)
+	bobMsgs := waitForMessages(bob, 1, 2*time.Second)
+
+	checkPresenceCount := func(name string, msgs [][]byte, want int) {
+		t.Helper()
+		if len(msgs) == 0 {
+			t.Fatalf("%s: expected presence message, got none", name)
+		}
+		var pres PresenceMessage
+		if err := json.Unmarshal(msgs[len(msgs)-1], &pres); err != nil {
+			t.Fatalf("%s: unmarshal: %v", name, err)
+		}
+		if len(pres.Users) != want {
+			t.Errorf("%s: expected %d users in presence, got %d", name, want, len(pres.Users))
+		}
+	}
+
+	checkPresenceCount("alice", aliceMsgs, 2)
+	checkPresenceCount("bob", bobMsgs, 2)
+}
+
+func TestRoom_UnregisterClient_RemovedFromPresence(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-unreg")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-unreg")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-unreg")
+
+	room.RegisterClient(alice)
+	room.RegisterClient(bob)
+	// Drain initial presence messages
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+	drain(bob, 200*time.Millisecond)
+
+	// Now unregister alice
+	room.UnregisterClient(alice)
+
+	// Bob should receive a presence update with 1 user
+	msgs := waitForMessages(bob, 1, 2*time.Second)
+	if len(msgs) == 0 {
+		t.Fatal("expected presence update after unregister, got none")
+	}
+	var pres PresenceMessage
+	if err := json.Unmarshal(msgs[len(msgs)-1], &pres); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if len(pres.Users) != 1 {
+		t.Errorf("expected 1 user after alice unregisters, got %d", len(pres.Users))
+	}
+	if pres.Users[0].DID != "did:plc:bob" {
+		t.Errorf("expected bob to remain, got %q", pres.Users[0].DID)
+	}
+}
+
+// --- Room broadcast tests ---
+
+func TestRoom_Broadcast_SendsToAll(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-broadcast")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-broadcast")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-broadcast")
+
+	room.RegisterClient(alice)
+	room.RegisterClient(bob)
+	// Drain presence messages
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+	drain(bob, 200*time.Millisecond)
+
+	room.Broadcast([]byte(`{"type":"ping"}`))
+
+	aliceMsgs := waitForMessages(alice, 1, time.Second)
+	bobMsgs := waitForMessages(bob, 1, time.Second)
+
+	if len(aliceMsgs) == 0 {
+		t.Error("alice: expected broadcast message")
+	}
+	if len(bobMsgs) == 0 {
+		t.Error("bob: expected broadcast message")
+	}
+}
+
+func TestRoom_BroadcastExcept_SkipsSender(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-except")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-except")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-except")
+
+	room.RegisterClient(alice)
+	room.RegisterClient(bob)
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+	drain(bob, 200*time.Millisecond)
+
+	room.BroadcastExcept([]byte(`{"type":"edit"}`), alice)
+
+	// Bob should receive it; alice should not
+	bobMsgs := waitForMessages(bob, 1, time.Second)
+	aliceMsgs := drain(alice, 300*time.Millisecond)
+
+	if len(bobMsgs) == 0 {
+		t.Error("bob: expected broadcast message")
+	}
+	if len(aliceMsgs) > 0 {
+		t.Errorf("alice: should not receive her own broadcast, got %d messages", len(aliceMsgs))
+	}
+}
+
+// --- Room ApplyEdits tests ---
+
+func TestRoom_ApplyEdit_BroadcastsToOthers(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-edit")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-edit")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-edit")
+
+	room.RegisterClient(alice)
+	room.RegisterClient(bob)
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+	drain(bob, 200*time.Millisecond)
+
+	room.ApplyEdits([]Operation{{From: 0, To: -1, Insert: "hello world"}}, alice)
+
+	// Bob should receive the edit message; alice should not
+	bobMsgs := waitForMessages(bob, 1, time.Second)
+	aliceMsgs := drain(alice, 300*time.Millisecond)
+
+	if len(bobMsgs) == 0 {
+		t.Fatal("bob: expected edit message, got none")
+	}
+	if len(aliceMsgs) > 0 {
+		t.Errorf("alice: should not receive her own edit, got %d messages", len(aliceMsgs))
+	}
+
+	var msg map[string]interface{}
+	if err := json.Unmarshal(bobMsgs[0], &msg); err != nil {
+		t.Fatalf("unmarshal edit msg: %v", err)
+	}
+	if msg["type"] != "edit" {
+		t.Errorf("expected type=edit, got %q", msg["type"])
+	}
+	if msg["author"] != "did:plc:alice" {
+		t.Errorf("expected author did:plc:alice, got %q", msg["author"])
+	}
+	if msg["content"] != "hello world" {
+		t.Errorf("expected content %q, got %q", "hello world", msg["content"])
+	}
+}
+
+func TestRoom_ApplyEdit_UpdatesOTState(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-ot-state")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-ot-state")
+	room.RegisterClient(alice)
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+
+	room.ApplyEdits([]Operation{{From: 0, To: -1, Insert: "first"}}, alice)
+	room.ApplyEdits([]Operation{{From: 5, To: 5, Insert: " second"}}, alice)
+
+	// Give room goroutine time to process
+	time.Sleep(100 * time.Millisecond)
+
+	if got := room.ot.GetText(); got != "first second" {
+		t.Errorf("OT state: got %q, want %q", got, "first second")
+	}
+}
+
+func TestRoom_ApplyEdits_MultipleOpsAppliedInOrder(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-multi-ops")
+
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-multi-ops")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-multi-ops")
+	room.RegisterClient(alice)
+	room.RegisterClient(bob)
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+	drain(bob, 200*time.Millisecond)
+
+	ops := []Operation{
+		{From: 0, To: -1, Insert: "hello"},
+		{From: 5, To: 5, Insert: " world"},
+	}
+	room.ApplyEdits(ops, alice)
+
+	bobMsgs := waitForMessages(bob, 1, time.Second)
+	if len(bobMsgs) == 0 {
+		t.Fatal("bob: expected edit message")
+	}
+
+	var msg struct {
+		Type    string      `json:"type"`
+		Deltas  []Operation `json:"deltas"`
+		Content string      `json:"content"`
+		Author  string      `json:"author"`
+	}
+	if err := json.Unmarshal(bobMsgs[0], &msg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if msg.Type != "edit" {
+		t.Errorf("type: got %q, want edit", msg.Type)
+	}
+	if msg.Content != "hello world" {
+		t.Errorf("content: got %q, want %q", msg.Content, "hello world")
+	}
+	if len(msg.Deltas) != 2 {
+		t.Errorf("deltas: got %d, want 2", len(msg.Deltas))
+	}
+	if msg.Author != "did:plc:alice" {
+		t.Errorf("author: got %q", msg.Author)
+	}
+}
+
+func TestRoom_ApplyEdits_UpdatesOTState(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-multi-ot")
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-multi-ot")
+	room.RegisterClient(alice)
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+
+	room.ApplyEdits([]Operation{
+		{From: 0, To: -1, Insert: "abc"},
+		{From: 3, To: 3, Insert: "def"},
+	}, alice)
+
+	time.Sleep(50 * time.Millisecond)
+	if got := room.ot.GetText(); got != "abcdef" {
+		t.Errorf("OT state: got %q, want %q", got, "abcdef")
+	}
+}
+
+func TestRoom_ApplyEdits_EmptyOpsIsNoop(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-empty-ops")
+	alice := stubClient(hub, "did:plc:alice", "Alice", "#ff0000", "doc-empty-ops")
+	bob := stubClient(hub, "did:plc:bob", "Bob", "#0000ff", "doc-empty-ops")
+	room.RegisterClient(alice)
+	room.RegisterClient(bob)
+	time.Sleep(100 * time.Millisecond)
+	drain(alice, 200*time.Millisecond)
+	drain(bob, 200*time.Millisecond)
+
+	room.ApplyEdits(nil, alice)
+
+	bobMsgs := drain(bob, 300*time.Millisecond)
+	if len(bobMsgs) > 0 {
+		t.Errorf("expected no broadcast for empty ops, got %d messages", len(bobMsgs))
+	}
+}
+
+// --- GetPresence ---
+
+func TestRoom_GetPresence_Empty(t *testing.T) {
+	hub := NewHub()
+	room := hub.GetOrCreateRoom("doc-empty-presence")
+	users := room.GetPresence()
+	if len(users) != 0 {
+		t.Errorf("expected 0 users in new room, got %d", len(users))
+	}
+}