fix(aws/recommendations): parseRecommendations silently drops unparseable offers, so a search can present an incomplete menu as complete

[cristim]

Summary

parseRecommendations in providers/aws/recommendations/parser_ri.go:30 logs a
warning to stderr and continues when a single recommendation detail fails to
parse. The failed detail is dropped from the returned slice and no signal of
the drop reaches the caller. That is defensible graceful degradation for a
batch job, but it became load-bearing when cudly_search_recommendations
started promising a complete offer menu, so it should now fail loud (at least
for the MCP caller).

// providers/aws/recommendations/parser_ri.go
rec, err := c.parseRecommendationDetail(ctx, &details, params)
if err != nil {
    log.Printf("Warning: Failed to parse recommendation detail %d: %v", i, err)
    continue
}

Why the search fan-out made this load-bearing

mcp/tools/search_recommendations.go fans an AWS reservation search out over
every (term, payment option) combination when the caller omits term_years
and/or payment_option, because GetReservationPurchaseRecommendation returns
only one cell per request. fetchSearchCombos deliberately fails the WHOLE
search when any one combo's API call errors, and its own comment says why:

silently returning 5 of 6 offers is indistinguishable from "these are all
your options" and recreates the exact defect this fan-out exists to fix.

That guarantee only holds at the combo boundary. Inside a combo, the shared
parser silently drops individual offers, so the same defect the fan-out was
built to remove is reproduced one level down, where nothing checks for it.

Failure scenario

1. A model calls cudly_search_recommendations with provider="aws",
   service="ec2" and no term_years / payment_option, so the tool issues
   all 6 combos.
2. For the 3yr/all-upfront combo, Cost Explorer returns several recommendation
   details. The best one carries a quantity or cost field the parser cannot
   handle (unparseable numeric string, a field AWS newly leaves unset, an
   instance type the sizing path rejects).
3. parseRecommendations logs to stderr and continues. The MCP client never
   sees stderr; the JSON-RPC response carries the remaining offers only.
4. The tool returns success with a menu the model presents as complete. The
   model recommends the best SURVIVING offer, and the human buys it. The
   dropped offer may have been the cheapest one.

This is silent under-reporting on a purchase-decision path: the caller cannot
distinguish "AWS had nothing better" from "we could not parse the better one".
The same drop also reaches the scheduler's discovery sweep and the CLI's CSV
output, which is why the fix is not a one-line change here.

Why this is filed rather than fixed in PR LeanerCloud/cloud-commitments-cli#1495

parseRecommendations is shared. internal/scheduler's discovery sweep calls
into the same path and legitimately prefers partial progress over none in a
batch job, so flipping continue to a hard error changes behaviour for a
consumer that wants the current semantics. The fix needs a per-caller policy
(strict for the interactive/purchase-decision path, tolerant for the batch
sweep) or a returned skipped-count the MCP tool can refuse on, plus tests on
both sides. That is outside the scope of PR LeanerCloud/cloud-commitments-cli#1495, which only touches
mcp/tools.

Suggested direction (not prescriptive)

• Have parseRecommendations report what it dropped (count plus per-detail
  reason) instead of only logging it.
• Let the caller decide: cudly_search_recommendations refuses the search with
  an error naming the unparseable detail, matching fetchSearchCombos'
  existing all-or-nothing contract; the scheduler keeps tolerating and records
  the skipped count.
• Regression test with a fixture whose detail genuinely fails
  parseRecommendationDetail, asserting the MCP search errors rather than
  returning a short menu, and that the scheduler path still returns the
  survivors.

References

• providers/aws/recommendations/parser_ri.go:18-40 (parseRecommendations)
• mcp/tools/search_recommendations.go (fetchSearchCombos, searchCombos)
• Surfaced by the adversarial review of PR feat(mcp): CUDly MCP server for RI/SP/CUD purchases across AWS, Azure, GCP cloud-commitments-cli#1495; the fan-out landed in
  d76aa7c00.

[cristim]

Refs LeanerCloud/cloud-commitments-mcp#37. The MCP rollout portion is merged at e073769ac91505956c5deab733208a264e215fde (2026-10-03), with reviewed head d04d15bb6fec29e99ee7a3a7d21a5ecdf3b44a4a.

The merged tree is exactly 1d39ebab8f9e54713389d14e7c684e033da9201d, identical to the independently reviewed and locally verified head. A narrow additive fetch and full tree comparison found no byte differences, so the native macOS proof applies to the merged source.

Independent gpt-6-astra review covered the full MCP PR and the published AWS producer contract. All eight registered MCP protocol cases passed at the final head; fresh parent-dependency and diagnostic-drop controls each failed the four malformed cases and passed the four valid/empty controls. Full race tests, actual command build, and frozen-database source/binary scans passed. These are connected synthetic-fixture checks, with no live cloud calls or purchases. The dependency repair preserves all four producer pins and selects the patched OTEL SDK closure.

Both exact-merge-SHA workflows passed: pre-commit and CI - Build & Test. The final PR verification summary records the independent review and the original head's checks.

This records the MCP rollout portion. Keep this issue open for the coordinated CLI and Platform rollout scope; their state is preserved by this update.

[cristim]

CLI rollout verified: cloud-commitments-cli PR 2129 merged as 1ce4658a0c29ca3f8bd289a428166883a0b0d1de on 2026-10-03 at 19:21:15 UTC.

The additively fetched merge tree is exactly ac116220c0c1adaf74a230f9ec847c614d061fb6, identical to independently reviewed head 6abdb1469c04b0876bb983d22eb6af9ca92b4b20. Native Go 1.26.6 proof therefore applies unchanged: full race suite, 15 actual root-command/config-loader/published-SDK/CSV scenarios plus five helper cases, actual command build, and fixed source/binary vulnerability scans. Provider boundaries used synthetic fixtures; no live purchases or cloud execution are claimed. All published provider pins are preserved, including AWS v0.0.0-20261002152209-006ef5c8d0a2.

Both exact merge-SHA workflows passed: pre-commit 37147583239 and CI 37147583241. Their watcher handles are terminal and reaped. Source checkout and existing branches were preserved.

CLI rollout is complete. Platform rollout remains pending, so this shared issue stays open.