Stop the earlier poll loop on a second /connect (closes #228)

A second /connect started a new poll loop without stopping the first.
The first loop then polled with the new client too, so each message
could be shown twice. It also read stopPoll without a.mu while
cmdConnect wrote it.

cmdConnect now closes the earlier loop's channel in the same locked
step that replaces the client and the cursor, and hands the new
channel to the new loop. The loop checks its channel under a.mu
before each poll and again before it moves the cursor, so a stopped
loop never polls with the new client or moves the new cursor.

Model: opus-5-5

README.md

@@ -1236,7 +1236,7 @@ IRC listener sends the usual 3-digit code. Over the HTTP API, 005 arrives named
 | `001` | RPL_WELCOME          | After session creation or login, on MOTD | `{"command":"RPL_WELCOME","code":1,"to":"alice","body":["Welcome to the network, alice"]}`                                                                                                                   |
 | `002` | RPL_YOURHOST         | After session creation or login, on MOTD | `{"command":"RPL_YOURHOST","code":2,"to":"alice","body":["Your host is neoirc, running version 0.1"]}`                                                                                                       |
 | `003` | RPL_CREATED          | After session creation or login, on MOTD | `{"command":"RPL_CREATED","code":3,"to":"alice","body":["This server was created 2026-02-10"]}`                                                                                                              |
-| `004` | RPL_MYINFO           | After session creation or login, on MOTD | `{"command":"RPL_MYINFO","code":4,"to":"alice","params":["neoirc","0.1","","ikmnostl"]}`                                                                                                                     |
+| `004` | RPL_MYINFO           | After session creation or login, on MOTD | `{"command":"RPL_MYINFO","code":4,"to":"alice","params":["neoirc","0.1","ow","ikmnostl"]}`                                                                                                                   |
 | `005` | RPL_ISUPPORT         | After session creation or login, on MOTD | `{"command":"RPL_BOUNCE","code":5,"to":"alice","params":["CHANTYPES=#","NICKLEN=32","PREFIX=(ov)@+","CHANMODES=b,k,Hl,imnst","NETWORK=neoirc","CASEMAPPING=ascii"],"body":["are supported by this server"]}` |
 | `221` | RPL_UMODEIS          | In response to user MODE                 | `{"command":"RPL_UMODEIS","code":221,"to":"alice","body":["+"]}`                                                                                                                                             |
 | `251` | RPL_LUSERCLIENT      | On connect, MOTD or LUSERS               | `{"command":"RPL_LUSERCLIENT","code":251,"to":"alice","body":["There are 5 users and 0 invisible on 1 servers"]}`                                                                                            |

internal/cli/app.go

@@ -199,11 +199,21 @@ func (a *App) cmdConnect(serverURL string) {
 		return
 	}
 
+	// The poll loop of an earlier connection is stopped in the same
+	// step that replaces the client, so it never polls with the new
+	// one.
 	a.mu.Lock()
 	a.client = client
 	a.nick = resp.Nick
 	a.connected = true
 	a.lastQID = 0
+
+	if a.stopPoll != nil {
+		close(a.stopPoll)
+	}
+
+	stop := make(chan struct{})
+	a.stopPoll = stop
 	a.mu.Unlock()
 
 	a.ui.AddStatus(fmt.Sprintf(
@@ -212,9 +222,7 @@ func (a *App) cmdConnect(serverURL string) {
 	))
 	a.ui.SetStatus(resp.Nick, "", "connected")
 
-	a.stopPoll = make(chan struct{})
-
-	go a.pollLoop()
+	go a.pollLoop(stop)
 }
 
 func (a *App) cmdNick(nick string) {
@@ -702,16 +710,23 @@ func (a *App) cmdHelp() {
 	}
 }
 
-// pollLoop long-polls for messages in the background.
-func (a *App) pollLoop() {
+// pollLoop long-polls for messages in the background until stop is
+// closed. A later /connect closes stop under a.mu as it replaces the
+// client and the cursor, so the loop checks stop under a.mu too. A
+// stopped loop then neither polls with the new client nor moves the
+// new cursor.
+func (a *App) pollLoop(stop <-chan struct{}) {
 	for {
+		a.mu.Lock()
+
 		select {
-		case <-a.stopPoll:
+		case <-stop:
+			a.mu.Unlock()
+
 			return
 		default:
 		}
 
-		a.mu.Lock()
 		client := a.client
 		lastQID := a.lastQID
 		a.mu.Unlock()
@@ -729,12 +744,24 @@ func (a *App) pollLoop() {
 			continue
 		}
 
-		if result.LastID > 0 {
-			a.mu.Lock()
-			a.lastQID = result.LastID
+		// A poll sent before stop was closed answers for the earlier
+		// connection.
+		a.mu.Lock()
+
+		select {
+		case <-stop:
 			a.mu.Unlock()
+
+			return
+		default:
 		}
 
+		if result.LastID > 0 {
+			a.lastQID = result.LastID
+		}
+
+		a.mu.Unlock()
+
 		for i := range result.Messages {
 			a.handleServerMessage(&result.Messages[i])
 		}

internal/cli/app_internal_test.go

@@ -28,8 +28,9 @@ const (
 // it fails.
 const eventLoopTimeout = 5 * time.Second
 
-// holdTime is how long TestLinesKeepOrder holds the first line,
-// which gives the second line time to start too early.
+// holdTime is how long a test waits to see that something does not
+// happen: the second line handled while the first is held, or a
+// poll from a poll loop that was stopped.
 const holdTime = 200 * time.Millisecond
 
 // welcomeLines is the number of lines run adds to the status
@@ -422,6 +423,191 @@ func TestLinesKeepOrder(t *testing.T) {
 	waitHandled(t, handled, secondLine)
 }
 
+// TestSecondConnectStopsFirstPollLoop connects to one server, then
+// to a second that holds every poll. Only the second connection's
+// poll loop may poll the second server.
+func TestSecondConnectStopsFirstPollLoop(t *testing.T) {
+	t.Parallel()
+
+	const session = `{"id": 1, "nick": "erin"}`
+
+	// The first server answers every request at once, so the first
+	// poll loop polls it until the second /connect.
+	first := newServer(t, session)
+	second, polled := newHoldingServer(t, session)
+
+	app := &App{ui: NewUI(), nick: testNick}
+	runUI(t, app.ui)
+
+	app.cmdConnect(first.URL)
+	app.cmdConnect(second.URL)
+	waitPoll(t, polled, "the second server")
+
+	// The second connection's poll loop waits on its held poll, so
+	// another poll comes from the first loop.
+	select {
+	case <-polled:
+		t.Fatal("the first poll loop polled again after the second /connect")
+	case <-time.After(holdTime):
+	}
+
+	// Stop the second poll loop as /quit does. /quit itself also
+	// stops the UI, and runUI's cleanup stopping it again is a data
+	// race inside tview.
+	app.mu.Lock()
+	close(app.stopPoll)
+	app.mu.Unlock()
+}
+
+// TestSecondConnectIgnoresHeldPoll holds a poll of the earlier
+// connection at its server while a second /connect runs, then
+// answers it with a cursor and a message. It fails if that answer
+// moves the cursor or shows the message.
+func TestSecondConnectIgnoresHeldPoll(t *testing.T) {
+	t.Parallel()
+
+	const session = `{"id": 1, "nick": "erin"}`
+
+	const answer = `{"messages": [{"command": "PRIVMSG", "from": "bob",
+		"to": "#chan", "body": ["hello"]}], "last_id": 42}`
+
+	// The first server sends on polled when the poll arrives, and
+	// answers it once release is closed.
+	polled := make(chan struct{})
+	release := make(chan struct{})
+	first := httptest.NewServer(http.HandlerFunc(
+		func(writer http.ResponseWriter, request *http.Request) {
+			select {
+			case polled <- struct{}{}:
+			case <-request.Context().Done():
+				return
+			}
+
+			select {
+			case <-release:
+				_, _ = io.WriteString(writer, answer)
+			case <-request.Context().Done():
+			}
+		},
+	))
+	t.Cleanup(first.Close)
+	t.Cleanup(first.CloseClientConnections)
+
+	// The second server answers every request at once with the
+	// session. A poll answered with it has no cursor and no messages,
+	// so the second poll loop never moves the cursor.
+	second := newServer(t, session)
+
+	// The test runs the earlier connection's poll loop itself, so
+	// that it can wait for the loop to end.
+	stop := make(chan struct{})
+	app := &App{
+		ui:        NewUI(),
+		client:    api.NewClient(first.URL),
+		nick:      testNick,
+		connected: true,
+		stopPoll:  stop,
+	}
+	runUI(t, app.ui)
+
+	done := make(chan struct{})
+
+	go func() {
+		app.pollLoop(stop)
+		close(done)
+	}()
+
+	select {
+	case <-polled:
+	case <-time.After(eventLoopTimeout):
+		t.Fatalf("the first server got no poll within %v", eventLoopTimeout)
+	}
+
+	app.cmdConnect(second.URL)
+	close(release)
+
+	select {
+	case <-done:
+	case <-time.After(eventLoopTimeout):
+		t.Fatalf("the earlier poll loop did not end within %v", eventLoopTimeout)
+	}
+
+	app.mu.Lock()
+	cursor := app.lastQID
+	app.mu.Unlock()
+
+	if cursor != 0 {
+		t.Errorf("the earlier connection's answer moved the cursor to %d", cursor)
+	}
+
+	lines, _ := bufferState(app.ui, testChannel)
+	if lines != 0 {
+		t.Errorf("the earlier connection's message was shown in %s", testChannel)
+	}
+
+	// Stop the second poll loop as /quit does, without stopping the UI.
+	app.mu.Lock()
+	close(app.stopPoll)
+	app.mu.Unlock()
+}
+
+// TestSecondConnectAfterFailedPoll holds a poll of the earlier
+// connection at its server while a second /connect runs, then
+// fails it. A poll loop waits pollRetry after a failed poll, then
+// polls again. The test fails if the earlier poll loop polls the
+// second server.
+func TestSecondConnectAfterFailedPoll(t *testing.T) {
+	t.Parallel()
+
+	const session = `{"id": 1, "nick": "erin"}`
+
+	first, firstPolled := newHoldingServer(t, session)
+
+	// The second server holds the one poll of the second
+	// connection's poll loop, so any other poll comes from the
+	// earlier loop.
+	second, secondPolled := newHoldingServer(t, session)
+
+	// The test runs the earlier connection's poll loop itself, so
+	// that it can wait for the loop to end.
+	stop := make(chan struct{})
+	app := &App{
+		ui:        NewUI(),
+		client:    api.NewClient(first.URL),
+		nick:      testNick,
+		connected: true,
+		stopPoll:  stop,
+	}
+	runUI(t, app.ui)
+
+	done := make(chan struct{})
+
+	go func() {
+		app.pollLoop(stop)
+		close(done)
+	}()
+
+	waitPoll(t, firstPolled, "the first server")
+	app.cmdConnect(second.URL)
+	waitPoll(t, secondPolled, "the second server")
+
+	// Closing the held poll's connection makes that poll fail.
+	first.CloseClientConnections()
+
+	select {
+	case <-done:
+	case <-secondPolled:
+		t.Fatal("the earlier poll loop polled the second server")
+	case <-time.After(eventLoopTimeout):
+		t.Fatalf("the earlier poll loop did not end within %v", eventLoopTimeout)
+	}
+
+	// Stop the second poll loop as /quit does, without stopping the UI.
+	app.mu.Lock()
+	close(app.stopPoll)
+	app.mu.Unlock()
+}
+
 // TestStartUp runs the client's start-up path on a simulation
 // screen. It fails unless the event loop starts and the status
 // window gets two lines: first the one that holds "Welcome to
@@ -706,6 +892,53 @@ func newServer(t *testing.T, body string) *httptest.Server {
 	return server
 }
 
+// newHoldingServer starts a server that sends on the returned
+// channel for each poll and holds the poll until its connection
+// closes. It answers any other request with body.
+func newHoldingServer(
+	t *testing.T, body string,
+) (*httptest.Server, <-chan struct{}) {
+	t.Helper()
+
+	polled := make(chan struct{})
+	server := httptest.NewServer(http.HandlerFunc(
+		func(writer http.ResponseWriter, request *http.Request) {
+			if request.Method != http.MethodGet ||
+				request.URL.Path != "/api/v1/messages" {
+				_, _ = io.WriteString(writer, body)
+
+				return
+			}
+
+			select {
+			case polled <- struct{}{}:
+			case <-request.Context().Done():
+			}
+
+			<-request.Context().Done()
+		},
+	))
+	// Close waits for the held polls, which end only once their
+	// connections close. Cleanups run last first, so the connections
+	// close before Close runs.
+	t.Cleanup(server.Close)
+	t.Cleanup(server.CloseClientConnections)
+
+	return server, polled
+}
+
+// waitPoll fails the test unless server gets a poll, sent on
+// polled, within eventLoopTimeout.
+func waitPoll(t *testing.T, polled <-chan struct{}, server string) {
+	t.Helper()
+
+	select {
+	case <-polled:
+	case <-time.After(eventLoopTimeout):
+		t.Fatalf("%s got no poll within %v", server, eventLoopTimeout)
+	}
+}
+
 // runUI runs ui on a simulated screen until the test ends.
 func runUI(t *testing.T, ui *UI) {
 	t.Helper()

internal/handlers/api.go

@@ -412,7 +412,7 @@ func (hdlr *Handlers) deliverWelcome(
 	// 004 RPL_MYINFO
 	hdlr.enqueueNumeric(
 		ctx, clientID, irc.RplMyInfo, nick,
-		[]string{srvName, version, "", "ikmnostl"},
+		[]string{srvName, version, service.UserModes, "ikmnostl"},
 		"",
 	)
 

internal/handlers/api_test.go

@@ -6797,6 +6797,30 @@ func TestISUPPORT(t *testing.T) {
 	}
 }
 
+// TestMyInfoListsUserModes verifies the 004 numeric lists
+// the user modes o and w as its third parameter.
+func TestMyInfoListsUserModes(t *testing.T) {
+	t.Parallel()
+
+	tserver := newTestServer(t)
+	token := tserver.createSession("myinfo")
+
+	msgs, _ := tserver.pollMessages(token, 0)
+
+	myInfo := findNumericWithParams(msgs, "004")
+	if myInfo == nil {
+		t.Fatal("expected 004 RPL_MYINFO")
+	}
+
+	params := getNumericParams(myInfo)
+	if len(params) != 4 || params[2] != "ow" {
+		t.Fatalf(
+			"004 params %v, want four with user modes ow third",
+			params,
+		)
+	}
+}
+
 // TestNonOpCannotSetModes verifies non-operators
 // cannot set +i, +s, +k, +l, +b.
 func TestNonOpCannotSetModes(t *testing.T) {

internal/ircserver/conn.go

@@ -605,7 +605,7 @@ func (c *Conn) deliverWelcome() {
 	)
 	c.sendNumeric(
 		irc.RplMyInfo,
-		c.serverSfx, "neoirc", "", "mnst",
+		c.serverSfx, "neoirc", service.UserModes, "mnst",
 	)
 	c.sendNumeric(
 		irc.RplIsupport,

internal/ircserver/server_test.go

@@ -486,6 +486,22 @@ func TestWelcomeContainsNick(t *testing.T) {
 	}
 }
 
+// TestMyInfoListsUserModes checks that RPL_MYINFO lists the
+// user modes o and w, with no empty parameter before the
+// channel modes.
+func TestMyInfoListsUserModes(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	lines := env.dial(t).register("carol")
+
+	assertContains(
+		t, lines,
+		":test.irc 004 carol test.irc neoirc ow mnst",
+		"RPL_MYINFO with user modes",
+	)
+}
+
 func TestPingPong(t *testing.T) {
 	t.Parallel()
 

internal/service/service.go

@@ -1169,6 +1169,12 @@ func (s *Service) ApplyUserMode(
 	return s.QueryUserMode(ctx, sessionID)
 }
 
+// UserModes lists the user modes the server supports, as
+// RPL_MYINFO sends them: o (server operator) and w
+// (receives WALLOPS). parseUserModeString must accept the
+// same letters.
+const UserModes = "ow"
+
 // parseUserModeString parses a user mode string such as
 // "+w", "-o" or "+w-o" into the new value of each flag it
 // names, or nil for a flag it does not name; a later letter

schema/README.md

@@ -144,8 +144,8 @@ Where a schema file and the HTTP API disagree, the HTTP API is right:
   with `ERR_NEEDMOREPARAMS` (`461`). A `body` of `[""]` clears the topic.
 - `003.json` describes the server's creation date. The HTTP API sends the date
   the server last started.
-- `004.json` has example mode lists. The HTTP API sends an empty string as the
-  user modes and `ikmnostl` as the channel modes.
+- `004.json` has example mode lists. The HTTP API sends `ow` as the user modes
+  and `ikmnostl` as the channel modes.
 - `322.json` says `LIST` gets one reply per channel. The server leaves out a
   secret channel, one with mode `+s`, unless the user is on it.
 - `332.json` says `RPL_TOPIC` answers a `TOPIC` query. The HTTP API has no such