Stop the chat client only after the API has stopped (closes #4)

The chat client's context no longer follows the bot's, so a stop no
longer sends it SIGTERM while the API is still finishing its requests.
The deferred stop in Run, which runs after the API's, stops it instead,
so a request in progress at a stop gets its answer rather than a 500.
The README says so.

The new test runs the whole bot with the test binary itself standing in
for simplex-chat on PATH. The stand-in holds its answer to the contacts
request until the test has told the bot to stop.

Model: opus-5-5

internal/bot/bot.go

@@ -67,7 +67,9 @@ func Run(ctx context.Context, log *slog.Logger, cfg *config.Config) error {
 
 	// Cancelling this stops the chat client; the deferred wait makes
 	// Run return only once it has exited, whatever path Run takes.
-	cliCtx, stopCLI := context.WithCancel(ctx)
+	// Cancelling ctx does not reach it, so that the API, stopped first,
+	// can finish its requests while the chat client still answers.
+	cliCtx, stopCLI := context.WithCancel(context.WithoutCancel(ctx))
 
 	cli, err := simplex.StartCLI(cliCtx, log, filepath.Join(cfg.DataDir, "simplex"),
 		DisplayName, chatPort)

internal/bot/run_test.go

@@ -0,0 +1,233 @@
+package bot_test
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"log/slog"
+	"net"
+	"net/http"
+	"net/netip"
+	"os"
+	"path/filepath"
+	"slices"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gorilla/websocket"
+	"sneak.berlin/go/simplexcalc/internal/bot"
+	"sneak.berlin/go/simplexcalc/internal/config"
+	"sneak.berlin/go/simplexcalc/internal/simplex"
+)
+
+const (
+	// credential is what the bot's API is configured with here.
+	credential = "a-credential-for-these-tests" //nolint:gosec // G101: invented for tests
+
+	// asked is the file the stand-in chat client creates in the data
+	// directory when it is asked for the contacts.
+	asked = "asked-for-contacts"
+
+	// contactsDelay is how long the stand-in then holds its answer:
+	// long enough for the test to stop the bot meanwhile, and well
+	// within the 5 seconds the API gets to finish its requests.
+	contactsDelay = time.Second
+)
+
+// TestMain lets this test binary be the chat client as well: started
+// under the chat client's name, as TestStopDuringRequest arranges, it is
+// the stand-in instead of running the tests.
+func TestMain(m *testing.M) {
+	if filepath.Base(os.Args[0]) == simplex.Binary {
+		standIn() // never returns
+	}
+
+	m.Run()
+}
+
+// standIn plays the chat client: it serves the WebSocket API on the
+// port it is given, on localhost, and answers the commands the bot
+// sends. SIGTERM ends it, as it ends the real client. Unlike the real
+// client, it also exits when the bot hangs up, so that it never
+// outlives a test run that was cut short. It takes port 5225, the chat
+// client's fixed port, so this test cannot run twice at once on one
+// machine.
+func standIn() {
+	arg := func(name string) string {
+		return os.Args[slices.Index(os.Args, name)+1]
+	}
+
+	marker := filepath.Join(filepath.Dir(arg("--database")), asked)
+
+	srv := &http.Server{
+		Addr:              "127.0.0.1:" + arg("--chat-server-port"),
+		ReadHeaderTimeout: time.Second,
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			answer(w, r, marker)
+		}),
+	}
+
+	err := srv.ListenAndServe()
+	_, _ = fmt.Fprintln(os.Stderr, err)
+
+	os.Exit(1)
+}
+
+// answer answers the commands on one connection with records reduced to
+// the fields the bot reads. Asked for the contacts, it first creates
+// the file marker, then holds its answer for contactsDelay.
+func answer(w http.ResponseWriter, r *http.Request, marker string) {
+	conn, err := (&websocket.Upgrader{}).Upgrade(w, r, nil)
+	if err != nil {
+		return
+	}
+
+	records := map[string]string{
+		"/user":              `{"type":"activeUser","user":{"userId":1}}`,
+		"/_show_address":     `{"type":"userContactLink","contactLink":{}}`,
+		"/_address_settings": `{"type":"userContactLinkUpdated"}`,
+		"/_contacts": `{"type":"contactsList","contacts":[{"contactId":3,` +
+			`"profile":{"displayName":"tester"},"contactStatus":"active"}]}`,
+	}
+
+	for {
+		var cmd map[string]string
+
+		err = conn.ReadJSON(&cmd)
+		if err != nil {
+			os.Exit(0)
+		}
+
+		name, _, _ := strings.Cut(cmd["cmd"], " ")
+
+		record, ok := records[name]
+		if !ok {
+			continue
+		}
+
+		if name == "/_contacts" {
+			_ = os.WriteFile(marker, nil, 0o600)
+
+			time.Sleep(contactsDelay)
+		}
+
+		_ = conn.WriteJSON(map[string]any{
+			"corrId": cmd["corrId"],
+			"resp":   json.RawMessage(record),
+		})
+	}
+}
+
+// TestStopDuringRequest: a request that is waiting on the chat client
+// when the bot is told to stop still gets the chat client's answer,
+// because the chat client is stopped only once the API has stopped.
+func TestStopDuringRequest(t *testing.T) {
+	// Run starts the chat client from PATH: put this test binary there
+	// under the chat client's name.
+	bin := t.TempDir()
+
+	exe, err := os.Executable()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = os.Symlink(exe, filepath.Join(bin, simplex.Binary))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Setenv("PATH", bin)
+
+	cfg := &config.Config{DataDir: t.TempDir(), Port: freePort(t), APIToken: credential}
+	ctx, stop := context.WithCancel(t.Context())
+	done := make(chan struct{})
+
+	var runErr error
+
+	go func() {
+		defer close(done)
+
+		runErr = bot.Run(ctx, slog.New(slog.DiscardHandler), cfg)
+	}()
+
+	t.Cleanup(func() {
+		stop()
+		<-done
+
+		if runErr != nil {
+			t.Errorf("Run: %v", runErr)
+		}
+	})
+
+	// Stop the bot once the request below is waiting on the chat client.
+	go func() {
+		for ctx.Err() == nil {
+			_, err := os.Stat(filepath.Join(cfg.DataDir, asked))
+			if err == nil {
+				stop()
+			}
+
+			time.Sleep(10 * time.Millisecond)
+		}
+	}()
+
+	status, body := getChats(t, cfg.Port, done)
+
+	want := `{"chats":[{"id":3,"display_name":"tester","contact_deleted":false}]}` + "\n"
+	if status != http.StatusOK || body != want {
+		t.Errorf("GET /api/v1/chats = %d %q, want 200 %q", status, body, want)
+	}
+}
+
+// freePort returns a TCP port that nothing listens on at the moment.
+func freePort(t *testing.T) int {
+	t.Helper()
+
+	l, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", ":0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() { _ = l.Close() }()
+
+	return int(netip.MustParseAddrPort(l.Addr().String()).Port())
+}
+
+// getChats asks the bot's API for the chats, trying again while the API
+// is not listening yet, and returns the answer's status and body. It
+// fails the test if Run returns first, which closes done.
+func getChats(t *testing.T, port int, done <-chan struct{}) (int, string) {
+	t.Helper()
+
+	url := "http://127.0.0.1:" + strconv.Itoa(port) + "/api/v1/chats"
+
+	for {
+		req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, url, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		req.Header.Set("Authorization", "Bearer "+credential)
+
+		resp, err := http.DefaultClient.Do(req)
+		if err == nil {
+			body, err := io.ReadAll(resp.Body)
+			_ = resp.Body.Close()
+
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			return resp.StatusCode, string(body)
+		}
+
+		select {
+		case <-done:
+			t.Fatal("Run returned before the API answered")
+		case <-time.After(50 * time.Millisecond):
+		}
+	}
+}