refactor(acp-nats): split global extensions into dedicated GLOBAL_EXT stream#71
Conversation
… stream Extensions are user-defined with unknown volume profiles. Isolating them prevents a noisy extension from evicting lifecycle audit events. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryMedium Risk Overview Adds Written by Cursor Bugbot for commit a4f616e. This will update automatically on new commits. Configure here. |
WalkthroughA new NATS JetStream stream configuration ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Code Coverage SummaryDetailsDiff against mainResults for commit: a4f616e Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rsworkspace/crates/acp-nats/src/jetstream/streams.rs (1)
92-106: Consider documenting the migration impact for existing deployments.The
global_confignow excludes{prefix}.agent.ext.>subjects. Existing consumers that were receiving extension messages from the GLOBAL stream will need to subscribe to the new GLOBAL_EXT stream after this change.If this is deployed to existing environments, consider:
- Adding migration notes to the PR description or changelog
- Ensuring consumers are updated to subscribe to the new stream before or alongside this deployment
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/acp-nats/src/jetstream/streams.rs` around lines 92 - 106, Update the PR/changelog and add migration guidance noting that global_config (the function building the GLOBAL stream via stream_name(prefix, "GLOBAL")) no longer includes the "{prefix}.agent.ext.>" subjects and that those messages now belong to the new GLOBAL_EXT stream; explicitly instruct operators to update consumers to subscribe to the new GLOBAL_EXT stream (or add the subjects back temporarily) and recommend a rollout order (create GLOBAL_EXT and update consumers before removing ext subjects from GLOBAL) so no messages are lost.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rsworkspace/crates/acp-nats/src/jetstream/streams.rs`:
- Around line 92-106: Update the PR/changelog and add migration guidance noting
that global_config (the function building the GLOBAL stream via
stream_name(prefix, "GLOBAL")) no longer includes the "{prefix}.agent.ext.>"
subjects and that those messages now belong to the new GLOBAL_EXT stream;
explicitly instruct operators to update consumers to subscribe to the new
GLOBAL_EXT stream (or add the subjects back temporarily) and recommend a rollout
order (create GLOBAL_EXT and update consumers before removing ext subjects from
GLOBAL) so no messages are lost.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95c0c912-3131-4f55-8315-cf9988eb7e09
📒 Files selected for processing (2)
rsworkspace/crates/acp-nats/src/jetstream/provision.rsrsworkspace/crates/acp-nats/src/jetstream/streams.rs
Summary
agent.ext.>from GLOBAL stream into its own GLOBAL_EXT stream with independent storage limitsinitialize,authenticate,session.new