You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Accept string-keyed options and array default_value in list-fields output - #3581
formidable-forms/list-fields returned the generic MCP "An error occurred while executing the tool." for some forms. WordPress validates ability output against the schema, and the list-fields output schema was stricter than what Formidable stores, so a single field failed the whole form:
options was type: array, but choice fields can store options keyed by option text (or other_<n>), which encodes as an object.
default_value was type: string, but array defaults (Address, multi-default checkbox) are stored as JSON and decoded to an array.
What changed
options: type => array( 'array', 'object' ) with additionalProperties matching items (same shape create-field already accepts as input). Options are not passed through array_values(), since the other_<n> keys are functional.
default_value: type => array( 'string', 'array', 'object' ).
create-field / update-field output schemas are plain object with no declared properties, so they needed no change.
New test test_list_fields_accepts_string_keyed_options_and_array_default_values.
How it was verified
Ran test_FrmAbilitiesFieldsController.php locally on the WP 7.0 test lib.
Before the schema change: the new test fails with output[location][options] is not of type array.
After: the whole file passes (10 tests, 62 assertions).
The test sets default_value => '' on the select field. A field created with no default_value at all stores NULL, which the schema still rejects. That is outside this issue, so I left it alone. Say if you want prepare_field_for_response() to coerce null to an empty string too.
formidable-api's FrmAPIAbilitiesFieldsController has the same strict list-fields types and will get its own issue.
We reviewed changes in 40f899d...2f92c21 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Heads-up for review: the PHPUnit workflow fails on every matrix job with Could not read "phpunit9.xml", before any test runs. .github/workflows/phpunit.yml still points at phpunit9.xml, which 6a6ad924b ("Remove phpunit xml file...") deleted from the repo. This is separate from this diff.
I ran test_FrmAbilitiesFieldsController.php locally instead. The new test fails on the old schema with output[location][options] is not of type array, and the whole file passes with the fix (10 tests, 62 assertions).
DeepSource: PHP also reports a failure, but I can't read its details from here.
The reason will be displayed to describe this comment to others. Learn more.
Approve. The schema change is correct and the bug is reproduced and fixed live. One non-blocking gap is left for Vivi (below).
Live before/after (wp-env tests-cli, PHPUnit 9.6, the PR's test file against the base controller, then against the PR's):
Base controller + PR test: 10 tests, 1 failure, Ability "formidable-forms/list-fields" has invalid output. Reason: output[location][options] is not of type array.
PR controller + PR test: OK (10 tests, 62 assertions).
Extra probes (throwaway test, removed): through list-fields, options keyed from 1, string-keyed options with an array checkbox default, other_1 as an object option, empty options, plain list options and a string options all validate. Not tested: any PHP/WP version other than this container's.
Non-blocking, for Vivi:
default_value NULL still fails validation, as the PR body says: a text or checkbox field made with FrmField::create() and no default_value gives output[...][default_value] is not of type string,array,object. create-field stores '', so this only hits fields created another way. Adding 'null' to the type list, or coercing NULL to '' in prepare_field_for_response(), would close it. Your call, you asked in the PR.
The options description ends "Or an object keyed by option key." — the sentence is a fragment, e.g. "...when using separate values, or an object keyed by option key."
CI, not from this diff:PHPUnit fails on every matrix job because .github/workflows/phpunit.yml still points at phpunit9.xml, which master no longer has (only phpunit.xml). So PHPUnit never ran on this PR in CI. The DeepSource PHP "undefined method" comments on the new test are false positives; assertNotWPError and assertArrayHasKey exist and the test ran. PHPCS, PHPStan and Cypress are label-gated and did not run, and I did not run them.
The reason will be displayed to describe this comment to others. Learn more.
Approve. Both notes from my last review are fixed at 2f92c21; nothing outstanding in this diff.
NULL default_value: the schema type now includes 'null'. In wp-env (tests-cli, PHPUnit 9.6) the updated test file passes (10 tests, 63 assertions). Against the previous controller (d0b62d9) the new FrmField::create() text field with no default fails with output[nodefault][default_value] is not of type string,array,object, so the new assertion covers the change.
Options description: the sentence fragment is now a comma clause, and the line stays under the 180-char limit.
The other Abilities test files (contract, form actions, payments, subscriptions, entries, forms, styles) still pass with the PR's controller: 15, 9, 6, 6, 5, 8, 7 tests.
Not run: PHPCS, PHPStan, Cypress (label-gated, not triggered), and any PHP/WP version other than this container's.
CI, not from this diff:PHPUnit still fails on every matrix job because .github/workflows/phpunit.yml points at the deleted phpunit9.xml. That needs a separate fix on master.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3580
What was broken
formidable-forms/list-fieldsreturned the generic MCP "An error occurred while executing the tool." for some forms. WordPress validates ability output against the schema, and thelist-fieldsoutput schema was stricter than what Formidable stores, so a single field failed the whole form:optionswastype: array, but choice fields can store options keyed by option text (orother_<n>), which encodes as an object.default_valuewastype: string, but array defaults (Address, multi-default checkbox) are stored as JSON and decoded to an array.What changed
options:type => array( 'array', 'object' )withadditionalPropertiesmatchingitems(same shapecreate-fieldalready accepts as input). Options are not passed througharray_values(), since theother_<n>keys are functional.default_value:type => array( 'string', 'array', 'object' ).create-field/update-fieldoutput schemas are plainobjectwith no declared properties, so they needed no change.test_list_fields_accepts_string_keyed_options_and_array_default_values.How it was verified
Ran
test_FrmAbilitiesFieldsController.phplocally on the WP 7.0 test lib.output[location][options] is not of type array.The test sets
default_value => ''on the select field. A field created with nodefault_valueat all stores NULL, which the schema still rejects. That is outside this issue, so I left it alone. Say if you wantprepare_field_for_response()to coerce null to an empty string too.formidable-api'sFrmAPIAbilitiesFieldsControllerhas the same strictlist-fieldstypes and will get its own issue.🤖 Generated with Claude Code