diff --git a/pkg/cmd/repo/rename/rename.go b/pkg/cmd/repo/rename/rename.go index bdb2707..1e68b95 100644 --- a/pkg/cmd/repo/rename/rename.go +++ b/pkg/cmd/repo/rename/rename.go @@ -132,6 +132,25 @@ func Run(ctx context.Context, opts *options) error { if has, _ := git.RemoteExists(opts.GitRunner, "", "origin"); !has { return nil } + + // CX2 guard: only rotate the CWD's origin if it actually points + // at the repo we just renamed. Without this check, running + // `repo rename foo -R owner/bar` from inside any unrelated git + // working tree silently overwrites that tree's origin to point at + // owner/bar — the kind of bug that loses work for users running + // shithub commands inside their own project directories. + cur, cerr := git.ResolveRemote("", "origin") + if cerr != nil { + fmt.Fprintf(opts.IO.ErrOut, "note: origin not updated (could not parse current URL): %v\n", cerr) + return nil + } + if !strings.EqualFold(cur.Owner, ref.Owner) || !strings.EqualFold(cur.Repo, ref.Name) { + fmt.Fprintf(opts.IO.ErrOut, + "note: origin not updated; this directory's origin points at %s, not %s/%s\n", + cur.String(), ref.Owner, ref.Name) + return nil + } + protocol := "https" if opts.GitProtocol != nil { protocol = opts.GitProtocol() diff --git a/pkg/cmd/repo/rename/rename_test.go b/pkg/cmd/repo/rename/rename_test.go index ea46287..511b5ba 100644 --- a/pkg/cmd/repo/rename/rename_test.go +++ b/pkg/cmd/repo/rename/rename_test.go @@ -7,9 +7,12 @@ import ( "encoding/json" "io" "net/http" + "os" + "strings" "testing" "github.com/tenseleyFlow/shithub-cli/internal/cmdutil/cmdutiltest" + "github.com/tenseleyFlow/shithub-cli/internal/git" "github.com/tenseleyFlow/shithub-cli/internal/repos" ) @@ -63,3 +66,117 @@ func TestRenameRejectsSameName(t *testing.T) { t.Fatal("expected error when renaming to same name") } } + +// initGitTreeWithOrigin spins up a fresh `git init` working tree and +// sets origin to the supplied URL. Returns the dir + a function to +// chdir back. Test must call the returned cleanup. +func initGitTreeWithOrigin(t *testing.T, originURL string) (dir string, restore func()) { + t.Helper() + gr, err := git.FromPath() + if err != nil { + t.Skipf("git not on PATH: %v", err) + } + dir = t.TempDir() + if err := git.Init(gr, dir, "trunk"); err != nil { + t.Fatalf("Init: %v", err) + } + if err := git.AddRemote(gr, dir, "origin", originURL); err != nil { + t.Fatalf("AddRemote: %v", err) + } + old, _ := os.Getwd() + if err := os.Chdir(dir); err != nil { + t.Fatalf("chdir: %v", err) + } + return dir, func() { _ = os.Chdir(old) } +} + +// TestRenameDoesNotClobberUnrelatedOrigin is the regression test for +// CX2: previously, `repo rename` overwrote the CWD's `origin` even +// when that origin pointed at a completely different repo. Run inside +// a git tree whose origin is `example.com:other/unrelated.git` and +// confirm origin survives the rename of `o/old -> o/newname`. +func TestRenameDoesNotClobberUnrelatedOrigin(t *testing.T) { + originalURL := "git@example.com:other/unrelated.git" + dir, restore := initGitTreeWithOrigin(t, originalURL) + defer restore() + + tf := cmdutiltest.New(t) + tf.Server.Handle(http.MethodPatch, "/api/v1/repos/o/old", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(repos.Repo{ + Name: "newname", FullName: "o/newname", Owner: repos.Owner{Login: "o"}, DefaultBranch: "trunk", + }) + }) + gr, _ := git.FromPath() + opts := &options{ + IO: tf.IOStreams, + Prompter: tf.Prompt, + HTTPClient: tf.Factory.HTTPClient, + DefaultHost: tf.Factory.DefaultHost, + GitProtocol: tf.Factory.GitProtocol, + GitRunner: gr, + RepoArg: "o/old", + NewName: "newname", + Yes: true, + } + if err := Run(context.Background(), opts); err != nil { + t.Fatalf("Run: %v", err) + } + + // origin URL must be unchanged. + got, err := git.ResolveRemote(dir, "origin") + if err != nil { + t.Fatalf("ResolveRemote: %v", err) + } + if got.Owner != "other" || got.Repo != "unrelated" { + t.Errorf("origin was clobbered: got %s, want other/unrelated", got.String()) + } + + // And the user must see why we declined to rotate. + stderr := tf.ErrOut.String() + if !strings.Contains(stderr, "origin not updated") { + t.Errorf("expected 'origin not updated' notice; got: %s", stderr) + } +} + +// TestRenameRotatesMatchingOrigin: the happy path we still want — +// inside a clone of the repo being renamed, origin should rotate to +// the new URL. +func TestRenameRotatesMatchingOrigin(t *testing.T) { + dir, restore := initGitTreeWithOrigin(t, "https://shithub.test/o/old.git") + defer restore() + + tf := cmdutiltest.New(t) + tf.Server.Handle(http.MethodPatch, "/api/v1/repos/o/old", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(repos.Repo{ + Name: "newname", FullName: "o/newname", Owner: repos.Owner{Login: "o"}, DefaultBranch: "trunk", + CloneURL: "https://shithub.test/o/newname.git", + }) + }) + gr, _ := git.FromPath() + opts := &options{ + IO: tf.IOStreams, + Prompter: tf.Prompt, + HTTPClient: tf.Factory.HTTPClient, + DefaultHost: tf.Factory.DefaultHost, + GitProtocol: tf.Factory.GitProtocol, + GitRunner: gr, + RepoArg: "o/old", + NewName: "newname", + Yes: true, + } + if err := Run(context.Background(), opts); err != nil { + t.Fatalf("Run: %v", err) + } + got, err := git.ResolveRemote(dir, "origin") + if err != nil { + t.Fatalf("ResolveRemote: %v", err) + } + if got.Owner != "o" || got.Repo != "newname" { + t.Errorf("origin did not rotate: got %s, want o/newname", got.String()) + } + if !strings.Contains(tf.ErrOut.String(), "Updated origin") { + t.Errorf("expected 'Updated origin' line; got: %s", tf.ErrOut.String()) + } +}