fix(call_log): take a synced call's direction from its creator, not the direction fields - #1295
Merged
Conversation
…he direction fields A call placed from the account's own handset arrived as `from_me: false`. Both fields that claim to carry direction are written differently depending on which client authored the mutation. WA Web's writer sets both from its local `fromMe` — `indexArgs: [d, p, m]` with `m = n.fromMe ? "1" : "0"`, and `isIncoming: n.fromMe` — so its `isIncoming` is inverted against its own name. Mutations authored by the phone carry both literally as `isIncoming`, so an outbound call arrives as `0`/`false`. App state fans a companion's mutations out to every other device, so both flavors reach this client and no fixed reading of either field is right for both. WA Web's reader does not read either one. `generateCallLogFromCallSyncRecord` destructures the record without touching `isIncoming`, and takes direction from `getCallLogTargetDetails`, which returns `fromMe: isMeAccount( callCreatorWid)` — the call creator compared against this account. Do the same, reusing `Client::is_own_jid`, which already compares a JID against both of the device's identities and whose doc already claimed to be the single source of truth for the message and call paths. The predicate is passed as `FnOnce` and consulted only after the mutation is claimed, so the other app-state mutation kinds do not pay for the device snapshot. The index's fourth part is no longer parsed at all, which also means a value we cannot read there no longer drops an otherwise usable call log.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughCall-log direction now comes from creator JID ownership. Dispatch receives the client ownership predicate, supports PN/LID matching, and accepts malformed fourth index values without rejecting otherwise valid mutations. Profile-picture handling, documentation, and tests use the updated ownership semantics. ChangesCall direction handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AppState
participant CallLog
participant EventBus
Client->>AppState: Dispatch call-log mutation
AppState->>CallLog: Pass is_own_jid predicate
CallLog->>Client: Check creator JID ownership
CallLog->>EventBus: Emit classified call event
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| src/client/app_state.rs | Supplies the client’s own-identity predicate when dispatching synchronized call-log mutations. |
| src/features/call_log.rs | Replaces direction-field parsing with creator identity comparison and adds focused regression tests. |
| src/features/contacts.rs | Reuses the shared own-identity helper when deciding whether to request a trusted-contact token. |
| src/send/tctoken_lifecycle.rs | Centralizes namespace-aware PN/LID self-detection and verifies device and cross-namespace behavior. |
| wacore/src/types/events.rs | Updates CallLogSync documentation to describe creator-derived direction semantics. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Decoded call_log mutation] --> B[Parse creator JID from index]
B --> C[Compare creator with own PN and LID]
C -->|Match| D[from_me = true]
C -->|No match| E[from_me = false]
D --> F[Dispatch CallLogSync]
E --> F
Reviews (2): Last reviewed commit: "fix(send): key self-detection on the add..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b879bcac51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📦 Binary size report
.text per crate
Baseline: |
…gits `is_own_jid` compared with `is_same_user_as`, which ignores the server. A LID is an assigned number in its own namespace, so one can spell a phone number belonging to somebody else — and that peer was reading as us. The repo already had the rule written down twice: `is_same_chat_as` exists to guard "the `is_same_user_as` looseness that ignores server", and `lid_recipient_without_own_lid_is_not_self_dm` records that WA Web's `isMeAccount` keys on `isSameAccountAndAddressingMode`. Self-detection now follows it, which matters more since the synced call log started deriving a call's direction from this: a false match there files a call the peer placed as one we placed, and on the privacy-token path it withholds a token that was due. The comparison moves into `is_own_identity`, a free function over the two identities, so it is unit-testable without a client. `features::contacts` had an inline copy of the old comparison and now calls `is_own_jid` instead, so the three sites share one rule rather than two.
greptile-apps
Bot
dismissed
their stale review
August 12, 2026 14:32
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Merged
1 task
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.
Closes #1294.
Summary
A call placed from the account's own handset arrived as
from_me: false. The report is right that the direction is backwards, and right thatrecord.is_incomingreads literally in the captures — but the cause is not that the crate picked the wrong one of two fields. It is that both fields are written differently depending on which client authored the mutation, so no fixed reading of either is right for all of them.The fix is the route the issue suggests at the end, and the one the official client actually takes: derive the direction from the call creator.
Protocol evidence
Bundle
2.3000.1044659339— the versionwhatspec.lock.jsoncurrently pins, so this is the same build the generated tree describes. The appstate IR only models the literalcall_logindex part, so this came from the raw bundle.The writer,
WAWebCallLogSync.getCallLogMutation:So WA Web sets the index's fourth part and
record.isIncomingfrom the same localfromMe. ItsisIncomingis genuinely inverted against its own name — which is what the crate's old doc block described, and it was accurate about this writer.The reader,
WAWebVoipActionWriteCallLogSync.generateCallLogFromCallSyncRecord, destructures the record ascallCreatorJid, callId, callLinkToken, callResult, groupJid, isCallLink, isVideo, silenceReason, startTime—isIncomingis not among them. It never sees the index either, sinceapplyMutationsis handed only the record. Direction comes fromWAWebVoipBackendCallLogTargetResolver.getCallLogTargetDetails:fromMeisisMeAccount(callCreatorWid)— nothing else. The helperc()resolves the creator through the LID↔PN mapping first.That closes the loop with the report: the two captures have
creator == mewith the direction fields saying0/false, which is contradictory under afromMereading and consistent under a literalisIncomingone. Under the creator rule they are simply outbound, and so is a WA Web-authored record with the same bytes. The fix does not depend on settling which writer is right, which is its main advantage over inverting the parse: inverting would fix phone-authored records and break WA Web-authored ones, and both reach a companion because app state fans a companion's mutations out to every other device.Changes
fix(call_log)—dispatch_call_log_mutationtakes anis_own_jidpredicate and setsfrom_mefrom the call creator. The index's fourth part is no longer parsed. Both doc blocks corrected:record.is_incomingis not a safe fallback under either name.fix(send)— self-detection now keys on the addressing mode.is_own_jidcompared withis_same_user_as, which ignores the server, so a peer LID whose digits spell our phone number read as us. The rule was already written down twice here:is_same_chat_as's test says it exists to guard "theis_same_user_aslooseness that ignores server", andlid_recipient_without_own_lid_is_not_self_dmrecords WA Web'sisMeAccountkeying onisSameAccountAndAddressingMode. Caught in review; I had reused the loose helper without checking which of the two it was.Fixed at the source rather than at the call site, so the privacy-token path gets it too — a false self-match there withholds a token that was due, the same bug wearing different clothes. The comparison moved into
is_own_identity, a free function over the two identities, so it is unit-testable without standing up a client.features::contactshad an inline copy of the old comparison and now callsis_own_jid, so three sites share one rule instead of two.Cost
Nothing added to the hot path. The predicate is
FnOnceand is consulted only after the mutation is claimed as a call log, so the other app-state mutation kinds — the bulk of a full sync — do not pay for the device snapshot; previously this dispatcher did no identity work at all, and it still does none for them. Per call log it is one cachedArc<Device>snapshot and at most two comparisons, replacing a string match on the index part. No new allocations, andis_same_chat_asis no more expensive thanis_same_user_ason a mismatch (it short-circuits on the user first).Binary size on the first commit: total size,
.textand dependency count all unchanged,llvm-lines whatsapp-rust−99.Behavior change worth flagging
An unreadable fourth index part used to drop the whole mutation, on the reasoning that guessing the direction would mislabel the call. Nothing reads that part now, so the reason is gone and dropping a usable call log over it would be a loss. Such a mutation is dispatched, with the direction still derived from the creator.
an_unreadable_direction_part_no_longer_drops_the_callpins it; the other index parts are still required, because the event is built from them.Checked and not changed
fromMe. The creator comparison is right for both.record.is_incomingas a fallback when the creator is missing: it isn't one. The creator is the only field the direction can come from, and the index always carries it — that is what WA Web'sd == null && (d = fromMe ? me : peerJid)fallback exists for. A mutation with no readable creator produces no event, as before.CallLogSyncshape.from_mekeeps its name and meaning; only how it is computed changed, so consumers need no migration — the ones that were filing calls backwards start being right.Validation
Call-log coverage: a call this account placed reads as
from_meunder either of our identities, with both direction fields set the way the phone sends them for an outbound call (the combination that produced the bug); a call the peer placed does not, whatever those fields claim, including the WA Web spelling that is byte-identical to the outbound fixture and told apart only by the creator; an unreadable fourth part no longer drops the call; an index missing a creator still produces no event. Identity coverage:a_peer_addressed_in_the_other_namespace_is_not_uscovers both namespace directions and the case where no LID is known.Semver Checks (informational)is red on this branch and onmainalike — the failures are waproto fields dropped by the whatspec regeneration in #1293, which this PR does not touch. Same failure on #1293 itself, already merged, with everything else green.Workspace clippy and the full matrix left to CI.