feat(mq): give every tenant a queue of its own

[taitelee]

Summary

Story 5b of the multi-tenant epic: every tenant gets a message queue of its own inside the embedded NATS server, and the internal/mq interfaces speak per tenant, never per stream, so nothing outside the package assumes that layout (an external implementation, where the Kubernetes operator owns stream config, can keep one shared stream). A settings directory that holds the four files sees no change beyond the stream names: tenant 0's own window and budget, as before.

• A stream pair per tenant. INGEST_<tenant> (ingest.<tenant>.>, DiscardNew) at the tenant's own mq.max_bytes_gb, and DLQ_<tenant> (dlq.<tenant>.>, DiscardOld) at a tenth of it. The prefixes differ in their first letter, so no tenant id makes one kind's name the other's (pinned by TestStreamNames_NeverCollide). Subjects are unchanged.
• Opened with the first budget, kept on removal. SetMaxBytes(ctx, tenant, bytes) opens a tenant's pair the first time (dead-letter stream first, so no row is queued that could not be parked) and resizes it after; the wiring hands every served tenant's budget at boot and after every reload, replacing the tenant-0-only onDefaultAdopt hook and its boot warning — and with it the tracking of tenant 0's last adopted store (defaultSetting), whose one reader left, a flat directory's ops gate, now reads tenant 0 through the registry (defaultPolicy). A publish or park that finds a stream missing reopens it at the budget last asked for, and so does a publish to a queue the broker has not recorded open: an open that timed out can leave a stream JetStream creates after all, which no consumer would hold. The publishes and parks that find a queue not open share one attempt, and after one fails the tenant's publishes and parks are refused at once for five seconds — so a queue that cannot open never holds the broker's lock for every retrying client, which every other tenant's reload would wait on — while a reload retries regardless; with none asked yet (the instant between a reload adopting a new tenant and its budget arriving) the publish is refused as ErrQueueFull, a 503. A removed or rejected tenant keeps its pair at the last budget, and boot takes stock of the pairs on disk, so its queued rows are still delivered and parked on its own dead-letter queue.
• Isolation. The ingest worker's and the hub bridge's durables are held on every tenant's stream, those opened later included; each tenant has its own ack floor, its own MaxAckPending, and a delivery goroutine of its own, so one tenant's backlog or stuck handler holds back no other. A publish that reopens a missing queue does it detached from the request's cancellation, and the consumers join on a budget of their own, so a client that goes away mid-open cannot leave a queue no consumer holds — which would fail the worker and stop the process. The worker's prefetch, and the hub bridge's fetch-ahead (the client default of 500), are each shared across the tenants' streams (at least one each) rather than multiplied by them. The durables are looked up before anything is written, so a boot over many queues writes nothing it need not (measured: 0.6 s at 1,000 tenants, against 15 s rewriting every stream and durable).
• Sweeper. PurgeAcked takes each tenant's cutoff; the sweeper hands it every tenant's own stream.gap_window_minutes (gapWindows replaces longestGapWindow). A rejected tenant keeps the window its folder last had — a rejection is the common reload failure, and its clients resume from Last-Event-ID once the folder is fixed (settings.Registry.Known now yields each tenant's last adopted store for this), and one rejected since boot, whose window this process never read, keeps all of its history until its folder validates — while a removed tenant's stream keeps no acknowledged history. Per-tenant purge lines are Debug, with one Info summary per sweep.
• GET /v1/ops/dlq/stats?tenant=, parsed strictly with opsTenant. Absent is tenant 0, the ops-read convention, and for a flat directory exactly the previous answer; no parameter no longer sums every tenant. The tenant is looked up in the MQ, not the settings, so a rejected or removed tenant's parked rows are read by name; a tenant with no dead-letter queue is a 404. The SDK's wh.dlq.list() and .table() take a tenant option, as the pipe reads did in feat(settings): nested settings directory with a per-tenant registry #598.
• Dead-letter shrink guard (mq(dlq): shrinking mq.max_bytes_gb silently deletes the oldest dead letters — investigate how the reload should treat a non-empty DLQ #532, interim). A reload that shrinks a budget never caps a dead-letter stream below the bytes it holds: it keeps what it has, and a warning names the tenant and both figures. mq(dlq): shrinking mq.max_bytes_gb silently deletes the oldest dead letters — investigate how the reload should treat a non-empty DLQ #532's ClickHouse-backed dead-letter table stays there.
• Disk accounting (decided: option A). JetStream counts every stream's cap as reserved disk and refuses a stream once they pass 75% of the free disk at boot; per tenant that refused the 569th tenant at the 1 GB minimum on an 834 GB disk, and would refuse the 12th at the seed's 50 GB. The embedded server's limit is set out of reach, so a budget is a cap and never a reservation; what the budgets add up to against the disk is mq: validate WH_MQ_MAX_BYTES_GB upper bound to prevent disk over-reservation #138's. The one flat difference beyond the stream names: a budget above three quarters of the free disk now boots where it was refused.
• Old data directories. WAVEHOUSE and WAVEHOUSE_DLQ claim ingest.> and dlq.>, which JetStream will not let overlap a new stream, so boot deletes them, logging what they held. What that made dead is gone: parseTopicKey's one-token subjects and deployment.md's "Upgrading across the tenant subject token". The v2-envelope upgrade runbook now says the old queue is deleted rather than carried over, so draining and replaying the parked rows has to happen before the upgrade, and the worker's comments and dead-letter hint no longer assume an undrained pre-v2 row can reach it.
• Sweep. Every "story 5b" and "until 5b", and every sentence this makes false, is updated: the mq interface comments, the sweeper, wire.go and app.go, app_test.go, the hub subscriber's one-goroutine comment, the settings comments, and deployment.md, settings-directory.mdx, api.md, architecture.md, ingest-pipeline.md (diagrams included), durability.md, configuration.mdx, why-wavehouse.md, the SDK admin and reference pages, AGENTS.md and the CHANGELOG.

Cost check

Measured in a scratch program against the same nats-server 2.14.6 and server options (fsync on every write), with both durables consuming on each tenant's stream, at 10 / 100 / 1,000 tenants, against the one shared pair:

• Resident memory, idle: 25 / 53 / 280–315 MB, against 21–35 MB shared — about 0.3 MB per tenant. Goroutines: 323 / 2,123 / 20,123, against 43.
• Under load, a tenant written in the last ~10 s holds a block buffer: at 1,000 tenants one write to each took the heap from 172 to 439 MB, back to 172 MB within 15 s. One open file per stream written, closed within about two minutes of quiet.
• Boot with data: 12 / 59 / 611 ms. A tenant's first creation costs about 20 ms (the pair, two durables, delivery), so a first boot of 1,000 new tenants pays about 20 s once.
• Publish throughput: 32 concurrent publishers 1,565 / 7,155 / 2,390 msg/s against ~1,170 shared; one publisher round-robin across tenants 999 / 560 / 503 against ~1,100.
• One sweep: 31 ms / 296 ms / 2.75 s, once a minute.
• Not measured, but by construction: while ClickHouse stalls, the worker can hold up to maxAckPending (10,000) unacknowledged rows per tenant with traffic — about 10 million at 1,000 tenants — where the shared stream held 10,000 for the whole process (see Follow-ups).

An upstream quirk

When a stream's store fails to open, nats-server 2.14.6 releases a disk reservation it never made, so its reserved count goes negative. Harmless under the default limit, but with the limit at math.MaxInt64 the subtraction overflows and every later stream is refused until restart. The limit is half the int64 range instead, and TestEmbeddedNATS_SetMaxBytes_AQueueThatCannotOpen pins it: one tenant's failed open, then the next tenant's queue opens.

Test plan

• make ci passes locally
• Pre-push reviewers (round 1: both iterate — fixed: the reopen detach, the v2 upgrade runbook, the remaining sweeper and create stream lines, the 503 rows, the disk-full sizing rule, the per-tenant consume callbacks in the goroutine topology; round 2: both iterate — fixed: a test for the boot rule of a queue that cannot open, the boot pass honoring New's context, the last pre-v2 mentions, the resume hole after a rejected folder, the runbook's replay step the old build cannot carry out, the boot warning's count; round 3: code iterate on one SHOULD — boot now re-applies a tenant's budget to a pair it finds split, or missing its dead-letter stream, as every boot rewrote both streams before; docs iterate — the boot/reload contexts in architecture.md, the runbook's opening, the per-tenant consumer line, and a full disk needing a restart; round 4: code iterate — a failed resize's undo now restores the ingest stream's actual cap (after a boot that found a pair split it would have restored 0, which JetStream reads as no cap), and a test pins the report of a queue a consumer cannot join; docs iterate — both open-timeout errors and when a later boot writes, three garden-path sentences; round 5: code iterate — a rejected tenant keeps its replay history at its folder's last window instead of losing it at the next sweep, and all of it while rejected since boot; docs iterate — disk sizing counts every tenant ever served, since removed tenants' queues are kept; round 6: code iterate — the hub bridge's fetch-ahead is shared across the tenants like the worker's prefetch, instead of 500 per tenant; docs iterate — a stale "stop the sweeper" sentence from the shared stream, and a dead-letter stream is opened, not guaranteed to exist, when its tenant is first served; round 7: code iterate — a publish goes by the broker's record of an open queue rather than a stream answering, so a stream an open gave up on but JetStream created anyway is joined before rows land in it, and the queue-full and publish-failure log lines name the tenant; docs iterate — the disk-sizing guidance gets its own paragraph, and the leftover tenant-0 tracking clause goes, with the tracking itself; round 8: code ship it; docs iterate — what a consumer that cannot join a queue opened at runtime does (the worker exits the process, the hub logs and the tenant's streams get no live rows until a restart), and a failed purge lookup ends that tenant's purge, not the sweep; round 9: code iterate — a tenant whose queue cannot open no longer holds the broker's lock for every retrying publish (publishes share one attempt, and after one fails are refused at once for five seconds; a reload retries regardless), and the worker's per-tenant memory ceiling is recorded here, for epic(settings): multi-tenant settings directory and tenant-scoped runtime #583's deferred worker rework; docs iterate — two wording fixes; round 10: code iterate — tests for one tenant's failed purge stopping no other and for a durable reused across a restart; docs iterate — the SDK replay note names the upgrade that deletes the old queue, and each tenant's own gap window; round 11: code ship it; docs iterate — disk sizing counts a dead-letter stream the shrink guard kept above a tenth, and the stats route says where the tenant is looked up; round 12: docs ship it — the round-12 commit is docs prose only, so the code reviewer's round-11 ship it stands for it, recorded as a logged skip)
• CodeRabbit round 1 (changes requested, two findings, fixed): the sweeper no longer logs other tenants' purge failures at WARN beside one tenant's missing consumer, and a park that finds the dead-letter stream missing is paced like a publish
• CodeRabbit round 2: approved
• Flake fix after Eric's heads-up from the stacked epic(distributed): shared backends and standalone workers for multi-node deployments #613 builds (f5d8f484): TestEmbeddedNATS_PacesTheRetriesOfAQueueThatCannotOpen re-created its obstacle while nats-server's post-failure goroutine was removing the emptied streams and account directories (EINVAL on APFS, ENOENT on Linux; 3 of 25 runs here); the test and the one app test with the pattern now keep another tenant's streams in the directory. CodeRabbit round 3: approved. eb9f7409 closes the same goroutine's much narrower window in TestEmbeddedNATS_SetMaxBytes_AQueueThatCannotOpen with a non-stream directory under streams/ (reserves nothing, so the test still pins the reservation overflow); Eric's second measurement (9 of 10 on feat/mq-nats-wiring) was on a branch forked before f5d8f484 — verified by applying the fix there (5 of 20 → 0 of 20). CodeRabbit round 4: approved
• mq: a reopen outlives the caller's cancellation and the consumer still joins (fails without the detach); a tenant's pair opens with its first budget, names and caps as above, and no other tenant's; a publish reopens a missing pair at the last budget and is refused with none asked; a publish opens a queue whose open gave up on a stream JetStream created anyway, and its row reaches a running consumer (fails without the gate); after a failed attempt a publish or park is refused without waiting on the broker's lock, and a publish tries again once the window has passed (fails without the pacing); one tenant at its budget is refused while another publishes; a tenant at MaxAckPending and one with a stuck handler hold back no other, and a tenant's messages arrive in order; both consumer paths join a queue opened after they started; one tenant's deleted durable reports on failed, and a stop is not a failure; the prefetch share, and the hub bridge's shared fetch-ahead; per-tenant resize, rollback and cancellation; the shrink guard keeps every parked row; per-tenant purge at each cutoff, and no history for a tenant not named; one tenant's failed purge — its durable or its stream gone — stops no later tenant's, and an ended sweep touches none (fails with either continue turned into a break); a durable on disk is reused across a restart, or updated in place when its settings differ, and delivery resumes past what it acknowledged; per-tenant dead-letter counts, ErrNoDeadLetterQueue, and a missing dead-letter stream reopened; a queue that cannot open costs that tenant alone; boot deletes the old shared pair; boot over existing pairs reads their budgets back and delivers a queued row of a tenant given no budget, and re-applies the budget to a split pair or one missing its dead-letter stream while leaving a guarded one alone; a failed resize's undo restores the ingest stream's own cap; a consumer that cannot join a queue opened later reports it on failed
• ingest: the sweeper hands each tenant's own cutoff, re-read every sweep, and logs a failed sweep at WARN only when every tenant's failure is a missing buffer consumer
• settings: Known yields a rejected tenant's last adopted store, and none for a folder that has not validated since boot
• api: dlq/stats without the parameter reads tenant 0, names a tenant's queue alone, 404s without a queue, and refuses a malformed, empty, repeated or misparsed tenant
• app: a queue that cannot open refuses a flat boot and costs a nested directory that tenant alone, whose next publish opens it; a cancelled boot opens no queue; each tenant's budget follows its own folder, and a rejected or removed folder keeps its queue at the last budget; gapWindows names each served tenant, a rejected one at its folder's last window (unbounded when rejected since boot), and no removed one
• SDK: tenant on wh.dlq.list() and .table()
• Manual: a nested directory with two tenants; fill one tenant's queue and see only its ingest answer 503; remove it and reload, then read its parked rows with GET /v1/ops/dlq/stats?tenant=

Follow-ups

• The nats-server reservation quirk above is worth reporting upstream.
• A consumer that cannot join a queue opened at runtime fails the ingest worker, so the process restarts. Now that a publish goes only into a queue recorded open, apply could leave such a queue unrecorded instead, so the tenant answers 503 and publishes and reloads retry the join, with no restart.
• The worker's in-memory bound is per tenant now: while ClickHouse stalls it can hold up to maxAckPending (10,000) rows for every tenant with traffic, documented at the constant and in the ingest pipeline's backpressure list. A process-wide bound would bring back one tenant holding the others back; it belongs with epic(settings): multi-tenant settings directory and tenant-scoped runtime #583's deferred worker rework.
• nats-server logs three Info lines per stream at every boot (restore and consumer recovery), about 6,000 at 1,000 tenants, through slogNATSLogger.
• sdk/admin.md's older DLQ example declares const { data } twice in one block, and shows a "users": 0 count the server never reports (both predate this branch).
• api.md's ?table= row says it returns only that table's count; total stays the tenant's whole count (predates this branch).
• settings-directory.mdx's DLQ switch still calls the unreadable-envelope exception "new in this release", which goes stale with the next one (predates this branch).

Related Issues

Part of #583 (story 5b).

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cbcce041-f03e-456e-9a0b-1eb65e4d6c09

📥 Commits

Reviewing files that changed from the base of the PR and between f5d8f48 and eb9f740.

📒 Files selected for processing (1)

• internal/mq/embedded_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (11)

• GitHub Check: Docs build
• GitHub Check: Unit tests
• GitHub Check: Integration tests
• GitHub Check: Coverage
• GitHub Check: E2E tests
• GitHub Check: Lint
• GitHub Check: Analyze (actions)
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (1)

internal/mq/embedded_test.go (1)

441-449: LGTM!

---

📝 Summary

Summary by CodeRabbit

• New Features
  • Message queues, dead-letter queues, storage budgets, and history retention are managed separately for each tenant.
  • Dead-letter statistics can be requested for a specific tenant; requests for tenants without a dead-letter queue return a not-found response.
  • The TypeScript admin client supports tenant selection when listing dead-letter statistics.
• Bug Fixes
  • Queue-capacity errors and delivery issues are isolated to the affected tenant.
  • Removing or rejecting a tenant preserves its last adopted queue budget and applicable history while other tenants continue operating.

Walkthrough

The pull request replaces shared JetStream streams with tenant-specific ingest and dead-letter streams. It adds per-tenant queue budgets and purge windows, updates message delivery and replay, and lets DLQ statistics requests select a tenant.

Changes

Tenant-scoped queue flow

Layer / File(s)	Summary
Tenant queue contracts and routing internal/mq/mq.go, internal/mq/subject.go, internal/mq/subject_test.go, CHANGELOG.md, AGENTS.md	MQ contracts and subject helpers now identify queues and message subjects by tenant. One-token topic tails are no longer assigned to the default tenant.
Embedded tenant queue lifecycle internal/mq/embedded.go, internal/mq/embedded_test.go, internal/ingest/worker.go, internal/ingest/worker_test.go, internal/stream/*, internal/testutil/*, internal/api/ingest.go, internal/api/router_test.go, docs/src/content/docs/{api,architecture,durability,ingest-pipeline,settings-directory,deployment}.md*, CHANGELOG.md, internal/settings/settings.go	The broker manages tenant-specific streams, budgets, consumers, queue reopening, purge cutoffs, and replay. Tests cover per-tenant queue state, recovery, delivery, and purge behavior.
Per-tenant settings and purge windows internal/app/*, internal/ingest/sweeper*, internal/settings/registry*, docs/src/content/docs/{architecture,configuration,deployment,settings-directory,sdk/streaming}.md*, CHANGELOG.md, AGENTS.md, internal/testutil/mocks.go	App wiring reconciles budgets for served tenants. The sweeper uses a cutoff for each known tenant, and the settings registry retains adopted stores for rejected tenants.
Tenant-specific DLQ statistics internal/api/dlq*, clients/ts/src/{dlq.ts,namespaces.test.ts,types.ts}, docs/src/content/docs/{api,deployment,sdk/admin,sdk/reference,why-wavehouse}.md*, AGENTS.md, internal/settings/settings.go	DLQ statistics requests accept a tenant query. The API returns 404 when the selected tenant has no queue, and the TypeScript methods accept tenant request options.

Sequence Diagram(s)

sequenceDiagram
  participant AppWire as app.wireMQ
  participant Broker as EmbeddedNATS
  participant Queue as Tenant JetStream streams
  participant FanIn as Tenant consumer fan-in
  participant Worker as Ingest worker
  AppWire->>Broker: Reconcile tenant queue budget
  Broker->>Queue: Open or resize tenant streams
  Worker->>FanIn: Register durable consumer
  FanIn->>Queue: Join tenant ingest stream
  Queue->>FanIn: Deliver tenant messages
  FanIn->>Worker: Pass messages to worker

Loading

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Suggested reviewers: ericandrechek

Merge Risk: ⚪ Minimal · up to eb9f7

No actionable merge-blocking risk remains from the reviewed changes.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to eb9f7

Tenant-specific limits improve isolation, but the combined queues can now consume more disk than the previous server-wide guard allowed. Upgrading also deletes the old queues, including parked rows and replay history. The administrative endpoint remains protected.

Retained concerns

• Medium · security · inferred: Per-tenant queue caps no longer have an effective server-wide disk reservation limit. Traffic that fills allowed tenant queues can exhaust their shared volume and affect other tenants, despite each tenant staying within its own configured budget.
• Medium · reliability · observed: Startup deletes the legacy ingest and dead-letter streams rather than migrating them. Draining can protect uninserted events, but parked rows and pre-upgrade SSE replay history still have no preservation path described for this upgrade.

Security review details

Security Blast Radius

• inferred — Authorized ingestion is limited by its tenant's stream cap, but tenants share a physical JetStream store without an aggregate cap tied to free disk. Exhausting that store could disrupt more than the tenant producing traffic.

Security Findings and Attack Paths

• inferred — A caller permitted to ingest for a tenant can generate queued data up to that tenant's configured limit. If combined configured limits exceed available disk, the removed server-wide guard no longer prevents aggregate consumption from affecting the shared store; this is an availability risk, not evidence of cross-tenant data access.

Trust Boundaries and Controls

• observed — DLQ tenant selection parses a single valid tenant ID inside the existing admin-gated operations route. A failed queue open is surfaced as queue-full backpressure, which the ingest handler maps to a retryable 503 rather than acknowledging publication.

Resilience and Maintainability Implications

• observed — Consumer-join failure is reported but does not undo an opened stream pair. The available failed-open test verifies stream-opening refusal and recovery, not a guarantee that publication waits for every consumer to attach.

Hardening Proposals

• proposed — Consider an aggregate disk or reservation policy in addition to per-tenant caps, so a tenant's permitted queue growth cannot consume the shared volume's operational headroom.
• proposed — Consider an explicit backup or migration gate before legacy-stream deletion, particularly where parked rows or replay continuity must survive deployment or rollback.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.47% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 111 functions across 27 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely states the main change: each tenant receives its own message queue. It matches the pull request objectives and changes.
Description check	✅ Passed	The description directly explains the per-tenant queue architecture, tenant-specific budgets and consumers, API and SDK changes, migration behavior, tests, and known manual verification gap.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

✨ Simplify code

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📚 Docs preview is live → https://e94fefb2-wavehouse-docs.wave-rf.workers.dev

• Commit — eb9f740: test(mq): keep the streams directory occupied through the first failed open too
• Author — @taitelee
• Committed — 2026-09-25 08:35 (UTC-04:00)
• Deployed — 2026-09-25 09:12 EDT

[github-code-quality]

Code Coverage Overview

Languages: Go

Go

The overall line coverage in commit eb9f740 in the mq-tenant-streams branch remains at 93%, unchanged from commit 93d8019 in the main branch.

Show a line coverage summary of the most impacted files.

File	main 93d8019	mq-tenant-streams eb9f740	+/-
internal/app/wire.go	92%	92%	0%
internal/stream/hub.go	97%	97%	0%
internal/ingest/worker.go	97%	97%	0%
internal/api/dlq.go	100%	100%	0%
internal/ingest/sweeper.go	100%	100%	0%
internal/mq/subject.go	100%	100%	0%
internal/settings/registry.go	100%	100%	0%
internal/mq/embedded.go	85%	88%	+3%

---

_{Updated September 25, 2026 12:38 UTC}

[taitelee]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: eac01c7c-fbee-4177-8af5-bf2260fa31ce

📥 Commits

Reviewing files that changed from the base of the PR and between 93d8019 and 060ca9e.

📒 Files selected for processing (40)

• AGENTS.md
• CHANGELOG.md
• clients/ts/src/dlq.ts
• clients/ts/src/namespaces.test.ts
• clients/ts/src/types.ts
• docs/src/content/docs/api.md
• docs/src/content/docs/architecture.md
• docs/src/content/docs/configuration.mdx
• docs/src/content/docs/deployment.md
• docs/src/content/docs/durability.md
• docs/src/content/docs/ingest-pipeline.md
• docs/src/content/docs/sdk/admin.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/sdk/streaming.md
• docs/src/content/docs/settings-directory.mdx
• docs/src/content/docs/why-wavehouse.md
• internal/api/dlq.go
• internal/api/dlq_test.go
• internal/api/ingest.go
• internal/api/router_test.go
• internal/app/app.go
• internal/app/app_test.go
• internal/app/wire.go
• internal/ingest/sweeper.go
• internal/ingest/sweeper_test.go
• internal/ingest/worker.go
• internal/ingest/worker_test.go
• internal/mq/embedded.go
• internal/mq/embedded_test.go
• internal/mq/mq.go
• internal/mq/subject.go
• internal/mq/subject_test.go
• internal/settings/registry.go
• internal/settings/registry_test.go
• internal/settings/settings.go
• internal/settings/store.go
• internal/stream/hub.go
• internal/stream/subscriber.go
• internal/testutil/mocks.go
• internal/testutil/testutil.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. (9)

• GitHub Check: Integration tests
• GitHub Check: E2E tests
• GitHub Check: Unit tests
• GitHub Check: Coverage
• GitHub Check: Docs build
• GitHub Check: Lint
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used

📓 Path-based instructions (5)

Fail-closed — preserve it when touching `internal/api`.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• internal/api/ingest.go
• internal/api/router_test.go
• internal/api/dlq.go
• internal/api/dlq_test.go

See [AGENTS.md](AGENTS.md) for project conventions, architecture notes, and AI agent instructions.

📄 CodeRabbit inference engine (CLAUDE.md)

Files:

• AGENTS.md

See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

• AGENTS.md

In MDX, leave a blank line between a JSX tag and a code fence.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• docs/src/content/docs/configuration.mdx
• docs/src/content/docs/settings-directory.mdx

WH001 applies to every tracked Markdown file, with no carve-out Editors see WH001 in `.md` only.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/why-wavehouse.md
• docs/src/content/docs/sdk/streaming.md
• docs/src/content/docs/sdk/admin.md
• docs/src/content/docs/durability.md
• AGENTS.md
• docs/src/content/docs/deployment.md
• docs/src/content/docs/architecture.md
• docs/src/content/docs/ingest-pipeline.md
• docs/src/content/docs/api.md
• CHANGELOG.md

🧠 Learnings (2)

📚 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/dlq_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/mq/embedded_test.go

🪛 LanguageTool

docs/src/content/docs/configuration.mdx

[style] ~100-~100: Since ownership is already implied, this phrasing may be redundant.
Context: Each tenant's queue has its own disk budget, mq.max_bytes_gb, a hot-r...

(PRP_OWN)

docs/src/content/docs/durability.md

[style] ~36-~36: Consider using the typographical ellipsis character here instead.
Context: ...eload that first serves the tenant — is open dlq stream: ... context deadline exceeded, or `open in...

(ELLIPSIS)

---

[style] ~36-~36: Consider using the typographical ellipsis character here instead.
Context: ...eam: ... context deadline exceeded, or open ingest stream: ...` (the two share the budget); a boot tha...

(ELLIPSIS)

---

[style] ~96-~96: Consider using the typographical ellipsis character here instead.
Context: ...eam: ... context deadline exceeded, or open ingest stream: ...`, when a tenant's queue first opens, at...

(ELLIPSIS)

---

[style] ~97-~97: Consider using the typographical ellipsis character here instead.
Context: ...very ended; ingestion has stoppedwithjoin its queue: ... context deadline exceeded`, and the pro...

(ELLIPSIS)

AGENTS.md

[style] ~43-~43: Consider using the typographical ellipsis character here instead.
Context: ...equest; settings.Store in production, Static(q...) in tests) - policy/ — Hasura-st...

(ELLIPSIS)

---

[style] ~47-~47: Since ownership is already implied, this phrasing may be redundant.
Context: ... table — and evaluates each event under its own tenant's policy and schema registry; `P...

(PRP_OWN)

docs/src/content/docs/deployment.md

[style] ~388-~388: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...t the parameter — and on SIGHUP — the whole directory is reloaded and mirrors its f...

(EN_REPEATEDWORDS_WHOLE)

---

[style] ~388-~388: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ved: delete its folder, then reload the whole directory. Its open streams end, its ro...

(EN_REPEATEDWORDS_WHOLE)

---

[style] ~388-~388: Since ownership is already implied, this phrasing may be redundant.
Context: ...queued rows are parked on the DLQ under its own subject; nothing it stored is deleted —...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...cides.** A request is evaluated against its own tenant's policies.json and `pipes.jso...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...s-directory#clickhouse) — and discovers its own tables from its own database on its own...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...se) — and discovers its own tables from its own database on its own `schema.refresh_int...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: .../v1/streamconnection is authorized by its own tenant'spolicies.json` and receives i...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...n tenant's policies.json and receives its own tenant's rows alone, the ingest worker ...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...e, the ingest worker inserts a row into its own tenant's ClickHouse, a failed row is pa...

(PRP_OWN)

---

[style] ~392-~392: Since ownership is already implied, this phrasing may be redundant.
Context: ...lickHouse, a failed row is parked under its own tenant's dlq.enabled and subject (`dl...

(PRP_OWN)

---

[style] ~394-~394: Since ownership is already implied, this phrasing may be redundant.
Context: ... while every other tenant's routes keep their own list. Tenant 0's own dedupe store clo...

(PRP_OWN)

docs/src/content/docs/architecture.md

[grammar] ~85-~85: Ensure spelling is correct
Context: ...ave-RF/WaveHouse/issues/319)). Gap-fill replay (mq.Replayer.ReplaySince on the conne...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

---

[style] ~92-~92: This phrase is redundant. Consider writing “last”.
Context: ...nt. The SIGHUP registration is released last of all. Handler, Registry, and MQ expose...

(LAST_OF_ALL)

---

[style] ~92-~92: Since ownership is already implied, this phrasing may be redundant.
Context: ...ons.Listenerlets one serve the API on its own listener instead ofserver.port`. - **...

(PRP_OWN)

---

[style] ~148-~148: Since ownership is already implied, this phrasing may be redundant.
Context: ...o a blocking handler is backpressure on its own tenant alone, spreads the prefetch acro...

(PRP_OWN)

---

[style] ~148-~148: A comma is missing here.
Context: ...ending on its own — ErrDeliveryEnded, e.g. a deleted consumer, a closed connection...

(EG_NO_COMMA)

---

[style] ~148-~148: Since ownership is already implied, this phrasing may be redundant.
Context: ...terer.DeadLetter` (park a message under its own topic, in its tenant's dead-letter queu...

(PRP_OWN)

---

[style] ~151-~151: ‘out of reach’ might be wordy. Consider a shorter alternative.
Context: ...75% of the free disk by default) is set out of reach, so a budget is a cap and never a reser...

(EN_WORDINESS_PREMIUM_OUT_OF_REACH)

---

[style] ~151-~151: For a more expressive style, consider rephrasing the sentence in the active voice.
Context: ...rts the budget last applied in full, so a failed resize is retried by the next reload. The consumers CreateConsumer and `Su...

(PASSIVE_VOICE_SIMPLE)

docs/src/content/docs/ingest-pipeline.md

[grammar] ~64-~64: Ensure spelling is correct
Context: ... as ClickHouse's calendar/epoch shapes, where basic read a plain DateTime column'...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

---

[grammar] ~64-~64: Ensure spelling is correct
Context: ...nds (shorter runs it rejected outright, where best_effort reads "2026" as a year)...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

---

[grammar] ~64-~64: Ensure spelling is correct
Context: ...effort "20260711"stores 2026-07-11, wherebasicstored 1970-08-23.DateTime64`...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

---

[style] ~205-~205: Consider using “who” when you are referring to people instead of objects.
Context: ...rker's own failed channel. A consumer that cannot start at all takes the same path...

(THAT_WHO)

---

[style] ~207-~207: To elevate your writing, try using a synonym here.
Context: ...at could delete a durable) this path is hard to reach; the likeliest way in is a ten...

(HARD_TO)

---

[style] ~227-~227: Consider an alternative for the overused word “exactly”.
Context: ...an fsync and therefore slow, which is exactly why acks run in the background (ackWg...

(EXACTLY_PRECISELY)

CHANGELOG.md

[typographical] ~13-~13: Consider using an em dash in dialogues and enumerations.
Context: - **A tenant removed or rejected at runti...

(DASH_RULE)

---

[style] ~13-~13: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: - A tenant removed or rejected at runtime has its open streams ended (internal/stream/{hub,subscriber,bucket}.go (+ tests), internal/api/stream.go (+ tests), internal/api/router.go, internal/settings/{registry,tree}.go (+ tests), internal/ingest/worker.go (+ tests), internal/app/wire.go (+ tests), clients/ts/src/stream/sse.ts, docs/src/content/docs/{api,deployment,architecture,ingest-pipeline}.md, docs/src/content/docs/settings-directory.mdx, AGENTS.md): story 3 of the multi-tenant epic (#583). A GET /v1/stream used to outlive i...

(TOO_LONG_SENTENCE)

---

[style] ~13-~13: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ld pass, and meets its DLQ switch once, whole, logged once per batch rather than twic...

(EN_REPEATEDWORDS_WHOLE)

---

[style] ~13-~13: Since ownership is already implied, this phrasing may be redundant.
Context: ...on, so its queued rows are parked under its own subject rather than dropped, inserted i...

(PRP_OWN)

---

[style] ~27-~27: Since ownership is already implied, this phrasing may be redundant.
Context: ...t-reloadable half of configuration gets its own page; configuration.mdx is boot confi...

(PRP_OWN)

---

[style] ~27-~27: For conciseness, consider replacing this expression with an adverb.
Context: ....tables` (resolved by the ingest worker at the moment a poison row is isolated: on → park it ...

(AT_THE_MOMENT)

---

[style] ~27-~27: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...out materializing settings directories. The settings directory is also the runtime authority for access control and named pipes (internal/settings/store.go, internal/policy/source.go (new), internal/pipes/pipes.go, internal/api/{policy,pipes,router}.go, internal/stream/hub.go, internal/auth/auth.go, cmd/wavehouse/main.go, Makefile, deployments/compose/settings/{policies,roles}.json, clients/ts/src/settings.ts (new); closes #229, #33, #461, #514, #460, #363; advances #48 and #214): roles.json, policies.json, and pipes.json are adopted with config.json as one snapshot and re-adopted on the same three triggers, and files are the only write path — standalone, the operator edits them on the host; on WaveHouse Cloud the control plane writes them — so there is no stored copy that can skip validation: every adoption runs the current rules (strict decode rejecting unknown and duplicate keys, the full policy validation including the claim-template grammar, pipe name/SQL/parameter-type rules, and the cross-file check that every role a grant or allowed_roles names is declared in roles.json), and a rejected edit keeps the previous good policy and pipes in effect. policies.json is one policy document ...

(TOO_LONG_SENTENCE)

---

[style] ~31-~31: Since ownership is already implied, this phrasing may be redundant.
Context: ...ter,LiveDemo}.astro`): the site tracked its own CTAs but nothing a reader did on the wa...

(PRP_OWN)

---

[style] ~31-~31: Since ownership is already implied, this phrasing may be redundant.
Context: ...docs area without each tracker carrying its own copy; it's stamped at capture time by a...

(PRP_OWN)

---

[style] ~31-~31: Since ownership is already implied, this phrasing may be redundant.
Context: ...ressive Code, and the 404 route all own their own markup — some of it created after page ...

(PRP_OWN)

---

[typographical] ~35-~35: Consider using an em dash in dialogues and enumerations.
Context: - **Each tenant has a message queue of it...

(DASH_RULE)

---

[style] ~35-~35: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: - Each tenant has a message queue of its own (internal/mq/{mq,subject,embedded}.go (+ tests), internal/ingest/{sweeper,worker}.go (+ tests), internal/api/{dlq,ingest}.go (+ tests), internal/app/{app,wire}.go (+ tests), internal/stream/{subscriber,hub}.go, internal/settings/{settings,store,registry}.go (+ tests), internal/testutil/{mocks,testutil}.go, clients/ts/src/{dlq,types}.ts (+ tests), docs/src/content/docs/{deployment,api,architecture,ingest-pipeline,durability,why-wavehouse}.md, docs/src/content/docs/sdk/{admin,reference,streaming}.md, docs/src/content/docs/{settings-directory,configuration}.mdx, AGENTS.md): story 5b of the multi-tenant epic (#583). The embedded NATS server keeps each ...

(TOO_LONG_SENTENCE)

---

[style] ~35-~35: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...g outside internal/mq names a stream. A tenant at its budget gets 503 while the others keep publishing, and the ingest worker's and the hub bridge's durables are held on every tenant's stream, each with its own ack floor and MaxAckPending, so one tenant's backlog holds back neither another's delivery nor its purge; the worker's prefetch and the hub bridge's fetch-ahead are each shared across the tenants' streams. A removed or rejected tenant's stream i...

(TOO_LONG_SENTENCE)

---

[style] ~37-~37: Since ownership is already implied, this phrasing may be redundant.
Context: ...ll topic and evaluates each event under its own tenant's policy, so a subscriber on one...

(PRP_OWN)

🔇 Additional comments (40)

clients/ts/src/dlq.ts (1)

2-2: LGTM!

Also applies to: 4-4, 18-22, 26-26, 33-34, 38-38

clients/ts/src/namespaces.test.ts (1)

149-166: LGTM!

clients/ts/src/types.ts (1)

474-475: LGTM!

docs/src/content/docs/sdk/admin.md (1)

68-75: LGTM!

docs/src/content/docs/sdk/reference.md (1)

107-108: LGTM!

docs/src/content/docs/why-wavehouse.md (1)

56-56: LGTM!

Also applies to: 159-159, 193-193, 224-224

internal/api/dlq.go (2)

10-10: LGTM!

Also applies to: 22-28, 34-35, 39-44

---

30-30: 🔒 Security & Privacy | 🛡️ Detected with Advanced Tier

Nested DLQ reads already require the operator key. The /v1/ops gate disables policy-based admin access for nested settings, so a non-operator admin token receives 403 before DLQHandler.Stats runs.

Likely an incorrect or invalid review comment.

internal/api/dlq_test.go (1)

18-22: LGTM!

Also applies to: 27-34, 36-49, 52-52, 58-58, 61-61, 64-64, 78-82, 85-85, 89-89, 91-91, 104-104, 109-109, 112-115, 122-161

AGENTS.md (1)

32-32: LGTM!

Also applies to: 41-41, 48-48, 59-59

CHANGELOG.md (1)

13-13: LGTM!

Also applies to: 27-27, 35-37

internal/mq/mq.go (1)

144-169: LGTM!

Also applies to: 192-228, 239-285, 299-299, 309-319

internal/mq/subject.go (1)

15-61: LGTM!

Also applies to: 97-115

internal/mq/subject_test.go (1)

4-7: LGTM!

Also applies to: 140-186

docs/src/content/docs/api.md (1)

277-277: LGTM!

Also applies to: 388-388, 748-765, 861-861, 879-881

docs/src/content/docs/architecture.md (1)

80-93: LGTM!

Also applies to: 142-151, 193-193, 231-235, 255-258

docs/src/content/docs/durability.md (1)

36-37: LGTM!

Also applies to: 84-84, 96-97, 104-104

docs/src/content/docs/ingest-pipeline.md (1)

25-33: LGTM!

Also applies to: 49-49, 64-64, 73-73, 92-96, 205-207, 215-217, 224-225, 231-239, 251-251

docs/src/content/docs/deployment.md (1)

382-394: LGTM!

Also applies to: 422-426, 438-445

docs/src/content/docs/settings-directory.mdx (1)

34-34: LGTM!

Also applies to: 127-135, 215-226, 234-234

internal/ingest/worker.go (1)

119-128: LGTM!

Also applies to: 227-245, 476-481, 497-497, 798-800

internal/ingest/worker_test.go (1)

125-125: LGTM!

Also applies to: 155-155, 246-246, 295-295, 1089-1089, 1180-1180, 1268-1271, 1782-1782

internal/api/ingest.go (1)

710-713: LGTM!

internal/api/router_test.go (1)

334-334: LGTM!

internal/mq/embedded_test.go (1)

5-7: LGTM!

Also applies to: 19-74, 135-135, 144-167, 178-178, 254-278, 319-319, 358-420, 422-643

internal/testutil/mocks.go (1)

224-227: LGTM!

Also applies to: 236-250

internal/testutil/testutil.go (1)

17-17: LGTM!

Also applies to: 43-60

internal/settings/settings.go (1)

166-168: LGTM!

Also applies to: 222-231

internal/stream/hub.go (1)

46-48: LGTM!

Also applies to: 397-398

internal/stream/subscriber.go (1)

58-68: LGTM!

docs/src/content/docs/configuration.mdx (1)

100-100: LGTM!

docs/src/content/docs/sdk/streaming.md (1)

119-119: LGTM!

internal/app/app.go (1)

21-21: LGTM!

Also applies to: 91-92, 143-145, 176-176

internal/app/app_test.go (1)

240-240: LGTM!

Also applies to: 249-250, 400-468, 574-617, 733-776

internal/app/wire.go (1)

9-9: LGTM!

Also applies to: 69-86, 108-132, 165-169, 522-541, 559-585, 602-606

internal/ingest/sweeper_test.go (1)

10-10: LGTM!

Also applies to: 19-53, 60-60, 72-72

internal/settings/registry.go (1)

157-167: LGTM!

internal/settings/registry_test.go (1)

309-347: LGTM!

internal/settings/store.go (1)

196-196: LGTM!

internal/mq/embedded.go (1)

637-642: 🩺 Stability & Availability

The concurrent Hub.Broadcast calls do not expose an unsynchronized Hub state race. Broadcast snapshots routes under h.mu.RLock and uses local projection state. deliver protects schema state with schemaMu, and Subscriber.Send uses a channel. Each subscriber receives events from one tenant's delivery goroutine, so different tenant broadcasts do not concurrently update the same subscriber state.

[EricAndrechek]

@/tmp/pr_edits/comment5831496372.md

[taitelee]

Those runs were on the pre-fix test: feat/mq-nats-wiring and feat/mq-external-broker both fork from 9344f1b8, one commit before f5d8f484, and their embedded_test.go is byte-identical to it. On a checkout of feat/mq-nats-wiring here the unfixed test fails 5 of 20 runs; with f5d8f484 applied it's 0 of 20, the app test with the same pattern 5 of 5, and the patch applies cleanly (nothing in #639's embedded.go changes touches the open path). eb9f7409 closes a much narrower window of the same kind in TestEmbeddedNATS_SetMaxBytes_AQueueThatCannotOpen. So rather than a second test-only fix on the integration branch, merge #612's head into feat/mq-external-broker — two fixes to the same test would conflict when #612 lands.

[taitelee]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.