Skip to content

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

Merged
mfwolffe merged 5 commits into
trunkfrom
i4/verb-suggestions
May 24, 2026
Merged

I4 [CRIT/CLI] Cross-NS verb redirects + subcommand typo suggestions (audit-I5 + audit-I6)#36
mfwolffe merged 5 commits into
trunkfrom
i4/verb-suggestions

Conversation

@espadonne
Copy link
Copy Markdown
Contributor

Summary

Two CLI papercuts that turn a typo into a wild-goose chase:

  • audit-I5: cobra's "Did you mean?" suggestion only fires at the root command's auto-error path. Once execution reaches a parent's RunE (which I3 landed across every parent), the suggestion machinery is bypassed. shithub reepo suggests repo, but shithub pr klose / org liat / repo cre got no help. Now they do — extended ParentRunE to walk the parent's children and Levenshtein-match (or 3+ char prefix-match) against the bad arg. shithub pr kloseDid you mean this? close.

  • audit-I6: the H2 cross-namespace verb redirects help when symmetric verbs (view, close, reopen, edit) hit the wrong side — shithub pr close 1 on an issue says try 'shithub issue close 1'. But the same template fires for asymmetric PR-only verbs (pr ready, pr merge, pr review), suggesting shithub issue ready 1 — a command that doesn't exist. The user chases a dead command. Added an Asymmetric flag to ErrWrongNamespace + a CheckAsymmetric helper; the three PR-only callers now emit:

pr ready: #1 is an issue; `pr ready` only applies to pull requests (run `shithub issue view 1` to inspect)

instead of the misleading try shithub issue ready 1 redirect.

Test plan

  • TestParentRunE_LevenshteinSuggestionkloseclose
  • TestParentRunE_PrefixSuggestioncrecreate
  • TestParentRunE_NoCloseMatchSkipsSuggestion — far-off typos don't trigger
  • TestParentRunE_HiddenChildrenIgnored — hidden subs stay hidden
  • TestCheckAsymmetric_PRReadyOnIssue — asymmetric flag suppresses issue ready suggestion
  • TestCheckAsymmetric_PRMergeOnIssue — same for pr merge
  • TestCheck_StillSuggestsForSymmetricVerb — regression guard: symmetric verbs (pr close) still redirect at issue close
  • Existing TestReadyWrongNamespaceRedirects updated for the new asymmetric message
  • make ci clean
  • golangci-lint run clean
  • Smoke dogfood: shithub pr klose / org liat / repo cre all suggest the right verb

Design notes

  • Check stays as a thin wrapper around CheckAsymmetric(..., false) so the 7 existing call sites for symmetric verbs (issue close, issue edit, issue reopen, pr close, etc.) don't need to change. Only the 3 asymmetric callers (pr ready, pr merge, pr review) opt into the new path.
  • Levenshtein implementation is inline (~30 lines) — no new dep, sized for the ~10-15 children per parent. Distance threshold = 2 (cobra's default), prefix-match wins outright for prefixes of length 3+.
  • I deliberately did NOT extend CheckAsymmetric to also drop the symmetric verbs from the issue side onto the PR side (e.g. issue lockpr lock). H2 already handles those, and issue→PR asymmetry isn't in the I-audit.

@mfwolffe mfwolffe merged commit da86b5c into trunk May 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants