test: address review feedback on mock removal changes

- Remove redundant CACHE_DIR alias, use tmpDir directly
- Add force:true to afterAll rmSync for robustness
- Wrap chmod test in try/finally to prevent leaked read-only state
- Remove vacuous conditional assertion in resolve-ui-dist-dir
- Switch get-trace to beforeEach/afterEach for test isolation consistency

Author: Hweinstock

src/cli/__tests__/update-notifier.test.ts

@@ -31,14 +31,13 @@ vi.mock('../commands/update/action.js', async importOriginal => {
   return { ...actual, fetchLatestVersion: mockFetchLatestVersion };
 });
 
-const CACHE_DIR = tmpDir;
 const CACHE_FILE = join(tmpDir, 'update-check.json');
 
 describe('checkForUpdate', () => {
   beforeEach(() => {
     vi.spyOn(Date, 'now').mockReturnValue(1708646400000);
     try {
-      rmSync(CACHE_DIR, { recursive: true });
+      rmSync(tmpDir, { recursive: true, force: true });
     } catch {}
   });
 
@@ -48,7 +47,7 @@ describe('checkForUpdate', () => {
   });
 
   afterAll(() => {
-    rmSync(tmpDir, { recursive: true });
+    rmSync(tmpDir, { recursive: true, force: true });
   });
 
   it('fetches from registry when no cache exists', async () => {
@@ -61,7 +60,7 @@ describe('checkForUpdate', () => {
   });
 
   it('uses cache when last check was less than 24 hours ago', async () => {
-    await mkdir(CACHE_DIR, { recursive: true });
+    await mkdir(tmpDir, { recursive: true });
     await writeFile(CACHE_FILE, JSON.stringify({ lastCheck: 1708646400000 - 1000, latestVersion: '2.0.0' }), 'utf-8');
 
     const result = await checkForUpdate();
@@ -71,7 +70,7 @@ describe('checkForUpdate', () => {
   });
 
   it('fetches from registry when cache is expired', async () => {
-    await mkdir(CACHE_DIR, { recursive: true });
+    await mkdir(tmpDir, { recursive: true });
     await writeFile(
       CACHE_FILE,
       JSON.stringify({ lastCheck: 1708646400000 - 25 * 60 * 60 * 1000, latestVersion: '1.5.0' }),
@@ -119,7 +118,7 @@ describe('checkForUpdate', () => {
   });
 
   it('returns null on cache parse error and fetch error', async () => {
-    await mkdir(CACHE_DIR, { recursive: true });
+    await mkdir(tmpDir, { recursive: true });
     await writeFile(CACHE_FILE, 'invalid json', 'utf-8');
     mockFetchLatestVersion.mockRejectedValue(new Error('network error'));
 
@@ -129,18 +128,20 @@ describe('checkForUpdate', () => {
   });
 
   it('succeeds even when cache write fails', async () => {
-    await mkdir(CACHE_DIR, { recursive: true });
+    await mkdir(tmpDir, { recursive: true });
     await writeFile(CACHE_FILE, '', 'utf-8');
     const { chmod } = await import('fs/promises');
-    await chmod(CACHE_DIR, 0o444);
+    await chmod(tmpDir, 0o444);
 
-    mockFetchLatestVersion.mockResolvedValue('2.0.0');
-
-    const result = await checkForUpdate();
+    try {
+      mockFetchLatestVersion.mockResolvedValue('2.0.0');
 
-    expect(result).toEqual({ updateAvailable: true, latestVersion: '2.0.0' });
+      const result = await checkForUpdate();
 
-    await chmod(CACHE_DIR, 0o755);
+      expect(result).toEqual({ updateAvailable: true, latestVersion: '2.0.0' });
+    } finally {
+      await chmod(tmpDir, 0o755);
+    }
   });
 });
 

src/cli/operations/dev/web-ui/__tests__/resolve-ui-dist-dir.test.ts

@@ -39,12 +39,4 @@ describe('resolveUIDistDir', () => {
 
     expect(resolveUIDistDir()).toBe(tmpDir);
   });
-
-  it('returns a bundled candidate when no env var is set and bundled path exists', () => {
-    const result = resolveUIDistDir();
-    // If a bundled candidate has index.html, it should be returned
-    if (result) {
-      expect(result).toContain('dist-assets');
-    }
-  });
 });

src/cli/operations/traces/__tests__/get-trace.test.ts

@@ -3,7 +3,7 @@ import type { FetchTraceRecordsOptions } from '../types';
 import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
-import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 const { mockSend } = vi.hoisted(() => ({
   mockSend: vi.fn(),
@@ -181,16 +181,15 @@ describe('fetchTraceRecords', () => {
 describe('getTrace', () => {
   let tmpDir: string;
 
-  beforeAll(() => {
+  beforeEach(() => {
     tmpDir = mkdtempSync(join(tmpdir(), 'get-trace-test-'));
   });
 
-  afterAll(() => {
+  afterEach(() => {
+    vi.clearAllMocks();
     rmSync(tmpDir, { recursive: true, force: true });
   });
 
-  afterEach(() => vi.clearAllMocks());
-
   it('calls fetchTraceRecords and writes result to disk', async () => {
     mockSend.mockResolvedValueOnce({ queryId: 'q-1' }).mockResolvedValueOnce({
       status: 'Complete',