fix: clearer MOSTRO_PUBKEY setup error for first-time users - #189
fix: clearer MOSTRO_PUBKEY setup error for first-time users#189VedantMadane wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe ChangesConfiguration guidance
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The CLI now provides clearer guidance when MOSTRO_PUBKEY is missing, including the manual configuration-directory setup requirement. No current merge-readiness risk is identified. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
- Expand MOSTRO_PUBKEY missing error with ~/.config/mostro setup hint - Clarify that the config directory is not auto-created Fixes MostroP2P#184 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
ff7f4a2 to
99f3a50
Compare
grunch
left a comment
grunch
left a comment
There was a problem hiding this comment.
Thanks for taking this on — #184 is a real, recurring papercut and the error message is the right place to fix it.
Mechanical checks pass: cargo build --release, cargo fmt --check and cargo clippy --release --all-targets are all clean, the branch is up to date with main, and I verified the rendered output (Rust's \-newline continuation strips the leading indentation, so the message aligns correctly).
That said, I'd like a rewording before merging.
1. Option "3)" is not a method the CLI supports
resolve_mostro_pubkey only reads the flag and std::env::var("MOSTRO_PUBKEY"). There is no dotenv support anywhere in the binary — the README says so explicitly in About .env files. Listing ~/.config/mostro/env.sh as a peer of --mostropubkey and export implies mostro-cli reads that file.
That is exactly the misconception behind #184: the reporter went looking for ~/.config/mostro because they believed the tool used it. As written, the fix risks reinforcing the confusion it's meant to remove.
Suggestion — keep two methods and add a note instead of a third item:
MOSTRO_PUBKEY not set.
Provide it using one of the following methods:
1) --mostropubkey <npub>
2) export MOSTRO_PUBKEY=<npub>
mostro-cli does not read any config file. To avoid re-exporting every
time, put the exports in a file (e.g. ~/.config/mostro/env.sh) and
`source` it before running the CLI.
See https://github.com/MostroP2P/mostro-cli#configuration
2. The parenthetical is confusing out of context, and the pointer isn't actionable
"the config dir is not auto-created" answers a question the user hasn't asked yet, and implies some config dir might otherwise have been created. And "see README Configuration" doesn't help the #184 case: someone who ran cargo install mostro-cli has no checkout. Please use the URL.
3. Inconsistent with resolve_relays
The MOSTRO_PUBKEY error now mentions RELAYS, but resolve_relays still lists only two methods. A user who follows the new step 3 halfway then hits RELAYS not set, which contradicts what they just read. Either update both messages or keep each error scoped to its own variable (I'd prefer the latter, plus the shared note above).
4. No test coverage
tests/cli_functions.rs exists but nothing pins this message, which is the entire point of the PR. resolve_mostro_pubkey is private, so a test that runs the binary with the env cleared and asserts the guidance appears would do it. Not blocking for a string change, but right now there's zero regression protection.
Happy to merge once 1–3 are addressed — they're all in the same string literal.
2 participants
Summary
Clearer MOSTRO_PUBKEY setup error
Changes
Fixes #184
Summary by CodeRabbit
MOSTRO_PUBKEYis missing.~/.config/mostro/env.sh.~/.config/mostrodirectory is not created automatically and must be created manually before adding the environment file.