Give each IRC listener test its own database (closes #129)

Each test environment in internal/ircserver named its shared in-memory
database after the test. A repeat run of a test in the same process
could open the database that an earlier run's server was still
holding, and start on that run's users. The database now gets a random
name, and a new test checks that two environments open at once do not
share one.

The database close and the server stop are now registered as soon as
each is opened, so a setup that fails part-way still closes its
database.

Model: opus-5-5

README.md

@@ -2409,7 +2409,7 @@ applied; `001_initial.sql` creates the tables below.
 #### `sessions`
 
 | Column          | Type     | Description                                                                      |
-| --------------- | -------- | ------------------------------------------------------------------------------- |
+| --------------- | -------- | -------------------------------------------------------------------------------- |
 | `id`            | INTEGER  | Primary key (auto-increment)                                                     |
 | `uuid`          | TEXT     | Unique session UUID                                                              |
 | `nick`          | TEXT     | Unique nick                                                                      |
@@ -2422,14 +2422,14 @@ applied; `001_initial.sql` creates the tables below.
 | `signing_key`   | TEXT     | Public signing key; nothing sets or reads it yet                                 |
 | `away_message`  | TEXT     | Away message (empty string if not away)                                          |
 | `created_at`    | DATETIME | Session creation time                                                            |
-| `last_seen`     | DATETIME | Last login or authenticated HTTP API request (or creation)                      |
+| `last_seen`     | DATETIME | Last login, authenticated HTTP API request or IRC listener command (or creation) |
 
 Index on `(uuid)`.
 
 #### `clients`
 
 | Column       | Type     | Description                                                               |
-| ------------ | -------- | ----------------------------------------------------------------------- |
+| ------------ | -------- | ------------------------------------------------------------------------- |
 | `id`         | INTEGER  | Primary key (auto-increment)                                              |
 | `uuid`       | TEXT     | Unique client UUID                                                        |
 | `session_id` | INTEGER  | FK → sessions.id (cascade delete)                                         |
@@ -2437,7 +2437,7 @@ Index on `(uuid)`.
 | `ip`         | TEXT     | Real IP address of this client connection                                 |
 | `hostname`   | TEXT     | Reverse DNS name of this client's IP, or the IP itself when it has none   |
 | `created_at` | DATETIME | Client creation time                                                      |
-| `last_seen`  | DATETIME | Last authenticated HTTP API request (or creation)                       |
+| `last_seen`  | DATETIME | Last authenticated HTTP API request or IRC listener command (or creation) |
 
 Indexes on `(token)` and `(session_id)`.
 
@@ -2555,10 +2555,9 @@ issues) and simpler than UUIDs (integer comparison vs. string comparison).
   session types in the cleanup path — `handleQuit` and `cleanupUser` both call
   `DeleteSession` unconditionally. Idle clients are automatically removed after
   `SESSION_IDLE_TIMEOUT` (default 30 days) without an authenticated HTTP API
-  request — the server runs a background cleanup loop that, for a session left
+  request or, on the IRC listener, a command — the server runs a background
-  with no clients, parts the user from all channels, broadcasts QUIT, and
+  cleanup loop that, for a session left with no clients, parts the user from all
-  releases the nick. A client on the IRC listener never makes such a request, so
+  channels, broadcasts QUIT, and releases the nick.
-  it counts as idle from when it connected.
 - **Clients**: Individual client auth cookies are invalidated on
   `POST /api/v1/logout`. A session can have multiple clients; removing one
   doesn't affect others. However, when the last client is removed (via logout),
@@ -2583,12 +2582,12 @@ environment variable wins over the file. An environment variable set to the
 empty string counts as unset, so the file's value or the default applies.
 
 | Variable               | Type   | Default                                             | Description                                                                                                                                                                                                                                                                             |
-| ---------------------- | ------ | --------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| ---------------------- | ------ | --------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `PORT`                 | int    | `8080`                                              | HTTP listen port                                                                                                                                                                                                                                                                        |
 | `DBURL`                | string | `file:///var/lib/neoirc/state.db?_journal_mode=WAL` | SQLite connection string. For file-based: `file:///path/to/db.db?_journal_mode=WAL`. For in-memory (testing): `file::memory:?cache=shared`. The SQLite driver ignores `_journal_mode`, so this does not turn on WAL mode.                                                               |
 | `DEBUG`                | bool   | `false`                                             | Switch logging to debug level                                                                                                                                                                                                                                                           |
 | `MESSAGE_MAX_AGE`      | string | `720h`                                              | Maximum age of messages as a Go duration string (e.g. `720h`, `24h`). Messages older than this are pruned. Default is 30 days.                                                                                                                                                          |
-| `SESSION_IDLE_TIMEOUT` | string | `720h`                                              | Idle timeout as a Go duration string (e.g. `720h`, `24h`). Clients with no authenticated HTTP API request for this long are removed, and a session left with none is expired and its nick released (see [Data Lifecycle](#data-lifecycle)). Default is 30 days. |
+| `SESSION_IDLE_TIMEOUT` | string | `720h`                                              | Idle timeout as a Go duration string (e.g. `720h`, `24h`). Clients with no authenticated HTTP API request or IRC listener command for this long are removed, and a session left with none is expired and its nick released (see [Data Lifecycle](#data-lifecycle)). Default is 30 days. |
 | `QUEUE_MAX_AGE`        | string | `720h`                                              | Maximum age of client output queue entries as a Go duration string (e.g. `720h`, `24h`). Entries older than this are pruned. Default is 30 days.                                                                                                                                        |
 | `MAX_MESSAGE_SIZE`     | int    | `4096`                                              | Maximum request body size in bytes for `POST /api/v1/session`, `POST /api/v1/login` and `POST /api/v1/messages`; a larger body gets `400` `invalid request body`                                                                                                                        |
 | `MOTD`                 | string | a built-in banner                                   | Message of the day, sent to clients as MOTD numerics and shown via `GET /api/v1/server`                                                                                                                                                                                                 |

internal/db/queries.go

@@ -204,21 +204,42 @@ func (database *Database) GetSessionByToken(
 		)
 	}
 
+	_ = database.UpdateLastSeen(ctx, sessionID, clientID)
+
+	return sessionID, clientID, nick, nil
+}
+
+// UpdateLastSeen sets last_seen to now on a session and on
+// one of its clients, so that the idle cleanup keeps them.
+func (database *Database) UpdateLastSeen(
+	ctx context.Context,
+	sessionID, clientID int64,
+) error {
 	now := time.Now()
 
-	_, _ = database.conn.ExecContext(
+	_, err := database.conn.ExecContext(
 		ctx,
 		"UPDATE sessions SET last_seen = ? WHERE id = ?",
 		now, sessionID,
 	)
+	if err != nil {
+		return fmt.Errorf(
+			"update session last_seen: %w", err,
+		)
+	}
 
-	_, _ = database.conn.ExecContext(
+	_, err = database.conn.ExecContext(
 		ctx,
 		"UPDATE clients SET last_seen = ? WHERE id = ?",
 		now, clientID,
 	)
+	if err != nil {
+		return fmt.Errorf(
+			"update client last_seen: %w", err,
+		)
+	}
 
-	return sessionID, clientID, nick, nil
+	return nil
 }
 
 // GetSessionByNick returns session id for a given nick.

internal/ircserver/conn.go

@@ -368,6 +368,17 @@ func (c *Conn) handleMessage(
 		return
 	}
 
+	// Every command is activity, so the idle cleanup must not
+	// remove this user.
+	err := c.database.UpdateLastSeen(
+		ctx, c.sessionID, c.clientID,
+	)
+	if err != nil {
+		c.log.Error(
+			"failed to update last_seen", "error", err,
+		)
+	}
+
 	handler, ok := c.commands[msg.Command]
 	if !ok {
 		c.sendNumeric(

internal/ircserver/server_test.go

@@ -2,6 +2,7 @@ package ircserver_test
 
 import (
 	"bufio"
+	"crypto/rand"
 	"database/sql"
 	"fmt"
 	"log/slog"
@@ -69,9 +70,11 @@ func newTestEnvWithConfig(
 ) *testEnv {
 	t.Helper()
 
+	// A random name, so that no other test environment, not
+	// even an earlier run of the same test, can share it.
 	dsn := fmt.Sprintf(
 		"file:%s?mode=memory&cache=shared&_journal_mode=WAL",
-		t.Name(),
+		rand.Text(),
 	)
 
 	conn, err := sql.Open("sqlite", dsn)
@@ -79,6 +82,13 @@ func newTestEnvWithConfig(
 		t.Fatalf("open db: %v", err)
 	}
 
+	t.Cleanup(func() {
+		err := conn.Close()
+		if err != nil {
+			t.Logf("close db: %v", err)
+		}
+	})
+
 	conn.SetMaxOpenConns(1)
 
 	_, err = conn.ExecContext(
@@ -123,14 +133,9 @@ func newTestEnvWithConfig(
 		t.Fatalf("start irc server: %v", err)
 	}
 
-	t.Cleanup(func() {
+	// Cleanups run last registered first, so the server stops
-		srv.Stop()
+	// before its database is closed.
-
+	t.Cleanup(srv.Stop)
-		err := conn.Close()
-		if err != nil {
-			t.Logf("close db: %v", err)
-		}
-	})
 
 	return &testEnv{
 		database: database,
@@ -342,6 +347,20 @@ func TestRegistration(t *testing.T) {
 	assertContains(t, lines, " 001 ", "RPL_WELCOME")
 }
 
+// TestEachTestEnvHasItsOwnDatabase checks that a test
+// environment starts on an empty database while another one
+// is still open.
+func TestEachTestEnvHasItsOwnDatabase(t *testing.T) {
+	t.Parallel()
+
+	first := newTestEnv(t)
+	first.dial(t).register("samenick")
+
+	second := newTestEnv(t)
+	lines := second.dial(t).register("samenick")
+	assertContains(t, lines, " 001 ", "RPL_WELCOME")
+}
+
 func TestWelcomeContainsNick(t *testing.T) {
 	t.Parallel()
 
@@ -369,6 +388,39 @@ func TestPingPong(t *testing.T) {
 	assertContains(t, lines, "PONG", "PONG response")
 }
 
+// TestIdleCleanupKeepsActiveUser checks that a user who
+// sends commands is kept by the idle cleanup, and a user who
+// sends nothing is not.
+func TestIdleCleanupKeepsActiveUser(t *testing.T) {
+	t.Parallel()
+
+	const idleTimeout = 500 * time.Millisecond
+
+	env := newTestEnv(t)
+
+	active := env.dial(t)
+	active.register("active")
+
+	idle := env.dial(t)
+	idle.register("idle")
+
+	time.Sleep(idleTimeout)
+	active.sendAndExpect("PING :still here", "PONG")
+
+	// The idle cleanup in internal/handlers removes exactly
+	// the users this returns.
+	stale, err := env.database.GetStaleOrphanSessions(
+		t.Context(), time.Now().Add(-idleTimeout),
+	)
+	if err != nil {
+		t.Fatalf("get stale sessions: %v", err)
+	}
+
+	if len(stale) != 1 || stale[0].Nick != "idle" {
+		t.Errorf("cleanup removes %v, want only idle", stale)
+	}
+}
+
 func TestJoinChannel(t *testing.T) {
 	t.Parallel()