docs(FR-2018): enhance Playwright E2E test agent documentation (#5229)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: yomybaby

.claude/agents/playwright-test-generator.md

@@ -20,6 +20,7 @@ application behavior.
 - Immediately after reading the test log, invoke `generator_write_test` with the generated source code
   - File should contain single test
   - File name must be fs-friendly scenario name (use kebab-case with `.spec.ts` extension)
+  - Place test files in appropriate feature directory: `e2e/session/`, `e2e/serving/`, `e2e/user/`, etc.
   - Test must be placed in a describe matching the top-level test plan item
   - **Test title must use user-scenario-based naming convention**
     - Format: `[Actor] can/cannot [action] [when/with/in condition]`
@@ -28,6 +29,7 @@ application behavior.
   - Includes a comment with the step text before each step execution. Do not duplicate comments if step requires
     multiple actions.
   - Always use best practices from the log when generating tests.
+  - **Include resource cleanup in `afterEach`** for tests that create resources (sessions, endpoints, users)
 
 **Critical: Avoid Unnecessary Visibility Checks and Fallback Logic**
 - **DO NOT** create visibility check variables like `isSomethingVisible` with fallback logic
@@ -82,6 +84,60 @@ await page.goto('/dashboard');
 await page.waitForSelector('[data-testid="dashboard-loaded"]');
 ```
 
+**Guideline: Avoid Using `page.waitForTimeout()` for Polling or Readiness**
+- **Avoid `page.waitForTimeout()` for polling/readiness** - it often causes flaky tests and wastes time
+- Prefer Playwright's `expect.poll()` for status checking or element-based waiting
+- Use short `waitForTimeout()` delays only as a last resort for brief stabilization
+
+```typescript
+// ❌ BAD: Manual polling with waitForTimeout
+let status = await getStatus();
+while (status !== 'READY') {
+  await page.waitForTimeout(1000);
+  status = await getStatus();
+}
+
+// ✅ GOOD: Use expect.poll() for status waiting
+await expect.poll(async () => await getStatus(), { timeout: 60000 }).toBe('READY');
+```
+
+**Resource Cleanup Pattern:**
+- Tests that create resources (sessions, endpoints, users) MUST include cleanup
+- Prefer `afterEach` for isolated tests; use `afterAll` when a single expensive resource is shared across serial tests
+- Track created resources and clean them up appropriately
+
+```typescript
+// ✅ GOOD: Test with proper cleanup
+test.describe('Session Tests', { tag: ['@session'] }, () => {
+  let createdSessionName: string | null = null;
+
+  test.afterEach(async ({ page }) => {
+    if (createdSessionName) {
+      try {
+        const launcher = new SessionLauncher(page);
+        launcher.withSessionName(createdSessionName);
+        await launcher.terminate();
+      } catch { /* ignore cleanup errors */ }
+    }
+  });
+
+  test('User can create session', async ({ page }) => {
+    const launcher = new SessionLauncher(page);
+    await launcher.withSessionName('test-session').create();
+    createdSessionName = launcher.getSessionName();
+    // ... assertions
+  });
+});
+```
+
+**Project-Specific Conventions:**
+- Test files use `.spec.ts` extension (per `e2e/E2E-TEST-NAMING-GUIDELINES.md`)
+- Tests are organized in feature directories: `e2e/session/`, `e2e/serving/`, `e2e/user/`, etc.
+- POM classes are in `e2e/utils/classes/{feature}/`
+- Use `loginAsAdmin` or `loginAsUser` from `test-util.ts` for authentication
+- Use `navigateTo(page, 'route')` for navigation
+- Use tags for categorization: `@critical`, `@session`, `@serving`, `@functional`
+
    <example-generation>
    For following plan:
 

.claude/agents/playwright-test-healer.md

@@ -66,6 +66,39 @@ await page.goto('/dashboard');
 await page.waitForSelector('[data-testid="dashboard-loaded"]');
 ```
 
+**Guideline: Avoid Using `page.waitForTimeout()` for Polling or Readiness**
+- **Avoid `page.waitForTimeout()` for polling/readiness** - it often causes flaky tests and wastes time
+- When fixing tests, **remove** any manual polling loops with `waitForTimeout()` and prefer assertion-based waiting
+- Replace with Playwright's `expect.poll()` for status checking or element-based waiting
+- Use short `waitForTimeout()` delays only as a last resort for brief stabilization
+
+```typescript
+// ❌ BAD: Manual polling with waitForTimeout
+let status = await getSessionStatus(sessionId);
+while (status !== 'RUNNING') {
+  await page.waitForTimeout(1000);
+  status = await getSessionStatus(sessionId);
+}
+
+// ✅ GOOD: Use expect.poll() for status waiting
+await expect.poll(
+  async () => await sessionDetailPage.getSessionStatus(sessionId),
+  { message: `Waiting for session ${sessionId} to be RUNNING`, timeout: 120000 }
+).toBe('RUNNING');
+
+// ✅ GOOD: Use expect.poll() with error state handling
+await expect.poll(
+  async () => {
+    const currentStatus = await getSessionStatus(sessionId);
+    if (currentStatus === 'ERROR' || currentStatus === 'CANCELLED') {
+      throw new Error(`Session entered ${currentStatus} state`);
+    }
+    return currentStatus;
+  },
+  { timeout: 120000 }
+).toBe('RUNNING');
+```
+
 **Critical: Avoid Unnecessary Visibility Checks and Fallback Logic**
 - **DO NOT** create visibility check variables like `isSomethingVisible` with fallback logic
 - **DO NOT** wrap actions in try-catch blocks with alternative fallback paths to continue testing
@@ -196,4 +229,81 @@ await page.locator('svg[data-icon="upload"]').click();
 
 - **General utilities** (`e2e/utils/test-util.ts`):
   - Import appropriate utility based on UI framework being tested
-  - Prefer utility functions over direct CSS selectors for maintainability
+  - Prefer utility functions over direct CSS selectors for maintainability
+
+**Resource Cleanup Pattern:**
+- Tests that create resources (sessions, endpoints, users) MUST ensure cleanup
+- Prefer `afterEach` for isolated tests; use `afterAll` when a single expensive resource is shared across serial tests
+- Use `SessionLauncher.terminate()` for session cleanup - handles all states (PENDING, RUNNING, etc.)
+- Never assume a resource was successfully created - wrap cleanup in try-catch
+
+```typescript
+// ✅ GOOD: Proper cleanup pattern for session tests
+let createdSessionName: string | null = null;
+
+test.beforeEach(async ({ page, request }) => {
+  await loginAsUser(page, request);
+  createdSessionName = null;
+});
+
+test.afterEach(async ({ page }) => {
+  if (createdSessionName) {
+    try {
+      const sessionLauncher = new SessionLauncher(page);
+      sessionLauncher.withSessionName(createdSessionName);
+      await sessionLauncher.terminate();
+    } catch (error) {
+      console.log(`Failed to terminate session ${createdSessionName}:`, error);
+    }
+  }
+});
+
+test('Create session', async ({ page }) => {
+  const sessionLauncher = new SessionLauncher(page);
+  await sessionLauncher.withSessionName('test-session').create();
+  createdSessionName = sessionLauncher.getSessionName(); // Track for cleanup
+  // ... test assertions
+});
+```
+
+**Sequential Test Execution:**
+- Use `test.describe.configure({ mode: 'serial' })` when tests share resources or must run in order
+- Prevents parallel execution that causes resource contention
+
+```typescript
+test.describe('Session Lifecycle', () => {
+  test.describe.configure({ mode: 'serial' }); // Run tests sequentially
+  // ... tests
+});
+```
+
+**Vaadin Grid → Ant Design Table Migration:**
+- Many components have migrated from Vaadin Grid to Ant Design Table
+- Update locators accordingly when fixing tests
+
+```typescript
+// ❌ OLD: Vaadin Grid locators
+const row = page.locator('vaadin-grid-cell-content');
+const cell = sessionRow.locator('cell');
+
+// ✅ NEW: Ant Design Table locators
+const row = page.getByRole('row');
+const cell = sessionRow.getByRole('cell');
+```
+
+**Action Buttons in Notification Alerts:**
+- Some action buttons (terminate, app launch, terminal) appear in notification alerts, not table rows
+- Look for `getByRole('alert')` when table row buttons are not found
+
+```typescript
+// ✅ Finding action buttons in notification area
+const alert = page.getByRole('alert').filter({ hasText: sessionName });
+await alert.getByRole('button', { name: 'terminate' }).click();
+```
+
+**Project-Specific Conventions:**
+- Test files use `.spec.ts` extension (per `e2e/E2E-TEST-NAMING-GUIDELINES.md`)
+- Tests are organized in feature directories: `e2e/session/`, `e2e/serving/`, `e2e/user/`, etc.
+- POM classes are in `e2e/utils/classes/{feature}/`
+- Use `loginAsAdmin` or `loginAsUser` from `test-util.ts` for authentication
+- Use `navigateTo(page, 'route')` for navigation

.claude/agents/playwright-test-planner.md

@@ -100,6 +100,27 @@ application features:
 - Include negative testing scenarios
 - Ensure scenarios are independent and can be run in any order
 
+**Resource Cleanup Planning**:
+- When designing tests that create resources (sessions, endpoints, users), always plan for cleanup
+- Include `afterEach` cleanup step in test plan documentation
+- Consider test isolation - each test should not depend on previous test's state
+- For sequential tests that share state, note the dependency explicitly
+
+```markdown
+## Test Infrastructure Notes
+- **Cleanup Required**: Yes - sessions must be terminated after each test
+- **Execution Mode**: Sequential (tests share session state)
+- **Tags**: @critical, @session
+```
+
+**Sequential vs Parallel Execution**:
+- Most tests should be independent and run in parallel (default)
+- Use `test.describe.configure({ mode: 'serial' })` only when:
+  - Tests share expensive resources (e.g., session creation)
+  - Tests depend on state from previous tests
+  - Resource contention would cause flaky tests
+- Document execution mode requirements in the test plan
+
 **Test Design Principles - No Fallback Logic**:
 - **DO NOT** design tests with visibility checks and fallback logic
 - Each step should be direct and deterministic - if an element should be present, it must be present
@@ -138,4 +159,12 @@ application features:
 ```
 
 **Output Format**: Always save the complete test plan as a markdown file with clear headings, numbered steps, and
-professional formatting suitable for sharing with development and QA teams.
+professional formatting suitable for sharing with development and QA teams.
+
+**Project-Specific Conventions:**
+- Test files use `.spec.ts` extension (per `e2e/E2E-TEST-NAMING-GUIDELINES.md`)
+- Tests are organized in feature directories: `e2e/session/`, `e2e/serving/`, `e2e/user/`, etc.
+- POM classes are in `e2e/utils/classes/{feature}/`
+- Use tags for categorization: `@smoke`, `@critical`, `@regression`, `@session`, `@serving`, `@functional`
+- Plan for proper authentication: `loginAsAdmin` or `loginAsUser`
+- Plan for navigation: `navigateTo(page, 'route')`

.claude/commands/amend-staged-changes.md

@@ -28,22 +28,25 @@ If no argument is provided, PR description will only be updated if there are sta
 
 ## Detailed Process
 
-### Step 0: Verify MCP Authentication
+### Step 0: Verify MCP Authentication (MUST BE FIRST)
 
-Verify that MCP tools are authenticated before starting the workflow.
+> **⚠️ CRITICAL**: This step MUST be executed BEFORE any other operation. Do NOT skip or defer this step. Do NOT read files, check git status, or perform any other action until MCP authentication is verified.
+
+Verify that Atlassian MCP is authenticated before starting the workflow.
 
 ```
-# Test Graphite MCP authentication
-mcp__graphite__run_gt_cmd with args: ["--version"]
+# Test Atlassian MCP authentication - THIS MUST BE THE VERY FIRST TOOL CALL
+mcp__Atlassian__atlassianUserInfo
 ```
 
 **If authentication fails:**
-- Inform the user that Graphite MCP needs re-authentication
+- **STOP IMMEDIATELY** - Do not proceed with any other steps
+- Inform the user that Atlassian MCP needs re-authentication
 - Provide guidance on how to re-authenticate:
   ```
   ⚠️ MCP Authentication Required
 
-  Graphite MCP needs re-authentication.
+  Atlassian MCP needs re-authentication.
   Please re-authenticate via MCP settings and run the command again.
   ```
 - Exit the workflow without making any changes