fix(cli): only print "CSV report written to" after a complete CSV write

[IAmJSD]

Root cause

writeMultiServiceCSVReport (cmd/multi_service_csv.go) had two problems:

1. It used defer writer.Flush() and never read writer.Error(). csv.Writer buffers its output, so for a normal-sized report every byte reaches the file at that flush. A failed flush (full disk, failing network mount) lost the header and rows, and the function still returned nil. A failed file.Close() was only logged.
2. With zero results it returned nil before os.Create, so no file was created.

Both callers in cmd/multi_service.go (writeReportAndSummary and runCSVPurchases) printed CSV report written to: <path> on any nil return. That told the operator a file existed when it was truncated, empty or missing. On a --purchase run, this CSV is the only record of what was bought.

Fix

• The rendering moves into writeMultiServiceCSV(io.Writer, results), which calls writer.Flush() explicitly and returns writer.Error().
• writeMultiServiceCSVReport keeps its signature and its empty-results behaviour (no file, nil). It now returns a file.Close() error instead of only logging it.
• A new writeCSVReport(results, path) helper is now used by both callers. With no results it prints No purchase results; CSV report not written. On error it logs the same warning as before. It prints CSV report written to: only after a complete, flushed and closed write.

Closes #2004

Verification

All commands were run in the worktree with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null, because this machine's default AWS profile is invalid and unrelated tests fail without it.

New tests in cmd/multi_service_csv_test.go:

• TestWriteMultiServiceCSV_FlushErrorIsReturned: a writer that always fails must produce an error.
• TestWriteReportAndSummary_NoResultsDoesNotClaimCSVWritten: a run with no results creates no file, does not print "written to", and does print "not written".
• TestWriteCSVReport_FailureDoesNotClaimCSVWritten: guard test. A create failure prints no success line, and a successful write prints the path. This one already passed before the fix.

Fail before (simulated): the old code can't compile against the new test helpers, so I built a pre-fix copy of this branch. It restores defer writer.Flush() with no writer.Error() check and drops the empty-results branch. On that copy:

go test ./cmd -run 'TestWriteReportAndSummary_NoResults|TestWriteMultiServiceCSV_|TestWriteCSVReport' -v
--- FAIL: TestWriteMultiServiceCSV_FlushErrorIsReturned   (An error is expected but got nil.)
--- FAIL: TestWriteReportAndSummary_NoResultsDoesNotClaimCSVWritten   ("📋 CSV report written to: .../report.csv" printed with no file)
--- PASS: TestWriteCSVReport_FailureDoesNotClaimCSVWritten

Pass after: all three new tests and the existing TestWriteMultiServiceCSVReport* tests pass.

Also run:

• go test ./...: ok
• go vet ./...: clean
• golangci-lint run ./... (v2.10.1, the CI pin): 0 issues
• make build: ok

Coverage gap: the flush failure is tested with a failing io.Writer, not a real full disk or network mount. No real AWS calls or purchases were made.

Notes

• Possible overlap: a parallel fix for fix(cli): classify errored provider purchase results as failures #1900 (errored purchase results) also touches cmd/multi_service.go. This PR only changes that file at the two CSV-report call sites, so any conflict should be small and easy to resolve.
• Maintainers: please copy the issue's triage labels onto this PR, since the author lacks label permissions: triaged, priority/p2, severity/medium, urgency/this-quarter, impact/few, effort/xs, type/bug.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • CSV reports now confirm success only after the file has been written successfully, and report write or close failures instead of indicating success.
  • When there are no results, the app no longer creates a CSV report or reports that one was written.
  • The output path is shown after a successful report write.

[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: Advanced
• Run ID: bc4c1753-e45e-4880-a553-9b3753befd5d

📥 Commits

Reviewing files that changed from the base of the PR and between 817ab4b and 7daac2b.

📒 Files selected for processing: 3

• cmd/multi_service.go
• cmd/multi_service_csv.go
• cmd/multi_service_csv_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

CSV report generation now returns buffered write and file close errors. Empty results do not create a report. The CLI reports the output path only after a successful write.

Changes: CSV reporting

Layer / File(s)	Summary
CSV write error propagation cmd/multi_service_csv.go, cmd/multi_service_csv_test.go	The renderer flushes explicitly and returns flush errors. The report writer returns file close errors when no earlier error occurred. A test checks that a buffered write error is returned.
Report status and caller handling cmd/multi_service.go, cmd/multi_service_csv.go, cmd/multi_service_csv_test.go	The report wrapper handles empty results and logs the output path only after a successful write. Both callers use the wrapper. Tests check empty results and success and failure log messages.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: cristim

Merge Risk: ⚪ Minimal · up to 7daac

No actionable merge-blocking risk is identified; the CSV report change is mergeable after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: print the CSV success message only after a complete write.
Linked Issues check		Issue #2004 requires returned CSV flush errors and no success message when no report file exists. writeMultiServiceCSV explicitly flushes and returns writer.Error(). writeMultiServiceCSVReport r…
Out of Scope Changes check		The changes are limited to CSV rendering and error handling, the two CSV report call sites, and focused tests. These changes directly support issue #2004. No unrelated implementation or tests are iden…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files.

Full details: Linked Issues check

Explanation

Issue #2004 requires returned CSV flush errors and no success message when no report file exists. writeMultiServiceCSV explicitly flushes and returns writer.Error(). writeMultiServiceCSVReport returns close errors. writeCSVReport handles zero results and prints CSV report written to: only after a successful write and close. Both writeReportAndSummary and runCSVPurchases use writeCSVReport. The added tests cover flush errors, zero results, and success-message behavior.

Full details: Out of Scope Changes check

Explanation

The changes are limited to CSV rendering and error handling, the two CSV report call sites, and focused tests. These changes directly support issue #2004. No unrelated implementation or tests are identified.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

[cristim]

Independent review verdict: CLEAN, head 7daac2b09d7c2ee5cfdae4ad805134f2eb73c165

An independent adversarial reviewer (did not author the change) found no blocking issues.

Verified against the code:

• The flush error is returned.
• A close error becomes the return value, with no double close.
• Both callers (writeReportAndSummary and runCSVPurchases) go through writeCSVReport, and there are no other callers.
• A write failure only warns and does not skip the purchase summary.
• The empty-results message doesn't collide with the existing CSV report written assertions.
• Merges cleanly with main and with fix(cli): classify errored provider purchase results as failures #2204.

Local verification on macOS (fixture/mock evidence: a failing io.Writer and a missing directory, not a real full disk):

• make build, go test ./... -count=1 and go vet ./... all pass.
• Mutation: replacing the writer.Error() check with nil makes TestWriteMultiServiceCSV_FlushErrorIsReturned FAIL.
• Mutation: removing the empty-results guard makes TestWriteReportAndSummary_NoResultsDoesNotClaimCSVWritten FAIL.
• With the fix restored, the tests pass.

Non-blocking:

• The file.Close() error path has no test.
• A truncated file stays on disk, and the warning doesn't name the path.
• The failure warning uses log.Printf rather than AppLogger (this predates the PR).

Labels mirrored from #2004.

Not yet mergeable:

• (1) Fork CI runs 38822081444 and 38822081450 are awaiting maintainer approval.
• (2) The PR is still a draft. @IAmJSD, mark it ready when you're done.
• (3) CodeRabbit hasn't reviewed it because it's a draft.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[cristim]

Merge gate met at 7daac2b09d7c2ee5cfdae4ad805134f2eb73c165

• Independent adversarial review: CLEAN at this SHA (see the verdict above).
• CI: green at this SHA after maintainer approval of the fork runs: CI - Build & Test 38822081444 and pre-commit 38822081450.
• CodeRabbit: the full review finished with no actionable comments.
• Local verification on macOS: recorded in the verdict. It is fixture/mock evidence, with fail-before/pass-after via mutation.
• Labels: mirrored from fix(cli): a short CSV write and a zero-result run both print "CSV report written to" #2004.
• Merge: clean onto current main (a2e7f4fc).

Squash-merging with --match-head-commit 7daac2b0.