Skip to content

Commit eb15627

Browse files
committed
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
1 parent 8e36b26 commit eb15627

15 files changed

Lines changed: 124 additions & 236 deletions

File tree

‎.agents/skills/ship/SKILL.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ When the user runs `/ship`:
4545
- 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.
4646
5. **Run migration safety** — only if the diff touches `packages/db/migrations/**` or `packages/db/schema.ts`:
4747
- Run `/db-migrate` to review the migration for zero-downtime safety (expand/contract phasing, backward-compatibility with the deployed app version).
48-
- `cd packages/db && bunx drizzle-kit generate && git status --porcelain ./migrations` must print nothing (CI's schema/migration sync step).
48+
- `(cd packages/db && bunx drizzle-kit generate && git status --porcelain ./migrations)` must print nothing (CI's schema/migration sync step).
4949
- `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.
5050
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.
5151
@@ -78,12 +78,13 @@ When the user runs `/ship`:
7878
# Runs every audit CI runs, concurrently, and replays the output of any that fail.
7979
# The audit list is derived in scripts/run-audits.ts — do not hand-list audits here.
8080
bun run check:audits || { echo "❌ audit(s) failed — do not ship"; exit 1; }
81+
bun run type-check || { echo "❌ type-check failed — do not ship"; exit 1; }
8182
# CI's "Verify docs manifest is in sync" step is not a `check:*` script, so the runner above
8283
# does not cover it. (CI's "Security audit" `bun audit` step is `continue-on-error` — advisory
8384
# only, not a gate — so it is deliberately not run here.)
8485
bun run docs-manifest:check || { echo "❌ docs manifest out of sync — do not ship"; exit 1; }
8586
```
86-
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.
87+
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.
8788
7. **Stage and commit** the changes with the generated message — including any files Phase A regenerated in step 6
8889
8. **Push to origin** using the current branch name — `--force-with-lease` if step 2's sync
8990
check did any history rewrite (a clean rebase or a cherry-pick rebuild) on a branch that had

‎.claude/rules/sim-components.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ When rendering or sorting a list of rows against a lookup collection (members, f
4040
react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix":
4141

4242
- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them.
43-
- `js-tosorted-immutable` — won't-fix anywhere; `check:utils` bans the ES2023 array methods repo-wide.
43+
- `js-tosorted-immutable` — won't-fix anywhere; `tsc` rejects the ES2023 array methods, because every tsconfig keeps `lib` at ES2022.
4444
- `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.
4545
- `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 `<input>`). 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).
4646
- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it.

‎.claude/rules/sim-react-performance.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ return items.sort(compare)
7777
return [...items].sort(compare)
7878
```
7979

80-
**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: <reason>`.
80+
**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.
8181

8282
## Run independent awaits in parallel
8383

‎.cursor/rules/sim-components.mdc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ When rendering or sorting a list of rows against a lookup collection (members, f
4141
react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix":
4242

4343
- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them.
44-
- `js-tosorted-immutable` — won't-fix anywhere; `check:utils` bans the ES2023 array methods repo-wide.
44+
- `js-tosorted-immutable` — won't-fix anywhere; `tsc` rejects the ES2023 array methods, because every tsconfig keeps `lib` at ES2022.
4545
- `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.
4646
- `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 `<input>`). 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).
4747
- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it.

‎.cursor/rules/sim-react-performance.mdc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ return items.sort(compare)
8080
return [...items].sort(compare)
8181
```
8282

83-
**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: <reason>`.
83+
**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.
8484

8585
## Run independent awaits in parallel
8686

‎CLAUDE.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ The `'use client'` server boundary, the app/worker runtime env split, and featur
8282

8383
## Code Conventions
8484

85-
- **Naming**: components PascalCase (`WorkflowList`); hooks `use*`; files kebab-case (`workflow-list.tsx`); constants SCREAMING_SNAKE_CASE; interfaces PascalCase with a suffix (`WorkflowListProps`); stores `stores/<feature>/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.
85+
- **Naming**: components PascalCase (`WorkflowList`); hooks `use*`; files kebab-case (`workflow-list.tsx`); constants SCREAMING_SNAKE_CASE; interfaces PascalCase with a suffix (`WorkflowListProps`); stores `stores/<feature>/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.
8686
- **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`.
8787
- **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<HTMLDivElement>(null)`).
88-
- **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.
88+
- **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.
8989
- **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.
9090
- **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`.
9191
- **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.

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,8 @@ export function TableFilter({
8787
if (deferredRule && !isCompleteRule(deferredRule)) {
8888
const previouslyAppliedRule = currentRules.find((rule) => rule.id === deferredRule.id)
8989
if (previouslyAppliedRule && isCompleteRule(previouslyAppliedRule)) {
90-
const deferredRules = deferredAppliedRules
91-
if (!deferredRules.has(deferredRule.id)) {
92-
deferredRules.set(deferredRule.id, previouslyAppliedRule)
90+
if (!deferredAppliedRules.has(deferredRule.id)) {
91+
deferredAppliedRules.set(deferredRule.id, previouslyAppliedRule)
9392
}
9493
}
9594
}

‎apps/sim/hooks/mcp/use-mcp-oauth-popup.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,12 @@ export function useMcpOauthPopup({ workspaceId }: UseMcpOauthPopupProps) {
125125
)
126126

127127
useEffect(() => {
128-
const pending = pendingFlows
129128
return () => {
130-
for (const { timeout, poll } of pending.values()) {
129+
for (const { timeout, poll } of pendingFlows.values()) {
131130
window.clearTimeout(timeout)
132131
if (poll !== undefined) window.clearInterval(poll)
133132
}
134-
pending.clear()
133+
pendingFlows.clear()
135134
}
136135
}, [])
137136

‎scripts/check-client-boundary-imports.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,34 @@
6363
*/
6464
import { readdir, readFile } from 'node:fs/promises'
6565
import path from 'node:path'
66-
import { directiveOn, leadingDirective } from './source-kind'
6766

6867
const ROOT = path.resolve(import.meta.dir, '..')
68+
69+
/** A lone directive statement, e.g. `'use server'` or `"use client";`. */
70+
const DIRECTIVE_STATEMENT = /^(['"])(use [a-z-]+)\1\s*;?$/
71+
72+
/**
73+
* The directive a single source line states, e.g. `use client`, or null. Notes may sit on the same
74+
* line, so `//` and inline `/* *\/` comments come off before matching.
75+
*/
76+
function directiveOn(line: string): string | null {
77+
const statement = line
78+
.replace(/\/\*.*?\*\//g, '')
79+
.replace(/\/\/.*$/, '')
80+
.trim()
81+
return DIRECTIVE_STATEMENT.exec(statement)?.[2] ?? null
82+
}
83+
84+
/** Comments and whitespace ahead of a module's first statement. */
85+
const LEADING_COMMENTS = /^(?:\s*(?:\/\/[^\n]*|\/\*[\s\S]*?\*\/))*\s*/
86+
87+
/**
88+
* The module's leading directive prologue, if any. A directive must be the first statement;
89+
* comments and blank lines may precede it.
90+
*/
91+
function leadingDirective(content: string): string | null {
92+
return directiveOn(content.replace(LEADING_COMMENTS, '').split('\n', 1)[0])
93+
}
6994
const APP_DIR = path.join(ROOT, 'apps/sim')
7095
/** Everything Next compiles into the app's module graph. */
7196
const DIRECTIVE_SCAN_DIRS = [path.join(ROOT, 'apps'), path.join(ROOT, 'packages')]

‎scripts/check-comment-hygiene.ts‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,16 @@ function firstCodeSpan(run: LineComment[], jsx: boolean): LineComment | undefine
153153
return undefined
154154
}
155155

156+
/**
157+
* A superset of every hit: a separator right after a comment opener, or a `//` line holding a
158+
* {@link CODE_PUNCTUATION} token. Files without one skip the parse, which dominates the run.
159+
*/
160+
const MAY_VIOLATE =
161+
/\/[/*][*\s]*(?:={3}|-{3}|─{3}|━{3}|\*{3}|~{3})|\/\/[^\n]*(?:[;{}]|=>|\b(?:const|let|return|await|import|export)\b|\w\.\w+\()/
162+
156163
/** Every banner and commented-out-code hit in one source file. */
157164
export function findViolations(file: string, source: string): Violation[] {
165+
if (!MAY_VIOLATE.test(source)) return []
158166
const jsx = /\.[jt]sx$/.test(file)
159167
let comments
160168
try {
@@ -213,12 +221,27 @@ export function findViolations(file: string, source: string): Violation[] {
213221
}
214222

215223
function sourceFiles(): string[] {
216-
return execFileSync('git', ['ls-files', '*.ts', '*.tsx', '*.mts', '*.cts', '*.mjs', '*.cjs'], {
217-
cwd: ROOT,
218-
encoding: 'utf8',
219-
// The listing is already ~1 MB, the default execFileSync ceiling.
220-
maxBuffer: 64 * 1024 * 1024,
221-
})
224+
return execFileSync(
225+
'git',
226+
[
227+
'ls-files',
228+
'--cached',
229+
'--others',
230+
'--exclude-standard',
231+
'*.ts',
232+
'*.tsx',
233+
'*.mts',
234+
'*.cts',
235+
'*.mjs',
236+
'*.cjs',
237+
],
238+
{
239+
cwd: ROOT,
240+
encoding: 'utf8',
241+
// The listing is already ~1 MB, the default execFileSync ceiling.
242+
maxBuffer: 64 * 1024 * 1024,
243+
}
244+
)
222245
.split('\n')
223246
.filter((file) => file && !EXCLUDED.some((pattern) => pattern.test(file)))
224247
}

0 commit comments

Comments
 (0)