fix(boot): non-fatal schema discovery, /health 503 with diagnostic - #125
Conversation
Previously, any error from the initial SchemaRegistry.Refresh in
cmd/wavehouse — connection-refused, missing database, transient blip —
called 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 though ClickHouse was otherwise
healthy. The binary was unrecoverable without intervention.
Now the first Refresh failure is non-fatal: a new api.BootState records
the diagnostic, the server still binds :8080, and a background
goroutine calls SchemaRegistry.RetryRefresh (exp backoff 2s → 60s)
until success or shutdown. While BootState.Err() != nil, /health
returns 503 with {"status":"degraded","error":"..."} so operators can
curl the endpoint instead of grepping a restart-loop log; /ready does
the same. Once a Refresh succeeds, BootState.Set(nil) flips both
endpoints back to 200 and stays there — transient runtime CH blips
after that point are reflected in /ready only.
Closes #95.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the boot-time reliability of the application by making initial schema discovery non-fatal. Instead of exiting when ClickHouse is unreachable, the service now enters a degraded state and retries the connection in the background. This ensures the process remains running and provides clear diagnostic information via health endpoints, eliminating the need for operators to rely on restart-loop logs to debug startup connectivity issues. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
documentation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
0 [MUST] · 1 [SHOULD] · 0 [MAY] — see inline thread for detail. The implementation is clean: The single [SHOULD] is a test-only style nit: Ship it — address or push back on the inline thread before merge. |
Per Claude review on #125: TestRetryRefresh_ReturnsOnContextCancel used a 50ms sleep before cancel() that violated AGENTS.md ("use channel sync not goroutine scheduling assumptions") and was unnecessary for correctness — `done` is the only sync we need, and the RetryRefresh loop observes ctx.Done() whether cancel fires before or after the goroutine enters its select. Verified with `go test -race -count=10`. Also corrects a stale doc comment on HealthHandler.Boot that said "consulted by Liveness" — Readiness consults it too, by design (a kubelet readiness probe should see "not ready" while boot is failing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request makes boot-time schema discovery non-fatal, preventing the binary from crash-looping if ClickHouse is unreachable at startup. It introduces a BootState to track startup diagnostics, an exponential backoff retry loop in the SchemaRegistry, and updates the /health and /ready endpoints to surface a 503 'degraded' status during the boot process. Feedback focuses on documenting these architectural changes in AGENTS.md per the style guide, sanitizing error messages in health probes to prevent sensitive data exposure, and optimizing the retry loop by using time.NewTimer to avoid potential memory leaks.
Per Gemini's review on #125. Boot-time degraded mode + retry loop is a meaningful design decision worth documenting alongside the other 'Async ingestion', 'DLQ', 'Active Sweeper' style entries — future agents working on cmd/wavehouse will want to know /health is allowed to 503 by design during initial Refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
1 [MUST] · 1 [SHOULD] · 0 [MAY] — see inline threads for detail. The core implementation is solid: Inline threads:
Iterate — the [MUST] must be addressed before merge. The most important thing: the deployment doc claims "The supervisor (docker, systemd, k8s) won't restart the binary every ~10s" but this is false for Kubernetes when |
Two Claude-review fixes from #125: 1) [MUST] docs/deployment.md previously claimed the binary "won't restart" — false for Kubernetes when /health is wired to livenessProbe (the example we ship), since kubelet still kills the pod after failureThreshold × periodSeconds. The retry-loop window can exceed that, recreating the same restart problem the PR is solving. Fixed by (a) softening the prose to be accurate about the binary-vs-orchestrator distinction and (b) adding a startupProbe to the K8s example so liveness and readiness are gated until the first schema discovery succeeds. Docker HEALTHCHECK is unaffected (doesn't restart by default). 2) [SHOULD] TestRetryRefresh_ClampsInvalidBackoffs slept ~1s in the unit suite to observe the clamp-to-default behaviour. Extracted the clamp into a private clampBackoff helper so the invariant is now testable in microseconds via TestClampBackoff (6 table-driven cases). Old slow test removed; discovery coverage rose 89.6% → 89.8%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
0 [MUST] · 0 [SHOULD] · 0 [MAY] — no open findings on HEAD `2144b3a`. Both items from the previous review have been resolved:
Ship it — address or push back on any open inline threads before merging (ruleset requires resolution), then this is good to go. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSchema discovery boot failures are now non-fatal. The process binds :8080, tracks degradation in BootState, returns 503 from /health and /ready with diagnostics while RetryRefresh runs with exponential backoff, and clears BootState on success so probes return 200 thereafter. ChangesNon-fatal Boot with Diagnostic Health Probes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HealthHandler
participant BootState
participant SchemaRegistry
participant ClickHouse
Client->>HealthHandler: GET /health or GET /ready
HealthHandler->>BootState: Err()
alt BootState.Err() != nil
BootState-->>HealthHandler: diagnostic error
HealthHandler-->>Client: 503 with diagnostic JSON
else
BootState-->>HealthHandler: nil
HealthHandler->>SchemaRegistry: (for readiness) ping / check
SchemaRegistry->>ClickHouse: Query schema / ping
ClickHouse-->>SchemaRegistry: response
SchemaRegistry-->>HealthHandler: readiness result
HealthHandler-->>Client: 200 or 503 based on readiness
end
Note over SchemaRegistry,ClickHouse: On boot failure, main starts RetryRefresh(ctx) in background\nSchemaRegistry->>SchemaRegistry: RetryRefresh with exponential backoff
SchemaRegistry->>ClickHouse: Retry Refresh attempts
ClickHouse-->>SchemaRegistry: fail/succeed responses
SchemaRegistry-->>BootState: on success clear BootState (Set(nil))
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 9: The Markdown heading "### Changed" currently lacks required
surrounding blank lines; edit CHANGELOG.md to insert one blank line immediately
before and one blank line immediately after the "### Changed" heading so it has
a blank line above and below, satisfying the MD022 lint rule.
- Line 10: Update the wording in CHANGELOG.md to remove the contradiction:
change the sentence that currently claims both endpoints "stay 200 for the rest
of the process lifetime" to clarify that once BootState.Set(nil) succeeds the
/health endpoint remains sticky-200 for the rest of the process lifetime while
/ready remains conditional on current ClickHouse reachability (i.e.,
BootState.Err() is nil makes /health 200 permanently but /ready can still return
503 on transient runtime blips); reference SchemaRegistry.RetryRefresh and
BootState.Set(nil)/BootState.Err() in the sentence to make the behavior
explicit.
In `@cmd/wavehouse/main.go`:
- Around line 123-147: The current startup can run registry.RetryRefresh and
registry.StartAutoRefresh concurrently causing overlapping Refresh calls and
stale bootState; ensure StartAutoRefresh is only started after boot recovery
(i.e., after a successful registry.Refresh or when the retry loop succeeds) or
make any successful Refresh (including the periodic auto-refresh) clear
bootState; specifically, move or guard the call to registry.StartAutoRefresh so
it only executes after bootState is cleared (or call bootState.Set(nil) inside
the success path of the periodic refresh handler), referencing registry.Refresh,
registry.RetryRefresh, registry.StartAutoRefresh and bootState.Set to locate and
update the logic.
In `@docs/api.md`:
- Line 86: Update the phrasing in the `/health` docs to say the gateway “isn't
ready to serve traffic yet” instead of “hasn't bound traffic yet”; edit the
sentence describing boot-degraded behavior (mentioning boot-degraded, `/health`,
and port `:8080`) so it clarifies that the service binds `:8080` and serves
diagnostics but is not yet ready to serve traffic, keeping the rest of the
explanation about schema discovery backoff and `/ready` behavior unchanged.
In `@internal/api/health_test.go`:
- Around line 100-163: Replace the manual JSON parsing and assertions in
TestHealth_Liveness_BootDegraded, TestHealth_Liveness_BootReadyFlipsTo200, and
TestHealth_Readiness_BootDegradedReports503 with the shared test helpers: call
testutil.AssertJSONContains(t, w, http.StatusServiceUnavailable, "connection
refused") (or the appropriate status/substring) for the degraded/error checks
and use testutil.AssertJSONResponse(t, w, http.StatusOK,
map[string]string{"status":"ok"}) (or matching expected map) for the OK
readiness case; update the calls around h.Liveness and h.Readiness invocations
and remove the json.Unmarshal/resp variable and individual header/status
assertions that these helpers cover.
In `@internal/discovery/discovery_test.go`:
- Around line 404-509: These four separate tests for RetryRefresh
(TestRetryRefresh_SucceedsOnFirstAttempt, TestRetryRefresh_RetriesUntilSuccess,
TestRetryRefresh_ReturnsOnContextCancel, TestRetryRefresh_BackoffIsBounded and
the NilOnAttempt case) should be collapsed into a single table-driven suite:
create tests := []struct{name string; errs []error; initial time.Duration; max
time.Duration; setupCtx func() (context.Context, func()); wantErr bool;
onAttempt func(error); assertions func(t *testing.T, connFake interface{},
captured []error, start time.Time)} and iterate with for _, tt := range tests {
t.Run(tt.name, func(t *testing.T){ t.Parallel(); sr, conn := newFakeRegistry(t,
tt.errs); ctx, cancel := tt.setupCtx(); defer cancel(); start := time.Now(); var
captured []error; err := sr.RetryRefresh(ctx, tt.initial, tt.max, func(e error){
if tt.onAttempt!=nil { tt.onAttempt(e) } captured = append(captured, e) }); if
tt.wantErr { require.Error(t, err) } else { require.NoError(t, err) };
tt.assertions(t, conn, captured, start) }) }, ensuring each original assertion
(call counts, captured errors, elapsed bounds, context cancellation behavior,
nil onAttempt safety) is implemented in the corresponding test case entry and
referencing RetryRefresh and newFakeRegistry to locate the logic.
- Around line 494-497: The wall-clock upper bound in the test is too tight and
causes CI flakiness; in the block using start and elapsed (computed via elapsed
:= time.Since(start)) update the assertion that checks the upper bound
(currently assert.Less(t, elapsed, 100*time.Millisecond)) to a more permissive
threshold (e.g., 200–250ms) so slow shared runners won't fail while keeping the
lower bound assert.GreaterOrEqual(t, elapsed, 10*time.Millisecond) intact;
adjust only the upper-bound value in the assertion that references elapsed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a78f2fb3-1182-4e2b-a2fa-a4fbba952fa4
📒 Files selected for processing (10)
AGENTS.mdCHANGELOG.mdcmd/wavehouse/main.godocs/api.mddocs/architecture.mddocs/deployment.mdinternal/api/health.gointernal/api/health_test.gointernal/discovery/discovery.gointernal/discovery/discovery_test.go
Resolves AGENTS.md KDD collision: keep main's #15 Observability invariants and #16 Bearer-token CORS posture; renumber non-fatal boot to #17. Docs auto-merged from docs/*.md → docs/src/content/docs/*.md (Astro restructure landed on main in #7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four CodeRabbit findings on commit 2144b3a: 1) [Minor] CHANGELOG MD022 + sticky-/health vs conditional-/ready wording. The previous prose said both endpoints "stay 200 for the rest of the process lifetime" then contradicted itself by noting /ready reflects transient blips. Rewrote to make explicit: /health is sticky-200 after the first Refresh success (it answers "did boot complete once"); /ready remains conditional on current ClickHouse reachability (it answers "can I serve traffic right now"). Also added the missing blank line above ### Changed and updated the doc paths to the post-Astro-merge locations. 2) [Major] StartAutoRefresh + RetryRefresh race. Two goroutines were calling Refresh concurrently during boot (mutex-safe, but stale bootState if the auto-refresh tick succeeded first). Restructured so StartAutoRefresh only starts after the first successful Refresh (sync or retry). One Refresh caller at a time, no stale-bootState window. 3) [Minor] /health docs bind-state phrasing — "hasn't bound traffic yet" was inaccurate (the binary IS bound on :8080 in boot-degraded mode, it serves /health diagnostics — it just isn't accepting ingest/query traffic). Reworded to "isn't ready to serve traffic yet" with an explicit clarifying sentence. 4) [Major] TestRetryRefresh_BackoffIsBounded wall-clock upper bound tightened to 100ms was too aggressive for shared CI runners — the 15ms-of-real-sleeps budget can stretch past 100ms under scheduler pressure. Relaxed to 250ms, which still catches a real unbounded- backoff regression (orders-of-magnitude blowup) without flaking. Pushed back on two: - testutil.AssertJSONResponse/AssertJSONContains aren't a clean fit for these tests (substring-on-error-message + deliberate header coverage on the 503 branches — see TestHealth_Readiness_PingFails comment). Existing health_test.go tests use the same manual-parsing pattern; one consistent style across the file > selective helper adoption. - 5-into-1 table-driven consolidation of the RetryRefresh tests would produce a struct with mostly-nil optional fields per row (different setup, different assertions, ctx-cancel needs goroutine+chan orchestration). Current form is short, top-down readable. The neighbour TestClampBackoff IS table-driven (6 pure-fn input/output rows) — used where it actually fits. All four Gemini concerns from the earlier review remain resolved (Gemini explicitly acknowledged each pushback in the merged review threads on 2c57041 — sanitization, time.After memory leak, AGENTS.md placement). make verify + make test-unit pass; discovery coverage holds at 89.8%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
0 [MUST] · 0 [SHOULD] · 0 [MAY] — no open findings on HEAD All items from prior review cycles are resolved:
Implementation notes for the record: Ship it — resolve any open CodeRabbit threads before merge (ruleset requires thread resolution), then this is good to go. |
taitelee
left a comment
taitelee
left a comment
There was a problem hiding this comment.
New bootstrap architecture that allows the server to start and answer health probes even if ClickHouse is initially unreachable. With a new background RetryRefresh loop with exponential backoff, WaveHouse avoids immediate startup crashes and remains in a "degraded" state until the database becomes available. The main.go file now properly handles this lifecycle by injecting a bootState into the health handler and making sure background tasks are tied to a process wide context for clean shutdowns. New unit tests added to confirm that backoff intervals are capped and that the system gracefully handles service interruptions without data loss or logging noise.
# Conflicts: # CHANGELOG.md
EnsureDLQStream, policy.NewStore, and pipes.NewStore previously took context.Background(). Since #125 creates the process-lifetime ctx earlier (for the schema discovery retry goroutine), these three init calls can now share it — letting SIGINT/SIGTERM cancel them cleanly rather than relying on Docker SIGKILL as the only escape. All three operate against embedded JetStream (in-process), so the practical risk of a hang is near zero — but the inconsistency with the surrounding ctx usage was noted in PR #125 review (taitelee) and the change is mechanical with zero behavioral impact in the happy path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
04709d8
Review — 0 [MUST], 0 [SHOULD], 0 [MAY]Re-review against HEAD What the final commit adds: moves Everything I re-checked against AGENTS.md:
Ship it — architecture is sound, wiring is correct, coverage is strong, all required doc updates are present. Prior advisory threads (timer leak, error sanitization) are tracked and the author's documented decision to accept both stands. |
github-project-automation
Bot
moved this from Ready
to In progress
in WaveHouse Task Board
github-project-automation
Bot
moved this from In progress
to Done
in WaveHouse Task Board
…ator After merging main's boot-resilience and health work (#125, #122), e2e coverage dipped to 49.9% (gate is 50%) because the Readiness handler and the cmd/wavehouse/health.go probe binary were uncovered by the SDK harness. Both are production code paths the operator-facing contract (k8s readiness, Docker HEALTHCHECK) depends on — exercising them in the e2e harness is principled, not a coverage hack. Brings e2e from 49.9% → 50.9%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Umbrella PR setting up shared Claude Code + AI agent infrastructure for the WaveHouse team. Two work streams: 1. **AI rules drift cleanup** — corrected stale references that AI tools (Claude Code, Gemini Code Assist, Copilot, CodeRabbit) were following blindly. 2. **Claude Code native tooling** — committed `.claude/` configuration and `.githooks/` so every teammate gets identical dev affordances out of the box, with agent-specific gating layered on top. The team just got Max 20x subscriptions across the board; this lands the team-wide config so everyone is on the same agentic dev experience by default. ## Scope ### 1. AI rules drift cleanup - **17 doc-path references corrected** across AGENTS.md + CONTRIBUTING.md to `docs/src/content/docs/*.md` (the actual Astro Starlight location, not the old flat layout). - **`.github/copilot-instructions.md` shrunk to a pointer** — was drifting on Go 1.25 (vs current 1.26.3) and 60% coverage (vs current 80% total / 70% unit per `.testcoverage.yml`). - **`.gemini/styleguide.md`** — stale `#67` / 60% claim fixed (issue closed, 70% restored); duplicated doc-sync bullet collapsed to defer to AGENTS.md (already authoritative). - **`.github/labeler.yml`** — dropped non-existent `cmd/wavehouse-{api,worker}/**` entries; fixed `tests/{compose.yaml,sdk/**}` → `tests/e2e/...`; added `cmd/wavehouse/**` to `area/infra`. - **`.github/prompts/pr-review.md`** — doc-sync list collapsed; vestigial "tenant" wording dropped (no tenant model in WaveHouse); hard-wrap reflowed (180 → 81 lines). - **AGENTS.md** — `cmd/*/main.go` (plural) → `cmd/wavehouse/main.go` (one binary); `tests/fixtures/` → `tests/e2e/fixtures/`; removed stale "update `triage.yml` area enumeration" step (workflow now discovers `area/*` labels dynamically); fixed internal-package count. - **CONTRIBUTING.md** — vestigial "tenant isolation" wording removed. - **TODO.md deleted** — audit summary below. ### 2. Claude Code native tooling - **`.claude/`** — shared configuration: `settings.json` (deny rules + worktree config + three hooks wired), `agents/pre-push-reviewer.md`, `hooks/agent-bash-gate.sh` (PreToolUse Bash gate), `hooks/review-marker.sh` (PostToolUse Agent marker writer), `hooks/gofumpt-on-save.sh` (auto-format), `skills/pr-review-locally/`, `skills/pr-sync-with-main/`, `commands/cover.md`. - **`.githooks/`** — universal team hooks installed by `make tools`: `pre-commit` runs `make verify`; `pre-push` requires `tmp/ci-passed-<HEAD-sha>` marker (written by `make ci`). - **`.config/wt.toml`** — worktrunk project hooks so parallel-agent worktrees install `.githooks/` correctly. - **AGENTS.md §"Agent PR Discipline"** — new section codifying the agent-only ruleset: - Drafts-only PR creation; human-only ready/approve/request-changes/reviewer-add transitions. - Bot reviewer re-triggers go through PR comments (`@coderabbitai review`, `@gemini-code-assist`, `@claude` / `/review`). - **Pre-push self-review mandatory** on PR branches: agent invokes `pre-push-reviewer` subagent in fresh context. `ship_it` requires zero findings at any severity — any `[MUST]` / `[SHOULD]` / `[MAY]` forces iterate; the orchestrator loops review → fix → review until clean. - **Honest-agent marker policy**: `--no-verify` regex-blocked + the obvious marker-write idioms denied at the permission layer (`Bash(touch tmp/ci-passed:*)`, `Write`/`Edit` on the canonical paths); everything else is a documented rule, not regex-enforced. Bash can write a file by a dozen paths and regex enforcement is a porous game of whack-a-mole. - **`docs/src/content/docs/claude-code.md`** — contributor-facing page documenting the four-layer model (universal git hooks → agent gate → ergonomic hooks → skills/agents/commands), quick setup, and discipline rules. - **CHANGELOG.md** — `[Unreleased]` entry covering all of the above. ## Out-of-tree GitHub changes that pair with the AI-rules cleanup Done via `gh` CLI as part of the same audit: - **Closed #46** (Graceful Shutdown) — verified shipped in `cmd/wavehouse/main.go:378-393` (SIGINT/SIGTERM → bounded shutCtx → ingestStream.Stop → srv.Shutdown → promSrv.Shutdown). - **Scope notes added to #44, #50, #94** with current-status / boundary info (ldflags shipped vs `/version` remaining; DLQ shipped vs retry remaining + scope boundary with #91; per-component logger source field as a #94 complement). - **Opened 4 new issues from orphan TODOs**: #143 (pprof), #144 (K8s `/healthz` + per-dep health), #145 (RequireRoles fail-closed), #146 (split `internal/api/` into focused subpackages). ## TODO.md audit (one-time, for record) | Bucket | Count | Disposition | |--------|-------|-------------| | Already shipped per closed issues (#11, #14, #16, #28, #40, #41, #42, #45) + current code | ~12 | Deleted from TODO | | Tracked as open issues (#32, #33, #34, #37, #39, #44, #48, #49, #50, #51, #94) | ~12 | Kept as issues, scope notes added where useful | | In-flight via open PRs (#83, #92, #119, #122, #125, #136, #137) | 4 | Untouched | | #46 Graceful Shutdown | 1 | Verified shipped, closed with comment | | Orphan items | 4 | Split into #143-146 | | Aspirational ("more tests", "update README") | 2 | Deleted — covered by AGENTS.md doc-sync rules | Projects #7 board + triage automation is now the single canonical backlog. ## Test plan - [x] `make ci` passes locally for each push (gated by `.githooks/pre-push`) - [x] CI green on the latest HEAD (8fbd7db) - [x] PR-title-lint accepts the title (`chore: claude code native improvements`) - [x] All `docs/src/content/docs/*.md` paths in AGENTS.md resolve to real files - [x] Labeler workflow auto-labels correctly per the updated paths - [x] `pre-push-reviewer` subagent loop reached `VERDICT: ship_it` with zero findings under the strict rubric before the final push (validated end-to-end across five iterations on this branch — each surfacing a real doc-sync / off-by-one / quote-strip issue and forcing a fix before the marker auto-wrote) - [x] `agent-bash-gate.sh` quote-strip generalization sanity-tested live: `echo "git push to deploy"` passes through; `git push --no-verify` and `git commit --no-verify` still block (`bash -n` clean, JSON wiring valid) - [ ] Human review ## Related issues - Closed during this work: #46 - Scope notes added: #44, #50, #94 - New follow-up issues created: #143, #144, #145, #146 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Claude Code integration: local pre-push reviewer with strict ship/iterate/block verdicts, push gating via CI/review markers, automatic review-marker creation, and a coverage-reporting command. * **Documentation** * Comprehensive Claude Code & agent docs, new skill guides for PR review/sync, updated README/CONTRIBUTING/CHANGELOG/styleguide, and site sidebar/page additions. * **Chores** * Added git and agent hooks, CI marker creation, worktrunk config, labeler tweaks, and simplified Copilot instructions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/Wave-RF/WaveHouse/pull/147?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/Wave-RF/WaveHouse/pull/147?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
2 participants
Summary
Closes #95.
cmd/wavehouseno longer crash-loops when ClickHouse is unreachable on boot. The firstSchemaRegistry.Refreshfailure now sets a newapi.BootStatediagnostic instead ofos.Exit(1); a goroutine calls the newSchemaRegistry.RetryRefresh(exponential backoff 2s → 60s) until success or shutdown./healthreturns503 {"status":"degraded","error":"schema discovery: …"}and/readyreturns503 {"status":"not ready", …}so an operator cancurl /healthto learn why the gateway isn't accepting traffic instead of grepping a restart-loop log. Once a Refresh succeeds, both flip back to 200 and stay there — transient runtime CH blips after boot are reflected in/readyonly.BootStateget/set, Liveness degraded → ready transition, Readiness boot-degraded reporting).make cipasses locally: combined coverage 81.2% (≥ 80% gate), discovery 98.1%, all per-suite gates green, all 30 E2E SDK tests pass.Test plan
make verify(tidy + fmt + vulncheck + lint)make test-unit(unit gate: 74.5% ≥ 70%)make test-sdk,make test-integration,make test-e2emake ciend-to-end: combined gate 81.2% ≥ 80%curl /healthreturns 503 with diagnostic, then 200 once CH is brought up (not run locally; reviewers welcome to spot-check viamake deps-down && bin/wavehousein a separate terminal)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation