dev!: require string $name for registration and introduce ability_class arg

[justlevine]

What

This PR changes both wp_register_ability() and WP_Ability_Registry::register() to only take a string $name, instead of either a name or a pre-instantiated WP_Ability.

Also removed the "Optional" text $properties param doc - since they aren't in fact optional, they just silently error with a _doing_it_wrong().

(Note: I'm just code-reviewing and leaving feedback. If you think this warrants a wider discussion I can open a matching issue, otherwise I think a discussion here following by [accept | reject] is enough)

Why

Per the rest of the inline docs, neither WP_Abilities_API or WP_Ability are meant to be called directly, so there isn't really a scenario where wp_register_ability() should be called with a pre-instantiated WP_Ability().

Keeping the API intentionally limited makes the potential for breaking API changes (and unintentional misuse/adoption), making both our and users lives easier.

[gziolo]

It's a similar behavior to block types registry:

https://github.com/WordPress/wordpress-develop/blob/197f0a71ad27d0688b6380c869aeaf92addd1451/src/wp-includes/class-wp-block-type-registry.php#L33-L53

Per the rest of the inline docs, neither WP_Abilities_API or WP_Ability are meant to be called directly, so there isn't really a scenario where wp_register_ability() should be called with a pre-instantiated WP_Ability().

The documentation included promotes usage through wp_register_ability helper function. What problems do you anticipate in allowing a developer to construct an instance of WP_Ability in the way it works better for them?

[justlevine]

It's a similar behavior to block types registry:

https://github.com/WordPress/wordpress-develop/blob/197f0a71ad27d0688b6380c869aeaf92addd1451/src/wp-includes/class-wp-block-type-registry.php#L33-L53

To me this is a great example of what we want to avoid. Since it was merged in 5.0 without (as far as I can tell) a specific use case in mind, it's now irremovable tech debt. WP_Block_Type and WP_Block_Type_Registry both need extra code and tests to maintain the possible usage patterns and hook lifecycle for each.

In contrast, look how the WP6.5 wp_register_font_collection() or register_block_template() errors if given an instance of the type they're meant to register.

• https://github.com/WordPress/wordpress-develop/blob/946649af8db44b4c969c092d2f77643a1dc2ec4e/src/wp-includes/fonts.php#L96
• https://github.com/WordPress/wordpress-develop/blob/946649af8db44b4c969c092d2f77643a1dc2ec4e/src/wp-includes/block-template.php#L411

The documentation included promotes usage through wp_register_ability helper function.

I'ts more than "promotes", the WP_Ability class is specifically marked as @internal and inline docs say not to use. That will fail lints and confuse both human and🤖 contributors and needs to be changed if we wish to support string|\WP_Ability (along with some other things see below)

What problems do you anticipate in allowing a developer to construct an instance of WP_Ability in the way it works better for them?

Biggest problem is that this is a 6.9 core merge candidate so none of us really have time to anticipate problems before it becomes tech debt. It's a nonbreaking change to make $names a union if justified in the future. We cannot remove it once it's in core.

"Smaller" problems:

• WP Lifecycle is super finicky. By only allowing names, we don't need to worry about any WP_Ability() hooks being called too early (or a second time by the duplicate).
• WP Core wont support union params in type signatures for years minimally, which means instead of getting immediate errors, downstream developers either need linting + phpdocs or to run and trigger the problematic code to know you're passing an incorrect value to $name. (Current gen 🤖 particularly appreciate method signatures )
• A thorough implementation that supports string|\WP_Ability on all three levels means adding more boilerplate than we have now. I didn't look in depth since I was focused on removing but quick e.g. all the validation layer stuff right now is bypassed as run-time fatals about missing/invalid class property names which is problematic when the user is proving an array of $args.

tl;dr

So really (and in light of the WP philosophy on decisions, not options) IMO the question is reversed: if this is only about conceptual flexibility and we don't have a specific use case in mind, then why complicate both the dx and the maintenance so early, and in an irreversible way

[artpi]

Hey @justlevine !
I am a person that pushed @gziolo to include the ability to pass WP_Ability.

You are right - backwards compatibility is both a biggest advantage of WP and its biggest problem.

there isn't really a scenario where wp_register_ability() should be called with a pre-instantiated WP_Ability().

if this is only about conceptual flexibility and we don't have a specific use case in mind, then why complicate both the dx and the maintenance so early, and in an irreversible way

The reason to allow for instantiating a preregistered Ability is to introduce the way to extend the behavior of WP_Ability without merging entire API to do to. It's opening the door slightly so that more advanced use-cases are not yet supported by the api.

Let me give you a simplified example of what we need. This is a tool that is currently used in some capacity on production and I would love to rewrite it to Abilities API. It is meant to let the model rewrite content in specific gutenberg blocks on a page and it does call openai on its own

class Content_Rewrite_Tool extends AI_Tool {
	private const REWRITE_CONTENT_PROMPT = <<<PROMPT
		## GOAL
		Your sole purpose is to rewrite the content of blocks based on the user's request.
		....
	PROMPT;

	public array $parameters;

	public function __construct() {
		$this->name        = 'rewrite_content';
		$this->description = 'Rewrite content to match a user request. If a user requests a change in tone but doesn\'t provide a specific tone, ask for clarification.';

		$this->parameters = [
			'userRequest'   => [
				'type'        => 'string',
				'description' => 'The user\'s request describing how to rewrite the content.',
			],
			'blockClientId' => [
				'type'        => 'string',
				'description' => 'The client ID of the block containing the content to rewrite.',
			],
			'followUpTasks' => [
				'type'        => 'boolean',
				'description' => 'Set to true if the user request has to be broken into multiple steps and other tasks remain to be done after this one.',
			],
		];
	}

	public function invoke( array $arguments ): \WPCOM\AI\Tool_Call_Result {
		$user_request    = $arguments['userRequest'] ?? '';
		$block_client_id = $arguments['blockClientId'] ?? '';

		// Get the page content from context
		$page_content = ...

		// Get all blocks to rewrite (either single block or block with inner blocks)
		$blocks_to_rewrite = $this->get_blocks_to_rewrite( $block_client_id, $page_content );


		// Prepare the prompts
		$blocks_data_prompt  = '<blocks>' . json_encode( $blocks_to_rewrite ) . '</blocks>';
		$user_request_prompt = '<user_request>' . $user_request . '</user_request>';

		$system_prompt = \WPCOM\AI\Context::process(
			$this->context,
			self::REWRITE_CONTENT_PROMPT
		);

		// Parameters for the rewrite tool to handle multiple blocks
		$parameters = [
			'type'                 => 'object',
			'required'             => [ 'updates', 'summary' ],
			'additionalProperties' => false,
			'properties'           => [
				'updates' => [
					'type'        => 'array',
					'description' => 'Array of block updates with clientId and rewritten content.',
					'items'       => [
						'type'                 => 'object',
						'required'             => [ 'clientId', 'content' ],
						'additionalProperties' => false,
						'properties'           => [
							'clientId' => [
								'type'        => 'string',
								'description' => 'The client ID of the block to update.',
							],
							'content' => [
								'type'        => 'string',
								'description' => 'The rewritten content for this block.',
							],
						],
					],
				],
				'summary' => [
					'type'        => 'string',
					'description' => 'A brief summary of the changes made.',
				],
			],
		];

		require_lib( 'openai' );
		$openai   = new \OpenAI( $this->agent->feature ?? 'big-sky' );
		return $openai->request_chat_completion(
			[
				[
					'role'    => 'system',
					'content' => $system_prompt,
				],
				[
					'role'    => 'user',
					'content' => $blocks_data_prompt . "\n" . $user_request_prompt,
				],
			],
			null,
			'gpt-4.1-2025-04-14',
			array(),
			[
				[
					'type'     => 'function',
					'function' => [
						'name'        => 'apply_block_edits',
						'description' => 'Apply the rewritten content.',
						'parameters'  => $parameters,
					],
				],
			],
			'text',
			null,
			false,
			600
		);


	}

	/**
	 * Get blocks to rewrite based on the selected block.
	 *
	 * @param string $client_id      The client ID of the selected block.
	 * @param mixed  $page_structure The page structure to search in.
	 * @return array Array of blocks to rewrite.
	 */
	private function get_blocks_to_rewrite( $client_id, $page_structure ) {
		$selected_block = $this->get_block_by_client_id( $client_id, $page_structure );
		if ( ! $selected_block ) {
			return [];
		}

		// Collect all content blocks from the selected block and its children
		$blocks_to_rewrite = [];
		$this->collect_content_blocks( $selected_block, $blocks_to_rewrite );

		return $blocks_to_rewrite;
	}

	/**
	 * Recursively collect content blocks that should be rewritten.
	 *
	 * @param array $block              The block to process.
	 * @param array &$blocks_to_rewrite Array to collect blocks.
	 */
	private function collect_content_blocks( $block, &$blocks_to_rewrite ) {
		// Check if this block has content that can be rewritten
		$rewritable_blocks = [ 'core/heading', 'core/paragraph', 'core/button' ];

		if ( in_array( $block['name'], $rewritable_blocks, true ) ) {
			// Clean the block data to remove originalContent which contains HTML tags
			$clean_block = [
				'clientId'    => $block['clientId'],
				'name'        => $block['name'],
				'attributes'  => $block['attributes'],
				'innerBlocks' => isset( $block['innerBlocks'] ) ? $block['innerBlocks'] : [],
			];

			// Only add blocks that have content
			if ( $block['name'] === 'core/button' && ! empty( $block['attributes']['text'] ) ) {
				$blocks_to_rewrite[] = $clean_block;
			} elseif ( in_array( $block['name'], [ 'core/heading', 'core/paragraph' ], true ) && ! empty( $block['attributes']['content'] ) ) {
				$blocks_to_rewrite[] = $clean_block;
			}
		}

		// Process inner blocks
		if ( ! empty( $block['innerBlocks'] ) && is_array( $block['innerBlocks'] ) ) {
			foreach ( $block['innerBlocks'] as $inner_block ) {
				$this->collect_content_blocks( $inner_block, $blocks_to_rewrite );
			}
		}
	}

	/**
	 * Get block by client ID from page structure.
	 *
	 * @param string $client_id      The client ID to search for.
	 * @param mixed  $page_structure The page structure to search in.
	 * @return array|null The found block or null.
	 */
	private function get_block_by_client_id( $client_id, $page_structure ) {
		return $this->find_block_recursive( $page_structure, $client_id );
	}

	/**
	 * Recursively search for a block by client ID.
	 *
	 * @param array|mixed $blocks    The blocks to search through.
	 * @param string      $target_id The target client ID.
	 * @return array|null The found block or null.
	 */
	private function find_block_recursive( $blocks, $target_id ) {
		if ( ! is_array( $blocks ) ) {
			return null;
		}

		foreach ( $blocks as $block ) {
			if ( isset( $block['clientId'] ) && $block['clientId'] === $target_id ) {
				return $block;
			}

			// Check innerBlocks if they exist
			if ( isset( $block['innerBlocks'] ) && is_array( $block['innerBlocks'] ) ) {
				$found = $this->find_block_recursive( $block['innerBlocks'], $target_id );
				if ( $found ) {
					return $found;
				}
			}
		}

		return null;
	}
}

Keep in mind this is simplified, but what we found is that simple abilities are not enough. because

• We found early on that we need to pass what we call context - random bag of useful information that will be filled into prompts to "hydrate them" with additional information about the user, product, page, etc.
• We need to share this context with tools as the tool prompts need to be hydrated too
• Some tools need a fair amonut of helper logic, which can be neatly encapsulated inside the WP_Ability class having it be self-contained
• Some tools will need something akin to state. In this case the tool is aware of the agent calling it in $this->agent for exmaple for logging purposes. This is also something we found - if we have complex tools used by multiple agents, they need to be aware of whats using them even if only for logging.

Now technically this could have been all achieved by having a seperate class tied into wp_ability with hooks using its methods and I could keep this complexity "on the side" of wp_ability. I, as a developer could do that but:

1. What do I need WP_Ability for then?
2. For what purpose I would introduce this much complexity?

What we found is that these complex abilities are needed sooner than we expected - a simple clean stateless ability sounds nice in theory but in real production use cases this extendability is needed pretty soon.

Every way that I could think of was working against this API introducing even more technical debt and lifecycle problems and this seemed like the cleanest - it at least acknowledges the need and channels these advanced use cases into one place.

I would hate to see community to come up with for example global variables to manage state in abilites.

[justlevine]

Hey @artpi thanks so much for the considered and detailed response 🤩

I want to rubberduck a few different approaches (including just some internal polish on the polymorphic approach) using the trimmed down example and will try and get back here by my EOD with a real reply.

In the interim some quick notes (mainly so I don't forget while I'm AFK),

• We need a way to 'extend' WP_Ability functionality, and extending the class is the best flexibility v. tech debt ratio.
• We need to document common usage patterns for the parts we dont implement, or may wind up being forced to support antipatterns .
• A partially unsealed $args is still IMO the best way to to handle state/context/etc without forcing core into more premature entry points/apis, but it's either not enough or needs a clear usage example to illustrate.
  • cc @gziolo (without ...<unsealed array shape> users can only pass the args we explicitly list. And if we allow users to new MyType() extends WP_Ability(), we'd need it there too or args they add would be a contravariant violation). At minimum of the constructor boilerplate or param juggling from the example would be needed)
• Context != a resolver chain. Abilities need to be able to infer where they are without being told (and without having to keep the calling instance around.
• possibities (nonexhaustive):
  • filtering the WP_Ability class by $name before instantiation (e.g. block_parser_class)
  • adding straight to the registry without reinstantiation
  • keeping as is but still somehow stabilizing the (future) hooks inside WP_Ability.
  • DI (it's not a WordPress pattern, but will be the easiest to futureproof when core has an 'approved way' to handle ability_supports( 'context' ) or whatever that API looks like. Technically WPGraphQL does this for $context.
  • The 'WP' way of using hooks instead of extending WP_Ability. Not ideal considering we also need to capture non-WP developers, but falls squarely into 'making the easy things easy, and the hard things possible', which has been the mantra so far.
  • Waiting until later in pre6.9 or even core v7.0 for class-based promotion. Absolutely worst case, and sounds like the fine folks at VIP will shoot me, but the 6.9 milestone is the minimum requirements to support the MCP Adapter, and AI Experiments. We have time to reach this API holistically

[gziolo]

(without ... users can only pass the args we explicitly list. And if we allow users to new MyType() extends WP_Ability(), we'd need it there too or args they add would be a contravariant violation). At minimum of the constructor boilerplate or param juggling from the example would be needed)

I'm not sure I fully understand the problem here. You pass either a ability name (string) and its properrties (array with a specific shape documented with PHPStan annotation), or an instance of an object that extends WP_Ability that has known definition. The return value is always a WP_Ability instance (the same that was passed, or a new one that was created).

I think the root of the problem is method overloading (same method name, different parameter signatures) here which is a standard feature in many languages like Java, C++, C#, or even it is supported in TypeScript. I don't know how far we can go with it in PHP, and the existing tooling. However, I can think of some ways how to mitigate the potential challenges for extenders:

• we can limit wp_register_ability to support only the canonical way with the name and properties, but keep the flexibility inside WP_Abilities_Registry
• we can offer wp_register_ability_from_instance (potentially add something similar to WP_Abilities_Registry if we want to have clarity with all types used)

adding straight to the registry without reinstantiation

We pass the instance directly after very light verification of the name:

abilities-api/includes/abilities-api/class-wp-abilities-registry.php

Lines 66 to 97 in f36c71f

	$ability = null;
	if ( $name instanceof WP_Ability ) {
	$ability = $name;
	$name = $ability->get_name();
	}
	if ( ! preg_match( '/^[a-z0-9-]+\/[a-z0-9-]+$/', $name ) ) {
	_doing_it_wrong(
	__METHOD__,
	esc_html__(
	'Ability name must be a string containing a namespace prefix, i.e. "my-plugin/my-ability". It can only contain lowercase alphanumeric characters, dashes and the forward slash.'
	),
	'0.1.0'
	);
	return null;
	}
	if ( $this->is_registered( $name ) ) {
	_doing_it_wrong(
	__METHOD__,
	/* translators: %s: Ability name. */
	esc_html( sprintf( __( 'Ability "%s" is already registered.' ), $name ) ),
	'0.1.0'
	);
	return null;
	}
	// If the ability is already an instance, we can skip the rest of the validation.
	if ( null !== $ability ) {
	$this->registered_abilities[ $name ] = $ability;
	return $ability;
	}

The 'WP' way of using hooks instead of extending WP_Ability. Not ideal considering we also need to capture non-WP developers, but falls squarely into 'making the easy things easy, and the hard things possible', which has been the mantra so far.

For now, I would prefer to focus on the hooks that optimize for 3rd party usage like plugins, so they could modify things like

• input/output schema
• permission check
• execution result

I'm also thinking about a filter that allows removing some of the abilities when fetching the full list from the registry, in the case when site owner would like to filter something out in a given context.

filtering the WP_Ability class by $name before instantiation (e.g. block_parser_class)

That might work. We could offer an init_callback as another alternative which would default to function( $name, $properties ) { return new WP_Ability( $name, $properties ) }.

I'ts more than "promotes", the WP_Ability class is specifically marked as @internal and inline docs say not to use. That will fail lints and confuse both human and🤖 contributors and needs to be changed if we wish to support string|\WP_Ability (along with some other things see below)

Right, the @internal part is peculiar in WordPress land. In general, you don't want it to be widely used and advertised in the developers handbook, but you also know this class will have to be supported forever because folks will use directly 😄

In this case, we should not mark it internal, because devs need to use the public API like WP_Ability::execute().

So really (and in light of the WP philosophy on decisions, not options) IMO the question is reversed: if this is only about conceptual flexibility and we don't have a specific use case in mind, then why complicate both the dx and the maintenance so early, and in an irreversible way

I think it's mostly the question how this is integrated with tooling and how it's documented for devs so they could benefit from it. I wouldn't like if we limit the creativity of developers or don't respect their design choices. At the same time, as @justlevine noted, we don't want to run into backward compatibility lock so we can't evolve the API as we see need for new use cases.

[codecov]

Codecov Report

❌ Patch coverage is 32.00000% with 17 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (trunk@ad22938). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
includes/abilities-api/class-wp-ability.php	8.33%	11 Missing ⚠️
...udes/abilities-api/class-wp-abilities-registry.php	50.00%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             trunk      #21   +/-   ##
========================================
  Coverage         ?   89.82%           
  Complexity       ?       94           
========================================
  Files            ?        7           
  Lines            ?      511           
  Branches         ?        0           
========================================
  Hits             ?      459           
  Misses           ?       52           
  Partials         ?        0           

Flag	Coverage Δ
unit	89.82% <32.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[justlevine]

Hey folks, sorry it took longer to get back here than expected but I wanted to make sure I really worked through it all (and confirmed how much refactoring @artpi 's example would minimally require to work with the existing WP_Ability vs the current wp_register_ability() and some of the other intermediary patterns.

adding straight to the registry without reinstantiation
We pass the instance directly after very light verification of the name:

Yeah, that's the problem IMO. It means we cant rely that the things masquerading as a WP_Ability inside the registry have all the properties the registry is supposed to guarantee.

Which is why:

I think it's mostly the question how this is integrated with tooling and how it's documented for devs so they could benefit from it. I wouldn't like if we limit the creativity of developers or don't respect their design choices. At the same time, as @justlevine noted, we don't want to run into backward compatibility lock so we can't evolve the API as we see need for new use cases.

Unless I misunderstood @artpi ("without merging entire API to do so") this isn't about tooling, its about being about being able to minimize the migration requirements in order to get preexisting AI tools onto WP_Ability_Registry.

This sort of polymorphism IMO treats (and forces us to treat) WP_Ability as an interface, and means that neither extenders/tools in the chain, other parts of core, or even WP_Ability itself wouldn't be able to rely on

• base class properties - aka our already super-minimal api.
• lifecycle - if new My_Extends_WP_Ability() can be called anywhere, then we cant rely the order whatever future hooks are called in.

Both of these severely limit any future evolution of APIs, so if the Chesterton Fence around current WPCore code patterns isnt enough to deter this, please at least consider that.

---

In the last commit I add 39bf7c9 which adds the wp_ability_class filter allowing for all sorts of WP_Ability polymorphism .

With it, @artpi 's example would go from

add_action( 'abilities_api_init', static function() {
  // ... did some stuff to justify this.
  $tool_instance = new Content_Rewrite_Tool(); // if we're lucky. If this is before 'abilities_api_init' the possible hook order is even less stable.
  wp_register_ability( $tool_instance );
} );

to

add_filter( 'wp_ability_class', static function( string $class_name, string $ability_name ) {
  return 'wpcom/rewrite_content' === $ability_name ? Content_Rewrite_Tool::class : $class_name;
, 10, 2 );
add_action( 'abilities_api_init, static function() {
  // maybe do some stuff, maybe not. our API is immune.
  wp_register_ability( Content_Rewrite_Tool::NAME, Content_Rewrite_Tool::PROPS ); // or however they refactor to minimally comply with WP_Ability's public signature.
  }

In other words, for just 2 extra LOC (and IMO likely more saved elsewhere), we can support the exact same use case without the other downsides. Plus, core actually has a way to deprecate hooks, unlike the first parameter of a global function or public method.

(Not that I think this will get deprecated. It's an existing and modern core pattern, clean and performant, and provides an easy path forward to experiment with and eventually core support Ability Types, Ability Chains, Nested Abilities, etc without any of the extra baggage).

---

Separately wanted to confirm that the rest of concerns / needs raised can be done by passing variables to the different closures on $properties. Could not find anything that either isnt doable or already required by extending WP_Ability in PHP8.0+ (let alone with strict_types.)

E.g.

wp_register_ability(
  'wpcom/rewrite_content',
  [
    ...$other_args,
    'execute_callback' => function( array $input ) {
      $user_request    = $input['userRequest'] ?? '';
      $block_client_id = $input['blockClientId'] ?? '';

      // Calling a function from a class.
      $blocks_to_rewrite = $this->get_blocks_to_rewrite( $block_client_id, $page_content );
      // calling a singleton method.
      $system_prompt = \WPCOM\AI\Context::process(
        $this->context,
        self::REWRITE_CONTENT_PROMPT
      );
  
      try {
        require_lib( 'openai' );
       $openai   = new \OpenAI( $this->agent->feature ?? 'big-sky' );
 
       return $openai->doSomething();
     } catch ( \Throwable $e ) {
       return new WP_Error(
         'my-code'
         $e->getMessage()
       );
   },
 ]
);

@artpi please double check me with your IRL tool, but from the example provided your short-term migration path would likely be shorter this way than trying to conform with WP_Ability. You'd basically be refactoring your constructor to feed those values into a wp_register_ability() call, with $properties['do_callback'] = fn ( $input ) => $this->invoke( $input), and leave the rest of your lifecycle in place until you have time to move over (or a post-6.9 release adds the future APIs that justify such a refactor).

[artpi]

Hey @justlevine , Im definitely open to those and i havent had the time to digest it properly, but here is some early feedback:

add_filter( 'wp_ability_class', static function( string $class_name, string $ability_name ) {
  return 'wpcom/rewrite_content' === $ability_name ? Content_Rewrite_Tool::class : $class_name;
, 10, 2 );
add_action( 'abilities_api_init, static function() {
  // maybe do some stuff, maybe not. our API is immune.
  wp_register_ability( Content_Rewrite_Tool::NAME, Content_Rewrite_Tool::PROPS ); // or however they refactor to minimally comply with WP_Ability's public signature.
  }

The idea of a filter that whitelists a class is fine definitely, but in this example we have a props problem:
It is a very common pattern we discovered that ability needs to be able to set its props dynamically, especially the enum field. With the option to extend wp_ability we can do so in the constructor.

Regarding this example:

Could not find anything that either isnt doable or already required by extending WP_Ability in PHP8.0+ (let alone with strict_types.)

wp_register_ability(
  'wpcom/rewrite_content',
  [
    ...$other_args,
    'execute_callback' => function( array $input ) {
      $user_request    = $input['userRequest'] ?? '';
      $block_client_id = $input['blockClientId'] ?? '';

      // Calling a function from a class.
      $blocks_to_rewrite = $this->get_blocks_to_rewrite( $block_client_id, $page_content );
      // calling a singleton method.
      $system_prompt = \WPCOM\AI\Context::process(
        $this->context,
        self::REWRITE_CONTENT_PROMPT
      );
  
      try {
        require_lib( 'openai' );
       $openai   = new \OpenAI( $this->agent->feature ?? 'big-sky' );

The problem here is state. With classes, consumers of abilities can ammend them during execution context adding stateful things like $this->agent->feature.

I'll think of how to make it work with your concerns, but come to think of it, my reasons for pushing for extending WP_Ability are:

1. State - some abilities need to be stateful. That includes dynamic input / output props (again, especially enums) and passing some metadata for logging where stuff gets instantiated or dependency-injected during the lifecycle
2. Easier organization of commmon patterns - like for exmaple we could have WP_Ability_Edit_Post_Type and that could be extended to WP_Ability_Edit_Post.

[justlevine]

Thanks @artpi !

The idea of a filter that whitelists a class is fine definitely, but in this example we have a props problem:
It is a very common pattern we discovered that ability needs to be able to set its props dynamically, especially the enum field. With the option to extend wp_ability we can do so in the constructor.

Custom props can still be passed, they would just be passed via a custom key on the $properties array:

class Content_Rewrite_Tool extends \WP_Ability {
  private ?MyStateDTO $state;
  private array $some_other_complex_prop;
  // other WP_Ability overrides.
}

...

wp_register_ability( 'wpcom/rewrite_content', [
  ...$otherProperties,
  'state' => $my_state_or_whatever,
  'some_other_complex_prop' => $nested_array_or_whatever
] );

(Technically you could even #AllowDynamicProperties and overload the __construct() to support not just dynamic properties, but nondeterminate ones if you really wanted to)

---

The problem here is state. With classes, consumers of abilities can ammend them during execution context adding stateful things like $this->agent->feature.

Not sure I understand this: is $this->agent supposed to a WP_Ability that you're stuffing with a feature, or is it a prop that you're passing to \WP_Ability?

my reasons for pushing for extending WP_Ability are:

To clarify, I don't believe we ever disagreed about the importance of being able to extend WP_Ability. I just

• thought we could wait and iterate over the next few months instead of needing it urgently for v0.1.0
• still think that there are ways to do it without needing to compromise on lifecycle stability or a holistic API to do so.

1. State - some abilities need to be stateful. That includes dynamic input / output props (again, especially enums) and passing some metadata for logging where stuff gets instantiated or dependency-injected during the lifecycle
2. Easier organization of commmon patterns - like for exmaple we could have WP_Ability_Edit_Post_Type and that could be extended to WP_Ability_Edit_Post.

All of this is possible with the current PR and straightforward with this API (either wp_register_api() alone, or by extending the WP_Ability class with the filter), and more importantly without needing to manually and early instantiate a new WP_Ability()

I'm happy to keep pseudocoding if you think of some other examples to try while you're reviewing - it's good prior art both for the docs, and the soon to commence experiments work.

[justlevine]

As we're getting closer to cutting an initial release, please remember that even if this gets merged, we can continue the conversation and restore the polymorphic string|WP_Ability $name in a v0.1.1 nonbreaking release. The other way (leaving things as they are and merging after v0.1.0 is cut) is a breaking change.

Still here and happy to assuage any additional concerns people make have.

[gziolo]

Looks good to me. I'm fine with starting with the strict approach that still allows to use a custom implementation of class extending WP_Ability.

[gziolo]

I took an attempt to see how the example provided by @artpi woulf fit. It looks like the following would be possible with the changes proposed:

class Content_Rewrite_Tool extends WP_Ability {

	public array $parameters;

	public function __construct( string $name,  array $properties  ) {
		parent::_construct( $name, $properties );

		$this->parameters = [
			'userRequest'   => [
				'type'        => 'string',
				'description' => 'The user\'s request describing how to rewrite the content.',
			],
			'blockClientId' => [
				'type'        => 'string',
				'description' => 'The client ID of the block containing the content to rewrite.',
			],
			'followUpTasks' => [
				'type'        => 'boolean',
				'description' => 'Set to true if the user request has to be broken into multiple steps and other tasks remain to be done after this one.',
			],
		];
	}

	// All other methods. 
 }

$result = wp_register_ability(
	'wpcom/rewrite_content,
	array(
		'label' => 'rewrite_content',
		'description' => 'Rewrite content to match a user request. If a user requests a change in tone but doesn\'t provide a specific tone, ask for clarification.',
		'execute_callback' => array( Content_Rewrite_Tool::class, 'invoke' ),
		'ability_class' => Content_Rewrite_Tool::class,
	)
);

invoke on Content_Rewrite_Tool would have to be static to fit here because execute_callback is mandatory at the moment during ability registration (label and description, too). If that is too limiting, we could revisit $properties validation. I also think that we shold make $properties mandatory as you still must provide these 3 properties so validation passes before the ability instance gets created.