Skip to content

Commit f77fddb

Browse files
committed
fix(v2): bind the offset cursor to the query state it counts positions in
An offset names a position in one exact sequence. `GET /skills` accepted a bare `{offset}` cursor and applied it to whatever sequence the next request asked for, so following `nextCursor` with a different `search`, `sortBy` or `sortOrder` silently skipped rows, repeated them, or landed past the end and returned an empty page while the cursor implied more. Fixed in the codec rather than the route so the sibling could not keep the gap: `decodeOffsetCursor` now takes a scope stamp and rejects a cursor minted under a different one, which is what `decodeSortedCursor` has always done for keysets. `offsetCursorScope()` builds the stamp from every param that filters or orders the sequence; `limit` is excluded because it selects how much of the sequence to return, not what the sequence is, so paging with a different page size still works. `GET /knowledge/{id}/documents` had the identical latent gap and gets the same treatment — the compiler surfaced it as soon as the signature changed.
1 parent 039e61f commit f77fddb

10 files changed

Lines changed: 247 additions & 29 deletions

File tree

.agents/skills/v2-api-conventions/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ That gives `limit` (integer, 1..`V2_MAX_PAGE_SIZE`, defaulting to `V2_DEFAULT_PA
7979
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:
8080

8181
- **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.
82-
- **Offset** (`decodeOffsetCursor` / `encodeCursor({ offset })`) — 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.
82+
- **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.
8383

8484
**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.
8585

.claude/commands/v2-api-conventions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ That gives `limit` (integer, 1..`V2_MAX_PAGE_SIZE`, defaulting to `V2_DEFAULT_PA
7878
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:
7979

8080
- **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.
81-
- **Offset** (`decodeOffsetCursor` / `encodeCursor({ offset })`) — 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.
81+
- **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.
8282

8383
**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.
8484

.cursor/commands/v2-api-conventions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ That gives `limit` (integer, 1..`V2_MAX_PAGE_SIZE`, defaulting to `V2_DEFAULT_PA
7373
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:
7474

7575
- **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.
76-
- **Offset** (`decodeOffsetCursor` / `encodeCursor({ offset })`) — 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.
76+
- **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.
7777

7878
**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.
7979

apps/sim/app/api/v2/knowledge/[id]/documents/route.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import { captureServerEvent } from '@/lib/posthog/server'
2929
import { MAX_KNOWLEDGE_DOCUMENT_FILE_SIZE } from '@/lib/uploads/shared/types'
3030
import { validateFileType } from '@/lib/uploads/utils/validation'
3131
import { serializeDate } from '@/app/api/v1/knowledge/utils'
32-
import { decodeOffsetCursor, encodeCursor } from '@/app/api/v2/lib/response'
32+
import {
33+
decodeOffsetCursor,
34+
encodeOffsetCursor,
35+
offsetCursorScope,
36+
} from '@/app/api/v2/lib/response'
3337

3438
export const dynamic = 'force-dynamic'
3539
export const revalidate = 0
@@ -71,21 +75,37 @@ export const GET = defineV2JsonRoute({
7175
operation: knowledgeOperations.listDocuments,
7276
rateLimit: v2RateLimits.publicApi,
7377
errorPolicy: v2KnowledgeErrorPolicies.concealKnowledgeBaseAuthorization,
74-
mapInput: ({ params, query }) => ({
75-
knowledgeBaseId: params.id,
76-
assertedWorkspaceId: query.workspaceId,
77-
enabledFilter: query.enabledFilter,
78-
search: query.search,
79-
limit: query.limit,
80-
offset: decodeOffsetCursor(query.cursor),
81-
sortBy: query.sortBy,
82-
sortOrder: query.sortOrder,
83-
}),
78+
mapInput: ({ params, query }) => {
79+
/**
80+
* The offset counts positions in the filtered, sorted document sequence, so
81+
* every param that changes that sequence is stamped into the cursor and
82+
* re-checked here. `limit` selects how much of the sequence to return, not
83+
* what the sequence is, so it stays out.
84+
*/
85+
const cursorScope = offsetCursorScope({
86+
knowledgeBaseId: params.id,
87+
enabledFilter: query.enabledFilter,
88+
search: query.search,
89+
sortBy: query.sortBy,
90+
sortOrder: query.sortOrder,
91+
})
92+
return {
93+
knowledgeBaseId: params.id,
94+
assertedWorkspaceId: query.workspaceId,
95+
enabledFilter: query.enabledFilter,
96+
search: query.search,
97+
limit: query.limit,
98+
offset: decodeOffsetCursor(query.cursor, cursorScope),
99+
sortBy: query.sortBy,
100+
sortOrder: query.sortOrder,
101+
cursorScope,
102+
}
103+
},
84104
useCase: listKnowledgeDocuments,
85-
present: ({ documents, pagination }) => ({
105+
present: ({ documents, pagination, cursorScope }) => ({
86106
data: documents.map(toV2DocumentSummary),
87107
nextCursor: pagination.hasMore
88-
? encodeCursor({ offset: pagination.offset + pagination.limit })
108+
? encodeOffsetCursor(cursorScope ?? '', pagination.offset + pagination.limit)
89109
: null,
90110
}),
91111
})

apps/sim/app/api/v2/lib/response.ts

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,18 +174,56 @@ export function decodeCursor<T = Record<string, unknown>>(cursor: string): T | n
174174
}
175175
}
176176

177+
interface OffsetCursorPayload {
178+
/** The query state the offset counts positions within. */
179+
scope: string
180+
offset: number
181+
}
182+
183+
/**
184+
* The filters and sort an offset cursor was minted under.
185+
*
186+
* An offset is only meaningful against one exact sequence, so everything that
187+
* reorders or re-filters that sequence has to travel with it. Build the stamp
188+
* from every such param; a value that does not affect ordering or membership
189+
* (the page size itself) must stay out, or paging with a different `limit`
190+
* would be rejected for no reason.
191+
*/
192+
export function offsetCursorScope(parts: Record<string, string | boolean | undefined>): string {
193+
return Object.keys(parts)
194+
.sort()
195+
.map((key) => `${key}=${parts[key] ?? ''}`)
196+
.join('&')
197+
}
198+
199+
/** An offset cursor stamped with the query state that produced it. */
200+
export function encodeOffsetCursor(scope: string, offset: number): string {
201+
return encodeCursor({ scope, offset } satisfies OffsetCursorPayload)
202+
}
203+
177204
/**
178-
* Reads back an offset cursor minted by `encodeCursor({ offset })`.
205+
* Reads back an offset cursor, refusing one minted under different filters or a
206+
* different sort.
179207
*
180208
* An absent cursor means page one. A cursor that is not valid base64-JSON, or
181209
* that does not carry a non-negative integer `offset`, is rejected rather than
182210
* coerced to 0: silently restarting at page one while the caller believes it is
183-
* paging forward makes a paging client loop over the first page forever. The v2
184-
* error policies render the thrown validation error as the canonical 400.
211+
* paging forward makes a paging client loop over the first page forever.
212+
*
213+
* The `scope` check is the offset counterpart of {@link decodeSortedCursor}'s
214+
* sort stamp. A bare offset replayed against a newly filtered or re-sorted
215+
* sequence names a different position in it, which silently skips rows, repeats
216+
* them, or lands past the end and returns an empty page — the failure a keyset
217+
* cursor is already protected from. The v2 error policies render the thrown
218+
* validation error as the canonical 400.
185219
*/
186-
export function decodeOffsetCursor(cursor: string | undefined): number {
220+
export function decodeOffsetCursor(cursor: string | undefined, scope: string): number {
187221
if (!cursor) return 0
188-
const offset = decodeCursor<{ offset?: unknown }>(cursor)?.offset
222+
const decoded = decodeCursor<Partial<OffsetCursorPayload>>(cursor)
223+
if (!decoded || decoded.scope !== scope) {
224+
throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE)
225+
}
226+
const { offset } = decoded
189227
if (typeof offset !== 'number' || !Number.isInteger(offset) || offset < 0) {
190228
throw new OrchestrationError('validation', 'Invalid cursor')
191229
}

apps/sim/app/api/v2/skills/route.test.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ vi.mock('@/lib/skills/application/use-cases', () => ({
5353
import { GET, POST } from '@/app/api/v2/skills/route'
5454

5555
const WORKSPACE_ID = 'workspace-1'
56+
57+
/**
58+
* The scope stamp the route mints for the default query. Written out rather
59+
* than imported so the test pins the wire format a shipped cursor carries.
60+
*/
61+
const SCOPE = ({
62+
search = '',
63+
sortBy = 'createdAt',
64+
sortOrder = 'desc',
65+
}: Record<string, string> = {}) =>
66+
`search=${search}&sortBy=${sortBy}&sortOrder=${sortOrder}&workspaceId=${WORKSPACE_ID}`
5667
const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' }
5768
const AUTH = {
5869
principal: PRINCIPAL,
@@ -118,14 +129,21 @@ describe('/api/v2/skills', () => {
118129
limit: 50,
119130
cursor: undefined,
120131
offset: 0,
132+
cursorScope: SCOPE(),
121133
},
122134
request: expect.anything(),
123135
})
124136
})
125137

126138
it('resumes from the offset cursor and mints the next one while pages remain', async () => {
127-
mocks.list.mockResolvedValueOnce({ skills: [skill], hasMore: true, offset: 2, limit: 2 })
128-
const cursor = Buffer.from(JSON.stringify({ offset: 2 })).toString('base64')
139+
mocks.list.mockResolvedValueOnce({
140+
skills: [skill],
141+
hasMore: true,
142+
offset: 2,
143+
limit: 2,
144+
cursorScope: SCOPE(),
145+
})
146+
const cursor = Buffer.from(JSON.stringify({ scope: SCOPE(), offset: 2 })).toString('base64')
129147

130148
const response = await GET(
131149
request(
@@ -136,13 +154,31 @@ describe('/api/v2/skills', () => {
136154

137155
expect(response.status).toBe(200)
138156
expect((await response.json()).nextCursor).toBe(
139-
Buffer.from(JSON.stringify({ offset: 4 })).toString('base64')
157+
Buffer.from(JSON.stringify({ scope: SCOPE(), offset: 4 })).toString('base64')
140158
)
141159
expect(mocks.list).toHaveBeenCalledWith(
142160
expect.objectContaining({ input: expect.objectContaining({ limit: 2, offset: 2 }) })
143161
)
144162
})
145163

164+
/**
165+
* An offset means nothing against a sequence it was not counted in, so a
166+
* cursor minted under one sort must not silently resume under another.
167+
*/
168+
it('rejects a cursor replayed under a different sort', async () => {
169+
const cursor = Buffer.from(JSON.stringify({ scope: SCOPE(), offset: 2 })).toString('base64')
170+
171+
const response = await GET(
172+
request(
173+
'GET',
174+
`/api/v2/skills?workspaceId=${WORKSPACE_ID}&sortBy=name&cursor=${encodeURIComponent(cursor)}`
175+
)
176+
)
177+
178+
expect(response.status).toBe(400)
179+
expect(mocks.list).not.toHaveBeenCalled()
180+
})
181+
146182
it('rejects a malformed cursor rather than silently restarting at page one', async () => {
147183
const response = await GET(
148184
request('GET', `/api/v2/skills?workspaceId=${WORKSPACE_ID}&cursor=not-a-cursor`)

apps/sim/app/api/v2/skills/route.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,28 @@ import {
88
import { captureServerEvent } from '@/lib/posthog/server'
99
import { skillOperations } from '@/lib/skills/application/operations'
1010
import { createSkillUseCase, listSkillsUseCase } from '@/lib/skills/application/use-cases'
11-
import { decodeOffsetCursor, encodeCursor } from '@/app/api/v2/lib/response'
11+
import {
12+
decodeOffsetCursor,
13+
encodeOffsetCursor,
14+
offsetCursorScope,
15+
} from '@/app/api/v2/lib/response'
1216
import { toV2Skill, toV2SkillSummary } from '@/app/api/v2/skills/utils'
1317

18+
/** The query state a skills offset cursor is only valid within. */
19+
function skillCursorScope(query: {
20+
workspaceId: string
21+
search?: string
22+
sortBy: string
23+
sortOrder: string
24+
}): string {
25+
return offsetCursorScope({
26+
workspaceId: query.workspaceId,
27+
search: query.search,
28+
sortBy: query.sortBy,
29+
sortOrder: query.sortOrder,
30+
})
31+
}
32+
1433
export const dynamic = 'force-dynamic'
1534
export const revalidate = 0
1635

@@ -21,11 +40,24 @@ export const GET = defineV2JsonRoute({
2140
auth: v2ApiKeyAuth,
2241
rateLimit: v2RateLimits.publicApi,
2342
errorPolicy: v2OrchestrationErrorPolicy,
24-
mapInput: ({ query }) => ({ ...query, offset: decodeOffsetCursor(query.cursor) }),
43+
mapInput: ({ query }) => {
44+
/**
45+
* The offset counts positions in the merged, filtered, sorted sequence, so
46+
* every param that changes that sequence is stamped into the cursor and
47+
* re-checked here. `limit` is deliberately absent — it selects how much of
48+
* the sequence to return, not what the sequence is.
49+
*/
50+
const scope = skillCursorScope(query)
51+
return {
52+
...query,
53+
offset: decodeOffsetCursor(query.cursor, scope),
54+
cursorScope: scope,
55+
}
56+
},
2557
useCase: listSkillsUseCase,
26-
present: ({ skills, hasMore, offset, limit }) => ({
58+
present: ({ skills, hasMore, offset, limit, cursorScope }) => ({
2759
data: skills.map(toV2SkillSummary),
28-
nextCursor: hasMore ? encodeCursor({ offset: offset + limit }) : null,
60+
nextCursor: hasMore ? encodeOffsetCursor(cursorScope, offset + limit) : null,
2961
}),
3062
})
3163

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import {
6+
decodeOffsetCursor,
7+
encodeOffsetCursor,
8+
offsetCursorScope,
9+
} from '@/app/api/v2/lib/response'
10+
11+
/**
12+
* An offset names a position in one exact sequence. Replay it against a
13+
* differently filtered or sorted sequence and it names a different row —
14+
* silently skipping rows, repeating them, or landing past the end and returning
15+
* an empty page while `nextCursor` implied more.
16+
*
17+
* The keyset cursor has always been protected from this by its sort stamp
18+
* (`decodeSortedCursor`). These assertions hold the offset cursor — used by
19+
* `GET /skills` and `GET /knowledge/{id}/documents` — to the same rule.
20+
*/
21+
describe('offset cursor scope', () => {
22+
const base = { workspaceId: 'ws-1', search: undefined, sortBy: 'name', sortOrder: 'asc' }
23+
24+
it('resumes a cursor replayed under the same query state', () => {
25+
const scope = offsetCursorScope(base)
26+
expect(decodeOffsetCursor(encodeOffsetCursor(scope, 40), scope)).toBe(40)
27+
})
28+
29+
it('rejects a cursor replayed under a different sort', () => {
30+
const cursor = encodeOffsetCursor(offsetCursorScope(base), 40)
31+
32+
expect(() =>
33+
decodeOffsetCursor(cursor, offsetCursorScope({ ...base, sortBy: 'createdAt' }))
34+
).toThrow(/does not match the requested/)
35+
expect(() =>
36+
decodeOffsetCursor(cursor, offsetCursorScope({ ...base, sortOrder: 'desc' }))
37+
).toThrow(/does not match the requested/)
38+
})
39+
40+
it('rejects a cursor replayed under a different filter', () => {
41+
const cursor = encodeOffsetCursor(offsetCursorScope(base), 40)
42+
43+
expect(() =>
44+
decodeOffsetCursor(cursor, offsetCursorScope({ ...base, search: 'deploy' }))
45+
).toThrow(/does not match the requested/)
46+
expect(() =>
47+
decodeOffsetCursor(cursor, offsetCursorScope({ ...base, workspaceId: 'ws-2' }))
48+
).toThrow(/does not match the requested/)
49+
})
50+
51+
it('treats an absent cursor as page one', () => {
52+
expect(decodeOffsetCursor(undefined, offsetCursorScope(base))).toBe(0)
53+
})
54+
55+
it('rejects a cursor that is not valid base64-JSON', () => {
56+
expect(() => decodeOffsetCursor('not-a-cursor', offsetCursorScope(base))).toThrow()
57+
})
58+
59+
it('rejects an offset that is not a non-negative integer', () => {
60+
const scope = offsetCursorScope(base)
61+
expect(() => decodeOffsetCursor(encodeOffsetCursor(scope, -1), scope)).toThrow('Invalid cursor')
62+
expect(() => decodeOffsetCursor(encodeOffsetCursor(scope, 1.5), scope)).toThrow(
63+
'Invalid cursor'
64+
)
65+
})
66+
67+
/**
68+
* `limit` selects how much of the sequence to return, not what the sequence
69+
* is, so paging with a different page size must keep working.
70+
*/
71+
it('is unaffected by the page size', () => {
72+
expect(offsetCursorScope({ ...base, sortBy: 'name' })).toBe(offsetCursorScope(base))
73+
})
74+
75+
it('does not depend on the order the parts are written', () => {
76+
expect(offsetCursorScope({ sortBy: 'name', workspaceId: 'ws-1', sortOrder: 'asc' })).toBe(
77+
offsetCursorScope({ sortOrder: 'asc', workspaceId: 'ws-1', sortBy: 'name' })
78+
)
79+
})
80+
})

apps/sim/lib/knowledge/application/documents.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ export interface ListKnowledgeDocumentsInput {
7474
sortBy?: DocumentSortField
7575
sortOrder?: SortOrder
7676
tagFilters?: TagFilterCondition[]
77+
/**
78+
* The query state `offset` counts positions within, echoed back so a surface
79+
* presenter can stamp the next cursor with it. Surface-only; the read itself
80+
* does not use it.
81+
*/
82+
cursorScope?: string
7783
}
7884

7985
export interface ReadKnowledgeDocumentInput {
@@ -219,7 +225,7 @@ export const listKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({
219225
},
220226
generateRequestId()
221227
)
222-
return { ...result, workspaceId: context.workspaceId }
228+
return { ...result, workspaceId: context.workspaceId, cursorScope: input.cursorScope }
223229
},
224230
})
225231

0 commit comments

Comments
 (0)