Skip to content

Commit fb900db

Browse files
committed
improvement(audits): close detector gaps in check:utils and the deployment-shape rule
- check:utils: match useRef(new X()) with nested generics, honor utils-lint-allow above formatter-wrapped statements, drop h-screen/w-screen from the size-N rule, and skip server-only App Router files in the ES2023 rule - deployment-shape rule: cover stores/, hooks/, blocks/ and surface hooks, read namespace imports, derive the flag list from deployment-shape.ts, parse long import clauses whole, and allowlist the panel store's module-init isChatEnabled - zustand persist message names the hoisted-options escape - biome: allow console in *.integration.ts, *.spec.ts, and desktop e2e
1 parent 4ebbf7a commit fb900db

4 files changed

Lines changed: 98 additions & 43 deletions

File tree

‎biome.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@
162162
"**/scripts/**",
163163
"**/*.test.ts",
164164
"**/*.test.tsx",
165+
"**/*.integration.ts",
166+
"**/*.spec.ts",
167+
"apps/desktop/e2e/**",
165168
"apps/sim/vitest.setup.ts",
166169
"packages/cli/**",
167170
"packages/sim-cli/**",

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

Lines changed: 70 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,16 @@
4646
* Escape hatch: `// client-boundary-allow: <reason>` on the line directly above
4747
* the import (reason required). Use only for a genuinely browser-only code path.
4848
*
49-
* ## Deployment-shape flags in client surfaces
49+
* ## Deployment-shape flags in client code
5050
*
51-
* A `'use client'` module under the workspace, organization, or standalone settings
52-
* surfaces must not import `isHosted`, `isBillingEnabled`, `isChatEnabled`, or the
53-
* enterprise feature flags from `env-flags`: those freeze at module init from the root
51+
* Client code — `stores/`, `hooks/`, `blocks/`, and any `'use client'` module or hook
52+
* under the workspace, organization, or standalone settings surfaces — must not read
53+
* `isHosted`, `isBillingEnabled`, `isChatEnabled`, or the enterprise feature flags from
54+
* `env-flags`, by named or namespace import: those freeze at module init from the root
5455
* layout's `NEXT_PUBLIC_*` transport, which a recovered 404 or `global-error` tab never
5556
* ran, so Sim Cloud renders as self-hosted. Read them through `useDeploymentShape()` /
56-
* `getDeploymentShape()` from `@/lib/core/config/deployment-shape` (CLAUDE.md).
57+
* `getDeploymentShape()` from `@/lib/core/config/deployment-shape` (CLAUDE.md). The flag
58+
* list is read from that module's own `env-flags` import, so it cannot drift.
5759
*
5860
* Usage:
5961
* bun run scripts/check-client-boundary-imports.ts # report
@@ -76,32 +78,52 @@ function isServerSurface(rel: string): boolean {
7678
return false
7779
}
7880

79-
/** `env-flags` exports that `@/lib/core/config/deployment-shape` re-serves to the browser. */
80-
const DEPLOYMENT_SHAPE_FLAGS = new Set([
81-
'isHosted',
82-
'isBillingEnabled',
83-
'isChatEnabled',
84-
'isAzureConfigured',
85-
'isCohereConfigured',
86-
'isAccessControlEnabled',
87-
'isAuditLogsEnabled',
88-
'isCustomBlocksEnabled',
89-
'isDataDrainsEnabled',
90-
'isDataRetentionEnabled',
91-
'isInboxEnabled',
92-
'isSandboxesEnabled',
93-
'isScimEnabled',
94-
'isSessionPoliciesEnabled',
95-
'isSsoEnabled',
96-
'isUsageMonitoringEnabled',
97-
'isWhitelabelingEnabled',
81+
const ENV_FLAGS = '@/lib/core/config/env-flags'
82+
const DEPLOYMENT_SHAPE_MODULE = path.join(APP_DIR, 'lib/core/config/deployment-shape.ts')
83+
84+
/**
85+
* Known deployment-shape violations awaiting a product decision (paths relative to apps/sim).
86+
* Each entry names why it cannot simply move to the reader.
87+
*/
88+
const DEPLOYMENT_SHAPE_ALLOWLIST = new Set([
89+
// Picks the panel's default tab from `isChatEnabled` at module init, before any surface
90+
// seeds the shape; moving it to the reader changes the first-render tab, a product call.
91+
'stores/panel/store.ts',
9892
])
9993

10094
/** Surfaces whose shell seeds the server-resolved deployment shape (paths relative to apps/sim). */
10195
function isDeploymentShapeSurface(rel: string): boolean {
10296
return /^(?:app\/(?:workspace|o|account|selfhost\/settings)|components\/settings|ee)\//.test(rel)
10397
}
10498

99+
/**
100+
* Client code bound by the deployment-shape rule: client-only directories by path, and
101+
* `'use client'` modules or hooks (a `hooks/` folder or `use-*` file) inside a surface.
102+
*/
103+
async function isDeploymentShapeClient(rel: string, absFile: string): Promise<boolean> {
104+
if (/\.(?:test|spec|integration)\.tsx?$/.test(rel)) return false
105+
if (/^(?:stores|hooks|blocks)\//.test(rel)) return true
106+
if (!isDeploymentShapeSurface(rel)) return false
107+
return /(?:^|\/)(?:hooks\/|use-[^/]+\.tsx?$)/.test(rel) || isUseClientModule(absFile)
108+
}
109+
110+
/**
111+
* Exports of `env-flags` an import clause reads: its named members, or, for a namespace
112+
* import (`* as flags`), every `flags.<name>` access in the file.
113+
*/
114+
function envFlagReads(clause: string, content: string): string[] {
115+
const namespace = /^\*\s+as\s+(\w+)$/.exec(clause.trim())?.[1]
116+
if (namespace) {
117+
return [...content.matchAll(new RegExp(`\\b${namespace}\\.(\\w+)`, 'g'))].map((m) => m[1])
118+
}
119+
if (!clause.includes('{')) return []
120+
return clause
121+
.slice(clause.indexOf('{') + 1, clause.lastIndexOf('}'))
122+
.split(',')
123+
.map((member) => member.trim().split(/\s+as\s+/)[0])
124+
.filter(Boolean)
125+
}
126+
105127
const SOURCE_EXTENSIONS = ['.ts', '.tsx']
106128
const ALLOW_DIRECTIVE = 'client-boundary-allow'
107129
const sourceCache = new Map<string, string>()
@@ -231,9 +253,11 @@ function parseImports(content: string): ImportInfo[] {
231253
const imports: ImportInfo[] = []
232254
const re = /^\s*import\s+([\s\S]*?)\s+from\s+['"]([^'"]+)['"]/
233255
for (let i = 0; i < lines.length; i++) {
234-
if (!/^\s*import\b/.test(lines[i]) || !lines[i].includes('import')) continue
235-
// Join up to 12 following lines to capture multi-line import clauses.
236-
const block = lines.slice(i, i + 12).join('\n')
256+
if (!/^\s*import\b/.test(lines[i]) || /^\s*import\s*['"(]/.test(lines[i])) continue
257+
// Join through the `from` line so a long multi-line clause is captured whole.
258+
let end = i
259+
while (end < lines.length - 1 && !/\bfrom\s+['"]/.test(lines[end])) end++
260+
const block = lines.slice(i, end + 1).join('\n')
237261
const match = re.exec(block)
238262
if (!match) continue
239263
imports.push({ line: i + 1, clause: match[1], specifier: match[2] })
@@ -345,30 +369,40 @@ async function main() {
345369
}
346370
}
347371

372+
const shapeFlags = new Set(
373+
parseImports(await readSource(DEPLOYMENT_SHAPE_MODULE))
374+
.filter((imp) => imp.specifier === ENV_FLAGS)
375+
.flatMap((imp) => envFlagReads(imp.clause, ''))
376+
)
377+
if (shapeFlags.size === 0) {
378+
throw new Error(
379+
`${DEPLOYMENT_SHAPE_MODULE} no longer imports from env-flags; update this check`
380+
)
381+
}
348382
const shapeViolations: Array<Violation & { flags: string[] }> = []
349383
for (const absFile of allFiles) {
350384
if (!absFile.startsWith(`${APP_DIR}${path.sep}`)) continue
351385
const rel = path.relative(APP_DIR, absFile)
352-
if (!isDeploymentShapeSurface(rel) || !(await isUseClientModule(absFile))) continue
386+
if (DEPLOYMENT_SHAPE_ALLOWLIST.has(rel) || !(await isDeploymentShapeClient(rel, absFile))) {
387+
continue
388+
}
353389
const content = await readSource(absFile)
354390
for (const imp of parseImports(content)) {
355-
if (imp.specifier !== '@/lib/core/config/env-flags' || !importsAValue(imp.clause)) continue
356-
const braced = imp.clause.slice(imp.clause.indexOf('{') + 1, imp.clause.lastIndexOf('}'))
357-
const flags = braced
358-
.split(',')
359-
.map((member) => member.trim().split(/\s+as\s+/)[0])
360-
.filter((name) => DEPLOYMENT_SHAPE_FLAGS.has(name))
391+
if (imp.specifier !== ENV_FLAGS || !importsAValue(imp.clause)) continue
392+
const flags = [...new Set(envFlagReads(imp.clause, content))].filter((name) =>
393+
shapeFlags.has(name)
394+
)
361395
if (flags.length === 0 || hasAllowDirective(content, imp.line)) continue
362396
shapeViolations.push({ file: rel, line: imp.line, specifier: imp.specifier, flags })
363397
}
364398
}
365399

366400
if (shapeViolations.length === 0) {
367-
console.log('✓ No client settings surface reads deployment-shape flags from env-flags.')
401+
console.log('✓ No client code reads deployment-shape flags from env-flags.')
368402
} else {
369403
failed = true
370404
console.error(
371-
`\n✗ ${shapeViolations.length} 'use client' module(s) in a workspace/organization/settings surface import deployment-shape flags from env-flags.\n` +
405+
`\n✗ ${shapeViolations.length} client module(s) read deployment-shape flags from env-flags.\n` +
372406
` Those constants freeze from the root layout's NEXT_PUBLIC_* transport, so a recovered 404/global-error tab renders Sim Cloud as self-hosted.\n` +
373407
` Read them via useDeploymentShape() (components) or getDeploymentShape() (helpers) from @/lib/core/config/deployment-shape.\n`
374408
)

‎scripts/check-utils-enforcement.ts‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,17 @@ const ALLOWLISTED_FILES = new Set([
5656
/** Leading `'use client'` directive, after any comment header. */
5757
const USE_CLIENT_PROLOGUE = /^(?:\s*(?:\/\/[^\n]*|\/\*[\s\S]*?\*\/))*\s*['"]use client['"]/
5858

59+
/** App Router files Next only ever evaluates on the server, plus `*.server.ts` modules. */
60+
const SERVER_ONLY_FILE =
61+
/\/(?:route|sitemap|robots|manifest)\.[jt]sx?$|\/(?:opengraph-image|twitter-image|icon|apple-icon)\.[^/]+$|\.server\.[jt]sx?$/
62+
5963
/**
6064
* Files that ship to the browser: client directories plus any `'use client'` module. Server-only
61-
* code (route handlers, tests) may use Node 20+ runtime methods freely.
65+
* code (route handlers, metadata routes, tests) may use Node 20+ runtime methods freely.
6266
*/
6367
function isClientRenderPath(rel: string, content: string): boolean {
6468
if (/\.(test|spec)\.[cm]?[jt]sx?$/.test(rel) || /\/app\/api\//.test(rel)) return false
69+
if (SERVER_ONLY_FILE.test(rel)) return false
6570
return (
6671
/^apps\/sim\/(app|components|hooks|stores)\//.test(rel) ||
6772
/^apps\/docs\/(app|components)\//.test(rel) ||
@@ -165,13 +170,13 @@ const BANNED_PATTERNS: Array<{
165170
appliesTo: isClientRenderPath,
166171
},
167172
{
168-
pattern: /\buseRef(?:<[^>]*>)?\(\s*new\s+[A-Z]\w*/g,
173+
pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g,
169174
description: 'useRef(new X()) allocates a throwaway X on every render',
170175
suggestion: 'useRef<X | null>(null), then `ref.current ??= new X()` before first use',
171176
},
172177
{
173178
pattern:
174-
/(?:^|[ \t'"`])((?:[\w-]+:)*)(?:h-(\[[^\]\s]+\]|[\d.]+|px|full|screen|auto|fit|min|max)\s+\1w-\2|w-(\[[^\]\s]+\]|[\d.]+|px|full|screen|auto|fit|min|max)\s+\1h-\3)(?=[\s'"`]|$)/gm,
179+
/(?:^|[ \t'"`])((?:[\w-]+:)*)(?:h-(\[[^\]\s]+\]|[\d.]+|px|full|auto|fit|min|max)\s+\1w-\2|w-(\[[^\]\s]+\]|[\d.]+|px|full|auto|fit|min|max)\s+\1h-\3)(?=[\s'"`]|$)/gm,
175180
prefilter: /[hw]-\S+\s+(?:[\w-]+:)*[hw]-/,
176181
description: 'h-N w-N with equal N',
177182
suggestion: 'size-N (Tailwind) — e.g. `size-4`, `size-full`',
@@ -229,15 +234,28 @@ function lineAt(lineStarts: number[], offset: number): number {
229234
return low + 1
230235
}
231236

237+
/** The line before ends mid-expression: an open bracket, a comma, or a binary/arrow operator. */
238+
const ENDS_OPEN = /(?:[([,=?:+]|=>|&&|\|\||\?\?)$/
239+
/** The line starts mid-expression: a member access, a closing bracket, or a ternary/logical operator. */
240+
const STARTS_CONTINUED = /^(?:[.?:)\]]|&&|\|\|)/
241+
232242
/**
233243
* True if a `// utils-lint-allow: <reason>` annotation sits just above `line` (1-based).
234244
*
235245
* The reason must be non-empty: an annotation that does not say why is the thing this
236-
* check exists to prevent. Scans up to three comment lines above, so the annotation can
237-
* carry context lines with it.
246+
* check exists to prevent. A match the formatter wrapped onto a continuation line is first
247+
* walked up to its statement's first line (the repo omits semicolons, so continuation is
248+
* read from the bracket or operator at the seam), then up to three comment lines above
249+
* that are scanned, so the annotation can carry context lines with it.
238250
*/
239251
function hasAllow(lines: string[], line: number): boolean {
240-
for (let i = line - 2; i >= 0 && i >= line - 5; i--) {
252+
let start = line - 1
253+
while (start > 0) {
254+
const previous = lines[start - 1].trim()
255+
if (!ENDS_OPEN.test(previous) && !STARTS_CONTINUED.test(lines[start].trim())) break
256+
start--
257+
}
258+
for (let i = start - 1; i >= 0 && i >= start - 4; i--) {
241259
const text = lines[i]?.trim() ?? ''
242260
if (text.includes(ALLOW)) {
243261
return text.slice(text.indexOf(ALLOW) + ALLOW.length).trim().length > 0

‎scripts/check-zustand-v5-selectors.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ function auditPersist(file: string, source: string): Violation[] {
326326
line: lineNumberAt(source, match.index),
327327
description: spreadsState
328328
? 'persist partialize spreads the whole state; return an explicit whitelist of durable fields'
329-
: 'persist has no partialize; add `partialize: (state) => ({ <durable fields> })` (sim-stores.md)',
329+
: `persist has no partialize; add \`partialize: (state) => ({ <durable fields> })\` (sim-stores.md). If the options object is hoisted into a variable that has one, mark the call // ${SAFE_ANNOTATION} <where>`,
330330
snippet: oneLineSnippet(
331331
source,
332332
match.index,

0 commit comments

Comments
 (0)