Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .testcoverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ suites:
sdk: 50 # statements (vitest gates via --coverage.thresholds.statements)

exclude:
# Global excludes apply to every suite render AND to the merged total.
# Use for paths that aren't meaningful coverage anywhere.
paths:
# internal/testutil/ contains shared test helpers (mocks, JWT builders,
# HTTP assertions). It is imported only from *_test.go files, never from
Expand All @@ -41,3 +43,21 @@ exclude:
# aren't part of the shipped binary; they show up in `-coverpkg=./...`
# but never get exercised by tests.
- ^scripts/

# Per-suite excludes layer on top of `paths` but only for that suite's
# render. Crucially, they do NOT apply to the merged total — so a file
# excluded from `unit` here still counts toward the project-wide gate
# via whichever suite(s) actually do cover it. See scripts/cov/main.go
# config.excludesFor for the wiring.
per-suite:
unit:
# cmd/wavehouse/main.go is the binary entry point — 400+ lines of
# service wiring that's only meaningfully exercised by the e2e suite
# (which runs bin/wavehouse-cov against a real ClickHouse + NATS).
# Excluding from `unit` keeps that gate from being dragged down by
# 0% coverage on an untestable wiring file. The merged total still
# counts e2e's coverage of this file. health.go in the same package
# IS unit-testable (httptest server + WH_SERVER_PORT) and stays in
# coverage everywhere — see issue #140 for the deferred refactor
# that would shrink main.go enough to drop this exclude entirely.
- ^cmd/wavehouse/main\.go$
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Twelve internal packages under `internal/`:
14. **TypeScript SDK**: `@wavehouse/sdk` — zero-dependency client with typed query builder, real-time SSE, live queries with smart aggregation classification (incrementable/decomposable/poll), and codegen CLI.
15. **Observability invariants**: Stdout is *always* 100% — the slog logger fans out to stdout AND OTLP, and stdout sampling would silently hide records that scraping pipelines (Promtail/Alloy/Vector → Loki) are paying to store. Sampling knobs apply only to OTLP push. WARN+ERROR records always export at 100% regardless of `logs.sample_rate` — silently dropping errors during incidents would be a worse failure mode than the cost of forwarding them all (this is a *non-configurable* floor; do not expose it). gRPC OTel exporters dial lazily, so an unreachable collector never blocks startup; transient failures surface via the OTel SDK's error handler. The OTel Prometheus exporter (when enabled) uses a *private* `prometheus.Registry` to avoid leaking process/Go collectors that `prometheus.DefaultRegisterer` auto-registers into our `/metrics` output. When changing the logger, the sampler, or the provider wiring, preserve these invariants.
16. **Bearer-token-only CORS posture**: WaveHouse is a Bearer-token API — `Authorization: Bearer <jwt>` on every authenticated request, no cookies, no session middleware. The CORS middleware (`internal/api/router.go` `corsMiddleware`) deliberately **never** emits `Access-Control-Allow-Credentials`, because (a) we don't need it (Bearer tokens are explicit request headers, not browser-managed credentials) and (b) the historical pairing of `Allow-Credentials: true` with `Allow-Origin: *` is a CORS spec violation that browsers reject. The `cors_allowed_origins` allowlist controls *which origins can read responses*, not cookie scope. CSRF protection is structural: cross-site requests can't smuggle a Bearer token because the browser won't auto-attach `Authorization` headers cross-origin. Do not reintroduce cookie-based auth or `Access-Control-Allow-Credentials` without a separate design discussion — the current posture is the answer to GitHub issues #29 and #30.
17. **Non-fatal boot**: Schema-discovery failure on boot (ClickHouse unreachable, missing database, transient network blip) is non-fatal — `cmd/wavehouse` records an `api.BootState` diagnostic, binds `:8080`, and retries via `SchemaRegistry.RetryRefresh` (exp backoff 2s → 60s) in the background. `/health` and `/ready` return 503 with the latest diagnostic until a Refresh succeeds, after which `/health` stays 200 for the rest of the process. Keeps supervisor restart loops bounded and gives operators a queryable failure surface (`curl /health`) instead of a restart-log grep.

## Code Conventions

Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Changed
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

- **Boot-time schema discovery no longer crashes the binary** (`cmd/wavehouse/main.go`, `internal/api/health.go`, `internal/discovery/discovery.go`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`). Previously, any error from the initial `SchemaRegistry.Refresh` — connection-refused, missing database, transient network blip — caused `cmd/wavehouse` to call `os.Exit(1)`. The supervisor would restart the process every ~10s in an unbounded loop, port 8080 never bound, and operators got `connection refused` on probes even when ClickHouse was otherwise healthy. Now the first Refresh failure is non-fatal: a new `api.BootState` is set with the diagnostic, the server still binds `:8080`, and a background goroutine calls the new `SchemaRegistry.RetryRefresh(ctx, 2s, 60s, onAttempt)` with exponential backoff until success or shutdown. While `BootState.Err()` is non-nil, `/health` returns 503 with `{"status":"degraded","error":"…"}` so an operator can `curl /health` to learn why the gateway isn't ready to serve traffic instead of grepping a restart-loop log; `/ready` returns 503 with `{"status":"not ready",…}` for the same reason. Once `BootState.Set(nil)` fires (either from the initial sync `Refresh` or from `SchemaRegistry.RetryRefresh` success), `/health` stays at `200 OK` for the rest of the process lifetime — that flip is sticky and reflects "boot completed once." `/ready`, by contrast, remains conditional on current ClickHouse reachability: it can flip back to `503` on transient runtime blips after boot, since readiness is a probe of "can I serve traffic right now," not "did I ever boot." Resolves #95.

### Removed
- **`project-orchestrator.yml` workflow + its three composite-action artifacts** (`.github/workflows/project-orchestrator.yml`, `.github/actions/board-upsert-status/`, `.github/actions/set-linked-issues-status/`, `.github/scripts/board-fetch-item.sh`, `AGENTS.md`, `CHANGELOG.md`): −887 lines net. The orchestrator was the largest single source of cross-trigger complexity on this repo (3-4 workflow_run-chained runs per PR push, `statusCheckRollup` GraphQL perms quirks, integration-token `NONE` for private-org members) for behaviour that is mostly either provided natively by GitHub or a one-click manual operation on a 4-person team. Replaced by: reviewer-assign step in `housekeeping.yml` that fires once on `pull_request_target: opened` / `ready_for_review` (not per-synchronize, so it doesn't re-spam after `dismiss_stale_reviews_on_push`), plus GitHub's native Projects v2 workflows (`Auto-add to project`, `Item added`, `Pull request merged`) configured in the project UI. Trade-offs explicit in the PR body: drafts no longer auto-flip on bot-clean, `CHANGES_REQUESTED` doesn't auto-move the board card, linked-issue card mirroring is dropped. AGENTS.md §"Governance Files" + §"Task Board state machine" + §"Review tooling reference" all rewritten to match. `dependabot-automerge.yml` trimmed in parallel: no more board-upsert step (native handles placement), `PROJECT_BOARD_TOKEN` guard removed (no longer used in this workflow), reviewer list sourced from `board-config.env`'s `ADMINS` via `replace()`, major-bump comment uses the marker-comment upsert pattern from `housekeeping.yml`.
- **`STATUS_*` and old `ADMINS` consumers in `board-config.env`** — STATUS option IDs had only orchestrator-side consumers and are now unreferenced. `ADMINS` was restored to `board-config.env` after the initial orchestrator-removal commit dropped it (Gemini and Claude both flagged the resulting drift across three inlined copies); both `housekeeping.yml` and `dependabot-automerge.yml` now load `ADMINS` from `board-config.env`. `admin-approval.yml` keeps its own inline copy with the documented latency-avoidance reasoning.
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ test-unit: go-mod-download ## Run Go unit tests + render coverage + gate thresho
@printf "$(CYAN)==> Running Unit Tests...$(RESET)\n"
@rm -rf $(COV_UNIT)/data && mkdir -p $(COV_UNIT)/data
@GOCOVERDIR="$(CURDIR)/$(COV_UNIT)/data" go tool gotestsum --format $(GOTESTSUM_FMT) -- \
-tags="$(TAGS)" -cover -race ./internal/... $(ARGS) \
-tags="$(TAGS)" -cover -race ./internal/... ./cmd/... $(ARGS) \
-args -test.gocoverdir="$(CURDIR)/$(COV_UNIT)/data"
@go run ./scripts/cov render unit

Expand Down
88 changes: 88 additions & 0 deletions cmd/wavehouse/health_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package main

import (
"context"
"net"
"net/http"
"net/http/httptest"
"strconv"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// runHealthCheck is the Dockerfile HEALTHCHECK probe — the bridge between
// the in-process /health endpoint and the docker daemon's container-health
// state. These tests pin the contract that holds the boot-non-fatal promise
// together: when wavehouse is boot-degraded, /health returns 503 and the
// probe must return 1 so Docker marks the container unhealthy. Without this
// coverage, a regression that swallowed non-200 responses would silently
// re-create the "stuck container" failure mode the PR is solving.
//
// Tests don't run in parallel because they all mutate WH_SERVER_PORT.
// t.Setenv handles save/restore.

// portFromListener pulls the integer port off a net.Listener. We use the
// listener directly (rather than parsing httptest.Server.URL) so the same
// helper works for both started-server cases and the connection-refused
// case where we listen-then-close to claim a known-unused port.
func portFromListener(t *testing.T, ln net.Listener) string {
t.Helper()
addr, ok := ln.Addr().(*net.TCPAddr)
require.True(t, ok, "listener should be a TCP listener")
return strconv.Itoa(addr.Port)
}

func TestRunHealthCheck_Success(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// The probe targets /health specifically — pin that contract here
// so a refactor that points the probe at /ready (which requires
// ClickHouse) doesn't slip through silently.
assert.Equal(t, "/health", r.URL.Path)
w.WriteHeader(http.StatusOK)
}))
defer srv.Close()

t.Setenv("WH_SERVER_PORT", portFromListener(t, srv.Listener))
assert.Equal(t, 0, runHealthCheck(), "200 OK should map to exit 0")
}

func TestRunHealthCheck_BootDegraded503(t *testing.T) {
// This is the user-facing contract under boot-degraded state: the
// in-process /health returns 503 with a diagnostic body, the probe
// sees the non-200, returns 1, and Docker's HEALTHCHECK marks the
// container unhealthy. The K8s startupProbe path in deployment.md
// relies on exactly this signal.
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusServiceUnavailable)
_, _ = w.Write([]byte(`{"status":"degraded","error":"schema discovery: connection refused"}`))
}))
defer srv.Close()

t.Setenv("WH_SERVER_PORT", portFromListener(t, srv.Listener))
assert.Equal(t, 1, runHealthCheck(), "503 should map to exit 1 — Docker marks unhealthy")
}

func TestRunHealthCheck_ConnectionRefused(t *testing.T) {
// Wavehouse hasn't bound :8080 yet (e.g., still in the pre-listen
// phase of `run()`, or crashed entirely). Bind+close a TCP listener
// to claim a port that's now guaranteed unused, so the probe's GET
// gets a real connect-refused rather than a slow timeout.
var lc net.ListenConfig
ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:0")
require.NoError(t, err)
port := portFromListener(t, ln)
require.NoError(t, ln.Close())

t.Setenv("WH_SERVER_PORT", port)
assert.Equal(t, 1, runHealthCheck(), "connection refused should map to exit 1")
}

func TestRunHealthCheck_InvalidPortEnv(t *testing.T) {
// Garbage env var should fail fast at the parse step, not propagate
// a confusing dial error. Catches the case where an operator
// fat-fingers WH_SERVER_PORT in a deployment manifest.
t.Setenv("WH_SERVER_PORT", "not-a-number")
assert.Equal(t, 1, runHealthCheck(), "non-integer WH_SERVER_PORT should exit 1")
}
49 changes: 38 additions & 11 deletions cmd/wavehouse/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,42 @@ func run() int {
}
defer func() { _ = chConn.Close() }()

// Schema discovery — discover ClickHouse table schemas on boot.
// Process-lifetime context — cancelled by the SIGINT/SIGTERM handler
// below. Created here (instead of further down) so the boot-time schema
// discovery retry goroutine ties cleanly to shutdown.
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

// Schema discovery — non-fatal on boot. If the first Refresh fails
// (ClickHouse unreachable, database missing, etc.) we mark the binary
// degraded via bootState (which /health surfaces as 503 + diagnostic)
// and retry in the background with exponential backoff. The process
// still binds :8080 so operators can `curl /health` instead of grepping
// a restart-loop log. Once a Refresh succeeds, bootState flips to nil
// and /health returns 200. The periodic auto-refresh ticker is started
// only after the first successful Refresh (sync or retry) so it never
// races RetryRefresh on Refresh calls or on bootState writes.
bootState := api.NewBootState(nil)
refreshInterval := time.Duration(cfg.Schema.RefreshInterval) * time.Second
registry := discovery.NewSchemaRegistry(chConn, cfg.ClickHouse.Database, refreshInterval, logger)
if err := registry.Refresh(context.Background()); err != nil {
logger.Error("schema discovery failed on boot", "error", err)
return 1
if err := registry.Refresh(ctx); err != nil {
Comment thread
EricAndrechek marked this conversation as resolved.
Resolved
logger.Warn("schema discovery failed on boot, retrying in background", "error", err)
bootState.Set(fmt.Errorf("schema discovery: %w", err))
go func() {
retryErr := registry.RetryRefresh(ctx, 2*time.Second, 60*time.Second, func(attemptErr error) {
Comment thread
EricAndrechek marked this conversation as resolved.
Resolved
logger.Warn("schema discovery retry failed", "error", attemptErr)
bootState.Set(fmt.Errorf("schema discovery: %w", attemptErr))
})
if retryErr != nil {
// ctx cancelled before success — process is shutting down.
return
}
logger.Info("schema discovery succeeded after retry, /health now 200")
bootState.Set(nil)
go registry.StartAutoRefresh(ctx)
}()
Comment thread
EricAndrechek marked this conversation as resolved.
Resolved
} else {
go registry.StartAutoRefresh(ctx)
}

// State directories — one configurable root, fixed subdir convention.
Expand Down Expand Up @@ -243,12 +273,6 @@ func run() int {
// Hub for streaming fan-out.
hub := api.NewHub()

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

// Start schema auto-refresh.
go registry.StartAutoRefresh(ctx)

// Start policy watch for cluster-wide updates.
go policyStore.Watch(ctx)

Expand Down Expand Up @@ -311,6 +335,9 @@ func run() int {
queryHandler := api.NewQueryHandler(chConn, tiered, time.Duration(cfg.Cache.DefaultTTL)*time.Second)
queryHandler.PolicyStore = policyStore

healthHandler := api.NewHealthHandler(chConn)
healthHandler.Boot = bootState

sseHandler := api.NewSSEHandler(hub, js)
sseHandler.PolicyStore = policyStore

Expand All @@ -322,7 +349,7 @@ func run() int {
Query: queryHandler,
SSE: sseHandler,
WS: wsHandler,
Health: api.NewHealthHandler(chConn),
Health: healthHandler,
Schema: api.NewSchemaHandler(registry),
DLQ: dlqHandler,
Policy: api.NewPolicyHandler(policyStore),
Expand Down
16 changes: 13 additions & 3 deletions docs/src/content/docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,29 @@ The per-endpoint error tables below list the bodies you can expect for each stat

### `GET /health` — Liveness Probe

Returns `200 OK` if the process is running. No authentication required.
Returns `200 OK` once the gateway has discovered ClickHouse table schemas at least once. Returns `503 Service Unavailable` with a diagnostic body while the boot-time schema discovery retry loop is still running (ClickHouse unreachable, target database missing, etc.). No authentication required.

**Response:**
**Response (ready):**

```json
{"status": "ok"}
```

**Response (boot-degraded):**

```json
{"status": "degraded", "error": "schema discovery: dial tcp 127.0.0.1:9000: connect: connection refused"}
```

Status code: `503 Service Unavailable`

The boot-degraded response lets an operator `curl /health` to learn why the gateway isn't ready to serve traffic yet, instead of grepping a restart-loop log. The binary is bound on `:8080` and serves diagnostics, but is not yet accepting ingest/query traffic. Schema discovery retries with exponential backoff (2s → 60s); once a Refresh succeeds, `/health` flips to `200` and stays there for the rest of the process lifetime — transient ClickHouse blips after that point are reflected in `/ready`, not `/health`.

---

### `GET /ready` — Readiness Probe

Returns `200 OK` if the process is running and ClickHouse is reachable. No authentication required.
Returns `200 OK` if the process is fully booted (schema discovery complete) and ClickHouse is currently reachable. Returns `503 Service Unavailable` otherwise. No authentication required.

**Response (ready):**

Expand Down
4 changes: 2 additions & 2 deletions docs/src/content/docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with standar
- **schema.go** — Schema discovery API: list all schemas, get one table, trigger refresh.
- **dlq.go** — DLQ stats endpoint and `EnsureDLQStream` helper for creating the `WAVEHOUSE_DLQ` NATS stream.
- **hub.go** — In-process pub/sub for broadcasting MQ messages to connected streaming clients.
- **health.go** — Liveness (`/health`) and readiness (`/ready`) probes.
- **health.go** — Liveness (`/health`) and readiness (`/ready`) probes. Both consult an optional `BootState` so they can return 503 with a diagnostic while boot-time schema discovery is still failing in the retry loop (see `cmd/wavehouse/main.go`); once `BootState.Set(nil)` fires, `/health` returns 200 and stays there.

### `cache/` — Query Cache

Expand All @@ -99,7 +99,7 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with standar

### `discovery/` — Schema Discovery & Validation

- **discovery.go** — `SchemaRegistry` queries `system.columns` to discover ClickHouse table schemas. Supports periodic auto-refresh and on-demand refresh. Thread-safe via `sync.RWMutex`.
- **discovery.go** — `SchemaRegistry` queries `system.columns` to discover ClickHouse table schemas. Supports periodic auto-refresh, on-demand refresh, and `RetryRefresh` (boot-time exponential backoff loop used by `cmd/wavehouse` so a transiently unreachable ClickHouse doesn't crash-loop the binary). Thread-safe via `sync.RWMutex`.
- **validation.go** — `Validate(schema, data)` checks incoming JSON against the discovered schema: unknown fields, type compatibility, missing required columns, null handling.
- **discovery_test.go** — Unit tests for validation logic.

Expand Down
Loading
Loading