feat(groups): resolve LID to phone number in requests list output#273
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 6:35 AM ET / 10:35 UTC. Summary Reproducibility: not applicable. as a feature PR. Current-main source shows the behavior gap because Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the additive JSON field and appended table field if maintainers accept the default table-output compatibility tradeoff, with PR wording aligned to the final column order. Do we have a high-confidence way to reproduce the issue? Not applicable as a feature PR. Current-main source shows the behavior gap because Is this the best way to solve the issue? Yes, mostly. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against be2d22fe9d8c. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
cbefbae to
7e0d7aa
Compare
7e0d7aa to
ddf1af4
Compare
The requests list subcommand previously returned only opaque @lid JIDs, making it impossible to cross-reference results against phone-number-based allowlists without a separate resolution step. ResolveLIDToPN is already part of the WAClient interface; this change wires it into the list output so each entry includes a phone_number field when the LID is present in the local store. - JSON output: new phone_number key (omitted when unresolved via omitempty, keeping the schema backward-compatible) - Table output: adds a phone column between JID and timestamp - Tests: two new registration tests for requests list/approve/reject
ddf1af4 to
5a8bf88
Compare
steipete
left a comment
There was a problem hiding this comment.
Maintainer review: accepting the appended third table field because the existing JID and timestamp field positions are preserved, JSON keeps the existing keys and adds optional phone_number, and the branch is green with focused/full/race/release checks plus clean autoreview.
|
Landed as 00439d4. Design/proof summary:
Local verification on 5a8bf88:
CI on 5a8bf88 passed: test, linux-release-builds, docker. Live proof: the PR body includes contributor-provided redacted real WhatsApp output showing 25 pending group join request entries, 23 with phone_number resolved. I could not safely produce fresh local live output in this checkout because read-only checks showed accounts=[] and the default store authenticated=false, connected=false. |
Summary
groups requests listpreviously returned opaque@lidJIDs; this PR adds aphone_numberfield by calling the already-availableResolveLIDToPNfrom theWAClientinterface.phone_numberkey added, existingJIDandRequestedAtkeyspreserved — backward-compatible.
Why
Callers that maintain a phone-number-based allowlist (e.g. an alumni roster) cannot
correlate pending join requests against the list without a separate LID-to-phone
resolution step. With this change, the resolved number is included directly in the
output when it is available in the local contact store.
Changes
newGroupsRequestsListCmdRunE: resolve each LID viaResolveLIDToPN, emitrequestListEntryslice (preservesJID+RequestedAtkeys, addsphone_number).resolveRequestEntrieshelper extracted for unit testing.TestResolveRequestEntriesResolved— resolved LID emits correctphone_number.TestResolveRequestEntriesUnresolved— unresolved LID emits nophone_numberfield.TestRequestListEntryJSONCompatibility— JSON keysJID,RequestedAt,phone_numberpresent/absent as expected.
Proof — live output (redacted)