From 2767531cb2a22ea1c1adbdbc3d46d6f9183878b3 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:38:55 -0400 Subject: [PATCH 1/8] pulls/merge: accept merge_commit_sha alongside sha (B1) --- internal/pulls/merge.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/internal/pulls/merge.go b/internal/pulls/merge.go index 1fafc0d..e243f5d 100644 --- a/internal/pulls/merge.go +++ b/internal/pulls/merge.go @@ -31,10 +31,25 @@ type MergeInput struct { } // MergeResult is the response from a successful merge. +// +// gh-compat: the canonical field is `merge_commit_sha` (GitHub's +// PUT /merges response). shithub's earlier API surface used a bare +// `sha`; we accept both so the success line ("Merged PR #N at ") +// stays populated regardless of which name the server emits. The +// CommitSHA() accessor prefers the gh-compat form. type MergeResult struct { - SHA string `json:"sha"` - Merged bool `json:"merged"` - Message string `json:"message,omitempty"` + SHA string `json:"sha,omitempty"` + MergeCommitSHA string `json:"merge_commit_sha,omitempty"` + Merged bool `json:"merged"` + Message string `json:"message,omitempty"` +} + +// CommitSHA returns the merge commit hash, tolerating both wire names. +func (m *MergeResult) CommitSHA() string { + if m.MergeCommitSHA != "" { + return m.MergeCommitSHA + } + return m.SHA } // Merge calls PUT /pulls/{n}/merge. The --admin override is communicated From 937d43d3c0aabad208386d6a6535a706b1a0d843 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:38:55 -0400 Subject: [PATCH 2/8] pulls/merge: test CommitSHA gh-compat decode --- internal/pulls/merge_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/internal/pulls/merge_test.go b/internal/pulls/merge_test.go index 89948c8..483bcae 100644 --- a/internal/pulls/merge_test.go +++ b/internal/pulls/merge_test.go @@ -38,6 +38,37 @@ func TestMergeSquash(t *testing.T) { } } +// TestMergeCommitSHA_GhCompat confirms the decoder accepts gh's +// `merge_commit_sha` field and exposes it through CommitSHA(). The +// CLI success line went blank on shithub server builds that emitted +// only this field (B-audit B1). +func TestMergeCommitSHA_GhCompat(t *testing.T) { + srv := fakeapi.New(t) + srv.Handle(http.MethodPut, "/api/v1/repos/o/r/pulls/1/merge", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"merged":true,"merge_commit_sha":"feedface"}`)) + }) + + c := NewClient(srv.NewClient()) + out, err := c.Merge(context.Background(), "o", "r", 1, MergeInput{MergeMethod: MergeMerge}, false) + if err != nil { + t.Fatalf("Merge: %v", err) + } + if got := out.CommitSHA(); got != "feedface" { + t.Errorf("CommitSHA: got %q, want feedface (raw: %+v)", got, out) + } +} + +// TestMergeCommitSHA_PrefersGhCompat: when both fields are populated +// (legacy + gh-compat), the gh-compat name wins so the CLI doesn't +// display two different shas across server versions. +func TestMergeCommitSHA_PrefersGhCompat(t *testing.T) { + r := &MergeResult{SHA: "legacy", MergeCommitSHA: "ghcompat"} + if r.CommitSHA() != "ghcompat" { + t.Errorf("CommitSHA: got %q, want ghcompat", r.CommitSHA()) + } +} + func TestMergeAdminHeader(t *testing.T) { srv := fakeapi.New(t) var seen string From e89f416e206a0c5270bd273e5fe77f15e2a80f1d Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:38:55 -0400 Subject: [PATCH 3/8] pr/merge: use MergeResult.CommitSHA for success line --- pkg/cmd/pr/merge/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 0b6e3bb..e2d3854 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -201,7 +201,7 @@ func Run(ctx context.Context, opts *options) error { } return fmt.Errorf("pr merge: %s", msg) } - fmt.Fprintf(opts.IO.ErrOut, "%s Merged PR #%d (%s) at %s\n", opts.IO.SuccessIcon(), ref.Number, method, shortSHA(result.SHA)) + fmt.Fprintf(opts.IO.ErrOut, "%s Merged PR #%d (%s) at %s\n", opts.IO.SuccessIcon(), ref.Number, method, shortSHA(result.CommitSHA())) if opts.DeleteBranch { if err := deleteBranch(ctx, pc, ref, pr); err != nil { From 323a7634f072e199ec271b04bb91c91938bdfb3e Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:39:02 -0400 Subject: [PATCH 4/8] repo view: expose forkCount/stargazerCount/watcherCount aliases (B2) --- pkg/cmd/repo/view/export.go | 59 ++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/repo/view/export.go b/pkg/cmd/repo/view/export.go index 93ac930..e24de2d 100644 --- a/pkg/cmd/repo/view/export.go +++ b/pkg/cmd/repo/view/export.go @@ -17,12 +17,18 @@ func (exporter) Fields() []string { return exportableFields } // exportableFields is the canonical projection catalog. Order matters for // `--json` (no value) listings; keep alphabetical. +// +// `forkCount`/`stargazerCount`/`watcherCount` are the gh-compat aliases +// (B2): gh's `gh repo view --json` exposes those names, and ported +// scripts break without them. The legacy `forks`/`stargazers`/`watchers` +// stay populated for one release cycle. var exportableFields = []string{ "archived", "createdAt", "defaultBranch", "description", "fork", + "forkCount", "forks", "fullName", "homepage", @@ -36,11 +42,13 @@ var exportableFields = []string{ "owner", "pushedAt", "size", + "stargazerCount", "stargazers", "topics", "updatedAt", "url", "visibility", + "watcherCount", "watchers", } @@ -62,30 +70,33 @@ func (exporter) Filter(v any) (any, error) { } } return map[string]any{ - "archived": r.Archived, - "createdAt": r.CreatedAt, - "defaultBranch": r.DefaultBranch, - "description": r.Description, - "fork": r.Fork, - "forks": r.Forks, - "fullName": r.FullName, - "homepage": r.Homepage, - "id": r.ID, - "isPrivate": r.Private, - "isTemplate": r.IsTemplate, - "language": r.Language, - "license": license, - "name": r.Name, - "openIssues": r.OpenIssues, - "owner": map[string]any{"login": r.Owner.Login, "type": r.Owner.Type}, - "pushedAt": r.PushedAt, - "size": r.Size, - "stargazers": r.Stargazers, - "topics": r.Topics, - "updatedAt": r.UpdatedAt, - "url": r.HTMLURL, - "visibility": visibilityField(r), - "watchers": r.Watchers, + "archived": r.Archived, + "createdAt": r.CreatedAt, + "defaultBranch": r.DefaultBranch, + "description": r.Description, + "fork": r.Fork, + "forkCount": r.Forks, + "forks": r.Forks, + "fullName": r.FullName, + "homepage": r.Homepage, + "id": r.ID, + "isPrivate": r.Private, + "isTemplate": r.IsTemplate, + "language": r.Language, + "license": license, + "name": r.Name, + "openIssues": r.OpenIssues, + "owner": map[string]any{"login": r.Owner.Login, "type": r.Owner.Type}, + "pushedAt": r.PushedAt, + "size": r.Size, + "stargazerCount": r.Stargazers, + "stargazers": r.Stargazers, + "topics": r.Topics, + "updatedAt": r.UpdatedAt, + "url": r.HTMLURL, + "visibility": visibilityField(r), + "watcherCount": r.Watchers, + "watchers": r.Watchers, }, nil } From 6c28dc4bfd388190390b8f050a5279fba23db576 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:39:02 -0400 Subject: [PATCH 5/8] repo view: test gh-compat count aliases in JSON projection --- pkg/cmd/repo/view/view_test.go | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index 915d003..ec0163a 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -100,6 +100,41 @@ func TestViewJSONExport(t *testing.T) { } } +// TestViewJSONGhCompatAliases confirms B2: ported gh scripts requesting +// `forkCount`, `stargazerCount`, `watcherCount` see the same numbers as +// the legacy `forks`/`stargazers`/`watchers`. Without the aliases the +// JSON projection emits the field with a `null` value. +func TestViewJSONGhCompatAliases(t *testing.T) { + tf := cmdutiltest.New(t) + tf.Server.RegisterJSON(http.MethodGet, "/api/v1/repos/octo/hello", 200, repos.Repo{ + Name: "hello", + FullName: "octo/hello", + Owner: repos.Owner{Login: "octo"}, + DefaultBranch: "trunk", + Stargazers: 7, + Forks: 3, + Watchers: 11, + }) + + opts := &options{ + IO: tf.IOStreams, + HTTPClient: tf.Factory.HTTPClient, + DefaultHost: tf.Factory.DefaultHost, + RepoArg: "octo/hello", + } + opts.Exporter.JSONFields = "stargazerCount,forkCount,watcherCount" + opts.Exporter.JSONSet = true + if err := Run(context.Background(), opts); err != nil { + t.Fatalf("Run: %v", err) + } + out := tf.Out.String() + for _, want := range []string{`"stargazerCount":7`, `"forkCount":3`, `"watcherCount":11`} { + if !strings.Contains(out, want) { + t.Errorf("--json projection missing %q; got %s", want, out) + } + } +} + func TestViewRejectsArgAndFlagCombo(t *testing.T) { tf := cmdutiltest.New(t) opts := &options{ From ecf19bbf9b8bec57098e7a858a7e47af0ffafd17 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:39:02 -0400 Subject: [PATCH 6/8] repo list: expose forkCount/stargazerCount aliases (B2) --- pkg/cmd/repo/list/export.go | 41 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/repo/list/export.go b/pkg/cmd/repo/list/export.go index ba7a987..f5e95b1 100644 --- a/pkg/cmd/repo/list/export.go +++ b/pkg/cmd/repo/list/export.go @@ -15,12 +15,16 @@ type exporter struct{} func (exporter) Fields() []string { return listExportableFields } +// listExportableFields mirrors view's catalog (subset) with the same +// gh-compat aliases: forkCount/stargazerCount sit beside the legacy +// forks/stargazers for one release cycle. See view/export.go. var listExportableFields = []string{ "archived", "createdAt", "defaultBranch", "description", "fork", + "forkCount", "forks", "fullName", "isPrivate", @@ -28,6 +32,7 @@ var listExportableFields = []string{ "name", "owner", "pushedAt", + "stargazerCount", "stargazers", "topics", "updatedAt", @@ -43,23 +48,25 @@ func (exporter) Filter(v any) (any, error) { out := make([]map[string]any, 0, len(rs)) for _, r := range rs { out = append(out, map[string]any{ - "archived": r.Archived, - "createdAt": r.CreatedAt, - "defaultBranch": r.DefaultBranch, - "description": r.Description, - "fork": r.Fork, - "forks": r.Forks, - "fullName": r.FullName, - "isPrivate": r.Private, - "language": r.Language, - "name": r.Name, - "owner": map[string]any{"login": r.Owner.Login, "type": r.Owner.Type}, - "pushedAt": r.PushedAt, - "stargazers": r.Stargazers, - "topics": r.Topics, - "updatedAt": r.UpdatedAt, - "url": r.HTMLURL, - "visibility": visibilityField(&r), + "archived": r.Archived, + "createdAt": r.CreatedAt, + "defaultBranch": r.DefaultBranch, + "description": r.Description, + "fork": r.Fork, + "forkCount": r.Forks, + "forks": r.Forks, + "fullName": r.FullName, + "isPrivate": r.Private, + "language": r.Language, + "name": r.Name, + "owner": map[string]any{"login": r.Owner.Login, "type": r.Owner.Type}, + "pushedAt": r.PushedAt, + "stargazerCount": r.Stargazers, + "stargazers": r.Stargazers, + "topics": r.Topics, + "updatedAt": r.UpdatedAt, + "url": r.HTMLURL, + "visibility": visibilityField(&r), }) } return out, nil From 29dbd1ac8ad789ed5d5c8266a1225cb4b7a3ce58 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:39:08 -0400 Subject: [PATCH 7/8] repo fork: pre-flight own-repo check with clear error (B4) --- pkg/cmd/repo/fork/fork.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index fda3903..8aaedcb 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -117,6 +117,17 @@ func Run(ctx context.Context, opts *options) error { } rc := repos.NewClient(client) + // B4: detect the own-repo case up front so the user gets a clear + // message instead of the server's generic 422. We only consult the + // authenticated user when the caller hasn't passed --org (an org + // destination is, by definition, not "yourself"). + if opts.Org == "" { + me, merr := client.CurrentUser(ctx) + if merr == nil && strings.EqualFold(me.Login, ref.Owner) { + return fmt.Errorf("repo fork: cannot fork your own repository (%s/%s); use --org to fork into an organization", ref.Owner, ref.Name) + } + } + in := repos.ForkInput{ Organization: opts.Org, Name: opts.Name, From 4c745eb33f9890ee886e933386bf754fc91f5725 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 14:39:08 -0400 Subject: [PATCH 8/8] repo fork: test own-repo rejection + --org bypass --- pkg/cmd/repo/fork/fork_test.go | 63 ++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 7e3039c..e8486a1 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -110,6 +110,69 @@ func TestForkOrgFlag(t *testing.T) { } } +// TestForkRejectsOwnRepo covers B4: forking your own repo gets a clear +// pre-flight error citing the owner — not the server's generic 422. +// The forks endpoint is intentionally not stubbed so the test fails +// loudly if the pre-check ever regresses and we round-trip to the API. +func TestForkRejectsOwnRepo(t *testing.T) { + tf := cmdutiltest.New(t) + tf.Server.RegisterJSON(http.MethodGet, "/api/v1/user", 200, map[string]any{ + "id": 1, "login": "octo", + }) + + opts := &options{ + IO: tf.IOStreams, + Prompter: tf.Prompt, + HTTPClient: tf.Factory.HTTPClient, + DefaultHost: tf.Factory.DefaultHost, + GitProtocol: tf.Factory.GitProtocol, + RepoArg: "octo/hello", + RemoteName: "origin", + } + err := Run(context.Background(), opts) + if err == nil { + t.Fatal("expected error when forking own repo, got nil") + } + if !strings.Contains(err.Error(), "cannot fork your own repository") { + t.Errorf("error message: %v", err) + } + if !strings.Contains(err.Error(), "octo/hello") { + t.Errorf("error should cite the repo: %v", err) + } +} + +// TestForkOwnRepoIntoOrgAllowed: --org bypasses the own-repo guard, +// since forking your own repo into a different org is a real workflow. +func TestForkOwnRepoIntoOrgAllowed(t *testing.T) { + tf := cmdutiltest.New(t) + tf.Server.RegisterJSON(http.MethodGet, "/api/v1/user", 200, map[string]any{ + "id": 1, "login": "octo", + }) + called := false + tf.Server.Handle(http.MethodPost, "/api/v1/repos/octo/hello/forks", func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(repos.Repo{Name: "hello", FullName: "acme/hello", Owner: repos.Owner{Login: "acme"}, Fork: true}) + }) + + opts := &options{ + IO: tf.IOStreams, + Prompter: tf.Prompt, + HTTPClient: tf.Factory.HTTPClient, + DefaultHost: tf.Factory.DefaultHost, + GitProtocol: tf.Factory.GitProtocol, + RepoArg: "octo/hello", + Org: "acme", + RemoteName: "origin", + } + if err := Run(context.Background(), opts); err != nil { + t.Fatalf("Run: %v", err) + } + if !called { + t.Error("expected forks endpoint to be hit when --org is set") + } +} + func TestForkCloneWithUpstream(t *testing.T) { if runtime.GOOS == "windows" { // Windows TempDir paths look like `C:\...` which dirFromCloneURL