From 915f56ee02d88f34f520b9f0b6f86644d9c0ddee Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 02:49:53 +0200 Subject: [PATCH 1/2] Fix the data races and the test timeout that fail make test (closes #104) The HTTP server built its router inside the goroutine that starts serving, so the start hook returned before the router existed, and the internal/handlers tests raced with it or hit a nil router. The start hook now runs configure, enableSentry and SetupRoutes, in that order, then serves in the background. The goroutine that sends queued messages to an IRC client read c.nick without c.mu while NICK changed it. Every such read now takes the lock. Under -race in the Docker build, internal/handlers takes over 30s on database work, not clock waits, so both go test runs in make test use -timeout 120s. The || retry stays (https://git.eeqj.de/sneak/neoirc/issues/101). Model: opus-5-5 --- Makefile | 2 +- internal/ircserver/relay.go | 48 ++++++++++++++++++++++++++++++------- internal/server/server.go | 18 +++++++------- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index 67ceb3e..f00e4b7 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ fmt-check: @test -z "$$(gofmt -l .)" || (echo "Files not formatted:" && gofmt -l . && exit 1) test: ensure-web-dist - go test -timeout 30s -race -cover ./... || go test -timeout 30s -race -v ./... + go test -timeout 120s -race -cover ./... || go test -timeout 120s -race -v ./... # check runs all validation without making changes # Used by CI and Docker build — fails if anything is wrong diff --git a/internal/ircserver/relay.go b/internal/ircserver/relay.go index cb7d185..6d4195f 100644 --- a/internal/ircserver/relay.go +++ b/internal/ircserver/relay.go @@ -128,7 +128,11 @@ func (c *Conn) deliverIRCMessage( default: // Unknown command — deliver as server notice. if text != "" { - c.sendFromServer("NOTICE", c.nick, text) + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + + c.sendFromServer("NOTICE", nick, text) } } } @@ -158,8 +162,12 @@ func (c *Conn) deliverNumeric( _ = json.Unmarshal(msg.Params, ¶ms) } + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + allParams := make([]string, 0, 1+len(params)+1) - allParams = append(allParams, c.nick) + allParams = append(allParams, nick) allParams = append(allParams, params...) if text != "" { @@ -177,8 +185,12 @@ func (c *Conn) deliverTextMessage( from := msg.From target := msg.To + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + // Don't echo our own messages back. - if strings.EqualFold(from, c.nick) { + if strings.EqualFold(from, nick) { return } @@ -192,9 +204,13 @@ func (c *Conn) deliverTextMessage( // deliverJoin sends a JOIN notification. func (c *Conn) deliverJoin(msg *db.IRCMessage) { + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + // Don't echo our own JOINs (we already sent them // during joinChannel). - if strings.EqualFold(msg.From, c.nick) { + if strings.EqualFold(msg.From, nick) { return } @@ -206,7 +222,11 @@ func (c *Conn) deliverJoin(msg *db.IRCMessage) { // deliverPart sends a PART notification. func (c *Conn) deliverPart(msg *db.IRCMessage, text string) { - if strings.EqualFold(msg.From, c.nick) { + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + + if strings.EqualFold(msg.From, nick) { return } @@ -227,7 +247,11 @@ func (c *Conn) deliverNickChange( msg *db.IRCMessage, newNick string, ) { - if strings.EqualFold(msg.From, c.nick) { + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + + if strings.EqualFold(msg.From, nick) { return } @@ -241,7 +265,11 @@ func (c *Conn) deliverQuitMsg( msg *db.IRCMessage, text string, ) { - if strings.EqualFold(msg.From, c.nick) { + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + + if strings.EqualFold(msg.From, nick) { return } @@ -302,7 +330,11 @@ func (c *Conn) deliverInviteMsg( _ *db.IRCMessage, text string, ) { - c.sendFromServer("NOTICE", c.nick, text) + c.mu.Lock() + nick := c.nick + c.mu.Unlock() + + c.sendFromServer("NOTICE", nick, text) } // deliverMode sends a MODE change notification. diff --git a/internal/server/server.go b/internal/server/server.go index 50216fa..1006342 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -71,7 +71,14 @@ func New( lifecycle.Append(fx.Hook{ OnStart: func(_ context.Context) error { srv.startupTime = time.Now() - go srv.Run() //nolint:contextcheck + + // Build the router before OnStart returns, so srv can + // handle requests as soon as the app has started. + srv.configure() + srv.enableSentry() + srv.SetupRoutes() + + go srv.serve() //nolint:contextcheck return nil }, @@ -83,13 +90,6 @@ func New( return srv, nil } -// Run starts the server configuration, Sentry, and begins serving. -func (srv *Server) Run() { - srv.configure() - srv.enableSentry() - srv.serve() -} - // ServeHTTP delegates to the chi router. func (srv *Server) ServeHTTP( writer http.ResponseWriter, @@ -202,8 +202,6 @@ func (srv *Server) serveUntilShutdown() { Handler: srv, } - srv.SetupRoutes() - srv.log.Info( "http begin listen", "listenaddr", listenAddr, ) -- 2.54.0 From 729c671561ea8f1fd5ef9f8aaa280c3003e27388 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 04:13:07 +0200 Subject: [PATCH 2/2] Remove BUILDARCH and stamp the git version in Docker builds (closes #107, closes #109) The Makefile no longer passes a build architecture; it set a variable that does not exist, and the architecture is available at run time. A plain docker build now stamps neoircd with the tag or short commit: .dockerignore sends .git, without its config, and the builder takes the version from the VERSION build arg when given, otherwise from git describe. With .git present, an empty, dev or unknown version fails the build. Model: opus-5-5 --- .dockerignore | 3 ++- Dockerfile | 15 +++++++++++++-- Makefile | 3 +-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/.dockerignore b/.dockerignore index 9b75277..3850038 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,4 +1,5 @@ -.git +# .git is sent so the build can stamp the version, without its config. +.git/config *.md !README.md neoircd diff --git a/Dockerfile b/Dockerfile index 32acda7..b90d0ca 100644 --- a/Dockerfile +++ b/Dockerfile @@ -39,8 +39,19 @@ COPY --from=web-builder /web/dist/ web/dist/ RUN make test # Build static binaries (no cgo needed at runtime — modernc.org/sqlite is pure Go) -ARG VERSION=dev -RUN CGO_ENABLED=0 go build -trimpath -ldflags="-s -w -X main.Version=${VERSION}" -o /neoircd ./cmd/neoircd/ +# +# neoircd is stamped with the VERSION build arg when one is given, otherwise +# with the tag or short commit from the .git in the build context. With .git +# present, a version that is still empty, dev or unknown fails the build. +ARG VERSION +RUN VERSION="${VERSION:-$(git describe --tags --always)}"; \ + if [ -e .git ]; then \ + case "$VERSION" in ""|dev|unknown) \ + echo "version is '$VERSION' although .git is present" >&2; \ + exit 1 ;; \ + esac; \ + fi; \ + CGO_ENABLED=0 go build -trimpath -ldflags="-s -w -X main.Version=${VERSION}" -o /neoircd ./cmd/neoircd/ RUN CGO_ENABLED=0 go build -trimpath -ldflags="-s -w" -o /neoirc-cli ./cmd/neoirc-cli/ # Runtime stage diff --git a/Makefile b/Makefile index f00e4b7..6758284 100644 --- a/Makefile +++ b/Makefile @@ -2,8 +2,7 @@ BINARY := neoircd VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev") -BUILDARCH := $(shell go env GOARCH) -LDFLAGS := -X main.Version=$(VERSION) -X main.Buildarch=$(BUILDARCH) +LDFLAGS := -X main.Version=$(VERSION) all: check build -- 2.54.0