feat: native WhatsApp Web channel via wa-rs#294
Conversation
…nd wiring
- Remove WhatsAppConfig, bridge env overrides, bridge channel file (whatsapp.rs)
- Add WhatsAppWebConfig with auth_dir + E.164 allowlist to config/types.rs
- Add ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_{AUTH_DIR,ENABLED} env overrides
- Update ChannelsConfig: whatsapp → whatsapp_web field
- Update factory.rs: remove bridge registration, add #[cfg(feature = "whatsapp-web")] block
- Update channels/mod.rs: feature-gate whatsapp_web module and re-export
- Update lib.rs: remove WhatsAppChannel from pub re-exports
- Update cli/channel.rs: whatsapp → whatsapp_web in list/setup/test commands
- Update cli/gateway.rs: remove collect_enabled_channel_deps, install_and_start_dep, and bridge deps import
- Update cli/onboard.rs: replace bridge setup with whatsapp_web setup
- Add whatsapp_web.rs stub channel implementation (full impl in Task 10)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the stub whatsapp_web.rs with the full Channel trait implementation including: - E.164 phone normalization for allowlist matching - QR code terminal rendering (unicode half-blocks) - Exponential backoff helper (2s base, 120s cap) - OutboundWaMessage with 64-slot bounded mpsc queue - Panic-isolated spawned event loop (AssertUnwindSafe) - Proper error returns from send() when channel not started - 35 unit tests covering all helper functions and channel lifecycle The wa-rs event loop body is a TODO placeholder — will be wired when the wa-rs crate is available. All scaffolding (shutdown watch channel, outbound mpsc, AtomicBool running flag) follows existing channel patterns (mqtt, serial, whatsapp_cloud). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace whatsapp.rs bridge references with whatsapp_web.rs (wa-rs) - Remove gateway auto-install dependency bullet points - Add whatsapp-web to Cargo Features section with build example - Add WHATSAPP_WEB_ENABLED and WHATSAPP_WEB_AUTH_DIR env vars - Update README channels table Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the bridge-based WhatsApp channel with a native, feature-gated WhatsApp Web implementation ( Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Gateway/CLI
participant Manager as ChannelManager
participant WAChan as WhatsAppWebChannel
participant Store as SQLiteStore
participant WA as wa-rs Bot
participant Bus as MessageBus
CLI->>Manager: start configured channels
Manager->>WAChan: instantiate (config, bus)
WAChan->>Store: open/create auth sqlite at auth_dir
WAChan->>WA: init Bot with Store + transport/http
WA->>WAChan: emit pairing QR / pair code / login events
WAChan->>Bus: publish InboundMessage (after allowlist check)
Bus->>WAChan: deliver OutboundMessage
WAChan->>WA: send message to JID
WA->>Store: persist session updates
WAChan->>Manager: report running / stopped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
632-652:⚠️ Potential issue | 🟡 MinorUpdate the channel command examples in this doc to the new identifier too.
This section now documents WhatsApp Web, but the quick-reference block above still shows
channel setup whatsapp/channel test whatsapp. If the CLI was renamed towhatsapp_webin this PR, those examples are no longer runnable. As per coding guidelines "Keep README/docs claims aligned with executable behavior".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 632 - 652, Update the quick-reference CLI examples to match the renamed WhatsApp Web channel identifier: replace occurrences of the old commands "channel setup whatsapp" and "channel test whatsapp" with the new "channel setup whatsapp_web" and "channel test whatsapp_web" so the documented examples match the actual CLI behavior (search for those strings in CLAUDE.md and update them wherever the WhatsApp Web section or the quick-reference block appears).
🧹 Nitpick comments (1)
src/channels/whatsapp_web.rs (1)
274-281: Consider distinguishing queue-full vs channel-closed errors.
try_sendcan fail due to either a full queue (TrySendError::Full) or a closed channel (TrySendError::Closed). The current error message doesn't distinguish these cases, which could help with debugging.♻️ Optional: distinguish error cases
Some(tx) => { - if let Err(e) = tx.try_send(wa_msg) { - return Err(ZeptoError::Channel(format!( - "WhatsApp Web: failed to queue outbound message: {}", - e - ))); + if let Err(e) = tx.try_send(wa_msg) { + let reason = match e { + mpsc::error::TrySendError::Full(_) => "outbound queue full", + mpsc::error::TrySendError::Closed(_) => "event loop stopped", + }; + return Err(ZeptoError::Channel(format!( + "WhatsApp Web: {}", reason + ))); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/whatsapp_web.rs` around lines 274 - 281, The outbound send error handling for self.outbound_tx using tx.try_send(wa_msg) currently maps all failures to a generic ZeptoError::Channel; change the match to inspect the TrySendError variants so you can return distinct errors for TrySendError::Full vs TrySendError::Closed (or include that detail in the error string) when handling in the match in the block around outbound_tx/try_send, e.g., map Full to a "queue full" ZeptoError::Channel and Closed to a "channel closed" ZeptoError::Channel (or include the variant name in the formatted message) so callers can differentiate the two failure modes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config/mod.rs`:
- Around line 903-919: Handle legacy env var aliases for the WhatsApp channel:
when setting WhatsAppWebConfig fields in the block that touches
channels.whatsapp_web and WhatsAppWebConfig, check both the new names
(ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR and
ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_ENABLED) and the legacy names (e.g.,
ZEPTOCLAW_CHANNELS_WHATSAPP_AUTH_DIR and ZEPTOCLAW_CHANNELS_WHATSAPP_ENABLED) as
fallbacks—use the new var if present, otherwise read the legacy var (parse bool
for enabled with the same .parse::<bool>() pattern), and populate
channel.auth_dir and channel.enabled accordingly; if you prefer failing fast
instead, detect presence of any legacy vars when the new ones are absent and
return an error or panic so upgrades don’t silently drop configuration.
In `@src/config/types.rs`:
- Around line 748-749: Add a Serde alias so older configs using the
channels.whatsapp key still deserialize into the new field: annotate the
whatsapp_web field (type Option<WhatsAppWebConfig>) with serde(alias =
"whatsapp") so both "whatsapp" and "whatsapp_web" map to the same field and
preserve backwards compatibility.
In `@src/lib.rs`:
- Around line 47-50: The crate root currently re-exports WhatsAppCloudChannel
but removed the old WhatsApp export; restore a backward-compatible re-export by
adding WhatsAppWebChannel to the pub use list (or add a separate pub use)
guarded by the appropriate feature flag so downstream imports using
zeptoclaw::WhatsAppWebChannel continue to work; update the export near
BaseChannelConfig, Channel, ChannelManager, ChannelPluginAdapter, SlackChannel,
TelegramChannel, WhatsAppCloudChannel so it includes WhatsAppWebChannel behind
the same feature gate.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 632-652: Update the quick-reference CLI examples to match the
renamed WhatsApp Web channel identifier: replace occurrences of the old commands
"channel setup whatsapp" and "channel test whatsapp" with the new "channel setup
whatsapp_web" and "channel test whatsapp_web" so the documented examples match
the actual CLI behavior (search for those strings in CLAUDE.md and update them
wherever the WhatsApp Web section or the quick-reference block appears).
---
Nitpick comments:
In `@src/channels/whatsapp_web.rs`:
- Around line 274-281: The outbound send error handling for self.outbound_tx
using tx.try_send(wa_msg) currently maps all failures to a generic
ZeptoError::Channel; change the match to inspect the TrySendError variants so
you can return distinct errors for TrySendError::Full vs TrySendError::Closed
(or include that detail in the error string) when handling in the match in the
block around outbound_tx/try_send, e.g., map Full to a "queue full"
ZeptoError::Channel and Closed to a "channel closed" ZeptoError::Channel (or
include the variant name in the formatted message) so callers can differentiate
the two failure modes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 756d8b78-eed4-4437-818f-abd37a81099a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
CLAUDE.mdCargo.tomlREADME.mdsrc/channels/factory.rssrc/channels/mod.rssrc/channels/whatsapp.rssrc/channels/whatsapp_web.rssrc/cli/channel.rssrc/cli/gateway.rssrc/cli/onboard.rssrc/config/mod.rssrc/config/types.rssrc/lib.rs
💤 Files with no reviewable changes (1)
- src/channels/whatsapp.rs
- Add qrcode crate (v0.14) behind whatsapp-web feature for terminal QR display - Render QR codes using Unicode half-block characters for clean terminal output - Display bordered instruction box during pairing with timeout countdown - Add deny_by_default prompt to onboarding wizard when allowlist is empty - Add post-configuration pairing instructions to onboard output Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/config/mod.rs (1)
903-935:⚠️ Potential issue | 🟠 MajorPrefer canonical
WHATSAPP_WEB_*vars over legacy aliases.Line 911 and Line 927 run after the canonical reads, so
ZEPTOCLAW_CHANNELS_WHATSAPP_AUTH_DIR/ZEPTOCLAW_CHANNELS_WHATSAPP_ENABLEDoverwrite the new...WHATSAPP_WEB_*values whenever both are set. Mixed deployments will silently keep stale legacy config instead of honoring the renamed vars.Suggested fix
- if let Ok(val) = std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR") { + if let Ok(val) = std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR") + .or_else(|_| std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_AUTH_DIR")) + { let channel = self .channels .whatsapp_web .get_or_insert_with(WhatsAppWebConfig::default); channel.auth_dir = val; } - if let Ok(val) = std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_AUTH_DIR") { - let channel = self - .channels - .whatsapp_web - .get_or_insert_with(WhatsAppWebConfig::default); - channel.auth_dir = val; - } - if let Ok(Ok(enabled)) = - std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_ENABLED").map(|v| v.parse::<bool>()) + if let Ok(Ok(enabled)) = std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_ENABLED") + .or_else(|_| std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_ENABLED")) + .map(|v| v.parse::<bool>()) { let channel = self .channels .whatsapp_web .get_or_insert_with(WhatsAppWebConfig::default); channel.enabled = enabled; } - if let Ok(Ok(enabled)) = - std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_ENABLED").map(|v| v.parse::<bool>()) - { - let channel = self - .channels - .whatsapp_web - .get_or_insert_with(WhatsAppWebConfig::default); - channel.enabled = enabled; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/mod.rs` around lines 903 - 935, The legacy WHATSAPP_* env reads are currently applied after the canonical WHATSAPP_WEB_* reads and can overwrite them; change the legacy handling so canonical vars take precedence by only applying legacy values when the corresponding canonical env is not set. Concretely, for the ZEPTOCLAW_CHANNELS_WHATSAPP_AUTH_DIR and ZEPTOCLAW_CHANNELS_WHATSAPP_ENABLED blocks wrap their logic with a check like std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR").is_err() (and similarly for ENABLED) before mutating self.channels.whatsapp_web (WhatsAppWebConfig) so you only set channel.auth_dir or channel.enabled from the legacy var when the canonical WHATSAPP_WEB_* env is absent.
🧹 Nitpick comments (3)
src/channels/manager.rs (1)
511-518: Consider simplifying the bidirectional fallback.The fallback logic maps
"whatsapp"→"whatsapp_web"and vice versa. However, since this PR removes the old bridge-based"whatsapp"channel implementation, the"whatsapp_web"→"whatsapp"fallback will never find a match. Consider simplifying to a unidirectional alias:♻️ Suggested simplification
channels .get(channel_name) .cloned() .or_else(|| match channel_name { "whatsapp" => channels.get("whatsapp_web").cloned(), - "whatsapp_web" => channels.get("whatsapp").cloned(), _ => None, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/manager.rs` around lines 511 - 518, The current fallback in channels.get(channel_name).cloned().or_else(...) implements a bidirectional alias between "whatsapp" and "whatsapp_web" even though the old "whatsapp" bridge impl is removed; update the or_else closure to only provide a unidirectional alias (map "whatsapp" -> channels.get("whatsapp_web").cloned()) and remove the "whatsapp_web" -> "whatsapp" arm so lookups use channels.get(channel_name).cloned().or_else(|_| channels.get("whatsapp_web").cloned()) for that single fallback, keeping references to channel_name and channels.get/ .cloned/ .or_else to locate the change.src/cli/onboard.rs (1)
815-821: Feature check behavior clarification.The
cfg!(feature = "whatsapp-web")macro evaluates at compile time, not runtime. If the binary is built without thewhatsapp-webfeature, this function will always return an error when called from the full wizard path (line 270).This is acceptable behavior, but consider whether the full wizard should skip calling this function entirely when the feature is disabled:
♻️ Alternative: skip at call site
// In cmd_onboard() around line 270: - configure_whatsapp_channel(&mut config)?; + #[cfg(feature = "whatsapp-web")] + configure_whatsapp_channel(&mut config)?;This avoids the error message during onboarding for users who intentionally built without the feature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/onboard.rs` around lines 815 - 821, The function configure_whatsapp_channel currently bails when the whatsapp-web feature is not compiled in; instead, change the caller in the full onboarding/wizard flow to skip calling configure_whatsapp_channel when the feature is disabled by wrapping the call in a compile-time check (e.g., if cfg!(feature = "whatsapp-web") { configure_whatsapp_channel(...)? } ), or use #[cfg(feature = "whatsapp-web")] on a small wrapper function that invokes configure_whatsapp_channel so the flow silently omits WhatsApp setup when the feature is not present.src/channels/whatsapp_web.rs (1)
200-208: Avoid blocking filesystem calls insidestart().Line 202 uses
std::fs::create_dir_all()insideasync fn start(). Switch totokio::fs::create_dir_all()or wrap it inspawn_blockingso channel startup does not block the Tokio worker thread.Suggested fix
- if let Some(parent) = store_path.parent() { - fs::create_dir_all(parent).map_err(|e| { + if let Some(parent) = store_path.parent() { + tokio::fs::create_dir_all(parent).await.map_err(|e| { ZeptoError::Channel(format!( "WhatsApp Web: failed to create auth directory {}: {}", parent.display(), e )) })?; }As per coding guidelines "All async I/O must use Tokio async runtime exclusively, with
spawn_blockingwrappers for sync operations (memory, filesystem) in async context".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/whatsapp_web.rs` around lines 200 - 208, The call to std::fs::create_dir_all inside async fn start() (around the sqlite_store_path(&self.config.auth_dir) block) blocks the Tokio worker thread; replace it with an async-friendly operation by either calling tokio::fs::create_dir_all(parent).await (preferred) or wrapping the synchronous call in tokio::task::spawn_blocking and awaiting its JoinHandle; ensure the error is mapped into ZeptoError::Channel the same way it is now (preserving the message that includes parent.display() and the underlying error) and update the matching import/usages accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/channels/whatsapp_web.rs`:
- Around line 272-279: The handler currently flips running to false on
Event::Disconnected which can occur during transient network issues while the
RuntimeState and spawned bot task remain alive, causing is_running() to lie and
the supervisor to spawn duplicates; change the logic so running.store(false,
Ordering::SeqCst) is only called when the bot task truly exits (e.g., on
Event::LoggedOut with final reason or in the task's exit/cleanup path or when
RuntimeState transitions to an Exited/Stopped state), not on
Event::Disconnected; apply the same fix to the other similar block around the
Event handling at the 343-350 region so the ChannelManager’s polling/restart
logic sees a channel as dead only after the actual task shutdown.
- Around line 246-270: The code is logging pairing secrets from
Event::PairingQrCode and Event::PairingCode; instead of emitting the raw secret
to structured logs (the warn! and info! calls), stop logging the `code` value
and either write it only to the attached TTY using eprintln!/print (use the
existing render_qr_terminal flow or eprintln! for the pairing code) or redact it
before logging (e.g., replace with “[REDACTED_PAIRING_CODE]”); update the
branches handling Event::PairingQrCode (where render_qr_terminal returns None)
and Event::PairingCode to remove the secret from warn!/info! and use
terminal-only output or a redacted log message.
In `@src/cli/channel.rs`:
- Around line 330-337: The test test_channel_test_whatsapp_web_not_configured
currently assumes whatsapp-web is available; update it to handle both build
variants by either guarding the test with #[cfg(feature = "whatsapp-web")] so it
only runs when that feature is enabled, or change the assertions to branch on
cfg!(feature = "whatsapp-web") — when true assert
test_whatsapp_web(&config).await returns an error containing "not configured",
otherwise assert it returns an error containing "not available in this build";
reference the test function name test_channel_test_whatsapp_web_not_configured
and the call to test_whatsapp_web(&config).await to locate where to apply the
change.
- Around line 196-205: The current logic only updates wa_config.allow_from when
the read_line() result is non-empty, so pressing Enter never clears a previously
set allowlist; change the handling around read_line()/allowlist so that if
allowlist.is_empty() you explicitly clear wa_config.allow_from (e.g., set it to
an empty Vec or default “allow all” value), otherwise parse the comma-separated
values as before (use wa_config.allow_from, read_line, .split(','), .map(|s|
s.trim().to_string()), .filter(|s| !s.is_empty()), .collect()).
---
Duplicate comments:
In `@src/config/mod.rs`:
- Around line 903-935: The legacy WHATSAPP_* env reads are currently applied
after the canonical WHATSAPP_WEB_* reads and can overwrite them; change the
legacy handling so canonical vars take precedence by only applying legacy values
when the corresponding canonical env is not set. Concretely, for the
ZEPTOCLAW_CHANNELS_WHATSAPP_AUTH_DIR and ZEPTOCLAW_CHANNELS_WHATSAPP_ENABLED
blocks wrap their logic with a check like
std::env::var("ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR").is_err() (and
similarly for ENABLED) before mutating self.channels.whatsapp_web
(WhatsAppWebConfig) so you only set channel.auth_dir or channel.enabled from the
legacy var when the canonical WHATSAPP_WEB_* env is absent.
---
Nitpick comments:
In `@src/channels/manager.rs`:
- Around line 511-518: The current fallback in
channels.get(channel_name).cloned().or_else(...) implements a bidirectional
alias between "whatsapp" and "whatsapp_web" even though the old "whatsapp"
bridge impl is removed; update the or_else closure to only provide a
unidirectional alias (map "whatsapp" -> channels.get("whatsapp_web").cloned())
and remove the "whatsapp_web" -> "whatsapp" arm so lookups use
channels.get(channel_name).cloned().or_else(|_|
channels.get("whatsapp_web").cloned()) for that single fallback, keeping
references to channel_name and channels.get/ .cloned/ .or_else to locate the
change.
In `@src/channels/whatsapp_web.rs`:
- Around line 200-208: The call to std::fs::create_dir_all inside async fn
start() (around the sqlite_store_path(&self.config.auth_dir) block) blocks the
Tokio worker thread; replace it with an async-friendly operation by either
calling tokio::fs::create_dir_all(parent).await (preferred) or wrapping the
synchronous call in tokio::task::spawn_blocking and awaiting its JoinHandle;
ensure the error is mapped into ZeptoError::Channel the same way it is now
(preserving the message that includes parent.display() and the underlying error)
and update the matching import/usages accordingly.
In `@src/cli/onboard.rs`:
- Around line 815-821: The function configure_whatsapp_channel currently bails
when the whatsapp-web feature is not compiled in; instead, change the caller in
the full onboarding/wizard flow to skip calling configure_whatsapp_channel when
the feature is disabled by wrapping the call in a compile-time check (e.g., if
cfg!(feature = "whatsapp-web") { configure_whatsapp_channel(...)? } ), or use
#[cfg(feature = "whatsapp-web")] on a small wrapper function that invokes
configure_whatsapp_channel so the flow silently omits WhatsApp setup when the
feature is not present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4319915f-5f95-4359-b3d8-2f3b0aafc4fa
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
AGENTS.mdCLAUDE.mdCargo.tomlsrc/api/routes/channels.rssrc/channels/factory.rssrc/channels/manager.rssrc/channels/whatsapp_web.rssrc/cli/channel.rssrc/cli/mod.rssrc/cli/onboard.rssrc/config/mod.rssrc/config/types.rssrc/lib.rssrc/tools/message.rs
✅ Files skipped from review due to trivial changes (1)
- src/cli/mod.rs
…up commands - Express onboard now offers Telegram/WhatsApp/Discord/Slack channel setup - `channel setup telegram` works standalone (no longer redirects to onboard) - `channel setup discord` works standalone with token + allowlist prompts - `channel setup slack` works standalone with bot + app token prompts - `channel setup webhook` works standalone with bind/port/auth prompts - Add configure_discord() and configure_slack() to onboard.rs for full wizard Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/onboard.rs (1)
872-881:⚠️ Potential issue | 🟡 MinorSame allowlist-clearing issue as in
channel.rs.The prompt says "Enter for all" but empty input preserves the existing allowlist. Apply the same fix pattern as suggested for the other setup functions.
Suggested fix
print!("Phone number allowlist (comma-separated E.164, e.g. +60123456789, or Enter for all): "); io::stdout().flush()?; let allowlist = read_line()?; - if !allowlist.is_empty() { - whatsapp_config.allow_from = allowlist - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); - } + whatsapp_config.allow_from = allowlist + .split(',') + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .collect();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/onboard.rs` around lines 872 - 881, The prompt indicates "Enter for all" but the current logic leaves whatsapp_config.allow_from unchanged when the user presses Enter; update the handling around the allowlist/read_line block so an empty input clears the allowlist (set whatsapp_config.allow_from = Vec::new()) instead of preserving it, and otherwise parse the comma-separated entries into whatsapp_config.allow_from as currently done (use the existing allowlist variable, read_line(), and whatsapp_config.allow_from identifiers to locate and modify the code).
🧹 Nitpick comments (1)
src/cli/onboard.rs (1)
901-930: Consider adding allowlist prompt for consistency.The
setup_discordfunction inchannel.rs(lines 263-272) prompts for an allowlist, but thisconfigure_discordin onboard.rs does not. Consider aligning the two flows, or document the intentional difference (e.g., express mode skips allowlist for brevity).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/onboard.rs` around lines 901 - 930, The configure_discord function currently sets token and enabled but omits the allowlist prompt used in setup_discord; mirror the allowlist flow from setup_discord by prompting the user after token entry (e.g., "Enter allowed channels/users (comma-separated) or press Enter to allow all"), parse the input into the same field on the discord config (e.g., discord_config.allowlist) and assign it the parsed value or leave it empty/default when skipped, ensuring the logic and field names match those used in setup_discord so both flows behave consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/channel.rs`:
- Around line 263-272: The prompt says Enter should clear the allowlist but the
current code only updates dc.allow_from when allowlist is non-empty, so pressing
Enter preserves the old list; modify the handling around read_line()/allowlist
so that if allowlist.trim().is_empty() you explicitly set dc.allow_from =
Vec::new() (clearing it), otherwise parse allowlist.split(',').map(|s|
s.trim().to_string()).filter(|s| !s.is_empty()).collect() to populate
dc.allow_from; update the logic near the read_line() call that currently gates
assignment on if !allowlist.is_empty() to implement this clear-or-parse
behavior.
- Around line 223-232: The prompt "Enter for all" currently doesn't clear an
existing allowlist because the code only updates tg.allow_from when input is
non-empty; change the logic in the block handling read_line()/allowlist so that
you trim the input first and if the trimmed string is empty you clear
tg.allow_from (set to an empty Vec or call clear()), otherwise split the trimmed
input on ',' and assign the filtered, trimmed values to tg.allow_from; update
the branch around allowlist/read_line() to use this new behavior so pressing
Enter clears the allowlist as intended.
---
Outside diff comments:
In `@src/cli/onboard.rs`:
- Around line 872-881: The prompt indicates "Enter for all" but the current
logic leaves whatsapp_config.allow_from unchanged when the user presses Enter;
update the handling around the allowlist/read_line block so an empty input
clears the allowlist (set whatsapp_config.allow_from = Vec::new()) instead of
preserving it, and otherwise parse the comma-separated entries into
whatsapp_config.allow_from as currently done (use the existing allowlist
variable, read_line(), and whatsapp_config.allow_from identifiers to locate and
modify the code).
---
Nitpick comments:
In `@src/cli/onboard.rs`:
- Around line 901-930: The configure_discord function currently sets token and
enabled but omits the allowlist prompt used in setup_discord; mirror the
allowlist flow from setup_discord by prompting the user after token entry (e.g.,
"Enter allowed channels/users (comma-separated) or press Enter to allow all"),
parse the input into the same field on the discord config (e.g.,
discord_config.allowlist) and assign it the parsed value or leave it
empty/default when skipped, ensuring the logic and field names match those used
in setup_discord so both flows behave consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f495f6c-018d-480e-bb78-b03f79b27fbb
📒 Files selected for processing (2)
src/cli/channel.rssrc/cli/onboard.rs
- Add `channel setup whatsapp_cloud` standalone wizard - Add `channel test whatsapp_cloud` status check - Add WhatsApp Cloud to `channel list` display - Add WhatsApp Cloud option to express and full onboard wizards - Add Discord + Slack to full wizard (previously only Telegram + WhatsApp Web) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/onboard.rs (1)
875-884:⚠️ Potential issue | 🟡 MinorEmpty input doesn't clear existing allowlist (same issue as in channel.rs).
The prompt says "Enter for all" but an empty input only skips updating, leaving any existing allowlist intact. This matches the pattern flagged in
src/cli/channel.rspast reviews.Suggested fix
print!("Phone number allowlist (comma-separated E.164, e.g. +60123456789, or Enter for all): "); io::stdout().flush()?; let allowlist = read_line()?; - if !allowlist.is_empty() { - whatsapp_config.allow_from = allowlist - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); - } + whatsapp_config.allow_from = allowlist + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/onboard.rs` around lines 875 - 884, The prompt says "Enter for all" but empty input currently leaves whatsapp_config.allow_from unchanged; modify the logic around read_line()/allowlist handling so that when allowlist.is_empty() you explicitly clear whatsapp_config.allow_from (set to an empty Vec) to represent "all" and otherwise parse the comma-separated values as before (retain the existing split/trim/filter/collect flow); reference whatsapp_config.allow_from and the local allowlist/read_line() usage to locate the block to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/channel.rs`:
- Around line 394-400: The code reads sensitive values into token and
verify_token using read_line(), exposing them; switch both prompts to use the
masked input helper (read_secret()) instead of read_line(), updating the imports
if needed and replacing calls for the "Enter permanent access token:" and
"Choose a webhook verify token (any secret string):" prompts so token and
verify_token are populated via read_secret() to avoid echoing secrets to the
console.
- Around line 307-318: The prompts currently call read_line() for sensitive
values (bot_token and app_token) in the interactive flow (the block that prints
"Enter Slack bot token..." and "Enter Slack app-level token..."), so update
those reads to use a masked/no-echo input method (e.g., rpassword::read_password
or read_password_from_tty) instead of read_line(); keep the existing empty-check
behavior for bot_token (treat empty as skip) and ensure you still flush stdout
before calling the masked read so the prompt is visible; update variable usages
(bot_token, app_token) and any imports to include the chosen password-reading
crate/function.
- Around line 267-270: The code currently echoes the Discord token when reading
it (print! + read_line()), so change the input to masked/no-echo input: keep the
prompt and flush, then replace read_line() with a no-echo reader such as
rpassword::read_password() (or an equivalent crate) and assign its result to the
token variable; ensure error handling remains the same for the function that
contains print!/flush and the let token = ... statement (refer to the token
variable and the surrounding prompt/flush code in the same function).
- Around line 358-363: Replace the plain echoing input used to collect the
webhook bearer token with a masked password-style prompt: instead of calling
read_line() for the auth token, use a masked input function (e.g.,
rpassword::read_password or equivalent) to read the secret without echoing, trim
the result, and set wh.auth_token = Some(auth) only if the trimmed string is
non-empty; keep the existing prompt/flush behavior but ensure the masked-read
handles terminal input and errors similarly to read_line().
In `@src/cli/onboard.rs`:
- Around line 965-972: Replace the plaintext read_line() call with the masked
input helper read_secret() when collecting the Discord token: change the prompt
to still ask for the token, call read_secret() to read it without echoing, keep
the existing check if !token.is_empty() and assignment to discord_config.token,
and ensure the function read_secret() is in scope (import or use the existing
helper) so onboarding uses masked input for the sensitive Discord token.
- Around line 996-1008: The prompt currently uses read_line() for sensitive
Slack credentials (bot_token and app_token) which exposes input; update the
onboarding prompts in onboard.rs to use read_secret() instead of read_line() for
both bot_token and app_token, preserve the same empty-check logic (is_empty())
and messages ("Skipped Slack configuration.") and ensure stdout flush calls
remain before calling read_secret() so input is masked while keeping the flow in
the same function/block.
- Around line 924-930: Replace the plaintext reads for sensitive tokens with
masked input: change the calls that populate token and verify_token (currently
using read_line()) to use read_secret() instead, and add read_secret to the
module imports so the function can be resolved; ensure the prompts and flush()
calls remain but the variables token and verify_token are acquired via
read_secret() to prevent echoing sensitive input.
---
Outside diff comments:
In `@src/cli/onboard.rs`:
- Around line 875-884: The prompt says "Enter for all" but empty input currently
leaves whatsapp_config.allow_from unchanged; modify the logic around
read_line()/allowlist handling so that when allowlist.is_empty() you explicitly
clear whatsapp_config.allow_from (set to an empty Vec) to represent "all" and
otherwise parse the comma-separated values as before (retain the existing
split/trim/filter/collect flow); reference whatsapp_config.allow_from and the
local allowlist/read_line() usage to locate the block to update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aefcac5e-0fef-4ff0-be2e-2170054cde64
📒 Files selected for processing (2)
src/cli/channel.rssrc/cli/onboard.rs
- Use ANSI 256-color (black/white) instead of plain unicode blocks - Proper white quiet zone border (2 modules) for reliable scanning - Half-block ▀ with fg/bg colors halves vertical height - Renders cleanly on both light and dark terminal themes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 2605975.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/channels/whatsapp_web.rs (2)
281-283:⚠️ Potential issue | 🟠 MajorKeep
runningtrue until the bot task actually exits.
Event::Disconnectedcan be transient while theRuntimeStateand spawned task are still alive. Clearing the flag here makesis_running()lie and can let the supervisor start a second bot during reconnect. As per coding guidelines, "Channel implementations must setrunning = falseon exit to prevent staleis_running()flags, and ChannelManager must poll for dead channels every 15 seconds with 60s restart cooldown, max 5 restarts".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/whatsapp_web.rs` around lines 281 - 283, The handler for Event::Disconnected should not clear the running flag because disconnects can be transient; remove or avoid calling running.store(false, Ordering::SeqCst) inside the Event::Disconnected branch and instead set running.store(false, Ordering::SeqCst) only when the bot task truly exits (e.g., on the RuntimeState/worker shutdown path or immediately after the spawned bot task finishes/joins or in the explicit exit/error branch that indicates permanent termination). Ensure is_running() reflects the actual lifecycle by moving the running=false write to the true-exit code path (referencing Event::Disconnected, running, is_running(), RuntimeState and the spawned task shutdown/exit handling).
251-275:⚠️ Potential issue | 🟠 MajorDon’t write pairing secrets to structured logs.
The QR fallback branch and the pair-code branch both emit one-time login secrets into
warn!/info!. Those values can leak into log collectors while still valid. Keep them on the attached TTY only, or log a redacted placeholder instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/whatsapp_web.rs` around lines 251 - 275, Event::PairingQrCode and Event::PairingCode currently log one-time pairing secrets via warn! and info!, which can leak into structured log collectors; instead, stop emitting raw secrets to logs: keep the full QR/code only printed to the TTY (using render_qr_terminal and eprint/eprintln paths) and replace any warn!/info! calls that include `code` with a redacted placeholder message (e.g., "pairing code/Qr token redacted") or remove them entirely; update the branches handling Event::PairingQrCode and Event::PairingCode to ensure warn!/info! do not interpolate `code`, and only display the secret to the attached terminal output paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/channels/whatsapp_web.rs`:
- Around line 88-96: normalize_phone currently only trims, removes a leading '+'
and strips a JID suffix, so formatted inputs like "+1 (555) 123-4567" or "6012
3456789" won't match allowlist entries; update the normalize_phone function to:
trim and strip the JID part (as it already does), then remove all non-digit
characters (e.g., using a filter to keep only '0'..'9') so the returned string
is a digits-only canonical phone identifier used by allowlist checks (keep the
function name normalize_phone and its return type String so callers remain
unchanged).
- Line 6: The start() function uses blocking std::fs::create_dir_all which
blocks the Tokio runtime; replace the blocking call with the async Tokio
equivalent by removing the std::fs import and calling
tokio::fs::create_dir_all(path).await inside start() (and propagate async
context/errors as needed), ensuring you import tokio::fs and await the future
where fs::create_dir_all(...) was previously used.
---
Duplicate comments:
In `@src/channels/whatsapp_web.rs`:
- Around line 281-283: The handler for Event::Disconnected should not clear the
running flag because disconnects can be transient; remove or avoid calling
running.store(false, Ordering::SeqCst) inside the Event::Disconnected branch and
instead set running.store(false, Ordering::SeqCst) only when the bot task truly
exits (e.g., on the RuntimeState/worker shutdown path or immediately after the
spawned bot task finishes/joins or in the explicit exit/error branch that
indicates permanent termination). Ensure is_running() reflects the actual
lifecycle by moving the running=false write to the true-exit code path
(referencing Event::Disconnected, running, is_running(), RuntimeState and the
spawned task shutdown/exit handling).
- Around line 251-275: Event::PairingQrCode and Event::PairingCode currently log
one-time pairing secrets via warn! and info!, which can leak into structured log
collectors; instead, stop emitting raw secrets to logs: keep the full QR/code
only printed to the TTY (using render_qr_terminal and eprint/eprintln paths) and
replace any warn!/info! calls that include `code` with a redacted placeholder
message (e.g., "pairing code/Qr token redacted") or remove them entirely; update
the branches handling Event::PairingQrCode and Event::PairingCode to ensure
warn!/info! do not interpolate `code`, and only display the secret to the
attached terminal output paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91f9bcea-0e2d-4c8d-a968-66a836518f0c
📒 Files selected for processing (1)
src/channels/whatsapp_web.rs
…ching WhatsApp Web sends messages with LID (Linked ID) sender JIDs instead of phone numbers. Use sender_alt field to resolve LID senders to phone JIDs so the allowlist check matches correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Express onboard now supports multi-select channels (like providers) - Remove redundant "Enable?" confirmation when user already chose the channel - Use read_secret() for all tokens (hidden terminal input) - Remove unimplemented memory backend options from wizard - Clean up WhatsApp Web setup headings and instructions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…t features - Update quinn-proto 0.11.13 → 0.11.14 (RUSTSEC-2026-0037 DoS fix) - Fix test_channel_test_whatsapp_web_not_configured to handle both "not available" (no feature) and "not configured" (feature present) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace blocking std::fs with tokio::fs in WhatsApp Web start() - Don't set running=false on Disconnected (only on LoggedOut/task exit) - Don't log pairing secrets to structured logs (terminal only) - Strip all non-digit chars in normalize_phone for robust matching - Allow clearing allowlists by pressing Enter in channel setup - Use read_secret() for webhook bearer auth token Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace dead WhatsApp bridge channel (`whatsapp.rs`, whatsmeow-bridge) with native WhatsApp Web channel (`whatsapp_web.rs`, wa-rs) - Feature-gated behind `--features whatsapp-web` to keep default binary unaffected - Full Channel trait implementation with E.164 normalization, QR rendering, backoff, panic isolation, and 35 unit tests | File | Change | |------|--------| | `Cargo.toml` | Add `wa-rs`, `wa-rs-sqlite-storage` optional deps + `whatsapp-web` feature | | `src/channels/whatsapp_web.rs` | **New** — full channel implementation (635 lines, 35 tests) | | `src/channels/whatsapp.rs` | **Deleted** — dead bridge channel (~1180 lines) | | `src/channels/mod.rs` | Replace `whatsapp` module with feature-gated `whatsapp_web` | | `src/channels/factory.rs` | Replace bridge registration with `WhatsAppWebChannel` (feature-gated) | | `src/config/types.rs` | Replace `WhatsAppConfig` with `WhatsAppWebConfig`, update `ChannelsConfig` | | `src/config/mod.rs` | Replace bridge env overrides with `WHATSAPP_WEB_*` overrides | | `src/cli/onboard.rs` | Update onboarding wizard for WhatsApp Web config | | `src/cli/channel.rs` | Update channel list, setup, and test commands | | `src/cli/gateway.rs` | Remove bridge dependency collection logic | | `src/lib.rs` | Remove `WhatsAppChannel` from re-exports | | `CLAUDE.md` | Update architecture, features, env vars | | `README.md` | Update channels table | - [x] `cargo clippy -- -D warnings` (no feature) — clean - [x] `cargo clippy --features whatsapp-web -- -D warnings` — clean - [x] `cargo test --lib` — 3073 passed - [x] `cargo test --lib --features whatsapp-web -- whatsapp_web` — 35 passed - [x] `cargo fmt -- --check` — clean Closes qhkm#288 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Native WhatsApp Web channel (enable with --features whatsapp-web) with QR pairing and terminal QR rendering. * **Configuration Changes** * Channel canonical name is "whatsapp_web" (legacy "whatsapp" auto-mapped). New auth_dir-based config, allowlist controls, and env vars ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_ENABLED / ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR. * **CLI/Docs** * Interactive setup, listing, test commands, onboarding flows, and docs updated for WhatsApp Web and WhatsApp Cloud. * **Behavior Changes** * Outbound routing tolerates whatsapp ↔ whatsapp_web aliases; gateway no longer auto-installs channel dependencies on startup. * **Tests** * Test coverage updated for feature-gated whatsapp-web scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
- Replace dead WhatsApp bridge channel (`whatsapp.rs`, whatsmeow-bridge) with native WhatsApp Web channel (`whatsapp_web.rs`, wa-rs) - Feature-gated behind `--features whatsapp-web` to keep default binary unaffected - Full Channel trait implementation with E.164 normalization, QR rendering, backoff, panic isolation, and 35 unit tests | File | Change | |------|--------| | `Cargo.toml` | Add `wa-rs`, `wa-rs-sqlite-storage` optional deps + `whatsapp-web` feature | | `src/channels/whatsapp_web.rs` | **New** — full channel implementation (635 lines, 35 tests) | | `src/channels/whatsapp.rs` | **Deleted** — dead bridge channel (~1180 lines) | | `src/channels/mod.rs` | Replace `whatsapp` module with feature-gated `whatsapp_web` | | `src/channels/factory.rs` | Replace bridge registration with `WhatsAppWebChannel` (feature-gated) | | `src/config/types.rs` | Replace `WhatsAppConfig` with `WhatsAppWebConfig`, update `ChannelsConfig` | | `src/config/mod.rs` | Replace bridge env overrides with `WHATSAPP_WEB_*` overrides | | `src/cli/onboard.rs` | Update onboarding wizard for WhatsApp Web config | | `src/cli/channel.rs` | Update channel list, setup, and test commands | | `src/cli/gateway.rs` | Remove bridge dependency collection logic | | `src/lib.rs` | Remove `WhatsAppChannel` from re-exports | | `CLAUDE.md` | Update architecture, features, env vars | | `README.md` | Update channels table | - [x] `cargo clippy -- -D warnings` (no feature) — clean - [x] `cargo clippy --features whatsapp-web -- -D warnings` — clean - [x] `cargo test --lib` — 3073 passed - [x] `cargo test --lib --features whatsapp-web -- whatsapp_web` — 35 passed - [x] `cargo fmt -- --check` — clean Closes qhkm#288 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Native WhatsApp Web channel (enable with --features whatsapp-web) with QR pairing and terminal QR rendering. * **Configuration Changes** * Channel canonical name is "whatsapp_web" (legacy "whatsapp" auto-mapped). New auth_dir-based config, allowlist controls, and env vars ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_ENABLED / ZEPTOCLAW_CHANNELS_WHATSAPP_WEB_AUTH_DIR. * **CLI/Docs** * Interactive setup, listing, test commands, onboarding flows, and docs updated for WhatsApp Web and WhatsApp Cloud. * **Behavior Changes** * Outbound routing tolerates whatsapp ↔ whatsapp_web aliases; gateway no longer auto-installs channel dependencies on startup. * **Tests** * Test coverage updated for feature-gated whatsapp-web scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
whatsapp.rs, whatsmeow-bridge) with native WhatsApp Web channel (whatsapp_web.rs, wa-rs)--features whatsapp-webto keep default binary unaffectedChanges
Cargo.tomlwa-rs,wa-rs-sqlite-storageoptional deps +whatsapp-webfeaturesrc/channels/whatsapp_web.rssrc/channels/whatsapp.rssrc/channels/mod.rswhatsappmodule with feature-gatedwhatsapp_websrc/channels/factory.rsWhatsAppWebChannel(feature-gated)src/config/types.rsWhatsAppConfigwithWhatsAppWebConfig, updateChannelsConfigsrc/config/mod.rsWHATSAPP_WEB_*overridessrc/cli/onboard.rssrc/cli/channel.rssrc/cli/gateway.rssrc/lib.rsWhatsAppChannelfrom re-exportsCLAUDE.mdREADME.mdTest plan
cargo clippy -- -D warnings(no feature) — cleancargo clippy --features whatsapp-web -- -D warnings— cleancargo test --lib— 3073 passedcargo test --lib --features whatsapp-web -- whatsapp_web— 35 passedcargo fmt -- --check— cleanCloses #288
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration Changes
CLI/Docs
Behavior Changes
Tests