Skip to content

Implement connection protocol hooks#137

Merged
leynos merged 1 commit intomainfrom
codex/implement-protocol-hooks-and-tests
Jun 26, 2025
Merged

Implement connection protocol hooks#137
leynos merged 1 commit intomainfrom
codex/implement-protocol-hooks-and-tests

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Jun 26, 2025

Summary

  • add ProtocolHooks structure for connection callbacks
  • invoke hooks in ConnectionActor
  • test hook behaviour in connection_actor tests
  • export ProtocolHooks via library module
  • mark roadmap item complete

Testing

  • cargo fmt --all
  • mdformat-all
  • cargo clippy --all-targets --all-features -- -D warnings
  • RUSTFLAGS="-D warnings" cargo test --quiet

https://chatgpt.com/codex/tasks/task_e_685ca1e53108832295f719eb4aef1e14

Summary by Sourcery

Implement ProtocolHooks for connection actors by defining hook callbacks, integrating them into ConnectionActor, exposing them in the public API, and adding tests to validate hook execution.

New Features:

  • Introduce ProtocolHooks struct for customizable connection callbacks
  • Extend ConnectionActor with a with_hooks constructor to inject ProtocolHooks
  • Expose ProtocolHooks in the public library API

Enhancements:

  • Invoke before_send hooks before writing frames and on_command_end hooks after streaming responses

Documentation:

  • Update roadmap documentation to mark internal protocol hooks as complete

Tests:

  • Add tests in connection_actor to verify before_send and on_command_end hook behavior

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Jun 26, 2025

Reviewer's Guide

Introduces a ProtocolHooks abstraction with before_send and on_command_end callbacks, integrates hook invocations into ConnectionActor, exports the hooks via the public API, updates tests to cover hook behavior, and marks the feature complete in the roadmap.

Class diagram for ProtocolHooks and ConnectionActor integration

classDiagram
    class ProtocolHooks {
        +Option<BeforeSendHook<F>> before_send
        +Option<OnCommandEndHook> on_command_end
        +before_send(&mut self, frame: &mut F)
        +on_command_end(&mut self)
    }
    class ConnectionActor {
        +PushQueues<F> queues
        +Option<FrameStream<F, E>> response
        +CancellationToken shutdown
        +ProtocolHooks<F> hooks
        +with_hooks(...)
        +handle_push(&mut self, ...)
        +handle_response(&mut self, ...)
    }
    ConnectionActor --> ProtocolHooks : uses
Loading

Class diagram for ProtocolHooks callback types

classDiagram
    class BeforeSendHook {
        <<type alias>>
        +FnMut(&mut F)
    }
    class OnCommandEndHook {
        <<type alias>>
        +FnMut()
    }
    ProtocolHooks --> BeforeSendHook : before_send
    ProtocolHooks --> OnCommandEndHook : on_command_end
Loading

File-Level Changes

Change Details Files
Introduce ProtocolHooks container and public API export
  • Added new hooks.rs defining ProtocolHooks struct with default and hook methods
  • Defined BeforeSendHook and OnCommandEndHook type aliases
  • Updated lib.rs to pub mod hooks and pub use ProtocolHooks
src/hooks.rs
src/lib.rs
Extend ConnectionActor to accept and invoke hooks
  • Added hooks field and with_hooks constructor
  • Updated new() to delegate to with_hooks with default hooks
  • Modified handle_push and handle_response to call before_send on frames
  • Invoked on_command_end when response stream closes
src/connection.rs
Update and add tests for ProtocolHooks behavior
  • Refactored existing tests to drop push handle before running actor
  • Added before_send_hook_modifies_frames test
  • Added on_command_end_hook_runs test
tests/connection_actor.rs
Mark protocol hooks roadmap item as complete
  • Changed checklist entry for internal protocol hooks from unchecked to checked
docs/asynchronous-outbound-messaging-roadmap.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 26, 2025

Summary by CodeRabbit

  • New Features

    • Added support for custom protocol hooks, allowing actions to be performed before sending frames and after command completion.
  • Documentation

    • Updated the asynchronous outbound messaging roadmap to reflect completed work on protocol hooks.
  • Tests

    • Introduced new tests to verify the behaviour of protocol hooks and ensure correct integration with the connection actor.

Walkthrough

This change introduces internal protocol hooks to the ConnectionActor, allowing custom logic to be executed before sending frames and upon command completion. A new hooks module defines the hook structure and logic. The actor and its methods are updated to invoke these hooks, and new tests verify their behaviour.

