Skip to content

Commit 53f9e71

Browse files
author
chaodu-agent
committed
test(discord): add pure should_process_reaction helper + focused gating tests
Extract reaction gating logic into a testable pure function and add 8 focused tests covering: - Mentions mode always rejects reactions - Non-thread channels skip participation check - Uninvolved thread rejected - Involved thread accepted - MultibotMentions single-bot thread accepted - MultibotMentions targets this bot accepted - MultibotMentions targets other bot rejected - MultibotMentions non-thread rejected This pins the gating ordering that regressed during review.
1 parent 8213774 commit 53f9e71

1 file changed

Lines changed: 141 additions & 18 deletions

File tree

crates/openab-core/src/discord.rs

Lines changed: 141 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,24 +1034,15 @@ impl EventHandler for Handler {
10341034

10351035
// Gating decision based on allow_user_messages mode.
10361036
let message_author_id = reaction.message_author_id;
1037-
match self.allow_user_messages {
1038-
AllowUsers::Mentions => return, // unreachable (early-returned above) but exhaustive
1039-
AllowUsers::Involved => {
1040-
if !is_thread || !bot_involved {
1041-
return;
1042-
}
1043-
}
1044-
AllowUsers::MultibotMentions => {
1045-
if !is_thread || !bot_involved {
1046-
return;
1047-
}
1048-
if other_bot_present {
1049-
match message_author_id {
1050-
Some(author) if author == bot_id => {}
1051-
_ => return,
1052-
}
1053-
}
1054-
}
1037+
let targets_this_bot = message_author_id.is_some_and(|a| a == bot_id);
1038+
if !should_process_reaction(
1039+
self.allow_user_messages,
1040+
is_thread,
1041+
bot_involved,
1042+
other_bot_present,
1043+
targets_this_bot,
1044+
) {
1045+
return;
10551046
}
10561047

10571048
// --- Spawn: user resolution + is_denied_user + dispatch ---
@@ -2504,6 +2495,39 @@ fn should_process_user_message(
25042495
}
25052496
}
25062497

2498+
/// Pure decision function: should a reaction event be processed?
2499+
/// Returns `true` if the reaction should trigger the mapped prompt.
2500+
///
2501+
/// Unlike message gating, reactions have no @mention concept. In
2502+
/// MultibotMentions mode, targeting is determined by whether the reaction
2503+
/// was placed on this bot's message (`targets_this_bot`).
2504+
///
2505+
/// This function is called AFTER:
2506+
/// - channel/thread allowlist has passed
2507+
/// - `is_thread` is known from `detect_thread`
2508+
/// - `bot_involved` is from `bot_participated_in_thread` (only if is_thread)
2509+
fn should_process_reaction(
2510+
mode: AllowUsers,
2511+
is_thread: bool,
2512+
bot_involved: bool,
2513+
other_bot_present: bool,
2514+
targets_this_bot: bool,
2515+
) -> bool {
2516+
match mode {
2517+
AllowUsers::Mentions => false,
2518+
AllowUsers::Involved => is_thread && bot_involved,
2519+
AllowUsers::MultibotMentions => {
2520+
if !is_thread || !bot_involved {
2521+
return false;
2522+
}
2523+
if other_bot_present {
2524+
return targets_this_bot;
2525+
}
2526+
true
2527+
}
2528+
}
2529+
}
2530+
25072531
/// Returns true if any bot message in `messages` contains a turn limit warning.
25082532
/// Used to dedup `WarnAndStop` across multiple bot processes sharing a thread. (#530)
25092533
/// Note: this is best-effort — a narrow race window exists where two bots fetch
@@ -3451,4 +3475,103 @@ mod tests {
34513475
fn dedup_returns_false_for_empty_messages() {
34523476
assert!(!turn_limit_warning_present(&[]));
34533477
}
3478+
3479+
// --- should_process_reaction tests ---
3480+
// Pins the reaction gating logic to prevent regressions (F1/F2/F3 review cycle).
3481+
3482+
/// GIVEN: Mentions mode (reactions cannot @mention)
3483+
/// THEN: always rejected
3484+
#[test]
3485+
fn reaction_mentions_mode_always_rejected() {
3486+
assert!(!should_process_reaction(
3487+
AllowUsers::Mentions,
3488+
true, true, false, false,
3489+
));
3490+
}
3491+
3492+
/// GIVEN: Involved mode, non-thread channel
3493+
/// THEN: rejected (participation check never runs for non-threads)
3494+
#[test]
3495+
fn reaction_involved_non_thread_rejected() {
3496+
assert!(!should_process_reaction(
3497+
AllowUsers::Involved,
3498+
false, // is_thread
3499+
false, // bot_involved (irrelevant for non-thread)
3500+
false, false,
3501+
));
3502+
}
3503+
3504+
/// GIVEN: Involved mode, thread, bot NOT involved
3505+
/// THEN: rejected
3506+
#[test]
3507+
fn reaction_involved_thread_not_participated_rejected() {
3508+
assert!(!should_process_reaction(
3509+
AllowUsers::Involved,
3510+
true, // is_thread
3511+
false, // bot_involved
3512+
false, false,
3513+
));
3514+
}
3515+
3516+
/// GIVEN: Involved mode, thread, bot IS involved
3517+
/// THEN: accepted
3518+
#[test]
3519+
fn reaction_involved_thread_participated_accepted() {
3520+
assert!(should_process_reaction(
3521+
AllowUsers::Involved,
3522+
true, // is_thread
3523+
true, // bot_involved
3524+
false, false,
3525+
));
3526+
}
3527+
3528+
/// GIVEN: MultibotMentions mode, single-bot thread, bot involved
3529+
/// THEN: accepted (no multibot contention)
3530+
#[test]
3531+
fn reaction_multibot_single_bot_thread_accepted() {
3532+
assert!(should_process_reaction(
3533+
AllowUsers::MultibotMentions,
3534+
true, // is_thread
3535+
true, // bot_involved
3536+
false, // other_bot_present
3537+
false, // targets_this_bot (irrelevant when no other bot)
3538+
));
3539+
}
3540+
3541+
/// GIVEN: MultibotMentions mode, multi-bot thread, reaction targets THIS bot's message
3542+
/// THEN: accepted
3543+
#[test]
3544+
fn reaction_multibot_targets_this_bot_accepted() {
3545+
assert!(should_process_reaction(
3546+
AllowUsers::MultibotMentions,
3547+
true, // is_thread
3548+
true, // bot_involved
3549+
true, // other_bot_present
3550+
true, // targets_this_bot
3551+
));
3552+
}
3553+
3554+
/// GIVEN: MultibotMentions mode, multi-bot thread, reaction targets OTHER bot's message
3555+
/// THEN: rejected
3556+
#[test]
3557+
fn reaction_multibot_targets_other_bot_rejected() {
3558+
assert!(!should_process_reaction(
3559+
AllowUsers::MultibotMentions,
3560+
true, // is_thread
3561+
true, // bot_involved
3562+
true, // other_bot_present
3563+
false, // targets_this_bot
3564+
));
3565+
}
3566+
3567+
/// GIVEN: MultibotMentions mode, non-thread channel
3568+
/// THEN: rejected
3569+
#[test]
3570+
fn reaction_multibot_non_thread_rejected() {
3571+
assert!(!should_process_reaction(
3572+
AllowUsers::MultibotMentions,
3573+
false, // is_thread
3574+
false, false, false,
3575+
));
3576+
}
34543577
}

0 commit comments

Comments
 (0)