Skip to content

D3d: CLI MIN cleanups (C18+C22+C23+C25+C26)#8

Merged
mfwolffe merged 5 commits into
trunkfrom
d3d/min-cleanups
May 17, 2026
Merged

D3d: CLI MIN cleanups (C18+C22+C23+C25+C26)#8
mfwolffe merged 5 commits into
trunkfrom
d3d/min-cleanups

Conversation

@espadonne
Copy link
Copy Markdown
Contributor

Summary

Five MIN findings from the C-audit, all single-file CLI polish. Each commit is one finding so reviewers and bisect tools can trace cleanly. Tested two MIN findings from the audit that turned out to already exit non-zero on trunk (C22 completion no-arg, C27 api -H invalid header) — A4 and unrelated work incidentally fixed them. C22 still gets a text wording polish here.

  • C18shithub cache <typo> printed help + exited 0. Cobra's default for unknown subcommands on a parent with no RunE is "show help". Now cache rejects unknown positional args with unknown subcommand "X" for "shithub cache" and exit 1. Bare shithub cache (no args) still shows help, exit 0.
  • C22completion (no shell arg) error message said --shell is required. After A4's positional-arg support the help shows completion <shell> — message now matches: shell is required (one of: ...).
  • C23repo list --web was missing. gh has it. Now opens https://<host>/<owner>?tab=repositories (or the host's "your repos" page for the authenticated user). Note: MarkWebMutuallyExclusive integration is left as a TODO comment — depends on D3a (D3a [CRIT]: --web mutually exclusive with --json/--jq/--template (C5+C15) #5) landing first.
  • C25secret set accepted lowercase, mixed-case, and leading-digit names. Tightened to gh-compat ^[A-Z_][A-Z0-9_]*$, no leading digit, no GITHUB_ prefix. Comment in the original code claimed those rules but didn't enforce them.
  • C26alias set foo "X" silently overwrote an existing foo alias. Now prints ! Changing alias foo from <old> to <new> to stderr before re-saving (matches gh exactly).

Test plan

  • TestValidateNameRejectsLowercase / RejectsLeadingDigit / RejectsGithubPrefix / Accepts — four focused tests on the secret regex.
  • All existing alias/cache/completion/repo-list/secret tests still green.
  • Manual repro on the built binary:
    • shithub cache delete-all → exit 1, "unknown subcommand"
    • shithub cache (no args) → exit 0, help printed
    • shithub completion → exit 1, error says "shell is required"
  • make ci green locally.

Notes

  • C21 (repo clone error wrap) and C27 (api -H invalid header) deliberately not in this PR: both already work correctly on trunk; deeper investigation would be its own batch.

@mfwolffe mfwolffe merged commit c40af98 into trunk May 17, 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