fix(telegram): prioritize topic-level requireMention over activation override#49980
Conversation
Greptile SummaryThis PR fixes a bug in Telegram forum supergroups where per-topic Key changes:
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/telegram/src/bot-message-context.topic-require-mention.test.ts
Line: 96-111
Comment:
**Missing inverse priority test case**
The four tests only verify that `topicConfig.requireMention=false` wins over various higher-priority items. There's no test covering the symmetric case: `topicConfig.requireMention=true` overriding `activationOverride=false` (i.e., a user running `/activate` in a topic that has `requireMention=true` configured).
With the new ordering, the `/activate` command should be silently ignored for that topic, and a message without a `@mention` should still be dropped (return `null`). It would be worth adding a test to confirm this behavior is intentional and doesn't regress:
```typescript
it("topic requireMention=true overrides activationOverride=false (/activate ignored)", async () => {
const ctx = await buildTelegramMessageContextForTest({
message: buildForumMessage(),
// activationOverride=false → /activate was run (meaning requireMention=false)
resolveGroupActivation: () => false,
resolveGroupRequireMention: () => false,
resolveTelegramGroupConfig: () => ({
groupConfig: { requireMention: false },
topicConfig: { requireMention: true },
}),
});
// Topic says requireMention=true; /activate override should not bypass it.
expect(ctx).toBeNull();
});
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix(telegram): prior..." |
| it("activationOverride still works when no topic config is present", async () => { | ||
| const ctx = await buildTelegramMessageContextForTest({ | ||
| message: buildForumMessage(), | ||
| // activationOverride=false → requireMention=false (from /activate) | ||
| resolveGroupActivation: () => false, | ||
| resolveGroupRequireMention: () => true, | ||
| resolveTelegramGroupConfig: () => ({ | ||
| groupConfig: { requireMention: true }, | ||
| topicConfig: undefined, | ||
| }), | ||
| }); | ||
|
|
||
| // No topic config, but activation override says requireMention=false → | ||
| // message should be processed. | ||
| expect(ctx).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
The four tests only verify that topicConfig.requireMention=false wins over various higher-priority items. There's no test covering the symmetric case: topicConfig.requireMention=true overriding activationOverride=false (i.e., a user running /activate in a topic that has requireMention=true configured).
With the new ordering, the /activate command should be silently ignored for that topic, and a message without a @mention should still be dropped (return null). It would be worth adding a test to confirm this behavior is intentional and doesn't regress:
it("topic requireMention=true overrides activationOverride=false (/activate ignored)", async () => {
const ctx = await buildTelegramMessageContextForTest({
message: buildForumMessage(),
// activationOverride=false → /activate was run (meaning requireMention=false)
resolveGroupActivation: () => false,
resolveGroupRequireMention: () => false,
resolveTelegramGroupConfig: () => ({
groupConfig: { requireMention: false },
topicConfig: { requireMention: true },
}),
});
// Topic says requireMention=true; /activate override should not bypass it.
expect(ctx).toBeNull();
});Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/telegram/src/bot-message-context.topic-require-mention.test.ts
Line: 96-111
Comment:
**Missing inverse priority test case**
The four tests only verify that `topicConfig.requireMention=false` wins over various higher-priority items. There's no test covering the symmetric case: `topicConfig.requireMention=true` overriding `activationOverride=false` (i.e., a user running `/activate` in a topic that has `requireMention=true` configured).
With the new ordering, the `/activate` command should be silently ignored for that topic, and a message without a `@mention` should still be dropped (return `null`). It would be worth adding a test to confirm this behavior is intentional and doesn't regress:
```typescript
it("topic requireMention=true overrides activationOverride=false (/activate ignored)", async () => {
const ctx = await buildTelegramMessageContextForTest({
message: buildForumMessage(),
// activationOverride=false → /activate was run (meaning requireMention=false)
resolveGroupActivation: () => false,
resolveGroupRequireMention: () => false,
resolveTelegramGroupConfig: () => ({
groupConfig: { requireMention: false },
topicConfig: { requireMention: true },
}),
});
// Topic says requireMention=true; /activate override should not bypass it.
expect(ctx).toBeNull();
});
```
How can I resolve this? If you propose a fix, please make it concise.82f9d8b to
e84f7bf
Compare
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. Source inspection gives a high-confidence path: current main resolves Next step before merge Security Review detailsBest possible solution: Land this focused Telegram plugin patch after normal CI or changed-gate validation, keeping the explicit topic config precedence and closing the linked #49864 through the PR. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection gives a high-confidence path: current main resolves Is this the best way to solve the issue? Yes. Moving What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against a4f2bf273a21. Re-review progress:
|
e84f7bf to
e911455
Compare
Summary
Per-topic
requireMentionoverride is ignored in Telegram forum supergroups — group-level setting always wins.Fixes #49864
Root Cause
In
bot-message-context.ts, therequireMentionresolution chain usedfirstDefined()with this order:activationOverride(persisted from/activateor/deactivatecommands) was evaluated beforetopicConfig?.requireMention, so any persisted activation state would override explicit per-topic config.Fix
Reorder
firstDefined()so topic config is evaluated first:Testing
Added 4 new tests in a dedicated test file:
requireMention=falseoverrides grouprequireMention=trueactivationOverriderequireMentionstill works when no topic configactivationOverridestill works when no topic configAll 4 tests pass.
Made-with: Claude Code (Claude Opus 4.6)