Issue #3518120: Define tools with attributes on typed methods

Matt Glamanrequested to merge
issue/tool-3518120:3518120-attribute-defined-tools into 1.0.x

Closes #3518120 (closed)

Ready for review as one MR. The pieces that stand on their own already landed separately (see Reviewing it); what is left only makes sense together, and splitting it further would land attributes that nothing reads.

Three people ran the branch against real modules before it was ready for review: Dries (photo albums over MCP), svendecabooter (translation export), and f0ns (a booking service with a read and a write tool, kernel and FunctionalJavascript tests). Their findings are folded in below.

The problem

Exposing behavior that already exists takes a plugin class per tool. tool_belt's SystemStatus is 141 lines around one call to SystemManager::listRequirements(). Dries counted 123 lines for his find_image tool, 40 of which do the search. The rest is the class, create(), checkAccess(), and definition arrays that restate what the method signature already says.

The idea

Put the description on the method. Parameters are inputs, the return value is the output, the docblock supplies the text.

A service method:

/**
 * Publishes an album and returns the number of tracks made visible.
 *
 * @param \Drupal\node\NodeInterface $album
 *   The album to publish.
 * @param bool $notify
 *   Whether to email subscribers.
 *
 * @return int
 *   The number of published tracks.
 */
#[ToolMethod(id: 'album.publish', operation: ToolOperation::Write)]
#[ToolOutput(name: 'track_count')]
public function publish(
  #[ToolEntityInput(op: 'update')] NodeInterface $album,
  bool $notify = TRUE,
): int {

That is the tool album.publish: a required entity:node input, an optional boolean defaulting to TRUE, one integer output, and access decided by the node access handler's update check on the album passed in. No plugin class.

A service method that returns plain PHP objects. This one replaces tool_belt's 141-line SystemStatus plugin:

final class Requirement {

  /**
   * @param string $key
   *   The requirement key, such as "php" or "update_status".
   * @param string $title
   *   The requirement title.
   * @param string $value
   *   The current value, such as a version number.
   * @param string $description
   *   Explains the value or what to do about it.
   * @param \Drupal\Core\Extension\Requirement\RequirementSeverity $severity
   *   How serious the state is.
   */
  public function __construct(
    #[ToolProperty]
    public readonly string $key,
    #[ToolProperty]
    public readonly string $title,
    #[ToolProperty]
    public readonly string $value,
    #[ToolProperty]
    public readonly string $description,
    #[ToolProperty]
    public readonly RequirementSeverity $severity,
  ) {}

}

/**
 * Lists the site's runtime requirements, the status report as data.
 *
 * @return \Drupal\tool_belt_poc\Requirement[]
 *   The requirements, most severe first.
 */
#[ToolMethod(id: 'tool_belt_poc.status_reporter.list_requirements', operation: ToolOperation::Read, permission: 'administer site configuration')]
#[ToolOutput(name: 'requirements')]
public function listRequirements(): array {

The output requirements is a list of maps with key, title, value, description, and severity, each described by its @param line. severity advertises as an integer with a Choice of the enum's cases, and the value object is published as an array by the same rule that built the schema. Only marked members are published. A bare #[ToolProperty] publishes the member as its type and @param line say; its arguments rename, retype, or describe when inference gets it wrong. The plugin it replaces advertised one untyped map.

An existing plugin, same attributes, no new base class:

#[Tool(
  id: 'greeting_tool',
  label: new TranslatableMarkup('Greeting Tool'),
  description: new TranslatableMarkup('Generates a greeting message.'),
  operation: ToolOperation::Transform,
  permission: 'access content',
)]
final class GreetingTool extends ToolBase {

  /**
   * @param string $name
   *   The name to greet.
   * @param string|null $greeting
   *   The greeting to use.
   *
   * @return string
   *   The generated greeting message.
   */
  #[ToolOutput(name: 'message')]
  public function __invoke(string $name, ?string $greeting = NULL): string {
    return ($greeting ?? 'Hello') . ", $name!";
  }

}

input_definitions and output_definitions the attribute still declares are kept as written and the rest are inferred, so a plugin converts one definition at a time.

What is inferred

  • int, float, string, bool, entity classes (entity:TYPE, or entity when several types match), backed enums (a Choice on the cases; the method receives the case), DateTimeInterface (datetime_iso8601; the method receives the object).
  • Defaults and nullability make an input optional.
  • Docblock types where PHP cannot say enough: string[], list<T>, and array{title: string, year?: int} shapes, nested, wrapped across lines. {@inheritdoc} follows the interface.
  • A return shape is an output map, one typed output per key. A value object in a return type is described from the members it marks with #[ToolProperty], and published as an array by the same rule. A class that marks nothing fails discovery instead of publishing what it encodes to.
  • config: 'system.site' on an input or output builds the definition from config schema.
  • Anything it cannot type fails discovery with a message naming the parameter. #[ToolInput(definition: ...)] and #[ToolOutput(definition: ...)] take the same definition objects #[Tool] does.

Access is never inferred. A service method declares a permission, an entity operation with #[ToolEntityInput(op:)], or an access method on the same service. Every entity parameter must carry #[ToolEntityInput]. A method that declares no check fails discovery.

Reviewing it

Landed on 1.0.x while this branch was open, and this branch builds on them:

Not in this MR:

  • #3583059 (!184), config entities as machine names. Attribute-defined tools do not need it.

The code reads best bottom up. Each layer only depends on the ones before it:

Layer Classes Tests
Docblock types DocType, MethodDocBlock DocTypeTest, MethodDocBlockTest (unit)
Value objects ValueObject, ValueObjectMember, #[ToolProperty] ValueObjectTest (unit)
Describer #[ToolInput], #[ToolEntityInput], #[ToolOutput], TypedMethodDescriber, MethodSignature MethodToolTest
Typed plugins ToolBase::doExecute(), TypedInvocationTrait, ToolManager::processDefinition() InvokablePluginTest, InvalidTypedPluginTest
Method discovery ToolMethodPass, MethodToolDiscoveryDecorator, MethodToolDefinitionBuilder, MethodTool MethodToolTest

docs/developers/tool-methods.md is the user-facing guide, and the fixtures under tests/modules/tool_test/src/Method double as examples.

Changes to existing behavior

  1. ToolBase::doExecute() is no longer abstract. Its default calls __invoke(). Existing plugins override it and are unaffected. A concrete plugin with neither is logged and left out at discovery.
  2. ToolManager::__construct() takes @entity_type.manager, @config.typed, @entity_type.bundle.info, the %tool.method_tools% parameter, and @logger.channel.tool, appended in tool.services.yml. ToolManager is final, so this only affects code that calls new ToolManager() itself, such as a test.
  3. ContextDefinitionNormalizer omits description when a definition has none, instead of repeating the label. This changes the schema of every tool, not only method tools.
  4. A Choice constraint object with a callback, the kind added in hook_tool_info_alter(), now produces an enum and feeds near-miss input coercion. Before, it crashed the normalizer under Choice and produced no enum under AllowedValues.
  5. A service that carries #[ToolMethod] is made public at container build, since the tool fetches it by ID at run time.
  6. tool_post_update_method_tools() is empty; running it rebuilds the container for the new ToolManager arguments.
  7. ToolBase uses TypedInvocationTrait, so every plugin inherits the protected methods typedMethodSignature(), invokeTyped(), and entityInputAccess(). A plugin that declares a method with one of those names must rename it.

For the release notes: a method tool reports its own \InvalidArgumentException as a failed run, while a #[Tool] plugin still reports it as an input error. A plugin converted to a method should throw InputException for messages meant for the caller.

Measured: the service scan

The pass has to know which service classes carry the attribute. On this site, 915 services, opcache on:

Classes Time
Autoload and reflect every service class 915 134 ms
Reflect them once loaded 915 1 ms
ConfigAction's EntityMethodDeriver, for scale 106 17 ms
Read each class file for the attribute name, load only matches 915 10.5 ms
Cold container rebuild, whole 330 to 440 ms

The cost was autoloading, not reflection. The pass reads the source first and loads nothing else, so it stays zero-config. A service that inherits an attributed method without naming the attribute in its own file opts in with the tool.methods tag.

Settled questions

Fons answered these in the MR thread.

  • Namespace. Everything stays under Drupal\tool\Tool\Method. The parsers and the describer also serve typed __invoke() plugins, but they are @internal and can move later without a break.
  • inputs: on #[ToolMethod]. Dropped. #[ToolInput(definition:)] does the same job on the parameter it replaces, with no second list keyed by parameter name. Adding it later costs nothing; removing it after release would break code.
  • Provider. The lowercased first namespace segment, the way core's attribute discovery derives it: Drupal\album\AlbumManager gives album, Drupal\Core\... gives core. A vendor class falls back to tool.
  • Value objects as inputs. Deferred. Outputs only; a parameter typed with one fails discovery, so support can come later without a break.
  • Core. ToolProperty and ToolMethod (with the ToolOperation enum) copy across. definition: on ToolInput, ToolEntityInput, and ToolOutput is typed against this module's definition interfaces, so it is the part that will not copy straight.

Feedback from real modules

Dries: photo albums over MCP

Dries integrated the branch into his photo album module and served it over MCP: one service method, three lines, against a value object with a promoted constructor and 18 getters. Discovery, inference, nested value objects, nullability, and the MCP hints all worked without configuration. His notes turned into these commits:

  • Opt-in members. The first cut published every public property or getter unless it was marked ignore: TRUE. Dries's class has 13 internal getters and 7 to publish, and the 14th getter would have leaked. Now a member is published only when it carries #[ToolProperty], the way JMS Serializer's ExclusionPolicy('all') and Jackson's JsonAutoDetect(NONE) work. A returned class that marks nothing fails discovery, naming the class, so forgetting is loud rather than a leak. ignore is gone.
  • Members at the call site. One tool may want fewer members than the class marks. #[ToolOutput(members: ['title', 'url', 'caption', 'date'])] publishes those and nothing else, in that order, and only their getters ever run. A name in the list is itself a mark, so it can also select a public property or getter of a class you cannot annotate, and a named ToolProperty in the list refines it there. A name that is not a member fails discovery.
  • What a string contains. media_type on #[ToolInput], #[ToolOutput], and #[ToolProperty] is emitted as the JSON Schema contentMediaType keyword, through the MediaType constraint from #3583076 (closed). This matches Canvas, which uses contentMediaType: text/html and image/* on its component props.
  • No description that repeats the title. A definition without a description no longer gets its label as the description. That is a change to ContextDefinitionNormalizer for every tool, not only method tools.
  • Docs open with the single-attribute example and say getters must be cheap and side-effect free, since they run when a value is published.

Two of his points stay outside this MR. tools/list over MCP carries no outputSchema, so nothing describes the data in the structuredContent envelope the bridge already returns; that is a mcp_server_tool_bridge change, filed as mcp_server_tool_bridge#3618724 (closed), and Tool API already has the schema ready. And static methods or autowirable classes as tools would be a second place to discover tools, which the same reasoning that ruled out entity classes rules out here.

svendecabooter: translation export

Sven built a translation-export tool on the branch. His review turned into:

  • One broken tool does not take down discovery. An invalid method tool or typed plugin is logged on logger.channel.tool and left out, instead of throwing for every tool on the site. The access checks 1.0.x runs on every #[Tool] plugin are the exception: a plugin with no permission and no access override, or with a permission that mixes , and +, still fails discovery for the site, as it does without this MR.
  • ExecutableResult returns. A typed method may return an ExecutableResult to choose its own message and failure category. It is used as is, so its outputs need data_type, definition, or config.
  • id is required on #[ToolMethod]. A default built from the service ID and method name would change silently on a rename, and agent, ECA, and MCP configuration refer to tools by ID.
  • snake_case members. Value object properties publish in snake_case.
  • Nested call-site selection. members: ['langcode', 'documents' => ['uri', 'problems']].

f0ns: a booking service