Changes

File(s) Change Summary
src/hooks.rs New module defining ProtocolHooks struct with optional before_send and on_command_end hooks.
src/connection.rs ConnectionActor gains a hooks field; methods updated to call hooks; new constructor for custom hooks.
src/lib.rs Adds and re-exports the hooks module and ProtocolHooks.
tests/connection_actor.rs Adds two async tests for hooks; updates existing tests to explicitly drop unused handles.
docs/asynchronous-outbound-messaging-roadmap.md Marks the checklist item for protocol hooks as complete.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ConnectionActor
    participant ProtocolHooks

    User->>ConnectionActor: Send frame or command
    ConnectionActor->>ProtocolHooks: before_send(frame)
    ProtocolHooks-->>ConnectionActor: (frame possibly modified)
    ConnectionActor->>User: Output frame

    alt Command ends
        ConnectionActor->>ProtocolHooks: on_command_end()
    end
Loading

Possibly related PRs

Suggested reviewers

  • codescene-delta-analysis

Poem

A rabbit hops through code anew,
With hooks before each frame goes through.
On command’s end, it gives a cheer,
Custom logic now lives here!
Frames are nudged, commands are done—
This actor’s journey has begun.
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between faffdbe and e1641e2.

📒 Files selected for processing (5)
  • docs/asynchronous-outbound-messaging-roadmap.md (1 hunks)
  • src/connection.rs (6 hunks)
  • src/hooks.rs (1 hunks)
  • src/lib.rs (2 hunks)
  • tests/connection_actor.rs (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/connection_actor.rs (2)
src/connection.rs (3)
  • new (31-37)
  • new (181-190)
  • with_hooks (41-53)
src/hooks.rs (1)
  • default (22-27)
src/connection.rs (1)
src/hooks.rs (1)
  • default (22-27)
🔇 Additional comments (12)
src/lib.rs (1)

13-13: LGTM! Clean API exposure.

The module declaration and re-export properly expose the new hooks functionality to library users following standard Rust conventions.

Also applies to: 23-23

docs/asynchronous-outbound-messaging-roadmap.md (1)

16-17: Documentation correctly updated.

Properly marks the internal protocol hooks feature as complete, aligning with the implementation.

src/hooks.rs (3)

7-11: Well-designed type aliases.

The boxed trait object approach with Send + 'static bounds is appropriate for async contexts and provides good flexibility for hook implementations.


13-19: Clean struct design with optional hooks.

The optional hook fields provide flexibility, allowing users to register only the hooks they need whilst maintaining a simple API.


30-44: Robust hook invocation methods.

The methods properly check for hook presence before invocation, preventing panics and providing clean semantics for optional hooks.

tests/connection_actor.rs (3)

53-54: Good resource cleanup improvement.

Explicitly binding and dropping the handle ensures proper resource management in the test setup.

Also applies to: 91-92, 115-116


158-179: Comprehensive test for before_send hook.

This test effectively verifies that the before_send hook can modify frames and that modifications are reflected in the output. The test covers both push queue frames (incremented from 1 to 2) and response stream frames (incremented from 2 to 3).


181-205: Well-designed test for on_command_end hook.

Using an atomic counter to verify the hook executes exactly once is an elegant approach. The test correctly verifies the hook is called when the response stream ends.

src/connection.rs (4)

31-37: Clean constructor delegation pattern.

The delegation from new() to with_hooks() with default hooks maintains backward compatibility whilst adding the new functionality.


41-53: Well-designed constructor for custom hooks.

The with_hooks constructor provides a clean way to inject custom protocol hooks whilst maintaining the same parameter order as the original constructor.


139-142: Proper before_send hook integration.

The hook is correctly called with a mutable reference to the frame before it's added to the output, allowing for frame modification as demonstrated in the tests.


159-162: Consider on_command_end hook timing.

The on_command_end hook is only called when the response stream ends normally (None case), not on errors (Some(Err(_))). This may be intentional design, but please verify this behaviour aligns with the expected semantics for command completion.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in Comment
  • Commit Unit Tests in branch codex/implement-protocol-hooks-and-tests

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @leynos - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Jun 26, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 26, 2025

✅ Actions performed

Review triggered.

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.

@leynos leynos merged commit 3c2cb4d into main Jun 26, 2025
5 checks passed
@leynos leynos deleted the codex/implement-protocol-hooks-and-tests branch June 26, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant