fix(mcp-server): accept file uploads on v1 String fields carrying the file picker widget

[Scra3]

Legacy v1 lianas declare action file inputs as type: 'String' with widget: 'file picker' — the pattern our own docs still recommend for multi-file upload. The front dispatches on the widget and works. The MCP dispatches on the declared type and does not:

• getActionForm reports "String", so the model never calls requestActionFileUpload
• encodeFileFieldValue throws when a File object reaches a non-File field

Reported by a customer on forest-rails 9.15.8 whose KYC/KYB/claim upload actions are all built this way.

Why the combination exists

It is a validation hole on our side, not a customer mistake. In private-api, generate-widget-properties runs the type/widget validator only on the v2 widgetEdit path; the v1 widget path just checks the widget name against an allowlist and stores it on any type. Verified in production: those fields carry widgetEdit: { name: "file picker", parameters: {} }.

v2 makes the combination impossible at the TypeScript level (only FileDynamicField accepts FilePicker), so this is a v1-only compat shim, in the same family as the ones agent-client already carries (the Ruby /hooks/load 404, the snake_case operators).

What changes

FieldGetter.getEffectiveTypeName() reads String / StringList carrying that widget as File / FileList, and feeds the two existing dispatch points:

	before	after
type getActionForm reports	String	File → the model calls requestActionFileUpload
type setFieldValue encodes with	String → throws	File → data uri

getTypeName() is untouched — it still drives the Enum branch and reports the wire type.

One behaviour change worth knowing

Value-shape validation on file-picker fields gets stricter, because they now take the File/FileList branch of encodeFileFieldValue:

• a non-array on a ['String'] + file-picker field now throws expects a list of files: pass an array. — it used to be forwarded verbatim
• a non-file object on a scalar String + file-picker field now throws instead of being stored as is

Plain strings still pass through untouched, so the common case (a data uri or a URL) is unaffected. Erroring matches what native File/FileList fields already did, and replaces a silently wrong-shaped payload with a message — but ['String'] + file picker is the shape the legacy docs recommend for multi-file upload, so it is called out rather than buried.

What does not change

• A File landing on a String without the widget still throws. That guard prevents a file being JSON-serialized into a column as {"buffer":{"type":"Buffer",...}} with no error. Covered by a test.
• Nothing on the customer side. On the wire both types carry a data uri, which is exactly what their handler already receives when a human drops a file in the front.
• getType() still returns the verbatim wire value, so what loadChanges echoes back to the agent is unchanged.

Tests

agent-client 400/400 · mcp-server 1015/1015 · lint 0 error.

New coverage: the two mappings and every negative case (no widget, other widget, non-String type); the guard still firing; list encoding; and the real customer shape — a file field carrying hook: "onFieldChanged", so the /hooks/change round-trip is exercised with the data uri in the body.

get-action-form.test.ts grew ~55 lines of mock churn: its ~20 hand-rolled field mocks each needed a getEffectiveTypeName alongside their existing getTypeName, mirroring the same literal. No behaviour change in those tests.

Noted, not fixed here

ActionFieldString.fill() calls value.toString() before the guard can see the value, so a File routed through that public wrapper would be stored as '[object Object]'. Unreachable from the MCP (both entry points go through FieldFormStates.setFieldValue), so it is out of scope — worth its own ticket.

fixes PRD-1116

🤖 Generated with Claude Code

Note

Accept file uploads on v1 String fields with file picker widget via getEffectiveTypeName()

• Adds getEffectiveTypeName() to FieldGetter and ActionField to map String/StringList fields carrying the file picker widget to File/FileList; other types are unchanged.
• FieldFormStates.setFieldValue and the get-action-form MCP tool now use the effective type instead of the declared type for encoding and serialization, so v1 file picker fields produce data URIs and report File/FileList in tool responses.
• Updates error text in encodeFileFieldValue to suggest declaring File or using the file picker widget.
• Behavioral Change: get-action-form tool responses now report File/FileList for v1 file picker String fields; consumers relying on the previous String/StringList type for those fields need to handle the new type.

Changes since #1869 opened

• Introduced distinction between declared type and effective type for field type resolution in FieldGetter class [a690374]
• Updated documentation in CLAUDE.md to explain the type name distinction for file picker widget handling [a690374]
• Updated test fixtures across packages/agent-client to use empty object for file picker widget parameters [a690374]
• Refined error message in encodeFileFieldValue function for invalid file assignment [a690374]

^{Macroscope summarized a550341.}

[linear-code]

PRD-1116

[qltysh]

1 new issue

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 4): encodeFileFieldValue	1

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (4)

Rating	File	% Diff	Uncovered Line #s
	packages/agent-client/src/action-fields/field-getter.ts	100.0%
	packages/agent-client/src/action-fields/field-form-states.ts	100.0%
	packages/agent-client/src/action-fields/file-value.ts	100.0%
	packages/agent-client/src/action-fields/action-field.ts	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[Scra3]

Spec (PRD-1116): conforms. The mapping is exactly the two cases the ticket prescribes, both dispatch points are the ones it names, getTypeName() is untouched, the anti-corruption guard still fires for a String without the widget, and the two tests the ticket asked for by name are present and strong — the /hooks/change round-trip pins the request body and the returned file_0: pdfDataUri, and the Ruby static-form case proves widgetEdit survives applyFallbackForm.

I verified the two claims the change rests on rather than taking them from the description: resolveUploadedFileValues substitutes upload handles without ever reading the field type, so the handle flow is preserved and the file still never enters the model's context; and the ~55 lines of mock churn in get-action-form.test.ts are behaviour-neutral — every mock mirrors its getTypeName literal except the three new cases, checked mechanically.

---

Claude Opus 5 (claude-opus-5[1m]): Preferential — the PR description says nothing about a behaviour change it introduces.

Applies to: the PR description

A caller that passes a non-array to a ['String'] + file-picker field used to have the value forwarded verbatim (type: 'StringList' matched neither file branch, so encodeFileFieldValue returned it); it now throws expects a list of files: pass an array. Same shape for a non-file object on a scalar String + file-picker field. Plain strings still pass through, so the common URL case is unaffected.

Erroring is the better behaviour — it matches what native File/FileList fields already did. But ['String'] + file picker is precisely the pattern Forest's own legacy docs recommend for multi-file upload, so this tightens validation on the exact shape the PR targets, and "no customer-side change" in the description currently reads as covering it. Worth one sentence.

---

Claude Opus 5 (claude-opus-5[1m]): Preferential — a documented statement goes stale with this PR.

Applies to: docs.forest.app → /product/embed/mcp-server.mdx

That page scopes the upload path to File fields in two places — the tool table ("Get an upload destination for an action's File or FileList field") and the section intro ("Actions with File fields work over MCP out of the box"). After this PR a String + file-picker field also participates, so both sentences under-describe the surface and a reader has no way to learn the combination is supported.

Distinct from the legacy ['String'] doc pages PRD-1116 already scopes out to a follow-up: this staleness is created by this change, not inherited.

[hercemer42]

Claude Opus 5 (claude-opus-5[1m]): Spec (PRD-1116): conforms. getEffectiveTypeName() implements the ticket's mapping table verbatim and is wired at exactly the two dispatch points it names — get-action-form.ts:128 (read) and field-form-states.ts:89 (write) — with getTypeName() left driving the Enum branch and getType() left as the wire value. I checked the premise against the two v1 lianas rather than the stored apimap alone: forest-rails (app/helpers/forest_liana/widgets_helper.rb, set_field_widget) and forest-express (src/utils/widgets.js, setFieldWidget) both rewrite widget: 'file picker' into widgetEdit: { name: 'file picker', parameters: {} } and delete the widget key, on the /hooks/load and /hooks/change responses. So the key this reads is present on both dynamic and static form paths, and it survives the change-hook round trip — the mid-form reversion this design would otherwise be exposed to does not occur.

---

Claude Opus 5 (claude-opus-5[1m]): Preferential

Applies to: the PR as a whole (packages/workflow-executor/src/adapters/agent-client-agent-port.ts:319, not in this diff)

The same v1 field is now File to an MCP model and still String to the workflow AI, so a file-picker field stays unfillable from a trigger-record-action step. agent-client-agent-port.ts:319 reports field.getType(), and that value goes into the form-fill prompt at trigger-record-action-step-executor.ts:308; under its never-guess rule the model either omits a required file field — the operator sees a generic form-validation failure with no mention of files — or invents a plausible string.

Not a defect in this PR: getType() is deliberately the wire value there, because agent-bff and workflow-executor put it straight into API responses, and changing it would change those response shapes. But the description's "what does not change" section reads as though the agent-client change reaches every consumer, and this is the one place it deliberately does not. Worth a line in the body or a follow-up ticket so it is inherited on purpose.