Peer Connections & Secure Messaging#73
Open
Ananya44444 wants to merge 3 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds peer connections, peer messaging, and secure inbox features. It introduces three new tables, a new backend module with API handlers, three client-side pages, navbar links, and tests for auth, validation, and dispatch routing. ChangesPeer Connections and Secure Messaging
Sequence Diagram(s)sequenceDiagram
participant Browser
participant api_connect as api_connect (peers.py)
participant api_handle_connection as api_handle_connection (peers.py)
participant api_send_peer_message as api_send_peer_message (peers.py)
participant DB
Browser->>api_connect: POST /api/peers/connect/{userId}
api_connect->>DB: insert peer_connections (status=pending)
Browser->>api_handle_connection: PATCH /api/peers/{id}/accept
api_handle_connection->>DB: update status=accepted, notify sender
Browser->>api_send_peer_message: POST /api/peers/messages/{peerId}
api_send_peer_message->>DB: insert peer_messages (encrypted), update connection timestamp
api_send_peer_message-->>Browser: ok response + notify recipient
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 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 `@public/peer-messages.html`:
- Around line 40-42: The message compose input in the peer messages template
currently relies only on placeholder text, so add an accessible name for the
input identified by message-input in the send-form. Fix this by adding a real
label element with a for attribute tied to message-input, or by providing an
aria-label on the input, so screen readers can reliably identify the control.
- Around line 129-151: The send handler in send-form only disables
message-input, so the submit button can still be clicked and trigger duplicate
POSTs to api/peers/messages/:peer_id. Update the submit flow in the
addEventListener('submit', ...) handler to guard the entire form submission path
by disabling the submit control as well (and re-enabling it in the finally
block), using the existing input disable/restore pattern as the place to wire
this in.
In `@public/peers.html`:
- Around line 39-43: The search input in peers.html has no accessible name
because it relies only on placeholder text. Update the search control near the
searchUsers() button by adding a proper label tied to search-q (an sr-only label
is fine) or an aria-label on the input so assistive technologies can identify it
consistently.
- Around line 103-129: The pending requests UI in renderPending is filtering out
all sent items by keeping only direction === 'received', so sent requests never
appear after loadPeers merges data from both directions. Update renderPending in
peers.html to include the sent pending connections as well, and adjust the
displayed label/text so both received and sent requests are shown correctly in
the single pending list.
In `@public/secure-inbox.html`:
- Around line 38-42: The compose form controls in secure-inbox.html currently
rely only on placeholder text, so add explicit accessible labeling for the
recipient input and message textarea. Update the compose form markup around
compose-form, recipient, and message-body to use visible <label> elements or
equivalent aria-label/aria-labelledby attributes so assistive technologies can
announce both fields correctly.
- Around line 169-186: The secure inbox submit handler in the compose form can
fire multiple times while the `/api/secure/send` request is still pending,
causing duplicate sends. Update the submit flow in the compose form listener to
disable the form controls (or the submit action) before calling `apiFetch`, and
re-enable them in a finally-style path after the request settles, regardless of
success or failure. Use the existing `compose-form`, `recipient`, and
`message-body` elements in the submit handler to locate and lock/unlock the UI
state.
In `@schema.sql`:
- Around line 151-158: The peer_connections schema still allows duplicate
reciprocal rows because UNIQUE(sender_id, receiver_id) does not enforce an
unordered pair; update the CREATE TABLE definition to use a canonical-pair
uniqueness guarantee so A→B and B→A collide, and mirror the same DDL change in
src/worker.py’s _DDL. Make the change in the peer_connections table definition
and ensure api_connect relies on this database-level constraint instead of only
the pre-insert directional check.
In `@src/peers.py`:
- Around line 101-112: The peer search in the users lookup is limiting discovery
too early by only decrypting the newest 200 rows in the logic around
rows.results and matches. Update the search flow in peers.py so exact username
matches use a blind-index lookup first, and if you still need fuzzy name
matching, paginate through bounded decrypt scans until limit matches are
collected instead of stopping at a fixed 200-row window.
- Line 305: Validate the JSON field types before using string methods in the
request handlers that read body values, especially the content/recipient_id
extraction in the logic around parse_json_object(). For the affected spots in
the peer-handling flow, check that each expected field is a string before
calling .strip() or otherwise assuming string behavior, and return a 400
validation error when the type is wrong instead of letting AttributeError bubble
up as a 500.
- Around line 373-376: The secure-message retrieval paths in _format_expires(),
the inbox query, and the download/decrypt flow still return messages after their
7-day expiry. Update the query logic in the affected peers.py code paths to
exclude expired rows before any decryption or plaintext return, and add cleanup
plus tests that verify expired messages are no longer readable through inbox or
download. Use the existing secure_messages selection/decryption functions as the
main touchpoints so the fix applies consistently across both query sites.
- Around line 451-470: The api_secure_download handler currently performs a
destructive delete as a side effect of a GET request. Update this flow so
api_secure_download only returns the decrypted content, and move the deletion of
unstarred secure_messages into an explicit non-GET action such as a POST
acknowledge/delete endpoint or a separate delete endpoint. Keep the existing
auth, lookup, and decrypt logic in api_secure_download, but remove the
env.DB.batch deletion from this GET path and wire the one-time cleanup through a
dedicated handler instead.
- Around line 41-46: The _notify helper currently lets notification creation
failures bubble up from _worker()._create_notification, which can turn an
otherwise successful persisted write into a 500. Update _notify to be
best-effort by catching and logging exceptions around the notification call
(with enough context like user_id/type_/related_id), or restructure the write
path so the DB write and notification are truly atomic; keep the change
localized to _notify and its caller flow in peers.py.
In `@src/worker.py`:
- Around line 2782-2784: The peer module import in the request handler is too
early: `peers_api = _get_peers_module()` runs before checking whether the
request actually matches `/api/peers` or `/api/secure`, so unrelated `/api/*`
paths can fail with an import error instead of 404. Move the lazy import into
the matched branches or guard it with `path.startswith(("/api/peers",
"/api/secure"))` in the `worker.py` routing logic, and keep the existing error
handling/logging around `_get_peers_module()` so import failures are only
surfaced for the relevant peer/secure routes.
In `@tests/helpers.py`:
- Around line 22-27: The load_worker helper currently inserts the new worker
module into sys.modules before exec_module() runs, so a failure leaves a
half-initialized module cached. Update load_worker() to only keep the module in
sys.modules if spec.loader.exec_module(mod) succeeds, and remove the worker
entry from sys.modules in the exception path. Use the existing load_worker,
spec.loader.exec_module, and sys.modules["worker"] symbols to place the cleanup
close to the module-loading logic.
In `@tests/test_peers_api.py`:
- Around line 92-129: Add happy-path coverage for the newly introduced worker
routes and batch-backed flows in peers.api_handle_connection,
peers.api_send_message, peers.api_read_messages, peers.api_secure_send, and
peers.api_secure_download, plus the remaining /api/secure* dispatch branches in
TestPeersDispatch. Use existing symbols like worker.on_fetch, peers.*,
MockDB.batch, and the secure API entrypoints to verify successful responses and
correct route matching so method/regex mistakes or broken batch integration are
caught.
- Around line 17-28: The local test helpers in the test_peers_api module are
missing explicit return type annotations, which now violates the typing rule for
changed Python files. Update the _parse and _auth_header helper definitions to
include precise return annotations that match their actual outputs, and keep the
existing helper behavior unchanged; use the function names _parse, _enc, and
_auth_header to locate and adjust the signatures.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1d21c34-c10e-4b08-8385-42c64de9435f
⛔ Files ignored due to path filters (1)
migrations/0009_add_peer_messaging.sqlis excluded by!**/migrations/**
📒 Files selected for processing (9)
public/partials/navbar.htmlpublic/peer-messages.htmlpublic/peers.htmlpublic/secure-inbox.htmlschema.sqlsrc/peers.pysrc/worker.pytests/helpers.pytests/test_peers_api.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/peers.py`:
- Around line 43-52: Remove the redundant broad Exception handler in the
notification wrapper around _worker()._create_notification, since
_create_notification already owns the best-effort failure handling and logging.
Update the surrounding peer helper so it simply awaits the call without
swallowing unexpected loader/binding errors, and drop the explicit return None
to satisfy Ruff BLE001 and RET501 while keeping the behavior delegated to
_create_notification.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16e8f336-7433-4660-8360-a59c817b78a3
⛔ Files ignored due to path filters (1)
migrations/0009_add_peer_messaging.sqlis excluded by!**/migrations/**
📒 Files selected for processing (8)
public/peer-messages.htmlpublic/peers.htmlpublic/secure-inbox.htmlschema.sqlsrc/peers.pysrc/worker.pytests/helpers.pytests/test_peers_api.py
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
1 participant
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.
Overview
This PR introduces the Peer Connections & Messaging (A4) feature for the Cloudflare Workers-based Learn platform. It adds peer connection management, encrypted peer messaging, and secure messaging while following the existing architecture and conventions used by previous features.
Changes
Database
peer_connectionspeer_messagessecure_messagesBackend
Implemented new REST APIs for:
Peer connection management
Peer messaging
Secure messaging
Security
verify_token)Frontend
Added new pages:
public/peers.htmlpublic/peer-messages.htmlpublic/secure-inbox.htmlFeatures include:
Routing & Navigation
Notes
This PR adds the “Peer Connections & Messaging (A4)” feature to the Cloudflare Workers-based Learn platform, enabling discovery, connection management, encrypted peer messaging, and a secure inbox experience.
Key backend changes include:
peer_connections,peer_messages, andsecure_messagestables with constraints and indexes to support request status, message threading, and inbox/star/read flows./api/peers/*and/api/secure/*for:verify_tokenauthentication and AES-GCM encryption helpers, and worker routing extended to dynamically dispatch these handlers.Key frontend changes include:
public/peers.html,public/peer-messages.html, andpublic/secure-inbox.html, each using Bearer token calls, handling401/403, and providing UI actions (send, refresh, star, download)./peers.htmland peer navigation is exposed in both desktop and mobile menus.Testing updates:
MockDB.batchstub.tests/test_peers_api.pycovering routing, auth/authorization (401/403), validation and constraint edge cases, and secure inbox star/download failure behavior.Overall impact: authenticated users can discover each other, request/accept connections, exchange encrypted messages through peer threads, and manage secure messages via a dedicated secure inbox UI.