Open
Conversation
Standardizes durable consumer config across worker services via a shared pkg/stream helper plus per-service MaxAckPending recommendations.
📝 WalkthroughWalkthroughAdds pkg/stream.DurableConsumerDefaults() and exported default constants; refactors workers to use per-service buildConsumerConfig() helpers derived from those defaults (setting Durable and per-service MaxAckPending), replaces inline consumer literals with helpers, adds unit tests, and adds design/plans/spec docs. ChangesJetStream Consumer Defaults Design & Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-08-jetstream-consumer-defaults-design.md`:
- Around line 21-23: Update the sentence that currently claims "No runtime
impact on existing durable consumers" to explicitly qualify that only the
consumer cursor position is preserved by DeliverPolicy, and that runtime
redelivery behavior can change if MaxDeliver is modified; reference the symbols
DeliverPolicy, MaxDeliver and durable consumers and add a short note that
reducing MaxDeliver (e.g., from unlimited to 5) will limit subsequent
redeliveries for existing durables even though their cursor positions are
unchanged so migrations or operator notice may be required.
- Line 183: The statement on line 183 claiming tests don't rely on unlimited
redeliveries is incorrect because several tests create consumers without setting
MaxDeliver (e.g., ConsumerConfig instances in tools/loadgen/integration_test.go,
search-sync-worker/integration_test.go, and inbox_integration_test.go), so
update the doc and/or tests: either modify those tests to set an explicit
MaxDeliver (for example MaxDeliver: 6) and run/record verification that they
pass with that limit, or change the design doc sentence to read that "test
consumers will be updated to use explicit MaxDeliver limits instead of relying
on defaults" and list the affected test files
(tools/loadgen/integration_test.go, search-sync-worker/integration_test.go,
inbox_integration_test.go) so readers know which consumers must be changed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6c03213-b4a7-403d-91c1-905393724156
📒 Files selected for processing (1)
docs/superpowers/specs/2026-05-08-jetstream-consumer-defaults-design.md
| - `AckPolicy` is unchanged. | ||
| - `AckWait = 30s` matches the prior NATS default. | ||
| - `MaxDeliver = 5` is permissive enough for any test that previously | ||
| relied on default unlimited redeliveries (none exist in the codebase). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: surface potential retry/redelivery dependencies.
set -euo pipefail
echo "== Consumer retry-related config usage =="
rg -n --iglob '*.go' 'MaxDeliver|MaxRedeliver|AckWait|BackOff|CreateOrUpdateConsumer\('
echo
echo "== Deploy/env references to MAX_REDELIVER =="
rg -n --iglob '*.{yml,yaml,tpl,env,sh,md}' 'MAX_REDELIVER|MaxRedeliver'
echo
echo "== Tests with explicit redelivery/retry expectations =="
rg -n --iglob '*_test.go' 'redeliver|retry|MaxDeliver|BackOff|Nak\(|Term\('Repository: hmchangw/chat
Length of output: 5166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Test files creating ConsumerConfig without explicit MaxDeliver =="
# Search for ConsumerConfig{} or ConsumerConfig{ blocks in test files to see patterns
rg -A 15 'ConsumerConfig\{' --iglob '*_test.go' | head -100
echo
echo "== Check message-worker main.go MaxRedeliver usage =="
cat -n message-worker/main.go | sed -n '30,125p'Repository: hmchangw/chat
Length of output: 11630
Line 183's "none exist in the codebase" claim is unsupported—test files create consumers without MaxDeliver.
The design doc claims tests don't rely on unlimited redeliveries defaults, but test code shows otherwise. ConsumerConfig creations in tools/loadgen/integration_test.go, search-sync-worker/integration_test.go, and multiple search-sync-worker/inbox_integration_test.go tests omit the MaxDeliver field, meaning they implicitly rely on JetStream's default unlimited redeliveries behavior during test execution.
Either add verification showing these tests pass with explicit MaxDeliver limits (e.g., MaxDeliver: 6), or revise the claim to "test consumers will be updated to use explicit MaxDeliver limits instead of relying on defaults."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-08-jetstream-consumer-defaults-design.md` at
line 183, The statement on line 183 claiming tests don't rely on unlimited
redeliveries is incorrect because several tests create consumers without setting
MaxDeliver (e.g., ConsumerConfig instances in tools/loadgen/integration_test.go,
search-sync-worker/integration_test.go, and inbox_integration_test.go), so
update the doc and/or tests: either modify those tests to set an explicit
MaxDeliver (for example MaxDeliver: 6) and run/record verification that they
pass with that limit, or change the design doc sentence to read that "test
consumers will be updated to use explicit MaxDeliver limits instead of relying
on defaults" and list the affected test files
(tools/loadgen/integration_test.go, search-sync-worker/integration_test.go,
inbox_integration_test.go) so readers know which consumers must be changed.
9-task TDD plan to add a pkg/stream helper for durable consumer defaults and apply per-service MaxAckPending recommendations across all 7 worker services.
…t reduction The unified MaxDeliver=5 reduces message-worker's retry budget by 1 attempt (was 6, now 5 total deliveries). This is intentional as part of standardizing the project-wide default; the prior wording "no operational behavior change" was inaccurate.
Add stream.ConsumerSettings (AckWait, MaxDeliver, MaxWaiting,
MaxAckPending) and parameterize DurableConsumerDefaults to take it as
input. Each service's Config gains:
Consumer stream.ConsumerSettings `envPrefix:"CONSUMER_"`
driven by CONSUMER_ACK_WAIT, CONSUMER_MAX_DELIVER, CONSUMER_MAX_WAITING,
and CONSUMER_MAX_ACK_PENDING. Defaults: 30s, 5, 512, 1000 — applied
uniformly via the envDefault tags on ConsumerSettings.
Architectural invariants (AckPolicy=Explicit, DeliverPolicy=New) remain
hardcoded in the helper. Per-service buildConsumerConfig functions no
longer hardcode MaxAckPending; operators tune per-service values via the
service's deployment env.
DEPLOY-CONFIG DEPENDENCY: prior code shipped per-service MaxAckPending
defaults (gatekeeper/broadcast=1000, message/notification/search-sync=500,
room=200, inbox=100). The new code-level default is 1000 across the
board. To preserve current production behavior, set CONSUMER_MAX_ACK_PENDING
explicitly in the K8s manifests for message-worker (500),
notification-worker (500), search-sync-worker (500), room-worker (200),
and inbox-worker (100).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
notification-worker/consumer_config_test.go (1)
31-44: ⚡ Quick winRe-verify architectural invariants in the second subtest.
The "overrides flow through" subtest validates that custom settings propagate correctly but omits assertions for the fixed policies (
AckPolicy,DeliverPolicy). Re-checking these invariants would confirm they remain independent of the settings values.✅ Add invariant assertions for robustness
assert.Equal(t, "notification-worker", cc.Durable) assert.Equal(t, 500, cc.MaxAckPending) + assert.Equal(t, jetstream.AckExplicitPolicy, cc.AckPolicy) assert.Equal(t, 45*time.Second, cc.AckWait) assert.Equal(t, 3, cc.MaxDeliver) assert.Equal(t, 256, cc.MaxWaiting) + assert.Equal(t, jetstream.DeliverNewPolicy, cc.DeliverPolicy) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@notification-worker/consumer_config_test.go` around lines 31 - 44, The test "overrides flow through" omits asserting immutable policies; update the subtest to also assert that buildConsumerConfig(stream.ConsumerSettings{...}) returns cc with the expected invariant policies (check cc.AckPolicy and cc.DeliverPolicy) so these remain unchanged when AckWait/MaxDeliver/MaxWaiting/MaxAckPending are overridden; locate the test and add assertions comparing cc.AckPolicy and cc.DeliverPolicy to the expected constant values used elsewhere in the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@notification-worker/consumer_config_test.go`:
- Around line 31-44: The test "overrides flow through" omits asserting immutable
policies; update the subtest to also assert that
buildConsumerConfig(stream.ConsumerSettings{...}) returns cc with the expected
invariant policies (check cc.AckPolicy and cc.DeliverPolicy) so these remain
unchanged when AckWait/MaxDeliver/MaxWaiting/MaxAckPending are overridden;
locate the test and add assertions comparing cc.AckPolicy and cc.DeliverPolicy
to the expected constant values used elsewhere in the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca943dd8-a7ed-4dfd-915d-17295df8d070
📒 Files selected for processing (16)
broadcast-worker/consumer_config_test.gobroadcast-worker/main.goinbox-worker/consumer_config_test.goinbox-worker/main.gomessage-gatekeeper/consumer_config_test.gomessage-gatekeeper/main.gomessage-worker/consumer_config_test.gomessage-worker/main.gonotification-worker/consumer_config_test.gonotification-worker/main.gopkg/stream/consumer.gopkg/stream/consumer_test.goroom-worker/consumer_config_test.goroom-worker/main.gosearch-sync-worker/consumer_config_test.gosearch-sync-worker/main.go
🚧 Files skipped from review as they are similar to previous changes (6)
- room-worker/consumer_config_test.go
- message-worker/consumer_config_test.go
- message-gatekeeper/consumer_config_test.go
- room-worker/main.go
- search-sync-worker/main.go
- message-worker/main.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standardizes durable consumer config across worker services via a
shared pkg/stream helper plus per-service MaxAckPending recommendations.
Summary by CodeRabbit