Skip to content

fix(sms): accept spaced prefixed phone numbers#111111

Merged
steipete merged 4 commits into
openclaw:mainfrom
VectorPeak:fix/sms-prefixed-number-spacing-v2
Jul 20, 2026
Merged

fix(sms): accept spaced prefixed phone numbers#111111
steipete merged 4 commits into
openclaw:mainfrom
VectorPeak:fix/sms-prefixed-number-spacing-v2

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is OpenClaw’s 111,111th pull request. :)

What Problem This Solves

Fixes an issue where SMS users could provide a documented prefixed phone number with a space after the sms: or twilio-sms: prefix and have OpenClaw normalize it into an invalid double-plus phone number.

For example, sms:+1 (555) 123-4567 already normalized correctly, but the visually equivalent copied form sms: +1 (555) 123-4567 normalized to ++15551234567. That then failed SMS phone validation and could break explicit outbound targets, allowlist entries, and Twilio/RCS-style inbound sender normalization paths that reuse the SMS phone helper.

Why This Change Was Made

The fix stays inside the SMS-local phone normalizer and trims the value again after removing the optional sms: / twilio-sms: prefix. Existing punctuation and spacing tolerance inside phone numbers is unchanged; this only handles whitespace introduced between the prefix separator and the phone number.

This avoids broad shared E.164 helper changes and keeps the behavior scoped to SMS prefixes that this extension already accepts.

User Impact

Users can now paste or configure SMS targets and allowlist entries in either attached or spaced prefixed form, such as sms:+15551234567 or sms: +15551234567, and get the same canonical E.164-ish normalized number.

This makes SMS sender matching and explicit sends more forgiving without changing final validation: normalized numbers still must match the existing + followed by 7-15 digits rule.

Evidence

Before/after behavior from a local probe:

sms:+1 (555) 123-4567 => before +15551234567 valid=true; after +15551234567 valid=true
sms: +1 (555) 123-4567 => before ++15551234567 valid=false; after +15551234567 valid=true
twilio-sms: +1 (555) 123-4567 => before ++15551234567 valid=false; after +15551234567 valid=true

User-realistic call chain:

SMS target / allowlist / Twilio-style sender value
  -> normalizeSmsPhoneNumber() / normalizeSmsAllowFrom()
  -> looksLikeSmsPhoneNumber() or allowlist matching
  -> previously rejected or stored ++1555... when a space followed the prefix

Validation:

D:\ZXY\Github\openclaw\node_modules\.bin\oxfmt.cmd --check extensions/sms/src/phone.ts extensions/sms/src/phone.test.ts - passed
node scripts/run-vitest.mjs run extensions/sms/src/phone.test.ts - passed, 1 test file / 2 tests
git diff --check - passed
node scripts/run-vitest.mjs run extensions/sms/src/phone.test.ts extensions/sms/src/twilio.test.ts extensions/sms/src/accounts.test.ts - passed, 3 test files / 40 tests

@VectorPeak @chatgpt-codex-connector
fix(sms): trim prefixed phone number spacing
0bc5fa2 
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
@openclaw-barnacle openclaw-barnacle Bot added channel: sms Channel integration: sms size: XS labels Jul 19, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. P2 Normal backlog priority with limited blast radius. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 10:35 PM ET / July 19, 2026, 02:35 UTC.

Summary
The branch trims whitespace left after an sms: or twilio-sms: prefix and adds regression coverage for spaced prefixed phone values.

PR surface: Source +3, Tests +4. Total +7 across 2 files.

Reproducibility: yes. at source level: a spaced sms: or twilio-sms: prefix leaves a leading space after prefix removal, which punctuation stripping turns into a second +; the added regression cases exercise that path.

Review metrics: none identified.

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:

  • none.

Next step before merge

  • [P2] No mechanical repair is identified; this PR is ready for ordinary maintainer merge review.

Security
Cleared: The two-file TypeScript and test-only diff adds no dependency, workflow, permission, secret, package-resolution, or executable supply-chain surface.

Review details

Best possible solution:

Merge the focused SMS-local normalization repair after the normal current-head merge check; retain the regression cases so spaced and attached prefixes stay equivalent.

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

Yes, at source level: a spaced sms: or twilio-sms: prefix leaves a leading space after prefix removal, which punctuation stripping turns into a second +; the added regression cases exercise that path.

Is this the best way to solve the issue?

Yes. Re-trimming only after the SMS-specific prefix removal is the narrowest maintainable repair and avoids changing shared phone parsing outside the affected extension.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded SMS normalization bug that can reject otherwise valid explicit targets or allowlist values, without evidence of broad runtime failure.
  • 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 (terminal): The PR includes after-fix terminal output for the affected inputs, focused SMS test results, and successful real-behavior-proof checks; redact any private phone numbers if future evidence is added.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR includes after-fix terminal output for the affected inputs, focused SMS test results, and successful real-behavior-proof checks; redact any private phone numbers if future evidence is added.
Evidence reviewed

PR surface:

Source +3, Tests +4. Total +7 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 4 1 +3
Tests 1 4 0 +4
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 8 1 +7

What I checked:

  • Focused implementation: The proposed change keeps prefix-whitespace handling inside the SMS-local normalizer, preserving the existing final E.164-ish validation boundary. (extensions/sms/src/phone.ts:2, 2f7c1c9ec527)
  • Regression coverage: The PR adds attached and spaced sms:/twilio-sms: cases plus a looksLikeSmsPhoneNumber assertion, covering normalization, allowlist normalization, and validation entry points. (extensions/sms/src/phone.test.ts:11, 2f7c1c9ec527)
  • Prior review continuity: The prior two completed ClawSweeper review cycles found no actionable findings, and the current reviewed head matches the latest prior reviewed head. (2f7c1c9ec527)
  • Behavior proof: The PR body provides before/after terminal output for the previously invalid spaced-prefix inputs and reports focused SMS tests passing; GitHub context also records successful real-behavior-proof and changed-path checks. (2f7c1c9ec527)

Likely related people:

  • Unknown: The available review context identifies the proposed branch author but does not provide readable current-main SMS history sufficient to attribute ownership without guessing. (role: SMS area owner not established; confidence: low; files: extensions/sms/src/phone.ts, extensions/sms/src/phone.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.
Review history (2 earlier review cycles)
  • reviewed 2026-07-19T01:46:01.917Z sha 59956e9 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-19T01:53:14.689Z sha 2f7c1c9 :: needs maintainer review before merge. :: none

@steipete
steipete merged commit 3ef2d8c into openclaw:main Jul 20, 2026
83 checks passed
@steipete

steipete commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 20, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 21, 2026
@VectorPeak @chatgpt-codex-connector @steipete
fix(sms): trim prefixed phone number spacing (openclaw#111111)
e6828cd 
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: sms Channel integration: sms P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VectorPeak @steipete