diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md index 7de2fa0e872..60778644da4 100644 --- a/.agents/skills/add-block/SKILL.md +++ b/.agents/skills/add-block/SKILL.md @@ -15,23 +15,9 @@ When the user asks you to create a block: 2. Configure all subBlocks with proper types, conditions, and dependencies 3. Wire up tools correctly -## Hard Rule: No Guessed Tool Outputs +## No guessed tool outputs -Blocks depend on tool outputs. If the underlying tool response schema is not documented or live-verified, you MUST tell the user instead of guessing block outputs. - -When block work changes tool execution, same-process work must use a registered -`InternalToolConfig.operation`. Never add a Sim `/api/...` self-hop or the retired -`directExecution` property. - -- Do NOT invent block outputs for undocumented tool responses -- Do NOT describe unknown JSON shapes as if they were confirmed -- Do NOT wire fields into the block just because they seem likely to exist - -If the tool outputs are not known, do one of these instead: -1. Ask the user for sample tool responses -2. Ask the user for test credentials so the tool responses can be verified -3. Limit the block to operations whose outputs are documented -4. Leave uncertain outputs out and explicitly tell the user what remains unknown +Block outputs mirror tool outputs. When a tool's response schema is neither documented nor live-verified, don't infer field names or JSON shapes — ask the user for sample responses or test credentials, limit the block to operations whose outputs are documented, or leave the uncertain outputs out and say exactly what remains unknown. ## Block Configuration Structure @@ -323,12 +309,10 @@ When several fields are mutually exclusive alternatives, mark them all `required "exactly one" at execution — a conditionally-required canonical pair rejects the workflow before the other paths ever get a chance to supply the value. -**Critical constraints:** -- `canonicalParamId` must NOT match any subblock's `id` in the same block -- A canonical group is **block-wide**, not per-operation: `buildCanonicalIndex` keys groups by - `canonicalParamId` across every subblock, and a group has exactly one `basicId`. Two operations - that each need a file pair need two distinct `canonicalParamId` values. -- All members of a group must share the same `required` status +**Constraints (block-wide):** +- `canonicalParamId` must not equal any subblock `id` in the block. +- One canonical id links exactly one basic/advanced pair for one logical parameter. Groups are keyed by canonical id across every subblock and hold one `basicId`, so two operations that each need a pair need two canonical ids. +- All members of a group share the same `required` status. ### Normalizing File Input in tools.config @@ -548,12 +532,6 @@ Maps multiple UI fields to a single serialized parameter: - In advanced mode: `channelId` input value → `params.channel` - The serializer consolidates based on current mode -**Critical constraints:** -- `canonicalParamId` must NOT match any other subblock's `id` in the same block (causes conflicts) -- A `canonicalParamId` links exactly one basic/advanced pair for a single logical parameter. Do NOT reuse the same `canonicalParamId` for different parameters, even under mutually-exclusive conditions/operations -- ONLY use `canonicalParamId` to link basic/advanced mode alternatives for the same logical parameter -- Do NOT use it for any other purpose - ## WandConfig Pattern Enables AI-assisted field generation. @@ -581,9 +559,9 @@ Enables AI-assisted field generation. - `'sql-query'` - SQL statements - `'timestamp'` - Adds current date/time context -## Tools Configuration +Use `wandConfig` on fields that are hard to fill by hand — timestamps (`generationType: 'timestamp'` injects the current date), comma-separated ID lists, complex query strings. Keep the prompt specific about the return format (e.g. 'Return ONLY the ISO 8601 timestamp string'). -**Important:** `tools.config.tool` runs during serialization before variable resolution. Put `Number()` and other type coercions in `tools.config.params` instead, which runs at execution time after variables are resolved. +## Tools Configuration **Preferred:** Use tool names directly as dropdown option IDs to avoid switch cases: ```typescript @@ -654,19 +632,12 @@ outputs: { // Use type: 'json' for complex objects or arrays (NOT type: 'array' with items) items: { type: 'json', description: 'List of items' }, metadata: { type: 'json', description: 'Response metadata' }, - - // Nested outputs (for structured data) - user: { - id: { type: 'string', description: 'User ID' }, - name: { type: 'string', description: 'User name' }, - email: { type: 'string', description: 'User email' }, - }, } ``` ### Typed JSON Outputs -When using `type: 'json'` and you know the object shape in advance, **describe the inner fields in the description** so downstream blocks know what properties are available. Block outputs have no nested `properties` form — always keep the output flat and put the shape in the `description`: +When using `type: 'json'` and you know the object shape in advance, **describe the inner fields in the description** so downstream blocks know what properties are available. Keep the output flat and put the shape in the `description`: ```typescript outputs: { @@ -681,10 +652,6 @@ outputs: { } ``` -Nested object outputs (`plan: { id: { type: 'string' }, ... }`) are a **tool-output** feature only — `OutputFieldDefinition` for blocks does not allow them and they fail TypeScript at build time. - -If the output shape is unknown because the underlying tool response is undocumented, you MUST tell the user and stop. Unknown is not the same as variable. Never guess block outputs. - ## V2 Block Pattern When creating V2 blocks (alongside legacy V1): @@ -727,7 +694,7 @@ export const ServiceV2Block: BlockConfig = { ## Registering Blocks -After creating the block, remind the user to register it in `apps/sim/blocks/registry-maps.ts` (the data maps live here; `registry.ts` holds only the accessor functions). Add the import and an entry to each map alphabetically: +Register the block in `apps/sim/blocks/registry-maps.ts` — add the import and an entry to each map alphabetically: ```typescript import { ServiceBlock, ServiceBlockMeta } from '@/blocks/blocks/service' @@ -887,41 +854,6 @@ Optional fields that are rarely used should be set to `mode: 'advanced'` so they } ``` -## WandConfig for Complex Inputs - -Use `wandConfig` for fields that are hard to fill out manually, such as timestamps, comma-separated lists, and complex query strings. This gives users an AI-assisted input experience. - -```typescript -// Timestamps - use generationType: 'timestamp' to inject current date context -{ - id: 'startTime', - title: 'Start Time', - type: 'short-input', - mode: 'advanced', - wandConfig: { - enabled: true, - prompt: 'Generate an ISO 8601 timestamp based on the user description. Return ONLY the timestamp string.', - generationType: 'timestamp', - }, -} - -// Comma-separated lists - simple prompt without generationType -{ - id: 'mediaIds', - title: 'Media IDs', - type: 'short-input', - mode: 'advanced', - wandConfig: { - enabled: true, - prompt: 'Generate a comma-separated list of media IDs. Return ONLY the comma-separated values.', - }, -} -``` - -## Naming Convention - -All tool IDs referenced in `tools.access` and returned by `tools.config.tool` MUST use `snake_case` (e.g., `x_create_tweet`, `slack_send_message`). Never use camelCase or PascalCase. - ## BlockMeta (Required) Every block file must export a `{Service}BlockMeta` alongside the block — **minimum 7 templates**. Look at existing examples in `apps/sim/blocks/blocks/` (e.g. `browser_use.ts`, `google_sheets.ts`) for the pattern. @@ -998,7 +930,7 @@ bun run apps/sim/scripts/check-canvas-sentences.ts --block={service} Adding a block on its own needs no **tool metadata** regeneration — a block references existing tool IDs through `tools.access` and does not change any tool's shape. -But if the same change also adds, edits **or removes** a tool, run `bun run tool-metadata:generate` and commit the result, or CI fails on stale artifacts. That matters here because a block's `outputs` are authored to match its tools' outputs, and the UI now reads those from the generated metadata rather than the executable registry — an unregenerated tool change makes the block's outputs disagree with what the panel renders. See `.agents/skills/tool-registry-boundary/SKILL.md`. +But if the same change also adds, edits **or removes** a tool, run `bun run tool-metadata:generate` and commit the result, or CI fails on stale artifacts. That matters here because a block's `outputs` are authored to match its tools' outputs, and the UI reads those from the generated metadata, not the executable registry — an unregenerated tool change makes the block's outputs disagree with what the panel renders. See `.agents/skills/tool-registry-boundary/SKILL.md`. A visible integration block does require the generated integration catalog and docs to be refreshed. After adding or changing one, run: @@ -1052,9 +984,9 @@ changes. ## Final Validation (Required) -After creating the block, you MUST validate it against every tool it references: +Validate the block against every tool in `tools.access`: -1. **Read every tool definition** that appears in `tools.access` — do not skip any +1. **Read each tool definition** in `tools.access` 2. **For each tool, verify the block has correct:** - SubBlock inputs that cover all required tool params (with correct `condition` to show for that operation) - SubBlock input types that match the tool param types (e.g., dropdown for enums, short-input for strings) @@ -1063,11 +995,11 @@ After creating the block, you MUST validate it against every tool it references: 3. **Verify block outputs** cover the key fields returned by all tools 4. **Verify conditions** — each subBlock should only show for the operations that actually use it 5. **Verify `{Service}BlockMeta` is exported** with at least 7 templates, each having `icon`, `title`, `prompt`, `modules`, `category`, and `tags` -6. **If any tool outputs are still unknown**, explicitly tell the user instead of guessing block outputs +6. **List any tool outputs still unknown** rather than guessing block outputs 7. **Verify the tool execution boundary** — blocks never create or call API routes. Every referenced tool must already be either a registered `InternalToolConfig.operation` or an absolute external - HTTP(S) `ToolConfig.request`. If transport needs to change, use the `add-tools` skill; do not add a - same-origin `/api/...` hop from the block. + HTTP(S) `ToolConfig.request`. If transport needs to change, use the `add-tools` skill; never add a + same-origin `/api/...` hop or a `directExecution` property from the block. ## Option Lists: `selectorKey` or `options`, never a per-block fetcher @@ -1103,7 +1035,7 @@ options: (params) => { } ``` -**Never fetch inside `options`, and never reach into the stores from a block definition.** A fetcher that resolves its credential with `readSubBlockValue(blockId, ...)` only works on the canvas — every surface that is not the editor gets an empty list. `fetchOptions`/`fetchOptionById` were removed for exactly this reason. +**Never fetch inside `options`, and never reach into the stores from a block definition.** A fetcher that resolves its credential with `readSubBlockValue(blockId, ...)` only works on the canvas — every surface that is not the editor gets an empty list. Two rules the checks enforce: diff --git a/.agents/skills/add-column-type/SKILL.md b/.agents/skills/add-column-type/SKILL.md index 1c54b12cb3a..4462dacb060 100644 --- a/.agents/skills/add-column-type/SKILL.md +++ b/.agents/skills/add-column-type/SKILL.md @@ -8,7 +8,7 @@ argument-hint: A column type is **one file** in `apps/sim/lib/table/column-types/` plus a registry entry. Everything that varies per type — label, icon, storage cast, coercion, validation, conversion compatibility, formatting, editor, filter operators — lives on that one object, so no consumer needs editing. -This was not always true: adding `currency` originally took ~40 edits across 32 `switch` arms and 26 UI branches, each of which failed **silently** when missed. The registry exists to make that impossible, so the rule is absolute: **if you find yourself adding a `case 'yourtype':` anywhere outside `column-types/`, the registry is missing a field. Add the field instead.** +A `case 'yourtype':` outside `column-types/` fails **silently** when missed (a wrong `jsonbCast` breaks every filter on the column). The registry exists to make that impossible, so the rule is absolute with one documented exception (`import.ts`'s `coerceValue`, see "Traps" below): **if you find yourself adding a `case 'yourtype':` anywhere else outside `column-types/`, the registry is missing a field. Add the field instead.** ## Hard Rule: the compiler tells you what to do @@ -116,9 +116,9 @@ Prefer set-based SQL. When the transform genuinely needs JS (`currency`'s separa ## Watch out -- **Import cycles.** `column-types/select.ts` imports `select-values.ts`, so `select-values.ts` must **not** import the registry — that closes a cycle and fails at module init. Inside a type's own helper module the string literal is the implementation, not a config leak. +- **Import cycles.** `column-types/select.ts` imports `lib/table/select-values.ts`, so `select-values.ts` must **not** import the registry — that closes a cycle and fails at module init. Inside a type's own helper module the string literal is the implementation, not a config leak. - **The client-safe boundary.** `registry.ts` and everything it imports must stay free of `@sim/db`, `drizzle-orm`, and `next/server` — the tables grid imports it directly. A React icon is fine (it's a component *reference*, never called server-side). Only `registry.server.ts` may touch drizzle. -- **Don't re-export the registry from `@/lib/table`.** 44 server modules import that barrel; routing this through it pulls `@sim/emcn/icons` into all of them. Deep-import `@/lib/table/column-types`. +- **Don't re-export the registry from `@/lib/table`.** Dozens of server modules import that barrel; routing this through it pulls `@sim/emcn/icons` into all of them. Deep-import `@/lib/table/column-types`. - **`import.ts`'s `coerceValue` is a SECOND write path and is not opt-in.** Importing into a column of your type always hits it, and its `default` arm silently `String(value)`s — so a missing `case` stores text in a column whose `jsonbCast` is numeric, and then every filter and sort on that column errors in Postgres. Add a `case`, even though the switch compiles without one. (It is deliberately separate from the registry's `coerce`: an import wants an unparseable value to survive as its raw string so the row error can name it.) - **CSV inference** is an ordered heuristic in `import.ts`, deliberately not registry-driven. A new type is not inferred from a CSV unless you extend `inferColumnType` — usually you should not, since inference cannot supply configuration (an option set, a currency code). diff --git a/.agents/skills/add-connector/SKILL.md b/.agents/skills/add-connector/SKILL.md index c14c5a11ade..6d7ef0b2930 100644 --- a/.agents/skills/add-connector/SKILL.md +++ b/.agents/skills/add-connector/SKILL.md @@ -469,7 +469,7 @@ The assigned mapping (`semantic id → slot`) is stored in `sourceConfig.tagSlot ## `@/connectors/utils` Helpers -Reuse these instead of inlining the same logic (the validator enforces them): +Reuse these instead of inlining the same logic: - `htmlToPlainText(html)` — strip HTML to plain text before indexing `ExternalDocument.content`. Never index raw HTML. - `computeContentHash(content)` — stable content hash for change detection. @@ -538,7 +538,7 @@ If `ExternalDocument.sourceUrl` is set, the sync engine stores it on the documen If `listDocuments` can ever return **less than the full source set** on a non-incremental sync — a `maxItems`/`maxDocuments`-style cap, or a transient per-item error that drops a still-existing document from the listing — it MUST set `syncContext.listingCapped = true` when that happens. -The sync engine reconciles deletions by comparing the full listing against stored documents: anything not seen is **hard-deleted** (sync-engine.ts, gated on `!syncContext?.listingCapped`). A truncated listing without this flag deletes every real document beyond the cap. This was the single most common bug found when auditing connectors — do not omit it. +The sync engine reconciles deletions by comparing the full listing against stored documents (`shouldReconcileDeletions` in `lib/knowledge/connectors/sync-engine.ts`, gated on `!syncContext?.listingCapped`). Anything not seen is tombstoned on that sync and hard-deleted when the next sync still does not see it — so a truncated listing without this flag eventually removes every real document beyond the cap. ```typescript if (hitLimit && syncContext) { @@ -565,7 +565,7 @@ You never need to modify the sync engine when adding a connector. ## Icon -The `icon` field on `ConnectorConfig` is used throughout the UI — in the connector list, the add-connector modal, and as the document icon in the knowledge base table (replacing the generic file type icon for connector-sourced documents). The icon is read from `CONNECTOR_REGISTRY[connectorType].icon` at runtime — no separate icon map to maintain. +The `icon` field on `ConnectorConfig` is used throughout the UI — in the connector list, the add-connector modal, and as the document icon in the knowledge base table (replacing the generic file type icon for connector-sourced documents). The icon is read from `CONNECTOR_META_REGISTRY[connectorType].icon` (the client-safe registry) at runtime — no separate icon map to maintain. If the service already has an icon in `apps/sim/components/icons.tsx` (from a tool integration), reuse it. Otherwise, ask the user to provide the SVG. @@ -602,7 +602,8 @@ export const CONNECTOR_META_REGISTRY: ConnectorMetaRegistry = { - **OAuth + contentDeferred**: `apps/sim/connectors/google-drive/google-drive.ts` — file download with metadata-based hash, `orderBy` for deterministic pagination - **OAuth + contentDeferred (blocks API)**: `apps/sim/connectors/notion/notion.ts` — complex block content extraction deferred to `getDocument` - **OAuth + contentDeferred (git)**: `apps/sim/connectors/github/github.ts` — blob SHA hash, tree listing -- **OAuth + inline content**: `apps/sim/connectors/confluence/confluence.ts` — multiple config field types, `mapTags`, label fetching +- **OAuth + inline content**: `apps/sim/connectors/slack/slack.ts` — list API returns message content inline, metadata-derived `contentHash` +- **OAuth + contentDeferred + config fields**: `apps/sim/connectors/confluence/confluence.ts` — multiple config field types, `mapTags`, label fetching - **API key**: `apps/sim/connectors/fireflies/fireflies.ts` — GraphQL API with Bearer token auth ## Checklist diff --git a/.agents/skills/add-enrichment/SKILL.md b/.agents/skills/add-enrichment/SKILL.md index 8ea6117db57..34810e72f41 100644 --- a/.agents/skills/add-enrichment/SKILL.md +++ b/.agents/skills/add-enrichment/SKILL.md @@ -22,9 +22,9 @@ Because enrichments run on Sim's hosted keys by default, **every provider tool y ## Architecture (what you're plugging into) -- **`enrichments/types.ts`** — `EnrichmentConfig { id, name, description, icon, inputs, outputs, providers }` and `EnrichmentProvider { id, label, toolId, buildParams, mapOutput }`. Providers are **plain data** (no `@/tools` import) so the catalog stays client-safe. -- **`enrichments/providers.ts`** — `toolProvider(...)` (typed passthrough) plus shared input helpers: `str(v)`, `normalizeDomain(v)`, `firstNonEmpty(arr)`, `splitName(fullName)`. -- **`enrichments/run.ts`** — the server-only cascade runner. Calls `executeTool(provider.toolId, { ...params, _context: { workspaceId } })`, accumulates hosted-key cost, returns the first non-empty mapped result. **You do not edit this** — it works for any registry entry. +- **`enrichments/types.ts`** — `EnrichmentConfig { id, name, description, icon, inputs, outputs, providers }` and `EnrichmentProvider { id, label, toolId, buildParams, projectFailure, mapOutput }` — `toolProvider` fills `projectFailure` with the standard HTTP projection; override it only for a provider whose failure shape is nonstandard (see `enrichments/provider-failures/`). Providers are **plain data** (no `@/tools` import) so the catalog stays client-safe. +- **`enrichments/providers.ts`** — `toolProvider(...)` (typed passthrough) plus shared input helpers: `str(v)`, `normalizeDomain(v)`, `firstNonEmpty(arr)`, `splitName(fullName)`, and `projectEnrichmentProviderFailure`. +- **`enrichments/run.ts`** — the server-only cascade runner. Calls `executeTool(provider.toolId, { ...params, _context: { workspaceId, userId } })`, accumulates hosted-key cost, returns the first non-empty mapped result. **You do not edit this** — it works for any registry entry. - **`enrichments/registry.ts`** — `ENRICHMENT_REGISTRY` / `ALL_ENRICHMENTS` / `getEnrichment`. Register new entries here. Outputs automatically become table columns; billing, the catalog/sidebar UI, the column meta-header icon, and per-row execution all work with no extra wiring. @@ -60,7 +60,7 @@ Why it matters: the cascade runner only bills (and only reads `output.cost.total ## Step 3: Write the enrichment definition -Create `apps/sim/enrichments/{name}/{name}.ts` and a barrel `index.ts`. Mirror the existing entries (`work-email`, `phone-number`, `company-domain`, `company-info`). +Create `apps/sim/enrichments/{name}/{name}.ts` and a barrel `index.ts`. Mirror the entries registered in `enrichments/registry.ts`. ```typescript import { SomeIcon } from '@sim/emcn/icons' diff --git a/.agents/skills/add-hosted-key/SKILL.md b/.agents/skills/add-hosted-key/SKILL.md index 78f127b10f8..a90895d5a46 100644 --- a/.agents/skills/add-hosted-key/SKILL.md +++ b/.agents/skills/add-hosted-key/SKILL.md @@ -202,7 +202,7 @@ The visibility is controlled by `isSubBlockHidden()` in `lib/workflows/subblocks ### Excluding Specific Operations from Hosted Key Support -When a block has multiple operations but some operations should **not** use a hosted key (e.g., the underlying API is deprecated, unsupported, or too expensive), use the **duplicate apiKey subblock** pattern. This is the same pattern Exa uses for its `research` operation: +When a block has multiple operations but some operations should **not** use a hosted key (e.g., the underlying API is deprecated, unsupported, or too expensive), use the **duplicate apiKey subblock** pattern: 1. **Remove the `hosting` config** from the tool definition for that operation — it must not have a `hosting` object at all. 2. **Duplicate the `apiKey` subblock** in the block config with opposing conditions: @@ -235,9 +235,7 @@ Both subblocks share the same `id: 'apiKey'`, so the same value flows to the too To exclude multiple operations, use an array: `{ field: 'operation', value: ['op_a', 'op_b'] }`. -**Reference implementations:** -- **Exa** (`blocks/blocks/exa.ts`): `exa_research` operation excluded from hosting — duplicate `apiKey` pair around lines ~348-365 -- **Google Maps** (`blocks/blocks/google_maps.ts`): `speed_limits` operation excluded from hosting (deprecated Roads API) +**Reference implementation:** `blocks/blocks/google_maps.ts` — `speed_limits` (deprecated Roads API) is excluded from hosting with the duplicate `apiKey` pair. ## Step 5: Add to the BYOK Settings UI diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md index b727fa06bb3..850571a9cef 100644 --- a/.agents/skills/add-integration/SKILL.md +++ b/.agents/skills/add-integration/SKILL.md @@ -68,7 +68,7 @@ Choose the tool boundary before writing the declaration: - Use `ToolConfig.request` only for an absolute external HTTP(S) provider endpoint. Never point a tool at `/api/...`, construct an absolute URL back to Sim, declare -`request.internal`, add the retired `directExecution` property, or add an API route merely to reuse code, normalize files, or authorize +`request.internal`, add a `directExecution` property (it fails `bun run check:tool-request-boundary`), or add an API route merely to reuse code, normalize files, or authorize resources. A real external/browser route and an in-process tool may share the same operation, but neither calls the other. Follow the full transport and handler rules in the `add-tools` skill. @@ -92,31 +92,7 @@ export interface {Service}Response extends ToolResponse { } ``` -**Tool file pattern:** -```typescript -export const {service}{Action}Tool: InternalToolConfig = { - id: '{service}_{action}', - name: '{Service} {Action}', - description: '...', - version: '1.0.0', - - oauth: { required: true, provider: '{service}' }, // If OAuth - - params: { - accessToken: { type: 'string', required: true, visibility: 'hidden', description: '...' }, - // ... other params - }, - - operation: { - input: (params) => ({ - accessToken: params.accessToken, - // Map only the semantic operation input. - }), - }, - - outputs: { /* ... */ }, -} -``` +**Tool file pattern:** an external provider API uses `ToolConfig` with `request` (absolute `https://` URL, headers, body, `transformResponse`); same-process Sim work uses `InternalToolConfig` with `operation`. Both full templates, param visibility rules, and output typing live in `.agents/skills/add-tools/SKILL.md` — read it before writing the first tool. ### Critical Rules - `visibility: 'hidden'` for OAuth tokens @@ -127,226 +103,38 @@ export const {service}{Action}Tool: InternalToolConfig = { - Set `optional: true` for outputs that may not exist - Never output raw JSON dumps - extract meaningful fields - When using `type: 'json'` and you know the object shape, define `properties` with the inner fields so downstream consumers know the structure. Only use bare `type: 'json'` when the shape is truly dynamic -- If you do not know the response JSON shape from docs or verified examples, you MUST tell the user and stop. Never guess outputs or response mappings. ### Resolved Secrets at Model and Persistence Boundaries -Classify every request field before implementing the tool: - -This is opt-in, not a blanket integration migration. Add a model-input declaration only when the -service's official documentation or an unambiguous local execution path proves that the exact -field is consumed by an AI model. If that cannot be established, preserve existing tool behavior -and leave the field unannotated. - -- **Ordinary provider/API input:** leave it unchanged. Explicit `{{...}}` references resolve and are - sent with their normal request semantics. A URL, domain, resource ID, control field, or opaque - payload is not model-visible merely because the provider is AI-backed or may process the - referenced resource later. -- **Text or structured content consumed by an AI model:** declare `request.modelInput` for an - external provider request or `operation.modelInput` for an in-process operation, with - `mode: 'project'` and select only the exact model-visible fields. The shared executor replaces - activated Sim secrets with canonical `{{NAME}}` labels before request formatting. For nested or - JSON-string fields, use a small shared selector plus `applyProjected`; verify that selecting the - rebuilt params reproduces the projected selection. -- **Serialized model content sent directly to an external provider:** include the serialized - top-level param in `request.modelInput`. Project the private copy before the existing request - formatter parses it; keep formatter behavior deterministic when a whole-value placeholder is not - valid in the serialized grammar. Do not introduce a second hard-rejection path. -- **Opaque model input owned by an in-process operation** such as inline audio, image, video, or - document bytes: add `privateProvenance` to the operation model-input declaration, or use - `mode: 'private-provenance'` when there is no textual projection. Do not select storage keys, - paths, signed URLs, or ordinary remote URLs as byte provenance; the owning operation must - authorize stored bytes independently at model egress. The operation must call - `validateOpaqueModelInputProvenance` before downloading or sending content to the model and must - apply the workspace-file provenance guard before reading a persisted workspace file. -- **Sim-owned durable storage or internal execution handoff** that can later enter a workflow/model - (table cells, Agent memory, knowledge documents/chunks, workspace-file contents, or child-workflow - input): transport encrypted field-scoped provenance with `operation.secretProvenance`. The - operation validates the exact selection and trusted scope, then persists, imports, or propagates - it at the owning boundary. Preserve shared legacy behavior for rows/files whose provenance marker - is `NULL`; never invent a tool-local migration rule. - -Hard rules: - -- Never substitute secret plaintext into source or serialize plaintext provenance. -- Never hand-roll private provenance headers/envelopes; the shared `executeTool` boundary owns - transport and strips private metadata from functional results. -- Never attach private provenance to an external URL. Project proven - model-visible external fields with `request.modelInput`; otherwise preserve ordinary request - semantics. Use a registered in-process operation when encrypted provenance must cross the - boundary. -- Never sanitize arbitrary third-party tool results. Projection applies only to secrets activated - by Sim's resolved-secret provenance for that execution/tool call. -- Do not add provenance merely because a value is persisted, returned by a tool, or appears in a - filename. Require a concrete Sim `{{...}}` resolution path and a later model/log boundary. If an - unsupported field can resolve a secret but does not justify durable tracking (for example a - `file_write` path), reject it at that exact ingress. -- At diagnostic boundaries, project only values carrying execution-scoped provenance. Ordinary - provider responses, filenames, URLs, and errors remain unchanged when Sim did not resolve a - secret into them. - -Add focused tests covering named projection, ordinary identical text without provenance, nested and -serialized shape handling, unchanged ordinary external inputs, malformed/incomplete private metadata -failing closed, headerless legacy requests, and absence of private metadata in the public tool result. -For durable sinks, also cover legacy `NULL` markers, exact-empty new writes, tracked secret writes, -stale/missing sidecars, and scope isolation. +Classify every request field (ordinary provider input / AI-consumed text / opaque model bytes / +Sim-durable storage) before implementing the tool and apply the shared projection or provenance +mechanism only where a concrete Sim `{{...}}` resolution path reaches a later model or log boundary. +Full rules and the required tests are in `.agents/skills/add-tools/SKILL.md` → "Resolved Secrets and +Provenance Boundaries". ## Step 3: Create Block ### File Location `apps/sim/blocks/blocks/{service}.ts` -### Block Structure -```typescript -import { {Service}Icon } from '@/components/icons' -import type { BlockConfig } from '@/blocks/types' -import { AuthMode, IntegrationType } from '@/blocks/types' -import { getScopesForService } from '@/lib/oauth/utils' - -export const {Service}Block: BlockConfig = { - type: '{service}', - name: '{Service}', - description: '...', - longDescription: '...', - docsLink: 'https://docs.sim.ai/integrations/{service}', - category: 'tools', - integrationType: IntegrationType.X, // Primary category (see IntegrationType enum) - tags: ['oauth', 'api'], // Cross-cutting tags (see IntegrationTag type) - bgColor: '#HEXCOLOR', - icon: {Service}Icon, - authMode: AuthMode.OAuth, // or AuthMode.ApiKey - - subBlocks: [ - // Operation dropdown - { - id: 'operation', - title: 'Operation', - type: 'dropdown', - options: [ - { label: 'Operation 1', id: 'action1' }, - { label: 'Operation 2', id: 'action2' }, - ], - value: () => 'action1', - }, - // Credential field - { - id: 'credential', - title: '{Service} Account', - type: 'oauth-input', - serviceId: '{service}', - requiredScopes: getScopesForService('{service}'), - required: true, - }, - // Conditional fields per operation - // ... - ], - - tools: { - access: ['{service}_action1', '{service}_action2'], - config: { - tool: (params) => `{service}_${params.operation}`, - }, - }, - - outputs: { /* ... */ }, -} -``` - -### Key SubBlock Patterns - -**Condition-based visibility:** -```typescript -{ - id: 'resourceId', - title: 'Resource ID', - type: 'short-input', - condition: { field: 'operation', value: ['read', 'update', 'delete'] }, - required: { field: 'operation', value: ['read', 'update', 'delete'] }, -} -``` - -**DependsOn for cascading selectors:** -```typescript -{ - id: 'project', - type: 'project-selector', - selectorKey: '{service}.projects', - dependsOn: ['credential'], -}, -{ - id: 'issue', - type: 'file-selector', - selectorKey: '{service}.issues', - dependsOn: ['credential', 'project'], -} -``` - -Every remote `selectorKey` must use the unified server selector path. Apply the `add-selector` skill: -add browser-safe metadata to `apps/sim/lib/selectors/manifest.ts`, reuse or extract a server-only -provider listing primitive, and add a credential- and destination-bound server attachment. Do not -add code under `hooks/selectors/providers`, a provider-specific query key, browser token acquisition, -or a selector-only API route. The shared context builder sends only active `dependsOn` values and -preserves exact `{{KEY}}` environment references for server-side resolution. - -**Basic/Advanced mode for dual UX:** -```typescript -// Basic: Visual selector -{ - id: 'channelSelector', - type: 'channel-selector', - mode: 'basic', - canonicalParamId: 'channel', - dependsOn: ['credential'], -}, -// Advanced: Manual input -{ - id: 'channelId', - type: 'short-input', - mode: 'advanced', - canonicalParamId: 'channel', -} -``` - -Note neither subblock `id` is `channel` — the canonical id is a third name that both members map -onto, and it is the only one that survives serialization. - -**Critical Canonical Param Rules:** -- `canonicalParamId` must NOT match any subblock's `id` in the block -- `canonicalParamId` must be unique **block-wide**, not per operation. `buildCanonicalIndex` keys - groups by `canonicalParamId` across all subblocks and a group holds exactly one `basicId`, so two - operations that each need their own pair must use two different canonical ids -- Only use `canonicalParamId` to link basic/advanced alternatives for the same logical parameter. - A pair carries ONE concept — for files that means upload (basic) + file reference (advanced), as - in Gmail attachments (`blocks/blocks/gmail.ts`). Never overload the advanced side with alternate - identifiers like a URL or a provider asset ID; give those their own subblocks, mark all the - mutually exclusive sources `required: false`, and enforce "exactly one" at execution -- `mode` only controls UI visibility, NOT serialization. Without `canonicalParamId`, both basic and advanced field values would be sent -- Every subblock `id` must be unique within the block. Duplicate IDs cause conflicts even with different conditions -- **Required consistency:** If one subblock in a canonical group has `required: true`, ALL subblocks in that group must have `required: true` (prevents bypassing validation by switching modes) -- **Inputs section:** Must list canonical param IDs (e.g., `fileId`), NOT raw subblock IDs (e.g., `fileSelector`, `manualFileId`) -- **Params function:** Must use canonical param IDs, NOT raw subblock IDs (raw IDs are deleted after canonical transformation) - -### BlockMeta (Required) - -Export a `{Service}BlockMeta` in the same file as the block — **minimum 7 templates**. See `.agents/skills/add-block/SKILL.md` → "BlockMeta (Required)" for valid `modules` and `category` values and the full pattern. - -```typescript -export const {Service}BlockMeta = { - tags: ['tag1', 'tag2'], - templates: [ - { - icon: {Service}Icon, - title: '{Service} ', - prompt: 'Build a workflow that...', // concrete trigger → transformation → output - modules: ['agent', 'workflows'], - category: 'operations', - tags: ['automation'], - alsoIntegrations: ['slack'], // when the prompt references another service - }, - // ... at least 6 more - ], -} as const satisfies BlockMeta -``` +Follow `.agents/skills/add-block/SKILL.md` for the block structure, subBlock types, +`condition`/`dependsOn`/`required`/`mode` syntax, outputs, `canvasPresentation` sentences, and the +`{Service}BlockMeta` export (minimum 7 templates, plus `url` and `skills`). Every block declares +`canvasPresentation`; `bun run apps/sim/scripts/check-canvas-sentences.ts --block={service}` must +pass (CI runs `check:canvas-sentences --require-coverage`). + +Two rules that are easy to get wrong when copying from existing blocks: + +- Every remote `selectorKey` must use the unified server selector path. Apply the `add-selector` skill: + add browser-safe metadata to `apps/sim/lib/selectors/manifest.ts`, reuse or extract a server-only + provider listing primitive, and add a credential- and destination-bound server attachment. Do not + add code under `hooks/selectors/providers`, a provider-specific query key, browser token acquisition, + or a selector-only API route. The shared context builder sends only active `dependsOn` values and + preserves exact `{{KEY}}` environment references for server-side resolution. +- A `canonicalParamId` is a third name that neither member of a basic/advanced pair uses as its `id` + (e.g. `channelSelector` + `channelId` → `canonicalParamId: 'channel'`). It is the only key that + survives serialization, so `inputs` and `tools.config.params` reference the canonical id, never the + subblock ids. It is unique block-wide, and every member of a group shares the same `required` value. ## Step 4: Add Icon @@ -370,14 +158,7 @@ export function {Service}Icon(props: SVGProps) { ``` ### Getting Icons -**Do NOT search for icons yourself.** At the end of implementation, ask the user to provide the SVG: - -``` -I've completed the integration. Before I can add the icon, please provide the SVG for {Service}. -You can usually find this in the service's brand/press kit page, or copy it from their website. - -Paste the SVG code here and I'll convert it to a React component. -``` +**Do not search for icons yourself.** At the end of implementation, ask the user to paste the service's SVG (usually on its brand/press kit page). Once the user provides the SVG: 1. Extract the SVG paths/content @@ -411,69 +192,10 @@ in both light and dark mode. ## Step 5: Create Triggers (Optional) -If the service supports webhooks, create triggers using the generic `buildTriggerSubBlocks` helper. - -### Directory Structure -``` -apps/sim/triggers/{service}/ -├── index.ts # Barrel exports -├── utils.ts # Trigger options, setup instructions, extra fields -├── {event_a}.ts # Primary trigger (includes dropdown) -├── {event_b}.ts # Secondary triggers (no dropdown) -└── webhook.ts # Generic webhook (optional) -``` - -### Key Pattern - -```typescript -import { buildTriggerSubBlocks } from '@/triggers' -import { {service}TriggerOptions, {service}SetupInstructions, build{Service}ExtraFields } from './utils' - -// Primary trigger - includeDropdown: true -export const {service}EventATrigger: TriggerConfig = { - id: '{service}_event_a', - subBlocks: buildTriggerSubBlocks({ - triggerId: '{service}_event_a', - triggerOptions: {service}TriggerOptions, - includeDropdown: true, // Only for primary trigger! - setupInstructions: {service}SetupInstructions('Event A'), - extraFields: build{Service}ExtraFields('{service}_event_a'), - }), - // ... -} - -// Secondary triggers - no dropdown -export const {service}EventBTrigger: TriggerConfig = { - id: '{service}_event_b', - subBlocks: buildTriggerSubBlocks({ - triggerId: '{service}_event_b', - triggerOptions: {service}TriggerOptions, - // No includeDropdown! - setupInstructions: {service}SetupInstructions('Event B'), - extraFields: build{Service}ExtraFields('{service}_event_b'), - }), - // ... -} -``` - -### Connect to Block -```typescript -import { getTrigger } from '@/triggers' - -export const {Service}Block: BlockConfig = { - triggers: { - enabled: true, - available: ['{service}_event_a', '{service}_event_b'], - }, - subBlocks: [ - // Tool fields... - ...getTrigger('{service}_event_a').subBlocks, - ...getTrigger('{service}_event_b').subBlocks, - ], -} -``` - -See `/add-trigger` skill for complete documentation. +If the service supports webhooks or needs polling, follow `.agents/skills/add-trigger/SKILL.md` +(directory layout, `buildTriggerSubBlocks`, provider handler, polling handler); then wire +`triggers.enabled` / `triggers.available` into the block and spread each trigger's +`getTrigger(id).subBlocks` after the tool subBlocks. ## Step 6: Register Everything @@ -623,8 +345,8 @@ If creating V2 versions (API-aligned outputs): - [ ] Created tool file for each operation - [ ] Chose exactly one boundary per tool: registered `InternalToolConfig.operation` or absolute external HTTP(S) `ToolConfig.request` -- [ ] No tool points to `/api/...`, constructs a URL back to Sim, declares `request.internal` or the - retired `directExecution` property, or has an HTTP fallback for an in-process operation +- [ ] No tool points to `/api/...`, constructs a URL back to Sim, declares `request.internal` or a + `directExecution` property (fails `bun run check:tool-request-boundary`), or has an HTTP fallback for an in-process operation - [ ] All params have correct visibility - [ ] All nullable fields use `?? null` - [ ] All optional outputs have `optional: true` @@ -658,6 +380,8 @@ If creating V2 versions (API-aligned outputs): - [ ] If triggers: set `triggers.enabled` and `triggers.available` - [ ] If triggers: spread trigger subBlocks with `getTrigger()` - [ ] Exported `{Service}BlockMeta` with at least 7 templates +- [ ] `canvasPresentation.sentences` covers every operation; `bun run apps/sim/scripts/check-canvas-sentences.ts --block={service}` passes +- [ ] `{Service}BlockMeta` also sets `url` (verified external homepage) and `skills` (grounded in `tools.access`, sourced from real use cases) — see add-block → BlockMeta ### OAuth Scopes (if OAuth service) - [ ] Defined scopes in `lib/oauth/oauth.ts` under `OAUTH_PROVIDERS` @@ -707,52 +431,13 @@ If creating V2 versions (API-aligned outputs): - [ ] If any response schema remained unknown, explicitly told the user instead of guessing - [ ] `{Service}BlockMeta` exported with at least 7 templates, each having `icon`, `title`, `prompt`, `modules`, `category`, and `tags` -## Example Command - -When the user asks to add an integration: - -``` -User: Add a Stripe integration - -You: I'll add the Stripe integration. Let me: - -1. First, research the Stripe API using Context7 -2. Create the tools for key operations (payments, subscriptions, etc.) -3. Create the block with operation dropdown -4. Register everything -5. Generate docs -6. Ask you for the Stripe icon SVG - -[Proceed with implementation...] - -[After completing steps 1-5...] - -I've completed the Stripe integration. Before I can add the icon, please provide the SVG for Stripe. -You can usually find this in the service's brand/press kit page, or copy it from their website. - -Paste the SVG code here and I'll convert it to a React component. -``` - ## File Handling When your integration handles file uploads or downloads, follow these patterns to work with `UserFile` objects consistently. ### What is a UserFile? -A `UserFile` is the standard file representation in Sim: - -```typescript -interface UserFile { - id: string // Unique identifier - name: string // Original filename - url: string // Presigned URL for download - size: number // File size in bytes - type: string // MIME type (e.g., 'application/pdf') - base64?: string // Optional base64 content (if small file) - key?: string // Internal storage key - context?: object // Storage context metadata -} -``` +`UserFile` (`apps/sim/executor/types.ts`) is the standard file representation in Sim — id, name, an access `url` (not guaranteed presigned — `remoteUrl` is the short-lived signed one, set only for providers that fetch by URL), size, MIME `type`, storage `key`, and optional inline `base64` / provider file handles. Read file bytes through the documented upload helpers, never by fetching `url` directly. Read the interface rather than relying on a copy here. ### File Input Pattern (Uploads) @@ -819,13 +504,11 @@ export const {service}UploadTool: InternalToolConfig = { // ... params: { file: { type: 'file', required: false, visibility: 'user-or-llm' }, - fileContent: { type: 'string', required: false, visibility: 'hidden' }, // Legacy }, operation: { input: (params) => ({ accessToken: params.accessToken, file: params.file, - fileContent: params.fileContent, }), }, } @@ -939,17 +622,7 @@ requiredScopes: getScopesForService('{service}'), ### Common Gotchas 1. **OAuth serviceId must match** - The `serviceId` in oauth-input must match the OAuth provider configuration -2. **All tool IDs MUST be snake_case** - `stripe_create_payment`, not `stripeCreatePayment`. This applies to tool `id` fields, registry keys, `tools.access` arrays, and `tools.config.tool` return values -3. **Block type is snake_case** - `type: 'stripe'`, not `type: 'Stripe'` -4. **Alphabetical ordering** - Keep imports and registry entries alphabetically sorted -5. **Required can be conditional** - Use `required: { field: 'op', value: 'create' }` instead of always true -6. **DependsOn clears options** - When an active dependency changes, the shared selector facade +2. **DependsOn clears options** - When an active dependency changes, the shared selector facade refetches with an opaque query revision; dependency values and references never enter query keys -7. **Never pass Buffer directly to fetch** - Convert to `new Uint8Array(buffer)` for TypeScript compatibility -8. **Always handle legacy file params** - Keep hidden `fileContent` params for backwards compatibility -9. **Optional fields use advanced mode** - Set `mode: 'advanced'` on rarely-used optional fields -10. **Complex inputs need wandConfig** - Timestamps, JSON arrays, and other hard-to-type values should have `wandConfig` enabled -11. **Never hardcode scopes** - Use `getScopesForService()` in blocks and `getCanonicalScopesForProvider()` in auth.ts -12. **Always add scope descriptions** - New scopes must have entries in `SCOPE_DESCRIPTIONS` within `lib/oauth/utils.ts` -13. **OAuth service IDs need deployment capabilities** - Every visible OAuth integration must resolve through `OAUTH_CLIENT_CAPABILITIES`; shared Google/Microsoft aliases map to their provider capability -14. **Keep runtime and presentation separate** - Runtime OAuth fields live in `packages/deployment-config/src/env-capabilities.ts`; CLI input modes live in the exhaustively checked `packages/sim-setup/src/capability-config.ts` mapping +3. **Never pass Buffer directly to fetch** - Convert to `new Uint8Array(buffer)` for TypeScript compatibility +4. **Legacy `fileContent` params** - Only an existing tool that already accepted base64 `fileContent` keeps that hidden param; new tools take `file` only diff --git a/.agents/skills/add-model/SKILL.md b/.agents/skills/add-model/SKILL.md index d418773ff2e..c7630651b10 100644 --- a/.agents/skills/add-model/SKILL.md +++ b/.agents/skills/add-model/SKILL.md @@ -31,7 +31,7 @@ In priority order — fetch all that exist for the provider: | Provider | Models index | Pricing | Reasoning/parameter caveats | |---|---|---|---| | OpenAI | platform.openai.com/docs/models | openai.com/api/pricing | platform.openai.com/docs/guides/reasoning | -| Anthropic | docs.anthropic.com/en/docs/about-claude/models | anthropic.com/pricing | docs.anthropic.com/en/docs/build-with-claude/extended-thinking | +| Anthropic | platform.claude.com/docs/en/about-claude/models/overview | claude.com/pricing (API section) | platform.claude.com/docs/en/build-with-claude/extended-thinking | | Google (Gemini) | ai.google.dev/gemini-api/docs/models | ai.google.dev/pricing | ai.google.dev/gemini-api/docs/thinking | | xAI | docs.x.ai/developers/models | docs.x.ai/developers/models (per-model detail page) | docs.x.ai/developers/model-capabilities/text/reasoning | | Mistral | docs.mistral.ai/getting-started/models/models_overview | mistral.ai/pricing | n/a | @@ -49,13 +49,13 @@ Use a precise WebFetch prompt: *"Extract for {model_id}: exact model id string, |---|---|---| | `temperature` | All providers (passed through if set) | Safe but inert on always-reasoning models that reject it | | `toolUsageControl` | All providers (provider-level, not per-model) | n/a — set on `ProviderDefinition`, not models | -| `reasoningEffort` | `openai/core.ts`, `azure-openai`, `anthropic/core.ts` (mapped to thinking), `gemini/core.ts` | **Dead on xai, deepseek, mistral, groq, cerebras, openrouter, fireworks, bedrock, vertex** unless their core consumes it — re-grep before assuming | +| `reasoningEffort` | `openai/core.ts`, `azure-openai`, `xai`, `deepseek`, `groq`, `zai`, `meta`, `litellm` (each `index.ts`) | Not read by anthropic/gemini (they use `thinking`) or by mistral, cerebras, openrouter, fireworks, vertex — re-grep before assuming | | `verbosity` | `openai/core.ts`, `azure-openai/index.ts` only | Dead elsewhere | -| `thinking` | `anthropic/core.ts`, `gemini/core.ts` | Dead elsewhere | +| `thinking` | `anthropic/core.ts`, `gemini/core.ts`; `deepseek`, `groq`, `zai`, `kimi` (each `index.ts`) read the resolved `thinkingLevel` | Dead elsewhere | | `thinking.streamed` | Docs generator + `getThinkingStreamVisibility` (`models.ts`); `anthropic/core.ts` uses `'summary'` to request `display: 'summarized'` on agent-events runs | **Mandatory on Anthropic-family thinking models** (`agent-stream-docs:check` fails without it); other families fall back to provider defaults | -| `nativeStructuredOutputs` | `anthropic/core.ts`, `fireworks/index.ts`, `openrouter/index.ts` | Dead on openai, xai, google, vertex, bedrock, azure-openai, deepseek, mistral, groq, cerebras | +| `nativeStructuredOutputs` | `anthropic/core.ts`, `bedrock/index.ts` (via `models.ts` `supportsNativeStructuredOutputs`, which reads the flag) | Dead elsewhere — fireworks/baseten/together/openrouter call their own provider-level `supportsNativeStructuredOutputs` that ignores the model flag (always on, always off, or OpenRouter API metadata) | | `maxOutputTokens` | Read by UI + executor for token estimation | Always meaningful — set if provider documents a cap | -| `computerUse` | `anthropic/core.ts` | Dead elsewhere | +| `computerUse` | `providers/utils.ts` (`getComputerUseModels` → `computerUseModels` routing) | Set only on actual computer-use SKUs | | `deepResearch` | UI flag for routing to deep-research SKUs | Set only on actual deep-research model IDs | | `memory: false` | Conversation persistence opt-out | Set only when model genuinely cannot maintain history (e.g., deep-research) | @@ -98,13 +98,15 @@ Model id MUST be prefixed: `azure/`, `azure-anthropic/`, `vertex/`, `bedrock/`, ### Insertion order -Within a family, newest first (matches existing convention: GPT-5.5 above GPT-5.4 above GPT-5.2). Across families, biggest/flagship at top of list. +Within a family, newest first (as the existing entries are ordered). Across families, biggest/flagship at top of list. ### `recommended` / `speedOptimized` - At most one or two `recommended: true` per provider — the current flagship(s). - If you're adding a new flagship, ask the user before removing `recommended` from the previous flagship. Never silently flip it. - `speedOptimized: true` only on the smallest/fastest tier (nano, flash-lite, haiku class). +- Use today's date for `pricing.updatedAt`; never copy a sibling's. +- `cachedInput` is an explicit documented number — never derived from `input` (ratios vary by provider). ## Step 4: Repo-side touchpoints beyond the entry @@ -112,21 +114,9 @@ Adding the `models.ts` entry is most of the job because nearly every consumer is ### Hosted = auto-billed, by provider -`getHostedModels()` in `apps/sim/providers/models.ts` returns **every** model under `openai`, `anthropic`, and `google`: +`getHostedModels()` in `apps/sim/providers/models.ts` returns the model IDs served with Sim's rotating hosted key and billed to the workspace via `shouldBillModelUsage()` (`providers/utils.ts`). It builds that list by expanding whole providers (`getProviderModels('openai')`, `'anthropic'`, `'google'`, and others) plus the static Fireworks catalog, so any model added under one of those providers is hosted automatically. Read the function before inserting — the provider set changes. Before you insert: -```ts -export function getHostedModels(): string[] { - return [ - ...getProviderModels('openai'), - ...getProviderModels('anthropic'), - ...getProviderModels('google'), - ] -} -``` - -So a model added to any of those three providers is **automatically served with Sim's rotating hosted key and billed** to the workspace via `shouldBillModelUsage()` (`providers/utils.ts`). Before you insert: - -- **If the model should be BYOK-only / never-billed**, do NOT drop it under `openai`/`anthropic`/`google` as-is — that silently enrolls it in hosted billing. Confirm hosting/billing intent with the user. (Precedent: Ollama Cloud is a deliberately separate `isReseller` provider specifically to stay BYOK-only/never-billed.) +- **If the model should be BYOK-only / never-billed**, do not add it under a provider that `getHostedModels()` expands — that silently enrolls it in hosted billing. After inserting, verify with `getHostedModels().includes('')` (a one-line `bun -e` or the assertion in `providers/utils.test.ts`). Confirm hosting/billing intent with the user. (Ollama Cloud is a deliberately separate `isReseller` provider specifically to stay BYOK-only/never-billed.) - **If the model should be hosted**, the deployment must actually have a key for it — the provider's `{PREFIX}_COUNT` / `{PREFIX}_1..N` env vars must be set, or hosted runs fail at execution time. - State the hosted/billing status explicitly in the verification report. @@ -145,7 +135,7 @@ If anything matches, run the affected provider tests and update assertions as ne ### New API behavior is NOT data-driven -The Consumption Matrix (Step 2) tells you which capability *flags* are honored by existing provider code. But if the new model needs **net-new** request handling that the provider doesn't implement yet — a new beta header (e.g. Anthropic's `anthropic-beta` structured-outputs header in `anthropic/index.ts`), a new thinking/reasoning encoding, a Responses-API quirk — you must edit `apps/sim/providers//core.ts` / `index.ts`. Setting a flag whose behavior isn't implemented is a silent no-op. When you do edit provider code, reuse the shared helpers rather than hand-rolling: streaming responses are assembled via `createStreamingExecution` (`@/providers/streaming-execution`) and tool schemas via `adaptOpenAIChatToolSchema` / `adaptAnthropicToolSchema` (`@/providers/tool-schema-adapter`). +The Consumption Matrix (Step 2) tells you which capability *flags* are honored by existing provider code. But if the new model needs **net-new** request handling that the provider doesn't implement yet — a new beta header, a new thinking/reasoning encoding, a Responses-API quirk — you must edit `apps/sim/providers//core.ts` / `index.ts`. Setting a flag whose behavior isn't implemented is a silent no-op. When you do edit provider code, reuse the shared helpers rather than hand-rolling: streaming responses are assembled via `createStreamingExecution` (`@/providers/streaming-execution`) and tool schemas via `adaptOpenAIChatToolSchema` / `adaptAnthropicToolSchema` (`@/providers/tool-schema-adapter`). ### Thinking/reasoning models: `streamed` visibility + generated docs @@ -168,7 +158,7 @@ bun run lint bun run agent-stream-docs:generate # only when the entry has thinking/reasoningEffort ``` -Lint must pass before reporting done. **If lint fails:** read the error, fix the syntax/typing issue in the entry you just wrote (do not delete the entry — it's the work product), re-run lint, and note the fix in a "Lint adjustments" line in the verification report. Never report done with lint failing. +Lint must pass before you report done — fix the entry you wrote, never delete it to make lint pass. ## Step 6: Verification report (mandatory format) @@ -187,7 +177,7 @@ End with this exact structure: | `capabilities.temperature` | `{ min: 0, max: 1 }` | matches sibling entries | — pattern-match only | | `capabilities.reasoningEffort` | NOT SET | provider docs say API rejects it for this model | ✓ correctly omitted | | `releaseDate` | 2026-04-30 | https://docs.x.ai/... announcement | ✓ verified | -| hosted/billing | BYOK-only (xai not in `getHostedModels`) | `providers/models.ts` | — confirmed intent | +| hosted/billing | hosted (`getHostedModels().includes(id)`) or BYOK-only | `providers/models.ts` | — confirmed intent | **Disagreements** - _none_ OR _OpenRouter says X, provider docs say Y — used Y per provider rule_ @@ -206,17 +196,3 @@ Omitting a field is **not the same as verifying it**. Any field you cannot confi - Context window missing → do NOT guess. Ask the user; mark ❓ UNVERIFIED. - Release date missing → omit the field; mark ❓ UNVERIFIED in the report. - Capability uncertain → omit the flag (safer than setting a dead/wrong one); mark ❓ UNVERIFIED so the user knows you didn't confirm it either way. - -## Anti-patterns this skill exists to prevent - -- ❌ Trusting a marketing email (xAI's grok-4.3 email claimed "3 reasoning efforts" but the API rejects `reasoning_effort` — verified by official docs only) -- ❌ Setting `nativeStructuredOutputs: true` on xai/openai/google (dead — only anthropic/fireworks/openrouter consume it) -- ❌ Setting `thinking` on non-Anthropic/non-Gemini providers -- ❌ Adding an Anthropic-family thinking model without `thinking.streamed` (CI `agent-stream-docs:check` fails), or skipping `bun run agent-stream-docs:generate` after adding any thinking/reasoning model -- ❌ Setting `verbosity` on anything other than OpenAI gpt-5.x -- ❌ Copying `pricing.updatedAt` from a sibling instead of using today's date -- ❌ Inventing a `cachedInput` price by dividing input by 4 (varies by provider — find an explicit number) -- ❌ Stamping `recommended: true` on the new model without removing it from the previous flagship -- ❌ Adding a BYOK-only model under `openai`/`anthropic`/`google` (silently enrolls it in hosted billing via `getHostedModels()`) -- ❌ Reporting "done" after only `bun run lint` when you touched a hosted (openai/anthropic/google) or flagship model with assertions in `providers/utils.test.ts` -- ❌ Reporting "done" with any UNVERIFIED row in the table diff --git a/.agents/skills/add-permission-group-item/SKILL.md b/.agents/skills/add-permission-group-item/SKILL.md index c218ddf7a38..e413adcd70f 100644 --- a/.agents/skills/add-permission-group-item/SKILL.md +++ b/.agents/skills/add-permission-group-item/SKILL.md @@ -109,11 +109,11 @@ Capability ids are **domain-shaped** (`tables.create`); config keys are **surfac Use `'PERMISSION_GROUP_CAPABILITY_BLOCKED'` for `detailCode`. Four rules carry a more specific one — `deploy.chat.auth_mode` (`CHAT_AUTH_MODE_NOT_PERMITTED`), `file_share.publish` / `file_share.auth_mode` (`PUBLIC_SHARING_NOT_ALLOWED`), `personal_api_key.use` (`PERSONAL_API_KEYS_DISABLED`) — which is why a call site reads the code off the rule and never spells one out. The set in `lib/core/application/forbidden.ts` is closed **over remedies, not causes** — a new code is warranted only when the remedy differs from "ask an organization admin", and requires an entry in `FORBIDDEN_DETAIL_CODE_DESCRIPTIONS` (a compile-time gate) plus a new value in the generated OpenAPI 403 description. -A **parameterized** rule is the same shape with `kind: 'parameterized'` and a `deniedBy` taking the request value second — `'knowledge.connectors'` is `(config, connectorType) => allowlistDenies(config.allowedKnowledgeConnectors, connectorType)`. It **cannot be declared on an operation**: the funnel decides from principal, workspace and operation, never request input, and widening it would touch all ~315 operations for the sake of two keys. `defineWorkspaceOperation` throws at definition time (`Operation declares parameterized capability ; assert it from the use case instead`) rather than letting the operation read as gated while the gate never fires. +A **parameterized** rule is the same shape with `kind: 'parameterized'` and a `deniedBy` taking the request value second — `'knowledge.connectors'` is `(config, connectorType) => allowlistDenies(config.allowedKnowledgeConnectors, connectorType)`. It **cannot be declared on an operation**: the funnel decides from principal, workspace and operation, never request input, and widening it would touch every one of the hundreds of operations for the sake of two keys. `defineWorkspaceOperation` throws at definition time (`Operation declares parameterized capability ; assert it from the use case instead`) rather than letting the operation read as gated while the gate never fires. ## Step 4: Declare it on the operations it governs, or assert it at the call site -`capability` is **required on the `ApplicationOperation` base type** (`lib/core/application/operation.ts:31`), typed `StaticPermissionGroupCapability | 'none'` — required there, not only on `defineWorkspaceOperation`, so a bare object literal minted by a domain factory does not compile without it (five OAuth-connection operations once shipped capability-less that way) — *and* guarded at definition time (`Operation declares no capability; name one, or 'none' with a reason`). The guard is not redundant: **`apps/sim/tsconfig.json` excludes `*.test.ts` / `*.test.tsx` from type-checking** and the enforcement audit walks past test files, so a fixture is the one construction site no static check reads. An absent capability does not deny — it throws `Cannot read properties of undefined` inside `capabilityDeniedBy`, and **only for a caller whose organization actually has a permission group**. It passes CI and every personal workspace, then fails in the tenants that bought the feature. +`capability` is **required on the `ApplicationOperation` base type** (the `capability` field in `lib/core/application/operation.ts`), typed `StaticPermissionGroupCapability | 'none'` — required there, not only on `defineWorkspaceOperation`, so a bare object literal minted by a domain factory does not compile without it — *and* guarded at definition time (`Operation declares no capability; name one, or 'none' with a reason`). The guard is not redundant: **`apps/sim/tsconfig.json` excludes `*.test.ts` / `*.test.tsx` from type-checking** and the enforcement audit walks past test files, so a fixture is the one construction site no static check reads. An absent capability does not deny — it throws `Cannot read properties of undefined` inside `capabilityDeniedBy`, and **only for a caller whose organization actually has a permission group**. It passes CI and every personal workspace, then fails in the tenants that bought the feature. **Static, and the operation is the whole decision** — set `capability` and write no gate code: @@ -127,7 +127,7 @@ export const shareWidget = defineWorkspaceOperation({ }) ``` -**The factory trap.** An operation minted by a factory that does not call `defineWorkspaceOperation` — a hand-frozen object — bypasses the required type *and*, once bypassed, the audit; twenty-one operations across six domains were invisible that way, and the file still printed a tick because some other operation in it was counted. The audit now matches the whole `defineOperation` family, resolves a same-file `function` factory (capability fixed in the body or taken as a positional second argument — `lib/table/application/operations.ts` shows both, with **no default** on the positional form so nothing inherits `tables.use` unreviewed), and cross-checks the members of every exported `*Operations` registry against what it parsed. Keep new operations inside an exported `*Operations` registry, mint them through a `define*Operation` builder taking an object literal with a string `id`, and use a `function` factory rather than an arrow const. +**The factory trap.** An operation minted by a factory that does not call `defineWorkspaceOperation` — a hand-frozen object — bypasses the required type *and*, once bypassed, the audit. The audit therefore matches the whole `defineOperation` family, resolves a same-file `function` factory (capability fixed in the body or taken as a positional second argument — `lib/table/application/operations.ts` shows both, with **no default** on the positional form so nothing inherits `tables.use` unreviewed), and cross-checks the members of every exported `*Operations` registry against what it parsed, so a registry member it read no operation from is a finding rather than a tick. Keep new operations inside an exported `*Operations` registry, mint them through a `define*Operation` builder taking an object literal with a string `id`, and use a `function` factory rather than an arrow const. **Static, but no operation to hang it on** — a raw route or an organization-level action. @@ -179,7 +179,7 @@ Always route through `CAPABILITY_RULES` and raise with `refuseCapability` — a When the subject is **persisted and read back later** — the table dispatch pipeline stamps it on `table_run_dispatches` / `table_row_executions` so auto-fired cells run under the person the write was gated for — declare it `capabilityGovernedUserId: string | null`, required with an explicit `null` and never optional. An optional field with a fallback is how every producer that had not been taught the distinction silently inherited `triggeredByUserId`, an *attribution* naming the billed account; making omission a compile error is the whole enforcement. A persisted subject also has a lifecycle: `lib/users/account-deletion.ts` cancels the dispatches stamped with a deleted user. - **`/api/v1`** authorizes in `app/api/v1/middleware.ts`. Every route threads a `V1RouteCapability` (`StaticPermissionGroupCapability | 'none'`, required and spelled out) whose value must match what its v2 or internal counterpart declares — v1 gets no mapping of its own. `check-capability-subject.ts` audits v1's subjects only, because the bug has shipped and been fixed twice there. -- **Raw internal table routes** (`/api/table/**`) share one gate in `checkAccess` (`app/api/table/utils.ts`), whose signature takes a `TableAccessPrincipal` union — `{ kind: 'user'; userId }` or `{ kind: 'workspace_api_key'; keyCreatorUserId }` — so a bare id no longer type-checks and only the kind that says so skips the gate. `tableAccessPrincipal(rateLimit)` builds it for v1. +- **Raw internal table routes** (`/api/table/**`) share one gate in `checkAccess` (`app/api/table/utils.ts`), whose signature takes a `TableAccessPrincipal` union — `{ kind: 'user'; userId }` or `{ kind: 'workspace_api_key'; keyCreatorUserId }` — so a bare id does not type-check and only the kind that says so skips the gate. `tableAccessPrincipal(rateLimit)` builds it for v1. - **The route-wrapper graph.** `withRouteHandler` imports `request-scope.server.ts` and nothing heavier. Import a resolver at the *call site*, never from the wrapper or `lib/core/application` — see Step 6. ## Step 5: Add it to the golden corpus @@ -215,12 +215,12 @@ cd apps/sim && bunx vitest run lib/permission-groups Also `bun run check:api-validation` if you touched a contract or the group routes. `bun run check:audits` runs all of these; it derives its list from the `check:*` scripts in `package.json`, so a new audit is opted *out* deliberately rather than opted in. -Read the success lines, not the exit codes — the counts should have grown by your operation and capability: +Read the success lines, not the exit codes — compare the counts against the previous run and check they grew by exactly what you added: an operation-declared capability adds one operation and one capability; a raw-route or parameterized capability adds one capability and no operation; an executor-gated or UI-only item adds neither: ``` -✓ permission-group enforcement: 322 operations declare a capability, 35 capabilities all enforced -✅ Application graph clean: 5 roots reach none of 11 forbidden module trees -check:capability-subject — 32 v1 files, 5 capability subjects resolved through capabilityGovernedUserId. +✓ permission-group enforcement: operations declare a capability, capabilities all enforced +✅ Application graph clean: roots reach none of forbidden module trees +check:capability-subject — v1 files, capability subjects resolved through capabilityGovernedUserId. ``` The enforcement audit is all-or-nothing — one success line or findings, no migration mode that exits 0 with work outstanding. Because it reads source text it also refuses success when its own parsers come up empty or disagree with each other; if a self-check fires, teach the parsers the new form rather than working around it. diff --git a/.agents/skills/add-tools/SKILL.md b/.agents/skills/add-tools/SKILL.md index 985073b2b5d..e0f68e70c34 100644 --- a/.agents/skills/add-tools/SKILL.md +++ b/.agents/skills/add-tools/SKILL.md @@ -54,7 +54,7 @@ Every tool must use exactly one of these configurations: HTTP(S) provider endpoint. Never set a tool URL to `/api/...`, construct an absolute URL back to Sim, declare -`request.internal`, add the retired `directExecution` property, import a route module, or create an API route merely to normalize files, +`request.internal`, add a `directExecution` property (it fails `bun run check:tool-request-boundary`), import a route module, or create an API route merely to normalize files, authorize access, or reuse server code. A real browser/API route may remain as a thin adapter, but the route and the tool must call the same operation directly. A true cross-process/capability boundary uses an explicit server client and is not disguised as a tool self-hop. @@ -93,7 +93,7 @@ export const {serviceName}{Action}Tool: ToolConfig< }, params: { - // Hidden params (system-injected, only use hidden for oauth accessToken) + // Hidden params (system-injected, e.g. the OAuth accessToken) accessToken: { type: 'string', required: true, @@ -201,23 +201,66 @@ fallback, or caller-controlled `_context` authority. ## Resolved Secrets and Provenance Boundaries -- Leave ordinary external API inputs and third-party results unchanged. Add provenance handling only - when an exact field is proven to cross a Sim model, durable-storage, or internal-execution boundary. -- Project AI-consumed text/structured fields with the smallest exact model-input selector: - `request.modelInput` for an external request or `operation.modelInput` for an in-process operation. -- Treat URLs, domains, resource IDs, and control fields as ordinary request values unless the exact - field is proven model-visible. For serialized external model content, project the serialized - top-level param through `request.modelInput` before the existing formatter parses it; do not add a - separate hard-rejection mechanism. -- For in-process operations, use `operation.modelInput` for actual inline/raw model bytes or - `operation.secretProvenance` for durable writes and execution handoffs. Do not treat a storage key, - path, signed URL, or remote URL as provenance for fetched bytes; authorize tracked stored bytes at - the owning model-egress boundary. Validate the exact selection and trusted scope, then import or - propagate provenance at the receiving operation boundary. -- Never substitute secret plaintext into source, serialize plaintext provenance, hand-roll private - headers, or blanket-sanitize tool results. -- Add focused tests for named projection, identical unproven public text, malformed/incomplete - metadata, metadata stripping, scope isolation, and legacy compatibility where applicable. +Classify every request field before implementing the tool. + +This is opt-in, not a blanket integration migration. Add a model-input declaration only when the +service's official documentation or an unambiguous local execution path proves that the exact +field is consumed by an AI model. If that cannot be established, preserve existing tool behavior +and leave the field unannotated. + +- **Ordinary provider/API input:** leave it unchanged. Explicit `{{...}}` references resolve and are + sent with their normal request semantics. A URL, domain, resource ID, control field, or opaque + payload is not model-visible merely because the provider is AI-backed or may process the + referenced resource later. +- **Text or structured content consumed by an AI model:** declare `request.modelInput` for an + external provider request or `operation.modelInput` for an in-process operation, with + `mode: 'project'` and select only the exact model-visible fields. The shared executor replaces + activated Sim secrets with canonical `{{NAME}}` labels before request formatting. For nested or + JSON-string fields, use a small shared selector plus `applyProjected`; verify that selecting the + rebuilt params reproduces the projected selection. +- **Serialized model content sent directly to an external provider:** include the serialized + top-level param in `request.modelInput`. Project the private copy before the existing request + formatter parses it; keep formatter behavior deterministic when a whole-value placeholder is not + valid in the serialized grammar. Do not introduce a second hard-rejection path. +- **Opaque model input owned by an in-process operation** such as inline audio, image, video, or + document bytes: add `privateInputPaths` to the `mode: 'project'` operation model-input + declaration, or use `mode: 'private-provenance'` with `inputPaths` when there is no textual + projection (see the `modelInput` union in `apps/sim/tools/types.ts`). Do not select storage keys, + paths, signed URLs, or ordinary remote URLs as byte provenance; the owning operation must + authorize stored bytes independently at model egress. The operation must call + `validateOpaqueModelInputProvenance` before downloading or sending content to the model and must + apply the workspace-file provenance guard before reading a persisted workspace file. +- **Sim-owned durable storage or internal execution handoff** that can later enter a workflow/model + (table cells, Agent memory, knowledge documents/chunks, workspace-file contents, or child-workflow + input): transport encrypted field-scoped provenance with `operation.secretProvenance`. The + operation validates the exact selection and trusted scope, then persists, imports, or propagates + it at the owning boundary. Preserve shared legacy behavior for rows/files whose provenance marker + is `NULL`; never invent a tool-local migration rule. + +Hard rules: + +- Never substitute secret plaintext into source or serialize plaintext provenance. +- Never hand-roll private provenance headers/envelopes; the shared `executeTool` boundary owns + transport and strips private metadata from functional results. +- Never attach private provenance to an external URL. Project proven + model-visible external fields with `request.modelInput`; otherwise preserve ordinary request + semantics. Use a registered in-process operation when encrypted provenance must cross the + boundary. +- Never sanitize arbitrary third-party tool results. Projection applies only to secrets activated + by Sim's resolved-secret provenance for that execution/tool call. +- Do not add provenance merely because a value is persisted, returned by a tool, or appears in a + filename. Require a concrete Sim `{{...}}` resolution path and a later model/log boundary. If an + unsupported field can resolve a secret but does not justify durable tracking (for example a + `file_write` path), reject it at that exact ingress. +- At diagnostic boundaries, project only values carrying execution-scoped provenance. Ordinary + provider responses, filenames, URLs, and errors remain unchanged when Sim did not resolve a + secret into them. + +Add focused tests covering named projection, ordinary identical text without provenance, nested and +serialized shape handling, unchanged ordinary external inputs, malformed/incomplete private metadata +failing closed, headerless legacy requests, and absence of private metadata in the public tool result. +For durable sinks, also cover legacy `NULL` markers, exact-empty new writes, tracked secret writes, +stale/missing sidecars, and scope isolation. ## Critical Rules for Outputs @@ -275,9 +318,7 @@ items: { }, ``` -Only use bare `type: 'json'` without `properties` when the shape is truly dynamic or unknown. - -If the response shape is unknown because the docs do not provide it, you MUST tell the user and stop. Unknown is not the same as dynamic. Never guess outputs. +Only use bare `type: 'json'` without `properties` when the shape is truly dynamic. Unknown is not the same as dynamic — see the Hard Rule above. ## Critical Rules for transformResponse @@ -513,10 +554,6 @@ If creating V2 tools (API-aligned outputs), use `_v2` suffix: - Version: `'2.0.0'` - Outputs: Flat, API-aligned (no content/metadata wrapper) -## Naming Convention - -All tool IDs MUST use `snake_case`: `{service}_{action}` (e.g., `x_create_tweet`, `slack_send_message`). Never use camelCase or PascalCase for tool IDs. - ## Checklist Before Finishing - [ ] All tool IDs use snake_case @@ -544,9 +581,9 @@ All tool IDs MUST use `snake_case`: `{service}_{action}` (e.g., `x_create_tweet` ## Final Validation (Required) -After creating all tools, you MUST validate every tool before finishing: +Before finishing, validate each tool file against the API docs: -1. **Read every tool file** you created — do not skip any +1. **Re-read each tool file** you created 2. **Cross-reference with the API docs** to verify: - All required params are marked `required: true` - All optional params are marked `required: false` diff --git a/.agents/skills/add-trigger/SKILL.md b/.agents/skills/add-trigger/SKILL.md index c648b9d0d61..2e784a7f318 100644 --- a/.agents/skills/add-trigger/SKILL.md +++ b/.agents/skills/add-trigger/SKILL.md @@ -218,7 +218,7 @@ If none apply, you don't need a handler. The default handler provides bearer tok ```typescript import crypto from 'crypto' import { createLogger } from '@sim/logger' -import { safeCompare } from '@/lib/core/security/encryption' +import { safeCompare } from '@sim/security/compare' import type { EventMatchContext, FormatInputContext, FormatInputResult, WebhookProviderHandler } from '@/lib/webhooks/providers/types' import { createHmacVerifier } from '@/lib/webhooks/providers/utils' @@ -252,7 +252,7 @@ export const {service}Handler: WebhookProviderHandler = { return { input: { eventType: b.type, - resourceId: (b.data as Record)?.id || '', + resourceId: (b.data as Record)?.id ?? null, resource: b.data, }, } @@ -460,11 +460,17 @@ Add to `helm/sim/values.yaml` under the existing polling cron jobs: ```yaml {service}WebhookPoll: + enabled: true + name: {service}-webhook-poll schedule: "*/1 * * * *" + path: "/api/webhooks/poll/{service}" concurrencyPolicy: Forbid - url: "http://sim:3000/api/webhooks/poll/{service}" + successfulJobsHistoryLimit: 3 + failedJobsHistoryLimit: 1 ``` +Mirror the existing `rssWebhookPoll` entry. + ### Reference Implementations - Simple: `apps/sim/lib/webhooks/polling/rss.ts` + `apps/sim/triggers/rss/poller.ts` @@ -490,8 +496,10 @@ through `selectors.execute`; never add a client provider module or selector-only `canonicalParamId: 'oauthCredential'` on the credential sub-block is the line people forget. The shared context builder uses trigger mode and projects only active `dependsOn` values under their canonical ids. Exact `{{KEY}}` environment references remain unresolved until the authorized server -executor. A credential field is also recognized by its `oauth-input` type as a compatibility -fallback. +executor. The builder does not infer a credential from `type: 'oauth-input'`; only the legacy ids +`credential` / `botCredential` / `customBotCredential` / `manualBotCredential` are aliased. Give the +field `canonicalParamId: 'oauthCredential'`, or declare a manifest `sourceFields` alias when a +legacy source id must be retained. **`options` — everything else.** A static array, or a pure function of the block's own values for a list that narrows to a sibling's selection. No I/O. @@ -516,7 +524,8 @@ Webhook and polling routes are legitimate external ingress boundaries. They must Sim app's own API routes to reuse provider or business logic. Extract the shared provider operation or authorized application use case and call it directly from the trigger handler and any other server adapter. HTTP is reserved for an actual cross-process/capability boundary. Tool work uses a -registered `InternalToolConfig.operation`; the retired `directExecution` property must not return. +registered `InternalToolConfig.operation`; a `directExecution` property fails +`bun run check:tool-request-boundary`. ### Trigger Definition - [ ] Created `utils.ts` with options, instructions, extra fields, and output builders diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index 16f8cef5b32..d7bf60a75e4 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -73,10 +73,8 @@ conditions freshly after every push. reviewThreads(first: 50) { pageInfo { hasNextPage endCursor } nodes { id isResolved path line comments(first: 5) { nodes { id databaseId author { login } body } } } } } } }' ``` - `[.comments[]] | last | .body`, not `... | .body | tail -1` — the latter pipes every matching - comment's full multi-line body through the pipeline and keeps only the final *line* of that - combined output (usually the "Reviews (n): Last reviewed commit..." footer), not the last - *comment*, so it silently misses the actual "Confidence Score: X/5" line. + The score is a line inside the body of Greptile's *latest* comment (`| last | .body`), which + it edits in place across rounds. `reviewThreads(first: 50)` is a single page — check `pageInfo.hasNextPage`. If `true`, don't stop yet: re-run the same query with `after: ""` and keep paging until `hasNextPage` is `false` before evaluating "clean." A PR with more than 50 threads is rare but @@ -142,9 +140,6 @@ conditions freshly after every push. git fetch origin staging && git log --oneline --reverse origin/staging..HEAD gh pr view --json commits -q '.commits[].messageHeadline' ``` - `--reverse` makes `git log` oldest-first, matching the PR commit list's order — plain - `git log` is newest-first, so without it a positional comparison can spuriously fail on any - multi-commit branch. These two lists must describe the same commits. A review loop runs many pushes across many rounds; checking sync only before the push (step 6) and never after is how a bad push or a PR whose commit history quietly went stale between rounds goes unnoticed. @@ -177,13 +172,10 @@ thread count across both bots, and whether every check finished and passed. ## Public-repo hygiene Every reply, comment and commit you post here is public and permanent, and review bots quote -your replies back so a leak propagates. Before each post, strip anything that ties the change to -a tenant: customer/company names, workspace/user/org/KB/connector IDs, emails, tenant hostnames, -verbatim document/sheet/folder names, log lines, and per-tenant DB output. Cite the mechanism and -aggregate numbers instead — see `/ship`'s "What to Omit" for the full list and the pre-publish -grep. Triaging a finding often means pasting evidence you gathered from prod; that is exactly the -moment this gets violated. Check before posting, not after: editing a comment does not unsend its -notification email. +your replies back, so a leak propagates. `/ship`'s "What to Omit" (the category list and the +pre-publish grep) applies to every post in this loop. Triaging a finding often means pasting +evidence gathered from prod — that is exactly the moment it gets violated. Run the grep on the +reply before posting, not after: editing a comment does not unsend its notification email. ## Hard rules diff --git a/.agents/skills/cleanup/SKILL.md b/.agents/skills/cleanup/SKILL.md index c2b6fdf8de7..4a286d701ed 100644 --- a/.agents/skills/cleanup/SKILL.md +++ b/.agents/skills/cleanup/SKILL.md @@ -14,11 +14,11 @@ User arguments: $ARGUMENTS ## Step 1 — Parallel analysis (read-only) -First parse the user's `$ARGUMENTS` into `scope` and `fix`: extract the `fix=true|false` token wherever it appears in the string (start, middle, or end), and treat everything else — with that token removed — as `scope`. Defaults: `scope` = your current changes, `fix` = true. The `fix` value is consumed by Step 3 — it does NOT propagate to these passes, which always run `fix=false`. +Parse `$ARGUMENTS` into `scope` and `fix`: extract the `fix=true|false` token wherever it appears in the string and strip it from `scope`; defaults are the current changes and `fix=true`. `fix` is consumed by Step 3 only — the passes below always run `fix=false`. Spawn all eight passes concurrently as subagents in a **single message** (multiple Agent tool calls). Each runs its skill on the parsed `scope` with `fix=false` — analysis and proposals ONLY, no edits. Instruct each agent to return its findings as a structured list: for every proposed change, the file path, line range, a one-line description of the change, and the exact before/after so the orchestrator can apply it without re-deriving. -Run these eight in parallel, substituting the parsed `scope` for `` in each invocation (pass the real scope text, never the literal ``): +Run these eight in parallel on the parsed `scope`: 1. `/you-might-not-need-an-effect fix=false` 2. `/you-might-not-need-a-memo fix=false` @@ -57,7 +57,6 @@ After all edits, run `bun run lint:check` (it runs `turbo run lint:check` across Output a summary across all eight passes: what each found, what was applied vs. skipped-as-redundant, and any proposals that need a human decision. -## Boundary Audit Guidance +## Boundary findings -- When removing route-local Zod schemas, replacing raw `fetch(` calls in hooks, or removing `as unknown as X` casts, do not introduce `// boundary-raw-fetch: ` or `// double-cast-allowed: ` annotations to silence the audit. Fix the underlying call instead — adopt a contract from `@/lib/api/contracts/**` and use `requestJson(contract, ...)` from `@/lib/api/client/request`, or refine the type so the double cast is unnecessary. -- Annotations are reserved for legitimate exceptions only: streaming responses, binary downloads, multipart uploads, signed-URL flows, OAuth redirects, external-origin requests, and double casts where no narrower type is available. Each annotation requires a non-empty reason; empty reasons fail `bun run check:api-validation:strict`. +Never resolve a boundary finding by adding a `// boundary-raw-fetch` / `// double-cast-allowed` annotation — fix the call (adopt the contract + `requestJson`, or narrow the type). Annotations are only for the documented exceptions in CLAUDE.md → Boundary annotations. diff --git a/.agents/skills/design-taste-frontend/SKILL.md b/.agents/skills/design-taste-frontend/SKILL.md index 319e5200abf..f7f472a74e9 100644 --- a/.agents/skills/design-taste-frontend/SKILL.md +++ b/.agents/skills/design-taste-frontend/SKILL.md @@ -4,6 +4,8 @@ source: https://github.com/leonxlnx/taste-skill — skills/taste-skill/SKILL.md description: Anti-slop frontend skill for landing pages, portfolios, and redesigns. The agent reads the brief, infers the right design direction, and ships interfaces that do not look templated. Real design systems when applicable, audit-first on redesigns, strict pre-flight check. --- +> **In this repo:** Tailwind 3.4 (`apps/sim/tailwind.config.ts`); animation via `import { motion } from 'framer-motion'` (not `motion/react` — rewrite every `motion/react` import in the samples below); icons from `@sim/emcn/icons`; colors through the CSS-variable tokens in `.claude/rules/sim-styling.md` (no hardcoded `text-gray-*`/hex/`zinc` utilities, no paired `dark:` utilities). This note overrides any conflicting guidance or code sample anywhere in this file. + # tasteskill: Anti-Slop Frontend Skill > Landing pages, portfolios, and redesigns. Not dashboards, not data tables, not multi-step product UI. @@ -130,7 +132,7 @@ Unless the design read picks a real design system (Section 2.A), these are the d * **INTERACTIVITY ISOLATION:** Any component using Motion, scroll listeners, or pointer physics MUST be an isolated leaf with `'use client'` at the top. Server Components render static layouts only. * **Styling:** **Tailwind v4** (default). Tailwind v3 only if the existing project demands it. * For v4: do NOT use `tailwindcss` plugin in `postcss.config.js`. Use `@tailwindcss/postcss` or the Vite plugin. -* **Animation:** **Motion** (the library formerly known as Framer Motion). Import from `motion/react` (`import { motion } from "motion/react"`). The `framer-motion` package still works as a legacy alias - prefer `motion/react` in new code. +* **Animation:** **Motion** (the library formerly known as Framer Motion). Outside this repo import from `motion/react`; in this repo import from `framer-motion` (see the note at the top). * **Fonts:** Always use `next/font` (Next.js) or self-host with `@font-face` + `font-display: swap`. Never link Google Fonts via `` in production. ### 3.B State @@ -139,11 +141,7 @@ Unless the design read picks a real design system (Section 2.A), these are the d * **NEVER** use `useState` to track continuous values driven by user input (mouse position, scroll progress, pointer physics, magnetic hover). Use Motion's `useMotionValue` / `useTransform` / `useScroll`. `useState` re-renders the React tree on every change and collapses on mobile. ### 3.C Icons -* **Allowed libraries (priority order):** `@phosphor-icons/react`, `hugeicons-react`, `@radix-ui/react-icons`, `@tabler/icons-react`. -* **Discouraged:** `lucide-react`. Acceptable only when the user explicitly asks for it or the project already depends on it. -* **NEVER hand-roll SVG icons.** If a glyph is missing, install a second library or compose from primitives - do not draw icon paths from scratch. -* **One family per project.** Do not mix Phosphor with Lucide in the same component tree. -* **Standardize `strokeWidth` globally** (e.g. `1.5` or `2.0`). +* **Icons:** in this repo, `@sim/emcn/icons` only — one family per tree, `strokeWidth` standardized. Outside this repo, pick one maintained library and standardize on it. ### 3.D Emoji Policy Discouraged by default in code, markup, and visible text. Replace symbols with icon-library glyphs. **Override:** allow emojis only when the user explicitly asks for a playful / chat-style / social-native vibe - and even then use them sparingly with intent. @@ -171,51 +169,28 @@ LLMs default to clichés. Override these defaults proactively. Each rule has a c * **Override:** Inter is acceptable when the user explicitly asks for a neutral / standard / Linear-style feel, or when the brief is a public-sector / accessibility-first site. * **Pairings to know:** `Geist` + `Geist Mono`, `Satoshi` + `JetBrains Mono`, `Cabinet Grotesk` + `Inter Tight`, `GT America` + `IBM Plex Mono`. -* **SERIF DISCIPLINE (VERY DISCOURAGED AS DEFAULT):** - * Serif is **very discouraged as the default font for any project.** "It feels creative / premium / editorial" is NOT a reason to reach for serif. The agent's default mental model that "creative brief = serif" is the single most-tested AI tell in production rounds. - * **Serif is only acceptable when ONE of these is explicitly true:** - - The brand brief literally names a serif font, OR - - The aesthetic family is genuinely editorial / luxury / publication / manuscript / heritage / vintage AND you can articulate why this specific serif fits this specific brand - * For everything else (creative agency, design studio, modern brand, premium consumer, portfolio, lifestyle), **default sans-serif display** (Geist Display, ABC Diatype, Söhne Breit, Cabinet Grotesk Display, Migra Sans, GT Walsheim, Inter Display, PP Neue Montreal). Sans display fonts are not "boring" — they are the default for the same reason black is the default in fashion. - * **EMPHASIS RULE (related):** When you want to emphasize a word within a headline (the kinetic "and `spatial` design" type move), use **italic or bold of the SAME font**. Do NOT inject a random serif word into a sans headline (or vice versa) just to add visual interest. Mixed-family emphasis is amateur. Italic/bold emphasis in the same family is the right move. - * **Specifically BANNED as defaults:** `Fraunces` and `Instrument_Serif` (the two LLM-favorite display serifs). - * **If a serif is justified** (rare, per the above), rotate from this pool, do NOT reuse the same serif across consecutive projects: PP Editorial New, GT Sectra Display, Cardinal Grotesque, Reckless Neue, Tiempos Headline, Recoleta, Cormorant Garamond, Playfair Display, EB Garamond, IvyPresto, Migra, Editorial Old, Saol Display, Söhne Breit Kursiv, Domaine Display, Canela, Schnyder, Tobias, NB Architekt, ITC Galliard. +* **Serif discipline:** Default to a sans display face (Geist Display, ABC Diatype, Söhne Breit, Cabinet Grotesk Display, Migra Sans, GT Walsheim, Inter Display, PP Neue Montreal). Use a serif only when the brand names one or the aesthetic is genuinely editorial/luxury/heritage and you can say in one line why this serif fits this brand; `Fraunces` and `Instrument Serif` are the generic picks, so prefer another (PP Editorial New, GT Sectra Display, Reckless Neue, Tiempos Headline, Cormorant Garamond, EB Garamond, Domaine Display, Canela). Emphasize a word with italic or bold of the same family, not a second family. -* **ITALIC DESCENDER CLEARANCE (mandatory):** When italic is used in display type and the word contains a descender letter (`y g j p q`), `leading-[1]` or `leading-none` will clip the descender. Use `leading-[1.1]` minimum and add `pb-1` or `mb-1` reserve on the wrapping element. Audit every italic word in display headlines before shipping. +* **Italic descender clearance:** When italic is used in display type and the word contains a descender letter (`y g j p q`), `leading-[1]` or `leading-none` will clip the descender. Use `leading-[1.1]` minimum and add `pb-1` or `mb-1` reserve on the wrapping element. Audit every italic word in display headlines before shipping. ### 4.2 Color Calibration * Max 1 accent color. Saturation < 80% by default. -* **THE LILA RULE:** The "AI Purple / Blue glow" aesthetic is discouraged as a default. No automatic purple button glows, no random neon gradients. Use neutral bases (Zinc / Slate / Stone) with high-contrast singular accents (Emerald, Electric Blue, Deep Rose, Burnt Orange, etc.). +* **The lila rule:** The "AI Purple / Blue glow" aesthetic is discouraged as a default. No automatic purple button glows, no random neon gradients. Use neutral bases (Zinc / Slate / Stone) with high-contrast singular accents (Emerald, Electric Blue, Deep Rose, Burnt Orange, etc.). * **Override:** if the brand or brief explicitly asks for purple / violet / lila, embrace it. But execute with intent: consistent palette, harmonised neutrals, restrained gradients. Not generic AI gradient slop. * **One palette per project.** Do not fluctuate between warm and cool grays within the same project. -* **COLOR CONSISTENCY LOCK (mandatory):** Once an accent color is chosen for a page, it is used on the WHOLE page. A warm-grey site does not suddenly get a blue CTA in section 7. A rose-accented site does not get a teal status badge in the footer. Pick one accent, lock it, audit every component before shipping. - -* **PREMIUM-CONSUMER PALETTE BAN (mandatory, second-most-recurring AI-tell):** - * For premium-consumer briefs (cookware, wellness, artisan, luxury, heritage craft, DTC home goods, etc.) the LLM default is **warm beige/cream + brass/clay/oxblood/ochre + espresso/ink dark text**. Concretely banned hex families as default backgrounds and accents: - - Backgrounds: `#f5f1ea`, `#f7f5f1`, `#fbf8f1`, `#efeae0`, `#ece6db`, `#faf7f1`, `#e8dfcb` (all "warm paper / cream / chalk / bone") - - Accents: `#b08947`, `#b6553a`, `#9a2436`, `#9c6e2a`, `#bc7c3a`, `#7d5621` (all "brass / clay / oxblood / ochre") - - Text: `#1a1714`, `#1a1814`, `#1b1814` (all "espresso / warm near-black") - * This palette is BANNED as the default reach for premium-consumer briefs. Every premium-consumer site you have ever shipped uses this exact palette. The brand becomes invisible. - * **Default alternatives (rotate, do not reuse):** - - **Cold Luxury:** silver-grey + chrome + smoke (think Tesla, Apple Watch Hermes-without-the-leather) - - **Forest:** deep green + bone + amber accent (think Filson, Patagonia premium) - - **Black and Tan:** true off-black + warm tan, sharp contrast, no beige - - **Cobalt + Cream:** saturated blue against a single neutral, no brass - - **Terracotta + Slate:** warm rust against cool grey, no brass - - **Olive + Brick + Paper:** muted olive plus brick-red accent - - **Pure monochrome + single saturated pop:** off-white + off-black + one bright accent (electric blue, emerald, hot pink, etc.) - * **Palette-rotation rule:** if the previous premium-consumer project you generated used the beige+brass family, this one MUST use a different family. Do not ship the same warm-craft palette twice in a row. - * **Override:** the beige+brass+espresso palette is acceptable ONLY when the brand brief explicitly names those colors, or when the brand identity is genuinely vintage / artisan / warm-craft AND you can articulate why this specific palette fits this specific brand. Default-reaching for it because "this is a cookware brief" is banned. +* **Color consistency lock:** Once an accent color is chosen for a page, it is used on the whole page. A warm-grey site does not suddenly get a blue CTA in section 7. A rose-accented site does not get a teal status badge in the footer. Pick one accent, lock it, audit every component before shipping. + +* **Premium-consumer palettes:** For premium-consumer briefs (cookware, wellness, artisan, DTC home goods), the warm cream + brass/clay/oxblood + espresso palette is the generic default; choose the palette from the brand's own assets and state the reason in one line. Alternatives that read as premium without the cliché: cold silver/chrome, deep green + bone + amber, off-black + tan, cobalt + one neutral, terracotta + slate, olive + brick, monochrome + one saturated accent. ### 4.3 Layout Diversification -* **ANTI-CENTER BIAS:** Centered Hero / H1 sections are avoided when `DESIGN_VARIANCE > 4`. Force "Split Screen" (50/50), "Left-aligned content / right-aligned asset", "Asymmetric white-space", or scroll-pinned structures. +* **Anti-center bias:** Centered Hero / H1 sections are avoided when `DESIGN_VARIANCE > 4`. Prefer "Split Screen" (50/50), "Left-aligned content / right-aligned asset", "Asymmetric white-space", or scroll-pinned structures. * **Override:** centered hero is OK for editorial / manifesto / launch-announcement briefs where the message itself is the design. ### 4.4 Materiality, Shadows, Cards -* Use cards ONLY when elevation communicates real hierarchy. Otherwise group with `border-t`, `divide-y`, or negative space. +* Use cards only when elevation communicates real hierarchy. Otherwise group with `border-t`, `divide-y`, or negative space. * When a shadow is used, tint it to the background hue. No pure-black drop shadows on light backgrounds. -* For `VISUAL_DENSITY > 7`: generic card containers are banned. Data metrics breathe in plain layout. -* **SHAPE CONSISTENCY LOCK (mandatory):** Pick ONE corner-radius scale for the page and stick to it. Options: all-sharp (radius 0), all-soft (radius 12-16px), all-pill (full radius for interactive). Mixed systems are allowed only when there is a documented rule (e.g. "buttons are full-pill, cards are 16px, inputs are 8px") and that rule is followed everywhere. Round buttons in a square layout, or square cards on a pill-button page, is broken design. +* For `VISUAL_DENSITY > 7`: no generic card containers. Data metrics breathe in plain layout. +* **Shape consistency lock:** Pick one corner-radius scale for the page and stick to it. Options: all-sharp (radius 0), all-soft (radius 12-16px), all-pill (full radius for interactive). Mixed systems are allowed only when there is a documented rule (e.g. "buttons are full-pill, cards are 16px, inputs are 8px") and that rule is followed everywhere. Round buttons in a square layout, or square cards on a pill-button page, is broken design. ### 4.5 Interactive UI States LLMs default to "static successful state only." Always implement full cycles: @@ -223,63 +198,63 @@ LLMs default to "static successful state only." Always implement full cycles: * **Empty States:** Beautifully composed; indicate how to populate. * **Error States:** Clear, inline (forms), or contextual (toasts only for transient). * **Tactile Feedback:** On `:active`, use `-translate-y-[1px]` or `scale-[0.98]` to simulate a physical push. -* **BUTTON CONTRAST CHECK (mandatory, a11y):** Before shipping any button, verify the button text is readable against the button background. White button + white text, `bg-white` CTA with `text-white` label, transparent button against the page background with no border → all banned. Audit every CTA: contrast ratio WCAG AA min (4.5:1 for body, 3:1 for large text 18px+). Same rule applies to ghost buttons over photographic backgrounds (use a backdrop, scrim, or stroke). -* **CTA BUTTON WRAP BAN (mandatory):** Button text MUST fit on one line at desktop. If a label like "VIEW SELECTED WORK" wraps to 2 or 3 lines, the button is broken. Fix by EITHER shortening the label (3 words max for primary CTAs, ideally 1-2) OR widening the button (do not artificially constrain `max-width` on CTAs). Wrapped CTAs at desktop are a Pre-Flight Fail. -* **NO DUPLICATE CTA INTENT (mandatory):** Two CTAs with the same intent on one page is a Pre-Flight Fail. Examples of same intent: "Get in touch" + "Contact us" + "Let's talk" + "Start a project" + "Start something" + "Reach out" = all "contact" intent → pick ONE label and use it everywhere on the page (nav, hero, footer). Same for "Try free" + "Get started" + "Sign up free" (all "signup" intent) and "View work" + "See selected work" + "Browse projects" (all "portfolio" intent). One label per intent. -* **FORM CONTRAST CHECK (mandatory, a11y):** Form inputs, placeholder text, focus rings, helper text, and error text all pass WCAG AA contrast against the section background. Light placeholders on a near-white form, white form on white page section, form labels grayer than 4.5:1 contrast → all banned. Audit every form before shipping. +* **Button contrast check (a11y):** Before shipping any button, verify the button text is readable against the button background: no white button + white text, no `bg-white` CTA with `text-white` label, no transparent button against the page background without a border. Audit every CTA: contrast ratio WCAG AA min (4.5:1 for body, 3:1 for large text 18px+). Same rule applies to ghost buttons over photographic backgrounds (use a backdrop, scrim, or stroke). +* **CTA button wrap:** Button text fits on one line at desktop. If a label like "VIEW SELECTED WORK" wraps to 2 or 3 lines, the button is broken. Fix by either shortening the label (3 words max for primary CTAs, ideally 1-2) OR widening the button (do not artificially constrain `max-width` on CTAs). Wrapped CTAs at desktop are a Pre-Flight Fail. +* **No duplicate CTA intent:** Two CTAs with the same intent on one page is a Pre-Flight Fail. Examples of same intent: "Get in touch" + "Contact us" + "Let's talk" + "Start a project" + "Start something" + "Reach out" = all "contact" intent → pick ONE label and use it everywhere on the page (nav, hero, footer). Same for "Try free" + "Get started" + "Sign up free" (all "signup" intent) and "View work" + "See selected work" + "Browse projects" (all "portfolio" intent). One label per intent. +* **Form contrast check (a11y):** Form inputs, placeholder text, focus rings, helper text, and error text all pass WCAG AA contrast against the section background: no light placeholders on a near-white form, no white form on a white section, no labels below 4.5:1. Audit every form before shipping. ### 4.6 Data & Form Patterns * Label ABOVE input. Helper text optional but present in markup. Error text BELOW input. Standard `gap-2` for input blocks. * No placeholder-as-label. Ever. -### 4.7 Layout Discipline (Hard Rules. Failing any of these is shipping broken work) +### 4.7 Layout Discipline -* **Hero MUST fit in the initial viewport.** Headline max 2 lines on desktop, subtext max **20 words** AND max 3-4 lines, CTAs visible without scroll. If the copy is too long: reduce font scale OR cut copy. If you cannot describe the value-prop in 20 words of subtext, the value-prop is unclear, not the rule too tight. Never let the hero overflow and force scroll to find the CTA. +* **Hero fits in the initial viewport.** Headline max 2 lines on desktop, subtext max **20 words** AND max 3-4 lines, CTAs visible without scroll. If the copy is too long: reduce font scale OR cut copy. If you cannot describe the value-prop in 20 words of subtext, the value-prop is unclear, not the rule too tight. Never let the hero overflow and force scroll to find the CTA. * **Hero font-scale discipline.** Plan font size and image size *together*. If the hero asset is large and the headline is more than 6 words, do not start at `text-7xl/text-8xl`. Default sensible range: `text-4xl md:text-5xl lg:text-6xl` for most heroes; `text-6xl md:text-7xl` only when the headline is 3-5 words. A 4-line hero headline is always a font-size error, never a copy-length error. -* **HERO TOP PADDING CAP (mandatory):** Hero top padding max `pt-24` (≈6rem) at desktop. More than that means the hero content floats halfway down the viewport and reads as a layout bug, not as intentional space. If your hero needs more breathing room, increase font scale or asset size, not top padding. -* **HERO STACK DISCIPLINE (max 4 text elements).** The hero is a single moment, not a feature list. Allowed text elements, max 4 in total: +* **Hero top padding cap:** Hero top padding max `pt-24` (≈6rem) at desktop. More than that means the hero content floats halfway down the viewport and reads as a layout bug, not as intentional space. If your hero needs more breathing room, increase font scale or asset size, not top padding. +* **Hero stack discipline (max 4 text elements).** The hero is a single moment, not a feature list. Allowed text elements, max 4 in total: 1. Eyebrow (small uppercase label) OR brand strip OR neither - pick zero or one 2. Headline (max 2 lines, see above) 3. Subtext (max 20 words, max 4 lines) 4. CTAs (1 primary + max 1 secondary) - - **BANNED in the hero:** tiny tagline below CTAs ("Works with GitHub, GitLab, and self-hosted Git"), trust micro-strip ("Used by engineering teams at..."), pricing teaser ("Free for solo, $10/user for teams"), feature bullet list, social-proof avatar row. All of those move to dedicated sections directly below the hero. + - **Not in the hero:** tiny tagline below CTAs ("Works with GitHub, GitLab, and self-hosted Git"), trust micro-strip ("Used by engineering teams at..."), pricing teaser ("Free for solo, $10/user for teams"), feature bullet list, social-proof avatar row. All of those move to dedicated sections directly below the hero. - If you have an eyebrow AND a tagline below CTAs in the same hero, drop the tagline. If you have a brand strip AND a tagline, drop the tagline. One small text element per hero, max. -* **"Used by" / "Trusted by" logo wall belongs UNDER the hero, never inside it.** The hero is for the value prop and primary CTA. The logo wall is a separate section directly below. Do not stuff trust logos into the same flex row as the hero copy. -* **Navigation MUST render on a single line on desktop.** If items don't fit at `lg` (1024px), condense labels, drop secondary items, or move to a hamburger. A two-line nav at desktop is broken design. +* **"Used by" / "Trusted by" logo wall belongs under the hero, not inside it.** The hero is for the value prop and primary CTA. The logo wall is a separate section directly below. Do not stuff trust logos into the same flex row as the hero copy. +* **Navigation renders on a single line on desktop.** If items don't fit at `lg` (1024px), condense labels, drop secondary items, or move to a hamburger. A two-line nav at desktop is broken design. * **Navigation height cap: 80px max desktop, default 64-72px.** No huge "agency" nav bars that eat 15% of the viewport. -* **Bento grids MUST have rhythm, not one-sided repetition.** Do not stack 6 left-image / right-text rows. Vary the composition: alternate full-width feature rows, asymmetric tile sizes, vertical breaks. -* **BENTO CELL COUNT RULE (mandatory):** A bento grid has EXACTLY as many cells as you have content for. 3 items → 3 cells (1+2 split, or 2+1, or asymmetric trio). 5 items → 5 cells (2+3, 3+2, hero+4, etc.). If your grid has an empty cell in the middle or at the end, you planned wrong. Re-shape the grid; do not paste a blank tile. +* **Bento grids have rhythm, not one-sided repetition.** Do not stack 6 left-image / right-text rows. Vary the composition: alternate full-width feature rows, asymmetric tile sizes, vertical breaks. +* **Bento cell count:** A bento grid has exactly as many cells as you have content for. 3 items → 3 cells (1+2 split, or 2+1, or asymmetric trio). 5 items → 5 cells (2+3, 3+2, hero+4, etc.). If your grid has an empty cell in the middle or at the end, you planned wrong. Re-shape the grid; do not paste a blank tile. * **Section-Layout-Repetition Ban.** Once you use a layout family for a section (e.g., 3-column-image-cards, full-width-quote, split-text-image), that family can appear at most ONCE on the page. "Selected commissions" must not look like "What we do." A landing page with 8 sections must use at least 4 different layout families. -* **ZIGZAG ALTERNATION CAP (mandatory).** Alternating "left-image + right-text" then "left-text + right-image" zigzag layout = banal. Max 2 sections in a row with this image+text-split pattern. The 3rd consecutive image+text split is a Pre-Flight Fail. Break the pattern with a full-width section, a vertical-stack section, a bento grid, a marquee, or a different layout family. -* **EYEBROW RESTRAINT (mandatory, the #1 violated rule in production tests).** An "eyebrow" is the small uppercase wide-tracking label sitting above a section headline (e.g. `FOUR COLORWAYS`, `SELECTED WORK`, `THE HARDWARE`, `Git-native task management`). Typical CSS signature: `text-[11px] uppercase tracking-[0.18em]`, `font-mono text-[10.5px] uppercase tracking-[0.22em]`. Every AI-built site puts an eyebrow above EVERY section header, producing the same templated rhythm. Hard rule: +* **Zigzag alternation cap.** Alternating "left-image + right-text" then "left-text + right-image" zigzag layout = banal. Max 2 sections in a row with this image+text-split pattern. The 3rd consecutive image+text split is a Pre-Flight Fail. Break the pattern with a full-width section, a vertical-stack section, a bento grid, a marquee, or a different layout family. +* **Eyebrow restraint.** An "eyebrow" is the small uppercase wide-tracking label sitting above a section headline (e.g. `FOUR COLORWAYS`, `SELECTED WORK`, `THE HARDWARE`, `Git-native task management`). Typical CSS signature: `text-[11px] uppercase tracking-[0.18em]`, `font-mono text-[10.5px] uppercase tracking-[0.22em]`. Every AI-built site puts an eyebrow above EVERY section header, producing the same templated rhythm. Hard rule: - **Maximum 1 eyebrow per 3 sections.** Hero counts as 1. So a page with 9 sections may use at most 3 eyebrows total. - If section A has an eyebrow, the next 2 sections cannot have one. - **Pre-Flight Check is mechanical:** count instances of `uppercase tracking` (or similar small-caps mono labels above headlines) across all section components. If count > ceil(sectionCount / 3), the output fails. - **What to do instead of an eyebrow:** drop it entirely. The headline alone is enough. If you need to categorize a section, the section's location on the page already categorizes it; no label needed. -* **SPLIT-HEADER BAN (mandatory).** The pattern "left big headline + right small explainer paragraph" as a section header (left col-span-7/8, right col-span-4/5 with a small body paragraph floating in the right column) is **banned as default**. Sections should have ONE focused message. If you genuinely need both a headline and an explainer paragraph, stack them vertically (headline on top, body below, max-width 65ch). Reach for the split-header pattern only when there is a real compositional reason (e.g., the right column carries a visual or interactive element, not just filler text). -* **Bento Background Diversity (mandatory).** Bento and feature-grid sections cannot be 6 white-on-white cards with text inside. At least 2-3 cells in any multi-cell grid need real visual variation: a real image, a brand-appropriate gradient (not AI-purple), a pattern, a tinted background. A cream-on-cream bento with only typography inside reads as boring AI default, even when the rest of the page is good. +* **Split-header.** The pattern "left big headline + right small explainer paragraph" as a section header (left col-span-7/8, right col-span-4/5 with a small body paragraph floating in the right column) is not a default. Sections should have ONE focused message. If you genuinely need both a headline and an explainer paragraph, stack them vertically (headline on top, body below, max-width 65ch). Reach for the split-header pattern only when there is a real compositional reason (e.g., the right column carries a visual or interactive element, not just filler text). +* **Bento background diversity.** Bento and feature-grid sections cannot be 6 white-on-white cards with text inside. At least 2-3 cells in any multi-cell grid need real visual variation: a real image, a brand-appropriate gradient (not AI-purple), a pattern, a tinted background. A cream-on-cream bento with only typography inside reads as boring AI default, even when the rest of the page is good. * **Mobile collapse must be explicit per section.** For every multi-column layout, declare the `< 768px` fallback in the same component. No "it'll work, Tailwind handles it" assumptions. ### 4.8 Image & Visual Asset Strategy -Landing pages and portfolios are **visual products**. Text-only pages with fake-screenshot divs are slop. +Landing pages and portfolios are **visual products**. Text-only pages with fake-screenshot divs read as unfinished. **Priority order for visual assets:** -1. **Image-generation tool first.** If ANY image-gen tool is available in the environment (`generate_image`, MCP image tool, IDE-integrated gen, OpenAI image tools, etc.) you MUST use it to create section-specific assets: hero photography, product shots, texture backgrounds, mood images. Generate at the right aspect ratio for the section. Do not skip this step because hand-rolled CSS feels faster. +1. **Image-generation tool first.** When an image-gen tool is available, use it for section-specific assets (hero photography, product shots, textures) at the section's aspect ratio. 2. **Real web images second.** When no gen tool is available, use real photography sources. Acceptable defaults: * `https://picsum.photos/seed/{descriptive-seed}/{w}/{h}` for placeholder photography (seed should describe the section, e.g. `marrow-cookware-kitchen`) * Actual stock or brand URLs when the brief provides them * Open-license sources (Unsplash via direct URL, Pexels) if explicitly allowed -3. **Last resort: tell the user.** If neither is possible, do NOT fill the page with hand-rolled SVG illustrations or div-based "fake screenshots." Instead, leave clearly-labeled placeholder slots (``) and at the end of the response say: *"This page needs real images at: \[list of placements\]. Please generate or provide them."* +3. **Last resort: tell the user.** If neither is possible, do not fill the page with hand-rolled SVG illustrations or div-based "fake screenshots." Instead, leave clearly-labeled placeholder slots (``) and at the end of the response say: *"This page needs real images at: \[list of placements\]. Please generate or provide them."* **Even minimalist sites need real images.** A pure-text page is not minimalism. It is incomplete work. Even an editorial Linear-style site needs at least 2-3 real images (hero, one product/lifestyle shot, one supporting image). Generate B&W minimalist photography if the brief is restrained; do not skip images entirely because the dial is low. -**Real company logos for social proof.** When the brief calls for a "Trusted by / Used by / Customers" logo wall, do NOT default to plain text wordmarks (`Acme Co` styled in a row). Use real SVG logos: +**Real company logos for social proof.** When the brief calls for a "Trusted by / Used by / Customers" logo wall, do not default to plain text wordmarks (`Acme Co` styled in a row). Use real SVG logos: * **Source: Simple Icons** (`https://cdn.simpleicons.org/{slug}/ffffff` for any color, or `simple-icons` npm package). Covers most known brands. * **Alternative: devicon** for tech-stack logos (`@svgr/cli` or CDN). * **Make-up the brand name? Then make-up an SVG mark too.** Generate a simple monogram (one letter in a circle, two-letter ligature, abstract glyph) rendered as an inline `` matching the page style. Plain text wordmarks for invented brand names look generic. * **Always** ensure logos render in both light and dark mode (white-on-dark, black-on-light, or single-color theme variable). -* **LOGO-ONLY rule (mandatory):** logo wall = logos and nothing else. Do NOT print industry / category labels below each logo (no `Vercel` + `hosting` underneath, no `Stripe` + `payments`, no `Cloudflare` + `infra`). The logo is the credibility, the label adds nothing the user does not already know. Optional: brand name as alt-text for screen readers, optional link to the brand's site. That is it. +* **Logo-only rule:** logo wall = logos and nothing else. Do not print industry / category labels below each logo (no `Vercel` + `hosting` underneath, no `Stripe` + `payments`, no `Cloudflare` + `infra`). The logo is the credibility, the label adds nothing the user does not already know. Optional: brand name as alt-text for screen readers, optional link to the brand's site. That is it. **Hand-rolled illustrations:** * SVG icons from libraries: fine (see Section 3.C). @@ -288,7 +263,7 @@ Landing pages and portfolios are **visual products**. Text-only pages with fake- - It's a single, simple geometric mark (a square, a circle, a wordmark in display type) - You're confident in the output quality -**Div-based fake screenshots are banned.** A "hand-built product preview" rendered with `
` rectangles, fake task lists, fake dashboards, fake terminal windows is a Tell. If you need to show a product: +**No div-based fake screenshots.** A "hand-built product preview" rendered with `
` rectangles, fake task lists, fake dashboards, fake terminal windows is a Tell. If you need to show a product: * Use a real screenshot URL if one exists * Generate one via image tool * Use a real component preview (an actual mini-version of the UI inside the page) @@ -313,13 +288,13 @@ Landing pages live on the **first impression**, not the full read. Cut ruthlessl - Carousel for breadth-heavy lists (testimonials, logos, capabilities) - Marquee for "lots-of-things-that-don't-need-individual-attention" A spec sheet with 10 rows + a hairline under every row is the WORST default. Either group rows into 2-3 chunks with sparse dividers, or move to a card-per-spec layout. -* **Spec sheets specifically (the Marrow-cookware pattern).** A long product specification table with `border-b` on every row is the AI default for cookware / hardware / apparel / artisan-goods briefs. Banned. Concrete alternatives: +* **Spec sheets specifically.** A long product specification table with `border-b` on every row is the generic default for cookware / hardware / apparel / artisan-goods briefs. Concrete alternatives: - **2-col card grid:** each spec gets its own card with the spec name, the value (large display number), and a one-line "why it matters" body. Cards arranged 2-col on desktop, 1-col mobile. - **Scroll-snap horizontal pills:** each spec is a pill, user can flick through. - **Grouped chunks:** group 10 specs into 3 logical clusters (e.g. "Materials", "Cooking", "Warranty"), each cluster gets ONE soft divider and a cluster heading. - **Featured-vs-rest:** 3-4 hero specs visualised as large display tiles, the rest collapsed under a "View full specifications" disclosure. -* **COPY SELF-AUDIT (mandatory before ship):** Before declaring any task done, re-read every visible string on the page (headlines, subheads, eyebrows, button labels, body copy, captions, alt text, footer text, error messages). Flag any string that is: +* **Copy self-audit before ship:** Before declaring any task done, re-read every visible string on the page (headlines, subheads, eyebrows, button labels, body copy, captions, alt text, footer text, error messages). Flag any string that is: - **Grammatically broken** ("free on its past", "two plans but one is honest", "to put it on the table" out of context) - **Has unclear referents** ("we plan to stay that way" without prior context) - **Sounds like AI hallucination** (cute-but-wrong wordplay, forced metaphors that don't track, "elegant nothing" phrases) @@ -335,15 +310,15 @@ Landing pages live on the **first impression**, not the full read. Cut ruthlessl * **Max 3 lines** of quote body. Never 6. If the original quote is longer → cut it. A landing-page quote is a snippet, not the full review. * For very small font sizes (e.g. footer-style testimonials), the line cap can stretch slightly. Spirit: "fits in a glance." -* **No em-dashes inside the quote text** as design flourish (long pauses, kinetic em-dashes, em-dash-bullets). See Section 9.G - em-dash is completely banned. +* No em-dashes in quote text (Section 9.G). * Attribution: name + role + (optionally) company. Never name only ("- Sarah"). * Quote marks: use real typographic quotes ( " " ) or none at all. Not straight ASCII ( " ). ### 4.11 Page Theme Lock (Light / Dark Mode Consistency) -The page has ONE theme. Sections do not invert. +The page has one theme. Sections do not invert. -* If the page is dark mode, ALL sections are dark mode. No light-mode-warm-paper section sandwiched between dark sections (or vice versa). The user must not feel they walked into a different website mid-scroll. +* If the page is dark mode, all sections are dark mode. No light-mode-warm-paper section sandwiched between dark sections (or vice versa). The user must not feel they walked into a different website mid-scroll. * The exception: if the brief explicitly calls for a "Color Block Story" or "Theme Switch on Scroll" device AND that is a deliberate composition (one full theme switch with a strong transition, not random alternation), it is allowed once per page. * Default behaviour: pick light, dark, or auto (`prefers-color-scheme`) at the page level and lock it. Section-level background tints within the same theme family are fine (`bg-zinc-950` next to `bg-zinc-900`); flipping to `bg-amber-50` in the middle of a `bg-zinc-950` page is broken. * When using a design system with built-in theming (Radix Themes, shadcn/ui with ``), set the theme ONCE in `layout.tsx` or the page root. Do not let individual sections override. @@ -355,11 +330,11 @@ The page has ONE theme. Sections do not invert. These are tools, not defaults. Use them when the design read calls for them. **None of these fire automatically.** * **Liquid Glass / Glassmorphism:** Appropriate for premium consumer, Apple-adjacent, luxury brand, or media-overlay vibes. Inappropriate for dashboards, public-sector, or "boring B2B." When used, go beyond `backdrop-blur`: add a 1px inner border (`border-white/10`) and a subtle inner shadow (`shadow-[inset_0_1px_0_rgba(255,255,255,0.1)]`) for physical edge refraction. Provide a solid-fill fallback under `prefers-reduced-transparency`. -* **Magnetic Micro-physics:** Use when `MOTION_INTENSITY > 5` AND the brief reads premium / playful / agency. Implement EXCLUSIVELY with Motion's `useMotionValue` / `useTransform` outside the React render cycle. Never `useState`. See Section 3.B. +* **Magnetic Micro-physics:** Use when `MOTION_INTENSITY > 5` and the brief reads premium / playful / agency. Implement with Motion's `useMotionValue` / `useTransform` outside the React render cycle, not `useState`. See Section 3.B. * **Perpetual Micro-Interactions** (Pulse, Typewriter, Float, Shimmer, Carousel): Use when `MOTION_INTENSITY > 5` AND the section actively benefits from motion (status indicators, live feeds, AI-feel). **Not every card needs an infinite loop.** If a section is informational, leave it still. Apply Spring Physics (`type: "spring", stiffness: 100, damping: 20`) - no linear easing. * **"Motion claimed, motion shown."** If `MOTION_INTENSITY > 4`, the page must actually move: entry transitions on hero, scroll-reveal on key sections, hover physics on CTAs, at minimum. A static page that claims `MOTION_INTENSITY: 7` is broken. Conversely, if you cannot ship working motion in the available scope, drop the dial to 3 and ship a clean static page. Never half-build motion that breaks (cut-off ScrollTriggers, jumpy enters, missing cleanups). -* **MOTION MUST BE MOTIVATED (mandatory).** Before adding any animation, ask: "what does this animation communicate?" Valid answers: hierarchy (drawing attention to the right thing), storytelling (revealing content in sequence that matches a narrative), feedback (acknowledging a user action), state transition (showing something changed). Invalid answer: "it looked cool". GSAP everywhere because GSAP is available is amateur. Each ScrollTrigger, each marquee, each pinned section needs a reason. If you cannot articulate the reason in one sentence, drop the animation. -* **MARQUEE MAX-ONE-PER-PAGE (mandatory).** Horizontal scrolling text marquees ("logos endlessly scrolling", "manifesto scrolling sideways", "kinetic word strip") are appropriate at most ONCE per page. Two or more marquees on the same page reads as lazy filler. Pick the one section where the marquee actually serves the content; the others get a different layout. +* **Motion is motivated.** Before adding any animation, ask: "what does this animation communicate?" Valid answers: hierarchy (drawing attention to the right thing), storytelling (revealing content in sequence that matches a narrative), feedback (acknowledging a user action), state transition (showing something changed). Invalid answer: "it looked cool". GSAP everywhere because GSAP is available is amateur. Each ScrollTrigger, each marquee, each pinned section needs a reason. If you cannot articulate the reason in one sentence, drop the animation. +* **Marquee: at most one per page.** Horizontal scrolling text marquees ("logos endlessly scrolling", "manifesto scrolling sideways", "kinetic word strip") are appropriate at most ONCE per page. Two or more marquees on the same page reads as lazy filler. Pick the one section where the marquee actually serves the content; the others get a different layout. * **GSAP Sticky-Stack Pattern (when scroll-stack is used).** A "card stack on scroll" must be a REAL sticky-stack, not a sequential reveal list. See Section 5.A below for the canonical code skeleton. Common failure: trigger fires halfway through scroll instead of pinning at viewport top. Fix: `start: "top top"` not `start: "top center"` or `"top 80%"`. * **GSAP Horizontal-Pan Pattern (when horizontal scroll-hijack is used).** See Section 5.B below for the canonical skeleton. Common failure: animation starts before the section is pinned, so the user sees half a slide. Same fix: `start: "top top"`, pin the wrapper, scrub the inner track. @@ -509,28 +484,28 @@ Use this for: feature lists, testimonial grids, logo walls, anything that just n ### 5.D Forbidden Animation Patterns -* **`window.addEventListener("scroll", ...)`** is banned. It runs on every scroll frame, jank-prone, no batching. Use Motion's `useScroll()`, GSAP's `ScrollTrigger`, IntersectionObserver, or CSS `scroll-driven animations` (`animation-timeline: view()`). +* **`window.addEventListener("scroll", ...)`.** It runs on every scroll frame, jank-prone, no batching. Use Motion's `useScroll()`, GSAP's `ScrollTrigger`, IntersectionObserver, or CSS `scroll-driven animations` (`animation-timeline: view()`). * **Custom scroll progress calculations using `window.scrollY`** in React state. Same reason. Re-renders on every frame. * **`requestAnimationFrame` loops that touch React state.** Use motion values (`useMotionValue` + `useTransform`) instead. * **Layout Transitions:** Use Motion's `layout` and `layoutId` props for visible state changes (re-ordering lists, expanding modals, shared elements between routes). Do not wrap static content in `layout` props "for safety" - it costs measurement work. -* **Staggered Orchestration:** Use `staggerChildren` (Motion) or CSS cascade (`animation-delay: calc(var(--index) * 100ms)`) for reveal moments where sequence matters. For `staggerChildren`, parent (`variants`) and children MUST share the same Client Component tree. +* **Staggered Orchestration:** Use `staggerChildren` (Motion) or CSS cascade (`animation-delay: calc(var(--index) * 100ms)`) for reveal moments where sequence matters. For `staggerChildren`, parent (`variants`) and children share the same Client Component tree. --- ## 6. PERFORMANCE & ACCESSIBILITY GUARDRAILS ### 6.A Hardware Acceleration -* Animate ONLY `transform` and `opacity`. Never animate `top`, `left`, `width`, `height`. +* Animate only `transform` and `opacity`, not `top`, `left`, `width`, `height`. * Use `will-change: transform` sparingly - only on elements that will actually animate. -### 6.B Reduced Motion (mandatory) -* **Any motion above `MOTION_INTENSITY > 3` MUST honor `prefers-reduced-motion`.** This is non-negotiable. +### 6.B Reduced Motion +* **Any motion above `MOTION_INTENSITY > 3` honors `prefers-reduced-motion`.** * In Motion: wrap with `useReducedMotion()` and degrade to static. * In CSS: gate animations behind `@media (prefers-reduced-motion: no-preference)` or provide an override block under `@media (prefers-reduced-motion: reduce)` that disables. -* Infinite loops, parallax, scroll-hijack, and magnetic physics MUST collapse to static / instant under reduced motion. +* Infinite loops, parallax, scroll-hijack, and magnetic physics collapse to static / instant under reduced motion. -### 6.C Dark Mode (mandatory for any consumer-facing page) -* Design for **both modes from the start**. Never ship light-only or dark-only without explicit user instruction. +### 6.C Dark Mode (consumer-facing pages) +* Design for **both modes from the start**; ship light-only or dark-only only on explicit user instruction. * Use Tailwind `dark:` variant OR CSS variables for tokens. Pick one strategy per project. * **Do not prescribe specific dark-mode colors here.** The brief decides. Maintain visual hierarchy, brand identity, and WCAG AA contrast (AAA for body) across both modes. * Respect `prefers-color-scheme: dark`. Default to system preference unless the brand insists on one mode. @@ -542,11 +517,11 @@ Use this for: feature lists, testimonial grids, logo walls, anything that just n * Run Lighthouse before declaring a page done. ### 6.E DOM Cost -* Apply grain / noise filters EXCLUSIVELY to fixed, `pointer-events-none` pseudo-elements (e.g., `fixed inset-0 z-[60] pointer-events-none`). NEVER on scrolling containers - continuous GPU repaints destroy mobile FPS. +* Apply grain / noise filters only to fixed, `pointer-events-none` pseudo-elements (e.g., `fixed inset-0 z-[60] pointer-events-none`), not to scrolling containers - continuous GPU repaints destroy mobile FPS. * Be aware of bundle size. Motion is not tiny. Three.js is large. Lazy-load anything that's not above-the-fold. ### 6.F Z-Index Restraint -NEVER spam arbitrary `z-50` or `z-10`. Use z-index strictly for systemic layer contexts (sticky navbars, modals, overlays, grain). Document the z-index scale in a project constants file. +No arbitrary `z-50` or `z-10`. Use z-index only for systemic layer contexts (sticky navbars, modals, overlays, grain). Document the z-index scale in a project constants file. --- @@ -556,17 +531,17 @@ NEVER spam arbitrary `z-50` or `z-10`. Use z-index strictly for systemic layer c * **1-3 (Predictable):** Symmetrical CSS Grid (12-col, equal fr-units), equal paddings, centered alignment. * **4-7 (Offset):** `margin-top: -2rem` overlaps, varied image aspect ratios (4:3 next to 16:9), left-aligned headers over center-aligned data. * **8-10 (Asymmetric):** Masonry layouts, CSS Grid with fractional units (`grid-template-columns: 2fr 1fr 1fr`), massive empty zones (`padding-left: 20vw`). -* **MOBILE OVERRIDE:** For levels 4-10, asymmetric layouts above `md:` MUST collapse to strict single-column (`w-full`, `px-4`, `py-8`) on viewports `< 768px`. +* **Mobile override:** For levels 4-10, asymmetric layouts above `md:` collapse to strict single-column (`w-full`, `px-4`, `py-8`) on viewports `< 768px`. ### MOTION_INTENSITY (Level 1-10) * **1-3 (Static):** No automatic animations. CSS `:hover` and `:active` states only. `prefers-reduced-motion` is the default mode anyway. -* **4-7 (Fluid CSS):** `transition: all 0.3s cubic-bezier(0.16, 1, 0.3, 1)`. `animation-delay` cascades for load-ins. Focus on `transform` and `opacity`. -* **8-10 (Advanced Choreography):** Complex scroll-triggered reveals, parallax, scroll-driven animation (CSS `animation-timeline` or GSAP ScrollTrigger). Use Motion hooks. **NEVER use `window.addEventListener('scroll')`** - it is a hard ban, not a "prefer-not." See Section 5.D for the allowed alternatives. +* **4-7 (Fluid CSS):** `transition: transform 0.3s cubic-bezier(0.16, 1, 0.3, 1), opacity 0.3s cubic-bezier(0.16, 1, 0.3, 1)`; never `transition: all`. `animation-delay` cascades for load-ins. Focus on `transform` and `opacity`. +* **8-10 (Advanced Choreography):** Complex scroll-triggered reveals, parallax, scroll-driven animation (CSS `animation-timeline` or GSAP ScrollTrigger). Use Motion hooks, never `window.addEventListener('scroll')` (Section 5.D has the alternatives). ### VISUAL_DENSITY (Level 1-10) * **1-3 (Art Gallery):** Lots of white space. Huge section gaps (`py-32` to `py-48`). Expensive, clean. * **4-7 (Daily App):** Standard web app spacing (`py-16` to `py-24`). -* **8-10 (Cockpit):** Tight paddings. No card boxes; 1px lines separate data. Mandatory: `font-mono` for all numbers. +* **8-10 (Cockpit):** Tight paddings. No card boxes; 1px lines separate data. `font-mono` for all numbers. --- @@ -621,16 +596,15 @@ Avoid these signatures unless the brief explicitly asks for them. * **NO filler verbs.** "Elevate", "Seamless", "Unleash", "Next-Gen", "Revolutionize" → concrete verbs only. ### 9.E External Resources & Components -* **NO hand-rolled SVG icons.** Use Phosphor / HugeIcons / Radix / Tabler. Lucide on explicit request only. * **Hand-rolled decorative SVGs strongly discouraged** as default (see Section 4.8). * **NO div-based fake screenshots.** Never build a fake product UI out of `
` rectangles to simulate a screenshot. Use real images, generated images, or skip the preview. * **NO broken Unsplash links.** Use `https://picsum.photos/seed/{descriptive-string}/{w}/{h}`, or generated photo placeholders, or actual assets. * **shadcn/ui customization:** Allowed, but NEVER in default state. Customize radii, colors, shadows, typography to the project aesthetic. * **Production-Ready Cleanliness:** Code visually clean, memorable, meticulously refined. -### 9.F Production-Test Tells (banned outright) +### 9.F Production Tells -These patterns came out of real LLM-generated landing-page tests. They are the signatures the model defaults to when it tries to "look designed." Treat them as hard bans unless the brief explicitly calls for one. +Common signatures of generated landing pages. Skip them unless the brief asks for one. **Hero & top-of-page** * **NO version labels in the hero.** `V0.6`, `v2.0`, `BETA`, `INVITE-ONLY PREVIEW`, `EARLY ACCESS`, `ALPHA` - banned as default eyebrows. Only acceptable when the brief is explicitly about a product launch / preview status. @@ -647,7 +621,7 @@ These patterns came out of real LLM-generated landing-page tests. They are the s * **NO decorative colored status dots on every list/nav/badge.** A colored dot before "ONE Q4 SLOT OPEN" or before every nav link, or every task row - banned by default. Acceptable only when the dot conveys actual semantic state (a server status, an availability flag) and is used sparingly. **Em-dashes & typography flourishes** -* **NO em-dash (`—`) as a design element OR anywhere else.** See Section 9.G below for the complete, non-negotiable ban. The em-dash character is forbidden in headlines, eyebrows, pills, body copy, quotes, attribution, captions, button text, and alt text. Use the regular hyphen (`-`). +* **No em-dashes.** See Section 9.G. * **NO `
`-broken-and-italicized headlines** as a default "design move." "for thirty\*years.*" type splits. Headlines should read naturally first, get clever only when the brief demands it. * **NO vertical rotated text** ("INDEX OF WORK, 2018 - 2026" rotated 90°). Agency-portfolio cliché. Use it only when the brief is explicitly agency / Awwwards / experimental AND it serves a real composition purpose. * **NO crosshair / hairline grid lines as decoration.** Vertical and horizontal lines drawn just to make the page "feel designed" - banned. Use them only when they organize real content. @@ -681,31 +655,17 @@ These patterns came out of real LLM-generated landing-page tests. They are the s **Locale, time, scroll cues** * **Locale / city-name / time / weather strips are banned for 99% of briefs.** "Lisbon, working with founders" in the hero, "1200-690 Lisbon, Portugal" in the footer, "Lisbon 14:23 · 18°C" in the nav. These are agency-portfolio decoration tells. Allowed ONLY when: the brief explicitly describes a globally-distributed studio with timezone-relevant work, OR a travel-focused brand, OR a real-world physical venue. A single contact-address mention in the footer is fine; an atmospheric locale strip is not. * **Scroll cues are banned.** `Scroll`, `↓ scroll`, `Scroll to explore`, `Scroll to walk through it`, animated mouse-wheel icons. If the user has not scrolled yet, they are looking at the hero. They know what scroll is. The bottom of the viewport does not need a label. -* **ZERO decorative status dots by default.** A coloured dot before nav items, before list rows, before badges, before status labels is a Tell. Only acceptable when conveying real semantic state (a live indicator on actual server status, a live availability flag) and limited to one per page section. - -### 9.G EM-DASH BAN (the single most-violated Tell) - -**Em-dash (`—`) is COMPLETELY banned.** It is the LLM's signature stylistic crutch and it is the #1 visual Tell in production tests. There is no "limited use" allowance, no "natural language frequency" allowance, no "in body copy is fine" allowance. None. - -* **Banned in headlines.** Use a period or a comma. -* **Banned in eyebrows / labels / pills / button text / image captions / nav items.** Replace with line breaks, columns, or hairlines. -* **Banned in body copy.** Restructure the sentence: two sentences with a period, OR a comma, OR parentheses, OR a colon. -* **Banned in quote attribution.** Use a normal hyphen with spaces (` - `) or a line break + smaller-weight name. -* **Banned in en-dash form too (`–`) when used as a separator.** Date ranges (`2018-2026`) use a hyphen. Number ranges (`€40-80k`) use a hyphen. +* **No decorative status dots by default.** A coloured dot before nav items, before list rows, before badges, before status labels is a Tell. Only acceptable when conveying real semantic state (a live indicator on actual server status, a live availability flag) and limited to one per page section. -The ONLY permitted dash characters on the page are: -* Regular hyphen `-` (for compound words, ranges, line dividers in markup) -* Minus sign in math (`-5°C`) +### 9.G Dashes -If your output contains a single `—` or `–` anywhere visible to the user, the output fails the Pre-Flight Check and must be rewritten. - -This rule is non-negotiable. The agent has historically ignored em-dash limits when phrased as "use sparingly." The phrasing here is binary: zero em-dashes. +Visible copy uses hyphens, commas, periods, colons, or parentheses; no em-dashes (`—`) or en-dash separators (`–`). Date and number ranges use a hyphen. --- ## 10. REFERENCE VOCABULARY (Pattern Names the Agent Should Know) -This is a vocabulary, not a library. The agent should KNOW these pattern names to communicate about them, design with them in mind, and reach for them when the design read calls for them. **Implementations and code sketches live in the Block Library (Section 12), which is populated iteratively.** +This is a vocabulary, not a library. Know these pattern names to communicate about them, design with them in mind, and reach for them when the design read calls for them. ### Hero Paradigms * **Asymmetric Split Hero** - Text on one side, asset on the other, generous white space. @@ -777,7 +737,7 @@ This is a vocabulary, not a library. The agent should KNOW these pattern names t * **Motion (`motion/react`)** - default for UI / Bento / state-change motion. * **GSAP + ScrollTrigger** - for full-page scrolltelling and scroll hijacks. Isolate in dedicated leaf components with `useEffect` cleanup. * **Three.js / WebGL** - for canvas backgrounds and 3D scenes. Same isolation rule. -* **NEVER mix GSAP / Three.js with Motion in the same component tree.** They fight over the same frames. +* **Do not mix GSAP / Three.js with Motion in the same component tree.** They fight over the same frames. --- @@ -833,67 +793,6 @@ Never modify without explicit user approval: --- -## 12. THE BLOCK LIBRARY (Contract - Implementations Land Here Iteratively) - -The Reference Vocabulary (Section 10) names patterns. The Block Library implements them with real props, real motion specs, and real code sketches. - -**Status:** schema defined here. Blocks will be added iteratively. Do not freelance new blocks without following this schema. - -### 12.A File Location -``` -skills/taste-skill/blocks/ - hero/ - asymmetric-split.md - editorial-manifesto.md - kinetic-type.md - ... - feature/ - bento-grid.md - sticky-scroll-stack.md - zig-zag.md - ... - social-proof/ - pricing/ - cta/ - footer/ - navigation/ - portfolio/ - transition/ -``` - -### 12.B Required Frontmatter -```yaml ---- -name: asymmetric-split-hero -category: hero -dial_compatibility: - variance: [6, 10] - motion: [3, 10] - density: [2, 5] -when_to_use: "Landing pages with one strong asset and one strong message. Default hero for SaaS, agency, premium consumer." -not_for: "Editorial / manifesto launches where the message IS the design." -stack: ["react", "next", "tailwind", "motion"] ---- -``` - -### 12.C Required Body Sections -1. **Visual sketch** - short ASCII or description of the layout. -2. **Props API** - the component's interface. -3. **Code sketch** - minimal working implementation (Server Component default, Client island for motion). -4. **Mobile fallback** - explicit collapse rules for `< 768px`. -5. **Motion variants** - one variant per `MOTION_INTENSITY` band (1-3, 4-7, 8-10). Reduced-motion fallback explicit. -6. **Dark-mode notes** - token strategy specific to this block. -7. **Anti-patterns** - common ways this block goes wrong. -8. **References** - links to real examples in production. - -### 12.D Block-Library Discipline -* One block per file. No multi-block files. -* Every block must work standalone (drop it into a page, it renders). -* Every block must pass the Pre-Flight Check (Section 14). -* Blocks that depend on a design system from Section 2.A live under `blocks//--.md` (e.g. `feature/bento-grid--material.md`). - ---- - ## 13. OUT OF SCOPE This skill is NOT for: @@ -912,21 +811,21 @@ If the brief is one of the above, **say so explicitly**, point to the right tool Run this matrix before outputting code. This is the last filter. -**THIS IS NOT OPTIONAL. Run every box. If any box fails, the output is not done.** +Run through this list before delivering; fix what applies to the brief. - [ ] **Brief inference** declared (Section 0.B one-liner)? - [ ] **Dial values** explicit and reasoned from the brief, not silently using baseline? - [ ] **Design system** chosen from Section 2 if applicable, or aesthetic labeled honestly? - [ ] **Redesign mode** detected and audit performed (if applicable, Section 11)? -- [ ] **ZERO em-dashes (`—`) anywhere on the page.** Headlines, eyebrows, pills, body, quotes, attribution, captions, buttons, alt text. Zero. (Section 9.G - non-negotiable.) +- [ ] No em/en-dashes in visible copy (Section 9.G)? - [ ] **Page Theme Lock**: ONE theme (light, dark, or auto) for the whole page. No section flips to inverted mode mid-page (Section 4.11)? - [ ] **Color Consistency Lock**: one accent color used identically across all sections (Section 4.2)? - [ ] **Shape Consistency Lock**: one corner-radius system applied consistently (Section 4.4)? - [ ] **Button Contrast Check**: every CTA text is readable against its background (no white-on-white, WCAG AA 4.5:1)? - [ ] **CTA Button Wrap**: no CTA label wraps to 2+ lines at desktop? - [ ] **Form Contrast Check**: form inputs, placeholders, focus rings, labels all pass WCAG AA against the section background? -- [ ] **Serif discipline**: if a serif is used, it is NOT Fraunces or Instrument_Serif (or it is, with explicit brand justification)? Different serif from your previous project? -- [ ] **Premium-consumer palette check**: if the brief is premium-consumer (cookware / wellness / artisan / luxury), the palette is NOT the AI-default beige+brass+oxblood+espresso family? Different family from your previous premium-consumer project? +- [ ] **Serif discipline**: if a serif is used, there is a one-line brand reason for it (Section 4.1)? +- [ ] **Premium-consumer palette**: chosen from the brand, not the category default (Section 4.2)? - [ ] **Italic descender clearance**: every italic word with `y g j p q` has `leading-[1.1]` min + `pb-1` reserve? - [ ] **Hero fits the viewport**: headline ≤ 2 lines, subtext ≤ 20 words AND ≤ 4 lines, CTA visible without scroll, font scale planned around image? - [ ] **Hero top padding**: max `pt-24` at desktop, hero content does not float halfway down the viewport? @@ -971,14 +870,11 @@ Run this matrix before outputting code. This is the last filter. - [ ] **`useEffect` animations** have strict cleanup functions? - [ ] **Empty / loading / error** states provided? - [ ] **Cards omitted** in favor of spacing where possible? -- [ ] **Icons** from an allowed library only (Phosphor / HugeIcons / Radix / Tabler), no hand-rolled SVG paths? - [ ] **Motion** isolated in client-leaf components with `'use client'` at the top, memoized? - [ ] **No AI Tells** from Section 9 (Inter as default, AI-purple, three-equal cards, Jane Doe, Acme, "Quietly in use at")? - [ ] **Core Web Vitals** plausibly hit (LCP < 2.5s, INP < 200ms, CLS < 0.1)? - [ ] **One design system** per project (no Material + shadcn mixed)? -If a single checkbox cannot be honestly ticked, the page is not done. Fix it before delivering. - --- # APPENDICES - Real Source-Backed Reference Material diff --git a/.agents/skills/emcn-design-review/SKILL.md b/.agents/skills/emcn-design-review/SKILL.md index 09a9932d4b1..2f14b66c83c 100644 --- a/.agents/skills/emcn-design-review/SKILL.md +++ b/.agents/skills/emcn-design-review/SKILL.md @@ -27,9 +27,8 @@ This codebase uses **emcn**, a custom component library built on Radix UI primit ## Imports -- Import from `@/components/emcn` barrel, never subpaths +- Components, `cn`, and tokens from the `@sim/emcn` barrel, never component subpaths - Icons from `@sim/emcn/icons` -- Use `cn` from `@/lib/core/utils/cn` for conditional classes ## Design Tokens @@ -37,7 +36,7 @@ Use CSS variable pattern (`text-[var(--text-primary)]`), never Tailwind semantic **Text**: `--text-primary`, `--text-secondary`, `--text-tertiary`, `--text-muted`, `--text-body` (canonical value text), `--text-icon`, `--text-placeholder`, `--text-subtle`, `--text-inverse`, `--text-error` **Surfaces**: `--bg`, `--surface-1` through `--surface-7`, `--surface-hover`, `--surface-active` -**Borders**: `--border`, `--border-1`, `--border-muted` +**Borders**: `--border` (`--border-1`/`--border-muted` are legacy aliases resolving to it — flag new uses) **Brand/accent**: `--brand-secondary`, `--brand-accent` **Z-Index**: `--z-dropdown` (100), `--z-toast` (150), `--z-modal` (200), `--z-popover` (300), `--z-tooltip` (400), `--z-takeover` (500), `--z-shell-gate` (600) **Shadows**: `shadow-subtle`, `shadow-medium`, `shadow-overlay`, `shadow-card` @@ -62,7 +61,7 @@ Intent-to-variant mapping (read the actual `buttonVariants` in `packages/emcn/sr ## Toast -`toast.success()`, `toast.error()`, `toast()` from `@/components/emcn`. Never custom notification UI. +`toast.success()`, `toast.error()`, `toast()` from `@sim/emcn`. Never custom notification UI. ## Badges diff --git a/.agents/skills/emil-design-eng/SKILL.md b/.agents/skills/emil-design-eng/SKILL.md index b919c161db2..05a5b4991e3 100644 --- a/.agents/skills/emil-design-eng/SKILL.md +++ b/.agents/skills/emil-design-eng/SKILL.md @@ -6,14 +6,6 @@ description: This skill encodes Emil Kowalski's philosophy on UI polish, compone # Design Engineering -## Initial Response - -When this skill is first invoked without a specific question, respond only with: - -> I'm ready to help you build interfaces that feel right, my knowledge comes from Emil Kowalski's design engineering philosophy. If you want to dive even deeper, check out Emil’s course: [animations.dev](https://animations.dev/). - -Do not provide any other information until the user asks a question. - You are a design engineer with the craft sensibility. You build interfaces where every detail compounds into something that feels right. You understand that in a world where everyone's software is good enough, taste is the differentiator. ## Core Philosophy @@ -36,9 +28,9 @@ Every decision below exists because the aggregate of invisible correctness creat People select tools based on the overall experience, not just functionality. Good defaults and good animations are real differentiators. Beauty is underutilized in software. Use it as leverage to stand out. -## Review Format (Required) +## Review Format -When reviewing UI code, you MUST use a markdown table with Before/After columns. Do NOT use a list with "Before:" and "After:" on separate lines. Always output an actual markdown table like this: +Present review findings as one markdown table with `Before | After | Why` columns, one row per issue: | Before | After | Why | | --- | --- | --- | @@ -48,18 +40,6 @@ When reviewing UI code, you MUST use a markdown table with Before/After columns. | No `:active` state on button | `transform: scale(0.97)` on `:active` | Buttons must feel responsive to press | | `transform-origin: center` on popover | `transform-origin: var(--radix-popover-content-transform-origin)` | Popovers should scale from their trigger (not modals — modals stay centered) | -Wrong format (never do this): - -``` -Before: transition: all 300ms -After: transition: transform 200ms ease-out -──────────────────────────── -Before: scale(0) -After: scale(0.95) -``` - -Correct format: A single markdown table with | Before | After | Why | columns, one row per issue found. The "Why" column briefly explains the reasoning. - ## The Animation Decision Framework Before writing any animation code, answer these questions in order: diff --git a/.agents/skills/make-interfaces-feel-better/SKILL.md b/.agents/skills/make-interfaces-feel-better/SKILL.md index 41a30dc1263..38e81ff26c4 100644 --- a/.agents/skills/make-interfaces-feel-better/SKILL.md +++ b/.agents/skills/make-interfaces-feel-better/SKILL.md @@ -29,7 +29,7 @@ When geometric centering looks off, align optically. Buttons with icons, play tr ### 3. Shadows Over Borders -Layer multiple transparent `box-shadow` values for natural depth. Shadows adapt to any background; solid borders don't. +For elevation (dropdowns, modals, cards) use the `shadow-subtle`/`shadow-medium`/`shadow-overlay`/`shadow-card` tokens. In this repo neutral edges and dividers stay as `--border` borders (`.claude/rules/sim-styling.md`, Line weight) — do not swap them for `0 0 0 1px` shadow rings. ### 4. Interruptible Animations @@ -45,7 +45,7 @@ Use a small fixed `translateY` instead of full height. Exits should be softer th ### 7. Contextual Icon Animations -Animate icons with `opacity`, `scale`, and `blur` instead of toggling visibility. Use exactly these values: scale from `0.25` to `1`, opacity from `0` to `1`, blur from `4px` to `0px`. If the project has `motion` or `framer-motion` in `package.json`, use `transition: { type: "spring", duration: 0.3, bounce: 0 }` — bounce must always be `0`. If no motion library is installed, keep both icons in the DOM (one absolute-positioned) and cross-fade with CSS transitions using `cubic-bezier(0.2, 0, 0, 1)` — this gives both enter and exit animations without any dependency. +Animate contextual icons with opacity, scale, and blur instead of toggling visibility; see animations.md for the Motion and CSS cross-fade patterns. ### 8. Font Smoothing @@ -61,11 +61,11 @@ Use `text-wrap: balance` on headings. Use `text-wrap: pretty` for body text to a ### 11. Image Outlines -Add a subtle `1px` outline with low opacity to images for consistent depth. The color must be pure black in light mode (`rgba(0, 0, 0, 0.1)`) and pure white in dark mode (`rgba(255, 255, 255, 0.1)`) — never a near-black like slate, zinc, or any tinted neutral. A tinted outline picks up the surface color underneath it and reads as dirt on the image edge. +Add a subtle 1px low-opacity outline to images (`outline-black/10` light, `outline-white/10` dark); see surfaces.md. ### 12. Scale on Press -A subtle `scale(0.96)` on click gives buttons tactile feedback. Always use `0.96`. Never use a value smaller than `0.95` — anything below feels exaggerated. Add a `static` prop to disable it when motion would be distracting. +A subtle scale-down (about 0.96-0.97) on press gives tactile feedback. In this repo a press affordance belongs in the emcn `Button`/`Chip` chrome (`packages/emcn`), not in consumer classes — neither component implements one today, so propose it there rather than adding per-call-site transforms. ### 13. Skip Animation on Page Load @@ -89,7 +89,7 @@ Interactive elements need at least 40×40px hit area. Extend with a pseudo-eleme | --- | --- | | Same border radius on parent and child | Calculate `outerRadius = innerRadius + padding` | | Icons look off-center | Adjust optically with padding or fix SVG directly | -| Hard borders between sections | Use layered `box-shadow` with transparency | +| Hard borders between sections | In this repo, the `--border` hairline token; elsewhere, layered `box-shadow` with transparency | | Jarring enter/exit animations | Split, stagger, and keep exits subtle | | Numbers cause layout shift | Apply `tabular-nums` | | Heavy text on macOS | Apply `antialiased` to root | @@ -100,7 +100,7 @@ Interactive elements need at least 40×40px hit area. Extend with a pseudo-eleme ## Review Output Format -Always present changes as a markdown table with **Before** and **After** columns. Include every change you made — not just a subset. Never list findings as separate "Before:" / "After:" lines outside of a table. Group changes by principle using a heading above each table, and keep each row focused on a single diff so the reader can scan the whole list quickly. +Present changes as markdown tables with **Before** and **After** columns, one table per principle with a heading above it, one diff per row, and cite file and property when the snippet is not self-explanatory. ### Example diff --git a/.agents/skills/make-interfaces-feel-better/animations.md b/.agents/skills/make-interfaces-feel-better/animations.md index e0515e02dcf..2c4c8291027 100644 --- a/.agents/skills/make-interfaces-feel-better/animations.md +++ b/.agents/skills/make-interfaces-feel-better/animations.md @@ -272,15 +272,11 @@ The non-absolute icon (InactiveIcon) defines the layout size. The absolute icon | Icons in contextual toolbars | Icons that are always visible | | Loading/success state indicators | Icon labels (text next to icon) | -**Important:** Always use exactly these values for contextual icon animations — do not deviate: -- `scale`: `0.25` → `1` (never use `0.5` or `0.6`) -- `opacity`: `0` → `1` -- `filter`: `"blur(4px)"` → `"blur(0px)"` -- `transition`: `{ type: "spring", duration: 0.3, bounce: 0 }` — **bounce must always be `0`**, never `0.1` or any other value +Default values: scale 0.25→1, opacity 0→1, blur 4px→0, `{ type: "spring", duration: 0.3, bounce: 0 }`. ## Scale on Press -A subtle scale-down on click gives buttons tactile feedback. Always use `scale(0.96)`. Never use a value smaller than `0.95` — anything below feels exaggerated. Use CSS transitions for interruptibility — if the user releases mid-press, it should smoothly return. +A subtle scale-down on click (about 0.96-0.97) gives buttons tactile feedback. Use CSS transitions for interruptibility — if the user releases mid-press, it should smoothly return. Not every button needs this. Add a `static` prop to your button component that disables the scale effect when the motion would be distracting. diff --git a/.agents/skills/make-interfaces-feel-better/surfaces.md b/.agents/skills/make-interfaces-feel-better/surfaces.md index 180de509a48..1f3cd6b4a5c 100644 --- a/.agents/skills/make-interfaces-feel-better/surfaces.md +++ b/.agents/skills/make-interfaces-feel-better/surfaces.md @@ -116,6 +116,8 @@ Some icons have uneven visual weight. The best fix is adjusting the SVG directly ## Shadows Instead of Borders +> In this repo, use the `shadow-subtle`/`shadow-medium`/`shadow-overlay`/`shadow-card` tokens for elevation and keep neutral edges as `--border` borders (`.claude/rules/sim-styling.md`, Line weight); do not replace them with `0 0 0 1px` shadow rings. The pattern below is for projects without that token system. + For **buttons, cards, and containers** that use a border for depth or elevation, prefer replacing it with a subtle `box-shadow`. Shadows adapt to any background since they use transparency; solid borders don't. This also helps when using images or multiple colors as backgrounds — solid border colors don't work well on backgrounds other than the ones they were designed for. **Do not apply this to dividers** (`border-b`, `border-t`, side borders) or any border whose purpose is layout separation rather than element depth. Those should stay as borders. @@ -179,12 +181,11 @@ Apply the variable and add `transition-[box-shadow]` for a smooth hover: Add a subtle `1px` outline with low opacity to images. This creates consistent depth, especially in design systems where other elements use borders or shadows. -### Color rules (non-negotiable) +### Color -- **Light mode**: pure black — `rgba(0, 0, 0, 0.1)`. Exact values: R=0, G=0, B=0. -- **Dark mode**: pure white — `rgba(255, 255, 255, 0.1)`. Exact values: R=255, G=255, B=255. -- Never use a near-black or near-white from the project palette (e.g. slate-900, zinc-900, `#0a0a0a`, `#111827`, `#f5f5f7`). Tinted outlines pick up the surrounding surface color and read as dirt on the image edge. -- Never match the outline to the project's accent or ink color. The outline is a neutral separator, not a themed element. +- **Light mode**: pure black — `rgba(0, 0, 0, 0.1)`. +- **Dark mode**: pure white — `rgba(255, 255, 255, 0.1)`. +- Tinted neutrals (slate-900, zinc-900, `#0a0a0a`, `#f5f5f7`) and accent/ink colors pick up the surrounding surface color and read as dirt on the image edge; the outline is a neutral separator, not a themed element. ### Light Mode diff --git a/.agents/skills/memory-load-check/SKILL.md b/.agents/skills/memory-load-check/SKILL.md index 340f6b9757c..157e7a89db8 100644 --- a/.agents/skills/memory-load-check/SKILL.md +++ b/.agents/skills/memory-load-check/SKILL.md @@ -31,7 +31,7 @@ Read these when doing a deeper pass: - `chunkedBatchDelete`: bounded SELECT -> optional side effect -> DELETE loop. - `batchDeleteByWorkspaceAndTimestamp`: common workspace/timestamp cleanup wrapper. - `selectRowsByIdChunks`: chunks large ID sets and enforces an overall row cap. - - `chunkArray`: use only after the input set itself is already bounded. +- `chunkArray` from `@sim/utils/helpers`: use only after the input set itself is already bounded. - `apps/sim/lib/core/utils/stream-limits.ts` - `PayloadSizeLimitError` - `assertKnownSizeWithinLimit` @@ -45,7 +45,7 @@ Read these when doing a deeper pass: - dispatch concrete chunks (`workspaceIds`, retention, label) instead of one giant scope - prefer Trigger.dev queue/concurrency keys when available - execute inline fallback chunks sequentially, not with unbounded `Promise.all` -- File parse route pattern in `apps/sim/app/api/files/parse/route.ts` +- File parse pattern in `apps/sim/lib/internal/file/parser.ts` and `apps/sim/lib/uploads/contexts/workspace/fetch-external-url.ts` - cap downloads and parsed output separately - preserve partial results when a later item exceeds the cap - never read untrusted response bodies without a byte cap diff --git a/.agents/skills/migrate-application-operation/SKILL.md b/.agents/skills/migrate-application-operation/SKILL.md index 2786721895c..d8be0eaf32c 100644 --- a/.agents/skills/migrate-application-operation/SKILL.md +++ b/.agents/skills/migrate-application-operation/SKILL.md @@ -156,6 +156,7 @@ rename: defineWorkspaceOperation({ id: 'widgets.rename', minimumRole: 'write', workspaceApiKey: 'allow', + capability: 'widgets.use', principalKinds: ['session', 'personal_api_key', 'workspace_api_key', 'delegated'], delegatedServices: ['copilot'], }) @@ -165,6 +166,8 @@ Do not create internal-, public-, or Copilot-specific versions of the same seman Choose principal kinds from actual behavior. Do not accept every principal merely because the use case is shared. Workspace API keys have a write ceiling and cannot satisfy admin operations. The operation definition must fail fast when its role, workspace-key policy, and principal kinds disagree. +`capability` is required — name the permission-group capability that governs the operation, or `'none'` with a `// permission-group-exempt: ` comment directly above it. `defineWorkspaceOperation` throws at definition time when it is absent. See `add-permission-group-item`. + Route declarations, tool adapters, and use cases must use the same literal operation. Runtime operation selection is permitted only from a trusted, code-defined registry. Never accept an operation ID or permission tag from an HTTP body, model argument, or other untrusted input. ### Unified selector execution is one operation @@ -237,7 +240,7 @@ Keep the route module declarative. If several internal routes repeat authenticat ## Adapt public or versioned APIs -Use the appropriate public/versioned route builder, such as `defineV2JsonRoute`, with API-key authentication, explicit semantic operation and rate policy, external error projection, input mapping, application use case, and an external presenter. V2 rollout admission is centralized by the builder; do not invent a route-local rollout policy. +Use the appropriate public/versioned route builder, such as `defineV2JsonRoute`, with API-key authentication, explicit semantic operation and rate policy, external error projection, input mapping, application use case, and an external presenter. Authentication and HTTP formatting may differ from internal APIs; authorization and business behavior must not. Rate-limit using the credential or principal subject, never a billed owner. Resolve billing attribution only for billing, quota, or legacy required-user fields. @@ -320,7 +323,7 @@ Add focused tests for every migrated surface and principal kind allowed by the o - Operation registry: role/workspace-key/principal-kind/delegated-service consistency and fail-fast rejection of invalid definitions. - Repository: canonical active lookup, workspace-predicated writes, archived resources, authoritative affected rows, and database error propagation. - Internal API: authentication before parsing, exact contract, typed errors, and surface analytics only after success. -- Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. +- Public API: personal and workspace keys, rate behavior, concealment, exact external envelope, and rate headers. - Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. - Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. - Compatibility characterization: legacy normalization, exact response/redirect/cookie behavior, concealment, error subclass precedence, and branch-specific output. diff --git a/.agents/skills/react-query-best-practices/SKILL.md b/.agents/skills/react-query-best-practices/SKILL.md index 2bce17ad86f..99f96b259ce 100644 --- a/.agents/skills/react-query-best-practices/SKILL.md +++ b/.agents/skills/react-query-best-practices/SKILL.md @@ -25,15 +25,9 @@ Read these before analyzing: ## Rules to enforce -### Query key factories -- Every file in `hooks/queries/` must have a hierarchical key factory with an `all` root key -- Keys must include intermediate plural keys (`lists`, `details`) for prefix invalidation -- Key factories are colocated with their query hooks, not in a global keys file - -### Query hooks -- Every `queryFn` must forward `signal` for request cancellation -- Every query must have an explicit `staleTime` (default 0 is almost never correct), assigned from a named exported constant — never an inline numeric literal. A server-side prefetch hydrating the same query key must import and reuse that constant instead of restating the number -- `keepPreviousData` / `placeholderData` only on variable-key queries (where params change), never on static keys +### Query keys and hooks +Enforce CLAUDE.md "React Query" and `.claude/rules/sim-queries.md` (key factory with `all` + plural prefixes, `signal` forwarding, named `staleTime` constants reused by prefetches, `keepPreviousData` only on variable keys, `requestJson` boundary). Additionally: +- Key factories live next to their hooks — except a factory, standalone fetcher/mapper, or `staleTime` constant that a server module (a `prefetch.ts`, route, block, trigger) imports, which must live in a non-`'use client'` module under `hooks/queries/utils/` per `.claude/rules/sim-queries.md` (a `'use client'` export called from the server crashes SSR) - Use `enabled` to prevent queries from running without required params - Warm data for hover/focus intent with `queryClient.prefetchQuery` and shared `queryOptions`; never temporarily enable a mounted hidden observer, which can remain active after focus restoration and refetch data for closed UI - When gating a query by view or modal state, move every consumer to the active query too: imperative refresh/pagination, loading and error feedback, and data-derived controls must never read a disabled query or placeholder data from a previous key @@ -43,16 +37,14 @@ Read these before analyzing: - Server prefetches must call the authorized use case, apply the route presenter/response schema, and reuse the client's exact key, mapper, and stale time. Keep all fallible auth/read/parse work inside `queryFn` so an optional warm cannot fail the page, and never bypass a route that redacts fields. ### Mutations -- Use `onSettled` (not `onSuccess`) for cache reconciliation — it fires on both success and error -- For optimistic updates: save previous data in `onMutate`, roll back in `onError` -- Use targeted invalidation (`entityKeys.lists()`) not broad (`entityKeys.all`) when possible -- Don't include mutation objects in `useCallback` deps — `.mutate()` is stable +Enforce CLAUDE.md "Mutation Hooks" (targeted invalidation, `onMutate`/`onError` rollback, mutation objects out of `useCallback` deps). Additionally: +- Plain mutations invalidate in `onSuccess`; optimistic mutations reconcile in `onSettled` (fires on success and error) with rollback in `onError` — see `.claude/rules/sim-queries.md` "Mutation Hook" / "Optimistic Updates" ### Server state ownership - Never copy query data into useState. Use query data directly in components. - Never copy query data into Zustand stores (exception: mutation callbacks that coordinate cross-store state like temp ID replacement) -- The query cache is not a local state manager — `setQueryData` is for optimistic updates only -- Forms are the one deliberate exception: once query data exists, initialize a keyed form subtree from it with lazy state initializers. Do not synchronize query data into draft state with an Effect; key the form by resource identity so switching resources resets every draft/modal/upload field together. Keep independent queries in the outer wrapper so they still start in parallel. +- The query cache is not a local state manager — `setQueryData` is for optimistic updates and the server-prefetch seeding case in `.claude/rules/sim-queries.md` "Server prefetching", nothing else +- Forms are the one deliberate exception (a keyed form child initialized lazily from loaded query data) — the pattern is owned by `/you-might-not-need-an-effect` "Query-backed forms"; do not duplicate its finding ## Steps diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index f9e8aef93bf..bb6ccff5bb8 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -13,25 +13,25 @@ You help ship code by creating commits, pushing to the remote branch, and creati When the user runs `/ship`: 1. **Check git status** - See what files have changed -2. **Sync check**: `git fetch origin staging && git log --oneline origin/staging..HEAD`. Read the actual commit list, not just how many there are — it must show ONLY commits you can attribute to this session (recognizable subjects/SHAs). A worktree/branch can silently be cut from a stale local `staging`, dragging in unrelated commits; a corrupted branch's inflated commit *count* can coincidentally match a later check even when the *commits* are wrong, so always compare content, never just a number. +2. **Sync check**: `git fetch origin staging && git log --oneline origin/staging..HEAD`. The list must contain ONLY commits you can attribute to this session (recognizable subjects/SHAs) — a worktree/branch cut from a stale local `staging` silently drags in unrelated commits. - If it shows commits you don't recognize, fix it now, **before** staging/committing any new work (step 7 hasn't run yet): - If the working tree has uncommitted changes, stash them first — `git stash push -u -m ship-sync-fix` — so the rebase below isn't blocked by dirty state. Restore with `git stash pop` once the branch is fixed. - Try `git rebase origin/staging` first. - **A rebase finishing without conflicts does NOT by itself mean the branch is clean** — it can replay stray commits onto the new base with no conflict at all. After the rebase (clean or not), re-run `git log --oneline origin/staging..HEAD` and re-check the commit list against what you recognize. - - If the rebase conflicted on commits you don't recognize, OR it finished cleanly but the re-checked log still shows commits you don't recognize, abandon that result (`git rebase --abort` if still mid-rebase) and rebuild instead, in this exact order: - 1. **While still on ``**, identify the SHA(s) to preserve — **not** the whole range. `git log --oneline --reverse origin/staging..` lists everything ahead of `origin/staging`, but in exactly this scenario that range also contains the unrecognized/stray commits you're trying to leave behind — blindly cherry-picking the full range recreates the same polluted branch. Read the list and write down only the SHA(s) you recognize as your own session's work (e.g. `abc1234 def5678`); do this *before* touching any temp branch, since once you check out `ship-sync-tmp` at `origin/staging` in step 4, `HEAD` no longer contains these commits and the same lookup at that point returns nothing. - 2. `git checkout ` — harmless no-op if you're already there, but required if an earlier interrupted attempt left you sitting on `ship-sync-tmp`: git refuses to delete the branch you're currently on, so deleting it before switching away silently fails and blocks the rest of the rebuild. - 3. Delete any leftover from an earlier attempt: `git branch -D ship-sync-tmp 2>/dev/null || true` — always succeeds, including when there's nothing to delete (a first attempt), so it never blocks the rest of the rebuild on its own exit code. - 4. `git checkout -b ship-sync-tmp origin/staging`. - 5. `git cherry-pick` the SHAs captured in step 1, **in that oldest-first order** — cherry-picking more than one session commit out of order can fail or produce the wrong history. Resolve conflicts. - 6. `git branch -f HEAD`, `git checkout `, and delete `ship-sync-tmp` (`git branch -D ship-sync-tmp`). + - If the rebase conflicted on unrecognized commits, OR finished cleanly but the log still shows them, abandon it (`git rebase --abort` if mid-rebase) and rebuild, in this exact order: + 1. Still on ``, list `git log --oneline --reverse origin/staging..` and write down ONLY the SHA(s) that are this session's work — the range also contains the stray commits, so cherry-picking the whole range recreates the polluted branch. Capture them now; after step 4 they are no longer in `HEAD`. + 2. `git checkout ` (required if an interrupted attempt left you on `ship-sync-tmp`) + 3. `git branch -D ship-sync-tmp 2>/dev/null || true` + 4. `git checkout -b ship-sync-tmp origin/staging` + 5. `git cherry-pick` the captured SHAs, oldest-first. Resolve conflicts. + 6. `git branch -f HEAD && git checkout && git branch -D ship-sync-tmp` - Re-verify with `git log --oneline origin/staging..HEAD` — it must list only commits you recognize before you proceed to committing new work. 3. **Generate a commit message** following this format: `type(scope): description` - Types: `fix`, `feat`, `improvement`, `chore` - Scope: short identifier (e.g., `undo-redo`, `api`, `ui`) - Keep it concise 4. **Run the cleanup pass** — only if the diff modifies UI code (any `.tsx` file, or anything under `apps/sim/components/`, `apps/sim/hooks/`, or `apps/sim/stores/`): `/cleanup` - - The six code-quality skills (effects, memo, callbacks, state, React Query, emcn) only apply to React code, so skip this step entirely when no UI was touched. When it runs, it applies fixes so they land in this commit. + - `/cleanup` fans out the React/UI passes (effects, memo, callbacks, state, React Query, emcn, url-state) plus the comment pass; skip it when no UI was touched. When it runs, it applies fixes so they land in this commit. 5. **Run migration safety** — only if the diff touches `packages/db/migrations/**` or `packages/db/schema.ts`: - Run `/db-migrate` to review the migration for zero-downtime safety (expand/contract phasing, backward-compatibility with the deployed app version). - `bun run check:migrations origin/staging` must pass (staging is the PR base). Do not silence a flagged statement with a `-- migration-safe:` annotation unless `/db-migrate` confirmed the old code no longer depends on it; otherwise split the destructive change into a later deploy. @@ -45,10 +45,8 @@ When the user runs `/ship`: done wait # any non-zero line is a FAILED generator — read /tmp/ship-gen-.log and fix before shipping; - # a silently-failed generate leaves a stale artifact that Phase B / CI then rejects. - # The `exit 1` makes this block itself exit non-zero on failure, so anything gating on the - # command's status (an agent, or a wrapping script) actually stops — do NOT collapse it to - # `grep … && echo ❌ || echo ✅`, which always exits 0 and silently lets ship continue. + # a silently-failed generate leaves a stale artifact that Phase B / CI then rejects. Keep the + # `exit 1`: it is what makes the block's own status non-zero so a caller actually stops. if grep -vE '^0 ' /tmp/ship-gen-results; then echo "❌ generator(s) failed — do not ship"; exit 1; fi echo "✅ artifacts regenerated" ``` @@ -66,8 +64,7 @@ When the user runs `/ship`: exit 1 } # Runs every audit CI runs, concurrently, and replays the output of any that fail. - # Do not hand-list the audits here: the list is derived in scripts/run-audits.ts, and the - # copy that used to live in this file had already drifted five audits behind package.json. + # The audit list is derived in scripts/run-audits.ts — do not hand-list audits here. bun run check:audits || { echo "❌ audit(s) failed — do not ship"; exit 1; } ``` If Phase A regenerated a file, its matching `:check` in Phase B now passes trivially — that parity is the point. Do not ship with any generator or audit failing; fix the cause (never silence it) and re-run. `check:migrations` and `type-check` are covered by steps 5 and CI respectively and are not repeated here. diff --git a/.agents/skills/tool-registry-boundary/SKILL.md b/.agents/skills/tool-registry-boundary/SKILL.md index ca22f8c856d..af54a09ad35 100644 --- a/.agents/skills/tool-registry-boundary/SKILL.md +++ b/.agents/skills/tool-registry-boundary/SKILL.md @@ -5,18 +5,18 @@ description: Keep the executable tool registry out of client-reachable module gr # Tool Registry Boundary Skill -You keep the 4,300-tool executable registry out of module graphs that don't execute tools. +You keep the 5,000+-tool executable registry out of module graphs that don't execute tools. ## The rule > Client-reachable code reads tool **metadata**. Only code that actually executes a tool imports the **registry**. -`@/tools/registry` is a ~9,000-line barrel importing every tool. External `ToolConfig` entries mix +`@/tools/registry` is a 10,000+-line barrel importing every tool. External `ToolConfig` entries mix plain data (`params`, `outputs`, `name`) with request/response closures, while `InternalToolConfig` entries contain semantic input projection and load their server implementation through `lib/internal/tool-operations/registry.server.ts`. Request closures can still reach SDK clients, API helpers, and parsers, which is what makes the executable barrel expensive: reaching it -costs ~4,700 additional modules. +costs roughly 4,700 additional modules (measured; re-measure with `--verbose`). `getTool()` returns the whole `ToolConfig`, so a single `getTool` import anywhere in a client-reachable file drags all of it in. @@ -33,7 +33,7 @@ costs ~4,700 additional modules. Three modules, cheapest first. Ids are their own artifact because resolution and existence checks need only the key set; outputs are their own because they are the larger half of the data with a single consumer. `@/tools/metadata` and `@/tools/metadata-outputs` both resolve ids through `@/tools/tool-ids`, which is what keeps them independent of each other — do not "helpfully" re-export one from another, or every caller pays for all three. -All lookups guard with `Object.hasOwn`. `JSON.parse` yields an object with the normal prototype, so a bare bracket lookup returns inherited members: `getToolMetadata('constructor')` returned a *function* typed as tool metadata before that was fixed. +All lookups guard with `Object.hasOwn`. `JSON.parse` yields an object with the normal prototype, so a bare bracket lookup returns inherited members: a bare bracket lookup would answer `getToolMetadata('constructor')` with a *function* typed as tool metadata. ## The generated artifacts @@ -48,10 +48,10 @@ Never hand-edit them. If you add a tool or change a tool's `params`/`outputs`, r Three non-obvious properties, each of which was measured and is easy to undo by accident: -- **The data is a JSON string parsed at runtime, not an imported `.json` and not an object literal.** With `resolveJsonModule` (which this repo enables), a `.json` import makes TypeScript infer a literal type for all 4,300+ entries and takes `tsc --noEmit` from **12.6s to 8m07s** — a 38x regression. An ambient `declare module` does *not* short-circuit it, and an object literal costs the same. A single string literal is one cheap token for both the compiler and the bundler, and `JSON.parse` beats evaluating the equivalent literal at runtime. Do not "clean this up" into a `.json` import. +- **The data is a JSON string parsed at runtime, not an imported `.json` and not an object literal.** With `resolveJsonModule` (which this repo enables), a `.json` import makes TypeScript infer a literal type for all 5,000+ entries and takes `tsc --noEmit` from **12.6s to 8m07s** — a 38x regression. An ambient `declare module` does *not* short-circuit it, and an object literal costs the same. A single string literal is one cheap token for both the compiler and the bundler, and `JSON.parse` beats evaluating the equivalent literal at runtime. Do not "clean this up" into a `.json` import. - **The generator refuses to emit function values.** If you add a field to `METADATA_FIELDS` that contains a closure, generation fails loudly rather than shipping executable config to the client. `hosting` and `schemaEnrichment` are excluded for exactly this reason (`hosting.enabled`, `pricing`, and `enrichSchema` are functions) — they are server-only. - **Empty param entries are stripped.** The registry contains one (`stt_deepgram_v2`), which crashes callers that read `param.type` while iterating. -- **Lookups resolve versions.** `getTool` maps an unversioned name onto the newest version, and 246 tools are versioned. A plain key lookup would silently report them missing — a quiet correctness bug, not a crash. `resolveToolId` reproduces that against the id set and is differentially tested against the original. +- **Lookups resolve versions.** `getTool` maps an unversioned name onto the newest version, and a few hundred tools are versioned. A plain key lookup would silently report them missing — a quiet correctness bug, not a crash. `resolveToolId` reproduces that against the id set and is differentially tested against the original. ## Testing code that reads tool metadata diff --git a/.agents/skills/v2-api-conventions/SKILL.md b/.agents/skills/v2-api-conventions/SKILL.md index 44d0b298e6c..a7174f61e30 100644 --- a/.agents/skills/v2-api-conventions/SKILL.md +++ b/.agents/skills/v2-api-conventions/SKILL.md @@ -31,7 +31,6 @@ Each was one line. The rules below are the generalisations. | Concern | File | |---|---| | Envelope + error codes + cursor codecs | `apps/sim/app/api/v2/lib/response.ts` | -| Rollout gate (`v2-api` flag) | `apps/sim/app/api/v2/lib/gate.ts` | | Cross-tenant concealment | `apps/sim/lib/api/server/routes/resource-concealment.ts` | | Route builder | `apps/sim/lib/api/server/routes/v2-json-route.ts` | | Contracts | `apps/sim/lib/api/contracts/v2/**` | @@ -51,9 +50,9 @@ A route built with `defineV2JsonRoute` gets this for free: its `present` returns |---|---|---| | 200 / 201 | — | Success. 201 only for a created resource. | | 400 | `BAD_REQUEST` | Contract validation. Carries field-level `details` from `serializeZodIssues`. | -| 401 | `UNAUTHORIZED` | No/!valid API key. **Runs before the rollout gate.** | +| 401 | `UNAUTHORIZED` | No/!valid API key. | | 403 | `FORBIDDEN` | Authenticated, same tenant, insufficient rights. Where the cause is one a caller can act on it is named in `details.code`, from the closed set in `lib/core/application/forbidden.ts` (e.g. `INSUFFICIENT_WORKSPACE_ROLE`, `PERSONAL_API_KEYS_DISABLED`). A few domain refusals still reach the wire without one. | -| 404 | `NOT_FOUND` | Not found, **and** cross-tenant concealment, **and** the rollout gate, **and** an unknown path. | +| 404 | `NOT_FOUND` | Not found, **and** cross-tenant concealment, **and** an unknown path. | | 409 | `CONFLICT` | Uniqueness/state conflict, human-readable message. | | 413 | `PAYLOAD_TOO_LARGE` | Body over the route's `maxBodyBytes` — **and** a collection the response must materialize that is over *its* ceiling. Fourteen bodyless `GET`/`DELETE` operations publish it for the folder-tree cap (`FolderCollectionLimitExceededError`) or the rendered-artifact cap. | | 429 | `RATE_LIMITED` | With `Retry-After` and `X-RateLimit-*`. | @@ -61,7 +60,7 @@ A route built with `defineV2JsonRoute` gets this for free: its `present` returns Two of these carry real design weight: -**404 is deliberately overloaded.** A workspace the caller cannot reach answers `404 "Workspace not found"`, never 403 — a 403 would confirm the resource exists. `createV2ResourceConcealmentPolicy` does this by mapping a cross-tenant authorization failure to `v2Error('NOT_FOUND', ...)`. The rollout gate answers the same way for the same reason (`gate.ts`: "an ungated caller cannot distinguish 'not in the rollout cohort' from 'no such endpoint'"), and so does the unknown-path catch-all at `app/api/v2/[[...segments]]/route.ts` — its body is byte-identical to the gate's on purpose. +**404 is deliberately overloaded.** A workspace the caller cannot reach answers `404 "Workspace not found"`, never 403 — a 403 would confirm the resource exists. `createV2ResourceConcealmentPolicy` does this by mapping a cross-tenant authorization failure to `v2Error('NOT_FOUND', ...)`. The unknown-path catch-all at `app/api/v2/[[...segments]]/route.ts` answers with the same body on purpose. **500 is never caller-reachable.** Any input a caller can send must be rejected at the contract boundary with a 400. If you can construct a query string or body that produces a 500, that is a bug in the contract, not something to wrap in a `try`/`catch`. `v2ErrorForOrchestration` also replaces the message on an unclassified failure with a generic one, so internal detail never leaks. A caller-reachable 500 has shipped three times — a fractional `limit` reaching `LIMIT 2.5`, a plain `HEAD` tripping the builder's method guard, and a keyset cursor's timestamp reaching `date_trunc` as an untyped placeholder — so treat "a well-formed request produced a 500" as the highest-severity class of defect on this surface. @@ -84,7 +83,7 @@ Use the shared sets in `contracts/v2/openapi/shared.ts` — `RESOURCE_ERRORS`, ` **HEAD is answered by the `GET` handler, not rejected.** Next aliases a missing `HEAD` export onto `GET` and drops the body when sending, so a route's `GET` legitimately runs with `request.method === 'HEAD'`. The builders' method guard accepts that pairing via `methodMatchesContract`; any other mismatch stays a hard error. Never hand-write a `HEAD` export to "fix" this. -**A `GET` with side effects must declare `headSafe: false`.** The aliasing above is only sound because RFC 9110 §9.2.1 defines `HEAD` as safe. A `GET` that writes a row or opens an outbound connection is not, and an uptime monitor or link checker walking the documented URL list would drive those effects on every probe — `GET /files/{fileId}` records a `FILE_DOWNLOADED` audit event, so a `HEAD` used to fabricate a download that never happened. Both the JSON and binary builders take `headSafe`: a route that sets it `false` still authenticates and rate-limits a `HEAD`, then answers `v2HeadNoEffect()` — a bodiless 200 — before parsing or executing. Nothing observable is lost, because `HEAD` carries no body either way. Audit this whenever a read acquires an audit projection or an outbound call. +**A `GET` with side effects must declare `headSafe: false`.** The aliasing above is only sound because RFC 9110 §9.2.1 defines `HEAD` as safe. A `GET` that writes a row or opens an outbound connection is not, and an uptime monitor or link checker walking the documented URL list would drive those effects on every probe — `GET /files/{fileId}` records a `FILE_DOWNLOADED` audit event, so without `headSafe: false` a `HEAD` would fabricate a download that never happened. Both the JSON and binary builders take `headSafe`: a route that sets it `false` still authenticates and rate-limits a `HEAD`, then answers `v2HeadNoEffect()` — a bodiless 200 — before parsing or executing. Nothing observable is lost, because `HEAD` carries no body either way. Audit this whenever a read acquires an audit projection or an outbound call. ## Rule 3 — a collection that returns `nextCursor` must accept `limit` + `cursor`, and must apply them @@ -103,9 +102,9 @@ Three cursor schemes exist. Two are the shared codecs in `response.ts`, both opa - **Keyset** (`readSortedCursor` in, `encodeSortedCursor` out) — the default. Requires the page to come from one ordered SQL read. The sort AND the filters are stamped into the cursor and re-checked on replay, so changing `sortBy` or any filter mid-pagination is a 400, not a silently skipped page. - **Offset** (`decodeOffsetCursor` / `encodeOffsetCursor`) — only when a keyset is impossible. Two lists qualify: `GET /skills` merges a static in-code registry with DB rows and re-sorts in JS, and `GET /knowledge/{id}/documents` sits on a limit/offset query. A bare offset replayed against a re-sorted or re-filtered sequence names a different row, which skips or repeats results. -Both take the same two stamps: `cursorSortKey(sortBy, sortOrder)` for the ordering, and `cursorFilterScope({ ... })` for every param that filters the sequence. **`limit` is never a stamp** — it selects how much of the sequence to return, not what the sequence is, and binding it strands every cursor the moment a caller changes page size. Params that only shape the response body are out for the same reason. +Both take the same two stamps: `cursorSortKey(sortBy, sortOrder)` for the ordering, and `cursorScopeKey(cursorRoute(contract, pathParams), { ... })` for every param that filters the sequence — the route identity is the first argument, the filter parts the second. **`limit` is never a stamp** — it selects how much of the sequence to return, not what the sequence is, and binding it strands every cursor the moment a caller changes page size. Params that only shape the response body are out for the same reason. -The third is **per-domain**: a list whose read predates the shared codecs, or whose page boundary is not expressible as one, mints its own — a bare `encodeCursor({ version })` on `GET /workflows/{id}/versions` and `encodeCursor({ email })` on the workspace member list, the local codecs in `lib/audit-logs/query.ts`, `lib/logs/list-logs.ts`, and `lib/table/rows/cursor.ts`, and a usage-event id passed straight through by `GET /billing/logs`. Those tokens stay opaque and untouched, but a domain-minted cursor on a list a caller can re-filter is wrapped at the surface with `encodeScopedCursor(cursorFilterScope({...}), token)` and unwrapped with `readScopedCursor`, so it carries the same binding as the shared schemes. **A new list picks one of the two shared schemes.** Do not add a fourth. +The third is **per-domain**: a list whose read predates the shared codecs, or whose page boundary is not expressible as one, mints its own — a bare `encodeCursor({ version })` on `GET /workflows/{id}/versions` and `encodeCursor({ email })` on the workspace member list, the local codecs in `lib/audit-logs/query.ts`, `lib/logs/list-logs.ts`, and `lib/table/rows/cursor.ts`, and a usage-event id passed straight through by `GET /billing/logs`. Those tokens stay opaque and untouched, but a domain-minted cursor on a list a caller can re-filter is wrapped at the surface with `encodeScopedCursor(cursorScopeKey(cursorRoute(contract, pathParams), {...}), token)` and unwrapped with `readScopedCursor`, so it carries the same binding as the shared schemes. **A new list picks one of the two shared schemes.** Do not add a fourth. Every paged list's binding is declared in `lib/api/contracts/v2/__tests__/list-pagination.test.ts` and checked against what the contract actually accepts, in both directions. A new list, or a new filter on an existing one, fails that test until its binding is declared or the param is explicitly recorded as unable to change the sequence. @@ -115,15 +114,15 @@ Return `nextCursor: null` on the last page and only then. Never construct a curs **Ordering is `sortBy` + `sortOrder`, except where there is nothing to sort by.** Nearly every paged list takes the pair; `CURSOR_BINDINGS` in `contracts/v2/__tests__/list-pagination.test.ts` is the authoritative set. Exactly one — `GET /workflows/{workflowId}/runs` — has a single sortable column (start time), so there is no `sortBy` to pair with and the direction rides on a single `order` param; `sortBy`/`sortOrder` are not accepted there. That is the *only* sanctioned deviation, and it is documented in its contract. A new list picks the pair. Do not "fix" it by accepting `sortOrder` as an alias: an alias is a second spelling of one thing with undefined precedence when both arrive, which is its own inconsistency. -`GET /logs` was the second exception until it absorbed `POST /logs/query`. That fold is the cautionary tale for this rule: the justification for the `order` spelling was "logs have exactly one sortable column", and a second endpoint sorting the same rows four ways had already disproved it. When a rule's premise is contradicted by another endpoint on the same collection, fix the premise rather than documenting the exception. +Before documenting a second `order`-style exception, check every other endpoint on the same collection: if one of them already sorts those rows more than one way, the "exactly one sortable column" premise is false — fix the premise rather than documenting the exception. -**A boolean query param is a real boolean**, declared with `booleanQueryFlagSchema` from `contracts/primitives.ts`. It coerces `'true'`/`'1'` and `'false'`/`'0'`/`''`, so it is a strict widening of a `z.enum(['true','false'])` — which is what two v2 params used to be, purely by inheritance from the internal shapes they reused. Reusing an internal `.shape.x` inherits the internal spelling; re-declare instead when the internal one is not the v2 convention. +**A boolean query param is a real boolean**, declared with `booleanQueryFlagSchema` from `contracts/primitives.ts`; it coerces `'true'`/`'1'` and `'false'`/`'0'`/`''`. Reusing an internal `.shape.x` inherits the internal spelling (often a `z.enum(['true','false'])`); re-declare instead when the internal one is not the v2 convention. ## Rule 4 — reject what you do not implement **Every contract declares a `query`, even when the endpoint takes none** — `query: noInputSchema` (`z.object({}).strict()`), never omission. `parseRequest` validates the query slice only when the contract declares one, so an omitted `query` means "never look at the query string", not "takes no query params". The two were indistinguishable, which is how 69 contracts ended up accepting anything without anyone deciding they should: `GET /workflows/{id}?bogus=1` answered 200 while every list answered 400 for the same shape. `query-declaration.test.ts` sweeps the tree so contract 70 fails at authoring time rather than shipping unvalidated. -Declaring them is a **deliberate tightening** of endpoints that previously ignored an unknown param. It was weighed and kept: the v2 body slice on those same endpoints was already strict, so the split was arbitrary rather than a promise to callers; a mistyped param that is silently dropped is the bug class this rule exists to prevent; and no first-party caller sends an undeclared v2 query param (the two SDKs send only `includeOutput`/`selectedOutputs`, both declared; the UI makes no v2 calls at all; `requestJson` appends nothing implicitly and no v2 cache buster exists). Third-party callers appending a tracking tag or cache buster do break, which is why `api-reference/getting-started.mdx` documents the behavior rather than leaving it to be discovered from a 400. +Unknown query params are a 400. That is safe for first-party callers — the two SDKs send only `includeOutput`/`selectedOutputs`, both declared; the UI makes no v2 calls; `requestJson` appends nothing implicitly and there is no v2 cache buster — and it matches the already-strict body slice. Third-party callers appending a tracking tag or cache buster do break, which is why `api-reference/getting-started.mdx` documents the behavior rather than leaving it to be discovered from a 400. Query and body schemas are **`.strict()`** — and `.strict()` binds the **top level only**. A strict body containing a non-strict nested object still drops unknown keys one level down, which is the headline `filter` bug at a smaller scale: `sort: [{ field, direction, nulls: 'last' }]` answered 200 and ordered by the default. Strictness belongs on the shared nested schema (`sortSpecSchema`'s element, `tableViewConfigSchema`), not restated per body. @@ -163,7 +162,7 @@ The 503 default is applied by `v2Error` keyed on the response *status* — `Retr Do not add a default for any other code. 400/403/404/409 are not fixed by waiting, and 402 (`USAGE_LIMIT_EXCEEDED`) is resolved by a billing change, not by time. -**Where a policy already knows the wait, carry it — do not re-guess it at the transport.** The admission descriptors in `lib/core/admission/transient-failure` declare `retryAfterSeconds` per denial. That value used to be dropped when the descriptor was mapped onto a preprocess error, so a concurrency denial arrived as a bare 429 with no `Retry-After` even though the policy had named the wait. It now travels `descriptor.retryAfterSeconds → PreprocessExecutionError.retryAfterMs → ExecuteWorkflowServiceFailure.retryAfterMs → serviceFailureResponse`. The `v2Error` default is the floor for paths with *no* policy signal, not the source of truth. +**Where a policy already knows the wait, carry it — do not re-guess it at the transport.** The admission descriptors in `lib/core/admission/transient-failure` declare `retryAfterSeconds` per denial. Carry it the whole way: `descriptor.retryAfterSeconds → PreprocessExecutionError.retryAfterMs → ExecuteWorkflowServiceFailure.retryAfterMs → serviceFailureResponse`; a mapping that drops it turns a concurrency denial into a bare 429 with no `Retry-After` even though the policy named the wait. The `v2Error` default is the floor for paths with *no* policy signal, not the source of truth. **A failure whose outcome is unknown must not advise a retry.** `ASYNC_ENQUEUE_AMBIGUOUS` is a 503 whose enqueue may have succeeded — it deliberately retains its execution-ID claim. Telling that caller to come back in 5 seconds invites a client with no `X-Run-Id` to start and bill a second run. It passes `omitRetryAfter: true` and returns the run id so the caller reconciles instead. Any future "we don't know if it happened" failure does the same. @@ -176,7 +175,7 @@ Audited against the primary specs and against Stripe, GitHub, and Google's AIPs. | Practice | Verdict | Why | |---|---|---| | **RFC 9457 `application/problem+json`** | No | 9457 §4 steers APIs with an existing format toward keeping it: "Problem details are intended to avoid the necessity of establishing new 'fault' or 'error' document formats, **not to replace existing domain-specific formats**." Nothing in it is a `MUST` to adopt, and none of Stripe, GitHub, or Google use it. Our envelope is load-bearing for every client. **The default error shape does not change.** | -| **`RateLimit`/`RateLimit-Policy` (IETF draft)** | No | Still an unpublished draft (`-11`, May 2026), returned "Not ready" at HTTPDIR review, and on its **third mutually incompatible wire format** — anything built against `-07` or earlier is already broken. None of the three surveyed APIs emit it; GitHub uses `x-ratelimit-*`, as we do. | +| **`RateLimit`/`RateLimit-Policy` (IETF draft)** | No | Still an unpublished IETF draft whose wire format has changed incompatibly across revisions — anything built against an earlier revision is already broken. None of the three surveyed APIs emit it; GitHub uses `x-ratelimit-*`, as we do. Re-check the draft's status before re-opening. | | **Renaming `X-RateLimit-*` per RFC 6648** | No | 6648 is a `SHOULD NOT` binding *creators of new* parameters, and §1 item 4 "**makes no recommendation as to whether existing 'X-' parameters ought to remain in use or be migrated**". Appendix B argues the migration is itself the interoperability harm. A rename is a client-visible break bought with nothing. | | **`X-RateLimit-Reset` as delta-seconds** | No | It is an absolute ISO 8601 timestamp, so it is clock-skew sensitive — but the response where timing actually decides behaviour (429) also carries `Retry-After`, which is skew-free. The absolute value stays useful for scheduling. | | **422 for semantic validation** | No | RFC 9110 §15.5.21 defines 422, but Appendix B.3 records that 9110 **deleted** RFC 4918's clause saying 400 was inappropriate. 400 covers "cannot or will not process… perceived to be a client error". The split is convention, not requirement — GitHub splits, Stripe and Google do not. Our machine-readable `error.code` already carries the distinction, and restatusing now breaks clients. | diff --git a/.agents/skills/validate-connector/SKILL.md b/.agents/skills/validate-connector/SKILL.md index 81070a8e2a3..9e25db59753 100644 --- a/.agents/skills/validate-connector/SKILL.md +++ b/.agents/skills/validate-connector/SKILL.md @@ -159,11 +159,11 @@ For each API endpoint the connector calls: - [ ] The connector does NOT hit known API pagination limits silently (e.g., HubSpot search 10k cap) ### Deletion-Reconciliation Safety (`listingCapped`) — CRITICAL -The sync engine hard-deletes any stored document absent from a full listing. Audit every path where `listDocuments` can return less than the full source set: +The sync engine tombstones, then hard-deletes, any stored document absent from a full listing. Audit every path where `listDocuments` can return less than the full source set: - [ ] `syncContext.listingCapped = true` is set when a `maxItems`-style cap truncates the listing while more documents exist - [ ] `listingCapped` is set when a transient per-item error drops a still-existing document from the listing - [ ] `listingCapped` is NOT set when the source is genuinely exhausted (deleted documents must reconcile) or for intentional scope filters (date cutoffs) -This is the most common connector bug class — verify it explicitly against `sync-engine.ts`'s reconciliation gate. +Verify it explicitly against `shouldReconcileDeletions` in `sync-engine.ts`. ### Pagination State Across Pages - [ ] `syncContext` is used to cache state across pages (user names, field maps, instance URLs, portal IDs, etc.) @@ -311,7 +311,7 @@ Group findings by severity: - Incorrect response field mapping (accessing wrong path) - SOQL/query fields that don't exist on the target object - Pagination that silently hits undocumented API limits -- Missing `syncContext.listingCapped = true` when a cap or transient error truncates the listing — the sync engine hard-deletes the documents absent from the partial listing +- Missing `syncContext.listingCapped = true` when a cap or transient error truncates the listing — the sync engine tombstones and later hard-deletes the documents absent from the partial listing - Missing error handling that would crash the sync - `requiredScopes` not a subset of OAuth provider scopes - Query/filter injection: user-controlled values interpolated into OData `$filter`, SOQL, or query strings without escaping diff --git a/.agents/skills/validate-integration/SKILL.md b/.agents/skills/validate-integration/SKILL.md index 541f9e37846..308df1d6691 100644 --- a/.agents/skills/validate-integration/SKILL.md +++ b/.agents/skills/validate-integration/SKILL.md @@ -215,7 +215,7 @@ For **each tool** in `tools.access`: - Enum/fixed options → `dropdown` - Free text → `short-input` - Long text/content → `long-input` - - True/false → `dropdown` with Yes/No options (not `switch` unless purely UI toggle) + - True/false → `switch` (a Yes/No `dropdown` only when the tool needs a third "unset" state) - Credentials → `oauth-input` with correct `serviceId` - [ ] Dropdown `value: () => 'default'` is set for dropdowns with a sensible default @@ -235,19 +235,18 @@ For **each tool** in `tools.access`: - [ ] Timestamp fields have `wandConfig` with `generationType: 'timestamp'` - [ ] Comma-separated list fields have `wandConfig` with a descriptive prompt - [ ] Complex filter/query fields have `wandConfig` with format examples in the prompt -- [ ] All `wandConfig` prompts end with "Return ONLY the [format] - no explanations, no extra text." +- [ ] All `wandConfig` prompts end with an explicit `Return ONLY the ` instruction so the generated value can be pasted directly into the field - [ ] `wandConfig.placeholder` describes what to type in natural language ### Tools Config - [ ] `tools.access` lists **every** tool ID the block can use — none missing - [ ] `tools.config.tool` returns the correct tool ID for each operation -- [ ] Type coercions are in `tools.config.params` (runs at execution time), NOT in `tools.config.tool` (runs at serialization time before variable resolution) +- [ ] Type coercions are in `tools.config.params` (runs at execution time), NOT in `tools.config.tool` (runs at serialization time before variable resolution — coercing there destroys dynamic references like ``) - [ ] `tools.config.params` handles: - `Number()` conversion for numeric params that come as strings from inputs - `Boolean` / string-to-boolean conversion for toggle params - Empty string → `undefined` conversion for optional dropdown values - Any subBlock ID → tool param name remapping -- [ ] No `Number()`, `JSON.parse()`, or other coercions in `tools.config.tool` — these would destroy dynamic references like `` ### Block Outputs - [ ] Outputs cover the key fields returned by ALL tools (not just one operation) @@ -481,7 +480,7 @@ After fixing, confirm: - [ ] Reported all issues grouped by severity - [ ] Fixed all critical and warning issues - [ ] Ran `bun run tool-metadata:generate` if any tool outputs/params changed, and confirmed `bun run tool-metadata:check` passes -- [ ] Ran `bun run generate-docs` if any block metadata changed, and committed the full generated diff — including stale-page catch-up for other integrations (`bun run docs:check` fails CI on reverted generator output) +- [ ] Ran `bun run scripts/generate-docs.ts` if any block metadata changed, and committed the full generated diff — including stale-page catch-up for other integrations (`bun run docs:check` fails CI on reverted generator output) - [ ] Ran `bun run lint` after fixes - [ ] Verified TypeScript compiles clean - [ ] Verified added tests fail without their fix diff --git a/.agents/skills/validate-model/SKILL.md b/.agents/skills/validate-model/SKILL.md index d7d9cc88c6f..06d982ebe10 100644 --- a/.agents/skills/validate-model/SKILL.md +++ b/.agents/skills/validate-model/SKILL.md @@ -43,27 +43,7 @@ If a fetch fails (404, timeout, paywall), record the URL attempted and mark depe ## Step 3: Build the consumption map for this provider -Re-grep before trusting the snapshot below: - -```bash -rg "reasoningEffort|reasoning_effort" apps/sim/providers// -rg "verbosity" apps/sim/providers// -rg "request\.thinking|thinking:" apps/sim/providers// -rg "supportsNativeStructuredOutputs|nativeStructuredOutputs" apps/sim/providers// -``` - -Snapshot (verify before relying): - -| Capability | Consumed by | -|---|---| -| `reasoningEffort` | `openai/core.ts`, `azure-openai`, `anthropic/core.ts` (mapped via thinking), `gemini/core.ts` | -| `verbosity` | `openai/core.ts`, `azure-openai/index.ts` | -| `thinking` | `anthropic/core.ts`, `gemini/core.ts` | -| `nativeStructuredOutputs` | `anthropic/core.ts`, `fireworks/index.ts`, `openrouter/index.ts` | -| `computerUse` | `anthropic/core.ts` | -| `temperature` | All providers (passthrough) | - -A flag set in `models.ts` but not in the consumption list for this provider = **warning: dead flag**. +Use the Consumption Matrix in `.agents/skills/add-model/SKILL.md` Step 2 and run its re-grep commands for the target provider before relying on it. A flag set in `models.ts` that the provider's code does not read = **warning: dead flag**. ## Step 4: Run the checklist @@ -90,7 +70,7 @@ For each model, evaluate every row. Statuses: ✓ matches docs, ✗ disagrees, - [ ] `verbosity.values` — only on OpenAI gpt-5.x family; values match docs - [ ] `thinking.levels` + `thinking.default` — only on Anthropic/Gemini; values match docs - [ ] `thinking.streamed` — REQUIRED on Anthropic-family thinking models (`'full'` for generations returning full thinking deltas, `'summary'` for omitted-display generations like Opus 4.7+/Sonnet 5/Fable 5 where Sim requests `display: 'summarized'`); verify against the provider's thinking-display docs. After any change, run `bun run agent-stream-docs:generate` so the Agent block docs table stays in sync (CI diffs it) -- [ ] `nativeStructuredOutputs` — only on anthropic/fireworks/openrouter; provider must document Structured Outputs / JSON-mode for this model +- [ ] `nativeStructuredOutputs` — only on providers whose code consumes it (see the Consumption Matrix); provider must document Structured Outputs / JSON-mode for this model - [ ] `toolUsageControl` — provider supports `tool_choice` semantics - [ ] `computerUse` — provider implements computer-use loop AND model is a computer-use SKU - [ ] `deepResearch` — only on actual deep-research SKUs @@ -101,7 +81,7 @@ For each model, evaluate every row. Statuses: ✓ matches docs, ✗ disagrees, - [ ] `speedOptimized: true` — only on smallest/fastest tier (nano / flash-lite / haiku class) ### Hosting / billing -- [ ] If the model is under `openai`/`anthropic`/`google`, it is automatically in `getHostedModels()` → served with Sim's rotating key and billed via `shouldBillModelUsage()`. Confirm that is the intent (a BYOK-only model parked under one of these providers is a billing bug — warning). +- [ ] If `getHostedModels()` includes the model ID (`providers/models.ts` expands whole providers — more than openai/anthropic/google — plus the static Fireworks catalog), the model is served with Sim's rotating key and billed via `shouldBillModelUsage()`. Confirm that is the intent (a BYOK-only model parked under a hosted provider is a billing bug — warning). - [ ] If the model is hosted, the deployment is expected to have its `{PREFIX}_COUNT` / `{PREFIX}_1..N` env vars set (ops concern; note if it looks unset for a model claiming hosted support). ## Step 5: Report (mandatory format) @@ -148,17 +128,9 @@ After reporting, ask: *"Want me to fix the critical and warning items? I'll prin - 🔵 **suggestion** — style/consistency. Examples: field order, missing `speedOptimized` on a clearly smallest-tier model. - ❓ **unverified** — could not fetch an authoritative source for this field. Surface it; never silently confirm. -## Common bugs this skill catches - -- Pricing drift after a provider price cut (very common — providers cut quarterly) -- `reasoningEffort` set on always-reasoning models that reject the parameter (grok-4.3, o3-pro pattern) -- `nativeStructuredOutputs` set on providers that don't consume the flag (dead) -- `thinking` set on non-Anthropic/non-Gemini providers -- `verbosity` set on non-gpt-5.x models -- Wrong context window (e.g., 128k claimed vs 200k actual) -- Stale `pricing.updatedAt` -- Multiple `recommended: true` per provider after a flagship swap -- Missing `deprecated: true` on retired models (e.g., the xAI batch retiring May 15, 2026) +## Common drift + +Pricing changes after provider price cuts; `reasoningEffort`/`thinking`/`verbosity` set on a model whose provider code or API does not accept them; stale `pricing.updatedAt`; wrong context window; more than one `recommended` after a flagship swap; missing `deprecated: true` after a provider retirement announcement. ## What "I cannot verify this" looks like diff --git a/.agents/skills/validate-permission-group-item/SKILL.md b/.agents/skills/validate-permission-group-item/SKILL.md index febb6c278ec..3488765cdf2 100644 --- a/.agents/skills/validate-permission-group-item/SKILL.md +++ b/.agents/skills/validate-permission-group-item/SKILL.md @@ -77,7 +77,7 @@ The second grep misses a gate whose annotation sits in a TSDoc block above the e Classify into exactly one of: 1. **Declared on operations.** The funnel enforces in `requireCurrentHumanAccess` → `requireCapability`. Verify the set is *complete*: enumerate every route and tool reaching the same behavior. One declaring `capability: 'none'` is the hole. -2. **Asserted at a call site** with a `// permission-group-enforced: — ` annotation. Verify it goes through `capability-assertions.ts` (`assertWorkspaceCapability`, `isWorkspaceCapabilityWithheld`, `isOrganizationCapabilityWithheld`, `capabilityDeniedBy`), through `isCapabilityWithheldForUser` (`lib/permission-groups/user-scope.server.ts` — workspace group first, else the organization's default, for a user-level act that may or may not name a workspace; outside `capability-assertions.ts` on purpose because it reads org membership through the billing graph, a guarded root of `check:application-graph`; `app/api/cli/auth/approve/route.ts` is the shape), or a direct `CAPABILITY_RULES[''].deniedBy(...)` rather than reading `config.disableX` inline, **and** that it *raises* through `refuseCapability` / renders `capabilityRefusal(cap)` rather than building its own `ForbiddenOperationError` with a hand-written message — the easy half to miss, because the decision looks right. Use-case shape: `validatePublicFileSharing`, `validateChatDeployAuth` (`ee/access-control/utils/permission-check.ts`), `assertConnectorTypeAllowed` (`lib/knowledge/application/connectors.ts`). Raw-route shape: `app/api/logs/stats/route.ts`, `app/api/table/[tableId]/export/route.ts`. A raw route should render through `capabilityRefusalResponse` (`lib/permission-groups/capability-response.ts`), which reads `details.code` off the rule — a hand-rolled `NextResponse.json({ error: capabilityRefusal(cap) }, { status: 403 })` drops it, reporting the four specifically-coded capabilities (`deploy.chat.auth_mode`, `file_share.publish`, `file_share.auth_mode`, `personal_api_key.use`) as the generic block. Convergence is partial — the inbox, api-keys, oauth-credentials, cli-approve and `logs/export` routes still hand-roll it, harmlessly today because all of their capabilities carry the generic code, so report one only if its capability gains a specific code. v1 is deliberately not converged on it (`resolveCapabilityRefusal` in `app/api/v1/middleware.ts`). +2. **Asserted at a call site** with a `// permission-group-enforced: — ` annotation. Verify it goes through `capability-assertions.ts` (`assertWorkspaceCapability`, `isWorkspaceCapabilityWithheld`, `isOrganizationCapabilityWithheld`, `capabilityDeniedBy`), through `isCapabilityWithheldForUser` (`lib/permission-groups/user-scope.server.ts` — workspace group first, else the organization's default, for a user-level act that may or may not name a workspace; outside `capability-assertions.ts` on purpose because it reads org membership through the billing graph, a guarded root of `check:application-graph`; `app/api/cli/auth/approve/route.ts` is the shape), or a direct `CAPABILITY_RULES[''].deniedBy(...)` rather than reading `config.disableX` inline, **and** that it *raises* through `refuseCapability` / renders `capabilityRefusal(cap)` rather than building its own `ForbiddenOperationError` with a hand-written message — the easy half to miss, because the decision looks right. Use-case shape: `validatePublicFileSharing`, `validateChatDeployAuth` (`ee/access-control/utils/permission-check.ts`), `assertConnectorTypeAllowed` (`lib/knowledge/application/connectors.ts`). Raw-route shape: `app/api/logs/stats/route.ts`, `app/api/table/[tableId]/export/route.ts`. A raw route should render through `capabilityRefusalResponse` (`lib/permission-groups/capability-response.ts`), which reads `details.code` off the rule — a hand-rolled `NextResponse.json({ error: capabilityRefusal(cap) }, { status: 403 })` drops it, reporting the four specifically-coded capabilities (`deploy.chat.auth_mode`, `file_share.publish`, `file_share.auth_mode`, `personal_api_key.use`) as the generic block. Convergence is partial: `grep -rln "capabilityRefusal(" apps/sim/app --include=route.ts` lists the raw routes that still hand-roll it (ignore `*.test.ts`, `app/api/v1/middleware.ts`, `app/api/table/utils.ts`, and the v2 envelope, which are not raw-route responses). A raw route you add or touch renders through `capabilityRefusalResponse`; report an untouched hand-rolled one as a finding when its capability carries a specific code, otherwise as a note. v1 is deliberately not converged on it (`resolveCapabilityRefusal` in `app/api/v1/middleware.ts`). 3. **Executor-gated** by `assertPermissionsAllowed`, per block / tool / model, matching through the shared primitives in `lib/permission-groups/` — `block-access.ts`, `operation-access.ts`, `model-access.ts`, `integration-allowlist.ts` — which the editor and Copilot projections read too, so a second copy of a match rule is a finding. Verify the branch throws a real error and that the id it compares against is the vocabulary the admin UI writes — `deniedTools` holds block `tools.access` ids verbatim, version suffix included. `allowedIntegrations` is *also* enforced off the run, by `assertSelectorIntegrationAllowed` (`lib/selectors/server/integration-access.ts`), so an executor key's coverage is not complete until every non-run path that reaches the third party is checked too. 4. **A field projection, not a gate.** `logs.trace_spans` and `logs.cost` withhold fields, so the logs routes correctly declare `capability: 'none'`. Single owner: `lib/logs/log-projection.ts` (`resolveLogFieldProjection`, `projectExecutionData`, `projectCostTotal`), which carries both annotations. A **second** implementation of the same redaction is the finding — as is a query that lets a caller filter or sort on a withheld field, which turns the projection into an oracle. 5. **Nothing.** Report as a defect: "an organization that sets this believes it applied a restriction that does not exist". @@ -93,8 +93,8 @@ For an allowlist the three states must be tested separately — `null` permits e **Read the subject, not the nearest user id.** Every capability sink must take its subject from the `capabilityGoverned*` helper for the identity the surface holds — `capabilityGovernedPrincipalUserId` for a `Principal` (`lib/core/application`), `capabilityGovernedUserId` for a v1 `RateLimitResult` or a `TableAccessPrincipal`, `capabilityGovernedAuthUserId` for a `checkSessionOrInternalAuth` result. Each returns `null` where no group governs, and `null` is a pass. Reading `rateLimit.userId`, `auth.userId`, `subjectUserId` or `triggeredByUserId` into a sink is the finding: for a workspace key the first is the key's *creator*, for an internal JWT the second is the run's actor, and the last is a billing *attribution*. `check-capability-subject.ts` audits **v1 only**, so every other surface is on you. Where the subject is persisted and read back later (`capabilityGovernedUserId` on `table_run_dispatches` / `table_row_executions`), it must be declared required as `string | null` — an optional field with a fallback is exactly how producers re-inherited `triggeredByUserId`, so a proposal to make it optional is a finding. - **`/api/v1`** authorizes in `app/api/v1/middleware.ts`, not through `authorizeWorkspaceOperation`; `capabilityGovernedUserId(rateLimit)` branches on `keyType`, never on the presence of a user id. Each route also threads a required, spelled-out `V1RouteCapability`. -- **Raw internal table routes** gate `tables.use` in `checkAccess` (`app/api/table/utils.ts`) via a `TableAccessPrincipal` union — `{ kind: 'user'; userId }` or `{ kind: 'workspace_api_key'; keyCreatorUserId }` — so a bare id no longer type-checks. `tableAccessPrincipal(rateLimit)` is the one place v1 builds it. -- **The definition-time `undefined` guard** on `defineWorkspaceOperation` is not redundant even though `capability` is required on the `ApplicationOperation` **base type** (`lib/core/application/operation.ts:31`, not merely on the builder — which is what stops a bare-literal factory from compiling): `apps/sim/tsconfig.json` excludes `*.test.ts` / `*.test.tsx` and the enforcement audit walks past test files, so a fixture is the one construction site no static check reads. Without it a capability-less operation defines cleanly and then throws `Cannot read properties of undefined` inside `capabilityDeniedBy` **only for tenants that actually have a permission group**, passing CI and every personal workspace. A proposal to drop it is a finding. +- **Raw internal table routes** gate `tables.use` in `checkAccess` (`app/api/table/utils.ts`) via a `TableAccessPrincipal` union — `{ kind: 'user'; userId }` or `{ kind: 'workspace_api_key'; keyCreatorUserId }` — so a bare id does not type-check. `tableAccessPrincipal(rateLimit)` is the one place v1 builds it. +- **The definition-time `undefined` guard** on `defineWorkspaceOperation` is not redundant even though `capability` is required on the `ApplicationOperation` **base type** (the `capability` field in `lib/core/application/operation.ts`, not merely on the builder — which is what stops a bare-literal factory from compiling): `apps/sim/tsconfig.json` excludes `*.test.ts` / `*.test.tsx` and the enforcement audit walks past test files, so a fixture is the one construction site no static check reads. Without it a capability-less operation defines cleanly and then throws `Cannot read properties of undefined` inside `capabilityDeniedBy` **only for tenants that actually have a permission group**, passing CI and every personal workspace. A proposal to drop it is a finding. ## Step 6: Tests @@ -112,12 +112,12 @@ bun run check:capability-subject cd apps/sim && bun run type-check && bunx vitest run lib/permission-groups ``` -All three are inside `check:audits`, which derives its list from the `check:*` scripts in `package.json` — a new audit is opted *out* deliberately. Read the output, not the exit codes. Reference success lines (counts grow): +All three are inside `check:audits`, which derives its list from the `check:*` scripts in `package.json` — a new audit is opted *out* deliberately. Read the output, not the exit codes. Success-line shapes (the counts must include the item under audit): ``` -✓ permission-group enforcement: 322 operations declare a capability, 35 capabilities all enforced -✅ Application graph clean: 5 roots reach none of 11 forbidden module trees -check:capability-subject — 32 v1 files, 5 capability subjects resolved through capabilityGovernedUserId. +✓ permission-group enforcement: operations declare a capability, capabilities all enforced +✅ Application graph clean: roots reach none of forbidden module trees +check:capability-subject — v1 files, capability subjects resolved through capabilityGovernedUserId. ``` | Audit | What it catches | @@ -128,7 +128,7 @@ check:capability-subject — 32 v1 files, 5 capability subjects resolved through Two ways the enforcement audit passes without proving what you want: -- **Vacuous parse.** It reads source text with regexes, so it refuses success when the three registries parse to nothing, cross-checks rule count against capability count, reports per call any unreadable `id`, fails a file that mints an operation but parses to **zero** declarations, and flags any exported `*Operations` registry member it read no operation from. If one fires the audit is broken, not the code — fix the parsers rather than leaving it green. (That last guard exists because an operation minted by a factory that never calls the builder bypasses the required type *and* the audit; twenty-one operations across six domains were invisible that way while the file still printed a tick.) +- **Vacuous parse.** It reads source text with regexes, so it refuses success when the three registries parse to nothing, cross-checks rule count against capability count, reports per call any unreadable `id`, fails a file that mints an operation but parses to **zero** declarations, and flags any exported `*Operations` registry member it read no operation from. If one fires the audit is broken, not the code — fix the parsers rather than leaving it green. (That last guard is what catches an operation minted by a factory that never calls the builder, which bypasses the required type *and* the audit.) - **A capability declared on an operation nothing routes to.** Assertion C is satisfied by the declaration alone. The audits prove *reachability*, never correctness — that a capability is named, a key is read by some rule, a subject came from the right helper. Step 5 is what covers the rest. diff --git a/.agents/skills/validate-trigger/SKILL.md b/.agents/skills/validate-trigger/SKILL.md index 715a6642d28..e2855c3d26c 100644 --- a/.agents/skills/validate-trigger/SKILL.md +++ b/.agents/skills/validate-trigger/SKILL.md @@ -222,7 +222,7 @@ After reporting, fix every **critical** and **warning** issue. Apply **suggestio After fixing, confirm: 1. `bun run type-check` passes 2. Re-read all modified files to verify fixes are correct -3. Provider handler tests pass (if they exist): `bun test {service}` +3. Provider handler tests pass (if they exist): `bun run --cwd apps/sim test lib/webhooks/providers/` — handler files are kebab-case (`azure-devops.ts`) while trigger directories are snake_case (`azure_devops`), so use the handler's actual basename 4. Any remaining unknown webhook payload schemas were explicitly reported to the user instead of guessed ## Checklist Summary diff --git a/.agents/skills/you-might-not-need-a-callback/SKILL.md b/.agents/skills/you-might-not-need-a-callback/SKILL.md index 51962f68201..46507b99628 100644 --- a/.agents/skills/you-might-not-need-a-callback/SKILL.md +++ b/.agents/skills/you-might-not-need-a-callback/SKILL.md @@ -38,14 +38,11 @@ If none of those apply — if the function is only called inline, or passed to a 4. **useCallback wrapping functions that return new objects/arrays**: Stable function identity, unstable return value — memoization is at the wrong level. Use `useMemo` on the return value instead, or restructure. 5. **useCallback with empty deps when deps are needed**: Stale closure — reads initial values forever. This is a correctness bug, not just a performance issue. 6. **Pairing useCallback + React.memo on trivially cheap renders**: If the child renders in < 1ms and re-renders rarely, the memo infrastructure costs more than it saves. -7. **useCallback in custom hooks that don't need stable references**: Not every hook return needs to be memoized. Only stabilize callbacks when consumers depend on referential equality. +7. **Internal helpers inside custom hooks wrapped for no observer**: functions a hook only calls internally need no `useCallback`. Functions a hook *returns* are wrapped by convention (`.claude/rules/sim-hooks.md` Rule 4, matching react.dev) — do not flag those for lacking an observer, but still check their dependency arrays (patterns 2-5 apply to them as much as to any other `useCallback`). ## Patterns that ARE correct — do not flag -- `useCallback` whose result is in a `useEffect` dep array — prevents the effect from re-running on every render -- `useCallback` whose result is in a `useMemo` dep array — prevents the memo from recomputing on every render -- `useCallback` whose result is a dep of another `useCallback` — stabilises a callback chain -- `useCallback` passed to a `React.memo`-wrapped child — the whole point of the pattern +- Any `useCallback` with an observer from the list above - This codebase's ref pattern: `useRef` + callback with empty deps that reads the ref inside — correct, do not flag: ```tsx diff --git a/.agents/skills/you-might-not-need-a-comment/SKILL.md b/.agents/skills/you-might-not-need-a-comment/SKILL.md index 2ea2290197c..788bbbda174 100644 --- a/.agents/skills/you-might-not-need-a-comment/SKILL.md +++ b/.agents/skills/you-might-not-need-a-comment/SKILL.md @@ -32,7 +32,7 @@ This codebase's convention: **TSDoc for documentation, no non-TSDoc comments, no - A `//` comment that explains a **non-obvious why**: a workaround for an upstream bug, an ordering constraint, a perf reason, a spec/edge-case the code can't self-document (`// first-match wins — matches the old find() semantics`). - Existing TSDoc `/** ... */` blocks on declarations — leave them (only tighten if verbose). -- `// boundary-raw-fetch:`, `// double-cast-allowed:`, `// boundary-raw-json:`, `// untyped-response:`, `// migration-safe:` and other **machine-read annotations** — these are load-bearing, never touch them. +- `// boundary-raw-fetch:`, `// double-cast-allowed:`, `// boundary-raw-json:`, `// untyped-response:`, `// migration-safe:`, `// rq-lint-allow:`, `// client-boundary-allow:` and any other `: ` annotation a script under `scripts/` greps for, in line-comment or block-comment form (e.g. the `/** svg-path-precision-exception: ... */` directive on icon paths) — these are load-bearing, never touch them. - `// biome-ignore`, `// eslint-disable`, `// @ts-expect-error` and other tooling directives. - `// TODO` / `// FIXME` that point at real, still-open work. diff --git a/.agents/skills/you-might-not-need-state/SKILL.md b/.agents/skills/you-might-not-need-state/SKILL.md index 8c9a43458d3..6006a2b3362 100644 --- a/.agents/skills/you-might-not-need-state/SKILL.md +++ b/.agents/skills/you-might-not-need-state/SKILL.md @@ -27,7 +27,7 @@ Read these before analyzing: 1. **Derived state stored in useState**: If a value can be computed from props, other state, or query data, compute it inline during render instead of storing it in state. 2. **Server state copied into useState**: Never `useState` + `useEffect` to sync React Query data into local state. Use query data directly. The only exception is forms where users edit server data. -3. **Props mirrored into state**: Never `useState(prop)` + `useEffect(() => setState(prop))`. Use the prop directly, or use a key to reset component state. +3. **Props mirrored into state**: Never `useState(prop)` + `useEffect(() => setState(prop))`. Use the prop directly, reset with a remount `key`, or — for seed-on-transition (e.g. a modal opening) — adjust during render with the `useState` prev-tracker in `.claude/rules/sim-hooks.md` "State shape" (mind its sentinel-on-mount and no-`useRef` rules). 4. **Chained useEffect state updates**: Never chain Effects that set state to trigger other Effects. Calculate all derived values in the event handler or inline during render. 5. **Storing objects when an ID suffices**: Store `selectedId` not a copy of the selected object. Derive the object: `items.find(i => i.id === selectedId)`. 6. **State that duplicates Zustand or React Query**: If the data already lives in a store or query cache, don't create a parallel useState. diff --git a/.agents/skills/you-might-not-need-url-state/SKILL.md b/.agents/skills/you-might-not-need-url-state/SKILL.md index 561829e4152..7eeec0b1b37 100644 --- a/.agents/skills/you-might-not-need-url-state/SKILL.md +++ b/.agents/skills/you-might-not-need-url-state/SKILL.md @@ -16,10 +16,6 @@ User arguments: $ARGUMENTS Shareable client view-state (active tab/panel, filters, search query, sort, pagination, selected-entity id, an open "view" modal/drawer that is a destination) lives in the URL via [`nuqs`](https://nuqs.dev) — driven by a co-located `search-params.ts`, never read via `useSearchParams().get(...)` and never mutated by hand-built query strings. Remote data stays in React Query; high-frequency / large / ephemeral / socket-synced state stays in Zustand; purely local UI stays in `useState`. -Shared helpers own the two repeated wirings — never hand-roll them inline: -- Sort: `createSortParams` from `@/lib/url-state` (in `search-params.ts`) + `useUrlSort` from `@/hooks/use-url-sort` (in the component) — defaulted mode for lists with a fixed default ordering, nullable mode when "no active sort" is distinct from the default column. -- Debounced search: `useDebouncedSearchSetter` from `@/hooks/use-debounced-search-setter` (grouped or single-param); settings list search boxes use `useSettingsSearch()` from `settings/components/use-settings-search`. Never write a trimmed value to a param that controls the input — trim on read. - `.claude/rules/sim-url-state.md` is the source of truth — read it first. ## References @@ -32,7 +28,7 @@ Read these before analyzing: ## Anti-patterns to detect -1. **Manual param reads for state**: `useSearchParams().get(...)` or `new URLSearchParams(window.location.search)` used to *read* view-state. Replace with `useQueryState`/`useQueryStates` bound to a `search-params.ts`. (Read-once auth/invite/redirect tokens — `token`, `callbackUrl`, `redirect`, `error`, `invite_flow`, `code` — are NOT view-state; leave them on `useSearchParams`.) +1. **Manual param reads for state**: `useSearchParams().get(...)` or `new URLSearchParams(window.location.search)` used to *read* view-state. Replace with `useQueryState`/`useQueryStates` bound to a `search-params.ts`. (Read-once auth/invite/redirect signals are NOT view-state — the list and the per-surface `new` caveat are in `.claude/rules/sim-url-state.md` "Read-once auth / redirect signals"; leave those on `useSearchParams`.) 2. **Hand-built query mutation**: constructing a query string + `router.replace`/`router.push` to change a param on the current path. Use a nuqs setter. (A `router.push` that changes the route *path* is fine; an outbound `new URLSearchParams` building an `href`/`window.open`/download/API URL is fine.) 3. **`window.history.replaceState`/`pushState`** to mutate a param. 4. **URL state duplicated into a store/useState + synced with an effect** (or a `popstate` listener). The URL is the single source of truth; derive from it, don't mirror it. diff --git a/.claude/skills/add-settings-page/SKILL.md b/.claude/skills/add-settings-page/SKILL.md index f3440cda8fd..02af76f3a8f 100644 --- a/.claude/skills/add-settings-page/SKILL.md +++ b/.claude/skills/add-settings-page/SKILL.md @@ -5,13 +5,16 @@ description: Add a new Sim settings page, or audit existing settings pages for d # Settings Page (add / audit) -Sim settings pages all render through the shared **`SettingsPanel`** primitive, -which owns the page chrome and renders a nav-driven title + description. The full +Settings page chrome (header bar, scroll region, content column, nav-driven +title + description) is owned by the `settings/[section]/layout.tsx` shell. Each +page renders through **`SettingsPanel`**, which registers the page's header data +(actions, search, back) with that shell and renders only the body. The full convention lives in `.claude/rules/sim-settings-pages.md` — read it first; this skill is the procedure. Key paths: -- Layout primitive: `apps/sim/app/workspace/[workspaceId]/settings/components/settings-panel/settings-panel.tsx` +- Chrome shell: `apps/sim/app/workspace/[workspaceId]/settings/[section]/layout.tsx` (`SettingsHeaderShell`) +- `SettingsPanel` registrar: `apps/sim/components/settings/settings-panel.tsx` - Nav metadata (titles + descriptions): `apps/sim/components/settings/navigation.ts` - Section switch + provider: `apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx` - Pages: `apps/sim/app/workspace/[workspaceId]/settings/components//.tsx` and EE pages under `apps/sim/ee//components/` @@ -25,10 +28,8 @@ Key paths: `requiresEnterprise`, etc.). 2. **Wire the switch.** Add the component to the `effectiveSection` render switch in `settings/[section]/settings.tsx` (lazy `dynamic(...)` like its siblings). -3. **Build the body inside `SettingsPanel`.** Never hand-roll the shell, header - bar, scroll region, content column, or title block. Put header buttons in - `actions`, a standalone search in `search={{ value, onChange, placeholder }}`, - and the page content as `children`. Modals go beside the panel inside a `<>`. +3. **Build the body inside `SettingsPanel`** per the rule's canonical page shape: + `actions`, `search`, `children`, modal siblings in a fragment. 4. **If the page has editable state**, wire the shared save/discard stack — put `SaveDiscardActions` (dirty-gated Discard+Save chips) in `actions`, and call `useSettingsUnsavedGuard({ isDirty })` **before any early-return gate**.