Make WordPress Core

Opened 5 weeks ago

Last modified 3 weeks ago

#64616 new enhancement

Abilities API: Add core/update-settings ability

Reported by: jorgefilipecosta's profile jorgefilipecosta Owned by:
Milestone: 7.1 Priority: normal
Severity: normal Version: trunk
Component: AI Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

This ticket proposes adding a core/update-settings ability to complement the existing core/get-settings ability, providing a symmetric API for reading and writing settings through the Abilities API.

Proposed Solution

Add core/update-settings ability with the following characteristics:

Input/Output Symmetry:

  • Input accepts settings grouped by registration group (same structure returned by core/get-settings)
  • Output returns updated_settings (with updated values) and validation_errors (with error messages) in the same grouped structure

Example Usage:

// Get current settings
const settings = await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/get-settings/run'
});

// Modify and update
settings.general.blogname = 'New Site Title';
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/update-settings/run',
  method: 'POST',
  data: { input: { settings } }
});

Security:

  • Requires manage_options capability
  • Only settings with show_in_abilities = true can be modified
  • Settings must be placed in their correct registration group
  • Values are validated against registered schemas
  • Registered sanitize callbacks are applied

Annotations:

  • readonly: false - This ability modifies settings
  • destructive: false - Changes can be reverted
  • idempotent: true - Same input produces same result

Change History (8)

This ticket was mentioned in PR #10892 on WordPress/wordpress-develop by @jorgefilipecosta.


5 weeks ago
#1

  • Keywords has-unit-tests added

Part of: https://github.com/WordPress/ai/issues/40
Inspired by the work on https://github.com/galatanovidiu/mcp-adapter-implementation-example/tree/experiment/layerd-mcp-tools/includes/Abilities by @galatanovidiu.
Ticket: https://core.trac.wordpress.org/ticket/64616

This PR adds a core/update-settings ability to the WordPress Abilities API. This ability allows updating WordPress settings that have show_in_abilities = true. It complements the existing core/get-settings ability by providing a symmetric API for reading and writing settings.

The input and output structures are designed for symmetry with core/get-settings:

  • Input accepts settings grouped by registration group (same structure returned by core/get-settings)
  • Output returns updated_settings (with updated values) and validation_errors (with error messages) in the same grouped structure

## Organization

Following the pattern established in #10747, this PR extends the WP_Settings_Abilities class in src/wp-includes/abilities/class-wp-settings-abilities.php. The implementation maximizes code reuse:

  • Schema Reuse: Uses a permissive input schema that allows the execute callback to handle validation
  • Shared Methods: Reuses get_allowed_settings(), check_manage_options(), and cast_value() from the existing implementation
  • Registration: register_update_settings() registers the ability with appropriate annotations (readonly: false, destructive: false, idempotent: true)
  • Execution: execute_update_settings() validates, sanitizes, and updates settings, returning both successful updates and validation errors

## Test plan

  • Open /wp-admin/post-new.php
  • Open the browser console and run the following examples:
// Update a single setting
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/update-settings/run',
  method: 'POST',
  data: {
    input: {
      settings: {
        general: {
          blogname: 'New Site Title'
        }
      }
    }
  }
});
  • [ ] Verify the setting is updated in the database
// Update multiple settings across groups
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/update-settings/run',
  method: 'POST',
  data: {
    input: {
      settings: {
        general: {
          blogname: 'New Site Title',
          blogdescription: 'New Tagline'
        },
        reading: {
          posts_per_page: 15
        }
      }
    }
  }
});
  • [ ] Verify multiple settings across different groups are updated
// Check the ability schema
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/update-settings'
});
  • [ ] Verify the input schema documents the settings structure
  • [ ] Verify the output schema documents updated_settings and validation_errors
// Workflow: Get settings, modify, update (symmetry test)
const settings = await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/get-settings/run'
});
settings.general.blogname = 'Modified Title';
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/update-settings/run',
  method: 'POST',
  data: { input: { settings } }
});
  • [ ] Verify the get -> modify -> update workflow works seamlessly
// Test partial success with invalid settings
await wp.apiFetch({
  path: '/wp-abilities/v1/abilities/core/update-settings/run',
  method: 'POST',
  data: {
    input: {
      settings: {
        general: {
          blogname: 'Valid Title',
          unknown_setting: 'should fail'
        }
      }
    }
  }
});
  • [ ] Verify blogname is in updated_settings and unknown_setting is in validation_errors
  • [ ] Test permission check: Verify non-admin users cannot access the ability (requires manage_options capability)
  • [ ] Verify settings without show_in_abilities cannot be modified
  • [ ] Verify settings placed in wrong groups are rejected with appropriate error messages

@jorgefilipecosta commented on PR #10892:


4 weeks ago
#2

Thank you @westonruter for reviewing this PR, all your suggestions were applied.

#3 @jorgefilipecosta
4 weeks ago

  • Type changed from defect (bug) to enhancement

#4 @justlevine
4 weeks ago

Left some feedback on the PR. That said, we might want to consider holding on this (and reverting #64605) if we can't get https://core.trac.wordpress.org/ticket/64596 resolved in time.

@justlevine commented on PR #10892:


4 weeks ago
#5

Related to https://core.trac.wordpress.org/ticket/64605, but since that got merged I'll ask here. How exactly does show_in_abilities work here?

Is show_in_abilities: bool|array{schema:array<string,mixed}, or are we in an awkward place where show_in_abilities is a bool, but we're going to infer any schemas from show_in_rest['schema'] ? I'm also not seeing anywhere in this or that PR where's we're loading/coercing the existing schema, am I missing it or was it left out for some reason?

#6 @jorgefilipecosta
4 weeks ago

Hi @justlevine, thank you so much for the thorough review! I really appreciate the detailed feedback and will apply the suggested changes on the PR.

Left some feedback on the PR. That said, we might want to consider holding on this (and reverting #64605) if we can't get https://core.trac.wordpress.org/ticket/64596 resolved in time.

Regarding the potential revert, I believe that independently of the nesting discussion, core/get-settings and core/update-settings should still keep these names. While nesting may make sense for posts, where grouping by post type feels natural (e.g., core/post/get, core/page/get) — settings don't really have that same need for hierarchy.

We already follow a flat naming pattern for similar abilities: core/get-site-info, core/get-user-info, core/get-environment-info, etc. So core/get-settings and core/update-settings are simply consistent additions to that existing convention.

Is show_in_abilities: bool|array{schema:array<string,mixed}, or are we in an awkward place where show_in_abilities is a bool, but we're going to infer any schemas from show_in_restschema? ? I'm also not seeing anywhere in this or that PR where's we're loading/coercing the existing schema, am I missing it or was it left out for some reason?

Regarding show_in_abilities and schemas, Great point! For now, show_in_abilities is a simple boolean: true means the setting is exposed in abilities, false means it isn't. We intentionally kept it simple because we don't support things like renaming settings for abilities, which I don't think is needed and could confuse agents. Ideally, we should use the raw setting names that map directly to what get_option / update_option expect in core.
That said, you raise a valid point, some settings do have richer schema definitions (like enums) in REST that we're currently not carrying over for abilities I agree that show_in_abilities should support bool|array{schema: array<string, mixed>} rather than being limited to just a boolean, as I believe you're suggesting. I'll put together a commit for that. Thanks for flagging it!

#7 @justlevine
4 weeks ago

I believe that independently of the nesting discussion, core/get-settings and core/update-settings should still keep these names. While nesting may make sense for posts, where grouping by post type feels natural (e.g., core/post/get, core/page/get) — settings don't really have that same need for hierarchy.

Hey @jorgefilipecosta I'm not sure I follow why core/{post?}/get or whatever it end's up shaped like would follow a different pattern than settings, that also have a get, update, delete, and potentially more like cache/* sync migrate export upgrade, some ai use case etc etc.

But that's not even my primary concern here: if we need to deprecate for an improved pattern after the inefficiencies prove themselves justifiable, while sad and wasteful, it's still less sad and less wasteful than a lot of other prior art, and the API there was designed with intentional futurecompat to allow for this discussion and evolution to take place (the discussion happening on that ticket).

The questions raised there are about the holistic process and design form - and testing/evolution - the API decisions have gone through before they hit core. This is about learning from WordPress 5.0 and the dozens of community summits since: We don't have 5+ years to convince folks about AI, if we don't have community participation throughout the entire process we'll lose no matter how good of an API we build. The only currency that matters in the age of AI is trust.

We intentionally kept it simple

To cc @johnbillion from https://github.com/WordPress/ai/issues/40#issuecomment-3909870721, I believe the the question being asked is "who is we", i.e. how much intentionality and forethought has been given both to the current show_in_abilities shape, and its future compatibility with whatever other possibilities might make more sense. I'm only one of the two recorded code reviewers, but I personally didn't give show_in_abilities any thought until I was reviewing this PR, which I think validates the concern they're raising and also kinda underlies generally it's important to get more eyes and a bit more time and to these things holistically, and tie this and get-settings to the discussion about namespacing, since that decision can the ideal schema shapes.

We already follow a flat naming pattern for similar abilities: core/get-site-info, core/get-user-info, core/get-environment-info, etc.

FWIW I am firmly on record against shipping those for 6.9, and considering that I still haven't been shown any compelling IRL use case for get-environment-info or get-site-info, and if we do go for namespaces, we've now got a core/user/* to reconcile with core/get-user-info, so yeah I do think the argument to "learn from the experience" (IMO) and give these more than a few weeks to breathe is a good idea, especially if thats what seasoned committers are requesting.

#8 @audrasjb
3 weeks ago

  • Milestone changed from 7.0 to 7.1

WP 7.0 pre-beta1 Triage:
Unfortunately this enhancement didn't make it before beta 1 code freeze. Thus, let's move it to milestone 7.1.

Note: See TracTickets for help on using tickets.