Add health endpoints and graceful shutdown

[Aloento]

Changes

• Add top-level GET /healthz (liveness, no DB access) and GET /readyz (readiness, DB ping with a 3s request-scoped timeout, 503 when the database is unreachable). Both are registered in InitRoutes without auth/RBAC middleware, ahead of the static NoRoute fallback.
• db.DB gains Ping(ctx) wrapping sql.PingContext.
• cmd/main.go: shutdown is driven by a fresh 15s timeout context instead of the already-cancelled signal context (which made Shutdown return context.Canceled and exit with code 1, skipping checker.Shutdown). Shutdown failures are logged as errors instead of fatal.
• App.Shutdown closes the database pool after the HTTP server drains.

Verification

• go build ./..., go vet ./... — clean
• go test ./internal/... — all pass; new internal/api/health_test.go covers liveness without a DB (200), readiness with an unreachable DB (503), and top-level GET registration
• go vet ./tests/... — integration suite compiles; it cannot run locally (no Docker for testcontainers)
• golangci-lint not usable locally (installed binary predates the Go 1.27 export format); relying on CI

[ecosquad-autoreview]

Summary

Adds /healthz (liveness, no DB) and /readyz (DB ping with a 3s request-scoped timeout, 503 on failure) without auth middleware, plus a graceful-shutdown fix: the signal context (already cancelled) is replaced with a fresh 15s timeout, shutdown failures are logged instead of fatal, and App.Shutdown now closes the DB pool after the HTTP servers drain. The change is correct and well tested.

Findings

Warning — checker can race with the closed DB pool (cmd/main.go:60, internal/app/app.go:163-166)

main calls s.Shutdown(shutdownCtx) — which now closes the DB pool — and only then calls ch.Shutdown(). The checker's Run loop (internal/checker/checker.go) issues DB queries via WithAdvisoryLock(context.Background(), ...) on a 2-minute ticker with no knowledge of the shutdown. If a scan tick fires in the window between App.Shutdown closing the pool and ch.Shutdown cancelling the loop, the scan hits a closed *sql.DB. The failure is only logged and the process exits right after, so impact is negligible — but the ordering is backwards. Fix: call ch.Shutdown() (or at least cancel the checker) before s.Shutdown, so no checker work is in flight when the pool is closed.

Suggestion — single 15s budget shared by both servers (internal/app/app.go:154-166)

metricsSrv.Shutdown(ctx) and srv.Shutdown(ctx) share the same 15s deadline. If draining the metrics listener consumes most of the budget, the main server gets less than 15s. Unlikely to matter in practice (metrics has few in-flight requests), but consider a separate timeout or draining both concurrently.

Suggestion — readiness ping connection latency

db.Ping opens a real connection from the pool per probe. With dbMaxIdleConns = 10 and connection setup overhead, a probe right after idle can approach the 3s bound under load. Fine as-is; just be aware that a slow-but-healthy database could flap /readyz.

Notes

• Health routes are registered on the engine directly, so they only get the global middleware (logging, recovery, security headers, CORS) and no auth — matches the stated intent. Gin's NoRoute fallback is independent of registration order, so the ordering in InitRoutes is fine.
• health_test.go covers liveness without a DB (200), readiness against an unreachable DB (503), and top-level GET registration. Good.
• CI (go-test, build) was still queued at review time, so verification here is by reading only; the PR author reports go build, go vet, and unit tests passing locally.

Verdict: approve — the changes are correct; the checker/DB-close ordering is worth a follow-up but does not block merging.

[ecosquad-autoreview]

Summary

Adds GET /healthz / GET /readyz (registered ahead of the NoRoute fallback, without auth) and fixes graceful shutdown: checker stopped before the pool is closed, HTTP drain driven by a fresh 15s context instead of the cancelled signal context, shutdown errors logged rather than fatal, and App.Shutdown now closes the DB pool.

The changes are correct and the ordering is right. No critical issues found.

Findings

Warning

1. internal/app/app.go:154-166 — no wait for the notification worker between workerCancel() and DB.Close()
workerCancel() only signals the worker's context; the loop (started in App.Run) may still be mid-iteration, e.g. holding a query on the pool when a.DB.Close() runs. The worker will log an error from that in-flight query but won't corrupt anything (writes happen before the close, and there's no final DB access after termination), so it's not data loss — but the shutdown is not actually deterministic. If you want a clean stop, track the worker with a done channel (like Checker) and wait for it in Shutdown with a deadline before closing the pool.

Suggestions

2. internal/api/health.go — responses have empty bodies. Fine for probes, but curl -v on /healthz is unreadable; consider c.Status(200); c.String(200, "ok") or returning a tiny JSON body. Optional.

3. internal/api/health_test.go:19 — db.New(...) with an unreachable address relies on the driver opening connections lazily (which pgx/lib/pq do by default, so the 200 test passes). It's coupled to that behavior; a comment would make the intent explicit. Also, the readiness test's 503 comes from the 3s dial timeout on 127.0.0.1:1, so that test takes up to ~3s — acceptable, but worth knowing.

Verified OK

• cmd/main.go:49-59 — correct order: ch.Shutdown() (waits for the in-flight scan to finish) → HTTP drain with fresh 15s ctx → App.Shutdown closes the pool. Logging shutdown failure as error (exit 0) is the right call for k8s, since SIGTERM must not hang.
• internal/api/routes.go:19 / internal/api/api.go — initHealthRoutes runs on the root engine before initV2Routes, so /healthz and /readyz are exact top-level GET routes that shadow the NoRoute(catchAll) static proxy; the registration test in health_test.go covers this.
• internal/db/db.go:62 — Ping correctly wraps sql.PingContext; with the 3s request-scoped timeout a hung DB won't stall the probe.
• internal/checker/checker.go — Shutdown is safe to call before/without Run, and Check() uses context.Background() (not the Run context), so the scan it drains is truly finished by the time Shutdown returns — the main.go comment is accurate.
• CI is still in progress at review time; no check failures so far.

Verdict

approve — no correctness or security problems; the worker-stop gap (#1) is a robustness improvement, not a merge blocker.

[ecosquad-autoreview]

Summary

Adds GET /healthz (liveness) and GET /readyz (readiness with a 3s-bounded DB ping), a db.DB.Ping wrapper, and fixes the shutdown path in cmd/main.go (fresh 15s timeout context instead of the already-cancelled signal context) plus closes the DB pool from App.Shutdown. The changes are well-scoped, well-commented, and the shutdown ordering (checker → server drain → pool close) is correct: Checker.Shutdown waits for Run's select on ctx.Done(), which only returns after Check()'s runScan has finished its WaitGroup.Wait(), so no scan can be mid-DB-call when App.Shutdown closes the pool. Tests cover liveness without a DB, 503 on unreachable DB, and unauthenticated route registration.

Findings

Warning

1. Shutdown failure is silently swallowed (exit code 0). cmd/main.go:63-65

   if err = s.Shutdown(shutdownCtx); err != nil {
       logger.Error("app shutdown failed", zap.Error(err))
   }
   logger.Info("app exited")

   Changing logger.Fatal to logger.Error means the process now exits 0 even when the drain times out or DB.Close() fails. On Kubernetes this hides failure from the orchestrator (no restart triggered, and the pod's graceful-termination budget may still be consumed). If a non-zero exit on failed shutdown is desired, os.Exit(1) (or logger.Fatal) after the error branch is the minimal fix; if swallowing is intentional, add a comment saying so, because it is not obvious that "app exited" can follow a failed shutdown.

2. Readiness probe can be blocked on pool exhaustion, not just DB liveness. internal/api/health.go:24-33
sql.DB.PingContext blocks until it can obtain a connection from the pool. With dbMaxOpenConns = 25 (db.go:24) and a full pool, a probe can sit behind a waiting PingContext even though the database is healthy. The defer cancel() bounds it at 3s, but the symptom — the probe reporting 503 for a healthy DB while the app is just slow under load — is a false negative that can pull a healthy pod out of rotation. Common mitigation: give /readyz its own *sql.DB with MaxOpenConns(1) and Wait(ctx) semantics, or document/accept the behaviour. At minimum, be aware of it when sizing probes.

Suggestion

3. err reuse across s.Run() and s.Shutdown() in cmd/main.go. err from conf.LoadConf/app.New is reused for s.Shutdown(shutdownCtx). Functionally fine (the goroutine already captures the Run error into its own err closure), but a fresh if shutdownErr := s.Shutdown(shutdownCtx); shutdownErr != nil { ... } avoids confusion.

4. /readyz only pings the pool, not the Ent schema. db.DB.Ping wraps sql.PingContext, which verifies the Postgres connection but not that the ent schema/queries are healthy. Fine as a "is the database reachable" signal, but worth a comment on readinessHandler so future readers don't assume it validates schema.

5. CI is still pending (Analyze (go) queued); the PR description notes the integration suite can't run locally. Nothing to do before merge beyond letting CI finish.

Non-issues (checked)

• internal/api/routes.go:19 registers health routes on the root engine, before initV2Routes/initRSSRoutes and before NoRoute is resolved at request time — Gin's route table is consulted before NoRoute, so /healthz and /readyz are not shadowed by the static-site catch-all.
• internal/app/app.go:162-166 closes the DB only after srv.Shutdown returns, and after ch.Shutdown() in main — no in-flight query can hit a closed pool.
• internal/db/db.go:62-64 Ping is a thin wrapper; no new concurrency or state.
• health_test.go uses an unreachable 127.0.0.1:1 and asserts 503 — correct for the readiness contract.

Verdict

Approve — no correctness bugs found; the two warnings are about shutdown-visibility and pool-exhaustion semantics, not defects in the happy path.