diff --git a/server/src/chat/transport/matrix/notifications.rs b/server/src/chat/transport/matrix/notifications.rs index 3ad84581..45b39f2f 100644 --- a/server/src/chat/transport/matrix/notifications.rs +++ b/server/src/chat/transport/matrix/notifications.rs @@ -27,21 +27,6 @@ pub fn stage_display_name(stage: &str) -> &'static str { } } -/// Infer the previous pipeline stage for a given destination stage. -/// -/// Returns `None` for `1_backlog` since items are created there (not -/// transitioned from another stage). -pub fn inferred_from_stage(to_stage: &str) -> Option<&'static str> { - match to_stage { - "2_current" => Some("Backlog"), - "3_qa" => Some("Current"), - "4_merge" => Some("QA"), - "5_done" => Some("Merge"), - "6_archived" => Some("Done"), - _ => None, - } -} - /// Extract the numeric story number from an item ID like `"261_story_slug"`. pub fn extract_story_number(item_id: &str) -> Option<&str> { item_id @@ -233,12 +218,14 @@ pub fn spawn_notification_listener( ref from_stage, .. }) => { - // Determine from_display: prefer the actual from_stage recorded - // in the event (AC3); fall back to inference for synthetic events. + // Only notify for transitions with a known source stage. + // Synthetic events (reassign, creation) have from_stage=None + // and must be skipped — the old inferred_from_stage fallback + // produced wrong notifications for stories that skipped stages + // (e.g. "QA → Merge" when QA was never entered). let from_display = from_stage .as_deref() - .map(stage_display_name) - .or_else(|| inferred_from_stage(stage)); + .map(stage_display_name); let Some(from_display) = from_display else { continue; // creation or unknown transition — skip }; @@ -593,7 +580,7 @@ mod tests { item_id: "10_story_foo".to_string(), action: "qa".to_string(), commit_msg: "huskies: qa 10_story_foo".to_string(), - from_stage: None, + from_stage: Some("2_current".to_string()), }).unwrap(); // Wait longer than STAGE_TRANSITION_DEBOUNCE (200ms) so the coalesced @@ -628,7 +615,7 @@ mod tests { item_id: "10_story_foo".to_string(), action: "qa".to_string(), commit_msg: "huskies: qa 10_story_foo".to_string(), - from_stage: None, + from_stage: Some("2_current".to_string()), }).unwrap(); tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -650,27 +637,6 @@ mod tests { assert_eq!(stage_display_name("unknown"), "Unknown"); } - // ── inferred_from_stage ───────────────────────────────────────────────── - - #[test] - fn inferred_from_stage_returns_previous_stage() { - assert_eq!(inferred_from_stage("2_current"), Some("Backlog")); - assert_eq!(inferred_from_stage("3_qa"), Some("Current")); - assert_eq!(inferred_from_stage("4_merge"), Some("QA")); - assert_eq!(inferred_from_stage("5_done"), Some("Merge")); - assert_eq!(inferred_from_stage("6_archived"), Some("Done")); - } - - #[test] - fn inferred_from_stage_returns_none_for_backlog() { - assert_eq!(inferred_from_stage("1_backlog"), None); - } - - #[test] - fn inferred_from_stage_returns_none_for_unknown() { - assert_eq!(inferred_from_stage("9_unknown"), None); - } - // ── extract_story_number ──────────────────────────────────────────────── #[test] @@ -1123,4 +1089,89 @@ mod tests { let calls = calls.lock().unwrap(); assert_eq!(calls.len(), 1, "Only the first warning should be sent; second should be suppressed after hot-reload"); } + + // ── Bug 549: synthetic events with from_stage=None must not notify ────── + + /// Synthetic events (reassign, creation) have from_stage=None and must + /// not produce stage-transition notifications. Before the fix, the + /// inferred_from_stage fallback would emit e.g. "QA → Merge" for a + /// reassign event within the merge stage. + #[tokio::test] + async fn synthetic_event_without_from_stage_does_not_notify() { + let tmp = tempfile::tempdir().unwrap(); + + let (watcher_tx, watcher_rx) = broadcast::channel::(16); + let (transport, calls) = MockTransport::new(); + + spawn_notification_listener( + transport, + || vec!["!room1:example.org".to_string()], + watcher_rx, + tmp.path().to_path_buf(), + ); + + // Synthetic reassign event within 4_merge — no actual stage change. + watcher_tx.send(WatcherEvent::WorkItem { + stage: "4_merge".to_string(), + item_id: "549_story_skip_qa".to_string(), + action: "reassign".to_string(), + commit_msg: String::new(), + from_stage: None, + }).unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(350)).await; + + let calls = calls.lock().unwrap(); + assert_eq!( + calls.len(), 0, + "Synthetic events with from_stage=None must not generate notifications" + ); + } + + /// Stories that skip QA (qa: server) move directly from Current to Merge. + /// The notification must say "Current → Merge", not "QA → Merge". + #[tokio::test] + async fn skip_qa_shows_current_to_merge_not_qa_to_merge() { + let tmp = tempfile::tempdir().unwrap(); + let stage_dir = tmp.path().join(".huskies").join("work").join("4_merge"); + std::fs::create_dir_all(&stage_dir).unwrap(); + std::fs::write( + stage_dir.join("549_story_skip_qa.md"), + "---\nname: Skip QA Story\n---\n", + ) + .unwrap(); + + let (watcher_tx, watcher_rx) = broadcast::channel::(16); + let (transport, calls) = MockTransport::new(); + + spawn_notification_listener( + transport, + || vec!["!room1:example.org".to_string()], + watcher_rx, + tmp.path().to_path_buf(), + ); + + // Story skips QA: from_stage is 2_current, not 3_qa. + watcher_tx.send(WatcherEvent::WorkItem { + stage: "4_merge".to_string(), + item_id: "549_story_skip_qa".to_string(), + action: "merge".to_string(), + commit_msg: "huskies: merge 549_story_skip_qa".to_string(), + from_stage: Some("2_current".to_string()), + }).unwrap(); + + tokio::time::sleep(std::time::Duration::from_millis(350)).await; + + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 1, "Should send exactly one notification"); + assert!( + calls[0].1.contains("Current \u{2192} Merge"), + "Notification should say 'Current → Merge', got: {}", + calls[0].1 + ); + assert!( + !calls[0].1.contains("QA \u{2192} Merge"), + "Must NOT say 'QA → Merge' when QA was skipped" + ); + } }