fix(cli): address 16 CLI consistency issues from 2026-07-20 audit#46854
Conversation
- TG-1: Add missing article 'the' before 'workflow_dispatch trigger' in trial help
- TG-2: Fix phrasing 'created as private and kept' → 'created as a private repository and retained' in trial help
- TG-3: Capitalize 'Markdown' as proper noun in cli.md
- TG-4: Fix tense inconsistency in checks command ('are blocking' → 'blocked')
- TG-5: Add 'Requires a clean working directory' to --push flag help text
- FN-1: Add --delete-host-repo-before flag; keep --force-delete-host-repo-before as deprecated
- FN-2: Add --no-remove-orphans flag for remove command; keep --keep-orphans as deprecated
- FN-3: Remove uppercase -F shorthand from --raw-field; update example to --raw-field
- DM-3: Add [HOST/] prefix to doctor --repo flag description
- DM-4: Update root help 'View execution logs' → 'Download and analyze execution logs'
- DM-5: Fix 'pinned Actions' → 'pinned actions' in cli.md
- DM-6: Fix 'from The Agentics collection' → 'from the Agentics collection' in cli.md
- DM-7: Add note about default action-bump behavior to update command Long description
- ID-1: Standardize --evals flag to positive phrasing in audit command
- ID-2: Expand run --approve description to mention strict-mode behavior
- ID-3: Fix new --force description to 'Overwrite existing workflow files without confirmation'
- Update all related tests and docs
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Rename forceDeleteHostRepo variable to legacyForceDelete for clarity when merging the deprecated --force-delete-host-repo-before and new --delete-host-repo-before flags, making the intent more explicit. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #46854 does not have the 'implementation' label and has only 21 new lines of code in business logic directories (threshold: 100). |
There was a problem hiding this comment.
Pull request overview
Aligns CLI flags, help text, and documentation with the consistency audit.
Changes:
- Adds deprecated aliases for renamed trial and remove flags.
- Updates flag descriptions, examples, grammar, and terminology.
- Removes the
-Fshorthand and updates related tests.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/update_command.go |
Documents default action version updates. |
pkg/cli/trial_command.go |
Adds the new repository deletion flag and alias. |
pkg/cli/remove_command.go |
Updates orphan-removal guidance. |
pkg/cli/flags_test.go |
Changes shorthand consistency coverage. |
pkg/cli/doctor_command.go |
Documents GHES repository syntax. |
pkg/cli/checks_command.go |
Revises normalized-state wording. |
pkg/cli/audit.go |
Standardizes eval filtering help. |
docs/src/content/docs/setup/cli.md |
Updates CLI documentation and examples. |
docs/src/content/docs/experimental/trial-ops.md |
Uses the renamed trial flag. |
cmd/gh-aw/main.go |
Updates flags, aliases, and command help. |
cmd/gh-aw/main_help_text_test.go |
Updates run approval help assertion. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 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().Bool("push", false, "Commit and push workflow files (including transitive imports) before running") | ||
| runCmd.Flags().StringArray("raw-field", []string{}, "Pass a workflow dispatch input in key=value format (can be specified multiple times)") |
| runCmd.Flags().StringArrayP("raw-field", "F", []string{}, "Pass a workflow dispatch input in key=value format (can be specified multiple times)") | ||
| runCmd.Flags().Bool("push", false, "Commit and push workflow files (including transitive imports) before running") | ||
| runCmd.Flags().StringArray("raw-field", []string{}, "Pass a workflow dispatch input in key=value format (can be specified multiple times)") | ||
| runCmd.Flags().Bool("push", false, "Commit and push workflow files (including transitive imports) before running. Requires a clean working directory.") |
| pending - checks are still running or queued | ||
| no_checks - no checks configured or triggered | ||
| policy_blocked - policy or account gates are blocking the PR | ||
| policy_blocked - policy or account gates blocked the PR |
| name: "run command does not have -F for --raw-field", | ||
| shortFlag: "F", | ||
| longFlag: "raw-field", | ||
| commandSetup: func() *cobra.Command { return createRunCommandStub() }, | ||
| shouldExist: true, | ||
| description: "run should have raw-field short flag (uppercase F)", | ||
| shouldExist: false, | ||
| description: "run should not have an uppercase -F short flag (CLI-wide lowercase convention)", |
🧪 Test Quality Sentinel Report✅ Test Quality Score: 85/100 — Excellent
📊 Metrics (4 tests)
Verdict
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
The changes correctly address most of the 16 identified CLI consistency issues. Existing inline comments have already flagged the key concerns.
Issues already noted in inline comments:
-Fshorthand removal (main.goline 801) — breaking change for existing scripts; violates CLI compatibility rules.--pushhelp text (main.goline 802) — overstated constraint; implementation only rejects unrelated staged files, not all unclean states.checksMCP duplicate (checks_command.goline 88) —pkg/cli/mcp_tools_readonly.gostill uses old phrasing, leaving the stated consistency goal unmet.- Stub-only test (
flags_test.goline 78) — test validates a hand-written stub, not the real productionrunCmd.
Changes that look correct:
- Typo/grammar fixes in
trial_command.goandcli.md --no-remove-orphansalias with deprecated--keep-orphans--delete-host-repo-beforealias with deprecated--force-delete-host-repo-beforedoctor --repodescription updated with[HOST/]audit --evalsrewritten with positive phrasingupdate_command.golong-help note about--no-release-bump(flag already existed)new --forcedescription scoped to workflow files
The four issues already called out should be addressed before merge.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 27.5 AIC · ⌖ 4.43 AIC · ⊞ 5K
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 24.9 AIC · ⌖ 4.28 AIC · ⊞ 6.7K
Comment /matt to run again
There was a problem hiding this comment.
Review: CLI Consistency Fixes
Non-blocking observations overall — the flag renaming and help text cleanup are sound. One new finding on top of existing comments.
### Findings summary
New finding (inline comment): — the added Long help text and the existing flag description for say contradictory things about what the flag does. The flag description should be updated to match the Long text and the docs.
Pre-existing comments (not duplicated):
- Removing shorthand may break existing scripts — a breaking change
- description says "requires a clean working directory" but the implementation only enforces clean staged state
- description inconsistency with the MCP tool copy
- Test isolation gap in
🔎 Code quality review by PR Code Quality Reviewer · 45.2 AIC · ⌖ 4.47 AIC · ⊞ 5.6K
Comment /review to run again
Comments that could not be inline-anchored
pkg/cli/update_command.go:287
Contradictory --no-release-bump description: the newly added Long help says update bumps all referenced Actions by default and --no-release-bump restricts to core only — but the flag's own description says it "Disable automatic major version bumps for all actions (only core actions/* are force-updated)", implying core is always force-updated even with the flag set.
<details>
<summary>💡 Suggested fix</summary>
Change the flag description in update_command.go from:
Disable …
</details>
|
@copilot please run the Unresolved review threads:
Failed checks:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "pi.dev"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot please run the Unresolved review threads:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "pi.dev"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in 377675e, 97dbd96, and 435600a. I restored Local validation passed with |
Addressed by 377675e, 97dbd96, and 435600a. The branch already contains the review follow-ups: |
|
🎉 This pull request is included in a new release. Release: |
Automated CLI consistency audit identified 16 issues across help text, flag naming, and docs/help mismatches. This PR resolves all of them.
Typos & Grammar
trial: "support the workflow_dispatch trigger", "created as a private repository and retained by default"cli.md: "natural language Markdown files", "pinned actions", "from the Agentics collection"checks:policy_blockeddescription uses past tense consistently: "gates blocked the PR"run --push: help now mentions "Requires a clean working directory"Flag Naming Inconsistencies
trial: adds--delete-host-repo-before(consistent with--delete-host-repo-after);--force-delete-host-repo-beforedeprecatedremove: adds--no-remove-orphansfollowing the--no-*convention;--keep-orphansdeprecatedrun: removes uppercase-Fshorthand from--raw-field(CLI-wide lowercase convention); example updated to--raw-fieldDocs ↔ Help Text Mismatches
doctor --repo: description now includes[HOST/]GHES prefix"View execution logs"→"Download and analyze execution logs"updateLong: adds note that update bumps all action refs to latest major version by default (--no-release-bumpto restrict)Inconsistent Flag Descriptions
audit --evals: changed from negative phrasing ("Skip runs that do not contain…") to positive ("Filter to runs containing evals results (evals.jsonl)"), matchinglogs --evalsrun --approve: expanded to mention strict-mode behavior, consistent withcompile/upgradenew --force:"Overwrite existing **workflow** files without confirmation", matchingadd/deployAll renamed flags retain deprecated aliases for backward compatibility.
pr-sous-chef run https://github.com/github/gh-aw/actions/runs/29761732160
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
pi.devSee Network Configuration for more information.