Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion pkg/cli/compile_compiler_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func setupRepositoryContext(compiler *workflow.Compiler, config CompileConfig) {
}

// Set repository slug for schedule scattering
repoSlug := getRepositorySlugFromRemote()
repoSlug := getRepositorySlugFromRemotePreferringUpstream()
if repoSlug != "" {
compiler.SetRepositorySlug(repoSlug)
compileCompilerSetupLog.Printf("Repository slug set: %s", repoSlug)
Expand Down
19 changes: 19 additions & 0 deletions pkg/cli/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,25 @@ func getRepositorySlugFromRemote() string {
return slug
}

// getRepositorySlugFromRemotePreferringUpstream extracts the repository slug (owner/repo)
// from git remotes, preferring the 'upstream' remote when available.
// This keeps schedule scattering stable for fork checkouts where origin points to the fork.
func getRepositorySlugFromRemotePreferringUpstream() string {
if output, err := exec.Command("git", "config", "--get", "remote.upstream.url").Output(); err == nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/zoom-out] This function invokes exec.Command directly while its sibling getRepositorySlugFromRemote() delegates to the resolveRemoteURL(dir) abstraction. That abstraction centralises git-invocation logic (e.g. the -C dir pattern for path-relative calls).

Consider extending resolveRemoteURL to accept an optional preferred remote name, or extracting a small resolveNamedRemoteURL(name string) helper, so the upstream lookup flows through the same seam:

func getRepositorySlugFromRemotePreferringUpstream() string {
    if url, _, err := resolveNamedRemoteURL("upstream"); err == nil {
        if slug := parseGitHubRepoSlugFromURL(url); slug != "" {
            return slug
        }
    }
    return getRepositorySlugFromRemote()
}

This keeps a single call site for git subprocess invocation and means any future hardening of resolveRemoteURL (env isolation, custom git path, etc.) automatically applies here too.

upstreamURL := strings.TrimSpace(string(output))
if upstreamURL != "" {
slug := parseGitHubRepoSlugFromURL(upstreamURL)
Comment on lines +223 to +240
if slug != "" {
gitLog.Printf("Repository slug from upstream remote: %s", slug)
return slug
}
gitLog.Printf("Unable to parse repository slug from upstream remote URL %q; falling back", upstreamURL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The fallback branch when parseGitHubRepoSlugFromURL returns empty (e.g. upstream points to a GitLab or non-GitHub URL) is exercised by the log message but has no corresponding test. Consider adding a sub-case:

t.Run("falls back to origin when upstream URL is not a GitHub slug", func(t *testing.T) {
    addRemote(t, "upstream", "(gitlab.com/redacted)
    addRemote(t, "origin",   "https://github.com/myorg/myrepo.git")

    slug := getRepositorySlugFromRemotePreferringUpstream()
    if slug != "myorg/myrepo" {
        t.Errorf("got %q, want \"myorg/myrepo\"", slug)
    }
})

Without this, the gitLog.Printf("Unable to parse...; falling back") path is dead code from a testing perspective — a future refactor could silently remove the fallback and no test would catch it.

}
}

return getRepositorySlugFromRemote()
}

// getRepositorySlugFromRemoteForPath extracts the repository slug (owner/repo) from the git remote URL
// of the repository containing the specified file path.
// It prefers the 'origin' remote for backward compatibility. If 'origin' is not
Expand Down
58 changes: 58 additions & 0 deletions pkg/cli/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,64 @@ func TestResolveRemoteURL(t *testing.T) {
})
}

func TestGetRepositorySlugFromRemotePreferringUpstream(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-slug-upstream-*")

originalDir, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get current directory: %v", err)
}
defer func() {
if err := os.Chdir(originalDir); err != nil {
t.Logf("Warning: failed to restore directory: %v", err)
}
}()

if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp directory: %v", err)
}

if err := exec.Command("git", "init").Run(); err != nil {
t.Skip("Git not available")
}

addRemote := func(t *testing.T, name, remoteURL string) {
t.Helper()
if err := exec.Command("git", "remote", "get-url", name).Run(); err == nil {
if err := exec.Command("git", "remote", "remove", name).Run(); err != nil {
t.Fatalf("Failed to remove existing %s remote: %v", name, err)
}
}
if err := exec.Command("git", "remote", "add", name, remoteURL).Run(); err != nil {
t.Fatalf("Failed to add %s remote: %v", name, err)
}
t.Cleanup(func() {
if err := exec.Command("git", "remote", "remove", name).Run(); err != nil {
t.Logf("Warning: failed to remove %s remote during cleanup: %v", name, err)
}
})
}

t.Run("prefers upstream when both origin and upstream exist", func(t *testing.T) {
addRemote(t, "origin", "https://github.com/fork/repo.git")
addRemote(t, "upstream", "https://github.com/upstream/repo.git")

Comment on lines +775 to +778
slug := getRepositorySlugFromRemotePreferringUpstream()
if slug != "upstream/repo" {
t.Errorf("getRepositorySlugFromRemotePreferringUpstream() = %q, want %q", slug, "upstream/repo")
}
})

t.Run("falls back to origin when upstream missing", func(t *testing.T) {
addRemote(t, "origin", "https://github.com/myorg/myrepo.git")

slug := getRepositorySlugFromRemotePreferringUpstream()
if slug != "myorg/myrepo" {
t.Errorf("getRepositorySlugFromRemotePreferringUpstream() = %q, want %q", slug, "myorg/myrepo")
}
})
}

func TestGetRepositorySlugFromRemoteFallback(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-slug-fallback-*")

Expand Down
Loading