test(cli): isolate the exec image tests from the models.dev refresh

[Vasanthdev2004]

What changed

The three tests in internal/cli/exec_streamjson_images_test.go now start with isolateCLIUserState(t). In TestRunExecImageAttachmentHistory it replaces the hand-written loop that pointed HOME, APPDATA and the XDG variables at a temporary directory. The other two tests had no isolation at all.

Why

runExec calls modelregistry.StartModelsDevRefresh(), which downloads the models.dev catalog in the background unless ZERO_DISABLE_MODELS_FETCH is set. The hand-written loop moved the cache under the test's temporary home but didn't turn the fetch off. So the test downloaded about 5.7 MB from the network, and the write raced the t.TempDir cleanup. On the macOS runner the cleanup lost:

--- FAIL: TestRunExecImageAttachmentHistory/stream_and_file_images (0.62s)
    testing.go:1464: TempDir RemoveAll cleanup: unlinkat /var/folders/.../001: directory not empty

That was on #886, whose only change since its last green run was a comment, so the flake comes from the test on main (#1152). The two older tests in the file had the same background fetch, and with no isolation it wrote to the real user cache.

isolateCLIUserState is the shared helper for this. It redirects the same user directories, points ZERO_MODELS_CACHE_PATH into the fixture, sets ZERO_DISABLE_MODELS_FETCH and keeps credentials off the OS keyring.

Verification

• A temporary probe at the end of TestRunExecImageAttachmentHistory (not committed) showed the mechanism. On main, modelsdev.json appeared in the test's home 3 seconds after the run, 5,756,570 bytes, for all three cases. With this change, no cache file is written.
• The three tests pass three times in a row, and internal/cli passes on Windows.
• go vet ./internal/cli/ for windows, linux and darwin, gofmt, git diff HEAD --check, and go run ./cmd/zero-release build and smoke are clean.

Summary by CodeRabbit

• Tests
  • Improved consistency of automated checks for image attachments in stream-JSON CLI workflows. This change affects test setup only; it does not change CLI behavior or introduce any user-facing features.

[Vasanthdev2004]

@lhk0504 for your awareness: this is the test from #1152. Nothing wrong with the change itself. runExec starts the models.dev refresh in the background, and the hand-written environment loop moved its cache into the TempDir without turning it off, so the cleanup sometimes raced the download. isolateCLIUserState is the helper to reach for in internal/cli tests.

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: f2e7c146376d
Changed files (1): internal/cli/exec_streamjson_images_test.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: Twigpine/zero/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 88a799dd-6a10-4d6a-80a5-90176b053a00

📥 Commits

Reviewing files that changed from the base of the PR and between ef4da13 and f2e7c14.

📒 Files selected for processing: 1

• internal/cli/exec_streamjson_images_test.go

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

Walkthrough

The image-related CLI tests now call isolateCLIUserState(t). The history test replaces manual user-state environment setup with the helper.

Changes

CLI image test state isolation

Layer / File(s)	Summary
Isolate test user state internal/cli/exec_streamjson_images_test.go	The history test replaces manual user-state environment setup with isolateCLIUserState(t). Both stream-JSON image tests call the helper before running.

Priority: ⬇️ Low

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

Change: Other

Suggested reviewers: lhk0504, pierrunoyt

Merge Risk: ⚪ Minimal · up to f2e7c

The image tests now isolate their user state and avoid the background catalog refresh; no merge-blocking risk was found.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 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 identifies the main change: isolating CLI image tests from the models.dev refresh. It is concise and specific.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• 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.}

[lhk0504]

@lhk0504 for your awareness: this is the test from #1152. Nothing wrong with the change itself. runExec starts the models.dev refresh in the background, and the hand-written environment loop moved its cache into the TempDir without turning it off, so the cleanup sometimes raced the download. isolateCLIUserState is the helper to reach for in internal/cli tests.

Thanks for tracking this down and fixing it! I missed the background models.dev refresh when setting up the test isolation. I’ll use isolateCLIUserState(t) for future CLI tests.

[kevincodex1]

lgtm