Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 155 additions & 0 deletions apps/staged/src-tauri/src/session_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,13 @@ pub async fn start_branch_session(
) -> Result<BranchSessionResponse, String> {
let store = get_store(&store)?;

if matches!(
session_type,
BranchSessionType::Commit | BranchSessionType::Review
) {
cancel_in_flight_auto_review_for_branch(&store, &registry, &branch_id)?;
}

// Resolve branch → project
let branch = store
.get_branch(&branch_id)
Expand Down Expand Up @@ -835,6 +842,7 @@ pub async fn start_branch_session(
#[tauri::command(rename_all = "camelCase")]
pub fn queue_branch_session(
store: tauri::State<'_, Mutex<Option<Arc<Store>>>>,
registry: tauri::State<'_, Arc<session_runner::SessionRegistry>>,
branch_id: String,
prompt: String,
session_type: BranchSessionType,
Expand All @@ -843,6 +851,13 @@ pub fn queue_branch_session(
) -> Result<BranchSessionResponse, String> {
let store = get_store(&store)?;

if matches!(
session_type,
BranchSessionType::Commit | BranchSessionType::Review
) {
cancel_in_flight_auto_review_for_branch(&store, &registry, &branch_id)?;
}

// Validate that the branch exists.
let _branch = store
.get_branch(&branch_id)
Expand Down Expand Up @@ -1385,6 +1400,59 @@ fn latest_git_commit_ms(store: &Arc<Store>, branch_id: &str) -> i64 {
commits.iter().map(|c| c.timestamp).max().unwrap_or(0) * 1000
}

fn cancel_in_flight_auto_review_for_branch(
store: &Arc<Store>,
registry: &session_runner::SessionRegistry,
branch_id: &str,
) -> Result<bool, String> {
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 => Ok(true),
Some(store::SessionStatus::Cancelled) => {
store.delete_review(&review.id).map_err(|e| e.to_string())?;
Ok(true)
}
_ => Ok(false),
};
}
store.delete_review(&review.id).map_err(|e| e.to_string())?;

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(
Expand Down Expand Up @@ -2377,6 +2445,46 @@ Rules:
#[cfg(test)]
mod tests {
use super::*;
use std::path::Path;
use std::sync::Arc;

fn setup_branch_store() -> (Arc<Store>, store::Branch) {
let store = Arc::new(Store::in_memory().unwrap());
let project = store::Project::new("test-owner/test-repo");
store.create_project(&project).unwrap();
let branch = store::Branch::new(&project.id, "feature", "main");
store.create_branch(&branch).unwrap();
(store, branch)
}

fn create_auto_review(
store: &Arc<Store>,
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)
}

#[test]
fn infer_branch_resume_session_type_detects_pr_prompts() {
Expand Down Expand Up @@ -2434,4 +2542,51 @@ mod tests {
));
assert!(prompt.contains("do not output any preamble, commentary, or thinking before it"));
}

#[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, &registry, &branch.id).unwrap();

assert!(cancelled);
assert!(store.get_session(&session.id).unwrap().is_none());
assert!(store.get_review(&review.id).unwrap().is_none());
}

#[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, &registry, &branch.id).unwrap();

assert!(cancelled);
assert!(store.get_session(&session.id).unwrap().is_none());
assert!(store.get_review(&review.id).unwrap().is_none());
}

#[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, &registry, &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);
}
}
26 changes: 26 additions & 0 deletions apps/staged/src-tauri/src/store/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,32 @@ impl Store {
Ok(rows > 0)
}

/// Transition session status only if it is currently `queued` or `running`.
///
/// Returns `true` if the row was updated, `false` if the session already
/// moved to another state or didn't exist. This is the safe path for
/// cancelling work that may still be in the queue.
pub fn transition_from_active(
&self,
id: &str,
new_status: SessionStatus,
error_message: Option<&str>,
completion_reason: Option<&CompletionReason>,
) -> Result<bool, StoreError> {
let conn = self.conn.lock().unwrap();
let error_msg = if new_status == SessionStatus::Error {
error_message
} else {
None
};
let rows = conn.execute(
"UPDATE sessions SET status = ?1, error_message = ?2, completion_reason = ?3, updated_at = ?4
WHERE id = ?5 AND status IN ('queued', 'running')",
params![new_status.as_str(), error_msg, completion_reason.map(|r| r.as_str()), now_timestamp(), id],
)?;
Ok(rows > 0)
}

/// Atomically transition a session to `Running`, but only if it is NOT
/// already running. Returns `true` if the row was updated, `false` if
/// the session was already running (or didn't exist).
Expand Down
35 changes: 35 additions & 0 deletions apps/staged/src-tauri/src/store/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,41 @@ fn test_transition_from_running_succeeds_when_running() {
assert_eq!(final_state.status, SessionStatus::Completed);
}

#[test]
fn test_transition_from_active_succeeds_when_queued() {
let store = Store::in_memory().unwrap();

let session = Session::new_queued("queued");
store.create_session(&session).unwrap();

let transitioned = store
.transition_from_active(&session.id, SessionStatus::Cancelled, None, None)
.unwrap();
assert!(transitioned);

let final_state = store.get_session(&session.id).unwrap().unwrap();
assert_eq!(final_state.status, SessionStatus::Cancelled);
}

#[test]
fn test_transition_from_active_does_not_overwrite_completed_session() {
let store = Store::in_memory().unwrap();

let session = Session::new_running("completed first", Path::new("/tmp"));
store.create_session(&session).unwrap();
store
.update_session_status(&session.id, SessionStatus::Completed, None, None)
.unwrap();

let transitioned = store
.transition_from_active(&session.id, SessionStatus::Cancelled, None, None)
.unwrap();
assert!(!transitioned);

let final_state = store.get_session(&session.id).unwrap().unwrap();
assert_eq!(final_state.status, SessionStatus::Completed);
}

#[test]
fn test_completion_reason_round_trips() {
let store = Store::in_memory().unwrap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export default class BranchCardSessionManager {
const branch = this.getBranch();
const isRemote = this.getIsRemote();

if (this.autoReviewSessionId && mode !== 'review') {
if (this.autoReviewSessionId && mode !== 'note') {
this.cancelAutoReview();
}

Expand Down Expand Up @@ -270,7 +270,7 @@ export default class BranchCardSessionManager {
const branch = this.getBranch();
const isRemote = this.getIsRemote();

if (this.autoReviewSessionId && mode !== 'review') {
if (this.autoReviewSessionId && mode !== 'note') {
this.cancelAutoReview();
}

Expand Down Expand Up @@ -382,10 +382,6 @@ export default class BranchCardSessionManager {
return;
}

if (data.mode === 'review' && this.autoReviewSessionId) {
this.cancelAutoReview();
}

const prompt =
data.prompt || (data.mode === 'review' ? 'Review the code changes on this branch.' : '');
void this.startOrQueueSession(data.mode, prompt, data.imageIds);
Expand Down