main() is one call into internal/cli (closes #160)

cmd/netwatch-server/main.go parsed the command line, ran the check-cidr
and prepare-data-dir subcommands and built the fx application itself;
the Go style guide puts all of that in internal/ or pkg/. The new
package internal/cli does it, with the same arguments, messages and
exit codes. main() is now os.Exit(cli.Main(Appname, Version,
os.Args[1:], cli.RunServer)): Main takes the function that runs the
server, so its tests call it with one that fails them. Appname and
Version stay in main, where the build sets Version. Each subcommand
writes its error to a writer it is given, and prepare-data-dir also
takes the volume and the user's name, so its own tests give it a
temporary directory and the user they run as.

Model: opus-5-5

README.md

@@ -199,9 +199,7 @@ stops. Its routes:
   address may send a limited number a minute, and the report files are capped in
   size, the oldest deleted first
 - `GET /.well-known/healthcheck` — answers 200 with `"status":"ok"`, the
-  server's version and its uptime; while reports cannot be stored, as the last
+  server's version and its uptime
-  write of a report file failed, it answers 503 with `"status":"error"` and the
-  same fields (see [Report storage](#report-storage))
 - `GET /metrics` — Prometheus metrics behind basic auth, only when
   `METRICS_USERNAME` and `METRICS_PASSWORD` are set; each client address may
   make a limited number of requests to it a minute
@@ -279,11 +277,10 @@ The reports of a failed write stay in memory, and the next flush, within a
 minute, tries again to write them. The server writes one file at a time, so a
 file holds its reports in the order they arrived, those of a failed write
 included. From the failed write until a write succeeds, `POST /api/v1/reports`
-refuses every report with 500 and keeps nothing of it, and the health check
+refuses every report with 500 and keeps nothing of it. The page sends the
-answers 503 with `"status":"error"`. The page sends the refused samples again in
+refused samples again in its next report, as long as it still holds them: at the
-its next report, as long as it still holds them: at the 1-second interval one
+1-second interval one refused report already loses some of those in the history
-refused report already loses some of those in the history (see
+(see [Reporting](#reporting)). Reports already answered 200 are lost only if the
-[Reporting](#reporting)). Reports already answered 200 are lost only if the
 server stops before a write succeeds; the final flush at stop tries them too,
 and if it fails, the process exits non-zero.
 
@@ -557,16 +554,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
       stops the container. Empty, the backend sends nothing to Sentry
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless
-  both nginx and the backend answer. It also fails while reports cannot be
+  both nginx and the backend answer. upaas reads the container's health 60
-  stored, as when the volume is full or `DATA_DIR` cannot be written: the health
+  seconds after a deploy and fails the deploy unless it is `healthy`. The
-  check answers 503 from a failed write of a report file until a write succeeds,
+  container also stops when either process exits.
-  and the backend tries the write again every minute. Three failed checks in a
-  row make the container `unhealthy`, and one that passes makes it `healthy`
-  again. upaas reads the container's health once, 60 seconds after a deploy, and
-  fails the deploy unless it is `healthy`. A container that turns `unhealthy`
-  after that is neither restarted nor reported by upaas, whose `unless-stopped`
-  restart policy ignores health; only `docker ps` and `docker inspect` show it.
-  The container also stops when either process exits.
 
 ## Browser Compatibility
 

TODO.md

@@ -27,22 +27,12 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
   the `check-cidr` and `prepare-data-dir` subcommands, with the same arguments,
   messages and exit codes, and otherwise the server, with the `fx` wiring.
   `Appname` and `Version` stay in `main`, where the build sets `Version`, and
-  `main` gives `cli.Main` stderr and the function that runs the server, which
+  `main` gives `cli.Main` the function that runs the server, which the tests
-  the tests replace with a buffer and a function that fails them. Unit tests
+  replace with one that fails them. Unit tests call `cli.Main` with `check-cidr`
-  call `cli.Main` with `check-cidr` and a CIDR it takes and one it refuses, and
+  and a CIDR it takes and one it refuses, and with `prepare-data-dir` and a path
-  check the exit code, and with `prepare-data-dir` and a path outside `/data`,
+  outside `/data`, and check the exit code. Others give each subcommand input it
-  and check that it exits 1 with the error from looking up the `netwatch` user,
+  takes and input it refuses, and check the exit code and what it writes to
-  which the test image does not have. Others give each subcommand input it takes
+  stderr
-  and input it refuses, and check the exit code and what it writes to stderr
-- 2026-10-19: while report files cannot be written, the health check answers 503
-  with `"status":"error"` and the same fields, so the image's `HEALTHCHECK`
-  reports the container `unhealthy`
-  ([#142](https://git.eeqj.de/sneak/netwatch/issues/142)). It answered 200
-  `"status":"ok"` whatever happened, so the platform saw a healthy container
-  while every report was refused with 500. It answers 200 at start, before any
-  write has failed, and again from the first write that succeeds. A unit test
-  removes `DATA_DIR` to make a write fail and checks the 503, then puts it back
-  and checks the 200 after the next write
 - 2026-10-19: report file writes no longer overlap
   ([#150](https://git.eeqj.de/sneak/netwatch/issues/150)), so the reports of
   failed writes are written in the order they arrived. Two writes that failed at

cmd/netwatch-server/main.go

@@ -15,5 +15,5 @@ var (
 )
 
 func main() {
-	os.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer))
+	os.Exit(cli.Main(Appname, Version, os.Args[1:], cli.RunServer))
 }

internal/cli/cli.go

@@ -7,6 +7,7 @@ package cli
 import (
 	"fmt"
 	"io"
+	"os"
 	"os/user"
 
 	"sneak.berlin/go/netwatch/internal/config"
@@ -23,17 +24,15 @@ import (
 )
 
 // Main runs netwatch-server with args, its command line after the
-// program's name, and returns the exit code; the subcommands write
+// program's name, and returns the exit code. Unless args are
-// their errors to stderr. Unless args are "check-cidr CIDR" or
+// "check-cidr CIDR" or "prepare-data-dir DATA_DIR", Main runs the
-// "prepare-data-dir DATA_DIR", Main runs the server by calling serve
+// server by calling serve with appname and version, the name and
-// with appname and version, the name and version the server logs and
+// version the server logs and reports. main passes RunServer; the
-// reports. main passes os.Stderr and RunServer; the tests pass a
+// tests pass a function that fails them, so that no test starts the
-// buffer and a function that fails them, so that no test starts the
 // server. When serve is RunServer and the server fails to start or to
 // stop, fx's Run exits the process with status 1 and Main never
 // returns.
 func Main(
-	stderr io.Writer,
 	appname, version string,
 	args []string,
 	serve func(appname, version string),
@@ -42,7 +41,7 @@ func Main(
 	// this server would refuse CIDR in its TRUSTED_PROXIES.
 	// bin/entrypoint.sh runs it on each entry it gives nginx.
 	if len(args) == 2 && args[0] == "check-cidr" {
-		return checkCIDR(stderr, args[1])
+		return checkCIDR(os.Stderr, args[1])
 	}
 
 	// "netwatch-server prepare-data-dir DATA_DIR" gets DATA_DIR ready
@@ -50,7 +49,7 @@ func Main(
 	// reportbuf.PrepareDataDir. bin/entrypoint.sh runs it as root
 	// before it starts this server as that user.
 	if len(args) == 2 && args[0] == "prepare-data-dir" {
-		return prepareDataDir(stderr, "/data", args[1], "netwatch")
+		return prepareDataDir(os.Stderr, "/data", args[1], "netwatch")
 	}
 
 	serve(appname, version)

internal/cli/cli_test.go

@@ -2,7 +2,6 @@ package cli_test
 
 import (
 	"bytes"
-	"io"
 	"os"
 	"os/user"
 	"path/filepath"
@@ -21,31 +20,25 @@ func TestMainRunsCheckCIDR(t *testing.T) {
 	t.Parallel()
 
 	for cidr, want := range map[string]int{"10.0.0.0/8": 0, "1.2.3/32": 1} {
-		code := cli.Main(io.Discard, "netwatch", "test",
+		code := cli.Main("netwatch", "test", []string{"check-cidr", cidr},
-			[]string{"check-cidr", cidr}, failIfServerStarts(t))
+			failIfServerStarts(t))
 		if code != want {
 			t.Errorf("check-cidr %q: exit %d, want %d", cidr, code, want)
 		}
 	}
 }
 
-// TestMainRunsPrepareDataDir: Main given prepare-data-dir runs the
+// TestMainRunsPrepareDataDir: Main given prepare-data-dir returns 1 for
-// prepare-data-dir subcommand for the netwatch user, and does not start
+// a DATA_DIR outside /data, and does not start the server. The test
-// the server. The test image has no netwatch user, so that subcommand
+// image has no netwatch user, so there the 1 comes from looking that
-// exits 1 with the error from looking the user up;
+// user up; TestPrepareDataDirRefusesPathOutsideVolume checks the path.
-// TestPrepareDataDirRefusesPathOutsideVolume checks the path.
 func TestMainRunsPrepareDataDir(t *testing.T) {
 	t.Parallel()
 
-	var stderr bytes.Buffer
+	code := cli.Main("netwatch", "test", []string{"prepare-data-dir", "/etc"},
-
+		failIfServerStarts(t))
-	code := cli.Main(&stderr, "netwatch", "test",
+	if code != 1 {
-		[]string{"prepare-data-dir", "/etc"}, failIfServerStarts(t))
+		t.Errorf("prepare-data-dir /etc: exit %d, want 1", code)
-
-	want := user.UnknownUserError("netwatch").Error() + "\n"
-	if code != 1 || stderr.String() != want {
-		t.Errorf("prepare-data-dir /etc: exit %d, stderr %q; want exit 1 and %q",
-			code, stderr.String(), want)
 	}
 }
 

internal/config/config_test.go

@@ -13,8 +13,8 @@ import (
 	"go.uber.org/fx"
 )
 
-// requireConfigError builds the config as cli.RunServer does and fails
+// requireConfigError builds the config as main does and fails the
-// the test unless that fails with an error naming setting. It uses
+// test unless that fails with an error naming setting. It uses
 // fx.New, because fxtest.New fails the test itself on an error.
 func requireConfigError(t *testing.T, setting string) {
 	t.Helper()
@@ -31,8 +31,8 @@ func requireConfigError(t *testing.T, setting string) {
 	}
 }
 
-// requireTrustedProxies builds the config as cli.RunServer does and
+// requireTrustedProxies builds the config as main does and fails the
-// fails the test unless its TrustedProxies are want.
+// test unless its TrustedProxies are want.
 func requireTrustedProxies(t *testing.T, want []string) {
 	t.Helper()
 

internal/handlers/healthcheck.go

@@ -3,17 +3,9 @@ package handlers
 import "net/http"
 
 // HandleHealthCheck returns a handler for the health check
-// endpoint. It answers 200 while the status is "ok" and 503 otherwise,
+// endpoint.
-// so that the image's HEALTHCHECK fails.
 func (s *Handlers) HandleHealthCheck() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		resp := s.hc.Healthcheck()
+		s.respondJSON(w, r, s.hc.Healthcheck(), http.StatusOK)
-
-		code := http.StatusOK
-		if resp.Status != "ok" {
-			code = http.StatusServiceUnavailable
-		}
-
-		s.respondJSON(w, r, resp, code)
 	}
 }

internal/handlers/healthcheck_test.go

@@ -9,19 +9,17 @@ import (
 	"testing"
 	"time"
 
-	"sneak.berlin/go/netwatch/internal/config"
 	"sneak.berlin/go/netwatch/internal/globals"
 	"sneak.berlin/go/netwatch/internal/handlers"
 	"sneak.berlin/go/netwatch/internal/healthcheck"
 	"sneak.berlin/go/netwatch/internal/logger"
-	"sneak.berlin/go/netwatch/internal/reportbuf"
 
 	"go.uber.org/fx/fxtest"
 )
 
 // newStartedHandlers builds Handlers with a real health check for the
-// server named in g, and a report buffer in a temporary directory, and
+// server named in g, and starts them, which records the time the
-// starts them, which records the time the uptime counts from.
+// uptime counts from.
 func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers {
 	t.Helper()
 
@@ -32,23 +30,14 @@ func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers {
 		t.Fatalf("logger: %v", err)
 	}
 
-	buf, err := reportbuf.New(lc, reportbuf.Params{
-		Config: &config.Config{DataDir: t.TempDir(), DataDirMaxBytes: 1 << 20},
-		Logger: log,
-	})
-	if err != nil {
-		t.Fatalf("report buffer: %v", err)
-	}
-
 	hc, err := healthcheck.New(lc,
-		healthcheck.Params{Buffer: buf, Globals: g, Logger: log})
+		healthcheck.Params{Globals: g, Logger: log})
 	if err != nil {
 		t.Fatalf("health check: %v", err)
 	}
 
-	h, err := handlers.New(lc, handlers.Params{
+	h, err := handlers.New(lc,
-		Buffer: buf, Globals: g, Healthcheck: hc, Logger: log,
+		handlers.Params{Globals: g, Healthcheck: hc, Logger: log})
-	})
 	if err != nil {
 		t.Fatalf("handlers: %v", err)
 	}
@@ -59,12 +48,10 @@ func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers {
 	return h
 }
 
-// TestHandleHealthCheck checks the health check's answer at start, when
+// TestHandleHealthCheck checks the health check's answer: 200, a JSON
-// no write of a report file has failed: 200, a JSON content type, and a
+// content type, and a JSON object with exactly the fields of
-// JSON object with exactly the fields of healthcheck.HealthcheckResponse,
+// healthcheck.HealthcheckResponse, carrying this server's name and
-// carrying "status":"ok", this server's name and version and an uptime
+// version and an uptime counted from its start.
-// counted from its start. The answer while writes fail is checked in
-// internal/reportbuf, whose tests can make a write fail.
 func TestHandleHealthCheck(t *testing.T) {
 	t.Parallel()
 

internal/healthcheck/healthcheck.go

@@ -10,7 +10,6 @@ import (
 	"sneak.berlin/go/netwatch/internal/config"
 	"sneak.berlin/go/netwatch/internal/globals"
 	"sneak.berlin/go/netwatch/internal/logger"
-	"sneak.berlin/go/netwatch/internal/reportbuf"
 
 	"go.uber.org/fx"
 )
@@ -19,7 +18,6 @@ import (
 type Params struct {
 	fx.In
 
-	Buffer  *reportbuf.Buffer
 	Config  *config.Config
 	Globals *globals.Globals
 	Logger  *logger.Logger
@@ -70,18 +68,12 @@ func New(
 	return s, nil
 }
 
-// Healthcheck returns the current health status of the application:
+// Healthcheck returns the current health status of the application.
-// "ok", or "error" while report files cannot be written.
 func (s *Healthcheck) Healthcheck() *HealthcheckResponse {
-	status := "ok"
-	if s.params.Buffer.LastWriteFailed() {
-		status = "error"
-	}
-
 	return &HealthcheckResponse{
 		Appname:       s.params.Globals.Appname,
 		Now:           time.Now().UTC().Format(time.RFC3339Nano),
-		Status:        status,
+		Status:        "ok",
 		UptimeHuman:   s.uptime().String(),
 		UptimeSeconds: int64(s.uptime().Seconds()),
 		Version:       s.params.Globals.Version,

internal/reportbuf/reportbuf.go

@@ -255,16 +255,6 @@ func (b *Buffer) Append(v any) error {
 	return nil
 }
 
-// LastWriteFailed reports whether the last write of a report file to
-// finish failed: false at start until a write fails, and false again
-// once a write succeeds. While it is true, Append refuses reports.
-func (b *Buffer) LastWriteFailed() bool {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
-	return b.writeErr != nil
-}
-
 // deleteOldestFiles deletes report files, oldest first, until n more
 // bytes fit under maxBytes. It deletes none when the reports waiting
 // to be written leave no room for n even with every file gone, since

internal/reportbuf/reportbuf_test.go

@@ -334,9 +334,9 @@ func stopWhileHeld(
 	return <-stopped
 }
 
-// startApp starts a Buffer through fx, as cli.RunServer does, with the
+// startApp starts a Buffer through fx, as main does, with the DATA_DIR
-// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set, and returns
+// and DATA_DIR_MAX_BYTES the calling test has set, and returns it with
-// it with the app, for tests that stop the app themselves.
+// the app, for tests that stop the app themselves.
 func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	t.Helper()
 
@@ -358,8 +358,8 @@ func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	return buf, app
 }
 
-// startBuffer starts a Buffer through fx, as cli.RunServer does, with
+// startBuffer starts a Buffer through fx, as main does, with the
-// the DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
+// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
 func startBuffer(t *testing.T) *reportbuf.Buffer {
 	t.Helper()
 
@@ -1074,54 +1074,9 @@ func TestReportsRefusedWhileWritesFail(t *testing.T) {
 	}
 }
 
-// TestHealthCheckFailsWhileWritesFail asks the health check handler
-// for its answer around a failed write. It answers 200 with
-// "status":"ok" at start, 503 with "status":"error" from the failed
-// write until a write succeeds, and 200 with "status":"ok" again after.
-func TestHealthCheckFailsWhileWritesFail(t *testing.T) {
-	dir := t.TempDir()
-	t.Setenv("DATA_DIR", dir)
-
-	buf, h := startBufferWithHandlers(t)
-
-	checkHealthCheck(t, h, "at start", http.StatusOK, "ok")
-
-	// A flush writes a file only when a report is waiting.
-	err := buf.Append(map[string]string{"id": "kept"})
-	if err != nil {
-		t.Fatalf("append: %v", err)
-	}
-
-	// With the data directory gone, the write has nowhere to go.
-	err = os.Remove(dir)
-	if err != nil {
-		t.Fatalf("remove data dir: %v", err)
-	}
-
-	err = buf.Flush()
-	if err == nil {
-		t.Fatal("flush succeeded with the data directory gone")
-	}
-
-	checkHealthCheck(t, h, "after the failed write",
-		http.StatusServiceUnavailable, "error")
-
-	err = os.Mkdir(dir, 0o750)
-	if err != nil {
-		t.Fatalf("make data dir: %v", err)
-	}
-
-	err = buf.Flush()
-	if err != nil {
-		t.Fatalf("flush with the data directory back: %v", err)
-	}
-
-	checkHealthCheck(t, h, "after a write succeeded", http.StatusOK, "ok")
-}
-
 // startBufferWithHandlers starts a Buffer and the Handlers that store
-// reports in it through fx, as cli.RunServer does, with the DATA_DIR
+// reports in it through fx, as main does, with the DATA_DIR the calling
-// the calling test has set.
+// test has set.
 func startBufferWithHandlers(
 	t *testing.T,
 ) (*reportbuf.Buffer, *handlers.Handlers) {
@@ -1166,39 +1121,6 @@ func postReport(t *testing.T, h *handlers.Handlers, clientID string) int {
 	return rec.Code
 }
 
-// checkHealthCheck asks the health check handler for its answer, and
-// fails the test, saying when it asked, unless the answer has status
-// code wantCode and a "status" field of wantStatus.
-func checkHealthCheck(
-	t *testing.T,
-	h *handlers.Handlers,
-	when string,
-	wantCode int,
-	wantStatus string,
-) {
-	t.Helper()
-
-	rec := httptest.NewRecorder()
-	req := httptest.NewRequestWithContext(t.Context(),
-		http.MethodGet, "/.well-known/healthcheck", http.NoBody)
-	h.HandleHealthCheck().ServeHTTP(rec, req)
-
-	var body struct {
-		Status string `json:"status"`
-	}
-
-	err := json.Unmarshal(rec.Body.Bytes(), &body)
-	if err != nil {
-		t.Fatalf("health check %s: body not JSON: %v (%q)",
-			when, err, rec.Body.String())
-	}
-
-	if rec.Code != wantCode || body.Status != wantStatus {
-		t.Fatalf("health check %s: %d with status %q, want %d with %q",
-			when, rec.Code, body.Status, wantCode, wantStatus)
-	}
-}
-
 // TestConcurrentAppendsStopAtCap appends from many goroutines at once
 // with room for exactly roomFor reports: exactly that many must be
 // taken, which holds only if Append checks and counts each report

internal/server/routes_test.go

@@ -22,9 +22,9 @@ import (
 	"go.uber.org/fx/fxtest"
 )
 
-// newServer builds a Server from the same constructors as
+// newServer builds a Server from the same constructors as main,
-// cli.RunServer, configured from the environment. It is never started,
+// configured from the environment. It is never started, so nothing
-// so nothing listens.
+// listens.
 func newServer(t *testing.T) *server.Server {
 	t.Helper()