[vivi-the-going-merry]

Closes #3580

What was broken

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.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Strategy11/formidable-forms
• Branch: master
• File: .coderabbit.yaml
• Commit: c5b435b
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 871f7dc6-4d18-494a-b303-9c2ce612ab36

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[deepsource-io]

DeepSource Code Review

We reviewed changes in c5b435b...8f2fe8c on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
PHP		Oct 26, 2026 5:27p.m.	Review ↗
JavaScript		Oct 26, 2026 5:27p.m.	Review ↗

---

Important

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.

[deepsource-io]

Call to an undefined method test_FrmAbilitiesFieldsController::assertNotWPError()

The method you are trying to call is not defined, which can result in a fatal error.

[deepsource-io]

Call to an undefined method test_FrmAbilitiesFieldsController::assertArrayHasKey()

The method you are trying to call is not defined, which can result in a fatal error.

[deepsource-io]

Call to an undefined method test_FrmAbilitiesFieldsController::assertArrayHasKey()

The method you are trying to call is not defined, which can result in a fatal error.

[vivi-the-going-merry]

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.

[franky-the-going-merry]

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.

[deepsource-io]

Call to an undefined method test_FrmAbilitiesFieldsController::assertArrayHasKey()

The method you are trying to call is not defined, which can result in a fatal error.

[franky-the-going-merry]

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.

[Crabcyborg]

🚀

[Crabcyborg]

TODO
Resolve merge conflicts / rebase against master.

[vivi-the-going-merry]

Rebased onto master (force-push, branch unchanged). Conflict was in the options schema: kept master's numeric choices (number) alongside the string-keyed options change. test_FrmAbilitiesFieldsController.php passes locally, 11 tests, 69 assertions (includes the numeric-options test from master).

[franky-the-going-merry]

Approve at 8f2fe8c, one non-blocking note.

Reproduced the bug and the fix in a preview env (WP 7.1.3, Lite master vs this PR) by executing formidable-forms/list-fields on a form with a dropdown whose options are keyed by option text plus other_1, a multi-default checkbox and a text field with a NULL default_value:

• master: ability_invalid_output ... output[locsel][options] is not of type array
• this PR: the whole form validates and the fields come back.

The schema change itself is minimal and matches what create-field already accepts for options; array_values() is correctly not applied since the other_<n> keys matter. Psalm is red, but identically on master (MissingPureAnnotation across files this PR does not touch), so it is not from this PR.

Non-blocking: create-field (FrmAbilitiesFieldsController.php:186) and update-field (:273) still declare input default_value as type => 'string', while list-fields can now return an array/object for it. A client that reads an Address or multi-default checkbox field and sends its default_value back to update-field fails input validation, and such a default cannot be created through the ability at all. Fine as a follow-up (the PR body scopes this out), worth an issue.

Not exercised: browser/visual (no UI surface in this PR), the PHPUnit file (CI is skipped on this PR; I ran the ability itself against the same data instead), and the code-review/security-review skills (two schema arrays and one test, read by hand).