Charon/docs/plans/current_spec.md

# E2E Test Failures Remediation Plan

**Created:** 2026-02-01
**Status:** Planning
**Priority:** P0 - Blocking CI/CD Pipeline
**Assignee:** Playwright_Dev, QA_Security

---

## Executive Summary

Two categories of E2E test failures blocking CI:
1. **Feature Toggle Timeouts** (4 tests) - Promise.all() race condition with PUT + GET requests
2. **Clipboard Access Failure** (1 test) - WebKit security restrictions in CI

Both issues have clear root causes and established remediation patterns in the codebase.

---

## Issue 1: Feature Toggle Timeouts

### Affected Tests

All in `tests/settings/system-settings.spec.ts`:

| Test Name | Line Range | Status |
|-----------|------------|--------|
| "should toggle Cerberus security feature" | ~131-153 | ❌ Timeout |
| "should toggle CrowdSec console enrollment" | ~165-187 | ❌ Timeout |
| "should toggle uptime monitoring" | ~198-220 | ❌ Timeout |
| "should persist feature toggle changes" | ~231-261 | ❌ Timeout |

### Root Cause Analysis

**Current Implementation (Problematic):**
```typescript
await Promise.all([
  page.waitForResponse(r => r.url().includes('/feature-flags') && r.request().method() === 'PUT'),
  page.waitForResponse(r => r.url().includes('/feature-flags') && r.request().method() === 'GET'),
  toggle.click({ force: true })
]);
```

**Why This Fails:**
1. **Race Condition**: `Promise.all()` expects both responses to complete, but:
   - Click triggers PUT request to update feature flag
   - Frontend immediately makes GET request to refresh state
   - In CI: network latency causes GET to arrive before PUT completes
   - Or: GET completes but PUT timeout is still waiting
2. **No Timeout Specified**: Default Playwright timeout is 30s, masking the issue
3. **Network Latency**: CI environments have higher latency than local Docker
4. **Backend Behavior Validated**:
   - Backend handler exists at `backend/internal/api/handlers/feature_flags_handler.go`
   - GET endpoint: `protected.GET("/feature-flags", ...)` (line 255)
   - PUT endpoint: `protected.PUT("/feature-flags", ...)` (line 256)
   - No backend bugs found - this is purely a test timing issue

**Evidence from Codebase:**
- Backend API is correctly implemented (verified in search results)
- No similar `Promise.all()` patterns exist for `waitForResponse` in other tests
- Similar API calls in other tests use sequential waits or `clickAndWaitForResponse` helper

### Solution Design

**Pattern to Follow:**
Use existing `clickAndWaitForResponse` helper from `tests/utils/wait-helpers.ts` (lines 30-56):
```typescript
export async function clickAndWaitForResponse(
  page: Page,
  clickTarget: Locator | string,
  urlPattern: string | RegExp,
  options: { status?: number; timeout?: number } = {}
): Promise<Response> {
  const { status = 200, timeout = 30000 } = options;
  const locator = typeof clickTarget === 'string' ? page.locator(clickTarget) : clickTarget;

  const [response] = await Promise.all([
    page.waitForResponse(
      (resp) => {
        const urlMatch = typeof urlPattern === 'string'
          ? resp.url().includes(urlPattern)
          : urlPattern.test(resp.url());
        return urlMatch && resp.status() === status;
      },
      { timeout }
    ),
    locator.click(),
  ]);

  return response;
}
```

**New Implementation Strategy:**
1. Use `clickAndWaitForResponse` for PUT request (handles click + first response atomically)
2. Add explicit 10s timeout for the PUT request
3. Wait separately for GET request with explicit timeout
4. Add verification of final state after both requests complete

**Code Changes Required:**

**File:** `tests/settings/system-settings.spec.ts`

**Before (Lines ~145-153):**
```typescript
const initialState = await toggle.isChecked().catch(() => false);
// Use force to bypass sticky header interception
await Promise.all([
  page.waitForResponse(r => r.url().includes('/feature-flags') && r.request().method() === 'PUT'),
  page.waitForResponse(r => r.url().includes('/feature-flags') && r.request().method() === 'GET'),
  toggle.click({ force: true })
]);

const newState = await toggle.isChecked().catch(() => !initialState);
expect(newState).not.toBe(initialState);
```

**After:**
```typescript
const initialState = await toggle.isChecked().catch(() => false);

// Step 1: Click toggle and wait for PUT request to complete (atomic operation)
const putResponse = await clickAndWaitForResponse(
  page,
  toggle,
  /\/feature-flags/,
  { status: 200, timeout: 10000 }
);
expect(putResponse.ok()).toBeTruthy();

// Step 2: Wait for subsequent GET request to refresh state
const getResponse = await waitForAPIResponse(
  page,
  /\/feature-flags/,
  { status: 200, timeout: 5000 }
);
expect(getResponse.ok()).toBeTruthy();

// Step 3: Verify toggle state changed
const newState = await toggle.isChecked().catch(() => !initialState);
expect(newState).not.toBe(initialState);
```

**Imports to Add (Line ~6):**
```typescript
import { waitForLoadingComplete, waitForToast, waitForAPIResponse, clickAndWaitForResponse } from '../utils/wait-helpers';
```

### Implementation Tasks

#### Phase 1: Update Test Helper Imports
- [ ] **Task 1.1**: Add `clickAndWaitForResponse` and `waitForAPIResponse` imports to `tests/settings/system-settings.spec.ts`
  - File: `tests/settings/system-settings.spec.ts`
  - Line: 6 (import statement)
  - Expected: Import compilation succeeds

#### Phase 2: Fix Feature Toggle Tests
- [ ] **Task 2.1**: Update "should toggle Cerberus security feature" test
  - File: `tests/settings/system-settings.spec.ts`
  - Lines: 145-153
  - Change: Replace `Promise.all()` with sequential `clickAndWaitForResponse` + `waitForAPIResponse`
  - Expected: Test completes in <5s, no timeout errors

- [ ] **Task 2.2**: Update "should toggle CrowdSec console enrollment" test
  - File: `tests/settings/system-settings.spec.ts`
  - Lines: 177-185
  - Change: Same pattern as Task 2.1
  - Expected: Test completes in <5s, no timeout errors

- [ ] **Task 2.3**: Update "should toggle uptime monitoring" test
  - File: `tests/settings/system-settings.spec.ts`
  - Lines: 210-218
  - Change: Same pattern as Task 2.1
  - Expected: Test completes in <5s, no timeout errors

- [ ] **Task 2.4**: Update "should persist feature toggle changes" test (2 toggle operations)
  - File: `tests/settings/system-settings.spec.ts`
  - Lines: 245-253, 263-271
  - Change: Apply pattern to both toggle clicks in the test
  - Expected: Test completes in <10s, state persists across page reload

#### Phase 3: Validation
- [ ] **Task 3.1**: Run all system-settings tests locally
  - Command: `npx playwright test tests/settings/system-settings.spec.ts --project=chromium`
  - Expected: All 4 feature toggle tests pass, execution time <30s total

- [ ] **Task 3.2**: Run tests against Docker container (E2E environment)
  - Command: `.github/skills/scripts/skill-runner.sh docker-rebuild-e2e && npm run e2e -- tests/settings/system-settings.spec.ts`
  - Expected: All tests pass in Docker environment

- [ ] **Task 3.3**: Run cross-browser validation
  - Command: `npx playwright test tests/settings/system-settings.spec.ts --project=chromium --project=firefox --project=webkit`
  - Expected: All browsers pass without timeouts

### Risk Assessment

| Risk | Impact | Likelihood | Mitigation |
|------|--------|------------|------------|
| Sequential waits slower than parallel | Low (adds ~1-2s per test) | High | Acceptable trade-off for reliability |
| Other tests have similar pattern | Medium (more failures) | Low | Isolated to feature-flags endpoint |
| Backend timing changes in future | Low (tests become brittle) | Low | Explicit timeouts catch regressions early |
| CI network latency varies | Medium (flakiness) | Medium | 10s timeout provides buffer for slow CI |

---

## Issue 2: Clipboard Access Failure

### Affected Tests

| Test Name | File | Line | Status |
|-----------|------|------|--------|
| "should copy invite link" | `tests/settings/user-management.spec.ts` | ~431 | ❌ NotAllowedError (WebKit) |

### Root Cause Analysis

**Error Message:**
```
NotAllowedError: The request is not allowed by the user agent or the platform in the current context
```

**Why This Fails:**
1. **Browser Security**: Clipboard API requires explicit permissions
2. **Playwright Limitations**:
   - Only Chromium supports `clipboard-read`/`clipboard-write` permission grants
   - Firefox/WebKit: Playwright cannot grant clipboard permissions in CI contexts
3. **CI Environment**: Headless WebKit is particularly strict about clipboard access
4. **Current Test**: Attempts clipboard verification on all browsers without browser-specific logic

**Evidence from Codebase:**
Similar test in `tests/settings/account-settings.spec.ts` (lines 602-657) already implements the correct pattern:
```typescript
test('should copy API key to clipboard', async ({ page, context }, testInfo) => {
  // Grant clipboard permissions. Firefox/WebKit do not support 'clipboard-read'
  const browserName = testInfo.project?.name || '';
  if (browserName === 'chromium') {
    await context.grantPermissions(['clipboard-read', 'clipboard-write']);
  }

  // ... later in test ...

  await test.step('Verify clipboard contains API key (Chromium-only); verify toast for other browsers', async () => {
    if (browserName !== 'chromium') {
      // Non-Chromium: verify success toast instead
      const apiKeyInput = page.locator('input[readonly].font-mono');
      await expect(apiKeyInput).toHaveValue(/\S+/);
      return; // skip clipboard-read on non-Chromium
    }

    // Chromium-only: verify clipboard contents
    const clipboardText = await page.evaluate(async () => {
      try {
        return await navigator.clipboard.readText();
      } catch (err) {
        throw new Error(`clipboard.readText() failed: ${err?.message || err}`);
      }
    });

    expect(clipboardText).toContain('accept-invite');
    expect(clipboardText).toContain('token=');
  });
});
```

### Solution Design

**Pattern to Follow:**
1. **Access `testInfo.project?.name`** to detect browser
2. **Grant permissions on Chromium only**: `context.grantPermissions(['clipboard-read', 'clipboard-write'])`
3. **Skip clipboard verification on Firefox/WebKit**: Return early from that test step
4. **Fallback verification**: Verify success toast on non-Chromium browsers

**Code Changes Required:**

**File:** `tests/settings/user-management.spec.ts`

**Before (Lines ~402-431):**
```typescript
test('should copy invite link', async ({ page, context }, testInfo) => {
  // Grant clipboard permissions only on Chromium — Firefox/WebKit don't support clipboard-read/write.
  const browserName = testInfo.project?.name || '';
  if (browserName === 'chromium') {
    await context.grantPermissions(['clipboard-read', 'clipboard-write']);
  }

  const testEmail = `copy-test-${Date.now()}@test.local`;

  await test.step('Create an invite', async () => {
    // ... existing code ...
  });

  await test.step('Click copy button', async () => {
    const copyButton = page.getByRole('button', { name: /copy/i }).or(
      page.getByRole('button').filter({ has: page.locator('svg.lucide-copy') })
    );

    await expect(copyButton.first()).toBeVisible();
    await copyButton.first().click();
  });

  await test.step('Verify copy success toast', async () => {
    const copiedToast = page.locator('[data-testid="toast-success"]').filter({
      hasText: /copied|clipboard/i,
    });
    await expect(copiedToast).toBeVisible({ timeout: 10000 });
  });

  await test.step('Verify clipboard contains invite link', async () => {
    const clipboardText = await page.evaluate(() => navigator.clipboard.readText());
    expect(clipboardText).toContain('accept-invite');
    expect(clipboardText).toContain('token=');
  });
});
```

