Resolve permit_params in controller context, and two form-description fixes

CHANGELOG.md

@@ -88,6 +88,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `activeadmin-mcp-X.Y.Z.mcpb`, so installing the bundle no longer means
   digging a build artifact out of the Actions tab.
 
+### Fixed
+
+- A resource whose `permit_params` is a block was refused by `create`, `update`
+  and `describe_form` with `Resource declares no permit_params, so nothing may
+  be written`, which was not merely unhelpful but wrong. ActiveAdmin
+  `instance_exec`s such a block on the controller, and the gem resolved it
+  against a bare controller instance, so a block reading `current_admin_user`
+  raised `NameError` — and the rescue that exists to recognise a resource with
+  no `permit_params` at all swallowed it. The block is now resolved against a
+  controller carrying the MCP user, as a dispatched call gets.
+
+- `describe_form` reported no writable attributes at all for a resource whose
+  `form` block declares none of its own — `form do |f| f.inputs; f.actions end`,
+  the shape of ActiveAdmin's own default form, where Formtastic expands the
+  inputs only at render time. There is nothing in such a block to read, so the
+  description now falls back to the resource's permitted params, as it already
+  did for a resource with no `form` block.
+
+- `describe_form` reported the fields of an `inputs for: :association` block as
+  attributes of the record being described, rather than as a nested group.
+  They belong to the associated record and `create` and `update` will drop
+  them. `has_many` was already handled this way; both routes into an
+  association's fields now behave the same.
+
 ### Changed
 
 - A batch action's own ActiveAdmin `:if` proc is now honoured: one the admin UI

CLAUDE.md

@@ -64,6 +64,38 @@ a snapshot taken after migrating and seeding. So examples must not depend on
 what another one left behind, and the suite runs in random order to keep that
 honest. Write each one as though it runs alone, because it might.
 
+### Cover the declaration shapes, not only the behaviours
+
+The rule above asks for an e2e test per MCP action. That is necessary and it is
+not sufficient, because it is organised around what this gem does rather than
+around what the applications it reads look like. Nearly every defect found in
+review so far has been the same thing: a shape of ActiveAdmin declaration that
+no fixture used, so no test could fail on it.
+
+So, for any ActiveAdmin construct this gem reads, enumerate the forms
+ActiveAdmin accepts and make sure a fixture exists for each. For example
+`permit_params` may be a list, a block, a block that reaches for controller
+state, absent entirely, or set on the namespace; a `form` block may declare
+inputs, declare none and leave Formtastic to expand a bare `f.inputs`, or nest
+with `has_many` or `inputs for:`; a `batch_action` may be named with a symbol
+or a String title, and its `form:` may be a hash or a proc.
+
+**When you read ActiveAdmin's source to answer a question, enumerate the
+branches you did not take.** Two of those defects were in lines already quoted
+in the notes justifying the change — `block ? instance_exec(&block) : args` and
+a bare `f.inputs` in ActiveAdmin's own default form. The information was not
+missing; the question "what else does this line permit?" was never asked.
+
+**Mutation checking does not cover this, and can disguise it.** Breaking a
+fixture and watching the example fail proves the test is sensitive to that
+fixture. It says nothing about a shape no fixture has, and no mutation of the
+existing fixtures will ever reveal one. Treat a passing mutation check as
+evidence about test sensitivity, and never describe it as evidence of coverage.
+
+When the shapes are invented rather than observed, they tend to match whatever
+was just built. Prefer shapes taken from real applications or from ActiveAdmin's
+own source and test suite.
+
 ### Writing an e2e example
 
 `E2E::McpClient` speaks the protocol: `tools_list` returns the `tools/list`

README.md

@@ -174,6 +174,13 @@ Two limits worth knowing:
 - A field a form block declares but `permit_params` omits is described and then
   silently dropped on write. This cannot arise on the `permit_params` fallback
   path.
+- On the `permit_params` fallback path, only fields backed by a database column
+  are reported. ActiveAdmin keeps no list of the params it permits — only a
+  method that filters against them — so the names are recovered by offering it
+  every column the model has and seeing which survive. Permitted params that
+  are not columns, such as `tag_ids` or a nested `*_attributes` key, are
+  therefore missing from the description even though `create` and `update`
+  will accept them.
 
 ### Running member, collection and batch actions
 

lib/activeadmin_mcp/form_description.rb

@@ -29,8 +29,12 @@ def call(action: "new")
       refusal = write_refusal(write_action)
       return refusal if refusal
 
+      # An empty result is not the same as no form block, but it means the same
+      # thing here: ActiveAdmin's own default form is a bare `f.inputs` that
+      # Formtastic expands only at render time, so a block written that way has
+      # nothing in it to read and the permitted params are all there is.
       declared = declared_inputs
-      return describe(action, "form", declared) if declared
+      return describe(action, "form", declared) if declared&.any?
 
       permitted = permitted_inputs
       return permit_params_refusal unless permitted
@@ -114,9 +118,13 @@ def permitted_inputs
       names.map { |name| { name: name } }
     end
 
+    # The controller carries the MCP user: ActiveAdmin instance_execs a
+    # block-form permit_params on it, so a block reading current_admin_user
+    # raises on a bare instance and the resource looks unwritable.
     def permitted_names
       param_key = @config.param_key.to_sym
-      controller = @config.controller.new
+      controller = ControllerDispatcher.new(config: @config, current_user: @current_user)
+                                       .controller_with_mcp_user
       controller.params = ActionController::Parameters.new(
         param_key => @resource[:model].column_names.index_with { nil }
       )

lib/activeadmin_mcp/form_field_collector.rb

@@ -32,7 +32,14 @@ def input(name, *_args, **options, &_block)
       self
     end
 
-    def inputs(*_args, **_opts, &block)
+    # `inputs for: :author` scopes its fields to an association, the same way
+    # has_many does; only an unscoped `inputs` groups fields of the record
+    # itself. Descending into a scoped one would advertise the associated
+    # record's fields as attributes of the record being written.
+    def inputs(*_args, **options, &block)
+      association = options[:for]
+      return nest(association, &block) if association
+
       instance_exec(self, &block) if block
       self
     end
@@ -41,11 +48,7 @@ def inputs(*_args, **_opts, &block)
     # can tell an association's fields from the record's own. Flattening them
     # would advertise `body` as an attribute of the parent record.
     def has_many(name, *_args, **_opts, &block)
-      return self unless name.respond_to?(:to_sym)
-      return self if declared?(name.to_sym)
-
-      @inputs << { name: name.to_sym, nested: block ? self.class.new.collect(&block) : [] }
-      self
+      nest(name, &block)
     end
 
     def method_missing(_name, *_args, **_opts, &block)
@@ -59,6 +62,17 @@ def respond_to_missing?(_name, _include_private = false)
 
     private
 
+    # `for:` may name the association or give it as [name, object]; only the
+    # name says anything to a client.
+    def nest(association, &block)
+      name = association.is_a?(Array) ? association.first : association
+      return self unless name.respond_to?(:to_sym)
+      return self if declared?(name.to_sym)
+
+      @inputs << { name: name.to_sym, nested: block ? self.class.new.collect(&block) : [] }
+      self
+    end
+
     def declared?(name)
       @inputs.any? { |input| input[:name] == name }
     end

lib/activeadmin_mcp/record_writer.rb

@@ -133,9 +133,15 @@ def permit_refusal(permitted)
 
     # Asks the resource's own controller what it would permit, or nil when it
     # has nothing to say because permit_params was never declared.
+    #
+    # The controller carries the MCP user, because ActiveAdmin instance_execs a
+    # block-form permit_params on it: a block reading current_admin_user raises
+    # on a bare instance, and the rescue below would then report a resource
+    # that permits plenty as one that permits nothing.
     def resolve_permitted(attributes)
       param_key = @config.param_key.to_sym
-      controller = @config.controller.new
+      controller = ControllerDispatcher.new(config: @config, current_user: @current_user)
+                                       .controller_with_mcp_user
       controller.params = ActionController::Parameters.new(param_key => attributes)
       permitted = controller.send(:permitted_params)
       scoped = permitted && permitted[param_key]

spec/activeadmin_mcp/form_description_spec.rb

@@ -80,6 +80,24 @@ def attribute(result, name)
       expect(attribute(result, "name")[:required]).to be(true)
     end
 
+    # ActiveAdmin's own default form is a bare `f.inputs`, which Formtastic
+    # expands only at render time. A resource writing that out by hand has a
+    # form block with nothing in it to read, which is not the same as a form
+    # that permits nothing.
+    it "falls back to permitted params when the form block declares no inputs of its own" do
+      result = describe_form("Roster")
+
+      expect(result[:source]).to eq("permit_params")
+      expect(result[:attributes].map { |attribute| attribute[:name] }).to eq(%w[name notes])
+    end
+
+    it "resolves a block-form permit_params, which ActiveAdmin evaluates on the controller" do
+      result = describe_form("Placement")
+
+      expect(result[:error]).to be_nil
+      expect(result[:attributes].map { |attribute| attribute[:name] }).to eq(%w[name notes])
+    end
+
     it "refuses a resource that declares no permit_params either, because nothing may be written" do
       expect(describe_form("Sighting")[:error]).to match(/permit_params/)
     end

spec/activeadmin_mcp/form_field_collector_spec.rb

@@ -107,6 +107,35 @@ def collect(&block)
     end
   end
 
+  # Formtastic's other route into an association's fields. has_many already
+  # reports one as a nested group; flattening this one would advertise the
+  # associated record's fields as attributes of the record being written.
+  it "records an inputs block scoped to an association as a nested group, as it does has_many" do
+    inputs = collect do |_f|
+      input :title
+      inputs for: :author do |a|
+        a.input :name
+      end
+    end
+
+    expect(inputs).to eq(
+      [
+        { name: :title },
+        { name: :author, nested: [{ name: :name }] },
+      ]
+    )
+  end
+
+  it "still descends into a plain inputs block, which is not scoped to anything" do
+    inputs = collect do |_f|
+      inputs "Details" do
+        input :title
+      end
+    end
+
+    expect(inputs).to eq([{ name: :title }])
+  end
+
   it "records each field once when a form declares the same input twice" do
     inputs = collect do |_f|
       input :title

spec/activeadmin_mcp/record_writer_spec.rb

@@ -8,6 +8,7 @@
   let(:sightings) { ActiveadminMcp::ResourceRegistry.find("Sighting") }
 
   after do
+    Placement.delete_all
     Volunteer.delete_all
     Sighting.delete_all
     AdminUser.delete_all
@@ -88,6 +89,31 @@ def writer(resource)
     end
   end
 
+  # ActiveAdmin instance_execs a block-form permit_params on the controller, so
+  # resolving it needs the same controller context a dispatched call gets. A
+  # bare controller instance cannot answer current_admin_user, and a resolution
+  # that rescues the resulting NameError reports the resource as declaring no
+  # permitted params at all.
+  describe "a resource whose permit_params is a block reading controller state" do
+    let(:placements) { ActiveadminMcp::ResourceRegistry.find("Placement") }
+
+    it "creates the record, writing every attribute the block permitted" do
+      result = writer(placements).create(attributes: { "name" => "Ann", "notes" => "Weekends" })
+
+      expect(result[:error]).to be_nil
+      expect(Placement.find(result[:id])).to have_attributes(name: "Ann", notes: "Weekends")
+    end
+
+    it "updates the record rather than refusing it for want of permitted params" do
+      placement = Placement.create!(name: "Ann")
+
+      result = writer(placements).update(id: placement.id, attributes: { "notes" => "Weekends" })
+
+      expect(result[:error]).to be_nil
+      expect(placement.reload.notes).to eq("Weekends")
+    end
+  end
+
   describe "updating a record" do
     let!(:volunteer) { Volunteer.create!(name: "Ann", active: true) }
 

spec/e2e/fixture_app/app/admin/assignments.rb

@@ -0,0 +1,20 @@
+# Declared in the two shapes the rest of the fixture application does not use,
+# so the e2e suite covers them rather than only the shapes the gem was written
+# against:
+#
+#   * `permit_params` as a block that reads controller state, which ActiveAdmin
+#     instance_execs on the controller;
+#   * a `form` block declaring no inputs of its own, leaving Formtastic to
+#     expand a bare `f.inputs` at render time, as ActiveAdmin's own default
+#     form does — so there is nothing in the block to read and the description
+#     has to come from the permitted params instead.
+ActiveAdmin.register Assignment do
+  permit_params do
+    current_admin_user ? %i[name notes] : %i[name]
+  end
+
+  form do |f|
+    f.inputs
+    f.actions
+  end
+end

spec/e2e/fixture_app/app/admin/reviews.rb

@@ -15,6 +15,11 @@
       f.input :body, hint: "Shown beneath the post"
       f.input :status, as: :select, collection: %w[pending approved], label: "Moderation status"
     end
+    # Formtastic's other route into an association's fields, alongside
+    # has_many: its inputs belong to the post, not to the review.
+    f.inputs for: :post do |post|
+      post.input :title
+    end
     f.actions
   end
 end

spec/e2e/fixture_app/app/models/assignment.rb

@@ -0,0 +1,6 @@
+class Assignment < ApplicationRecord
+  # See the note in post.rb: Ransack 4 requires an explicit allowlist.
+  def self.ransackable_attributes(_auth_object = nil)
+    column_names
+  end
+end

spec/e2e/fixture_app/app/models/review.rb

@@ -1,5 +1,6 @@
 class Review < ApplicationRecord
   belongs_to :post, optional: true
+  accepts_nested_attributes_for :post
 
   validates :body, presence: true
 

spec/e2e/fixture_app/db/migrate/20260101000006_create_assignments.rb

@@ -0,0 +1,10 @@
+class CreateAssignments < ActiveRecord::Migration[7.2]
+  def change
+    create_table :assignments do |t|
+      t.string :name
+      t.string :notes
+
+      t.timestamps
+    end
+  end
+end

spec/e2e/mcp_tools_spec.rb

@@ -104,6 +104,29 @@ def attribute(result, name)
       end
     end
 
+    # Two declaration shapes the rest of the fixture application does not use.
+    # Both were wrong when first shipped, and neither could fail against a
+    # suite whose every resource declared permit_params as a list.
+    context "for a resource declared in the less common shapes" do
+      let(:result) { client.call_tool("describe_form", resource: "Assignment") }
+
+      it "resolves a permit_params block that reads controller state, rather than reporting the resource as permitting nothing" do
+        expect(result["error"]).to be_nil
+        expect(result["attributes"].map { |attribute| attribute["name"] }).to eq(%w[name notes])
+      end
+
+      it "falls back to permitted params when the form block leaves its inputs to Formtastic" do
+        expect(result["source"]).to eq("permit_params")
+      end
+    end
+
+    it "reports an association's fields as nested rather than as attributes of the record itself" do
+      result = client.call_tool("describe_form", resource: "Review")
+
+      expect(result["attributes"].map { |attribute| attribute["name"] }).not_to include("title")
+      expect(result["nested"]).to include("name" => "post", "attributes" => [{ "name" => "title" }])
+    end
+
     it "describes the edit form when asked for it" do
       result = client.call_tool("describe_form", resource: "Post", action: "edit")
 
@@ -167,6 +190,19 @@ def posts
       expect(posts.length).to eq(3)
     end
 
+    it "creates a record for a resource whose permit_params is a block reading controller state" do
+      result = client.call_tool(
+        "create",
+        resource: "Assignment",
+        attributes: { name: "Saturday sort", notes: "Two volunteers" }
+      )
+
+      expect(result["error"]).to be_nil
+
+      created = client.call_tool("query", resource: "Assignment", q: { id_eq: result["id"] })["records"].first
+      expect(created).to include("name" => "Saturday sort", "notes" => "Two volunteers")
+    end
+
     it "refuses to create a record for a resource registered without the create action" do
       result = client.call_tool("create", resource: "Author", attributes: { name: "Iain" })