Skip to content

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Oct 7, 2025

Noted in #83.

Introduces show_in_rest arg in the server-side registry in the case devs don't want their abilities to be discoverable in REST API endpoints. By default, abilities won't get exposed which aligns with many other fundamental pieces in WordPress like post types, taxonomies, meta, etc.

It also adds a new method on WP_Ability object show_in_rest() to allow checking whether the ability should be exposed through REST API.

Example

// Ability that does not show in REST.
$ability = wp_register_ability(
	'test/not-show-in-rest',
	array(
		'label'               => 'Hidden from REST',
		'description'         => 'It does not show in REST.',
		'execute_callback'    => static function (): int {
			return 0;
		},
		'permission_callback' => '__return_true',
	)
);
// false === $ability->show_in_rest() 


// Ability that shows in REST.
$ability = wp_register_ability(
	'test/show-in-rest',
	array(
		'label'               => 'Show in REST',
		'description'         => 'It shows in REST.',
		'execute_callback'    => static function (): int {
			return 0;
		},
		'permission_callback' => '__return_true',
		'show_in_rest' => true,
	)
);
// true === $ability->show_in_rest() 

Testing

Run npm run test:php to execute extended test coverage for the new functionality.

Copy link

codecov bot commented Oct 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.17%. Comparing base (a9039a3) to head (31bd9f5).
⚠️ Report is 1 commits behind head on trunk.

Additional details and impacted files
@@             Coverage Diff              @@
##              trunk     #103      +/-   ##
============================================
+ Coverage     85.98%   86.17%   +0.19%     
- Complexity      105      110       +5     
============================================
  Files            16       16              
  Lines           792      803      +11     
  Branches         85       86       +1     
============================================
+ Hits            681      692      +11     
  Misses          111      111              
Flag Coverage Δ
javascript 92.62% <ø> (ø)
unit 83.78% <100.00%> (+0.31%) ⬆️

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.

@gziolo gziolo self-assigned this Oct 7, 2025
@gziolo gziolo added [Type] Enhancement New feature or request [Status] In Progress Assigned work scheduled labels Oct 7, 2025
@gziolo gziolo requested a review from Copilot October 7, 2025 14:38
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new show_in_rest argument to the abilities API that controls whether an ability is exposed through the REST API endpoints. When set to false (the default), abilities are hidden from REST API listing and individual endpoint access.

  • Adds show_in_rest boolean property to the WP_Ability class with validation
  • Updates REST API controllers to filter out abilities where show_in_rest is false
  • Adds comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
includes/abilities-api/class-wp-ability.php Adds show_in_rest property and has_show_in_rest() method with validation
includes/rest-api/endpoints/class-wp-rest-abilities-list-controller.php Filters abilities list and individual ability access based on show_in_rest
includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php Prevents execution of abilities not shown in REST
includes/abilities-api/class-wp-abilities-registry.php Updates documentation to include show_in_rest parameter
includes/abilities-api.php Updates function documentation for wp_register_ability
tests/unit/abilities-api/wpAbility.php Tests default behavior and explicit setting of show_in_rest
tests/unit/abilities-api/wpAbilitiesRegistry.php Tests validation of invalid show_in_rest values
tests/unit/abilities-api/wpRegisterAbility.php Updates test to verify show_in_rest property handling
tests/unit/rest-api/wpRestAbilitiesListController.php Tests filtering and 404 responses for hidden abilities
tests/unit/rest-api/wpRestAbilitiesRunController.php Tests execution blocking for hidden abilities

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@gziolo gziolo removed the [Status] In Progress Assigned work scheduled label Oct 7, 2025
@gziolo gziolo added this to the pre WP 6.9 milestone Oct 7, 2025
@gziolo gziolo marked this pull request as ready for review October 7, 2025 15:35
Copy link

github-actions bot commented Oct 7, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: gziolo <[email protected]>
Co-authored-by: JasonTheAdams <[email protected]>
Co-authored-by: justlevine <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition, @gziolo! One very minor suggestion.

@gziolo gziolo requested a review from JasonTheAdams October 8, 2025 07:19
@gziolo gziolo force-pushed the update/show-in-rest-arg branch from 9f21064 to 4ff40cb Compare October 8, 2025 08:01
@justlevine
Copy link
Contributor

@gziolo what if we relocated this to our first official "meta" property: $meta['show_in_rest']?

This would allow us to drop the extra property + public getter + usage in favor of the existing $ability->get_meta( 'show_in_rest' ), keeping the api the same size while still supporting the same functionality. It also scales better for both known and unknown future requirements (e.g. show_in_mcp, acp and whatever else), and makes more sense from a semantic perspective (and therefore DX).

Copy link
Member

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! LGTM!

@JasonTheAdams
Copy link
Member

I didn't see @justlevine's comment when I approved. Do we have a clear definition of what does and does not belong in meta?

@gziolo
Copy link
Member Author

gziolo commented Oct 8, 2025

@emdashcodes suggested in #38 (comment) that show_in_rest defaults to true. That’s the behavior before this PR, and since it’s a brand new concept I’m open to going with true. What other people think about it?

As for using meta, we didn’t arrive to conclusions but my thinking is it’s primarily for extenders to put additional details they need. When I put show_in_rest as top-level property I mirrored other concepts (post types, taxonomies, meta, options), but I don’t feel strongly about it. One thing important to keep in mind is we need a default value here and have it explicitly documented.

@justlevine
Copy link
Contributor

@emdashcodes suggested in #38 (comment) that show_in_rest defaults to true. That’s the behavior before this PR, and since it’s a brand new concept I’m open to going with true. What other people think about it?

I'm good either way. Long term it's probably going to get inferred from a public: bool prop anyway, and since as long as permission_callback is for all intents and purposes required, there's not too much risk to defaulting to true

@gziolo
Copy link
Member Author

gziolo commented Oct 9, 2025

Based on the comment from @JasonTheAdams in #106 (comment), I will land this PR as is. However, I will immediately start working on a follow-up scoped to the following:

  • Move show_in_rest to meta.show_in_rest.
  • Update the default value to true as a separate commit, in case we decide to stay with false.

@gziolo gziolo merged commit 76f9cea into trunk Oct 9, 2025
21 checks passed
@gziolo gziolo deleted the update/show-in-rest-arg branch October 9, 2025 09:00
galatanovidiu added a commit to galatanovidiu/abilities-api that referenced this pull request Oct 9, 2025
Fixes test failures after merging trunk by adding the required `category` field to `test/not-show-in-rest` abilities. These abilities were introduced in PR WordPress#103 (show_in_rest feature) on trunk before the category requirement existed.

Also fixes code bugs issues in WP_Ability class:
- Add missing PHPDoc comment opener for show_in_rest property
- Add missing PHPDoc comment opener for show_in_rest() method
- Remove extra blank line in validate_args()
@emdashcodes
Copy link
Contributor

Thanks @gziolo! I feel pretty strongly that we should default to true for the same argument that we're making categories required. If it's set to false, most people aren't going to read the documentation and enable it, and a lot of these won't automatically be discoverable by the client.

Having these opted-in by default allows for a lot of cool things like the command palette and automatically discovering them on the client and passing them to a client agent for example, and it would be nice if most of these were enabled "by default".

@justlevine
Copy link
Contributor

Having these opted-in by default allows for a lot of cool things like the command palette

(My hunch is this too will more likely be encapsulated by a show_in_cp prop that also inherited from an eventual public: bool )

@gziolo
Copy link
Member Author

gziolo commented Oct 9, 2025

I have a follow-up ready for review based on my previous comment #103 (comment):

@gziolo
Copy link
Member Author

gziolo commented Oct 9, 2025

Regarding Command Palette, the plan is to introduce the Workflows concept as a facade for consuming Abilities. The point still holds that abilities need to be exposed on the client in the first place 👍🏻

galatanovidiu added a commit that referenced this pull request Oct 13, 2025
* Implement ability categories in the Abilities API

* Add functions to register, unregister, and retrieve ability categories.
* Introduce WP_Ability_Category and WP_Abilities_Category_Registry classes for managing categories.
* Update WP_Ability class to support categories and modify the abilities retrieval process to filter by category.
* Enhance REST API to allow filtering abilities by category and include category information in responses.
* Bump version to 0.3.0 to reflect new features.

* Refactor ability category registration and retrieval logic

* Update the `register` method in `WP_Abilities_Category_Registry` to check for existing slugs before validating format.
* Modify the `WP_Abilities_Registry` class to return abilities as an associative array keyed by ability name.
* Enhance the `WP_Ability_Category` constructor to throw an exception for empty slugs and streamline property assignment.

* Refactor(Abilities): Change ability category from an array to a single required string

* Refactor(Abilities): Update registry for single category model

* Refactor(Abilities): Update REST API for single category model

* Feat(Abilities): Validate that an ability's category is registered

* Fix: Correct PHPDoc type hints in WP_Abilities_Category_Registry

* Refactor: Rename abilities_category_registry_init hook for consistency

* Refactor: Improve ability filtering logic

* Chore: Add missing newlines at end of files

* test(abilities-api): Add tests for ability categories

* test(abilities-api): Update unit tests to use ability categories

* test(rest-api): Update tests to use ability categories

* test(rest-api): Add tests for category filtering and schema

* docs: Document Ability Categories feature

* docs: Update examples to use Ability Categories

* docs: Document Category support in REST API

* Refactor ability category registration validation

Move the action hook validation for ability category registration from the public API function into the registry class itself. This change centralizes the validation logic, ensuring it's consistently applied.

The validation is also made more specific, now only permitting registration during the `abilities_api_category_registry_init` action to enforce a stricter and more predictable initialization order.

* Refactor tests to register ability categories on the init hook

Update unit tests to register ability categories using the `abilities_api_category_registry_init` action hook.

Previously, tests registered categories after this hook had already fired, which does not reflect the intended API usage. This change ensures that the test setup accurately simulates how categories should be registered, making the test suite more robust and reliable.

A helper method has also been introduced in the `wpAbilityCategory` test class to streamline this process and reduce code duplication.

* Rename category registration hook

The `abilities_api_category_registry_init` action hook is renamed to the more concise and intuitive `abilities_api_categories_init`.

This change improves developer experience by making the hook's purpose clearer and aligning it more closely with standard WordPress naming conventions. All related code, documentation, and tests have been updated to use the new hook name.

* Remove ability filtering by category

Removes the `wp_get_abilities_by_category()` function and its underlying implementation from the abilities registry.

* Improve tests for ability category validation

Enhances the unit test suite for the Abilities API by introducing a helper to capture and assert `_doing_it_wrong()` notices. This allows for more explicit and robust testing of error conditions.

Existing tests are updated to assert that incorrect usage, such as providing an invalid slug or registering a duplicate category, triggers the expected notice.

New test cases are also added to cover:
- Invalid arguments for label and description (non-string, empty).
- The `register_ability_category_args` filter.
- An exception being thrown when attempting to unserialize a category object.

* Fix: Correct formatting and assertions in unit tests

Corrects a malformed comment block and adjusts a method's closing brace.

Updates a test assertion for the REST API to match the expected metadata.

* Changes the version annotations from `0.3.0` to `n.e.x.t`

* Update version annotations from `0.3.0` to `n.e.x.t` in abilities category and registry classes

* Revert version to 0.2.0

* Removes slug sanitization for ability categories

The validation is done on WP_Abilities_Category_Registry::register()

* test: Replace conditional checks with assertions for non-existent categories

Improved test quality by using assertions instead of conditional checks when verifying behavior with non-existent categories. This ensures tests fail loudly if test isolation is broken, rather than silently working around the problem.

Changes:
- test_register_ability_nonexistent_category: Assert category doesn't exist before testing
- test_ability_requires_existing_category: Assert category doesn't exist before testing
- test_filter_by_nonexistent_category: Assert category doesn't exist before testing

This makes test intent clearer and helps catch test isolation issues early.

* Fix: Add missing category to show_in_rest test abilities

