Skip to content

Commit b586897

Browse files
committed
Merge remote-tracking branch 'origin/staging' into integrate/v2-w5
# Conflicts: # apps/sim/app/api/v2/tables/[tableId]/groups/route.ts
2 parents c36c1b2 + c49751b commit b586897

102 files changed

Lines changed: 1461 additions & 1165 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/tool-registry-boundary/SKILL.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ If it fails, do not add the entry to an allowlist — there isn't one. Find the
6868

6969
Run it with `--verbose` to print per-route module counts, which is also the quickest way to see whether a change moved the graph.
7070

71+
The same command also ratchets those counts against `check-tool-registry-boundary.baseline.json`. `--check` (what CI runs) fails when an entry exceeds its baseline by more than `max(25 modules, 2%)`, naming the import chain responsible. This catches bloat the registry rule misses — a prefetch importing `listTables` cost the Tables page 444 modules without ever touching `@/tools/registry`.
72+
73+
Re-record with `--update-baseline` and commit the JSON when growth is deliberate. A *shrink* passes but is reported — re-record then too, or the win is silently spendable again.
74+
7175
## How to verify an edge actually got cut
7276

7377
Do not eyeball imports — the registry is reached through several redundant paths, so cutting one buys nothing while another survives. Walk the graph:

.claude/commands/tool-registry-boundary.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ If it fails, do not add the entry to an allowlist — there isn't one. Find the
6767

6868
Run it with `--verbose` to print per-route module counts, which is also the quickest way to see whether a change moved the graph.
6969

70+
The same command also ratchets those counts against `check-tool-registry-boundary.baseline.json`. `--check` (what CI runs) fails when an entry exceeds its baseline by more than `max(25 modules, 2%)`, naming the import chain responsible. This catches bloat the registry rule misses — a prefetch importing `listTables` cost the Tables page 444 modules without ever touching `@/tools/registry`.
71+
72+
Re-record with `--update-baseline` and commit the JSON when growth is deliberate. A *shrink* passes but is reported — re-record then too, or the win is silently spendable again.
73+
7074
## How to verify an edge actually got cut
7175

7276
Do not eyeball imports — the registry is reached through several redundant paths, so cutting one buys nothing while another survives. Walk the graph:

.claude/rules/sim-queries.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,22 @@ const handler = useCallback(() => {
143143
}, [data])
144144
```
145145

146+
## Server prefetching
147+
148+
A server prefetch fills the *same* cache key a client hook fills, so it must be indistinguishable from a client fetch. Five rules:
149+
150+
1. **Read the data layer, never our own API over HTTP.** A server-to-server call to `/api/...` costs a round trip and a second authentication for data the process can already read. Where the route runs an application use case, call that same use case with a principal from the same auth policy the route declares — not a manager underneath it.
151+
2. **Match the wire shape the hook caches.** The hook's data is whatever `requestJson(contract, …)` produced, so the seed must equal it. Two traps: a contract field declared `z.coerce.date()` means the hook holds a `Date` where raw route JSON holds a string; a passthrough response schema (`z.custom`) means the hook caches route JSON *verbatim*, so seeding raw rows leaks `Date`s and server-only fields. When the route projects before responding, share that projection — have the route and the prefetch call one function.
152+
3. **Prove the viewer.** Data-layer reads carry no authorization; the route used to provide it. Resolve the viewer (`getWorkspaceHostContextForViewer`, already `cache`d by the layout so it costs nothing) and return early on failure, caching nothing — the client fetch then reaches the route for the real 403. Never widen what a viewer can see.
153+
4. **Always `await`.** Only a settled query is dehydrated, so an unawaited prefetch is silently dropped from the payload and the pane waterfalls anyway.
154+
5. **Don't repeat what the layout already seeded.** `getQueryClient()` builds a new client per server call, so a page re-seeding a layout key is a genuine second read — and `HydrationBoundary` defers an already-seen query to an effect, which SSR never runs, so it never reaches the server render either.
155+
156+
Reuse the hook's exported `staleTime` constant and its key factory; `dehydrate` carries neither options nor `staleTime`, and freshness is per-observer.
157+
158+
Seed with `setQueryData` only when the prefetch must be able to *decline* to create an entry (an empty list that has to fall through to a route's creation path). `prefetchQuery` and `ensureQueryData` always create one.
159+
160+
Keep prefetch imports light. A page prefetch's imports land in that route's server graph, so pulling a barrel to reach one function can drag thousands of modules behind it — `bun run check:tool-registry-boundary` gates this per page.
161+
146162
## Boundary Types
147163

148164
- Hooks import named type aliases from `@/lib/api/contracts/**` (e.g., `import { listEntitiesContract, type EntityList } from '@/lib/api/contracts/entities'`). Never write `z.input<...>` / `z.output<...>` in hooks, and never `import { z } from 'zod'` in client code.

.cursor/commands/tool-registry-boundary.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ If it fails, do not add the entry to an allowlist — there isn't one. Find the
6363

6464
Run it with `--verbose` to print per-route module counts, which is also the quickest way to see whether a change moved the graph.
6565

66+
The same command also ratchets those counts against `check-tool-registry-boundary.baseline.json`. `--check` (what CI runs) fails when an entry exceeds its baseline by more than `max(25 modules, 2%)`, naming the import chain responsible. This catches bloat the registry rule misses — a prefetch importing `listTables` cost the Tables page 444 modules without ever touching `@/tools/registry`.
67+
68+
Re-record with `--update-baseline` and commit the JSON when growth is deliberate. A *shrink* passes but is reported — re-record then too, or the win is silently spendable again.
69+
6670
## How to verify an edge actually got cut
6771

6872
Do not eyeball imports — the registry is reached through several redundant paths, so cutting one buys nothing while another survives. Walk the graph:

apps/sim/app/_shell/providers/get-query-client.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { defaultShouldDehydrateQuery, isServer, QueryClient } from '@tanstack/react-query'
1+
import { isServer, QueryClient } from '@tanstack/react-query'
22
import { isDesktopApp } from '@/lib/desktop'
33

44
export function makeQueryClient() {
55
return new QueryClient({
66
defaultOptions: {
77
queries: {
88
staleTime: 30 * 1000,
9-
gcTime: 5 * 60 * 1000,
109
// The desktop app window lives for days, so cross-session changes —
1110
// an admin upgrading your org/workspace role, a workspace you were
1211
// auto-added to, seat/entitlement changes — would otherwise stay
@@ -18,16 +17,19 @@ export function makeQueryClient() {
1817
// frequent and noisy. Per-query overrides (e.g. useWorkspaceSchedules
1918
// pins this off) always win over this default.
2019
refetchOnWindowFocus: isDesktopApp(),
21-
retry: 1,
20+
/**
21+
* Query core already defaults retries to 0 on the server and 3 in the browser;
22+
* only the browser number is ours to change. Stating one value for both would
23+
* silently opt server prefetches into a retry, and because the layout awaits
24+
* them that spends a retry backoff of document latency on a read whose failure
25+
* the client recovers from on its own.
26+
*/
27+
retry: isServer ? 0 : 1,
2228
retryOnMount: false,
2329
},
2430
mutations: {
2531
retry: false,
2632
},
27-
dehydrate: {
28-
shouldDehydrateQuery: (query) =>
29-
defaultShouldDehydrateQuery(query) || query.state.status === 'pending',
30-
},
3133
},
3234
})
3335
}

apps/sim/app/api/pinned-items/route.ts

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,40 +2,21 @@ import { db, pinnedItem } from '@sim/db'
22
import { createLogger } from '@sim/logger'
33
import { getPostgresErrorCode } from '@sim/utils/errors'
44
import { generateId } from '@sim/utils/id'
5-
import { and, eq, ne } from 'drizzle-orm'
65
import { type NextRequest, NextResponse } from 'next/server'
76
import {
87
createPinnedItemContract,
98
listPinnedItemsContract,
109
type PinnedItemApi,
11-
pinnedResourceTypeSchema,
1210
} from '@/lib/api/contracts'
1311
import { parseRequest } from '@/lib/api/server'
1412
import { getSession } from '@/lib/auth'
1513
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
16-
import { filterToActiveResources, pinnableResourceExists } from '@/lib/pinned-items/resources'
14+
import { listPinnedItemsForUser } from '@/lib/pinned-items/queries'
15+
import { pinnableResourceExists } from '@/lib/pinned-items/resources'
1716
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1817

1918
const logger = createLogger('PinnedItemsAPI')
2019

