fix(agent-bff): enforce the action input contract on execute

[Tonours]

What

/execute validates the submitted values against the live form before running the action.

• A required field left empty answers 400 missing_required_fields. canExecute was a form-time hint nobody re-read at execute; the gate and the hint now come from the same missingRequiredFieldNames helper, so they cannot disagree.
• An out-of-enum or wrongly typed value answers 422 invalid_action_value, list items included — the message names the failing index (index 1 to be a number).
• A record-picker field takes a packed id string, whatever PK type the schema publishes.

Validation runs after setFields, on the form the change hooks rebuilt: what is checked is exactly what execute() would submit. A loaded default satisfies a required field.

fixes PRD-1096

Why

The document promised required / enum / type and the BFF forwarded everything unvalidated, so a destructive action gated on canExecute ran anyway.

What is not checked

The OpenAPI text says so now, because the gap is invisible otherwise:

• A declared string format (date-time, uuid) — a Date field takes any string.
• A widget's own option list. Dropdown / RadioGroup / CheckboxGroup carry their values in widgetEdit.parameters.static.options, not in enums, and a searchable dropdown computes them per query — a static list is not an exhaustive one, so enforcing it would reject valid values.
• Json and File fields, by design. A File value is already a data-uri string by the time the validator runs.

Observability

Every rejection logs a Warn with the field names — never the values, which carry customer data. An enum field the BFF had to accept without options logs too: that is a change hook returning an empty or malformed enums, and it silently disables the check.

Cross-SDK check

The validator branches on field.getReference(), so an agent that omits reference on a record picker would see packed ids rejected against the PK's numeric type. Checked, not assumed: the Python agent emits it (agent-python/.../generator_action.py:76, output["reference"] = f"{collection.name}.{pk}") and so does Ruby (agent-ruby/.../generator_action.rb:110), both alongside the PK column type, exactly like Node (generator-actions.ts:145-149). No SDK gap found.

Shared rules

The column-type algebra lives in one place, read-model/field-type.ts: FieldType, normalizeFieldType, isEnumFieldType, primitiveShapeOf, enumOptionsOf. The OpenAPI document builder and the runtime validator both read it, so a primitive a newer agent introduces is unconstrained on both sides or constrained on both — never one. NumberList and the array form ['Number'] normalize to the same thing, so agents that differ on that encoding are judged identically.

Known limitation

Breaking for JSON callers that sent coerced values: a string in a Number field is the ticket's own repro and now answers 422.

How to test

• yarn workspace @forestadmin/agent-bff test and yarn workspace @forestadmin/agent-client test.
• Repros pinned in test/action/action-execute-validation.test.ts: values: {} with a required field → 400, Tier: "NOPE" → 422, Amount: "nan" → 422, and canExecute:false → 400 instead of executing.

Definition of Done

General

• Write an explicit title for the Pull Request, following Conventional Commits specification
• Test manually the implemented changes
• Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

• Consider the security impact of the changes made

[linear-code]

PRD-1096

[qltysh]

5 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 4): expectedWhenViolated	4
qlty	Structure	Function with high complexity (count = 16): assertActionValuesExecutable	1

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (13)

Rating	File	% Diff	Uncovered Line #s
	packages/agent-bff/src/openapi/field-schemas.ts	100.0%
	packages/agent-bff/src/openapi/unfolded-paths.ts	100.0%
	packages/agent-bff/src/action/action-form-mapper.ts	100.0%
	packages/agent-bff/src/openapi/collect-unfolding.ts	100.0%
	packages/agent-client/src/errors.ts	100.0%
	packages/agent-bff/src/read-model/capabilities-cache.ts	100.0%
	packages/agent-bff/src/validation/validation-errors.ts	100.0%
	packages/agent-client/src/index.ts	100.0%
	packages/agent-bff/src/action/action-routes-middleware.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%
	packages/agent-bff/src/read-model/field-type.ts	100.0%
	packages/agent-bff/src/action/action-values-validator.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.

[nbouliol]

Solid fix: the three repro cases and the canExecute gate are each gated and tested before execute(). Main concern is that the same rule now lives in two places in a few spots — document vs runtime — which is the drift class the ticket is about.

[Tonours]

Second push, from the review passes rather than from the thread comments.

The validation path was invisible. BffHttpError is serialized and returned before any logging, and there is no access log middleware — so every 400 and 422 this PR introduces left zero server-side trace. An agent shipping a change hook that starts marking a field required would produce a wave of 400s with silent logs. Each rejection now logs a Warn carrying the field names only; action values carry customer data and never enter a log line, which a test asserts.

An enum with no usable options silently stops being validated. enums: [] is legitimate (a hook can compute an empty list) but enums: 'gold' is an agent bug, and both ended in the same place: the field falls back to a plain string check and accepts anything. It logs a Warn now.

The list message was ambiguous. tags (expected a string) meant either "send an array" or "item 3 is wrong". It names the index: index 1 to be a number, and reports every bad index rather than stopping at the first.

Two things the OpenAPI text claimed more broadly than the code delivers, now stated:

• A declared string format is not enforced. A Date field takes any string.
• A widget's option list is not checked. Dropdown / RadioGroup / CheckboxGroup carry their values in widgetEdit.parameters.static.options, not enums. Enforcing them is the wrong call, not just missing work: a searchable dropdown computes its options per query, so the static list is not exhaustive and rejecting against it would refuse valid values.

Cross-SDK check on the record-picker branch, since it decides between "packed id string" and the PK's numeric type. Python emits reference (agent-python/.../generator_action.py:76) and so does Ruby (agent-ruby/.../generator_action.rb:110), both alongside the PK column type, like Node. No gap.

Also: test/action/action-execute-validation.test.ts:158 claimed "before executing" without asserting the spy — fixed, both branches assert it now.

Adjacent bug found and deliberately left alone: agent-client/src/action-fields/file-value.ts:69 tests isFile(value) on a single object, so an array of File objects sent to a ['String'] or Json field passes through and serializes as {"buffer":{"type":"Buffer",...}}. Pre-existing, and through the BFF the new validator now happens to catch it — but mcp-server calls the same setFields with no validator, so that path still writes corrupt data and reports success. Worth its own ticket.

[nbouliol]

Round 1 findings all addressed — the shared field-type module is the right seam. Remaining points below are minor; CI's only red is the unrelated ai-proxy LLM integration test.

[nbouliol]

This dropped the closing line the comment had on main (// omitted its hooks, so "static" is never certain.), so the sentence now ends on "that simply".

[nbouliol]

All six threads from the earlier rounds are addressed in code: the primitive table, the enum-options predicate and the required-field predicate now share read-model/field-type.ts and action-form-mapper.ts, mapActionForm covers a legacy EnumList, the record-picker schema keeps its required description, and a malformed reference publishes as null. The three repros and the canExecute gate are each gated and tested before execute().