From 3645857ddb3be863eb2e8ef02f7b3b4bef23f58d Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 25 Aug 2026 21:02:59 -0700 Subject: [PATCH] fix(v2): stop six responses reporting less than the layer beneath them knew (#7097) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(v2): stop six responses reporting less than the layer beneath them knew Findings from cubic's review of the v0.8.12 release PR, all on the v2 surface. - `GET /workflows/{id}/runs/{runId}` documented `includeFileBase64` as requiring `includeOutput`, and the read honours that — files are projected inside the `includeOutput` branch alone. Nothing enforced it, so the flag parsed, was accepted, and was then dropped: a 200 carrying no files and no reason why. Now a 400 naming the missing flag, matching how `GET /billing/logs` refuses a window bound its period will not read. - `POST /files/{id}/unzip` rendered a malformed or over-cap archive as 500. `ArchiveError` had no arm in the v2 policy, though the internal extract route beside it has mapped the same failures to 400/413 all along. - Both workflow-MCP lists cut their tool inventory at a ceiling and published `nextCursor: null` regardless. The use cases were already returning `truncated`; only the v2 presenters dropped it, while the copilot handler published it. A reconciling caller read a partial set as the complete one. - A table dispatch scoped by filter reported neither the filter nor its exclusions, and `rowIds: undefined` documented itself as "every eligible row" — so a filtered run and an unfiltered one were indistinguishable. The compiled filter stays unpublished, as before; `selectAll` and `excludeRowIds` name the distinction. - `GET /files/uploads/{id}` promised the registered file after finalization and passed null unconditionally, so a caller polling a transfer it lost track of could watch a session reach `completed` and never learn what it created. - `HEAD` on a run file resolved the file downstream of authorization, so it answered 200 for an id the `GET` beside it would 404. - v2 bulk table delete audited `FOLDER_DELETED.resourceId` from the caller-keyed projection, writing a display path where the single-folder delete writes the canonical id. Two further findings needed no change: the audit-log cursor scope is bound by `member_user_id_unique` and a membership check, so the cross-organization replay it described cannot arise; and both documentation findings had already been fixed on staging by #7092. * fix(v2): report a dispatch's two narrowings separately Review follow-up. `selectAll` was set from the stored filter alone, but the run rejects only `rowIds` *with* `excludeRowIds` — so an exclusion set with no filter is a scope a caller can create, and the walk applies it. Those dispatches published exclusions with no discriminator beside them, which is the shape the flag existed to rule out. One flag could not cover both: an exclusion-only scope is neither filtered nor unnarrowed. So the two narrowings are reported as what they are — `filtered` for the unpublished stored predicate, `excludeRowIds` for the deselections — and every combination is now distinguishable from a run over every eligible row. `excludeRowIds` mirrors the walk's own condition and is withheld beside `rowIds`, where the dispatcher would ignore it. * fix(v2): let a failed file read fail, and point truncation at its own check Two review follow-ups. `getWorkspaceFile` logs and returns null on a read failure unless told otherwise, so a transient database error would have reported a finalized upload as fileless — to the one caller polling to learn what it created, who would then stop, having been told there was nothing. Read with `throwOnError` so the failure surfaces and the poll can retry, matching how the sibling record read loads the same row. A file genuinely deleted still answers null, which is a different question with a different answer. The server list pointed callers at `/tools` for an authoritative inventory without saying that endpoint applies the same ceiling. It now names the `truncated` flag this PR added there, so "authoritative" has a condition attached instead of being asserted. --- apps/docs/openapi-v2-resources.json | 18 +++- apps/docs/openapi-v2-tables.json | 13 ++- .../api/v2/files/[fileId]/unzip/route.test.ts | 34 +++++++ .../app/api/v2/files/[fileId]/unzip/route.ts | 2 +- .../api/v2/files/uploads/[uploadId]/route.ts | 2 +- apps/sim/app/api/v2/tables/presenters.test.ts | 61 ++++++++++-- apps/sim/app/api/v2/tables/presenters.ts | 26 +++++ .../[serverId]/tools/route.test.ts | 17 ++++ .../[serverId]/tools/route.ts | 8 +- .../api/v2/workflow-mcp-servers/route.test.ts | 37 +++++++ .../app/api/v2/workflow-mcp-servers/route.ts | 3 +- .../runs/[runId]/files/[fileId]/route.ts | 6 +- .../[workflowId]/runs/[runId]/route.test.ts | 26 +++++ .../lib/api/contracts/v2/openapi/resources.ts | 3 +- apps/sim/lib/api/contracts/v2/tables.ts | 16 ++- .../api/contracts/v2/workflow-mcp-servers.ts | 16 ++- apps/sim/lib/api/contracts/v2/workflows.ts | 21 ++++ .../mcp/application/workflow-deployments.ts | 14 +++ apps/sim/lib/table/application/bulk.test.ts | 37 +++++++ apps/sim/lib/table/application/bulk.ts | 23 ++++- .../upload-session/application.test.ts | 98 ++++++++++++++++++- .../lib/uploads/upload-session/application.ts | 37 ++++++- .../download-workflow-run-file.test.ts | 25 +++++ .../application/download-workflow-run-file.ts | 51 +++++++--- .../lib/workspace-files/api/route-policies.ts | 23 +++++ packages/sim-cli/src/generated/v2-api.ts | 8 ++ 26 files changed, 587 insertions(+), 38 deletions(-) diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index 6a33679caf..bbc64e3043 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -7390,9 +7390,13 @@ } ], "description": "Opaque cursor for the next page. Send it back as `cursor`; `null` means there is nothing further to fetch. Never construct one yourself." + }, + "toolNamesTruncated": { + "type": "boolean", + "description": "Whether `toolCount` and `toolNames` under-report. The names are gathered for the whole page under one ceiling, so a page whose servers publish more tools than that ceiling between them reports only part of each server's inventory. Read `GET /api/v2/workflow-mcp-servers/{serverId}/tools` for one server's inventory and check that response's own `truncated`, which reports the same ceiling applied to a single server — only an untruncated response is the authoritative set. Unrelated to `nextCursor`, which is how this list says there are further servers." } }, - "required": ["data", "nextCursor"], + "required": ["data", "nextCursor", "toolNamesTruncated"], "additionalProperties": false, "title": "List workflow MCP servers response", "description": "A cursor-paginated page of published MCP servers.", @@ -7411,7 +7415,8 @@ "toolNames": ["triage_ticket"] } ], - "nextCursor": null + "nextCursor": null, + "toolNamesTruncated": false } ] }, @@ -7653,9 +7658,13 @@ } ], "description": "Always `null` — this list has no `cursor` or `limit` param and returns its whole bounded set in one page. Present so the list can gain pages later without a shape change." + }, + "truncated": { + "type": "boolean", + "description": "Whether this inventory was cut short by the server-side ceiling on how many tools one response may carry. `nextCursor` is null either way — this list takes no `cursor`, so a truncated set cannot be paged past and this flag is the only way to tell a partial inventory from a complete one. A reconciling caller must not treat a truncated set as the full published inventory." } }, - "required": ["data", "nextCursor"], + "required": ["data", "nextCursor", "truncated"], "additionalProperties": false, "title": "List workflow MCP tools response", "description": "The tools a published MCP server exposes.", @@ -7674,7 +7683,8 @@ "updatedAt": "2026-06-12T10:30:00.000Z" } ], - "nextCursor": null + "nextCursor": null, + "truncated": false } ] }, diff --git a/apps/docs/openapi-v2-tables.json b/apps/docs/openapi-v2-tables.json index 546006fba5..52a8d00198 100644 --- a/apps/docs/openapi-v2-tables.json +++ b/apps/docs/openapi-v2-tables.json @@ -9964,7 +9964,18 @@ "description": "Workflow groups the dispatch runs." }, "rowIds": { - "description": "Explicit rows the dispatch targets; absent means every eligible row.", + "description": "Explicit rows the dispatch targets. Absent means it was given no row list and walks every eligible row, narrowed by `filtered` and `excludeRowIds` when either is present.", + "type": "array", + "items": { + "type": "string" + } + }, + "filtered": { + "description": "Present and true when a stored filter narrows which rows run. The filter itself is not published: it is held compiled, in a different grammar from the predicate the request was written in. Absent means no filter narrows the dispatch — which, with no `rowIds` and no `excludeRowIds`, is what means every eligible row.", + "type": "boolean" + }, + "excludeRowIds": { + "description": "Rows the walk skips. Independent of `filtered`: a dispatch may exclude rows from a filtered set or from every eligible row. Never present alongside `rowIds`, which the run rejects and the walk would ignore.", "type": "array", "items": { "type": "string" diff --git a/apps/sim/app/api/v2/files/[fileId]/unzip/route.test.ts b/apps/sim/app/api/v2/files/[fileId]/unzip/route.test.ts index f623d411b2..d0308db68c 100644 --- a/apps/sim/app/api/v2/files/[fileId]/unzip/route.test.ts +++ b/apps/sim/app/api/v2/files/[fileId]/unzip/route.test.ts @@ -28,6 +28,7 @@ vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock) import { NoWorkspaceAccessError } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { ArchiveError } from '@/lib/uploads/archive' import { POST } from '@/app/api/v2/files/[fileId]/unzip/route' const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8' @@ -165,4 +166,37 @@ describe('POST /api/v2/files/[fileId]/unzip', () => { expect(response.status).toBe(400) }) + + /** + * The cases above all raise `OrchestrationError`, which every v2 policy + * already renders — but the extraction use case does not: a payload it cannot + * parse or that busts a cap raises `ArchiveError`, and with no arm for that + * class the route answered `500`. The internal extract route beside it has + * mapped these all along, so the two disagreed about whose fault a bad + * archive was. + */ + it('renders a malformed archive as 400 rather than a server fault', async () => { + mocks.extract.mockRejectedValueOnce( + new ArchiveError( + 'invalid', + 'Not a valid .zip archive — its central directory could not be parsed.' + ) + ) + + const response = await POST(unzipRequest(), context) + + expect(response.status).toBe(400) + expect((await response.json()).error.message).toContain('valid .zip archive') + }) + + it('renders an over-cap archive as 413 rather than a server fault', async () => { + mocks.extract.mockRejectedValueOnce( + new ArchiveError('too_many_entries', 'Archive has 1001 files; the maximum is 1000.') + ) + + const response = await POST(unzipRequest(), context) + + expect(response.status).toBe(413) + expect((await response.json()).error.message).toContain('maximum is 1000') + }) }) diff --git a/apps/sim/app/api/v2/files/[fileId]/unzip/route.ts b/apps/sim/app/api/v2/files/[fileId]/unzip/route.ts index 6defcf4add..8bfc0efd14 100644 --- a/apps/sim/app/api/v2/files/[fileId]/unzip/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/unzip/route.ts @@ -34,7 +34,7 @@ export const POST = defineV2JsonRoute({ auth: v2ApiKeyAuth, operation: fileOperations.extractArchive, rateLimit: v2RateLimits.publicApi, - errorPolicy: v2FileErrorPolicies.concealResourceAuthorization, + errorPolicy: v2FileErrorPolicies.concealExtractionAuthorization, mapInput: ({ params, body }) => ({ fileId: params.fileId, assertedWorkspaceId: body.workspaceId, diff --git a/apps/sim/app/api/v2/files/uploads/[uploadId]/route.ts b/apps/sim/app/api/v2/files/uploads/[uploadId]/route.ts index 54512fd923..c6ab1327c0 100644 --- a/apps/sim/app/api/v2/files/uploads/[uploadId]/route.ts +++ b/apps/sim/app/api/v2/files/uploads/[uploadId]/route.ts @@ -28,7 +28,7 @@ export const GET = defineV2JsonRoute({ uploadToken: headers['upload-token'], }), useCase: readWorkspaceFileUploadOperation, - present: async (session) => ({ data: await toV2FileUpload(session, null) }), + present: async ({ session, file }) => ({ data: await toV2FileUpload(session, file) }), }) export const DELETE = defineV2JsonRoute({ diff --git a/apps/sim/app/api/v2/tables/presenters.test.ts b/apps/sim/app/api/v2/tables/presenters.test.ts index e5c831c17f..537944feda 100644 --- a/apps/sim/app/api/v2/tables/presenters.test.ts +++ b/apps/sim/app/api/v2/tables/presenters.test.ts @@ -188,12 +188,61 @@ describe('presentV2TableDispatch', () => { expect(presented).not.toHaveProperty('triggeredByUserId') }) - /** The stored scope also carries a compiled filter, which is not public. */ - it('publishes only the addressable half of the run scope', () => { - expect(presentV2TableDispatch(stored).scope).toEqual({ - groupIds: ['group-1'], - rowIds: ['row-1'], - }) + /** + * The stored scope also carries a compiled filter, which stays unpublished — + * it is held in a different grammar from the predicate the request was + * written in. + */ + it('withholds the compiled filter itself', () => { + expect(presentV2TableDispatch(stored).scope).not.toHaveProperty('filter') + }) + + /** + * Withholding the filter must not also withhold the fact that there was one. + * A filtered dispatch has no `rowIds` either, so without `filtered` the + * response described it exactly like a run over every eligible row. + */ + it('says a filtered scope is a filtered scope', () => { + expect( + presentV2TableDispatch({ + ...stored, + scope: { groupIds: ['group-1'], filter: { status: 'open' }, excludeRowIds: ['row-9'] }, + }).scope + ).toEqual({ groupIds: ['group-1'], filtered: true, excludeRowIds: ['row-9'] }) + }) + + /** + * The run rejects only `rowIds` *with* `excludeRowIds`, so exclusions with no + * filter are a scope a caller can really create, and the walk applies them. + * Reporting it as filtered would be false; reporting it bare would be the + * original bug, since it targets every eligible row *except* these. + */ + it('reports exclusions that narrow an otherwise unfiltered run', () => { + expect( + presentV2TableDispatch({ + ...stored, + scope: { groupIds: ['group-1'], excludeRowIds: ['row-9'] }, + }).scope + ).toEqual({ groupIds: ['group-1'], excludeRowIds: ['row-9'] }) + }) + + /** + * The walk ignores exclusions once a row list is given, so publishing them + * beside `rowIds` would describe a narrowing that never happens. + */ + it('withholds exclusions the walk would ignore', () => { + expect( + presentV2TableDispatch({ + ...stored, + scope: { groupIds: ['group-1'], rowIds: ['row-1'], excludeRowIds: ['row-9'] }, + }).scope + ).toEqual({ groupIds: ['group-1'], rowIds: ['row-1'] }) + }) + + /** Nothing narrowing it is the one shape that really does mean every eligible row. */ + it('leaves an unnarrowed scope unmarked', () => { + const scope = presentV2TableDispatch({ ...stored, scope: { groupIds: ['group-1'] } }).scope + expect(scope).toEqual({ groupIds: ['group-1'] }) }) }) diff --git a/apps/sim/app/api/v2/tables/presenters.ts b/apps/sim/app/api/v2/tables/presenters.ts index 726959ba83..d733b576d3 100644 --- a/apps/sim/app/api/v2/tables/presenters.ts +++ b/apps/sim/app/api/v2/tables/presenters.ts @@ -61,9 +61,35 @@ export function presentV2TableDispatch(dispatch: DispatchRow): V2TableRunDispatc */ status: dispatch.status === 'cancelled' ? 'canceled' : dispatch.status, mode: dispatch.mode, + /** + * A dispatch with no row list narrows what it walks by a compiled filter, + * an exclusion set, or both, and the dispatcher applies each independently. + * Publishing only `groupIds` and `rowIds` described all of those exactly + * like a run over every eligible row — and `POST` on this same path accepts + * `filter` and `excludeRowIds`, so a caller could create a scope this + * resource then denied having. + * + * The filter itself stays unpublished, with the scheduler `cursor` and the + * internal identities: it is held compiled, in a different grammar from the + * predicate the request was written in, so returning it would publish an + * internal artifact under a name callers would read back as their own + * input. `filtered` names the distinction without claiming to reproduce it. + * + * The two narrowings are reported separately because they are separate: the + * run rejects only `rowIds` *with* `excludeRowIds`, so an exclusion set with + * no filter is a scope a caller can really create, and one flag covering + * both would have to call it either filtered (it is not) or unnarrowed (it + * is not). `excludeRowIds` mirrors the walk's own condition and is withheld + * where `rowIds` would make the dispatcher ignore it, so the scope reports + * what will actually happen. + */ scope: { groupIds: dispatch.scope.groupIds, ...(dispatch.scope.rowIds ? { rowIds: dispatch.scope.rowIds } : {}), + ...(dispatch.scope.filter ? { filtered: true as const } : {}), + ...(!dispatch.scope.rowIds?.length && dispatch.scope.excludeRowIds?.length + ? { excludeRowIds: dispatch.scope.excludeRowIds } + : {}), }, limit: dispatch.limit, processedCount: dispatch.processedCount, diff --git a/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.test.ts b/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.test.ts index 17d1270040..c509616529 100644 --- a/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.test.ts +++ b/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.test.ts @@ -342,6 +342,23 @@ describe('/api/v2/workflow-mcp-servers/[serverId]/tools', () => { const body = await (await get()).json() expect(body.nextCursor).toBeNull() + expect(body.truncated).toBe(false) + }) + + /** + * `nextCursor` is null whether or not the ceiling cut the set, and this list + * takes no `cursor`, so a truncated inventory cannot be paged past. Without + * this flag a reconciling caller read a partial set as the whole published + * inventory — and the use case had been computing it all along. + */ + it('says when the ceiling cut the inventory short', async () => { + mocks.getServer.mockResolvedValue(serverRow) + mocks.listTools.mockResolvedValue({ tools: [toolRow], truncated: true }) + + const body = await (await get()).json() + + expect(body.truncated).toBe(true) + expect(body.nextCursor).toBeNull() }) it('conceals a server in another workspace as not found', async () => { diff --git a/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.ts b/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.ts index bf714b290e..7ca939d501 100644 --- a/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.ts +++ b/apps/sim/app/api/v2/workflow-mcp-servers/[serverId]/tools/route.ts @@ -29,6 +29,11 @@ export const revalidate = 0 * is bounded by the workspace's deployed workflows, and a caller reconciling it * wants all of it. `nextCursor` is therefore always null. * + * That bound is not unlimited, though, and a set cut short by the ceiling + * cannot be paged past — so the response carries `truncated`. Without it a + * reconciling caller read a partial inventory as the complete one and would + * have unpublished every tool past the cut. + * * Head-safe: nothing is written and no audit is projected. */ export const GET = defineV2JsonRoute({ @@ -39,9 +44,10 @@ export const GET = defineV2JsonRoute({ errorPolicy: workflowMcpServerErrorPolicy, mapInput: ({ params }) => ({ serverId: params.serverId }), useCase: listWorkflowMcpDeploymentTools, - present: ({ tools }) => ({ + present: ({ tools, truncated }) => ({ data: tools.map(toV2WorkflowMcpToolListItem), nextCursor: null, + truncated, }), }) diff --git a/apps/sim/app/api/v2/workflow-mcp-servers/route.test.ts b/apps/sim/app/api/v2/workflow-mcp-servers/route.test.ts index 4808864e4a..161a3fd23d 100644 --- a/apps/sim/app/api/v2/workflow-mcp-servers/route.test.ts +++ b/apps/sim/app/api/v2/workflow-mcp-servers/route.test.ts @@ -164,9 +164,46 @@ describe('/api/v2/workflow-mcp-servers', () => { }, ], nextCursor: null, + toolNamesTruncated: false, }) }) + /** + * `toolNames` and `toolCount` are gathered for the whole page under one + * ceiling, so a page that trips it under-reports every server's inventory. + * Publishing only the names left a reconciling caller reading a partial set + * as the complete one. + */ + it('says when the tool names it published were cut short', async () => { + mocks.listToolNames.mockResolvedValue({ + namesByServerId: new Map([['wfmcp-1', ['triage_ticket']]]), + truncated: true, + }) + + expect((await (await get()).json()).toolNamesTruncated).toBe(true) + }) + + /** + * A further page is what `nextCursor` says. Folding it into the truncation + * flag would report an incomplete inventory on every page with a successor, + * which is most of them. + */ + it('does not call a paged inventory truncated', async () => { + mocks.listServers.mockResolvedValue({ + data: [serverRow()], + nextCursorKeys: [{ key: 'createdAt', value: '2026-06-12T10:30:00.000Z' }], + }) + mocks.listToolNames.mockResolvedValue({ + namesByServerId: new Map([['wfmcp-1', ['triage_ticket']]]), + truncated: false, + }) + + const body = await (await get()).json() + + expect(body.nextCursor).not.toBeNull() + expect(body.toolNamesTruncated).toBe(false) + }) + /** * The row carries `createdBy` and `deletedAt`; the response schema strips * them rather than the presenter enumerating what to keep, so a column added diff --git a/apps/sim/app/api/v2/workflow-mcp-servers/route.ts b/apps/sim/app/api/v2/workflow-mcp-servers/route.ts index be1975a756..ecc48f0a92 100644 --- a/apps/sim/app/api/v2/workflow-mcp-servers/route.ts +++ b/apps/sim/app/api/v2/workflow-mcp-servers/route.ts @@ -52,8 +52,9 @@ export const GET = defineV2JsonRoute({ ), }), useCase: listWorkflowMcpDeployments, - present: ({ servers, nextCursorKeys }, { query }) => ({ + present: ({ servers, nextCursorKeys, toolNamesTruncated }, { query }) => ({ data: servers.map(toV2WorkflowMcpServerListItem), + toolNamesTruncated, nextCursor: writeSortedCursor( nextCursorKeys, query.sortBy, diff --git a/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/files/[fileId]/route.ts b/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/files/[fileId]/route.ts index 589bc8bf97..5679f08562 100644 --- a/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/files/[fileId]/route.ts +++ b/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/files/[fileId]/route.ts @@ -27,7 +27,11 @@ export const revalidate = 0 * so the response cannot be used to probe which ids exist. * * `headSafe: false` because downloading records a `FILE_DOWNLOADED` audit event - * and pulls the bytes out of object storage. + * and pulls the bytes out of object storage. `HEAD` therefore runs the + * authorization phase alone — which is why the use case resolves the addressed + * file while loading canonical context rather than in `execute`, so a `HEAD` + * answers the same existence question a `GET` would instead of succeeding for + * an id that has no file behind it. */ export const GET = defineV2BinaryRoute({ contract: v2DownloadRunFileContract, diff --git a/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/route.test.ts b/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/route.test.ts index 3f25447138..e289efc328 100644 --- a/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/route.test.ts +++ b/apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/route.test.ts @@ -198,6 +198,32 @@ describe('v2 run detail and cancel adapters', () => { ) }) + /** + * The contract has always said `includeFileBase64` requires `includeOutput`, + * and the read honours it: files are projected inside the `includeOutput` + * branch alone. Nothing enforced it, so the flag parsed, was accepted, and + * was then dropped — a `200` carrying no files and no reason why. + */ + it('rejects inlining files without asking for the output they hang off', async () => { + const response = await callStatus('?includeFileBase64=true') + + expect(response.status).toBe(400) + expect((await response.json()).error.message).toContain('includeOutput') + expect(mocks.readRun).not.toHaveBeenCalled() + }) + + it('rejects a ceiling for an inlining that was never requested', async () => { + const response = await callStatus('?base64MaxBytes=4096') + + expect(response.status).toBe(400) + expect(mocks.readRun).not.toHaveBeenCalled() + }) + + /** Explicitly declining the inlining is not a request for it. */ + it('accepts includeFileBase64=false on its own', async () => { + expect((await callStatus('?includeFileBase64=false')).status).toBe(200) + }) + it('rejects a base64MaxBytes above the inline ceiling', async () => { const response = await callStatus( `?includeOutput=true&includeFileBase64=true&base64MaxBytes=${64 * 1024 * 1024}` diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index 29d5c1661f..0f822354b6 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -1483,7 +1483,7 @@ const declaredRoutes = [ 'ListWorkflowMcpServersResponse', 'List workflow MCP servers response', 'A cursor-paginated page of published MCP servers.', - [{ data: [WORKFLOW_MCP_SERVER_LIST_EXAMPLE], nextCursor: null }] + [{ data: [WORKFLOW_MCP_SERVER_LIST_EXAMPLE], nextCursor: null, toolNamesTruncated: false }] ), } ), @@ -1550,6 +1550,7 @@ const declaredRoutes = [ { data: [omitUpdated(WORKFLOW_MCP_TOOL_EXAMPLE)], nextCursor: null, + truncated: false, }, ] ), diff --git a/apps/sim/lib/api/contracts/v2/tables.ts b/apps/sim/lib/api/contracts/v2/tables.ts index 5dd9288719..59e9c970e2 100644 --- a/apps/sim/lib/api/contracts/v2/tables.ts +++ b/apps/sim/lib/api/contracts/v2/tables.ts @@ -2546,7 +2546,21 @@ export const v2TableRunDispatchSchema = z rowIds: z .array(z.string()) .optional() - .describe('Explicit rows the dispatch targets; absent means every eligible row.'), + .describe( + 'Explicit rows the dispatch targets. Absent means it was given no row list and walks every eligible row, narrowed by `filtered` and `excludeRowIds` when either is present.' + ), + filtered: z + .boolean() + .optional() + .describe( + 'Present and true when a stored filter narrows which rows run. The filter itself is not published: it is held compiled, in a different grammar from the predicate the request was written in. Absent means no filter narrows the dispatch — which, with no `rowIds` and no `excludeRowIds`, is what means every eligible row.' + ), + excludeRowIds: z + .array(z.string()) + .optional() + .describe( + 'Rows the walk skips. Independent of `filtered`: a dispatch may exclude rows from a filtered set or from every eligible row. Never present alongside `rowIds`, which the run rejects and the walk would ignore.' + ), }) .strict() .describe('What the dispatch was asked to run.'), diff --git a/apps/sim/lib/api/contracts/v2/workflow-mcp-servers.ts b/apps/sim/lib/api/contracts/v2/workflow-mcp-servers.ts index 31c511ba2d..cb92abe2bc 100644 --- a/apps/sim/lib/api/contracts/v2/workflow-mcp-servers.ts +++ b/apps/sim/lib/api/contracts/v2/workflow-mcp-servers.ts @@ -352,7 +352,13 @@ export const v2ListWorkflowMcpServersContract = defineRouteContract({ query: v2ListWorkflowMcpServersQuerySchema, response: { mode: 'json', - schema: v2CursorListResponse(v2WorkflowMcpServerListItemSchema), + schema: v2CursorListResponse(v2WorkflowMcpServerListItemSchema).extend({ + toolNamesTruncated: z + .boolean() + .describe( + "Whether `toolCount` and `toolNames` under-report. The names are gathered for the whole page under one ceiling, so a page whose servers publish more tools than that ceiling between them reports only part of each server's inventory. Read `GET /api/v2/workflow-mcp-servers/{serverId}/tools` for one server's inventory and check that response's own `truncated`, which reports the same ceiling applied to a single server — only an untruncated response is the authoritative set. Unrelated to `nextCursor`, which is how this list says there are further servers." + ), + }), }, }) @@ -408,7 +414,13 @@ export const v2ListWorkflowMcpToolsContract = defineRouteContract({ params: v2WorkflowMcpServerParamsSchema, response: { mode: 'json', - schema: v2CursorListResponse(v2WorkflowMcpToolListItemSchema, { paged: false }), + schema: v2CursorListResponse(v2WorkflowMcpToolListItemSchema, { paged: false }).extend({ + truncated: z + .boolean() + .describe( + 'Whether this inventory was cut short by the server-side ceiling on how many tools one response may carry. `nextCursor` is null either way — this list takes no `cursor`, so a truncated set cannot be paged past and this flag is the only way to tell a partial inventory from a complete one. A reconciling caller must not treat a truncated set as the full published inventory.' + ), + }), }, }) diff --git a/apps/sim/lib/api/contracts/v2/workflows.ts b/apps/sim/lib/api/contracts/v2/workflows.ts index 7e35ac9293..abe9070bcf 100644 --- a/apps/sim/lib/api/contracts/v2/workflows.ts +++ b/apps/sim/lib/api/contracts/v2/workflows.ts @@ -1821,6 +1821,27 @@ export const v2GetWorkflowRunContract = defineRouteContract({ ), }) .strict() + /** + * `includeFileBase64` is documented as requiring `includeOutput`, and the + * read honours that: file projection happens inside the `includeOutput` + * branch alone. Nothing enforced it, so asking for base64 without output + * parsed, was accepted, and was then dropped — the response came back `200` + * carrying no files and nothing to say why. `base64MaxBytes` rides on the + * same projection and was ignored the same way. Rejecting names the missing + * flag instead, matching `GET /billing/logs`, which refuses a window bound + * its period will not read rather than answering over a different one. + */ + .superRefine((query, ctx) => { + if (query.includeOutput) return + for (const field of ['includeFileBase64', 'base64MaxBytes'] as const) { + if (query[field] === undefined || query[field] === false) continue + ctx.addIssue({ + code: 'custom', + message: `${field} is only accepted with includeOutput=true; without it the run's files are never projected`, + path: [field], + }) + } + }) .meta({ id: 'GetWorkflowRunQuery', title: 'Get workflow run query', diff --git a/apps/sim/lib/mcp/application/workflow-deployments.ts b/apps/sim/lib/mcp/application/workflow-deployments.ts index c62bcbf6e4..68d0af45c6 100644 --- a/apps/sim/lib/mcp/application/workflow-deployments.ts +++ b/apps/sim/lib/mcp/application/workflow-deployments.ts @@ -125,6 +125,20 @@ export const listWorkflowMcpDeployments = defineAuthorizedWorkspaceUseCase({ toolNames: namesByServerId.get(server.id) ?? [], })), nextCursorKeys: page.nextCursorKeys, + /** + * The name cap alone: `toolCount` and `toolNames` under-report because + * this page's servers publish more tools between them than + * `MAX_LISTED_WORKFLOW_MCP_TOOLS`. Kept apart from `truncated` because a + * surface that already publishes a `nextCursor` expresses "there is more + * to fetch" with the cursor, and would otherwise report an incomplete + * inventory on every page that simply has a successor. + */ + toolNamesTruncated: truncated, + /** + * Anything at all is unseen — the name cap, or a further page. For a + * caller with no cursor to follow, which is how the copilot handler reads + * this, those are the same fact. + */ truncated: page.nextCursorKeys !== null || truncated, } }, diff --git a/apps/sim/lib/table/application/bulk.test.ts b/apps/sim/lib/table/application/bulk.test.ts index 0bafc8bfce..db7ad035c2 100644 --- a/apps/sim/lib/table/application/bulk.test.ts +++ b/apps/sim/lib/table/application/bulk.test.ts @@ -602,6 +602,43 @@ describe('path-keyed bulk table selections', () => { expect(result.deleted).toEqual([{ kind: 'folder', id: '/Sales', name: '/Sales' }]) }) + /** + * The published `deleted` is keyed the way the caller addressed the batch, so + * on this route a folder's id is replaced by its display path. The audit must + * not inherit that: `FOLDER_DELETED.resourceId` recorded `/Sales` here while + * `DELETE /api/folders/[id]` recorded the canonical id for the same action, + * leaving two spellings of one resource that no query could join. + */ + it('audits a path-keyed folder deletion by its canonical id', async () => { + mocks.bulkDeleteFolders.mockResolvedValue({ + succeeded: [{ id: 'folder-1', name: 'Sales' }], + failed: [], + folderCount: 1, + resourceCount: 0, + }) + mocks.planFolderSelection.mockResolvedValue({ + selected: [{ id: 'folder-1', name: 'Sales' }], + notFound: [], + contained: [], + covered: new Set(), + }) + + const result = await bulkDeleteTables.execute({ + principal, + input: { + assertedWorkspaceId: 'workspace-1', + folderKeying: 'paths' as const, + tableIds: [], + folders: ['/Sales'], + }, + }) + + expect(result.deleted).toEqual([{ kind: 'folder', id: '/Sales', name: '/Sales' }]) + expect(result.auditedDeletions).toEqual([ + expect.objectContaining({ kind: 'folder', id: 'folder-1' }), + ]) + }) + /** * One index for the whole batch: `resolveTableFolderPath` takes the folder * tree lock per call, so per-path resolution would be a lock acquisition each. diff --git a/apps/sim/lib/table/application/bulk.ts b/apps/sim/lib/table/application/bulk.ts index c369c6bf7d..a75c89d3b3 100644 --- a/apps/sim/lib/table/application/bulk.ts +++ b/apps/sim/lib/table/application/bulk.ts @@ -129,7 +129,14 @@ interface BulkMoveTablesExecutionResult extends BulkMoveTablesResult, TableBatch } interface BulkDeleteTablesExecutionResult extends BulkDeleteTablesResult, - TableBatchExecutionResult {} + TableBatchExecutionResult { + /** + * Every deletion keyed by canonical id, for the audit projection only. The + * published `deleted` is keyed the way the caller addressed the batch, which + * on the path-keyed route is a display path. + */ + auditedDeletions: BulkTableItem[] +} async function resolveBulkTablesContext( input: BulkTablesSelectionInput, @@ -539,6 +546,18 @@ export const bulkDeleteTables = defineAuthorizedTableUseCase({ }) return { deleted: deleted.map((item) => projectFolderItem(item, context)), + /** + * The same deletions still keyed by canonical id. + * + * `deleted` above is keyed for the caller, and on the path-keyed v2 + * route `projectFolderItem` replaces a folder's id with its display + * path. Auditing from that list wrote the path into + * `FOLDER_DELETED.resourceId`, so the same action recorded a path here + * and an id from `DELETE /api/folders/[id]` — two spellings of one + * resource that no query could join. The projection stays a + * presentation concern; the audit reads the canonical ids. + */ + auditedDeletions: deleted.map((item) => ({ ...item })), deletedItems, ...withUnresolvedFolders(projectBulkOutcome(outcome, context), context), ...(terminalError !== undefined && { terminalFailure: { error: terminalError } }), @@ -555,7 +574,7 @@ export const bulkDeleteTables = defineAuthorizedTableUseCase({ * thousands of audit rows. */ projectAudit: ({ result }) => - result.deleted.map((item) => + result.auditedDeletions.map((item) => item.kind === 'folder' ? { action: AuditAction.FOLDER_DELETED, diff --git a/apps/sim/lib/uploads/upload-session/application.test.ts b/apps/sim/lib/uploads/upload-session/application.test.ts index d5bef756c3..70ae6b01c6 100644 --- a/apps/sim/lib/uploads/upload-session/application.test.ts +++ b/apps/sim/lib/uploads/upload-session/application.test.ts @@ -11,6 +11,11 @@ const mocks = vi.hoisted(() => ({ getOwnedSession: vi.fn(), getPrincipalSession: vi.fn(), reauthorizeWorkspacePurpose: vi.fn(), + getWorkspaceFile: vi.fn(), +})) + +vi.mock('@/lib/uploads/contexts/workspace', () => ({ + getWorkspaceFile: mocks.getWorkspaceFile, })) vi.mock('@/lib/uploads/upload-session/service', () => ({ @@ -91,7 +96,7 @@ describe('upload session application', () => { * workspace permission rather than trusting the session lookup alone. */ it('re-authorizes a session read against the read operation', async () => { - const session = await readWorkspaceUploadSession(principal, { + const { session } = await readWorkspaceUploadSession(principal, { uploadId: 'upload-1', workspaceId: 'workspace-1', uploadToken: 'upload-token', @@ -105,6 +110,97 @@ describe('upload session application', () => { ) }) + /** + * The resource documents `file` as the registered file after finalization, + * and a caller polling a transfer it lost track of is exactly who needs it — + * it is the only way to learn the id the upload produced without having held + * the `complete` response. The read answered `null` unconditionally, so that + * caller could see a session reach `completed` and still never learn what it + * had created. + */ + it('returns the registered file once the session has completed', async () => { + const completed = { ...workspaceUploadSession(), status: 'completed' as const, completedFileId: 'file-1' } + mocks.getOwnedSession.mockResolvedValue(completed) + mocks.getPrincipalSession.mockResolvedValue(completed) + mocks.getWorkspaceFile.mockResolvedValue({ id: 'file-1', name: 'file.txt' }) + + const { file } = await readWorkspaceUploadSession(principal, { + uploadId: 'upload-1', + workspaceId: 'workspace-1', + uploadToken: 'upload-token', + }) + + expect(file).toEqual({ id: 'file-1', name: 'file.txt' }) + expect(mocks.getWorkspaceFile).toHaveBeenCalledWith('workspace-1', 'file-1', { + throwOnError: true, + }) + }) + + it('does not look for a file before finalization completes', async () => { + const { file } = await readWorkspaceUploadSession(principal, { + uploadId: 'upload-1', + workspaceId: 'workspace-1', + uploadToken: 'upload-token', + }) + + expect(file).toBeNull() + expect(mocks.getWorkspaceFile).not.toHaveBeenCalled() + }) + + /** + * `getWorkspaceFile` logs and returns null on a read failure unless told + * otherwise, which would report a finalized upload as fileless to the one + * caller polling to learn what it created — and they would stop, believing + * there was nothing. A failed read is not the same answer as no file. + */ + it('surfaces a failed file read instead of reporting the upload fileless', async () => { + const completed = { ...workspaceUploadSession(), status: 'completed' as const, completedFileId: 'file-1' } + mocks.getOwnedSession.mockResolvedValue(completed) + mocks.getPrincipalSession.mockResolvedValue(completed) + mocks.getWorkspaceFile.mockRejectedValue(new Error('connection terminated')) + + await expect( + readWorkspaceUploadSession(principal, { + uploadId: 'upload-1', + workspaceId: 'workspace-1', + uploadToken: 'upload-token', + }) + ).rejects.toThrow('connection terminated') + }) + + it('reads the completed file with throwOnError so a fault cannot read as absence', async () => { + const completed = { ...workspaceUploadSession(), status: 'completed' as const, completedFileId: 'file-1' } + mocks.getOwnedSession.mockResolvedValue(completed) + mocks.getPrincipalSession.mockResolvedValue(completed) + mocks.getWorkspaceFile.mockResolvedValue({ id: 'file-1', name: 'file.txt' }) + + await readWorkspaceUploadSession(principal, { + uploadId: 'upload-1', + workspaceId: 'workspace-1', + uploadToken: 'upload-token', + }) + + expect(mocks.getWorkspaceFile).toHaveBeenCalledWith('workspace-1', 'file-1', { + throwOnError: true, + }) + }) + + /** A completed session whose file was since deleted has nothing to address. */ + it('answers null when the completed file is gone', async () => { + const gone = { ...workspaceUploadSession(), status: 'completed' as const, completedFileId: 'file-1' } + mocks.getOwnedSession.mockResolvedValue(gone) + mocks.getPrincipalSession.mockResolvedValue(gone) + mocks.getWorkspaceFile.mockResolvedValue(null) + + const { file } = await readWorkspaceUploadSession(principal, { + uploadId: 'upload-1', + workspaceId: 'workspace-1', + uploadToken: 'upload-token', + }) + + expect(file).toBeNull() + }) + it('does not return a session whose re-authorization fails', async () => { mocks.reauthorizeWorkspacePurpose.mockRejectedValueOnce(new Error('Upload session not found')) diff --git a/apps/sim/lib/uploads/upload-session/application.ts b/apps/sim/lib/uploads/upload-session/application.ts index 2cebdf4731..9fb8514941 100644 --- a/apps/sim/lib/uploads/upload-session/application.ts +++ b/apps/sim/lib/uploads/upload-session/application.ts @@ -3,7 +3,7 @@ import type { CreateInternalFileUploadBody } from '@/lib/api/contracts/upload-se import type { OrchestrationRequestContext } from '@/lib/core/orchestration/types' import { OrchestrationError } from '@/lib/core/orchestration/types' import { loadActiveFolderPathIndex, resolveFolderPathFromIndex } from '@/lib/folders/queries' -import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' +import { getWorkspaceFile, type WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' import { abortUploadSession, assertUploadSessionAuthBinding, @@ -191,13 +191,44 @@ export async function loadAuthorizedWorkspaceUploadSession( * auth binding and the caller's present workspace permission are both * re-checked here rather than the session being looked up on its id alone. */ +export interface ReadWorkspaceUploadSessionResult { + session: UploadSessionRecord + /** + * The file the session registered, once it has one. + * + * The resource documents `file` as "the registered file after finalization", + * and a caller polling a transfer it lost track of is exactly who needs it — + * it is the only way to learn the id the upload produced without having held + * the `complete` response. Loaded here rather than in the presenter because + * it is a protected read. + * + * Still null when a completed session's file has since been deleted, which is + * the same shape as "not finalized yet" and needs no separate signal: in both + * cases there is no file to address. + * + * A failed *read*, though, is not that shape. `getWorkspaceFile` logs and + * returns null by default, which would let a transient database error present + * a finalized upload as fileless to the one caller polling to learn what it + * created — and they would stop, having been told there was nothing. It is + * read with `throwOnError` so the failure surfaces and the poll can retry, + * matching how `readWorkspaceFileRecord` loads the same record. + */ + file: WorkspaceFileRecord | null +} + export async function readWorkspaceUploadSession( principal: Principal, input: UploadSessionControlInput -): Promise { +): Promise { const session = await loadAuthorizedWorkspaceUploadSession(principal, input) await reauthorizeWorkspaceUploadPurpose(principal, session, fileOperations.uploadRead) - return session + if (session.status !== 'completed' || !session.completedFileId || !session.workspaceId) { + return { session, file: null } + } + const file = await getWorkspaceFile(session.workspaceId, session.completedFileId, { + throwOnError: true, + }) + return { session, file: file ?? null } } /** Issues multipart URLs after current workspace authorization. */ diff --git a/apps/sim/lib/workflows/application/download-workflow-run-file.test.ts b/apps/sim/lib/workflows/application/download-workflow-run-file.test.ts index 58ec147eee..a3e4c04918 100644 --- a/apps/sim/lib/workflows/application/download-workflow-run-file.test.ts +++ b/apps/sim/lib/workflows/application/download-workflow-run-file.test.ts @@ -110,6 +110,31 @@ describe('downloadWorkflowRunFileStream', () => { expect(result.contentLength).toBe(3) }) + /** + * `HEAD` on this route runs the authorization phase alone. With the file + * lookup downstream of it, `authorize` succeeded for any id at all, so a + * `HEAD` answered `200` for a file the `GET` beside it would `404` — the two + * verbs disagreed about whether the resource existed. The route test above + * could not catch it: it mocks this use case and makes `authorize` reject, so + * it only ever proved the route renders a rejection. + */ + it('refuses to authorize a file id the run never produced', async () => { + await expect( + downloadWorkflowRunFileStream.authorize({ + principal: principals[0], + input: input({ fileId: 'file_absent' }), + }) + ).rejects.toThrow('File not found') + expect(mocks.downloadFileStream).not.toHaveBeenCalled() + }) + + it('authorizes a file the run did produce', async () => { + await expect( + downloadWorkflowRunFileStream.authorize({ principal: principals[0], input: input() }) + ).resolves.not.toThrow() + expect(mocks.downloadFileStream).not.toHaveBeenCalled() + }) + it('denies a principal below the read role', async () => { mocks.resolvePermission.mockResolvedValue(null) diff --git a/apps/sim/lib/workflows/application/download-workflow-run-file.ts b/apps/sim/lib/workflows/application/download-workflow-run-file.ts index 397a5037fd..d8bbac08b4 100644 --- a/apps/sim/lib/workflows/application/download-workflow-run-file.ts +++ b/apps/sim/lib/workflows/application/download-workflow-run-file.ts @@ -35,14 +35,32 @@ export interface DownloadWorkflowRunFileResult { contentLength: number } -async function executeDownloadWorkflowRunFile({ - context, - input, -}: AuthorizedWorkspaceUseCaseContext< - typeof workflowOperations.downloadRunFile, - DownloadWorkflowRunFileInput, - ActiveWorkflowRunApplicationContext ->): Promise { +interface DownloadWorkflowRunFileContext extends ActiveWorkflowRunApplicationContext { + file: UserFile +} + +/** + * Resolves the run *and* the file the path addresses. + * + * The file lookup belongs here rather than in `execute` because `HEAD` runs the + * authorization phase alone: with the lookup downstream of it, `HEAD` answered + * a success for a file id no `GET` on the same path would ever serve, so the + * two disagreed about whether the resource existed. Resolving the addressed + * sub-resource while loading canonical context is what `readTableDispatch` does + * with its dispatch id, and it makes both verbs answer from one decision. + * + * Every failure keeps the shared message: an unknown run, an unknown file, a + * file belonging to another run, and a concealed cross-tenant denial are all + * `File not found`, so ordering this before the authorization phase separates + * none of them. + */ +async function resolveDownloadWorkflowRunFileContext( + input: DownloadWorkflowRunFileInput +): Promise { + const context = await resolveActiveWorkflowRunApplicationContext({ + runId: input.runId, + assertedWorkflowId: input.workflowId, + }) const runFiles = await getWorkflowRunFiles({ workflowId: context.workflowId, runId: context.runId, @@ -68,6 +86,18 @@ async function executeDownloadWorkflowRunFile({ const file = runFiles.filesById.get(input.fileId) if (!file) throw new OrchestrationError('not_found', FILE_NOT_FOUND_MESSAGE) + return { ...context, file } +} + +async function executeDownloadWorkflowRunFile({ + context, +}: AuthorizedWorkspaceUseCaseContext< + typeof workflowOperations.downloadRunFile, + DownloadWorkflowRunFileInput, + DownloadWorkflowRunFileContext +>): Promise { + const { file } = context + /** * The storage context is inferred from the key rather than read off the * record's `context` field, so the bucket a read targets is always the one @@ -110,10 +140,7 @@ async function executeDownloadWorkflowRunFile({ export const downloadWorkflowRunFileStream = defineAuthorizedWorkflowUseCase({ operation: workflowOperations.downloadRunFile, resolveContext: ({ input }: { input: DownloadWorkflowRunFileInput }) => - resolveActiveWorkflowRunApplicationContext({ - runId: input.runId, - assertedWorkflowId: input.workflowId, - }), + resolveDownloadWorkflowRunFileContext(input), execute: executeDownloadWorkflowRunFile, projectAudit: ({ context, result }) => ({ action: AuditAction.FILE_DOWNLOADED, diff --git a/apps/sim/lib/workspace-files/api/route-policies.ts b/apps/sim/lib/workspace-files/api/route-policies.ts index 4698ba8afc..c8b62bd602 100644 --- a/apps/sim/lib/workspace-files/api/route-policies.ts +++ b/apps/sim/lib/workspace-files/api/route-policies.ts @@ -4,7 +4,9 @@ import { type V2ErrorPolicy, v2OrchestrationErrorPolicy, } from '@/lib/api/server/routes' +import { ArchiveError, statusForArchiveError } from '@/lib/uploads/archive' import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization' +import { v2CaughtOrchestrationError, v2ErrorForOrchestration } from '@/app/api/v2/lib/response' export const internalSessionOrExecutorAuth = createInternalSessionOrExecutorAuth({ audience: WORKSPACE_FILES_DELEGATION_AUDIENCE, @@ -37,4 +39,25 @@ export const v2FileErrorPolicies = { concealUploadAuthorization: createV2ResourceConcealmentPolicy({ notFoundMessage: 'Upload session not found', }) satisfies V2ErrorPolicy, + /** + * Unarchiving is the one file operation whose *payload* can be at fault: a + * malformed archive is the caller's request being wrong, and an archive over + * a cap is a limit they can act on. Without an arm for it both rendered as a + * `500`, so the v2 caller was told the server had failed when the answer was + * `400` or `413` — the internal extract route beside it has said so all + * along. The status stays decided by `statusForArchiveError`, which classifies + * every reason in one place; this only carries it into the v2 vocabulary. + */ + concealExtractionAuthorization: createV2ResourceConcealmentPolicy({ + notFoundMessage: 'File not found', + render(error) { + if (error instanceof ArchiveError) { + return v2ErrorForOrchestration( + statusForArchiveError(error) === 413 ? 'payload_too_large' : 'validation', + error.message + ) + } + return v2CaughtOrchestrationError(error) + }, + }) satisfies V2ErrorPolicy, } as const diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 79500c574c..d023a361c5 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -752,6 +752,8 @@ type CancelTableDispatchResponseRef0 = { scope: { groupIds: Array rowIds?: Array + filtered?: boolean + excludeRowIds?: Array } limit: { type: 'rows' @@ -4266,6 +4268,8 @@ type GetTableDispatchResponseRef0 = { scope: { groupIds: Array rowIds?: Array + filtered?: boolean + excludeRowIds?: Array } limit: { type: 'rows' @@ -5867,6 +5871,8 @@ type ListTableDispatchesResponseRef0 = { scope: { groupIds: Array rowIds?: Array + filtered?: boolean + excludeRowIds?: Array } limit: { type: 'rows' @@ -6161,6 +6167,7 @@ type ListWorkflowMcpServersResponseRef0 = { export type ListWorkflowMcpServersResponse = { data: Array nextCursor: string | null + toolNamesTruncated: boolean } /** `GET /api/v2/workflow-mcp-servers/[serverId]/tools` */ @@ -6185,6 +6192,7 @@ type ListWorkflowMcpToolsResponseRef0 = { export type ListWorkflowMcpToolsResponse = { data: Array nextCursor: string | null + truncated: boolean } /** `GET /api/v2/workflows/[workflowId]/runs` */