diff --git a/.agents/skills/public-api/SKILL.md b/.agents/skills/public-api/SKILL.md index a7b8860106d..575c1d0ef45 100644 --- a/.agents/skills/public-api/SKILL.md +++ b/.agents/skills/public-api/SKILL.md @@ -133,11 +133,15 @@ model; reuse only what applies. Decorators, all from `@n8n/decorators`: ## List endpoints (cursor pagination) -Copy the cursor flow from `tags.public.controller.ts`. Use `publicApiPaginationSchema` -plus `decodeCursor` / `encodeNextCursor` from the shared pagination service; the +Copy the cursor flow from `tags.public.controller.ts`. The input DTO takes +`limit: publicApiPaginationSchema.limit` plus `cursor: z.string().optional()` — +pick `limit` off the schema, never spread the whole `publicApiPaginationSchema` +(it also exports `offset`, which must never be a Public API query param). Use +`decodeCursor` / `encodeNextCursor` from the shared pagination service; the cursor is opaque; return `{ data, nextCursor }` (never a bare array) with `nextCursor: null` on the last page; an invalid cursor is a `400`. Preserve an -existing endpoint's pagination as-is. Detail: +existing endpoint's cursor semantics as-is — but an `offset` param is a +defect to remove, not a contract to preserve. Detail: [List endpoints and cursor pagination](reference.md#list-endpoints-and-cursor-pagination). ## Wiring checklist diff --git a/.agents/skills/public-api/reference.md b/.agents/skills/public-api/reference.md index e5b78d731ad..996be1cce8f 100644 --- a/.agents/skills/public-api/reference.md +++ b/.agents/skills/public-api/reference.md @@ -13,18 +13,28 @@ Copy the working flow from `v1/controllers/tags.public.controller.ts` (and `workflows.public.controller.ts` for a `@Param` list) rather than pasting a snippet here — a copy would drift. The moving parts: -- Input DTO composes `publicApiPaginationSchema` from `@n8n/api-types` (a `limit` - plus an opaque `cursor`). +- Input DTO takes `limit: publicApiPaginationSchema.limit` plus an opaque + `cursor: z.string().optional()` — cherry-pick `limit` but never spread the whole + `publicApiPaginationSchema`. That schema also exports `offset`, used by + internal-API-style page params elsewhere; a Public API list DTO must never + expose it as a query param. See `ListTagsQueryDto` for the shape to copy. - `decodeCursor` / `encodeNextCursor` live in `v1/shared/services/pagination.service.ts`. Decode the incoming cursor to `{ offset, limit }`, guard the decoded shape, and pass `offset`/`limit` to the - service. + service — `offset` is an internal implementation detail of the cursor here, + never a client-facing query param. - Treat the cursor as opaque; never hand-encode a token. - Return an envelope `{ data, nextCursor }` — never a bare array. - `encodeNextCursor(...)` returns `null` when there is no further page; surface that as `nextCursor: null`. - An invalid/undecodable cursor is a `400` via the existing bad-request error. -- For an existing list endpoint, keep its current pagination semantics unchanged. +- For an existing list endpoint, keep its current *cursor* semantics unchanged. + A leaked `offset` query param (DTO spreading `publicApiPaginationSchema` + instead of picking `limit`) is a defect to remove, not a contract to + preserve — decorator-routed DTOs validate via a plain `z.object()`, which + silently strips unknown query keys rather than rejecting them, so removing + `offset` from the DTO makes it inert rather than erroring for existing + callers. The output DTO wraps the list as `{ data, nextCursor }` and is declared with `@ApiResponse(...)` so the registry strips undeclared fields. diff --git a/packages/@n8n/api-types/src/dto/workflow-history/list-workflow-history-query.dto.ts b/packages/@n8n/api-types/src/dto/workflow-history/list-workflow-history-query.dto.ts index a94c12139da..b140ae6248b 100644 --- a/packages/@n8n/api-types/src/dto/workflow-history/list-workflow-history-query.dto.ts +++ b/packages/@n8n/api-types/src/dto/workflow-history/list-workflow-history-query.dto.ts @@ -4,6 +4,6 @@ import { Z } from '../../zod-class'; import { publicApiPaginationSchema } from '../pagination/pagination.dto'; export class ListWorkflowHistoryQueryDto extends Z.class({ - ...publicApiPaginationSchema, + limit: publicApiPaginationSchema.limit, cursor: z.string().optional(), }) {} diff --git a/packages/cli/src/public-api/v1/handlers/workflows/spec/paths/getWorkflowHistory.generated.yml b/packages/cli/src/public-api/v1/handlers/workflows/spec/paths/getWorkflowHistory.generated.yml index 5978cd0fe29..ab9ff77afed 100644 --- a/packages/cli/src/public-api/v1/handlers/workflows/spec/paths/getWorkflowHistory.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/workflows/spec/paths/getWorkflowHistory.generated.yml @@ -5,7 +5,6 @@ summary: Retrieve workflow version history description: Returns a paginated list of workflow versions (version IDs and metadata) for a workflow. x-required-scope: workflow:read parameters: - - $ref: ../../../../shared/spec/parameters/offset.yml - $ref: ../../../../shared/spec/parameters/limit.yml - $ref: ../../../../shared/spec/parameters/cursor.yml - schema: diff --git a/packages/cli/test/integration/public-api/workflows.test.ts b/packages/cli/test/integration/public-api/workflows.test.ts index dae4550dd1a..a82a74e4828 100644 --- a/packages/cli/test/integration/public-api/workflows.test.ts +++ b/packages/cli/test/integration/public-api/workflows.test.ts @@ -1210,26 +1210,6 @@ describe('GET /workflows/:id/history', () => { expect(thirdPage.body.nextCursor).toBeNull(); }); - test('should paginate with limit and offset', async () => { - const workflow = await createWorkflow({}, owner); - await createManyWorkflowHistoryItems(workflow.id, 5); - - const firstPage = await authOwnerAgent.get(`/workflows/${workflow.id}/history`).query({ - limit: '2', - offset: '0', - }); - expect(firstPage.statusCode).toBe(200); - expect(firstPage.body.data).toHaveLength(2); - - const secondPage = await authOwnerAgent.get(`/workflows/${workflow.id}/history`).query({ - limit: '2', - offset: '2', - }); - expect(secondPage.statusCode).toBe(200); - expect(secondPage.body.data).toHaveLength(2); - expect(secondPage.body.data[0].versionId).not.toBe(firstPage.body.data[0].versionId); - }); - test('should retrieve history for non-owned workflow when owner', async () => { const workflow = await createWorkflow({}, member); await createWorkflowHistoryItem(workflow.id, { name: 'Member Version' });