fix(v2): close the defects live probing found (#6681)

* fix(v2): close the defects live probing found

Staging finally deployed the merged release, so the surface could be
exercised for real. Every fix already shipped held up. These are the
defects only live traffic surfaced, plus the ones a static sweep had
found and left.

A cursor named a position in a sequence without naming the sequence.
`cursorScopeKey` hashed only the caller's filters, so any two lists
filtering on nothing but `workspaceId` produced one fingerprint and
accepted each other's tokens: a tables cursor replayed against the
knowledge list answered 200 and silently skipped a row. Table rows never
reached that check at all, so a cursor from one table paged another.
Identity now comes from the route's own contract — method plus resolved
path — because a hand-written name is the step an author forgets, and
forgetting it is invisible. An unresolved path placeholder throws rather
than fingerprinting the template, so a misconfigured route fails on every
request instead of an unlucky one. Every token minted before this is
refused with an accurate message; they are single-walk and unpersisted.

Knowledge search and the document list answered different questions.
Search grouped same-tag filters by slot and joined them with OR while the
list conjoined every filter, so `gte 9` and `lte 2` on one tag returned
nothing from the list and a full billed page from search. Search now
conjoins. The OR grouping replaced an explicit `|OR|` mechanism that was
deleted outright, was never documented in any contract, and cost the
ability to express a range on a single tag; the union it gave is still
reachable as separate searches. The search body also accepted an
unbounded query that was billed and then silently truncated to the
embedding model's window, and ignored the tag-filter cap the list
enforces.

A body over ten mebibytes was reported as malformed JSON. Next's proxy
truncates there, well under this app's fifty-megabyte ceiling, so the
parse failed on a body the caller sent whole and the size branch was
unreachable. The ceiling is now clamped to what the proxy will pass.

Also: a group's output columns accepted a `workflowGroupId` and discarded
it; an enrichment group could never gain an output, because a new output
coordinate demanded workflow metadata a group with no workflow cannot
have; `newOutputColumns` alone reported success and created nothing; a
saved view stored layout references to columns that do not exist while
refusing the same name in a filter; an MCP server stored `retries: 0` as
three and overrode an explicit auth type; a disabled server answered tool
discovery with an unclassified fault; rotating a header server's headers
left it reading connected; and a run whose workflow was deleted reported
the root folder path while also reporting the workflow deleted.

Where the honest fix was out of reach, the contract was corrected instead
of half-fixing the code: the polled run resource rebuilds `error.code` by
matching the persisted message, so it can never report the two codes that
need block attribution, and now says so. `OUTPUT_TOO_LARGE` is removed —
no path ever emitted it.

`triggers` was left alone deliberately. It reads as a closed enum but
production holds 43 distinct values, because a webhook run stores its
provider id; pinning the enum would refuse legitimate history a log
search exists to find. The description now says the vocabulary is open.

* fix(v2): clamp explicit body caps to the proxy ceiling too

The previous commit clamped the default JSON body cap but left explicit
per-route overrides alone, so a route declaring a larger `maxBodyBytes`
still fell into the truncation it was meant to report: the four inline
workspace-file routes at 70 MB and the deployed-chat route at 220 MB.

Next attaches `proxyClientMaxBodySize` to every request and clones the
body unconditionally for any non-GET method on a matched path, pushing
EOF at ten mebibytes with only a warning, so the handler reads a
truncated prefix. Those routes therefore already fail above that size —
as a malformed-JSON 400. Clamping the effective limit inside the two
body readers makes the same request fail as payload-too-large, quoting
the limit actually in force.

One existing test asserted the unreachable case, allowing a sixty-mebibyte
base64 body; it now asserts what the proxy will forward intact.

The inline-file path still advertises fifty mebibytes and cannot exceed
the proxy ceiling until that ceiling is raised, which changes buffering
for every route and belongs in its own change.

* fix(v2): close the two holes the first review round found

Both are places where a fix in this branch shut one door and left a
smaller one open in the same wall.

Letting an enrichment group gain an output meant skipping workflow
resolution — but that resolution was the only thing validating a new
output, so a PATCH began storing coordinates the runner can never fill.
It fills a cell from `result[out.outputId]` and skips an output with no
`outputId` at all, while the writer diffs on that same id and the sidebar
reads and writes by it; the contract leaves it optional. The regression
test added with that fix was itself asserting such a dead coordinate.
Create's registry checks are now two shared helpers both paths call, and
on update an output is exempt only when an identical binding already
existed, so renaming a group whose enrichment has since changed still
works while anything added or repointed must name a real output.
`mappingUpdates` on an enrichment group now says it is inexpressible
rather than resolving an empty workflow id into a missing workflow.

The layout-reference check was handed the tolerant column set, so a
placeholder minted to keep a dangling filter writable also whitelisted a
brand-new layout reference — storing an entry the next read discards,
which is the inconsistency the check was added to remove. Layout now
resolves against the live columns, which is exactly what pruning keeps,
while filters and sorts keep the exemption they need.
This commit is contained in:
Waleed
2026-08-13 16:12:40 -07:00
committed by GitHub
parent 44524d1b07
commit ab8a64fcae
53 changed files with 1827 additions and 279 deletions
+7 -5
View File
@@ -728,9 +728,9 @@
"name": "tagFilters",
"in": "query",
"required": false,
"description": "A JSON-encoded array of at most 10 tag filters, using the same display-name shape as knowledge search: `[{\"tagName\":\"category\",\"operator\":\"eq\",\"value\":\"billing\"}]`. A name that is not defined in this knowledge base is rejected, never ignored.",
"description": "A JSON-encoded array of at most 10 tag filters, using the same display-name shape as knowledge search: `[{\"tagName\":\"category\",\"operator\":\"eq\",\"value\":\"billing\"}]`. Every filter must hold, including two that name the same tag. A name that is not defined in this knowledge base is rejected, never ignored.",
"schema": {
"description": "A JSON-encoded array of at most 10 tag filters, using the same display-name shape as knowledge search: `[{\"tagName\":\"category\",\"operator\":\"eq\",\"value\":\"billing\"}]`. A name that is not defined in this knowledge base is rejected, never ignored.",
"description": "A JSON-encoded array of at most 10 tag filters, using the same display-name shape as knowledge search: `[{\"tagName\":\"category\",\"operator\":\"eq\",\"value\":\"billing\"}]`. Every filter must hold, including two that name the same tag. A name that is not defined in this knowledge base is rejected, never ignored.",
"examples": [
"[{\"tagName\":\"category\",\"operator\":\"eq\",\"value\":\"billing\"}]"
],
@@ -2820,9 +2820,10 @@
"examples": [["7c9e6679-7425-40de-944b-e07fc1f90ae7"]]
},
"query": {
"description": "Natural-language query; required when tag filters are omitted.",
"description": "Natural-language query; required when tag filters are omitted. At most 32768 characters — longer text exceeds the embedding model's per-input token ceiling and would be truncated before the billed search ran.",
"examples": ["How do I reset my password?"],
"type": "string"
"type": "string",
"maxLength": 32768
},
"topK": {
"default": 10,
@@ -2832,7 +2833,8 @@
"maximum": 100
},
"tagFilters": {
"description": "Structured tag filters. Each filtered tag must resolve to the same slot and field type in every knowledge base selected; one missing from any of them, or defined inconsistently across them, is rejected rather than ignored, and those knowledge bases must be searched separately. List the available names with `GET /api/v2/knowledge/{id}/tags`.",
"description": "Structured tag filters, at most 10 of them. Every filter must hold, including two that name the same tag: repeating one tag narrows the result rather than widening it, matching `GET /api/v2/knowledge/{id}/documents`. To match either of two values for one tag, issue a search per value. Each filtered tag must resolve to the same slot and field type in every knowledge base selected; one missing from any of them, or defined inconsistently across them, is rejected rather than ignored, and those knowledge bases must be searched separately. List the available names with `GET /api/v2/knowledge/{id}/tags`.",
"maxItems": 10,
"type": "array",
"items": {
"$ref": "#/components/schemas/V2KnowledgeSearchTagFilter"
+2 -2
View File
@@ -64,10 +64,10 @@
"name": "triggers",
"in": "query",
"required": false,
"description": "Comma-separated trigger types to include. An empty entry is rejected. The literal value `all` is a sentinel that disables this filter entirely, so a list containing it returns runs of every trigger type; no real trigger type is named `all`.",
"description": "Comma-separated trigger types to include. An empty entry is rejected. Values are matched exactly and are case-sensitive — every recorded trigger is lowercase, so `API` matches nothing while `api` matches. The vocabulary is open: it covers the core trigger types (`manual`, `api`, `schedule`, `chat`, `webhook`, `mcp`, `copilot`, `workflow`, `custom_block`) and the provider id of any webhook trigger (`slack`, `gmail`, `github`, …), so an unrecognized member is not rejected — it selects no runs. The literal value `all` is a sentinel that disables this filter entirely, so a list containing it returns runs of every trigger type; no real trigger type is named `all`.",
"schema": {
"type": "string",
"description": "Comma-separated trigger types to include. An empty entry is rejected. The literal value `all` is a sentinel that disables this filter entirely, so a list containing it returns runs of every trigger type; no real trigger type is named `all`."
"description": "Comma-separated trigger types to include. An empty entry is rejected. Values are matched exactly and are case-sensitive — every recorded trigger is lowercase, so `API` matches nothing while `api` matches. The vocabulary is open: it covers the core trigger types (`manual`, `api`, `schedule`, `chat`, `webhook`, `mcp`, `copilot`, `workflow`, `custom_block`) and the provider id of any webhook trigger (`slack`, `gmail`, `github`, …), so an unrecognized member is not rejected — it selects no runs. The literal value `all` is a sentinel that disables this filter entirely, so a list containing it returns runs of every trigger type; no real trigger type is named `all`."
}
},
{
+3 -5
View File
@@ -2492,7 +2492,7 @@
"description": "Whether the server tools are available to workflows."
},
"connectionStatus": {
"description": "Result of the most recent connection attempt. Registration and re-registration store a configuration without contacting the endpoint, so a server begins and returns to `disconnected` until a tool discovery runs.",
"description": "Result of the most recent connection attempt. Registration and re-registration establish no connection — the auth-type probe they may send does not count as one — so a server begins, and returns to, `disconnected` until a tool discovery runs.",
"type": "string",
"enum": ["connected", "disconnected", "error"]
},
@@ -2694,8 +2694,7 @@
"description": "Absolute HTTP or HTTPS endpoint URL without `{{ENV_VAR}}` references. It determines server identity and is immutable: delete and recreate the server to change endpoints."
},
"authType": {
"description": "Authentication method. Applied server-side as `headers` when omitted; registration never contacts the server, so an omitted value is never detected from it.",
"default": "headers",
"description": "Authentication method. When omitted, and no `headers` are sent, registration probes the endpoint once to classify it, falling back to `headers` when the probe fails or the server does not advertise OAuth. A server publishing RFC 9728 metadata is therefore stored as `oauth`, and headers configured afterwards will not authenticate — send this field explicitly to pin the method.",
"type": "string",
"enum": ["none", "headers", "oauth"]
},
@@ -2881,8 +2880,7 @@
"maxLength": 2048
},
"authType": {
"description": "Authentication method. Applied server-side as `headers` when omitted; registration never contacts the server, so an omitted value is never detected from it.",
"default": "headers",
"description": "Authentication method. When omitted, and no `headers` are sent, registration probes the endpoint once to classify it, falling back to `headers` when the probe fails or the server does not advertise OAuth. A server publishing RFC 9728 metadata is therefore stored as `oauth`, and headers configured afterwards will not authenticate — send this field explicitly to pin the method.",
"type": "string",
"enum": ["none", "headers", "oauth"]
},
+4 -2
View File
@@ -6611,7 +6611,8 @@
"type": "boolean"
}
},
"required": ["name", "type"]
"required": ["name", "type"],
"additionalProperties": false
},
"description": "Columns created for producer outputs."
},
@@ -6750,7 +6751,8 @@
"type": "boolean"
}
},
"required": ["name", "type"]
"required": ["name", "type"],
"additionalProperties": false
}
},
"mappingUpdates": {
+12 -13
View File
@@ -1423,9 +1423,9 @@
"name": "includeOutput",
"in": "query",
"required": false,
"description": "Include final and block outputs when true.",
"description": "Include the final workflow output when true. It does not gate `blockOutputs`, which `selectedOutputs` selects on its own.",
"schema": {
"description": "Include final and block outputs when true.",
"description": "Include the final workflow output when true. It does not gate `blockOutputs`, which `selectedOutputs` selects on its own.",
"type": "boolean"
}
},
@@ -1433,9 +1433,9 @@
"name": "selectedOutputs",
"in": "query",
"required": false,
"description": "Comma-separated block output references to include.",
"description": "Comma-separated block output references to include, as `blockId` or `blockId.path`. Block *names* are not resolved here — unlike the execute request, this resource reads a recorded run and matches ids only, so a name selects nothing and yields an empty `blockOutputs`.",
"schema": {
"description": "Comma-separated block output references to include.",
"description": "Comma-separated block output references to include, as `blockId` or `blockId.path`. Block *names* are not resolved here — unlike the execute request, this resource reads a recorded run and matches ids only, so a name selects nothing and yields an empty `blockOutputs`.",
"type": "string"
}
}
@@ -3882,21 +3882,20 @@
"INVALID_INPUT",
"BLOCK_EXECUTION_FAILED",
"CHILD_WORKFLOW_FAILED",
"OUTPUT_TOO_LARGE",
"EXECUTION_FAILED"
],
"description": "Stable machine-readable execution failure code."
"description": "Stable machine-readable execution failure code. `BLOCK_EXECUTION_FAILED` and `CHILD_WORKFLOW_FAILED` are reported only where block attribution is available; elsewhere a block-level failure is reported as `EXECUTION_FAILED`."
},
"blockId": {
"description": "Identifier of the failing block, when attributable.",
"description": "Identifier of the failing block. Present on the synchronous execute response only; the polled run resource and the resume response cannot attribute a block.",
"type": "string"
},
"blockName": {
"description": "Display name of the failing block.",
"description": "Display name of the failing block. Present on the synchronous execute response only.",
"type": "string"
},
"blockType": {
"description": "Integration or block type that failed.",
"description": "Integration or block type that failed. Present on the synchronous execute response only.",
"type": "string"
}
},
@@ -4292,7 +4291,7 @@
"type": "null"
}
],
"description": "Trigger type, or null before the run is recorded."
"description": "Trigger type that started the run. Backfilled as `api` for a run that is still queued, so it is populated from the first poll."
},
"startedAt": {
"anyOf": [
@@ -4303,7 +4302,7 @@
"type": "null"
}
],
"description": "ISO 8601 start timestamp, or null while queued.",
"description": "ISO 8601 start timestamp. A queued run reports the time it was enqueued, so it is populated from the first poll.",
"format": "date-time"
},
"endedAt": {
@@ -4453,7 +4452,7 @@
"type": "null"
}
],
"description": "Structured execution failure, or null when none occurred."
"description": "Structured execution failure, or null when none occurred. Reclassified from the persisted error message, so `blockId`/`blockName`/`blockType` are absent and a block-level failure reports `EXECUTION_FAILED` here even when the same run reported `BLOCK_EXECUTION_FAILED` on its synchronous execute response."
},
"output": {
"anyOf": [
@@ -4481,7 +4480,7 @@
"type": "null"
}
],
"description": "Selected block outputs when requested, otherwise null."
"description": "Outputs of the blocks named by `selectedOutputs`, or null when none were requested. Gated by `selectedOutputs` alone — `includeOutput` governs `output` only."
}
},
"required": [
+7 -2
View File
@@ -1,5 +1,10 @@
import { v2ListAuditLogsContract } from '@/lib/api/contracts/v2/audit-logs'
import { cursorScopeKey, instantScopePart, unorderedScopePart } from '@/lib/api/cursor-binding'
import {
cursorRoute,
cursorScopeKey,
instantScopePart,
unorderedScopePart,
} from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -23,7 +28,7 @@ function auditLogCursorFilters(query: {
startDate?: string
endDate?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListAuditLogsContract), {
organizationId: query.organizationId,
includeDeparted: query.includeDeparted,
action: query.action,
@@ -24,7 +24,8 @@ vi.mock('@/lib/billing/application/list-billing-logs', () => ({
listBillingLogs: { operation: { id: 'billing.logs.list' }, execute: mocks.execute },
}))
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { v2ListBillingLogsContract } from '@/lib/api/contracts/v2/billing'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { UNKNOWN_CURSOR_MESSAGE } from '@/lib/billing/core/usage-log'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { GET } from '@/app/api/v2/billing/logs/route'
@@ -35,7 +36,7 @@ function ledgerCursor(
inner: string,
filters: { source?: string; workspaceId?: string; period?: string }
): string {
return encodeScopedCursor(cursorScopeKey(filters), inner)
return encodeScopedCursor(cursorScopeKey(cursorRoute(v2ListBillingLogsContract), filters), inner)
}
const auth = {
+2 -2
View File
@@ -1,5 +1,5 @@
import { v2ListBillingLogsContract } from '@/lib/api/contracts/v2/billing'
import { cursorScopeKey, instantScopePart } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey, instantScopePart } from '@/lib/api/cursor-binding'
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
import { v2BillingErrorPolicies } from '@/lib/billing/api/route-policies'
import { listBillingLogs } from '@/lib/billing/application/list-billing-logs'
@@ -32,7 +32,7 @@ function billingLogCursorFilters(query: {
startDate?: string
endDate?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListBillingLogsContract), {
source: query.source,
workspaceId: query.workspaceId,
period: query.period,
+2 -2
View File
@@ -1,6 +1,6 @@
import type { V2Credential } from '@/lib/api/contracts/v2/credentials'
import { v2ListCredentialsContract } from '@/lib/api/contracts/v2/credentials'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -42,7 +42,7 @@ function credentialCursorFilters(query: {
providerId?: string
search?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListCredentialsContract), {
workspaceId: query.workspaceId,
type: query.type,
providerId: query.providerId,
+2 -2
View File
@@ -2,7 +2,7 @@ import {
v2CreateCustomToolContract,
v2ListCustomToolsContract,
} from '@/lib/api/contracts/v2/custom-tools'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -22,7 +22,7 @@ export const revalidate = 0
/** Every param that changes which custom tools, in which order, this list returns. */
function customToolCursorFilters(query: { workspaceId: string; search?: string }) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListCustomToolsContract), {
workspaceId: query.workspaceId,
search: query.search,
})
+2 -2
View File
@@ -3,7 +3,7 @@ import {
v2CreateFileContract,
v2ListFilesContract,
} from '@/lib/api/contracts/v2/files'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
@@ -24,7 +24,7 @@ function fileCursorFilters(query: {
folderPath?: string
search?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListFilesContract), {
workspaceId: query.workspaceId,
scope: query.scope,
folderPath: query.folderPath,
@@ -5,7 +5,7 @@ import {
v2ListKnowledgeDocumentsContract,
v2UploadKnowledgeDocumentContract,
} from '@/lib/api/contracts/v2/knowledge'
import { cursorScopeKey, unorderedScopeOf } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey, unorderedScopeOf } from '@/lib/api/cursor-binding'
import {
defineV2BodyLifecycleRoute,
defineV2JsonRoute,
@@ -59,8 +59,7 @@ function documentCursorFilters(
query: { workspaceId: string; enabledFilter?: string; search?: string; tagFilters?: string }
) {
const parsed = parseV2KnowledgeTagFiltersParam(query.tagFilters)
return cursorScopeKey({
knowledgeBaseId,
return cursorScopeKey(cursorRoute(v2ListKnowledgeDocumentsContract, { id: knowledgeBaseId }), {
workspaceId: query.workspaceId,
enabledFilter: query.enabledFilter,
search: query.search,
+2 -2
View File
@@ -2,7 +2,7 @@ import {
v2CreateKnowledgeBaseContract,
v2ListKnowledgeBasesContract,
} from '@/lib/api/contracts/v2/knowledge'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -28,7 +28,7 @@ function knowledgeCursorFilters(query: {
folderPath?: string
search?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListKnowledgeBasesContract), {
workspaceId: query.workspaceId,
folderPath: query.folderPath,
search: query.search,
+34 -42
View File
@@ -251,22 +251,14 @@ export function decodeCursor<T = Record<string, unknown>>(cursor: string): T | n
interface OffsetCursorPayload {
/** The ordering the offset counts positions within. */
sort: string
/** Fingerprint of the filters the offset counts positions within. */
filter?: string
/** Fingerprint of the list and filters the offset counts positions within. */
filter: string
offset: number
}
/** An offset cursor stamped with the sort and filters that produced it. */
export function encodeOffsetCursor(
sort: string,
filter: string | undefined,
offset: number
): string {
return encodeCursor({
sort,
...(filter ? { filter } : {}),
offset,
} satisfies OffsetCursorPayload)
/** An offset cursor stamped with the sort and scope that produced it. */
export function encodeOffsetCursor(sort: string, filter: string, offset: number): string {
return encodeCursor({ sort, filter, offset } satisfies OffsetCursorPayload)
}
/**
@@ -288,14 +280,15 @@ export function encodeOffsetCursor(
export function decodeOffsetCursor(
cursor: string | undefined,
sort: string,
filter?: string | undefined
filter: string
): number {
if (!cursor) return 0
const decoded = decodeCursor<Partial<OffsetCursorPayload>>(cursor)
if (!decoded || decoded.sort !== sort) {
if (!decoded) throw new OrchestrationError('validation', UNREADABLE_CURSOR_MESSAGE)
if (decoded.sort !== sort) {
throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE)
}
if ((decoded.filter ?? undefined) !== (filter || undefined)) {
if ((decoded.filter ?? undefined) !== filter) {
throw new OrchestrationError('validation', REFILTERED_CURSOR_MESSAGE)
}
const { offset } = decoded
@@ -317,29 +310,27 @@ export function cursorSortKey(sortBy: string, sortOrder: string): string {
interface SortedCursorPayload {
sort: string
keys: CursorKey[]
/** Fingerprint of the filters the page was read under; absent = unfiltered. */
filter?: string
/** Fingerprint of the list and filters the page was read under. */
filter: string
}
/**
* A keyset cursor stamped with the sort AND the filters that produced it. The
* A keyset cursor stamped with the sort AND the scope that produced it. The
* keys are only meaningful under that exact ordering, and only name a useful
* position within that exact row set, so both stamps travel with them.
*/
export function encodeSortedCursor(
sort: string,
keys: CursorKey[],
filter?: string | undefined
): string {
return encodeCursor({ sort, keys, ...(filter ? { filter } : {}) } satisfies SortedCursorPayload)
export function encodeSortedCursor(sort: string, keys: CursorKey[], filter: string): string {
return encodeCursor({ sort, keys, filter } satisfies SortedCursorPayload)
}
type DecodedSortedCursor =
| { status: 'absent' }
| { status: 'ok'; keys: CursorKey[] }
/** Malformed, or minted under a different sort — the page cannot be resumed. */
/** Not a pagination cursor at all — it does not decode into one. */
| { status: 'unreadable' }
/** Minted under a different sort — the keys compare the wrong column. */
| { status: 'invalid' }
/** Minted under different filters — the position names another sequence. */
/** Minted under a different list or different filters — another sequence. */
| { status: 'refiltered' }
/**
@@ -365,14 +356,15 @@ type DecodedSortedCursor =
export function decodeSortedCursor(
cursor: string | undefined,
sort: string,
filter?: string | undefined
filter: string
): DecodedSortedCursor {
if (!cursor) return { status: 'absent' }
const decoded = decodeCursor<Partial<SortedCursorPayload>>(cursor)
if (!decoded || decoded.sort !== sort || !Array.isArray(decoded.keys)) {
return { status: 'invalid' }
if (!decoded || typeof decoded.sort !== 'string' || !Array.isArray(decoded.keys)) {
return { status: 'unreadable' }
}
if ((decoded.filter ?? undefined) !== (filter || undefined)) return { status: 'refiltered' }
if (decoded.sort !== sort) return { status: 'invalid' }
if ((decoded.filter ?? undefined) !== filter) return { status: 'refiltered' }
return { status: 'ok', keys: decoded.keys }
}
@@ -392,9 +384,12 @@ export function readSortedCursor(
cursor: string | undefined,
sortBy: string,
sortOrder: string,
filter?: string | undefined
filter: string
): CursorKey[] | undefined {
const decoded = decodeSortedCursor(cursor, cursorSortKey(sortBy, sortOrder), filter)
if (decoded.status === 'unreadable') {
throw new OrchestrationError('validation', UNREADABLE_CURSOR_MESSAGE)
}
if (decoded.status === 'invalid') {
throw new OrchestrationError('validation', INVALID_CURSOR_MESSAGE)
}
@@ -416,14 +411,14 @@ export function writeSortedCursor(
keys: CursorKey[] | null | undefined,
sortBy: string,
sortOrder: string,
filter?: string | undefined
filter: string
): string | null {
return keys ? encodeSortedCursor(cursorSortKey(sortBy, sortOrder), keys, filter) : null
}
interface ScopedCursorPayload {
/** Fingerprint of the filters and sort the inner token was minted under. */
scope?: string
/** Fingerprint of the list, filters, and sort the inner token was minted under. */
scope: string
/** The domain codec's own opaque token, passed through untouched. */
inner: string
}
@@ -438,8 +433,8 @@ interface ScopedCursorPayload {
* lists the same binding as the rest of the surface one rule for v2 callers
* rather than "some lists notice, some don't".
*/
export function encodeScopedCursor(scope: string | undefined, inner: string): string {
return encodeCursor({ ...(scope ? { scope } : {}), inner } satisfies ScopedCursorPayload)
export function encodeScopedCursor(scope: string, inner: string): string {
return encodeCursor({ scope, inner } satisfies ScopedCursorPayload)
}
/**
@@ -456,16 +451,13 @@ export function encodeScopedCursor(scope: string | undefined, inner: string): st
* reached through the wrapper instead of through the token, and it slipped past
* the unresolvable-cursor 400 that exists to stop it.
*/
export function readScopedCursor(
cursor: string | undefined,
scope: string | undefined
): string | undefined {
export function readScopedCursor(cursor: string | undefined, scope: string): string | undefined {
if (!cursor) return undefined
const decoded = decodeCursor<Partial<ScopedCursorPayload>>(cursor)
if (!decoded || typeof decoded.inner !== 'string' || decoded.inner.length === 0) {
throw new OrchestrationError('validation', UNREADABLE_CURSOR_MESSAGE)
}
if ((decoded.scope ?? undefined) !== (scope || undefined)) {
if ((decoded.scope ?? undefined) !== scope) {
throw new OrchestrationError('validation', REFILTERED_CURSOR_MESSAGE)
}
return decoded.inner
+3 -2
View File
@@ -24,7 +24,8 @@ vi.mock('@/lib/logs/application/list-public-logs', () => ({
listPublicLogs: { operation: { id: 'logs.list' }, execute: mocks.execute },
}))
import { cursorScopeKey, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
import { v2ListLogsContract } from '@/lib/api/contracts/v2/logs'
import { cursorRoute, cursorScopeKey, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { encodeScopedCursor } from '@/app/api/v2/lib/response'
import { GET } from '@/app/api/v2/logs/route'
@@ -202,7 +203,7 @@ describe('GET /api/v2/logs', () => {
*/
it('rejects a cursor whose inner token is empty instead of restarting at page one', async () => {
const cursor = encodeScopedCursor(
cursorScopeKey({ workspaceId: WORKSPACE_ID, order: 'desc' }),
cursorScopeKey(cursorRoute(v2ListLogsContract), { workspaceId: WORKSPACE_ID, order: 'desc' }),
''
)
+2 -1
View File
@@ -5,6 +5,7 @@ import {
v2LogStatusSchema,
} from '@/lib/api/contracts/v2/logs'
import {
cursorRoute,
cursorScopeKey,
instantScopePart,
parseUnorderedList,
@@ -45,7 +46,7 @@ function logCursorFilters(query: {
folderPaths?: string
order?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListLogsContract), {
workspaceId: query.workspaceId,
workflowIds: unorderedScopePart(query.workflowIds),
triggers: unorderedScopePart(query.triggers),
+2 -2
View File
@@ -2,7 +2,7 @@ import {
v2CreateMcpServerContract,
v2ListMcpServersContract,
} from '@/lib/api/contracts/v2/mcp-servers'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -20,7 +20,7 @@ export const revalidate = 0
/** Every param that changes which MCP servers, in which order, this list returns. */
function mcpServerCursorFilters(query: { workspaceId: string; search?: string }) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListMcpServersContract), {
workspaceId: query.workspaceId,
search: query.search,
})
+2 -2
View File
@@ -1,5 +1,5 @@
import { v2ListSecretsContract } from '@/lib/api/contracts/v2/secrets'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -16,7 +16,7 @@ export const revalidate = 0
/** Every param that changes which secrets, in which order, this list returns. */
function secretCursorFilters(query: { workspaceId: string; scope?: string; search?: string }) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListSecretsContract), {
workspaceId: query.workspaceId,
scope: query.scope,
search: query.search,
+3 -2
View File
@@ -50,7 +50,8 @@ vi.mock('@/lib/skills/application/use-cases', () => ({
createSkillUseCase: { operation: { id: 'skills.create' }, execute: mocks.create },
}))
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { v2ListSkillsContract } from '@/lib/api/contracts/v2/skills'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { PrincipalKindAuthorizationError } from '@/lib/core/application'
import { cursorSortKey, encodeOffsetCursor } from '@/app/api/v2/lib/response'
import { GET, POST } from '@/app/api/v2/skills/route'
@@ -75,7 +76,7 @@ function skillCursor({
}): string {
return encodeOffsetCursor(
cursorSortKey(sortBy, sortOrder),
cursorScopeKey({ workspaceId: WORKSPACE_ID, search }),
cursorScopeKey(cursorRoute(v2ListSkillsContract), { workspaceId: WORKSPACE_ID, search }),
offset
)
}
+5 -2
View File
@@ -1,5 +1,5 @@
import { v2CreateSkillContract, v2ListSkillsContract } from '@/lib/api/contracts/v2/skills'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -14,7 +14,10 @@ import { toV2Skill, toV2SkillSummary } from '@/app/api/v2/skills/utils'
/** Every param that changes which skills, in which order, this list returns. */
function skillCursorFilters(query: { workspaceId: string; search?: string }) {
return cursorScopeKey({ workspaceId: query.workspaceId, search: query.search })
return cursorScopeKey(cursorRoute(v2ListSkillsContract), {
workspaceId: query.workspaceId,
search: query.search,
})
}
export const dynamic = 'force-dynamic'
@@ -39,8 +39,16 @@ vi.mock('@/lib/table/application/rows', () => ({
queryTableRows: { operation: { id: 'tables.rows.query' }, execute: mocks.queryRows },
}))
import { v2QueryRowsContract } from '@/lib/api/contracts/v2/tables'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { encodeScopedCursor } from '@/app/api/v2/lib/response'
import { POST } from '@/app/api/v2/tables/[tableId]/query/route'
/** A query cursor exactly as the route mints one, for the table given. */
function queryCursor(tableId: string, inner: string): string {
return encodeScopedCursor(cursorScopeKey(cursorRoute(v2QueryRowsContract, { tableId })), inner)
}
const WORKSPACE_ID = 'workspace-1'
const PRINCIPAL = {
kind: 'workspace_api_key' as const,
@@ -152,12 +160,31 @@ describe('POST /api/v2/tables/[tableId]/query', () => {
new MockTableRowsValidationError('Invalid cursor', { code: 'INVALID_CURSOR' })
)
const response = await call({ workspaceId: WORKSPACE_ID, cursor: 'malformed' }).response
const response = await call({
workspaceId: WORKSPACE_ID,
cursor: queryCursor('table-1', 'malformed'),
}).response
expect(response.status).toBe(400)
expect((await response.json()).error.details).toEqual({ code: 'INVALID_CURSOR' })
})
/**
* The row codec binds the predicate and sort a page was produced under but
* carries no table identity, so an unfiltered token from one table decoded
* cleanly against another and answered 200 with that other table's rows.
*/
it('refuses a query cursor minted on a different table', async () => {
const response = await call({
workspaceId: WORKSPACE_ID,
cursor: queryCursor('table-2', 'native-row-cursor'),
}).response
expect(response.status).toBe(400)
expect((await response.json()).error.message).toMatch(/requested filters/)
expect(mocks.queryRows).not.toHaveBeenCalled()
})
it('enforces the one MiB body cap before delegation', async () => {
const response = await call({ workspaceId: WORKSPACE_ID, cursor: 'x'.repeat(1024 * 1024) })
.response
@@ -1,15 +1,29 @@
import { TABLE_QUERY_MAX_BODY_BYTES } from '@/lib/api/contracts/tables'
import { V2_DEFAULT_ROW_LIMIT, v2QueryRowsContract } from '@/lib/api/contracts/v2/tables'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
import { v2TableRowsErrorPolicy } from '@/lib/table/api/row-route-policies'
import { tableOperations } from '@/lib/table/application/operations'
import { queryTableRows } from '@/lib/table/application/rows'
import { namedRowMapper } from '@/lib/table/cell-format'
import { encodeScopedCursor, readScopedCursor } from '@/app/api/v2/lib/response'
import { toApiRow } from '@/app/api/v2/tables/utils'
export const dynamic = 'force-dynamic'
export const revalidate = 0
/**
* The sequence a query cursor names a position in: this list, on THIS table.
*
* The row codec binds the predicate and sort a page was produced under, but not
* the table so an unfiltered token from one table decoded cleanly against
* another and answered 200 with that other table's rows. The table id lives in
* the path, so the route is the only place that knows it.
*/
function queryRowCursorScope(tableId: string): string {
return cursorScopeKey(cursorRoute(v2QueryRowsContract, { tableId }))
}
export const POST = defineV2JsonRoute({
contract: v2QueryRowsContract,
operation: tableOperations.queryRows,
@@ -22,17 +36,19 @@ export const POST = defineV2JsonRoute({
assertedWorkspaceId: body.workspaceId,
predicate: body.predicate,
sort: body.sort,
cursor: body.cursor,
cursor: readScopedCursor(body.cursor, queryRowCursorScope(params.tableId)),
limit:
body.limit === undefined ? V2_DEFAULT_ROW_LIMIT : body.limit === 0 ? undefined : body.limit,
includeTotal: false,
}),
useCase: queryTableRows,
present: ({ table, rows, nextCursor }) => {
present: ({ table, rows, nextCursor }, { params }) => {
const toNamedRow = namedRowMapper(table.schema.columns)
return {
data: rows.map((row) => toApiRow(row, toNamedRow)),
nextCursor,
nextCursor: nextCursor
? encodeScopedCursor(queryRowCursorScope(params.tableId), nextCursor)
: null,
}
},
})
@@ -38,8 +38,19 @@ vi.mock('@/lib/table/application/rows', () => ({
deleteTableRows: { operation: { id: 'tables.rows.delete_many' }, execute: mocks.deleteRows },
}))
import { v2ListTableRowsContract } from '@/lib/api/contracts/v2/tables'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { encodeScopedCursor } from '@/app/api/v2/lib/response'
import { DELETE, GET, PATCH, POST } from '@/app/api/v2/tables/[tableId]/rows/route'
/** A row cursor exactly as the route mints one, for the table given. */
function rowCursor(tableId: string, inner: string): string {
return encodeScopedCursor(
cursorScopeKey(cursorRoute(v2ListTableRowsContract, { tableId })),
inner
)
}
const WORKSPACE_ID = 'workspace-1'
const PRINCIPAL = {
kind: 'workspace_api_key' as const,
@@ -102,7 +113,7 @@ describe('/api/v2/tables/[tableId]/rows', () => {
})
it('passes the opaque native row cursor through the route unchanged', async () => {
const cursor = 'native-row-cursor'
const cursor = rowCursor('table-1', 'native-row-cursor')
mocks.listRows.mockResolvedValue({
table: TABLE,
rows: [ROW],
@@ -122,11 +133,32 @@ describe('/api/v2/tables/[tableId]/rows', () => {
tableId: 'table-1',
assertedWorkspaceId: WORKSPACE_ID,
limit: 25,
cursor,
cursor: 'native-row-cursor',
},
request: req,
})
expect((await response.json()).nextCursor).toBe('next-native-cursor')
expect((await response.json()).nextCursor).toBe(rowCursor('table-1', 'next-native-cursor'))
})
/**
* The row codec binds the sort and predicate a page was produced under but
* carries no table identity, so an unfiltered token from one table decoded
* cleanly against another and answered 200 with that other table's rows.
*/
it('refuses a row cursor minted on a different table', async () => {
const foreign = rowCursor('table-2', 'native-row-cursor')
const response = await GET(
request(
'GET',
undefined,
`?workspaceId=${WORKSPACE_ID}&limit=25&cursor=${encodeURIComponent(foreign)}`
),
CONTEXT
)
expect(response.status).toBe(400)
expect((await response.json()).error.message).toMatch(/requested filters/)
expect(mocks.listRows).not.toHaveBeenCalled()
})
it('rejects an unauthenticated request', async () => {
@@ -4,6 +4,7 @@ import {
v2ListTableRowsContract,
v2UpdateRowsByFilterContract,
} from '@/lib/api/contracts/v2/tables'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
import { v2TableRowsErrorPolicy } from '@/lib/table/api/row-route-policies'
import { tableOperations } from '@/lib/table/application/operations'
@@ -14,11 +15,24 @@ import {
updateTableRows,
} from '@/lib/table/application/rows'
import { namedRowMapper } from '@/lib/table/cell-format'
import { encodeScopedCursor, readScopedCursor } from '@/app/api/v2/lib/response'
import { toApiRow } from '@/app/api/v2/tables/utils'
export const dynamic = 'force-dynamic'
export const revalidate = 0
/**
* The sequence a row cursor names a position in: this list, on THIS table.
*
* The row codec binds a token to the sort and predicate it was minted under but
* carries no table identity, so an unfiltered token from one table decoded
* cleanly against another and answered 200 with that other table's rows. The
* table id lives in the path, so the route is the only place that knows it.
*/
function tableRowCursorScope(tableId: string): string {
return cursorScopeKey(cursorRoute(v2ListTableRowsContract, { tableId }))
}
export const GET = defineV2JsonRoute({
contract: v2ListTableRowsContract,
operation: tableOperations.listRows,
@@ -29,14 +43,16 @@ export const GET = defineV2JsonRoute({
tableId: params.tableId,
assertedWorkspaceId: query.workspaceId,
limit: query.limit,
cursor: query.cursor,
cursor: readScopedCursor(query.cursor, tableRowCursorScope(params.tableId)),
}),
useCase: listTableRows,
present: ({ table, rows, nextCursor }) => {
present: ({ table, rows, nextCursor }, { params }) => {
const toNamedRow = namedRowMapper(table.schema.columns)
return {
data: rows.map((row) => toApiRow(row, toNamedRow)),
nextCursor,
nextCursor: nextCursor
? encodeScopedCursor(tableRowCursorScope(params.tableId), nextCursor)
: null,
}
},
})
+2 -2
View File
@@ -1,5 +1,5 @@
import { v2CreateTableContract, v2ListTablesContract } from '@/lib/api/contracts/v2/tables'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
import { v2TableErrorPolicies } from '@/lib/table/api'
import { tableOperations } from '@/lib/table/application/operations'
@@ -12,7 +12,7 @@ export const revalidate = 0
/** Every param that changes which tables, in which order, this list returns. */
function tableCursorFilters(query: { workspaceId: string; folderPath?: string; search?: string }) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListTablesContract), {
workspaceId: query.workspaceId,
folderPath: query.folderPath,
search: query.search,
@@ -4,6 +4,7 @@ import {
v2WorkflowRunListStatusValueSchema,
} from '@/lib/api/contracts/v2/workflows'
import {
cursorRoute,
cursorScopeKey,
instantScopePart,
UNREADABLE_CURSOR_MESSAGE,
@@ -23,8 +24,7 @@ function runCursorFilters(
workflowId: string,
query: { status?: string; trigger?: string; startDate?: string; endDate?: string }
) {
return cursorScopeKey({
workflowId,
return cursorScopeKey(cursorRoute(v2ListWorkflowRunsContract, { id: workflowId }), {
status: query.status,
trigger: query.trigger,
startDate: instantScopePart(query.startDate),
+2 -2
View File
@@ -1,6 +1,6 @@
import type { V2WorkflowListItem } from '@/lib/api/contracts/v2/workflows'
import { v2CreateWorkflowContract, v2ListWorkflowsContract } from '@/lib/api/contracts/v2/workflows'
import { cursorScopeKey } from '@/lib/api/cursor-binding'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import {
defineV2JsonRoute,
v2ApiKeyAuth,
@@ -22,7 +22,7 @@ function workflowCursorFilters(query: {
deployedOnly: boolean
search?: string
}) {
return cursorScopeKey({
return cursorScopeKey(cursorRoute(v2ListWorkflowsContract), {
workspaceId: query.workspaceId,
folderPath: query.folderPath,
deployedOnly: query.deployedOnly,
@@ -135,9 +135,9 @@ describe('PUT /api/workspaces/[id]/files/[fileId]/content', () => {
})
})
it('allows JSON bodies above the default 50 MiB cap for base64 expansion', async () => {
it('allows a base64 JSON body up to what the proxy forwards intact', async () => {
const response = await PUT(
createRequest({ content: 'TQ==', encoding: 'base64' }, 60 * 1024 * 1024),
createRequest({ content: 'TQ==', encoding: 'base64' }, 10 * 1024 * 1024),
routeContext
)
@@ -145,8 +145,13 @@ describe('PUT /api/workspaces/[id]/files/[fileId]/content', () => {
expect(mocks.updateContent).toHaveBeenCalled()
})
it('rejects a JSON body above the inline-content cap after admission', async () => {
const response = await PUT(createRequest({ content: '' }, 70 * 1024 * 1024 + 1), routeContext)
/**
* The route declares a 70 MB inline cap, but Next's proxy truncates a client
* body past 10 MiB, so the parser clamps to that ceiling and answers 413
* rather than letting a truncated prefix surface as malformed JSON.
*/
it('rejects a JSON body above the proxy ceiling after admission', async () => {
const response = await PUT(createRequest({ content: '' }, 10 * 1024 * 1024 + 1), routeContext)
expect(response.status).toBe(413)
expect(mocks.admit).toHaveBeenCalled()
@@ -2,11 +2,13 @@
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { v2ExecutionErrorSchema } from '@/lib/api/contracts/v2/workflows'
import type { ExecutionResult } from '@/executor/types'
import {
attachExecutionResult,
buildBlockExecutionError,
classifyExecutionError,
type WorkflowExecutionErrorCode,
} from '@/executor/utils/errors'
function failedResult(partial?: Partial<ExecutionResult>): ExecutionResult {
@@ -146,4 +148,37 @@ describe('classifyExecutionError', () => {
blockType: undefined,
})
})
/**
* The published enum promises callers a code to route on, so a member this
* function cannot produce is a branch no response ever takes the state
* `OUTPUT_TOO_LARGE` was in until it was retired, since an oversize run
* response is an HTTP 413 and never reaches classification. Pinning the two
* together makes the next unreachable member fail here instead of shipping
* into SDK types.
*/
it('produces every code the public execution-error contract publishes', () => {
const producedBy: Record<WorkflowExecutionErrorCode, unknown> = {
TIMEOUT: new Error('Execution timed out after 5 minutes'),
CANCELLED: new Error('Run was cancelled'),
USAGE_LIMIT_EXCEEDED: new Error('Usage limit exceeded for this billing period'),
INVALID_INPUT: new Error('Invalid input format for the workflow'),
BLOCK_EXECUTION_FAILED: buildBlockExecutionError({
block: { id: 'block-1', metadata: { name: 'Send Email', id: 'gmail' } } as never,
error: new Error('Invalid credentials'),
}),
CHILD_WORKFLOW_FAILED: buildBlockExecutionError({
block: { id: 'block-2', metadata: { name: 'Child', id: 'workflow' } } as never,
error: new Error('Child run failed'),
}),
EXECUTION_FAILED: new Error('something odd'),
}
for (const [code, error] of Object.entries(producedBy)) {
expect(classifyExecutionError(error).code).toBe(code)
}
expect(new Set(v2ExecutionErrorSchema.shape.code.options)).toEqual(
new Set(Object.keys(producedBy))
)
})
})
+10 -1
View File
@@ -138,6 +138,16 @@ export function normalizeError(error: unknown): string {
* substring-matching messages; this module is the single place raw errors are
* interpreted, so the executor can later attach codes natively at throw sites
* without a wire change.
*
* Append-only binds what a caller can observe, and an oversize run response has
* never been observable here: it is not an in-band failure at all. The execution
* service short-circuits it into an HTTP 413 carrying `workflow_response_too_large`
* in the error envelope, so no run ever reaches classification with an oversize
* output. `OUTPUT_TOO_LARGE` was therefore a code no caller could ever have
* routed on publishing it only invited a branch that never runs, and an
* exhaustive `switch` in a generated SDK to claim coverage it does not have.
* Retiring an unreachable member narrows the published type without changing a
* single response; adding a reachable member later is still the append-only path.
*/
export type WorkflowExecutionErrorCode =
| 'TIMEOUT'
@@ -146,7 +156,6 @@ export type WorkflowExecutionErrorCode =
| 'INVALID_INPUT'
| 'BLOCK_EXECUTION_FAILED'
| 'CHILD_WORKFLOW_FAILED'
| 'OUTPUT_TOO_LARGE'
| 'EXECUTION_FAILED'
export interface StructuredExecutionError {
@@ -11,6 +11,7 @@ import { V2_SEARCH_MAX_LENGTH } from '@/lib/api/contracts/v2/shared'
import * as tableContracts from '@/lib/api/contracts/v2/tables'
import {
V2_TABLE_IMPORT_OPTIONS_MAX_BYTES,
v2AddWorkflowGroupBodySchema,
v2ApiTableSchema,
v2CreateTableBodySchema,
v2CreateTableColumnBodySchema,
@@ -25,6 +26,7 @@ import {
v2TableImportStatusSchema,
v2TableUploadImportSourceSchema,
v2UpdateTableColumnBodySchema,
v2UpdateWorkflowGroupBodySchema,
} from '@/lib/api/contracts/v2/tables'
import { getValidationErrorMessage } from '@/lib/api/server/validation'
import { MAX_RUN_TARGET_ROW_IDS, TABLE_LIMITS } from '@/lib/table/constants'
@@ -80,6 +82,51 @@ describe('v2 table column contracts', () => {
expect(issueCodes(result.error?.issues ?? [])).toContain('unrecognized_keys')
})
/**
* Same field, same reason, one level down: the group body's `.strict()` binds
* its own level, so an `outputColumns` entry that kept `workflowGroupId` was
* stripped, overwritten with the server-minted id, and answered 201.
*/
it('refuses a workflow group id on a group output column', () => {
const result = v2AddWorkflowGroupBodySchema.safeParse({
workspaceId: WORKSPACE_ID,
group: {
type: 'enrichment',
enrichmentId: 'company-domain',
outputs: [{ blockId: '', path: 'domain', columnName: 'zz_y' }],
},
outputColumns: [{ name: 'zz_y', type: 'string', workflowGroupId: 'wfg_does_not_exist' }],
})
expect(result.success).toBe(false)
expect(issueCodes(result.error?.issues ?? [])).toContain('unrecognized_keys')
})
it('refuses a workflow group id on an added output column', () => {
const result = v2UpdateWorkflowGroupBodySchema.safeParse({
workspaceId: WORKSPACE_ID,
groupId: 'group-1',
newOutputColumns: [{ name: 'zz_z', type: 'string', workflowGroupId: 'wfg_does_not_exist' }],
})
expect(result.success).toBe(false)
expect(issueCodes(result.error?.issues ?? [])).toContain('unrecognized_keys')
})
it('accepts a group output column that leaves the group id to the server', () => {
expect(
v2AddWorkflowGroupBodySchema.safeParse({
workspaceId: WORKSPACE_ID,
group: {
type: 'enrichment',
enrichmentId: 'company-domain',
outputs: [{ blockId: '', path: 'domain', columnName: 'zz_y' }],
},
outputColumns: [{ name: 'zz_y', type: 'string' }],
}).success
).toBe(true)
})
it('keeps required in table responses for existing stored schemas', () => {
expect(
v2ApiTableSchema.safeParse({
@@ -0,0 +1,106 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import {
MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS,
MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH,
parseV2KnowledgeTagFiltersParam,
v2KnowledgeSearchBodySchema,
} from '@/lib/api/contracts/v2/knowledge'
const workspaceId = '7a6cce2b-78b8-40bc-b8d3-0a2a6dfd9023'
const knowledgeBaseIds = ['ae814592-a730-4ad6-8741-b51e48843300']
function tagFilters(count: number) {
return Array.from({ length: count }, (_, index) => ({
tagName: 'Messages in Thread',
operator: 'gte' as const,
value: String(index),
}))
}
function issueMessages(result: ReturnType<typeof v2KnowledgeSearchBodySchema.safeParse>) {
return result.success ? [] : result.error.issues.map((issue) => issue.message)
}
/**
* Search is billed per call, so an input it cannot honour must be rejected at the
* boundary rather than accepted and quietly reshaped.
*/
describe('v2 knowledge search body bounds', () => {
it('accepts a query at the maximum length', () => {
const result = v2KnowledgeSearchBodySchema.safeParse({
workspaceId,
knowledgeBaseIds,
query: 'a'.repeat(MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH),
})
expect(result.success).toBe(true)
})
it('rejects a query one character past the maximum, naming the limit', () => {
const result = v2KnowledgeSearchBodySchema.safeParse({
workspaceId,
knowledgeBaseIds,
query: 'a'.repeat(MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH + 1),
})
expect(result.success).toBe(false)
expect(issueMessages(result)).toContain(
`query cannot exceed ${MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH} characters`
)
})
it('bounds the query by the embedding model per-input token ceiling', () => {
expect(MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH).toBe(8192 * 4)
})
it('accepts tag filters at the cap', () => {
const result = v2KnowledgeSearchBodySchema.safeParse({
workspaceId,
knowledgeBaseIds,
tagFilters: tagFilters(MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS),
})
expect(result.success).toBe(true)
})
/**
* The document list has always rejected an eleventh filter. Search shares the
* tag vocabulary, so it must share the policy the same request cannot be a
* 400 on one surface and an applied 200 on the other.
*/
it('rejects one tag filter past the cap with the same message the document list uses', () => {
const overLimit = tagFilters(MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS + 1)
const expected = `tagFilters cannot contain more than ${MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS} filters`
const searchResult = v2KnowledgeSearchBodySchema.safeParse({
workspaceId,
knowledgeBaseIds,
tagFilters: overLimit,
})
expect(searchResult.success).toBe(false)
expect(issueMessages(searchResult)).toContain(expected)
const listResult = parseV2KnowledgeTagFiltersParam(JSON.stringify(overLimit))
expect(listResult.success).toBe(false)
expect(listResult.success === false && listResult.message).toContain(expected)
})
it('still accepts an ordinary single filter of each field type', () => {
const result = v2KnowledgeSearchBodySchema.safeParse({
workspaceId,
knowledgeBaseIds,
query: 'How do I reset my password?',
tagFilters: [
{ tagName: 'From', operator: 'contains', value: 'brex' },
{ tagName: 'Messages in Thread', operator: 'gte', value: 9 },
{
tagName: 'Last Message',
operator: 'between',
value: '2026-01-01',
valueTo: '2026-12-31',
},
],
})
expect(result.success).toBe(true)
})
})
+31 -7
View File
@@ -821,6 +821,21 @@ export const v2KnowledgeSearchTagFilterSchema = v1SearchTagFilterSchema
description: 'A structured tag filter applied to knowledge search.',
})
/** Maximum tag filters accepted on one document-list or search request. */
export const MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS = 10
/**
* Maximum `query` length accepted by knowledge search.
*
* Every knowledge-base-eligible embedding model caps a single input at 8192
* tokens, and the embedding client silently truncates anything longer, so a
* caller paid for a billed search whose query was mostly discarded. The bound is
* that ceiling expressed in characters using the four-characters-per-token
* conversion the tokenizer's own fallback uses, which is generous enough that
* nothing that could have been embedded whole is rejected.
*/
export const MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH = 8192 * 4
export const v2KnowledgeSearchBodySchema = v1KnowledgeSearchBodySchema
.safeExtend({
workspaceId: v1KnowledgeSearchBodySchema.shape.workspaceId.describe(
@@ -829,17 +844,29 @@ export const v2KnowledgeSearchBodySchema = v1KnowledgeSearchBodySchema
knowledgeBaseIds: v1KnowledgeSearchBodySchema.shape.knowledgeBaseIds
.describe('One knowledge base identifier or an array of up to 20 identifiers.')
.meta({ examples: [['7c9e6679-7425-40de-944b-e07fc1f90ae7']] }),
query: v1KnowledgeSearchBodySchema.shape.query
.describe('Natural-language query; required when tag filters are omitted.')
query: z
.string()
.max(
MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH,
`query cannot exceed ${MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH} characters`
)
.optional()
.describe(
`Natural-language query; required when tag filters are omitted. At most ${MAX_V2_KNOWLEDGE_SEARCH_QUERY_LENGTH} characters — longer text exceeds the embedding model's per-input token ceiling and would be truncated before the billed search ran.`
)
.meta({ examples: ['How do I reset my password?'] }),
topK: v1KnowledgeSearchBodySchema.shape.topK.describe(
'Maximum number of search results to return. Must be a whole number between 1 and 100; the boundary schema only bounds the range, so a fractional value is admitted here and then rejected with 400 during search.'
),
tagFilters: z
.array(v2KnowledgeSearchTagFilterSchema)
.max(
MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS,
`tagFilters cannot contain more than ${MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS} filters`
)
.optional()
.describe(
'Structured tag filters. Each filtered tag must resolve to the same slot and field type in every knowledge base selected; one missing from any of them, or defined inconsistently across them, is rejected rather than ignored, and those knowledge bases must be searched separately. List the available names with `GET /api/v2/knowledge/{id}/tags`.'
`Structured tag filters, at most ${MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS} of them. Every filter must hold, including two that name the same tag: repeating one tag narrows the result rather than widening it, matching \`GET /api/v2/knowledge/{id}/documents\`. To match either of two values for one tag, issue a search per value. Each filtered tag must resolve to the same slot and field type in every knowledge base selected; one missing from any of them, or defined inconsistently across them, is rejected rather than ignored, and those knowledge bases must be searched separately. List the available names with \`GET /api/v2/knowledge/{id}/tags\`.`
),
searchMode: v1KnowledgeSearchBodySchema.shape.searchMode.describe(
'Retrieval strategy: vector is semantic-only, while hybrid also runs full-text search.'
@@ -895,9 +922,6 @@ export const v2SearchKnowledgeContract = defineRouteContract({
},
})
/** Maximum tag filters accepted on one document-list request. */
export const MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS = 10
const v2KnowledgeDocumentTagFiltersSchema = z
.array(v2KnowledgeSearchTagFilterSchema)
.max(
@@ -974,7 +998,7 @@ export const v2ListKnowledgeDocumentsQuerySchema = v1ListKnowledgeDocumentsQuery
.string()
.optional()
.describe(
`A JSON-encoded array of at most ${MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS} tag filters, using the same display-name shape as knowledge search: \`[{"tagName":"category","operator":"eq","value":"billing"}]\`. A name that is not defined in this knowledge base is rejected, never ignored.`
`A JSON-encoded array of at most ${MAX_V2_KNOWLEDGE_DOCUMENT_TAG_FILTERS} tag filters, using the same display-name shape as knowledge search: \`[{"tagName":"category","operator":"eq","value":"billing"}]\`. Every filter must hold, including two that name the same tag. A name that is not defined in this knowledge base is rejected, never ignored.`
)
.meta({ examples: ['[{"tagName":"category","operator":"eq","value":"billing"}]'] }),
})
+19 -1
View File
@@ -294,9 +294,27 @@ export const v2ListLogsQuerySchema = v1ListLogsQuerySchema
'workflowIds',
'Comma-separated workflow identifiers to include. An empty entry is rejected.'
).optional(),
/**
* Not a closed enum, which is why an unrecognized member is not a 400.
* `workflow_execution_logs.trigger` holds the core trigger types *and* the
* webhook provider id a run arrived on `executeWebhookJobInternal` passes
* `payload.provider` straight through as the trigger so the live
* vocabulary is the union of the core set and every webhook provider that
* has ever fired, including spellings retired since (`microsoft-teams`
* alongside `microsoftteams`). Pinning an enum here would reject the
* historical values a diagnostic search exists to find, so the filter states
* that an unmatched member simply selects nothing rather than pretending to
* validate one.
*
* Matching is exact and case-sensitive because the column is: every value
* ever written is lowercase, so `API` and `ALL` name nothing. They are
* caller mistakes, but the boundary cannot tell them apart from an unknown
* provider id, and normalizing case here would silently repair one class of
* typo while leaving the rest so the case rule is documented instead.
*/
triggers: v2CommaListSchema(
'triggers',
'Comma-separated trigger types to include. An empty entry is rejected. The literal value `all` is a sentinel that disables this filter entirely, so a list containing it returns runs of every trigger type; no real trigger type is named `all`.'
'Comma-separated trigger types to include. An empty entry is rejected. Values are matched exactly and are case-sensitive — every recorded trigger is lowercase, so `API` matches nothing while `api` matches. The vocabulary is open: it covers the core trigger types (`manual`, `api`, `schedule`, `chat`, `webhook`, `mcp`, `copilot`, `workflow`, `custom_block`) and the provider id of any webhook trigger (`slack`, `gmail`, `github`, …), so an unrecognized member is not rejected — it selects no runs. The literal value `all` is a sentinel that disables this filter entirely, so a list containing it returns runs of every trigger type; no real trigger type is named `all`.'
).optional(),
level: z.enum(['info', 'error']).describe('Severity level to include.').optional(),
startDate: v2RunWindowBoundSchema('startDate').optional(),
+14 -7
View File
@@ -117,12 +117,13 @@ export const v2McpServerSchema = z
'Whether the server tools are available to workflows.'
),
/**
* These three are written only by a real discovery. Registration stores a
* configuration without contacting the endpoint, so it leaves all three at
* their defaults rather than asserting a connection nothing has verified.
* These three are written only by a real discovery. Registration's one
* outbound touch is the auth-type probe, which classifies the endpoint and
* never records a connection, so registration leaves all three at their
* defaults rather than asserting a connection nothing has verified.
*/
connectionStatus: mcpServerSchema.shape.connectionStatus.describe(
'Result of the most recent connection attempt. Registration and re-registration store a configuration without contacting the endpoint, so a server begins and returns to `disconnected` until a tool discovery runs.'
'Result of the most recent connection attempt. Registration and re-registration establish no connection — the auth-type probe they may send does not count as one — so a server begins, and returns to, `disconnected` until a tool discovery runs.'
),
lastError: mcpServerSchema.shape.lastError.describe(
'Message from the most recent failed connection, or null when absent. A re-registration clears it, since the configuration it described no longer applies.'
@@ -221,12 +222,18 @@ export const v2CreateMcpServerBodySchema = z
)
.meta({ default: 'streamable-http' }),
url: v2McpServerUrlSchema,
/**
* No published `default`: when the field is omitted the stored value is
* detected, not defaulted, so a `default` here would be wrong on create and
* inherited by the `.partial()` update body would make an SDK that
* materializes JSON-Schema defaults revoke a stored OAuth grant on every
* unrelated PATCH.
*/
authType: mcpAuthTypeSchema
.optional()
.describe(
'Authentication method. Applied server-side as `headers` when omitted; registration never contacts the server, so an omitted value is never detected from it.'
)
.meta({ default: 'headers' }),
'Authentication method. When omitted, and no `headers` are sent, registration probes the endpoint once to classify it, falling back to `headers` when the probe fails or the server does not advertise OAuth. A server publishing RFC 9728 metadata is therefore stored as `oauth`, and headers configured afterwards will not authenticate — send this field explicitly to pin the method.'
),
/** Write-only. Reads expose `hasHeaders` and `headerNames` instead. */
headers: v2McpServerHeadersSchema
.optional()
+12 -3
View File
@@ -1274,10 +1274,19 @@ export const v2ListWorkflowGroupsContract = defineRouteContract({
* shape carries `workflowGroupId` because the client mints the group id before
* posting; v2 server-generates it, so the field is stamped from the group being
* written rather than being a caller's to supply (and get wrong).
*
* `.strict()` is what makes that omission observable. `omit()` yields an
* ordinary object schema, and the enclosing body's `.strict()` binds its own
* level only so a caller who kept the first-party `workflowGroupId` had it
* stripped, silently overwritten with the server-minted id, and answered 201.
* The same key is now a 400 naming the field, matching how `POST /v2/tables`
* refuses it on initial columns.
*/
const v2WorkflowGroupOutputColumnSchema = workflowGroupOutputColumnSchema.omit({
workflowGroupId: true,
})
const v2WorkflowGroupOutputColumnSchema = workflowGroupOutputColumnSchema
.omit({
workflowGroupId: true,
})
.strict()
/**
* A group names its producer two mutually exclusive ways, and the underlying
+79 -14
View File
@@ -787,6 +787,15 @@ export const v2RollbackWorkflowContract = defineRouteContract({
* `@/executor/utils/errors` (duplicated literally: contracts are
* client-importable and must not pull executor modules). APPEND-ONLY: callers
* route on these instead of substring-matching messages.
*
* `OUTPUT_TOO_LARGE` was retired rather than kept for the append-only promise:
* an oversize run response is not an in-band failure and never reached
* classification, so the code was never emitted on any response. See
* `WorkflowExecutionErrorCode` for the full reasoning; the oversize case is an
* HTTP 413 carrying `workflow_response_too_large` in the error envelope.
*
* Block attribution is NOT uniform across the operations that carry this
* object see `blockId` below.
*/
export const v2ExecutionErrorSchema = z
.object({
@@ -799,14 +808,41 @@ export const v2ExecutionErrorSchema = z
'INVALID_INPUT',
'BLOCK_EXECUTION_FAILED',
'CHILD_WORKFLOW_FAILED',
'OUTPUT_TOO_LARGE',
'EXECUTION_FAILED',
])
.describe('Stable machine-readable execution failure code.'),
/** Failing block, when attributable. Deliberately crosses the workspace boundary for shared/child workflows — the runId + block context is the reproducible handle a caller hands the workflow provider. */
blockId: z.string().optional().describe('Identifier of the failing block, when attributable.'),
blockName: z.string().optional().describe('Display name of the failing block.'),
blockType: z.string().optional().describe('Integration or block type that failed.'),
.describe(
'Stable machine-readable execution failure code. `BLOCK_EXECUTION_FAILED` and `CHILD_WORKFLOW_FAILED` are reported only where block attribution is available; elsewhere a block-level failure is reported as `EXECUTION_FAILED`.'
),
/**
* Failing block, when attributable. Deliberately crosses the workspace boundary for shared/child workflows the runId + block context is the reproducible handle a caller hands the workflow provider.
*
* Attribution is produced at execution time from the failing block's throw
* site, so it reaches the caller only on the synchronous execute response.
* The polled run resource and the resume response reclassify a persisted
* error *string* the log row keeps no structured error so these three
* fields are absent there and the code collapses to `EXECUTION_FAILED`.
* That is a known asymmetry, documented rather than silently promised:
* a caller that needs the failing block reads the run's trace spans, which
* do carry `blockId`, `name`, and `type`.
*/
blockId: z
.string()
.optional()
.describe(
'Identifier of the failing block. Present on the synchronous execute response only; the polled run resource and the resume response cannot attribute a block.'
),
blockName: z
.string()
.optional()
.describe(
'Display name of the failing block. Present on the synchronous execute response only.'
),
blockType: z
.string()
.optional()
.describe(
'Integration or block type that failed. Present on the synchronous execute response only.'
),
})
.meta({
id: 'ExecutionError',
@@ -1191,19 +1227,36 @@ export const v2ListWorkflowRunsContract = defineRouteContract({
/**
* The polled run resource. `queued` is backfilled from the async job
* queue before the worker writes the durable log row v1's jobs endpoint 404
* window doesn't exist here. `error` is the same structured object the execute
* response carries.
* window doesn't exist here. `error` is the same *shape* the execute response
* carries, but not the same content: this resource reclassifies the persisted
* error string, so it never attributes a block. See `v2ExecutionErrorSchema`.
*/
export const v2WorkflowRunStatusSchema = z
.object({
runId: v2WorkflowRunIdSchema,
workflowId: z.string().describe('Workflow that produced the run.'),
status: v2WorkflowRunStatusValueSchema,
trigger: z.string().nullable().describe('Trigger type, or null before the run is recorded.'),
/**
* Kept nullable on the wire while never being null in practice: every
* projection this resource has the queued job, the queued resume, and the
* durable log row backfills both fields (`api` and the job's creation time
* when the run is not yet recorded), so no caller has observed a null here.
* The nullability is the schema's tolerance for a future projection, not a
* state a caller needs to branch on, which is why neither description
* promises a null that never arrives.
*/
trigger: z
.string()
.nullable()
.describe(
'Trigger type that started the run. Backfilled as `api` for a run that is still queued, so it is populated from the first poll.'
),
startedAt: z
.string()
.nullable()
.describe('ISO 8601 start timestamp, or null while queued.')
.describe(
'ISO 8601 start timestamp. A queued run reports the time it was enqueued, so it is populated from the first poll.'
)
.meta({ format: 'date-time' }),
endedAt: z
.string()
@@ -1224,7 +1277,9 @@ export const v2WorkflowRunStatusSchema = z
.describe('Credit cost, or null when unavailable.'),
error: v2ExecutionErrorSchema
.nullable()
.describe('Structured execution failure, or null when none occurred.'),
.describe(
'Structured execution failure, or null when none occurred. Reclassified from the persisted error message, so `blockId`/`blockName`/`blockType` are absent and a block-level failure reports `EXECUTION_FAILED` here even when the same run reported `BLOCK_EXECUTION_FAILED` on its synchronous execute response.'
),
/** Populated only with `includeOutput=true` on completed runs. */
output: z
.unknown()
@@ -1234,7 +1289,9 @@ export const v2WorkflowRunStatusSchema = z
blockOutputs: z
.record(z.string(), z.unknown().describe('Output value produced by one workflow block.'))
.nullable()
.describe('Selected block outputs when requested, otherwise null.'),
.describe(
'Outputs of the blocks named by `selectedOutputs`, or null when none were requested. Gated by `selectedOutputs` alone — `includeOutput` governs `output` only.'
),
})
.meta({
id: 'WorkflowRunStatus',
@@ -1257,11 +1314,19 @@ export const v2GetWorkflowRunContract = defineRouteContract({
* working identically; it only widens what parses.
*/
includeOutput: booleanQueryFlagSchema
.describe('Include final and block outputs when true.')
.describe(
'Include the final workflow output when true. It does not gate `blockOutputs`, which `selectedOutputs` selects on its own.'
)
.optional()
.default(false),
/**
* Block *ids*, unlike the execute request's `selectedOutputs`, which also
* accepts `BlockName.path` and resolves it against the live workflow. This
* resource reads a recorded run and never loads the workflow's blocks, so
* a name has no id to resolve to and selects nothing.
*/
selectedOutputs: workflowExecutionStatusQuerySchema.shape.selectedOutputs.describe(
'Comma-separated block output references to include.'
'Comma-separated block output references to include, as `blockId` or `blockId.path`. Block *names* are not resolved here — unlike the execute request, this resource reads a recorded run and matches ids only, so a name selects nothing and yields an empty `blockOutputs`.'
),
})
.strict()
+169 -24
View File
@@ -1,9 +1,12 @@
/**
* @vitest-environment node
*/
import { globSync, readFileSync } from 'node:fs'
import { join } from 'node:path'
import { describe, expect, it } from 'vitest'
import { parseV2KnowledgeTagFiltersParam } from '@/lib/api/contracts/v2/knowledge'
import {
cursorRoute,
cursorScopeKey,
instantScopePart,
parseUnorderedList,
@@ -14,6 +17,7 @@ import {
cursorSortKey,
decodeOffsetCursor,
decodeSortedCursor,
encodeCursor,
encodeOffsetCursor,
encodeScopedCursor,
encodeSortedCursor,
@@ -33,11 +37,18 @@ import {
* `GET /skills` and `GET /knowledge/{id}/documents`, the keyset used by the nine
* SQL-ordered lists, and the wrapper that binds the domain-minted tokens on
* `GET /logs`, `GET /audit-logs`, and `GET /billing/logs` to that one rule.
*
* The scope also carries the list's own identity, because two lists that filter
* identically are still two sequences see the `list identity` block.
*/
/** Stand-in for one v2 list route, as its contract declares it. */
const LIST = cursorRoute({ method: 'GET', path: '/api/v2/files' })
describe('v2 cursor binding', () => {
const sort = cursorSortKey('name', 'asc')
const filters = { workspaceId: 'ws-1', search: undefined as string | undefined }
const scope = cursorScopeKey(filters)
const scope = cursorScopeKey(LIST, filters)
describe('offset cursor', () => {
it('resumes a cursor replayed under the same query state', () => {
@@ -59,10 +70,10 @@ describe('v2 cursor binding', () => {
const cursor = encodeOffsetCursor(sort, scope, 40)
expect(() =>
decodeOffsetCursor(cursor, sort, cursorScopeKey({ ...filters, search: 'deploy' }))
decodeOffsetCursor(cursor, sort, cursorScopeKey(LIST, { ...filters, search: 'deploy' }))
).toThrow(/requested filters/)
expect(() =>
decodeOffsetCursor(cursor, sort, cursorScopeKey({ ...filters, workspaceId: 'ws-2' }))
decodeOffsetCursor(cursor, sort, cursorScopeKey(LIST, { ...filters, workspaceId: 'ws-2' }))
).toThrow(/requested filters/)
})
@@ -100,7 +111,7 @@ describe('v2 cursor binding', () => {
*/
it('rejects a cursor replayed under a different filter', () => {
const cursor = encodeSortedCursor(sort, keys, scope)
const narrowed = cursorScopeKey({ ...filters, search: 'deploy' })
const narrowed = cursorScopeKey(LIST, { ...filters, search: 'deploy' })
expect(decodeSortedCursor(cursor, sort, narrowed)).toEqual({ status: 'refiltered' })
expect(() => readSortedCursor(cursor, 'name', 'asc', narrowed)).toThrow(/requested filters/)
@@ -117,13 +128,10 @@ describe('v2 cursor binding', () => {
).toThrow(/sortBy\/sortOrder/)
})
it('refuses an unfiltered cursor replayed under a filter, and the reverse', () => {
const unfiltered = encodeSortedCursor(sort, keys, undefined)
it('refuses a cursor minted with no scope stamp at all', () => {
const unstamped = encodeCursor({ sort, keys })
expect(() => readSortedCursor(unfiltered, 'name', 'asc', scope)).toThrow(/requested filters/)
expect(() =>
readSortedCursor(encodeSortedCursor(sort, keys, scope), 'name', 'asc', undefined)
).toThrow(/requested filters/)
expect(() => readSortedCursor(unstamped, 'name', 'asc', scope)).toThrow(/requested filters/)
})
it('treats an absent cursor as page one', () => {
@@ -142,7 +150,7 @@ describe('v2 cursor binding', () => {
const cursor = encodeScopedCursor(scope, 'domain-token')
expect(() =>
readScopedCursor(cursor, cursorScopeKey({ ...filters, search: 'deploy' }))
readScopedCursor(cursor, cursorScopeKey(LIST, { ...filters, search: 'deploy' }))
).toThrow(/requested filters/)
})
@@ -173,15 +181,20 @@ describe('v2 cursor binding', () => {
})
it('does not depend on the order the parts are written', () => {
expect(cursorScopeKey({ b: '2', a: '1' })).toBe(cursorScopeKey({ a: '1', b: '2' }))
expect(cursorScopeKey(LIST, { b: '2', a: '1' })).toBe(
cursorScopeKey(LIST, { a: '1', b: '2' })
)
})
it('treats an omitted part and an undefined part as the same scope', () => {
expect(cursorScopeKey({ a: '1', b: undefined })).toBe(cursorScopeKey({ a: '1' }))
expect(cursorScopeKey(LIST, { a: '1', b: undefined })).toBe(cursorScopeKey(LIST, { a: '1' }))
})
it('has no fingerprint at all when nothing is filtered', () => {
expect(cursorScopeKey({ a: undefined })).toBeUndefined()
it('still fingerprints the list when nothing is filtered', () => {
const bare = cursorScopeKey(LIST, { a: undefined })
expect(bare).toHaveLength(22)
expect(bare).toBe(cursorScopeKey(LIST))
})
/**
@@ -189,27 +202,119 @@ describe('v2 cursor binding', () => {
* and `{a:'1|2'}` are different reads and must fingerprint differently.
*/
it('separates parts rather than concatenating their values', () => {
expect(cursorScopeKey({ a: '1', b: '2' })).not.toBe(cursorScopeKey({ a: '1|2' }))
expect(cursorScopeKey({ a: '1' })).not.toBe(cursorScopeKey({ b: '1' }))
expect(cursorScopeKey(LIST, { a: '1', b: '2' })).not.toBe(cursorScopeKey(LIST, { a: '1|2' }))
expect(cursorScopeKey(LIST, { a: '1' })).not.toBe(cursorScopeKey(LIST, { b: '1' }))
})
it('stays short enough to sit inside an opaque token', () => {
expect(cursorScopeKey({ search: 'x'.repeat(200) })).toHaveLength(22)
expect(cursorScopeKey(LIST, { search: 'x'.repeat(200) })).toHaveLength(22)
})
})
/**
* The whole point of the scope: two lists that filter identically are still
* two sequences. Every v2 workspace list shares a `workspaceId`-only filter
* set and the same `{sort,keys,filter}` payload shape, so a fingerprint over
* filters alone made each of them accept the others' tokens and answer 200
* with a page that silently skipped rows.
*/
describe('list identity', () => {
const workspace = { workspaceId: 'ws-1' }
it('separates two lists that filter identically', () => {
const tables = cursorRoute({ method: 'GET', path: '/api/v2/tables' })
const knowledge = cursorRoute({ method: 'GET', path: '/api/v2/knowledge' })
const credentials = cursorRoute({ method: 'GET', path: '/api/v2/credentials' })
const keys = new Set(
[tables, knowledge, credentials].map((route) => cursorScopeKey(route, workspace))
)
expect(keys.size).toBe(3)
})
it('refuses a cursor minted by another list', () => {
const tablesScope = cursorScopeKey(cursorRoute({ method: 'GET', path: '/api/v2/tables' }), {
workspaceId: 'ws-1',
})
const knowledgeScope = cursorScopeKey(
cursorRoute({ method: 'GET', path: '/api/v2/knowledge' }),
{ workspaceId: 'ws-1' }
)
const fromTables = encodeSortedCursor(
sort,
['2026-03-17T10:09:31.755Z', 'tbl_1'],
tablesScope
)
expect(() => readSortedCursor(fromTables, 'name', 'asc', knowledgeScope)).toThrow(
/requested filters/
)
expect(readSortedCursor(fromTables, 'name', 'asc', tablesScope)).toEqual([
'2026-03-17T10:09:31.755Z',
'tbl_1',
])
})
it('separates two parents of the same nested list', () => {
const rowsOf = (tableId: string) =>
cursorScopeKey(
cursorRoute({ method: 'GET', path: '/api/v2/tables/[tableId]/rows' }, { tableId })
)
const fromX = encodeScopedCursor(rowsOf('tbl_x'), 'domain-token')
expect(() => readScopedCursor(fromX, rowsOf('tbl_y'))).toThrow(/requested filters/)
expect(readScopedCursor(fromX, rowsOf('tbl_x'))).toBe('domain-token')
})
/**
* A template binds every parent to one scope, which is the defect this
* exists to prevent so an unresolved placeholder is a hard failure rather
* than a fingerprint of the literal `[tableId]`.
*/
it('refuses to fingerprint an unresolved path param', () => {
expect(() =>
cursorScopeKey(cursorRoute({ method: 'GET', path: '/api/v2/tables/[tableId]/rows' }))
).toThrow(/tableId/)
})
})
/**
* A token that does not decode says nothing about the sort, and the sort
* message tells a caller who changed nothing to go re-read the sort docs.
*/
describe('undecodable cursor', () => {
it('reports that the token is unreadable, not that the sort changed', () => {
expect(() => readSortedCursor('GARBAGE', 'name', 'asc', scope)).toThrow(
/not a valid pagination cursor/
)
expect(() => readSortedCursor('GARBAGE', 'name', 'asc', scope)).not.toThrow(/sortBy/)
expect(() => decodeOffsetCursor('GARBAGE', sort, scope)).toThrow(
/not a valid pagination cursor/
)
expect(() => decodeOffsetCursor('GARBAGE', sort, scope)).not.toThrow(/sortBy/)
})
it('still names the sort when the token decodes and the sort is what changed', () => {
expect(() =>
readSortedCursor(encodeSortedCursor(sort, ['a', 'b'], scope), 'createdAt', 'asc', scope)
).toThrow(/sortBy\/sortOrder/)
})
})
})
describe('unordered filter scope parts', () => {
it('fingerprints a reordered set identically', () => {
const a = cursorScopeKey({ workflowIds: unorderedScopePart('A,B') })
const b = cursorScopeKey({ workflowIds: unorderedScopePart('B,A') })
const a = cursorScopeKey(LIST, { workflowIds: unorderedScopePart('A,B') })
const b = cursorScopeKey(LIST, { workflowIds: unorderedScopePart('B,A') })
expect(a).toBe(b)
})
it('fingerprints a duplicate-bearing set identically', () => {
// The filters compile to `inArray`, which is set membership, so A,A,B
// selects exactly what A,B does and must resume the same page.
expect(cursorScopeKey({ workflowIds: unorderedScopePart('A,A,B') })).toBe(
cursorScopeKey({ workflowIds: unorderedScopePart('A,B') })
expect(cursorScopeKey(LIST, { workflowIds: unorderedScopePart('A,A,B') })).toBe(
cursorScopeKey(LIST, { workflowIds: unorderedScopePart('A,B') })
)
expect(unorderedScopePart('B,A,B')).toBe('A,B')
})
@@ -292,8 +397,8 @@ describe('unordered filter scope parts', () => {
})
it('still separates genuinely different sets', () => {
expect(cursorScopeKey({ workflowIds: unorderedScopePart('A,B') })).not.toBe(
cursorScopeKey({ workflowIds: unorderedScopePart('A,C') })
expect(cursorScopeKey(LIST, { workflowIds: unorderedScopePart('A,B') })).not.toBe(
cursorScopeKey(LIST, { workflowIds: unorderedScopePart('A,C') })
)
})
it('treats an all-empty list as absent, matching the parsers', () => {
@@ -301,3 +406,43 @@ describe('unordered filter scope parts', () => {
expect(unorderedScopePart('A,,B')).toBe('A,B')
})
})
/**
* The declaration sweep in `contracts/v2/__tests__/list-pagination.test.ts`
* reconciles each list's cursor binding against its CONTRACT, and never looks at
* the `cursorScopeKey` call the route actually makes which is precisely where
* the missing list and parent identity sat. This closes that half: the scope a
* route builds must start from its own route identity, and a nested list must
* resolve the path params that name its parent.
*/
describe('every v2 route binds its cursor to its route identity', () => {
const routeFiles = globSync('app/api/v2/**/route.ts', {
cwd: join(import.meta.dirname, '..', '..'),
absolute: true,
})
it('finds the v2 route tree', () => {
expect(routeFiles.length).toBeGreaterThan(50)
})
it.each(routeFiles.filter((file) => readFileSync(file, 'utf8').includes('cursorScopeKey(')))(
'%s',
(file) => {
const source = readFileSync(file, 'utf8')
const calls = [...source.matchAll(/cursorScopeKey\(\s*([A-Za-z_][\w.]*)/g)].map(
(match) => match[1]
)
expect(calls.length).toBeGreaterThan(0)
for (const first of calls) {
expect(first).toBe('cursorRoute')
}
if (file.includes('[')) {
for (const call of source.matchAll(/cursorRoute\(([^)]*)\)/g)) {
expect(call[1]).toContain(',')
}
}
}
)
})
+81 -18
View File
@@ -158,24 +158,87 @@ export function fingerprint(canonical: string): string {
}
/**
* The fingerprint of a list's sequence-affecting params, or `undefined` when
* the caller supplied none of them.
* The list a cursor names a position in: the route that mints it, with the path
* params that pick out *which* parent resource resolved into the path.
*
* A cursor names a position in *one* sequence, so everything that reorders or
* re-filters that sequence has to travel with it otherwise replaying the token
* against a re-filtered read silently answers from a sequence the caller never
* asked for. Pass every param that changes *which rows, in which order*. Keep
* `limit` out: it selects how much of the sequence to return, not what the
* sequence is, so a caller may change page size mid-walk. Response-shaping
* params (whether to inline trace spans, say) stay out for the same reason.
*
* `undefined` is a real state rather than an empty hash: an unstamped cursor is
* an unfiltered one, so it stays short, and replaying it under a filter still
* mismatches (`undefined !== <hash>`). Params whose value is `undefined` are
* dropped, so omitting a filter and never having sent it are the same scope.
* Identity is taken from the contract rather than a per-route literal on
* purpose. A hand-written name is a step an author can forget, and forgetting it
* is invisible: two lists whose only filter is `workspaceId` fingerprint
* identically and silently accept each other's tokens. The contract already
* carries the one string that is unique per list and impossible to omit.
*/
export function cursorScopeKey(parts: Record<string, CursorScopePart>): string | undefined {
const present = filterUndefined(parts)
if (Object.keys(present).length === 0) return undefined
return fingerprint(canonicalJson(present))
export interface CursorScopeRoute {
method: string
path: string
/** Resolved values for the path's `[placeholders]`. */
params?: Record<string, string | undefined>
}
/**
* The route identity half of a cursor scope, built from the route's own
* contract so it cannot drift from the endpoint it binds.
*
* Pass `params` for every `[placeholder]` in the contract path those name the
* parent resource, and a nested list bound only to its filters accepts a sibling
* parent's cursor and answers with the wrong rows.
*/
export function cursorRoute(
contract: { method: string; path: string },
params?: Record<string, string | undefined>
): CursorScopeRoute {
return { method: contract.method, path: contract.path, params }
}
const PATH_PARAM_PATTERN = /\[([^\]]+)\]/g
/**
* `METHOD /concrete/path`, with every `[placeholder]` replaced by its value.
*
* An unresolved placeholder throws rather than fingerprinting the template: a
* template binds every parent resource to one scope, which is the defect this
* exists to prevent, and a route whose params never reach here is misconfigured
* for every request rather than for an unlucky one.
*/
function resolveRouteIdentity(route: CursorScopeRoute): string {
const path = route.path.replace(PATH_PARAM_PATTERN, (_match, name: string) => {
const value = route.params?.[name]
if (value === undefined || value === '') {
throw new Error(
`cursorScopeKey: ${route.method} ${route.path} has no value for path param "${name}"`
)
}
return encodeURIComponent(value)
})
return `${route.method} ${path}`
}
/**
* The fingerprint of the exact sequence a cursor names a position in: the list
* itself, plus the params that reorder or re-filter it.
*
* A cursor names a position in *one* sequence, so everything that decides which
* sequence that is has to travel with it otherwise replaying the token
* silently answers from a sequence the caller never asked for. That includes the
* list's own identity, not only its filters: `GET /v2/tables` and
* `GET /v2/knowledge` share both a `{sort,keys,filter}` payload shape and a
* `workspaceId`-only filter set, so a fingerprint over filters alone made every
* such pair accept each other's tokens and answer 200 with a page that silently
* skipped rows.
*
* Pass every param that changes *which rows, in which order*. Keep `limit` out:
* it selects how much of the sequence to return, not what the sequence is, so a
* caller may change page size mid-walk. Response-shaping params (whether to
* inline trace spans, say) stay out for the same reason.
*
* Always a string, never `undefined`: the route identity is always present, so
* there is no "unstamped" cursor to represent. Params whose value is `undefined`
* are dropped, so omitting a filter and never having sent it are the same scope.
*/
export function cursorScopeKey(
route: CursorScopeRoute,
parts: Record<string, CursorScopePart> = {}
): string {
return fingerprint(
canonicalJson({ route: resolveRouteIdentity(route), filters: filterUndefined(parts) })
)
}
+143
View File
@@ -0,0 +1,143 @@
/**
* @vitest-environment node
*/
import { NextRequest } from 'next/server'
import { describe, expect, it } from 'vitest'
import {
DEFAULT_MAX_JSON_BODY_BYTES,
parseJsonBody,
parseOptionalJsonBody,
} from '@/lib/api/server/validation'
/**
* Next.js truncates a proxied client body past `experimental.proxyClientMaxBodySize`
* without signalling it, so this is the largest body a handler can actually receive.
*/
const PROXY_CLIENT_MAX_BODY_BYTES = 10 * 1024 * 1024
/** Mirrors `MAX_WORKSPACE_FILE_INLINE_BODY_BYTES` — an explicit override above the ceiling. */
const INLINE_FILE_BODY_BYTES = 70 * 1024 * 1024
/** Mirrors the knowledge-search override — below the ceiling, so the clamp must not touch it. */
const BELOW_CEILING_BODY_BYTES = 2 * 1024 * 1024
/**
* Declares `content-length` independently of the bytes actually attached, which is
* how the guard sees an oversized request without buffering one in the test.
*/
function requestDeclaring(contentLength: number, body: string): NextRequest {
return new NextRequest('http://localhost/api/v2/widgets', {
method: 'POST',
headers: {
'content-type': 'application/json',
'content-length': String(contentLength),
},
body,
})
}
describe('DEFAULT_MAX_JSON_BODY_BYTES', () => {
it('never exceeds the body size the proxy will forward intact', () => {
expect(DEFAULT_MAX_JSON_BODY_BYTES).toBeLessThanOrEqual(PROXY_CLIENT_MAX_BODY_BYTES)
})
})
describe('parseJsonBody default size boundary', () => {
it('accepts a body at the proxy cap', async () => {
const result = await parseJsonBody(
requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES, JSON.stringify({ value: 'ok' }))
)
expect(result.success).toBe(true)
})
it('accepts a body just under the proxy cap', async () => {
const result = await parseJsonBody(
requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES - 1, JSON.stringify({ value: 'ok' }))
)
expect(result.success).toBe(true)
})
it('rejects a body just over the proxy cap as too large, not as malformed', async () => {
const result = await parseJsonBody(
requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES + 1, JSON.stringify({ value: 'ok' }))
)
expect(result.success).toBe(false)
if (result.success) return
expect(result.reason).toBe('too_large')
expect(result.response.status).toBe(413)
})
it('rejects an explicit over-ceiling override the same way, quoting the enforced limit', async () => {
const result = await parseJsonBody(
requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES + 1, JSON.stringify({ value: 'ok' })),
'response',
INLINE_FILE_BODY_BYTES
)
expect(result.success).toBe(false)
if (result.success) return
expect(result.reason).toBe('too_large')
expect(result.response.status).toBe(413)
await expect(result.response.json()).resolves.toEqual({
error: `Request body exceeds the maximum allowed size of ${PROXY_CLIENT_MAX_BODY_BYTES} bytes`,
})
})
it('still accepts a body at the ceiling under an over-ceiling override', async () => {
const result = await parseJsonBody(
requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES, JSON.stringify({ value: 'ok' })),
'response',
INLINE_FILE_BODY_BYTES
)
expect(result.success).toBe(true)
})
it('leaves an override below the ceiling exactly as declared', async () => {
const atLimit = await parseJsonBody(
requestDeclaring(BELOW_CEILING_BODY_BYTES, JSON.stringify({ value: 'ok' })),
'response',
BELOW_CEILING_BODY_BYTES
)
expect(atLimit.success).toBe(true)
const overLimit = await parseJsonBody(
requestDeclaring(BELOW_CEILING_BODY_BYTES + 1, JSON.stringify({ value: 'ok' })),
'response',
BELOW_CEILING_BODY_BYTES
)
expect(overLimit.success).toBe(false)
if (overLimit.success) return
expect(overLimit.reason).toBe('too_large')
await expect(overLimit.response.json()).resolves.toEqual({
error: `Request body exceeds the maximum allowed size of ${BELOW_CEILING_BODY_BYTES} bytes`,
})
})
it('applies the same clamp to an optional body', async () => {
const result = await parseOptionalJsonBody(
requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES + 1, JSON.stringify({ value: 'ok' })),
INLINE_FILE_BODY_BYTES
)
expect(result.success).toBe(false)
if (result.success) return
expect(result.reason).toBe('too_large')
expect(result.response.status).toBe(413)
})
it('still reports a genuinely malformed body as malformed', async () => {
const result = await parseJsonBody(requestDeclaring(7, '{"a": '))
expect(result.success).toBe(false)
if (result.success) return
expect(result.reason).toBe('invalid_json')
expect(result.response.status).toBe(400)
await expect(result.response.json()).resolves.toEqual({
error: 'Request body must be valid JSON',
})
})
})
+53 -14
View File
@@ -21,15 +21,51 @@ import {
} from '@/lib/core/utils/stream-limits'
/**
* Default upper bound on the JSON request body that contract routes will read
* and parse into memory. Next.js App Router imposes no body cap, so without
* this an unauthenticated caller could buffer an arbitrarily large body before
* schema validation runs. Override per-route via `ParseRequestOptions.maxBodyBytes`.
* Falls back to 50 MB if the env value is missing or non-numeric so a misconfig
* can never silently disable the cap (a NaN limit would never reject).
* Next.js buffers the client body for the proxy and *silently truncates* anything
* past `experimental.proxyClientMaxBodySize` (default 10 MB), and `apps/sim/proxy.ts`
* matches `/api/:path*`. A larger body therefore reaches the handler as a truncated
* prefix, which fails JSON parsing so an oversized request has to be rejected on
* its declared size before it is read, or the truncation gets misreported as a
* malformed body.
*/
export const DEFAULT_MAX_JSON_BODY_BYTES =
Number.parseInt(env.API_MAX_JSON_BODY_BYTES, 10) || 50 * 1024 * 1024
const PROXY_CLIENT_MAX_BODY_BYTES = 10 * 1024 * 1024
/**
* Default upper bound on the JSON request body that contract routes will read
* and parse into memory. Without a cap an unauthenticated caller could buffer a
* large body before schema validation runs. Override per-route via
* `ParseRequestOptions.maxBodyBytes`.
*
* Falls back to 50 MB if the env value is missing or non-numeric so a misconfig
* can never silently disable the cap (a NaN limit would never reject), then
* clamps to {@link PROXY_CLIENT_MAX_BODY_BYTES} because the app can never
* actually receive more than the proxy forwards.
*/
export const DEFAULT_MAX_JSON_BODY_BYTES = Math.min(
Number.parseInt(env.API_MAX_JSON_BODY_BYTES, 10) || 50 * 1024 * 1024,
PROXY_CLIENT_MAX_BODY_BYTES
)
/**
* Clamps a per-route body cap to {@link PROXY_CLIENT_MAX_BODY_BYTES}.
*
* A route that raises `maxBodyBytes` above the proxy ceiling cannot actually
* receive a body that large: the proxy truncates the stream, the handler parses
* a prefix, and the caller gets `400 "Request body must be valid JSON"` for a
* request whose only fault was its size. Clamping at the point of use turns that
* into an accurate `413`; nothing that succeeds today changes, because a body
* over the ceiling already fails just less honestly.
*
* Consequence worth keeping in view: `MAX_WORKSPACE_FILE_INLINE_BODY_BYTES`
* (70 MB) exists so a 50 MiB file can be sent inline as base64, and that ceiling
* stays unreachable until `experimental.proxyClientMaxBodySize` is raised in
* `apps/sim/next.config.ts`. Raising it changes the memory profile of every
* `/api` route, so it is a separate decision this clamp only makes the limit
* that is actually in force report itself correctly.
*/
function clampToProxyLimit(maxBytes: number): number {
return Math.min(maxBytes, PROXY_CLIENT_MAX_BODY_BYTES)
}
export interface ValidationErrorBody {
error: string
@@ -61,7 +97,8 @@ export interface ParseRequestOptions {
/**
* Maximum number of bytes to read for the JSON body before rejecting with a
* 413. Defaults to {@link DEFAULT_MAX_JSON_BODY_BYTES}. Raise this only for
* routes that legitimately accept large JSON payloads (e.g. inline file uploads).
* routes that legitimately accept large JSON payloads (e.g. inline file uploads);
* a value above what the proxy forwards is clamped see {@link clampToProxyLimit}.
*/
maxBodyBytes?: number
/** Treat an absent or whitespace-only body as `undefined` before contract validation. */
@@ -148,8 +185,9 @@ export async function parseJsonBody(
response: NextResponse<{ error: string }>
}
> {
const limit = clampToProxyLimit(maxBytes)
try {
return { success: true, data: await readJsonBodyWithLimit(request, maxBytes) }
return { success: true, data: await readJsonBodyWithLimit(request, limit) }
} catch (error) {
if (invalidJson === 'throw') throw error
if (isPayloadSizeLimitError(error)) {
@@ -157,7 +195,7 @@ export async function parseJsonBody(
success: false,
reason: 'too_large',
response: NextResponse.json(
{ error: `Request body exceeds the maximum allowed size of ${maxBytes} bytes` },
{ error: `Request body exceeds the maximum allowed size of ${limit} bytes` },
{ status: 413 }
),
}
@@ -188,13 +226,14 @@ export async function parseOptionalJsonBody(
response: NextResponse<{ error: string }>
}
> {
const limit = clampToProxyLimit(maxBytes)
try {
assertContentLengthWithinLimit(request.headers, maxBytes, REQUEST_BODY_LABEL)
assertContentLengthWithinLimit(request.headers, limit, REQUEST_BODY_LABEL)
const stream = request.body
const text = stream
? new TextDecoder().decode(
await readStreamToBufferWithLimit(stream, { maxBytes, label: REQUEST_BODY_LABEL })
await readStreamToBufferWithLimit(stream, { maxBytes: limit, label: REQUEST_BODY_LABEL })
)
: await request.text()
@@ -208,7 +247,7 @@ export async function parseOptionalJsonBody(
success: false,
reason: 'too_large',
response: NextResponse.json(
{ error: `Request body exceeds the maximum allowed size of ${maxBytes} bytes` },
{ error: `Request body exceeds the maximum allowed size of ${limit} bytes` },
{ status: 413 }
),
}
+72 -7
View File
@@ -2,6 +2,7 @@
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { buildTagFilterCondition } from '@/lib/knowledge/documents/tag-filter'
import { getStructuredTagFilters } from '@/lib/knowledge/search/queries'
import type { StructuredFilter } from '@/lib/knowledge/types'
@@ -20,10 +21,14 @@ const embeddingTable = {
* The global `drizzle-orm` mock renders `sql` fragments to a `?`-placeholder
* string via `toSQL()`, so we can assert the exact predicate each filter builds.
*/
function render(condition: unknown) {
return (condition as { toSQL: () => { sql: string; params: unknown[] } }).toSQL()
}
function renderOne(filters: StructuredFilter[]) {
const conditions = getStructuredTagFilters(filters, embeddingTable)
expect(conditions).toHaveLength(1)
return (conditions[0] as unknown as { toSQL: () => { sql: string; params: unknown[] } }).toSQL()
return render(conditions[0])
}
describe('getStructuredTagFilters', () => {
@@ -152,8 +157,17 @@ describe('getStructuredTagFilters', () => {
expect(sql).toBe('? != ?')
expect(params).toEqual(['boolean1', false])
})
})
it('ORs two filters on the same slot and keeps them one condition', () => {
/**
* The callers spread the returned conditions into `and(...)`, so one condition
* per filter is what makes the whole array conjunctive. Grouping same-slot
* filters into a single OR'd condition made search answer an impossible
* predicate with a full page while the document list, which ANDs the same
* filters, answered with nothing.
*/
describe('every filter is a conjunct, including two naming the same tag', () => {
it('emits one condition per filter for two filters on the same slot', () => {
const conditions = getStructuredTagFilters(
[
{ tagSlot: 'tag1', fieldType: 'text', operator: 'eq', value: 'a' },
@@ -161,11 +175,62 @@ describe('getStructuredTagFilters', () => {
],
embeddingTable
)
expect(conditions).toHaveLength(1)
const joined = (conditions[0] as unknown as { values: unknown[] }).values[0] as {
toSQL: () => { sql: string; params: unknown[] }
}
expect(joined.toSQL().params).toEqual(['tag1', 'a', 'tag1', 'b'])
expect(conditions).toHaveLength(2)
expect(render(conditions[0]).params).toEqual(['tag1', 'a'])
expect(render(conditions[1]).params).toEqual(['tag1', 'b'])
})
it('keeps an impossible same-tag range as two conditions rather than a union', () => {
const conditions = getStructuredTagFilters(
[
{ tagSlot: 'number1', fieldType: 'number', operator: 'gte', value: '9' },
{ tagSlot: 'number1', fieldType: 'number', operator: 'lte', value: '2' },
],
embeddingTable
)
expect(conditions).toHaveLength(2)
expect(render(conditions[0]).sql).toBe('? >= ?')
expect(render(conditions[1]).sql).toBe('? <= ?')
expect(conditions.every((condition) => !render(condition).sql.includes('OR'))).toBe(true)
})
it('still emits one condition per filter across different slots', () => {
const conditions = getStructuredTagFilters(
[
{ tagSlot: 'tag1', fieldType: 'text', operator: 'eq', value: 'a' },
{ tagSlot: 'number1', fieldType: 'number', operator: 'gte', value: '9' },
{ tagSlot: 'boolean1', fieldType: 'boolean', operator: 'eq', value: 'true' },
],
embeddingTable
)
expect(conditions).toHaveLength(3)
})
})
/**
* The document list builds one predicate per filter and pushes each into a
* single `and(...)`. Search must yield the same number of conjuncts for the
* same filters, or the two surfaces answer different questions over one tag
* vocabulary.
*/
describe('agreement with the document-list surface', () => {
it('produces the same number of conjuncts as the document-list builder', () => {
const filters: StructuredFilter[] = [
{ tagSlot: 'number1', fieldType: 'number', operator: 'gte', value: '9' },
{ tagSlot: 'number1', fieldType: 'number', operator: 'lte', value: '2' },
]
const listConditions = filters.map((filter) =>
buildTagFilterCondition({
tagSlot: filter.tagSlot,
fieldType: 'number',
operator: filter.operator,
value: filter.value,
})
)
expect(listConditions.every((condition) => condition !== undefined)).toBe(true)
expect(getStructuredTagFilters(filters, embeddingTable)).toHaveLength(listConditions.length)
})
})
})
+17 -33
View File
@@ -282,9 +282,18 @@ function buildFilterCondition(filter: StructuredFilter, embeddingTable: any) {
}
/**
* Build SQL conditions from structured filters with operator support
* - Same tag multiple times: OR logic
* - Different tags: AND logic
* Build SQL conditions from structured filters with operator support. Every
* filter is a conjunct, including two that name the same tag.
*
* Search used to group filters by slot and OR same-slot conditions together,
* which made the two surfaces over the same tag vocabulary answer different
* questions: the document list ANDs every filter, so `gte 9` plus `lte 2` on one
* number tag returned nothing there and a full page of results from search
* a widening on the billed endpoint, the same failure mode as dropping a filter.
* OR also made a range on a single text tag (`contains A` and `contains B`)
* inexpressible, while the union it produced stays reachable as separate
* searches. Neither contract ever documented the OR, so no caller could have
* been relying on it deliberately.
*
* Every filter reaching here has already been validated, so one that fails to
* compile is a defect rather than a predicate to skip. Skipping it dropped the
@@ -293,36 +302,11 @@ function buildFilterCondition(filter: StructuredFilter, embeddingTable: any) {
* paid for the widened scan. It is reported as a validation failure instead.
*/
export function getStructuredTagFilters(filters: StructuredFilter[], embeddingTable: any) {
// Group filters by tagSlot
const filtersBySlot = new Map<string, StructuredFilter[]>()
for (const filter of filters) {
const slot = filter.tagSlot
if (!filtersBySlot.has(slot)) {
filtersBySlot.set(slot, [])
}
filtersBySlot.get(slot)!.push(filter)
}
// Build conditions: OR within same slot, AND across different slots
const conditions: ReturnType<typeof sql>[] = []
for (const [, slotFilters] of filtersBySlot) {
const slotConditions = slotFilters.map((f) => {
const condition = buildFilterCondition(f, embeddingTable)
if (condition === null) throw uncompilableTagFilterError(f)
return condition
})
if (slotConditions.length === 1) {
// Single condition for this slot
conditions.push(slotConditions[0])
} else {
// Multiple conditions for same slot - OR them together
conditions.push(sql`(${sql.join(slotConditions, sql` OR `)})`)
}
}
return conditions
return filters.map((filter) => {
const condition = buildFilterCondition(filter, embeddingTable)
if (condition === null) throw uncompilableTagFilterError(filter)
return condition
})
}
/**
@@ -48,11 +48,20 @@ export interface GetPublicLogResult {
* folder means the caller's own tree is inconsistent; it is wrong for a
* diagnostic log read, where the run may long outlive the folder it ran in and a
* 500 would withhold the whole run over one unresolvable field.
*
* `workflowExists` is the join, not the folder: the log's `workflow_id` is set
* null when the workflow is deleted, so the left join yields a null `folderId`
* that is indistinguishable from a workflow sitting at the workspace root. Left
* unseparated, a run whose workflow is gone reports the root path next to
* `deleted: true` a path the caller can hand back to `folderPaths` as a filter
* for a workflow that is no longer in any folder at all.
*/
function publicLogFolderPath(
pathById: ReadonlyMap<string, string>,
folderId: string | null
folderId: string | null,
workflowExists: boolean
): string | null {
if (!workflowExists) return null
if (!folderId) return ROOT_FOLDER_PATH
return pathById.get(folderId) ?? null
}
@@ -90,7 +99,11 @@ export const getPublicLog = defineAuthorizedWorkspaceUseCase({
}
return {
log: { ...log, workflowState: sanitizeExecutionSnapshotState(log.workflowState) },
workflowFolderPath: publicLogFolderPath(folderIndex.pathById, log.workflowFolderId),
workflowFolderPath: publicLogFolderPath(
folderIndex.pathById,
log.workflowFolderId,
log.workflowName !== null
),
executionData,
}
},
@@ -101,6 +101,8 @@ const log = {
executionId: 'run-1',
workspaceId: 'workspace-1',
workflowId: 'workflow-1',
/** Null exactly when the left join found no workflow row — the delete signal. */
workflowName: 'Support triage',
workflowFolderId: 'folder-1',
workflowUserId: 'owner-1',
workflowOwnerEmail: 'owner@example.com',
@@ -196,6 +198,35 @@ describe('public log application use cases', () => {
expect(result.workflowFolderPath).toBeNull()
})
/**
* A deleted workflow nulls the log's `workflow_id`, so the left join returns a
* null folder that is shape-identical to a workflow sitting at the root. Read
* as the root, the run reports `/` beside `deleted: true` and hands the caller
* a `folderPaths` value for a workflow that is in no folder at all.
*/
it('reports no folder path for a run whose workflow has been deleted', async () => {
mocks.getLogScope.mockResolvedValueOnce({
executionId: 'run-1',
workspaceId: 'workspace-1',
workflowId: null,
})
mocks.getLog.mockResolvedValueOnce({
...log,
workflowId: null,
workflowName: null,
workflowFolderId: null,
workflowUserId: null,
workflowOwnerEmail: null,
})
const result = await getPublicLog.execute({
principal: workspacePrincipal,
input: { runId: 'run-1' },
})
expect(result.workflowFolderPath).toBeNull()
})
it('redacts credential values from the run snapshot', async () => {
mocks.getLog.mockResolvedValueOnce({
...log,
@@ -257,6 +257,24 @@ describe('MCP server application use cases', () => {
expect(mocks.discoverServerTools).not.toHaveBeenCalled()
})
it('answers a disabled server with a conflict instead of an untyped fault', async () => {
mocks.getServer.mockResolvedValueOnce({ ...server, enabled: false })
await expect(
discoverMcpServerToolsUseCase.execute({
principal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' },
input: { workspaceId: workspace.workspaceId, serverId: server.id },
})
).rejects.toMatchObject({ code: 'conflict' })
/**
* Discovery loads its configuration through a query that filters on
* `enabled`, so reaching it raised a plain `Error` rendered as a 500 for
* a documented registration value and stamped a bogus failure on the row.
*/
expect(mocks.discoverServerTools).not.toHaveBeenCalled()
})
it('discovers one server tools for the acting subject, honouring refresh', async () => {
const tools = [
{
+14
View File
@@ -159,6 +159,20 @@ export const discoverMcpServerToolsUseCase = defineAuthorizedWorkspaceUseCase({
resolveServerContext(input.workspaceId, input.serverId),
authorizationOptions,
async execute({ principal, input, context }) {
/**
* `enabled: false` is a documented registration value, but discovery loads
* its configuration through a query that filters on `enabled`, so a
* disabled server surfaced as an untyped "not accessible" fault rendered
* as a Sim-side 500 and stamped a bogus failure on the row on the way
* out. It is a state conflict the caller resolves by enabling the server.
*/
if (!context.server.enabled) {
throw new OrchestrationError(
'conflict',
'The MCP server is disabled; enable it before listing its tools'
)
}
const tools = await mcpService.discoverServerTools(
requirePrincipalSubjectUserId(principal),
context.server.id,
@@ -167,6 +167,81 @@ describe('MCP server lifecycle orchestration', () => {
)
})
it('resets the connection when a headers server rotates the headers that authenticate it', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([
{
url: 'https://example.com/mcp',
authType: 'headers',
headers: { authorization: 'Bearer original' },
oauthClientId: null,
oauthClientSecret: null,
},
])
dbChainMockFns.returning.mockResolvedValueOnce([
{
id: 'server-1',
workspaceId: 'workspace-1',
name: 'Example',
transport: 'streamable-http',
url: 'https://example.com/mcp',
authType: 'headers',
},
])
const result = await performUpdateMcpServer({
workspaceId: 'workspace-1',
userId: 'user-1',
serverId: 'server-1',
headers: { authorization: 'Bearer rotated' },
})
expect(result.success).toBe(true)
expect(dbChainMockFns.set).toHaveBeenCalledWith(
expect.objectContaining({
headers: { authorization: 'Bearer rotated' },
connectionStatus: 'disconnected',
lastConnected: null,
lastError: null,
})
)
// Rotating headers invalidates nothing OAuth holds, so the grant survives.
expect(mockRevokeOauthTokens).not.toHaveBeenCalled()
})
it('leaves the connection alone when a headers rewrite changes nothing', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([
{
url: 'https://example.com/mcp',
authType: 'headers',
headers: { authorization: 'Bearer original' },
oauthClientId: null,
oauthClientSecret: null,
},
])
dbChainMockFns.returning.mockResolvedValueOnce([
{
id: 'server-1',
workspaceId: 'workspace-1',
name: 'Example',
transport: 'streamable-http',
url: 'https://example.com/mcp',
authType: 'headers',
},
])
const result = await performUpdateMcpServer({
workspaceId: 'workspace-1',
userId: 'user-1',
serverId: 'server-1',
headers: { authorization: 'Bearer original' },
})
expect(result.success).toBe(true)
expect(dbChainMockFns.set).not.toHaveBeenCalledWith(
expect.objectContaining({ connectionStatus: 'disconnected' })
)
})
it('audits only the columns an edit wrote, not the params it was handed', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([
{
@@ -278,6 +353,64 @@ describe('MCP server lifecycle orchestration', () => {
)
})
it('stores an explicit retries of 0 rather than folding it into the default', async () => {
mockGenerateMcpServerId.mockReturnValue('server-1')
dbChainMockFns.limit.mockResolvedValueOnce([])
dbChainMockFns.limit.mockResolvedValueOnce([
{
id: 'server-1',
workspaceId: 'workspace-1',
name: 'Example',
transport: 'streamable-http',
url: 'https://example.com/anything',
authType: 'headers',
},
])
const result = await performCreateMcpServer({
workspaceId: 'workspace-1',
userId: 'user-1',
name: 'Example',
url: 'https://example.com/anything',
authType: 'headers',
retries: 0,
})
expect(result.success).toBe(true)
expect(dbChainMockFns.values).toHaveBeenCalledWith(expect.objectContaining({ retries: 0 }))
})
it('keeps an explicit auth type when an OAuth client ID is also supplied', async () => {
mockGenerateMcpServerId.mockReturnValue('server-1')
dbChainMockFns.limit.mockResolvedValueOnce([])
dbChainMockFns.limit.mockResolvedValueOnce([
{
id: 'server-1',
workspaceId: 'workspace-1',
name: 'Example',
transport: 'streamable-http',
url: 'https://example.com/anything',
authType: 'headers',
},
])
const result = await performCreateMcpServer({
workspaceId: 'workspace-1',
userId: 'user-1',
name: 'Example',
url: 'https://example.com/anything',
authType: 'headers',
oauthClientId: 'client-1',
oauthClientIdProvided: true,
})
expect(result.success).toBe(true)
expect(result.authType).toBe('headers')
expect(dbChainMockFns.values).toHaveBeenCalledWith(
expect.objectContaining({ authType: 'headers', oauthClientId: 'client-1' })
)
})
it('leaves a re-registered server disconnected until discovery re-runs', async () => {
mockGenerateMcpServerId.mockReturnValue('server-1')
dbChainMockFns.limit.mockResolvedValueOnce([
@@ -142,7 +142,13 @@ export async function createMcpServer(
const transport = params.transport || 'streamable-http'
const timeout = params.timeout || 30000
const retries = params.retries || 3
/**
* `0` is a published, in-range value meaning "no retry", so the default may
* only apply when the field is absent. `||` folded it into `3`, which is the
* opposite of what the caller asked for; the update path already guards on
* `!== undefined`.
*/
const retries = params.retries ?? 3
const enabled = params.enabled !== false
const serverId = params.url ? generateMcpServerId(params.workspaceId, params.url) : generateId()
@@ -195,7 +201,14 @@ export async function createMcpServer(
}
}
}
if (params.oauthClientId) resolvedAuthType = 'oauth'
/**
* An OAuth client id only *implies* an auth type; it may not overrule one
* the caller stated. Unconditional promotion turned an explicit
* `authType: 'headers'` into `oauth`, so the caller's own header
* configuration was never used to authenticate. The update path already
* promotes only when `authType` is absent.
*/
if (!params.authType && params.oauthClientId) resolvedAuthType = 'oauth'
if (existingServer) {
const credsChanged = await oauthCredsChanged({
@@ -378,6 +391,7 @@ export async function updateMcpServer(
.select({
url: mcpServers.url,
authType: mcpServers.authType,
headers: mcpServers.headers,
oauthClientId: mcpServers.oauthClientId,
oauthClientSecret: mcpServers.oauthClientSecret,
})
@@ -415,8 +429,26 @@ export async function updateMcpServer(
// Turning OAuth off must revoke and delete its now-orphaned tokens, not just reset the connection.
const oauthDisabled = currentServer.authType === 'oauth' && resolvedAuthType !== 'oauth'
const shouldClearOauth = urlChanged || credsChanged || oauthDisabled
/**
* On a `headers` server the headers *are* the credential, so rotating them
* invalidates the connection exactly as an OAuth credential change does
* and the registration path already counts headers as a connection input.
* The reset is scoped to that auth type: under `oauth` (or `none`) the
* headers authenticate nothing, and clearing an OAuth server's status
* strands it, since discovery only reruns for an OAuth row that is
* `connected`. Header revocation never revokes the OAuth grant, so this
* stays out of `shouldClearOauth`.
*/
const headersInvalidateAuth =
resolvedAuthType === 'headers' &&
params.headers !== undefined &&
!isEqual(currentServer.headers ?? {}, params.headers)
// An auth-type flip (either direction) or OAuth creds/URL change invalidates the connection: reset and clear stale state.
if (authTypeChanged || (shouldClearOauth && resolvedAuthType === 'oauth')) {
if (
authTypeChanged ||
headersInvalidateAuth ||
(shouldClearOauth && resolvedAuthType === 'oauth')
) {
updateData.connectionStatus = 'disconnected'
updateData.lastConnected = null
updateData.lastError = null
@@ -111,6 +111,29 @@ const table: TableDefinition = {
createdAt: new Date('2026-08-01T00:00:00.000Z'),
updatedAt: new Date('2026-08-01T00:00:00.000Z'),
}
/** An enrichment group stores `workflowId: ''` — there is no workflow to resolve. */
const enrichmentGroup: WorkflowGroup = {
id: 'group-enrichment',
workflowId: '',
enrichmentId: 'company-domain',
type: 'enrichment',
outputs: [{ blockId: '', path: '', outputId: 'domain', columnName: 'column-domain' }],
}
const enrichmentTable: TableDefinition = {
...table,
schema: {
columns: [
{ id: 'column-name', name: 'name', type: 'string' },
{
id: 'column-domain',
name: 'domain',
type: 'string',
workflowGroupId: 'group-enrichment',
},
],
workflowGroups: [enrichmentGroup],
},
}
const principal = {
kind: 'delegated' as const,
serviceId: 'copilot' as const,
@@ -151,6 +174,37 @@ function tableWithGroup(nextGroup: WorkflowGroup, columns = table.schema.columns
}
}
/**
* Points the command at the enrichment table and a registry entry that defines a
* second output, so an extension has something valid to ask for.
*/
function useEnrichmentTable(): void {
mocks.resolveContext.mockResolvedValue({
tableId: table.id,
table: enrichmentTable,
workspaceId: table.workspaceId,
workspaceOrganizationId: null,
allowPersonalApiKeys: true,
billedAccountUserId: 'billing-owner-1',
})
mocks.getEnrichment.mockReturnValue({
id: 'company-domain',
name: 'Company Domain',
inputs: [{ id: 'company', name: 'Company', type: 'string', required: true }],
outputs: [
{ id: 'domain', name: 'domain', type: 'string' },
{ id: 'company_name', name: 'company name', type: 'string' },
],
})
mocks.updateGroup.mockImplementation(async (input) => ({
...enrichmentTable,
schema: {
...enrichmentTable.schema,
workflowGroups: [{ ...enrichmentGroup, outputs: input.outputs ?? enrichmentGroup.outputs }],
},
}))
}
describe('workflow and enrichment Table application commands', () => {
beforeEach(() => {
vi.clearAllMocks()
@@ -406,6 +460,168 @@ describe('workflow and enrichment Table application commands', () => {
)
})
it('extends an enrichment group with a registry output without resolving a workflow', async () => {
useEnrichmentTable()
await updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: enrichmentGroup.id,
outputs: [
...enrichmentGroup.outputs,
{ blockId: '', path: '', outputId: 'company_name', columnName: 'zz_z' },
],
newOutputColumns: [{ name: 'zz_z', type: 'string' }],
},
})
expect(mocks.resolveWorkflowContext).not.toHaveBeenCalled()
expect(mocks.updateGroup).toHaveBeenCalledWith(
expect.objectContaining({
groupId: enrichmentGroup.id,
newOutputColumns: [{ name: 'zz_z', type: 'string', workflowGroupId: enrichmentGroup.id }],
}),
'request-1'
)
})
it('refuses an enrichment output the registry does not define', async () => {
useEnrichmentTable()
await expect(
updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: enrichmentGroup.id,
outputs: [
...enrichmentGroup.outputs,
{ blockId: '', path: '', outputId: 'invented', columnName: 'zz_z' },
],
newOutputColumns: [{ name: 'zz_z', type: 'string' }],
},
})
).rejects.toMatchObject({
code: 'validation',
message: 'Enrichment "Company Domain" has no output "invented"',
})
expect(mocks.updateGroup).not.toHaveBeenCalled()
expect(mocks.audit).not.toHaveBeenCalled()
})
it('refuses an enrichment output coordinate that carries no registry output id', async () => {
useEnrichmentTable()
await expect(
updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: enrichmentGroup.id,
outputs: [...enrichmentGroup.outputs, { blockId: '', path: 'name', columnName: 'zz_z' }],
newOutputColumns: [{ name: 'zz_z', type: 'string' }],
},
})
).rejects.toMatchObject({
code: 'validation',
message: 'Enrichment "Company Domain" has no output ""',
})
expect(mocks.updateGroup).not.toHaveBeenCalled()
})
it('leaves an untouched enrichment binding alone while renaming the group', async () => {
useEnrichmentTable()
await updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: enrichmentGroup.id,
name: 'Renamed enrichment',
outputs: enrichmentGroup.outputs,
},
})
expect(mocks.getEnrichment).not.toHaveBeenCalled()
expect(mocks.updateGroup).toHaveBeenCalledWith(
expect.objectContaining({ name: 'Renamed enrichment' }),
'request-1'
)
})
it('names the enrichment instead of a missing workflow for a mapping update', async () => {
useEnrichmentTable()
await expect(
updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: enrichmentGroup.id,
mappingUpdates: [{ columnName: 'column-domain', blockId: 'block-1', path: 'content' }],
},
})
).rejects.toMatchObject({
code: 'validation',
message: 'Mapping updates are not supported for an enrichment group; send outputs[] instead',
})
expect(mocks.resolveWorkflowContext).not.toHaveBeenCalled()
expect(mocks.updateGroup).not.toHaveBeenCalled()
})
it('still resolves the workflow for a new output coordinate on a manual group', async () => {
await updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: group.id,
outputs: [...group.outputs, { blockId: 'block-2', path: 'score', columnName: 'score' }],
newOutputColumns: [{ name: 'score', type: 'number' }],
},
})
expect(mocks.resolveWorkflowContext).toHaveBeenCalledWith({
workflowId: 'workflow-1',
assertedWorkspaceId: 'workspace-1',
})
expect(mocks.getEnrichment).not.toHaveBeenCalled()
expect(mocks.updateGroup).toHaveBeenCalledWith(
expect.objectContaining({
newOutputColumns: [{ name: 'score', type: 'number', workflowGroupId: group.id }],
}),
'request-1'
)
})
it('refuses an output column no output names instead of dropping it', async () => {
await expect(
updateTableGroupUseCase.execute({
principal,
input: {
tableId: table.id,
workspaceId: table.workspaceId,
groupId: group.id,
newOutputColumns: [{ name: 'zz_w', type: 'string' }],
},
})
).rejects.toMatchObject({
code: 'validation',
message: 'newOutputColumns entry "zz_w" has no matching outputs[].columnName',
})
expect(mocks.updateGroup).not.toHaveBeenCalled()
})
it('does not start auto-run when the generic update saves a legacy enabled group', async () => {
await updateTableGroupUseCase.execute({
principal,
+103 -20
View File
@@ -37,6 +37,7 @@ import { resolveActiveWorkflowApplicationContext } from '@/lib/workflows/applica
import type { ResolveWorkflowOutputsResult } from '@/lib/workflows/application/resolve-workflow-outputs'
import { loadResolvedWorkflowOutputs } from '@/lib/workflows/application/resolve-workflow-outputs'
import { getEnrichment } from '@/enrichments/registry'
import type { EnrichmentConfig } from '@/enrichments/types'
const logger = createLogger('TableGroupApplication')
@@ -120,6 +121,38 @@ function validateRequestedOutputs(
)
}
/** Resolves the registry enrichment a group is bound to, refusing an unknown id. */
function requireEnrichment(enrichmentId: string | undefined): EnrichmentConfig {
const enrichment = getEnrichment(enrichmentId)
if (!enrichment) {
throw new OrchestrationError(
'validation',
`Unknown enrichment "${enrichmentId ?? ''}". Call list_enrichments to see available ids.`
)
}
return enrichment
}
/**
* Refuses an output id the enrichment registry does not define. A run fills a
* cell by reading `result[outputId]`, so a coordinate carrying an unknown or
* absent output id names a column no run can ever write.
*/
function requireKnownEnrichmentOutputIds(
enrichment: EnrichmentConfig,
outputIds: Array<string | undefined>
): void {
const known = new Set(enrichment.outputs.map((output) => output.id))
for (const outputId of outputIds) {
if (!outputId || !known.has(outputId)) {
throw new OrchestrationError(
'validation',
`Enrichment "${enrichment.name}" has no output "${outputId ?? ''}"`
)
}
}
}
function workflowOutputColumnType(
requestedType: string | undefined,
resolvedLeafType: string | undefined
@@ -413,13 +446,7 @@ export const createTableEnrichmentGroup = defineAuthorizedTableUseCase({
`Enrichment output names cannot exceed ${TABLE_LIMITS.MAX_COLUMNS_PER_TABLE} entries`
)
}
const enrichment = getEnrichment(input.enrichmentId)
if (!enrichment) {
throw new OrchestrationError(
'validation',
`Unknown enrichment "${input.enrichmentId}". Call list_enrichments to see available ids.`
)
}
const enrichment = requireEnrichment(input.enrichmentId)
const enrichmentInputIds = new Set(
enrichment.inputs.map((enrichmentInput) => enrichmentInput.id)
@@ -440,15 +467,7 @@ export const createTableEnrichmentGroup = defineAuthorizedTableUseCase({
}
mappingByInput.set(mapping.inputName, mapping.columnName)
}
const enrichmentOutputIds = new Set(enrichment.outputs.map((output) => output.id))
for (const outputId of Object.keys(input.outputColumnNames ?? {})) {
if (!enrichmentOutputIds.has(outputId)) {
throw new OrchestrationError(
'validation',
`Enrichment "${enrichment.name}" has no output "${outputId}"`
)
}
}
requireKnownEnrichmentOutputIds(enrichment, Object.keys(input.outputColumnNames ?? {}))
const existingColumns = new Set(context.table.schema.columns.map((column) => column.name))
for (const enrichmentInput of enrichment.inputs) {
const mapped = mappingByInput.get(enrichmentInput.id)
@@ -567,15 +586,79 @@ export const updateTableGroupUseCase = defineAuthorizedTableUseCase({
const previousGroup = (context.table.schema.workflowGroups ?? []).find(
(group) => group.id === input.groupId
)
/**
* An enrichment group's outputs come from the registry, not from a workflow,
* and it stores `workflowId: ''` so there is nothing to resolve a new
* output coordinate against. Validating one anyway resolved the empty id and
* answered `404 Workflow not found`, which made an enrichment group's output
* set permanently unextendable. Only a body that supplies a `workflowId`
* converts the group to workflow-backed and needs workflow metadata.
*/
const producerIsEnrichment =
input.workflowId === undefined &&
previousGroup !== undefined &&
(previousGroup.type === 'enrichment' || !previousGroup.workflowId)
/**
* Skipping workflow resolution must not mean skipping validation. An
* enrichment run fills a column by registry `outputId`, so a coordinate the
* registry does not define is a column nothing can ever populate. Hold an
* added or repointed output to the same check enrichment creation
* applies, and leave an untouched existing binding alone so a group whose
* enrichment has since changed stays editable.
*/
if (producerIsEnrichment && input.outputs?.length) {
const boundOutputKeys = new Set(
previousGroup?.outputs.map((output) => `${output.columnName}::${output.outputId ?? ''}`) ??
[]
)
const addedOutputs = input.outputs.filter(
(output) => !boundOutputKeys.has(`${output.columnName}::${output.outputId ?? ''}`)
)
if (addedOutputs.length > 0) {
requireKnownEnrichmentOutputIds(
requireEnrichment(previousGroup?.enrichmentId),
addedOutputs.map((output) => output.outputId)
)
}
}
/**
* `mappingUpdates` repoints a column at a new `(blockId, path)` coordinates
* an enrichment output does not have and the writer cannot translate into an
* `outputId`. Say that, rather than resolving the group's empty workflow id
* and answering `404 Workflow not found`.
*/
if (producerIsEnrichment && input.mappingUpdates?.length) {
throw new OrchestrationError(
'validation',
'Mapping updates are not supported for an enrichment group; send outputs[] instead'
)
}
/**
* A `newOutputColumns` entry that no resulting output names is dropped by the
* writer, so a caller asking for a column got a 200 and no column. Refuse it
* the way group creation refuses an orphan `outputColumns` entry.
*/
if (input.newOutputColumns?.length) {
const requestedOutputNames = new Set((input.outputs ?? []).map((output) => output.columnName))
const orphan = input.newOutputColumns.find((column) => !requestedOutputNames.has(column.name))
if (orphan) {
throw new OrchestrationError(
'validation',
`newOutputColumns entry "${orphan.name}" has no matching outputs[].columnName`
)
}
}
const previousOutputKeys = new Set(
previousGroup?.outputs.map((output) => `${output.blockId}::${output.path}`) ?? []
)
const workflowChanged =
input.workflowId !== undefined && input.workflowId !== previousGroup?.workflowId
const outputCoordinatesToValidate =
input.outputs?.filter(
(output) => workflowChanged || !previousOutputKeys.has(`${output.blockId}::${output.path}`)
) ?? []
const outputCoordinatesToValidate = producerIsEnrichment
? []
: (input.outputs?.filter(
(output) =>
workflowChanged || !previousOutputKeys.has(`${output.blockId}::${output.path}`)
) ?? [])
const workflowMetadataRequired =
input.workflowId !== undefined ||
outputCoordinatesToValidate.length > 0 ||
+91
View File
@@ -408,6 +408,42 @@ describe('view config column-reference normalization', () => {
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
})
/**
* The layout half used to answer 200 and store a name no read ever shows,
* while the same unknown name in `filter` answered 400 one request, two
* policies.
*/
it('refuses a hidden column that does not exist for a strict caller', async () => {
queueTableRows(tableViews, [{ total: 0 }])
dbChainMockFns.returning.mockResolvedValueOnce([storedRow])
await expect(create({ hiddenColumns: ['ghost'] })).rejects.toMatchObject({
name: 'TableViewValidationError',
})
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
})
it('refuses an unknown column in every other layout key too', async () => {
for (const config of [
{ columnOrder: ['ghost'] },
{ pinnedColumns: ['ghost'] },
{ columnWidths: { ghost: 120 } },
]) {
queueTableRows(tableViews, [{ total: 0 }])
await expect(create(config)).rejects.toMatchObject({ name: 'TableViewValidationError' })
}
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
})
it('keeps storing a first-party layout reference that no longer resolves', async () => {
queueTableRows(tableViews, [{ total: 0 }])
dbChainMockFns.returning.mockResolvedValueOnce([storedRow])
await create({ hiddenColumns: ['col_gone'] }, false)
expect(insertedConfig().hiddenColumns).toEqual(['col_gone'])
})
it('refuses a sort on a column that does not exist for a strict caller', async () => {
queueTableRows(tableViews, [{ total: 0 }])
dbChainMockFns.returning.mockResolvedValueOnce([storedRow])
@@ -511,6 +547,61 @@ describe('view config column-reference normalization', () => {
).rejects.toMatchObject({ name: 'TableViewValidationError' })
})
/**
* The carried-forward exemption exists so a dangling FILTER ref stays
* writable, not so it becomes a valid target for a NEW layout ref. Without
* scoping, a strict caller could store `hiddenColumns: ['col_gone']` purely
* because `col_gone` survives in the stored filter a layout entry the very
* next read drops, which is the asymmetry the strict check closes.
*/
it('refuses a NEW layout reference that resolves only via a carried-forward filter ref', async () => {
const stale = { all: [{ field: 'col_gone', op: 'eq' as const, value: 'x' }] }
queueTableRows(tableViews, [{ ...storedRow, config: { filter: stale } }])
dbChainMockFns.returning.mockResolvedValueOnce([storedRow])
await expect(
updateTableView({
viewId: 'view-1',
tableId: 'table-1',
config: { filter: stale, hiddenColumns: ['col_gone'] },
columns,
strictRefs: true,
})
).rejects.toMatchObject({ name: 'TableViewValidationError' })
expect(dbChainMockFns.update).not.toHaveBeenCalled()
})
it('still lets a strict save carry the stale filter reference forward with a valid layout', async () => {
const stale = { all: [{ field: 'col_gone', op: 'eq' as const, value: 'x' }] }
queueTableRows(tableViews, [{ ...storedRow, config: { filter: stale } }])
dbChainMockFns.returning.mockResolvedValueOnce([storedRow])
await expect(
updateTableView({
viewId: 'view-1',
tableId: 'table-1',
config: { filter: stale, hiddenColumns: ['col_a'] },
columns,
strictRefs: true,
})
).resolves.not.toBeNull()
})
it('leaves the non-strict grid path free to save that same layout reference', async () => {
const stale = { all: [{ field: 'col_gone', op: 'eq' as const, value: 'x' }] }
queueTableRows(tableViews, [{ ...storedRow, config: { filter: stale } }])
dbChainMockFns.returning.mockResolvedValueOnce([storedRow])
await expect(
updateTableView({
viewId: 'view-1',
tableId: 'table-1',
config: { filter: stale, hiddenColumns: ['col_gone'] },
columns,
})
).resolves.not.toBeNull()
})
it('accepts that same new reference from a first-party caller', async () => {
const stale = { all: [{ field: 'col_gone', op: 'eq' as const, value: 'x' }] }
queueTableRows(tableViews, [{ ...storedRow, config: { filter: stale } }])
+66 -8
View File
@@ -177,10 +177,19 @@ function tolerantColumns(
*
* `filter` and `sort` are then validated: a predicate or sort naming no column
* is one the query routes answer 400 for, so storing it would save a view that
* can never load. Column LAYOUT is deliberately not validated it auto-saves as
* can never load. Column LAYOUT is not validated by default it auto-saves as
* the user drags, and racing a concurrent column delete must self-heal through
* {@link pruneViewConfig}, not fail the drag.
*
* `strictLayoutRefs` extends the same refusal to the LAYOUT keys, for the same
* caller that gets it on `filter` and `sort`: one request naming an unknown
* column twice answered 400 for the filter leaf and 200 for the hidden column,
* whose name was then stored in a blob no read ever shows (`pruneViewConfig`
* drops it) and would silently take effect if a column of that name were later
* created. It stays off for the first-party grid, whose layout auto-saves as the
* user drags and must self-heal past a concurrent column delete. A v2 caller
* cannot carry a dangling layout ref forward: the read it echoes is pruned.
*
* `carriedForward` names the references that are exempt from that refusal.
* Deleting a column leaves every view that filtered on it dangling
* `pruneViewConfig` deliberately does not prune a filter so without the
@@ -191,11 +200,20 @@ function tolerantColumns(
* already held, so a reference the caller INTRODUCES is refused; a first-party
* caller exempts its own refs too, which is the behavior the grid has always
* had see {@link CreateTableViewData.strictRefs}.
*
* The exemption is scoped to `filter` and `sort`, the only halves it exists for:
* the layout check runs against the LIVE columns, not the tolerant set. A
* carried-forward placeholder standing in for a deleted column named in the
* stored filter must not also make that name a valid target for a new layout
* ref, which `pruneViewConfig` would drop on the very next read the same
* write-accepts / read-discards asymmetry `strictLayoutRefs` closes. Validating
* against `columns` makes the strict path accept exactly what a read keeps.
*/
export function normalizeViewConfigForStorage(
config: TableViewConfig,
columns: ColumnDefinition[],
carriedForward: readonly string[] = []
carriedForward: readonly string[] = [],
strictLayoutRefs = false
): TableViewConfig {
const stored = remapViewConfigColumnRefs(config, viewConfigRefMap(columns))
const known = tolerantColumns(columns, carriedForward)
@@ -208,9 +226,38 @@ export function normalizeViewConfigForStorage(
}
throw error
}
if (strictLayoutRefs) assertLayoutRefsResolve(stored, columns)
return stored
}
/** The layout keys, paired with the references each one holds. */
function layoutRefEntries(config: TableViewConfig): Array<[string, readonly string[]]> {
return [
['columnOrder', config.columnOrder ?? []],
['pinnedColumns', config.pinnedColumns ?? []],
['hiddenColumns', config.hiddenColumns ?? []],
['columnWidths', Object.keys(config.columnWidths ?? {})],
]
}
/**
* Refuses a layout reference naming no column. Mirrors what the query
* validators do for `filter` and `sort`, including the message shape, so one
* config gets one answer whichever half the unknown name landed in.
*/
function assertLayoutRefsResolve(config: TableViewConfig, known: ColumnDefinition[]): void {
const live = new Set(known.map(getColumnId))
for (const [key, refs] of layoutRefEntries(config)) {
for (const ref of refs) {
if (!live.has(ref)) {
throw new TableViewValidationError(
`Unknown ${key} column "${ref}". It is not a column on this table.`
)
}
}
}
}
/**
* Migrates a config stored before the grammar switch. The feature never
* released, so legacy-shaped rows exist only from pre-refactor testing: a
@@ -371,9 +418,9 @@ export interface CreateTableViewData {
userId: string
columns: ColumnDefinition[]
/**
* Whether to refuse a filter or sort reference naming no live column. Set by
* the `/api/v2` surface only, whose caller authored the config in this request
* and can be told which reference was wrong.
* Whether to refuse a filter, sort, or column-layout reference naming no live
* column. Set by the `/api/v2` surface only, whose caller authored the config
* in this request and can be told which reference was wrong.
*
* Absent the first-party grid, which does not author these refs so much as
* carry them: a view filtered on a since-deleted column keeps the dangling
@@ -405,7 +452,8 @@ export async function createTableView(data: CreateTableViewData): Promise<TableV
const config = normalizeViewConfigForStorage(
data.config,
data.columns,
data.strictRefs ? [] : configColumnRefs(data.config)
data.strictRefs ? [] : configColumnRefs(data.config),
data.strictRefs === true
)
const row = await withTableViewsLock(data.tableId, async (trx) => {
@@ -500,11 +548,21 @@ export async function updateTableView(data: UpdateTableViewData): Promise<TableV
const config =
data.config === undefined
? undefined
: normalizeViewConfigForStorage(data.config, data.columns, carriedForward)
: normalizeViewConfigForStorage(
data.config,
data.columns,
carriedForward,
data.strictRefs === true
)
const configPatch =
data.configPatch === undefined
? undefined
: normalizeViewConfigForStorage(data.configPatch, data.columns, carriedForward)
: normalizeViewConfigForStorage(
data.configPatch,
data.columns,
carriedForward,
data.strictRefs === true
)
const patch: Partial<typeof tableViews.$inferInsert> = { updatedAt: new Date() }
if (data.name !== undefined) patch.name = normalizeName(data.name)