diff --git a/AGENTS.md b/AGENTS.md index b04e6b77557..a9e3ab10951 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,6 +135,7 @@ for _, tt := range tests { - Add godoc comments to all exported functions, types, and constants - Avoid unnecessary code comments — only comment when the *why* isn't obvious from the code - Do not comment just to restate what the code does +- Never use em dashes (—) in code, comments, or documentation; use regular dashes (-) or rewrite the sentence instead ## Error Handling diff --git a/internal/skills/discovery/discovery_test.go b/internal/skills/discovery/discovery_test.go index 7f74c299876..f9abc593cf8 100644 --- a/internal/skills/discovery/discovery_test.go +++ b/internal/skills/discovery/discovery_test.go @@ -925,21 +925,21 @@ func TestMatchesSkillPath(t *testing.T) { func TestMatchSkillPath(t *testing.T) { tests := []struct { - testName string + name string path string wantName string wantNamespace string }{ - {testName: "skills convention", path: "skills/code-review/SKILL.md", wantName: "code-review", wantNamespace: ""}, - {testName: "namespaced convention", path: "skills/monalisa/issue-triage/SKILL.md", wantName: "issue-triage", wantNamespace: "monalisa"}, - {testName: "plugins convention", path: "plugins/hubot/skills/pr-summary/SKILL.md", wantName: "pr-summary", wantNamespace: "hubot"}, - {testName: "non-skill file", path: "README.md", wantName: "", wantNamespace: ""}, - {testName: "same name different namespace 1", path: "skills/kynan/commit/SKILL.md", wantName: "commit", wantNamespace: "kynan"}, - {testName: "same name different namespace 2", path: "skills/will/commit/SKILL.md", wantName: "commit", wantNamespace: "will"}, - {testName: "root convention", path: "my-skill/SKILL.md", wantName: "my-skill", wantNamespace: ""}, + {name: "skills convention", path: "skills/code-review/SKILL.md", wantName: "code-review", wantNamespace: ""}, + {name: "namespaced convention", path: "skills/monalisa/issue-triage/SKILL.md", wantName: "issue-triage", wantNamespace: "monalisa"}, + {name: "plugins convention", path: "plugins/hubot/skills/pr-summary/SKILL.md", wantName: "pr-summary", wantNamespace: "hubot"}, + {name: "non-skill file", path: "README.md", wantName: "", wantNamespace: ""}, + {name: "same name different namespace 1", path: "skills/kynan/commit/SKILL.md", wantName: "commit", wantNamespace: "kynan"}, + {name: "same name different namespace 2", path: "skills/will/commit/SKILL.md", wantName: "commit", wantNamespace: "will"}, + {name: "root convention", path: "my-skill/SKILL.md", wantName: "my-skill", wantNamespace: ""}, } for _, tt := range tests { - t.Run(tt.testName, func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { name, namespace := MatchSkillPath(tt.path) assert.Equal(t, tt.wantName, name) assert.Equal(t, tt.wantNamespace, namespace) diff --git a/pkg/cmd/root/official_extension_stub.go b/pkg/cmd/root/official_extension_stub.go new file mode 100644 index 00000000000..af52e43663e --- /dev/null +++ b/pkg/cmd/root/official_extension_stub.go @@ -0,0 +1,72 @@ +package root + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/extensions" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +// NewCmdOfficialExtensionStub creates a hidden stub command for an official +// extension that has not yet been installed. When invoked, it suggests +// installing the extension and, in interactive sessions, offers to do so +// immediately. After a successful install, the extension is dispatched with +// the original arguments. +func NewCmdOfficialExtensionStub(io *iostreams.IOStreams, p prompter.Prompter, em extensions.ExtensionManager, ext *extensions.OfficialExtension) *cobra.Command { + cmd := &cobra.Command{ + Use: ext.Name, + Short: fmt.Sprintf("Install the official %s extension", ext.Name), + Hidden: true, + GroupID: "extension", + // Accept any args/flags the user may have passed so we don't get + // cobra validation errors before reaching RunE. + DisableFlagParsing: true, + RunE: func(cmd *cobra.Command, args []string) error { + return officialExtensionStubRun(io, p, em, ext) + }, + } + + cmdutil.DisableAuthCheck(cmd) + + return cmd +} + +func officialExtensionStubRun(io *iostreams.IOStreams, p prompter.Prompter, em extensions.ExtensionManager, ext *extensions.OfficialExtension) error { + stderr := io.ErrOut + + if !io.CanPrompt() { + fmt.Fprint(stderr, heredoc.Docf(` + %[1]s is available as an official extension. + To install it, run: + gh extension install %[2]s/%[3]s + `, fmt.Sprintf("gh %s", ext.Name), ext.Owner, ext.Repo)) + return nil + } + + prompt := heredoc.Docf(` + %[1]s is available as an official extension. + Would you like to install it now? + `, fmt.Sprintf("gh %s", ext.Name)) + confirmed, err := p.Confirm(prompt, true) + if err != nil { + return err + } + if !confirmed { + return nil + } + + repo := ext.Repository() + io.StartProgressIndicatorWithLabel(fmt.Sprintf("Installing %s/%s...", ext.Owner, ext.Repo)) + installErr := em.Install(repo, "") + io.StopProgressIndicator() + if installErr != nil { + return fmt.Errorf("failed to install extension: %w", installErr) + } + + fmt.Fprintf(stderr, "Successfully installed %s/%s\n", ext.Owner, ext.Repo) + return nil +} diff --git a/pkg/cmd/root/official_extension_stub_test.go b/pkg/cmd/root/official_extension_stub_test.go new file mode 100644 index 00000000000..d2fd4624204 --- /dev/null +++ b/pkg/cmd/root/official_extension_stub_test.go @@ -0,0 +1,119 @@ +package root + +import ( + "fmt" + "testing" + + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/extensions" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOfficialExtensionStubRun(t *testing.T) { + ext := &extensions.OfficialExtension{Name: "cool", Owner: "github", Repo: "gh-cool"} + + tests := []struct { + name string + isTTY bool + confirmResult bool + confirmErr error + installErr error + wantErr string + wantStderr string + wantInstalled bool + }{ + { + name: "non-TTY prints install instructions", + isTTY: false, + wantStderr: "gh extension install github/gh-cool", + }, + { + name: "TTY confirmed installs", + isTTY: true, + confirmResult: true, + wantStderr: "Successfully installed github/gh-cool", + wantInstalled: true, + }, + { + name: "TTY declined does not install", + isTTY: true, + confirmResult: false, + }, + { + name: "TTY prompt error is propagated", + isTTY: true, + confirmErr: fmt.Errorf("prompt interrupted"), + wantErr: "prompt interrupted", + }, + { + name: "TTY install error is propagated", + isTTY: true, + confirmResult: true, + installErr: fmt.Errorf("network error"), + wantErr: "network error", + wantInstalled: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, stderr := iostreams.Test() + if tt.isTTY { + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + } + + em := &extensions.ExtensionManagerMock{ + InstallFunc: func(_ ghrepo.Interface, _ string) error { + return tt.installErr + }, + } + p := &prompter.PrompterMock{ + ConfirmFunc: func(_ string, _ bool) (bool, error) { + return tt.confirmResult, tt.confirmErr + }, + } + + err := officialExtensionStubRun(ios, p, em, ext) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + } + + if tt.wantStderr != "" { + assert.Contains(t, stderr.String(), tt.wantStderr) + } + + if tt.wantInstalled { + require.NotEmpty(t, em.InstallCalls()) + repo := em.InstallCalls()[0].InterfaceMoqParam + assert.Equal(t, "github", repo.RepoOwner()) + assert.Equal(t, "gh-cool", repo.RepoName()) + assert.Equal(t, "github.com", repo.RepoHost()) + } else if tt.isTTY && !tt.confirmResult && tt.confirmErr == nil { + assert.Empty(t, em.InstallCalls()) + } + }) + } +} + +func TestNewCmdOfficialExtensionStub_Properties(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ext := &extensions.OfficialExtension{Name: "cool", Owner: "github", Repo: "gh-cool"} + em := &extensions.ExtensionManagerMock{} + p := &prompter.PrompterMock{} + + cmd := NewCmdOfficialExtensionStub(ios, p, em, ext) + + assert.Equal(t, "cool", cmd.Use) + assert.True(t, cmd.Hidden) + assert.Equal(t, "extension", cmd.GroupID) + assert.True(t, cmd.DisableFlagParsing) +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index d44ad840c8b..37684b40c8c 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -45,6 +45,7 @@ import ( versionCmd "github.com/cli/cli/v2/pkg/cmd/version" workflowCmd "github.com/cli/cli/v2/pkg/cmd/workflow" "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/extensions" "github.com/google/shlex" "github.com/spf13/cobra" ) @@ -231,6 +232,17 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) (*cobra.Command, } } + // Official extension stubs: hidden commands that suggest installing + // GitHub-owned extensions when invoked. Registered after real extensions + // and aliases so that both take priority over stubs. + for i := range extensions.OfficialExtensions { + ext := &extensions.OfficialExtensions[i] + if _, _, err := cmd.Find([]string{ext.Name}); err == nil { + continue + } + cmd.AddCommand(NewCmdOfficialExtensionStub(io, f.Prompter, em, ext)) + } + cmdutil.DisableAuthCheck(cmd) // The reference command produces paged output that displays information on every other command. diff --git a/pkg/cmd/skills/publish/publish.go b/pkg/cmd/skills/publish/publish.go index 213afeba53d..d8278187694 100644 --- a/pkg/cmd/skills/publish/publish.go +++ b/pkg/cmd/skills/publish/publish.go @@ -42,8 +42,7 @@ type PublishOptions struct { DryRun bool Tag string - client *api.Client // injectable for tests; nil means use factory - host string // resolved from config in production + host string // resolved from config in production } // publishDiagnostic is a single validation finding. @@ -178,11 +177,10 @@ func publishRun(opts *PublishOptions) error { canPrompt := opts.IO.CanPrompt() - // Use injected client or create one from the factory HttpClient. - // Initialization is deferred until after local validation so that + // Client initialization is deferred until after local validation so that // simple errors (missing skills/, bad SKILL.md, etc.) are reported // without requiring an HTTP client. - client := opts.client + var client *api.Client host := opts.host var diagnostics []publishDiagnostic @@ -343,14 +341,11 @@ func publishRun(opts *PublishOptions) error { hasTopic := false var existingTags []tagEntry if owner != "" && repo != "" { - // Create API client from factory if not already injected (tests inject directly). - if client == nil { - httpClient, err := opts.HttpClient() - if err != nil { - return err - } - client = api.NewClientFromHTTP(httpClient) + httpClient, err := opts.HttpClient() + if err != nil { + return err } + client = api.NewClientFromHTTP(httpClient) if host == "" && repoInfo != nil { host = repoInfo.Repo.RepoHost() @@ -658,10 +653,11 @@ func ensurePushed(opts *PublishOptions, dir, remoteName string) error { } ref := fmt.Sprintf("HEAD:refs/heads/%s", currentBranch) - fmt.Fprintf(opts.IO.ErrOut, "%s Pushing %s to %s\n", cs.SuccessIcon(), currentBranch, remoteName) + fmt.Fprintf(opts.IO.ErrOut, "Pushing %s to %s...\n", currentBranch, remoteName) if err := gitClient.Push(ctx, remoteName, ref); err != nil { return fmt.Errorf("failed to push branch %s: %w", currentBranch, err) } + fmt.Fprintf(opts.IO.ErrOut, "%s Pushed %s to %s\n", cs.SuccessIcon(), currentBranch, remoteName) return nil } @@ -924,7 +920,9 @@ type gitHubRemote struct { } // detectGitHubRemote attempts to detect the GitHub owner/repo from git remotes -// in the given directory. +// in the given directory. Remotes are tried in the order returned by +// gitClient.Remotes (upstream > github > origin > rest), so the first +// GitHub-pointing remote wins. func detectGitHubRemote(gitClient *git.Client, dir string) (*gitHubRemote, error) { if gitClient == nil { return nil, nil @@ -933,26 +931,11 @@ func detectGitHubRemote(gitClient *git.Client, dir string) (*gitHubRemote, error dirClient := gitClient.Copy() dirClient.RepoDir = dir - // Try origin first - if url, err := dirClient.RemoteURL(context.Background(), "origin"); err == nil { - repo, parseErr := parseGitHubURL(url) - if parseErr != nil { - return nil, parseErr - } - if repo != nil { - return &gitHubRemote{Repo: repo, RemoteName: "origin"}, nil - } - } - - // Fall back to any remote that points to GitHub remotes, err := dirClient.Remotes(context.Background()) if err != nil { return nil, nil //nolint:nilerr // failing to list remotes is not an error; it just means no repo detected } for _, r := range remotes { - if r.Name == "origin" { - continue - } if url, err := dirClient.RemoteURL(context.Background(), r.Name); err == nil { repo, parseErr := parseGitHubURL(url) if parseErr != nil { diff --git a/pkg/cmd/skills/publish/publish_test.go b/pkg/cmd/skills/publish/publish_test.go index 8c7205a3deb..f83117b5b5e 100644 --- a/pkg/cmd/skills/publish/publish_test.go +++ b/pkg/cmd/skills/publish/publish_test.go @@ -2,17 +2,17 @@ package publish import ( "bytes" + "fmt" "net/http" "os" - "os/exec" "path/filepath" - "strings" + "regexp" "testing" "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -21,33 +21,31 @@ import ( "github.com/stretchr/testify/require" ) -// initGitRepo initializes a git repo in the given directory and adds remotes. -// Use this when the git repo must live in the same directory as the skill files. -// A local bare repo is created as the push target so that ensurePushed can work -// during publish tests, while the fetch URL remains the GitHub URL so that -// detectGitHubRemote still resolves the correct owner/repo. -func initGitRepo(t *testing.T, dir string, remoteURLs map[string]string) { - t.Helper() - - bareDir := filepath.Join(t.TempDir(), "upstream.git") - require.NoError(t, os.MkdirAll(bareDir, 0o755)) - runGitInDir(t, bareDir, "init", "--bare", "--initial-branch=main") +// newTestGitClient returns a git.Client with a fake git path to avoid real git resolution. +func newTestGitClient() *git.Client { + return &git.Client{GitPath: "some/path/git"} +} - runGitInDir(t, dir, "init", "--initial-branch=main") - runGitInDir(t, dir, "config", "user.email", "monalisa@github.com") - runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat") +// stubGitRemote registers CommandStubber stubs for git remote detection. +func stubGitRemote(cs *run.CommandStubber, remoteURLs map[string]string) { + var remoteLines string for name, url := range remoteURLs { - runGitInDir(t, dir, "remote", "add", name, url) - runGitInDir(t, dir, "remote", "set-url", "--push", name, bareDir) + remoteLines += fmt.Sprintf("%[1]s\t%[2]s (fetch)\n%[1]s\t%[2]s (push)\n", name, url) } - - runGitInDir(t, dir, "add", ".") - runGitInDir(t, dir, "commit", "--allow-empty", "-m", "init") - if _, ok := remoteURLs["origin"]; ok { - runGitInDir(t, dir, "push", "origin", "main") + cs.Register(`git( .+)? remote -v`, 0, remoteLines) + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + for name, url := range remoteURLs { + cs.Register(fmt.Sprintf(`git( .+)? remote get-url -- %s`, regexp.QuoteMeta(name)), 0, url+"\n") } } +// stubEnsurePushed registers stubs for ensurePushed + runPublishRelease CurrentBranch calls. +func stubEnsurePushed(cs *run.CommandStubber, branch string) { + cs.Register(`git( .+)? symbolic-ref --quiet HEAD`, 0, "refs/heads/"+branch+"\n") + cs.Register(`git( .+)? rev-list --count @\{push\}\.\.HEAD`, 0, "0\n") + cs.Register(`git( .+)? symbolic-ref --quiet HEAD`, 0, "refs/heads/"+branch+"\n") +} + // stubAllSecureRemote registers the standard stubs for a fully-configured remote // repo (topics, tags, rulesets, security) so publishRun skips all remote warnings. func stubAllSecureRemote(reg *httpmock.Registry, owner, repo string) { @@ -159,14 +157,17 @@ func TestPublishRun_UnsupportedHost(t *testing.T) { Body. `)) + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + stubGitRemote(cs, map[string]string{"origin": "https://github.com/monalisa/skills-repo.git"}) + ios, _, _, _ := iostreams.Test() - initGitRepo(t, dir, map[string]string{"origin": "https://github.com/monalisa/skills-repo.git"}) err := publishRun(&PublishOptions{ - IO: ios, - Dir: dir, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{}), - host: "acme.ghes.com", + IO: ios, + Dir: dir, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return nil, nil }, + host: "acme.ghes.com", }) require.ErrorContains(t, err, "supports only github.com") } @@ -177,6 +178,7 @@ func TestPublishRun(t *testing.T) { isTTY bool setup func(t *testing.T, dir string) stubs func(*httpmock.Registry) + cmdStubs func(*run.CommandStubber) opts func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions verify func(t *testing.T, dir string) wantErr string @@ -276,22 +278,24 @@ func TestPublishRun(t *testing.T) { --- Body. `)), 0o644)) - initGitRepo(t, dir, map[string]string{ - "origin": "https://github.com/monalisa/skills-repo.git", - }) }, stubs: func(reg *httpmock.Registry) { stubAllSecureRemote(reg, "monalisa", "skills-repo") }, + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ + "origin": "https://github.com/monalisa/skills-repo.git", + }) + }, opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - DryRun: true, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + DryRun: true, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "1 skill(s) validated successfully", @@ -313,22 +317,24 @@ func TestPublishRun(t *testing.T) { --- Body. `)), 0o644)) - initGitRepo(t, dir, map[string]string{ - "origin": "https://github.com/monalisa/skills-repo.git", - }) }, stubs: func(reg *httpmock.Registry) { stubAllSecureRemote(reg, "monalisa", "skills-repo") }, + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ + "origin": "https://github.com/monalisa/skills-repo.git", + }) + }, opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - DryRun: true, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + DryRun: true, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "1 skill(s) validated successfully", @@ -351,18 +357,20 @@ func TestPublishRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { stubAllSecureRemote(reg, "monalisa", "skills-repo") }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - DryRun: true, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + DryRun: true, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "1 skill(s) validated successfully", @@ -404,11 +412,14 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + stubEnsurePushed(cs, "main") + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ IO: ios, Dir: dir, @@ -416,9 +427,9 @@ func TestPublishRun(t *testing.T) { Prompter: &prompter.PrompterMock{ ConfirmFunc: func(msg string, def bool) (bool, error) { return true, nil }, }, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "Published v1.0.1", @@ -558,17 +569,19 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/octocat/secure-repo.git", }) + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "secret scanning is not enabled", @@ -611,17 +624,19 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/octocat/tag-repo.git", }) + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "tag protection", @@ -674,18 +689,20 @@ func TestPublishRun(t *testing.T) { httpmock.StatusStringResponse(404, "not found"), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/octocat/code-repo.git", }) + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - DryRun: true, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + DryRun: true, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStderr: "code scanning", @@ -739,18 +756,20 @@ func TestPublishRun(t *testing.T) { httpmock.StatusStringResponse(404, "not found"), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/octocat/dep-repo.git", }) + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - DryRun: true, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + DryRun: true, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStderr: "Dependabot", @@ -768,21 +787,25 @@ func TestPublishRun(t *testing.T) { --- Body. `)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "installed"), 0o755)) + }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? check-ignore -q -- .agents/skills`, 1, "") + cs.Register(`git( .+)? remote -v`, 0, "") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + cs.Register(`git( .+)? rev-parse --git-dir`, 0, ".git\n") + cs.Register(`git( .+)? remote -v`, 0, "") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") }, opts: func(ios *iostreams.IOStreams, dir string, _ *httpmock.Registry) *PublishOptions { t.Helper() - require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "installed"), 0o755)) - runGitInDir(t, dir, "init", "--initial-branch=main") - runGitInDir(t, dir, "config", "user.email", "monalisa@github.com") - runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat") - return &PublishOptions{ IO: ios, Dir: dir, - GitClient: &git.Client{RepoDir: dir}, + GitClient: &git.Client{GitPath: "some/path/git", RepoDir: dir}, } }, - wantStdout: ".gitignore", + wantStdout: "may contain installed skills that are not gitignored", }, { name: "installed skill dirs gitignored no warning", @@ -797,20 +820,21 @@ func TestPublishRun(t *testing.T) { Body. `)) require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "installed"), 0o755)) - - runGitInDir(t, dir, "init", "--initial-branch=main") - runGitInDir(t, dir, "config", "user.email", "monalisa@github.com") - runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat") - require.NoError(t, os.WriteFile(filepath.Join(dir, ".gitignore"), []byte(".agents/skills\n"), 0o644)) - runGitInDir(t, dir, "add", ".gitignore") - runGitInDir(t, dir, "commit", "-m", "init") + }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? check-ignore -q -- .agents/skills`, 0, "") + cs.Register(`git( .+)? remote -v`, 0, "") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + cs.Register(`git( .+)? rev-parse --git-dir`, 0, ".git\n") + cs.Register(`git( .+)? remote -v`, 0, "") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") }, opts: func(ios *iostreams.IOStreams, dir string, _ *httpmock.Registry) *PublishOptions { t.Helper() return &PublishOptions{ IO: ios, Dir: dir, - GitClient: &git.Client{RepoDir: dir}, + GitClient: &git.Client{GitPath: "some/path/git", RepoDir: dir}, } }, wantStdout: "no git remote", @@ -831,15 +855,22 @@ func TestPublishRun(t *testing.T) { --- Body. `)) - // Create install dir but do NOT init git so check-ignore will fail require.NoError(t, os.MkdirAll(filepath.Join(dir, ".agents", "skills", "installed"), 0o755)) }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? check-ignore -q -- .agents/skills`, 128, "") + cs.Register(`git( .+)? remote -v`, 0, "") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + cs.Register(`git( .+)? rev-parse --git-dir`, 0, ".git\n") + cs.Register(`git( .+)? remote -v`, 0, "") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + }, opts: func(ios *iostreams.IOStreams, dir string, _ *httpmock.Registry) *PublishOptions { t.Helper() return &PublishOptions{ IO: ios, Dir: dir, - GitClient: &git.Client{RepoDir: dir}, + GitClient: &git.Client{GitPath: "some/path/git", RepoDir: dir}, } }, wantStdout: "may contain installed skills that are not gitignored", @@ -856,17 +887,22 @@ func TestPublishRun(t *testing.T) { --- Body. `)) - runGitInDir(t, dir, "init", "--initial-branch=main") - runGitInDir(t, dir, "config", "user.email", "monalisa@github.com") - runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat") - runGitInDir(t, dir, "remote", "add", "origin", "https://gitlab.com/hubot/bar.git") + }, + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ + "origin": "https://gitlab.com/hubot/bar.git", + }) + cs.Register(`git( .+)? rev-parse --git-dir`, 0, ".git\n") + cs.Register(`git( .+)? remote -v`, 0, "origin\thttps://gitlab.com/hubot/bar.git (fetch)\norigin\thttps://gitlab.com/hubot/bar.git (push)\n") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + cs.Register(fmt.Sprintf(`git( .+)? remote get-url -- %s`, regexp.QuoteMeta("origin")), 0, "https://gitlab.com/hubot/bar.git\n") }, opts: func(ios *iostreams.IOStreams, dir string, _ *httpmock.Registry) *PublishOptions { t.Helper() return &PublishOptions{ IO: ios, Dir: dir, - GitClient: &git.Client{RepoDir: dir}, + GitClient: &git.Client{GitPath: "some/path/git", RepoDir: dir}, } }, wantStdout: "not a GitHub repository", @@ -888,19 +924,21 @@ func TestPublishRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { stubAllSecureRemote(reg, "octocat", "repo") }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? remote -v`, 0, "origin\thttps://gitlab.com/hubot/bar.git (fetch)\norigin\thttps://gitlab.com/hubot/bar.git (push)\nupstream\tgit@github.com:octocat/repo.git (fetch)\nupstream\tgit@github.com:octocat/repo.git (push)\n") + cs.Register(`git( .+)? config --get-regexp \^remote\\\.`, 1, "") + // upstream sorts first (score 3 > 1), so only upstream's get-url is called + cs.Register(fmt.Sprintf(`git( .+)? remote get-url -- %s`, regexp.QuoteMeta("upstream")), 0, "git@github.com:octocat/repo.git\n") + }, opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { t.Helper() - initGitRepo(t, dir, map[string]string{ - "origin": "https://gitlab.com/hubot/bar.git", - "upstream": "git@github.com:octocat/repo.git", - }) return &PublishOptions{ - IO: ios, - Dir: dir, - DryRun: true, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + DryRun: true, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStderr: "octocat/repo", @@ -975,11 +1013,14 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + stubEnsurePushed(cs, "main") + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ IO: ios, Dir: dir, @@ -987,12 +1028,11 @@ func TestPublishRun(t *testing.T) { Prompter: &prompter.PrompterMock{ ConfirmFunc: func(msg string, def bool) (bool, error) { return true, nil }, }, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, - wantStdout: "Added \"agent-skills\" topic", }, { name: "tag suggestion uses existing tags", @@ -1053,18 +1093,21 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + stubEnsurePushed(cs, "main") + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - Tag: "v2.3.5", - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + Tag: "v2.3.5", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "Published v2.3.5", @@ -1085,18 +1128,22 @@ func TestPublishRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { stubAllSecureRemote(reg, "monalisa", "skills-repo") }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + cs.Register(`git( .+)? symbolic-ref --quiet HEAD`, 0, "refs/heads/main\n") + cs.Register(`git( .+)? rev-list --count @\{push\}\.\.HEAD`, 0, "0\n") + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - Tag: "v1.0.0", // same as stubAllSecureRemote's existing tag - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + Tag: "v1.0.0", // same as stubAllSecureRemote's existing tag + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantErr: "tag v1.0.0 already exists", @@ -1118,17 +1165,19 @@ func TestPublishRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { stubAllSecureRemote(reg, "monalisa", "skills-repo") }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ - IO: ios, - Dir: dir, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: dir, + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "ok", @@ -1216,12 +1265,15 @@ func TestPublishRun(t *testing.T) { }), ) }, + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ + "origin": "https://github.com/monalisa/skills-repo.git", + }) + stubEnsurePushed(cs, "main") + }, opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { t.Helper() confirmCall := 0 - initGitRepo(t, dir, map[string]string{ - "origin": "https://github.com/monalisa/skills-repo.git", - }) return &PublishOptions{ IO: ios, Dir: dir, @@ -1237,9 +1289,9 @@ func TestPublishRun(t *testing.T) { return "v1.0.0", nil // accept suggested tag }, }, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "Published v1.0.0", @@ -1275,11 +1327,14 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + stubEnsurePushed(cs, "main") + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ IO: ios, Dir: dir, @@ -1294,9 +1349,9 @@ func TestPublishRun(t *testing.T) { return "beta-1", nil }, }, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "Published beta-1", @@ -1326,12 +1381,15 @@ func TestPublishRun(t *testing.T) { httpmock.JSONResponse(map[string]interface{}{"default_branch": "main"}), ) }, + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ + "origin": "https://github.com/monalisa/skills-repo.git", + }) + stubEnsurePushed(cs, "main") + }, opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { t.Helper() confirmCall := 0 - initGitRepo(t, dir, map[string]string{ - "origin": "https://github.com/monalisa/skills-repo.git", - }) return &PublishOptions{ IO: ios, Dir: dir, @@ -1350,9 +1408,9 @@ func TestPublishRun(t *testing.T) { return "v1.0.1", nil }, }, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantErr: "CancelError", @@ -1395,11 +1453,14 @@ func TestPublishRun(t *testing.T) { }), ) }, - opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { - t.Helper() - initGitRepo(t, dir, map[string]string{ + cmdStubs: func(cs *run.CommandStubber) { + stubGitRemote(cs, map[string]string{ "origin": "https://github.com/monalisa/skills-repo.git", }) + stubEnsurePushed(cs, "main") + }, + opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions { + t.Helper() return &PublishOptions{ IO: ios, Dir: dir, @@ -1414,9 +1475,9 @@ func TestPublishRun(t *testing.T) { return "v1.0.1", nil }, }, - GitClient: &git.Client{}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + GitClient: newTestGitClient(), + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", } }, wantStdout: "Enabled immutable releases", @@ -1439,6 +1500,12 @@ func TestPublishRun(t *testing.T) { tt.stubs(reg) } + if tt.cmdStubs != nil { + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + tt.cmdStubs(cs) + } + opts := tt.opts(ios, dir, reg) err := publishRun(opts) @@ -1462,22 +1529,17 @@ func TestPublishRun(t *testing.T) { } func TestDetectGitHubRemote_UsesDir(t *testing.T) { - // Create two separate git repos: "cwd-repo" simulates the working directory - // and "target-repo" simulates the directory argument passed to publish. - cwdRepo := t.TempDir() - initGitRepo(t, cwdRepo, map[string]string{ - "origin": "https://github.com/monalisa/cwd-repo.git", + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + stubGitRemote(cs, map[string]string{ + "origin": "https://github.com/monalisa/target-repo.git", }) + cwdRepo := t.TempDir() targetRepo := t.TempDir() - initGitRepo(t, targetRepo, map[string]string{ - "origin": "https://github.com/monalisa/target-repo.git", - }) - // gitClient points at cwd-repo (simulating factory-provided client) - gitClient := &git.Client{RepoDir: cwdRepo} + gitClient := &git.Client{GitPath: "some/path/git", RepoDir: cwdRepo} - // detectGitHubRemote should use targetRepo's remotes, not cwdRepo's repo, err := detectGitHubRemote(gitClient, targetRepo) require.NoError(t, err) require.NotNil(t, repo) @@ -1486,24 +1548,14 @@ func TestDetectGitHubRemote_UsesDir(t *testing.T) { } func TestPublishRun_DirArgUsesTargetRemote(t *testing.T) { - // Regression test: when a directory argument is provided, remote detection - // must use that directory's git remotes, not the factory client's directory. - // - // Scenario: - // 1. User is in cwd-repo (has remote → monalisa/cwd-repo) - // 2. User runs: gh skill publish /path/to/target-repo - // 3. target-repo has remote → monalisa/target-repo - // 4. API calls must go to target-repo, NOT cwd-repo - - cwdRepo := t.TempDir() - initGitRepo(t, cwdRepo, map[string]string{ - "origin": "https://github.com/monalisa/cwd-repo.git", + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + stubGitRemote(cs, map[string]string{ + "origin": "https://github.com/monalisa/target-repo.git", }) + cwdRepo := t.TempDir() targetRepo := t.TempDir() - initGitRepo(t, targetRepo, map[string]string{ - "origin": "https://github.com/monalisa/target-repo.git", - }) writeSkill(t, targetRepo, "my-skill", heredoc.Doc(` --- @@ -1521,19 +1573,15 @@ func TestPublishRun_DirArgUsesTargetRemote(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) - - // Stub API calls for target-repo (the correct repo). - // If the bug is present, these stubs won't be called because the code - // would try to hit cwd-repo endpoints instead, and reg.Verify would fail. stubAllSecureRemote(reg, "monalisa", "target-repo") err := publishRun(&PublishOptions{ - IO: ios, - Dir: targetRepo, - DryRun: true, - GitClient: &git.Client{RepoDir: cwdRepo}, - client: api.NewClientFromHTTP(&http.Client{Transport: reg}), - host: "github.com", + IO: ios, + Dir: targetRepo, + DryRun: true, + GitClient: &git.Client{GitPath: "some/path/git", RepoDir: cwdRepo}, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + host: "github.com", }) require.NoError(t, err) @@ -1548,112 +1596,36 @@ func writeSkill(t *testing.T, dir, name, content string) { require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) } -// runGitInDir runs a git command in the given directory with isolation env vars. -func runGitInDir(t *testing.T, dir string, args ...string) { - t.Helper() - cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) - cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "HOME="+dir) - out, err := cmd.CombinedOutput() - require.NoError(t, err, "git %v: %s", args, out) -} - -// newTestGitClientWithUpstream creates a git repo with a local bare "remote" -// and an initial commit, so we can test push/rev-list behavior realistically. -// It returns the git client and the working directory path. -func newTestGitClientWithUpstream(t *testing.T) (*git.Client, string) { - t.Helper() - parentDir := t.TempDir() - bareDir := filepath.Join(parentDir, "upstream.git") - workDir := filepath.Join(parentDir, "work") - - gitEnv := append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "HOME="+parentDir) - - run := func(dir string, args ...string) { - t.Helper() - c := exec.Command("git", append([]string{"-C", dir}, args...)...) - c.Env = gitEnv - out, err := c.CombinedOutput() - require.NoError(t, err, "git %v: %s", args, out) - } - - // Create bare upstream - require.NoError(t, os.MkdirAll(bareDir, 0o755)) - run(bareDir, "init", "--bare", "--initial-branch=main") - - // Clone into working dir - c := exec.Command("git", "clone", bareDir, workDir) - c.Env = gitEnv - out, err := c.CombinedOutput() - require.NoError(t, err, "git clone: %s", out) - - run(workDir, "config", "user.email", "monalisa@github.com") - run(workDir, "config", "user.name", "Monalisa Octocat") - - // Create initial commit and push - require.NoError(t, os.WriteFile(filepath.Join(workDir, "README.md"), []byte("# Test"), 0o644)) - run(workDir, "add", ".") - run(workDir, "commit", "-m", "initial commit") - run(workDir, "push", "origin", "main") - - return &git.Client{ - RepoDir: workDir, - GitPath: "git", - Stderr: &bytes.Buffer{}, - Stdin: &bytes.Buffer{}, - Stdout: &bytes.Buffer{}, - }, workDir -} - func TestEnsurePushed(t *testing.T) { tests := []struct { name string - setup func(t *testing.T, workDir string) - verify func(t *testing.T, workDir string) + cmdStubs func(*run.CommandStubber) wantErr string wantStderr string }{ { name: "no unpushed commits is a no-op", - setup: func(_ *testing.T, _ string) { - // initial commit already pushed by helper + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? symbolic-ref --quiet HEAD`, 0, "refs/heads/main\n") + cs.Register(`git( .+)? rev-list --count @\{push\}\.\.HEAD`, 0, "0\n") }, }, { name: "unpushed commits are pushed automatically", - setup: func(t *testing.T, workDir string) { - t.Helper() - require.NoError(t, os.WriteFile(filepath.Join(workDir, "new.txt"), []byte("new"), 0o644)) - runGitInDir(t, workDir, "add", ".") - runGitInDir(t, workDir, "commit", "-m", "unpushed change") - }, - verify: func(t *testing.T, workDir string) { - t.Helper() - // After push, rev-list should show 0 unpushed commits - cmd := exec.Command("git", "-C", workDir, "rev-list", "--count", "@{push}..HEAD") - cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "HOME="+workDir) - out, err := cmd.CombinedOutput() - require.NoError(t, err, "rev-list: %s", out) - assert.Equal(t, "0", strings.TrimSpace(string(out))) + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? symbolic-ref --quiet HEAD`, 0, "refs/heads/main\n") + cs.Register(`git( .+)? rev-list --count @\{push\}\.\.HEAD`, 0, "1\n") + cs.Register(`git( .+)? push --set-upstream origin HEAD:refs/heads/main`, 0, "") }, wantStderr: "Pushing main to origin", }, { - name: "new branch never pushed is pushed automatically", - setup: func(t *testing.T, workDir string) { - t.Helper() - runGitInDir(t, workDir, "checkout", "-b", "feature") - require.NoError(t, os.WriteFile(filepath.Join(workDir, "feat.txt"), []byte("feat"), 0o644)) - runGitInDir(t, workDir, "add", ".") - runGitInDir(t, workDir, "commit", "-m", "new branch commit") - }, - verify: func(t *testing.T, workDir string) { - t.Helper() - // After push, the branch should exist on the remote - cmd := exec.Command("git", "-C", workDir, "rev-list", "--count", "@{push}..HEAD") - cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "HOME="+workDir) - out, err := cmd.CombinedOutput() - require.NoError(t, err, "rev-list: %s", out) - assert.Equal(t, "0", strings.TrimSpace(string(out))) + name: "new branch that has not been pushed is pushed automatically", + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git( .+)? symbolic-ref --quiet HEAD`, 0, "refs/heads/feature\n") + // rev-list fails when branch is not pushed + cs.Register(`git( .+)? rev-list --count @\{push\}\.\.HEAD`, 1, "") + cs.Register(`git( .+)? push --set-upstream origin HEAD:refs/heads/feature`, 0, "") }, wantStderr: "Pushing feature to origin", }, @@ -1661,8 +1633,11 @@ func TestEnsurePushed(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gitClient, workDir := newTestGitClientWithUpstream(t) - tt.setup(t, workDir) + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + tt.cmdStubs(cs) + + workDir := t.TempDir() ios, _, _, stderr := iostreams.Test() ios.SetStdoutTTY(true) @@ -1670,7 +1645,7 @@ func TestEnsurePushed(t *testing.T) { opts := &PublishOptions{ IO: ios, - GitClient: gitClient, + GitClient: &git.Client{GitPath: "some/path/git", RepoDir: workDir}, } err := ensurePushed(opts, workDir, "origin") @@ -1684,9 +1659,6 @@ func TestEnsurePushed(t *testing.T) { if tt.wantStderr != "" { assert.Contains(t, stderr.String(), tt.wantStderr) } - if tt.verify != nil { - tt.verify(t, workDir) - } }) } } diff --git a/pkg/cmd/skills/search/search.go b/pkg/cmd/skills/search/search.go index f7d4975a7ee..05511484eae 100644 --- a/pkg/cmd/skills/search/search.go +++ b/pkg/cmd/skills/search/search.go @@ -772,7 +772,14 @@ func fetchPrimaryPages(client *api.Client, host, query string, displayPage, disp // deduplicateResults extracts unique (repo, namespace, skill name) triples from code search hits. func deduplicateResults(items []codeSearchItem) []skillResult { - seen := make(map[string]struct{}) + // skillResultKey is a typed map key that deduplicates by (repo, namespace, + // skill name). All fields are lowercased for case-insensitive comparison. + type skillResultKey struct { + repo string + namespace string + skillName string + } + seen := make(map[skillResultKey]struct{}) var results []skillResult for _, item := range items { @@ -780,7 +787,11 @@ func deduplicateResults(items []codeSearchItem) []skillResult { if skillName == "" { continue } - key := item.Repository.FullName + "/" + namespace + "/" + skillName + key := skillResultKey{ + repo: strings.ToLower(item.Repository.FullName), + namespace: strings.ToLower(namespace), + skillName: strings.ToLower(skillName), + } if _, ok := seen[key]; ok { continue } diff --git a/pkg/extensions/official.go b/pkg/extensions/official.go new file mode 100644 index 00000000000..a07c426df91 --- /dev/null +++ b/pkg/extensions/official.go @@ -0,0 +1,26 @@ +package extensions + +import ( + "github.com/cli/cli/v2/internal/ghrepo" +) + +// OfficialExtension describes a GitHub-owned CLI extension that can be +// suggested to users when they invoke an unknown command. +type OfficialExtension struct { + Name string + Owner string + Repo string +} + +// Repository returns a ghrepo.Interface pinned to github.com so that GHES +// users install from github.com rather than their enterprise host. +func (e *OfficialExtension) Repository() ghrepo.Interface { + return ghrepo.NewWithHost(e.Owner, e.Repo, "github.com") +} + +// OfficialExtensions is the registry of GitHub-owned extensions that gh will +// offer to install when the user invokes the corresponding command name. +var OfficialExtensions = []OfficialExtension{ + {Name: "aw", Owner: "github", Repo: "gh-aw"}, + {Name: "stack", Owner: "github", Repo: "gh-stack"}, +} diff --git a/pkg/extensions/official_test.go b/pkg/extensions/official_test.go new file mode 100644 index 00000000000..047af580af1 --- /dev/null +++ b/pkg/extensions/official_test.go @@ -0,0 +1,15 @@ +package extensions + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOfficialExtension_Repository(t *testing.T) { + ext := &OfficialExtension{Name: "stack", Owner: "github", Repo: "gh-stack"} + repo := ext.Repository() + assert.Equal(t, "github", repo.RepoOwner()) + assert.Equal(t, "gh-stack", repo.RepoName()) + assert.Equal(t, "github.com", repo.RepoHost()) +}