docs(e2e): refine Playwright and Cucumber skill guidance (#41617)

.agents/skills/e2e-cucumber-playwright/SKILL.md

@@ -7,12 +7,14 @@ description: Use when writing, changing, or reviewing Cucumber and Playwright te
 
 `e2e/AGENTS.md` owns the suite architecture, lifecycle, commands, tags, generated-client boundaries, fixtures, and cleanup contracts. Read the nearest feature-scoped `AGENTS.md` when one exists. This skill adds no parallel package policy.
 
+Cucumber owns scenario and hook execution and reporting. Playwright provides browser automation, contexts, pages, locators, actions, assertions, request, and tracing APIs; protocol exceptions remain with the package-defined owner. Their timeout domains remain separate, and importing from `@playwright/test` does not make Playwright Test runner configuration, fixtures, or retries applicable to this suite.
+
 ## Topic Routing
 
 Read only the bundled reference required by the change:
 
 - Locator, assertion, isolation, or waiting decisions: [`references/playwright-best-practices.md`][playwright]
-- Scenario wording, step granularity, expressions, or tag design: [`references/cucumber-best-practices.md`][cucumber]
+- Scenario wording, step granularity, expressions, World state, hook visibility, or tag design: [`references/cucumber-best-practices.md`][cucumber]
 
 Check current official Playwright or Cucumber documentation before introducing a framework pattern that local code and references do not already establish.
 
@@ -23,7 +25,7 @@ Check current official Playwright or Cucumber documentation before introducing a
 3. Read the target scenario, matching step definitions, and lifecycle files only when session or shared state matters.
 4. Reuse an existing step when wording and behavior match; add one coherent scenario or step when they do not.
 5. Keep browser actions and assertions at the public user boundary; keep setup, seed, polling, and cleanup at their package-defined owners.
-6. Run the narrowest tagged scenario and package checks documented in `e2e/AGENTS.md`; broaden only for shared hooks, tags, or support changes.
+6. For changes, run the narrowest tagged scenario and package checks documented in `e2e/AGENTS.md`; broaden only for shared hooks, tags, or support changes.
 
 For review requests, lead with reproducible correctness failures, flake sources, or demonstrated architecture drift. Report the behavior verified and any external-runtime, browser, or environment gap.
 

.agents/skills/e2e-cucumber-playwright/references/cucumber-best-practices.md

@@ -4,15 +4,18 @@ Use this reference when writing or reviewing Gherkin scenarios, step definitions
 
 Official sources:
 
-- https://cucumber.io/docs/guides/10-minute-tutorial
-- https://cucumber.io/docs/cucumber/step-definitions
+- https://cucumber.io/docs/bdd/better-gherkin
+- https://cucumber.io/docs/gherkin/reference
+- https://cucumber.io/docs/gherkin/step-organization
+- https://cucumber.io/docs/cucumber/state
 - https://cucumber.io/docs/cucumber/cucumber-expressions
+- https://github.com/cucumber/cucumber-js/blob/main/docs/support_files/world.md
 
 ## What Matters Most
 
 ### 1. Treat scenarios as executable specifications
 
-Cucumber scenarios should describe examples of behavior, not test implementation recipes.
+Cucumber scenarios should usually describe behavior declaratively, not reproduce an interaction script.
 
 Apply it like this:
 
@@ -20,11 +23,15 @@ Apply it like this:
 - avoid UI-internal wording such as selector details, DOM structure, or component names
 - keep language concrete enough that the scenario reads like living documentation
 
+Procedural detail is appropriate when the interaction mechanism is the behavior under test, such as keyboard navigation, focus movement, or a required multi-actor sequence.
+
 ### 2. Keep scenarios focused
 
-A scenario should usually prove one workflow or business outcome. If a scenario wanders across several unrelated behaviors, split it.
+A scenario should prove one business rule or coherent outcome. Cucumber's three-to-five-step recommendation is a review heuristic, not a hard limit: when a scenario is longer, check for multiple outcomes, incidental setup, or UI procedure that can move behind a domain step.
 
-Keep each scenario centered on one coherent outcome. Avoid hidden dependencies on another scenario's side effects, and keep unavoidable setup outside the behavior narrative unless the precondition matters to the specification.
+Use `Given` for initial context, `When` for the event, and `Then` for the expected outcome. Restarting those phases is a prompt to review the narrative, not an automatic failure. Avoid hidden dependencies on another scenario's side effects.
+
+Use `Rule` when several examples illustrate one named business rule. Use a short `Background` only for shared context the reader needs to understand; do not hide fixture preparation, runtime readiness, or long setup there.
 
 ### 3. Reuse steps, but only when behavior really matches
 
@@ -45,6 +52,8 @@ Write a new step when:
 
 Do not optimize for a low step count by making vague steps. Optimize for a small set of truthful, domain-owned steps.
 
+Step definitions share one global matching namespace regardless of their directory. Organize them by domain capability and avoid feature-coupled glue or expressions broad enough to overlap unrelated behavior.
+
 ### 4. Prefer Cucumber Expressions
 
 Use Cucumber Expressions for parameters unless regex is clearly necessary.
@@ -62,18 +71,24 @@ Use regex for a bounded natural-language alternative only when it keeps Gherkin
 
 ### 5. Keep step definitions thin and meaningful
 
-Step definitions are glue between Gherkin and automation, not a second abstraction language.
+Step definitions are glue between Gherkin and automation, not a second abstraction language. A step should express one coherent domain action or outcome; it may call several implementation helpers when that keeps UI mechanics out of the specification.
 
-Keep each step to one user-visible action or assertion. In JavaScript and TypeScript, use `async function` when the step reads Cucumber World state because Cucumber binds `this`; do not leak state across scenarios through module globals.
+Cucumber creates a new World for each scenario. Keep scenario state there rather than in module globals. Cucumber.js supports a `world` proxy for arrow functions, but this suite consistently uses typed `async function (this: DifyWorld, ...)` so ownership and TypeScript discovery remain explicit.
+
+Hooks are invisible to feature readers. Keep them for low-level browser lifecycle, cleanup, and diagnostics; express business-relevant context in Gherkin.
 
 ### 6. Use tags intentionally
 
-Tags should communicate selection or execution intent, not become ad hoc metadata. A tag does not change runtime behavior unless configuration or hooks implement it.
+Tags may communicate stable capability grouping, selection, fixture dependency, or conditional-hook intent. A tag changes runtime behavior only when a runner, seed profile, or hook owns that meaning.
 
 ## Review Questions
 
 - Does the scenario read like a real example of product behavior?
+- Does it prove one outcome, or combine several independent phases?
 - Are the steps behavior-oriented instead of implementation-oriented?
+- Is procedural wording necessary to the behavior being tested?
 - Is a reused step still truthful in this feature?
+- Could a new expression overlap an existing step in the global namespace?
+- Would `Rule` clarify the business rule, or would `Background` hide important setup?
 - Is a new tag documenting real behavior, or inventing semantics that the suite does not implement?
 - Would a new reader understand the outcome without opening the step-definition file?

.agents/skills/e2e-cucumber-playwright/references/playwright-best-practices.md

@@ -6,8 +6,12 @@ Official sources:
 
 - https://playwright.dev/docs/best-practices
 - https://playwright.dev/docs/locators
+- https://playwright.dev/docs/actionability
 - https://playwright.dev/docs/test-assertions
+- https://playwright.dev/docs/test-timeouts
 - https://playwright.dev/docs/browser-contexts
+- https://playwright.dev/docs/events
+- https://playwright.dev/docs/trace-viewer
 
 ## What Matters Most
 
@@ -21,28 +25,22 @@ Apply it like this:
 - keep scenario state in the runner's scenario-owned context rather than module globals
 - model special authentication or session setup through explicit per-scenario fixtures rather than shared mutable state
 
-### 2. Prefer user-facing locators
+### 2. Select locators by user contract
 
-Playwright recommends built-in locators that reflect what users perceive on the page.
+Prefer built-in locators that match how the target is exposed to users:
 
-Preferred order:
+- interactive controls: role and accessible name
+- form controls: associated label; use placeholder when it is the relevant stable contract, especially when no label exists
+- non-interactive content: visible text or the relevant text alternative
+- elements without a meaningful user-facing contract: an intentional test id
 
-1. `getByRole`
-2. `getByLabel`
-3. `getByPlaceholder`
-4. `getByText`
-5. `getByTestId` when an explicit test contract is the most stable option
+This is a semantic choice, not a fixed ranking. Do not add incorrect roles or accessible names solely to satisfy a locator. When the product element should have user-facing semantics, fix that contract instead.
 
 Avoid raw CSS/XPath selectors unless no stable user-facing contract exists and adding one is not practical.
 
-Also remember:
+Locators are strict for single-element actions. Scope to a stable region or use `filter({ has, hasText })` to make the intended target unique. Treat `.first()`, `.last()`, or `.nth()` as a review signal: the positional choice should be intentional and stable, not merely silence ambiguity.
 
-- repeated content usually needs scoping to a stable container
-- exact text matching is often too brittle when role/name or label already exists
-- `getByTestId` is acceptable when semantics are weak but the contract is intentional
-- when a real UI region, card, status, or icon lacks an accessible name, prefer adding that semantic contract in product code before falling back to `getByTestId`
-
-### 3. Use web-first assertions
+### 3. Use web-first assertions with the right timeout owner
 
 Playwright assertions auto-wait and retry. Prefer them over manual state inspection.
 
@@ -64,6 +62,8 @@ If a condition genuinely needs custom retry logic, use Playwright's polling/asse
 
 Use `expect.poll` for non-DOM truth such as API state, backend eventual consistency, generated resources, or captured browser events. For DOM state, use locator assertions so Playwright can apply actionability and web-first retry semantics.
 
+Cucumber step and hook timeouts, Playwright locator/action timeouts, and Playwright assertion timeouts are separate budgets. `browserContext.setDefaultTimeout()` does not change the default five-second assertion timeout. Put an explicit longer assertion timeout only on the readiness owner that needs it; do not raise an outer timeout to mask a shorter inner failure.
+
 ### 4. Let actions wait for actionability
 
 Locator actions already wait for the element to be actionable. Do not preface every click/fill with extra timing logic unless the action needs a specific visible/ready assertion for clarity.
@@ -77,15 +77,20 @@ Bad pattern:
 
 - stack arbitrary waits before every action
 - wait on unstable implementation details instead of the visible state the user cares about
+- use `force: true` to bypass a real hit-target, overlay, or disabled-state failure
+
+When synchronizing on a one-shot popup, download, request, or response, create the wait before triggering the action, then await the event. A scenario-owned listener may instead capture events for a later assertion. Do not use `networkidle` as an application-readiness assertion; wait for the user-visible state or owned backend contract.
 
 ### 5. Match debugging to the active harness
 
-Playwright supports traces, screenshots, page snapshots, and browser logs. Configure artifact capture at the runner boundary instead of adding parallel diagnostics to individual scenarios.
+Playwright supports traces, screenshots, page snapshots, and browser logs. Configure artifact capture in Cucumber hooks instead of adding parallel diagnostics to individual scenarios. If tracing is introduced, use `browserContext.tracing`; Playwright Test options such as `trace: 'on-first-retry'`, projects, workers, fixtures, reporters, and retries do not configure this harness.
 
 ## Review Questions
 
 - Would this locator survive DOM refactors that do not change user-visible behavior?
+- Is a positional locator expressing product order, or hiding an ambiguous match?
 - Is this assertion using Playwright's retrying semantics?
-- Is any explicit wait masking a real readiness problem?
+- Does an explicit timeout belong to the condition that is actually slow?
+- Was an event wait registered before the action that emits it?
 - Does this code preserve per-scenario isolation?
 - Is a new abstraction really needed, or does it bypass the runner's scenario-owned context and lifecycle?