From fd2e26f8e6705af416154d4e3a5a4a0cdf348376 Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:13:37 +0800 Subject: [PATCH] refactor(agents): simplify repository context (#39583) --- .agents/skills/backend-code-review/SKILL.md | 176 ++------ .../references/architecture-rule.md | 5 +- .../references/db-schema-rule.md | 5 - .../references/repositories-rule.md | 1 - .../references/sqlalchemy-rule.md | 6 +- .../skills/e2e-cucumber-playwright/SKILL.md | 90 +--- .../references/cucumber-best-practices.md | 35 +- .../references/playwright-best-practices.md | 26 +- .agents/skills/frontend-code-review/SKILL.md | 110 ++--- .../references/component-architecture.md | 5 +- .../references/data-query-contracts.md | 2 +- .../references/dify-ui.md | 2 +- .agents/skills/frontend-testing/SKILL.md | 37 +- .../skills/how-to-write-component/SKILL.md | 155 ++----- .../how-to-write-component/references/data.md | 42 ++ .../references/interactions.md | 31 ++ .../references/ownership.md | 39 ++ .../references/runtime.md | 23 + .../references/state.md | 38 ++ .agents/skills/karpathy-guidelines/SKILL.md | 33 -- .claude/skills/component-refactoring | 1 - .claude/skills/karpathy-guidelines | 1 - AGENTS.md | 46 +- api/AGENTS.md | 237 +--------- api/controllers/API_SCHEMA_GUIDE.md | 3 + cli/AGENTS.md | 103 +---- cli/ARD.md | 114 ++--- cli/src/commands/AGENTS.md | 10 +- dify-agent/AGENTS.md | 189 +------- e2e/AGENTS.md | 403 ++---------------- packages/dify-ui/AGENTS.md | 106 +---- packages/dify-ui/README.md | 30 +- web/AGENTS.md | 59 +-- 33 files changed, 498 insertions(+), 1665 deletions(-) create mode 100644 .agents/skills/how-to-write-component/references/data.md create mode 100644 .agents/skills/how-to-write-component/references/interactions.md create mode 100644 .agents/skills/how-to-write-component/references/ownership.md create mode 100644 .agents/skills/how-to-write-component/references/runtime.md create mode 100644 .agents/skills/how-to-write-component/references/state.md delete mode 100644 .agents/skills/karpathy-guidelines/SKILL.md delete mode 120000 .claude/skills/component-refactoring delete mode 120000 .claude/skills/karpathy-guidelines diff --git a/.agents/skills/backend-code-review/SKILL.md b/.agents/skills/backend-code-review/SKILL.md index 35dc54173e4..66ab8dc6099 100644 --- a/.agents/skills/backend-code-review/SKILL.md +++ b/.agents/skills/backend-code-review/SKILL.md @@ -1,168 +1,40 @@ --- name: backend-code-review -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. +description: Use only when the user explicitly requests a review or audit of backend code under `api/`. Supports pending-change, file-focused, and pasted-diff reviews. Do not use for implementation-only requests, diagnosis without review intent, frontend code, or backend code outside `api/`. --- # Backend Code Review -## When to use this skill +Review the requested scope for concrete, reproducible defects. The nearest `AGENTS.md` owns package facts and commands; this skill owns the review workflow and routes to its bundled rule packs. -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: +## Evidence First -- **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`). +1. Establish the requested review scope and inspect the relevant diff or files. +2. Read the changed lines, their behavior owner, nearby tests, and local docstrings or comments that define contracts. +3. Trace callers, persistence boundaries, authorization, generated schemas, or external I/O only when they decide correctness. +4. Report only findings tied to an observable failure, violated contract, security boundary, data integrity risk, or demonstrated maintenance problem. -Do NOT use this skill when: +## Rule Routing -- 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/`). +Read only the packs matched by the diff: -## How to use this skill +- Models or migrations: [`references/db-schema-rule.md`][db-schema] +- Controller, service, core/domain, library, or model dependency direction: [`references/architecture-rule.md`][architecture] +- Table access outside an established repository boundary: [`references/repositories-rule.md`][repositories] +- SQLAlchemy sessions, queries, transactions, CRUD, concurrency, or raw SQL: [`references/sqlalchemy-rule.md`][sqlalchemy] -Follow these steps when using this skill: +When no pack applies, review correctness, security, behavior changes, and test evidence directly. Check current official documentation only when local code and contracts do not settle framework or library behavior. -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**. +## Severity And Output -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. +- **P0**: security or privacy exposure, data loss, or a production-wide outage. +- **P1**: user-visible regression, broken authorization or tenant isolation, invalid public contract, or failed primary workflow. +- **P2**: concrete correctness, performance, maintainability, or test defect likely to cause incorrect behavior. +- **P3**: minor actionable cleanup; omit unless the user requested a thorough audit. -## Checklist +Lead with findings ordered by severity. Include a tight file and line reference, the failing contract or reproduction path, impact, and a concrete fix direction. If there are no findings, say `No issues found.` and state any material verification gap. Do not add praise sections, speculative risks, or an unsolicited offer to implement fixes. -- 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 critical issues need to be fixed: - -## 🔴 Critical (Must Fix) - -### 1. - -FilePath: line - - -#### Explanation - - - -#### Suggested Fix - -1. -2. (optional, omit if not applicable) - ---- -... (repeat for each critical issue) ... - -Found suggestions for improvement: - -## 🟡 Suggestions (Should Consider) - -### 1. - -FilePath: line - - -#### Explanation - - - -#### Suggested Fix - -1. -2. (optional, omit if not applicable) - ---- -... (repeat for each suggestion) ... - -Found optional nits: - -## 🟢 Nits (Optional) -### 1. - -FilePath: line - - -#### Explanation - - - -#### Suggested Fix - -- - ---- -... (repeat for each nits) ... - -## ✅ What's Good - -- -``` - -- 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?" - -### Template B (no issues) - -```markdown -## Code Review Summary -✅ No issues found. -``` \ No newline at end of file +[architecture]: references/architecture-rule.md +[db-schema]: references/db-schema-rule.md +[repositories]: references/repositories-rule.md +[sqlalchemy]: references/sqlalchemy-rule.md diff --git a/.agents/skills/backend-code-review/references/architecture-rule.md b/.agents/skills/backend-code-review/references/architecture-rule.md index c3fd08bf033..02ee49d8fd9 100644 --- a/.agents/skills/backend-code-review/references/architecture-rule.md +++ b/.agents/skills/backend-code-review/references/architecture-rule.md @@ -7,7 +7,6 @@ ### Keep business logic out of controllers - Category: maintainability -- Severity: critical - 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: @@ -34,7 +33,6 @@ ### Preserve layer dependency direction - Category: best practices -- Severity: critical - 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: @@ -58,7 +56,6 @@ ### 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. @@ -88,4 +85,4 @@ def should_archive_conversation(conversation, tenant_id: str) -> bool: threshold_days = 90 if has_paid_plan(tenant_id) else 30 return older_than_days(conversation.idle_days, threshold_days) - ``` \ No newline at end of file + ``` diff --git a/.agents/skills/backend-code-review/references/db-schema-rule.md b/.agents/skills/backend-code-review/references/db-schema-rule.md index 8feae2596a1..8f922bc3dc0 100644 --- a/.agents/skills/backend-code-review/references/db-schema-rule.md +++ b/.agents/skills/backend-code-review/references/db-schema-rule.md @@ -8,7 +8,6 @@ ### 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. @@ -41,7 +40,6 @@ ### 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. @@ -70,7 +68,6 @@ ### Detect and avoid duplicate/redundant indexes - Category: performance -- Severity: suggestion - 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. @@ -94,7 +91,6 @@ ### 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. @@ -122,7 +118,6 @@ ### 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. diff --git a/.agents/skills/backend-code-review/references/repositories-rule.md b/.agents/skills/backend-code-review/references/repositories-rule.md index 555de98eb04..c0f16a21282 100644 --- a/.agents/skills/backend-code-review/references/repositories-rule.md +++ b/.agents/skills/backend-code-review/references/repositories-rule.md @@ -8,7 +8,6 @@ ### 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. diff --git a/.agents/skills/backend-code-review/references/sqlalchemy-rule.md b/.agents/skills/backend-code-review/references/sqlalchemy-rule.md index cda3a5dc98d..2ed3be4bbb5 100644 --- a/.agents/skills/backend-code-review/references/sqlalchemy-rule.md +++ b/.agents/skills/backend-code-review/references/sqlalchemy-rule.md @@ -8,7 +8,6 @@ ### 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. @@ -47,7 +46,6 @@ ### 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. - Example: @@ -67,7 +65,6 @@ ### 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: @@ -89,7 +86,6 @@ ### 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. @@ -136,4 +132,4 @@ ).scalar_one() run.status = "cancelled" session.commit() - ``` \ No newline at end of file + ``` diff --git a/.agents/skills/e2e-cucumber-playwright/SKILL.md b/.agents/skills/e2e-cucumber-playwright/SKILL.md index 5762bf2076d..62c2012dce8 100644 --- a/.agents/skills/e2e-cucumber-playwright/SKILL.md +++ b/.agents/skills/e2e-cucumber-playwright/SKILL.md @@ -1,87 +1,31 @@ --- name: e2e-cucumber-playwright -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. +description: Use when writing, changing, or reviewing Cucumber and Playwright tests under `e2e/`, including feature files, step definitions, support code, scenario tags, locators, and assertions. Do not use for Vitest, React Testing Library, backend tests, or generic browser automation outside the E2E suite. --- -# Dify E2E Cucumber + Playwright +# E2E Cucumber And Playwright -Use this skill for Dify's repository-level E2E suite in `e2e/`. Use [`e2e/AGENTS.md`](../../../e2e/AGENTS.md) as the canonical package guide for local architecture and conventions, then read any feature-scoped `AGENTS.md` that owns the target area. Apply Playwright/Cucumber best practices only where they fit the current suite. +`e2e/AGENTS.md` owns the suite architecture, lifecycle, commands, tags, generated-client boundaries, fixtures, and cleanup contracts. Read the nearest feature-scoped `AGENTS.md` when one exists. This skill adds no parallel package policy. -## Scope +## Topic Routing -- 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/`. +Read only the bundled reference required by the change: -## Read Order +- Locator, assertion, isolation, or waiting decisions: [`references/playwright-best-practices.md`][playwright] +- Scenario wording, step granularity, expressions, or tag design: [`references/cucumber-best-practices.md`][cucumber] -1. Read [`e2e/AGENTS.md`](../../../e2e/AGENTS.md) first. -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. - -Keep this skill focused on Cucumber, Playwright, and package-level E2E guidance. Put feature-specific conventions in the owning feature's `AGENTS.md` instead of adding them here. - -## 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. -- Treat `e2e/AGENTS.md`, `features/support/hooks.ts`, and the Cucumber configuration as the owners of current session and tag semantics. Verify them when behavior depends on session state instead of copying a tag inventory into this skill. -- Do not import Playwright Test runner patterns that bypass the current Cucumber + `DifyWorld` architecture unless the task is explicitly about changing that architecture. -- Perform the behavior under test through Playwright. APIs are allowed for setup, seed preparation, persistence polling, and cleanup, but ordinary Console JSON and representable multipart operations must use the scenario- or process-owned generated oRPC client with request and response validation enabled. Keep the setup/cleanup API identity independent from an unauthenticated or logged-out behavior browser. -- Consume generated operations directly. Do not add one-to-one API wrappers, handwritten endpoint URLs, response DTO casts, duplicate schemas, global mutable clients, or TanStack Query caching in Cucumber. Keep helpers only for real fixture construction, multi-operation orchestration, invariants, polling, derived test views, or protocol adapters. -- Keep SSE, binary, redirect-only, external-service, and readiness exceptions centralized under their protocol owner. A contract mismatch must fail and be fixed at the backend schema owner followed by regeneration; never weaken validation to make E2E pass. +Check current official Playwright or Cucumber documentation before introducing a framework pattern that local code and references do not already establish. ## 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. - - Before adding several similar steps, scan the target capability for an existing domain noun that can be parameterized without hiding behavior. - - 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}`. - - Use a bounded regex only when the accepted values are a small explicit domain set and Cucumber Expressions would make the Gherkin less natural. - - Do not create one-off steps for each case variant when the same domain action or outcome applies to named surfaces, modes, or resources. - - 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. - - Use `expect.poll` for API persistence, backend eventual consistency, captured browser events, or other non-DOM state; prefer locator assertions for DOM readiness and visible UI state. - - If a product element has real user-facing semantics but no accessible name, prefer fixing that accessible contract over adding a test id. -5. Validate narrowly. - - Run the narrowest tagged scenario or flow that exercises the change. - - Run the package-required static checks documented in `e2e/AGENTS.md`. - - Broaden verification only when the change affects hooks, tags, setup, or shared step semantics. +1. Add E2E coverage only for a critical user journey with a cross-boundary outcome that cheaper owner-level tests do not already prove. +2. Identify the user-visible behavior and its feature owner. Start from real product defaults and actor roles; setup may establish preconditions but must not manufacture the opposite state to make the scenario meaningful. +3. Read the target scenario, matching step definitions, and lifecycle files only when session or shared state matters. +4. Reuse an existing step when wording and behavior match; add one coherent scenario or step when they do not. +5. Keep browser actions and assertions at the public user boundary; keep setup, seed, polling, and cleanup at their package-defined owners. +6. Run the narrowest tagged scenario and package checks documented in `e2e/AGENTS.md`; broaden only for shared hooks, tags, or support changes. -## Review Checklist +For review requests, lead with reproducible correctness failures, flake sources, or demonstrated architecture drift. Report the behavior verified and any external-runtime, browser, or environment gap. -- 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? -- Does setup/cleanup use the generated client directly, with any remaining helper owning more than a one-to-one endpoint forward? -- Is every raw HTTP call a documented protocol or infrastructure exception rather than an ordinary Console operation? - -Lead findings with correctness, flake risk, and architecture drift. - -## References - -- [`references/playwright-best-practices.md`](references/playwright-best-practices.md) -- [`references/cucumber-best-practices.md`](references/cucumber-best-practices.md) +[cucumber]: references/cucumber-best-practices.md +[playwright]: references/playwright-best-practices.md diff --git a/.agents/skills/e2e-cucumber-playwright/references/cucumber-best-practices.md b/.agents/skills/e2e-cucumber-playwright/references/cucumber-best-practices.md index 06177faa5c7..a02e2e3b0e2 100644 --- a/.agents/skills/e2e-cucumber-playwright/references/cucumber-best-practices.md +++ b/.agents/skills/e2e-cucumber-playwright/references/cucumber-best-practices.md @@ -1,12 +1,12 @@ -# Cucumber Best Practices For Dify E2E +# Cucumber Best Practices -Use this reference when writing or reviewing Gherkin scenarios, step definitions, parameter expressions, and step reuse in Dify's `e2e/` suite. +Use this reference when writing or reviewing Gherkin scenarios, step definitions, parameter expressions, and step reuse. Official sources: -- https://cucumber.io/docs/guides/10-minute-tutorial/ -- https://cucumber.io/docs/cucumber/step-definitions/ -- https://cucumber.io/docs/cucumber/cucumber-expressions/ +- https://cucumber.io/docs/guides/10-minute-tutorial +- https://cucumber.io/docs/cucumber/step-definitions +- https://cucumber.io/docs/cucumber/cucumber-expressions ## What Matters Most @@ -24,11 +24,7 @@ Apply it like this: A scenario should usually prove one workflow or business outcome. If a scenario wanders across several unrelated behaviors, split it. -In Dify's suite, this means: - -- one capability-focused scenario per feature path -- no long setup chains when existing bootstrap or reusable steps already cover them -- no hidden dependency on another scenario's side effects +Keep each scenario centered on one coherent outcome. Avoid hidden dependencies on another scenario's side effects, and keep unavoidable setup outside the behavior narrative unless the precondition matters to the specification. ### 3. Reuse steps, but only when behavior really matches @@ -68,26 +64,11 @@ Use regex for a bounded natural-language alternative only when it keeps Gherkin Step definitions are glue between Gherkin and automation, not a second abstraction language. -For Dify: - -- type `this` as `DifyWorld` -- use `async function` -- keep each step to one user-visible action or assertion -- rely on `DifyWorld` and existing support code for shared context -- avoid leaking cross-scenario state +Keep each step to one user-visible action or assertion. In JavaScript and TypeScript, use `async function` when the step reads Cucumber World state because Cucumber binds `this`; do not leak state across scenarios through module globals. ### 6. Use tags intentionally -Tags should communicate run scope or session semantics, not become ad hoc metadata. - -In Dify's current suite: - -- capability tags group related scenarios -- `@unauthenticated` changes session behavior -- `@authenticated` is descriptive/selective, not a behavior switch by itself -- `@fresh` belongs to reset/full-install flows only - -If a proposed tag implies behavior, verify that hooks or runner configuration actually implement it. +Tags should communicate selection or execution intent, not become ad hoc metadata. A tag does not change runtime behavior unless configuration or hooks implement it. ## Review Questions diff --git a/.agents/skills/e2e-cucumber-playwright/references/playwright-best-practices.md b/.agents/skills/e2e-cucumber-playwright/references/playwright-best-practices.md index deb29e72430..e6709e00d75 100644 --- a/.agents/skills/e2e-cucumber-playwright/references/playwright-best-practices.md +++ b/.agents/skills/e2e-cucumber-playwright/references/playwright-best-practices.md @@ -1,6 +1,6 @@ -# Playwright Best Practices For Dify E2E +# Playwright Best Practices -Use this reference when writing or reviewing locator, assertion, isolation, or synchronization logic for Dify's Cucumber-based E2E suite. +Use this reference when writing or reviewing locator, assertion, isolation, or synchronization logic. Official sources: @@ -13,20 +13,19 @@ Official sources: ### 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`. +Playwright's model is built around clean browser contexts so one test does not leak into another. 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 +- keep scenario state in the runner's scenario-owned context rather than module globals +- model special authentication or session setup through explicit per-scenario fixtures rather than shared mutable state ### 2. Prefer user-facing locators Playwright recommends built-in locators that reflect what users perceive on the page. -Preferred order in this repository: +Preferred order: 1. `getByRole` 2. `getByLabel` @@ -79,16 +78,9 @@ 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 +### 5. Match debugging to the active harness -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. +Playwright supports traces, screenshots, page snapshots, and browser logs. Configure artifact capture at the runner boundary instead of adding parallel diagnostics to individual scenarios. ## Review Questions @@ -96,4 +88,4 @@ Use the existing artifact flow by default. If a task is specifically about impro - 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? +- Is a new abstraction really needed, or does it bypass the runner's scenario-owned context and lifecycle? diff --git a/.agents/skills/frontend-code-review/SKILL.md b/.agents/skills/frontend-code-review/SKILL.md index 85a8b1d9ef6..b5e262affc7 100644 --- a/.agents/skills/frontend-code-review/SKILL.md +++ b/.agents/skills/frontend-code-review/SKILL.md @@ -1,94 +1,48 @@ --- name: frontend-code-review -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. +description: Use only when the user explicitly requests a review or audit of frontend code under `web/` or `packages/dify-ui/`. Supports pending-change, file-focused, and pasted-diff reviews. Do not use for implementation-only requests, diagnosis without review intent, or backend-only code. --- # Frontend Code Review -## When To Use +Review the requested scope for concrete, reproducible regressions. This skill owns the review phase and routes directly to its bundled rule packs. For a combined review-and-fix request, establish findings before applying implementation or testing guidance. -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. +## Evidence First -Supported modes: +1. Establish the review scope from the requested files or current diff. +2. Read the changed lines, their behavior owner, and the nearest scoped `AGENTS.md`. +3. Trace public consumers, generated contracts, primitive APIs, or runtime configuration only when they decide correctness. +4. Report only findings tied to an observable failure, violated contract, security boundary, or demonstrated maintenance risk. -- **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. +## Rule Routing -Do not use this skill for backend-only code under `api/`; use `backend-code-review` instead. +Read only the packs matched by the diff: -## Required Context +- DOM semantics, focus, keyboard, forms, disabled state, or visible interaction: [`references/accessibility-ui.md`][accessibility] +- Dify UI imports, Base UI wrappers, overlays, tokens, or primitive contracts: [`references/dify-ui.md`][dify-ui] +- Component ownership, props, state, Effects, navigation, or module boundaries: [`references/component-architecture.md`][component-architecture] +- Generated clients, Query, mutations, auth, SSR, URL state, or persistence: [`references/data-query-contracts.md`][data-query] +- Test files or a concrete missing-regression-test finding: [`references/testing.md`][testing] +- Bundle, waterfall, rendering, or subscription cost supported by evidence: [`references/performance.md`][performance] +- Stable Dify runtime invariants in the named paths: [`references/dify-invariants.md`][dify-invariants] +- General TypeScript or styling quality not owned above: [`references/code-quality.md`][code-quality] -Before reviewing, read the relevant local contracts: +Read `packages/dify-ui/README.md`, `packages/dify-ui/AGENTS.md`, `web/docs/overlay.md`, or `web/docs/test.md` only when the reviewed code falls under that contract. Check current official documentation when local code and bundled references do not settle a framework, browser, or accessibility behavior. -- `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. -- `karpathy-guidelines` for scope control and focused, verifiable changes. -- `how-to-write-component` when reviewing React component structure, ownership, effects, query/mutation contracts, or memoization. +## Severity And Output -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: +- **P0**: security or privacy leak, data loss, production crash, or inaccessible critical workflow. +- **P1**: user-visible regression, invalid API or authorization contract, hydration failure, or broken primary interaction. +- **P2**: concrete maintainability, performance, test, or accessibility defect likely to cause incorrect behavior. +- **P3**: minor actionable cleanup; omit unless the user requested a thorough audit. -```text -https://raw.githubusercontent.com/vercel-labs/web-interface-guidelines/main/command.md -``` +Lead with findings ordered by severity. Include a tight file and line reference, the failing contract or reproduction path, impact, and a concrete fix direction. If there are no findings, say `No issues found.` and state any material verification gap. Do not add praise sections, speculative risks, or an unsolicited offer to implement fixes. -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/component-architecture.md](references/component-architecture.md) — component ownership, props, state, effects, exports, wrappers, and feature organization. -- [references/data-query-contracts.md](references/data-query-contracts.md) — generated contracts, TanStack Query, mutations, workspace/auth/SSR boundaries, URL/local storage state. -- [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. 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. - -## Severity - -- **P0**: security/privacy/auth leak, data loss, production crash, inaccessible critical flow, or broken primary workflow. -- **P1**: user-visible regression, hydration/SSR failure, invalid API/query contract, broken keyboard/focus behavior, or serious design-system/a11y violation. -- **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 - -Lead with findings, ordered by severity. Use this structure: - -```markdown -## Findings - -- [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. - -## Open Questions - -- Question or assumption, if any. - -## Summary - -Brief secondary context. Mention tests not run or residual risk. -``` - -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. +[accessibility]: references/accessibility-ui.md +[code-quality]: references/code-quality.md +[component-architecture]: references/component-architecture.md +[data-query]: references/data-query-contracts.md +[dify-invariants]: references/dify-invariants.md +[dify-ui]: references/dify-ui.md +[performance]: references/performance.md +[testing]: references/testing.md diff --git a/.agents/skills/frontend-code-review/references/component-architecture.md b/.agents/skills/frontend-code-review/references/component-architecture.md index 9f66533d215..ffabbf4cd69 100644 --- a/.agents/skills/frontend-code-review/references/component-architecture.md +++ b/.agents/skills/frontend-code-review/references/component-architecture.md @@ -46,14 +46,13 @@ When existing components already own interaction logic, prefer reusing or extend Flag: -- `React.FC` / `FC`. -- Default exports outside framework-required files. +- Declaration or export rewrites made only for stylistic uniformity, without changing an owned behavior or contract. - 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. +Do not flag `FC`, `React.FC`, function declarations, arrow functions, named exports, or default exports by syntax alone. Report them only when the chosen form causes a concrete type, lifecycle, export, framework, or enforced package-contract defect. ## Effects diff --git a/.agents/skills/frontend-code-review/references/data-query-contracts.md b/.agents/skills/frontend-code-review/references/data-query-contracts.md index c1dadfafaf6..db2e2017d27 100644 --- a/.agents/skills/frontend-code-review/references/data-query-contracts.md +++ b/.agents/skills/frontend-code-review/references/data-query-contracts.md @@ -12,7 +12,7 @@ Flag: - 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. +Backend Pydantic and OpenAPI schemas own API shape. Generated clients and schemas under `packages/contracts/generated/*` are authoritative at frontend boundaries and use the `{ params, query?, body? }` input shape. ## Queries diff --git a/.agents/skills/frontend-code-review/references/dify-ui.md b/.agents/skills/frontend-code-review/references/dify-ui.md index 27eced33b6a..93484a6a0fc 100644 --- a/.agents/skills/frontend-code-review/references/dify-ui.md +++ b/.agents/skills/frontend-code-review/references/dify-ui.md @@ -122,7 +122,7 @@ Flag: - 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 the Figma radius mapping from `packages/dify-ui/README.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. diff --git a/.agents/skills/frontend-testing/SKILL.md b/.agents/skills/frontend-testing/SKILL.md index cb71dc14772..168a3906c16 100644 --- a/.agents/skills/frontend-testing/SKILL.md +++ b/.agents/skills/frontend-testing/SKILL.md @@ -1,35 +1,16 @@ --- name: frontend-testing -description: Write, update, or review Dify frontend tests using Vitest and Testing Library. Trigger for frontend specs, test coverage requests, regressions, testability, or testing strategy under web/ or packages/dify-ui/. +description: Use when writing or changing Vitest or React Testing Library tests under `web/` or `packages/dify-ui/`, or when the user explicitly requests frontend test strategy, including evaluation of an existing strategy. Do not use for frontend code-review-only requests, general testability discussion, Python tests, or Cucumber/Playwright E2E. --- -# Dify Frontend Testing +# Frontend Testing -Use this skill for Vitest work under `web/` and `packages/dify-ui/`. Do not use it for Python tests or Cucumber/Playwright tests under `e2e/`. +`web/docs/test.md` is the single policy owner. Read it before changing frontend tests; this skill adds no separate requirements. -## Required Source +1. Identify the observable contract and regression risk. +2. Choose the smallest boundary that includes the behavior owner. +3. Establish the failing case first when practical, then implement one coherent scenario. +4. Run the focused spec before the affected suite and relevant static checks. +5. Report the behavior verified and any remaining browser, visual, or end-to-end risk. -Before writing, changing, or reviewing frontend tests, read `web/docs/test.md` completely. It is the single source of truth. This skill provides an execution checklist and must not redefine or extend that policy. - -## Workflow - -1. Read the source, its behavior owner, nearby specs, and relevant public dependencies. -1. Apply the canonical guide to decide whether a test is needed and choose its boundary. -1. For a behavior change or bug fix, write or identify the failing scenario first when practical. -1. Implement one coherent scenario at a time and run the focused spec before expanding scope. -1. Finish with the affected suite and relevant repository checks. -1. Report what behavior was verified and any risk that still requires browser, visual, or end-to-end validation. - -When reviewing existing tests, recommend deleting low-value tests as readily as adding missing behavior coverage. - -Run focused tests from the owning workspace: - -```bash -# web/ -vp test run path/to/spec-or-directory - -# packages/dify-ui/ -vp test run --project unit src/path/to/spec -``` - -Run Dify UI Storybook tests with `vp test --project storybook --run`. Run broader checks only after the focused behavior passes. +Recommend deleting low-value tests as readily as adding missing behavior coverage. Use `web/docs/test.md` for policy and Web commands; use the `packages/dify-ui/README.md` Development section for Dify UI commands. diff --git a/.agents/skills/how-to-write-component/SKILL.md b/.agents/skills/how-to-write-component/SKILL.md index 3572d03e757..f9c0a2ccc64 100644 --- a/.agents/skills/how-to-write-component/SKILL.md +++ b/.agents/skills/how-to-write-component/SKILL.md @@ -1,144 +1,41 @@ --- name: how-to-write-component -description: Use when writing, refactoring, or reviewing React/TypeScript components in Dify web, especially decisions about component ownership, props/types, URL/query state, Jotai state, async state, generated API contracts, queries/mutations, overlays, effects, navigation, performance, and empty states. +description: Use when implementing or refactoring React/TypeScript components and the task requires decisions about component ownership, feature boundaries, state, data flow, effects, or interaction ownership. Do not use for review-only requests, test-only work, copy-only edits, or styling-only changes. --- # How To Write A Component -Use this as the component decision guide for Dify web. Existing code is reference material, not automatic precedent; if touched code violates these rules, adapt it and fix equivalent patterns in the same feature branch. +Use this skill to route component architecture decisions to its bundled references. Read only the references required by the current change. ## First Decisions -| Question | Default | Promote or extract only when | +| Question | Default | Promote only when | | --- | --- | --- | -| Where should code live? | Keep it local to the feature workflow, route, or owner. | Multiple verticals need the same stable primitive. | -| How should route/tab folders be named? | Match the current route segment, tab name, or user-visible surface. | Keep a historical or broader parent only when it still owns multiple surfaces. | -| Who owns state, data, and handlers? | The lowest component that uses them. | A parent coordinates shared loading, errors, empty UI, selection, submission, navigation, or one consistent snapshot. | -| Should this become Jotai state? | Keep synchronous UI/form state in component or DOM state. | Siblings need one source of truth, the value drives atoms, or scoped workflow state must survive hidden/unmounted steps. | -| Should URL state enter Jotai? | Let Next.js route params and `nuqs` own URL state and updates. | Query atoms or shared derived atoms need a read-only bridge hydrated at the route/surface boundary. | -| Should this query/mutation become an atom? | Use TanStack Query hooks at the lowest owner. | It reads atom state, feeds derived atoms, or participates in shared Jotai workflow orchestration. | -| Should this be a helper/wrapper? | Prefer direct readable code at the use site. | The name captures a stable domain rule or the wrapper owns real behavior, validation, state, error handling, or semantics. | -| Where should a hotkey live? | Keep a single-owner hotkey constant in its component. | Multiple production files share one command, or the feature owns a real command registry with shared metadata and behavior. | -| Is an Effect needed? | No. Derive during render or handle the user action in the event handler. | It synchronizes with an external system such as browser APIs, subscriptions, timers, analytics, or imperative DOM/non-React widgets. | +| Where should code live? | In the product workflow, route, or feature owner. | Several verticals need the same stable contract. | +| Who owns state and handlers? | The lowest visual owner that consumes them. | A parent coordinates one workflow or consistent snapshot. | +| Should state enter Jotai? | Keep component and form state local. | Siblings need one source of truth or scoped workflow persistence. | +| Who owns URL state? | Next.js route APIs and `nuqs`. | Atoms require a read-only route-identity bridge. | +| Who owns remote state? | TanStack Query at the lowest consumer. | Atom state drives the query or shared derivations consume it. | +| Is a wrapper needed? | Use the primitive or direct code. | The wrapper owns behavior, validation, state, or semantics. | +| Is an Effect needed? | Derive during render or handle the user action. | A named external system must be synchronized. | -## Core Defaults +## Topic Routing -- Search before adding UI, hooks, helpers, query utilities, or styling patterns. Reuse existing base components, feature components, hooks, utilities, and design styles when they fit. -- 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 choices. -- Preserve visible keyboard focus states on the final focusable element. Prefer styled `@langgenius/dify-ui/*` controls when available, because components such as `Button` and form/control primitives carry the standard Dify UI `focus-visible` styling. Do not assume every Dify UI export provides visual focus styles: headless anatomy parts and direct Base UI re-exports such as dialog/popover/tooltip/drawer triggers usually only provide behavior and semantics. When using native `button` / `a`, custom trigger `render` props, clickable rows, icon buttons, menu-like items, or direct trigger parts, verify the rendered focusable element has a visible focus state. If it does not, add the standard Dify UI focus style: `outline-hidden focus-visible:ring-2 focus-visible:ring-state-accent-solid`. Do not hide outlines without an equivalent visible `focus-visible` indicator. Component-specific focus styles should follow an existing styled primitive pattern or a concrete design constraint, not a new ad hoc style. -- Group feature code by workflow, route, or ownership area with route-aligned names: components, hooks, local types, query helpers, atoms, constants, tests, and small utilities should live near the code that changes with them. -- For each feature module, keep a module-local `README.md` as a boundary note. Start with the module name, a brief one-sentence description, then split dependencies into `Internal Modules` and `External Modules` sections; keep both sections and write `None.` when one category is empty. `Internal Modules` lists modules inside the same overall feature using paths from that feature root, such as `shared/domain/runtime-status`; `External Modules` lists project modules outside the feature using paths from the web root without a `web/` prefix, such as `app/components/base/skeleton`. Omit npm packages, workspace package dependencies, and whitelisted plumbing modules. Do not copy caller-relative import paths into the README. -- Module README whitelist: `@/service/client`, `@/next/*`. -- Keep source/default selection, validation, dirty checks, and payload shaping close to the workflow that owns submit behavior. Do not hide flow-specific priority order, fallback behavior, or submit semantics in generic utilities. -- Prefer direct conditionals for small branch-specific decisions, especially form source selection and request payload assembly. -- Loading states for page sections, cards, lists, tables, forms, and drawers should be skeletons scoped to the content being loaded. Use spinners only for small inline busy indicators. +- Component moves, module boundaries, props, types, or owner placement: read [`references/ownership.md`][ownership]. +- Jotai, form drafts, route identity, URL state, or persistence: read [`references/state.md`][state]. +- Generated contracts, nullable API data, Query, mutations, SSR, auth, or workspace state: read [`references/data.md`][data]. +- Hotkeys, focus, dialogs, menus, popovers, or other secondary surfaces: read [`references/interactions.md`][interactions] and the overlay guide it references when applicable. +- Effects, navigation, memoization, preloading, or render cost: read [`references/runtime.md`][runtime]. -## Layout And Ownership +## Workflow -- State-heavy wizards, drawers, modals, and secondary workflows can be a small feature surface: an entry file, one feature-local state file when Jotai is actually needed, and shallow `ui/` owners that match real visual regions. -- The entry file handles route integration, provider wiring, close behavior, and surface mounting. The composition owner handles high-level workflow branching. The closest visual owner handles section branching. -- When a page or tab maps to a route segment, name its feature folder after that route/tab surface instead of a stale parent grouping. Remove misleading intermediate folders when only one surface remains. -- When a tab folder grows into several independent sections or action areas, split the first level by product/visual owners. Keep the root for the entry component and cross-owner state, colocate tests with the owner folder, and put truly shared local UI under a specifically named `components/` file. -- Repeated TanStack query calls in sibling components are acceptable when each component independently consumes the data; TanStack Query deduplicates and shares cache. -- Pass stable domain identity across boundaries. Do not forward derived presentation state when the receiver can derive it from its own data source. -- A component that owns a visual surface should also own data access, loading, empty, and error states for content rendered inside it unless a parent truly coordinates that state. -- 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 flow. -- Do not replace prop drilling with one large view-model hook threaded through section props. Move each hook, query, derived value, and handler to the concrete section that consumes it. -- Keep callbacks in a parent only for workflow coordination such as form submission, shared selection, batch behavior, or navigation. Otherwise let the child, menu, or row own the action. +1. Identify the behavior owner and the public contract being changed. +2. Read the nearby implementation, tests, and only the routed skill references. +3. Implement one coherent vertical slice. Do not expand into equivalent patterns elsewhere unless the current contract cannot be completed without them. +4. Verify observable behavior at the narrowest sufficient boundary, then run the checks documented by the owning package: `web/docs/test.md` or `web/docs/lint.md` for Web, and the `packages/dify-ui/README.md` Development section for Dify UI. -## Feature-Scoped Jotai - -- A Jotai-backed feature has one feature-local state file for shared primitive atoms, query atoms, derived atoms, write-only actions, mutation atoms, submission orchestration, provider exports, and optional scope configuration. -- Keep component-owned synchronous UI state local even inside Jotai features: dialog open flags, menus/popovers, confirmations, field drafts, and selected local options usually belong in component state. -- Use uncontrolled `@langgenius/dify-ui/form` and `@langgenius/dify-ui/field` controls for edit/create forms whose fields are read only at submit time. Initialize query-backed defaults with `defaultValue` and keyed remounts. -- Promote form state to atoms only when another component must react to in-progress values, a draft must survive unmount/remount in the scoped workflow, or multiple steps share the same editable draft before submit. -- Treat `useParams`, route args, and `nuqs` query state as framework-owned state. When atom logic needs those values, hydrate primitive atoms at the route or surface boundary, such as with `useHydrateAtoms(..., { dangerouslyForceHydrate: true })`; keep URL updates in the route/query-state APIs instead of write atoms. -- Within a route-owned feature, choose one source for route identity. If route params are bridged into feature atoms, use that bridge consistently for route-derived queries and actions instead of also threading the same route id through page, tab, and section props. -- For async work tied to atom state, use `atomWithQuery` or `atomWithMutation`; write atoms should update only the inputs that drive those atoms. This applies to pure frontend async work as well as network requests, so do not hand-roll loading/error/in-flight state with `useState` or `useRef` for atom-orchestrated async behavior. For component-owned remote work, use `useQuery` or `useMutation` directly. -- `jotai-tanstack-query` query atoms do not support TanStack Query tracked properties. A component that reads `useAtomValue(queryAtom)` subscribes to the whole query result, even if it only accesses `data`, `isLoading`, or `isError`. Export field-specific derived atoms and have components read the exact fields they render; use `selectAtom(queryAtom, result => result.field)` for query-result fields so unchanged selections do not notify subscribers. Keep direct `useAtomValue(queryAtom)` only when the component or hook genuinely needs the full observer result. -- Row-local async state belongs to the row owner unless it participates in a shared Jotai workflow or needs atom-scoped reset semantics. -- Leave query and mutation atoms unscoped so they keep shared QueryClient cache and invalidation behavior. Scope resettable primitives and explicit hydration tuples; scope a derived atom only when every dependency should be private to that surface. -- For scoped primitives that are always hydrated by `ScopeProvider`, prefer `atomWithLazy(() => { throw new Error(...) })` when consumers should see a non-null type. -- Order state files by dependency graph: types/constants, primitives, query atoms, query-data derived atoms, business/readiness derived atoms, write actions, mutation atoms, submission orchestration, provider exports. -- Name derived atoms as business facts and write atoms as user or workflow commands. Components should read or write the exact atom they need with `useAtomValue` or `useSetAtom`. -- Menu/dialog `open` state usually stays local, but a scoped atom is acceptable when a composed menu plus secondary surface would otherwise pass confusing `open`/`onClose` props through unrelated layers. Scope that primitive with the surface instance so reset behavior stays local. -- Keep independent dialog lifecycles separate. Avoid one discriminated "current action dialog" atom when dialogs have separate open state, loading guards, or reset behavior. - -## 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. -- Avoid barrel files that only re-export secondary owners. `index.tsx` is acceptable for a route/tab entry component; import header controls, switches, sections, and row owners from their concrete owner 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 and one-off UI extensions beside the component that needs them. -- Preserve domain value types for selection components. Do not widen enum, union, boolean, numeric, object, or nullable select/radio values to `string`; keep wrappers and option value carriers typed from their feature option collection. -- Avoid `common.tsx` buckets for shared UI. Use a feature-local `components/` folder with concrete filenames that describe the shared role. -- Do not create type aliases that only rename another type. Use aliases only for real UI concepts, refinements, or reusable local contracts. -- Name values by their domain role and backend API contract, especially persistent IDs and route params. Normalize framework or route params at the boundary. -- Put fallback and invariant checks in the lowest component that already handles that state. Do not extract helpers whose only behavior is hiding missing display data. - -## Keyboard Shortcuts - -- Distinguish application commands from local keyboard semantics before choosing an API. Use `@tanstack/react-hotkeys` for application commands. Keep menu navigation, dialog Escape handling owned by a primitive, editor commands, and other widget-scoped ARIA interactions in their local component or primitive. -- Use `useHotkey` or `useHotkeys` for registered commands. For a command intentionally owned by an existing `onKeyDown`, use `matchesKeyboardEvent` instead of hand-written `metaKey` / `ctrlKey` parsing or a second global listener. -- Define a reusable string command with `satisfies Hotkey` and an object-form command with `satisfies RawHotkey`. Reserve `RegisterableHotkey` for API boundaries that intentionally accept either form. A one-time inline literal passed directly to TanStack is already type-checked; extract it when registration, display, metadata, or another production consumer needs the same source. -- Keep registered hotkeys distinct from held keys and display-only accelerators. Use `IndividualKey` with `useKeyHold` for held-key interactions, and use an explicitly named `displayKey` for local widget accelerators that are not registered `Hotkey` values. -- Keep registration and keycap/menu display derived from one canonical command. Do not maintain a hotkey string beside a separate `['Mod', ...]` display array. -- Keep a single-owner command constant in its owning component. Create a feature-local `hotkeys.ts` only when multiple production files consume the same command. Keep a dedicated definitions/registry module when a feature owns a real command system with IDs, metadata, alternate bindings, and centralized registration. Tests do not count as another production owner, and file-name uniformity alone is not a reason to extract. -- Make scope and availability explicit. Use `enabled` for business or surface lifecycle, `ignoreInputs` for whether input-like elements may trigger the command, and `target` when the command belongs to a concrete DOM subtree. Global application commands may use the document target; inline editors and composed overlays should prefer the actual editor or Base UI Popup ref when that owner is exposed. -- Put a scoped ref on the real behavior owner. Do not add a wrapper DOM element solely to obtain a hotkey target. If a shared overlay convenience component hides the Popup ref, either rely on its modal lifecycle/focus boundary when that is sufficient or design the primitive API separately; do not create a fake owner at the call site. -- Set `preventDefault` and `stopPropagation` according to the existing product behavior and browser interaction. Do not silently accept TanStack defaults when migrating from another listener if that changes typing, submission, or propagation semantics. -- Test observable command behavior, disabled/input/target scope, and the shared registration/display contract at the owning feature boundary. Prefer partial mocks that retain TanStack formatting and matching behavior when a registration boundary must be isolated. - -## Generated API And Nullable Data - -- Treat generated contracts as authoritative at API, query, mutation, cache, and service boundaries. For enterprise APIs, use `packages/contracts/generated/enterprise/*`. -- Do not hand-write DTO mirrors, widen generated fields/enums, or add parallel frontend enum/status layers unless they model product state not represented by the API. -- Use generated enum objects and union types directly in props, comparisons, status logic, and i18n keys. Presentation-only tone maps should be keyed by generated enums. -- Normalize or coerce only at real boundaries: user-entered forms, search, URL/query params, file names, DOM IDs, or legacy adapters. -- Do not coerce nullable or optional API strings to `''` in query, derived model, or payload-building code. Keep `null` or `undefined` until the final boundary requiring a string. -- Do not use `value || undefined` for mutation fields where `''` means "clear this value". Trim or normalize at the form boundary, then preserve intentional empty strings. -- Prefer nullable-tolerant render props for API-returned rows. Narrow only where a real value is required, such as mutation params, route hrefs, select values, query input, or required React keys. -- Build required values in the same branch that proves them, using `flatMap`, a local loop, or an early return. Avoid truthiness guards, `filter(Boolean)`, `filter(item => item.id)`, and `!` after filters. -- Use conditional spreads or explicit pushes for conditional array items instead of `undefined` placeholders followed by narrowing filters. -- Empty collection fallbacks are for not-yet-loaded query data or genuinely nullable collections at the owning render boundary, not for hiding required API fields. - -## Queries And Mutations - -- Keep `web/contract/*` as the API shape source of truth and follow the `{ params, query?, body? }` input shape. -- Consume generated queries with `useQuery(consoleQuery.xxx.queryOptions(...))` or `useQuery(marketplaceQuery.xxx.queryOptions(...))`. -- If a generated query input comes from an atom, including a route-identity bridge atom, keep the query in `atomWithQuery`; do not unwrap the atom in a component just to call `useQuery`. -- Consume owner-local mutations with `useMutation(consoleQuery.xxx.mutationOptions(...))` or `useMutation(marketplaceQuery.xxx.mutationOptions(...))` when pending/error state is not consumed by feature atoms. -- In `atomWithQuery`, `atomWithInfiniteQuery`, and `atomWithMutation`, return generated `queryOptions()`, `infiniteOptions()`, or `mutationOptions()` directly. Pass `enabled`, `retry`, `placeholderData`, `select`, and pagination options into the generated call instead of spreading options into a hand-built object. -- For generated oRPC options with missing required input, branch the whole input with `input: condition ? validInput : skipToken` and `enabled: Boolean(condition)`. Never place `skipToken` inside a nested placeholder payload or coerce required IDs to `''`. -- When prefetch and render use the same request, extract local query options or a query-options atom so `prefetchQuery` and `useQuery`/`atomWithQuery` share the exact options. -- For custom query or mutation functions, wrap options with TanStack `queryOptions(...)` or `mutationOptions(...)`. -- Do not extract generated `queryOptions(...)` into a helper solely to share input construction; extract only when prefetch/render must share exact options or the helper owns real domain behavior. -- Avoid pass-through hooks and thin `web/service/use-*` wrappers that only rename generated options. Keep feature hooks for real orchestration, workflow state, or shared domain behavior. -- Put shared cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`. Component or atom callbacks may handle local toasts, closing dialogs, and navigation, but should not replace shared invalidation or patch shared server state locally. -- For overlays that may open heavier secondary content, prefetch from the trigger/menu open event with `queryClient.prefetchQuery(queryOptions)` when `onOpenChange` is available. Do not mount hidden subscribers just to warm cache. -- Do not use deprecated `useInvalid` or `useReset`. -- Prefer `mutate(...)`; use `mutateAsync(...)` only when Promise semantics are required, and wrap awaited calls in `try/catch`. - -## Boundaries And Overlays - -- Use the first level below a page or tab to organize independent page sections when it adds structure or the root folder becomes noisy. This layer is layout/semantic first, not automatically the data owner. -- Treat component names, semantic roles, and user- or design-marked visual regions as boundary constraints. Keep adjacent UI as a sibling owner or introduce a correctly named broader owner. -- 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 when hidden content would obscure the parent. -- Preserve composability by separating behavior ownership from placement ownership: an action can own trigger/open/menu content while the caller owns slots, offsets, and alignment. -- When a dialog, dropdown, or popover accepts controlled `open`, mount it unconditionally unless unmounting is required for performance or reset semantics. Use keyed scope or local state reset instead of `{open && }` wrappers. -- When opening a dialog from a menu item, keep the menu and dialog as sibling surfaces. Let the menu command open the dialog, and mount the dialog outside menu popup content. -- For dialogs and alert dialogs, keep the root responsible for `open` wiring and put query/mutation hooks inside the content component when work should mount only after the overlay opens. -- Prefer uncontrolled overlay roots when the library can own open state. Use `onOpenChange` for side effects and CSS/data selectors for open-state styling. -- Avoid wrapper DOM unless it provides layout, semantics, accessibility, state ownership, or library integration. Avoid shallow wrappers, hook-to-props adapters, layout-only render props, children pass-through wrappers, and prop renaming unless they add real behavior or a real boundary. - -## Effects, Navigation, And Performance - -- Use Effects only to synchronize with external systems. Do not use Effects to transform props/state for rendering, handle user actions, copy state, reset state from props, or fetch data. -- For forms initialized from query data, prefer keyed remounts or surface-entry atom hydration over Effects that copy query data into form state. -- Prefer framework data APIs or TanStack Query for data fetching. -- Prefer `Link` for normal navigation. Use router APIs only for command-flow side effects such as mutation success, guarded redirects, or form submission. -- Before using `memo`, move changing state down to the smallest component that uses it. If state must wrap stable content, lift the stable content up and pass it as `children`. -- Avoid `memo`, `useMemo`, and `useCallback` unless there is a clear performance reason. +[data]: references/data.md +[interactions]: references/interactions.md +[ownership]: references/ownership.md +[runtime]: references/runtime.md +[state]: references/state.md diff --git a/.agents/skills/how-to-write-component/references/data.md b/.agents/skills/how-to-write-component/references/data.md new file mode 100644 index 00000000000..e151c2c0b5b --- /dev/null +++ b/.agents/skills/how-to-write-component/references/data.md @@ -0,0 +1,42 @@ +# Component Data And Queries + +Read this document when a component consumes generated contracts, nullable API values, TanStack Query, mutations, prefetching, authentication, or workspace state. + +## Generated Contracts + +- Treat generated contracts as authoritative at API, query, mutation, cache, and service boundaries. Enterprise APIs use `packages/contracts/generated/enterprise/*`. +- Backend Pydantic and OpenAPI schemas own API shape. Follow the generated `{ params, query?, body? }` input shape; when it is wrong, fix the backend schema and regenerate `packages/contracts/generated/*`. +- Do not hand-write DTO mirrors, widen generated fields or enums, edit generated output, or add a parallel frontend status layer unless it models product state absent from the API. +- Check deprecated markers, schema shape, and the actual consumer before assuming that a generated operation is ready to use. +- Normalize only at real boundaries such as user input, search, URL params, filenames, DOM IDs, or a required legacy adapter. +- Preserve `null`, `undefined`, and intentional empty strings until the final boundary. Do not use `value || undefined` when an empty string means clearing a field. +- Build required values in the branch that proves them. Avoid `filter(Boolean)`, truthiness filters, non-null assertions after filters, and placeholder values used only to satisfy types. + +## Queries + +- Use generated options directly with `useQuery(consoleQuery.xxx.queryOptions(...))`, `marketplaceQuery`, or the equivalent generated client. +- If query input comes from atom state, keep it in `atomWithQuery`; do not unwrap the atom in a component solely to call `useQuery`. +- For missing required input, branch the whole generated input with `skipToken`. Add `enabled` only for an independent execution condition; do not put `skipToken` inside a placeholder payload or coerce IDs to empty strings. +- Return generated `queryOptions()`, `infiniteOptions()`, or `mutationOptions()` directly from TanStack Query atoms. Pass supported options into the generated call instead of spreading into a parallel object. +- Share the exact options between prefetch and render when they represent the same request. Do not extract option helpers merely to reuse input construction. +- Avoid pass-through service hooks that only rename generated options. Keep feature hooks for actual orchestration or shared domain behavior. + +## Mutations And Cache + +- Use generated `mutationOptions()` directly for owner-local mutations. +- Put shared invalidation, retries, and cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`. Local callbacks may own toast, close, and navigation effects but must not replace shared cache policy. +- Prefer `mutate(...)`. Use `mutateAsync(...)` only when Promise composition is required, and catch awaited failures. +- Preserve intentional empty values and current list/detail ownership when updating data. Do not add optimistic updates without a verified owner contract. + +## Prefetch And Hidden Surfaces + +- Prefetch expensive secondary content from the trigger or menu-open event when it benefits the visible path. Do not mount hidden subscribers solely to warm the cache. +- `prefetchQuery` is cache warmup, not an authorization or availability gate. Use a hard fetch boundary when the server must decide whether rendering may proceed. + +## SSR, Authentication, And Workspace + +- Static configuration owns path-invariant routing. Request-dependent authentication, setup, role, and tenant decisions belong to SSR or runtime decision boundaries. +- Distinguish soft SSR cache warming from authoritative decisions. Prefetched or placeholder data must not grant access or represent successful availability. +- Never reuse tenant-scoped state after switching workspaces. Discard it at the switch boundary or isolate it by workspace identity. +- Do not make product or authorization decisions from bootstrap defaults. Wait for authoritative data, or render an explicit loading or error state. +- Keep loading and Suspense behavior inside the feature that owns the request. Do not add fake global data merely to bypass that boundary. diff --git a/.agents/skills/how-to-write-component/references/interactions.md b/.agents/skills/how-to-write-component/references/interactions.md new file mode 100644 index 00000000000..a89f2b88633 --- /dev/null +++ b/.agents/skills/how-to-write-component/references/interactions.md @@ -0,0 +1,31 @@ +# Component Interactions And Overlays + +Read this document when a change involves application hotkeys, focus, dialogs, menus, popovers, or other secondary surfaces. Overlay primitive selection and layering are owned by the [overlay guide]. + +## Focus And Semantics + +- Preserve a visible focus indicator on the final focusable element. Styled Dify UI controls usually provide it; headless anatomy parts and direct trigger exports may not. +- Native buttons, links, custom trigger renderers, clickable rows, icon controls, and menu-like items must retain their correct native semantics and accessible name. +- Do not hide an outline without an equivalent visible `focus-visible` treatment. Follow an existing Dify UI pattern rather than inventing a call-site style. + +## Keyboard Commands + +- Distinguish application commands from widget-local keyboard semantics. Use `@tanstack/react-hotkeys` for application commands; keep menu navigation, dialog Escape handling, editor behavior, and ARIA widget keys in their local primitive or owner. +- Use `useHotkey` or `useHotkeys` for registered commands. When an existing `onKeyDown` intentionally owns the command, use `matchesKeyboardEvent` rather than duplicating modifier parsing or adding another global listener. +- Keep registration and keycap or menu display derived from one canonical command. Distinguish registered commands, held keys, and display-only accelerators. +- Keep a single-owner command beside its component. Create a feature-local hotkey module only when several production files share it; tests alone do not justify extraction. +- Make availability and scope explicit with `enabled`, `ignoreInputs`, and `target`. Put a target ref on the actual behavior owner rather than creating wrapper DOM solely for hotkey scope. +- Preserve existing `preventDefault` and propagation behavior when migrating command APIs. +- Test observable command behavior, disabled and input scope, target scope, and the registration/display contract at the owning feature boundary. + +## Secondary Surfaces + +- Follow `web/docs/overlay.md` for primitive choice. Dify UI primitives are the default, with package-approved Web wrappers such as `Infotip` where the overlay guide allows them. +- Separate behavior ownership from placement ownership: the action may own trigger, open state, and menu content while the caller owns slots, offsets, and alignment. +- Keep menu and dialog surfaces as siblings when a menu command opens a dialog. Mount the dialog outside popup content. +- Mount controlled overlays unconditionally unless unmounting is required for performance or reset semantics. Prefer keyed or owner-local reset over conditional wrappers. +- Put query and mutation work inside dialog or alert-dialog content when it should mount only after opening. +- Prefer uncontrolled roots when the primitive can own open state. Use controlled state only for business coordination, analytics, cleanup, or explicit reset behavior. +- Do not add manual portals or call-site z-index escalation. Fix ownership and stacking structure at the shared boundary. + +[overlay guide]: ../../../../web/docs/overlay.md diff --git a/.agents/skills/how-to-write-component/references/ownership.md b/.agents/skills/how-to-write-component/references/ownership.md new file mode 100644 index 00000000000..d8da521aafc --- /dev/null +++ b/.agents/skills/how-to-write-component/references/ownership.md @@ -0,0 +1,39 @@ +# Component Ownership And Modules + +Read this document when adding, moving, splitting, or refactoring React components or feature modules. + +## Vertical Modules + +- Organize code by product workflow, route, or behavior owner. Keep components, hooks, local types, atoms, query helpers, tests, and small utilities beside the code that changes with them. +- Name page and tab folders after the current route, tab, or user-visible surface. Do not preserve stale parent groupings that no longer own multiple surfaces. +- Split a growing page or tab by product or visual owners. Keep the feature root for its public entrypoint and genuinely cross-owner coordination. +- Import other features only through explicit public entrypoints. Avoid barrels that merely re-export secondary owners. +- Promote code outside a feature only when multiple verticals use the same stable contract. Possible future reuse is not sufficient. + +## Component Ownership + +- Put state, data access, loading, empty, error, and handlers in the lowest visual owner that uses them. +- Keep coordination in a parent only when it needs one consistent snapshot or coordinates submission, shared selection, batch behavior, navigation, or cross-section loading and errors. +- Repeated TanStack Query calls in siblings are acceptable when each sibling independently consumes the data; the cache already deduplicates requests. +- Pass stable domain identity across boundaries. Do not pass raw server data together with separately derived flags for the same concept. +- One pass-through prop layer is acceptable. Repeated forwarding means ownership should move closer to the consumer or into feature-scoped shared state. +- Do not replace prop drilling with one large view-model hook. Move each query, derived value, and handler to the concrete owner that consumes it. +- Keep source selection, defaults, validation, dirty checks, and payload shaping beside the workflow that owns submission. + +## Boundaries + +- State-heavy wizards, drawers, modals, and secondary workflows can form a small vertical surface with an entrypoint, optional feature-local state, and shallow owners matching real visual regions. +- The entrypoint owns route integration, provider wiring, close behavior, and mounting. Composition owners handle workflow branches; the closest visual owner handles section branches. +- Separate hidden dialogs, dropdowns, and popovers into small local owners when their content obscures the parent flow. +- Keep cohesive forms, menu bodies, and one-off helpers local unless they have their own state, reuse, or semantic boundary. +- Avoid wrapper components and wrapper DOM that only rename props, pass children through, or hide the real primitive. A wrapper must own behavior, validation, state, accessibility, layout, or library integration. +- Loading states for page sections, cards, lists, tables, forms, and drawers should use skeletons scoped to the loaded content. Reserve spinners for small inline busy indicators. + +## Components And Types + +- Choose component declaration and export forms from the actual component contract, framework requirements, and enforced package rules. Existing style is context, not authority; do not rewrite unaffected code solely to normalize `FC`, `function`, arrow-function, named-export, or default-export forms. +- Type simple one-off props inline. Name a `Props` type when it is reused, exported, complex, or materially clearer. +- Use API-generated or API-returned types at component boundaries. Keep one-off UI refinements and conversions beside their owner. +- Preserve domain value types for selections. Do not widen enums, unions, booleans, numbers, objects, or nullable values to `string` before a real boundary requires it. +- Avoid generic `common.tsx` buckets and aliases that only rename another type. Name files, values, and public types after their domain role. +- Put fallback and invariant checks in the lowest component that already renders that state. Do not extract helpers whose only purpose is hiding missing display data. diff --git a/.agents/skills/how-to-write-component/references/runtime.md b/.agents/skills/how-to-write-component/references/runtime.md new file mode 100644 index 00000000000..c2b5a667355 --- /dev/null +++ b/.agents/skills/how-to-write-component/references/runtime.md @@ -0,0 +1,23 @@ +# Component Effects, Navigation, And Runtime Cost + +Read this document when a change introduces Effects, navigation side effects, memoization, preloading, or render-cost optimizations. + +## Effects + +- Use Effects only to synchronize with a named external system such as a browser API, subscription, timer, analytics integration, non-React widget, or imperative DOM API. +- Do not use Effects to transform render state, handle user actions, copy query data, reset state from props, or fetch data owned by framework APIs or TanStack Query. +- Initialize query-backed forms with keyed remounts or surface-entry hydration instead of copying data through Effects. + +## Navigation + +- Use `Link` for ordinary navigation. +- Use router APIs for command-flow side effects such as mutation success, guarded redirects, or form submission. +- Keep shareable navigation state in the URL rather than hidden component state. + +## Runtime Cost + +- Move changing state to the smallest consumer before considering memoization. Stable parent content can be lifted and passed as children. +- Avoid `memo`, `useMemo`, and `useCallback` unless identity or computation has a demonstrated consumer or measurable cost. +- Start independent remote work together and await it near the branch that consumes it. Avoid introducing request waterfalls. +- Load heavy optional surfaces on demand when they sit behind a dialog, tab, command, or feature activation. +- Use narrow selectors or field-level atoms for broad stores and subscriptions. Do not optimize simple primitive expressions merely for stylistic consistency. diff --git a/.agents/skills/how-to-write-component/references/state.md b/.agents/skills/how-to-write-component/references/state.md new file mode 100644 index 00000000000..0ede0a85e92 --- /dev/null +++ b/.agents/skills/how-to-write-component/references/state.md @@ -0,0 +1,38 @@ +# Component State And URL Ownership + +Read this document when a change involves Jotai, form drafts, route identity, shared client state, or local persistence. + +## Choose The Owner + +- Keep synchronous state local when one component owns it: dialog and menu state, confirmations, field drafts, and local selections usually belong to the component or DOM. +- Use feature-scoped Jotai when siblings need one source of truth, values drive other atoms, or a scoped workflow must preserve state across hidden or unmounted steps. +- Keep server and cache state in TanStack Query. Use existing feature stores for complex, high-frequency interaction state such as workflow canvas drag, resize, and runtime panels. +- Use feature-owned storage only for low-frequency client preferences, dismissed notices, and UI defaults. Live application state does not belong in local storage. + +## Forms + +- Prefer uncontrolled Dify UI form and field controls when values are only read at submit time. Initialize query-backed defaults with `defaultValue` and keyed remounts. +- Promote form values to atoms only when another owner reacts to in-progress values, the draft must survive scoped unmounting, or several workflow steps edit the same draft. +- Keep validation, source priority, fallback behavior, dirty checks, and payload assembly in the workflow that owns submission. + +## Route And URL State + +- Treat `useParams`, route arguments, and `nuqs` as the owners of URL identity and updates. +- Hydrate a primitive atom at the route or surface boundary only when query atoms or shared derived atoms require route identity. Keep URL writes in route and query-state APIs. +- Within one route-owned feature, choose one route-identity source. Do not hydrate route identity into atoms while also threading the same ID through multiple component layers. +- Put shareable filters, tabs, pagination, and search state in the URL. Keep one-shot navigation signals and transient UI state out of persistent subscriptions. + +## Jotai And Query + +- A Jotai-backed feature may keep one feature-local state module ordered by dependency: types and constants, primitives, query atoms, query-data derivations, business facts, commands, mutations, submission orchestration, and provider exports. +- Use `atomWithQuery` or `atomWithMutation` for async work driven by atom state. Do not hand-roll loading, error, or in-flight state for atom-orchestrated work. +- Use field-specific derived atoms for query results. `jotai-tanstack-query` does not provide TanStack Query tracked properties, so reading a whole query atom subscribes to the entire observer result. +- Leave query and mutation atoms unscoped so they retain the shared QueryClient cache. Scope resettable primitives and hydration tuples; scope a derived atom only when all dependencies should be private to the surface. +- Use non-null lazy primitives for values always hydrated by a scope provider. Name derived atoms as business facts and write atoms as user or workflow commands. +- Keep independent dialog lifecycles separate. A scoped open-state atom is acceptable only when composed sibling surfaces would otherwise pass confusing lifecycle props through unrelated owners. + +## Persistence + +- Use feature-owned storage modules built on `createLocalStorageState`; callers should not scatter direct storage access or raw keys. +- Persist high-frequency interaction state only on commit or after updates settle. +- Do not add ad hoc global event listeners for shared state. Centralize subscriptions through the owning atom, store, or subscription hook. diff --git a/.agents/skills/karpathy-guidelines/SKILL.md b/.agents/skills/karpathy-guidelines/SKILL.md deleted file mode 100644 index 2b7330f5b83..00000000000 --- a/.agents/skills/karpathy-guidelines/SKILL.md +++ /dev/null @@ -1,33 +0,0 @@ ---- -name: karpathy-guidelines -description: Lightweight coding guardrails for making focused, simple, and verifiable changes in this repo. Use for all coding work. ---- - -# Karpathy Guidelines - -Use this skill whenever you touch code in this repository. - -## Principles - -- Keep the change small and directly tied to the user request. -- Prefer the simplest implementation that fits the existing codebase. -- Read the nearby code first, then match its patterns. -- Avoid unrelated refactors, broad rewrites, or style churn. -- Preserve existing behavior unless the user explicitly asked to change it. -- Treat regressions as a signal to narrow the change, not to add workaround layers. - -## Workflow - -1. Inspect the current implementation and tests around the change. -2. Make the smallest coherent edit. -3. Add or update focused tests when the behavior changes or the risk is non-trivial. -4. Run the narrowest relevant verification first. -5. Report exactly what was verified and anything left unverified. - -## Review Checklist - -- Does this change solve the stated problem without expanding scope? -- Did it preserve existing route/component/data-flow semantics? -- Are new abstractions justified by real complexity? -- Are tests focused on the behavior that could regress? -- Are unrelated files and generated artifacts left alone? diff --git a/.claude/skills/component-refactoring b/.claude/skills/component-refactoring deleted file mode 120000 index 53ae67e2f2e..00000000000 --- a/.claude/skills/component-refactoring +++ /dev/null @@ -1 +0,0 @@ -../../.agents/skills/component-refactoring \ No newline at end of file diff --git a/.claude/skills/karpathy-guidelines b/.claude/skills/karpathy-guidelines deleted file mode 120000 index 743bef5277d..00000000000 --- a/.claude/skills/karpathy-guidelines +++ /dev/null @@ -1 +0,0 @@ -../../.agents/skills/karpathy-guidelines \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index 0da1d9e9eb6..046c2e1dcec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,46 +1,8 @@ # AGENTS.md -## Project Overview +Dify is an open-source platform for building LLM applications, agentic workflows, and RAG pipelines. This monorepo contains the backend API (`api/`), frontend application (`web/`), deployment assets (`docker/`), standalone agent backend (`dify-agent/`), CLI (`cli/`), and end-to-end suite (`e2e/`). Follow the nearest scoped `AGENTS.md` for the files being changed. -Dify is an open-source platform for developing LLM applications with an intuitive interface combining agentic AI workflows, RAG pipelines, agent capabilities, and model management. +## Repository Gotchas -The codebase is split into: - -- **Backend API** (`/api`): Python Flask application organized with Domain-Driven Design -- **Frontend Web** (`/web`): Next.js application using TypeScript and React -- **Docker deployment** (`/docker`): Containerized deployment configurations -- **Dify Agent Backend** (`/dify-agent`): Backend services for managing and executing agent - -## Backend Workflow - -- Read `api/AGENTS.md` for details -- Run backend CLI commands through `uv run --project api `. -- Integration tests are CI-only and are not expected to run in the local environment. - -## Frontend Workflow - -- Read `web/AGENTS.md` for details - -## Testing & Quality Practices - -- Follow TDD: red → green → refactor. -- Use `pytest` for backend tests with Arrange-Act-Assert structure. -- Enforce strong typing; avoid `Any` and prefer explicit type annotations. -- Write self-documenting code; only add comments that explain intent. - -## Language Style - -- **Python**: Keep type hints on functions and attributes, and implement relevant special methods (e.g., `__repr__`, `__str__`). Prefer `TypedDict` over `dict` or `Mapping` for type safety and better code documentation. -- **TypeScript**: Use the strict config, run `pnpm check` for formatting, Oxlint, ESLint non-code checks, and type checking, and avoid `any` types. - -## General Practices - -- Prefer editing existing files; add new documentation only when requested. -- Inject dependencies through constructors and preserve clean architecture boundaries. -- Handle errors with domain-specific exceptions at the correct layer. - -## Project Conventions - -- Backend architecture adheres to DDD and Clean Architecture principles. -- Async work runs through Celery with Redis as the broker. -- Frontend user-facing strings must use `web/i18n/en-US/`; avoid hardcoded text. +- Run backend commands through `uv run --project api `. +- Backend integration tests are CI-only and are not expected to run locally. diff --git a/api/AGENTS.md b/api/AGENTS.md index 474da7800b2..ac77301f8dd 100644 --- a/api/AGENTS.md +++ b/api/AGENTS.md @@ -1,232 +1,25 @@ # API Agent Guide -## Notes for Agent (must-check) +Read surrounding module, class, and function docstrings plus non-obvious comments before changing backend behavior. They are local contracts; update them only when their owned behavior changes, and keep them aligned with the current code. -Before changing any backend code under `api/`, you MUST read the surrounding docstrings and comments. These notes contain required context (invariants, edge cases, trade-offs) and are treated as part of the spec. +## Commands -Look for: +Run backend checks from the repository root: -- The module (file) docstring at the top of a source code file -- Docstrings on classes and functions/methods -- Paragraph/block comments for non-obvious logic - -### What to write where - -- Keep notes scoped: module notes cover module-wide context, class notes cover class-wide context, function/method notes cover behavioural contracts, and paragraph/block comments cover local “why”. Avoid duplicating the same content across scopes unless repetition prevents misuse. -- **Module (file) docstring**: purpose, boundaries, key invariants, and “gotchas” that a new reader must know before editing. - - Include cross-links to the key collaborators (modules/services) when discovery is otherwise hard. - - Prefer stable facts (invariants, contracts) over ephemeral “today we…” notes. -- **Class docstring**: responsibility, lifecycle, invariants, and how it should be used (or not used). - - If the class is intentionally stateful, note what state exists and what methods mutate it. - - If concurrency/async assumptions matter, state them explicitly. -- **Function/method docstring**: behavioural contract. - - Document arguments, return shape, side effects (DB writes, external I/O, task dispatch), and raised domain exceptions. - - Add examples only when they prevent misuse. -- **Paragraph/block comments**: explain *why* (trade-offs, historical constraints, surprising edge cases), not what the code already states. - - Keep comments adjacent to the logic they justify; delete or rewrite comments that no longer match reality. - -### Rules (must follow) - -In this section, “notes” means module/class/function docstrings plus any relevant paragraph/block comments. - -- **Before working** - - Read the notes in the area you’ll touch; treat them as part of the spec. - - If a docstring or comment conflicts with the current code, treat the **code as the single source of truth** and update the docstring or comment to match reality. - - If important intent/invariants/edge cases are missing, add them in the closest docstring or comment (module for overall scope, function for behaviour). -- **During working** - - Keep the notes in sync as you discover constraints, make decisions, or change approach. - - If you move/rename responsibilities across modules/classes, update the affected docstrings and comments so readers can still find the “why” and the invariants. - - Record non-obvious edge cases, trade-offs, and the test/verification plan in the nearest docstring or comment that will stay correct. - - Keep the notes **coherent**: integrate new findings into the relevant docstrings and comments; avoid append-only “recent fix” / changelog-style additions. -- **When finishing** - - Update the notes to reflect what changed, why, and any new edge cases/tests. - - Remove or rewrite any comments that could be mistaken as current guidance but no longer apply. - - Keep docstrings and comments concise and accurate; they are meant to prevent repeated rediscovery. - -## Coding Style - -This is the default standard for backend code in this repo. Follow it for new code and use it as the checklist when reviewing changes. - -### Linting & Formatting - -- Use Ruff for formatting and linting (follow `.ruff.toml`). -- Keep each line under 120 characters (including spaces). - -### Naming Conventions - -- Use `snake_case` for variables and functions. -- Use `PascalCase` for classes. -- Use `UPPER_CASE` for constants. - -### Typing & Class Layout - -- Code should usually include type annotations that match the repo’s current Python version (avoid untyped public APIs and “mystery” values). -- Prefer modern typing forms (e.g. `list[str]`, `dict[str, int]`) and avoid `Any` unless there’s a strong reason. -- For dictionary-like data with known keys and value types, prefer `TypedDict` over `dict[...]` or `Mapping[...]`. -- For optional keys in typed payloads, use `NotRequired[...]` (or `total=False` when most fields are optional). -- Keep `dict[...]` / `Mapping[...]` for truly dynamic key spaces where the key set is unknown. - -```python -from datetime import datetime -from typing import NotRequired, TypedDict - - -class UserProfile(TypedDict): - user_id: str - email: str - created_at: datetime - nickname: NotRequired[str] -``` - -- For classes, declare all member variables explicitly with types at the top of the class body (before `__init__`), even when the class is not a dataclass or Pydantic model, so the class shape is obvious at a glance: - -```python -from datetime import datetime - - -class Example: - user_id: str - created_at: datetime - - def __init__(self, user_id: str, created_at: datetime) -> None: - self.user_id = user_id - self.created_at = created_at -``` - -### General Rules - -- Use Pydantic v2 conventions. -- Use `uv` for Python package management in this repo (usually with `--project api`). -- Prefer simple functions over small “utility classes” for lightweight helpers. -- Avoid implementing dunder methods unless it’s clearly needed and matches existing patterns. -- Never start long-running services as part of agent work (`uv run app.py`, `flask run`, etc.); running tests is allowed. -- Keep files below ~800 lines; split when necessary. -- Keep code readable and explicit—avoid clever hacks. - -### Architecture & Boundaries - -- Mirror the layered architecture: controller → service → core/domain. -- Reuse existing helpers in `core/`, `services/`, and `libs/` before creating new abstractions. -- Optimise for observability: deterministic control flow, clear logging, actionable errors. - -### Owner-Bound Resource References - -- Resolve and validate the outer owner before binding a nested resource ID. -- For stable single-parent chains, use immutable nested `NamedTuple` refs. -- Root refs carry tenant plus root ID; child refs carry the parent ref. -- In production, construct refs through the domain ref service. -- Python allowing direct construction does not grant authorization. -- Scope every consuming query with complete owner predicates; refs are not security tokens. -- Keep polymorphic owners flat until explicit nominal owner types exist. -- Do not add generic ref bases or compatibility fields only for uniformity. -- Reconstruct internal refs from validated database state after payload or async boundaries. - -### Logging & Errors - -- Never use `print`; use a module-level logger: - - `logger = logging.getLogger(__name__)` -- Include tenant/app/workflow identifiers in log context when relevant. -- Raise domain-specific exceptions (`services/errors`, `core/errors`) and translate them into HTTP responses in controllers. -- Log retryable events at `warning`, terminal failures at `error`. - -### SQLAlchemy Patterns - -- Models inherit from `models.base.TypeBase`; do not create ad-hoc metadata or engines. -- Open sessions with context managers: - -```python -from sqlalchemy.orm import Session - -with Session(db.engine, expire_on_commit=False) as session: - stmt = select(Workflow).where( - Workflow.id == workflow_id, - Workflow.tenant_id == tenant_id, - ) - workflow = session.execute(stmt).scalar_one_or_none() -``` - -- Prefer SQLAlchemy expressions; avoid raw SQL unless necessary. -- Always scope queries by `tenant_id` and protect write paths with safeguards (`FOR UPDATE`, row counts, etc.). -- Introduce repository abstractions only for very large tables (e.g., workflow executions) or when alternative storage strategies are required. - -### Storage & External I/O - -- Access storage via `extensions.ext_storage.storage`. -- Use `core.helper.ssrf_proxy` for outbound HTTP fetches. -- Background tasks that touch storage must be idempotent, and should log relevant object identifiers. - -### Pydantic Usage - -- Define DTOs with Pydantic v2 models and forbid extras by default. -- Use `@field_validator` / `@model_validator` for domain rules. - -Example: - -```python -from pydantic import BaseModel, ConfigDict, HttpUrl, field_validator - - -class TriggerConfig(BaseModel): - endpoint: HttpUrl - secret: str - - model_config = ConfigDict(extra="forbid") - - @field_validator("secret") - def ensure_secret_prefix(cls, value: str) -> str: - if not value.startswith("dify_"): - raise ValueError("secret must start with dify_") - return value -``` - -### Generics & Protocols - -- Use `typing.Protocol` to define behavioural contracts (e.g., cache interfaces). -- Apply generics (`TypeVar`, `Generic`) for reusable utilities like caches or providers. -- Validate dynamic inputs at runtime when generics cannot enforce safety alone. - -### Tooling & Checks - -Quick checks while iterating: - -- Format: `make format` -- Lint (includes auto-fix): `make lint` +- Format and lint: `make lint` - Type check: `make type-check` - Unit tests: `make test` -- Full backend tests, including Docker-backed suites: `make test-all` -- Targeted tests: `make test TARGET_TESTS=./api/tests/` +- Targeted tests: `make test TARGET_TESTS=./api/tests/` -Before opening a PR / submitting: +Run direct Python commands through `uv run --project api`. Docker-backed integration suites are normally CI-owned. Do not start long-running services as part of routine agent work. -- `make lint` -- `make type-check` -- `make test` +## Architecture And Boundaries -### Controllers & Services - -- Controllers: parse input via Pydantic, invoke services, return serialised responses; no business logic. -- Services: coordinate repositories, providers, background tasks; keep side effects explicit. -- Document non-obvious behaviour with concise docstrings and comments. -- For `204 No Content` responses, return an empty body only; never return a dict, model, or other payload. -- For Flask-RESTX controller request, query, and response schemas, follow `controllers/API_SCHEMA_GUIDE.md`. - In short: use Pydantic models, document GET query params with `query_params_from_model(...)`, register response - DTOs with `register_response_schema_models(...)`, serialize response DTOs with `dump_response(...)`, - and avoid adding new legacy `ns.model(...)`, `@marshal_with(...)`, or GET `@ns.expect(...)` patterns. - -### System Features Contract - -- Treat the shared Console/Web `/system-features` response as a minimal unauthenticated bootstrap allowlist, not a - general configuration or feature-discovery endpoint. Existing fields do not establish precedent. -- Before adding a field, read `controllers/API_SCHEMA_GUIDE.md#public-system-features-contract` and provide evidence - that both Console and Web have production consumers that require it before authentication. -- Never place backend-only policy, surface-specific configuration, post-authentication state, speculative values, or - large/slow payloads in `SystemFeatureModel`. Use the consumer or domain owner described in the schema guide. -- Agents and reviewers must reject additions whose owner, public exposure, pre-authentication need, or root SSR cost - is not explicit. - -### Miscellaneous - -- Use `configs.dify_config` for configuration—never read environment variables directly. -- Maintain tenant awareness end-to-end; `tenant_id` must flow through every layer touching shared resources. -- Queue async work through `services/async_workflow_service`; implement tasks under `tasks/` with explicit queue selection. -- Keep experimental scripts under `dev/`; do not ship them in production builds. +- Keep transport parsing and serialization in controllers, orchestration in services, and domain policy in `core/` or its domain owner. Keep `libs/` business-agnostic and reuse existing owners before adding abstractions. +- Before changing controller schemas, generated API contracts, or `SystemFeatureModel`, read `controllers/API_SCHEMA_GUIDE.md`. Treat `/system-features` as a minimal unauthenticated bootstrap allowlist, not a general configuration registry. +- Scope tenant-owned reads and writes by the complete owner chain, and propagate `tenant_id` across every affected layer. Reconstruct trusted internal references from validated database state after payload or async boundaries. +- Keep write transactions explicit and bounded. Do not perform external I/O inside an open transaction unless a documented consistency contract requires it. +- Read configuration through `configs.dify_config`, access storage through `extensions.ext_storage.storage`, and route outbound HTTP through the existing SSRF-safe owner in `core.helper.ssrf_proxy`. +- Use Pydantic v2 for request and response models. Reuse domain-specific exceptions and translate them at the controller boundary. +- Use existing Celery task and queue owners for asynchronous work; do not route unrelated jobs through workflow-specific services. +- Celery tasks that may be retried or redelivered must keep side effects idempotent and log affected resource identifiers. diff --git a/api/controllers/API_SCHEMA_GUIDE.md b/api/controllers/API_SCHEMA_GUIDE.md index a1e412a3630..30e6ddc0a8d 100644 --- a/api/controllers/API_SCHEMA_GUIDE.md +++ b/api/controllers/API_SCHEMA_GUIDE.md @@ -162,6 +162,9 @@ That documents a GET request body and is not the expected contract. ## Responses +`204 No Content` responses must not serialize a response body. Return the status using the established controller pattern; +do not return a dictionary, response model, or other payload. + Response models should inherit from `ResponseModel`: ```python diff --git a/cli/AGENTS.md b/cli/AGENTS.md index 56a59128c03..446c85abdcc 100644 --- a/cli/AGENTS.md +++ b/cli/AGENTS.md @@ -1,99 +1,26 @@ # AGENTS.md — difyctl (TypeScript CLI) -TypeScript port of difyctl. Stack: custom CLI framework (`src/framework/`), Node 22+, ESM, ky for HTTP, Vitest, and Vite+ formatting and linting. +This package is the Node 22+, ESM TypeScript implementation of `difyctl`. Development also requires the Bun version pinned in `.bun-version`; command-tree generation and the `dev`, `test`, and `build` pre-scripts invoke it. Read [`ARD.md`] before adding a command or changing shared CLI infrastructure. Read `src/commands/AGENTS.md` for command-folder and registry rules. -> Architecture patterns, scaffolding recipe, printer chain, strategy pattern, testing conventions, anti-patterns: see **[`ARD.md`]**. +## Architecture Boundaries -## Code rules +- Every leaf command extends `DifyCommand`; command classes own framework parsing and delegate behavior to domain modules. +- Each command folder keeps its framework shell in `index.ts`. Extract behavior into sibling modules such as `run.ts` and `handlers.ts` when it needs an independently testable owner; those modules receive typed dependencies and do not import `src/framework/`. +- `src/http/` owns ky middleware and client construction; `src/api/` owns resource clients; `src/sys/io/` owns process streams and progress UI; `src/types/` remains a pure data and schema leaf. +- Preserve flags, output, and exit codes during refactors. Do not add dependencies or compatibility shims unless the task explicitly requires them. +- `ARD.md` owns CLI code structure. Keep wire behavior aligned with typed API clients and the real mock-server behavior tests. -- **Spaces, not tabs.** -- **Minimum comments.** Code speak for self. Comment only non-obvious WHY — hidden constraints, subtle invariants, bug-workaround notes. Never restate code. Never reference tasks, PRs, current callers. -- **No magic strings or numbers.** Enums or named constants for bounded value sets. -- **No long positional arg lists.** Use options objects. -- **No long if/switch ladders on discriminator.** Polymorphism, dispatch tables, or strategy pattern. Name concept, let implementations plug in. -- **No `any`. No `unknown` outside genuine wire boundaries** (HTTP body parse, env vars). Narrow types everywhere else. -- **Avoid `!` non-null assertions.** Narrow instead. -- **`readonly` on inputs not mutated.** -- **Discriminated unions** for variant data (SSE events, run outputs, error shapes), not optional-field bags. -- **No backwards-compat shims.** No re-exports of old names, no `// removed:` markers, no deprecation notes. Delete, update callers. -- **No new dependencies without explicit approval.** -- **No CLI behavior changes in refactor commit.** Same flags, same output, same exit codes. -- **Every leaf command extends `DifyCommand`.** Add `static agentGuide` string when command benefits from agent workflow docs — see `src/commands/AGENTS.md`. +## Commands -## Layering +Run package scripts from `cli/`: -| Layer | Path | Role | -| --------- | -------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | -| commands | `src/commands/` | Command class shells (extend `DifyCommand`). Only place framework imports run. | -| domain | `src/run/`, `src/get/`, etc. | Plain TS modules. Take typed deps via options. Testable without the framework. | -| api | `src/api/` | One typed client per resource. Each takes `KyInstance`. | -| http | `src/http/` | `createClient` + middleware (auth, retry, logging, error mapping). Only place ky runs. | -| io | `src/io/` | Streams + spinner. Fence between data-out and progress UI. | -| printers | `src/printers/` | `CompositePrintFlags` + `-o {json,yaml,name,wide,text}` matrix. | -| errors | `src/errors/` | `BaseError`, `ErrorCode` enum, `ExitCode` enum, dispatch table, `formatErrorForCli`. | -| guide | `src/commands/**//guide.ts` | Per-command agent guide string. Export `agentGuide`, assign `static agentGuide = agentGuide` in command class. Surfaced via `--help`. | -| cache | `src/cache/` | On-disk caches (app-info, etc.). | -| auth | `src/auth/` | Hosts file, token store, login flow. | -| config | `src/config/` | XDG dir resolution, config.yml load/save. | -| workspace | `src/workspace/` | Resolver: flag → env → bundle. | -| types | `src/types/` | Pure data + zod schemas for server contracts. No runtime imports outward. | +- Source CLI: `pnpm dev [args...]` +- Tests: `pnpm test` +- Build: `pnpm build` +- Regenerate and verify the registry: `pnpm tree:gen` and `pnpm tree:check` -## Command Structure +Run the scoped static check from the repository root with `vp check cli`. -Scaffold recipe + checklist: see `ARD.md §New command scaffold`. Full folder convention (subcommands, guide.ts): see `src/commands/AGENTS.md`. - -Layer rules: - -- Commands thin shells. Use `this.authedCtx(opts)` for bearer context; delegate to domain function. -- Domain receives deps via options; never imports `src/framework/`. -- Only `src/http/client.ts` and `src/api/*` import ky at runtime; elsewhere use `import type { KyInstance }`. -- `process.*` lives in `src/io/`, `src/store/dir.ts`, `src/util/browser.ts`. Nowhere else. -- No circular imports. `types/` pure leaf. - -## Dev commands - -```sh -pnpm install # one-time -pnpm dev [args...] # run CLI from source (no -- separator) -pnpm test # vitest -pnpm test:coverage # with coverage -pnpm -w check # repository-wide static check -pnpm -w check:fix # repository-wide static fixes -pnpm build # production bundle (vp pack) -pnpm tree:gen # regenerate src/commands/tree.ts (registry) -pnpm tree:check # verify tree.ts is up-to-date with the fs -``` - -Release binaries (5 platform targets, Bun-compiled) are produced by `pnpm build:bin` (called from `.github/workflows/cli-release.yml`). - -## Tests - -- Behavior tests run against real Hono mock at `test/fixtures/dify-mock/`. No `nock`, `msw`, or `fetchMock` — every test exercises real HTTP. -- Test files co-located: `foo.test.ts` next to `foo.ts`. -- The repository-wide static check and full test suite must be green before any commit. - -## Spec docs (`docs/specs/`) - -Behavior contracts. Living tree — amended in place, no version subfolders. - -**Keep:** HTTP wire shape (req/resp JSON, headers, status codes), SQL DDL, Redis keys + TTL, state transitions, audit event names + payload, error/exit codes, rate-limit values, JWS/cookie envelope claims. - -**Cut:** language type decls, internal helper sigs, decorator snippets, file-path tables, pseudocode mirroring code, "Open items"/"Handler walk"/"CI guard"/"Migration" sections, rationale (`Rejected:`/`Why X not Y`/`Historical note:`/product comparisons), release-pipeline lines, version-pinning (`in v1.0`, `post-v1.0`, milestone codes), frontmatter `date`/`status`/`author`. - -**Test:** "rewrite in Rust tomorrow, does spec hold?" HTTP/SQL/Redis stays; type defs go. - -**Rules:** behavior, not rationale. One topic per file; cross-refs = `auth.md §Storage`. Tables beat prose. Code wins on drift — update spec. - -## Out of scope for unrelated work - -Do not modify in passing: - -- `test/fixtures/dify-mock/` public surface (endpoints, JSON shapes, status codes, scenario names) — that's the dify-api contract. -- `bin/`, `scripts/`, `Makefile`, `lint.config.ts`, `tsconfig*.json`, `package.json` (unless the change is required by the task). - -## Commits - -- One concern per commit. Style: `(): ` lowercase. Body explains why if non-obvious. -- Never push, amend, force-push, or skip hooks (`--no-verify`) without explicit user approval. +Behavior tests use the real Hono server under `test/fixtures/dify-mock/`; do not replace it with `nock`, `msw`, or `fetchMock`. Keep tests colocated with their source files. [`ARD.md`]: ARD.md diff --git a/cli/ARD.md b/cli/ARD.md index a87b01dd85f..e485e74be60 100644 --- a/cli/ARD.md +++ b/cli/ARD.md @@ -2,8 +2,6 @@ Onboarding ref for `dify/cli/` contributors. Cover canonical patterns, layer contracts, scaffolding recipe, dev workflow, anti-patterns. Read before adding command or touching shared infra. -Spec authority: [`docs/specs/`]. Specs own HTTP wire shape + server behavior; this file owns CLI code structure. - --- ## Project layout @@ -17,7 +15,7 @@ src/ config/ config.yml read/write errors/ BaseError, ErrorCode, exit codes http/ ky client factory + middleware - io/ IOStreams, spinner, printer chain + sys/io/ IOStreams, prompts, spinner, output rendering limit/ --limit flag parsing types/ shared TypeScript types util/ small pure helpers @@ -38,17 +36,17 @@ src/commands/// Examples: `get/app/`, `auth/devices/revoke/`, `describe/app/`. -**2. Mandatory files** +**2. Mandatory file** -| File | Responsibility | -| ---------- | --------------------------------------------------------------------------------------- | -| `index.ts` | `DifyCommand` subclass. Flag/arg declaration + `run()` wiring only. No business logic. | -| `run.ts` | Pure async function. Typed options + deps. Returns string. No `src/framework/` imports. | +| File | Responsibility | +| ---------- | ------------------------------------------------------------------------------------ | +| `index.ts` | `DifyCommand` subclass. Owns flag/arg parsing, framework output, and command wiring. | **3. Optional files — add as needed** | File | Purpose | | ------------------ | ------------------------------------------------------------------ | +| `run.ts` | Typed behavior owner when logic merits independent tests or reuse | | `handlers.ts` | Output types implementing `FormattedPrintable` or `TablePrintable` | | `payload-shape.ts` | Response type narrowing/transformation | | `run.test.ts` | Behavior tests against `run.ts` | @@ -58,12 +56,12 @@ Examples: `get/app/`, `auth/devices/revoke/`, `describe/app/`. - [ ] `index.ts` extends `DifyCommand` - [ ] Authed command calls `this.authedCtx()`; non-authed skips -- [ ] No try/catch in `run()` — `DifyCommand.catch()` handles `BaseError` -- [ ] `run.ts` returns string; no direct stdout write -- [ ] `run.ts` no `src/framework/` imports +- [ ] Let the command boundary handle `BaseError`; catch only when the command owns recovery +- [ ] Keep framework parsing and output construction in `index.ts` +- [ ] When present, `run.ts` returns typed behavior data or owns explicit streaming/interactive I/O and does not import `src/framework/` - [ ] HTTP client via factory dep, not direct -- [ ] `run.test.ts` written before impl (test-first) -- [ ] `pnpm tree:gen` run after adding command (updates `src/commands/tree.ts`) +- [ ] Add focused behavior tests when the command changes an observable contract +- [ ] `pnpm tree:gen` run after adding command (updates `src/commands/tree.generated.ts`) - [ ] README command table updated by hand --- @@ -74,27 +72,22 @@ All commands extend `DifyCommand`, not `Command`. ```typescript export default class MyCommand extends DifyCommand { - async run(): Promise { + async run(argv: string[]) { const { args, flags } = this.parse(MyCommand, argv) - // Authed: authedCtx() sets outputFormat + builds context - const ctx = await this.authedCtx({ format: flags.output }) - - process.stdout.write( - await runMyThing( - { - // args - }, - { bundle: ctx.bundle, http: ctx.http, io: ctx.io }, - ), + const ctx = await this.authedCtx({ retryFlag: undefined, format: flags.output }) + const result = await runMyThing( + { id: args.id }, + { active: ctx.active, http: ctx.http, io: ctx.io }, ) + return formatted({ format: flags.output, data: result.data }) } } ``` -**`authedCtx(opts)`** — wraps `buildAuthedContext`. Sets `this.outputFormat` as side effect. Required for any command needing bearer token. +**`authedCtx(opts)`** — wraps `buildAuthedContext` and returns the authenticated registry, account, HTTP, I/O, and optional cache dependencies. Pass the selected output format so authentication failures use the same serialization contract. Required for commands that need a bearer token. -**`catch(err)` override** — auto-handles `BaseError` with format-aware serialization. Never wrap `run()` in try/catch. Throw `BaseError`; base class catches. +The framework runner in `src/framework/run.ts` catches command errors, normalizes unknown failures, and serializes `BaseError` according to the selected output format. Catch inside a command only when that command owns a real recovery path. --- @@ -103,8 +96,8 @@ export default class MyCommand extends DifyCommand { Throw `BaseError`. Never throw raw `Error` for domain failures. ```typescript -import { BaseError } from '../../errors/base.js' -import { ErrorCode } from '../../errors/codes.js' +import { BaseError } from '@/errors/base' +import { ErrorCode } from '@/errors/codes' throw new BaseError({ code: ErrorCode.UsageMissingArg, @@ -113,7 +106,7 @@ throw new BaseError({ }) ``` -`ErrorCode` exhaustive const object — never use raw strings. `exitFor(code)` maps to exit codes auto. `DifyCommand.catch()` calls `formatErrorForCli` with `outputFormat` so JSON/YAML consumers get machine-readable error output. +`ErrorCode` is the exhaustive error-code object; do not scatter raw code strings. `exitFor(code)` maps it to a process exit code, and the framework runner calls `formatErrorForCli` so JSON/YAML consumers receive machine-readable errors. | Exit | Meaning | | ---- | ----------------------------------------- | @@ -122,6 +115,7 @@ throw new BaseError({ | 2 | Usage error (bad flag, missing arg) | | 4 | Auth error (not logged in, token expired) | | 6 | Version/compat error | +| 7 | Rate limited | New error code: add to `ErrorCode` + map to `ExitCode` in `codes.ts`. Never scatter exit codes inline. @@ -172,7 +166,7 @@ Output rendering separated from data fetching via protocol objects. - Data classes implement `TablePrintable` or `FormattedPrintable` from `src/framework/output`. - Streaming commands implement `StreamPrinter` from `src/framework/stream`. -- `index.ts` wraps the result with `table({format, data})` or `formatted({format, data})` and returns it; the base class calls `stringifyOutput()`. +- `index.ts` wraps the result with `table({format, data})` or `formatted({format, data})` and returns it; `src/framework/run.ts` calls `stringifyOutput()`. - Commands that write incrementally (streaming) write directly from the strategy via `deps.io.out.write(stringifyOutput(...))`. ```typescript @@ -220,51 +214,15 @@ New mode = new class + one line in picker. Singletons avoid per-call allocation. ## HTTP clients -One file per resource under `src/api/`. Each exports class wrapping `KyInstance`. +Keep resource clients under `src/api/`. They receive the shared `HttpClient` and call generated oRPC operations through `createOpenApiClient(...)` when the OpenAPI contract covers the endpoint. Reuse generated request and response types instead of duplicating wire shapes. -```typescript -export class AppsClient { - private readonly http: KyInstance - constructor(http: KyInstance) { - this.http = http - } - - async list(params: ListParams): Promise { - /* ... */ throw new Error('elided') - } - async describe(id: string, workspaceId: string, fields: string[]): Promise { - /* ... */ throw new Error('elided') - } -} -``` - -Inject via factory dep in `run.ts` for testability: - -```typescript -type GetAppDeps = { - appsFactory?: (http: KyInstance) => AppsClient -} -// default: (h) => new AppsClient(h) -``` - -Never instantiate clients in `index.ts`. +Pass `HttpClient` into behavior owners. Add a client or factory dependency only when it owns a real substitution or lifecycle boundary; behavior tests normally exercise the real client stack against `test/fixtures/dify-mock/`. Keep client construction out of `index.ts` so the command remains a framework and output boundary. --- ## Testing -**Test-first.** Write failing test, run to confirm fail, then implement. - -Tests live in `run.test.ts` alongside command. Test `run.ts` direct — never the `DifyCommand` class. - -```typescript -const io = bufferStreams() -const result = await runGetApp( - { format: 'json', appId: 'app-1' }, - { bundle, http: mockHttp, io, appsFactory: () => fakeClient }, -) -expect(JSON.parse(result).data).toHaveLength(1) -``` +Keep tests beside the owner as `*.test.ts`. When a command has a behavior module, test that public function directly for domain and protocol behavior. Test the command class or framework boundary when argument parsing, flags, help, output construction, or command wiring is the observable contract. Establish a failing case first when practical for behavior changes and bug fixes. ### dify-mock fixture server @@ -306,14 +264,14 @@ expect(JSON.parse(out).workspaces).toHaveLength(2) | `pnpm dev [args]` | Run CLI from source during dev | | `pnpm test` | Full vitest suite — run before every commit | | `pnpm test:coverage` | Coverage report | -| `pnpm -w check` | Repository-wide static check | -| `pnpm -w check:fix` | Repository-wide static fixes | +| `vp check cli` | Scoped static check from the repository root | +| `vp check --fix cli` | Scoped static fixes from the repository root | | `pnpm build` | Production bundle (`vp pack`) | -| `pnpm tree:gen` | Regenerate `src/commands/tree.ts` (registry) | -| `pnpm tree:check` | Verify `tree.ts` matches the filesystem | +| `pnpm tree:gen` | Regenerate `src/commands/tree.generated.ts` | +| `pnpm tree:check` | Verify the generated tree matches the commands | | `pnpm build:bin` | Cross-compile standalone binaries via Bun (CI) | -**`pnpm tree:gen` rule:** run after adding, removing, renaming any command. The generated `tree.ts` is the runtime command registry — stale tree causes commands to be invisible at runtime. (Runs implicitly via `prebuild`/`predev`/`pretest`.) +**`pnpm tree:gen` rule:** run after adding, removing, or renaming any command. The generated `tree.generated.ts` is the runtime command registry; a stale tree makes commands invisible at runtime. It also runs through `prebuild`, `predev`, and `pretest`. **README hand-maintained.** When adding a command, update the command table in `README.md` manually. @@ -331,7 +289,7 @@ The repository runs Vite+ Oxlint as the primary code-quality linter, an explicit | `unicorn/no-new-array` | Use `Array.from({ length: n })` not `new Array(n)` | | `noUncheckedIndexedAccess` (tsc) | `arr[i]` is `T \| undefined`; guard before use | -Run `pnpm -w check:fix` for Oxlint, ESLint, TypeScript, and Oxfmt fixes and diagnostics. +Run `vp check --fix cli` from the repository root for scoped formatting, lint, and TypeScript fixes and diagnostics. --- @@ -350,14 +308,12 @@ Run `pnpm -w check:fix` for Oxlint, ESLint, TypeScript, and Oxfmt fixes and diag | Pattern | Do instead | | -------------------------------------------------------------------- | -------------------------------------------------------------------------- | | `if (format === 'json') { ... }` in `run.ts` | Printer handler per format | -| `try { ... } catch (e) { if (isBaseError(e)) ... }` in every command | Throw `BaseError`; `DifyCommand.catch()` handles | +| `try { ... } catch (e) { if (isBaseError(e)) ... }` in every command | Throw `BaseError`; `src/framework/run.ts` normalizes and formats it | | Raw string error codes `'not_logged_in'` | `ErrorCode.NotLoggedIn` | | `enabled: !isHuman` in `runWithSpinner` | Set `outputFormat` on `IOStreams`; spinner auto-detects | | Long positional arg lists | Options struct | | `Record` dispatch map | Named singletons + picker function | | `src/framework/` import in `run.ts`, `api/`, or `auth/` | Framework imports belong in `index.ts`, `handlers.ts`, and strategies only | | `buildAuthedContext(this, opts)` in command body | `this.authedCtx(opts)` | -| `console.log` in `src/` | Return string from `run.ts`; write in `index.ts` | +| `console.log` in `src/` | Return `CommandOutput` from the command or use owned I/O for streaming | | New dependency without approval | Check first | - -[`docs/specs/`]: docs/specs/ diff --git a/cli/src/commands/AGENTS.md b/cli/src/commands/AGENTS.md index 5df83849779..6d675ef70a9 100644 --- a/cli/src/commands/AGENTS.md +++ b/cli/src/commands/AGENTS.md @@ -13,7 +13,7 @@ src/commands/ / / index.ts ← command class (extends DifyCommand; the ONLY file the registry discovers) - run.ts ← business logic (not a command, invisible to the registry) + run.ts ← optional behavior owner (not a command, invisible to the registry) handlers.ts ← helpers guide.ts ← agent guide string (optional) *.test.ts ← tests @@ -23,7 +23,7 @@ src/commands/ .ts ``` -The registry generator (`pnpm tree:gen` → `src/commands/tree.ts`) discovers +The registry generator (`pnpm tree:gen` → `src/commands/tree.generated.ts`) discovers commands only via `**/index.+(js|cjs|mjs|ts)`. All other files in command folders are invisible to the registry — add freely without glob exclusions. Folders prefixed with `_` (e.g. `_shared/`, `_strategies/`) are excluded from @@ -32,7 +32,7 @@ registry discovery and from coverage checks. ## Adding a new command 1. Create `src/commands///index.ts` extending `DifyCommand`. -1. Add business logic in sibling files (e.g. `run.ts`, `handlers.ts`). +1. Keep small owner-local behavior in `index.ts`; extract sibling modules such as `run.ts` or `handlers.ts` when logic needs independent tests, reuse, or a clearer owner. 1. Run `pnpm tree:gen` to regenerate the command tree (also runs implicitly via `prebuild`/`predev`/`pretest`). 1. Run `pnpm test` to verify coverage. @@ -54,7 +54,9 @@ registry discovery and from coverage checks. import { agentGuide } from './guide.js' export default class MyCmd extends DifyCommand { - static agentGuide = agentGuide + override agentGuide(): string { + return agentGuide + } } ``` 1. The guide appears at the bottom of `difyctl --help` automatically. diff --git a/dify-agent/AGENTS.md b/dify-agent/AGENTS.md index 43c68448f20..ee33943a3aa 100644 --- a/dify-agent/AGENTS.md +++ b/dify-agent/AGENTS.md @@ -1,184 +1,21 @@ # Agent Guide -## Notes for Agent (must-check) +Read surrounding docstrings and non-obvious comments before changing behavior. They are local contracts; update them only when their owned behavior changes, and keep them aligned with the current code. Read `docs/dify-agent/index.md` when changing the public runtime contract. -Before changing any source code under this folder, you MUST read the surrounding docstrings and comments. These notes contain required context (invariants, edge cases, trade-offs) and are treated as part of the spec. +## Commands -Look for: +Run package commands from `dify-agent/`: -- The module (file) docstring at the top of a source code file -- Docstrings on classes and functions/methods -- Paragraph/block comments for non-obvious logic +- Lint: `make check` +- Format and fix lint: `make fix` +- Type check: `make typecheck` +- Tests: `make test` -### What to write where +Use the package's `uv` environment and Pydantic v2 APIs. Inspect current dependency source or official documentation before integrating, implementing, or mocking an API whose runtime contract is not established locally. -- Keep notes scoped: module notes cover module-wide context, class notes cover class-wide context, function/method notes cover behavioural contracts, and paragraph/block comments cover local “why”. Avoid duplicating the same content across scopes unless repetition prevents misuse. -- **Module (file) docstring**: purpose, boundaries, key invariants, and “gotchas” that a new reader must know before editing. - - Include cross-links to the key collaborators (modules/services) when discovery is otherwise hard. - - Prefer stable facts (invariants, contracts) over ephemeral “today we…” notes. -- **Class docstring**: responsibility, lifecycle, invariants, and how it should be used (or not used). - - If the class is intentionally stateful, note what state exists and what methods mutate it. - - If concurrency/async assumptions matter, state them explicitly. -- **Function/method docstring**: behavioural contract. - - Document arguments, return shape, side effects (DB writes, external I/O, task dispatch), and raised domain exceptions. - - Add examples only when they prevent misuse. -- **Paragraph/block comments**: explain *why* (trade-offs, historical constraints, surprising edge cases), not what the code already states. - - Keep comments adjacent to the logic they justify; delete or rewrite comments that no longer match reality. +## Tests And Boundaries -### Rules (must follow) - -In this section, “notes” means module/class/function docstrings plus any relevant paragraph/block comments. - -- **Before working** - - Read the notes in the area you’ll touch; treat them as part of the spec. - - If a docstring or comment conflicts with the current code, treat the **code as the single source of truth** and update the docstring or comment to match reality. - - If important intent/invariants/edge cases are missing, add them in the closest docstring or comment (module for overall scope, function for behaviour). -- **During working** - - Keep the notes in sync as you discover constraints, make decisions, or change approach. - - If you move/rename responsibilities across modules/classes, update the affected docstrings and comments so readers can still find the “why” and the invariants. - - Record non-obvious edge cases, trade-offs, and the test/verification plan in the nearest docstring or comment that will stay correct. - - Keep the notes **coherent**: integrate new findings into the relevant docstrings and comments; avoid append-only “recent fix” / changelog-style additions. -- **When finishing** - - Update the notes to reflect what changed, why, and any new edge cases/tests. - - Remove or rewrite any comments that could be mistaken as current guidance but no longer apply. - - Keep docstrings and comments concise and accurate; they are meant to prevent repeated rediscovery. - -## Coding Style - -This is the default standard for backend code in this repo. Follow it for new code and use it as the checklist when reviewing changes. - -### Linting & Formatting - -- Use Ruff for formatting and linting (follow `.ruff.toml`). -- Keep each line under 120 characters (including spaces). - -### Naming Conventions - -- Use `snake_case` for variables and functions. -- Use `PascalCase` for classes. -- Use `UPPER_CASE` for constants. - -### Typing & Class Layout - -- Code should usually include type annotations that match the repo’s current Python version (avoid untyped public APIs and “mystery” values). -- Prefer modern typing forms (e.g. `list[str]`, `dict[str, int]`) and avoid `Any` unless there’s a strong reason. -- For dictionary-like data with known keys and value types, prefer `TypedDict` over `dict[...]` or `Mapping[...]`. -- For optional keys in typed payloads, use `NotRequired[...]` (or `total=False` when most fields are optional). -- Keep `dict[...]` / `Mapping[...]` for truly dynamic key spaces where the key set is unknown. - -```python -from datetime import datetime -from typing import NotRequired, TypedDict - - -class UserProfile(TypedDict): - user_id: str - email: str - created_at: datetime - nickname: NotRequired[str] -``` - -- For classes, declare all member variables explicitly with types at the top of the class body (before `__init__`), even when the class is not a dataclass or Pydantic model, so the class shape is obvious at a glance: - -```python -from datetime import datetime - - -class Example: - user_id: str - created_at: datetime - - def __init__(self, user_id: str, created_at: datetime) -> None: - self.user_id = user_id - self.created_at = created_at -``` - -- For dataclasses, prefer `field(default_factory=...)` over `field(init=False)` when a default can be provided declaratively. -- Prefer dataclasses with `slots=True` when defining lightweight data containers: - -```python -from dataclasses import dataclass -from datetime import datetime - - -@dataclass(slots=True) -class Example: - user_id: str - created_at: datetime -``` - -### General Rules - -- Use Pydantic v2 conventions. -- Use `uv` for Python package management in this repo (usually with `--project dify-agent`). -- Use `make typecheck` to run `basedpyright` against `dify-agent/src` and `dify-agent/tests`. -- Keep type checking passing after every edit you make. -- Use `pytest` for all tests in this package. -- When integrating with, implementing, or mocking a dependency, inspect the dependency's source code to confirm its API shape and runtime behavior instead of guessing from names alone. -- Prefer simple functions over small “utility classes” for lightweight helpers. -- Avoid implementing dunder methods unless it’s clearly needed and matches existing patterns. -- Keep code readable and explicit—avoid clever hacks. - -### Testing - -- Work in TDD style: write or update a failing test first when changing behavior, then make the implementation pass, then refactor while keeping tests and typecheck green. -- Use `make test` to run the agent pytest suite. -- Keep local tests under `dify-agent/tests/local/`. -- Mirror the `dify-agent/src/` package structure inside `dify-agent/tests/local/` so test locations stay predictable. - -#### Local Tests - -- Write local tests for stable, externally observable behavior that can run quickly without real external services. -- In this repo, code, comments, docs, and tests are expected to change together. Because of that, a local test is only useful if it would still be correct after an internal refactor that does not change the intended contract. -- Local tests should verify: - - what callers and downstream code can observe and rely on - - how the unit is expected to use its dependencies at the boundary - - how the unit handles dependency success, failure, empty responses, malformed responses, and documented error cases - - documented invariants, error mapping, and output/input shape guarantees -- When asserting dependency interactions, assert only the parts of the request or response that are part of the real boundary contract. Do not over-specify incidental details that callers or dependencies do not rely on. -- It is acceptable to mock dependencies in local tests, but only when the mock represents a real contract, schema, documented behavior, or known regression. -- Tests may use line-scoped type-ignore comments when intentionally exercising runtime validation paths that static typing would normally reject. Keep the ignore on the exact invalid call. -- Do not use local tests to prove real integration, network wiring, serialization, framework configuration, or third-party runtime behavior; cover those in higher-level tests. -- Meaningless local tests include: - - tests that only mirror the current implementation or must be updated whenever internal code changes even though the contract did not change - - tests of private helpers, local variables, temporary state, internal branching, or exact internal call order unless those details are part of the published contract - - tests with mocked dependency behavior that is invented only to make the current implementation pass - - tests that add no value beyond static type checking or linting - -### Logging & Errors - -- Never use `print`; use a module-level logger: - - `logger = logging.getLogger(__name__)` -- Include tenant/app/workflow identifiers in log context when relevant. -- Raise domain-specific exceptions and translate them into HTTP responses in controllers. -- Log retryable events at `warning`, terminal failures at `error`. - -### Pydantic Usage - -- Define DTOs with Pydantic v2 models and forbid extras by default. -- Use `@field_validator` / `@model_validator` for domain rules. - -Example: - -```python -from pydantic import BaseModel, ConfigDict, HttpUrl, field_validator - - -class TriggerConfig(BaseModel): - endpoint: HttpUrl - secret: str - - model_config = ConfigDict(extra="forbid") - - @field_validator("secret") - def ensure_secret_prefix(cls, value: str) -> str: - if not value.startswith("dify_"): - raise ValueError("secret must start with dify_") - return value -``` - -### Generics & Protocols - -- Use `typing.Protocol` to define behavioural contracts (e.g., cache interfaces). -- Apply generics (`TypeVar`, `Generic`) for reusable utilities like caches or providers. -- Validate dynamic inputs at runtime when generics cannot enforce safety alone. +- Keep local tests under `tests/local/` and mirror the `src/` package structure. +- Test stable behavior and real dependency boundaries. Do not use local mocks to claim real network, framework wiring, serialization, or third-party runtime coverage. +- Keep tests, public docs, and local contracts aligned with behavior changes. +- Preserve the existing runtime and layer owners; do not add generic utilities or compatibility boundaries to bypass them. diff --git a/e2e/AGENTS.md b/e2e/AGENTS.md index abbf2a27b4b..a1cb741fd26 100644 --- a/e2e/AGENTS.md +++ b/e2e/AGENTS.md @@ -1,381 +1,62 @@ # E2E -This package contains the repository-level end-to-end tests for Dify. +This package contains Dify's repository-level Cucumber scenarios with Playwright as the browser layer. This file owns current package architecture, runtime, session and tag semantics, seed, protocol, and cleanup contracts. The repo-local `e2e-cucumber-playwright` skill owns authoring and review methodology; feature-specific facts belong in the nearest feature `AGENTS.md`. -This file is the canonical package guide for `e2e/`. Keep detailed workflow, architecture, debugging, and reporting documentation here. Keep `README.md` as a minimal pointer to this file so the two documents do not drift. +## Commands -The suite uses Cucumber for scenario definitions and Playwright as the browser execution layer. +Run commands from the repository root. Install dependencies and browsers once with `pnpm install` and `pnpm -C e2e e2e:install`. Run only one local `pnpm -C e2e e2e*` process at a time because runners share ports, auth state, and log paths. -It tests: +- Existing initialized instance: `pnpm -C e2e e2e` +- Reset, initialize, and run deterministic scenarios: `pnpm -C e2e e2e:full` +- Tagged subset: `pnpm -C e2e e2e -- --tags @smoke` +- Headed debugging: `pnpm -C e2e e2e:headed -- --tags @smoke` +- External runtime preparation and run: `pnpm -C e2e e2e:external:prepare`, then `pnpm -C e2e e2e:external` +- Reset persisted E2E state: `pnpm -C e2e e2e:reset` +- Middleware lifecycle: `pnpm -C e2e e2e:middleware:up` and `pnpm -C e2e e2e:middleware:down` +- Scoped static checks: `vp check e2e` -- backend API started from source -- frontend served from the production artifact -- middleware services started from Docker +The runner reuses `web/.next/BUILD_ID` when present. Set `E2E_FORCE_WEB_BUILD=1` to force a frontend rebuild. Use `E2E_BROWSER=webkit` for focused cross-browser runs and `E2E_SLOW_MO=500` with a headed command for local action debugging. -## Prerequisites +## Runtime Ownership -- Node.js `^22.22.1` -- `pnpm` -- `uv` -- Docker +- `scripts/setup.ts` owns reset, middleware, backend, and frontend startup. +- `scripts/run-cucumber.ts` owns E2E orchestration and Cucumber invocation. +- `support/web-server.ts` owns frontend reuse, readiness, and shutdown. +- `features/support/hooks.ts` owns shared auth bootstrap, scenario lifecycle, and diagnostics. +- `features/support/world.ts` owns `DifyWorld`, the per-scenario behavior `BrowserContext`, and its authenticated setup and cleanup client. Browser and API identities remain separate so unauthenticated and logout journeys cannot invalidate fixture ownership. +- Cross-actor scenarios keep each actor in a separate `BrowserContext` and typed `DifyWorld` state so diagnostics and cleanup cover every actor. +- `features/step-definitions/` contains capability-oriented glue; `common/` is reserved for genuinely cross-capability steps. +- Step definitions that access World state use `async function (this: DifyWorld, ...)`; arrow functions cannot receive Cucumber's bound World instance. -Run the following commands from the repository root. +An uninitialized instance is installed and authenticated lazily; an initialized instance signs in and reuses authenticated state. Full runs prove reset and bootstrap during setup rather than through a Gherkin scenario. Cucumber's exit status is the behavior gate, and the runner also requires at least one `testCaseStarted` message so an empty tag selection cannot pass. Do not replace this gate with scenario-count baselines or skipped-scenario allowlists. -Install Playwright browsers once: +## Tags And External Runtime -```bash -pnpm install -pnpm -C e2e e2e:install -``` +- Default scenarios use shared authenticated storage state. `@unauthenticated` creates a clean context; `@authenticated` is an intent and selection tag only. +- `@prepared` requires the strict post-merge seed profile. +- `@external-model` and `@external-tool` identify scenarios that call real external runtimes. Deterministic commands exclude these tags; external commands are opt-in. +- `@microphone` uses the checked-in fake audio fixture and an isolated Chromium context. +- `@browser-smoke` runs focused keyboard and navigation coverage in Chromium and WebKit CI lanes. +- Feature-owned services use their own tags. Agent v2 runtime scenarios use `@agent-backend-runtime` and require the explicit runtime-availability step. Set `E2E_START_AGENT_BACKEND=1` to start it locally, or provide `E2E_AGENT_BACKEND_URL` / `AGENT_BACKEND_BASE_URL`. -`pnpm install` is resolved through the repository workspace and uses the shared root lockfile plus `pnpm-workspace.yaml`. +Do not overload runtime tags to imply unrelated services or silently skip behavior when a required fixture is missing. -Run only one `pnpm -C e2e e2e*` process against a local workspace at a time. Separate runner processes share the frontend port, backend port, auth bootstrap state, and log paths; running them in parallel can create startup or authorization failures that are not scenario failures. +## Browser, API, And Contract Boundaries -Use root lint plus the package type check as the default local verification step after editing E2E TypeScript, Cucumber support code, or feature glue: +The action under test belongs to the browser. APIs may prepare fixtures, poll persistence, and clean up; they do not replace the user's `When` action. Prefer a user-observable browser result unless persisted backend state is the contract under test. -```bash -vp lint --fix --quiet -pnpm -C e2e type-check -``` +For ordinary Console JSON and representable multipart operations, use the scenario- or process-owned generated oRPC client with request and response validation enabled. Call generated operations directly. Do not add handwritten endpoint URLs, duplicate DTOs or schemas, response casts, one-to-one forwarding wrappers, mutable cross-scenario clients, or TanStack Query caching. -Common commands: +Keep helpers only when they own fixture construction, multi-operation orchestration, cleanup registries, invariants, eventual-consistency polling, narrowed test views, or a protocol adapter. SSE, binary downloads, redirect-only flows, external services, and infrastructure readiness may use centralized adapters under their real owner. -```bash -# deterministic regression against an initialized instance -# expects backend API, frontend artifact, and middleware stack to already be running -pnpm -C e2e e2e +Validation failures are contract failures. Trace them to the backend schema owner, update `api/controllers/API_SCHEMA_GUIDE.md` contracts when required, regenerate `@dify/contracts`, and keep the scenario aligned with the product's real state owner. Do not disable validation or add fallback schemas to make E2E pass. -# reset, initialize, and run deterministic scenarios -# starts required middleware/dependencies for you -pnpm -C e2e e2e:full +## Seeds, Cleanup, And Diagnostics -# run a tagged subset -pnpm -C e2e e2e -- --tags @smoke +- Generate disposable resource names through `support/naming.ts` with an `E2E` prefix. +- Keep deterministic upload material in `fixtures/test-materials/` and resolve it through `support/test-materials.ts`. +- Seed scripts own shared long-lived fixtures; scenarios own disposable resources they create and must register cleanup. +- Use typed `DifyWorld` cleanup fields for known resource types and `registerCleanup(...)` for additional lifecycle owners. Registered callbacks run LIFO after typed cleanup queues. +- Remove child and referencing resources before owners. Attach cleanup failures to the report instead of swallowing them. -# prepare external runtime seed resources for opt-in external suites -pnpm -C e2e e2e:external:prepare - -# run scenarios that call real external providers -pnpm -C e2e e2e:external - -# headed browser -pnpm -C e2e e2e:headed -- --tags @smoke - -# slow down browser actions for local debugging -E2E_SLOW_MO=500 pnpm -C e2e e2e:headed -- --tags @smoke - -# focused keyboard and cross-browser smoke coverage -E2E_BROWSER=webkit pnpm -C e2e e2e -- --tags @browser-smoke -``` - -Frontend artifact behavior: - -- if `web/.next/BUILD_ID` exists, E2E reuses the existing build by default -- if you set `E2E_FORCE_WEB_BUILD=1`, E2E rebuilds the frontend before starting it - -## Lifecycle - -```mermaid -flowchart TD - A["Start E2E run"] --> B["run-cucumber.ts orchestrates setup/API/frontend"] - B --> C["support/web-server.ts starts or reuses frontend directly"] - C --> D["Cucumber loads config, steps, and support modules"] - D --> E["The first Before hook lazily bootstraps shared auth state"] - E --> F{"Which command is running?"} - F -->|`pnpm -C e2e e2e`| G["Run deterministic scenarios; exclude @prepared and external runtime"] - F -->|`pnpm -C e2e e2e:full*`| H["Reset and run deterministic scenarios; exclude @prepared and external runtime"] - G --> I["Per-scenario BrowserContext from shared browser"] - H --> I - I --> J["Failure artifacts written to cucumber-report/artifacts"] -``` - -Ownership is split like this: - -- `scripts/setup.ts` is the single environment entrypoint for reset, middleware, backend, and frontend startup -- `run-cucumber.ts` orchestrates the E2E run and Cucumber invocation -- `support/web-server.ts` manages frontend reuse, startup, readiness, and shutdown -- `features/support/hooks.ts` manages auth bootstrap, scenario lifecycle, and diagnostics -- `features/support/world.ts` owns the per-scenario behavior BrowserContext and authenticated setup/cleanup client; their identities remain separate so unauthenticated and logout journeys cannot invalidate fixture ownership -- `features/step-definitions/` holds domain-oriented glue so the official VS Code Cucumber plugin works with default conventions when `e2e/` is opened as the workspace root - -Package layout: - -- `features/`: Gherkin scenarios grouped by capability -- `features/step-definitions/`: domain-oriented step definitions -- `features/support/hooks.ts`: suite lifecycle, auth-state bootstrap, diagnostics -- `features/support/world.ts`: shared scenario context -- `support/web-server.ts`: typed frontend startup/reuse logic -- `scripts/setup.ts`: reset and service lifecycle commands -- `scripts/run-cucumber.ts`: Cucumber orchestration entrypoint - -Behavior depends on instance state: - -- uninitialized instance: completes install and stores authenticated state -- initialized instance: signs in and reuses authenticated state - -The `pnpm -C e2e e2e:full*` flows prove reset and authentication bootstrap by failing setup when initialization cannot complete; they do not model bootstrap state as a Gherkin scenario. Deterministic runs exclude `@prepared`, `@external-model`, and `@external-tool`. Post-merge first seeds required fixtures, then runs prepared and external scenarios. - -Reset all persisted E2E state: - -```bash -pnpm -C e2e e2e:reset -``` - -This removes: - -- `docker/volumes/db/data` -- `docker/volumes/redis/data` -- `docker/volumes/weaviate` -- `docker/volumes/plugin_daemon` -- `e2e/.auth` -- `e2e/.logs` -- `e2e/.logs-non-external` -- `e2e/.logs-webkit` -- `e2e/cucumber-report` -- `e2e/cucumber-report-non-external` -- `e2e/cucumber-report-webkit` -- `e2e/seed-report` - -Start the full middleware stack: - -```bash -pnpm -C e2e e2e:middleware:up -``` - -Stop the full middleware stack: - -```bash -pnpm -C e2e e2e:middleware:down -``` - -The middleware stack includes: - -- PostgreSQL -- Redis -- Weaviate -- Sandbox -- SSRF proxy -- Plugin daemon - -Fresh install verification: - -```bash -pnpm -C e2e e2e:full -``` - -Run the Cucumber suite against an already running middleware stack: - -```bash -pnpm -C e2e e2e:middleware:up -pnpm -C e2e e2e -pnpm -C e2e e2e:middleware:down -``` - -Artifacts and diagnostics: - -- `cucumber-report/report.html`: HTML report -- `cucumber-report/report.ndjson`: Cucumber Messages report -- `cucumber-report/artifacts/`: failure screenshots and HTML captures -- `cucumber-report-non-external/`: Chromium core report preserved before later CI lanes -- `cucumber-report-webkit/`: focused WebKit keyboard/browser smoke report -- `.logs/cucumber-api.log`: backend startup log -- `.logs/cucumber-web.log`: frontend startup log -- `.logs-non-external/`: non-external logs preserved before an external CI run -- `.logs-webkit/`: focused WebKit lane logs -- `seed-report/`: JSON readiness reports emitted by external runtime seed packs - -Cucumber's exit status is the behavior gate. The runner also requires at least one -`testCaseStarted` message so an empty or broken tag selector cannot pass silently. Do not add -scenario-count baselines or skipped-scenario allowlists. - -Open the HTML report locally with: - -```bash -open cucumber-report/report.html -``` - -## Scenario admission and behavior ownership - -Add an E2E scenario only when it protects a critical user journey and a cross-boundary result that -cheaper owner-level tests do not already prove. A control changing its own label is not sufficient -E2E evidence when component or integration tests can own that contract. - -Start from product truth, including real defaults and actor roles. API fixtures may establish -preconditions, but they must not manufacture an opposite state merely to make the intended action -look meaningful. When a product default is part of the journey, make it explicit and observable. - -For cross-actor journeys, isolate each actor's browser state, keep their pages in typed `DifyWorld` -state, and include them in failure diagnostics and cleanup. Assert the downstream user-observable -effect, not only the initiating control's local state. - -When a run exposes behavior that conflicts with the intended product contract, identify the first -layer that misclassifies the business state. Fix that owner or report the mismatch explicitly; do -not make the E2E pass by encoding an accidental redirect, stale label, or misleading error state. - -## Writing new scenarios - -### Workflow - -1. Create a `.feature` file under `features//` -1. Add step definitions under `features/step-definitions//` -1. Reuse existing steps from `common/` and other definition files before writing new ones -1. Run with `pnpm -C e2e e2e -- --tags @your-tag` to verify -1. Run `vp lint --fix --quiet` from the repository root and `pnpm -C e2e type-check` before committing - -### Feature file conventions - -Tag every feature or scenario with a capability tag. Add auth tags only when they clarify intent or change the browser session behavior: - -```gherkin -@datasets @authenticated -Feature: Create dataset - Scenario: Create a new empty dataset - Given I am signed in as the default E2E admin - When I open the datasets page - ... -``` - -- Capability tags (`@apps`, `@auth`, `@datasets`, …) group related scenarios for selective runs -- Auth/session tags: - - default behavior — scenarios run with the shared authenticated storageState unless marked otherwise - - `@unauthenticated` — uses a clean BrowserContext with no cookies or storage - - `@authenticated` — optional intent tag for readability or selective runs; it does not currently change hook behavior on its own -- `@prepared` — deterministic user behavior that requires the strict post-merge seed profile -- `@external-model` — scenario execution can call a real model provider. Use this only for runtime requests, not for scenarios that only require an active model fixture. -- `@external-tool` — scenario execution can call a real third-party tool provider. Use this only for runtime tool execution, not for plugin installation, discovery, or local deterministic tools. -- `@microphone` — runs the scenario in an isolated Chromium instance backed by the checked-in fake audio fixture and grants microphone permission only to that scenario context. -- `@browser-smoke` — focused keyboard and navigation coverage that runs in Chromium with the core suite and again in WebKit on CI. - External runtime commands are opt-in. `pnpm -C e2e e2e:external:prepare` prepares the fixed Agent v2 external-runtime seed and `pnpm -C e2e e2e:external` runs every `@external-model` or `@external-tool` scenario. CI uses `e2e:post-merge:prepare` followed by `e2e:post-merge` to run `@prepared` and external scenarios against one strict seed. - -The Agent v2 external runtime seed also prepares the workspace default Speech-to-Text model. `E2E_SPEECH_TO_TEXT_MODEL_PROVIDER` and `E2E_SPEECH_TO_TEXT_MODEL_NAME` select an existing model or the model configured through `E2E_MODEL_PROVIDER_CREDENTIALS_JSON`; they default to `openai` and `gpt-4o-mini-transcribe`. - -Some external runtime scenarios need feature-owned services in addition to a real model or tool provider. Do not overload `@external-model` or `@external-tool` to mean those services are available. For Agent v2, scenarios that require the standalone `dify-agent` run server use the feature tag `@agent-backend-runtime` plus the explicit step `the Agent v2 runtime backend is available`. Run them with `E2E_START_AGENT_BACKEND=1` to let E2E start `dify-agent` and the shellctl local sandbox required by its `dify.config`/`dify.shell` runtime layers, or set `E2E_AGENT_BACKEND_URL`/`AGENT_BACKEND_BASE_URL` when an existing server should be reused. - -Keep scenarios short and declarative. Each step should describe **what** the user does, not **how** the UI works. - -### Step definition conventions - -```typescript -import type { DifyWorld } from '../../support/world' -import { Then, When } from '@cucumber/cucumber' -import { expect } from '@playwright/test' - -When('I open the datasets page', async function (this: DifyWorld) { - await this.getPage().goto('/datasets') -}) -``` - -Rules: - -- Always type `this` as `DifyWorld` for proper context access -- Use `async function` (not arrow functions — Cucumber binds `this`) -- One step = one user-visible action or one assertion -- Keep steps stateless across scenarios; use `DifyWorld` properties for in-scenario state - -### Locator priority - -Follow the Playwright recommended locator strategy, in order of preference: - -| Priority | Locator | Example | When to use | -| -------- | ------------------ | ----------------------------------------- | ----------------------------------------- | -| 1 | `getByRole` | `getByRole('button', { name: 'Create' })` | Default choice — accessible and resilient | -| 2 | `getByLabel` | `getByLabel('App name')` | Form inputs with visible labels | -| 3 | `getByPlaceholder` | `getByPlaceholder('Enter name')` | Inputs without visible labels | -| 4 | `getByText` | `getByText('Welcome')` | Static text content | -| 5 | `getByTestId` | `getByTestId('workflow-canvas')` | Only when no semantic locator works | - -Avoid raw CSS/XPath selectors. They break when the DOM structure changes. - -### Assertions - -Use `@playwright/test` `expect` — it auto-waits and retries until the condition is met or the timeout expires: - -```typescript -// URL assertion -await expect(page).toHaveURL(/\/datasets\/[a-f0-9-]+\/documents/) - -// Element visibility -await expect(page.getByRole('button', { name: 'Save' })).toBeVisible() - -// Element state -await expect(page.getByRole('button', { name: 'Submit' })).toBeEnabled() - -// Negation -await expect(page.getByText('Loading')).not.toBeVisible() -``` - -Do not use manual `waitForTimeout` or polling loops. If you need a longer wait for a specific assertion, pass `{ timeout: 30_000 }` to the assertion. - -### Cucumber expressions - -Use Cucumber expression parameter types to extract values from Gherkin steps: - -| Type | Pattern | Example step | -| ---------- | ------------- | ---------------------------------- | -| `{string}` | Quoted string | `I select the "Workflow" app type` | -| `{int}` | Integer | `I should see {int} items` | -| `{float}` | Decimal | `the progress is {float} percent` | -| `{word}` | Single word | `I click the {word} tab` | - -Prefer `{string}` for UI labels, names, and text content — it maps naturally to Gherkin's quoted values. - -### Scoping locators - -When the page has multiple similar elements, scope locators to a container: - -```typescript -When('I fill in the app name in the dialog', async function (this: DifyWorld) { - const dialog = this.getPage().getByRole('dialog') - await dialog.getByPlaceholder('Give your app a name').fill('My App') -}) -``` - -### Failure diagnostics - -The `After` hook automatically captures diagnostics for failed, ambiguous, pending, undefined, or unknown scenarios: - -- Full-page screenshot (PNG) -- Page HTML dump -- Console errors and page errors - -Artifacts are saved to `cucumber-report/artifacts/` and attached to the HTML report. No extra code needed in step definitions. - -### Seed and fixture contracts - -Use `support/naming.ts` for generated test resource names. New app, Agent, dataset, file, or credential seeds should start with `E2E` so local and shared environments can identify disposable resources. - -Use `fixtures/test-materials/` for checked-in files that scenarios upload, preview, index, or retrieve. Keep these fixtures small and deterministic, and use `support/test-materials.ts` to resolve their absolute paths. - -Seed scripts own long-lived models, plugins, datasets, and fixed apps. Selected scenarios may resolve and verify those fixtures through explicit `Given` steps, but a missing or drifted fixture must fail the scenario. Do not represent environment readiness as Gherkin scenarios and do not conditionally skip behavior. - -Keep package-level support limited to broadly reusable primitives such as API clients, naming, fixture path resolution, and cleanup helpers. Feature-specific seed and fixture contracts belong under the owning feature's support folder. - -Use generated API contracts for Console/Web/Service API request, response, and payload shapes. Import the concrete type directly from `@dify/contracts/.../types.gen` when it exists, and do not hand-write duplicate response shapes or wrap generated types in local aliases just to preserve an older helper name. Keep local E2E types only for scenario state, fixture registries, helper input options, and intentionally narrowed test view models that are not complete API responses. - -### Console API and protocol boundaries - -The action under test belongs to the browser. `When` steps must use Playwright to perform the user action; do not replace the action with an API request. `Given` setup, seed preparation, persistence polling, and `After` cleanup may use APIs when that makes the scenario faster and more deterministic. `Then` should prefer a user-observable browser result; an API read is appropriate only when persistence itself is the asserted contract and the endpoint owns that state. - -For ordinary Console JSON operations and multipart uploads represented by Console OpenAPI, use the generated oRPC router with generated request and response validation enabled. A scenario client belongs to its `DifyWorld` and uses a scenario-owned authenticated request context that is independent from the behavior browser; seed processes own a standalone client for their process lifetime. Do not create a mutable cross-scenario API client, add TanStack Query caching to Cucumber, hand-write Console endpoint URLs, cast response JSON to an API DTO, or duplicate a generated Zod schema. When a browser action's captured response must provide an ID for cleanup, parse it with the generated response schema. - -Do not add a helper that only renames or forwards one generated operation. Call the generated client directly from the owning step, hook, or fixture orchestration. Keep a helper only when it owns a real test concern such as constructing a valid domain fixture, coordinating multiple operations, maintaining an invariant or cleanup registry, polling eventual consistency, deriving a narrowed test view, or adapting a non-OpenAPI protocol. - -SSE/event streams, binary downloads, redirect-only flows, external services, and infrastructure health/readiness checks may use a dedicated protocol adapter. Keep each exception centralized under its real owner and continue to use generated payload types where the contract covers the request. Multipart is not an exception merely because it carries a file: fix the backend OpenAPI schema and regenerate when the operation can be represented. - -Request or response validation failures are contract failures. Do not suppress them with casts, permissive fallback schemas, disabled validation, swallowed cleanup errors, or a second handwritten request path. Trace the mismatch to the endpoint's backend schema owner, update it according to `api/controllers/API_SCHEMA_GUIDE.md`, regenerate `@dify/contracts`, and keep the E2E assertion aligned with the product's real state owner rather than an internal backing resource. - -Use typed cleanup fields on `DifyWorld` for resource types created by scenarios, and use `DifyWorld.registerCleanup(...)` when a scenario creates any resource type that is not covered by typed cleanup fields. Typed cleanup should remove child or referencing resources before their owners, such as Agent files before Agents and workflow apps before Agents they reference. Cleanup failures should be attached to the report instead of being swallowed silently. Cleanup callbacks run after typed cleanup queues, even when the scenario fails. - -Scenario-owned setup may create disposable apps, Agents, files, credentials, drafts, or access toggles when the scenario owns their lifecycle and cleanup. Do not use scenario setup to silently fix a shared fixture; a missing or drifted fixed resource is a seed failure. - -Feature-specific seed contracts, resource readiness rules, tags, and scenario ownership can be documented in one scoped `AGENTS.md` at the feature root when a module becomes large enough to need it. Do not add deeper `AGENTS.md` files unless the nested module becomes independently owned. - -## Reusing existing steps - -Before writing a new step definition, inspect the existing step definition files first. Reuse a matching step when the wording and behavior already fit, and only add a new step when the scenario needs a genuinely new user action or assertion. Steps in `common/` are designed for broad reuse across all features. - -Or browse the step definition files directly: - -- `features/step-definitions/common/` — auth guards and navigation assertions shared by all features -- `features/step-definitions//` — domain-specific steps scoped to a single feature area +Failures produce screenshots and HTML captures under `cucumber-report/artifacts/`; the HTML and Cucumber Messages reports live under `cucumber-report/`. Backend and frontend startup logs live under `.logs/`. Additional CI lanes preserve their own report and log directories. diff --git a/packages/dify-ui/AGENTS.md b/packages/dify-ui/AGENTS.md index d37f44e934d..255d56f6bed 100644 --- a/packages/dify-ui/AGENTS.md +++ b/packages/dify-ui/AGENTS.md @@ -1,91 +1,31 @@ # @langgenius/dify-ui -Shared design tokens, the `cn()` utility, CSS-first Tailwind styles, and headless primitive components consumed by `web/`. +This package owns shared design tokens, CSS-first Tailwind styles, the `cn()` utility, and headless primitives consumed by `web/`. Read only the matching topic in [`README.md`] for public imports, forms, typed values, pickers, overlays, tokens, or tests. ## Component Authoring Rules -- Use `@base-ui/react` primitives + `cva` + `cn`. -- Inside dify-ui, cross-component imports use relative paths (`../button`). External consumers use subpath exports (`@langgenius/dify-ui/button`). -- No imports from `web/`. No dependencies on next / i18next / ky / jotai / zustand. -- One component per folder: `src//index.tsx`, optional `index.stories.tsx` and `__tests__/index.spec.tsx`. Add a matching `./` subpath to `package.json#exports`. -- Name the canonical public boundary and its associated public types after the primitive without a `Root` suffix (`Select` / `SelectProps`). Keep `Root` only when the subpath also exports a higher-level convenience component that must be distinguished from the low-level anatomy root (`CheckboxRoot` / `Checkbox`). Preserve the upstream anatomy in implementation type sources such as `BaseSelect.Root.Props`. -- Props pattern: `Omit & VariantProps & { /* custom */ }`. -- Use plain `Omit<...>` only for non-union Base UI props. When a prop changes the valid shape of related props (for example `value` / `defaultValue`, `multiple` / `value`, or `clearable` / `onChange`), model that relationship with an explicit discriminated union or a distributive helper instead of flattening the props. -- Preserve Base UI generic value contracts in wrappers. If the upstream primitive is generic, expose the same generic parameters and pass them through to the Base UI part, such as `Select.Root`, `RadioGroup`, or `Radio.Root`. -- Do not hard-code selection wrappers to `string` unless the upstream primitive is string-only. Select and radio wrappers that carry selected values should preserve the primitive's generic value contract. -- When a component accepts a prop typed from a shared internal module, `export type` it from that component so consumers import it from the component subpath. -- Prefer Base UI data attributes and CSS variables for visual states; do not mirror state in React solely to add classes. -- When a Base UI API or selector contract is unclear, read the docs linked from `README.md` and the local `@base-ui/react` `.d.ts` files before coding. +- Build primitives from `@base-ui/react`, `cva`, and `cn`. +- Use relative cross-component imports inside the package and subpath exports such as `@langgenius/dify-ui/button` from consumers. Add a matching `package.json#exports` entry for each public primitive. +- Keep one primitive per `src//` folder with optional colocated stories and tests. +- Do not import from `web/` or depend on Next.js, i18n, application state, or data-fetching libraries. +- Preserve upstream Base UI anatomy and generic value contracts. Use discriminated unions when one prop changes the valid shape of related props; do not flatten those relationships or hard-code selectable values to `string`. +- Export shared public types from the owning component subpath. +- Prefer Base UI data attributes and CSS variables for visual states; do not mirror primitive state in React solely to add classes. +- When a Base UI API or selector contract is unclear, read the current official documentation and local `@base-ui/react` type declarations before coding. -## Overlay Primitive Selection: Tooltip vs PreviewCard vs Popover +Use the README sections as the detailed owners: -Pick by the **trigger's purpose** and **a11y reach**, not visual richness. +- [Imports and public boundaries] +- [Typed value contracts] +- [Search and picker selection] +- [Tailwind and Figma radius mapping] +- [Overlay and portal contracts] +- [Development and test boundaries] -| Primitive | Opens on | Trigger's purpose | Content | Reachable on touch / SR? | -| ------------- | --------------------- | --------------------- | ------------------------- | ------------------------ | -| `Tooltip` | hover / focus | has its own action | short plain-text label | ❌ (label only) | -| `PreviewCard` | hover / focus | navigate through link | link destination preview | ❌ (visual enhancement) | -| `Popover` | click / tap (+ hover) | **open the popup** | anything, incl. long text | ✅ | - -Base UI decision rule ([docs]): - -> _"If the trigger's purpose is to open the popup itself, it's a popover. -> If the trigger's purpose is unrelated to opening the popup, it's a tooltip."_ - -Apply this first, then narrow: - -- `Tooltip` — ephemeral visual label. Trigger must already carry its own `aria-label` / visible text; tooltip mirrors it for sighted mouse/keyboard users. No interactive UI, no multi-line prose. Not dwell-able. -- `PreviewCard` — a visual enhancement for a link that previews its destination. Prefer the canonical anchor trigger and keep the popup non-interactive. Do not place unique or essential information or actions in the popup unless they are also available at the linked destination; touch and screen reader users cannot access the preview. If opening the popup is itself the trigger's purpose, or its content must be accessible across input modes, use `Popover` instead. -- `Popover` — any popup with its own interactions, or any "infotip" (`?` / `(i)` glyph whose sole purpose is to reveal help text). Pass `openOnHover` on `PopoverTrigger` for the infotip case — unlike `Tooltip` / `PreviewCard`, this stays accessible to touch and SR users because the popover still opens on tap and focus. - -Product-level polymorphic trigger compositions are local feature decisions. Document them in the owning feature and do not broaden or weaken the shared primitive contract to match one business workflow. - -## Border Radius: Figma Token → Tailwind Class Mapping - -The Figma design system uses `--radius/*` tokens whose scale is **offset by one step** from Tailwind CSS v4 defaults. When translating Figma specs to code, always use this mapping — never use `radius-*` as a CSS class, and never extend `borderRadius` in the preset. - -| Figma Token | Value | Tailwind Class | -| --------------- | ----- | ---------------- | -| `--radius/2xs` | 2px | `rounded-xs` | -| `--radius/xs` | 4px | `rounded-sm` | -| `--radius/sm` | 6px | `rounded-md` | -| `--radius/md` | 8px | `rounded-lg` | -| `--radius/lg` | 10px | `rounded-[10px]` | -| `--radius/xl` | 12px | `rounded-xl` | -| `--radius/2xl` | 16px | `rounded-2xl` | -| `--radius/3xl` | 20px | `rounded-[20px]` | -| `--radius/6xl` | 28px | `rounded-[28px]` | -| `--radius/full` | 999px | `rounded-full` | - -### Rules - -- **Do not** add custom `borderRadius` theme values. We use Tailwind v4 defaults and arbitrary values (`rounded-[Npx]`) for sizes without a standard equivalent. -- **Do not** use `radius-*` as CSS class names. The old `@utility radius-*` definitions have been removed. -- When the Figma MCP returns `rounded-[var(--radius/sm, 6px)]`, convert it to the standard Tailwind class from the table above (e.g. `rounded-md`). -- For values without a standard Tailwind equivalent (10px, 20px, 28px), use arbitrary values like `rounded-[10px]`. - -## Search / Picker Primitive Selection: Autocomplete vs Combobox vs Select - -Pick by whether the user is entering free-form text, choosing a remembered value, or selecting from a closed list. - -Base UI decision rules: - -- [Autocomplete docs]: use `Combobox` instead of `Autocomplete` if the selection should be remembered and the input value cannot be custom. -- [Combobox docs]: do not use `Combobox` for simple search widgets that require unrestricted text entry; use `Autocomplete` instead. - -Apply this split in Dify UI: - -- `Autocomplete` — free-form text input with optional suggestions or completions. The input value may be custom and does not necessarily become a selected option. Use for search boxes, command-style suggestions, tag suggestions, and async text completion. -- `Combobox` — searchable picker whose value is one or more selected items from a collection. The chosen value is remembered by the root, and free-form text is not the final value. Use for model pickers, user pickers, dataset/document pickers, and multi-select chips. -- `Select` — closed-list picker without text entry. Use when the option set is small or already scannable and filtering is unnecessary. - -Composition rules: - -- Keep Base UI primitive semantics visible in the public API. Export compound parts such as `ComboboxInputGroup`, `ComboboxInput`, `ComboboxContent`, `ComboboxList`, `ComboboxItem`, and `ComboboxItemIndicator` instead of wrapping them into one business component. -- For `Combobox` multiple selection, follow the official chips pattern: `ComboboxInputGroup` contains `ComboboxChips`, `ComboboxValue` renders `ComboboxChip` items, and `ComboboxInput` remains inside the chips row. Chips should wrap and let the input group grow vertically instead of forcing horizontal overflow. -- Content primitives must own their Base UI `Portal` and use `z-50` on `Positioner`, matching the overlay contract in `README.md`. Toast owns `z-60`. -- Use `w-(--anchor-width)` with viewport-aware max-width for `Autocomplete` and `Combobox` popups. Do not add `min-w-(--anchor-width)` when it would defeat available-width clamping. - -[Autocomplete docs]: https://base-ui.com/react/components/autocomplete.md#usage-guidelines -[Combobox docs]: https://base-ui.com/react/components/combobox.md#usage-guidelines -[docs]: https://base-ui.com/react/components/tooltip#infotips +[Development and test boundaries]: README.md#development +[Imports and public boundaries]: README.md#imports +[Overlay and portal contracts]: README.md#overlay--portal-contract +[Search and picker selection]: README.md#search-and-picker-selection +[Tailwind and Figma radius mapping]: README.md#tailwind-css-v4-integration +[Typed value contracts]: README.md#typed-value-contracts +[`README.md`]: README.md diff --git a/packages/dify-ui/README.md b/packages/dify-ui/README.md index 6345c4e010e..a39f4144768 100644 --- a/packages/dify-ui/README.md +++ b/packages/dify-ui/README.md @@ -129,6 +129,16 @@ For complex business forms, keep state ownership outside these primitives. TanSt Migration rule for `web/`: if a UI has a save/submit action, do not leave it as unrelated `Input` and `Button` pieces. Give it a real submit boundary with `Form` or a native `
`, attach visible field names through the appropriate label primitive (`FieldLabel`, `SelectLabel`, `SliderLabel`, or `FieldsetLegend`), expose helper/error text through `FieldDescription` / `FieldError`, and keep non-submit buttons as `type="button"`. +## Search and picker selection + +Choose the primitive by its value contract: + +- `Autocomplete` accepts free-form text with optional suggestions or completions. +- `Combobox` selects and remembers one or more values from a searchable collection. +- `Select` chooses from a closed, scannable list without text entry. + +Keep Base UI anatomy visible in public APIs instead of wrapping a picker into one business component. Multiple-selection comboboxes follow the official chips composition: chips and input share the input group, chips wrap, and the group grows vertically. Autocomplete and Combobox popups own their portals, use the package overlay layer, and size from `--anchor-width` with viewport-aware maximum width; do not force a minimum width that defeats viewport clamping. + ## Tailwind CSS v4 integration This package uses Tailwind CSS v4's CSS-first configuration model. Consumers should import Tailwind from their own root stylesheet, then import this package's CSS entry: @@ -144,6 +154,23 @@ If a consumer uses Dify UI source files through the workspace, add an explicit s @source '../packages/dify-ui/src'; ``` +Figma radius tokens are offset by one step from Tailwind CSS v4 defaults. Use this mapping rather than adding custom theme values or `radius-*` utilities: + +| Figma token | Tailwind class | +| --------------- | ---------------- | +| `--radius/2xs` | `rounded-xs` | +| `--radius/xs` | `rounded-sm` | +| `--radius/sm` | `rounded-md` | +| `--radius/md` | `rounded-lg` | +| `--radius/lg` | `rounded-[10px]` | +| `--radius/xl` | `rounded-xl` | +| `--radius/2xl` | `rounded-2xl` | +| `--radius/3xl` | `rounded-[20px]` | +| `--radius/6xl` | `rounded-[28px]` | +| `--radius/full` | `rounded-full` | + +Convert Figma output such as `rounded-[var(--radius/sm, 6px)]` to the mapped Tailwind class. Use an arbitrary value only when no standard class matches. + ## Overlay & portal contract Overlay primitives render their floating surfaces inside a [Base UI Portal] attached to `document.body`. This is the Base UI default — see the upstream [Portals][Base UI Portal] docs for the underlying behavior. Convenience content components such as `DialogContent`, `PopoverContent`, and `SelectContent` own their portal internally; primitives with explicit portal anatomy such as `Drawer` expose the matching `DrawerPortal` part so consumers can compose the full Base UI structure. @@ -189,7 +216,7 @@ See `[web/docs/overlay.md](../../web/docs/overlay.md)` for the web app overlay b ## Development -- `vp run @langgenius/dify-ui#lint` (from the repository root) — strict Oxlint checks for component source, stories, tests, and package configuration. +- `vp check packages/dify-ui` (from the repository root) — formatting, lint, and TypeScript diagnostics for the package. - `pnpm -C packages/dify-ui test` — Vitest unit tests for primitives. - `pnpm -C packages/dify-ui storybook` — Storybook on the default port. Each primitive has `index.stories.tsx`. - `pnpm -C packages/dify-ui test:storybook` — Storybook component tests in Vitest browser mode. Stories without `play` are render and a11y smoke tests; stories with `play` should cover public UI contracts such as opening overlays, keyboard navigation, disabled/loading guards, form submission, and controlled state updates. @@ -228,7 +255,6 @@ Set the Base UI test flag in a Vitest setup file to skip those waits: See `[AGENTS.md](./AGENTS.md)` for: - Component authoring rules (one-component-per-folder, `cva` + `cn`, relative imports inside the package, subpath imports from consumers). -- Figma `--radius/`_ token → Tailwind `rounded-_` class mapping. ## Not part of this package diff --git a/web/AGENTS.md b/web/AGENTS.md index abce2ee1352..4ef0f64bb39 100644 --- a/web/AGENTS.md +++ b/web/AGENTS.md @@ -1,53 +1,14 @@ ## Frontend Workflow -- Refer to the `./docs/test.md` and `./docs/lint.md` for detailed frontend workflow instructions. -- For frontend coding tasks, also apply the repo-local `how-to-write-component` skill when the change touches React components, state ownership, routing, styling, or Tailwind classes. -- For frontend reviews, use the repo-local `frontend-code-review` skill as the canonical checklist. +- Read `docs/test.md` only for frontend test work and `docs/lint.md` only when running or changing static checks. +- Use the repo-local `how-to-write-component` skill when implementation requires component ownership, state, data-flow, effect, or interaction-boundary decisions. Do not load it for test-only, copy-only, or styling-only changes. +- Use `frontend-code-review` only for explicit frontend review or audit requests, including test reviews. Use `frontend-testing` when writing or changing Vitest or React Testing Library tests. -## i18n +## Package Contracts -- User-facing strings must use `web/i18n/en-US/` keys instead of hardcoded text. -- When adding or renaming an i18n key, update all supported locale files with correct localized values. Do not leave fallback English in non-English locales unless the repo already intentionally does so for that exact key. - -## Backend API Calls - -- For new backend calls, and for surfaces already migrated to generated contracts, use `consoleQuery` / `consoleClient` from `@/service/client`. Do not add handwritten REST helpers, handwritten API types, mock-backed app state, or direct edits to generated contract files. - -## Overlay Components (Mandatory) - -- `../packages/dify-ui/README.md` is the permanent contract for overlay primitives, portals, root `isolation: isolate`, and the `z-50` / `z-60` layering. -- `./docs/overlay.md` records the current web overlay best practices. -- In new or modified code, use only overlay primitives from `@langgenius/dify-ui/*`. -- Do not introduce overlay imports from `@/app/components/base/*`; when touching existing callers, migrate them. - -## UI Components - -- Use `@langgenius/dify-ui/*` primitives and primitive data/CSS selectors first. Add call-site Tailwind only for real design deltas, avoid arbitrary values when token utilities exist, and keep focus rings visible without making inert layout regions focusable. - -## SVG Icons (Mandatory) - -- New custom SVG icons must be added under `../packages/iconify-collections/assets/...`. -- Run `pnpm --filter @dify/iconify-collections generate` and consume generated icons with Tailwind `i-custom-*` classes. -- Restart the web dev server after regenerating icons because Tailwind loads the custom icon collection at startup. -- Do not add new generated React icon components or JSON files under `app/components/base/icons/src/...`. -- See `../packages/iconify-collections/README.md` for the full workflow. - -## Design Token Mapping - -- When translating Figma designs to code, read `../packages/dify-ui/AGENTS.md` for the Figma `--radius/*` token to Tailwind `rounded-*` class mapping. The two scales are offset by one step. - -## Client State Management - -- Use local component state for state owned by one component. -- Use feature-level Jotai atoms for simple client state shared across components in the same feature, especially when components need a shared source of truth, derived values, or shared actions. -- Use existing feature stores for complex or high-frequency interaction state such as workflow canvas, drag, resize, and panel runtime state. -- For shared low-frequency, client-only persistence such as user preferences, dismissed notices, and UI defaults, use feature-owned storage modules built with `createLocalStorageState`. -- For high-frequency interactions, update the feature state during interaction and persist storage only on commit or settled updates. -- Keep storage keys and raw/custom formats in the owner module; callers should import the named storage hooks instead of scattering direct storage access. -- Do not add ad hoc global event listeners for shared state. Prefer atoms, existing stores, or a shared subscription hook so listeners are centralized and deduplicated. - -## Frontend Testing - -- `./docs/test.md` is the single source of truth for frontend automated test policy. -- Use the `frontend-testing` skill to apply that policy when writing or reviewing Vitest and React Testing Library tests. The skill must not introduce separate requirements. -- Add tests based on observable behavior and regression risk, not file count, hook usage, or coverage percentages. +- User-facing strings must use `web/i18n/en-US/` keys. When adding or renaming a key, update every supported locale with the correct localized value. +- For new backend calls and migrated surfaces, use generated `consoleQuery` / `consoleClient` APIs from `@/service/client`. Do not add handwritten REST helpers or DTO mirrors, mock-backed app state, or direct edits to generated contracts. +- Prefer `@langgenius/dify-ui/*` primitives, data attributes, and design tokens. Preserve a visible focus indicator on the final focusable element. +- Follow `docs/overlay.md` for overlay selection and migration. Migrate a legacy overlay only when the current behavior change actually involves that overlay boundary. +- For custom SVG icons, follow `../packages/iconify-collections/README.md`; do not add generated React icons under `app/components/base/icons/src/`. +- `docs/test.md` is the single source of truth for frontend automated-test policy. Skills may route and execute that policy but must not redefine it.