← Back to PRs

#18511: fix(signal): signal:-prefixed phone numbers fail target resolution (#18495)

by yinghaosang open 2026-02-16 19:39 View on GitHub →
stale size: XS trusted-contributor
## Summary `signal:+15551234567` doesn't resolve as a direct target — the agent falls back to a directory lookup that fails with an unknown target error. Bare `+15551234567` works fine. Closes #18495 lobster-biscuit ## Root Cause `looksLikeSignalTargetId` in `signal.ts` handles the `signal:` prefix for group, username, and uuid patterns, but the phone number fallback regex (`/^\+?\d{3,}$/`) doesn't account for it. So `signal:+15551234567` fails the check, falls through to `stripTargetPrefixes` (which also doesn't strip `signal:`), and the directory lookup gets the raw prefixed string. ## Changes - Before: `signal:+15551234567` → `looksLikeId` returns false → directory lookup with `signal:+15551234567` → unknown target error - After: `signal:+15551234567` → `looksLikeId` returns true → direct target resolution works One-line regex change: `/^\+?\d{3,}$/` → `/^(signal:)?\+?\d{3,}$/` ## Tests - `signal.test.ts` — 1 new test covering `signal:+15551234567`, `signal:15551234567`, `signal:+4915551234567`; all fail before fix, pass after - All 6 tests in `signal.test.ts` pass - `pnpm vitest run src/channels/plugins/normalize/` — 8/8 pass <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes `signal:`-prefixed phone numbers (e.g., `signal:+15551234567`) failing target resolution by extending the phone number regex in `looksLikeSignalTargetId` to optionally match the `signal:` prefix. - One-line regex change in `src/channels/plugins/normalize/signal.ts`: `/^\+?\d{3,}$/` → `/^(signal:)?\+?\d{3,}$/` - Added 3 test cases in `plugins-channel.test.ts` covering prefixed phone numbers with/without `+` and international formats - The downstream normalization via `normalizeSignalMessagingTarget` already strips the `signal:` prefix correctly, so the resolved target is clean <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — the fix is minimal, well-tested, and correctly addresses the reported bug. - The one-line regex change is straightforward and the downstream normalization already handles stripping the `signal:` prefix. The only minor issue is the missing `i` flag for case-insensitive matching of the `signal:` prefix, which is inconsistent with the other patterns in the same function. - `src/channels/plugins/normalize/signal.ts` — the phone number regex should use the `i` flag for consistency with the other prefix patterns in the function. <sub>Last reviewed commit: bbd0552</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs