← Back to PRs

#16562: fix(signal): preserve case for group and username targets

by akalypse open 2026-02-14 21:38 View on GitHub →
channel: signal stale size: S
## Summary - Problem: Signal target normalization lowercased group: and username: target values. - Why it matters: Case-sensitive identifiers can be mutated, causing misrouting or failed target resolution. In particular, it failed doing emoji reactions on group messages. - What changed: normalizeSignalMessagingTarget now preserves value casing for group: and username: while still recognizing prefixes. - What did NOT change (scope boundary): UUID normalization paths and other channel normalizers remain unchanged. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [x] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes # - Related # - If #12793 covers this exact normalization path, I’m happy to close this as duplicate. ## User-visible / Behavior Changes Signal group: and username: target values keep original case after normalization. Previously they were lowercased. ## Security Impact (required) - New permissions/capabilities? (No) - Secrets/tokens handling changed? (No) - New/changed network calls? (No) - Command/tool execution surface changed? (No) - Data access scope changed? (No) - If any Yes, explain risk + mitigation: ## Repro + Verification ### Environment - OS: macOS - Runtime/container: local dev runtime - Model/provider: N/A - Integration/channel (if any): Signal - Relevant config (redacted): Signal target values using group: and username: prefixes ### Steps 1. Call normalizeSignalMessagingTarget("group:AbC123-CaseSensitive"). 2. Call normalizeSignalMessagingTarget("username:MyMixedCaseUser"). 3. Compare normalized outputs. ### Expected - group:AbC123-CaseSensitive - username:MyMixedCaseUser ### Actual - Before fix: values were lowercased. - After fix: original case is preserved. ## Evidence Attach at least one: - [x] Failing test/log before + passing after - [ ] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) ## Human Verification (required) - Verified scenarios: - Added tests in src/channels/plugins/normalize/signal.test.ts for case preservation. - Ran pnpm test -- src/channels/plugins/normalize/signal.test.ts (passing). - Edge cases checked: - Existing UUID normalization tests still pass. - What you did not verify: - Full end-to-end Signal daemon flow in production-like environment. ## Compatibility / Migration - Backward compatible? (Yes) - Config/env changes? (No) - Migration needed? (No) - If yes, exact upgrade steps: ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: Revert commit fix(signal): preserve case for group and username targets. - Files/config to restore: src/channels/plugins/normalize/signal.ts, src/channels/plugins/normalize/signal.test.ts - Known bad symptoms reviewers should watch for: - Unexpected lowercasing reappearing for group: or username: targets. ## Risks and Mitigations - Risk: Downstream logic may have implicitly relied on lowercased group:/username: values. - Mitigation: Change is narrowly scoped; UUID normalization behavior unchanged; regression tests added. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes Signal target normalization to preserve case for `group:` and `username:` prefixed values, which were previously lowercased by `normalizeSignalMessagingTarget`. This caused issues with case-sensitive identifiers (e.g., base64 group IDs used for emoji reactions). It also expands `looksLikeSignalTargetId` to recognize `signal:<uuid>` and `signal:+E164` direct target formats, and updates the resolver hint string accordingly. - **Bug fix**: Removed `.toLowerCase()` from `username:` and `u:` normalization paths in `signal.ts` — group IDs were already case-preserving - **Enhancement**: `looksLikeSignalTargetId` now accepts `signal:<uuid>` and `signal:+E164` patterns directly - **Tests**: Good coverage added for case preservation and new target patterns - **Minor**: Duplicate test description `"preserves case for group targets"` on lines 23 and 44 of the test file <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — narrowly scoped fix with good test coverage and correct logic. - The changes are minimal and well-targeted: two `.toLowerCase()` removals for username normalization, and an additive expansion of target ID detection. The logic is consistent with the downstream `parseTarget` in `send.ts`. Tests cover the new behavior. One minor style issue (duplicate test description) does not affect correctness. - No files require special attention. <sub>Last reviewed commit: a5ce5a8</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs