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

USERHOST, VERSION, ADMIN, INFO, TIME, KILL and WALLOPS on both the
HTTP API and the IRC listener, and user mode +w.

Model: opus-5-5

README.md

@@ -1575,31 +1575,24 @@ reference with all required and optional fields.
 
 **Command dispatch table:**
 
-| Command    | Required Fields | Optional | Response Status |
+| Command   | Required Fields | Optional | Response Status |
-| ---------- | --------------- | -------- | --------------- |
+| --------- | --------------- | -------- | --------------- |
-| `PRIVMSG`  | `to`, `body`    | `meta`   | 200 OK          |
+| `PRIVMSG` | `to`, `body`    | `meta`   | 200 OK          |
-| `NOTICE`   | `to`, `body`    | `meta`   | 200 OK          |
+| `NOTICE`  | `to`, `body`    | `meta`   | 200 OK          |
-| `JOIN`     | `to`            |          | 200 OK          |
+| `JOIN`    | `to`            |          | 200 OK          |
-| `PART`     | `to`            | `body`   | 200 OK          |
+| `PART`    | `to`            | `body`   | 200 OK          |
-| `NICK`     | `body`          |          | 200 OK          |
+| `NICK`    | `body`          |          | 200 OK          |
-| `PASS`     | `body`          |          | 200 OK          |
+| `PASS`    | `body`          |          | 200 OK          |
-| `TOPIC`    | `to`, `body`    |          | 200 OK          |
+| `TOPIC`   | `to`, `body`    |          | 200 OK          |
-| `MODE`     | `to`            | `body`   | 200 OK          |
+| `MODE`    | `to`            |          | 200 OK          |
-| `NAMES`    | `to`            |          | 200 OK          |
+| `NAMES`   | `to`            |          | 200 OK          |
-| `LIST`     |                 |          | 200 OK          |
+| `LIST`    |                 |          | 200 OK          |
-| `WHOIS`    | `to` or `body`  |          | 200 OK          |
+| `WHOIS`   | `to` or `body`  |          | 200 OK          |
-| `WHO`      | `to`            |          | 200 OK          |
+| `WHO`     | `to`            |          | 200 OK          |
-| `LUSERS`   |                 |          | 200 OK          |
+| `LUSERS`  |                 |          | 200 OK          |
-| `USERHOST` | `body`          |          | 200 OK          |
+| `OPER`    | `body`          |          | 200 OK          |
-| `VERSION`  |                 |          | 200 OK          |
+| `QUIT`    |                 | `body`   | 200 OK          |
-| `ADMIN`    |                 |          | 200 OK          |
+| `PING`    |                 |          | 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
@@ -1626,11 +1619,7 @@ 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):**
 
@@ -1641,13 +1630,11 @@ 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             |
+| 221     | RPL_UMODEIS       | In response to user MODE query       |
 | 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                 |
@@ -1655,10 +1642,6 @@ 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          |
@@ -2401,13 +2384,13 @@ IRC_LISTEN_ADDR=
 
 ### Supported Commands
 
-| Category   | Commands                                                                                 |
+| Category   | Commands                                                           |
-| ---------- | ---------------------------------------------------------------------------------------- |
+| ---------- | ------------------------------------------------------------------ |
-| Connection | `NICK`, `USER`, `PASS`, `QUIT`, `PING`/`PONG`, `CAP`                                     |
+| Connection | `NICK`, `USER`, `PASS`, `QUIT`, `PING`/`PONG`, `CAP`               |
-| Channels   | `JOIN`, `PART`, `MODE`, `TOPIC`, `NAMES`, `LIST`, `KICK`, `INVITE`                       |
+| Channels   | `JOIN`, `PART`, `MODE`, `TOPIC`, `NAMES`, `LIST`, `KICK`, `INVITE` |
-| Messaging  | `PRIVMSG`, `NOTICE`                                                                      |
+| 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
 
@@ -2425,13 +2408,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
+  `ERR_USERSDONTMATCH` (502), for both queries and changes. Nick comparison
-  case-insensitive.
+  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
+  the IRC listener regardless of whether the `KILL` arrived over IRC or the
-  API.
+  HTTP API.
 
 ### Bridge to HTTP API
 

internal/db/export_test.go

@@ -59,8 +59,10 @@ func (database *Database) Close() error {
 	return nil
 }
 
