-
Notifications
You must be signed in to change notification settings - Fork 343
fix: fuzzy schedule scattering works with non-origin remote names
#24390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,9 @@ import ( | |
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/github/gh-aw/pkg/console" | ||
| "github.com/github/gh-aw/pkg/logger" | ||
| "github.com/github/gh-aw/pkg/workflow" | ||
| ) | ||
|
|
@@ -111,7 +113,7 @@ func createAndConfigureCompiler(config CompileConfig) *workflow.Compiler { | |
| } | ||
|
|
||
| // Set up repository context | ||
| setupRepositoryContext(compiler) | ||
| setupRepositoryContext(compiler, config) | ||
|
|
||
| return compiler | ||
| } | ||
|
|
@@ -195,9 +197,25 @@ func setupActionMode(compiler *workflow.Compiler, actionMode string, actionTag s | |
| } | ||
|
|
||
| // setupRepositoryContext sets the repository slug for schedule scattering | ||
| func setupRepositoryContext(compiler *workflow.Compiler) { | ||
| func setupRepositoryContext(compiler *workflow.Compiler, config CompileConfig) { | ||
| compileCompilerSetupLog.Print("Setting up repository context") | ||
|
|
||
| // If a schedule seed is explicitly provided, use it directly | ||
| if config.ScheduleSeed != "" { | ||
| // Validate owner/repo format: must contain exactly one '/' with non-empty parts | ||
| parts := strings.SplitN(config.ScheduleSeed, "/", 2) | ||
| if len(parts) != 2 || parts[0] == "" || parts[1] == "" { | ||
| compileCompilerSetupLog.Printf("Invalid --schedule-seed value %q: expected 'owner/repo' format", config.ScheduleSeed) | ||
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage( | ||
| fmt.Sprintf("--schedule-seed %q is not in 'owner/repo' format; ignoring and falling back to git remote detection", config.ScheduleSeed), | ||
| )) | ||
| } else { | ||
| compiler.SetRepositorySlug(config.ScheduleSeed) | ||
| compileCompilerSetupLog.Printf("Repository slug overridden via --schedule-seed: %s", config.ScheduleSeed) | ||
| return | ||
| } | ||
|
Comment on lines
+203
to
+216
|
||
| } | ||
|
|
||
| // Set repository slug for schedule scattering | ||
| repoSlug := getRepositorySlugFromRemote() | ||
| if repoSlug != "" { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -118,39 +118,85 @@ func extractHostFromRemoteURL(remoteURL string) string { | |||||||||||
| return "github.com" | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // getHostFromOriginRemote returns the hostname of the git origin remote. | ||||||||||||
| // resolveRemoteURL resolves the best git remote URL to use for a given directory. | ||||||||||||
| // It first tries the 'origin' remote for backward compatibility. If 'origin' is not | ||||||||||||
| // configured but exactly one other remote exists, that remote is used instead. | ||||||||||||
| // Returns the remote URL, the remote name used, and any error. | ||||||||||||
| // dir may be empty to use the current working directory. | ||||||||||||
| func resolveRemoteURL(dir string) (string, string, error) { | ||||||||||||
| gitArgs := func(args ...string) *exec.Cmd { | ||||||||||||
| if dir != "" { | ||||||||||||
| return exec.Command("git", append([]string{"-C", dir}, args...)...) | ||||||||||||
| } | ||||||||||||
| return exec.Command("git", args...) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // First try 'origin' for backward compatibility | ||||||||||||
| if output, err := gitArgs("config", "--get", "remote.origin.url").Output(); err == nil { | ||||||||||||
| url := strings.TrimSpace(string(output)) | ||||||||||||
| if url != "" { | ||||||||||||
| gitLog.Print("Using 'origin' remote") | ||||||||||||
| return url, "origin", nil | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+134
to
+141
|
||||||||||||
|
|
||||||||||||
| // Fall back: list all remotes | ||||||||||||
| output, err := gitArgs("remote").Output() | ||||||||||||
| if err != nil { | ||||||||||||
| return "", "", fmt.Errorf("failed to list git remotes: %w", err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| remoteNames := strings.Fields(strings.TrimSpace(string(output))) | ||||||||||||
| if len(remoteNames) == 0 { | ||||||||||||
| return "", "", errors.New("no git remotes configured") | ||||||||||||
| } | ||||||||||||
| if len(remoteNames) > 1 { | ||||||||||||
| return "", "", fmt.Errorf("multiple git remotes configured (%s), no 'origin' remote found", strings.Join(remoteNames, ", ")) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Exactly one remote — use it | ||||||||||||
| remoteName := remoteNames[0] | ||||||||||||
| urlOutput, err := gitArgs("config", "--get", "remote."+remoteName+".url").Output() | ||||||||||||
| if err != nil { | ||||||||||||
| return "", "", fmt.Errorf("failed to get URL for remote %q: %w", remoteName, err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| url := strings.TrimSpace(string(urlOutput)) | ||||||||||||
|
||||||||||||
| url := strings.TrimSpace(string(urlOutput)) | |
| url := strings.TrimSpace(string(urlOutput)) | |
| if url == "" { | |
| return "", "", fmt.Errorf("remote %q has no URL configured", remoteName) | |
| } |
Copilot
AI
Apr 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getHostFromOriginRemote now falls back to a non-origin remote when origin is absent, so the function name is no longer accurate. If this is an internal API, consider renaming to something like getHostFromRemote and keeping getHostFromOriginRemote as a thin backward-compatible wrapper (or update call sites to the new name) to avoid future confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--schedule-seedvalidation does not enforce the documented “exactly one '/'” requirement. Usingstrings.SplitN(seed, "/", 2)allows values likeowner/repo/extrato be treated as valid and used as the repository slug, which can lead to unexpected scattering seeds. Consider validating withstrings.Count(seed, "/") == 1(or splitting on all '/' and requiring exactly 2 non-empty parts) and optionally trimming surrounding whitespace before validation.