Conversation
|
Warning Review limit reached
On-demand reviews are free for the next 26 days. After that, they cost $0.25 per reviewed file. Or wait 52 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change adds GravityKit deletion configuration, normalizes MCP annotations, and gates destructive ability and field deletion execution. Documentation and tests cover configuration inheritance, blocked operations, permitted DELETE requests, and tool metadata. ChangesDeletion Policy and Ability Execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change aligns destructive-operation gating and tool metadata across the two tool planes while preserving existing non-destructive behavior. The documented checks passed, and no actionable merge-blocking risk remains beyond normal review. Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant loadAbilitiesAsTools
participant buildTools
participant WordPressAPI
MCPClient->>loadAbilitiesAsTools: load catalog tools with allowDelete
loadAbilitiesAsTools->>buildTools: pass deletion policy
buildTools->>buildTools: normalize annotations and create handler
buildTools->>WordPressAPI: dispatch DELETE when deletion is enabled
``
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
<details>
<summary>🚥 Pre-merge checks | ✅ 5</summary>
<details>
<summary>✅ Passed checks (5 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (2 skipped: 2 … |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately and concisely summarizes the main change: aligning destructive-operation gates across the static and dynamic tool planes. |
</details>
<details>
<summary>Full details: Docstring Coverage</summary>
**Explanation**
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (2 skipped: 2 unsupported.)
</details>
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches</summary>
<details>
<summary>🧪 Generate unit tests (beta)</summary>
- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=GravityKit/MCP&utm_content=13)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
<details>
<summary>❤️ Share</summary>
- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
</details>
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
|
Follow-up hardening in a378208: an independent review caught that WordPress Abilities uses null for unknown annotations. The mapper now omits unknown/null MCP hints instead of converting them to false, preserving MCP's conservative defaults (notably destructiveHint defaults to true when unspecified). Added a regression test for the null/unknown case; targeted ability-loader and field-manager suites pass. |
…tated tools Two safety defaults that were pointed the wrong way. gf_delete_field never checked allowDelete, while deleteForm, deleteEntry and deleteFeed all do -- so a server configured to refuse deletions still let an assistant remove a field, and a deleted field does not go to the Trash the way a form or entry does. The gate is mechkw's, cherry-picked from PR #13; his test needed rebasing onto the current mock, which had no allowDelete at all. The annotation mapping coerced an absent `destructive` to false, publishing "safe" for an ability that had simply said nothing. The MCP spec defaults an omitted destructiveHint to true, and WordPress core defaults ability annotations to null, so this was reachable rather than theoretical. Unstated now resolves to destructive unless the ability declares readonly. What we BLOCK still keys on an explicit declaration, so a product that forgets its annotations looks cautious rather than broken. One fixture declared `annotations: {}` for a create ability; all 50 real GravityView abilities declare `destructive` explicitly, so it was corrected rather than accommodated, and the unknown case got its own coverage for both null and {}. Raised in Fable's review of the open PRs.
|
Thanks @mechkw, and sorry this sat so long. Your PR addresses three things. We took one, fixed another because you flagged it, and had already replaced the third.
Closing the PR, but two of three shipped with your name on them. Thanks! |
2 participants
Summary
Align destructive-operation safety across the MCP server's two tool planes while preserving the autonomous, single-credential workflow:
gf_delete_fieldwith the sameGRAVITY_FORMS_ALLOW_DELETEpolicy already used for form, entry, and feed deletion;permission_callback;ToolAnnotations, so clients can correctly distinguish read-only, mutating, destructive, and idempotent tools;GRAVITY_FORMS_ALLOW_DELETE, with an optionalGRAVITYKIT_ALLOW_DELETEoverride for deployments that need separate policy.Why
The previous behavior had two inconsistent paths:
gf_delete_fieldreplaced the complete form without consultingapi.allowDelete, even though every other static delete operation required the flag.For autonomous operators, the clean model is one explicit full-authority switch plus rollback-first execution—not per-object credential swapping or misleadingly incomplete guardrails. For conservative deployments, deletes remain off by default and the product-specific override can be set independently.
Compatibility
GRAVITY_FORMS_ALLOW_DELETE=trueretain delete access across both tool planes.GRAVITYKIT_ALLOW_DELETEis optional; when omitted it inherits the existing Gravity Forms flag.Verification
npm run prepublishOnly— passedOperational note
Snapshots, readback diffs, browser QC, and automatic rollback are orchestration concerns because the MCP server cannot infer the user's requested task boundary or guarantee restoration of every WordPress-side effect. This patch deliberately keeps that policy out of core while ensuring the server's local guardrails and MCP metadata are truthful and consistent.
cc @zackkatz — this is meant as a small, compatibility-oriented hardening patch rather than a broader privilege-model redesign. I would appreciate your view on whether the shared-flag inheritance is the right default for upstream.
Summary by CodeRabbit
New Features
GRAVITYKIT_ALLOW_DELETE, with fallback to the existing Gravity Forms setting.Bug Fixes