-// ExecForTest runs a raw statement, such as a trigger that
+// ExecForTest runs a raw statement against the test
-// makes a write fail.
+// 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.
 func (database *Database) ExecForTest(
 	ctx context.Context,
 	query string,

internal/db/queries.go

@@ -2425,57 +2425,63 @@ func (database *Database) SetChannelUserLimit(
 	return nil
 }
 
-// SetSessionUserModes sets a session's wallops (+w) and
+// SetSessionUserModes applies a set of user-mode flag
-// oper (+o) flags in one transaction, so that either both
+// changes to a session inside a single transaction, so a
-// changes are stored or neither is. A nil flag is left as
+// multi-mode change such as "+w-o" is all-or-nothing. A nil
-// it is.
+// pointer means the caller did not mention that mode and
+// the stored value must be left untouched.
 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 {
-		_, err = transaction.ExecContext(
+		if _, 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 {
-		_, err = transaction.ExecContext(
+		if _, 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)
 		}
 	}
 
-	err = transaction.Commit()
+	if err := transaction.Commit(); err != nil {
-	if err != nil {
+		_ = transaction.Rollback()
+
 		return fmt.Errorf("commit user modes: %w", err)
 	}
 
 	return nil
 }
 
-// boolToInt returns the 0 or 1 that the schema stores for a
+// boolToInt renders a Go bool as the 0/1 integer used for
-// boolean.
+// boolean columns in the SQLite schema.
 func boolToInt(value bool) int {
 	if value {
 		return 1
@@ -2484,6 +2490,30 @@ 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(
@@ -2527,19 +2557,16 @@ 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", err,
+				"scan wallops session: %w", scanErr,
 			)
 		}
 
 		ids = append(ids, sessionID)
 	}
 
-	err = rows.Err()
+	if err := rows.Err(); err != nil {
-	if err != nil {
 		return nil, fmt.Errorf(
 			"iterate wallops sessions: %w", err,
 		)
@@ -2558,11 +2585,17 @@ type UserhostInfo struct {
 }
 
 // GetUserhostInfo returns USERHOST info for the given
-// nicks. Nicks with no session are left out.
+// 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.
 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 {
@@ -2578,11 +2611,11 @@ func (database *Database) GetUserhostInfo(
 			&info.Nick, &info.Username, &info.Hostname,
 			&info.IsOper, &info.AwayMessage,
 		)
-		if errors.Is(err, sql.ErrNoRows) {
-			continue
-		}
-
 		if err != nil {
+			if errors.Is(err, sql.ErrNoRows) {
+				continue // nick not online
+			}
+
 			return nil, fmt.Errorf(
 				"userhost lookup %q: %w", nick, err,
 			)

internal/db/queries_test.go

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

internal/handlers/api.go

@@ -962,10 +962,12 @@ func (hdlr *Handlers) dispatchCommand(
 	bodyLines func() []string,
 ) {
 	switch command {
-	case irc.CmdAway:
+	case irc.CmdAway, irc.CmdNick,
-		hdlr.handleAway(
+		irc.CmdPass, irc.CmdInvite:
+		hdlr.dispatchBodyOnlyCommand(
 			writer, request,
-			sessionID, clientID, nick, bodyLines,
+			sessionID, clientID, nick,
+			command, bodyLines,
 		)
 	case irc.CmdPrivmsg, irc.CmdNotice:
 		hdlr.handlePrivmsg(
@@ -984,27 +986,12 @@ 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,
@@ -1018,9 +1005,12 @@ 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:
+	case irc.CmdMotd, irc.CmdPing,
+		irc.CmdVersion, irc.CmdAdmin,
+		irc.CmdInfo, irc.CmdTime:
 		hdlr.dispatchInfoCommand(
 			writer, request,
 			sessionID, clientID, nick,
@@ -1080,22 +1070,6 @@ 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,
@@ -1889,7 +1863,8 @@ func (hdlr *Handlers) deliverSetTopicNumerics(
 }
 
 // dispatchInfoCommand handles informational IRC commands
-// that produce server-side numerics (MOTD, PING).
+// that produce server-side numerics (MOTD, PING,
+// VERSION, ADMIN, INFO, TIME).
 func (hdlr *Handlers) dispatchInfoCommand(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -1915,6 +1890,34 @@ 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,15 +1,56 @@
 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"
 )
 
-// dispatchOperCommand routes the server operator commands
+// maxUserhostNicks is the maximum number of nicks allowed
-// OPER, KILL and WALLOPS to their handlers.
+// 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.
 func (hdlr *Handlers) dispatchOperCommand(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -36,8 +77,8 @@ func (hdlr *Handlers) dispatchOperCommand(
 	}
 }
 
-// handleUserhost handles USERHOST: user@host for each of
+// handleUserhost handles the USERHOST command.
-// up to five nicks given in the body.
+// Returns user@host info for up to 5 nicks.
 func (hdlr *Handlers) handleUserhost(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -45,6 +86,8 @@ func (hdlr *Handlers) handleUserhost(
 	nick string,
 	bodyLines func() []string,
 ) {
+	ctx := request.Context()
+
 	lines := bodyLines()
 	if len(lines) == 0 {
 		hdlr.respondIRCError(
@@ -57,47 +100,107 @@ func (hdlr *Handlers) handleUserhost(
 		return
 	}
 
-	reply, err := hdlr.svc.UserhostReply(
+	// Limit to 5 nicks per RFC 2812.
-		request.Context(), lines, hdlr.serverName(),
+	nicks := lines
+	if len(nicks) > maxUserhostNicks {
+		nicks = nicks[:maxUserhostNicks]
+	}
+
+	infos, err := hdlr.params.Database.GetUserhostInfo(
+		ctx, nicks,
 	)
-	if hdlr.handleServiceError(
+	if err != nil {
-		writer, request, clientID, sessionID, nick, err,
+		hdlr.log.Error(
-	) {
+			"userhost query failed", "error", err,
+		)
+		hdlr.respondError(
+			writer, request,
+			"internal error",
+			http.StatusInternalServerError,
+		)
+
 		return
 	}
 
+	replyStr := hdlr.buildUserhostReply(infos)
+
 	hdlr.enqueueNumeric(
-		request.Context(), clientID,
+		ctx, clientID, irc.RplUserHost, nick, nil,
-		irc.RplUserHost, nick, nil, reply,
+		replyStr,
 	)
+
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleVersion handles VERSION.
+// 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.
 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(
-		request.Context(), clientID, irc.RplVersion, nick,
+		ctx, clientID, irc.RplVersion, nick,
-		[]string{
+		[]string{version + ".", srvName},
-			hdlr.svc.ServerVersion() + ".",
-			hdlr.serverName(),
-		},
 		"",
 	)
+
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleAdmin handles ADMIN.
+// handleAdmin handles the ADMIN command.
+// Returns server admin contact info.
 func (hdlr *Handlers) handleAdmin(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -107,29 +210,39 @@ 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{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleInfo handles INFO.
+// handleInfo handles the INFO command.
+// Returns server software information.
 func (hdlr *Handlers) handleInfo(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -139,41 +252,51 @@ 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{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleTime handles TIME: the server's local time.
+// handleTime handles the TIME command.
+// Returns the server's local time in RFC format.
 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(
-		request.Context(), clientID, irc.RplTime, nick,
+		ctx, clientID, irc.RplTime, nick,
-		[]string{hdlr.serverName()},
+		[]string{srvName},
 		time.Now().Format(time.RFC1123),
 	)
+
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleKill handles KILL: the body is the target nick and
+// handleKill handles the KILL command.
-// an optional reason.
+// Forcibly disconnects a user (oper only).
 func (hdlr *Handlers) handleKill(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -181,9 +304,25 @@ 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()
 
-	targetNick := ""
+	var targetNick string
 	if len(lines) > 0 {
 		targetNick = strings.TrimSpace(lines[0])
 	}
@@ -204,22 +343,47 @@ func (hdlr *Handlers) handleKill(
 		reason = lines[1]
 	}
 
-	err := hdlr.svc.KillUser(
+	targetSID, lookupErr := hdlr.params.Database.
-		request.Context(), sessionID, nick,
+		GetSessionByNick(ctx, targetNick)
-		targetNick, reason,
+	if lookupErr != nil {
-	)
+		hdlr.respondIRCError(
-	if hdlr.handleServiceError(
+			writer, request, clientID, sessionID,
-		writer, request, clientID, sessionID, nick, err,
+			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,
+	)
+
 	hdlr.respondJSON(writer, request,
-		map[string]string{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleWallops handles WALLOPS: the body is the message.
+// handleWallops handles the WALLOPS command.
+// Broadcasts a message to all users with +w usermode
+// (oper only).
 func (hdlr *Handlers) handleWallops(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -227,6 +391,22 @@ 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(
@@ -239,25 +419,54 @@ func (hdlr *Handlers) handleWallops(
 		return
 	}
 
-	err := hdlr.svc.SendWallops(
+	message := strings.Join(lines, " ")
-		request.Context(), sessionID, nick,
+
-		strings.Join(lines, " "),
+	wallopsSIDs, err := hdlr.params.Database.
-	)
+		GetWallopsSessionIDs(ctx)
-	if hdlr.handleServiceError(
+	if err != nil {
-		writer, request, clientID, sessionID, nick, err,
+		hdlr.log.Error(
-	) {
+			"get wallops sessions failed", "error", err,
+		)
+		hdlr.respondError(
+			writer, request,
+			"internal error",
+			http.StatusInternalServerError,
+		)
+
 		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{statusKey: "ok"},
+		map[string]string{"status": "ok"},
 		http.StatusOK)
 }
 
-// handleUserMode handles MODE for a nick: a query when the
+// handleUserMode handles user mode queries and changes
-// body is empty, otherwise a change by the mode string in
+// (e.g., MODE nick, MODE nick +w). Delegates to the
-// the body. Only the caller's own nick, in any letter case,
+// shared service.ApplyUserMode / service.QueryUserMode so
-// is allowed.
+// that mode string processing is identical for both the
+// HTTP API and IRC wire protocol.
 func (hdlr *Handlers) handleUserMode(
 	writer http.ResponseWriter,
 	request *http.Request,
@@ -265,7 +474,14 @@ func (hdlr *Handlers) handleUserMode(
 	nick, target string,
 	bodyLines func() []string,
 ) {
-	if !strings.EqualFold(target, nick) {
+	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) {
 		hdlr.respondIRCError(
 			writer, request, clientID, sessionID,
 			irc.ErrUsersDoNotMatch, nick, nil,
@@ -275,33 +491,86 @@ func (hdlr *Handlers) handleUserMode(
 		return
 	}
 
-	ctx := request.Context()
-
-	var (
-		modes string
-		err   error
-	)
-
 	lines := bodyLines()
+
+	// Mode change requested.
 	if len(lines) > 0 {
-		modes, err = hdlr.svc.ApplyUserMode(
+		hdlr.changeUserMode(
-			ctx, sessionID, lines[0],
+			writer, request,
+			sessionID, clientID, nick, lines[0],
 		)
-	} else {
+
-		modes, err = hdlr.svc.QueryUserMode(ctx, sessionID)
+		return
 	}
 
-	if hdlr.handleServiceError(
+	// Mode query — delegate to shared service.
-		writer, request, clientID, sessionID, nick, err,
+	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, modes,
+		ctx, clientID, irc.RplUmodeIs, nick, nil,
+		modeStr,
 	)
 	hdlr.broker.Notify(sessionID)
 	hdlr.respondJSON(writer, request,
-		map[string]string{statusKey: "ok"},
+		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,
+	)
+	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,
+		)
+
+		return
+	}
+
+	hdlr.enqueueNumeric(
+		ctx, clientID, irc.RplUmodeIs, nick, nil,
+		newModes,
+	)
+
+	hdlr.broker.Notify(sessionID)
+	hdlr.respondJSON(writer, request,
+		map[string]string{"status": "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, and logs any other error.
+// reply on the wire.
 func (c *Conn) sendIRCError(err error) {
 	var ircErr *service.IRCError
 	if errors.As(err, &ircErr) {
@@ -21,11 +21,7 @@ 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.
@@ -693,14 +689,14 @@ func (c *Conn) applyChannelModes(
 	}
 }
 
-// handleUserMode handles MODE for a nick: a query without a
+// handleUserMode handles MODE for users.
-// 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,
 ) {
-	if !strings.EqualFold(msg.Params[0], c.currentNick()) {
+	target := msg.Params[0]
+
+	if !strings.EqualFold(target, c.currentNick()) {
 		c.sendNumeric(
 			irc.ErrUsersDoNotMatch,
 			"Can't change mode for other users",
@@ -709,26 +705,48 @@ func (c *Conn) handleUserMode(
 		return
 	}
 
-	var (
+	// Mode query (no mode string).
-		modes string
+	if len(msg.Params) < 2 { //nolint:mnd
-		err   error
+		modes, err := c.svc.QueryUserMode(
-	)
+			ctx, c.sessionID,
-
-	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.log.Error(
+				"query user mode failed", "error", err,
+			)
+			c.sendNumeric(
+				irc.ErrUmodeUnknownFlag,
+				"Unable to read user modes",
+			)
 
-	if err != nil {
+			return
-		c.sendIRCError(err)
+		}
+
+		c.sendNumeric(irc.RplUmodeIs, modes)
 
 		return
 	}
 
-	c.sendNumeric(irc.RplUmodeIs, modes)
+	newModes, err := c.svc.ApplyUserMode(
+		ctx, c.sessionID, msg.Params[1],
+	)
+	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",
+		)
+
+		return
+	}
+
+	c.sendNumeric(irc.RplUmodeIs, newModes)
 }
 
 // handleNames replies with channel member list.
@@ -1276,114 +1294,224 @@ func (c *Conn) handleInvite(
 	)
 }
 
-// handleUserhost replies with user@host for each of up to
+// handleUserhost replies with USERHOST info.
-// five nicks.
 func (c *Conn) handleUserhost(
 	ctx context.Context,
 	msg *Message,
 ) {
 	if len(msg.Params) < 1 {
-		c.sendNumeric(
+		return
-			irc.ErrNeedMoreParams,
+	}
-			irc.CmdUserhost, "Not enough parameters",
+
+	replies := make([]string, 0, len(msg.Params))
+
+	for _, nick := range msg.Params {
+		sid, err := c.database.GetSessionByNick(ctx, nick)
+		if err != nil {
+			continue
+		}
+
+		hostInfo, _ := c.database.GetSessionHostInfo(
+			ctx, sid,
 		)
 
-		return
+		host := "*"
+		if hostInfo != nil {
+			host = hostInfo.Hostname
+		}
+
+		isOper, _ := c.database.IsSessionOper(ctx, sid)
+
+		operStar := ""
+		if isOper {
+			operStar = "*"
+		}
+
+		replies = append(
+			replies,
+			nick+operStar+"=+"+nick+"@"+host,
+		)
 	}
 
-	reply, err := c.svc.UserhostReply(
+	c.sendNumeric(
-		ctx, msg.Params, c.serverSfx,
+		irc.RplUserHost,
+		strings.Join(replies, " "),
 	)
-	if err != nil {
-		c.sendIRCError(err)
-
-		return
-	}
-
-	c.sendNumeric(irc.RplUserHost, reply)
 }
 
-// handleVersion replies with the server version.
+// handleVersion replies with the server version string.
 func (c *Conn) handleVersion() {
 	c.sendNumeric(
 		irc.RplVersion,
-		c.svc.ServerVersion()+".", c.serverSfx, "",
+		c.svc.ServerVersion()+".", c.serverSfx,
+		"",
 	)
 }
 
-// handleAdmin replies with the server's admin info.
+// handleAdmin replies with server admin info.
 func (c *Conn) handleAdmin() {
+	srvName := c.serverSfx
+
 	c.sendNumeric(
-		irc.RplAdminMe, c.serverSfx, "Administrative info",
+		irc.RplAdminMe,
+		srvName, "Administrative info",
+	)
+
+	c.sendNumeric(
+		irc.RplAdminLoc1,
+		"neoirc server",
+	)
+
+	c.sendNumeric(
+		irc.RplAdminLoc2,
+		"IRC over HTTP",
+	)
+
+	c.sendNumeric(
+		irc.RplAdminEmail,
+		"admin@"+srvName,
 	)
-	c.sendNumeric(irc.RplAdminLoc1, "neoirc server")
-	c.sendNumeric(irc.RplAdminLoc2, "IRC over HTTP")
-	c.sendNumeric(irc.RplAdminEmail, "admin@"+c.serverSfx)
 }
 
-// handleInfo replies with the server's software info.
+// handleInfo replies with server 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")
-}
-
-// handleTime replies with the server's local time.
-func (c *Conn) handleTime() {
 	c.sendNumeric(
-		irc.RplTime,
+		irc.RplEndOfInfo,
-		c.serverSfx, time.Now().Format(time.RFC1123),
+		"End of /INFO list",
 	)
 }
 
-// handleKill handles KILL <nick> [<reason>].
+// handleTime replies with the server's current time.
-func (c *Conn) handleKill(
+func (c *Conn) handleTime() {
+	srvName := c.serverSfx
+
+	c.sendNumeric(
+		irc.RplTime,
+		srvName, time.Now().Format(time.RFC1123),
+	)
+}
+
+// handleKillCmd forcibly disconnects a target user (oper
+// only).
+func (c *Conn) handleKillCmd(
 	ctx context.Context,
 	msg *Message,
 ) {
-	if len(msg.Params) < 1 {
+	isOper, err := c.database.IsSessionOper(
+		ctx, c.sessionID,
+	)
+	if err != nil || !isOper {
 		c.sendNumeric(
-			irc.ErrNeedMoreParams,
+			irc.ErrNoPrivileges,
-			irc.CmdKill, "Not enough parameters",
+			"Permission Denied- "+
+				"You're not an IRC operator",
 		)
 
 		return
 	}
 
+	if len(msg.Params) < 1 {
+		c.sendNumeric(
+			irc.ErrNeedMoreParams,
+			"KILL", "Not enough parameters",
+		)
+
+		return
+	}
+
+	targetNick := msg.Params[0]
+
 	reason := "KILLed"
 	if len(msg.Params) > 1 {
 		reason = msg.Params[1]
 	}
 
-	err := c.svc.KillUser(
+	killerNick := c.currentNick()
-		ctx, c.sessionID, c.currentNick(),
-		msg.Params[0], reason,
-	)
-	if err != nil {
-		c.sendIRCError(err)
-	}
-}
 
-// handleWallops handles WALLOPS <message>.
+	if strings.EqualFold(targetNick, killerNick) {
-func (c *Conn) handleWallops(
-	ctx context.Context,
-	msg *Message,
-) {
-	if len(msg.Params) < 1 {
 		c.sendNumeric(
-			irc.ErrNeedMoreParams,
+			irc.ErrCantKillServer,
-			irc.CmdWallops, "Not enough parameters",
+			"You cannot KILL yourself",
 		)
 
 		return
 	}
 
-	err := c.svc.SendWallops(
+	targetSID, lookupErr := c.database.GetSessionByNick(
-		ctx, c.sessionID, c.currentNick(), msg.Params[0],
+		ctx, targetNick,
 	)
-	if err != nil {
+	if lookupErr != nil {
-		c.sendIRCError(err)
+		c.sendNumeric(
+			irc.ErrNoSuchNick,
+			targetNick, "No such nick/channel",
+		)
+
+		return
+	}
+
+	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(
+	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",
+		)
+
+		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,
+		)
 	}
 }

internal/ircserver/conn.go

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

internal/ircserver/conn_test.go

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

internal/ircserver/export_test.go

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

internal/ircserver/integration_test.go

@@ -4,8 +4,6 @@ import (
 	"strings"
 	"testing"
 	"time"
-
-	"sneak.berlin/go/neoirc/internal/config"
 )
 
 // TestIntegrationTwoClients is a comprehensive integration
@@ -779,28 +777,39 @@ func TestIntegrationUserhost(t *testing.T) {
 	bob := env.dial(t)
 	bob.register("bob")
 
-	bob.send("AWAY :lunch")
+	// Query single nick.
-	bob.readUntil(func(l string) bool {
+	alice.send("USERHOST bob")
-		return strings.Contains(l, " 306 ")
+
+	aliceReply := alice.readUntil(func(l string) bool {
+		return strings.Contains(l, " 302 ")
 	})
+	assertContains(
+		t, aliceReply, " 302 ",
+		"RPL_USERHOST",
+	)
+	assertContains(
+		t, aliceReply, "bob",
+		"USERHOST contains queried nick",
+	)
 
-	for _, step := range []struct {
+	// Query multiple nicks.
-		line, want string
+	bob.send("USERHOST alice bob")
-	}{
-		{"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)
 
-		reply := alice.readUntil(func(l string) bool {
+	bobReply := bob.readUntil(func(l string) bool {
-			return strings.Contains(l, " 302 ") ||
+		return strings.Contains(l, " 302 ")
-				strings.Contains(l, " 461 ")
+	})
-		})
+	assertContains(
-		assertContains(t, reply, step.want, step.line)
+		t, bobReply, " 302 ",
-	}
+		"RPL_USERHOST multi-nick",
+	)
+	assertContains(
+		t, bobReply, "alice",
+		"USERHOST multi contains alice",
+	)
+	assertContains(
+		t, bobReply, "bob",
+		"USERHOST multi contains bob",
+	)
 }
 
 // TestIntegrationVersion verifies the VERSION command
@@ -910,54 +919,17 @@ func TestIntegrationTime(t *testing.T) {
 		"RPL_TIME",
 	)
 	assertContains(
-		t, aliceReply, testServerName,
+		t, aliceReply, "test.irc",
 		"TIME reply includes server name",
 	)
 }
 
-// becomeOper sends OPER with newTestEnvWithOper's
+// TestIntegrationKill verifies the KILL command: oper can
-// credentials and waits for RPL_YOUREOPER.
+// kill a user, non-oper cannot, and — most importantly —
-func (tc *testClient) becomeOper() {
+// that the victim is actually disconnected rather than
-	tc.t.Helper()
+// merely announced as having quit.
-
+//
-	tc.send("OPER testoper testpass")
+//nolint:funlen // one KILL scenario asserted end to end
-	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()
 
@@ -969,6 +941,7 @@ 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")
 
@@ -978,9 +951,30 @@ func TestIntegrationKill(t *testing.T) {
 			strings.Contains(l, "bob")
 	})
 
-	alice.becomeOper()
+	// 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.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",
@@ -1030,6 +1024,17 @@ 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:
@@ -1056,7 +1061,11 @@ func TestIntegrationWallops(t *testing.T) {
 		"ERR_NOPRIVILEGES for non-oper WALLOPS",
 	)
 
-	alice.becomeOper()
+	// alice becomes oper.
+	alice.send("OPER testoper testpass")
+	alice.readUntil(func(l string) bool {
+		return strings.Contains(l, " 381 ")
+	})
 
 	// bob sets +w to receive wallops.
 	bob.send("MODE bob +w")
@@ -1083,52 +1092,6 @@ 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.
@@ -1283,29 +1246,79 @@ func TestIntegrationThirdClientObserver(t *testing.T) {
 	)
 }
 
-// TestIntegrationDefaultServerNameFallback checks that with
+// assertNoEmptyParam fails if any line contains a doubled
-// SERVER_NAME unset, as it is by default, VERSION, ADMIN and
+// space, which is what FormatMessage emits for an empty
-// TIME name the server "neoirc".
+// 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.
 func TestIntegrationDefaultServerNameFallback(t *testing.T) {
 	t.Parallel()
 
-	env := newTestEnvWithConfig(t, &config.Config{})
+	env := newTestEnvWithServerName(t, "")
 
 	alice := env.dial(t)
 	alice.register("alice")
 
-	for _, step := range []struct {
+	alice.send("VERSION")
-		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)
 
-		reply := alice.readUntil(func(l string) bool {
+	versionReply := alice.readUntil(func(l string) bool {
-			return strings.Contains(l, step.lastNumeric)
+		return strings.Contains(l, " 351 ")
-		})
+	})
-		assertContains(t, reply, step.want, step.line)
+	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",
+	)
 }

internal/ircserver/server_test.go

@@ -36,36 +36,20 @@ 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 newTestEnvWithConfig(t, &config.Config{
+	return newTestEnvWithServerName(t, "test.irc")
-		ServerName: testServerName,
-		MOTD:       "Welcome to test IRC",
-	})
 }
 
-// newTestEnvWithOper is newTestEnv with the operator name
+// newTestEnvWithServerName creates a test environment with
-// testoper and password testpass.
+// an explicit SERVER_NAME. Passing "" exercises the shipped
-func newTestEnvWithOper(t *testing.T) *testEnv {
+// default from config.go, under which the server must fall
-	t.Helper()
+// back to "neoirc" rather than emitting an empty
-
+// server-name parameter.
-	return newTestEnvWithConfig(t, &config.Config{
+func newTestEnvWithServerName(
-		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,
-	cfg *config.Config,
+	serverName string,
 ) *testEnv {
 	t.Helper()
 
@@ -97,6 +81,11 @@ func newTestEnvWithConfig(
 
 	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")
@@ -140,6 +129,87 @@ func newTestEnvWithConfig(
 	}
 }
 
+// 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()
@@ -240,9 +310,11 @@ func (tc *testClient) register(nick string) []string {
 	})
 }
 
-// readUntilClosed returns the lines received until the
+// readUntilClosed reads until the peer closes the
-// server closes the connection, and fails the test if it is
+// connection, returning the lines received first.  It
-// still open after testTimeout.
+// 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.
 func (tc *testClient) readUntilClosed() []string {
 	tc.t.Helper()
 
@@ -259,7 +331,8 @@ func (tc *testClient) readUntilClosed() []string {
 	err := tc.scanner.Err()
 	if err != nil {
 		tc.t.Fatalf(
-			"connection not closed: %v (lines: %v)",
+			"expected EOF on victim socket, got %v "+
+				"(lines: %v)",
 			err, lines,
 		)
 	}

internal/service/service.go

@@ -5,9 +5,7 @@ package service
 import (
 	"context"
 	"crypto/subtle"
-	"database/sql"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"log/slog"
 	"strconv"
@@ -27,14 +25,9 @@ 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
@@ -46,11 +39,14 @@ type Params struct {
 	Globals  *globals.Globals
 }
 
-// WireConn is a registered IRC connection, which KILL closes.
+// WireConn is a live client connection that a transport
-// HTTP clients hold no connection and register none.
+// 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.
 type WireConn interface {
-	// Disconnect tells the client why and closes the
+	// Disconnect terminates the connection, telling the
-	// connection.
+	// client why before closing the socket.
 	Disconnect(reason string)
 }
 
@@ -68,7 +64,7 @@ type Service struct {
 
 // New creates a new Service.
 func New(params Params) *Service {
-	return &Service{
+	return &Service{ //nolint:exhaustruct // mutex zero value
 		db:        params.Database,
 		broker:    params.Broker,
 		config:    params.Config,
@@ -87,7 +83,7 @@ func NewTestService(
 	globs *globals.Globals,
 	log *slog.Logger,
 ) *Service {
-	return &Service{
+	return &Service{ //nolint:exhaustruct // mutex zero value
 		db:        database,
 		broker:    brk,
 		config:    cfg,
@@ -97,81 +93,46 @@ func NewTestService(
 	}
 }
 
-// ServerVersion returns the version that VERSION and INFO
+// ServerVersion returns the canonical server version string
-// report on both transports, such as "neoirc-1.2.3".
+// 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.
 func (s *Service) ServerVersion() string {
-	version := s.globals.Version
+	name := "neoirc"
-	if version == "" {
+	ver := "dev"
-		version = "dev"
+
+	if s.globals != nil {
+		if s.globals.Appname != "" {
+			name = s.globals.Appname
+		}
+
+		if s.globals.Version != "" {
+			ver = s.globals.Version
+		}
 	}
 
-	return s.globals.Appname + "-" + version
+	return name + "-" + ver
 }
 
-// InfoLines returns the lines that INFO sends on both
+// InfoLines returns the RPL_INFO body.  Both transports
-// transports.
+// send exactly these lines so that INFO does not diverge
+// between the wire protocol and the HTTP API.
 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: " + s.globals.StartTime.Format(time.RFC1123),
+		"Started: " + started,
 	}
 }
 
-// UserhostReply returns the RPL_USERHOST text for the first
+// RegisterWireConn associates a live wire connection with
-// five of the given nicks: "nick=+user@host" entries joined
+// its session ID so that KillSession can reach it.
-// 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,
@@ -182,9 +143,9 @@ func (s *Service) RegisterWireConn(
 	s.wireConns[sessionID] = conn
 }
 
-// UnregisterWireConn removes what RegisterWireConn recorded,
+// UnregisterWireConn removes the association created by
-// unless the session has since registered another
+// RegisterWireConn.  It is a no-op if the session has
-// connection.
+// already been rebound to a different connection.
 func (s *Service) UnregisterWireConn(
 	sessionID int64,
 	conn WireConn,
@@ -197,90 +158,28 @@ func (s *Service) UnregisterWireConn(
 	}
 }
 
-// KillUser carries out an operator's KILL: the target's
+// KillSession forcibly removes a user from the server: the
-// channel peers see it quit, its session is deleted, and
+// victim's channel peers are told via QUIT, the victim's
-// its IRC connection, if it has one, is closed.
+// session is deleted, and any live wire connection it holds
-func (s *Service) KillUser(
+// 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(
 	ctx context.Context,
 	sessionID int64,
-	nick, targetNick, reason string,
+	nick, reason string,
-) error {
+) {
-	err := s.requireOper(ctx, sessionID)
+	s.BroadcastQuit(ctx, sessionID, nick, reason)
-	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[targetSID]
+	conn := s.wireConns[sessionID]
 	s.wireMu.Unlock()
 
 	if conn != nil {
-		conn.Disconnect(quitReason)
+		conn.Disconnect(reason)
 	}
-
-	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
@@ -651,7 +550,7 @@ func (s *Service) KickUser(
 		return &IRCError{
 			irc.ErrNoSuchNick,
 			[]string{targetNick},
-			msgNoSuchNick,
+			"No such nick/channel",
 		}
 	}
 
@@ -863,7 +762,7 @@ func (s *Service) ApplyMemberMode(
 		return &IRCError{
 			irc.ErrNoSuchNick,
 			[]string{targetNick},
-			msgNoSuchNick,
+			"No such nick/channel",
 		}
 	}
 
@@ -1006,8 +905,12 @@ func (s *Service) QueryChannelMode(
 	return modes + modeParams
 }
 
-// QueryUserMode returns the session's user mode string,
+// QueryUserMode returns the current user mode string for
-// such as "+", "+w" or "+ow".
+// 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.
 func (s *Service) QueryUserMode(
 	ctx context.Context,
 	sessionID int64,
@@ -1041,52 +944,80 @@ func (s *Service) QueryUserMode(
 	return modes, nil
 }
 
-// ApplyUserMode applies a user mode string to the session
+// userModeOp is a single parsed user-mode change collected
-// and returns the resulting mode string. The whole string
+// by parseUserModeString before any DB writes happen.
-// is parsed before anything is written, and the flags are
+type userModeOp struct {
-// written in one transaction, so a change that is rejected
+	char   rune
-// or fails leaves the modes as they were.
+	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.
 func (s *Service) ApplyUserMode(
 	ctx context.Context,
 	sessionID int64,
 	modeStr string,
 ) (string, error) {
-	wallops, oper, err := parseUserModeString(modeStr)
+	ops, err := parseUserModeString(modeStr)
 	if err != nil {
 		return "", err
 	}
 
-	err = s.db.SetSessionUserModes(
+	wallops, oper, err := collapseUserModeOps(ops)
-		ctx, sessionID, wallops, oper,
-	)
 	if err != nil {
+		return "", err
+	}
+
+	if err := s.db.SetSessionUserModes(
+		ctx, sessionID, wallops, oper,
+	); err != nil {
+		s.log.Error(
+			"apply user modes failed", "error", err,
+		)
+
 		return "", fmt.Errorf("apply user modes: %w", err)
 	}
 
 	return s.QueryUserMode(ctx, sessionID)
 }
 
-// parseUserModeString parses a user mode string such as
+// parseUserModeString validates and parses a user-mode
-// "+w", "-o" or "+w-o" into the new value of each flag it
+// string into a list of operations. The string must begin
-// names, or nil for a flag it does not name; a later letter
+// with '+' or '-'; subsequent '+' / '-' characters flip the
-// overrides an earlier one. The string must start with + or
+// active sign, and letters between them are applied with
-// - and name at least one mode. w may be set or unset, o
+// the current sign. Every letter must be a recognized user
-// only unset (OPER sets it). Anything else is rejected with
+// mode for this server, and '+o' is never allowed via MODE
-// ERR_UMODEUNKNOWNFLAG.
+// (use OPER to become operator). If any character is
+// invalid, no operations are returned and an IRCError with
+// ERR_UMODEUNKNOWNFLAG (501) is returned.
 func parseUserModeString(
 	modeStr string,
-) (*bool, *bool, error) {
+) ([]userModeOp, error) {
 	unknownFlag := &IRCError{
-		irc.ErrUmodeUnknownFlag, nil, "Unknown MODE flag",
+		Code:    irc.ErrUmodeUnknownFlag,
+		Params:  nil,
+		Message: "Unknown MODE flag",
 	}
 
-	if !strings.HasPrefix(modeStr, "+") &&
+	if modeStr == "" {
-		!strings.HasPrefix(modeStr, "-") {
+		return nil, unknownFlag
-		return nil, nil, unknownFlag
 	}
 
-	var wallops, oper *bool
+	first := modeStr[0]
+	if first != '+' && first != '-' {
+		return nil, unknownFlag
+	}
 
+	ops := make([]userModeOp, 0, len(modeStr)-1)
 	adding := true
 
 	for _, modeChar := range modeStr {
@@ -1095,50 +1026,80 @@ 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':
-			value := adding
+			val := modeOp.adding
-			wallops = &value
+			wallops = &val
 		case 'o':
-			if adding {
+			if modeOp.adding {
 				return nil, nil, unknownFlag
 			}
 
-			value := false
+			val := false
-			oper = &value
+			oper = &val
 		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,8 +10,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"slices"
-	"strings"
 	"testing"
 	"time"
 
@@ -374,6 +372,7 @@ 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)
@@ -383,7 +382,8 @@ func TestQueryUserMode(t *testing.T) {
 		t.Errorf("expected +, got %s", modes)
 	}
 
-	setUserModes(ctx, t, env.db, sid, userModes{wallops: true})
+	// Set wallops.
+	_ = env.db.SetSessionWallops(ctx, sid, true)
 
 	modes, err = env.svc.QueryUserMode(ctx, sid)
 	if err != nil {
@@ -394,9 +394,8 @@ func TestQueryUserMode(t *testing.T) {
 		t.Errorf("expected +w, got %s", modes)
 	}
 
-	setUserModes(
+	// Set oper.
-		ctx, t, env.db, sid, userModes{oper: true, wallops: true},
+	_ = env.db.SetSessionOper(ctx, sid, true)
-	)
 
 	modes, err = env.svc.QueryUserMode(ctx, sid)
 	if err != nil {
@@ -408,188 +407,408 @@ func TestQueryUserMode(t *testing.T) {
 	}
 }
 
-func TestUserhostReply(t *testing.T) {
+// applyUserModeCaseState is the subset of session user-mode
-	env := newTestEnv(t)
+// state the rigorous TestApplyUserMode suite asserts on. It
-	ctx := t.Context()
+// mirrors the columns (oper, wallops) that the parser is
-
+// permitted to mutate.
-	createSession(ctx, t, env.db, "alice")
+type applyUserModeCaseState struct {
-	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
 }
 
-// applyUserModeTest is one mode string, the flags stored
+// applyUserModeCase describes one rigor-suite case for
-// before it is applied, and the expected outcome. An empty
+// Service.ApplyUserMode: the pre-call DB state, the mode
-// wantModes means the string is rejected with
+// string input, and the expected post-call observable state
-// ERR_UMODEUNKNOWNFLAG, and then after must equal before.
+// (mode string on success, IRC error code on rejection, and
-type applyUserModeTest struct {
+// persisted session state either way).
-	modeStr   string
+type applyUserModeCase struct {
-	before    userModes
+	name         string
-	wantModes string
+	initialState applyUserModeCaseState
-	after     userModes
+	modeStr      string
+	wantModes    string
+	wantErr      bool
+	wantErrCode  irc.IRCMessageType
+	wantState    applyUserModeCaseState
 }
 
-func applyUserModeTests() []applyUserModeTest {
+// applyUserModeCases returns every case listed in sneak's
-	none := userModes{}
+// review of PR #96 plus a few adjacent ones. Split across
-	oper := userModes{oper: true}
+// helpers by category so each stays under funlen.
-	wallops := userModes{wallops: true}
+func applyUserModeCases() []applyUserModeCase {
-	both := userModes{oper: true, wallops: true}
+	cases := applyUserModeHappyPathCases()
+	cases = append(cases, applyUserModeSignTransitionCases()...)
+	cases = append(cases, applyUserModeMalformedCases()...)
+	cases = append(cases, applyUserModeUnknownLetterCases()...)
 
-	return []applyUserModeTest{
+	return cases
-		{"+w", none, "+w", wallops},
+}
-		{"-w", wallops, "+", none},
+
-		{"-o", oper, "+", none},
+// applyUserModeHappyPathCases covers valid single-char and
-		{"-wo", both, "+", none},
+// multi-char-without-sign-transition mode operations.
-		{"+w-o", oper, "+w", wallops},
+func applyUserModeHappyPathCases() []applyUserModeCase {
-		{"-w+w", none, "+w", wallops},
+	return []applyUserModeCase{
-		{"+w-w", wallops, "+", none},
+		{
-		{"-w+o", both, "", both},
+			name:         "+w from empty",
-		{"+o-w+w", both, "", both},
+			initialState: applyUserModeCaseState{oper: false, wallops: false},
-		{"+wo", none, "", none},
+			modeStr:      "+w",
-		{"+wz", none, "", none},
+			wantModes:    "+w",
-		{"+z", none, "", none},
+			wantErr:      false,
-		{"-x+y", none, "", none},
+			wantErrCode:  0,
-		{"+y-x", none, "", none},
+			wantState:    applyUserModeCaseState{oper: false, wallops: true},
-		{"w", none, "", none},
+		},
-		{"xw", wallops, "", wallops},
+		{
-		{"", none, "", none},
+			name:         "-w from +w",
-		{"+", none, "", none},
+			initialState: applyUserModeCaseState{oper: false, wallops: true},
-		{"-", none, "", none},
+			modeStr:      "-w",
-		{"+-+", none, "", none},
+			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},
+		},
 	}
 }
 
+// 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) {
-	env := newTestEnv(t)
+	for _, testCase := range applyUserModeCases() {
-
+		t.Run(testCase.name, func(t *testing.T) {
-	for i, test := range applyUserModeTests() {
+			runApplyUserModeCase(t, testCase)
-		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,
-				)
-			}
 		})
 	}
 }
 
-func checkApplyUserModeResult(
+// 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,
 	t *testing.T,
-	wantModes, modes string,
+	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,
 	err error,
 ) {
 	t.Helper()
 
-	if wantModes == "" {
+	if testCase.wantErr {
-		var ircErr *service.IRCError
+		verifyApplyUserModeError(t, testCase, err)
-		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 modes != wantModes {
+	if result != testCase.wantModes {
-		t.Errorf("modes: want %q, got %q", wantModes, modes)
+		t.Errorf(
+			"modes: want %q got %q",
+			testCase.wantModes, result,
+		)
 	}
 }
 
-func setUserModes(
+// 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(
 	ctx context.Context,
 	t *testing.T,
 	database *db.Database,
 	sid int64,
-	modes userModes,
+	wantState applyUserModeCaseState,
 ) {
 	t.Helper()
 
-	err := database.SetSessionUserModes(
+	gotOper, err := database.IsSessionOper(ctx, sid)
-		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)
 	}
 
-	wallops, err := database.IsSessionWallops(ctx, sid)
+	gotWallops, err := database.IsSessionWallops(ctx, sid)
 	if err != nil {
 		t.Fatalf("read wallops: %v", err)
 	}
 
-	return userModes{oper: oper, wallops: wallops}
+	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,
+		)
+	}
 }