Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Yang <yang@Yangs-MacBook-Pro.local>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
When a GitHub user's profile email is null (hidden/private), the OAuth callback fails with HTTP 400 because `GitHubRawUserInfo` validates `email` as a required non-null string. Even after the type was relaxed to `NotRequired[str | None]` in #33882, the flow still raises a `ValueError` when no email can be resolved, blocking sign-in entirely.
This PR improves the email resolution strategy so that users with private GitHub emails can still sign in.
The current frontend implementation closes the connection once `workflow_paused` SSE event is received and establish a new connection to subscribe new events. The implementation of `StreamsBroadcastChannel` sets initial `_last_id` to `0-0`, consumes streams from start and send `workflow_paused` event created before pauses to frontend, causing excessive connections being established.
This PR fixes the issue by setting initial id to `$`, which means only new messages are received by the subscription.
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR:
1. Fixes the bug that email body of `HumanInput` node are sent as-is, without markdown rendering or sanitization
2. Applies HTML sanitization to email subject and body
3. Removes `\r` and `\n` from email subject to prevent SMTP header injection
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: QuantumGhost <obelisk.reg+git@gmail.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
description: Review backend code for quality, security, maintainability, and best practices based on established checklist rules. Use when the user requests a review, analysis, or improvement of backend files (e.g., `.py`) under the `api/` directory. Do NOT use for frontend files (e.g., `.tsx`, `.ts`, `.js`). Supports pending-change review, code snippets review, and file-focused review.
---
# Backend Code Review
## When to use this skill
Use this skill whenever the user asks to **review, analyze, or improve** backend code (e.g., `.py`) under the `api/` directory. Supports the following review modes:
- **Pending-change review**: when the user asks to review current changes (inspect staged/working-tree files slated for commit to get the changes).
- **Code snippets review**: when the user pastes code snippets (e.g., a function/class/module excerpt) into the chat and asks for a review.
- **File-focused review**: when the user points to specific files and asks for a review of those files (one file or a small, explicit set of files, e.g., `api/...`, `api/app.py`).
Do NOT use this skill when:
- The request is about frontend code or UI (e.g., `.tsx`, `.ts`, `.js`, `web/`).
- The user is not asking for a review/analysis/improvement of backend code.
- The scope is not under `api/` (unless the user explicitly asks to review backend-related changes outside `api/`).
## How to use this skill
Follow these steps when using this skill:
1.**Identify the review mode** (pending-change vs snippet vs file-focused) based on the user’s input. Keep the scope tight: review only what the user provided or explicitly referenced.
2. Follow the rules defined in **Checklist** to perform the review. If no Checklist rule matches, apply **General Review Rules** as a fallback to perform the best-effort review.
3. Compose the final output strictly follow the **Required Output Format**.
Notes when using this skill:
- Always include actionable fixes or suggestions (including possible code snippets).
- Use best-effort `File:Line` references when a file path and line numbers are available; otherwise, use the most specific identifier you can.
## Checklist
- db schema design: if the review scope includes code/files under `api/models/` or `api/migrations/`, follow [references/db-schema-rule.md](references/db-schema-rule.md) to perform the review
- architecture: if the review scope involves controller/service/core-domain/libs/model layering, dependency direction, or moving responsibilities across modules, follow [references/architecture-rule.md](references/architecture-rule.md) to perform the review
- repositories abstraction: if the review scope contains table/model operations (e.g., `select(...)`, `session.execute(...)`, joins, CRUD) and is not under `api/repositories`, `api/core/repositories`, or `api/extensions/*/repositories/`, follow [references/repositories-rule.md](references/repositories-rule.md) to perform the review
- sqlalchemy patterns: if the review scope involves SQLAlchemy session/query usage, db transaction/crud usage, or raw SQL usage, follow [references/sqlalchemy-rule.md](references/sqlalchemy-rule.md) to perform the review
## General Review Rules
### 1. Security Review
Check for:
- SQL injection vulnerabilities
- Server-Side Request Forgery (SSRF)
- Command injection
- Insecure deserialization
- Hardcoded secrets/credentials
- Improper authentication/authorization
- Insecure direct object references
### 2. Performance Review
Check for:
- N+1 queries
- Missing database indexes
- Memory leaks
- Blocking operations in async code
- Missing caching opportunities
### 3. Code Quality Review
Check for:
- Code forward compatibility
- Code duplication (DRY violations)
- Functions doing too much (SRP violations)
- Deep nesting / complex conditionals
- Magic numbers/strings
- Poor naming
- Missing error handling
- Incomplete type coverage
### 4. Testing Review
Check for:
- Missing test coverage for new code
- Tests that don't test behavior
- Flaky test patterns
- Missing edge cases
## Required Output Format
When this skill invoked, the response must exactly follow one of the two templates:
### Template A (any findings)
```markdown
# Code Review Summary
Found <X> critical issues need to be fixed:
## 🔴 Critical (Must Fix)
### 1. <brief description of the issue>
FilePath: <path> line <line>
<relevantcodesnippetorpointer>
#### Explanation
<detailedexplanationandreferencesoftheissue>
#### Suggested Fix
1.<briefdescriptionofsuggestedfix>
2.<codeexample> (optional, omit if not applicable)
---
... (repeat for each critical issue) ...
Found <Y> suggestions for improvement:
## 🟡 Suggestions (Should Consider)
### 1. <brief description of the suggestion>
FilePath: <path> line <line>
<relevantcodesnippetorpointer>
#### Explanation
<detailedexplanationandreferencesofthesuggestion>
#### Suggested Fix
1.<briefdescriptionofsuggestedfix>
2.<codeexample> (optional, omit if not applicable)
---
... (repeat for each suggestion) ...
Found <Z> optional nits:
## 🟢 Nits (Optional)
### 1. <brief description of the nit>
FilePath: <path> line <line>
<relevantcodesnippetorpointer>
#### Explanation
<explanationandreferencesoftheoptionalnit>
#### Suggested Fix
-<minorsuggestions>
---
... (repeat for each nits) ...
## ✅ What's Good
-<Positivefeedbackongoodpatterns>
```
- If there are no critical issues or suggestions or option nits or good points, just omit that section.
- If the issue number is more than 10, summarize as "Found 10+ critical issues/suggestions/optional nits" and only output the first 10 items.
- Don't compress the blank lines between sections; keep them as-is for readability.
- If there is any issue requires code changes, append a brief follow-up question to ask whether the user wants to apply the fix(es) after the structured output. For example: "Would you like me to use the Suggested fix(es) to address these issues?"
- Description: Controllers should parse input, call services, and return serialized responses. Business decisions inside controllers make behavior hard to reuse and test.
- Suggested fix: Move domain/business logic into the service or core/domain layer. Keep controller handlers thin and orchestration-focused.
- Example:
- Bad:
```python
@bp.post("/apps/<app_id>/publish")
def publish_app(app_id: str):
payload = request.get_json() or {}
if payload.get("force") and current_user.role != "admin":
raise ValueError("only admin can force publish")
app = App.query.get(app_id)
app.status = "published"
db.session.commit()
return {"result": "ok"}
```
- Good:
```python
@bp.post("/apps/<app_id>/publish")
def publish_app(app_id: str):
payload = PublishRequest.model_validate(request.get_json() or {})
- Description: Controllers may depend on services, and services may depend on core/domain abstractions. Reversing this direction (for example, core importing controller/web modules) creates cycles and leaks transport concerns into domain code.
- Suggested fix: Extract shared contracts into core/domain or service-level modules and make upper layers depend on lower, not the reverse.
- Example:
- Bad:
```python
# core/policy/publish_policy.py
from controllers.console.app import request_context
def can_publish() -> bool:
return request_context.current_user.is_admin
```
- Good:
```python
# core/policy/publish_policy.py
def can_publish(role: str) -> bool:
return role == "admin"
# service layer adapts web/user context to domain input
allowed = can_publish(role=current_user.role)
```
### Keep libs business-agnostic
- Category: maintainability
- Severity: critical
- Description: Modules under `api/libs/` should remain reusable, business-agnostic building blocks. They must not encode product/domain-specific rules, workflow orchestration, or business decisions.
- Suggested fix:
- If business logic appears in `api/libs/`, extract it into the appropriate `services/` or `core/` module and keep `libs` focused on generic, cross-cutting helpers.
- Covers: model/base inheritance, schema boundaries in model properties, tenant-aware schema design, index redundancy checks, dialect portability in models, and cross-database compatibility in migrations.
- Does NOT cover: session lifecycle, transaction boundaries, and query execution patterns (handled by `sqlalchemy-rule.md`).
## Rules
### Do not query other tables inside `@property`
- Category: [maintainability, performance]
- Severity: critical
- Description: A model `@property` must not open sessions or query other tables. This hides dependencies across models, tightly couples schema objects to data access, and can cause N+1 query explosions when iterating collections.
- Suggested fix:
- Keep model properties pure and local to already-loaded fields.
- Move cross-table data fetching to service/repository methods.
- For list/batch reads, fetch required related data explicitly (join/preload/bulk query) before rendering derived values.
- Example:
- Bad:
```python
class Conversation(TypeBase):
__tablename__ = "conversations"
@property
def app_name(self) -> str:
with Session(db.engine, expire_on_commit=False) as session:
# Service/repository layer performs explicit batch fetch for related App rows.
```
### Prefer including `tenant_id` in model definitions
- Category: maintainability
- Severity: suggestion
- Description: In multi-tenant domains, include `tenant_id` in schema definitions whenever the entity belongs to tenant-owned data. This improves data isolation safety and keeps future partitioning/sharding strategies practical as data volume grows.
- Suggested fix:
- Add a `tenant_id` column and ensure related unique/index constraints include tenant dimension when applicable.
- Propagate `tenant_id` through service/repository contracts to keep access paths tenant-aware.
- Exception: if a table is explicitly designed as non-tenant-scoped global metadata, document that design decision clearly.
- Description: Review index definitions for leftmost-prefix redundancy. For example, index `(a, b, c)` can safely cover most lookups for `(a, b)`. Keeping both may increase write overhead and can mislead the optimizer into suboptimal execution plans.
- Suggested fix:
- Before adding an index, compare against existing composite indexes by leftmost-prefix rules.
- Drop or avoid creating redundant prefixes unless there is a proven query-pattern need.
- Apply the same review standard in both model `__table_args__` and migration index DDL.
### Avoid PostgreSQL-only dialect usage in models; wrap in `models.types`
- Category: maintainability
- Severity: critical
- Description: Model/schema definitions should avoid PostgreSQL-only constructs directly in business models. When database-specific behavior is required, encapsulate it in `api/models/types.py` using both PostgreSQL and MySQL dialect implementations, then consume that abstraction from model code.
- Suggested fix:
- Do not directly place dialect-only types/operators in model columns when a portable wrapper can be used.
- Add or extend wrappers in `models.types` (for example, `AdjustedJSON`, `LongText`, `BinaryData`) to normalize behavior across PostgreSQL and MySQL.
### Guard migration incompatibilities with dialect checks and shared types
- Category: maintainability
- Severity: critical
- Description: Migration scripts under `api/migrations/versions/` must account for PostgreSQL/MySQL incompatibilities explicitly. For dialect-sensitive DDL or defaults, branch on the active dialect (for example, `conn.dialect.name == "postgresql"`), and prefer reusable compatibility abstractions from `models.types` where applicable.
- Suggested fix:
- In migration upgrades/downgrades, bind connection and branch by dialect for incompatible SQL fragments.
- Reuse `models.types` wrappers in column definitions when that keeps behavior aligned with runtime models.
- Avoid one-dialect-only migration logic unless there is a documented, deliberate compatibility exception.
- Example:
- Bad:
```python
with op.batch_alter_table("dataset_keyword_tables") as batch_op:
- Covers: when to reuse existing repository abstractions, when to introduce new repositories, and how to preserve dependency direction between service/core and infrastructure implementations.
- Does NOT cover: SQLAlchemy session lifecycle and query-shape specifics (handled by `sqlalchemy-rule.md`), and table schema/migration design (handled by `db-schema-rule.md`).
## Rules
### Introduce repositories abstraction
- Category: maintainability
- Severity: suggestion
- Description: If a table/model already has a repository abstraction, all reads/writes/queries for that table should use the existing repository. If no repository exists, introduce one only when complexity justifies it, such as large/high-volume tables, repeated complex query logic, or likely storage-strategy variation.
- Suggested fix:
- First check `api/repositories`, `api/core/repositories`, and `api/extensions/*/repositories/` to verify whether the table/model already has a repository abstraction. If it exists, route all operations through it and add missing repository methods instead of bypassing it with ad-hoc SQLAlchemy access.
- If no repository exists, add one only when complexity warrants it (for example, repeated complex queries, large data domains, or multiple storage strategies), while preserving dependency direction (service/core depends on abstraction; infra provides implementation).
- Example:
- Bad:
```python
# Existing repository is ignored and service uses ad-hoc table queries.
- Covers: SQLAlchemy session and transaction lifecycle, query construction, tenant scoping, raw SQL boundaries, and write-path concurrency safeguards.
- Does NOT cover: table/model schema and migration design details (handled by `db-schema-rule.md`).
## Rules
### Use Session context manager with explicit transaction control behavior
- Category: best practices
- Severity: critical
- Description: Session and transaction lifecycle must be explicit and bounded on write paths. Missing commits can silently drop intended updates, while ad-hoc or long-lived transactions increase contention, lock duration, and deadlock risk.
- Suggested fix:
- Use **explicit `session.commit()`** after completing a related write unit.
- Or use **`session.begin()` context manager** for automatic commit/rollback on a scoped block.
- Keep transaction windows short: avoid network I/O, heavy computation, or unrelated work inside the transaction.
- Example:
- Bad:
```python
# Missing commit: write may never be persisted.
with Session(db.engine, expire_on_commit=False) as session:
run = session.get(WorkflowRun, run_id)
run.status = "cancelled"
# Long transaction: external I/O inside a DB transaction.
with Session(db.engine, expire_on_commit=False) as session, session.begin():
run = session.get(WorkflowRun, run_id)
run.status = "cancelled"
call_external_api()
```
- Good:
```python
# Option 1: explicit commit.
with Session(db.engine, expire_on_commit=False) as session:
run = session.get(WorkflowRun, run_id)
run.status = "cancelled"
session.commit()
# Option 2: scoped transaction with automatic commit/rollback.
with Session(db.engine, expire_on_commit=False) as session, session.begin():
run = session.get(WorkflowRun, run_id)
run.status = "cancelled"
# Keep non-DB work outside transaction scope.
call_external_api()
```
### Enforce tenant_id scoping on shared-resource queries
- Category: security
- Severity: critical
- Description: Reads and writes against shared tables must be scoped by `tenant_id` to prevent cross-tenant data leakage or corruption.
- Suggested fix: Add `tenant_id` predicate to all tenant-owned entity queries and propagate tenant context through service/repository interfaces.
### Prefer SQLAlchemy expressions over raw SQL by default
- Category: maintainability
- Severity: suggestion
- Description: Raw SQL should be exceptional. ORM/Core expressions are easier to evolve, safer to compose, and more consistent with the codebase.
- Suggested fix: Rewrite straightforward raw SQL into SQLAlchemy `select/update/delete` expressions; keep raw SQL only when required by clear technical constraints.
- Example:
- Bad:
```python
row = session.execute(
text("SELECT * FROM workflows WHERE id = :id AND tenant_id = :tenant_id"),
{"id": workflow_id, "tenant_id": tenant_id},
).first()
```
- Good:
```python
stmt = select(Workflow).where(
Workflow.id == workflow_id,
Workflow.tenant_id == tenant_id,
)
row = session.execute(stmt).scalar_one_or_none()
```
### Protect write paths with concurrency safeguards
- Category: quality
- Severity: critical
- Description: Multi-writer paths without explicit concurrency control can silently overwrite data. Choose the safeguard based on contention level, lock scope, and throughput cost instead of defaulting to one strategy.
- Suggested fix:
- **Optimistic locking**: Use when contention is usually low and retries are acceptable. Add a version (or updated_at) guard in `WHERE` and treat `rowcount == 0` as a conflict.
- **Redis distributed lock**: Use when the critical section spans multiple steps/processes (or includes non-DB side effects) and you need cross-worker mutual exclusion.
- **SELECT ... FOR UPDATE**: Use when contention is high on the same rows and strict in-transaction serialization is required. Keep transactions short to reduce lock wait/deadlock risk.
- In all cases, scope by `tenant_id` and verify affected row counts for conditional writes.
- Example:
- Bad:
```python
# No tenant scope, no conflict detection, and no lock on a contested write path.
- This skill is for component decomposition, not query/mutation design.
- Do not introduce deprecated `useInvalid` / `useReset`.
- Do not add thin passthrough `useQuery` wrappers during refactoring; only extract a custom hook when it truly orchestrates multiple queries/mutations or shared derived state.
**Dify Examples**:
-`web/service/use-workflow.ts`
@ -408,7 +365,7 @@ For each extraction:
┌────────────────────────────────────────┐
│ 1. Extract code │
│ 2. Run: pnpm lint:fix │
│ 3. Run: pnpm type-check:tsgo │
│ 3. Run: pnpm type-check │
│ 4. Run: pnpm test │
│ 5. Test functionality manually │
│ 6. PASS? → Next extraction │
@ -480,4 +437,4 @@ const useButtonState = () => {
### Related Skills
-`frontend-testing` - For testing refactored components
description: Write, update, or review Dify end-to-end tests under `e2e/` that use Cucumber, Gherkin, and Playwright. Use when the task involves `.feature` files, `features/step-definitions/`, `features/support/`, `DifyWorld`, scenario tags, locator/assertion choices, or E2E testing best practices for this repository.
---
# Dify E2E Cucumber + Playwright
Use this skill for Dify's repository-level E2E suite in `e2e/`. Use [`e2e/AGENTS.md`](../../../e2e/AGENTS.md) as the canonical guide for local architecture and conventions, then apply Playwright/Cucumber best practices only where they fit the current suite.
## Scope
- Use this skill for `.feature` files, Cucumber step definitions, `DifyWorld`, hooks, tags, and E2E review work under `e2e/`.
- Do not use this skill for Vitest or React Testing Library work under `web/`; use `frontend-testing` instead.
- Do not use this skill for backend test or API review tasks under `api/`.
2. Read only the files directly involved in the task:
- target `.feature` files under `e2e/features/`
- related step files under `e2e/features/step-definitions/`
-`e2e/features/support/hooks.ts` and `e2e/features/support/world.ts` when session lifecycle or shared state matters
-`e2e/scripts/run-cucumber.ts` and `e2e/cucumber.config.ts` when tags or execution flow matter
3. Read [`references/playwright-best-practices.md`](references/playwright-best-practices.md) only when locator, assertion, isolation, or waiting choices are involved.
4. Read [`references/cucumber-best-practices.md`](references/cucumber-best-practices.md) only when scenario wording, step granularity, tags, or expression design are involved.
5. Re-check official Playwright or Cucumber docs with the available documentation tools before introducing a new framework pattern.
## Local Rules
-`e2e/` uses Cucumber for scenarios and Playwright as the browser layer.
-`DifyWorld` is the per-scenario context object. Type `this` as `DifyWorld` and use `async function`, not arrow functions.
- Keep glue organized by capability under `e2e/features/step-definitions/`; use `common/` only for broadly reusable steps.
- Browser session behavior comes from `features/support/hooks.ts`:
- default: authenticated session with shared storage state
-`@unauthenticated`: clean browser context
-`@authenticated`: readability/selective-run tag only unless implementation changes
-`@fresh`: only for `e2e:full*` flows
- Do not import Playwright Test runner patterns that bypass the current Cucumber + `DifyWorld` architecture unless the task is explicitly about changing that architecture.
## Workflow
1. Rebuild local context.
- Inspect the target feature area.
- Reuse an existing step when wording and behavior already match.
- Add a new step only for a genuinely new user action or assertion.
- Keep edits close to the current capability folder unless the step is broadly reusable.
2. Write behavior-first scenarios.
- Describe user-observable behavior, not DOM mechanics.
- Keep each scenario focused on one workflow or outcome.
- Keep scenarios independent and re-runnable.
3. Write step definitions in the local style.
- Keep one step to one user-visible action or one assertion.
- Prefer Cucumber Expressions such as `{string}` and `{int}`.
- Scope locators to stable containers when the page has repeated elements.
- Avoid page-object layers or extra helper abstractions unless repeated complexity clearly justifies them.
4. Use Playwright in the local style.
- Prefer user-facing locators: `getByRole`, `getByLabel`, `getByPlaceholder`, `getByText`, then `getByTestId` for explicit contracts.
- Use web-first `expect(...)` assertions.
- Do not use `waitForTimeout`, manual polling, or raw visibility checks when a locator action or retrying assertion already expresses the behavior.
5. Validate narrowly.
- Run the narrowest tagged scenario or flow that exercises the change.
- Run `pnpm -C e2e check`.
- Broaden verification only when the change affects hooks, tags, setup, or shared step semantics.
## Review Checklist
- Does the scenario describe behavior rather than implementation?
- Does it fit the current session model, tags, and `DifyWorld` usage?
- Should an existing step be reused instead of adding a new one?
- Are locators user-facing and assertions web-first?
- Does the change introduce hidden coupling across scenarios, tags, or instance state?
- Does it document or implement behavior that differs from the real hooks or configuration?
Lead findings with correctness, flake risk, and architecture drift.
Use this reference when writing or reviewing locator, assertion, isolation, or synchronization logic for Dify's Cucumber-based E2E suite.
Official sources:
- https://playwright.dev/docs/best-practices
- https://playwright.dev/docs/locators
- https://playwright.dev/docs/test-assertions
- https://playwright.dev/docs/browser-contexts
## What Matters Most
### 1. Keep scenarios isolated
Playwright's model is built around clean browser contexts so one test does not leak into another. In Dify's suite, that principle maps to per-scenario session setup in `features/support/hooks.ts` and `DifyWorld`.
Apply it like this:
- do not depend on another scenario having run first
- do not persist ad hoc scenario state outside `DifyWorld`
- do not couple ordinary scenarios to `@fresh` behavior
- when a flow needs special auth/session semantics, express that through the existing tag model or explicit hook changes
### 2. Prefer user-facing locators
Playwright recommends built-in locators that reflect what users perceive on the page.
Preferred order in this repository:
1.`getByRole`
2.`getByLabel`
3.`getByPlaceholder`
4.`getByText`
5.`getByTestId` when an explicit test contract is the most stable option
Avoid raw CSS/XPath selectors unless no stable user-facing contract exists and adding one is not practical.
Also remember:
- 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
### 3. Use web-first assertions
Playwright assertions auto-wait and retry. Prefer them over manual state inspection.
Prefer:
-`await expect(page).toHaveURL(...)`
-`await expect(locator).toBeVisible()`
-`await expect(locator).toBeHidden()`
-`await expect(locator).toBeEnabled()`
-`await expect(locator).toHaveText(...)`
Avoid:
-`expect(await locator.isVisible()).toBe(true)`
- custom polling loops for DOM state
-`waitForTimeout` as synchronization
If a condition genuinely needs custom retry logic, use Playwright's polling/assertion tools deliberately and keep that choice local and explicit.
### 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.
Good pattern:
- assert a meaningful visible state when that is part of the behavior
- then click/fill/select via locator APIs
Bad pattern:
- stack arbitrary waits before every action
- wait on unstable implementation details instead of the visible state the user cares about
### 5. Match debugging to the current suite
Playwright's wider ecosystem supports traces and rich debugging tools. Dify's current suite already captures:
- full-page screenshots
- page HTML
- console errors
- page errors
Use the existing artifact flow by default. If a task is specifically about improving diagnostics, confirm the change fits the current Cucumber architecture before importing broader Playwright tooling.
## Review Questions
- Would this locator survive DOM refactors that do not change user-visible behavior?
- Is this assertion using Playwright's retrying semantics?
- Is any explicit wait masking a real readiness problem?
- Does this code preserve per-scenario isolation?
- Is a new abstraction really needed, or does it bypass the existing `DifyWorld` + step-definition model?
description: "Trigger when the user requests a review of frontend files (e.g., `.tsx`, `.ts`, `.js`). Support both pending-change reviews and focused file reviews while applying the checklist rules."
description: Review Dify frontend code for correctness, accessibility, component design, dify-ui usage, data/query boundaries, performance, and tests. Trigger for `.tsx`, `.ts`, `.js`, UI, React, Next.js, pending-change, or focused frontend review requests.
---
# Frontend Code Review
## Intent
Use this skill whenever the user asks to review frontend code (especially `.tsx`, `.ts`, or `.js` files). Support two review modes:
## When To Use
1.**Pending-change review**– inspect staged/working-tree files slated for commit and flag checklist violations before submission.
2.**File-targeted review**– review the specific file(s) the user names and report the relevant checklist findings.
Use this skill when the user asks to review, audit, analyze, or sanity-check frontend code under `web/`, `packages/dify-ui/`, or frontend-adjacent TypeScript files.
Stick to the checklist below for every applicable file and mode.
Supported modes:
## Checklist
See [references/code-quality.md](references/code-quality.md), [references/performance.md](references/performance.md), [references/business-logic.md](references/business-logic.md) for the living checklist split by category—treat it as the canonical set of rules to follow.
- **Pending-change review**: inspect staged and working-tree changes.
- **File-focused review**: inspect explicitly named files or paths.
- **Diff/snippet review**: review pasted diffs or snippets using best-effort references.
Flag each rule violation with urgency metadata so future reviewers can prioritize fixes.
Do not use this skill for backend-only code under `api/`; use `backend-code-review` instead.
## Required Context
Before reviewing, read the relevant local contracts:
-`web/AGENTS.md` for Dify frontend workflow, overlays, design tokens, state, and tests.
-`packages/dify-ui/README.md` and `packages/dify-ui/AGENTS.md` when code uses or changes `@langgenius/dify-ui/*`.
-`web/docs/overlay.md` when reviewing dialogs, drawers, popovers, tooltips, menus, selects, comboboxes, or other floating UI.
-`web/docs/test.md` and the `frontend-testing` skill when reviewing tests or testability.
-`how-to-write-component` when reviewing React component structure, ownership, effects, query/mutation contracts, or memoization.
For any UI, UX, or accessibility review, fetch the latest Web Interface Guidelines before finalizing findings. Treat them as a required baseline, not the complete source of accessibility truth:
If the review depends on a current framework, SDK, browser API, or accessibility behavior and local code does not settle it, check the current official docs first. For browser compatibility, deprecation, or behavior-sensitive frontend APIs, verify MDN or the relevant standard.
## Rule Packs
Apply every relevant rule pack:
- [references/accessibility-ui.md](references/accessibility-ui.md) — accessibility, semantic HTML, focus, forms, keyboard, disabled states, copy, and long-content behavior. Combines Web Interface Guidelines with Dify UI, Base UI, MDN, and local primitive contracts.
- [references/dify-ui.md](references/dify-ui.md) — Dify UI primitive usage, Base UI semantics, overlays, forms, tokens, radius mapping, and primitive boundaries.
- [references/performance.md](references/performance.md) — React/Next performance review rules from Vercel guidance, scoped to real risk.
- [references/testing.md](references/testing.md) — frontend test review rules.
- [references/dify-invariants.md](references/dify-invariants.md) — stable Dify-specific runtime invariants that generic React/a11y rules will not catch.
- [references/code-quality.md](references/code-quality.md) — general TypeScript, styling, naming, and maintainability rules.
## Review Process
1. Open the relevant component/module. Gather lines that relate to class names, React Flow hooks, prop memoization, and styling.
2. For each rule in the review point, note where the code deviates and capture a representative snippet.
3. Compose the review section per the template below. Group violations first by **Urgent** flag, then by category order (Code Quality, Performance, Business Logic).
## Required output
When invoked, the response must exactly follow one of the two templates:
1. Identify the review scope. For pending changes, inspect `git diff --stat`, `git diff`, and staged diff if relevant. For file-focused reviews, stay within the named files unless a referenced owner/contract must be read.
2. Read code around the changed lines and the owning module. Do not review by isolated snippets when nearby ownership, labels, query inputs, or overlay structure decide correctness.
3. Check user-visible regressions first: accessibility, broken interaction, auth/permission leaks, query/hydration errors, data loss, navigation mistakes, and impossible states.
4. Then check maintainability and performance: ownership, effects, wrappers, memoization, bundle/waterfall risks, tests, and design-system drift.
5. Report only actionable findings. Do not list speculative risks, style preferences, or broad refactors unless they are directly tied to a reproducible issue in scope.
### Template A (any findings)
```
# Code review
Found <N> urgent issues need to be fixed:
## Severity
## 1 <brief description of bug>
FilePath: <path> line <line>
<relevant code snippet or pointer>
- **P0**: security/privacy/auth leak, data loss, production crash, inaccessible critical flow, or broken primary workflow.
- **P2**: maintainability or performance issue likely to cause bugs, duplicated state, incorrect ownership, missing tests for risky behavior, or non-critical a11y issue.
- **P3**: minor cleanup with clear value. Omit unless the user asked for a thorough audit.
## Output Format
### Suggested fix
<brief description of suggested fix>
Lead with findings, ordered by severity. Use this structure:
---
... (repeat for each urgent issue) ...
```markdown
## Findings
Found <M> suggestions for improvement:
- [P1] Short issue title
File: `path/to/file.tsx:123`
Why it matters and how to reproduce or reason about it.
Suggested fix: concrete fix direction.
## 1 <brief description of suggestion>
FilePath: <path> line <line>
<relevant code snippet or pointer>
## Open Questions
- Question or assumption, if any.
### Suggested fix
<brief description of suggested fix>
## Summary
---
... (repeat for each suggestion) ...
Brief secondary context. Mention tests not run or residual risk.
```
If there are no urgent issues, omit that section. If there are no suggestions, omit that section.
If the issue number is more than 10, summarize as "10+ urgent issues" or "10+ suggestions" and just output the first 10 issues.
Don't compress the blank lines between sections; keep them as-is for readability.
If you use Template A (i.e., there are issues to fix) and at least one issue requires code changes, append a brief follow-up question after the structured output asking whether the user wants you to apply the suggested fix(es). For example: "Would you like me to use the Suggested fix section to address these issues?"
### Template B (no issues)
```
## Code review
No issues found.
```
Rules:
- If there are no findings, say `No issues found.` and mention any test gaps or residual risk.
- Always include file and line when available.
- Keep findings concrete and reproducible.
- Do not include praise sections by default.
- Do not ask to apply fixes unless the user explicitly wants review plus implementation.
Do not treat that document as the complete accessibility rule set. Combine it with:
-`packages/dify-ui/README.md`, `packages/dify-ui/AGENTS.md`, and the relevant primitive implementation when code uses `@langgenius/dify-ui/*`.
- Base UI docs and local `.d.ts` contracts when primitive semantics, focus target, labels, or popup reachability are unclear.
- MDN or relevant WAI-ARIA/browser standards when behavior, compatibility, or deprecation status matters.
- The current feature's product semantics, because an accessible primitive can still be used in an inaccessible workflow.
## Semantic HTML
Flag:
- Clickable `div` or `span` used for actions.
- Router navigation implemented with button or `onClick` when a `Link` / `<a>` is the real semantic element.
- Icon-only buttons without `aria-label` or `aria-labelledby`.
- Decorative icons missing `aria-hidden="true"`.
- Images without `alt`; use `alt=""` only when truly decorative.
- Heading levels that skip hierarchy in page-level content.
Prefer semantic HTML before ARIA.
## Keyboard And Focus
Flag:
- Interactive elements without visible `focus-visible` treatment.
-`outline-none` / `outline-hidden` without an equivalent focus-visible ring or state.
- Custom interactive elements missing keyboard handling.
- Focus trapped, lost, or sent to the wrong surface after dialog/popover/menu close.
- Focus ring applied to the wrong DOM node. Verify the actual focus target, especially with Base UI controls such as Slider.
Use `focus-visible` for keyboard focus. Use `focus-within` or `has-[:focus-visible]` when the visual wrapper is not the focused element.
## Forms
Flag:
- Inputs, selects, switches, checkboxes, radios, comboboxes, or sliders without a label relationship.
- Missing stable `name` on form fields that submit or validate.
- Incorrect input `type`, `inputMode`, `autoComplete`, or `spellCheck` for email, token, URL, number, search, code, or username fields.
- Labels that are not clickable.
- Submit buttons disabled before a request starts, preventing normal submit behavior.
- Non-submit buttons inside forms missing `type="button"`.
- Errors not associated with fields or not reachable by screen readers.
- Error recovery that does not focus or expose the first invalid field.
-`onPaste` blocking paste.
- Placeholder text used as the only label.
- Password managers accidentally triggered on non-auth fields because autocomplete is missing or wrong.
Prefer visible labels. If visible surrounding text already labels the control, use a visually hidden label or a precise `aria-label`.
## Disabled, Loading, And Async States
Flag:
- Loading state without `aria-busy`, `role="status"`, or another accessible update path when it changes user interaction.
- Spinner or decorative loading icon exposed to screen readers.
- Disabled controls that hide the reason users cannot proceed.
-`aria-disabled` used without manually blocking click, Space, and Enter.
- Toasts, inline validation, or async status changes that are not announced when users need the update to continue.
- Icon-only loading/error affordances without text or accessible status where the state matters.
Use native `disabled` when the control must not be interactive. Use `aria-disabled` only when the element must remain focusable and the code handles all blocked interactions.
For repeated shared disabled reasons, prefer a visible group message or badge plus native disabled controls. Use per-control popover/info only when the reason is item-specific.
## Overlays And Popup Reachability
Flag:
- Tooltip used for long, structured, interactive, or unique information.
- Tooltip content required to understand or complete a flow.
- PreviewCard content that touch or screen-reader users cannot reach through the trigger's click destination.
- Popover/dialog/menu triggers without accessible names.
- Popup content without title/description where the primitive requires them.
Use Popover for explanatory content, rich help, and infotips. Use Tooltip only as a short visual label for a trigger that already has an accessible name.
## Long Content And Layout
Flag:
- Text in flex/grid children without `min-w-0` when it can overflow.
- Names, labels, file names, model names, workspace names, or user content lacking `truncate`, `line-clamp`, or `break-words`.
- Right-side icons, badges, checks, or actions that shrink before the text area.
- Empty arrays or empty strings rendering broken layout instead of an empty state.
- Button, tab, badge, chip, menu item, or card text that can overlap sibling controls at common viewport widths.
The usual Dify layout chain is: container has width constraints, text region uses `min-w-0 flex-1 truncate`, adornments use `shrink-0`.
## Motion, Images, And Copy
Flag:
-`transition-all`.
- Animations that do not respect reduced motion.
- Layout-affecting animation where transform/opacity would work.
- Images without dimensions.
- Loading copy using `...` instead of `…`.
- Hardcoded dates, times, numbers, or currency formats instead of `Intl.*`.
File path pattern of node components: `web/app/components/workflow/nodes/[nodeName]/node.tsx`
Node components are also used when creating a RAG Pipe from a template, but in that context there is no workflowStore Provider, which results in a blank screen. [This Issue](https://github.com/langgenius/dify/issues/29168) was caused by exactly this reason.
### Suggested Fix
Use `import { useNodes } from 'reactflow'` instead of `import useNodes from '@/app/components/workflow/store/workflow/use-nodes'`.
Flag changes that expand beyond the requested feature or review scope:
### Description
- Repo-wide cleanup mixed into a targeted fix.
- Compatibility exports, aliases, shims, or wrapper layers added without an explicit migration requirement.
- Shared abstractions created before there is stable cross-feature reuse.
- Business components moved into generic shared locations without a clear ownership boundary.
Ensure conditional CSS is handled via the shared `classNames` instead of custom ternaries, string concatenation, or template strings. Centralizing class logic keeps components consistent and easier to maintain.
-`any` or broad `Record<string, any>` where generated/API types or local domain types exist.
- Re-declared API shapes instead of importing generated or returned types.
- Weak route/query param typing that leaks `string | string[] | undefined` deep into components.
- Runtime wrappers added only to satisfy TypeScript when a narrower type boundary would preserve the existing runtime shape.
## Tailwind-first styling
Prefer:
IsUrgent: True
Category: Code Quality
- Explicit domain names that match the API contract.
- Type narrowing at route/API boundaries.
- Small conversion helpers colocated with the component that needs them.
### Description
## Styling
Favor Tailwind CSS utility classes instead of adding new `.module.css` files unless a Tailwind combination cannot achieve the required styling. Keeping styles in Tailwind improves consistency and reduces maintenance overhead.
Flag:
Update this file when adding, editing, or removing Code Quality rules so the catalog remains accurate.
- New CSS modules or ad hoc CSS when Tailwind utilities and Dify tokens cover the need.
- Generic color utilities where Dify semantic tokens exist.
- Hardcoded magic class values for colors, spacing, radius, shadow, z-index, or typography when Dify tokens, component variants, or documented radius mappings exist.
-`!` important modifiers or important CSS overrides without a narrow, documented reason.
- Manual string concatenation for conditional classes.
- JS conditional class branches for primitive visual states already exposed by Dify UI/Base UI `data-*` selectors.
- Incoming `className` placed before default classes in `cn(...)`, preventing call-site overrides.
- Arbitrary z-index or one-off layering fixes on overlays.
## Classname ordering for easy overrides
Use:
### Description
-`cn(...)` from the local package or utility already used by the file.
- Dify semantic tokens and Tailwind v4 utilities.
- Existing component variants before one-off class forks.
- Primitive selectors such as `data-disabled:*`, `data-checked:*`, `data-highlighted:*`, `group-data-*`, `peer-data-*`, and `has-[:focus-visible]` before adding React state or boolean props solely for styling.
- Component-level variants, semantic tokens, and normal cascade/order before `!` overrides. Use `!` only for a contained compatibility override that cannot be expressed through the component API or local selector structure.
When writing components, always place the incoming `className` prop after the component’s own class values so that downstream consumers can override or extend the styling. This keeps your component’s defaults but still lets external callers change or remove specific styles.
## Imports
Example:
Flag:
```tsx
import{cn}from'@/utils/classnames'
- Barrel imports from `@langgenius/dify-ui`; consumers must use subpath exports.
- New overlay imports from legacy `@/app/components/base/modal`, `dialog`, or `drawer`.
- Cross-feature imports that bypass explicit top-level public files.
- Direct imports from generated/internal implementation files when a feature contract already exposes the intended surface.
Use these rules for React component structure, ownership, state, props, effects, and module organization.
## Ownership
Flag:
- State, query, mutation, or handlers hoisted above the lowest component that actually uses them.
- Parent components owning row/item actions that do not coordinate a workflow.
- Prop drilling through multiple pass-through layers.
- A page/tab-level section component becoming the data owner without needing a shared snapshot or shared loading/error/empty UI.
- Feature code promoted to shared only because it appears once or might be reused later.
Accept repeated TanStack Query calls in siblings when each component independently consumes the data. Cache deduplication is not a reason to hoist by itself.
## Component Boundaries
Flag:
- Shallow wrappers that only rename props or hide the real primitive.
- Extra DOM wrappers that do not provide layout, semantics, accessibility, state ownership, or library integration.
- Dialog/dropdown/popover hidden surfaces that obscure the parent flow when they should be extracted into a small local component.
- Business forms, menu bodies, or one-off helpers moved away from their owner without reuse or semantic value.
Prefer colocated components split by actual data and state needs.
## Bad Component Design Patterns
Flag:
- Components that mix data fetching, mutation side effects, popup state, form validation, layout, and row rendering without a clear owner.
- Generic components with many boolean props that encode one feature's workflow.
- A shared component that imports feature-specific copy, routes, or API contracts.
- A feature component that accepts pre-rendered fragments only to avoid placing ownership correctly.
- A child component that receives both raw server data and separately derived flags for the same concept.
- A wrapper that changes accessible semantics of the primitive it wraps.
- A component that exposes controlled props but still keeps a competing private state for the same value.
- A component that cannot render empty, loading, or missing optional API fields without caller-side preprocessing.
- Named `Props` types for trivial one-off props where inline typing is clearer.
- Props named by UI implementation instead of domain/API role.
- API data converted too early or under a generic name that breaks traceability.
- Callers duplicating fallback checks that the lowest rendering component already handles.
Prefer top-level `function` declarations for components and module helpers. Use arrow functions for callbacks and local lambdas.
## Effects
Flag effects that:
- Transform props/state for rendering.
- Copy one state value into another representing the same concept.
- Handle user actions that belong in event handlers.
- Reset state from props when a keyed reset, stable ID, or render-time derivation would work.
- Fetch data that belongs in framework APIs or TanStack Query.
If an effect remains, it must synchronize with a named external system: browser API, subscription, timer, analytics-on-visibility, non-React widget, or imperative DOM integration.
## State Modeling
Flag:
- Storing derived booleans, disabled flags, default tabs, or loading labels that can be calculated from current query/feature state.
- Local state used to fake server data or generated contract fields.
- UI state persisted to localStorage when it is live app state.
- Feature-local mock shells wired to unrelated existing APIs before the real API is confirmed.
Prefer render-time derivation. Keep true local state for user choices, transient input, controlled popups, and feature UI state that has no server source.
## Navigation
Flag:
- Imperative router navigation for ordinary links.
- Button semantics used for navigation.
- Navigation state hidden in component state when URL state is required for shareable filters, tabs, or pagination.
Use `Link` for normal navigation. Use router APIs for mutation success, guarded redirects, command flows, or form submission side effects.
Use these rules for generated contracts, TanStack Query, mutations, auth/SSR boundaries, URL state, and client persistence.
## Generated Contracts
Flag:
- New legacy service/helper wrappers around generated `queryOptions()` or `mutationOptions()`.
- Continuing to use deprecated contract operations when a ready generated contract exists.
- Assuming a generated file means an operation is ready without checking deprecated markers, schema shape, and the actual UI consumer.
- Re-declaring API DTOs in components.
- Adding compatibility layers instead of migrating the pointed line and deleting the old layer.
Use `web/contract/*` as the API shape source of truth. Follow existing `{ params, query?, body? }` input shape.
## Queries
Flag:
-`enabled` used to hide missing required input instead of `input: skipToken`.
- Fake fallback IDs or placeholder inputs used to force a query to run.
- Query results copied into local state for rendering.
- Shared query behavior such as invalidation, stale defaults, or retry rules reimplemented at call sites.
-`prefetchQuery` treated as a hard gate or as returning data/errors to the caller.
Use `useQuery(consoleQuery.xxx.queryOptions(...))` or `useQuery(marketplaceQuery.xxx.queryOptions(...))` directly unless a feature hook performs real orchestration.
## Mutations
Flag:
- Deprecated `useInvalid` or `useReset`.
-`mutateAsync` used without a need for Promise semantics.
- Awaited mutations without `try/catch`.
- Components owning shared cache invalidation that belongs in query defaults.
- Optimistic updates that do not match current list/detail ownership.
Use generated `mutationOptions()` directly when possible. Put shared cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`.
## SSR, Auth, And Route Boundaries
Flag:
- Request-time auth, setup, workspace role, or tenant decisions moved into static `next.config redirects()`.
- Dynamic role gates depending on `workspaces.current` implemented as static path redirects.
- Authorization logic depending on soft `prefetchQuery`.
- Removing a client fallback before server API unavailable behavior is defined.
- Global placeholder query contracts introduced to solve a route-local Suspense issue.
- Branding-sensitive UI reading placeholder defaults without checking pending/placeholder state.
Separate hard gates from soft prefetches. `fetchQuery` can be a server decision boundary; `prefetchQuery` is cache warmup.
## Workspace And Tenant
Flag:
- Treating workspace switch as ordinary CRUD invalidation when the current app flow performs server switch plus full reload.
- Query keys that omit workspace/tenant identity when the query truly varies by workspace and no full reload boundary applies.
- Mixing `workspace_id` and `tenant_id` without tracing the current backend/API contract.
Current Dify workspace switch should be reviewed as a tenant cache boundary first.
## URL State And Local Storage
Flag:
- Shareable filters, tabs, pagination, selected panels, or search state hidden only in component state.
- One-shot navigation signals modeled as subscribed persistent state.
- Live app state stored in localStorage.
- Direct `window.localStorage`, `globalThis.localStorage`, or raw storage calls in app code.
- High-frequency interaction state persisted on every change instead of on commit/settle.
Use URL state for shareable UI state, feature/Jotai/store state for live UI state, and `@/hooks/use-local-storage` only for low-frequency client-only preferences, dismissed notices, and UI defaults.
Use these stable Dify-specific runtime rules in addition to the generic review packs.
This file is not a place for active feature notes. Do not add rules for one branch, one PR, or a short-lived product decision such as a specific agent-v2, plugin, model-provider, or onboarding task. Keep a rule here only when all of these are true:
- It is a stable Dify runtime invariant.
- Generic React, TypeScript, accessibility, dify-ui, query, or performance rules would not catch it.
- The failure mode is concrete enough to produce a file-line review finding.
- The rule is likely to remain valid across normal feature work.
## Workflow Nodes And RAG Pipe
Flag:
- Node components under `web/app/components/workflow/nodes/[nodeName]/node.tsx` importing workflow store hooks that are unavailable in RAG Pipe template rendering.
- Node UI relying on provider context that is not mounted in every rendering surface.
- Store reads in render where React Flow `useNodes` / `useEdges` provide the actual node/edge source.
Known failure mode: workflow node components can also render while creating a RAG Pipe from a template. In that context there may be no workflowStore provider, causing a blank screen.
Prefer React Flow hooks for node/edge UI consumption. Use store APIs only where the provider is guaranteed and the code path is workflow-only.
Use these rules whenever a review touches `packages/dify-ui/` or code consuming `@langgenius/dify-ui/*`.
Before finalizing findings for those files, read the current local docs that apply:
-`packages/dify-ui/README.md`
-`packages/dify-ui/AGENTS.md`
-`web/docs/overlay.md` for floating UI
-`packages/dify-ui/src/<primitive>/index.tsx` for the primitive being changed or consumed
## Package Boundary
Flag in `packages/dify-ui`:
- Imports from `web/`.
- Dependencies on Next.js, i18n, ky, Jotai, Zustand, TanStack Query, oRPC, or business APIs.
- Business-specific component behavior that belongs in `web/`.
- Multiple unrelated primitives in one component folder.
`packages/dify-ui` is a primitive layer: Base UI headless components + `cva` + `cn` + Dify design tokens.
## Imports And Exports
Flag:
- Consumer imports from `@langgenius/dify-ui` without a subpath.
- Missing `package.json#exports` entry for a new primitive.
- Internal package imports using workspace subpaths instead of relative paths.
- Exported props using internal-only types that consumers cannot import from the component subpath.
Consumers use subpath exports such as `@langgenius/dify-ui/button`.
## Props And State
Flag:
- Flattened props where related values need a discriminated union, such as `value` / `defaultValue`, `multiple` / `value`, or `clearable` / `onChange`.
- React state used only to mirror Base UI state for class names.
- JavaScript conditional class logic for visual states that the Dify UI/Base UI primitive already exposes through `data-*` attributes or CSS variables.
- Controlled props added when uncontrolled DOM state or CSS variables would be enough.
- Thin wrappers that rename Base UI parts without adding semantics.
Prefer Base UI/Dify UI data attributes and CSS variables for visual state: `data-open`, `data-checked`, `data-disabled`, `data-highlighted`, `data-popup-open`, `group-data-*`, `peer-data-*`, `has-[:focus-visible]`, and primitive CSS variables such as anchor width or transform origin. Use JS conditional classes for product/business state that the primitive does not expose.
## Forms
Flag:
- Form-like UI using unrelated `Input` and `Button` pieces without a submit boundary.
- Text-like fields not composed through `FieldRoot`, `FieldLabel`, and `FieldControl` when using Dify UI form semantics.
- Select fields using `FieldLabel` instead of `SelectLabel`.
- Slider fields using a generic label instead of `SliderLabel`.
- Checkbox/radio groups missing `FieldsetRoot` and `FieldsetLegend`.
- Field errors or descriptions rendered without `FieldDescription` / `FieldError` relationships.
`Form` is the submit boundary. Dify UI form primitives are not a form state-management framework; business validation and schema-driven behavior belong in `web/`.
## Overlay Contract
Flag:
- Legacy web overlay imports in new or modified code.
- Manual portals around Dify UI overlay primitives.
- Call-site `z-*` overrides on overlays.
- Missing root `isolation: isolate` assumptions when debugging overlay stacking.
- Repeated backdrop, z-index, or portal chrome at call sites.
- Tooltip used for infotips, long text, or interactive content.
All Dify UI body-portalled overlays use `z-50`. Toast uses `z-60`. DOM order handles stacking between overlays.
## Primitive Selection
Flag:
-`Tabs` used for simple mode/filter/view selection where `SegmentedControl` is the semantic primitive.
-`SegmentedControl` used where `tablist` / `tabpanel` semantics are required.
-`Select` used for searchable or free-form input.
-`Combobox` used for unrestricted search text where no selected option is remembered.
-`Autocomplete` used for closed-list selection.
- Tooltip or PreviewCard used for content that must be reachable on touch or by screen readers.
Use:
-`Autocomplete` for free-form text with optional suggestions.
-`Combobox` for searchable selected values from a collection.
-`Select` for closed, scannable option sets.
-`Popover` for infotips, help text, rich content, or interactions.
## Bad Usage Patterns To Flag
Flag:
- Styling a raw Base UI primitive directly in `web/` when a Dify UI primitive exists.
- Wrapping a Dify UI primitive in a feature component that hides its label, error, disabled, or focus contract.
- Replacing a semantic primitive with a generic `div` plus classes to match a screenshot.
- Using `Tooltip` because it is visually convenient when the content is actually help text or needs touch access.
- Adding a `z-*` override to make a child popup appear over a parent dialog.
- Adding a new app-level wrapper around Dialog, Drawer, Popover, Select, or Combobox that repeats portal/backdrop/positioner logic.
- Using dify-ui `Input` as a drop-in replacement for legacy inputs that include search, clear, copy, unit, localized placeholder, or number normalization behavior.
- Building a form row from loose text and controls instead of the matching Field/Form primitives.
- Adding component state only to style `data-open`, `data-checked`, `data-disabled`, or highlighted states that Base UI already exposes.
- Passing booleans down only so children can toggle classes already expressible with primitive `data-*` selectors.
## Tokens, Radius, And Styling
Flag:
-`radius-*` class names.
- Custom Tailwind `borderRadius` extension for Figma radius values.
- Generic colors where semantic Dify tokens exist.
- Hardcoded design values where Dify tokens, component variants, or documented Figma radius mappings exist.
-`!` important modifiers used to fight primitive styles instead of fixing the variant, selector, or component composition.
- Manual class strings that duplicate primitive variants.
-`min-w-(--anchor-width)` on picker popups when it defeats viewport clamping.
Use the Figma radius mapping from `packages/dify-ui/AGENTS.md`; for example `--radius/sm` maps to `rounded-md`, and `--radius/md` maps to `rounded-lg`.
Use `!` only for a tightly scoped compatibility override after confirming the primitive API, data attributes, and selector structure cannot express the state.
## Focus Details
Flag focus rings attached to the wrong element. For example, Base UI `Slider.Thumb` focuses an internal `input[type=range]`, so the visible thumb wrapper needs `has-[:focus-visible]` rather than direct wrapper `focus-visible`.
Review performance only where there is realistic impact. Do not request `memo`, `useMemo`, `useCallback`, virtualization, or caching as style preferences.
IsUrgent: True
Category: Performance
## Async Waterfalls
### Description
Flag:
When rendering React Flow, prefer `useNodes`/`useEdges` for UI consumption and rely on `useStoreApi` inside callbacks that mutate or read node/edge state. Avoid manually pulling Flow data outside of these hooks.
- Awaiting remote feature flags or fetches before checking cheap synchronous conditions.
- Sequential awaits for independent operations.
- API routes or server components starting requests late when they could start early.
- Nested per-item fetches running serially when each item can fetch in parallel.
- Suspense boundaries that force the whole page to wait when a lower boundary could stream or isolate loading.
## Complex prop memoization
Prefer `Promise.all` for independent work and branch-local awaits for conditionally needed data.
IsUrgent: True
Category: Performance
## Bundle Size
### Description
Flag:
Wrap complex prop values (objects, arrays, maps) in `useMemo` prior to passing them into child components to guarantee stable references and prevent unnecessary renders.
- Barrel imports from heavy libraries or `@langgenius/dify-ui`.
- Dynamic paths that prevent static trace analysis.
- Heavy components loaded eagerly when hidden behind a dialog, tab, command, or feature activation.
- Analytics, logging, editor, visualization, or third-party SDK code loaded before it is needed.
- Feature-local optional modules imported at top level only for rare flows.
Update this file when adding, editing, or removing Performance rules so the catalog remains accurate.
Use direct imports and `next/dynamic` where the user-visible path benefits.
Wrong:
## Server Rendering
```tsx
<HeavyComp
config={{
provider: ...,
detail: ...
}}
/>
```
Flag:
Right:
- Request-specific mutable state stored at module scope in SSR/RSC paths.
- Large duplicate data serialized across RSC/client boundaries.
- Static I/O repeated per request when it could be hoisted safely.
- Cross-request cache without a bounded invalidation strategy.
- Server actions lacking API-route-equivalent auth checks.
```tsx
constconfig=useMemo(()=>({
provider: ...,
detail: ...
}),[provider,detail]);
Use request-scoped deduplication such as `React.cache()` when repeated server reads in one request are the problem.
<HeavyComp
config={config}
/>
```
## Re-rendering
Flag:
- Effects or subscriptions reading broad state when a derived boolean or narrower selector is enough.
- Components defined inside components.
- Derived rendering state stored in state/effects.
- Non-primitive default props recreated for memoized children.
- Expensive work recalculated on every render where it affects real interaction cost.
- High-frequency transient values stored in state when refs or CSS variables would avoid render loops.
Do not flag simple primitive expressions wrapped or not wrapped in `useMemo`; prefer no memo for simple work.
Require stable object/array/function identity only when:
- The child is memoized and identity affects renders.
- The value is an effect/query dependency.
- A library API requires stable references.
- Profiling or local behavior shows avoidable re-rendering.
## DOM, Lists, And Rendering
Flag:
- Layout reads in render (`getBoundingClientRect`, `offset*`, `scrollTop`).
- Interleaved DOM reads/writes that can cause layout thrashing.
- Large lists rendering without virtualization, pagination, or `content-visibility`.
- SVG/animation code animating expensive properties when transform/opacity would work.
-`transition-all`.
- Long-running non-critical browser work performed immediately instead of idle/deferred scheduling.
## React Flow
For workflow React Flow components, keep this Dify-specific rule:
- UI consumption should use React Flow hooks such as `useNodes` / `useEdges`.
- Callback-only reads or mutations can use `useStoreApi`.
- Node components under `web/app/components/workflow/nodes/[nodeName]/node.tsx` must not depend on workflow stores that are absent in RAG Pipe template rendering.
Use these rules when reviewing test files, testability of changed code, or risky frontend changes that should have tests.
## Missing Coverage
Flag missing tests when the change affects:
- User-visible behavior, navigation, form submission, validation, permissions, or loading/error/empty states.
- Query/mutation cache behavior.
- Accessibility-critical behavior such as labels, keyboard flow, focus, disabled state, or popup reachability.
- URL state parsing/serialization.
- Storage persistence or one-shot signals.
- Regression-prone workflow or generated contract migration paths.
Do not request tests for purely mechanical renames or styling-only changes unless the styling affects layout, focus, or interaction.
## Selectors
Flag:
-`getByTestId` used where role, label, text, placeholder, landmark, or scoped dialog/menu queries are available.
- Production `data-testid` added only to satisfy tests.
- Assertions against decorative icons rather than the named control.
- Tests that cannot find controls semantically but leave broken markup unchanged.
Prefer `getByRole` with accessible name, then `getByLabelText`, `getByPlaceholderText`, `getByText`, and `within(...)`.
## Mocking
Flag:
- Mocking `@langgenius/dify-ui/*` primitives.
- Mocking `@/app/components/base/*` components when the real component is practical.
- Mocking sibling or child components in the same directory for integration behavior.
- Mocks that do not match the real component's conditional rendering.
- Module-level mock state not reset in `beforeEach`.
-`vi.clearAllMocks()` in `afterEach` instead of `beforeEach`.
Use real project components for integration behavior. Mock APIs, `next/navigation`, browser shims, or complex providers only when setup would dominate the test.
## Behavior
Flag:
- Tests inspecting implementation details instead of user-observable behavior.
- Assertions that hardcode brittle copy when pattern matching or semantic roles would express behavior better.
- Fake timers used without real timing behavior.
- Async assertions missing `await`, `findBy*`, or `waitFor`.
- Test data missing required fields because inline partial objects bypass real types.
Use typed factory functions with complete defaults and partial overrides.
## URL State
For `nuqs` or query-state hooks, flag tests that:
- Mock URL state when URL synchronization is the behavior under review.
- Do not test parser serialize/parse round trips for custom parsers.
- Do not assert default-clearing behavior when defaults should be removed from the URL.
Prefer shared `NuqsTestingAdapter` helpers when available.
@ -5,9 +5,9 @@ description: Generate Vitest + React Testing Library tests for Dify frontend com
# Dify Frontend Testing Skill
This skill enables Claude to generate high-quality, comprehensive frontend tests for the Dify project following established conventions and best practices.
This skill enables Codex to generate high-quality, comprehensive frontend tests for the Dify project following established conventions and best practices.
> **⚠️ Authoritative Source**: This skill is derived from `web/testing/testing.md`. Use Vitest mock/timer APIs (`vi.*`).
> **⚠️ Authoritative Source**: This skill is derived from `web/docs/test.md`. Use Vitest mock/timer APIs (`vi.*`).
## When to Apply This Skill
@ -24,35 +24,27 @@ Apply this skill when the user:
**Do NOT apply** when:
- User is asking about backend/API tests (Python/pytest)
- User is asking about E2E tests (Playwright/Cypress)
- User is asking about E2E tests (Cucumber + Playwright under `e2e/`)
- User is only asking conceptual questions without code context
- Test files: `ComponentName.spec.tsx`(same directory as component)
- Test files: `ComponentName.spec.tsx`inside a same-level `__tests__/` directory
- Placement rule: Component, hook, and utility tests must live in a sibling `__tests__/` folder at the same level as the source under test. For example, `foo/index.tsx` maps to `foo/__tests__/index.spec.tsx`, and `foo/bar.ts` maps to `foo/__tests__/bar.spec.ts`.
- Integration tests: `web/__tests__/` directory
## Test Structure Template
@ -199,11 +192,21 @@ When assigned to test a directory/path, test **ALL content** within that path:
- ✅ **Import real project components** directly (including base components and siblings)
- ❌ **DO NOT mock** base components (`@/app/components/base/*`)
- ❌ **DO NOT mock** base components (`@/app/components/base/*`) or dify-ui primitives (`@langgenius/dify-ui/*`)
- ❌ **DO NOT mock** sibling/child components in the same directory
> See [Test Structure Template](#test-structure-template) for correct import/mock patterns.
### `nuqs` Query State Testing (Required for URL State Hooks)
When a component or hook uses `useQueryState` / `useQueryStates`:
- ✅ Use `NuqsTestingAdapter` (prefer shared helpers in `web/test/nuqs-testing.tsx`)
- ✅ Assert URL synchronization via `onUrlUpdate` (`searchParams`, `options.history`)
- ✅ For custom parsers (`createParser`), keep `parse` and `serialize` bijective and add round-trip edge cases (`%2F`, `%25`, spaces, legacy encoded values)
- ✅ Verify default-clearing behavior (default values should be removed from URL when applicable)
- ⚠️ Only mock `nuqs` directly when URL behavior is explicitly out of scope for the test
## Core Principles
### 1. AAA Pattern (Arrange-Act-Assert)
@ -217,7 +220,10 @@ Every test should clearly separate:
### 2. Black-Box Testing
- Test observable behavior, not implementation details
- Use semantic queries (getByRole, getByLabelText)
- Use semantic queries (`getByRole` with accessible `name`, `getByLabelText`, `getByPlaceholderText`, `getByText`, and scoped `within(...)`)
- Treat `getByTestId` as a last resort. If a control cannot be found by role/name, label, landmark, or dialog scope, fix the component accessibility first instead of adding or relying on `data-testid`.
- Remove production `data-testid` attributes when semantic selectors can cover the behavior. Keep them only for non-visual mocked boundaries, editor/browser shims such as Monaco, canvas/chart output, or third-party widgets with no accessible DOM in the test environment.
- Do not assert decorative icons by test id. Assert the named control that contains them, or mark decorative icons `aria-hidden`.
- Avoid testing internal state directly
- **Prefer pattern matching over hardcoded strings** in assertions:
@ -309,17 +315,17 @@ For more detailed information, refer to:
### Primary Specification (MUST follow)
- **`web/testing/testing.md`** - The canonical testing specification. This skill is derived from this document.
- **`web/docs/test.md`** - The canonical testing specification. This skill is derived from this document.
### Reference Examples in Codebase
-`web/utils/classnames.spec.ts` - Utility function tests
- Modules are not mocked automatically. Global mocks live in `web/vitest.setup.ts` (for example `react-i18next`, `next/image`); mock other modules like `ky` or `mime` locally in test files.
Prefer mocking `@/service/*` modules or spying on `global.fetch` / `ky` clients with deterministic responses. Do not introduce an HTTP interception dependency such as `nock` or MSW unless it is already declared in the workspace or adding it is part of the task.
@ -4,7 +4,7 @@ This guide defines the workflow for generating tests, especially for complex com
## Scope Clarification
This guide addresses **multi-file workflow** (how to process multiple test files). For coverage requirements within a single test file, see `web/testing/testing.md` § Coverage Goals.
This guide addresses **multi-file workflow** (how to process multiple test files). For coverage requirements within a single test file, see `web/docs/test.md` § Coverage Goals.
| Scope | Rule |
|-------|------|
@ -227,12 +227,12 @@ Failing tests compound:
**Fix failures immediately before proceeding.**
## Integration with Claude's Todo Feature
## Integration with Codex's Todo Feature
When using Claude for multi-file testing:
When using Codex for multi-file testing:
1.**Ask Claude to create a todo list** before starting
1.**Request one file at a time** or ensure Claude processes incrementally
1.**Create a todo list** before starting
1.**Process one file at a time**
1.**Verify each test passes** before asking for the next
description: React/TypeScript component style guide. Use when writing, refactoring, or reviewing React components, especially around props typing, state boundaries, shared local state with Jotai atoms, API types, query/mutation contracts, navigation, memoization, wrappers, and empty-state handling.
---
# How To Write A Component
Use this as the decision guide for React/TypeScript component structure. Existing code is reference material, not automatic precedent; when it conflicts with these rules, adapt the approach instead of reproducing the violation.
## Core Defaults
- Search before adding UI, hooks, helpers, or styling patterns. Reuse existing base components, feature components, hooks, utilities, and design styles when they fit.
- Group code by feature workflow, route, or ownership area: components, hooks, local types, query helpers, atoms, constants, and small utilities should live near the code that changes with them.
- Promote code to shared only when multiple verticals need the same stable primitive. Otherwise keep it local and compose shared primitives inside the owning feature.
- Follow Dify's CSS-first Tailwind v4 contract from `packages/dify-ui/README.md` and `packages/dify-ui/AGENTS.md`. Prefer design-system tokens, utilities, and radius mappings over generic Tailwind guidance.
## Ownership
- Put local state, queries, mutations, handlers, and derived UI data in the lowest component that uses them. Extract a purpose-built owner component only when the logic has no natural home.
- Repeated TanStack query calls in sibling components are acceptable when each component independently consumes the data. Do not hoist a query only because it is duplicated; TanStack Query handles deduplication and cache sharing.
- Hoist state, queries, or callbacks to a parent only when the parent consumes the data, coordinates shared loading/error/empty UI, needs one consistent snapshot, or owns a workflow spanning children.
- Avoid prop drilling. One pass-through layer is acceptable; repeated forwarding means ownership should move down or into feature-scoped Jotai UI state. Keep server/cache state in query and API data flow.
- Keep callbacks in a parent only for workflow coordination such as form submission, shared selection, batch behavior, or navigation. Otherwise let the child or row own its action.
- Prefer uncontrolled DOM state and CSS variables before adding controlled props.
## Components, Props, And Types
- Type component signatures directly; do not use `FC` or `React.FC`.
- Prefer `function` for top-level components and module helpers. Use arrow functions for local callbacks, handlers, and lambda-style APIs.
- Prefer named exports. Use default exports only where the framework requires them, such as Next.js route files.
- Type simple one-off props inline. Use a named `Props` type only when reused, exported, complex, or clearer.
- Use API-generated or API-returned types at component boundaries. Keep small UI conversion helpers beside the component that needs them.
- Name values by their domain role and backend API contract, and keep that name stable across the call chain, especially IDs like `appInstanceId`. Normalize framework or route params at the boundary.
- Keep fallback and invariant checks at the lowest component that already handles that state; callers should pass raw values through instead of duplicating checks.
## Queries And Mutations
- Keep `web/contract/*` as the single source of truth for API shape; follow existing domain/router patterns and the `{ params, query?, body? }` input shape.
- Consume queries directly with `useQuery(consoleQuery.xxx.queryOptions(...))` or `useQuery(marketplaceQuery.xxx.queryOptions(...))`.
- Avoid pass-through hooks and thin `web/service/use-*` wrappers that only rename `queryOptions()` or `mutationOptions()`. Extract a small `queryOptions` helper only when repeated call-site options justify it.
- Keep feature hooks for real orchestration, workflow state, or shared domain behavior.
- For missing required query input, use `input: skipToken`; use `enabled` only for extra business gating after the input is valid.
- Consume mutations directly with `useMutation(consoleQuery.xxx.mutationOptions(...))` or `useMutation(marketplaceQuery.xxx.mutationOptions(...))`; use oRPC clients as `mutationFn` only for custom flows.
- Put shared cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`; components may add UI feedback callbacks, but should not own shared invalidation rules.
- Do not use deprecated `useInvalid` or `useReset`.
- Prefer `mutate(...)`; use `mutateAsync(...)` only when Promise semantics are required, and wrap awaited calls in `try/catch`.
## Component Boundaries
- Use the first level below a page or tab to organize independent page sections when it adds real structure. This layer is layout/semantic first, not automatically the data owner.
- Split deeper components by the data and state each layer actually needs. Each component should access only necessary data, and ownership should stay at the lowest consumer.
- Keep cohesive forms, menu bodies, and one-off helpers local unless they need their own state, reuse, or semantic boundary.
- Separate hidden secondary surfaces from the trigger's main flow. For dialogs, dropdowns, popovers, and similar branches, extract a small local component that owns the trigger, open state, and hidden content when it would obscure the parent flow.
- Preserve composability by separating behavior ownership from layout ownership. A dropdown action may own its trigger, open state, and menu content; the caller owns placement such as slots, offsets, and alignment.
- Avoid unnecessary DOM hierarchy. Do not add wrapper elements unless they provide layout, semantics, accessibility, state ownership, or integration with a library API; prefer fragments or styling an existing element when possible.
- Avoid shallow wrappers and prop renaming unless the wrapper adds validation, orchestration, error handling, state ownership, or a real semantic boundary.
## You Might Not Need An Effect
- Use Effects only to synchronize with external systems such as browser APIs, non-React widgets, subscriptions, timers, analytics that must run because the component was shown, or imperative DOM integration.
- Do not use Effects to transform props or state for rendering. Calculate derived values during render, and use `useMemo` only when the calculation is actually expensive.
- Do not use Effects to handle user actions. Put action-specific logic in the event handler where the cause is known.
- Do not use Effects to copy one state value into another state value representing the same concept. Pick one source of truth and derive the rest during render.
- Do not reset or adjust state from props with an Effect. Prefer a `key` reset, storing a stable ID and deriving the selected object, or guarded same-component render-time adjustment when truly necessary.
- Prefer framework data APIs or TanStack Query for data fetching instead of writing request Effects in components.
- If an Effect still seems necessary, first name the external system it synchronizes with. If there is no external system, remove the Effect and restructure the state or event flow.
## Navigation And Performance
- Prefer `Link` for normal navigation. Use router APIs only for command-flow side effects such as mutation success, guarded redirects, or form submission.
- Avoid `memo`, `useMemo`, and `useCallback` unless there is a clear performance reason.
description: Guide for implementing oRPC contract-first API patterns in Dify frontend. Triggers when creating new API contracts, adding service endpoints, integrating TanStack Query with typed contracts, or migrating legacy service calls to oRPC. Use for all API layer work in web/contract and web/service directories.
---
# oRPC Contract-First Development
## Project Structure
```
web/contract/
├── base.ts # Base contract (inputStructure: 'detailed')
├── router.ts # Router composition & type exports
├── marketplace.ts # Marketplace contracts
└── console/ # Console contracts by domain
├── system.ts
└── billing.ts
```
## Workflow
1.**Create contract** in `web/contract/console/{domain}.ts`
- Import `base` from `../base` and `type` from `@orpc/contract`
- Define route with `path`, `method`, `input`, `output`
2.**Register in router** at `web/contract/router.ts`
- Import directly from domain file (no barrel files)
- Nest by API prefix: `billing: { invoices, bindPartnerStack }`
3.**Create hooks** in `web/service/use-{domain}.ts`
- Use `consoleQuery.{group}.{contract}.queryKey()` for query keys
- Use `consoleClient.{group}.{contract}()` for API calls
## Key Rules
- **Input structure**: Always use `{ params, query?, body? }` format
- **Path params**: Use `{paramName}` in path, match in `params` object
- **Router nesting**: Group by API prefix (e.g., `/billing/*` → `billing: {}`)
- **No barrel files**: Import directly from specific files
- **Types**: Import from `@/types/`, use `type<T>()` helper
description: Guide for creating effective skills. This skill should be used when users want to create a new skill (or update an existing skill) that extends Claude's capabilities with specialized knowledge, workflows, or tool integrations.
---
# Skill Creator
This skill provides guidance for creating effective skills.
## About Skills
Skills are modular, self-contained packages that extend Claude's capabilities by providing
specialized knowledge, workflows, and tools. Think of them as "onboarding guides" for specific
domains or tasks—they transform Claude from a general-purpose agent into a specialized agent
equipped with procedural knowledge that no model can fully possess.
### What Skills Provide
1. Specialized workflows - Multi-step procedures for specific domains
2. Tool integrations - Instructions for working with specific file formats or APIs
3. Domain expertise - Company-specific knowledge, schemas, business logic
4. Bundled resources - Scripts, references, and assets for complex and repetitive tasks
## Core Principles
### Concise is Key
The context window is a public good. Skills share the context window with everything else Claude needs: system prompt, conversation history, other Skills' metadata, and the actual user request.
**Default assumption: Claude is already very smart.** Only add context Claude doesn't already have. Challenge each piece of information: "Does Claude really need this explanation?" and "Does this paragraph justify its token cost?"
Prefer concise examples over verbose explanations.
### Set Appropriate Degrees of Freedom
Match the level of specificity to the task's fragility and variability:
**High freedom (text-based instructions)**: Use when multiple approaches are valid, decisions depend on context, or heuristics guide the approach.
**Medium freedom (pseudocode or scripts with parameters)**: Use when a preferred pattern exists, some variation is acceptable, or configuration affects behavior.
**Low freedom (specific scripts, few parameters)**: Use when operations are fragile and error-prone, consistency is critical, or a specific sequence must be followed.
Think of Claude as exploring a path: a narrow bridge with cliffs needs specific guardrails (low freedom), while an open field allows many routes (high freedom).
### Anatomy of a Skill
Every skill consists of a required SKILL.md file and optional bundled resources:
```
skill-name/
├── SKILL.md (required)
│ ├── YAML frontmatter metadata (required)
│ │ ├── name: (required)
│ │ └── description: (required)
│ └── Markdown instructions (required)
└── Bundled Resources (optional)
├── scripts/ - Executable code (Python/Bash/etc.)
├── references/ - Documentation intended to be loaded into context as needed
└── assets/ - Files used in output (templates, icons, fonts, etc.)
```
#### SKILL.md (required)
Every SKILL.md consists of:
- **Frontmatter** (YAML): Contains `name` and `description` fields. These are the only fields that Claude reads to determine when the skill gets used, thus it is very important to be clear and comprehensive in describing what the skill is, and when it should be used.
- **Body** (Markdown): Instructions and guidance for using the skill. Only loaded AFTER the skill triggers (if at all).
#### Bundled Resources (optional)
##### Scripts (`scripts/`)
Executable code (Python/Bash/etc.) for tasks that require deterministic reliability or are repeatedly rewritten.
- **When to include**: When the same code is being rewritten repeatedly or deterministic reliability is needed
- **Example**: `scripts/rotate_pdf.py` for PDF rotation tasks
- **Benefits**: Token efficient, deterministic, may be executed without loading into context
- **Note**: Scripts may still need to be read by Claude for patching or environment-specific adjustments
##### References (`references/`)
Documentation and reference material intended to be loaded as needed into context to inform Claude's process and thinking.
- **When to include**: For documentation that Claude should reference while working
- **Examples**: `references/finance.md` for financial schemas, `references/mnda.md` for company NDA template, `references/policies.md` for company policies, `references/api_docs.md` for API specifications
- **Use cases**: Database schemas, API documentation, domain knowledge, company policies, detailed workflow guides
- **Benefits**: Keeps SKILL.md lean, loaded only when Claude determines it's needed
- **Best practice**: If files are large (>10k words), include grep search patterns in SKILL.md
- **Avoid duplication**: Information should live in either SKILL.md or references files, not both. Prefer references files for detailed information unless it's truly core to the skill—this keeps SKILL.md lean while making information discoverable without hogging the context window. Keep only essential procedural instructions and workflow guidance in SKILL.md; move detailed reference material, schemas, and examples to references files.
##### Assets (`assets/`)
Files not intended to be loaded into context, but rather used within the output Claude produces.
- **When to include**: When the skill needs files that will be used in the final output
- **Examples**: `assets/logo.png` for brand assets, `assets/slides.pptx` for PowerPoint templates, `assets/frontend-template/` for HTML/React boilerplate, `assets/font.ttf` for typography
- **Use cases**: Templates, images, icons, boilerplate code, fonts, sample documents that get copied or modified
- **Benefits**: Separates output resources from documentation, enables Claude to use files without loading them into context
#### What to Not Include in a Skill
A skill should only contain essential files that directly support its functionality. Do NOT create extraneous documentation or auxiliary files, including:
- README.md
- INSTALLATION_GUIDE.md
- QUICK_REFERENCE.md
- CHANGELOG.md
- etc.
The skill should only contain the information needed for an AI agent to do the job at hand. It should not contain auxilary context about the process that went into creating it, setup and testing procedures, user-facing documentation, etc. Creating additional documentation files just adds clutter and confusion.
### Progressive Disclosure Design Principle
Skills use a three-level loading system to manage context efficiently:
1.**Metadata (name + description)** - Always in context (~100 words)
2.**SKILL.md body** - When skill triggers (<5kwords)
For complex tasks, break operations into clear, sequential steps. It is often helpful to give Claude an overview of the process towards the beginning of SKILL.md:
```markdown
Filling a PDF form involves these steps:
1. Analyze the form (run analyze_form.py)
2. Create field mapping (edit fields.json)
3. Validate mapping (run validate_fields.py)
4. Fill the form (run fill_form.py)
5. Verify output (run verify_output.py)
```
## Conditional Workflows
For tasks with branching logic, guide Claude through decision points:
```markdown
1. Determine the modification type:
**Creating new content?** → Follow "Creation workflow" below
description: [TODO: Complete and informative explanation of what the skill does and when to use it. Include WHEN to use this skill - specific scenarios, file types, or tasks that trigger it.]
---
# {skill_title}
## Overview
[TODO: 1-2 sentences explaining what this skill enables]
## Structuring This Skill
[TODO: Choose the structure that best fits this skill's purpose. Common patterns:
**1. Workflow-Based** (best for sequential processes)
- Works well when there are clear step-by-step procedures
- BigQuery: API reference documentation and query examples
- Finance: Schema documentation, company policies
**Appropriate for:** In-depth documentation, API references, database schemas, comprehensive guides, or any detailed information that Claude should reference while working.
### assets/
Files not intended to be loaded into context, but rather used within the output Claude produces.
**Examples from other skills:**
- Brand styling: PowerPoint template files (.pptx), logo files
**Appropriate for:** Templates, boilerplate code, document templates, images, icons, fonts, or any files meant to be copied or used in the final output.
---
**Any unneeded directories can be deleted.** Not every skill requires all three types of resources.
"""
EXAMPLE_SCRIPT='''#!/usr/bin/env python3
"""
Example helper script for {skill_name}
This is a placeholder script that can be executed directly.
Replace with actual implementation or delete if not needed.
Example real scripts from other skills:
- pdf/scripts/fill_fillable_fields.py - Fills PDF form fields
- pdf/scripts/convert_pdf_to_images.py - Converts PDF pages to images
"""
def main():
print("This is an example script for {skill_name}")
# TODO: Add actual script logic here
# This could be data processing, file conversion, API calls, etc.
if __name__ == "__main__":
main()
'''
EXAMPLE_REFERENCE="""# Reference Documentation for {skill_title}
This is a placeholder for detailed reference documentation.
Replace with actual reference content or delete if not needed.
Example real reference docs from other skills:
- product-management/references/communication.md - Comprehensive guide for status updates
- product-management/references/context_building.md - Deep-dive on gathering context
- bigquery/references/ - API references and query examples
## When Reference Docs Are Useful
Reference docs are ideal for:
- Comprehensive API documentation
- Detailed workflow guides
- Complex multi-step processes
- Information too lengthy for main SKILL.md
- Content that's only needed for specific use cases
## Structure Suggestions
### API Reference Example
- Overview
- Authentication
- Endpoints with examples
- Error codes
- Rate limits
### Workflow Guide Example
- Prerequisites
- Step-by-step instructions
- Common patterns
- Troubleshooting
- Best practices
"""
EXAMPLE_ASSET="""# Example Asset File
This placeholder represents where asset files would be stored.
Replace with actual asset files (templates, images, fonts, etc.) or delete if not needed.
Asset files are NOT intended to be loaded into context, but rather used within
description: React and Next.js performance optimization guidelines from Vercel Engineering. This skill should be used when writing, reviewing, or refactoring React/Next.js code to ensure optimal performance patterns. Triggers on tasks involving React components, Next.js pages, data fetching, bundle optimization, or performance improvements.
license: MIT
metadata:
author: vercel
version: "1.0.0"
---
# Vercel React Best Practices
Comprehensive performance optimization guide for React and Next.js applications, maintained by Vercel. Contains 45 rules across 8 categories, prioritized by impact to guide automated refactoring and code generation.
## When to Apply
Reference these guidelines when:
- Writing new React components or Next.js pages
- Implementing data fetching (client or server-side)
`useEffectEvent` provides a cleaner API for the same pattern: it creates a stable function reference that always calls the latest version of the handler.
Import directly from source files instead of barrel files to avoid loading thousands of unused modules. **Barrel files** are entry points that re-export multiple modules (e.g., `index.js` that does `export * from './module'`).
Popular icon and component libraries can have **up to 10,000 re-exports** in their entry file. For many React packages, **it takes 200-800ms just to import them**, affecting both development speed and production cold starts.
**Why tree-shaking doesn't help:** When a library is marked as external (not bundled), the bundler can't optimize it. If you bundle it to enable tree-shaking, builds become substantially slower analyzing the entire module graph.
## Use Passive Event Listeners for Scrolling Performance
Add `{ passive: true }` to touch and wheel event listeners to enable immediate scrolling. Browsers normally wait for listeners to finish to check if `preventDefault()` is called, causing scroll delay.
Avoid interleaving style writes with layout reads. When you read a layout property (like `offsetWidth`, `getBoundingClientRect()`, or `getComputedStyle()`) between style changes, the browser is forced to trigger a synchronous reflow.
**Incorrect (interleaved reads and writes force reflows):**
Prefer CSS classes over inline styles when possible. CSS files are cached by the browser, and classes provide better separation of concerns and are easier to maintain.
When comparing arrays with expensive operations (sorting, deep equality, serialization), check lengths first. If lengths differ, the arrays cannot be equal.
In real-world applications, this optimization is especially valuable when the comparison runs in hot paths (event handlers, render loops).
Two O(n log n) sorts run even when `current.length` is 5 and `original.length` is 100. There is also overhead of joining the arrays and comparing the strings.
Single pass through the array, no copying, no sorting.
**Alternative (Math.min/Math.max for small arrays):**
```typescript
constnumbers=[5,2,8,1,9]
constmin=Math.min(...numbers)
constmax=Math.max(...numbers)
```
This works for small arrays, but can be slower or just throw an error for very large arrays due to spread operator limitations. Maximal array length is approximately 124000 in Chrome 143 and 638000 in Safari 18; exact numbers may vary - see [the fiddle](https://jsfiddle.net/qw1jabsx/4/). Use the loop approach for reliability.
## Use toSorted() Instead of sort() for Immutability
`.sort()` mutates the array in place, which can cause bugs with React state and props. Use `.toSorted()` to create a new sorted array without mutation.
This applies to all CSS transforms and transitions (`transform`, `opacity`, `translate`, `scale`, `rotate`). The wrapper div allows browsers to use GPU acceleration for smoother animations.
Use explicit ternary operators (`? :`) instead of `&&` for conditional rendering when the condition can be `0`, `NaN`, or other falsy values that render.
**Incorrect (renders "0" when count is 0):**
```tsx
functionBadge({count}:{count: number}){
return(
<div>
{count&&<spanclassName="badge">{count}</span>}
</div>
)
}
// When count = 0, renders: <div>0</div>
// When count = 5, renders: <div><span class="badge">5</span></div>
This is especially helpful for large and static SVG nodes, which can be expensive to recreate on every render.
**Note:** If your project has [React Compiler](https://react.dev/learn/react-compiler) enabled, the compiler automatically hoists static JSX elements and optimizes component re-renders, making manual hoisting unnecessary.
When rendering content that depends on client-side storage (localStorage, cookies), avoid both SSR breakage and post-hydration flickering by injecting a synchronous script that updates the DOM before React hydrates.
var theme = localStorage.getItem('theme') || 'light';
var el = document.getElementById('theme-wrapper');
if (el) el.className = theme;
} catch (e) {}
})();
`,
}}
/>
</>
)
}
```
The inline script executes synchronously before showing the element, ensuring the DOM already has the correct value. No flickering, no hydration mismatch.
This pattern is especially useful for theme toggles, user preferences, authentication states, and any client-only data that should render immediately without flashing default values.
Reduce SVG coordinate precision to decrease file size. The optimal precision depends on the viewBox size, but in general reducing precision should be considered.
**Incorrect (excessive precision):**
```svg
<pathd="M 10.293847 20.847362 L 30.938472 40.192837"/>
When updating state based on the current state value, use the functional update form of setState instead of directly referencing the state variable. This prevents stale closures, eliminates unnecessary dependencies, and creates stable callback references.
**Incorrect (requires state as dependency):**
```tsx
functionTodoList() {
const[items,setItems]=useState(initialItems)
// Callback must depend on items, recreated on every items change
The first callback is recreated every time `items` changes, which can cause child components to re-render unnecessarily. The second callback has a stale closure bug—it will always reference the initial `items` value.
**Correct (stable callbacks, no stale closures):**
```tsx
functionTodoList() {
const[items,setItems]=useState(initialItems)
// Stable callback, never recreated
constaddItems=useCallback((newItems: Item[])=>{
setItems(curr=>[...curr,...newItems])
},[])// ✅ No dependencies needed
// Always uses latest state, no stale closure risk
1.**Stable callback references** - Callbacks don't need to be recreated when state changes
2.**No stale closures** - Always operates on the latest state value
3.**Fewer dependencies** - Simplifies dependency arrays and reduces memory leaks
4.**Prevents bugs** - Eliminates the most common source of React closure bugs
**When to use functional updates:**
- Any setState that depends on the current state value
- Inside useCallback/useMemo when state is needed
- Event handlers that reference state
- Async operations that update state
**When direct updates are fine:**
- Setting state to a static value: `setCount(0)`
- Setting state from props/arguments only: `setName(newName)`
- State doesn't depend on previous value
**Note:** If your project has [React Compiler](https://react.dev/learn/react-compiler) enabled, the compiler can automatically optimize some cases, but functional updates are still recommended for correctness and to prevent stale closure bugs.
Pass a function to `useState` for expensive initial values. Without the function form, the initializer runs on every render even though the value is only used once.
**Incorrect (runs on every render):**
```tsx
functionFilteredList({items}:{items: Item[]}){
// buildSearchIndex() runs on EVERY render, even after initialization
Use lazy initialization when computing initial values from localStorage/sessionStorage, building data structures (indexes, maps), reading from the DOM, or performing heavy transformations.
For simple primitives (`useState(0)`), direct references (`useState(props.value)`), or cheap literals (`useState({})`), the function form is unnecessary.
**Note:** If your project has [React Compiler](https://react.dev/learn/react-compiler) enabled, manual memoization with `memo()` and `useMemo()` is not necessary. The compiler automatically optimizes re-renders.
Use Next.js's `after()` to schedule work that should execute after a response is sent. This prevents logging, analytics, and other side effects from blocking the response.
`React.cache()` only works within one request. For data shared across sequential requests (user clicks button A then button B), use an LRU cache.
**Implementation:**
```typescript
import{LRUCache}from'lru-cache'
constcache=newLRUCache<string,any>({
max: 1000,
ttl: 5*60*1000// 5 minutes
})
exportasyncfunctiongetUser(id: string){
constcached=cache.get(id)
if(cached)returncached
constuser=awaitdb.user.findUnique({where:{id}})
cache.set(id,user)
returnuser
}
// Request 1: DB query, result cached
// Request 2: cache hit, no DB query
```
Use when sequential user actions hit multiple endpoints needing the same data within seconds.
**With Vercel's [Fluid Compute](https://vercel.com/docs/fluid-compute):** LRU caching is especially effective because multiple concurrent requests can share the same function instance and cache. This means the cache persists across requests without needing external storage like Redis.
**In traditional serverless:** Each invocation runs in isolation, so consider Redis for cross-process caching.
If you must pass objects, pass the same reference:
```typescript
constparams={uid: 1}
getUser(params)// Query runs
getUser(params)// Cache hit (same reference)
```
**Next.js-Specific Note:**
In Next.js, the `fetch` API is automatically extended with request memoization. Requests with the same URL and options are automatically deduplicated within a single request, so you don't need `React.cache()` for `fetch` calls. However, `React.cache()` is still essential for other async tasks:
- Database queries (Prisma, Drizzle, etc.)
- Heavy computations
- Authentication checks
- File system operations
- Any non-fetch async work
Use `React.cache()` to deduplicate these operations across your component tree.
The React Server/Client boundary serializes all object properties into strings and embeds them in the HTML response and subsequent RSC requests. This serialized data directly impacts page weight and load time, so **size matters a lot**. Only pass fields that the client actually uses.
description: Review UI code for Web Interface Guidelines compliance. Use when asked to "review my UI", "check accessibility", "audit design", "review UX", or "check my site against best practices".
metadata:
author: vercel
version: "1.0.0"
argument-hint: <file-or-pattern>
---
# Web Interface Guidelines
Review files for compliance with Web Interface Guidelines.
## How It Works
1. Fetch the latest guidelines from the source URL below
2. Read the specified files (or prompt user for files/pattern)
3. Check against all rules in the fetched guidelines
4. Output findings in the terse `file:line` format
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
