fix: hide stale reviews for missing commits

apps/staged/src-tauri/src/git/cli.rs

@@ -2,6 +2,8 @@ use std::path::Path;
 use std::process::Command;
 use thiserror::Error;
 
+use super::strip_git_env;
+
 #[derive(Error, Debug)]
 pub enum GitError {
     #[error("git not found - is git installed?")]
@@ -20,24 +22,6 @@ pub enum GitError {
     InvalidPath(String),
 }
 
-const GIT_LOCAL_ENV_VARS: &[&str] = &[
-    "GIT_ALTERNATE_OBJECT_DIRECTORIES",
-    "GIT_CONFIG",
-    "GIT_CONFIG_PARAMETERS",
-    "GIT_CONFIG_COUNT",
-    "GIT_OBJECT_DIRECTORY",
-    "GIT_DIR",
-    "GIT_WORK_TREE",
-    "GIT_IMPLICIT_WORK_TREE",
-    "GIT_GRAFT_FILE",
-    "GIT_INDEX_FILE",
-    "GIT_NO_REPLACE_OBJECTS",
-    "GIT_REPLACE_REF_BASE",
-    "GIT_PREFIX",
-    "GIT_SHALLOW_FILE",
-    "GIT_COMMON_DIR",
-];
-
 /// Run a git command and return stdout as a string
 pub fn run(repo: &Path, args: &[&str]) -> Result<String, GitError> {
     let repo_str = repo
@@ -46,9 +30,7 @@ pub fn run(repo: &Path, args: &[&str]) -> Result<String, GitError> {
 
     let mut command = Command::new("git");
     command.args(["-C", repo_str]).args(args);
-    for key in GIT_LOCAL_ENV_VARS {
-        command.env_remove(key);
-    }
+    strip_git_env(&mut command);
 
     let output = command.output().map_err(|e| {
         if e.kind() == std::io::ErrorKind::NotFound {

apps/staged/src-tauri/src/git/env.rs

@@ -0,0 +1,15 @@
+use std::ffi::OsStr;
+use std::process::Command;
+
+/// Remove inherited git-specific environment from a command before running git.
+pub(crate) fn strip_git_env(command: &mut Command) {
+    for (key, _) in std::env::vars_os() {
+        if starts_with_git_prefix(&key) {
+            command.env_remove(key);
+        }
+    }
+}
+
+fn starts_with_git_prefix(key: &OsStr) -> bool {
+    key.to_string_lossy().starts_with("GIT_")
+}

apps/staged/src-tauri/src/git/mod.rs

@@ -1,6 +1,7 @@
 mod cli;
 mod commit;
 mod diff;
+mod env;
 mod files;
 pub mod github;
 mod refs;
@@ -10,6 +11,7 @@ mod worktree;
 pub use cli::GitError;
 pub use commit::commit;
 pub use diff::{get_file_diff, get_unified_diff, list_diff_files};
+pub(crate) use env::strip_git_env;
 pub use files::{get_file_at_ref, search_files};
 pub use github::{
     check_github_auth, check_monorepo_modules, create_pull_request, detect_default_branch_for_repo,

apps/staged/src-tauri/src/lib.rs

@@ -26,6 +26,9 @@ pub(crate) mod terminal_output;
 pub mod timeline;
 pub mod util_commands;
 
+#[cfg(test)]
+pub mod test_utils;
+
 use serde::Serialize;
 use std::path::PathBuf;
 use std::sync::atomic::{AtomicBool, Ordering};

apps/staged/src-tauri/src/session_runner.rs

@@ -2215,6 +2215,7 @@ pub fn emit_session_running(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::git::strip_git_env;
 
     #[cfg(unix)]
     #[tokio::test]
@@ -2293,25 +2294,7 @@ mod tests {
     fn run_git(dir: &std::path::Path, args: &[&str]) -> String {
         let mut command = std::process::Command::new("git");
         command.args(args).current_dir(dir);
-        for key in [
-            "GIT_ALTERNATE_OBJECT_DIRECTORIES",
-            "GIT_CONFIG",
-            "GIT_CONFIG_PARAMETERS",
-            "GIT_CONFIG_COUNT",
-            "GIT_OBJECT_DIRECTORY",
-            "GIT_DIR",
-            "GIT_WORK_TREE",
-            "GIT_IMPLICIT_WORK_TREE",
-            "GIT_GRAFT_FILE",
-            "GIT_INDEX_FILE",
-            "GIT_NO_REPLACE_OBJECTS",
-            "GIT_REPLACE_REF_BASE",
-            "GIT_PREFIX",
-            "GIT_SHALLOW_FILE",
-            "GIT_COMMON_DIR",
-        ] {
-            command.env_remove(key);
-        }
+        strip_git_env(&mut command);
         let output = command.output().unwrap();
         assert!(
             output.status.success(),

apps/staged/src-tauri/src/test_utils.rs

@@ -0,0 +1,68 @@
+use std::fs;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+use uuid::Uuid;
+
+use crate::git::strip_git_env;
+
+/// A temporary git repository for use in tests.
+///
+/// Creates a fresh git repo in a temp directory and cleans it up on drop.
+pub struct TempGitRepo {
+    path: PathBuf,
+}
+
+impl TempGitRepo {
+    pub fn new() -> Self {
+        let path = std::env::temp_dir().join(format!("staged-test-{}", Uuid::new_v4()));
+        fs::create_dir_all(&path).unwrap();
+
+        let repo = Self { path };
+        repo.run_git(&["init", "--initial-branch=main"]);
+        repo.run_git(&["config", "user.email", "test@example.com"]);
+        repo.run_git(&["config", "user.name", "Test"]);
+        repo
+    }
+
+    pub fn path(&self) -> &Path {
+        &self.path
+    }
+
+    pub fn write_file(&self, name: &str, content: &str) {
+        fs::write(self.path.join(name), content).unwrap();
+    }
+
+    pub fn commit(&self, message: &str) -> String {
+        self.run_git(&["add", "."]);
+        self.run_git(&["commit", "-m", message]);
+        self.run_git(&["rev-parse", "HEAD"]).trim().to_string()
+    }
+
+    pub fn run_git(&self, args: &[&str]) -> String {
+        let mut command = Command::new("git");
+        command
+            .arg("-c")
+            .arg("core.hooksPath=/dev/null")
+            .arg("-C")
+            .arg(&self.path)
+            .args(args);
+        strip_git_env(&mut command);
+
+        let output = command.output().unwrap();
+
+        assert!(
+            output.status.success(),
+            "git {:?} failed: {}",
+            args,
+            String::from_utf8_lossy(&output.stderr)
+        );
+
+        String::from_utf8(output.stdout).unwrap()
+    }
+}
+
+impl Drop for TempGitRepo {
+    fn drop(&mut self) {
+        let _ = fs::remove_dir_all(&self.path);
+    }
+}

apps/staged/src-tauri/src/timeline.rs

@@ -2,11 +2,12 @@
 
 use crate::git;
 use crate::session_runner;
-use crate::store::Store;
+use crate::store::{CommentAuthor, Review, Store};
 use crate::{
     blox, branches, BranchTimeline, CommitTimelineItem, ImageTimelineItem, NoteTimelineItem,
     ReviewTimelineItem,
 };
+use std::collections::HashSet;
 use std::path::Path;
 use std::sync::{Arc, Mutex};
 
@@ -146,6 +147,12 @@ fn build_branch_timeline(store: &Arc<Store>, branch_id: &str) -> Result<BranchTi
         }
     }
 
+    let visible_shas: HashSet<&str> = commits
+        .iter()
+        .filter(|c| !c.sha.is_empty())
+        .map(|c| c.sha.as_str())
+        .collect();
+
     // Get notes
     let db_notes = store
         .list_notes_for_branch(branch_id)
@@ -175,6 +182,7 @@ fn build_branch_timeline(store: &Arc<Store>, branch_id: &str) -> Result<BranchTi
         .map_err(|e| e.to_string())?;
     let reviews: Vec<ReviewTimelineItem> = db_reviews
         .into_iter()
+        .filter(|r| review_is_visible_in_timeline(r, &visible_shas))
         .map(|r| {
             let resolved = store.resolve_session_status(r.session_id.as_deref());
             let comment_count = r.comments.len();
@@ -225,6 +233,15 @@ fn build_branch_timeline(store: &Arc<Store>, branch_id: &str) -> Result<BranchTi
     })
 }
 
+fn review_is_visible_in_timeline(review: &Review, visible_shas: &HashSet<&str>) -> bool {
+    review.commit_sha.is_empty()
+        || visible_shas.contains(review.commit_sha.as_str())
+        || review
+            .comments
+            .iter()
+            .any(|comment| comment.author == CommentAuthor::User)
+}
+
 #[tauri::command]
 pub async fn get_branch_timeline(
     store: tauri::State<'_, Mutex<Option<Arc<Store>>>>,
@@ -447,3 +464,109 @@ pub async fn delete_commit(
     .await
     .map_err(|e| format!("Delete task failed: {e}"))?
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::git::Span;
+    use crate::store::models::{Branch, Comment, Project, ReviewScope, Workdir};
+    use crate::test_utils::TempGitRepo;
+
+    fn repo_with_visible_and_stale_commit() -> (TempGitRepo, String, String) {
+        let repo = TempGitRepo::new();
+        repo.write_file("file.txt", "base\n");
+        repo.commit("base");
+        repo.run_git(&["update-ref", "refs/remotes/origin/main", "main"]);
+        repo.run_git(&["checkout", "-b", "feature"]);
+        repo.write_file("file.txt", "base\nvisible\n");
+        let visible_sha = repo.commit("visible change");
+        repo.write_file("file.txt", "base\nvisible\nstale\n");
+        let stale_sha = repo.commit("stale change");
+        repo.run_git(&["reset", "--hard", &visible_sha]);
+
+        (repo, visible_sha, stale_sha)
+    }
+
+    fn store_with_branch(repo: &TempGitRepo) -> (Arc<Store>, Branch) {
+        let store = Arc::new(Store::in_memory().unwrap());
+        let project = Project::new("test-owner/test-repo");
+        store.create_project(&project).unwrap();
+        let branch = Branch::new(&project.id, "feature", "main");
+        store.create_branch(&branch).unwrap();
+        let workdir =
+            Workdir::new(&project.id, repo.path().to_str().unwrap()).with_branch(&branch.id);
+        store.create_workdir(&workdir).unwrap();
+
+        (store, branch)
+    }
+
+    #[test]
+    fn build_branch_timeline_hides_reviews_for_commits_missing_from_git_history() {
+        let (repo, visible_sha, stale_sha) = repo_with_visible_and_stale_commit();
+        let (store, branch) = store_with_branch(&repo);
+
+        let visible_review = Review::new(&branch.id, &visible_sha, ReviewScope::Commit);
+        let stale_review = Review::new(&branch.id, &stale_sha, ReviewScope::Commit);
+        store.create_review(&visible_review).unwrap();
+        store.create_review(&stale_review).unwrap();
+
+        let timeline = build_branch_timeline(&store, &branch.id).unwrap();
+
+        assert_eq!(timeline.commits.len(), 1);
+        assert_eq!(timeline.commits[0].sha, visible_sha);
+        assert_eq!(timeline.reviews.len(), 1);
+        assert_eq!(timeline.reviews[0].id, visible_review.id);
+    }
+
+    #[test]
+    fn build_branch_timeline_hides_stale_reviews_with_only_agent_comments() {
+        let (repo, _visible_sha, stale_sha) = repo_with_visible_and_stale_commit();
+        let (store, branch) = store_with_branch(&repo);
+
+        let stale_review = Review::new(&branch.id, &stale_sha, ReviewScope::Commit);
+        let agent_comment = Comment::new("file.txt", Span::new(2, 3), "Agent note")
+            .with_author(CommentAuthor::Agent);
+        store.create_review(&stale_review).unwrap();
+        store.add_comment(&stale_review.id, &agent_comment).unwrap();
+
+        let timeline = build_branch_timeline(&store, &branch.id).unwrap();
+
+        assert_eq!(timeline.commits.len(), 1);
+        assert!(timeline.reviews.is_empty());
+    }
+
+    #[test]
+    fn build_branch_timeline_keeps_stale_reviews_with_user_comments() {
+        let (repo, _visible_sha, stale_sha) = repo_with_visible_and_stale_commit();
+        let (store, branch) = store_with_branch(&repo);
+
+        let stale_review = Review::new(&branch.id, &stale_sha, ReviewScope::Commit);
+        let user_comment = Comment::new("file.txt", Span::new(2, 3), "Please follow up");
+        store.create_review(&stale_review).unwrap();
+        store.add_comment(&stale_review.id, &user_comment).unwrap();
+
+        let timeline = build_branch_timeline(&store, &branch.id).unwrap();
+
+        assert_eq!(timeline.commits.len(), 1);
+        assert_eq!(timeline.reviews.len(), 1);
+        assert_eq!(timeline.reviews[0].id, stale_review.id);
+        assert_eq!(timeline.reviews[0].comment_count, 1);
+    }
+
+    #[test]
+    fn build_branch_timeline_hides_stale_reviews_with_deleted_user_comments() {
+        let (repo, _visible_sha, stale_sha) = repo_with_visible_and_stale_commit();
+        let (store, branch) = store_with_branch(&repo);
+
+        let stale_review = Review::new(&branch.id, &stale_sha, ReviewScope::Commit);
+        let user_comment = Comment::new("file.txt", Span::new(2, 3), "Please follow up");
+        store.create_review(&stale_review).unwrap();
+        store.add_comment(&stale_review.id, &user_comment).unwrap();
+        store.delete_comment(&user_comment.id).unwrap();
+
+        let timeline = build_branch_timeline(&store, &branch.id).unwrap();
+
+        assert_eq!(timeline.commits.len(), 1);
+        assert!(timeline.reviews.is_empty());
+    }
+}