cherry-pick(#37694): chore: move best practices into the journal

Author: pavelfeldman

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

@@ -24,6 +24,7 @@ application behavior.
   - Test title must match the scenario name
   - 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.
 
    <example-generation>
    For following plan:
@@ -55,16 +56,4 @@ application behavior.
      });
    });
    ```
-   </example-generation>
-
-# Best practices
-- Each test has clear, descriptive assertions that validate the expected behavior
-- Includes proper error handling and meaningful failure messages
-- Uses Playwright best practices (page.waitForLoadState, expect.toBeVisible, etc.)
-- Do not improvise, do not add directives that were not asked for
-- Uses reliable locators (preferring data-testid, role-based, or text-based selectors over fragile CSS selectors)
-- Uses local variables for locators that are used multiple times
-- Uses explicit waits rather than arbitrary timeouts
-- Never waits for networkidle or use other discouraged or deprecated apis
-- Is self-contained and can run independently
-- Is deterministic and not prone to flaky behavior
+   </example-generation>

examples/todomvc/.github/chatmodes/🎭 generator.chatmode.md

@@ -21,6 +21,7 @@ application behavior.
   - Test title must match the scenario name
   - 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.
 
    <example-generation>
    For following plan:
@@ -53,17 +54,5 @@ application behavior.
    });
    ```
    </example-generation>
-
-# Best practices
-- Each test has clear, descriptive assertions that validate the expected behavior
-- Includes proper error handling and meaningful failure messages
-- Uses Playwright best practices (page.waitForLoadState, expect.toBeVisible, etc.)
-- Do not improvise, do not add directives that were not asked for
-- Uses reliable locators (preferring data-testid, role-based, or text-based selectors over fragile CSS selectors)
-- Uses local variables for locators that are used multiple times
-- Uses explicit waits rather than arbitrary timeouts
-- Never waits for networkidle or use other discouraged or deprecated apis
-- Is self-contained and can run independently
-- Is deterministic and not prone to flaky behavior
 <example>Context: User wants to test a login flow on their web application. user: 'I need a test that logs into my app at localhost:3000 with username [email protected] and password 123456, then verifies the dashboard page loads' assistant: 'I'll use the generator agent to create and validate this login test for you' <commentary> The user needs a specific browser automation test created, which is exactly what the generator agent is designed for. </commentary></example>
 <example>Context: User has built a new checkout flow and wants to ensure it works correctly. user: 'Can you create a test that adds items to cart, proceeds to checkout, fills in payment details, and confirms the order?' assistant: 'I'll use the generator agent to build a comprehensive checkout flow test' <commentary> This is a complex user journey that needs to be automated and tested, perfect for the generator agent. </commentary></example>

packages/playwright/src/agents/generator.md

@@ -46,6 +46,7 @@ application behavior.
   - Test title must match the scenario name
   - 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.
 
    <example-generation>
    For following plan:
@@ -79,18 +80,6 @@ application behavior.
    ```
    </example-generation>
 
-# Best practices
-- Each test has clear, descriptive assertions that validate the expected behavior
-- Includes proper error handling and meaningful failure messages
-- Uses Playwright best practices (page.waitForLoadState, expect.toBeVisible, etc.)
-- Do not improvise, do not add directives that were not asked for
-- Uses reliable locators (preferring data-testid, role-based, or text-based selectors over fragile CSS selectors)
-- Uses local variables for locators that are used multiple times
-- Uses explicit waits rather than arbitrary timeouts
-- Never waits for networkidle or use other discouraged or deprecated apis
-- Is self-contained and can run independently
-- Is deterministic and not prone to flaky behavior
-
 <example>
   Context: User wants to test a login flow on their web application.
   user: 'I need a test that logs into my app at localhost:3000 with username [email protected] and password 123456, then

packages/playwright/src/mcp/test/testContext.ts

@@ -65,6 +65,7 @@ export class GeneratorJournal {
 \`\`\`ts
 ${step.code}
 \`\`\``).join('\n\n'));
+    result.push(bestPracticesMarkdown);
     return result.join('\n\n');
   }
 }
@@ -177,3 +178,16 @@ export class TestContext {
   async close() {
   }
 }
+
+const bestPracticesMarkdown = `
+# Best practices
+- Do not improvise, do not add directives that were not asked for
+- Use clear, descriptive assertions to validate the expected behavior
+- Use reliable locators from this log
+- Use local variables for locators that are used multiple times
+- Use Playwright waiting assertions and best practices from this log
+- NEVER! use page.waitForLoadState()
+- NEVER! use page.waitForNavigation()
+- NEVER! use page.waitForTimeout()
+- NEVER! use page.evaluate()
+`;

tests/mcp/generator.spec.ts

@@ -101,7 +101,7 @@ test('generator_setup_page', async ({ startClient }) => {
   expect(await client.callTool({
     name: 'generator_read_log',
     arguments: {},
-  })).toHaveTextResponse(`# Plan
+  })).toHaveTextResponse(expect.stringContaining(`# Plan
 
 Test plan
 
@@ -125,7 +125,11 @@ Test plan
 ### Click submit button
 \`\`\`ts
 await page.getByRole('button', { name: 'Submit' }).click();
-\`\`\``);
+\`\`\`
+
+
+# Best practices
+`));
 });
 
 test('click after generator_log_action', async ({ startClient }) => {
@@ -164,7 +168,7 @@ test('click after generator_log_action', async ({ startClient }) => {
   expect(await client.callTool({
     name: 'generator_read_log',
     arguments: {},
-  })).toHaveTextResponse(`# Plan
+  })).toHaveTextResponse(expect.stringContaining(`# Plan
 
 Test plan
 
@@ -188,7 +192,11 @@ Test plan
 ### Click submit button
 \`\`\`ts
 await page.getByRole('button', { name: 'Submit' }).click();
-\`\`\``);
+\`\`\`
+
+
+# Best practices
+`));
 });
 
 test('generator_setup_page is required', async ({ startClient }) => {