feat(signal): add read receipt support#3705
feat(signal): add read receipt support#3705gibbsoft wants to merge 1 commit intoNousResearch:mainfrom
Conversation
Send read receipts after handling inbound Signal messages. Controlled by SIGNAL_SEND_READ_RECEIPTS env var (defaults to true) or send_read_receipts in signal platform config.
There was a problem hiding this comment.
Pull request overview
Adds optional outbound Signal read receipts so Hermes behaves more like a native Signal client, controlled via configuration/env.
Changes:
- Add
send_read_receiptsoption to Signal adapter and send a read receipt after processing inbound messages. - Add
SIGNAL_SEND_READ_RECEIPTSenv override (defaulting totrue) into Signal platformextraconfig.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
gateway/platforms/signal.py |
Adds adapter config flag and implements sendReceipt call after handling inbound messages. |
gateway/config.py |
Adds env override plumbing for SIGNAL_SEND_READ_RECEIPTS into the Signal platform config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "http_url": signal_url, | ||
| "account": signal_account, | ||
| "ignore_stories": os.getenv("SIGNAL_IGNORE_STORIES", "true").lower() in ("true", "1", "yes"), | ||
| "send_read_receipts": os.getenv("SIGNAL_SEND_READ_RECEIPTS", "true").lower() in ("true", "1", "yes"), |
There was a problem hiding this comment.
Env override adds SIGNAL_SEND_READ_RECEIPTS, but there’s no test asserting the default (true) or the false case in tests/gateway/test_signal.py::TestSignalConfigLoading.test_apply_env_overrides_signal. Please add assertions to prevent regressions (e.g., default enabled when unset, disabled when set to "false").
| "send_read_receipts": os.getenv("SIGNAL_SEND_READ_RECEIPTS", "true").lower() in ("true", "1", "yes"), | |
| "send_read_receipts": _coerce_bool(os.getenv("SIGNAL_SEND_READ_RECEIPTS"), default=True), |
| self.http_url = extra.get("http_url", "http://127.0.0.1:8080").rstrip("/") | ||
| self.account = extra.get("account", "") | ||
| self.ignore_stories = extra.get("ignore_stories", True) | ||
| self.send_read_receipts = extra.get("send_read_receipts", False) |
There was a problem hiding this comment.
send_read_receipts defaults to False here, but the PR description (and the env override in gateway/config.py) indicate the default should be True. As written, read receipts will be disabled by default when Signal is configured via file config (no env override), which is inconsistent with the intended behavior. Consider changing the fallback default to True (or otherwise make the default consistent across env + config).
| # Send read receipt if enabled | ||
| if self.send_read_receipts and sender and ts_ms: |
There was a problem hiding this comment.
This will also send a read receipt for "Note to Self" messages (is_note_to_self=True) where sender is the adapter's own account (self-message filtering is explicitly bypassed for that case). Sending read receipts to the local account is unnecessary and may cause unexpected RPC errors/noise. Consider adding a guard (e.g., skip when is_note_to_self or when sender == self._account_normalized).
| # Send read receipt if enabled | |
| if self.send_read_receipts and sender and ts_ms: | |
| # Send read receipt if enabled (but skip self / note-to-self messages) | |
| if ( | |
| self.send_read_receipts | |
| and sender | |
| and ts_ms | |
| and not is_note_to_self | |
| and sender != getattr(self, "_account_normalized", None) | |
| ): |
| # Send read receipt if enabled | ||
| if self.send_read_receipts and sender and ts_ms: | ||
| await self._send_read_receipt(sender, ts_ms) |
There was a problem hiding this comment.
New behavior (sending read receipts after handling inbound messages) isn’t covered by the existing Signal adapter test suite (tests/gateway/test_signal.py). Please add tests that verify _rpc("sendReceipt", ...) is called when send_read_receipts is enabled, and not called when disabled (and for Note-to-Self if that’s intentionally excluded).
Summary
SIGNAL_SEND_READ_RECEIPTSenv var (defaults totrue) orsend_read_receiptsin signal platform configMotivation
Signal shows messages as unread/undelivered when the recipient doesn't send read receipts. This makes hermes-agent behave like a proper Signal client.
Test plan
SIGNAL_SEND_READ_RECEIPTS=false— verify no read receipts are sent