From ded17e26e37299bb0ded9803e8afcca382e972b4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 09:24:20 -0700 Subject: [PATCH 1/9] fix(audits): close guardrail detector gaps and correct guidance from release review --- .agents/skills/add-block-preview/SKILL.md | 2 +- .agents/skills/add-block/SKILL.md | 2 +- .agents/skills/add-connector/SKILL.md | 2 +- .agents/skills/add-settings-page/SKILL.md | 11 ++++--- .agents/skills/babysit/SKILL.md | 7 +++-- .claude/rules/sim-react-performance.md | 2 +- .claude/rules/sim-settings-pages.md | 5 ++- .claude/rules/sim-styling.md | 2 +- .claude/rules/sim-url-state.md | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- .cursor/rules/sim-settings-pages.mdc | 5 ++- .cursor/rules/sim-styling.mdc | 2 +- .cursor/rules/sim-url-state.mdc | 2 +- scripts/check-application-graph.ts | 23 ++++++++++++-- scripts/check-client-boundary-imports.ts | 38 +++++++++++------------ scripts/check-guidance-refs.ts | 2 +- scripts/check-utils-enforcement.ts | 7 +++-- scripts/check-zustand-v5-selectors.ts | 24 +++++++++++--- 18 files changed, 91 insertions(+), 49 deletions(-) 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..b80277bcad2 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` must not equal the `id` of a subblock that has no `canonicalParamId`. A group member may share it, as `channel` does in the canonicalParamId Pattern below. - 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/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index 668e667a5d8..abbd64e2833 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. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and any `.with(index, value)` call whose second argument is not a function literal; OpenTelemetry's `context.with(ctx, () => …)` passes, and one handed its callback as an identifier needs `// utils-lint-allow: `. ## 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-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index cf526216c5f..9807f125623 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. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and any `.with(index, value)` call whose second argument is not a function literal; OpenTelemetry's `context.with(ctx, () => …)` passes, and one handed its callback as an identifier needs `// utils-lint-allow: `. ## 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/scripts/check-application-graph.ts b/scripts/check-application-graph.ts index b2b9e17045d..439421aee0a 100644 --- a/scripts/check-application-graph.ts +++ b/scripts/check-application-graph.ts @@ -283,19 +283,36 @@ 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() + ? 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 +326,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 +344,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 +360,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-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..60776caf62a 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -146,10 +146,13 @@ 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, + /** `.with(i, value)`; a function second argument is OpenTelemetry's `context.with(ctx, fn)`. */ + pattern: + /\.(?:toSorted|toReversed|toSpliced)\s*\(|\.with\(\s*[^,()]+,(?!\s*(?:async\s*)?(?:\([^)]*\)\s*=>|\w+\s*=>|function\b))/g, description: 'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)', - suggestion: 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice()', + suggestion: + 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice(), or [...arr] then next[i] = value', }, { pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g, diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index 9293994785e..6251024d640 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -298,6 +298,14 @@ function auditFile(file: string, source: string): Violation[] { const PERSIST_IMPORT = /import\s*\{[^}]*\bpersist\b(?:\s+as\s+(\w+))?[^}]*\}\s*from\s*'zustand\/middleware'/ +/** Removes comments while leaving string literals (which may contain `//`) intact. */ +function stripComments(code: string): string { + return code.replace( + /('(?:\\.|[^'\\])*'|"(?:\\.|[^"\\])*"|`(?:\\.|[^`\\])*`)|\/\*[\s\S]*?\*\/|\/\/[^\n]*/g, + (_, literal: string | undefined) => literal ?? '' + ) +} + /** * `.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` — @@ -315,12 +323,18 @@ function auditPersist(file: string, source: string): Violation[] { 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 args = splitTopLevelArguments( + stripComments(source.slice(openParenIndex + 1, closeParenIndex)) + ) + const options = args.length > 1 ? args[args.length - 1] : '' + const hasPartialize = /(?:^|[{,])\s*partialize\s*[:(,}]/.test(options) + /** + * `(s) => s`, `(s) => ({ ...s })`, or a block body returning either — with `s` also bound as + * `({ ...s })` — persists the whole state. + */ 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 + /\bpartialize\s*:\s*\(?\s*(?:\{\s*\.\.\.)?(\w+)[^)]*\)?\s*=>\s*(?:\1\b(?!\s*[.[])|\(\s*\{\s*\.\.\.\1\b|\{[^}]*\breturn\s+(?:\1\b(?!\s*[.[])|\{\s*\.\.\.\1\b))/.test( + options ) if (hasPartialize && !spreadsState) continue violations.push({ From d5b5572ade53139b673e86ee75ec00e58c74d1ca Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 09:44:11 -0700 Subject: [PATCH 2/9] fix(audits): detect partialize and .with by AST instead of regex --- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- scripts/check-application-graph.ts | 5 +- scripts/check-utils-enforcement.ts | 94 +++++++++++-- scripts/check-zustand-v5-selectors.ts | 180 +++++++++++++++++++----- 5 files changed, 236 insertions(+), 47 deletions(-) diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index abbd64e2833..9964840c5dd 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 any `.with(index, value)` call whose second argument is not a function literal; OpenTelemetry's `context.with(ctx, () => …)` passes, and one handed its callback as an identifier needs `// utils-lint-allow: `. +**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 every two-argument `.with(index, value)` call on any receiver except an OpenTelemetry context object (a receiver named like `context`, `otelContext`, `otelContextApi`); a deliberate exception carries `// utils-lint-allow: `. ## Run independent awaits in parallel diff --git a/.cursor/rules/sim-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index 9807f125623..52ac260b8ab 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 any `.with(index, value)` call whose second argument is not a function literal; OpenTelemetry's `context.with(ctx, () => …)` passes, and one handed its callback as an identifier needs `// utils-lint-allow: `. +**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 every two-argument `.with(index, value)` call on any receiver except an OpenTelemetry context object (a receiver named like `context`, `otelContext`, `otelContextApi`); a deliberate exception carries `// utils-lint-allow: `. ## Run independent awaits in parallel diff --git a/scripts/check-application-graph.ts b/scripts/check-application-graph.ts index 439421aee0a..4abb45df17e 100644 --- a/scripts/check-application-graph.ts +++ b/scripts/check-application-graph.ts @@ -286,11 +286,14 @@ export function findViolations({ root, forbidden }: GuardedRoot): GraphViolation /** A module a runtime import can resolve to: not a test, integration test, or declaration file. */ const RUNTIME_MODULE = /(? entry.isDirectory() - ? containsRuntimeModule(resolve(dir, entry.name)) + ? !TEST_SUPPORT_DIRS.has(entry.name) && containsRuntimeModule(resolve(dir, entry.name)) : RUNTIME_MODULE.test(entry.name) ) } diff --git a/scripts/check-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index 60776caf62a..91ba7c9bf23 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -19,6 +19,8 @@ */ 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, '..') @@ -51,11 +53,18 @@ const TRUNCATE_PREFILTER = /\.(?:slice|substring)\(\s*0\s*,[^)]*\)\s*(?:\}|\+)/ /** Literal gate shared by the `filterUndefined` and `omit` patterns. */ const FROM_ENTRIES = /Object\.fromEntries\(/ +/** Shared by the toSorted/toReversed/toSpliced pattern and the AST-based `.with` check. */ +const ES2023_ARRAY_METHOD = { + description: + 'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)', + suggestion: + 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice(), or [...arr] then next[i] = value', +} + 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,13 +155,8 @@ const BANNED_PATTERNS: Array<{ }, // Render-path rules (.claude/rules/sim-react-performance.md, sim-styling.md) { - /** `.with(i, value)`; a function second argument is OpenTelemetry's `context.with(ctx, fn)`. */ - pattern: - /\.(?:toSorted|toReversed|toSpliced)\s*\(|\.with\(\s*[^,()]+,(?!\s*(?:async\s*)?(?:\([^)]*\)\s*=>|\w+\s*=>|function\b))/g, - description: - 'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)', - suggestion: - 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice(), or [...arr] then next[i] = value', + pattern: /\.(?:toSorted|toReversed|toSpliced)\s*\(/g, + ...ES2023_ARRAY_METHOD, }, { pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g, @@ -169,6 +173,75 @@ const BANNED_PATTERNS: Array<{ }, ] +/** Cheap gate: only files that contain a `.with(` call are parsed. */ +const WITH_CALL = /\.with\s*\(/ + +/** OpenTelemetry's `context.with(ctx, fn)` shares the shape; its receivers are named for the context API. */ +const OTEL_CONTEXT_RECEIVER = /context/i + +/** A Babel AST node, read structurally rather than through `@babel/types`. */ +interface SyntaxNode extends Record { + type: string + start: number +} + +function isSyntaxNode(value: unknown): value is SyntaxNode { + return ( + typeof value === 'object' && value !== null && 'type' in value && typeof value.type === 'string' + ) +} + +function walkNodes(node: SyntaxNode, visit: (node: SyntaxNode) => void): void { + visit(node) + 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) + } +} + +/** The name a member call is made on: `arr` in `arr.with(…)`, `items` in `this.items.with(…)`. */ +function receiverName(receiver: unknown): string | undefined { + if (!isSyntaxNode(receiver)) return undefined + if (receiver.type === 'Identifier' && typeof receiver.name === 'string') return receiver.name + const isMember = + receiver.type === 'MemberExpression' || receiver.type === 'OptionalMemberExpression' + if (!isMember || receiver.computed === true || !isSyntaxNode(receiver.property)) return undefined + return typeof receiver.property.name === 'string' ? receiver.property.name : undefined +} + +/** + * Offsets of every `Array.prototype.with(index, value)` call: a two-argument `.with` on any + * receiver except an OpenTelemetry context object. Drizzle's one-argument `.with(cte)` and + * `index().with({ … })` never match. + */ +function findArrayWithCalls(file: string, content: string): number[] { + let program: unknown + try { + program = parse(content, { + sourceType: 'module', + plugins: ['typescript', ...(/\.[jt]sx$/.test(file) ? (['jsx'] as const) : [])], + errorRecovery: true, + }).program + } catch (error) { + throw new Error(`Cannot parse ${file} to check its .with calls: ${getErrorMessage(error)}`) + } + if (!isSyntaxNode(program)) return [] + + const offsets: number[] = [] + walkNodes(program, (node) => { + if (node.type !== 'CallExpression' && node.type !== 'OptionalCallExpression') return + if (!Array.isArray(node.arguments) || node.arguments.length !== 2) return + const callee = node.callee + if (!isSyntaxNode(callee) || callee.computed === true || !isSyntaxNode(callee.property)) return + if (callee.type !== 'MemberExpression' && callee.type !== 'OptionalMemberExpression') return + if (callee.property.name !== 'with') return + if (OTEL_CONTEXT_RECEIVER.test(receiverName(callee.object) ?? '')) return + offsets.push(callee.property.start) + }) + return offsets +} + async function walk(dir: string, results: string[] = []): Promise { let entries try { @@ -285,6 +358,11 @@ async function main() { matches.push({ index: match.index, description, suggestion }) } } + if (WITH_CALL.test(content)) { + for (const index of findArrayWithCalls(rel, content)) { + matches.push({ index, ...ES2023_ARRAY_METHOD }) + } + } if (matches.length === 0) continue const lines = content.split('\n') diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index 6251024d640..55ff60aacbf 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,14 +300,121 @@ function auditFile(file: string, source: string): Violation[] { const PERSIST_IMPORT = /import\s*\{[^}]*\bpersist\b(?:\s+as\s+(\w+))?[^}]*\}\s*from\s*'zustand\/middleware'/ -/** Removes comments while leaving string literals (which may contain `//`) intact. */ -function stripComments(code: string): string { - return code.replace( - /('(?:\\.|[^'\\])*'|"(?:\\.|[^"\\])*"|`(?:\\.|[^`\\])*`)|\/\*[\s\S]*?\*\/|\/\/[^\n]*/g, - (_, literal: string | undefined) => literal ?? '' +/** 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) + 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(param: unknown): { name: string; reason: string } | null { + 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 + return isSyntaxNode(property.value) ? property.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` — @@ -315,41 +424,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 args = splitTopLevelArguments( - stripComments(source.slice(openParenIndex + 1, closeParenIndex)) - ) - const options = args.length > 1 ? args[args.length - 1] : '' - const hasPartialize = /(?:^|[{,])\s*partialize\s*[:(,}]/.test(options) - /** - * `(s) => s`, `(s) => ({ ...s })`, or a block body returning either — with `s` also bound as - * `({ ...s })` — persists the whole state. - */ - const spreadsState = - /\bpartialize\s*:\s*\(?\s*(?:\{\s*\.\.\.)?(\w+)[^)]*\)?\s*=>\s*(?:\1\b(?!\s*[.[])|\(\s*\{\s*\.\.\.\1\b|\{[^}]*\breturn\s+(?:\1\b(?!\s*[.[])|\{\s*\.\.\.\1\b))/.test( - options - ) - 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 } From dc13597b7c8a055bb7fddd3cbc94dc90962b5c72 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 10:15:07 -0700 Subject: [PATCH 3/9] fix(audits): exempt only the imported OpenTelemetry context from the .with rule; unwrap cast partialize --- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- scripts/check-utils-enforcement.ts | 63 ++++++++++++++++++++----- scripts/check-zustand-v5-selectors.ts | 4 +- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index 9964840c5dd..d2f01359ed7 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 every two-argument `.with(index, value)` call on any receiver except an OpenTelemetry context object (a receiver named like `context`, `otelContext`, `otelContextApi`); a deliberate exception carries `// utils-lint-allow: `. +**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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import); a deliberate exception carries `// utils-lint-allow: `. ## Run independent awaits in parallel diff --git a/.cursor/rules/sim-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index 52ac260b8ab..c23bd0585b4 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 every two-argument `.with(index, value)` call on any receiver except an OpenTelemetry context object (a receiver named like `context`, `otelContext`, `otelContextApi`); a deliberate exception carries `// utils-lint-allow: `. +**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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import); a deliberate exception carries `// utils-lint-allow: `. ## Run independent awaits in parallel diff --git a/scripts/check-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index 91ba7c9bf23..65d49819118 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -176,9 +176,6 @@ const BANNED_PATTERNS: Array<{ /** Cheap gate: only files that contain a `.with(` call are parsed. */ const WITH_CALL = /\.with\s*\(/ -/** OpenTelemetry's `context.with(ctx, fn)` shares the shape; its receivers are named for the context API. */ -const OTEL_CONTEXT_RECEIVER = /context/i - /** A Babel AST node, read structurally rather than through `@babel/types`. */ interface SyntaxNode extends Record { type: string @@ -200,19 +197,58 @@ function walkNodes(node: SyntaxNode, visit: (node: SyntaxNode) => void): void { } } -/** The name a member call is made on: `arr` in `arr.with(…)`, `items` in `this.items.with(…)`. */ -function receiverName(receiver: unknown): string | undefined { - if (!isSyntaxNode(receiver)) return undefined - if (receiver.type === 'Identifier' && typeof receiver.name === 'string') return receiver.name - const isMember = - receiver.type === 'MemberExpression' || receiver.type === 'OptionalMemberExpression' - if (!isMember || receiver.computed === true || !isSyntaxNode(receiver.property)) return undefined - return typeof receiver.property.name === 'string' ? receiver.property.name : undefined +/** + * Local names bound to OpenTelemetry's context API, whose `context.with(ctx, fn)` shares the + * array method's shape: `context` (or an alias) imported from `@opentelemetry/api`, and any + * namespace import of it (whose `.context` member is the same object). + */ +function otelContextBindings(program: SyntaxNode): { + contexts: Set + namespaces: Set +} { + const contexts = new Set() + const namespaces = new Set() + const body = Array.isArray(program.body) ? program.body : [] + for (const statement of body) { + if (!isSyntaxNode(statement) || statement.type !== 'ImportDeclaration') continue + if (!isSyntaxNode(statement.source) || statement.source.value !== '@opentelemetry/api') continue + for (const specifier of Array.isArray(statement.specifiers) ? statement.specifiers : []) { + if (!isSyntaxNode(specifier) || !isSyntaxNode(specifier.local)) continue + const local = specifier.local.name + if (typeof local !== 'string') continue + if (specifier.type === 'ImportNamespaceSpecifier') namespaces.add(local) + else if ( + specifier.type === 'ImportSpecifier' && + isSyntaxNode(specifier.imported) && + specifier.imported.name === 'context' + ) + contexts.add(local) + } + } + return { contexts, namespaces } +} + +/** Whether `receiver` is OpenTelemetry's context object: `context`, an alias, or `api.context`. */ +function isOtelContext( + receiver: unknown, + bindings: { contexts: Set; namespaces: Set } +): boolean { + if (!isSyntaxNode(receiver)) return false + if (receiver.type === 'Identifier') return bindings.contexts.has(String(receiver.name)) + return ( + receiver.type === 'MemberExpression' && + receiver.computed !== true && + isSyntaxNode(receiver.object) && + receiver.object.type === 'Identifier' && + bindings.namespaces.has(String(receiver.object.name)) && + isSyntaxNode(receiver.property) && + receiver.property.name === 'context' + ) } /** * Offsets of every `Array.prototype.with(index, value)` call: a two-argument `.with` on any - * receiver except an OpenTelemetry context object. Drizzle's one-argument `.with(cte)` and + * receiver except OpenTelemetry's context API (resolved through its `@opentelemetry/api` import). Drizzle's one-argument `.with(cte)` and * `index().with({ … })` never match. */ function findArrayWithCalls(file: string, content: string): number[] { @@ -228,6 +264,7 @@ function findArrayWithCalls(file: string, content: string): number[] { } if (!isSyntaxNode(program)) return [] + const bindings = otelContextBindings(program) const offsets: number[] = [] walkNodes(program, (node) => { if (node.type !== 'CallExpression' && node.type !== 'OptionalCallExpression') return @@ -236,7 +273,7 @@ function findArrayWithCalls(file: string, content: string): number[] { if (!isSyntaxNode(callee) || callee.computed === true || !isSyntaxNode(callee.property)) return if (callee.type !== 'MemberExpression' && callee.type !== 'OptionalMemberExpression') return if (callee.property.name !== 'with') return - if (OTEL_CONTEXT_RECEIVER.test(receiverName(callee.object) ?? '')) return + if (isOtelContext(callee.object, bindings)) return offsets.push(callee.property.start) }) return offsets diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index 55ff60aacbf..e89719aab73 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -410,7 +410,9 @@ function findPartialize(options: SyntaxNode): SyntaxNode | null { const keyName = isSyntaxNode(key) ? (key.type === 'Identifier' ? key.name : key.value) : null if (keyName !== 'partialize') continue if (property.type === 'ObjectMethod') return property - return isSyntaxNode(property.value) ? property.value : null + // 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 } From a7050ef21e9060b5c9effd79d6dc3a11d330faac Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 10:29:29 -0700 Subject: [PATCH 4/9] fix(audits): drop the OTel exemption for rebound names, catch optional .with calls and defaulted partialize params --- .agents/skills/add-block/SKILL.md | 2 +- .claude/rules/sim-integrations.md | 2 +- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-integrations.mdc | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- scripts/check-utils-enforcement.ts | 39 ++++++++++++++++++++++++- scripts/check-zustand-v5-selectors.ts | 5 +++- 7 files changed, 47 insertions(+), 7 deletions(-) diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md index b80277bcad2..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 the `id` of a subblock that has no `canonicalParamId`. A group member may share it, as `channel` does in the canonicalParamId Pattern below. +- `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/.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 d2f01359ed7..45a52d41eac 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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import); a deliberate exception carries `// utils-lint-allow: `. +**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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import, when the file does not redeclare that name); a deliberate exception carries `// utils-lint-allow: `. ## Run independent awaits in parallel 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 c23bd0585b4..2e1d6af625a 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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import); a deliberate exception carries `// utils-lint-allow: `. +**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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import, when the file does not redeclare that name); a deliberate exception carries `// utils-lint-allow: `. ## Run independent awaits in parallel diff --git a/scripts/check-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index 65d49819118..69606547052 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -174,7 +174,7 @@ const BANNED_PATTERNS: Array<{ ] /** Cheap gate: only files that contain a `.with(` call are parsed. */ -const WITH_CALL = /\.with\s*\(/ +const WITH_CALL = /\.with\s*(?:\?\.\s*)?\(/ /** A Babel AST node, read structurally rather than through `@babel/types`. */ interface SyntaxNode extends Record { @@ -228,6 +228,38 @@ function otelContextBindings(program: SyntaxNode): { return { contexts, namespaces } } +/** Names bound by a pattern: `a`, `{ a, b: c }`, `[a, ...rest]`, `a = 1`. */ +function patternNames(pattern: unknown, names: string[]): void { + if (!isSyntaxNode(pattern)) return + if (pattern.type === 'Identifier' && typeof pattern.name === 'string') names.push(pattern.name) + else if (pattern.type === 'AssignmentPattern') patternNames(pattern.left, names) + else if (pattern.type === 'RestElement') patternNames(pattern.argument, names) + else if (pattern.type === 'ArrayPattern' && Array.isArray(pattern.elements)) { + for (const element of pattern.elements) patternNames(element, names) + } else if (pattern.type === 'ObjectPattern' && Array.isArray(pattern.properties)) { + for (const property of pattern.properties) { + patternNames( + isSyntaxNode(property) && property.type === 'ObjectProperty' ? property.value : property, + names + ) + } + } +} + +/** Every name a variable, parameter, catch clause, function, or class declares in the file. */ +function declaredNames(program: SyntaxNode): Set { + const names: string[] = [] + walkNodes(program, (node) => { + if (node.type === 'VariableDeclarator') patternNames(node.id, names) + else if (node.type === 'CatchClause') patternNames(node.param, names) + else if (/Function|ObjectMethod|ClassMethod/.test(node.type)) { + if (Array.isArray(node.params)) for (const param of node.params) patternNames(param, names) + if (node.type === 'FunctionDeclaration') patternNames(node.id, names) + } else if (node.type === 'ClassDeclaration') patternNames(node.id, names) + }) + return new Set(names) +} + /** Whether `receiver` is OpenTelemetry's context object: `context`, an alias, or `api.context`. */ function isOtelContext( receiver: unknown, @@ -265,6 +297,11 @@ function findArrayWithCalls(file: string, content: string): number[] { if (!isSyntaxNode(program)) return [] const bindings = otelContextBindings(program) + // A name the file also declares elsewhere may be shadowed at the call; exempt only unique bindings. + for (const name of declaredNames(program)) { + bindings.contexts.delete(name) + bindings.namespaces.delete(name) + } const offsets: number[] = [] walkNodes(program, (node) => { if (node.type !== 'CallExpression' && node.type !== 'OptionalCallExpression') return diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index e89719aab73..7b8831b8e4d 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -372,7 +372,10 @@ function returnedExpressions(fn: SyntaxNode): unknown[] { * 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(param: unknown): { name: string; reason: string } | null { +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' } From 22dbd983f2c4d3906cdf785a3da90d6fefe93f73 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 10:45:08 -0700 Subject: [PATCH 5/9] fix(audits): treat logical and conditional partialize returns of the whole state as leaks --- scripts/check-zustand-v5-selectors.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/check-zustand-v5-selectors.ts b/scripts/check-zustand-v5-selectors.ts index 7b8831b8e4d..6492c87f313 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -346,6 +346,13 @@ function isIdentifierNamed(node: unknown, name: string): boolean { 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. + if (isSyntaxNode(unwrapped) && unwrapped.type === 'LogicalExpression') { + return isWholeBinding(unwrapped.left, name) || isWholeBinding(unwrapped.right, 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( From a40e36506c01981cb5b9bb65e0c2e7e7f9eaa7be Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 11:15:47 -0700 Subject: [PATCH 6/9] improvement(audits): let tsc guard ES2023 methods, tighten ratchet hints, fix && partialize false positive - check:utils drops the ES2023 call-site detector; every tsconfig keeps lib at ES2022 so tsc rejects toSorted/with by type, and the check now fails a tsconfig that raises lib past it (the #5340 cause) - check:zustand-v5 treats only the right side of && as a returned value - check:explicit-any counts group-level and bare lint biome-ignore suppressions, and no longer pairs a deleted file with an unrelated new one as a rename - check:file-names drops its rule-matched rename hint, which pointed new violations at old baseline entries - check:comment-hygiene skips the parse for files with no possible hit and scans untracked files - inline the single-consumer source-kind helper and two leftover ref aliases - CLAUDE.md naming states the check:file-names scope; /ship runs type-check --- .agents/skills/ship/SKILL.md | 5 +- .claude/rules/sim-components.md | 2 +- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-components.mdc | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- CLAUDE.md | 4 +- .../components/table-filter/table-filter.tsx | 5 +- apps/sim/hooks/mcp/use-mcp-oauth-popup.ts | 5 +- scripts/check-client-boundary-imports.ts | 27 ++- scripts/check-comment-hygiene.ts | 35 ++- scripts/check-explicit-any.ts | 31 +-- scripts/check-file-names.ts | 9 - scripts/check-utils-enforcement.ts | 200 +++--------------- scripts/check-zustand-v5-selectors.ts | 6 +- scripts/source-kind.ts | 25 --- 15 files changed, 124 insertions(+), 236 deletions(-) delete mode 100644 scripts/source-kind.ts 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..0ae35aa6b41 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 every tsconfig keeps `lib` at 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-react-performance.md b/.claude/rules/sim-react-performance.md index 45a52d41eac..21a6253c479 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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import, when the file does not redeclare that name); a deliberate exception carries `// utils-lint-allow: `. +**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 ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/.cursor/rules/sim-components.mdc b/.cursor/rules/sim-components.mdc index 15be6706173..1c5fcdea1b1 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 every tsconfig keeps `lib` at 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-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index 2e1d6af625a..c4f963c3df2 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 every two-argument `.with(index, value)` call on any receiver except OpenTelemetry's context API (the `context` export of `@opentelemetry/api`, under any alias or via a namespace import, when the file does not redeclare that name); a deliberate exception carries `// utils-lint-allow: `. +**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 ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. ## Run independent awaits in parallel 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-client-boundary-imports.ts b/scripts/check-client-boundary-imports.ts index fb0d977bec1..ed9cace6ed8 100644 --- a/scripts/check-client-boundary-imports.ts +++ b/scripts/check-client-boundary-imports.ts @@ -63,9 +63,34 @@ */ import { readdir, readFile } from 'node:fs/promises' import path from 'node:path' -import { directiveOn, leadingDirective } from './source-kind' const ROOT = path.resolve(import.meta.dir, '..') + +/** 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. + */ +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. + */ +function leadingDirective(content: string): string | null { + return directiveOn(content.replace(LEADING_COMMENTS, '').split('\n', 1)[0]) +} const APP_DIR = path.join(ROOT, 'apps/sim') /** Everything Next compiles into the app's module graph. */ const DIRECTIVE_SCAN_DIRS = [path.join(ROOT, 'apps'), path.join(ROOT, 'packages')] 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-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index 69606547052..e30a17aa2f5 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -4,9 +4,13 @@ * * 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 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). * * 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,10 +21,9 @@ * 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' -import { parse } from '@babel/parser' -import { getErrorMessage } from '@sim/utils/errors' const ROOT = path.resolve(import.meta.dir, '..') @@ -53,14 +56,6 @@ const TRUNCATE_PREFILTER = /\.(?:slice|substring)\(\s*0\s*,[^)]*\)\s*(?:\}|\+)/ /** Literal gate shared by the `filterUndefined` and `omit` patterns. */ const FROM_ENTRIES = /Object\.fromEntries\(/ -/** Shared by the toSorted/toReversed/toSpliced pattern and the AST-based `.with` check. */ -const ES2023_ARRAY_METHOD = { - description: - 'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)', - suggestion: - 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice(), or [...arr] then next[i] = value', -} - const BANNED_PATTERNS: Array<{ pattern: RegExp description: string @@ -154,10 +149,6 @@ const BANNED_PATTERNS: Array<{ suggestion: 'escapeRegExp(value) from @sim/utils/string', }, // Render-path rules (.claude/rules/sim-react-performance.md, sim-styling.md) - { - pattern: /\.(?:toSorted|toReversed|toSpliced)\s*\(/g, - ...ES2023_ARRAY_METHOD, - }, { pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g, description: 'useRef(new X()) allocates a throwaway X on every render', @@ -173,149 +164,6 @@ const BANNED_PATTERNS: Array<{ }, ] -/** Cheap gate: only files that contain a `.with(` call are parsed. */ -const WITH_CALL = /\.with\s*(?:\?\.\s*)?\(/ - -/** A Babel AST node, read structurally rather than through `@babel/types`. */ -interface SyntaxNode extends Record { - type: string - start: number -} - -function isSyntaxNode(value: unknown): value is SyntaxNode { - return ( - typeof value === 'object' && value !== null && 'type' in value && typeof value.type === 'string' - ) -} - -function walkNodes(node: SyntaxNode, visit: (node: SyntaxNode) => void): void { - visit(node) - 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) - } -} - -/** - * Local names bound to OpenTelemetry's context API, whose `context.with(ctx, fn)` shares the - * array method's shape: `context` (or an alias) imported from `@opentelemetry/api`, and any - * namespace import of it (whose `.context` member is the same object). - */ -function otelContextBindings(program: SyntaxNode): { - contexts: Set - namespaces: Set -} { - const contexts = new Set() - const namespaces = new Set() - const body = Array.isArray(program.body) ? program.body : [] - for (const statement of body) { - if (!isSyntaxNode(statement) || statement.type !== 'ImportDeclaration') continue - if (!isSyntaxNode(statement.source) || statement.source.value !== '@opentelemetry/api') continue - for (const specifier of Array.isArray(statement.specifiers) ? statement.specifiers : []) { - if (!isSyntaxNode(specifier) || !isSyntaxNode(specifier.local)) continue - const local = specifier.local.name - if (typeof local !== 'string') continue - if (specifier.type === 'ImportNamespaceSpecifier') namespaces.add(local) - else if ( - specifier.type === 'ImportSpecifier' && - isSyntaxNode(specifier.imported) && - specifier.imported.name === 'context' - ) - contexts.add(local) - } - } - return { contexts, namespaces } -} - -/** Names bound by a pattern: `a`, `{ a, b: c }`, `[a, ...rest]`, `a = 1`. */ -function patternNames(pattern: unknown, names: string[]): void { - if (!isSyntaxNode(pattern)) return - if (pattern.type === 'Identifier' && typeof pattern.name === 'string') names.push(pattern.name) - else if (pattern.type === 'AssignmentPattern') patternNames(pattern.left, names) - else if (pattern.type === 'RestElement') patternNames(pattern.argument, names) - else if (pattern.type === 'ArrayPattern' && Array.isArray(pattern.elements)) { - for (const element of pattern.elements) patternNames(element, names) - } else if (pattern.type === 'ObjectPattern' && Array.isArray(pattern.properties)) { - for (const property of pattern.properties) { - patternNames( - isSyntaxNode(property) && property.type === 'ObjectProperty' ? property.value : property, - names - ) - } - } -} - -/** Every name a variable, parameter, catch clause, function, or class declares in the file. */ -function declaredNames(program: SyntaxNode): Set { - const names: string[] = [] - walkNodes(program, (node) => { - if (node.type === 'VariableDeclarator') patternNames(node.id, names) - else if (node.type === 'CatchClause') patternNames(node.param, names) - else if (/Function|ObjectMethod|ClassMethod/.test(node.type)) { - if (Array.isArray(node.params)) for (const param of node.params) patternNames(param, names) - if (node.type === 'FunctionDeclaration') patternNames(node.id, names) - } else if (node.type === 'ClassDeclaration') patternNames(node.id, names) - }) - return new Set(names) -} - -/** Whether `receiver` is OpenTelemetry's context object: `context`, an alias, or `api.context`. */ -function isOtelContext( - receiver: unknown, - bindings: { contexts: Set; namespaces: Set } -): boolean { - if (!isSyntaxNode(receiver)) return false - if (receiver.type === 'Identifier') return bindings.contexts.has(String(receiver.name)) - return ( - receiver.type === 'MemberExpression' && - receiver.computed !== true && - isSyntaxNode(receiver.object) && - receiver.object.type === 'Identifier' && - bindings.namespaces.has(String(receiver.object.name)) && - isSyntaxNode(receiver.property) && - receiver.property.name === 'context' - ) -} - -/** - * Offsets of every `Array.prototype.with(index, value)` call: a two-argument `.with` on any - * receiver except OpenTelemetry's context API (resolved through its `@opentelemetry/api` import). Drizzle's one-argument `.with(cte)` and - * `index().with({ … })` never match. - */ -function findArrayWithCalls(file: string, content: string): number[] { - let program: unknown - try { - program = parse(content, { - sourceType: 'module', - plugins: ['typescript', ...(/\.[jt]sx$/.test(file) ? (['jsx'] as const) : [])], - errorRecovery: true, - }).program - } catch (error) { - throw new Error(`Cannot parse ${file} to check its .with calls: ${getErrorMessage(error)}`) - } - if (!isSyntaxNode(program)) return [] - - const bindings = otelContextBindings(program) - // A name the file also declares elsewhere may be shadowed at the call; exempt only unique bindings. - for (const name of declaredNames(program)) { - bindings.contexts.delete(name) - bindings.namespaces.delete(name) - } - const offsets: number[] = [] - walkNodes(program, (node) => { - if (node.type !== 'CallExpression' && node.type !== 'OptionalCallExpression') return - if (!Array.isArray(node.arguments) || node.arguments.length !== 2) return - const callee = node.callee - if (!isSyntaxNode(callee) || callee.computed === true || !isSyntaxNode(callee.property)) return - if (callee.type !== 'MemberExpression' && callee.type !== 'OptionalMemberExpression') return - if (callee.property.name !== 'with') return - if (isOtelContext(callee.object, bindings)) return - offsets.push(callee.property.start) - }) - return offsets -} - async function walk(dir: string, results: string[] = []): Promise { let entries try { @@ -398,6 +246,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) { @@ -432,11 +305,6 @@ async function main() { matches.push({ index: match.index, description, suggestion }) } } - if (WITH_CALL.test(content)) { - for (const index of findArrayWithCalls(rel, content)) { - matches.push({ index, ...ES2023_ARRAY_METHOD }) - } - } if (matches.length === 0) continue const lines = content.split('\n') @@ -454,6 +322,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 6492c87f313..10318c36337 100644 --- a/scripts/check-zustand-v5-selectors.ts +++ b/scripts/check-zustand-v5-selectors.ts @@ -346,9 +346,11 @@ function isIdentifierNamed(node: unknown, name: string): boolean { 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. + // `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') { - return isWholeBinding(unwrapped.left, name) || isWholeBinding(unwrapped.right, name) + 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) 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]) -} From 93ef435d131c55d3b319845f3fe819e905c89ab0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 11:44:37 -0700 Subject: [PATCH 7/9] chore(guidance): state the tsconfig lib invariant as at or below ES2022 --- .claude/rules/sim-components.md | 2 +- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-components.mdc | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- scripts/check-utils-enforcement.ts | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.claude/rules/sim-components.md b/.claude/rules/sim-components.md index 0ae35aa6b41..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; `tsc` rejects the ES2023 array methods, because every tsconfig keeps `lib` at ES2022. +- `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-react-performance.md b/.claude/rules/sim-react-performance.md index 21a6253c479..732aca4d5e4 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. Every tsconfig keeps `"lib"` at ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. +**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 (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. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/.cursor/rules/sim-components.mdc b/.cursor/rules/sim-components.mdc index 1c5fcdea1b1..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; `tsc` rejects the ES2023 array methods, because every tsconfig keeps `lib` at ES2022. +- `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-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index c4f963c3df2..f5e8c62e1bc 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. Every tsconfig keeps `"lib"` at ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. +**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 (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. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/scripts/check-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index e30a17aa2f5..02919a24d03 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -8,9 +8,9 @@ * convention. * * ES2023 array methods (`toSorted`, `with`, …) throw on Safari/iOS 15, and SWC does not polyfill - * them. Every tsconfig keeps `lib` at 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). + * 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). * * 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'` From 0db61dd33f11acba4b02e5f5c964a0b90f0bc4e5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 12:12:06 -0700 Subject: [PATCH 8/9] fix(audits): keep matching toSorted/toReversed/toSpliced in source for any receivers --- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- scripts/check-utils-enforcement.ts | 9 ++++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index 732aca4d5e4..36cc4d487f4 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. Every tsconfig keeps `"lib"` at or below ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. +**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 (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. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/.cursor/rules/sim-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index f5e8c62e1bc..dea01fda974 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. Every tsconfig keeps `"lib"` at or below ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. +**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 (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. Never raise it to make one type-check. ## Run independent awaits in parallel diff --git a/scripts/check-utils-enforcement.ts b/scripts/check-utils-enforcement.ts index 02919a24d03..52ab18ab7cb 100644 --- a/scripts/check-utils-enforcement.ts +++ b/scripts/check-utils-enforcement.ts @@ -10,7 +10,8 @@ * 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). + * 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'` @@ -149,6 +150,12 @@ const BANNED_PATTERNS: Array<{ suggestion: 'escapeRegExp(value) from @sim/utils/string', }, // Render-path rules (.claude/rules/sim-react-performance.md, sim-styling.md) + { + // 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()', + }, { pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g, description: 'useRef(new X()) allocates a throwaway X on every render', From a11e051cb171886ef30c793a54077e6544c82666 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 2 Oct 2026 12:25:10 -0700 Subject: [PATCH 9/9] chore(guidance): scope the ES2023 tsc guarantee to typed receivers --- .claude/rules/sim-react-performance.md | 2 +- .cursor/rules/sim-react-performance.mdc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index 36cc4d487f4..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. Every tsconfig keeps `"lib"` at or below ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. +**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-react-performance.mdc b/.cursor/rules/sim-react-performance.mdc index dea01fda974..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. Every tsconfig keeps `"lib"` at or below ES2022 so `tsc` rejects these at each call site (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. Never raise it to make one type-check. +**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