-
-
Notifications
You must be signed in to change notification settings - Fork 12
Fix issue #107: Skip acknowledgment prompt in non-interactive mode #121
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| # Add Signal messenger support via signal-cli | ||
|
|
||
| Fixes #115 | ||
|
|
||
| ## Summary | ||
|
|
||
| This PR implements Signal Private Messenger integration for RustyClaw using the `signal-cli` external tool. This follows the CLI-based messenger approach outlined in the issue, providing a clean integration without requiring complex native Signal protocol libraries. | ||
|
|
||
| ## Changes Made | ||
|
|
||
| ### Core Implementation | ||
| - **`crates/rustyclaw-core/src/messengers/signal_cli.rs`**: Complete SignalCliMessenger implementation | ||
| - **`crates/rustyclaw-core/Cargo.toml`**: Added `signal-cli` feature flag | ||
| - **`crates/rustyclaw-core/src/messengers/mod.rs`**: Exported SignalCliMessenger with feature gating | ||
|
|
||
| ### Documentation | ||
| - **`docs/signal-cli-setup.md`**: Comprehensive setup and configuration guide | ||
|
|
||
| ## Features | ||
|
|
||
| ✅ **Send messages** to individual phone numbers | ||
| ✅ **Receive messages** with JSON parsing | ||
| ✅ **Phone number normalization** to E.164 format | ||
| ✅ **Media attachment support** via signal-cli | ||
| ✅ **Group messaging** support (requires group IDs) | ||
| ✅ **Proper error handling** with descriptive messages | ||
| ✅ **Connection management** and health checks | ||
| ✅ **Integration tests** for core functionality | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| This implementation requires: | ||
|
|
||
| 1. **signal-cli**: Must be installed separately from https://github.com/AsamK/signal-cli | ||
| 2. **Signal registration**: Phone number must be registered with Signal | ||
| 3. **Configuration**: Proper setup in RustyClaw config.toml | ||
|
|
||
| ## Configuration Example | ||
|
|
||
| ```toml | ||
| [messengers.signal] | ||
| type = "signal-cli" | ||
| phone_number = "+1234567890" | ||
| signal_cli_path = "/usr/local/bin/signal-cli" # Optional | ||
| enabled = true | ||
| ``` | ||
|
|
||
| ## Usage Example | ||
|
|
||
| ```rust | ||
| use rustyclaw_core::messengers::{SignalCliMessenger, Messenger}; | ||
|
|
||
| let mut messenger = SignalCliMessenger::new( | ||
| "my_signal".to_string(), | ||
| "+1234567890".to_string(), | ||
| ); | ||
|
|
||
| messenger.initialize().await?; | ||
| let message_id = messenger.send_message("+19876543210", "Hello!").await?; | ||
| ``` | ||
|
|
||
| ## Testing | ||
|
|
||
| Build with Signal support: | ||
| ```bash | ||
| cargo build --features signal-cli | ||
| ``` | ||
|
|
||
| Run tests: | ||
| ```bash | ||
| cargo test signal_cli --features signal-cli | ||
| ``` | ||
|
|
||
| ## Architecture | ||
|
|
||
| This implementation follows the existing RustyClaw messenger pattern: | ||
| - Implements the `Messenger` trait | ||
| - Uses external process execution via `tokio::process::Command` | ||
| - Provides proper async/await support | ||
| - Includes comprehensive error handling | ||
| - Follows the feature-gated compilation model | ||
|
|
||
| ## Limitations | ||
|
|
||
| - **External dependency**: Requires signal-cli installation | ||
| - **Process overhead**: Each operation spawns a signal-cli process | ||
| - **Limited native features**: Some Signal features may not be available | ||
| - **Rate limiting**: Subject to Signal's rate limits | ||
|
|
||
| ## Backward Compatibility | ||
|
|
||
| This change is fully backward compatible: | ||
| - New feature is behind a feature flag (`signal-cli`) | ||
| - No changes to existing messenger APIs | ||
| - No breaking changes to configuration format | ||
| - Existing messengers remain unaffected | ||
|
|
||
| ## Future Improvements | ||
|
|
||
| Potential enhancements for follow-up PRs: | ||
| - Process pooling for better performance | ||
| - Enhanced group chat management | ||
| - Better media handling and validation | ||
| - Signal sticker support | ||
| - Message reaction support | ||
|
|
||
| ## Related Issues | ||
|
|
||
| - Closes #115: Messenger support with CLI-based approach | ||
| - Part of the two-tier messenger strategy discussed in #115 | ||
|
|
||
| --- | ||
|
|
||
| **Review Notes**: This implementation provides a solid foundation for Signal messaging in RustyClaw while maintaining the project's architectural principles. The CLI-based approach ensures reliability and easier maintenance compared to native protocol implementations. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🔴
non_interactiveflag only skips the safety acknowledgment but leaves all other interactive prompts active, causing the wizard to hangThe
non_interactiveparameter is only checked atcrates/rustyclaw-tui/src/onboard.rs:42to skip the safety acknowledgment prompt (step 0). However, the rest of the wizard (steps 0b through 8) still callsprompt_line,prompt_secret, andarrow_selectwithout checkingnon_interactive, so the wizard immediately blocks waiting for stdin at the "Agent name" prompt (crates/rustyclaw-tui/src/onboard.rs:105). This is particularly problematic because the CLI flags documentnon_interactiveas "Run wizard without prompts" (crates/rustyclaw-cli/src/main.rs:107-108) and "Run without prompts" (crates/rustyclaw-cli/src/main.rs:199-201), and forrustyclaw setup --non-interactivethe flag's presence alone triggers the wizard path (crates/rustyclaw-cli/src/main.rs:598). Users in automated/CI environments will find the process hangs indefinitely.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.