diff --git a/.agents/skills/add-block-preview/SKILL.md b/.agents/skills/add-block-preview/SKILL.md index c3633e8dbc6..6bc91f76219 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.** 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. +- **Process-global caches stay ungated.** Shared builders such as `getExposedIntegrationTools` (`apps/sim/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 c8a83361a6b..877472ce3c5 100644 --- a/.agents/skills/add-block/SKILL.md +++ b/.agents/skills/add-block/SKILL.md @@ -310,7 +310,7 @@ When several fields are mutually exclusive alternatives, mark them all `required other paths ever get a chance to supply the value. **Constraints (block-wide):** -- `canonicalParamId` must not equal any subblock `id` in the block. +- `canonicalParamId` may equal only the `id` of a member of its own group, as `channel` does in the canonicalParamId Pattern below; it must never equal any other subblock's `id`. (`blocks.test.ts` enforces the case of a subblock with no `canonicalParamId`.) - One canonical id links exactly one basic/advanced pair for one logical parameter. Groups are keyed by canonical id across every subblock and hold one `basicId`, so two operations that each need a pair need two canonical ids. - All members of a group share the same `required` status. diff --git a/.agents/skills/add-connector/SKILL.md b/.agents/skills/add-connector/SKILL.md index 801ef6ee4fe..d856462fb9c 100644 --- a/.agents/skills/add-connector/SKILL.md +++ b/.agents/skills/add-connector/SKILL.md @@ -214,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` (connector config fields differ from block subBlocks here; the block rule that `canonicalParamId` must not equal a subblock id does not apply). +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 the id of a subblock without a `canonicalParamId` 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) diff --git a/.agents/skills/add-settings-page/SKILL.md b/.agents/skills/add-settings-page/SKILL.md index c59c187e738..5cddb31b5a9 100644 --- a/.agents/skills/add-settings-page/SKILL.md +++ b/.agents/skills/add-settings-page/SKILL.md @@ -46,11 +46,12 @@ Each grep lists candidates; review every match against the expected ones named b 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/'` - — 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`. + — expected matches: the workspace and organization `settings/layout.tsx` shells and the + shared header shell (`components/settings/settings-header.tsx`); an entitlement/loading gate + is also fine. `CredentialDetailLayout` (the `settings/secrets/[credentialId]` exception) is + an exempt hand-rolled shell outside these pathspecs. 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 diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index a8f44bdf75d..853124a32c9 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -88,9 +88,10 @@ conditions freshly after every push. across all pages has `isResolved: true`, and every check has finished and passed, stop — report the outcome (see "Reporting" below) and skip the rest of this list. -2. **If the PR has a merge conflict**, resolve it with step 6 (its rebase-based sync flow and - the `/ship` gates; a merge commit would be discarded by that rebase), then steps 7–8: push - with `--force-with-lease` and re-trigger review. +2. **If the PR has a merge conflict**, rebase rather than merge (step 6's rebase would discard a + merge commit): `git fetch origin staging && git rebase origin/staging`, resolve each conflict + and `git rebase --continue` until the rebase finishes. Then run step 6 (the sync check and the + `/ship` gates), then steps 7–8: push with `--force-with-lease` and re-trigger review. 3. **If no review has run yet** (fresh PR, no bot comments): both run automatically on PR open — confirm via `gh pr checks ` (look for `Greptile Review` and `cubic · AI code reviewer`) and diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index 66b722b81a8..fdd7e52ad4f 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -45,7 +45,7 @@ When the user runs `/ship`: - Then run root `bun run test` from the repo root. It chains `test:scripts` (the `scripts/*.test.ts` suite CI runs) before every workspace suite; workspace-scoped runs skip it, which is how a `scripts/check-*.test.ts` failure has reached CI. A failing test aborts ship. 5. **Run migration safety** — only if the diff touches `packages/db/migrations/**` or `packages/db/schema.ts`: - Run `/db-migrate` to review the migration for zero-downtime safety (expand/contract phasing, backward-compatibility with the deployed app version). - - `cd packages/db && bunx drizzle-kit generate && git status --porcelain ./migrations` must print nothing (CI's schema/migration sync step). + - `(cd packages/db && bunx drizzle-kit generate && git status --porcelain ./migrations)` must print nothing (CI's schema/migration sync step). - `bun run check:migrations origin/staging` must pass (staging is the PR base). Do not silence a flagged statement with a `-- migration-safe:` annotation unless `/db-migrate` confirmed the old code no longer depends on it; otherwise split the destructive change into a later deploy. 6. **Run pre-ship checks** from the repo root before staging. This has two phases: first **regenerate** every committed artifact so generated files never drift into a CI failure (this is what catches things like `agent-stream-docs` going stale after a `models.ts` edit), then run the **full audit suite** CI's `Lint and Test` job enforces. Both phases parallelize — but only across commands that write **disjoint** outputs — and a bare `wait` swallows child exit codes, so both phases below explicitly collect each job's status and abort ship if any failed. @@ -78,12 +78,13 @@ When the user runs `/ship`: # Runs every audit CI runs, concurrently, and replays the output of any that fail. # The audit list is derived in scripts/run-audits.ts — do not hand-list audits here. bun run check:audits || { echo "❌ audit(s) failed — do not ship"; exit 1; } + bun run type-check || { echo "❌ type-check failed — do not ship"; exit 1; } # CI's "Verify docs manifest is in sync" step is not a `check:*` script, so the runner above # does not cover it. (CI's "Security audit" `bun audit` step is `continue-on-error` — advisory # only, not a gate — so it is deliberately not run here.) bun run docs-manifest:check || { echo "❌ docs manifest out of sync — do not ship"; exit 1; } ``` - If Phase A regenerated a file, its matching `:check` in Phase B now passes trivially — that parity is the point. Do not ship with any generator or audit failing; fix the cause (never silence it) and re-run. `check:migrations` and `type-check` are covered by steps 5 and CI respectively and are not repeated here. + If Phase A regenerated a file, its matching `:check` in Phase B now passes trivially — that parity is the point. Do not ship with any generator or audit failing; fix the cause (never silence it) and re-run. `check:migrations` is covered by step 5 and is not repeated here. 7. **Stage and commit** the changes with the generated message — including any files Phase A regenerated in step 6 8. **Push to origin** using the current branch name — `--force-with-lease` if step 2's sync check did any history rewrite (a clean rebase or a cherry-pick rebuild) on a branch that had diff --git a/.claude/rules/sim-components.md b/.claude/rules/sim-components.md index 194359e448b..2c6ee7a3f9c 100644 --- a/.claude/rules/sim-components.md +++ b/.claude/rules/sim-components.md @@ -40,7 +40,7 @@ When rendering or sorting a list of rows against a lookup collection (members, f react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix": - `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them. -- `js-tosorted-immutable` — won't-fix anywhere; `check:utils` bans the ES2023 array methods repo-wide. +- `js-tosorted-immutable` — won't-fix anywhere; `tsc` rejects the ES2023 array methods, because no tsconfig raises `lib` past ES2022. - `rerender-state-only-in-handlers` / "state set but never rendered" — a false positive when the `useState` is consumed by a `useEffect`/`useLayoutEffect` dependency (the effect must re-run on change). Only convert to a ref when nothing reads the value reactively. - `no-render-in-render` — a helper *called inline* (`{renderRow()}`) is reconciled by position and does **not** remount, so extracting it to a component is usually pure churn and can regress behavior (prop-drilling many closures, focus/scroll loss on the inner ``). Apply it only when the helper is genuinely a *component defined during render*, or when the move is mechanical (a stateless, ref-free helper whose closures become a small, explicit prop set). - `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it. diff --git a/.claude/rules/sim-integrations.md b/.claude/rules/sim-integrations.md index 558978c96cd..bb71f32beb4 100644 --- a/.claude/rules/sim-integrations.md +++ b/.claude/rules/sim-integrations.md @@ -17,7 +17,7 @@ The full authoring instructions — tool/block/icon/trigger scaffolding, SubBloc - Tool IDs and the two registration/coercion rules are in the root `CLAUDE.md` → Integrations. `blocks/registry.ts` holds only the accessor functions; triggers register in `triggers/registry.ts`. - 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`), where both fields deliberately share one value. - Keep block outputs aligned with what the referenced tools actually return, and block `tools.access` aligned with the registered tool IDs. -- `canonicalParamId` must NOT match the `id` of a subblock that has no `canonicalParamId` (a group member may share it, as the `add-block` skill's `channel` example does), must be unique **block-wide** (groups are keyed by canonical id across every subblock and hold exactly one `basicId`, so two operations that each need a pair need two different canonical ids), and all subblocks in a canonical group must share the same `required` status. The `inputs` section and the params function reference canonical IDs, not raw subblock IDs — the serializer deletes the subblock IDs and republishes the active member's value under the canonical ID. +- `canonicalParamId` may match only the `id` of a member of its own group (as the `add-block` skill's `channel` example does), never any other subblock's `id`, must be unique **block-wide** (groups are keyed by canonical id across every subblock and hold exactly one `basicId`, so two operations that each need a pair need two different canonical ids), and all subblocks in a canonical group must share the same `required` status. The `inputs` section and the params function reference canonical IDs, not raw subblock IDs — the serializer deletes the subblock IDs and republishes the active member's value under the canonical ID. - A canonical pair carries ONE concept. For files that is upload (basic) + file reference (advanced), normalized with `normalizeFileInput`, as in Gmail attachments (`blocks/blocks/gmail.ts`). Never overload the advanced side with alternate identifiers (URL, provider asset ID) — give those their own subblocks, mark mutually exclusive sources `required: false`, and enforce "exactly one" at execution. - A sub-block's option list is EITHER `selectorKey` (a registered selector — the only way to load a remote list, and the only one that works off the canvas) OR `options` (a static array, or a pure function of the block's own values). Never fetch from a block definition, and never read the workflow stores there. A credential sub-block needs `canonicalParamId: 'oauthCredential'` for its dependants' selectors to resolve. A secret must never appear in a selector's `getQueryKey`. `bun run check:fork-dependent-coverage` fails a `dependsOn` under a credential/KB/table anchor that the fork sync modal cannot offer. - Integration blocks (`category: 'tools'`) must set `integrationType` (`integration-catalog:check` fails without it) and export a `{Service}BlockMeta` (with `tags`); set `authMode` and `docsLink` too, which otherwise fall back to a credential-subblock guess and the generated docs page — see the `/add-block` skill's BlockMeta section. `{Service}BlockMeta.skills` must be grounded in operations the block exposes via `tools.access` and sourced from real, popular use cases found online — never invented. diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index 668e667a5d8..f37fb447299 100644 --- a/.claude/rules/sim-react-performance.md +++ b/.claude/rules/sim-react-performance.md @@ -77,7 +77,7 @@ return items.sort(compare) return [...items].sort(compare) ``` -**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and `with` when called with a numeric index (an identifier index is indistinguishable from OpenTelemetry's `context.with`). +**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. Every tsconfig keeps `"lib"` at or below ES2022 so `tsc` rejects these at each call site on a typed receiver (and still accepts OpenTelemetry's `context.with`, which it tells apart by type); `check:utils` fails if a tsconfig raises `lib` past ES2022, which is how they shipped in #5340, and also matches `toSorted`/`toReversed`/`toSpliced` in source, since tsc accepts any method on an `any` receiver. `.with` on an `any` receiver is caught by neither, so type a parsed array before copying from it. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/.claude/rules/sim-settings-pages.md b/.claude/rules/sim-settings-pages.md index da5447c642c..02c8b75661d 100644 --- a/.claude/rules/sim-settings-pages.md +++ b/.claude/rules/sim-settings-pages.md @@ -93,7 +93,10 @@ return ( ## Title + description live in navigation metadata `apps/sim/components/settings/navigation.ts` is the single source of truth (the -`settings/navigation.ts` in the route tree is only a re-export shim). Every `SETTINGS_SECTION_REGISTRY` entry carries a one-line `description`; `SettingsPanel` +`settings/navigation.ts` in the route tree is only a re-export shim). Each `SETTINGS_SECTION_REGISTRY` entry's one-line description is +`unified.description` (a plane projection's `planes..description` overrides it where that +plane's scope differs), or, for a section that exists only on a standalone plane, its +`planes..description`; `SettingsPanel` resolves both via `getSettingsSectionMeta(plane, section)` and the `SettingsSectionProvider` the settings shell wraps around the active section. diff --git a/.claude/rules/sim-styling.md b/.claude/rules/sim-styling.md index 264fa106e37..f146e7a547c 100644 --- a/.claude/rules/sim-styling.md +++ b/.claude/rules/sim-styling.md @@ -95,7 +95,7 @@ Draw a line with a real `border-*` utility. Never hand-roll one as `shadow-[inse - **Errors** → `error` prop. Never `className={cn(err && 'border-[var(--text-error)]')}`. - **Leading icon** → `icon` prop (rendered 14px in `--text-icon`). - **Trailing buttons** (reveal/copy/fetch) → `endAdornment`. -- **Inner-input styling** (e.g. `font-mono`, number-spinner reset) → `inputClassName` (ChipInput only). See `app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx`. +- **Inner-input styling** (e.g. `font-mono`, number-spinner reset) → `inputClassName` (ChipInput only). See `ee/whitelabeling/components/whitelabeling-settings.tsx`. - **`ChipModalField` controls take NO className.** Pass `title`/`value`/`onChange`/`error`/`hint`/`required`/`flush`. The field owns label, control, and error/hint rendering. See `app/workspace/[workspaceId]/skills/components/skill-modal/skill-modal.tsx`. ### What className MAY carry diff --git a/.claude/rules/sim-url-state.md b/.claude/rules/sim-url-state.md index fb93787f16e..6114efc83bc 100644 --- a/.claude/rules/sim-url-state.md +++ b/.claude/rules/sim-url-state.md @@ -19,7 +19,7 @@ Pick exactly one home for each piece of state (table below). Put state in the UR | Home | Trigger | Example | | --- | --- | --- | | **URL (nuqs)** | Client view-state worth a link: tab, filter, search, sort, pagination, selected-entity id, an open "view" modal/drawer that is a destination | `?tab=licenses`, `?category=Communication`, `?page=3`, `?skillId=abc` | -| **React Query** | Server/remote data fetched from an endpoint | `useMcpServers(workspaceId)`, `useSkills(workspaceId)` | +| **React Query** | Server/remote data fetched from an endpoint (hook rules: `.claude/rules/sim-queries.md`) | `useMcpServers(workspaceId)`, `useSkills(workspaceId)` | | **Zustand** | Cross-component client state that must NOT be in the URL: high-frequency, large, ephemeral, socket-synced | canvas pan/zoom, live cursor, drag state, resize widths, unsaved buffers | | **`useState`** | Purely local single-component UI; also the snappy mirror of a debounced URL search | a hover flag, a transient dialog target, the live text of a debounced search box | diff --git a/.cursor/rules/sim-components.mdc b/.cursor/rules/sim-components.mdc index 15be6706173..dc7bb99300c 100644 --- a/.cursor/rules/sim-components.mdc +++ b/.cursor/rules/sim-components.mdc @@ -41,7 +41,7 @@ When rendering or sorting a list of rows against a lookup collection (members, f react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix": - `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them. -- `js-tosorted-immutable` — won't-fix anywhere; `check:utils` bans the ES2023 array methods repo-wide. +- `js-tosorted-immutable` — won't-fix anywhere; `tsc` rejects the ES2023 array methods, because no tsconfig raises `lib` past ES2022. - `rerender-state-only-in-handlers` / "state set but never rendered" — a false positive when the `useState` is consumed by a `useEffect`/`useLayoutEffect` dependency (the effect must re-run on change). Only convert to a ref when nothing reads the value reactively. - `no-render-in-render` — a helper *called inline* (`{renderRow()}`) is reconciled by position and does **not** remount, so extracting it to a component is usually pure churn and can regress behavior (prop-drilling many closures, focus/scroll loss on the inner ``). Apply it only when the helper is genuinely a *component defined during render*, or when the move is mechanical (a stateless, ref-free helper whose closures become a small, explicit prop set). - `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it. diff --git a/.cursor/rules/sim-integrations.mdc b/.cursor/rules/sim-integrations.mdc index cfe5ed66d92..624d388b8cf 100644 --- a/.cursor/rules/sim-integrations.mdc +++ b/.cursor/rules/sim-integrations.mdc @@ -16,7 +16,7 @@ The full authoring instructions — tool/block/icon/trigger scaffolding, SubBloc - Tool IDs and the two registration/coercion rules are in the root `CLAUDE.md` → Integrations. `blocks/registry.ts` holds only the accessor functions; triggers register in `triggers/registry.ts`. - 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`), where both fields deliberately share one value. - Keep block outputs aligned with what the referenced tools actually return, and block `tools.access` aligned with the registered tool IDs. -- `canonicalParamId` must NOT match the `id` of a subblock that has no `canonicalParamId` (a group member may share it, as the `add-block` skill's `channel` example does), must be unique **block-wide** (groups are keyed by canonical id across every subblock and hold exactly one `basicId`, so two operations that each need a pair need two different canonical ids), and all subblocks in a canonical group must share the same `required` status. The `inputs` section and the params function reference canonical IDs, not raw subblock IDs — the serializer deletes the subblock IDs and republishes the active member's value under the canonical ID. +- `canonicalParamId` may match only the `id` of a member of its own group (as the `add-block` skill's `channel` example does), never any other subblock's `id`, must be unique **block-wide** (groups are keyed by canonical id across every subblock and hold exactly one `basicId`, so two operations that each need a pair need two different canonical ids), and all subblocks in a canonical group must share the same `required` status. The `inputs` section and the params function reference canonical IDs, not raw subblock IDs — the serializer deletes the subblock IDs and republishes the active member's value under the canonical ID. - A canonical pair carries ONE concept. For files that is upload (basic) + file reference (advanced), normalized with `normalizeFileInput`, as in Gmail attachments (`blocks/blocks/gmail.ts`). Never overload the advanced side with alternate identifiers (URL, provider asset ID) — give those their own subblocks, mark mutually exclusive sources `required: false`, and enforce "exactly one" at execution. - A sub-block's option list is EITHER `selectorKey` (a registered selector — the only way to load a remote list, and the only one that works off the canvas) OR `options` (a static array, or a pure function of the block's own values). Never fetch from a block definition, and never read the workflow stores there. A credential sub-block needs `canonicalParamId: 'oauthCredential'` for its dependants' selectors to resolve. A secret must never appear in a selector's `getQueryKey`. `bun run check:fork-dependent-coverage` fails a `dependsOn` under a credential/KB/table anchor that the fork sync modal cannot offer. - Integration blocks (`category: 'tools'`) must set `integrationType` (`integration-catalog:check` fails without it) and export a `{Service}BlockMeta` (with `tags`); set `authMode` and `docsLink` too, which otherwise fall back to a credential-subblock guess and the generated docs page — see the `/add-block` skill's BlockMeta section. `{Service}BlockMeta.skills` must be grounded in operations the block exposes via `tools.access` and sourced from real, popular use cases found online — never invented. diff --git a/.cursor/rules/sim-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index cf526216c5f..4ad3ca74971 100644 --- a/.cursor/rules/sim-react-performance.mdc +++ b/.cursor/rules/sim-react-performance.mdc @@ -80,7 +80,7 @@ return items.sort(compare) return [...items].sort(compare) ``` -**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and `with` when called with a numeric index (an identifier index is indistinguishable from OpenTelemetry's `context.with`). +**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. Every tsconfig keeps `"lib"` at or below ES2022 so `tsc` rejects these at each call site on a typed receiver (and still accepts OpenTelemetry's `context.with`, which it tells apart by type); `check:utils` fails if a tsconfig raises `lib` past ES2022, which is how they shipped in #5340, and also matches `toSorted`/`toReversed`/`toSpliced` in source, since tsc accepts any method on an `any` receiver. `.with` on an `any` receiver is caught by neither, so type a parsed array before copying from it. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/.cursor/rules/sim-settings-pages.mdc b/.cursor/rules/sim-settings-pages.mdc index c34ff9f5e82..e5b83eb9796 100644 --- a/.cursor/rules/sim-settings-pages.mdc +++ b/.cursor/rules/sim-settings-pages.mdc @@ -90,7 +90,10 @@ return ( ## Title + description live in navigation metadata `apps/sim/components/settings/navigation.ts` is the single source of truth (the -`settings/navigation.ts` in the route tree is only a re-export shim). Every `SETTINGS_SECTION_REGISTRY` entry carries a one-line `description`; `SettingsPanel` +`settings/navigation.ts` in the route tree is only a re-export shim). Each `SETTINGS_SECTION_REGISTRY` entry's one-line description is +`unified.description` (a plane projection's `planes..description` overrides it where that +plane's scope differs), or, for a section that exists only on a standalone plane, its +`planes..description`; `SettingsPanel` resolves both via `getSettingsSectionMeta(plane, section)` and the `SettingsSectionProvider` the settings shell wraps around the active section. diff --git a/.cursor/rules/sim-styling.mdc b/.cursor/rules/sim-styling.mdc index 54ef96abfb8..3cc6ebcd23b 100644 --- a/.cursor/rules/sim-styling.mdc +++ b/.cursor/rules/sim-styling.mdc @@ -95,7 +95,7 @@ Draw a line with a real `border-*` utility. Never hand-roll one as `shadow-[inse - **Errors** → `error` prop. Never `className={cn(err && 'border-[var(--text-error)]')}`. - **Leading icon** → `icon` prop (rendered 14px in `--text-icon`). - **Trailing buttons** (reveal/copy/fetch) → `endAdornment`. -- **Inner-input styling** (e.g. `font-mono`, number-spinner reset) → `inputClassName` (ChipInput only). See `app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx`. +- **Inner-input styling** (e.g. `font-mono`, number-spinner reset) → `inputClassName` (ChipInput only). See `ee/whitelabeling/components/whitelabeling-settings.tsx`. - **`ChipModalField` controls take NO className.** Pass `title`/`value`/`onChange`/`error`/`hint`/`required`/`flush`. The field owns label, control, and error/hint rendering. See `app/workspace/[workspaceId]/skills/components/skill-modal/skill-modal.tsx`. ### What className MAY carry diff --git a/.cursor/rules/sim-url-state.mdc b/.cursor/rules/sim-url-state.mdc index 3537973d9aa..1aa10918c8e 100644 --- a/.cursor/rules/sim-url-state.mdc +++ b/.cursor/rules/sim-url-state.mdc @@ -16,7 +16,7 @@ Pick exactly one home for each piece of state (table below). Put state in the UR | Home | Trigger | Example | | --- | --- | --- | | **URL (nuqs)** | Client view-state worth a link: tab, filter, search, sort, pagination, selected-entity id, an open "view" modal/drawer that is a destination | `?tab=licenses`, `?category=Communication`, `?page=3`, `?skillId=abc` | -| **React Query** | Server/remote data fetched from an endpoint | `useMcpServers(workspaceId)`, `useSkills(workspaceId)` | +| **React Query** | Server/remote data fetched from an endpoint (hook rules: `.claude/rules/sim-queries.md`) | `useMcpServers(workspaceId)`, `useSkills(workspaceId)` | | **Zustand** | Cross-component client state that must NOT be in the URL: high-frequency, large, ephemeral, socket-synced | canvas pan/zoom, live cursor, drag state, resize widths, unsaved buffers | | **`useState`** | Purely local single-component UI; also the snappy mirror of a debounced URL search | a hover flag, a transient dialog target, the live text of a debounced search box | diff --git a/CLAUDE.md b/CLAUDE.md index 87172b4fd6a..a3f7da54018 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -82,10 +82,10 @@ The `'use client'` server boundary, the app/worker runtime env split, and featur ## Code Conventions -- **Naming**: components PascalCase (`WorkflowList`); hooks `use*`; files kebab-case (`workflow-list.tsx`); constants SCREAMING_SNAKE_CASE; interfaces PascalCase with a suffix (`WorkflowListProps`); stores `stores//store.ts`. A file never repeats its folder's name (inside `logs/`, `views.ts` not `log-views.ts`; inside `utils/`, `date.ts` not `date-utils.ts`); `check:file-names` enforces this. +- **Naming**: components PascalCase (`WorkflowList`); hooks `use*`; files kebab-case (`workflow-list.tsx`); constants SCREAMING_SNAKE_CASE; interfaces PascalCase with a suffix (`WorkflowListProps`); stores `stores//store.ts`. Inside `lib/`, `executor/`, `providers/`, `stores/`, `hooks/`, `serializer/`, and a package's `src/`, a file never starts with its folder's name (`logs/views.ts`, not `logs/log-views.ts`), and a file in `utils/` or `helpers/` never repeats that role (`date.ts`, not `date-utils.ts`); `feature/feature.tsx` stays the component convention. `check:file-names` ratchets this. - **Imports**: absolute (`@/...`) only, never relative (a barrel `index.ts` re-exports its own siblings relatively). A folder with 3+ exports gets an `index.ts` barrel; never re-export from a non-barrel file. `import type` for type-only imports. Order and lazy-loading through barrels: `.claude/rules/sim-imports.md`. - **TypeScript**: no `any` and no non-null `!` (use precise types or `unknown` with guards; `check:explicit-any` ratchets both); no export nothing imports (`check:unused-exports`); a props interface for every component; `as const` for constant objects/arrays; explicit ref types (`useRef(null)`). -- **Unused bindings** fail lint (biome `noUnusedVariables`, `noUnusedFunctionParameters`): delete the dead variable, import, or parameter and update callers; write `catch {}` when the error is unused. Prefix `_` only for a parameter that must hold its position because a later one is used. `const { a, ...rest } = obj` to omit keys is allowed. The rules carry no autofix, so `bun run lint` will not rename anything for you. +- **Unused bindings** fail lint (biome `noUnusedVariables`, `noUnusedFunctionParameters`): delete the dead variable, import, or parameter and update callers; write `catch {}` when the error is unused. Prefix `_` only for a parameter that must hold its position because a later one is used, or a type parameter kept for API shape. `const { a, ...rest } = obj` to omit keys is allowed. The rules carry no autofix, so `bun run lint` will not rename anything for you. - **Components**: `'use client'` only for hooks or browser APIs (`check:client-boundary` guards the server boundary). Structure order, extraction thresholds, and list-render rules: `.claude/rules/sim-components.md`. Render-performance idioms (lazy-init refs, hoisting, `Map` pre-indexing, `[...arr].sort()`, never `toSorted()`): `.claude/rules/sim-react-performance.md`. For effect/state/memo/callback anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI. - **State ownership**: React Query owns server data — never `useState` + `fetch`; shareable client view-state (tabs, filters, search, pagination, selected id) lives in the URL via `nuqs`; Zustand owns global client state; `useState` owns UI-only state. Hooks: `.claude/rules/sim-hooks.md`. Stores (`devtools`, `persist` only with an explicit `partialize` whitelist, workflow value invariants): `.claude/rules/sim-stores.md`. URL state: `.claude/rules/sim-url-state.md`. - **Utils**: inline a helper with one consumer; create `utils.ts` when 2+ files share it — in `lib/` (app-wide) or `feature/utils/` (feature-scoped). Check `lib/` before writing a new one. diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter.tsx index 85de6b4709a..bc1e125b79d 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter.tsx @@ -87,9 +87,8 @@ export function TableFilter({ if (deferredRule && !isCompleteRule(deferredRule)) { const previouslyAppliedRule = currentRules.find((rule) => rule.id === deferredRule.id) if (previouslyAppliedRule && isCompleteRule(previouslyAppliedRule)) { - const deferredRules = deferredAppliedRules - if (!deferredRules.has(deferredRule.id)) { - deferredRules.set(deferredRule.id, previouslyAppliedRule) + if (!deferredAppliedRules.has(deferredRule.id)) { + deferredAppliedRules.set(deferredRule.id, previouslyAppliedRule) } } } diff --git a/apps/sim/hooks/mcp/use-mcp-oauth-popup.ts b/apps/sim/hooks/mcp/use-mcp-oauth-popup.ts index 5fbd589bed2..b0ee6e6f0f0 100644 --- a/apps/sim/hooks/mcp/use-mcp-oauth-popup.ts +++ b/apps/sim/hooks/mcp/use-mcp-oauth-popup.ts @@ -125,13 +125,12 @@ export function useMcpOauthPopup({ workspaceId }: UseMcpOauthPopupProps) { ) useEffect(() => { - const pending = pendingFlows return () => { - for (const { timeout, poll } of pending.values()) { + for (const { timeout, poll } of pendingFlows.values()) { window.clearTimeout(timeout) if (poll !== undefined) window.clearInterval(poll) } - pending.clear() + pendingFlows.clear() } }, []) diff --git a/scripts/check-application-graph.ts b/scripts/check-application-graph.ts index b2b9e17045d..4abb45df17e 100644 --- a/scripts/check-application-graph.ts +++ b/scripts/check-application-graph.ts @@ -283,19 +283,39 @@ export function findViolations({ root, forbidden }: GuardedRoot): GraphViolation return violations } +/** A module a runtime import can resolve to: not a test, integration test, or declaration file. */ +const RUNTIME_MODULE = /(? + entry.isDirectory() + ? !TEST_SUPPORT_DIRS.has(entry.name) && containsRuntimeModule(resolve(dir, entry.name)) + : RUNTIME_MODULE.test(entry.name) + ) +} + /** * Whether `prefix` names a directory or an importable module under `apps/sim`. A prefix that * matches nothing guards nothing: the tree was renamed, and the audit would keep passing over it. - * A test file left behind does not count, because no runtime import resolves to it. + * A test or declaration file left behind does not count, because no runtime import resolves to it. */ function prefixMatchesAnything(prefix: string): boolean { - if (prefix.endsWith('/')) return existsSync(resolve(APP_ROOT, prefix)) + if (prefix.endsWith('/')) { + const dir = resolve(APP_ROOT, prefix) + return existsSync(dir) && containsRuntimeModule(dir) + } const dir = resolve(APP_ROOT, dirname(prefix)) if (!existsSync(dir)) return false return readdirSync(dir, { withFileTypes: true }).some( (entry) => entry.name.startsWith(basename(prefix)) && - (entry.isDirectory() || /(?` access in the file. - */ -function envFlagReads(clause: string, content: string): string[] { - const namespace = /^\*\s+as\s+(\w+)$/.exec(clause.trim())?.[1] - if (namespace) { - return [...content.matchAll(new RegExp(`\\b${namespace}\\.(\\w+)`, 'g'))].map((m) => m[1]) - } +/** The named members an import clause brings in. */ +function namedImports(clause: string): string[] { if (!clause.includes('{')) return [] return clause .slice(clause.indexOf('{') + 1, clause.lastIndexOf('}')) @@ -331,8 +351,11 @@ async function main() { const shapeFlags = new Set( parseImports(await readSource(DEPLOYMENT_SHAPE_MODULE)) - .filter((imp) => imp.specifier === ENV_FLAGS) - .flatMap((imp) => envFlagReads(imp.clause, '')) + .filter( + (imp) => + resolveSpecifier(imp.specifier, DEPLOYMENT_SHAPE_MODULE, sourceFiles) === ENV_FLAGS_MODULE + ) + .flatMap((imp) => namedImports(imp.clause)) ) if (shapeFlags.size === 0) { throw new Error( @@ -346,10 +369,11 @@ async function main() { if (!(await isDeploymentShapeClient(rel, absFile))) continue const content = await readSource(absFile) for (const imp of parseImports(content)) { - if (imp.specifier !== ENV_FLAGS || !importsAValue(imp.clause)) continue - const flags = [...new Set(envFlagReads(imp.clause, content))].filter((name) => - shapeFlags.has(name) - ) + if (!importsAValue(imp.clause)) continue + if (resolveSpecifier(imp.specifier, absFile, sourceFiles) !== ENV_FLAGS_MODULE) continue + const flags = /\*\s*as\s/.test(imp.clause) + ? [imp.clause.trim()] + : namedImports(imp.clause).filter((name) => shapeFlags.has(name)) if (flags.length === 0 || hasAllowDirective(content, imp.line)) continue shapeViolations.push({ file: rel, line: imp.line, specifier: imp.specifier, flags }) } @@ -361,7 +385,8 @@ async function main() { failed = true console.error( `\n✗ ${shapeViolations.length} client module(s) read deployment-shape flags from env-flags.\n` + - ` Read them via useDeploymentShape() (components) or getDeploymentShape() (helpers) from @/lib/core/config/deployment-shape.\n` + ` Read them via useDeploymentShape() (components) or getDeploymentShape() (helpers) from @/lib/core/config/deployment-shape;\n` + + ` import any other env-flags export by name, never as a namespace.\n` ) for (const v of shapeViolations) { console.error(` ${v.file}:${v.line} imports ${v.flags.join(', ')} from '${v.specifier}'`) diff --git a/scripts/check-comment-hygiene.ts b/scripts/check-comment-hygiene.ts index a747f9fc799..e51e4e0b468 100644 --- a/scripts/check-comment-hygiene.ts +++ b/scripts/check-comment-hygiene.ts @@ -153,8 +153,16 @@ function firstCodeSpan(run: LineComment[], jsx: boolean): LineComment | undefine return undefined } +/** + * A superset of every hit: a separator right after a comment opener, or a `//` line holding a + * {@link CODE_PUNCTUATION} token. Files without one skip the parse, which dominates the run. + */ +const MAY_VIOLATE = + /\/[/*][*\s]*(?:={3}|-{3}|─{3}|━{3}|\*{3}|~{3})|\/\/[^\n]*(?:[;{}]|=>|\b(?:const|let|return|await|import|export)\b|\w\.\w+\()/ + /** Every banner and commented-out-code hit in one source file. */ export function findViolations(file: string, source: string): Violation[] { + if (!MAY_VIOLATE.test(source)) return [] const jsx = /\.[jt]sx$/.test(file) let comments try { @@ -213,12 +221,27 @@ export function findViolations(file: string, source: string): Violation[] { } function sourceFiles(): string[] { - return execFileSync('git', ['ls-files', '*.ts', '*.tsx', '*.mts', '*.cts', '*.mjs', '*.cjs'], { - cwd: ROOT, - encoding: 'utf8', - // The listing is already ~1 MB, the default execFileSync ceiling. - maxBuffer: 64 * 1024 * 1024, - }) + return execFileSync( + 'git', + [ + 'ls-files', + '--cached', + '--others', + '--exclude-standard', + '*.ts', + '*.tsx', + '*.mts', + '*.cts', + '*.mjs', + '*.cjs', + ], + { + cwd: ROOT, + encoding: 'utf8', + // The listing is already ~1 MB, the default execFileSync ceiling. + maxBuffer: 64 * 1024 * 1024, + } + ) .split('\n') .filter((file) => file && !EXCLUDED.some((pattern) => pattern.test(file))) } diff --git a/scripts/check-explicit-any.ts b/scripts/check-explicit-any.ts index 788593941bf..9737c10d3f5 100644 --- a/scripts/check-explicit-any.ts +++ b/scripts/check-explicit-any.ts @@ -85,10 +85,19 @@ function collect(): Baseline { return counts } -/** `biome-ignore` comments for either rule, which would hide a hit from Biome's count. */ +/** + * `biome-ignore` comments that would hide a hit from Biome's count: either rule, or a group + * (`lint/suspicious`) or bare `lint` suppression that covers it. + */ function suppressions(): string[] { + const covering = Object.values(METRICS) + .map((rule) => { + const [, group, name] = rule.split('/') + return `/${group}(/${name})?` + }) + .join('|') // Anchored to a comment opener so the phrase inside a string literal (a test fixture) is not a hit. - const pattern = `^[[:space:]]*(//|/\\*|\\{/\\*)[[:space:]]*biome-ignore(-all|-start)?[[:space:]]+(${Object.values(METRICS).join('|')})` + const pattern = `^[[:space:]]*(//|/\\*|\\{/\\*)[[:space:]]*biome-ignore(-all|-start)?[[:space:]]+lint(${covering})?([[:space:]:(]|$)` const result = Bun.spawnSync( ['git', 'grep', '-nE', '--untracked', pattern, '--', 'apps', 'packages', 'scripts'], { cwd: ROOT, stdout: 'pipe', stderr: 'pipe' } @@ -177,8 +186,8 @@ if (process.argv.includes('--update')) { let regressed = 0 let stale = 0 -/** A vanished baselined file next to a new file with no more hits: likely a rename. */ -const renames = new Set() +/** A baselined file no longer has any hits, so a regression elsewhere may be the same file renamed. */ +let vanished = false for (const metric of Object.keys(METRICS) as Metric[]) { const before = baseline[metric] ?? {} @@ -191,13 +200,7 @@ for (const metric of Object.keys(METRICS) as Metric[]) { const shrunk = Object.entries(before) .filter(([file, count]) => (after[file] ?? 0) < count) .map(([file, count]) => ` ${file}: ${after[file] ?? 0} (baseline ${count})`) - for (const [oldFile, oldCount] of Object.entries(before)) { - if (oldFile in after) continue - const renamed = Object.entries(after).find( - ([file, count]) => !(file in before) && count <= oldCount - ) - if (renamed) renames.add(`${oldFile} → ${renamed[0]}`) - } + if (Object.keys(before).some((file) => !(file in after))) vanished = true if (regressions.length) { console.error(`✗ ${metric}: ${regressions.length} file(s) gained ${METRICS[metric]} hits`) console.error(regressions.sort().join('\n')) @@ -222,10 +225,10 @@ if (regressed || stale || suppressed.length) { ) } if (regressed) console.error('\nNever raise the baseline to make a new `any` or `!` pass.') - for (const rename of renames) { + if (regressed && vanished) { console.error( - `\nLooks like a rename: ${rename}. Move its baseline entries to the new path in ` + - `${path.relative(ROOT, BASELINE)} (debt carries over; it may not grow).` + `If a regressed file is a baselined file you only moved (git mv), move its entry to the new ` + + `path in ${path.relative(ROOT, BASELINE)}; its count may not grow.` ) } process.exit(1) diff --git a/scripts/check-file-names.ts b/scripts/check-file-names.ts index 7ca4e3122db..de7b7d9792c 100644 --- a/scripts/check-file-names.ts +++ b/scripts/check-file-names.ts @@ -270,15 +270,6 @@ if (added.length || stale.length) { 'baseline: bun run scripts/check-file-names.ts --update' ) } - for (const entry of added) { - const old = stale.find((s) => s.split('\t')[0] === entry.split('\t')[0]) - if (old) { - console.error( - `\nLooks like a rename: ${old.split('\t')[1]} → ${entry.split('\t')[1]}. Move its baseline ` + - `entry to the new path in ${path.relative(ROOT, BASELINE)} (debt carries over; it may not grow).` - ) - } - } process.exit(1) } diff --git a/scripts/check-guidance-refs.ts b/scripts/check-guidance-refs.ts index bfd9c1e6513..99d035e01f5 100644 --- a/scripts/check-guidance-refs.ts +++ b/scripts/check-guidance-refs.ts @@ -190,7 +190,7 @@ function importResolves(spec: string, packages: Map): if (tail === undefined) return key === subpath && moduleResolves(pkg.dir, target) if (!subpath.startsWith(head) || !subpath.endsWith(tail)) return false const matched = subpath.slice(head.length, subpath.length - tail.length) - return moduleResolves(pkg.dir, target.replace('*', matched)) + return moduleResolves(pkg.dir, target.replaceAll('*', matched)) }) } diff --git a/scripts/check-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index e0fbfe55d45..52ab18ab7cb 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -4,9 +4,14 @@ * * Most patterns point at an `@sim/utils` helper (CLAUDE.md "Common utilities"). A few encode * render-path rules from `.claude/rules/sim-react-performance.md` and `sim-styling.md` that no - * linter covers: ES2023 array methods that Safari 15 lacks (banned everywhere, since whether a - * module reaches the browser is not visible from its path and a copy-then-sort costs the same), `useRef(new X())` allocating on - * every render, and `h-N w-N` where `size-N` is the convention. + * linter covers: `useRef(new X())` allocating on every render, and `h-N w-N` where `size-N` is the + * convention. + * + * ES2023 array methods (`toSorted`, `with`, …) throw on Safari/iOS 15, and SWC does not polyfill + * them. Every tsconfig keeps `lib` at or below ES2022 so `tsc` rejects them at each call site, + * telling `Array.prototype.with` apart from OpenTelemetry's `context.with` by type; this script + * fails if a tsconfig raises `lib` past that, which is how they shipped once (#5340). The three + * names nothing else uses are also matched in source, since tsc accepts them on an `any` receiver. * * Biome's noRestrictedImports covers the import-based bans it lists — today `nanoid` and * `uuid`. It does NOT cover named crypto imports; `import { randomBytes } from 'node:crypto'` @@ -17,6 +22,7 @@ * multi-token expression that the formatter wraps at 100 columns, and a line-scoped scan sees * none of the wrapped forms. Deliberate exceptions carry `// utils-lint-allow: `. */ +import { readFileSync } from 'node:fs' import { readdir, readFile } from 'node:fs/promises' import path from 'node:path' @@ -55,7 +61,6 @@ const BANNED_PATTERNS: Array<{ pattern: RegExp description: string suggestion: string - /** Restricts the pattern to matching files; unrestricted patterns apply everywhere. */ /** Cheap literal test that skips the pattern on files that cannot match; memoized per file. */ prefilter?: RegExp }> = [ @@ -146,9 +151,9 @@ const BANNED_PATTERNS: Array<{ }, // Render-path rules (.claude/rules/sim-react-performance.md, sim-styling.md) { - pattern: /\.(?:toSorted|toReversed|toSpliced)\s*\(|\.with\(\s*-?\d+\s*,/g, - description: - 'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)', + // tsc rejects these under the ES2022 lib, except on an `any` receiver (`JSON.parse(s).toSorted()`). + pattern: /\.(?:toSorted|toReversed|toSpliced)\s*\(/g, + description: 'ES2023 array method (throws on Safari/iOS 15; SWC does not polyfill it)', suggestion: 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice()', }, { @@ -248,6 +253,31 @@ function hasAllow(lines: string[], line: number): boolean { return false } +/** A `lib` entry at ES2023 or later, including its sub-libs (`ES2023.Array`) and `ESNext`. */ +const LIB_PAST_ES2022 = /^es(?:20(?:2[3-9]|[3-9]\d)|next)\b/i + +/** Tracked tsconfigs whose `lib` admits the ES2023 runtime methods `tsc` would otherwise reject. */ +function es2023LibViolations(): Violation[] { + const listed = Bun.spawnSync(['git', 'ls-files', '*tsconfig*.json'], { cwd: ROOT }) + const violations: Violation[] = [] + for (const file of listed.stdout.toString().split('\n').filter(Boolean)) { + const content = readFileSync(path.join(ROOT, file), 'utf8') + const lib = /"lib"\s*:\s*\[([^\]]*)\]/.exec(content) + const entries = lib?.[1]?.match(/"[^"]*"/g) ?? [] + if (!entries.some((entry) => LIB_PAST_ES2022.test(entry.slice(1, -1)))) continue + const line = content.slice(0, lib?.index).split('\n').length + violations.push({ + file, + line, + description: + '"lib" past ES2022 lets ES2023 array methods (toSorted, with, …) type-check; they throw on Safari/iOS 15 and SWC does not polyfill them', + suggestion: '"lib" at ES2022, and a copy in code: [...arr].sort(), [...arr].reverse()', + snippet: (content.split('\n')[line - 1] ?? '').trim(), + }) + } + return violations +} + async function main() { const allFiles: string[] = [] for (const dir of SCAN_DIRS) { @@ -299,6 +329,8 @@ async function main() { } } + violations.push(...es2023LibViolations()) + if (violations.length === 0) { console.log('✓ No banned patterns found.') process.exit(0) diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index 9293994785e..10318c36337 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -1,6 +1,8 @@ #!/usr/bin/env bun import { readdir, readFile } from 'node:fs/promises' import path from 'node:path' +import { parse } from '@babel/parser' +import { getErrorMessage } from '@sim/utils/errors' const ROOT = path.resolve(import.meta.dir, '..') const APP_DIR = path.join(ROOT, 'apps/sim') @@ -298,6 +300,135 @@ function auditFile(file: string, source: string): Violation[] { const PERSIST_IMPORT = /import\s*\{[^}]*\bpersist\b(?:\s+as\s+(\w+))?[^}]*\}\s*from\s*'zustand\/middleware'/ +/** A Babel AST node, read structurally rather than through `@babel/types`. */ +interface SyntaxNode extends Record { + type: string + start: number + end: number +} + +function isSyntaxNode(value: unknown): value is SyntaxNode { + return ( + typeof value === 'object' && value !== null && 'type' in value && typeof value.type === 'string' + ) +} + +/** Visits `node` and every node beneath it, depth first; `visit` returning false skips a subtree. */ +function walkNodes(node: SyntaxNode, visit: (node: SyntaxNode) => boolean | undefined): void { + if (visit(node) === false) return + for (const value of Object.values(node)) { + if (isSyntaxNode(value)) walkNodes(value, visit) + else if (Array.isArray(value)) + for (const item of value) if (isSyntaxNode(item)) walkNodes(item, visit) + } +} + +/** Strips the type-only wrappers (`as`, `satisfies`, `!`) around an expression. */ +function unwrapExpression(node: unknown): unknown { + let current = node + while ( + isSyntaxNode(current) && + (current.type === 'TSAsExpression' || + current.type === 'TSSatisfiesExpression' || + current.type === 'TSNonNullExpression') + ) { + current = current.expression + } + return current +} + +function isIdentifierNamed(node: unknown, name: string): boolean { + const unwrapped = unwrapExpression(node) + return isSyntaxNode(unwrapped) && unwrapped.type === 'Identifier' && unwrapped.name === name +} + +/** `name` itself, or an object literal that spreads `...name` (extra keys still carry everything). */ +function isWholeBinding(node: unknown, name: string): boolean { + if (isIdentifierNamed(node, name)) return true + const unwrapped = unwrapExpression(node) + // `state || {}`, `state ?? {}`, and `cond ? state : {}` can each return the whole state; + // `a && b` returns `a` only when it is falsy, which a state object never is. + if (isSyntaxNode(unwrapped) && unwrapped.type === 'LogicalExpression') { + if (isWholeBinding(unwrapped.right, name)) return true + return unwrapped.operator !== '&&' && isWholeBinding(unwrapped.left, name) + } + if (isSyntaxNode(unwrapped) && unwrapped.type === 'ConditionalExpression') { + return isWholeBinding(unwrapped.consequent, name) || isWholeBinding(unwrapped.alternate, name) + } + if (!isSyntaxNode(unwrapped) || unwrapped.type !== 'ObjectExpression') return false + const properties = Array.isArray(unwrapped.properties) ? unwrapped.properties : [] + return properties.some( + (property) => + isSyntaxNode(property) && + property.type === 'SpreadElement' && + isIdentifierNamed(property.argument, name) + ) +} + +/** The expressions a function returns: its expression body, or every `return` in its own block. */ +function returnedExpressions(fn: SyntaxNode): unknown[] { + if (!isSyntaxNode(fn.body)) return [] + if (fn.body.type !== 'BlockStatement') return [fn.body] + const returned: unknown[] = [] + walkNodes(fn.body, (node) => { + if (node.type === 'ReturnStatement') returned.push(node.argument) + return !/Function|ObjectMethod|ClassMethod/.test(node.type) + }) + return returned +} + +/** + * The parameter binding that holds every state field: `s` in `(s) => …`, or `rest` in + * `({ a, ...rest }) => …`, which holds every field but the ones named (a deny-list). + */ +function wholeStateBinding(rawParam: unknown): { name: string; reason: string } | null { + // A default (`(state = {} as State) => …`) binds the same value. + const param = + isSyntaxNode(rawParam) && rawParam.type === 'AssignmentPattern' ? rawParam.left : rawParam + if (!isSyntaxNode(param)) return null + if (param.type === 'Identifier' && typeof param.name === 'string') { + return { name: param.name, reason: 'persist partialize spreads the whole state' } + } + if (param.type !== 'ObjectPattern' || !Array.isArray(param.properties)) return null + const rest = param.properties.find( + (property): property is SyntaxNode => isSyntaxNode(property) && property.type === 'RestElement' + ) + if (!rest || !isSyntaxNode(rest.argument) || typeof rest.argument.name !== 'string') return null + return { + name: rest.argument.name, + reason: 'persist partialize persists everything but the fields it names (a deny-list)', + } +} + +/** + * Why an inline `partialize` persists more than a whitelist, or null when it returns one: + * any return of the whole-state binding itself or of `{ ...binding }`. + */ +function partializeLeak(fn: SyntaxNode): string | null { + const binding = wholeStateBinding(Array.isArray(fn.params) ? fn.params[0] : undefined) + if (!binding) return null + return returnedExpressions(fn).some((expression) => isWholeBinding(expression, binding.name)) + ? `${binding.reason}; return an explicit whitelist of durable fields` + : null +} + +/** The top-level `partialize` of a persist options object literal, or null when it has none. */ +function findPartialize(options: SyntaxNode): SyntaxNode | null { + const properties = Array.isArray(options.properties) ? options.properties : [] + for (const property of properties) { + if (!isSyntaxNode(property) || property.computed === true) continue + if (property.type !== 'ObjectProperty' && property.type !== 'ObjectMethod') continue + const key = property.key + const keyName = isSyntaxNode(key) ? (key.type === 'Identifier' ? key.name : key.value) : null + if (keyName !== 'partialize') continue + if (property.type === 'ObjectMethod') return property + // A cast such as `((s) => s) as Partialize` still hands zustand the inner function. + const value = unwrapExpression(property.value) + return isSyntaxNode(value) ? value : null + } + return null +} + /** * `.claude/rules/sim-stores.md`: every `persist` names its durable fields in `partialize`. * Without one, zustand writes the whole state — transient flags, drag state, `_hasHydrated` — @@ -307,35 +438,40 @@ function auditPersist(file: string, source: string): Violation[] { const persistImport = PERSIST_IMPORT.exec(source) if (!persistImport) return [] const local = persistImport[1] ?? 'persist' - /** A call of the imported middleware; `.persist(` (an instance method) is excluded. */ - const persistCall = new RegExp(`(?)?\\s*\\(`, 'g') + let program: unknown + try { + program = parse(source, { + sourceType: 'module', + plugins: ['typescript', ...(/\.[jt]sx$/.test(file) ? (['jsx'] as const) : [])], + errorRecovery: true, + }).program + } catch (error) { + throw new Error(`Cannot parse ${file} to audit its persist calls: ${getErrorMessage(error)}`) + } + if (!isSyntaxNode(program)) return [] + const violations: Violation[] = [] - for (let match = persistCall.exec(source); match; match = persistCall.exec(source)) { - if (hasSafeAnnotation(source, match.index)) continue - const openParenIndex = match.index + match[0].length - 1 - const closeParenIndex = findMatchingParen(source, openParenIndex) - if (closeParenIndex === -1) continue - const call = source.slice(openParenIndex + 1, closeParenIndex) - const hasPartialize = /\bpartialize\b/.test(call) - /** `(s) => s`, `(s) => ({ ...s })`, or a block body returning either: the whole state persists. */ - const spreadsState = - /\bpartialize\s*:\s*\(?\s*(\w+)[^)]*\)?\s*=>\s*(?:\1\b(?!\s*[.[])|\(\s*\{\s*\.\.\.\1\b|\{[^}]*\breturn\s+(?:\1\b(?!\s*[.[])|\{\s*\.\.\.\1\b))/.test( - call - ) - if (hasPartialize && !spreadsState) continue + walkNodes(program, (node) => { + if (node.type !== 'CallExpression' || !isIdentifierNamed(node.callee, local)) return + if (hasSafeAnnotation(source, node.start)) return + const args = Array.isArray(node.arguments) ? node.arguments : [] + const options = args.length > 1 ? unwrapExpression(args[args.length - 1]) : null + const partialize = + isSyntaxNode(options) && options.type === 'ObjectExpression' ? findPartialize(options) : null + const isInlineFunction = + partialize !== null && + /^(?:ArrowFunctionExpression|FunctionExpression|ObjectMethod)$/.test(partialize.type) + const leak = partialize && isInlineFunction ? partializeLeak(partialize) : null + if (partialize && !leak) return violations.push({ file, - line: lineNumberAt(source, match.index), - description: spreadsState - ? 'persist partialize spreads the whole state; return an explicit whitelist of durable fields' - : `persist has no partialize; add \`partialize: (state) => ({ })\` (sim-stores.md). If the options object is hoisted into a variable that has one, mark the call // ${SAFE_ANNOTATION} `, - snippet: oneLineSnippet( - source, - match.index, - Math.min(closeParenIndex + 1, match.index + 180) - ), + line: lineNumberAt(source, node.start), + description: + leak ?? + `persist has no partialize; add \`partialize: (state) => ({ })\` (sim-stores.md). If the options object is hoisted into a variable that has one, mark the call // ${SAFE_ANNOTATION} `, + snippet: oneLineSnippet(source, node.start, Math.min(node.end, node.start + 180)), }) - } + }) return violations } diff --git a/scripts/source-kind.ts b/scripts/source-kind.ts deleted file mode 100644 index 92406f1e426..00000000000 --- a/scripts/source-kind.ts +++ /dev/null @@ -1,25 +0,0 @@ -/** A lone directive statement, e.g. `'use server'` or `"use client";`. */ -const DIRECTIVE_STATEMENT = /^(['"])(use [a-z-]+)\1\s*;?$/ - -/** - * The directive a single source line states, e.g. `use client`, or null. Notes may sit on the same - * line, so `//` and inline `/* *\/` comments come off before matching. - */ -export function directiveOn(line: string): string | null { - const statement = line - .replace(/\/\*.*?\*\//g, '') - .replace(/\/\/.*$/, '') - .trim() - return DIRECTIVE_STATEMENT.exec(statement)?.[2] ?? null -} - -/** Comments and whitespace ahead of a module's first statement. */ -const LEADING_COMMENTS = /^(?:\s*(?:\/\/[^\n]*|\/\*[\s\S]*?\*\/))*\s*/ - -/** - * The module's leading directive prologue, if any. A directive must be the first statement; - * comments and blank lines may precede it. - */ -export function leadingDirective(content: string): string | null { - return directiveOn(content.replace(LEADING_COMMENTS, '').split('\n', 1)[0]) -}