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
25 changes: 3 additions & 22 deletions src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -852,9 +852,9 @@ mod tests {
selected_task_from_side_panel_rows, side_panel_rows_from, sorted_categories_with_indexes,
};
use super::workflows::{
build_attach_popup_lines, parse_existing_branch_name, popup_style_from_theme,
reconcile_startup_tasks, repo_match_candidates, repo_selection_command_id,
repo_selection_usage_map, resolve_repo_for_creation, tmux_hex_color,
build_attach_popup_lines, popup_style_from_theme, reconcile_startup_tasks,
repo_match_candidates, repo_selection_command_id, repo_selection_usage_map,
resolve_repo_for_creation, tmux_hex_color,
};
use super::*;

Expand Down Expand Up @@ -1172,25 +1172,6 @@ mod tests {
assert_eq!(ranked.first().copied(), Some(1));
}

#[test]
fn parse_existing_branch_name_detects_git_branch_collision() {
let detail =
"stderr: Preparing worktree (new branch 'c')\nfatal: a branch named 'c' already exists";
assert_eq!(parse_existing_branch_name(detail), Some("c".to_string()));
}

#[test]
fn create_task_error_dialog_state_branch_collision_is_concise() {
let err = anyhow::anyhow!(
"worktree creation failed: failed to create worktree `/home/cc/.opencode-kanban-worktrees/test/c-2` for branch `c` from `main`: git command failed in /home/cc/codes/playgrounds/test: git worktree add -b c /home/cc/.opencode-kanban-worktrees/test/c-2 main\nstdout:\nstderr: Preparing worktree (new branch 'c')\nfatal: a branch named 'c' already exists"
);

let dialog = create_task_error_dialog_state(&err);
assert_eq!(dialog.title, "Branch already exists");
assert!(dialog.detail.contains("Branch `c` already exists"));
assert!(!dialog.detail.contains("git worktree add -b"));
}

#[test]
fn resolve_repo_for_creation_accepts_fuzzy_existing_repo_query() -> Result<()> {
let db = Database::open(":memory:")?;
Expand Down
25 changes: 23 additions & 2 deletions src/app/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use std::path::{Path, PathBuf};
use anyhow::{Context, Result};

use crate::git::{
git_check_branch_up_to_date, git_create_worktree, git_detect_default_branch, git_fetch,
git_is_valid_repo, git_remove_worktree, git_resolve_remote_ref, git_set_upstream,
git_check_branch_up_to_date, git_create_worktree, git_create_worktree_from_existing_branch,
git_detect_default_branch, git_fetch, git_is_valid_repo, git_local_branch_exists,
git_remove_worktree, git_resolve_remote_ref, git_set_upstream,
};
use crate::process::command;
use crate::tmux::{
Expand Down Expand Up @@ -120,6 +121,7 @@ pub trait CreateTaskRuntime {
fn git_fetch(&self, repo_path: &Path) -> Result<()>;
fn git_resolve_remote_ref(&self, repo_path: &Path, source: &str) -> Result<String>;
fn git_validate_branch(&self, repo_path: &Path, branch_name: &str) -> Result<()>;
fn git_local_branch_exists(&self, repo_path: &Path, branch_name: &str) -> bool;
fn git_check_branch_up_to_date(&self, repo_path: &Path, base_ref: &str) -> Result<()>;
fn git_create_worktree(
&self,
Expand All @@ -128,6 +130,12 @@ pub trait CreateTaskRuntime {
branch_name: &str,
base_ref: &str,
) -> Result<()>;
fn git_create_worktree_from_existing_branch(
&self,
repo_path: &Path,
worktree_path: &Path,
branch_name: &str,
) -> Result<()>;
fn git_set_upstream(&self, repo_path: &Path, branch: &str, remote_source: &str) -> Result<()>;
fn git_remove_worktree(&self, repo_path: &Path, worktree_path: &Path) -> Result<()>;
fn tmux_session_exists(&self, session_name: &str) -> bool;
Expand Down Expand Up @@ -238,6 +246,10 @@ impl CreateTaskRuntime for RealCreateTaskRuntime {
git_check_branch_up_to_date(repo_path, base_ref)
}

fn git_local_branch_exists(&self, repo_path: &Path, branch_name: &str) -> bool {
git_local_branch_exists(repo_path, branch_name)
}

fn git_create_worktree(
&self,
repo_path: &Path,
Expand All @@ -248,6 +260,15 @@ impl CreateTaskRuntime for RealCreateTaskRuntime {
git_create_worktree(repo_path, worktree_path, branch_name, base_ref)
}

fn git_create_worktree_from_existing_branch(
&self,
repo_path: &Path,
worktree_path: &Path,
branch_name: &str,
) -> Result<()> {
git_create_worktree_from_existing_branch(repo_path, worktree_path, branch_name)
}

fn git_remove_worktree(&self, repo_path: &Path, worktree_path: &Path) -> Result<()> {
git_remove_worktree(repo_path, worktree_path)
}
Expand Down
156 changes: 122 additions & 34 deletions src/app/workflows/create_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,30 +115,8 @@ pub(crate) fn create_task_pipeline_with_runtime(
.git_validate_branch(&repo_path, &branch)
.context("branch validation failed")?;

let mut base_ref = if state.base_input.trim().is_empty() {
runtime.git_detect_default_branch(&repo_path)
} else {
state.base_input.trim().to_string()
};

if state.base_is_remote {
runtime
.git_fetch(&repo_path)
.context("failed to fetch origin; no task was created")?;
base_ref = runtime
.git_resolve_remote_ref(&repo_path, &base_ref)
.context("selected origin branch is no longer available; no task was created")?;
} else if let Err(err) = runtime.git_fetch(&repo_path) {
let message = format!("fetch from origin failed, continuing offline: {err:#}");
tracing::warn!("{message}");
warning = Some(message);
}

if state.ensure_base_up_to_date {
runtime
.git_check_branch_up_to_date(&repo_path, &base_ref)
.context("base branch check failed")?;
}
let reuse_existing_branch = !state.branch_input.trim().is_empty()
&& runtime.git_local_branch_exists(&repo_path, &branch);

let worktrees_root = worktrees_root_for_repo(&repo_path);
fs::create_dir_all(&worktrees_root).with_context(|| {
Expand All @@ -149,16 +127,54 @@ pub(crate) fn create_task_pipeline_with_runtime(
})?;
let derived_worktree_path = derive_worktree_path(&worktrees_root, &repo_path, &branch);

runtime
.git_create_worktree(&repo_path, &derived_worktree_path, &branch, &base_ref)
.context("worktree creation failed")?;
if reuse_existing_branch {
runtime
.git_create_worktree_from_existing_branch(
&repo_path,
&derived_worktree_path,
&branch,
)
.context("worktree creation failed")?;
} else {
let mut base_ref = if state.base_input.trim().is_empty() {
runtime.git_detect_default_branch(&repo_path)
} else {
state.base_input.trim().to_string()
};

if state.base_is_remote {
runtime
.git_fetch(&repo_path)
.context("failed to fetch origin; no task was created")?;
base_ref = runtime
.git_resolve_remote_ref(&repo_path, &base_ref)
.context(
"selected origin branch is no longer available; no task was created",
)?;
} else if let Err(err) = runtime.git_fetch(&repo_path) {
let message = format!("fetch from origin failed, continuing offline: {err:#}");
tracing::warn!("{message}");
warning = Some(message);
}

if state.base_is_remote
&& let Err(error) = runtime.git_set_upstream(&derived_worktree_path, &branch, &base_ref)
{
let _ = runtime.git_remove_worktree(&repo_path, &derived_worktree_path);
return Err(error)
.context("worktree was created but upstream tracking could not be configured");
if state.ensure_base_up_to_date {
runtime
.git_check_branch_up_to_date(&repo_path, &base_ref)
.context("base branch check failed")?;
}

runtime
.git_create_worktree(&repo_path, &derived_worktree_path, &branch, &base_ref)
.context("worktree creation failed")?;

if state.base_is_remote
&& let Err(error) =
runtime.git_set_upstream(&derived_worktree_path, &branch, &base_ref)
{
let _ = runtime.git_remove_worktree(&repo_path, &derived_worktree_path);
return Err(error)
.context("worktree was created but upstream tracking could not be configured");
}
}

(repo, branch, repo_path, derived_worktree_path, true)
Expand Down Expand Up @@ -507,7 +523,7 @@ mod tests {
use crate::db::Database;
use crate::types::Repo;
use anyhow::Result;
use std::cell::RefCell;
use std::cell::{Cell, RefCell};
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use tempfile::TempDir;
Expand All @@ -516,8 +532,12 @@ mod tests {
struct FakeCreateRuntime {
fetch_error: Option<String>,
resolve_error: Option<String>,
existing_branch: Cell<bool>,
reuse_error: Cell<bool>,
fetched: RefCell<bool>,
created: RefCell<bool>,
reused: RefCell<bool>,
session_created: Cell<bool>,
upstream: RefCell<Vec<String>>,
}

Expand All @@ -526,8 +546,12 @@ mod tests {
Self {
fetch_error: fetch_error.map(str::to_string),
resolve_error: resolve_error.map(str::to_string),
existing_branch: Cell::new(false),
reuse_error: Cell::new(false),
fetched: RefCell::new(false),
created: RefCell::new(false),
reused: RefCell::new(false),
session_created: Cell::new(false),
upstream: RefCell::new(Vec::new()),
}
}
Expand Down Expand Up @@ -562,13 +586,29 @@ mod tests {
fn git_validate_branch(&self, _: &Path, _: &str) -> Result<()> {
Ok(())
}
fn git_local_branch_exists(&self, _: &Path, _: &str) -> bool {
self.existing_branch.get()
}
fn git_check_branch_up_to_date(&self, _: &Path, _: &str) -> Result<()> {
Ok(())
}
fn git_create_worktree(&self, _: &Path, _: &Path, _: &str, _: &str) -> Result<()> {
*self.created.borrow_mut() = true;
Ok(())
}
fn git_create_worktree_from_existing_branch(
&self,
_: &Path,
_: &Path,
_: &str,
) -> Result<()> {
*self.created.borrow_mut() = true;
*self.reused.borrow_mut() = true;
if self.reuse_error.get() {
anyhow::bail!("branch is already checked out");
}
Ok(())
}
fn git_set_upstream(&self, _: &Path, branch: &str, source: &str) -> Result<()> {
self.upstream
.borrow_mut()
Expand All @@ -582,6 +622,7 @@ mod tests {
false
}
fn tmux_create_session(&self, _: &str, _: &Path, _: Option<&str>) -> Result<()> {
self.session_created.set(true);
Ok(())
}
fn tmux_apply_task_status_bar(
Expand Down Expand Up @@ -705,6 +746,53 @@ mod tests {
assert!(local_runtime.upstream.borrow().is_empty());
}

#[test]
fn existing_local_branch_skips_base_and_upstream_handling() {
let (_temp, db, repo) = pipeline_fixture();
let category = db.list_categories().expect("categories")[0].id;
let runtime = FakeCreateRuntime::new(Some("fetch must not run"), None);
runtime.existing_branch.set(true);

create_task_pipeline_with_runtime(
&db,
&mut vec![repo.clone()],
category,
&pipeline_state(Path::new(&repo.path), true),
None,
&runtime,
)
.expect("existing branch task");

assert!(*runtime.created.borrow());
assert!(*runtime.reused.borrow());
assert!(!*runtime.fetched.borrow());
assert!(runtime.upstream.borrow().is_empty());
assert_eq!(db.list_tasks().expect("tasks").len(), 1);
}

#[test]
fn existing_local_branch_failure_stops_before_session_and_task_creation() {
let (_temp, db, repo) = pipeline_fixture();
let category = db.list_categories().expect("categories")[0].id;
let runtime = FakeCreateRuntime::new(None, None);
runtime.existing_branch.set(true);
runtime.reuse_error.set(true);

let error = create_task_pipeline_with_runtime(
&db,
&mut vec![repo.clone()],
category,
&pipeline_state(Path::new(&repo.path), false),
None,
&runtime,
)
.expect_err("checked-out branch should fail");

assert!(error.to_string().contains("worktree creation failed"));
assert!(!runtime.session_created.get());
assert_eq!(db.list_tasks().expect("tasks").len(), 0);
}

#[test]
fn resolve_create_task_branch_rejects_empty_branch_and_title() {
let err = resolve_create_task_branch("", "").expect_err("empty branch+title must fail");
Expand Down
22 changes: 0 additions & 22 deletions src/app/workflows/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,6 @@ use crate::app::ErrorDialogState;
pub(crate) fn create_task_error_dialog_state(err: &anyhow::Error) -> ErrorDialogState {
let detail = format!("{err:#}");

if let Some(branch) = parse_existing_branch_name(&detail) {
return ErrorDialogState {
title: "Branch already exists".to_string(),
detail: format!(
"Branch `{branch}` already exists in this repository, so a new worktree branch cannot be created.\n\nChoose a different branch name, or delete/rename the existing local branch and try again."
),
};
}

let title = if detail.contains("worktree creation failed") {
"Worktree creation failed".to_string()
} else if detail.contains("tmux session creation failed") {
Expand All @@ -22,16 +13,3 @@ pub(crate) fn create_task_error_dialog_state(err: &anyhow::Error) -> ErrorDialog

ErrorDialogState { title, detail }
}

pub(crate) fn parse_existing_branch_name(detail: &str) -> Option<String> {
detail.lines().find_map(|line| {
let trimmed = line.trim();
let rest = trimmed.strip_prefix("fatal: a branch named '")?;
let (branch_name, _) = rest.split_once("' already exists")?;
if branch_name.is_empty() {
None
} else {
Some(branch_name.to_string())
}
})
}
2 changes: 0 additions & 2 deletions src/app/workflows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,4 @@ pub(crate) use create_task::{
repo_match_candidates, repo_selection_command_id, resolve_repo_for_creation,
};
pub(crate) use errors::create_task_error_dialog_state;
#[cfg(test)]
pub(crate) use errors::parse_existing_branch_name;
pub(crate) use recovery::reconcile_startup_tasks;
Loading
Loading