Fail closed on the two publish-post preflights that only warned - #218
Merged
Conversation
A self-closing custom block one-liner is what Gutenberg's inserter produces, but it saves an EMPTY block when written through WP-CLI: the InnerBlocks template is hydrated client-side on insert, so nothing server-side ever expands it. Nothing errors, the block is absent from the rendered post, and a post_content diff shows nothing missing. publish-post only warned about this, and printed the warning next to the success output of a write that had already happened. verify-post has always treated the same condition as a hard FAIL — so the tool waved the draft through and then failed it afterwards, at the one point the mistake could no longer be prevented cheaply. Two posts on imagewize.com shipped with an empty CTA block in four days this way. Promote it to a refusal in both entry points, matching the near-empty body check that sits directly above it, with --allow-self-closing-blocks (allowSelfClosingBlocks on the MCP tool) for a genuinely dynamic, server-rendered block that is safe self-closed. The message now names the offending blocks, which verify-post already did and publish-post did not, and both entry points now use verify-post's pattern, so the attribute-less form is caught too. Closes #216
publish-post.sh flags four never-valid block prefixes; publishPost.ts flagged three. wp:nynaeve/ was missing from the MCP tool, which is the path in daily use. The prefix is never correct — nynaeve is a block category, not a namespace, and every block in that theme is imagewize/* — so any wp:nynaeve/x in a draft is a typo, of exactly the kind that was found and fixed repo-wide in August. Hoist the list into a named constant so the two entry points can be compared at a glance the next time one of them grows an entry.
imageAttachmentId and imagePath are documented as interchangeable ways to set the featured image, but only imagePath maintained the schema: injectArticleImage() sat inside the imagePath branch, so an attachment ID set _thumbnail_id and nothing else. The consequence is silent and one-directional. A post first published with imagePath has an `image` in its Article JSON-LD that the local draft never contained — wp-ops added it at publish time. Re-publishing that draft with imageAttachmentId, to avoid a duplicate attachment, overwrites the live post with a version that has no image field. Every check passes: publish_post compares source bytes to stored bytes and the source was already missing it, and verify_post counts JSON-LD blocks, which never changed. Hit on post 13755 on imagewize.com. Resolve the URL from the attachment (wp_get_attachment_url) and run the same injection, so both options end at one code path. A non-existent attachment ID is now an error rather than a bogus _thumbnail_id, and the "Featured image:" output line carries a URL for both options. Refs #217
Both halves of the toolchain verify that what was sent is what landed. Neither notices that what was sent is worse than what it replaced, and on an update that is the whole gap: publish-post compares source bytes to stored bytes, verify-post counts JSON-LD blocks, and a field vanishing from inside a block moves neither number. Drafts are the reason this matters. wp-ops enriches a post at publish time — the image URL is only known once the upload has happened — so the draft on disk is permanently poorer than the post it produced. Re-publishing that draft strips the enrichment back off, silently. It happened to `image` on post 13755; `datePublished`, `author` and `publisher` are strippable exactly the same way. Read the live post before writing, compare its Article JSON-LD keys against the draft's, and refuse an update that drops any, listing them. --allow-schema-regression / allowSchemaRegression: true overrides. The MCP tool does this in preflight so a dry run reports it before anything is written; the shell script does it inside the PHP worker, where the per-target `wp eval-file` run is the first thing that can see the target at all. Closes #217
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #216 and #217. Both are the same shape of problem: the preflight already knew enough to prevent a bad publish and let it through anyway, and every verification downstream reported success.
Self-closing custom blocks are now refused (#216)
The one-liner Gutenberg's inserter produces (
<!-- wp:imagewize/cta {...} /-->) saves an empty block when written through WP-CLI — theInnerBlockstemplate is hydrated client-side on insert, so nothing expands it server-side. Nothing errors, the block is missing from the rendered post, and apost_contentdiff shows nothing. Two posts on imagewize.com shipped with an empty CTA block in four days.verify-posthas always treated this as a hard FAIL, so the tool waved the draft through and then failed it afterwards, at the one point the mistake could no longer be prevented cheaply.publishPost.tsandpublish-post.shnow throw /exit 1, matching the near-empty-body check directly above it.verify-postalready did andpublish-postdidn't.verify-post's pattern now, so the attribute-less form (<!-- wp:ns/block /-->) is caught too — the publish-side regex previously required attributes.--allow-self-closing-blocks/allowSelfClosingBlocks: truepublishes anyway, for a genuinely dynamic, server-rendered block; it downgrades to a warning rather than disabling the check.Secondary: the MCP tool gained the
wp:nynaeve/bad-block prefixpublish-post.shalready had.nynaeveis a block category, not a namespace — every block isimagewize/*— so anywp:nynaeve/xis a typo. The list is now a named constant so the two entry points can be compared at a glance.imageAttachmentIdwrites the Article schema image (#217)injectArticleImage()sat inside theimagePathbranch, so an attachment ID set_thumbnail_idand nothing else, even though the two options are documented as interchangeable.A post first published with
imagePathcarries animagefield the local draft never had — wp-ops added it at publish time. A later content-only update passingimageAttachmentId(to avoid a duplicate attachment) therefore republished it without one, and every check passed:publish_postcompares source bytes to stored bytes and the source was already missing it;verify_postcounts JSON-LD blocks and the count never changed.The URL is now resolved with
wp_get_attachment_url()and both options end at the same injection. A non-existent attachment ID is an error rather than a bogus_thumbnail_id, and theFeatured image:output line carries a URL either way.An update that drops live Article schema fields is refused (#217)
Both tools verify that what was sent is what landed. Neither noticed that what was sent was poorer than what it replaced — and on an update, the previous state is the only state nobody looks at.
Since a draft is written before wp-ops enriches the post, re-publishing it strips that enrichment back off. It happened to
image;datePublished,authorandpublisherare strippable exactly the same way.publish-postnow reads the live post before writing and compares its Article JSON-LD keys against the draft's, refusing the update and listing what would be dropped.--allow-schema-regression/allowSchemaRegression: trueoverrides. The MCP tool checks in preflight sodryRun: truereports it before anything is written; the shell script checks inside the PHP worker, the first point at which a per-target run can see the target at all.Also
go/internal/catalog/catalog.jsonregenerated for the two new flags.mcp-server/README.mdand thepublish_posttool description updated.Verification
tsc --noEmit,npm run build,bash -n publish-post.sh,go test ./...— all pass. The generated PHP worker was linted withphp -l, and the PHP and TypeScript implementations of Article-key extraction and self-closing block-name extraction were checked against the same fixtures to confirm they agree. No live publish was exercised; that needs a real site.