Bind history to the recipient session and channel, not the name (closes #148)

History was selected by msg_to, so whoever next held a released nick, or
created a deleted channel again under its old name, could read the
earlier holder's messages. Each message row now records the session a
DM or INVITE was sent to (to_session_id) and the channel a channel
message was sent to (to_channel_id), and GET /api/v1/history selects by
the one that applies, using its index. A session's DM history now also
follows it across nick changes. A DM or channel message whose recipient
is deleted before it is stored gets the unknown nick or channel error.

Model: opus-5-5

README.md

@@ -2528,9 +2528,9 @@ Unique constraint on `(channel_id, session_id)`.
 
 Indexes on `(to_session_id, id)`, `(to_channel_id, id)` and `(created_at)`.
 `to_session_id` and `to_channel_id` are null on a message sent to neither, such
-as `QUIT`. `GET /api/v1/history` selects by these two columns rather than by
-`msg_to`, because nicks and channel names are reused once released, and session
-and channel ids are not.
+as `QUIT`. `GET /api/v1/history` selects by one of these two columns rather than
+by `msg_to`, because nicks and channel names are reused once released, and
+session and channel ids are not.
 
 #### `spent_hashcash`
 

internal/broker/broker.go

@@ -47,6 +47,16 @@ func (b *Broker) Notify(userID int64) {
 	}
 }
 
+// ListenerCount returns how many channels from Wait are still
+// registered for the user: those that neither Notify nor Remove
+// has taken out.
+func (b *Broker) ListenerCount(userID int64) int {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	return len(b.listeners[userID])
+}
+
 // Remove removes a specific wait channel (for cleanup on timeout).
 func (b *Broker) Remove(
 	userID int64,

internal/broker/broker_test.go

@@ -59,6 +59,39 @@ func TestRemove(t *testing.T) {
 	}
 }
 
