Skip to content

feat(groups): resolve LID to phone number in requests list output#273

Merged
steipete merged 2 commits into
openclaw:mainfrom
cielecki:feat/requests-phone-resolution
Jun 10, 2026
Merged

feat(groups): resolve LID to phone number in requests list output#273
steipete merged 2 commits into
openclaw:mainfrom
cielecki:feat/requests-phone-resolution

Conversation

@cielecki

@cielecki cielecki commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • groups requests list previously returned opaque @lid JIDs; this PR adds a
    phone_number field by calling the already-available ResolveLIDToPN from the
    WAClient interface.
  • JSON output: new phone_number key added, existing JID and RequestedAt keys
    preserved — backward-compatible.
  • Table output: appends a phone column after the timestamp.

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

  • newGroupsRequestsListCmd RunE: resolve each LID via ResolveLIDToPN, emit
    requestListEntry slice (preserves JID + RequestedAt keys, adds phone_number).
  • resolveRequestEntries helper extracted for unit testing.
  • TestResolveRequestEntriesResolved — resolved LID emits correct phone_number.
  • TestResolveRequestEntriesUnresolved — unresolved LID emits no phone_number field.
  • TestRequestListEntryJSONCompatibility — JSON keys JID, RequestedAt, phone_number
    present/absent as expected.

Proof — live output (redacted)

{"JID": "XXXXXXXX@lid", "phone_number": "+4175471XXXX", "RequestedAt": "2026-06-03T19:06:23+02:00"}
{"JID": "XXXXXXXX@lid", "phone_number": "+44795764XXXX", "RequestedAt": "2026-06-06T15:55:14+02:00"}
{"JID": "XXXXXXXX@lid", "phone_number": "+4850015XXXX", "RequestedAt": "2026-06-05T18:56:34+02:00"}
... (25 total entries, 23 with phone resolved)

@clawsweeper

clawsweeper Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 6:35 AM ET / 10:35 UTC.

Summary
The PR converts group join request list output to entries with optional resolved phone_number, appends phone as a third tabular field, and adds resolver/output-shape tests.

Reproducibility: not applicable. as a feature PR. Current-main source shows the behavior gap because groups requests list --json writes raw requests without phone_number, and the PR body provides redacted after-fix live output.

Review metrics: 2 noteworthy metrics.

  • Output surface changed: 1 command, 2 output modes. The PR changes both JSON and default tabular output for an existing command, so compatibility is the main review surface.
  • Tests added: 6 added. The diff adds resolver, JSON shape, command registration, and table field order coverage for the changed command.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Update the PR body's table-output sentence so it matches the current diff: phone is appended after the timestamp.

Risk before merge

  • [P1] Appending a third default table field can still affect consumers that compare the full line or require exactly two tab-separated fields, even though the existing JID and timestamp positions are preserved.
  • [P1] I did not run local tests because this was a read-only review; validation is from source, diff, history, and the contributor's redacted live output.

Maintainer options:

  1. Accept the appended table field (recommended)
    Maintainers can accept the compatibility risk now that the existing JID and timestamp field positions are preserved and a table-order test is included.
  2. Limit phone output to JSON or an explicit mode
    If default table output must remain exactly two fields, keep phone_number in JSON or put table phone output behind a deliberate option.

Next step before merge

  • [P2] A maintainer should decide whether expanding default table output to a third field is acceptable; no narrow automated code repair is currently indicated.

Security
Cleared: The diff only changes CLI output shaping and tests; it adds no dependencies, workflows, secrets handling, or artifact execution paths.

Review details

Best 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 groups requests list --json writes raw requests without phone_number, and the PR body provides redacted after-fix live output.

Is this the best way to solve the issue?

Yes, mostly. ResolveLIDToPN is the existing primitive, JSON keys are preserved, and the latest diff appends the table field after timestamp; maintainers still need to accept the default table output expanding from two to three fields.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against be2d22fe9d8c.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted copied live output from groups requests list showing after-fix phone_number values in JSON output.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a useful limited-surface CLI output improvement with a compatibility consideration, not an urgent runtime regression.
  • merge-risk: 🚨 compatibility: The diff expands an existing default tabular command output from two fields to three, which green tests cannot fully settle for downstream parsers.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted copied live output from groups requests list showing after-fix phone_number values in JSON output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted copied live output from groups requests list showing after-fix phone_number values in JSON output.
Evidence reviewed

What I checked:

  • Repository policy read: Read the full target AGENTS.md and applied its read-only review and CLI output compatibility guidance. (AGENTS.md:1, be2d22fe9d8c)
  • Current main has no request phone field: Current main writes the raw group participant request slice for JSON, so the requested phone_number output is not already implemented. (cmd/wacli/groups_admin.go:262, be2d22fe9d8c)
  • Current main table contract: Current main emits two tab-separated fields for groups requests list: request JID followed by local timestamp. (cmd/wacli/groups_admin.go:265, be2d22fe9d8c)
  • Resolver is existing API: The app WA client interface and WhatsApp client wrapper already expose ResolveLIDToPN, making the PR use an existing resolver instead of adding a parallel path. (internal/app/app.go:35, be2d22fe9d8c)
  • Latest PR preserves table field order: The latest diff appends phone after timestamp in table output and adds a focused table column order test. (cmd/wacli/groups_admin.go:294, ddf1af4b4e7c)
  • Real behavior proof: The PR body includes redacted live JSON output from groups requests list showing after-fix phone_number values on pending requests. (ddf1af4b4e7c)

Likely related people:

  • Dovocoder: Introduced the group admin commands, including request listing, and later touched adjacent command surface. (role: introduced behavior and recent area contributor; confidence: high; commits: dc75b680c4ae, 5b2ff7366d73; files: cmd/wacli/groups_admin.go, cmd/wacli/groups_admin_test.go, internal/wa/groups.go)
  • Peter Steinberger: Current blame ties the ResolveLIDToPN interface and client implementation used by this PR to the release baseline commit. (role: adjacent LID resolver contributor; confidence: medium; commits: 201f7adcaf16; files: internal/app/app.go, internal/wa/client.go)
  • Dinakar Sarbada: The group admin command introduction commit credits this person as a co-author, making them relevant for original command behavior context. (role: co-author on related feature history; confidence: medium; commits: dc75b680c4ae; files: cmd/wacli/groups_admin.go, cmd/wacli/groups_admin_test.go, internal/wa/groups.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 7, 2026
@cielecki cielecki force-pushed the feat/requests-phone-resolution branch from cbefbae to 7e0d7aa Compare June 7, 2026 10:21
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 7, 2026
@cielecki cielecki force-pushed the feat/requests-phone-resolution branch from 7e0d7aa to ddf1af4 Compare June 7, 2026 10:32
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 7, 2026
cielecki and others added 2 commits June 10, 2026 10:55
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
@steipete steipete force-pushed the feat/requests-phone-resolution branch from ddf1af4 to 5a8bf88 Compare June 10, 2026 09:58

@steipete steipete left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@steipete steipete merged commit 00439d4 into openclaw:main Jun 10, 2026
3 checks passed
@steipete

Copy link
Copy Markdown
Collaborator

Landed as 00439d4.

Design/proof summary:

  • Reused the existing ResolveLIDToPN boundary; no separate LID mapping path was added.
  • JSON output preserves existing JID and RequestedAt keys and adds optional phone_number when the resolved JID is on s.whatsapp.net.
  • Table output keeps the existing JID and timestamp field positions and appends phone as the third tab-separated field.
  • Added the maintainer changelog entry and updated the PR body to match the final table order.

Local verification on 5a8bf88:

  • go test ./cmd/wacli -run 'TestGroupsRequests|TestResolveRequestEntries|TestRequestListEntry'
  • pnpm -s docs:site && pnpm -s format:check && pnpm -s lint && pnpm -s test && pnpm -s build && git diff --check
  • go test -race ./cmd/wacli ./internal/app ./internal/wa ./internal/fsutil
  • go run golang.org/x/tools/cmd/deadcode@v0.45.0 -test -tags sqlite_fts5 ./...
  • goreleaser check --config .goreleaser.yaml && goreleaser check --config .goreleaser-linux-windows.yaml
  • autoreview --mode branch --base origin/main: clean, no accepted/actionable findings

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants