Skip to content

Let an agent apply recommendations that are a single setting #783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ilicfilip merged 4 commits into from
Sep 29, 2026
Merged

Let an agent apply recommendations that are a single setting #783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e78bffe
Add a short confirmation code to the test email
ilicfilip Sep 18, 2026
ff2fafe
Let an agent clear WordPress's placeholder content
ilicfilip Sep 18, 2026
4b10ec5
Let a caller record which page serves the About, Contact or FAQ role
ilicfilip Sep 18, 2026
b8d3900
Merge branch 'filip/abilities-api' into filip/abilities-api-write
ilicfilip Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions classes/abilities/class-abilities.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
]
)
);
Expand All @@ -173,8 +179,9 @@ public function register_abilities() {
* @return array<string, mixed>
*/
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(
[
Expand All @@ -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,
],
],
Expand Down
222 changes: 222 additions & 0 deletions classes/abilities/class-recommendation-fixes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,70 @@ class Recommendation_Fixes {
'value' => false,
'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.
*
* 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
Expand Down Expand Up @@ -225,6 +289,14 @@ 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 );
}

if ( isset( $fix['seo'] ) ) {
return self::apply_seo_setting( $fix );
}
Expand All @@ -246,6 +318,156 @@ 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<string, mixed> $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<int, string>
*/
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.
*
* 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.
*
Expand Down
48 changes: 41 additions & 7 deletions classes/abilities/class-recommendations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}
Expand Down Expand Up @@ -174,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();
}

/**
Expand All @@ -198,8 +226,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;
}

Expand Down Expand Up @@ -285,6 +318,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 ),
];
}
}
6 changes: 5 additions & 1 deletion classes/abilities/class-schemas.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' ),
],
],
];
Expand Down Expand Up @@ -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' ),
],
],
];
}
Expand Down
Loading
Loading