Sync changes from WordPress core after 6.9 beta 1 #126

gziolo · 2025-10-22T17:15:26Z

Closes #112 – action names changes, and the strategy got unified.
Closes #97 – PHPStan was removed as WP core doesn't support it.

I did an initial pass for syncing back all the changes applied to WordPress Core:

We need to verify the linting errors reported as some of them might be valid. For example:

0c7f621 – this fixes a potential issue reported where the instance could be null.
76f1fad – PHPCS reported wrong order of types, as it should be boolean|null. It's worth updating in WordPress core, too.

There are several linting errors that are expected in WordPress core, so we have to disable them. However, the one about best practices for escaping translations was a bit surprising. It still has to be confirmed, as it isn't enforced in WP core and I simply followed feedback from review when updated esc_html__() to __(). It's mostly for _doing_it_wrong() calls, so maybe that's perfectly fine.

As part of the effort, I had to redo WP_REST_Abilities_Init, which is file-specific only to this repository. I added a check to ensure that the controllers are never re-declared or re-registered if they were previously registered in WordPress core.

codecov · 2025-10-22T17:16:50Z

Codecov Report

❌ Patch coverage is 87.32782% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.57%. Comparing base (9da857e) to head (a0355d9).
⚠️ Report is 1 commits behind head on trunk.

Files with missing lines	Patch %	Lines
includes/abilities-api/class-wp-ability.php	72.72%	15 Missing ⚠️
...ities-api/class-wp-ability-categories-registry.php	77.77%	14 Missing ⚠️
includes/bootstrap.php	0.00%	7 Missing ⚠️
...udes/abilities-api/class-wp-abilities-registry.php	91.17%	3 Missing ⚠️
...cludes/abilities-api/class-wp-ability-category.php	76.92%	3 Missing ⚠️
...ints/class-wp-rest-abilities-v1-run-controller.php	94.00%	3 Missing ⚠️
includes/rest-api/class-wp-rest-abilities-init.php	94.73%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk     #126      +/-   ##
============================================
- Coverage     90.59%   90.57%   -0.03%     
- Complexity      177      188      +11     
============================================
  Files            20       20              
  Lines          1489     1485       -4     
  Branches        117      119       +2     
============================================
- Hits           1349     1345       -4     
  Misses          140      140

Flag	Coverage Δ
javascript	`94.31% <100.00%> (+0.10%)`	⬆️
unit	`89.48% <87.00%> (-0.10%)`	⬇️

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 · 2025-10-22T18:26:35Z

@gziolo on the off chance I save you 30 seconds, here are the FQCN rules for phpcs that need to be removed . Considering the current plan is to archive, I see no point in conditionally disabling/allowlisting.

abilities-api/phpcs.xml.dist

Lines 141 to 154 in 9da857e

    
           <rule ref="SlevomatCodingStandard.Namespaces.FullyQualifiedClassNameInAnnotation"> 
        
           	<properties> 
        
           		<!-- Support PHPStan annotations --> 
        
           		<property name="ignoredAnnotationNames" type="array"> 
        
           			<element value="@phpstan-import-type" /> 
        
           			<element value="@phpstan-param" /> 
        
           			<element value="@phpstan-return" /> 
        
           			<element value="@phpstan-template" /> 
        
           			<element value="@phpstan-type" /> 
        
           			<element value="@phpstan-var" /> 
        
           		</property> 
        
           	</properties> 
        
           </rule> 
        
           <rule ref="SlevomatCodingStandard.Namespaces.FullyQualifiedExceptions" />

JasonTheAdams

Just a few questions for you, @gziolo!

includes/rest-api/class-wp-rest-abilities-init.php

JasonTheAdams · 2025-10-22T20:26:17Z

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

