diff --git a/packages/agent-client/src/action-fields/action-field.ts b/packages/agent-client/src/action-fields/action-field.ts index cee6ed924e..94c7582fd3 100644 --- a/packages/agent-client/src/action-fields/action-field.ts +++ b/packages/agent-client/src/action-fields/action-field.ts @@ -26,6 +26,10 @@ export default abstract class ActionField { return this.field?.getTypeName(); } + getEffectiveTypeName(): string { + return this.field?.getEffectiveTypeName(); + } + getValue() { return this.field?.getValue(); } diff --git a/packages/agent-client/src/action-fields/field-form-states.ts b/packages/agent-client/src/action-fields/field-form-states.ts index 76c17b7618..fdf81b5472 100644 --- a/packages/agent-client/src/action-fields/field-form-states.ts +++ b/packages/agent-client/src/action-fields/field-form-states.ts @@ -86,7 +86,7 @@ export default class FieldFormStates { const field = this.getField(name); if (!field) throw new Error(`Field "${name}" not found in action "${this.actionName}"`); - field.getPlainField().value = encodeFileFieldValue(field.getTypeName(), value, name); + field.getPlainField().value = encodeFileFieldValue(field.getEffectiveTypeName(), value, name); const fieldHasHook = field.getPlainField().hook; diff --git a/packages/agent-client/src/action-fields/field-getter.ts b/packages/agent-client/src/action-fields/field-getter.ts index cf182910fe..df1c86266e 100644 --- a/packages/agent-client/src/action-fields/field-getter.ts +++ b/packages/agent-client/src/action-fields/field-getter.ts @@ -25,13 +25,29 @@ export default class FieldGetter { } /** - * The same type as a single name, `['File']` becoming `'FileList'`. For dispatching on the type - * and for reporting it to a reader; never for anything that goes back to an agent, which echoes - * `plainField` verbatim through loadChanges and matches only the array form. + * The declared type as a single name, `['File']` becoming `'FileList'`. For dispatching on the + * declared type — the Enum branch of getActionForm — never for anything that goes back to an + * agent, which echoes `plainField` verbatim through loadChanges and matches only the array form. */ getTypeName(): string { const { type } = this.plainField; return Array.isArray(type) ? `${type[0]}List` : type; } + + /** + * What the field really holds, and what a reader is told. A v1 liana has no File type in its + * action forms: it declares a file input as a String and says so only through the widget, so a + * declared type read on its own would miss it. Use this to report a type or to encode a value; + * use getTypeName when the declared type is what matters. + */ + getEffectiveTypeName(): string { + const typeName = this.getTypeName(); + const hasFilePickerWidget = this.plainField.widgetEdit?.name === 'file picker'; + + if (hasFilePickerWidget && typeName === 'String') return 'File'; + if (hasFilePickerWidget && typeName === 'StringList') return 'FileList'; + + return typeName; + } } diff --git a/packages/agent-client/src/action-fields/file-value.ts b/packages/agent-client/src/action-fields/file-value.ts index e4f09b343c..a6444371eb 100644 --- a/packages/agent-client/src/action-fields/file-value.ts +++ b/packages/agent-client/src/action-fields/file-value.ts @@ -61,7 +61,12 @@ export default function encodeFileFieldValue( // A file reaching a field that is not declared as one is never intentional, and it would be // JSON-serialized into the column as {"buffer":{"type":"Buffer",...}} without any error. if (isFile(value)) { - throw fileError(fieldName, `is a ${type} field and cannot hold a file.`); + throw fileError( + fieldName, + `takes ${type}, not a file: send the file to a File field instead. If this field is ` + + 'meant to take one, the action must declare it as type File or carry the ' + + '"file picker" widget.', + ); } return value; diff --git a/packages/agent-client/src/action-fields/types.ts b/packages/agent-client/src/action-fields/types.ts index 708f395f17..cb43c9dd1f 100644 --- a/packages/agent-client/src/action-fields/types.ts +++ b/packages/agent-client/src/action-fields/types.ts @@ -20,8 +20,11 @@ export type PlainField = { isReadOnly: boolean; hook?: string; widgetEdit?: { + name?: string; + // `static` is the choice widgets' slice. A file picker carries none of it: v1 emits an empty + // object, v2 the upload constraints — so requiring it would only make fixtures invent one. parameters: { - static: { + static?: { options?: PlainFieldOption[]; enableOpacity?: boolean; quickPalette?: string[]; diff --git a/packages/agent-client/test/action-fields/action-fields.test.ts b/packages/agent-client/test/action-fields/action-fields.test.ts index a590eaf65e..4fcfebacb1 100644 --- a/packages/agent-client/test/action-fields/action-fields.test.ts +++ b/packages/agent-client/test/action-fields/action-fields.test.ts @@ -14,6 +14,7 @@ import ActionFieldRadioGroup from '../../src/action-fields/action-field-radio-gr import ActionFieldString from '../../src/action-fields/action-field-string'; import ActionFieldStringList from '../../src/action-fields/action-field-string-list'; import FieldFormStates from '../../src/action-fields/field-form-states'; +import Action from '../../src/domains/action'; jest.mock('../../src/http-requester'); @@ -864,5 +865,84 @@ describe('ActionField implementations', () => { const field = new ActionFieldString('testField', fieldFormStates); expect(field.isRequired()).toBe(true); }); + + it('should report a String carrying the file picker widget as File while keeping its declared type', async () => { + await setupFields([ + { + field: 'file_0', + type: 'String', + isRequired: true, + isReadOnly: false, + widgetEdit: { name: 'file picker', parameters: {} }, + }, + ]); + + const field = new ActionFieldString('file_0', fieldFormStates); + + expect(field.getEffectiveTypeName()).toBe('File'); + expect(field.getType()).toBe('String'); + }); + + it('should report a String list carrying the file picker widget as FileList', async () => { + await setupFields([ + { + field: 'files', + type: ['String'], + isRequired: false, + isReadOnly: false, + widgetEdit: { name: 'file picker', parameters: {} }, + }, + ]); + + const field = new ActionFieldString('files', fieldFormStates); + + expect(field.getEffectiveTypeName()).toBe('FileList'); + }); + + it('should report a String without the file picker widget as String', () => { + const field = new ActionFieldString('testField', fieldFormStates); + expect(field.getEffectiveTypeName()).toBe('String'); + }); + }); + + describe('Action.getFields on a v1 file picker form', () => { + it('should return a String-typed field that reports itself as File', async () => { + httpRequester.query.mockResolvedValue({ + fields: [ + { + field: 'doc_type_0', + type: 'Enum', + isRequired: true, + isReadOnly: false, + enums: ['passport', 'id_card'], + }, + { + field: 'file_0', + type: 'String', + isRequired: true, + isReadOnly: false, + widgetEdit: { name: 'file picker', parameters: {} }, + }, + ], + layout: [], + }); + + await fieldFormStates.loadInitialState(); + + const action = new Action( + 'users', + 'Upload doc', + httpRequester, + '/forest/actions/test', + fieldFormStates, + ['1'], + ); + const [docType, file] = action.getFields(); + + expect(file).toBeInstanceOf(ActionFieldString); + expect(file.getType()).toBe('String'); + expect(file.getEffectiveTypeName()).toBe('File'); + expect(docType.getEffectiveTypeName()).toBe('Enum'); + }); }); }); diff --git a/packages/agent-client/test/action-fields/field-form-states.test.ts b/packages/agent-client/test/action-fields/field-form-states.test.ts index ad58a5deb4..ce58b1d9ef 100644 --- a/packages/agent-client/test/action-fields/field-form-states.test.ts +++ b/packages/agent-client/test/action-fields/field-form-states.test.ts @@ -699,4 +699,144 @@ describe('FieldFormStates', () => { expect(httpRequester.query).not.toHaveBeenCalled(); }); }); + + describe('on a v1 form declaring its file field as a String with the file picker widget', () => { + const pdf = { mimeType: 'application/pdf', buffer: Buffer.from('%PDF-1.4'), name: 'kyc.pdf' }; + const pdfDataUri = `data:application/pdf;name=kyc.pdf;base64,${Buffer.from('%PDF-1.4').toString( + 'base64', + )}`; + const filePicker = { name: 'file picker', parameters: {} }; + + const loadedFields = [ + { + field: 'doc_type_0', + type: 'Enum', + isRequired: true, + isReadOnly: false, + value: null, + hook: 'onFieldChanged', + widgetEdit: null, + enums: ['passport', 'id_card'], + }, + { + field: 'file_0', + type: 'String', + isRequired: true, + isReadOnly: false, + value: null, + hook: 'onFieldChanged', + widgetEdit: filePicker, + }, + { + field: 'comments_0', + type: 'String', + isRequired: false, + isReadOnly: false, + value: null, + hook: 'onFieldChanged', + widgetEdit: null, + }, + ]; + + const buildFormStates = ( + options: Partial[0]> = {}, + ) => + new FieldFormStates({ + actionName: 'KYC - Upload doc.', + actionPath: '/forest/actions/kyc-upload-doc', + collectionName: 'users', + httpRequester, + ids: ['1', '2'], + ...options, + }); + + const loadForm = async () => { + const formStates = buildFormStates({ hooks: { load: true, change: ['onFieldChanged'] } }); + httpRequester.query.mockResolvedValue({ fields: loadedFields, layout: [] }); + await formStates.loadInitialState(); + httpRequester.query.mockReset(); + + return formStates; + }; + + it('sends the data uri to the change hook and keeps what the agent answers', async () => { + const formStates = await loadForm(); + httpRequester.query.mockResolvedValue({ + fields: [ + { ...loadedFields[0], value: 'passport' }, + { ...loadedFields[1], value: pdfDataUri }, + { ...loadedFields[2], value: 'Passport received, expires 2031' }, + ], + layout: [], + }); + + await formStates.setFieldValue('file_0', pdf); + + expect(httpRequester.query).toHaveBeenLastCalledWith({ + method: 'post', + path: '/forest/actions/kyc-upload-doc/hooks/change', + body: { + data: { + attributes: { + collection_name: 'users', + changed_field: 'file_0', + ids: ['1', '2'], + fields: [ + expect.objectContaining({ field: 'doc_type_0', value: null }), + expect.objectContaining({ + field: 'file_0', + type: 'String', + value: pdfDataUri, + widgetEdit: filePicker, + }), + expect.objectContaining({ field: 'comments_0', value: null }), + ], + }, + type: 'custom-action-hook-requests', + }, + }, + }); + + expect(formStates.getFieldValues()).toEqual({ + doc_type_0: 'passport', + file_0: pdfDataUri, + comments_0: 'Passport received, expires 2031', + }); + expect(formStates.getField('file_0')?.getEffectiveTypeName()).toBe('File'); + }); + + it('keeps the file picker widget on the ruby static-form path', async () => { + const formStates = buildFormStates({ + hooks: { load: false, change: ['onFieldChanged'] }, + fallbackFields: [ + { + field: 'doc_type_0', + type: 'Enum', + isRequired: true, + hook: 'onFieldChanged', + enums: ['passport', 'id_card'], + }, + { + field: 'file_0', + type: 'String', + isRequired: true, + hook: 'onFieldChanged', + widgetEdit: { name: 'file picker', parameters: {} }, + }, + { field: 'comments_0', type: 'String', hook: 'onFieldChanged' }, + ], + }); + + await formStates.loadInitialState(); + + expect(httpRequester.query).not.toHaveBeenCalled(); + expect(formStates.getField('file_0')?.getPlainField().widgetEdit).toEqual({ + name: 'file picker', + parameters: {}, + }); + expect(formStates.getField('file_0')?.getEffectiveTypeName()).toBe('File'); + expect(formStates.getField('doc_type_0')?.getEffectiveTypeName()).toBe('Enum'); + expect(formStates.getField('comments_0')?.getEffectiveTypeName()).toBe('String'); + }); + }); }); diff --git a/packages/agent-client/test/action-fields/field-getter.test.ts b/packages/agent-client/test/action-fields/field-getter.test.ts index 9c89b45f68..1c0b6b9d65 100644 --- a/packages/agent-client/test/action-fields/field-getter.test.ts +++ b/packages/agent-client/test/action-fields/field-getter.test.ts @@ -71,4 +71,75 @@ describe('FieldGetter', () => { expect(new FieldGetter(createPlainField({ type: 'Json' })).getType()).toBe('Json'); }); }); + + describe('getEffectiveTypeName', () => { + const filePicker = { name: 'file picker', parameters: {} }; + + it('should map a String carrying the file picker widget to File', () => { + const fieldGetter = new FieldGetter( + createPlainField({ type: 'String', widgetEdit: filePicker }), + ); + + expect(fieldGetter.getEffectiveTypeName()).toBe('File'); + expect(fieldGetter.getTypeName()).toBe('String'); + }); + + it('should map a String list carrying the file picker widget to FileList', () => { + const fieldGetter = new FieldGetter( + createPlainField({ type: ['String'], widgetEdit: filePicker }), + ); + + expect(fieldGetter.getEffectiveTypeName()).toBe('FileList'); + expect(fieldGetter.getTypeName()).toBe('StringList'); + }); + + it('should leave a String without the file picker widget untouched', () => { + expect(new FieldGetter(createPlainField({ type: 'String' })).getEffectiveTypeName()).toBe( + 'String', + ); + expect( + new FieldGetter( + createPlainField({ + type: 'String', + widgetEdit: { name: 'text area editor', parameters: {} }, + }), + ).getEffectiveTypeName(), + ).toBe('String'); + }); + + it('should leave a String untouched on the null widgetEdit the saas stores', () => { + const plainField = { + ...createPlainField({ type: 'String' }), + widgetEdit: null, + } as unknown as PlainField; + + expect(new FieldGetter(plainField).getEffectiveTypeName()).toBe('String'); + }); + + it('should leave a non String type untouched even with the file picker widget', () => { + const fieldGetter = new FieldGetter( + createPlainField({ type: 'Number', widgetEdit: filePicker }), + ); + + expect(fieldGetter.getEffectiveTypeName()).toBe('Number'); + }); + + it('should leave an Enum untouched', () => { + const fieldGetter = new FieldGetter( + createPlainField({ type: 'Enum', enums: ['passport', 'id_card'] }), + ); + + expect(fieldGetter.getEffectiveTypeName()).toBe('Enum'); + expect(fieldGetter.getTypeName()).toBe('Enum'); + }); + + it('should report a native File as File and a native File list as FileList', () => { + expect(new FieldGetter(createPlainField({ type: 'File' })).getEffectiveTypeName()).toBe( + 'File', + ); + expect(new FieldGetter(createPlainField({ type: ['File'] })).getEffectiveTypeName()).toBe( + 'FileList', + ); + }); + }); }); diff --git a/packages/agent-client/test/action-fields/file-value.test.ts b/packages/agent-client/test/action-fields/file-value.test.ts index 886d145f22..be0041f0b7 100644 --- a/packages/agent-client/test/action-fields/file-value.test.ts +++ b/packages/agent-client/test/action-fields/file-value.test.ts @@ -163,7 +163,82 @@ describe('file values in action forms', () => { await setupFields([{ field: 'comment', type: 'String' }]); await expect(fieldFormStates.setFieldValue('comment', pdf)).rejects.toThrow( - 'Field "comment" is a String field and cannot hold a file.', + 'Field "comment" takes String, not a file: send the file to a File field instead. ' + + 'If this field is meant to take one, the action must declare it as type File or ' + + 'carry the "file picker" widget.', + ); + }); + }); + + describe('on a v1 field declared String with the file picker widget', () => { + const filePicker = { name: 'file picker', parameters: {} }; + + it('encodes a file object as a data uri, as if the field were a File', async () => { + await setupFields([{ field: 'file_0', type: 'String', widgetEdit: filePicker }]); + + await fieldFormStates.setFieldValue('file_0', pdf); + + expect(fieldFormStates.getFieldValues()).toEqual({ file_0: pdfDataUri }); + }); + + it('encodes every item when the field is declared as a list', async () => { + await setupFields([{ field: 'files', type: ['String'], widgetEdit: filePicker }]); + + await fieldFormStates.setFieldValue('files', [pdf, pdf]); + + expect(fieldFormStates.getFieldValues()).toEqual({ files: [pdfDataUri, pdfDataUri] }); + }); + + it('reports the effective type while keeping the wire type intact', async () => { + await setupFields([ + { field: 'file_0', type: 'String', widgetEdit: filePicker }, + { field: 'files', type: ['String'], widgetEdit: filePicker }, + ]); + + expect(fieldFormStates.getField('file_0')?.getEffectiveTypeName()).toBe('File'); + expect(fieldFormStates.getField('file_0')?.getTypeName()).toBe('String'); + expect(fieldFormStates.getField('file_0')?.getType()).toBe('String'); + expect(fieldFormStates.getField('files')?.getEffectiveTypeName()).toBe('FileList'); + expect(fieldFormStates.getField('files')?.getTypeName()).toBe('StringList'); + expect(fieldFormStates.getField('files')?.getType()).toEqual(['String']); + }); + + it('leaves a plain string untouched, as the front already sends a data uri', async () => { + await setupFields([{ field: 'file_0', type: 'String', widgetEdit: filePicker }]); + + await fieldFormStates.setFieldValue('file_0', pdfDataUri); + + expect(fieldFormStates.getFieldValues()).toEqual({ file_0: pdfDataUri }); + }); + + it('rejects an object that is not a file, instead of storing it as is', async () => { + await setupFields([{ field: 'file_0', type: 'String', widgetEdit: filePicker }]); + + await expect(fieldFormStates.setFieldValue('file_0', { foo: 1 })).rejects.toThrow( + 'Field "file_0" expects a file: pass { buffer, mimeType, name } ' + + 'or a string holding a data uri.', + ); + }); + + it('rejects a bare value where the list field expects an array', async () => { + await setupFields([{ field: 'files', type: ['String'], widgetEdit: filePicker }]); + + await expect(fieldFormStates.setFieldValue('files', pdfDataUri)).rejects.toThrow( + 'Field "files" expects a list of files: pass an array.', + ); + }); + + it('leaves a String field carrying another widget rejecting files', async () => { + await setupFields([ + { + field: 'comment', + type: 'String', + widgetEdit: { name: 'text area editor', parameters: {} }, + }, + ]); + + await expect(fieldFormStates.setFieldValue('comment', pdf)).rejects.toThrow( + 'Field "comment" takes String, not a file', ); }); }); diff --git a/packages/forestadmin-client/src/types.ts b/packages/forestadmin-client/src/types.ts index bb7616ae17..a3a118cc4c 100644 --- a/packages/forestadmin-client/src/types.ts +++ b/packages/forestadmin-client/src/types.ts @@ -205,8 +205,11 @@ export interface ForestSchemaAction { // Widget options (dropdown/radio/checkbox/color values...) — the static form's only source // for consumers like getMultipleChoiceField() once the /hooks/load probe is skipped. widgetEdit?: { + name?: string; parameters: { - static: { + // Absent on widgets that carry no choices — a file picker emits `{}` on v1 and the upload + // constraints on v2. + static?: { options?: { label: string; value: string }[]; enableOpacity?: boolean; quickPalette?: string[]; diff --git a/packages/mcp-server/CLAUDE.md b/packages/mcp-server/CLAUDE.md index 0a4eee3a3f..5b7742c556 100644 --- a/packages/mcp-server/CLAUDE.md +++ b/packages/mcp-server/CLAUDE.md @@ -18,7 +18,7 @@ Key flows that only make sense across files: - **The workflow tools are the exception.** `listWorkflows` / `triggerWorkflow` / `getWorkflowRun` never reach the agent: workflows live in the orchestrator, so these go through `forestServerClient.workflowsService` over the `/api/workflow-orchestrator/mcp-workflows/*` HTTP contract. Two consequences worth knowing before editing them: responses are **projected onto an explicit whitelist** in `forest-http-api.ts` (their payload is stringified straight into a model's context, so a new server field must not arrive by itself) — with one deliberate hole, `stepDefinition`, forwarded whole so the model can reason about the step, which means a field added to a *step* type does reach the model unannounced; and `triggerWorkflow` resolves the workflow by id **before** anything is written so its audit label exists ahead of the side effect. - **Two cross-cutting wrappers.** Every tool uses `registerToolWithLogging`; most, but not all, also use `withActivityLog` — `getActionForm`, `listWorkflows`, `getWorkflowRun` and `requestActionFileUpload` are unaudited (tracked in PRD-967), so do not assume a tool writes an activity log without checking. `registerToolWithLogging` (`src/utils/tool-with-logging.ts`) registers the tool and converts thrown errors into `{ isError: true }` results (per MCP spec) instead of protocol errors. Inside the handler, `withActivityLog` (`src/utils/with-activity-log.ts`) brackets the operation with a pending→succeeded/failed Forest activity log and runs `parseAgentError` + optional `errorEnhancer` (e.g. `list` appends sortable field names on "Invalid sort"). - **Audit fail policy lives in one place.** `createPendingActivityLog` (`src/utils/activity-logs-creator.ts`) decides what happens when the audit log cannot be created, for every tool: a **write** is blocked (no unaudited side effect), a **read** proceeds with a warning (an audit-store outage must not take down the read surface), and an **authorization refusal (401/403) propagates either way** — it is not an outage. This covers both a rejection and a `200` with no log id. Changing it changes behaviour for all tools at once; the policy is pinned in both directions in `test/utils/activity-logs-creator.test.ts`. -- **Action file uploads are an opt-in side-channel** (`src/file-uploads/`), on by default: `EphemeralStorage` holds the objects in memory unless `fileUploads.storage` provides a backend. `fileUploads: false` is the off switch (it drops `requestActionFileUpload` from the enabled set, so the tool, the upload endpoint and the `executeAction` instructions all follow); leaving the tool out of `enabledTools` does the same, at the cost of freezing the allowlist. The `requestActionFileUpload` tool (`src/tools/request-action-file-upload.ts`) returns a pre-authorized upload URL plus a user-bound JWT handle signed with `authSecret`. `FieldGetter.getType()` returns the wire value (`['File']`) because agent-bff and workflow-executor put it straight into API responses; `getTypeName()` is the collapsed `'FileList'`, for dispatch and for what this server reports to a model. It is a tool rather than an HTTP route so clients discover it through `tools/list` instead of a sentence in a description. It checks the `mcp:action` scope itself, because `/mcp` only requires `mcp:read` while the route it replaced required `mcp:action`. `executeAction` swaps `"$uploadedFile:"` values for the downloaded `File` object (`resolve.ts`) and lets **agent-client** encode it — this package never builds a data uri itself. `parseFileReference` (`file-reference.ts`) is the single place that recognizes a reference, so SEP-2631 file URIs can be added there without touching the resolution path. `getActionForm` leaves handles unresolved on purpose, because it echoes values back into the model's context — and withholds them from `tryToSetFields` so a change hook fired by another field never reads `.buffer` off a handle string, while still echoing them back as the field's value and counting them as filling a required field, or `canExecute` could never become true. `download` is **not** consume-on-read: `resolve.ts` fetches every reference before `setFields` and `execute`, so a later failure must leave the objects retryable. With no `storage`, `EphemeralStorage` holds the objects in memory and serves a PUT endpoint under `${prefix}/mcp/uploads` — registered **before the body parsers** (they would consume the raw stream) and before `allowedMethods(['POST'])`; `makeIsMcpRoute` already claims everything under `/mcp/`, so no routing change was needed. Per-instance, so it warns on the first `createUploadUrl` rather than at startup — uploads being on by default, a boot warning would reach agents that have no file field at all. +- **Action file uploads are an opt-in side-channel** (`src/file-uploads/`), on by default: `EphemeralStorage` holds the objects in memory unless `fileUploads.storage` provides a backend. `fileUploads: false` is the off switch (it drops `requestActionFileUpload` from the enabled set, so the tool, the upload endpoint and the `executeAction` instructions all follow); leaving the tool out of `enabledTools` does the same, at the cost of freezing the allowlist. The `requestActionFileUpload` tool (`src/tools/request-action-file-upload.ts`) returns a pre-authorized upload URL plus a user-bound JWT handle signed with `authSecret`. `FieldGetter.getType()` returns the wire value (`['File']`) because agent-bff and workflow-executor put it straight into API responses; `getTypeName()` is the collapsed `'FileList'`, and is now only the Enum dispatch; what this server reports to a model is `getEffectiveTypeName()`, which additionally reads a v1 `String`/`StringList` carrying the `file picker` widget as `File`/`FileList` — the encoding in `setFieldValue` dispatches on it too. It is a tool rather than an HTTP route so clients discover it through `tools/list` instead of a sentence in a description. It checks the `mcp:action` scope itself, because `/mcp` only requires `mcp:read` while the route it replaced required `mcp:action`. `executeAction` swaps `"$uploadedFile:"` values for the downloaded `File` object (`resolve.ts`) and lets **agent-client** encode it — this package never builds a data uri itself. `parseFileReference` (`file-reference.ts`) is the single place that recognizes a reference, so SEP-2631 file URIs can be added there without touching the resolution path. `getActionForm` leaves handles unresolved on purpose, because it echoes values back into the model's context — and withholds them from `tryToSetFields` so a change hook fired by another field never reads `.buffer` off a handle string, while still echoing them back as the field's value and counting them as filling a required field, or `canExecute` could never become true. `download` is **not** consume-on-read: `resolve.ts` fetches every reference before `setFields` and `execute`, so a later failure must leave the objects retryable. With no `storage`, `EphemeralStorage` holds the objects in memory and serves a PUT endpoint under `${prefix}/mcp/uploads` — registered **before the body parsers** (they would consume the raw stream) and before `allowedMethods(['POST'])`; `makeIsMcpRoute` already claims everything under `/mcp/`, so no routing change was needed. Per-instance, so it warns on the first `createUploadUrl` rather than at startup — uploads being on by default, a boot warning would reach agents that have no file field at all. - **`collectionNames` → `z.enum`.** `fetchCollectionNames()` populates the schema's collection list; tools turn it into a `z.enum` for `collectionName` so the LLM gets autocomplete/validation. If schema fetch fails the server logs a warning and runs "degraded" with `z.string()`. ## Commands diff --git a/packages/mcp-server/src/tools/get-action-form.ts b/packages/mcp-server/src/tools/get-action-form.ts index f47c269ee8..c333262eaf 100644 --- a/packages/mcp-server/src/tools/get-action-form.ts +++ b/packages/mcp-server/src/tools/get-action-form.ts @@ -125,7 +125,7 @@ The response includes: const description = field.getPlainField()?.description; const baseField = { name: field.getName(), - type: field.getTypeName(), + type: field.getEffectiveTypeName(), value: valueOf(field), isRequired: field.isRequired() ?? false, ...(description ? { description } : {}), diff --git a/packages/mcp-server/test/tools/get-action-form.test.ts b/packages/mcp-server/test/tools/get-action-form.test.ts index 275470f39a..13f50dbbea 100644 --- a/packages/mcp-server/test/tools/get-action-form.test.ts +++ b/packages/mcp-server/test/tools/get-action-form.test.ts @@ -318,6 +318,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'Document', getType: () => 'File', getTypeName: () => 'File', + getEffectiveTypeName: () => 'File', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -355,6 +356,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'Attachments', getType: () => ['File'], getTypeName: () => 'FileList', + getEffectiveTypeName: () => 'FileList', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -383,6 +385,171 @@ describe('declareGetActionFormTool', () => { expect(payload.fields[0].value).toEqual(['$uploadedFile:a', '$uploadedFile:b']); }); + it('reports a v1 String field carrying the file picker widget as a File, and leaves its neighbours alone', async () => { + const mockGetEnumField = jest.fn().mockReturnValue({ + getOptions: () => ['Passport', 'Driving licence'], + }); + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([ + { + getName: () => 'doc_type_0', + getType: () => 'Enum', + getTypeName: () => 'Enum', + getEffectiveTypeName: () => 'Enum', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({ hook: 'onFieldChanged' }), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + { + getName: () => 'file_0', + getType: () => 'String', + getTypeName: () => 'String', + getEffectiveTypeName: () => 'File', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({ hook: 'onFieldChanged' }), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + { + getName: () => 'comments_0', + getType: () => 'String', + getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', + getValue: () => undefined, + isRequired: () => false, + getPlainField: () => ({ hook: 'onFieldChanged' }), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + ]), + tryToSetFields: jest.fn().mockResolvedValue([]), + getEnumField: mockGetEnumField, + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const result = await registeredToolHandler( + { collectionName: 'users', actionName: 'KYC - Upload doc.', recordIds: [1] }, + mockExtra, + ); + const payload = JSON.parse((result as { content: { text: string }[] }).content[0].text); + + expect(payload.fields).toEqual([ + { + name: 'doc_type_0', + type: 'Enum', + value: undefined, + isRequired: true, + enumValues: ['Passport', 'Driving licence'], + }, + { name: 'file_0', type: 'File', value: undefined, isRequired: true }, + { name: 'comments_0', type: 'String', value: undefined, isRequired: false }, + ]); + expect(mockGetEnumField).toHaveBeenCalledWith('doc_type_0'); + }); + + it('reports a v1 list of Strings carrying the file picker widget as a FileList, and a native File as a File', async () => { + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([ + { + getName: () => 'files_0', + getType: () => ['String'], + getTypeName: () => 'StringList', + getEffectiveTypeName: () => 'FileList', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({}), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + { + getName: () => 'Document', + getType: () => 'File', + getTypeName: () => 'File', + getEffectiveTypeName: () => 'File', + getValue: () => undefined, + isRequired: () => false, + getPlainField: () => ({}), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + ]), + tryToSetFields: jest.fn().mockResolvedValue([]), + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const result = await registeredToolHandler( + { collectionName: 'users', actionName: 'KYC - Upload doc.', recordIds: [1] }, + mockExtra, + ); + const payload = JSON.parse((result as { content: { text: string }[] }).content[0].text); + + expect(payload.fields).toEqual([ + { name: 'files_0', type: 'FileList', value: undefined, isRequired: true }, + { name: 'Document', type: 'File', value: undefined, isRequired: false }, + ]); + }); + + it('withholds and echoes a handle sent to a v1 String file picker field, still reported as a File', async () => { + const mockTryToSetFields = jest.fn().mockResolvedValue([]); + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([ + { + getName: () => 'file_0', + getType: () => 'String', + getTypeName: () => 'String', + getEffectiveTypeName: () => 'File', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({ hook: 'onFieldChanged' }), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + { + getName: () => 'comments_0', + getType: () => 'String', + getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', + getValue: () => 'signed today', + isRequired: () => false, + getPlainField: () => ({ hook: 'onFieldChanged' }), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + ]), + tryToSetFields: mockTryToSetFields, + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const result = await registeredToolHandler( + { + collectionName: 'users', + actionName: 'KYC - Upload doc.', + recordIds: [1], + values: { file_0: '$uploadedFile:kyc-token', comments_0: 'signed today' }, + }, + mockExtra, + ); + const payload = JSON.parse((result as { content: { text: string }[] }).content[0].text); + + expect(mockTryToSetFields).toHaveBeenCalledWith({ comments_0: 'signed today' }); + expect(payload.fields).toEqual([ + { + name: 'file_0', + type: 'File', + value: '$uploadedFile:kyc-token', + isRequired: true, + }, + { name: 'comments_0', type: 'String', value: 'signed today', isRequired: false }, + ]); + expect(payload.canExecute).toBe(true); + expect(payload.requiredFields).toEqual([]); + }); + // `in` would walk the prototype chain: a field literally named toString would read as filled // by Object.prototype.toString — a function — and canExecute would come back true on an empty // form. @@ -393,6 +560,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'toString', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -448,6 +616,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'subject', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -457,6 +626,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'message', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => 'Default message', isRequired: () => false, getPlainField: () => ({}), @@ -501,6 +671,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'subject', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => 'Test Subject', isRequired: () => true, getPlainField: () => ({}), @@ -510,6 +681,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'message', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => 'Test Message', isRequired: () => true, getPlainField: () => ({}), @@ -545,6 +717,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'subject', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -554,6 +727,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'message', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => 'Test Message', isRequired: () => false, getPlainField: () => ({}), @@ -589,6 +763,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'quantity', getType: () => 'Number', getTypeName: () => 'Number', + getEffectiveTypeName: () => 'Number', getValue: () => 0, isRequired: () => true, getPlainField: () => ({}), @@ -624,6 +799,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'isActive', getType: () => 'Boolean', getTypeName: () => 'Boolean', + getEffectiveTypeName: () => 'Boolean', getValue: () => false, isRequired: () => true, getPlainField: () => ({}), @@ -659,6 +835,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'notes', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => '', isRequired: () => true, getPlainField: () => ({}), @@ -694,6 +871,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'subject', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => null, isRequired: () => true, getPlainField: () => ({}), @@ -729,6 +907,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'optionalField', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => false, getPlainField: () => ({}), @@ -764,6 +943,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'status', getType: () => 'Enum', getTypeName: () => 'Enum', + getEffectiveTypeName: () => 'Enum', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -773,6 +953,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'message', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => false, getPlainField: () => ({}), @@ -820,6 +1001,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'plan', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({ description: 'Subscription plan' }), @@ -834,6 +1016,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'priority', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => false, getPlainField: () => ({}), @@ -919,6 +1102,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'subject', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => 'Test Subject', isRequired: () => true, getPlainField: () => ({}), @@ -961,6 +1145,7 @@ describe('declareGetActionFormTool', () => { getName: () => 'subject', getType: () => 'String', getTypeName: () => 'String', + getEffectiveTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}),