diff options
| author | soryu <soryu@soryu.co> | 2026-01-22 12:58:58 +0000 |
|---|---|---|
| committer | soryu <soryu@soryu.co> | 2026-01-22 13:00:01 +0000 |
| commit | 1b1b737006f9505b2a188a669c5a37671658ce3f (patch) | |
| tree | b9c71776f8803282fc5e303e7ebddebe9b921319 | |
| parent | 9e286c146e29e714b3b209b4d948d75cce179b05 (diff) | |
| download | soryu-1b1b737006f9505b2a188a669c5a37671658ce3f.tar.gz soryu-1b1b737006f9505b2a188a669c5a37671658ce3f.zip | |
Fix completion actions: default to PR and support remote repos
- Change default completion action from 'branch' to 'pr' for tasks
using daemon working directory
- Allow PR completion action to work without target_repo_path if the
worktree already has an origin remote configured (e.g., when cloned
from a remote URL)
- Update create_pull_request to accept optional target_repo parameter
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| -rw-r--r-- | makima/src/daemon/task/manager.rs | 31 | ||||
| -rw-r--r-- | makima/src/daemon/worktree/manager.rs | 131 | ||||
| -rw-r--r-- | makima/src/server/handlers/mesh_chat.rs | 4 |
3 files changed, 108 insertions, 58 deletions
diff --git a/makima/src/daemon/task/manager.rs b/makima/src/daemon/task/manager.rs index 555cd2a..6c99bee 100644 --- a/makima/src/daemon/task/manager.rs +++ b/makima/src/daemon/task/manager.rs @@ -4007,16 +4007,25 @@ impl TaskManagerInner { target_repo_path: Option<&str>, target_branch: Option<&str>, ) -> Result<Option<String>, String> { + // For PR action, we can use the worktree's origin directly if target_repo_path is not set let target_repo = match target_repo_path { - Some(path) => crate::daemon::worktree::expand_tilde(path), + Some(path) => Some(crate::daemon::worktree::expand_tilde(path)), None => { - tracing::warn!(task_id = %task_id, "No target_repo_path configured, skipping completion action"); - return Ok(None); + if action == "pr" { + // For PR action, check if worktree has an origin remote we can use directly + None + } else { + tracing::warn!(task_id = %task_id, "No target_repo_path configured, skipping completion action"); + return Ok(None); + } } }; - if !target_repo.exists() { - return Err(format!("Target repo not found: {} (expanded from {:?})", target_repo.display(), target_repo_path)); + // Validate target_repo exists if provided + if let Some(ref repo) = target_repo { + if !repo.exists() { + return Err(format!("Target repo not found: {} (expanded from {:?})", repo.display(), target_repo_path)); + } } // Get the branch name: makima/{task-name-with-dashes}-{short-id} @@ -4026,13 +4035,14 @@ impl TaskManagerInner { crate::daemon::worktree::short_uuid(task_id) ); - // Determine target branch - use provided value or detect default branch of target repo + // Determine target branch - use provided value or detect default branch let target_branch = match target_branch { Some(branch) => branch.to_string(), None => { - // Detect default branch (main, master, develop, etc.) + // Detect default branch from target_repo if available, otherwise from worktree + let detect_path = target_repo.as_ref().map(|p| p.as_path()).unwrap_or(worktree_path); self.worktree_manager - .detect_default_branch(&target_repo) + .detect_default_branch(detect_path) .await .unwrap_or_else(|_| "master".to_string()) } @@ -4047,6 +4057,7 @@ impl TaskManagerInner { match action { "branch" => { + let target_repo = target_repo.ok_or_else(|| "No target_repo_path configured for branch action".to_string())?; // Just push the branch to target repo self.worktree_manager .push_to_target_repo(worktree_path, &target_repo, &branch_name, task_name) @@ -4062,6 +4073,7 @@ impl TaskManagerInner { Ok(None) } "merge" => { + let target_repo = target_repo.ok_or_else(|| "No target_repo_path configured for merge action".to_string())?; // Push and merge into target branch let commit_sha = self.worktree_manager .merge_to_target(worktree_path, &target_repo, &branch_name, &target_branch, task_name) @@ -4078,6 +4090,7 @@ impl TaskManagerInner { } "pr" => { // Push and create PR + // For PR, we can use target_repo if provided, or create PR directly from worktree let title = task_name.to_string(); let body = format!( "Automated PR from makima task.\n\nTask ID: `{}`", @@ -4086,7 +4099,7 @@ impl TaskManagerInner { let pr_url = self.worktree_manager .create_pull_request( worktree_path, - &target_repo, + target_repo.as_deref(), &branch_name, &target_branch, &title, diff --git a/makima/src/daemon/worktree/manager.rs b/makima/src/daemon/worktree/manager.rs index d370828..5edd7b1 100644 --- a/makima/src/daemon/worktree/manager.rs +++ b/makima/src/daemon/worktree/manager.rs @@ -1470,10 +1470,11 @@ impl WorktreeManager { /// Create a GitHub pull request using the gh CLI. /// /// This pushes the branch first, then creates a PR. + /// If target_repo is None, uses the worktree's origin remote directly (for repos already cloned from remote). pub async fn create_pull_request( &self, worktree_path: &Path, - target_repo: &Path, + target_repo: Option<&Path>, source_branch: &str, target_branch: &str, title: &str, @@ -1481,7 +1482,7 @@ impl WorktreeManager { ) -> Result<String, WorktreeError> { tracing::info!( worktree = %worktree_path.display(), - target_repo = %target_repo.display(), + target_repo = ?target_repo.map(|p| p.display().to_string()), source_branch = %source_branch, target_branch = %target_branch, title = %title, @@ -1504,60 +1505,96 @@ impl WorktreeManager { source_branch.to_string() }; - // Push to the target repo's origin - // First, check if target_repo has an origin remote - let output = Command::new("git") - .args(["remote", "get-url", "origin"]) - .current_dir(target_repo) - .output() - .await?; + // Get the origin URL - either from target_repo or from worktree directly + let (origin_url, gh_working_dir) = if let Some(target_repo) = target_repo { + // Use target_repo's origin + let output = Command::new("git") + .args(["remote", "get-url", "origin"]) + .current_dir(target_repo) + .output() + .await?; - if !output.status.success() { - return Err(WorktreeError::GitCommand( - "Target repository has no origin remote configured".to_string(), - )); - } + if !output.status.success() { + return Err(WorktreeError::GitCommand( + "Target repository has no origin remote configured".to_string(), + )); + } + + let url = String::from_utf8_lossy(&output.stdout).trim().to_string(); + (url, target_repo.to_path_buf()) + } else { + // Check if worktree has an origin remote directly + let output = Command::new("git") + .args(["remote", "get-url", "origin"]) + .current_dir(worktree_path) + .output() + .await?; - let origin_url = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if !output.status.success() { + return Err(WorktreeError::GitCommand( + "Repository has no origin remote configured. Either set target_repo_path or ensure the worktree was cloned from a remote repository.".to_string(), + )); + } + + let url = String::from_utf8_lossy(&output.stdout).trim().to_string(); + (url, worktree_path.to_path_buf()) + }; // Push the branch from worktree to the remote - // First add the remote to worktree - let _ = Command::new("git") - .args(["remote", "remove", "pr-origin"]) - .current_dir(worktree_path) - .output() - .await; + // First add the remote to worktree (if not using worktree's origin directly) + if target_repo.is_some() { + let _ = Command::new("git") + .args(["remote", "remove", "pr-origin"]) + .current_dir(worktree_path) + .output() + .await; - let output = Command::new("git") - .args(["remote", "add", "pr-origin", &origin_url]) - .current_dir(worktree_path) - .output() - .await?; + let output = Command::new("git") + .args(["remote", "add", "pr-origin", &origin_url]) + .current_dir(worktree_path) + .output() + .await?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(WorktreeError::GitCommand(format!( - "Failed to add remote: {}", - stderr - ))); - } + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(WorktreeError::GitCommand(format!( + "Failed to add remote: {}", + stderr + ))); + } - // Push to the remote - let output = Command::new("git") - .args(["push", "-u", "pr-origin", &format!("{}:{}", current_branch, source_branch)]) - .current_dir(worktree_path) - .output() - .await?; + // Push to the remote + let output = Command::new("git") + .args(["push", "-u", "pr-origin", &format!("{}:{}", current_branch, source_branch)]) + .current_dir(worktree_path) + .output() + .await?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(WorktreeError::GitCommand(format!( - "Failed to push branch: {}", - stderr - ))); + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(WorktreeError::GitCommand(format!( + "Failed to push branch: {}", + stderr + ))); + } + } else { + // Push directly to origin + let output = Command::new("git") + .args(["push", "-u", "origin", &format!("{}:{}", current_branch, source_branch)]) + .current_dir(worktree_path) + .output() + .await?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(WorktreeError::GitCommand(format!( + "Failed to push branch: {}", + stderr + ))); + } } - // Create PR using gh CLI in the target repo + // Create PR using gh CLI let output = Command::new("gh") .args([ "pr", @@ -1567,7 +1604,7 @@ impl WorktreeManager { "--head", source_branch, "--base", target_branch, ]) - .current_dir(target_repo) + .current_dir(&gh_working_dir) .output() .await?; diff --git a/makima/src/server/handlers/mesh_chat.rs b/makima/src/server/handlers/mesh_chat.rs index 0fc5513..8e134bd 100644 --- a/makima/src/server/handlers/mesh_chat.rs +++ b/makima/src/server/handlers/mesh_chat.rs @@ -995,8 +995,8 @@ async fn handle_mesh_request( }; (Some(action), target) } else if is_daemon_working_dir { - // No merge_mode but using daemon working dir - default to "branch" - (Some("branch".to_string()), repository_url.clone()) + // No merge_mode but using daemon working dir - default to "pr" + (Some("pr".to_string()), repository_url.clone()) } else { (None, None) }; |
