fix(client): fall back to X-HTTP-Method-Override when a host 405s PUT/PATCH/DELETE - #67
Conversation
…/PATCH/DELETE Some managed hosts (Convesio among them) front WordPress with a WAF that answers PUT, PATCH, and DELETE with a bare nginx 405 before the request reaches PHP. GET and POST pass, so reads and create_post kept working while every editing tool failed: update_block, update_blocks, delete_block, rewrite_post_blocks, update_post, and the two Yoast writers — eight call sites in all. The `?rest_route=` form is what the WAF singles out: the same PUT against the pretty `/wp-json/` path reaches WordPress fine. Switching to pretty permalinks was rejected because `?rest_route=` exists precisely so tool calls don't 404 on plain-permalink sites (see src/rest-url.ts). WordPress core honours `X-HTTP-Method-Override` on a POST, so a rejected request is replayed in that shape and the host is remembered for the life of the client, leaving later writes a single round-trip. The fallback is adaptive rather than blanket: hosts that accept the real verbs never see an override header, so the change is inert everywhere it isn't needed. The editing routes are registered as literal PUT / PATCH / DELETE rather than the EDITABLE alias, so a plain POST would not match them; the override header is what carries the intended verb through. Verified against production: update_post on a docs post returned 405 before and succeeds after. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
…EADME Broadens the fallback coverage from 6 cases to 16. New cases pin the parts a naive replay gets wrong: the request body, query parameters, and Basic auth all have to survive the replay, or a write silently lands empty or unauthenticated. Also pins the boundaries. The fallback must not engage for a 405 on POST (not an override verb) or for a non-405 failure on PATCH, since masking a real 403 behind a replay would hide a permission problem. When a host rejects the replay too, the error surfaces after exactly two attempts rather than looping, and the override survives a 429 backoff retry so the two interceptors compose. Reverting src/client.ts fails 11 of the 16; the 5 that still pass are the ones asserting the fallback stays dormant (three permissive-host cases, two scope guards), which is the signature we want. README: adds a 405 section to Error Codes explaining that the status comes from the host's firewall rather than the plugin, that the client replays such requests itself, and what to ask the host for when even the replay is rejected. Test counts were stale (257/335); verified counts are 885 Vitest and 1,440 PHPUnit. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
WalkthroughThe client detects edge HTTP 405 responses for ChangesMethod Override Fallback
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client as WordPressBlockClient
participant Host as Edge host
participant API as WordPress API
Client->>Host: Send PUT/PATCH/DELETE request
Host-->>Client: Return HTTP 405
Client->>Host: Replay as POST with X-HTTP-Method-Override
Host->>API: Forward overridden method
API-->>Client: Return response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client.ts`:
- Around line 260-269: Update the retry condition in the request error handling
around edgeRejectedVerb so every rejected original PUT/PATCH/DELETE request is
replayed, rather than gating replay on !this.useMethodOverride. Preserve
METHOD_OVERRIDE_VERBS.has(method) as the loop-prevention check, and add a
concurrent parallel-write regression case where both original requests receive
405 responses and are replayed successfully.
In `@wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs`:
- Around line 40163-40167: Update the retry condition in the request handling
branch around edgeRejectedVerb so every eligible original-method 405 with
config3 is replayed, even when useMethodOverride is already enabled; keep
enabling the flag only when needed, and return client.request(config3) for all
such responses. The replay uses POST and must not recurse through this
condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1fc336e3-f360-442f-ae64-9f07bc7f3350
📒 Files selected for processing (5)
README.mdsrc/client.tstests/client-method-override.test.tswordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/readme.txt
Concurrent writes all go out as real verbs, because none of them has seen a 405 yet. Gating the replay on `!useMethodOverride` meant the first rejection set the flag and replayed while the rest fell through, surfacing a 405 the caller could do nothing about. The flag was never what prevented recursion: a replay carries method `post`, which is not an override verb, so it cannot re-enter the branch. Requests issued after the flag is set are converted by the request interceptor and likewise arrive as `post`. Dropping the guard therefore only affects the race window. Caught by CodeRabbit on #67. The added test fails against the guarded client and passes here. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
|
Good catch on the concurrency race, fixed in 46b1c6e. You're right that the flag was never what prevented recursion. A replay carries method Dropped the guard and added a regression test that fires three concurrent writes: it fails against the guarded client (one rejects with a 405) and passes now, with all three replayed. |
Fixes BLOCK-41.
The bug
Every editing tool fails against gravitykit.com (Convesio) with an opaque
Block API Error (405)carrying an nginx HTML body, while reads andcreate_postkeep working. That asymmetry makes it read like a permissions or tool problem rather than a transport one. Eight call sites are affected:update_block,update_blocks,delete_block,rewrite_post_blocks,update_post,edit_block_tree,yoast_update_seo,yoast_bulk_update_seo.Root cause
Convesio's WAF rejects
PUT/PATCH/DELETEon the?rest_route=form, before the request reaches PHP. The same verb against the pretty/wp-json/path is fine:PUT /wp-json/gk-block-api/v1/posts/{id}/blocksPUT /?rest_route=/gk-block-api/v1/posts/{id}/blocksOPTIONSon the routeallow: GET, POST, PUTPOST+X-HTTP-Method-Override: PUTvia?rest_route=POST+X-HTTP-Method-Override: DELETEvia?rest_route=src/client.tsbuilds every request on?rest_route=viarestRouteUrl(), which exists deliberately so tool calls don't 404 on plain-permalink sites.GETandPOSTpass the WAF, which is exactly why reads and creates kept working.The fix
Fall back to
POST+X-HTTP-Method-Overridewhen one of those verbs returns 405, cached per client instance so later writes take a single round-trip.PUT/PATCH/DELETErather than theEDITABLEalias, so POST alone does not match them. The override header carries the intended verb.?rest_route=exists to survive plain-permalink sites; switching would trade this bug for the 404 bug it prevents.POST, not an override verb, so a second 405 surfaces after exactly two attempts.Verification
src/client.tsfails 11 of 16; the 5 that still pass are the ones asserting the fallback stays dormant (3 permissive-host, 2 scope guards).update_poston a docs post returned 405 before and succeeds after.Also in this PR
README.md: new 405 section under Error Codes (the status comes from the host's firewall, not the plugin, and the client now handles it), plus corrected test counts (the stated 257/335 were stale).readme.txt: changelog entry under= develop =.assets/mcp-server/index.cjs, since CI does not rebuild the server bundle.https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
Summary by CodeRabbit
New Features
X-HTTP-Method-Overrideheader for intended PUT/PATCH/DELETE verbs.Documentation
Tests
💾 Build file (46b1c6e).