fix: address 14 CLI consistency issues (branding, descriptions, docs)#47592
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Standardizes CLI help text, branding, flag descriptions, and documentation.
Changes:
- Aligns command descriptions and shared flag helpers.
- Corrects deprecated flag guidance and stale branding.
- Expands documentation for existing CLI options.
Show a summary per file
| File | Description |
|---|---|
cmd/gh-aw/main.go |
Updates branding and deprecation guidance. |
pkg/cli/audit_diff_command.go |
Marks the legacy command deprecated. |
pkg/cli/validate_command.go |
Uses the shared JSON flag helper. |
pkg/cli/mcp_server_command.go |
Aligns MCP server descriptions. |
pkg/cli/lint_command.go |
Clarifies lint scope. |
pkg/cli/env_command.go |
Updates branding and repository help. |
pkg/cli/doctor_command.go |
Revises repository flag help. |
docs/src/content/docs/setup/cli.md |
Documents additional flags and scanners. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/cli/env_command.go:201
- The new help text advertises the three-part
HOST/owner/repoform, butresolveDefaultsTargetpasses this value torepoutil.SplitRepoSlug, which rejects anything other than exactlyowner/repo(pkg/repoutil/repoutil.go:17-20). Users following this help receive an invalid repository error. Keep the documented format to what this command currently accepts, or add host parsing and host selection before advertising it.
cmd.Flags().StringVarP(&repo, "repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository")
- Files reviewed: 8/8 changed files
- Comments generated: 4
- Review effort level: Medium
| runCmd.Flags().Bool("auto-merge-prs", false, "Auto-merge any pull requests created during the workflow execution") | ||
| runCmd.Flags().StringArrayP("raw-field", "F", []string{}, "Pass a workflow dispatch input in key=value format (can be specified multiple times)") | ||
| _ = runCmd.Flags().MarkShorthandDeprecated("raw-field", "use --raw-field instead") | ||
| _ = runCmd.Flags().MarkShorthandDeprecated("raw-field", "use the long form --raw-field instead") |
There was a problem hiding this comment.
Fixed in the latest commit — the test assertion now matches the new deprecation text "use the long form --raw-field instead".
|
|
||
| cmd.Flags().StringVar(&scope, "scope", "", "Variable scope (repo|org|ent). Defaults to repo") | ||
| cmd.Flags().StringVarP(&repo, "repo", "r", "", "Target repository in owner/repo format. When omitted, defaults to current repository") | ||
| cmd.Flags().StringVarP(&repo, "repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository") |
There was a problem hiding this comment.
Fixed in the latest commit — both --repo descriptions in env_command.go (lines 162 and 201) now document owner/repo format, matching what resolveDefaultsTarget/SplitRepoSlug actually accepts.
| } | ||
|
|
||
| cmd.Flags().StringP("repo", "r", "", "Target repository in [HOST/]owner/repo format") | ||
| cmd.Flags().StringP("repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository") |
There was a problem hiding this comment.
Fixed in the latest commit — the --repo description in doctor_command.go now reads "Target repository (owner/repo format)". The misleading "Defaults to current repository" claim is removed since omitting --repo triggers auth-only diagnostics, not repository checks.
| - **`--syft`:** Generates a Software Bill of Materials (SBOM) for container images referenced in compiled workflows using the Syft scanner. | ||
| - **`--grype`:** Scans container images referenced in compiled workflows for known vulnerabilities using the Grype vulnerability scanner. |
There was a problem hiding this comment.
Fixed in the latest commit — --grype and --syft are now included in the canonical compile Options list, in alphabetical order (--grype between --grant and --json/-j; --syft between --strict and --trial).
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. This PR (#47592) contains only documentation updates and CLI consistency fixes (branding, descriptions, docs). Test Quality Sentinel analysis skipped. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #47592 does not have the 'implementation' label and has only 8 new lines of code in business logic directories (threshold: 100). |
|
@copilot please run the Unresolved review threads (newest first):
Failed checks:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR makes solid consistency improvements across CLI descriptions and docs. However, there are 3 blocking issues that must be addressed before merge (all already called out in inline comments):
-
Test failure (
cmd/gh-aw/main_help_text_test.go:27) — The test still asserts"use --raw-field instead"but the changed code now emits"use the long form --raw-field instead". The test will fail deterministically. Update the test to match. -
Misleading
--repodescription inenv_command.go— The new help text advertises[HOST/]owner/repo, butresolveDefaultsTargetpasses this value torepoutil.SplitRepoSlug, which only acceptsowner/repo. Advertising a format the code cannot handle misleads users. -
Incorrect
doctor --repodefault description —"Defaults to current repository"implies omitting--reporuns full repo diagnostics, but therepo == ""branch only runs auth-only diagnostics. The description must match actual behavior.
The docs-only changes (--evals, --engine/-e, --syft/--grype/--runner-guard callouts, --repo/-r for pr transfer, --disable-codemod rationale) are all clean and correct.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 21.3 AIC · ⌖ 4.57 AIC · ⊞ 5K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /grill-with-docs — requesting changes on test correctness and documentation consistency issues.
📋 Key Themes & Highlights
Key Issues (see existing inline comments)
The four existing inline comments from @Copilot already identify the most impactful problems:
- Test breakage (
cmd/gh-aw/main.go:814) — The deprecation message change breaksmain_help_text_test.go:27; the assertion must be updated to match. - Misleading flag doc vs. actual parsing (
pkg/cli/env_command.go:162) — Help text advertisesHOST/owner/repobutresolveDefaultsTarget→repoutil.SplitRepoSlugonly acceptsowner/repo; either fix the parser or revert the doc. - False default claim (
pkg/cli/doctor_command.go:62) — "Defaults to current repository" is inaccurate: the empty-repo branch runs auth-only diagnostics, not repo diagnostics. - Docs options list incomplete (
docs/src/content/docs/setup/cli.md:350) —--syftand--grypeare described in prose but absent from the canonical Options list above.
Positive Highlights
- ✅ Clean use of
addJSONFlag(cmd)helper invalidate_command.go— good single-source-of-truth fix - ✅
[Deprecated]prefix inaudit_diff_command.goShort is discoverable and consistent - ✅ "GitHub Actions variables" rename is accurate and aligns with platform terminology
- ✅
--disable-codemodrationale note in docs is genuinely helpful for users wondering why the pattern differs
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 18.3 AIC · ⌖ 4.61 AIC · ⊞ 6.7K
Comment /matt to run again
There was a problem hiding this comment.
4 blocking issues require fixes before merge.
📋 Blocking themes
Test suite breakage
The deprecation message change in main.go breaks an existing unit test that asserts the exact old string. CI will fail deterministically.
Misleading flag documentation
Two flag descriptions claim behaviour the implementation does not deliver:
doctor --repo: says "Defaults to current repository" but omitting--reposkips repo diagnostics entirely (auth-only path).env --repo: advertises[HOST/]owner/repobut the underlying resolver rejects that form.
Incomplete docs
The compile Options list still omits --syft and --grype even though the new prose now describes those flags.
All other changes (branding, addJSONFlag refactor, short description wording) look correct.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 32.5 AIC · ⌖ 4.52 AIC · ⊞ 5.7K
Comment /review to run again
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…ptions list) Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
🤖 PR Triage
Score breakdown: Impact 28 + Urgency 20 + Quality 17 Batch: Rationale: Not draft, CI is passing (canary-go, Windows), fixes 14 real CLI UX gaps (flag discoverability, branding, docs). Multiple bot review rounds with changes requested. Good candidate for expedited human review after addressing bot feedback.
|
|
@copilot please run the Unresolved review feedback (newest first):
Run: https://github.com/github/gh-aw/actions/runs/30035273314
|
All four issues raised in both CHANGES_REQUESTED reviews are already fixed in commit
Full local test suite passes. The two CHANGES_REQUESTED reviews from |
|
@copilot please run the Unresolved review feedback (newest first):
|
All four items from the
What a maintainer needs to do:
|
|
@copilot please run the
|
…cy-fix-documentation # Conflicts: # .github/workflows/daily-team-evolution-insights.lock.yml Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Branch is now up to date with All four unresolved review threads remain addressed on the current HEAD — fixes were in Merge-readiness summary:
Still needed from a maintainer: re-trigger CI and dismiss the two stale |
Fixes 14 of 15 CLI consistency issues found by static source analysis across
pkg/cli/*.goanddocs/src/content/docs/setup/cli.md. Issues ranged from users unable to discover available flags to stale branding and self-referential deprecation messages.Source code
cmd/gh-aw/main.go: Remove stale "from GitHub Next" branding from root commandShort/Long; fix circular-Fdeprecation message ("use --raw-field instead"→"use the long form --raw-field instead")pkg/cli/audit_diff_command.go: Surface deprecation inShort—"[Deprecated] Compare workflow runs (use: gh aw audit <base> <compare...>)"pkg/cli/validate_command.go: Replace inlinecmd.Flags().BoolP("json", "j", ...)withaddJSONFlag(cmd)helper (single source of truth)pkg/cli/mcp_server_command.go: AlignShortto matchLong's relative-clause phrasingpkg/cli/lint_command.go:"workflows"→"workflow files"inShortpkg/cli/env_command.go:"GitHub variables"→"GitHub Actions variables"inShort; standardize--repodescriptions to[HOST/]owner/repoformatpkg/cli/doctor_command.go: Standardize--repodescription to canonical form (add "Defaults to current repository")Docs (
docs/src/content/docs/setup/cli.md)pr transfer(High): Add missing**Options:** \--repo/-r`` — users had no way to discover this flaglogs: Add--evalsto Options list (implemented but absent)secrets bootstrap:--engine→--engine/-eto expose implemented short formfix: Add rationale note explaining why--disable-codemoduses value-taking flag semantics instead of the--no-Xpattern used elsewherecompile: Add prose callouts for--syft(SBOM),--grype(vulnerability scanner),--runner-guard(taint analysis) — flags were listed in Options but undescribedNot applied
The
deploy --pralias finding is inapplicable:deployunconditionally creates a PR and has no--create-pull-requestboolean flag to alias.$
Run: https://github.com/github/gh-aw/actions/runs/30035273314
Run: https://github.com/github/gh-aw/actions/runs/30042541462