Skip to content

Commit 6fe9f17

Browse files
committed
docs(agents): fix stale facts in connector, model, column-type, and selector skills
Verified against the code: fetchWithRetry moved to secure-fetch.server, deletion reconciliation is checkpoint.unsafe (shouldReconcileDeletions is gone), the legacy Search toggle is removed, inline-content connectors may hash content, reasoningEffort/thinking provider lists, column-type metadata update path, check:api-validation:strict as the gate. CLAUDE.md now names the coerceValue switch and relative barrel re-exports as the documented exceptions the code relies on. Selector rules in connector skills point at validate-selector instead of restating it.
1 parent 66ab4f7 commit 6fe9f17

13 files changed

Lines changed: 52 additions & 95 deletions

File tree

‎.agents/skills/add-block-preview/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ To pull an already-GA block from discovery surfaces on hosted (incident, depreca
5353
- **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.
5454
- **Keys are registry block types.** Never `custom_block_*` (parse drops them — custom blocks have their own enabled/disabled lifecycle).
5555
- **The shared hidden-predicate is `isHiddenUnder`** (`apps/sim/blocks/visibility/context.ts`). Never restate the preview/disabled rule inline at a new consumer.
56-
- **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.
56+
- **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.
5757
- Gating is **surface hiding, not secrecy** — the full config ships in the client JS bundle. Anything truly secret cannot be a registered block.
5858

5959
## Tests

‎.agents/skills/add-column-type/SKILL.md‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ A `case 'yourtype':` outside `column-types/` fails **silently** when missed (a w
1212

1313
## Hard Rule: the compiler tells you what to do
1414

15-
Do **not** hunt for places to edit. Add your type to the `ColumnType` union first and let `tsc` produce the list:
15+
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:
1616

1717
```bash
1818
cd apps/sim && bun run type-check
@@ -86,7 +86,7 @@ export function Type{Pascal}(props: SVGProps<SVGSVGElement>) {
8686

8787
## Step 3: Write the type file
8888

89-
`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.
89+
`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.
9090

9191
The three that are easy to get wrong:
9292

@@ -132,18 +132,18 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not,
132132
| `column-types/types.ts` `TYPE_SPECIFIC_COLUMN_KEYS` | it is never stripped on conversion, and poisons the target type |
133133
| `lib/api/contracts/tables.ts` — the schema slot in all three column schemas, plus `refineColumnOptions` | zod strips it at the boundary; silently never saved |
134134
| `columns/service.ts` `addTableColumn` param type | callers cannot pass it |
135-
| 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 |
135+
| A metadata-only update in `lib/table/columns/service.ts` (`updateColumnCurrency` is the model) + a branch in `updateColumn` in `lib/table/orchestration/columns.ts` | changing it on an existing column is a silent 200 no-op |
136136
| `column-config-sidebar.tsx` | no UI to set it |
137137
| `table-grid.tsx` delete-column undo + `use-table-undo.ts` restore | undo silently resets it to the default |
138138

139139
`normalizeColumn`, `buildConvertedColumn`, and the undo snapshot read `TYPE_SPECIFIC_COLUMN_KEYS` generically, so those three are already zero-edit.
140140

141-
**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.
141+
Copy `currency`'s service function and orchestration branch.
142142

143143
## Checklist Before Finishing
144144

145-
- [ ] Added to the `ColumnType` union in `column-types/types.ts`
146-
- [ ] `column-types/{id}.ts` created, every interface field filled in
145+
- [ ] Id appended to `COLUMN_TYPES` in `column-types/types.ts`
146+
- [ ] `column-types/{id}.ts` created, every required field filled in
147147
- [ ] Registered in **both** `registry.ts` and `registry.server.ts`
148148
- [ ] Icon added, centered on the family's optical center, exported alphabetically
149149
- [ ] `migrateCellsTo` / `migrateCellsFrom` added if the stored bytes change
@@ -155,6 +155,6 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not,
155155

156156
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.
157157
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.
158-
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.
159-
4. **`bun run lint`, `bun run check:api-validation`, `bun run check:client-boundary`** from the repo root.
158+
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.
159+
4. **`bun run lint`, `bun run check:api-validation:strict`, `bun run check:client-boundary`** from the repo root.
160160
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.

‎.agents/skills/add-connector/SKILL.md‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ argument-hint: <service-name> [api-docs-url]
1010

1111
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.
1212

13-
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.
13+
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.
1414

1515
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.
1616

@@ -56,11 +56,7 @@ connectors/{service}/
5656

5757
Connectors use a discriminated union for auth config (`ConnectorAuthConfig` in `connectors/types.ts`):
5858

59-
```typescript
60-
type ConnectorAuthConfig =
61-
| { mode: 'oauth'; provider: OAuthService; requiredScopes?: string[] }
62-
| { mode: 'apiKey'; label?: string; placeholder?: string }
63-
```
59+
See `ConnectorAuthConfig` in `apps/sim/connectors/types.ts`: `oauth` takes `provider`, `requiredScopes`, and optional service-account/admin scopes; `apiKey` takes `label`, `placeholder`, and `optional`.
6460

6561
### OAuth mode
6662
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
107103

108104
```typescript
109105
import { createLogger } from '@sim/logger'
110-
import { fetchWithRetry } from '@/lib/knowledge/documents/utils'
106+
import { fetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
111107
import { {service}ConnectorMeta } from '@/connectors/{service}/meta'
112108
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
113109

@@ -218,7 +214,7 @@ The user sees a toggle button (ArrowLeftRight) to switch between the selector dr
218214

219215
1. **Every selector field MUST have a canonical pair** — a corresponding `short-input` (or `dropdown`) field with the same `canonicalParamId` and `mode: 'advanced'`.
220216
2. **`required` must be set identically on both fields** in a pair. If the selector is required, the manual input must also be required.
221-
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`.
217+
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).
222218
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.
223219

224220
### Selector canonical pair example (Airtable base → table cascade)
@@ -350,7 +346,7 @@ Every document returned from `listDocuments`/`getDocument` must include:
350346
title: string // Document title
351347
content: string // Extracted plain text (or '' if contentDeferred)
352348
contentDeferred?: boolean // true = content will be fetched via getDocument
353-
mimeType: 'text/plain' // Always text/plain (content is extracted)
349+
mimeType: 'text/plain' // extracted text; for a format the KB pipeline parses (PDF, Office), set `sourceFile` instead
354350
contentHash: string // Metadata-based hash for change detection
355351
sourceUrl?: string // Link back to original (stored on document record)
356352
metadata?: Record<string, unknown> // Source-specific data (fed to mapTags)
@@ -376,7 +372,7 @@ This pattern is critical for reliability: the sync engine processes documents in
376372

377373
### Content Hash Strategy
378374

379-
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.
375+
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`).
380376

381377
Good metadata hash sources:
382378
- `modifiedTime` / `lastModifiedDateTime` — changes when file is edited
@@ -519,12 +515,13 @@ mapTags: (metadata: Record<string, unknown>): Record<string, unknown> => {
519515

520516
## External API Calls — Use `fetchWithRetry`
521517

522-
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.
518+
All external API calls must use `fetchWithRetry` from `@/lib/knowledge/documents/secure-fetch.server` (SSRF-guarded) instead of raw `fetch()`; 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.
523519

524-
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).
520+
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).
525521

526522
```typescript
527-
import { VALIDATE_RETRY_OPTIONS, fetchWithRetry } from '@/lib/knowledge/documents/utils'
523+
import { fetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
524+
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
528525

529526
// Background sync — use defaults
530527
const response = await fetchWithRetry(url, {
@@ -544,7 +541,7 @@ If `ExternalDocument.sourceUrl` is set, the sync engine stores it on the documen
544541

545542
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.
546543

547-
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.
544+
The engine reconciles deletions only when the listing is marked safe (`checkpoint.unsafe` in `lib/knowledge/connectors/listing-checkpoint.ts`): `syncContext.listingCapped`, `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.
548545

549546
```typescript
550547
if (hitLimit && syncContext) {
@@ -564,14 +561,14 @@ The sync engine (`lib/knowledge/connectors/sync-engine.ts`) is connector-agnosti
564561
1. Calls `listDocuments` with pagination until `hasMore` is false
565562
2. Compares `contentHash` to detect new/changed/unchanged documents
566563
3. Stores `sourceUrl` and calls `mapTags` on insert/update automatically
567-
4. Handles soft-delete of removed documents
564+
4. Tombstones documents absent from a reconciliation-safe listing and hard-deletes them when the next safe listing still omits them
568565
5. Resolves access tokens automatically — OAuth tokens are refreshed, API keys are decrypted from the `encryptedApiKey` column
569566

570567
You never need to modify the sync engine when adding a connector.
571568

572569
## Icon
573570

574-
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.
571+
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.
575572

576573
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.
577574

@@ -608,7 +605,7 @@ export const CONNECTOR_META_REGISTRY: ConnectorMetaRegistry = {
608605
- **OAuth + contentDeferred**: `apps/sim/connectors/google-drive/google-drive.ts` — file download with metadata-based hash, `orderBy` for deterministic pagination
609606
- **OAuth + contentDeferred (blocks API)**: `apps/sim/connectors/notion/notion.ts` — complex block content extraction deferred to `getDocument`
610607
- **OAuth + contentDeferred (git)**: `apps/sim/connectors/github/github.ts` — blob SHA hash, tree listing
611-
- **OAuth + inline content**: `apps/sim/connectors/slack/slack.ts` — list API returns message content inline, metadata-derived `contentHash`
608+
- **OAuth + inline content**: `apps/sim/connectors/slack/slack.ts` — list API returns message content inline; `contentHash` hashes that content
612609
- **OAuth + contentDeferred + config fields**: `apps/sim/connectors/confluence/confluence.ts` — multiple config field types, `mapTags`, label fetching
613610
- **API key**: `apps/sim/connectors/fireflies/fireflies.ts` — GraphQL API with Bearer token auth
614611

@@ -626,11 +623,8 @@ export const CONNECTOR_META_REGISTRY: ConnectorMetaRegistry = {
626623
- `selectorKey` exists in `apps/sim/lib/selectors/manifest.ts`
627624
- `dependsOn` references selector field IDs (not `canonicalParamId`)
628625
- Each projected dependency key is a `SelectorContextKey` allowed by the selector manifest
629-
- Every remote key has one server attachment with credential provider binding and a reviewed
630-
`fixed`, `credential-bound`, or `user-controlled` destination policy
631-
- No connector selector adds a client provider module, browser token request, or selector-only
632-
API route
633-
- [ ] `listDocuments` handles pagination with metadata-based content hashes
626+
- Validate the selector key itself with the `validate-selector` skill
627+
- [ ] `listDocuments` handles pagination; deferred-content connectors use metadata-based content hashes
634628
- [ ] `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
635629
- [ ] `contentDeferred: true` used if content requires per-doc API calls (file download, export, blocks fetch)
636630
- [ ] `contentHash` is metadata-based (not content-based) and identical between stub and `getDocument`

‎.agents/skills/add-enrichment/SKILL.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Because enrichments run on Sim's hosted keys by default, **every provider tool y
2727
- **`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.
2828
- **`enrichments/registry.ts`** — `ENRICHMENT_REGISTRY` / `ALL_ENRICHMENTS` / `getEnrichment`. Register new entries here.
2929

30-
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.
30+
Outputs automatically become table columns; billing, the catalog/sidebar UI, and per-row execution work with no extra wiring.
3131

3232
## Step 1: Pick the data-source tool(s)
3333

@@ -118,7 +118,7 @@ Rules:
118118
In `apps/sim/enrichments/registry.ts`, import and add the entry (catalog order is registration order):
119119

120120
```typescript
121-
import { myEnrichment } from '@/enrichments/{id}/{id}'
121+
import { myEnrichment } from '@/enrichments/{id}'
122122

123123
export const ENRICHMENT_REGISTRY: EnrichmentRegistry = {
124124
// ...existing

0 commit comments

Comments
 (0)