I4 [CRIT/CLI] Cross-NS verb redirects + subcommand typo suggestions (audit-I5 + audit-I6)

internal/cmdutil/parent.go

@@ -4,6 +4,7 @@ package cmdutil
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/spf13/cobra"
 )
@@ -15,15 +16,113 @@ import (
 // the call through the parent's help text and exits 0, which made
 // typos like `shithub org INVALID` falsely look successful (E-audit E16).
 //
+// audit-I5: subcommand-level "did you mean?" suggestions. cobra's
+// suggestion machinery only fires at the root command's auto-error
+// path; once execution reaches a parent's RunE the suggestion logic
+// is bypassed. We walk the parent's children and Levenshtein-match
+// the bad arg ourselves so `shithub pr klose` says "Did you mean
+// close?" the same way `shithub reepo` already does.
+//
 // gh emits "unknown command \"INVALID\" for \"gh org\""; we mirror that
 // shape and let the user discover the available subcommands via the
-// suggestion line cobra appends automatically.
+// suggestion line.
 func ParentRunE() func(*cobra.Command, []string) error {
 	return func(c *cobra.Command, args []string) error {
 		if len(args) == 0 {
 			return c.Help()
 		}
-		return fmt.Errorf("unknown command %q for %q; run '%s --help' for usage",
+		base := fmt.Sprintf("unknown command %q for %q; run '%s --help' for usage",
 			args[0], c.CommandPath(), c.CommandPath())
+		if suggestion := suggestSubcommand(c, args[0]); suggestion != "" {
+			return fmt.Errorf("%s\n\nDid you mean this?\n\t%s", base, suggestion)
+		}
+		return fmt.Errorf("%s", base)
+	}
+}
+
+// suggestSubcommand returns the best-matching subcommand name when
+// `want` is within Levenshtein-distance 2 of any visible child, or
+// shares a 3+ char prefix. Returns "" when nothing's close enough.
+//
+// We don't reuse cobra's internal SuggestionsFor because it's
+// unexported on *Command outside the auto-error path. The
+// implementation is small and matches cobra's `SuggestionsMinimumDistance=2`
+// default behavior.
+func suggestSubcommand(parent *cobra.Command, want string) string {
+	type scored struct {
+		name string
+		dist int
+	}
+	var best scored
+	best.dist = -1
+	wantLower := strings.ToLower(want)
+	for _, sub := range parent.Commands() {
+		if sub.Hidden {
+			continue
+		}
+		name := sub.Name()
+		lower := strings.ToLower(name)
+		// Prefix match wins outright — `shithub repo cre` → `create`.
+		if len(want) >= 3 && strings.HasPrefix(lower, wantLower) {
+			return name
+		}
+		d := levenshtein(wantLower, lower)
+		if d <= 2 && (best.dist == -1 || d < best.dist) {
+			best = scored{name: name, dist: d}
+		}
+		// Also check aliases.
+		for _, a := range sub.Aliases {
+			al := strings.ToLower(a)
+			if len(want) >= 3 && strings.HasPrefix(al, wantLower) {
+				return name
+			}
+			d := levenshtein(wantLower, al)
+			if d <= 2 && (best.dist == -1 || d < best.dist) {
+				best = scored{name: name, dist: d}
+			}
+		}
+	}
+	return best.name
+}
+
+// levenshtein returns the edit distance between a and b. Tight loop
+// suitable for the ~10-15 children typical of a shithub parent.
+func levenshtein(a, b string) int {
+	if a == b {
+		return 0
+	}
+	if len(a) == 0 {
+		return len(b)
+	}
+	if len(b) == 0 {
+		return len(a)
+	}
+	prev := make([]int, len(b)+1)
+	curr := make([]int, len(b)+1)
+	for j := range prev {
+		prev[j] = j
+	}
+	for i := 1; i <= len(a); i++ {
+		curr[0] = i
+		for j := 1; j <= len(b); j++ {
+			cost := 1
+			if a[i-1] == b[j-1] {
+				cost = 0
+			}
+			curr[j] = min3(curr[j-1]+1, prev[j]+1, prev[j-1]+cost)
+		}
+		prev, curr = curr, prev
+	}
+	return prev[len(b)]
+}
+
+func min3(a, b, c int) int {
+	m := a
+	if b < m {
+		m = b
+	}
+	if c < m {
+		m = c
 	}
+	return m
 }

internal/cmdutil/parent_test.go

@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package cmdutil_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+
+	"github.com/tenseleyFlow/shithub-cli/internal/cmdutil"
+)
+
+// TestParentRunE_NoArgsPrintsHelp pins the bare-invocation path.
+func TestParentRunE_NoArgsPrintsHelp(t *testing.T) {
+	parent := &cobra.Command{Use: "shithub", RunE: cmdutil.ParentRunE()}
+	parent.SetOut(&strings.Builder{})
+	parent.SetErr(&strings.Builder{})
+	parent.AddCommand(&cobra.Command{Use: "create"})
+	if err := parent.RunE(parent, nil); err != nil {
+		t.Fatalf("bare invoke should print help and return nil; got %v", err)
+	}
+}
+
+// TestParentRunE_UnknownSubcommandErrors pins exit-code-1 path.
+func TestParentRunE_UnknownSubcommandErrors(t *testing.T) {
+	parent := &cobra.Command{Use: "parent", RunE: cmdutil.ParentRunE()}
+	parent.AddCommand(&cobra.Command{Use: "create"})
+	err := parent.RunE(parent, []string{"totally-unknown-name"})
+	if err == nil {
+		t.Fatal("unknown subcommand should return non-nil error")
+	}
+	if !strings.Contains(err.Error(), `unknown command "totally-unknown-name"`) {
+		t.Errorf("error should quote the bad arg: %v", err)
+	}
+}
+
+// TestParentRunE_LevenshteinSuggestion pins audit-I5: typos at the
+// subcommand level get a "Did you mean?" line. Pre-fix only the root
+// command's auto-error path had suggestions; once execution reached
+// a parent's RunE, the suggestion machinery was bypassed.
+func TestParentRunE_LevenshteinSuggestion(t *testing.T) {
+	parent := &cobra.Command{Use: "pr", RunE: cmdutil.ParentRunE()}
+	parent.AddCommand(&cobra.Command{Use: "close"})
+	parent.AddCommand(&cobra.Command{Use: "create"})
+	parent.AddCommand(&cobra.Command{Use: "review"})
+
+	err := parent.RunE(parent, []string{"klose"})
+	if err == nil {
+		t.Fatal("expected error from typo")
+	}
+	if !strings.Contains(err.Error(), "Did you mean this?") {
+		t.Errorf("missing suggestion preface: %v", err)
+	}
+	if !strings.Contains(err.Error(), "close") {
+		t.Errorf("expected to suggest 'close': %v", err)
+	}
+}
+
+// TestParentRunE_PrefixSuggestion: 3+ char prefix match wins over
+// Levenshtein. `shithub repo cre` → `create`.
+func TestParentRunE_PrefixSuggestion(t *testing.T) {
+	parent := &cobra.Command{Use: "repo", RunE: cmdutil.ParentRunE()}
+	parent.AddCommand(&cobra.Command{Use: "create"})
+	parent.AddCommand(&cobra.Command{Use: "delete"})
+
+	err := parent.RunE(parent, []string{"cre"})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if !strings.Contains(err.Error(), "create") {
+		t.Errorf("prefix 'cre' should suggest 'create': %v", err)
+	}
+}
+
+// TestParentRunE_NoCloseMatchSkipsSuggestion: a typo far from every
+// child should NOT emit a misleading suggestion.
+func TestParentRunE_NoCloseMatchSkipsSuggestion(t *testing.T) {
+	parent := &cobra.Command{Use: "pr", RunE: cmdutil.ParentRunE()}
+	parent.AddCommand(&cobra.Command{Use: "close"})
+	parent.AddCommand(&cobra.Command{Use: "create"})
+
+	err := parent.RunE(parent, []string{"absolutely-nothing-like-our-verbs"})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if strings.Contains(err.Error(), "Did you mean this?") {
+		t.Errorf("should not suggest for far-off typo: %v", err)
+	}
+}
+
+// TestParentRunE_HiddenChildrenIgnored: hidden subcommands don't
+// pollute the suggestion candidate set.
+func TestParentRunE_HiddenChildrenIgnored(t *testing.T) {
+	parent := &cobra.Command{Use: "x", RunE: cmdutil.ParentRunE()}
+	parent.AddCommand(&cobra.Command{Use: "visible"})
+	parent.AddCommand(&cobra.Command{Use: "hidden-target", Hidden: true})
+
+	err := parent.RunE(parent, []string{"hiddn-targt"})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if strings.Contains(err.Error(), "hidden-target") {
+		t.Errorf("hidden subcommand should not be suggested: %v", err)
+	}
+}

pkg/cmd/pr/merge/merge.go

@@ -184,9 +184,11 @@ func Run(ctx context.Context, opts *options) error {
 	if err != nil {
 		// H2: cross-namespace verb routing — if the number is an issue,
 		// surface a friendly redirect instead of "pull request not found".
+		// audit-I6: `pr merge` is PR-only — mark asymmetric so the
+		// redirect doesn't point at the non-existent `issue merge`.
 		if api.IsNotFoundError(err) {
 			ic := issues.NewClient(client)
-			if cerr := crosskind.Check(ctx, ic, pc, ref.Repo.Owner, ref.Repo.Name, ref.Number, "pr", "pr merge", "merge"); cerr != nil {
+			if cerr := crosskind.CheckAsymmetric(ctx, ic, pc, ref.Repo.Owner, ref.Repo.Name, ref.Number, "pr", "pr merge", "merge", true); cerr != nil {
 				return cerr
 			}
 		}

pkg/cmd/pr/ready/ready.go

@@ -98,9 +98,11 @@ func Run(ctx context.Context, opts *options) error {
 
 	// H2: cross-namespace verb routing — if the number is actually an
 	// issue, surface a friendly redirect rather than "pull request not
-	// found".
+	// found". audit-I6: `pr ready` is PR-only, so flag the redirect as
+	// asymmetric so the message explains the constraint instead of
+	// pointing at `issue ready` (which doesn't exist).
 	ic := issues.NewClient(client)
-	if err := crosskind.Check(ctx, ic, pc, ref.Repo.Owner, ref.Repo.Name, ref.Number, "pr", "pr ready", "ready"); err != nil {
+	if err := crosskind.CheckAsymmetric(ctx, ic, pc, ref.Repo.Owner, ref.Repo.Name, ref.Number, "pr", "pr ready", "ready", true); err != nil {
 		return err
 	}
 

pkg/cmd/pr/ready/ready_test.go

@@ -148,7 +148,16 @@ func TestReadyWrongNamespaceRedirects(t *testing.T) {
 	if !strings.Contains(err.Error(), "is an issue") {
 		t.Errorf("error should redirect: %v", err)
 	}
-	if !strings.Contains(err.Error(), "shithub issue ready 5") {
-		t.Errorf("error should suggest issue ready: %v", err)
+	// audit-I6: `pr ready` is asymmetric — error must NOT point at
+	// `issue ready` (which doesn't exist) and SHOULD point at the
+	// `issue view` inspection path.
+	if strings.Contains(err.Error(), "shithub issue ready") {
+		t.Errorf("must NOT suggest non-existent `issue ready`: %v", err)
+	}
+	if !strings.Contains(err.Error(), "only applies to pull requests") {
+		t.Errorf("expected asymmetry explanation: %v", err)
+	}
+	if !strings.Contains(err.Error(), "shithub issue view 5") {
+		t.Errorf("expected view-redirect fallback: %v", err)
 	}
 }

pkg/cmd/pr/review/review.go

@@ -142,8 +142,10 @@ func Run(ctx context.Context, opts *options) error {
 
 	// H2: cross-namespace verb routing. Submitting a review on an
 	// issue number surfaced "pull request not found"; redirect instead.
+	// audit-I6: `pr review` is PR-only — flag asymmetric so we don't
+	// emit "try `shithub issue review N`" pointing at a dead command.
 	ic := issues.NewClient(client)
-	if cerr := crosskind.Check(ctx, ic, pc, ref.Repo.Owner, ref.Repo.Name, ref.Number, "pr", "pr review", "review"); cerr != nil {
+	if cerr := crosskind.CheckAsymmetric(ctx, ic, pc, ref.Repo.Owner, ref.Repo.Name, ref.Number, "pr", "pr review", "review", true); cerr != nil {
 		return cerr
 	}
 

pkg/cmd/shared/crosskind/crosskind.go

@@ -38,6 +38,14 @@ type ErrWrongNamespace struct {
 	// SuggestedVerb is the command tail to suggest (e.g. "close",
 	// "ready"). Defaults to the verb portion of CmdName when empty.
 	SuggestedVerb string
+	// Asymmetric marks verbs that exist on only one side of the
+	// namespace (e.g. `pr ready`, `pr merge`, `pr review`). When
+	// true, the formatted message explains the constraint instead
+	// of pointing at an `issue <verb>` that doesn't exist.
+	// audit-I6: pre-fix `pr ready 1` on an issue suggested
+	// `try shithub issue ready 1` — but `issue ready` doesn't
+	// exist, so the user chased a dead command.
+	Asymmetric bool
 }
 
 // errWrongNamespaceSentinel is the marker for errors.Is matching;
@@ -56,10 +64,34 @@ func (e *ErrWrongNamespace) Error() string {
 	} else {
 		kindNoun = "pull request"
 	}
+	// audit-I6: asymmetric verbs (e.g. `pr ready`, `pr merge`,
+	// `pr review`) don't exist on the other side of the namespace.
+	// Pre-fix we still emitted "try `shithub issue ready N`" — a
+	// dead command. Now we explain the asymmetry and steer the
+	// user at `view` (which exists everywhere) for inspection.
+	if e.Asymmetric {
+		return fmt.Sprintf("%s: #%d is %s %s; `%s` only applies to %ss (run `shithub %s view %d` to inspect)",
+			e.CmdName, e.Number, articleFor(kindNoun), kindNoun,
+			e.CmdName, asymmetricSubjectFor(e.CmdName), tree, e.Number)
+	}
 	return fmt.Sprintf("%s: #%d is %s %s; try `shithub %s %s %d`",
 		e.CmdName, e.Number, articleFor(kindNoun), kindNoun, tree, suggested, e.Number)
 }
 
+// asymmetricSubjectFor returns the noun-form ("pull request" or
+// "issue") the asymmetric verb operates on, derived from the
+// command prefix in CmdName. Used by Error() to render messages
+// like "`pr ready` only applies to pull requests".
+func asymmetricSubjectFor(cmdName string) string {
+	switch {
+	case len(cmdName) >= 3 && cmdName[:3] == "pr ":
+		return "pull request"
+	case len(cmdName) >= 6 && cmdName[:6] == "issue ":
+		return "issue"
+	}
+	return "the calling kind"
+}
+
 // Is supports errors.Is matching against the sentinel — callers that
 // don't need the typed fields can write `errors.Is(err, ErrWrongNamespaceSentinel)`.
 func (e *ErrWrongNamespace) Is(target error) bool {
@@ -94,6 +126,16 @@ func articleFor(noun string) string {
 // and runs only on the wrong-side path because the expected-side
 // lookup is what the mutation already needs to do.
 func Check(ctx context.Context, ic *issues.Client, pc *pulls.Client, owner, name string, number int, expectedKind, cmdName, suggestedVerb string) error {
+	return CheckAsymmetric(ctx, ic, pc, owner, name, number, expectedKind, cmdName, suggestedVerb, false)
+}
+
+// CheckAsymmetric is Check with an explicit `asymmetric` flag for
+// verbs that exist on only one side of the namespace (e.g.
+// `pr ready`, `pr merge`, `pr review`, `pr checkout`, `pr diff`,
+// `pr update-branch`). When asymmetric=true and the wrong-side
+// case fires, the error message explains the constraint instead
+// of pointing at an `issue <verb>` that doesn't exist. audit-I6.
+func CheckAsymmetric(ctx context.Context, ic *issues.Client, pc *pulls.Client, owner, name string, number int, expectedKind, cmdName, suggestedVerb string, asymmetric bool) error {
 	if number <= 0 {
 		return nil
 	}
@@ -109,6 +151,7 @@ func Check(ctx context.Context, ic *issues.Client, pc *pulls.Client, owner, name
 			return &ErrWrongNamespace{
 				Number: number, CmdName: cmdName,
 				Found: "pr", SuggestedVerb: suggestedVerb,
+				Asymmetric: asymmetric,
 			}
 		}
 	case "pr":
@@ -119,6 +162,7 @@ func Check(ctx context.Context, ic *issues.Client, pc *pulls.Client, owner, name
 			return &ErrWrongNamespace{
 				Number: number, CmdName: cmdName,
 				Found: "issue", SuggestedVerb: suggestedVerb,
+				Asymmetric: asymmetric,
 			}
 		}
 	default: