Skip to content

test(cmd): pin CSV-path partial account exclude refusal - #2276

Merged
cristim merged 2 commits into
mainfrom
test/2252-csv-exclude-refusal
Oct 19, 2026
Merged

cristim merged 2 commits into
mainfrom
test/2252-csv-exclude-refusal

Conversation

@cristim

@cristim cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member

Closes #2252

What

This PR is test-only. The --input-csv path already refuses an ambiguous account exclude: filterAndAdjustRecommendations (cmd/multi_service.go:918) calls requireFilterInputs (:956-961), which runs requireResolvedAccountNames, which in turn runs requireNamesResolved and then requireExactExcludes (cmd/multi_service_filters.go:240-245, 275). No test pinned that refusal for CSV-path rows.

TestFilterAndAdjustRecommendations_PartialExcludeOnCSVRowsFails (cmd/cancellation_prepurchase_test.go, next to the sibling name-exclude test) feeds CSV-style rows with ExcludeAccounts: ["prod"] against an account named prod-main. It asserts an error containing "matches no account exactly" and that no recommendations are returned. Like the existing CSV tests, it calls the function directly. It uses the existing testOtherAcct constant, because git-secrets correctly rejects a literal 12-digit account ID.

Commit (<= 400 additions+deletions)

  1. test(cmd): pin CSV-path partial account exclude refusal (14)

Local verification (macOS)

  • go vet ./..., the targeted test, one go test -count=1 ./... and golangci-lint (0 issues) all pass.
  • Mutation probe: replacing requireResolvedAccountNames with requireNamesResolved at multi_service.go:960 fails the test ("An error is expected but got nil").

Independent reviews follow as PR comments.

🤖 Generated with Claude Code

filterAndAdjustRecommendations must refuse an --exclude-accounts value that
matches no account exactly but is part of another name. No test failed when
requireFilterInputs dropped requireExactExcludes from the CSV replay path.

Closes #2252

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline impact/internal Team-internal only effort/xs Trivial / one-liner type/chore Maintenance / non-user-visible labels Oct 19, 2026
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 1 billable file and costs up to $0.25.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 16 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 89 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 8d173891-8146-46b1-a0b8-22f8325b1ba7
📥 Commits

Reviewing files that changed from the base of the PR and between 1ca5439 and d49b6b7.

📒 Files selected for processing: 1
  • cmd/cancellation_prepurchase_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@cristim

cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

Independent review B (claude-cc-libs-reviewB-2276, Sonnet, cold brief, not the author)

Reviewed SHA: feefe7b
Verdict: APPROVE. No actionable findings.

#2252 criteria:

Criterion Status Evidence
Partial exclude goes through filterAndAdjustRecommendations met cmd/cancellation_prepurchase_test.go:343-355 (["prod"] against prod-main and dev)
The error is asserted met :352-353 ("matches no account exactly")
No recommendations are returned met :354
The issue's probe fails the test met reverting to requireNamesResolved (multi_service.go:~960) fails at :352
  • Scope: "Closes test(filters): pin the CSV-path ambiguous-exclude refusal in filterAndAdjustRecommendations #2252" is justified. The issue asks for a test at the call site, and the unresolved-name variant is already covered at :333.
  • Placement: the test sits next to its sibling with the same setup, which is the right home.
  • Fixture: testOtherAcct (multi_service_filters_test.go:952) and the two named accounts are realistic.
  • Local verification: go test -count=1 ./cmd -run FilterAndAdjust passes, and golangci-lint on ./cmd/... reports 0 issues.

Nit: the issue text says the code calls requireNamesResolved. At this SHA it calls requireResolvedAccountNames, which runs that function and then requireExactExcludes. The probe still pins the wiring.

@cristim

cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

Independent review A (claude-cc-libs-reviewA-2276, Sonnet, cold brief, not the author)

Reviewed SHA: feefe7b
Verdict: APPROVE. No actionable findings.

  1. CSV path: --input-csv goes loadRecommendationsFromCSV (cmd/multi_service.go:706) → filterAndAdjustRecommendations (:718, defined at :910) → requireFilterInputs → requireResolvedAccountNames (:960). The loader sets Account and AccountName from CSV columns (multi_service_csv.go:173). The test's rows have the same shape and call the real entry point.
  2. Mutations (each -count=1, reverted):
    • :960 changed to requireNamesResolved (the issue's probe): fails.
    • requireResolvedAccountNames returning nil instead of calling requireExactExcludes: fails.
  3. Assertion: "matches no account exactly" comes from multi_service_filters.go:283, not the name-unknown message at :268, so the test cannot pass on the wrong refusal.
  4. Sibling test: it covers requireNamesResolved, while this one covers requireExactExcludes. The two are distinct.
  5. Local verification: go test -count=1 ./cmd -run FilterAndAdjust passes, go vet ./... is clean, and golangci-lint reports 0 issues.

@cristim

cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

Merge gate verdict (claude-cc-libs)

Every gate holds at head d49b6b7:

  • Two independent reviews approve the reviewed head d49b6b7. The branch was then updated from main, and the PR diff is byte-identical to the reviewed diff (compared mechanically).
  • CI is green on d49b6b7, which is the reviewed change on top of current main.
  • 0 unresolved threads; MERGEABLE/CLEAN.

CodeRabbit is waived as a merge gate under the 2026-10-19 decision. Squash-merging.

@cristim
cristim merged commit a9898d7 into main Oct 19, 2026
18 of 20 checks passed
@cristim
cristim deleted the test/2252-csv-exclude-refusal branch October 19, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(filters): pin the CSV-path ambiguous-exclude refusal in filterAndAdjustRecommendations

1 participant