fix(utils): normalizeE164 returns "" when there are no digits by he-yufeng · Pull Request #95974 · openclaw/openclaw

he-yufeng · 2026-06-23T05:04:59Z

What Problem This Solves

An inbound From that contains no digits normalizes to a stray "+". Two real cases hit this:

a blocked caller that the carrier reports as a literal Anonymous
a bare scheme like sms:// with no number attached

"+" is not a valid E.164 number, and because it is truthy, deriveSessionKey's from || "unknown" fallback never fires. Those messages get keyed to a junk "+" session bucket instead of the intended "unknown", so anything that special-cases the unknown sender silently skips them.

Why This Change Was Made

normalizeE164 only ever returned `+${digits}`, so empty digits produced a bare "+". The fix returns "" when there is nothing to normalize — which is exactly the value the internal caller already treats as "fall back to unknown". Valid numbers go through the same path as before.

User Impact

Anonymous / unidentified senders now collapse to the intended "unknown" session bucket instead of a bogus "+" one, so unknown-sender handling and session bookkeeping behave as designed. No change for normal phone numbers.

Evidence

The full vitest suite won't start on my Windows checkout (the @rolldown native binding is missing — npm optional-dependency bug), so I ran a small script that imports the real post-fix normalizeE164 from src/utils.ts, puts it next to main's verbatim pre-fix implementation, and feeds both through the verbatim direct-chat branch of deriveSessionKey (const from = ctx.From ? normalizeE164(ctx.From) : ""; return from || "unknown"), using the repo's bundled tsx:

$ node --import ./node_modules/tsx/dist/loader.mjs _repro_e164.mts
input                | before        | after  | sessionKey before | sessionKey after
---------------------|---------------|--------|-------------------|-----------------
"+1 (415) 555-0100"  | "+14155550100" | "+14155550100" | "+14155550100"    | "+14155550100"
"Anonymous"          | "+"           | ""     | "+"               | "unknown"
"sms://"             | "+"           | ""     | "+"               | "unknown"
"+"                  | "+"           | ""     | "+"               | "unknown"
""                   | "+"           | ""     | "unknown"         | "unknown"

So for a no-digit sender like Anonymous / sms:// / +, the session key moves from the bogus "+" bucket to "unknown", while a real number is untouched. (An empty From already short-circuited to "unknown" because the ternary never calls normalizeE164; the regression was specifically the non-empty no-digit inputs.)

Added a normalizeE164 unit test in src/utils.test.ts covering both valid numbers and the no-digit cases; it fails on the old behavior and passes with this change (runs in CI).

Note for maintainers

normalizeE164 is re-exported through the plugin SDK, so this is an intentional contract change for invalid no-digit input: plugin callers that previously saw "+" will now see "". That seems correct since "+" was never a valid E.164 value, but happy to gate it behind a guard at the deriveSessionKey call site instead if you'd rather not change the exported helper's behavior.

A From with no digits — a blocked caller the carrier reports as "Anonymous", or a bare "sms://" scheme — normalized to a stray "+", which is not a valid E.164. Since "+" is truthy, deriveSessionKey's `from || "unknown"` fallback never fired and those messages landed in a junk "+" session bucket instead of "unknown". Return "" when there is nothing to normalize so callers fall back as intended; valid numbers are unaffected.

clawsweeper · 2026-06-23T19:21:18Z

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 1:32 PM ET / 17:32 UTC.

Summary
The PR changes normalizeE164 to return an empty string for no-digit input and adds utility tests for valid and no-digit phone-like values.

PR surface: Source +4, Tests +20. Total +24 across 2 files.

Reproducibility: yes. Source inspection on current main shows no-digit normalizeE164 input returns a bare +, and deriveSessionKey then treats that truthy value as the session key instead of falling back to unknown; I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

Exported SDK Helper Behavior: 1 changed. normalizeE164 is exported through plugin SDK subpaths, so maintainers should notice the invalid-input contract change before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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:

Have a maintainer explicitly accept the empty-string invalid-input SDK contract or request the caller-only guard shape.

Risk before merge

[P1] Merging changes the invalid-input behavior of normalizeE164 across exported plugin SDK surfaces: existing plugin callers that observed + for no-digit input would now observe "".
[P1] If maintainers decide the exported helper must preserve its current invalid-input contract for one release, the safer merge shape is a caller-only guard in deriveSessionKey rather than changing the shared normalizer now.

Maintainer options:

Accept Empty Invalid-Input Result (recommended)
Maintainers can explicitly accept "" as the exported helper result for no-digit input because a bare + is not valid E.164 and breaks caller fallbacks.
Keep SDK Behavior For Now
If plugin SDK compatibility must win this release, move the no-digit guard to deriveSessionKey and keep normalizeE164 unchanged until a documented SDK change is approved.

Next step before merge

[P2] Manual review is needed because the remaining decision is whether to accept the exported SDK helper behavior change, not a mechanical repair.

Security
Cleared: The diff only changes deterministic string normalization and colocated tests; it does not touch dependencies, workflows, secrets, network calls, or code-execution surfaces.

Review details

Best possible solution:

Land the shared normalizer fix after maintainer sign-off that empty string is the SDK invalid-input contract; otherwise preserve the helper behavior and guard deriveSessionKey.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows no-digit normalizeE164 input returns a bare +, and deriveSessionKey then treats that truthy value as the session key instead of falling back to unknown; I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes, if maintainers accept the SDK contract correction: the shared normalizer should not manufacture an invalid E.164 value. If SDK compatibility is prioritized, the narrow alternative is a local no-digit guard in deriveSessionKey.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f57a30289d32.

Label changes

Label justifications:

P2: This is a focused normalization/session bug with bounded blast radius, but it affects real sender/session bookkeeping.
merge-risk: 🚨 compatibility: The PR changes behavior of an exported plugin SDK helper for invalid no-digit inputs.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 copied before/after live output from a script importing the post-fix helper and applying the session-key fallback, which is sufficient for this non-visual utility change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied before/after live output from a script importing the post-fix helper and applying the session-key fallback, which is sufficient for this non-visual utility change.

Evidence reviewed

PR surface:

Source +4, Tests +20. Total +24 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	3	+4
Tests	1	20	0	+20
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	27	3	+24

What I checked:

Current main normalizer behavior: On current main, normalizeE164 strips a scheme and non-phone characters, then always prepends +; no-digit non-empty input therefore returns a bare +. (src/utils.ts:56, f57a30289d32)
Current session fallback path: deriveSessionKey normalizes ctx.From and then uses from || "unknown", so a truthy bare + prevents the unknown-sender fallback. (src/config/sessions/session-key.ts:28, f57a30289d32)
Exported SDK surface: normalizeE164 is re-exported from plugin SDK utility/account/setup surfaces, so invalid-input behavior is visible to plugin callers, not only the local session-key caller. (src/plugin-sdk/text-utility-runtime.ts:12, f57a30289d32)
PR implementation: The PR computes the digit body without a leading plus, returns "" when no digits remain, and keeps valid phone-like inputs on the +digits output shape. (src/utils.ts:56, 2d40ae3be89d)
PR regression coverage: The PR adds tests for normal phone-like strings and no-digit values including Anonymous, sms://, empty string, and +. (src/utils.test.ts:21, 2d40ae3be89d)
Latest release still has old behavior: The latest release tag v2026.6.10 points at aa69b12d0086b631b139c1435c9621a5783e3a40 and still contains the bare-+ normalizer plus the same deriveSessionKey fallback path. (src/utils.ts:56, aa69b12d0086)

Likely related people:

steipete: History shows Peter Steinberger centralizing phone target normalization in src/utils.ts and adding session key normalizers that this fallback path depends on. (role: feature-history owner; confidence: high; commits: 9cd2662a86a5, ad7399b6e605, 4ca07559abe4; files: src/utils.ts, src/config/sessions/session-key.ts, src/plugin-sdk/account-core.ts)
vincentkoc: Recent history includes default-agent session routing compatibility work in src/config/sessions/session-key.ts, the caller whose fallback is affected by the bare +. (role: recent session-routing contributor; confidence: medium; commits: 61a18e5596c8; files: src/config/sessions/session-key.ts)
Ayaan Zaidi: Recent plugin SDK work touched src/plugin-sdk/text-utility-runtime.ts, one exported surface that exposes normalizeE164 to plugin callers. (role: recent plugin SDK utility surface contributor; confidence: medium; commits: d498b1cce49c; files: src/plugin-sdk/text-utility-runtime.ts)
CZH-THU: An older closed unmerged PR proposed returning an empty string for empty/invalid normalizeE164 inputs, making this person useful context for the prior attempt rather than current ownership. (role: prior same-behavior contributor; confidence: low; commits: 9594192b1a92; files: src/utils.ts, src/utils.test.ts)

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.

he-yufeng · 2026-06-24T17:15:55Z

Added inspectable before/after terminal output (real post-fix normalizeE164 plus the deriveSessionKey unknown fallback) to the PR description, and flagged the plugin-SDK contract change for maintainer sign-off. @clawsweeper re-review

steipete · 2026-07-06T01:22:09Z

Superseded by #100467, landed in f1a374a73a29589a19a99fd8737fcf7df1361a9c.

That canonical fix includes this PR's digit-free normalization behavior, plus stray-plus canonicalization, Signal invalid-input/UUID handling, iMessage non-phone handle preservation, and the corresponding regression coverage. Its exact-head hosted CI and maintainer review passed before merge.

Thanks @he-yufeng for identifying and fixing the no-digit case.

openclaw-barnacle Bot added the size: XS label Jun 23, 2026

clawsweeper Bot mentioned this pull request Jun 25, 2026

test(utils): add unit tests for E.164 phone number normalization #96744

Closed

clawsweeper Bot mentioned this pull request Jul 5, 2026

fix(channels): normalize phone identities with stray plus signs #100467

Merged

3 tasks

steipete closed this Jul 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(utils): normalizeE164 returns "" when there are no digits#95974

fix(utils): normalizeE164 returns "" when there are no digits#95974
he-yufeng wants to merge 1 commit into
openclaw:mainfrom
he-yufeng:fix/normalize-e164-empty-on-no-digits

he-yufeng commented Jun 23, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

he-yufeng commented Jun 24, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

he-yufeng commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Note for maintainers

Uh oh!

clawsweeper Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

he-yufeng commented Jun 24, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

he-yufeng commented Jun 23, 2026 •

edited

Loading

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading