Make prod green: the check fails on the commit main, next and prod share #104

Open
opened 2026-09-29 12:33:16 +02:00 by clawbot · 1 comment
Collaborator

sneak, 2026-09-29 in chat (verbatim):

please make neoirc prod branch green

prod was created today from main (f829f9e3, 1 April) for upaas on fsn1app1, so main, next and prod are the same commit, and its check fails (the Gitea run on the push to prod failed after 1m39s). #101 (make test retry masks real data races and timeouts) may be the cause or part of it.

Definition of done:

  • The cause of the failure is found by cloning the commit and running the repo's own check entrypoint the way CI runs it, and is stated here in a few lines.
  • It is fixed properly on next (one PR, independently reviewed, squash-merged): no test is skipped, deleted or retried into passing, and no check is weakened.
  • next is green, then goes to main in a next-to-main PR that sneak merges; then a main-to-prod PR (it lands as a squash commit) makes prod green, also merged by sneak.
  • If the failure is flaky rather than deterministic, the flake is the defect: find and fix its cause.

Model: opus-5-5

sneak, 2026-09-29 in chat (verbatim): > please make neoirc prod branch green `prod` was created today from `main` (`f829f9e3`, 1 April) for upaas on fsn1app1, so `main`, `next` and `prod` are the same commit, and its check fails (the Gitea run on the push to `prod` failed after 1m39s). https://git.eeqj.de/sneak/neoirc/issues/101 (`make test` retry masks real data races and timeouts) may be the cause or part of it. Definition of done: - The cause of the failure is found by cloning the commit and running the repo's own check entrypoint the way CI runs it, and is stated here in a few lines. - It is fixed properly on `next` (one PR, independently reviewed, squash-merged): no test is skipped, deleted or retried into passing, and no check is weakened. - `next` is green, then goes to `main` in a `next`-to-`main` PR that sneak merges; then a `main`-to-`prod` PR (it lands as a squash commit) makes `prod` green, also merged by sneak. - If the failure is flaky rather than deterministic, the flake is the defect: find and fix its cause. Model: opus-5-5
clawbot self-assigned this 2026-09-29 12:33:16 +02:00
Author
Collaborator

Cause, from running what CI runs (docker build --no-cache .) on f829f9e3: lint and fmt-check pass. The build stops at RUN make test because internal/handlers fails on both passes of make test. So the retry from #101 is not what makes it red, and that issue stays separate.

Why: server.New adds an OnStart hook that runs go srv.Run(). The router is only built later, inside that goroutine (serveUntilShutdown calls SetupRoutes, internal/server/server.go:205). OnStart therefore returns before srv.router exists. The handler tests call app.RequireStart() and then serve srv through httptest.NewServer straight away (internal/handlers/api_test.go:123-125), so their first requests hit one of two problems:

  • they race with SetupRoutes (a data race on srv.router and on chi's route tree, which fails the test under -race, e.g. TestLongPollTimeout, TestTopicLockOpCanChange), or
  • they find srv.router still nil (a nil-pointer panic in chi.(*Mux).ServeHTTP, which the test sees as EOF, e.g. TestJoinChannel, TestChannelList).

The running daemon is not affected, because there SetupRoutes runs before ListenAndServe in the same goroutine. The defect is that a started Server has no routes yet.

Plan, one PR on next:

  • Build the router before OnStart returns, so a started Server can serve. SetupRoutes reads srv.sentryEnabled, so enableSentry has to run before it: keep the order configure, enableSentry, SetupRoutes, then start serving in the goroutine. Remove the later SetupRoutes call from serveUntilShutdown.
  • Leave the tests alone: no sleeps, retries or skips. -race stays, and so do the 30s timeout and the || retry (#101).
  • Done when docker build --no-cache . passes on the branch three times in a row, with no DATA RACE and no second (-v) pass of make test in its output.
  • If those runs fail for some other reason (the internal/handlers 30s timeout, the ircserver race from #101), report it on the PR and stop. That gets decided separately.

Model: opus-5-5

Cause, from running what CI runs (`docker build --no-cache .`) on `f829f9e3`: lint and `fmt-check` pass. The build stops at `RUN make test` because `internal/handlers` fails on both passes of `make test`. So the retry from https://git.eeqj.de/sneak/neoirc/issues/101 is not what makes it red, and that issue stays separate. Why: `server.New` adds an `OnStart` hook that runs `go srv.Run()`. The router is only built later, inside that goroutine (`serveUntilShutdown` calls `SetupRoutes`, `internal/server/server.go:205`). `OnStart` therefore returns before `srv.router` exists. The handler tests call `app.RequireStart()` and then serve `srv` through `httptest.NewServer` straight away (`internal/handlers/api_test.go:123-125`), so their first requests hit one of two problems: - they race with `SetupRoutes` (a data race on `srv.router` and on chi's route tree, which fails the test under `-race`, e.g. `TestLongPollTimeout`, `TestTopicLockOpCanChange`), or - they find `srv.router` still nil (a nil-pointer panic in `chi.(*Mux).ServeHTTP`, which the test sees as `EOF`, e.g. `TestJoinChannel`, `TestChannelList`). The running daemon is not affected, because there `SetupRoutes` runs before `ListenAndServe` in the same goroutine. The defect is that a started `Server` has no routes yet. Plan, one PR on `next`: - Build the router before `OnStart` returns, so a started `Server` can serve. `SetupRoutes` reads `srv.sentryEnabled`, so `enableSentry` has to run before it: keep the order configure, enableSentry, `SetupRoutes`, then start serving in the goroutine. Remove the later `SetupRoutes` call from `serveUntilShutdown`. - Leave the tests alone: no sleeps, retries or skips. `-race` stays, and so do the 30s timeout and the `||` retry (https://git.eeqj.de/sneak/neoirc/issues/101). - Done when `docker build --no-cache .` passes on the branch three times in a row, with no `DATA RACE` and no second (`-v`) pass of `make test` in its output. - If those runs fail for some other reason (the `internal/handlers` 30s timeout, the `ircserver` race from https://git.eeqj.de/sneak/neoirc/issues/101), report it on the PR and stop. That gets decided separately. Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/neoirc#104