STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #13169 from cli/sammorrowdrums/fix-skills-publish-remote-detection

Browse source
This commit is contained in:
Sam Morrow 2026-04-16 01:17:06 +02:00 • committed by GitHub
commit 980428c304
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 271 additions and 121 deletions
Download patch file Download diff file Expand all files Collapse all files

58
acceptance/testdata/skills/skills-publish-dir-remote.txtar vendored Normal file
View file

@ -0,0 +1,58 @@
# When a directory argument is provided to `gh skill publish --dry-run`,
# the remote detection must use the target directory's git remotes,
# not the current working directory's remotes.
#
# This test creates two separate git repos:
# - cwd-repo (the working directory) with remote pointing to owner/cwd-repo
# - target-repo (the dir argument) with remote pointing to owner/target-repo
#
# If the bug is present, the command would detect cwd-repo's remote instead of
# target-repo's remote.
# Set up credential helper
exec gh auth setup-git
# Create two test repos on GitHub
exec gh repo create $ORG/$SCRIPT_NAME-cwd-$RANDOM_STRING --private --add-readme
defer gh repo delete --yes $ORG/$SCRIPT_NAME-cwd-$RANDOM_STRING
exec gh repo create $ORG/$SCRIPT_NAME-target-$RANDOM_STRING --private --add-readme
defer gh repo delete --yes $ORG/$SCRIPT_NAME-target-$RANDOM_STRING
# Clone both repos
exec gh repo clone $ORG/$SCRIPT_NAME-cwd-$RANDOM_STRING cwd-repo
exec gh repo clone $ORG/$SCRIPT_NAME-target-$RANDOM_STRING target-repo
# Add a skill to the target repo only
mkdir target-repo/skills/hello-world
cp $WORK/skill.md target-repo/skills/hello-world/SKILL.md
exec git -C $WORK/target-repo add -A
exec git -C $WORK/target-repo commit -m 'Add test skill'
exec git -C $WORK/target-repo push origin main
# Run publish dry-run from cwd-repo, pointing at target-repo
cd cwd-repo
exec gh skill publish --dry-run $WORK/target-repo
# Verify the output references the target repo, not the cwd repo
stdout 'hello-world'
# Publish with a tag from within cwd-repo, targeting target-repo
exec gh skill publish --tag v0.1.0 $WORK/target-repo
# Verify the release was created on the TARGET repo, not the cwd repo
exec gh release view v0.1.0 --repo $ORG/$SCRIPT_NAME-target-$RANDOM_STRING
stdout 'v0.1.0'
# Verify NO release was created on the cwd repo
! exec gh release view v0.1.0 --repo $ORG/$SCRIPT_NAME-cwd-$RANDOM_STRING
-- skill.md --
---
name: hello-world
description: A test skill that greets the user.
---
# Hello World
Greet the user warmly.

16
pkg/cmd/skills/publish/publish.go
View file

@ -338,7 +338,7 @@ func publishRun(opts *PublishOptions) error {
diagnostics = append(diagnostics, installedDirDiags...)
// Remote repository checks (best-effort)
repoInfo, remoteErr := detectGitHubRemote(opts.GitClient)
repoInfo, remoteErr := detectGitHubRemote(opts.GitClient, dir)
if remoteErr != nil {
return remoteErr
}
@ -867,14 +867,18 @@ func suggestNextTag(latest string) string {
return fmt.Sprintf("%s%s.%s.%d", prefix, major, minor, patch+1)
}
// detectGitHubRemote attempts to detect the GitHub owner/repo from git remotes.
func detectGitHubRemote(gitClient *git.Client) (ghrepo.Interface, error) {
// detectGitHubRemote attempts to detect the GitHub owner/repo from git remotes
// in the given directory.
func detectGitHubRemote(gitClient *git.Client, dir string) (ghrepo.Interface, error) {
if gitClient == nil {
return nil, nil
}
dirClient := gitClient.Copy()
dirClient.RepoDir = dir
// Try origin first
if url, err := gitClient.RemoteURL(context.Background(), "origin"); err == nil {
if url, err := dirClient.RemoteURL(context.Background(), "origin"); err == nil {
repo, parseErr := parseGitHubURL(url)
if parseErr != nil {
return nil, parseErr
@ -885,7 +889,7 @@ func detectGitHubRemote(gitClient *git.Client) (ghrepo.Interface, error) {
}
// Fall back to any remote that points to GitHub
remotes, err := gitClient.Remotes(context.Background())
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
}
@ -893,7 +897,7 @@ func detectGitHubRemote(gitClient *git.Client) (ghrepo.Interface, error) {
if r.Name == "origin" {
continue
}
if url, err := gitClient.RemoteURL(context.Background(), r.Name); err == nil {
if url, err := dirClient.RemoteURL(context.Background(), r.Name); err == nil {
repo, parseErr := parseGitHubURL(url)
if parseErr != nil {
return nil, parseErr

318
pkg/cmd/skills/publish/publish_test.go
View file

@ -20,23 +20,16 @@ import (
"github.com/stretchr/testify/require"
)
func newTestGitClient(t *testing.T, remoteURLs map[string]string) *git.Client {
// 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.
func initGitRepo(t *testing.T, dir string, remoteURLs map[string]string) {
t.Helper()
dir := t.TempDir()
runGit := func(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)
}
runGit("init", "--initial-branch=main")
runGit("config", "user.email", "monalisa@github.com")
runGit("config", "user.name", "Monalisa Octocat")
runGitInDir(t, dir, "init", "--initial-branch=main")
runGitInDir(t, dir, "config", "user.email", "monalisa@github.com")
runGitInDir(t, dir, "config", "user.name", "Monalisa Octocat")
for name, url := range remoteURLs {
runGit("remote", "add", name, url)
runGitInDir(t, dir, "remote", "add", name, url)
}
return &git.Client{RepoDir: dir}
}
// stubAllSecureRemote registers the standard stubs for a fully-configured remote
@ -151,10 +144,11 @@ func TestPublishRun_UnsupportedHost(t *testing.T) {
`))
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: newTestGitClient(t, map[string]string{"origin": "https://github.com/monalisa/skills-repo.git"}),
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{}),
host: "acme.ghes.com",
})
@ -270,15 +264,16 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
DryRun: true,
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
DryRun: true,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "1 skill(s) validated successfully",
@ -322,6 +317,9 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
@ -329,11 +327,9 @@ func TestPublishRun(t *testing.T) {
Prompter: &prompter.PrompterMock{
ConfirmFunc: func(msg string, def bool) (bool, error) { return true, nil },
},
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "Published v1.0.1",
@ -475,14 +471,15 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/octocat/secure-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/octocat/secure-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "secret scanning is not enabled",
@ -527,14 +524,15 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/octocat/tag-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/octocat/tag-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "tag protection",
@ -589,15 +587,16 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/octocat/code-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
DryRun: true,
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/octocat/code-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
DryRun: true,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStderr: "code scanning",
@ -653,15 +652,16 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/octocat/dep-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
DryRun: true,
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/octocat/dep-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
DryRun: true,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStderr: "Dependabot",
@ -801,16 +801,17 @@ func TestPublishRun(t *testing.T) {
},
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: newTestGitClient(t, map[string]string{
"origin": "https://gitlab.com/hubot/bar.git",
"upstream": "git@github.com:octocat/repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
DryRun: true,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStderr: "octocat/repo",
@ -887,6 +888,9 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
@ -894,11 +898,9 @@ func TestPublishRun(t *testing.T) {
Prompter: &prompter.PrompterMock{
ConfirmFunc: func(msg string, def bool) (bool, error) { return true, nil },
},
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "Added \"agent-skills\" topic",
@ -964,15 +966,16 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
Tag: "v2.3.5",
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
Tag: "v2.3.5",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "Published v2.3.5",
@ -995,15 +998,16 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
Tag: "v1.0.0", // same as stubAllSecureRemote's existing tag
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
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: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantErr: "tag v1.0.0 already exists",
@ -1027,14 +1031,15 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
IO: ios,
Dir: dir,
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "ok",
@ -1125,6 +1130,9 @@ func TestPublishRun(t *testing.T) {
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,
@ -1140,11 +1148,9 @@ func TestPublishRun(t *testing.T) {
return "v1.0.0", nil // accept suggested tag
},
},
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "Published v1.0.0",
@ -1182,6 +1188,9 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
@ -1196,11 +1205,9 @@ func TestPublishRun(t *testing.T) {
return "beta-1", nil
},
},
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "Published beta-1",
@ -1233,6 +1240,9 @@ func TestPublishRun(t *testing.T) {
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,
@ -1251,11 +1261,9 @@ func TestPublishRun(t *testing.T) {
return "v1.0.1", nil
},
},
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantErr: "CancelError",
@ -1300,6 +1308,9 @@ func TestPublishRun(t *testing.T) {
},
opts: func(ios *iostreams.IOStreams, dir string, reg *httpmock.Registry) *PublishOptions {
t.Helper()
initGitRepo(t, dir, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
})
return &PublishOptions{
IO: ios,
Dir: dir,
@ -1314,11 +1325,9 @@ func TestPublishRun(t *testing.T) {
return "v1.0.1", nil
},
},
GitClient: newTestGitClient(t, map[string]string{
"origin": "https://github.com/monalisa/skills-repo.git",
}),
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
GitClient: &git.Client{},
client: api.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
}
},
wantStdout: "Enabled immutable releases",
@ -1363,6 +1372,85 @@ 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",
})
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}
// detectGitHubRemote should use targetRepo's remotes, not cwdRepo's
repo, err := detectGitHubRemote(gitClient, targetRepo)
require.NoError(t, err)
require.NotNil(t, repo)
assert.Equal(t, "monalisa", repo.RepoOwner())
assert.Equal(t, "target-repo", repo.RepoName())
}
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",
})
targetRepo := t.TempDir()
initGitRepo(t, targetRepo, map[string]string{
"origin": "https://github.com/monalisa/target-repo.git",
})
writeSkill(t, targetRepo, "my-skill", heredoc.Doc(`
---
name: my-skill
description: A test skill
license: MIT
---
Body text.
`))
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
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",
})
require.NoError(t, err)
assert.Contains(t, stdout.String(), "1 skill(s) validated successfully")
}
// writeSkill creates skills/<name>/SKILL.md with the given content.
func writeSkill(t *testing.T, dir, name, content string) {
t.Helper()