fix(azure): dedupe subscriptions in the multi-subscription fan-out to prevent double-counted savings #59

Summary

NewMultiSubscriptionRecommendationsClient (providers/azure/recommendations_multi_subscription.go) builds one subscriptionClient per entry in the accounts slice it is handed, without deduplicating by subscription ID. If that slice ever contains the same subscription twice, the fan-out queries it twice and mergeSubscriptionResults concatenates both responses — double-counting that subscription's recommendations, and therefore its reported savings.

What is and is not verified

• Verified: the constructor does not dedupe, and the merge step is a plain concatenation, so a duplicate entry would double-count. Attempted in PartialSubscriptionFailureError would also count the subscription twice.
• Verified: the AccountFilter path is deduplicated (added in feat(azure): org-wide multi-subscription recommendation collection (closes #553) cloud-commitments-cli#1520 while fixing the partially-matching-filter gap), so a duplicated filter entry is safe. The unguarded path is the accounts slice itself, which reaches the constructor from getOrFetchAccounts / ARM subscriptions.List.
• Not verified: whether ARM's subscriptions.List can actually return the same subscriptionID on more than one page. The review that surfaced this could not establish that it can, and could not establish that it cannot. It is stated openly here rather than asserted either way. Pagination overlap under concurrent tenant mutation is the plausible mechanism, but that is a hypothesis, not an observation.

Given the uncertainty, this is filed as cheap defensive hardening on a money-affecting aggregate rather than as a confirmed bug.

Suggested direction

Dedupe by subscription ID in NewMultiSubscriptionRecommendationsClient, keeping first occurrence, and add a unit test that passes a duplicated account and asserts the subscription is queried once and its recommendations appear once. If a duplicate is dropped, consider a logging.Warnf so a genuinely duplicate-emitting API surface becomes visible rather than silently absorbed.

An alternative placement is fetchAccounts in providers/azure/accounts_cache.go, which would protect every consumer of GetAccounts rather than just the fan-out. That is the broader fix and may be the better one.

Provenance

Found during the adversarial review of LeanerCloud/cloud-commitments-cli#1520; not fixed there to keep that PR's diff scoped to the reported findings.