Fons served two tools from one service, booking.slots (Read) and booking.book (Write), over MCP, with kernel and FunctionalJavascript tests. Discovery, defaults and nullability, data_type: 'email', Choice callbacks, ExecutableResult returns, and nested #[ToolOutput] definitions all worked. His testing found that a declared Choice on a list<string> parameter had moved onto the list instead of each item, and that a Choice object's callback was never called. Both are fixed on the branch with tests.

Follow-ups

  • Infer outputs from @return ExecutableResult<array{...}>, so a method that returns a result object keeps output inference. Fons has a working patch and will file it once this lands.
  • A public way to describe a value object outside a tool call, so a Single Directory Component's props can use the schema the tools publish. TypedMethodDescriber::valueObjectOutput() builds it but is private. Dries has a describeValueObject(string $class, array $members = []) wrapper and offered a patch (comment).
  • Tighten the docblock parser. DocType and MethodDocBlock are about 420 lines that phpstan/phpdoc-parser would replace, but core does not require it, so neither does this module. Every type spelling the parser does not know fails discovery, so most of this work only makes more methods discoverable. Only the @phpstan-* item changes a schema that works today:
    • Merge ValueObjectMember::parseVar() into MethodDocBlock. It strips the comment on its own and does not join a type wrapped across lines, so an @var array{...} shape wrapped at 80 columns fails while the same shape on @param works.
    • Fail with a message that names the supported spellings. positive-int, non-empty-string, class-string, and int<0, 10> fail today with "not a class that can be loaded. Write classes in docblocks fully qualified", which sends a developer the wrong way.
    • Read @phpstan-param, @phpstan-return, and @phpstan-var, or document that they are skipped. Today the plain tag or the native type is used without a warning. Reading them later changes the schema of any tool that has them.
    • If core ever requires phpstan/phpdoc-parser, directly or through symfony/type-info, replace both classes with it. symfony/type-info also resolves use statements, so docblock class names would no longer need to be fully qualified.
  • $ref and shared definitions in the normalizer, so a value object used twice is described once.
  • Value objects as inputs.
  • Title and description split, #3583032 (closed).

What it does to tool_belt

tool_belt's 44 plugins sort into four groups under this model:

Group Tools What happens
Thin wrapper over a core service method 3, such as SystemStatus Two attribute lines on the core method. The class goes away.
Wrapper over an entity method 12, such as UserBlock A five-line attributed service method. The class goes away.
ID-or-label lookup around a core call 7, such as WorkspaceSwitch The lookup becomes an entity-typed parameter. The class goes away.
Orchestration with real logic 22, such as EntityList Stays a plugin with a typed __invoke() and loses its definition arrays.

UserBlock is the clearest case of the second group. Today it is 93 lines around $user->block()->save(), gated on the administer users permission. As an attributed service method it is five lines, and the access rule gets stronger, because it asks the user access handler about this account instead of checking one site-wide permission:

/**
 * Blocks a user account.
 */
#[ToolMethod(id: 'user.block', operation: ToolOperation::Write, destructive: TRUE)]
#[ToolOutput(name: 'user')]
public function block(#[ToolEntityInput(op: 'update')] UserInterface $user): UserInterface {
  $user->block()->save();
  return $user;
}

44 classes become 22, and 7,968 lines become roughly 3,200. SystemStatus already runs this way in a proof of concept: one attributed method returning a Requirement value object, no plugin class, with a richer schema than the 141-line plugin advertises because the output is typed instead of a bare map.

For core itself to carry these attributes, the attribute classes have to live somewhere core may reference, the way ActionMethod does for config actions. Discovery and execution can stay in this module, since they read attributes and do not have to own them. So the path is: land the attributes here, move only the attribute classes to core, then core services gain them one method at a time in ordinary core issues.

The full tool_belt analysis, with worked examples for SystemStatus, UserBlock, WorkspaceSwitch, and EntityList

tool_belt (1.0.x at 862d960) ships 44 tool plugins, 7,968 lines in all. Every one of them wraps behavior that already exists in core: a service method, an entity method, or a storage call. This document walks through what happens to those plugins when the attributes in this merge request are applied, and what the code in Drupal core would have to look like for the plugin classes to go away.

