fix(call_log): take a synced call's direction from its creator, not the direction fields

src/client/app_state.rs

@@ -3055,8 +3055,15 @@ impl Client {
             return;
         }
 
-        if crate::features::call_log::dispatch_call_log_mutation(&self.core.event_bus, m, full_sync)
-        {
+        // A call's direction is its creator compared against this account; the
+        // predicate is only consulted once the mutation is known to be a call
+        // log, so the other mutation kinds do not pay for the snapshot.
+        if crate::features::call_log::dispatch_call_log_mutation(
+            &self.core.event_bus,
+            m,
+            full_sync,
+            |jid| self.is_own_jid(jid),
+        ) {
             return;
         }
 

src/features/call_log.rs

@@ -6,23 +6,39 @@
 //!
 //! # The index carries what the record can leave out
 //!
-//! The index is `["call_log", callCreatorJid, callId, fromMe]`, not the bare
+//! The index is `["call_log", callCreatorJid, callId, direction]`, not the bare
 //! literal `schemas::CALL_LOG.index_parts` declares. WA Web builds it as
 //! `JSON.stringify([action, ...indexArgs])` (`WAWebSyncdActionUtils.buildIndex`)
-//! and `getCallLogMutation` passes `indexArgs: [d, p, m]` — the call creator, the
-//! call id, and whether this account placed the call.
+//! and `getCallLogMutation` passes `indexArgs: [d, p, m]` — the call creator,
+//! the call id, and a direction flag whose meaning is not agreed on (below).
 //!
 //! That matters because the *record*'s `callCreatorJid` is optional and WA Web
 //! leaves it unset for calls it did not receive one for, while the index's is
 //! filled in either way — `d == null && (d = fromMe ? me : peerJid)`. A consumer
 //! given only the record cannot always say who the call was with.
 //!
-//! # `is_incoming` does not mean what it says
+//! # The direction fields disagree with each other, so neither is read
 //!
-//! WA Web writes the record with `isIncoming: n.fromMe`, so the field carries
-//! "this account placed the call" — the opposite of its name. The index's
-//! `fromMe` is the same value under an honest name, which is why
-//! [`CallLogSync::from_me`] comes from there and is the field to trust.
+//! Two fields claim to carry the call's direction, and which one is honest
+//! depends on who wrote the mutation:
+//!
+//! - WA Web's own writer sets both from its local `fromMe`: `indexArgs: [d, p,
+//!   m]` with `m = n.fromMe ? "1" : "0"`, and `isIncoming: n.fromMe`. Its
+//!   `isIncoming` is therefore inverted against its own name.
+//! - Mutations authored by the phone carry them the other way round, literally
+//!   as `isIncoming` — a call the account placed arrives as `0`/`false`.
+//!
+//! Since app state fans a companion's mutations out to every other device, both
+//! flavors reach this client, and no fixed reading of either field is right for
+//! both.
+//!
+//! WA Web's reader sidesteps the whole thing, and so does this module:
+//! `generateCallLogFromCallSyncRecord` destructures the record without ever
+//! touching `isIncoming`, and takes direction from
+//! `getCallLogTargetDetails`, which is `fromMe: isMeAccount(callCreatorWid)` —
+//! the call creator compared against this account. [`CallLogSync::from_me`] is
+//! derived the same way. That is also why the creator is worth having in the
+//! index: it is what the direction is computed from.
 
 use crate::appstate_sync::Mutation;
 use wacore::appstate::schemas;
@@ -32,10 +48,15 @@ use waproto::whatsapp as wa;
 
 /// Dispatch inbound call-log mutations synced from the primary device.
 /// Returns `true` if handled, `false` if the mutation is not a call log.
+///
+/// `is_own_jid` decides the call's direction and is consulted only once the
+/// mutation is known to be a call log, so the app-state path pays nothing for
+/// it on the mutations it is not.
 pub(crate) fn dispatch_call_log_mutation(
     event_bus: &wacore::types::events::CoreEventBus,
     m: &Mutation,
     full_sync: bool,
+    is_own_jid: impl FnOnce(&Jid) -> bool,
 ) -> bool {
     if m.operation != wa::syncd_mutation::SyncdOperation::Set
         || m.index.first().map(String::as_str) != Some(schemas::CALL_LOG.name)
@@ -53,16 +74,10 @@ pub(crate) fn dispatch_call_log_mutation(
         log::warn!("Skipping call_log mutation: missing call id in index");
         return true;
     };
-    // WA Web writes "1"/"0" (`m = n.fromMe ? "1" : "0"`). Anything else is a
-    // shape we do not know how to read, and guessing would mislabel the call.
-    let from_me = match m.index.get(3).map(String::as_str) {
-        Some("1") => true,
-        Some("0") => false,
-        other => {
-            log::warn!("Skipping call_log mutation: unreadable fromMe {other:?} in index");
-            return true;
-        }
-    };
+    // The creator, not the index's fourth part and not `record.is_incoming`:
+    // those two disagree between writers, and this comparison is what WA Web's
+    // own reader does. See the module docs.
+    let from_me = is_own_jid(&call_creator_jid);
 
     // The mutation's own time, which is metadata rather than the call's: WA Web
     // measures it against the pairing timestamp to drop records that predate the
@@ -141,11 +156,31 @@ mod tests {
         }
     }
 
+    /// This account, as either identity: a creator matching one of these is a
+    /// call we placed. Stands in for `Client::is_own_jid`, which compares a JID
+    /// against the device's own PN and LID.
+    const OWN_PN: &str = "5511888880000@s.whatsapp.net";
+    const OWN_LID: &str = "111122223333444@lid";
+    /// Whoever we were talking to. Not us, under either identity.
+    const PEER: &str = "5511999990000@s.whatsapp.net";
+
+    fn is_own(jid: &Jid) -> bool {
+        matches!(jid.user.as_str(), "5511888880000" | "111122223333444")
+    }
+
     fn dispatch(mutation: &Mutation, full_sync: bool) -> (bool, Vec<Arc<Event>>) {
+        dispatch_as(mutation, full_sync, is_own)
+    }
+
+    fn dispatch_as(
+        mutation: &Mutation,
+        full_sync: bool,
+        is_own_jid: impl FnOnce(&Jid) -> bool,
+    ) -> (bool, Vec<Arc<Event>>) {
         let bus = CoreEventBus::new();
         let recorder = Arc::new(Recorder::default());
         bus.subscribe_handler(recorder.clone()).detach();
-        let handled = dispatch_call_log_mutation(&bus, mutation, full_sync);
+        let handled = dispatch_call_log_mutation(&bus, mutation, full_sync, is_own_jid);
         let events = recorder.events.lock().unwrap().clone();
         (handled, events)
     }
@@ -165,8 +200,12 @@ mod tests {
         }
     }
 
+    /// A call this account placed: the creator is us, which is what the
+    /// direction is read from. The fourth part is the writers' disputed field
+    /// and is deliberately set to the value that would give the wrong answer if
+    /// anything still read it as `fromMe`.
     fn full_index() -> [&'static str; 4] {
-        ["call_log", "5511999990000@s.whatsapp.net", "call-42", "1"]
+        ["call_log", OWN_PN, "call-42", "0"]
     }
 
     #[test]
@@ -206,46 +245,82 @@ mod tests {
         let Event::CallLogSync(update) = events[0].as_ref() else {
             panic!("expected CallLogSync event");
         };
-        assert_eq!(
-            update.call_creator_jid.to_string(),
-            "5511999990000@s.whatsapp.net"
-        );
+        assert_eq!(update.call_creator_jid.to_string(), OWN_PN);
         assert_eq!(update.call_id, "call-42");
-        assert!(
-            update.from_me,
-            "the index says this account placed the call"
-        );
         assert_eq!(update.timestamp.timestamp_millis(), 1_700_000_000_000);
     }
 
-    /// WA Web writes the record's `isIncoming` from its local `fromMe`, so the
-    /// two disagree by name and agree by value. A consumer trusting the field
-    /// name would file every call backwards; `from_me` is the honest one. Both
-    /// directions, so neither a constant nor the record can stand in for it.
+    /// The regression: a call this account placed reads as `from_me`.
+    ///
+    /// Both of the fields that claim to carry direction are set the way the
+    /// phone sends them for an outbound call — index `"0"`, record
+    /// `is_incoming: false` — because that combination is what made this read
+    /// backwards. Direction comes from the creator, so it survives them.
+    #[test]
+    fn a_call_this_account_placed_is_from_me() {
+        for creator in [OWN_PN, OWN_LID] {
+            let record = wa::CallLogRecord {
+                is_incoming: Some(false),
+                ..Default::default()
+            };
+            let index = ["call_log", creator, "call-7", "0"];
+            let (_, events) = dispatch(&call_log_mutation(&index, Some(record)), false);
+
+            let Event::CallLogSync(update) = events[0].as_ref() else {
+                panic!("expected CallLogSync event");
+            };
+            assert!(
+                update.from_me,
+                "{creator} is this account, so it placed the call"
+            );
+        }
+    }
+
+    /// The other direction, and the failure case for the fix: a call somebody
+    /// else placed must not become ours just because the direction fields say
+    /// so. WA Web's writer sets both to `fromMe`, so an inbound call it logged
+    /// carries index `"0"` and `is_incoming: false` — identical to the outbound
+    /// fixture above, and told apart only by the creator.
     #[test]
-    fn from_me_comes_from_the_index_not_the_record() {
-        for (index_from_me, record_is_incoming, expected) in
-            [("0", Some(true), false), ("1", Some(false), true)]
-        {
+    fn a_call_the_peer_placed_is_not_from_me() {
+        for (index_part, record_is_incoming) in [("0", Some(false)), ("1", Some(true))] {
             let record = wa::CallLogRecord {
                 is_incoming: record_is_incoming,
                 ..Default::default()
             };
-            let index = [
-                "call_log",
-                "5511999990000@s.whatsapp.net",
-                "call-7",
-                index_from_me,
-            ];
+            let index = ["call_log", PEER, "call-7", index_part];
             let (_, events) = dispatch(&call_log_mutation(&index, Some(record)), false);
 
             let Event::CallLogSync(update) = events[0].as_ref() else {
                 panic!("expected CallLogSync event");
             };
-            assert_eq!(
-                update.from_me, expected,
-                "the index is authoritative, and it says fromMe={index_from_me}"
+            assert!(
+                !update.from_me,
+                "the creator is the peer, whatever index[3]={index_part} claims"
+            );
+        }
+    }
+
+    /// The fourth index part is no longer read, so a value we cannot parse is
+    /// no longer a reason to drop the call: everything the event carries comes
+    /// from parts we did read.
+    #[test]
+    fn an_unreadable_direction_part_no_longer_drops_the_call() {
+        for index in [
+            ["call_log", OWN_PN, "call-42", "yes"],
+            ["call_log", OWN_PN, "call-42", ""],
+        ] {
+            let (handled, events) = dispatch(
+                &call_log_mutation(&index, Some(wa::CallLogRecord::default())),
+                false,
             );
+
+            assert!(handled);
+            assert_eq!(events.len(), 1, "{index:?} is still a usable call log");
+            let Event::CallLogSync(update) = events[0].as_ref() else {
+                panic!("expected CallLogSync event");
+            };
+            assert!(update.from_me);
         }
     }
 
@@ -259,14 +334,13 @@ mod tests {
 
     /// An index we cannot read is still ours — handing it on would only offer it
     /// to dispatchers keyed on other indexes — but it cannot be turned into an
-    /// event a consumer could trust.
+    /// event a consumer could trust. Only the parts the event is built from:
+    /// without a creator there is no direction either.
     #[test]
     fn an_unreadable_index_is_claimed_without_event() {
         for index in [
             &["call_log"][..],
-            &["call_log", "5511999990000@s.whatsapp.net"][..],
-            &["call_log", "5511999990000@s.whatsapp.net", "call-42"][..],
-            &["call_log", "5511999990000@s.whatsapp.net", "call-42", "yes"][..],
+            &["call_log", PEER][..],
             &["call_log", "not a jid", "call-42", "1"][..],
         ] {
             let (handled, events) = dispatch(

src/send/tctoken_lifecycle.rs

@@ -4,7 +4,13 @@ impl Client {
     /// Whether `jid` is our own account (PN or LID). The privacy-token paths
     /// never attach to or issue for ourselves; a single source of truth keeps
     /// the message and call paths from drifting apart.
-    fn is_own_jid(&self, jid: &Jid) -> bool {
+    ///
+    /// Both identities, because a JID reaches us as a phone number or a LID
+    /// depending on the thread's migration state and only one of the two will
+    /// match. That is also what makes this the right test for a synced call
+    /// log's direction (`features::call_log`), which is its creator compared
+    /// against this account.
+    pub(crate) fn is_own_jid(&self, jid: &Jid) -> bool {
         let snapshot = self.persistence_manager.get_device_snapshot();
         snapshot
             .pn

wacore/src/types/events.rs

@@ -2417,10 +2417,12 @@ pub struct CallLogSync {
     pub call_id: String,
     /// Whether *this account* placed the call.
     ///
-    /// Read this rather than `record.is_incoming`, which despite its name holds
-    /// the same thing rather than its opposite: WA Web writes the record with
-    /// `isIncoming: fromMe`, so a consumer taking the field at its word files
-    /// every call backwards.
+    /// Derived by comparing [`call_creator_jid`](Self::call_creator_jid)
+    /// against this account, which is what WA Web's own reader does. Read this
+    /// rather than `record.is_incoming`: that field means the opposite of its
+    /// name in mutations WA Web wrote and exactly its name in ones the phone
+    /// wrote, so no fixed reading of it is right for both, and a consumer
+    /// taking it at its word files some calls backwards.
     pub from_me: bool,
     /// When the mutation was written, not when the call happened — the call's
     /// own time is `record.start_time`.