Merge pull request #362 from rajbos/unit-testing

Improve unit test setup and coverage

Author: rajbos

.devcontainer/devcontainer.json

@@ -27,8 +27,12 @@
   "initializeCommand": "powershell -ExecutionPolicy Bypass -File .devcontainer/init-mounts.ps1",
   "mounts": [
     "source=${localEnv:APPDATA}/Code/User,target=/home/node/.config/Code/User,type=bind,readonly=true",
-    "source=${localEnv:APPDATA}/Code - Insiders/User,target=/home/node/.config/Code - Insiders/User,type=bind,readonly=true"
+    "source=${localEnv:APPDATA}/Code - Insiders/User,target=/home/node/.config/Code - Insiders/User,type=bind,readonly=true",
+    "source=github-copilot-token-usage-nodemodules,target=/workspaces/github-copilot-token-usage/node_modules,type=volume"
   ],
+  "containerEnv": {
+    "NODE_OPTIONS": "--max-old-space-size=4096"
+  },
   "postCreateCommand": "npm ci",
   "remoteUser": "node"
 }

.github/agents/refactor.agent.md

@@ -0,0 +1,75 @@
+---
+description: "Improve code quality, apply security best practices, and enhance design whilst maintaining green tests."
+name: "Code Refactor - Improve Quality & Security"
+tools: ["execute/runTests", "execute/getTerminalOutput", "execute/runInTerminal", "read/terminalLastCommand", "read/terminalSelection", "search/codebase", "read/problems", "execute/testFailure"]
+---
+
+# Code Refactor - Improve Quality & Security
+
+Clean up code, apply security best practices, and enhance design whilst keeping all tests green.
+
+## Core Principles
+
+### Code Quality Improvements
+
+- **Remove duplication** - Extract common code into reusable functions or classes
+- **Improve readability** - Use intention-revealing names and clear structure
+- **Apply SOLID principles** - Single responsibility, dependency inversion, etc.
+- **Simplify complexity** - Break down large functions, reduce cyclomatic complexity
+
+### Security Hardening
+
+- **Input validation** - Sanitise and validate all external inputs
+- **Authentication/Authorisation** - Implement proper access controls
+- **Data protection** - Encrypt sensitive data, use secure connection strings
+- **Error handling** - Avoid information disclosure through exception details
+- **Dependency scanning** - Check for vulnerable npm packages
+- **Secrets management** - Use environment variables or secure storage, never hard-code credentials
+- **OWASP compliance** - Address common security vulnerabilities
+
+### Design Excellence
+
+- **Design patterns** - Apply appropriate patterns (Factory, Strategy, Observer, etc.)
+- **Dependency injection** - Use DI for loose coupling
+- **Configuration management** - Externalise settings using VS Code configuration API
+- **Logging and monitoring** - Add structured logging for troubleshooting
+- **Performance optimisation** - Use async/await, efficient data structures, memoization
+
+### TypeScript Best Practices
+
+- **Strict type checking** - Enable strict mode in tsconfig.json
+- **Type safety** - Use proper types instead of `any`, leverage union types and discriminated unions
+- **Modern TypeScript features** - Use optional chaining, nullish coalescing, template literal types
+- **Immutability** - Prefer `const` and `readonly`, use immutable data patterns
+- **Error handling** - Use proper error types, avoid swallowing errors
+- **VS Code Extension API** - Follow VS Code extension best practices and API guidelines
+
+## Security Checklist
+
+- [ ] Input validation on all public functions
+- [ ] XSS protection for webview content
+- [ ] Command injection prevention (sanitise shell commands)
+- [ ] Authorisation checks on sensitive operations
+- [ ] Secure configuration (no secrets in code)
+- [ ] Error handling without information disclosure
+- [ ] Dependency vulnerability scanning (npm audit)
+- [ ] OWASP Top 10 considerations addressed
+
+## Execution Guidelines
+
+1. **Ensure green tests** - All tests must pass before refactoring
+2. **Small incremental changes** - Refactor in tiny steps, running tests frequently
+3. **Apply one improvement at a time** - Focus on single refactoring technique
+4. **Run security analysis** - Use static analysis tools (ESLint, SonarQube)
+5. **Document security decisions** - Add comments for security-critical code
+
+## Refactor Phase Checklist
+
+- [ ] Code duplication eliminated
+- [ ] Names clearly express intent
+- [ ] Functions have single responsibility
+- [ ] Security vulnerabilities addressed
+- [ ] Performance considerations applied
+- [ ] All tests remain green
+- [ ] Code coverage maintained or improved
+- [ ] TypeScript strict mode enabled and compliant

.github/copilot-instructions.md

@@ -129,6 +129,40 @@ To check if data is available:
 [ -f ./usage-data/usage-agg-daily.json ] && echo "Aggregated data available"
 ```
 
+## DevContainer Terminal Behavior
+
+This repository uses a devcontainer (`.devcontainer/devcontainer.json`). When working inside the devcontainer, **terminal output capture is unreliable** — commands execute successfully but the `run_in_terminal` tool often returns empty or truncated output. This is a known limitation of the remote filesystem layer.
+
+### What NOT to do
+
+Do not enter retry loops trying to capture terminal output. These patterns waste turns and never converge:
+- Running commands repeatedly hoping output will appear
+- Redirecting output to `/tmp/` files and using `read_file` to read them (the remote FS often fails on newly-written temp files)
+- Spawning background terminals with `sleep && tail` to poll for results
+- Delegating to subagents to "run tests in a clean way"
+
+### What to do instead
+
+1. **Use `npm` scripts for standard operations.** The project defines scripts for common tasks:
+   - `npm run compile` — lint + build
+   - `npm run compile-tests` — compile test files to `out/`
+   - `npm run test:node` — compile + run a single test file
+   - `npm run test:coverage` — compile + run tests with coverage thresholds
+
+2. **Use `get_errors` to validate compilation.** After edits, call `get_errors` on the changed files instead of running `tsc` in the terminal. This is more reliable than parsing terminal output.
+
+3. **Run tests in small batches.** Instead of running all 25+ test files in one command, run one file at a time:
+   ```bash
+   node --require ./out/test/unit/vscode-shim-register.js --test out/test/unit/sessionParser.test.js
+   ```
+   Small commands are more likely to return output before the capture times out.
+
+4. **Accept a single run.** If a test command runs without returning output, do **not** re-run it. Instead, move on and note that the tests were executed but output was not captured. The user can verify locally.
+
+5. **Use `runTests` tool when available.** Prefer the dedicated test-runner tool over terminal commands for running tests, though note it may not support the Node.js built-in test runner (`node --test`).
+
+6. **Write output to the workspace (not `/tmp/`).** If you must capture output to a file, write it inside the workspace (e.g., `test-results.txt` in the repo root) where `read_file` can reliably access it. Add the file to `.gitignore` if it doesn't already match. Clean it up after reading.
+
 ## Webview Navigation Buttons
 
 To maintain a consistent, VS Code-native look across all webview panels (Details, Chart, Usage Analysis, Diagnostics), use the VS Code Webview UI Toolkit for top-level navigation buttons.

.github/workflows/ci.yml

@@ -14,8 +14,9 @@ jobs:
     runs-on: ubuntu-latest
 
     strategy:
+      fail-fast: false
       matrix:
-        node-version: [18.x, 20.x]
+        node-version: [20.x, 22.x, 24.x]
 
     steps:
     - name: Harden the runner (Audit all outbound calls)
@@ -52,6 +53,9 @@ jobs:
 
     - name: Compile tests
       run: npm run compile-tests
+
+    - name: Run Node.js unit tests
+      run: node --require ./out/test/unit/vscode-shim-register.js --test out/test/unit/*.test.js
 
     - name: Run tests
       uses: coactions/setup-xvfb@b6b4fcfb9f5a895edadc3bc76318fae0ac17c8b3 # v1.0.1

.vscode-test.mjs

@@ -1,7 +1,7 @@
 import { defineConfig } from '@vscode/test-cli';
 
 export default defineConfig({
-	files: 'out/test/**/*.test.js',
+	files: 'out/test/integration/**/*.test.js',
 	mocha: {
 		timeout: 60000,
 		parallel: false

.vscodeignore

@@ -10,8 +10,6 @@ out
 out/**
 test
 test/**
-test-node
-test-node/**
 node_modules
 node_modules/**
 .github

docs/specs/backend.md

@@ -514,7 +514,7 @@ All settings in VS Code user settings (global scope, Settings Sync compatible):
 ### Test Organization
 
 ```
-src/test-node/
+test/unit/
 ├── backend-identity.test.ts          # Identity & validation (317 lines)
 ├── backend-settings.test.ts          # Configuration parsing
 ├── backend-rollups.test.ts           # Aggregation logic (117 lines)

package-lock.json

@@ -278,8 +278,8 @@
     "lint:css": "stylelint \"src/webview/**/*.css\"",
     "lint:json": "node scripts/validate-json.js",
     "test": "vscode-test",
-    "test:node": "npm run compile-tests && node --test out/test/test-node/backend-identity.test.js",
-    "test:coverage": "npm run compile-tests && node --require ./out/test/test-node/vscode-shim-register.js --experimental-test-coverage --test --test-coverage-lines=60 --test-coverage-functions=60 --test-coverage-branches=60 --test-coverage-include=out/test/backend/**/*.js --test-coverage-include=out/test/utils/**/*.js out/test/test-node/backend-identity.test.js out/test/test-node/utils-errors.test.js out/test/test-node/backend-settings.test.js out/test/test-node/backend-copyConfig.test.js out/test/test-node/backend-integration.test.js out/test/test-node/backend-commands.test.js out/test/test-node/backend-facade-helpers.test.js out/test/test-node/backend-facade-rollups.test.js out/test/test-node/backend-facade-query.test.js",
+    "test:node": "npm run compile-tests && node --require ./out/test/unit/vscode-shim-register.js --test --test-force-exit out/test/unit/*.test.js",
+    "test:coverage": "npm run compile-tests && node --require ./out/test/unit/vscode-shim-register.js --experimental-test-coverage --test --test-force-exit --test-coverage-lines=80 --test-coverage-functions=80 --test-coverage-branches=60 --test-coverage-include=out/src/backend/**/*.js --test-coverage-include=out/src/utils/**/*.js out/test/unit/*.test.js",
     "pre-release": "node scripts/pre-release.js",
     "capture-screenshots": "pwsh -File scripts/capture-screenshots.ps1",
     "sync-changelog": "node scripts/sync-changelog.js",

package.json

@@ -12,7 +12,7 @@ const MAX_TEAM_ALIAS_LENGTH = 32;
 const COMMON_NAME_PATTERNS = /\b(john|jane|smith|doe|admin|user|dev|test|demo)\b/i;
 
 export function validateTeamAlias(input: string): TeamAliasValidationResult {
-	const alias = (input ?? '').trim();
+	const alias = (input ?? '').trim().toLowerCase();
 	if (!alias) {
 		return { 
 			valid: false,