joshsmithxrm
diff --git a/‎.claude/commands/test.md‎
Lines changed: 49 additions & 1 deletion b/‎.claude/commands/test.md‎
Lines changed: 49 additions & 1 deletion
diff --git a/‎.claude/commands/tui-review.md‎
Lines changed: 97 additions & 0 deletions b/‎.claude/commands/tui-review.md‎
Lines changed: 97 additions & 0 deletions
diff --git a/‎.github/workflows/tui-e2e.yml‎
Lines changed: 76 additions & 0 deletions b/‎.github/workflows/tui-e2e.yml‎
Lines changed: 76 additions & 0 deletions
diff --git a/‎CLAUDE.md‎
Lines changed: 2 additions & 0 deletions b/‎CLAUDE.md‎
Lines changed: 2 additions & 0 deletions
diff --git a/‎docs/adr/0028_TUI_TESTING_STRATEGY.md‎
Lines changed: 40 additions & 0 deletions b/‎docs/adr/0028_TUI_TESTING_STRATEGY.md‎
Lines changed: 40 additions & 0 deletions
diff --git a/‎src/PPDS.Auth/Credentials/MsalClientBuilder.cs‎
Lines changed: 14 additions & 3 deletions b/‎src/PPDS.Auth/Credentials/MsalClientBuilder.cs‎
Lines changed: 14 additions & 3 deletions
diff --git a/‎src/PPDS.Auth/Credentials/TokenCacheManager.cs‎
Lines changed: 8 additions & 3 deletions b/‎src/PPDS.Auth/Credentials/TokenCacheManager.cs‎
Lines changed: 8 additions & 3 deletions
@@ -34,17 +34,64 @@ Fast, no external dependencies. Default choice.
 
 ### TUI Tests
 
+TUI testing has three layers:
+1. **TuiUnit** - State assertions (fast, no terminal needed)
+2. **tui-e2e** - Visual snapshots using @microsoft/tui-test (needs Node.js)
+3. **TuiIntegration** - Integration with FakeXrmEasy (future)
+
 ```bash
 # Build first
 dotnet build src/PPDS.Cli/PPDS.Cli.csproj --no-restore
 
-# TUI unit tests (fast)
+# TUI unit tests - state assertions (fast)
 dotnet test tests/PPDS.Cli.Tests/PPDS.Cli.Tests.csproj --filter "Category=TuiUnit" --no-build
 
+# TUI visual snapshots (if Node.js available)
+# Use --prefix to run npm from the repo root
+if (Get-Command node -ErrorAction SilentlyContinue) {
+    npm test --prefix tests/tui-e2e
+}
+
 # TUI integration tests (if unit pass)
 dotnet test tests/PPDS.Cli.Tests/PPDS.Cli.Tests.csproj --filter "Category=TuiIntegration" --no-build
 ```
 
+**Updating snapshots:** If visual changes are intentional, update snapshots:
+```bash
+npm test --prefix tests/tui-e2e -- --update-snapshots
+```
+
+#### TUI Testing Philosophy
+
+TUI tests verify **presentation**, not business logic. CLI and TUI share Application Services (ADR-0015), so service logic is tested once at the CLI layer.
+
+**What TUI E2E tests verify:**
+- Does the screen render correctly? (snapshots)
+- Do keyboard shortcuts work? (key sequences)
+- Does navigation flow correctly? (screen transitions)
+- Do errors display properly? (error dialogs)
+
+**What TUI E2E tests do NOT verify:**
+- Query execution correctness (CLI tests `ISqlQueryService`)
+- Export format validity (CLI tests `IExportService`)
+- Authentication flows (CLI tests auth)
+
+**Trust the service layer:** If CLI tests pass, services work. Don't duplicate that coverage.
+
+#### Interpreting Snapshot Diffs
+
+When E2E tests fail with snapshot diffs:
+
+1. **Read the ASCII diff** - Shows expected vs actual terminal output
+2. **Identify what's wrong** - Layout shifted? Text missing? Wrong content?
+3. **Fix the TUI code** - The rendering issue is in the view/screen code
+4. **Re-run tests** - Verify fix with `npm test --prefix tests/tui-e2e`
+5. **If change was intentional** - Update snapshots with `--update-snapshots`
+
+Snapshots are the visual spec. They define "correct" appearance. When you see a diff:
+- If the "actual" looks wrong → fix the code
+- If the "actual" looks right (intentional change) → update the snapshot
+
 ### Integration Tests
 
 Requires `.env.local` with Dataverse credentials.
@@ -177,3 +224,4 @@ ppds plugins --help
 
 - ADR-0028: TUI Testing Strategy
 - ADR-0029: Testing Strategy
+- `tests/tui-e2e/` - TUI visual snapshot tests using @microsoft/tui-test
@@ -0,0 +1,97 @@
+# TUI Review
+
+Evaluate TUI screens for UX quality and identify improvements.
+
+## Usage
+
+`/tui-review` - Review the main TUI experience
+`/tui-review sql` - Review SQL Query screen specifically
+`/tui-review profile` - Review profile management dialogs
+`/tui-review [screen]` - Review a specific screen
+
+## Setup
+
+1. Build the CLI:
+   ```bash
+   dotnet build src/PPDS.Cli/PPDS.Cli.csproj -f net10.0
+   ```
+
+2. Run automated tests first to ensure baseline works:
+   ```bash
+   dotnet test --filter "Category=TuiUnit"
+   ```
+
+## Review Process
+
+### Step 1: Launch TUI
+```bash
+.\src\PPDS.Cli\bin\Debug\net10.0\ppds.exe interactive
+```
+
+### Step 2: Explore the Target Screen
+
+**Main Menu:** Navigation, menu items, status bar, keyboard shortcuts
+**SQL Query (F2):** Query input, execution, results table, pagination, filter, export
+**Profile Selector (Alt+P):** Profile list, create/delete, switching
+**Environment Selector (Alt+E):** Environment discovery, selection, details
+**Error Log (F12):** Error display, details, copy functionality
+
+### Step 3: Test These Interactions
+- Keyboard navigation (Tab, arrows, Enter, Escape)
+- Hotkeys (F1, F2, Alt+P, Alt+E, Ctrl+Q)
+- Mouse clicks (if applicable)
+- Error states (what happens when things fail?)
+- Empty states (no data, no profiles, etc.)
+- Loading states (spinners, status messages)
+
+### Step 4: Capture State (Optional)
+Use the state capture API to inspect internal state:
+```csharp
+// Components implement ITuiStateCapture<T>
+var state = screen.CaptureState();
+// Inspect: QueryText, StatusText, ResultCount, etc.
+```
+
+## Report Format
+
+Structure your findings as:
+
+### What Works Well
+- List polished/intuitive aspects
+
+### UX Issues
+| Issue | Severity | Description |
+|-------|----------|-------------|
+| ... | P1/P2/P3 | ... |
+
+**Severity:**
+- P1: Blocks core functionality
+- P2: Annoying but workable
+- P3: Nice to have improvement
+
+### Missing Features
+What would users expect that isn't there?
+
+### Suggested Improvements
+Specific, actionable recommendations with rationale.
+
+### Comparison to Similar Tools
+How does it compare to lazygit, k9s, or other TUI tools?
+
+## Important
+
+- **DO NOT make code changes** - This is evaluation only
+- **DO create issues** for P1/P2 findings using `/create-issue`
+- Reference `src/PPDS.Cli/Tui/` for screen locations
+- Reference `docs/adr/0028_TUI_TESTING_STRATEGY.md` for testing patterns
+
+## Screen Locations
+
+| Screen | File |
+|--------|------|
+| Main Window | `src/PPDS.Cli/Tui/MainWindow.cs` |
+| SQL Query | `src/PPDS.Cli/Tui/Screens/SqlQueryScreen.cs` |
+| Profile Selector | `src/PPDS.Cli/Tui/Dialogs/ProfileSelectorDialog.cs` |
+| Environment Selector | `src/PPDS.Cli/Tui/Dialogs/EnvironmentSelectorDialog.cs` |
+| Status Bar | `src/PPDS.Cli/Tui/Views/TuiStatusBar.cs` |
+| Error Details | `src/PPDS.Cli/Tui/Dialogs/ErrorDetailsDialog.cs` |
@@ -0,0 +1,76 @@
+name: TUI E2E Tests
+
+on:
+  push:
+    branches: [main]
+    paths:
+      - 'src/PPDS.Cli/Tui/**'
+      - 'tests/tui-e2e/**'
+  pull_request:
+    branches: [main]
+    paths:
+      - 'src/PPDS.Cli/Tui/**'
+      - 'tests/tui-e2e/**'
+  workflow_dispatch:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  tui-e2e:
+    runs-on: windows-latest
+    timeout-minutes: 30
+
+    steps:
+    - uses: actions/checkout@v6
+      with:
+        fetch-depth: 0
+
+    - name: Setup .NET
+      uses: actions/setup-dotnet@v4
+      with:
+        dotnet-version: |
+          8.0.x
+          9.0.x
+          10.0.x
+
+    - name: Setup Node.js
+      uses: actions/setup-node@v4
+      with:
+        node-version: '20'
+        cache: 'npm'
+        cache-dependency-path: tests/tui-e2e/package-lock.json
+
+    - name: Build PPDS CLI
+      run: dotnet build src/PPDS.Cli/PPDS.Cli.csproj -c Debug -f net10.0
+
+    - name: Install tui-e2e dependencies
+      working-directory: tests/tui-e2e
+      run: npm ci
+
+    - name: Install Playwright browsers
+      working-directory: tests/tui-e2e
+      run: npx playwright install --with-deps chromium
+
+    - name: Run TUI E2E tests
+      working-directory: tests/tui-e2e
+      run: npm test
+
+    - name: Upload test results
+      uses: actions/upload-artifact@v4
+      if: always()
+      with:
+        name: tui-e2e-results
+        path: |
+          tests/tui-e2e/playwright-report/
+          tests/tui-e2e/test-results/
+        retention-days: 7
+
+    - name: Upload snapshots on failure
+      uses: actions/upload-artifact@v4
+      if: failure()
+      with:
+        name: tui-e2e-snapshot-diff
+        path: tests/tui-e2e/__snapshots__/
+        retention-days: 7
@@ -29,11 +29,13 @@ SDK, CLI, TUI, VS Code Extension, and MCP server for Power Platform development.
 | Throw raw exceptions from Application Services | Wrap in `PpdsException` with ErrorCode/UserMessage (ADR-0026) |
 | Use comma-separated issues in `Closes` | GitHub only auto-closes first; use separate `Closes #N` lines |
 | Add TUI service code without tests | Use MockServiceProviderFactory for testability (ADR-0028) |
+| Re-test service logic in TUI E2E tests | CLI tests verify services; TUI tests verify presentation/navigation (ADR-0028) |
 | Use bash-specific syntax in C# process commands | `2>/dev/null`, `||`, pipes don't work on Windows; handle errors in code |
 | File issues in wrong repo | Issues belong in target repo (ppds-docs issues in ppds-docs, not ppds) |
 | Start implementation without plan citations | Cite `docs/patterns/` or ADRs in plan's "Patterns I'll Follow" section |
 | Omit "What I'm NOT Doing" from plans | Explicit boundaries prevent scope creep; required for approval |
 | Implement in design sessions | Design sessions produce plans and issues; workers implement |
+| Use non-black text on blue/cyan TUI backgrounds | Black is the only readable color; see `TuiColorPalette.ValidateBlueBackgroundRule()` |
 
 ## ALWAYS
 
 
@@ -59,6 +59,46 @@ public InteractiveSession(
 - `FakeExportService` - Tracks export operations
 - `TempProfileStore` - Isolated ProfileStore with temp directory
 
+### Test Responsibility Matrix
+
+CLI and TUI share Application Services (ADR-0015). This means service logic is tested once at the CLI/service layer, not duplicated in TUI tests.
+
+| Concern | Tested By | Why |
+|---------|-----------|-----|
+| Query execution logic | CLI/Service tests | Shared `ISqlQueryService` |
+| Data transformation | CLI/Service tests | Service layer responsibility |
+| Export format validity | CLI/Service tests | Shared `IExportService` |
+| Authentication flows | CLI/Service tests | Shared auth infrastructure |
+| Screen rendering | TUI E2E snapshots | Visual verification |
+| Keyboard navigation | TUI E2E | TUI-specific behavior |
+| Session state management | TuiUnit | TUI-specific state (profile switching, caching) |
+| Error dialog display | TUI E2E | Presentation of errors |
+| Threading/UI loop | TUI E2E | TUI-specific concurrency |
+
+**Principle:** If CLI tests pass, the service works. TUI tests verify the presentation layer only.
+
+### TUI Development Workflow
+
+When implementing TUI features:
+
+1. **Implement** the feature in code
+2. **Run TuiUnit** tests for logic: `dotnet test --filter Category=TuiUnit`
+3. **Run E2E** tests: `npm test --prefix tests/tui-e2e`
+4. **Read snapshot diffs** - they show exactly what's rendering
+5. **If diff shows bug** → fix code → re-run
+6. **If diff shows intentional change** → update snapshots: `npm test --prefix tests/tui-e2e -- --update-snapshots`
+7. **Repeat** until tests pass
+
+### What TUI Tests Should NOT Verify
+
+Do not write TUI E2E tests for:
+- Query results are correct (CLI tests `ISqlQueryService`)
+- Export format is valid (CLI tests `IExportService`)
+- Authentication works (CLI tests auth flows)
+- Bulk operations succeed (CLI tests `BulkOperationExecutor`)
+
+These are already covered by CLI/service tests. Duplicating them in TUI E2E wastes effort and creates maintenance burden.
+
 ## Consequences
 
 ### Positive
 
@@ -74,11 +74,12 @@ public static IPublicClientApplication CreateClient(
         IPublicClientApplication client,
         bool warnOnFailure = true)
     {
+        var cacheFilePath = System.IO.Path.Combine(ProfilePaths.DataDirectory, ProfilePaths.TokenCacheFileName);
+
         try
         {
             ProfilePaths.EnsureDirectoryExists();
 
-            var cacheFilePath = System.IO.Path.Combine(ProfilePaths.DataDirectory, ProfilePaths.TokenCacheFileName);
             var cacheFileExists = System.IO.File.Exists(cacheFilePath);
             var cacheFileSize = cacheFileExists ? new System.IO.FileInfo(cacheFilePath).Length : 0;
 
@@ -91,14 +92,24 @@ public static IPublicClientApplication CreateClient(
                 .Build();
 
             var cacheHelper = await MsalCacheHelper.CreateAsync(storageProperties).ConfigureAwait(false);
+
+            // Verify persistence works before registering - performs write/read/clear test
+            // Throws MsalCachePersistenceException if persistence is unavailable
+            cacheHelper.VerifyPersistence();
+            AuthDebugLog.WriteLine("[MsalCache] Persistence verification PASSED");
+
             cacheHelper.RegisterCache(client.UserTokenCache);
 
-            AuthDebugLog.WriteLine("[MsalCache] Cache registered successfully");
+            cacheFileExists = System.IO.File.Exists(cacheFilePath);
+            AuthDebugLog.WriteLine($"[MsalCache] Cache registered successfully. File exists: {cacheFileExists}");
+
             return cacheHelper;
         }
         catch (MsalCachePersistenceException ex)
         {
-            AuthDebugLog.WriteLine($"[MsalCache] Cache registration FAILED: {ex.Message}");
+            AuthDebugLog.WriteLine($"[MsalCache] Persistence verification FAILED: {ex.Message}");
+            AuthDebugLog.WriteLine($"[MsalCache] Cache file path: {cacheFilePath}");
+
             if (warnOnFailure)
             {
                 AuthenticationOutput.WriteLine(
 
@@ -12,15 +12,20 @@ public static class TokenCacheManager
     /// <summary>
     /// Clears the MSAL file-based token cache.
     /// </summary>
+    /// <param name="tokenCachePath">
+    /// Optional path to the token cache file. If null, uses the default global path.
+    /// Pass a custom path when using isolated test directories to avoid clearing production cache.
+    /// </param>
     /// <remarks>
     /// This method deletes the unprotected file-based token cache used by the CLI.
     /// The cache file location matches <see cref="MsalClientBuilder.CreateAndRegisterCacheAsync"/>.
     /// </remarks>
-    public static Task ClearAllCachesAsync()
+    public static Task ClearAllCachesAsync(string? tokenCachePath = null)
     {
-        if (File.Exists(ProfilePaths.TokenCacheFile))
+        var path = tokenCachePath ?? ProfilePaths.TokenCacheFile;
+        if (File.Exists(path))
         {
-            File.Delete(ProfilePaths.TokenCacheFile);
+            File.Delete(path);
         }
 
         return Task.CompletedTask;