The system status example below runs as working code with a kernel test in a local proof of concept module. It cannot change core, so the method sits on a small service of its own; in core it would be a method on SystemManager.

The prerequisite nobody should skip

A core service method can only carry #[ToolMethod] if the attribute class is somewhere core is allowed to reference. Today it is Drupal\tool\Attribute\ToolMethod in a contrib module. PHP would tolerate that, since an attribute's class is only loaded when something calls newInstance(), but core policy would not, and it should not.

So the honest sequence is:

  1. The attributes land in the tool contrib module, in this merge request.
  2. The attribute classes, and only those, move to a core component, the way Drupal\Core\Config\Action\Attribute\ActionMethod did for config actions. Discovery and execution can stay in contrib for as long as needed, because they read attributes; they do not have to own them.
  3. Core services grow attributes, one method at a time, in normal core issues.

Everything below assumes step 2 has happened and writes #[ToolMethod] on core code as if it were Drupal\Core\Tool\Attribute\ToolMethod.

Four kinds of tool_belt tool

Reading all 44, they fall into four groups. The group decides what happens to each.

Group Tools What changes
A. Thin wrapper over a core service method SystemStatus, DisplayMessage, LogMessage The plugin class disappears. The core method gets two attribute lines.
B. Wrapper over an entity method UserBlock, UserUnblock, UserAddRole, UserRemoveRole, WorkspacePublish, WorkspaceRevert, most workspace tools The plugin class is replaced by a five-line attributed service method in core. Tools are discovered on services only, on purpose.
C. Orchestration with real logic EntityList, EntityLoadByProperty, FieldAdd, EntityBundleUpdate, EntityStub, FieldSetValue, most of tool_belt_entity and tool_belt_content The class stays, because the logic is the tool. It becomes a typed __invoke() plugin and loses its definition arrays and most of its boilerplate.
D. Wrapper whose real job is input resolution WorkspaceSwitch, WorkspaceExists, WorkspaceGetActive, EntityLoadById The resolution moves into the input type. #[ToolEntityInput] on a WorkspaceInterface parameter replaces a lookup by ID or label; handles carry the entity across the wire.

The counts: 3 tools in group A, 12 in group B, about 22 in group C, 7 in group D. Groups A, B, and D are 22 classes and roughly 3,000 lines that become attributes on methods core already has or should have. Group C keeps its classes but drops the attribute arrays, create(), and the getContextValue plumbing.

Group A worked example: SystemStatus

tool_belt_system/src/Plugin/tool/Tool/SystemStatus.php is 141 lines. It calls SystemManager::listRequirements(), reshapes the result, and declares three outputs: a map called status_report, and two booleans, has_warnings and has_errors. The map is untyped, so an agent is told "array containing the system status report data" and nothing else.

What core has today
// core/modules/system/src/SystemManager.php
/**
 * Displays the site status report. Can also be used as a pure check.
 *
 * @return array
 *   An array of system requirements.
 */
public function listRequirements() {

Untyped return, arrays of arrays with title, value, description, and a severity that is sometimes an int and sometimes a RequirementSeverity.

The minimal core change

Two attribute lines and a documented return shape:

/**
 * Displays the site status report. Can also be used as a pure check.
 *
 * @return array<string, array{title: string, value?: string,
 *   description?: string, severity?: \Drupal\Core\Extension\Requirement\RequirementSeverity}>
 *   The requirements keyed by requirement key.
 */
#[ToolMethod(id: 'system.requirements', operation: ToolOperation::Read, permission: 'administer site configuration')]
#[ToolOutput(name: 'requirements')]
public function listRequirements() {

That produces the tool system.requirements with no inputs and one map output. It is honest but still weak: array<string, ...> has unknown keys, so the schema cannot list the properties, and the values are loosely typed because the underlying arrays are. The has_warnings and has_errors outputs are gone, but they were derived: an agent that sees a severity per row can derive them too.

The change core actually wants

Core added the RequirementSeverity enum in 11.x and is moving away from the REQUIREMENT_* constants. The next step in that direction is a value object, and it is the same step that makes the tool output fully typed:

// core/lib/Drupal/Core/Extension/Requirement/Requirement.php
final class Requirement {

  public function __construct(
    public readonly string $key,
    public readonly string $title,
    public readonly string $value,
    public readonly string $description,
    public readonly RequirementSeverity $severity,
  ) {}

}

// core/modules/system/src/SystemManager.php
/**
 * Lists the site's runtime requirements, the status report as data.
 *
 * @return \Drupal\Core\Extension\Requirement\Requirement[]
 *   The requirements, most severe first.
 */
#[ToolMethod(id: 'system.requirements', operation: ToolOperation::Read, permission: 'administer site configuration')]
#[ToolOutput(name: 'requirements', members: ['key', 'title', 'value', 'description', 'severity'])]
public function getRequirements(): array {

Requirement carries no Tool API attribute; the tool names the members it publishes. The prototype reads those promoted properties, takes each description from the constructor's @param lines, and turns the enum into an integer with a Choice constraint on its cases. The advertised JSON Schema is an array of objects with key, title, value, description, and severity in [-1, 0, 1, 2]. That is more than the 141-line plugin advertised, from about 20 lines that core would benefit from with or without tools.

This is what the proof of concept runs, with two differences: its StatusReporter service wraps SystemManager instead of being it, and it marks the members on its own Requirement class, since it owns the class.

Group A, the other two

DisplayMessage (123 lines) wraps Messenger::addMessage():

// core/lib/Drupal/Core/Messenger/Messenger.php
/**
 * Adds a new message to the queue.
 *
 * @param string|\Drupal\Component\Render\MarkupInterface $message
 *   The message to show.
 * @param string $type
 *   The message type: status, warning, or error.
 * @param bool $repeat
 *   Whether an identical message may be added again.
 */
#[ToolMethod(id: 'messenger.add_message', operation: ToolOperation::Trigger, permission: 'access content')]
public function addMessage(
  $message,
  #[ToolInput(constraints: ['Choice' => ['choices' => [self::TYPE_STATUS, self::TYPE_WARNING, self::TYPE_ERROR]]])]
  $type = self::TYPE_STATUS,
  $repeat = FALSE,
): static {

Two things to note. The $message parameter is a union in the docblock, so the docblock would need string alone or the parameter needs #[ToolInput(data_type: 'string')]. And the fluent static return becomes no output, which is right: the plugin's displayed: TRUE output carried no information.

LogMessage (145 lines) is a 25-line switch on the level name around LoggerChannel::log(). The tool is log() itself with a Choice on the level, and the switch goes away with the class.

SendEmail (143 lines) does not belong in this group. MailManager::mail() takes a module and key that mean nothing to a caller, so the tool is the thing that chooses them. It stays a plugin and moves to group C.

Group B worked example: UserBlock

tool_belt_user/src/Plugin/tool/Tool/UserBlock.php is 93 lines around one call:

$user->block();
$user->save();

The behavior lives on the entity, not on a service, and tools are discovered on services only. That is deliberate: one place to look. So the core change is a service method whose first parameter is the entity:

// core/modules/user/src/UserAccountManager.php
/**
 * Blocks an account so it can no longer log in.
 *
 * @return \Drupal\user\UserInterface
 *   The blocked account.
 */
#[ToolMethod(id: 'user.block', operation: ToolOperation::Write, destructive: TRUE)]
#[ToolOutput(name: 'user')]
public function block(#[ToolEntityInput(op: 'update')] UserInterface $user): UserInterface {
  $user->block()->save();
  return $user;
}

Five lines instead of ninety-three, and the access rule is stronger than the plugin's: the plugin checked administer users, this checks update on the specific account, which the user access handler already knows how to decide, including that user 1 is special.

ConfigAction scans config entity classes for #[ActionMethod], and the same scan over content entities would let User::block() itself carry the attribute. It was considered and set aside: it is a second place for tools to hide, and a service method is the natural home for an operation that also saves.

Group D worked example: WorkspaceSwitch

WorkspaceSwitch (175 lines) takes a string, tries it as an ID, then as a label, and calls WorkspaceManager::setActiveWorkspace(). Sixty of its lines are the lookup, shared through a trait with six sibling tools.

// core/modules/workspaces/src/WorkspaceManager.php
/**
 * Sets the active workspace via the workspace negotiators.
 *
 * @return $this
 */
#[ToolMethod(id: 'workspaces.switch', operation: ToolOperation::Write)]
public function setActiveWorkspace(
  #[ToolEntityInput(op: 'view')] WorkspaceInterface $workspace,
  bool $persist = TRUE,
): static {

The lookup is gone because the input is the workspace. An MCP client passes a handle it got from a previous call; a PHP caller passes the entity. What is lost is "by label", which the plugin offered and an agent will miss. The right place for that is a separate read tool, WorkspaceRepository::findByLabel(), one attributed method rather than sixty lines copied into seven plugins.

Group C worked example: EntityList

EntityList (362 lines) builds an entity query from up to eight inputs, runs it, and returns entities with a total. There is no core method to attribute; the plugin is the logic. It converts to a typed plugin in place:

#[Tool(
  id: 'tool_belt:entity_list',
  label: new TranslatableMarkup('List entities'),
  description: new TranslatableMarkup('Lists content entities of a type, filtered, sorted, and paged.'),
  operation: ToolOperation::Read,
)]
final class EntityList extends ToolBase {

  /**
   * Lists entities.
   *
   * @param string $entity_type_id
   *   The entity type machine name.
   * @param array<string, mixed> $conditions
   *   Field conditions keyed by field name.
   * @param int $limit
   *   Maximum number of entities to return.
   * @param int $offset
   *   Number of entities to skip.
   *
   * @return array{entities: \Drupal\Core\Entity\ContentEntityInterface[], total: int}
   *   The matching entities and the total before paging.
   */
  public function __invoke(
    #[ToolInput(constraints: ['PluginExists' => ['manager' => 'entity_type.manager', 'interface' => ContentEntityInterface::class]])]
    string $entity_type_id,
    array $conditions = [],
    int $limit = 50,
    int $offset = 0,
  ): array {
    // The 120 lines of query building stay exactly as they are.
  }

  protected function checkAccess(array $values, AccountInterface $account, bool $return_as_object = FALSE): bool|AccessResultInterface {
    // Unchanged.
  }

}

The attribute shrinks from roughly 90 lines of definitions to 6. create() and the $values destructuring go. The two outputs come from the return shape. The query code, which is the reason the tool exists, is untouched. Across group C that is the pattern: about a third of each file disappears and what remains is the part a reviewer wanted to read anyway.

What does not get better

  • Inputs that are lookups by name. entity_type_id as a string with a PluginExists constraint is exactly as it was. That is fine; it is a string.
  • Untyped core. Where core returns arrays of arrays, the tool is only as typed as the docblock someone writes. The Requirement value object is the fix, and it is a core improvement in its own right.
  • Entity methods. Group B needs a service method per operation, because tools are discovered on services only. Five lines instead of ninety.
  • Handles from the CLI. drush tool:run cannot pass an entity by ID today, so group D tools are less convenient from Drush than the string lookup they replace. That is a Drush command gap, filed separately.

Numbers

Today With attributes
Group A, 3 tools 409 lines, 3 classes 6 attribute lines on 3 core methods, 0 classes
Group B, 12 tools about 1,700 lines, 12 classes 12 five-line attributed service methods
Group D, 7 tools about 1,000 lines, 7 classes 14 attribute lines on core methods plus one findByLabel() read tool
Group C, 22 tools about 4,900 lines, 22 classes about 3,200 lines, 22 classes, no definition arrays

The plugin count drops from 44 to 22, the lines by more than half, and every remaining line is behavior rather than description of behavior.

AI-Generated: Yes (Claude Code produced the implementation, tests, and this description under my direction; I reviewed the design decisions and the code).

Edited by Matt Glaman

Merge request reports