← Back to PRs

#17878: Refactor: share allowlist normalization

by iyoda open 2026-02-16 08:09 View on GitHub →
channel: imessage channel: signal channel: slack stale size: S
## Summary Describe the problem and fix in 2–5 bullets: - Problem: multiple channels implement identical allowlist normalization logic. - Why it matters: duplicates increase drift risk and make fixes error-prone. - What changed: added shared normalize helpers and reused them in Slack/Signal/iMessage. - What did NOT change (scope boundary): allowlist semantics, policy decisions, or runtime behavior. ## Change Type (select all) - [ ] Bug fix - [ ] Feature - [x] 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 # ## User-visible / Behavior Changes None. ## 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 (darwin) - Runtime/container: Node 22 + pnpm (corepack) - Model/provider: N/A - Integration/channel (if any): Slack, Signal, iMessage - Relevant config (redacted): N/A ### Steps 1. pnpm tsgo 2. pnpm build 3. pnpm test src/channels/allowlists/normalize.test.ts src/signal/monitor.test.ts src/imessage/monitor.gating.test.ts src/slack/monitor.test.ts ### Expected - Commands complete successfully. ### Actual - Commands completed successfully. ## Evidence - [ ] Failing test/log before + passing after - [x] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) ## Human Verification (required) - Verified scenarios: typecheck, build, and targeted tests listed above. - Edge cases checked: N/A (refactor only). - What you did **not** verify: manual channel runtime behavior. ## 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 the four refactor commits on this branch. - Files/config to restore: allowlists helper + Slack/Signal/iMessage usage sites. - Known bad symptoms reviewers should watch for: unexpected allowlist normalization. ## Risks and Mitigations None. <!-- greptile_comment --> <h3>Greptile Summary</h3> Extracted duplicate `normalizeAllowList` implementations from Slack, Signal, and iMessage channels into a shared utility at `src/channels/allowlists/normalize.ts`. All three channels now import and use the same normalization logic, ensuring consistency across allowlist handling. - Removed identical `normalizeAllowList` functions from `src/imessage/monitor/runtime.ts`, `src/signal/monitor.ts`, and `src/slack/monitor/allow-list.ts` - Created shared `normalizeAllowList` helper with added `| null` type support (backward compatible) - Added `stripChannelPrefix` helper (tested but not yet used in codebase) - Slack re-exports `normalizeAllowList` from shared module to maintain existing import paths - All tests pass per PR description verification <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - Pure refactoring that extracts identical logic without changing behavior. The shared implementation is functionally identical to all three original implementations (only adds `| null` type support which is backward compatible). Test coverage added for the new shared module, and existing channel tests verify behavior preservation. - No files require special attention <sub>Last reviewed commit: df1d8b9</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs