-
Notifications
You must be signed in to change notification settings - Fork 26
Proposal: Add ability anotations #99
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.
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
base: trunk
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #99 +/- ##
============================================
- Coverage 85.69% 85.60% -0.09%
- Complexity 103 105 +2
============================================
Files 16 16
Lines 776 792 +16
Branches 86 86
============================================
+ Hits 665 678 +13
- Misses 111 114 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0a65ff7
to
324d332
Compare
1ca8dcb
to
1708839
Compare
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. |
1708839
to
9306cd9
Compare
9306cd9
to
a84b388
Compare
b0824a5
to
47ef053
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot. I prefer the annotations over the current tool/resource pattern.
The instructions
will also be useful for providing tool instructions that various things like the MCP Adapter or Agents can adapt.
* @see WP_Abilities_Registry | ||
*/ | ||
class WP_Ability { | ||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to suggest we align our internal naming conventions with the established MCP protocol standards to reduce developer cognitive load and maintain consistency across the systems.
Current approach:
- Internally:
read_only
,destructive
,idempotent
- MCP:
readOnlyHint
,destructiveHint
,idempotentHint
Suggested approach:
Since MCP is a well-established protocol with defined conventions, I think we should adopt the MCP naming directly in our abilities API:
- Use:
readOnlyHint
,destructiveHint
,idempotentHint
internally - This eliminates the need for translation in the MCP adapter
- Developers working with both our API and MCP won't need to context-switch between naming conventions
- Minimizes developer confusion when working across both systems
Additional consideration - Instructions handling:
For the instructions
field, since MCP uses the tool description as instructions, we can handle this in the MCP adapter by appending our instructions
content to the description field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The counterpoint would be that WordPress uses snake case convention for naming properties. REST API is a good reflection of that. It's also inconvenient with JavaScript that uses camel case similarly to the MCP protocol. Sometimes, we even mix the two when functionality spans all layers, as with block types that also support the JSON format, which again uses camel case.
@felixarntz, and @swissspidy, what are your thoughts on that aspect?
Closes #62.
What
I took some inspiration from ToolAnnotation in MCP. For starters, we will need
read_only
to replace the current logic that usesmeta.tool
to distinguish between HTTP method usage (i.e.,GET
andPOST
) in requests.Why
The best explanation of the root problem that is being addressed comes from @galatanovidiu in WordPress/mcp-adapter#48 (comment)
Here, we remove all references to
tool
andresource
nomenclature. Instead, we perform detection of theGET
vsPOST
method in the REST API calls based on annotations that describe what the ability does: read only vs updates data.Fute considerations
With more properties, we could also start using
DELETE
HTTP method as I exploreidempotent
anddestructive
annotations. I'm also interested in including freeforminstructions
so devs could consist of usage examples for both the user and for consideration to pass to LLMs.Testing
Run test: