Skip to content

Commit 3f0dc69

Browse files
committed
more comments
1 parent 08c8d2b commit 3f0dc69

16 files changed

Lines changed: 248 additions & 121 deletions

File tree

apps/sim/AGENTS.md

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ These rules apply to files under `apps/sim/` in addition to the repository root
55
## Architecture
66

77
### Core Principles
8+
89
1. **Single Responsibility**: Each component, hook, store has one clear purpose
910
2. **Composition Over Complexity**: Break down complex logic into smaller pieces
1011
3. **Type Safety First**: TypeScript interfaces for all props, state, return types
@@ -47,6 +48,7 @@ feature/
4748
```
4849

4950
### Naming Conventions
51+
5052
- **Components**: PascalCase (`WorkflowList`)
5153
- **Hooks**: `use` prefix (`useWorkflowOperations`)
5254
- **Files**: kebab-case (`workflow-list.tsx`)
@@ -71,7 +73,7 @@ feature/
7173

7274
## API Contracts
7375

74-
Boundary HTTP request and response shapes for all routes under `apps/sim/app/api/**` live in `apps/sim/lib/api/contracts/**` (one file per resource family). Routes never define route-local boundary Zod schemas, and clients never define ad-hoc wire types — both sides consume the same contract.
76+
Boundary HTTP request and response shapes for all routes under `apps/sim/app/api/`** live in `apps/sim/lib/api/contracts/**` (one file per resource family). Routes never define route-local boundary Zod schemas, and clients never define ad-hoc wire types — both sides consume the same contract.
7577

7678
- Each contract is built with `defineRouteContract({ method, path, params?, query?, body?, headers?, response: { mode: 'json', schema } })` from `@/lib/api/contracts`.
7779
- Contracts export named schemas AND named TypeScript type aliases (e.g., `export type CreateFolderBody = z.input<typeof createFolderBodySchema>`). Clients import the named aliases — never `z.input<...>` / `z.output<...>` in hooks.
@@ -83,10 +85,10 @@ Boundary HTTP request and response shapes for all routes under `apps/sim/app/api
8385

8486
A small number of legitimate exceptions to the boundary rules are tolerated when annotated. The audit script recognizes four annotation forms:
8587

86-
- `// boundary-raw-fetch: <reason>` — placed on the line directly above a raw `fetch(` call inside `apps/sim/hooks/queries/**` or `apps/sim/hooks/selectors/**`. Use only for documented exceptions: streaming responses, binary downloads, multipart uploads, signed-URL flows, OAuth redirects, and external-origin requests.
88+
- `// boundary-raw-fetch: <reason>` — placed on the line directly above a raw `fetch(` call inside `apps/sim/hooks/queries/**`, `apps/sim/hooks/selectors/**`, or any other client/UI source under `apps/sim/**` that targets a same-origin `/api/...` URL. Use only for documented exceptions: streaming responses, binary downloads, multipart uploads, signed-URL flows, OAuth redirects, and external-origin requests.
8789
- `// double-cast-allowed: <reason>` — placed on the line directly above an `as unknown as X` cast outside test files.
88-
- `// boundary-raw-json: <reason>` — placed on the line directly above a raw `await request.json()` / `await req.json()` read in a route handler. Use only when the body is a JSON-RPC envelope, a tolerant `.catch(() => ({}))` parse, or otherwise cannot go through `parseRequest`.
89-
- `// untyped-response: <reason>` — placed on the line directly above a `schema: z.unknown()` response declaration in a contract file. Use only when the response body is genuinely opaque (user-supplied data, third-party passthrough).
90+
- `// boundary-raw-json: <reason>` — placed on the line directly above a raw `await request.json()` / `await req.json()` read (or the multi-line `await request.clone().json()` shim variant) in a route handler. Use only when the body is a JSON-RPC envelope, a tolerant `.catch(() => ({}))` parse, or otherwise cannot go through `parseRequest`.
91+
- `// untyped-response: <reason>` — placed on the line directly above a `schema: z.unknown()` / `schema: z.object({}).passthrough()` / `schema: z.record(z.string(), z.unknown())` response declaration (or a simple alias to one of those) in a contract file. Use only when the response body is genuinely opaque (user-supplied data, third-party passthrough).
9092

9193
Placement rule: the annotation must immediately precede the call or cast. Up to three non-empty preceding comment lines are tolerated, so additional context comments above the annotation are fine. The reason must be non-empty after trimming — annotations with empty reasons fail strict mode (`annotationsMissingReason`).
9294

@@ -104,6 +106,19 @@ const response = await fetch(`/api/copilot/chat/stream?chatId=${chatId}`, { sign
104106
const provider = config as unknown as LegacyProvider
105107
```
106108

109+
```ts
110+
// boundary-raw-json: shim pre-validates the mothership envelope before delegating to the copilot handler that consumes the body
111+
const body = await request
112+
.clone()
113+
.json()
114+
.catch(() => undefined)
115+
```
116+
117+
```ts
118+
// untyped-response: forwards firecrawl /v2/parse response unchanged for downstream tool consumers
119+
output: z.unknown(),
120+
```
121+
107122
## API Route Pattern
108123

109124
Routes never `import { z } from 'zod'` and never define route-local boundary schemas. They consume the contract from `@/lib/api/contracts/**` and validate with canonical helpers from `@/lib/api/server`:
@@ -149,18 +164,18 @@ When adding a new route + client surface, follow this order. Each step has one p
149164

150165
LLMs will write contracts that compile but are sloppy. The human reviewer should optimize attention on:
151166

152-
- **`required` vs `optional` vs `nullable` is correct**. `optional()` allows omission; `nullable()` allows `null`; chaining both creates a tri-state that's almost never what you want.
167+
- `**required` vs `optional` vs `nullable` is correct**. `optional()` allows omission; `nullable()` allows `null`; chaining both creates a tri-state that's almost never what you want.
153168
- **Response schema matches the route's actual JSON output**. The most common drift bug — route emits a field the schema doesn't declare, or omits a required field. Walk every `NextResponse.json(...)` callsite against the schema.
154169
- **Error messages are descriptive**. `'fileName cannot be empty'` beats `'Required'`. Use the second arg of `min(1, '...')`, `nonempty('...')`, etc. For cross-field refines, use `superRefine` with a `path` and a message that names the failing field.
155170
- **Bounds are set** on arrays (`.min(1)`, `.max(N)`), strings (`.min(1).max(N)` for IDs/names), and numbers (`.min().max()` for limits/sizes).
156-
- **`z.unknown()` is a smell** unless the data is genuinely arbitrary (provider passthrough, user-defined tool result, JSON-RPC envelope). When kept, must be annotated `// untyped-response: <specific reason>` in a `schema:` slot.
171+
- `**z.unknown()` is a smell** unless the data is genuinely arbitrary (provider passthrough, user-defined tool result, JSON-RPC envelope). When kept, must be annotated `// untyped-response: <specific reason>` in a `schema:` slot.
157172
- **Discriminated unions over plain unions** when the wire has a discriminant field — gives clients exhaustive narrowing.
158173

159174
CI (`bun run check:api-validation:strict`) catches structural violations (Zod imports in routes, raw `request.json()`, double casts, missing annotations). It does **not** catch these schema-quality judgments — that's the human's job in PR review.
160175

161176
## React Query Client Boundary
162177

163-
Hooks in `apps/sim/hooks/queries/**` consume contracts the same way routes do. Every same-origin JSON call must go through `requestJson(contract, ...)` from `@/lib/api/client/request` instead of raw `fetch`:
178+
Hooks in `apps/sim/hooks/queries/`** consume contracts the same way routes do. Every same-origin JSON call must go through `requestJson(contract, ...)` from `@/lib/api/client/request` instead of raw `fetch`:
164179

165180
- Hooks import named type aliases from `@/lib/api/contracts/**`. Never write `z.input<...>` / `z.output<...>` in hooks, and never `import { z } from 'zod'` in client code.
166181
- `requestJson` parses params, query, body, and headers against the contract on the way out and validates the JSON response on the way back. Hooks always forward `signal` for cancellation.
@@ -204,3 +219,4 @@ export function useEntityList(workspaceId?: string) {
204219
- **Create `utils.ts` when** 2+ files need the same helper
205220
- **Check existing sources** before duplicating (`lib/` has many utilities)
206221
- **Location**: `lib/` (app-wide) → `feature/utils/` (feature-scoped) → inline (single-use)
222+

apps/sim/app/api/mothership/chat/abort/route.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import type { NextRequest } from 'next/server'
22
import { mothershipChatAbortEnvelopeSchema } from '@/lib/api/contracts/mothership-tasks'
33
import { validationErrorResponse } from '@/lib/api/server'
4+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
45
import { POST as copilotAbortPost } from '@/app/api/copilot/chat/abort/route'
56

6-
export async function POST(request: NextRequest) {
7+
export const POST = withRouteHandler(async (request: NextRequest) => {
8+
// boundary-raw-json: shim pre-validates the mothership envelope before delegating to the copilot handler that consumes the body
79
const body = await request
810
.clone()
911
.json()
@@ -14,4 +16,4 @@ export async function POST(request: NextRequest) {
1416
}
1517

1618
return copilotAbortPost(request, undefined)
17-
}
19+
})
Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import type { NextRequest, NextResponse } from 'next/server'
22
import { mothershipChatResourceEnvelopeSchema } from '@/lib/api/contracts/mothership-tasks'
33
import { validationErrorResponse } from '@/lib/api/server'
4+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
45
import {
56
DELETE as copilotResourcesDelete,
67
PATCH as copilotResourcesPatch,
78
POST as copilotResourcesPost,
89
} from '@/app/api/copilot/chat/resources/route'
910

1011
async function validateResourceRequestEnvelope(request: NextRequest): Promise<NextResponse | null> {
12+
// boundary-raw-json: shim pre-validates the mothership envelope before delegating to the copilot handler that consumes the body
1113
const body = await request
1214
.clone()
1315
.json()
@@ -19,23 +21,23 @@ async function validateResourceRequestEnvelope(request: NextRequest): Promise<Ne
1921
return null
2022
}
2123

22-
export async function POST(request: NextRequest) {
24+
export const POST = withRouteHandler(async (request: NextRequest) => {
2325
const validationResponse = await validateResourceRequestEnvelope(request)
2426
if (validationResponse) return validationResponse
2527

2628
return copilotResourcesPost(request, undefined)
27-
}
29+
})
2830

29-
export async function PATCH(request: NextRequest) {
31+
export const PATCH = withRouteHandler(async (request: NextRequest) => {
3032
const validationResponse = await validateResourceRequestEnvelope(request)
3133
if (validationResponse) return validationResponse
3234

3335
return copilotResourcesPatch(request, undefined)
34-
}
36+
})
3537

36-
export async function DELETE(request: NextRequest) {
38+
export const DELETE = withRouteHandler(async (request: NextRequest) => {
3739
const validationResponse = await validateResourceRequestEnvelope(request)
3840
if (validationResponse) return validationResponse
3941

4042
return copilotResourcesDelete(request, undefined)
41-
}
43+
})

apps/sim/app/api/mothership/chat/route.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import type { NextRequest } from 'next/server'
1+
import { type NextRequest, NextResponse } from 'next/server'
22
import {
33
mothershipChatGetQuerySchema,
44
mothershipChatPostEnvelopeSchema,
55
} from '@/lib/api/contracts/mothership-tasks'
66
import { validationErrorResponse } from '@/lib/api/server'
7+
import { getSession } from '@/lib/auth'
78
import { handleUnifiedChatPost, maxDuration } from '@/lib/copilot/chat/post'
89
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
910
import { GET as copilotChatGet } from '@/app/api/copilot/chat/queries'
@@ -21,6 +22,12 @@ export const GET = withRouteHandler((request: NextRequest) => {
2122
})
2223

2324
export const POST = withRouteHandler(async (request: NextRequest) => {
25+
const session = await getSession()
26+
if (!session?.user?.id) {
27+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
28+
}
29+
30+
// boundary-raw-json: shim pre-validates the mothership envelope before delegating to the copilot handler that consumes the body
2431
const body = await request
2532
.clone()
2633
.json()
Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import type { NextRequest } from 'next/server'
22
import { mothershipChatStopEnvelopeSchema } from '@/lib/api/contracts/mothership-tasks'
33
import { validationErrorResponse } from '@/lib/api/server'
4+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
45
import { POST as copilotStopPost } from '@/app/api/copilot/chat/stop/route'
56

6-
// Unified stop route surface.
7-
export async function POST(request: NextRequest) {
7+
export const POST = withRouteHandler(async (request: NextRequest) => {
8+
// boundary-raw-json: shim pre-validates the mothership envelope before delegating to the copilot handler that consumes the body
89
const body = await request
910
.clone()
1011
.json()
@@ -15,4 +16,4 @@ export async function POST(request: NextRequest) {
1516
}
1617

1718
return copilotStopPost(request, undefined)
18-
}
19+
})

apps/sim/app/api/workflows/[id]/route.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,22 @@ export const PUT = withRouteHandler(
347347
}
348348
}
349349

350-
// Update the workflow
351350
const [updatedWorkflow] = await db
352351
.update(workflow)
353352
.set(updateData)
354353
.where(eq(workflow.id, workflowId))
355-
.returning()
354+
.returning({
355+
id: workflow.id,
356+
name: workflow.name,
357+
description: workflow.description,
358+
color: workflow.color,
359+
workspaceId: workflow.workspaceId,
360+
folderId: workflow.folderId,
361+
sortOrder: workflow.sortOrder,
362+
createdAt: workflow.createdAt,
363+
updatedAt: workflow.updatedAt,
364+
archivedAt: workflow.archivedAt,
365+
})
356366

357367
const elapsed = Date.now() - startTime
358368
logger.info(`[${requestId}] Successfully updated workflow ${workflowId} in ${elapsed}ms`, {

apps/sim/app/api/workflows/route.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1313
import { captureServerEvent } from '@/lib/posthog/server'
1414
import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults'
1515
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
16-
import { deduplicateWorkflowName, listWorkflows } from '@/lib/workflows/utils'
16+
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
1717
import { getUserEntityPermissions, workspaceExists } from '@/lib/workspaces/permissions/utils'
1818
import { verifyWorkspaceMembership } from '@/app/api/workflows/utils'
1919

@@ -69,10 +69,39 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
6969

7070
let workflows
7171

72+
/**
73+
* Project only the columns declared in `workflowListItemSchema` so the
74+
* wire response matches the contract shape exactly. The full row is
75+
* larger (`state`, `variables`, `apiKey`, `runCount`, etc.) and would
76+
* be dropped client-side by Zod parse anyway — narrowing here saves
77+
* bytes over the wire. Keep this list aligned with the contract.
78+
*/
79+
const listColumns = {
80+
id: workflow.id,
81+
name: workflow.name,
82+
description: workflow.description,
83+
color: workflow.color,
84+
workspaceId: workflow.workspaceId,
85+
folderId: workflow.folderId,
86+
sortOrder: workflow.sortOrder,
87+
createdAt: workflow.createdAt,
88+
updatedAt: workflow.updatedAt,
89+
archivedAt: workflow.archivedAt,
90+
} as const
7291
const orderByClause = [asc(workflow.sortOrder), asc(workflow.createdAt), asc(workflow.id)]
7392

