Files
noitu/plans/reports/review-260929-1939-whole-project-review-and-fixes.md

4.0 KiB

Whole-project review and fixes (dev, 2026-09-29)

Four parallel reviews (server core, wsapi, web, security/ops) followed by three parallel implementation passes with disjoint file ownership. Nothing is committed. Baseline before and after: Go vet, gofmt, golangci-lint and go test ./... -race clean; web check, lint and vitest clean (270 → 326 tests).

Reviews

Implementation

Highest-impact fixes:

  1. SIGTERM no longer kills every game before the drain: the restart notice and NOITU_DRAIN_TIMEOUT now work; a second signal exits immediately.
  2. A kicked player's token can no longer resume into whoever now holds that seat.
  3. One client can no longer hold the global connection or room caps: sockets that never send Hello close after 10s, and the room budget is charged per address.
  4. A player who reconnects into the lobby after their game ended while away is now replayed that game's GameOver, so the UI is no longer stuck on a frozen board; refused resumes now clear the stale room or board on the client.
  5. Hard bot prefers the slower loss in lost positions (41/3000 boards fixed).
  6. Non-breaking spaces in a typed word are now treated as spaces.
  7. Container HEALTHCHECK via noitu-server -healthcheck; LICENSE in the image; security headers; IdleTimeout; CI runs govulncheck and npm audit; buf action replaced; checkout without persisted credentials.

Decisions taken in this session (product questions the reviewers raised):

  • Resume-after-game-over replays GameOver rather than adding a proto field.
  • Reloading /play starts a fresh game on the same rung (the page's documented intent); a socket drop inside the same tab still resumes.
  • A kicked player who returns gets session_not_resumable; kicks do not revoke tokens.
  • Ready toggles count as lobby activity for the idle window.
  • Resigning out of turn follows the README: no immediate knock-out.
  • Added after the web pass: /play shows a "Chơi lại" button when a refused resume leaves the board idle under the error banner.

Manual steps outside the repo

Coolify (verified today: the app deploys main with no env vars and the dashboard health check off):

  • Set NOITU_TRUSTED_PROXIES to the Traefik/Docker network range, then NOITU_MAX_CONNECTIONS_PER_IP=32.
  • Set NOITU_DRAIN_TIMEOUT so that drain + 2s is under the container stop grace (6s or less under Docker's 10s default), or raise the grace.
  • After the next deploy, confirm the Dockerfile HEALTHCHECK is picked up.

GitHub:

  • The maintainer chose to drop dependabot.yml rather than move it to main; the CI govulncheck and npm audit steps are the dependency signal instead. Enable Dependabot alerts and secret scanning in repo settings if wanted.
  • Set default workflow permissions to read-only and disable "Allow GitHub Actions to create and approve pull requests".

Needs a human visual check (no browser on this host)

  • The .primary buttons in light and dark, all states, on the online page, landing page, ChatPanel, WordInput and GameOverPanel.
  • The /online heading at 1.5rem (was 1.3rem); the away banner and footer on a phone width; the new /play restart button.
  • Reconnect flows on /online (held actions after a drop, leave while offline, chat while offline) and the /play refused-resume path.
  • Playwright was not run.

Unresolved questions

  • Confirm the wording of the new unknown_message string in vi.js.
  • Whether the first proto.yml run with bufbuild/buf-action@v1 and no version input resolves to the newest buf.