feat(tools/bb): reply, edit, and unsend message actions

[maxtongwang]

Summary

• Base branch target: main
• Problem: The LLM agent cannot reply to a specific iMessage thread, edit a sent message, or retract a sent message — limiting conversational accuracy.
• Why it matters: Thread-aware reply, edit, and unsend are standard iMessage features required for full agent parity with OpenClaw's BlueBubbles extension.
• What changed: New bluebubbles_message tool with a single action dispatch (reply, edit, unsend). Uses BB private-API endpoints: reply via selectedMessageGuid, edit via /message/{guid}/edit, unsend via /message/{guid}/unsend. Registered in the tool factory alongside group and send-attachment tools.
• What did not change: No changes to the channel receive path, config schema, or existing tools.

Label Snapshot (required)

• Risk label: risk: medium
• Size label: size: S
• Scope labels: tool
• Module labels: tool: bluebubbles
• Contributor tier label: (auto-managed)

Change Metadata

• Change type: feature
• Primary scope: tool

Linked Issue

• Depends on feat(tool/bluebubbles): send_attachment tool for iMessage media #2582
• Linear issue key(s): external contributor — no Linear key

Supersede Attribution (required when Supersedes # is used)

N/A

Validation Evidence (required)

cargo fmt --all -- --check   # pass
cargo clippy --all-targets -- -D warnings  # pass (delta scope)
cargo test   # pass

• Evidence: fork PR [LOW] HEARTBEAT.md created with default permissions (world-readable) #18 merged after full CI + CodeRabbit + human review. Build clean on stacked branch.

Security Impact (required)

• New permissions/capabilities? Yes — agent can now edit/delete previously sent iMessages.
• New external network calls? Yes — PATCH/DELETE to BB server message endpoints.
• Secrets/tokens handling changed? No
• File system access scope changed? No
• Risk: Edit/unsend could modify messages the user did not intend. Mitigated by the agent only acting on explicit user instruction.

Privacy and Data Hygiene (required)

• Data-hygiene status: pass
• Neutral wording confirmation: pass

Compatibility / Migration

• Backward compatible? Yes — purely additive
• Config/env changes? No
• Migration needed? No

i18n Follow-Through (required when docs or user-facing wording changes)

• i18n follow-through triggered? No — no user-facing docs updated.

Human Verification (required)

• Verified scenarios: all three actions (reply, edit, unsend) schema validated against BB API docs; partIndex default 0 documented.
• Edge cases: empty message_id, missing text for reply/edit, unknown action returns structured error.
• What was not verified: live iMessage thread on physical hardware.

Side Effects / Blast Radius (required)

• Affected subsystems: src/tools/ (new file + mod.rs registration).
• Potential unintended effects: None — purely additive.

Agent Collaboration Notes (recommended)

• Agent tools used: Claude Code
• Confirmation: naming + architecture boundaries followed per AGENTS.md + CONTRIBUTING.md.

Rollback Plan (required)

• Fast rollback: revert src/tools/bluebubbles_message.rs + three-line mod.rs change.
• Feature flags: none.
• Observable failure symptoms: BlueBubblesMessageTool absent from tool list; API failures surface as ToolResult { success: false }.

Risks and Mitigations

• Risk: unsend on already-read messages may fail silently on older iOS.
  • Mitigation: BB server returns HTTP error; tool surfaces it as ToolResult { success: false, error: Some(...) }.

Summary by CodeRabbit

• New Features

  • BlueBubbles iMessage channel integration with webhook support
  • Audio message transcription via local Whisper
  • Message reactions and tapback support
  • OAuth authentication system (Google with PKCE)
  • BlueBubbles group management, messaging, and attachment tools
  • DM and group chat policy controls (Open/Allowlist/Disabled)
  • Read receipt support for BlueBubbles

• Documentation

  • Updated channel reference with BlueBubbles and ACP configuration
  • Extended configuration documentation with policy and transcription settings
  • Added internationalization support (Greek, Vietnamese)

• Bug Fixes

  • Fixed HTTP redirect handling to properly validate policy for redirect targets

Review Fixes (Round 2)

Propagated docs fix from base branch feat/bluebubbles-transcription-tapback via rebase:

Chinese command text in English docs: docs/channels-reference.md natural-language approval command examples updated from 授权工具 shell to approve tool shell.

Review Fixes (Round 3)

No-panic constructor: bluebubbles_message.rs replaced .expect("valid reqwest client config") with .unwrap_or_else(|_| reqwest::Client::new()) to prevent panic on TLS misconfiguration.

Also propagated from base branches: transcription temp dir cleanup, error_page title HTML escaping, icon size check on decoded bytes.

Review Fixes (Round 6)

• oauth.rs: propagate HTTP client build failure — no silent timeout loss
• oauth.rs: validate PKCE state before consuming session file (CSRF fix)
• oauth.rs: use POST form body for token revocation (RFC 7009 compliance)
• transcription.rs: explicit fail on non-UTF-8 temp paths, not empty-string passthrough
• transcription.rs: clean up partial ffmpeg WAV on conversion failure
• bluebubbles.rs: remove attachment GUID from error log
• claude-review.yml: fix step-level PR condition (!= '' → != null)

Review Fixes (Round 7)

• bluebubbles_message.rs: explicit anyhow::Result<Self> constructor
• bluebubbles_message.rs: all 3 transport error paths (reply, edit, unsend) return Ok(ToolResult { success: false }) instead of propagating Err
• src/tools/mod.rs: registration updated to handle anyhow::Result constructor

Review Fixes (Round 8)

• bluebubbles_message.rs: inject Arc<SecurityPolicy> — added can_act() and record_action() checks matching the pattern used by all other side-effect tools (ShellTool, FileReadTool, CronAddTool, etc.)

Review Fixes (cargo fmt)

• Applied cargo fmt --all to files changed in this PR's diff — collapsed single-use iterator chains and reformatted match arm bodies to comply with project rustfmt settings

[maxtongwang]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[maxtongwang]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[maxtongwang]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

♻️ Duplicate comments (6)

src/tools/bluebubbles_send_attachment.rs (2)

161-165: ⚠️ Potential issue | 🟡 Minor

Trim caption before using it in the multipart form.

Whitespace-only captions currently pass through and are sent as message text. Trim at ingestion to keep sanitization consistent with other string inputs.

🧼 Suggested patch

         let caption = args
             .get("caption")
             .and_then(|v| v.as_str())
+            .map(str::trim)
             .unwrap_or("")
             .to_string();

As per coding guidelines: `src/tools/**/*.rs` requires validating and sanitizing all inputs.

Also applies to: 217-219

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/bluebubbles_send_attachment.rs` around lines 161 - 165, The caption
string extracted via args.get("caption").and_then(|v|
v.as_str()).unwrap_or("").to_string() must be trimmed immediately to strip
whitespace-only values; replace the current assignment with one that calls
.trim() (and consider .to_string() after trimming) so empty/whitespace captions
become empty strings before being added to the multipart form, and apply the
same trimming/sanitization to the other caption extraction occurrences around
the code referenced (similar to lines 217-219) to keep input handling
consistent; update any downstream logic that checks for empty captions to rely
on the trimmed value (e.g., the local variable caption).

---

128-191: ⚠️ Potential issue | 🟡 Minor

Enforce a hard decoded-byte limit after base64 decode.

Line 128 checks encoded size only. Add an explicit file_bytes.len() cap after Line 182 so boundary behavior is exact and deterministic.

📏 Suggested patch

-        const MAX_ATTACHMENT_B64_LEN: usize = 34 * 1024 * 1024; // ~25 MiB decoded (base64 overhead ~4/3)
+        const MAX_ATTACHMENT_BYTES: usize = 25 * 1024 * 1024;
+        const MAX_ATTACHMENT_B64_LEN: usize = ((MAX_ATTACHMENT_BYTES + 2) / 3) * 4;
@@
         let file_bytes =
             match base64::Engine::decode(&base64::engine::general_purpose::STANDARD, data_b64) {
                 Ok(b) => b,
@@
             };
+        if file_bytes.len() > MAX_ATTACHMENT_BYTES {
+            return Ok(ToolResult {
+                success: false,
+                output: String::new(),
+                error: Some(format!(
+                    "decoded attachment exceeds maximum allowed size ({MAX_ATTACHMENT_BYTES} bytes)"
+                )),
+            });
+        }

As per coding guidelines: `src/{security,runtime,gateway,tools}/**/*.rs` changes should include boundary/failure-mode validation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/bluebubbles_send_attachment.rs` around lines 128 - 191, The code
only enforces an encoded-size cap (MAX_ATTACHMENT_B64_LEN) but not a hard
decoded-byte limit; after the base64 decode that produces file_bytes (the Ok(b)
arm in the match that assigns file_bytes) add a boundary check like a
MAX_ATTACHMENT_DECODED_LEN constant and if file_bytes.len() >
MAX_ATTACHMENT_DECODED_LEN return the same ToolResult failure shape with an
error message indicating the decoded attachment exceeds the allowed size; place
this check immediately after the decode success (where file_bytes is assigned)
so failure-mode is deterministic and consistent with other validations
(reference the file_bytes variable and the base64::Engine::decode match).

src/gateway/mod.rs (4)

2377-2378: ⚠️ Potential issue | 🟡 Minor

Fix stale response-code comment (200 vs 202).

The comment says “Returning 200 immediately”, but the handler returns StatusCode::ACCEPTED (202) at Line 2485.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/gateway/mod.rs` around lines 2377 - 2378, The comment above the handler
is stale: it says “Returning 200 immediately” but the code returns
StatusCode::ACCEPTED (202). Update the comment to reflect the actual response
code (202 / StatusCode::ACCEPTED) and/or reference "Accepted" semantics so it
matches the handler that returns StatusCode::ACCEPTED (e.g., in the async
request handler where the comment appears).

---

562-591: ⚠️ Potential issue | 🟠 Major

Do not silently disable BlueBubbles on constructor failure.

Line 590-Line 591 converts initialization errors into None, which can hide misconfiguration and continue with reduced capabilities.

Suggested fix

 let bluebubbles_channel: Option<Arc<BlueBubblesChannel>> = config
     .channels_config
     .bluebubbles
     .as_ref()
@@
         .transpose()
-        .map_err(|e| {
-            tracing::error!("Failed to initialize BlueBubbles channel: {e}");
-            e
-        })
-        .ok()
-        .flatten();
+        .map_err(|e| anyhow::anyhow!("Failed to initialize BlueBubbles channel: {e}"))?;

As per coding guidelines: src/**/*.rs: “Prefer explicit bail!/errors for unsupported or unsafe states; never silently broaden permissions/capabilities.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/gateway/mod.rs` around lines 562 - 591, The current chain around
BlueBubblesChannel::new maps initialization errors into None
(bluebubbles_channel) which silently disables the channel; instead, surface the
error so initialization fails loudly: remove the .ok().flatten() pattern and the
.map_err(...).ok() swallowing, and return or propagate the error (e.g., using
anyhow::bail! or ? from the surrounding function) when BlueBubblesChannel::new
(and its .with_policies/.with_transcription) fails, so misconfiguration does not
silently continue with reduced capabilities.

---

2381-2482: ⚠️ Potential issue | 🟠 Major

Add an outer timeout to bound semaphore permit lifetime.

The detached worker keeps the permit until the task finishes; if downstream work hangs, permits can be pinned indefinitely and eventually starve the pool.

Suggested fix

 tokio::spawn(async move {
     let _permit = permit; // released when this task completes
-    let messages = bb.parse_webhook_payload_with_transcription(&payload).await;
+    let work = async {
+        let messages = bb.parse_webhook_payload_with_transcription(&payload).await;
 
-    // Delivery receipt: send immediately for ALL inbound group messages, whether
-    // or not they pass the mention gate.  This shows "Delivered" on the sender's
-    // side as long as ZeroClaw is online.  DM delivery is handled by the OS/iMessage
-    // stack automatically; we only need to fire it explicitly for group chats.
-    {
+        // existing background processing body...
+        {
             // existing logic unchanged
-    }
+        }
 
-    for msg in &messages {
-        // existing logic unchanged
-    }
+        for msg in &messages {
+            // existing logic unchanged
+        }
+    };
+
+    if tokio::time::timeout(Duration::from_secs(180), work)
+        .await
+        .is_err()
+    {
+        tracing::error!("BlueBubbles background worker timed out");
+    }
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/gateway/mod.rs` around lines 2381 - 2482, The spawned worker currently
captures the semaphore permit via let _permit = permit and holds it until the
whole async task finishes, so add an outer timeout (using tokio::time::timeout)
around the task's main work to bound how long the permit can be held: wrap the
body that starts with messages =
bb.parse_webhook_payload_with_transcription(&payload).await and includes the for
msg in &messages loop and run_gateway_chat_with_tools(...) in a
tokio::time::timeout(duration, async move { ... }).await, handle the
Err(elapsed) case by logging the timeout and explicitly dropping the permit
(i.e., ensuring _permit is dropped) so the semaphore is released, and keep the
normal success path unchanged so permit is released when the task completes.

---

2358-2375: ⚠️ Potential issue | 🟠 Major

Please add explicit tests for the new BlueBubbles 202/503 boundary behavior.

This path introduces important queue-boundary semantics (accepted vs exhausted) but there is no direct assertion in this file’s tests for those outcomes.

Based on learnings: for src/security/**/*.rs|src/runtime/**/*.rs|src/gateway/**/*.rs|src/tools/**/*.rs, include at least one boundary/failure-mode validation and document rollback strategy.

Also applies to: 2484-2487

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/gateway/mod.rs` around lines 2358 - 2375, Add explicit unit/integration
tests that assert the BlueBubbles webhook path returns 202 when
BB_WORKER_SEMAPHORE.try_acquire_owned() succeeds and returns
StatusCode::SERVICE_UNAVAILABLE (503) with the JSON error payload when the
semaphore is exhausted; simulate both cases by mocking or temporarily replacing
BB_WORKER_SEMAPHORE to provide an owned permit and to force try_acquire_owned()
-> Err, then call the handler (the function invoking try_acquire_owned) and
assert the response status and body match the expected 202/JSON and
503/{"error":"worker pool exhausted, retry later"} outcomes; also add a short
doc comment or test metadata describing the rollback/retry strategy for the
queued work in failure mode so the boundary behavior is documented alongside the
tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/gateway/mod.rs`:
- Around line 2377-2378: The comment above the handler is stale: it says
“Returning 200 immediately” but the code returns StatusCode::ACCEPTED (202).
Update the comment to reflect the actual response code (202 /
StatusCode::ACCEPTED) and/or reference "Accepted" semantics so it matches the
handler that returns StatusCode::ACCEPTED (e.g., in the async request handler
where the comment appears).
- Around line 562-591: The current chain around BlueBubblesChannel::new maps
initialization errors into None (bluebubbles_channel) which silently disables
the channel; instead, surface the error so initialization fails loudly: remove
the .ok().flatten() pattern and the .map_err(...).ok() swallowing, and return or
propagate the error (e.g., using anyhow::bail! or ? from the surrounding
function) when BlueBubblesChannel::new (and its
.with_policies/.with_transcription) fails, so misconfiguration does not silently
continue with reduced capabilities.
- Around line 2381-2482: The spawned worker currently captures the semaphore
permit via let _permit = permit and holds it until the whole async task
finishes, so add an outer timeout (using tokio::time::timeout) around the task's
main work to bound how long the permit can be held: wrap the body that starts
with messages = bb.parse_webhook_payload_with_transcription(&payload).await and
includes the for msg in &messages loop and run_gateway_chat_with_tools(...) in a
tokio::time::timeout(duration, async move { ... }).await, handle the
Err(elapsed) case by logging the timeout and explicitly dropping the permit
(i.e., ensuring _permit is dropped) so the semaphore is released, and keep the
normal success path unchanged so permit is released when the task completes.
- Around line 2358-2375: Add explicit unit/integration tests that assert the
BlueBubbles webhook path returns 202 when
BB_WORKER_SEMAPHORE.try_acquire_owned() succeeds and returns
StatusCode::SERVICE_UNAVAILABLE (503) with the JSON error payload when the
semaphore is exhausted; simulate both cases by mocking or temporarily replacing
BB_WORKER_SEMAPHORE to provide an owned permit and to force try_acquire_owned()
-> Err, then call the handler (the function invoking try_acquire_owned) and
assert the response status and body match the expected 202/JSON and
503/{"error":"worker pool exhausted, retry later"} outcomes; also add a short
doc comment or test metadata describing the rollback/retry strategy for the
queued work in failure mode so the boundary behavior is documented alongside the
tests.

In `@src/tools/bluebubbles_send_attachment.rs`:
- Around line 161-165: The caption string extracted via
args.get("caption").and_then(|v| v.as_str()).unwrap_or("").to_string() must be
trimmed immediately to strip whitespace-only values; replace the current
assignment with one that calls .trim() (and consider .to_string() after
trimming) so empty/whitespace captions become empty strings before being added
to the multipart form, and apply the same trimming/sanitization to the other
caption extraction occurrences around the code referenced (similar to lines
217-219) to keep input handling consistent; update any downstream logic that
checks for empty captions to rely on the trimmed value (e.g., the local variable
caption).
- Around line 128-191: The code only enforces an encoded-size cap
(MAX_ATTACHMENT_B64_LEN) but not a hard decoded-byte limit; after the base64
decode that produces file_bytes (the Ok(b) arm in the match that assigns
file_bytes) add a boundary check like a MAX_ATTACHMENT_DECODED_LEN constant and
if file_bytes.len() > MAX_ATTACHMENT_DECODED_LEN return the same ToolResult
failure shape with an error message indicating the decoded attachment exceeds
the allowed size; place this check immediately after the decode success (where
file_bytes is assigned) so failure-mode is deterministic and consistent with
other validations (reference the file_bytes variable and the
base64::Engine::decode match).

---

ℹ️ Review info

Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52da9d61-69fd-4db9-b69d-ba47be72d4e0

📥 Commits

Reviewing files that changed from the base of the PR and between 361c5f5 and 47699f9.

📒 Files selected for processing (8)

• docs/channels-reference.md
• docs/i18n/vi/channels-reference.md
• src/channels/bluebubbles.rs
• src/gateway/mod.rs
• src/tools/bluebubbles_group.rs
• src/tools/bluebubbles_message.rs
• src/tools/bluebubbles_send_attachment.rs
• src/tools/mod.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• src/tools/mod.rs
• src/tools/bluebubbles_group.rs
• docs/i18n/vi/channels-reference.md

[maxtongwang]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[maxtongwang]

@coderabbitai review

[maxtongwang]

Closing — no longer needed.