Fix account move verification and notify followers

[pfefferle]

Proposed changes:

Reviewed the outgoing Move handling (Move::externally() / Move::internally()) against FEP-7628 and fixed four things:

• Verify before advertising. externally() set movedTo before fetching and checking the target, so a failed or unverifiable move left the actor pointing at a destination with no Move federated. movedTo is now written only after the target is verified.
• Verify the id we actually send. The link check now tests the actor id sent as the Move's object (get_id()) against the target's alsoKnownAs, instead of the possibly non-canonical input URL, which is what receiving servers verify against.
• alsoKnownAs on the target. internally() recorded the old URL on the source actor. It now records it on the target, so the new actor links back to the old one and receiving servers accept the move. For a domain change the two resolve to the same actor, so that path is unchanged; for a move between two different local actors the target now links back.
• Notify followers (FEP-7628). After a move, the actor's profile is federated as an Update so followers refresh the cached movedTo/alsoKnownAs. It is queued after the Move so a follower that reacts to movedTo still processes the migration first.

Delivery is unchanged and already matches FEP-7628: the Move is addressed to the old actor's followers.

Other information:

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

Testing instructions:

• wp activitypub move <from> <to> (WP-CLI) or trigger a move via the settings.
• An external move to a target whose alsoKnownAs does not list your actor is rejected and leaves movedTo unset.
• A verified move sets movedTo, federates the Move to followers, and federates a profile Update.
• A domain change still migrates every actor as before.

Changelog entry

The changelog entry is already included in the branch (.github/changelog/fix-account-move-verification), so the auto-create box below is left unchecked.

Changelog Entry Details

Significance

• Patch

Type

• Fixed - for any bug fixes

Message

Fix Fediverse account migration so a move is verified before it takes effect and reliably reaches your followers on other servers.

[Copilot]

Pull request overview

Improves outgoing ActivityPub account migration (Move) handling to better align with FEP-7628 by verifying targets before persisting movedTo, ensuring link-back validation uses the canonical actor id, and notifying followers via a profile Update after a move.

Changes:

• Adjust Move::externally() to validate the target’s alsoKnownAs against the canonical actor id and only persist movedTo after verification.
• Adjust Move::internally() to record the source URL on the target actor’s alsoKnownAs (so the new actor links back).
• Add/extend PHPUnit coverage for verified moves, rejected targets, internal moves between distinct local users, and profile-update federation; add changelog entry.

Reviewed changes

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

File	Description
includes/class-move.php	Updates external/internal move validation and adds follower-notification via profile Update scheduling.
tests/phpunit/tests/includes/class-test-move.php	Expands tests for verification behavior, follower notification, and internal moves across distinct users.
.github/changelog/fix-account-move-verification	Adds a patch-level changelog entry describing the migration fix.

Comments suppressed due to low confidence (4)

includes/class-move.php:193

• schedule_profile_update() runs even when add_to_outbox() fails, which can result in profile Updates being federated without a corresponding Move activity.

		$outbox_id = add_to_outbox( $activity, null, $user->get__id(), ACTIVITYPUB_CONTENT_VISIBILITY_QUIET_PUBLIC );

		/*
		 * Notify followers of the changed profile on both actors by federating an Update (FEP-7628).
		 * Queued after the Move so a follower that reacts to `movedTo` still processes the migration first.

tests/phpunit/tests/includes/class-test-move.php:109

• This pre_http_request filter callback is registered with the default accepted-args (1), but the closure declares 0 parameters. That produces “too many arguments” warnings on PHP 7.x and can become an ArgumentCountError on PHP 8+.

		$filter = function () use ( $from ) {

tests/phpunit/tests/includes/class-test-move.php:149

• This pre_http_request filter callback is registered with the default accepted-args (1), but the closure declares 0 parameters. That produces “too many arguments” warnings on PHP 7.x and can become an ArgumentCountError on PHP 8+.

		$filter = function () {

tests/phpunit/tests/includes/class-test-move.php:205

• This pre_http_request filter callback is registered with the default accepted-args (1), but the closure declares 0 parameters. That produces “too many arguments” warnings on PHP 7.x and can become an ArgumentCountError on PHP 8+.

		$filter = function () use ( $from ) {

[Copilot]

Pull request overview

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

Suppressed comments (6)

tests/phpunit/tests/includes/class-test-move.php:113

• The mocked target actor JSON returned via pre_http_request is missing required actor fields like id (and typically type). That means Move::externally() builds a target actor without an id, so the federated Move ends up with an empty target, and the test can still pass. Include at least id (and type) in the mock to better match real actor objects and exercise the intended code path.

		$filter = function () use ( $from ) {
			return array(
				'body'     => wp_json_encode( array( 'alsoKnownAs' => array( $from ) ) ),
				'response' => array( 'code' => 200 ),
			);

tests/phpunit/tests/includes/class-test-move.php:153

• The mocked target actor JSON returned via pre_http_request is missing required actor fields like id (and typically type). That means Move::externally() builds a target actor without an id, so the federated Move ends up with an empty target, and the test can still pass. Include at least id (and type) in the mock to better match real actor objects and exercise the intended code path.

		$http = function () use ( $from ) {
			return array(
				'body'     => wp_json_encode( array( 'alsoKnownAs' => array( $from ) ) ),
				'response' => array( 'code' => 200 ),
			);

tests/phpunit/tests/includes/class-test-move.php:204

• This pre_http_request mock returns an object without an id (and type). Move::externally() uses the decoded actor’s id as the Move activity target; omitting it means the test doesn’t exercise the real-world path where the target has an id. Include at least id/type in the mocked JSON response.

		$filter = function () {
			return array(
				'body'     => wp_json_encode( array( 'alsoKnownAs' => array( 'https://newsite.com/user/999' ) ) ),
				'response' => array( 'code' => 200 ),
			);

tests/phpunit/tests/includes/class-test-move.php:260

• The mocked target actor JSON returned via pre_http_request is missing required actor fields like id (and typically type). That means Move::externally() builds a target actor without an id, so the federated Move ends up with an empty target, and the test can still pass. Include at least id (and type) in the mock to better match real actor objects and exercise the intended code path.

		$filter = function () use ( $from ) {
			return array(
				'body'     => wp_json_encode( array( 'alsoKnownAs' => array( $from ) ) ),
				'response' => array( 'code' => 200 ),
			);

includes/class-move.php:109

• externally() persists movedTo using the raw $to input even though the target actor is fetched and the canonical id is available. If $to is a non-canonical URL (or redirects), followers may cache a movedTo that doesn’t match the actor id used in the federated Move (set_target( $target_actor->get_id() )). Persist the verified/canonical target id instead to match what is federated.

		// Advertise the move only after the target is verified, so a failed attempt never leaves the actor pointing at an unverified target.
		if ( $user->get__id() > 0 ) {
			\update_user_option( $user->get__id(), 'activitypub_moved_to', $to );
		} else {
			\update_option( 'activitypub_blog_user_moved_to', $to );

tests/phpunit/tests/includes/class-test-move.php:48

• The mocked target actor JSON returned via pre_http_request is missing required actor fields like id (and typically type). That means Move::externally() builds a target actor without an id, so the federated Move ends up with an empty target, and the test can still pass. Include at least id (and type) in the mock to better match real actor objects and exercise the intended code path.

This issue also appears in the following locations of the same file:

• line 109
• line 149
• line 200
• line 256

		$filter = function () use ( $from ) {
			return array(
				'body'     => wp_json_encode( array( 'alsoKnownAs' => array( $from ) ) ),
				'response' => array( 'code' => 200 ),
			);

[pfefferle]

Copilot suppressed six comments in the last review. One of them was right, so I fixed it.

externally() stored movedTo from the raw $to input, but federated set_target( $target_actor->get_id() ). If the input url is an alias, or redirects, the two are not the same. That matters, because some servers compare exactly these two values:

• Pixelfed refetches the old actor and requires its movedTo to equal the Move's target (ProcessMovePipeline.php:126-150), else it aborts the whole move chain.
• Misskey resolves movedTo and then requires src.movedToUri === dst.uri (ApPersonService.ts:781), else it returns skip: missmatch uri.

So the move was just silently ignored there. movedTo now stores the canonical id, the same value we send as target. The bug is older than this PR, trunk has it too, so I gave it its own changelog entry.

I also had to add a guard for a target document without an id. Without it the canonical id is empty, so we would advertise an empty movedTo and send a Move with no target at all. The other five suppressed comments were all about the test mocks missing id and type, which is exactly why nobody noticed. The mocks have them now.

Two new tests, both verified by breaking the fix again:

• test_account_stores_canonical_target_id, the target's id differs from the url the move was requested with
• test_account_rejects_target_without_id

Without the guard the second one fails with "Failed asserting that 10 is an instance of WP_Error", so the move really did succeed with an empty target before.

Full suite green, 2915 tests.

[Copilot]

Pull request overview

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

Suppressed comments (1)

tests/phpunit/tests/includes/class-test-move.php:409

• The new follower-notification behavior for internal moves (scheduling profile Update activities for both source and target actors) isn’t asserted anywhere. Adding an assertion here would prevent regressions (e.g., only one actor being updated, or no Update being federated at all).

		// The source points at the target.
		$this->assertEquals( $to, Actors::get_by_id( self::$user_id )->get_moved_to() );

		// The target links back to the source via alsoKnownAs, so receiving servers accept the move.
		$this->assertContains( $from, Actors::get_by_id( $target_id )->get_also_known_as() );