Fixes test failures after merging trunk by adding the required `category` field to `test/not-show-in-rest` abilities. These abilities were introduced in PR #103 (show_in_rest feature) on trunk before the category requirement existed.

Also fixes code bugs issues in WP_Ability class:
- Add missing PHPDoc comment opener for show_in_rest property
- Add missing PHPDoc comment opener for show_in_rest() method
- Remove extra blank line in validate_args()

* Added a comment to the __wakeup method to clarify that unserialization is disabled as a security measure against vulnerabilities like object injection and remote code execution.

* Add meta support to ability categories

This change adds metadata support to WP_Ability_Category, following
the same pattern used in WP_Ability class for consistency.

Changes:
- Add optional meta property to WP_Ability_Category class
- Update constructor to use foreach loop for property assignment
- Add validation for meta array in prepare_properties() method
- Add get_meta() getter method to retrieve category metadata
- Update PHPStan type annotations to include meta parameter
- Add comprehensive test coverage for meta functionality

The meta field is optional and defaults to an empty array when not
provided. This allows developers to attach arbitrary metadata to
categories for custom use cases.

* Remove validation for ability category slug format

- The validation is done already when the category is created
- Code style improvements

* Change WP_Ability_Category class to final

* Fix: Correct assertion in REST API ability get_item test

* Refactor: Improve ability category cleanup in tests

* Chore: Fix indentation in WP_Ability class

* Docs: Move category registration to a dedicated file

Extracts the documentation for registering ability categories from the "Registering Abilities" page into a new, dedicated file.

The new category documentation is also expanded to include details on the optional `meta` parameter, the `register_ability_category_args` filter, and provides more comprehensive examples for slug conventions.

* Harden: Prevent serialization of ability and registry classes

Disallow serialization and unserialization for the `WP_Ability_Category` and `WP_Abilities_Category_Registry` classes.

This is a security hardening measure to mitigate potential object injection vulnerabilities that can arise from processing untrusted serialized data. Throwing an exception from the `__sleep()` and `__wakeup()` magic methods ensures these objects are not intended for storage or transport.

* docs: clarify category registration step

Co-authored-by: Greg Ziółkowski <[email protected]>

* Update translator comment and string to specify “Ability category” for clearer context during translation.

Co-authored-by: Greg Ziółkowski <[email protected]>

* Update translator comment and string to specify “Ability category” for clearer context during translation.

Co-authored-by: Greg Ziółkowski <[email protected]>

* Update translator comment and string to specify “Ability category” for clearer context during translation.

Co-authored-by: Greg Ziółkowski <[email protected]>

* Docs: Remove documentation for category registration filter

This parts is coverded  in Hooks documentation

* Fix PHPStan annotations for category registration

Corrects the PHPStan type annotations for wp_register_ability_category()
and WP_Abilities_Category_Registry::register() to accurately reflect the
actual implementation:

- Mark `label` and `description` as required fields (removed optional `?`)
- Add `meta` as an optional property (array<string,mixed>)
- Update docblock to mention `meta` parameter

The label and description fields are validated as required in the
WP_Ability_Category::prepare_properties() method, while meta is
truly optional. The annotations now match the runtime behavior.

* Update tests/unit/abilities-api/wpAbilityCategory.php

Co-authored-by: Greg Ziółkowski <[email protected]>

* Refactor category slug tests to use @dataProvider

Replace loop-based slug validation tests with PHPUnit data providers
for better test isolation and clearer failure reporting.

- Add valid_slug_provider() for valid slug format tests
- Add invalid_slug_provider() for invalid slug format tests

* Refactor test_get_all_categories to use register_category_during_hook

Simplify the test by replacing the callback function with direct calls to register_category_during_hook

* Remove 'annotations' field from abilities schema and update required fields to include 'meta'

* Update required fields in abilities schema to include 'input_schema' and 'output_schema'

* Add category property tests to abilities schema validation

- Assert the count of properties in the schema to ensure it matches expected values.
- Verify the existence and details of the 'category' property, including its type and readonly status.
- Confirm that 'category' is included in the required fields of the schema.
- Remove redundant test method for category schema validation.

---------

Co-authored-by: Greg Ziółkowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants