Repository navigation
Preserve relay origin when replying by local event ID - #174
signet-bot-codex wants to merge 1 commit into
Conversation
A reply to an imported (relayed) message can name the event number the receiving device shows for it. send stored that bare local number in reply_to, so on the origin device it pointed at an unrelated local event, and anything that matches replies to requests across machines lost the link. When the referenced event is an imported message, send now records reply_to as the origin reference "<origin id>:<device short>" taken from its _relay metadata. It keeps reply_to_local for local use. Qualified references and genuine local references are unchanged. A malformed or ambiguous relay identity is refused instead of exporting a misleading number, and the refusal comes before any inline bundle is created. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
[Medium impact] The change affects how replies are linked across relay-connected devices. Appears safe to merge, with a non-blocking concern about reply speed on large message histories. Findings
SummaryBare replies to imported messages now store the origin reference in
|
| let short = relay.get("short")?.as_str()?; | ||
| let qualified = format!("{origin_id}:{short}"); | ||
| crate::relay::control::split_device_suffix(&qualified)?; | ||
| (resolve_reply_to_local(db, &qualified) == Some(local_id)).then_some(qualified) |
There was a problem hiding this comment.
Each valid bare reply to an imported message now searches every message to rule out duplicate origins. resolve_reply_to_local checks JSON fields with no matching index, and both cmd_send and send_message run this check. Reply time therefore grows with the whole message history, unlike the previous lookup by local ID. Add an index for the origin fields so this required check stays cheap.
There was a problem hiding this comment.
Thanks, confirmed. EXPLAIN QUERY PLAN for the resolver's query is SEARCH events USING INDEX idx_type_instance (type=?), so it visits every message row and evaluates the JSON. On a database with about 70,000 messages a lookup that finds no match takes about 0.3 s. Qualified --reply-to <id>:<device> replies have paid this once since #125; with this change a bare reply to an imported message pays it up to twice (the early refusal in cmd_send and the check in send_message), and replies to local messages pay nothing new. The proper fix is an index on the two _relay fields, for example a partial expression index on json_extract(data, '$._relay.short') and json_extract(data, '$._relay.id') where type = 'message'. It would speed up the existing qualified path too, so I would rather offer it as a separate change than widen this one. Happy to add it here if you prefer.
There was a problem hiding this comment.
Thanks for confirming and measuring the impact. Please keep the index as a separate change: this PR can stay focused on portable reply references, while the expression index can address the scan for both the existing qualified path and the new validation path. The measured 0.3 s lookup is useful context for prioritizing that follow-up.
In practice: when an agent asks an agent on another device something, the answer is delivered, but it does not show up as the reply to that question: it links to an unrelated message or to nothing.
When two hcom devices are joined over the relay, a reply written with the event number the replying device shows does not point
back at the message it answers.
Example: an agent on device A sends a request. Its event ID on A is 42, and A's device short is BOXE. The request reaches device B and
is stored there as local event 100. The agent on B answers with
hcom send --reply-to 100.Before this change:
reply_to = "100", its own local row.After this change:
reply_to = "42:BOXE", the origin reference, and keepsreply_to_local = 100for its own use.Invalid --reply-to: relay origin is malformed or ambiguous, so no reply is sent with a misleading number.Details:
--reply-toresolves to an imported message (one with_relaymetadata),reply_tois stored as<origin id>:<device short>.--reply-to 42:BOXE) and references to genuine local messages are stored as before.the same local message. Otherwise the send is refused, before any message or inline bundle is created. Unknown or ambiguous
qualified references keep the existing
Invalid --reply-to: event not found or not a messageerror.<id>:<device>references to the local row; this one makes a barelocal reference to an imported message portable when it is stored.
reply_to. No protocol, schema or configuration change, andreply_to_localhandling on import is unchanged.
Tests, in
src/commands/send.rs:are unchanged;
inline bundle flags are given.
cargo testpasses.