Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
26 changes: 24 additions & 2 deletions acceptance/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func TestRunWatch_SHA(t *testing.T) {

result := binary.RunCLI(t, binary.RunOpts{
Binary: binaryPath,
Args: []string{"run", "watch", "--sha", "abc1234",
Args: []string{"run", "watch", "--sha", "abc1234def5678abcdef1234567890abcdef1234",
"--project", watchSlug, "--branch", "main"},
Env: env.Environ(),
WorkDir: t.TempDir(),
Expand All @@ -245,7 +245,7 @@ func TestRunWatch_SHA_NotFound(t *testing.T) {

result := binary.RunCLI(t, binary.RunOpts{
Binary: binaryPath,
Args: []string{"run", "watch", "--sha", "deadbeef",
Args: []string{"run", "watch", "--sha", "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
"--project", watchSlug, "--branch", "main"},
Env: env.Environ(),
WorkDir: t.TempDir(),
Expand All @@ -255,6 +255,28 @@ func TestRunWatch_SHA_NotFound(t *testing.T) {
assert.Check(t, cmp.Contains(result.Stderr, "No run found"), "stderr: %s", result.Stderr)
}

// --- --sha: short SHA outside a git repo → exit 2 (bad arguments) ---

func TestRunWatch_SHA_ShortOutsideGitRepo(t *testing.T) {
fake := fakes.NewCircleCI(t)
addProjectBySlug(fake, watchSlug, watchProjectID)

env := testenv.New(t)
env.Token = testToken
env.CircleCIURL = fake.URL()

result := binary.RunCLI(t, binary.RunOpts{
Binary: binaryPath,
Args: []string{"run", "watch", "--sha", "abc1234",
"--project", watchSlug, "--branch", "main"},
Env: env.Environ(),
WorkDir: t.TempDir(),
})

assert.Equal(t, result.ExitCode, 2, "stderr: %s", result.Stderr) // ExitBadArguments
assert.Check(t, cmp.Contains(result.Stderr, "local git repository is not accessible"), "stderr: %s", result.Stderr)
}

// --- --failfast: exit immediately when a job fails, without waiting for the rest of the run ---

func TestRunWatch_FailFast(t *testing.T) {
Expand Down
32 changes: 30 additions & 2 deletions internal/cmd/run/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,16 @@ func runWatch(ctx context.Context, client *apiclient.Client, args []string, proj
if needsGit {
info, err := gitremote.Detect()
if err != nil {
return cmdutil.GitDetectErr(err, "Or specify --project and --branch explicitly")

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.

Could the short-SHA problem be fixed more simply by doing a client-side prefix match on the SHA returned by the API, rather than expanding locally via go-git? Something like strings.HasPrefix(pipeline.GitRevision, sha) instead of a server-side exact filter. That would remove the local git repo dependency entirely — no new error sentinels, no os.Chdir in tests, works in shallow clones and CI environments without access to the repo.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Possibly! If I'm understanding correctly, the drawback of that approach would be if the run you're looking for isn't in the first page of runs returned, the client side check would return with nothing even though the run does exist (unless we keep paging, which doesn't seem great). I can keep poking at this if you feel strongly about the approach!

Regardless, I did a bit of cleanup on the error sentinels (removing ErrSHANotHex) and the tests (removing the need for os.Chdir).

suggestion := "Or specify --project and --branch explicitly"
if sha != "" {
suggestion = "Or specify --project explicitly"
}
return cmdutil.GitDetectErr(err, suggestion)
}
if projectSlug == "" {
projectSlug = info.Slug
}
if branch == "" {
if branch == "" && sha == "" {
branch = info.Branch
}
}
Expand Down Expand Up @@ -234,6 +238,30 @@ func waitForRunBySHA(ctx context.Context, client *apiclient.Client, projectSlug,
interval := 5 * time.Second
printed := false

expanded, expandErr := gitremote.ExpandSHA(sha)
switch {
case expandErr == nil:
sha = expanded
case errors.Is(expandErr, gitremote.ErrSHANotHex):
return nil, clierrors.New("run.invalid_sha_format", "Invalid SHA format",
fmt.Sprintf("%q does not look like a commit SHA; expected hex characters only.", sha)).
WithSuggestions("Pass a hex commit SHA, e.g. from 'git log --oneline'").
WithExitCode(clierrors.ExitBadArguments)
case errors.Is(expandErr, gitremote.ErrSHARepoInaccessible):
return nil, clierrors.New("run.sha_unresolvable", "Could not resolve short SHA",
fmt.Sprintf("Cannot expand %q: local git repository is not accessible.", sha)).
WithSuggestions("Pass the full 40-character SHA to skip local resolution").
WithExitCode(clierrors.ExitBadArguments)
case errors.Is(expandErr, gitremote.ErrSHANotFound):
return nil, clierrors.New("run.invalid_sha", "Commit not found",
fmt.Sprintf("Commit %q does not exist in the local repository.", sha)).
WithSuggestions(
"Check the SHA is correct: git log --oneline",
"If using a shallow clone, pass the full 40-character SHA obtained from the remote",
).
WithExitCode(clierrors.ExitNotFound)
}

filter := fmt.Sprintf("pipeline.git.revision == %q", sha)
if branch != "" {
filter += fmt.Sprintf(" and pipeline.git.branch == %q", branch)
Expand Down
36 changes: 36 additions & 0 deletions internal/gitremote/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ var (
sshProtoRemote = regexp.MustCompile(`^ssh://git@([^/]+)/([^/]+)/(.+?)(?:\.git)?$`)
// matches https://github.com/org/repo.git
httpsRemote = regexp.MustCompile(`^https?://([^/]+)/([^/]+)/(.+?)(?:\.git)?$`)

hexRE = regexp.MustCompile(`^[0-9a-fA-F]+$`)
)

var (
// ErrSHANotHex is returned by ExpandSHA when the input contains non-hex
// characters (e.g. a branch name passed by mistake).
ErrSHANotHex = errors.New("input is not a valid hex SHA")
// ErrSHARepoInaccessible is returned by ExpandSHA when the local git
// repository cannot be opened, so a short SHA cannot be expanded.
ErrSHARepoInaccessible = errors.New("local git repository is not accessible")
// ErrSHANotFound is returned by ExpandSHA when the short SHA does not
// resolve to any object in the local repository.
ErrSHANotFound = errors.New("SHA not found in local repository")
)

// DetectNamespace returns the organization name (namespace) from the git remote.
Expand Down Expand Up @@ -283,6 +297,28 @@ func gitCurrentBranch(repo *git.Repository) (string, error) {
return head.Name().Short(), nil
}

// ExpandSHA attempts to resolve an abbreviated git SHA to its full 40-character
// form. It returns the (possibly expanded) SHA and nil on success, or the
// original input and one of ErrSHANotHex, ErrSHARepoInaccessible, or
// ErrSHANotFound on failure.
func ExpandSHA(sha string) (string, error) {
if !hexRE.MatchString(sha) {
return sha, ErrSHANotHex
}
if len(sha) == 40 {
return sha, nil
}
repo, err := openRepo()
if err != nil {
return sha, ErrSHARepoInaccessible
}
hash, err := repo.ResolveRevision(plumbing.Revision(sha))
if err != nil {
return sha, ErrSHANotFound
}
return hash.String(), nil
}

// gitDefaultBranch returns the short name of the remote default branch (e.g.
// "main"), read from the symbolic ref refs/remotes/origin/HEAD. This is the
// "origin/"-stripped equivalent of `git rev-parse --abbrev-ref origin/HEAD`.
Expand Down
78 changes: 78 additions & 0 deletions internal/gitremote/detect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
package gitremote

import (
"errors"
"os"
"path/filepath"
"testing"
"time"

"github.com/go-git/go-git/v6"
"github.com/go-git/go-git/v6/config"
Expand Down Expand Up @@ -175,6 +177,82 @@ func TestDetect_SurfacesMalformedInfoYml(t *testing.T) {
assert.Check(t, err != nil, "expected Detect to surface a malformed info.yml rather than fall back")
}

func TestExpandSHA(t *testing.T) {
origDir, err := os.Getwd()
assert.NilError(t, err)

t.Run("already 40 hex chars returns input unchanged", func(t *testing.T) {
full := "1234567890abcdef1234567890abcdef12345678"
got, err := ExpandSHA(full)
assert.NilError(t, err)
assert.Check(t, cmp.Equal(got, full))
})

t.Run("non-hex input returns ErrSHANotHex", func(t *testing.T) {
_, err := ExpandSHA("main")
assert.Check(t, errors.Is(err, ErrSHANotHex), "got: %v", err)
})

t.Run("repo inaccessible returns ErrSHARepoInaccessible", func(t *testing.T) {
dir := t.TempDir()
assert.NilError(t, os.Chdir(dir))
t.Cleanup(func() { _ = os.Chdir(origDir) })

_, err := ExpandSHA("abc1234")
assert.Check(t, errors.Is(err, ErrSHARepoInaccessible), "got: %v", err)
})

t.Run("SHA not found in repo returns ErrSHANotFound", func(t *testing.T) {
dir := t.TempDir()
_, err := git.PlainInit(dir, false)
assert.NilError(t, err)
assert.NilError(t, os.Chdir(dir))
t.Cleanup(func() { _ = os.Chdir(origDir) })

_, err = ExpandSHA("deadbeef")
assert.Check(t, errors.Is(err, ErrSHANotFound), "got: %v", err)
})

t.Run("short SHA expands to full 40-char hash", func(t *testing.T) {
dir := t.TempDir()
fullHash := initRepoWithCommit(t, dir)
assert.NilError(t, os.Chdir(dir))
t.Cleanup(func() { _ = os.Chdir(origDir) })

short := fullHash[:7]
got, err := ExpandSHA(short)
assert.NilError(t, err)
assert.Check(t, cmp.Equal(got, fullHash))
})
}

// initRepoWithCommit initialises a new git repository in dir, adds one commit,
// and returns the full 40-character SHA of that commit.
func initRepoWithCommit(t *testing.T, dir string) string {
t.Helper()
repo, err := git.PlainInit(dir, false)
assert.NilError(t, err)

err = os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0o644)
assert.NilError(t, err)

wt, err := repo.Worktree()
assert.NilError(t, err)

_, err = wt.Add("README")
assert.NilError(t, err)

hash, err := wt.Commit("initial commit", &git.CommitOptions{
Author: &object.Signature{
Name: "Test",
Email: "test@example.com",
When: time.Now(),
},
})
assert.NilError(t, err)
return hash.String()
}

// Sanity check that DetectFromRemote does not consult info.yml — used by
// `project link` to avoid short-circuiting against an existing entry.
func TestDetectFromRemote_IgnoresInfoYml(t *testing.T) {
Expand Down