[repository-quality] 🎯 Repository Quality Improvement Report - CLI Flag Boilerplate Duplication & Legacy-Alias Drift

[github-actions[bot]]

Analysis Date: 2026-07-28
Focus Area: CLI Flag Boilerplate Duplication & Legacy-Alias Drift
Strategy Type: Custom
Custom Area: Yes — pkg/cli already has a flags.go module with shared helpers (addEngineFlag, addRepoFlag, addOutputFlag, addJSONFlag) specifically to avoid per-command flag duplication, yet a newer deprecated-flag pattern (--no-security-scanner / legacy --disable-security-scanner) was introduced independently in 5 separate command files instead of being added to this shared module, reintroducing the exact class of duplication the module was built to prevent.

Executive Summary

gh-aw's CLI layer (pkg/cli) maintains a flags.go file whose explicit purpose is to centralize flag registration so commands stay consistent. Despite this, the --no-security-scanner flag (with its deprecated --disable-security-scanner alias) was copy-pasted into add_command.go, add_wizard_command.go, deploy_command.go, trial_command.go, and update_command.go — five near-identical blocks of registration, deprecation-marking, and OR-fallback resolution logic (~4 lines each, 20+ lines total plus five near-duplicate unit tests). A similar smaller-scale duplication exists for --force-delete-host-repo-before → --delete-host-repo-before and --disable-release-bump → --no-release-bump.

This drift creates real risk: any future change (new alias, help text tweak, additional mutual-exclusivity rule) must be replicated correctly across all five call sites, and a missed update already shows early divergence — update_command_test.go's test name (HasDisableSecurityScannerFlag) does not match the naming convention used by the other four tests (DeprecatesDisableSecurityScannerFlag), suggesting the update path wasn't kept in sync with the same rigor.

We recommend adding two small helpers to pkg/cli/flags.go — addSecurityScannerFlag(cmd) and a generic resolveDeprecatedBoolFlag(cmd, newName, oldName string) bool — and migrating all five commands to use them, consolidating ~20 lines of duplicated logic and its associated tests into one authoritative implementation.

Full Analysis Report

Focus Area: CLI Flag Boilerplate Duplication & Legacy-Alias Drift

Current State Assessment

pkg/cli/flags.go currently exposes 4 shared flag helpers (addEngineFlag, addEngineFilterFlag, addRepoFlag, addOutputFlag, addJSONFlag) — a good pattern that keeps flag definitions DRY. However, the deprecated boolean flag pattern used for security-scanner opt-outs was implemented ad hoc in each consuming command file rather than added to this module.

Metrics Collected:

Metric	Value	Status
Files duplicating no-security-scanner/disable-security-scanner registration	5 (add_command.go, add_wizard_command.go, deploy_command.go, trial_command.go, update_command.go)	⚠️
Lines of near-identical registration+resolution logic	~20 (4 per file)	⚠️
Corresponding near-duplicate unit tests	5 (*_test.go, one inconsistently named)	⚠️
Other deprecated-flag pairs with same pattern (force-delete-host-repo-before, disable-release-bump)	2	⚠️
Existing shared flag helpers in flags.go	5	✅
Total MarkDeprecated call sites across pkg/cli	8	⚠️

Findings

Strengths

• pkg/cli/flags.go establishes a clear, working precedent for centralizing repeated flag definitions (engine, repo, output, json).
• Deprecated flags are consistently marked via cmd.Flags().MarkDeprecated(...), so cobra will still emit its built-in deprecation warning to users.
• Each command correctly ORs the new and legacy flag values, so no functional regression exists today — this is a maintainability/consistency issue, not a bug.

Areas for Improvement

• [Medium] Duplicated --no-security-scanner/--disable-security-scanner registration and OR-resolution logic across 5 files (add_command.go:222-224, add_wizard_command.go:134-136, deploy_command.go:102-104, trial_command.go:129-131, update_command.go:178-180).
• [Low] Test naming drift: update_command_test.go:85 uses TestNewUpdateCommand_HasDisableSecurityScannerFlag while the other four analogous tests use the Deprecates... naming convention, signalling the update path was written independently rather than derived from a shared source of truth.
• [Low] Two other deprecated boolean flag pairs (force-delete-host-repo-before/delete-host-repo-before in trial_command.go, disable-release-bump/no-release-bump in update_command.go) follow the same one-off pattern and would benefit from the same generic helper.

Detailed Analysis

The --no-security-scanner flag with its --disable-security-scanner legacy alias appears in:

• pkg/cli/add_command.go:222-224 (register) / :131-133 (resolve)
• pkg/cli/add_wizard_command.go:134-136 (register) / :76-78 (resolve)
• pkg/cli/deploy_command.go:102-104 (register) / :152-154 (resolve)
• pkg/cli/trial_command.go:129-131 (register)
• pkg/cli/update_command.go:178,180 (register) / :86-88 (resolve)

Each resolve site repeats the identical 3-line idiom:

disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner")
disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner")
disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy

A single generic helper in flags.go, e.g. resolveDeprecatedBoolFlag(cmd *cobra.Command, newFlag, oldFlag string) bool, would remove the duplication risk and make future flag deprecations trivially consistent — that helper could also power the force-delete-host-repo-before and disable-release-bump cases with no additional code.

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Add shared addSecurityScannerFlag helper to flags.go

Priority: Medium
Estimated Effort: Small
Focus Area: CLI Flag Boilerplate Duplication

Description: Add a new function addSecurityScannerFlag(cmd *cobra.Command) to pkg/cli/flags.go that registers both --no-security-scanner and the deprecated --disable-security-scanner alias, and marks the legacy one deprecated in a single call, matching the existing help text used in all 5 call sites.

Acceptance Criteria:

• New function added to pkg/cli/flags.go with doc comment following existing style
• Function registers both flags with identical wording to current implementations
• Unit test added covering flag registration and deprecation marking

Code Region: pkg/cli/flags.go

In pkg/cli/flags.go, add a function `addSecurityScannerFlag(cmd *cobra.Command)` that:
1. Registers `cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content")`
2. Registers `cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content")`
3. Marks the legacy flag deprecated: `_ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead")`
Follow the existing doc-comment style used for addEngineFlag/addJSONFlag in the same file. Add a small test in flags_test.go (create if it doesn't exist) verifying both flags exist on a test cobra.Command and that the legacy flag is marked deprecated.

---

Task 2: Add generic resolveDeprecatedBoolFlag helper and migrate all 5 call sites

Priority: High
Estimated Effort: Medium
Focus Area: CLI Flag Boilerplate Duplication

Description: Add a generic helper resolveDeprecatedBoolFlag(cmd *cobra.Command, newFlag, oldFlag string) bool to pkg/cli/flags.go that reads both flags and returns true if either is set. Replace the duplicated 3-line OR-resolution idiom in add_command.go, add_wizard_command.go, deploy_command.go, update_command.go (and the analogous pattern for force-delete-host-repo-before/delete-host-repo-before and disable-release-bump/no-release-bump in trial_command.go and update_command.go) with a call to this helper. Also replace the direct flag registration in these 5 files that currently duplicate the two cmd.Flags().Bool(...) + MarkDeprecated calls for security-scanner with the new addSecurityScannerFlag helper from Task 1.

Acceptance Criteria:

• resolveDeprecatedBoolFlag added to flags.go with unit test covering both-unset, new-set, old-set, and both-set cases
• All 5 command files (add_command.go, add_wizard_command.go, deploy_command.go, trial_command.go, update_command.go) use addSecurityScannerFlag for registration instead of duplicated cmd.Flags().Bool + MarkDeprecated pairs
• Resolution sites in add_command.go, add_wizard_command.go, deploy_command.go, update_command.go use resolveDeprecatedBoolFlag instead of the manual OR pattern
• trial_command.go's force-delete-host-repo-before/delete-host-repo-before pair and update_command.go's disable-release-bump/no-release-bump pair also migrated to use resolveDeprecatedBoolFlag where a resolution site exists
• Rename TestNewUpdateCommand_HasDisableSecurityScannerFlag to TestNewUpdateCommand_DeprecatesDisableSecurityScannerFlag for naming consistency with sibling tests
• All existing tests pass: go test ./pkg/cli/...
• No functional/behavioral change — flags still resolve to the same effective boolean in every command

Code Region: pkg/cli/flags.go, pkg/cli/add_command.go:131-133,222-224, pkg/cli/add_wizard_command.go:76-78,134-136, pkg/cli/deploy_command.go:102-104,152-154, pkg/cli/trial_command.go:118-131, pkg/cli/update_command.go:86-88,177-180

In pkg/cli/flags.go, add:

func resolveDeprecatedBoolFlag(cmd *cobra.Command, newFlag, oldFlag string) bool {
	newVal, _ := cmd.Flags().GetBool(newFlag)
	oldVal, _ := cmd.Flags().GetBool(oldFlag)
	return newVal || oldVal
}

Add a doc comment explaining it resolves a new flag against its deprecated legacy alias, returning true if either is set.

Then, in each of pkg/cli/add_command.go, pkg/cli/add_wizard_command.go, pkg/cli/deploy_command.go, and pkg/cli/update_command.go, replace the existing 3-line pattern:
    disableSecurityScanner, _ := cmd.Flags().GetBool("no-security-scanner")
    disableSecurityScannerLegacy, _ := cmd.Flags().GetBool("disable-security-scanner")
    disableSecurityScanner = disableSecurityScanner || disableSecurityScannerLegacy
with:
    disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner")

Also replace the flag-registration blocks:
    cmd.Flags().Bool("no-security-scanner", false, "Skip security scanning of workflow markdown content")
    cmd.Flags().Bool("disable-security-scanner", false, "Skip security scanning of workflow markdown content")
    _ = cmd.Flags().MarkDeprecated("disable-security-scanner", "use --no-security-scanner instead")
in add_command.go, add_wizard_command.go, deploy_command.go, trial_command.go, and update_command.go with a single call:
    addSecurityScannerFlag(cmd)

In trial_command.go, also consider applying resolveDeprecatedBoolFlag if/when a resolution site for force-delete-host-repo-before is added (currently only registered, not read back — verify current usage before changing behavior). In update_command.go, apply resolveDeprecatedBoolFlag to the disable-release-bump/no-release-bump pair if a matching resolution site exists.

Rename the test TestNewUpdateCommand_HasDisableSecurityScannerFlag in pkg/cli/update_command_test.go to TestNewUpdateCommand_DeprecatesDisableSecurityScannerFlag to match the naming convention in add_command_test.go, add_wizard_command_test.go, deploy_command_test.go, and trial_command_test.go.

Run go test ./pkg/cli/... and make fmt after the change, and ensure all existing flag-deprecation tests still pass unmodified in behavior (only the renamed test name should differ).

---

Task 3: Add regression test for deprecated flag resolution helper edge cases

Priority: Low
Estimated Effort: Small
Focus Area: CLI Flag Boilerplate Duplication / Testing

Description: Add a dedicated flags_test.go (or extend if it exists) covering resolveDeprecatedBoolFlag behavior in isolation: neither flag set (false), only new flag set (true), only legacy flag set (true), both set (true). This guards the new shared helper against regressions independently of the 5 commands that consume it.

Acceptance Criteria:

• pkg/cli/flags_test.go contains a table-driven test for resolveDeprecatedBoolFlag covering all 4 truth-table combinations
• Test uses a throwaway *cobra.Command with both bool flags registered, setting them via cmd.Flags().Set(...)
• go test ./pkg/cli/... -run TestResolveDeprecatedBoolFlag passes

Code Region: pkg/cli/flags_test.go

Create or extend pkg/cli/flags_test.go with a table-driven test function TestResolveDeprecatedBoolFlag that:
1. Constructs a throwaway *cobra.Command with two bool flags "new-flag" and "old-flag" both defaulting to false.
2. For each of the 4 combinations (both false, new-flag true, old-flag true, both true), sets the flags via cmd.Flags().Set("new-flag", "true") / cmd.Flags().Set("old-flag", "true") as needed and asserts resolveDeprecatedBoolFlag(cmd, "new-flag", "old-flag") returns the expected boolean (false only when both are false).
3. Follows existing test style/conventions in the pkg/cli package (e.g., use of testify or plain t.Errorf as used elsewhere in that package).

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-07-27	Output Stream Abstraction Gap — Hardcoded os.Stdout in Render Functions	Custom	Y	Identified 3 tasks for stream injection
2026-07-26	Error Chain Transparency & Non-Wrapping fmt.Errorf Gap	Custom	Y	4 tasks generated
2026-07-25	Options Struct Pattern Adoption for High-Arity Functions	Custom	Y	3 tasks generated
2026-07-24	Compile-Time Interface Satisfaction Assertions	Custom	Y	3 tasks generated
2026-07-23	Logger Name Collision & Instantiation Hygiene	Custom	Y	4 tasks generated

---

🎯 Recommendations

Immediate Actions (This Week)

1. Implement Task 1 (addSecurityScannerFlag helper) — Priority: Medium
2. Implement Task 2 (generic resolution helper + migration) — Priority: High

Short-term Actions (This Month)

1. Audit remaining pkg/cli deprecated-flag pairs (force-delete-host-repo-before, disable-release-bump) for the same consolidation — Priority: Medium
2. Add contributor guidance in pkg/cli package doc noting that new deprecated boolean flags should use resolveDeprecatedBoolFlag — Priority: Low

Long-term Actions (This Quarter)

1. Consider a lightweight lint/codemod check (per pkg/linters) that flags any new cmd.Flags().Bool(...) + MarkDeprecated pair not routed through the shared helper, to prevent regression — Priority: Low

---

📈 Success Metrics

• Duplicated flag-registration blocks for security-scanner: 5 → 1 (via shared helper)
• Duplicated OR-resolution idioms: 4 → 0 (via resolveDeprecatedBoolFlag)
• Test naming consistency across deprecated-flag tests: 4/5 consistent → 5/5 consistent

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate this focus area in 1 quarter (revisit only if new deprecated-flag pairs emerge without using the shared helper)

---

Generated by Repository Quality Improvement Agent
Next analysis: 2026-07-29 — Focus area selected by diversity algorithm

Generated by ⚡ Repository Quality Improvement Agent · sonnet50 · 48.1 AIC · ⌖ 5.24 AIC · ⊞ 9.1K · ◷

• expires on Jul 29, 2026, 5:27 AM UTC-08:00