+func TestListenerCount(t *testing.T) {
+	t.Parallel()
+
+	brk := broker.New()
+
+	wantCount := func(step string, userID int64, want int) {
+		t.Helper()
+
+		got := brk.ListenerCount(userID)
+		if got != want {
+			t.Errorf("%s: user %d: got %d, want %d",
+				step, userID, got, want)
+		}
+	}
+
+	wantCount("before Wait", 1, 0)
+
+	waitCh := brk.Wait(1)
+	brk.Wait(1)
+	brk.Wait(2)
+
+	wantCount("after two Waits", 1, 2)
+
+	brk.Remove(1, waitCh)
+
+	wantCount("after Remove", 1, 1)
+
+	brk.Notify(1)
+
+	wantCount("after Notify", 1, 0)
+	wantCount("after Notify", 2, 1)
+}
+
 func TestMultipleWaiters(t *testing.T) {
 	t.Parallel()
 

internal/db/errors.go

@@ -17,3 +17,14 @@ func IsUniqueConstraintError(err error) bool {
 
 	return sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_UNIQUE
 }
+
+// IsForeignKeyConstraintError reports whether err is a
+// SQLite foreign-key constraint violation.
+func IsForeignKeyConstraintError(err error) bool {
+	var sqliteErr *sqlite.Error
+	if !errors.As(err, &sqliteErr) {
+		return false
+	}
+
+	return sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_FOREIGNKEY
+}

internal/db/queries.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"strconv"
 	"strings"
 	"time"
@@ -1068,11 +1069,10 @@ func (database *Database) PollMessages(
 
 // GetHistory returns the PRIVMSG history of one recipient:
 // the DMs sent to session toSessionID, or the messages sent
-// to channel toChannelID. Pass 0 for the other, which
-// matches no message. A non-empty before is the id of a
-// message in that history, and only messages older than it
-// are returned; an id with no such message gives an error
-// wrapping sql.ErrNoRows.
+// to channel toChannelID. Pass 0 for the other. A non-empty
+// before is the id of a PRIVMSG in that history, and only
+// messages older than it are returned; an id with no such
+// message gives an error wrapping sql.ErrNoRows.
 func (database *Database) GetHistory(
 	ctx context.Context,
 	toSessionID, toChannelID int64,
@@ -1083,12 +1083,13 @@ func (database *Database) GetHistory(
 		limit = defaultHistLimit
 	}
 
-	var beforeID int64
+	// With no before, every message is older than this.
+	beforeID := int64(math.MaxInt64)
 
 	if before != "" {
 		err := database.conn.QueryRowContext(ctx,
 			`SELECT id FROM messages
-			 WHERE uuid = ?
+			 WHERE uuid = ? AND command = 'PRIVMSG'
 			   AND (to_session_id = ? OR to_channel_id = ?)`,
 			before, toSessionID, toChannelID,
 		).Scan(&beforeID)
@@ -1120,21 +1121,23 @@ func (database *Database) GetHistory(
 	return msgs, nil
 }
 
+// queryHistory selects by to_session_id when toSessionID is
+// set and by to_channel_id otherwise, so that the column's
+// (column, id) index serves the order and the limit.
 func (database *Database) queryHistory(
 	ctx context.Context,
 	toSessionID, toChannelID, beforeID int64,
 	limit int,
 ) (*sql.Rows, error) {
-	if beforeID > 0 {
+	if toSessionID != 0 {
 		rows, err := database.conn.QueryContext(ctx,
 			`SELECT id, uuid, command, msg_from,
 			   msg_to, params, body, meta, created_at
 			 FROM messages
-			 WHERE (to_session_id = ? OR to_channel_id = ?)
-			   AND id < ?
+			 WHERE to_session_id = ? AND id < ?
 			   AND command = 'PRIVMSG'
 			 ORDER BY id DESC LIMIT ?`,
-			toSessionID, toChannelID, beforeID, limit)
+			toSessionID, beforeID, limit)
 		if err != nil {
 			return nil, fmt.Errorf(
 				"query history: %w", err,
@@ -1148,10 +1151,10 @@ func (database *Database) queryHistory(
 		`SELECT id, uuid, command, msg_from,
 		   msg_to, params, body, meta, created_at
 		 FROM messages
-		 WHERE (to_session_id = ? OR to_channel_id = ?)
+		 WHERE to_channel_id = ? AND id < ?
 		   AND command = 'PRIVMSG'
 		 ORDER BY id DESC LIMIT ?`,
-		toSessionID, toChannelID, limit)
+		toChannelID, beforeID, limit)
 	if err != nil {
 		return nil, fmt.Errorf("query history: %w", err)
 	}

internal/handlers/api_test.go

@@ -1654,9 +1654,25 @@ func TestHistoryUnknownBefore(t *testing.T) {
 		)
 	}
 
+	// History holds only PRIVMSG, so the id of the JOIN to
+	// #here is not in it.
+	var joinID string
+
+	queued, _ := tserver.pollMessages(token, 0)
+	for _, msg := range queued {
+		if msg[commandKey] == joinCmd && msg[toKey] == "#here" {
+			joinID, _ = msg["id"].(string)
+		}
+	}
+
+	if joinID == "" {
+		t.Fatal("expected a JOIN to #here in the queue")
+	}
+
 	cases := []struct{ name, before string }{
 		{"made-up id", "no-such-message"},
 		{"id from another channel", elsewhere[0].ID},
+		{"id of a JOIN", joinID},
 	}
 
 	for _, tc := range cases {

internal/ircserver/export_test.go

@@ -60,6 +60,10 @@ func (s *Server) Listener() net.Listener {
 // to a killed victim.
 const KillWriteWindow = killWriteWindow
 
+// PollInterval is how often the relay checks the client
+// output queue when no broker notification arrives.
+const PollInterval = pollInterval
+
 // NewTestConn returns a Conn for nick on tcpConn, without a
 // server, database or service behind it.
 func NewTestConn(

internal/ircserver/relay.go

@@ -42,7 +42,10 @@ func (c *Conn) relayMessages(ctx context.Context) {
 		case <-waitCh:
 			// New message notification — loop back.
 		case <-ticker.C:
-			// Periodic check.
+			// Periodic check. Nothing used waitCh, so take it
+			// out of the broker before the next pass adds
+			// another.
+			c.brk.Remove(c.sessionID, waitCh)
 		case <-ctx.Done():
 			c.brk.Remove(c.sessionID, waitCh)
 

internal/ircserver/relay_test.go

@@ -5,6 +5,9 @@ import (
 	"slices"
 	"strings"
 	"testing"
+	"time"
+
+	"sneak.berlin/go/neoirc/internal/ircserver"
 )
 
 // queueMessage queues a message for the user with nick
@@ -44,6 +47,51 @@ func (env *testEnv) queueMessage(
 	env.brk.Notify(sessionID)
 }
 
+// TestRelayKeepsOneWaitChannel checks that an idle IRC
+// listener connection holds at most one broker wait channel
+// across several relay ticks, and none once it closes.
+func TestRelayKeepsOneWaitChannel(t *testing.T) {
+	t.Parallel()
+
+	const idleTicks = 5
+
+	env := newTestEnv(t)
+
+	client := env.dial(t)
+	client.register("idle")
+
+	sessionID, err := env.database.GetSessionByNick(t.Context(), "idle")
+	if err != nil {
+		t.Fatalf("get session of idle: %v", err)
+	}
+
+	for tick := range idleTicks {
+		time.Sleep(ircserver.PollInterval)
+
+		got := env.brk.ListenerCount(sessionID)
+		if got > 1 {
+			t.Fatalf("tick %d: got %d wait channels, want at most 1",
+				tick+1, got)
+		}
+	}
+
+	err = client.conn.Close()
+	if err != nil {
+		t.Fatalf("close: %v", err)
+	}
+
+	deadline := time.Now().Add(testTimeout)
+
+	for env.brk.ListenerCount(sessionID) != 0 {
+		if time.Now().After(deadline) {
+			t.Fatalf("after close: got %d wait channels, want 0",
+				env.brk.ListenerCount(sessionID))
+		}
+
+		time.Sleep(relayCheckStep)
+	}
+}
+
 // TestRelayMultiLineBody checks that each line of a PRIVMSG
 // or NOTICE body sent over the HTTP API reaches an IRC
 // listener user as its own line.

internal/service/service.go

@@ -400,13 +400,13 @@ func (s *Service) SendChannelMessage(
 	nick, command, channel string,
 	body, meta json.RawMessage,
 ) (int64, string, error) {
+	noSuchChannel := &IRCError{
+		irc.ErrNoSuchChannel, []string{channel}, msgNoSuchChannel,
+	}
+
 	chID, err := s.db.GetChannelByName(ctx, channel)
 	if err != nil {
-		return 0, "", &IRCError{
-			irc.ErrNoSuchChannel,
-			[]string{channel},
-			msgNoSuchChannel,
-		}
+		return 0, "", noSuchChannel
 	}
 
 	if command == irc.CmdPrivmsg {
@@ -466,6 +466,11 @@ func (s *Service) SendChannelMessage(
 		ctx, command, nick, channel, 0, chID,
 		nil, body, meta, recipients,
 	)
+	// The channel was deleted after the lookup above.
+	if db.IsForeignKeyConstraintError(fanErr) {
+		return 0, "", noSuchChannel
+	}
+
 	if fanErr != nil {
 		return 0, "", fanErr
 	}
@@ -482,13 +487,13 @@ func (s *Service) SendDirectMessage(
 	nick, command, target string,
 	body, meta json.RawMessage,
 ) (*DirectMsgResult, error) {
+	noSuchNick := &IRCError{
+		irc.ErrNoSuchNick, []string{target}, "No such nick",
+	}
+
 	targetSID, err := s.db.GetSessionByNick(ctx, target)
 	if err != nil {
-		return nil, &IRCError{
-			irc.ErrNoSuchNick,
-			[]string{target},
-			"No such nick",
-		}
+		return nil, noSuchNick
 	}
 
 	away, _ := s.db.GetAway(ctx, targetSID)
@@ -502,6 +507,11 @@ func (s *Service) SendDirectMessage(
 		ctx, command, nick, target, targetSID, 0,
 		nil, body, meta, recipients,
 	)
+	// The target's session was deleted after the lookup above.
+	if db.IsForeignKeyConstraintError(fanErr) {
+		return nil, noSuchNick
+	}
+
 	if fanErr != nil {
 		return nil, fanErr
 	}

internal/service/service_test.go

@@ -339,6 +339,78 @@ func TestBroadcastQuit(t *testing.T) {
 	}
 }
 
+// TestSendDirectMessageTargetQuits checks that a DM whose
+// target quits after its nick is looked up gets the same
+// error as a DM to an unknown nick. The trigger deletes the
+// target's session just before the DM is stored.
+func TestSendDirectMessageTargetQuits(t *testing.T) {
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	sid := createSession(ctx, t, env.db, "alice")
+	createSession(ctx, t, env.db, "bob")
+
+	_, err := env.db.GetDB().ExecContext(ctx,
+		`CREATE TRIGGER delete_recipient
+		 BEFORE INSERT ON messages
+		 BEGIN
+		   DELETE FROM sessions WHERE id = NEW.to_session_id;
+		 END`)
+	if err != nil {
+		t.Fatalf("create trigger: %v", err)
+	}
+
+	body, _ := json.Marshal([]string{"hi"}) //nolint:errchkjson
+
+	_, err = env.svc.SendDirectMessage(
+		ctx, sid, "alice", irc.CmdPrivmsg, "bob", body, nil,
+	)
+
+	var ircErr *service.IRCError
+	if !errors.As(err, &ircErr) || ircErr.Code != irc.ErrNoSuchNick {
+		t.Fatalf("want ERR_NOSUCHNICK, got %v", err)
+	}
+}
+
+// TestSendChannelMessageChannelDeleted checks that a channel
+// message whose channel is deleted after it is looked up gets
+// the same error as a message to an unknown channel. The
+// trigger deletes the channel just before the message is
+// stored.
+func TestSendChannelMessageChannelDeleted(t *testing.T) {
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	sid := createSession(ctx, t, env.db, "alice")
+
+	_, err := env.svc.JoinChannel(ctx, sid, "alice", "#gone", "")
+	if err != nil {
+		t.Fatalf("join alice: %v", err)
+	}
+
+	_, err = env.db.GetDB().ExecContext(ctx,
+		`CREATE TRIGGER delete_recipient
+		 BEFORE INSERT ON messages
+		 BEGIN
+		   DELETE FROM channels WHERE id = NEW.to_channel_id;
+		 END`)
+	if err != nil {
+		t.Fatalf("create trigger: %v", err)
+	}
+
+	body, _ := json.Marshal([]string{"hi"}) //nolint:errchkjson
+
+	_, _, err = env.svc.SendChannelMessage(
+		ctx, sid, "alice", irc.CmdPrivmsg, "#gone", body, nil,
+	)
+
+	var ircErr *service.IRCError
+	if !errors.As(err, &ircErr) ||
+		ircErr.Code != irc.ErrNoSuchChannel {
+		t.Fatalf("want ERR_NOSUCHCHANNEL, got %v", err)
+	}
+}
+
 func TestSendChannelMessage_Moderated(t *testing.T) {
 	env := newTestEnv(t)
 	ctx := t.Context()