docs: addressed PR review concerns

docs/explanation/features/tea-overview.md

@@ -199,7 +199,7 @@ Epic/Release Gate → TEA: *nfr-assess, *trace Phase 2 (release decision)
 
 TEA uniquely requires:
 
-- **Extensive domain knowledge**: 33 fragments covering test patterns, CI/CD, fixtures, quality practices, and optional playwright-utils integration
+- **Extensive domain knowledge**: 30+ fragments covering test patterns, CI/CD, fixtures, quality practices, and optional playwright-utils integration
 - **Cross-cutting concerns**: Domain-specific testing patterns that apply across all BMad projects (vs project-specific artifacts like PRDs/stories)
 - **Optional integrations**: MCP capabilities (exploratory, verification) and playwright-utils support
 

src/modules/bmm/testarch/knowledge/api-request.md

@@ -61,10 +61,11 @@ test('should fetch user data', async ({ apiRequest }) => {
 
 ```typescript
 import { test } from '@seontechnologies/playwright-utils/api-request/fixtures';
+import { z } from 'zod';
 
-test('should validate response schema', async ({ apiRequest }) => {
+// JSON Schema validation
-  // JSON Schema validation
+test('should validate response schema (JSON Schema)', async ({ apiRequest }) => {
-  const response = await apiRequest({
+  const { status, body } = await apiRequest({
     method: 'GET',
     path: '/api/users/123',
     validateSchema: {
@@ -78,22 +79,25 @@ test('should validate response schema', async ({ apiRequest }) => {
     },
   });
   // Throws if schema validation fails
+  expect(status).toBe(200);
+});
 
-  // Zod schema validation
+// Zod schema validation
-  import { z } from 'zod';
+const UserSchema = z.object({
-
-  const UserSchema = z.object({
   id: z.string(),
   name: z.string(),
   email: z.string().email(),
-  });
+});
 
-  const response = await apiRequest({
+test('should validate response schema (Zod)', async ({ apiRequest }) => {
+  const { status, body } = await apiRequest({
     method: 'GET',
     path: '/api/users/123',
     validateSchema: UserSchema,
   });
   // Response body is type-safe AND validated
+  expect(status).toBe(200);
+  expect(body.email).toContain('@');
 });
 ```
 

src/modules/bmm/testarch/knowledge/auth-session.md

@@ -247,90 +247,152 @@ test('parallel test 2', async ({ page }) => {
 
 ### Example 6: Pure API Authentication (No Browser)
 
-**Context**: Get auth tokens for API-only tests without any browser context.
+**Context**: Get auth tokens for API-only tests using auth-session disk persistence.
 
 **Implementation**:
 
 ```typescript
-// tests/api/authenticated-api.spec.ts
+// Step 1: Create API-only auth provider (no browser needed)
-import { test, expect } from '@seontechnologies/playwright-utils/fixtures';
+// playwright/support/api-auth-provider.ts
+import { type AuthProvider } from '@seontechnologies/playwright-utils/auth-session';
 
-test.describe('Authenticated API Tests (No Browser)', () => {
+const apiAuthProvider: AuthProvider = {
-  let authToken: string;
+  getEnvironment: (options) => options.environment || 'local',
+  getUserIdentifier: (options) => options.userIdentifier || 'api-user',
 
-  test.beforeAll(async ({ request }) => {
+  extractToken: (storageState) => {
-    // Get token via API login - no browser needed!
+    // Token stored in localStorage format for disk persistence
-    const response = await request.post('/api/auth/login', {
+    const tokenEntry = storageState.origins?.[0]?.localStorage?.find(
-      data: {
+      (item) => item.name === 'auth_token'
-        email: process.env.TEST_USER_EMAIL,
+    );
-        password: process.env.TEST_USER_PASSWORD,
+    return tokenEntry?.value;
   },
+
+  isTokenExpired: (storageState) => {
+    const expiryEntry = storageState.origins?.[0]?.localStorage?.find(
+      (item) => item.name === 'token_expiry'
+    );
+    if (!expiryEntry) return true;
+    return Date.now() > parseInt(expiryEntry.value, 10);
+  },
+
+  manageAuthToken: async (request, options) => {
+    const email = process.env.TEST_USER_EMAIL;
+    const password = process.env.TEST_USER_PASSWORD;
+
+    if (!email || !password) {
+      throw new Error('TEST_USER_EMAIL and TEST_USER_PASSWORD must be set');
+    }
+
+    // Pure API login - no browser!
+    const response = await request.post('/api/auth/login', {
+      data: { email, password },
     });
 
-    const { token } = await response.json();
+    if (!response.ok()) {
-    authToken = token;
+      throw new Error(`Auth failed: ${response.status()}`);
-  });
+    }
 
-  test('should access protected endpoint', async ({ apiRequest }) => {
+    const { token, expiresIn } = await response.json();
+    const expiryTime = Date.now() + expiresIn * 1000;
+
+    // Return storage state format for disk persistence
+    return {
+      cookies: [],
+      origins: [
+        {
+          origin: process.env.API_BASE_URL || 'http://localhost:3000',
+          localStorage: [
+            { name: 'auth_token', value: token },
+            { name: 'token_expiry', value: String(expiryTime) },
+          ],
+        },
+      ],
+    };
+  },
+};
+
+export default apiAuthProvider;
+
+// Step 2: Create auth fixture
+// playwright/support/fixtures.ts
+import { test as base } from '@playwright/test';
+import { createAuthFixtures, setAuthProvider } from '@seontechnologies/playwright-utils/auth-session';
+import apiAuthProvider from './api-auth-provider';
+
+setAuthProvider(apiAuthProvider);
+
+export const test = base.extend(createAuthFixtures());
+
+// Step 3: Use in tests - token persisted to disk!
+// tests/api/authenticated-api.spec.ts
+import { test } from '../support/fixtures';
+import { expect } from '@playwright/test';
+
+test('should access protected endpoint', async ({ authToken, apiRequest }) => {
+  // authToken is automatically loaded from disk or fetched if expired
   const { status, body } = await apiRequest({
     method: 'GET',
     path: '/api/me',
-      headers: {
+    headers: { Authorization: `Bearer ${authToken}` },
-        Authorization: `Bearer ${authToken}`,
-      },
   });
 
   expect(status).toBe(200);
-    expect(body.email).toBe(process.env.TEST_USER_EMAIL);
+});
-  });
 
-  test('should create resource with auth', async ({ apiRequest }) => {
+test('should create resource with auth', async ({ authToken, apiRequest }) => {
   const { status, body } = await apiRequest({
     method: 'POST',
     path: '/api/orders',
-      headers: {
+    headers: { Authorization: `Bearer ${authToken}` },
-        Authorization: `Bearer ${authToken}`,
+    body: { items: [{ productId: 'prod-1', quantity: 2 }] },
-      },
-      body: {
-        items: [{ productId: 'prod-1', quantity: 2 }],
-      },
   });
 
   expect(status).toBe(201);
   expect(body.id).toBeDefined();
-  });
 });
 ```
 
 **Key Points**:
 
-- Token obtained via API login (no browser!)
+- Token persisted to disk (not in-memory) - survives test reruns
-- Token reused across all tests in describe block
+- Provider fetches token once, reuses until expired
-- Pure API testing - fast and lightweight
+- Pure API authentication - no browser context needed
-- No `page` or `context` needed
+- `authToken` fixture handles disk read/write automatically
+- Environment variables validated with clear error message
 
 ### Example 7: Service-to-Service Authentication
 
-**Context**: Test microservice authentication patterns (API keys, service tokens, mTLS simulation).
+**Context**: Test microservice authentication patterns (API keys, service tokens) with proper environment validation.
 
 **Implementation**:
 
 ```typescript
 // tests/api/service-auth.spec.ts
-import { test, expect } from '@seontechnologies/playwright-utils/fixtures';
+import { test as base, expect } from '@playwright/test';
+import { test as apiFixture } from '@seontechnologies/playwright-utils/api-request/fixtures';
+import { mergeTests } from '@playwright/test';
+
+// Validate environment variables at module load
+const SERVICE_API_KEY = process.env.SERVICE_API_KEY;
+const INTERNAL_SERVICE_URL = process.env.INTERNAL_SERVICE_URL;
+
+if (!SERVICE_API_KEY) {
+  throw new Error('SERVICE_API_KEY environment variable is required');
+}
+if (!INTERNAL_SERVICE_URL) {
+  throw new Error('INTERNAL_SERVICE_URL environment variable is required');
+}
+
+const test = mergeTests(base, apiFixture);
 
 test.describe('Service-to-Service Auth', () => {
-  const SERVICE_API_KEY = process.env.SERVICE_API_KEY;
-  const INTERNAL_SERVICE_URL = process.env.INTERNAL_SERVICE_URL;
-
   test('should authenticate with API key', async ({ apiRequest }) => {
     const { status, body } = await apiRequest({
       method: 'GET',
       path: '/internal/health',
       baseUrl: INTERNAL_SERVICE_URL,
-      headers: {
+      headers: { 'X-API-Key': SERVICE_API_KEY },
-        'X-API-Key': SERVICE_API_KEY,
-      },
     });
 
     expect(status).toBe(200);
@@ -342,9 +404,7 @@ test.describe('Service-to-Service Auth', () => {
       method: 'GET',
       path: '/internal/health',
       baseUrl: INTERNAL_SERVICE_URL,
-      headers: {
+      headers: { 'X-API-Key': 'invalid-key' },
-        'X-API-Key': 'invalid-key',
-      },
     });
 
     expect(status).toBe(401);
@@ -352,18 +412,15 @@ test.describe('Service-to-Service Auth', () => {
   });
 
   test('should call downstream service with propagated auth', async ({ apiRequest }) => {
-    // Simulate service calling another service with forwarded auth
     const { status, body } = await apiRequest({
       method: 'POST',
       path: '/internal/aggregate-data',
       baseUrl: INTERNAL_SERVICE_URL,
       headers: {
         'X-API-Key': SERVICE_API_KEY,
-        'X-Request-ID': `test-${Date.now()}`, // Correlation ID
+        'X-Request-ID': `test-${Date.now()}`,
-      },
-      body: {
-        sources: ['users', 'orders', 'inventory'],
       },
+      body: { sources: ['users', 'orders', 'inventory'] },
     });
 
     expect(status).toBe(200);
@@ -374,11 +431,14 @@ test.describe('Service-to-Service Auth', () => {
 
 **Key Points**:
 
-- API key authentication (no OAuth flow)
+- Environment variables validated at module load with clear errors
+- API key authentication (simpler than OAuth - no disk persistence needed)
 - Test internal/service endpoints
 - Validate auth rejection scenarios
 - Correlation ID for request tracing
 
+> **Note**: API keys are typically static secrets that don't expire, so disk persistence (auth-session) isn't needed. For rotating service tokens, use the auth-session provider pattern from Example 6.
+
 ## Custom Auth Provider Pattern
 
 **Context**: Adapt auth-session to your authentication system (OAuth2, JWT, SAML, custom).

src/modules/bmm/testarch/knowledge/file-utils.md

@@ -280,25 +280,25 @@ expect(headers).toContain('age');
 ### CSV Reader Options
 
 | Option         | Type               | Default  | Description                            |
-| -------------- | ------------------------ | ------- | --------------------------------------- |
+| -------------- | ------------------ | -------- | -------------------------------------- |
 | `filePath`     | `string`           | -        | Path to CSV file (mutually exclusive)  |
 | `content`      | `string \| Buffer` | -        | Direct content (mutually exclusive)    |
 | `delimiter`    | `string \| 'auto'` | `','`    | Value separator, auto-detect if 'auto' |
-| `encoding`     | `string`                 | `'utf8'`| File encoding                           |
+| `encoding`     | `string`           | `'utf8'` | File encoding                          |
 | `parseHeaders` | `boolean`          | `true`   | Use first row as headers               |
 | `trim`         | `boolean`          | `true`   | Trim whitespace from values            |
 
 ### XLSX Reader Options
 
 | Option      | Type     | Description                    |
-| ----------- | -------- | ------------------------------------ |
+| ----------- | -------- | ------------------------------ |
 | `filePath`  | `string` | Path to XLSX file              |
 | `sheetName` | `string` | Name of sheet to set as active |
 
 ### PDF Reader Options
 
 | Option       | Type      | Default | Description                 |
-| ------------ | --------- | ------- | -------------------------------- |
+| ------------ | --------- | ------- | --------------------------- |
 | `filePath`   | `string`  | -       | Path to PDF file (required) |
 | `mergePages` | `boolean` | `true`  | Merge text from all pages   |
 | `maxPages`   | `number`  | -       | Maximum pages to extract    |
@@ -307,10 +307,67 @@ expect(headers).toContain('age');
 ### ZIP Reader Options
 
 | Option          | Type     | Description                        |
-| --------------- | -------- | -------------------------------------- |
+| --------------- | -------- | ---------------------------------- |
 | `filePath`      | `string` | Path to ZIP file                   |
 | `fileToExtract` | `string` | Specific file to extract to Buffer |
 
+### Return Values
+
+#### CSV Reader Return Value
+
+```typescript
+{
+  content: {
+    data: Array<Array<string | number>>,  // Parsed rows (excludes header row if parseHeaders: true)
+    headers: string[] | null              // Column headers (null if parseHeaders: false)
+  }
+}
+```
+
+#### XLSX Reader Return Value
+
+```typescript
+{
+  content: {
+    worksheets: Array<{
+      name: string,                       // Sheet name
+      rows: Array<Array<any>>,            // All rows including headers
+      headers?: string[]                  // First row as headers (if present)
+    }>
+  }
+}
+```
+
+#### PDF Reader Return Value
+
+```typescript
+{
+  content: string,                        // Extracted text (merged or per-page based on mergePages)
+  pagesCount: number,                     // Total pages in PDF
+  fileName?: string,                      // Original filename if available
+  info?: Record<string, any>              // PDF metadata (author, title, etc.)
+}
+```
+
+> **Note**: When `mergePages: false`, `content` is an array of strings (one per page). When `maxPages` is set, only that many pages are extracted.
+
+#### ZIP Reader Return Value
+
+```typescript
+{
+  content: {
+    entries: Array<{
+      name: string,                       // File/directory path within ZIP
+      size: number,                       // Uncompressed size in bytes
+      isDirectory: boolean                // True for directories
+    }>,
+    extractedFiles: Record<string, Buffer | string>  // Extracted file contents by path
+  }
+}
+```
+
+> **Note**: When `fileToExtract` is specified, only that file appears in `extractedFiles`.
+
 ## Download Cleanup Pattern
 
 ```typescript

src/modules/bmm/testarch/knowledge/log.md

@@ -196,6 +196,9 @@ class TodoPage {
 ```typescript
 import { functionTestStep } from '@seontechnologies/playwright-utils';
 
+// Define todo items for the test
+const TODO_ITEMS = ['buy groceries', 'pay bills', 'schedule meeting'];
+
 const createDefaultTodos = functionTestStep('Create default todos', async (page: Page) => {
   await log.info('Creating default todos');
   await log.step('step within a functionWrapper');

src/modules/bmm/testarch/knowledge/recurse.md

@@ -267,7 +267,8 @@ test('kafka event processed', async ({ recurse, apiRequest }) => {
   const inventoryResult = await recurse(
     () => apiRequest({ method: 'GET', path: '/api/inventory/ABC123' }),
     (res) => {
-      // Inventory should decrease by 2 after consumer processes event
+      // Assumes test fixture seeds inventory at 100; in production tests,
+      // fetch baseline first and assert: expect(res.body.available).toBe(baseline - 2)
       expect(res.body.available).toBeLessThanOrEqual(98);
     },
     {
@@ -329,7 +330,7 @@ test('end-to-end polling', async ({ apiRequest, recurse }) => {
 ### RecurseOptions
 
 | Option     | Type               | Default     | Description                          |
-| ---------- | ----------------------- | ----------- | ---------------------------------------------- |
+| ---------- | ------------------ | ----------- | ------------------------------------ |
 | `timeout`  | `number`           | `30000`     | Maximum time to wait (ms)            |
 | `interval` | `number`           | `1000`      | Time between polls (ms)              |
 | `log`      | `string`           | `undefined` | Message logged on each poll          |
@@ -340,10 +341,10 @@ test('end-to-end polling', async ({ apiRequest, recurse }) => {
 ### Error Types
 
 | Error Type              | When Thrown                             | Properties                               |
-| ---------------------- | ---------------------------------------- | -------------------------------------------- |
+| ----------------------- | --------------------------------------- | ---------------------------------------- |
 | `RecurseTimeoutError`   | Predicate never passed within timeout   | `lastCommandValue`, `lastPredicateError` |
 | `RecurseCommandError`   | Command function threw an error         | `cause` (original error)                 |
-| `RecursePredicateError`| Predicate threw (not assertion failure) | `cause` (original error)                     |
+| `RecursePredicateError` | Predicate threw (not assertion failure) | `cause` (original error)                 |
 
 ## Comparison with Vanilla Playwright