Add the tier 3 utility IRC commands (closes #87)

USERHOST, VERSION, ADMIN, INFO, TIME, KILL and WALLOPS work on the
HTTP API and on the IRC listener. The USERHOST reply, the VERSION and
INFO text, KILL and WALLOPS are each one service function that both
transports call. User mode +w selects who receives WALLOPS. MODE on
your own nick queries or changes your user modes through one parser
on both transports: the whole mode string is checked before anything
is stored, and both flags are stored in one transaction. KILL closes
the victim's IRC connection after sending it KILL and ERROR. The
sessions table gains is_wallops in 001_initial.sql, as the pre-1.0
rule in REPO_POLICIES.md asks.

Model: opus-5-5

README.md

@@ -1575,24 +1575,31 @@ reference with all required and optional fields.
 
 **Command dispatch table:**
 
-| Command   | Required Fields | Optional | Response Status |
-| --------- | --------------- | -------- | --------------- |
-| `PRIVMSG` | `to`, `body`    | `meta`   | 200 OK          |
-| `NOTICE`  | `to`, `body`    | `meta`   | 200 OK          |
-| `JOIN`    | `to`            |          | 200 OK          |
-| `PART`    | `to`            | `body`   | 200 OK          |
-| `NICK`    | `body`          |          | 200 OK          |
-| `PASS`    | `body`          |          | 200 OK          |
-| `TOPIC`   | `to`, `body`    |          | 200 OK          |
-| `MODE`    | `to`            |          | 200 OK          |
-| `NAMES`   | `to`            |          | 200 OK          |
-| `LIST`    |                 |          | 200 OK          |
-| `WHOIS`   | `to` or `body`  |          | 200 OK          |
-| `WHO`     | `to`            |          | 200 OK          |
-| `LUSERS`  |                 |          | 200 OK          |
-| `OPER`    | `body`          |          | 200 OK          |
-| `QUIT`    |                 | `body`   | 200 OK          |
-| `PING`    |                 |          | 200 OK          |
+| Command    | Required Fields | Optional | Response Status |
+| ---------- | --------------- | -------- | --------------- |
+| `PRIVMSG`  | `to`, `body`    | `meta`   | 200 OK          |
+| `NOTICE`   | `to`, `body`    | `meta`   | 200 OK          |
+| `JOIN`     | `to`            |          | 200 OK          |
+| `PART`     | `to`            | `body`   | 200 OK          |
+| `NICK`     | `body`          |          | 200 OK          |
+| `PASS`     | `body`          |          | 200 OK          |
+| `TOPIC`    | `to`, `body`    |          | 200 OK          |
+| `MODE`     | `to`            | `body`   | 200 OK          |
+| `NAMES`    | `to`            |          | 200 OK          |
+| `LIST`     |                 |          | 200 OK          |
+| `WHOIS`    | `to` or `body`  |          | 200 OK          |
+| `WHO`      | `to`            |          | 200 OK          |
+| `LUSERS`   |                 |          | 200 OK          |
+| `USERHOST` | `body`          |          | 200 OK          |
+| `VERSION`  |                 |          | 200 OK          |
+| `ADMIN`    |                 |          | 200 OK          |
+| `INFO`     |                 |          | 200 OK          |
+| `TIME`     |                 |          | 200 OK          |
+| `OPER`     | `body`          |          | 200 OK          |
+| `KILL`     | `body`          |          | 200 OK          |
+| `WALLOPS`  | `body`          |          | 200 OK          |
+| `QUIT`     |                 | `body`   | 200 OK          |
+| `PING`     |                 |          | 200 OK          |
 
 All IRC commands return HTTP 200 OK. IRC-level success and error responses are
 delivered as **numeric replies** through the message queue (see
@@ -1619,7 +1626,11 @@ auth cookies (401), and server errors (500).
 | 433     | ERR_NICKNAMEINUSE    | NICK target is taken                         |
 | 442     | ERR_NOTONCHANNEL     | Not a member of the target channel           |
 | 461     | ERR_NEEDMOREPARAMS   | Missing required fields (to, body)           |
+| 481     | ERR_NOPRIVILEGES     | KILL or WALLOPS by a non-operator            |
+| 483     | ERR_CANTKILLSERVER   | KILL of yourself                             |
 | 491     | ERR_NOOPERHOST       | Failed OPER authentication                   |
+| 501     | ERR_UMODEUNKNOWNFLAG | User MODE string not accepted                |
+| 502     | ERR_USERSDONTMATCH   | User MODE for a nick other than your own     |
 
 **IRC numeric success replies (delivered via message queue):**
 
@@ -1630,11 +1641,13 @@ auth cookies (401), and server errors (500).
 | 003     | RPL_CREATED       | Sent on session creation/login       |
 | 004     | RPL_MYINFO        | Sent on session creation/login       |
 | 005     | RPL_ISUPPORT      | Sent on session creation/login       |
-| 221     | RPL_UMODEIS       | In response to user MODE query       |
+| 221     | RPL_UMODEIS       | In response to user MODE             |
 | 251     | RPL_LUSERCLIENT   | On connect or LUSERS command         |
 | 252     | RPL_LUSEROP       | On connect or LUSERS command         |
 | 254     | RPL_LUSERCHANNELS | On connect or LUSERS command         |
 | 255     | RPL_LUSERME       | On connect or LUSERS command         |
+| 256–259 | RPL_ADMINME etc.  | ADMIN info                           |
+| 302     | RPL_USERHOST      | USERHOST reply                       |
 | 311     | RPL_WHOISUSER     | WHOIS user info                      |
 | 312     | RPL_WHOISSERVER   | WHOIS server info                    |
 | 313     | RPL_WHOISOPERATOR | WHOIS target is oper                 |
@@ -1642,6 +1655,10 @@ auth cookies (401), and server errors (500).
 | 318     | RPL_ENDOFWHOIS    | End of WHOIS list                    |
 | 319     | RPL_WHOISCHANNELS | WHOIS channels list                  |
 | 338     | RPL_WHOISACTUALLY | WHOIS client IP (oper-only)          |
+| 351     | RPL_VERSION       | VERSION reply                        |
+| 371     | RPL_INFO          | INFO line                            |
+| 374     | RPL_ENDOFINFO     | End of INFO                          |
+| 391     | RPL_TIME          | TIME reply                           |
 | 322     | RPL_LIST          | Channel in LIST response             |
 | 323     | RPL_LISTEND       | End of LIST                          |
 | 324     | RPL_CHANNELMODEIS | Channel mode query response          |
@@ -2384,13 +2401,13 @@ IRC_LISTEN_ADDR=
 
 ### Supported Commands
 
-| Category   | Commands                                                           |
-| ---------- | ------------------------------------------------------------------ |
-| Connection | `NICK`, `USER`, `PASS`, `QUIT`, `PING`/`PONG`, `CAP`               |
-| Channels   | `JOIN`, `PART`, `MODE`, `TOPIC`, `NAMES`, `LIST`, `KICK`, `INVITE` |
-| Messaging  | `PRIVMSG`, `NOTICE`                                                |
+| Category   | Commands                                                                                 |
+| ---------- | ---------------------------------------------------------------------------------------- |
+| Connection | `NICK`, `USER`, `PASS`, `QUIT`, `PING`/`PONG`, `CAP`                                     |
+| Channels   | `JOIN`, `PART`, `MODE`, `TOPIC`, `NAMES`, `LIST`, `KICK`, `INVITE`                       |
+| Messaging  | `PRIVMSG`, `NOTICE`                                                                      |
 | Info       | `WHO`, `WHOIS`, `LUSERS`, `MOTD`, `AWAY`, `USERHOST`, `VERSION`, `ADMIN`, `INFO`, `TIME` |
-| Operator   | `OPER`, `KILL`, `WALLOPS` (requires `NEOIRC_OPER_NAME` and `NEOIRC_OPER_PASSWORD`) |
+| Operator   | `OPER`, `KILL`, `WALLOPS` (requires `NEOIRC_OPER_NAME` and `NEOIRC_OPER_PASSWORD`)       |
 
 ### Protocol Details
 
@@ -2408,13 +2425,13 @@ IRC_LISTEN_ADDR=
   (voice)
 - **User modes**: `+o` (operator, set only via `OPER`), `+w` (receives
   `WALLOPS`). `MODE` for any nick other than your own is rejected with
-  `ERR_USERSDONTMATCH` (502), for both queries and changes. Nick comparison
-  is case-insensitive.
+  `ERR_USERSDONTMATCH` (502), for both queries and changes. Nick comparison is
+  case-insensitive.
 - **KILL**: an operator's `KILL` broadcasts the victim's `QUIT` to its channel
   peers, deletes its session, and then sends the victim a `KILL` and
   `ERROR :Closing Link` before closing its socket. This applies to victims on
-  the IRC listener regardless of whether the `KILL` arrived over IRC or the
-  HTTP API.
+  the IRC listener regardless of whether the `KILL` arrived over IRC or the HTTP
+  API.
 
 ### Bridge to HTTP API
 

internal/db/export_test.go

@@ -59,10 +59,8 @@ func (database *Database) Close() error {
 	return nil
 }
 
-// ExecForTest runs a raw statement against the test
-// database.  Tests use it to install SQLite triggers that
-// force a specific write to fail, so that the atomicity of
-// multi-statement helpers can be exercised.
+// ExecForTest runs a raw statement, such as a trigger that
+// makes a write fail.
 func (database *Database) ExecForTest(
 	ctx context.Context,
 	query string,

internal/db/queries.go

@@ -2425,63 +2425,57 @@ func (database *Database) SetChannelUserLimit(
 	return nil
 }
 
-// SetSessionUserModes applies a set of user-mode flag
-// changes to a session inside a single transaction, so a
-// multi-mode change such as "+w-o" is all-or-nothing. A nil
-// pointer means the caller did not mention that mode and
-// the stored value must be left untouched.
+// SetSessionUserModes sets a session's wallops (+w) and
+// oper (+o) flags in one transaction, so that either both
+// changes are stored or neither is. A nil flag is left as
+// it is.
 func (database *Database) SetSessionUserModes(
 	ctx context.Context,
 	sessionID int64,
 	wallops *bool,
 	oper *bool,
 ) error {
-	if wallops == nil && oper == nil {
-		return nil
-	}
-
 	transaction, err := database.conn.BeginTx(ctx, nil)
 	if err != nil {
 		return fmt.Errorf("begin tx: %w", err)
 	}
 
 	if wallops != nil {
-		if _, err := transaction.ExecContext(
+		_, err = transaction.ExecContext(
 			ctx,
 			`UPDATE sessions SET is_wallops = ? WHERE id = ?`,
 			boolToInt(*wallops), sessionID,
-		); err != nil {
+		)
+		if err != nil {
 			_ = transaction.Rollback()
 
-			return fmt.Errorf(
-				"set session wallops: %w", err,
-			)
+			return fmt.Errorf("set session wallops: %w", err)
 		}
 	}
 
 	if oper != nil {
-		if _, err := transaction.ExecContext(
+		_, err = transaction.ExecContext(
 			ctx,
 			`UPDATE sessions SET is_oper = ? WHERE id = ?`,
 			boolToInt(*oper), sessionID,
-		); err != nil {
+		)
+		if err != nil {
 			_ = transaction.Rollback()
 
 			return fmt.Errorf("set session oper: %w", err)
 		}
 	}
 
-	if err := transaction.Commit(); err != nil {
-		_ = transaction.Rollback()
-
+	err = transaction.Commit()
+	if err != nil {
 		return fmt.Errorf("commit user modes: %w", err)
 	}
 
 	return nil
 }
 
-// boolToInt renders a Go bool as the 0/1 integer used for
-// boolean columns in the SQLite schema.
+// boolToInt returns the 0 or 1 that the schema stores for a
+// boolean.
 func boolToInt(value bool) int {
 	if value {
 		return 1
@@ -2490,30 +2484,6 @@ func boolToInt(value bool) int {
 	return 0
 }
 
-// SetSessionWallops sets the wallops (+w) flag on a
-// session.
-func (database *Database) SetSessionWallops(
-	ctx context.Context,
-	sessionID int64,
-	enabled bool,
-) error {
-	val := 0
-	if enabled {
-		val = 1
-	}
-
-	_, err := database.conn.ExecContext(
-		ctx,
-		`UPDATE sessions SET is_wallops = ? WHERE id = ?`,
-		val, sessionID,
-	)
-	if err != nil {
-		return fmt.Errorf("set session wallops: %w", err)
-	}
-
-	return nil
-}
-
 // IsSessionWallops returns whether the session has the
 // wallops (+w) usermode set.
 func (database *Database) IsSessionWallops(
@@ -2557,16 +2527,19 @@ func (database *Database) GetWallopsSessionIDs(
 
 	for rows.Next() {
 		var sessionID int64
-		if scanErr := rows.Scan(&sessionID); scanErr != nil {
+
+		err = rows.Scan(&sessionID)
+		if err != nil {
 			return nil, fmt.Errorf(
-				"scan wallops session: %w", scanErr,
+				"scan wallops session: %w", err,
 			)
 		}
 
 		ids = append(ids, sessionID)
 	}
 
-	if err := rows.Err(); err != nil {
+	err = rows.Err()
+	if err != nil {
 		return nil, fmt.Errorf(
 			"iterate wallops sessions: %w", err,
 		)
@@ -2585,17 +2558,11 @@ type UserhostInfo struct {
 }
 
 // GetUserhostInfo returns USERHOST info for the given
-// nicks. Nicks with no session are omitted from the
-// result; any other database failure is returned, because
-// an unreadable row is not the same as an absent one.
+// nicks. Nicks with no session are left out.
 func (database *Database) GetUserhostInfo(
 	ctx context.Context,
 	nicks []string,
 ) ([]UserhostInfo, error) {
-	if len(nicks) == 0 {
-		return nil, nil
-	}
-
 	results := make([]UserhostInfo, 0, len(nicks))
 
 	for _, nick := range nicks {
@@ -2611,11 +2578,11 @@ func (database *Database) GetUserhostInfo(
 			&info.Nick, &info.Username, &info.Hostname,
 			&info.IsOper, &info.AwayMessage,
 		)
-		if err != nil {
-			if errors.Is(err, sql.ErrNoRows) {
-				continue // nick not online
-			}
+		if errors.Is(err, sql.ErrNoRows) {
+			continue
+		}
 
+		if err != nil {
 			return nil, fmt.Errorf(
 				"userhost lookup %q: %w", nick, err,
 			)

internal/db/queries_test.go

@@ -1492,13 +1492,9 @@ func TestChannelUserLimit(t *testing.T) {
 	}
 }
 
-// TestSetSessionUserModesIsAtomic proves that a multi-mode
-// change is all-or-nothing.  A trigger makes the is_oper
-// UPDATE fail after the is_wallops UPDATE has already run,
-// which is exactly the "+w-o" partial-failure the previous
-// implementation exhibited: it issued the two UPDATEs
-// independently, so +w persisted while the caller reported
-// total failure.
+// TestSetSessionUserModesIsAtomic makes the is_oper write
+// fail after the is_wallops write has run, and checks that
+// the wallops change was rolled back.
 func TestSetSessionUserModesIsAtomic(t *testing.T) {
 	t.Parallel()
 
@@ -1540,16 +1536,12 @@ func TestSetSessionUserModesIsAtomic(t *testing.T) {
 	}
 
 	if gotWallops {
-		t.Error(
-			"wallops persisted despite the transaction " +
-				"failing; the apply stage is not atomic",
-		)
+		t.Error("wallops was stored although the change failed")
 	}
 }
 
-// TestSetSessionUserModesAppliesBoth is the success-path
-// counterpart: when nothing fails, both flags are written,
-// and a nil pointer leaves that flag untouched.
+// TestSetSessionUserModesAppliesBoth checks that both flags
+// are written, and that a nil flag is left as it is.
 func TestSetSessionUserModesAppliesBoth(t *testing.T) {
 	t.Parallel()
 
@@ -1563,18 +1555,18 @@ func TestSetSessionUserModesAppliesBoth(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := database.SetSessionOper(
-		ctx, sessionID, true,
-	); err != nil {
+	err = database.SetSessionOper(ctx, sessionID, true)
+	if err != nil {
 		t.Fatal(err)
 	}
 
 	wallops := true
 	oper := false
 
-	if err := database.SetSessionUserModes(
+	err = database.SetSessionUserModes(
 		ctx, sessionID, &wallops, &oper,
-	); err != nil {
+	)
+	if err != nil {
 		t.Fatal(err)
 	}
 
@@ -1597,10 +1589,10 @@ func TestSetSessionUserModesAppliesBoth(t *testing.T) {
 		)
 	}
 
-	// A nil pointer must leave the stored value alone.
-	if err := database.SetSessionUserModes(
+	err = database.SetSessionUserModes(
 		ctx, sessionID, nil, nil,
-	); err != nil {
+	)
+	if err != nil {
 		t.Fatal(err)
 	}
 
@@ -1612,6 +1604,6 @@ func TestSetSessionUserModesAppliesBoth(t *testing.T) {
 	}
 
 	if !gotWallops {
-		t.Error("nil pointers must not clear wallops")
+		t.Error("a nil wallops flag cleared wallops")
 	}
 }

internal/handlers/api.go

@@ -962,12 +962,10 @@ func (hdlr *Handlers) dispatchCommand(
 	bodyLines func() []string,
 ) {
 	switch command {
-	case irc.CmdAway, irc.CmdNick,
-		irc.CmdPass, irc.CmdInvite:
-		hdlr.dispatchBodyOnlyCommand(
+	case irc.CmdAway:
+		hdlr.handleAway(
 			writer, request,
-			sessionID, clientID, nick,
-			command, bodyLines,
+			sessionID, clientID, nick, bodyLines,
 		)
 	case irc.CmdPrivmsg, irc.CmdNotice:
 		hdlr.handlePrivmsg(
@@ -986,12 +984,27 @@ func (hdlr *Handlers) dispatchCommand(
 			writer, request,
 			sessionID, clientID, nick, target, body,
 		)
+	case irc.CmdNick:
+		hdlr.handleNick(
+			writer, request,
+			sessionID, clientID, nick, bodyLines,
+		)
+	case irc.CmdPass:
+		hdlr.handlePass(
+			writer, request,
+			sessionID, clientID, nick, bodyLines,
+		)
 	case irc.CmdTopic:
 		hdlr.handleTopic(
 			writer, request,
 			sessionID, clientID, nick,
 			target, body, bodyLines,
 		)
+	case irc.CmdInvite:
+		hdlr.handleInvite(
+			writer, request,
+			sessionID, clientID, nick, bodyLines,
+		)
 	case irc.CmdKick:
 		hdlr.handleKick(
 			writer, request,
@@ -1005,12 +1018,9 @@ func (hdlr *Handlers) dispatchCommand(
 	case irc.CmdOper, irc.CmdKill, irc.CmdWallops:
 		hdlr.dispatchOperCommand(
 			writer, request,
-			sessionID, clientID, nick,
-			command, bodyLines,
+			sessionID, clientID, nick, command, bodyLines,
 		)
-	case irc.CmdMotd, irc.CmdPing,
-		irc.CmdVersion, irc.CmdAdmin,
-		irc.CmdInfo, irc.CmdTime:
+	case irc.CmdMotd, irc.CmdPing:
 		hdlr.dispatchInfoCommand(
 			writer, request,
 			sessionID, clientID, nick,
@@ -1070,6 +1080,22 @@ func (hdlr *Handlers) dispatchQueryCommand(
 			writer, request,
 			sessionID, clientID, nick, bodyLines,
 		)
+	case irc.CmdVersion:
+		hdlr.handleVersion(
+			writer, request, sessionID, clientID, nick,
+		)
+	case irc.CmdAdmin:
+		hdlr.handleAdmin(
+			writer, request, sessionID, clientID, nick,
+		)
+	case irc.CmdInfo:
+		hdlr.handleInfo(
+			writer, request, sessionID, clientID, nick,
+		)
+	case irc.CmdTime:
+		hdlr.handleTime(
+			writer, request, sessionID, clientID, nick,
+		)
 	default:
 		hdlr.enqueueNumeric(
 			request.Context(), clientID,
@@ -1863,8 +1889,7 @@ func (hdlr *Handlers) deliverSetTopicNumerics(
 }
 
 // dispatchInfoCommand handles informational IRC commands
-// that produce server-side numerics (MOTD, PING,
-// VERSION, ADMIN, INFO, TIME).
+// that produce server-side numerics (MOTD, PING).
 func (hdlr *Handlers) dispatchInfoCommand(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -1890,34 +1915,6 @@ func (hdlr *Handlers) dispatchInfoCommand(
 			},
 			http.StatusOK)
 
-		return
-	case irc.CmdVersion:
-		hdlr.handleVersion(
-			writer, request,
-			sessionID, clientID, nick,
-		)
-
-		return
-	case irc.CmdAdmin:
-		hdlr.handleAdmin(
-			writer, request,
-			sessionID, clientID, nick,
-		)
-
-		return
-	case irc.CmdInfo:
-		hdlr.handleInfo(
-			writer, request,
-			sessionID, clientID, nick,
-		)
-
-		return
-	case irc.CmdTime:
-		hdlr.handleTime(
-			writer, request,
-			sessionID, clientID, nick,
-		)
-
 		return
 	}
 

internal/handlers/utility.go

@@ -1,56 +1,15 @@
 package handlers
 
 import (
-	"encoding/json"
-	"errors"
 	"net/http"
 	"strings"
 	"time"
 
-	"sneak.berlin/go/neoirc/internal/db"
-	"sneak.berlin/go/neoirc/internal/service"
 	"sneak.berlin/go/neoirc/pkg/irc"
 )
 
-// maxUserhostNicks is the maximum number of nicks allowed
-// in a single USERHOST query (RFC 2812).
-const maxUserhostNicks = 5
-
-// dispatchBodyOnlyCommand routes commands that take
-// (writer, request, sessionID, clientID, nick, bodyLines).
-func (hdlr *Handlers) dispatchBodyOnlyCommand(
-	writer http.ResponseWriter,
-	request *http.Request,
-	sessionID, clientID int64,
-	nick, command string,
-	bodyLines func() []string,
-) {
-	switch command {
-	case irc.CmdAway:
-		hdlr.handleAway(
-			writer, request,
-			sessionID, clientID, nick, bodyLines,
-		)
-	case irc.CmdNick:
-		hdlr.handleNick(
-			writer, request,
-			sessionID, clientID, nick, bodyLines,
-		)
-	case irc.CmdPass:
-		hdlr.handlePass(
-			writer, request,
-			sessionID, clientID, nick, bodyLines,
-		)
-	case irc.CmdInvite:
-		hdlr.handleInvite(
-			writer, request,
-			sessionID, clientID, nick, bodyLines,
-		)
-	}
-}
-
-// dispatchOperCommand routes oper-related commands (OPER,
-// KILL, WALLOPS) to their handlers.
+// dispatchOperCommand routes the server operator commands
+// OPER, KILL and WALLOPS to their handlers.
 func (hdlr *Handlers) dispatchOperCommand(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -77,8 +36,8 @@ func (hdlr *Handlers) dispatchOperCommand(
 	}
 }
 
-// handleUserhost handles the USERHOST command.
-// Returns user@host info for up to 5 nicks.
+// handleUserhost handles USERHOST: user@host for each of
+// up to five nicks given in the body.
 func (hdlr *Handlers) handleUserhost(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -86,8 +45,6 @@ func (hdlr *Handlers) handleUserhost(
 	nick string,
 	bodyLines func() []string,
 ) {
-	ctx := request.Context()
-
 	lines := bodyLines()
 	if len(lines) == 0 {
 		hdlr.respondIRCError(
@@ -100,107 +57,47 @@ func (hdlr *Handlers) handleUserhost(
 		return
 	}
 
-	// Limit to 5 nicks per RFC 2812.
-	nicks := lines
-	if len(nicks) > maxUserhostNicks {
-		nicks = nicks[:maxUserhostNicks]
-	}
-
-	infos, err := hdlr.params.Database.GetUserhostInfo(
-		ctx, nicks,
+	reply, err := hdlr.svc.UserhostReply(
+		request.Context(), lines, hdlr.serverName(),
 	)
-	if err != nil {
-		hdlr.log.Error(
-			"userhost query failed", "error", err,
-		)
-		hdlr.respondError(
-			writer, request,
-			"internal error",
-			http.StatusInternalServerError,
-		)
-
+	if hdlr.handleServiceError(
+		writer, request, clientID, sessionID, nick, err,
+	) {
 		return
 	}
 
-	replyStr := hdlr.buildUserhostReply(infos)
-
 	hdlr.enqueueNumeric(
-		ctx, clientID, irc.RplUserHost, nick, nil,
-		replyStr,
+		request.Context(), clientID,
+		irc.RplUserHost, nick, nil, reply,
 	)
-
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// buildUserhostReply builds the RPL_USERHOST reply
-// string per RFC 2812.
-func (hdlr *Handlers) buildUserhostReply(
-	infos []db.UserhostInfo,
-) string {
-	replies := make([]string, 0, len(infos))
-
-	for idx := range infos {
-		info := &infos[idx]
-
-		username := info.Username
-		if username == "" {
-			username = info.Nick
-		}
-
-		hostname := info.Hostname
-		if hostname == "" {
-			hostname = hdlr.serverName()
-		}
-
-		operStar := ""
-		if info.IsOper {
-			operStar = "*"
-		}
-
-		awayPrefix := "+"
-		if info.AwayMessage != "" {
-			awayPrefix = "-"
-		}
-
-		replies = append(replies,
-			info.Nick+operStar+"="+
-				awayPrefix+username+"@"+hostname,
-		)
-	}
-
-	return strings.Join(replies, " ")
-}
-
-// handleVersion handles the VERSION command.
-// Returns the server version string.
+// handleVersion handles VERSION.
 func (hdlr *Handlers) handleVersion(
 	writer http.ResponseWriter,
 	request *http.Request,
 	sessionID, clientID int64,
 	nick string,
 ) {
-	ctx := request.Context()
-	srvName := hdlr.serverName()
-	version := hdlr.svc.ServerVersion()
-
-	// 351 RPL_VERSION
 	hdlr.enqueueNumeric(
-		ctx, clientID, irc.RplVersion, nick,
-		[]string{version + ".", srvName},
+		request.Context(), clientID, irc.RplVersion, nick,
+		[]string{
+			hdlr.svc.ServerVersion() + ".",
+			hdlr.serverName(),
+		},
 		"",
 	)
-
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// handleAdmin handles the ADMIN command.
-// Returns server admin contact info.
+// handleAdmin handles ADMIN.
 func (hdlr *Handlers) handleAdmin(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -210,39 +107,29 @@ func (hdlr *Handlers) handleAdmin(
 	ctx := request.Context()
 	srvName := hdlr.serverName()
 
-	// 256 RPL_ADMINME
 	hdlr.enqueueNumeric(
 		ctx, clientID, irc.RplAdminMe, nick,
-		[]string{srvName},
-		"Administrative info",
+		[]string{srvName}, "Administrative info",
 	)
-
-	// 257 RPL_ADMINLOC1
 	hdlr.enqueueNumeric(
 		ctx, clientID, irc.RplAdminLoc1, nick, nil,
 		"neoirc server",
 	)
-
-	// 258 RPL_ADMINLOC2
 	hdlr.enqueueNumeric(
 		ctx, clientID, irc.RplAdminLoc2, nick, nil,
 		"IRC over HTTP",
 	)
-
-	// 259 RPL_ADMINEMAIL
 	hdlr.enqueueNumeric(
 		ctx, clientID, irc.RplAdminEmail, nick, nil,
 		"admin@"+srvName,
 	)
-
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// handleInfo handles the INFO command.
-// Returns server software information.
+// handleInfo handles INFO.
 func (hdlr *Handlers) handleInfo(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -252,51 +139,41 @@ func (hdlr *Handlers) handleInfo(
 	ctx := request.Context()
 
 	for _, line := range hdlr.svc.InfoLines() {
-		// 371 RPL_INFO
 		hdlr.enqueueNumeric(
-			ctx, clientID, irc.RplInfo, nick, nil,
-			line,
+			ctx, clientID, irc.RplInfo, nick, nil, line,
 		)
 	}
 
-	// 374 RPL_ENDOFINFO
 	hdlr.enqueueNumeric(
 		ctx, clientID, irc.RplEndOfInfo, nick, nil,
 		"End of /INFO list",
 	)
-
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// handleTime handles the TIME command.
-// Returns the server's local time in RFC format.
+// handleTime handles TIME: the server's local time.
 func (hdlr *Handlers) handleTime(
 	writer http.ResponseWriter,
 	request *http.Request,
 	sessionID, clientID int64,
 	nick string,
 ) {
-	ctx := request.Context()
-	srvName := hdlr.serverName()
-
-	// 391 RPL_TIME
 	hdlr.enqueueNumeric(
-		ctx, clientID, irc.RplTime, nick,
-		[]string{srvName},
+		request.Context(), clientID, irc.RplTime, nick,
+		[]string{hdlr.serverName()},
 		time.Now().Format(time.RFC1123),
 	)
-
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// handleKill handles the KILL command.
-// Forcibly disconnects a user (oper only).
+// handleKill handles KILL: the body is the target nick and
+// an optional reason.
 func (hdlr *Handlers) handleKill(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -304,25 +181,9 @@ func (hdlr *Handlers) handleKill(
 	nick string,
 	bodyLines func() []string,
 ) {
-	ctx := request.Context()
-
-	// Check oper status.
-	isOper, err := hdlr.params.Database.IsSessionOper(
-		ctx, sessionID,
-	)
-	if err != nil || !isOper {
-		hdlr.respondIRCError(
-			writer, request, clientID, sessionID,
-			irc.ErrNoPrivileges, nick, nil,
-			"Permission Denied- You're not an IRC operator",
-		)
-
-		return
-	}
-
 	lines := bodyLines()
 
-	var targetNick string
+	targetNick := ""
 	if len(lines) > 0 {
 		targetNick = strings.TrimSpace(lines[0])
 	}
@@ -343,47 +204,22 @@ func (hdlr *Handlers) handleKill(
 		reason = lines[1]
 	}
 
-	targetSID, lookupErr := hdlr.params.Database.
-		GetSessionByNick(ctx, targetNick)
-	if lookupErr != nil {
-		hdlr.respondIRCError(
-			writer, request, clientID, sessionID,
-			irc.ErrNoSuchNick, nick,
-			[]string{targetNick},
-			"No such nick/channel",
-		)
-
-		return
-	}
-
-	// Do not allow killing yourself.
-	if targetSID == sessionID {
-		hdlr.respondIRCError(
-			writer, request, clientID, sessionID,
-			irc.ErrCantKillServer, nick, nil,
-			"You cannot KILL yourself",
-		)
-
-		return
-	}
-
-	quitReason := "Killed (" + nick + " (" + reason + "))"
-
-	// KillSession broadcasts the QUIT, deletes the session
-	// and disconnects the victim's wire connection if it
-	// holds one.
-	hdlr.svc.KillSession(
-		ctx, targetSID, targetNick, quitReason,
+	err := hdlr.svc.KillUser(
+		request.Context(), sessionID, nick,
+		targetNick, reason,
 	)
+	if hdlr.handleServiceError(
+		writer, request, clientID, sessionID, nick, err,
+	) {
+		return
+	}
 
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// handleWallops handles the WALLOPS command.
-// Broadcasts a message to all users with +w usermode
-// (oper only).
+// handleWallops handles WALLOPS: the body is the message.
 func (hdlr *Handlers) handleWallops(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -391,22 +227,6 @@ func (hdlr *Handlers) handleWallops(
 	nick string,
 	bodyLines func() []string,
 ) {
-	ctx := request.Context()
-
-	// Check oper status.
-	isOper, err := hdlr.params.Database.IsSessionOper(
-		ctx, sessionID,
-	)
-	if err != nil || !isOper {
-		hdlr.respondIRCError(
-			writer, request, clientID, sessionID,
-			irc.ErrNoPrivileges, nick, nil,
-			"Permission Denied- You're not an IRC operator",
-		)
-
-		return
-	}
-
 	lines := bodyLines()
 	if len(lines) == 0 {
 		hdlr.respondIRCError(
@@ -419,54 +239,25 @@ func (hdlr *Handlers) handleWallops(
 		return
 	}
 
-	message := strings.Join(lines, " ")
-
-	wallopsSIDs, err := hdlr.params.Database.
-		GetWallopsSessionIDs(ctx)
-	if err != nil {
-		hdlr.log.Error(
-			"get wallops sessions failed", "error", err,
-		)
-		hdlr.respondError(
-			writer, request,
-			"internal error",
-			http.StatusInternalServerError,
-		)
-
+	err := hdlr.svc.SendWallops(
+		request.Context(), sessionID, nick,
+		strings.Join(lines, " "),
+	)
+	if hdlr.handleServiceError(
+		writer, request, clientID, sessionID, nick, err,
+	) {
 		return
 	}
 
-	if len(wallopsSIDs) > 0 {
-		body, mErr := json.Marshal([]string{message})
-		if mErr != nil {
-			hdlr.log.Error(
-				"marshal wallops body", "error", mErr,
-			)
-			hdlr.respondError(
-				writer, request,
-				"internal error",
-				http.StatusInternalServerError,
-			)
-
-			return
-		}
-
-		_ = hdlr.fanOutSilent(
-			request, irc.CmdWallops, nick, "*",
-			json.RawMessage(body), wallopsSIDs,
-		)
-	}
-
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }
 
-// handleUserMode handles user mode queries and changes
-// (e.g., MODE nick, MODE nick +w). Delegates to the
-// shared service.ApplyUserMode / service.QueryUserMode so
-// that mode string processing is identical for both the
-// HTTP API and IRC wire protocol.
+// handleUserMode handles MODE for a nick: a query when the
+// body is empty, otherwise a change by the mode string in
+// the body. Only the caller's own nick, in any letter case,
+// is allowed.
 func (hdlr *Handlers) handleUserMode(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -474,14 +265,7 @@ func (hdlr *Handlers) handleUserMode(
 	nick, target string,
 	bodyLines func() []string,
 ) {
-	ctx := request.Context()
-
-	// Users can only query or change their own modes. The
-	// check is above the query/change split so that both
-	// forms are rejected, and uses EqualFold because IRC
-	// nicks are case-insensitive — matching the wire path
-	// in ircserver.handleUserMode.
-	if target != "" && !strings.EqualFold(target, nick) {
+	if !strings.EqualFold(target, nick) {
 		hdlr.respondIRCError(
 			writer, request, clientID, sessionID,
 			irc.ErrUsersDoNotMatch, nick, nil,
@@ -491,86 +275,33 @@ func (hdlr *Handlers) handleUserMode(
 		return
 	}
 
-	lines := bodyLines()
-
-	// Mode change requested.
-	if len(lines) > 0 {
-		hdlr.changeUserMode(
-			writer, request,
-			sessionID, clientID, nick, lines[0],
-		)
-
-		return
-	}
-
-	// Mode query — delegate to shared service.
-	modeStr, err := hdlr.svc.QueryUserMode(ctx, sessionID)
-	if err != nil {
-		hdlr.log.Error(
-			"query user mode failed", "error", err,
-		)
-		hdlr.respondError(
-			writer, request,
-			"internal error",
-			http.StatusInternalServerError,
-		)
-
-		return
-	}
-
-	hdlr.enqueueNumeric(
-		ctx, clientID, irc.RplUmodeIs, nick, nil,
-		modeStr,
-	)
-	hdlr.broker.Notify(sessionID)
-	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
-		http.StatusOK)
-}
-
-// changeUserMode applies a mode string to the caller's own
-// session. The caller has already verified that the target
-// nick is the caller's own.
-func (hdlr *Handlers) changeUserMode(
-	writer http.ResponseWriter,
-	request *http.Request,
-	sessionID, clientID int64,
-	nick, modeStr string,
-) {
 	ctx := request.Context()
 
-	newModes, err := hdlr.svc.ApplyUserMode(
-		ctx, sessionID, modeStr,
+	var (
+		modes string
+		err   error
 	)
-	if err != nil {
-		var ircErr *service.IRCError
-		if errors.As(err, &ircErr) {
-			hdlr.respondIRCError(
-				writer, request,
-				clientID, sessionID,
-				ircErr.Code, nick, ircErr.Params,
-				ircErr.Message,
-			)
 
-			return
-		}
-
-		hdlr.respondError(
-			writer, request,
-			"internal error",
-			http.StatusInternalServerError,
+	lines := bodyLines()
+	if len(lines) > 0 {
+		modes, err = hdlr.svc.ApplyUserMode(
+			ctx, sessionID, lines[0],
 		)
+	} else {
+		modes, err = hdlr.svc.QueryUserMode(ctx, sessionID)
+	}
 
+	if hdlr.handleServiceError(
+		writer, request, clientID, sessionID, nick, err,
+	) {
 		return
 	}
 
 	hdlr.enqueueNumeric(
-		ctx, clientID, irc.RplUmodeIs, nick, nil,
-		newModes,
+		ctx, clientID, irc.RplUmodeIs, nick, nil, modes,
 	)
-
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{"status": "ok"},
+		map[string]string{statusKey: "ok"},
 		http.StatusOK)
 }

internal/ircserver/commands.go

@@ -13,7 +13,7 @@ import (
 )
 
 // sendIRCError maps a service.IRCError to an IRC numeric
-// reply on the wire.
+// reply on the wire, and logs any other error.
 func (c *Conn) sendIRCError(err error) {
 	var ircErr *service.IRCError
 	if errors.As(err, &ircErr) {
@@ -21,7 +21,11 @@ func (c *Conn) sendIRCError(err error) {
 		args = append(args, ircErr.Params...)
 		args = append(args, ircErr.Message)
 		c.sendNumeric(ircErr.Code, args...)
+
+		return
 	}
+
+	c.log.Error("command failed", "error", err)
 }
 
 // handleCAP silently acknowledges CAP negotiation.
@@ -689,14 +693,14 @@ func (c *Conn) applyChannelModes(
 	}
 }
 
-// handleUserMode handles MODE for users.
+// handleUserMode handles MODE for a nick: a query without a
+// mode string, otherwise a change. Only the client's own
+// nick, in any letter case, is allowed.
 func (c *Conn) handleUserMode(
 	ctx context.Context,
 	msg *Message,
 ) {
-	target := msg.Params[0]
-
-	if !strings.EqualFold(target, c.currentNick()) {
+	if !strings.EqualFold(msg.Params[0], c.currentNick()) {
 		c.sendNumeric(
 			irc.ErrUsersDoNotMatch,
 			"Can't change mode for other users",
@@ -705,48 +709,26 @@ func (c *Conn) handleUserMode(
 		return
 	}
 
-	// Mode query (no mode string).
-	if len(msg.Params) < 2 { //nolint:mnd
-		modes, err := c.svc.QueryUserMode(
-			ctx, c.sessionID,
-		)
-		if err != nil {
-			c.log.Error(
-				"query user mode failed", "error", err,
-			)
-			c.sendNumeric(
-				irc.ErrUmodeUnknownFlag,
-				"Unable to read user modes",
-			)
-
-			return
-		}
-
-		c.sendNumeric(irc.RplUmodeIs, modes)
-
-		return
-	}
-
-	newModes, err := c.svc.ApplyUserMode(
-		ctx, c.sessionID, msg.Params[1],
+	var (
+		modes string
+		err   error
 	)
-	if err != nil {
-		var ircErr *service.IRCError
-		if errors.As(err, &ircErr) {
-			c.sendNumeric(ircErr.Code, ircErr.Message)
 
-			return
-		}
-
-		c.sendNumeric(
-			irc.ErrUmodeUnknownFlag,
-			"Unknown MODE flag",
+	if len(msg.Params) == 1 {
+		modes, err = c.svc.QueryUserMode(ctx, c.sessionID)
+	} else {
+		modes, err = c.svc.ApplyUserMode(
+			ctx, c.sessionID, msg.Params[1],
 		)
+	}
+
+	if err != nil {
+		c.sendIRCError(err)
 
 		return
 	}
 
-	c.sendNumeric(irc.RplUmodeIs, newModes)
+	c.sendNumeric(irc.RplUmodeIs, modes)
 }
 
 // handleNames replies with channel member list.
@@ -1294,224 +1276,114 @@ func (c *Conn) handleInvite(
 	)
 }
 
-// handleUserhost replies with USERHOST info.
+// handleUserhost replies with user@host for each of up to
+// five nicks.
 func (c *Conn) handleUserhost(
 	ctx context.Context,
 	msg *Message,
 ) {
 	if len(msg.Params) < 1 {
+		c.sendNumeric(
+			irc.ErrNeedMoreParams,
+			irc.CmdUserhost, "Not enough parameters",
+		)
+
 		return
 	}
 
-	replies := make([]string, 0, len(msg.Params))
+	reply, err := c.svc.UserhostReply(
+		ctx, msg.Params, c.serverSfx,
+	)
+	if err != nil {
+		c.sendIRCError(err)
 
-	for _, nick := range msg.Params {
-		sid, err := c.database.GetSessionByNick(ctx, nick)
-		if err != nil {
-			continue
-		}
-
-		hostInfo, _ := c.database.GetSessionHostInfo(
-			ctx, sid,
-		)
-
-		host := "*"
-		if hostInfo != nil {
-			host = hostInfo.Hostname
-		}
-
-		isOper, _ := c.database.IsSessionOper(ctx, sid)
-
-		operStar := ""
-		if isOper {
-			operStar = "*"
-		}
-
-		replies = append(
-			replies,
-			nick+operStar+"=+"+nick+"@"+host,
-		)
+		return
 	}
 
-	c.sendNumeric(
-		irc.RplUserHost,
-		strings.Join(replies, " "),
-	)
+	c.sendNumeric(irc.RplUserHost, reply)
 }
 
-// handleVersion replies with the server version string.
+// handleVersion replies with the server version.
 func (c *Conn) handleVersion() {
 	c.sendNumeric(
 		irc.RplVersion,
-		c.svc.ServerVersion()+".", c.serverSfx,
-		"",
+		c.svc.ServerVersion()+".", c.serverSfx, "",
 	)
 }
 
-// handleAdmin replies with server admin info.
+// handleAdmin replies with the server's admin info.
 func (c *Conn) handleAdmin() {
-	srvName := c.serverSfx
-
 	c.sendNumeric(
-		irc.RplAdminMe,
-		srvName, "Administrative info",
-	)
-
-	c.sendNumeric(
-		irc.RplAdminLoc1,
-		"neoirc server",
-	)
-
-	c.sendNumeric(
-		irc.RplAdminLoc2,
-		"IRC over HTTP",
-	)
-
-	c.sendNumeric(
-		irc.RplAdminEmail,
-		"admin@"+srvName,
+		irc.RplAdminMe, c.serverSfx, "Administrative info",
 	)
+	c.sendNumeric(irc.RplAdminLoc1, "neoirc server")
+	c.sendNumeric(irc.RplAdminLoc2, "IRC over HTTP")
+	c.sendNumeric(irc.RplAdminEmail, "admin@"+c.serverSfx)
 }
 
-// handleInfo replies with server software info.
+// handleInfo replies with the server's software info.
 func (c *Conn) handleInfo() {
 	for _, line := range c.svc.InfoLines() {
 		c.sendNumeric(irc.RplInfo, line)
 	}
 
-	c.sendNumeric(
-		irc.RplEndOfInfo,
-		"End of /INFO list",
-	)
+	c.sendNumeric(irc.RplEndOfInfo, "End of /INFO list")
 }
 
-// handleTime replies with the server's current time.
+// handleTime replies with the server's local time.
 func (c *Conn) handleTime() {
-	srvName := c.serverSfx
-
 	c.sendNumeric(
 		irc.RplTime,
-		srvName, time.Now().Format(time.RFC1123),
+		c.serverSfx, time.Now().Format(time.RFC1123),
 	)
 }
 
-// handleKillCmd forcibly disconnects a target user (oper
-// only).
-func (c *Conn) handleKillCmd(
+// handleKill handles KILL <nick> [<reason>].
+func (c *Conn) handleKill(
 	ctx context.Context,
 	msg *Message,
 ) {
-	isOper, err := c.database.IsSessionOper(
-		ctx, c.sessionID,
-	)
-	if err != nil || !isOper {
-		c.sendNumeric(
-			irc.ErrNoPrivileges,
-			"Permission Denied- "+
-				"You're not an IRC operator",
-		)
-
-		return
-	}
-
 	if len(msg.Params) < 1 {
 		c.sendNumeric(
 			irc.ErrNeedMoreParams,
-			"KILL", "Not enough parameters",
+			irc.CmdKill, "Not enough parameters",
 		)
 
 		return
 	}
 
-	targetNick := msg.Params[0]
-
 	reason := "KILLed"
 	if len(msg.Params) > 1 {
 		reason = msg.Params[1]
 	}
 
-	killerNick := c.currentNick()
-
-	if strings.EqualFold(targetNick, killerNick) {
-		c.sendNumeric(
-			irc.ErrCantKillServer,
-			"You cannot KILL yourself",
-		)
-
-		return
-	}
-
-	targetSID, lookupErr := c.database.GetSessionByNick(
-		ctx, targetNick,
+	err := c.svc.KillUser(
+		ctx, c.sessionID, c.currentNick(),
+		msg.Params[0], reason,
 	)
-	if lookupErr != nil {
-		c.sendNumeric(
-			irc.ErrNoSuchNick,
-			targetNick, "No such nick/channel",
-		)
-
-		return
+	if err != nil {
+		c.sendIRCError(err)
 	}
-
-	quitReason := "Killed (" + killerNick +
-		" (" + reason + "))"
-
-	// KillSession broadcasts the QUIT, deletes the session
-	// and disconnects the victim's wire connection.
-	c.svc.KillSession(
-		ctx, targetSID, targetNick, quitReason,
-	)
 }
 
-// handleWallopsCmd broadcasts to all +w users (oper only).
-func (c *Conn) handleWallopsCmd(
+// handleWallops handles WALLOPS <message>.
+func (c *Conn) handleWallops(
 	ctx context.Context,
 	msg *Message,
 ) {
-	isOper, err := c.database.IsSessionOper(
-		ctx, c.sessionID,
-	)
-	if err != nil || !isOper {
-		c.sendNumeric(
-			irc.ErrNoPrivileges,
-			"Permission Denied- "+
-				"You're not an IRC operator",
-		)
-
-		return
-	}
-
 	if len(msg.Params) < 1 {
 		c.sendNumeric(
 			irc.ErrNeedMoreParams,
-			"WALLOPS", "Not enough parameters",
+			irc.CmdWallops, "Not enough parameters",
 		)
 
 		return
 	}
 
-	message := msg.Params[0]
-
-	wallopsSIDs, wallErr := c.database.
-		GetWallopsSessionIDs(ctx)
-	if wallErr != nil {
-		c.log.Error(
-			"get wallops sessions failed",
-			"error", wallErr,
-		)
-
-		return
-	}
-
-	if len(wallopsSIDs) > 0 {
-		body, mErr := json.Marshal([]string{message})
-		if mErr != nil {
-			return
-		}
-
-		_, _, _ = c.svc.FanOut(
-			ctx, irc.CmdWallops, c.currentNick(), "*",
-			nil, body, nil, wallopsSIDs,
-		)
+	err := c.svc.SendWallops(
+		ctx, c.sessionID, c.currentNick(), msg.Params[0],
+	)
+	if err != nil {
+		c.sendIRCError(err)
 	}
 }

internal/ircserver/conn.go

@@ -47,10 +47,9 @@ type Conn struct {
 	serverSfx string
 	commands  map[string]cmdHandler
 
-	// writeMu serializes writes to conn.  A connection is
-	// written to by its own read loop, by its relay
-	// goroutine, and — when an operator KILLs it — by
-	// another client's goroutine.
+	// writeMu serializes writes to conn, which come from
+	// serve(), from the relay goroutine and, for KILL, from
+	// Disconnect.
 	writeMu sync.Mutex
 
 	mu        sync.Mutex
@@ -141,7 +140,7 @@ func (c *Conn) Disconnect(reason string) {
 func (c *Conn) notifyKilledAndClose(
 	nick, host, reason string,
 ) {
-	defer c.conn.Close() //nolint:errcheck
+	defer func() { _ = c.conn.Close() }()
 
 	c.sendWithin(
 		killWriteWindow,
@@ -188,13 +187,13 @@ func (c *Conn) buildCommandMap() map[string]cmdHandler {
 		"CAP": func(_ context.Context, msg *Message) {
 			c.handleCAP(msg)
 		},
-		"USERHOST":     c.handleUserhost,
-		irc.CmdVersion: func(context.Context, *Message) { c.handleVersion() },
-		irc.CmdAdmin:   func(context.Context, *Message) { c.handleAdmin() },
-		irc.CmdInfo:    func(context.Context, *Message) { c.handleInfo() },
-		irc.CmdTime:    func(context.Context, *Message) { c.handleTime() },
-		irc.CmdKill:    c.handleKillCmd,
-		irc.CmdWallops: c.handleWallopsCmd,
+		irc.CmdUserhost: c.handleUserhost,
+		irc.CmdVersion:  func(context.Context, *Message) { c.handleVersion() },
+		irc.CmdAdmin:    func(context.Context, *Message) { c.handleAdmin() },
+		irc.CmdInfo:     func(context.Context, *Message) { c.handleInfo() },
+		irc.CmdTime:     func(context.Context, *Message) { c.handleTime() },
+		irc.CmdKill:     c.handleKill,
+		irc.CmdWallops:  c.handleWallops,
 	}
 }
 
@@ -263,9 +262,8 @@ func (c *Conn) cleanup(ctx context.Context) {
 	if wasRegistered && sessID > 0 {
 		c.svc.UnregisterWireConn(sessID, c)
 
-		// A KILLed session has already been broadcast and
-		// deleted by the killer; broadcasting again would
-		// fan out a QUIT for a session row that is gone.
+		// KILL has already sent the QUIT and deleted the
+		// session.
 		if !wasKilled {
 			c.svc.BroadcastQuit(
 				ctx, sessID, nick, "Connection closed",
@@ -276,8 +274,8 @@ func (c *Conn) cleanup(ctx context.Context) {
 	c.conn.Close() //nolint:errcheck,gosec
 }
 
-// isClosed reports whether the connection has been marked
-// for teardown, either by QUIT or by an operator KILL.
+// isClosed reports whether QUIT or KILL has ended the
+// connection.
 func (c *Conn) isClosed() bool {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -285,9 +283,7 @@ func (c *Conn) isClosed() bool {
 	return c.closed
 }
 
-// currentNick returns the connection's registered nick.
-// c.nick is written under c.mu during registration and
-// NICK changes, so every read must take the mutex.
+// currentNick returns c.nick, read under c.mu.
 func (c *Conn) currentNick() string {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -300,10 +296,8 @@ func (c *Conn) send(line string) {
 	c.sendWithin(writeTimeout, line)
 }
 
-// sendWithin writes a formatted IRC line to the connection
-// under the given write deadline.  Callers that must not be
-// held hostage by an unresponsive peer pass a shorter window
-// than writeTimeout.
+// sendWithin writes a formatted IRC line to the connection,
+// giving up after timeout.
 func (c *Conn) sendWithin(
 	timeout time.Duration,
 	line string,
@@ -507,8 +501,7 @@ func (c *Conn) completeRegistration(ctx context.Context) {
 	c.registered = true
 	c.mu.Unlock()
 
-	// Make this connection reachable by session ID so that
-	// KILL from either transport can disconnect it.
+	// So that KILL, from either transport, can close it.
 	c.svc.RegisterWireConn(sessionID, c)
 
 	// If PASS was provided before registration, set the

internal/ircserver/conn_test.go

@@ -13,43 +13,37 @@ import (
 	"sneak.berlin/go/neoirc/internal/ircserver"
 )
 
-// disconnectBudget is how long Disconnect is allowed to
-// take when the victim never reads.  It is far below the
-// 30s writeTimeout that the old synchronous implementation
-// would have burned on each of its two writes.
-const disconnectBudget = 2 * time.Second
+// newKillVictim returns a Conn for the nick victim whose
+// socket is one end of a net.Pipe, and the other end, from
+// which the test reads what the victim is sent. A write to
+// the pipe blocks until the other end reads.
+func newKillVictim(t *testing.T) (*ircserver.Conn, net.Conn) {
+	t.Helper()
 
-// TestDisconnectDoesNotBlockOnUnresponsiveVictim proves
-// that an operator KILL cannot be stalled by its target.
-// The victim's socket is a net.Pipe, so every write blocks
-// until the peer reads and the peer here never does.  The
-// old implementation performed both notification writes on
-// the killer's goroutine, which wedged the operator's own
-// serve() loop on the IRC path and the API request on the
-// HTTP path for as long as the victim cared to stay silent.
+	serverSide, clientSide := net.Pipe()
+
+	t.Cleanup(func() { _ = clientSide.Close() })
+
+	log := slog.New(slog.NewTextHandler(
+		os.Stderr,
+		&slog.HandlerOptions{Level: slog.LevelError},
+	))
+	cfg := &config.Config{ServerName: testServerName}
+
+	return ircserver.NewTestConn(
+		log, cfg, serverSide, "victim",
+	), clientSide
+}
+
+// TestDisconnectDoesNotBlockOnUnresponsiveVictim checks
+// that Disconnect returns although the victim never reads,
+// so a KILL cannot stall the operator who sent it.
 func TestDisconnectDoesNotBlockOnUnresponsiveVictim(
 	t *testing.T,
 ) {
 	t.Parallel()
 
-	serverSide, clientSide := net.Pipe()
-
-	t.Cleanup(func() {
-		_ = clientSide.Close()
-	})
-
-	log := slog.New(slog.NewTextHandler(
-		os.Stderr,
-		&slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct
-	))
-	cfg := &config.Config{ //nolint:exhaustruct
-		ServerName: "test.irc",
-	}
-
-	victim := ircserver.NewTestConn(
-		log, cfg, serverSide, "victim",
-	)
-
+	victim, _ := newKillVictim(t)
 	returned := make(chan struct{})
 
 	go func() {
@@ -59,66 +53,44 @@ func TestDisconnectDoesNotBlockOnUnresponsiveVictim(
 
 	select {
 	case <-returned:
-	case <-time.After(disconnectBudget):
-		t.Fatal(
-			"Disconnect blocked on the victim's socket; " +
-				"the killer must not be held hostage",
-		)
+	case <-time.After(testTimeout):
+		t.Fatal("Disconnect waited for the victim to read")
 	}
 }
 
-// TestDisconnectNotifiesAndClosesVictim is the other half
-// of the contract: moving the notification off the killer's
-// goroutine must not lose it.  A victim that does read gets
-// both the KILL and the ERROR line, and then its socket is
-// closed so its read loop unblocks.
+// TestDisconnectNotifiesAndClosesVictim checks that a
+// victim that reads is sent KILL and ERROR, and that its
+// connection is then closed.
 func TestDisconnectNotifiesAndClosesVictim(t *testing.T) {
 	t.Parallel()
 
-	serverSide, clientSide := net.Pipe()
-
-	t.Cleanup(func() {
-		_ = clientSide.Close()
-	})
-
-	log := slog.New(slog.NewTextHandler(
-		os.Stderr,
-		&slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct
-	))
-	cfg := &config.Config{ //nolint:exhaustruct
-		ServerName: "test.irc",
-	}
-
-	victim := ircserver.NewTestConn(
-		log, cfg, serverSide, "victim",
-	)
-
-	lines := make(chan []string, 1)
+	victim, clientSide := newKillVictim(t)
+	received := make(chan []string, 1)
 
 	go func() {
-		var got []string
+		var lines []string
 
 		scanner := bufio.NewScanner(clientSide)
 		for scanner.Scan() {
-			got = append(got, scanner.Text())
+			lines = append(lines, scanner.Text())
 		}
 
-		lines <- got
+		received <- lines
 	}()
 
 	victim.Disconnect("killed by oper")
 
-	var got []string
+	var lines []string
 
 	select {
-	case got = <-lines:
-	case <-time.After(5 * time.Second):
-		t.Fatal("victim socket was never closed")
+	case lines = <-received:
+	case <-time.After(testTimeout):
+		t.Fatal("the victim's connection was not closed")
 	}
 
-	joined := strings.Join(got, "\n")
+	joined := strings.Join(lines, "\n")
 
-	if !strings.Contains(joined, "KILL victim") {
+	if !strings.Contains(joined, "KILL victim :killed by oper") {
 		t.Errorf("missing KILL line, got: %q", joined)
 	}
 

internal/ircserver/export_test.go

@@ -56,9 +56,8 @@ func (s *Server) Listener() net.Listener {
 	return s.listener
 }
 
-// NewTestConn wraps an already-established net.Conn in a
-// Conn so tests can drive connection-level behaviour such
-// as Disconnect without standing up a whole server.
+// NewTestConn returns a Conn for nick on tcpConn, without a
+// server, database or service behind it.
 func NewTestConn(
 	log *slog.Logger,
 	cfg *config.Config,

internal/ircserver/integration_test.go

@@ -4,6 +4,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"sneak.berlin/go/neoirc/internal/config"
 )
 
 // TestIntegrationTwoClients is a comprehensive integration
@@ -777,39 +779,28 @@ func TestIntegrationUserhost(t *testing.T) {
 	bob := env.dial(t)
 	bob.register("bob")
 
-	// Query single nick.
-	alice.send("USERHOST bob")
-
-	aliceReply := alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 302 ")
+	bob.send("AWAY :lunch")
+	bob.readUntil(func(l string) bool {
+		return strings.Contains(l, " 306 ")
 	})
-	assertContains(
-		t, aliceReply, " 302 ",
-		"RPL_USERHOST",
-	)
-	assertContains(
-		t, aliceReply, "bob",
-		"USERHOST contains queried nick",
-	)
 
-	// Query multiple nicks.
-	bob.send("USERHOST alice bob")
+	for _, step := range []struct {
+		line, want string
+	}{
+		{"USERHOST alice", " 302 alice alice=+alice@"},
+		{"USERHOST alice bob", " :alice=+alice@"},
+		{"USERHOST alice bob", " bob=-bob@"},
+		{"USERHOST nobody", " 302 alice :"},
+		{"USERHOST", " 461 alice USERHOST :"},
+	} {
+		alice.send(step.line)
 
-	bobReply := bob.readUntil(func(l string) bool {
-		return strings.Contains(l, " 302 ")
-	})
-	assertContains(
-		t, bobReply, " 302 ",
-		"RPL_USERHOST multi-nick",
-	)
-	assertContains(
-		t, bobReply, "alice",
-		"USERHOST multi contains alice",
-	)
-	assertContains(
-		t, bobReply, "bob",
-		"USERHOST multi contains bob",
-	)
+		reply := alice.readUntil(func(l string) bool {
+			return strings.Contains(l, " 302 ") ||
+				strings.Contains(l, " 461 ")
+		})
+		assertContains(t, reply, step.want, step.line)
+	}
 }
 
 // TestIntegrationVersion verifies the VERSION command
@@ -919,17 +910,54 @@ func TestIntegrationTime(t *testing.T) {
 		"RPL_TIME",
 	)
 	assertContains(
-		t, aliceReply, "test.irc",
+		t, aliceReply, testServerName,
 		"TIME reply includes server name",
 	)
 }
 
-// TestIntegrationKill verifies the KILL command: oper can
-// kill a user, non-oper cannot, and — most importantly —
-// that the victim is actually disconnected rather than
-// merely announced as having quit.
-//
-//nolint:funlen // one KILL scenario asserted end to end
+// becomeOper sends OPER with newTestEnvWithOper's
+// credentials and waits for RPL_YOUREOPER.
+func (tc *testClient) becomeOper() {
+	tc.t.Helper()
+
+	tc.send("OPER testoper testpass")
+	tc.readUntil(func(l string) bool {
+		return strings.Contains(l, " 381 ")
+	})
+}
+
+// TestIntegrationKillRefused covers the KILL errors: not an
+// operator, no such nick, and killing yourself.
+func TestIntegrationKillRefused(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnvWithOper(t)
+
+	alice := env.dial(t)
+	alice.register("alice")
+
+	bob := env.dial(t)
+	bob.register("bob")
+
+	for _, step := range []struct {
+		line, numeric string
+	}{
+		{"KILL bob :nope", " 481 "},
+		{"OPER testoper testpass", " 381 "},
+		{"KILL nobody123 :gone", " 401 "},
+		{"KILL alice :me", " 483 "},
+	} {
+		alice.send(step.line)
+
+		reply := alice.readUntil(func(l string) bool {
+			return strings.Contains(l, step.numeric)
+		})
+		assertContains(t, reply, step.numeric, step.line)
+	}
+}
+
+// TestIntegrationKill checks that the victim of a KILL is
+// told why, is disconnected, and is gone from its channels.
 func TestIntegrationKill(t *testing.T) {
 	t.Parallel()
 
@@ -941,7 +969,6 @@ func TestIntegrationKill(t *testing.T) {
 	bob := env.dial(t)
 	bob.register("bob")
 
-	// Both join a channel so KILL's QUIT is visible.
 	alice.joinAndDrain("#killtest")
 	bob.joinAndDrain("#killtest")
 
@@ -951,30 +978,9 @@ func TestIntegrationKill(t *testing.T) {
 			strings.Contains(l, "bob")
 	})
 
-	// Non-oper KILL should fail.
-	alice.send("KILL bob :nope")
-
-	aliceKillFail := alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 481 ")
-	})
-	assertContains(
-		t, aliceKillFail, " 481 ",
-		"ERR_NOPRIVILEGES for non-oper KILL",
-	)
-
-	// alice becomes oper.
-	alice.send("OPER testoper testpass")
-	alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 381 ")
-	})
-
-	// Oper KILL should succeed.
+	alice.becomeOper()
 	alice.send("KILL bob :bad behavior")
 
-	// The victim must be told why and then disconnected.
-	// Reading to EOF is the assertion that matters: a KILL
-	// that only broadcasts a QUIT leaves bob holding a
-	// socket that looks alive but delivers nothing.
 	bobLines := bob.readUntilClosed()
 	assertContains(
 		t, bobLines, "KILL",
@@ -1024,17 +1030,6 @@ func TestIntegrationKill(t *testing.T) {
 		t, aliceWho, "bob",
 		"killed user must not appear in WHO",
 	)
-
-	// KILL nonexistent nick.
-	alice.send("KILL nobody123 :gone")
-
-	aliceNoSuch := alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 401 ")
-	})
-	assertContains(
-		t, aliceNoSuch, " 401 ",
-		"ERR_NOSUCHNICK for KILL missing target",
-	)
 }
 
 // TestIntegrationWallops verifies the WALLOPS command:
@@ -1061,11 +1056,7 @@ func TestIntegrationWallops(t *testing.T) {
 		"ERR_NOPRIVILEGES for non-oper WALLOPS",
 	)
 
-	// alice becomes oper.
-	alice.send("OPER testoper testpass")
-	alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 381 ")
-	})
+	alice.becomeOper()
 
 	// bob sets +w to receive wallops.
 	bob.send("MODE bob +w")
@@ -1092,6 +1083,52 @@ func TestIntegrationWallops(t *testing.T) {
 	)
 }
 
+// TestIntegrationUserMode covers MODE for a nick: changes
+// and queries of your own modes in any letter case, refusal
+// for another nick, and a rejected mode string changing
+// nothing.
+func TestIntegrationUserMode(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	alice := env.dial(t)
+	alice.register("alice")
+
+	bob := env.dial(t)
+	bob.register("bob")
+
+	const (
+		notYours = " 502 alice :Can't change mode for other users"
+		unknown  = " 501 alice :Unknown MODE flag"
+	)
+
+	for _, step := range []struct {
+		line, want string
+	}{
+		{"MODE ALICE +w", " 221 alice +w"},
+		{"MODE bob", notYours},
+		{"MODE bob -w", notYours},
+		{"MODE alice xw", unknown},
+		{"MODE alice -w+z", unknown},
+		{"MODE alice", " 221 alice +w"},
+		{"MODE alice +w-w", " 221 alice +"},
+	} {
+		alice.send(step.line)
+
+		reply := alice.readUntil(func(l string) bool {
+			return strings.Contains(l, " 221 ") ||
+				strings.Contains(l, " 501 ") ||
+				strings.Contains(l, " 502 ")
+		})
+
+		last := reply[len(reply)-1]
+		if !strings.HasSuffix(last, step.want) {
+			t.Errorf("%s: want %q, got %q", step.line, step.want, last)
+		}
+	}
+}
+
 // TestIntegrationModeSecret tests +s (secret) channel
 // mode — verifies that +s can be set and the mode is
 // reflected in MODE queries.
@@ -1246,79 +1283,29 @@ func TestIntegrationThirdClientObserver(t *testing.T) {
 	)
 }
 
-// assertNoEmptyParam fails if any line contains a doubled
-// space, which is what FormatMessage emits for an empty
-// non-trailing parameter.  A numeric whose server-name
-// parameter came out empty is malformed on the wire even
-// though it still contains the numeric code, so the other
-// assertions in this file would not catch it.
-func assertNoEmptyParam(
-	t *testing.T,
-	lines []string,
-	context string,
-) {
-	t.Helper()
-
-	for _, line := range lines {
-		if strings.Contains(line, "  ") {
-			t.Errorf(
-				"%s: empty parameter in wire line: %q",
-				context, line,
-			)
-		}
-	}
-}
-
-// TestIntegrationDefaultServerNameFallback runs the wire
-// server with SERVER_NAME unset, which is the shipped
-// default from config.go, and verifies that VERSION, ADMIN
-// and TIME fall back to "neoirc" exactly as the HTTP path
-// does instead of emitting an empty server-name parameter.
-//
-// Every other wire test hardcodes ServerName: "test.irc",
-// so none of them exercise the default configuration.
+// TestIntegrationDefaultServerNameFallback checks that with
+// SERVER_NAME unset, as it is by default, VERSION, ADMIN and
+// TIME name the server "neoirc".
 func TestIntegrationDefaultServerNameFallback(t *testing.T) {
 	t.Parallel()
 
-	env := newTestEnvWithServerName(t, "")
+	env := newTestEnvWithConfig(t, &config.Config{})
 
 	alice := env.dial(t)
 	alice.register("alice")
 
-	alice.send("VERSION")
+	for _, step := range []struct {
+		line, lastNumeric, want string
+	}{
+		{"VERSION", " 351 ", " 351 alice neoirc-test. neoirc "},
+		{"ADMIN", " 259 ", " 256 alice neoirc "},
+		{"TIME", " 391 ", " 391 alice neoirc "},
+	} {
+		alice.send(step.line)
 
-	versionReply := alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 351 ")
-	})
-	assertNoEmptyParam(t, versionReply, "VERSION")
-	assertContains(
-		t, versionReply, "neoirc",
-		"VERSION falls back to default server name",
-	)
-
-	alice.send("ADMIN")
-
-	adminReply := alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 259 ")
-	})
-	assertNoEmptyParam(t, adminReply, "ADMIN")
-	assertContains(
-		t, adminReply, " 256 alice neoirc ",
-		"RPL_ADMINME names the default server",
-	)
-	assertContains(
-		t, adminReply, "admin@neoirc",
-		"RPL_ADMINEMAIL is a well-formed address",
-	)
-
-	alice.send("TIME")
-
-	timeReply := alice.readUntil(func(l string) bool {
-		return strings.Contains(l, " 391 ")
-	})
-	assertNoEmptyParam(t, timeReply, "TIME")
-	assertContains(
-		t, timeReply, " 391 alice neoirc ",
-		"RPL_TIME names the default server",
-	)
+		reply := alice.readUntil(func(l string) bool {
+			return strings.Contains(l, step.lastNumeric)
+		})
+		assertContains(t, reply, step.want, step.line)
+	}
 }

internal/ircserver/server_test.go

@@ -36,20 +36,36 @@ type testEnv struct {
 	srv      *ircserver.Server
 }
 
+// testServerName is the SERVER_NAME of newTestEnv.
+const testServerName = "test.irc"
+
 func newTestEnv(t *testing.T) *testEnv {
 	t.Helper()
 
-	return newTestEnvWithServerName(t, "test.irc")
+	return newTestEnvWithConfig(t, &config.Config{
+		ServerName: testServerName,
+		MOTD:       "Welcome to test IRC",
+	})
 }
 
-// newTestEnvWithServerName creates a test environment with
-// an explicit SERVER_NAME. Passing "" exercises the shipped
-// default from config.go, under which the server must fall
-// back to "neoirc" rather than emitting an empty
-// server-name parameter.
-func newTestEnvWithServerName(
+// newTestEnvWithOper is newTestEnv with the operator name
+// testoper and password testpass.
+func newTestEnvWithOper(t *testing.T) *testEnv {
+	t.Helper()
+
+	return newTestEnvWithConfig(t, &config.Config{
+		ServerName:   testServerName,
+		MOTD:         "Welcome to test IRC",
+		OperName:     "testoper",
+		OperPassword: "testpass",
+	})
+}
+
+// newTestEnvWithConfig starts an IRC server with cfg on a
+// fresh database.
+func newTestEnvWithConfig(
 	t *testing.T,
-	serverName string,
+	cfg *config.Config,
 ) *testEnv {
 	t.Helper()
 
@@ -81,11 +97,6 @@ func newTestEnvWithServerName(
 
 	brk := broker.New()
 
-	cfg := &config.Config{ //nolint:exhaustruct
-		ServerName: serverName,
-		MOTD:       "Welcome to test IRC",
-	}
-
 	var listenConfig net.ListenConfig
 
 	listener, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0")
@@ -129,87 +140,6 @@ func newTestEnvWithServerName(
 	}
 }
 
-// newTestEnvWithOper creates a test environment with oper
-// credentials configured.
-func newTestEnvWithOper(t *testing.T) *testEnv {
-	t.Helper()
-
-	dsn := fmt.Sprintf(
-		"file:%s?mode=memory&cache=shared&_journal_mode=WAL",
-		t.Name(),
-	)
-
-	conn, err := sql.Open("sqlite", dsn)
-	if err != nil {
-		t.Fatalf("open db: %v", err)
-	}
-
-	conn.SetMaxOpenConns(1)
-
-	_, err = conn.ExecContext(
-		t.Context(), "PRAGMA foreign_keys = ON",
-	)
-	if err != nil {
-		t.Fatalf("pragma: %v", err)
-	}
-
-	database := db.NewTestDatabaseFromConn(conn)
-
-	err = database.RunMigrations(t.Context())
-	if err != nil {
-		t.Fatalf("migrate: %v", err)
-	}
-
-	brk := broker.New()
-
-	cfg := &config.Config{ //nolint:exhaustruct
-		ServerName:   "test.irc",
-		MOTD:         "Welcome to test IRC",
-		OperName:     "testoper",
-		OperPassword: "testpass",
-	}
-
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatalf("listen: %v", err)
-	}
-
-	addr := listener.Addr().String()
-
-	err = listener.Close()
-	if err != nil {
-		t.Fatalf("close listener: %v", err)
-	}
-
-	log := slog.New(slog.NewTextHandler(
-		os.Stderr,
-		&slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct
-	))
-
-	srv := ircserver.NewTestServer(log, cfg, database, brk)
-
-	err = srv.Start(addr)
-	if err != nil {
-		t.Fatalf("start irc server: %v", err)
-	}
-
-	t.Cleanup(func() {
-		srv.Stop()
-
-		err := conn.Close()
-		if err != nil {
-			t.Logf("close db: %v", err)
-		}
-	})
-
-	return &testEnv{
-		database: database,
-		brk:      brk,
-		cfg:      cfg,
-		srv:      srv,
-	}
-}
-
 // dial connects to the test server.
 func (env *testEnv) dial(t *testing.T) *testClient {
 	t.Helper()
@@ -310,11 +240,9 @@ func (tc *testClient) register(nick string) []string {
 	})
 }
 
-// readUntilClosed reads until the peer closes the
-// connection, returning the lines received first.  It
-// fails the test if the connection is still open when the
-// read deadline expires, which is what a KILL that never
-// terminates the victim's socket looks like.
+// readUntilClosed returns the lines received until the
+// server closes the connection, and fails the test if it is
+// still open after testTimeout.
 func (tc *testClient) readUntilClosed() []string {
 	tc.t.Helper()
 
@@ -331,8 +259,7 @@ func (tc *testClient) readUntilClosed() []string {
 	err := tc.scanner.Err()
 	if err != nil {
 		tc.t.Fatalf(
-			"expected EOF on victim socket, got %v "+
-				"(lines: %v)",
+			"connection not closed: %v (lines: %v)",
 			err, lines,
 		)
 	}

internal/service/service.go

@@ -5,7 +5,9 @@ package service
 import (
 	"context"
 	"crypto/subtle"
+	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"strconv"
@@ -25,9 +27,14 @@ import (
 // Error texts that several commands reply with.
 const (
 	msgNoSuchChannel = "No such channel"
+	msgNoSuchNick    = "No such nick/channel"
 	msgNotChannelOp  = "You're not channel operator"
 )
 
+// maxUserhostNicks is how many nicks one USERHOST answers
+// for (RFC 2812).
+const maxUserhostNicks = 5
+
 // Params defines the dependencies for creating a Service.
 type Params struct {
 	fx.In
@@ -39,14 +46,11 @@ type Params struct {
 	Globals  *globals.Globals
 }
 
-// WireConn is a live client connection that a transport
-// registers with the service so that commands such as KILL
-// can reach it.  The IRC wire server registers one per
-// registered connection; the HTTP transport has no
-// long-lived socket and registers nothing.
+// WireConn is a registered IRC connection, which KILL closes.
+// HTTP clients hold no connection and register none.
 type WireConn interface {
-	// Disconnect terminates the connection, telling the
-	// client why before closing the socket.
+	// Disconnect tells the client why and closes the
+	// connection.
 	Disconnect(reason string)
 }
 
@@ -64,7 +68,7 @@ type Service struct {
 
 // New creates a new Service.
 func New(params Params) *Service {
-	return &Service{ //nolint:exhaustruct // mutex zero value
+	return &Service{
 		db:        params.Database,
 		broker:    params.Broker,
 		config:    params.Config,
@@ -83,7 +87,7 @@ func NewTestService(
 	globs *globals.Globals,
 	log *slog.Logger,
 ) *Service {
-	return &Service{ //nolint:exhaustruct // mutex zero value
+	return &Service{
 		db:        database,
 		broker:    brk,
 		config:    cfg,
@@ -93,46 +97,81 @@ func NewTestService(
 	}
 }
 
-// ServerVersion returns the canonical server version string
-// used by every transport, e.g. "neoirc-1.2.3".  Both the
-// IRC wire protocol and the HTTP API must report the same
-// string, so this is the only place it is built.
+// ServerVersion returns the version that VERSION and INFO
+// report on both transports, such as "neoirc-1.2.3".
 func (s *Service) ServerVersion() string {
-	name := "neoirc"
-	ver := "dev"
-
-	if s.globals != nil {
-		if s.globals.Appname != "" {
-			name = s.globals.Appname
-		}
-
-		if s.globals.Version != "" {
-			ver = s.globals.Version
-		}
+	version := s.globals.Version
+	if version == "" {
+		version = "dev"
 	}
 
-	return name + "-" + ver
+	return s.globals.Appname + "-" + version
 }
 
-// InfoLines returns the RPL_INFO body.  Both transports
-// send exactly these lines so that INFO does not diverge
-// between the wire protocol and the HTTP API.
+// InfoLines returns the lines that INFO sends on both
+// transports.
 func (s *Service) InfoLines() []string {
-	started := "unknown"
-	if s.globals != nil && !s.globals.StartTime.IsZero() {
-		started = s.globals.StartTime.Format(time.RFC1123)
-	}
-
 	return []string{
 		"neoirc — IRC semantics over HTTP",
 		"Version: " + s.ServerVersion(),
 		"Written in Go",
-		"Started: " + started,
+		"Started: " + s.globals.StartTime.Format(time.RFC1123),
 	}
 }
 
-// RegisterWireConn associates a live wire connection with
-// its session ID so that KillSession can reach it.
+// UserhostReply returns the RPL_USERHOST text for the first
+// five of the given nicks: "nick=+user@host" entries joined
+// by spaces, with * after the nick of an operator and - in
+// place of + for a user who is away. Nicks with no session
+// are left out. An empty hostname is reported as
+// serverName.
+func (s *Service) UserhostReply(
+	ctx context.Context,
+	nicks []string,
+	serverName string,
+) (string, error) {
+	if len(nicks) > maxUserhostNicks {
+		nicks = nicks[:maxUserhostNicks]
+	}
+
+	infos, err := s.db.GetUserhostInfo(ctx, nicks)
+	if err != nil {
+		return "", fmt.Errorf("userhost: %w", err)
+	}
+
+	replies := make([]string, 0, len(infos))
+
+	for _, info := range infos {
+		operStar := ""
+		if info.IsOper {
+			operStar = "*"
+		}
+
+		away := "+"
+		if info.AwayMessage != "" {
+			away = "-"
+		}
+
+		username := info.Username
+		if username == "" {
+			username = info.Nick
+		}
+
+		hostname := info.Hostname
+		if hostname == "" {
+			hostname = serverName
+		}
+
+		replies = append(replies,
+			info.Nick+operStar+"="+away+username+"@"+hostname,
+		)
+	}
+
+	return strings.Join(replies, " "), nil
+}
+
+// RegisterWireConn records the IRC connection of a session,
+// so that KillUser can close it.
 func (s *Service) RegisterWireConn(
 	sessionID int64,
 	conn WireConn,
@@ -143,9 +182,9 @@ func (s *Service) RegisterWireConn(
 	s.wireConns[sessionID] = conn
 }
 
-// UnregisterWireConn removes the association created by
-// RegisterWireConn.  It is a no-op if the session has
-// already been rebound to a different connection.
+// UnregisterWireConn removes what RegisterWireConn recorded,
+// unless the session has since registered another
+// connection.
 func (s *Service) UnregisterWireConn(
 	sessionID int64,
 	conn WireConn,
@@ -158,28 +197,90 @@ func (s *Service) UnregisterWireConn(
 	}
 }
 
-// KillSession forcibly removes a user from the server: the
-// victim's channel peers are told via QUIT, the victim's
-// session is deleted, and any live wire connection it holds
-// is disconnected.  Both the IRC KILL command and the HTTP
-// KILL endpoint route through here so the two transports
-// cannot diverge.
-func (s *Service) KillSession(
+// KillUser carries out an operator's KILL: the target's
+// channel peers see it quit, its session is deleted, and
+// its IRC connection, if it has one, is closed.
+func (s *Service) KillUser(
 	ctx context.Context,
 	sessionID int64,
-	nick, reason string,
-) {
-	s.BroadcastQuit(ctx, sessionID, nick, reason)
+	nick, targetNick, reason string,
+) error {
+	err := s.requireOper(ctx, sessionID)
+	if err != nil {
+		return err
+	}
+
+	targetSID, err := s.db.GetSessionByNick(ctx, targetNick)
+	if errors.Is(err, sql.ErrNoRows) {
+		return &IRCError{
+			irc.ErrNoSuchNick,
+			[]string{targetNick},
+			msgNoSuchNick,
+		}
+	}
+
+	if err != nil {
+		return fmt.Errorf("kill: %w", err)
+	}
+
+	if targetSID == sessionID {
+		return &IRCError{
+			irc.ErrCantKillServer,
+			nil,
+			"You cannot KILL yourself",
+		}
+	}
+
+	quitReason := "Killed (" + nick + " (" + reason + "))"
+
+	s.BroadcastQuit(ctx, targetSID, targetNick, quitReason)
 
-	// A session with no registered wire connection (an
-	// HTTP-only client) has nothing left to disconnect.
 	s.wireMu.Lock()
-	conn := s.wireConns[sessionID]
+	conn := s.wireConns[targetSID]
 	s.wireMu.Unlock()
 
 	if conn != nil {
-		conn.Disconnect(reason)
+		conn.Disconnect(quitReason)
 	}
+
+	return nil
+}
+
+// SendWallops carries out an operator's WALLOPS: the
+// message goes to every user with user mode +w.
+func (s *Service) SendWallops(
+	ctx context.Context,
+	sessionID int64,
+	nick, message string,
+) error {
+	err := s.requireOper(ctx, sessionID)
+	if err != nil {
+		return err
+	}
+
+	recipients, err := s.db.GetWallopsSessionIDs(ctx)
+	if err != nil {
+		return fmt.Errorf("wallops: %w", err)
+	}
+
+	if len(recipients) == 0 {
+		return nil
+	}
+
+	body, err := json.Marshal([]string{message})
+	if err != nil {
+		return fmt.Errorf("wallops: %w", err)
+	}
+
+	_, _, err = s.FanOut(
+		ctx, irc.CmdWallops, nick, "*",
+		nil, body, nil, recipients,
+	)
+	if err != nil {
+		return fmt.Errorf("wallops: %w", err)
+	}
+
+	return nil
 }
 
 // IRCError represents an IRC protocol-level error with a
@@ -550,7 +651,7 @@ func (s *Service) KickUser(
 		return &IRCError{
 			irc.ErrNoSuchNick,
 			[]string{targetNick},
-			"No such nick/channel",
+			msgNoSuchNick,
 		}
 	}
 
@@ -762,7 +863,7 @@ func (s *Service) ApplyMemberMode(
 		return &IRCError{
 			irc.ErrNoSuchNick,
 			[]string{targetNick},
-			"No such nick/channel",
+			msgNoSuchNick,
 		}
 	}
 
@@ -905,12 +1006,8 @@ func (s *Service) QueryChannelMode(
 	return modes + modeParams
 }
 
-// QueryUserMode returns the current user mode string for
-// the given session (e.g. "+ow", "+w", "+").  A database
-// failure is returned rather than being reported as an
-// unset flag: an unreadable mode is not the same as an
-// absent one, and reporting "+" for either would tell the
-// user they are de-opered when the truth is unknown.
+// QueryUserMode returns the session's user mode string,
+// such as "+", "+w" or "+ow".
 func (s *Service) QueryUserMode(
 	ctx context.Context,
 	sessionID int64,
@@ -944,80 +1041,52 @@ func (s *Service) QueryUserMode(
 	return modes, nil
 }
 
-// userModeOp is a single parsed user-mode change collected
-// by parseUserModeString before any DB writes happen.
-type userModeOp struct {
-	char   rune
-	adding bool
-}
-
-// ApplyUserMode parses an IRC user-mode string and applies
-// the resulting changes atomically. It supports multiple
-// sign transitions (e.g. "+w-o", "-w+o", "+o-w+w") and
-// rejects malformed input (empty string, no leading sign,
-// bare sign with no mode letters, unknown mode letters,
-// +o which must be set via OPER) with an IRCError. On
-// failure, no persistent change is made: parsing happens
-// before any write, and the writes themselves run inside a
-// single database transaction that is rolled back whole if
-// any statement fails. On success, the resulting mode
-// string is returned.
+// ApplyUserMode applies a user mode string to the session
+// and returns the resulting mode string. The whole string
+// is parsed before anything is written, and the flags are
+// written in one transaction, so a change that is rejected
+// or fails leaves the modes as they were.
 func (s *Service) ApplyUserMode(
 	ctx context.Context,
 	sessionID int64,
 	modeStr string,
 ) (string, error) {
-	ops, err := parseUserModeString(modeStr)
+	wallops, oper, err := parseUserModeString(modeStr)
 	if err != nil {
 		return "", err
 	}
 
-	wallops, oper, err := collapseUserModeOps(ops)
-	if err != nil {
-		return "", err
-	}
-
-	if err := s.db.SetSessionUserModes(
+	err = s.db.SetSessionUserModes(
 		ctx, sessionID, wallops, oper,
-	); err != nil {
-		s.log.Error(
-			"apply user modes failed", "error", err,
-		)
-
+	)
+	if err != nil {
 		return "", fmt.Errorf("apply user modes: %w", err)
 	}
 
 	return s.QueryUserMode(ctx, sessionID)
 }
 
-// parseUserModeString validates and parses a user-mode
-// string into a list of operations. The string must begin
-// with '+' or '-'; subsequent '+' / '-' characters flip the
-// active sign, and letters between them are applied with
-// the current sign. Every letter must be a recognized user
-// mode for this server, and '+o' is never allowed via MODE
-// (use OPER to become operator). If any character is
-// invalid, no operations are returned and an IRCError with
-// ERR_UMODEUNKNOWNFLAG (501) is returned.
+// parseUserModeString parses a user mode string such as
+// "+w", "-o" or "+w-o" into the new value of each flag it
+// names, or nil for a flag it does not name; a later letter
+// overrides an earlier one. The string must start with + or
+// - and name at least one mode. w may be set or unset, o
+// only unset (OPER sets it). Anything else is rejected with
+// ERR_UMODEUNKNOWNFLAG.
 func parseUserModeString(
 	modeStr string,
-) ([]userModeOp, error) {
+) (*bool, *bool, error) {
 	unknownFlag := &IRCError{
-		Code:    irc.ErrUmodeUnknownFlag,
-		Params:  nil,
-		Message: "Unknown MODE flag",
+		irc.ErrUmodeUnknownFlag, nil, "Unknown MODE flag",
 	}
 
-	if modeStr == "" {
-		return nil, unknownFlag
+	if !strings.HasPrefix(modeStr, "+") &&
+		!strings.HasPrefix(modeStr, "-") {
+		return nil, nil, unknownFlag
 	}
 
-	first := modeStr[0]
-	if first != '+' && first != '-' {
-		return nil, unknownFlag
-	}
+	var wallops, oper *bool
 
-	ops := make([]userModeOp, 0, len(modeStr)-1)
 	adding := true
 
 	for _, modeChar := range modeStr {
@@ -1026,80 +1095,50 @@ func parseUserModeString(
 			adding = true
 		case '-':
 			adding = false
-		default:
-			if !isKnownUserModeChar(modeChar) {
-				return nil, unknownFlag
-			}
-
-			if modeChar == 'o' && adding {
-				return nil, unknownFlag
-			}
-
-			ops = append(ops, userModeOp{
-				char: modeChar, adding: adding,
-			})
-		}
-	}
-
-	if len(ops) == 0 {
-		return nil, unknownFlag
-	}
-
-	return ops, nil
-}
-
-// isKnownUserModeChar reports whether the character is a
-// recognized user mode letter.
-func isKnownUserModeChar(modeChar rune) bool {
-	switch modeChar {
-	case 'w', 'o':
-		return true
-	default:
-		return false
-	}
-}
-
-// collapseUserModeOps reduces an already-parsed operation
-// list to the final desired value of each user mode flag.
-// A nil result for a flag means the mode string never
-// mentioned it, so it must be left untouched. Later
-// operations win over earlier ones for the same letter
-// (e.g. "+w-w" ends with wallops off), which matches the
-// left-to-right semantics of applying each op in turn.
-// parseUserModeString must have validated every character
-// and sign before this runs; the default branch here is
-// defence-in-depth only.
-func collapseUserModeOps(
-	ops []userModeOp,
-) (*bool, *bool, error) {
-	unknownFlag := &IRCError{
-		Code:    irc.ErrUmodeUnknownFlag,
-		Params:  nil,
-		Message: "Unknown MODE flag",
-	}
-
-	var wallops, oper *bool
-
-	for _, modeOp := range ops {
-		switch modeOp.char {
 		case 'w':
-			val := modeOp.adding
-			wallops = &val
+			value := adding
+			wallops = &value
 		case 'o':
-			if modeOp.adding {
+			if adding {
 				return nil, nil, unknownFlag
 			}
 
-			val := false
-			oper = &val
+			value := false
+			oper = &value
 		default:
 			return nil, nil, unknownFlag
 		}
 	}
 
+	if wallops == nil && oper == nil {
+		return nil, nil, unknownFlag
+	}
+
 	return wallops, oper, nil
 }
 
+// requireOper returns ERR_NOPRIVILEGES unless the session
+// is a server operator.
+func (s *Service) requireOper(
+	ctx context.Context,
+	sessionID int64,
+) error {
+	isOper, err := s.db.IsSessionOper(ctx, sessionID)
+	if err != nil {
+		return fmt.Errorf("check oper: %w", err)
+	}
+
+	if !isOper {
+		return &IRCError{
+			irc.ErrNoPrivileges,
+			nil,
+			"Permission Denied- You're not an IRC operator",
+		}
+	}
+
+	return nil
+}
+
 // broadcastNickChange notifies channel peers of a nick
 // change.
 func (s *Service) broadcastNickChange(

internal/service/service_test.go

@@ -10,6 +10,8 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"slices"
+	"strings"
 	"testing"
 	"time"
 
@@ -372,7 +374,6 @@ func TestQueryUserMode(t *testing.T) {
 
 	sid := createSession(ctx, t, env.db, "alice")
 
-	// Fresh session has no modes.
 	modes, err := env.svc.QueryUserMode(ctx, sid)
 	if err != nil {
 		t.Fatalf("query user mode: %v", err)
@@ -382,8 +383,7 @@ func TestQueryUserMode(t *testing.T) {
 		t.Errorf("expected +, got %s", modes)
 	}
 
-	// Set wallops.
-	_ = env.db.SetSessionWallops(ctx, sid, true)
+	setUserModes(ctx, t, env.db, sid, userModes{wallops: true})
 
 	modes, err = env.svc.QueryUserMode(ctx, sid)
 	if err != nil {
@@ -394,8 +394,9 @@ func TestQueryUserMode(t *testing.T) {
 		t.Errorf("expected +w, got %s", modes)
 	}
 
-	// Set oper.
-	_ = env.db.SetSessionOper(ctx, sid, true)
+	setUserModes(
+		ctx, t, env.db, sid, userModes{oper: true, wallops: true},
+	)
 
 	modes, err = env.svc.QueryUserMode(ctx, sid)
 	if err != nil {
@@ -407,408 +408,188 @@ func TestQueryUserMode(t *testing.T) {
 	}
 }
 
-// applyUserModeCaseState is the subset of session user-mode
-// state the rigorous TestApplyUserMode suite asserts on. It
-// mirrors the columns (oper, wallops) that the parser is
-// permitted to mutate.
-type applyUserModeCaseState struct {
+func TestUserhostReply(t *testing.T) {
+	env := newTestEnv(t)
+	ctx := t.Context()
+
+	createSession(ctx, t, env.db, "alice")
+	bobID := createSession(ctx, t, env.db, "bob")
+
+	// No username or hostname, and an operator.
+	operID, _, _, err := env.db.CreateSession(
+		ctx, "oper", "", "", "",
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	setUserModes(ctx, t, env.db, operID, userModes{oper: true})
+
+	_, err = env.svc.SetAway(ctx, bobID, "gone fishing")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	reply, err := env.svc.UserhostReply(ctx, []string{
+		"alice", "bob", "nobody", "oper",
+	}, "srv")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := "alice=+alice@localhost bob=-bob@localhost " +
+		"oper*=+oper@srv"
+	if reply != want {
+		t.Errorf("want %q, got %q", want, reply)
+	}
+
+	reply, err = env.svc.UserhostReply(
+		ctx, slices.Repeat([]string{"bob"}, 6), "srv",
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if strings.Count(reply, "bob=") != 5 {
+		t.Errorf("want 5 entries for 6 nicks, got %q", reply)
+	}
+}
+
+// userModes is the stored state of a session's user mode
+// flags.
+type userModes struct {
 	oper    bool
 	wallops bool
 }
 
-// applyUserModeCase describes one rigor-suite case for
-// Service.ApplyUserMode: the pre-call DB state, the mode
-// string input, and the expected post-call observable state
-// (mode string on success, IRC error code on rejection, and
-// persisted session state either way).
-type applyUserModeCase struct {
-	name         string
-	initialState applyUserModeCaseState
-	modeStr      string
-	wantModes    string
-	wantErr      bool
-	wantErrCode  irc.IRCMessageType
-	wantState    applyUserModeCaseState
+// applyUserModeTest is one mode string, the flags stored
+// before it is applied, and the expected outcome. An empty
+// wantModes means the string is rejected with
+// ERR_UMODEUNKNOWNFLAG, and then after must equal before.
+type applyUserModeTest struct {
+	modeStr   string
+	before    userModes
+	wantModes string
+	after     userModes
 }
 
-// applyUserModeCases returns every case listed in sneak's
-// review of PR #96 plus a few adjacent ones. Split across
-// helpers by category so each stays under funlen.
-func applyUserModeCases() []applyUserModeCase {
-	cases := applyUserModeHappyPathCases()
-	cases = append(cases, applyUserModeSignTransitionCases()...)
-	cases = append(cases, applyUserModeMalformedCases()...)
-	cases = append(cases, applyUserModeUnknownLetterCases()...)
+func applyUserModeTests() []applyUserModeTest {
+	none := userModes{}
+	oper := userModes{oper: true}
+	wallops := userModes{wallops: true}
+	both := userModes{oper: true, wallops: true}
 
-	return cases
-}
-
-// applyUserModeHappyPathCases covers valid single-char and
-// multi-char-without-sign-transition mode operations.
-func applyUserModeHappyPathCases() []applyUserModeCase {
-	return []applyUserModeCase{
-		{
-			name:         "+w from empty",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+w",
-			wantModes:    "+w",
-			wantErr:      false,
-			wantErrCode:  0,
-			wantState:    applyUserModeCaseState{oper: false, wallops: true},
-		},
-		{
-			name:         "-w from +w",
-			initialState: applyUserModeCaseState{oper: false, wallops: true},
-			modeStr:      "-w",
-			wantModes:    "+",
-			wantErr:      false,
-			wantErrCode:  0,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "-o from +o",
-			initialState: applyUserModeCaseState{oper: true, wallops: false},
-			modeStr:      "-o",
-			wantModes:    "+",
-			wantErr:      false,
-			wantErrCode:  0,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "-wo from +ow",
-			initialState: applyUserModeCaseState{oper: true, wallops: true},
-			modeStr:      "-wo",
-			wantModes:    "+",
-			wantErr:      false,
-			wantErrCode:  0,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
+	return []applyUserModeTest{
+		{"+w", none, "+w", wallops},
+		{"-w", wallops, "+", none},
+		{"-o", oper, "+", none},
+		{"-wo", both, "+", none},
+		{"+w-o", oper, "+w", wallops},
+		{"-w+w", none, "+w", wallops},
+		{"+w-w", wallops, "+", none},
+		{"-w+o", both, "", both},
+		{"+o-w+w", both, "", both},
+		{"+wo", none, "", none},
+		{"+wz", none, "", none},
+		{"+z", none, "", none},
+		{"-x+y", none, "", none},
+		{"+y-x", none, "", none},
+		{"w", none, "", none},
+		{"xw", wallops, "", wallops},
+		{"", none, "", none},
+		{"+", none, "", none},
+		{"-", none, "", none},
+		{"+-+", none, "", none},
 	}
 }
 
-// applyUserModeSignTransitionCases covers multi-char mode
-// strings where '+' and '-' flip partway through. +o is
-// never legal via MODE, so strings containing it must be
-// rejected atomically.
-func applyUserModeSignTransitionCases() []applyUserModeCase {
-	return []applyUserModeCase{
-		{
-			name:         "+w-o from +o",
-			initialState: applyUserModeCaseState{oper: true, wallops: false},
-			modeStr:      "+w-o",
-			wantModes:    "+w",
-			wantErr:      false,
-			wantErrCode:  0,
-			wantState:    applyUserModeCaseState{oper: false, wallops: true},
-		},
-		{
-			// +o is rejected before any op applies; wallops
-			// stays set.
-			name:         "-w+o always rejects +o",
-			initialState: applyUserModeCaseState{oper: true, wallops: true},
-			modeStr:      "-w+o",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: true, wallops: true},
-		},
-		{
-			// Wallops must NOT be cleared; oper must NOT be
-			// cleared. Rejection is fully atomic.
-			name:         "+o-w+w rejects because of +o",
-			initialState: applyUserModeCaseState{oper: true, wallops: true},
-			modeStr:      "+o-w+w",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: true, wallops: true},
-		},
-	}
-}
-
-// applyUserModeMalformedCases covers inputs that lack a
-// leading '+' or '-' and inputs that consist of bare signs
-// without mode letters. All must be rejected with no side
-// effects.
-func applyUserModeMalformedCases() []applyUserModeCase {
-	return []applyUserModeCase{
-		{
-			name:         "w no prefix rejects",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "w",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			// Prove wallops is NOT cleared — the whole point
-			// of sneak's review.
-			name: "xw no prefix rejects (would have been" +
-				" silently -w before)",
-			initialState: applyUserModeCaseState{oper: false, wallops: true},
-			modeStr:      "xw",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: true},
-		},
-		{
-			name:         "empty string rejects",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "bare + rejects",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "bare - rejects",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "-",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "+-+ rejects (no mode letters)",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+-+",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-	}
-}
-
-// applyUserModeUnknownLetterCases covers well-formed prefix
-// strings that contain unknown mode letters. Rejection must
-// be atomic: any valid letters before the invalid one must
-// not persist.
-func applyUserModeUnknownLetterCases() []applyUserModeCase {
-	return []applyUserModeCase{
-		{
-			name:         "-x+y rejects unknown -x",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "-x+y",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "+y-x rejects unknown +y",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+y-x",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name:         "+z unknown mode rejects",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+z",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			// Wallops must NOT be set.
-			name: "+wz rejects whole thing; +w side effect" +
-				" must NOT persist",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+wz",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-		{
-			name: "+wo rejects whole thing; +w side effect" +
-				" must NOT persist",
-			initialState: applyUserModeCaseState{oper: false, wallops: false},
-			modeStr:      "+wo",
-			wantModes:    "",
-			wantErr:      true,
-			wantErrCode:  irc.ErrUmodeUnknownFlag,
-			wantState:    applyUserModeCaseState{oper: false, wallops: false},
-		},
-	}
-}
-
-// TestApplyUserMode is the rigorous table-driven suite for
-// the shared user-mode parser. It covers every case listed
-// in sneak's review of PR #96 plus a few adjacent ones.
-// Each case asserts the resulting mode string AND the
-// persisted session state, to prove that rejected input
-// leaves no side effects.
 func TestApplyUserMode(t *testing.T) {
-	for _, testCase := range applyUserModeCases() {
-		t.Run(testCase.name, func(t *testing.T) {
-			runApplyUserModeCase(t, testCase)
+	env := newTestEnv(t)
+
+	for i, test := range applyUserModeTests() {
+		t.Run(test.modeStr, func(t *testing.T) {
+			ctx := t.Context()
+			sid := createSession(
+				ctx, t, env.db, fmt.Sprintf("user%d", i),
+			)
+			setUserModes(ctx, t, env.db, sid, test.before)
+
+			modes, err := env.svc.ApplyUserMode(
+				ctx, sid, test.modeStr,
+			)
+			checkApplyUserModeResult(t, test.wantModes, modes, err)
+
+			got := getUserModes(ctx, t, env.db, sid)
+			if got != test.after {
+				t.Errorf(
+					"stored modes: want %+v, got %+v",
+					test.after, got,
+				)
+			}
 		})
 	}
 }
 
-// runApplyUserModeCase executes one applyUserModeCase: seed
-// the session state, invoke ApplyUserMode, and verify both
-// the returned value and the post-call persisted state.
-func runApplyUserModeCase(
-	t *testing.T, testCase applyUserModeCase,
-) {
-	t.Helper()
-
-	env := newTestEnv(t)
-	ctx := t.Context()
-	sid := createSession(ctx, t, env.db, "alice")
-
-	seedApplyUserModeState(ctx, t, env.db, sid, testCase.initialState)
-
-	result, err := env.svc.ApplyUserMode(
-		ctx, sid, testCase.modeStr,
-	)
-
-	verifyApplyUserModeOutcome(t, testCase, result, err)
-	verifyApplyUserModeState(ctx, t, env.db, sid, testCase.wantState)
-}
-
-// seedApplyUserModeState installs the pre-call session
-// state described by initialState.
-func seedApplyUserModeState(
-	ctx context.Context,
+func checkApplyUserModeResult(
 	t *testing.T,
-	database *db.Database,
-	sid int64,
-	initialState applyUserModeCaseState,
-) {
-	t.Helper()
-
-	if initialState.oper {
-		if err := database.SetSessionOper(
-			ctx, sid, true,
-		); err != nil {
-			t.Fatalf("init oper: %v", err)
-		}
-	}
-
-	if initialState.wallops {
-		if err := database.SetSessionWallops(
-			ctx, sid, true,
-		); err != nil {
-			t.Fatalf("init wallops: %v", err)
-		}
-	}
-}
-
-// verifyApplyUserModeOutcome asserts the direct return
-// value of ApplyUserMode. It dispatches to the error- or
-// success-specific verifier based on wantErr.
-func verifyApplyUserModeOutcome(
-	t *testing.T,
-	testCase applyUserModeCase,
-	result string,
+	wantModes, modes string,
 	err error,
 ) {
 	t.Helper()
 
-	if testCase.wantErr {
-		verifyApplyUserModeError(t, testCase, err)
+	if wantModes == "" {
+		var ircErr *service.IRCError
+		if !errors.As(err, &ircErr) ||
+			ircErr.Code != irc.ErrUmodeUnknownFlag {
+			t.Fatalf("want ERR_UMODEUNKNOWNFLAG, got %v", err)
+		}
 
 		return
 	}
 
-	verifyApplyUserModeSuccess(t, testCase, result, err)
-}
-
-// verifyApplyUserModeError checks that err is a
-// *service.IRCError whose code matches wantErrCode.
-func verifyApplyUserModeError(
-	t *testing.T, testCase applyUserModeCase, err error,
-) {
-	t.Helper()
-
-	var ircErr *service.IRCError
-	if !errors.As(err, &ircErr) {
-		t.Fatalf("expected IRCError, got %v", err)
-	}
-
-	if ircErr.Code != testCase.wantErrCode {
-		t.Errorf(
-			"code: want %d got %d",
-			testCase.wantErrCode, ircErr.Code,
-		)
-	}
-}
-
-// verifyApplyUserModeSuccess checks that err is nil and the
-// returned mode string matches wantModes.
-func verifyApplyUserModeSuccess(
-	t *testing.T,
-	testCase applyUserModeCase,
-	result string,
-	err error,
-) {
-	t.Helper()
-
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
-	if result != testCase.wantModes {
-		t.Errorf(
-			"modes: want %q got %q",
-			testCase.wantModes, result,
-		)
+	if modes != wantModes {
+		t.Errorf("modes: want %q, got %q", wantModes, modes)
 	}
 }
 
-// verifyApplyUserModeState asserts the post-call persisted
-// session state. This is the atomicity guarantee sneak
-// demanded: whether the call succeeded or was rejected, the
-// DB must match wantState exactly.
-func verifyApplyUserModeState(
+func setUserModes(
 	ctx context.Context,
 	t *testing.T,
 	database *db.Database,
 	sid int64,
-	wantState applyUserModeCaseState,
+	modes userModes,
 ) {
 	t.Helper()
 
-	gotOper, err := database.IsSessionOper(ctx, sid)
+	err := database.SetSessionUserModes(
+		ctx, sid, &modes.wallops, &modes.oper,
+	)
+	if err != nil {
+		t.Fatalf("set user modes: %v", err)
+	}
+}
+
+func getUserModes(
+	ctx context.Context,
+	t *testing.T,
+	database *db.Database,
+	sid int64,
+) userModes {
+	t.Helper()
+
+	oper, err := database.IsSessionOper(ctx, sid)
 	if err != nil {
 		t.Fatalf("read oper: %v", err)
 	}
 
-	gotWallops, err := database.IsSessionWallops(ctx, sid)
+	wallops, err := database.IsSessionWallops(ctx, sid)
 	if err != nil {
 		t.Fatalf("read wallops: %v", err)
 	}
 
-	if gotOper != wantState.oper {
-		t.Errorf(
-			"oper: want %v got %v",
-			wantState.oper, gotOper,
-		)
-	}
-
-	if gotWallops != wantState.wallops {
-		t.Errorf(
-			"wallops: want %v got %v",
-			wantState.wallops, gotWallops,
-		)
-	}
+	return userModes{oper: oper, wallops: wallops}
 }