From 4b24cf028a3fb7b2f705f91ddcdbd816a2cfb089 Mon Sep 17 00:00:00 2001 From: Alaister Young Date: Fri, 24 Jul 2026 11:58:49 +0800 Subject: [PATCH] chore(claude): improve CLAUDE.md files and skill triggering (#48261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves the repo's agent guidance: distills the always-required `studio-best-practices` skill into `apps/studio/CLAUDE.md`, tunes every skill description for reliable triggering, and mechanically enforces the generated-files rule. Grounded in Anthropic's official CLAUDE.md guidance (see justifications below). ## The main change: Studio CLAUDE.md gets a Code style section **Why:** `studio-best-practices` was a skill that instructed agents to *always* load it before any Studio code work. Anthropic's guidance draws the line as: sometimes-relevant guidance → skill (loaded on demand); always-relevant guidance → CLAUDE.md. A skill that must always load has failed the test for being a skill — it costs a tool-call round trip and, worse, silently does nothing in sessions that forget to load it. Since `apps/studio/CLAUDE.md` is lazy-loaded only when an agent touches Studio files, inlining is properly scoped: non-Studio sessions never pay for it. **Why not verbatim:** the skill was 175 lines, mostly ❌/✅ worked examples teaching practices models already know. Inlining it whole would push the file past the ~200-line point where Anthropic warns rules start getting lost. Instead each section was distilled to the rule it exists to enforce — e.g. the loading/error/success section kept its code block because the *shape* (early returns at top level, flat `&&` chains inline) is the prescription, and prose loses it. **The framing that makes the generic rules earn their place:** models default to matching surrounding code, and not all existing Studio code follows these practices. The section opens with "older Studio code predates some of these conventions — follow them rather than mirroring nearby legacy patterns," which converts otherwise-redundant React advice into an explicit instruction to break from local precedent. One rule was added that the old skill lacked: `useEffect` is for external-system sync only (~364 Studio files contain effects, many in patterns we don't want copied). **Changed:** - `apps/studio/CLAUDE.md` — new Code style section (84 lines total, within budget); skills table no longer mandates a pre-load - `.claude/CLAUDE.md` — dropped `pnpm install` from commands (guessable; Anthropic's test: "would removing this cause mistakes?") **Removed:** - `.claude/skills/studio-best-practices/` — fully absorbed; its cross-references to other skills were already covered by the skills routing table ## Skill description tuning Descriptions are the only signal an agent sees before deciding to load a skill, and the observed failure mode is under-triggering on tasks that don't name the skill. Nine descriptions reworded: front-loaded matchable keywords, added incidental-trigger cases (e.g. a new feature that adds copy is a `copywriting` moment), and disambiguated overlaps (`vitest` is now the API reference deferring to `studio-testing` for strategy). The `safe-sql-execution` rewrite was additionally validated with skill-creator's trigger-eval loop against 20 realistic queries: held-out test accuracy 54% → 71%, with zero false triggers across all iterations. (`vitest` shows under `.agents/` because `.claude/skills/vitest` symlinks there.) ## Generated-files enforcement **Added:** `permissions.deny` rules in `.claude/settings.json` for the six generated-file globs the root CLAUDE.md already lists. CLAUDE.md prose is advisory; permission rules are mechanical and also gate sandboxed Bash writes. (Verified live: the rule blocked an unintended regeneration of `database-types.ts` during testing.) ## To test - CI: prettier + typos checks pass (docs-only + settings change, no app code) - In a fresh Claude Code session in the repo: ask it to edit `apps/studio/routeTree.gen.ts` — should be denied by the new permission rule - Ask it to do any Studio UI task — it should pick up the Code style rules from `apps/studio/CLAUDE.md` without loading a best-practices skill ## Summary by CodeRabbit * **Documentation** * Updated development guidance for testing, copywriting, SQL safety, telemetry, queries, error handling, and toolbar reviews. * Restructured Vitest references into clearer tables and improved formatting across several guides. * Added Studio code-style conventions and clarified when task-specific guidance should be applied. * Removed outdated Studio best-practices guidance. * **Chores** * Added safeguards preventing edits to generated and protected files. * Simplified the documented development command sequence. --------- Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com> --- .agents/skills/vitest/SKILL.md | 54 +++--- .claude/CLAUDE.md | 1 - .claude/settings.json | 10 + .claude/skills/copywriting/SKILL.md | 2 +- .claude/skills/dev-toolbar-review/SKILL.md | 13 +- .claude/skills/safe-sql-execution/SKILL.md | 16 +- .claude/skills/studio-best-practices/SKILL.md | 175 ------------------ .claude/skills/studio-e2e-tests/SKILL.md | 8 +- .claude/skills/studio-error-handling/SKILL.md | 5 +- .claude/skills/studio-queries/SKILL.md | 3 +- .claude/skills/studio-testing/SKILL.md | 28 +-- .claude/skills/telemetry-standards/SKILL.md | 14 +- apps/studio/CLAUDE.md | 33 +++- 13 files changed, 134 insertions(+), 228 deletions(-) delete mode 100644 .claude/skills/studio-best-practices/SKILL.md diff --git a/.agents/skills/vitest/SKILL.md b/.agents/skills/vitest/SKILL.md index 0578bdcf3a8..69776b6edad 100644 --- a/.agents/skills/vitest/SKILL.md +++ b/.agents/skills/vitest/SKILL.md @@ -1,15 +1,21 @@ --- name: vitest -description: Vitest fast unit testing framework powered by Vite with Jest-compatible API. Use when writing tests, mocking, configuring coverage, or working with test filtering and fixtures. +description: >- + Vitest API and config reference (Jest-compatible) — mocking with vi.*, spies, + fake timers, coverage configuration, fixtures, snapshots, and test filtering. + Use for Vitest API and configuration questions anywhere in the monorepo; for + Studio-specific test strategy and component-test setup, start with + studio-testing and studio-mock-api-tests. metadata: author: Anthony Fu - version: "2026.1.28" + version: '2026.1.28' source: Generated from https://github.com/vitest-dev/vitest, scripts located at https://github.com/antfu/skills --- Vitest is a next-generation testing framework powered by Vite. It provides a Jest-compatible API with native ESM, TypeScript, and JSX support out of the box. Vitest shares the same config, transformers, resolvers, and plugins with your Vite app. **Key Features:** + - Vite-native: Uses Vite's transformation pipeline for fast HMR-like test updates - Jest-compatible: Drop-in replacement for most Jest test suites - Smart watch mode: Only reruns affected tests based on module graph @@ -22,31 +28,31 @@ Vitest is a next-generation testing framework powered by Vite. It provides a Jes ## Core -| Topic | Description | Reference | -|-------|-------------|-----------| -| Configuration | Vitest and Vite config integration, defineConfig usage | [core-config](references/core-config.md) | -| CLI | Command line interface, commands and options | [core-cli](references/core-cli.md) | -| Test API | test/it function, modifiers like skip, only, concurrent | [core-test-api](references/core-test-api.md) | -| Describe API | describe/suite for grouping tests and nested suites | [core-describe](references/core-describe.md) | -| Expect API | Assertions with toBe, toEqual, matchers and asymmetric matchers | [core-expect](references/core-expect.md) | -| Hooks | beforeEach, afterEach, beforeAll, afterAll, aroundEach | [core-hooks](references/core-hooks.md) | +| Topic | Description | Reference | +| ------------- | --------------------------------------------------------------- | -------------------------------------------- | +| Configuration | Vitest and Vite config integration, defineConfig usage | [core-config](references/core-config.md) | +| CLI | Command line interface, commands and options | [core-cli](references/core-cli.md) | +| Test API | test/it function, modifiers like skip, only, concurrent | [core-test-api](references/core-test-api.md) | +| Describe API | describe/suite for grouping tests and nested suites | [core-describe](references/core-describe.md) | +| Expect API | Assertions with toBe, toEqual, matchers and asymmetric matchers | [core-expect](references/core-expect.md) | +| Hooks | beforeEach, afterEach, beforeAll, afterAll, aroundEach | [core-hooks](references/core-hooks.md) | ## Features -| Topic | Description | Reference | -|-------|-------------|-----------| -| Mocking | Mock functions, modules, timers, dates with vi utilities | [features-mocking](references/features-mocking.md) | -| Snapshots | Snapshot testing with toMatchSnapshot and inline snapshots | [features-snapshots](references/features-snapshots.md) | -| Coverage | Code coverage with V8 or Istanbul providers | [features-coverage](references/features-coverage.md) | -| Test Context | Test fixtures, context.expect, test.extend for custom fixtures | [features-context](references/features-context.md) | -| Concurrency | Concurrent tests, parallel execution, sharding | [features-concurrency](references/features-concurrency.md) | -| Filtering | Filter tests by name, file patterns, tags | [features-filtering](references/features-filtering.md) | +| Topic | Description | Reference | +| ------------ | -------------------------------------------------------------- | ---------------------------------------------------------- | +| Mocking | Mock functions, modules, timers, dates with vi utilities | [features-mocking](references/features-mocking.md) | +| Snapshots | Snapshot testing with toMatchSnapshot and inline snapshots | [features-snapshots](references/features-snapshots.md) | +| Coverage | Code coverage with V8 or Istanbul providers | [features-coverage](references/features-coverage.md) | +| Test Context | Test fixtures, context.expect, test.extend for custom fixtures | [features-context](references/features-context.md) | +| Concurrency | Concurrent tests, parallel execution, sharding | [features-concurrency](references/features-concurrency.md) | +| Filtering | Filter tests by name, file patterns, tags | [features-filtering](references/features-filtering.md) | ## Advanced -| Topic | Description | Reference | -|-------|-------------|-----------| -| Vi Utilities | vi helper: mock, spyOn, fake timers, hoisted, waitFor | [advanced-vi](references/advanced-vi.md) | -| Environments | Test environments: node, jsdom, happy-dom, custom | [advanced-environments](references/advanced-environments.md) | -| Type Testing | Type-level testing with expectTypeOf and assertType | [advanced-type-testing](references/advanced-type-testing.md) | -| Projects | Multi-project workspaces, different configs per project | [advanced-projects](references/advanced-projects.md) | +| Topic | Description | Reference | +| ------------ | ------------------------------------------------------- | ------------------------------------------------------------ | +| Vi Utilities | vi helper: mock, spyOn, fake timers, hoisted, waitFor | [advanced-vi](references/advanced-vi.md) | +| Environments | Test environments: node, jsdom, happy-dom, custom | [advanced-environments](references/advanced-environments.md) | +| Type Testing | Type-level testing with expectTypeOf and assertType | [advanced-type-testing](references/advanced-type-testing.md) | +| Projects | Multi-project workspaces, different configs per project | [advanced-projects](references/advanced-projects.md) | diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 175aec83c51..3cabdcbc36c 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -24,7 +24,6 @@ pnpm 11 + Turborepo monorepo. Requires Node >= 22.13. ## Common Commands ```bash -pnpm install # install dependencies pnpm dev:studio # run Studio dev server → http://localhost:8082 pnpm dev:docs # run docs dev server pnpm dev:www # run www dev server diff --git a/.claude/settings.json b/.claude/settings.json index 3ffd1f88913..66fdfd3e7e5 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,4 +1,14 @@ { + "permissions": { + "deny": [ + "Edit(packages/api-types/types/**)", + "Edit(**/routeTree.gen.ts)", + "Edit(**/__generated__/**)", + "Edit(apps/docs/features/docs/generated/**)", + "Edit(apps/www/.generated/**)", + "Edit(supabase/functions/common/database-types.ts)" + ] + }, "hooks": { "SessionStart": [ { diff --git a/.claude/skills/copywriting/SKILL.md b/.claude/skills/copywriting/SKILL.md index 1a6994d8020..f69a7e0ed90 100644 --- a/.claude/skills/copywriting/SKILL.md +++ b/.claude/skills/copywriting/SKILL.md @@ -1,6 +1,6 @@ --- name: copywriting -description: Write or audit UI copy (buttons, labels, empty states, error messages, tooltips, form text) anywhere in the monorepo. Always check this before shipping or reviewing user-facing text. +description: Write or audit UI copy (buttons, labels, empty states, error messages, tooltips, form text) anywhere in the monorepo. Load it before shipping or reviewing any user-facing text — including when copy is incidental to the task, like a new feature that adds buttons, toasts, dialogs, or validation messages. --- # Copywriting diff --git a/.claude/skills/dev-toolbar-review/SKILL.md b/.claude/skills/dev-toolbar-review/SKILL.md index 8ead0b413f4..ae6b81d10ab 100644 --- a/.claude/skills/dev-toolbar-review/SKILL.md +++ b/.claude/skills/dev-toolbar-review/SKILL.md @@ -1,6 +1,7 @@ --- name: dev-toolbar-review -description: Use when reviewing PRs that touch packages/dev-tools/, packages/common/posthog-client.ts, +description: Safety rules for the dev toolbar, PostHog client, and feature flags. Use + when writing or reviewing any change to packages/dev-tools/, packages/common/posthog-client.ts, or packages/common/feature-flags.tsx. Covers environment guards, flag override cookies, telemetry event subscription, and SSE stream safety. --- @@ -30,10 +31,12 @@ so PRs touching only those files won't auto-request review. Watch for these in t **Files:** `packages/dev-tools/index.ts`, `DevToolbar.tsx`, `DevToolbarTrigger.tsx`, `DevToolbarContext.tsx` The toolbar uses two layers of protection: + - **Build-time tree-shaking** in `index.ts`: `process.env.NODE_ENV !== 'development'` ternaries that replace components with noops/stubs so the implementation is eliminated from production bundles. - **Runtime guards** in components: `IS_LOCAL_DEV` checks — `DevToolbar` and `DevToolbarTrigger` return `null` to hide themselves, while `DevToolbarProvider` passes children through (`<>{children}`) to preserve the component tree. **Check for:** + - Guards being removed or broadened. The toolbar is expanding to staging and preview deploys but must remain invisible in production. - Tree-shaking ternaries in `index.ts` staying intact — these are the primary production safety mechanism. - New components or exports that bypass the existing guard pattern. @@ -43,14 +46,17 @@ The toolbar uses two layers of protection: **Files:** `packages/dev-tools/DevToolbar.tsx`, `packages/common/posthog-client.ts`, `packages/common/feature-flags.tsx` The toolbar writes two cookies that override feature flags locally: + - `x-ph-flag-overrides` — PostHog flag overrides - `x-cc-flag-overrides` — ConfigCat flag overrides These are read by: + - `posthog-client.ts:getFeatureFlag()` — checks the PostHog override cookie before querying the SDK - `feature-flags.tsx` — merges both override cookies into the flag store during initialization **Check for:** + - Cookie name changes (must stay in sync across writer and all readers) - Changes to the merge/precedence logic in `feature-flags.tsx` (currently: `vercel-flag-overrides` first, then `x-cc-flag-overrides` takes precedence in local dev) - Override cookies being read outside the `IS_LOCAL_DEV` / `isLocalDev` guard — overrides must never affect production flag evaluation @@ -66,6 +72,7 @@ and `identify`. Note: `captureExperimentExposure` calls `posthog.capture()` dire without emitting to dev listeners — experiment exposure events are invisible in the toolbar. **Check for:** + - Changes to `emitToDevListeners` or `subscribeToEvents` that could introduce side effects on the actual capture path (e.g., throwing errors, blocking, mutating event data) - The listener set (`devListeners`) being iterated synchronously in a way that could delay event dispatch - New PostHog client methods that capture events but don't call `emitToDevListeners` (gap in toolbar visibility) @@ -78,6 +85,7 @@ The toolbar connects to `${apiUrl}/telemetry/stream` via Server-Sent Events to d server-side telemetry. Uses exponential backoff on connection errors. **Check for:** + - Changes to the SSE endpoint URL or `session_id` cookie handling - Reconnection logic changes that could cause excessive retries or connection leaks - Note: the stream endpoint lives in the platform repo — cross-repo changes need coordinated review @@ -85,16 +93,19 @@ server-side telemetry. Uses exponential backoff on connection errors. ### 5. App-Level Mounting **Provider + toolbar panel** (`DevToolbarProvider`, `DevToolbar`): + - `apps/studio/pages/_app.tsx` - `apps/www/pages/_app.tsx`, `apps/www/app/providers.tsx` - `apps/docs/features/app.providers.tsx` **Trigger button** (`DevToolbarTrigger`) — rendered separately in nav/header components: + - `apps/studio/components/layouts/Navigation/LayoutHeader/LayoutHeader.tsx` - `apps/www/components/Nav/index.tsx` - `apps/docs/components/Navigation/NavigationMenu/TopNavBar.tsx` **Check for:** + - Provider being added or removed from an app - `apiUrl` prop changes (must point to the correct platform API) - Rendering order changes that could affect the toolbar's access to PostHog context diff --git a/.claude/skills/safe-sql-execution/SKILL.md b/.claude/skills/safe-sql-execution/SKILL.md index c0f43cb8f8f..37674ce1114 100644 --- a/.claude/skills/safe-sql-execution/SKILL.md +++ b/.claude/skills/safe-sql-execution/SKILL.md @@ -1,6 +1,20 @@ --- name: safe-sql-execution -description: Safely execute SQL queries against a user database without risking SQL injection or other security vulnerabilities. +description: >- + Use whenever code will build, return, fetch, or execute SQL that runs against + a user's real Postgres database — even when the request reads like an ordinary + feature or bug fix and never says "security," "injection," or + "SafeSqlFragment." This covers: writing or editing any pg-meta function, query + builder, or endpoint that builds/returns SQL for database objects (tables, + views, functions, DB triggers, indexes, RLS policies); interpolating a + schema/table/column/search/route-param value into SQL text; storing, fetching, + or re-running SQL that round-trips from the database (a policy's definition, a + function/view definition, a snippet's saved content); and any + "Run"/"Apply"/"Execute" action that sends SQL to a project's database (SQL + editor run-selection, policy editor apply, snippet runner). Load this BEFORE + writing such code, not only when reviewing a finished diff. Skip only for + changes that never touch SQL text or execution — styling, unrelated data + hooks, non-SQL form validation, or UI layout work. --- # Safe SQL execution diff --git a/.claude/skills/studio-best-practices/SKILL.md b/.claude/skills/studio-best-practices/SKILL.md deleted file mode 100644 index 62eca4c6d1d..00000000000 --- a/.claude/skills/studio-best-practices/SKILL.md +++ /dev/null @@ -1,175 +0,0 @@ ---- -name: studio-best-practices -description: React and TypeScript best practices for Supabase Studio. Use when writing - or reviewing Studio components — covers boolean naming, component structure, loading/error - states, state management, custom hooks, event handlers, conditional rendering, - performance, and TypeScript conventions. ---- - -# Studio Best Practices - -Applies to `apps/studio/**/*.{ts,tsx}`. - -## Boolean Naming - -Use descriptive prefixes — derive from existing state rather than storing separately: - -- `is` — state/identity: `isLoading`, `isPaused`, `isNewRecord` -- `has` — possession: `hasPermission`, `hasData` -- `can` — capability: `canUpdateColumns`, `canDelete` -- `should` — conditional behavior: `shouldFetch`, `shouldRender` - -Extract complex conditions into named variables: - -```tsx -// ❌ inline multi-condition -{ - !isSchemaLocked && isTableLike(selectedTable) && canUpdateColumns && !isLoading &&