diff --git a/.agents/skills/v2-api-conventions/SKILL.md b/.agents/skills/v2-api-conventions/SKILL.md new file mode 100644 index 00000000000..52ab14450ad --- /dev/null +++ b/.agents/skills/v2-api-conventions/SKILL.md @@ -0,0 +1,134 @@ +--- +name: v2-api-conventions +description: The response, error, pagination, and validation contract every `/api/v2` endpoint must satisfy. Use when adding or changing a route under `apps/sim/app/api/v2/`, or when auditing one for conformance. +argument-hint: +--- + +# v2 API Conventions + +The v2 surface makes one promise: **every response is the same two shapes, and a caller-supplied value can never produce a 500.** + +``` +success (single) { "data": {...} } +success (collection) { "data": [...], "nextCursor": "..." | null } +failure (always) { "error": { "code": "...", "message": "...", "details"?: ... } } +``` + +Nothing else at the top level. No `success: true`, no bare `{ "error": "string" }`, no HTML. + +That promise is worth stating as a rule because it has been broken four separate ways, each time by a route or a builder taking a shortcut that looked local: + +- `GET /workflows?limit=1.5` returned **500**. The contract was copied from a sibling and lost its `.int()`, so a fractional limit passed validation and reached Postgres as `LIMIT 2.5`. +- A malformed JSON body returned **`{"error":"Request body must be valid JSON"}`** — a bare string. The envelope was a per-route opt-in that only 8 of 77 routes remembered. +- `GET /api/v2/nonexistent` returned a **full HTML 404 document**, because no route file matched and the request fell through to the app's global not-found page. +- Four collections returned `nextCursor` while **silently discarding** any `limit` the caller sent, because Zod strips unknown keys unless the schema is `.strict()`. + +Each was one line. The rules below are the generalisations. + +## Where the machinery lives + +| Concern | File | +|---|---| +| Envelope + error codes + cursor codecs | `apps/sim/app/api/v2/lib/response.ts` | +| Rollout gate (`v2-api` flag) | `apps/sim/app/api/v2/lib/gate.ts` | +| Cross-tenant concealment | `apps/sim/lib/api/server/routes/resource-concealment.ts` | +| Route builder | `apps/sim/lib/api/server/routes/v2-json-route.ts` | +| Contracts | `apps/sim/lib/api/contracts/v2/**` | +| Shared list/keyset helpers | `apps/sim/lib/api/list-query.ts` | + +## Rule 1 — the envelope is produced by helpers, never by hand + +`v2Data`, `v2CursorList`, and `v2Error` in `response.ts` are the only things that build a v2 body. They also set `Cache-Control: private, no-store`, which every v2 response needs because every v2 response is authed per-caller data. + +A route built with `defineV2JsonRoute` gets this for free: its `present` returns the *body shape* and the builder renders it. Never call `NextResponse.json` from a v2 route. + +**The envelope must hold for every failure mode, including the ones that happen before your handler runs.** That is what the four bugs above have in common. Defaults for the transport-level failures live on the builder — `v2PayloadTooLargeResponse` (413) and `v2InvalidJsonResponse` (400) — precisely so a route cannot forget them. + +## Rule 2 — status codes mean specific things + +| Status | `code` | Meaning | +|---|---|---| +| 200 / 201 | — | Success. 201 only for a created resource. | +| 400 | `BAD_REQUEST` | Contract validation. Carries field-level `details` from `serializeZodIssues`. | +| 401 | `UNAUTHORIZED` | No/!valid API key. **Runs before the rollout gate.** | +| 403 | `FORBIDDEN` | Authenticated, same tenant, insufficient rights. Carry a machine-readable `details.code` (e.g. `WORKFLOW_NOT_DEPLOYED`). | +| 404 | `NOT_FOUND` | Not found, **and** cross-tenant concealment, **and** the rollout gate, **and** an unknown path. | +| 409 | `CONFLICT` | Uniqueness/state conflict, human-readable message. | +| 413 | `PAYLOAD_TOO_LARGE` | Body over the route's `maxBodyBytes`. | +| 429 | `RATE_LIMITED` | With `Retry-After` and `X-RateLimit-*`. | +| 500 | `INTERNAL_ERROR` | Genuine server fault only. Message is always generic. | + +Two of these carry real design weight: + +**404 is deliberately overloaded.** A workspace the caller cannot reach answers `404 "Workspace not found"`, never 403 — a 403 would confirm the resource exists. `createV2ResourceConcealmentPolicy` does this by mapping a cross-tenant authorization failure to `v2Error('NOT_FOUND', ...)`. The rollout gate answers the same way for the same reason (`gate.ts`: "an ungated caller cannot distinguish 'not in the rollout cohort' from 'no such endpoint'"), and so does the unknown-path catch-all at `app/api/v2/[[...segments]]/route.ts` — its body is byte-identical to the gate's on purpose. + +**500 is never caller-reachable.** Any input a caller can send must be rejected at the contract boundary with a 400. If you can construct a query string or body that produces a 500, that is a bug in the contract, not something to wrap in a `try`/`catch`. `v2ErrorForOrchestration` also replaces the message on an unclassified failure with a generic one, so internal detail never leaks. + +## Rule 3 — a collection that returns `nextCursor` must accept `limit` + `cursor`, and must apply them + +Every list returns `{ data, nextCursor }`. Whether it *pages* is a separate, pinned decision — see `lib/api/contracts/v2/__tests__/list-pagination.test.ts`, which enumerates both sets and fails when a new list is in neither. + +Build the query slice from the shared helper, never by hand: + +```ts +...v2PaginationFields({ description: 'Maximum widgets to return per page.' }) +``` + +That gives `limit` (integer, 1..`V2_MAX_PAGE_SIZE`, defaulting to `V2_DEFAULT_PAGE_SIZE` = 50) and an opaque `cursor`. Re-declaring `limit: z.coerce.number()...` inline is how the 500 happened; there is one schema so the family cannot drift again. + +Two cursor schemes exist, both opaque base64-JSON from `response.ts`. Which one you use is decided by what the read can express, not by taste: + +- **Keyset** (`readSortedCursor` in, `encodeSortedCursor` out) — the default. Requires the page to come from one ordered SQL read. The sort is stamped into the cursor and re-checked on replay, so changing `sortBy` mid-pagination is a 400, not a silently skipped page. +- **Offset** (`decodeOffsetCursor` / `encodeOffsetCursor`) — only when a keyset is impossible. Two lists qualify: `GET /skills` merges a static in-code registry with DB rows and re-sorts in JS, and `GET /knowledge/{id}/documents` sits on a limit/offset query. An offset cursor **must** be stamped with `offsetCursorScope(...)` covering every param that filters or orders the sequence (not `limit`, which only selects how much of it to return). A bare offset replayed against a re-sorted or re-filtered sequence names a different row, which skips or repeats results — the exact failure the keyset's sort stamp already prevents. + +**A keyset's key list must end in a unique column (`id`).** A non-unique trailing key cannot separate tied rows, so the page boundary either repeats or drops them. `lib/api/list-keyset-paging.test.ts` demonstrates the failure. + +Return `nextCursor: null` on the last page and only then. Never construct a cursor client-side. + +## Rule 4 — reject what you do not implement + +Query and body schemas are **`.strict()`**. Zod strips unknown keys by default, so a non-strict schema answers `?limit=1` with 200 and the whole set — the caller believes it bounded the response and it did not. That is a contract lie, and on an uncapped list it is also an unbounded-response risk. + +Error messages name the field and, where there is one, the escape hatch: + +``` +limit must be a whole number +limit cannot exceed 100 +search cannot be empty +sortBy: expected one of "name" | "createdAt" | "updatedAt" +Limit cannot exceed 1000; use limit=0 to stream all rows, or create an export +``` + +That last one is the standard to aim for. A message that only says `Invalid input` fails this rule — the caller cannot act on it. + +## Rule 5 — contract first, then use case, then route + +Order matters because each layer is checked against the one before it. + +1. **Contract** in `lib/api/contracts/v2/.ts` via `defineRouteContract`. Response schemas are `.parse`d on the way out, so a field the producer does not actually emit becomes a 500 on a successful read — assert only what you can prove. +2. **Application use case** owns canonical loading, authorization, business behavior, and audit. The route's `present` receives **only the use-case result**, so anything the presenter needs (e.g. the active `sortBy`/`sortOrder` to stamp a cursor) must be returned by the use case. +3. **Route** with `defineV2JsonRoute`, declaring `contract`, `auth: v2ApiKeyAuth`, `operation`, `rateLimit`, `errorPolicy`, `mapInput`, `useCase`, `present`. Auth and rate limiting run before parsing. +4. **OpenAPI description** in `lib/api/contracts/v2/openapi/.ts`, then `bun run generate:openapi`. A description that claims behaviour the route does not have is the same class of bug as a wrong schema. + +## Checklist + +Run this against any new or changed v2 endpoint. + +- [ ] Success body is exactly `{data}` or `{data, nextCursor}`; failures are exactly `{error:{code,message,details?}}`. +- [ ] Route uses a shared builder; no hand-built `NextResponse.json`. +- [ ] Query and body schemas are `.strict()`. +- [ ] No caller-supplied value can produce a 500 — check every numeric param reaches SQL as a validated integer. +- [ ] `limit` comes from `v2PaginationFields`, not a hand-written `z.coerce.number()`. +- [ ] If the response carries `nextCursor`, the query accepts `limit` + `cursor` and the query actually applies them. +- [ ] Keyset sorts end in a unique `id` key. +- [ ] The list is classified in `list-pagination.test.ts`. +- [ ] Cross-tenant access answers 404, never 403. +- [ ] 403s carry a machine-readable `details.code`. +- [ ] Validation messages name the field and echo the valid set. +- [ ] Response schema matches every field the route actually emits. +- [ ] OpenAPI description regenerated and truthful about pagination. +- [ ] `bun run type-check`, `bun run check:api-validation`, `bun run check:openapi` pass. + +## Known gap + +A 405 on a path that *does* have a route file but does not export that verb is generated by Next.js before any Sim code runs: zero-byte body, no `content-type`, and no `Allow` header, which RFC 9110 §15.5.6 requires. Fixing it means either exporting explicit rejecting handlers from all 77 v2 route files or intercepting in `apps/sim/proxy.ts` with a static path→methods table. Neither is done. Unknown *paths* are handled — the catch-all covers those. diff --git a/.claude/commands/v2-api-conventions.md b/.claude/commands/v2-api-conventions.md new file mode 100644 index 00000000000..c7740489850 --- /dev/null +++ b/.claude/commands/v2-api-conventions.md @@ -0,0 +1,133 @@ +--- +description: The response, error, pagination, and validation contract every `/api/v2` endpoint must satisfy. Use when adding or changing a route under `apps/sim/app/api/v2/`, or when auditing one for conformance. +argument-hint: +--- + +# v2 API Conventions + +The v2 surface makes one promise: **every response is the same two shapes, and a caller-supplied value can never produce a 500.** + +``` +success (single) { "data": {...} } +success (collection) { "data": [...], "nextCursor": "..." | null } +failure (always) { "error": { "code": "...", "message": "...", "details"?: ... } } +``` + +Nothing else at the top level. No `success: true`, no bare `{ "error": "string" }`, no HTML. + +That promise is worth stating as a rule because it has been broken four separate ways, each time by a route or a builder taking a shortcut that looked local: + +- `GET /workflows?limit=1.5` returned **500**. The contract was copied from a sibling and lost its `.int()`, so a fractional limit passed validation and reached Postgres as `LIMIT 2.5`. +- A malformed JSON body returned **`{"error":"Request body must be valid JSON"}`** — a bare string. The envelope was a per-route opt-in that only 8 of 77 routes remembered. +- `GET /api/v2/nonexistent` returned a **full HTML 404 document**, because no route file matched and the request fell through to the app's global not-found page. +- Four collections returned `nextCursor` while **silently discarding** any `limit` the caller sent, because Zod strips unknown keys unless the schema is `.strict()`. + +Each was one line. The rules below are the generalisations. + +## Where the machinery lives + +| Concern | File | +|---|---| +| Envelope + error codes + cursor codecs | `apps/sim/app/api/v2/lib/response.ts` | +| Rollout gate (`v2-api` flag) | `apps/sim/app/api/v2/lib/gate.ts` | +| Cross-tenant concealment | `apps/sim/lib/api/server/routes/resource-concealment.ts` | +| Route builder | `apps/sim/lib/api/server/routes/v2-json-route.ts` | +| Contracts | `apps/sim/lib/api/contracts/v2/**` | +| Shared list/keyset helpers | `apps/sim/lib/api/list-query.ts` | + +## Rule 1 — the envelope is produced by helpers, never by hand + +`v2Data`, `v2CursorList`, and `v2Error` in `response.ts` are the only things that build a v2 body. They also set `Cache-Control: private, no-store`, which every v2 response needs because every v2 response is authed per-caller data. + +A route built with `defineV2JsonRoute` gets this for free: its `present` returns the *body shape* and the builder renders it. Never call `NextResponse.json` from a v2 route. + +**The envelope must hold for every failure mode, including the ones that happen before your handler runs.** That is what the four bugs above have in common. Defaults for the transport-level failures live on the builder — `v2PayloadTooLargeResponse` (413) and `v2InvalidJsonResponse` (400) — precisely so a route cannot forget them. + +## Rule 2 — status codes mean specific things + +| Status | `code` | Meaning | +|---|---|---| +| 200 / 201 | — | Success. 201 only for a created resource. | +| 400 | `BAD_REQUEST` | Contract validation. Carries field-level `details` from `serializeZodIssues`. | +| 401 | `UNAUTHORIZED` | No/!valid API key. **Runs before the rollout gate.** | +| 403 | `FORBIDDEN` | Authenticated, same tenant, insufficient rights. Carry a machine-readable `details.code` (e.g. `WORKFLOW_NOT_DEPLOYED`). | +| 404 | `NOT_FOUND` | Not found, **and** cross-tenant concealment, **and** the rollout gate, **and** an unknown path. | +| 409 | `CONFLICT` | Uniqueness/state conflict, human-readable message. | +| 413 | `PAYLOAD_TOO_LARGE` | Body over the route's `maxBodyBytes`. | +| 429 | `RATE_LIMITED` | With `Retry-After` and `X-RateLimit-*`. | +| 500 | `INTERNAL_ERROR` | Genuine server fault only. Message is always generic. | + +Two of these carry real design weight: + +**404 is deliberately overloaded.** A workspace the caller cannot reach answers `404 "Workspace not found"`, never 403 — a 403 would confirm the resource exists. `createV2ResourceConcealmentPolicy` does this by mapping a cross-tenant authorization failure to `v2Error('NOT_FOUND', ...)`. The rollout gate answers the same way for the same reason (`gate.ts`: "an ungated caller cannot distinguish 'not in the rollout cohort' from 'no such endpoint'"), and so does the unknown-path catch-all at `app/api/v2/[[...segments]]/route.ts` — its body is byte-identical to the gate's on purpose. + +**500 is never caller-reachable.** Any input a caller can send must be rejected at the contract boundary with a 400. If you can construct a query string or body that produces a 500, that is a bug in the contract, not something to wrap in a `try`/`catch`. `v2ErrorForOrchestration` also replaces the message on an unclassified failure with a generic one, so internal detail never leaks. + +## Rule 3 — a collection that returns `nextCursor` must accept `limit` + `cursor`, and must apply them + +Every list returns `{ data, nextCursor }`. Whether it *pages* is a separate, pinned decision — see `lib/api/contracts/v2/__tests__/list-pagination.test.ts`, which enumerates both sets and fails when a new list is in neither. + +Build the query slice from the shared helper, never by hand: + +```ts +...v2PaginationFields({ description: 'Maximum widgets to return per page.' }) +``` + +That gives `limit` (integer, 1..`V2_MAX_PAGE_SIZE`, defaulting to `V2_DEFAULT_PAGE_SIZE` = 50) and an opaque `cursor`. Re-declaring `limit: z.coerce.number()...` inline is how the 500 happened; there is one schema so the family cannot drift again. + +Two cursor schemes exist, both opaque base64-JSON from `response.ts`. Which one you use is decided by what the read can express, not by taste: + +- **Keyset** (`readSortedCursor` in, `encodeSortedCursor` out) — the default. Requires the page to come from one ordered SQL read. The sort is stamped into the cursor and re-checked on replay, so changing `sortBy` mid-pagination is a 400, not a silently skipped page. +- **Offset** (`decodeOffsetCursor` / `encodeOffsetCursor`) — only when a keyset is impossible. Two lists qualify: `GET /skills` merges a static in-code registry with DB rows and re-sorts in JS, and `GET /knowledge/{id}/documents` sits on a limit/offset query. An offset cursor **must** be stamped with `offsetCursorScope(...)` covering every param that filters or orders the sequence (not `limit`, which only selects how much of it to return). A bare offset replayed against a re-sorted or re-filtered sequence names a different row, which skips or repeats results — the exact failure the keyset's sort stamp already prevents. + +**A keyset's key list must end in a unique column (`id`).** A non-unique trailing key cannot separate tied rows, so the page boundary either repeats or drops them. `lib/api/list-keyset-paging.test.ts` demonstrates the failure. + +Return `nextCursor: null` on the last page and only then. Never construct a cursor client-side. + +## Rule 4 — reject what you do not implement + +Query and body schemas are **`.strict()`**. Zod strips unknown keys by default, so a non-strict schema answers `?limit=1` with 200 and the whole set — the caller believes it bounded the response and it did not. That is a contract lie, and on an uncapped list it is also an unbounded-response risk. + +Error messages name the field and, where there is one, the escape hatch: + +``` +limit must be a whole number +limit cannot exceed 100 +search cannot be empty +sortBy: expected one of "name" | "createdAt" | "updatedAt" +Limit cannot exceed 1000; use limit=0 to stream all rows, or create an export +``` + +That last one is the standard to aim for. A message that only says `Invalid input` fails this rule — the caller cannot act on it. + +## Rule 5 — contract first, then use case, then route + +Order matters because each layer is checked against the one before it. + +1. **Contract** in `lib/api/contracts/v2/.ts` via `defineRouteContract`. Response schemas are `.parse`d on the way out, so a field the producer does not actually emit becomes a 500 on a successful read — assert only what you can prove. +2. **Application use case** owns canonical loading, authorization, business behavior, and audit. The route's `present` receives **only the use-case result**, so anything the presenter needs (e.g. the active `sortBy`/`sortOrder` to stamp a cursor) must be returned by the use case. +3. **Route** with `defineV2JsonRoute`, declaring `contract`, `auth: v2ApiKeyAuth`, `operation`, `rateLimit`, `errorPolicy`, `mapInput`, `useCase`, `present`. Auth and rate limiting run before parsing. +4. **OpenAPI description** in `lib/api/contracts/v2/openapi/.ts`, then `bun run generate:openapi`. A description that claims behaviour the route does not have is the same class of bug as a wrong schema. + +## Checklist + +Run this against any new or changed v2 endpoint. + +- [ ] Success body is exactly `{data}` or `{data, nextCursor}`; failures are exactly `{error:{code,message,details?}}`. +- [ ] Route uses a shared builder; no hand-built `NextResponse.json`. +- [ ] Query and body schemas are `.strict()`. +- [ ] No caller-supplied value can produce a 500 — check every numeric param reaches SQL as a validated integer. +- [ ] `limit` comes from `v2PaginationFields`, not a hand-written `z.coerce.number()`. +- [ ] If the response carries `nextCursor`, the query accepts `limit` + `cursor` and the query actually applies them. +- [ ] Keyset sorts end in a unique `id` key. +- [ ] The list is classified in `list-pagination.test.ts`. +- [ ] Cross-tenant access answers 404, never 403. +- [ ] 403s carry a machine-readable `details.code`. +- [ ] Validation messages name the field and echo the valid set. +- [ ] Response schema matches every field the route actually emits. +- [ ] OpenAPI description regenerated and truthful about pagination. +- [ ] `bun run type-check`, `bun run check:api-validation`, `bun run check:openapi` pass. + +## Known gap + +A 405 on a path that *does* have a route file but does not export that verb is generated by Next.js before any Sim code runs: zero-byte body, no `content-type`, and no `Allow` header, which RFC 9110 §15.5.6 requires. Fixing it means either exporting explicit rejecting handlers from all 77 v2 route files or intercepting in `apps/sim/proxy.ts` with a static path→methods table. Neither is done. Unknown *paths* are handled — the catch-all covers those. diff --git a/.cursor/commands/v2-api-conventions.md b/.cursor/commands/v2-api-conventions.md new file mode 100644 index 00000000000..a19ab57e6aa --- /dev/null +++ b/.cursor/commands/v2-api-conventions.md @@ -0,0 +1,128 @@ +# v2 API Conventions + +The v2 surface makes one promise: **every response is the same two shapes, and a caller-supplied value can never produce a 500.** + +``` +success (single) { "data": {...} } +success (collection) { "data": [...], "nextCursor": "..." | null } +failure (always) { "error": { "code": "...", "message": "...", "details"?: ... } } +``` + +Nothing else at the top level. No `success: true`, no bare `{ "error": "string" }`, no HTML. + +That promise is worth stating as a rule because it has been broken four separate ways, each time by a route or a builder taking a shortcut that looked local: + +- `GET /workflows?limit=1.5` returned **500**. The contract was copied from a sibling and lost its `.int()`, so a fractional limit passed validation and reached Postgres as `LIMIT 2.5`. +- A malformed JSON body returned **`{"error":"Request body must be valid JSON"}`** — a bare string. The envelope was a per-route opt-in that only 8 of 77 routes remembered. +- `GET /api/v2/nonexistent` returned a **full HTML 404 document**, because no route file matched and the request fell through to the app's global not-found page. +- Four collections returned `nextCursor` while **silently discarding** any `limit` the caller sent, because Zod strips unknown keys unless the schema is `.strict()`. + +Each was one line. The rules below are the generalisations. + +## Where the machinery lives + +| Concern | File | +|---|---| +| Envelope + error codes + cursor codecs | `apps/sim/app/api/v2/lib/response.ts` | +| Rollout gate (`v2-api` flag) | `apps/sim/app/api/v2/lib/gate.ts` | +| Cross-tenant concealment | `apps/sim/lib/api/server/routes/resource-concealment.ts` | +| Route builder | `apps/sim/lib/api/server/routes/v2-json-route.ts` | +| Contracts | `apps/sim/lib/api/contracts/v2/**` | +| Shared list/keyset helpers | `apps/sim/lib/api/list-query.ts` | + +## Rule 1 — the envelope is produced by helpers, never by hand + +`v2Data`, `v2CursorList`, and `v2Error` in `response.ts` are the only things that build a v2 body. They also set `Cache-Control: private, no-store`, which every v2 response needs because every v2 response is authed per-caller data. + +A route built with `defineV2JsonRoute` gets this for free: its `present` returns the *body shape* and the builder renders it. Never call `NextResponse.json` from a v2 route. + +**The envelope must hold for every failure mode, including the ones that happen before your handler runs.** That is what the four bugs above have in common. Defaults for the transport-level failures live on the builder — `v2PayloadTooLargeResponse` (413) and `v2InvalidJsonResponse` (400) — precisely so a route cannot forget them. + +## Rule 2 — status codes mean specific things + +| Status | `code` | Meaning | +|---|---|---| +| 200 / 201 | — | Success. 201 only for a created resource. | +| 400 | `BAD_REQUEST` | Contract validation. Carries field-level `details` from `serializeZodIssues`. | +| 401 | `UNAUTHORIZED` | No/!valid API key. **Runs before the rollout gate.** | +| 403 | `FORBIDDEN` | Authenticated, same tenant, insufficient rights. Carry a machine-readable `details.code` (e.g. `WORKFLOW_NOT_DEPLOYED`). | +| 404 | `NOT_FOUND` | Not found, **and** cross-tenant concealment, **and** the rollout gate, **and** an unknown path. | +| 409 | `CONFLICT` | Uniqueness/state conflict, human-readable message. | +| 413 | `PAYLOAD_TOO_LARGE` | Body over the route's `maxBodyBytes`. | +| 429 | `RATE_LIMITED` | With `Retry-After` and `X-RateLimit-*`. | +| 500 | `INTERNAL_ERROR` | Genuine server fault only. Message is always generic. | + +Two of these carry real design weight: + +**404 is deliberately overloaded.** A workspace the caller cannot reach answers `404 "Workspace not found"`, never 403 — a 403 would confirm the resource exists. `createV2ResourceConcealmentPolicy` does this by mapping a cross-tenant authorization failure to `v2Error('NOT_FOUND', ...)`. The rollout gate answers the same way for the same reason (`gate.ts`: "an ungated caller cannot distinguish 'not in the rollout cohort' from 'no such endpoint'"), and so does the unknown-path catch-all at `app/api/v2/[[...segments]]/route.ts` — its body is byte-identical to the gate's on purpose. + +**500 is never caller-reachable.** Any input a caller can send must be rejected at the contract boundary with a 400. If you can construct a query string or body that produces a 500, that is a bug in the contract, not something to wrap in a `try`/`catch`. `v2ErrorForOrchestration` also replaces the message on an unclassified failure with a generic one, so internal detail never leaks. + +## Rule 3 — a collection that returns `nextCursor` must accept `limit` + `cursor`, and must apply them + +Every list returns `{ data, nextCursor }`. Whether it *pages* is a separate, pinned decision — see `lib/api/contracts/v2/__tests__/list-pagination.test.ts`, which enumerates both sets and fails when a new list is in neither. + +Build the query slice from the shared helper, never by hand: + +```ts +...v2PaginationFields({ description: 'Maximum widgets to return per page.' }) +``` + +That gives `limit` (integer, 1..`V2_MAX_PAGE_SIZE`, defaulting to `V2_DEFAULT_PAGE_SIZE` = 50) and an opaque `cursor`. Re-declaring `limit: z.coerce.number()...` inline is how the 500 happened; there is one schema so the family cannot drift again. + +Two cursor schemes exist, both opaque base64-JSON from `response.ts`. Which one you use is decided by what the read can express, not by taste: + +- **Keyset** (`readSortedCursor` in, `encodeSortedCursor` out) — the default. Requires the page to come from one ordered SQL read. The sort is stamped into the cursor and re-checked on replay, so changing `sortBy` mid-pagination is a 400, not a silently skipped page. +- **Offset** (`decodeOffsetCursor` / `encodeOffsetCursor`) — only when a keyset is impossible. Two lists qualify: `GET /skills` merges a static in-code registry with DB rows and re-sorts in JS, and `GET /knowledge/{id}/documents` sits on a limit/offset query. An offset cursor **must** be stamped with `offsetCursorScope(...)` covering every param that filters or orders the sequence (not `limit`, which only selects how much of it to return). A bare offset replayed against a re-sorted or re-filtered sequence names a different row, which skips or repeats results — the exact failure the keyset's sort stamp already prevents. + +**A keyset's key list must end in a unique column (`id`).** A non-unique trailing key cannot separate tied rows, so the page boundary either repeats or drops them. `lib/api/list-keyset-paging.test.ts` demonstrates the failure. + +Return `nextCursor: null` on the last page and only then. Never construct a cursor client-side. + +## Rule 4 — reject what you do not implement + +Query and body schemas are **`.strict()`**. Zod strips unknown keys by default, so a non-strict schema answers `?limit=1` with 200 and the whole set — the caller believes it bounded the response and it did not. That is a contract lie, and on an uncapped list it is also an unbounded-response risk. + +Error messages name the field and, where there is one, the escape hatch: + +``` +limit must be a whole number +limit cannot exceed 100 +search cannot be empty +sortBy: expected one of "name" | "createdAt" | "updatedAt" +Limit cannot exceed 1000; use limit=0 to stream all rows, or create an export +``` + +That last one is the standard to aim for. A message that only says `Invalid input` fails this rule — the caller cannot act on it. + +## Rule 5 — contract first, then use case, then route + +Order matters because each layer is checked against the one before it. + +1. **Contract** in `lib/api/contracts/v2/.ts` via `defineRouteContract`. Response schemas are `.parse`d on the way out, so a field the producer does not actually emit becomes a 500 on a successful read — assert only what you can prove. +2. **Application use case** owns canonical loading, authorization, business behavior, and audit. The route's `present` receives **only the use-case result**, so anything the presenter needs (e.g. the active `sortBy`/`sortOrder` to stamp a cursor) must be returned by the use case. +3. **Route** with `defineV2JsonRoute`, declaring `contract`, `auth: v2ApiKeyAuth`, `operation`, `rateLimit`, `errorPolicy`, `mapInput`, `useCase`, `present`. Auth and rate limiting run before parsing. +4. **OpenAPI description** in `lib/api/contracts/v2/openapi/.ts`, then `bun run generate:openapi`. A description that claims behaviour the route does not have is the same class of bug as a wrong schema. + +## Checklist + +Run this against any new or changed v2 endpoint. + +- [ ] Success body is exactly `{data}` or `{data, nextCursor}`; failures are exactly `{error:{code,message,details?}}`. +- [ ] Route uses a shared builder; no hand-built `NextResponse.json`. +- [ ] Query and body schemas are `.strict()`. +- [ ] No caller-supplied value can produce a 500 — check every numeric param reaches SQL as a validated integer. +- [ ] `limit` comes from `v2PaginationFields`, not a hand-written `z.coerce.number()`. +- [ ] If the response carries `nextCursor`, the query accepts `limit` + `cursor` and the query actually applies them. +- [ ] Keyset sorts end in a unique `id` key. +- [ ] The list is classified in `list-pagination.test.ts`. +- [ ] Cross-tenant access answers 404, never 403. +- [ ] 403s carry a machine-readable `details.code`. +- [ ] Validation messages name the field and echo the valid set. +- [ ] Response schema matches every field the route actually emits. +- [ ] OpenAPI description regenerated and truthful about pagination. +- [ ] `bun run type-check`, `bun run check:api-validation`, `bun run check:openapi` pass. + +## Known gap + +A 405 on a path that *does* have a route file but does not export that verb is generated by Next.js before any Sim code runs: zero-byte body, no `content-type`, and no `Allow` header, which RFC 9110 §15.5.6 requires. Fixing it means either exporting explicit rejecting handlers from all 77 v2 route files or intercepting in `apps/sim/proxy.ts` with a static path→methods table. Neither is done. Unknown *paths* are handled — the catch-all covers those. diff --git a/apps/docs/openapi-v2-billing.json b/apps/docs/openapi-v2-billing.json index 05e85b78dd7..a03fc6a80eb 100644 --- a/apps/docs/openapi-v2-billing.json +++ b/apps/docs/openapi-v2-billing.json @@ -174,10 +174,10 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum usage events per page, from 1 to 100.", + "description": "Maximum usage events per page. Must be a whole number from 1 to 100. Defaults to 50.", "schema": { "default": 50, - "description": "Maximum usage events per page, from 1 to 100.", + "description": "Maximum usage events per page. Must be a whole number from 1 to 100. Defaults to 50.", "type": "integer", "minimum": 1, "maximum": 100 diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index 53404f27332..702da1a9924 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -104,11 +104,13 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum files per page, clamped to 1–1000.", + "description": "Maximum files per page. Values outside 1–1000 are truncated and clamped into that range rather than rejected. Defaults to 100.", "schema": { - "description": "Maximum files per page, clamped to 1–1000.", - "default": 100, - "type": "number" + "description": "Maximum files per page. Values outside 1–1000 are truncated and clamped into that range rather than rejected. Defaults to 100.", + "type": "integer", + "minimum": 1, + "maximum": 1000, + "default": 100 } }, { @@ -984,11 +986,11 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum entries per page, from 1 to 100.", + "description": "Maximum audit entries to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "schema": { "default": 50, - "description": "Maximum entries per page, from 1 to 100.", - "type": "number", + "description": "Maximum audit entries to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", "minimum": 1, "maximum": 100 } @@ -1000,7 +1002,8 @@ "description": "Opaque cursor returned by the previous page.", "schema": { "description": "Opaque cursor returned by the previous page.", - "type": "string" + "type": "string", + "minLength": 1 } }, { diff --git a/apps/docs/openapi-v2-knowledge.json b/apps/docs/openapi-v2-knowledge.json index 37a2238c614..2ec9e730198 100644 --- a/apps/docs/openapi-v2-knowledge.json +++ b/apps/docs/openapi-v2-knowledge.json @@ -36,7 +36,7 @@ "get": { "operationId": "listKnowledgeBases", "summary": "List Knowledge Bases", - "description": "List knowledge bases in a workspace with folder filtering, search, sorting, and the canonical cursor envelope. The bounded workspace set is returned in one page with `nextCursor` always null; there is no second page to fetch. An unknown `folderPath` is a 404. A workspace whose folder tree exceeds 10,000 folders is a 413, because the response needs the whole tree to render folder paths.", + "description": "List knowledge bases in a workspace with folder filtering, search, and sorting. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null. An unknown `folderPath` is a 404. A workspace whose folder tree exceeds 10,000 folders is a 413, because the response needs the whole tree to render folder paths.", "tags": ["Knowledge Bases"], "parameters": [ { @@ -95,6 +95,30 @@ "type": "string", "enum": ["asc", "desc"] } + }, + { + "name": "limit", + "in": "query", + "required": false, + "description": "Maximum knowledge bases to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "schema": { + "default": 50, + "description": "Maximum knowledge bases to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", + "minimum": 1, + "maximum": 100 + } + }, + { + "name": "cursor", + "in": "query", + "required": false, + "description": "Opaque cursor returned by the previous page.", + "schema": { + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 + } } ], "responses": { diff --git a/apps/docs/openapi-v2-logs.json b/apps/docs/openapi-v2-logs.json index d4df4c2b0df..e38fe2e6026 100644 --- a/apps/docs/openapi-v2-logs.json +++ b/apps/docs/openapi-v2-logs.json @@ -187,21 +187,24 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum log entries per page, clamped to 1–1000.", + "description": "Maximum log entries per page. Values outside 1–1000 are truncated and clamped into that range rather than rejected. Defaults to 100.", "schema": { - "description": "Maximum log entries per page, clamped to 1–1000.", - "default": 100, - "type": "number" + "description": "Maximum log entries per page. Values outside 1–1000 are truncated and clamped into that range rather than rejected. Defaults to 100.", + "type": "integer", + "minimum": 1, + "maximum": 1000, + "default": 100 } }, { "name": "cursor", "in": "query", "required": false, - "description": "Opaque cursor returned by a previous page.", + "description": "Opaque cursor returned by the previous page.", "schema": { + "description": "Opaque cursor returned by the previous page.", "type": "string", - "description": "Opaque cursor returned by a previous page." + "minLength": 1 } }, { diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index 920147e91ce..741b4b84251 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -139,10 +139,10 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum members to return. Defaults to 50 and cannot exceed 100.", + "description": "Maximum members to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "schema": { "default": 50, - "description": "Maximum members to return. Defaults to 50 and cannot exceed 100.", + "description": "Maximum members to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "type": "integer", "minimum": 1, "maximum": 100 @@ -152,9 +152,9 @@ "name": "cursor", "in": "query", "required": false, - "description": "Opaque cursor returned by the preceding page.", + "description": "Opaque cursor returned by the previous page.", "schema": { - "description": "Opaque cursor returned by the preceding page.", + "description": "Opaque cursor returned by the previous page.", "type": "string", "minLength": 1 } @@ -599,7 +599,7 @@ "get": { "operationId": "listSkills", "summary": "List Skills", - "description": "List workspace and built-in skills. Built-ins are marked read-only. The list omits skill bodies and uses the standard cursor envelope with `nextCursor` always null, so there is no second page to fetch; fetch one skill to read its content.", + "description": "List workspace and built-in skills. Built-ins are marked read-only. The list omits skill bodies; fetch one skill to read its content. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null.", "tags": ["Skills"], "parameters": [ { @@ -648,6 +648,30 @@ "type": "string", "enum": ["asc", "desc"] } + }, + { + "name": "limit", + "in": "query", + "required": false, + "description": "Maximum skills to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "schema": { + "default": 50, + "description": "Maximum skills to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", + "minimum": 1, + "maximum": 100 + } + }, + { + "name": "cursor", + "in": "query", + "required": false, + "description": "Opaque cursor returned by the previous page.", + "schema": { + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 + } } ], "responses": { @@ -991,7 +1015,7 @@ "get": { "operationId": "listCustomTools", "summary": "List Custom Tools", - "description": "List code-backed custom tools defined in a workspace. Legacy personal tools are excluded. The bounded workspace set uses the standard cursor envelope with `nextCursor` always null; there is no second page to fetch.", + "description": "List code-backed custom tools defined in a workspace. Legacy personal tools are excluded. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null.", "tags": ["Custom Tools"], "parameters": [ { @@ -1040,6 +1064,30 @@ "type": "string", "enum": ["asc", "desc"] } + }, + { + "name": "limit", + "in": "query", + "required": false, + "description": "Maximum custom tools to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "schema": { + "default": 50, + "description": "Maximum custom tools to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", + "minimum": 1, + "maximum": 100 + } + }, + { + "name": "cursor", + "in": "query", + "required": false, + "description": "Opaque cursor returned by the previous page.", + "schema": { + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 + } } ], "responses": { @@ -1383,7 +1431,7 @@ "get": { "operationId": "listCredentials", "summary": "List Credentials", - "description": "List OAuth and service-account connections visible to the caller. Secret material is never returned. Credential mutations and single-resource reads are intentionally not exposed. The bounded set uses the standard cursor envelope with `nextCursor` always null; there is no second page to fetch.", + "description": "List OAuth and service-account connections visible to the caller. Secret material is never returned. Credential mutations and single-resource reads are intentionally not exposed. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null.", "tags": ["Credentials"], "parameters": [ { @@ -1454,6 +1502,30 @@ "type": "string", "enum": ["asc", "desc"] } + }, + { + "name": "limit", + "in": "query", + "required": false, + "description": "Maximum credentials to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "schema": { + "default": 50, + "description": "Maximum credentials to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", + "minimum": 1, + "maximum": 100 + } + }, + { + "name": "cursor", + "in": "query", + "required": false, + "description": "Opaque cursor returned by the previous page.", + "schema": { + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 + } } ], "responses": { @@ -1506,7 +1578,7 @@ "get": { "operationId": "listSecrets", "summary": "List Secrets", - "description": "List workspace and caller-owned personal secret metadata. Only names, scope, role, and timestamps are returned; secret values are never read or returned. The bounded set uses the standard cursor envelope with `nextCursor` always null; there is no second page to fetch. A workspace API key cannot call this operation and is rejected with `403`; use a personal API key.", + "description": "List workspace and caller-owned personal secret metadata. Only names, scope, role, and timestamps are returned; secret values are never read or returned. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null. A workspace API key cannot call this operation and is rejected with `403`; use a personal API key.", "tags": ["Secrets"], "parameters": [ { @@ -1566,6 +1638,30 @@ "type": "string", "enum": ["asc", "desc"] } + }, + { + "name": "limit", + "in": "query", + "required": false, + "description": "Maximum secrets to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "schema": { + "default": 50, + "description": "Maximum secrets to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", + "minimum": 1, + "maximum": 100 + } + }, + { + "name": "cursor", + "in": "query", + "required": false, + "description": "Opaque cursor returned by the previous page.", + "schema": { + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 + } } ], "responses": { diff --git a/apps/docs/openapi-v2-tables.json b/apps/docs/openapi-v2-tables.json index 20c7ec7bb62..64601b2a7ef 100644 --- a/apps/docs/openapi-v2-tables.json +++ b/apps/docs/openapi-v2-tables.json @@ -100,9 +100,9 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum tables to return (1-1000). Fractional or out-of-range values are truncated and clamped into that range rather than rejected.", + "description": "Maximum tables to return per page. Values outside 1–1000 are truncated and clamped into that range rather than rejected. Defaults to 100.", "schema": { - "description": "Maximum tables to return (1-1000). Fractional or out-of-range values are truncated and clamped into that range rather than rejected.", + "description": "Maximum tables to return per page. Values outside 1–1000 are truncated and clamped into that range rather than rejected. Defaults to 100.", "type": "integer", "minimum": 1, "maximum": 1000, @@ -113,9 +113,9 @@ "name": "cursor", "in": "query", "required": false, - "description": "Opaque cursor from the previous page.", + "description": "Opaque cursor returned by the previous page.", "schema": { - "description": "Opaque cursor from the previous page.", + "description": "Opaque cursor returned by the previous page.", "type": "string", "minLength": 1 } diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index 722d9688c06..77cb3151340 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -78,11 +78,11 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum workflows to return per page.", + "description": "Maximum workflows to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "schema": { "default": 50, - "description": "Maximum workflows to return per page.", - "type": "number", + "description": "Maximum workflows to return per page. Must be a whole number from 1 to 100. Defaults to 50.", + "type": "integer", "minimum": 1, "maximum": 100 } @@ -91,10 +91,11 @@ "name": "cursor", "in": "query", "required": false, - "description": "Opaque pagination cursor returned by a previous request.", + "description": "Opaque cursor returned by the previous page.", "schema": { - "description": "Opaque pagination cursor returned by a previous request.", - "type": "string" + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 } }, { @@ -496,10 +497,10 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum deployment versions to return per page.", + "description": "Maximum deployment versions to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "schema": { "default": 50, - "description": "Maximum deployment versions to return per page.", + "description": "Maximum deployment versions to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "type": "integer", "minimum": 1, "maximum": 100 @@ -509,10 +510,11 @@ "name": "cursor", "in": "query", "required": false, - "description": "Opaque pagination cursor returned by a previous request.", + "description": "Opaque cursor returned by the previous page.", "schema": { - "description": "Opaque pagination cursor returned by a previous request.", - "type": "string" + "description": "Opaque cursor returned by the previous page.", + "type": "string", + "minLength": 1 } } ], @@ -1237,10 +1239,10 @@ "name": "limit", "in": "query", "required": false, - "description": "Maximum workflow runs to return per page.", + "description": "Maximum workflow runs to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "schema": { "default": 50, - "description": "Maximum workflow runs to return per page.", + "description": "Maximum workflow runs to return per page. Must be a whole number from 1 to 100. Defaults to 50.", "type": "integer", "minimum": 1, "maximum": 100 @@ -1250,9 +1252,9 @@ "name": "cursor", "in": "query", "required": false, - "description": "Opaque pagination cursor returned by a previous request.", + "description": "Opaque cursor returned by the previous page.", "schema": { - "description": "Opaque pagination cursor returned by a previous request.", + "description": "Opaque cursor returned by the previous page.", "type": "string", "minLength": 1 } diff --git a/apps/sim/app/api/credentials/route.ts b/apps/sim/app/api/credentials/route.ts index 20b4a4bcac0..69ec1fb54e2 100644 --- a/apps/sim/app/api/credentials/route.ts +++ b/apps/sim/app/api/credentials/route.ts @@ -222,7 +222,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { types: type ? [type] : undefined, providerId, }) - const credentials = visible.map(({ hasServiceAccountKey: _hasKey, ...rest }) => rest) + const credentials = visible.data.map(({ hasServiceAccountKey: _hasKey, ...rest }) => rest) return NextResponse.json({ credentials }) } catch (error) { diff --git a/apps/sim/app/api/v2/[[...segments]]/route.test.ts b/apps/sim/app/api/v2/[[...segments]]/route.test.ts new file mode 100644 index 00000000000..4d4e14da713 --- /dev/null +++ b/apps/sim/app/api/v2/[[...segments]]/route.test.ts @@ -0,0 +1,50 @@ +/** + * @vitest-environment node + */ +import { NextRequest } from 'next/server' +import { describe, expect, it } from 'vitest' +import { DELETE, GET, PATCH, POST, PUT } from '@/app/api/v2/[[...segments]]/route' + +/** + * An unknown path under `/api/v2` used to fall through to the app's global + * `not-found` page, so a mistyped URL handed an API client a full HTML document + * — the one v2 response a JSON-parsing caller cannot read. + * + * The body must stay byte-identical to the rollout gate's 404 + * (`v2ApiGateError`), which answers 404 so an ungated caller cannot tell "not in + * the cohort" from "no such endpoint". A different body here would give that + * distinction straight back. + */ +describe('unknown /api/v2 path', () => { + const EXPECTED = { error: { code: 'NOT_FOUND', message: 'Not found' } } + + function probe(method: string) { + return new NextRequest('http://localhost/api/v2/nonexistent', { method }) + } + + it('answers JSON, not an HTML document', async () => { + const response = await GET(probe('GET'), undefined) + + expect(response.status).toBe(404) + expect(response.headers.get('content-type')).toContain('application/json') + expect(await response.json()).toEqual(EXPECTED) + }) + + it.each([ + ['POST', POST], + ['PUT', PUT], + ['PATCH', PATCH], + ['DELETE', DELETE], + ])('answers the same envelope for %s', async (method, handler) => { + const response = await handler(probe(method), undefined) + + expect(response.status).toBe(404) + expect(await response.json()).toEqual(EXPECTED) + }) + + it('does not require an API key, so probing a typo cannot become a 401', async () => { + const response = await GET(probe('GET'), undefined) + + expect(response.status).toBe(404) + }) +}) diff --git a/apps/sim/app/api/v2/[[...segments]]/route.ts b/apps/sim/app/api/v2/[[...segments]]/route.ts new file mode 100644 index 00000000000..e02fa71a017 --- /dev/null +++ b/apps/sim/app/api/v2/[[...segments]]/route.ts @@ -0,0 +1,40 @@ +import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { v2Error } from '@/app/api/v2/lib/response' + +export const dynamic = 'force-dynamic' +export const revalidate = 0 + +/** + * JSON 404 for any `/api/v2` path that matches no route file. + * + * Without it a mistyped path falls through to the app's global `not-found` + * page and hands an API client a full HTML document, which is the one v2 + * response a JSON-parsing caller cannot read. Every other v2 failure — including + * the rollout gate's own 404 — is the `{ error: { code, message } }` envelope. + * + * The body is deliberately byte-identical to `v2ApiGateError`'s. The gate + * answers 404 so an ungated caller cannot distinguish "not in the rollout + * cohort" from "no such endpoint"; a different body here would reintroduce + * exactly that distinction. + * + * This is a documented raw-`withRouteHandler` route rather than a contract + * builder: it has no contract, no operation, and no authentication, because a + * caller probing an unknown path must get the same answer whether or not it + * holds a key — requiring auth first would turn the 404 into a 401 and confirm + * that the path is special. + * + * Next.js only routes a request here when no literal segment matches, so the 77 + * real v2 routes are unaffected. The optional form (`[[...segments]]`) also + * covers bare `/api/v2`. It cannot fix a 405 on a path that *does* have a route + * file but does not export that verb — Next generates that response itself, + * before any handler runs. + */ +const notFound = () => v2Error('NOT_FOUND', 'Not found') + +export const GET = withRouteHandler(notFound) +export const POST = withRouteHandler(notFound) +export const PUT = withRouteHandler(notFound) +export const PATCH = withRouteHandler(notFound) +export const DELETE = withRouteHandler(notFound) +export const HEAD = withRouteHandler(notFound) +export const OPTIONS = withRouteHandler(notFound) diff --git a/apps/sim/app/api/v2/credentials/route.test.ts b/apps/sim/app/api/v2/credentials/route.test.ts index a1987f1b5af..a84ba1f6d68 100644 --- a/apps/sim/app/api/v2/credentials/route.test.ts +++ b/apps/sim/app/api/v2/credentials/route.test.ts @@ -27,6 +27,7 @@ vi.mock('@/lib/credentials/application/list-workspace-credentials', () => ({ }, })) +import { V2_DEFAULT_PAGE_SIZE } from '@/lib/api/contracts/v2/shared' import { GET } from '@/app/api/v2/credentials/route' const WORKSPACE_ID = '11111111-2222-4333-8444-555555555555' @@ -65,7 +66,12 @@ describe('GET /api/v2/credentials', () => { v2RouteMocks.gate.mockResolvedValue(null) v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED) v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED) - mocks.execute.mockResolvedValue({ credentials: [credential] }) + mocks.execute.mockResolvedValue({ + credentials: [credential], + nextCursorKeys: null, + sortBy: 'createdAt', + sortOrder: 'desc', + }) }) it('authenticates and charges before validating workspace input', async () => { @@ -93,6 +99,9 @@ describe('GET /api/v2/credentials', () => { search: undefined, sortBy: 'createdAt', sortOrder: 'desc', + limit: V2_DEFAULT_PAGE_SIZE, + cursor: undefined, + cursorKeys: undefined, }, request, }) diff --git a/apps/sim/app/api/v2/credentials/route.ts b/apps/sim/app/api/v2/credentials/route.ts index 0312ea3a957..057ab1012be 100644 --- a/apps/sim/app/api/v2/credentials/route.ts +++ b/apps/sim/app/api/v2/credentials/route.ts @@ -8,6 +8,7 @@ import { import { listWorkspaceCredentials } from '@/lib/credentials/application/list-workspace-credentials' import { credentialOperations } from '@/lib/credentials/application/operations' import { toV2Credential } from '@/app/api/v2/credentials/utils' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -19,10 +20,15 @@ export const GET = defineV2JsonRoute({ operation: credentialOperations.listConnections, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - mapInput: ({ query }) => query, + mapInput: ({ query }) => ({ + ...query, + cursorKeys: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), + }), useCase: listWorkspaceCredentials, - present: ({ credentials }) => ({ + present: ({ credentials, nextCursorKeys, sortBy, sortOrder }) => ({ data: credentials.map(toV2Credential), - nextCursor: null, + nextCursor: nextCursorKeys + ? encodeSortedCursor(cursorSortKey(sortBy, sortOrder), nextCursorKeys) + : null, }), }) diff --git a/apps/sim/app/api/v2/custom-tools/route.test.ts b/apps/sim/app/api/v2/custom-tools/route.test.ts index f0d113e8961..b4609531cff 100644 --- a/apps/sim/app/api/v2/custom-tools/route.test.ts +++ b/apps/sim/app/api/v2/custom-tools/route.test.ts @@ -54,6 +54,7 @@ vi.mock('@/lib/custom-tools/application/use-cases', () => ({ }, })) +import { V2_DEFAULT_PAGE_SIZE } from '@/lib/api/contracts/v2/shared' import { GET, POST } from '@/app/api/v2/custom-tools/route' const WORKSPACE_ID = 'workspace-1' @@ -121,9 +122,10 @@ describe('/api/v2/custom-tools', () => { principal: PRINCIPAL, input: { workspaceId: WORKSPACE_ID, - search: undefined, sortBy: 'createdAt', sortOrder: 'desc', + limit: V2_DEFAULT_PAGE_SIZE, + cursorKeys: undefined, }, request: expect.anything(), }) diff --git a/apps/sim/app/api/v2/custom-tools/route.ts b/apps/sim/app/api/v2/custom-tools/route.ts index 0da00d8ce8f..88d691efd3a 100644 --- a/apps/sim/app/api/v2/custom-tools/route.ts +++ b/apps/sim/app/api/v2/custom-tools/route.ts @@ -14,6 +14,7 @@ import { listWorkspaceCustomToolsUseCase, } from '@/lib/custom-tools/application/use-cases' import { toV2CustomTool } from '@/app/api/v2/custom-tools/utils' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -25,9 +26,17 @@ export const GET = defineV2JsonRoute({ auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - mapInput: ({ query }) => query, + mapInput: ({ query }) => ({ + ...query, + cursorKeys: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), + }), useCase: listWorkspaceCustomToolsUseCase, - present: ({ tools }) => ({ data: tools.map(toV2CustomTool), nextCursor: null }), + present: ({ tools, nextCursorKeys, sortBy, sortOrder }) => ({ + data: tools.map(toV2CustomTool), + nextCursor: nextCursorKeys + ? encodeSortedCursor(cursorSortKey(sortBy, sortOrder), nextCursorKeys) + : null, + }), }) /** POST /api/v2/custom-tools — Create a custom tool. */ diff --git a/apps/sim/app/api/v2/files/[fileId]/content/route.ts b/apps/sim/app/api/v2/files/[fileId]/content/route.ts index 48d35dd94b7..c2a64201d23 100644 --- a/apps/sim/app/api/v2/files/[fileId]/content/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/content/route.ts @@ -8,7 +8,6 @@ import { } from '@/lib/workspace-files/application/update-workspace-file-content' import { MAX_WORKSPACE_FILE_INLINE_BODY_BYTES } from '@/lib/workspace-files/orchestration' import { toV2File } from '@/app/api/v2/files/utils' -import { v2Error } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -21,7 +20,6 @@ export const PUT = defineV2JsonRoute({ rateLimit: v2RateLimits.publicApi, errorPolicy: v2FileErrorPolicies.concealResourceAuthorization, parseOptions: { - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), maxBodyBytes: MAX_WORKSPACE_FILE_INLINE_BODY_BYTES, }, beforeParse: async ({ principal, params }) => { diff --git a/apps/sim/app/api/v2/files/route.ts b/apps/sim/app/api/v2/files/route.ts index 9b905ca9342..bf3c3b4a291 100644 --- a/apps/sim/app/api/v2/files/route.ts +++ b/apps/sim/app/api/v2/files/route.ts @@ -3,9 +3,7 @@ import { v2CreateFileContract, v2ListFilesContract, } from '@/lib/api/contracts/v2/files' -import { INVALID_CURSOR_MESSAGE } from '@/lib/api/list-query' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' -import { OrchestrationError } from '@/lib/core/orchestration/types' import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' import { createWorkspaceFile } from '@/lib/workspace-files/application/create-workspace-file' @@ -13,12 +11,7 @@ import { queryWorkspaceFilePage } from '@/lib/workspace-files/application/list-w import { fileOperations } from '@/lib/workspace-files/application/operations' import { MAX_WORKSPACE_FILE_INLINE_BODY_BYTES } from '@/lib/workspace-files/orchestration' import { toV2File, toV2Files } from '@/app/api/v2/files/utils' -import { - cursorSortKey, - decodeSortedCursor, - encodeSortedCursor, - v2Error, -} from '@/app/api/v2/lib/response' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -30,23 +23,16 @@ export const GET = defineV2JsonRoute({ operation: fileOperations.list, rateLimit: v2RateLimits.publicApi, errorPolicy: v2FileErrorPolicies.default, - mapInput: ({ query }) => { - const cursorSort = cursorSortKey(query.sortBy, query.sortOrder) - const decoded = decodeSortedCursor(query.cursor, cursorSort) - if (decoded.status === 'invalid') { - throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE) - } - return { - workspaceId: query.workspaceId, - folderPath: query.folderPath, - search: query.search, - sortBy: query.sortBy, - sortOrder: query.sortOrder, - limit: query.limit, - after: decoded.status === 'ok' ? decoded.keys : undefined, - cursorSort, - } - }, + mapInput: ({ query }) => ({ + workspaceId: query.workspaceId, + folderPath: query.folderPath, + search: query.search, + sortBy: query.sortBy, + sortOrder: query.sortOrder, + limit: query.limit, + after: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), + cursorSort: cursorSortKey(query.sortBy, query.sortOrder), + }), useCase: queryWorkspaceFilePage, present: async ({ files, nextKeys, cursorSort }) => { const items: V2File[] = await toV2Files(files) @@ -62,7 +48,6 @@ export const POST = defineV2JsonRoute({ rateLimit: v2RateLimits.publicApi, errorPolicy: v2FileErrorPolicies.default, parseOptions: { - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), maxBodyBytes: MAX_WORKSPACE_FILE_INLINE_BODY_BYTES, }, mapInput: ({ body }) => ({ diff --git a/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts b/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts index 6ba62a1a941..dfd8dd1255c 100644 --- a/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts +++ b/apps/sim/app/api/v2/knowledge/[id]/documents/route.ts @@ -29,7 +29,11 @@ import { captureServerEvent } from '@/lib/posthog/server' import { MAX_KNOWLEDGE_DOCUMENT_FILE_SIZE } from '@/lib/uploads/shared/types' import { validateFileType } from '@/lib/uploads/utils/validation' import { serializeDate } from '@/app/api/v1/knowledge/utils' -import { decodeOffsetCursor, encodeCursor } from '@/app/api/v2/lib/response' +import { + decodeOffsetCursor, + encodeOffsetCursor, + offsetCursorScope, +} from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -71,21 +75,37 @@ export const GET = defineV2JsonRoute({ operation: knowledgeOperations.listDocuments, rateLimit: v2RateLimits.publicApi, errorPolicy: v2KnowledgeErrorPolicies.concealKnowledgeBaseAuthorization, - mapInput: ({ params, query }) => ({ - knowledgeBaseId: params.id, - assertedWorkspaceId: query.workspaceId, - enabledFilter: query.enabledFilter, - search: query.search, - limit: query.limit, - offset: decodeOffsetCursor(query.cursor), - sortBy: query.sortBy, - sortOrder: query.sortOrder, - }), + mapInput: ({ params, query }) => { + /** + * The offset counts positions in the filtered, sorted document sequence, so + * every param that changes that sequence is stamped into the cursor and + * re-checked here. `limit` selects how much of the sequence to return, not + * what the sequence is, so it stays out. + */ + const cursorScope = offsetCursorScope({ + knowledgeBaseId: params.id, + enabledFilter: query.enabledFilter, + search: query.search, + sortBy: query.sortBy, + sortOrder: query.sortOrder, + }) + return { + knowledgeBaseId: params.id, + assertedWorkspaceId: query.workspaceId, + enabledFilter: query.enabledFilter, + search: query.search, + limit: query.limit, + offset: decodeOffsetCursor(query.cursor, cursorScope), + sortBy: query.sortBy, + sortOrder: query.sortOrder, + cursorScope, + } + }, useCase: listKnowledgeDocuments, - present: ({ documents, pagination }) => ({ + present: ({ documents, pagination, cursorScope }) => ({ data: documents.map(toV2DocumentSummary), nextCursor: pagination.hasMore - ? encodeCursor({ offset: pagination.offset + pagination.limit }) + ? encodeOffsetCursor(cursorScope ?? '', pagination.offset + pagination.limit) : null, }), }) diff --git a/apps/sim/app/api/v2/knowledge/[id]/route.ts b/apps/sim/app/api/v2/knowledge/[id]/route.ts index b9bcee78298..6e2fdf65ba3 100644 --- a/apps/sim/app/api/v2/knowledge/[id]/route.ts +++ b/apps/sim/app/api/v2/knowledge/[id]/route.ts @@ -13,7 +13,6 @@ import { } from '@/lib/knowledge/application/knowledge-bases' import { knowledgeOperations } from '@/lib/knowledge/application/operations' import { toV2KnowledgeBase } from '@/app/api/v2/knowledge/utils' -import { v2Error } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -42,9 +41,6 @@ export const PATCH = defineV2JsonRoute({ operation: knowledgeOperations.update, rateLimit: v2RateLimits.publicApi, errorPolicy: v2KnowledgeErrorPolicies.concealKnowledgeBaseAuthorization, - parseOptions: { - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), - }, mapInput: ({ params, body }) => ({ knowledgeBaseId: params.id, assertedWorkspaceId: body.workspaceId, diff --git a/apps/sim/app/api/v2/knowledge/folders/route.ts b/apps/sim/app/api/v2/knowledge/folders/route.ts index 45887f4c4b9..8392f343e68 100644 --- a/apps/sim/app/api/v2/knowledge/folders/route.ts +++ b/apps/sim/app/api/v2/knowledge/folders/route.ts @@ -18,7 +18,6 @@ import { relocateKnowledgeFolder, } from '@/lib/knowledge/application/folders' import { knowledgeOperations } from '@/lib/knowledge/application/operations' -import { v2Error } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -49,9 +48,6 @@ export const POST = defineV2JsonRoute({ operation: knowledgeOperations.createFolder, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - parseOptions: { - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), - }, mapInput: ({ body }) => ({ workspaceId: body.workspaceId, path: body.path, source: 'api' }), useCase: createKnowledgeFolder, present: ({ folder }) => ({ data: toFolderPathView(folder, folder.path) }), @@ -63,9 +59,6 @@ export const PATCH = defineV2JsonRoute({ operation: knowledgeOperations.relocateFolder, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - parseOptions: { - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), - }, mapInput: ({ body }) => ({ workspaceId: body.workspaceId, path: body.path, diff --git a/apps/sim/app/api/v2/knowledge/route.test.ts b/apps/sim/app/api/v2/knowledge/route.test.ts index 22084ec7aca..d2a9bf3e81a 100644 --- a/apps/sim/app/api/v2/knowledge/route.test.ts +++ b/apps/sim/app/api/v2/knowledge/route.test.ts @@ -59,6 +59,7 @@ vi.mock('@/lib/users/queries', () => ({ requireResolvedUserEmail: (emails: Map, userId: string) => emails.get(userId)!, })) +import { V2_DEFAULT_PAGE_SIZE } from '@/lib/api/contracts/v2/shared' import { GET, POST } from '@/app/api/v2/knowledge/route' const WORKSPACE_ID = 'workspace-1' @@ -104,6 +105,9 @@ describe('/api/v2/knowledge route composition', () => { mockGetUserEmailsByIds.mockResolvedValue(new Map([['user-1', 'owner@example.com']])) mockList.mockResolvedValue({ knowledgeBases: [{ knowledgeBase: buildKnowledgeBase(), folderPath: '/' }], + nextCursorKeys: null, + sortBy: 'name', + sortOrder: 'desc', }) mockCreate.mockResolvedValue({ knowledgeBase: buildKnowledgeBase(), folderPath: '/' }) }) @@ -125,6 +129,8 @@ describe('/api/v2/knowledge route composition', () => { search: 'support', sortBy: 'name', sortOrder: 'desc', + limit: V2_DEFAULT_PAGE_SIZE, + cursorKeys: undefined, }, request, }) diff --git a/apps/sim/app/api/v2/knowledge/route.ts b/apps/sim/app/api/v2/knowledge/route.ts index f8f0ee2e4a5..789db35d934 100644 --- a/apps/sim/app/api/v2/knowledge/route.ts +++ b/apps/sim/app/api/v2/knowledge/route.ts @@ -16,7 +16,7 @@ import { import { knowledgeOperations } from '@/lib/knowledge/application/operations' import { captureServerEvent } from '@/lib/posthog/server' import { toV2KnowledgeBase, toV2KnowledgeBases } from '@/app/api/v2/knowledge/utils' -import { v2Error } from '@/app/api/v2/lib/response' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -34,11 +34,15 @@ export const GET = defineV2JsonRoute({ search: query.search, sortBy: query.sortBy, sortOrder: query.sortOrder, + limit: query.limit, + cursorKeys: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), }), useCase: listKnowledgeBases, - present: async ({ knowledgeBases }) => ({ + present: async ({ knowledgeBases, nextCursorKeys, sortBy, sortOrder }) => ({ data: await toV2KnowledgeBases(knowledgeBases), - nextCursor: null, + nextCursor: nextCursorKeys + ? encodeSortedCursor(cursorSortKey(sortBy, sortOrder), nextCursorKeys) + : null, }), }) @@ -49,9 +53,6 @@ export const POST = defineV2JsonRoute({ operation: knowledgeOperations.create, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - parseOptions: { - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), - }, mapInput: ({ body }) => ({ workspaceId: body.workspaceId, name: body.name, diff --git a/apps/sim/app/api/v2/knowledge/search/route.ts b/apps/sim/app/api/v2/knowledge/search/route.ts index 070e1342015..95bef90bd1b 100644 --- a/apps/sim/app/api/v2/knowledge/search/route.ts +++ b/apps/sim/app/api/v2/knowledge/search/route.ts @@ -3,7 +3,6 @@ import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/ import { v2KnowledgeErrorPolicies } from '@/lib/knowledge/api/route-policies' import { knowledgeOperations } from '@/lib/knowledge/application/operations' import { searchKnowledge } from '@/lib/knowledge/application/search' -import { v2Error } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -24,7 +23,6 @@ export const POST = defineV2JsonRoute({ errorPolicy: v2KnowledgeErrorPolicies.concealKnowledgeBaseUsageAuthorization, parseOptions: { maxBodyBytes: V2_KNOWLEDGE_SEARCH_MAX_BODY_BYTES, - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), }, mapInput: ({ body }) => ({ workspaceId: body.workspaceId, diff --git a/apps/sim/app/api/v2/lib/response.ts b/apps/sim/app/api/v2/lib/response.ts index c0567024d65..93c983828cb 100644 --- a/apps/sim/app/api/v2/lib/response.ts +++ b/apps/sim/app/api/v2/lib/response.ts @@ -174,18 +174,56 @@ export function decodeCursor>(cursor: string): T | n } } +interface OffsetCursorPayload { + /** The query state the offset counts positions within. */ + scope: string + offset: number +} + /** - * Reads back an offset cursor minted by `encodeCursor({ offset })`. + * The filters and sort an offset cursor was minted under. + * + * An offset is only meaningful against one exact sequence, so everything that + * reorders or re-filters that sequence has to travel with it. Build the stamp + * from every such param; a value that does not affect ordering or membership + * (the page size itself) must stay out, or paging with a different `limit` + * would be rejected for no reason. + */ +export function offsetCursorScope(parts: Record): string { + return Object.keys(parts) + .sort() + .map((key) => `${key}=${parts[key] ?? ''}`) + .join('&') +} + +/** An offset cursor stamped with the query state that produced it. */ +export function encodeOffsetCursor(scope: string, offset: number): string { + return encodeCursor({ scope, offset } satisfies OffsetCursorPayload) +} + +/** + * Reads back an offset cursor, refusing one minted under different filters or a + * different sort. * * An absent cursor means page one. A cursor that is not valid base64-JSON, or * that does not carry a non-negative integer `offset`, is rejected rather than * coerced to 0: silently restarting at page one while the caller believes it is - * paging forward makes a paging client loop over the first page forever. The v2 - * error policies render the thrown validation error as the canonical 400. + * paging forward makes a paging client loop over the first page forever. + * + * The `scope` check is the offset counterpart of {@link decodeSortedCursor}'s + * sort stamp. A bare offset replayed against a newly filtered or re-sorted + * sequence names a different position in it, which silently skips rows, repeats + * them, or lands past the end and returns an empty page — the failure a keyset + * cursor is already protected from. The v2 error policies render the thrown + * validation error as the canonical 400. */ -export function decodeOffsetCursor(cursor: string | undefined): number { +export function decodeOffsetCursor(cursor: string | undefined, scope: string): number { if (!cursor) return 0 - const offset = decodeCursor<{ offset?: unknown }>(cursor)?.offset + const decoded = decodeCursor>(cursor) + if (!decoded || decoded.scope !== scope) { + throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE) + } + const { offset } = decoded if (typeof offset !== 'number' || !Number.isInteger(offset) || offset < 0) { throw new OrchestrationError('validation', 'Invalid cursor') } @@ -241,9 +279,25 @@ export function decodeSortedCursor(cursor: string | undefined, sort: string): De return { status: 'ok', keys: decoded.keys } } -/** The 400 for a cursor that cannot be resumed under the request's sort. */ -export function v2CursorSortError(): NextResponse { - return v2Error('BAD_REQUEST', INVALID_CURSOR_MESSAGE) +/** + * The keyset a paged list should resume from, or `undefined` for page one. + * + * This is the `mapInput` half of every keyset list: it stamps the request's + * sort, reads the cursor back under it, and turns a cursor that was minted + * under a different sort into the canonical 400 rather than letting mismatched + * keys reach `keysetAfter`. Sharing it is what keeps "a bad cursor is a 400" + * from being re-decided per route. + */ +export function readSortedCursor( + cursor: string | undefined, + sortBy: string, + sortOrder: string +): CursorKey[] | undefined { + const decoded = decodeSortedCursor(cursor, cursorSortKey(sortBy, sortOrder)) + if (decoded.status === 'invalid') { + throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE) + } + return decoded.status === 'ok' ? decoded.keys : undefined } const V2_CODE_BY_ORCHESTRATION_ERROR: Record = { diff --git a/apps/sim/app/api/v2/secrets/route.test.ts b/apps/sim/app/api/v2/secrets/route.test.ts index 5c173b4d883..47147b25f91 100644 --- a/apps/sim/app/api/v2/secrets/route.test.ts +++ b/apps/sim/app/api/v2/secrets/route.test.ts @@ -46,6 +46,7 @@ vi.mock('@/lib/secrets/application/use-cases', () => ({ listSecretsUseCase: { operation: { id: 'secrets.list' }, execute: mocks.list }, })) +import { V2_DEFAULT_PAGE_SIZE } from '@/lib/api/contracts/v2/shared' import { GET } from '@/app/api/v2/secrets/route' const WORKSPACE_ID = 'workspace-1' @@ -88,7 +89,13 @@ describe('GET /api/v2/secrets', () => { mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) mocks.gate.mockResolvedValue(null) - mocks.list.mockResolvedValue({ secrets: [secret], userId: 'user-1' }) + mocks.list.mockResolvedValue({ + secrets: [secret], + userId: 'user-1', + nextCursorKeys: null, + sortBy: 'name', + sortOrder: 'asc', + }) }) it('lists secret metadata without exposing values', async () => { @@ -121,6 +128,9 @@ describe('GET /api/v2/secrets', () => { search: undefined, sortBy: 'name', sortOrder: 'asc', + limit: V2_DEFAULT_PAGE_SIZE, + cursor: undefined, + cursorKeys: undefined, }, request: expect.anything(), }) diff --git a/apps/sim/app/api/v2/secrets/route.ts b/apps/sim/app/api/v2/secrets/route.ts index 62a1685dc88..c0b64d7f338 100644 --- a/apps/sim/app/api/v2/secrets/route.ts +++ b/apps/sim/app/api/v2/secrets/route.ts @@ -7,6 +7,7 @@ import { } from '@/lib/api/server/routes' import { secretOperations } from '@/lib/secrets/application/operations' import { listSecretsUseCase } from '@/lib/secrets/application/use-cases' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' import { toV2Secret } from '@/app/api/v2/secrets/utils' export const dynamic = 'force-dynamic' @@ -19,10 +20,15 @@ export const GET = defineV2JsonRoute({ auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - mapInput: ({ query }) => query, + mapInput: ({ query }) => ({ + ...query, + cursorKeys: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), + }), useCase: listSecretsUseCase, - present: ({ secrets, userId }) => ({ + present: ({ secrets, userId, nextCursorKeys, sortBy, sortOrder }) => ({ data: secrets.map((secret) => toV2Secret(secret, userId)), - nextCursor: null, + nextCursor: nextCursorKeys + ? encodeSortedCursor(cursorSortKey(sortBy, sortOrder), nextCursorKeys) + : null, }), }) diff --git a/apps/sim/app/api/v2/skills/route.test.ts b/apps/sim/app/api/v2/skills/route.test.ts index 6d7770e05a0..6e193ae8d11 100644 --- a/apps/sim/app/api/v2/skills/route.test.ts +++ b/apps/sim/app/api/v2/skills/route.test.ts @@ -53,6 +53,17 @@ vi.mock('@/lib/skills/application/use-cases', () => ({ import { GET, POST } from '@/app/api/v2/skills/route' const WORKSPACE_ID = 'workspace-1' + +/** + * The scope stamp the route mints for the default query. Written out rather + * than imported so the test pins the wire format a shipped cursor carries. + */ +const SCOPE = ({ + search = '', + sortBy = 'createdAt', + sortOrder = 'desc', +}: Record = {}) => + `search=${search}&sortBy=${sortBy}&sortOrder=${sortOrder}&workspaceId=${WORKSPACE_ID}` const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' } const AUTH = { principal: PRINCIPAL, @@ -97,7 +108,7 @@ describe('/api/v2/skills', () => { mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) mocks.gate.mockResolvedValue(null) - mocks.list.mockResolvedValue({ skills: [skill] }) + mocks.list.mockResolvedValue({ skills: [skill], hasMore: false, offset: 0, limit: 50 }) mocks.create.mockResolvedValue({ skill }) }) @@ -105,7 +116,9 @@ describe('/api/v2/skills', () => { const response = await GET(request('GET', `/api/v2/skills?workspaceId=${WORKSPACE_ID}`)) expect(response.status).toBe(200) - expect((await response.json()).data[0]).not.toHaveProperty('content') + const body = await response.json() + expect(body.data[0]).not.toHaveProperty('content') + expect(body.nextCursor).toBeNull() expect(mocks.list).toHaveBeenCalledWith({ principal: PRINCIPAL, input: { @@ -113,11 +126,68 @@ describe('/api/v2/skills', () => { search: undefined, sortBy: 'createdAt', sortOrder: 'desc', + limit: 50, + cursor: undefined, + offset: 0, + cursorScope: SCOPE(), }, request: expect.anything(), }) }) + it('resumes from the offset cursor and mints the next one while pages remain', async () => { + mocks.list.mockResolvedValueOnce({ + skills: [skill], + hasMore: true, + offset: 2, + limit: 2, + cursorScope: SCOPE(), + }) + const cursor = Buffer.from(JSON.stringify({ scope: SCOPE(), offset: 2 })).toString('base64') + + const response = await GET( + request( + 'GET', + `/api/v2/skills?workspaceId=${WORKSPACE_ID}&limit=2&cursor=${encodeURIComponent(cursor)}` + ) + ) + + expect(response.status).toBe(200) + expect((await response.json()).nextCursor).toBe( + Buffer.from(JSON.stringify({ scope: SCOPE(), offset: 4 })).toString('base64') + ) + expect(mocks.list).toHaveBeenCalledWith( + expect.objectContaining({ input: expect.objectContaining({ limit: 2, offset: 2 }) }) + ) + }) + + /** + * An offset means nothing against a sequence it was not counted in, so a + * cursor minted under one sort must not silently resume under another. + */ + it('rejects a cursor replayed under a different sort', async () => { + const cursor = Buffer.from(JSON.stringify({ scope: SCOPE(), offset: 2 })).toString('base64') + + const response = await GET( + request( + 'GET', + `/api/v2/skills?workspaceId=${WORKSPACE_ID}&sortBy=name&cursor=${encodeURIComponent(cursor)}` + ) + ) + + expect(response.status).toBe(400) + expect(mocks.list).not.toHaveBeenCalled() + }) + + it('rejects a malformed cursor rather than silently restarting at page one', async () => { + const response = await GET( + request('GET', `/api/v2/skills?workspaceId=${WORKSPACE_ID}&cursor=not-a-cursor`) + ) + + expect(response.status).toBe(400) + expect(mocks.list).not.toHaveBeenCalled() + }) + it('creates a skill with the v2 source and status', async () => { const response = await POST( request('POST', '/api/v2/skills', { diff --git a/apps/sim/app/api/v2/skills/route.ts b/apps/sim/app/api/v2/skills/route.ts index b49c685fe90..e1356ef7ea0 100644 --- a/apps/sim/app/api/v2/skills/route.ts +++ b/apps/sim/app/api/v2/skills/route.ts @@ -8,8 +8,28 @@ import { import { captureServerEvent } from '@/lib/posthog/server' import { skillOperations } from '@/lib/skills/application/operations' import { createSkillUseCase, listSkillsUseCase } from '@/lib/skills/application/use-cases' +import { + decodeOffsetCursor, + encodeOffsetCursor, + offsetCursorScope, +} from '@/app/api/v2/lib/response' import { toV2Skill, toV2SkillSummary } from '@/app/api/v2/skills/utils' +/** The query state a skills offset cursor is only valid within. */ +function skillCursorScope(query: { + workspaceId: string + search?: string + sortBy: string + sortOrder: string +}): string { + return offsetCursorScope({ + workspaceId: query.workspaceId, + search: query.search, + sortBy: query.sortBy, + sortOrder: query.sortOrder, + }) +} + export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -20,9 +40,25 @@ export const GET = defineV2JsonRoute({ auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - mapInput: ({ query }) => query, + mapInput: ({ query }) => { + /** + * The offset counts positions in the merged, filtered, sorted sequence, so + * every param that changes that sequence is stamped into the cursor and + * re-checked here. `limit` is deliberately absent — it selects how much of + * the sequence to return, not what the sequence is. + */ + const scope = skillCursorScope(query) + return { + ...query, + offset: decodeOffsetCursor(query.cursor, scope), + cursorScope: scope, + } + }, useCase: listSkillsUseCase, - present: ({ skills }) => ({ data: skills.map(toV2SkillSummary), nextCursor: null }), + present: ({ skills, hasMore, offset, limit, cursorScope }) => ({ + data: skills.map(toV2SkillSummary), + nextCursor: hasMore ? encodeOffsetCursor(cursorScope, offset + limit) : null, + }), }) /** POST /api/v2/skills — Create a skill. */ diff --git a/apps/sim/app/api/v2/skills/utils.ts b/apps/sim/app/api/v2/skills/utils.ts index a1cc30ceb02..34ffbc18002 100644 --- a/apps/sim/app/api/v2/skills/utils.ts +++ b/apps/sim/app/api/v2/skills/utils.ts @@ -3,6 +3,7 @@ import type { NextResponse } from 'next/server' import type { V2Skill, V2SkillSummary } from '@/lib/api/contracts/v2/skills' import type { SkillOrchestrationErrorCode } from '@/lib/skills/orchestration' import { isBuiltinSkillId } from '@/lib/workflows/skills/builtin-skills' +import type { SkillSummaryRow } from '@/lib/workflows/skills/operations' import { v2Error } from '@/app/api/v2/lib/response' /** @@ -11,8 +12,11 @@ import { v2Error } from '@/app/api/v2/lib/response' type SkillRow = typeof skill.$inferSelect -/** List projection — no `content`; skill bodies are fetched per skill. */ -export function toV2SkillSummary(row: SkillRow): V2SkillSummary { +/** + * List projection — no `content`; skill bodies are fetched per skill. It takes + * the body-less row so the list query never has to load one. + */ +export function toV2SkillSummary(row: SkillSummaryRow): V2SkillSummary { return { id: row.id, name: row.name, diff --git a/apps/sim/app/api/v2/tables/route.ts b/apps/sim/app/api/v2/tables/route.ts index b9515fdb91a..9099a1f51b9 100644 --- a/apps/sim/app/api/v2/tables/route.ts +++ b/apps/sim/app/api/v2/tables/route.ts @@ -1,11 +1,9 @@ import { v2CreateTableContract, v2ListTablesContract } from '@/lib/api/contracts/v2/tables' -import { INVALID_CURSOR_MESSAGE } from '@/lib/api/list-query' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' -import { OrchestrationError } from '@/lib/core/orchestration/types' import { v2TableErrorPolicies } from '@/lib/table/api' import { tableOperations } from '@/lib/table/application/operations' import { createTableUseCase, listTablesUseCase } from '@/lib/table/application/tables' -import { cursorSortKey, decodeSortedCursor, encodeSortedCursor } from '@/app/api/v2/lib/response' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' import { toApiTable, toApiTables } from '@/app/api/v2/tables/utils' export const dynamic = 'force-dynamic' @@ -18,22 +16,15 @@ export const GET = defineV2JsonRoute({ auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2TableErrorPolicies.default, - mapInput: ({ query }) => { - const sort = cursorSortKey(query.sortBy, query.sortOrder) - const decoded = decodeSortedCursor(query.cursor, sort) - if (decoded.status === 'invalid') { - throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE) - } - return { - workspaceId: query.workspaceId, - folderPath: query.folderPath, - search: query.search, - sortBy: query.sortBy, - sortOrder: query.sortOrder, - limit: query.limit, - after: decoded.status === 'ok' ? decoded.keys : undefined, - } - }, + mapInput: ({ query }) => ({ + workspaceId: query.workspaceId, + folderPath: query.folderPath, + search: query.search, + sortBy: query.sortBy, + sortOrder: query.sortOrder, + limit: query.limit, + after: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), + }), present: async ({ tables, nextKeys, sortBy, sortOrder }) => ({ data: await toApiTables(tables), nextCursor: nextKeys ? encodeSortedCursor(cursorSortKey(sortBy, sortOrder), nextKeys) : null, diff --git a/apps/sim/app/api/v2/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/v2/workflows/[id]/deploy/route.test.ts index 8e77d94b1dd..21e6ad41708 100644 --- a/apps/sim/app/api/v2/workflows/[id]/deploy/route.test.ts +++ b/apps/sim/app/api/v2/workflows/[id]/deploy/route.test.ts @@ -30,6 +30,12 @@ import { workflowOperations } from '@/lib/workflows/application/operations' import { DELETE, POST } from '@/app/api/v2/workflows/[id]/deploy/route' describe('/api/v2/workflows/[id]/deploy route definitions', () => { + /** + * Both the malformed-body 400 and the oversized-body 413 are v2 builder + * defaults, so neither belongs on the route. The envelope they produce is + * asserted once against the builder in + * `lib/api/server/routes/v2-error-envelope.test.ts`. + */ it('keeps an omitted deploy body valid and binds the authorized deployment use case', async () => { expect(v2DeployWorkflowContract.body?.parse(undefined)).toEqual({}) expect(POST).toMatchObject({ @@ -46,15 +52,7 @@ describe('/api/v2/workflows/[id]/deploy route definitions', () => { }) ) - const invalidJsonResponse = Reflect.get( - Reflect.get(POST, 'parseOptions'), - 'invalidJsonResponse' - )() - expect(invalidJsonResponse.status).toBe(400) - expect(await invalidJsonResponse.json()).toEqual({ - error: { code: 'BAD_REQUEST', message: 'Request body must be valid JSON' }, - }) - + expect(Reflect.get(Reflect.get(POST, 'parseOptions'), 'invalidJsonResponse')).toBeUndefined() expect( Reflect.get(Reflect.get(POST, 'parseOptions'), 'payloadTooLargeResponse') ).toBeUndefined() diff --git a/apps/sim/app/api/v2/workflows/[id]/deploy/route.ts b/apps/sim/app/api/v2/workflows/[id]/deploy/route.ts index a72e3838f7f..93f0e6a8cdb 100644 --- a/apps/sim/app/api/v2/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/v2/workflows/[id]/deploy/route.ts @@ -8,7 +8,6 @@ import { captureServerEvent } from '@/lib/posthog/server' import { v2WorkflowErrorPolicies } from '@/lib/workflows/api' import { deployWorkflow, undeployWorkflow } from '@/lib/workflows/application/deployments' import { workflowOperations } from '@/lib/workflows/application/operations' -import { v2Error } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const runtime = 'nodejs' @@ -22,7 +21,6 @@ export const POST = defineV2JsonRoute({ errorPolicy: v2WorkflowErrorPolicies.concealWorkflowAuthorization, parseOptions: { optionalJsonBody: true, - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), }, mapInput: ({ params, body }) => ({ workflowId: params.id, diff --git a/apps/sim/app/api/v2/workflows/[id]/execute/route.ts b/apps/sim/app/api/v2/workflows/[id]/execute/route.ts index b9fe5eab1e9..95015eb4055 100644 --- a/apps/sim/app/api/v2/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/v2/workflows/[id]/execute/route.ts @@ -12,6 +12,7 @@ import { import { parseRequest } from '@/lib/api/server' import { admitOptionalV2Request, + V2_PARSE_DEFAULTS, V2RouteInfrastructureError, v2ApiKeyAuth, v2RateLimits, @@ -185,6 +186,7 @@ export const POST = withRouteHandler( try { const parsed = await parseRequest(v2ExecuteWorkflowContract, req, context, { + ...V2_PARSE_DEFAULTS, maxBodyBytes: 10 * 1024 * 1024, validationErrorResponse: v2ValidationError, }) diff --git a/apps/sim/app/api/v2/workflows/[id]/rollback/route.test.ts b/apps/sim/app/api/v2/workflows/[id]/rollback/route.test.ts index 2bd018a3df9..e06621ad0cc 100644 --- a/apps/sim/app/api/v2/workflows/[id]/rollback/route.test.ts +++ b/apps/sim/app/api/v2/workflows/[id]/rollback/route.test.ts @@ -25,6 +25,12 @@ import { workflowOperations } from '@/lib/workflows/application/operations' import { POST } from '@/app/api/v2/workflows/[id]/rollback/route' describe('/api/v2/workflows/[id]/rollback route definition', () => { + /** + * Both the malformed-body 400 and the oversized-body 413 are v2 builder + * defaults, so neither belongs on the route. The envelope they produce is + * asserted once against the builder in + * `lib/api/server/routes/v2-error-envelope.test.ts`. + */ it('keeps an omitted rollback body valid and delegates version selection to the use case', async () => { expect(v2RollbackWorkflowContract.body?.parse(undefined)).toEqual({}) expect(POST).toMatchObject({ @@ -41,15 +47,7 @@ describe('/api/v2/workflows/[id]/rollback route definition', () => { }) ) - const invalidJsonResponse = Reflect.get( - Reflect.get(POST, 'parseOptions'), - 'invalidJsonResponse' - )() - expect(invalidJsonResponse.status).toBe(400) - expect(await invalidJsonResponse.json()).toEqual({ - error: { code: 'BAD_REQUEST', message: 'Request body must be valid JSON' }, - }) - + expect(Reflect.get(Reflect.get(POST, 'parseOptions'), 'invalidJsonResponse')).toBeUndefined() expect( Reflect.get(Reflect.get(POST, 'parseOptions'), 'payloadTooLargeResponse') ).toBeUndefined() diff --git a/apps/sim/app/api/v2/workflows/[id]/rollback/route.ts b/apps/sim/app/api/v2/workflows/[id]/rollback/route.ts index c0b9b6c20e0..3c40d92b4ba 100644 --- a/apps/sim/app/api/v2/workflows/[id]/rollback/route.ts +++ b/apps/sim/app/api/v2/workflows/[id]/rollback/route.ts @@ -4,7 +4,6 @@ import { generateRequestId } from '@/lib/core/utils/request' import { v2WorkflowErrorPolicies } from '@/lib/workflows/api' import { activateWorkflowVersion } from '@/lib/workflows/application/deployments' import { workflowOperations } from '@/lib/workflows/application/operations' -import { v2Error } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const runtime = 'nodejs' @@ -18,7 +17,6 @@ export const POST = defineV2JsonRoute({ errorPolicy: v2WorkflowErrorPolicies.concealWorkflowAuthorization, parseOptions: { optionalJsonBody: true, - invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'), }, mapInput: ({ params, body }) => ({ workflowId: params.id, diff --git a/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.test.ts b/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.test.ts index 2d726aa208d..87d14c90168 100644 --- a/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.test.ts +++ b/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.test.ts @@ -45,6 +45,7 @@ vi.mock('@/lib/api/server/routes', () => { V2RouteInfrastructureError, v2ApiKeyAuth: { kind: 'v2-api-key' }, v2RateLimits: { publicApi: { kind: 'public-api' } }, + V2_PARSE_DEFAULTS: {}, v2OrchestrationErrorPolicy: { render: renderOrchestrationError }, } }) diff --git a/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.ts b/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.ts index cc0938d8232..5e459182179 100644 --- a/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.ts +++ b/apps/sim/app/api/v2/workflows/[id]/runs/[runId]/resume/route.ts @@ -8,6 +8,7 @@ import { import { parseRequest } from '@/lib/api/server' import { admitV2Request, + V2_PARSE_DEFAULTS, V2RouteInfrastructureError, v2ApiKeyAuth, v2RateLimits, @@ -52,6 +53,7 @@ export const POST = withRouteHandler( if (!admission.success) return admission.response const parsed = await parseRequest(v2ResumeWorkflowContract, request, context, { + ...V2_PARSE_DEFAULTS, maxBodyBytes: 10 * 1024 * 1024, validationErrorResponse: v2ValidationError, }) diff --git a/apps/sim/app/api/v2/workflows/route.ts b/apps/sim/app/api/v2/workflows/route.ts index f4dc34c3ef7..8fd65da66ab 100644 --- a/apps/sim/app/api/v2/workflows/route.ts +++ b/apps/sim/app/api/v2/workflows/route.ts @@ -1,17 +1,15 @@ import type { V2WorkflowListItem } from '@/lib/api/contracts/v2/workflows' import { v2CreateWorkflowContract, v2ListWorkflowsContract } from '@/lib/api/contracts/v2/workflows' -import { INVALID_CURSOR_MESSAGE } from '@/lib/api/list-query' import { defineV2JsonRoute, v2ApiKeyAuth, v2OrchestrationErrorPolicy, v2RateLimits, } from '@/lib/api/server/routes' -import { OrchestrationError } from '@/lib/core/orchestration/types' import { createWorkflow } from '@/lib/workflows/application/create-workflow' import { listWorkflows } from '@/lib/workflows/application/list-workflows' import { workflowOperations } from '@/lib/workflows/application/operations' -import { cursorSortKey, decodeSortedCursor, encodeSortedCursor } from '@/app/api/v2/lib/response' +import { cursorSortKey, encodeSortedCursor, readSortedCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' export const revalidate = 0 @@ -22,23 +20,16 @@ export const GET = defineV2JsonRoute({ operation: workflowOperations.list, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - mapInput: ({ query }) => { - const sort = cursorSortKey(query.sortBy, query.sortOrder) - const decoded = decodeSortedCursor(query.cursor, sort) - if (decoded.status === 'invalid') { - throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE) - } - return { - workspaceId: query.workspaceId, - folderPath: query.folderPath, - deployedOnly: query.deployedOnly, - search: query.search, - sortBy: query.sortBy, - sortOrder: query.sortOrder, - cursorKeys: decoded.status === 'ok' ? decoded.keys : undefined, - limit: query.limit, - } - }, + mapInput: ({ query }) => ({ + workspaceId: query.workspaceId, + folderPath: query.folderPath, + deployedOnly: query.deployedOnly, + search: query.search, + sortBy: query.sortBy, + sortOrder: query.sortOrder, + cursorKeys: readSortedCursor(query.cursor, query.sortBy, query.sortOrder), + limit: query.limit, + }), useCase: listWorkflows, present: ({ workflows, nextCursorKeys, sortBy, sortOrder }) => ({ data: workflows.map( diff --git a/apps/sim/lib/api/contracts/v2/__tests__/list-pagination.test.ts b/apps/sim/lib/api/contracts/v2/__tests__/list-pagination.test.ts index de20ecbc46f..cbaf49cc562 100644 --- a/apps/sim/lib/api/contracts/v2/__tests__/list-pagination.test.ts +++ b/apps/sim/lib/api/contracts/v2/__tests__/list-pagination.test.ts @@ -46,9 +46,14 @@ const CONTRACTS_DIR = path.resolve(import.meta.dirname, '..', '..') const PAGED_LISTS = [ 'GET /api/v2/audit-logs', 'GET /api/v2/billing/logs', + 'GET /api/v2/credentials', + 'GET /api/v2/custom-tools', 'GET /api/v2/files', + 'GET /api/v2/knowledge', 'GET /api/v2/knowledge/[id]/documents', 'GET /api/v2/logs', + 'GET /api/v2/secrets', + 'GET /api/v2/skills', 'GET /api/v2/tables', 'GET /api/v2/tables/[tableId]/rows', 'POST /api/v2/tables/[tableId]/query', @@ -61,22 +66,35 @@ const PAGED_LISTS = [ /** * Lists that accept neither param and always return `nextCursor: null`, because * the set is small and bounded per workspace or per table. + * + * Every remaining entry but the MCP server list is a *folder* list, and a folder + * tree is already capped where it is loaded + * (`MAX_*_FOLDERS_PER_WORKSPACE`) — bounded by construction rather than by a + * caller's `limit`. */ const FULL_SET_LISTS = [ - 'GET /api/v2/credentials', - 'GET /api/v2/custom-tools', 'GET /api/v2/files/folders', - 'GET /api/v2/knowledge', 'GET /api/v2/knowledge/folders', 'GET /api/v2/mcp-servers', - 'GET /api/v2/secrets', - 'GET /api/v2/skills', 'GET /api/v2/tables/[tableId]/groups', 'GET /api/v2/tables/[tableId]/views', 'GET /api/v2/tables/folders', 'GET /api/v2/workflows/folders', ] as const +/** + * Lists that deliberately truncate a fractional `limit` instead of rejecting it. + * + * These three shipped that leniency and published it in their OpenAPI + * description, so tightening them would turn a currently-successful request into + * an error. Everything else must reject — see the fractional-limit test below. + */ +const CLAMPED_LIMIT_LISTS = new Set([ + 'GET /api/v2/files', + 'GET /api/v2/logs', + 'GET /api/v2/tables', +]) + interface ContractLike { method: string path: string @@ -201,6 +219,47 @@ function paginationParams(variants: string[][]): { any: string[]; all: string[] } } +/** + * Whether a schema rejects keys it does not declare, i.e. is `.strict()`. + * + * This is what separates "this list does not page" from "this list quietly + * throws your `limit` away". Zod strips unknown keys by default, so a full-set + * list that is not strict answers `?limit=1` with 200 and the entire set — the + * caller believes it bounded the response and it did not. Only the outermost + * object is inspected; that is where a query's unknown key is caught. + */ +function rejectsUnknownKeys(schema: unknown, depth: number = MAX_SCHEMA_DEPTH): boolean | null { + if (!schema || depth <= 0) return null + const def = (schema as { def?: Record }).def + if (!def) return null + if (def.type === 'object') { + return (def.catchall as { def?: { type?: string } } | undefined)?.def?.type === 'never' + } + const inner = def.innerType ?? def.in ?? def.schema + return inner ? rejectsUnknownKeys(inner, depth - 1) : null +} + +/** + * Whether a fractional `limit` is rejected by whichever slot carries it. + * + * The other required params are left out on purpose: the schema reports every + * failure at once, so it is enough to ask whether one of them is about `limit`. + * That keeps this free of per-resource fixtures and lets it run over every list + * the sweep finds — which matters, because the one list that still carried the + * original `LIMIT 2.5` defect was the one nobody remembered to add to a + * hand-written list. + */ +function rejectsFractionalLimit(contract: ContractLike): boolean { + for (const schema of [contract.query, contract.body]) { + if (!schema) continue + const result = schema.safeParse({ limit: '1.5' }) + if (!result.success && result.error.issues.some((issue) => issue.path[0] === 'limit')) { + return true + } + } + return false +} + function listContractFiles(dir: string): string[] { const files: string[] = [] for (const entry of readdirSync(dir, { withFileTypes: true }).sort((a, b) => @@ -221,6 +280,10 @@ interface V2ListContract { key: string name: string params: { any: string[]; all: string[] } + /** `undefined` when the contract has no `query`; `null` when it could not be introspected. */ + strictQuery: boolean | null | undefined + /** Whether a fractional `limit` draws a validation issue on `limit` itself. */ + rejectsFractionalLimit: boolean } /** @@ -245,7 +308,13 @@ async function sweepV2ListContracts(): Promise { const label = `${name} (${key})` if (!isListResponse(label, value.response?.schema)) continue if (found.has(key)) continue - found.set(key, { key, name, params: paginationParams(inputVariants(label, value)) }) + found.set(key, { + key, + name, + params: paginationParams(inputVariants(label, value)), + strictQuery: value.query ? rejectsUnknownKeys(value.query) : undefined, + rejectsFractionalLimit: rejectsFractionalLimit(value), + }) } } return [...found.values()].sort((a, b) => a.key.localeCompare(b.key)) @@ -268,7 +337,7 @@ describe('v2 list pagination split', () => { expect( unclassified, - 'A new v2 list must be classified in PAGED_LISTS or FULL_SET_LISTS.' + 'A new v2 list must be classified in PAGED_LISTS or FULL_SET_LISTS. See .agents/skills/v2-api-conventions/SKILL.md.' ).toEqual([]) expect(contracts.map((c) => c.key).sort()).toEqual([...classified].sort()) }, @@ -282,7 +351,7 @@ describe('v2 list pagination split', () => { for (const key of PAGED_LISTS) { expect( byKey.get(key)?.params.all, - `${key} is declared paged, so every input shape it accepts must offer limit and cursor` + `${key} is declared paged, so every input shape it accepts must offer limit and cursor. See .agents/skills/v2-api-conventions/SKILL.md.` ).toEqual(['limit', 'cursor']) } }) @@ -294,11 +363,55 @@ describe('v2 list pagination split', () => { for (const key of FULL_SET_LISTS) { expect( byKey.get(key)?.params.any, - `${key} returns the full set; adding a defaulted limit to any accepted input shape would truncate existing callers` + `${key} returns the full set; adding a defaulted limit to any accepted input shape would truncate existing callers. See .agents/skills/v2-api-conventions/SKILL.md.` ).toEqual([]) } }) + it('makes every v2 list query reject a param it does not implement', async () => { + const contracts = await loadV2ListContracts() + const byKey = new Map(contracts.map((c) => [c.key, c])) + + for (const key of [...PAGED_LISTS, ...FULL_SET_LISTS]) { + const strictQuery = byKey.get(key)?.strictQuery + /** + * `undefined` is a list that takes no query at all — `POST .../query` + * carries its input in the body. `null` means the walk could not tell, + * which must fail rather than pass silently. + */ + if (strictQuery === undefined) continue + expect( + strictQuery, + `${key} must declare its query \`.strict()\`. Zod strips unknown keys by default, so a non-strict list answers ?limit=1 with 200 and the whole set — the caller believes it bounded the response and it did not. See .agents/skills/v2-api-conventions/SKILL.md.` + ).toBe(true) + } + }) + + it('never lets a fractional limit reach the query as a fractional LIMIT', async () => { + const contracts = await loadV2ListContracts() + const byKey = new Map(contracts.map((c) => [c.key, c])) + + for (const key of PAGED_LISTS) { + if (CLAMPED_LIMIT_LISTS.has(key)) continue + expect( + byKey.get(key)?.rejectsFractionalLimit, + `${key} accepts a fractional limit, which reaches Postgres as \`LIMIT 2.5\` and answers 500. Build the param from v2PaginationFields. See .agents/skills/v2-api-conventions/SKILL.md.` + ).toBe(true) + } + }) + + it('holds the clamping lists to truncation rather than rejection', async () => { + const contracts = await loadV2ListContracts() + const byKey = new Map(contracts.map((c) => [c.key, c])) + + for (const key of CLAMPED_LIMIT_LISTS) { + expect( + byKey.get(key)?.rejectsFractionalLimit, + `${key} published that it truncates a fractional limit; rejecting it now would break callers relying on that.` + ).toBe(false) + } + }) + it('sees a pagination param hidden in a single union member', () => { const unionQuery = z.union([ z.object({ workspaceId: z.string(), limit: z.coerce.number().default(50) }), diff --git a/apps/sim/lib/api/contracts/v2/__tests__/pagination-limit.test.ts b/apps/sim/lib/api/contracts/v2/__tests__/pagination-limit.test.ts new file mode 100644 index 00000000000..497c4a71c55 --- /dev/null +++ b/apps/sim/lib/api/contracts/v2/__tests__/pagination-limit.test.ts @@ -0,0 +1,112 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { v2ListBillingLogsContract } from '@/lib/api/contracts/v2/billing' +import { v2ListCredentialsContract } from '@/lib/api/contracts/v2/credentials' +import { v2ListCustomToolsContract } from '@/lib/api/contracts/v2/custom-tools' +import { v2ListKnowledgeBasesContract } from '@/lib/api/contracts/v2/knowledge' +import { v2ListSecretsContract } from '@/lib/api/contracts/v2/secrets' +import { + V2_DEFAULT_PAGE_SIZE, + V2_MAX_PAGE_SIZE, + v2LimitSchema, +} from '@/lib/api/contracts/v2/shared' +import { v2ListSkillsContract } from '@/lib/api/contracts/v2/skills' +import { v2ListWorkflowsContract } from '@/lib/api/contracts/v2/workflows' +import { v2ListWorkspaceMembersContract } from '@/lib/api/contracts/v2/workspaces' + +/** + * `limit` is the one v2 query param whose value reaches SQL as a number rather + * than a bound comparison, so a value that survives validation but is not a + * whole number lands in `LIMIT`. `GET /api/v2/workflows` shipped without + * `.int()` — copied from a sibling that had it — and `?limit=1.5` therefore + * became `LIMIT 2.5`, a Postgres type error surfacing as 500. A caller-supplied + * query value must never be able to produce a 500. + */ +describe('v2 limit validation', () => { + const WORKSPACE = '00000000-0000-4000-8000-000000000000' + + function parseLimit(limit: string) { + return v2ListWorkflowsContract.query?.safeParse({ workspaceId: WORKSPACE, limit }) + } + + it('rejects a fractional limit instead of passing it through to SQL', () => { + for (const limit of ['1.5', '2.5', '3.7', '99.9']) { + const parsed = parseLimit(limit) + expect(parsed?.success, `limit=${limit} must not parse`).toBe(false) + expect(parsed?.error?.issues[0]?.message).toBe('limit must be a whole number') + } + }) + + it('still accepts an integral limit written with a decimal point', () => { + const parsed = parseLimit('2.0') + expect(parsed?.success).toBe(true) + expect(parsed?.success && parsed.data.limit).toBe(2) + }) + + it('names the parameter for a non-finite limit rather than "expected number, received number"', () => { + for (const limit of ['Infinity', '-Infinity', 'abc']) { + const parsed = parseLimit(limit) + expect(parsed?.success).toBe(false) + expect(parsed?.error?.issues[0]?.message).toBe('limit must be a number') + } + }) + + it('reports the bound it enforces', () => { + expect(parseLimit('0')?.error?.issues[0]?.message).toBe('limit must be at least 1') + expect(parseLimit('101')?.error?.issues[0]?.message).toBe( + `limit cannot exceed ${V2_MAX_PAGE_SIZE}` + ) + }) + + /** + * The five collections that gained pagination in this change, plus the three + * that already had it. Every one must answer a fractional `limit` the same + * way — the divergence is what let `/workflows` drift into a 500 while its + * two same-file siblings stayed correct. + */ + const PAGED_CONTRACTS = [ + ['workflows', v2ListWorkflowsContract, { workspaceId: WORKSPACE }], + ['workspace members', v2ListWorkspaceMembersContract, {}], + ['billing logs', v2ListBillingLogsContract, { workspaceId: WORKSPACE }], + ['credentials', v2ListCredentialsContract, { workspaceId: WORKSPACE }], + ['custom tools', v2ListCustomToolsContract, { workspaceId: WORKSPACE }], + ['knowledge bases', v2ListKnowledgeBasesContract, { workspaceId: WORKSPACE }], + ['secrets', v2ListSecretsContract, { workspaceId: WORKSPACE }], + ['skills', v2ListSkillsContract, { workspaceId: WORKSPACE }], + ] as const + + it.each(PAGED_CONTRACTS)('rejects a fractional limit on %s', (_name, contract, base) => { + const parsed = contract.query?.safeParse({ ...base, limit: '1.5' }) + expect(parsed?.success).toBe(false) + expect(parsed?.error?.issues[0]?.message).toBe('limit must be a whole number') + }) + + it.each(PAGED_CONTRACTS)('defaults %s to the shared page size', (_name, contract, base) => { + const parsed = contract.query?.safeParse(base) + expect(parsed?.success).toBe(true) + expect(parsed?.success && parsed.data.limit).toBe(V2_DEFAULT_PAGE_SIZE) + }) + + /** + * `/files`, `/logs`, and `/tables` shipped truncating and clamping instead, + * and published that leniency in their OpenAPI description, so they keep it. + * Pinning it here is what makes the difference deliberate rather than another + * copy that lost its `.int()`. + */ + describe('the clamping variant kept for already-shipped lenient lists', () => { + const clamped = v2LimitSchema({ max: 1000, fallback: 100, outOfRange: 'clamp' }) + + it('truncates a fractional limit rather than rejecting it', () => { + expect(clamped.parse('1.5')).toBe(1) + expect(clamped.parse('7.9')).toBe(7) + }) + + it('clamps out-of-range values into the accepted band', () => { + expect(clamped.parse('0')).toBe(1) + expect(clamped.parse('-5')).toBe(1) + expect(clamped.parse('99999')).toBe(1000) + }) + }) +}) diff --git a/apps/sim/lib/api/contracts/v2/audit-logs.ts b/apps/sim/lib/api/contracts/v2/audit-logs.ts index b2485d29cfc..8df2a91061e 100644 --- a/apps/sim/lib/api/contracts/v2/audit-logs.ts +++ b/apps/sim/lib/api/contracts/v2/audit-logs.ts @@ -5,7 +5,11 @@ import { v1AuditLogParamsSchema, v1ListAuditLogsQuerySchema, } from '@/lib/api/contracts/v1/audit-logs' -import { v2CursorListResponse, v2DataResponse } from '@/lib/api/contracts/v2/shared' +import { + v2CursorListResponse, + v2DataResponse, + v2PaginationFields, +} from '@/lib/api/contracts/v2/shared' /** * v2 audit-logs contracts. These are org-scoped enterprise endpoints. The @@ -84,12 +88,7 @@ export const v2ListAuditLogsQuerySchema = v1ListAuditLogsQuerySchema includeDeparted: v1ListAuditLogsQuerySchema.shape.includeDeparted.describe( 'Include actions by users who have left the organization.' ), - limit: v1ListAuditLogsQuerySchema.shape.limit.describe( - 'Maximum entries per page, from 1 to 100.' - ), - cursor: v1ListAuditLogsQuerySchema.shape.cursor.describe( - 'Opaque cursor returned by the previous page.' - ), + ...v2PaginationFields({ description: 'Maximum audit entries to return per page.' }), organizationId: organizationIdSchema.describe( 'Organization whose audit trail should be queried.' ), diff --git a/apps/sim/lib/api/contracts/v2/billing.ts b/apps/sim/lib/api/contracts/v2/billing.ts index 3dcfd28b9d9..78eb5759bdb 100644 --- a/apps/sim/lib/api/contracts/v2/billing.ts +++ b/apps/sim/lib/api/contracts/v2/billing.ts @@ -2,7 +2,11 @@ import { z } from 'zod' import { workspaceIdSchema } from '@/lib/api/contracts/primitives' import { defineRouteContract } from '@/lib/api/contracts/types' import { usageLogPeriodSchema, usageLogSourceSchema } from '@/lib/api/contracts/user' -import { v2CursorListResponse, v2DataResponse } from '@/lib/api/contracts/v2/shared' +import { + v2CursorListResponse, + v2DataResponse, + v2PaginationFields, +} from '@/lib/api/contracts/v2/shared' /** * v2 billing contracts — separate read-only status and ledger resources. @@ -141,20 +145,9 @@ export const v2BillingLogsQuerySchema = z endDate: parseableDateSchema .optional() .describe('End of a custom window as a Date-parseable string; defaults to now.'), - limit: z.coerce - .number() - .int() - .min(1) - .max(100) - .optional() - .default(50) - .describe('Maximum usage events per page, from 1 to 100.'), - cursor: z - .string() - .min(1, 'cursor must be a non-empty token') - .optional() - .describe('Opaque cursor returned by the previous page.'), + ...v2PaginationFields({ description: 'Maximum usage events per page.' }), }) + .strict() .refine((query) => query.period !== 'custom' || query.startDate !== undefined, { error: 'startDate is required when period is "custom"', path: ['startDate'], diff --git a/apps/sim/lib/api/contracts/v2/credentials.ts b/apps/sim/lib/api/contracts/v2/credentials.ts index 5e164406730..6694a626a3c 100644 --- a/apps/sim/lib/api/contracts/v2/credentials.ts +++ b/apps/sim/lib/api/contracts/v2/credentials.ts @@ -4,6 +4,7 @@ import { workspaceIdSchema } from '@/lib/api/contracts/primitives' import { defineRouteContract } from '@/lib/api/contracts/types' import { v2CursorListResponse, + v2PaginationFields, v2SearchSchema, v2SortFields, v2TimestampSchema, @@ -47,22 +48,29 @@ export type V2Credential = z.output export const v2CredentialSortFields = ['displayName', 'createdAt', 'updatedAt'] as const export type V2CredentialSortBy = (typeof v2CredentialSortFields)[number] -export const v2ListCredentialsQuerySchema = z.object({ - workspaceId: workspaceIdSchema.describe('Workspace whose credentials should be listed.'), - type: v2CredentialTypeSchema.optional().describe('Restrict results to this credential type.'), - providerId: z - .string() - .min(1, 'providerId cannot be empty') - .optional() - .describe('Restrict results to credentials for this integration provider.'), - search: v2SearchSchema.describe( - 'Case-insensitive substring match against the credential display name.' - ), - ...v2SortFields(v2CredentialSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), -}) +export const v2ListCredentialsQuerySchema = z + .object({ + workspaceId: workspaceIdSchema.describe('Workspace whose credentials should be listed.'), + type: v2CredentialTypeSchema.optional().describe('Restrict results to this credential type.'), + providerId: z + .string() + .min(1, 'providerId cannot be empty') + .optional() + .describe('Restrict results to credentials for this integration provider.'), + search: v2SearchSchema.describe( + 'Case-insensitive substring match against the credential display name.' + ), + ...v2SortFields(v2CredentialSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), + ...v2PaginationFields({ description: 'Maximum credentials to return per page.' }), + }) + .strict() export type V2ListCredentialsQuery = z.output -/** Lists OAuth and service-account connections. Credential mutations are intentionally absent. */ +/** + * Lists OAuth and service-account connections, keyset-paginated over the active + * sort. Credential mutations are intentionally absent. Nothing capped the + * per-workspace set before pagination, so the response grew without bound. + */ export const v2ListCredentialsContract = defineRouteContract({ method: 'GET', path: '/api/v2/credentials', diff --git a/apps/sim/lib/api/contracts/v2/custom-tools.ts b/apps/sim/lib/api/contracts/v2/custom-tools.ts index 9b259ca1686..ae764afba70 100644 --- a/apps/sim/lib/api/contracts/v2/custom-tools.ts +++ b/apps/sim/lib/api/contracts/v2/custom-tools.ts @@ -8,6 +8,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types' import { v2CursorListResponse, v2DataResponse, + v2PaginationFields, v2SearchSchema, v2SortFields, v2TimestampSchema, @@ -119,10 +120,13 @@ export const v2CustomToolSortFields = ['title', 'createdAt', 'updatedAt'] as con export type V2CustomToolSortBy = (typeof v2CustomToolSortFields)[number] -export const v2ListCustomToolsQuerySchema = v2CustomToolWorkspaceQuerySchema.extend({ - search: v2SearchSchema.describe('Case-insensitive substring match against the tool title.'), - ...v2SortFields(v2CustomToolSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), -}) +export const v2ListCustomToolsQuerySchema = v2CustomToolWorkspaceQuerySchema + .extend({ + search: v2SearchSchema.describe('Case-insensitive substring match against the tool title.'), + ...v2SortFields(v2CustomToolSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), + ...v2PaginationFields({ description: 'Maximum custom tools to return per page.' }), + }) + .strict() export type V2ListCustomToolsQuery = z.output @@ -159,9 +163,9 @@ export const v2UpdateCustomToolBodySchema = z export type V2UpdateCustomToolBody = z.input /** - * Custom tool list. The per-workspace set is small and bounded, so the full set - * is returned as a single page (`nextCursor` is always `null`); the canonical - * cursor envelope keeps the v2 list surface uniform. + * Custom tool list, keyset-paginated over the active sort. Nothing capped the + * per-workspace set, and each row carries its full `code` and `schema`, so the + * response grew without bound before pagination. */ export const v2ListCustomToolsContract = defineRouteContract({ method: 'GET', diff --git a/apps/sim/lib/api/contracts/v2/files.ts b/apps/sim/lib/api/contracts/v2/files.ts index 01a50144b8b..3e6d2b6e2a7 100644 --- a/apps/sim/lib/api/contracts/v2/files.ts +++ b/apps/sim/lib/api/contracts/v2/files.ts @@ -16,6 +16,7 @@ import { v2FolderPathSchema, v2FolderSchema, v2ListFoldersQuerySchema, + v2PaginationFields, v2RelocateFolderBodySchema, v2SearchSchema, v2SortFields, @@ -287,13 +288,12 @@ export const v2ListFilesQuerySchema = z .describe('Restrict results to files directly inside this folder.'), search: v2SearchSchema.describe('Case-insensitive substring match against the file name.'), ...v2SortFields(v2FileSortFields, { sortBy: 'uploadedAt', sortOrder: 'asc' }), - limit: z.coerce - .number() - .optional() - .default(100) - .transform((v) => Math.min(Math.max(1, Math.trunc(v)), 1000)) - .describe('Maximum files per page, clamped to 1–1000.'), - cursor: z.string().min(1).optional().describe('Opaque cursor returned by the previous page.'), + ...v2PaginationFields({ + max: 1000, + fallback: 100, + outOfRange: 'clamp', + description: 'Maximum files per page.', + }), }) .strict() diff --git a/apps/sim/lib/api/contracts/v2/knowledge.ts b/apps/sim/lib/api/contracts/v2/knowledge.ts index acc4ecf3ed1..1f8e4d4a6c2 100644 --- a/apps/sim/lib/api/contracts/v2/knowledge.ts +++ b/apps/sim/lib/api/contracts/v2/knowledge.ts @@ -25,6 +25,7 @@ import { v2FolderPathSchema, v2FolderSchema, v2ListFoldersQuerySchema, + v2PaginationFields, v2RelocateFolderBodySchema, v2SearchSchema, v2SortFields, @@ -504,6 +505,7 @@ export const v2ListKnowledgeBasesQuerySchema = z .describe('Restrict results to knowledge bases in this folder.'), search: v2SearchSchema, ...v2SortFields(v2KnowledgeBaseSortFields, { sortBy: 'createdAt', sortOrder: 'asc' }), + ...v2PaginationFields({ description: 'Maximum knowledge bases to return per page.' }), }) .strict() @@ -578,11 +580,12 @@ export const v2UpdateKnowledgeBaseBodySchema = z }) /** - * KB list. `getKnowledgeBases` returns the full workspace set (a small, bounded - * per-workspace list), so today the cursor list is a single full page - * (`nextCursor` always `null`). The canonical cursor envelope keeps the v2 list - * surface uniform; real pagination can be added later behind the opaque cursor. - * Search, folder filter, and sort all run in that query, not over its result. + * KB list, keyset-paginated over the active sort. Search, folder filter, and + * sort all run in the query, not over its result. + * + * Before pagination this list returned `nextCursor` while rejecting `limit` and + * `cursor` outright, so it advertised a pagination scheme no caller could + * drive. */ export const v2ListKnowledgeBasesContract = defineRouteContract({ method: 'GET', @@ -782,6 +785,7 @@ export const v2ListKnowledgeDocumentsQuerySchema = v1ListKnowledgeDocumentsQuery sortOrder: v1ListKnowledgeDocumentsQuerySchema.shape.sortOrder.describe('Sort direction.'), cursor: z.string().min(1).optional().describe('Opaque cursor returned by the previous page.'), }) + .strict() export type V2ListKnowledgeDocumentsQuery = z.output export const v2ListKnowledgeDocumentsContract = defineRouteContract({ diff --git a/apps/sim/lib/api/contracts/v2/logs.ts b/apps/sim/lib/api/contracts/v2/logs.ts index 18895a327cf..a8f7c1af30e 100644 --- a/apps/sim/lib/api/contracts/v2/logs.ts +++ b/apps/sim/lib/api/contracts/v2/logs.ts @@ -8,6 +8,7 @@ import { v2DataResponse, v2FolderPathInputSchema, v2FolderPathSchema, + v2PaginationFields, v2TimestampSchema, } from '@/lib/api/contracts/v2/shared' import { PERSISTED_WORKFLOW_EXECUTION_STATUSES } from '@/lib/logs/types' @@ -225,13 +226,12 @@ export const v2ListLogsQuerySchema = v1ListLogsQuerySchema .describe('Whether to include the final workflow output.') .optional() .default(false), - limit: z.coerce - .number() - .optional() - .default(100) - .transform((value) => Math.min(Math.max(1, Math.trunc(value)), 1000)) - .describe('Maximum log entries per page, clamped to 1–1000.'), - cursor: z.string().describe('Opaque cursor returned by a previous page.').optional(), + ...v2PaginationFields({ + max: 1000, + fallback: 100, + outOfRange: 'clamp', + description: 'Maximum log entries per page.', + }), order: z .enum(['desc', 'asc']) .describe('Sort order by execution start time.') diff --git a/apps/sim/lib/api/contracts/v2/mcp-servers.ts b/apps/sim/lib/api/contracts/v2/mcp-servers.ts index 86d2db5d08f..4ceae20d290 100644 --- a/apps/sim/lib/api/contracts/v2/mcp-servers.ts +++ b/apps/sim/lib/api/contracts/v2/mcp-servers.ts @@ -176,10 +176,12 @@ export const v2McpServerSortFields = ['name', 'createdAt', 'updatedAt'] as const export type V2McpServerSortBy = (typeof v2McpServerSortFields)[number] -export const v2ListMcpServersQuerySchema = v2McpServerWorkspaceQuerySchema.extend({ - search: v2SearchSchema.describe('Case-insensitive substring match against the server name.'), - ...v2SortFields(v2McpServerSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), -}) +export const v2ListMcpServersQuerySchema = v2McpServerWorkspaceQuerySchema + .extend({ + search: v2SearchSchema.describe('Case-insensitive substring match against the server name.'), + ...v2SortFields(v2McpServerSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), + }) + .strict() export type V2ListMcpServersQuery = z.output diff --git a/apps/sim/lib/api/contracts/v2/openapi/knowledge.ts b/apps/sim/lib/api/contracts/v2/openapi/knowledge.ts index 3e86d717eb9..4e4b7a1f68b 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/knowledge.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/knowledge.ts @@ -66,7 +66,7 @@ const routes = [ knowledgeOperation({ operationId: 'listKnowledgeBases', summary: 'List Knowledge Bases', - description: `List knowledge bases in a workspace with folder filtering, search, sorting, and the canonical cursor envelope. The bounded workspace set is returned in one page with \`nextCursor\` always null; there is no second page to fetch. An unknown \`folderPath\` is a 404. ${FOLDER_TREE_TOO_LARGE}`, + description: `List knowledge bases in a workspace with folder filtering, search, and sorting. Paginate with \`limit\` and \`cursor\`, stopping when \`nextCursor\` is null. An unknown \`folderPath\` is a 404. ${FOLDER_TREE_TOO_LARGE}`, errors: [...RESOURCE_ERRORS, 'PayloadTooLarge'], success: { description: 'A page of knowledge bases.' }, }), diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index 2ab84ab4128..66ebd341d63 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -409,7 +409,7 @@ const routes = [ operationId: 'listSkills', summary: 'List Skills', description: - 'List workspace and built-in skills. Built-ins are marked read-only. The list omits skill bodies and uses the standard cursor envelope with `nextCursor` always null, so there is no second page to fetch; fetch one skill to read its content.', + 'List workspace and built-in skills. Built-ins are marked read-only. The list omits skill bodies; fetch one skill to read its content. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null.', errors: [...WORKSPACE_ERRORS, 'NotFound'], success: { description: 'Skills available in the workspace.' }, }), @@ -564,7 +564,7 @@ const routes = [ operationId: 'listCustomTools', summary: 'List Custom Tools', description: - 'List code-backed custom tools defined in a workspace. Legacy personal tools are excluded. The bounded workspace set uses the standard cursor envelope with `nextCursor` always null; there is no second page to fetch.', + 'List code-backed custom tools defined in a workspace. Legacy personal tools are excluded. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null.', errors: [...WORKSPACE_ERRORS, 'NotFound'], success: { description: 'Custom tools defined in the workspace.' }, }), @@ -720,7 +720,7 @@ const routes = [ operationId: 'listCredentials', summary: 'List Credentials', description: - 'List OAuth and service-account connections visible to the caller. Secret material is never returned. Credential mutations and single-resource reads are intentionally not exposed. The bounded set uses the standard cursor envelope with `nextCursor` always null; there is no second page to fetch.', + 'List OAuth and service-account connections visible to the caller. Secret material is never returned. Credential mutations and single-resource reads are intentionally not exposed. Paginate with `limit` and `cursor`, stopping when `nextCursor` is null.', errors: [...WORKSPACE_ERRORS, 'NotFound'], success: { description: 'Credentials visible to the caller.' }, }), @@ -745,7 +745,7 @@ const routes = [ resourceOperation('Secrets', { operationId: 'listSecrets', summary: 'List Secrets', - description: `List workspace and caller-owned personal secret metadata. Only names, scope, role, and timestamps are returned; secret values are never read or returned. The bounded set uses the standard cursor envelope with \`nextCursor\` always null; there is no second page to fetch. ${WORKSPACE_API_KEY_DENIED}`, + description: `List workspace and caller-owned personal secret metadata. Only names, scope, role, and timestamps are returned; secret values are never read or returned. Paginate with \`limit\` and \`cursor\`, stopping when \`nextCursor\` is null. ${WORKSPACE_API_KEY_DENIED}`, errors: [...WORKSPACE_ERRORS, 'NotFound'], success: { description: 'Secret metadata visible to the caller.' }, }), diff --git a/apps/sim/lib/api/contracts/v2/secrets.ts b/apps/sim/lib/api/contracts/v2/secrets.ts index ee566ec649a..13744bf83cb 100644 --- a/apps/sim/lib/api/contracts/v2/secrets.ts +++ b/apps/sim/lib/api/contracts/v2/secrets.ts @@ -5,6 +5,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types' import { v2CursorListResponse, v2DataResponse, + v2PaginationFields, v2SearchSchema, v2SortFields, v2TimestampSchema, @@ -57,12 +58,15 @@ export type V2SecretDeleteData = z.output export const v2SecretSortFields = ['name', 'createdAt', 'updatedAt'] as const export type V2SecretSortBy = (typeof v2SecretSortFields)[number] -export const v2ListSecretsQuerySchema = z.object({ - workspaceId: workspaceIdSchema.describe('Workspace whose secret metadata should be listed.'), - scope: v2SecretScopeSchema.optional().describe('Restrict results to one ownership scope.'), - search: v2SearchSchema.describe('Case-insensitive substring match against the secret name.'), - ...v2SortFields(v2SecretSortFields, { sortBy: 'name', sortOrder: 'asc' }), -}) +export const v2ListSecretsQuerySchema = z + .object({ + workspaceId: workspaceIdSchema.describe('Workspace whose secret metadata should be listed.'), + scope: v2SecretScopeSchema.optional().describe('Restrict results to one ownership scope.'), + search: v2SearchSchema.describe('Case-insensitive substring match against the secret name.'), + ...v2SortFields(v2SecretSortFields, { sortBy: 'name', sortOrder: 'asc' }), + ...v2PaginationFields({ description: 'Maximum secrets to return per page.' }), + }) + .strict() export type V2ListSecretsQuery = z.output export const v2SecretParamsSchema = z.object({ @@ -90,7 +94,10 @@ export const v2DeleteSecretQuerySchema = z.object({ }) export type V2DeleteSecretQuery = z.output -/** Lists names and metadata only. There is deliberately no single-secret GET. */ +/** + * Lists names and metadata only, keyset-paginated over the active sort. There is + * deliberately no single-secret GET, and no response ever carries a value. + */ export const v2ListSecretsContract = defineRouteContract({ method: 'GET', path: '/api/v2/secrets', diff --git a/apps/sim/lib/api/contracts/v2/shared.ts b/apps/sim/lib/api/contracts/v2/shared.ts index d6e05660d9e..62647fa6a74 100644 --- a/apps/sim/lib/api/contracts/v2/shared.ts +++ b/apps/sim/lib/api/contracts/v2/shared.ts @@ -70,9 +70,9 @@ import { FolderPathError, parseFolderPath, requireNonRootFolderPath } from '@/li * on the surface (`scope`, `folderPath`, `deployedOnly`, `type`, `providerId`, * `resourceType`). No generic filter expression. * - * Every one of these is pushed into SQL, except on `GET /skills` (which merges - * the static builtin registry into the DB rows, then re-filters and re-sorts the - * merged array) and `GET /files/folders` (which applies `parentPath` and `search` + * Every one of these is pushed into SQL, except on `GET /skills` (which narrows the + * static builtin registry with the same search term, merges it into the DB rows, + * then re-sorts the merged array) and `GET /files/folders` (which applies `parentPath` and `search` * in JS; its sort is pushed into SQL like every other folder list). Both read a * full result set to produce a page; neither is a pattern to copy. * @@ -82,10 +82,25 @@ import { FolderPathError, parseFolderPath, requireNonRootFolderPath } from '@/li * not restated here. A full-set list returns `nextCursor: null` on every * response — its OpenAPI description says so explicitly, so a caller never * writes a pagination loop that can only ever run once. - * Adding `limit`/`cursor` to a full-set list is additive, - * but making a `limit` *default* would silently truncate callers that rely on - * the full set today, so a default page size cannot be introduced without a - * version bump. + * + * Every list whose result set grows with workspace content is now paged. What + * remains full-set is the folder lists, whose trees are already capped where + * they load, plus `GET /mcp-servers`. + * + * Adding `limit`/`cursor` to a full-set list is additive, but giving it a + * *default* `limit` truncates callers reading the whole set today, so it is a + * breaking change. Five lists took exactly that change while `v2-api` was off + * in production and enabled only for a staging cohort — the window in which it + * costs nothing. Once v2 is generally available, moving a shipped full-set list + * to a defaulted page size needs a version bump. + * + * Both cursor schemes are opaque base64-JSON from `app/api/v2/lib/response.ts`, + * and which one a list uses is decided by what its read can express rather than + * by preference: a keyset (`encodeSortedCursor`) wherever the page comes from + * one ordered SQL read, and an offset (`encodeCursor({ offset })`) only where it + * cannot — `GET /skills`, which merges the static builtin registry into the DB + * rows and re-sorts in JS, and `GET /knowledge/{id}/documents`, whose underlying + * query is limit/offset. Prefer the keyset; an offset needs that kind of reason. * * ## Sort and the opaque cursor * @@ -151,6 +166,112 @@ export const v2CursorListResponse = (itemSchema: T) => ), }) +/** + * Default and maximum page size for a v2 paged list. + * + * These are the values the majority of already-paged v2 lists shipped with + * (`/workflows`, `/workflows/{id}/versions`, `/workflows/{id}/runs`, + * `/workspaces/{id}/members`, `/billing/logs`), so they are what a list adopting + * pagination now inherits. + */ +export const V2_DEFAULT_PAGE_SIZE = 50 +export const V2_MAX_PAGE_SIZE = 100 + +/** + * How a `limit` outside `1..max` is handled. + * + * `reject` is the rule for every list: an out-of-range or fractional page size + * is a 400 naming the bound. `clamp` exists only for the three lists that + * shipped truncating and clamping instead (`/files`, `/logs`, `/tables`) and + * published that leniency in their OpenAPI description — flipping them to + * `reject` would turn a currently-successful request into an error for callers + * already relying on it. New lists must use `reject`; `clamp` is not a pattern + * to copy and should be collapsed into `reject` at the next major version. + */ +export type V2LimitOutOfRange = 'reject' | 'clamp' + +interface V2LimitOptions { + max?: number + /** Page size applied when the caller omits `limit`. */ + fallback?: number + /** Defaults to `reject`. */ + outOfRange?: V2LimitOutOfRange + /** Overrides the generated `describe()` text. */ + description?: string +} + +/** + * The v2 `limit` param, as one schema so the family cannot drift per route. + * + * It exists because it already drifted: `GET /api/v2/workflows` was copied from + * a sibling and lost its `.int()`, so a fractional `limit` passed validation and + * reached Postgres as `LIMIT 2.5`, which is a 500. A caller-supplied query value + * must never be able to produce a 500, and the only durable fix is for every + * list to derive its bound from one place rather than restating it. + * + * `z.coerce.number()` is what accepts the query string at all, so JS numeric + * parsing applies (`1e2` is 100, `0x10` is 16, surrounding whitespace is + * ignored). That leniency is inherited from the coercion, not chosen here; the + * bounds below are what actually constrain the value. + */ +export function v2LimitSchema(options: V2LimitOptions = {}) { + const { + max = V2_MAX_PAGE_SIZE, + fallback = V2_DEFAULT_PAGE_SIZE, + outOfRange = 'reject', + description, + } = options + + /** + * The bounds are appended rather than left to the caller's sentence, so a + * per-list `description` cannot drop them from the published parameter — the + * numbers a caller needs are the ones this schema actually enforces. + */ + const bounds = + outOfRange === 'clamp' + ? `Values outside 1–${max} are truncated and clamped into that range rather than rejected. Defaults to ${fallback}.` + : `Must be a whole number from 1 to ${max}. Defaults to ${fallback}.` + const described = `${description ?? 'Maximum items to return per page.'} ${bounds}` + + const base = z.coerce.number({ error: 'limit must be a number' }) + + if (outOfRange === 'clamp') { + return base + .optional() + .default(fallback) + .transform((value) => Math.min(Math.max(1, Math.trunc(value)), max)) + .describe(described) + .meta({ type: 'integer', minimum: 1, maximum: max }) + } + + return base + .int('limit must be a whole number') + .min(1, 'limit must be at least 1') + .max(max, `limit cannot exceed ${max}`) + .optional() + .default(fallback) + .describe(described) +} + +/** + * The v2 `cursor` param: the opaque token a previous page returned as + * `nextCursor`. Empty is rejected rather than treated as "start over", so a + * caller that accidentally forwards an empty string learns about it instead of + * looping on page one. + */ +export function v2CursorSchema(description = 'Opaque cursor returned by the previous page.') { + return z.string().min(1, 'cursor must be a non-empty token').optional().describe(description) +} + +/** + * The `limit` + `cursor` pair for a paged v2 list. Spread into a query object; + * a list that returns `nextCursor` must accept both, and must actually apply + * them. + */ +export function v2PaginationFields(options: V2LimitOptions = {}) { + return { limit: v2LimitSchema(options), cursor: v2CursorSchema() } +} + /** * The v2 `search` term: a case-insensitive substring match on the resource's * natural name field. Bounded at 200 characters — a longer term cannot match diff --git a/apps/sim/lib/api/contracts/v2/skills.ts b/apps/sim/lib/api/contracts/v2/skills.ts index c307677993a..31a182d3b67 100644 --- a/apps/sim/lib/api/contracts/v2/skills.ts +++ b/apps/sim/lib/api/contracts/v2/skills.ts @@ -9,6 +9,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types' import { v2CursorListResponse, v2DataResponse, + v2PaginationFields, v2SearchSchema, v2SortFields, v2TimestampSchema, @@ -96,10 +97,13 @@ export const v2SkillSortFields = ['name', 'createdAt', 'updatedAt'] as const export type V2SkillSortBy = (typeof v2SkillSortFields)[number] -export const v2ListSkillsQuerySchema = v2SkillWorkspaceQuerySchema.extend({ - search: v2SearchSchema.describe('Case-insensitive substring match against the skill name.'), - ...v2SortFields(v2SkillSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), -}) +export const v2ListSkillsQuerySchema = v2SkillWorkspaceQuerySchema + .extend({ + search: v2SearchSchema.describe('Case-insensitive substring match against the skill name.'), + ...v2SortFields(v2SkillSortFields, { sortBy: 'createdAt', sortOrder: 'desc' }), + ...v2PaginationFields({ description: 'Maximum skills to return per page.' }), + }) + .strict() export type V2ListSkillsQuery = z.output @@ -143,9 +147,11 @@ export const v2UpdateSkillBodySchema = z export type V2UpdateSkillBody = z.input /** - * Skill list. The per-workspace set is small and bounded, so the full set is - * returned as a single page (`nextCursor` is always `null`); the canonical - * cursor envelope keeps the v2 list surface uniform. + * Skill list, paginated by an opaque offset cursor rather than the keyset the + * other v2 lists use: the list merges the code-only built-in skills — which + * have no DB row for a SQL cursor predicate to act on — with the workspace's + * rows and re-sorts the result in memory, so no keyset over `skill` columns can + * name a position inside that merged sequence. */ export const v2ListSkillsContract = defineRouteContract({ method: 'GET', diff --git a/apps/sim/lib/api/contracts/v2/tables.ts b/apps/sim/lib/api/contracts/v2/tables.ts index a84d0306321..37480e4275c 100644 --- a/apps/sim/lib/api/contracts/v2/tables.ts +++ b/apps/sim/lib/api/contracts/v2/tables.ts @@ -51,6 +51,7 @@ import { v2FolderPathSchema, v2FolderSchema, v2ListFoldersQuerySchema, + v2PaginationFields, v2RelocateFolderBodySchema, v2SearchSchema, v2SortFields, @@ -339,31 +340,24 @@ export const v2ListTablesQuerySchema = z .describe('Restrict results to tables in this folder.'), search: v2SearchSchema, ...v2SortFields(v2TableSortFields, { sortBy: 'createdAt', sortOrder: 'asc' }), - /** - * The `.transform()` clamp erases every bound from `z.toJSONSchema`, which - * published a bare `type: number` while the description promised a 1–1000 - * range. `.meta()` restores the JSON Schema keywords without touching the - * runtime pipeline: the published schema states the supported domain, and - * the server stays deliberately lenient about inputs outside it. - */ - limit: z.coerce - .number() - .optional() - .default(100) - .transform((v) => Math.min(Math.max(1, Math.trunc(v)), 1000)) - .describe( - 'Maximum tables to return (1-1000). Fractional or out-of-range values are truncated and clamped into that range rather than rejected.' - ) - .meta({ type: 'integer', minimum: 1, maximum: 1000 }), - cursor: z.string().min(1).optional().describe('Opaque cursor from the previous page.'), + ...v2PaginationFields({ + max: 1000, + fallback: 100, + outOfRange: 'clamp', + description: 'Maximum tables to return per page.', + }), }) .strict() export type V2ListTablesQuery = z.output -export const v2TableWorkspaceQuerySchema = v1ListTablesQuerySchema.extend({ - workspaceId: v1ListTablesQuerySchema.shape.workspaceId.describe('Workspace that owns the table.'), -}) +export const v2TableWorkspaceQuerySchema = v1ListTablesQuerySchema + .extend({ + workspaceId: v1ListTablesQuerySchema.shape.workspaceId.describe( + 'Workspace that owns the table.' + ), + }) + .strict() export type V2TableWorkspaceQuery = z.output const v2TableColumnInputShape = { @@ -671,6 +665,7 @@ export const v2TableRowsQuerySchema = tableRowsQueryBaseSchema .optional() .describe('Opaque cursor returned by the previous page.'), }) + .strict() export type V2TableRowsQuery = z.output /** Cursor-paginated row list. */ diff --git a/apps/sim/lib/api/contracts/v2/workflows.ts b/apps/sim/lib/api/contracts/v2/workflows.ts index 2eb1131dd71..f8dc403eb4d 100644 --- a/apps/sim/lib/api/contracts/v2/workflows.ts +++ b/apps/sim/lib/api/contracts/v2/workflows.ts @@ -25,6 +25,7 @@ import { v2FolderPathSchema, v2FolderSchema, v2ListFoldersQuerySchema, + v2PaginationFields, v2RelocateFolderBodySchema, v2SearchSchema, v2SortFields, @@ -135,17 +136,7 @@ export const v2ListWorkflowsQuerySchema = z .optional() .default(false) .describe('Return only workflows with an active deployment when true.'), - limit: z.coerce - .number() - .min(1) - .max(100) - .optional() - .default(50) - .describe('Maximum workflows to return per page.'), - cursor: z - .string() - .optional() - .describe('Opaque pagination cursor returned by a previous request.'), + ...v2PaginationFields({ description: 'Maximum workflows to return per page.' }), search: v2SearchSchema, ...v2SortFields(v2WorkflowSortFields, { sortBy: 'position', sortOrder: 'asc' }), }) @@ -574,19 +565,9 @@ export type V2WorkflowVersion = z.output */ export const v2ListWorkflowVersionsQuerySchema = z .object({ - limit: z.coerce - .number() - .int() - .min(1) - .max(100) - .optional() - .default(50) - .describe('Maximum deployment versions to return per page.'), - cursor: z - .string() - .optional() - .describe('Opaque pagination cursor returned by a previous request.'), + ...v2PaginationFields({ description: 'Maximum deployment versions to return per page.' }), }) + .strict() .meta({ id: 'ListWorkflowVersionsQuery', title: 'List workflow versions query', @@ -1038,19 +1019,7 @@ export const v2ListWorkflowRunsQuerySchema = z .datetime() .optional() .describe('Include runs started at or before this ISO 8601 timestamp.'), - limit: z.coerce - .number() - .int() - .min(1) - .max(100) - .optional() - .default(50) - .describe('Maximum workflow runs to return per page.'), - cursor: z - .string() - .min(1, 'cursor cannot be empty') - .optional() - .describe('Opaque pagination cursor returned by a previous request.'), + ...v2PaginationFields({ description: 'Maximum workflow runs to return per page.' }), /** * Deliberate deviation from the v2 `sortBy` + `sortOrder` convention. Runs * have exactly one sortable column (start time), so there is no `sortBy` to diff --git a/apps/sim/lib/api/contracts/v2/workspaces.ts b/apps/sim/lib/api/contracts/v2/workspaces.ts index 761c54ddb78..6624ff5b075 100644 --- a/apps/sim/lib/api/contracts/v2/workspaces.ts +++ b/apps/sim/lib/api/contracts/v2/workspaces.ts @@ -4,6 +4,7 @@ import { defineRouteContract } from '@/lib/api/contracts/types' import { v2CursorListResponse, v2DataResponse, + v2PaginationFields, v2TimestampSchema, } from '@/lib/api/contracts/v2/shared' @@ -58,15 +59,7 @@ export type V2WorkspaceMember = z.output export const v2ListWorkspaceMembersQuerySchema = z .object({ - limit: z.coerce - .number() - .int() - .min(1) - .max(100) - .optional() - .default(50) - .describe('Maximum members to return. Defaults to 50 and cannot exceed 100.'), - cursor: z.string().min(1).optional().describe('Opaque cursor returned by the preceding page.'), + ...v2PaginationFields({ description: 'Maximum members to return per page.' }), }) .strict() export type V2ListWorkspaceMembersQuery = z.output diff --git a/apps/sim/lib/api/list-convention.test.ts b/apps/sim/lib/api/list-convention.test.ts index fcb2594dc26..83640d97c87 100644 --- a/apps/sim/lib/api/list-convention.test.ts +++ b/apps/sim/lib/api/list-convention.test.ts @@ -126,7 +126,7 @@ const CASES: ListCase[] = [ sort: { sortBy: 'name', sortOrder: 'asc', - columns: [schemaMock.knowledgeBase.name, schemaMock.knowledgeBase.createdAt], + columns: [schemaMock.knowledgeBase.name, schemaMock.knowledgeBase.id], }, }, { diff --git a/apps/sim/lib/api/list-keyset-paging.test.ts b/apps/sim/lib/api/list-keyset-paging.test.ts new file mode 100644 index 00000000000..b22011f8769 --- /dev/null +++ b/apps/sim/lib/api/list-keyset-paging.test.ts @@ -0,0 +1,173 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + type CursorKey, + encodeKeyset, + type KeysetKey, + type ListSortOrder, +} from '@/lib/api/list-query' +import { cursorSortKey, decodeSortedCursor, encodeSortedCursor } from '@/app/api/v2/lib/response' + +/** + * Page-boundary correctness for the keyset every v2 cursor list shares. + * + * `keysetAfter` renders SQL, so it cannot be evaluated here. What *can* be + * evaluated — and is the part that actually decides whether a page boundary + * loses or repeats a row — is the algebra the SQL implements: rows are ordered + * by the key tuple, a page is the first `limit` of them, and the next page is + * every row whose tuple is strictly greater than the last emitted row's. This + * models exactly that, over the same `KeysetKey.encode` functions the real + * cursors are minted from. + * + * The dataset is deliberately degenerate — repeated names and a timestamp + * shared by four rows — because that is the only place the property can fail. A + * keyset whose trailing key is not unique cannot separate tied rows, so the + * "strictly after" step either skips them or serves them twice. + */ + +interface Row { + id: string + name: string + createdAt: Date +} + +/** A `KeysetKey` reduced to what this simulation needs: how to read its value. */ +type EncodeOnly = Pick, 'encode'> + +const SHARED_INSTANT = new Date('2024-03-01T12:00:00.000Z') + +const ROWS: Row[] = [ + { id: 'a', name: 'alpha', createdAt: SHARED_INSTANT }, + { id: 'b', name: 'alpha', createdAt: SHARED_INSTANT }, + { id: 'c', name: 'alpha', createdAt: SHARED_INSTANT }, + { id: 'd', name: 'beta', createdAt: SHARED_INSTANT }, + { id: 'e', name: 'beta', createdAt: new Date('2024-03-02T09:30:00.000Z') }, + { id: 'f', name: 'gamma', createdAt: new Date('2024-03-03T00:00:00.000Z') }, + { id: 'g', name: 'delta', createdAt: new Date('2024-03-04T18:45:00.000Z') }, +] + +const idKey: EncodeOnly = { encode: (row) => row.id } +const nameKey: EncodeOnly = { encode: (row) => row.name } +const createdAtKey: EncodeOnly = { encode: (row) => row.createdAt.toISOString() } + +/** The trailing `id` is what the production sorts add; the third omits it. */ +const SORTS = { + name: [nameKey, idKey], + createdAt: [createdAtKey, idKey], + /** Deliberately non-total — used only to prove this test can fail. */ + nameWithoutTiebreaker: [nameKey], +} satisfies Record + +function compareKeys(a: CursorKey[], b: CursorKey[]): number { + for (const [i, left] of a.entries()) { + const right = b[i] + if (left < right) return -1 + if (left > right) return 1 + } + return 0 +} + +function ordered(rows: Row[], keys: EncodeOnly[], order: ListSortOrder): Row[] { + const direction = order === 'asc' ? 1 : -1 + return [...rows].sort( + (a, b) => + direction * + compareKeys( + encodeKeyset(keys as KeysetKey[], a), + encodeKeyset(keys as KeysetKey[], b) + ) + ) +} + +/** One page: the first `limit` rows strictly after `cursorKeys` in sort order. */ +function page( + keys: EncodeOnly[], + order: ListSortOrder, + limit: number, + cursorKeys?: CursorKey[] +): { data: Row[]; nextCursorKeys: CursorKey[] | null } { + const direction = order === 'asc' ? 1 : -1 + const remaining = ordered(ROWS, keys, order).filter( + (row) => + !cursorKeys || + direction * compareKeys(encodeKeyset(keys as KeysetKey[], row), cursorKeys) > 0 + ) + const data = remaining.slice(0, limit) + const last = data.at(-1) + return { + data, + nextCursorKeys: + remaining.length > limit && last ? encodeKeyset(keys as KeysetKey[], last) : null, + } +} + +/** Walks every page, capped so a cursor that fails to advance cannot hang the suite. */ +function walk(keys: EncodeOnly[], order: ListSortOrder, limit: number): Row[] { + const seen: Row[] = [] + let cursorKeys: CursorKey[] | undefined + for (let guard = 0; guard <= ROWS.length + 1; guard++) { + const result = page(keys, order, limit, cursorKeys) + seen.push(...result.data) + if (result.nextCursorKeys === null) return seen + cursorKeys = result.nextCursorKeys + } + throw new Error('pagination did not terminate') +} + +describe('v2 keyset page boundaries', () => { + const SORT_CASES = [ + ['name', SORTS.name], + ['createdAt', SORTS.createdAt], + ] as const + const ORDERS: ListSortOrder[] = ['asc', 'desc'] + + for (const [sortName, keys] of SORT_CASES) { + for (const order of ORDERS) { + for (const limit of [1, 2, 3, ROWS.length]) { + it(`emits every row exactly once paging ${sortName} ${order} at limit ${limit}`, () => { + const seen = walk(keys, order, limit) + + expect(seen).toHaveLength(ROWS.length) + expect(new Set(seen.map((row) => row.id)).size).toBe(ROWS.length) + expect(seen.map((row) => row.id)).toEqual(ordered(ROWS, keys, order).map((row) => row.id)) + }) + } + } + } + + it('reports a null nextCursor on the final page and not before', () => { + expect(page(SORTS.name, 'asc', 3).nextCursorKeys).not.toBeNull() + + const lastPage = page(SORTS.name, 'asc', 3, page(SORTS.name, 'asc', 3).nextCursorKeys ?? []) + expect(lastPage.nextCursorKeys).not.toBeNull() + + expect(page(SORTS.name, 'asc', ROWS.length).nextCursorKeys).toBeNull() + }) + + /** + * Proves the harness above is capable of failing. Without a unique trailing + * key the three rows sharing `name: 'alpha'` cannot be separated, so resuming + * "strictly after alpha" drops the other two. + */ + it('loses rows when the keyset has no unique trailing key', () => { + const seen = walk(SORTS.nameWithoutTiebreaker, 'asc', 1) + + expect(seen.length).toBeLessThan(ROWS.length) + }) + + it('round-trips a cursor only under the sort that minted it', () => { + const keys = encodeKeyset(SORTS.name as KeysetKey[], ROWS[0]) + const cursor = encodeSortedCursor(cursorSortKey('name', 'asc'), keys) + + expect(decodeSortedCursor(cursor, cursorSortKey('name', 'asc'))).toEqual({ + status: 'ok', + keys, + }) + expect(decodeSortedCursor(cursor, cursorSortKey('name', 'desc'))).toEqual({ status: 'invalid' }) + expect(decodeSortedCursor(cursor, cursorSortKey('createdAt', 'asc'))).toEqual({ + status: 'invalid', + }) + }) +}) diff --git a/apps/sim/lib/api/list-query.ts b/apps/sim/lib/api/list-query.ts index 6607ecde36a..d45d28dacf0 100644 --- a/apps/sim/lib/api/list-query.ts +++ b/apps/sim/lib/api/list-query.ts @@ -12,6 +12,7 @@ import { type SQLWrapper, sql, } from 'drizzle-orm' +import { OrchestrationError } from '@/lib/core/orchestration/types' export const LIST_SORT_ORDERS = ['asc', 'desc'] as const export type ListSortOrder = (typeof LIST_SORT_ORDERS)[number] @@ -147,6 +148,56 @@ export function encodeKeyset(keys: readonly KeysetKey[], row: Row): Cu return keys.map((key) => key.encode(row)) } +/** + * The `WHERE` fragment that resumes a page, or `undefined` for page one. + * + * Wraps {@link keysetAfter}'s `null` — a cursor that does not fit this sort — in + * the canonical validation failure, so every keyset list renders a bad cursor as + * the same 400 instead of each deciding for itself. A cursor is caller-supplied, + * so the one outcome that must never happen is it reaching the query and + * surfacing as a 500. + */ +export function resumeKeyset( + keys: readonly KeysetKey[], + cursorKeys: CursorKey[] | undefined, + order: ListSortOrder +): SQL | undefined { + if (!cursorKeys) return undefined + const after = keysetAfter(keys, cursorKeys, order) + if (after === null) throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE) + return after +} + +/** One page of a keyset list, plus the keys that resume it. */ +export interface KeysetPage { + data: Row[] + nextCursorKeys: CursorKey[] | null +} + +/** + * Cuts an over-fetched row set down to the requested page. + * + * Pair with `.limit(limit + 1)`: the extra row is how "is there a next page" + * is answered without a second count query. Writing the `rows.length > limit` + * comparison out per resource is how an off-by-one becomes a page that repeats + * its last row, so it lives here once. + * + * `limit` is optional because several of these readers are also called by + * unpaged internal surfaces that want the whole set; an absent `limit` means no + * `LIMIT` clause was applied, and such a read can never carry a cursor. + */ +export function keysetPage( + keys: readonly KeysetKey[], + rows: Row[], + limit: number | undefined +): KeysetPage { + if (limit === undefined) return { data: rows, nextCursorKeys: null } + const hasMore = rows.length > limit + const data = rows.slice(0, limit) + const last = data.at(-1) + return { data, nextCursorKeys: hasMore && last ? encodeKeyset(keys, last) : null } +} + /** * The `WHERE` half of the keyset: strictly after `values` in the requested * direction, expanded lexicographically so ties on a leading key fall through diff --git a/apps/sim/lib/api/offset-cursor-scope.test.ts b/apps/sim/lib/api/offset-cursor-scope.test.ts new file mode 100644 index 00000000000..b81cfb1dffe --- /dev/null +++ b/apps/sim/lib/api/offset-cursor-scope.test.ts @@ -0,0 +1,80 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + decodeOffsetCursor, + encodeOffsetCursor, + offsetCursorScope, +} from '@/app/api/v2/lib/response' + +/** + * An offset names a position in one exact sequence. Replay it against a + * differently filtered or sorted sequence and it names a different row — + * silently skipping rows, repeating them, or landing past the end and returning + * an empty page while `nextCursor` implied more. + * + * The keyset cursor has always been protected from this by its sort stamp + * (`decodeSortedCursor`). These assertions hold the offset cursor — used by + * `GET /skills` and `GET /knowledge/{id}/documents` — to the same rule. + */ +describe('offset cursor scope', () => { + const base = { workspaceId: 'ws-1', search: undefined, sortBy: 'name', sortOrder: 'asc' } + + it('resumes a cursor replayed under the same query state', () => { + const scope = offsetCursorScope(base) + expect(decodeOffsetCursor(encodeOffsetCursor(scope, 40), scope)).toBe(40) + }) + + it('rejects a cursor replayed under a different sort', () => { + const cursor = encodeOffsetCursor(offsetCursorScope(base), 40) + + expect(() => + decodeOffsetCursor(cursor, offsetCursorScope({ ...base, sortBy: 'createdAt' })) + ).toThrow(/does not match the requested/) + expect(() => + decodeOffsetCursor(cursor, offsetCursorScope({ ...base, sortOrder: 'desc' })) + ).toThrow(/does not match the requested/) + }) + + it('rejects a cursor replayed under a different filter', () => { + const cursor = encodeOffsetCursor(offsetCursorScope(base), 40) + + expect(() => + decodeOffsetCursor(cursor, offsetCursorScope({ ...base, search: 'deploy' })) + ).toThrow(/does not match the requested/) + expect(() => + decodeOffsetCursor(cursor, offsetCursorScope({ ...base, workspaceId: 'ws-2' })) + ).toThrow(/does not match the requested/) + }) + + it('treats an absent cursor as page one', () => { + expect(decodeOffsetCursor(undefined, offsetCursorScope(base))).toBe(0) + }) + + it('rejects a cursor that is not valid base64-JSON', () => { + expect(() => decodeOffsetCursor('not-a-cursor', offsetCursorScope(base))).toThrow() + }) + + it('rejects an offset that is not a non-negative integer', () => { + const scope = offsetCursorScope(base) + expect(() => decodeOffsetCursor(encodeOffsetCursor(scope, -1), scope)).toThrow('Invalid cursor') + expect(() => decodeOffsetCursor(encodeOffsetCursor(scope, 1.5), scope)).toThrow( + 'Invalid cursor' + ) + }) + + /** + * `limit` selects how much of the sequence to return, not what the sequence + * is, so paging with a different page size must keep working. + */ + it('is unaffected by the page size', () => { + expect(offsetCursorScope({ ...base, sortBy: 'name' })).toBe(offsetCursorScope(base)) + }) + + it('does not depend on the order the parts are written', () => { + expect(offsetCursorScope({ sortBy: 'name', workspaceId: 'ws-1', sortOrder: 'asc' })).toBe( + offsetCursorScope({ sortOrder: 'asc', workspaceId: 'ws-1', sortBy: 'name' }) + ) + }) +}) diff --git a/apps/sim/lib/api/server/routes/index.ts b/apps/sim/lib/api/server/routes/index.ts index 9c72405f0b9..904f8cd1731 100644 --- a/apps/sim/lib/api/server/routes/index.ts +++ b/apps/sim/lib/api/server/routes/index.ts @@ -23,6 +23,7 @@ export { admitOptionalV2Request, admitV2Request, defineV2JsonRoute, + V2_PARSE_DEFAULTS, type V2ErrorPolicy, V2RouteInfrastructureError, v2ApiKeyAuth, diff --git a/apps/sim/lib/api/server/routes/v2-binary-route.ts b/apps/sim/lib/api/server/routes/v2-binary-route.ts index 67cc35af1f6..1802816a19a 100644 --- a/apps/sim/lib/api/server/routes/v2-binary-route.ts +++ b/apps/sim/lib/api/server/routes/v2-binary-route.ts @@ -8,11 +8,11 @@ import type { } from '@/lib/api/server/routes/types' import { admitV2Request, + V2_PARSE_DEFAULTS, type V2ErrorPolicy, type V2RateLimitPolicy, V2RouteInfrastructureError, type v2ApiKeyAuth, - v2PayloadTooLargeResponse, } from '@/lib/api/server/routes/v2-json-route' import { parseRequest } from '@/lib/api/server/validation' import type { ApplicationOperation } from '@/lib/core/application' @@ -59,7 +59,7 @@ export function defineV2BinaryRoute< if (!admission.success) return admission.response const parsed = await parseRequest(options.contract, request, context ?? {}, { - payloadTooLargeResponse: v2PayloadTooLargeResponse, + ...V2_PARSE_DEFAULTS, validationErrorResponse: v2ValidationError, }) if (!parsed.success) return parsed.response diff --git a/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts b/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts index a6c415ec493..79767ef4641 100644 --- a/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts +++ b/apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts @@ -9,11 +9,11 @@ import type { } from '@/lib/api/server/routes/types' import { admitV2Request, + V2_PARSE_DEFAULTS, type V2ErrorPolicy, type V2RateLimitPolicy, V2RouteInfrastructureError, type v2ApiKeyAuth, - v2PayloadTooLargeResponse, } from '@/lib/api/server/routes/v2-json-route' import { type ParsedRequest, @@ -129,7 +129,7 @@ export function defineV2BodyLifecycleRoute< if (!routeAdmission.success) return routeAdmission.response const parsed = await parseRequest(options.contract, request, context ?? {}, { - payloadTooLargeResponse: v2PayloadTooLargeResponse, + ...V2_PARSE_DEFAULTS, ...options.parseOptions, validationErrorResponse: v2ValidationError, }) diff --git a/apps/sim/lib/api/server/routes/v2-error-envelope.test.ts b/apps/sim/lib/api/server/routes/v2-error-envelope.test.ts new file mode 100644 index 00000000000..ceab810f8b5 --- /dev/null +++ b/apps/sim/lib/api/server/routes/v2-error-envelope.test.ts @@ -0,0 +1,38 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { v2InvalidJsonResponse } from '@/lib/api/server/routes/v2-json-route' + +/** + * v2 promises exactly one failure shape — `{ error: { code, message } }` — and a + * client written against it reads `error.code`. That promise held for validation + * errors and broke for transport-level ones: a malformed or absent JSON body + * fell through to `parseRequest`'s framework default, a bare + * `{ "error": "Request body must be valid JSON" }` string, so `error.code` was + * `undefined` exactly when the request was malformed. + * + * The fix made the envelope a builder default rather than a per-route opt-in + * that only 8 of 77 routes remembered, so this asserts the default itself. + */ +describe('v2 malformed-body envelope', () => { + it('renders the canonical error envelope, not a bare string', async () => { + const response = v2InvalidJsonResponse() + + expect(response.status).toBe(400) + expect(response.headers.get('content-type')).toContain('application/json') + expect(await response.json()).toEqual({ + error: { code: 'BAD_REQUEST', message: 'Request body must be valid JSON' }, + }) + }) + + it('keeps the body out of shared caches like every other v2 response', () => { + expect(v2InvalidJsonResponse().headers.get('Cache-Control')).toBe('private, no-store') + }) + + it('exposes a machine-readable code, which the bare-string form could not', async () => { + const body = (await v2InvalidJsonResponse().json()) as { error: { code?: string } } + expect(typeof body.error).toBe('object') + expect(body.error.code).toBe('BAD_REQUEST') + }) +}) diff --git a/apps/sim/lib/api/server/routes/v2-json-route.ts b/apps/sim/lib/api/server/routes/v2-json-route.ts index 20261375568..960aa46415c 100644 --- a/apps/sim/lib/api/server/routes/v2-json-route.ts +++ b/apps/sim/lib/api/server/routes/v2-json-route.ts @@ -111,6 +111,33 @@ export const v2RateLimits = { export const v2PayloadTooLargeResponse = () => v2Error('PAYLOAD_TOO_LARGE', 'Request body is too large') +/** + * Default `400` for a body that is absent or not valid JSON, for the same + * reason as {@link v2PayloadTooLargeResponse}: `parseRequest`'s fallback is a + * bare `{ "error": "Request body must be valid JSON" }` carrying no + * `error.code`, so a client reading `error.code` off every other v2 failure + * gets `undefined` exactly when its request was malformed. + * + * It is a default rather than a per-route opt-in because the opt-in *was* the + * bug: only 8 of the 77 v2 routes remembered to pass it, so the envelope held + * for validation errors and broke for transport-level ones. A route supplying + * its own `invalidJsonResponse` still wins. + */ +export const v2InvalidJsonResponse = () => v2Error('BAD_REQUEST', 'Request body must be valid JSON') + +/** + * The parse failures every v2 route renders the same way. + * + * The builders spread this, and so must the handful of raw `withRouteHandler` + * v2 routes that call `parseRequest` directly — they are exactly the routes a + * builder default cannot reach, and leaving them out is what kept the bare + * `{ "error": string }` body alive on two of the busiest v2 POSTs. + */ +export const V2_PARSE_DEFAULTS = { + payloadTooLargeResponse: v2PayloadTooLargeResponse, + invalidJsonResponse: v2InvalidJsonResponse, +} as const + export interface V2ErrorPolicy { render(error: unknown): NextResponse | null } @@ -250,7 +277,7 @@ export function defineV2JsonRoute< } const parsed = await parseRequest(options.contract, request, context ?? {}, { - payloadTooLargeResponse: v2PayloadTooLargeResponse, + ...V2_PARSE_DEFAULTS, ...options.parseOptions, validationErrorResponse: v2ValidationError, }) diff --git a/apps/sim/lib/credentials/application/list-workspace-credentials.ts b/apps/sim/lib/credentials/application/list-workspace-credentials.ts index 2f92c8f1a86..45f2eee5c67 100644 --- a/apps/sim/lib/credentials/application/list-workspace-credentials.ts +++ b/apps/sim/lib/credentials/application/list-workspace-credentials.ts @@ -1,3 +1,4 @@ +import type { CursorKey, ListSortOrder } from '@/lib/api/list-query' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { credentialOperations } from '@/lib/credentials/application/operations' @@ -15,11 +16,20 @@ export interface ListWorkspaceCredentialsInput { providerId?: string search?: string sortBy: 'displayName' | 'createdAt' | 'updatedAt' - sortOrder: 'asc' | 'desc' + sortOrder: ListSortOrder + limit: number + cursorKeys?: CursorKey[] } export interface ListWorkspaceCredentialsResult { credentials: VisibleWorkspaceCredential[] + nextCursorKeys: CursorKey[] | null + /** + * Echoed back because the route's presenter receives only this result, and the + * cursor it hands out has to be stamped with the sort that produced it. + */ + sortBy: ListWorkspaceCredentialsInput['sortBy'] + sortOrder: ListSortOrder } export const listWorkspaceCredentials = defineAuthorizedWorkspaceUseCase({ @@ -34,34 +44,36 @@ export const listWorkspaceCredentials = defineAuthorizedWorkspaceUseCase({ const types: Array<'oauth' | 'service_account'> = input.type ? [input.type] : ['oauth', 'service_account'] + const sort = { sortBy: input.sortBy, sortOrder: input.sortOrder } + if (principal.kind === 'workspace_api_key') { - return { - credentials: await listWorkspacePrincipalCredentials({ - workspaceId: context.workspaceId, - types, - providerId: input.providerId, - search: input.search, - sortBy: input.sortBy, - sortOrder: input.sortOrder, - }), - } + const page = await listWorkspacePrincipalCredentials({ + workspaceId: context.workspaceId, + types, + providerId: input.providerId, + search: input.search, + ...sort, + limit: input.limit, + cursorKeys: input.cursorKeys, + }) + return { credentials: page.data, nextCursorKeys: page.nextCursorKeys, ...sort } } const workspaceAccess = await checkWorkspaceAccess(context.workspaceId, principal.userId) if (!workspaceAccess.hasAccess) { throw new OrchestrationError('forbidden', 'Access denied') } - return { - credentials: await listVisibleWorkspaceCredentials({ - workspaceId: context.workspaceId, - userId: principal.userId, - workspaceAccess, - types, - providerId: input.providerId, - search: input.search, - sortBy: input.sortBy, - sortOrder: input.sortOrder, - }), - } + const page = await listVisibleWorkspaceCredentials({ + workspaceId: context.workspaceId, + userId: principal.userId, + workspaceAccess, + types, + providerId: input.providerId, + search: input.search, + ...sort, + limit: input.limit, + cursorKeys: input.cursorKeys, + }) + return { credentials: page.data, nextCursorKeys: page.nextCursorKeys, ...sort } }, }) diff --git a/apps/sim/lib/credentials/queries.test.ts b/apps/sim/lib/credentials/queries.test.ts index d4391d06206..cc5dfb3a368 100644 --- a/apps/sim/lib/credentials/queries.test.ts +++ b/apps/sim/lib/credentials/queries.test.ts @@ -11,7 +11,7 @@ describe('listWorkspacePrincipalCredentials', () => { }) it('selects and returns only connection metadata', async () => { - dbChainMockFns.orderBy.mockResolvedValueOnce([ + dbChainMockFns.limit.mockResolvedValueOnce([ { id: 'credential-1', workspaceId: 'workspace-1', @@ -30,9 +30,11 @@ describe('listWorkspacePrincipalCredentials', () => { const result = await listWorkspacePrincipalCredentials({ workspaceId: 'workspace-1', types: ['oauth', 'service_account'], + limit: 50, }) - expect(result).toEqual([ + expect(result.nextCursorKeys).toBeNull() + expect(result.data).toEqual([ { id: 'credential-1', workspaceId: 'workspace-1', @@ -57,19 +59,20 @@ describe('listWorkspacePrincipalCredentials', () => { it('fails fast on an empty connection-type policy', async () => { await expect( - listWorkspacePrincipalCredentials({ workspaceId: 'workspace-1', types: [] }) + listWorkspacePrincipalCredentials({ workspaceId: 'workspace-1', types: [], limit: 50 }) ).rejects.toThrow('Workspace credential types cannot be empty') expect(dbChainMockFns.select).not.toHaveBeenCalled() }) it('propagates database failures', async () => { const failure = new Error('database unavailable') - dbChainMockFns.orderBy.mockRejectedValueOnce(failure) + dbChainMockFns.limit.mockRejectedValueOnce(failure) await expect( listWorkspacePrincipalCredentials({ workspaceId: 'workspace-1', types: ['oauth', 'service_account'], + limit: 50, }) ).rejects.toBe(failure) }) diff --git a/apps/sim/lib/credentials/queries.ts b/apps/sim/lib/credentials/queries.ts index 9724fadd410..bacb9d2cda7 100644 --- a/apps/sim/lib/credentials/queries.ts +++ b/apps/sim/lib/credentials/queries.ts @@ -1,9 +1,20 @@ import { db } from '@sim/db' import { credential, credentialMember } from '@sim/db/schema' -import { and, type Column, eq, inArray, isNotNull, or, sql } from 'drizzle-orm' +import { and, eq, inArray, isNotNull, or, sql } from 'drizzle-orm' import type { V2CredentialSortBy } from '@/lib/api/contracts/v2/credentials' -import type { ListSortOrder } from '@/lib/api/list-query' -import { listOrderBy, searchFilter } from '@/lib/api/list-query' +import { + type CursorKey, + type KeysetKey, + type KeysetPage, + keysetColumns, + keysetPage, + type ListSortOrder, + listOrderBy, + resumeKeyset, + searchFilter, + textKey, + timestampKey, +} from '@/lib/api/list-query' import { isSharedCredentialType, SHARED_CREDENTIAL_TYPES } from '@/lib/credentials/access' import type { WorkspaceAccess } from '@/lib/workspaces/permissions/utils' @@ -31,6 +42,38 @@ export interface VisibleWorkspaceCredential { role: 'admin' | 'member' } +const credentialIdKey = textKey(credential.id, (row) => row.id) + +/** + * Keyset orderings for the public list's sortable fields, made total over the + * contract enum by `satisfies`. Each ends in `id` so credentials sharing a + * display name or a timestamp still come back in a stable order — which is also + * what makes the cursor resumable, since a non-unique final key can repeat or + * skip a row at a page boundary. + * + * The keys encode from {@link VisibleWorkspaceCredential} — the mapped row, not + * the raw select — because both readers below derive their projection before + * the page is cut, so the cursor is always stamped from the row the caller + * actually received. + */ +const CREDENTIAL_SORTS = { + displayName: [ + textKey(credential.displayName, (row) => row.displayName), + credentialIdKey, + ], + createdAt: [ + timestampKey(credential.createdAt, (row) => row.createdAt), + credentialIdKey, + ], + updatedAt: [ + timestampKey(credential.updatedAt, (row) => row.updatedAt), + credentialIdKey, + ], +} satisfies Record[]> + +/** One page of workspace credentials plus the keys that resume it. */ +export type WorkspaceCredentialPage = KeysetPage + /** * The credentials a user may see in a workspace. * @@ -38,17 +81,6 @@ export interface VisibleWorkspaceCredential { * admins — every shared-type credential, plus the caller's own personal env * credentials. Encrypted secret material is never selected. */ -/** - * Orderings for the public list's sortable fields, made total over the contract - * enum by `satisfies`. Each ends in `id` so credentials sharing a display name - * or a timestamp still come back in a stable order. - */ -const CREDENTIAL_SORTS = { - displayName: [credential.displayName, credential.id], - createdAt: [credential.createdAt, credential.id], - updatedAt: [credential.updatedAt, credential.id], -} satisfies Record - export async function listVisibleWorkspaceCredentials(params: { workspaceId: string userId: string @@ -59,7 +91,22 @@ export async function listVisibleWorkspaceCredentials(params: { search?: string sortBy?: V2CredentialSortBy sortOrder?: ListSortOrder -}): Promise { + /** Page size. Omitted by the unpaged session surface — see {@link keysetPage}. */ + limit?: number + cursorKeys?: CursorKey[] + /** + * Restricts personal env credentials to the caller's own, in SQL. + * + * The secrets surface used to apply this as a JS filter over the query's + * result. That is invisible on an unpaged read but a correctness bug once the + * query is paged: a page of `limit` rows trimmed afterwards hands the caller + * fewer rows than it asked for while `nextCursor` still claims more. It is a + * pure page-size fix — the predicate drops exactly the rows the JS filter + * dropped (another user's `env_personal` row, reachable only via an explicit + * `credential_member` grant), and never widens what a caller can see. + */ + ownedEnvSecretsOnly?: boolean +}): Promise { const { workspaceId, userId, @@ -69,11 +116,21 @@ export async function listVisibleWorkspaceCredentials(params: { search, sortBy = 'createdAt', sortOrder = 'desc', + limit, } = params const whereClauses = [eq(credential.workspaceId, workspaceId)] if (types?.length) whereClauses.push(inArray(credential.type, types)) if (providerId) whereClauses.push(eq(credential.providerId, providerId)) + const ownedEnvSecretsClause = params.ownedEnvSecretsOnly + ? or( + eq(credential.type, 'env_workspace'), + and(eq(credential.type, 'env_personal'), eq(credential.envOwnerUserId, userId)) + ) + : undefined + + const keys = CREDENTIAL_SORTS[sortBy] + const after = resumeKeyset(keys, params.cursorKeys, sortOrder) const isWorkspaceAdmin = workspaceAccess.canAdmin const accessClause = isWorkspaceAdmin @@ -84,7 +141,7 @@ export async function listVisibleWorkspaceCredentials(params: { ) : or(isNotNull(credentialMember.id), eq(credential.envOwnerUserId, userId)) - const rows = await db + const query = db .select({ id: credential.id, workspaceId: credential.workspaceId, @@ -110,20 +167,34 @@ export async function listVisibleWorkspaceCredentials(params: { eq(credentialMember.status, 'active') ) ) - .where(and(...whereClauses, accessClause, searchFilter(credential.displayName, search))) - .orderBy(...listOrderBy(CREDENTIAL_SORTS[sortBy], sortOrder)) + .where( + and( + ...whereClauses, + accessClause, + ownedEnvSecretsClause, + searchFilter(credential.displayName, search), + after + ) + ) + .orderBy(...listOrderBy(keysetColumns(keys), sortOrder)) - return rows.map(({ memberRole, encryptedServiceAccountKey, ...rest }) => ({ + const rows = await (limit === undefined ? query : query.limit(limit + 1)) + + const mapped = rows.map(({ memberRole, encryptedServiceAccountKey, ...rest }) => ({ ...rest, hasServiceAccountKey: Boolean(encryptedServiceAccountKey), + /** + * An `env_personal` credential's own env owner administers it regardless of + * workspace role — otherwise the owner of a personal secret can't manage it. + */ role: - // An `env_personal` credential's own env owner administers it regardless of - // workspace role — otherwise the owner of a personal secret can't manage it. (rest.type === 'env_personal' && rest.envOwnerUserId === userId) || (isWorkspaceAdmin && isSharedCredentialType(rest.type)) - ? 'admin' - : (memberRole ?? 'member'), + ? ('admin' as const) + : (memberRole ?? ('member' as const)), })) + + return keysetPage(keys, mapped, limit) } /** @@ -141,7 +212,9 @@ export async function listWorkspacePrincipalCredentials(params: { search?: string sortBy?: V2CredentialSortBy sortOrder?: ListSortOrder -}): Promise { + limit: number + cursorKeys?: CursorKey[] +}): Promise { const { workspaceId, types, @@ -149,13 +222,17 @@ export async function listWorkspacePrincipalCredentials(params: { search, sortBy = 'createdAt', sortOrder = 'desc', + limit, } = params if (types.length === 0) throw new Error('Workspace credential types cannot be empty') const whereClauses = [eq(credential.workspaceId, workspaceId), inArray(credential.type, types)] if (providerId) whereClauses.push(eq(credential.providerId, providerId)) - const rows = await db + const keys = CREDENTIAL_SORTS[sortBy] + const after = resumeKeyset(keys, params.cursorKeys, sortOrder) + + const query = db .select({ id: credential.id, workspaceId: credential.workspaceId, @@ -170,15 +247,19 @@ export async function listWorkspacePrincipalCredentials(params: { hasServiceAccountKey: sql`${credential.encryptedServiceAccountKey} IS NOT NULL`, }) .from(credential) - .where(and(...whereClauses, searchFilter(credential.displayName, search))) - .orderBy(...listOrderBy(CREDENTIAL_SORTS[sortBy], sortOrder)) + .where(and(...whereClauses, searchFilter(credential.displayName, search), after)) + .orderBy(...listOrderBy(keysetColumns(keys), sortOrder)) - return rows.map((row) => ({ + const rows = await query.limit(limit + 1) + + const mapped = rows.map((row) => ({ ...row, envKey: null, envOwnerUserId: null, - role: 'member', + role: 'member' as const, })) + + return keysetPage(keys, mapped, limit) } /** diff --git a/apps/sim/lib/custom-tools/application/use-cases.ts b/apps/sim/lib/custom-tools/application/use-cases.ts index 853e7662b4b..dd0fb4e44de 100644 --- a/apps/sim/lib/custom-tools/application/use-cases.ts +++ b/apps/sim/lib/custom-tools/application/use-cases.ts @@ -6,7 +6,7 @@ import { } from '@sim/auth/principal' import type { customTools } from '@sim/db/schema' import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors' -import type { ListSortOrder } from '@/lib/api/list-query' +import type { CursorKey, ListSortOrder } from '@/lib/api/list-query' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { customToolDelegationPolicy } from '@/lib/custom-tools/application/authorization' @@ -92,6 +92,8 @@ export interface ListWorkspaceCustomToolsInput { search?: string sortBy?: CustomToolSortBy sortOrder?: ListSortOrder + limit: number + cursorKeys?: CursorKey[] } export const listWorkspaceCustomToolsUseCase = defineAuthorizedWorkspaceUseCase({ @@ -100,8 +102,13 @@ export const listWorkspaceCustomToolsUseCase = defineAuthorizedWorkspaceUseCase( resolveWorkspaceContext(input.workspaceId), authorizationOptions, async execute({ input, context }) { - const tools = await listWorkspaceCustomTools({ ...input, workspaceId: context.workspaceId }) - return { tools } + const page = await listWorkspaceCustomTools({ ...input, workspaceId: context.workspaceId }) + return { + tools: page.data, + nextCursorKeys: page.nextCursorKeys, + sortBy: input.sortBy ?? 'createdAt', + sortOrder: input.sortOrder ?? 'desc', + } }, }) diff --git a/apps/sim/lib/folders/application-folder-caps.test.ts b/apps/sim/lib/folders/application-folder-caps.test.ts index 7a6d2524f25..08c63d8798a 100644 --- a/apps/sim/lib/folders/application-folder-caps.test.ts +++ b/apps/sim/lib/folders/application-folder-caps.test.ts @@ -27,7 +27,6 @@ vi.mock('@/lib/workflows/application/context', () => ({ resolveActiveWorkspaceApplicationContext: mocks.resolveWorkflowWorkspace, })) vi.mock('@/lib/workflows/queries', () => ({ - InvalidWorkflowListCursorError: class InvalidWorkflowListCursorError extends Error {}, listWorkspaceWorkflows: mocks.listWorkflows, })) vi.mock('@/lib/table/application/context', () => ({ diff --git a/apps/sim/lib/knowledge/application/documents.ts b/apps/sim/lib/knowledge/application/documents.ts index 30b9445fbd3..11b1add79b2 100644 --- a/apps/sim/lib/knowledge/application/documents.ts +++ b/apps/sim/lib/knowledge/application/documents.ts @@ -74,6 +74,12 @@ export interface ListKnowledgeDocumentsInput { sortBy?: DocumentSortField sortOrder?: SortOrder tagFilters?: TagFilterCondition[] + /** + * The query state `offset` counts positions within, echoed back so a surface + * presenter can stamp the next cursor with it. Surface-only; the read itself + * does not use it. + */ + cursorScope?: string } export interface ReadKnowledgeDocumentInput { @@ -219,7 +225,7 @@ export const listKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({ }, generateRequestId() ) - return { ...result, workspaceId: context.workspaceId } + return { ...result, workspaceId: context.workspaceId, cursorScope: input.cursorScope } }, }) diff --git a/apps/sim/lib/knowledge/application/knowledge-bases.test.ts b/apps/sim/lib/knowledge/application/knowledge-bases.test.ts index c3318e7daa1..f3328157bae 100644 --- a/apps/sim/lib/knowledge/application/knowledge-bases.test.ts +++ b/apps/sim/lib/knowledge/application/knowledge-bases.test.ts @@ -143,7 +143,7 @@ describe('knowledge base application use cases', () => { }) mocks.loadFolderIndex.mockResolvedValue({ pathById: new Map() }) mocks.createRecord.mockResolvedValue(knowledgeBase) - mocks.listRecords.mockResolvedValue([]) + mocks.listRecords.mockResolvedValue({ data: [], nextCursorKeys: null }) mocks.listInternalRecords.mockResolvedValue([knowledgeBase]) mocks.getRecord.mockResolvedValue(knowledgeBase) mocks.getRestorableRecord.mockResolvedValue(knowledgeBase) @@ -188,7 +188,7 @@ describe('knowledge base application use cases', () => { }) it('loads the active knowledge catalog and tag metadata only after workspace authorization', async () => { - mocks.listRecords.mockResolvedValueOnce([knowledgeBase]) + mocks.listRecords.mockResolvedValueOnce({ data: [knowledgeBase], nextCursorKeys: null }) dbChainMockFns.orderBy.mockResolvedValueOnce([ { knowledgeBaseId: 'knowledge-1', diff --git a/apps/sim/lib/knowledge/application/knowledge-bases.ts b/apps/sim/lib/knowledge/application/knowledge-bases.ts index 44180f1a1b7..9fd5ba27b57 100644 --- a/apps/sim/lib/knowledge/application/knowledge-bases.ts +++ b/apps/sim/lib/knowledge/application/knowledge-bases.ts @@ -4,6 +4,7 @@ import { db } from '@sim/db' import { knowledgeBaseTagDefinitions } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { inArray } from 'drizzle-orm' +import type { CursorKey } from '@/lib/api/list-query' import { authorizeWorkspaceOperation, type OperationUseCase, @@ -69,6 +70,12 @@ export interface ListKnowledgeBasesInput { search?: string sortBy?: 'name' | 'createdAt' | 'updatedAt' sortOrder?: 'asc' | 'desc' + /** + * Page size for the public list. Omitted reads the whole workspace set, which + * is what the internal catalog caller still wants. + */ + limit?: number + cursorKeys?: CursorKey[] } export interface KnowledgeBaseResult { @@ -78,6 +85,9 @@ export interface KnowledgeBaseResult { export interface ListKnowledgeBasesResult { knowledgeBases: KnowledgeBaseResult[] + nextCursorKeys: CursorKey[] | null + sortBy: 'name' | 'createdAt' | 'updatedAt' + sortOrder: 'asc' | 'desc' } export interface ListArchivedKnowledgeBasesResult { @@ -218,29 +228,37 @@ async function executeListKnowledgeBases(args: { input: ListKnowledgeBasesInput context: KnowledgeWorkspaceContext }): Promise { - const index = await loadActiveFolderPathIndex( - args.context.workspaceId, - 'knowledge_base', - undefined, - { maxRows: MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACE } - ) - const folderId = + /** + * The folder index renders each row's `folderPath` and the folder filter + * resolves the caller's `folderPath` to an id. Neither reads the other, so + * they run together rather than adding a serial round-trip to a list route. + */ + const [index, folderId] = await Promise.all([ + loadActiveFolderPathIndex(args.context.workspaceId, 'knowledge_base', undefined, { + maxRows: MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACE, + }), args.input.folderPath === undefined ? undefined - : await resolveKnowledgeFolderPath(args.context.workspaceId, args.input.folderPath).then( + : resolveKnowledgeFolderPath(args.context.workspaceId, args.input.folderPath).then( (resolved) => resolved.folderId - ) - const rows = await getWorkspaceKnowledgeBases(args.context.workspaceId, 'active', { + ), + ]) + const page = await getWorkspaceKnowledgeBases(args.context.workspaceId, 'active', { folderId, search: args.input.search, sortBy: args.input.sortBy, sortOrder: args.input.sortOrder, + limit: args.input.limit, + cursorKeys: args.input.cursorKeys, }) return { - knowledgeBases: rows.map((knowledgeBase) => ({ + knowledgeBases: page.data.map((knowledgeBase) => ({ knowledgeBase, folderPath: knowledgeFolderPathForId(index, knowledgeBase.folderId), })), + nextCursorKeys: page.nextCursorKeys, + sortBy: args.input.sortBy ?? 'createdAt', + sortOrder: args.input.sortOrder ?? 'asc', } } @@ -397,7 +415,7 @@ export const listArchivedKnowledgeBases = defineAuthorizedKnowledgeUseCase({ resolveKnowledgeWorkspaceContext(input), async execute({ context }): Promise { return { - knowledgeBases: await getWorkspaceKnowledgeBases(context.workspaceId, 'archived'), + knowledgeBases: (await getWorkspaceKnowledgeBases(context.workspaceId, 'archived')).data, } }, }) diff --git a/apps/sim/lib/knowledge/application/knowledge-vfs.ts b/apps/sim/lib/knowledge/application/knowledge-vfs.ts index da538dae88e..f7fbff3e8d7 100644 --- a/apps/sim/lib/knowledge/application/knowledge-vfs.ts +++ b/apps/sim/lib/knowledge/application/knowledge-vfs.ts @@ -29,7 +29,7 @@ async function resolveKnowledgeBaseByVfsName( context: KnowledgeWorkspaceContext, sourceName: string ): Promise { - const rows = await getWorkspaceKnowledgeBases(context.workspaceId, 'active', { + const { data: rows } = await getWorkspaceKnowledgeBases(context.workspaceId, 'active', { search: sourceName, }) const matches = rows.filter((row) => row.name === sourceName) diff --git a/apps/sim/lib/knowledge/service.ts b/apps/sim/lib/knowledge/service.ts index 1ab816f87ae..d3b9ffa5e37 100644 --- a/apps/sim/lib/knowledge/service.ts +++ b/apps/sim/lib/knowledge/service.ts @@ -10,22 +10,18 @@ import { import { createLogger } from '@sim/logger' import { getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { - and, - type Column, - count, - eq, - exists, - inArray, - isNotNull, - isNull, - ne, - or, - sql, -} from 'drizzle-orm' +import { and, count, eq, exists, inArray, isNotNull, isNull, ne, or, sql } from 'drizzle-orm' import type { V2KnowledgeBaseSortBy } from '@/lib/api/contracts/v2/knowledge' -import type { ListSortOrder } from '@/lib/api/list-query' -import { listOrderBy, searchFilter } from '@/lib/api/list-query' +import type { CursorKey, KeysetKey, ListSortOrder } from '@/lib/api/list-query' +import { + keysetColumns, + keysetPage, + listOrderBy, + resumeKeyset, + searchFilter, + textKey, + timestampKey, +} from '@/lib/api/list-query' import type { HighestPrioritySubscription } from '@/lib/billing/core/plan' import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription' import { ensureUserStatsExists } from '@/lib/billing/core/usage' @@ -126,16 +122,36 @@ type KnowledgeBaseStorageMove = ownerUserId: string } +/** The columns a knowledge-base keyset orders and resumes on. */ +interface KnowledgeBaseSortRow { + id: string + name: string + createdAt: Date + updatedAt: Date +} + +const knowledgeBaseIdKey = textKey(knowledgeBase.id, (row) => row.id) + /** - * Orderings for the public list's sortable fields, made total over the contract - * enum by `satisfies`. Each ends in `createdAt` so knowledge bases sharing a - * name still come back in a stable order. + * Keyset orderings for the public list's sortable fields, made total over the + * contract enum by `satisfies`. + * + * Each ends in `id` rather than `createdAt`. `createdAt` is not unique, so it + * could not separate two knowledge bases created in the same millisecond — which + * left ties in an order the planner chose, and would let a cursor repeat or skip + * a row at a page boundary now that the list pages. */ const KNOWLEDGE_BASE_SORTS = { - name: [knowledgeBase.name, knowledgeBase.createdAt], - createdAt: [knowledgeBase.createdAt], - updatedAt: [knowledgeBase.updatedAt, knowledgeBase.createdAt], -} satisfies Record + name: [textKey(knowledgeBase.name, (row) => row.name), knowledgeBaseIdKey], + createdAt: [ + timestampKey(knowledgeBase.createdAt, (row) => row.createdAt), + knowledgeBaseIdKey, + ], + updatedAt: [ + timestampKey(knowledgeBase.updatedAt, (row) => row.updatedAt), + knowledgeBaseIdKey, + ], +} satisfies Record[]> export interface GetKnowledgeBasesOptions { /** Restrict to one knowledge-base folder; `undefined` lists all and `null` lists the root. */ @@ -144,6 +160,14 @@ export interface GetKnowledgeBasesOptions { search?: string sortBy?: V2KnowledgeBaseSortBy sortOrder?: ListSortOrder + /** + * Page size. Omitted reads the whole workspace set as one page, capped by + * {@link MAX_KNOWLEDGE_BASES_PER_WORKSPACE} — what the internal callers that + * need every row still do. + */ + limit?: number + /** Keyset to resume after, from the previous page's `nextCursorKeys`. */ + cursorKeys?: CursorKey[] } async function attachConnectorTypes( @@ -195,8 +219,23 @@ export async function getWorkspaceKnowledgeBases( workspaceId: string, scope: KnowledgeBaseScope = 'active', options?: GetKnowledgeBasesOptions -): Promise { - const { folderId, search, sortBy = 'createdAt', sortOrder = 'asc' } = options ?? {} +): Promise<{ data: KnowledgeBaseWithCounts[]; nextCursorKeys: CursorKey[] | null }> { + const { + folderId, + search, + sortBy = 'createdAt', + sortOrder = 'asc', + limit, + cursorKeys, + } = options ?? {} + const keys = KNOWLEDGE_BASE_SORTS[sortBy] + const resumeAfter = resumeKeyset(keys, cursorKeys, sortOrder) + + /** + * An unpaged read still reads one row past the cap so an oversized workspace + * is a hard failure rather than a silently truncated list. + */ + const readLimit = (limit ?? MAX_KNOWLEDGE_BASES_PER_WORKSPACE) + 1 const scopeCondition = scope === 'all' ? undefined @@ -240,26 +279,32 @@ export async function getWorkspaceKnowledgeBases( : folderId === null ? isNull(knowledgeBase.folderId) : eq(knowledgeBase.folderId, folderId), - searchFilter(knowledgeBase.name, search) + searchFilter(knowledgeBase.name, search), + resumeAfter ) ) .groupBy(knowledgeBase.id) - .orderBy(...listOrderBy(KNOWLEDGE_BASE_SORTS[sortBy], sortOrder)) - .limit(MAX_KNOWLEDGE_BASES_PER_WORKSPACE + 1) + .orderBy(...listOrderBy(keysetColumns(keys), sortOrder)) + .limit(readLimit) - if (rows.length > MAX_KNOWLEDGE_BASES_PER_WORKSPACE) { + if (limit === undefined && rows.length > MAX_KNOWLEDGE_BASES_PER_WORKSPACE) { throw new Error( `Knowledge base list exceeds the ${MAX_KNOWLEDGE_BASES_PER_WORKSPACE} row limit` ) } - return attachConnectorTypes( - rows.map((kb) => ({ - ...kb, - chunkingConfig: kb.chunkingConfig as ChunkingConfig, - docCount: Number(kb.docCount), - })) - ) + const page = keysetPage(keys, rows, limit) + + return { + data: await attachConnectorTypes( + page.data.map((kb) => ({ + ...kb, + chunkingConfig: kb.chunkingConfig as ChunkingConfig, + docCount: Number(kb.docCount), + })) + ), + nextCursorKeys: page.nextCursorKeys, + } } /** @@ -352,7 +397,7 @@ export async function getKnowledgeBases( ) ) .groupBy(knowledgeBase.id) - .orderBy(...listOrderBy(KNOWLEDGE_BASE_SORTS[sortBy], sortOrder)) + .orderBy(...listOrderBy(keysetColumns(KNOWLEDGE_BASE_SORTS[sortBy]), sortOrder)) const kbIds = knowledgeBasesWithCounts.map((kb) => kb.id) diff --git a/apps/sim/lib/secrets/application/use-cases.test.ts b/apps/sim/lib/secrets/application/use-cases.test.ts index 1bb47279bd4..dd2abd92a3f 100644 --- a/apps/sim/lib/secrets/application/use-cases.test.ts +++ b/apps/sim/lib/secrets/application/use-cases.test.ts @@ -82,7 +82,7 @@ describe('secret application use cases', () => { mocks.workspaceAccess.mockResolvedValue({ hasAccess: true, canWrite: true, canAdmin: false }) mocks.keyAccess.mockResolvedValue({ knownKeys: new Set(), adminKeys: new Set() }) mocks.setWorkspace.mockResolvedValue({ created: true }) - mocks.listCredentials.mockResolvedValue([secret]) + mocks.listCredentials.mockResolvedValue({ data: [secret], nextCursorKeys: null }) }) it('rejects workspace keys before resolving or reading secret state', async () => { diff --git a/apps/sim/lib/secrets/application/use-cases.ts b/apps/sim/lib/secrets/application/use-cases.ts index abd1b5c27ce..2fa1f1a5a07 100644 --- a/apps/sim/lib/secrets/application/use-cases.ts +++ b/apps/sim/lib/secrets/application/use-cases.ts @@ -1,6 +1,6 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import type { Principal } from '@sim/auth/principal' -import type { ListSortOrder } from '@/lib/api/list-query' +import type { CursorKey, ListSortOrder } from '@/lib/api/list-query' import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getWorkspaceEnvKeyAdminAccess } from '@/lib/credentials/environment' @@ -46,6 +46,18 @@ function credentialTypes(scope?: SecretScope) { return ['env_workspace', 'env_personal'] as const } +/** + * Reads secret metadata for the caller. + * + * `ownedEnvSecretsOnly` is what keeps another user's personal secret out of the + * result. It used to be a JS filter applied after the query; it lives in SQL now + * because trimming rows after the page is cut would hand the caller fewer than + * `limit` rows while `nextCursor` still promised more. + * + * The secret's public `name` is stored as the credential `displayName`, so the + * caller-facing `name` sort aliases to that column here. The alias must not + * escape into the cursor's sort stamp — see {@link listSecretsUseCase}. + */ async function listSecretMetadata(params: { workspaceId: string userId: string @@ -53,9 +65,11 @@ async function listSecretMetadata(params: { search?: string sortBy: SecretSortBy sortOrder: ListSortOrder -}): Promise { + limit?: number + cursorKeys?: CursorKey[] +}): Promise<{ data: VisibleWorkspaceCredential[]; nextCursorKeys: CursorKey[] | null }> { const workspaceAccess = await checkWorkspaceAccess(params.workspaceId, params.userId) - const rows = await listVisibleWorkspaceCredentials({ + return listVisibleWorkspaceCredentials({ workspaceId: params.workspaceId, userId: params.userId, workspaceAccess, @@ -63,8 +77,10 @@ async function listSecretMetadata(params: { search: params.search, sortBy: params.sortBy === 'name' ? 'displayName' : params.sortBy, sortOrder: params.sortOrder, + limit: params.limit, + cursorKeys: params.cursorKeys, + ownedEnvSecretsOnly: true, }) - return rows.filter((row) => row.type === 'env_workspace' || row.envOwnerUserId === params.userId) } async function requireWorkspaceSecretMutationAccess(params: { @@ -101,13 +117,13 @@ async function getSecretMetadata(params: { scope: SecretScope name: string }): Promise { - const rows = await listSecretMetadata({ + const { data } = await listSecretMetadata({ ...params, search: params.name, sortBy: 'name', sortOrder: 'asc', }) - const row = rows.find( + const row = data.find( (candidate) => candidate.envKey === params.name && (params.scope === 'workspace' @@ -126,8 +142,19 @@ export interface ListSecretsInput { search?: string sortBy: SecretSortBy sortOrder: ListSortOrder + limit: number + cursorKeys?: CursorKey[] } +/** + * Lists secret metadata as one keyset page. + * + * `sortBy`/`sortOrder` are echoed back in the caller's own vocabulary — `name`, + * not the `displayName` column it aliases to — because the presenter stamps the + * cursor with them and the route re-checks that stamp on replay. Stamping the + * aliased column name would make every cursor minted under `name` fail to + * validate on the next request. + */ export const listSecretsUseCase = defineAuthorizedWorkspaceUseCase({ operation: secretOperations.list, resolveContext: ({ input }: { input: ListSecretsInput }) => @@ -135,12 +162,18 @@ export const listSecretsUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions, async execute({ principal, input, context }) { const userId = principalUserId(principal) - const secrets = await listSecretMetadata({ + const page = await listSecretMetadata({ ...input, workspaceId: context.workspaceId, userId, }) - return { secrets, userId } + return { + secrets: page.data, + nextCursorKeys: page.nextCursorKeys, + userId, + sortBy: input.sortBy, + sortOrder: input.sortOrder, + } }, }) diff --git a/apps/sim/lib/skills/application/use-cases.ts b/apps/sim/lib/skills/application/use-cases.ts index 258280e26b8..8f54b55dd69 100644 --- a/apps/sim/lib/skills/application/use-cases.ts +++ b/apps/sim/lib/skills/application/use-cases.ts @@ -19,7 +19,7 @@ import { import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace' import { getSkillById, - listSkills, + listSkillSummariesPage, listSkillsForUser, type SkillSortBy, } from '@/lib/workflows/skills/operations' @@ -58,20 +58,42 @@ export interface ListSkillsInput { search?: string sortBy: SkillSortBy sortOrder: ListSortOrder + limit: number + /** Position in the merged built-in + workspace list, read from the cursor. */ + offset: number + /** + * The query state that position is valid within, echoed back so the presenter + * can stamp the next cursor with it. + */ + cursorScope: string } +/** + * The public skill list. Returns one page plus the window it was taken from, + * because the surface presenter sees only this result and needs both to mint + * the next cursor. + */ export const listSkillsUseCase = defineAuthorizedWorkspaceUseCase({ operation: skillOperations.list, resolveContext: ({ input }: { input: ListSkillsInput }) => resolveWorkspaceContext(input.workspaceId), authorizationOptions, async execute({ input, context }) { - const skills = await listSkills({ + const page = await listSkillSummariesPage({ workspaceId: context.workspaceId, search: input.search, - sort: { sortBy: input.sortBy, sortOrder: input.sortOrder }, + sortBy: input.sortBy, + sortOrder: input.sortOrder, + limit: input.limit, + offset: input.offset, }) - return { skills } + return { + skills: page.skills, + hasMore: page.hasMore, + offset: page.offset, + limit: page.limit, + cursorScope: input.cursorScope, + } }, }) diff --git a/apps/sim/lib/workflows/application/list-workflows.ts b/apps/sim/lib/workflows/application/list-workflows.ts index f00ade5e38b..cf92b3863f2 100644 --- a/apps/sim/lib/workflows/application/list-workflows.ts +++ b/apps/sim/lib/workflows/application/list-workflows.ts @@ -8,7 +8,6 @@ import { resolveActiveWorkspaceApplicationContext } from '@/lib/workflows/applic import { workflowOperations } from '@/lib/workflows/application/operations' import { workflowFolderPathForId } from '@/lib/workflows/application/workflow-folders' import { - InvalidWorkflowListCursorError, listWorkspaceWorkflows, type WorkflowSortBy, type WorkflowSortOrder, @@ -48,24 +47,16 @@ export const listWorkflows = defineAuthorizedWorkflowUseCase({ throw new OrchestrationError('not_found', 'Folder not found') } - let page - try { - page = await listWorkspaceWorkflows({ - workspaceId: context.workspaceId, - folderId, - deployedOnly: input.deployedOnly, - search: input.search, - sortBy: input.sortBy, - sortOrder: input.sortOrder, - cursorKeys: input.cursorKeys, - limit: input.limit, - }) - } catch (error) { - if (error instanceof InvalidWorkflowListCursorError) { - throw new OrchestrationError('validation', error.message) - } - throw error - } + const page = await listWorkspaceWorkflows({ + workspaceId: context.workspaceId, + folderId, + deployedOnly: input.deployedOnly, + search: input.search, + sortBy: input.sortBy, + sortOrder: input.sortOrder, + cursorKeys: input.cursorKeys, + limit: input.limit, + }) logger.info('Listed workflows', { workspaceId: context.workspaceId, diff --git a/apps/sim/lib/workflows/application/workflow-crud.test.ts b/apps/sim/lib/workflows/application/workflow-crud.test.ts index f71e3546d9b..e6df3d563b8 100644 --- a/apps/sim/lib/workflows/application/workflow-crud.test.ts +++ b/apps/sim/lib/workflows/application/workflow-crud.test.ts @@ -72,7 +72,6 @@ vi.mock('@/lib/folders/queries', () => ({ })) vi.mock('@/lib/workflows/queries', () => ({ - InvalidWorkflowListCursorError: class InvalidWorkflowListCursorError extends Error {}, listWorkspaceWorkflows: mocks.listRows, loadWorkflowReadSnapshot: mocks.loadSnapshot, })) diff --git a/apps/sim/lib/workflows/custom-tools/operations.ts b/apps/sim/lib/workflows/custom-tools/operations.ts index aedc1f6cc2b..79e84b18511 100644 --- a/apps/sim/lib/workflows/custom-tools/operations.ts +++ b/apps/sim/lib/workflows/custom-tools/operations.ts @@ -2,8 +2,19 @@ import { db } from '@sim/db' import { customTools } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateShortId } from '@sim/utils/id' -import { and, type Column, desc, eq, isNull, or } from 'drizzle-orm' -import { type ListSortOrder, listOrderBy, searchFilter } from '@/lib/api/list-query' +import { and, desc, eq, isNull, or } from 'drizzle-orm' +import { + type CursorKey, + type KeysetKey, + keysetColumns, + keysetPage, + type ListSortOrder, + listOrderBy, + resumeKeyset, + searchFilter, + textKey, + timestampKey, +} from '@/lib/api/list-query' import { generateRequestId } from '@/lib/core/utils/request' const logger = createLogger('CustomToolsOperations') @@ -131,6 +142,29 @@ export async function listCustomTools(params: { userId: string; workspaceId?: st .orderBy(desc(customTools.createdAt)) } +type CustomToolRow = typeof customTools.$inferSelect + +const customToolId = textKey(customTools.id, (row) => row.id) + +/** + * Keyset orderings for the public list's sortable fields, made total over the + * contract enum by `satisfies`. Each ends in `id` so tools sharing a title or a + * timestamp still come back in a stable order — which is also what makes the + * cursor resumable, since a non-unique final key can repeat or skip a row at a + * page boundary. + */ +const CUSTOM_TOOL_SORTS = { + title: [textKey(customTools.title, (row) => row.title), customToolId], + createdAt: [ + timestampKey(customTools.createdAt, (row) => row.createdAt), + customToolId, + ], + updatedAt: [ + timestampKey(customTools.updatedAt, (row) => row.updatedAt), + customToolId, + ], +} satisfies Record[]> + /** * Workspace-scoped reads and deletes. * @@ -139,35 +173,33 @@ export async function listCustomTools(params: { userId: string; workspaceId?: st * scoped in every direction, so it uses these instead — a caller holding a * workspace key must never reach another user's personal tool. */ -/** - * Orderings for the public list's sortable fields, made total over the contract - * enum by `satisfies`. Each ends in `id` so tools sharing a timestamp still come - * back in a stable order. - */ -const CUSTOM_TOOL_SORTS = { - title: [customTools.title, customTools.id], - createdAt: [customTools.createdAt, customTools.id], - updatedAt: [customTools.updatedAt, customTools.id], -} satisfies Record - export async function listWorkspaceCustomTools(params: { workspaceId: string /** Case-insensitive substring match on the tool title. */ search?: string sortBy?: CustomToolSortBy sortOrder?: ListSortOrder + limit: number + cursorKeys?: CursorKey[] }) { - const { sortBy = 'createdAt', sortOrder = 'desc' } = params - return db + const { sortBy = 'createdAt', sortOrder = 'desc', limit } = params + const keys = CUSTOM_TOOL_SORTS[sortBy] + const resumeAfter = resumeKeyset(keys, params.cursorKeys, sortOrder) + + const rows = await db .select() .from(customTools) .where( and( eq(customTools.workspaceId, params.workspaceId), - searchFilter(customTools.title, params.search) + searchFilter(customTools.title, params.search), + resumeAfter ) ) - .orderBy(...listOrderBy(CUSTOM_TOOL_SORTS[sortBy], sortOrder)) + .orderBy(...listOrderBy(keysetColumns(keys), sortOrder)) + .limit(limit + 1) + + return keysetPage(keys, rows, limit) } export async function getWorkspaceCustomTool(params: { workspaceId: string; toolId: string }) { diff --git a/apps/sim/lib/workflows/queries.ts b/apps/sim/lib/workflows/queries.ts index f4a4a11c198..8e7fa6f0663 100644 --- a/apps/sim/lib/workflows/queries.ts +++ b/apps/sim/lib/workflows/queries.ts @@ -4,13 +4,13 @@ import { and, asc, eq, inArray, isNull, type SQL, sql } from 'drizzle-orm' import type { WorkflowListItem } from '@/lib/api/contracts/workflows' import { type CursorKey, - encodeKeyset, type KeysetKey, - keysetAfter, keysetColumns, + keysetPage, type ListSortOrder, listOrderBy, numberKey, + resumeKeyset, searchFilter, textKey, timestampKey, @@ -62,13 +62,6 @@ const WORKFLOW_SORTS = { ], } satisfies Record[]> -export class InvalidWorkflowListCursorError extends Error { - constructor() { - super('Cursor does not match the requested workflow sort') - this.name = 'InvalidWorkflowListCursorError' - } -} - export interface ListWorkspaceWorkflowsInput { workspaceId: string folderId?: string | null @@ -83,10 +76,7 @@ export interface ListWorkspaceWorkflowsInput { /** Cursor-paged active workflow query used by the public workflow adapter. */ export async function listWorkspaceWorkflows(input: ListWorkspaceWorkflowsInput) { const keys = WORKFLOW_SORTS[input.sortBy] - const resumeAfter = input.cursorKeys - ? keysetAfter(keys, input.cursorKeys, input.sortOrder) - : undefined - if (resumeAfter === null) throw new InvalidWorkflowListCursorError() + const resumeAfter = resumeKeyset(keys, input.cursorKeys, input.sortOrder) const folderCondition: SQL | undefined = input.folderId === undefined @@ -124,13 +114,7 @@ export async function listWorkspaceWorkflows(input: ListWorkspaceWorkflowsInput) .orderBy(...listOrderBy(keysetColumns(keys), input.sortOrder)) .limit(input.limit + 1) - const hasMore = rows.length > input.limit - const data = rows.slice(0, input.limit) - const last = data.at(-1) - return { - data, - nextCursorKeys: hasMore && last ? encodeKeyset(keys, last) : null, - } + return keysetPage(keys, rows, input.limit) } /** diff --git a/apps/sim/lib/workflows/skills/operations.test.ts b/apps/sim/lib/workflows/skills/operations.test.ts index 62f49c01b57..e4e3efdc19e 100644 --- a/apps/sim/lib/workflows/skills/operations.test.ts +++ b/apps/sim/lib/workflows/skills/operations.test.ts @@ -1,7 +1,13 @@ /** * @vitest-environment node */ -import { dbChainMock, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' +import { + dbChainMock, + dbChainMockFns, + queueTableRows, + resetDbChainMock, + schemaMock, +} from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { getEditableSkillIdsMock } = vi.hoisted(() => ({ @@ -14,7 +20,12 @@ vi.mock('@/lib/skills/access', () => ({ getEditableSkillIds: getEditableSkillIdsMock, })) -import { listSkills, listSkillsForUser } from '@/lib/workflows/skills/operations' +import { BUILTIN_SKILLS } from '@/lib/workflows/skills/builtin-skills' +import { + listSkillSummariesPage, + listSkills, + listSkillsForUser, +} from '@/lib/workflows/skills/operations' describe('listSkills includeBuiltins', () => { beforeEach(() => { @@ -47,6 +58,69 @@ describe('listSkills includeBuiltins', () => { }) }) +/** + * The page is sliced after the built-ins are merged in and the merged array is + * re-sorted, so these assert the window over that merged sequence — not over + * the DB rows alone. + */ +describe('listSkillSummariesPage', () => { + const page = (limit: number, offset: number) => + listSkillSummariesPage({ + workspaceId: 'ws-1', + sortBy: 'name', + sortOrder: 'asc', + limit, + offset, + }) + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + afterAll(() => { + resetDbChainMock() + }) + + it('returns at most `limit` skills and reports that more remain', async () => { + const result = await page(2, 0) + + expect(result.skills).toHaveLength(2) + expect(result.hasMore).toBe(true) + expect(result).toMatchObject({ offset: 0, limit: 2 }) + }) + + it('resumes at the offset and reports the end of the list', async () => { + const first = await page(BUILTIN_SKILLS.length, 0) + const second = await page(BUILTIN_SKILLS.length, BUILTIN_SKILLS.length) + + expect(first.skills).toHaveLength(BUILTIN_SKILLS.length) + expect(first.hasMore).toBe(false) + expect(second.skills).toHaveLength(0) + expect(second.hasMore).toBe(false) + }) + + it('pages over the merged built-in and workspace sequence, not the DB rows alone', async () => { + queueTableRows(schemaMock.skill, [ + { id: 'sk-1', name: 'aaa-mine', description: 'd', workspaceId: 'ws-1' }, + ]) + + const result = await page(1, 0) + + expect(result.skills.map((s) => s.id)).toEqual(['sk-1']) + expect(result.hasMore).toBe(true) + }) + + it('never selects the skill body for a list page', async () => { + await page(50, 0) + + const projection = dbChainMockFns.select.mock.calls.at(-1)?.[0] + expect(projection).toBeDefined() + expect(projection).not.toHaveProperty('content') + expect(projection).toHaveProperty('name') + }) +}) + describe('listSkillsForUser', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/workflows/skills/operations.ts b/apps/sim/lib/workflows/skills/operations.ts index bdc1347fe5f..9ad431d3516 100644 --- a/apps/sim/lib/workflows/skills/operations.ts +++ b/apps/sim/lib/workflows/skills/operations.ts @@ -19,21 +19,57 @@ const logger = createLogger('SkillsOperations') /** Stable epoch timestamp for built-in (template) skills, which have no DB row. */ const BUILTIN_SKILL_TIMESTAMP = new Date(0) -/** Shape a built-in skill as a `skill` table row so it can ride alongside DB skills. */ -function builtinSkillRow(workspaceId: string, builtin: BuiltinSkill): typeof skill.$inferSelect { +type SkillRow = typeof skill.$inferSelect + +/** + * A skill row without its body. Skill content runs to 50 000 characters, so + * list surfaces that only render metadata select this projection instead of + * loading every body just to drop it in the presenter. + */ +export type SkillSummaryRow = Omit + +/** The `skill` columns that make up {@link SkillSummaryRow}. */ +const SKILL_SUMMARY_COLUMNS = { + id: skill.id, + workspaceId: skill.workspaceId, + userId: skill.userId, + name: skill.name, + description: skill.description, + createdAt: skill.createdAt, + updatedAt: skill.updatedAt, +} + +/** Shape a built-in skill as a body-less `skill` row so it can ride alongside DB skills. */ +function builtinSkillSummaryRow(workspaceId: string, builtin: BuiltinSkill): SkillSummaryRow { return { id: builtin.id, workspaceId, userId: null, name: builtin.name, description: builtin.description, - content: builtin.content, createdAt: BUILTIN_SKILL_TIMESTAMP, updatedAt: BUILTIN_SKILL_TIMESTAMP, } } -type SkillRow = typeof skill.$inferSelect +function builtinSkillRow(workspaceId: string, builtin: BuiltinSkill): SkillRow { + return { ...builtinSkillSummaryRow(workspaceId, builtin), content: builtin.content } +} + +/** + * The built-ins a list should show: those no DB row shadows by name, narrowed + * by the same search term the DB half ran. + * + * Restricting `dbNames` to the searched rows is safe: a DB skill only shadows a + * built-in by sharing its name, and a name that matches the search on the + * built-in matches it on the DB row too. + */ +function visibleBuiltins(dbNames: Set, search?: string): BuiltinSkill[] { + const term = search?.toLowerCase() + return BUILTIN_SKILLS.filter( + (b) => !dbNames.has(b.name.toLowerCase()) && (!term || b.name.toLowerCase().includes(term)) + ) +} export type SkillSortBy = 'name' | 'createdAt' | 'updatedAt' /** @@ -48,12 +84,12 @@ const SKILL_SORTS = { } satisfies Record /** The sort key {@link SKILL_SORTS} orders on, for one row. */ -function skillSortKey(row: SkillRow, sortBy: SkillSortBy): [string | number, string] { +function skillSortKey(row: SkillSummaryRow, sortBy: SkillSortBy): [string | number, string] { if (sortBy === 'name') return [row.name, row.id] return [(sortBy === 'createdAt' ? row.createdAt : row.updatedAt).getTime(), row.id] } -function compareSkills(a: SkillRow, b: SkillRow, sortBy: SkillSortBy): number { +function compareSkills(a: SkillSummaryRow, b: SkillSummaryRow, sortBy: SkillSortBy): number { const [aKey, aId] = skillSortKey(a, sortBy) const [bKey, bId] = skillSortKey(b, sortBy) if (aKey !== bKey) return aKey < bKey ? -1 : 1 @@ -79,6 +115,9 @@ function compareSkills(a: SkillRow, b: SkillRow, sortBy: SkillSortBy): number { * The merged ordering compares names with JS string order rather than the * database collation. Only the handful of ASCII built-in names are placed by * it, so the two agree in practice. + * + * This returns the whole set. The public v2 list pages through + * {@link listSkillSummariesPage} instead. */ export async function listSkills(params: { workspaceId: string @@ -100,16 +139,10 @@ export async function listSkills(params: { return dbRows } - /** - * Restricting `dbNames` to the searched rows is safe: a DB skill only shadows - * a built-in by sharing its name, and a name that matches the search on the - * built-in matches it on the DB row too. - */ const dbNames = new Set(dbRows.map((r) => r.name.toLowerCase())) - const term = params.search?.toLowerCase() - const builtins = BUILTIN_SKILLS.filter( - (b) => !dbNames.has(b.name.toLowerCase()) && (!term || b.name.toLowerCase().includes(term)) - ).map((b) => builtinSkillRow(params.workspaceId, b)) + const builtins = visibleBuiltins(dbNames, params.search).map((b) => + builtinSkillRow(params.workspaceId, b) + ) if (!params.sort) return [...builtins, ...dbRows] @@ -117,6 +150,65 @@ export async function listSkills(params: { return [...builtins, ...dbRows].sort((a, b) => direction * compareSkills(a, b, sortBy)) } +/** One page of skill summaries, plus the window it was taken from. */ +export interface SkillSummaryPage { + skills: SkillSummaryRow[] + hasMore: boolean + /** The window actually applied, so a caller can mint the next cursor. */ + offset: number + limit: number +} + +/** + * A page of the public skill list: built-ins merged with the workspace's DB + * rows, sorted as requested, then sliced. + * + * **Why the window is an offset and not a keyset.** Every other v2 list resumes + * from a keyset cursor pushed into the SQL `WHERE`. This list cannot: half its + * items are {@link BUILTIN_SKILLS}, a code constant with no DB row for a + * predicate to act on, and the merged array is re-sorted in JS afterwards. A + * keyset over `skill` columns therefore cannot express a position inside the + * merged sequence — under `createdAt asc` the epoch-stamped built-ins all sort + * ahead of every DB row and a SQL cursor would skip or repeat them. So this + * follows the offset-cursor pattern that `GET /api/v2/knowledge/[id]/documents` + * already uses. + * + * The consequence of that merge is that the DB half is read whole on every + * page: the slice happens after the merge, so `LIMIT` cannot be pushed down. + * The projection drops `content` to keep that read cheap. + */ +export async function listSkillSummariesPage(params: { + workspaceId: string + search?: string + sortBy: SkillSortBy + sortOrder: ListSortOrder + limit: number + offset: number +}): Promise { + const { sortBy, sortOrder, limit, offset } = params + + const dbRows = await db + .select(SKILL_SUMMARY_COLUMNS) + .from(skill) + .where(and(eq(skill.workspaceId, params.workspaceId), searchFilter(skill.name, params.search))) + .orderBy(...listOrderBy(SKILL_SORTS[sortBy], sortOrder)) + + const dbNames = new Set(dbRows.map((r) => r.name.toLowerCase())) + const builtins = visibleBuiltins(dbNames, params.search).map((b) => + builtinSkillSummaryRow(params.workspaceId, b) + ) + + const direction = sortOrder === 'asc' ? 1 : -1 + const merged = [...builtins, ...dbRows].sort((a, b) => direction * compareSkills(a, b, sortBy)) + + return { + skills: merged.slice(offset, offset + limit), + hasMore: offset + limit < merged.length, + offset, + limit, + } +} + /** A skill row tagged with whether the caller can edit it (always false on builtins). */ export type SkillWithAccess = typeof skill.$inferSelect & { canEdit: boolean } diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index 63030a06ee2..c62f448314d 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries') const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors') const BASELINE = { - totalRoutes: 1098, - zodRoutes: 1098, + totalRoutes: 1099, + zodRoutes: 1099, nonZodRoutes: 0, } as const @@ -31,6 +31,11 @@ const BOUNDARY_POLICY_BASELINE = { } as const const INDIRECT_ZOD_ROUTES = new Set([ + // Catch-all JSON 404 for unknown /api/v2 paths. It has no contract by + // construction: it exists precisely for requests that match no operation, so + // there is no input to validate and its only response is the fixed v2 error + // envelope. + 'apps/sim/app/api/v2/[[...segments]]/route.ts', 'apps/sim/app/api/demo-requests/route.ts', // Input-less session-bound GET: nothing to validate; response is // contract-typed via `satisfies InvitationDetails` in the route.