Skip to content

refactor(acp-nats): switch NOTIFICATIONS stream to file storage - #69

Merged
yordis merged 1 commit into
mainfrom
refactor/notifications-file-storage
Mar 31, 2026
Merged

refactor(acp-nats): switch NOTIFICATIONS stream to file storage#69
yordis merged 1 commit into
mainfrom
refactor/notifications-file-storage

Conversation

@yordis

@yordis yordis commented Mar 31, 2026

Copy link
Copy Markdown
Member

Summary

  • Switch NOTIFICATIONS stream from memory-backed to file-backed storage for consistent security audit coverage across all JetStream streams
  • Align NOTIFICATIONS max age to 30 days (matching COMMANDS, RESPONSES, CLIENT_OPS)
  • Remove unused DEFAULT_MEMORY_MAX_AGE constant

@cursor

cursor Bot commented Mar 31, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Medium risk because it changes JetStream storage/retention behavior for the NOTIFICATIONS stream (memory to file, longer retention), which can affect disk usage and message availability semantics.

Overview
Switches the JetStream NOTIFICATIONS stream from memory-backed to file-backed storage and aligns its max_age retention to 30 days, matching the other ACP streams.

Introduces a shared DEFAULT_STREAM_MAX_AGE constant and updates stream config tests to assert all streams use file storage and the unified max age.

Written by Cursor Bugbot for commit 305ca67. This will update automatically on new commits. Configure here.

@coderabbitai

coderabbitai Bot commented Mar 31, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@yordis has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 41 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 41 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ae0eb50f-0509-4f8c-8a8c-3a18b0d58884

📥 Commits

Reviewing files that changed from the base of the PR and between 1805a5d and 305ca67.

📒 Files selected for processing (2)
  • rsworkspace/crates/acp-nats/src/constants.rs
  • rsworkspace/crates/acp-nats/src/jetstream/streams.rs

Walkthrough

A single file change removes a constant and migrates the notification stream configuration from memory-backed storage with a 5-minute retention to file-backed storage with a 30-day retention period. Unit tests are updated to verify all stream configurations use file storage and consistent maximum age values.

Changes

Cohort / File(s) Summary
Notification Stream Storage Migration
rsworkspace/crates/acp-nats/src/jetstream/streams.rs
Removed DEFAULT_MEMORY_MAX_AGE constant. Updated notifications_config to use StorageType::File and DEFAULT_FILE_MAX_AGE instead of memory storage with 5-minute max age. Modified storage and max-age unit tests to assert all four stream configs use file storage and 30-day retention.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 From fleeting memories to files so grand,
Thirty glorious days our messages stand!
No more five-minute fading away,
Our notifications now forever stay! 📁✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the primary change: switching the NOTIFICATIONS stream to file storage, which directly corresponds to the main objectives of the pull request.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the motivation (consistent audit coverage), the specific changes (storage type and max age alignment), and cleanup (removing unused constant).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/notifications-file-storage

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Mar 31, 2026

Copy link
Copy Markdown

badge

Code Coverage Summary

Details
Filename                                                     Stmts    Miss  Cover    Missing
---------------------------------------------------------  -------  ------  -------  ---------------------------------------------------------------------------------------------
crates/acp-nats-agent/src/connection.rs                       1356       9  99.34%   472, 669-671, 678, 1826-1827, 1840-1841
crates/acp-nats-ws/src/config.rs                                83       0  100.00%
crates/acp-nats-ws/src/connection.rs                           162      35  78.40%   71-78, 83-94, 110, 112-113, 118, 129-131, 138, 142, 146, 149-157, 168, 172, 175, 178-182, 216
crates/acp-nats-ws/src/upgrade.rs                               57       2  96.49%   59, 90
crates/acp-nats-ws/src/main.rs                                 157       2  98.73%   84, 247
crates/acp-nats/src/agent/test_support.rs                      267       0  100.00%
crates/acp-nats/src/agent/set_session_model.rs                  76       0  100.00%
crates/acp-nats/src/agent/new_session.rs                        91       0  100.00%
crates/acp-nats/src/agent/ext_notification.rs                   88       0  100.00%
crates/acp-nats/src/agent/load_session.rs                      114       0  100.00%
crates/acp-nats/src/agent/prompt.rs                           1085       1  99.91%   144
crates/acp-nats/src/agent/set_session_config_option.rs          76       0  100.00%
crates/acp-nats/src/agent/close_session.rs                      72       0  100.00%
crates/acp-nats/src/agent/resume_session.rs                    113       0  100.00%
crates/acp-nats/src/agent/bridge.rs                            142      13  90.85%   171-183
crates/acp-nats/src/agent/cancel.rs                            104       0  100.00%
crates/acp-nats/src/agent/js_request.rs                        296       0  100.00%
crates/acp-nats/src/agent/fork_session.rs                      117       0  100.00%
crates/acp-nats/src/agent/set_session_mode.rs                   79       0  100.00%
crates/acp-nats/src/agent/list_sessions.rs                      50       0  100.00%
crates/acp-nats/src/agent/initialize.rs                         82       0  100.00%
crates/acp-nats/src/agent/mod.rs                                61       0  100.00%
crates/acp-nats/src/agent/authenticate.rs                       52       0  100.00%
crates/acp-nats/src/agent/ext_method.rs                         92       0  100.00%
crates/acp-nats/src/nats/subjects.rs                           294       0  100.00%
crates/acp-nats/src/nats/token.rs                                8       0  100.00%
crates/acp-nats/src/nats/extensions.rs                           3       0  100.00%
crates/acp-nats/src/nats/parsing.rs                            280       1  99.64%   151
crates/trogon-std/src/json.rs                                   30       0  100.00%
crates/trogon-std/src/args.rs                                   10       0  100.00%
crates/trogon-nats/src/jetstream/traits.rs                      96       0  100.00%
crates/trogon-nats/src/jetstream/mocks.rs                      435       0  100.00%
crates/acp-nats/src/jetstream/streams.rs                       176       0  100.00%
crates/acp-nats/src/jetstream/provision.rs                      56       0  100.00%
crates/acp-nats/src/jetstream/consumers.rs                      76       0  100.00%
crates/acp-nats/src/jetstream/ext_policy.rs                     26       0  100.00%
crates/acp-nats/src/telemetry/metrics.rs                        65       0  100.00%
crates/acp-nats-stdio/src/config.rs                             72       0  100.00%
crates/acp-nats-stdio/src/main.rs                              113      11  90.27%   58, 106-113, 119-121, 138
crates/acp-telemetry/src/service_name.rs                        16       0  100.00%
crates/acp-telemetry/src/trace.rs                               32       4  87.50%   23-24, 31-32
crates/acp-telemetry/src/log.rs                                 70       2  97.14%   39-40
crates/acp-telemetry/src/signal.rs                               3       3  0.00%    4-43
crates/acp-telemetry/src/metric.rs                              35       4  88.57%   30-31, 38-39
crates/acp-telemetry/src/lib.rs                                153      22  85.62%   39-46, 81, 86, 91, 105-120
crates/trogon-std/src/fs/system.rs                              29      12  58.62%   17-19, 31-45
crates/trogon-std/src/fs/mem.rs                                220      10  95.45%   61-63, 77-79, 133-135, 158
crates/trogon-std/src/dirs/system.rs                            98      11  88.78%   57, 65, 67, 75, 77, 85, 87, 96, 98, 109, 154
crates/trogon-std/src/dirs/fixed.rs                             84       0  100.00%
crates/acp-nats/src/client/terminal_create.rs                  294       0  100.00%
crates/acp-nats/src/client/fs_read_text_file.rs                384       0  100.00%
crates/acp-nats/src/client/terminal_kill.rs                    309       0  100.00%
crates/acp-nats/src/client/ext.rs                              365       8  97.81%   193-204, 229-240
crates/acp-nats/src/client/session_update.rs                    55       0  100.00%
crates/acp-nats/src/client/terminal_wait_for_exit.rs           396       0  100.00%
crates/acp-nats/src/client/fs_write_text_file.rs               451       0  100.00%
crates/acp-nats/src/client/ext_session_prompt_response.rs      150       0  100.00%
crates/acp-nats/src/client/rpc_reply.rs                         71       0  100.00%
crates/acp-nats/src/client/request_permission.rs               338       0  100.00%
crates/acp-nats/src/client/terminal_release.rs                 357       0  100.00%
crates/acp-nats/src/client/mod.rs                             2981       0  100.00%
crates/acp-nats/src/client/terminal_output.rs                  223       0  100.00%
crates/acp-nats/src/jsonrpc.rs                                   6       0  100.00%
crates/acp-nats/src/lib.rs                                      73       0  100.00%
crates/acp-nats/src/ext_method_name.rs                          85       0  100.00%
crates/acp-nats/src/client_proxy.rs                            196       0  100.00%
crates/acp-nats/src/pending_prompt_waiters.rs                  112       0  100.00%
crates/acp-nats/src/error.rs                                    84       0  100.00%
crates/acp-nats/src/config.rs                                  201       0  100.00%
crates/acp-nats/src/in_flight_slot_guard.rs                     32       0  100.00%
crates/acp-nats/src/acp_prefix.rs                               63       0  100.00%
crates/acp-nats/src/session_id.rs                               88       0  100.00%
crates/trogon-std/src/time/system.rs                            24       0  100.00%
crates/trogon-std/src/time/mock.rs                             123       0  100.00%
crates/trogon-std/src/env/in_memory.rs                          81       0  100.00%
crates/trogon-std/src/env/system.rs                             17       0  100.00%
crates/trogon-nats/src/messaging.rs                            533       4  99.25%   141-146, 156-157
crates/trogon-nats/src/client.rs                                25      25  0.00%    50-89
crates/trogon-nats/src/mocks.rs                                304       0  100.00%
crates/trogon-nats/src/auth.rs                                 114       3  97.37%   45-47
crates/trogon-nats/src/connect.rs                               96      16  83.33%   22-24, 37, 49, 68-151
TOTAL                                                        15983     198  98.76%

Diff against main

Filename                                    Stmts    Miss  Cover
----------------------------------------  -------  ------  --------
crates/acp-nats/src/agent/prompt.rs             0      -1  +0.09%
crates/acp-nats/src/jetstream/streams.rs       -7       0  +100.00%
TOTAL                                          -7      -1  +0.01%

Results for commit: 305ca67

Minimum allowed coverage is 95%

♻️ This comment has been updated with latest results

@yordis
yordis force-pushed the refactor/notifications-file-storage branch 3 times, most recently from 838df76 to 6718e8d Compare March 31, 2026 15:47
All JetStream streams now use file-backed storage for consistent
security audit coverage.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis force-pushed the refactor/notifications-file-storage branch from 6718e8d to 305ca67 Compare March 31, 2026 15:59
@yordis
yordis merged commit 8dc7e33 into main Mar 31, 2026
7 checks passed
@yordis
yordis deleted the refactor/notifications-file-storage branch March 31, 2026 16:12
yordis added a commit that referenced this pull request Jun 16, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 22, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 22, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 23, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 23, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 23, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 25, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 25, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 26, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 26, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 26, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 26, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis added a commit that referenced this pull request Jun 26, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant