feat(settings): hot-reload the settings directory at boot and runtime - #508
feat(settings): hot-reload the settings directory at boot and runtime#508taitelee wants to merge 3 commits into
Conversation
… required keys, and init-settings
github-actions
Bot
added
documentation
|
Warning Review limit reached
Next review available in: 10 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe application now loads tenant settings from a required JSON directory. It seeds default files, validates them at boot, reloads them through watch, ChangesSettings directory runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟠 High · up to The current implementation can silently drop valid records when identical IDs occur in different tables, and settings changes may be missed after filesystem notification overflow, leaving stale runtime configuration active. These concrete correctness and availability risks should be fixed or explicitly accepted before merge; the floating runtime base image also weakens deployment reproducibility. Sequence Diagram(s)sequenceDiagram
participant Client
participant SettingsHandler
participant SettingsStore
participant APIConsumers
Client->>SettingsHandler: POST /v1/ops/settings/reload
SettingsHandler->>SettingsStore: TriggerReload("api")
SettingsStore-->>SettingsHandler: Findings and adoption status
APIConsumers->>SettingsStore: Read current settings
SettingsStore-->>APIConsumers: Dedupe, query, schema, and CORS values
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📚 Docs preview is live → https://14883740-wavehouse-docs.wave-rf.workers.dev |
Code Coverage OverviewLanguages: Go GoThe overall line coverage in commit 5646dab in the Show a line coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e472820d-ea24-4cec-91dd-ebe59cdf26b7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (50)
.goreleaser.yaml.testcoverage.ymlAGENTS.mdCHANGELOG.mdcmd/wavehouse/init_settings.gocmd/wavehouse/init_settings_test.gocmd/wavehouse/main.gocmd/wavehouse/validate_test.goconfig.yamldeployments/Dockerfiledeployments/Dockerfile.goreleaserdeployments/compose/standalone.yamldocs/src/content/docs/api.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/development.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/sdk/index.mdxgo.modinternal/api/ingest.gointernal/api/ingest_test.gointernal/api/router.gointernal/api/router_test.gointernal/api/settings.gointernal/api/settings_test.gointernal/api/structured_query.gointernal/api/structured_query_test.gointernal/config/config.gointernal/config/config_test.gointernal/discovery/discovery.gointernal/settings/finding.gointernal/settings/seed.gointernal/settings/seed/config.jsoninternal/settings/seed/pipes.jsoninternal/settings/seed/policies.jsoninternal/settings/seed/roles.jsoninternal/settings/settings.gointernal/settings/store.gointernal/settings/store_test.gointernal/settings/validate.gointernal/settings/validate_test.gointernal/settings/watch.gointernal/settings/watch_test.gotests/e2e/fixtures/config.yamltests/e2e/fixtures/settings/config.jsontests/e2e/fixtures/settings/pipes.jsontests/e2e/fixtures/settings/policies.jsontests/e2e/fixtures/settings/roles.jsontests/integration/query_limits_test.go
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: E2E tests
- GitHub Check: Coverage
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: - No global state: Dependencies are passed explicitly (constructor injection).
- Go 1.26, strict formatting (
gofumpt, enforced by CI)
Files:
tests/integration/query_limits_test.gocmd/wavehouse/validate_test.gointernal/settings/finding.gocmd/wavehouse/init_settings.gointernal/api/settings_test.gointernal/settings/watch_test.gointernal/api/structured_query_test.gointernal/settings/seed.gointernal/api/structured_query.gocmd/wavehouse/init_settings_test.gointernal/api/ingest.gointernal/settings/watch.gointernal/api/settings.gointernal/settings/settings.gointernal/settings/validate.gointernal/discovery/discovery.gointernal/settings/validate_test.gointernal/api/router.gointernal/api/ingest_test.gointernal/settings/store_test.gointernal/config/config.gointernal/settings/store.gointernal/api/router_test.gointernal/config/config_test.gocmd/wavehouse/main.go
tests/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
- Table-driven tests: Use
tests := []struct{ name string; ... }witht.Run(tt.name, ...)for test cases.
Files:
tests/integration/query_limits_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
- Every new function should have corresponding test cases. Run
make lintandmake testbefore considering work complete.
Files:
tests/integration/query_limits_test.gocmd/wavehouse/validate_test.gointernal/api/settings_test.gointernal/settings/watch_test.gointernal/api/structured_query_test.gocmd/wavehouse/init_settings_test.gointernal/settings/validate_test.gointernal/api/ingest_test.gointernal/settings/store_test.gointernal/api/router_test.gointernal/config/config_test.go
internal/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
- Package naming: Lowercase, single word (or abbreviated).
internal/enforces module privacy.
Files:
internal/settings/finding.gointernal/api/settings_test.gointernal/settings/watch_test.gointernal/api/structured_query_test.gointernal/settings/seed.gointernal/api/structured_query.gointernal/api/ingest.gointernal/settings/watch.gointernal/api/settings.gointernal/settings/settings.gointernal/settings/validate.gointernal/discovery/discovery.gointernal/settings/validate_test.gointernal/api/router.gointernal/api/ingest_test.gointernal/settings/store_test.gointernal/config/config.gointernal/settings/store.gointernal/api/router_test.gointernal/config/config_test.go
internal/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
- Bearer-token-only CORS posture (security) — Bearer JWT on every request, no cookies/sessions;
corsMiddlewaredeliberately never emitsAccess-Control-Allow-Credentials(not needed, and*+ credentials is a spec violation browsers reject).
Files:
internal/api/settings_test.gointernal/api/structured_query_test.gointernal/api/structured_query.gointernal/api/ingest.gointernal/api/settings.gointernal/api/router.gointernal/api/ingest_test.gointernal/api/router_test.go
**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
- In MDX, leave a blank line between a JSX tag and a code fence.
Files:
docs/src/content/docs/sdk/index.mdxdocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
docs/src/content/**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
docs/src/content/**/*.mdx: - Opt a page into the Cloud CTA withcloudCtafrontmatter, not by importing the component.
- Never hand-write
®or™in prose.- Never hand-write
utm_*params orrelon a link towavehouse.cloudorwave-rf.com. UsecloudLink()/relFor()fromdocs/src/config/outbound.ts.
Files:
docs/src/content/docs/sdk/index.mdxdocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
- Never hard-wrap prose. One paragraph is one line. No wrapping at 72/80 columns, no "semantic linefeeds" splitting a paragraph at sentence boundaries.
Files:
docs/src/content/docs/getting-started.mddocs/src/content/docs/development.mdAGENTS.mddocs/src/content/docs/api.mdCHANGELOG.mddocs/src/content/docs/deployment.md
**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
New workflows must follow the same pattern — never
@mainor floating tags on third-party actions.
Files:
deployments/compose/standalone.yamltests/e2e/fixtures/config.yamlconfig.yaml
🧠 Learnings (2)
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/settings/validate_test.gointernal/settings/store_test.go
📚 Learning: 2026-08-13T12:17:52.620Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: docs/src/content/docs/reverse-proxy.mdx:137-144
Timestamp: 2026-08-13T12:17:52.620Z
Learning: For Wave-RF/WaveHouse documentation, verify claims about implementation control flow against the authoritative implementation source (for example, internal/auth/auth.go) rather than relying solely on docs/** content. Documentation may lag behind or paraphrase behavior, so control-flow claims should be confirmed in source code.
Applied to files:
docs/src/content/docs/configuration.mdx
🪛 Checkov (3.3.10)
deployments/Dockerfile
[low] 42-42: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
🪛 Hadolint (2.15.1)
deployments/Dockerfile
[warning] 42-42: Always tag the version of an image explicitly
(DL3006)
[info] 45-45: Non-numeric user-id may not be resolvable by host system
(DL3066)
🪛 LanguageTool
CHANGELOG.md
[style] ~13-~13: Consider an alternative for the overused word “exactly”.
Context: ...issing one), so the adopted snapshot is exactly what the files say, and once adopted it...
(EXACTLY_PRECISELY)
[style] ~13-~13: Since ownership is already implied, this phrasing may be redundant.
Context: ...et (bare docker run boots; bind-mount your own directory over it), and the dev `config...
(PRP_OWN)
docs/src/content/docs/configuration.mdx
[style] ~188-~188: Consider an alternative for the overused word “exactly”.
Context: ...hey live, and what the server adopts is exactly what the files say. The container image...
(EXACTLY_PRECISELY)
[style] ~188-~188: Since ownership is already implied, this phrasing may be redundant.
Context: ...o a bare docker run boots; bind-mount your own directory over that path to edit. Chec...
(PRP_OWN)
[style] ~204-~204: Consider using the typographical ellipsis character here instead.
Context: ...ttings/reload** (admin-only) — returns {"adopted": bool, "findings": [...]}; 200when adopted,422` when reje...
(ELLIPSIS)
🪛 Trivy (0.73.0)
deployments/Dockerfile
[warning] 42-42: ':latest' tag used
Specify a tag in the 'FROM' statement for image 'gcr.io/distroless/static-debian12'
Rule: DS-0001
(IaC/Dockerfile)
🔇 Additional comments (41)
AGENTS.md (2)
61-62: LGTM!Also applies to: 65-65
57-57: 🗄️ Data Integrity & Integration | 🏗️ Heavy liftMake the Issue
#222table-isolation contract explicit and executable.The changes mention per-table
id_fieldselection but do not represent the required table-qualified deduplication keyspace in the invariant or the e2e settings fixture.
AGENTS.md#L57-L57: state that deduplication keys include table identity and that writes and lookups use the same namespace.tests/e2e/fixtures/settings/config.json#L1-L16: add table overrides and test equal IDs in different tables, or link an existing test that proves both behaviors..testcoverage.yml (1)
75-86: LGTM!config.yaml (1)
53-55: LGTM!Also applies to: 85-99
tests/e2e/fixtures/config.yaml (1)
8-14: LGTM!Also applies to: 31-36
tests/e2e/fixtures/settings/pipes.json (1)
1-1: LGTM!tests/e2e/fixtures/settings/policies.json (1)
1-3: LGTM!tests/e2e/fixtures/settings/roles.json (1)
1-3: LGTM!docs/src/content/docs/api.md (1)
597-597: LGTM!Also applies to: 799-813
docs/src/content/docs/configuration.mdx (1)
26-27: LGTM!Also applies to: 42-43, 70-70, 116-116, 131-137, 188-236, 303-304, 328-331, 392-392
docs/src/content/docs/deployment.md (1)
62-62: LGTM!Also applies to: 157-171, 369-369
docs/src/content/docs/development.md (1)
147-147: LGTM!Also applies to: 231-238
docs/src/content/docs/getting-started.md (1)
98-98: LGTM!Also applies to: 116-116
docs/src/content/docs/reverse-proxy.mdx (1)
25-25: LGTM!Also applies to: 199-199
docs/src/content/docs/sdk/index.mdx (1)
404-404: LGTM!go.mod (1)
23-23: LGTM!internal/settings/finding.go (1)
18-24: LGTM!internal/settings/store_test.go (1)
13-157: LGTM!cmd/wavehouse/validate_test.go (1)
22-22: LGTM!.goreleaser.yaml (1)
85-88: LGTM!internal/api/settings.go (1)
11-51: LGTM!internal/api/structured_query.go (1)
30-35: LGTM!Also applies to: 52-52, 122-126
internal/settings/seed.go (1)
1-61: LGTM!internal/settings/seed/config.json (1)
1-16: LGTM!internal/settings/seed/pipes.json (1)
1-4: LGTM!internal/settings/seed/policies.json (1)
1-2: LGTM!internal/settings/seed/roles.json (1)
1-4: LGTM!internal/settings/validate_test.go (1)
4-4: LGTM!Also applies to: 30-64, 96-96, 264-271, 335-335, 358-368, 378-378
tests/integration/query_limits_test.go (1)
102-102: LGTM!internal/settings/settings.go (1)
63-84: LGTM!Also applies to: 98-118
internal/settings/validate.go (1)
286-309: LGTM!Also applies to: 338-362
internal/settings/store.go (1)
10-131: LGTM!internal/config/config.go (1)
13-32: LGTM!Also applies to: 45-52, 108-112, 142-147, 222-224
internal/config/config_test.go (1)
14-34: LGTM!Also applies to: 101-107, 189-190, 203-216, 227-228, 239-241, 263-264, 282-283, 302-303, 319-320, 344-345, 372-373, 391-392, 427-428, 445-446, 458-459, 472-473, 487-488
cmd/wavehouse/init_settings.go (1)
14-50: LGTM!cmd/wavehouse/main.go (1)
31-31: LGTM!Also applies to: 102-116, 136-137, 177-188, 295-331, 473-473, 531-537
cmd/wavehouse/init_settings_test.go (1)
12-42: LGTM!internal/api/ingest.go (1)
39-45: LGTM!internal/api/router.go (1)
20-43: LGTM!Also applies to: 196-198, 299-341
internal/api/structured_query_test.go (1)
32-43: LGTM!Also applies to: 285-298
internal/discovery/discovery.go (1)
49-59: LGTM!Also applies to: 217-255
github-project-automation
Bot
moved this from In progress
to In review
in WaveHouse Task Board
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/settings/watch.go (1)
80-86: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReconcile settings after a watcher queue overflow.
fsnotify.ErrEventOverflowindicates that filesystem notifications were lost. When this error occurs, reset the debounce timer so its normal path callss.TriggerReload("watch"). That reload reparses the complete settings directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b2f6f5c1-0bde-4b64-8caa-cd85c395c4cc
📒 Files selected for processing (6)
CHANGELOG.mddeployments/compose/standalone.yamldocs/src/content/docs/configuration.mdxinternal/api/router_test.gointernal/settings/watch.gointernal/settings/watch_test.go
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: E2E tests
- GitHub Check: Integration tests
- GitHub Check: Docs build
- GitHub Check: Unit tests
- GitHub Check: Coverage
- GitHub Check: Validate snapshot build
- GitHub Check: Lint
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: - Go 1.26, strict formatting (gofumpt, enforced by CI)
- No global state: Dependencies are passed explicitly (constructor injection).
- Structured logging with
log/slog(JSON handler)
Files:
internal/settings/watch.gointernal/api/router_test.gointernal/settings/watch_test.go
internal/*/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
- Package naming: Lowercase, single word (or abbreviated).
internal/enforces module privacy.
Files:
internal/settings/watch.gointernal/api/router_test.gointernal/settings/watch_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: - Table-driven tests: Usetests := []struct{ name string; ... }witht.Run(tt.name, ...)for test cases.
- Every new function should have corresponding test cases. Run
make lintandmake testbefore considering work complete.
Files:
internal/api/router_test.gointernal/settings/watch_test.go
docs/src/content/docs/**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
docs/src/content/docs/**/*.mdx: - Opt a page into the Cloud CTA withcloudCtafrontmatter, not by importing the component.
- Never hand-write
®or™in prose.- Never hand-write
utm_*params orrelon a link towavehouse.cloudorwave-rf.com. UsecloudLink()/relFor()fromdocs/src/config/outbound.ts.
Files:
docs/src/content/docs/configuration.mdx
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Every code change should update the corresponding docs in the same PR. A code change without its doc update is incomplete.
Files:
CHANGELOG.md
🧠 Learnings (2)
📚 Learning: 2026-08-13T12:17:52.620Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: docs/src/content/docs/reverse-proxy.mdx:137-144
Timestamp: 2026-08-13T12:17:52.620Z
Learning: For Wave-RF/WaveHouse documentation, verify claims about implementation control flow against the authoritative implementation source (for example, internal/auth/auth.go) rather than relying solely on docs/** content. Documentation may lag behind or paraphrase behavior, so control-flow claims should be confirmed in source code.
Applied to files:
docs/src/content/docs/configuration.mdx
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/settings/watch_test.go
🪛 LanguageTool
CHANGELOG.md
[style] ~13-~13: Since ownership is already implied, this phrasing may be redundant.
Context: ...et (bare docker run boots; bind-mount your own directory over it), and the dev `config...
(PRP_OWN)
🔇 Additional comments (6)
internal/api/router_test.go (1)
8-8: LGTM!Also applies to: 160-160, 180-180, 198-198, 217-217, 256-256, 273-273, 291-291, 406-406
CHANGELOG.md (1)
13-13: LGTM!docs/src/content/docs/configuration.mdx (1)
26-27: LGTM!Also applies to: 42-43, 70-70, 116-116, 131-137, 188-236, 283-283, 298-304, 315-315, 328-331, 360-360, 373-376, 386-392
deployments/compose/standalone.yaml (1)
42-58: LGTM!internal/settings/watch.go (1)
6-6: LGTM!Also applies to: 17-17, 26-79
internal/settings/watch_test.go (1)
56-67: LGTM!Also applies to: 70-92
1 participant
Summary
The server now consumes the settings directory #500 only validated.
settings.dir/WH_SETTINGS_DIRis required: boot validates and adopts it (missing or invalid refuses to start, same contract aspolicy.file_path), and a running instance re-validates and re-adopts on a directory watch (fsnotify with debounce; survives a delete-and-recreate),SIGHUP, or admin-gatedPOST /v1/ops/settings/reload, all through one serialized reload path. A rejected reload, a deleted file, or a vanished directory keeps the previous good snapshot.config.json:dedupe.id_field/dedupe.require_id(with per-table overrides, Per-table dedupe id_field (+ fix cross-table dedupe keyspace collision) #222),query.default_max_rows,schema.refresh_interval,cors.allowed_origins. The YAML/env keys for these are removed;dedupe.enabledstays boot config since it owns Pebble's lifecycle.config.jsonkey is required and the binary carries no compiled defaults, so the adopted snapshot is what the files say. Defaults live in one checked-in seed (internal/settings/seed/,go:embedded), shipped three ways: the newwavehouse init-settings <dir>writes it (refusing a non-empty directory), both container images bake it at/app/settingswithWH_SETTINGS_DIRpreset, and the devconfig.yamland e2e fixture point at it. The seed ships no policy (fail-closed, warned).IngestHandler.DedupeSettings, the structured-querydefaultMaxRowsgetter,corsMiddlewareorigins getter,SchemaRegistry.SetIntervalSource) so a reload applies without a restart andinternal/apistays testable without a directory.Known gap: only
config.jsonchanges affect a running server.roles.json,policies.json, andpipes.jsonare validated and loaded into the snapshot, but editing them does nothing yet — access control still comes frompolicy.file_path+ NATS KV, and pipes from the/v1/ops/pipesAPI + NATS KV. The next PR makes the snapshot the runtime authority for all three and, per discussion with Eric, deletes thePUT /v1/ops/policyandPUT/DELETE /v1/ops/pipes/{name}endpoints and the NATS KV stores behind them: without KV there is no bundled node sync, so a node-local write path would let distributed nodes drift. Files (edited directly on standalone, written by the control plane in cloud) become the only write path.Related Issues
Advances #48 (reload wiring; reverses the #48-era runtime-settings direction) and #222 (per-table
id_field; the keyspace fix stays open there). Follow-up surfaced in review: #510 (distroless digest pin).