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
12 changes: 8 additions & 4 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,12 +918,16 @@ func Update(ctx context.Context) ([]UpdateCheck, error) {
continue
}

ref := u.Entry.Source.Ref
if ref == "" {
ref = "main"
ref, err := resolveCloneRef(ctx, "https://github.com/"+owner+"/"+repo+".git", u.Entry.Source.Ref)
if err != nil {
continue
}

srcURL := "https://github.com/" + owner + "/" + repo + "/tree/" + ref + "/" + strings.TrimPrefix(u.Entry.SkillPath, "/")
srcURL := "https://github.com/" + owner + "/" + repo
if ref != "" {
srcURL += "/tree/" + ref
}
srcURL += "/" + strings.TrimPrefix(u.Entry.SkillPath, "/")
Comment on lines +926 to +930

@devin-ai-integration devin-ai-integration Bot Mar 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Update constructs invalid GitHub URL (losing skill subpath) when resolveCloneRef returns empty ref

When resolveCloneRef returns an empty ref (which occurs when detectDefaultBranch at git.go:85-106 returns ErrDefaultBranchNotFound—e.g., on git servers that don't support --symref), the URL is built as https://github.com/owner/repo/skills/my-skill instead of including /tree/<branch>/. When this URL is re-parsed by ParseSource at source.go:54-62, the githubTreeRe regex won't match (no /tree/ segment), so the fallback path splits on / and returns only Owner and Repo—the skill subpath (u.Entry.SkillPath) is silently lost. This causes Add to clone the repo and scan from the root instead of the correct subdirectory, potentially installing wrong skills or failing to find the expected skill.

URL construction path when ref is empty vs non-empty

When ref is non-empty (normal case), URL is:
https://github.com/owner/repo/tree/main/skills/my-skill → ParseSource extracts Subpath correctly.

When ref is empty (ErrDefaultBranchNotFound), URL is:
https://github.com/owner/repo/skills/my-skill → ParseSource returns Owner="owner", Repo="repo", Subpath="" (lost).

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: Update() now resolves the repo default branch when no ref is stored, so it always reconstructs a valid /tree/<branch>/... source URL and preserves the skill subpath.

_, _ = Add(ctx, AddOptions{
Source: srcURL,
Global: true,
Expand Down
63 changes: 55 additions & 8 deletions git.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"strings"
)

var ErrDefaultBranchNotFound = errors.New("default branch not found")

type clonedRepo struct {
Dir string
}
Expand All @@ -23,6 +25,26 @@ func cloneRepo(ctx context.Context, src SkillSource) (*clonedRepo, error) {
_ = os.RemoveAll(tmp)
}

repoURL := normalizeRepoURL(src)

args := []string{"clone", "--depth", "1"}
if src.Ref != "" {
args = append(args, "--branch", src.Ref)
}
args = append(args, repoURL, tmp)

cmd := exec.CommandContext(ctx, "git", args...)

out, err := cmd.CombinedOutput()
if err != nil {
cleanup()
return nil, errors.New("git clone failed: " + strings.TrimSpace(string(out)))
}

return &clonedRepo{Dir: tmp}, nil
}

func normalizeRepoURL(src SkillSource) string {
repoURL := src.SourceURL
if src.Type == SkillSourceTypeGitHub && src.Owner != "" && src.Repo != "" && !strings.HasPrefix(repoURL, "http") {
repoURL = "https://github.com/" + src.Owner + "/" + src.Repo + ".git"
Expand All @@ -36,21 +58,46 @@ func cloneRepo(ctx context.Context, src SkillSource) (*clonedRepo, error) {
repoURL = repoURL + ".git"
}

ref := src.Ref
if ref == "" {
ref = "main"
return repoURL
}

func resolveCloneRef(ctx context.Context, repoURL string, requestedRef string) (string, error) {
if requestedRef != "" {
return requestedRef, nil
}

args := []string{"clone", "--depth", "1", "--branch", ref, repoURL, tmp}
cmd := exec.CommandContext(ctx, "git", args...)
ref, err := detectDefaultBranch(ctx, repoURL)
if err != nil {
if errors.Is(err, ErrDefaultBranchNotFound) {
return "", nil
}
return "", err
}

return ref, nil
}

func detectDefaultBranch(ctx context.Context, repoURL string) (string, error) {
cmd := exec.CommandContext(ctx, "git", "ls-remote", "--symref", repoURL, "HEAD")
out, err := cmd.CombinedOutput()
if err != nil {
cleanup()
return nil, errors.New("git clone failed: " + strings.TrimSpace(string(out)))
return "", errors.New("git ls-remote failed: " + strings.TrimSpace(string(out)))
}

return &clonedRepo{Dir: tmp}, nil
for _, line := range strings.Split(string(out), "\n") {
if !strings.HasPrefix(line, "ref:") || !strings.Contains(line, "\tHEAD") {
continue
}

fields := strings.Fields(line)
if len(fields) < 2 {
continue
}

return strings.TrimPrefix(fields[1], "refs/heads/"), nil
}

return "", ErrDefaultBranchNotFound
}

func (r *clonedRepo) ResolveSubdir(subpath string) (string, error) {
Expand Down
43 changes: 43 additions & 0 deletions git_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package skills

import (
"context"
"os/exec"
"path/filepath"
"strings"
"testing"
)

func TestDetectDefaultBranch(t *testing.T) {
tmp := t.TempDir()
remote := filepath.Join(tmp, "remote.git")
work := filepath.Join(tmp, "work")

runCmd(t, tmp, "git", "init", "--bare", remote)
runCmd(t, tmp, "git", "clone", remote, work)
runCmd(t, work, "git", "checkout", "-b", "master")
runCmd(t, work, "git", "config", "user.name", "tester")
runCmd(t, work, "git", "config", "user.email", "tester@example.com")
runCmd(t, work, "git", "commit", "--allow-empty", "-m", "init")
runCmd(t, work, "git", "push", "-u", "origin", "master")

branch, err := detectDefaultBranch(context.Background(), remote)
if err != nil {
t.Fatalf("detectDefaultBranch returned error: %v", err)
}

if branch != "master" {
t.Fatalf("expected master, got %q", branch)
}
}

func runCmd(t *testing.T, dir string, name string, args ...string) {
t.Helper()

cmd := exec.Command(name, args...)
cmd.Dir = dir
out, err := cmd.CombinedOutput()
if err != nil {
t.Fatalf("%s %s failed: %v\n%s", name, strings.Join(args, " "), err, out)
}
}
2 changes: 0 additions & 2 deletions source.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func ParseSource(input string) (SkillSource, error) {
SourceURL: in,
Owner: parts[0],
Repo: strings.TrimSuffix(parts[1], ".git"),
Ref: "main",
}, nil
}
}
Expand Down Expand Up @@ -111,7 +110,6 @@ func ParseSource(input string) (SkillSource, error) {
SourceURL: "https://github.com/" + repoPart,
Owner: owner,
Repo: repo,
Ref: "main",
SkillFilter: skillFilter,
}, nil
}
Expand Down
15 changes: 15 additions & 0 deletions source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@ func TestParseSource_GitHubShorthand(t *testing.T) {
if src.Owner != "vercel-labs" || src.Repo != "agent-skills" {
t.Fatalf("owner/repo mismatch: %s/%s", src.Owner, src.Repo)
}

if src.Ref != "" {
t.Fatalf("expected empty ref for default branch discovery, got %q", src.Ref)
}
}

func TestParseSource_GitHubRepoURL(t *testing.T) {
src, err := ParseSource("https://github.com/a/b")
if err != nil {
t.Fatalf("expected nil err, got %v", err)
}

if src.Type != SkillSourceTypeGitHub || src.Ref != "" {
t.Fatalf("expected github source with empty ref, got %#v", src)
}
}

func TestParseSource_GitHubTreeURL(t *testing.T) {
Expand Down
Loading