diff --git a/.agents/skills/add-block-preview/SKILL.md b/.agents/skills/add-block-preview/SKILL.md index c583a54c2cf..c3633e8dbc6 100644 --- a/.agents/skills/add-block-preview/SKILL.md +++ b/.agents/skills/add-block-preview/SKILL.md @@ -53,7 +53,7 @@ To pull an already-GA block from discovery surfaces on hosted (incident, depreca - **Clone-not-remove:** gated blocks stay in `getAllBlocks()` output as clones with `hideFromToolbar: true` — `.find`-by-type consumers rely on this. Never filter them out. - **Keys are registry block types.** Never `custom_block_*` (parse drops them — custom blocks have their own enabled/disabled lifecycle). - **The shared hidden-predicate is `isHiddenUnder`** (`apps/sim/blocks/visibility/context.ts`). Never restate the preview/disabled rule inline at a new consumer. -- **Process-global caches stay ungated.** `getStaticComponentFiles` (VFS) and `getExposedIntegrationTools` build the ungated universe; per-viewer filtering happens at stamp/consumer time. Never move gating into a shared builder. +- **Process-global caches stay ungated.** Shared builders such as `getExposedIntegrationTools` (`lib/integrations/tool-catalog.ts`) build the ungated universe; per-viewer filtering happens at consumer time via `isHiddenUnder`. Never move gating into a shared builder. - Gating is **surface hiding, not secrecy** — the full config ships in the client JS bundle. Anything truly secret cannot be a registered block. ## Tests diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md index 24a0366b91f..c8a83361a6b 100644 --- a/.agents/skills/add-block/SKILL.md +++ b/.agents/skills/add-block/SKILL.md @@ -35,7 +35,6 @@ export const {ServiceName}Block: BlockConfig = { docsLink: 'https://docs.sim.ai/integrations/{service}', category: 'tools', // 'tools' | 'blocks' | 'triggers' integrationType: IntegrationType.X, // Primary category (see IntegrationType enum) - tags: ['oauth', 'api'], // Cross-cutting tags (see IntegrationTag type) bgColor: '#HEXCOLOR', // Brand color icon: {ServiceName}Icon, @@ -63,7 +62,7 @@ export const {ServiceName}Block: BlockConfig = { }, inputs: { - // Optional: define expected inputs from other blocks + // Required: the params the block accepts, keyed by tool param / canonical id }, outputs: { @@ -74,7 +73,7 @@ export const {ServiceName}Block: BlockConfig = { ## SubBlock Types Reference -**Critical:** Every subblock `id` must be unique within the block. Duplicate IDs cause conflicts even with different conditions. +**Critical:** Give every subblock a unique `id`: duplicates collide silently (the last definition wins). `blocks.test.ts` fails a duplicate within one condition unless the copies are a basic/advanced mode-swap pair, one basic plus trigger-mode copies, or all carry `canonicalParamId`. The only sanctioned cross-condition reuse is the hosted-key `apiKey` pair (`add-hosted-key` skill), where both fields deliberately share one value. ### Text Inputs ```typescript @@ -129,6 +128,7 @@ export const {ServiceName}Block: BlockConfig = { id: 'credential', title: 'Account', type: 'oauth-input', + canonicalParamId: 'oauthCredential', serviceId: '{service}', // Must match OAuth provider service key requiredScopes: getScopesForService('{service}'), // Import from @/lib/oauth/utils placeholder: 'Select account', @@ -370,8 +370,6 @@ Declare the **canonical** id with `type: 'json'` — the subblock ids never reac ```typescript inputs: { file: { type: 'json', description: 'File to upload (UserFile or reference)' }, - // Legacy field for backwards compatibility - fileContent: { type: 'string', description: 'Legacy: base64 encoded content' }, } ``` @@ -500,6 +498,7 @@ Controls which UI view shows the field. - `'advanced'` - Only in advanced view - `'both'` - Both views (default if not specified) - `'trigger'` - Only in trigger configuration +- `'trigger-advanced'` - The advanced side of a trigger field (a canonical pair member, or a standalone field under the block-level advanced toggle) ### canonicalParamId Pattern @@ -616,7 +615,7 @@ tools: { - `items` property - This is only for tool outputs with array types Block outputs only support: -- `type` - The data type ('string', 'number', 'boolean', 'json', 'array') +- `type` - The data type ('string', 'number', 'boolean', 'json', 'array', 'file', 'file[]', 'any') - `description` - Human readable description - `condition` - Optional visibility condition - `hiddenFromDisplay` - Optional flag to hide from the output display @@ -679,7 +678,7 @@ export const ServiceV2Block: BlockConfig = { access: ServiceBlock.tools?.access?.map(id => `${id}_v2`) || [], config: { tool: createVersionedToolSelector({ - baseToolSelector: (params) => (ServiceBlock.tools?.config as any)?.tool(params), + baseToolSelector: (params) => ServiceBlock.tools.config?.tool(params) ?? 'service_default', suffix: '_v2', fallbackToolId: 'service_default_v2', }), @@ -697,7 +696,7 @@ export const ServiceV2Block: BlockConfig = { 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' +import { ServiceBlock, ServiceBlockMeta } from '@/blocks/blocks/{service}' export const BLOCK_REGISTRY: Record = { // ... existing blocks ... @@ -726,11 +725,23 @@ export const ServiceBlock: BlockConfig = { docsLink: 'https://docs.sim.ai/integrations/service', category: 'tools', integrationType: IntegrationType.DeveloperTools, - tags: ['oauth', 'api'], bgColor: '#FF6B6B', icon: ServiceIcon, authMode: AuthMode.OAuth, + // Sentence rules: apps/sim/blocks/AGENTS.md → "Canvas sentences" + canvasPresentation: { + defaultTitle: 'Create Resource', + sentences: { + byOperation: { + create: [{ text: 'Create resource', field: 'name', core: true }], + read: [{ text: 'Read resource', field: 'resourceId', core: true }], + update: [{ text: 'Update resource', field: 'resourceId', core: true }], + delete: [{ text: 'Delete resource', field: 'resourceId', core: true }], + }, + }, + }, + subBlocks: [ { id: 'operation', @@ -748,6 +759,7 @@ export const ServiceBlock: BlockConfig = { id: 'credential', title: 'Service Account', type: 'oauth-input', + canonicalParamId: 'oauthCredential', serviceId: 'service', requiredScopes: getScopesForService('service'), placeholder: 'Select account', @@ -778,6 +790,13 @@ export const ServiceBlock: BlockConfig = { }, }, + inputs: { + operation: { type: 'string', description: 'Operation to perform' }, + oauthCredential: { type: 'string', description: 'Service access token' }, + resourceId: { type: 'string', description: 'Resource ID' }, + name: { type: 'string', description: 'Resource name' }, + }, + outputs: { id: { type: 'string', description: 'Resource ID' }, name: { type: 'string', description: 'Resource name' }, @@ -937,30 +956,17 @@ 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 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: - -```bash -bun run scripts/generate-docs.ts -bun run deployment-config:generate -bun run integration-catalog:check -bun run deployment-config:check -bun run docs:check -``` - -The catalog check independently derives deployment metadata from the executable block registry and -compares it with the committed `packages/deployment-config/src/integrations.json`. The deployment -config check verifies the generated service-account facts against the canonical OAuth registry and -catalog. `docs:check` re-renders every generated docs artifact in memory and fails on any committed -file that differs — it runs in CI via `check:audits`, so commit the full generator output. If the -generator also trues up pages an earlier PR left stale, commit that catch-up too; reverting it as -"unrelated drift" makes `docs:check` fail. Review the generated diff and keep only intentional -changes. +A visible integration block does require the generated integration catalog and docs to be refreshed: +`bun run tool-metadata:generate` (only when a tool changed), `bun run scripts/generate-docs.ts`, +`bun run deployment-config:generate`, then `bun run check:audits`. Also run +`bun run apps/sim/scripts/check-block-registry.ts origin/staging` (CI runs it outside `check:audits`). Commit the +full generator output. For what each check verifies, see the `validate-integration` skill → +Regenerate Derived Artifacts. ## Checklist Before Finishing - [ ] `integrationType` is set to the correct `IntegrationType` enum value -- [ ] `tags` array includes all applicable `IntegrationTag` values +- [ ] `{Service}BlockMeta.tags` lists every applicable `IntegrationTag` (tags live on the meta, not the block) - [ ] All subBlocks have `id`, `title` (except switch), and `type` - [ ] Conditions use correct syntax (field, value, not, and) - [ ] DependsOn set for fields that need other values @@ -996,7 +1002,7 @@ Validate the block against every tool 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) - - `tools.config.params` correctly maps subBlock IDs to tool param names (if they differ) + - Each subBlock (or its `canonicalParamId`) is named exactly after the tool param it fills. A required `user-only` param that is only renamed in `tools.config.params` fails `bun run apps/sim/scripts/check-block-registry.ts origin/staging`; remap only optional or `user-or-llm` params - Type coercions in `tools.config.params` for any params that need conversion (Number(), Boolean(), JSON.parse()) 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 diff --git a/.agents/skills/add-column-type/SKILL.md b/.agents/skills/add-column-type/SKILL.md index 1ac05a8c9ec..91c27e4083c 100644 --- a/.agents/skills/add-column-type/SKILL.md +++ b/.agents/skills/add-column-type/SKILL.md @@ -12,7 +12,7 @@ A `case 'yourtype':` outside `column-types/` fails **silently** when missed (a w ## Hard Rule: the compiler tells you what to do -Do **not** hunt for places to edit. Add your type to the `ColumnType` union first and let `tsc` produce the list: +Do **not** hunt for places to edit. Append your type's id to the `COLUMN_TYPES` array in `column-types/types.ts` first (`ColumnType` derives from it) and let `tsc` produce the list: ```bash cd apps/sim && bun run type-check @@ -86,7 +86,7 @@ export function Type{Pascal}(props: SVGProps) { ## Step 3: Write the type file -`apps/sim/lib/table/column-types/{name}.ts`. Copy the closest existing type and change what differs. Every field is required by the interface, so the compiler enumerates them for you — read the TSDoc in `types.ts` rather than guessing. +`apps/sim/lib/table/column-types/{name}.ts`. Copy the closest existing type and change what differs. Required fields are compiler-enforced; optional hooks (`isCompatibleWith`, `salvage`, `valueForEquality`, `filterOperatorsFor`, …) default sensibly — read the TSDoc in `types.ts` before overriding. The three that are easy to get wrong: @@ -132,18 +132,18 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not, | `column-types/types.ts` `TYPE_SPECIFIC_COLUMN_KEYS` | it is never stripped on conversion, and poisons the target type | | `lib/api/contracts/tables.ts` — the schema slot in all three column schemas, plus `refineColumnOptions` | zod strips it at the boundary; silently never saved | | `columns/service.ts` `addTableColumn` param type | callers cannot pass it | -| A metadata-only update path (`updateColumnCurrency` is the model) + a branch in both column routes + the copilot tool | changing it on an existing column is a silent 200 no-op | +| A metadata-only update in `lib/table/columns/service.ts` (`updateColumnCurrency` is the model) + a branch in `performUpdateTableColumn` in `lib/table/orchestration/columns.ts` | changing it on an existing column is a silent 200 no-op | | `column-config-sidebar.tsx` | no UI to set it | | `table-grid.tsx` delete-column undo + `use-table-undo.ts` restore | undo silently resets it to the default | `normalizeColumn`, `buildConvertedColumn`, and the undo snapshot read `TYPE_SPECIFIC_COLUMN_KEYS` generically, so those three are already zero-edit. -**Known gap:** the metadata-only update path is ~6 near-identical copies (service + 2 routes + copilot). A `metadataUpdate` descriptor on `ColumnTypeServerDefinition` would collapse them; until that exists, copy `currency`'s. +Copy `currency`'s service function and orchestration branch. ## Checklist Before Finishing -- [ ] Added to the `ColumnType` union in `column-types/types.ts` -- [ ] `column-types/{id}.ts` created, every interface field filled in +- [ ] Id appended to `COLUMN_TYPES` in `column-types/types.ts` +- [ ] `column-types/{id}.ts` created, every required field filled in - [ ] Registered in **both** `registry.ts` and `registry.server.ts` - [ ] Icon added, centered on the family's optical center, exported alphabetically - [ ] `migrateCellsTo` / `migrateCellsFrom` added if the stored bytes change @@ -155,6 +155,6 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not, 1. **`cd apps/sim && bun run type-check`** — must be clean. If any file *outside* `column-types/` errors, that file has a hardcoded type list; fix it to read the registry. 2. **Grep for leaks** — `grep -rnE "(===|!==) '{id}'|case '{id}':" apps/sim --include='*.ts' --include='*.tsx' | grep -v column-types/`. (All three forms: a plain `!==` and a `case` are how half of `currency`'s real branches are written.) Hits are expected; judge each. A hit is fine when it mounts a specific React component or encodes a genuinely one-off behavior (`json`'s mono textarea, `date`'s timezone-aware parsing). A hit is a **leak** when it restates something the registry could answer — an icon, a label, a colour, an operator set, a cast, a coercion. Leaks get a registry field, not a new branch. -3. **Run the suite** — `bun run --cwd apps/sim test lib/table 'app/workspace/[workspaceId]/tables' lib/api app/api/table app/api/v1 lib/copilot/tools/server/table`. Existing tests must pass **unchanged**; needing to edit one means you changed behavior for the other types. -4. **`bun run lint`, `bun run check:api-validation`, `bun run check:client-boundary`** from the repo root. +3. **Run the suite** — `bun run --cwd apps/sim test lib/table 'app/workspace/[workspaceId]/tables' lib/api app/api/table app/api/v1`. Existing tests must pass **unchanged**; needing to edit one means you changed behavior for the other types. +4. **`bun run lint`, `bun run check:api-validation:strict`, `bun run check:client-boundary`** from the repo root. 5. **Exercise it in the running app** on a table with one column of every type: create, edit inline / in the expanded popover / in the row modal, paste from a spreadsheet, filter, sort, convert to and from other types, export CSV, undo a column delete. diff --git a/.agents/skills/add-connector/SKILL.md b/.agents/skills/add-connector/SKILL.md index c3ca20556a0..801ef6ee4fe 100644 --- a/.agents/skills/add-connector/SKILL.md +++ b/.agents/skills/add-connector/SKILL.md @@ -10,7 +10,7 @@ argument-hint: [api-docs-url] For **Sim Search**, use the live provider workflow in [the federated Search developer guide](../../../apps/sim/lib/sim-search/live/README.md#adding-a-live-search-connector). Its browser-safe provider catalog owns provider IDs, API origins, credential aliases, and account modes; its typed runtime registry requires both search and read handlers. `ConnectorMeta` remains the owner of logos and setup fields. Member mode has no admin resource filters. Service mode requires independent live source verification; resource pickers use shared selectors with canonical manual-input pairs, and plain inputs are fine where no picker applies. Do not implement a Search source by adding a crawler, embeddings, or a scheduled ACL build. -The ingestion instructions below apply to **ordinary knowledge-base connectors** and the explicit legacy Search backend (`SIM_SEARCH_LIVE=false`). If a provider supports both, implement and test both runtimes; adding `search: true` to metadata alone does not implement federated search. Preserve indexing documentation and behavior for those KB/legacy callers. +The ingestion instructions below apply to **ordinary knowledge-base connectors**. Sim Search always uses the live backend, so setting `search: true` in metadata does not implement federated search; a provider that serves both needs the live handlers and this KB ingestion path. You are an expert at adding knowledge base connectors to Sim. A connector syncs documents from an external source (Confluence, Google Drive, Notion, etc.) into a knowledge base. @@ -56,11 +56,7 @@ connectors/{service}/ Connectors use a discriminated union for auth config (`ConnectorAuthConfig` in `connectors/types.ts`): -```typescript -type ConnectorAuthConfig = - | { mode: 'oauth'; provider: OAuthService; requiredScopes?: string[] } - | { mode: 'apiKey'; label?: string; placeholder?: string } -``` +See `ConnectorAuthConfig` in `apps/sim/connectors/types.ts`: `oauth` takes `provider`, `requiredScopes`, and optional service-account/admin scopes; `apiKey` takes `label`, `placeholder`, and `optional`. ### OAuth mode For services with existing OAuth providers in `apps/sim/lib/oauth/types.ts`. The `provider` must match an `OAuthService`. The modal shows a credential picker and handles token refresh automatically. @@ -107,7 +103,7 @@ Keep `meta.ts` free of any server/runtime import. Only the icon, the `ConnectorM ```typescript import { createLogger } from '@sim/logger' -import { fetchWithRetry } from '@/lib/knowledge/documents/utils' +import { fetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server' import { {service}ConnectorMeta } from '@/connectors/{service}/meta' import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types' @@ -218,7 +214,7 @@ The user sees a toggle button (ArrowLeftRight) to switch between the selector dr 1. **Every selector field MUST have a canonical pair** — a corresponding `short-input` (or `dropdown`) field with the same `canonicalParamId` and `mode: 'advanced'`. 2. **`required` must be set identically on both fields** in a pair. If the selector is required, the manual input must also be required. -3. **`canonicalParamId` must match the key the connector expects in `sourceConfig`** (e.g. `baseId`, `channel`, `teamId`). The advanced field's `id` should typically match `canonicalParamId`. +3. **`canonicalParamId` must match the key the connector expects in `sourceConfig`** (e.g. `baseId`, `channel`, `teamId`). The advanced field's `id` should typically match `canonicalParamId` (connector config fields differ from block subBlocks here; the block rule that `canonicalParamId` must not equal a subblock id does not apply). 4. **`dependsOn` references the selector field's `id`**, not the `canonicalParamId`. The modal propagates dependency clearing across canonical siblings automatically — changing either field in a parent pair clears dependent children. ### Selector canonical pair example (Airtable base → table cascade) @@ -350,8 +346,8 @@ Every document returned from `listDocuments`/`getDocument` must include: title: string // Document title content: string // Extracted plain text (or '' if contentDeferred) contentDeferred?: boolean // true = content will be fetched via getDocument - mimeType: 'text/plain' // Always text/plain (content is extracted) - contentHash: string // Metadata-based hash for change detection + mimeType: 'text/plain' // extracted text; for a format the KB pipeline parses (PDF, Office), set `sourceFile` instead + contentHash: string // Change-detection hash (metadata-based when content is deferred) sourceUrl?: string // Link back to original (stored on document record) metadata?: Record // Source-specific data (fed to mapTags) } @@ -376,7 +372,7 @@ This pattern is critical for reliability: the sync engine processes documents in ### Content Hash Strategy -Use a **metadata-based** `contentHash` — never a content-based hash. The hash must be derivable from the list response metadata alone, so the sync engine can detect changes without downloading content. +Deferred-content connectors (`contentDeferred: true`) must use a **metadata-based** `contentHash` derivable from the list response alone, so the sync engine can detect changes without downloading content. Inline-content connectors may hash the content they already hold (`computeContentHash`). Good metadata hash sources: - `modifiedTime` / `lastModifiedDateTime` — changes when file is edited @@ -519,12 +515,13 @@ mapTags: (metadata: Record): Record => { ## External API Calls — Use `fetchWithRetry` -All external API calls must use `fetchWithRetry` from `@/lib/knowledge/documents/utils` instead of raw `fetch()`. This provides exponential backoff with retries on 429/502/503/504 errors. It returns a standard `Response` — all `.ok`, `.json()`, `.text()` checks work unchanged. +All external API calls must use `fetchWithRetry` from `@/lib/knowledge/documents/secure-fetch.server` instead of raw `fetch()`. It does not validate the host (on a direct outbound route it calls plain `fetch`), so use `secureFetchWithRetry` for user-controlled hosts. This provides exponential backoff with retries on 429/502/503/504 errors. It returns a standard `Response` — all `.ok`, `.json()`, `.text()` checks work unchanged. -For `validateConfig` (user-facing, called on save), pass `VALIDATE_RETRY_OPTIONS` to cap wait time at ~7s. Background operations (`listDocuments`, `getDocument`) use the built-in defaults (5 retries, ~31s max). +For `validateConfig` (user-facing, called on save), pass `VALIDATE_RETRY_OPTIONS` to cap wait time at ~7s. Background operations (`listDocuments`, `getDocument`) use the built-in defaults (5 retries within a 150s budget). ```typescript -import { VALIDATE_RETRY_OPTIONS, fetchWithRetry } from '@/lib/knowledge/documents/utils' +import { fetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server' +import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils' // Background sync — use defaults const response = await fetchWithRetry(url, { @@ -544,7 +541,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 (`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. +The engine reconciles deletions only when the listing is marked safe (`checkpoint.unsafe` in `lib/knowledge/connectors/listing-checkpoint.ts`): `syncContext.listingCapped`, `syncContext.listingTruncated`, `syncContext.reconciliationUnsafe`, `ExternalDocumentList.reconciliationSafe: false` (required for offset/unstable pagination), or a non-null `listingFailures` marks it unsafe. Anything absent from a safe listing 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) { @@ -564,14 +561,14 @@ The sync engine (`lib/knowledge/connectors/sync-engine.ts`) is connector-agnosti 1. Calls `listDocuments` with pagination until `hasMore` is false 2. Compares `contentHash` to detect new/changed/unchanged documents 3. Stores `sourceUrl` and calls `mapTags` on insert/update automatically -4. Handles soft-delete of removed documents +4. Tombstones documents absent from a reconciliation-safe listing and hard-deletes them when the next safe listing still omits them 5. Resolves access tokens automatically — OAuth tokens are refreshed, API keys are decrypted from the `encryptedApiKey` column 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_META_REGISTRY[connectorType].icon` (the client-safe registry) 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 for connector-sourced documents in the knowledge base table. 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. @@ -608,7 +605,7 @@ 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/slack/slack.ts` — list API returns message content inline, metadata-derived `contentHash` +- **OAuth + inline content**: `apps/sim/connectors/airtable/airtable.ts` — list API returns record fields inline; `listDocuments` and `getDocument` share `recordToDocument`, which hashes that content - **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 @@ -626,14 +623,11 @@ export const CONNECTOR_META_REGISTRY: ConnectorMetaRegistry = { - `selectorKey` exists in `apps/sim/lib/selectors/manifest.ts` - `dependsOn` references selector field IDs (not `canonicalParamId`) - Each projected dependency key is a `SelectorContextKey` allowed by the selector manifest - - Every remote key has one server attachment with credential provider binding and a reviewed - `fixed`, `credential-bound`, or `user-controlled` destination policy - - No connector selector adds a client provider module, browser token request, or selector-only - API route -- [ ] `listDocuments` handles pagination with metadata-based content hashes + - Validate the selector key itself with the `validate-selector` skill +- [ ] `listDocuments` handles pagination; deferred-content connectors use metadata-based content hashes - [ ] `syncContext.listingCapped = true` set whenever the listing is truncated (max-items cap or transient per-item error) — required to prevent the engine's deletion reconciliation from removing unseen documents - [ ] `contentDeferred: true` used if content requires per-doc API calls (file download, export, blocks fetch) -- [ ] `contentHash` is metadata-based (not content-based) and identical between stub and `getDocument` +- [ ] `contentHash` is metadata-based for deferred-content connectors (inline-content ones may use `computeContentHash`) and identical between stub and `getDocument` - [ ] `sourceUrl` set on each ExternalDocument (full URL, not relative) - [ ] `metadata` includes source-specific data for tag mapping - [ ] `tagDefinitions` declared for each semantic key returned by `mapTags` diff --git a/.agents/skills/add-enrichment/SKILL.md b/.agents/skills/add-enrichment/SKILL.md index 34810e72f41..32b1bad4417 100644 --- a/.agents/skills/add-enrichment/SKILL.md +++ b/.agents/skills/add-enrichment/SKILL.md @@ -16,7 +16,7 @@ Because enrichments run on Sim's hosted keys by default, **every provider tool y |------|------|-------| | 1 | Pick the data-source tool(s) for each output | `tools/{service}/` + `tools/registry.ts` | | 2 | **Verify each tool has `hosting`; if not, run `/add-hosted-key`** | `tools/{service}/{action}.ts` | -| 3 | Write the enrichment definition | `enrichments/{name}/{name}.ts` + `index.ts` | +| 3 | Write the enrichment definition | `enrichments/{id}/{id}.ts` + `index.ts` | | 4 | Register it | `enrichments/registry.ts` | | 5 | Verify | tsc / biome / manual run | @@ -27,7 +27,7 @@ Because enrichments run on Sim's hosted keys by default, **every provider tool y - **`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. +Outputs automatically become table columns; billing, the catalog/sidebar UI, and per-row execution work with no extra wiring. ## Step 1: Pick the data-source tool(s) @@ -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 entries registered in `enrichments/registry.ts`. +Create `apps/sim/enrichments/{id}/{id}.ts` and a barrel `index.ts`. Mirror the entries registered in `enrichments/registry.ts`. ```typescript import { SomeIcon } from '@sim/emcn/icons' @@ -104,7 +104,7 @@ export const myEnrichment: EnrichmentConfig = { ``` ```typescript -// apps/sim/enrichments/{name}/index.ts +// apps/sim/enrichments/{id}/index.ts export { myEnrichment } from './my-enrichment' ``` @@ -118,7 +118,7 @@ Rules: In `apps/sim/enrichments/registry.ts`, import and add the entry (catalog order is registration order): ```typescript -import { myEnrichment } from '@/enrichments/my-enrichment' +import { myEnrichment } from '@/enrichments/{id}' export const ENRICHMENT_REGISTRY: EnrichmentRegistry = { // ...existing diff --git a/.agents/skills/add-hosted-key/SKILL.md b/.agents/skills/add-hosted-key/SKILL.md index a90895d5a46..cf265339d2a 100644 --- a/.agents/skills/add-hosted-key/SKILL.md +++ b/.agents/skills/add-hosted-key/SKILL.md @@ -198,7 +198,7 @@ In the block config (`blocks/blocks/{service}.ts`), add `hideWhenHosted: true` t }, ``` -The visibility is controlled by `isSubBlockHidden()` in `lib/workflows/subblocks/visibility.ts`, which checks both the `isHosted` feature flag (`hideWhenHosted`) and optional env var conditions (`hideWhenEnvSet`). +The visibility is controlled by `isSubBlockHidden()` in `lib/workflows/subblocks/visibility.ts`, which checks both `getDeploymentShape().hosted` (`hideWhenHosted`) and optional env var conditions (`hideWhenEnvSet`). ### Excluding Specific Operations from Hosted Key Support @@ -251,6 +251,9 @@ Add an entry to the `PROVIDERS` array in the BYOK settings component so users ca }, ``` +Then add the id to exactly one section's `ids` in `PROVIDER_SECTIONS` (same file), and run +`bun run check:byok-providers`. + ## Step 6: Summarize Pricing and Throttling Comparison After all code changes are complete, output a detailed summary to the user covering: @@ -296,5 +299,6 @@ This summary helps reviewers verify that the pricing and rate limiting are well- - [ ] Cost data captured in `transformResponse` or `postProcess` if API provides it - [ ] `hideWhenHosted: true` added to the API key subblock in the block config - [ ] Provider entry added to the BYOK settings UI with icon and description +- [ ] Provider id listed in exactly one `PROVIDER_SECTIONS` section's `ids`; `bun run check:byok-providers` passes - [ ] Env vars documented: `{PREFIX}_COUNT` and `{PREFIX}_1..N` - [ ] Pricing and throttling summary provided to reviewer diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md index 865681fa09b..07c33c130b7 100644 --- a/.agents/skills/add-integration/SKILL.md +++ b/.agents/skills/add-integration/SKILL.md @@ -128,13 +128,11 @@ Three 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 + add a client provider fetcher, 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. +- Basic/advanced pairs use a `canonicalParamId`; its constraints are in + `.claude/rules/sim-integrations.md` and the `add-block` skill → canonicalParamId Pattern. - Every text-entry subBlock (`short-input`, `long-input`, `code`) and every selector declares a `placeholder`; an empty box tells the user nothing. Secrets read `Enter your {thing}` (e.g. `Enter your API key`), free text names what to type (`Enter branch name`), and formatted values @@ -215,7 +213,7 @@ import { } from '@/tools/{service}' // Add to tools object (alphabetically) -export const tools: Record = { +export const tools: Record = { // ... existing tools ... {service}_action1: {service}Action1Tool, {service}_action2: {service}Action2Tool, @@ -304,16 +302,11 @@ a resolvable capability must fail validation. ## Step 8: Generate and Validate the Catalog -Run the documentation generator: -```bash -bun run scripts/generate-docs.ts -bun run deployment-config:generate -bun run integration-catalog:check -bun run deployment-config:check -bun run docs:check -``` +Run `bun run tool-metadata:generate`, `bun run scripts/generate-docs.ts`, +`bun run deployment-config:generate`, then `bun run check:audits` (see the `validate-integration` +skill → Regenerate Derived Artifacts for the full list and what each check verifies). -This creates `apps/docs/content/docs/integrations/{service}.mdx` — one page per service carrying the block's Actions and, if it has one, its Triggers section. Never hand-edit generated pages; the only editable region is the `{/* MANUAL-CONTENT */}` block (see `scripts/README.md`). +The docs generator creates `apps/docs/content/docs/integrations/{service}.mdx` — one page per service carrying the block's Actions and, if it has one, its Triggers section. Never hand-edit generated pages; the only editable region is the `{/* MANUAL-CONTENT */}` block (see `scripts/README.md`). Every generated integration page carries a hand-written intro directly under ``. The generator preserves it across regenerations, so write it once after the first generate: @@ -392,7 +385,7 @@ If creating V2 versions (API-aligned outputs): ### Block - [ ] Created `blocks/blocks/{service}.ts` - [ ] Set `integrationType` to the correct `IntegrationType` enum value -- [ ] Set `tags` array with all applicable `IntegrationTag` values +- [ ] `{Service}BlockMeta.tags` lists every applicable `IntegrationTag` (tags live on the meta, not the block) - [ ] Defined operation dropdown with all operations - [ ] Added credential field with `requiredScopes: getScopesForService('{service}')` - [ ] Added conditional fields per operation @@ -415,7 +408,7 @@ If creating V2 versions (API-aligned outputs): ### OAuth Scopes (if OAuth service) - [ ] Defined scopes in `lib/oauth/oauth.ts` under `OAUTH_PROVIDERS` - [ ] Added scope descriptions in `SCOPE_DESCRIPTIONS` within `lib/oauth/utils.ts` -- [ ] Used `getCanonicalScopesForProvider()` in `auth.ts` (never hardcode) +- [ ] Used `getCanonicalScopesForProvider()` in `lib/auth/connectors/providers.ts` (never hardcode) - [ ] Used `getScopesForService()` in block `requiredScopes` (never hardcode) ### Deployment Availability (if OAuth service) @@ -504,7 +497,7 @@ Use the basic/advanced mode pattern: }, ``` -**Critical:** `canonicalParamId` must NOT match any subblock `id`. +**Critical:** `canonicalParamId` must NOT match the `id` of a subblock outside its canonical group. #### 2. Normalize File Input in Block Config @@ -548,51 +541,21 @@ Implement `apps/sim/lib/internal/{service}/execute-tool.ts` and keep the file/pr operations beside it. The handler validates `request.input`, derives storage authority only from trusted `request.context`, authorizes every stored file before reading bytes, forwards `request.signal`, enforces declared and actual byte caps, and returns the canonical tool response. -Register `{service}_upload` in `apps/sim/lib/internal/tool-operations/registry.server.ts` and add a -registry/direct-handler test. There is no HTTP fallback. +Register `{service}_upload` in `apps/sim/lib/internal/tool-operations/registry.server.ts`; the +sweep in `apps/sim/tools/request-transport.test.ts` fails a forgotten registration +(`registry.server.test.ts` checks registered ids are canonical with loadable handlers). For anything more, run the +`test-audit` gate. There is no HTTP fallback. ### File Output Pattern (Downloads) -For tools that return files, use `FileToolProcessor` to store files and return `UserFile` objects. - -#### In Tool transformResponse - -```typescript -import { FileToolProcessor } from '@/executor/utils/file-tool-processor' - -transformResponse: async (response, context) => { - const data = await response.json() - - // Process file outputs to UserFile objects - const fileProcessor = new FileToolProcessor(context) - const file = await fileProcessor.processFileData({ - data: data.content, // base64 or buffer - mimeType: data.mimeType, - filename: data.filename, - }) - - return { - success: true, - output: { file }, - } -} -``` - -#### In the operation handler (for complex file handling) +Declare a `file` / `file[]` output on the tool. For a raw binary endpoint, set +`request.responseType: 'binary'` and return `output.file = { name, mimeType, data: buffer, size }` +from `transformResponse(response, params?, context?)`. The executor's `FileToolProcessor` stores it +and replaces it with a `UserFile`; tools never call it. -```typescript -// Return file data that FileToolProcessor can handle. No API route is involved. -return Response.json({ - success: true, - output: { - file: { - data: base64Content, - mimeType: 'application/pdf', - filename: 'document.pdf', - }, - }, -}) -``` +In an operation handler, return `createInternalToolFileResult` / `createInternalToolFilesResult` +from `lib/internal/tool-operations/file-result.ts` — never base64 JSON. See the `add-tools` skill → +File Downloads and Generated Files. ### Key Helpers Reference @@ -601,7 +564,7 @@ return Response.json({ | `normalizeFileInput` | `@/blocks/utils` | Normalize file params in block config | | `processFilesToUserFiles` | `@/lib/uploads/utils/file-utils` | Convert raw inputs to UserFile[] | | `downloadFileFromStorage` | `@/lib/uploads/utils/file-utils.server` | Get file Buffer from UserFile | -| `FileToolProcessor` | `@/executor/utils/file-tool-processor` | Process tool output files | +| `FileToolProcessor` | `@/executor/utils/file-tool-processor` | Executor-side; stores declared file outputs (not called by tools) | | `isUserFile` | `@/lib/core/utils/user-file` | Type guard for UserFile objects | | `FileInputSchema` | `@/lib/uploads/utils/file-schemas` | Zod schema for file validation | @@ -636,13 +599,13 @@ Scopes are maintained in a single source of truth and reused everywhere: 1. **Define scopes** in `lib/oauth/oauth.ts` under `OAUTH_PROVIDERS[provider].services[service].scopes` 2. **Add descriptions** in `SCOPE_DESCRIPTIONS` within `lib/oauth/utils.ts` for the OAuth modal UI -3. **Reference in auth.ts** using `getCanonicalScopesForProvider(providerId)` from `@/lib/oauth/utils` +3. **Reference in `lib/auth/connectors/providers.ts`** (`buildConnectorProviders`) using `getCanonicalScopesForProvider(providerId)` from `@/lib/oauth/utils` 4. **Reference in blocks** using `getScopesForService(serviceId)` from `@/lib/oauth/utils` -**Never hardcode scope arrays** in `auth.ts` or block `requiredScopes`. Always import from the centralized source. +**Never hardcode scope arrays** in the Better Auth connector providers or block `requiredScopes`. Always import from the centralized source. ```typescript -// In auth.ts (Better Auth config) +// In lib/auth/connectors/providers.ts (Better Auth connector providers) scopes: getCanonicalScopesForProvider('{service}'), // In block credential sub-block diff --git a/.agents/skills/add-managed-cli/SKILL.md b/.agents/skills/add-managed-cli/SKILL.md index 0917a55a34b..c5bb3c9d0bf 100644 --- a/.agents/skills/add-managed-cli/SKILL.md +++ b/.agents/skills/add-managed-cli/SKILL.md @@ -43,7 +43,7 @@ Use `@-r`. - New upstream version: append a new ID ending in `-r1`. - Recipe-only change for the same upstream version: append `-r2`, `-r3`, and so on. - Never mutate or delete an existing ID or recipe. Persisted sandboxes must continue resolving to the bytes and behavior they selected. -- On upgrade, retain the old ID and recipe and set its metadata to `selectable: false`. Only the newest version keeps the public label selectable. +- On upgrade, retain the old ID and recipe and mark the old metadata non-selectable; only the newest version keeps the public label selectable. `SandboxCliToolMetadata` has no retirement field yet, so the first upgrade adds one generically (type, selector, and contract validation) per the next paragraph. Before shipping the first upgrade for a tool family, verify that editing a sandbox cannot leave both the retired and replacement IDs selected. If the generic selector and API validation do not already replace or reject colliding versions, address that once at the generic registry boundary with focused UI and contract tests; never special-case the individual CLI or silently install two versions that expose the same executable. @@ -123,7 +123,7 @@ From the repository root: ```bash bun run type-check -bun run check:api-validation +bun run check:api-validation:strict bunx biome check \ apps/sim/lib/execution/remote-sandbox/cli-tools.ts \ apps/sim/lib/execution/remote-sandbox/cli-tools.server.ts \ diff --git a/.agents/skills/add-model/SKILL.md b/.agents/skills/add-model/SKILL.md index c7630651b10..75721721c4b 100644 --- a/.agents/skills/add-model/SKILL.md +++ b/.agents/skills/add-model/SKILL.md @@ -48,8 +48,10 @@ Use a precise WebFetch prompt: *"Extract for {model_id}: exact model id string, | Capability | Honored by | Effect if set elsewhere | |---|---|---| | `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`, `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 | +| `toolUsageControl` | All providers (provider-level default) | Override per model only when that model differs | +| `forcedToolUse` | `anthropic/core.ts` (anthropic, azure-anthropic, kie); defaults to `toolUsageControl` | Ignored by every other provider; set `false` only on a model behind that core that cannot force tools | +| `promptCaching` | Caller-placed cache breakpoints | Set only where the vendor charges for opt-in caching (absent for OpenAI/Gemini implicit caching) | +| `reasoningEffort` | `openai/core.ts`, `azure-openai`, `xai`, `deepseek`, `groq`, `zai`, `kimi`, `cerebras`, `meta`, `litellm` (each `index.ts`) | Not read by anthropic/gemini (they use `thinking`) or by mistral, 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`; `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 | diff --git a/.agents/skills/add-permission-group-item/SKILL.md b/.agents/skills/add-permission-group-item/SKILL.md index 43ff11e4106..68ae7fba2a3 100644 --- a/.agents/skills/add-permission-group-item/SKILL.md +++ b/.agents/skills/add-permission-group-item/SKILL.md @@ -8,11 +8,11 @@ argument-hint: You are adding one governed item an organization admin can withhold from a cohort of members. One entry in `apps/sim/lib/permission-groups/fields.ts` produces the write schema, the read schema, the `PermissionGroupConfig` type, the defaults, the tolerant parser, and (for a boolean) the admin editor row. -**The registry does not produce enforcement.** Twelve keys once shipped with a checkbox, a hint, and no server check — an organization that ticked `hideCopilot` believed it had withheld a capability while every route still answered. Hence the `enforcement` field, the required `capability` field on every operation, and `scripts/check-permission-group-enforcement.ts`. You are done when something *refuses*, not when the key parses. +**The registry does not produce enforcement.** A key with an admin checkbox but no server gate gives an admin a restriction that does not exist. The `enforcement` field, the required `capability` field on every operation, and `scripts/check-permission-group-enforcement.ts` exist to prevent that. You are done when something *refuses*, not when the key parses. ## Read the system first -- `lib/permission-groups/fields.ts` — registry, three field builders, `permissionGroupConfigSchema`, `tolerantArray`, `parsePermissionGroupConfig`. There is **no `types.ts`** (folded in here); the DB constraint maps live in `constraints.ts` +- `lib/permission-groups/fields.ts` — registry, three field builders, `permissionGroupConfigSchema`, `tolerantArray`, `parsePermissionGroupConfig`. `PermissionGroupConfig` and its schemas are defined here; the DB constraint maps live in `constraints.ts` - `lib/permission-groups/capabilities.ts` — `CAPABILITY_IDS`, `CAPABILITY_RULES`, `capabilityRefusal`, `refuseCapability`, the static/parameterized split - `lib/permission-groups/capability-assertions.ts` — the sanctioned assertion API; re-exports `capabilityRefusal`. `capability-error.ts` holds the thrown error, `capability-response.ts` the raw-route 403 - `lib/permission-groups/integration-allowlist.ts` — the canonicalizing allowlist algebra, over the generated `block-successors.generated.ts` @@ -41,7 +41,7 @@ Allowlist when the safe posture is "only what the admin named" and the member se | `'executor'` | Read per block/tool/model at run time by `assertPermissionsAllowed` in `ee/access-control/utils/permission-check.ts`. Governs what a *run* may do, which no operation gate can express (one API call executes fifty blocks). Only `allowedIntegrations`, `allowedModelProviders`, `deniedModels`, `deniedTools` live here. The matching primitives these four keys are compared with live in `lib/permission-groups/` — `block-access.ts` (exemptions, superseded-version resolution), `operation-access.ts` (`createToolAccessGate`), `model-access.ts` (`createModelAccessGate`), `integration-allowlist.ts` — shared so the run-time gate and the editor/Copilot projections cannot drift. `allowedIntegrations` alone is also asserted outside a run, by `assertSelectorIntegrationAllowed` (`lib/selectors/server/integration-access.ts`) ahead of the provider call in `selectors.execute`, against the selector's own `resourceServiceId` / `integrationBlockTypes` rather than the credentials it accepts — reaching a provider API is a use of the integration, so a key here can still need a non-run enforcement site | | `'ui-only'` | Hides a surface without withholding it. **Almost never right** — nothing ships as `ui-only`. Justify in the `enforcement` comment why a determined caller reaching the data is acceptable, and expect review to question it | -**Is it per-operation at all?** `personal_api_key.use` is the one capability that is not: it withholds a *principal kind* across every operation, checked in the funnel's `personal_api_key` branch (`workspace-authorization.ts`) and again in `app/api/v1/middleware.ts`, so no operation declares it and its absence from every `capability:` field is correct rather than a hole. +**Is it per-operation at all?** `personal_api_key.use` and `oauth_apps.use` are principal-wide (`PRINCIPAL_WIDE_CAPABILITIES` in `lib/core/application/operation.ts`): they withhold a principal kind across every operation, checked in the funnel's `personal_api_key` / `oauth_access_token` branches (`workspace-authorization.ts`); `app/api/v1/middleware.ts` repeats only the `personal_api_key.use` check, since v1 accepts API keys, not OAuth tokens. Declaring one on an operation throws at definition time, so its absence from every `capability:` field is correct. **Is the decision knowable from the config alone?** A rule needing a request value (an auth mode, a connector id) is *parameterized* and cannot be declared on an operation — see Step 3. @@ -60,7 +60,7 @@ Allowlist when the safe posture is "only what the admin named" and the member se The second argument is the field's `feature` (`PlatformFeatureMeta`); `PLATFORM_FEATURES` spreads it and appends `configKey`, so those four values are what the editor renders. `PLATFORM_FEATURES` is *derived* from the registry in `features.ts`, so a boolean key cannot reach the config without reaching the editor. -- **Declaration order is the wire order** of `PermissionGroupConfig`, both zod schemas, and every config JSON crossing the API. `fields.test.ts` pins it with a key-order contract test, and `ee/access-control/components/group-detail.tsx` dirty-checks by comparing stringified configs — a moved key fails the suite *and* makes every open editor read as unsaved. Extend the tail; do not tidy the middle. +- **Declaration order is the wire order** of `PermissionGroupConfig`, both zod schemas, and every config JSON crossing the API, since all of them derive from the registry. Extend the tail; do not tidy the middle. - **The default must be the permissive value.** Every stored `permission_group.config` row predates your key; `parsePermissionGroupConfig` fills the gap from the default and the update route merges a partial write over the stored config, so a restrictive default silently applies a new restriction to every existing group in every enterprise org. The builders hardcode `false` / `null` / `[]`, so a new key must be *phrased* so the permissive value is falsy: a `requireWidgetApproval` whose safe default is `true` must be inverted before it can use `booleanRestriction`. - **The checkbox is inverted.** `group-detail.tsx` renders `checked={!editingConfig[feature.configKey]}` — ticked means *allowed*, so an `allowX` name renders backwards. - **The hint must describe access withheld, never a surface hidden.** A `'capability'` key refuses at the API; "Hide the Tables module from the sidebar" tells an admin they are tidying a nav bar while they revoke a module. The same string is read again by `getActivePermissionGroupRestrictions` in `features.ts` as the prose for an *active* restriction — reaching users through the Copilot workspace VFS and the enterprise platform context — where "hide" is simply false. Write "Revoke the Tables module. Members cannot read or write any table." `PlatformFeatureMeta.hint` carries the rule in its TSDoc. @@ -107,13 +107,13 @@ Capability ids are **domain-shaped** (`tables.create`); config keys are **surfac `configKeys` is what the audit reads to prove your key is enforced — it must list every key `deniedBy` reads. `describe` is the subject of one shared sentence, `" is not available under your organization's permission group"`, so make it a singular noun or gerund that agrees with "is". Exactly two functions build it, both defined in `capabilities.ts`: `refuseCapability(cap)` throws it as a `PermissionGroupCapabilityError`; `capabilityRefusal(cap)` returns it as a string for a raw route rendering its own body (`capability-assertions.ts` re-exports it so an inline gate reaches both through one module). Never write the sentence at a call site. -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. +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 needs a TSDoc-documented entry in the closed `FORBIDDEN_DETAIL_CODES` tuple, which is published to OpenAPI as the `V2ForbiddenDetailCode` enum via `v2ForbiddenDetailCodeSchema`. 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** (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. +`capability` is **required on the `ApplicationOperation` base type** (the `capability` field in `lib/core/application/operation.ts`), typed `OperationDeclarableCapability | '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: @@ -176,9 +176,9 @@ Always route through `CAPABILITY_RULES` and raise with `refuseCapability` — a | `TableAccessPrincipal` | `capabilityGovernedUserId(principal)` (`app/api/table/utils.ts`) | | `AuthResult` from `checkSessionOrInternalAuth` | `capabilityGovernedAuthUserId` (same file) — an internal JWT's `userId` is the run's actor, a bystander | -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. +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 lets producers silently use `triggeredByUserId` (an *attribution* naming the billed account) as the subject; 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. +- **`/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 subjects only; other surfaces rely on the `capabilityGoverned*` helpers. - **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. @@ -186,7 +186,7 @@ When the subject is **persisted and read back later** — the table dispatch pip Add the key to **both** the `input` and `expected` objects of the `'a fully populated config'` fixture in `lib/permission-groups/fields.test.ts`, set to a non-default value. That fixture is the pinned coercion corpus: a row that changes in a later diff is a semantic decision someone defends rather than a silent regression. The file's other assertions derive from `DEFAULT_PERMISSION_GROUP_CONFIG` (wire order, idempotence, read-schema acceptance, the 2000-iteration seeded fuzz, write/default/read key-set agreement, boolean-to-`PLATFORM_FEATURES` coverage) and pick your key up for free, as does `features.test.ts`. -**Give the funnel test a real `workspaceOrganizationId`.** `requireCapability` short-circuits on `context.workspaceOrganizationId === null` (`lib/core/application/workspace-authorization.ts:204`), so a fixture whose workspace context leaves it null passes with the gate present *and* with it removed — a vacuous test that reads as load-bearing. +**Give the funnel test a real `workspaceOrganizationId`.** `requireCapability` short-circuits on `context.workspaceOrganizationId === null` (`requireCapability` in `lib/core/application/workspace-authorization.ts`), so a fixture whose workspace context leaves it null passes with the gate present *and* with it removed — a vacuous test that reads as load-bearing. Add a case to `capabilities.test.ts` for any rule with logic beyond reading one key. For an allowlist assert all three states — `null` permits every member, a populated list only the named ones, `[]` permits **none** — as `capabilities.test.ts` already does for `knowledge.connectors`. @@ -197,11 +197,11 @@ Add a case to `capabilities.test.ts` for any rule with logic beyond reading one | Guarded root | Forbidden | |---|---| | `lib/core/application/index.ts`, and `lib/permission-groups/` `capabilities.ts` / `capability-assertions.ts` / `config-scope.server.ts` | `providers/`, `blocks/`, `tools/`, `executor/`, `lib/uploads/`, `lib/workflows/` | -| `lib/core/utils/with-route-handler.ts` | those six **plus** `lib/billing/`, `lib/permission-groups/resolve.server`, `lib/auth`, `lib/copilot/`, `lib/knowledge/` | +| `lib/core/utils/with-route-handler.ts` | those six **plus** `lib/billing/`, `lib/permission-groups/resolve.server`, `lib/auth`, `lib/mothership/`, `lib/knowledge/` | `lib/billing/` stays allowed for the funnel roots because `resolve.server.ts` legitimately reads the subscription to decide whether an organization is on an enterprise plan; the wrapper is a lifecycle shim that opens the memo scope and nothing more. That split is why the scope is two files. -Breaking this never announces itself — past regressions surfaced only as unrelated tests failing on partial mocks of modules they never meant to load. After adding an import, run this audit first. +A broken edge shows up only indirectly (unrelated tests failing on partial mocks), so run this audit after adding any import. ## Step 7: Verify @@ -213,7 +213,7 @@ cd apps/sim && bun run type-check bun run --cwd apps/sim test 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. +Also `bun run check:api-validation:strict` if you touched a contract or the group routes. `bun run check:audits` runs every `check:*` command here (including the `:strict` variant) but not type-check or the tests; 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 — 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: @@ -229,7 +229,7 @@ The audits prove *reachability*: your capability is named somewhere, your key is ## Traps -**An operation carries exactly ONE capability, and a narrower capability must subsume the broader one it replaced.** `knowledge.create` and `knowledge.upload` list `configKeys: ['disableKnowledgeBaseCreation', 'hideKnowledgeBaseTab']` and OR both in `deniedBy`, because moving KB creation off `knowledge.use` would otherwise let a group that withheld the whole module still create one through the API. Any time you re-point an operation to a more specific capability, the specific rule must read both keys. +**An operation carries exactly ONE capability, and a narrower capability must subsume the broader one it replaced.** `knowledge.create` (`disableKnowledgeBaseCreation`) and `knowledge.upload` (`disableKnowledgeBaseFileUpload`) each also list and OR `hideKnowledgeBaseTab` in `deniedBy`, because moving KB creation off `knowledge.use` would otherwise let a group that withheld the whole module still create one through the API. Any time you re-point an operation to a more specific capability, the specific rule must read both keys. **`.catch()` on an array field is a fail-open security bug.** `z.array(item).catch(fallback)` is whole-value tolerant: one bad member discards every good one. On an allowlist the fallback is `null`, and `null` means **unrestricted** — a partly corrupt allowlist stops restricting anything. `tolerantArray` in `fields.ts` filters element by element, keeping what parses and failing closed. Never swap it for `.catch()`, never hand-roll a parallel coercion path. diff --git a/.agents/skills/add-selector/SKILL.md b/.agents/skills/add-selector/SKILL.md index 3c4f0099f9d..4f3f6d40082 100644 --- a/.agents/skills/add-selector/SKILL.md +++ b/.agents/skills/add-selector/SKILL.md @@ -76,9 +76,9 @@ Choose the destination policy deliberately: - `user-controlled`: the user selects the destination. Hidden use-only authentication requires an explicit security policy; do not combine it with an arbitrary destination by default. -Reuse or extract a server-only provider listing primitive. If an existing provider route has -non-selector callers, keep the route as a thin caller of that primitive. If it is selector-only, -move the logic and remove the obsolete route and contract. Never import a route handler or make an +Reuse or extract a server-only provider listing primitive. A provider route exists only when it +has a non-selector caller, and then it is a thin caller of that primitive; there are no +selector-only routes or contracts. Never import a route handler or make an internal HTTP request from an attachment. The attachment must return normalized selector results only. It must never deliberately or @@ -98,7 +98,7 @@ Keep connector selector/manual canonical pairs and fork reconfiguration behavior Do not add: -- A module under `hooks/selectors/providers` or any client provider fetcher. +- A client provider fetcher. - A provider-specific React Query key. - A selector-specific OAuth-token request. - A selector-only API route when the provider primitive can be called directly. diff --git a/.agents/skills/add-settings-page/SKILL.md b/.agents/skills/add-settings-page/SKILL.md index 9500ac39c73..c59c187e738 100644 --- a/.agents/skills/add-settings-page/SKILL.md +++ b/.agents/skills/add-settings-page/SKILL.md @@ -21,65 +21,58 @@ Key paths: ## Mode A — Add a new settings page -1. **Navigation.** In `navigation.ts`: add the id to the `SettingsSection` union, - then a `NavigationItem` with `label` AND a one-line `description` (verb-first, - ~40–55 chars, product voice per `.claude/rules/constitution.md`). Place it in - the right `section` group and set any gating flags (`requiresHosted`, - `requiresEnterprise`, etc.). -2. **Wire the switch.** Add the component to the `effectiveSection` render switch - in `settings/[section]/settings.tsx` (lazy `dynamic(...)` like its siblings). +1. **Navigation.** In `navigation.ts`: add the id to `UnifiedSettingsSection`, then a + `SETTINGS_SECTION_REGISTRY` entry with `label`, `icon`, and + `unified: { id, description, group, order }` (description verb-first, ~40–55 chars, + product voice per `.claude/rules/constitution.md`). Set gating flags + (`requiresHosted`, `requiresEnterprise`, …) on `unified`, and add `planes` only if the + section also exists on a standalone plane. +2. **Wire the switch.** Register the module in `SECTION_MODULES` + (`settings/section-warmers.ts`), then add + `const X = dynamic(() => SECTION_MODULES.x().then((m) => m.X))` and its case in the + `effectiveSection` switch in `settings/[section]/settings.tsx`. 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**. - Detail sub-views additionally route the back chip through - `guard.guardBack(closeFn)` and render the shared `UnsavedChangesModal`. Never - hand-roll a Save button, a `beforeunload`, or an "Unsaved changes" modal — - they're centralized. See the "Save / Discard + unsaved-changes guard" section - in `.claude/rules/sim-settings-pages.md`. -5. **Verify:** `cd apps/sim && bun run type-check`; `bunx biome check --write `. +4. **If the page has editable state**, wire the shared save/discard stack exactly as in + `.claude/rules/sim-settings-pages.md` "Save / Discard + unsaved-changes guard" + (`saveDiscardActions()`, `useSettingsUnsavedGuard` before any early return). +5. **Verify:** the local gate in the root `CLAUDE.md` ("How your work is checked"). ## Mode B — Audit existing settings pages For each page component, confirm the checklist in `.claude/rules/sim-settings-pages.md`: +Each grep lists candidates; review every match against the expected ones named below. + 1. Find hand-rolled shells that should be `SettingsPanel`: - `git grep -n "flex h-full flex-col bg-\[var(--bg)\]" -- 'apps/sim/**/settings/' 'apps/sim/ee/'` - — every match should be either `settings-panel.tsx`, a **detail sub-view** - (has a `` back button), or an entitlement/loading - **gate** early-return. Anything else is a page that still needs migrating. -2. Find hand-rolled title blocks (should be 0 outside detail views): - `git grep -n "text-\[var(--text-body)\] text-lg" -- 'apps/sim/**/settings/' 'apps/sim/ee/'` -3. Find literal pixel text sizes (should be 0 — see "Text-scale tokens" in - `.claude/rules/sim-settings-pages.md` for the token map and the row - title/subtitle pairing convention): - `git grep -nE "text-\[1[0-8]px\]" -- 'apps/sim/**/settings/' 'apps/sim/ee/'` — should - be 0. Display type above the scale (`text-[40px]` hero headings) is deliberate - and out of scope. -4. Confirm each page imports `SettingsPanel` and that its `NavigationItem` has an + `git grep -n "flex h-full flex-col bg-\[var(--bg)\]" -- 'apps/sim/**/settings/**' 'apps/sim/ee/'` + — expected matches: the workspace and organization `settings/layout.tsx` shells, the shared + header shell (`components/settings/settings-header.tsx`), `CredentialDetailLayout` (the + `settings/secrets/[credentialId]` exception), or an entitlement/loading gate. A detail + sub-view is never a match: it passes `back={{ text, icon: ArrowLeft, onSelect }}` to + `SettingsPanel`. Anything else is a violation: render it through `SettingsPanel`. +2. Find hand-rolled title blocks: + `git grep -n "text-\[var(--text-body)\] text-lg" -- 'apps/sim/**/settings/**' 'apps/sim/ee/'` + — the only title is the `

` in `settings-header.tsx`; a non-heading value at that size + (e.g. the credit balance in `ee/organization-usage/components/usage-credits.tsx`) is fine. +3. Find literal pixel text sizes (should be 0 — see "Text Scale" in + `.claude/rules/sim-styling.md`): + `git grep -nE "text-\[1[0-8]px\]" -- 'apps/sim/**/settings/**' 'apps/sim/ee/'`. + Display type above the scale (`text-[40px]` hero headings) is deliberate and out of + scope. +4. Confirm each page imports `SettingsPanel` and that its registry entry has an accurate `description` of consistent length with its peers. - - Editable pages: confirm Save/Discard go through `SaveDiscardActions` and + - Editable pages: confirm Save/Discard go through `saveDiscardActions()` and dirty is wired via `useSettingsUnsavedGuard` (called before early-return gates) — flag any hand-rolled Save button, `beforeunload`, or unsaved modal. - `git grep -n "beforeunload" -- 'apps/sim/**/settings/' 'apps/sim/ee/'` + `git grep -n "beforeunload" -- 'apps/sim/**/settings/**' 'apps/sim/ee/'` should only hit the centralized `use-settings-before-unload.ts`. -5. When migrating a page, change ONLY the structural shell→`SettingsPanel` swap: - move header chips to `actions`, the standalone search to `search`, delete the - `

` title block, replace the three closing `` (column/scroll/shell) - with ``, and keep modal siblings in a `<>` fragment. Do NOT - touch handlers, state, queries, conditional rendering, or detail/gate returns. - Drop per-page `gap-*`/`pt-*` on the content column in favor of the panel default. -6. When fixing literal pixel text sizes, replace ONLY the size class with its - exact-pixel-equivalent named token (e.g. `text-[12px]` → `text-caption`, - never a different size) — this must render pixel-identical, not restyle the - page. Leave color tokens (`--text-primary` vs `--text-body`, etc.) untouched - unless they're also being changed for an unrelated, deliberate reason. -7. Remove now-unused imports (`ChipInput`/`Search`) ONLY after grepping that - they are not still used elsewhere in the file (e.g. by a detail view). -8. **Verify the whole sweep:** `bun run type-check`, `biome check` on every touched - file, and run the affected pages' tests. Diff each file against the base and - confirm the change is purely structural before shipping. +5. Fix each finding with the smallest structural change that satisfies the checklist; + do not touch handlers, state, queries, or gate returns. A pixel-size fix swaps + only the size class for its exact-pixel token (`text-[12px]` → `text-caption`). +6. **Verify the whole sweep:** the local gate in the root `CLAUDE.md`, plus the + affected pages' tests. Diff each file against the base and confirm the change is + purely structural before shipping. ## Mode C — Migrate list rows to `SettingsResourceRow` @@ -91,20 +84,13 @@ contract. Then, per page: Every match outside `settings-resource-row.tsx` is either a row to migrate or a genuinely different shape (multi-line body, tabular columns, a grid) that stays bespoke — decide which, and say so. -2. Replace the row *and* its wrapper: a `