feat: add clone command to fetch remote skills repository#20
feat: add clone command to fetch remote skills repository#20pixincreate wants to merge 10 commits intomasterfrom
Conversation
- Add 'capsync clone <repo>' command to clone remote GitHub repos - Auto-detect default branch (main/master) from remote - Support --branch flag for specific branch - Handle existing source: update vs override with backup - Warn when local has unpushed changes before override - Auto-sync after clone unless --no-sync is passed - Add git2 dependency for programmatic git operations Also: - Rename detailed-working.md to how-it-works.md - Add agents.md documentation for AI agents - Remove unused docs/ directory - Fix clippy warnings (unused fields)
There was a problem hiding this comment.
Pull request overview
This PR adds a new capsync clone <repo> workflow so users can pull a remote skills repository into their configured skills_source, then optionally run a sync to wire those skills into all enabled tool destinations.
Changes:
- Introduces
clonemodule +capsync cloneCLI subcommand with branch selection / auto-detection and post-clone sync. - Adds
git2dependency to perform clone/fetch/reset operations programmatically. - Updates/introduces documentation under
documentation/and adjusts README examples.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
src/main.rs |
Wires in the new clone module for the binary. |
src/lib.rs |
Exposes clone module from the library crate. |
src/clone.rs |
Implements repo URL parsing, default-branch detection, clone/update/override flows. |
src/cli.rs |
Adds capsync clone subcommand and integrates clone + optional sync into CLI. |
README.md |
Adjusts example paths (casing) in the symlink walkthrough + config snippet. |
documentation/how-it-works.md |
Adds detailed architecture/behavior narrative (needs an update for new clone behavior). |
documentation/agents.md |
Adds agent-oriented command/config reference including clone. |
Cargo.toml |
Adds git2 dependency. |
Cargo.lock |
Locks new transitive dependencies (libgit2/openssl/etc). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix URL parsing to handle owner/repo.git without doubling .git - Fix has_unpushed_changes to use proper bitflag methods - Check for dirty tree before update (hard reset would lose changes) - Prompt for confirmation when skills_source is not a git repo - Fix remote ref lookup using find_branch with Remote type - Propagate fetch errors instead of ignoring silently - Fix override prompt text mismatch - Improve clone_repo error handling (distinguish file-not-found) - Fix README case consistency (~/Dev/ -> ~/dev/) - Remove GitHub hardcoding, accept any valid git URL
Test parse_repo_url with: - Full https/http URLs - Git SSH URLs - Non-GitHub providers (Codeberg, GitLab, Bitbucket) - Whitespace/trailing slash trimming - Invalid URL handling
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Err(anyhow!( | ||
| "Invalid repository: '{}'. Must be a valid git URL.", | ||
| input | ||
| )) |
There was a problem hiding this comment.
parse_repo_url currently rejects owner/repo inputs (it only accepts http(s):// and git@ URLs), but the CLI help/docs say capsync clone <repo> supports owner/repo. Either add support for owner/repo (e.g., expanding to a canonical URL) or update the CLI/docs to match the actual accepted formats.
| Err(anyhow!( | |
| "Invalid repository: '{}'. Must be a valid git URL.", | |
| input | |
| )) | |
| let parts: Vec<&str> = input.split('/').collect(); | |
| if parts.len() == 2 | |
| && parts.iter().all(|part| !part.is_empty()) | |
| && !input.contains(char::is_whitespace) | |
| { | |
| Ok(format!("https://github.com/{}/{}.git", parts[0], parts[1])) | |
| } else { | |
| Err(anyhow!( | |
| "Invalid repository: '{}'. Must be a valid git URL or owner/repo.", | |
| input | |
| )) | |
| } |
| fn find_default_branch(repository: &Repository) -> Result<String> { | ||
| let branches = repository.branches(Some(git2::BranchType::Remote))?; | ||
|
|
||
| let mut candidates: Vec<(String, bool)> = Vec::new(); | ||
|
|
||
| for branch_result in branches { | ||
| let (branch, _) = branch_result.context("Failed to iterate branches")?; | ||
| let name_result = branch.name().context("Failed to get branch name")?; | ||
| if let Some(branch_name) = name_result { | ||
| let is_main = branch_name == "origin/main"; | ||
| let is_master = branch_name == "origin/master"; | ||
| let is_preferred = is_main || is_master; | ||
| candidates.push((branch_name.to_string(), is_preferred)); | ||
| } | ||
| } | ||
|
|
||
| candidates.sort_by(|candidate_a, candidate_b| { | ||
| let (_, a_is_preferred) = candidate_a; | ||
| let (_, b_is_preferred) = candidate_b; | ||
| match (a_is_preferred, b_is_preferred) { | ||
| (true, false) => std::cmp::Ordering::Less, | ||
| (false, true) => std::cmp::Ordering::Greater, | ||
| _ => candidate_a.0.cmp(&candidate_b.0), | ||
| } | ||
| }); | ||
|
|
||
| candidates | ||
| .first() | ||
| .map(|(branch_name, _)| branch_name.trim_start_matches("origin/").to_string()) | ||
| .ok_or_else(|| anyhow!("No branches found in remote repository")) | ||
| } |
There was a problem hiding this comment.
find_default_branch may select origin/HEAD (trimmed to HEAD) when the remote’s default branch is not main/master, because origin/HEAD is often present among remote branches and sorts early alphabetically. Consider ignoring origin/HEAD or resolving it to the branch it points to, otherwise cloning with --branch HEAD will fail.
| let (action, backup_path) = if source_exists { | ||
| let current_remote = get_remote_url(source); | ||
|
|
||
| if current_remote.is_some() { | ||
| println!("\nSkills source already exists."); | ||
|
|
||
| let is_same_repo = current_remote | ||
| .as_ref() | ||
| .map(|remote_url| remote_url.contains(&options.repo) || url.contains(remote_url)) | ||
| .unwrap_or(false); | ||
|
|
||
| if is_same_repo { | ||
| loop { | ||
| print!("Update (git pull) or Override (fresh clone)? [U/o]: "); | ||
| io::stdout().flush()?; | ||
| let mut input = String::new(); | ||
| io::stdin().read_line(&mut input)?; | ||
| let input = input.trim().to_lowercase(); | ||
|
|
||
| if input.is_empty() || input == "u" { | ||
| update_existing(source)?; | ||
| return Ok(CloneResult { | ||
| action: CloneAction::Updated, | ||
| backup_path: None, | ||
| }); | ||
| } else if input == "o" { | ||
| break; | ||
| } | ||
| println!("Please enter U or o."); | ||
| } | ||
| } else { | ||
| loop { | ||
| print!("Override with different repository? [y/N]: "); | ||
| io::stdout().flush()?; | ||
| let mut input = String::new(); | ||
| io::stdin().read_line(&mut input)?; | ||
| let input = input.trim().to_lowercase(); | ||
|
|
||
| if input == "y" { | ||
| break; | ||
| } else if input.is_empty() || input == "n" { | ||
| return Err(anyhow!("Aborted.")); | ||
| } | ||
| println!("Please enter y or n."); | ||
| } | ||
| } | ||
| } else { | ||
| loop { | ||
| print!("Skills source exists but is not a git repository. Override? [y/N]: "); | ||
| io::stdout().flush()?; |
There was a problem hiding this comment.
get_remote_url returning None is treated as “not a git repository”, but it can also mean “git repo without an origin remote”. That will show an incorrect prompt and then proceed with override flow. Consider explicitly checking Repository::open(source) first and handling “no origin remote” separately from “not a git repo”.
| use capsync::clone::parse_repo_url; | ||
|
|
||
| #[test] | ||
| fn test_parse_full_https_url() { | ||
| let result = parse_repo_url("https://github.com/user/repo").unwrap(); | ||
| assert_eq!(result, "https://github.com/user/repo.git"); | ||
|
|
||
| let result2 = parse_repo_url("https://github.com/user/repo.git").unwrap(); | ||
| assert_eq!(result2, "https://github.com/user/repo.git"); | ||
| } |
There was a problem hiding this comment.
No test currently covers the advertised owner/repo input form for capsync clone <repo>. Once parse_repo_url is updated (or the CLI contract is changed), add a test case for parse_repo_url("user/repo") to prevent regressions.
| ``` | ||
|
|
||
| **Arguments:** | ||
| - `<repo>`: Repository in `owner/repo` format or full URL |
There was a problem hiding this comment.
This doc states <repo> supports owner/repo format, but the current implementation (parse_repo_url) only accepts http(s):// and git@ URLs. Either update the implementation to match the documented CLI contract, or update this section to document the actually supported input formats.
| - `<repo>`: Repository in `owner/repo` format or full URL | |
| - `<repo>`: Full Git repository URL in `https://...`, `http://...`, or `git@...` format |
- Add owner/repo shorthand support in parse_repo_url - Filter out origin/HEAD in find_default_branch - Use actual remote name in fetch refspec (not hardcoded origin) - Fix error message to say 'uncommitted or unpushed changes' - Change get_remote_url to return Result<Option<String>> to distinguish no remote (None) from not a git repo (error) - Add test for owner/repo shorthand
pixincreate
force-pushed
the
feature/clone
branch
from
732f593 to
cd1b4f6
Compare
- Look up local branch first, then its upstream (fixes update existing) - Use if let instead of match for single patterns - Remove redundant comments that state the obvious
Changed 'fresh clone' to 'download new' to match actual behavior (backup is only created when local changes exist)
- Fix prompt text in agents.md (fresh clone → download new)
Also: