← Back to PRs

#17865: Refactor: centralize dm/group policy resolution

by iyoda open 2026-02-16 07:53 View on GitHub →
channel: discord channel: imessage channel: signal channel: slack channel: whatsapp-web stale size: S
## Summary Describe the problem and fix in 2–5 bullets: - Problem: dmPolicy/groupPolicy resolution duplicated across channel implementations. - Why it matters: duplicates risk divergent defaults and policy behavior drift. - What changed: added shared policy resolver helpers and wired Slack/Discord/Signal/LINE/Web/iMessage to use them. - What did NOT change (scope boundary): policy defaults, allowlist logic, 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, Discord, Signal, LINE, WhatsApp (web), iMessage - Relevant config (redacted): N/A ### Steps 1. pnpm tsgo 2. pnpm build 3. pnpm test src/slack/monitor.test.ts src/discord/monitor.test.ts src/signal/monitor.test.ts src/line/bot-handlers.test.ts src/web/inbound/access-control.test.ts src/imessage/monitor.gating.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 channel policy 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 seven refactor commits on this branch. - Files/config to restore: channel provider files + src/channels/policy-resolve.ts. - Known bad symptoms reviewers should watch for: unexpected dmPolicy/groupPolicy behavior. ## Risks and Mitigations None. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR successfully centralizes DM and group policy resolution logic across six channel integrations (Slack, Discord, Signal, LINE, Web/WhatsApp, and iMessage). The refactor extracts duplicated policy fallback chains into two shared helper functions (`resolveDmPolicy` and `resolveGroupPolicy`) in `src/channels/policy-resolve.ts`. **Key changes:** - New shared resolver functions use named parameters with clear precedence: `accountPolicy` → `channelPolicy` → `legacyPolicy` → `defaultPolicy` - All six channels now use consistent resolution logic instead of inline `??` chains - Maintains exact behavioral equivalence - the nullish coalescing order is preserved - Each channel keeps its correct default policies (e.g., "open" for Discord/Slack/iMessage/Web, "allowlist" for Signal/LINE) - Legacy `dmConfig?.policy` fallback preserved for Discord and Slack backward compatibility **Quality observations:** - Well-structured commit history with one commit per channel plus the initial helper - Type-safe implementation with proper TypeScript types from `types.base.ts` - No test additions needed since behavior is unchanged (existing tests cover this) - The `channelPolicy` parameter exists in both resolvers but isn't used yet - appears to be forward-looking design for future feature work <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - it's a pure refactor that maintains exact behavioral equivalence - Score reflects that this is a textbook refactoring with no behavior changes. Each channel's policy resolution was verified to maintain identical logic through the new helper functions. The changes are well-contained, type-safe, and preserve all edge cases including legacy fallbacks. The commit structure is clean and the PR description accurately describes scope boundaries. - No files require special attention - all changes are straightforward mechanical refactors <sub>Last reviewed commit: 4bbf7e6</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs