test(cli): restore toolCfg after validator tests so shuffled runs pass

[cristim]

Closes #2208

Summary

Four tests in cmd/validators_test.go set the package-level toolCfg = Config{} and never restored it:

• TestValidateNumericRanges
• TestValidatePaymentAndTerm
• TestValidateFilePaths
• TestValidateTargetCoverage

The CLI flags bind into &toolCfg.* with defaults set at init. When one of these four ran first, tests that rely on those defaults failed with "coverage-lookback-days must be >= 1": TestValidateFlags*, TestFilterFlagValidation and TestFilterAndAdjustRecommendations_ZeroCoverage. That only happens in a shuffled order. The default declaration order happened to hide it.

Each of the four tests now registers t.Cleanup(saveGlobalVars().restore), reusing the existing helper in cmd/multi_service_test_common_test.go. The change is test-only: 4 added lines.

Evidence

All runs are local on macOS with GOTOOLCHAIN=go1.26.9.

• Before:
  • Seed 1792159022300456000 with -run 'Validate|Flag|Filter' -count=2 failed about 57 tests.
  • Seed 1792159305657431000 on the full package failed the five default-dependent tests.
• After:
  • Both seeds pass.
  • go test ./cmd/ -shuffle=on -count=3 and -count=5 pass.
  • The full package passes with -shuffle=101, 202 and 303.
• Mutation probe: an independent reviewer removed one of the four lines. The validation subset then failed under 6 of 8 seeds.
• Completeness: the reviewer read every toolCfg assignment in the test files. Every other one already restores, and saveGlobalVars snapshots Config by value.
• Checks:
  • make build, go vet ./... and go test -race ./cmd/... pass.
  • golangci-lint v2.10.1 reports 0 issues.
  • scripts/test-git-secrets-allowlist.sh passes 44/44.
• Review: two passes by an independent non-author reviewer (sonnet, high) found no actionable findings at 3d8c8c59. Pass 2 compared this against a TestMain reset or editing every test that relies on defaults, and found restoring at the polluting tests the simplest correct fix.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Validator tests now restore global settings after each subtest, helping prevent test state from affecting later tests.

[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: 6cd6c627-176e-4eca-a3b1-9e2e8c06a78d

📥 Commits

Reviewing files that changed from the base of the PR and between 26c660b and 0db56a1.

📒 Files selected for processing: 1

• cmd/validators_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

Four groups of validator table tests now register cleanup to restore saved global variables after each subtest.

Changes: Validator test state

Layer / File(s)	Summary
Restore globals in validator tests cmd/validators_test.go	Numeric-range, payment-and-term, file-path, and target-coverage subtests register cleanup to restore saved global variables.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 0db56

The validator tests now restore their prior configuration between subtests, addressing the reported shuffle-order issue without changing production behavior. No concrete merge-blocking risk remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

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 test-only change: restoring toolCfg after validator tests to prevent failures during shuffled test runs.
Linked Issues check		Issue #2208 requires cmd tests to isolate or restore shared flag and configuration state. The PR adds t.Cleanup(saveGlobalVars().restore) to TestValidateNumericRanges, `TestValidatePaymentAndTer…
Out of Scope Changes check		The reported change adds four cleanup registrations in cmd/validators_test.go. The change is test-only and directly restores shared state required by issue #2208. The supplied summary identifies no …

Full details: Linked Issues check

Explanation

Issue #2208 requires cmd tests to isolate or restore shared flag and configuration state. The PR adds t.Cleanup(saveGlobalVars().restore) to TestValidateNumericRanges, TestValidatePaymentAndTerm, TestValidateFilePaths, and TestValidateTargetCoverage. These are the four tests identified as leaving toolCfg as Config{}. The supplied PR evidence reports that the affected shuffled runs pass after this change. The repository diff was unavailable, so this assessment relies on the supplied change summary and test evidence.

Full details: Out of Scope Changes check

Explanation

The reported change adds four cleanup registrations in cmd/validators_test.go. The change is test-only and directly restores shared state required by issue #2208. The supplied summary identifies no unrelated production or test changes.

• 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

• 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 at 0db56a1

Reviewer: fresh Claude agent (Sonnet, high effort), not the author. Coordinator: claude-cc-reviewer.

What was checked:

• The three added t.Cleanup(saveGlobalVars().restore) calls (cmd/validators_test.go:143, 241, 433) cover the only unrestored toolCfg mutations. Each snapshot is taken before toolCfg = Config{}. The other validator tests already restore (:568, 622, 661, 722, 765, 790, 834), as do the other cmd/*_test.go files grepped.
• A shallow struct copy is sufficient, because these tests assign Config{} wholesale and never mutate slices or maps in place.
• Fail-before: on origin/main, go test -count=2 -shuffle=on ./cmd/ failed 3 of 3 runs (seeds 1792161156820552000, 1792161228701672000, 1792161310408108000) in TestValidateFlags*, TestFilterFlagValidation and TestValidateFlags_CSVPaths.
• At this head: -count=10 -shuffle=on and -race -count=3 -shuffle=on pass. -race -count=10 hit the 10-minute timeout under heavy machine load with no test failure, and its seed re-run passes.

Gate:

• CI is green, and mergeability is CLEAN.
• Labels are mirrored from test(cli): cmd tests fail under -shuffle=on via shared flag state #2208.
• CodeRabbit reports no actionable comments. (claude-cc-reviewer)