fix: update to alby go-nostr fork which reconnects after receiving CLOSED message#2331
fix: update to alby go-nostr fork which reconnects after receiving CLOSED message#2331
Conversation
📝 WalkthroughWalkthroughThis PR replaces the external Nostr library dependency from ChangesNostr Dependency Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
go.mod (1)
10-10: ⚡ Quick winMigration to fork is complete; consider tagging releases for long-term maintainability.
Verification confirms no lingering references to
github.com/nbd-wtf/go-nostrremain—the dependency swap is clean andgo.sumhas been properly regenerated.The fork uses an untagged pseudo-version (
v0.0.0-20260509070347-31e205cac904). For reproducibility and maintenance, consider:
- Cutting tagged releases on the fork, or
- Filing an upstream PR to
nbd-wtf/go-nostrwith the CLOSED-reconnect fix and retiring the fork later.🤖 Prompt for 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. In `@go.mod` at line 10, The go.mod now points to the forked module github.com/getAlby/go-nostr using an untagged pseudo-version; to ensure reproducible builds and long-term maintainability, either create semantic version tags on the fork (e.g., v0.1.0) and update go.mod to use that tag, or prepare a PR against the original module (nbd-wtf/go-nostr) that includes the CLOSED-reconnect fix so you can later switch back and remove the fork—update the dependency entry for github.com/getAlby/go-nostr accordingly once a tagged release or upstream merge is available.service/start.go (1)
250-256: ⚡ Quick winUpdate the inline comment to reflect the fork's automatic reconnection behavior.
The comment at lines 251–252 describes the original go-nostr behavior ("currently if the relay sends a close that is not 'auth-required:' this will trigger closing the subscription channel"). However, this PR updates to the Alby fork, which—as noted in commit 9d625f8—"reconnects after receiving CLOSED message."
Update the comment to clarify the current fork behavior and whether:
- The subscription channel still closes for non-auth CLOSED messages (in which case the outer loop's resubscribe still applies and the comment should reference the fork's behavior), or
- The fork now handles reconnection internally, and
eventsChannelClosedonly fires on context exit or true disconnects (in which case the surrounding error/resubscribe logic may need review to avoid redundant reconnection attempts).Refreshing this comment will prevent future readers from being confused by references to the original upstream behavior.
🤖 Prompt for 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. In `@service/start.go` around lines 250 - 256, Update the inline comment above the eventsChannelClosed case to reflect the Alby fork's behavior: state that the Alby fork (per commit 9d625f8) reconnects after receiving a CLOSED message, then clearly explain whether eventsChannelClosed still fires for non-auth CLOSED messages or only on context cancellation/true disconnects; reference the symbols eventsChannelClosed, logger.Logger.Info("Subscription was exited abnormally"), and the subsequent return errors.New("subscription exited abnormally") and, based on the correct behavior, note whether the outer loop's resubscribe logic is still required or redundant.
🤖 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 `@nip47/notifications/nip47_notifier.go`:
- Line 8: The import and go.mod reference uses a non-public fork
"github.com/getAlby/go-nostr" which cannot be resolved; update the dependency
resolution by either switching imports and go.mod to the upstream
"github.com/nbd-wtf/go-nostr", adding a go.mod replace directive pointing to a
reachable module (or pushing the fork publicly), and ensure all references in
nip47_notifier.go import the corrected module path (the current import string
"github.com/getAlby/go-nostr") so the build can resolve the package; if the fork
is private, add a note in docs about access or change to the upstream module
accordingly.
---
Nitpick comments:
In `@go.mod`:
- Line 10: The go.mod now points to the forked module
github.com/getAlby/go-nostr using an untagged pseudo-version; to ensure
reproducible builds and long-term maintainability, either create semantic
version tags on the fork (e.g., v0.1.0) and update go.mod to use that tag, or
prepare a PR against the original module (nbd-wtf/go-nostr) that includes the
CLOSED-reconnect fix so you can later switch back and remove the fork—update the
dependency entry for github.com/getAlby/go-nostr accordingly once a tagged
release or upstream merge is available.
In `@service/start.go`:
- Around line 250-256: Update the inline comment above the eventsChannelClosed
case to reflect the Alby fork's behavior: state that the Alby fork (per commit
9d625f8) reconnects after receiving a CLOSED message, then clearly explain
whether eventsChannelClosed still fires for non-auth CLOSED messages or only on
context cancellation/true disconnects; reference the symbols
eventsChannelClosed, logger.Logger.Info("Subscription was exited abnormally"),
and the subsequent return errors.New("subscription exited abnormally") and,
based on the correct behavior, note whether the outer loop's resubscribe logic
is still required or redundant.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17fe5d99-a970-447a-ac27-a723a87ada37
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (49)
apps/apps_service.gogo.modnip47/cipher/cipher.gonip47/cipher/cipher_test.gonip47/controllers/cancel_hold_invoice_controller.gonip47/controllers/cancel_hold_invoice_controller_test.gonip47/controllers/create_connection_controller.gonip47/controllers/create_connection_controller_test.gonip47/controllers/get_balance_controller.gonip47/controllers/get_balance_controller_test.gonip47/controllers/get_budget_controller.gonip47/controllers/get_budget_controller_test.gonip47/controllers/get_info_controller.gonip47/controllers/get_info_controller_test.gonip47/controllers/list_transactions_controller.gonip47/controllers/list_transactions_controller_test.gonip47/controllers/lookup_invoice_controller.gonip47/controllers/lookup_invoice_controller_test.gonip47/controllers/make_hold_invoice_controller.gonip47/controllers/make_hold_invoice_controller_test.gonip47/controllers/make_invoice_controller.gonip47/controllers/make_invoice_controller_test.gonip47/controllers/models.gonip47/controllers/multi_pay_invoice_controller.gonip47/controllers/multi_pay_invoice_controller_test.gonip47/controllers/multi_pay_keysend_controller.gonip47/controllers/multi_pay_keysend_controller_test.gonip47/controllers/pay_invoice_controller.gonip47/controllers/pay_invoice_controller_test.gonip47/controllers/pay_keysend_controller.gonip47/controllers/pay_keysend_controller_test.gonip47/controllers/settle_hold_invoice_controller.gonip47/controllers/settle_hold_invoice_controller_test.gonip47/controllers/sign_message_controller.gonip47/event_handler.gonip47/event_handler_test.gonip47/nip47_service.gonip47/notifications/nip47_notifier.gonip47/notifications/nip47_notifier_test.gonip47/publish_nip47_info.gonostr/models/models.goservice/create_app_consumer.goservice/delete_app_consumer.goservice/keys/keys.goservice/keys/keys_test.goservice/start.goservice/update_app_consumer.gotests/create_app.gotests/create_mock_relay.go
| "encoding/json" | ||
| "errors" | ||
|
|
||
| "github.com/getAlby/go-nostr" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the Alby go-nostr fork exists and check the specified commit
# Check if the repository exists and the commit is valid
echo "Checking fork repository and commit..."
git ls-remote https://github.com/getAlby/go-nostr.git 31e205cac904* || echo "Commit not found"
# Check for available tags/releases
echo -e "\nChecking for tagged releases..."
git ls-remote --tags https://github.com/getAlby/go-nostr.git | tail -10Repository: getAlby/hub
Length of output: 127
🏁 Script executed:
# First, find the file mentioned in the review
find . -name "nip47_notifier.go" -type f
# Check if go.mod exists and search for the nostr dependency
find . -name "go.mod" -type f | head -5
# List the repository structure
git ls-files | grep -E "(go.mod|go.sum)" | head -10Repository: getAlby/hub
Length of output: 118
🏁 Script executed:
# Check line 8 of the nip47_notifier.go file
head -15 ./nip47/notifications/nip47_notifier.go
# Check go.mod for nostr dependency
cat ./go.mod | grep -A2 -B2 "nostr"
# Check go.sum for nostr entries
grep "nostr" ./go.sumRepository: getAlby/hub
Length of output: 880
🏁 Script executed:
# Read the actual implementation to check for guideline compliance
cat -n ./nip47/notifications/nip47_notifier.go | head -100
# Check for error handling patterns
grep -n "fmt.Errorf\|errors\|return err" ./nip47/notifications/nip47_notifier.go | head -20
# Check for logging patterns
grep -n "logger\|logrus\|fmt.Print" ./nip47/notifications/nip47_notifier.go | head -20Repository: getAlby/hub
Length of output: 5267
🌐 Web query:
getAlby go-nostr fork GitHub repository commit 31e205cac904
💡 Result:
I couldn’t find any public GitHub page for the commit hash 31e205cac904 in the getAlby “go-nostr”/Nostr Wallet Connect repositories. What I did find: - getAlby has a Go-based Nostr Wallet Connect server repo at https://github.com/getAlby/nostr-wallet-connect [1]. - There is also a forked/related repo at https://github.com/getAlby/nwc.getalby.com (it’s a fork of getAlby/nostr-wallet-connect) [2]. However, none of the searched results surfaced a GitHub commit URL or page matching 31e205cac904 for those repositories (or for a getAlby “go-nostr fork” specifically). So I can’t reliably identify which repository/commit you mean, or provide the commit details. If you share either (a) the exact repository name you mean (e.g., getAlby/nostr-wallet-connect vs getAlby/nwc.getalby.com), or (b) the full commit URL you expect, I can re-run the lookup against that exact path and extract the commit content.
Citations:
🌐 Web query:
getAlby/go-nostr repository GitHub
💡 Result:
There is no GitHub repository at getAlby/go-nostr. What I found instead: 1) The Alby repo getAlby/nostr-wallet-connect (NIP-47 “Nostr Wallet Connect” app) is at https://github.com/getAlby/nostr-wallet-connect and is the main getAlby/Nostr-related project with Go code [1][2]. 2) Another related getAlby repo is getAlby/http-nostr (HTTP endpoints bridging the Nostr protocol with NIP-47) at https://github.com/getAlby/http-nostr [3]. If you meant the Go Nostr library commonly used by these projects, that is github.com/nbd-wtf/go-nostr (separate repository/org) at https://github.com/nbd-wtf/go-nostr [4].
Citations:
- 1: https://github.com/getAlby/nostr-wallet-connect
- 2: https://pkg.go.dev/github.com/getAlby/nostr-wallet-connect
- 3: https://github.com/getAlby/http-nostr
- 4: https://github.com/nbd-wtf/go-nostr
🏁 Script executed:
# Check if go.mod has any replace directives
cat ./go.mod | grep -A5 "replace"
# Check entire go.mod for context
wc -l ./go.mod
head -50 ./go.mod
tail -20 ./go.modRepository: getAlby/hub
Length of output: 3205
Critical: The fork repository github.com/getAlby/go-nostr does not exist publicly.
The dependency in go.mod points to github.com/getAlby/go-nostr v0.0.0-20260509070347-31e205cac904, but this repository does not exist on GitHub. The original library is github.com/nbd-wtf/go-nostr (different organization).
This breaks the build—the dependency cannot be resolved. Either:
- This is a private repository (clarify access requirements in documentation)
- The import path in go.mod is incorrect and should reference the original library
- The fork needs to be pushed to a public repository, or a
replacedirective added to go.mod
Clarify the dependency resolution before merging.
🤖 Prompt for 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.
In `@nip47/notifications/nip47_notifier.go` at line 8, The import and go.mod
reference uses a non-public fork "github.com/getAlby/go-nostr" which cannot be
resolved; update the dependency resolution by either switching imports and
go.mod to the upstream "github.com/nbd-wtf/go-nostr", adding a go.mod replace
directive pointing to a reachable module (or pushing the fork publicly), and
ensure all references in nip47_notifier.go import the corrected module path (the
current import string "github.com/getAlby/go-nostr") so the build can resolve
the package; if the fork is private, add a note in docs about access or change
to the upstream module accordingly.
2 participants
Tested running two local relays, switching between real relays and relays that always return CLOSED. Alby Hub reconnects as expected.
Summary by CodeRabbit
Release Notes