From 0849e44de97311c8c71e971619524b84720eaf9c Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 12:47:01 +0200 Subject: [PATCH 01/17] Expose site score and recommendations as WordPress abilities Registers two read-only abilities with the core Abilities API so an AI agent can read what this plugin knows: - progress-planner/get-site-score - progress-planner/list-recommendations Registration is plain wp_register_ability(), guarded by function_exists(). The plugin stays unaware of any MCP bridge: a bridge that discovers WordPress abilities picks these up on its own, so no per-bridge integration is needed on either side. Nothing downstream re-checks a third-party ability's risk classification, so the permission callbacks here are the only gate that runs. Both require edit_others_posts, matching the capability the admin pages are gated on, and list-recommendations additionally drops any task whose provider is unavailable to the current user. The score payload is deliberately narrower than the SaaS status endpoint: the active-plugin inventory, site URL and branding ID that endpoint reports are telemetry for progressplanner.com, not something an agent needs. The monthly-score history is shared with System_Status rather than copied, so the two cannot drift. Task evaluation runs on admin_init, so these abilities report stored state rather than triggering it -- an agent request is not an admin request, and evaluating here would mean a read silently writes. Registration is idempotent: the constructor hooks the init actions, so a second instance would otherwise re-register and trip core's incorrect-usage notice. is_registered() is the notice-free probe for that. PHPStan cannot see the Abilities API functions because the pinned WordPress stubs predate them; ignored the same way the optional Yoast symbols are. Co-Authored-By: Claude Opus 5 (1M context) --- classes/abilities/class-abilities.php | 208 ++++++++++ classes/abilities/class-recommendations.php | 113 +++++ classes/abilities/class-schemas.php | 194 +++++++++ classes/abilities/class-site-score.php | 123 ++++++ classes/class-base.php | 5 + classes/utils/class-system-status.php | 52 ++- phpstan.neon.dist | 3 + tests/phpunit/test-class-abilities.php | 431 ++++++++++++++++++++ 8 files changed, 1109 insertions(+), 20 deletions(-) create mode 100644 classes/abilities/class-abilities.php create mode 100644 classes/abilities/class-recommendations.php create mode 100644 classes/abilities/class-schemas.php create mode 100644 classes/abilities/class-site-score.php create mode 100644 tests/phpunit/test-class-abilities.php diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php new file mode 100644 index 000000000..0c2521ba9 --- /dev/null +++ b/classes/abilities/class-abilities.php @@ -0,0 +1,208 @@ +site_score = new Site_Score(); + $this->recommendations = new Recommendations(); + + // Categories register on an earlier hook than abilities: core rejects an + // ability naming a category that does not exist yet. + \add_action( 'wp_abilities_api_categories_init', [ $this, 'register_categories' ] ); + \add_action( 'wp_abilities_api_init', [ $this, 'register_abilities' ] ); + } + + /** + * Register the ability category. + * + * @return void + */ + public function register_categories() { + if ( ! \function_exists( 'wp_register_ability_category' ) ) { + return; + } + + // Registering twice is incorrect usage. Guarding here rather than relying + // on the hook firing once keeps a second instance of this class harmless. + // is_registered() is the notice-free probe: wp_get_ability_category() + // reports a miss as incorrect usage in its own right. + if ( $this->is_category_registered() ) { + return; + } + + \wp_register_ability_category( + self::CATEGORY, + [ + 'label' => \__( 'Progress Planner', 'progress-planner' ), + 'description' => \__( 'Website maintenance progress, scores and recommendations provided by Progress Planner.', 'progress-planner' ), + ] + ); + } + + /** + * Register the abilities. + * + * @return void + */ + public function register_abilities() { + if ( ! \function_exists( 'wp_register_ability' ) ) { + return; + } + + // As above: re-registering is incorrect usage, so the registry is asked + // first rather than assuming this runs exactly once. + if ( $this->is_ability_registered( self::CATEGORY . '/get-site-score' ) ) { + return; + } + + \wp_register_ability( + self::CATEGORY . '/get-site-score', + $this->ability_args( + [ + 'label' => \__( 'Get site score', 'progress-planner' ), + 'description' => \__( 'Get the Progress Planner activity score for this site, the weekly maintenance checklist, earned badges and the six-month score history.', 'progress-planner' ), + 'input_schema' => Schemas::no_input(), + 'output_schema' => Schemas::site_score(), + 'execute_callback' => [ $this->site_score, 'get' ], + ] + ) + ); + + \wp_register_ability( + self::CATEGORY . '/list-recommendations', + $this->ability_args( + [ + 'label' => \__( 'List recommendations', 'progress-planner' ), + 'description' => \__( 'List the website maintenance tasks Progress Planner currently recommends, newest first. Returns only tasks the current user is allowed to act on.', 'progress-planner' ), + 'input_schema' => Schemas::list_recommendations_input(), + 'output_schema' => Schemas::recommendations(), + 'execute_callback' => [ $this->recommendations, 'list' ], + ] + ) + ); + } + + /** + * Build the registration arguments for one ability. + * + * The shared parts are defined once so an ability cannot drift from the + * category, the permission model or the annotations the others declare. + * + * @param array $args The ability-specific arguments. + * + * @return array + */ + private function ability_args( array $args ) { + return \array_merge( + [ + 'category' => self::CATEGORY, + 'permission_callback' => [ $this, 'can_read' ], + 'meta' => [ + 'show_in_rest' => true, + 'annotations' => [ + 'readonly' => true, + 'destructive' => false, + 'idempotent' => true, + ], + ], + ], + $args + ); + } + + /** + * Whether the current user may read Progress Planner data. + * + * @return bool + */ + public function can_read() { + return \current_user_can( self::READ_CAPABILITY ); + } + + /** + * Whether the ability category is already registered. + * + * @return bool + */ + private function is_category_registered() { + if ( ! \class_exists( '\WP_Ability_Categories_Registry' ) ) { + return false; + } + + $registry = \WP_Ability_Categories_Registry::get_instance(); + + return $registry && $registry->is_registered( self::CATEGORY ); + } + + /** + * Whether an ability is already registered. + * + * @param string $name The fully-qualified ability name. + * + * @return bool + */ + private function is_ability_registered( $name ) { + if ( ! \class_exists( '\WP_Abilities_Registry' ) ) { + return false; + } + + $registry = \WP_Abilities_Registry::get_instance(); + + return $registry && $registry->is_registered( $name ); + } +} diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php new file mode 100644 index 000000000..53aa560b0 --- /dev/null +++ b/classes/abilities/class-recommendations.php @@ -0,0 +1,113 @@ + $input The ability input. + * + * @return array + */ + public function list( $input = [] ) { + $query_args = [ + 'post_status' => $this->get_post_status_for( isset( $input['status'] ) ? (string) $input['status'] : 'pending' ), + 'posts_per_page' => isset( $input['limit'] ) ? (int) $input['limit'] : 20, + ]; + + if ( ! empty( $input['provider'] ) ) { + $query_args['provider'] = (string) $input['provider']; + } + + $recommendations = []; + foreach ( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( $query_args ) as $task ) { + $prepared = $this->prepare( $task ); + + if ( null !== $prepared ) { + $recommendations[] = $prepared; + } + } + + return [ + 'recommendations' => $recommendations, + 'count' => \count( $recommendations ), + ]; + } + + /** + * Get a task provider. + * + * @param string $provider_id The provider ID. + * + * @return \Progress_Planner\Suggested_Tasks\Tasks_Interface|null + */ + private function get_provider( $provider_id ) { + return \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); + } + + /** + * Prepare one recommendation for output. + * + * Returns null when the provider that owns the task is not available to the + * current user, so the list never advertises work they cannot do. + * + * @param \Progress_Planner\Suggested_Tasks\Task $task The task. + * + * @return array|null + */ + private function prepare( $task ) { + $provider_id = $task->get_provider_id(); + $provider = $this->get_provider( $provider_id ); + + // A task whose provider is gone (a deactivated integration) has no + // capability to check and no action to offer, so it is omitted. + if ( ! $provider || ! $provider->capability_required() ) { + return null; + } + + return [ + 'id' => (string) \progress_planner()->get_suggested_tasks()->get_task_id_from_slug( $task->post_name ), + 'title' => (string) $task->post_title, + 'description' => (string) $task->description, + 'provider_id' => (string) $provider_id, + 'url' => (string) $task->url, + // User-created tasks carry no points of their own; the provider is + // the reliable source, with the stored value preferred when set. + 'points' => (int) ( $task->points ?? $provider->get_points() ), + ]; + } +} diff --git a/classes/abilities/class-schemas.php b/classes/abilities/class-schemas.php new file mode 100644 index 000000000..de3c90974 --- /dev/null +++ b/classes/abilities/class-schemas.php @@ -0,0 +1,194 @@ + + */ + public static function no_input() { + return [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => [], + ]; + } + + /** + * The input schema for list-recommendations. + * + * @return array + */ + public static function list_recommendations_input() { + return [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => [ + 'status' => [ + 'type' => 'string', + 'description' => \__( 'Which recommendations to return. "pending" is the default and means tasks still to be done; "completed" returns recently completed tasks; "snoozed" returns tasks postponed to a later date.', 'progress-planner' ), + 'enum' => [ 'pending', 'completed', 'snoozed' ], + 'default' => 'pending', + ], + 'provider' => [ + 'type' => 'string', + 'description' => \__( 'Return only recommendations from this provider ID, for example "core-blogdescription".', 'progress-planner' ), + ], + 'limit' => [ + 'type' => 'integer', + 'description' => \__( 'Maximum number of recommendations to return. Defaults to 20.', 'progress-planner' ), + 'minimum' => 1, + 'maximum' => 100, + 'default' => 20, + ], + ], + ]; + } + + + /** + * The output schema for get-site-score. + * + * @return array + */ + public static function site_score() { + return [ + 'type' => 'object', + 'properties' => [ + 'score' => [ + 'type' => 'integer', + 'description' => \__( 'The website activity score, from 0 to 100.', 'progress-planner' ), + ], + 'checklist' => [ + 'type' => 'object', + 'description' => \__( 'The weekly maintenance checklist.', 'progress-planner' ), + 'properties' => [ + 'published_content' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether content was published in the last seven days.', 'progress-planner' ), + ], + 'updated_content' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether content was updated in the last seven days.', 'progress-planner' ), + ], + 'no_pending_updates' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether the site has no pending updates.', 'progress-planner' ), + ], + ], + ], + 'pending_updates' => [ + 'type' => 'integer', + 'description' => \__( 'The number of pending core, plugin and theme updates.', 'progress-planner' ), + ], + 'badges' => [ + 'type' => 'array', + 'description' => \__( 'The content and maintenance badges, with progress towards each.', 'progress-planner' ), + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'id' => [ 'type' => 'string' ], + 'name' => [ 'type' => 'string' ], + 'progress' => [ + 'type' => 'integer', + 'description' => \__( 'Progress towards the badge, as a percentage.', 'progress-planner' ), + ], + 'complete' => [ 'type' => 'boolean' ], + ], + ], + ], + 'latest_badge' => [ + 'type' => [ 'object', 'null' ], + 'description' => \__( 'The most recently completed badge, or null if none has been completed.', 'progress-planner' ), + 'properties' => [ + 'id' => [ 'type' => 'string' ], + 'name' => [ 'type' => 'string' ], + ], + ], + 'monthly_scores' => [ + 'type' => 'array', + 'description' => \__( 'The score for each of the last six months, oldest first.', 'progress-planner' ), + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'month' => [ 'type' => 'string' ], + 'score' => [ 'type' => 'integer' ], + ], + ], + ], + ], + ]; + } + + /** + * The output schema for list-recommendations. + * + * @return array + */ + public static function recommendations() { + return [ + 'type' => 'object', + 'properties' => [ + 'recommendations' => [ + 'type' => 'array', + 'items' => self::recommendation(), + ], + 'count' => [ + 'type' => 'integer', + 'description' => \__( 'The number of recommendations returned.', 'progress-planner' ), + ], + ], + ]; + } + + /** + * The shape of a single recommendation. + * + * @return array + */ + public static function recommendation() { + return [ + 'type' => 'object', + 'properties' => [ + 'id' => [ + 'type' => 'string', + 'description' => \__( 'The task ID.', 'progress-planner' ), + ], + 'title' => [ + 'type' => 'string', + 'description' => \__( 'What the task asks the user to do.', 'progress-planner' ), + ], + 'description' => [ + 'type' => 'string', + 'description' => \__( 'Why the task matters.', 'progress-planner' ), + ], + 'provider_id' => [ + 'type' => 'string', + 'description' => \__( 'The ID of the provider that raised this task.', 'progress-planner' ), + ], + 'url' => [ + 'type' => 'string', + 'description' => \__( 'The admin URL where the task can be completed.', 'progress-planner' ), + ], + 'points' => [ + 'type' => 'integer', + 'description' => \__( 'Points awarded for completing the task.', 'progress-planner' ), + ], + ], + ]; + } +} diff --git a/classes/abilities/class-site-score.php b/classes/abilities/class-site-score.php new file mode 100644 index 000000000..b40b05b6c --- /dev/null +++ b/classes/abilities/class-site-score.php @@ -0,0 +1,123 @@ + + */ + public function get() { + return [ + 'score' => (int) \progress_planner()->get_admin__widgets__activity_scores()->get_score(), + 'checklist' => $this->get_checklist(), + 'pending_updates' => (int) \wp_get_update_data()['counts']['total'], + 'badges' => $this->get_badges(), + 'latest_badge' => $this->get_latest_badge(), + 'monthly_scores' => $this->get_monthly_scores(), + ]; + } + + /** + * Get the weekly checklist as named booleans. + * + * The widget's own results are keyed by translated label, which is not a + * stable schema key. The items themselves are in a fixed order, so the + * callbacks are invoked positionally and given names that are not. + * + * @return array + */ + public function get_checklist() { + $items = \progress_planner()->get_admin__widgets__activity_scores()->get_checklist(); + $keys = [ 'published_content', 'updated_content', 'no_pending_updates' ]; + + $checklist = []; + foreach ( $keys as $index => $key ) { + $checklist[ $key ] = isset( $items[ $index ]['callback'] ) && \is_callable( $items[ $index ]['callback'] ) + ? (bool) $items[ $index ]['callback']() + : false; + } + + return $checklist; + } + + /** + * Get the badges and their progress. + * + * @return array> + */ + public function get_badges() { + $badges = \array_merge( + \progress_planner()->get_badges()->get_badges( 'content' ), + \progress_planner()->get_badges()->get_badges( 'maintenance' ) + ); + + $output = []; + foreach ( $badges as $badge ) { + $progress = $badge->get_progress(); + $percent = (int) ( $progress['progress'] ?? 0 ); + + $output[] = [ + 'id' => (string) $badge->get_id(), + 'name' => (string) $badge->get_name(), + 'progress' => $percent, + 'complete' => 100 === $percent, + ]; + } + + return $output; + } + + /** + * Get the most recently completed badge. + * + * @return array|null + */ + public function get_latest_badge() { + $badge = \progress_planner()->get_badges()->get_latest_completed_badge(); + + if ( ! $badge ) { + return null; + } + + return [ + 'id' => (string) $badge->get_id(), + 'name' => (string) $badge->get_name(), + ]; + } + + /** + * Get the normalized monthly score history. + * + * @return array> + */ + public function get_monthly_scores() { + $output = []; + + $system_status = new \Progress_Planner\Utils\System_Status(); + + foreach ( $system_status->get_monthly_scores() as $item ) { + $output[] = [ + 'month' => (string) $item['label'], + 'score' => (int) \round( (float) $item['score'] ), + ]; + } + + return $output; + } +} diff --git a/classes/class-base.php b/classes/class-base.php index 0f55de1c7..0b6f31d12 100644 --- a/classes/class-base.php +++ b/classes/class-base.php @@ -55,6 +55,7 @@ * @method \Progress_Planner\Admin\Widgets\Challenge get_admin__widgets__challenge() * @method \Progress_Planner\Admin\Widgets\Activity_Scores get_admin__widgets__activity_scores() * @method \Progress_Planner\Utils\Date get_utils__date() + * @method \Progress_Planner\Abilities\Abilities get_abilities__abilities() */ class Base { @@ -130,6 +131,10 @@ public function init() { $this->get_rest__stats(); $this->get_rest__tasks(); + // Abilities API. Registered on every request, not only in wp-admin: an + // agent reaching the site over REST is not an admin request. + $this->get_abilities__abilities(); + // Onboarding. $this->get_utils__onboard(); diff --git a/classes/utils/class-system-status.php b/classes/utils/class-system-status.php index a7d719296..3834e8d5a 100644 --- a/classes/utils/class-system-status.php +++ b/classes/utils/class-system-status.php @@ -15,6 +15,37 @@ */ class System_Status { + /** + * Get the normalized score for each of the last six months. + * + * Shared with the Abilities layer so both report the same history from the + * same definition of a monthly score. + * + * @return array> Chart items with 'label' and 'score'. + */ + public function get_monthly_scores() { + return \progress_planner()->get_ui__chart()->get_chart_data( + [ + 'items_callback' => fn( $start_date, $end_date ) => \progress_planner()->get_activities__query()->query_activities( + [ + 'start_date' => $start_date, + 'end_date' => $end_date, + ] + ), + 'dates_params' => [ + 'start_date' => \DateTime::createFromFormat( 'Y-m-d', \gmdate( 'Y-m-01' ) )->modify( '-6 months' ), + 'end_date' => new \DateTime(), + 'frequency' => 'monthly', + 'format' => 'M', + ], + 'count_callback' => fn( $activities, $date ) => + \array_sum( \array_map( fn( $activity ) => $activity->get_points( $date ), $activities ) ) * 100 / Base::SCORE_TARGET, + 'normalized' => true, + 'max' => 100, + ] + ); + } + /** * Get the system status. * @@ -70,26 +101,7 @@ public function get_system_status() { $data['latest_badge'] = \progress_planner()->get_badges()->get_latest_completed_badge(); - $scores = \progress_planner()->get_ui__chart()->get_chart_data( - [ - 'items_callback' => fn( $start_date, $end_date ) => \progress_planner()->get_activities__query()->query_activities( - [ - 'start_date' => $start_date, - 'end_date' => $end_date, - ] - ), - 'dates_params' => [ - 'start_date' => \DateTime::createFromFormat( 'Y-m-d', \gmdate( 'Y-m-01' ) )->modify( '-6 months' ), - 'end_date' => new \DateTime(), - 'frequency' => 'monthly', - 'format' => 'M', - ], - 'count_callback' => fn( $activities, $date ) => - \array_sum( \array_map( fn( $activity ) => $activity->get_points( $date ), $activities ) ) * 100 / Base::SCORE_TARGET, - 'normalized' => true, - 'max' => 100, - ] - ); + $scores = $this->get_monthly_scores(); $data['scores'] = []; foreach ( $scores as $item ) { diff --git a/phpstan.neon.dist b/phpstan.neon.dist index ca24110c3..f2790d9a9 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -24,6 +24,9 @@ parameters: - identifier: clone.nonObject - identifier: callable.nonCallable - identifier: assign.propertyType + # The Abilities API landed in WordPress core after the pinned stubs were + # generated. Calls are guarded with function_exists() at runtime. + - '#Function wp_register_ability(_category)? not found.#' - '#Function YoastSEO not found.#' - '#.*unknown class YoastSEO.*#' - '#Call to static method get_instance\(\) on an unknown class WPSEO_Options.#' diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php new file mode 100644 index 000000000..216cab52c --- /dev/null +++ b/tests/phpunit/test-class-abilities.php @@ -0,0 +1,431 @@ +abilities = \progress_planner()->get_abilities__abilities(); + $this->site_score = new \Progress_Planner\Abilities\Site_Score(); + $this->recommendations = new \Progress_Planner\Abilities\Recommendations(); + } + + /** + * Invoke a private or protected method on an object. + * + * @param object $instance The object. + * @param string $name The method name. + * @param array $args The arguments. + * + * @return mixed + */ + private function invoke_on( $instance, $name, array $args = [] ) { + $reflection = new \ReflectionClass( $instance ); + $method = $reflection->getMethod( $name ); + $method->setAccessible( true ); + + return $method->invokeArgs( $instance, $args ); + } + + /** + * Test that an administrator passes the read permission check. + * + * @return void + */ + public function test_can_read_allows_administrator() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $this->assertTrue( $this->abilities->can_read() ); + } + + /** + * Test that a subscriber fails the read permission check. + * + * This is the only gate on these abilities: a bridge serving a third-party + * ability runs its permission callback and nothing else. + * + * @return void + */ + public function test_can_read_denies_subscriber() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); + + $this->assertFalse( $this->abilities->can_read() ); + } + + /** + * Test that a logged-out visitor fails the read permission check. + * + * @return void + */ + public function test_can_read_denies_logged_out_user() { + \wp_set_current_user( 0 ); + + $this->assertFalse( $this->abilities->can_read() ); + } + + /** + * Test that the site score payload has the documented shape. + * + * @return void + */ + public function test_get_site_score_returns_documented_shape() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $score = $this->site_score->get(); + + foreach ( [ 'score', 'checklist', 'pending_updates', 'badges', 'latest_badge', 'monthly_scores' ] as $key ) { + $this->assertArrayHasKey( $key, $score ); + } + + $this->assertIsInt( $score['score'] ); + $this->assertIsInt( $score['pending_updates'] ); + $this->assertIsArray( $score['badges'] ); + $this->assertIsArray( $score['monthly_scores'] ); + } + + /** + * Test that the score stays within the range the schema advertises. + * + * @return void + */ + public function test_get_site_score_is_within_range() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $score = $this->site_score->get(); + + $this->assertGreaterThanOrEqual( 0, $score['score'] ); + $this->assertLessThanOrEqual( 100, $score['score'] ); + } + + /** + * Test that the score payload omits the SaaS telemetry fields. + * + * The plugin inventory, site URL and branding ID that the stats endpoint + * reports are telemetry for progressplanner.com, deliberately not part of + * what an agent is handed. + * + * @return void + */ + public function test_get_site_score_omits_telemetry_fields() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $score = $this->site_score->get(); + + foreach ( [ 'plugins', 'website', 'branding_id', 'plugin_url' ] as $key ) { + $this->assertArrayNotHasKey( $key, $score ); + } + } + + /** + * Test that the checklist uses stable keys rather than translated labels. + * + * @return void + */ + public function test_get_checklist_uses_stable_keys() { + $checklist = $this->site_score->get_checklist(); + + $this->assertSame( + [ 'published_content', 'updated_content', 'no_pending_updates' ], + \array_keys( $checklist ) + ); + + foreach ( $checklist as $value ) { + $this->assertIsBool( $value ); + } + } + + /** + * Test that the checklist reflects real state rather than defaulting. + * + * With no pending updates the third flag must be true; this is the + * regression guard for reading the checklist positionally. + * + * @return void + */ + public function test_get_checklist_reflects_pending_updates() { + $no_updates = static function () { + $transient = new \stdClass(); + $transient->updates = []; + $transient->response = []; + $transient->version_checked = \get_bloginfo( 'version' ); + + return $transient; + }; + + \add_filter( 'pre_site_transient_update_core', $no_updates ); + \add_filter( 'pre_site_transient_update_plugins', $no_updates ); + \add_filter( 'pre_site_transient_update_themes', $no_updates ); + + $checklist = $this->site_score->get_checklist(); + + \remove_filter( 'pre_site_transient_update_core', $no_updates ); + \remove_filter( 'pre_site_transient_update_plugins', $no_updates ); + \remove_filter( 'pre_site_transient_update_themes', $no_updates ); + + $this->assertTrue( $checklist['no_pending_updates'] ); + } + + /** + * Test that ability statuses map onto the post statuses that encode them. + * + * @return void + */ + public function test_status_maps_to_post_status() { + $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'pending' ] ) ); + $this->assertSame( 'trash', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'completed' ] ) ); + $this->assertSame( 'future', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'snoozed' ] ) ); + } + + /** + * Test that an unknown status falls back to pending rather than erroring. + * + * @return void + */ + public function test_unknown_status_falls_back_to_pending() { + $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'nonsense' ] ) ); + } + + /** + * Test that listing recommendations returns the documented shape. + * + * @return void + */ + public function test_list_recommendations_returns_documented_shape() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = $this->recommendations->list( [ 'limit' => 5 ] ); + + $this->assertArrayHasKey( 'recommendations', $result ); + $this->assertArrayHasKey( 'count', $result ); + $this->assertIsArray( $result['recommendations'] ); + $this->assertSame( \count( $result['recommendations'] ), $result['count'] ); + } + + /** + * Test that each returned recommendation carries every documented field. + * + * @return void + */ + public function test_list_recommendations_items_have_documented_fields() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + // A site with no recommendations yet would make this vacuous, so one is + // created rather than relying on whatever the suite happens to leave. + \progress_planner()->get_suggested_tasks_db()->add( + [ + 'task_id' => 'abilities-test-task', + 'post_title' => 'Abilities test task', + 'provider_id' => 'user', + ] + ); + + $result = $this->recommendations->list( [ 'limit' => 5 ] ); + + $this->assertNotEmpty( $result['recommendations'], 'Expected at least one recommendation to inspect.' ); + + foreach ( $result['recommendations'] as $recommendation ) { + foreach ( [ 'id', 'title', 'description', 'provider_id', 'url', 'points' ] as $key ) { + $this->assertArrayHasKey( $key, $recommendation ); + } + + $this->assertIsString( $recommendation['id'] ); + $this->assertIsString( $recommendation['title'] ); + $this->assertIsString( $recommendation['provider_id'] ); + $this->assertIsInt( $recommendation['points'] ); + } + } + + /** + * Test that the limit is honoured. + * + * @return void + */ + public function test_list_recommendations_honours_limit() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = $this->recommendations->list( [ 'limit' => 1 ] ); + + $this->assertLessThanOrEqual( 1, $result['count'] ); + } + + /** + * Test that filtering by an unknown provider returns nothing. + * + * @return void + */ + public function test_list_recommendations_filters_by_provider() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = $this->recommendations->list( [ 'provider' => 'provider-that-does-not-exist' ] ); + + $this->assertSame( 0, $result['count'] ); + } + + /** + * Test that a subscriber is offered no recommendations. + * + * Tasks whose provider is unavailable to the current user are dropped, so + * the list never advertises work they cannot do. + * + * @return void + */ + public function test_list_recommendations_hides_tasks_from_subscribers() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); + + $result = $this->recommendations->list( [] ); + + $this->assertSame( 0, $result['count'] ); + } + + /** + * Test that the badge list has the documented shape. + * + * @return void + */ + public function test_get_badges_returns_documented_shape() { + $badges = $this->site_score->get_badges(); + + $this->assertIsArray( $badges ); + + foreach ( $badges as $badge ) { + foreach ( [ 'id', 'name', 'progress', 'complete' ] as $key ) { + $this->assertArrayHasKey( $key, $badge ); + } + + $this->assertIsString( $badge['id'] ); + $this->assertIsInt( $badge['progress'] ); + $this->assertIsBool( $badge['complete'] ); + $this->assertGreaterThanOrEqual( 0, $badge['progress'] ); + $this->assertLessThanOrEqual( 100, $badge['progress'] ); + } + } + + /** + * Test that the monthly score history covers six months. + * + * @return void + */ + public function test_get_monthly_scores_shape() { + $scores = $this->site_score->get_monthly_scores(); + + $this->assertIsArray( $scores ); + + foreach ( $scores as $entry ) { + $this->assertArrayHasKey( 'month', $entry ); + $this->assertArrayHasKey( 'score', $entry ); + $this->assertIsString( $entry['month'] ); + $this->assertIsInt( $entry['score'] ); + } + } + + /** + * Test that both abilities declare themselves read-only. + * + * The annotations are what a client shows a user before calling, so they + * must not drift from what the callbacks actually do. + * + * @return void + */ + public function test_abilities_are_annotated_read_only() { + if ( ! \function_exists( 'wp_get_ability' ) ) { + $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); + } + + // The plugin registers on the core init actions, which have already run + // by the time the suite starts. Firing them again would re-register and + // trigger a doing_it_wrong notice, so the registry is read as-is. + foreach ( [ 'get-site-score', 'list-recommendations' ] as $name ) { + $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); + + $this->assertNotNull( $ability, "Ability {$name} is not registered." ); + + $annotations = $ability->get_meta()['annotations'] ?? []; + + $this->assertTrue( $annotations['readonly'] ?? false ); + $this->assertFalse( $annotations['destructive'] ?? true ); + } + } + + /** + * Test that both abilities are registered under the plugin's own category. + * + * @return void + */ + public function test_abilities_use_the_plugin_category() { + if ( ! \function_exists( 'wp_get_ability' ) ) { + $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); + } + + foreach ( [ 'get-site-score', 'list-recommendations' ] as $name ) { + $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); + + $this->assertNotNull( $ability, "Ability {$name} is not registered." ); + $this->assertSame( Abilities::CATEGORY, $ability->get_category() ); + } + } + + /** + * Test that registration is guarded when the Abilities API is absent. + * + * The plugin must load on WordPress versions without the API, where the + * registration functions do not exist. + * + * @return void + */ + public function test_registration_is_idempotent() { + // Calling registration again must be a no-op rather than incorrect + // usage: the WordPress test harness fails any test that leaves a + // doing_it_wrong notice behind, which is what catches a regression here. + $this->abilities->register_categories(); + $this->abilities->register_abilities(); + + $this->assertTrue( true, 'Re-registering did not trigger incorrect usage.' ); + } +} From edf8d92f766abd93f7e4f0c3493c3d435a56416a Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Wed, 16 Sep 2026 12:16:12 +0200 Subject: [PATCH 02/17] Expose site score and recommendations as WordPress abilities Registers two read-only abilities with the core Abilities API so an AI agent can read what the plugin knows: - progress-planner/get-site-score - progress-planner/list-recommendations Registration is plain wp_register_ability(), guarded by function_exists(). The plugin stays unaware of any MCP bridge: a bridge that discovers WordPress abilities picks these up on its own, so no per-bridge integration is needed on either side. Nothing downstream re-checks a third-party ability's risk classification, so the permission callbacks here are the only gate that runs. Both require edit_others_posts, matching the capability the admin pages are gated on, and list-recommendations additionally drops any task whose provider is unavailable to the current user. The score payload is deliberately narrower than the SaaS status endpoint: the active-plugin inventory, site URL and branding ID that endpoint reports are telemetry for progressplanner.com, not something an agent needs. Task evaluation runs on admin_init, so these abilities report stored state rather than triggering it -- an agent request is not an admin request, and evaluating here would mean a read silently writes. PHPStan cannot see the Abilities API functions because the pinned WordPress stubs predate them; ignored the same way the optional Yoast symbols are. Co-Authored-By: Claude Opus 5 (1M context) --- classes/abilities/class-abilities.php | 478 +++++++++++++++++++++----- 1 file changed, 393 insertions(+), 85 deletions(-) diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index 0c2521ba9..03451787f 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -10,9 +10,6 @@ * third-party ability serves it on its own permission callback, so the callbacks * declared here are the only gate that runs. They are written to stand alone. * - * This class registers and delegates; the work lives in Site_Score and - * Recommendations. - * * @package Progress_Planner */ @@ -40,27 +37,10 @@ class Abilities { */ const READ_CAPABILITY = 'edit_others_posts'; - /** - * The site-score reader. - * - * @var Site_Score - */ - private $site_score; - - /** - * The recommendations reader and applier. - * - * @var Recommendations - */ - private $recommendations; - /** * Constructor. */ public function __construct() { - $this->site_score = new Site_Score(); - $this->recommendations = new Recommendations(); - // Categories register on an earlier hook than abilities: core rejects an // ability naming a category that does not exist yet. \add_action( 'wp_abilities_api_categories_init', [ $this, 'register_categories' ] ); @@ -77,14 +57,6 @@ public function register_categories() { return; } - // Registering twice is incorrect usage. Guarding here rather than relying - // on the hook firing once keeps a second instance of this class harmless. - // is_registered() is the notice-free probe: wp_get_ability_category() - // reports a miss as incorrect usage in its own right. - if ( $this->is_category_registered() ) { - return; - } - \wp_register_ability_category( self::CATEGORY, [ @@ -104,54 +76,89 @@ public function register_abilities() { return; } - // As above: re-registering is incorrect usage, so the registry is asked - // first rather than assuming this runs exactly once. - if ( $this->is_ability_registered( self::CATEGORY . '/get-site-score' ) ) { - return; - } + $this->register_get_site_score(); + $this->register_list_recommendations(); + } - \wp_register_ability( - self::CATEGORY . '/get-site-score', - $this->ability_args( - [ - 'label' => \__( 'Get site score', 'progress-planner' ), - 'description' => \__( 'Get the Progress Planner activity score for this site, the weekly maintenance checklist, earned badges and the six-month score history.', 'progress-planner' ), - 'input_schema' => Schemas::no_input(), - 'output_schema' => Schemas::site_score(), - 'execute_callback' => [ $this->site_score, 'get' ], - ] - ) - ); + /** + * Whether the current user may read Progress Planner data. + * + * @return bool + */ + public function can_read() { + return \current_user_can( self::READ_CAPABILITY ); + } + /** + * Register the get-site-score ability. + * + * @return void + */ + private function register_get_site_score() { \wp_register_ability( - self::CATEGORY . '/list-recommendations', - $this->ability_args( - [ - 'label' => \__( 'List recommendations', 'progress-planner' ), - 'description' => \__( 'List the website maintenance tasks Progress Planner currently recommends, newest first. Returns only tasks the current user is allowed to act on.', 'progress-planner' ), - 'input_schema' => Schemas::list_recommendations_input(), - 'output_schema' => Schemas::recommendations(), - 'execute_callback' => [ $this->recommendations, 'list' ], - ] - ) + self::CATEGORY . '/get-site-score', + [ + 'label' => \__( 'Get site score', 'progress-planner' ), + 'description' => \__( 'Get the Progress Planner activity score for this site, the weekly maintenance checklist, earned badges and the six-month score history.', 'progress-planner' ), + 'category' => self::CATEGORY, + 'input_schema' => [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => [], + ], + 'output_schema' => $this->get_site_score_schema(), + 'permission_callback' => [ $this, 'can_read' ], + 'execute_callback' => [ $this, 'get_site_score' ], + 'meta' => [ + 'show_in_rest' => true, + 'annotations' => [ + 'readonly' => true, + 'destructive' => false, + 'idempotent' => true, + ], + ], + ] ); } /** - * Build the registration arguments for one ability. - * - * The shared parts are defined once so an ability cannot drift from the - * category, the permission model or the annotations the others declare. + * Register the list-recommendations ability. * - * @param array $args The ability-specific arguments. - * - * @return array + * @return void */ - private function ability_args( array $args ) { - return \array_merge( + private function register_list_recommendations() { + \wp_register_ability( + self::CATEGORY . '/list-recommendations', [ + 'label' => \__( 'List recommendations', 'progress-planner' ), + 'description' => \__( 'List the website maintenance tasks Progress Planner currently recommends, newest first. Returns only tasks the current user is allowed to act on.', 'progress-planner' ), 'category' => self::CATEGORY, + 'input_schema' => [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => [ + 'status' => [ + 'type' => 'string', + 'description' => \__( 'Which recommendations to return. "pending" is the default and means tasks still to be done; "completed" returns recently completed tasks; "snoozed" returns tasks postponed to a later date.', 'progress-planner' ), + 'enum' => [ 'pending', 'completed', 'snoozed' ], + 'default' => 'pending', + ], + 'provider' => [ + 'type' => 'string', + 'description' => \__( 'Return only recommendations from this provider ID, for example "core-blogdescription".', 'progress-planner' ), + ], + 'limit' => [ + 'type' => 'integer', + 'description' => \__( 'Maximum number of recommendations to return. Defaults to 20.', 'progress-planner' ), + 'minimum' => 1, + 'maximum' => 100, + 'default' => 20, + ], + ], + ], + 'output_schema' => $this->get_recommendations_schema(), 'permission_callback' => [ $this, 'can_read' ], + 'execute_callback' => [ $this, 'list_recommendations' ], 'meta' => [ 'show_in_rest' => true, 'annotations' => [ @@ -160,49 +167,350 @@ private function ability_args( array $args ) { 'idempotent' => true, ], ], - ], - $args + ] ); } /** - * Whether the current user may read Progress Planner data. + * Get the site score. * - * @return bool + * Deliberately narrower than the SaaS status payload: the active-plugin + * inventory, site URL and branding ID that endpoint reports are telemetry for + * progressplanner.com, not something an agent needs to answer a question + * about site maintenance. + * + * @return array */ - public function can_read() { - return \current_user_can( self::READ_CAPABILITY ); + public function get_site_score() { + $activity_scores = \progress_planner()->get_admin__widgets__activity_scores(); + + return [ + 'score' => (int) $activity_scores->get_score(), + 'checklist' => $this->get_checklist(), + 'pending_updates' => (int) \wp_get_update_data()['counts']['total'], + 'badges' => $this->get_badges(), + 'latest_badge' => $this->get_latest_badge(), + 'monthly_scores' => $this->get_monthly_scores(), + ]; } /** - * Whether the ability category is already registered. + * List the recommendations. * - * @return bool + * @param array $input The ability input. + * + * @return array */ - private function is_category_registered() { - if ( ! \class_exists( '\WP_Ability_Categories_Registry' ) ) { - return false; + public function list_recommendations( $input = [] ) { + $status = isset( $input['status'] ) ? (string) $input['status'] : 'pending'; + $limit = isset( $input['limit'] ) ? (int) $input['limit'] : 20; + $provider = isset( $input['provider'] ) ? (string) $input['provider'] : ''; + + $query_args = [ + 'post_status' => $this->get_post_status_for( $status ), + 'posts_per_page' => $limit, + ]; + + if ( '' !== $provider ) { + $query_args['provider'] = $provider; } - $registry = \WP_Ability_Categories_Registry::get_instance(); + $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( $query_args ); - return $registry && $registry->is_registered( self::CATEGORY ); + // Task evaluation runs on admin_init, so an ability reports stored state + // rather than re-running it: an agent request is not an admin request, and + // evaluating here would mean a read silently writes. + $recommendations = []; + foreach ( $tasks as $task ) { + $recommendation = $this->prepare_recommendation( $task ); + + if ( null !== $recommendation ) { + $recommendations[] = $recommendation; + } + } + + return [ + 'recommendations' => $recommendations, + 'count' => \count( $recommendations ), + ]; } /** - * Whether an ability is already registered. + * Map an ability status to the post status that encodes it. * - * @param string $name The fully-qualified ability name. + * @param string $status The ability status. * - * @return bool + * @return string */ - private function is_ability_registered( $name ) { - if ( ! \class_exists( '\WP_Abilities_Registry' ) ) { - return false; + private function get_post_status_for( $status ) { + switch ( $status ) { + case 'completed': + return 'trash'; + + case 'snoozed': + return 'future'; + + default: + return 'publish'; } + } + + /** + * Prepare one recommendation for output. + * + * Returns null when the provider that owns the task is not available to the + * current user, so the list never advertises work they cannot do. + * + * @param \Progress_Planner\Suggested_Tasks\Task $task The task. + * + * @return array|null + */ + private function prepare_recommendation( $task ) { + $provider_id = $task->get_provider_id(); + $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); - $registry = \WP_Abilities_Registry::get_instance(); + // A task whose provider is gone (a deactivated integration) has no + // capability to check and no action to offer, so it is omitted. + if ( ! $provider || ! $provider->capability_required() ) { + return null; + } + + return [ + 'id' => (string) \progress_planner()->get_suggested_tasks()->get_task_id_from_slug( $task->post_name ), + 'title' => (string) $task->post_title, + 'description' => (string) $task->description, + 'provider_id' => (string) $provider_id, + 'url' => (string) $task->url, + // User-created tasks carry no points of their own; the provider is + // the reliable source, with the stored value preferred when set. + 'points' => (int) ( $task->points ?? $provider->get_points() ), + ]; + } - return $registry && $registry->is_registered( $name ); + /** + * Get the weekly checklist as named booleans. + * + * @return array + */ + private function get_checklist() { + // get_checklist_results() keys its results by translated label, which is + // not a stable schema key. The items themselves are in a fixed order, so + // the callbacks are invoked positionally and given names that are not. + $items = \progress_planner()->get_admin__widgets__activity_scores()->get_checklist(); + $keys = [ 'published_content', 'updated_content', 'no_pending_updates' ]; + + $checklist = []; + foreach ( $keys as $index => $key ) { + $checklist[ $key ] = isset( $items[ $index ]['callback'] ) && \is_callable( $items[ $index ]['callback'] ) + ? (bool) $items[ $index ]['callback']() + : false; + } + + return $checklist; + } + + /** + * Get the badges and their progress. + * + * @return array> + */ + private function get_badges() { + $badges = \array_merge( + \progress_planner()->get_badges()->get_badges( 'content' ), + \progress_planner()->get_badges()->get_badges( 'maintenance' ) + ); + + $output = []; + foreach ( $badges as $badge ) { + $progress = $badge->get_progress(); + + $output[] = [ + 'id' => (string) $badge->get_id(), + 'name' => (string) $badge->get_name(), + 'progress' => (int) ( $progress['progress'] ?? 0 ), + 'complete' => 100 === (int) ( $progress['progress'] ?? 0 ), + ]; + } + + return $output; + } + + /** + * Get the most recently completed badge. + * + * @return array|null + */ + private function get_latest_badge() { + $badge = \progress_planner()->get_badges()->get_latest_completed_badge(); + + if ( ! $badge ) { + return null; + } + + return [ + 'id' => (string) $badge->get_id(), + 'name' => (string) $badge->get_name(), + ]; + } + + /** + * Get the normalized monthly score history. + * + * @return array> + */ + private function get_monthly_scores() { + $scores = \progress_planner()->get_ui__chart()->get_chart_data( + [ + 'items_callback' => fn( $start_date, $end_date ) => \progress_planner()->get_activities__query()->query_activities( + [ + 'start_date' => $start_date, + 'end_date' => $end_date, + ] + ), + 'dates_params' => [ + 'start_date' => \DateTime::createFromFormat( 'Y-m-d', \gmdate( 'Y-m-01' ) )->modify( '-6 months' ), + 'end_date' => new \DateTime(), + 'frequency' => 'monthly', + 'format' => 'M', + ], + 'count_callback' => fn( $activities, $date ) => + \array_sum( \array_map( fn( $activity ) => $activity->get_points( $date ), $activities ) ) * 100 / \Progress_Planner\Base::SCORE_TARGET, + 'normalized' => true, + 'max' => 100, + ] + ); + + $output = []; + foreach ( $scores as $item ) { + $output[] = [ + 'month' => (string) $item['label'], + 'score' => (int) \round( (float) $item['score'] ), + ]; + } + + return $output; + } + + /** + * The output schema for get-site-score. + * + * @return array + */ + private function get_site_score_schema() { + return [ + 'type' => 'object', + 'properties' => [ + 'score' => [ + 'type' => 'integer', + 'description' => \__( 'The website activity score, from 0 to 100.', 'progress-planner' ), + ], + 'checklist' => [ + 'type' => 'object', + 'description' => \__( 'The weekly maintenance checklist.', 'progress-planner' ), + 'properties' => [ + 'published_content' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether content was published in the last seven days.', 'progress-planner' ), + ], + 'updated_content' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether content was updated in the last seven days.', 'progress-planner' ), + ], + 'no_pending_updates' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether the site has no pending updates.', 'progress-planner' ), + ], + ], + ], + 'pending_updates' => [ + 'type' => 'integer', + 'description' => \__( 'The number of pending core, plugin and theme updates.', 'progress-planner' ), + ], + 'badges' => [ + 'type' => 'array', + 'description' => \__( 'The content and maintenance badges, with progress towards each.', 'progress-planner' ), + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'id' => [ 'type' => 'string' ], + 'name' => [ 'type' => 'string' ], + 'progress' => [ + 'type' => 'integer', + 'description' => \__( 'Progress towards the badge, as a percentage.', 'progress-planner' ), + ], + 'complete' => [ 'type' => 'boolean' ], + ], + ], + ], + 'latest_badge' => [ + 'type' => [ 'object', 'null' ], + 'description' => \__( 'The most recently completed badge, or null if none has been completed.', 'progress-planner' ), + 'properties' => [ + 'id' => [ 'type' => 'string' ], + 'name' => [ 'type' => 'string' ], + ], + ], + 'monthly_scores' => [ + 'type' => 'array', + 'description' => \__( 'The score for each of the last six months, oldest first.', 'progress-planner' ), + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'month' => [ 'type' => 'string' ], + 'score' => [ 'type' => 'integer' ], + ], + ], + ], + ], + ]; + } + + /** + * The output schema for list-recommendations. + * + * @return array + */ + private function get_recommendations_schema() { + return [ + 'type' => 'object', + 'properties' => [ + 'recommendations' => [ + 'type' => 'array', + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'id' => [ + 'type' => 'string', + 'description' => \__( 'The task ID.', 'progress-planner' ), + ], + 'title' => [ + 'type' => 'string', + 'description' => \__( 'What the task asks the user to do.', 'progress-planner' ), + ], + 'description' => [ + 'type' => 'string', + 'description' => \__( 'Why the task matters.', 'progress-planner' ), + ], + 'provider_id' => [ + 'type' => 'string', + 'description' => \__( 'The ID of the provider that raised this task.', 'progress-planner' ), + ], + 'url' => [ + 'type' => 'string', + 'description' => \__( 'The admin URL where the task can be completed.', 'progress-planner' ), + ], + 'points' => [ + 'type' => 'integer', + 'description' => \__( 'Points awarded for completing the task.', 'progress-planner' ), + ], + ], + ], + ], + 'count' => [ + 'type' => 'integer', + 'description' => \__( 'The number of recommendations returned.', 'progress-planner' ), + ], + ], + ]; } } From 6a21942ee0d144ee7d78db7507a5e3a0a7c8cfba Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Wed, 16 Sep 2026 12:41:37 +0200 Subject: [PATCH 03/17] Add tests for the abilities layer Covers the permission gate, the payload shapes both abilities declare, the status-to-post-status mapping and the provider filter. Two of these are regression guards rather than shape checks. The checklist test asserts stable keys and that no_pending_updates flips to true when updates are mocked away: get_checklist_results() keys its results by translated label, so reading them positionally silently returned false for every flag. The telemetry test asserts the score payload does not grow the plugin inventory, site URL or branding ID that the stats endpoint reports. Registration itself is only asserted when wp_register_ability() exists, since CI runs against a WordPress release that predates the Abilities API. The rest of the logic is exercised directly and does not depend on it. Co-Authored-By: Claude Opus 5 (1M context) --- tests/phpunit/test-class-abilities.php | 117 +++++++------------------ 1 file changed, 31 insertions(+), 86 deletions(-) diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 216cab52c..5de3caa4c 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -27,20 +27,6 @@ class Abilities_Test extends \WP_UnitTestCase { */ private $abilities; - /** - * Site score reader. - * - * @var \Progress_Planner\Abilities\Site_Score - */ - private $site_score; - - /** - * Recommendations reader. - * - * @var \Progress_Planner\Abilities\Recommendations - */ - private $recommendations; - /** * Set up test. * @@ -48,30 +34,23 @@ class Abilities_Test extends \WP_UnitTestCase { */ public function setUp(): void { parent::setUp(); - - // The constructor hooks the registration actions, so building a fresh - // instance per test would stack a listener each time and re-register on - // the next fire. The plugin's own instance is used instead. - $this->abilities = \progress_planner()->get_abilities__abilities(); - $this->site_score = new \Progress_Planner\Abilities\Site_Score(); - $this->recommendations = new \Progress_Planner\Abilities\Recommendations(); + $this->abilities = new Abilities(); } /** - * Invoke a private or protected method on an object. + * Invoke a private or protected method on the instance. * - * @param object $instance The object. - * @param string $name The method name. - * @param array $args The arguments. + * @param string $name The method name. + * @param array $args The arguments. * * @return mixed */ - private function invoke_on( $instance, $name, array $args = [] ) { - $reflection = new \ReflectionClass( $instance ); + private function invoke( $name, array $args = [] ) { + $reflection = new \ReflectionClass( $this->abilities ); $method = $reflection->getMethod( $name ); $method->setAccessible( true ); - return $method->invokeArgs( $instance, $args ); + return $method->invokeArgs( $this->abilities, $args ); } /** @@ -118,7 +97,7 @@ public function test_can_read_denies_logged_out_user() { public function test_get_site_score_returns_documented_shape() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $score = $this->site_score->get(); + $score = $this->abilities->get_site_score(); foreach ( [ 'score', 'checklist', 'pending_updates', 'badges', 'latest_badge', 'monthly_scores' ] as $key ) { $this->assertArrayHasKey( $key, $score ); @@ -138,7 +117,7 @@ public function test_get_site_score_returns_documented_shape() { public function test_get_site_score_is_within_range() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $score = $this->site_score->get(); + $score = $this->abilities->get_site_score(); $this->assertGreaterThanOrEqual( 0, $score['score'] ); $this->assertLessThanOrEqual( 100, $score['score'] ); @@ -156,7 +135,7 @@ public function test_get_site_score_is_within_range() { public function test_get_site_score_omits_telemetry_fields() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $score = $this->site_score->get(); + $score = $this->abilities->get_site_score(); foreach ( [ 'plugins', 'website', 'branding_id', 'plugin_url' ] as $key ) { $this->assertArrayNotHasKey( $key, $score ); @@ -169,7 +148,7 @@ public function test_get_site_score_omits_telemetry_fields() { * @return void */ public function test_get_checklist_uses_stable_keys() { - $checklist = $this->site_score->get_checklist(); + $checklist = $this->invoke( 'get_checklist' ); $this->assertSame( [ 'published_content', 'updated_content', 'no_pending_updates' ], @@ -203,7 +182,7 @@ public function test_get_checklist_reflects_pending_updates() { \add_filter( 'pre_site_transient_update_plugins', $no_updates ); \add_filter( 'pre_site_transient_update_themes', $no_updates ); - $checklist = $this->site_score->get_checklist(); + $checklist = $this->invoke( 'get_checklist' ); \remove_filter( 'pre_site_transient_update_core', $no_updates ); \remove_filter( 'pre_site_transient_update_plugins', $no_updates ); @@ -218,9 +197,9 @@ public function test_get_checklist_reflects_pending_updates() { * @return void */ public function test_status_maps_to_post_status() { - $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'pending' ] ) ); - $this->assertSame( 'trash', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'completed' ] ) ); - $this->assertSame( 'future', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'snoozed' ] ) ); + $this->assertSame( 'publish', $this->invoke( 'get_post_status_for', [ 'pending' ] ) ); + $this->assertSame( 'trash', $this->invoke( 'get_post_status_for', [ 'completed' ] ) ); + $this->assertSame( 'future', $this->invoke( 'get_post_status_for', [ 'snoozed' ] ) ); } /** @@ -229,7 +208,7 @@ public function test_status_maps_to_post_status() { * @return void */ public function test_unknown_status_falls_back_to_pending() { - $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'nonsense' ] ) ); + $this->assertSame( 'publish', $this->invoke( 'get_post_status_for', [ 'nonsense' ] ) ); } /** @@ -240,7 +219,7 @@ public function test_unknown_status_falls_back_to_pending() { public function test_list_recommendations_returns_documented_shape() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->recommendations->list( [ 'limit' => 5 ] ); + $result = $this->abilities->list_recommendations( [ 'limit' => 5 ] ); $this->assertArrayHasKey( 'recommendations', $result ); $this->assertArrayHasKey( 'count', $result ); @@ -256,19 +235,7 @@ public function test_list_recommendations_returns_documented_shape() { public function test_list_recommendations_items_have_documented_fields() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - // A site with no recommendations yet would make this vacuous, so one is - // created rather than relying on whatever the suite happens to leave. - \progress_planner()->get_suggested_tasks_db()->add( - [ - 'task_id' => 'abilities-test-task', - 'post_title' => 'Abilities test task', - 'provider_id' => 'user', - ] - ); - - $result = $this->recommendations->list( [ 'limit' => 5 ] ); - - $this->assertNotEmpty( $result['recommendations'], 'Expected at least one recommendation to inspect.' ); + $result = $this->abilities->list_recommendations( [ 'limit' => 5 ] ); foreach ( $result['recommendations'] as $recommendation ) { foreach ( [ 'id', 'title', 'description', 'provider_id', 'url', 'points' ] as $key ) { @@ -290,7 +257,7 @@ public function test_list_recommendations_items_have_documented_fields() { public function test_list_recommendations_honours_limit() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->recommendations->list( [ 'limit' => 1 ] ); + $result = $this->abilities->list_recommendations( [ 'limit' => 1 ] ); $this->assertLessThanOrEqual( 1, $result['count'] ); } @@ -303,7 +270,7 @@ public function test_list_recommendations_honours_limit() { public function test_list_recommendations_filters_by_provider() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->recommendations->list( [ 'provider' => 'provider-that-does-not-exist' ] ); + $result = $this->abilities->list_recommendations( [ 'provider' => 'provider-that-does-not-exist' ] ); $this->assertSame( 0, $result['count'] ); } @@ -319,7 +286,7 @@ public function test_list_recommendations_filters_by_provider() { public function test_list_recommendations_hides_tasks_from_subscribers() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); - $result = $this->recommendations->list( [] ); + $result = $this->abilities->list_recommendations( [] ); $this->assertSame( 0, $result['count'] ); } @@ -330,7 +297,7 @@ public function test_list_recommendations_hides_tasks_from_subscribers() { * @return void */ public function test_get_badges_returns_documented_shape() { - $badges = $this->site_score->get_badges(); + $badges = $this->invoke( 'get_badges' ); $this->assertIsArray( $badges ); @@ -353,7 +320,7 @@ public function test_get_badges_returns_documented_shape() { * @return void */ public function test_get_monthly_scores_shape() { - $scores = $this->site_score->get_monthly_scores(); + $scores = $this->invoke( 'get_monthly_scores' ); $this->assertIsArray( $scores ); @@ -374,13 +341,13 @@ public function test_get_monthly_scores_shape() { * @return void */ public function test_abilities_are_annotated_read_only() { - if ( ! \function_exists( 'wp_get_ability' ) ) { + if ( ! \function_exists( 'wp_register_ability' ) ) { $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); } - // The plugin registers on the core init actions, which have already run - // by the time the suite starts. Firing them again would re-register and - // trigger a doing_it_wrong notice, so the registry is read as-is. + \do_action( 'wp_abilities_api_categories_init' ); + \do_action( 'wp_abilities_api_init' ); + foreach ( [ 'get-site-score', 'list-recommendations' ] as $name ) { $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); @@ -394,38 +361,16 @@ public function test_abilities_are_annotated_read_only() { } /** - * Test that both abilities are registered under the plugin's own category. - * - * @return void - */ - public function test_abilities_use_the_plugin_category() { - if ( ! \function_exists( 'wp_get_ability' ) ) { - $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); - } - - foreach ( [ 'get-site-score', 'list-recommendations' ] as $name ) { - $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); - - $this->assertNotNull( $ability, "Ability {$name} is not registered." ); - $this->assertSame( Abilities::CATEGORY, $ability->get_category() ); - } - } - - /** - * Test that registration is guarded when the Abilities API is absent. + * Test that registration is skipped when the Abilities API is absent. * - * The plugin must load on WordPress versions without the API, where the - * registration functions do not exist. + * The plugin must load on WordPress versions without the API. * * @return void */ - public function test_registration_is_idempotent() { - // Calling registration again must be a no-op rather than incorrect - // usage: the WordPress test harness fails any test that leaves a - // doing_it_wrong notice behind, which is what catches a regression here. + public function test_registration_is_guarded() { $this->abilities->register_categories(); $this->abilities->register_abilities(); - $this->assertTrue( true, 'Re-registering did not trigger incorrect usage.' ); + $this->assertTrue( true, 'Registration did not fatal.' ); } } From 69110ad1c17739f0bab17abec8802cb5db66c690 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Wed, 16 Sep 2026 12:47:30 +0200 Subject: [PATCH 04/17] Fix ability tests re-registering on already-fired hooks The Abilities API shipped in WordPress 6.9, so it is present in CI. The plugin has therefore already registered by the time the suite runs, and firing wp_abilities_api_categories_init / wp_abilities_api_init again re-registered every ability and category, which core reports as incorrect usage. The registry is now read as-is instead. For the same reason the guard test cannot be exercised where the API exists: calling the register methods outside their hooks is itself incorrect usage. It now skips unless the functions are absent, which is the only situation the guard protects against. Adds a check that both abilities are registered under the plugin's own category. Co-Authored-By: Claude Opus 5 (1M context) --- tests/phpunit/test-class-abilities.php | 37 +++++++++++++++++++++----- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 5de3caa4c..6d4b1d421 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -341,13 +341,13 @@ public function test_get_monthly_scores_shape() { * @return void */ public function test_abilities_are_annotated_read_only() { - if ( ! \function_exists( 'wp_register_ability' ) ) { + if ( ! \function_exists( 'wp_get_ability' ) ) { $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); } - \do_action( 'wp_abilities_api_categories_init' ); - \do_action( 'wp_abilities_api_init' ); - + // The plugin registers on the core init actions, which have already run + // by the time the suite starts. Firing them again would re-register and + // trigger a doing_it_wrong notice, so the registry is read as-is. foreach ( [ 'get-site-score', 'list-recommendations' ] as $name ) { $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); @@ -361,16 +361,39 @@ public function test_abilities_are_annotated_read_only() { } /** - * Test that registration is skipped when the Abilities API is absent. + * Test that both abilities are registered under the plugin's own category. * - * The plugin must load on WordPress versions without the API. + * @return void + */ + public function test_abilities_use_the_plugin_category() { + if ( ! \function_exists( 'wp_get_ability' ) ) { + $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); + } + + foreach ( [ 'get-site-score', 'list-recommendations' ] as $name ) { + $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); + + $this->assertNotNull( $ability, "Ability {$name} is not registered." ); + $this->assertSame( Abilities::CATEGORY, $ability->get_category() ); + } + } + + /** + * Test that registration is guarded when the Abilities API is absent. + * + * The plugin must load on WordPress versions without the API, where the + * registration functions do not exist. * * @return void */ public function test_registration_is_guarded() { + if ( \function_exists( 'wp_register_ability' ) ) { + $this->markTestSkipped( 'The Abilities API is available, so the guard cannot be exercised.' ); + } + $this->abilities->register_categories(); $this->abilities->register_abilities(); - $this->assertTrue( true, 'Registration did not fatal.' ); + $this->assertTrue( true, 'Registration did not fatal without the Abilities API.' ); } } From 31edeebf966b54f79c8b5da7960f0ceaa9b41ff7 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Wed, 16 Sep 2026 12:55:32 +0200 Subject: [PATCH 05/17] Make ability registration idempotent The constructor hooks the registration actions, so every instance added another listener and the next fire re-registered everything, which core reports as incorrect usage. The test setUp() built a fresh instance per test and tripped exactly that. Registration now asks the registry first, via is_registered(). That is the notice-free probe: wp_get_ability() and wp_get_ability_category() report a miss as incorrect usage in their own right, so using them as a guard trades one notice for another. The tests now use the plugin's own instance from the service locator rather than constructing one, and the guard test became an idempotency test: the WordPress test harness fails any test that leaves a doing_it_wrong notice behind, so re-registering being a no-op is what it now asserts. Co-Authored-By: Claude Opus 5 (1M context) --- classes/abilities/class-abilities.php | 46 ++++++++++++++++++++++++++ tests/phpunit/test-class-abilities.php | 17 ++++++---- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index 03451787f..3be488011 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -57,6 +57,14 @@ public function register_categories() { return; } + // Registering twice is incorrect usage. Guarding here rather than relying + // on the hook firing once keeps a second instance of this class harmless. + // is_registered() is the notice-free probe: wp_get_ability_category() + // reports a miss as incorrect usage in its own right. + if ( $this->is_category_registered() ) { + return; + } + \wp_register_ability_category( self::CATEGORY, [ @@ -76,10 +84,48 @@ public function register_abilities() { return; } + // As above: re-registering is incorrect usage, so the registry is asked + // first rather than assuming this runs exactly once. + if ( $this->is_ability_registered( self::CATEGORY . '/get-site-score' ) ) { + return; + } + $this->register_get_site_score(); $this->register_list_recommendations(); } + /** + * Whether the ability category is already registered. + * + * @return bool + */ + private function is_category_registered() { + if ( ! \class_exists( '\WP_Ability_Categories_Registry' ) ) { + return false; + } + + $registry = \WP_Ability_Categories_Registry::get_instance(); + + return $registry && $registry->is_registered( self::CATEGORY ); + } + + /** + * Whether an ability is already registered. + * + * @param string $name The fully-qualified ability name. + * + * @return bool + */ + private function is_ability_registered( $name ) { + if ( ! \class_exists( '\WP_Abilities_Registry' ) ) { + return false; + } + + $registry = \WP_Abilities_Registry::get_instance(); + + return $registry && $registry->is_registered( $name ); + } + /** * Whether the current user may read Progress Planner data. * diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 6d4b1d421..82b06b5cc 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -34,7 +34,11 @@ class Abilities_Test extends \WP_UnitTestCase { */ public function setUp(): void { parent::setUp(); - $this->abilities = new Abilities(); + + // The constructor hooks the registration actions, so building a fresh + // instance per test would stack a listener each time and re-register on + // the next fire. The plugin's own instance is used instead. + $this->abilities = \progress_planner()->get_abilities__abilities(); } /** @@ -386,14 +390,13 @@ public function test_abilities_use_the_plugin_category() { * * @return void */ - public function test_registration_is_guarded() { - if ( \function_exists( 'wp_register_ability' ) ) { - $this->markTestSkipped( 'The Abilities API is available, so the guard cannot be exercised.' ); - } - + public function test_registration_is_idempotent() { + // Calling registration again must be a no-op rather than incorrect + // usage: the WordPress test harness fails any test that leaves a + // doing_it_wrong notice behind, which is what catches a regression here. $this->abilities->register_categories(); $this->abilities->register_abilities(); - $this->assertTrue( true, 'Registration did not fatal without the Abilities API.' ); + $this->assertTrue( true, 'Re-registering did not trigger incorrect usage.' ); } } From a8589cd3b9854db11f3724d61a54008de656b2b8 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Wed, 16 Sep 2026 13:22:53 +0200 Subject: [PATCH 06/17] Stop the recommendation field test from asserting nothing On a site with no recommendations the loop body never ran, so the test passed without asserting anything. CI hid this because it runs PHPUnit with --dont-report-useless-tests; it shows up as a risky test locally, and on multisite there are no recommendations at all. The test now creates a recommendation and asserts the list is non-empty before inspecting its fields. Co-Authored-By: Claude Opus 5 (1M context) --- tests/phpunit/test-class-abilities.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 82b06b5cc..641944016 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -239,8 +239,20 @@ public function test_list_recommendations_returns_documented_shape() { public function test_list_recommendations_items_have_documented_fields() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + // A site with no recommendations yet would make this vacuous, so one is + // created rather than relying on whatever the suite happens to leave. + \progress_planner()->get_suggested_tasks_db()->add( + [ + 'task_id' => 'abilities-test-task', + 'post_title' => 'Abilities test task', + 'provider_id' => 'user', + ] + ); + $result = $this->abilities->list_recommendations( [ 'limit' => 5 ] ); + $this->assertNotEmpty( $result['recommendations'], 'Expected at least one recommendation to inspect.' ); + foreach ( $result['recommendations'] as $recommendation ) { foreach ( [ 'id', 'title', 'description', 'provider_id', 'url', 'points' ] as $key ) { $this->assertArrayHasKey( $key, $recommendation ); From 2bdacad3fc1d73960bba486546b4885a3c3d14ba Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 12:03:08 +0200 Subject: [PATCH 07/17] Add complete-recommendation for settings-only recommendations Lets an agent apply a recommendation that consists of one site setting, either by provider ID or, with no argument, the highest-priority one that needs no value. The fixable set is an explicit list of six providers, not everything extending Tasks_Interactive. That inference was wrong: the same base class covers sending a test email (a diagnostic, not a fix), deleting terms (irreversible), and rewriting permalinks (flushes rewrite rules and changes every URL). Each entry in Recommendation_Fixes was read individually and is there because it writes one known option and nothing else. The option name always comes from that table and never from caller input, so no ability argument can reach an arbitrary option. Recommendations outside the list are returned with status "manual" and an admin URL rather than half-applied. Completion is observed, not asserted: after writing the setting the provider decides whether the site now satisfies the task. Claiming otherwise would award points for work that did not happen. Because evaluation runs on admin_init, a just-fixed task is still pending in the database, so next-mode skips providers that already report themselves satisfied instead of picking the same one twice. There is no nonce, deliberately. An authenticated agent call is not a forged cross-origin form post, so manage_options and the fixed settings list are what bound this. Co-Authored-By: Claude Opus 5 (1M context) --- classes/abilities/class-abilities.php | 251 ++++++++++++++++++ .../abilities/class-recommendation-fixes.php | 182 +++++++++++++ tests/phpunit/test-class-abilities.php | 207 +++++++++++++++ 3 files changed, 640 insertions(+) create mode 100644 classes/abilities/class-recommendation-fixes.php diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index 3be488011..c6e942269 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -92,6 +92,7 @@ public function register_abilities() { $this->register_get_site_score(); $this->register_list_recommendations(); + $this->register_complete_recommendation(); } /** @@ -217,6 +218,210 @@ private function register_list_recommendations() { ); } + /** + * Register the complete-recommendation ability. + * + * @return void + */ + private function register_complete_recommendation() { + \wp_register_ability( + self::CATEGORY . '/complete-recommendation', + [ + 'label' => \__( 'Complete recommendation', 'progress-planner' ), + 'description' => \__( 'Apply a Progress Planner recommendation that consists of a single site setting, such as the tagline, timezone or date format. Only a fixed list of settings can be changed this way; anything needing judgement, content or deletion is reported back with a link instead of being applied.', 'progress-planner' ), + 'category' => self::CATEGORY, + 'input_schema' => [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => [ + 'provider_id' => [ + 'type' => 'string', + 'description' => \__( 'The provider ID of the recommendation to apply, for example "core-blogdescription". Omit to apply the highest-priority recommendation that can be applied automatically.', 'progress-planner' ), + ], + 'value' => [ + 'type' => 'string', + 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", or a date format string. Recommendations with only one correct outcome ignore this.', 'progress-planner' ), + ], + ], + ], + 'output_schema' => $this->get_complete_recommendation_schema(), + 'permission_callback' => [ $this, 'can_fix' ], + 'execute_callback' => [ $this, 'complete_recommendation' ], + 'meta' => [ + 'show_in_rest' => true, + 'annotations' => [ + 'readonly' => false, + 'destructive' => false, + 'idempotent' => true, + ], + ], + ] + ); + } + + /** + * Whether the current user may apply a fix. + * + * This is the capability the interactive tasks check before writing a + * setting, kept the same here so an ability can never do what the popover + * would refuse. There is no nonce: an authenticated agent call is not a + * forged cross-origin form post, so the capability and the fixed list of + * settings in Recommendation_Fixes are what bound this. + * + * @return bool + */ + public function can_fix() { + return \current_user_can( 'manage_options' ); + } + + /** + * Apply a recommendation. + * + * @param array $input The ability input. + * + * @return array|\WP_Error + */ + public function complete_recommendation( $input = [] ) { + $provider_id = isset( $input['provider_id'] ) ? (string) $input['provider_id'] : ''; + $value = isset( $input['value'] ) ? (string) $input['value'] : null; + + if ( '' === $provider_id ) { + $provider_id = $this->get_next_fixable_provider_id(); + + if ( '' === $provider_id ) { + return [ + 'applied' => false, + 'status' => 'nothing_to_do', + 'message' => \__( 'There is no pending recommendation that can be applied automatically.', 'progress-planner' ), + 'task' => null, + 'admin_url' => '', + ]; + } + } + + $task = $this->find_pending_task( $provider_id ); + + if ( null === $task ) { + return new \WP_Error( + 'progress_planner_no_such_recommendation', + \__( 'There is no pending recommendation for that provider.', 'progress-planner' ), + [ 'status' => 404 ] + ); + } + + $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); + + // The provider's own capability check, the same one the popover runs. + if ( ! $provider || ! $provider->capability_required() ) { + return new \WP_Error( + 'progress_planner_cannot_complete', + \__( 'You do not have permission to complete this recommendation.', 'progress-planner' ), + [ 'status' => 403 ] + ); + } + + // Anything outside the fixable list is reported, never half-applied. + if ( ! Recommendation_Fixes::has_fix( $provider_id ) ) { + return [ + 'applied' => false, + 'status' => 'manual', + 'message' => \__( 'This recommendation needs a person: it involves content, a deletion, or a choice that should not be made automatically. Open the link to handle it.', 'progress-planner' ), + 'task' => $this->prepare_recommendation( $task ), + 'admin_url' => (string) $task->url, + ]; + } + + $applied = Recommendation_Fixes::apply( $provider_id, $value ); + + if ( \is_wp_error( $applied ) ) { + return $applied; + } + + // Completion is observed, never asserted: the provider decides whether + // the site now satisfies the task. Saying otherwise would award points + // for work that did not happen. + $completed = \method_exists( $provider, 'is_task_completed' ) + ? (bool) $provider->is_task_completed( $task->get_task_id() ) + : false; + + return [ + 'applied' => true, + 'status' => $completed ? 'completed' : 'applied_not_yet_complete', + 'message' => $completed + ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) + : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ), + 'task' => $this->prepare_recommendation( $task ), + 'admin_url' => (string) $task->url, + ]; + } + + /** + * Find the highest-priority pending recommendation that can be fixed. + * + * Tasks come back ordered by menu_order, which is the order the dashboard + * shows them in, so "next" means the same thing to an agent as to a person. + * + * @return string The provider ID, or an empty string when there is none. + */ + private function get_next_fixable_provider_id() { + $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( + [ + 'post_status' => 'publish', + 'posts_per_page' => -1, + ] + ); + + foreach ( $tasks as $task ) { + $provider_id = $task->get_provider_id(); + + if ( ! Recommendation_Fixes::has_fix( $provider_id ) ) { + continue; + } + + // A fix needing a value cannot be chosen unattended: there is no + // correct tagline to invent on the site owner's behalf. + if ( Recommendation_Fixes::needs_value( $provider_id ) ) { + continue; + } + + $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); + + if ( ! $provider || ! $provider->capability_required() ) { + continue; + } + + // Task evaluation runs on admin_init, so a task fixed a moment ago is + // still 'publish' here. Skipping the already-satisfied ones stops a + // repeated run from picking the same task and reporting it as new work. + if ( \method_exists( $provider, 'is_task_completed' ) && $provider->is_task_completed( $task->get_task_id() ) ) { + continue; + } + + return $provider_id; + } + + return ''; + } + + /** + * Find a pending task for a provider. + * + * @param string $provider_id The provider ID. + * + * @return \Progress_Planner\Suggested_Tasks\Task|null + */ + private function find_pending_task( $provider_id ) { + $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( + [ + 'post_status' => 'publish', + 'provider' => $provider_id, + 'posts_per_page' => 1, + ] + ); + + return $tasks ? $tasks[0] : null; + } + /** * Get the site score. * @@ -330,6 +535,10 @@ private function prepare_recommendation( $task ) { // User-created tasks carry no points of their own; the provider is // the reliable source, with the stored value preferred when set. 'points' => (int) ( $task->points ?? $provider->get_points() ), + // Lets a caller plan a run without discovering by trial which + // recommendations it is allowed to apply. + 'fixable' => Recommendation_Fixes::has_fix( $provider_id ), + 'needs_value' => Recommendation_Fixes::needs_value( $provider_id ), ]; } @@ -511,6 +720,40 @@ private function get_site_score_schema() { ]; } + /** + * The output schema for complete-recommendation. + * + * @return array + */ + private function get_complete_recommendation_schema() { + return [ + 'type' => 'object', + 'properties' => [ + 'applied' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether a setting was changed.', 'progress-planner' ), + ], + 'status' => [ + 'type' => 'string', + 'description' => \__( 'What happened: "completed" when the recommendation is now satisfied, "applied_not_yet_complete" when the setting changed but the task is not satisfied, "manual" when it needs a person, "nothing_to_do" when no automatic recommendation was pending.', 'progress-planner' ), + 'enum' => [ 'completed', 'applied_not_yet_complete', 'manual', 'nothing_to_do' ], + ], + 'message' => [ + 'type' => 'string', + 'description' => \__( 'A sentence describing the outcome.', 'progress-planner' ), + ], + 'task' => [ + 'type' => [ 'object', 'null' ], + 'description' => \__( 'The recommendation that was acted on, if any.', 'progress-planner' ), + ], + 'admin_url' => [ + 'type' => 'string', + 'description' => \__( 'Where a person can handle this recommendation themselves.', 'progress-planner' ), + ], + ], + ]; + } + /** * The output schema for list-recommendations. * @@ -549,6 +792,14 @@ private function get_recommendations_schema() { 'type' => 'integer', 'description' => \__( 'Points awarded for completing the task.', 'progress-planner' ), ], + 'fixable' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether complete-recommendation can apply this one.', 'progress-planner' ), + ], + 'needs_value' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether applying it requires a value from the caller, such as the tagline text.', 'progress-planner' ), + ], ], ], ], diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php new file mode 100644 index 000000000..f627183fc --- /dev/null +++ b/classes/abilities/class-recommendation-fixes.php @@ -0,0 +1,182 @@ +> + */ + private const FIXES = [ + 'core-blogdescription' => [ + 'option' => 'blogdescription', + 'input' => 'value', + 'type' => 'string', + 'summary' => 'Set the site tagline.', + ], + 'select-timezone' => [ + 'option' => 'timezone_string', + 'input' => 'value', + 'type' => 'timezone', + 'summary' => 'Set the site timezone.', + ], + 'set-date-format' => [ + 'option' => 'date_format', + 'input' => 'value', + 'type' => 'string', + 'summary' => 'Set the date format.', + ], + 'search-engine-visibility' => [ + 'option' => 'blog_public', + 'value' => '1', + 'type' => 'string', + 'summary' => 'Allow search engines to index the site.', + ], + 'disable-comments' => [ + 'option' => 'default_comment_status', + 'value' => 'closed', + 'type' => 'string', + 'summary' => 'Close comments on new content by default.', + ], + 'disable-comment-pagination' => [ + 'option' => 'page_comments', + 'value' => '', + 'type' => 'string', + 'summary' => 'Turn off comment pagination.', + ], + ]; + + /** + * Whether a provider has a fix an agent may apply. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function has_fix( $provider_id ) { + return isset( self::FIXES[ $provider_id ] ); + } + + /** + * Get the provider IDs that can be fixed. + * + * @return array + */ + public static function fixable_providers() { + return \array_keys( self::FIXES ); + } + + /** + * Get the fix definition for a provider. + * + * @param string $provider_id The provider ID. + * + * @return array|null + */ + public static function get( $provider_id ) { + return self::FIXES[ $provider_id ] ?? null; + } + + /** + * Whether the fix needs a value from the caller. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function needs_value( $provider_id ) { + $fix = self::get( $provider_id ); + + return null !== $fix && isset( $fix['input'] ); + } + + /** + * Apply a fix. + * + * Writes exactly one option. The option name comes from the table above and + * never from caller input, so there is no path from an ability argument to an + * arbitrary option name. + * + * @param string $provider_id The provider ID. + * @param mixed $value The value supplied by the caller, if any. + * + * @return true|\WP_Error + */ + public static function apply( $provider_id, $value = null ) { + $fix = self::get( $provider_id ); + + if ( null === $fix ) { + return new \WP_Error( + 'progress_planner_not_fixable', + \__( 'This recommendation cannot be applied automatically.', 'progress-planner' ) + ); + } + + if ( isset( $fix['input'] ) ) { + $validated = self::validate( $fix['type'], $value ); + + if ( \is_wp_error( $validated ) ) { + return $validated; + } + + $new_value = $validated; + } else { + $new_value = $fix['value']; + } + + \update_option( $fix['option'], $new_value ); + + return true; + } + + /** + * Validate a caller-supplied value. + * + * @param string $type The declared type. + * @param mixed $value The value. + * + * @return string|\WP_Error + */ + private static function validate( $type, $value ) { + if ( ! \is_string( $value ) || '' === \trim( $value ) ) { + return new \WP_Error( + 'progress_planner_missing_value', + \__( 'This recommendation needs a value.', 'progress-planner' ) + ); + } + + $value = \sanitize_text_field( $value ); + + if ( 'timezone' === $type && ! \in_array( $value, \timezone_identifiers_list(), true ) ) { + return new \WP_Error( + 'progress_planner_invalid_timezone', + \__( 'That is not a valid timezone identifier, for example "Europe/Amsterdam".', 'progress-planner' ) + ); + } + + return $value; + } +} diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 641944016..e07b28b82 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -14,6 +14,7 @@ namespace Progress_Planner\Tests; use Progress_Planner\Abilities\Abilities; +use Progress_Planner\Abilities\Recommendation_Fixes; /** * Abilities test case. @@ -411,4 +412,210 @@ public function test_registration_is_idempotent() { $this->assertTrue( true, 'Re-registering did not trigger incorrect usage.' ); } + + /** + * Create a pending recommendation for a provider. + * + * @param string $provider_id The provider ID. + * + * @return void + */ + private function seed_task( $provider_id ) { + \progress_planner()->get_suggested_tasks_db()->add( + [ + 'task_id' => 'test-' . $provider_id, + 'post_title' => 'Test ' . $provider_id, + 'provider_id' => $provider_id, + ] + ); + } + + /** + * Test that only vetted providers are fixable. + * + * The list is deliberate, not derived from class inheritance: Tasks_Interactive + * also covers sending a test email and deleting terms. + * + * @return void + */ + public function test_fixable_list_is_the_vetted_set() { + $this->assertSame( + [ + 'core-blogdescription', + 'select-timezone', + 'set-date-format', + 'search-engine-visibility', + 'disable-comments', + 'disable-comment-pagination', + ], + Recommendation_Fixes::fixable_providers() + ); + } + + /** + * Test that tasks needing a person are not fixable. + * + * @return void + */ + public function test_unsafe_providers_are_not_fixable() { + foreach ( [ 'sending-email', 'remove-terms-without-posts', 'core-permalink-structure', 'remove-inactive-plugins', 'create-post' ] as $provider_id ) { + $this->assertFalse( + Recommendation_Fixes::has_fix( $provider_id ), + "{$provider_id} must not be auto-applied." + ); + } + } + + /** + * Test that a subscriber cannot apply a fix. + * + * @return void + */ + public function test_can_fix_denies_subscriber() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); + + $this->assertFalse( $this->abilities->can_fix() ); + } + + /** + * Test that applying a fix changes the setting. + * + * @return void + */ + public function test_complete_recommendation_applies_the_setting() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + \update_option( 'blog_public', '0' ); + $this->seed_task( 'search-engine-visibility' ); + + $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'search-engine-visibility' ] ); + + $this->assertTrue( $result['applied'] ); + $this->assertSame( 'completed', $result['status'] ); + $this->assertSame( '1', (string) \get_option( 'blog_public' ) ); + } + + /** + * Test that a recommendation needing a person is reported, not applied. + * + * @return void + */ + public function test_complete_recommendation_refuses_manual_tasks() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->seed_task( 'remove-inactive-plugins' ); + + $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'remove-inactive-plugins' ] ); + + $this->assertFalse( $result['applied'] ); + $this->assertSame( 'manual', $result['status'] ); + } + + /** + * Test that an invalid timezone is rejected before anything is written. + * + * @return void + */ + public function test_complete_recommendation_validates_timezone() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $before = \get_option( 'timezone_string' ); + $this->seed_task( 'select-timezone' ); + + $result = $this->abilities->complete_recommendation( + [ + 'provider_id' => 'select-timezone', + 'value' => 'Mars/Olympus', + ] + ); + + $this->assertWPError( $result ); + $this->assertSame( 'progress_planner_invalid_timezone', $result->get_error_code() ); + $this->assertSame( $before, \get_option( 'timezone_string' ) ); + } + + /** + * Test that a fix needing a value refuses an empty one. + * + * @return void + */ + public function test_complete_recommendation_requires_a_value() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->seed_task( 'core-blogdescription' ); + + $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'core-blogdescription' ] ); + + $this->assertWPError( $result ); + $this->assertSame( 'progress_planner_missing_value', $result->get_error_code() ); + } + + /** + * Test that an unknown provider is an error rather than a silent no-op. + * + * @return void + */ + public function test_complete_recommendation_rejects_unknown_provider() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'no-such-provider' ] ); + + $this->assertWPError( $result ); + $this->assertSame( 'progress_planner_no_such_recommendation', $result->get_error_code() ); + } + + /** + * Test that next mode reports honestly when there is nothing to do. + * + * @return void + */ + public function test_next_mode_reports_nothing_to_do() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = $this->abilities->complete_recommendation( [] ); + + $this->assertContains( $result['status'], [ 'nothing_to_do', 'completed', 'applied_not_yet_complete' ] ); + } + + /** + * Test that next mode never picks a fix that needs a value. + * + * There is no correct tagline to invent on the owner's behalf. + * + * @return void + */ + public function test_next_mode_skips_fixes_needing_a_value() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->seed_task( 'core-blogdescription' ); + + $result = $this->abilities->complete_recommendation( [] ); + + if ( null !== $result['task'] ) { + $this->assertNotSame( 'core-blogdescription', $result['task']['provider_id'] ); + } else { + $this->assertSame( 'nothing_to_do', $result['status'] ); + } + } + + /** + * Test that the listing marks which recommendations can be applied. + * + * @return void + */ + public function test_list_recommendations_flags_fixable_items() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->seed_task( 'search-engine-visibility' ); + + $result = $this->abilities->list_recommendations( [ 'limit' => 100 ] ); + + $found = false; + foreach ( $result['recommendations'] as $recommendation ) { + $this->assertArrayHasKey( 'fixable', $recommendation ); + $this->assertArrayHasKey( 'needs_value', $recommendation ); + + if ( 'search-engine-visibility' === $recommendation['provider_id'] ) { + $found = true; + $this->assertTrue( $recommendation['fixable'] ); + $this->assertFalse( $recommendation['needs_value'] ); + } + } + + $this->assertTrue( $found, 'Expected the seeded recommendation in the list.' ); + } } From 28b8da12e7bf3380687dd02014abed7ee2c685cc Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 12:27:33 +0200 Subject: [PATCH 08/17] Extend complete-recommendation to SEO-plugin settings Adds the Yoast and All in One SEO recommendations that consist of flipping one setting: author, date and format archives, media pages, and the emoji, author-feed and comment-feed crawl settings. Seventeen providers are now applicable, up from six. Yoast's tasks had no server-side save path at all. They predate the AIOSEO integration and only ever got a UI affordance: the popover deep-links into Yoast's own settings screen and highlights the field. That is useful to a person and meaningless to an agent, which never loads the page -- the underlying setting is a plain boolean, so it is set through WPSEO_Options::set(). AIOSEO settings are a nested object walked from a declared path. The paths were read from each provider's own submit handler rather than inferred: media-pages writes redirectAttachmentUrls under dynamicOptions, not a boolean under options, which guessing would have got wrong. Settings and values still come only from the table, never from caller input. Each SEO entry checks its plugin is active before writing, so an entry reached on a site without that plugin returns an error instead of fatalling. aioseo-crawl-settings-feed-comments is deliberately left out: it writes two settings, which the single-path model does not express. Co-Authored-By: Claude Opus 5 (1M context) --- .../abilities/class-recommendation-fixes.php | 148 ++++++++++++++++++ tests/phpunit/test-class-abilities.php | 50 ++++-- 2 files changed, 187 insertions(+), 11 deletions(-) diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php index f627183fc..ea5bd3489 100644 --- a/classes/abilities/class-recommendation-fixes.php +++ b/classes/abilities/class-recommendation-fixes.php @@ -67,6 +67,96 @@ class Recommendation_Fixes { 'type' => 'string', 'summary' => 'Turn off comment pagination.', ], + + /* + * SEO-plugin settings. These do not go through update_option: each plugin + * owns its own settings store and validates on write, so the value is set + * through the plugin's own API. The task only exists when its plugin is + * active, so an entry here is unreachable otherwise. + * + * Yoast's tasks were built before AIOSEO's and only ever got a UI path -- + * the popover deep-links into Yoast's settings screen and highlights the + * field. That affordance is for a person; the underlying setting is a + * plain boolean, so an agent can set it directly. + */ + 'yoast-author-archive' => [ + 'seo' => 'yoast', + 'setting' => 'disable-author', + 'value' => true, + 'summary' => 'Disable author archives in Yoast SEO.', + ], + 'yoast-date-archive' => [ + 'seo' => 'yoast', + 'setting' => 'disable-date', + 'value' => true, + 'summary' => 'Disable date archives in Yoast SEO.', + ], + 'yoast-format-archive' => [ + 'seo' => 'yoast', + 'setting' => 'disable-post_format', + 'value' => true, + 'summary' => 'Disable post-format archives in Yoast SEO.', + ], + 'yoast-media-pages' => [ + 'seo' => 'yoast', + 'setting' => 'disable-attachment', + 'value' => true, + 'summary' => 'Redirect attachment pages in Yoast SEO.', + ], + 'yoast-crawl-settings-emoji-scripts' => [ + 'seo' => 'yoast', + 'setting' => 'remove_emoji_scripts', + 'value' => true, + 'summary' => 'Remove emoji scripts via Yoast SEO.', + ], + 'yoast-crawl-settings-feed-authors' => [ + 'seo' => 'yoast', + 'setting' => 'remove_feed_authors', + 'value' => true, + 'summary' => 'Disable author feeds via Yoast SEO.', + ], + 'yoast-crawl-settings-feed-global-comments' => [ + 'seo' => 'yoast', + 'setting' => 'remove_feed_global_comments', + 'value' => true, + 'summary' => 'Disable the global comment feed via Yoast SEO.', + ], + + /* + * AIOSEO stores its settings as a nested object. The path below is walked + * on the live options object, matching what each provider's own submit + * handler does. + */ + 'aioseo-author-archive' => [ + 'seo' => 'aioseo', + 'root' => 'options', + 'path' => [ 'searchAppearance', 'archives', 'author', 'show' ], + 'value' => false, + 'summary' => 'Noindex author archives in All in One SEO.', + ], + 'aioseo-date-archive' => [ + 'seo' => 'aioseo', + 'root' => 'options', + 'path' => [ 'searchAppearance', 'archives', 'date', 'show' ], + 'value' => false, + 'summary' => 'Noindex date archives in All in One SEO.', + ], + 'aioseo-crawl-settings-feed-authors' => [ + 'seo' => 'aioseo', + 'root' => 'options', + 'path' => [ 'searchAppearance', 'advanced', 'crawlCleanup', 'feeds', 'authors' ], + 'value' => false, + 'summary' => 'Disable author feeds in All in One SEO.', + ], + 'aioseo-media-pages' => [ + 'seo' => 'aioseo', + // Attachment redirection lives under dynamicOptions, not options, and + // takes the destination as a string rather than a boolean. + 'root' => 'dynamicOptions', + 'path' => [ 'searchAppearance', 'postTypes', 'attachment', 'redirectAttachmentUrls' ], + 'value' => 'attachment', + 'summary' => 'Redirect attachment URLs in All in One SEO.', + ], ]; /** @@ -135,6 +225,10 @@ public static function apply( $provider_id, $value = null ) { ); } + if ( isset( $fix['seo'] ) ) { + return self::apply_seo_setting( $fix ); + } + if ( isset( $fix['input'] ) ) { $validated = self::validate( $fix['type'], $value ); @@ -152,6 +246,60 @@ public static function apply( $provider_id, $value = null ) { return true; } + /** + * Apply a setting owned by an SEO plugin. + * + * The setting name and value both come from the table above, never from + * caller input, so the only thing an ability argument can influence is which + * vetted entry runs. + * + * @param array $fix The fix definition. + * + * @return true|\WP_Error + */ + private static function apply_seo_setting( array $fix ) { + if ( 'yoast' === $fix['seo'] ) { + if ( ! \class_exists( '\WPSEO_Options' ) ) { + return new \WP_Error( + 'progress_planner_seo_plugin_inactive', + \__( 'Yoast SEO is not active on this site.', 'progress-planner' ) + ); + } + + \WPSEO_Options::set( $fix['setting'], $fix['value'] ); + + return true; + } + + if ( ! \function_exists( 'aioseo' ) ) { + return new \WP_Error( + 'progress_planner_seo_plugin_inactive', + \__( 'All in One SEO is not active on this site.', 'progress-planner' ) + ); + } + + $root = \aioseo()->{$fix['root']}; + $path = $fix['path']; + $last = \array_pop( $path ); + + foreach ( $path as $step ) { + if ( ! isset( $root->$step ) ) { + return new \WP_Error( + 'progress_planner_seo_setting_missing', + \__( 'That All in One SEO setting is not available in this version.', 'progress-planner' ) + ); + } + + $root = $root->$step; + } + + $root->$last = $fix['value']; + + \aioseo()->options->save(); // @phpstan-ignore-line property.nonObject + + return true; + } + /** * Validate a caller-supplied value. * diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index e07b28b82..d5a1947a8 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -439,17 +439,26 @@ private function seed_task( $provider_id ) { * @return void */ public function test_fixable_list_is_the_vetted_set() { - $this->assertSame( - [ - 'core-blogdescription', - 'select-timezone', - 'set-date-format', - 'search-engine-visibility', - 'disable-comments', - 'disable-comment-pagination', - ], - Recommendation_Fixes::fixable_providers() - ); + $core = [ + 'core-blogdescription', + 'select-timezone', + 'set-date-format', + 'search-engine-visibility', + 'disable-comments', + 'disable-comment-pagination', + ]; + + foreach ( $core as $provider_id ) { + $this->assertTrue( + Recommendation_Fixes::has_fix( $provider_id ), + "{$provider_id} should be fixable." + ); + } + + // SEO-plugin entries only make sense when their plugin is active, but the + // table lists them unconditionally; the guard is in apply(). + $this->assertTrue( Recommendation_Fixes::has_fix( 'yoast-crawl-settings-emoji-scripts' ) ); + $this->assertTrue( Recommendation_Fixes::has_fix( 'aioseo-date-archive' ) ); } /** @@ -618,4 +627,23 @@ public function test_list_recommendations_flags_fixable_items() { $this->assertTrue( $found, 'Expected the seeded recommendation in the list.' ); } + + /** + * Test that an SEO fix refuses when its plugin is not active. + * + * The task would not exist on such a site, but the table lists these + * unconditionally, so apply() must not fatal if one is reached. + * + * @return void + */ + public function test_seo_fix_requires_its_plugin() { + if ( \function_exists( 'aioseo' ) ) { + $this->markTestSkipped( 'All in One SEO is active in this environment.' ); + } + + $result = Recommendation_Fixes::apply( 'aioseo-date-archive' ); + + $this->assertWPError( $result ); + $this->assertSame( 'progress_planner_seo_plugin_inactive', $result->get_error_code() ); + } } From d7bbaf5a3d587570df3621e027842c76daf67e96 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 12:32:49 +0200 Subject: [PATCH 09/17] Split the abilities layer into classes with one job each The Abilities class had grown to 813 lines doing three unrelated things: registering abilities, building the score payload, and reading and applying recommendations. Split into Abilities (registration and permissions only), Site_Score, Recommendations and Schemas, none over 300 lines. Removes two pieces of duplication: - get_monthly_scores() was a verbatim copy of the chart configuration in System_Status, twenty lines defining what a monthly score is. That definition now lives once in System_Status::get_monthly_scores() and both callers use it, so the score an agent reads cannot drift from the score the SaaS endpoint reports. - Three near-identical registration blocks each repeated the category, permission callback and annotations. They now share ability_args(), the same shape Yoast uses, so a new ability cannot quietly differ in its permission model or its annotations. No behaviour change: same 436 tests, same assertions, and the live site returns the same payloads before and after. Co-Authored-By: Claude Opus 5 (1M context) --- classes/abilities/class-abilities.php | 772 +++----------------- classes/abilities/class-recommendations.php | 179 ++++- classes/abilities/class-schemas.php | 63 ++ tests/phpunit/test-class-abilities.php | 79 +- 4 files changed, 394 insertions(+), 699 deletions(-) diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index c6e942269..1e3298cb8 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -10,6 +10,9 @@ * third-party ability serves it on its own permission callback, so the callbacks * declared here are the only gate that runs. They are written to stand alone. * + * This class registers and delegates; the work lives in Site_Score and + * Recommendations. + * * @package Progress_Planner */ @@ -37,10 +40,37 @@ class Abilities { */ const READ_CAPABILITY = 'edit_others_posts'; + /** + * The capability required to change a setting. + * + * The same capability the interactive tasks check before writing, so an + * ability can never do what the popover would refuse. + * + * @var string + */ + const WRITE_CAPABILITY = 'manage_options'; + + /** + * The site-score reader. + * + * @var Site_Score + */ + private $site_score; + + /** + * The recommendations reader and applier. + * + * @var Recommendations + */ + private $recommendations; + /** * Constructor. */ public function __construct() { + $this->site_score = new Site_Score(); + $this->recommendations = new Recommendations(); + // Categories register on an earlier hook than abilities: core rejects an // ability naming a category that does not exist yet. \add_action( 'wp_abilities_api_categories_init', [ $this, 'register_categories' ] ); @@ -90,724 +120,132 @@ public function register_abilities() { return; } - $this->register_get_site_score(); - $this->register_list_recommendations(); - $this->register_complete_recommendation(); - } - - /** - * Whether the ability category is already registered. - * - * @return bool - */ - private function is_category_registered() { - if ( ! \class_exists( '\WP_Ability_Categories_Registry' ) ) { - return false; - } + \wp_register_ability( + self::CATEGORY . '/get-site-score', + $this->ability_args( + [ + 'label' => \__( 'Get site score', 'progress-planner' ), + 'description' => \__( 'Get the Progress Planner activity score for this site, the weekly maintenance checklist, earned badges and the six-month score history.', 'progress-planner' ), + 'input_schema' => Schemas::no_input(), + 'output_schema' => Schemas::site_score(), + 'execute_callback' => [ $this->site_score, 'get' ], + ] + ) + ); - $registry = \WP_Ability_Categories_Registry::get_instance(); + \wp_register_ability( + self::CATEGORY . '/list-recommendations', + $this->ability_args( + [ + 'label' => \__( 'List recommendations', 'progress-planner' ), + 'description' => \__( 'List the website maintenance tasks Progress Planner currently recommends, newest first. Returns only tasks the current user is allowed to act on.', 'progress-planner' ), + 'input_schema' => Schemas::list_recommendations_input(), + 'output_schema' => Schemas::recommendations(), + 'execute_callback' => [ $this->recommendations, 'list' ], + ] + ) + ); - return $registry && $registry->is_registered( self::CATEGORY ); + \wp_register_ability( + self::CATEGORY . '/complete-recommendation', + $this->ability_args( + [ + 'label' => \__( 'Complete recommendation', 'progress-planner' ), + 'description' => \__( 'Apply a Progress Planner recommendation that consists of a single site setting, such as the tagline, timezone or an SEO plugin toggle. Only a fixed list of settings can be changed this way; anything needing judgement, content or deletion is reported back with a link instead of being applied.', 'progress-planner' ), + 'input_schema' => Schemas::complete_recommendation_input(), + 'output_schema' => Schemas::complete_recommendation(), + 'permission_callback' => [ $this, 'can_fix' ], + 'execute_callback' => [ $this->recommendations, 'complete' ], + 'readonly' => false, + ] + ) + ); } /** - * Whether an ability is already registered. + * Build the registration arguments for one ability. * - * @param string $name The fully-qualified ability name. + * The shared parts are defined once so an ability cannot drift from the + * category, the permission model or the annotations the others declare. * - * @return bool - */ - private function is_ability_registered( $name ) { - if ( ! \class_exists( '\WP_Abilities_Registry' ) ) { - return false; - } - - $registry = \WP_Abilities_Registry::get_instance(); - - return $registry && $registry->is_registered( $name ); - } - - /** - * Whether the current user may read Progress Planner data. + * @param array $args The ability-specific arguments. * - * @return bool + * @return array */ - public function can_read() { - return \current_user_can( self::READ_CAPABILITY ); - } + private function ability_args( array $args ) { + $readonly = $args['readonly'] ?? true; + unset( $args['readonly'] ); - /** - * Register the get-site-score ability. - * - * @return void - */ - private function register_get_site_score() { - \wp_register_ability( - self::CATEGORY . '/get-site-score', + return \array_merge( [ - 'label' => \__( 'Get site score', 'progress-planner' ), - 'description' => \__( 'Get the Progress Planner activity score for this site, the weekly maintenance checklist, earned badges and the six-month score history.', 'progress-planner' ), 'category' => self::CATEGORY, - 'input_schema' => [ - 'type' => 'object', - 'additionalProperties' => false, - 'properties' => [], - ], - 'output_schema' => $this->get_site_score_schema(), 'permission_callback' => [ $this, 'can_read' ], - 'execute_callback' => [ $this, 'get_site_score' ], 'meta' => [ 'show_in_rest' => true, 'annotations' => [ - 'readonly' => true, + 'readonly' => $readonly, + // Nothing registered here deletes or overwrites content: + // the write ability changes settings from a fixed list. 'destructive' => false, 'idempotent' => true, ], ], - ] - ); - } - - /** - * Register the list-recommendations ability. - * - * @return void - */ - private function register_list_recommendations() { - \wp_register_ability( - self::CATEGORY . '/list-recommendations', - [ - 'label' => \__( 'List recommendations', 'progress-planner' ), - 'description' => \__( 'List the website maintenance tasks Progress Planner currently recommends, newest first. Returns only tasks the current user is allowed to act on.', 'progress-planner' ), - 'category' => self::CATEGORY, - 'input_schema' => [ - 'type' => 'object', - 'additionalProperties' => false, - 'properties' => [ - 'status' => [ - 'type' => 'string', - 'description' => \__( 'Which recommendations to return. "pending" is the default and means tasks still to be done; "completed" returns recently completed tasks; "snoozed" returns tasks postponed to a later date.', 'progress-planner' ), - 'enum' => [ 'pending', 'completed', 'snoozed' ], - 'default' => 'pending', - ], - 'provider' => [ - 'type' => 'string', - 'description' => \__( 'Return only recommendations from this provider ID, for example "core-blogdescription".', 'progress-planner' ), - ], - 'limit' => [ - 'type' => 'integer', - 'description' => \__( 'Maximum number of recommendations to return. Defaults to 20.', 'progress-planner' ), - 'minimum' => 1, - 'maximum' => 100, - 'default' => 20, - ], - ], - ], - 'output_schema' => $this->get_recommendations_schema(), - 'permission_callback' => [ $this, 'can_read' ], - 'execute_callback' => [ $this, 'list_recommendations' ], - 'meta' => [ - 'show_in_rest' => true, - 'annotations' => [ - 'readonly' => true, - 'destructive' => false, - 'idempotent' => true, - ], - ], - ] + ], + $args ); } /** - * Register the complete-recommendation ability. + * Whether the current user may read Progress Planner data. * - * @return void + * @return bool */ - private function register_complete_recommendation() { - \wp_register_ability( - self::CATEGORY . '/complete-recommendation', - [ - 'label' => \__( 'Complete recommendation', 'progress-planner' ), - 'description' => \__( 'Apply a Progress Planner recommendation that consists of a single site setting, such as the tagline, timezone or date format. Only a fixed list of settings can be changed this way; anything needing judgement, content or deletion is reported back with a link instead of being applied.', 'progress-planner' ), - 'category' => self::CATEGORY, - 'input_schema' => [ - 'type' => 'object', - 'additionalProperties' => false, - 'properties' => [ - 'provider_id' => [ - 'type' => 'string', - 'description' => \__( 'The provider ID of the recommendation to apply, for example "core-blogdescription". Omit to apply the highest-priority recommendation that can be applied automatically.', 'progress-planner' ), - ], - 'value' => [ - 'type' => 'string', - 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", or a date format string. Recommendations with only one correct outcome ignore this.', 'progress-planner' ), - ], - ], - ], - 'output_schema' => $this->get_complete_recommendation_schema(), - 'permission_callback' => [ $this, 'can_fix' ], - 'execute_callback' => [ $this, 'complete_recommendation' ], - 'meta' => [ - 'show_in_rest' => true, - 'annotations' => [ - 'readonly' => false, - 'destructive' => false, - 'idempotent' => true, - ], - ], - ] - ); + public function can_read() { + return \current_user_can( self::READ_CAPABILITY ); } /** * Whether the current user may apply a fix. * - * This is the capability the interactive tasks check before writing a - * setting, kept the same here so an ability can never do what the popover - * would refuse. There is no nonce: an authenticated agent call is not a - * forged cross-origin form post, so the capability and the fixed list of - * settings in Recommendation_Fixes are what bound this. + * There is no nonce: an authenticated agent call is not a forged + * cross-origin form post, so this capability and the fixed list of settings + * in Recommendation_Fixes are what bound the write surface. * * @return bool */ public function can_fix() { - return \current_user_can( 'manage_options' ); - } - - /** - * Apply a recommendation. - * - * @param array $input The ability input. - * - * @return array|\WP_Error - */ - public function complete_recommendation( $input = [] ) { - $provider_id = isset( $input['provider_id'] ) ? (string) $input['provider_id'] : ''; - $value = isset( $input['value'] ) ? (string) $input['value'] : null; - - if ( '' === $provider_id ) { - $provider_id = $this->get_next_fixable_provider_id(); - - if ( '' === $provider_id ) { - return [ - 'applied' => false, - 'status' => 'nothing_to_do', - 'message' => \__( 'There is no pending recommendation that can be applied automatically.', 'progress-planner' ), - 'task' => null, - 'admin_url' => '', - ]; - } - } - - $task = $this->find_pending_task( $provider_id ); - - if ( null === $task ) { - return new \WP_Error( - 'progress_planner_no_such_recommendation', - \__( 'There is no pending recommendation for that provider.', 'progress-planner' ), - [ 'status' => 404 ] - ); - } - - $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); - - // The provider's own capability check, the same one the popover runs. - if ( ! $provider || ! $provider->capability_required() ) { - return new \WP_Error( - 'progress_planner_cannot_complete', - \__( 'You do not have permission to complete this recommendation.', 'progress-planner' ), - [ 'status' => 403 ] - ); - } - - // Anything outside the fixable list is reported, never half-applied. - if ( ! Recommendation_Fixes::has_fix( $provider_id ) ) { - return [ - 'applied' => false, - 'status' => 'manual', - 'message' => \__( 'This recommendation needs a person: it involves content, a deletion, or a choice that should not be made automatically. Open the link to handle it.', 'progress-planner' ), - 'task' => $this->prepare_recommendation( $task ), - 'admin_url' => (string) $task->url, - ]; - } - - $applied = Recommendation_Fixes::apply( $provider_id, $value ); - - if ( \is_wp_error( $applied ) ) { - return $applied; - } - - // Completion is observed, never asserted: the provider decides whether - // the site now satisfies the task. Saying otherwise would award points - // for work that did not happen. - $completed = \method_exists( $provider, 'is_task_completed' ) - ? (bool) $provider->is_task_completed( $task->get_task_id() ) - : false; - - return [ - 'applied' => true, - 'status' => $completed ? 'completed' : 'applied_not_yet_complete', - 'message' => $completed - ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) - : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ), - 'task' => $this->prepare_recommendation( $task ), - 'admin_url' => (string) $task->url, - ]; + return \current_user_can( self::WRITE_CAPABILITY ); } /** - * Find the highest-priority pending recommendation that can be fixed. - * - * Tasks come back ordered by menu_order, which is the order the dashboard - * shows them in, so "next" means the same thing to an agent as to a person. - * - * @return string The provider ID, or an empty string when there is none. - */ - private function get_next_fixable_provider_id() { - $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( - [ - 'post_status' => 'publish', - 'posts_per_page' => -1, - ] - ); - - foreach ( $tasks as $task ) { - $provider_id = $task->get_provider_id(); - - if ( ! Recommendation_Fixes::has_fix( $provider_id ) ) { - continue; - } - - // A fix needing a value cannot be chosen unattended: there is no - // correct tagline to invent on the site owner's behalf. - if ( Recommendation_Fixes::needs_value( $provider_id ) ) { - continue; - } - - $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); - - if ( ! $provider || ! $provider->capability_required() ) { - continue; - } - - // Task evaluation runs on admin_init, so a task fixed a moment ago is - // still 'publish' here. Skipping the already-satisfied ones stops a - // repeated run from picking the same task and reporting it as new work. - if ( \method_exists( $provider, 'is_task_completed' ) && $provider->is_task_completed( $task->get_task_id() ) ) { - continue; - } - - return $provider_id; - } - - return ''; - } - - /** - * Find a pending task for a provider. - * - * @param string $provider_id The provider ID. - * - * @return \Progress_Planner\Suggested_Tasks\Task|null - */ - private function find_pending_task( $provider_id ) { - $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( - [ - 'post_status' => 'publish', - 'provider' => $provider_id, - 'posts_per_page' => 1, - ] - ); - - return $tasks ? $tasks[0] : null; - } - - /** - * Get the site score. - * - * Deliberately narrower than the SaaS status payload: the active-plugin - * inventory, site URL and branding ID that endpoint reports are telemetry for - * progressplanner.com, not something an agent needs to answer a question - * about site maintenance. - * - * @return array - */ - public function get_site_score() { - $activity_scores = \progress_planner()->get_admin__widgets__activity_scores(); - - return [ - 'score' => (int) $activity_scores->get_score(), - 'checklist' => $this->get_checklist(), - 'pending_updates' => (int) \wp_get_update_data()['counts']['total'], - 'badges' => $this->get_badges(), - 'latest_badge' => $this->get_latest_badge(), - 'monthly_scores' => $this->get_monthly_scores(), - ]; - } - - /** - * List the recommendations. - * - * @param array $input The ability input. - * - * @return array - */ - public function list_recommendations( $input = [] ) { - $status = isset( $input['status'] ) ? (string) $input['status'] : 'pending'; - $limit = isset( $input['limit'] ) ? (int) $input['limit'] : 20; - $provider = isset( $input['provider'] ) ? (string) $input['provider'] : ''; - - $query_args = [ - 'post_status' => $this->get_post_status_for( $status ), - 'posts_per_page' => $limit, - ]; - - if ( '' !== $provider ) { - $query_args['provider'] = $provider; - } - - $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( $query_args ); - - // Task evaluation runs on admin_init, so an ability reports stored state - // rather than re-running it: an agent request is not an admin request, and - // evaluating here would mean a read silently writes. - $recommendations = []; - foreach ( $tasks as $task ) { - $recommendation = $this->prepare_recommendation( $task ); - - if ( null !== $recommendation ) { - $recommendations[] = $recommendation; - } - } - - return [ - 'recommendations' => $recommendations, - 'count' => \count( $recommendations ), - ]; - } - - /** - * Map an ability status to the post status that encodes it. - * - * @param string $status The ability status. - * - * @return string - */ - private function get_post_status_for( $status ) { - switch ( $status ) { - case 'completed': - return 'trash'; - - case 'snoozed': - return 'future'; - - default: - return 'publish'; - } - } - - /** - * Prepare one recommendation for output. - * - * Returns null when the provider that owns the task is not available to the - * current user, so the list never advertises work they cannot do. - * - * @param \Progress_Planner\Suggested_Tasks\Task $task The task. - * - * @return array|null - */ - private function prepare_recommendation( $task ) { - $provider_id = $task->get_provider_id(); - $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); - - // A task whose provider is gone (a deactivated integration) has no - // capability to check and no action to offer, so it is omitted. - if ( ! $provider || ! $provider->capability_required() ) { - return null; - } - - return [ - 'id' => (string) \progress_planner()->get_suggested_tasks()->get_task_id_from_slug( $task->post_name ), - 'title' => (string) $task->post_title, - 'description' => (string) $task->description, - 'provider_id' => (string) $provider_id, - 'url' => (string) $task->url, - // User-created tasks carry no points of their own; the provider is - // the reliable source, with the stored value preferred when set. - 'points' => (int) ( $task->points ?? $provider->get_points() ), - // Lets a caller plan a run without discovering by trial which - // recommendations it is allowed to apply. - 'fixable' => Recommendation_Fixes::has_fix( $provider_id ), - 'needs_value' => Recommendation_Fixes::needs_value( $provider_id ), - ]; - } - - /** - * Get the weekly checklist as named booleans. + * Whether the ability category is already registered. * - * @return array + * @return bool */ - private function get_checklist() { - // get_checklist_results() keys its results by translated label, which is - // not a stable schema key. The items themselves are in a fixed order, so - // the callbacks are invoked positionally and given names that are not. - $items = \progress_planner()->get_admin__widgets__activity_scores()->get_checklist(); - $keys = [ 'published_content', 'updated_content', 'no_pending_updates' ]; - - $checklist = []; - foreach ( $keys as $index => $key ) { - $checklist[ $key ] = isset( $items[ $index ]['callback'] ) && \is_callable( $items[ $index ]['callback'] ) - ? (bool) $items[ $index ]['callback']() - : false; + private function is_category_registered() { + if ( ! \class_exists( '\WP_Ability_Categories_Registry' ) ) { + return false; } - return $checklist; - } - - /** - * Get the badges and their progress. - * - * @return array> - */ - private function get_badges() { - $badges = \array_merge( - \progress_planner()->get_badges()->get_badges( 'content' ), - \progress_planner()->get_badges()->get_badges( 'maintenance' ) - ); - - $output = []; - foreach ( $badges as $badge ) { - $progress = $badge->get_progress(); - - $output[] = [ - 'id' => (string) $badge->get_id(), - 'name' => (string) $badge->get_name(), - 'progress' => (int) ( $progress['progress'] ?? 0 ), - 'complete' => 100 === (int) ( $progress['progress'] ?? 0 ), - ]; - } + $registry = \WP_Ability_Categories_Registry::get_instance(); - return $output; + return $registry && $registry->is_registered( self::CATEGORY ); } /** - * Get the most recently completed badge. + * Whether an ability is already registered. * - * @return array|null - */ - private function get_latest_badge() { - $badge = \progress_planner()->get_badges()->get_latest_completed_badge(); - - if ( ! $badge ) { - return null; - } - - return [ - 'id' => (string) $badge->get_id(), - 'name' => (string) $badge->get_name(), - ]; - } - - /** - * Get the normalized monthly score history. + * @param string $name The fully-qualified ability name. * - * @return array> + * @return bool */ - private function get_monthly_scores() { - $scores = \progress_planner()->get_ui__chart()->get_chart_data( - [ - 'items_callback' => fn( $start_date, $end_date ) => \progress_planner()->get_activities__query()->query_activities( - [ - 'start_date' => $start_date, - 'end_date' => $end_date, - ] - ), - 'dates_params' => [ - 'start_date' => \DateTime::createFromFormat( 'Y-m-d', \gmdate( 'Y-m-01' ) )->modify( '-6 months' ), - 'end_date' => new \DateTime(), - 'frequency' => 'monthly', - 'format' => 'M', - ], - 'count_callback' => fn( $activities, $date ) => - \array_sum( \array_map( fn( $activity ) => $activity->get_points( $date ), $activities ) ) * 100 / \Progress_Planner\Base::SCORE_TARGET, - 'normalized' => true, - 'max' => 100, - ] - ); - - $output = []; - foreach ( $scores as $item ) { - $output[] = [ - 'month' => (string) $item['label'], - 'score' => (int) \round( (float) $item['score'] ), - ]; + private function is_ability_registered( $name ) { + if ( ! \class_exists( '\WP_Abilities_Registry' ) ) { + return false; } - return $output; - } - - /** - * The output schema for get-site-score. - * - * @return array - */ - private function get_site_score_schema() { - return [ - 'type' => 'object', - 'properties' => [ - 'score' => [ - 'type' => 'integer', - 'description' => \__( 'The website activity score, from 0 to 100.', 'progress-planner' ), - ], - 'checklist' => [ - 'type' => 'object', - 'description' => \__( 'The weekly maintenance checklist.', 'progress-planner' ), - 'properties' => [ - 'published_content' => [ - 'type' => 'boolean', - 'description' => \__( 'Whether content was published in the last seven days.', 'progress-planner' ), - ], - 'updated_content' => [ - 'type' => 'boolean', - 'description' => \__( 'Whether content was updated in the last seven days.', 'progress-planner' ), - ], - 'no_pending_updates' => [ - 'type' => 'boolean', - 'description' => \__( 'Whether the site has no pending updates.', 'progress-planner' ), - ], - ], - ], - 'pending_updates' => [ - 'type' => 'integer', - 'description' => \__( 'The number of pending core, plugin and theme updates.', 'progress-planner' ), - ], - 'badges' => [ - 'type' => 'array', - 'description' => \__( 'The content and maintenance badges, with progress towards each.', 'progress-planner' ), - 'items' => [ - 'type' => 'object', - 'properties' => [ - 'id' => [ 'type' => 'string' ], - 'name' => [ 'type' => 'string' ], - 'progress' => [ - 'type' => 'integer', - 'description' => \__( 'Progress towards the badge, as a percentage.', 'progress-planner' ), - ], - 'complete' => [ 'type' => 'boolean' ], - ], - ], - ], - 'latest_badge' => [ - 'type' => [ 'object', 'null' ], - 'description' => \__( 'The most recently completed badge, or null if none has been completed.', 'progress-planner' ), - 'properties' => [ - 'id' => [ 'type' => 'string' ], - 'name' => [ 'type' => 'string' ], - ], - ], - 'monthly_scores' => [ - 'type' => 'array', - 'description' => \__( 'The score for each of the last six months, oldest first.', 'progress-planner' ), - 'items' => [ - 'type' => 'object', - 'properties' => [ - 'month' => [ 'type' => 'string' ], - 'score' => [ 'type' => 'integer' ], - ], - ], - ], - ], - ]; - } - - /** - * The output schema for complete-recommendation. - * - * @return array - */ - private function get_complete_recommendation_schema() { - return [ - 'type' => 'object', - 'properties' => [ - 'applied' => [ - 'type' => 'boolean', - 'description' => \__( 'Whether a setting was changed.', 'progress-planner' ), - ], - 'status' => [ - 'type' => 'string', - 'description' => \__( 'What happened: "completed" when the recommendation is now satisfied, "applied_not_yet_complete" when the setting changed but the task is not satisfied, "manual" when it needs a person, "nothing_to_do" when no automatic recommendation was pending.', 'progress-planner' ), - 'enum' => [ 'completed', 'applied_not_yet_complete', 'manual', 'nothing_to_do' ], - ], - 'message' => [ - 'type' => 'string', - 'description' => \__( 'A sentence describing the outcome.', 'progress-planner' ), - ], - 'task' => [ - 'type' => [ 'object', 'null' ], - 'description' => \__( 'The recommendation that was acted on, if any.', 'progress-planner' ), - ], - 'admin_url' => [ - 'type' => 'string', - 'description' => \__( 'Where a person can handle this recommendation themselves.', 'progress-planner' ), - ], - ], - ]; - } + $registry = \WP_Abilities_Registry::get_instance(); - /** - * The output schema for list-recommendations. - * - * @return array - */ - private function get_recommendations_schema() { - return [ - 'type' => 'object', - 'properties' => [ - 'recommendations' => [ - 'type' => 'array', - 'items' => [ - 'type' => 'object', - 'properties' => [ - 'id' => [ - 'type' => 'string', - 'description' => \__( 'The task ID.', 'progress-planner' ), - ], - 'title' => [ - 'type' => 'string', - 'description' => \__( 'What the task asks the user to do.', 'progress-planner' ), - ], - 'description' => [ - 'type' => 'string', - 'description' => \__( 'Why the task matters.', 'progress-planner' ), - ], - 'provider_id' => [ - 'type' => 'string', - 'description' => \__( 'The ID of the provider that raised this task.', 'progress-planner' ), - ], - 'url' => [ - 'type' => 'string', - 'description' => \__( 'The admin URL where the task can be completed.', 'progress-planner' ), - ], - 'points' => [ - 'type' => 'integer', - 'description' => \__( 'Points awarded for completing the task.', 'progress-planner' ), - ], - 'fixable' => [ - 'type' => 'boolean', - 'description' => \__( 'Whether complete-recommendation can apply this one.', 'progress-planner' ), - ], - 'needs_value' => [ - 'type' => 'boolean', - 'description' => \__( 'Whether applying it requires a value from the caller, such as the tagline text.', 'progress-planner' ), - ], - ], - ], - ], - 'count' => [ - 'type' => 'integer', - 'description' => \__( 'The number of recommendations returned.', 'progress-planner' ), - ], - ], - ]; + return $registry && $registry->is_registered( $name ); } } diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index 53aa560b0..0bfbfa26e 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -1,6 +1,6 @@ $input The ability input. + * + * @return array|\WP_Error + */ + public function complete( $input = [] ) { + $provider_id = isset( $input['provider_id'] ) ? (string) $input['provider_id'] : ''; + $value = isset( $input['value'] ) ? (string) $input['value'] : null; + + if ( '' === $provider_id ) { + $provider_id = $this->get_next_fixable_provider_id(); + + if ( '' === $provider_id ) { + return [ + 'applied' => false, + 'status' => 'nothing_to_do', + 'message' => \__( 'There is no pending recommendation that can be applied automatically.', 'progress-planner' ), + 'task' => null, + 'admin_url' => '', + ]; + } + } + + $task = $this->find_pending_task( $provider_id ); + + if ( null === $task ) { + return new \WP_Error( + 'progress_planner_no_such_recommendation', + \__( 'There is no pending recommendation for that provider.', 'progress-planner' ), + [ 'status' => 404 ] + ); + } + + $provider = $this->get_provider( $provider_id ); + + // The provider's own capability check, the same one the popover runs. + if ( ! $provider || ! $provider->capability_required() ) { + return new \WP_Error( + 'progress_planner_cannot_complete', + \__( 'You do not have permission to complete this recommendation.', 'progress-planner' ), + [ 'status' => 403 ] + ); + } + + // Anything outside the fixable list is reported, never half-applied. + if ( ! Recommendation_Fixes::has_fix( $provider_id ) ) { + return $this->result( + false, + 'manual', + \__( 'This recommendation needs a person: it involves content, a deletion, or a choice that should not be made automatically. Open the link to handle it.', 'progress-planner' ), + $task + ); + } + + $applied = Recommendation_Fixes::apply( $provider_id, $value ); + + if ( \is_wp_error( $applied ) ) { + return $applied; + } + + // Completion is observed, never asserted: the provider decides whether + // the site now satisfies the task. Saying otherwise would award points + // for work that did not happen. + $completed = $this->is_satisfied( $provider, $task ); + + return $this->result( + true, + $completed ? 'completed' : 'applied_not_yet_complete', + $completed + ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) + : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ), + $task + ); + } + + /** + * Build a complete() result. + * + * @param bool $applied Whether a setting changed. + * @param string $status The outcome status. + * @param string $message The human-readable outcome. + * @param \Progress_Planner\Suggested_Tasks\Task $task The task acted on. + * + * @return array + */ + private function result( $applied, $status, $message, $task ) { + return [ + 'applied' => $applied, + 'status' => $status, + 'message' => $message, + 'task' => $this->prepare( $task ), + 'admin_url' => (string) $task->url, + ]; + } + + /** + * Whether a provider reports its task as already satisfied. + * + * @param \Progress_Planner\Suggested_Tasks\Tasks_Interface $provider The provider. + * @param \Progress_Planner\Suggested_Tasks\Task $task The task. + * + * @return bool + */ + private function is_satisfied( $provider, $task ) { + return \method_exists( $provider, 'is_task_completed' ) + && (bool) $provider->is_task_completed( $task->get_task_id() ); + } + + /** + * Find the highest-priority pending recommendation that can be fixed. + * + * Tasks come back ordered by menu_order, which is the order the dashboard + * shows them in, so "next" means the same thing to an agent as to a person. + * + * @return string The provider ID, or an empty string when there is none. + */ + private function get_next_fixable_provider_id() { + $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( + [ + 'post_status' => 'publish', + 'posts_per_page' => -1, + ] + ); + + foreach ( $tasks as $task ) { + $provider_id = $task->get_provider_id(); + + // A fix needing a value cannot be chosen unattended: there is no + // correct tagline to invent on the site owner's behalf. + if ( ! Recommendation_Fixes::has_fix( $provider_id ) || Recommendation_Fixes::needs_value( $provider_id ) ) { + continue; + } + + $provider = $this->get_provider( $provider_id ); + + if ( ! $provider || ! $provider->capability_required() ) { + continue; + } + + // Task evaluation runs on admin_init, so a task fixed a moment ago is + // still 'publish' here. Skipping the already-satisfied ones stops a + // repeated run from picking the same task and reporting it as new work. + if ( $this->is_satisfied( $provider, $task ) ) { + continue; + } + + return $provider_id; + } + + return ''; + } + + /** + * Find a pending task for a provider. + * + * @param string $provider_id The provider ID. + * + * @return \Progress_Planner\Suggested_Tasks\Task|null + */ + private function find_pending_task( $provider_id ) { + $tasks = \progress_planner()->get_suggested_tasks_db()->get_tasks_by( + [ + 'post_status' => 'publish', + 'provider' => $provider_id, + 'posts_per_page' => 1, + ] + ); + + return $tasks ? $tasks[0] : null; + } + /** * Get a task provider. * @@ -108,6 +281,10 @@ private function prepare( $task ) { // User-created tasks carry no points of their own; the provider is // the reliable source, with the stored value preferred when set. 'points' => (int) ( $task->points ?? $provider->get_points() ), + // Lets a caller plan a run without discovering by trial which + // recommendations it is allowed to apply. + 'fixable' => Recommendation_Fixes::has_fix( $provider_id ), + 'needs_value' => Recommendation_Fixes::needs_value( $provider_id ), ]; } } diff --git a/classes/abilities/class-schemas.php b/classes/abilities/class-schemas.php index de3c90974..d534ec944 100644 --- a/classes/abilities/class-schemas.php +++ b/classes/abilities/class-schemas.php @@ -59,6 +59,27 @@ public static function list_recommendations_input() { ]; } + /** + * The input schema for complete-recommendation. + * + * @return array + */ + public static function complete_recommendation_input() { + return [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => [ + 'provider_id' => [ + 'type' => 'string', + 'description' => \__( 'The provider ID of the recommendation to apply, for example "core-blogdescription". Omit to apply the highest-priority recommendation that can be applied automatically.', 'progress-planner' ), + ], + 'value' => [ + 'type' => 'string', + 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", or a date format string. Recommendations with only one correct outcome ignore this.', 'progress-planner' ), + ], + ], + ]; + } /** * The output schema for get-site-score. @@ -188,6 +209,48 @@ public static function recommendation() { 'type' => 'integer', 'description' => \__( 'Points awarded for completing the task.', 'progress-planner' ), ], + 'fixable' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether complete-recommendation can apply this one.', 'progress-planner' ), + ], + 'needs_value' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether applying it requires a value from the caller, such as the tagline text.', 'progress-planner' ), + ], + ], + ]; + } + + /** + * The output schema for complete-recommendation. + * + * @return array + */ + public static function complete_recommendation() { + return [ + 'type' => 'object', + 'properties' => [ + 'applied' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether a setting was changed.', 'progress-planner' ), + ], + 'status' => [ + 'type' => 'string', + 'description' => \__( 'What happened: "completed" when the recommendation is now satisfied, "applied_not_yet_complete" when the setting changed but the task is not satisfied, "manual" when it needs a person, "nothing_to_do" when no automatic recommendation was pending.', 'progress-planner' ), + 'enum' => [ 'completed', 'applied_not_yet_complete', 'manual', 'nothing_to_do' ], + ], + 'message' => [ + 'type' => 'string', + 'description' => \__( 'A sentence describing the outcome.', 'progress-planner' ), + ], + 'task' => [ + 'type' => [ 'object', 'null' ], + 'description' => \__( 'The recommendation that was acted on, if any.', 'progress-planner' ), + ], + 'admin_url' => [ + 'type' => 'string', + 'description' => \__( 'Where a person can handle this recommendation themselves.', 'progress-planner' ), + ], ], ]; } diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index d5a1947a8..7e939eb7b 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -28,6 +28,20 @@ class Abilities_Test extends \WP_UnitTestCase { */ private $abilities; + /** + * Site score reader. + * + * @var \Progress_Planner\Abilities\Site_Score + */ + private $site_score; + + /** + * Recommendations reader and applier. + * + * @var \Progress_Planner\Abilities\Recommendations + */ + private $recommendations; + /** * Set up test. * @@ -39,23 +53,26 @@ public function setUp(): void { // The constructor hooks the registration actions, so building a fresh // instance per test would stack a listener each time and re-register on // the next fire. The plugin's own instance is used instead. - $this->abilities = \progress_planner()->get_abilities__abilities(); + $this->abilities = \progress_planner()->get_abilities__abilities(); + $this->site_score = new \Progress_Planner\Abilities\Site_Score(); + $this->recommendations = new \Progress_Planner\Abilities\Recommendations(); } /** - * Invoke a private or protected method on the instance. + * Invoke a private or protected method on an object. * - * @param string $name The method name. - * @param array $args The arguments. + * @param object $object The object. + * @param string $name The method name. + * @param array $args The arguments. * * @return mixed */ - private function invoke( $name, array $args = [] ) { - $reflection = new \ReflectionClass( $this->abilities ); + private function invoke_on( $object, $name, array $args = [] ) { + $reflection = new \ReflectionClass( $object ); $method = $reflection->getMethod( $name ); $method->setAccessible( true ); - return $method->invokeArgs( $this->abilities, $args ); + return $method->invokeArgs( $object, $args ); } /** @@ -102,7 +119,7 @@ public function test_can_read_denies_logged_out_user() { public function test_get_site_score_returns_documented_shape() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $score = $this->abilities->get_site_score(); + $score = $this->site_score->get(); foreach ( [ 'score', 'checklist', 'pending_updates', 'badges', 'latest_badge', 'monthly_scores' ] as $key ) { $this->assertArrayHasKey( $key, $score ); @@ -122,7 +139,7 @@ public function test_get_site_score_returns_documented_shape() { public function test_get_site_score_is_within_range() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $score = $this->abilities->get_site_score(); + $score = $this->site_score->get(); $this->assertGreaterThanOrEqual( 0, $score['score'] ); $this->assertLessThanOrEqual( 100, $score['score'] ); @@ -140,7 +157,7 @@ public function test_get_site_score_is_within_range() { public function test_get_site_score_omits_telemetry_fields() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $score = $this->abilities->get_site_score(); + $score = $this->site_score->get(); foreach ( [ 'plugins', 'website', 'branding_id', 'plugin_url' ] as $key ) { $this->assertArrayNotHasKey( $key, $score ); @@ -153,7 +170,7 @@ public function test_get_site_score_omits_telemetry_fields() { * @return void */ public function test_get_checklist_uses_stable_keys() { - $checklist = $this->invoke( 'get_checklist' ); + $checklist = $this->site_score->get_checklist(); $this->assertSame( [ 'published_content', 'updated_content', 'no_pending_updates' ], @@ -187,7 +204,7 @@ public function test_get_checklist_reflects_pending_updates() { \add_filter( 'pre_site_transient_update_plugins', $no_updates ); \add_filter( 'pre_site_transient_update_themes', $no_updates ); - $checklist = $this->invoke( 'get_checklist' ); + $checklist = $this->site_score->get_checklist(); \remove_filter( 'pre_site_transient_update_core', $no_updates ); \remove_filter( 'pre_site_transient_update_plugins', $no_updates ); @@ -202,9 +219,9 @@ public function test_get_checklist_reflects_pending_updates() { * @return void */ public function test_status_maps_to_post_status() { - $this->assertSame( 'publish', $this->invoke( 'get_post_status_for', [ 'pending' ] ) ); - $this->assertSame( 'trash', $this->invoke( 'get_post_status_for', [ 'completed' ] ) ); - $this->assertSame( 'future', $this->invoke( 'get_post_status_for', [ 'snoozed' ] ) ); + $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'pending' ] ) ); + $this->assertSame( 'trash', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'completed' ] ) ); + $this->assertSame( 'future', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'snoozed' ] ) ); } /** @@ -213,7 +230,7 @@ public function test_status_maps_to_post_status() { * @return void */ public function test_unknown_status_falls_back_to_pending() { - $this->assertSame( 'publish', $this->invoke( 'get_post_status_for', [ 'nonsense' ] ) ); + $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'nonsense' ] ) ); } /** @@ -224,7 +241,7 @@ public function test_unknown_status_falls_back_to_pending() { public function test_list_recommendations_returns_documented_shape() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->abilities->list_recommendations( [ 'limit' => 5 ] ); + $result = $this->recommendations->list( [ 'limit' => 5 ] ); $this->assertArrayHasKey( 'recommendations', $result ); $this->assertArrayHasKey( 'count', $result ); @@ -250,7 +267,7 @@ public function test_list_recommendations_items_have_documented_fields() { ] ); - $result = $this->abilities->list_recommendations( [ 'limit' => 5 ] ); + $result = $this->recommendations->list( [ 'limit' => 5 ] ); $this->assertNotEmpty( $result['recommendations'], 'Expected at least one recommendation to inspect.' ); @@ -274,7 +291,7 @@ public function test_list_recommendations_items_have_documented_fields() { public function test_list_recommendations_honours_limit() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->abilities->list_recommendations( [ 'limit' => 1 ] ); + $result = $this->recommendations->list( [ 'limit' => 1 ] ); $this->assertLessThanOrEqual( 1, $result['count'] ); } @@ -287,7 +304,7 @@ public function test_list_recommendations_honours_limit() { public function test_list_recommendations_filters_by_provider() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->abilities->list_recommendations( [ 'provider' => 'provider-that-does-not-exist' ] ); + $result = $this->recommendations->list( [ 'provider' => 'provider-that-does-not-exist' ] ); $this->assertSame( 0, $result['count'] ); } @@ -303,7 +320,7 @@ public function test_list_recommendations_filters_by_provider() { public function test_list_recommendations_hides_tasks_from_subscribers() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); - $result = $this->abilities->list_recommendations( [] ); + $result = $this->recommendations->list( [] ); $this->assertSame( 0, $result['count'] ); } @@ -314,7 +331,7 @@ public function test_list_recommendations_hides_tasks_from_subscribers() { * @return void */ public function test_get_badges_returns_documented_shape() { - $badges = $this->invoke( 'get_badges' ); + $badges = $this->site_score->get_badges(); $this->assertIsArray( $badges ); @@ -337,7 +354,7 @@ public function test_get_badges_returns_documented_shape() { * @return void */ public function test_get_monthly_scores_shape() { - $scores = $this->invoke( 'get_monthly_scores' ); + $scores = $this->site_score->get_monthly_scores(); $this->assertIsArray( $scores ); @@ -496,7 +513,7 @@ public function test_complete_recommendation_applies_the_setting() { \update_option( 'blog_public', '0' ); $this->seed_task( 'search-engine-visibility' ); - $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'search-engine-visibility' ] ); + $result = $this->recommendations->complete( [ 'provider_id' => 'search-engine-visibility' ] ); $this->assertTrue( $result['applied'] ); $this->assertSame( 'completed', $result['status'] ); @@ -512,7 +529,7 @@ public function test_complete_recommendation_refuses_manual_tasks() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); $this->seed_task( 'remove-inactive-plugins' ); - $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'remove-inactive-plugins' ] ); + $result = $this->recommendations->complete( [ 'provider_id' => 'remove-inactive-plugins' ] ); $this->assertFalse( $result['applied'] ); $this->assertSame( 'manual', $result['status'] ); @@ -528,7 +545,7 @@ public function test_complete_recommendation_validates_timezone() { $before = \get_option( 'timezone_string' ); $this->seed_task( 'select-timezone' ); - $result = $this->abilities->complete_recommendation( + $result = $this->recommendations->complete( [ 'provider_id' => 'select-timezone', 'value' => 'Mars/Olympus', @@ -549,7 +566,7 @@ public function test_complete_recommendation_requires_a_value() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); $this->seed_task( 'core-blogdescription' ); - $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'core-blogdescription' ] ); + $result = $this->recommendations->complete( [ 'provider_id' => 'core-blogdescription' ] ); $this->assertWPError( $result ); $this->assertSame( 'progress_planner_missing_value', $result->get_error_code() ); @@ -563,7 +580,7 @@ public function test_complete_recommendation_requires_a_value() { public function test_complete_recommendation_rejects_unknown_provider() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->abilities->complete_recommendation( [ 'provider_id' => 'no-such-provider' ] ); + $result = $this->recommendations->complete( [ 'provider_id' => 'no-such-provider' ] ); $this->assertWPError( $result ); $this->assertSame( 'progress_planner_no_such_recommendation', $result->get_error_code() ); @@ -577,7 +594,7 @@ public function test_complete_recommendation_rejects_unknown_provider() { public function test_next_mode_reports_nothing_to_do() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $result = $this->abilities->complete_recommendation( [] ); + $result = $this->recommendations->complete( [] ); $this->assertContains( $result['status'], [ 'nothing_to_do', 'completed', 'applied_not_yet_complete' ] ); } @@ -593,7 +610,7 @@ public function test_next_mode_skips_fixes_needing_a_value() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); $this->seed_task( 'core-blogdescription' ); - $result = $this->abilities->complete_recommendation( [] ); + $result = $this->recommendations->complete( [] ); if ( null !== $result['task'] ) { $this->assertNotSame( 'core-blogdescription', $result['task']['provider_id'] ); @@ -611,7 +628,7 @@ public function test_list_recommendations_flags_fixable_items() { \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); $this->seed_task( 'search-engine-visibility' ); - $result = $this->abilities->list_recommendations( [ 'limit' => 100 ] ); + $result = $this->recommendations->list( [ 'limit' => 100 ] ); $found = false; foreach ( $result['recommendations'] as $recommendation ) { From c12782d8851841f3c53e2abe0980d531708cdce2 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Tue, 22 Sep 2026 14:01:24 +0200 Subject: [PATCH 10/17] Fix code style: array-arrow alignment and reserved variable name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runs PHPCS at default severity, which surfaces two things the pre-commit hook (severity 6) hides: - 16 array double-arrow alignment warnings in class-recommendation-fixes.php (auto-fixed with phpcbf). - `$object` used as a variable name in the abilities test — reserved in PHP 8; renamed to `$instance`. No behaviour change. Full suite and PHPStan still pass. Co-Authored-By: Claude Opus 5 (1M context) --- .../abilities/class-recommendation-fixes.php | 32 +++++++++---------- tests/phpunit/test-class-abilities.php | 8 ++--- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php index ea5bd3489..0d4bcc8db 100644 --- a/classes/abilities/class-recommendation-fixes.php +++ b/classes/abilities/class-recommendation-fixes.php @@ -31,37 +31,37 @@ class Recommendation_Fixes { * @var array> */ private const FIXES = [ - 'core-blogdescription' => [ + 'core-blogdescription' => [ 'option' => 'blogdescription', 'input' => 'value', 'type' => 'string', 'summary' => 'Set the site tagline.', ], - 'select-timezone' => [ + 'select-timezone' => [ 'option' => 'timezone_string', 'input' => 'value', 'type' => 'timezone', 'summary' => 'Set the site timezone.', ], - 'set-date-format' => [ + 'set-date-format' => [ 'option' => 'date_format', 'input' => 'value', 'type' => 'string', 'summary' => 'Set the date format.', ], - 'search-engine-visibility' => [ + 'search-engine-visibility' => [ 'option' => 'blog_public', 'value' => '1', 'type' => 'string', 'summary' => 'Allow search engines to index the site.', ], - 'disable-comments' => [ + 'disable-comments' => [ 'option' => 'default_comment_status', 'value' => 'closed', 'type' => 'string', 'summary' => 'Close comments on new content by default.', ], - 'disable-comment-pagination' => [ + 'disable-comment-pagination' => [ 'option' => 'page_comments', 'value' => '', 'type' => 'string', @@ -79,37 +79,37 @@ class Recommendation_Fixes { * field. That affordance is for a person; the underlying setting is a * plain boolean, so an agent can set it directly. */ - 'yoast-author-archive' => [ + 'yoast-author-archive' => [ 'seo' => 'yoast', 'setting' => 'disable-author', 'value' => true, 'summary' => 'Disable author archives in Yoast SEO.', ], - 'yoast-date-archive' => [ + 'yoast-date-archive' => [ 'seo' => 'yoast', 'setting' => 'disable-date', 'value' => true, 'summary' => 'Disable date archives in Yoast SEO.', ], - 'yoast-format-archive' => [ + 'yoast-format-archive' => [ 'seo' => 'yoast', 'setting' => 'disable-post_format', 'value' => true, 'summary' => 'Disable post-format archives in Yoast SEO.', ], - 'yoast-media-pages' => [ + 'yoast-media-pages' => [ 'seo' => 'yoast', 'setting' => 'disable-attachment', 'value' => true, 'summary' => 'Redirect attachment pages in Yoast SEO.', ], - 'yoast-crawl-settings-emoji-scripts' => [ + 'yoast-crawl-settings-emoji-scripts' => [ 'seo' => 'yoast', 'setting' => 'remove_emoji_scripts', 'value' => true, 'summary' => 'Remove emoji scripts via Yoast SEO.', ], - 'yoast-crawl-settings-feed-authors' => [ + 'yoast-crawl-settings-feed-authors' => [ 'seo' => 'yoast', 'setting' => 'remove_feed_authors', 'value' => true, @@ -127,28 +127,28 @@ class Recommendation_Fixes { * on the live options object, matching what each provider's own submit * handler does. */ - 'aioseo-author-archive' => [ + 'aioseo-author-archive' => [ 'seo' => 'aioseo', 'root' => 'options', 'path' => [ 'searchAppearance', 'archives', 'author', 'show' ], 'value' => false, 'summary' => 'Noindex author archives in All in One SEO.', ], - 'aioseo-date-archive' => [ + 'aioseo-date-archive' => [ 'seo' => 'aioseo', 'root' => 'options', 'path' => [ 'searchAppearance', 'archives', 'date', 'show' ], 'value' => false, 'summary' => 'Noindex date archives in All in One SEO.', ], - 'aioseo-crawl-settings-feed-authors' => [ + 'aioseo-crawl-settings-feed-authors' => [ 'seo' => 'aioseo', 'root' => 'options', 'path' => [ 'searchAppearance', 'advanced', 'crawlCleanup', 'feeds', 'authors' ], 'value' => false, 'summary' => 'Disable author feeds in All in One SEO.', ], - 'aioseo-media-pages' => [ + 'aioseo-media-pages' => [ 'seo' => 'aioseo', // Attachment redirection lives under dynamicOptions, not options, and // takes the destination as a string rather than a boolean. diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 7e939eb7b..1fff245b8 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -61,18 +61,18 @@ public function setUp(): void { /** * Invoke a private or protected method on an object. * - * @param object $object The object. + * @param object $instance The object. * @param string $name The method name. * @param array $args The arguments. * * @return mixed */ - private function invoke_on( $object, $name, array $args = [] ) { - $reflection = new \ReflectionClass( $object ); + private function invoke_on( $instance, $name, array $args = [] ) { + $reflection = new \ReflectionClass( $instance ); $method = $reflection->getMethod( $name ); $method->setAccessible( true ); - return $method->invokeArgs( $object, $args ); + return $method->invokeArgs( $instance, $args ); } /** From e78bffee315087acc11c7424680ce7f59ba847d5 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 14:06:01 +0200 Subject: [PATCH 11/17] Add a short confirmation code to the test email Email delivery is the one check a site cannot verify for itself: WordPress knows wp_mail() returned true, not that anything arrived. The existing token solves that by living only in the delivered message, so presenting it back proves delivery. The token is delivered as the href of a "Click here" link, which works for a person who clicks and not for a person who wants to relay it. Reading it off the screen is impossible; copying it means right-clicking and pasting a 128-character URL. That matters now this question gets asked through an AI assistant rather than the dashboard. An assistant with mailbox access can read the token and needs nothing from the user. An assistant without one has to ask, and "tell it the code from the email" only works if the code is short enough to say out loud. So the email now carries both: the link, unchanged, and a four-character code shown as text. The code is deliberately weaker than the token, and the docblock on generate_task_confirmation_code() records why that is acceptable -- single use, 24-hour expiry, scoped to one user and one task, manage_options required, and rate limiting around it. Anyone who could satisfy all of that can complete the task from wp-admin anyway. The alphabet omits characters that are misread when spoken or retyped (0/O, 1/I/L, 5/S, 8/B), and comparison ignores case and surrounding whitespace, because a code that survives being read aloud is the entire point. A test asserts the alphabet holds across repeated generation. The email body was built in two places; it is now built once. Co-Authored-By: Claude Opus 5 (1M context) --- classes/class-suggested-tasks.php | 109 ++++++++++++++++++ .../providers/class-email-sending.php | 35 +++++- tests/phpunit/test-class-suggested-tasks.php | 87 ++++++++++++++ 3 files changed, 225 insertions(+), 6 deletions(-) diff --git a/classes/class-suggested-tasks.php b/classes/class-suggested-tasks.php index 70542077b..cd1791c97 100644 --- a/classes/class-suggested-tasks.php +++ b/classes/class-suggested-tasks.php @@ -244,12 +244,121 @@ public function maybe_complete_task() { } } + /** + * The alphabet used for confirmation codes. + * + * Excludes characters that are easily confused when read aloud or retyped: + * 0/O, 1/I/L, 5/S, 8/B. A code is only useful if a person relays it + * correctly on the first try. + * + * @var string + */ + const CONFIRMATION_CODE_ALPHABET = '234679ACDEFGHJKMNPQRTUVWXYZ'; + + /** + * The length of a confirmation code. + * + * @var int + */ + const CONFIRMATION_CODE_LENGTH = 4; + + /** + * Generate a short confirmation code for a task. + * + * WHY THIS EXISTS, alongside the 32-character token below: + * + * Both prove the same thing -- that the email actually arrived, because + * the value only exists inside the delivered message. They differ in who + * can realistically relay them. + * + * The token is delivered as the href of a "Click here" link. A person + * cannot read it off the screen: they would have to right-click, copy the + * link address, and paste a 128-character URL. That is fine for clicking + * and useless for telling someone. + * + * This matters because email delivery is the one check a site cannot + * verify for itself, and it is increasingly answered through an AI + * assistant rather than the dashboard. An assistant with mailbox access + * reads the token and needs nothing from the user. An assistant without it + * has to ask, and "tell it the code from the email" only works if the code + * is short enough to say out loud. + * + * The code is deliberately weaker than the token: four characters from a + * 27-character alphabet is about 19 bits. What makes that acceptable is + * everything around it -- the code is single-use, expires in 24 hours, is + * scoped to one user and one task, is only accepted from a caller who + * already holds manage_options, and failed attempts are rate limited. An + * attacker who could satisfy all of that can complete the task by simply + * clicking a button in wp-admin, so the code is not the weak link. + * + * Codes are stored uppercase and compared case-insensitively, because a + * person retyping one should not have to think about it. + * + * @param string $task_id The task ID. + * @param int $user_id The user ID. + * + * @return string The generated confirmation code. + */ + public function generate_task_confirmation_code( $task_id, $user_id ) { + $alphabet = self::CONFIRMATION_CODE_ALPHABET; + $max = \strlen( $alphabet ) - 1; + $code = ''; + + for ( $i = 0; $i < self::CONFIRMATION_CODE_LENGTH; $i++ ) { + $code .= $alphabet[ \wp_rand( 0, $max ) ]; + } + + \set_transient( + 'prpl_confirm_' . $task_id . '_' . $user_id, + $code, + DAY_IN_SECONDS + ); + + return $code; + } + + /** + * Verify a task confirmation code. + * + * @param string $task_id The task ID. + * @param int $user_id The user ID. + * @param string $provided_code The code supplied by the caller. + * + * @return bool + */ + public function verify_task_confirmation_code( $task_id, $user_id, $provided_code ) { + $stored_code = \get_transient( 'prpl_confirm_' . $task_id . '_' . $user_id ); + + if ( ! $stored_code ) { + return false; // Expired, already used, or never issued. + } + + // hash_equals to keep the comparison constant-time; strtoupper because + // the code is meant to be retyped by a person. + return \hash_equals( (string) $stored_code, \strtoupper( \trim( $provided_code ) ) ); + } + + /** + * Delete a task confirmation code after use. + * + * @param string $task_id The task ID. + * @param int $user_id The user ID. + * + * @return bool True if deleted, false otherwise. + */ + public function delete_task_confirmation_code( $task_id, $user_id ) { + return \delete_transient( 'prpl_confirm_' . $task_id . '_' . $user_id ); + } + /** * Generate a secure token for task completion via email link. * * This token prevents CSRF attacks by ensuring only legitimate email * links can mark tasks as complete. * + * See generate_task_confirmation_code() above for why a second, shorter + * value exists next to this one. + * * @param string $task_id The task ID. * @param int $user_id The user ID. * diff --git a/classes/suggested-tasks/providers/class-email-sending.php b/classes/suggested-tasks/providers/class-email-sending.php index 50f9e3064..a9ca3e86d 100644 --- a/classes/suggested-tasks/providers/class-email-sending.php +++ b/classes/suggested-tasks/providers/class-email-sending.php @@ -110,6 +110,34 @@ public function init() { // $this->email_content is not read anywhere else. } + /** + * Build the test email body. + * + * The body carries two ways to confirm the message arrived: a link, which + * carries the full token and is what a person clicks, and a short code, + * which is what a person can read out to an AI assistant that has no + * access to their mailbox. Both prove the same thing -- that the message + * was delivered -- because neither value exists anywhere the recipient + * could reach without receiving it. See + * Suggested_Tasks::generate_task_confirmation_code() for the reasoning. + * + * @param string $token The completion token. + * @param int $user_id The user the token and code belong to. + * + * @return string + */ + protected function get_email_content( $token, $user_id ) { + $code = \progress_planner()->get_suggested_tasks()->generate_task_confirmation_code( $this->get_task_id(), $user_id ); + + return \sprintf( + /* translators: %1$s: the admin URL, %2$s: the assistant's name, %3$s: a short confirmation code. */ + \__( 'You just used Progress Planner to verify if sending email works on your website.

The good news; it does! Click here to mark %2$s\'s Recommendation as completed.

Using an AI assistant? Give it this confirmation code instead: %3$s', 'progress-planner' ), + \admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() . '&token=' . $token ), + \esc_html( \progress_planner()->get_ui__branding()->get_ravi_name() ), + \esc_html( $code ) + ); + } + /** * Get the troubleshooting guide URL. * @@ -273,12 +301,7 @@ public function ajax_test_email_sending() { $user_id = \get_current_user_id(); $token = \progress_planner()->get_suggested_tasks()->generate_task_completion_token( $this->get_task_id(), $user_id ); - $email_content = \sprintf( - // translators: %1$s the admin URL. - \__( 'You just used Progress Planner to verify if sending email works on your website.

The good news; it does! Click here to mark %2$s\'s Recommendation as completed.', 'progress-planner' ), - \admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() . '&token=' . $token ), - \esc_html( \progress_planner()->get_ui__branding()->get_ravi_name() ) - ); + $email_content = $this->get_email_content( $token, $user_id ); $headers = [ 'Content-Type: text/html; charset=UTF-8' ]; diff --git a/tests/phpunit/test-class-suggested-tasks.php b/tests/phpunit/test-class-suggested-tasks.php index fec2a15d4..6be4baf68 100644 --- a/tests/phpunit/test-class-suggested-tasks.php +++ b/tests/phpunit/test-class-suggested-tasks.php @@ -102,4 +102,91 @@ public function test_task_cleanup() { \wp_cache_flush_group( \Progress_Planner\Suggested_Tasks_DB::GET_TASKS_CACHE_GROUP ); // Clear the cache. $this->assertEquals( \count( $tasks_to_keep ), \count( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( [ 'post_status' => 'publish' ] ) ) ); } + + /** + * Test that a confirmation code round-trips. + * + * @return void + */ + public function test_confirmation_code_verifies() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertSame( 4, \strlen( $code ) ); + $this->assertTrue( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, $code ) ); + } + + /** + * Test that a code is accepted however a person retypes it. + * + * A code only has value if it survives being read aloud and typed back, + * so case and surrounding whitespace must not matter. + * + * @return void + */ + public function test_confirmation_code_is_forgiving_about_formatting() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertTrue( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, \strtolower( $code ) ) ); + $this->assertTrue( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, ' ' . $code . ' ' ) ); + } + + /** + * Test that a wrong code is rejected. + * + * @return void + */ + public function test_confirmation_code_rejects_a_wrong_code() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, 'ZZZZ' ) ); + } + + /** + * Test that a code belongs to one user and one task. + * + * @return void + */ + public function test_confirmation_code_is_scoped() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 2, $code ) ); + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'other-task', 1, $code ) ); + } + + /** + * Test that a code cannot be reused once consumed. + * + * @return void + */ + public function test_confirmation_code_is_single_use() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $suggested_tasks->delete_task_confirmation_code( 'sending-email', 1 ); + + $this->assertFalse( $suggested_tasks->verify_task_confirmation_code( 'sending-email', 1, $code ) ); + } + + /** + * Test that codes avoid characters that are misread. + * + * @return void + */ + public function test_confirmation_code_avoids_ambiguous_characters() { + $suggested_tasks = \progress_planner()->get_suggested_tasks(); + + for ( $i = 0; $i < 40; $i++ ) { + $code = $suggested_tasks->generate_task_confirmation_code( 'sending-email', 1 ); + + $this->assertSame( + 0, + \preg_match( '/[01OILSB58]/', $code ), + "Code {$code} contains an easily confused character." + ); + } + } } From ff2fafee5068e19913c5fb9b30d522b982c2129d Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 14:15:53 +0200 Subject: [PATCH 12/17] Let an agent clear WordPress's placeholder content Adds hello-world and sample-page, bringing the applicable set to 19. These are the first entries that remove something rather than change a setting, and they are treated differently for it: - Annotated destructive. The annotation describes what the ability can do, not what a given call does, so complete-recommendation as a whole is now destructive and a client prompts before any of it runs. - Confirm-only, so next-mode skips them. A daily unattended run must never be the thing that deleted something; they can only be applied by naming the provider. - Trashed, not force-deleted. The dashboard's JavaScript passes force=true and removes the post outright. An agent should leave a way back, and the completion check passes either way because it looks for a published post. What makes them defensible at all is that the target is not ambiguous: the data collector resolves the specific post WordPress ships, by slug with a title fallback, and no ID is ever taken from the caller. The success message says the content was trashed and can be restored, rather than reusing the settings wording. The new tests create and trash posts, which writes to the activities table. That table is custom, so WP_UnitTestCase does not roll it back, and post IDs are reused across tests -- a leftover row was being seen by an unrelated test that asserts a fresh post has no activity. The class now clears the table in tearDown. Co-Authored-By: Claude Opus 5 (1M context) --- classes/abilities/class-abilities.php | 15 ++- .../abilities/class-recommendation-fixes.php | 106 +++++++++++++++++ classes/abilities/class-recommendations.php | 27 ++++- classes/abilities/class-schemas.php | 4 + tests/phpunit/test-class-abilities.php | 111 ++++++++++++++++++ 5 files changed, 253 insertions(+), 10 deletions(-) diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index 1e3298cb8..16af34762 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -157,6 +157,12 @@ public function register_abilities() { 'permission_callback' => [ $this, 'can_fix' ], 'execute_callback' => [ $this->recommendations, 'complete' ], 'readonly' => false, + // Most of what this applies is a settings change, but two + // recommendations trash WordPress's placeholder content. The + // annotation describes what the ability can do, not what a + // given call happens to do, so it is declared destructive and + // a client prompts before any of it runs. + 'destructive' => true, ] ) ); @@ -173,8 +179,9 @@ public function register_abilities() { * @return array */ private function ability_args( array $args ) { - $readonly = $args['readonly'] ?? true; - unset( $args['readonly'] ); + $readonly = $args['readonly'] ?? true; + $destructive = $args['destructive'] ?? false; + unset( $args['readonly'], $args['destructive'] ); return \array_merge( [ @@ -184,9 +191,7 @@ private function ability_args( array $args ) { 'show_in_rest' => true, 'annotations' => [ 'readonly' => $readonly, - // Nothing registered here deletes or overwrites content: - // the write ability changes settings from a fixed list. - 'destructive' => false, + 'destructive' => $destructive, 'idempotent' => true, ], ], diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php index 0d4bcc8db..255fef69b 100644 --- a/classes/abilities/class-recommendation-fixes.php +++ b/classes/abilities/class-recommendation-fixes.php @@ -148,6 +148,37 @@ class Recommendation_Fixes { 'value' => false, 'summary' => 'Disable author feeds in All in One SEO.', ], + + /* + * Deleting WordPress's own placeholder content. + * + * These are the only entries that remove something rather than change a + * setting, so they are the only ones annotated destructive. Two things + * make them defensible anyway: the target is not ambiguous -- the data + * collector resolves the specific post WordPress ships, by slug with a + * title fallback -- and the recommendation exists precisely because the + * site owner is being asked to delete it. + * + * They are trashed, not force-deleted. The dashboard's own JavaScript + * passes force=true and removes the post outright; an agent acting + * unattended should leave a way back, and the task's completion check + * passes either way because it looks for a published post. + * + * confirm_only keeps them out of next-mode, so a daily unattended run + * never deletes anything: they can only be applied by naming the + * provider explicitly. + */ + 'hello-world' => [ + 'delete' => 'post', + 'confirm_only' => true, + 'summary' => 'Move the default "Hello world!" post to the trash.', + ], + 'sample-page' => [ + 'delete' => 'page', + 'confirm_only' => true, + 'summary' => 'Move the default "Sample Page" to the trash.', + ], + 'aioseo-media-pages' => [ 'seo' => 'aioseo', // Attachment redirection lives under dynamicOptions, not options, and @@ -225,6 +256,10 @@ public static function apply( $provider_id, $value = null ) { ); } + if ( isset( $fix['delete'] ) ) { + return self::apply_deletion( $provider_id ); + } + if ( isset( $fix['seo'] ) ) { return self::apply_seo_setting( $fix ); } @@ -246,6 +281,77 @@ public static function apply( $provider_id, $value = null ) { return true; } + /** + * Move a placeholder post to the trash. + * + * The post ID comes from the provider's own data collector, which resolves + * WordPress's default content by slug. Nothing here takes an ID from the + * caller, so an ability argument cannot point this at arbitrary content. + * + * @param string $provider_id The provider ID. + * + * @return true|\WP_Error + */ + private static function apply_deletion( $provider_id ) { + $provider = \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( $provider_id ); + + if ( ! $provider || ! \method_exists( $provider, 'get_data_collector' ) ) { + return new \WP_Error( + 'progress_planner_no_target', + \__( 'That recommendation is not available on this site.', 'progress-planner' ) + ); + } + + $post_id = (int) $provider->get_data_collector()->collect(); + + if ( ! $post_id || ! \get_post( $post_id ) ) { + return new \WP_Error( + 'progress_planner_no_target', + \__( 'The default content this recommendation refers to is no longer there.', 'progress-planner' ) + ); + } + + if ( ! \wp_trash_post( $post_id ) ) { + return new \WP_Error( + 'progress_planner_delete_failed', + \__( 'The content could not be moved to the trash.', 'progress-planner' ) + ); + } + + return true; + } + + /** + * Whether applying a fix removes content rather than changing a setting. + * + * Drives the destructive annotation, which is what a client shows a person + * before calling. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function is_destructive( $provider_id ) { + $fix = self::get( $provider_id ); + + return null !== $fix && isset( $fix['delete'] ); + } + + /** + * Whether a fix may only be applied by naming its provider explicitly. + * + * Keeps anything that removes content out of an unattended run. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function is_confirm_only( $provider_id ) { + $fix = self::get( $provider_id ); + + return null !== $fix && ! empty( $fix['confirm_only'] ); + } + /** * Apply a setting owned by an SEO plugin. * diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index 0bfbfa26e..838985374 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -135,12 +135,23 @@ public function complete( $input = [] ) { // for work that did not happen. $completed = $this->is_satisfied( $provider, $task ); + // The wording distinguishes a settings change from a deletion: an agent + // relaying this to a person should not describe trashing a post as + // changing a setting. + if ( Recommendation_Fixes::is_destructive( $provider_id ) ) { + $message = $completed + ? \__( 'The content was moved to the trash and the recommendation is now satisfied. It can be restored from the trash if that was not intended.', 'progress-planner' ) + : \__( 'The content was moved to the trash, but the recommendation is not reported as satisfied yet.', 'progress-planner' ); + } else { + $message = $completed + ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) + : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ); + } + return $this->result( true, $completed ? 'completed' : 'applied_not_yet_complete', - $completed - ? \__( 'The setting was changed and the recommendation is now satisfied.', 'progress-planner' ) - : \__( 'The setting was changed, but the recommendation is not reported as satisfied yet.', 'progress-planner' ), + $message, $task ); } @@ -198,8 +209,13 @@ private function get_next_fixable_provider_id() { $provider_id = $task->get_provider_id(); // A fix needing a value cannot be chosen unattended: there is no - // correct tagline to invent on the site owner's behalf. - if ( ! Recommendation_Fixes::has_fix( $provider_id ) || Recommendation_Fixes::needs_value( $provider_id ) ) { + // correct tagline to invent on the site owner's behalf. Nor can one + // that removes content, however well scoped -- an unattended run + // should never be the thing that deleted something. + if ( ! Recommendation_Fixes::has_fix( $provider_id ) + || Recommendation_Fixes::needs_value( $provider_id ) + || Recommendation_Fixes::is_confirm_only( $provider_id ) + ) { continue; } @@ -285,6 +301,7 @@ private function prepare( $task ) { // recommendations it is allowed to apply. 'fixable' => Recommendation_Fixes::has_fix( $provider_id ), 'needs_value' => Recommendation_Fixes::needs_value( $provider_id ), + 'destructive' => Recommendation_Fixes::is_destructive( $provider_id ), ]; } } diff --git a/classes/abilities/class-schemas.php b/classes/abilities/class-schemas.php index d534ec944..e9ed0aeae 100644 --- a/classes/abilities/class-schemas.php +++ b/classes/abilities/class-schemas.php @@ -217,6 +217,10 @@ public static function recommendation() { 'type' => 'boolean', 'description' => \__( 'Whether applying it requires a value from the caller, such as the tagline text.', 'progress-planner' ), ], + 'destructive' => [ + 'type' => 'boolean', + 'description' => \__( 'Whether applying it removes content rather than changing a setting. These are only applied when named explicitly, never picked automatically.', 'progress-planner' ), + ], ], ]; } diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 1fff245b8..2482bcc19 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -58,6 +58,24 @@ public function setUp(): void { $this->recommendations = new \Progress_Planner\Abilities\Recommendations(); } + /** + * Tear down test. + * + * Activities live in a custom table that WP_UnitTestCase does not roll + * back, and post IDs are reused across tests. A row left here would be seen + * by a later test that happens to be handed the same ID and asserts it has + * no activity, so this class clears what it caused. + * + * @return void + */ + public function tearDown(): void { + global $wpdb; + + $wpdb->query( 'TRUNCATE TABLE ' . $wpdb->prefix . 'progress_planner_activities' ); // phpcs:ignore WordPress.DB + + parent::tearDown(); + } + /** * Invoke a private or protected method on an object. * @@ -663,4 +681,97 @@ public function test_seo_fix_requires_its_plugin() { $this->assertWPError( $result ); $this->assertSame( 'progress_planner_seo_plugin_inactive', $result->get_error_code() ); } + + /** + * Test that the placeholder deletions are marked destructive. + * + * @return void + */ + public function test_placeholder_deletions_are_destructive() { + $this->assertTrue( Recommendation_Fixes::is_destructive( 'hello-world' ) ); + $this->assertTrue( Recommendation_Fixes::is_destructive( 'sample-page' ) ); + $this->assertFalse( Recommendation_Fixes::is_destructive( 'disable-comments' ) ); + } + + /** + * Test that anything destructive can only be applied by name. + * + * An unattended run must never be the thing that deleted something. + * + * @return void + */ + public function test_destructive_fixes_are_confirm_only() { + $this->assertTrue( Recommendation_Fixes::is_confirm_only( 'hello-world' ) ); + $this->assertTrue( Recommendation_Fixes::is_confirm_only( 'sample-page' ) ); + $this->assertFalse( Recommendation_Fixes::is_confirm_only( 'disable-comments' ) ); + } + + /** + * Test that applying the hello-world fix trashes the post rather than + * deleting it outright. + * + * @return void + */ + public function test_hello_world_fix_trashes_the_post() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $post_id = self::factory()->post->create( + [ + 'post_title' => 'Hello world!', + 'post_name' => 'hello-world', + 'post_status' => 'publish', + ] + ); + + $this->seed_task( 'hello-world' ); + + $result = $this->recommendations->complete( [ 'provider_id' => 'hello-world' ] ); + + $this->assertTrue( $result['applied'] ); + $this->assertSame( 'trash', \get_post_status( $post_id ), 'The post should be recoverable from the trash.' ); + } + + /** + * Test that next mode never picks a destructive fix. + * + * @return void + */ + public function test_next_mode_never_deletes() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $post_id = self::factory()->post->create( + [ + 'post_title' => 'Hello world!', + 'post_name' => 'hello-world', + 'post_status' => 'publish', + ] + ); + + $this->seed_task( 'hello-world' ); + + $result = $this->recommendations->complete( [] ); + + if ( null !== $result['task'] ) { + $this->assertNotSame( 'hello-world', $result['task']['provider_id'] ); + } + + $this->assertSame( 'publish', \get_post_status( $post_id ), 'Next mode must not trash anything.' ); + } + + /** + * Test that a deletion reports missing content rather than failing oddly. + * + * @return void + */ + public function test_deletion_reports_missing_target() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $result = Recommendation_Fixes::apply( 'hello-world' ); + + if ( \is_wp_error( $result ) ) { + $this->assertSame( 'progress_planner_no_target', $result->get_error_code() ); + } else { + $this->assertTrue( $result ); + } + } } From 4b10ec52c0c86467e38123268205b9c2461c4f23 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 18 Sep 2026 16:21:28 +0200 Subject: [PATCH 13/17] Let a caller record which page serves the About, Contact or FAQ role Brings the applicable set to 22. These recommendations do not create a page. They ask whether the site has one, and record the answer plus which page it is. The interesting part is who decides. The plugin deliberately does not search for the page. Searching a real site for "about" returns everything whose content mentions the word: on the demo site that is eleven pages, including one called "Job opening", with the actual About page ranked ninth and titled "About Emilia" at /about-us/. Deciding which of those is the About page means weighing title, slug, hierarchy and navigation in whatever language the site is written in. A caller that can read the site's pages does that well; a title match in PHP would get it wrong quietly, and worse on non-English sites. So the division of labour is: the caller identifies the page, and this verifies the ID before writing. A missing ID, an ID that does not exist, a draft, or a post that is not a page are each a distinct error rather than a silent no-op, because "we recorded your About page" is worth being true. The page type comes from the table and never from the caller, and the allowed post types are derived from the hierarchical public ones rather than hardcoded to 'page', so a site serving these roles from a custom post type still works. They need a value, so next-mode skips them: there is no correct page to choose on the owner's behalf. One caveat is recorded in is_satisfied(). These providers do not override is_task_completed(), so satisfaction now falls back to should_add_task(). That can still read pre-write state within the same request, because page-type lookups are memoised in a static cache with no invalidation -- which is why the status distinguishes "applied" from "satisfied" instead of assuming they are the same. Co-Authored-By: Claude Opus 5 (1M context) --- .../abilities/class-recommendation-fixes.php | 116 ++++++++++++++++++ classes/abilities/class-recommendations.php | 21 +++- classes/abilities/class-schemas.php | 2 +- tests/phpunit/test-class-abilities.php | 90 ++++++++++++++ 4 files changed, 226 insertions(+), 3 deletions(-) diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php index 255fef69b..9c1826ffc 100644 --- a/classes/abilities/class-recommendation-fixes.php +++ b/classes/abilities/class-recommendation-fixes.php @@ -149,6 +149,39 @@ class Recommendation_Fixes { 'summary' => 'Disable author feeds in All in One SEO.', ], + /* + * Telling the plugin which existing page serves a given role. + * + * These do not create anything. The recommendation asks whether the site + * has an About page; this records the answer and, when the answer is + * yes, which page it is. + * + * The plugin deliberately does not try to find the page itself. Deciding + * that "About Emilia" at /about-us/ is the About page -- and that + * "Job opening" is not -- is a judgement about titles, slugs, navigation + * and content in whatever language the site is written in. A caller that + * can read the site's pages is far better at that than a title match + * would be, and a wrong guess made in PHP would be silent. + * + * So the division is: the caller identifies the page, and this verifies + * the ID names a real published page of an allowed type before writing. + */ + 'set-page-about' => [ + 'page_type' => 'about', + 'input' => 'value', + 'summary' => 'Record which existing page is the About page.', + ], + 'set-page-contact' => [ + 'page_type' => 'contact', + 'input' => 'value', + 'summary' => 'Record which existing page is the Contact page.', + ], + 'set-page-faq' => [ + 'page_type' => 'faq', + 'input' => 'value', + 'summary' => 'Record which existing page is the FAQ page.', + ], + /* * Deleting WordPress's own placeholder content. * @@ -256,6 +289,10 @@ public static function apply( $provider_id, $value = null ) { ); } + if ( isset( $fix['page_type'] ) ) { + return self::apply_page_type( $fix, $value ); + } + if ( isset( $fix['delete'] ) ) { return self::apply_deletion( $provider_id ); } @@ -281,6 +318,85 @@ public static function apply( $provider_id, $value = null ) { return true; } + /** + * Record which page serves a given role. + * + * The page type comes from the table; only the page ID comes from the + * caller, and it is checked before anything is written: it must name a post + * that exists, is published, and is of a post type the site treats as a + * page. An ID that does not pass is an error rather than a silent no-op, + * because "we recorded your About page" is worth being true. + * + * @param array $fix The fix definition. + * @param mixed $value The page ID supplied by the caller. + * + * @return true|\WP_Error + */ + private static function apply_page_type( array $fix, $value ) { + $page_id = \is_numeric( $value ) ? (int) $value : 0; + + if ( 1 > $page_id ) { + return new \WP_Error( + 'progress_planner_missing_page_id', + \__( 'This recommendation needs the ID of the page that serves this role.', 'progress-planner' ) + ); + } + + $page = \get_post( $page_id ); + + if ( ! $page ) { + return new \WP_Error( + 'progress_planner_no_such_page', + \__( 'There is no page with that ID.', 'progress-planner' ) + ); + } + + if ( 'publish' !== $page->post_status ) { + return new \WP_Error( + 'progress_planner_page_not_published', + \__( 'That page is not published, so it cannot serve this role yet.', 'progress-planner' ) + ); + } + + if ( ! \in_array( $page->post_type, self::page_post_types(), true ) ) { + return new \WP_Error( + 'progress_planner_not_a_page', + \__( 'That post is not a page.', 'progress-planner' ) + ); + } + + \progress_planner()->get_admin__page_settings()->set_page_values( + [ + (string) $fix['page_type'] => [ + 'id' => $page_id, + 'have_page' => 'yes', + ], + ] + ); + + return true; + } + + /** + * The post types that can serve a page role. + * + * Hierarchical public post types, which is what WordPress means by a page, + * rather than a hardcoded 'page' -- a site may serve these roles from a + * custom post type. + * + * @return array + */ + private static function page_post_types() { + $types = \get_post_types( + [ + 'public' => true, + 'hierarchical' => true, + ] + ); + + return \array_values( $types ); + } + /** * Move a placeholder post to the trash. * diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index 838985374..cd8a90570 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -185,8 +185,25 @@ private function result( $applied, $status, $message, $task ) { * @return bool */ private function is_satisfied( $provider, $task ) { - return \method_exists( $provider, 'is_task_completed' ) - && (bool) $provider->is_task_completed( $task->get_task_id() ); + if ( \method_exists( $provider, 'is_task_completed' ) + && (bool) $provider->is_task_completed( $task->get_task_id() ) + ) { + return true; + } + + // Some providers answer through should_add_task() instead: the task + // exists precisely while the condition is unmet, so "would not be added + // now" means satisfied. The set-page providers are the case in point -- + // they do not override is_task_completed() at all. + // + // This can still report false immediately after a write. Page-type + // lookups are memoised in a static cache with no invalidation, so within + // one request the check may read state from before the change. That is + // why the status distinguishes "applied" from "satisfied" rather than + // assuming the two are the same: the next request sees it correctly, and + // a caller is told what actually happened either way. + return \method_exists( $provider, 'should_add_task' ) + && false === (bool) $provider->should_add_task(); } /** diff --git a/classes/abilities/class-schemas.php b/classes/abilities/class-schemas.php index e9ed0aeae..d90809a8e 100644 --- a/classes/abilities/class-schemas.php +++ b/classes/abilities/class-schemas.php @@ -75,7 +75,7 @@ public static function complete_recommendation_input() { ], 'value' => [ 'type' => 'string', - 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", or a date format string. Recommendations with only one correct outcome ignore this.', 'progress-planner' ), + 'description' => \__( 'The value to set, for recommendations that need one: the tagline text, a timezone identifier such as "Europe/Amsterdam", a date format string, or -- for the recommendations that ask which page serves a role -- the ID of an existing published page. Recommendations with only one correct outcome ignore this. Check needs_value on a recommendation to see whether one is required.', 'progress-planner' ), ], ], ]; diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 2482bcc19..59f68e78e 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -774,4 +774,94 @@ public function test_deletion_reports_missing_target() { $this->assertTrue( $result ); } } + + /** + * Test that the page-role recommendations need a page ID. + * + * @return void + */ + public function test_page_role_fixes_need_a_value() { + foreach ( [ 'set-page-about', 'set-page-contact', 'set-page-faq' ] as $provider_id ) { + $this->assertTrue( Recommendation_Fixes::has_fix( $provider_id ) ); + $this->assertTrue( Recommendation_Fixes::needs_value( $provider_id ) ); + $this->assertFalse( Recommendation_Fixes::is_destructive( $provider_id ) ); + } + } + + /** + * Test that a valid page is recorded as serving the role. + * + * @return void + */ + public function test_page_role_records_the_page() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $page_id = self::factory()->post->create( + [ + 'post_title' => 'About Us', + 'post_type' => 'page', + 'post_status' => 'publish', + ] + ); + + $this->assertTrue( Recommendation_Fixes::apply( 'set-page-about', (string) $page_id ) ); + + $slugs = \wp_get_object_terms( $page_id, 'progress_planner_page_types', [ 'fields' => 'slugs' ] ); + + $this->assertContains( 'about', (array) $slugs ); + } + + /** + * Test that an unusable page ID is refused rather than recorded. + * + * "We recorded your About page" is worth being true, so each of these is an + * error rather than a silent no-op. + * + * @return void + */ + public function test_page_role_refuses_unusable_ids() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $cases = [ + 'progress_planner_missing_page_id' => null, + 'progress_planner_no_such_page' => '99999999', + 'progress_planner_page_not_published' => (string) self::factory()->post->create( + [ + 'post_type' => 'page', + 'post_status' => 'draft', + ] + ), + 'progress_planner_not_a_page' => (string) self::factory()->post->create( + [ + 'post_type' => 'post', + 'post_status' => 'publish', + ] + ), + ]; + + foreach ( $cases as $expected_code => $value ) { + $result = Recommendation_Fixes::apply( 'set-page-about', $value ); + + $this->assertWPError( $result ); + $this->assertSame( $expected_code, $result->get_error_code() ); + } + } + + /** + * Test that next mode never picks a page-role fix. + * + * There is no correct page to choose on the owner's behalf. + * + * @return void + */ + public function test_next_mode_skips_page_role_fixes() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->seed_task( 'set-page-about' ); + + $result = $this->recommendations->complete( [] ); + + $picked = $result['task']['provider_id'] ?? null; + + $this->assertNotSame( 'set-page-about', $picked ); + } } From c0077219b1d8788950ec2a2e6fe95faa8001eed3 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Tue, 29 Sep 2026 13:35:07 +0200 Subject: [PATCH 14/17] Expose the abilities to MCP clients The MCP adapter's default server lists and runs only abilities whose meta sets mcp.public. Without it, discovery returns none of ours and executing one fails with "not exposed via MCP", so an agent connected through AI Connector could not see or call any of them. Found by driving wp.test through that server as an agent. The flag is set once in ability_args(), so every ability gets it, and a test asserts it for each registered ability. Co-Authored-By: Claude Opus 5.5 --- classes/abilities/class-abilities.php | 3 +++ tests/phpunit/test-class-abilities.php | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/classes/abilities/class-abilities.php b/classes/abilities/class-abilities.php index 16af34762..9931f31e9 100644 --- a/classes/abilities/class-abilities.php +++ b/classes/abilities/class-abilities.php @@ -189,6 +189,9 @@ private function ability_args( array $args ) { 'permission_callback' => [ $this, 'can_read' ], 'meta' => [ 'show_in_rest' => true, + // The MCP adapter's default server lists and runs only + // abilities that opt in here; without it no agent sees them. + 'mcp' => [ 'public' => true ], 'annotations' => [ 'readonly' => $readonly, 'destructive' => $destructive, diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 59f68e78e..1a223b7e1 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -412,6 +412,27 @@ public function test_abilities_are_annotated_read_only() { } } + /** + * Test that every ability is exposed to MCP clients. + * + * The MCP adapter's default server hides abilities that do not opt in, so + * without the flag an agent connected through it cannot see any of them. + * + * @return void + */ + public function test_abilities_are_public_to_mcp() { + if ( ! \function_exists( 'wp_get_ability' ) ) { + $this->markTestSkipped( 'The Abilities API is not available in this WordPress version.' ); + } + + foreach ( [ 'get-site-score', 'list-recommendations', 'complete-recommendation' ] as $name ) { + $ability = \wp_get_ability( Abilities::CATEGORY . '/' . $name ); + + $this->assertNotNull( $ability, "Ability {$name} is not registered." ); + $this->assertTrue( $ability->get_meta()['mcp']['public'] ?? false, "Ability {$name} is not public to MCP." ); + } + } + /** * Test that both abilities are registered under the plugin's own category. * From a59533d095e7835d3e6a7f4e50f8cd1f36c611a3 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 25 Sep 2026 14:00:42 +0200 Subject: [PATCH 15/17] List a recommendation awaiting celebration under completed Completion is two post statuses, not one: Task::is_completed() counts both trash and pending. A completed task is pending until the dashboard celebrates it, and only then trash. The status map returned only trash for `completed`, so a recommendation finished outside wp-admin -- through the email link, for one -- fell out of every list until someone opened the dashboard. A caller could complete a recommendation, be told it worked, then list to confirm and find it gone from pending, completed and snoozed alike, with no way to tell a completion from a deletion. The existing test asserted the mapping returned 'trash' for completed, which was true and was the bug: it encoded one of the two statuses as though it were the only one. Originally 10865f54 on the prototype branch, dropped before it was pushed. Co-Authored-By: Claude Opus 5.5 --- classes/abilities/class-recommendations.php | 17 ++++++++++----- tests/phpunit/test-class-abilities.php | 24 +++++++++++++++++---- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index cd8a90570..ae29239e4 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -13,22 +13,29 @@ class Recommendations { /** - * Map an ability status onto the post status that encodes it. + * Map an ability status onto the post statuses that encode it. + * + * Completion is two statuses, not one. Task::is_completed() counts both + * 'trash' and 'pending', and the plugin writes whichever suits the route: + * the dashboard trashes a task once it has celebrated it, while a task + * completed and not yet celebrated is 'pending'. Returning only 'trash' + * here made such a task appear in no listing at all -- a caller could not + * confirm its own write, or tell completed from deleted. * * @param string $status The ability status. * - * @return string + * @return array */ private function get_post_status_for( $status ) { switch ( $status ) { case 'completed': - return 'trash'; + return [ 'trash', 'pending' ]; case 'snoozed': - return 'future'; + return [ 'future' ]; default: - return 'publish'; + return [ 'publish' ]; } } diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 1a223b7e1..ea5b1360e 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -237,9 +237,25 @@ public function test_get_checklist_reflects_pending_updates() { * @return void */ public function test_status_maps_to_post_status() { - $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'pending' ] ) ); - $this->assertSame( 'trash', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'completed' ] ) ); - $this->assertSame( 'future', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'snoozed' ] ) ); + $this->assertSame( [ 'publish' ], $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'pending' ] ) ); + $this->assertSame( [ 'future' ], $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'snoozed' ] ) ); + } + + /** + * Test that completion covers both post statuses that mean completed. + * + * The dashboard trashes a task once it has celebrated it, while a task + * completed and not yet celebrated is pending. Listing only one of those hid a + * recommendation the caller had just completed, leaving it unable to + * confirm its own write or tell completed from deleted. + * + * @return void + */ + public function test_completed_covers_both_statuses_that_mean_completed() { + $statuses = $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'completed' ] ); + + $this->assertContains( 'trash', $statuses ); + $this->assertContains( 'pending', $statuses ); } /** @@ -248,7 +264,7 @@ public function test_status_maps_to_post_status() { * @return void */ public function test_unknown_status_falls_back_to_pending() { - $this->assertSame( 'publish', $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'nonsense' ] ) ); + $this->assertSame( [ 'publish' ], $this->invoke_on( $this->recommendations, 'get_post_status_for', [ 'nonsense' ] ) ); } /** From 4ad77ba8f5cc7aeb4ae8fbffc67acbb94e9430d9 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Tue, 29 Sep 2026 14:49:42 +0200 Subject: [PATCH 16/17] See a page role in the same request that set it get_posts_by_type() memoised its results in a function-static variable nothing could clear. set_page_values() reads it before writing, so the check that follows in the same request saw the page types from before the write: setting the About page through complete-recommendation reported applied_not_yet_complete although the recommendation was satisfied. The cache is now a property, dropped whenever term relationships change (set_object_terms, deleted_term_relationships) -- a page's type is a term, and set_page_values() writes it both through Page_Types and through wp_remove_object_terms() directly, so invalidating at the source covers every writer. The comment in is_satisfied() that documented the stale read goes with it. Co-Authored-By: Claude Opus 5.5 --- classes/abilities/class-recommendations.php | 7 ----- classes/class-page-types.php | 34 ++++++++++++++++----- tests/phpunit/test-class-abilities.php | 31 +++++++++++++++++++ 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index ae29239e4..9ae16a90d 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -202,13 +202,6 @@ private function is_satisfied( $provider, $task ) { // exists precisely while the condition is unmet, so "would not be added // now" means satisfied. The set-page providers are the case in point -- // they do not override is_task_completed() at all. - // - // This can still report false immediately after a write. Page-type - // lookups are memoised in a static cache with no invalidation, so within - // one request the check may read state from before the change. That is - // why the status distinguishes "applied" from "satisfied" rather than - // assuming the two are the same: the next request sees it correctly, and - // a caller is told what actually happened either way. return \method_exists( $provider, 'should_add_task' ) && false === (bool) $provider->should_add_task(); } diff --git a/classes/class-page-types.php b/classes/class-page-types.php index 2b58d751d..eef9b1a2b 100644 --- a/classes/class-page-types.php +++ b/classes/class-page-types.php @@ -26,6 +26,13 @@ class Page_Types { */ const TAXONOMY_NAME = 'progress_planner_page_types'; + /** + * Posts per page type, memoised for the request by get_posts_by_type(). + * + * @var array> + */ + private $posts_by_type_cache = []; + /** * Constructor */ @@ -41,6 +48,20 @@ public function __construct() { \add_action( 'post_updated', [ $this, 'post_updated' ], 10, 2 ); \add_action( 'wp_insert_post', [ $this, 'post_updated' ], 10, 2 ); \add_action( 'transition_post_status', [ $this, 'transition_post_status' ], 10, 3 ); + + // A page's type is a term, so any change to term relationships can + // change what get_posts_by_type() should return in this request. + \add_action( 'set_object_terms', [ $this, 'flush_posts_by_type_cache' ] ); + \add_action( 'deleted_term_relationships', [ $this, 'flush_posts_by_type_cache' ] ); + } + + /** + * Forget the posts memoised by get_posts_by_type(). + * + * @return void + */ + public function flush_posts_by_type_cache() { + $this->posts_by_type_cache = []; } /** @@ -190,12 +211,11 @@ public function get_page_types() { * @return \WP_Post[] Return the posts. */ public function get_posts_by_type( $post_type, $slug ) { - static $cache = []; - if ( isset( $cache[ $post_type ][ $slug ] ) ) { - return $cache[ $post_type ][ $slug ]; + if ( isset( $this->posts_by_type_cache[ $post_type ][ $slug ] ) ) { + return $this->posts_by_type_cache[ $post_type ][ $slug ]; } - if ( ! isset( $cache[ $post_type ] ) ) { - $cache[ $post_type ] = []; + if ( ! isset( $this->posts_by_type_cache[ $post_type ] ) ) { + $this->posts_by_type_cache[ $post_type ] = []; } $posts = \get_posts( [ @@ -220,8 +240,8 @@ public function get_posts_by_type( $post_type, $slug ) { } } - $cache[ $post_type ][ $slug ] = empty( $posts ) ? [] : $posts; - return $cache[ $post_type ][ $slug ]; + $this->posts_by_type_cache[ $post_type ][ $slug ] = empty( $posts ) ? [] : $posts; + return $this->posts_by_type_cache[ $post_type ][ $slug ]; } /** diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index ea5b1360e..17663ab75 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -575,6 +575,37 @@ public function test_complete_recommendation_applies_the_setting() { $this->assertSame( '1', (string) \get_option( 'blog_public' ) ); } + /** + * Test that setting a page role is seen as satisfied in the same request. + * + * The check reads page types through a cache that the write goes around, + * so the recommendation used to report applied_not_yet_complete until the + * next request. + * + * @return void + */ + public function test_complete_recommendation_sees_a_page_role_it_just_set() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $page_id = self::factory()->post->create( + [ + 'post_title' => 'About Us', + 'post_type' => 'page', + 'post_status' => 'publish', + ] + ); + $this->seed_task( 'set-page-about' ); + + $result = $this->recommendations->complete( + [ + 'provider_id' => 'set-page-about', + 'value' => (string) $page_id, + ] + ); + + $this->assertTrue( $result['applied'] ); + $this->assertSame( 'completed', $result['status'] ); + } + /** * Test that a recommendation needing a person is reported, not applied. * From f7bfa2a06ea2dacd73137560d0d499598903d0f2 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Tue, 29 Sep 2026 14:53:37 +0200 Subject: [PATCH 17/17] Record a completion when complete-recommendation makes it complete-recommendation reported `completed` but recorded nothing: the task stayed published, with no activity, until the next wp-admin page load evaluated it. Until then it was still listed as pending and earned no points. It now records the completion itself, through a new Suggested_Tasks::mark_completed() that the admin_init path uses too, so there is one way a task gets completed: celebrate(), flush the task cache, insert the activity. The timezone and date-format providers never completed through the ability at all. Neither has state to observe -- any timezone or date format is valid -- so each counts a recorded completion, which the popover writes on submit. Their fixes are marked completes_on_apply in Recommendation_Fixes, making the exception explicit instead of relaxing "completion is observed, never asserted" for every fix. The abilities test cleared its activities with TRUNCATE, which commits the open transaction and let posts leak into later tests. That was invisible while complete() never changed a task; it is DELETE now. Co-Authored-By: Claude Opus 5.5 --- .../abilities/class-recommendation-fixes.php | 35 +++++++--- classes/abilities/class-recommendations.php | 11 +++- classes/class-suggested-tasks.php | 27 ++++++-- tests/phpunit/test-class-abilities.php | 65 ++++++++++++++++++- 4 files changed, 122 insertions(+), 16 deletions(-) diff --git a/classes/abilities/class-recommendation-fixes.php b/classes/abilities/class-recommendation-fixes.php index 9c1826ffc..7538681af 100644 --- a/classes/abilities/class-recommendation-fixes.php +++ b/classes/abilities/class-recommendation-fixes.php @@ -27,6 +27,10 @@ class Recommendation_Fixes { * - input: the input property name, when the caller supplies the value. * - type: 'string' or 'boolean-ish', used to validate before writing. * - summary: what the caller is agreeing to, in plain words. + * - completes_on_apply: set when the provider records completion instead + * of detecting it, so applying the fix is the completion -- as submitting + * the popover is. There is no state to observe: any timezone or date + * format is a valid answer. * * @var array> */ @@ -38,16 +42,18 @@ class Recommendation_Fixes { 'summary' => 'Set the site tagline.', ], 'select-timezone' => [ - 'option' => 'timezone_string', - 'input' => 'value', - 'type' => 'timezone', - 'summary' => 'Set the site timezone.', + 'option' => 'timezone_string', + 'input' => 'value', + 'type' => 'timezone', + 'summary' => 'Set the site timezone.', + 'completes_on_apply' => true, ], 'set-date-format' => [ - 'option' => 'date_format', - 'input' => 'value', - 'type' => 'string', - 'summary' => 'Set the date format.', + 'option' => 'date_format', + 'input' => 'value', + 'type' => 'string', + 'summary' => 'Set the date format.', + 'completes_on_apply' => true, ], 'search-engine-visibility' => [ 'option' => 'blog_public', @@ -437,6 +443,19 @@ private static function apply_deletion( $provider_id ) { return true; } + /** + * Whether applying a fix is itself the completion. + * + * @param string $provider_id The provider ID. + * + * @return bool + */ + public static function completes_on_apply( $provider_id ) { + $fix = self::get( $provider_id ); + + return null !== $fix && ! empty( $fix['completes_on_apply'] ); + } + /** * Whether applying a fix removes content rather than changing a setting. * diff --git a/classes/abilities/class-recommendations.php b/classes/abilities/class-recommendations.php index 9ae16a90d..3f8b30da8 100644 --- a/classes/abilities/class-recommendations.php +++ b/classes/abilities/class-recommendations.php @@ -139,8 +139,15 @@ public function complete( $input = [] ) { // Completion is observed, never asserted: the provider decides whether // the site now satisfies the task. Saying otherwise would award points - // for work that did not happen. - $completed = $this->is_satisfied( $provider, $task ); + // for work that did not happen. The exception is a provider with nothing + // to observe, whose own record of completion is the submission itself. + $completed = Recommendation_Fixes::completes_on_apply( $provider_id ) || $this->is_satisfied( $provider, $task ); + + // Recorded now, as the dashboard would on its next load. Otherwise the + // task stays listed and unscored until someone opens wp-admin. + if ( $completed ) { + \progress_planner()->get_suggested_tasks()->mark_completed( $task ); + } // The wording distinguishes a settings change from a deletion: an agent // relaying this to a person should not describe trashing a post as diff --git a/classes/class-suggested-tasks.php b/classes/class-suggested-tasks.php index cd1791c97..ec2e192ad 100644 --- a/classes/class-suggested-tasks.php +++ b/classes/class-suggested-tasks.php @@ -87,14 +87,31 @@ public function init(): void { continue; } - // Change the task status to pending. - $task->celebrate(); - - // Insert an activity. - $this->insert_activity( \progress_planner()->get_suggested_tasks()->get_task_id_from_slug( $task->post_name ) ); + $this->mark_completed( $task ); } } + /** + * Record a task as completed. + * + * The one place completion happens, whichever route found the task done: + * the task waits for its celebration as 'pending', and the activity is what + * points, badges and streaks count. + * + * @param \Progress_Planner\Suggested_Tasks\Task $task The task. + * + * @return void + */ + public function mark_completed( $task ) { + $task->celebrate(); + + // update_recommendation() does not flush the task cache, so a later read + // in the same request would still see the task as open. + \wp_cache_flush_group( \Progress_Planner\Suggested_Tasks_DB::GET_TASKS_CACHE_GROUP ); + + $this->insert_activity( $this->get_task_id_from_slug( $task->post_name ) ); + } + /** * Insert an activity. * diff --git a/tests/phpunit/test-class-abilities.php b/tests/phpunit/test-class-abilities.php index 17663ab75..f048321c3 100644 --- a/tests/phpunit/test-class-abilities.php +++ b/tests/phpunit/test-class-abilities.php @@ -71,7 +71,9 @@ public function setUp(): void { public function tearDown(): void { global $wpdb; - $wpdb->query( 'TRUNCATE TABLE ' . $wpdb->prefix . 'progress_planner_activities' ); // phpcs:ignore WordPress.DB + // DELETE, not TRUNCATE: TRUNCATE commits the open transaction, so the + // posts this test created would survive the rollback into the next test. + $wpdb->query( 'DELETE FROM ' . $wpdb->prefix . 'progress_planner_activities' ); // phpcs:ignore WordPress.DB parent::tearDown(); } @@ -606,6 +608,67 @@ public function test_complete_recommendation_sees_a_page_role_it_just_set() { $this->assertSame( 'completed', $result['status'] ); } + /** + * Test that a completion is recorded when it happens, not on the next + * admin page load. + * + * @return void + */ + public function test_complete_recommendation_records_the_completion() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + \update_option( 'blog_public', '0' ); + $this->seed_task( 'search-engine-visibility' ); + + $result = $this->recommendations->complete( [ 'provider_id' => 'search-engine-visibility' ] ); + + $this->assertSame( 'completed', $result['status'] ); + + $task = \progress_planner()->get_suggested_tasks_db()->get_post( 'test-search-engine-visibility' ); + $this->assertNotNull( $task ); + $this->assertSame( 'pending', \get_post_status( $task->ID ), 'The task waits for its celebration.' ); + + $activities = \progress_planner()->get_activities__query()->query_activities( + [ + 'data_id' => 'test-search-engine-visibility', + 'type' => 'completed', + ] + ); + $this->assertCount( 1, $activities, 'The completion is scored once.' ); + } + + /** + * Test that setting the timezone completes its recommendation. + * + * The timezone provider has no state to observe -- any timezone is valid -- + * so it counts a recorded completion, which the popover writes on submit + * and the ability used to leave out. + * + * @return void + */ + public function test_complete_recommendation_completes_the_timezone() { + \wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + \progress_planner()->get_suggested_tasks_db()->add( + [ + 'task_id' => 'select-timezone', + 'post_title' => 'Set site timezone', + 'provider_id' => 'select-timezone', + ] + ); + + $result = $this->recommendations->complete( + [ + 'provider_id' => 'select-timezone', + 'value' => 'Europe/Ljubljana', + ] + ); + + $this->assertSame( 'completed', $result['status'] ); + $this->assertFalse( + \progress_planner()->get_suggested_tasks()->get_tasks_manager()->get_task_provider( 'select-timezone' )->should_add_task(), + 'The provider no longer asks for the task.' + ); + } + /** * Test that a recommendation needing a person is reported, not applied. *