feat(skills): auto-push unpushed commits before publish
Like gh pr create, skill publish now automatically pushes unpushed
local commits before creating a release. This prevents the footgun
where a release is created against stale remote state when the user
has local commits that haven't been pushed yet.
The ensurePushed function checks for unpushed commits using
git rev-list @{push}..HEAD. If commits exist or the branch has
never been pushed, it pushes automatically and prints a status
message. This matches the CLI's opinionated-defaults philosophy
of doing the right thing by default.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
c5cff0f298
commit
e559a7cd5b
2 changed files with 227 additions and 15 deletions
|
|
@ -10,6 +10,7 @@ import (
|
|||
"path/filepath"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/MakeNowJust/heredoc"
|
||||
|
|
@ -343,14 +344,14 @@ func publishRun(opts *PublishOptions) error {
|
|||
}
|
||||
owner, repo := "", ""
|
||||
if repoInfo != nil {
|
||||
owner = repoInfo.RepoOwner()
|
||||
repo = repoInfo.RepoName()
|
||||
owner = repoInfo.Repo.RepoOwner()
|
||||
repo = repoInfo.Repo.RepoName()
|
||||
}
|
||||
hasTopic := false
|
||||
var existingTags []tagEntry
|
||||
if owner != "" && repo != "" {
|
||||
if host == "" && repoInfo != nil {
|
||||
host = repoInfo.RepoHost()
|
||||
host = repoInfo.Repo.RepoHost()
|
||||
}
|
||||
if host != "" {
|
||||
if err := source.ValidateSupportedHost(host); err != nil {
|
||||
|
|
@ -438,7 +439,7 @@ func publishRun(opts *PublishOptions) error {
|
|||
|
||||
fmt.Fprintf(opts.IO.ErrOut, "\nPublishing to %s/%s...\n\n", owner, repo)
|
||||
|
||||
return runPublishRelease(opts, client, host, owner, repo, dir, hasTopic, existingTags)
|
||||
return runPublishRelease(opts, client, host, owner, repo, dir, repoInfo.RemoteName, hasTopic, existingTags)
|
||||
}
|
||||
|
||||
// repoHasTopic checks whether the repo has the agent-skills topic.
|
||||
|
|
@ -473,11 +474,11 @@ func fetchTags(client *api.Client, host, owner, repo string) []tagEntry {
|
|||
}
|
||||
|
||||
// runPublishRelease handles the interactive publish flow: topic, tag, release, immutability.
|
||||
func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, repo, dir string, hasTopic bool, existingTags []tagEntry) error {
|
||||
func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, repo, dir, remoteName string, hasTopic bool, existingTags []tagEntry) error {
|
||||
cs := opts.IO.ColorScheme()
|
||||
canPrompt := opts.IO.CanPrompt()
|
||||
|
||||
// 1. Add topic if missing
|
||||
// Add topic if missing
|
||||
if !hasTopic {
|
||||
addTopic := true
|
||||
if canPrompt {
|
||||
|
|
@ -498,7 +499,12 @@ func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, re
|
|||
}
|
||||
}
|
||||
|
||||
// 2. Determine tag
|
||||
// Push unpushed commits (like gh pr create)
|
||||
if err := ensurePushed(opts, dir, remoteName); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Determine tag
|
||||
tag := opts.Tag
|
||||
if tag == "" {
|
||||
suggested := "v1.0.0"
|
||||
|
|
@ -549,7 +555,7 @@ func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, re
|
|||
}
|
||||
}
|
||||
|
||||
// 3. Offer to enable immutable releases
|
||||
// Offer to enable immutable releases
|
||||
immutableEnabled := checkImmutableReleases(client, host, owner, repo)
|
||||
if !immutableEnabled && canPrompt {
|
||||
enableImmutable, err := opts.Prompter.Confirm(
|
||||
|
|
@ -567,7 +573,7 @@ func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, re
|
|||
}
|
||||
}
|
||||
|
||||
// 4. Inform if not on default branch
|
||||
// Inform if not on default branch
|
||||
var currentBranch string
|
||||
if opts.GitClient != nil {
|
||||
branchGitClient := opts.GitClient.Copy()
|
||||
|
|
@ -581,7 +587,7 @@ func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, re
|
|||
fmt.Fprintf(opts.IO.ErrOut, "%s Publishing from branch %q (default is %q)\n", cs.WarningIcon(), currentBranch, defaultBranch)
|
||||
}
|
||||
|
||||
// 5. Confirm and create release
|
||||
// Confirm and create release
|
||||
if canPrompt {
|
||||
confirmed, err := opts.Prompter.Confirm(
|
||||
fmt.Sprintf("Create release %s with auto-generated notes?", tag), true)
|
||||
|
|
@ -622,6 +628,56 @@ func runPublishRelease(opts *PublishOptions, client *api.Client, host, owner, re
|
|||
return nil
|
||||
}
|
||||
|
||||
// ensurePushed checks whether the current branch has unpushed commits and
|
||||
// pushes them automatically, consistent with how gh pr create behaves.
|
||||
func ensurePushed(opts *PublishOptions, dir, remoteName string) error {
|
||||
if opts.GitClient == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
cs := opts.IO.ColorScheme()
|
||||
gitClient := opts.GitClient.Copy()
|
||||
gitClient.RepoDir = dir
|
||||
|
||||
ctx := context.Background()
|
||||
currentBranch, err := gitClient.CurrentBranch(ctx)
|
||||
if err != nil {
|
||||
return nil //nolint:nilerr // not on a branch (detached HEAD); skip push check
|
||||
}
|
||||
|
||||
// Count commits ahead of the push target (remote tracking branch).
|
||||
// If the branch has no upstream, rev-list will fail; we treat that as
|
||||
// "everything is unpushed" and push the whole branch.
|
||||
unpushed := 0
|
||||
revCmd, err := gitClient.Command(ctx, "rev-list", "--count", "@{push}..HEAD")
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not check unpushed commits: %w", err)
|
||||
}
|
||||
out, revErr := revCmd.Output()
|
||||
if revErr != nil {
|
||||
// @{push} not resolvable; branch has never been pushed
|
||||
unpushed = -1
|
||||
} else {
|
||||
n, parseErr := strconv.Atoi(strings.TrimSpace(string(out)))
|
||||
if parseErr != nil {
|
||||
return fmt.Errorf("could not parse unpushed commit count: %w", parseErr)
|
||||
}
|
||||
unpushed = n
|
||||
}
|
||||
|
||||
if unpushed == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
ref := fmt.Sprintf("HEAD:refs/heads/%s", currentBranch)
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Pushing %s to %s\n", cs.SuccessIcon(), currentBranch, remoteName)
|
||||
if err := gitClient.Push(ctx, remoteName, ref); err != nil {
|
||||
return fmt.Errorf("failed to push branch %s: %w", currentBranch, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// detectDefaultBranch returns the default branch of the remote repo via the API.
|
||||
func detectDefaultBranch(client *api.Client, host, owner, repo string) string {
|
||||
if client == nil {
|
||||
|
|
@ -866,9 +922,15 @@ func suggestNextTag(latest string) string {
|
|||
return fmt.Sprintf("%s%s.%s.%d", prefix, major, minor, patch+1)
|
||||
}
|
||||
|
||||
// gitHubRemote holds a detected GitHub remote and its local name.
|
||||
type gitHubRemote struct {
|
||||
Repo ghrepo.Interface
|
||||
RemoteName string
|
||||
}
|
||||
|
||||
// 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) {
|
||||
func detectGitHubRemote(gitClient *git.Client, dir string) (*gitHubRemote, error) {
|
||||
if gitClient == nil {
|
||||
return nil, nil
|
||||
}
|
||||
|
|
@ -883,7 +945,7 @@ func detectGitHubRemote(gitClient *git.Client, dir string) (ghrepo.Interface, er
|
|||
return nil, parseErr
|
||||
}
|
||||
if repo != nil {
|
||||
return repo, nil
|
||||
return &gitHubRemote{Repo: repo, RemoteName: "origin"}, nil
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -902,7 +964,7 @@ func detectGitHubRemote(gitClient *git.Client, dir string) (ghrepo.Interface, er
|
|||
return nil, parseErr
|
||||
}
|
||||
if repo != nil {
|
||||
return repo, nil
|
||||
return &gitHubRemote{Repo: repo, RemoteName: r.Name}, nil
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import (
|
|||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/MakeNowJust/heredoc"
|
||||
|
|
@ -22,13 +23,28 @@ import (
|
|||
|
||||
// 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")
|
||||
|
||||
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 {
|
||||
runGitInDir(t, dir, "remote", "add", name, url)
|
||||
runGitInDir(t, dir, "remote", "set-url", "--push", name, bareDir)
|
||||
}
|
||||
|
||||
runGitInDir(t, dir, "add", ".")
|
||||
runGitInDir(t, dir, "commit", "--allow-empty", "-m", "init")
|
||||
if _, ok := remoteURLs["origin"]; ok {
|
||||
runGitInDir(t, dir, "push", "origin", "main")
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1392,8 +1408,8 @@ func TestDetectGitHubRemote_UsesDir(t *testing.T) {
|
|||
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())
|
||||
assert.Equal(t, "monalisa", repo.Repo.RepoOwner())
|
||||
assert.Equal(t, "target-repo", repo.Repo.RepoName())
|
||||
}
|
||||
|
||||
func TestPublishRun_DirArgUsesTargetRemote(t *testing.T) {
|
||||
|
|
@ -1467,3 +1483,137 @@ func runGitInDir(t *testing.T, dir string, args ...string) {
|
|||
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)
|
||||
wantErr string
|
||||
wantStderr string
|
||||
}{
|
||||
{
|
||||
name: "no unpushed commits is a no-op",
|
||||
setup: func(_ *testing.T, _ string) {
|
||||
// initial commit already pushed by helper
|
||||
},
|
||||
},
|
||||
{
|
||||
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)))
|
||||
},
|
||||
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)))
|
||||
},
|
||||
wantStderr: "Pushing feature to origin",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
gitClient, workDir := newTestGitClientWithUpstream(t)
|
||||
tt.setup(t, workDir)
|
||||
|
||||
ios, _, _, stderr := iostreams.Test()
|
||||
ios.SetStdoutTTY(true)
|
||||
ios.SetStderrTTY(true)
|
||||
|
||||
opts := &PublishOptions{
|
||||
IO: ios,
|
||||
GitClient: gitClient,
|
||||
}
|
||||
|
||||
err := ensurePushed(opts, workDir, "origin")
|
||||
|
||||
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.verify != nil {
|
||||
tt.verify(t, workDir)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue