fix(cli): resolve 32 CLI consistency issues (2026-07-01)#42832
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Nice work on this CLI consistency sweep, The diff touches 29 files and updates the corresponding test files alongside each command change, so coverage tracks the changes cleanly. The description is thorough and well-structured, making it easy for reviewers to understand the rationale for every fix. This looks ready for review. Nothing blocking here — just awaiting
|
There was a problem hiding this comment.
Pull request overview
This PR applies a broad set of CLI consistency fixes across gh aw, aligning flag naming, help text wording, and documentation with the actual command behavior and established conventions in the codebase.
Changes:
- Renamed
--disable-codemodto--no-codemodand updated related help text/tests/docs. - Added missing flag parity to
add-wizard(--name/-n,--force/-f,--append,--no-security-scanner) and wired these through the interactive flow. - Standardized help text and docs wording (e.g., “.lock.yml files”, “Markdown”, completion docs subcommand vs positional args,
run --pushdocs).
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/upgrade_command.go | Updates upgrade help text and renames codemod flag to --no-codemod. |
| pkg/cli/upgrade_command_test.go | Updates tests for --no-codemod and wording tweaks. |
| pkg/cli/update_command.go | Standardizes help strings (Markdown, redirect wording, .lock.yml wording, PR wording). |
| pkg/cli/update_command_test.go | Updates security scanner flag help text expectation (“Markdown”). |
| pkg/cli/trial_command.go | Improves trial help text and standardizes several flag descriptions (yes/dry-run/append/security scanner). |
| pkg/cli/project_command.go | Standardizes “GitHub Projects V2” naming and “Kanban” capitalization. |
| pkg/cli/project_command_test.go | Updates assertions for updated project command phrasing. |
| pkg/cli/outcomes_command.go | Adjusts --output help text to show the default logs directory. |
| pkg/cli/mcp_list_tools.go | Expands mcp list-tools description to include “find workflows using a server” use-case. |
| pkg/cli/mcp_list_tools_test.go | Updates tests to match revised mcp list-tools short description. |
| pkg/cli/logs_command.go | Minor grammar fix (“By default, …”). |
| pkg/cli/lint_command.go | Updates --dir wording to the standard “Workflow directory”. |
| pkg/cli/init_command.go | Minor grammar/article wording fixes in init help text. |
| pkg/cli/init_command_test.go | Updates assertion for “current repository” wording. |
| pkg/cli/health_command.go | Minor wording improvement (“over a time period”). |
| pkg/cli/hash_command.go | Standardizes “Markdown” capitalization in help text. |
| pkg/cli/forecast_command.go | Minor grammar improvement (“recently completed”). |
| pkg/cli/flags_test.go | Updates “force” wording in stubbed command help text. |
| pkg/cli/fix_command.go | Renames codemod flag to --no-codemod and updates retrieval. |
| pkg/cli/fix_command_test.go | Updates test name and assertion for the new --no-codemod flag. |
| pkg/cli/env_command.go | Standardizes “in bulk” wording. |
| pkg/cli/deploy_command.go | Standardizes .lock.yml wording, repo flag prose, Markdown capitalization, and append wording. |
| pkg/cli/add_wizard_command.go | Adds flag parity (--name, --force, --append, --no-security-scanner) and wires them into the interactive add config. |
| pkg/cli/add_wizard_command_test.go | Updates engine list wording and adds a test for --name multi-workflow rejection. |
| pkg/cli/add_interactive_orchestrator.go | Extends AddInteractiveConfig to carry new add-wizard options. |
| pkg/cli/add_interactive_git.go | Propagates add-wizard options into AddResolvedWorkflows options used for PR creation. |
| pkg/cli/add_command.go | Standardizes “Markdown” capitalization and append wording; keeps hidden security-scanner alias. |
| docs/src/content/docs/setup/cli.md | Updates CLI docs for run --push, completion shell values vs subcommands, and codemod flag rename. |
| cmd/gh-aw/main.go | Standardizes several command flag help strings (force/auto-merge-prs/dry-run/approve punctuation). |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 29/29 changed files
- Comments generated: 7
- Review effort level: Low
| verbose, _ := cmd.Flags().GetBool("verbose") | ||
| dir, _ := cmd.Flags().GetString("dir") | ||
| disabledCodemods, _ := cmd.Flags().GetStringSlice("disable-codemod") | ||
| disabledCodemods, _ := cmd.Flags().GetStringSlice("no-codemod") |
| auditFlag, _ := cmd.Flags().GetBool("audit") | ||
| jsonOutput, _ := cmd.Flags().GetBool("json") | ||
| disabledCodemods, _ := cmd.Flags().GetStringSlice("disable-codemod") | ||
| disabledCodemods, _ := cmd.Flags().GetStringSlice("no-codemod") |
| ### The `--push` Flag | ||
|
|
||
| `gh aw run --push` stages all changes, commits them, and pushes before dispatching the workflow. It requires a clean working directory. | ||
| `gh aw run --push` stages workflow files only (including transitive imports), commits them, and pushes before dispatching the workflow. It does not require a clean working directory. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer failed during the skills-based review. |
|
✅ Test Quality Sentinel completed test quality analysis. |
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (132 new lines in 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
🧪 Test Quality Sentinel Report🔶 Test Quality Score: 44/100 — Needs improvement
📊 Metrics (8 tests)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
REQUEST_CHANGES — blocking issues present
The existing inline comments (breaking --disable-codemod rename, vague --dry-run text, incorrect --push docs, --output default mismatch) are unresolved and sufficient to block merge on their own. New findings added this pass:
Additional issues found
High — test coverage gaps for new add-wizard flags (add_wizard_command_test.go): Three of the four flags added in this PR (--force, --append, --no-security-scanner) have zero tests. A flag-name typo in RunE silently zeros the value with no compile or runtime error.
High — fragile --output default in outcomes_command.go: Uses addOutputFlag(cmd, "") + a manual Lookup("output").Usage mutation instead of addOutputFlag(cmd, defaultLogsOutputDir) — the pattern every other output command uses. Cobra's DefValue remains "", so tab-completion and programmatic inspection report the wrong default. Fix is one line.
Medium — TestShortFlagConsistency not updated for add-wizard: New -f/--force and -n/--name shorthands were not added to the existing shorthand consistency test table.
🔎 Code quality review by PR Code Quality Reviewer · 128.9 AIC · ⌖ 9.96 AIC · ⊞ 1.6K
Comment /review to run again
| err := cmd.Execute() | ||
| require.Error(t, err) | ||
| assert.EqualError(t, err, "--name flag cannot be used when adding multiple workflows at once") | ||
| } |
There was a problem hiding this comment.
Three of the four new flags have zero test coverage: --force, --append, and --no-security-scanner are added to add-wizard but only --name is tested — and only for its error path.
💡 Why this matters
The flag reads in RunE use cmd.Flags().GetBool / GetString, but a typo in the flag name string would silently return the zero value with no compile or runtime error. The AddInteractiveConfig fields Force, AppendText, and DisableSecurityScanner are only populated via those reads — if the reads are wrong the flags are silently ignored.
Add flag-presence and type tests analogous to TestNewFixCommand_HasNoCodemodFlag:
func TestAddWizardCommand_HasForceFlag(t *testing.T) {
cmd := NewAddWizardCommand(func(string) error { return nil })
flag := cmd.Flags().Lookup("force")
require.NotNil(t, flag)
assert.Equal(t, "bool", flag.Value.Type())
shortFlag := cmd.Flags().ShorthandLookup("f")
require.NotNil(t, shortFlag, "-f shorthand should be registered")
}Same pattern for --append (string) and --no-security-scanner (bool).
| addJSONFlag(cmd) | ||
| addRepoFlag(cmd) | ||
| addOutputFlag(cmd, "") | ||
| cmd.Flags().Lookup("output").Usage = "Output directory for generated files (default: \".github/aw/logs\")" |
There was a problem hiding this comment.
Wrong pattern for default: fragile manual Usage override instead of consistent addOutputFlag call.
Every other command that has an --output flag calls addOutputFlag(cmd, defaultLogsOutputDir) so cobra's own default machinery is correct:
logs_command.go:359:addOutputFlag(logsCmd, defaultLogsOutputDir)audit.go:109:addOutputFlag(cmd, defaultLogsOutputDir)view_command.go:101:addOutputFlag(cmd, defaultLogsOutputDir)
This PR instead calls addOutputFlag(cmd, "") and then mutates the Usage string in-place. The cobra flag's DefValue remains "", so shell completions, programmatic flag inspection, and --help rendered by cobra's default formatter will report an empty string as the default while the usage prose says (default: ".github/aw/logs").
💡 Fix
Replace both lines with the same pattern used by every other output command:
addOutputFlag(cmd, defaultLogsOutputDir)This makes cobra's default correct, keeps the help consistent with sibling commands, and stops the hint from silently going stale if defaultLogsOutputDir is ever renamed.
| func createNewCommandStub() *cobra.Command { | ||
| cmd := &cobra.Command{Use: "new"} | ||
| cmd.Flags().BoolP("force", "f", false, "Overwrite existing files") | ||
| cmd.Flags().BoolP("force", "f", false, "Overwrite existing workflow files") |
There was a problem hiding this comment.
TestShortFlagConsistency was not updated for add-wizard's new -f/--force and -n/--name shorthands.
The test currently checks -f for new, add, update, and compile (lines 39–68), and -n for deploy and add, but add-wizard is not in either list despite this PR adding both shorthands to it.
💡 Suggested additions
Add cases to the -f and -n blocks in TestShortFlagConsistency:
// in the -f block
{
name: "add-wizard command has -f for --force",
shortFlag: "f",
longFlag: "force",
commandSetup: func() *cobra.Command { return NewAddWizardCommand(func(string) error { return nil }) },
description: "add-wizard should have force short flag",
},
// in the -n block
{
name: "add-wizard command has -n for --name",
shortFlag: "n",
longFlag: "name",
commandSetup: func() *cobra.Command { return NewAddWizardCommand(func(string) error { return nil }) },
description: "add-wizard should have name short flag",
},This is the exact purpose of this test — catching shorthand regressions across commands.
There was a problem hiding this comment.
Review: fix(cli): resolve 32 CLI consistency issues (2026-07-01)
The vast majority of changes are clean and correct — the help-text standardisations, Markdown capitalisation, article fixes, .lock.yml wording, add-wizard flag parity, and documentation clarifications are all well-executed and follow the established codebase conventions.
One blocking correctness issue must be resolved before merge.
🔴 Breaking change: --disable-codemod removed without a hidden backward-compat alias
Every other flag rename in this repository keeps the old name registered as a hidden alias so that existing scripts continue to work silently. Examples in this very PR:
| New name | Hidden alias kept |
|---|---|
--no-security-scanner |
--disable-security-scanner ✅ |
--no-release-bump |
--disable-release-bump ✅ |
--no-secret |
--skip-secret ✅ |
--no-codemod |
--disable-codemod ❌ missing |
Without the alias, any CI workflow or developer script that uses gh aw fix --disable-codemod foo or gh aw upgrade --disable-codemod foo will break immediately with unknown flag: --disable-codemod.
See inline comments on pkg/cli/fix_command.go (line 85) and pkg/cli/upgrade_command.go (line 156) for the exact pattern to follow.
🟡 Minor issues (non-blocking)
cmd/gh-aw/main.goline 432: The--dry-runexample comment still says# Validate without actually runningafter the flag description was changed to Preview
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 96.8 AIC · ⌖ 6.96 AIC · ⊞ 4.9K
Comments that could not be inline-anchored
pkg/cli/fix_command.go:85
Missing backward-compat hidden alias — breaking change.
Every other flag rename in this codebase keeps the old name as a hidden alias to avoid breaking existing scripts. For example, --disable-security-scanner, --disable-release-bump, and --skip-secret all use this pattern:
cmd.Flags().StringSlice("no-codemod", nil, "Disable specific codemod IDs during the fix step (repeatable)")
cmd.Flags().StringSlice("disable-codemod", nil, "Disable specific codemod IDs during the fix step …
</details>
<details><summary>pkg/cli/upgrade_command.go:156</summary>
Same issue as `fix_command.go`: `--disable-codemod` is removed without a hidden backward-compat alias.
Add the hidden alias to match every other rename in this codebase:
```go
cmd.Flags().StringSlice("no-codemod", nil, "Disable specific codemod IDs during the fix step (repeatable)")
cmd.Flags().StringSlice("disable-codemod", nil, "Disable specific codemod IDs during the fix step (repeatable)")
_ = cmd.Flags().MarkHidden("disable-codemod")And mirror the fix in the GetStringSlice call a…
cmd/gh-aw/main.go:432
Stale inline comment — the flag description was changed from "Validate without actually triggering execution" to "Preview execution without applying any changes", but this example comment still says # Validate without actually running.
Update to match:
gh aw run daily-perf-improver --dry-run # Preview execution without applying any changes@copilot please address this.
pkg/cli/lint_command.go:75
The previous description "Directory to scan for *.lock.yml files when no arguments are provided" carried a behavior-important qualifier: --dir is only used when no positional arguments are given (the command accepts [lock-file-or-directory]...). The new "Workflow directory" is correct for visual consistency but silently drops this behavioral hint for users who mix --dir with positional args.
Consider: "Workflow directory (used when no arguments are provided)" as a middle-ground…
PR Review SummaryApplied Two blocking issues require changes before merge:
Non-blocking: Positive highlights: great breadth, accurate @copilot please address the review comments above.
|
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 146.3 AIC · ⌖ 7.06 AIC · ⊞ 6.6K
Comment /matt to run again
Comments that could not be inline-anchored
pkg/cli/fix_command.go:85
[/diagnosing-bugs] Breaking change: --disable-codemod was removed entirely without a backward-compat hidden alias, unlike --disable-security-scanner which is kept as a hidden alias in every command that uses it. Existing CI scripts using gh aw fix --disable-codemod=rule-id will silently fail with an unknown-flag error after this rename.
<details>
<summary>💡 Suggested fix — mirror the --disable-security-scanner pattern</summary>
cmd.Flags().StringSlice("no-codemod", nil, "Di…
</details>
<details><summary>pkg/cli/upgrade_command.go:156</summary>
**[/diagnosing-bugs]** Same backward-compat gap as `fix_command.go`: `--disable-codemod` is removed with no hidden alias in `upgrade` either. Scripts using `gh aw upgrade --disable-codemod=rule-id` will break silently.
<details>
<summary>💡 Apply the same fix here</summary>
```go
cmd.Flags().StringSlice("no-codemod", nil, "Disable specific codemod IDs during the fix step (repeatable)")
cmd.Flags().StringSlice("disable-codemod", nil, "Disable specific codemod IDs during the fix step (repeatabl…
</details>
<details><summary>pkg/cli/add_wizard_command_test.go:26</summary>
**[/tdd]** The new `--name`, `--force`, `--append`, and `--no-security-scanner` flags added to `add-wizard` each have no dedicated test covering their presence and basic wiring — only the mutual-exclusion guard for `--name` is tested.
<details>
<summary>💡 Suggested additions</summary>
```go
func TestAddWizardCommand_HasFlagParity(t *testing.T) {
cmd := NewAddWizardCommand(func(string) error { return nil })
require.NotNil(t, cmd)
for _, flagName := range []string{"name", "force",…
</details>
<details><summary>cmd/gh-aw/main.go:793</summary>
**[/codebase-design]** `run --dry-run` changed from *"Validate workflow without actually triggering execution on GitHub Actions"* to *"Preview execution without applying any changes"* — the new wording is identical to `trial --dry-run` and loses the important detail that it suppresses actual GitHub Actions dispatch. Callers reading the help text may not realise this flag prevents the workflow from ever reaching GitHub.
<details>
<summary>💡 Suggested wording</summary>
```go
"Preview workflow …
</details>
<details><summary>pkg/cli/add_wizard_command.go:75</summary>
**[/diagnosing-bugs]** `disableSecurityScannerLegacy` is read from `"disable-security-scanner"` but that flag is also declared as a **full visible flag** on the wizard command (line 141) rather than a hidden alias. This means `--disable-security-scanner` will appear in `--help` output for `add-wizard` even though the PR intent is to make it undocumented. The flag should be marked hidden immediately after declaration, matching the pattern used in every other command.
<details>
<summary>💡 One-l…
</details>
<details><summary>cmd/gh-aw/main.go:432</summary>
**[/codebase-design]** The inline example comment on line 432 still reads `# Validate without actually running` but the flag description on line 793 was updated to `Preview execution without applying any changes`. These two phrases now describe the same flag differently, which creates noise for users scanning the help output.
<details>
<summary>💡 Suggested change</summary>
```go
gh aw run daily-perf-improver --dry-run # Preview without dispatching to GitHub ActionsKeeping both the exa…
🤖 PR Triage — §28558548389
Score breakdown: Impact 30 · Urgency 15 · Quality 5 Rationale: 30-file CLI consistency pass (flag naming, help text, flag parity). High user-facing value but currently blocked by CHANGES_REQUESTED reviews citing a breaking
|
|
@copilot please run the
|
…utcomes default, dry-run text, push docs Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
All unresolved review threads addressed in commit
Local validation: |
|
@copilot please run the
|
…ecurity-scanner and update TestShortFlagConsistency Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed all open threads in commit
Local: |
Automated CLI consistency inspection identified 32 issues across help text, flag naming, and docs/CLI mismatches. Fixes span 29 files touching flag definitions, command descriptions, and documentation.
High severity
--disable-codemod→--no-codemod— sole--disable-Xflag in a codebase that uses--no-Xeverywhere else (fix,upgrade)add-wizardflag parity — added--append,--force/-f,--name/-n,--no-security-scannerto matchadd/deploymcp list-toolsdescription — was missing the "find workflows using a server" use-case; parentmcpdescribed it, subcommand didn'tMarkdowncapitalisation — 6+ lowercasemarkdowninstances in flag prose acrossadd,add-wizard,deploy,trial,hashcompletiondocs —bash/zsh/fish/powershelllisted as subcommands; they'reValidArgspositional values; onlyinstall/uninstallare real subcommandsMedium severity
.lock.yml files— standardised from bare "lock files" indeploy,update,upgradeGitHub Projects V2(was singular),Kanban(was lowercase) inproject--repostyle indeploy— prose format aligned to parenthetical([HOST/]owner/repo format)used by all other commands--dirinlint— "Directory to scan for *.lock.yml files" → "Workflow directory" (standard form)run --pushdocs — removed incorrect claim about requiring a clean working directoryforecast,--appenddescription,--dry-run/--yeswording intrialLow severity
a/the), missing commas,in batch→in bulk,workflow(s)→workflows,current repo→current repository,prerelease→pre-releasein upgrade example--outputinoutcomesnow shows(default: ".github/aw/logs")--create-pull-requestand--auto-merge-prsdescriptions acrossadd/update/upgrade/run/trial