+	public function register( string $slug, array $args ): ?WP_Ability_Category {
+		if ( $this->is_registered( $slug ) ) {
+			_doing_it_wrong(
+				__METHOD__,
+				/* translators: %s: Ability category slug. */
+				sprintf( __( 'Ability category "%s" is already registered.' ), esc_html( $slug ) ),
+				'6.9.0'
+			);
+			return null;
+		}
+
+		if ( ! preg_match( '/^[a-z0-9]+(?:-[a-z0-9]+)*$/', $slug ) ) {
+			_doing_it_wrong(
+				__METHOD__,
+				__( 'Ability category slug must contain only lowercase alphanumeric characters and dashes.' ),
+				'6.9.0'
+			);
+			return null;
+		}


Are we not checking for the category init hook anymore?

It was moved to the wp_register_ability_category() and we switched to did_action( ‘wp_abilities_api_categories_init’ ). It works exactly the same as for abilities now.

I also raised concerns during the merge about the previous implementation. The issue is that wp_register_ability_category() internally calls get_instance(), which triggers the wp_abilities_api_categories_init action before the registry is created and returned on the first run.

This means that if someone tries to register an ability category directly—without using add_action—the code first needs to retrieve the shared registry before calling register and checking whether the action has been processed. As a result, the public method ends up unintentionally satisfying these conditions, creating unexpected behavior.

Ok, I think I'm tracking. Let me say it back to make sure I'm following:

Checking here requires that the registry is instantiated (obviously), meaning get_instance() already ran which triggered the hook, therefore it had always run before this function was called

Moving the check to wp_register_ability_category() means the hook can be checked before the registry is tentatively instantiated

In the "WP Way" this makes sense. It's admittedly why I'm not a fan of hooks being coupled to instantiation in some way, as it makes these sort of weird conditions. The only other way I could think to do this in WP would be to either:

Procedurally place the hooks somewhere in the WP bootstrapping process

Add a hook to init (or somewhere) that instantiates the registries and calls the hooks

Both proposed solutions remove most of the benefits of the current approach because it means all abilities will get registered on every page request when this init fires (no matter where and how it gets initiated). In particular, when rendering pages for visitors, you don't want to register all abilities if they will never be consumed, which I assume is going be the default case in WordPress core. If we don't care about the impact of that registration, then we could simply use init hook as the only mechanism for ensuring the abilities and their categories are registered at the right time.

That's interesting! I didn't realize we were using subsequent hooks as a way of narrowing the registered abilities by context. I don't disagree, it just wasn't a means of "filtering" I'd considered, yet. My assumption so far was that, yes, all abilities would be registered and then subsequent filtering would occur. There are certainly trade-offs of both approaches.

Just to help me wrap my head around the merits, here. Can you give me an example of an ability that would be best registered in a specific hook context?

JasonTheAdams · 2025-10-22T20:33:59Z

includes/abilities-api.php

 */
 function wp_register_ability( string $name, array $args ): ?WP_Ability {
-	if ( ! did_action( 'abilities_api_init' ) ) {
+	if ( ! did_action( 'wp_abilities_api_init' ) ) {


I'm curious why we're using did_action() and not doing_action? Is an action considered "done" immediately after do_action() is triggered? Or after the last hook function is fired?

Yes, https://developer.wordpress.org/reference/functions/do_action/ does two things, increments the counter for how many times the action was fired, and sets the current action name. Then it runs all added actions. did_action ensures that registration doesn’t happen too early and only when we know the registry is necessary for a given HTTP request, but the point was raised we should not prevent registering more abilities and categories later in the flow, by using doing_action.

Ahhh, thanks for clearing that up. I really should have looked up the code myself. Good to know how that works!

Personally, I'm not convinced on the merits of allowing folks to register at any point after this time, as those are the sort of moments in my WP dev history that are maddening – "wait, why isn't that available? Oh, they hook into that hook later with a priority of 500? Why!?" Not being able to assume that all abilities and categories have been initialized during... initialization is weird. Feels like an established WP quirk that we're perpetuating. 😆

I did provide similar feedback somewhere else (I believe on the Core PR) - I think allowing registration at any arbitrary point means we're being "forgiving" to developers doing it wrong, which can then lead to far more obscure problems down the line.

I think clearly defined APIs and, given WordPress's procedural nature, clearly defined registration timing are crucial to avoid more complex problems. Better tell the developer they should fix something rather than them running into a problem later that they have no idea on why it's occurring.

I’m happy to apply further updates. I was a bit on the clock when landing the entire API in WP core. So while I agreed with reasoning, I missed this important nuance at the time.

It’s a tiny change for production logic, but needs some solid tinkering for unit tests so they are in proper state while registering abilities and ability categories. I’ll try to sort out something tomorrow, so it’s ready before beta 3.

gziolo · 2025-10-23T08:10:12Z

PHPCS and PHPStan are now good after not-matching rules were removed or ignored... One unit test to fix, and then we need to update the client to use the new REST API endpoint paths. Potentially, we also need to adjust the types to account that meta.annotations has slightly changed.

github-actions · 2025-10-23T17:24:15Z

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 <gziolo@git.wordpress.org>
Co-authored-by: JasonTheAdams <jason_the_adams@git.wordpress.org>
Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: justlevine <justlevine@git.wordpress.org>
Co-authored-by: galatanovidiu <ovidiu-galatan@git.wordpress.org>

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

gziolo · 2025-10-23T17:27:57Z

It should be mostly ready to review.

The only remaining task is to mirror the HTTP method selection when executing the ability, and cover that with unit tests

Client:

abilities-api/packages/client/src/api.ts

Line 276 in 9da857e

const method = !! ability.meta?.annotations?.readonly ? 'GET' : 'POST';

Server:

abilities-api/includes/rest-api/endpoints/class-wp-rest-abilities-v1-run-controller.php

Lines 111 to 116 in 0a511e0

    
           $expected_method = 'POST'; 
        
           if ( ! empty( $annotations['readonly'] ) ) { 
        
           	$expected_method = 'GET'; 
        
           } elseif ( ! empty( $annotations['destructive'] ) && ! empty( $annotations['idempotent'] ) ) { 
        
           	$expected_method = 'DELETE'; 
        
           }

gziolo · 2025-10-23T18:43:44Z

It tests correctly when using the client-side API with core abilities:

gziolo · 2025-10-23T18:58:28Z

It turned out to be a simple task to add support for DELETE HTTP method also on the client with b70eec3.

Discovered when syncing code back to Abilities API repository in WordPress/abilities-api#126. Follow-up to [61032]. See #64098. git-svn-id: https://develop.svn.wordpress.org/trunk@61059 602fd350-edb4-49c9-b593-d223f7449a82

Discovered when syncing code back to Abilities API repository in WordPress/abilities-api#126. Follow-up to [61032]. See #64098. Built from https://develop.svn.wordpress.org/trunk@61059 git-svn-id: http://core.svn.wordpress.org/trunk@60395 1a063a9b-81f0-0310-95a4-ce76da25c4cd

Discovered when syncing code back to Abilities API repository in WordPress/abilities-api#126. Follow-up to [61032]. See #64098. Built from https://develop.svn.wordpress.org/trunk@61059 git-svn-id: https://core.svn.wordpress.org/trunk@60395 1a063a9b-81f0-0310-95a4-ce76da25c4cd

jorgefilipecosta

I left multiple comments, but all of them seem minor, and this is a sync so we may need to reflect what is in core and then update things on both places.
One thing I think we should consolidate is the since tags, on if we should use core version or plugin version.

Things worked well on the smoke tests I the code seems good 👍

includes/abilities-api/class-wp-ability-category.php

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

includes/abilities-api.php

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

jorgefilipecosta · 2025-10-27T09:37:25Z

includes/abilities-api.php

+ *     }
 *
- * @see WP_Abilities_Category_Registry::get_all_registered()
+ * @since 6.9.0


On since tags we are using 6.9.0, everywhere, should we be using the plugin version and not the core version?

I would keep files the same as in WP core, as we it's a direct copy. In the future, WP core becomes the source of truth and we would only continue keeping this repo up to date so folks can polyfill Abilities API for WP 6.8 and older if they want to start using it but their support policy includes a range of WP major versions. It's been the case for Woo that is actively testing usage of this API.

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

gziolo · 2025-10-27T10:14:19Z

@jorgefilipecosta, good catch with a few inconsistencies in WP core. I will update them here before landing, and then patch in WP core.

gziolo · 2025-10-27T11:30:13Z

I updated a patch against WP core to address the feedback first:

Abilities API: Code quality fixes from syncing to the upstream project wordpress-develop#10424

This aligns with how translations are handled across all places in the Abilities API codebase. It addresses the feedback raised during syncing back changes to Abilities API repository with WordPress/abilities-api#126. Developed in #10424. Follow-up [61032]. Props gziolo, jorgefilipecosta. See #64098. git-svn-id: https://develop.svn.wordpress.org/trunk@61071 602fd350-edb4-49c9-b593-d223f7449a82

This aligns with how translations are handled across all places in the Abilities API codebase. It addresses the feedback raised during syncing back changes to Abilities API repository with WordPress/abilities-api#126. Developed in WordPress/wordpress-develop#10424. Follow-up [61032]. Props gziolo, jorgefilipecosta. See #64098. Built from https://develop.svn.wordpress.org/trunk@61071 git-svn-id: http://core.svn.wordpress.org/trunk@60407 1a063a9b-81f0-0310-95a4-ce76da25c4cd

This aligns with how translations are handled across all places in the Abilities API codebase. It addresses the feedback raised during syncing back changes to Abilities API repository with WordPress/abilities-api#126. Developed in WordPress/wordpress-develop#10424. Follow-up [61032]. Props gziolo, jorgefilipecosta. See #64098. Built from https://develop.svn.wordpress.org/trunk@61071 git-svn-id: https://core.svn.wordpress.org/trunk@60407 1a063a9b-81f0-0310-95a4-ce76da25c4cd

Sync changes from WordPress core after 6.9 beta 1

6b88d37

gziolo requested a review from emdashcodes October 22, 2025 17:15

Use wp_has_ability_category helper function

0c7f621

gziolo requested review from JasonTheAdams and justlevine October 22, 2025 17:33

gziolo self-assigned this Oct 22, 2025

gziolo added [Priority] High Essential focuses for work [Type] Task Issues or PRs that have been broken down into an individual action to take labels Oct 22, 2025

JasonTheAdams reviewed Oct 22, 2025

View reviewed changes

gziolo added 2 commits October 23, 2025 09:10

Disable or fix issues reported by PHPCS

76f1fad

Ignore missingType.generics error in PHPStan

c03bbc7

gziolo mentioned this pull request Oct 23, 2025

Fix Abilities API tests for WordPress 6.9+ woocommerce/woocommerce#61560

Merged

gziolo added 3 commits October 23, 2025 11:47

Try to fix the failing unit test

550d0ef

Improve the REST API routes registration

5392975

Update client to use new REST API urls

0a511e0

gziolo force-pushed the update/sync-wordpress-core-trunk branch from ab066cc to 0a511e0 Compare October 23, 2025 17:23

gziolo marked this pull request as ready for review October 23, 2025 17:24

Add handling for destructive idempotent abilities to the client

b70eec3

gziolo requested a review from jorgefilipecosta October 23, 2025 18:53

This was referenced Oct 23, 2025

docs: fix wp_register_ability() args not marked as required. #97

Closed

Align doing_action() and did_action() usage for category and ability registration in the Abilities API #112

Closed

gziolo added this to WordPress Abilities API: planning Oct 24, 2025

gziolo added this to the WP 6.9 milestone Oct 24, 2025

gziolo moved this to Needs review in WordPress Abilities API: planning Oct 24, 2025

This was referenced Oct 24, 2025

docs: Improve permission checking documentation and examples #95

Open

Update: Simplify core abilties registration in preparation for core patch. #128

Merged

Bring the latest documentation updates added in WP Core

aa88f6a

jorgefilipecosta approved these changes Oct 27, 2025

View reviewed changes

gziolo mentioned this pull request Oct 27, 2025

Abilities API: Code quality fixes from syncing to the upstream project WordPress/wordpress-develop#10424

Closed

Address feedback from review

3f31cb5

Fix reported error by PHPCS

a0355d9

gziolo enabled auto-merge (squash) October 27, 2025 12:02

gziolo merged commit 4513526 into trunk Oct 27, 2025
18 of 20 checks passed

gziolo deleted the update/sync-wordpress-core-trunk branch October 27, 2025 12:02

github-project-automation bot moved this from Needs review to Done in WordPress Abilities API: planning Oct 27, 2025

Sync changes from WordPress core after 6.9 beta 1 #126

Sync changes from WordPress core after 6.9 beta 1 #126

Uh oh!

Conversation

gziolo commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

justlevine commented Oct 22, 2025

Uh oh!

JasonTheAdams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gziolo commented Oct 23, 2025

Uh oh!

github-actions bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gziolo commented Oct 23, 2025

Uh oh!

gziolo commented Oct 23, 2025

Uh oh!

gziolo commented Oct 23, 2025

Uh oh!

jorgefilipecosta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gziolo commented Oct 27, 2025

Uh oh!

gziolo commented Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gziolo commented Oct 22, 2025 •

edited

Loading

codecov bot commented Oct 22, 2025 •

edited

Loading

github-actions bot commented Oct 23, 2025 •

edited

Loading