diff --git a/AGENTS.md b/AGENTS.md index 63d90e238..86a1e0ce3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,16 +2,24 @@ This file provides guidance to AI coding agents (Claude Code, etc.) when working with code in this repository. -> **Note:** This is the single source of truth. `CLAUDE.md` only contains `@AGENTS.md` and re-imports this file — do not add content to `CLAUDE.md`; put all guidance here in `AGENTS.md`. +> **Note:** This is the single source of truth relative to `CLAUDE.md` — `CLAUDE.md` only contains `@AGENTS.md` +> and re-imports this file; don't split guidance between the two, put it here. Detailed guidance beyond +> engineering principles and the architecture map is owned by the docs linked below (see +> [`docs/DOC-MAINTENANCE.md`](docs/DOC-MAINTENANCE.md)'s ownership table) — cross-link them, don't duplicate +> their content here. > **Before writing any code, read [`docs/develop.md`](docs/develop.md)** — the development spec (commands, > project structure, coding style, UI & theme rules, testing mechanics, i18n, and the commit/PR workflow). This > file keeps only the non-negotiable engineering principles and the architecture map; the concrete "how" lives -> in `DEVELOP.md`, and deep internals in [`docs/architecture.md`](docs/architecture.md). +> in that same guide, and deep internals in [`docs/architecture.md`](docs/architecture.md). > **To manually verify a feature actually works, read [`docs/verification.md`](docs/verification.md)** — drive > the real built extension end-to-end with one-shot throwaway scratch scripts (not the committed test suite). +> **Before building or modifying any page, dialog, or block, read [`docs/design.md`](docs/design.md)** — the +> design system: color tokens, component palette, layout/motion/state patterns, and the new-page recipe. Its +> Core Constraints apply to every UI change, not just new ones. + > **Before any translation/localization work, read [`docs/translation.md`](docs/translation.md)** — > the single source of truth for translation. Whenever you add or change localized content > (`src/locales//*.json` namespace files, per-language docs, UI copy, or test snapshots), you must first @@ -32,15 +40,16 @@ ScriptCat — Manifest V3 browser extension that runs Tampermonkey-compatible us > **UI stack.** The presentation layer (`src/pages/`) is built with **shadcn/ui + Tailwind CSS v4** on > **React 19** (migrated from Arco Design + UnoCSS). The concrete UI/theme rules live in > [`docs/develop.md`](docs/develop.md); the design system (color tokens, components, layout/motion/state -> patterns, new-page recipe) lives in [`docs/design.md`](docs/design.md) — read it before building any page. +> patterns, new-page recipe) lives in [`docs/design.md`](docs/design.md). ## Engineering Principles -These are non-negotiable and apply to every change, regardless of what `docs/develop.md` says about mechanics. +These are non-negotiable, regardless of what `docs/develop.md` says about mechanics — where a principle's scope +isn't universal, that's called out in the item itself. -- **Fix root causes, not symptoms — refactor over patch.** No `as any` / `// @ts-ignore` / try-catch swallow / defensive skips to make errors disappear. When the clean fix needs restructuring, refactor rather than bolt on a patch (宁愿重构也不要打补丁). If a test fails, fix the code, not the test. -- **Confirm before you fix.** Before touching a reported bug, reproduce it and confirm it actually exists — never fix from assumption. Then capture it in a failing test, then fix, **in that order** (确定 bug 存在 → 写测试 → 修复). -- **TDD/BDD first.** Write failing tests **before** implementation, using BDD-style Chinese `describe`/`it` titles. (Runner, mocks, and how to run tests are in `docs/develop.md`.) +- **Fix root causes, not symptoms — refactor over patch.** No `as any` / `// @ts-ignore` / try-catch swallow / defensive skips to make errors disappear (宁愿重构也不要打补丁). If a test fails, fix the code, not the test — the narrow exceptions (a wrong test contract; a test that never carried value) are in [`docs/references/develop-testing.md`](docs/references/develop-testing.md#writing-meaningful-tests-what-to-clean-up--not-write). +- **Confirm before you fix.** Before touching a reported bug, reproduce it and confirm it actually exists — never fix from assumption. Capture the reproduction, then fix, **in that order** (确定 bug 存在 → 写测试或记录验证证据 → 修复); how to reproduce and what counts as capture are in [`docs/verification.md`](docs/verification.md) and the TDD entry below. +- **TDD/BDD first, for changes that alter observable behavior.** Write failing tests **before** implementing new or changed behavior, using BDD-style Chinese `describe`/`it` titles. Two narrow exceptions — neither a blanket file/task category — are in [`docs/references/develop-testing.md`](docs/references/develop-testing.md#when-tdd-doesnt-apply). (Runner, mocks, and how to run tests are in `docs/develop.md`.) - **SOLID, high cohesion & low coupling — applied to the existing extension points.** `Repo` for new entities, `Group.on(...)` for new messages. Inject `Group` / `IMessageQueue` / DAOs via constructor; don't `new` them inside methods. Depend on narrow interfaces (`IMessageQueue`, not `MessageQueue`). - **Direct replacement over adapter sandwiches.** When swapping a backend/library, replace in place — no `interface Foo + LegacyImpl + NewImpl` unless both must coexist at runtime. - **Scope discipline — stay in your lane.** Bug fix ≠ cleanup PR. Touch only the files the task requires; leave unrelated files untouched (不要动和任务不相干的文件). Don't add helpers, abstractions, validation, or backwards-compat shims you don't need today. Three similar lines beats a premature abstraction. @@ -89,16 +98,12 @@ Execution paths: page scripts → `chrome.userScripts`; background → SW → Of ### Key Packages -`message/` (with mocks), `filesystem/` (WebDAV + local), `cloudscript/`, `eslint/` (userscript lint config — `eslint-plugin-userscripts`-based `defaultConfig` for the in-app editor), `chrome-extension-mock/`. +`message/` (with mocks), `filesystem/` (WebDAV, cloud drive providers, zip export — see [`docs/cloud-sync.md`](docs/cloud-sync.md)), `cloudscript/`, `eslint/` (userscript lint config — `eslint-plugin-userscripts`-based `defaultConfig` for the in-app editor), `chrome-extension-mock/`. -> The project's own custom ESLint rules live in `eslint-rules/` at the repo root (wired in `eslint.config.mjs`, **not** in `packages/eslint/`) and act as a **mechanical harness** for conventions that would otherwise rely on memory: -> - `require-last-error-check` — enforces `chrome.runtime.lastError` handling. -> - `scriptcat/no-i18n-default-value` — bans `t(key, { defaultValue })` inline fallbacks (they leak hardcoded text to every language and bypass the `i18n-usage` key check); add the key to `src/locales//*.json` instead. -> - `scriptcat/no-raw-color-classname` (`src/pages/**/*.tsx`) — bans raw palette/hex colors in `className` (`bg-white`, `text-gray-500`, `dark:bg-gray-800`, `bg-[#fff]`); use design tokens (`bg-background`/`text-foreground`/…) so light & dark both work. -> -> Two conventions are enforced via built-in rules in `eslint.config.mjs`: `no-restricted-imports` bans `@radix-ui/react-*` single packages (use the merged `radix-ui`) and the `sonner` `toast` export (use `notify`); `no-restricted-syntax` bans `forwardRef` across `src/pages/**` (use React 19 `function` + ref-prop). All four syntax-based harness rules are covered by `eslint-rules/harness.test.mjs`. -> -> Separately, type-aware rules run on `src/pages/**` (tests excluded) via `projectService` — `@typescript-eslint/no-floating-promises`, `no-misused-promises` (with `checksVoidReturn.attributes: false`, so `async` JSX handlers are allowed), and `await-thenable`, all `error` — to catch missing `await`s and promises misused as void callbacks. These need type information, so they are *not* part of `harness.test.mjs`. +> The project's own custom ESLint rules (`eslint-rules/` at the repo root, wired in `eslint.config.mjs` — +> **not** `packages/eslint/`, which is the unrelated userscript lint config) are documented in +> [`docs/develop.md`](docs/develop.md#eslint-custom-rules): exact rule names, scopes, and which are covered by +> `eslint-rules/harness.test.mjs`. ## Pull Request Description Format diff --git a/docs/design.md b/docs/design.md index 54edc4f75..c293ceb52 100644 --- a/docs/design.md +++ b/docs/design.md @@ -1,6 +1,6 @@ # ScriptCat Design System -> **A reuse-oriented design reference.** It consolidates the visual language that lives in `src/index.css` and the shadcn component layer into one place you can copy from: **color tokens (full light/dark values), the theming mechanism, the component palette, layout & responsive patterns, motion, state patterns, and an end-to-end new-page recipe.** Read this before building any new page, dialog, or block so it stays visually and behaviorally consistent with the rest of the app. +> **A reuse-oriented design reference.** It consolidates the visual language that lives in `src/index.css` and the shadcn component layer into one place you can copy from: **color tokens (full light/dark values), the theming mechanism, the component palette, layout & responsive patterns, motion, state patterns, and an end-to-end new-page recipe.** Read this before building or modifying any page, dialog, or block so it stays visually and behaviorally consistent with the rest of the app. > **Stack in one line:** React 19 + shadcn/ui (Radix primitives, `new-york` style) + Tailwind CSS v4 + React Router. Colors and motion are defined in the `@theme inline` block of `src/index.css`. **There is no `tailwind.config.js`** (Tailwind v4); PostCSS runs through `@tailwindcss/postcss` (`postcss.config.mjs`); **class names have no prefix** (`bg-background`, not `tw-bg-background`). diff --git a/docs/develop.md b/docs/develop.md index 0ce0ab4bd..af3fe0c49 100644 --- a/docs/develop.md +++ b/docs/develop.md @@ -42,7 +42,36 @@ Core entry points live in `src` (`service_worker.ts`, `content.ts`, `inject.ts`, ## Coding Style & Naming Conventions -Use strict TypeScript, React JSX runtime, 2-space indentation, semicolons, double quotes, trailing commas where valid, and a 120-column Prettier width. Prefer aliases from `tsconfig.json`: `@App/*`, `@Packages/*`, and `@Tests/*`. ESLint requires type-only imports, allows `_`-prefixed unused variables, warns on literal JSX text, and enforces `chrome.runtime.lastError` checks. Use `pnpm run lint-fix` for mechanical fixes. +Use strict TypeScript, React JSX runtime, 2-space indentation, semicolons, double quotes, trailing commas where valid, and a 120-column Prettier width. Prefer aliases from `tsconfig.json`: `@App/*`, `@Packages/*`, and `@Tests/*`. ESLint requires type-only imports, allows `_`-prefixed unused variables, errors on literal JSX text, and enforces `chrome.runtime.lastError` checks. Use `pnpm run lint-fix` for mechanical fixes. + +### ESLint custom rules + +The project's own custom rules live in `eslint-rules/` at the repo root (wired in `eslint.config.mjs`, **not** +`packages/eslint/`, which is the unrelated userscript lint config for the in-app editor) and act as a mechanical +harness for conventions that would otherwise rely on memory. Lint enforces the rules themselves — code violating +them fails CI — but this list of exactly which rule covers which scope, and which are covered by +`eslint-rules/harness.test.mjs`, is hand-maintained prose; re-verify specifics with `grep` rather than trusting +it as settled fact: + +- `chrome-error/require-last-error-check` — enforces `chrome.runtime.lastError` handling. Not covered by + `harness.test.mjs`. +- `scriptcat/no-i18n-default-value` — bans `t(key, { defaultValue })` inline fallbacks (they leak hardcoded text + to every language and bypass the `i18n-usage` key check); add the key to `src/locales//*.json` instead. +- `scriptcat/no-raw-color-classname` (`src/pages/**/*.tsx`) — bans raw palette/hex colors in `className` + (`bg-white`, `text-gray-500`, `dark:bg-gray-800`, `bg-[#fff]`); use design tokens (`bg-background`/ + `text-foreground`/…) so light & dark both work. + +Two conventions are enforced via built-in rules in `eslint.config.mjs`: `no-restricted-imports` bans +`@radix-ui/react-*` single packages (use the merged `radix-ui`) and the `sonner` `toast` export (use `notify`); +`no-restricted-syntax` bans `forwardRef` across `src/pages/**` (use React 19 `function` + ref-prop). +`eslint-rules/harness.test.mjs` covers exactly four of these: `no-i18n-default-value`, `no-raw-color-classname`, +the `radix-ui` pattern of `no-restricted-imports`, and `no-restricted-syntax` — not `require-last-error-check`, +and not the `sonner` pattern of `no-restricted-imports`. + +Separately, type-aware rules run on `src/pages/**` (tests excluded) via `projectService` — +`@typescript-eslint/no-floating-promises`, `no-misused-promises` (with `checksVoidReturn.attributes: false`, so +`async` JSX handlers are allowed), and `await-thenable`, all `error` — to catch missing `await`s and promises +misused as void callbacks. These need type information, so they are *not* part of `harness.test.mjs`. ### Language Conventions @@ -68,7 +97,7 @@ React 19 + shadcn/ui (Radix UI primitives, "new-york" style) + Tailwind CSS v4 + - No hard-coded colors. - **Design system** — the full color-token reference (light/dark values), component palette, layout & responsive patterns, motion guidance, state patterns, and a new-page recipe live in - [`DESIGN.md`](./design.md). Read it before building a new page, dialog, or block. + [`design.md`](./design.md). Read it before building or modifying any page, dialog, or block. ## Testing @@ -78,7 +107,7 @@ This project uses Vitest for unit tests and Playwright for end-to-end tests. ## i18n -i18next, 8 locales (`src/locales/`: en-US, zh-CN, zh-TW, ja-JP, de-DE, vi-VN, ru-RU, tr-TR); extension strings in `src/assets/_locales/`. ESLint `react/jsx-no-literals: warn` enforces translation. Each locale is split by namespace into multiple `*.json` files (`common.json`, `popup.json`, `script.json`, …), re-exported via the locale's `index.ts` and merged in `src/locales/locales.ts`. `defaultNS` is `common`; keys in any other namespace need the `ns:` prefix (e.g. `t("script:tags")`). For localization, edit the relevant namespace `*.json` under `src/locales//`; new locales must also be registered in `src/locales/locales.ts`. +i18next; extension strings in `src/assets/_locales/`. The current locale list is owned by [`docs/translation.md`](./translation.md). ESLint `react/jsx-no-literals: error` enforces translation. Each locale is split by namespace into multiple `*.json` files (`common.json`, `popup.json`, `script.json`, …), re-exported via the locale's `index.ts` and merged in `src/locales/locales.ts`. `defaultNS` is `common`; keys in any other namespace need the `ns:` prefix (e.g. `t("script:tags")`). For localization, edit the relevant namespace `*.json` under `src/locales//`; new locales must also be registered in `src/locales/locales.ts`. **Before translating, read [`docs/translation.md`](./translation.md)** — the translation/localization guide (terminology rules + per-locale `terminology-.md` specs). diff --git a/docs/references/develop-testing.md b/docs/references/develop-testing.md index 2f381c40d..fc4c1c6aa 100644 --- a/docs/references/develop-testing.md +++ b/docs/references/develop-testing.md @@ -14,13 +14,22 @@ budget at once. Chrome APIs mocked via invalid-selector `DOMException` lazily — the upstream eager construction captures a deep stack on every `matches()`/`querySelector()` call and cost ~15% of TSX suite time. -- Write failing tests **before** implementation; co-locate `*.test.ts`/`*.test.tsx` next to source (or place in `tests`). +- Co-locate `*.test.ts`/`*.test.tsx` next to source (or place in `tests`). - BDD-style Chinese `describe`/`it` titles. Use `describe.concurrent()` / `it.concurrent()` where independent. - Single file: `pnpm test -- --run path/to/file.test.ts`. - Playwright tests are `*.spec.ts` files in `e2e`; they run with one worker and retain failure artifacts. Run targeted tests while iterating, then run `pnpm run lint` plus the relevant full suite before a PR. +### When TDD doesn't apply + +Two exceptions to the TDD/BDD-first principle, neither a blanket file/task category — write a failing test for everything else: + +- **Genuinely behavior-preserving work** — refactors, type cleanup, dead-code removal, mechanical renames, or a config/dependency change confirmed not to alter behavior. Verify it instead of testing it. +- **Automated coverage is genuinely infeasible** — a pure visual/animation tweak, a bug reproducible only in a specific browser version or extension lifecycle stage, a copy/translation wording change, platform behavior that can't be automated reliably. Verify it manually and record the evidence instead of committing a pass-through or low-value test just to satisfy the rule — [`../verification.md`](../verification.md)'s scratch-extension workflow when the change needs the built extension or browser APIs, a simpler noted check otherwise. + ### Writing meaningful tests (what to clean up / not write) +**Two distinct situations — don't conflate them.** A test that fails because *its own asserted contract* is wrong (a stale fixture, an assertion that was incorrect from the start, a contract that legitimately changed) — fix the test and say why. A test that never carried value regardless of pass/fail (see below) — clean it up independent of whether it's currently failing. Neither is license to weaken a valid regression test just to make CI pass. + A test earns its place by exercising **our own logic** and failing on a real regression. Don't write the "tests nothing" kinds below — and clean them up when you find them (delete the test; don't touch business logic): - **Tautology** — asserting a constant equals its own literal definition (source `const FOO = [Type.BAR]`, test `expect(FOO).toEqual([Type.BAR])`). diff --git a/docs/verification.md b/docs/verification.md index 5a1af17b3..81db6718c 100644 --- a/docs/verification.md +++ b/docs/verification.md @@ -23,9 +23,17 @@ Promoting a scenario into the permanent suite is a *separate, deliberate* decisi permanent regression coverage. That path is owned by [`DEVELOP.md`](./develop.md), not this guide. **Reproducing a bug you intend to fix is *not* "casual verification."** A scratch reproduction is the *确定 bug -存在* step from [`../AGENTS.md`](../AGENTS.md) (确定 bug 存在 → 写测试 → 修复): it confirms the bug is real but does -**not** replace the test. Next, promote it into a *failing* committed test, then fix, then confirm it goes green — -that permanent test is owned by [`DEVELOP.md`](./develop.md) / [`../AGENTS.md`](../AGENTS.md). +存在* step in [`../AGENTS.md`](../AGENTS.md)'s TDD / Confirm-before-you-fix policy. In the general case it +confirms the bug is real but is not itself the required test — promote it into a committed failing test before +fixing. Under that policy's infeasible-automated-coverage exception (criteria in +[`references/develop-testing.md`](./references/develop-testing.md#when-tdd-doesnt-apply)), this scratch +reproduction — its `report.md`, screenshots, and observations — *is* the required evidence; no committed test +is needed. `AGENTS.md` owns the governing principle; `references/develop-testing.md` owns the exception +criteria — don't expect the exception boundary spelled out in `AGENTS.md` itself. + +Choose the reproduction method by what the bug depends on: a failing unit test for pure logic/parser/utility +bugs; this guide's scratch-script workflow (above) when it depends on the built extension, browser APIs, or +cross-context behavior. ## Prerequisite gate (cheap signals first)