fix(voip): honor caller phone jid on incoming calls - #279
Conversation
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/voip/src/call/__tests__/call-manager.test.ts">
<violation number="1" location="packages/voip/src/call/__tests__/call-manager.test.ts:198">
P3: The new regression test only asserts callerPn is preserved in CallInfo metadata; it never verifies the fix's core behavior - that initMedia is called with the caller phone JID. A regression reverting initMedia back to peerJid would still pass. Consider capturing the initMedia peer argument (e.g., spy/stub session initMedia or assert on the derived peer SSRC) so the fix is actually covered.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| '2222222222:0@lid' | ||
| ) | ||
|
|
||
| assert.equal(manager.getCall(callId)?.callerPn, callerPn) |
There was a problem hiding this comment.
P3: The new regression test only asserts callerPn is preserved in CallInfo metadata; it never verifies the fix's core behavior - that initMedia is called with the caller phone JID. A regression reverting initMedia back to peerJid would still pass. Consider capturing the initMedia peer argument (e.g., spy/stub session initMedia or assert on the derived peer SSRC) so the fix is actually covered.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/voip/src/call/__tests__/call-manager.test.ts, line 198:
<comment>The new regression test only asserts callerPn is preserved in CallInfo metadata; it never verifies the fix's core behavior - that initMedia is called with the caller phone JID. A regression reverting initMedia back to peerJid would still pass. Consider capturing the initMedia peer argument (e.g., spy/stub session initMedia or assert on the derived peer SSRC) so the fix is actually covered.</comment>
<file context>
@@ -183,6 +184,20 @@ test('incoming offer with capacity creates a second session', async () => {
+ '2222222222:0@lid'
+ )
+
+ assert.equal(manager.getCall(callId)?.callerPn, callerPn)
+})
+
</file context>
1 participant
Summary
caller_pnfrom incoming call offersCallInfoWhy
The call signaling peer can be a LID while RTP/SSRC derivation needs the phone device JID advertised as
caller_pn. Ignoring it can leave an accepted call without usable inbound audio.Validation
@zapo-js/voiptests: 73/73 passed@zapo-js/voiptypecheck: passed@zapo-js/voipbuild (CJS and ESM): passedA real WhatsApp call is still recommended as integration validation because the unit suite cannot reproduce Meta relay traffic.