Skip to content

Preserve relay origin when replying by local event ID - #174

Open
signet-bot-codex wants to merge 1 commit into
aannoo:mainfrom
signet-bot-codex:fix/reply-to-relay-origin
Open

signet-bot-codex wants to merge 1 commit into
aannoo:mainfrom
signet-bot-codex:fix/reply-to-relay-origin

Conversation

@signet-bot-codex

@signet-bot-codex signet-bot-codex commented Oct 22, 2026 •

Copy link
Copy Markdown

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:

  • B stores reply_to = "100", its own local row.
  • On A, event 100 is some unrelated event (or none at all), so A cannot match the reply to its request 42.

After this change:

  • B stores reply_to = "42:BOXE", the origin reference, and keeps reply_to_local = 100 for its own use.
  • If B's copy of the request has malformed or ambiguous relay metadata, the send is refused with
    Invalid --reply-to: relay origin is malformed or ambiguous, so no reply is sent with a misleading number.

Details:

  • When --reply-to resolves to an imported message (one with _relay metadata), reply_to is stored as
    <origin id>:<device short>.
  • Qualified references (--reply-to 42:BOXE) and references to genuine local messages are stored as before.
  • For a bare reference to an imported message, the origin ID and device short must form a qualified reference that resolves back to
    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 message error.
  • Follows fix(send): resolve remote reply origin from relay metadata #125 (which fixed Remote reply-to device IDs can resolve an unrelated local event #124). That change resolves qualified <id>:<device> references to the local row; this one makes a bare
    local reference to an imported message portable when it is stored.
  • Scope: only what the sending device stores in reply_to. No protocol, schema or configuration change, and reply_to_local
    handling on import is unchanged.

Tests, in src/commands/send.rs:

  • a bare reply to an imported message stores the origin reference and inherits its thread; qualified and colliding local references
    are unchanged;
  • duplicate relay identities, a non-integer origin ID and a malformed device short are refused with no event stored, including when
    inline bundle flags are given.

cargo test passes.

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>
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dac5f497-5d32-4878-b507-a0ad00debb2b
📥 Commits

Reviewing files that changed from the base of the PR and between de33e31 and 45178f3.

📒 Files selected for processing: 1
  • src/commands/send.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Reply handling now stores a portable relay reference for imported messages addressed by local event ID. The send command rejects malformed or ambiguous relay identities before creating inline bundles. Qualified references and genuine local references retain their supplied form.

Changes: Reply reference handling

Layer / File(s) Summary
Validate and store portable reply references
src/commands/send.rs
portable_reply_to validates imported-message relay metadata and preserves qualified or genuine local references. cmd_send checks the reference before inline bundle creation, and send_message stores the portable reference. Tests cover preserved references and refusals without creating message or bundle events.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: aannoo

Merge Risk: ⚪ Minimal · up to 45178

Replies to imported messages retain a portable origin reference. No unresolved merge-blocking risk is established; normal checks remain appropriate.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title states the main change: when a reply uses a local event ID that points to an imported relay message, the stored reply keeps the relay origin reference. The raw summary confirms this behavior…
Linked Issues check Passed Issue #124 is closed and supplies historical context only. No active directly linked issue remains, so this pull request has no linked-issue coding requirements.
Out of Scope Changes check Passed The reported change is limited to src/commands/send.rs and implements the pull request scope: portable relay-origin storage for bare local references, validation before event or inline-bundle creati…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files.
Description check Passed The pull request description directly explains the relay-origin reply behavior, validation rules, unchanged cases, tests, and reported test status. It matches the changeset.
Full details: Title check

Explanation

The title states the main change: when a reply uses a local event ID that points to an imported relay message, the stored reply keeps the relay origin reference. The raw summary confirms this behavior in src/commands/send.rs, and the description and objectives describe the same change. The title is specific and concise, and it describes the primary change from the developer's point of view.

Full details: Out of Scope Changes check

Explanation

The reported change is limited to src/commands/send.rs and implements the pull request scope: portable relay-origin storage for bare local references, validation before event or inline-bundle creation, and unchanged qualified and local reference behavior. No unrelated change is identified.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5 Tier: apex

[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

  1. P2 Replies slow as history grows ▶

Summary

Bare replies to imported messages now store the origin reference in reply_to, while keeping the local number in reply_to_local.

  • Replies to imported messages carry the original device's event reference.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["--reply-to 100"] --> B["Read local message 100"]
  B --> C{"Imported message?"}
  C -->|No| D["Keep supplied reference"]
  C -->|Yes| E["Build origin reference: 42:BOXE"]
  E --> F{"Resolves uniquely to row 100?"}
  F -->|Yes| G["Store reply_to=42:BOXE and reply_to_local=100"]
  F -->|No| H["Refuse before creating an inline bundle"]
Loading

Reviews (1) · Last reviewed commit: "Preserve relay origin when replying by l..." · Reviewed by Greptile

Comment thread src/commands/send.rs
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Replies slow as history grows

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote reply-to device IDs can resolve an unrelated local event

1 participant