fix(ssr): harden credential query-key factory + fetchers against the 'use client' stub bug (#5206)

* fix(ssr): move credential query-key factory + fetchers to non-client modules

Preventively closes the same 'use client' SSR client-reference-stub class that
crashed the tables page. Server-evaluated modules (the credential block def, the
workflow-comparison helpers) imported workspaceCredentialKeys /
fetchWorkspaceCredentialList / fetchCredentialSetById from 'use client' hook
modules, where they resolve to client-reference stubs on the server (a future
server call path would throw 'X is not a function').

Extract them into non-client hooks/queries/utils/{credential-keys,
fetch-workspace-credentials,fetch-credential-set}.ts (mirroring folder-keys.ts /
fetch-workflow-envelope.ts) and import from there. No behavior change — these
values were only ever called from browser paths.

* docs+ci: codify the 'use client' server-import rule + add check:client-boundary

Document the Next.js rule that server code can only render a 'use client'
export as a component, never call it (server imports resolve to client-reference
stubs that throw — the tables-page crash). Add the rule to
.claude/rules/sim-queries.md + a cross-ref in sim-architecture.md.

Add scripts/check-client-boundary-imports.ts (wired into CI as check:client-boundary)
that flags any value import from a 'use client' module in a server-evaluated,
non-JSX surface (prefetch / route handler / trigger / block definition), so this
class can't silently recur. Escape hatch: // client-boundary-allow: <reason>.
This commit is contained in:
Waleed
2026-06-24 18:46:18 -07:00
committed by GitHub
parent cff7a49310
commit 6260eda226
16 changed files with 328 additions and 55 deletions
+4
View File
@@ -38,6 +38,10 @@ packages/ # @sim/* — audit, auth, db, logger, realtime-protocol
- `apps/* → packages/*` only. Packages never import from `apps/*`.
- `apps/realtime` avoids Next.js, React, the block/tool registry, provider SDKs, and the executor; never add `@/lib/webhooks/providers/*`, `@/executor/*`, `@/blocks/*`, or `@/tools/*` imports to any package it consumes. CI enforces this via `scripts/check-monorepo-boundaries.ts` and `scripts/check-realtime-prune-graph.ts`.
## The `'use client'` server boundary
Every export of a `'use client'` module becomes a *client reference* on the server — server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can only *render* it as a component or pass it as a prop, never *call* it (doing so throws at runtime, e.g. `tableKeys.list is not a function`; `next build` does not catch it). Keep server-importable query primitives (key factories, fetchers, mappers, constants) in non-`'use client'` modules — see `.claude/rules/sim-queries.md`. Enforced by `scripts/check-client-boundary-imports.ts`.
## Feature Organization
Features live under `app/workspace/[workspaceId]/`:
+11
View File
@@ -27,6 +27,17 @@ Never use inline query keys — always use the factory.
**Every identifier the `queryFn` forwards into the fetch MUST appear in the `queryKey`.** (Query-machinery identifiers — `signal`, `pageParam` — are exempt; they aren't fetch-scoping args.) If the fetch is scoped by `workspaceId`, `cursor`, `limit`, an org id, etc., those values must be part of the key — otherwise distinct fetch args share one cache entry (a cross-tenant / per-param cache collision). The lone exception is a globally-unique id used as the key while a second fetch arg is only an authz scope that cannot collide; annotate those with `// rq-lint-allow: <reason>`. Enforced by the `key-fetch-arg-drift` check in `scripts/check-react-query-patterns.ts`.
## Server-importable query primitives must NOT live in a `'use client'` module
Next.js rewrites **every** export of a `'use client'` module into a *client reference* in the server bundle. Server-evaluated code — RSC `page.tsx`/`layout.tsx`, `prefetch.ts`, route handlers, **block definitions**, triggers/workers — can only *render* such an export as a component or pass it as a prop; **calling** one throws at runtime (`Attempted to call X from the server but X is on the client` — for an object export it surfaces as `X.list is not a function`). `next build` does **not** catch this — only SSR/runtime does.
So any **query-key factory, standalone `requestJson` fetcher, mapper, or constant** that a server module imports must live in a **non-`'use client'`** module:
- key factories → `hooks/queries/utils/<entity>-keys.ts` (see `folder-keys.ts`, `table-keys.ts`, `credential-keys.ts`)
- standalone fetchers/mappers → `hooks/queries/utils/fetch-*.ts` / `*-list-query.ts` (see `fetch-workflow-envelope.ts`, `fetch-credential-set.ts`)
The `'use client'` hook module then imports these back for its hooks. **Never** define a server-imported factory/fetcher directly in a `'use client'` hooks file — it crashes SSR (this caused the tables-page crash). Enforced for prefetch/route/trigger/block files by `scripts/check-client-boundary-imports.ts` (`bun run check:client-boundary`, run in CI). Escape hatch for a genuinely browser-only path: `// client-boundary-allow: <reason>` on the line above the import.
## File Structure
```typescript
+3
View File
@@ -122,6 +122,9 @@ jobs:
- name: React Query pattern audit
run: bun run check:react-query
- name: Client boundary import audit
run: bun run check:client-boundary
- name: Verify realtime prune graph
run: bun run check:realtime-prune
@@ -17,11 +17,7 @@ import type { WorkspaceEnvironmentData } from '@/lib/environment/api'
import { UnsavedChangesModal } from '@/app/workspace/[workspaceId]/components/credential-detail'
import { SecretValueField } from '@/app/workspace/[workspaceId]/settings/components/secrets/components/secret-value-field'
import { isValidEnvVarName } from '@/executor/constants'
import {
useWorkspaceCredentials,
type WorkspaceCredential,
workspaceCredentialKeys,
} from '@/hooks/queries/credentials'
import { useWorkspaceCredentials, type WorkspaceCredential } from '@/hooks/queries/credentials'
import {
usePersonalEnvironment,
useRemoveWorkspaceEnvironment,
@@ -29,6 +25,7 @@ import {
useUpsertWorkspaceEnvironment,
useWorkspaceEnvironment,
} from '@/hooks/queries/environment'
import { workspaceCredentialKeys } from '@/hooks/queries/utils/credential-keys'
import { useWorkspacePermissionsQuery } from '@/hooks/queries/workspace'
import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
+2 -1
View File
@@ -2,7 +2,8 @@ import { CredentialIcon } from '@/components/icons'
import { getServiceConfigByProviderId } from '@/lib/oauth/utils'
import { getQueryClient } from '@/app/_shell/providers/get-query-client'
import type { BlockConfig } from '@/blocks/types'
import { fetchWorkspaceCredentialList, workspaceCredentialKeys } from '@/hooks/queries/credentials'
import { workspaceCredentialKeys } from '@/hooks/queries/utils/credential-keys'
import { fetchWorkspaceCredentialList } from '@/hooks/queries/utils/fetch-workspace-credentials'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
interface CredentialBlockOutput {
+1 -13
View File
@@ -19,7 +19,6 @@ import {
createCredentialSetContract,
createCredentialSetInvitationContract,
deleteCredentialSetContract,
getCredentialSetContract,
leaveCredentialSetContract,
listCredentialSetInvitationDetailsContract,
listCredentialSetInvitationsContract,
@@ -29,6 +28,7 @@ import {
removeCredentialSetMemberContract,
resendCredentialSetInvitationContract,
} from '@/lib/api/contracts'
import { fetchCredentialSetById } from '@/hooks/queries/utils/fetch-credential-set'
export type {
CreateCredentialSetData,
@@ -76,18 +76,6 @@ export function useCredentialSets(organizationId?: string, enabled = true) {
})
}
export async function fetchCredentialSetById(
id: string,
signal?: AbortSignal
): Promise<CredentialSet | null> {
if (!id) return null
const data = await requestJson(getCredentialSetContract, {
params: { id },
signal,
})
return data.credentialSet ?? null
}
export function useCredentialSetDetail(id?: string, enabled = true) {
return useQuery<CredentialSet | null>({
queryKey: credentialSetKeys.detail(id),
+2 -32
View File
@@ -20,6 +20,8 @@ import {
type WorkspaceCredentialType,
} from '@/lib/api/contracts'
import { environmentKeys } from '@/hooks/queries/environment'
import { workspaceCredentialKeys } from '@/hooks/queries/utils/credential-keys'
import { fetchWorkspaceCredentialList } from '@/hooks/queries/utils/fetch-workspace-credentials'
/**
* Key prefix for OAuth credential queries.
@@ -34,38 +36,6 @@ export type {
WorkspaceCredentialType,
}
export const workspaceCredentialKeys = {
all: ['workspaceCredentials'] as const,
lists: () => [...workspaceCredentialKeys.all, 'list'] as const,
list: (workspaceId?: string, type?: string, providerId?: string) =>
[
...workspaceCredentialKeys.lists(),
workspaceId ?? 'none',
type ?? 'all',
providerId ?? 'all',
] as const,
details: () => [...workspaceCredentialKeys.all, 'detail'] as const,
detail: (credentialId?: string) =>
[...workspaceCredentialKeys.details(), credentialId ?? 'none'] as const,
members: (credentialId?: string) =>
[...workspaceCredentialKeys.detail(credentialId), 'members'] as const,
}
/**
* Fetch workspace credential list from API.
* Used by the prefetch function for hover-based cache warming.
*/
export async function fetchWorkspaceCredentialList(
workspaceId: string,
signal?: AbortSignal
): Promise<WorkspaceCredential[]> {
const data = await requestJson(listWorkspaceCredentialsContract, {
query: { workspaceId },
signal,
})
return data.credentials ?? []
}
/**
* Prefetch workspace credentials into a QueryClient cache.
* Use on hover to warm data before navigation.
+1 -1
View File
@@ -11,8 +11,8 @@ import {
resendInvitationContract,
} from '@/lib/api/contracts/invitations'
import { updateWorkspacePermissionsContract } from '@/lib/api/contracts/workspaces'
import { workspaceCredentialKeys } from '@/hooks/queries/credentials'
import { organizationKeys } from '@/hooks/queries/organization'
import { workspaceCredentialKeys } from '@/hooks/queries/utils/credential-keys'
import { workspaceKeys } from '@/hooks/queries/workspace'
export const invitationKeys = {
+1 -1
View File
@@ -42,8 +42,8 @@ import {
import { client } from '@/lib/auth/auth-client'
import { isEnterprise, isPaid, isTeam } from '@/lib/billing/plan-helpers'
import { hasPaidSubscriptionStatus } from '@/lib/billing/subscriptions/utils'
import { workspaceCredentialKeys } from '@/hooks/queries/credentials'
import { subscriptionKeys } from '@/hooks/queries/subscription'
import { workspaceCredentialKeys } from '@/hooks/queries/utils/credential-keys'
import { workspaceKeys } from '@/hooks/queries/workspace'
const logger = createLogger('OrganizationQueries')
@@ -0,0 +1,24 @@
/**
* React Query key factory for workspace credentials.
*
* Lives in this standalone (non-`'use client'`) module — like
* {@link file://./folder-keys.ts} — so server-evaluated code (block
* definitions, server prefetch) can import it without pulling client-reference
* stubs from the `'use client'` `@/hooks/queries/credentials` module.
*/
export const workspaceCredentialKeys = {
all: ['workspaceCredentials'] as const,
lists: () => [...workspaceCredentialKeys.all, 'list'] as const,
list: (workspaceId?: string, type?: string, providerId?: string) =>
[
...workspaceCredentialKeys.lists(),
workspaceId ?? 'none',
type ?? 'all',
providerId ?? 'all',
] as const,
details: () => [...workspaceCredentialKeys.all, 'detail'] as const,
detail: (credentialId?: string) =>
[...workspaceCredentialKeys.details(), credentialId ?? 'none'] as const,
members: (credentialId?: string) =>
[...workspaceCredentialKeys.detail(credentialId), 'members'] as const,
}
@@ -0,0 +1,21 @@
import { requestJson } from '@/lib/api/client/request'
import { type CredentialSet, getCredentialSetContract } from '@/lib/api/contracts'
/**
* Fetches a credential set by id (returns `null` for an empty id).
*
* Lives in this standalone (non-`'use client'`) module so server-reachable
* workflow-comparison helpers can import it without pulling client-reference
* stubs from the `'use client'` `@/hooks/queries/credential-sets` module.
*/
export async function fetchCredentialSetById(
id: string,
signal?: AbortSignal
): Promise<CredentialSet | null> {
if (!id) return null
const data = await requestJson(getCredentialSetContract, {
params: { id },
signal,
})
return data.credentialSet ?? null
}
@@ -0,0 +1,20 @@
import { requestJson } from '@/lib/api/client/request'
import { listWorkspaceCredentialsContract, type WorkspaceCredential } from '@/lib/api/contracts'
/**
* Fetches the workspace credential list.
*
* Lives in this standalone (non-`'use client'`) module so block definitions and
* server prefetch can import it without pulling client-reference stubs from the
* `'use client'` `@/hooks/queries/credentials` module.
*/
export async function fetchWorkspaceCredentialList(
workspaceId: string,
signal?: AbortSignal
): Promise<WorkspaceCredential[]> {
const data = await requestJson(listWorkspaceCredentialsContract, {
query: { workspaceId },
signal,
})
return data.credentials ?? []
}
@@ -36,7 +36,7 @@ vi.mock('@/lib/workflows/subblocks/context', () => ({
buildSelectorContextFromBlock: vi.fn(() => ({})),
}))
vi.mock('@/hooks/queries/credential-sets', () => ({
vi.mock('@/hooks/queries/utils/fetch-credential-set', () => ({
fetchCredentialSetById: vi.fn(),
}))
@@ -4,8 +4,8 @@ import { buildSelectorContextFromBlock } from '@/lib/workflows/subblocks/context
import { getBlock } from '@/blocks/registry'
import { SELECTOR_TYPES_HYDRATION_REQUIRED, type SubBlockConfig } from '@/blocks/types'
import { CREDENTIAL_SET, isUuid } from '@/executor/constants'
import { fetchCredentialSetById } from '@/hooks/queries/credential-sets'
import { fetchOAuthCredentialDetail } from '@/hooks/queries/oauth/oauth-credentials'
import { fetchCredentialSetById } from '@/hooks/queries/utils/fetch-credential-set'
import { getSelectorDefinition, loadAllSelectorOptions } from '@/hooks/selectors/registry'
import { resolveSelectorForSubBlock } from '@/hooks/selectors/resolution'
import type { SelectorContext, SelectorKey } from '@/hooks/selectors/types'
+1
View File
@@ -28,6 +28,7 @@
"check:realtime-prune": "bun run scripts/check-realtime-prune-graph.ts",
"check:zustand-v5": "bun run scripts/check-zustand-v5-selectors.ts",
"check:react-query": "bun run scripts/check-react-query-patterns.ts --check",
"check:client-boundary": "bun run scripts/check-client-boundary-imports.ts --check",
"check:utils": "bun run scripts/check-utils-enforcement.ts",
"check:migrations": "bun run scripts/check-migrations-safety.ts",
"mship-contracts:generate": "bun run scripts/sync-mothership-stream-contract.ts",
+233
View File
@@ -0,0 +1,233 @@
#!/usr/bin/env bun
/**
* Guards against the Next.js `'use client'` server-import foot-gun.
*
* Next.js rewrites EVERY export of a `'use client'` module into a client
* reference in the server bundle. Server-evaluated code can only *render* such
* an export as a component or pass it as a prop — *calling* one throws at
* runtime ("Attempted to call X from the server but X is on the client"). The
* crash for an object export looks like `tableKeys.list is not a function`.
* `next build` does NOT catch this; only SSR/runtime does.
*
* This script flags any **value** import (not `import type`) that resolves to a
* `'use client'` module from a server-evaluated, non-JSX surface — the places
* that never legitimately render a client component and so only ever import a
* client module to (illegally) call its values:
*
* - `apps/sim/app/** /prefetch*.ts` (RSC server prefetch)
* - `apps/sim/app/api/** /route.ts(x)` (route handlers)
* - `apps/sim/triggers/**` (trigger.dev tasks/pollers/webhooks)
* - `apps/sim/blocks/**` (block definitions — evaluated server-side)
*
* Fix: move the imported query-key factory / standalone fetcher / mapper /
* constant into a non-`'use client'` module (e.g. `hooks/queries/utils/*-keys.ts`
* or `hooks/queries/utils/fetch-*.ts`) and import it from there. See the rule in
* `.claude/rules/sim-queries.md`.
*
* Escape hatch: `// client-boundary-allow: <reason>` on the line directly above
* the import (reason required). Use only for a genuinely browser-only code path.
*
* Usage:
* bun run scripts/check-client-boundary-imports.ts # report
* bun run scripts/check-client-boundary-imports.ts --check # CI gate (fail on any)
*/
import { readdir, readFile } from 'node:fs/promises'
import path from 'node:path'
const ROOT = path.resolve(import.meta.dir, '..')
const APP_DIR = path.join(ROOT, 'apps/sim')
/** Server-evaluated, non-JSX surfaces. A file matches if its path passes one. */
function isServerSurface(rel: string): boolean {
if (/(^|\/)prefetch[^/]*\.ts$/.test(rel)) return true
if (/^app\/api\/.+\/route\.tsx?$/.test(rel)) return true
if (/^triggers\//.test(rel)) return true
if (/^blocks\//.test(rel)) return true
return false
}
const SOURCE_EXTENSIONS = ['.ts', '.tsx']
const ALLOW_DIRECTIVE = 'client-boundary-allow'
async function listFiles(dir: string): Promise<string[]> {
const out: string[] = []
let entries: Awaited<ReturnType<typeof readdir>>
try {
entries = await readdir(dir, { withFileTypes: true })
} catch {
return out
}
for (const entry of entries) {
const full = path.join(dir, entry.name)
if (entry.isDirectory()) {
if (entry.name === 'node_modules' || entry.name === '.next') continue
out.push(...(await listFiles(full)))
} else if (SOURCE_EXTENSIONS.includes(path.extname(entry.name))) {
out.push(full)
}
}
return out
}
const useClientCache = new Map<string, boolean>()
async function isUseClientModule(absFile: string): Promise<boolean> {
const cached = useClientCache.get(absFile)
if (cached !== undefined) return cached
let content: string
try {
content = await readFile(absFile, 'utf8')
} catch {
useClientCache.set(absFile, false)
return false
}
// The directive must be the first statement (comments/blank lines may precede it).
let isClient = false
for (const raw of content.split('\n')) {
const line = raw.trim()
if (line === '' || line.startsWith('//') || line.startsWith('/*') || line.startsWith('*')) {
continue
}
isClient = line === "'use client'" || line === '"use client"'
break
}
useClientCache.set(absFile, isClient)
return isClient
}
/** Resolve an import specifier to an absolute source file, or null if external/unresolved. */
async function resolveSpecifier(spec: string, fromFile: string): Promise<string | null> {
let base: string
if (spec.startsWith('@/')) {
base = path.join(APP_DIR, spec.slice(2))
} else if (spec.startsWith('./') || spec.startsWith('../')) {
base = path.resolve(path.dirname(fromFile), spec)
} else {
return null // external package
}
const candidates = [
base,
...SOURCE_EXTENSIONS.map((ext) => base + ext),
...SOURCE_EXTENSIONS.map((ext) => path.join(base, `index${ext}`)),
]
for (const candidate of candidates) {
if (!SOURCE_EXTENSIONS.includes(path.extname(candidate))) continue
try {
await readFile(candidate, 'utf8')
return candidate
} catch {}
}
return null
}
interface ImportInfo {
line: number
specifier: string
clause: string
}
/** Parse `import ... from '...'` statements, skipping side-effect-only imports. */
function parseImports(content: string): ImportInfo[] {
const lines = content.split('\n')
const imports: ImportInfo[] = []
const re = /^\s*import\s+([\s\S]*?)\s+from\s+['"]([^'"]+)['"]/
for (let i = 0; i < lines.length; i++) {
if (!/^\s*import\b/.test(lines[i]) || !lines[i].includes('import')) continue
// Join up to 12 following lines to capture multi-line import clauses.
const block = lines.slice(i, i + 12).join('\n')
const match = re.exec(block)
if (!match) continue
imports.push({ line: i + 1, clause: match[1], specifier: match[2] })
}
return imports
}
/** True when the import brings in at least one runtime VALUE (not purely types). */
function importsAValue(clause: string): boolean {
const trimmed = clause.trim()
if (trimmed.startsWith('type ')) return false // `import type { ... }` / `import type X`
const braceStart = trimmed.indexOf('{')
// A default or namespace binding outside the braces is always a value.
const beforeBrace = braceStart === -1 ? trimmed : trimmed.slice(0, braceStart)
if (beforeBrace.replace(/[,\s]/g, '').length > 0) return true
if (braceStart === -1) return true
const inner = trimmed.slice(braceStart + 1, trimmed.lastIndexOf('}'))
// A named import is a value unless every member is `type`-prefixed.
return inner
.split(',')
.map((s) => s.trim())
.filter(Boolean)
.some((member) => !member.startsWith('type '))
}
function hasAllowDirective(content: string, importLine: number): boolean {
const lines = content.split('\n')
for (let i = importLine - 2; i >= 0 && i >= importLine - 5; i--) {
const line = lines[i]?.trim() ?? ''
if (line === '' || line.startsWith('//') || line.startsWith('*') || line.startsWith('/*')) {
if (line.includes(ALLOW_DIRECTIVE)) {
const reason =
line
.split(ALLOW_DIRECTIVE)[1]
?.replace(/^[:\s]+/, '')
.trim() ?? ''
return reason.length > 0
}
continue
}
break
}
return false
}
interface Violation {
file: string
line: number
specifier: string
}
async function main() {
const checkMode = process.argv.includes('--check')
const allFiles = await listFiles(APP_DIR)
const violations: Violation[] = []
for (const absFile of allFiles) {
const rel = path.relative(APP_DIR, absFile)
if (!isServerSurface(rel)) continue
// A server file that is itself `'use client'` is a client component — out of scope.
if (await isUseClientModule(absFile)) continue
const content = await readFile(absFile, 'utf8')
for (const imp of parseImports(content)) {
if (!importsAValue(imp.clause)) continue
const resolved = await resolveSpecifier(imp.specifier, absFile)
if (!resolved) continue
if (!(await isUseClientModule(resolved))) continue
if (hasAllowDirective(content, imp.line)) continue
violations.push({ file: rel, line: imp.line, specifier: imp.specifier })
}
}
if (violations.length === 0) {
console.log(
"✓ Client-boundary import check passed (no server file imports a value from a 'use client' module)."
)
return
}
console.error(
`\n✗ ${violations.length} server file(s) import a runtime value from a 'use client' module.\n` +
` On the server these resolve to client-reference stubs and throw when called (e.g. 'X.list is not a function').\n` +
` Move the imported factory/fetcher/constant into a non-'use client' module (hooks/queries/utils/*-keys.ts or fetch-*.ts).\n` +
` See .claude/rules/sim-queries.md. Escape hatch: // ${ALLOW_DIRECTIVE}: <reason> above the import.\n`
)
for (const v of violations) {
console.error(` ${v.file}:${v.line} imports from '${v.specifier}'`)
}
if (checkMode) process.exit(1)
}
main().catch((error) => {
console.error(error)
process.exit(1)
})