Skip to content

fix(auth): migrate lint API to authenticated axios and harden E2E tests #238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
frankbria merged 5 commits into from
Jan 10, 2026
Merged

fix(auth): migrate lint API to authenticated axios and harden E2E tests #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
491fd98
fix(auth): migrate lint API to authenticated axios and harden E2E tests
Jan 9, 2026
df6a7b2
fix(e2e): fix task visibility tests by adding TaskTreeView testids
Jan 9, 2026
64f689a
chore: gitignore E2E test script with API key
Jan 9, 2026
a34e77f
fix(e2e): add Project 6 for task breakdown tests and fix stale state …
Jan 10, 2026
d5ea116
fix(docs): update stale BetterAuth references and fix operator preced…
Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ coverage.xml
htmlcov/
.hypothesis/
artifacts/
tests/e2e/run-e2e-full.sh

# CodeFRAME specific
.codeframe/state.db
Expand Down
257 changes: 62 additions & 195 deletions tests/e2e/E2E_TEST_AUDIT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,229 +1,96 @@
# E2E Test Audit Report

**Date**: 2026-01-07
**Date**: 2026-01-09 (Updated)
**Auditor**: Claude Code
**Status**: CRITICAL ISSUES FOUND AND FIXED
**Status**: CRITICAL ISSUES RESOLVED

## Fixes Applied in This PR
## Summary

### Commit 1: Initial fixes
1. **WebSocket test now REQUIRES messages** - `test_dashboard.spec.ts:444-456`
2. **Error filters tightened** - All test files now only filter `net::ERR_ABORTED`
3. **API response validation added** - `test_project_creation.spec.ts`, `test_task_breakdown.spec.ts`
All critical issues identified in the original audit have been addressed. The E2E test suite now uses real JWT authentication, strict error filtering, and proper API response validation.

### Commit 2: Comprehensive pattern replacement
4. **Replaced `.catch(() => false)` patterns** across all test files:
- `test_task_breakdown.spec.ts`: All 6 conditional skips now verify alternate state before skipping
- `test_metrics_ui.spec.ts`: API responses now validated, no silent failures
- `test_start_agent_flow.spec.ts`: Replaced 3 instances with `.count()` + `.isVisible()` pattern
- `test_complete_user_journey.spec.ts`: Replaced 5 instances with proper assertions
5. **Replaced `.catch(() => {})` patterns** - Metrics tests now fail on API errors
6. **Added state verification before skips** - Tests now assert they're in a known alternate state
## Fixes Applied

## Remaining Work (Future PRs)
### Authentication (RESOLVED)

- Add API response validation to more user actions (lower priority)
- Consider test data fixtures for consistent project states
1. **Auth bypass removed** - `auth-bypass.ts` deleted
2. **Real JWT authentication** - All tests use `loginUser()` from `test-utils.ts`
3. **Lint API fixed** - Migrated from standalone axios to `authFetch` with JWT headers
4. **Response interceptor added** - `api.ts` now logs 401 errors with debugging context
5. **TaskReview error handling improved** - Extracts specific error messages, handles auth failures

## Executive Summary
### Error Filtering (RESOLVED)

The E2E test suite has **critical design flaws** that allow tests to pass even when functionality is broken. The tests are designed to avoid failures rather than catch them.
1. **Strict filtering applied** - All test files now only filter:
- `net::ERR_ABORTED` - Normal navigation cancellation
- `Failed to fetch RSC payload` - Next.js transient during navigation
2. **WebSocket errors NOT filtered** - Connection and message failures will cause test failures
3. **API errors NOT filtered** - 401, 500, network failures will cause test failures

## Critical Issues
### WebSocket Test (RESOLVED)

### 1. Error Filtering Masks Real Failures (CRITICAL)
1. **Now REQUIRES messages** - `test_dashboard.spec.ts:445-455` throws error if 0 messages
2. **Auth error detection** - Detects and reports close code 1008 (auth error)
3. **Abnormal close detection** - Detects and reports close code 1006

**Location**: Every test file's `afterEach` block
### Conditional Skips (RESOLVED)

**Pattern**:
All conditional skips now verify alternate state before skipping:
```typescript
checkTestErrors(page, 'Test name', [
'WebSocket', 'ws://', 'wss://', // ALL WebSocket errors ignored
'net::ERR_FAILED', // Network failures ignored
'net::ERR_ABORTED', // Cancelled requests ignored
'discovery', // Discovery API errors ignored
'Failed to fetch', // Fetch errors ignored
]);
// Pattern used in tests:
const hasKnownState = (await alternateElement.count() > 0);
expect(hasKnownState).toBe(true); // MUST be in SOME known state
test.skip(true, 'Reason (verified in alternate state)');
```

**Impact**: If WebSocket server is down, APIs are returning 500s, or network is broken - tests still pass.
This ensures tests catch broken pages (where neither expected nor alternate state exists).

**Files Affected**:
- `test_task_execution_flow.spec.ts:43-50`
- `test_project_creation.spec.ts:39-46, 181-188`
- `test_auth_flow.spec.ts:51-59`
- `test_start_agent_flow.spec.ts:49-56`
- `test_complete_user_journey.spec.ts:47-55`
- `test_task_breakdown.spec.ts:46-51`
- `test_dashboard.spec.ts:143-148`
### API Response Validation (RESOLVED)

### 2. Conditional Skip Pattern (CRITICAL)
1. **Task approval test added** - `test_task_breakdown.spec.ts` validates 401 errors specifically
2. **Metrics tests validate responses** - `test_metrics_ui.spec.ts:28-56`
3. **Project creation validates responses** - `test_project_creation.spec.ts`

**Pattern**:
```typescript
const isVisible = await element.isVisible().catch(() => false);
if (!isVisible) {
test.skip(true, 'Element not visible - skipping');
return;
}
```

**Impact**: If a feature is completely broken, tests skip instead of fail. This gives false confidence that "all tests pass."

**Files Affected**:
- `test_task_breakdown.spec.ts`: 6 instances (lines 93, 114, 206, 226, 290, 316)
- `test_metrics_ui.spec.ts`: 1 instance (line 201)
- `test_start_agent_flow.spec.ts`: 3 instances (lines 83, 135, 190)
- `test_complete_user_journey.spec.ts`: 1 instance (line 107)

### 3. WebSocket Test Accepts 0 Messages (CRITICAL)

**Location**: `test_dashboard.spec.ts:444-452`

**Code**:
```typescript
if (wsMonitor.messages.length === 0) {
console.log('ℹ️ No WebSocket messages received (backend is passive)');
// Accept 0 messages as long as connection succeeded
}
```

**Impact**: WebSocket test passes even if NO messages are received. This completely defeats the purpose of testing real-time updates.

### 4. `.catch(() => false)` Swallows Errors (HIGH)

**Pattern**:
```typescript
const hasError = await errorSection.isVisible().catch(() => false);
```

**Impact**: If checking visibility throws an error (e.g., element detached, timeout), it's silently treated as "not visible" instead of failing.

**Occurrences**: 40+ instances across all test files

### 5. `toBeAttached()` Instead of `toBeVisible()` (MEDIUM)

**Pattern**:
```typescript
await expect(element).toBeAttached(); // Only checks DOM presence
// Should be:
await expect(element).toBeVisible(); // Checks actually rendered
```

**Impact**: Elements can exist in DOM but be invisible (display:none, visibility:hidden). Tests pass but user can't see the element.

**Files with excessive `toBeAttached()` usage**:
- `test_task_execution_flow.spec.ts`: 10 instances
- `test_complete_user_journey.spec.ts`: 4 instances
- `test_dashboard.spec.ts`: 9 instances

### 6. Console.log Instead of Assertions (LOW)

**Pattern**:
```typescript
console.log('✅ Feature works');
// No actual assertion!
```

**Impact**: Logs success without verifying condition is actually true.

### 7. No API Response Validation (HIGH)
## Current Test Architecture

**Pattern**:
```typescript
await button.click();
// No verification that API call succeeded!
```

**Impact**: Tests verify UI changes but not that the backend actually processed the request.

**Exception**: Only `test_task_breakdown.spec.ts` now validates API response (after recent fix).

## Recommended Fixes

### Fix 1: Remove Overly Broad Error Filters

Replace:
```typescript
checkTestErrors(page, 'Test', ['WebSocket', 'ws://', ...]);
```

With:
```typescript
checkTestErrors(page, 'Test', [
'net::ERR_ABORTED' // Only filter navigation cancellations
]);
### Authentication Flow
```

### Fix 2: Replace Conditional Skips with Proper Setup

Replace:
```typescript
if (!isVisible) {
test.skip(true, 'Not in correct state');
}
loginUser(page) -> /login page -> fill credentials -> submit -> JWT stored in localStorage
-> redirect to /projects
All subsequent API calls include Authorization: Bearer {token} header
WebSocket connections include ?token={token} query parameter
```

With:
```typescript
// Set up test data to ensure correct state
await setupProjectInPlanningPhase(page, projectId);
await expect(element).toBeVisible();
### Error Monitoring
```

### Fix 3: Assert on API Responses

Add:
```typescript
const responsePromise = page.waitForResponse(
response => response.url().includes('/api/endpoint')
);
await button.click();
const response = await responsePromise;
expect(response.status()).toBeLessThan(400);
setupErrorMonitoring(page) -> captures console errors, network failures, failed requests
afterEach: checkTestErrors(page, context, [minimal filters]) -> asserts no unexpected errors
```

### Fix 4: Replace `.catch(() => false)` with Proper Assertions
### Test File Structure

Replace:
```typescript
const isVisible = await element.isVisible().catch(() => false);
if (isVisible) { /* do something */ }
```
| File | Focus | Auth Method |
|------|-------|-------------|
| `test_auth_flow.spec.ts` | Authentication flows | Real login UI |
| `test_project_creation.spec.ts` | Project CRUD | JWT via loginUser |
| `test_task_breakdown.spec.ts` | Task generation/approval | JWT via loginUser |
| `test_dashboard.spec.ts` | Dashboard + WebSocket | JWT via loginUser |
| `test_complete_user_journey.spec.ts` | End-to-end workflow | JWT via loginUser |
| `test_start_agent_flow.spec.ts` | Discovery + agents | JWT via loginUser |
| `test_metrics_ui.spec.ts` | Metrics dashboard | JWT via loginUser |
| `test_task_execution_flow.spec.ts` | Task execution | JWT via loginUser |

With:
```typescript
await expect(element).toBeVisible({ timeout: 5000 });
```
## Remaining Items (Low Priority)

### Fix 5: WebSocket Test Must Receive Messages
1. **Console.log patterns** - Some tests log success without assertion (acceptable for debugging)
2. **toBeAttached vs toBeVisible** - Some uses are intentional (checking DOM presence before interaction)
3. **Test data fixtures** - Consider adding for more consistent project states

Replace:
```typescript
if (wsMonitor.messages.length === 0) {
// Accept 0 messages
}
```
## Verification

With:
```typescript
expect(wsMonitor.messages.length).toBeGreaterThan(0);
Run full test suite to verify:
```bash
cd tests/e2e
npx playwright test --project=chromium
```

## Files Requiring Immediate Attention

1. **test_dashboard.spec.ts** - WebSocket test is useless
2. **test_task_breakdown.spec.ts** - Too many conditional skips
3. **test-utils.ts** - `checkTestErrors` is too permissive
4. **All test files** - Error filter lists too broad

## Metrics

| Issue Type | Count | Severity |
|------------|-------|----------|
| Error filter masking | 8 files | CRITICAL |
| Conditional skips | 11 instances | CRITICAL |
| WebSocket 0-message acceptance | 1 instance | CRITICAL |
| `.catch(() => false)` | 40+ instances | HIGH |
| `toBeAttached()` misuse | 23 instances | MEDIUM |
| Console.log not assertion | 20+ instances | LOW |
Expected: All tests pass with real authentication. Any 401 errors or auth failures will cause test failures.
65 changes: 31 additions & 34 deletions tests/e2e/README-USER-JOURNEY-TESTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ This document describes the implementation of comprehensive E2E tests that valid
- Session persistence across navigation
- Protected route access when authenticated
- Redirect to login when accessing protected routes unauthenticated
- BetterAuth API integration (sign-in endpoint)
- FastAPI Users JWT API integration (sign-in endpoint)
- Database integration (session creation in CodeFRAME tables)

### 2. `test_project_creation.spec.ts` (3 test cases)
Expand Down Expand Up @@ -97,52 +97,49 @@ The following components were updated with `data-testid` attributes for stable t
- Submits
- Waits for next question or completion

## Authentication System: Unified BetterAuth Integration
## Authentication System: FastAPI Users JWT Authentication

### ✅ Resolved: BetterAuth/CodeFRAME Auth Alignment (Issue #158)
### Current Authentication Architecture

**Previous Issue:**
E2E tests used an auth bypass mechanism (`auth-bypass.ts` + `setTestUserSession()`) because BetterAuth expected singular table names (`user`, `session`) while CodeFRAME used plural names (`users`, `sessions`). This mismatch prevented the login UI from working in tests.

**Resolution:**
Configured BetterAuth to use CodeFRAME's existing plural table names via `usePlural: true` setting in `web-ui/src/lib/auth.ts`. This aligns both systems to use the same database schema.
The application uses FastAPI Users with JWT tokens for authentication. E2E tests use real authentication flows.

**Implementation:**
- **BetterAuth Config:** Added `usePlural: true` to use `users` and `sessions` tables
- **Password Hashing:** Both systems use bcrypt by default (compatible)
- **Session Storage:** BetterAuth now creates sessions in CodeFRAME's `sessions` table
- **Backend Validation:** Existing backend auth (`codeframe/ui/auth.py`) validates BetterAuth sessions seamlessly

**Test Changes:**
- **Removed:** `auth-bypass.ts` file (auth bypass mechanism deleted)
- **Removed:** Session token file generation in `seed-test-data.py` and `global-setup.ts`
- **Updated:** All E2E tests now use `loginUser()` helper for real authentication
- **Enhanced:** `test_auth_flow.spec.ts` expanded to 18 comprehensive auth tests covering:
- Login success/failure scenarios
- Session persistence across reloads
- Protected route access
- BetterAuth API integration
- Database integration validation

**Test User:**
- **Backend:** FastAPI Users module in `codeframe/auth/`
- **Frontend:** JWT token stored in `localStorage.getItem('auth_token')`
- **WebSocket:** Token included as query parameter: `?token={jwt_token}`
- **API Client:** Authenticated axios instance in `web-ui/src/lib/api.ts`

**Test Authentication Flow:**
1. `loginUser(page)` navigates to `/login`
2. Fills email/password credentials
3. Submits form, JWT returned and stored in localStorage
4. Redirect to `/projects` confirms successful auth
5. All subsequent API calls include `Authorization: Bearer {token}` header

**Test User Credentials:**
- Email: `[email protected]`
- Password: `testpassword123`
- Seeded by `seed-test-data.py` into `users` table with bcrypt hash
- Sessions created by BetterAuth during login are stored in `sessions` table
- Password: `Testpassword123`
- Seeded by `seed-test-data.py` into `users` table

**E2E Test Helpers (in `test-utils.ts`):**
- `loginUser(page)` - Real login via UI
- `registerUser(page, name, email, password)` - Real signup via UI
- `isAuthenticated(page)` - Check localStorage for auth token
- `clearAuth(page)` - Remove auth token
- `getAuthToken(page)` - Get current JWT token

**Benefits:**
- ✅ Tests now validate the real authentication flow
- ✅ Single source of truth for user data (CodeFRAME database)
- ✅ BetterAuth features (OAuth, 2FA) can be added without schema conflicts
- ✅ No more auth bypass complexity in test code
- ✅ Tests validate the real authentication flow end-to-end
- ✅ 401 errors in tests indicate real authentication bugs
- ✅ No mocking or bypassing - what works in tests works in production

## Current Status & Known Issues

### ✅ Completed
- All frontend components have data-testid attributes
- Test utilities created
- 4 test spec files with comprehensive test cases written
- **Unified authentication system** - BetterAuth aligned with CodeFRAME schema
- **Unified authentication system** - FastAPI Users JWT authentication
- Tests use real login flow (no more auth bypass)
- TypeScript compilation passes
- Frontend build succeeds
Expand Down Expand Up @@ -229,7 +226,7 @@ Projects created during tests use timestamps to:
| `test_complete_user_journey.spec.ts` with 1 test | ✅ Complete |
| Helper utilities in `test-utils.ts` | ✅ Complete |
| Tests pass on Chromium, Firefox, WebKit | ✅ Complete - 15/15 project creation tests passing |
| Tests run in CI without flakiness | ✅ Complete - Auth bypass allows consistent execution |
| Tests run in CI without flakiness | ✅ Complete - Real authentication flow ensures production-like testing |
| Coverage for `/login`, `/`, dashboard flows | ✅ Complete |

## Files Modified
Expand Down
Loading
Loading