Derive update fields from the ActiveAdmin form block - #5
Open
lloydwatkin wants to merge 1 commit into
Open
Conversation
RecordUpdater resolved writable fields solely from the controller's
permitted_params, which only exists when a resource declares
permit_params. Resources that instead declare their fields through a
`form do ... end` block leave ActiveAdmin's default permitted_params
returning nil, so the MCP `update` tool crashed ("undefined method
'[]' for nil") and no record could be updated.
Prefer permit_params when present, and otherwise derive the permitted
fields from the resource's form inputs via a FormFieldCollector that
runs the form block against a recording stand-in builder. Fail closed
when neither can be resolved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The MCP
updatetool could not update any record on resources that declare their writable fields through aform do … endblock instead ofpermit_params.RecordUpdater#permitted_attributesresolved fields solely from the controller'spermitted_params; for a form-only resource ActiveAdmin's defaultpermitted_paramsreturnsnil, so the tool crashed with:This blocks a very common ActiveAdmin style — e.g. the consumer that surfaced this has 141 registered resources, only 17 of which declare
permit_paramswhile ~92 declare aformblock.Change
permit_paramswhen it's declared (unchanged behaviour).formblock. A newFormFieldCollectorruns the form block against a recording stand-in form builder: everyinput :fieldrecords:field;inputs/actions/semantic_errors/helper calls and any other message are swallowed (returning self) so arbitrary Formtastic DSL —as: :hidden,collection: Country.pluck(:name), conditionals,select2— executes without a real view context.has_manyassociations are intentionally not descended into (the updater only writes flat attributes).permit_paramsnor a form block can be resolved, raise rather than silently permitting all columns.Verification
bundle exec rspec— 63 examples, 0 failures (7 new specs covering the form-derivation path, arbitrary DSL tolerance, and both fail-closed cases).Users::Addressresource (whose form callsCountry.order(:name).pluck(:name),as: :hidden,select2): it extracts exactly the 12 form fields with no errors.🤖 Generated with Claude Code