21-
/**
22-
* Narrows a stored row to the wire shape, dropping any row whose `resourceType` this build does
23-
* not recognise.
24-
*
25-
* `pinned_item.resource_type` is plain `text` — deliberately, so the set of pinnable kinds can
26-
* grow — while the contract is a closed enum. During a rolling deploy an older pod can therefore
27-
* read a pin a newer one wrote. Returning it would fail response validation and take the WHOLE
28-
* list down rather than the single row, so the unknown kind is skipped instead.
29-
*
30-
* `filterToActiveResources` already drops these as a side effect of not having a table to look
31-
* them up in; this makes the guarantee explicit and compiler-checked at the wire boundary.
32-
*/
33-
function toPinnedItemApi(row: typeof pinnedItem.$inferSelect): PinnedItemApi | null {
34-
const resourceType = pinnedResourceTypeSchema.safeParse(row.resourceType)
35-
if (!resourceType.success) return null
36-
return { ...row, resourceType: resourceType.data, pinnedAt: row.pinnedAt.toISOString() }
37-
}
38-
3920
/** Lists the session user's pinned items in a workspace, optionally filtered to one `resourceType`. */
4021
export const GET = withRouteHandler(async (request: NextRequest) => {
4122
const session = await getSession()
@@ -52,30 +33,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5233
return NextResponse.json({ error: 'Access denied to this workspace' }, { status: 403 })
5334
}
5435

55-
const rows = await db
56-
.select()
57-
.from(pinnedItem)
58-
.where(
59-
and(
60-
eq(pinnedItem.userId, session.user.id),
61-
eq(pinnedItem.workspaceId, workspaceId),
62-
/**
63-
* A `workspace` pin stores `workspaceId === resourceId`, so it would otherwise
64-
* appear in this workspace's unscoped listing as a resource *inside* itself.
65-
* It is read from the workspace-list payload instead, so it is excluded here
66-
* rather than left for a future unscoped caller to mistake for a real resource.
67-
*/
68-
resourceType
69-
? eq(pinnedItem.resourceType, resourceType)
70-
: ne(pinnedItem.resourceType, 'workspace')
71-
)
72-
)
73-
74-
const activeRows = await filterToActiveResources(rows, workspaceId)
75-
76-
const pinnedItems = activeRows
77-
.map(toPinnedItemApi)
78-
.filter((item): item is PinnedItemApi => item !== null)
36+
const pinnedItems = await listPinnedItemsForUser(session.user.id, workspaceId, resourceType)
7937

8038
return NextResponse.json({ pinnedItems })
8139
})

apps/sim/app/api/table/[tableId]/columns/route.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,12 @@ vi.mock('@/lib/table/columns/service', () => ({
4949
updateColumnOptions: mockUpdateColumnOptions,
5050
updateColumnType: mockUpdateColumnType,
5151
}))
52+
vi.mock('@/lib/table/wire', () => ({
53+
normalizeColumn: (c: unknown) => c,
54+
}))
5255
vi.mock('@/app/api/table/utils', () => ({
5356
accessError: () => new Response('denied', { status: 403 }),
5457
checkAccess: mockCheckAccess,
55-
normalizeColumn: (c: unknown) => c,
5658
orchestrationOutcomeErrorResponse: (
5759
outcome: { error?: string; errorCode?: OrchestrationErrorCode },
5860
fallback: string

apps/sim/app/api/table/[tableId]/columns/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1313
import { addTableColumn, deleteColumn } from '@/lib/table'
1414
import { signalTableSchemaChanged } from '@/lib/table/events'
1515
import { performUpdateTableColumn } from '@/lib/table/orchestration'
16+
import { normalizeColumn } from '@/lib/table/wire'
1617
import {
1718
accessError,
1819
checkAccess,
19-
normalizeColumn,
2020
orchestrationOutcomeErrorResponse,
2121
rootErrorMessage,
2222
tableLockErrorResponse,

apps/sim/app/api/table/[tableId]/groups/route.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ vi.mock('@/lib/table/application/groups', () => ({
5454
updateTableGroupUseCase: mocks.useCases.update,
5555
}))
5656

57-
vi.mock('@/app/api/table/utils', () => ({
57+
vi.mock('@/lib/table/wire', () => ({
5858
normalizeColumn: vi.fn(),
5959
}))
6060

apps/sim/app/api/table/[tableId]/groups/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
} from '@/lib/table/application/groups'
1313
import { tableOperations } from '@/lib/table/application/operations'
1414
import type { TableDefinition } from '@/lib/table/types'
15-
import { normalizeColumn } from '@/app/api/table/utils'
15+
import { normalizeColumn } from '@/lib/table/wire'
1616

1717
const rateLimit = internalRateLimits.none({
1818
reason: 'Existing authenticated table group mutations have no request-rate policy',

0 commit comments

Comments
 (0)