**After:**
```typescript
test('should copy invite link', async ({ page, context }, testInfo) => {
  // Grant clipboard permissions only on Chromium — Firefox/WebKit don't support clipboard-read/write.
  const browserName = testInfo.project?.name || '';
  if (browserName === 'chromium') {
    await context.grantPermissions(['clipboard-read', 'clipboard-write']);
  }

  const testEmail = `copy-test-${Date.now()}@test.local`;

  await test.step('Create an invite', async () => {
    // ... existing code (unchanged) ...
  });

  await test.step('Click copy button', async () => {
    const copyButton = page.getByRole('button', { name: /copy/i }).or(
      page.getByRole('button').filter({ has: page.locator('svg.lucide-copy') })
    );

    await expect(copyButton.first()).toBeVisible();
    await copyButton.first().click();
  });

  await test.step('Verify copy success toast', async () => {
    const copiedToast = page.locator('[data-testid="toast-success"]').filter({
      hasText: /copied|clipboard/i,
    });
    await expect(copiedToast).toBeVisible({ timeout: 10000 });
  });

  await test.step('Verify clipboard contains invite link (Chromium-only); verify toast for other browsers', async () => {
    // WebKit/Firefox: Clipboard API throws NotAllowedError in CI
    // We've already verified the success toast above, which is sufficient proof
    if (browserName !== 'chromium') {
      // Additional verification: Ensure invite link is still visible (defensive check)
      const inviteLinkInput = page.locator('input[readonly]').filter({
        hasText: /accept-invite|token/i
      });
      const inviteLinkVisible = await inviteLinkInput.first().isVisible({ timeout: 2000 }).catch(() => false);
      if (inviteLinkVisible) {
        await expect(inviteLinkInput.first()).toHaveValue(/accept-invite.*token=/);
      }
      return; // Skip clipboard verification on non-Chromium
    }

    // Chromium-only: Verify clipboard contents
    // This is the only browser where we can reliably read clipboard in CI
    const clipboardText = await page.evaluate(async () => {
      try {
        return await navigator.clipboard.readText();
      } catch (err) {
        throw new Error(`clipboard.readText() failed: ${err?.message || err}`);
      }
    });

    expect(clipboardText).toContain('accept-invite');
    expect(clipboardText).toContain('token=');
  });
});
```

**Key Changes:**
1. **Line ~402**: Browser detection already exists (no change)
2. **Line ~431**: Wrap clipboard verification in browser check
3. **Lines ~432-442**: Add fallback verification for non-Chromium browsers (toast + optional input check)
4. **Lines ~444-453**: Move clipboard verification inside Chromium-only block
5. **Error Handling**: Add try-catch with descriptive error message

### Implementation Tasks

#### Phase 1: Update Clipboard Test
- [ ] **Task 1.1**: Update "should copy invite link" test with browser-specific logic
  - File: `tests/settings/user-management.spec.ts`
  - Lines: 431-455
  - Change: Add browser detection to last test step, skip clipboard read on non-Chromium
  - Expected: Test passes on all browsers (Chromium verifies clipboard, others verify toast)

#### Phase 2: Validation
- [ ] **Task 2.1**: Run test locally on Chromium
  - Command: `npx playwright test tests/settings/user-management.spec.ts --project=chromium --grep "should copy invite link"`
  - Expected: Test passes, clipboard verification succeeds

- [ ] **Task 2.2**: Run test locally on Firefox
  - Command: `npx playwright test tests/settings/user-management.spec.ts --project=firefox --grep "should copy invite link"`
  - Expected: Test passes, skips clipboard verification, verifies toast

- [ ] **Task 2.3**: Run test locally on WebKit
  - Command: `npx playwright test tests/settings/user-management.spec.ts --project=webkit --grep "should copy invite link"`
  - Expected: Test passes, skips clipboard verification, verifies toast

- [ ] **Task 2.4**: Run full user-management test suite cross-browser
  - Command: `npx playwright test tests/settings/user-management.spec.ts --project=chromium --project=firefox --project=webkit`
  - Expected: All tests pass on all browsers

#### Phase 3: Verify CI Behavior
- [ ] **Task 3.1**: Commit changes and push to feature branch
  - Expected: CI runs E2E tests

- [ ] **Task 3.2**: Verify CI test results
  - Expected: WebKit tests pass without NotAllowedError

### Risk Assessment

| Risk | Impact | Likelihood | Mitigation |
|------|--------|------------|------------|
| Toast verification insufficient | Low (false positive) | Low | Fallback input visibility check added |
| Chromium clipboard fails in CI | Medium (regression) | Low | Existing pattern works in account-settings test |
| Future clipboard changes break test | Low (maintenance burden) | Low | Pattern is well-documented and reusable |
| Copy functionality broken but toast still shows | Medium (false negative) | Low | Chromium provides full verification |

---

## Related Files Verification

### Configuration Files

| File | Status | Notes |
|------|--------|-------|
| `.gitignore` | ✅ Verified | Test artifacts properly excluded |
| `codecov.yml` | ✅ Verified | E2E coverage properly configured, patch threshold 85% |
| `.dockerignore` | ✅ Verified | Test files excluded from Docker context |
| `Dockerfile` | ✅ Verified | Backend endpoints exposed on port 8080, container healthy |
| `playwright.config.js` | ✅ Verified | Timeout: 30s global, 5s for expect(), base URL: http://localhost:8080 |

**Key Findings:**
- **Codecov**: Patch coverage threshold is 85%, must maintain 100% coverage for modified lines
- **Docker**: Container exposes backend API on port 8080, health check verifies `/api/v1/health`
- **Playwright**: Global timeout is 30s (explains why timeouts take so long), expect timeout is 5s
- **E2E Environment**: Tests run against Docker container on port 8080 (not Vite dev server)

---

## Test Execution Strategy

### Phase Order
1. **Phase 1**: Feature Toggle Tests (Issue 1) - Higher impact, affects 4 tests
2. **Phase 2**: Clipboard Test (Issue 2) - Lower impact, affects 1 test
3. **Phase 3**: Full Validation - Cross-browser, CI verification

### Pre-Execution Checklist
- [ ] Backend API running on port 8080
- [ ] Docker container healthy (health check passing)
- [ ] Database migrations applied
- [ ] Feature flags endpoint accessible: `curl http://localhost:8080/api/v1/feature-flags`
- [ ] Admin user exists in database
- [ ] Auth cookies are valid (session not expired)

### Validation Commands

**Local Docker Environment:**
```bash
# 1. Rebuild E2E container with latest code
.github/skills/scripts/skill-runner.sh docker-rebuild-e2e

# 2. Run affected tests (Issue 1)
npx playwright test tests/settings/system-settings.spec.ts \
  --grep "should toggle Cerberus security feature|should toggle CrowdSec console enrollment|should toggle uptime monitoring|should persist feature toggle changes" \
  --project=chromium

# 3. Run affected test (Issue 2)
npx playwright test tests/settings/user-management.spec.ts \
  --grep "should copy invite link" \
  --project=chromium --project=firefox --project=webkit

# 4. Full validation
npx playwright test tests/settings/ --project=chromium --project=firefox --project=webkit
```

**Coverage Validation:**
```bash
# Run E2E tests with coverage (uses Vite dev server on port 5173)
.github/skills/scripts/skill-runner.sh test-e2e-playwright-coverage

# Check coverage report
open coverage/e2e/index.html

# Verify LCOV file exists for Codecov
ls -la coverage/e2e/lcov.info
```

### Expected Outcomes

**Success Criteria:**
- [ ] All 4 feature toggle tests complete in <10s each
- [ ] "should copy invite link" test passes on all browsers
- [ ] No timeout errors in CI logs
- [ ] No NotAllowedError in WebKit tests
- [ ] Test execution time reduced from ~2 minutes to <30 seconds
- [ ] E2E coverage report shows non-zero coverage percentages

**Acceptance Criteria:**
1. **All E2E tests pass** in CI (Chromium, Firefox, WebKit)
2. **No test timeouts** (30s global timeout not reached)
3. **No clipboard errors** (WebKit/Firefox skip clipboard verification)
4. **Test reliability** improved to 100% pass rate across 3 consecutive CI runs
5. **Patch coverage** maintained at 100% for modified test files (Codecov)

---

## Implementation Priority

### Critical Path (Sequential)
1. ✅ **Research** (Complete) - Root cause identified, patterns researched
2. 🔄 **Planning** (Current) - Comprehensive plan documented
3. 🔲 **Implementation** - Execute tasks in order:
   - Phase 1.1: Update imports
   - Phase 2: Fix feature toggle tests (4 tests)
   - Phase 3: Validate locally
   - Phase 4: Fix clipboard test (1 test)
   - Phase 5: Cross-browser validation
4. 🔲 **CI Verification** - Merge and observe CI results

### Time Estimates
- **Phase 1 (Imports)**: 5 minutes
- **Phase 2 (Feature Toggles)**: 30 minutes (4 similar changes)
- **Phase 3 (Local Validation)**: 15 minutes
- **Phase 4 (Clipboard Test)**: 20 minutes
- **Phase 5 (Cross-Browser)**: 10 minutes
- **CI Verification**: 15 minutes (CI execution time)
- **Total**: ~1.5 hours end-to-end

### Blockers
- None identified - all dependencies available in codebase

---

## Success Metrics

| Metric | Before | Target After | Measurement |
|--------|--------|--------------|-------------|
| E2E Test Pass Rate | ~80% (timeouts) | 100% | CI test results |
| Feature Toggle Test Time | >30s (timeout) | <5s each | Playwright reporter |
| Clipboard Test Failures | 100% on WebKit | 0% | Cross-browser run |
| CI Build Time | ~15 minutes | ~10 minutes | GitHub Actions duration |
| Test Flakiness | High (timeouts) | Zero | 3 consecutive clean runs |

---

## Handoff Checklist

### For Playwright_Dev
- [ ] Implement all code changes in both test files
- [ ] Run local validation commands
- [ ] Verify no regressions in other settings tests
- [ ] Update test documentation with browser-specific behavior notes
- [ ] Create feature branch: `fix/e2e-test-failures`
- [ ] Commit with message: `fix(e2e): resolve feature toggle timeouts and clipboard access errors`

### For QA_Security
- [ ] Review code changes for security implications (clipboard data handling)
- [ ] Verify no sensitive data logged during failures
- [ ] Validate browser permission grants follow least-privilege principle
- [ ] Confirm error messages don't leak internal implementation details
- [ ] Test with security features enabled (Cerberus, CrowdSec)

### For Backend_Dev (Optional)
- [ ] Verify `/feature-flags` endpoint performance under load
- [ ] Check if backend logging can help debug future timing issues
- [ ] Consider adding response time metrics to feature-flags handler
- [ ] No backend changes required for this remediation

---

## Appendix A: Backend API Verification

**Feature Flags Endpoint:**
- **GET** `/api/v1/feature-flags` - Returns map of feature flags
- **PUT** `/api/v1/feature-flags` - Updates feature flags (requires admin auth)

**Handler Location:** `backend/internal/api/handlers/feature_flags_handler.go`

**Routes Configuration:** `backend/internal/api/routes/routes.go` (lines 254-256)

**Backend Behavior:**
1. PUT request updates settings in database (SQLite)
2. Returns `{"status": "ok"}` on success
3. GET request retrieves current state from database
4. No caching layer between PUT and GET

**Verified:** Backend implementation is correct, not the source of timing issues.

---

## Appendix B: Similar Tests Reference

**Account Settings Clipboard Test:**
- File: `tests/settings/account-settings.spec.ts`
- Lines: 602-657
- Pattern: Browser-specific clipboard verification with fallback
- Success Rate: 100% across all browsers

**Other API Toggle Tests:**
- No similar `Promise.all()` patterns found with `waitForResponse`
- Most tests use `clickAndWaitForResponse` or sequential waits
- Pattern: Atomic click + wait, then verify state

---

## Appendix C: Playwright Configuration Reference

**Timeouts:**
- `timeout`: 30000ms (global test timeout)
- `expect.timeout`: 5000ms (assertion timeout)
- `waitForResponse` default: 30000ms (must be overridden)

**Retry Strategy:**
- `retries`: 2 on CI, 0 locally
- `workers`: 1 on CI (sequential), undefined locally (parallel)

**Coverage:**
- Enabled via `PLAYWRIGHT_COVERAGE=1` environment variable
- Uses `@bgotink/playwright-coverage` for V8 coverage
- Requires Vite dev server (port 5173) for source maps

---

## Sign-Off

**Prepared By:** Principal Architect (Planning Agent)
**Review Required:** Supervisor Agent
**Implementation:** Playwright_Dev
**Validation:** QA_Security
**Target Completion:** Within 1 sprint (2 weeks)