Skip to content

Commit 2cef19f

Browse files
authored
improvement(audits): ratchet unused exports, explicit any, and file names (#8550)
* improvement(audits): ratchet knip unused exports, types, and duplicates check:unused-exports runs knip once with the dead-code issue types gated at zero plus exports/types/duplicates compared against a shrink-only baseline of path#symbol entries. Package entry exports stay public surface via an explicit includeEntryExports: false. run-audits skips check:dead-code since this pass covers it. * improvement(audits): ratchet explicit any and non-null assertions per file check:explicit-any runs Biome's noExplicitAny and noNonNullAssertion rules (off repo-wide) and fails when a file's count rises or drops without a baseline update. * improvement(audits): enforce file naming conventions with a ratchet check:file-names flags non-kebab-case paths, utils/helpers files that repeat their folder's role, and files that repeat their parent folder's name, with the expected short name in the failure output. * docs(agents): point naming and any rules at their checks; run root test in ship gate * improvement(audits): make baseline updates shrink-only and tolerate biome diagnostic exit codes * improvement(audits): fail closed on missing baselines, write nothing on refused updates, flag utils/utils.ts * improvement(audits): include root scripts in the explicit-any ratchet * fix(audits): gate every knip issue type and exclude generated contracts from the export ratchet knip's dependencies include also reports optionalPeerDependencies, which the strict list dropped. Gate every non-ratchet issue key so a new type fails closed. Generated contract files are excluded via ignoreIssues so rerunning their generators cannot trip the ratchet (359 baseline entries dropped). Print a rename hint when a baselined symbol moves files. * improvement(audits): lint tracked uploads source, reject any/! suppressions, name-check root scripts Anchor biome's build/out/uploads ignores to the real output and runtime dirs so apps/sim/lib/uploads and the uploads API routes are linted and counted by check:explicit-any (baseline grows only under those paths). check:explicit-any fails on biome-ignore comments for its two rules. check:file-names scans root scripts/ and vitest.shared.ts, allows Next.js interception segments and dot-prefixed names, ignores the old path of an unstaged mv, and points tool-mandated names at its allowlist. All three ratchets print a rename hint instead of only the shrink instruction. * improvement(audits): rebaseline ratchets on current staging, lint newly covered uploads files, anchor suppression detection to comments * improvement(audits): fail on unparsable files, refuse suppressed updates, require balanced dynamic segments
1 parent e3211dd commit 2cef19f

17 files changed

Lines changed: 9736 additions & 22 deletions

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ When the user runs `/ship`:
4242
- If the diff modifies UI code (any non-test `.tsx` file, or anything under `apps/sim/components/`, `apps/sim/hooks/`, or `apps/sim/stores/`), run `/cleanup`. It fans out the React/UI passes (effects, memo, callbacks, state, React Query, emcn, url-state), the comment pass, and the test-audit pass, and applies fixes so they land in this commit.
4343
- Otherwise, if the diff adds or changes tests (`*.test.ts(x)`, `*.integration.ts`, `**/e2e/**`, `apps/sim/scripts/test-*-e2e.ts`), run `/test-audit audit <changed test files>` on its own. Every new or changed test must pass the authoring gate; delete the ones that don't rather than shipping them.
4444
- Then run the test files the diff adds or changes, plus the existing tests beside changed source files, with `bun run --cwd <workspace> test <paths>` (`bun run --cwd apps/sim test <paths>` for the app; `*.integration.ts` needs the setup in `.claude/rules/sim-testing.md`). A failing test aborts ship.
45+
- 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.
4546
5. **Run migration safety** — only if the diff touches `packages/db/migrations/**` or `packages/db/schema.ts`:
4647
- Run `/db-migrate` to review the migration for zero-downtime safety (expand/contract phasing, backward-compatibility with the deployed app version).
4748
- `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.

‎CLAUDE.md‎

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

8686
## Code Conventions
8787

88-
- **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`.
88+
- **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 (`lib/logs/views.ts`, not `lib/logs/log-views.ts`; `utils/date.ts`, not `utils/date-utils.ts`); `check:file-names` enforces this.
8989
- **Imports**: absolute (`@/...`) only, never relative. 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`.
90-
- **TypeScript**: no `any` (use precise types or `unknown` with guards); a props interface for every component; `as const` for constant objects/arrays; explicit ref types (`useRef<HTMLDivElement>(null)`).
90+
- **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)`).
9191
- **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.
9292
- **Components**: `'use client'` only for hooks or browser APIs. 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()` on client paths): `.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.
9393
- **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`.

‎apps/sim/lib/uploads/contexts/workspace/workspace-file-manager-errors.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing'
2-
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { afterAll, beforeEach, describe, expect, it } from 'vitest'
33
import { listWorkspaceFiles } from './workspace-file-manager'
44

55
afterAll(resetDbChainMock)

‎apps/sim/lib/uploads/contexts/workspace/workspace-file-query.test.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,6 @@ describe('listWorkspaceFiles', () => {
152152
resetDbChainMock()
153153
})
154154

155-
const lastProjection = () =>
156-
Object.keys((dbChainMockFns.select.mock.calls.at(-1)?.[0] ?? {}) as Record<string, unknown>)
157-
158155
it('caps the rows read when the caller only needs to fit a budget', async () => {
159156
queueTableRows(schemaMock.workspaceFiles, [buildRow()])
160157

‎apps/sim/lib/uploads/providers/s3/client.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const {
99
mockSend,
10-
mockS3Client,
1110
mockS3ClientConstructor,
1211
mockPutObjectCommand,
1312
mockGetObjectCommand,

‎apps/sim/lib/uploads/server/markdown-export.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ describe('Markdown export image rewriting', () => {
4040

4141
it('returns large documents verbatim before parsing or fetching assets', async () => {
4242
const content = Buffer.from(
43-
'![image](/api/files/view/image-1)\n' + 'a'.repeat(MAX_EXPORT_MARKDOWN_PARSE_BYTES)
43+
`![image](/api/files/view/image-1)\n${'a'.repeat(MAX_EXPORT_MARKDOWN_PARSE_BYTES)}`
4444
)
4545
const result = await createMarkdownExport({
4646
content,

‎apps/sim/lib/uploads/utils/file-utils.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,6 @@ import {
1010
import { isUuid } from '@/executor/constants'
1111
import type { UserFile } from '@/executor/types'
1212

13-
interface FileAttachment {
14-
id: string
15-
key: string
16-
filename: string
17-
media_type: string
18-
size: number
19-
}
20-
2113
export interface MessageContent {
2214
type: 'text' | 'image' | 'document' | 'audio' | 'video'
2315
text?: string

‎biome.json‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@
88
"!**/.next",
99
"!**/.next",
1010
"!**/next-env.d.ts",
11-
"!**/out",
11+
"!out",
12+
"!apps/*/out",
13+
"!packages/*/out",
1214
"!**/dist",
13-
"!**/build",
15+
"!build",
16+
"!apps/*/build",
17+
"!packages/*/build",
1418
"!**/node_modules",
1519
"!**/.bun",
1620
"!**/.cache",
@@ -32,7 +36,8 @@
3236
"!**/apps/desktop/release",
3337
"!**/venv",
3438
"!**/.venv",
35-
"!**/uploads",
39+
"!uploads",
40+
"!apps/*/uploads",
3641
"!**/apps/sim/lib/execution/sandbox/bundles/*.cjs",
3742
"!**/test-results",
3843
"!**/playwright-report"

‎knip.jsonc‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
{
22
"$schema": "https://unpkg.com/knip@6/schema.json",
3-
// Gate reachability and dependency ownership. Export/type pruning needs review
4-
// of public package contracts and test-only consumers, not a blanket threshold.
3+
// Plain `knip` (check:dead-code) gates reachability and dependency ownership.
4+
// Unused exports, types, and duplicates are ratcheted by check:unused-exports,
5+
// which reuses this config in the same knip pass.
56
"include": ["files", "dependencies", "unlisted", "unresolved"],
7+
// knip's default, stated so it is a decision: no entry file's exports are reported.
8+
// That keeps package `exports`/`main`/`bin` contracts (ts-sdk, emcn, cli, …) and framework
9+
// entries (Next routes, Trigger tasks) public, and also exempts the other configured entries
10+
// (scripts, `*.integration.ts`, `background/**`, desktop and SDK examples).
11+
"includeEntryExports": false,
612
"workspaces": {
713
".": {
814
"entry": ["scripts/**/*.{ts,tsx}", "vitest.shared.ts"],
@@ -33,6 +39,9 @@
3339
// Required/discoverable barrels whose children have direct live imports.
3440
// Ignore only the barrel file finding, so it cannot keep dead children alive.
3541
"ignoreIssues": {
42+
// Generated contracts mirror their source of truth; regenerating them must not
43+
// trip the unused-export ratchet, and hand edits would be overwritten.
44+
"lib/mothership/generated/**": ["exports", "types", "duplicates"],
3645
"sandbox-tasks/index.ts": ["files"],
3746
"components/mcp/index.ts": ["files"],
3847
"triggers/quickbooks/index.ts": ["files"],
@@ -87,7 +96,11 @@
8796
},
8897
"packages/ts-sdk": { "entry": ["examples/*.ts"] },
8998
// The contract audit reads this snapshot by filename.
90-
"packages/desktop-bridge": { "entry": ["contract-snapshot.ts"] },
99+
// Generated, so its export surface is not ratcheted either.
100+
"packages/desktop-bridge": {
101+
"entry": ["contract-snapshot.ts"],
102+
"ignoreIssues": { "contract-snapshot.ts": ["exports", "types", "duplicates"] }
103+
},
91104
// This shared config is consumed by apps that own the Next dependency.
92105
"packages/tsconfig": { "ignoreUnresolved": ["next"] }
93106
}

‎package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@
8282
"check:desktop-bridge": "bun run scripts/check-desktop-bridge-contract.ts --check",
8383
"check:desktop-ipc": "bun run scripts/check-desktop-ipc-contract.ts",
8484
"check:route-verbs": "bun run scripts/check-route-verbs.ts",
85+
"check:unused-exports": "bun run scripts/check-unused-exports.ts",
86+
"check:explicit-any": "bun run scripts/check-explicit-any.ts",
87+
"check:file-names": "bun run scripts/check-file-names.ts",
8588
"desktop-bridge-contract:update": "bun run scripts/check-desktop-bridge-contract.ts --update",
8689
"mship-contracts:generate": "bun run scripts/sync-mothership-stream-contract.ts",
8790
"mship-contracts:check": "bun run scripts/sync-mothership-stream-contract.ts --check",

0 commit comments

Comments
 (0)