Skip to content

chore(cli): unread Config.Providers field and the test-only legacy processService path #2053

Description

@cristim

Summary

Two low-severity dead-code findings in cmd/. Config.Providers is the only occurrence of that identifier in the whole cmd/ tree: it is declared on the struct that documents the CLI's surface, never bound to a flag and never read, so a reader adding multi-provider support reasonably assumes a --providers flag exists. processService, processRegionRecommendations and applyCommonCoverage are reachable only from tests; the comments already call them a legacy, test-only path. Roughly 140 lines of purchase-executing code, including the --max-instances refusal guard at cmd/multi_service_helpers.go:412, are maintained and reviewed as if live, and the nine tests that drive them report coverage for a pipeline the CLI never runs, which overstates how well the real fetchAllRecs plus executePurchasePipeline path is tested.

Location

All at 3c0f8ac.

  • A10-026 cmd/main.go:47 (Providers []string); /usr/bin/grep -rn "Providers" cmd/ returns only that line and no .Providers selector exists anywhere. Delete the field.
  • A10-027 cmd/multi_service.go:653 (processService, callers: multi_service_coverage_test.go x3, multi_service_test.go x6), :667 (processRegionRecommendations, sole caller is processService), cmd/multi_service_helpers_test.go:277 (sole caller of applyCommonCoverage). Delete all three with the tests that exist only to drive them, or move whatever behaviour is still worth pinning onto fetchAndFilterRegionRecs and executePurchasePipeline.

Failure scenario

No runtime effect. The risk is that a guard on the legacy path (the --max-instances refusal) is believed to protect the live CLI when the live pipeline has to be checked separately.

Evidence

ExcludeAccounts        []string
Providers              []string
IncludeRegions         []string
// processService processes a single service and returns recommendations and results.
// Used by legacy callers; new code should use fetchAllRecs + executePurchasePipeline.
func processService(ctx context.Context, awsCfg aws.Config, ...

Suggested fix

Per checkbox above. Before deleting the legacy tests, confirm each behaviour they pin has an equivalent assertion on the live path.


Found by the 2026-09-02 codebase audit, findings A10-026, A10-027, each reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions