refactor(api)!: merge all admin-gated endpoints under /v1/ops - #479
Conversation
github-actions
Bot
added
documentation
|
📚 Docs preview is live → https://b91ea6ee-wavehouse-docs.wave-rf.workers.dev |
|
@coderabbitai review |
❌ Action failedReview failed.
|
|
Warning Review limit reached
Next review available in: 1 minute Limit details: You’ve used all 1 included review currently available under your plan. 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?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling 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 (12)
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:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe API consolidates administrative endpoints under ChangesOperations route migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to The PR moves all admin endpoints under /v1/ops and removes the old paths, but merge readiness is reduced by missing tests for the required 404 behavior and documentation inaccuracies around schema validation and authorization, including a health-endpoint description that could lead users to send privileged credentials to a public endpoint. Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant requireAdmin
participant OperationsHandler
Client->>Router: Request /v1/ops endpoint
Router->>requireAdmin: Apply shared admin gate
requireAdmin->>OperationsHandler: Forward authorized request
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
@coderabbitai full review |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ae594c5d-9f7b-4bc2-8879-d2744f17e45e
📒 Files selected for processing (63)
.github/prompts/pr-review.mdAGENTS.mdCHANGELOG.mdSECURITY.mdclients/ts/README.mdclients/ts/src/cli/codegen.tsclients/ts/src/client.test.tsclients/ts/src/client.tsclients/ts/src/dlq.tsclients/ts/src/http.test.tsclients/ts/src/namespaces.test.tsclients/ts/src/pipes.test.tsclients/ts/src/pipes.tsclients/ts/src/policy.tsclients/ts/src/schema.tsclients/ts/src/sql.tsclients/ts/src/table.test.tsclients/ts/src/table.tsclients/ts/src/url.test.tscmd/wavehouse/main.godeployments/compose/dev-policy.yamldocs/src/content/docs/access-control.mdxdocs/src/content/docs/api.mddocs/src/content/docs/architecture.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/ingest-pipeline.mddocs/src/content/docs/pipes.mdxdocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/index.mdxdocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/queries.mddocs/src/content/docs/sdk/reference.mddocs/src/content/docs/why-wavehouse.mdinternal/api/cache_key.gointernal/api/cache_key_test.gointernal/api/clickhouse_exec.gointernal/api/dlq_test.gointernal/api/errors.gointernal/api/errors_test.gointernal/api/ingest.gointernal/api/pipes.gointernal/api/pipes_test.gointernal/api/policy_test.gointernal/api/query.gointernal/api/query_test.gointernal/api/router.gointernal/api/router_test.gointernal/api/schema_test.gointernal/config/config.gointernal/config/config_test.gointernal/policy/roles.gointernal/policy/scalars.gointernal/policy/store.gotests/e2e/sdk/query.test.tstests/e2e/sdk/setup.tstests/integration/dlq_test.gotests/integration/ingest_test.gotests/integration/query_test.gotests/integration/setup_test.go
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Docs preview
- GitHub Check: E2E tests
- GitHub Check: Coverage
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{go,ts,tsx,js,jsx,md,mdx,yaml,yml}
📄 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:
docs/src/content/docs/why-wavehouse.mddocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/ingest-pipeline.mdclients/ts/src/table.tsinternal/api/cache_key_test.gointernal/policy/store.gointernal/api/ingest.gointernal/config/config.gotests/integration/ingest_test.gointernal/api/dlq_test.gotests/e2e/sdk/query.test.tsdeployments/compose/dev-policy.yamltests/integration/setup_test.goclients/ts/src/client.tstests/integration/dlq_test.gointernal/api/cache_key.goclients/ts/src/schema.tsclients/ts/src/table.test.tsinternal/config/config_test.godocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/queries.mdSECURITY.mdclients/ts/src/cli/codegen.tsinternal/api/schema_test.gointernal/policy/roles.gointernal/api/pipes.goclients/ts/src/policy.tstests/e2e/sdk/setup.tsinternal/api/router.goclients/ts/src/dlq.tsclients/ts/src/pipes.tscmd/wavehouse/main.godocs/src/content/docs/pipes.mdxdocs/src/content/docs/development.mdclients/ts/src/sql.tsinternal/api/clickhouse_exec.gointernal/policy/scalars.gointernal/api/errors.godocs/src/content/docs/sdk/reference.mdinternal/api/policy_test.gointernal/api/errors_test.godocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxclients/ts/README.mdclients/ts/src/url.test.tsAGENTS.mdinternal/api/query.goclients/ts/src/http.test.tsinternal/api/pipes_test.gointernal/api/router_test.goclients/ts/src/client.test.tsclients/ts/src/pipes.test.tsdocs/src/content/docs/deployment.mdinternal/api/query_test.godocs/src/content/docs/getting-started.mddocs/src/content/docs/access-control.mdxtests/integration/query_test.godocs/src/content/docs/reverse-proxy.mdxclients/ts/src/namespaces.test.tsdocs/src/content/docs/sdk/index.mdxdocs/src/content/docs/api.md
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
No global state: Dependencies are passed explicitly (constructor injection).
Files:
internal/api/cache_key_test.gointernal/policy/store.gointernal/api/ingest.gointernal/config/config.gotests/integration/ingest_test.gointernal/api/dlq_test.gotests/integration/setup_test.gotests/integration/dlq_test.gointernal/api/cache_key.gointernal/config/config_test.gointernal/api/schema_test.gointernal/policy/roles.gointernal/api/pipes.gointernal/api/router.gocmd/wavehouse/main.gointernal/api/clickhouse_exec.gointernal/policy/scalars.gointernal/api/errors.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/query.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.gotests/integration/query_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. Runmake lintandmake testbefore considering work complete.
Files:
internal/api/cache_key_test.gotests/integration/ingest_test.gointernal/api/dlq_test.gotests/integration/setup_test.gotests/integration/dlq_test.gointernal/config/config_test.gointernal/api/schema_test.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.gotests/integration/query_test.go
internal/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Register the route in
internal/api/router.go.
Files:
internal/api/cache_key_test.gointernal/api/ingest.gointernal/api/dlq_test.gointernal/api/cache_key.gointernal/api/schema_test.gointernal/api/pipes.gointernal/api/router.gointernal/api/clickhouse_exec.gointernal/api/errors.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/query.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.go
internal/config/config.go
📄 CodeRabbit inference engine (AGENTS.md)
Add the field to the appropriate struct in
internal/config/config.gowithyaml,env, andenv-defaulttags.
Files:
internal/config/config.go
tests/e2e/sdk/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/e2e/sdk/*.test.ts: Add new E2E scenarios astests/e2e/sdk/*.test.tsfiles using helpers fromtests/e2e/sdk/helpers.ts.
A new test file must (1) add its suite name toSUITESintables.tsand (2) get its names viaconst T = suiteTables("<suite>"), then referenceT.clicksetc. — never a bareclicks.
Files:
tests/e2e/sdk/query.test.ts
docs/src/content/docs/**/*.mdx
📄 CodeRabbit inference engine (AGENTS.md)
Never sit two large diagrams side-by-side. Wrap comparisons in
<div class="diagram-pair">…</div>, which stacks them vertically so each gets the full column width.
Files:
docs/src/content/docs/pipes.mdxdocs/src/content/docs/configuration.mdxdocs/src/content/docs/access-control.mdxdocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/sdk/index.mdx
🧠 Learnings (10)
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/why-wavehouse.mddocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/queries.mdSECURITY.mddocs/src/content/docs/development.mddocs/src/content/docs/sdk/reference.mddocs/src/content/docs/architecture.mdclients/ts/README.mdAGENTS.mddocs/src/content/docs/deployment.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/api.md
📚 Learning: 2026-08-11T15:22:20.507Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.
Applied to files:
clients/ts/src/table.tsclients/ts/src/client.tsclients/ts/src/schema.tsclients/ts/src/table.test.tsclients/ts/src/cli/codegen.tsclients/ts/src/policy.tsclients/ts/src/dlq.tsclients/ts/src/pipes.tsclients/ts/src/sql.tsclients/ts/src/url.test.tsclients/ts/src/http.test.tsclients/ts/src/client.test.tsclients/ts/src/pipes.test.tsclients/ts/src/namespaces.test.ts
📚 Learning: 2026-08-12T21:45:38.018Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, use the exported `PipeRequestOptions` type for `PipeRef.fetch` and shared pipe, table, or query-builder fetch options rather than `Pick<RequestOptions, "signal">` or `RequestOptions`. `PipeRequestOptions` declares `limit?: never`, ensuring object literals and named `RequestOptions` values containing `limit` fail type checking instead of silently dropping it. When only a signal is needed, use `PipeRequestOptions` or an inferred `{ signal }` object; do not expect a `RequestOptions` value to be assignable to `PipeRequestOptions`.
Applied to files:
clients/ts/src/table.tsclients/ts/src/client.tsclients/ts/src/schema.tsclients/ts/src/table.test.tsclients/ts/src/cli/codegen.tsclients/ts/src/policy.tsclients/ts/src/dlq.tsclients/ts/src/pipes.tsclients/ts/src/sql.tsclients/ts/src/url.test.tsclients/ts/src/http.test.tsclients/ts/src/client.test.tsclients/ts/src/pipes.test.tsclients/ts/src/namespaces.test.ts
📚 Learning: 2026-05-20T01:02:00.784Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:00.784Z
Learning: In WaveHouse’s internal API tests (files matching internal/api/**/*_test.go), follow the existing separation-of-concerns convention for testing the RequireRole middleware: inject `ContextKeyRole` directly into the request `context.Context` instead of using `testutil.MakeJWT`/JWT-driven flows. Do not refactor role-gate tests to use JWT tokens—JWT parsing and token handling are covered separately in `middleware_test.go` (the dedicated JWT parsing tests), and mixing those concerns would expand the failure surface and reduce isolation.
Applied to files:
internal/api/cache_key_test.gointernal/api/dlq_test.gointernal/api/schema_test.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.go
📚 Learning: 2026-05-23T01:23:59.268Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 174
File: internal/api/ingest_test.go:111-111
Timestamp: 2026-05-23T01:23:59.268Z
Learning: In WaveHouse Go tests in internal/api/**/*_test.go, use internal/testutil.AssertJSONErrorResponse(t, w) for HTTP error-path JSON assertions. Do not use (or reintroduce) package-local assertJSONErrorResponse helpers. AssertJSONErrorResponse verifies the response Content-Type is application/json, includes the X-Content-Type-Options: nosniff header, and that the JSON body contains an "error" field.
Applied to files:
internal/api/cache_key_test.gointernal/api/dlq_test.gointernal/api/schema_test.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.go
📚 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/api/cache_key_test.gotests/integration/ingest_test.gointernal/api/dlq_test.gotests/integration/setup_test.gotests/integration/dlq_test.gointernal/config/config_test.gointernal/api/schema_test.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.gotests/integration/query_test.go
📚 Learning: 2026-07-07T12:38:12.052Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:12.052Z
Learning: In this repo, do not add or recommend logging/tracing client IP addresses using naive or untrusted sources (e.g., `r.RemoteAddr` or directly trusting/deriving `X-Forwarded-For`) anywhere in the Go codebase. `middleware.RealIP` was removed due to IP-spoofing risks, and proper trusted-proxy-aware client-IP handling is intentionally deferred to issue `#333`. During code review, if proposed changes would record client IPs (including in audit paths such as `internal/auth/auth.go`), reject/redirect until `#333` lands with correct trusted-proxy configuration and safeguards.
Applied to files:
internal/api/cache_key_test.gointernal/policy/store.gointernal/api/ingest.gointernal/config/config.gotests/integration/ingest_test.gointernal/api/dlq_test.gotests/integration/setup_test.gotests/integration/dlq_test.gointernal/api/cache_key.gointernal/config/config_test.gointernal/api/schema_test.gointernal/policy/roles.gointernal/api/pipes.gointernal/api/router.gocmd/wavehouse/main.gointernal/api/clickhouse_exec.gointernal/policy/scalars.gointernal/api/errors.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/query.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.gotests/integration/query_test.go
📚 Learning: 2026-08-11T21:55:53.726Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/stream_test.go:159-176
Timestamp: 2026-08-11T21:55:53.726Z
Learning: For Go files in this repository, do not report direct type assertions solely because the `forcetypeassert` rule is commented out in `.golangci.yml`. Only flag a type assertion when there is an independent correctness, safety, or maintainability issue.
Applied to files:
internal/api/cache_key_test.gointernal/policy/store.gointernal/api/ingest.gointernal/config/config.gotests/integration/ingest_test.gointernal/api/dlq_test.gotests/integration/setup_test.gotests/integration/dlq_test.gointernal/api/cache_key.gointernal/config/config_test.gointernal/api/schema_test.gointernal/policy/roles.gointernal/api/pipes.gointernal/api/router.gocmd/wavehouse/main.gointernal/api/clickhouse_exec.gointernal/policy/scalars.gointernal/api/errors.gointernal/api/policy_test.gointernal/api/errors_test.gointernal/api/query.gointernal/api/pipes_test.gointernal/api/router_test.gointernal/api/query_test.gotests/integration/query_test.go
📚 Learning: 2026-05-20T20:30:15.808Z
Learnt from: taitelee
Repo: Wave-RF/WaveHouse PR: 172
File: internal/api/pipes_test.go:106-118
Timestamp: 2026-05-20T20:30:15.808Z
Learning: For WaveHouse pipes authorization allowlist checks, fix the empty-role fail-open behavior by (1) removing any outer guard that prevents allowlist evaluation when the incoming `role` is `""` (e.g., don’t short-circuit with `if role != "" { ... }`), and (2) during allowlist scanning, ensure only non-empty allowlist entries can match—e.g., require `ar != "" && ar == role` (so a malformed allowlist like `["" ]` cannot grant access to an empty incoming role via `"" == ""`).
Applied to files:
internal/api/pipes.gointernal/api/pipes_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/pipes.mdxdocs/src/content/docs/configuration.mdxdocs/src/content/docs/access-control.mdxdocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/sdk/index.mdx
🪛 Betterleaks (1.7.3)
docs/src/content/docs/development.md
[high] 214-216: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
docs/src/content/docs/architecture.md
[style] ~73-~73: Since ownership is already implied, this phrasing may be redundant.
Context: ...e JWT/JWKS authentication middleware is its own package, [auth/](#auth--authenticatio...
(PRP_OWN)
docs/src/content/docs/deployment.md
[typographical] ~363-~363: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...NATS stream under subjects dlq.{table}. This prevents infinite retry loops. Mon...
(WRB_QUESTION_MARK)
docs/src/content/docs/access-control.mdx
[typographical] ~435-~435: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...lidation**, or WaveHouse refuses to boot. That turns a typo, a missing mount, or ...
(WRB_QUESTION_MARK)
[typographical] ~436-~436: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...s denied (logged loudly, admin included). Seed one via PUT /v1/ops/policy using...
(WRB_QUESTION_MARK)
[style] ~463-~463: Since ownership is already implied, this phrasing may be redundant.
Context: ...ous callers get nothing, viewer reads its own tenant's rows with sensitive columns ma...
(PRP_OWN)
🔇 Additional comments (64)
internal/api/router.go (1)
151-170: LGTM!internal/api/router_test.go (1)
12-17: LGTM!Also applies to: 290-304, 414-414, 447-447, 507-520, 662-664, 690-690, 701-704
internal/api/errors_test.go (1)
68-68: LGTM!Also applies to: 78-78, 96-96, 118-118, 187-189, 206-213
internal/api/errors.go (1)
38-43: LGTM!Also applies to: 77-77
internal/api/pipes_test.go (1)
48-48: LGTM!Also applies to: 65-65, 80-80, 94-94, 300-300, 321-323, 337-340, 356-358, 373-373
internal/api/policy_test.go (1)
23-23: LGTM!Also applies to: 45-45, 70-70, 83-83, 97-97, 132-133, 167-167
internal/api/schema_test.go (1)
25-25: LGTM!Also applies to: 46-46, 64-64
internal/api/dlq_test.go (1)
25-25: LGTM!Also applies to: 64-64, 96-96
cmd/wavehouse/main.go (1)
349-349: LGTM!internal/api/query_test.go (1)
38-38: LGTM!Also applies to: 58-58, 87-87, 112-112, 479-479
clients/ts/src/client.test.ts (1)
147-154: LGTM!clients/ts/src/client.ts (1)
87-87: LGTM!clients/ts/src/pipes.test.ts (1)
82-93: LGTM!Also applies to: 102-117
clients/ts/src/pipes.ts (1)
71-71: LGTM!Also applies to: 82-82, 97-97, 109-109
clients/ts/src/policy.ts (1)
17-17: LGTM!Also applies to: 28-28, 43-43
clients/ts/src/sql.ts (1)
8-9: LGTM!Also applies to: 26-26, 44-44
clients/ts/src/table.ts (1)
187-187: LGTM!clients/ts/README.md (1)
13-14: LGTM!Also applies to: 35-35, 47-47, 155-155
clients/ts/src/cli/codegen.ts (1)
6-6: LGTM!Also applies to: 168-168
clients/ts/src/dlq.ts (1)
22-22: LGTM!Also applies to: 33-35
clients/ts/src/http.test.ts (1)
79-79: LGTM!Also applies to: 91-91, 103-103, 175-175, 200-205, 211-213
clients/ts/src/namespaces.test.ts (1)
25-30: LGTM!Also applies to: 71-90, 102-113, 123-130, 144-144
clients/ts/src/schema.ts (1)
18-18: LGTM!Also applies to: 41-41
clients/ts/src/table.test.ts (1)
208-208: LGTM!Also applies to: 218-218
clients/ts/src/url.test.ts (1)
25-26: LGTM!Also applies to: 31-32, 43-44, 55-55
.github/prompts/pr-review.md (1)
34-34: LGTM!AGENTS.md (1)
38-38: LGTM!Also applies to: 60-60
SECURITY.md (1)
30-30: LGTM!deployments/compose/dev-policy.yaml (1)
10-10: LGTM!Also applies to: 22-22
internal/api/cache_key_test.go (1)
10-10: LGTM!internal/api/clickhouse_exec.go (1)
23-23: LGTM!internal/config/config.go (1)
172-172: LGTM!internal/config/config_test.go (1)
448-448: LGTM!internal/policy/roles.go (1)
36-36: LGTM!Also applies to: 47-47
internal/policy/scalars.go (1)
19-19: LGTM!internal/api/cache_key.go (1)
12-12: LGTM!internal/api/query.go (1)
15-17: LGTM!internal/policy/store.go (1)
125-125: LGTM!tests/e2e/sdk/query.test.ts (1)
155-155: LGTM!tests/e2e/sdk/setup.ts (1)
51-61: LGTM!Also applies to: 88-88
tests/integration/dlq_test.go (1)
26-26: LGTM!Also applies to: 70-70, 114-114
tests/integration/ingest_test.go (1)
57-57: LGTM!tests/integration/query_test.go (1)
22-22: LGTM!Also applies to: 36-36, 57-64, 89-89, 137-137, 147-149
tests/integration/setup_test.go (1)
319-319: LGTM!internal/api/ingest.go (1)
167-167: LGTM!internal/api/pipes.go (1)
118-118: LGTM!docs/src/content/docs/access-control.mdx (2)
60-65: LGTM!Also applies to: 75-75, 90-90, 102-102, 237-237, 382-382, 413-422, 434-436, 463-468
56-58: 🔒 Security & PrivacyKeep the documented fallback behavior.
Invalid, expired, and malformed tokens resolve through
default_rolebeforeRequireAdminchecks admin access. Withdefault_role == admin_role, they receive admin access;401is returned only when a gate denies the request.docs/src/content/docs/api.md (1)
8-8: LGTM!Also applies to: 30-32, 188-193, 284-284, 380-380, 446-446, 501-501, 520-520, 537-537, 612-617, 635-635, 661-663, 687-697, 720-724, 760-774, 790-790, 813-819, 835-837
docs/src/content/docs/architecture.md (1)
72-78: LGTM!Also applies to: 123-124, 192-196, 210-216
docs/src/content/docs/configuration.mdx (1)
56-56: LGTM!Also applies to: 115-115, 148-157, 171-171, 269-269
docs/src/content/docs/sdk/pipes.md (1)
51-53: LGTM!docs/src/content/docs/sdk/queries.md (1)
72-74: LGTM!docs/src/content/docs/sdk/reference.md (1)
63-63: LGTM!Also applies to: 73-81, 106-106
docs/src/content/docs/why-wavehouse.md (1)
53-53: LGTM!docs/src/content/docs/deployment.md (1)
121-121: LGTM!Also applies to: 149-149, 255-255, 359-363
docs/src/content/docs/development.md (1)
122-128: LGTM!Also applies to: 214-216, 225-228
docs/src/content/docs/getting-started.md (1)
50-50: LGTM!Also applies to: 76-76, 98-98
docs/src/content/docs/ingest-pipeline.md (2)
27-28: LGTM!
23-25: 🗄️ Data Integrity & IntegrationThe event-envelope documentation is accurate.
EventMessagedefines the documented fields. The ingest handler publishes an emptyscope. The worker parsesscopefor cache invalidation.docs/src/content/docs/pipes.mdx (1)
124-137: LGTM!Also applies to: 205-205, 221-221
docs/src/content/docs/reverse-proxy.mdx (1)
21-21: LGTM!Also applies to: 95-98, 200-203, 236-237, 271-271
docs/src/content/docs/sdk/admin.md (1)
12-20: LGTM!docs/src/content/docs/sdk/index.mdx (1)
52-63: LGTM!Also applies to: 67-77, 126-128
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: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e3f0b436-9cff-4bda-b6e2-73e72e9a0bfb
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdclients/ts/README.mddocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/sdk/queries.mddocs/src/content/docs/sdk/reference.mdinternal/api/router_test.go
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.
📜 Review details
⚠️ CI failures not shown inline (9)
GitHub Actions: CI / 0_CI.txt: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run jq -r 'to_entries[] | "\(.value.result)\t\(.key)"' <<<"$NEEDS"
�[36;1mjq -r 'to_entries[] | "\(.value.result)\t\(.key)"' <<<"$NEEDS"�[0m
�[36;1mbad="$(jq -r '[to_entries[] | select(.value.result == "failure" or .value.result == "cancelled") | .key] | join(", ")' <<<"$NEEDS")"�[0m
�[36;1mif [ -n "$bad" ]; then�[0m
�[36;1m echo "::error::Jobs failed or were cancelled: $bad"�[0m
GitHub Actions: CI / Docs preview: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run log="$RUNNER_TEMP/wrangler-upload.log"
�[36;1mlog="$RUNNER_TEMP/wrangler-upload.log"�[0m
�[36;1m# Retry absorbs transient CF API blips; a persistent failure falls�[0m
�[36;1m# through to exit 1 and reds the PR.�[0m
�[36;1mn=0�[0m
�[36;1muntil pnpm exec wrangler versions upload 2>&1 | tee "$log"; do�[0m
�[36;1m n=$((n + 1))�[0m
�[36;1m if [ "$n" -ge 3 ]; then�[0m
�[36;1m echo "::error::wrangler versions upload failed after $n attempts — see log above."�[0m
GitHub Actions: CI / CI: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run jq -r 'to_entries[] | "\(.value.result)\t\(.key)"' <<<"$NEEDS"
�[36;1mjq -r 'to_entries[] | "\(.value.result)\t\(.key)"' <<<"$NEEDS"�[0m
�[36;1mbad="$(jq -r '[to_entries[] | select(.value.result == "failure" or .value.result == "cancelled") | .key] | join(", ")' <<<"$NEEDS")"�[0m
�[36;1mif [ -n "$bad" ]; then�[0m
�[36;1m echo "::error::Jobs failed or were cancelled: $bad"�[0m
GitHub Actions: CI / Coverage: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run scripts/ci/wait-artifact.sh --artifacts coverage-unit,coverage-integration,coverage-e2e --producers "Unit tests,Integration tests,E2E tests" --timeout 1800
�[36;1mscripts/ci/wait-artifact.sh --artifacts coverage-unit,coverage-integration,coverage-e2e --producers "Unit tests,Integration tests,E2E tests" --timeout 1800�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
GH_***REDACTED_SECRET_ASSIGNMENT***
##[endgroup]
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
##[error]Producer job(s) concluded without producing: Unit tests (failure) — cannot wait for: coverage-unit,coverage-integration,coverage-e2e
GitHub Actions: CI / Coverage: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mexport GH_HOST="${GITHUB_SERVER_URL#*://}"�[0m
�[36;1m�[0m
�[36;1mif [ "$GITHUB_EVENT_NAME" = "merge_group" ]; then�[0m
�[36;1m echo "::warning::Skipping coverage upload for merge queue. Configure your workflow to upload coverage for PRs and the default branch instead. To avoid spinning up a runner, add \"if: github.event_name != 'merge_group'\" to the upload job."�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [ "Wave-RF/WaveHouse" != "" ] && \�[0m
�[36;1m [ "Wave-RF/WaveHouse" != "$GITHUB_REPOSITORY" ]; then�[0m
�[36;1m echo "::notice::Skipping coverage upload for fork PR (from Wave-RF/WaveHouse)"�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [ "$GITHUB_EVENT_NAME" = "pull_request" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_target" ]; then�[0m
�[36;1m COMMIT_OID="e84abfef255f39011ed5edeb298a6b2a55c6d63a"�[0m
�[36;1m REF=""�[0m
�[36;1m PR_NUMBER="479"�[0m
�[36;1melse�[0m
�[36;1m COMMIT_OID="8f8fa8c7c2d1c7aa39730c010bdd0d408d13631f"�[0m
�[36;1m REF="refs/pull/479/merge"�[0m
�[36;1m PR_NUMBER=$(gh pr list \�[0m
�[36;1m --repo "$GITHUB_REPOSITORY" \�[0m
�[36;1m --head "479/merge" \�[0m
�[36;1m --state open \�[0m
�[36;1m --json number \�[0m
�[36;1m --jq '.[0].number // empty' 2>/dev/null || true)�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mCOMMIT_OID="$COMMIT_OID" \�[0m
�[36;1mREF="$REF" \�[0m
�[36;1mPR_NUMBER="$PR_NUMBER" \�[0m
�[36;1mpython3 "$GITHUB_ACTION_PATH/upload_coverage.py"�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
GH_***REDACTED_SECRET_ASSIGNMENT***
INPUT_FILE: tmp/coverage/go-coverage.xml
INPUT_LANGUAGE: go
INPUT_LABEL:
FAIL_ON_ERROR: true
WAIT_FOR_PROCESSING_TIMEOUT: 160
GITHUB_EVENT_NAME: pull_request
GITHUB_REPOSITORY: Wave-RF/WaveHouse
GITHUB_API_URL: https://api.github.com
##[endgrou...
GitHub Actions: CI / Unit tests: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run make test-unit test-ts COV_DEFER=1
�[36;1mmake test-unit test-ts COV_DEFER=1�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
�[36m==> Running Unit Tests...�[0m
�[32m✓�[0m internal/auth (1.048s) (coverage: 98.9% of statements)
�[32m✓�[0m internal/api (2.166s) (coverage: 86.7% of statements)
�[32m✓�[0m internal/cache (1.075s) (coverage: 96.2% of statements)
�[32m✓�[0m internal/chsql (1.01s) (coverage: 100.0% of statements)
�[32m✓�[0m internal/config (1.018s) (coverage: 95.0% of statements)
�[32m✓�[0m internal/discovery (1.104s) (coverage: 95.0% of statements)
�[32m✓�[0m internal/dedupe (1.371s) (coverage: 86.4% of statements)
�[32m✓�[0m internal/mq (1.772s) (coverage: 74.7% of statements)
�[32m✓�[0m internal/pipes (1.266s) (coverage: 85.3% of statements)
�[32m✓�[0m internal/policy (1.125s) (coverage: 93.5% of statements)
�[32m✓�[0m internal/query (1.031s) (coverage: 95.5% of statements)
=== RUN TestDispatchLoop_PerTableBatching_NoCrossTableContamination
=== PAUSE TestDispatchLoop_PerTableBatching_NoCrossTableContamination
=== CONT TestDispatchLoop_PerTableBatching_NoCrossTableContamination
worker_test.go:1080:
Error Trace: /home/runner/work/WaveHouse/WaveHouse/internal/ingest/worker_test.go:1080
Error: Condition never satisfied
Test: TestDispatchLoop_PerTableBatching_NoCrossTableContamination
Messages: table B should flush 100 rows within 2s of being published (table A's prior events must not strand B rows in a batch that waits for the maxWait timer)
--- FAIL: TestDispatchLoop_PerTableBatching_NoCrossTableContamination (4.32s)
�[31m✖�[0m internal/ingest (4.355s) (coverage: 95.0% of statements)
�[33m∅�[0m internal/testutil (288ms) (coverage: 0.0% of statements)
�[32m✓�[0m internal/stream (1.186s) (coverage: 97.9% of statements)
�[32m✓�[0m internal/observability (5.034s) (coverage: 79.2% of state...
GitHub Actions: CI / 7_Coverage.txt: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run scripts/ci/wait-artifact.sh --artifacts coverage-unit,coverage-integration,coverage-e2e --producers "Unit tests,Integration tests,E2E tests" --timeout 1800
�[36;1mscripts/ci/wait-artifact.sh --artifacts coverage-unit,coverage-integration,coverage-e2e --producers "Unit tests,Integration tests,E2E tests" --timeout 1800�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
GH_***REDACTED_SECRET_ASSIGNMENT***
##[endgroup]
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
Waiting for coverage-unit,coverage-integration,coverage-e2e (have: []) — retrying in 5s.
##[error]Producer job(s) concluded without producing: Unit tests (failure) — cannot wait for: coverage-unit,coverage-integration,coverage-e2e
GitHub Actions: CI / 3_Docs preview.txt: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run log="$RUNNER_TEMP/wrangler-upload.log"
�[36;1mlog="$RUNNER_TEMP/wrangler-upload.log"�[0m
�[36;1m# Retry absorbs transient CF API blips; a persistent failure falls�[0m
�[36;1m# through to exit 1 and reds the PR.�[0m
�[36;1mn=0�[0m
�[36;1muntil pnpm exec wrangler versions upload 2>&1 | tee "$log"; do�[0m
�[36;1m n=$((n + 1))�[0m
�[36;1m if [ "$n" -ge 3 ]; then�[0m
�[36;1m echo "::error::wrangler versions upload failed after $n attempts — see log above."�[0m
GitHub Actions: CI / 8_Unit tests.txt: refactor(api)!: merge all admin-gated endpoints under /v1/ops
Conclusion: failure
##[group]Run make test-unit test-ts COV_DEFER=1
�[36;1mmake test-unit test-ts COV_DEFER=1�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
�[36m==> Running Unit Tests...�[0m
�[32m✓�[0m internal/auth (1.048s) (coverage: 98.9% of statements)
�[32m✓�[0m internal/api (2.166s) (coverage: 86.7% of statements)
�[32m✓�[0m internal/cache (1.075s) (coverage: 96.2% of statements)
�[32m✓�[0m internal/chsql (1.01s) (coverage: 100.0% of statements)
�[32m✓�[0m internal/config (1.018s) (coverage: 95.0% of statements)
�[32m✓�[0m internal/discovery (1.104s) (coverage: 95.0% of statements)
�[32m✓�[0m internal/dedupe (1.371s) (coverage: 86.4% of statements)
�[32m✓�[0m internal/mq (1.772s) (coverage: 74.7% of statements)
�[32m✓�[0m internal/pipes (1.266s) (coverage: 85.3% of statements)
�[32m✓�[0m internal/policy (1.125s) (coverage: 93.5% of statements)
�[32m✓�[0m internal/query (1.031s) (coverage: 95.5% of statements)
=== RUN TestDispatchLoop_PerTableBatching_NoCrossTableContamination
=== PAUSE TestDispatchLoop_PerTableBatching_NoCrossTableContamination
=== CONT TestDispatchLoop_PerTableBatching_NoCrossTableContamination
worker_test.go:1080:
Error Trace: /home/runner/work/WaveHouse/WaveHouse/internal/ingest/worker_test.go:1080
Error: Condition never satisfied
Test: TestDispatchLoop_PerTableBatching_NoCrossTableContamination
Messages: table B should flush 100 rows within 2s of being published (table A's prior events must not strand B rows in a batch that waits for the maxWait timer)
--- FAIL: TestDispatchLoop_PerTableBatching_NoCrossTableContamination (4.32s)
�[31m✖�[0m internal/ingest (4.355s) (coverage: 95.0% of statements)
�[33m∅�[0m internal/testutil (288ms) (coverage: 0.0% of statements)
�[32m✓�[0m internal/stream (1.186s) (coverage: 97.9% of statements)
�[32m✓�[0m internal/observability (5.034s) (coverage: 79.2% of state...
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: - No global state: Dependencies are passed explicitly (constructor injection).
- Comment the why, not the what. Add a comment only when the reason isn't obvious from the code; a line that matches the surrounding pattern needs none. Keep comments to 1–2 lines and match the file's existing density.
Files:
internal/api/router_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.
- Shared mocks in
internal/testutil/: UseMockPublisher,MockCache,MockDeduplicator,MockSubscriberinstead of creating ad-hoc mocks. Seetestutil/mocks.go.- Every new function should have corresponding test cases. Run
make lintandmake testbefore considering work complete.
Files:
internal/api/router_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/router_test.go
🧠 Learnings (7)
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/sdk/queries.mdREADME.mddocs/src/content/docs/architecture.mdclients/ts/README.mddocs/src/content/docs/deployment.mddocs/src/content/docs/sdk/reference.mddocs/src/content/docs/api.md
📚 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/reverse-proxy.mdx
📚 Learning: 2026-05-20T01:02:00.784Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:00.784Z
Learning: In WaveHouse’s internal API tests (files matching internal/api/**/*_test.go), follow the existing separation-of-concerns convention for testing the RequireRole middleware: inject `ContextKeyRole` directly into the request `context.Context` instead of using `testutil.MakeJWT`/JWT-driven flows. Do not refactor role-gate tests to use JWT tokens—JWT parsing and token handling are covered separately in `middleware_test.go` (the dedicated JWT parsing tests), and mixing those concerns would expand the failure surface and reduce isolation.
Applied to files:
internal/api/router_test.go
📚 Learning: 2026-05-23T01:23:59.268Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 174
File: internal/api/ingest_test.go:111-111
Timestamp: 2026-05-23T01:23:59.268Z
Learning: In WaveHouse Go tests in internal/api/**/*_test.go, use internal/testutil.AssertJSONErrorResponse(t, w) for HTTP error-path JSON assertions. Do not use (or reintroduce) package-local assertJSONErrorResponse helpers. AssertJSONErrorResponse verifies the response Content-Type is application/json, includes the X-Content-Type-Options: nosniff header, and that the JSON body contains an "error" field.
Applied to files:
internal/api/router_test.go
📚 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/api/router_test.go
📚 Learning: 2026-07-07T12:38:12.052Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:12.052Z
Learning: In this repo, do not add or recommend logging/tracing client IP addresses using naive or untrusted sources (e.g., `r.RemoteAddr` or directly trusting/deriving `X-Forwarded-For`) anywhere in the Go codebase. `middleware.RealIP` was removed due to IP-spoofing risks, and proper trusted-proxy-aware client-IP handling is intentionally deferred to issue `#333`. During code review, if proposed changes would record client IPs (including in audit paths such as `internal/auth/auth.go`), reject/redirect until `#333` lands with correct trusted-proxy configuration and safeguards.
Applied to files:
internal/api/router_test.go
📚 Learning: 2026-08-11T21:55:53.726Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/stream_test.go:159-176
Timestamp: 2026-08-11T21:55:53.726Z
Learning: For Go files in this repository, do not report direct type assertions solely because the `forcetypeassert` rule is commented out in `.golangci.yml`. Only flag a type assertion when there is an independent correctness, safety, or maintainability issue.
Applied to files:
internal/api/router_test.go
🔇 Additional comments (13)
README.md (2)
81-81: LGTM!
105-105: LGTM!internal/api/router_test.go (2)
334-341: Extend the 404 regression matrix to cover the remaining moved admin paths.The table checks
/v1/admin/query, but it does not check/v1/admin/policyor/v1/admin/pipes. The route migration removes all old admin aliases. Add representative requests for each remaining moved path and asserthttp.StatusNotFound.This is the same coverage gap identified in the previous review; the current change covers only schema, DLQ, and query.
290-305: LGTM!Also applies to: 312-333, 346-353
clients/ts/README.md (1)
13-14: LGTM!Also applies to: 35-35, 47-47, 155-155
docs/src/content/docs/api.md (2)
813-819: 🗄️ Data Integrity & IntegrationVerify the
scopeprojection at the wire boundary.The documentation now promises that NATS messages may contain optional
scope, while SSE messages omit it. Confirm that the ingest consumers tolerate the optional field and that SSE serialization removes it before delivery.
8-8: LGTM!Also applies to: 30-32, 188-193, 284-284, 380-380, 446-446, 501-501, 520-520, 537-537, 612-617, 635-635, 661-663, 687-697, 720-724, 760-774, 790-790, 835-837
docs/src/content/docs/architecture.md (1)
72-78: LGTM!Also applies to: 123-124, 192-196, 214-220
docs/src/content/docs/deployment.md (2)
84-85: 📐 Maintainability & Code QualityVerify the image-tag publication claim.
This text makes a time-sensitive claim about published tags and 30-day retention. Confirm that
:devand:dev-<full-commit-sha>are published and that:devis valid for the Compose example before relying on these instructions.Also applies to: 311-311
123-123: LGTM!Also applies to: 151-151, 257-257, 361-365
docs/src/content/docs/reverse-proxy.mdx (1)
21-21: LGTM!Also applies to: 95-98, 200-202, 204-207, 240-241, 275-275
docs/src/content/docs/sdk/queries.md (1)
72-76: LGTM!Also applies to: 262-262
docs/src/content/docs/sdk/reference.md (1)
63-63: LGTM!Also applies to: 73-81
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1af02df9-8a9f-4e9d-a4fd-f2d007ccf28d
📒 Files selected for processing (17)
CHANGELOG.mdREADME.mdSECURITY.mdclients/ts/README.mdclients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.tsdocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/deployment.mddocs/src/content/docs/pipes.mdxdocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/queries.mddocs/src/content/docs/sdk/reference.mdinternal/api/query.gointernal/api/query_test.gointernal/api/router_test.gointernal/ingest/worker_test.go
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Structured logging withlog/slog(JSON handler)
Chi v5 for HTTP routing
Error handling: Return errors, don't panic. Wrap withfmt.Errorf("context: %w", err).
No global state: Dependencies are passed explicitly (constructor injection).
Package naming: Lowercase, single word (or abbreviated).internal/enforces module privacy.
Column-level access control is a hard cap on every read path. A role'sallow_columns/deny_columnsis enforced against every column a structured query references (projection, aggregations,filters,group_by,order_by,time_range) insidequery.Build, and aselect_allrequest expands to the role's allowed columns rather thanSELECT *
Files:
internal/ingest/worker_test.gointernal/api/query_test.gointernal/api/query.gointernal/api/router_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.
Shared mocks ininternal/testutil/: UseMockPublisher,MockCache,MockDeduplicator,MockSubscriberinstead of creating ad-hoc mocks. Seetestutil/mocks.go.
Every new function should have corresponding test cases. Runmake lintandmake testbefore considering work complete.
Files:
internal/ingest/worker_test.gointernal/api/query_test.gointernal/api/router_test.go
**/*.{md,mdx}
📄 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:
docs/src/content/docs/sdk/admin.mdSECURITY.mddocs/src/content/docs/sdk/reference.mddocs/src/content/docs/sdk/queries.mddocs/src/content/docs/pipes.mdxREADME.mddocs/src/content/docs/architecture.mddocs/src/content/docs/deployment.mddocs/src/content/docs/api.mdclients/ts/README.mdCHANGELOG.md
🧠 Learnings (9)
📚 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/ingest/worker_test.gointernal/api/query_test.gointernal/api/router_test.go
📚 Learning: 2026-07-07T12:38:12.052Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:12.052Z
Learning: In this repo, do not add or recommend logging/tracing client IP addresses using naive or untrusted sources (e.g., `r.RemoteAddr` or directly trusting/deriving `X-Forwarded-For`) anywhere in the Go codebase. `middleware.RealIP` was removed due to IP-spoofing risks, and proper trusted-proxy-aware client-IP handling is intentionally deferred to issue `#333`. During code review, if proposed changes would record client IPs (including in audit paths such as `internal/auth/auth.go`), reject/redirect until `#333` lands with correct trusted-proxy configuration and safeguards.
Applied to files:
internal/ingest/worker_test.gointernal/api/query_test.gointernal/api/query.gointernal/api/router_test.go
📚 Learning: 2026-08-11T21:55:53.726Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/stream_test.go:159-176
Timestamp: 2026-08-11T21:55:53.726Z
Learning: For Go files in this repository, do not report direct type assertions solely because the `forcetypeassert` rule is commented out in `.golangci.yml`. Only flag a type assertion when there is an independent correctness, safety, or maintainability issue.
Applied to files:
internal/ingest/worker_test.gointernal/api/query_test.gointernal/api/query.gointernal/api/router_test.go
📚 Learning: 2026-08-11T15:22:20.507Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.
Applied to files:
clients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.ts
📚 Learning: 2026-08-12T21:45:38.018Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, use the exported `PipeRequestOptions` type for `PipeRef.fetch` and shared pipe, table, or query-builder fetch options rather than `Pick<RequestOptions, "signal">` or `RequestOptions`. `PipeRequestOptions` declares `limit?: never`, ensuring object literals and named `RequestOptions` values containing `limit` fail type checking instead of silently dropping it. When only a signal is needed, use `PipeRequestOptions` or an inferred `{ signal }` object; do not expect a `RequestOptions` value to be assignable to `PipeRequestOptions`.
Applied to files:
clients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.ts
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/sdk/admin.mdSECURITY.mddocs/src/content/docs/sdk/reference.mddocs/src/content/docs/sdk/queries.mdREADME.mddocs/src/content/docs/architecture.mddocs/src/content/docs/deployment.mddocs/src/content/docs/api.mdclients/ts/README.mdCHANGELOG.md
📚 Learning: 2026-05-20T01:02:00.784Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:00.784Z
Learning: In WaveHouse’s internal API tests (files matching internal/api/**/*_test.go), follow the existing separation-of-concerns convention for testing the RequireRole middleware: inject `ContextKeyRole` directly into the request `context.Context` instead of using `testutil.MakeJWT`/JWT-driven flows. Do not refactor role-gate tests to use JWT tokens—JWT parsing and token handling are covered separately in `middleware_test.go` (the dedicated JWT parsing tests), and mixing those concerns would expand the failure surface and reduce isolation.
Applied to files:
internal/api/query_test.gointernal/api/router_test.go
📚 Learning: 2026-05-23T01:23:59.268Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 174
File: internal/api/ingest_test.go:111-111
Timestamp: 2026-05-23T01:23:59.268Z
Learning: In WaveHouse Go tests in internal/api/**/*_test.go, use internal/testutil.AssertJSONErrorResponse(t, w) for HTTP error-path JSON assertions. Do not use (or reintroduce) package-local assertJSONErrorResponse helpers. AssertJSONErrorResponse verifies the response Content-Type is application/json, includes the X-Content-Type-Options: nosniff header, and that the JSON body contains an "error" field.
Applied to files:
internal/api/query_test.gointernal/api/router_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/pipes.mdx
🪛 LanguageTool
SECURITY.md
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...tions — verify the container image with gh attestation verify oci://ghcr.io/wave-rf/wavehouse:<tag> --repo Wave-RF/WaveHouse --signer-workflow Wave-RF/WaveHouse/.github/workflows/release.yml (the rolling `:d...
(GITHUB)
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ... downloaded release-binary archive with gh attestation verify <file> --repo Wave-RF/WaveHouse --signer-workflow Wave-RF/WaveHouse/.github/workflows/release.yml, and the `@waveh...
(GITHUB)
docs/src/content/docs/deployment.md
[uncategorized] ~102-~102: The official name of this software platform is spelled with a capital “H”.
Context: ...tion — verify a downloaded archive with gh attestation verify <file> --repo Wave-RF/WaveHouse --signer-workflow Wave-RF/WaveHouse/.github/workflows/release.yml. (This covers th...
(GITHUB)
CHANGELOG.md
[typographical] ~28-~28: In American English, use a period after an abbreviation.
Context: ...ads 401-for-a-present-but-invalid-token vs 403-for-a-non-admin-role everywhere it'...
(MISSING_PERIOD_AFTER_ABBREVIATION)
🔇 Additional comments (16)
CHANGELOG.md (1)
28-28: LGTM!Also applies to: 30-30, 32-32, 34-34, 36-36, 38-38, 40-40, 42-42, 44-44, 46-46, 68-68, 70-70, 475-475
clients/ts/src/cli/codegen.ts (1)
14-15: LGTM!Also applies to: 191-215, 233-238, 250-271
clients/ts/README.md (1)
13-13: LGTM!Also applies to: 35-35, 47-47, 149-155
internal/api/router_test.go (1)
12-17: LGTM!Also applies to: 290-305, 312-343, 348-355, 423-423, 456-456, 516-526, 671-673, 699-699, 710-713
internal/api/query_test.go (1)
38-38: LGTM!Also applies to: 58-58, 112-112, 479-479
internal/api/query.go (1)
15-17: LGTM!Also applies to: 163-163
README.md (1)
81-81: LGTM!Also applies to: 95-109
docs/src/content/docs/api.md (1)
8-8: LGTM!Also applies to: 30-32, 188-193, 284-284, 380-380, 395-398, 446-446, 501-501, 520-520, 537-537, 604-617, 635-635, 661-663, 687-697, 720-724, 760-774, 790-790, 813-819, 835-837
docs/src/content/docs/architecture.md (1)
72-78: LGTM!Also applies to: 123-124, 192-196, 210-224
internal/ingest/worker_test.go (1)
999-1004: LGTM!Also applies to: 1082-1092
SECURITY.md (1)
31-31: LGTM!docs/src/content/docs/deployment.md (1)
84-102: LGTM!Also applies to: 131-131, 159-159, 265-265, 319-319, 369-373
docs/src/content/docs/pipes.mdx (1)
122-137: LGTM!Also applies to: 205-205, 221-221
docs/src/content/docs/sdk/admin.md (1)
12-20: LGTM!docs/src/content/docs/sdk/queries.md (1)
262-262: LGTM!docs/src/content/docs/sdk/reference.md (1)
63-63: LGTM!Also applies to: 73-81, 97-106
github-project-automation
Bot
moved this from Ready
to In review
in WaveHouse Task Board
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4a4b38b4-48cc-46c1-97f9-8c44f4c41e1d
📒 Files selected for processing (6)
CHANGELOG.mdclients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.tsdocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/queries.md
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Docs preview
- GitHub Check: E2E tests
- GitHub Check: Coverage
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{go,ts,tsx}
📄 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:
clients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.ts
🧠 Learnings (3)
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/queries.md
📚 Learning: 2026-08-11T15:22:20.507Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.
Applied to files:
clients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.ts
📚 Learning: 2026-08-12T21:45:38.018Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, use the exported `PipeRequestOptions` type for `PipeRef.fetch` and shared pipe, table, or query-builder fetch options rather than `Pick<RequestOptions, "signal">` or `RequestOptions`. `PipeRequestOptions` declares `limit?: never`, ensuring object literals and named `RequestOptions` values containing `limit` fail type checking instead of silently dropping it. When only a signal is needed, use `PipeRequestOptions` or an inferred `{ signal }` object; do not expect a `RequestOptions` value to be assignable to `PipeRequestOptions`.
Applied to files:
clients/ts/src/cli/codegen.test.tsclients/ts/src/cli/codegen.ts
🔇 Additional comments (5)
clients/ts/src/cli/codegen.ts (1)
164-198: LGTM!clients/ts/src/cli/codegen.test.ts (1)
65-73: LGTM!docs/src/content/docs/sdk/admin.md (1)
40-46: LGTM!Also applies to: 77-77
docs/src/content/docs/sdk/pipes.md (1)
50-54: LGTM!docs/src/content/docs/sdk/queries.md (1)
72-78: LGTM!Also applies to: 264-264
EricAndrechek
left a comment
EricAndrechek
left a comment
There was a problem hiding this comment.
Good, relatively small PR/changes – super minor tweak to router which required tweaks in the SDK and lots of test cases which are all fine. Docs changes also were required as a result of the change, but many (likely due to the long cycling docs review pre push claude hooks) overstepped and added more to the docs than we want, so basically the entirety of these requested changes are just stepping back what was made.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
28-28: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winWrite the abbreviation as
vs..Line 28 uses
vswithout the required period. Change401-for-a-present-but-invalid-token vs 403-for-a-non-admin-roleto401-for-a-present-but-invalid-token vs. 403-for-a-non-admin-role.Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bb1bc55b-eb3f-4099-8a15-0fdc31692f0d
📒 Files selected for processing (11)
CHANGELOG.mdclients/ts/README.mddocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/deployment.mddocs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/queries.mddocs/src/content/docs/sdk/reference.md
💤 Files with no reviewable changes (2)
- docs/src/content/docs/sdk/pipes.md
- docs/src/content/docs/reverse-proxy.mdx
Included review availability: Your plan includes up to 1 review per rolling 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
🧠 Learnings (1)
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/sdk/queries.mddocs/src/content/docs/sdk/admin.mddocs/src/content/docs/sdk/reference.mdclients/ts/README.mddocs/src/content/docs/deployment.mddocs/src/content/docs/architecture.mddocs/src/content/docs/api.mdCHANGELOG.md
🪛 LanguageTool
docs/src/content/docs/architecture.md
[typographical] ~123-~123: Consider using an em dash in dialogues and enumerations.
Context: - worker.go — StartIngestWorker lau...
(DASH_RULE)
CHANGELOG.md
[typographical] ~28-~28: In American English, use a period after an abbreviation.
Context: ...ads 401-for-a-present-but-invalid-token vs 403-for-a-non-admin-role everywhere it'...
(MISSING_PERIOD_AFTER_ABBREVIATION)
🔇 Additional comments (9)
clients/ts/README.md (1)
33-33: LGTM!Also applies to: 45-45, 147-153
docs/src/content/docs/sdk/reference.md (1)
97-100: LGTM!Also applies to: 106-106
CHANGELOG.md (1)
30-30: LGTM!Also applies to: 32-32, 34-34, 36-36, 38-38, 40-40, 42-42, 44-44, 46-46
docs/src/content/docs/api.md (1)
8-8: LGTM!Also applies to: 30-32, 188-193, 284-284, 380-380, 395-398, 446-446, 501-501, 520-520, 537-537, 604-612, 631-631, 657-659, 683-693, 716-720, 756-770, 786-786, 814-814, 830-830, 832-832
docs/src/content/docs/architecture.md (1)
72-78: LGTM!Also applies to: 123-123, 194-195, 209-223
docs/src/content/docs/deployment.md (1)
84-102: LGTM!Also applies to: 131-131, 159-159, 265-265, 369-373
docs/src/content/docs/ingest-pipeline.md (1)
25-26: LGTM!docs/src/content/docs/sdk/admin.md (1)
6-12: LGTM!Also applies to: 32-38, 69-69
docs/src/content/docs/sdk/queries.md (1)
72-76: LGTM!Also applies to: 262-262
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
github-project-automation
Bot
moved this from In review
to Done
in WaveHouse Task Board
#490 landed a better fix for the Dependabot composite-action gap than mine, so main's version wins on every shared file: - .github/dependabot.yml: taken wholesale. My two `updates:` entries are two independent Dependabot jobs and a group is scoped to its own job, so they would emit two actions-deps PRs every Monday -- the noise the group comment exists to prevent. main's `directories: [/, ...]` is one job, one PR. main's comment also carries the accuracy fix: actions/cache was uniformly v5.0.5 at every site, a major behind upstream rather than behind a caller. - .github/actions/setup-env/action.yml: taken wholesale. My bumps were a strict subset and stale (cache still v5.0.5, pnpm 11.1.3). - development.md Dependabot section: taken wholesale. My "four update configs" auto-merged silently and is wrong under the directories form -- it is three. main's also documents the typescript major hold (#487). - My CHANGELOG entry describing the two-entry mechanism is dropped; #490's entry on main is the accurate record of the same fix. Kept mine, reconciled by hand: - README's `--signer-workflow` fix -- #490 deliberately avoided it. - persist-credentials: false across the four release workflows. - clients/ts/README.md and sdk/index.mdx: #470 changed streaming from EventSource to fetch. Took its wording, kept my `latest`-is-a-dev- snapshot caveat and the corrected anchor -- main still links #releasing-the-sdk, a section this branch renamed. - CHANGELOG resolved with the same script as the #479 merge: main's entry text into this branch's structure. Verified 331 main entries + 29 branch entries, none lost, none invented, no duplicates. All action pins now match main exactly; no stale pnpm strings remain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015bVwHtNakQgmnBhMcfW8pe
The README's `gh attestation verify` was the last place still teaching `--repo` alone -- #479 added `--signer-workflow` to SECURITY.md and deployment.md, and 1b2dfc7 fixed the README itself but only reworded the release.yml entry, so the fix shipped without its own changelog line. Kept out of the merge commit deliberately: that commit's message claims the CHANGELOG resolution invented nothing, and hand-writing an entry into it would have made the claim false -- the same overclaim f29dd54 exists to correct, one commit earlier on this branch. The merge now audits as a pure resolution and this rides on top. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015bVwHtNakQgmnBhMcfW8pe
Three catch-up changes, all client-side. The server is untouched. Routes: main merged every admin-gated endpoint under /v1/ops (#479) with no aliases, so thirteen call sites were 404ing against a current server — schema list/refresh, DLQ stats, raw SQL, policy get/put/validate, and pipes CRUD, plus the codegen CLI's schema fetch. Rewrote them along with the tests, the shared wire_cases.json fixture, and the Go SDK docs. The fixture is replayed by both conformance runners, so the stale paths broke the TypeScript half too; `make test-conformance-ts` is back to 45/45. ClientOptions.Headers: the TypeScript SDK gained options.headers in #456 and Go had no equivalent. Headers now apply to every request the client makes, REST and SSE alike — which is also how an operator sends the server's non-JWT X-Operator-Key. The SDK's own headers are set afterwards and win a collision; net/http canonicalizes names, so matching is case-insensitive; the map is copied at construction so later mutation can't reach into requests. SSE robustness, mirroring main's fetch-based rewrite (#470). The Go SDK already authenticated by header, so that part was never stale, but three gaps were: - A credentialed stream followed redirects. net/http drops Authorization on a cross-host hop while forwarding custom headers verbatim, so a redirect either downgraded the stream to default_role in silence or handed configured secrets to wherever it pointed. Now refused with a terminal SSE_REDIRECT. Uncredentialed streams still follow. - A 200 with any content type was treated as an event stream, so an auth gateway's login page left the stream sitting in StatusLive delivering nothing. Now a terminal SSE_BAD_CONTENT_TYPE. - Every failure collapsed into one retryable SSE_ERROR, and malformed frames came back as a bare fmt.Errorf, so errors.As and IsRetryable didn't work on them. Replaced with the taxonomy the TypeScript SDK uses — SSE_AUTH_ERROR, SSE_NETWORK_ERROR, SSE_CONNECT_ERROR, SSE_REDIRECT, SSE_BAD_CONTENT_TYPE, SSE_PARSE_ERROR, SSE_READ_ERROR — each with its own retryable flag, all delivered as *Error. Also documents what main changed underneath the Go SDK without changing its code: DateTime values arrive canonicalized to RFC 3339 UTC (#402), SSE applies policy row-filters per subscriber and fails closed (#381, #457), /v1/stream is ungated so WaveHouse never 401s a stream, and /v1/ops/dlq/stats is absent (404) when the DLQ is disabled rather than returning empty stats. Tests: terminal-failure table (bad content type, missing content type, credentialed redirect, non-HTTP scheme), redirect-followed-when- uncredentialed, typed retryable parse errors, and header precedence and copying on both transports.
2 participants
Summary
BREAKING: every admin-gated endpoint now lives under a single
/v1/ops/*prefix, behind one tree-levelRequireAdmingate. Previously the admin surface was split between the/v1/admin/*subtree (raw SQL, policy CRUD, pipe CRUD) and three individually-gated top-level routes./v1/admin/{query,policy,pipes...}→/v1/ops/{query,policy,pipes...}/v1/schemaand/v1/schema/refresh→/v1/ops/schema[/refresh]/v1/dlq/stats→/v1/ops/dlq/statsNo aliases kept (pre-1.0); old paths return
404. Gate semantics unchanged (policy.admin_role, operator-key break-glass included). SDK path constants updated in lockstep — its method surface is unaffected, but an older SDK 404s against a renamed server. Docs swept throughout, including a new "Fencing the admin surface" reverse-proxy section (one proxy rule now covers the whole management surface).