test(recommendations): cover expiry through command output

[cristim]

Reopening of #2134 (closed unmerged when its base branch codex/go70-cli-expiry-consumer was deleted after #2133 merged). Same branch, retargeted at main. Original: test(recommendations): cover expiry through command output. Needs a rebase onto main to absorb #2133 and #2135.

Summary by CodeRabbit

• Tests
  • Expanded coverage for EC2 reservation-expiry reporting, including demand availability, reservation timing, account filtering, and coverage thresholds.
  • Added checks for CSV results, totals, warnings, and cloud-service request handling.
  • Updated completeness checks to use service-appropriate validations for EC2, RDS, and other supported services.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: c8aad556-56b4-4a8e-bb72-472d79f34f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 65a02a7 and ef3a2dc.

📒 Files selected for processing (3)

• cmd/recommendation_completeness_proxy_test.go
• cmd/recommendation_completeness_test.go
• cmd/reservation_expiry_command_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The completeness test harness now covers EC2 reservation-expiry scenarios. New command tests check scenario-specific CSV output and validate AWS requests against synthetic responses.

Changes

EC2 reservation-expiry test coverage

Layer / File(s)	Summary
Completeness harness integration cmd/recommendation_completeness_proxy_test.go, cmd/recommendation_completeness_test.go	The proxy serves reservation-expiry responses and checks EC2 requests. The harness passes details mode and reservation-expiry arguments, then checks EC2 CSV output.
Command scenarios and synthetic AWS responses cmd/reservation_expiry_command_test.go	Six scenarios check CSV rows, totals, warnings, and selected failure messages. A synthetic AWS proxy validates requests and records operations, coverage queries, accounts, and engines.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to ef3a2

The synthetic command tests distinguish the coverage boundary outcomes. No identified issue blocks merge after normal checks; real-account acceptance remains unverified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding recommendation tests that cover reservation-expiry behavior through command output.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Rebased onto main at 65a02a7. Final head: ef3a2dc.

The old stack contained the parent expiry fix and its dependency-pin commit. The parent fix is now merged. The only rebase conflict was the older Azure dependency pin in go.mod/go.sum; retained main's newer 56555e1 pin, so the redundant pin commit dropped. All three command-test files were retained unchanged (range-diff confirms the test patch is identical). No dependency diff remains.

Finding disposition: #2134 had a skipped review and #2143 had a rate-limited review, with no actionable findings or inline comments to fix, dismiss, or defer. A green CodeRabbit status for the rate-limited pass was not a completed review.

Fresh macOS verification at this head: formatting, go vet ./..., go build -o /private/tmp/cudly-cli2143-codex ./cmd, and go test ./cmd/... -count=1 passed. The command tests exercise the actual root command, SDK requests and CSV output against synthetic loopback TLS fixtures. Real-account acceptance remains required and unverified. No merge is performed; the separate reviewer retains all merge decisions and the exact claude-opus-5-5 final-SHA review gate remains open.

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

CodeRabbit completed the review at ef3a2dc: the walkthrough lists all three changed files, exact source/covered SHA matches this head, and says no actionable comments were generated. The CodeRabbit commit status says Review completed; there are no inline comments.

Disposition: no actionable, stale or out-of-scope findings. The automated docstring-coverage suggestion is dismissed because these unexported test helpers are covered by the owner's sparing-comments convention (default to no comments), and adding documentation solely for a coverage percentage would add unrelated prose. No code change or duplicate review request is needed.

Local verification is recorded above. Actions CI remains watched. Exact claude-opus-5-5 independent review at this head and real-account acceptance are still separate open gates; this is not a merge authorization.

[cristim]

Final implementation handoff at ef3a2dc: both watched workflows completed successfully:

• CI - Build & Test: https://github.com/LeanerCloud/cloud-commitments-cli/actions/runs/37644729390
• pre-commit: https://github.com/LeanerCloud/cloud-commitments-cli/actions/runs/37644729417

Fresh gh pr checks reports all workflow checks passing; standalone Trivy/gosec annotations are neutral/skipped, while the workflow Security Scanning job passed. CodeRabbit completed exact-head coverage with zero actionable/inline findings; the docstring suggestion disposition is recorded above. PR is OPEN, CLEAN and MERGEABLE in GitHub. Local worktree is clean and tracks the published head. Watch state is pushed-cr-clean; both CI watchers and the CodeRabbit watcher reached terminal states.

No additional source changes or fix commits were necessary after recovering the session. All original files and worktrees remain preserved. The implementation assignment is handed off without merging. Exact claude-opus-5-5 final-SHA independent review and real-account acceptance remain required; synthetic command fixtures do not satisfy either gate.

[cristim]

Independent adversarial review completed on exact head ef3a2dc076f73466babad2fc5e762f038ba76a6b by a separate Codex reviewer. Two passes: no actionable findings. Review traced the real root-command path through filtering, coverage, expiry sizing and CSV output, and checked pinned shared/AWS source rather than trusting this PR's claims.

Independent realistic local command verification passed all six TestReservationExpiryCommand scenarios (totals 36/18/6/36/8/6). An additive baseline with the pre-fix CLI helper reproduced the missing-demand failure: old output had 0% coverage and 8/account, total 24; reviewed code retained 60% coverage and 2/account, total 6. Other five baseline scenarios passed. This proves the CLI regression while retaining current dependency pins. Fixtures use synthetic credentials, isolated config and an operation allowlist that rejects purchase calls. No live-cloud acceptance or purchase was performed; live-account behavior remains a coverage gap.

CodeRabbit completed review of all three files on this exact head with no actionable findings (run c8aad556-56b4-4a8e-bb72-472d79f34f3f). CR was not waived. Exact-head CI Build & Test run 37644729390 and pre-commit run 37644729417 succeeded.

Owner's current override permits independent available-model review and realistic local verification, superseding the prior exact-model and mandatory live-account gates. Merge still requires adequate current local verification, unchanged reviewed head, freshly green CI, clean mergeability and ordinary branch protections.

Local full-suite reliability investigation: initial unchanged full suite passed, but later runs encountered unrelated GCP signal timing failures and synchronous test-output pipe blocking. Unchanged focused GCP signal tests passed three repetitions and a race run; unchanged CSV cap test passed under race. Ancillary AWS calls escaping mocks are tracked in #2148. Full-suite resource-contained re-verification remains in progress; this comment does not claim that latest full-suite gate is green.

[cristim]

Final local verification is green on unchanged reviewed head ef3a2dc076f73466babad2fc5e762f038ba76a6b:

GOTOOLCHAIN=go1.26.6 GOGC=10 GOFLAGS=-p=1 HTTP_PROXY=http://127.0.0.1:1 HTTPS_PROXY=http://127.0.0.1:1 NO_PROXY=localhost,127.0.0.1,::1 AWS_EC2_METADATA_DISABLED=true go test -short ./... -count=1 -timeout=20m

Exit 0, cmd package 383.064s. All standard local tests retained; no source overlay or test-specific exclusions. Reduced build concurrency and more frequent GC are resource containment, not a proven root-cause fix. Earlier unrelated signal timing/pipe-capture failures remain honestly recorded; current focused signal race and CSV-cap race tests passed unchanged. CI's exact-head unit and integration suites are also green. No live-account or purchase acceptance was performed.

Ready for ordinary owner-authorized SHA-guarded merge after immediately refreshing exact head, CI and clean mergeability. CodeRabbit actually completed, so there is no waived-review debt.

[cristim]

Merged normally as bfdb4f6cea1b56b2b2ea0a6482873e3b13123ced, guarded by exact reviewed head ef3a2dc076f73466babad2fc5e762f038ba76a6b. Immediately before merge: OPEN/CLEAN/MERGEABLE, unchanged head and all checks settled green. No admin bypass or branch deletion.

Post-merge CI Build & Test 37695066531 and pre-commit 37695066679 completed successfully on the exact merge commit. Merged cmd, go.mod and go.sum are identical to the reviewed head; the only other merged-tree difference is unrelated docs/cli/cloud-setup.md from main. Fresh command-path verification on the identical executable/test source passed all six synthetic scenarios, package3.475s, totals36/18/6/36/8/6. Fresh go build -o ... ./cmd and help smoke also passed. Full local suite passed383.064s; independent old-helper regression failed as expected and reviewed implementation passed.

Independent adversarial review and completed exact-head CodeRabbit review had no actionables. CR was not waived. Coverage is realistic local SDK/CLI/CSV fixtures, with no live-account or purchase acceptance; that coverage limit is recorded rather than used as a merge gate under the owner override.

Follow-up issues filed: #2148 for ancillary AWS mock isolation, #2150 for synchronous output capture and signal-failure diagnostics. Existing worktree and branch preserved; local tracking updated to merged. No duplicate merge, watcher or guidance edit created.