huskies: merge 549_bug_stale_stage_transition_notifications_for_stories_that_skipped_stages

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
+                    // Only notify for transitions with a known source stage.
-                    // in the event (AC3); fall back to inference for synthetic events.
+                    // 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)
+                        .map(stage_display_name);
-                        .or_else(|| inferred_from_stage(stage));
                     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::<WatcherEvent>(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::<WatcherEvent>(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"
+        );
+    }
 }