feat(messages): carry app-defined extra tags on kind-9 chat/reply#767
Conversation
Add extra_tags to the Chat/Reply intents and expose send_message_with_tags / reply_to_message_with_tags so host apps can attach out-of-band metadata (e.g. a message-effect key) as a real nostr kind-9 tag instead of smuggling it into the visible message body. Marmot stays agnostic about tag meaning; only malformed (empty) tags are dropped. Routes through the existing SendAppEvent worker path, so no worker/command-layer changes are needed.
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: Chapters generated by Stage for commit faddb20 on Jul 1, 2026 11:38pm UTC. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds an ChangesExtra Tags Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/marmot-app/src/messages/intents.rs`:
- Around line 149-160: The sanitized_extra_tags helper currently only removes
empty tags, so host-supplied imeta can still pass through and later be rejected
by decode_received_event during media validation. Update sanitized_extra_tags to
treat imeta as a reserved tag and filter it out before returning the tag list,
or otherwise clearly reserve it in this path. Use the sanitized_extra_tags
function and its call sites for Chat event outgoing tag preparation to locate
the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c779cd11-1370-4c30-aad2-5be60e512d75
📒 Files selected for processing (4)
crates/marmot-app/src/client/mod.rscrates/marmot-app/src/messages/intents.rscrates/marmot-app/src/runtime/mod.rscrates/marmot-app/src/tests.rs
erskingardner
left a comment
There was a problem hiding this comment.
P1: extra_tags lets callers inject reserved kind-9 semantics, not just out-of-band metadata.
sanitized_extra_tags drops only empty tags, then appends everything else to chat/reply events. But timeline projection treats kind-9 tags as first-class semantics: q/e make a row a reply, imeta makes it media, and stream* makes it an agent stream final. Since the new public runtime API accepts arbitrary Vec<Vec<String>>, a plain text send can be projected as a reply/media/stream message by passing reserved tags. That contradicts the “out of band / clients ignore it” framing for message effects.
I’d fix by whitelisting the intended extension tag, e.g. only ["effect", value], or by rejecting feature-owned/reserved tag names (e, q, imeta, stream, stream-start, stream-hash, etc.) from this helper. If this is meant to be a fully general app-event authoring escape hatch, I’d rename/document it that way rather than attaching it to chat/reply.
|
ok I'll add the rejection |
Address Jeff's P1: extra_tags let callers inject reserved kind-9 semantics (e/q/imeta/stream*), so a plain text send could be projected as a reply/media/stream message. sanitized_extra_tags now whitelists a well-formed ["effect", <value>] tag and drops everything else.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/marmot-app/src/messages/intents.rs`:
- Around line 168-170: The tag filter in the intent/message parsing logic is
currently allowing `effect` tags with extra trailing fields; tighten the match
in the `matches!` expression so `EFFECT_TAG` is accepted only when the tag has
exactly two elements, and apply the same validation anywhere else the `effect`
tag is parsed or filtered (including the other referenced locations) to keep the
`["effect", <value>]` shape enforced consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12f66c39-019f-4498-8d8c-a2f1a95b3515
📒 Files selected for processing (1)
crates/marmot-app/src/messages/intents.rs
Tighten sanitized_extra_tags so an effect tag with trailing fields is dropped too — enforce exactly two elements, not a two-or-more prefix.
What this does
Message effects (the Telegram-style bursts) used to travel inside the message body. The sender appended an invisible marker like
dmfx:fireto the text, and the receiver stripped it back off. Any client that did not know the trick showed the rawdmfx:firesuffix or a tofu box, and the control characters leaked into copy, quote, reply previews, and search.This moves the effect out of the body and onto the kind-9 event as a real nostr tag. Like a tidy marmot stamping a label on the envelope instead of scribbling inside the letter.
Changes
ChatandReplyintents gain anextra_tags: Vec<Vec<String>>field.build_inner_eventappends those tags after the protocol-derived ones. Only malformed (empty) tags are dropped; marmot stays agnostic about what a tag means.Runtime::send_message_with_tagsandRuntime::reply_to_message_with_tags. They route through the existingSendAppEventworker path, so the worker and command layers did not change.Why a tag
A nostr tag is out of band. A client that does not understand it ignores it and renders clean text, so no marker ever appears in the visible message. The receive side already carried arbitrary inner-event tags through the projection onto
AppMessageRecord.tags, so reading the effect back needs no protocol change.The host app (darkmatter-linux) owns the convention and sends
["effect", "fire"]. Marmot just carries whatever tags it is handed.Summary by CodeRabbit
send_message_with_tagsto post tagged messages, plusreply_to_message_with_tagsfor tagged replies.["effect", <value>]tags with a non-empty value are accepted; others (including reserved/empty forms) are ignored.