Conversation
|
@codex can you review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 299aa3b5fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for tag in event.tags.iter() { | ||
| let key = tag.kind().to_string(); | ||
| if key == "channel" || key == "h" { | ||
| if key == "h" { |
There was a problem hiding this comment.
Reject legacy channel-tagged events instead of treating them global
This change makes extract_channel_id accept only h, but handle_event enforces membership and archived-channel checks only when a channel_id is found. Any client still emitting the previous channel tag format (used before this migration) will now be stored as an unscoped global event, which bypasses private-channel authorization and archived-channel guards rather than failing closed. Please keep legacy parsing during rollout or explicitly reject channel message kinds that lack h.
Useful? React with 👍 / 👎.
| return { | ||
| kinds: [40001], | ||
| "#e": [channelId], | ||
| "#h": [channelId], |
There was a problem hiding this comment.
Keep desktop history compatible with pre-migration events
The desktop subscription filter now sends only #h, but existing stored messages from before this migration were written with legacy channel tags. Relay filter evaluation still requires literal tag matches (sprout-core/src/filter.rs), so those historical events stop matching channel history subscriptions and channels can appear empty until new h-tagged messages arrive. A temporary dual-read strategy (new + legacy tags) is needed to avoid dropping visible history.
Useful? React with 👍 / 👎.
| .custom_tag( | ||
| nostr::SingleLetterTag::lowercase(nostr::Alphabet::H), | ||
| [p.channel_id.as_str()], | ||
| ) |
There was a problem hiding this comment.
Preserve MCP canvas reads for legacy-tagged canvas events
get_canvas now queries only by #h, but older canvas events were written with the previous channel tag format. Since matching is tag-literal, pre-migration canvas documents will no longer be returned, causing MCP to report "No canvas set" even when a canvas already exists. Add a fallback path for legacy tags (or migrate existing canvas events) before removing old-read compatibility.
Useful? React with 👍 / 👎.
Summary
Testing