Repository navigation
Expose opted-in ActiveAdmin member, collection and batch actions as MCP tools - #10
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Use ActiveSupport's underscore method instead of hand-rolled snake_casing to correctly convert APIKey and Admin::Volunteer style class names to tool_name identifiers, preventing collisions. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
A declared param named \`id\` on a member action (or \`ids\` on a batch action) would silently collide with the record key injected by ActionSchema, corrupting the schema and creating confusing semantics. Validate this at declaration time rather than papering over it in the schema. - Member actions reserve \`:id\` - Batch actions reserve \`:ids\` - Collection actions reserve nothing ActionSchema also ensures \`required\` is unique as belt-and-braces. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The brief's code had two critical defects:
1. Enum checking occurred before type coercion. MCP arguments arrive as JSON
(usually strings), but enums are declared in native Ruby types. A client
sending "1" for enum: [1, 2, 3] was rejected because "1" != 1.
2. Coercion failures were caught and returned as raw values. So "abc" for
type: :integer silently reached the controller as a String.
Fixes:
- Add CoercionError, raised by coerce on failure
- Coerce first, catching CoercionError and returning error
- Check enum against the coerced value, not the raw one
- For :boolean, properly handle "false"/"true"/"0"/"1" and raise on invalid
Add 6 covering tests:
- Integer enum coercion (Finding 1)
- Integer coercion failure (Finding 2)
- Boolean string coercion ("false" -> false, not silent degradation)
- Boolean coercion failure
- Required boolean param sent as false (not treated as missing)
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Also fix a latent test-harness bug the new spec exposed: support/active_admin.rb and support/active_record.rb each pointed ActiveRecord::Base at a separate anonymous :memory: SQLite database, so whichever loaded last silently stranded the other's tables. Point both at the same shared-cache database and keep a raw connection open for the process lifetime so the shared cache survives ActiveRecord::Base.establish_connection being called more than once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ispatcher spec Adds a denying ActiveAdmin::AuthorizationAdapter context proving that neutralising the namespace authentication_method does not also bypass authorization: the write is blocked and no error hash masks a real regression. Also stops leaking Volunteer/AdminUser rows across the shared in-memory test database between examples. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds action_runner_spec coverage for the untested :collection permission-proc path (allow/refuse/string-reason), and strengthens the batch action spec to prove the right records were mutated rather than just the right count, by having the harness's suspend batch action actually update selected records. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…builder ControllerDispatcher#build_controller was private and reached through send(:...). It is now the public controller_with_mcp_user, since listing-time permission checks need the same context a dispatched call gets. It also stubs the ActiveAdmin namespace's own current_user_method alongside the configured one, so an action body calling either gets the MCP user rather than nil when the two settings diverge. Batch actions could only be authorized at the class level, because there is no single record, which left the submitted ids unchecked: a client could name records the adapter's scope_collection excludes and ActiveAdmin would mutate them. ActionRunner now runs the ids back through scope_collection and refuses the whole call if any falls outside it. Refusing rather than narrowing is deliberate - a client must never believe it acted on N records when it acted on fewer. Both rescue sites interpolated the exception message into the error returned to the client, which could carry raw SQL, table names or file paths. The detail now goes to the log and the client gets a generic failure naming the resource and action. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tools/list filtered action tools only on the permission: proc and never applied the ActiveAdmin authorization adapter, so a user authorized for nothing was still shown every opted-in action tool. It now runs the same adapter check list_resources makes, against the resource class - there is no record at listing time. That gate runs before the schema is built, not after the list is assembled: ActionSchema calls the application's suggestions: procs, which read the database, so filtering afterwards would hide the tool and still hand its rows to a fully denied user. Listing-time permission: procs were called bare, so self was the RequestHandler and current_admin_user and can? were undefined. The README's own example raised NameError, was swallowed, and hid the tool from every user including superusers with no diagnostic. They are now evaluated through MethodOrProcHelper.render_in_context against a real controller, exactly as ActionRunner evaluates them at call time, and a proc that raises hides its own tool and warns rather than doing so silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nstall ActiveAdmin's batch_action controller method slices submitted inputs down to the declared form: keys, so a param declared only under mcp: was advertised to the client, possibly as required, validated, JSON-encoded - and then dropped before the block ever saw it. ActionDefinition#validate! now rejects it at declaration time, alongside the existing reserved-name rejection. Only batch actions that actually declare a form: hash are affected; without one ActiveAdmin slices nothing. ActiveAdminExt.apply! returning false makes every opted-in tool vanish from tools/list, because ActionCatalog can no longer find an mcp: option anywhere. It now warns rather than turning the whole feature off in silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README claimed every call first passes the ActiveAdmin authorization adapter. That is now true for batch actions too, but the shape differs enough to state: batch authorization is resource-level plus an id-scope check through scope_collection, not a per-record policy evaluation. Also notes that neutralising the namespace's authentication_method disables its authorization half too when an application uses a combined authentication-and-authorization method, and documents the new batch form: declaration rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
batch_form_keys conflated "not a batch action" and "batch action with no form: hash" under a single nil return, so validate! skipped its check entirely for form-less batch actions. But ActiveAdmin's batch_action controller does inputs.slice(*valid_keys), and with no form: valid_keys is nil, so slice(*nil) == slice() drops every input, not none. A required: true param declared only under mcp: would be silently discarded before the block ran. Restructure batch_form_keys into three distinct outcomes (not a batch action, form: Hash with real keys, no-form with an empty permitted set) plus an explicit skip for Proc forms that ActiveAdmin evaluates in controller context. Invert the test that pinned the wrong behaviour and add coverage for the no-form and Proc-form cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Design specs and implementation plans for work in progress are kept on disk but out of the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches the convention adopted on main for the rest of the codebase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers member_action/collection_action/batch_action opt-in via mcp:, binding enum: enforcement, required param refusal, controller-context evaluation of a zero-argument permission: proc at tools/list time, and batch action scoping to selected records only. Along the way, found and fixed a real defect the new suite exposed: ControllerDispatcher's synthesized request carried no CSRF token, so Rails forgery protection refused every non-GET member action and every batch action. Unit specs never caught this because they mock around real ActiveAdmin/Rails dispatch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…taught us The listing example showed an action without an `mcp:` key is not advertised. Hidden and not-callable are different claims, though, and a tool that were merely hidden while still dispatchable by name would be a hole rather than an untidiness — so assert the refusal too. Breaking `ActionDefinition.build` to ignore the opt-in shows what the guarantee holds back: ActiveAdmin's default `destroy` batch actions appear as `post_destroy`, `author_destroy` and `admin_user_destroy`. CLAUDE.md gains the Ruby 4.0 invocation (there is no .ruby-version, and the failure when you miss it does not name the cause), and the lessons this work paid for: assert the side effect rather than the response, assert an untouched control record when an example is about which records were affected, prefer `include` over exact tool lists, and remember the seed environment is not the server's environment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lloydwatkin
force-pushed
the
member-actions-forms-hints
branch
from
September 19, 2026 08:30
512dcee to
45ce5f2
Compare
…by action type Two fixture actions and the examples they exist for: `member_action :explode` raises, proving a failing action comes back as a generic error naming the resource and action while the exception's own message — which can carry SQL, table names and file paths — stays out of the MCP client's reach. `member_action :feature` carries a `permission:` proc taking the record that returns a refusal string for a draft post. That covers three things nothing did before: a record-aware proc keeps its tool advertised, because it cannot be resolved at listing time; the refusal reaches the client in the proc's own wording; and the proc is consulted per record rather than refusing outright, which the accompanying allow case is there to pin. The examples are now grouped by action type, so `--format documentation` reads as a specification of what each kind of ActiveAdmin action does over MCP. Grouping surfaced a gap: nothing asserted a batch action's param types come from its ActiveAdmin `form:` hash rather than its `mcp:` declaration, so that example is added too. Each new example was verified able to fail — by leaking `e.message` from the dispatcher, by ignoring a string returned from a permission proc, and by not inheriting `form:` types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md asks for a long description over a comment explaining a short one. The comment on the unknown-tool example carried the reason that example exists, which belongs in the output someone reads when it fails; the one on the permission allow case only repeated what its description already said. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 19, 2026
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.
Closes #8.
Lets an application expose individual ActiveAdmin actions —
member_action,collection_actionandbatch_action— to MCP clients as tools, executed through the real ActiveAdmin controller so authorization and callbacks apply exactly as they do in the admin UI.This is phase 1 of the design in #8. Phases 2 (a
createtool, and movingupdateonto controller dispatch) and 3 (richer form description) are deliberately not included — see "Not in this PR" below.How an application opts in
Nothing is exposed unless an action says so, via an
mcp:key on the options hash it already passes:That registers a
volunteer_create_warningtool. Batch actions opt in the same way and inherit their param types from theform:hash they already declare.ActiveAdmin carries unknown option keys through untouched —
ResourceDSL#actionstrips only:title, and the router reads justnameandhttp_verb— so the engine's entire footprint here is anmcp_optionsreader mixed intoControllerActionandBatchAction, kept in one file with a guard spec that fails loudly if a future ActiveAdmin stops tolerating extra keys.Design notes
enum:binds,suggestions:advises. A staticenum:is validated before dispatch. Asuggestions:proc is evaluated at listing time, surfaced as JSON Schemaexamples, and never enforced — so it can pull live values from the database. A raising suggestions proc costs its suggestions, never the tool listing.permission:can only narrow. Every call first passes the resource's ActiveAdmin authorization adapter, exactly asqueryandupdatedo today; the proc is an additional gate on top, evaluated in controller context socurrent_admin_userandcan?are in scope. Returning aStringrefuses with that string as the reason, which gives an agent something actionable.Execution synthesizes a request rather than routing one. Routing would mean issuing a second HTTP request that authenticates by forging an admin session — a back door this gem should not have. Instead the request is built directly and the MCP-authenticated user injected onto the controller instance, so the namespace's authentication callback can be neutralised while authorization keeps running in full. A spec with a denying authorization adapter proves the write is blocked.
Batch calls are scoped. Requested ids are checked against the authorization adapter's
scope_collectionand the whole call refused if any fall outside — deliberately stricter than ActiveAdmin's ownbatch_action, which passescollection_selectionthrough unscoped.Results return status, redirect target and flash — never the rendered HTML body. Redirect-style (submit-side) actions are the supported case; a GET rendering a full admin view is best-effort.
Not in this PR
Phase 2 (
create, andupdatemoved onto dispatch so ActiveAdmin's callbacks fire — a behaviour change to an existing tool) and phase 3 (describe_form). Phase 2's central question — whetherRecordUpdater'sfrom_formpermit-params fallback still earns its place once writes dispatch — is answered by having this PR's dispatcher in hand, which is why it was not planned up front.Testing
Rebased onto
mainand brought in line with its conventions: Ruby 4.0.7, nofrozen_string_literalmagic comments.157 unit examples and 27 end-to-end examples, all green in CI.
Per
CLAUDE.md, every MCP action is covered end to end, not only by unitspecs.
spec/e2e/mcp_actions_spec.rbdrives a real generated Rails +ActiveAdmin + Devise application over HTTP with a real API token, and the
fixture application gained four actions under
spec/e2e/fixture_app/for itto bite on: an opted-in
member_actionwith a bindingenum:, one with nomcp:key at all, an opted-inbatch_action, and acollection_actionwhosezero-argument
permission:proc callscurrent_admin_user.The e2e suite found a real bug the unit specs could not. Rails' forgery
protection refused the synthesized request, so every
method: :postmemberaction and every batch action — batch always dispatches non-GET — failed
against a real application. The unit specs mock around real dispatch, so they
were green throughout.
ControllerDispatchernow marks the synthesizedrequest as verified, alongside the existing authentication no-op: the MCP
request has already authenticated by bearer token and carries no session, so
CSRF has nothing to protect here. Authorization is untouched and still runs in
full.
Each e2e example was verified able to fail by breaking the thing it covers and
watching it go red. Breaking the opt-in guarantee is the instructive one: it
exposes
post_destroy,author_destroyandadmin_user_destroy, ActiveAdmin'sdefault destroy batch actions, which is precisely what opt-in holds back.
Unit specs stay double-based in the existing style; the dispatcher and runner
specs boot a real minimal ActiveAdmin, since neither controller dispatch nor
the ActiveAdmin coupling can be honestly covered by doubles.
The design spec and implementation plan that drove this work are kept out of the repository (
docs/is gitignored); the spec's content is reproduced in #8.🤖 Generated with Claude Code