-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat: add support for custom nostr relays via environment variable #824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@pypeaday is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe relay URL list was renamed from a static constant to Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
libraries/nestjs-libraries/src/integrations/social/nostr.provider.tsOops! Something went wrong! :( ESLint: 8.57.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@eslint/eslintrc' imported from /eslint.config.mjs ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libraries/nestjs-libraries/src/integrations/social/nostr.provider.ts (3)
17-24
: Drop the duplicated default relay entry.
wss://relay.primal.net
appears twice indefaultRelays
.
Although the finalSet
removes the duplication, keeping the source list clean avoids accidental regressions (e.g. if someone later removes theSet
deduplication).const defaultRelays = [ 'wss://relay.primal.net', 'wss://relay.damus.io', 'wss://relay.snort.social', 'wss://nostr.wine', 'wss://nos.lol', - 'wss://relay.primal.net', ];
26-28
: Harden the env-relay parsing (basic URL validation).
extraRelaysFromEnv
accepts any comma-separated token, which could include malformed strings.
A quick sanity filter prevents accidental connection attempts to invalid URLs:-const extraRelaysFromEnv = - process.env.POSTIZ_NOSTR_EXTRA_RELAYS?.split(',').map((url: string) => url.trim()).filter((url: string) => url) || []; +const extraRelaysFromEnv = + (process.env.POSTIZ_NOSTR_EXTRA_RELAYS ?? '') + .split(',') + .map((url) => url.trim()) + .filter( + (url) => + url && + /^(ws|wss):\/\/.+/i.test(url) // basic scheme check + );This keeps the public interface unchanged while guarding against obvious bad inputs.
26-28
: Consider a clearer name thanlist
.
list
shadows the earlierdefaultRelays
concept and is fairly generic.
Renaming to something likerelays
improves readability throughout the class.No action required if you prefer the current name.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libraries/nestjs-libraries/src/integrations/social/nostr.provider.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-publish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also change the .env.example with an example to understand how to implement it
I pushed a change so that the env var is for overrides instead of extending the default - because at least for me I would want to only post to my local relay in dev, not my local relay and all the default ones... Example now has the defaults listed I'm not a JS dev at all so definitely fine with this not going in as is and happy to make any changes necessary for the feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.env.example (2)
74-75
: Clarify override vs. extension behavior
The comment on line 74 says “override or extend,” but the implementation either fully replaces the default list or merges with it (depending on how you handleNOSTR_RELAYS_OVERRIDE
). Please update the wording to accurately describe whether custom relays completely replace the defaults or are appended (with deduplication).
75-75
: Verify quoting in.env
values
The URLs are wrapped in quotes, which depending on your loader might become part of the value. Confirm that the parser strips them as intended or consider dropping the surrounding quotes for a plain comma-separated list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example
(1 hunks)libraries/nestjs-libraries/src/integrations/social/nostr.provider.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libraries/nestjs-libraries/src/integrations/social/nostr.provider.ts
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 75-75: [QuoteCharacter] The value has quote characters (', ")
[warning] 75-75: [UnorderedKey] The NOSTR_RELAYS_OVERRIDE key should go before the PINTEREST_CLIENT_ID key
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-publish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but @nevo-david needs to test it, as I can't verify it really works.
What kind of change does this PR introduce?
feature to allow users to add other nostr relays to publish to - useful for adding local relays for testing
Why was this change needed?
I wanted to publish from postiz to my self-hosted nostr relay, but unless I run postiz from a local fork and build then I can't affect which relays get hit
Other information:
I have discussed this with no one but it arose from thinking through a but I've opened about not being able to post to nostr at all anyways #772
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Summary by CodeRabbit