Repository navigation
feat(channels): expose WhatsApp Web data to agent via structured RPC API - #1308
Merged
senamakel merged 10 commits intoMay 6, 2026
Merged
Conversation
- Purge pre-login workspace conversation JSONL on first session activation so onboarding/demo threads do not linger under users/local. - When auth identity changes without a restart flip, clear thread Redux and reload from core so the UI cannot show pre-auth thread rows. Closes tinyhumansai#1157 Co-authored-by: Cursor <cursoragent@cursor.com>
- Gate post-identity thread clear/reload on snapshot request id so superseded refreshes cannot mutate thread state after a newer snapshot commits. - Skip error logging when the loadThreads promise loses to a newer refresh. - Document that conversation purge shares the process-wide store mutex with list_threads (no cross-RPC race in-process). Refs tinyhumansai#1157 Co-authored-by: Cursor <cursoragent@cursor.com>
…ligence Screen Recording, Accessibility, and Input Monitoring permissions are macOS-only. On Windows/Linux they showed "Unsupported" badges, misleading users into thinking setup was possible when it isn't. - ScreenIntelligenceSetupModal: early-return with "macOS only" message when platform_supported is false, skipping the permissions flow entirely - ScreenIntelligenceDebugPanel: replace three permission dots with "Platform not supported" indicator on unsupported platforms - Add tests covering both changed components for the unsupported-platform path Closes tinyhumansai#903
…ch after aria-label addition
…orted-permissions-on-non-macos
…orted-permissions-on-non-macos
Adds a dedicated `whatsapp_data` domain that persists and serves structured WhatsApp conversations and messages scraped from the existing CEF webview scanner, making them queryable by the agent. - New `src/openhuman/whatsapp_data/` domain: SQLite-backed store (`whatsapp_data.db`), ops layer with 90-day auto-prune, RPC handlers, controller schemas, and process-global singleton - Four new RPC methods: `openhuman.whatsapp_data_ingest`, `whatsapp_data_list_chats`, `whatsapp_data_list_messages`, `whatsapp_data_search_messages` - Dual-write in `whatsapp_scanner`: each scan tick fires an additive `post_whatsapp_data_ingest` call alongside the existing memory ingest path — no JS injection, existing behaviour untouched - 8 unit tests (store + ops) + E2E test covering ingest/list/search - Updated `about_app` catalog and `namespace_description` for the new namespace Closes tinyhumansai#1279
Contributor
📝 WalkthroughWalkthroughAdds a new whatsapp_data subsystem: sqlite-backed store, types, ops, RPC handlers and schemas, global initialization, core registry wiring, scanner dual-write ingestion, capability catalog entry, and tests (unit + e2e) to ingest, list and search WhatsApp messages. ChangesWhatsApp Data Persistence and RPC
Sequence Diagram(s)sequenceDiagram
participant Scanner as WhatsApp Scanner
participant CoreRPC as Core RPC
participant Global as Global Store
participant Store as SQLite Store
Scanner->>CoreRPC: POST openhuman.whatsapp_data_ingest(chats,messages)
activate CoreRPC
CoreRPC->>Global: require_store()
activate Global
Global-->>CoreRPC: WhatsAppDataStoreRef
deactivate Global
CoreRPC->>Store: upsert_chats(account,chats)
activate Store
Store-->>CoreRPC: upsert_chats count
CoreRPC->>Store: upsert_messages(account,messages)
Store-->>CoreRPC: upsert_messages count
CoreRPC->>Store: prune_old_messages(cutoff_ts)
Store-->>CoreRPC: pruned count
CoreRPC-->>Scanner: RpcOutcome(IngestResult)
deactivate CoreRPC
sequenceDiagram
participant Client as Agent/Client
participant CoreRPC as Core RPC
participant Global as Global Store
participant Store as SQLite Store
Client->>CoreRPC: whatsapp_data_list_messages(req)
activate CoreRPC
CoreRPC->>Global: require_store()
Global-->>CoreRPC: WhatsAppDataStoreRef
CoreRPC->>Store: list_messages(req)
Store-->>CoreRPC: Vec<WhatsAppMessage>
CoreRPC-->>Client: RpcOutcome(Vec<WhatsAppMessage>)
deactivate CoreRPC
Client->>CoreRPC: whatsapp_data_search_messages(req)
activate CoreRPC
CoreRPC->>Global: require_store()
CoreRPC->>Store: search_messages(req)
Store-->>CoreRPC: Vec<WhatsAppMessage>
CoreRPC-->>Client: RpcOutcome(Vec<WhatsAppMessage>)
deactivate CoreRPC
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 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: 10
🧹 Nitpick comments (1)
tests/json_rpc_e2e.rs (1)
3565-3590: ⚡ Quick winAdd an account-isolation assertion to the search path.
This only proves the happy-path search result. It still won't catch the privacy regression this feature is most sensitive to: returning messages from a different WhatsApp account when multiple accounts are ingested into the same store. Ingesting a second account and asserting that
search_messagesstays scoped would make this test much more valuable.🤖 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 `@tests/json_rpc_e2e.rs` around lines 3565 - 3590, Add a second WhatsApp account ingestion via post_json_rpc (use the same ingestion path your tests use) before calling openhuman.whatsapp_data_search_messages, then run the search and assert results are scoped to the expected account: use assert_no_jsonrpc_error on the search response, inspect the search result array (search_arr) and verify each message's account identifier (e.g., an account_id field) matches the first account and that no messages from the second account appear; also assert the length is still 1 and the message body contains "umbrella" as before to prove account isolation for search_messages.
🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs`:
- Around line 866-904: The msgs_for_ingest mapping currently only recognizes
full-scan keys and drops or zeroes fast-path DOM rows; update the filter_map
that builds msgs_for_ingest so it accepts fast-path fields: treat chat_id =
m.get("chatId").or_else(|| m.get("chat")).or_else(||
m.get("chat_id")).and_then(|v| v.as_str()).map(|s| s.to_string()), and for
sender/sender_jid fall back to m.get("author").cloned() or m.get("participant")
when from/fromName are missing; for timestamp use
m.get("timestamp").and_then(|v| v.as_i64()).or_else(||
m.get("preTimestamp").and_then(|v| v.as_i64()).or_else(|| parse string fields if
present)) so we don't default to 0; keep the existing msg_id logic but also
consider m.get("dataId")/m.get("id") fallbacks already present. Apply these
fallbacks inside the same filter_map that constructs the json! object
(msgs_for_ingest) so fast DOM ticks produce messages with usable metadata.
- Around line 1076-1091: The conversion for chats_param currently only reads
v.get("name"), which drops cases where the scanner passed the chat value itself
as a Value::String; update the mapping closure used to build chats_param so it
first checks if v.is_string() (or v.as_str()) and uses that string as the
"name", otherwise fall back to v.get("name").and_then(|n| n.as_str()), then wrap
that string as Value::String or Value::Null as currently done; adjust the
mapping for (jid, v) where chats_param is constructed so existing string-valued
chat names are preserved.
In `@src/openhuman/whatsapp_data/ops.rs`:
- Around line 24-29: The debug logs in ops.rs leak PII by printing
req.account_id, req.chats contents and req.messages/search text; change those
log calls (including the one at the top using log::debug! and the similar calls
around lines 58-89) to emit non-sensitive info only: use counts
(req.chats.len(), req.messages.len()), booleans (has_search =
!req.search_text.is_empty()), and redacted identifiers (e.g., truncated or
hashed account_id and chat IDs rather than full values). Keep the same log
prefixes/branches and call sites (the log::debug! invocations that reference
req.account_id, req.chats, req.messages, search_text) but replace the
interpolated raw fields with redacted_account_id, chat_count, message_count, and
has_search flags.
In `@src/openhuman/whatsapp_data/rpc.rs`:
- Around line 36-45: The RPC logging in the ingest path is emitting raw
identifiers and message content; change the log::debug and log::warn calls in
this function (the block around require_store(), ops::ingest(&store, req), and
the existing log statements) to avoid PII by logging only stable prefixes or
presence/count flags — e.g., replace full account_id/chat/message strings with
either a fixed stable prefix (first N chars + "...") or boolean/presence flags
and numeric counts, and ensure the error log for the ingest failure logs the
sanitized identifier/prefix plus the error text rather than raw IDs or message
text.
In `@src/openhuman/whatsapp_data/schemas.rs`:
- Around line 20-47: Remove the ingest controller from the agent-facing
registries: in all_controller_schemas() drop the schemas("ingest") entry and in
all_registered_controllers() remove the RegisteredController block that uses
schema: schemas("ingest") and handler: handle_ingest so only read-only
controllers remain; keep the list_chats, list_messages, and search_messages
entries intact (also apply the same removal in the analogous block around the
52-79 range if duplicated).
In `@src/openhuman/whatsapp_data/store.rs`:
- Around line 118-122: Replace direct printing of raw WhatsApp identifiers and
query text in the log::debug statements in src/openhuman/whatsapp_data/store.rs
with redacted values: keep stable prefixes if helpful, but log counts, boolean
presence flags, or a short redacted hash (e.g., first 6 chars of a hex HMAC)
instead of full account_id, chat_id or raw search text; update every debug call
referenced (the upsert logging around the upsert method that currently uses
variables like account_id and count, plus the other debug blocks around lines
where chat_id, query_text, and raw identifiers are logged) to compute and log
only non-sensitive metadata (counts, presence, and truncated/redacted
identifiers) and ensure no raw PII or full query strings are emitted.
- Around line 207-223: prune_old_messages currently deletes rows from
wa_messages but leaves wa_chats.message_count and wa_chats.last_message_ts
stale; modify prune_old_messages to run inside a transaction (use the same
open_conn() connection), first collect affected chat_ids (e.g. SELECT DISTINCT
chat_id FROM wa_messages WHERE timestamp > 0 AND timestamp < ?1), perform the
DELETE, then for each affected chat_id update wa_chats by recomputing aggregates
(message_count = COUNT(*) FROM wa_messages WHERE chat_id = ? and timestamp >= 0,
last_message_ts = MAX(timestamp) FROM wa_messages WHERE chat_id = ? or set to
0/null if no rows) so list_chats returns correct counts and ordering; ensure all
DB ops use the same transaction and propagate errors via the Result return from
prune_old_messages.
- Around line 183-193: The UPDATE can set last_message_ts to NULL when a chat
has no messages, violating wa_chats.last_message_ts NOT NULL; modify the SQL in
store.rs (the conn.execute(...) that updates wa_chats) to wrap the
MAX(timestamp) subquery with COALESCE, e.g. last_message_ts = COALESCE((SELECT
MAX(timestamp) FROM wa_messages WHERE wa_messages.account_id =
wa_chats.account_id AND wa_messages.chat_id = wa_chats.chat_id),
last_message_ts) so existing last_message_ts is preserved for empty chats (leave
params and the rest of the statement unchanged).
- Around line 138-140: The code currently drops any message where body is empty
(the check using let body = m.body.as_deref().unwrap_or(""); if body.is_empty()
{ continue; }), which loses non-text events and biases
message_count/last_message_ts; instead remove the continue and persist messages
with empty body by recording an explicit content type/placeholder and metadata:
detect and set a content_type (e.g., "sticker"/"image"/"system") or store an
empty-body flag from the message struct (inspect m.* fields) and ensure the
insert/update paths that update message_count and last_message_ts still run for
these messages (reference m, body, message_count, last_message_ts) so all events
update chat state even when body is empty.
In `@tests/json_rpc_e2e.rs`:
- Around line 3408-3410: The test directly calls
whatsapp_data::global::init(...) which relies on a process-wide OnceLock
singleton and can silently reuse a store from other tests; instead, change the
test to avoid calling the global init directly and either (a) start the system
via the same startup path that owns initialization (e.g., call
run_core_http_server or the server bootstrap helper used elsewhere so the global
is initialized consistently), or (b) add and call a test-only reset/inject API
on whatsapp_data::global to reinitialize or inject a per-test store before
exercising the router; update the test to use one of these approaches so
workspace isolation is preserved and subsequent reads/writes do not target
another test's workspace.
---
Nitpick comments:
In `@tests/json_rpc_e2e.rs`:
- Around line 3565-3590: Add a second WhatsApp account ingestion via
post_json_rpc (use the same ingestion path your tests use) before calling
openhuman.whatsapp_data_search_messages, then run the search and assert results
are scoped to the expected account: use assert_no_jsonrpc_error on the search
response, inspect the search result array (search_arr) and verify each message's
account identifier (e.g., an account_id field) matches the first account and
that no messages from the second account appear; also assert the length is still
1 and the message body contains "umbrella" as before to prove account isolation
for search_messages.
🪄 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: b5deba89-dc20-4199-b422-7359c50d51c7
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
app/src-tauri/src/whatsapp_scanner/mod.rssrc/core/all.rssrc/core/jsonrpc.rssrc/openhuman/about_app/catalog.rssrc/openhuman/mod.rssrc/openhuman/whatsapp_data/global.rssrc/openhuman/whatsapp_data/mod.rssrc/openhuman/whatsapp_data/ops.rssrc/openhuman/whatsapp_data/rpc.rssrc/openhuman/whatsapp_data/schemas.rssrc/openhuman/whatsapp_data/store.rssrc/openhuman/whatsapp_data/types.rstests/json_rpc_e2e.rs
…ternal ingest registry, scanner fast-path, account isolation test - ops.rs / rpc.rs: replace raw account_id/chat_id/search-query log params with has_account, has_chat booleans and counts (addresses @coderabbitai on ops.rs:24-89, rpc.rs:36-115) - store.rs: redact identifiers from upsert/prune/search debug logs; persist non-text messages (stickers/images) instead of dropping empty-body rows so message_count and last_message_ts stay accurate; COALESCE guard on last_message_ts UPDATE prevents NOT NULL violation for empty chats; refresh wa_chats aggregates after prune so list_chats ordering is correct immediately (addresses @coderabbitai on store.rs:122, 140, 193, 223) - schemas.rs / all.rs / mod.rs: move whatsapp_data_ingest to an internal-only controller registry (INTERNAL_REGISTRY) that is routable over JSON-RPC for the Tauri scanner but excluded from agent tool listings, schema discovery, and CLI help — only list_chats/list_messages/search_messages are agent-facing (addresses @coderabbitai on schemas.rs:47) - whatsapp_scanner/mod.rs: fast-path DOM rows now resolve chat_id from chatId/ chat/chat_id fallbacks, sender from fromName/from/author, timestamp from timestamp/preTimestamp; string-valued chat names from contact cache are preserved in chats_param conversion (addresses @coderabbitai on mod.rs:904, 1091) - tests/json_rpc_e2e.rs: add step 5 — ingest a second account and assert that account-scoped search_messages returns only messages from the requested account (addresses @coderabbitai nitpick on json_rpc_e2e.rs:3565-3590)
M3gA-Mind
commented
May 6, 2026
|
|
||
| // Init the whatsapp_data global before the router handles any requests. | ||
| openhuman_core::openhuman::whatsapp_data::global::init(openhuman_home.clone()) | ||
| .expect("whatsapp_data global init"); |
Collaborator
Author
There was a problem hiding this comment.
Deferred (architectural note): @coderabbitai flagged that the OnceLock singleton in whatsapp_data::global::init means the first call in the test binary wins and subsequent calls are no-ops, which can silently reuse a store from another test.
In practice this is safe today because all e2e tests serialise through json_rpc_e2e_env_lock() (a global Mutex) and no other e2e test initialises the whatsapp_data global — so this test always wins the race. The manual init() at line 3409 fires before build_core_http_router, ensuring the isolated temp workspace is used.
The fragility becomes real if a new test is added that touches whatsapp_data before this one. Suggested remediation: add a #[cfg(test)] pub fn reset_for_test() on global.rs using an unsafe pointer swap on the OnceLock, or replace it with RwLock<Option<...>> that tests can clear. Confirm desired approach before I change the global lifetime contract.
Contributor
There was a problem hiding this comment.
@M3gA-Mind — good summary of the trade-offs. Here's my recommendation:
Go with RwLock<Option<WhatsAppDataStoreRef>> — do not attempt the OnceLock pointer-swap.
std::sync::OnceLock exposes no stable reset() / take() API on a shared reference. Any "unsafe swap" would require reaching into its internal UnsafeCell via pointer-casting or transmutation — implementation-defined behaviour that can silently break across std versions. It's not worth it.
The RwLock<Option<T>> replacement is straightforward:
// global.rs
use std::sync::{Arc, RwLock};
use std::path::PathBuf;
use crate::openhuman::whatsapp_data::store::WhatsAppDataStore;
pub type WhatsAppDataStoreRef = Arc<WhatsAppDataStore>;
static GLOBAL_STORE: RwLock<Option<WhatsAppDataStoreRef>> =
RwLock::new(None);
pub fn init(workspace_dir: PathBuf) -> Result<WhatsAppDataStoreRef, String> {
// Fast-path: already initialised.
if let Some(existing) = GLOBAL_STORE
.read()
.map_err(|e| format!("global store lock poisoned: {e}"))?
.as_ref()
.cloned()
{
return Ok(existing);
}
let store = Arc::new(
WhatsAppDataStore::new(&workspace_dir)
.map_err(|e| format!("whatsapp_data store init failed: {e}"))?,
);
*GLOBAL_STORE
.write()
.map_err(|e| format!("global store write lock poisoned: {e}"))? = Some(Arc::clone(&store));
tracing::info!("[whatsapp_data] global store initialised at {:?}", workspace_dir);
Ok(store)
}
pub fn store() -> Result<WhatsAppDataStoreRef, String> {
GLOBAL_STORE
.read()
.map_err(|e| format!("global store lock poisoned: {e}"))?
.as_ref()
.cloned()
.ok_or_else(|| "whatsapp_data store not initialised — call init() at startup".into())
}
pub fn store_if_ready() -> Option<WhatsAppDataStoreRef> {
GLOBAL_STORE
.read()
.ok()?
.as_ref()
.cloned()
}
/// Resets the singleton for test isolation. Only compiled in `#[cfg(test)]`.
#[cfg(test)]
pub fn reset_for_test() {
*GLOBAL_STORE.write().expect("reset_for_test: lock poisoned") = None;
}Then in whatsapp_data_ingest_and_query_e2e (and any future whatsapp test), add a guard:
// Ensure a clean slate even if another test ran init() earlier in this binary.
openhuman_core::openhuman::whatsapp_data::global::reset_for_test();
openhuman_core::openhuman::whatsapp_data::global::init(openhuman_home.clone())
.expect("whatsapp_data global init");Why this beats the alternatives:
OnceLock + unsafe swap |
RwLock<Option<T>> |
|
|---|---|---|
| Stable API | ❌ (internal layout) | ✅ |
| Safe Rust | ❌ | ✅ |
| Test reset | Fragile | Clean reset_for_test() |
| Production perf | One-time write | One read() per call — negligible at RPC frequency |
Supports idempotent init() |
Only with atomic guard workaround | Native double-check |
The RwLock read in the production hot path is a shared lock held for microseconds — at RPC handler frequency this is unmeasurable overhead.
🧠 Learnings used
Learnt from: sanil-23
Repo: tinyhumansai/openhuman PR: 416
File: src/openhuman/memory/relex.rs:441-464
Timestamp: 2026-04-07T15:49:51.275Z
Learning: When integrating the `ort` Rust crate v2.x with the `load-dynamic` feature enabled, do NOT also require/enable individual provider EP Cargo features like `directml`, `coreml`, or `cuda`. In `ort` v2.x, EP registration for providers (e.g., DirectML, CoreML, CUDA, etc.) is already compiled in under source-level `#[cfg(any(feature = "load-dynamic", feature = "<provider>"))]` guards, such as in `ep/directml.rs`. Adding provider feature flags alongside `load-dynamic` can pull in static-linking dependencies that conflict with the dynamic-loading approach. Provider availability should be treated as runtime-determined by what the loaded `onnxruntime` library (`onnxruntime.dll`/`libonnxruntime.so`/`libonnxruntime.dylib`) actually supports.
Learnt from: oxoxDev
Repo: tinyhumansai/openhuman PR: 571
File: src/openhuman/local_ai/service/whisper_engine.rs:69-80
Timestamp: 2026-04-14T19:59:04.826Z
Learning: When reviewing Rust code in this repo that uses the upstream `whisper-rs` crate (v0.16.0), do not report `WhisperContextParameters::use_gpu(...)` or `WhisperContextParameters::flash_attn(...)` as missing/invalid APIs. These builder-style methods exist upstream and return `&mut Self`; they are not limited to `WhisperVadContextParams`.
Learnt from: senamakel
Repo: tinyhumansai/openhuman PR: 1173
File: tests/agent_memory_loader_public.rs:88-88
Timestamp: 2026-05-04T06:50:47.877Z
Learning: In this repository, the general camelCase naming guideline should not be applied to Rust source files. For all .rs files, Rust function (and related) names should use snake_case, and snake_case Rust function names should not be flagged—even for async test functions annotated with attributes like #[tokio::test]. This is consistent with Rust’s non_snake_case lint behavior.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src-tauri/src/whatsapp_scanner/mod.rs (1)
908-912: ⚖️ Poor tradeoffString-valued
preTimestampwon't parse to i64, resulting in timestamp=0.The DOM scraper produces
preTimestampas a string like"4:53 AM, 7/5/2025", butv.as_i64()returnsNonefor strings. This means DOM-only rows will storetimestamp = 0, degrading the accuracy oflast_message_tsfor chats that are only visible in the fast-tick DOM scan.Consider reusing the existing
parse_pre_timestamp_ymdhelper (lines 689-704) to extract at least the date, or parse the time component to derive a more accurate timestamp.💡 Potential fix sketch
let timestamp = m .get("timestamp") .and_then(|v| v.as_i64()) .or_else(|| m.get("preTimestamp").and_then(|v| v.as_i64())) + .or_else(|| { + m.get("preTimestamp") + .and_then(|v| v.as_str()) + .and_then(|s| parse_pre_timestamp_to_epoch(s)) + }) .unwrap_or(0);You'd need a helper that converts the WA date string to an approximate Unix timestamp (e.g., midnight UTC of that day).
🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs` around lines 908 - 912, The timestamp extraction currently only calls as_i64() on "timestamp" and "preTimestamp", so string-valued preTimestamp (e.g., "4:53 AM, 7/5/2025") becomes None and yields 0; update the logic in the block around the get("timestamp") chain to check if m.get("preTimestamp") is a string and, if so, call the existing parse_pre_timestamp_ymd helper to produce an approximate Unix timestamp (e.g., midday/midnight UTC or parsed time) before falling back to 0; ensure you still prefer numeric "timestamp" when present and only use parse_pre_timestamp_ymd on the string case for the key sequence used in this module.src/openhuman/whatsapp_data/store.rs (1)
172-177: 💤 Low valueError context leaks chat_id and message_id in error messages.
While logs are redacted, the
with_contexterror message at lines 172-177 includes rawchat_idandmessage_id. These can propagate to user-facing error surfaces or crash reports.🛡️ Suggested redaction
.with_context(|| { - format!( - "upsert wa_message chat={} msg={}", - m.chat_id, m.message_id - ) + "upsert wa_message failed (identifiers redacted)".to_string() })?;🤖 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 172 - 177, The error context currently built in the .with_context(...) call (the upsert/wa_message path using m.chat_id and m.message_id) leaks raw identifiers; change the .with_context message to avoid including raw m.chat_id and m.message_id (use a constant "redacted" or an anonymized/hashing function for those fields) so the closure passed to with_context does not embed PII—update the call site where the upsert for wa_message is performed (the .with_context(...) closure referencing m.chat_id and m.message_id) to emit non-identifying placeholders or hashed IDs instead.
🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs`:
- Around line 908-912: The timestamp extraction currently only calls as_i64() on
"timestamp" and "preTimestamp", so string-valued preTimestamp (e.g., "4:53 AM,
7/5/2025") becomes None and yields 0; update the logic in the block around the
get("timestamp") chain to check if m.get("preTimestamp") is a string and, if so,
call the existing parse_pre_timestamp_ymd helper to produce an approximate Unix
timestamp (e.g., midday/midnight UTC or parsed time) before falling back to 0;
ensure you still prefer numeric "timestamp" when present and only use
parse_pre_timestamp_ymd on the string case for the key sequence used in this
module.
In `@src/openhuman/whatsapp_data/store.rs`:
- Around line 172-177: The error context currently built in the
.with_context(...) call (the upsert/wa_message path using m.chat_id and
m.message_id) leaks raw identifiers; change the .with_context message to avoid
including raw m.chat_id and m.message_id (use a constant "redacted" or an
anonymized/hashing function for those fields) so the closure passed to
with_context does not embed PII—update the call site where the upsert for
wa_message is performed (the .with_context(...) closure referencing m.chat_id
and m.message_id) to emit non-identifying placeholders or hashed IDs instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c9f52b3-ecbd-4253-8db1-8f1e5d5836f0
📒 Files selected for processing (8)
app/src-tauri/src/whatsapp_scanner/mod.rssrc/core/all.rssrc/openhuman/whatsapp_data/mod.rssrc/openhuman/whatsapp_data/ops.rssrc/openhuman/whatsapp_data/rpc.rssrc/openhuman/whatsapp_data/schemas.rssrc/openhuman/whatsapp_data/store.rstests/json_rpc_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/json_rpc_e2e.rs
- src/openhuman/whatsapp_data/mod.rs
- src/openhuman/whatsapp_data/ops.rs
…t E2E (tinyhumansai#1275) Adds the test coverage that was missing from the memory ingestion pipeline to satisfy issue tinyhumansai#1275's diff-coverage gate. - Extract `merge_dom_into_snapshot` and `build_doc_ingest_params` helpers from `emit_snapshot`/`post_memory_doc_ingest` so they are unit-testable - 27 new unit tests in whatsapp_scanner::tests covering: - `seconds_to_ymd` (3 tests) - `parse_pre_timestamp_ymd` (6 tests — valid, empty, bad format, garbage) - Grouping logic: multi-chat/multi-day → correct group count and message counts - Transcript format: sender names, bodies, timestamp order - `build_doc_ingest_params` payload shape: namespace, key, source_type, content - DOM-IDB merge: body patch, active_chat_jid backfill, empty-body skip, dedup guard, empty DOM passthrough - 1 new JSON-RPC E2E test `whatsapp_memory_doc_ingest_e2e`: posts a WhatsApp-shaped payload to `openhuman.memory_doc_ingest` and verifies the doc lands via `openhuman.memory_doc_list` - Warn on ambiguous active-chat-jid resolution (previously silent) - Retry-once (500ms) on connection error in `post_memory_doc_ingest` to handle startup race where core sidecar is not yet listening Closes tinyhumansai#1275
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
app/src-tauri/src/whatsapp_scanner/mod.rs (1)
819-823:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDOM rows still collapse to
timestamp = 0on the normalpreTimestamppath.
preTimestampis typically the WhatsApp string form, but this branch only accepts numeric values. Those rows get stored with zero timestamps, which will skew ordering and any "last message" metadata derived from the structured store.🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs` around lines 819 - 823, The timestamp extraction falls back to preTimestamp but only accepts numeric JSON values, causing string-form WhatsApp timestamps to become 0; update the logic around m.get("timestamp") / m.get("preTimestamp") (the block that sets let timestamp = ...) to also check preTimestamp.as_str() and parse it to i64 (e.g., parse::<i64>().ok()) before defaulting to 0 so string timestamps are converted correctly and ordering/last-message metadata remain accurate.tests/json_rpc_e2e.rs (1)
3408-3410:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThis still makes the E2E depend on whichever test initialized the global store first.
The PR wiring now initializes
whatsapp_data::globalduring server startup, and this test file bootsbuild_core_http_router(false)many times before reaching this case. Becauseglobal::init(...)is process-wide and idempotent, this call can silently reuse an earlier test's workspace instead oftmp.path(), so the ingest/query assertions are no longer isolated to this test.🤖 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 `@tests/json_rpc_e2e.rs` around lines 3408 - 3410, This test calls whatsapp_data::global::init(...) directly which can reuse a previously-initialized, process-wide workspace and leak state between runs; remove the explicit global::init call here and let the server startup path (invoked by build_core_http_router(false)) perform initialization with the test's tmp.path(), or if you must initialize in-test ensure you initialize before calling build_core_http_router(false) and use a fresh temp dir per test (not a reused global) so the ingest/query assertions are isolated; in short, delete the whatsapp_data::global::init(openhuman_home.clone()) line (or move it to before router creation and bind it to tmp.path()) so the test no longer relies on or contaminates the process-global store.
🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs`:
- Around line 532-539: The warning logs in the ambiguous-active-chat branches
are emitting raw PII (the active chat label/account JID via the variable name);
replace those direct usages in the log::warn! calls inside the (e, c, s) match
arm and the other affected blocks (around the other occurrences) with a redacted
or stable fingerprint instead of the raw string — e.g., call a helper like
redact_identifier(name) or fingerprint_identifier(name) and log that result so
the messages no longer contain full JIDs/labels but a truncated/hash correlation
id; update all matching log::warn!/log::info! invocations in mod.rs (including
the other mentioned locations) to use the redacted/fingerprinted value.
- Around line 812-816: The code is incorrectly populating sender_jid from the
DOM-rendered "author" field; change the sender_jid assignment so it only uses
id-bearing keys (e.g., m.get("from") and m.get("participant")) and remove the
or_else that falls back to m.get("author"); keep m.get("author") only for the
separate 'sender' field so display names remain but do not become sender_jid
values (update the logic around sender_jid and ensure any downstream use expects
JIDs only).
In `@tests/json_rpc_e2e.rs`:
- Around line 3739-3751: The current closure used in docs_arr.iter().find uses
key_match || ns_match which returns true if any doc matches either field; change
the lookup to require both fields on the same document by using key_match &&
ns_match in the closure (i.e., in the docs_arr.iter().find closure that computes
key_match and ns_match) or alternatively first filter docs_arr by namespace and
then assert the key value on the found document so the test verifies the
ingested key for the same document.
---
Duplicate comments:
In `@app/src-tauri/src/whatsapp_scanner/mod.rs`:
- Around line 819-823: The timestamp extraction falls back to preTimestamp but
only accepts numeric JSON values, causing string-form WhatsApp timestamps to
become 0; update the logic around m.get("timestamp") / m.get("preTimestamp")
(the block that sets let timestamp = ...) to also check preTimestamp.as_str()
and parse it to i64 (e.g., parse::<i64>().ok()) before defaulting to 0 so string
timestamps are converted correctly and ordering/last-message metadata remain
accurate.
In `@tests/json_rpc_e2e.rs`:
- Around line 3408-3410: This test calls whatsapp_data::global::init(...)
directly which can reuse a previously-initialized, process-wide workspace and
leak state between runs; remove the explicit global::init call here and let the
server startup path (invoked by build_core_http_router(false)) perform
initialization with the test's tmp.path(), or if you must initialize in-test
ensure you initialize before calling build_core_http_router(false) and use a
fresh temp dir per test (not a reused global) so the ingest/query assertions are
isolated; in short, delete the
whatsapp_data::global::init(openhuman_home.clone()) line (or move it to before
router creation and bind it to tmp.path()) so the test no longer relies on or
contaminates the process-global store.
🪄 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: 50bf0610-fc05-448c-b250-821336aa3b14
📒 Files selected for processing (2)
app/src-tauri/src/whatsapp_scanner/mod.rstests/json_rpc_e2e.rs
Comment on lines
+532
to
+539
| (e, c, s) => { | ||
| let count = e + c + s; | ||
| if count > 1 { | ||
| log::warn!( | ||
| "[whatsapp_scanner] ambiguous active-chat resolution: {} candidates for '{}' — skipping backfill", | ||
| count, | ||
| name | ||
| ); |
Contributor
There was a problem hiding this comment.
Redact the raw chat/account identifiers from these new log lines.
These branches log the active chat label and full account JID directly. The repo Rust logging rule forbids full PII in logs, so please switch these to redacted/truncated values or a stable hash/correlation id instead. As per coding guidelines, **/*.rs: "never log secrets or full PII".
Also applies to: 1058-1064, 1129-1134
🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs` around lines 532 - 539, The
warning logs in the ambiguous-active-chat branches are emitting raw PII (the
active chat label/account JID via the variable name); replace those direct
usages in the log::warn! calls inside the (e, c, s) match arm and the other
affected blocks (around the other occurrences) with a redacted or stable
fingerprint instead of the raw string — e.g., call a helper like
redact_identifier(name) or fingerprint_identifier(name) and log that result so
the messages no longer contain full JIDs/labels but a truncated/hash correlation
id; update all matching log::warn!/log::info! invocations in mod.rs (including
the other mentioned locations) to use the redacted/fingerprinted value.
Comment on lines
+812
to
+816
| let sender_jid = m | ||
| .get("from") | ||
| .cloned() | ||
| .or_else(|| m.get("author").cloned()) | ||
| .or_else(|| m.get("participant").cloned()); |
Contributor
There was a problem hiding this comment.
Don't populate sender_jid from author.
author is the rendered display label from the DOM, not a WhatsApp JID. For DOM-only rows this stores values like "Alice" into the identifier field, so the structured RPC can return invalid sender metadata. Keep author for sender, and only fill sender_jid from actual id-bearing fields such as from / participant.
Proposed fix
- let sender_jid = m
- .get("from")
- .cloned()
- .or_else(|| m.get("author").cloned())
- .or_else(|| m.get("participant").cloned());
+ let sender_jid = m
+ .get("from")
+ .cloned()
+ .or_else(|| m.get("participant").cloned());🤖 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 `@app/src-tauri/src/whatsapp_scanner/mod.rs` around lines 812 - 816, The code
is incorrectly populating sender_jid from the DOM-rendered "author" field;
change the sender_jid assignment so it only uses id-bearing keys (e.g.,
m.get("from") and m.get("participant")) and remove the or_else that falls back
to m.get("author"); keep m.get("author") only for the separate 'sender' field so
display names remain but do not become sender_jid values (update the logic
around sender_jid and ensure any downstream use expects JIDs only).
Comment on lines
+3739
to
+3751
| let found = docs_arr.iter().find(|doc| { | ||
| let key_match = doc | ||
| .get("key") | ||
| .and_then(Value::as_str) | ||
| .map(|k| k == "alice@c.us:2026-05-07") | ||
| .unwrap_or(false); | ||
| let ns_match = doc | ||
| .get("namespace") | ||
| .and_then(Value::as_str) | ||
| .map(|n| n == "whatsapp-web:test-acct@c.us") | ||
| .unwrap_or(false); | ||
| key_match || ns_match | ||
| }); |
Contributor
There was a problem hiding this comment.
Tighten the final lookup so the test actually verifies the ingested key.
key_match || ns_match passes as soon as any document exists in the namespace, so a regression that writes the wrong key would still go green here. Match both fields on the same document, or assert the key directly after the namespace filter.
🤖 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 `@tests/json_rpc_e2e.rs` around lines 3739 - 3751, The current closure used in
docs_arr.iter().find uses key_match || ns_match which returns true if any doc
matches either field; change the lookup to require both fields on the same
document by using key_match && ns_match in the closure (i.e., in the
docs_arr.iter().find closure that computes key_match and ns_match) or
alternatively first filter docs_arr by namespace and then assert the key value
on the found document so the test verifies the ingested key for the same
document.
This was referenced May 7, 2026
12 tasks done
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
…API (tinyhumansai#1308) Co-authored-by: Cursor <cursoragent@cursor.com>
senamakel
pushed a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
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
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_dataRust domain that persists and serves structured WhatsApp conversations and messages, making them queryable by the agent through four new JSON-RPC methodswhatsapp_scannerto dual-write structured data alongside its currentmemory_doc_ingestpath — no JS injection, zero changes to existing scraping behaviourmerge_dom_into_snapshotandbuild_doc_ingest_paramshelpers for testabilityWhat changed
New:
src/openhuman/whatsapp_data/types.rs—WhatsAppChat,WhatsAppMessage, and all request/response structs (chat_id/JID, display_name, sender, timestamp, body, source, from_me, etc.)store.rs— SQLite-backed persistence inwhatsapp_data.db; WAL mode; upsert chats + messages; parameterised queries throughout; 90-day auto-prune; persists non-text messages (stickers, images) somessage_count/last_message_tsstay accurateops.rs— business logic:ingest,list_chats,list_messages,search_messages; PII-safe logging (no raw IDs/queries in logs)rpc.rs— RPC handlers usingRpcOutcome<T>; returns actionable error when store not initialisedschemas.rs/all.rs—whatsapp_data_ingestis internal-only (INTERNAL_REGISTRY, excluded from agent tool listings and schema discovery);list_chats,list_messages,search_messagesare agent-facingglobal.rs— idempotentOnceLocksingleton; initialised at core startupModified:
app/src-tauri/src/whatsapp_scanner/mod.rspost_whatsapp_data_ingest, fire-and-forget); existingpost_memory_doc_ingestpath untouchedchat_id/sender/timestampnow resolved from multiple field name fallbacksmerge_dom_into_snapshotandbuild_doc_ingest_paramshelpers for testabilitypost_memory_doc_ingestretries once (500ms) on connection error to handle startup raceTests
whatsapp_scanner::tests:seconds_to_ymd,parse_pre_timestamp_ymd, grouping, transcript format, payload shape, DOM-IDB merge (5 scenarios)whatsapp_data_ingest_and_query_e2e(structured store: ingest/list/search + account isolation) andwhatsapp_memory_doc_ingest_e2e(memory pipeline:memory_doc_ingest→memory_doc_list)Other
src/core/all.rs— controllers, schemas,namespace_descriptionregisteredsrc/core/jsonrpc.rs— global store initialised at startupsrc/openhuman/about_app/catalog.rs—channels.whatsapp_read_messagescapability added (LOCAL_RAWprivacy — data never leaves device)Test plan
NODE_ENV=test pnpm --filter openhuman-app compilepassesNODE_ENV=test pnpm --filter openhuman-app format:checkpassesNODE_ENV=test pnpm --filter openhuman-app lintpasses (0 errors)pnpm test:coveragepassescargo fmt --all -- --check+cargo clippy -p openhumanpasscargo test -p openhumanpasses — includeswhatsapp_data_ingest_and_query_e2e+whatsapp_memory_doc_ingest_e2ecargo test --manifest-path app/src-tauri/Cargo.toml whatsapp_scanner— 43 tests passpnpm buildpassesbash scripts/install.sh --dry-run --verbosepassesnode scripts/check-pr-checklist.mjs+check-coverage-matrix.mjspassPrivacy
All WhatsApp data stays in
whatsapp_data.dbinside the user's workspace directory. No external transmission. PII (account IDs, chat IDs, message bodies) is redacted from all log lines.Closes #1279
Closes #1275
Summary by CodeRabbit
New Features
Search & Retrieval
Tests