From 557f3ed86e0745a0eeda30a7bb75cb5c1da3f2b5 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 27 Oct 2026 03:42:55 +0000 Subject: [PATCH] Open a channel only when the server accepts the join (closes #197) When the server refuses a JOIN, POST /api/v1/messages answers 200 with status "error" and sends the error numeric to the queue. Both clients opened the channel anyway. JoinChannel in internal/cli/api now returns an error unless the status is "joined". cmdJoin then keeps its buffer and target and prints its usual "Join failed" line. The web client opens no tab for a refused join, whether it came from /join or from the saved channels a fresh session joins again. Model: opus-5-5 --- internal/cli/api/client.go | 31 ++++++++++-- internal/cli/app_internal_test.go | 78 +++++++++++++++++++++++++++++-- web/src/app.jsx | 8 +++- 3 files changed, 107 insertions(+), 10 deletions(-) diff --git a/internal/cli/api/client.go b/internal/cli/api/client.go index 91222a8..78f746b 100644 --- a/internal/cli/api/client.go +++ b/internal/cli/api/client.go @@ -25,7 +25,10 @@ const ( httpErrThreshold = 400 ) -var errHTTP = errors.New("HTTP error") +var ( + errHTTP = errors.New("HTTP error") + errJoinRefused = errors.New("the server refused the join") +) // Client wraps HTTP calls to the neoirc server API. type Client struct { @@ -206,13 +209,35 @@ func (client *Client) PollMessages( }, nil } -// JoinChannel joins a channel. +// JoinChannel joins a channel. It returns an error unless the +// server answers that the session joined. When the server +// refuses the join, it sends the reason to the message queue as +// an error numeric. func (client *Client) JoinChannel(channel string) error { - return client.SendMessage( + data, err := client.do( + http.MethodPost, "/api/v1/messages", &Message{ //nolint:exhaustruct // only command+to needed Command: irc.CmdJoin, To: channel, }, ) + if err != nil { + return err + } + + var resp struct { + Status string `json:"status"` + } + + err = json.Unmarshal(data, &resp) + if err != nil { + return fmt.Errorf("decode join: %w", err) + } + + if resp.Status != "joined" { + return errJoinRefused + } + + return nil } // PartChannel leaves a channel. diff --git a/internal/cli/app_internal_test.go b/internal/cli/app_internal_test.go index bb9e49f..15dba69 100644 --- a/internal/cli/app_internal_test.go +++ b/internal/cli/app_internal_test.go @@ -1,6 +1,7 @@ package cli import ( + "io" "net/http" "net/http/httptest" "testing" @@ -138,11 +139,9 @@ func TestOwnCommandShownOnce(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - // The server accepts every command and answers nothing. - server := httptest.NewServer(http.HandlerFunc( - func(http.ResponseWriter, *http.Request) {}, - )) - t.Cleanup(server.Close) + // The server accepts every command. Only the response + // to a JOIN is read, and it says the session joined. + server := newServer(t, `{"status": "joined"}`) app := &App{ ui: NewUI(), @@ -166,6 +165,60 @@ func TestOwnCommandShownOnce(t *testing.T) { } } +// TestJoinResponse checks that /join switches to the channel's +// buffer and makes it the target only when the server answers +// that the session joined. +func TestJoinResponse(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + response string + // The buffer shown and the target after the join. + want string + }{ + { + name: "joined", + response: `{"status": "joined", "channel": "#chan"}`, + want: testChannel, + }, + { + name: "refused", + response: `{"status": "error"}`, + want: testOtherNick, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + app := &App{ + ui: NewUI(), + client: api.NewClient(newServer(t, test.response).URL), + nick: testNick, + target: testOtherNick, + connected: true, + } + runUI(t, app.ui) + app.ui.SwitchToBuffer(testOtherNick) + + app.cmdJoin(testChannel) + + var shown string + + app.ui.app.QueueUpdate(func() { + shown = app.ui.buffers[app.ui.currentBuffer].Name + }) + + if shown != test.want || app.target != test.want { + t.Fatalf("got buffer %s and target %s, want %s", + shown, app.target, test.want) + } + }) + } +} + // TestOwnEventFromOtherClient checks that a message, notice, // join or part that the user's session makes from another // client shows as one line in the buffer it belongs to. @@ -248,6 +301,21 @@ func bufferState(ui *UI, name string) (int, int) { return lines, unread } +// newServer starts a server that answers every request with +// body until the test ends. +func newServer(t *testing.T, body string) *httptest.Server { + t.Helper() + + server := httptest.NewServer(http.HandlerFunc( + func(writer http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(writer, body) + }, + )) + t.Cleanup(server.Close) + + return server +} + // runUI runs ui on a simulated screen until the test ends. func runUI(t *testing.T, ui *UI) { t.Helper() diff --git a/web/src/app.jsx b/web/src/app.jsx index c707e1b..cd18ff1 100644 --- a/web/src/app.jsx +++ b/web/src/app.jsx @@ -831,10 +831,11 @@ function App() { // Fresh session — join any previously saved channels. for (const ch of saved) { try { - await api("/messages", { + const res = await api("/messages", { method: "POST", body: JSON.stringify({ command: "JOIN", to: ch }), }); + if (res?.status !== "joined") continue; setTabs((prev) => { if (prev.find((t) => t.type === "channel" && t.name === ch)) return prev; @@ -854,10 +855,13 @@ function App() { name = name.trim(); if (!name.startsWith("#")) name = "#" + name; try { - await api("/messages", { + const res = await api("/messages", { method: "POST", body: JSON.stringify({ command: "JOIN", to: name }), }); + // A refused join opens no tab. The error numeric that the + // server sends to the message queue tells the user why. + if (res?.status !== "joined") return; // Show the channel's tab. Preact runs this update at once on the // newest tab list, so the tab is found there by name: the user's // own JOIN event may already have added it, and other tabs may -- 2.54.0