Standardize CLI help section order and GHE note wording#41461
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Error: copilot coding agent is not available for this repository |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes gh aw CLI help rendering so Usage consistently appears before Examples, and aligns add-wizard’s GitHub Enterprise shorthand note wording with the canonical phrasing used elsewhere. It also adds a regression test to prevent future help-order regressions.
Changes:
- Moved examples for several commands (
new,remove,enable,disable,compile,run,version) into Cobra’sExamplefield so help output rendersUsagebeforeExamples. - Updated
add-wizardGitHub Enterprise note wording and adjusted its test assertions accordingly. - Added a help-format test that asserts
UsageprecedesExamplesfor the affected commands.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/add_wizard_command.go | Aligns GitHub Enterprise shorthand note wording in add-wizard help text. |
| pkg/cli/add_wizard_command_test.go | Updates assertions to match the new canonical note wording. |
| cmd/gh-aw/main.go | Migrates embedded examples from Long to Cobra Example for consistent help section ordering. |
| cmd/gh-aw/help_sections_order_test.go | Adds a regression test ensuring Usage appears before Examples in help output. |
| .github/workflows/smoke-copilot.lock.yml | Lockfile regenerated; includes compiled strictness/metadata changes. |
| .github/workflows/smoke-copilot-aoai-entra.lock.yml | Lockfile regenerated; includes compiled strictness/metadata changes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 2
| GH_AW_INFO_AWF_VERSION: "v0.27.10" | ||
| GH_AW_INFO_AWMG_VERSION: "" | ||
| GH_AW_INFO_FIREWALL_TYPE: "squid" | ||
| GH_AW_INFO_FRONTMATTER_EMOJI: "🧪" | ||
| GH_AW_COMPILED_STRICT: "true" | ||
| GH_AW_COMPILED_STRICT: "false" |
| GH_AW_INFO_FIREWALL_ENABLED: "true" | ||
| GH_AW_INFO_AWF_VERSION: "v0.27.10" | ||
| GH_AW_INFO_AWMG_VERSION: "" | ||
| GH_AW_INFO_FIREWALL_TYPE: "squid" | ||
| GH_AW_INFO_FRONTMATTER_EMOJI: "🧪" | ||
| GH_AW_COMPILED_STRICT: "true" | ||
| GH_AW_COMPILED_STRICT: "false" |
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel already completed for PR #41461 (workflow run 28177269137). Analysis result: 75/100 Acceptable, 2 design tests, 0 violations, APPROVE submitted. This is a duplicate invocation of the same workflow run. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #41461 does not have the 'implementation' label and has only 6 new lines of code in business logic directories (threshold: 100). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
Test Quality Sentinel ReportTest Quality Score: 75/100 - Acceptable Analyzed 2 test(s): 2 design, 0 implementation, 0 guideline violation(s). Metrics: 2 tests analyzed
Go: 2 (*_test.go); JavaScript: 0. Minor flags TestAddWizardCommand_UsesStandardThreePartWorkflowSpecWording (pkg/cli/add_wizard_command_test.go:20): bare assert.Contains calls without a descriptive third argument (guideline recommends a context message for every assertion). Also: no error-path coverage. Suggested fix: add a message arg like assert.Contains(t, cmd.Long, "...", "expected GHE note wording in Long help"). Test inflation note: cmd/gh-aw/help_sections_order_test.go adds 55 lines against main.go 16 net additions (ratio 3.4:1). The bulk is legitimate setup/teardown boilerplate for iterating 7 commands with t.Cleanup. Verdict Check passed. 0% implementation tests (threshold: 30%). Both tests verify observable user-facing behavior: the help section ordering contract and the exact CLI help text wording. Score breakdown: 40/40 behavioral + 15/30 edge-case + 20/20 duplication + 0/10 inflation = 75/100
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /improve-codebase-architecture and /zoom-out — approving with one minor suggestion on test coverage scope.
📋 Key Themes & Highlights
Positive Highlights
- ✅ Correct Cobra pattern: moving examples from
Longinto the dedicatedExamplefield is exactly how Cobra is meant to be used — it's idiomatic and ensures consistent help rendering across all output formats. - ✅ Mechanical consistency: the 7-command refactoring in
main.gois uniform and clean, with no behavioral changes. - ✅ Regression test added:
TestUsageAppearsBeforeExamplesInHelpOutputdirectly encodes the invariant this PR restores, which is excellent practice. - ✅ GHE note wording: the
add-wizardalignment withadd/updatecanonical phrasing is precise and the test assertions are updated in lockstep.
Observation
- The new test hardcodes exactly the 7 commands fixed in this PR. Adding the canonical commands (
addCmd,updateCmd,auditCmd) or switching to a dynamicrootCmd.Commands()walk would make the invariant self-enforcing for future contributors (see inline comment).
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 55.3 AIC · ⌖ 9.67 AIC · ⊞ 6.5K
| removeCmd, | ||
| runCmd, | ||
| versionCmd, | ||
| } |
There was a problem hiding this comment.
[/improve-codebase-architecture] The hardcoded list of 7 commands will silently miss any future command added with examples embedded in Long. New contributors have no signal that this list needs updating.
💡 Consider iterating dynamically over rootCmd.Commands()
Since this test lives in package main and has access to rootCmd, iterating over all subcommands would make the test self-maintaining:
for _, cmd := range rootCmd.Commands() {
if cmd.Example == "" {
continue // skip commands without examples
}
t.Run(cmd.CommandPath(), func(t *testing.T) {
// ... same assertions ...
})
}Alternatively, rounding out the list with addCmd, updateCmd, and auditCmd (the canonical commands this PR aligned add-wizard with) would at least confirm those also remain regression-free.
There was a problem hiding this comment.
No blocking issues. The core changes are correct: moving examples to Cobra's Example field is the right fix for help section ordering, the add-wizard wording is now consistent, and the regression test adds meaningful coverage.
Review themes
Lock file recompile (non-blocking): The GH_AW_COMPILED_STRICT flip in both smoke lock files looks alarming but is actually a correction — the source markdown declares strict: false (confirmed). The prior strict: true in the lock files was a pre-existing inconsistency. See inline comment on the cause.
Test maintainability (non-blocking): The help-order test hardcodes the 7 changed commands. This works today but won't automatically cover new commands that reintroduce the embedded-examples pattern. See inline comment for a self-maintaining alternative.
🔎 Code quality review by PR Code Quality Reviewer · 94.3 AIC · ⌖ 6.9 AIC · ⊞ 5.2K
| ) | ||
|
|
||
| func TestUsageAppearsBeforeExamplesInHelpOutput(t *testing.T) { | ||
| commands := []*cobra.Command{ |
There was a problem hiding this comment.
Hardcoded command list is a maintenance trap: new commands can silently reintroduce the Long-embedded-examples anti-pattern with zero test coverage.
💡 Suggested fix
Drive the test from rootCmd.Commands() rather than a static slice, skipping commands without an Example set:
for _, cmd := range rootCmd.Commands() {
if cmd.Example == "" {
continue // no examples to check ordering for
}
t.Run(cmd.CommandPath(), func(t *testing.T) {
// ... existing buffer / Help() / strings.Index logic
})
}This makes the check self-maintaining as new commands are added, with no manual list to keep in sync.
| GH_AW_INFO_FIREWALL_TYPE: "squid" | ||
| GH_AW_INFO_FRONTMATTER_EMOJI: "🧪" | ||
| GH_AW_COMPILED_STRICT: "true" | ||
| GH_AW_COMPILED_STRICT: "false" |
There was a problem hiding this comment.
Unintentional lock file recompile — result is correct, but traceability is lost. The previous value "true" was inconsistent with the source markdown, which declares strict: false on line 148 of smoke-copilot.md. The new value "false" is accurate.
However, this PR contains no changes to .github/workflows/*.md files, so running make recompile was unnecessary. Per AGENTS.md, the lock-file regeneration flow should only be triggered after workflow markdown changes. The silent fix to a pre-existing inconsistency is hard to audit — it looks like a behavioral change without an obvious cause.
💡 Suggestion
Add a PR description note explaining that the lock files were regenerated as a side effect and that the strictness change aligns with the source strict: false declaration. In future, avoid running make recompile in PRs that don't modify workflow .md sources.
CLI help output had two consistency gaps: 7 commands rendered
ExamplesbeforeUsage, andadd-wizardused different GitHub Enterprise shorthand-source note wording thanadd/update. This change normalizes both without altering command behavior.Help output ordering consistency
new,remove,enable,disable,compile,run, andversionto use CobraExamplefield instead of embedding examples inLong.Usage:Examples:Flags:GitHub Enterprise note wording consistency
add-wizardnote text withadd/update:shorthand source specsmicrosoft/* sourcessource URLs for other public github.com workflowsFocused regression coverage
Usageappears beforeExamplesfor all affected commands.add-wizardwording assertions to the canonical GHE phrasing.