diff --git a/apps/staged/src-tauri/src/branches.rs b/apps/staged/src-tauri/src/branches.rs index 097b4faa2..b18bf28aa 100644 --- a/apps/staged/src-tauri/src/branches.rs +++ b/apps/staged/src-tauri/src/branches.rs @@ -1525,7 +1525,6 @@ pub async fn create_remote_branch( #[tauri::command(rename_all = "camelCase")] pub async fn start_workspace( store: tauri::State<'_, Mutex>>>, - app_handle: AppHandle, branch_id: String, ) -> Result<(), String> { let store = get_store(&store)?; @@ -1535,12 +1534,6 @@ pub async fn start_workspace( .map_err(|e| e.to_string())? .ok_or_else(|| format!("Branch not found: {branch_id}"))?; - // Track whether this is the first workspace start (Starting → Running) - // vs a restart (Stopped → Running). We only trigger auto-review on the - // first start so that existing branches don't get a spurious review - // every time the workspace restarts. - let is_first_start = branch.workspace_status == Some(store::WorkspaceStatus::Starting); - let project = store .get_project(&branch.project_id) .map_err(|e| e.to_string())? @@ -1595,23 +1588,6 @@ pub async fn start_workspace( .update_branch_workspace_status(&branch_id, &store::WorkspaceStatus::Running) .map_err(|e| e.to_string())?; - // Trigger auto-review for the newly cloned secondary repo - // if this is the first start for this branch. - if is_first_start { - let store_bg = Arc::clone(&store); - let app_handle_bg = app_handle.clone(); - let branch_id_bg = branch_id.clone(); - tauri::async_runtime::spawn(async move { - crate::maybe_trigger_auto_review_for_new_repo( - &store_bg, - &app_handle_bg, - &branch_id_bg, - None, - ) - .await; - }); - } - return Ok(()); } } @@ -1696,24 +1672,6 @@ pub async fn start_workspace( ); } - // If this is the first workspace start for a new branch, check - // whether the branch already has commits (e.g. from an existing - // PR) and kick off an automatic code review. - if is_first_start { - let store_bg = Arc::clone(&store); - let app_handle_bg = app_handle.clone(); - let branch_id_bg = branch_id.clone(); - tauri::async_runtime::spawn(async move { - crate::maybe_trigger_auto_review_for_new_repo( - &store_bg, - &app_handle_bg, - &branch_id_bg, - None, - ) - .await; - }); - } - Ok(()) } Err(blox::BloxError::NotAuthenticated) => { diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index 5d1f18ad9..58a9b71fc 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -194,7 +194,6 @@ pub struct ReviewTimelineItem { pub completion_reason: Option, pub title: Option, pub comment_count: usize, - pub is_auto: bool, pub created_at: i64, pub updated_at: i64, pub completed_at: Option, @@ -310,70 +309,6 @@ fn confirm_reset_store( // Project commands // ============================================================================= -/// Check whether a newly added repo already has commits on its branch and, if -/// so, kick off an automatic code review. -/// -/// For **local** branches (`worktree_path: Some`) the check is done by -/// inspecting the worktree on disk before triggering. -/// For **remote** branches (`worktree_path: None`) the review is triggered -/// unconditionally — the caller is responsible for ensuring the workspace is -/// running before calling this (e.g. after `start_workspace` or -/// `setup_remote_repo_clone` completes). -/// -/// This is fire-and-forget — errors are logged but never propagated. -pub(crate) async fn maybe_trigger_auto_review_for_new_repo( - store: &Arc, - app_handle: &tauri::AppHandle, - branch_id: &str, - worktree_path: Option<&str>, -) { - // For local branches, check whether there are any commits on the branch - // relative to its base before spinning up a review session. - if let Some(path) = worktree_path { - let branch = match store.get_branch(branch_id) { - Ok(Some(b)) => b, - _ => return, - }; - let worktree = std::path::PathBuf::from(path); - let base_ref = git::origin_ref_for_branch(&branch.base_branch); - match git::get_commits_since_base(&worktree, &base_ref) { - Ok(commits) if commits.is_empty() => { - log::info!( - "[auto_review] branch {branch_id} has no commits yet — skipping auto review" - ); - return; - } - Err(e) => { - log::warn!("[auto_review] failed to check commits for branch {branch_id}: {e}"); - return; - } - Ok(_) => { /* has commits — fall through to trigger */ } - } - } - - let registry = app_handle.state::>(); - match session_commands::trigger_auto_review( - Arc::clone(store), - Arc::clone(®istry), - app_handle.clone(), - branch_id.to_string(), - None, - ) - .await - { - Ok(resp) => { - log::info!( - "[auto_review] triggered for new repo on branch {branch_id}: session={}, review={}", - resp.session_id, - resp.artifact_id, - ); - } - Err(e) => { - log::warn!("[auto_review] failed to trigger for branch {branch_id}: {e}"); - } - } -} - #[tauri::command] fn list_projects( store: tauri::State<'_, Mutex>>>, @@ -517,7 +452,7 @@ fn create_project( }) .await; - let worktree_path = match worktree_result { + match worktree_result { Ok(Ok(path)) => { log::info!("[create_project] worktree ready at {path}"); web_server::emit_to_all( @@ -525,7 +460,6 @@ fn create_project( "project-setup-progress", project_id.clone(), ); - path } Ok(Err(e)) => { log::warn!("[create_project] worktree setup failed: {e}"); @@ -535,7 +469,7 @@ fn create_project( log::warn!("[create_project] worktree task panicked: {e}"); return; } - }; + } let executor = app_handle.state::>(); let act_registry = app_handle.state::>(); @@ -553,21 +487,7 @@ fn create_project( { web_server::emit_to_all(&app_handle, "project-setup-progress", project_id); } - - // If the repo already has commits on this branch, kick off - // an automatic code review so the user gets immediate feedback. - maybe_trigger_auto_review_for_new_repo( - &store_bg, - &app_handle, - &branch_id, - Some(&worktree_path), - ) - .await; }); - } else { - // Remote branches: auto-review is deferred until `start_workspace` - // completes — the workspace isn't running yet at this point, so - // attempting to trigger a review here would fail. } } else if project.location == store::ProjectLocation::Remote { log::info!( @@ -681,7 +601,7 @@ async fn add_project_repo( }) .await; - let worktree_path = match worktree_result { + match worktree_result { Ok(Ok(path)) => { log::info!("[add_project_repo] worktree ready at {path}"); web_server::emit_to_all( @@ -689,7 +609,6 @@ async fn add_project_repo( "project-setup-progress", project_id.clone(), ); - path } Ok(Err(e)) => { log::warn!("[add_project_repo] worktree setup failed: {e}"); @@ -699,7 +618,7 @@ async fn add_project_repo( log::warn!("[add_project_repo] worktree task panicked: {e}"); return; } - }; + } let executor = app_handle.state::>(); let act_registry = app_handle.state::>(); @@ -720,16 +639,6 @@ async fn add_project_repo( project_id.clone(), ); } - - // If the repo already has commits on this branch, kick off - // an automatic code review so the user gets immediate feedback. - maybe_trigger_auto_review_for_new_repo( - &store, - &app_handle, - &branch.id, - Some(&worktree_path), - ) - .await; } else { // Remote branch: clone the repo into the running workspace, // fetch the base branch, and create the feature branch. @@ -750,16 +659,6 @@ async fn add_project_repo( } } web_server::emit_to_all(&app_handle, "project-setup-progress", project_id); - - // If the repo already has commits on this branch, kick off - // an automatic code review so the user gets immediate feedback. - maybe_trigger_auto_review_for_new_repo( - &store, - &app_handle, - &branch.id, - None, // remote — trigger_auto_review resolves HEAD via workspace - ) - .await; } } }); @@ -2337,8 +2236,6 @@ pub fn run() { session_commands::queue_branch_session, session_commands::drain_queued_sessions, session_commands::start_project_session, - session_commands::find_fresh_auto_review, - session_commands::set_review_auto, // Actions actions::commands::detect_repo_actions, actions::commands::run_branch_action, diff --git a/apps/staged/src-tauri/src/project_mcp.rs b/apps/staged/src-tauri/src/project_mcp.rs index 46fcc5053..2adca3225 100644 --- a/apps/staged/src-tauri/src/project_mcp.rs +++ b/apps/staged/src-tauri/src/project_mcp.rs @@ -628,22 +628,6 @@ impl ProjectToolsHandler { self.provider.clone() }; - // An in-flight auto review of the same branch would duplicate a requested - // review, and is invalidated by a commit (which triggers a fresh auto review - // once it lands), so cancel it first — same as user-initiated sessions. - if matches!( - p.expected_outcome, - RepoSessionOutcome::CodeReview | RepoSessionOutcome::Commit - ) { - if let Err(e) = crate::session_commands::cancel_in_flight_auto_review_for_branch( - &self.store, - &self.registry, - &target.branch.id, - ) { - return format!("Error cancelling in-flight auto review: {e}"); - } - } - let mut session = crate::store::Session::new_queued(&p.instructions); if let Some(ref provider) = session_provider { session = session.with_provider(provider); @@ -852,7 +836,6 @@ impl ProjectToolsHandler { branch_id: Some(branch_id.clone()), project_id: Some(self.project_id.clone()), session_type: None, - is_auto_review: false, }, ); let _ = crate::session_commands::drain_queued_sessions_for_branch( @@ -1035,7 +1018,7 @@ impl ProjectToolsHandler { }) .await; - let worktree_path = match worktree_result { + match worktree_result { Ok(Ok(path)) => { log::debug!("[project_mcp] add_project_repo: worktree ready at {}", path); // Notify UI that the worktree is ready so branch state updates @@ -1044,7 +1027,6 @@ impl ProjectToolsHandler { "project-setup-progress", self.project_id.clone(), ); - path } Ok(Err(e)) => { log::warn!( @@ -1063,15 +1045,13 @@ impl ProjectToolsHandler { "Added repository {github_repo} to project (worktree task error: {e})" ); } - }; + } // Prerun waits out any in-flight action detection and then runs // each setup action to completion — minutes, against an MCP // client's request timeout. Nothing in the reply derives from it, - // so the whole tail is detached, in one task so the auto-review - // still follows the setup actions. Structurally this is now the - // Tauri `add_project_repo` command's spawned setup task. Note that - // the reply therefore also lands before the auto-review is queued. + // so the whole tail is detached. Structurally this is now the + // Tauri `add_project_repo` command's spawned setup task. let store = Arc::clone(&self.store); let app_handle = self.app_handle.clone(); let project_id = self.project_id.clone(); @@ -1105,16 +1085,6 @@ impl ProjectToolsHandler { "[project_mcp] add_project_repo: no action executor available, skipping prerun actions" ); } - - // If the repo already has commits on this branch, kick off - // an automatic code review so the user gets immediate feedback. - crate::maybe_trigger_auto_review_for_new_repo( - &store, - &app_handle, - &branch_id, - Some(&worktree_path), - ) - .await; }); // The reply is the agent's whole account of what just happened, so diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index ba69486ae..4f8fcc671 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -903,7 +903,6 @@ pub struct ActiveSessionInfo { pub branch_id: Option, pub session_type: Option, pub status: String, - pub is_auto_review: bool, } /// Project one session to branch/project context and session type. @@ -963,7 +962,6 @@ fn project_active_session(store: &Store, session: &store::Session) -> ActiveSess branch_id, session_type, status: session.status.as_str().to_string(), - is_auto_review: linked_review.map(|review| review.is_auto).unwrap_or(false), } } @@ -1105,7 +1103,6 @@ pub async fn start_session( remote_working_dir: None, image_ids: vec![], queued_message_id: None, - pending_auto_review_branch_id: None, acp_config_selection, branch_id: None, project_id: None, @@ -1156,7 +1153,6 @@ pub async fn resume_session( branch_id, acp_config_selection, None, - None, ) .await } @@ -1174,7 +1170,6 @@ pub(crate) async fn resume_session_for_store( branch_id: Option, acp_config_selection: Option, queued_message_id: Option, - pending_auto_review_branch_id: Option, ) -> Result<(), String> { let session = store .get_session(&session_id) @@ -1323,7 +1318,6 @@ pub(crate) async fn resume_session_for_store( branch_id: event_branch_id, project_id: event_project_id.or(mcp_project_id.clone()), session_type, - is_auto_review: false, }, ); @@ -1364,7 +1358,6 @@ pub(crate) async fn resume_session_for_store( remote_working_dir, image_ids: image_ids.unwrap_or_default(), queued_message_id, - pending_auto_review_branch_id, acp_config_selection: effective_acp_config_selection, branch_id: config_branch_id, project_id: config_project_id, @@ -1467,7 +1460,6 @@ pub(crate) async fn send_queued_session_message_for_store( message.branch_id.clone(), None, Some(message.id.clone()), - None, ) .await; @@ -1486,7 +1478,6 @@ pub(crate) async fn drain_queued_message_for_session( action_registry: Arc, app_handle: tauri::AppHandle, session_id: String, - pending_auto_review_branch_id: Option, ) -> Result { let Some(message) = store .claim_oldest_queued_session_message(&session_id) @@ -1507,7 +1498,6 @@ pub(crate) async fn drain_queued_message_for_session( message.branch_id.clone(), None, Some(message.id.clone()), - pending_auto_review_branch_id, ) .await; @@ -1649,7 +1639,6 @@ pub fn cancel_session( branch_id, project_id, session_type: None, - is_auto_review: false, }, ); } @@ -1882,7 +1871,6 @@ impl BranchSessionScheduleKind { struct BranchSessionSchedule { kind: BranchSessionScheduleKind, review_id: Option, - blocks_queue: bool, } fn can_start_with_active_branch_sessions( @@ -1904,7 +1892,6 @@ fn note_session_schedule() -> BranchSessionSchedule { BranchSessionSchedule { kind: BranchSessionScheduleKind::Note, review_id: None, - blocks_queue: true, } } @@ -1912,17 +1899,15 @@ fn review_session_schedule(review: &store::Review) -> BranchSessionSchedule { BranchSessionSchedule { kind: BranchSessionScheduleKind::Review, review_id: Some(review.id.clone()), - blocks_queue: !review.is_auto, } } /// Schedule for the kinds that take the branch exclusively (commit sessions and -/// command pipelines): they always block the queue and carry no review. +/// command pipelines): they carry no review. fn exclusive_session_schedule(kind: BranchSessionScheduleKind) -> BranchSessionSchedule { BranchSessionSchedule { kind, review_id: None, - blocks_queue: true, } } @@ -2023,9 +2008,7 @@ fn running_branch_session_kinds( for session in running { if let Some(schedule) = resolve_branch_session_schedule(store, branch_id, &session, false)? { - if schedule.blocks_queue { - active.insert(schedule.kind); - } + active.insert(schedule.kind); } } Ok(active) @@ -2045,16 +2028,14 @@ pub(crate) fn branch_session_launch_lock_for(branch_id: &str) -> Arc> ) } -fn has_queued_user_branch_session(store: &Store, branch_id: &str) -> Result { +fn has_queued_branch_session(store: &Store, branch_id: &str) -> Result { let queued = store .get_queued_sessions_for_branch(branch_id) .map_err(|e| e.to_string())?; for session in queued { - if let Some(schedule) = resolve_branch_session_schedule(store, branch_id, &session, true)? { - if schedule.blocks_queue { - return Ok(true); - } + if resolve_branch_session_schedule(store, branch_id, &session, true)?.is_some() { + return Ok(true); } } @@ -2090,7 +2071,7 @@ fn should_queue_branch_session_start( return Ok(true); } - if has_queued_user_branch_session(store, branch_id)? { + if has_queued_branch_session(store, branch_id)? { return Ok(true); } @@ -2113,9 +2094,7 @@ fn drainable_session_ids_for_active_set( } drainable.push(session_id.clone()); - if schedule.blocks_queue { - active.insert(schedule.kind); - } + active.insert(schedule.kind); } drainable } @@ -2339,7 +2318,6 @@ pub async fn start_project_session( remote_working_dir: None, image_ids: image_ids.unwrap_or_default(), queued_message_id: None, - pending_auto_review_branch_id: None, acp_config_selection, branch_id: None, project_id: Some(project_id), @@ -2737,7 +2715,6 @@ fn launch_running_branch_session( remote_working_dir: prepared.remote_working_dir, image_ids, queued_message_id: None, - pending_auto_review_branch_id: None, acp_config_selection: acp_config_selection_for_session_start(&created.session), branch_id: Some(branch_id), project_id: Some(project_id), @@ -2771,13 +2748,6 @@ pub async fn start_or_queue_branch_session_for_store( ) -> Result { let image_ids = image_ids.unwrap_or_default(); - if matches!( - session_type, - BranchSessionType::Commit | BranchSessionType::Review - ) { - cancel_in_flight_auto_review_for_branch(&store, ®istry, &branch_id)?; - } - let provider = resolve_branch_session_provider(&store, &branch_id, &session_type, provider)?; let launch_lock = branch_session_launch_lock_for(&branch_id); @@ -2831,7 +2801,6 @@ pub async fn start_or_queue_branch_session_for_store( #[allow(clippy::too_many_arguments)] pub fn queue_branch_session_for_store( store: Arc, - registry: Arc, branch_id: String, prompt: String, session_type: BranchSessionType, @@ -2842,13 +2811,6 @@ pub fn queue_branch_session_for_store( ) -> Result { let image_ids = image_ids.unwrap_or_default(); - if matches!( - session_type, - BranchSessionType::Commit | BranchSessionType::Review - ) { - cancel_in_flight_auto_review_for_branch(&store, ®istry, &branch_id)?; - } - let provider = resolve_branch_session_provider(&store, &branch_id, &session_type, provider)?; let launch_lock = branch_session_launch_lock_for(&branch_id); let _guard = launch_lock.lock().unwrap(); @@ -2941,7 +2903,6 @@ pub async fn start_or_queue_branch_session( #[allow(clippy::too_many_arguments)] pub fn queue_branch_session( store: tauri::State<'_, Mutex>>>, - registry: tauri::State<'_, Arc>, branch_id: String, prompt: String, session_type: BranchSessionType, @@ -2953,7 +2914,6 @@ pub fn queue_branch_session( let store = get_store(&store)?; queue_branch_session_for_store( store, - Arc::clone(®istry), branch_id, prompt, session_type, @@ -3030,9 +2990,7 @@ pub async fn drain_queued_sessions_for_branch( if started { started_any = true; - if schedule.blocks_queue { - active.insert(schedule.kind); - } + active.insert(schedule.kind); } else { active = running_branch_session_kinds(&store, &branch_id)?; } @@ -3257,7 +3215,6 @@ async fn start_queued_session_for_branch( branch_id: Some(branch_id.clone()), project_id: Some(branch.project_id.clone()), session_type: Some(session_type_str.to_string()), - is_auto_review: false, }, ); @@ -3277,7 +3234,6 @@ async fn start_queued_session_for_branch( remote_working_dir, image_ids, queued_message_id: None, - pending_auto_review_branch_id: None, acp_config_selection: acp_config_selection_for_session_start(&session), branch_id: Some(branch_id), project_id: Some(branch.project_id.clone()), @@ -3296,7 +3252,7 @@ async fn start_queued_session_for_branch( } // ============================================================================= -// Auto review commands +// Review provider resolution // ============================================================================= /// Agents known to be available on remote Blox workstations. @@ -3501,312 +3457,6 @@ fn resolve_inherited_provider_from_ids( } } -/// Core logic for starting an automatic review for a branch. -/// -/// Creates a review with `is_auto = true`, starts a session, and emits -/// `session-status-changed` with `isAutoReview: true` so the frontend -/// can track it. -/// -/// When `provider` is `None`, resolves the user's current preferred agent -/// from persisted preferences so that auto-reviews match what the user -/// would get if they clicked "Review" manually. -/// -/// This is called both from the Tauri command and from the session runner -/// when a commit session completes. -pub async fn trigger_auto_review( - store: Arc, - registry: Arc, - app_handle: tauri::AppHandle, - branch_id: String, - provider: Option, -) -> Result { - // Resolve branch → project - let branch = store - .get_branch(&branch_id) - .map_err(|e| e.to_string())? - .ok_or_else(|| format!("Branch not found: {branch_id}"))?; - - let project = store - .get_project(&branch.project_id) - .map_err(|e| e.to_string())? - .ok_or_else(|| format!("Project not found: {}", branch.project_id))?; - - let is_remote = branch.workspace_name.is_some(); - - // Resolve the provider before inserting session/review rows. Auto reviews - // should only create agent-backed records when the provider is concrete. - let provider_was_explicit = provider.is_some(); - let provider = resolve_review_provider(provider, is_remote).map_err(|e| { - log::warn!("[auto_review] no provider available for branch {branch_id}: {e}"); - e - })?; - if !provider_was_explicit { - log::info!("[auto_review] resolved preferred provider: {provider}"); - } - - // Resolve working directory and branch context. - let (working_dir, branch_context) = if is_remote { - let fallback_dir = resolve_branch_repo_slug(&store, &project, &branch) - .and_then(|repo| crate::paths::repos_dir().map(|d| d.join(repo))) - .unwrap_or_else(|| PathBuf::from("/tmp")); - let workspace_name = branch.workspace_name.as_deref().unwrap().to_string(); - let base_branch = branch.base_branch.clone(); - let store_for_context = Arc::clone(&store); - let branch_id_for_context = branch_id.clone(); - let project_id_for_context = branch.project_id.clone(); - let remote_context = tauri::async_runtime::spawn_blocking(move || { - build_remote_branch_context( - &workspace_name, - &base_branch, - &store_for_context, - &branch_id_for_context, - &project_id_for_context, - RemotePikchrGrammarStaging::NotNeeded, - ) - }) - .await - .map_err(|e| format!("Failed to build remote branch context: {e}"))?; - (fallback_dir, remote_context.branch_context) - } else { - let workdir = store - .get_workdir_for_branch(&branch_id) - .map_err(|e| e.to_string())? - .ok_or_else(|| format!("No worktree for branch: {branch_id}"))?; - - let mut worktree_path = PathBuf::from(&workdir.path); - let effective_subpath = if let Some(repo_id) = branch.project_repo_id.as_deref() { - store - .get_project_repo(repo_id) - .ok() - .flatten() - .and_then(|repo| repo.subpath) - } else { - project.subpath.clone() - }; - if let Some(ref subpath) = effective_subpath { - worktree_path = worktree_path.join(subpath); - } - - let ctx = build_branch_context( - &worktree_path, - &branch.base_branch, - &store, - &branch_id, - &branch.project_id, - ); - (worktree_path, ctx) - }; - - // Get the current tip SHA for the review anchor - let tip_sha = review_tip_sha(&store, &branch, &working_dir).await?; - - // Build the full prompt (reuse Review prompt) - let prompt = "Review the latest changes on this branch.".to_string(); - let project_information = build_project_context(&store, &project, &branch); - let full_prompt = build_full_prompt( - &prompt, - &project_information, - &branch_context, - &BranchSessionType::Review, - None, - Some(&branch.base_branch), - ); - - // Create the session - let session = store::Session::new_running(&full_prompt, &working_dir).with_provider(&provider); - store.create_session(&session).map_err(|e| e.to_string())?; - - // Create auto review - let review = store::Review::new(&branch_id, &tip_sha, store::ReviewScope::Branch) - .with_session(&session.id) - .with_auto(); - store.create_review(&review).map_err(|e| e.to_string())?; - - // Emit session-status-changed with isAutoReview: true - crate::web_server::emit_to_all( - &app_handle, - "session-status-changed", - session_runner::SessionStatusEvent { - session_id: session.id.clone(), - status: "running".to_string(), - error_message: None, - completion_reason: None, - branch_id: Some(branch_id.clone()), - project_id: Some(branch.project_id.clone()), - session_type: Some("review".to_string()), - is_auto_review: true, - }, - ); - - // Resolve the remote working dir for remote branches - let remote_working_dir = if is_remote { - let ws_name = branch.workspace_name.as_deref().unwrap().to_string(); - let store_for_resolve = Arc::clone(&store); - let branch_for_resolve = branch.clone(); - match tauri::async_runtime::spawn_blocking(move || { - crate::branches::resolve_branch_workspace_subpath( - &store_for_resolve, - &branch_for_resolve, - ) - .ok() - .flatten() - .and_then(|subpath| { - crate::branches::resolve_workspace_repo_path(&ws_name, &subpath).ok() - }) - }) - .await - { - Ok(Some(path)) => Some(PathBuf::from(path)), - _ => None, - } - } else { - None - }; - - session_runner::start_session( - SessionConfig { - session_id: session.id.clone(), - prompt: full_prompt, - working_dir, - agent_session_id: None, - pre_head_sha: None, - provider: Some(provider), - workspace_name: branch.workspace_name.clone(), - extra_env: vec![], - mcp_project_id: None, - action_executor: None, - action_registry: None, - remote_working_dir, - image_ids: vec![], - queued_message_id: None, - pending_auto_review_branch_id: None, - acp_config_selection: None, - branch_id: Some(branch_id.clone()), - project_id: Some(branch.project_id.clone()), - // Auto-review sessions don't write notes. - expose_pikchr_tools: false, - parent_project_note_id: None, - }, - store, - app_handle, - Arc::clone(®istry), - )?; - - Ok(BranchSessionResponse { - session_id: session.id, - artifact_id: review.id, - session_status: BranchSessionLaunchStatus::Running, - }) -} - -/// Resolve the latest git committer timestamp (in milliseconds) for a -/// branch by querying the actual git log. This covers commits made -/// outside the app that are absent from the `commits` table. -/// -/// Returns `0` when the branch has no worktree, no commits, or when the -/// git query fails — callers fall back to the DB-only comparison in that -/// case. -fn latest_git_commit_ms(store: &Arc, branch_id: &str) -> i64 { - let branch = match store.get_branch(branch_id) { - Ok(Some(b)) => b, - _ => return 0, - }; - let workdir = match store.get_workdir_for_branch(branch_id) { - Ok(Some(w)) => w, - _ => return 0, - }; - let worktree_path = std::path::Path::new(&workdir.path); - if !worktree_path.exists() { - return 0; - } - let base_ref = git::origin_ref_for_branch(&branch.base_branch); - let commits = match git::get_commits_since_base(worktree_path, &base_ref) { - Ok(c) => c, - Err(_) => return 0, - }; - // `timestamp` is committer time, not the author time the timeline sorts - // on: a rebase *should* read as new activity here. In seconds, so convert - // to milliseconds. - commits.iter().map(|c| c.timestamp).max().unwrap_or(0) * 1000 -} - -pub(crate) fn cancel_in_flight_auto_review_for_branch( - store: &Arc, - registry: &session_runner::SessionRegistry, - branch_id: &str, -) -> Result { - let git_ts = latest_git_commit_ms(store, branch_id); - let Some(review) = store - .find_fresh_auto_review(branch_id, git_ts) - .map_err(|e| e.to_string())? - else { - return Ok(false); - }; - - let Some(session_id) = review.session_id.as_deref() else { - return Ok(false); - }; - - let Some(session) = store.get_session(session_id).map_err(|e| e.to_string())? else { - return Ok(false); - }; - - if !matches!( - session.status, - store::SessionStatus::Running | store::SessionStatus::Queued - ) { - return Ok(false); - } - - registry.cancel(session_id); - let cancelled = store - .transition_from_active( - session_id, - store::SessionStatus::Cancelled, - None, - Some(&store::CompletionReason::Interrupted), - ) - .map_err(|e| e.to_string())?; - if !cancelled { - let current = store.get_session(session_id).map_err(|e| e.to_string())?; - return match current.map(|session| session.status) { - None | Some(store::SessionStatus::Cancelled) => Ok(true), - _ => Ok(false), - }; - } - - Ok(true) -} - -/// Find an auto review created after all commits on a branch. -#[tauri::command(rename_all = "camelCase")] -pub async fn find_fresh_auto_review( - store: tauri::State<'_, Mutex>>>, - branch_id: String, -) -> Result, String> { - let store = get_store(&store)?; - tauri::async_runtime::spawn_blocking(move || { - let git_ts = latest_git_commit_ms(&store, &branch_id); - store - .find_fresh_auto_review(&branch_id, git_ts) - .map_err(|e| e.to_string()) - }) - .await - .map_err(|e| e.to_string())? -} - -/// Update the `is_auto` flag on a review. -#[tauri::command(rename_all = "camelCase")] -pub fn set_review_auto( - store: tauri::State<'_, Mutex>>>, - review_id: String, - is_auto: bool, -) -> Result<(), String> { - get_store(&store)? - .set_review_auto(&review_id, is_auto) - .map_err(|e| e.to_string()) -} - // ============================================================================= // Prompt construction helpers // ============================================================================= @@ -4767,9 +4417,6 @@ fn review_timeline_entries( let mut entries = Vec::new(); for review in &reviews { - if review.is_auto { - continue; - } // Hide reviews whose originating commit is no longer on the branch, // mirroring the branch card timeline so the agent never sees a review // the user can't see in the UI. @@ -4892,27 +4539,6 @@ fn shell_quote_arg(value: &str) -> String { format!("'{}'", value.replace('\'', "'\\''")) } -/// Assemble the full prompt from action instructions + branch context + user prompt. -pub(crate) fn build_full_prompt( - user_prompt: &str, - project_information: &str, - branch_context: &str, - session_type: &BranchSessionType, - launch_context: Option<&BranchSessionLaunchContext>, - base_branch: Option<&str>, -) -> String { - build_full_prompt_with_pikchr_reference( - user_prompt, - project_information, - branch_context, - session_type, - launch_context, - base_branch, - PIKCHR_GRAMMAR_URL, - false, - ) -} - #[allow(clippy::too_many_arguments)] pub(crate) fn build_full_prompt_with_pikchr_reference( user_prompt: &str, @@ -5171,6 +4797,29 @@ mod tests { use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; + /// Calls [`build_full_prompt_with_pikchr_reference`] with the default pikchr + /// grammar reference and no pikchr tools. Production callers thread both + /// through explicitly; only these tests want the defaults. + fn build_full_prompt( + user_prompt: &str, + project_information: &str, + branch_context: &str, + session_type: &BranchSessionType, + launch_context: Option<&BranchSessionLaunchContext>, + base_branch: Option<&str>, + ) -> String { + build_full_prompt_with_pikchr_reference( + user_prompt, + project_information, + branch_context, + session_type, + launch_context, + base_branch, + PIKCHR_GRAMMAR_URL, + false, + ) + } + fn setup_branch_store() -> (Arc, store::Branch) { let store = Arc::new(Store::in_memory().unwrap()); let project = store::Project::new("test-owner/test-repo"); @@ -5283,35 +4932,6 @@ mod tests { ); } - fn create_auto_review( - store: &Arc, - branch_id: &str, - status: store::SessionStatus, - ) -> (store::Session, store::Review) { - let session = match status { - store::SessionStatus::Running => { - store::Session::new_running("auto review", Path::new("/tmp")) - } - store::SessionStatus::Queued => store::Session::new_queued("auto review"), - store::SessionStatus::Completed => { - store::Session::new_running("auto review", Path::new("/tmp")) - } - other => panic!("unsupported auto review test status: {}", other.as_str()), - }; - store.create_session(&session).unwrap(); - if status != store::SessionStatus::Running && status != store::SessionStatus::Queued { - store - .update_session_status(&session.id, status, None, None) - .unwrap(); - } - - let review = store::Review::new(branch_id, "abc123", store::ReviewScope::Branch) - .with_session(&session.id) - .with_auto(); - store.create_review(&review).unwrap(); - (session, review) - } - fn create_session_with_status( store: &Arc, prompt: &str, @@ -5399,7 +5019,6 @@ mod tests { BranchSessionSchedule { kind, review_id: None, - blocks_queue: true, } } @@ -5500,7 +5119,6 @@ mod tests { ); assert_eq!(commit_info.session_type.as_deref(), Some("commit")); assert_eq!(commit_info.status, "running"); - assert!(!commit_info.is_auto_review); let note_info = info_for(¬e_session.id); assert_eq!(note_info.session_type.as_deref(), Some("note")); @@ -5508,20 +5126,6 @@ mod tests { let review_info = info_for(&review_session.id); assert_eq!(review_info.session_type.as_deref(), Some("review")); - assert!(!review_info.is_auto_review); - } - - #[test] - fn active_sessions_snapshot_marks_auto_reviews() { - let (store, branch) = setup_branch_store(); - let (session, _review) = - create_auto_review(&store, &branch.id, store::SessionStatus::Running); - - let snapshot = get_active_sessions_impl(&store).unwrap(); - assert_eq!(snapshot.len(), 1); - assert_eq!(snapshot[0].session_id, session.id); - assert_eq!(snapshot[0].session_type.as_deref(), Some("review")); - assert!(snapshot[0].is_auto_review); } #[test] @@ -5613,7 +5217,6 @@ mod tests { Some(branch.project_id.as_str()) ); assert_eq!(snapshot[0].session_type.as_deref(), Some("pr")); - assert!(!snapshot[0].is_auto_review); } #[test] @@ -5912,42 +5515,9 @@ mod tests { assert_eq!(drainable, vec!["note-1".to_string(), "note-2".to_string()]); } - #[test] - fn running_auto_review_does_not_block_queued_user_sessions() { - let (store, branch) = setup_branch_store(); - create_auto_review(&store, &branch.id, store::SessionStatus::Running); - - let active = running_branch_session_kinds(&store, &branch.id).unwrap(); - - assert!(active.is_empty()); - for kind in [ - BranchSessionScheduleKind::Note, - BranchSessionScheduleKind::Review, - BranchSessionScheduleKind::Commit, - ] { - assert!(can_start_with_active_branch_sessions(kind, &active)); - } - } - - #[test] - fn branch_start_decision_ignores_auto_review_barriers() { - let (store, branch) = setup_branch_store_with_workdir(); - create_auto_review(&store, &branch.id, store::SessionStatus::Running); - create_auto_review(&store, &branch.id, store::SessionStatus::Queued); - - for session_type in [ - BranchSessionType::Note, - BranchSessionType::Review, - BranchSessionType::Commit, - ] { - assert!(!should_queue_branch_session_start(&store, &branch.id, &session_type).unwrap()); - } - } - #[test] fn explicit_queue_response_reports_queued_status_and_stores_acp_config_selection() { let (store, branch) = setup_branch_store(); - let registry = Arc::new(session_runner::SessionRegistry::new()); let selection = store::AcpConfigSelection { model: Some(store::AcpConfigValueSelection { config_id: "model".to_string(), @@ -5963,7 +5533,6 @@ mod tests { let response = queue_branch_session_for_store( Arc::clone(&store), - registry, branch.id.clone(), "capture a note".to_string(), BranchSessionType::Note, @@ -7383,57 +6952,6 @@ mod tests { let _ = std::fs::remove_file(&temp_path); } - #[test] - fn cancel_in_flight_auto_review_cancels_running_review() { - let (store, branch) = setup_branch_store(); - let (session, review) = - create_auto_review(&store, &branch.id, store::SessionStatus::Running); - let registry = session_runner::SessionRegistry::new(); - - let cancelled = - cancel_in_flight_auto_review_for_branch(&store, ®istry, &branch.id).unwrap(); - - assert!(cancelled); - // Session transitions to Cancelled but both records survive for potential adoption - let session = store.get_session(&session.id).unwrap().unwrap(); - assert_eq!(session.status, store::SessionStatus::Cancelled); - assert!(store.get_review(&review.id).unwrap().is_some()); - } - - #[test] - fn cancel_in_flight_auto_review_cancels_queued_review() { - let (store, branch) = setup_branch_store(); - let (session, review) = - create_auto_review(&store, &branch.id, store::SessionStatus::Queued); - let registry = session_runner::SessionRegistry::new(); - - let cancelled = - cancel_in_flight_auto_review_for_branch(&store, ®istry, &branch.id).unwrap(); - - assert!(cancelled); - // Session transitions to Cancelled but both records survive for potential adoption - let session = store.get_session(&session.id).unwrap().unwrap(); - assert_eq!(session.status, store::SessionStatus::Cancelled); - assert!(store.get_review(&review.id).unwrap().is_some()); - } - - #[test] - fn cancel_in_flight_auto_review_leaves_completed_review_available_for_adoption() { - let (store, branch) = setup_branch_store(); - let (session, review) = - create_auto_review(&store, &branch.id, store::SessionStatus::Completed); - let registry = session_runner::SessionRegistry::new(); - - let cancelled = - cancel_in_flight_auto_review_for_branch(&store, ®istry, &branch.id).unwrap(); - - assert!(!cancelled); - let session = store.get_session(&session.id).unwrap().unwrap(); - assert_eq!(session.status, store::SessionStatus::Completed); - let review = store.get_review(&review.id).unwrap().unwrap(); - assert!(review.is_auto); - } - #[test] fn commit_prompt_appends_diff_viewer_context_to_branch_history() { let prompt = build_full_prompt( diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index 7e1cfdc97..1ac8c0e40 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -129,10 +129,6 @@ pub struct SessionStatusEvent { pub branch_id: Option, pub project_id: Option, pub session_type: Option, - /// When `true`, the session belongs to an automatically triggered review - /// (not user-initiated). The frontend uses this to suppress UI for auto reviews. - #[serde(default)] - pub is_auto_review: bool, } // ============================================================================= @@ -374,8 +370,6 @@ pub struct SessionConfig { pub image_ids: Vec, /// Queued follow-up row that produced this run, if any. pub queued_message_id: Option, - /// Branch with a commit waiting for auto-review once queued follow-ups drain. - pub pending_auto_review_branch_id: Option, /// Selected ACP config values to apply after session setup and before the /// prompt. Command handlers also store successful selections on the session /// row so queued and resumed sessions use their own selection. @@ -835,8 +829,7 @@ pub fn start_session( // Run post-completion hooks before transitioning status. // These detect artifacts produced by the session (commits, notes). - // Returns the branch_id when a new commit was detected. - let committed_branch_id = if completed_successfully { + if completed_successfully { run_post_completion_hooks( &config.session_id, &config.working_dir, @@ -847,10 +840,8 @@ pub fn start_session( .as_deref() .and_then(|dir| dir.to_str()), &store_for_status, - ) - } else { - None - }; + ); + } let status_enum = SessionStatus::parse(new_status).unwrap(); let transitioned = store_for_status @@ -891,11 +882,6 @@ pub fn start_session( if transitioned { let branch_id = config.branch_id.clone(); - let auto_review_branch_id = auto_review_branch_id_for_terminal_state( - committed_branch_id.clone(), - config.pending_auto_review_branch_id.clone(), - completed_successfully, - ); let should_drain_queued_message = completed_successfully; let session_id_for_follow_up = session_id_for_status.clone(); let action_executor_for_follow_up = config @@ -912,7 +898,6 @@ pub fn start_session( let registry_for_follow_up = Arc::clone(®istry); let app_handle_for_follow_up = app_handle.clone(); tauri::async_runtime::spawn(async move { - let mut queued_message_blocks_auto_review = false; if should_drain_queued_message { match crate::session_commands::drain_queued_message_for_session( Arc::clone(&store_for_follow_up), @@ -921,19 +906,16 @@ pub fn start_session( Arc::clone(&action_registry_for_follow_up), app_handle_for_follow_up.clone(), session_id_for_follow_up.clone(), - auto_review_branch_id.clone(), ) .await { Ok(true) => { - queued_message_blocks_auto_review = true; log::info!( "Drained queued follow-up message for session {session_id_for_follow_up}" ); } Ok(false) => {} Err(e) => { - queued_message_blocks_auto_review = true; log::error!( "Failed to drain queued follow-up message for session {session_id_for_follow_up}: {e}" ); @@ -954,52 +936,6 @@ pub fn start_session( Ok(true) => { log::info!("Drained next queued session for branch {branch_id}"); } - Ok(false) if !queued_message_blocks_auto_review => { - // Check if auto-review is enabled in user preferences - let auto_review_enabled = crate::preferences_store_path_buf() - .and_then(|path| std::fs::read_to_string(&path).ok()) - .and_then(|contents| { - serde_json::from_str::(&contents).ok() - }) - .and_then(|json| { - json.get("auto-start-code-reviews")? - .as_str() - .map(String::from) - }) - .map(|mode| mode != "never") - .unwrap_or_else(crate::blox::is_sq_available); - - if let Some(auto_review_branch_id) = - auto_review_branch_id.filter(|_| auto_review_enabled) - { - // Pass None so trigger_auto_review resolves - // the user's current preferred agent at - // trigger time, rather than reusing the - // (possibly stale) commit session provider. - match crate::session_commands::trigger_auto_review( - store_for_follow_up, - registry_for_follow_up, - app_handle_for_follow_up, - auto_review_branch_id.clone(), - None, - ) - .await - { - Ok(resp) => { - log::info!( - "Auto review triggered for branch {auto_review_branch_id}: session={}, review={}", - resp.session_id, - resp.artifact_id, - ); - } - Err(e) => { - log::error!( - "Failed to trigger auto review for branch {auto_review_branch_id}: {e}" - ); - } - } - } - } Ok(false) => {} Err(e) => { log::error!( @@ -1286,7 +1222,6 @@ pub fn start_pipeline_session( remote_working_dir: config.remote_working_dir.clone(), image_ids: vec![], queued_message_id: None, - pending_auto_review_branch_id: None, acp_config_selection: None, branch_id: config.branch_id.clone(), project_id: config.project_id.clone(), @@ -1721,7 +1656,6 @@ fn drain_queued_after_pipeline_terminal( Arc::new(ActionRegistry::new()), app_handle.clone(), session_id.clone(), - None, ) .await { @@ -2487,9 +2421,6 @@ pub(crate) fn is_process_alive(pid: u32) -> bool { /// For remote workspaces, HEAD is checked via `blox ws_exec`. /// - **Notes**: If an empty note is linked to this session, parse the /// assistant's last message for content after the first `---`. -/// -/// Returns the `branch_id` when a new commit was successfully detected, -/// so the caller can trigger follow-up work (e.g. auto review). fn run_post_completion_hooks( session_id: &str, working_dir: &std::path::Path, @@ -2497,9 +2428,7 @@ fn run_post_completion_hooks( workspace_name: Option<&str>, remote_working_dir: Option<&str>, store: &Arc, -) -> Option { - let mut committed_branch_id: Option = None; - +) { // --- Commit detection --- if let Some(pre_sha) = pre_head_sha { // Look for any commit linked to this session — not just pending (sha IS NULL) @@ -2524,15 +2453,15 @@ fn run_post_completion_hooks( // with the rebase still stopped on a conflict, HEAD is // detached on a partially applied commit — a SHA `git // rebase --abort` erases — so nothing may claim it: not - // the pending row, not an amend, and no auto-review via - // `committed_branch_id`. Skip the whole arm; the rows - // self-resolve on a later turn, because resumed sessions - // re-capture `pre_head_sha` and land back here once HEAD - // is attached again (after `--continue` finishes or - // `--abort` restores, reassociation plus the duplicate-SHA - // branch of `complete_pending_commit_sha` settle every - // row), while a turn that never comes leaves the pending - // row `sha IS NULL` — an ordinary failed commit attempt. + // the pending row, not an amend, and no diff cache keyed + // on it. Skip the whole arm; the rows self-resolve on a + // later turn, because resumed sessions re-capture + // `pre_head_sha` and land back here once HEAD is attached + // again (after `--continue` finishes or `--abort` + // restores, reassociation plus the duplicate-SHA branch of + // `complete_pending_commit_sha` settle every row), while a + // turn that never comes leaves the pending row + // `sha IS NULL` — an ordinary failed commit attempt. // // Asking is the same call that does the work: the rebase // rewrote SHAs just the same as the no-AI path, and @@ -2596,8 +2525,6 @@ fn run_post_completion_hooks( }; if recorded { - committed_branch_id = Some(commit.branch_id.clone()); - // Spawn background diff caching for remote branches. if let Some(ws_name) = workspace_name { let commit_shas: Vec = store @@ -2814,8 +2741,6 @@ fn run_post_completion_hooks( } } } - - committed_branch_id } fn terminal_state_completed_successfully( @@ -2826,18 +2751,6 @@ fn terminal_state_completed_successfully( && *completion_reason == CompletionReason::TurnComplete } -fn auto_review_branch_id_for_terminal_state( - committed_branch_id: Option, - pending_auto_review_branch_id: Option, - completed_successfully: bool, -) -> Option { - committed_branch_id.or_else(|| { - completed_successfully - .then_some(pending_auto_review_branch_id) - .flatten() - }) -} - /// Extract note content from a single assistant message. /// /// Callers are responsible for choosing which message to pass — typically the @@ -3307,7 +3220,6 @@ fn emit_status( branch_id, project_id, session_type: None, - is_auto_review: false, }; crate::web_server::emit_to_all(app_handle, "session-status-changed", &event); } @@ -3331,7 +3243,6 @@ pub fn emit_session_running( branch_id: Some(branch_id.to_string()), project_id: Some(project_id.to_string()), session_type: Some(session_type.to_string()), - is_auto_review: false, }; crate::web_server::emit_to_all(app_handle, "session-status-changed", &event); } @@ -3353,34 +3264,6 @@ mod tests { } } - #[test] - fn terminal_auto_review_reuses_pending_branch_only_after_successful_turn() { - assert_eq!( - auto_review_branch_id_for_terminal_state( - None, - Some("branch-pending".to_string()), - true, - ), - Some("branch-pending".to_string()) - ); - assert_eq!( - auto_review_branch_id_for_terminal_state( - None, - Some("branch-pending".to_string()), - false, - ), - None - ); - assert_eq!( - auto_review_branch_id_for_terminal_state( - Some("branch-commit".to_string()), - Some("branch-pending".to_string()), - false, - ), - Some("branch-commit".to_string()) - ); - } - #[test] fn pikchr_validation_skips_when_latest_message_is_not_assistant() { let messages = vec![session_message(MessageRole::User, "Thanks")]; @@ -4210,9 +4093,7 @@ mod tests { } /// End the handoff turn with the rebase still stopped on the conflict. - /// Returns the hooks' `committed_branch_id`, which must be `None` — a - /// mid-rebase state must not trigger the auto-review follow-up. - fn end_turn_mid_rebase(fixture: &ConflictedRebase) -> Option { + fn end_turn_mid_rebase(fixture: &ConflictedRebase) { run_post_completion_hooks( &fixture.rebase_session_id, fixture.repo.path(), @@ -4220,7 +4101,22 @@ mod tests { None, None, &fixture.store, - ) + ); + } + + /// A mid-rebase state must not be claimed as a completed commit: the + /// rebase session's pending row has to come out of the turn still + /// `sha IS NULL`. + fn assert_pending_unclaimed(fixture: &ConflictedRebase) { + let pending = fixture + .store + .get_commit(&fixture.pending_id) + .unwrap() + .unwrap(); + assert!( + pending.sha.is_none(), + "the pending row must not claim the detached mid-rebase SHA" + ); } fn assert_untouched(fixture: &ConflictedRebase) { @@ -4264,18 +4160,10 @@ mod tests { fn rebase_stopped_on_a_conflict_leaves_the_rows_alone() { let fixture = conflicted_rebase(); - assert!(end_turn_mid_rebase(&fixture).is_none()); + end_turn_mid_rebase(&fixture); assert_untouched(&fixture); - let pending = fixture - .store - .get_commit(&fixture.pending_id) - .unwrap() - .unwrap(); - assert!( - pending.sha.is_none(), - "the pending row must not claim the detached mid-rebase SHA" - ); + assert_pending_unclaimed(&fixture); } /// The deferred pending row resolves on the next turn: the resumed session @@ -4286,7 +4174,8 @@ mod tests { #[test] fn rebase_resumed_and_finished_resolves_the_deferred_pending_row() { let fixture = conflicted_rebase(); - assert!(end_turn_mid_rebase(&fixture).is_none()); + end_turn_mid_rebase(&fixture); + assert_pending_unclaimed(&fixture); let repo = &fixture.repo; let detached_head = repo.run_git(&["rev-parse", "HEAD"]).trim().to_string(); @@ -4336,7 +4225,8 @@ mod tests { #[test] fn rebase_resumed_and_aborted_drops_the_deferred_pending_row() { let fixture = conflicted_rebase(); - assert!(end_turn_mid_rebase(&fixture).is_none()); + end_turn_mid_rebase(&fixture); + assert_pending_unclaimed(&fixture); let repo = &fixture.repo; let detached_head = repo.run_git(&["rev-parse", "HEAD"]).trim().to_string(); diff --git a/apps/staged/src-tauri/src/store/migration_tests.rs b/apps/staged/src-tauri/src/store/migration_tests.rs index 93aac1a50..57a61730c 100644 --- a/apps/staged/src-tauri/src/store/migration_tests.rs +++ b/apps/staged/src-tauri/src/store/migration_tests.rs @@ -145,7 +145,7 @@ fn test_store_bootstraps_fresh_database_with_baseline_migration() { ) .unwrap(); - assert_eq!(version, 25); + assert_eq!(version, 26); assert_eq!(app_version, super::APP_VERSION); assert!(table_exists(&conn, "projects")); assert!(table_exists(&conn, "project_notes")); @@ -163,6 +163,7 @@ fn test_store_bootstraps_fresh_database_with_baseline_migration() { assert!(column_exists(&conn, "sessions", "branch_id")); assert!(column_exists(&conn, "sessions", "completion_effects_at")); assert!(column_exists(&conn, "notes", "parent_project_note_id")); + assert!(!column_exists(&conn, "reviews", "is_auto")); let trigger_count: i64 = conn .query_row( @@ -207,6 +208,11 @@ fn test_store_repairs_github_comment_tracking_user_version() { image_ids TEXT DEFAULT NULL ); CREATE TABLE notes (id TEXT PRIMARY KEY); + -- Only the table/column the 0026 auto-review cleanup targets. + CREATE TABLE reviews ( + id TEXT PRIMARY KEY, + is_auto INTEGER NOT NULL DEFAULT 0 + ); CREATE TABLE repo_badges ( github_repo TEXT NOT NULL, subpath TEXT NOT NULL DEFAULT '', @@ -239,7 +245,7 @@ fn test_store_repairs_github_comment_tracking_user_version() { let version: i64 = conn .query_row("PRAGMA user_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(version, 25); + assert_eq!(version, 26); assert!(column_exists(&conn, "sessions", "pipeline")); assert!(column_exists(&conn, "sessions", "acp_config_selection")); assert!(column_exists(&conn, "sessions", "acp_title")); @@ -282,6 +288,11 @@ fn test_store_repairs_pipeline_user_version() { image_ids TEXT DEFAULT NULL ); CREATE TABLE notes (id TEXT PRIMARY KEY); + -- Only the table/column the 0026 auto-review cleanup targets. + CREATE TABLE reviews ( + id TEXT PRIMARY KEY, + is_auto INTEGER NOT NULL DEFAULT 0 + ); CREATE TABLE repo_badges ( github_repo TEXT NOT NULL, subpath TEXT NOT NULL DEFAULT '', @@ -309,7 +320,7 @@ fn test_store_repairs_pipeline_user_version() { let version: i64 = conn .query_row("PRAGMA user_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(version, 25); + assert_eq!(version, 26); assert!(column_exists(&conn, "comments", "github_comment_id")); assert!(column_exists(&conn, "comments", "github_comment_type")); assert!(column_exists(&conn, "comments", "github_comment_stale")); @@ -357,6 +368,11 @@ fn test_completion_effects_migration_backfills_finished_pipeline_sessions() { ); -- Only the table the 0025 column add targets. CREATE TABLE notes (id TEXT PRIMARY KEY); + -- Only the table/column the 0026 auto-review cleanup targets. + CREATE TABLE reviews ( + id TEXT PRIMARY KEY, + is_auto INTEGER NOT NULL DEFAULT 0 + ); ", ) .unwrap(); @@ -369,7 +385,7 @@ fn test_completion_effects_migration_backfills_finished_pipeline_sessions() { let version: i64 = conn .query_row("PRAGMA user_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(version, 25); + assert_eq!(version, 26); assert!(column_exists(&conn, "sessions", "completion_effects_at")); let marker = |id: &str| -> Option { @@ -392,6 +408,54 @@ fn test_completion_effects_migration_backfills_finished_pipeline_sessions() { cleanup_db(&path); } +#[test] +fn test_auto_review_removal_migration_deletes_auto_reviews_and_drops_flag() { + let path = temp_db_path("auto-review-removal"); + let conn = Connection::open(&path).unwrap(); + conn.execute_batch( + " + PRAGMA user_version = 25; + CREATE TABLE app_metadata ( + id INTEGER PRIMARY KEY CHECK (id = 1), + app_version TEXT NOT NULL + ); + INSERT INTO app_metadata (id, app_version) VALUES (1, '0.2.9'); + CREATE TABLE reviews ( + id TEXT PRIMARY KEY, + is_auto INTEGER NOT NULL DEFAULT 0 + ); + INSERT INTO reviews (id, is_auto) VALUES + ('user-review', 0), + ('auto-review', 1); + ", + ) + .unwrap(); + drop(conn); + + let store = Store::new(&path).unwrap(); + drop(store); + + let conn = Connection::open(&path).unwrap(); + let version: i64 = conn + .query_row("PRAGMA user_version", [], |row| row.get(0)) + .unwrap(); + assert_eq!(version, 26); + assert!(!column_exists(&conn, "reviews", "is_auto")); + + // Reviews the removed auto-review feature created in the background are + // deleted; user-initiated (and adopted, is_auto = 0) reviews survive. + let ids: Vec = conn + .prepare("SELECT id FROM reviews") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .collect::, _>>() + .unwrap(); + assert_eq!(ids, vec!["user-review".to_string()]); + + cleanup_db(&path); +} + #[test] fn test_detecting_pid_migration_clears_orphaned_detection_flags() { let path = temp_db_path("detecting-pid-backfill"); @@ -413,6 +477,11 @@ fn test_detecting_pid_migration_clears_orphaned_detection_flags() { ('idle', 0); -- Only the table the 0025 column add targets. CREATE TABLE notes (id TEXT PRIMARY KEY); + -- Only the table/column the 0026 auto-review cleanup targets. + CREATE TABLE reviews ( + id TEXT PRIMARY KEY, + is_auto INTEGER NOT NULL DEFAULT 0 + ); ", ) .unwrap(); @@ -425,7 +494,7 @@ fn test_detecting_pid_migration_clears_orphaned_detection_flags() { let version: i64 = conn .query_row("PRAGMA user_version", [], |row| row.get(0)) .unwrap(); - assert_eq!(version, 25); + assert_eq!(version, 26); assert!(column_exists(&conn, "action_contexts", "detecting_pid")); // No shipped build ever cleared the flag from outside the process that set diff --git a/apps/staged/src-tauri/src/store/migrations/0026-remove-auto-reviews/up.sql b/apps/staged/src-tauri/src/store/migrations/0026-remove-auto-reviews/up.sql new file mode 100644 index 000000000..4d141fe6b --- /dev/null +++ b/apps/staged/src-tauri/src/store/migrations/0026-remove-auto-reviews/up.sql @@ -0,0 +1,7 @@ +-- The auto-review feature (background reviews started automatically after +-- commit sessions and when adding repos) has been removed. Delete the rows +-- it created — reviews adopted by the user were already flipped to +-- is_auto = 0 — and drop the flag. Children (reviewed_files, comments, +-- reference_files) cascade. +DELETE FROM reviews WHERE is_auto = 1; +ALTER TABLE reviews DROP COLUMN is_auto; diff --git a/apps/staged/src-tauri/src/store/models.rs b/apps/staged/src-tauri/src/store/models.rs index a6385b65d..2500371cf 100644 --- a/apps/staged/src-tauri/src/store/models.rs +++ b/apps/staged/src-tauri/src/store/models.rs @@ -1217,8 +1217,6 @@ pub struct Review { pub session_id: Option, /// AI-generated one-sentence title summarising the review's confidence. pub title: Option, - /// Whether this review was automatically generated (not user-initiated). - pub is_auto: bool, /// Paths that have been marked as reviewed. pub reviewed: Vec, /// Comments attached to specific locations. @@ -1230,10 +1228,6 @@ pub struct Review { /// When the AI session finished producing this review. /// `None` while the session is still running. pub completed_at: Option, - /// The AI provider used by the session that created this review. - /// Only populated by `find_fresh_auto_review`; `None` elsewhere. - #[serde(skip_serializing_if = "Option::is_none")] - pub session_provider: Option, } impl Review { @@ -1246,14 +1240,12 @@ impl Review { scope, session_id: None, title: None, - is_auto: false, reviewed: Vec::new(), comments: Vec::new(), reference_files: Vec::new(), created_at: now, updated_at: now, completed_at: None, - session_provider: None, } } @@ -1261,11 +1253,6 @@ impl Review { self.session_id = Some(session_id.to_string()); self } - - pub fn with_auto(mut self) -> Self { - self.is_auto = true; - self - } } /// Who authored a comment. diff --git a/apps/staged/src-tauri/src/store/reviews.rs b/apps/staged/src-tauri/src/store/reviews.rs index 70703b0ed..2b6cb0622 100644 --- a/apps/staged/src-tauri/src/store/reviews.rs +++ b/apps/staged/src-tauri/src/store/reviews.rs @@ -12,8 +12,8 @@ impl Store { pub fn create_review(&self, review: &Review) -> Result<(), StoreError> { let conn = self.conn.lock().unwrap(); conn.execute( - "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", + "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", params![ review.id, review.branch_id, @@ -21,7 +21,6 @@ impl Store { review.scope.as_str(), review.session_id, review.title, - review.is_auto, review.created_at, review.updated_at, review.completed_at, @@ -50,9 +49,9 @@ impl Store { // when multiple reviews share the same (branch, commit, scope) triple. let existing: Option = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at FROM reviews - WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 AND is_auto = 0 + WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 ORDER BY created_at DESC LIMIT 1", params![branch_id, commit_sha, scope.as_str()], Self::row_to_review_header, @@ -67,8 +66,8 @@ impl Store { // Create new let review = Review::new(branch_id, commit_sha, scope); conn.execute( - "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", + "INSERT INTO reviews (id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", params![ review.id, review.branch_id, @@ -76,7 +75,6 @@ impl Store { review.scope.as_str(), review.session_id, review.title, - review.is_auto, review.created_at, review.updated_at, review.completed_at, @@ -98,13 +96,11 @@ impl Store { let conn = self.conn.lock().unwrap(); // ORDER BY created_at DESC so we get the latest when multiple // reviews share the same (branch, commit, scope) triple. - // Exclude auto reviews — they are surfaced separately via - // find_fresh_auto_review and should not be returned here. let existing: Option = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at FROM reviews - WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 AND is_auto = 0 + WHERE branch_id = ?1 AND commit_sha = ?2 AND scope = ?3 ORDER BY created_at DESC LIMIT 1", params![branch_id, commit_sha, scope.as_str()], Self::row_to_review_header, @@ -125,7 +121,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at FROM reviews WHERE id = ?1", params![id], Self::row_to_review_header, @@ -145,7 +141,7 @@ impl Store { pub fn list_reviews_for_branch(&self, branch_id: &str) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at FROM reviews WHERE branch_id = ?1 ORDER BY created_at ASC", )?; let rows = stmt.query_map(params![branch_id], Self::row_to_review_header)?; @@ -347,7 +343,7 @@ impl Store { let conn = self.conn.lock().unwrap(); let review = conn .query_row( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at FROM reviews WHERE session_id = ?1", params![session_id], Self::row_to_review_header, @@ -380,7 +376,7 @@ impl Store { ) -> Result, StoreError> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, branch_id, commit_sha, scope, session_id, title, is_auto, created_at, updated_at, completed_at + "SELECT id, branch_id, commit_sha, scope, session_id, title, created_at, updated_at, completed_at FROM reviews WHERE branch_id = ?1 AND created_at >= ?2 ORDER BY created_at ASC", @@ -423,75 +419,6 @@ impl Store { Ok(()) } - /// Update the `is_auto` flag on a review. - pub fn set_review_auto(&self, id: &str, is_auto: bool) -> Result<(), StoreError> { - let conn = self.conn.lock().unwrap(); - let now = now_timestamp(); - conn.execute( - "UPDATE reviews - SET is_auto = ?1, - updated_at = ?2, - completed_at = CASE - WHEN is_auto = 1 AND ?1 = 0 AND completed_at IS NOT NULL THEN ?2 - ELSE completed_at - END - WHERE id = ?3", - params![is_auto, now, id], - )?; - Ok(()) - } - - /// Find the most recent auto review for a branch, but only if it was - /// created after every commit on the branch. This prevents stale auto - /// reviews (from before an amended commit or a new push) from being - /// adopted or surfaced. - /// - /// `git_latest_commit_ms` is the latest git committer timestamp - /// (converted to milliseconds) obtained from the full git log. This - /// covers commits made outside the app that are absent from the - /// `commits` table. The query takes the greater of this value and - /// `MAX(commits.updated_at)` so that both in-app and out-of-app - /// commits invalidate stale auto reviews. - pub fn find_fresh_auto_review( - &self, - branch_id: &str, - git_latest_commit_ms: i64, - ) -> Result, StoreError> { - let conn = self.conn.lock().unwrap(); - // LEFT JOIN sessions to fetch the provider in the same query, - // keeping the review row and session metadata consistent under a - // single lock acquisition. - let review: Option = conn - .query_row( - "SELECT r.id, r.branch_id, r.commit_sha, r.scope, r.session_id, r.title, r.is_auto, r.created_at, r.updated_at, r.completed_at, - s.provider - FROM reviews r - LEFT JOIN sessions s ON s.id = r.session_id - WHERE r.branch_id = ?1 AND r.is_auto = 1 - AND r.created_at >= MAX( - ?2, - COALESCE( - (SELECT MAX(updated_at) FROM commits WHERE branch_id = ?1 AND sha IS NOT NULL), - 0)) - ORDER BY r.created_at DESC LIMIT 1", - params![branch_id, git_latest_commit_ms], - |row| { - let mut r = Self::row_to_review_header(row)?; - r.session_provider = row.get(10)?; - Ok(r) - }, - ) - .optional()?; - - match review { - Some(mut r) => { - Self::load_review_children(&conn, &mut r)?; - Ok(Some(r)) - } - None => Ok(None), - } - } - // ========================================================================= // Internal helpers // ========================================================================= @@ -528,14 +455,12 @@ impl Store { scope: ReviewScope::parse(&scope_str).unwrap_or(ReviewScope::Commit), session_id: row.get(4)?, title: row.get(5)?, - is_auto: row.get(6)?, reviewed: Vec::new(), comments: Vec::new(), reference_files: Vec::new(), - created_at: row.get(7)?, - updated_at: row.get(8)?, - completed_at: row.get(9)?, - session_provider: None, + created_at: row.get(6)?, + updated_at: row.get(7)?, + completed_at: row.get(8)?, }) } diff --git a/apps/staged/src-tauri/src/store/sessions.rs b/apps/staged/src-tauri/src/store/sessions.rs index 2e56064c3..ba0c84eb0 100644 --- a/apps/staged/src-tauri/src/store/sessions.rs +++ b/apps/staged/src-tauri/src/store/sessions.rs @@ -336,10 +336,9 @@ impl Store { /// Check whether a branch already has a running session. /// - /// Auto-reviews (`is_auto = 1`) are excluded because they run in the - /// background and should never block user-initiated sessions. Sessions - /// linked through `sessions.branch_id` (running push pipelines) count, so a - /// push in flight blocks new branch work the same way a commit does. + /// Sessions linked through `sessions.branch_id` (running push pipelines) + /// count, so a push in flight blocks new branch work the same way a + /// commit does. pub fn has_running_session_for_branch(&self, branch_id: &str) -> Result { let conn = self.conn.lock().unwrap(); let count: i64 = conn.query_row( @@ -349,7 +348,7 @@ impl Store { s.branch_id = ?1 OR EXISTS (SELECT 1 FROM commits c WHERE c.session_id = s.id AND c.branch_id = ?1) OR EXISTS (SELECT 1 FROM notes n WHERE n.session_id = s.id AND n.branch_id = ?1) - OR EXISTS (SELECT 1 FROM reviews r WHERE r.session_id = s.id AND r.branch_id = ?1 AND r.is_auto = 0) + OR EXISTS (SELECT 1 FROM reviews r WHERE r.session_id = s.id AND r.branch_id = ?1) )", params![branch_id], |row| row.get(0), diff --git a/apps/staged/src-tauri/src/store/tests.rs b/apps/staged/src-tauri/src/store/tests.rs index cf21b1e19..e87fc4b51 100644 --- a/apps/staged/src-tauri/src/store/tests.rs +++ b/apps/staged/src-tauri/src/store/tests.rs @@ -2937,49 +2937,6 @@ fn test_set_comment_session_round_trips() { .is_err()); } -#[test] -fn test_set_review_auto_restamps_completed_at_when_made_visible() { - let store = 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 review = Review::new(&branch.id, "abc123", ReviewScope::Branch).with_auto(); - store.create_review(&review).unwrap(); - store - .update_review_title(&review.id, "Auto review") - .unwrap(); - - let auto_review = store.get_review(&review.id).unwrap().unwrap(); - let original_completed_at = auto_review.completed_at.unwrap(); - - std::thread::sleep(std::time::Duration::from_millis(2)); - store.set_review_auto(&review.id, false).unwrap(); - - let visible_review = store.get_review(&review.id).unwrap().unwrap(); - assert!(!visible_review.is_auto); - assert!(visible_review.completed_at.unwrap() > original_completed_at); -} - -#[test] -fn test_set_review_auto_leaves_incomplete_review_uncompleted() { - let store = 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 review = Review::new(&branch.id, "abc123", ReviewScope::Branch).with_auto(); - store.create_review(&review).unwrap(); - - store.set_review_auto(&review.id, false).unwrap(); - - let visible_review = store.get_review(&review.id).unwrap().unwrap(); - assert!(!visible_review.is_auto); - assert!(visible_review.completed_at.is_none()); -} - #[test] fn test_list_reviews_for_branch() { let store = Store::in_memory().unwrap(); diff --git a/apps/staged/src-tauri/src/timeline.rs b/apps/staged/src-tauri/src/timeline.rs index 41d4badd7..73b160159 100644 --- a/apps/staged/src-tauri/src/timeline.rs +++ b/apps/staged/src-tauri/src/timeline.rs @@ -562,7 +562,6 @@ fn build_branch_timeline(store: &Arc, branch_id: &str) -> Result Result Result Result { @@ -2945,7 +2915,6 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result Result Result Result Result Result { - let store = get_store(store_mutex)?; - let branch_id: String = arg(&args, "branchId")?; - let review = tauri::async_runtime::spawn_blocking(move || { - store - .find_fresh_auto_review(&branch_id, 0) - .map_err(|e| e.to_string()) - }) - .await - .map_err(|e| e.to_string())??; - Ok(serde_json::to_value(review).unwrap()) - } - "set_review_auto" => { - let store = get_store(store_mutex)?; - let review_id: String = arg(&args, "reviewId")?; - let is_auto: bool = arg(&args, "isAuto")?; - store - .set_review_auto(&review_id, is_auto) - .map_err(|e| e.to_string())?; - Ok(Value::Null) - } - // ===================================================================== // PRs // ===================================================================== diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index eba9ef378..e18aca1dd 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -1239,20 +1239,6 @@ export function removeReferenceFile(reviewId: string, path: string): Promise { - return invokeCommand('find_fresh_auto_review', { branchId }); -} - -/** Mark or unmark a review as auto-generated. */ -export function setReviewAuto(reviewId: string, isAuto: boolean): Promise { - return invokeCommand('set_review_auto', { reviewId, isAuto }); -} - // ============================================================================= // Git helpers // ============================================================================= diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 649e1ecd2..a8765ccd1 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -331,7 +331,7 @@ return ( tl.commits.some((c) => isSessionActive(c.sessionStatus)) || tl.notes.some((n) => isSessionActive(n.sessionStatus)) || - tl.reviews.some((r) => !r.isAuto && isSessionActive(r.sessionStatus)) + tl.reviews.some((r) => isSessionActive(r.sessionStatus)) ); } let commandPipelinePending = $state(false); @@ -430,7 +430,6 @@ const candidates: Candidate[] = []; for (const review of timeline.reviews) { - if (review.isAuto) continue; const ts = Math.floor((review.completedAt ?? review.createdAt) / 1000); candidates.push({ kind: 'review', @@ -560,9 +559,6 @@ getIsRemote: () => isRemote, loadTimeline: (opts) => loadTimeline(opts), getTimeline: () => timeline, - setTimeline: (tl) => { - timeline = tl; - }, }); let requestedTimelineKey: string | null = null; @@ -703,29 +699,11 @@ const branchId = branch.id; const unlistenStatus = onSessionStatusChanged((payload) => { - const { sessionId: eventSessionId, status, branchId: eventBranchId, isAutoReview } = payload; + const { sessionId: eventSessionId, status, branchId: eventBranchId } = payload; if (status === 'completed' || status === 'error' || status === 'cancelled') { - // If this is the auto review session completing, just clear tracking - if (eventSessionId === sessionMgr.autoReviewSessionId) { - sessionMgr.autoReviewSessionId = null; - return; - } - // Push/force-push session tracking lives in pushStateStore and is // cleared centrally by sessionStatusListener.handlePushCompletion. - // Skip normal completion handling for any auto review session - if (isAutoReview) { - return; - } - - // Skip reload for the adopted auto-review session completing — - // the timeline was already updated optimistically during adoption. - if (eventSessionId === sessionMgr.adoptedSessionId) { - sessionMgr.adoptedSessionId = null; - return; - } - // Only reload if this session belongs to our branch if (eventBranchId && eventBranchId !== branchId) return; @@ -740,21 +718,12 @@ prButton.handlePushSessionComplete(status); } } else if (status === 'running' && eventBranchId === branchId) { - // Track auto review sessions started by the backend - if (isAutoReview) { - sessionMgr.autoReviewSessionId = eventSessionId; - commands.findFreshAutoReview(branchId).then((review) => { - if (review) { - sessionMgr.autoReviewId = review.id; - } - }); - } // Refresh the timeline so the pending note/commit stub appears immediately. // Skip if a session start is in-flight (pending item has no sessionId yet), // because startOrQueueSession will call loadTimeline after // it gets the sessionId — otherwise pruning can't match the pending item // and both the pending and real items briefly render simultaneously. - if (!isAutoReview && !sessionMgr.isSessionStartPending) { + if (!sessionMgr.isSessionStartPending) { loadTimeline(); } } diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index bf84dff3c..9a16c5b1e 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -1,8 +1,7 @@ /** * BranchCardSessionManager — reactive session creation logic for BranchCard * - * Manages new session modal state, auto review adoption/cancellation, and - * branch-card session start orchestration. + * Manages new session modal state and branch-card session start orchestration. * * Instantiated with a reactive branch reference. Exposes state as reactive * properties and methods. Shared branch-scoped pending session state lives in @@ -16,11 +15,6 @@ import type { BranchTimeline as BranchTimelineData, BranchSessionType, } from '../../types'; -import * as commands from '../../api/commands'; -import { getPreferredAgent } from '../settings/preferences.svelte'; -import { agentState, REMOTE_AGENTS } from '../agents/agent.svelte'; -import { projectStateStore } from '../../stores/projectState.svelte'; -import { sessionRegistry } from '../../stores/sessionRegistry.svelte'; import { buildReferringPrompt } from '../../shared/buildReferringPrompt'; import { shouldQueueBranchSession } from './branchSessionQueue'; import { @@ -42,7 +36,6 @@ export default class BranchCardSessionManager { private loadTimeline: (opts?: { timelineKey?: string | null; force?: boolean }) => void = undefined!; private getTimeline: () => BranchTimelineData | null = () => null; - private setTimeline: (tl: BranchTimelineData) => void = undefined!; // New session modal state showNewSession = $state(false); @@ -58,13 +51,6 @@ export default class BranchCardSessionManager { return branch ? hasPendingQueuedSession(branch.id) : false; }); - // Auto review state — tracks a background review started after each commit - autoReviewSessionId = $state(null); - autoReviewId = $state(null); - // Tracks the session ID of an adopted auto-review so its completion event - // can be ignored (it would otherwise trigger a spurious timeline reload). - adoptedSessionId = $state(null); - // Session modal (opened after starting a branch session, or from timeline) openSessionId = $state(null); @@ -96,13 +82,11 @@ export default class BranchCardSessionManager { getIsRemote: () => boolean; loadTimeline: (opts?: { timelineKey?: string | null; force?: boolean }) => void; getTimeline: () => BranchTimelineData | null; - setTimeline: (tl: BranchTimelineData) => void; }) { this.getBranch = opts.getBranch; this.getIsRemote = opts.getIsRemote; this.loadTimeline = opts.loadTimeline; this.getTimeline = opts.getTimeline; - this.setTimeline = opts.setTimeline; } willQueueForMode(mode: BranchSessionType): boolean { @@ -114,105 +98,6 @@ export default class BranchCardSessionManager { }); } - /** Register a session on the frontend and mark it as running. */ - private registerRunningSession( - sessionId: string, - projectId: string, - mode: BranchSessionType, - branchId: string - ) { - sessionRegistry.register(sessionId, projectId, mode, branchId); - projectStateStore.addRunningSession(projectId, sessionId); - } - - cancelAutoReview() { - if (this.autoReviewSessionId) { - commands.cancelSession(this.autoReviewSessionId).catch(() => {}); - } - if (this.autoReviewId) { - commands.deleteReview(this.autoReviewId).catch(() => {}); - } - this.autoReviewSessionId = null; - this.autoReviewId = null; - } - - async tryAdoptAutoReview(): Promise { - if (this.hasCommitSessionInProgress) return false; - - const branch = this.getBranch(); - const isRemote = this.getIsRemote(); - - try { - const review = await commands.findFreshAutoReview(branch.id); - if (!review) return false; - - // Check that the autoreview's agent matches the user's current - // preferred agent. If they differ, skip adoption so a fresh review - // is started with the correct agent instead. - // A null reviewProvider means the session predates provider tracking — - // treat it as compatible to avoid discarding valid reviews. - const agents = isRemote ? REMOTE_AGENTS : agentState.providers; - const preferredAgent = getPreferredAgent(agents); - const reviewProvider = review.sessionProvider ?? null; - if (reviewProvider !== null && preferredAgent !== reviewProvider) { - return false; - } - - if (this.autoReviewSessionId) { - // We're tracking the session locally — register it before revealing - this.registerRunningSession( - this.autoReviewSessionId, - branch.projectId, - 'review', - branch.id - ); - } else if (!review.completedAt && review.sessionId) { - // The autoreview has a session we're not tracking. Check its status - // to decide whether to resume or just register it. - const session = await commands.getSession(review.sessionId); - if (session && session.status === 'running') { - // Session is already running (e.g. agent connected but frontend - // lost track) — just register it, no resume needed. - this.registerRunningSession(review.sessionId, branch.projectId, 'review', branch.id); - } else { - // Session exists but isn't running — resume it - await commands.resumeSession( - review.sessionId, - 'Continue reviewing the code changes on this branch.', - undefined, - branch.id - ); - this.registerRunningSession(review.sessionId, branch.projectId, 'review', branch.id); - } - } - - // Only reveal the review after all fallible operations succeed - await commands.setReviewAuto(review.id, false); - - // Optimistically update the local timeline so the review is visible - // immediately, before the backend reload completes. - const currentTimeline = this.getTimeline(); - if (currentTimeline) { - this.setTimeline({ - ...currentTimeline, - reviews: currentTimeline.reviews.map((r) => - r.id === review.id ? { ...r, isAuto: false } : r - ), - }); - } - - this.adoptedSessionId = this.autoReviewSessionId; - this.autoReviewSessionId = null; - this.autoReviewId = null; - - this.loadTimeline(); - return true; - } catch (e) { - console.error('[BranchCard] Failed to adopt auto review:', e); - return false; - } - } - async startOrQueueSession( mode: BranchSessionType, prompt: string, @@ -222,10 +107,6 @@ export default class BranchCardSessionManager { const branch = this.getBranch(); const isRemote = this.getIsRemote(); - if (this.autoReviewSessionId && mode !== 'note') { - this.cancelAutoReview(); - } - await startOrQueueBranchSessionWithPending({ branchId: branch.id, isRemote, @@ -263,9 +144,6 @@ export default class BranchCardSessionManager { this.draftPrompt = ''; this.draftImageIds = []; - const adopted = await this.tryAdoptAutoReview(); - if (adopted) return; - const reviewPrompt = 'Review the code changes on this branch.'; await this.startOrQueueSession('review', reviewPrompt); } @@ -289,23 +167,10 @@ export default class BranchCardSessionManager { this.draftPrompt = ''; this.draftImageIds = []; - if (data.mode === 'review' && !data.prompt.trim()) { - const adopted = await this.tryAdoptAutoReview(); - if (adopted) return; - void this.startOrQueueSession( - data.mode, - 'Review the code changes on this branch.', - data.imageIds, - { - provider: data.provider, - acpConfigSelection: data.acpConfigSelection, - } - ); - return; - } - const prompt = - data.prompt || (data.mode === 'review' ? 'Review the code changes on this branch.' : ''); + data.prompt.trim() === '' && data.mode === 'review' + ? 'Review the code changes on this branch.' + : data.prompt; void this.startOrQueueSession(data.mode, prompt, data.imageIds, { provider: data.provider, acpConfigSelection: data.acpConfigSelection, diff --git a/apps/staged/src/lib/features/branches/branchSessionQueue.test.ts b/apps/staged/src/lib/features/branches/branchSessionQueue.test.ts index 2a97390d7..e606664f4 100644 --- a/apps/staged/src/lib/features/branches/branchSessionQueue.test.ts +++ b/apps/staged/src/lib/features/branches/branchSessionQueue.test.ts @@ -3,16 +3,12 @@ import { shouldQueueBranchSession, type BranchSessionQueueTimeline } from './bra import type { BranchSessionType } from '../../types'; function timeline( - sessions: Partial> = {}, - autoReviewStatus: string | null = null + sessions: Partial> = {} ): BranchSessionQueueTimeline { return { commits: sessions.commit === undefined ? [] : [{ sessionStatus: sessions.commit }], notes: sessions.note === undefined ? [] : [{ sessionStatus: sessions.note }], - reviews: [ - ...(sessions.review === undefined ? [] : [{ sessionStatus: sessions.review, isAuto: false }]), - ...(autoReviewStatus === null ? [] : [{ sessionStatus: autoReviewStatus, isAuto: true }]), - ], + reviews: sessions.review === undefined ? [] : [{ sessionStatus: sessions.review }], }; } @@ -63,17 +59,6 @@ describe('shouldQueueBranchSession', () => { } }); - it('ignores auto reviews when deciding whether user work can start', () => { - for (const mode of ['commit', 'note', 'review'] satisfies BranchSessionType[]) { - expect( - shouldQueueBranchSession({ - mode, - timeline: timeline({}, 'running'), - }) - ).toBe(false); - } - }); - it('allows same-type notes but queues same-type reviews', () => { expect( shouldQueueBranchSession({ diff --git a/apps/staged/src/lib/features/branches/branchSessionQueue.ts b/apps/staged/src/lib/features/branches/branchSessionQueue.ts index 597588214..964cce687 100644 --- a/apps/staged/src/lib/features/branches/branchSessionQueue.ts +++ b/apps/staged/src/lib/features/branches/branchSessionQueue.ts @@ -4,14 +4,10 @@ type TimelineSessionItem = { sessionStatus: string | null; }; -type TimelineReviewSessionItem = TimelineSessionItem & { - isAuto: boolean; -}; - export interface BranchSessionQueueTimeline { commits: TimelineSessionItem[]; notes: TimelineSessionItem[]; - reviews: TimelineReviewSessionItem[]; + reviews: TimelineSessionItem[]; } export interface BranchSessionQueueOptions { @@ -33,7 +29,7 @@ export function hasQueuedBranchSession(timeline: BranchSessionQueueTimeline): bo return ( timeline.commits.some((commit) => isQueued(commit.sessionStatus)) || timeline.notes.some((note) => isQueued(note.sessionStatus)) || - timeline.reviews.some((review) => !review.isAuto && isQueued(review.sessionStatus)) + timeline.reviews.some((review) => isQueued(review.sessionStatus)) ); } @@ -46,7 +42,7 @@ function runningBranchSessionTypes(timeline: BranchSessionQueueTimeline): Set
isRunning(note.sessionStatus))) { running.add('note'); } - if (timeline.reviews.some((review) => !review.isAuto && isRunning(review.sessionStatus))) { + if (timeline.reviews.some((review) => isRunning(review.sessionStatus))) { running.add('review'); } diff --git a/apps/staged/src/lib/features/diff/DiffModal.svelte b/apps/staged/src/lib/features/diff/DiffModal.svelte index 61d78f535..2d295eab5 100644 --- a/apps/staged/src/lib/features/diff/DiffModal.svelte +++ b/apps/staged/src/lib/features/diff/DiffModal.svelte @@ -58,7 +58,7 @@ Span, } from '../../types'; import type { DiffScope } from '../../commands'; - import { findFreshAutoReview, getSession } from '../../commands'; + import { getSession } from '../../commands'; import * as commands from '../../api/commands'; import { startOrQueueBranchSessionWithPending } from '../branches/branchSessionLaunch.svelte'; import { buildBranchHashtagItems } from '../sessions/hashtagItems'; @@ -199,7 +199,7 @@ let showContextDropdown = $state(false); /** Whether a context switch is currently in-flight (disables the dropdown). */ let switchingContext = $state(false); - /** Tracks the active auto-review reload promise so it can be ignored on stale switches. */ + /** Bumped per context switch so async steps from a stale switch are ignored. */ let contextSwitchGeneration = 0; function reviewableScope(): 'branch' | 'commit' { @@ -241,10 +241,6 @@ activeAfterLabel = newCommitSha?.slice(0, 7) ?? 'head'; } - // Always stop polling on any context switch — will restart below if needed - stopAutoReviewPolling(); - autoReviewComments = []; - // Reset review state — the $effect will recreate it once the new commitSha resolves reviewHandle = null; @@ -259,25 +255,6 @@ if (thisGeneration !== contextSwitchGeneration) return; switchingContext = false; } - - // If switching to branch scope, reload auto-review annotations. - // Guard each async step against stale generations so a rapid switch - // doesn't start polling for an already-abandoned context. - if (newScope === 'branch') { - loadAutoReviewAnnotations().then((review) => { - if (thisGeneration !== contextSwitchGeneration) return; - if (review?.sessionId) { - getSession(review.sessionId) - .then((session) => { - if (thisGeneration !== contextSwitchGeneration) return; - if (session?.status === 'running') { - startAutoReviewPolling(review.sessionId!); - } - }) - .catch((e) => console.warn('Failed to check auto-review session status:', e)); - } - }); - } } /** Index of the focused option in the dropdown (-1 = none). 0..N-1 = commits, N = "All changes". */ @@ -384,73 +361,8 @@ // Annotation reveal state (hold A to reveal) let annotationsRevealed = $state(false); - // Auto review state (branch-scope only) - let autoReviewComments = $state([]); - let autoReviewPollTimer: ReturnType | null = null; - - async function loadAutoReviewAnnotations() { - try { - const review = await findFreshAutoReview(branchId); - if (!review) { - autoReviewComments = []; - return review; - } - autoReviewComments = review.comments.filter((c) => c.commentType === 'information'); - return review; - } catch (e) { - console.error('Failed to load auto review annotations:', e); - autoReviewComments = []; - return null; - } - } - - function startAutoReviewPolling(sessionId: string) { - stopAutoReviewPolling(); - autoReviewPollTimer = setInterval(async () => { - const review = await loadAutoReviewAnnotations(); - // Check if session is still running; if not, stop polling - if (!review?.sessionId) { - stopAutoReviewPolling(); - return; - } - try { - const session = await getSession(sessionId); - if (!session || session.status !== 'running') { - stopAutoReviewPolling(); - } - } catch { - stopAutoReviewPolling(); - } - }, 4000); - } - - function stopAutoReviewPolling() { - if (autoReviewPollTimer !== null) { - clearInterval(autoReviewPollTimer); - autoReviewPollTimer = null; - } - } - - // Load auto review annotations for branch-scope diffs (no specific reviewId) - // svelte-ignore state_referenced_locally - if (activeScope === 'branch' && !reviewId) { - loadAutoReviewAnnotations().then((review) => { - if (review?.sessionId) { - // Check if the session is still running to start polling - getSession(review.sessionId) - .then((session) => { - if (session?.status === 'running') { - startAutoReviewPolling(review.sessionId!); - } - }) - .catch((e) => console.warn('Failed to check auto-review session status:', e)); - } - }); - } - onDestroy(() => { flushCommentEditorsOnDestroy(); - stopAutoReviewPolling(); }); // ========================================================================== @@ -971,8 +883,8 @@ } }); - // Keep linked-session statuses live while the modal is open. Mirrors the - // auto-review polling precedent, but event-driven via the shared listener. + // Keep linked-session statuses live while the modal is open, event-driven + // via the shared listener. $effect(() => { const linkedSessionIds = new Set(); for (const comment of allComments) { @@ -988,10 +900,9 @@ return () => unlisten(); }); - /** Convert "information" comments to SmartDiffAnnotation for the overlay system. - * Merges annotations from both the user's review and the latest auto review. */ - let currentAnnotations = $derived([ - ...allComments + /** Convert "information" comments to SmartDiffAnnotation for the overlay system. */ + let currentAnnotations = $derived( + allComments .filter((c) => c.commentType === 'information') .map((c) => ({ id: c.id, @@ -999,15 +910,8 @@ after_span: { start: c.span.start, end: c.span.end }, content: c.content, category: 'explanation' as const, - })), - ...autoReviewComments.map((c) => ({ - id: c.id, - file_path: c.path, - after_span: { start: c.span.start, end: c.span.end }, - content: c.content, - category: 'explanation' as const, - })), - ]); + })) + ); let fileEntries = $derived( buildFileEntries( diff --git a/apps/staged/src/lib/features/sessions/hashtagItems.test.ts b/apps/staged/src/lib/features/sessions/hashtagItems.test.ts index a8b51e87a..292000eb0 100644 --- a/apps/staged/src/lib/features/sessions/hashtagItems.test.ts +++ b/apps/staged/src/lib/features/sessions/hashtagItems.test.ts @@ -184,7 +184,6 @@ describe('timelineToHashtagItems', () => { completionReason: null, title: 'Old review', commentCount: 0, - isAuto: false, createdAt: 3000, updatedAt: 3000, completedAt: 3000, @@ -199,7 +198,6 @@ describe('timelineToHashtagItems', () => { completionReason: null, title: 'New review', commentCount: 0, - isAuto: false, createdAt: 7000, updatedAt: 7000, completedAt: 7000, diff --git a/apps/staged/src/lib/features/sessions/hashtagItems.ts b/apps/staged/src/lib/features/sessions/hashtagItems.ts index fc77000fd..c0799cf44 100644 --- a/apps/staged/src/lib/features/sessions/hashtagItems.ts +++ b/apps/staged/src/lib/features/sessions/hashtagItems.ts @@ -265,7 +265,6 @@ function timelineToSortableHashtagItems( } for (const review of timeline.reviews) { - if (review.isAuto) continue; if (review.completedAt == null) continue; const title = review.title || review.commitSha.slice(0, 7); items.push({ diff --git a/apps/staged/src/lib/features/settings/GeneralSettingsPanel.svelte b/apps/staged/src/lib/features/settings/GeneralSettingsPanel.svelte index b2d082c39..c5b34df71 100644 --- a/apps/staged/src/lib/features/settings/GeneralSettingsPanel.svelte +++ b/apps/staged/src/lib/features/settings/GeneralSettingsPanel.svelte @@ -4,7 +4,6 @@ import Info from '@lucide/svelte/icons/info'; import Check from '@lucide/svelte/icons/check'; import ChevronDown from '@lucide/svelte/icons/chevron-down'; - import * as Select from '$lib/components/ui/select'; import * as Popover from '$lib/components/ui/popover'; import * as ToggleGroup from '$lib/components/ui/toggle-group'; import { Input } from '$lib/components/ui/input'; @@ -15,20 +14,13 @@ getAvailableSyntaxThemes, selectDiffTheme, setMode, - setAutoReviewMode, setBranchPrefix, loadAllThemePreviewColors, isLightTheme, type AppMode, - type AutoReviewMode, type ThemePreviewColors, } from './preferences.svelte'; - const autoReviewOptions: { value: AutoReviewMode; label: string }[] = [ - { value: 'never', label: 'Never' }, - { value: 'after-changes', label: 'After changes' }, - ]; - const modeOptions: { value: AppMode; label: string }[] = [ { value: 'light', label: 'Light' }, { value: 'dark', label: 'Dark' }, @@ -161,34 +153,6 @@

-
- - setAutoReviewMode(v as AutoReviewMode)} - > - - {autoReviewOptions.find((o) => o.value === preferences.autoReviewMode)?.label ?? ''} - - - {#each autoReviewOptions as opt (opt.value)} - {opt.label} - {/each} - - -

- - {#if preferences.autoReviewMode === 'after-changes'} - A code review will automatically start after each commit session completes. - {:else} - Code reviews will only start when you manually request them. - {/if} -

-
-
diff --git a/apps/staged/src/lib/features/settings/preferences.svelte.ts b/apps/staged/src/lib/features/settings/preferences.svelte.ts index e9b1e6aa4..805aea2c9 100644 --- a/apps/staged/src/lib/features/settings/preferences.svelte.ts +++ b/apps/staged/src/lib/features/settings/preferences.svelte.ts @@ -68,7 +68,6 @@ const ACP_CONFIG_PREFS_STORE_KEY = 'acp-config-prefs'; * sync with the Rust `DIAGRAM_SUBSESSION_CONFIG_KEY`. */ const DIAGRAM_SUBSESSION_CONFIG_STORE_KEY = 'diagram-subsession-config'; -const AUTO_REVIEW_STORE_KEY = 'auto-start-code-reviews'; /** Prefix applied to branch names inferred from project names (backend-read). */ const BRANCH_PREFIX_STORE_KEY = 'branch-prefix'; /** Maximum number of recent agents to remember. */ @@ -80,8 +79,6 @@ const DEFAULT_DIFF_THEME: SyntaxThemeName = 'laserwave'; export type AppMode = 'light' | 'dark' | 'system'; const DEFAULT_APP_MODE: AppMode = 'system'; -export type AutoReviewMode = 'never' | 'after-changes'; - function normalizeSize(size: number): number { return Math.min(SIZE_MAX, Math.max(SIZE_MIN, Math.round(size))); } @@ -166,8 +163,6 @@ export const preferences = $state({ * the store file. */ diagramSubsessionConfig: null as DiagramSubsessionConfig | null, - /** Whether auto code reviews are triggered after commits */ - autoReviewMode: 'after-changes' as AutoReviewMode, /** * Prefix for branch names generated from project names (e.g. when adding a * repo without picking a branch). Joined with a `/` unless it already ends @@ -245,12 +240,6 @@ function ensureSystemModeListener() { }); } -async function loadSqAvailabilityForDefault(): Promise { - // Keep preferences from taking a static dependency on the sq state module. - const { ensureSqAvailabilityLoaded } = await import('./sq.svelte'); - return ensureSqAvailabilityLoaded(); -} - // ============================================================================= // Initialization // ============================================================================= @@ -283,9 +272,9 @@ export async function initPreferences(): Promise { applyChromeTheme(); // Unblock the UI as soon as size + chrome theme are applied. Everything below - // (diff/Shiki theme, recent agents, auto-review) only feeds the diff viewer and - // settings, not the first paint of the project view — so gating the whole app - // on it just lengthens the staged reveal on resume. Loading continues below. + // (diff/Shiki theme, recent agents) only feeds the diff viewer and settings, + // not the first paint of the project view — so gating the whole app on it + // just lengthens the staged reveal on resume. Loading continues below. preferences.loaded = true; // Load diff theme (migrating from the legacy combined `syntax-theme` key). @@ -336,14 +325,6 @@ export async function initPreferences(): Promise { preferences.diagramSubsessionConfig = savedDiagramConfig; } - // Load auto-review mode - const savedAutoReview = await getStoreValue(AUTO_REVIEW_STORE_KEY); - if (savedAutoReview === 'never' || savedAutoReview === 'after-changes') { - preferences.autoReviewMode = savedAutoReview; - } else { - preferences.autoReviewMode = (await loadSqAvailabilityForDefault()) ? 'after-changes' : 'never'; - } - // Load branch prefix const savedBranchPrefix = await getStoreValue(BRANCH_PREFIX_STORE_KEY); if (typeof savedBranchPrefix === 'string') { @@ -386,15 +367,6 @@ export async function selectDiffTheme(name: string): Promise { applyDiffTheme(); } -// ============================================================================= -// Auto Review Actions -// ============================================================================= - -export function setAutoReviewMode(mode: AutoReviewMode): void { - preferences.autoReviewMode = mode; - setStoreValue(AUTO_REVIEW_STORE_KEY, mode); -} - // ============================================================================= // Branch Prefix Actions // ============================================================================= diff --git a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte index 87ed5fb8c..d7626dba3 100644 --- a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte @@ -322,7 +322,7 @@ if (note.sessionStatus === 'running') return true; } for (const review of timeline.reviews) { - if (!review.isAuto && review.sessionStatus === 'running') return true; + if (review.sessionStatus === 'running') return true; } for (const item of pendingItems) { if (item.sessionId && !item.type.startsWith('queued-')) return true; @@ -736,7 +736,6 @@ } for (const review of timeline.reviews) { - if (review.isAuto) continue; const breakdown = reviewCommentBreakdown[review.id]; const commentCount = breakdown?.comments ?? review.commentCount; const annotationCount = breakdown?.annotations ?? 0; diff --git a/apps/staged/src/lib/features/timeline/liveSessionHints.ts b/apps/staged/src/lib/features/timeline/liveSessionHints.ts index 952c66619..775c6db7b 100644 --- a/apps/staged/src/lib/features/timeline/liveSessionHints.ts +++ b/apps/staged/src/lib/features/timeline/liveSessionHints.ts @@ -173,7 +173,7 @@ export function collectRunningSessionIds( } } for (const review of timeline.reviews) { - if (!review.isAuto && review.sessionStatus === 'running' && review.sessionId) { + if (review.sessionStatus === 'running' && review.sessionId) { ids.add(review.sessionId); } } diff --git a/apps/staged/src/lib/listeners/sessionStatusListener.test.ts b/apps/staged/src/lib/listeners/sessionStatusListener.test.ts index 1f0bbbba4..bac01b269 100644 --- a/apps/staged/src/lib/listeners/sessionStatusListener.test.ts +++ b/apps/staged/src/lib/listeners/sessionStatusListener.test.ts @@ -190,7 +190,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: 'branch-1', sessionType: 'commit', status: 'running', - isAutoReview: false, }, { sessionId: 'queued-1', @@ -198,7 +197,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: 'branch-1', sessionType: 'commit', status: 'queued', - isAutoReview: false, }, { sessionId: 'unresolved-1', @@ -206,15 +204,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: null, sessionType: 'pr', status: 'running', - isAutoReview: false, - }, - { - sessionId: 'auto-review-1', - projectId: 'project-2', - branchId: 'branch-2', - sessionType: 'review', - status: 'running', - isAutoReview: true, }, { sessionId: 'untyped-1', @@ -222,7 +211,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: null, sessionType: null, status: 'running', - isAutoReview: false, }, ]); @@ -257,7 +245,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: null, sessionType: 'other', status: 'running', - isAutoReview: false, }, ]); @@ -345,7 +332,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: 'branch-1', sessionType: 'commit', status: 'running', - isAutoReview: false, }, ]; }); @@ -387,7 +373,6 @@ describe('sessionStatusListener busy-state hydration', () => { branchId: 'branch-1', sessionType: 'commit', status: 'running', - isAutoReview: false, }, ]); await hydrateActiveSessions(); diff --git a/apps/staged/src/lib/listeners/sessionStatusListener.ts b/apps/staged/src/lib/listeners/sessionStatusListener.ts index 844db0223..6bcc51853 100644 --- a/apps/staged/src/lib/listeners/sessionStatusListener.ts +++ b/apps/staged/src/lib/listeners/sessionStatusListener.ts @@ -90,13 +90,9 @@ async function handleSessionStatusChanged(payload: SessionStatusPayload): Promis branchId: eventBranchId, projectId: eventProjectId, sessionType, - isAutoReview, } = payload; - // Auto review sessions are handled by BranchCard — don't register them - // here so they don't cause the project list spinner. When the user - // adopts an auto review, BranchCard registers the session at that point. - if (status === 'running' && eventProjectId && !isAutoReview) { + if (status === 'running' && eventProjectId) { sessionRegistry.register( sessionId, eventProjectId, @@ -140,10 +136,9 @@ async function handleSessionStatusChanged(payload: SessionStatusPayload): Promis * stuck spinner after a missed terminal event. Per-session metadata prefers * what the client already knows: launch sites register pipeline (pr/push) * sessions with their real branch/project, which the snapshot cannot resolve - * (they link no artifact), and BranchCard registers adopted auto reviews. - * Snapshot entries the client has never seen are applied with the same - * gating as the live `running` event: running, resolved project, not an - * auto review. Queued sessions register when their own running event + * (they link no artifact). Snapshot entries the client has never seen are + * applied with the same gating as the live `running` event: running, with a + * resolved project. Queued sessions register when their own running event * arrives. Unread state is per-device UX state and is left untouched. * * Swept sessions that a workflow store is still rendering as in-progress are @@ -186,7 +181,7 @@ export async function hydrateActiveSessions(): Promise { for (const session of active) { if (session.status !== 'running') continue; - if (!session.projectId || session.isAutoReview) continue; + if (!session.projectId) continue; if (sessionRegistry.getMetadata(session.sessionId)) continue; if ((terminalWhileFetching.get(session.sessionId) ?? 0) >= fetchStartedAt) continue; sessionRegistry.register( diff --git a/apps/staged/src/lib/types.ts b/apps/staged/src/lib/types.ts index cfeb951cd..69cace856 100644 --- a/apps/staged/src/lib/types.ts +++ b/apps/staged/src/lib/types.ts @@ -180,7 +180,6 @@ export interface ReviewTimelineItem { completionReason: string | null; title: string | null; commentCount: number; - isAuto: boolean; createdAt: number; updatedAt: number; completedAt: number | null; @@ -531,7 +530,6 @@ export interface SessionStatusPayload { branchId?: string; projectId?: string; sessionType?: string; - isAutoReview?: boolean; } /** @@ -546,7 +544,6 @@ export interface ActiveSessionInfo { branchId: string | null; sessionType: string | null; status: SessionStatus; - isAutoReview: boolean; } /** diff --git a/packages/diff-viewer/src/lib/state/reviewState.test.ts b/packages/diff-viewer/src/lib/state/reviewState.test.ts index b7ca04f95..0a887df60 100644 --- a/packages/diff-viewer/src/lib/state/reviewState.test.ts +++ b/packages/diff-viewer/src/lib/state/reviewState.test.ts @@ -33,7 +33,6 @@ function createReview(overrides: Partial = {}): Review { createdAt: 1, updatedAt: 1, completedAt: null, - sessionProvider: null, ...overrides, }; } diff --git a/packages/diff-viewer/src/lib/types.ts b/packages/diff-viewer/src/lib/types.ts index c938dd4a2..d60a2c44a 100644 --- a/packages/diff-viewer/src/lib/types.ts +++ b/packages/diff-viewer/src/lib/types.ts @@ -121,8 +121,6 @@ export interface Review { updatedAt: number; /** When the AI session finished producing this review. `null` while running. */ completedAt: number | null; - /** The AI provider used by the session that created this review. */ - sessionProvider?: string | null; } // =============================================================================