7493
if (workspaceId) {
75-
workflows = await listWorkflows(workspaceId, { scope })
94+
workflows = await db
95+
.select(listColumns)
96+
.from(workflow)
97+
.where(
98+
scope === 'all'
99+
? eq(workflow.workspaceId, workspaceId)
100+
: scope === 'archived'
101+
? and(eq(workflow.workspaceId, workspaceId), sql`${workflow.archivedAt} IS NOT NULL`)
102+
: and(eq(workflow.workspaceId, workspaceId), isNull(workflow.archivedAt))
103+
)
104+
.orderBy(...orderByClause)
76105
} else {
77106
const workspacePermissionRows = await db
78107
.select({ workspaceId: permissions.entityId })
@@ -83,7 +112,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
83112
return NextResponse.json({ data: [] }, { status: 200 })
84113
}
85114
workflows = await db
86-
.select()
115+
.select(listColumns)
87116
.from(workflow)
88117
.where(
89118
scope === 'all'

apps/sim/app/workspace/page.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,7 @@ async function handleWorkflowRedirect(
8888
const workflowData = await requestJson(getWorkflowStateContract, {
8989
params: { id: workflowId },
9090
})
91-
const data = workflowData.data as Record<string, unknown> | undefined
92-
const workspaceId = typeof data?.workspaceId === 'string' ? data.workspaceId : undefined
91+
const workspaceId = workflowData.data.workspaceId
9392
if (workspaceId) {
9493
logger.info(`Redirecting workflow ${workflowId} to workspace ${workspaceId}`)
9594
router.replace(`/workspace/${workspaceId}/w/${workflowId}`)

apps/sim/lib/api/contracts/logs.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { z } from 'zod'
2+
import { booleanQueryFlagSchema } from '@/lib/api/contracts/primitives'
23
import { defineRouteContract } from '@/lib/api/contracts/types'
34

45
const comparisonOperatorSchema = z.enum(['=', '>', '<', '>=', '<=', '!='])
@@ -131,8 +132,8 @@ export const v1ListLogsQuerySchema = z.object({
131132
maxCost: z.coerce.number().optional(),
132133
model: z.string().optional(),
133134
details: z.enum(['basic', 'full']).optional().default('basic'),
134-
includeTraceSpans: z.coerce.boolean().optional().default(false),
135-
includeFinalOutput: z.coerce.boolean().optional().default(false),
135+
includeTraceSpans: booleanQueryFlagSchema.optional().default(false),
136+
includeFinalOutput: booleanQueryFlagSchema.optional().default(false),
136137
limit: z.coerce.number().optional().default(100),
137138
cursor: z.string().optional(),
138139
order: z.enum(['desc', 'asc']).optional().default('desc'),

apps/sim/lib/api/contracts/primitives.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,31 @@ export const workspaceIdSchema = z.string().min(1, 'Workspace ID is required')
4242
* stable, human-readable message.
4343
*/
4444
export const workflowIdSchema = z.string().min(1, 'Workflow ID is required')
45+
46+
/**
47+
* Boolean query-string primitive that correctly handles the literal strings
48+
* `"true"` / `"false"` (case-insensitive) in addition to real booleans.
49+
*
50+
* Do NOT use `z.coerce.boolean()` for query parameters: it coerces any
51+
* non-empty string to `true`, so `?flag=false` resolves to `true`. This
52+
* primitive treats `"false"` / `"0"` / `""` as `false` and `"true"` / `"1"`
53+
* as `true`, mirroring how query strings are commonly serialized by
54+
* frontends and CLIs.
55+
*
56+
* Real boolean inputs (e.g. when `requestJson` serializes a JS `true`) pass
57+
* through unchanged. Anything else fails validation with a clear message.
58+
*
59+
* Use `.optional()` / `.default(...)` at the call site, not here, so each
60+
* query field controls its own omission/default semantics.
61+
*/
62+
export const booleanQueryFlagSchema = z.preprocess(
63+
(value) => {
64+
if (typeof value === 'boolean') return value
65+
if (typeof value !== 'string') return value
66+
const normalized = value.trim().toLowerCase()
67+
if (normalized === 'true' || normalized === '1') return true
68+
if (normalized === 'false' || normalized === '0' || normalized === '') return false
69+
return value
70+
},
71+
z.boolean({ error: 'must be a boolean (true/false)' })
72+
)

0 commit comments

Comments
 (0)