Add core/content ability#739
Conversation
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
d1db766 to
88df83c
Compare
| // Expose curated core objects to the Abilities API, then register the | ||
| // `core/settings` and `core/content` abilities (overriding any core-provided copies). | ||
| Show_In_Abilities::register(); | ||
| Settings_Ability::init(); |
There was a problem hiding this comment.
Some changes leaked from another PR with the core/settings ability.
|
Nice work so far! A few things to iron out:
|
|
For reference, I'm sharing WP CLI commands related to read-only operations on posts:
|
|
@jorgefilipecosta looks like some code review feedback and merge conflicts to clean up to help move this along towards merge and inclusion in the next AI plugin release (to help get some usage testing & feedback before a parallel PR is landed for core in 7.1) |
The .mcp.json file holds developer-local MCP server config (xdebug, chrome-devtools) and should not be in version control. Remove it from the repo and add it to .gitignore; the file is kept locally.
The e2e global setup deletes all `post` entries, so querying `post_type: 'post'` for the field-limiting test returned zero posts in clean CI and failed the `posts.length > 0` assertion. Seed published posts via `requestUtils.createPost` in a `beforeAll` (tracking their IDs) and remove only those in `afterAll`, then query `post` as before.
Re-applied on top of the current branch tip (a prior push of these changes was
overwritten by a force-push). Mirrors the core/settings review fixes that apply here:
- Memoize the exposed post types so the input schema and the permission/execute
callbacks derive from a single walk of the registered post types.
- Default the input schema to an empty object so the type:object default serializes as {}.
- Use __() instead of esc_html__(), and @SInCE x.x.x per CONTRIBUTING.md.
- Harden input/value handling (type guards, capability resolver, non-negative int
helper, dynamic-property annotation) so the new code passes PHPStan at level max.
The `content` ability-category fallback is kept on purpose: unlike `site`, `content`
is a new category not present on the plugin's minimum WordPress (7.0).
The settings PR consolidated the e2e support plugins into a single `tests/e2e-testing` plugin. Mirror that here: register the sample `ai_e2e_sample` post type and seed its published post inside e2e-testing.php (next to the sample setting), drop the standalone e2e-sample-content plugin and its .wp-env.test.json entry, and point the core/content spec comment at e2e-testing.
Mirror the merged core/settings ability (and core's WP_Settings_Abilities): convert Content from a static class to a final, instance-based one. The externally-invoked entry points (init, register, register_category, execute_get_content, check_permission, return_raw_title_format) stay public; register_get_content() and the shared helpers become private; CATEGORY and the per-page bounds become private consts; the $fields list and the cached $exposed_post_types become instance state. Behaviour is unchanged. Note: core's WP_Content_Abilities is still static; this is the one structural divergence from the core twin (to be reconciled when core content migrates).
…ities Rebase off the merged settings PR left Show_In_Abilities as the old static, settings-only class. Restore develop's final, instance-based version and add the content polyfill on top as instance methods: mark_post_type() on the register_post_type_args filter, mark_registered_post_types() to patch core post types already registered during bootstrap, and a private post_types_map() (post, page). settings_map() and post_types_map() are both private, matching the scoped settings class.
This is an unrealistic and IMO unwanted end-goal. I think we need to accept that no new core abilities will ship in 7.1, even if they make it into AI@1.1.0 < 3 weeks is not enough time to get actual API design feedback for core. It's not even a full AI Plugin release cycle. By all means if we can get this cleaned up in time for the next plugin release, great, but considering that they're not even behind an Experiment toggle, I wouldn't want y'all rushing it in before you or @dkotter think its viable because of a desire to squeeze it into beta1. |
88df83c to
3a7df4a
Compare
Drive the Content and Show_In_Abilities tests through instances (held on the test case so the same instance detaches its hooks on tear down) instead of the old static calls. Bring ContentTest in line with core's wpRegisterCoreContentAbility: assert show_in_rest + destructive/idempotent annotations on registration, assert the input schema type and additionalProperties, add output-schema coverage, and add a test that a post type registered by another active plugin (flagged show_in_abilities) is exposed in the input enum and in query results.
3a7df4a to
b3748e6
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #739 +/- ##
=============================================
+ Coverage 76.41% 77.45% +1.04%
- Complexity 1828 1970 +142
=============================================
Files 87 88 +1
Lines 7764 8229 +465
=============================================
+ Hits 5933 6374 +441
- Misses 1831 1855 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Per review, the params are not interchangeable: fetching a single post by `id`
and querying a set by `post_type` are distinct modes, and query-only filters
(`status`, `author`, `parent`, `slug`, `page`, `per_page`) make no sense
alongside `id`. The previous flat schema (an `anyOf` requiring `id` or
`post_type`) accepted those combinations and silently ignored the extras.
Model the schema as a `oneOf` of two modes, each with `additionalProperties:
false`, so e.g. `{ id, per_page }` fails validation instead of dropping
`per_page`. `fields` is accepted in both modes; `post_type` stays allowed
alongside `id` as a guard. Tests cover the rejected and accepted combinations.
Remove the `.mcp.json` and `/CLAUDE.local.md` ignore rules that slipped in from local dev setup; they are unrelated to the core/content ability.
|
Hi @gziolo, your feedback was applied. |
peterwilsoncc
left a comment
There was a problem hiding this comment.
I've added a few notes inline.
To avoid burying the lead: I think the key question I have is why you don't include the rendered content so bots can be set up with a read-only account?
| $edit_posts_capability = $this->capability( $post_type_object, 'edit_posts', 'edit_posts' ); | ||
|
|
||
| return current_user_can( $edit_posts_capability ); // phpcs:ignore WordPress.WP.Capabilities.Undetermined -- Capability is resolved from the post type's capability object. |
There was a problem hiding this comment.
get_post_type_capabilities() ensures the caps are populated so you can avoid the conditional.
| $edit_posts_capability = $this->capability( $post_type_object, 'edit_posts', 'edit_posts' ); | |
| return current_user_can( $edit_posts_capability ); // phpcs:ignore WordPress.WP.Capabilities.Undetermined -- Capability is resolved from the post type's capability object. | |
| return current_user_can( $post_type_object->cap->edit_posts ); |
| * @param string $fallback Fallback capability name if unset or non-string. | ||
| * @return string The resolved capability name. | ||
| */ | ||
| private function capability( \WP_Post_Type $post_type_object, string $name, string $fallback ): string { |
| foreach ( get_post_types( array(), 'objects' ) as $post_type_object ) { | ||
| if ( empty( $post_type_object->show_in_abilities ) ) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
You should be able to use the core api to determine whether the post type is shown in the abilities.
| foreach ( get_post_types( array(), 'objects' ) as $post_type_object ) { | |
| if ( empty( $post_type_object->show_in_abilities ) ) { | |
| continue; | |
| } | |
| foreach ( get_post_types( array( 'show_in_abilities' => true ), 'objects' ) as $post_type_object ) { |
| * @return array<string, \WP_Post_Type> Exposed post type objects keyed by name. | ||
| */ | ||
| private function get_exposed_post_types(): array { | ||
| $exposed = array(); |
There was a problem hiding this comment.
For clarity: requires changes elsewhere.
| $exposed = array(); | |
| $exposed_post_types = array(); |
| return $this->fields; | ||
| } | ||
|
|
||
| $requested = array_filter( $input['fields'], 'is_string' ); |
There was a problem hiding this comment.
Clarity
| $requested = array_filter( $input['fields'], 'is_string' ); | |
| $requested_fields = array_filter( $input['fields'], 'is_string' ); |
| * | ||
| * @since x.x.x | ||
| */ | ||
| public function test_password_protected_content_visible_to_editor(): void { |
There was a problem hiding this comment.
Again, needs unhappy path for subs, authors, contribs.
| 'paged' => $page, | ||
| 'perm' => 'editable', | ||
| 'ignore_sticky_posts' => true, | ||
| ); |
There was a problem hiding this comment.
As you are not including terms or meta currently, I'd bypass priming the caches for now.
| * | ||
| * @return string[] List of public, non-internal post status slugs. | ||
| */ | ||
| private function get_available_statuses(): array { |
| private function login_as( string $role ): int { | ||
| $user_id = self::factory()->user->create( array( 'role' => $role ) ); | ||
| wp_set_current_user( $user_id ); | ||
| return $user_id; | ||
| } |
There was a problem hiding this comment.
To speed up the tests, add a user for each role type as a shared fixture in wpSetupBeforeClass rather than create a new user account each time.
Same can be done for a few posts, I suspect.
There was a problem hiding this comment.
For tests with multiple assertions, add a message to each per unit test standads.
|
After a closer inspection, I want to echo the point that @peterwilsoncc raised regarding the data formatting. We need to decide whether we return raw data or rendered/filtered data. This is what I see now:
What is needed will largely depend on the consumer. If they only want to present the data, then the preferred option would be using dates in the site's timezone, title, and content as rendered HTML. If they want to edit content, then maybe fetching raw data would make more sense. There are two ways to go about it:
@justlevine, thank you for the reminder about the timeline for the WP 7.1 release. Let's see how much work is left to iron out all the feedback raised so far. Either way, we want to get some early testing through the AI plugin. All the feedback is appreciated and will help shape this essential ability. |
| 'enum' => $post_types, | ||
| 'description' => __( 'Optional. Restrict the lookup to this post type; the post is returned only if it matches and the current user can edit it.', 'ai' ), | ||
| ), | ||
| 'fields' => $fields, |
There was a problem hiding this comment.
In both cases, fields can be defined, so you can lift it up as properties always available on the object, next to oneOf.
post_type is more nuanced as it is required in one case but optional in another, so maybe fine as is, but technically it could be lifted up, too.
There was a problem hiding this comment.
Hi @gziolo, this would not work because inside each one of branch we have additionalProperties false, so on strict validation the client uses fields would be considered an additional property for oneOf branches and validation would fail.
There was a problem hiding this comment.
Any LLM consuming this in any way needs to know what posts types are available. Either what @gziolo suggested or inside the filed description or inside the ability description.
|
|
||
| return array( | ||
| 'type' => 'object', | ||
| 'additionalProperties' => false, |
There was a problem hiding this comment.
For the sake of completeness, all properties should be marked as required:
| 'additionalProperties' => false, | |
| 'additionalProperties' => false, | |
| 'required' => [ 'posts', 'total', 'total_pages' ], |
| 'core/content', | ||
| array( | ||
| 'label' => __( 'Get Content', 'ai' ), | ||
| 'description' => __( 'Retrieves one or more readable posts of a post type exposed to abilities. Fetch a single readable post by ID or by slug, or query multiple readable posts filtered by post type, status, author, or parent. Returns a basic, support-aware set of fields per post, with raw fields limited to users who can edit the post.', 'ai' ), |
There was a problem hiding this comment.
"readable posts of a post type exposed to abilities." An LLM consuming this ability has no idea what "exposed to abilities" means.
This should contain the available posts types or instruct how it can get a list of available post types.
There was a problem hiding this comment.
Another ability that is being developed is called core/manage-content. So something hinting at reading could work here if we need more specificity. search- could work, too.
The title and description should cover the difference already, but if we want to have it also in the name then these options are available.
| $statuses = array_values( get_post_stati( array( 'internal' => false ) ) ); | ||
|
|
||
| wp_register_ability( | ||
| 'core/content', |
There was a problem hiding this comment.
with no verb, this ability name implies any action(CRUD+) on the entire content domain (posts, post-meta, terms, comments, media...)
Suggestion: core/read-content not ideal either, but at least has the verb saying what it does
| 'content_rendered' => array( | ||
| 'type' => 'string', | ||
| 'description' => __( 'The rendered post content. Present when the post type supports the editor. Empty when withheld for a password-protected post.', 'ai' ), | ||
| ), |
There was a problem hiding this comment.
If we return both content_raw and content_rendered, this will fill up the context window with duplicated content.
Suggestion: have a scope input field(similar to REST API)
editwill return the raw content that an agent can modify and update on a new requestreadit will return the "content_rendered". Probably best if we can strip html tags also (or transformed to MD :D [nice to have]) (I have no strong opinion on this one but with stripped tags we will be nice with the context window)
There was a problem hiding this comment.
readit will return the "content_rendered". Probably best if we can strip html tags also (or transformed to MD :D [nice to have]) (I have no strong opinion on this one but with stripped tags we will be nice with the context window)
Warning
Opinionated 😸
I'd be reluctant to add the required code to core given it would add quite a bit of work to each request.
Dries Buytaert wrote up his experience of providing a markdown version of his content for LLMs and finding that the agents simply crawled both versions, so I think it's pretty safe to assume that they're happy with HTML.
There was a problem hiding this comment.
IMO that article has little to do with Markdown vs HTML and is rather about sources of truth/context rot (LLMs.txt is ineffective) and misunderstandings about how agentic discovery works (nothings going to visit your MD endpoints if you don't explicitly encourage it to).
That aside , there are other implications with exposing raw vs rendered content, including security and end-user case. As abilities are meant to be a primitive and not a MCP-specific transport, I think the way to address that in MCP (or upstream in Abilities) is an arg to filter the output.
If in the short term this is a concern, then I'd say keep content_rendered without content_raw, since even if LLMs do prefer markdown to HTML, they'd want the markdown version of the rendered html, not the one fill with custom block markup or unprocessed synced patterns etc. But longer term, IMO the solution belongs in a different layer.
There was a problem hiding this comment.
That aside , there are other implications with exposing raw vs rendered content,
@jasonbahl any chance you have a few minutes to share your thoughts from on over in WPGraphQL-land? With how much influence we took in design language, it would make sense for the output ergonomics to wind up being similar, just maybe a bit flatter. Same considerations, at least 🤔
There was a problem hiding this comment.
In my earlier #739 (comment), I referenced WP CLI as a good example of where good defaults solve the issue you discuss. It could be replicated here by returning the same set: ID, post_title, post_name, post_date, and post_status. All other fields remain accessible, but the consumer needs to explicitly list them in the input using the fields array.
Summary
Part of: #40
Adds the read-only
core/contentability to the plugin, mirroring the WordPress coreWP_Content_Abilitiesimplementation (see the companion core PR) so the two stay in sync. It overrides any core-provided copy.Retrieves one or more posts of a post type exposed to abilities: fetch a single post by
idorslug, or query multiple posts filtered bypost_type,status,author,parent, with afieldsselector andpage/per_pagepagination. Output is{ posts, total, total_pages }.core PR WordPress/wordpress-develop#12195.
Security
Defense in depth: a coarse status/capability gate plus an authoritative per-post
read_postcheck on every row; default statuspublish; password-protected content withheld from non-editors; uniform not-found responses to avoid leaking post existence.Show_In_Abilities
WordPress core does not yet ship the
show_in_abilitiesflag this ability reads, so a self-containedShow_In_Abilitiescomponent polyfills it onto curated core post types (post,page). It is structured exactly like the settings polyfill from thecore/settingsPR (#691), so the two merge cleanly.This PR is independent of #691 and can be reviewed/merged on its own.
Tests
PHPUnit integration tests for the ability and the polyfill, plus an e2e spec (
tests/e2e/specs/abilities/core-content.spec.js) backed by a sample-content plugin that registers an ability-exposed post type.