ultanio/cobot
4
1
Fork 4
Code Issues 84 Pull requests 10 Projects Releases Packages Wiki Activity Actions

feat: communication event hooks (on_received, on_sent) #232

Open
doxios wants to merge 2 commits from feat/communication-events into main
pull from: feat/communication-events
merge into: ultanio:main
Conversation 2 Commits 2 Files changed 2 +137 -1
doxios commented 2026-03-08 21:20:19 +00:00
Collaborator
Copy link

Summary

Add two event extension points to the communication plugin:

  • communication.on_received — fires for each message after polling
  • communication.on_sent — fires after a message is successfully sent

Why

Observer plugins (archival, logging, analytics) need to capture message flow regardless of channel type. Currently the only way is to hook into channel-specific extension points (e.g. telegram.on_message), which couples observers to specific channels.

These events fire at the communication layer, so any plugin implementing them sees messages from all channels.

Changes

  • cobot/plugins/communication/plugin.py:
    • Add communication.on_received and communication.on_sent to extension_points
    • poll() fires on_received for each polled message
    • send() fires on_sent after successful send
    • Add _notify() helper for dispatching to implementers

Scope

Single plugin change (communication only). No breaking changes — existing implementations are unaffected.

Related

  • PR #208 (telegram-lurker) will be refactored to use these hooks instead of telegram.on_message
## Summary Add two event extension points to the communication plugin: - **`communication.on_received`** — fires for each message after polling - **`communication.on_sent`** — fires after a message is successfully sent ## Why Observer plugins (archival, logging, analytics) need to capture message flow regardless of channel type. Currently the only way is to hook into channel-specific extension points (e.g. `telegram.on_message`), which couples observers to specific channels. These events fire at the communication layer, so any plugin implementing them sees messages from all channels. ## Changes - `cobot/plugins/communication/plugin.py`: - Add `communication.on_received` and `communication.on_sent` to `extension_points` - `poll()` fires `on_received` for each polled message - `send()` fires `on_sent` after successful send - Add `_notify()` helper for dispatching to implementers ## Scope Single plugin change (communication only). No breaking changes — existing implementations are unaffected. ## Related - PR #208 (telegram-lurker) will be refactored to use these hooks instead of `telegram.on_message`
feat: add communication.on_received and communication.on_sent event hooks
All checks were successful
CI / lint (pull_request) Successful in 10s
Details
CI / test (3.11) (pull_request) Successful in 23s
Details
CI / test (3.12) (pull_request) Successful in 25s
Details
CI / test (3.13) (pull_request) Successful in 26s
Details
E2E Tests / e2e (pull_request) Successful in 20s
Details
CI / build (pull_request) Successful in 9s
Details
7366278a37 
Extend the communication plugin with two event extension points:
- communication.on_received: fires for each message after polling
- communication.on_sent: fires after a message is successfully sent

These enable observer plugins (like lurker/archival) to capture all
message flow regardless of channel type (telegram, discord, etc.).
doxios commented 2026-03-08 21:24:33 +00:00
Author
Collaborator
Copy link

🦊 PR Review: #232 — feat: communication event hooks

Summary

Adds communication.on_received and communication.on_sent event extension points to the communication plugin, enabling observer plugins to capture message flow across all channels.

Checklist (Plugin Design Guide)

  • Extension points follow <plugin_id>.<hook_name> naming convention
  • No edits required to existing plugins for consumers to hook in
  • No os.environ reads
  • No tool definitions (not applicable)
  • No hidden coupling — _notify uses _registry.get_implementations() which is the standard pattern
  • Priority unchanged (5) — appropriate for foundation-level plugin

Findings

🟢 Clean extension point design — New hooks follow the established call_extension / implements pattern. Observer plugins can simply declare implements: {"communication.on_received": "my_handler"} and they're wired in automatically.

🟢 No breaking changes — Existing implementations are unaffected. The new extension points are purely additive.

🟢 All checks pass — ruff check ✅, ruff format ✅, 1004 tests pass ✅

🟡 Minor: Untyped message parameter in _notify() — The message parameter is typed as bare message without a type hint. Consider message: IncomingMessage | OutgoingMessage (or Union[...]) for clarity and IDE support.

🟡 Minor: No tests for the new hooks — While the existing test suite passes, there are no tests verifying that on_received fires during poll() or on_sent fires during send(). Consider adding a test with a mock implementer.

Verdict: Approve ✅

Clean, minimal, well-scoped change that follows the plugin design guide. The two minor items (typing, tests) are nice-to-haves and don't block merge.

## 🦊 PR Review: #232 — feat: communication event hooks ### Summary Adds `communication.on_received` and `communication.on_sent` event extension points to the communication plugin, enabling observer plugins to capture message flow across all channels. ### Checklist (Plugin Design Guide) - [x] Extension points follow `<plugin_id>.<hook_name>` naming convention - [x] No edits required to existing plugins for consumers to hook in - [x] No `os.environ` reads - [x] No tool definitions (not applicable) - [x] No hidden coupling — `_notify` uses `_registry.get_implementations()` which is the standard pattern - [x] Priority unchanged (5) — appropriate for foundation-level plugin ### Findings 🟢 **Clean extension point design** — New hooks follow the established `call_extension` / `implements` pattern. Observer plugins can simply declare `implements: {"communication.on_received": "my_handler"}` and they're wired in automatically. 🟢 **No breaking changes** — Existing implementations are unaffected. The new extension points are purely additive. 🟢 **All checks pass** — ruff check ✅, ruff format ✅, 1004 tests pass ✅ 🟡 **Minor: Untyped `message` parameter in `_notify()`** — The `message` parameter is typed as bare `message` without a type hint. Consider `message: IncomingMessage | OutgoingMessage` (or `Union[...]`) for clarity and IDE support. 🟡 **Minor: No tests for the new hooks** — While the existing test suite passes, there are no tests verifying that `on_received` fires during `poll()` or `on_sent` fires during `send()`. Consider adding a test with a mock implementer. ### Verdict: **Approve** ✅ Clean, minimal, well-scoped change that follows the plugin design guide. The two minor items (typing, tests) are nice-to-haves and don't block merge.
fix: add type hint to _notify() and tests for event hooks
All checks were successful
CI / lint (pull_request) Successful in 10s
Details
CI / test (3.11) (pull_request) Successful in 21s
Details
CI / test (3.12) (pull_request) Successful in 24s
Details
CI / test (3.13) (pull_request) Successful in 24s
Details
E2E Tests / e2e (pull_request) Successful in 15s
Details
CI / build (pull_request) Successful in 6s
Details
5ad69122d9 
- Add Union[IncomingMessage, OutgoingMessage] type hint to _notify()
- Add tests: on_received fires on poll, on_sent fires on send,
  no event on failed send, extension points in meta
- 16 tests passing (was 13)
doxios commented 2026-03-09 05:30:36 +00:00
Author
Collaborator
Copy link

🦊 PR Review — #232: feat: communication event hooks

Verdict: ✅ Approve

Clean, well-scoped addition of observer event hooks to the communication plugin.

Findings

🟢 Good: Clean abstraction — _notify() helper follows existing patterns (send, typing) with proper error handling per implementer.

🟢 Good: Event semantics — on_received fires after poll (sorted), on_sent fires only on success. Both are correct.

🟢 Good: Test coverage — 3 new tests cover: on_received fires during poll, on_sent fires on success, on_sent does NOT fire on failure.

🟡 Minor: Union import — The Union import is added but Union["IncomingMessage", "OutgoingMessage"] could use IncomingMessage | OutgoingMessage (Python 3.10+ syntax) since the codebase likely targets 3.10+. Not blocking.

🟡 Minor: Pre-existing warnings — Tests emit RuntimeWarning: coroutine CommunicationPlugin.start was never awaited — this is pre-existing (not introduced by this PR) but worth a follow-up cleanup.

Labels

  • Scope: Single-Plugin
  • Kind: Feature
  • Priority: Medium

Automated review by Doxios 🦊

## 🦊 PR Review — #232: feat: communication event hooks ### Verdict: ✅ Approve Clean, well-scoped addition of observer event hooks to the communication plugin. ### Findings 🟢 **Good: Clean abstraction** — `_notify()` helper follows existing patterns (`send`, `typing`) with proper error handling per implementer. 🟢 **Good: Event semantics** — `on_received` fires after poll (sorted), `on_sent` fires only on success. Both are correct. 🟢 **Good: Test coverage** — 3 new tests cover: on_received fires during poll, on_sent fires on success, on_sent does NOT fire on failure. 🟡 **Minor: `Union` import** — The `Union` import is added but `Union["IncomingMessage", "OutgoingMessage"]` could use `IncomingMessage | OutgoingMessage` (Python 3.10+ syntax) since the codebase likely targets 3.10+. Not blocking. 🟡 **Minor: Pre-existing warnings** — Tests emit `RuntimeWarning: coroutine CommunicationPlugin.start was never awaited` — this is pre-existing (not introduced by this PR) but worth a follow-up cleanup. ### Labels - **Scope:** Single-Plugin - **Kind:** Feature - **Priority:** Medium --- *Automated review by Doxios 🦊*
All checks were successful
CI / lint (pull_request) Successful in 10s
Details
CI / test (3.11) (pull_request) Successful in 21s
Details
CI / test (3.12) (pull_request) Successful in 24s
Details
CI / test (3.13) (pull_request) Successful in 24s
Details
E2E Tests / e2e (pull_request) Successful in 15s
Details
CI / build (pull_request) Successful in 6s
Details
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/communication-events:feat/communication-events
git switch feat/communication-events
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
Compat/Breaking

Breaking change that won't be backward compatible

  
Kind/Bug

Something is not working

  
Kind/Competitor

Anything we found interesting at competitory

  
Kind/Documentation

Documentation changes

  
Kind/Enhancement

Improve existing functionality

  
Kind/Epic

Business requirement — groups related stories

  
Kind/Feature

New functionality

  
Kind/Security

This is security issue

  
Kind/Story

Implementable unit with AC — results in a PR

  
Kind/Testing

Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Scope/Core

Changes to core non-plugin code (CLI, config loading, etc.).

  
Scope/Cross-Plugin

Touches multiple existing plugins. Needs extra scrutiny, may require a PRD.

  
Scope/Plugin-System

Changes to plugin infrastructure (base.py, registry.py, interfaces.py, __init__.py, agent.py plugin dispatch). PRD required before merge.

  
Scope/Single-Plugin

Changes contained to one plugin (or adds one new plugin). Standard review.

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No labels
Compat/Breaking
Kind/Bug
Kind/Competitor
Kind/Documentation
Kind/Enhancement
Kind/Epic
Kind/Feature
Kind/Security
Kind/Story
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Scope/Core
Scope/Cross-Plugin
Scope/Plugin-System
Scope/Single-Plugin
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ultanio/cobot!232
No description provided.