Severity: P2. Confidence: high.
Affected files:
pkg/common/types.go:153-154providers/gcp/services/computeengine/client.go:43-52providers/aws/services/savingsplans/client.go:376-385providers/azure/services/internal/reservations/purchase.go:62providers/azure/internal/recommendations/converter.go:329internal/scheduler/scheduler.go:1306
Evidence:
types.go declares Term string // 1yr, 3yr and PaymentOption string on the central Recommendation struct, while sibling fields (Provider, Service, CommitmentType) are typed string enums in the same file. Grep finds ~181 term/payment literal occurrences across non-test code. Term parsing is independently re-implemented with different vocabularies: GCP accepts "1yr"/"1"/"12mo", AWS savingsplans only "1yr"/"1", memorydb additionally "36", Azure has ParseTermYears plus a separate termToMonths, and the scheduler re-parses "3yr" -> 3 with a default-to-3 fallback.
Impact:
Exactly the bug class the memory garden documents (feedback_prefer_typed_enums): each parser's accepted set differs, so a term valid in one layer is silently mis-handled in another. Live silent-default pockets remain: memorydb falls back to "1yr" for any unrecognized term, Azure ParseTermYears maps empty term to a 1-year purchase, termToMonths defaults unknown terms to 12 months in cost math, and scheduler.go defaults invalid terms to 3 in ID derivation. 181 call sites make any vocabulary change a shotgun edit.
Recommendation:
Introduce common.Term and common.PaymentOption typed string enums with a single ParseTerm/ParsePaymentOption (error on unknown) in pkg/common, validate at boundaries (CSV, HTTP, DB read-back), and migrate provider switches to the typed value.
Verifier verdict: confirmed - existing mitigations (NormalizePaymentOption for LeanerCloud/cloud-commitments-cli#698, per-provider whitelists) are compensating shims that are themselves evidence for the debt; the class has bitten repeatedly (LeanerCloud/cloud-commitments-cli#698, PR LeanerCloud/cloud-commitments-cli#1047).
Source: docs/reviews/codebase-review-2026-06-10.md (automated multi-dimension code review, adversarially verified for P1/P2)
Findings from the 2026-09-02 codebase audit
Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.
A09-019 (medium)
One more divergent term parser, and the only one whose failure mode is a recorded zero rather than a wrong default. termMonths (pkg/common/audit.go:84-99) matches two bare literals and returns 0 for everything else with a log.Printf warning, and NewAuditRecord writes that 0 into AuditRecord.Term unconditionally (:68). The JSONL audit log is the artifact reconciled against purchase_history, so a record saying a real commitment was purchased carries a zero-month term with no way to recover the real value. A non-canonical term reaches it in practice: normaliseTerm in providers/azure/internal/recommendations/converter.go:350-352 passes any Azure term other than P1Y/P3Y through verbatim onto Recommendation.Term. The single production caller is cmd/multi_service.go:401. A shared ParseTerm returning an error would make NewAuditRecord refuse the record instead. Audit finding A09-019.
Severity: P2. Confidence: high.
Affected files:
pkg/common/types.go:153-154providers/gcp/services/computeengine/client.go:43-52providers/aws/services/savingsplans/client.go:376-385providers/azure/services/internal/reservations/purchase.go:62providers/azure/internal/recommendations/converter.go:329internal/scheduler/scheduler.go:1306Evidence:
types.go declares
Term string // 1yr, 3yrandPaymentOption stringon the central Recommendation struct, while sibling fields (Provider, Service, CommitmentType) are typed string enums in the same file. Grep finds ~181 term/payment literal occurrences across non-test code. Term parsing is independently re-implemented with different vocabularies: GCP accepts "1yr"/"1"/"12mo", AWS savingsplans only "1yr"/"1", memorydb additionally "36", Azure has ParseTermYears plus a separate termToMonths, and the scheduler re-parses "3yr" -> 3 with a default-to-3 fallback.Impact:
Exactly the bug class the memory garden documents (feedback_prefer_typed_enums): each parser's accepted set differs, so a term valid in one layer is silently mis-handled in another. Live silent-default pockets remain: memorydb falls back to "1yr" for any unrecognized term, Azure ParseTermYears maps empty term to a 1-year purchase, termToMonths defaults unknown terms to 12 months in cost math, and scheduler.go defaults invalid terms to 3 in ID derivation. 181 call sites make any vocabulary change a shotgun edit.
Recommendation:
Introduce
common.Termandcommon.PaymentOptiontyped string enums with a single ParseTerm/ParsePaymentOption (error on unknown) in pkg/common, validate at boundaries (CSV, HTTP, DB read-back), and migrate provider switches to the typed value.Verifier verdict: confirmed - existing mitigations (NormalizePaymentOption for LeanerCloud/cloud-commitments-cli#698, per-provider whitelists) are compensating shims that are themselves evidence for the debt; the class has bitten repeatedly (LeanerCloud/cloud-commitments-cli#698, PR LeanerCloud/cloud-commitments-cli#1047).
Source: docs/reviews/codebase-review-2026-06-10.md (automated multi-dimension code review, adversarially verified for P1/P2)
Findings from the 2026-09-02 codebase audit
Added by an automated audit of
3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd(tip oforigin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report:docs/audits/codebase-audit-2026-09-02.md.A09-019 (medium)
One more divergent term parser, and the only one whose failure mode is a recorded zero rather than a wrong default. termMonths (pkg/common/audit.go:84-99) matches two bare literals and returns 0 for everything else with a log.Printf warning, and NewAuditRecord writes that 0 into AuditRecord.Term unconditionally (:68). The JSONL audit log is the artifact reconciled against purchase_history, so a record saying a real commitment was purchased carries a zero-month term with no way to recover the real value. A non-canonical term reaches it in practice: normaliseTerm in providers/azure/internal/recommendations/converter.go:350-352 passes any Azure term other than P1Y/P3Y through verbatim onto Recommendation.Term. The single production caller is cmd/multi_service.go:401. A shared ParseTerm returning an error would make NewAuditRecord refuse the record instead. Audit finding A09-019.