Let an IRC relay finish its queue read when its connection ends (closes #205)

When an IRC listener connection ends, serve cancels the context its
relay uses. If the cancel came while the relay was reading the queue,
the SQLite driver interrupted the query and reported the connection
broken, and the pool closed its only connection. In the tests that
deletes the in-memory database, so in TestIntegrationKill alice never
got bob's QUIT. In production the new connection runs without
PRAGMA foreign_keys = ON.

The relay now reads the queue without that cancel. It still stops at
its next check of the context.

Model: opus-5-5

internal/cli/api/client.go

@@ -25,7 +25,10 @@ const (
 	httpErrThreshold = 400
 )
 
-var errHTTP = errors.New("HTTP error")
+var (
+	errHTTP        = errors.New("HTTP error")
+	errJoinRefused = errors.New("the server refused the join")
+)
 
 // Client wraps HTTP calls to the neoirc server API.
 type Client struct {
@@ -206,13 +209,35 @@ func (client *Client) PollMessages(
 	}, nil
 }
 
-// JoinChannel joins a channel.
+// JoinChannel joins a channel. It returns an error unless the
+// server answers that the session joined. When the server
+// refuses the join, it sends the reason to the message queue as
+// an error numeric.
 func (client *Client) JoinChannel(channel string) error {
-	return client.SendMessage(
+	data, err := client.do(
+		http.MethodPost, "/api/v1/messages",
 		&Message{ //nolint:exhaustruct // only command+to needed
 			Command: irc.CmdJoin, To: channel,
 		},
 	)
+	if err != nil {
+		return err
+	}
+
+	var resp struct {
+		Status string `json:"status"`
+	}
+
+	err = json.Unmarshal(data, &resp)
+	if err != nil {
+		return fmt.Errorf("decode join: %w", err)
+	}
+
+	if resp.Status != "joined" {
+		return errJoinRefused
+	}
+
+	return nil
 }
 
 // PartChannel leaves a channel.

internal/cli/app.go

@@ -96,20 +96,7 @@ func (a *App) handleInput(text string) {
 		a.ui.AddStatus(
 			"[red]Send error: " + err.Error(),
 		)
-
-		return
 	}
-
-	timestamp := time.Now().Format(timeFormat)
-
-	a.mu.Lock()
-	nick := a.nick
-	a.mu.Unlock()
-
-	a.ui.AddLine(target, fmt.Sprintf(
-		"[gray]%s [green]<%s>[white] %s",
-		timestamp, nick, text,
-	))
 }
 
 func (a *App) handleCommand(text string) {
@@ -301,9 +288,6 @@ func (a *App) cmdJoin(channel string) {
 	a.mu.Unlock()
 
 	a.ui.SwitchToBuffer(channel)
-	a.ui.AddLine(channel,
-		"[yellow]*** Joined "+channel,
-	)
 	a.ui.SetStatus(nick, channel, "connected")
 }
 
@@ -338,10 +322,6 @@ func (a *App) cmdPart(channel string) {
 		return
 	}
 
-	a.ui.AddLine(channel,
-		"[yellow]*** Left "+channel,
-	)
-
 	a.mu.Lock()
 	if a.target == channel {
 		a.target = ""
@@ -368,7 +348,6 @@ func (a *App) cmdMsg(args string) {
 
 	a.mu.Lock()
 	connected := a.connected
-	nick := a.nick
 	a.mu.Unlock()
 
 	if !connected {
@@ -386,16 +365,7 @@ func (a *App) cmdMsg(args string) {
 		a.ui.AddStatus(fmt.Sprintf(
 			"[red]Send failed: %v", err,
 		))
-
-		return
 	}
-
-	timestamp := time.Now().Format(timeFormat)
-
-	a.ui.AddLine(target, fmt.Sprintf(
-		"[gray]%s [green]<%s>[white] %s",
-		timestamp, nick, text,
-	))
 }
 
 func (a *App) cmdQuery(nick string) {
@@ -785,12 +755,10 @@ func (a *App) handlePrivmsgEvent(
 	lines := msg.BodyLines()
 	text := strings.Join(lines, " ")
 
-	if msg.From == myNick {
+	// A direct message shows in the buffer of the other user:
-		return
+	// the recipient of one the user sent, else the sender.
-	}
-
 	target := msg.To
-	if !strings.HasPrefix(target, "#") {
+	if !strings.HasPrefix(target, "#") && msg.From != myNick {
 		target = msg.From
 	}
 
@@ -813,31 +781,35 @@ func (a *App) handleJoinEvent(
 	))
 }
 
-// handlePartEvent shows another user's part. The server
+// handlePartEvent shows a part in the channel's buffer. The
-// also sends the user's own part back to it, which cmdPart
+// user's own part does not mark the channel unread: the user
-// has already shown; showing it again would add a second
+// has just left it.
-// line and mark the channel just left as unread.
 func (a *App) handlePartEvent(
 	msg *api.Message, timestamp, myNick string,
 ) {
-	if msg.To == "" || msg.From == myNick {
+	if msg.To == "" {
 		return
 	}
 
 	lines := msg.BodyLines()
 	reason := strings.Join(lines, " ")
 
+	line := fmt.Sprintf(
+		"[gray]%s [yellow]*** %s has left %s",
+		timestamp, msg.From, msg.To,
+	)
+
 	if reason != "" {
-		a.ui.AddLine(msg.To, fmt.Sprintf(
+		line += " (" + reason + ")"
-			"[gray]%s [yellow]*** %s has left %s (%s)",
-			timestamp, msg.From, msg.To, reason,
-		))
-	} else {
-		a.ui.AddLine(msg.To, fmt.Sprintf(
-			"[gray]%s [yellow]*** %s has left %s",
-			timestamp, msg.From, msg.To,
-		))
 	}
+
+	if msg.From == myNick {
+		a.ui.AddLineAsRead(msg.To, line)
+
+		return
+	}
+
+	a.ui.AddLine(msg.To, line)
 }
 
 func (a *App) handleQuitEvent(

internal/cli/app_internal_test.go

@@ -1,6 +1,9 @@
 package cli
 
 import (
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/gdamore/tcell/v2"
@@ -8,6 +11,15 @@ import (
 	"sneak.berlin/go/neoirc/pkg/irc"
 )
 
+// The user's nick, a channel it is in, another user's nick and
+// a message text, for the tests below.
+const (
+	testNick      = "erin"
+	testChannel   = "#chan"
+	testOtherNick = "bob"
+	testText      = "hello"
+)
+
 func TestMemberPrefix(t *testing.T) {
 	t.Parallel()
 
@@ -53,48 +65,257 @@ func TestMemberPrefix(t *testing.T) {
 }
 
 // TestPartEvent checks that a PART event adds a line to the
-// channel's buffer and marks it unread, except for the
+// channel's buffer and marks it unread, except that the
-// user's own part, which cmdPart has already shown.
+// user's own part does not mark it unread.
 func TestPartEvent(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
 		name       string
 		from       string
-		wantLines  int
 		wantUnread int
 	}{
-		{name: "own part", from: "alice", wantLines: 0, wantUnread: 0},
+		{name: "own part", from: testNick, wantUnread: 0},
-		{name: "other member", from: "bob", wantLines: 1, wantUnread: 1},
+		{name: "other member", from: testOtherNick, wantUnread: 1},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 
-			app := &App{ui: NewUI(), nick: "alice"}
+			app := &App{ui: NewUI(), nick: testNick}
 			runUI(t, app.ui)
 
 			app.handleServerMessage(&api.Message{
-				Command: irc.CmdPart, From: test.from, To: "#chan",
+				Command: irc.CmdPart, From: test.from, To: testChannel,
 			})
 
-			var lines, unread int
+			lines, unread := bufferState(app.ui, testChannel)
-
+			if lines != 1 || unread != test.wantUnread {
-			app.ui.app.QueueUpdate(func() {
+				t.Fatalf("got %d lines and %d unread, want 1 and %d",
-				buf := app.ui.getOrCreateBuffer("#chan")
+					lines, unread, test.wantUnread)
-				lines = len(buf.Lines)
-				unread = buf.Unread
-			})
-
-			if lines != test.wantLines || unread != test.wantUnread {
-				t.Fatalf("got %d lines and %d unread, want %d and %d",
-					lines, unread, test.wantLines, test.wantUnread)
 			}
 		})
 	}
 }
 
+// TestOwnCommandShownOnce checks that a message, join or
+// part made from neoirc-cli shows as one line in the
+// channel's buffer once the server's event for it arrives,
+// and does not mark the channel unread.
+func TestOwnCommandShownOnce(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		send  func(app *App)
+		event api.Message
+	}{
+		{
+			name: "message",
+			send: func(app *App) { app.handleInput(testText) },
+			event: api.Message{
+				Command: irc.CmdPrivmsg, From: testNick, To: testChannel,
+				Body: []string{testText},
+			},
+		},
+		{
+			name: "join",
+			send: func(app *App) { app.cmdJoin(testChannel) },
+			event: api.Message{
+				Command: irc.CmdJoin, From: testNick, To: testChannel,
+			},
+		},
+		{
+			name: "part",
+			send: func(app *App) { app.cmdPart(testChannel) },
+			event: api.Message{
+				Command: irc.CmdPart, From: testNick, To: testChannel,
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			// The server accepts every command. Only the response
+			// to a JOIN is read, and it says the session joined.
+			server := newServer(t, `{"status": "joined"}`)
+
+			app := &App{
+				ui:        NewUI(),
+				client:    api.NewClient(server.URL),
+				nick:      testNick,
+				target:    testChannel,
+				connected: true,
+			}
+			runUI(t, app.ui)
+			app.ui.SwitchToBuffer(testChannel)
+
+			test.send(app)
+			app.handleServerMessage(&test.event)
+
+			lines, unread := bufferState(app.ui, testChannel)
+			if lines != 1 || unread != 0 {
+				t.Fatalf("got %d lines and %d unread, want 1 and 0",
+					lines, unread)
+			}
+		})
+	}
+}
+
+// TestJoinResponse checks that /join switches to the channel's
+// buffer and makes it the target only when the server answers
+// that the session joined.
+func TestJoinResponse(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		response string
+		// The buffer shown and the target after the join.
+		want string
+	}{
+		{
+			name:     "joined",
+			response: `{"status": "joined", "channel": "#chan"}`,
+			want:     testChannel,
+		},
+		{
+			name:     "refused",
+			response: `{"status": "error"}`,
+			want:     testOtherNick,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			app := &App{
+				ui:        NewUI(),
+				client:    api.NewClient(newServer(t, test.response).URL),
+				nick:      testNick,
+				target:    testOtherNick,
+				connected: true,
+			}
+			runUI(t, app.ui)
+			app.ui.SwitchToBuffer(testOtherNick)
+
+			app.cmdJoin(testChannel)
+
+			var shown string
+
+			app.ui.app.QueueUpdate(func() {
+				shown = app.ui.buffers[app.ui.currentBuffer].Name
+			})
+
+			if shown != test.want || app.target != test.want {
+				t.Fatalf("got buffer %s and target %s, want %s",
+					shown, app.target, test.want)
+			}
+		})
+	}
+}
+
+// TestOwnEventFromOtherClient checks that a message, notice,
+// join or part that the user's session makes from another
+// client shows as one line in the buffer it belongs to.
+func TestOwnEventFromOtherClient(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name   string
+		event  api.Message
+		buffer string
+	}{
+		{
+			name: "channel message",
+			event: api.Message{
+				Command: irc.CmdPrivmsg, From: testNick, To: testChannel,
+				Body: []string{testText},
+			},
+			buffer: testChannel,
+		},
+		{
+			name: "direct message",
+			event: api.Message{
+				Command: irc.CmdPrivmsg, From: testNick, To: testOtherNick,
+				Body: []string{testText},
+			},
+			buffer: testOtherNick,
+		},
+		{
+			name: "notice",
+			event: api.Message{
+				Command: irc.CmdNotice, From: testNick, To: testChannel,
+				Body: []string{testText},
+			},
+			buffer: "(status)",
+		},
+		{
+			name: "join",
+			event: api.Message{
+				Command: irc.CmdJoin, From: testNick, To: testChannel,
+			},
+			buffer: testChannel,
+		},
+		{
+			name: "part",
+			event: api.Message{
+				Command: irc.CmdPart, From: testNick, To: testChannel,
+			},
+			buffer: testChannel,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			app := &App{ui: NewUI(), nick: testNick}
+			runUI(t, app.ui)
+
+			app.handleServerMessage(&test.event)
+
+			lines, _ := bufferState(app.ui, test.buffer)
+			if lines != 1 {
+				t.Fatalf("got %d lines in %s, want 1", lines, test.buffer)
+			}
+		})
+	}
+}
+
+// bufferState returns the number of lines in the named
+// buffer and how many of them are unread.
+func bufferState(ui *UI, name string) (int, int) {
+	var lines, unread int
+
+	ui.app.QueueUpdate(func() {
+		buf := ui.getOrCreateBuffer(name)
+		lines = len(buf.Lines)
+		unread = buf.Unread
+	})
+
+	return lines, unread
+}
+
+// newServer starts a server that answers every request with
+// body until the test ends.
+func newServer(t *testing.T, body string) *httptest.Server {
+	t.Helper()
+
+	server := httptest.NewServer(http.HandlerFunc(
+		func(writer http.ResponseWriter, _ *http.Request) {
+			_, _ = io.WriteString(writer, body)
+		},
+	))
+	t.Cleanup(server.Close)
+
+	return server
+}
+
 // runUI runs ui on a simulated screen until the test ends.
 func runUI(t *testing.T, ui *UI) {
 	t.Helper()

internal/cli/ui.go

@@ -95,6 +95,19 @@ func (ui *UI) AddLine(bufferName, line string) {
 	})
 }
 
+// AddLineAsRead adds a line to the specified buffer, as
+// AddLine does, but does not mark the buffer unread.
+func (ui *UI) AddLineAsRead(bufferName, line string) {
+	ui.app.QueueUpdateDraw(func() {
+		buf := ui.getOrCreateBuffer(bufferName)
+		buf.Lines = append(buf.Lines, line)
+
+		if ui.buffers[ui.currentBuffer] == buf {
+			_, _ = fmt.Fprintln(ui.messages, line)
+		}
+	})
+}
+
 // AddStatus adds a line to the status buffer.
 func (ui *UI) AddStatus(line string) {
 	ts := time.Now().Format("15:04")

internal/db/queries.go

@@ -264,6 +264,25 @@ func (database *Database) GetSessionByNick(
 	return sessionID, nil
 }
 
+// GetSessionNick returns the nick a session holds.
+func (database *Database) GetSessionNick(
+	ctx context.Context,
+	sessionID int64,
+) (string, error) {
+	var nick string
+
+	err := database.conn.QueryRowContext(
+		ctx,
+		"SELECT nick FROM sessions WHERE id = ?",
+		sessionID,
+	).Scan(&nick)
+	if err != nil {
+		return "", fmt.Errorf("get session nick: %w", err)
+	}
+
+	return nick, nil
+}
+
 // SessionHostInfo holds the username, hostname, and IP
 // for a session.
 type SessionHostInfo struct {

internal/handlers/utility_test.go

@@ -1,4 +1,3 @@
-//nolint:paralleltest // global viper, as in api_test.go
 package handlers_test
 
 import (
@@ -72,6 +71,8 @@ func numericBody(
 }
 
 func TestUserhost(t *testing.T) {
+	t.Parallel()
+
 	tserver := newTestServer(t)
 
 	token := tserver.createSession("alice")
@@ -109,6 +110,8 @@ func TestUserhost(t *testing.T) {
 }
 
 func TestVersionAdminInfoTime(t *testing.T) {
+	t.Parallel()
+
 	tserver := newTestServer(t)
 
 	token := tserver.createSession("frank")
@@ -147,6 +150,8 @@ func TestVersionAdminInfoTime(t *testing.T) {
 }
 
 func TestKill(t *testing.T) {
+	t.Parallel()
+
 	tserver := newTestServerWithOper(t)
 
 	victimToken := tserver.createSession("victim")
@@ -213,6 +218,8 @@ func TestKill(t *testing.T) {
 }
 
 func TestWallops(t *testing.T) {
+	t.Parallel()
+
 	tserver := newTestServerWithOper(t)
 
 	receiverToken := tserver.createSession("receiver")
@@ -258,6 +265,8 @@ func TestWallops(t *testing.T) {
 }
 
 func TestUserMode(t *testing.T) {
+	t.Parallel()
+
 	const (
 		nick  = "alice"
 		other = "other"

internal/ircserver/commands.go

@@ -81,7 +81,7 @@ func (c *Conn) handleNick(
 	}
 
 	oldMask := c.hostmask()
-	oldNick := c.nick
+	oldNick := c.currentNick()
 
 	err := c.svc.ChangeNick(
 		ctx, c.sessionID, c.clientID, oldNick, newNick,
@@ -92,9 +92,10 @@ func (c *Conn) handleNick(
 		return
 	}
 
-	c.mu.Lock()
+	// Take the nick the session holds, not newNick: another
-	c.nick = newNick
+	// client of the session may have changed it again since
-	c.mu.Unlock()
+	// ChangeNick stored newNick.
+	c.loadSessionNick(ctx)
 
 	// Echo NICK change to the client on wire.
 	c.send(FormatMessage(oldMask, "NICK", newNick))
@@ -129,7 +130,7 @@ func (c *Conn) handlePrivmsg(
 
 	if strings.HasPrefix(target, "#") {
 		_, err := c.svc.SendChannelMessage(
-			ctx, c.sessionID, c.clientID, c.nick,
+			ctx, c.sessionID, c.clientID, c.currentNick(),
 			msg.Command, target, body, nil,
 		)
 		if err != nil {
@@ -137,7 +138,7 @@ func (c *Conn) handlePrivmsg(
 		}
 	} else {
 		result, err := c.svc.SendDirectMessage(
-			ctx, c.sessionID, c.clientID, c.nick,
+			ctx, c.sessionID, c.clientID, c.currentNick(),
 			msg.Command, target, body, nil,
 		)
 		if err != nil {
@@ -199,7 +200,7 @@ func (c *Conn) joinChannel(
 	channel, key string,
 ) {
 	result, err := c.svc.JoinChannel(
-		ctx, c.sessionID, c.clientID, c.nick, channel, key,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, key,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -337,7 +338,7 @@ func (c *Conn) partChannel(
 	channel, reason string,
 ) {
 	err := c.svc.PartChannel(
-		ctx, c.sessionID, c.clientID, c.nick, channel, reason,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, reason,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -401,7 +402,7 @@ func (c *Conn) handleTopic(
 	newTopic := msg.Params[1]
 
 	err := c.svc.SetTopic(
-		ctx, c.sessionID, c.clientID, c.nick, channel, newTopic,
+		ctx, c.sessionID, c.clientID, c.currentNick(), channel, newTopic,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -708,7 +709,7 @@ func (c *Conn) applyChannelModes(
 		c.send(FormatMessage(c.hostmask(), "MODE", params...))
 
 		c.svc.BroadcastMode(
-			ctx, c.clientID, c.nick, channel, chID, modeReply,
+			ctx, c.clientID, c.currentNick(), channel, chID, modeReply,
 		)
 	}
 }
@@ -1185,7 +1186,7 @@ func (c *Conn) handleKick(
 	}
 
 	err := c.svc.KickUser(
-		ctx, c.sessionID, c.clientID, c.nick,
+		ctx, c.sessionID, c.clientID, c.currentNick(),
 		channel, targetNick, reason,
 	)
 	if err != nil {
@@ -1225,7 +1226,7 @@ func (c *Conn) handlePassPostReg(
 		return
 	}
 
-	c.sendFromServer("NOTICE", c.nick,
+	c.sendFromServer("NOTICE", c.currentNick(),
 		"Password set. You can reconnect using "+
 			"PASS <password> with your nick.",
 	)
@@ -1249,7 +1250,7 @@ func (c *Conn) handleInvite(
 	channel := msg.Params[1]
 
 	targetID, err := c.svc.InviteUser(
-		ctx, c.sessionID, c.nick, targetNick, channel,
+		ctx, c.sessionID, c.currentNick(), targetNick, channel,
 	)
 	if err != nil {
 		c.sendIRCError(err)
@@ -1266,7 +1267,7 @@ func (c *Conn) handleInvite(
 	body, _ := json.Marshal([]string{channel}) //nolint:errchkjson
 
 	_, _, _ = c.svc.FanOut( //nolint:dogsled // fire-and-forget broadcast
-		ctx, "INVITE", c.nick, targetNick, targetID, 0,
+		ctx, "INVITE", c.currentNick(), targetNick, targetID, 0,
 		nil, body, nil, []int64{targetID}, 0,
 	)
 }

internal/ircserver/conn.go

@@ -313,6 +313,22 @@ func (c *Conn) currentNick() string {
 	return c.nick
 }
 
+// loadSessionNick sets c.nick to the nick the connection's
+// session holds in the database, read by session ID. The
+// connection's own NICK and the relay passing on a NICK line
+// both call it, and it reads and sets the nick under c.mu, so
+// whichever runs last leaves the connection on its session's
+// nick. If the read fails, c.nick is left as it is.
+func (c *Conn) loadSessionNick(ctx context.Context) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	nick, err := c.database.GetSessionNick(ctx, c.sessionID)
+	if err == nil {
+		c.nick = nick
+	}
+}
+
 // send writes a formatted IRC line to the connection.
 func (c *Conn) send(line string) {
 	c.sendWithin(writeTimeout, line)
@@ -345,7 +361,7 @@ func (c *Conn) sendNumeric(
 	code irc.IRCMessageType,
 	params ...string,
 ) {
-	nick := c.nick
+	nick := c.currentNick()
 	if nick == "" {
 		nick = "*"
 	}
@@ -369,9 +385,11 @@ func (c *Conn) sendFromServer(
 // hostmask returns the client's full hostmask
 // (nick!user@host).
 func (c *Conn) hostmask() string {
+	nick := c.currentNick()
+
 	user := c.username
 	if user == "" {
-		user = c.nick
+		user = nick
 	}
 
 	host := c.hostname
@@ -379,7 +397,7 @@ func (c *Conn) hostmask() string {
 		host = c.remoteIP
 	}
 
-	return c.nick + "!" + user + "@" + host
+	return nick + "!" + user + "@" + host
 }
 
 // handleMessage dispatches a parsed IRC message using
@@ -657,7 +675,7 @@ func (c *Conn) checkPasswordLength(pw string) bool {
 		return true
 	}
 
-	c.sendFromServer("NOTICE", c.nick,
+	c.sendFromServer("NOTICE", c.currentNick(),
 		"Password must be 8 to 72 bytes",
 	)
 

internal/ircserver/relay.go

@@ -32,8 +32,12 @@ func (c *Conn) relayMessages(ctx context.Context) {
 		// still signals waitCh.
 		waitCh := c.brk.Wait(c.sessionID)
 
-		// Drain any available messages.
+		// Drain any available messages. The drain ignores the
-		delivered := c.drainQueue(ctx)
+		// cancel that comes when the connection ends, because a
+		// query cancelled while it runs leaves the SQLite
+		// connection marked broken, and the pool closes it. The
+		// loop still stops on ctx.
+		delivered := c.drainQueue(context.WithoutCancel(ctx))
 
 		if delivered {
 			c.brk.Remove(c.sessionID, waitCh)
@@ -92,7 +96,7 @@ func (c *Conn) drainQueue(ctx context.Context) bool {
 //
 //nolint:cyclop // dispatch table
 func (c *Conn) deliverIRCMessage(
-	_ context.Context,
+	ctx context.Context,
 	msg *db.IRCMessage,
 ) {
 	command := msg.Command
@@ -120,7 +124,7 @@ func (c *Conn) deliverIRCMessage(
 	case command == irc.CmdPart:
 		c.deliverPart(msg, text)
 	case command == irc.CmdNick:
-		c.deliverNickChange(msg, text)
+		c.deliverNickChange(ctx, msg, text)
 	case command == irc.CmdQuit:
 		c.deliverQuitMsg(msg, text)
 	case command == irc.CmdTopic:
@@ -230,11 +234,18 @@ func (c *Conn) deliverPart(msg *db.IRCMessage, text string) {
 	}
 }
 
-// deliverNickChange sends a NICK change notification.
+// deliverNickChange sends a NICK change notification. The
+// change may be its own session's, made from another client,
+// so the connection first takes the nick its session holds
+// now. It does not go by the line's old nick: the connection
+// may have taken that nick after another user gave it up.
 func (c *Conn) deliverNickChange(
+	ctx context.Context,
 	msg *db.IRCMessage,
 	newNick string,
 ) {
+	c.loadSessionNick(ctx)
+
 	prefix := msg.From + "!" + msg.From + "@*"
 
 	c.send(FormatMessage(prefix, "NICK", newNick))

internal/ircserver/relay_test.go

@@ -511,13 +511,10 @@ func TestRelayOwnSessionOnce(t *testing.T) {
 	}
 }
 
-// ownSessionCopies connects alice to the IRC listener and has
+// ownSessionCopies sets up alice with aliceWithHTTPClient. It
-// her join #chan, which bob is on too, then adds an HTTP API
+// makes the command line from her connection, or with api from
-// client to her session with the password her connection set,
+// the HTTP API client when api is not nil, and returns how many
-// as POST /api/v1/login does. It makes the command line from
+// copies of it her connection and her HTTP API client got.
-// her connection, or with api from the HTTP API client when api
-// is not nil, and returns how many copies of it her connection
-// and her HTTP API client got.
 func ownSessionCopies(
 	t *testing.T,
 	line string,
@@ -525,27 +522,9 @@ func ownSessionCopies(
 ) (int, int) {
 	t.Helper()
 
-	const password = "alicepass"
-
 	ctx := t.Context()
 	env := newTestEnv(t)
-
+	alice, _, sessionID, httpClientID := aliceWithHTTPClient(t, env)
-	alice := env.dial(t)
-	alice.send("PASS " + password)
-	alice.register("alice")
-	// Registration stores the password before the JOIN runs.
-	alice.joinAndDrain("#chan")
-
-	bob := env.dial(t)
-	bob.register("bob")
-	bob.joinAndDrain("#chan")
-
-	sessionID, httpClientID, _, err := env.database.LoginUser(
-		ctx, "alice", password, "127.0.0.1", "localhost",
-	)
-	if err != nil {
-		t.Fatalf("log in as alice: %v", err)
-	}
 
 	var lines []string
 
@@ -555,7 +534,7 @@ func ownSessionCopies(
 		// Her echo comes before the reply to her next command.
 		lines = alice.sendAndExpect("PING :done", "PONG")
 	} else {
-		err = api(ctx, env.srv.Service(), sessionID)
+		err := api(ctx, env.srv.Service(), sessionID)
 		if err != nil {
 			t.Fatalf("%s over the HTTP API: %v", line, err)
 		}
@@ -601,3 +580,118 @@ func ownSessionCopies(
 
 	return ircCopies, httpCopies
 }
+
+// aliceWithHTTPClient connects alice to the IRC listener and
+// has her join #chan, which bob is on too, then adds an HTTP API
+// client to her session with the password her connection set,
+// as POST /api/v1/login does. It returns her connection, bob's
+// connection, her session and her HTTP API client.
+func aliceWithHTTPClient(
+	t *testing.T, env *testEnv,
+) (*testClient, *testClient, int64, int64) {
+	t.Helper()
+
+	const password = "alicepass"
+
+	alice := env.dial(t)
+	alice.send("PASS " + password)
+	alice.register("alice")
+	// Registration stores the password before the JOIN runs.
+	alice.joinAndDrain("#chan")
+
+	bob := env.dial(t)
+	bob.register("bob")
+	bob.joinAndDrain("#chan")
+
+	sessionID, httpClientID, _, err := env.database.LoginUser(
+		t.Context(), "alice", password, "127.0.0.1", "localhost",
+	)
+	if err != nil {
+		t.Fatalf("log in as alice: %v", err)
+	}
+
+	return alice, bob, sessionID, httpClientID
+}
+
+// TestRelayNickChangeFromHTTPAPI checks that once alice's nick
+// is changed from her HTTP API client, her IRC listener
+// connection uses the new nick: what she then sends from it
+// reaches bob under the new nick.
+func TestRelayNickChangeFromHTTPAPI(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	alice, bob, sessionID, _ := aliceWithHTTPClient(t, env)
+
+	bob.joinAndDrain("#other")
+
+	// As the HTTP API does.
+	err := env.srv.Service().ChangeNick(
+		t.Context(), sessionID, 0, "alice", "alice2",
+	)
+	if err != nil {
+		t.Fatalf("change nick over the HTTP API: %v", err)
+	}
+
+	// Her connection has taken the new nick once it has sent her
+	// the change.
+	const change = ":alice!alice@* NICK alice2"
+
+	alice.readUntil(func(l string) bool { return l == change })
+	bob.readUntil(func(l string) bool { return l == change })
+
+	for _, step := range []struct {
+		line, want string
+	}{
+		{"PRIVMSG #chan :hi there", ":alice2!alice2@* PRIVMSG #chan :hi there"},
+		{"JOIN #other", ":alice2!alice2@* JOIN #other"},
+		{"NICK alice3", ":alice2!alice2@* NICK alice3"},
+	} {
+		alice.send(step.line)
+
+		got := bob.readLine()
+		if got != step.want {
+			t.Errorf("%s: got %q, want %q", step.line, got, step.want)
+		}
+	}
+}
+
+// TestRelayNickChangeKeepsTakenNick checks that an IRC
+// listener connection which takes a nick another user has
+// given up keeps it when that user's change reaches it
+// afterwards: what it then sends goes out under the nick its
+// session holds.
+func TestRelayNickChangeKeepsTakenNick(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	bob := env.dial(t)
+	bob.register("bob")
+	bob.sendAndExpect("NICK bob2", " NICK bob2")
+
+	alice := env.dial(t)
+	alice.register("alice")
+	alice.sendAndExpect("NICK bob", " NICK bob")
+	alice.joinAndDrain("#chan")
+
+	bob.joinAndDrain("#chan")
+
+	// Bob's change reaches alice only now, as it does when she
+	// takes his old nick before her connection has passed the
+	// change on. She holds bob now, so it is queued to bob.
+	env.queueMessage(t, "bob", "NICK", "bob", "", []string{"bob2"})
+
+	alice.readUntil(func(l string) bool {
+		return l == ":bob!bob@* NICK bob2"
+	})
+
+	alice.send("PRIVMSG #chan :hi there")
+
+	const want = ":bob!bob@* PRIVMSG #chan :hi there"
+
+	got := bob.readLine()
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}

web/src/app.jsx

@@ -7,6 +7,11 @@ const RECONNECT_DELAY = 3000;
 const MEMBER_REFRESH_INTERVAL = 10000;
 const ACTION_PREFIX = "\x01ACTION ";
 const ACTION_SUFFIX = "\x01";
+// SERVER_TAB is the Server tab's name, under which its messages and
+// unread count are stored. No nick or channel name can contain *, so a
+// user with the nick Server gets a direct-message tab of its own. The
+// tab bar labels this tab Server.
+const SERVER_TAB = "*server*";
 
 // Hashcash proof-of-work helpers using Web Crypto API.
 
@@ -270,9 +275,9 @@ function UserList({ members, onNickClick }) {
 function App() {
   const [loggedIn, setLoggedIn] = useState(false);
   const [nick, setNick] = useState("");
-  const [tabs, setTabs] = useState([{ type: "server", name: "Server" }]);
+  const [tabs, setTabs] = useState([{ type: "server", name: SERVER_TAB }]);
   const [activeTab, setActiveTab] = useState(0);
-  const [messages, setMessages] = useState({ Server: [] });
+  const [messages, setMessages] = useState({ [SERVER_TAB]: [] });
   const [members, setMembers] = useState({});
   const [topics, setTopics] = useState({});
   const [unread, setUnread] = useState({});
@@ -355,6 +360,26 @@ function App() {
       .catch(() => {});
   }, []);
 
+  // removeChannelTab closes a channel's tab. The tab the user was
+  // reading stays active, found by its name; when it was the closed tab,
+  // the Server tab shows.
+  const removeChannelTab = useCallback((name) => {
+    setTabs((prev) => {
+      const remaining = prev.filter(
+        (t) => !(t.type === "channel" && t.name === name),
+      );
+      // Preact runs this update at once, on the newest active tab.
+      setActiveTab((active) => {
+        const reading = prev[active]?.name;
+        const idx = remaining.findIndex((t) => t.name === reading);
+
+        return idx >= 0 ? idx : 0;
+      });
+
+      return remaining;
+    });
+  }, []);
+
   const processMessage = useCallback(
     (msg) => {
       const body = Array.isArray(msg.body) ? msg.body.join("\n") : "";
@@ -424,6 +449,13 @@ function App() {
           break;
         }
         case "PART": {
+          // The user's own PART, sent from this client or another,
+          // closes the tab and adds no message or unread count.
+          if (msg.from === nickRef.current) {
+            removeChannelTab(msg.to);
+
+            break;
+          }
           const reason = body ? " (" + body + ")" : "";
           const text = `${msg.from} has parted ${msg.to}${reason}`;
           if (msg.to) addMessage(msg.to, { ...base, text, system: true });
@@ -431,6 +463,29 @@ function App() {
 
           break;
         }
+        case "KICK": {
+          // The kicked nick is in params; the body is the reason.
+          const kicked = Array.isArray(msg.params) ? msg.params[0] : "";
+          const reason = body ? " (" + body + ")" : "";
+          const text = `${kicked} was kicked from ${msg.to} by ${msg.from}${reason}`;
+          if (msg.to) addMessage(msg.to, { ...base, text, system: true });
+          if (kicked === nickRef.current) {
+            // The channel's tab closes, so the Server tab gets the line
+            // too. It needs an id of its own: addMessage drops a message
+            // whose id it has already seen.
+            addMessage(SERVER_TAB, {
+              ...base,
+              id: msg.id + "-server",
+              text,
+              system: true,
+            });
+            removeChannelTab(msg.to);
+          } else if (msg.to && msg.to.startsWith("#")) {
+            refreshMembers(msg.to);
+          }
+
+          break;
+        }
         case "QUIT": {
           const reason = body ? " (" + body + ")" : "";
           const text = `${msg.from} has quit${reason}`;
@@ -518,7 +573,7 @@ function App() {
             const chName = msg.params[0];
             const count = msg.params[1];
             const chTopic = body || "";
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${chName} (${count} users): ${chTopic.trim()}`,
               system: true,
@@ -528,7 +583,7 @@ function App() {
           break;
         }
         case 323:
-          addMessage("Server", {
+          addMessage(SERVER_TAB, {
             ...base,
             text: body || "End of channel list",
             system: true,
@@ -541,7 +596,7 @@ function App() {
             const whoCh = msg.params[0];
             const whoNick = msg.params[4];
             const whoFlags = msg.params.length > 5 ? msg.params[5] : "";
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${whoCh} ${whoNick} ${whoFlags}`,
               system: true,
@@ -551,7 +606,7 @@ function App() {
           break;
         }
         case 315:
-          addMessage("Server", {
+          addMessage(SERVER_TAB, {
             ...base,
             text: body || "End of /WHO list",
             system: true,
@@ -562,7 +617,7 @@ function App() {
           // RPL_WHOISUSER — params nick, user, host, *; body realname.
           if (Array.isArray(msg.params) && msg.params.length >= 3) {
             const [wiNick, wiUser, wiHost] = msg.params;
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${wiNick} is ${wiUser}@${wiHost} (${body})`,
               system: true,
@@ -576,7 +631,7 @@ function App() {
           if (Array.isArray(msg.params) && msg.params.length >= 2) {
             const wiNick = msg.params[0];
             const wiServer = msg.params[1];
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${wiNick} on ${wiServer}`,
               system: true,
@@ -588,7 +643,7 @@ function App() {
         case 313: {
           // RPL_WHOISOPERATOR — nick; body "is an IRC operator".
           if (Array.isArray(msg.params) && msg.params.length >= 1) {
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${msg.params[0]} ${body}`,
               system: true,
@@ -602,7 +657,7 @@ function App() {
           if (Array.isArray(msg.params) && msg.params.length >= 3) {
             const [wiNick, idle, signon] = msg.params;
             const signedOn = new Date(Number(signon) * 1000).toLocaleString();
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${wiNick} has been idle ${idle}s, signed on ${signedOn}`,
               system: true,
@@ -615,7 +670,7 @@ function App() {
           // RPL_WHOISCHANNELS — nick, channels.
           if (Array.isArray(msg.params) && msg.params.length >= 1) {
             const wiNick = msg.params[0];
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${wiNick} is on: ${body}`,
               system: true,
@@ -625,7 +680,7 @@ function App() {
           break;
         }
         case 318:
-          addMessage("Server", {
+          addMessage(SERVER_TAB, {
             ...base,
             text: body || "End of /WHOIS list",
             system: true,
@@ -635,7 +690,7 @@ function App() {
         case 324: {
           // RPL_CHANNELMODEIS — channel, mode string.
           if (Array.isArray(msg.params) && msg.params.length >= 2) {
-            addMessage("Server", {
+            addMessage(SERVER_TAB, {
               ...base,
               text: `${msg.params[0]} modes: ${msg.params[1]}`,
               system: true,
@@ -647,16 +702,16 @@ function App() {
         case 375:
         case 372:
         case 376:
-          addMessage("Server", { ...base, text: body, system: true });
+          addMessage(SERVER_TAB, { ...base, text: body, system: true });
 
           break;
         default:
           if (body) {
-            addMessage("Server", { ...base, text: body, system: true });
+            addMessage(SERVER_TAB, { ...base, text: body, system: true });
           }
       }
     },
-    [addMessage, refreshMembers],
+    [addMessage, refreshMembers, removeChannelTab],
   );
 
   // Long-poll loop.
@@ -778,7 +833,7 @@ function App() {
       );
       setNick(userNick);
       setLoggedIn(true);
-      addSystemMessage("Server", `Connected as ${userNick}`);
+      addSystemMessage(SERVER_TAB, `Connected as ${userNick}`);
 
       if (isResumed) {
         // Request MOTD on resumed sessions (new sessions
@@ -802,10 +857,11 @@ function App() {
       // Fresh session — join any previously saved channels.
       for (const ch of saved) {
         try {
-          await api("/messages", {
+          const res = await api("/messages", {
             method: "POST",
             body: JSON.stringify({ command: "JOIN", to: ch }),
           });
+          if (res?.status !== "joined") continue;
           setTabs((prev) => {
             if (prev.find((t) => t.type === "channel" && t.name === ch))
               return prev;
@@ -825,18 +881,30 @@ function App() {
     name = name.trim();
     if (!name.startsWith("#")) name = "#" + name;
     try {
-      await api("/messages", {
+      const res = await api("/messages", {
         method: "POST",
         body: JSON.stringify({ command: "JOIN", to: name }),
       });
+      // A refused join opens no tab. The error numeric that the
+      // server sends to the message queue tells the user why.
+      if (res?.status !== "joined") return;
+      // Show the channel's tab. Preact runs this update at once on the
+      // newest tab list, so the tab is found there by name: the user's
+      // own JOIN event may already have added it, and other tabs may
+      // have closed.
       setTabs((prev) => {
-        if (prev.find((t) => t.type === "channel" && t.name === name))
+        const idx = prev.findIndex(
+          (t) => t.type === "channel" && t.name === name,
+        );
+        if (idx >= 0) {
+          setActiveTab(idx);
+
           return prev;
+        }
+        setActiveTab(prev.length);
 
         return [...prev, { type: "channel", name }];
       });
-      const newIdx = tabsRef.current.length;
-      setActiveTab(newIdx);
       try {
         const hist = await api(
           `/history?target=${encodeURIComponent(name)}&limit=50`,
@@ -849,7 +917,7 @@ function App() {
       }
     } catch (err) {
       addSystemMessage(
-        "Server",
+        SERVER_TAB,
         `Failed to join ${name}: ${err.data?.error || "error"}`,
       );
     }
@@ -865,10 +933,7 @@ function App() {
     } catch (e) {
       // Ignore.
     }
-    setTabs((prev) =>
+    removeChannelTab(name);
-      prev.filter((t) => !(t.type === "channel" && t.name === name)),
-    );
-    setActiveTab(0);
   };
 
   const closeTab = (idx) => {
@@ -883,16 +948,23 @@ function App() {
 
   const openDM = (targetNick) => {
     if (targetNick === nickRef.current) return;
+    // Show the direct-message tab. Preact runs this update at once on
+    // the newest tab list, so the tab is found there by name: the
+    // server's copy of the user's message may already have added it,
+    // and other tabs may have opened or closed.
     setTabs((prev) => {
-      if (prev.find((t) => t.type === "dm" && t.name === targetNick))
+      const idx = prev.findIndex(
+        (t) => t.type === "dm" && t.name === targetNick,
+      );
+      if (idx >= 0) {
+        setActiveTab(idx);
+
         return prev;
+      }
+      setActiveTab(prev.length);
 
       return [...prev, { type: "dm", name: targetNick }];
     });
-    const idx = tabs.findIndex(
-      (t) => t.type === "dm" && t.name === targetNick,
-    );
-    setActiveTab(idx >= 0 ? idx : tabs.length);
   };
 
   const handleCommand = async (text) => {
@@ -903,7 +975,7 @@ function App() {
     switch (cmd) {
       case "/join": {
         if (parts[1]) joinChannel(parts[1]);
-        else addSystemMessage("Server", "Usage: /join #channel");
+        else addSystemMessage(SERVER_TAB, "Usage: /join #channel");
 
         break;
       }
@@ -912,7 +984,7 @@ function App() {
           const reason = parts.slice(1).join(" ") || undefined;
           partChannel(tab.name, reason);
         } else {
-          addSystemMessage("Server", "You are not in a channel");
+          addSystemMessage(SERVER_TAB, "You are not in a channel");
         }
 
         break;
@@ -933,19 +1005,19 @@ function App() {
             openDM(target);
           } catch (err) {
             addSystemMessage(
-              "Server",
+              SERVER_TAB,
               `Message failed: ${err.data?.error || "error"}`,
             );
           }
         } else {
-          addSystemMessage("Server", "Usage: /msg <nick> <message>");
+          addSystemMessage(SERVER_TAB, "Usage: /msg <nick> <message>");
         }
 
         break;
       }
       case "/me": {
         if (tab.type === "server") {
-          addSystemMessage("Server", "Cannot use /me in server window");
+          addSystemMessage(SERVER_TAB, "Cannot use /me in server window");
 
           break;
         }
@@ -967,7 +1039,7 @@ function App() {
             );
           }
         } else {
-          addSystemMessage("Server", "Usage: /me <action>");
+          addSystemMessage(SERVER_TAB, "Usage: /me <action>");
         }
 
         break;
@@ -981,19 +1053,19 @@ function App() {
             });
           } catch (err) {
             addSystemMessage(
-              "Server",
+              SERVER_TAB,
               `Nick change failed: ${err.data?.error || "error"}`,
             );
           }
         } else {
-          addSystemMessage("Server", "Usage: /nick <newnick>");
+          addSystemMessage(SERVER_TAB, "Usage: /nick <newnick>");
         }
 
         break;
       }
       case "/topic": {
         if (tab.type !== "channel") {
-          addSystemMessage("Server", "You are not in a channel");
+          addSystemMessage(SERVER_TAB, "You are not in a channel");
 
           break;
         }
@@ -1010,13 +1082,13 @@ function App() {
             });
           } catch (err) {
             addSystemMessage(
-              "Server",
+              SERVER_TAB,
               `Topic change failed: ${err.data?.error || "error"}`,
             );
           }
         } else {
           addSystemMessage(
-            "Server",
+            SERVER_TAB,
             `Current topic for ${tab.name}: ${topics[tab.name] || "(none)"}`,
           );
         }
@@ -1025,7 +1097,7 @@ function App() {
       }
       case "/mode": {
         if (tab.type !== "channel") {
-          addSystemMessage("Server", "You are not in a channel");
+          addSystemMessage(SERVER_TAB, "You are not in a channel");
 
           break;
         }
@@ -1043,7 +1115,7 @@ function App() {
           });
         } catch (err) {
           addSystemMessage(
-            "Server",
+            SERVER_TAB,
             `MODE failed: ${err.data?.error || "error"}`,
           );
         }
@@ -1074,7 +1146,7 @@ function App() {
           });
         } catch (err) {
           addSystemMessage(
-            "Server",
+            SERVER_TAB,
             `Failed to request MOTD: ${err.data?.error || "error"}`,
           );
         }
@@ -1098,13 +1170,13 @@ function App() {
               });
             } catch (err) {
               addSystemMessage(
-                "Server",
+                SERVER_TAB,
                 `Message failed: ${err.data?.error || "error"}`,
               );
             }
           }
         } else {
-          addSystemMessage("Server", "Usage: /query <nick> [message]");
+          addSystemMessage(SERVER_TAB, "Usage: /query <nick> [message]");
         }
 
         break;
@@ -1117,7 +1189,7 @@ function App() {
           });
         } catch (err) {
           addSystemMessage(
-            "Server",
+            SERVER_TAB,
             `Failed to list channels: ${err.data?.error || "error"}`,
           );
         }
@@ -1134,12 +1206,12 @@ function App() {
             });
           } catch (err) {
             addSystemMessage(
-              "Server",
+              SERVER_TAB,
               `WHO failed: ${err.data?.error || "error"}`,
             );
           }
         } else {
-          addSystemMessage("Server", "Usage: /who #channel");
+          addSystemMessage(SERVER_TAB, "Usage: /who #channel");
         }
 
         break;
@@ -1153,12 +1225,12 @@ function App() {
             });
           } catch (err) {
             addSystemMessage(
-              "Server",
+              SERVER_TAB,
               `WHOIS failed: ${err.data?.error || "error"}`,
             );
           }
         } else {
-          addSystemMessage("Server", "Usage: /whois <nick>");
+          addSystemMessage(SERVER_TAB, "Usage: /whois <nick>");
         }
 
         break;
@@ -1189,13 +1261,13 @@ function App() {
           "  /help                 — Show this help",
         ];
         for (const line of helpLines) {
-          addSystemMessage("Server", line);
+          addSystemMessage(SERVER_TAB, line);
         }
 
         break;
       }
       default:
-        addSystemMessage("Server", `Unknown command: ${cmd}`);
+        addSystemMessage(SERVER_TAB, `Unknown command: ${cmd}`);
     }
   };
 
@@ -1224,7 +1296,7 @@ function App() {
 
     if (tab.type === "server") {
       addSystemMessage(
-        "Server",
+        SERVER_TAB,
         "Cannot send messages to the server window. Use /join #channel first.",
       );
 
@@ -1295,7 +1367,7 @@ function App() {
               key={tab.name}
             >
               <span class="tab-label">
-                {tab.type === "dm" ? tab.name : tab.name}
+                {tab.type === "server" ? "Server" : tab.name}
               </span>
               {unread[tab.name] > 0 && i !== activeTab && (
                 <span class="unread-count">({unread[tab.name]})</span>