feat(tools/whatsapp_data): expose local WhatsApp store to agent (#1341)#1373
Merged
senamakel merged 13 commits intotinyhumansai:mainfrom May 9, 2026
Merged
Conversation
…eset Production callers still get strict idempotency: `init` is a no-op once a store is set. Tests can now call `reset_for_tests()` between cases so they do not inherit a stale handle pointing at a tempdir that has already been dropped (`WhatsAppDataStore::open_conn()` reopens sqlite per call, so that scenario surfaces as "Unable to open the database file" once the file is unlinked). Refs tinyhumansai#1341 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inyhumansai#1341) Wraps the existing `whatsapp_data_list_chats`, `whatsapp_data_list_messages`, and `whatsapp_data_search_messages` RPC handlers as agent-callable Tool impls. Each tool annotates its response with `"provider": "whatsapp"` so the orchestrator can cite WhatsApp as the source. The matching write-path controller `whatsapp_data_ingest` is intentionally NOT wrapped — adding a Tool impl for it would reopen the read-only boundary the registry already enforces (see `src/core/all.rs::build_internal_only_controllers`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ansai#1341) Adds the three new read-only WhatsApp tools alongside the memory_tree_* family in `all_tools_with_runtime`. The internal-only ingest handler remains registered separately via `build_internal_only_controllers` and is unreachable from this catalog. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sai#1341) Adds the three direct WhatsApp tools to the orchestrator's `[tools].named` list so it can route exact-lookup, per-chat read, and keyword-search intents to the local SQLite store. Cross-source / action-item flows continue to go through the existing `memory_tree_*` tools, which already see whatsapp data because the scanner dual-writes via `memory_doc_ingest`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures why the scanner writes to both `whatsapp_data.db` (exact lookup) and the memory tree (semantic / cross-source recall), how idempotency keys keep the dual-write safe to retry, where the read-only boundary is enforced, and which tool family covers which intent shape. Addresses the "no duplicate-storage confusion" acceptance criterion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `whatsapp_data_agent_tools_e2e_1341` exercising the three new read-only tools against the global store: chat enumeration, per-chat message read, keyword search (with account scoping and empty-result case), and an explicit boundary regression that asserts `whatsapp_data_ingest` is absent from the agent-facing controller schema list. Also resets the global store at the start of `whatsapp_data_ingest_and_query_e2e` so the suite is order-independent under the new `RwLock<Option<...>>` global. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yhumansai#1341) Tracks the new agent-tool surface added by this PR alongside the existing WhatsApp Connection (10.1.2) and Message Sync (10.3.1) rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughA new WhatsApp agent tool surface is added: three read-only tools (list chats, list messages, search messages) wrap WhatsApp RPC handlers with provenance. Module exports and registry wiring expose them to the orchestrator. Global store lifecycle was refactored for test isolation, search now matches sender/body, docs/coverage and E2E tests were added. ChangesWhatsApp Agent Tools Integration
Sequence Diagram(s)sequenceDiagram
participant Client
participant Orchestrator
participant Tool
participant RPC
participant DB
Client->>Orchestrator: user intent (e.g., "find messages")
Orchestrator->>Tool: invoke whatsapp_data_* tool
Tool->>RPC: whatsapp_rpc::... request
RPC->>DB: query whatsapp_data.db
DB-->>RPC: rows
RPC-->>Tool: RpcOutcome
Tool-->>Orchestrator: ToolResult {provider:"whatsapp", payload}
Orchestrator-->>Client: reply (with provenance)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/openhuman/whatsapp_data/global.rs (1)
79-90: 🏗️ Heavy liftKeep
reset_for_tests()out of production builds.This is now a public runtime API even though the doc comment says production callers must never invoke it. One accidental call can clear/rebind the process-global store mid-session and make later handlers fail or read from a different workspace. Please gate this behind a test-only feature or move it into a dedicated test-support surface instead of shipping it in the normal crate API.
One possible direction
-#[doc(hidden)] -pub fn reset_for_tests() { +#[cfg(any(test, feature = "test-utils"))] +#[doc(hidden)] +pub fn reset_for_tests() { if let Ok(mut guard) = GLOBAL_STORE.write() { *guard = None; } }You'd then enable that feature from the integration-test target rather than exposing the hook in normal builds.
🤖 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 `@src/openhuman/whatsapp_data/global.rs` around lines 79 - 90, The public function reset_for_tests currently ships in production; restrict it to test-only usage by gating it with a compile-time cfg (e.g., #[cfg(any(test, feature = "test-support"))]) or move it into a test-only module so it is not exported in normal builds; update the crate feature list to add an optional "test-support" feature and enable that feature from integration tests instead of leaving reset_for_tests public, and ensure the symbol referencing GLOBAL_STORE remains accessible under the same cfg so the implementation compiles only for test builds.src/openhuman/tools/impl/whatsapp_data/search_messages.rs (1)
51-69: ⚡ Quick winAdd error-path logging and safe correlation fields here.
This new tool flow only logs entry/exit, so invalid args and RPC failures disappear into a generic error string. Please log the failure branch too, and include non-PII correlation fields like
account_id,chat_id,limit, andquery_lenrather than the raw query text.Possible shape
async fn execute(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> { log::debug!("[tool][whatsapp_data] search_messages invoked"); let req: SearchMessagesRequest = serde_json::from_value(args).map_err(|e| { + log::debug!("[tool][whatsapp_data] search_messages invalid_args error={e}"); anyhow::anyhow!("invalid arguments for whatsapp_data_search_messages: {e}") })?; let outcome = whatsapp_rpc::whatsapp_data_search_messages(req) .await - .map_err(|e| anyhow::anyhow!("whatsapp_data_search_messages: {e}"))?; + .map_err(|e| { + log::debug!( + "[tool][whatsapp_data] search_messages rpc_error account_id={:?} chat_id={:?} limit={:?} query_len={} error={e}", + req.account_id, + req.chat_id, + req.limit, + req.query.len() + ); + anyhow::anyhow!("whatsapp_data_search_messages: {e}") + })?; let messages = outcome.value; log::debug!( - "[tool][whatsapp_data] search_messages returning count={}", - messages.len() + "[tool][whatsapp_data] search_messages returning account_id={:?} chat_id={:?} limit={:?} query_len={} count={}", + req.account_id, + req.chat_id, + req.limit, + req.query.len(), + messages.len() );As per coding guidelines, "Use
log/tracingatdebugortracelevel on RPC entry/exit, error paths, state transitions, and any branch that is hard to infer from tests; include stable prefixes (e.g.[domain],[rpc]) and correlation fields (request IDs, method names, entity IDs)."🤖 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 `@src/openhuman/tools/impl/whatsapp_data/search_messages.rs` around lines 51 - 69, In execute (async fn execute in whatsapp_data/search_messages.rs) add error-path logging and safe correlation fields: when serde_json::from_value fails, log a prefixed debug/error message (e.g. "[tool][whatsapp_data][parse]") including any available stable correlation IDs (if parsable) or at least a request context placeholder; after parsing but before calling whatsapp_rpc::whatsapp_data_search_messages, log an entry with prefix "[tool][whatsapp_data][rpc]" and include non-PII fields from the parsed SearchMessagesRequest (account_id, chat_id, limit, and query_len = req.query.len() or 0) but do not log the raw query text; on RPC error (map_err branch) log the error with a prefixed "[tool][whatsapp_data][rpc][error]" plus the same correlation fields and then return the mapped anyhow error as before so behavior is unchanged.src/openhuman/tools/impl/whatsapp_data/list_messages.rs (1)
60-67: ⚡ Quick winAdd explicit error-path debug logs with stable correlation fields.
Entry/exit logs are present, but failures in argument parsing and RPC execution are currently silent in logs. Add debug/trace logging in both
map_errbranches so production triage can distinguish bad tool args vs upstream RPC failures.Proposed patch
async fn execute(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> { log::debug!("[tool][whatsapp_data] list_messages invoked"); let req: ListMessagesRequest = serde_json::from_value(args).map_err(|e| { + log::debug!( + "[tool][whatsapp_data] list_messages invalid_args method=whatsapp_data_list_messages error={}", + e + ); anyhow::anyhow!("invalid arguments for whatsapp_data_list_messages: {e}") })?; let outcome = whatsapp_rpc::whatsapp_data_list_messages(req) .await - .map_err(|e| anyhow::anyhow!("whatsapp_data_list_messages: {e}"))?; + .map_err(|e| { + log::debug!( + "[tool][whatsapp_data] list_messages rpc_error method=whatsapp_data_list_messages error={}", + e + ); + anyhow::anyhow!("whatsapp_data_list_messages: {e}") + })?;As per coding guidelines: "Use
log/tracingatdebugortracelevel on RPC entry/exit, error paths, state transitions... include stable prefixes and correlation fields."🤖 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 `@src/openhuman/tools/impl/whatsapp_data/list_messages.rs` around lines 60 - 67, Add explicit debug/trace logging to the error branches in execute: when serde_json::from_value fails in the map_err for ListMessagesRequest parsing, log a stable prefix (e.g., "whatsapp_data:list_messages:parse_error") plus a correlation field (e.g., a generated short request_id or the raw args summary) and the parse error; similarly, when whatsapp_rpc::whatsapp_data_list_messages returns Err in its map_err, log a stable prefix (e.g., "whatsapp_data:list_messages:rpc_error"), include the same correlation field and the RPC error details before converting to anyhow::Error; update the execute function to generate/propagate the correlation id and use log::debug! or trace! for these paths so bad args and upstream failures are distinguishable.
🤖 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 `@docs/TEST-COVERAGE-MATRIX.md`:
- Around line 330-331: Change the duplicate matrix leaf label "10.3.3" for the
"Real-Time vs Delayed Sync" row to "10.3.4" in the TEST-COVERAGE-MATRIX.md table
(the row with the description "Real-Time vs Delayed Sync" and reference
`src/openhuman/channels/tests/runtime_dispatch.rs`), and update any
manually-maintained rolled-up totals or index references elsewhere in the matrix
to reflect the new numbering.
In `@docs/whatsapp-data-flow.md`:
- Around line 9-39: In docs/whatsapp-data-flow.md update the fenced diagram
block to include a language tag (for example "text") after the opening triple
backticks so the markdown linter stops flagging MD040; locate the diagram fenced
block (the ASCII diagram between the triple backticks) and change the opening
fence from ``` to ```text (no other changes).
---
Nitpick comments:
In `@src/openhuman/tools/impl/whatsapp_data/list_messages.rs`:
- Around line 60-67: Add explicit debug/trace logging to the error branches in
execute: when serde_json::from_value fails in the map_err for
ListMessagesRequest parsing, log a stable prefix (e.g.,
"whatsapp_data:list_messages:parse_error") plus a correlation field (e.g., a
generated short request_id or the raw args summary) and the parse error;
similarly, when whatsapp_rpc::whatsapp_data_list_messages returns Err in its
map_err, log a stable prefix (e.g., "whatsapp_data:list_messages:rpc_error"),
include the same correlation field and the RPC error details before converting
to anyhow::Error; update the execute function to generate/propagate the
correlation id and use log::debug! or trace! for these paths so bad args and
upstream failures are distinguishable.
In `@src/openhuman/tools/impl/whatsapp_data/search_messages.rs`:
- Around line 51-69: In execute (async fn execute in
whatsapp_data/search_messages.rs) add error-path logging and safe correlation
fields: when serde_json::from_value fails, log a prefixed debug/error message
(e.g. "[tool][whatsapp_data][parse]") including any available stable correlation
IDs (if parsable) or at least a request context placeholder; after parsing but
before calling whatsapp_rpc::whatsapp_data_search_messages, log an entry with
prefix "[tool][whatsapp_data][rpc]" and include non-PII fields from the parsed
SearchMessagesRequest (account_id, chat_id, limit, and query_len =
req.query.len() or 0) but do not log the raw query text; on RPC error (map_err
branch) log the error with a prefixed "[tool][whatsapp_data][rpc][error]" plus
the same correlation fields and then return the mapped anyhow error as before so
behavior is unchanged.
In `@src/openhuman/whatsapp_data/global.rs`:
- Around line 79-90: The public function reset_for_tests currently ships in
production; restrict it to test-only usage by gating it with a compile-time cfg
(e.g., #[cfg(any(test, feature = "test-support"))]) or move it into a test-only
module so it is not exported in normal builds; update the crate feature list to
add an optional "test-support" feature and enable that feature from integration
tests instead of leaving reset_for_tests public, and ensure the symbol
referencing GLOBAL_STORE remains accessible under the same cfg so the
implementation compiles only for test builds.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 666c7ea7-b56f-4fa1-bc3b-af104df34376
📒 Files selected for processing (11)
docs/TEST-COVERAGE-MATRIX.mddocs/whatsapp-data-flow.mdsrc/openhuman/agent/agents/orchestrator/agent.tomlsrc/openhuman/tools/impl/mod.rssrc/openhuman/tools/impl/whatsapp_data/list_chats.rssrc/openhuman/tools/impl/whatsapp_data/list_messages.rssrc/openhuman/tools/impl/whatsapp_data/mod.rssrc/openhuman/tools/impl/whatsapp_data/search_messages.rssrc/openhuman/tools/ops.rssrc/openhuman/whatsapp_data/global.rstests/json_rpc_e2e.rs
…nsai#1341) Person-name queries like "what did Alice say" only return rows when search also looks at the `sender` column — the sender's own name almost never appears inside the message body. Each branch of the dynamic-WHERE match now reads `body LIKE ? OR sender LIKE ?` with the existing pattern re-bound. Adds a `search_messages_matches_sender_name` unit test that ingests a message whose body has no occurrence of the sender's name and asserts the search still surfaces it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yhumansai#1341) Smoke testing turned up the orchestrator picking `search_messages` for time-window queries like "who messaged me on WhatsApp in the last 3 hours" and getting empty results because the search hits message body text only. Re-shapes the three tool descriptions so: - `list_chats` advertises the recency / contact-resolution intents and notes the `last_message_ts DESC` ordering. - `list_messages` becomes the explicit time-window read tool, with a `since_ts`/`until_ts` recipe paired with `current_time`. - `search_messages` says "do NOT use for time-only queries" and calls out the new sender-column match landed in the previous commit. Pure-text update — no behaviour change in the Rust code paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/openhuman/whatsapp_data/store.rs (2)
369-434: ⚡ Quick winAdd branch-selection diagnostics to
search_messages.This function now has four query branches but logs only the final row count. Add a debug entry log with stable fields (
scope,has_account_filter,has_chat_filter,limit,query_len) so branch behavior is observable without logging raw query text.Proposed patch
pub fn search_messages(&self, req: &SearchMessagesRequest) -> Result<Vec<WhatsAppMessage>> { if req.query.trim().is_empty() { return Ok(vec![]); } let conn = self.open_conn()?; let limit = req.limit.unwrap_or(20) as i64; let pattern = format!("%{}%", req.query.replace('%', "\\%").replace('_', "\\_")); + let scope = match (&req.account_id, &req.chat_id) { + (Some(_), Some(_)) => "account+chat", + (Some(_), None) => "account", + (None, Some(_)) => "chat", + (None, None) => "all", + }; + log::debug!( + "[whatsapp_data] search_messages start scope={} has_account_filter={} has_chat_filter={} limit={} query_len={}", + scope, + req.account_id.is_some(), + req.chat_id.is_some(), + limit, + req.query.chars().count() + );As per coding guidelines: "Use
log/tracingatdebugortracelevel on RPC entry/exit, error paths, state transitions, and any branch that is hard to infer from tests; include stable prefixes ... Never log ... full PII."🤖 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 `@src/openhuman/whatsapp_data/store.rs` around lines 369 - 434, search_messages currently executes one of four SQL branches but only logs the final row count; add a debug log immediately after selecting the branch (before executing/collecting rows) that records stable, non-PII diagnostics: scope="search_messages", has_account_filter = req.account_id.is_some(), has_chat_filter = req.chat_id.is_some(), limit = limit, query_len = pattern.len() (or pattern.chars().count()) so branch selection is observable without emitting raw query text or message bodies; place this debug call in the same scope where msgs is determined (inside the match handling for req.account_id/req.chat_id) or immediately after the match but before returning msgs, using the module's logging facility (e.g., log::debug! or tracing::debug!) and do not include any PII like account/chat IDs, message bodies, or sender identifiers.
616-667: 🏗️ Heavy lift
store.rsis beyond the size guideline—split tests out of this module.This file is already far above the 500-line target and keeps growing with additional tests. Move
#[cfg(test)] mod teststo a sibling test module/file (e.g.,store_tests.rs) so store runtime code stays focused and easier to navigate.As per coding guidelines: "Keep source files to ≤ ~500 lines per file; split modules when growing larger."
🤖 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 `@src/openhuman/whatsapp_data/store.rs` around lines 616 - 667, The tests block (including search_messages_matches_sender_name and other #[cfg(test)] items) should be moved out of the oversized store.rs into a sibling test module file: create a new file (e.g., store_tests.rs) containing #[cfg(test)] mod tests { use super::*; /* paste all test functions (make_store(), search_messages_matches_sender_name, etc.) and any test helpers */ } and remove the in-file #[cfg(test)] mod tests from store.rs; ensure any symbols referenced by tests (make_store, SearchMessagesRequest, upsert_messages, upsert_chats, ChatMeta, IngestMessage) have the needed visibility (pub(crate) or pub) so the tests can access them after moving.
🤖 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.
Nitpick comments:
In `@src/openhuman/whatsapp_data/store.rs`:
- Around line 369-434: search_messages currently executes one of four SQL
branches but only logs the final row count; add a debug log immediately after
selecting the branch (before executing/collecting rows) that records stable,
non-PII diagnostics: scope="search_messages", has_account_filter =
req.account_id.is_some(), has_chat_filter = req.chat_id.is_some(), limit =
limit, query_len = pattern.len() (or pattern.chars().count()) so branch
selection is observable without emitting raw query text or message bodies; place
this debug call in the same scope where msgs is determined (inside the match
handling for req.account_id/req.chat_id) or immediately after the match but
before returning msgs, using the module's logging facility (e.g., log::debug! or
tracing::debug!) and do not include any PII like account/chat IDs, message
bodies, or sender identifiers.
- Around line 616-667: The tests block (including
search_messages_matches_sender_name and other #[cfg(test)] items) should be
moved out of the oversized store.rs into a sibling test module file: create a
new file (e.g., store_tests.rs) containing #[cfg(test)] mod tests { use
super::*; /* paste all test functions (make_store(),
search_messages_matches_sender_name, etc.) and any test helpers */ } and remove
the in-file #[cfg(test)] mod tests from store.rs; ensure any symbols referenced
by tests (make_store, SearchMessagesRequest, upsert_messages, upsert_chats,
ChatMeta, IngestMessage) have the needed visibility (pub(crate) or pub) so the
tests can access them after moving.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93b95a59-cc3a-4941-a84a-4609162a694b
📒 Files selected for processing (4)
src/openhuman/tools/impl/whatsapp_data/list_chats.rssrc/openhuman/tools/impl/whatsapp_data/list_messages.rssrc/openhuman/tools/impl/whatsapp_data/search_messages.rssrc/openhuman/whatsapp_data/store.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/openhuman/tools/impl/whatsapp_data/list_messages.rs
- src/openhuman/tools/impl/whatsapp_data/search_messages.rs
- src/openhuman/tools/impl/whatsapp_data/list_chats.rs
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tinyhumansai#1341) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rtions) (tinyhumansai#1341) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/tools/impl/whatsapp_data/list_chats.rs (1)
37-40: ⚡ Quick winBound
limitto keep tool responses predictable and safe.Line 37 currently enforces only a minimum. Add a hard upper bound (schema + runtime validation) so a single call can’t request an excessively large result set and bloat tool output/context.
Proposed patch
"limit": { "type": "integer", "minimum": 1, + "maximum": 200, "description": "Maximum chats to return (default 50)." },let req: ListChatsRequest = serde_json::from_value(args).map_err(|e| { log::debug!("[tool][whatsapp_data] list_chats invalid_args error={e}"); anyhow::anyhow!("invalid arguments for whatsapp_data_list_chats: {e}") })?; + if let Some(limit) = req.limit { + if limit > 200 { + return Err(anyhow::anyhow!( + "invalid arguments for whatsapp_data_list_chats: `limit` must be <= 200" + )); + } + }Also applies to: 51-63
🤖 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 `@src/openhuman/tools/impl/whatsapp_data/list_chats.rs` around lines 37 - 40, The "limit" parameter currently has only a minimum; add a hard upper bound in the JSON schema (add "maximum": 100 or your chosen cap) and enforce that cap at runtime by introducing a MAX_LIMIT constant and validating the incoming limit in the list_chats handler (or the function that parses request params) — return a clear error if limit > MAX_LIMIT or clamp it to MAX_LIMIT before querying. Update both the schema entry for "limit" and the runtime validation path in list_chats (or the params parsing function) so schema and runtime behavior match.
🤖 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.
Nitpick comments:
In `@src/openhuman/tools/impl/whatsapp_data/list_chats.rs`:
- Around line 37-40: The "limit" parameter currently has only a minimum; add a
hard upper bound in the JSON schema (add "maximum": 100 or your chosen cap) and
enforce that cap at runtime by introducing a MAX_LIMIT constant and validating
the incoming limit in the list_chats handler (or the function that parses
request params) — return a clear error if limit > MAX_LIMIT or clamp it to
MAX_LIMIT before querying. Update both the schema entry for "limit" and the
runtime validation path in list_chats (or the params parsing function) so schema
and runtime behavior match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60301b3b-8cdc-427b-8446-b14e04211e7c
📒 Files selected for processing (6)
docs/TEST-COVERAGE-MATRIX.mddocs/whatsapp-data-flow.mdsrc/openhuman/tools/impl/whatsapp_data/list_chats.rssrc/openhuman/tools/impl/whatsapp_data/list_messages.rssrc/openhuman/tools/impl/whatsapp_data/search_messages.rssrc/openhuman/whatsapp_data/global.rs
✅ Files skipped from review due to trivial changes (1)
- docs/TEST-COVERAGE-MATRIX.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/openhuman/tools/impl/whatsapp_data/search_messages.rs
- src/openhuman/whatsapp_data/global.rs
- src/openhuman/tools/impl/whatsapp_data/list_messages.rs
4 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
whatsapp_data_list_chats,whatsapp_data_list_messages,whatsapp_data_search_messages— wrapping the existing JSON-RPC handlers so the orchestrator can route exact-lookup, per-chat read, and keyword-search intents to the local SQLite store.[tools].namedlist. The internal-onlywhatsapp_data_ingestwrite path stays out of the agent surface."provider": "whatsapp"so replies can cite WhatsApp as the source.docs/whatsapp-data-flow.mdand adds matrix row10.3.3.whatsapp_data::globalfromOnceLocktoRwLock<Option<...>>and adds a hiddenreset_for_tests()so suites using their own tempdirs no longer inherit a stale sqlite handle.Problem
whatsapp_dataalready persists chats/messages locally and exposes read-only RPC handlers (whatsapp_data_list_chats,whatsapp_data_list_messages,whatsapp_data_search_messages), but the orchestrator could not reach them: noToolimpl wrapped them, andsrc/openhuman/agent/agents/orchestrator/agent.tomldidn't list them. The agent could only see WhatsApp through the memory tree, which is great for summaries but loses the per-message structure (sender JID, exactchat_id, individual timestamps) that exact-lookup intents like "what did Alice say last Friday" need. Closes #1341.Solution
Three thin Tool impls under
src/openhuman/tools/impl/whatsapp_data/, each modelled onsrc/openhuman/tools/impl/memory/tree/query_global.rs:*Requestfromwhatsapp_data::types.whatsapp_data::rpc::*, unwrap theRpcOutcomeenvelope (the LLM does not need the logs side-channel).{ provider: "whatsapp", count, chats|messages: [...] }so provenance is unambiguous.The new tools are registered in
all_tools_with_runtime(src/openhuman/tools/ops.rs) alongside thememory_tree_*family; they are added to the orchestrator's named list.whatsapp_data_ingestis intentionally NOT wrapped — adding a Tool impl for it would reopen the read-only boundary thatsrc/core/all.rs::build_internal_only_controllersenforces. A regression assertion in the new e2e test enforces that contract.The
whatsapp_data::globalrefactor was incidental:OnceLockplus per-call sqlite open meant a second e2e test using its own tempdir would inherit a handle pointing at an unlinked file ("Unable to open the database file"). Switching toRwLock<Option<...>>and adding#[doc(hidden)] pub fn reset_for_tests()keeps production semantics (init is a no-op once set) while letting tests rebind cleanly. The pre-existingwhatsapp_data_ingest_and_query_e2enow resets before init too, so the suite is order-independent.Submission Checklist
docs/TESTING-STRATEGY.md— newwhatsapp_data_agent_tools_e2e_1341covers list/search/per-chat reads, missing-arg failures, empty-result envelope, and a boundary regression that assertswhatsapp_data_ingestis absent from the agent-facing schema set.whatsapp_data_agent_tools_e2e_1341exercises everyexecutepath. Local quality gates green:cargo test --lib openhuman::tools::implementations::whatsapp_data(9/9),cargo test --lib openhuman::whatsapp_data(8/8),cargo test --test json_rpc_e2e whatsapp(3/3). Coverage gate enforced in CI by.github/workflows/coverage.yml.10.3.3 WhatsApp Agent Retrievaltodocs/TEST-COVERAGE-MATRIX.md.## Related.docs/TESTING-STRATEGY.md).Closes #NNNin the## Relatedsection.Impact
RwLock::readplus one sqliteopen_conn(existing pattern from the RPC layer). No additional sync overhead.whatsapp_data::globalchange is an internal refactor; the public API (init,store,store_if_ready) preserved.reset_for_tests()ispubso it is reachable from integration tests but doc-hidden and explicitly annotated as not for production use.Related
10.1.2 WhatsApp Connection(existing, untouched),10.3.1 Incoming Message Sync(existing, untouched),10.3.3 WhatsApp Agent Retrieval(new, this PR).docs/whatsapp-data-flow.md.memory_tree_*retrieval primitive rather than a new provider-specific tool.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/1341-whatsapp-agent-tools7b4443d1(tip)Validation Run
pnpm --filter openhuman-app format:check— frontend untouched.pnpm typecheck— frontend untouched.cargo test --lib openhuman::tools::implementations::whatsapp_data(9/9),cargo test --lib openhuman::whatsapp_data(8/8),cargo test --test json_rpc_e2e whatsapp(3/3 incl. the newwhatsapp_data_agent_tools_e2e_1341).cargo fmt -- --checkclean on the seven changed core files;cargo check --testsclean (only pre-existing warnings onmain).app/src-tauri/untouched in this PR.Validation Blocked
Behavior Changes
providerprovenance tag.Parity Contract
memory_tree_*tools continue to surface WhatsApp data via the scanner's existingmemory_doc_ingestdual-write; the read-only/internal-only boundary atsrc/core/all.rs::build_internal_only_controllersis unchanged and explicitly tested.whatsapp_data_ingestis not advertised inall_whatsapp_data_controller_schemas(); existingwhatsapp_data_ingest_and_query_e2eandwhatsapp_memory_doc_ingest_e2econtinue to pass after the global refactor.Duplicate / Superseded PR Handling
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Improvements
Tests