feat: add scoped mention pattern policy#70864
Conversation
openclaw-barnacle
Bot
added
docs
Greptile SummaryThis PR introduces a shared Confidence Score: 5/5PR is safe to merge — no correctness or security issues found. Logic in the policy resolver is correct and thoroughly tested. The only finding is a P2 noting that per-provider scoping via Zod schemas is only wired up for Discord and Matrix; all other providers silently fall back to the global mode. This is a documentation/completeness gap, not a defect in the changed code. src/channels/mention-pattern-policy.ts — the provider-policy fallback path is called for many providers whose schemas cannot actually supply a policy. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/channels/mention-pattern-policy.ts
Line: 39-48
Comment:
**Per-provider policy only configurable for Discord and Matrix**
All 10+ other providers that call `resolveMentionPatternsEnabled` (WhatsApp, Signal, Slack, LINE, IRC, iMessage, Mattermost, Nextcloud Talk, Zalouser, Telegram, BlueBubbles) pass `provider: "<name>"` but their Zod schemas use `.strict()` without a `mentionPatterns` field, so `cfg.channels?.["whatsapp"]?.mentionPatterns` will always be `undefined` after Zod validation. Those providers silently fall back to the global `messages.groupChat.mentionPatternsMode` and can never use `allowIn`/`denyIn` scoping. Only Discord (`DiscordAccountSchema` line 543 in `zod-schema.providers-core.ts`) and Matrix (`config-schema.ts`) expose this field through their schemas.
If per-provider scoping is intentionally limited to Discord/Matrix in this PR, a note in the docs to that effect would prevent operator confusion when attempting e.g. `channels.whatsapp.mentionPatterns`.
How can I resolve this? If you propose a fix, please make it concise.Reviews (10): Last reviewed commit: "fix: address mention policy review follo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const allowMatched = | ||
| conversationId != null && normalizeIdList(providerPolicy?.allowIn).has(conversationId); | ||
| const denyMatched = | ||
| conversationId != null && normalizeIdList(providerPolicy?.denyIn).has(conversationId); | ||
| const enabled = effectiveMode === "allow" ? !denyMatched : allowMatched && !denyMatched; |
There was a problem hiding this comment.
allowIn is silently ignored in allow effective mode
When effectiveMode === "allow", allowMatched is computed but never used in the enabled expression (!denyMatched). This means that configuring { mode: "inherit", allowIn: ["chan-1"] } on a provider while the global mode is "allow" has no effect — the entry silently does nothing. A user could reasonably expect allowIn to produce a no-op in this combination, but it would be worth either a comment or a doc note to avoid confusion in misconfigured setups.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/channels/mention-pattern-policy.ts
Line: 63-67
Comment:
**`allowIn` is silently ignored in `allow` effective mode**
When `effectiveMode === "allow"`, `allowMatched` is computed but never used in the `enabled` expression (`!denyMatched`). This means that configuring `{ mode: "inherit", allowIn: ["chan-1"] }` on a provider while the global mode is `"allow"` has no effect — the entry silently does nothing. A user could reasonably expect `allowIn` to produce a no-op in this combination, but it would be worth either a comment or a doc note to avoid confusion in misconfigured setups.
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9dfc8e46a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const mentionPatternsEnabled = core.channel.mentions.resolveMentionPatternsEnabled({ | ||
| cfg, | ||
| provider: "matrix", | ||
| conversationId: roomId, | ||
| }); |
There was a problem hiding this comment.
This call only passes { cfg, provider, conversationId }, so policy resolution falls back to cfg.channels.matrix.mentionPatterns; in multi-account Matrix runs, cfg is not rebuilt with accountConfig.mentionPatterns (only selected allowlist fields are merged), so channels.matrix.accounts.<id>.mentionPatterns has no effect. That means account-scoped mode: "deny" or allowIn/denyIn settings are silently ignored and mention gating behavior is wrong for non-default accounts.
Useful? React with 👍 / 👎.
75d3ac3 to
512e3e6
Compare
512e3e6 to
cca7168
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cca7168d2c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export const GroupChatSchema = z | ||
| .object({ | ||
| mentionPatterns: z.array(z.string()).optional(), | ||
| mentionPatternsMode: MentionPatternsModeSchema.optional(), |
There was a problem hiding this comment.
Adding mentionPatternsMode to GroupChatSchema makes agents.list[].groupChat.mentionPatternsMode valid because agent entries reuse this schema (src/config/zod-schema.agent-runtime.ts), but the new policy resolver only reads messages.groupChat.mentionPatternsMode (src/channels/mention-pattern-policy.ts:52). In practice, per-agent mode values are silently ignored, so users can set an apparently supported agent-scoped override that has no runtime effect; this should either be removed from the shared agent schema or wired through mention-policy resolution with agent context.
Useful? React with 👍 / 👎.
cca7168 to
6cc50a8
Compare
0dc81df to
9558b44
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Merged latest Verification passed:
Outstanding non-code blockers remain dependency graph/secops authorization and ClawSweeper live/equivalent Telegram transport proof acceptance. @greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Fixed the new Verification:
Refreshed the PR body Real behavior proof for exact head Outstanding non-code blockers remain dependency-guard secops approval and maintainer/security acceptance for the ClawSweeper durable review items. @greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Merged latest Verification:
Outstanding non-code blockers remain dependency-guard secops approval and maintainer/security acceptance for the ClawSweeper durable review items. @greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Merged latest Verification:
Outstanding non-code blockers remain dependency-guard secops approval and maintainer/security acceptance for the ClawSweeper durable review items. @greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Fixed the new Verification:
Outstanding non-code blockers remain dependency-guard secops approval and maintainer/security acceptance for the ClawSweeper durable review items. @greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Merged latest Verification:
Outstanding non-code blockers remain dependency-guard secops approval and maintainer/security acceptance for the ClawSweeper durable review items. @greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Fixed the new |
|
Refresh note after duplicate cleanup:
Local verification after the refresh:
CI note: the new run is red. |
|
Fixed the refreshed-head CI failures and pushed Verification:
@greptileai review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Landing proof for rewritten head f2db66a:
Results: targeted inbound suite passed 61 tests, type tests passed, generated config/docs/API baselines are current, and autoreview reported no accepted/actionable findings. Known proof gap: I did not run live Discord, Matrix, Slack, Telegram, or WhatsApp inbound messages after the refactor. Using maintainer proof override for that gap because this rewrite limits the new behavior to explicit per-channel configured regex mention-pattern policy and keeps native mentions plus unsupported channels on their existing paths. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2db66ab73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| effectiveDmAllow: dmAllow.effectiveAllow, | ||
| groupConfig, | ||
| topicConfig, | ||
| providerMentionPatterns: cfg.channels?.telegram?.accounts?.[account.accountId]?.mentionPatterns, |
There was a problem hiding this comment.
When the configured Telegram account key is not already in canonical form (for example accounts.Work resolving to account id work), this direct map lookup misses the account entry even though Telegram account resolution supports normalized keys via resolveNormalizedAccountEntry. In that setup the bot still runs under the account, but its account-scoped mentionPatterns policy is ignored and policy falls back to the root/default behavior; use the resolved/merged Telegram account config instead of indexing the raw accounts object here.
Useful? React with 👍 / 👎.
|
Final landing proof for head 4cc9409:
Results: focused inbound suite passed 61 tests, test types passed, lint passed after the final lint-only cleanup, generated config/docs/API baselines are current, and autoreview was clean. Known proof gap remains live channel runtime coverage for Discord, Matrix, Slack, Telegram, and WhatsApp; maintained under proof override for this scoped config/helper refactor. |
2 participants
Summary
Refactors the scoped mention-pattern policy into a small shared helper instead of per-channel policy logic.
This rewrite keeps configured regex mention patterns as a fallback to native platform mentions, then lets selected channel providers scope those configured patterns by conversation ID:
channels.<provider>.mentionPatterns.mode: "allow"keeps current behavior and can disable configured patterns withdenyIn.channels.<provider>.mentionPatterns.mode: "deny"disables configured patterns for that provider by default and enables them only forallowInconversations.messages.groupChat.mentionPatterns/ per-agent regex patterns.The runtime wiring is intentionally narrow now: Discord, Matrix, Slack, Telegram, and WhatsApp pass provider/conversation policy facts into the shared
buildMentionRegexeshelper. Other channels keep current behavior until they are migrated explicitly.Why This Shape
The previous version touched too many plugins and duplicated the same allow/deny logic at each call site. This version moves the decision into
src/channels/mention-pattern-policy.tsand keeps channel code responsible only for supplying stable facts: provider id, conversation id, and any resolved account policy.Out of scope:
User-Facing Behavior
Users can now scope configured regex mention patterns for the migrated providers without changing native @mention handling. Default behavior remains unchanged when no provider-level mention-pattern policy is configured.
Verification
node scripts/run-vitest.mjs src/auto-reply/inbound.test.tspnpm check:test-typesgit diff --checkpnpm config:channels:checkpnpm config:docs:checkpnpm plugin-sdk:api:check.agents/skills/autoreview/scripts/autoreview --mode local-> clean, no accepted/actionable findingsReal behavior proof
Behavior addressed: Scoped configured group mention regexes by provider conversation, while preserving native platform mentions and default behavior when no policy is configured.
Real environment tested: Local source checkout on Node/pnpm with focused unit, type, generated config/API, and autoreview proof.
Exact steps or command run after this patch:
node scripts/run-vitest.mjs src/auto-reply/inbound.test.ts;pnpm check:test-types;git diff --check;pnpm config:channels:check;pnpm config:docs:check;pnpm plugin-sdk:api:check;.agents/skills/autoreview/scripts/autoreview --mode local.Evidence after fix: Focused mention helper tests pass, typecheck passes, generated config/channel/API baselines are current, and autoreview reports no accepted/actionable findings.
Observed result after fix: Provider deny/allow policy gates configured regex mentions through the shared helper; unsupported callers keep current behavior because there is no global deny mode.
What was not tested: Live Discord/Matrix/Slack/Telegram/WhatsApp inbound messages were not exercised in this refactor pass.