36002 workflow fire convert block editor markdown to prosemirror json on save server side

[hassandotcms]

What

Converts Story Block (Block Editor) field values supplied as Markdown to Tiptap/ProseMirror JSON server-side, on the shared content save path. Non-interactive clients (AI agents, headless imports) no longer need a human to open and re-save the contentlet for the field to read back as structured content.

Closes #36002 (Markdown scope; see Scope below).

How

• TiptapMarkdown.isTiptapDoc / isMarkdownRepresentable — pure discriminators.
• MapToContentletPopulator.fillFields — the seam shared by the workflow fire endpoints and the content REST API. For a Story Block value:
  • starts with { (JSON) or < (HTML) → stored unchanged;
  • otherwise (Markdown) → converted via TiptapMarkdown.toTiptap.
• Guards: already-valid JSON passes through untouched; a Markdown update is rejected (400) when the existing stored doc contains rich blocks Markdown can't represent (dotContent, dotVideo, grid, …) instead of silently destroying them; a conversion failure never blocks the save (stores raw + logs).
• OpenAPI fire note corrected to reflect automatic server-side conversion (regenerated).

Scope

• Markdown only. HTML is deferred to a follow-up PR. HTML continues to pass through unchanged (no regression).

Behavior change

• A Markdown overwrite of rich content now returns 400 instead of a silent success that
  corrupted the field. Additive and rollback-safe (stored JSON is read natively by N-1).

Testing

• Unit (65): TiptapMarkdownDocDetectionTest (13) + existing TiptapMarkdownTest /
  RoundTripContractTest.
• Integration (5): StoryBlockMarkdownPopulatorTest — convert + GraphQL read-back, JSON
  passthrough, HTML passthrough, primitive replace, rich-overwrite reject.
• Regression re-run clean: MapToContentletPopulatorTest (20), StoryBlockValidationTest (28).

[claude]

Claude finished @hassandotcms's task in 1m 53s —— View job

---

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get full PR diff
• Analyze diff against all unsafe categories
• Applied label AI: Not Safe To Rollback and posted analysis comment

Result: 🟡 MEDIUM — Not Safe To Rollback

Category matched: M-3 — REST / GraphQL / Headless API Contract Change

Why: The save path for Story Block (Block Editor) fields now auto-converts Markdown input to ProseMirror JSON on write (MapToContentletPopulator.java lines 258–313). After N deploys, headless/AI-agent clients that submit Markdown and read back the field expect a structured ProseMirror JSON object. Rolling back to N-1 reverts to the old pass-through behavior, and those same clients receive a plain Markdown string — breaking their expectation. The Postman and Karate test updates confirm the response shape changes from a Markdown string to a ProseMirror JSON object.

No unsafe categories found in: database migrations, ES mapping changes, content JSON model version, DROP TABLE/COLUMN, runonce tasks, or any CRITICAL / HIGH category.

Safer alternative: The ProseMirror JSON written by N is natively readable by N-1, so no data-layer two-phase migration is needed. The recommendation is to document the Markdown auto-conversion as rollback-incompatible behavior in the release notes, advising consumers that this feature requires version N or above.

[github-actions]

🤖 Bedrock Review — deepseek.v3.2

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:261 — The condition value instanceof String may miss cases where the value is a non-String object that could be valid for a Story Block field (e.g., a JSON object). This could cause unexpected behavior or data loss if a client sends a structured JSON object instead of a JSON string.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:293 — The check trimmed.isEmpty() || trimmed.charAt(0) == '{' || trimmed.charAt(0) == '<' assumes HTML starts with '<', but a valid HTML string could have leading whitespace or be a different case (e.g., <!DOCTYPE>). This may cause HTML to be incorrectly stored as-is without conversion when it should be.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:298 — The method contentlet.getStringProperty(field.getVelocityVarName()) may return null if the property is not set, which could lead to a NullPointerException when passed to TiptapMarkdown.isTiptapDoc(existing). The method isTiptapDoc handles null, but the call !TiptapMarkdown.isMarkdownRepresentable(existing) could still throw if existing is null? Actually, isMarkdownRepresentable also handles null, so this is safe. However, the logic depends on the existing value being a Tiptap doc; if it's null, the condition TiptapMarkdown.isTiptapDoc(existing) will be false, skipping the rich content check. This might allow Markdown to overwrite a null field even if the field previously had rich content? No, because the existing value is null. This is acceptable, but the behavior should be clear.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:121 — The method isTiptapDoc uses value.stripLeading() which is available in Java 11+. dotCMS uses Java 25, so it's fine, but ensure compatibility with the project's minimum Java version if different.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:130 — The JSON parsing with MAPPER.readTree(value) could be expensive for large documents. Since this is called on every save for Story Block fields, consider performance impact. However, the early check trimmed.charAt(0) != '{' mitigates this for non-JSON values.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:152 — The set MARKDOWN_NODE_TYPES includes "dotImage" which is a custom block; ensure that dotImage is indeed representable in Markdown (likely as an image syntax). If not, this could incorrectly allow overwrites that lose information.

[🟠 High] dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:168 — The method isMarkdownRepresentable catches IOException and returns true for malformed JSON. This means a malformed JSON string will be considered representable, potentially allowing a Markdown overwrite that should be rejected. This could lead to data loss. The behavior should be consistent with isTiptapDoc which returns false for malformed JSON.

[🟡 Medium] dotcms-integration/src/test/java/com/dotcms/rest/StoryBlockMarkdownPopulatorTest.java:113 — The test uses Mockito.mock but does not verify any interactions. This is fine for integration, but ensure mocks are properly configured to avoid false positives.

[🟡 Medium] dotcms-integration/src/test/java/com/dotcms/rest/StoryBlockMarkdownPopulatorTest.java:176 — The test expects an IllegalArgumentException with a message containing "rich content". If the exception message changes, the test could fail. Consider matching the exception type only or using a custom matcher.

[🟡 Medium] dotcms-postman/src/main/resources/postman/BringBack.postman_collection.json:115 — The Postman test updates the expectation for the body field from a plain string to a structured JSON object. This is correct for the new behavior, but ensure all related Postman collections are updated accordingly to avoid breaking CI/CD tests.

[🟡 Medium] dotcms-postman/src/main/resources/postman/JsScriptAPI.postman_collection.json:25 — Similar update for body field expectations. Ensure consistency across all API test suites.

[🟡 Medium] test-karate/src/test/java/tests/defaults/CheckingJSONAttributes.feature:71 — The Karate test now expects body to be a structured object. This is correct, but verify that all Karate scenarios relying on the old string format are updated.

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:314 — The Logger.warn call logs the exception message e.getMessage() which could contain sensitive information or implementation details. Consider logging a generic message or using a sanitized exception.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/workflow/WorkflowResource.java:231 — The updated documentation states that Markdown is the only format converted, but the code also handles HTML (by storing as-is). The documentation should reflect that HTML is still accepted but not converted, to avoid confusion.

[🟡 Medium] dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml:18675 — The OpenAPI specification updates the description for Block Editor fields. Ensure that the change is consistent across all endpoints (it appears to be updated in multiple places, which is good).

---

_{Run: #28038347219 · tokens: in: 11064 · out: 1323 · total: 12387}

[github-actions]

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

• 🔴 Critical: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:275 — processStoryBlockField calls toStoryBlockJson which may throw IllegalArgumentException if the existing content contains rich blocks, but this exception is not caught or logged. This will cause the entire contentlet save to fail with a 500 error, even though the spec requires "conversion failure never blocks the save". The contract explicitly permits Markdown conversion failure to be silent — but this path is now a hard failure.
• 🔴 Critical: dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:127 — isTiptapDoc calls MAPPER.readTree(value) without validating that value is not null or empty after stripLeading(). If value is a string of only whitespace (e.g., " "), trimmed.isEmpty() returns true and returns false — correct. But if value is null, stripLeading() returns null, then trimmed.charAt(0) throws NullPointerException. This is reachable via processStoryBlockField when contentlet.getStringProperty(...) returns null (common for new contentlets). The method must guard against null before charAt(0).
• 🔴 Critical: dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:159 — isMarkdownRepresentable(String) catches IOException and returns true, but isMarkdownRepresentable(JsonNode) does not handle null input. If MAPPER.readTree() returns null (e.g., malformed JSON), isMarkdownRepresentable(null) is called, which returns true — correct. But if node.path("type") returns a non-string (e.g., number), .asText("") is safe. However, node.path("content") may return a non-array (e.g., null, string, number), and isArray() returns false, so the loop is skipped — correct. But if node is null, node.path("type") returns a missing node, .asText("") returns "", and !type.isEmpty() is false — correct. However, the method assumes node is a valid JSON object — but if readTree() returns null due to malformed JSON, isMarkdownRepresentable(null) is called and returns true. This is acceptable per spec. But the real bug is in isTiptapDoc: it does not guard against null before charAt(0), and processStoryBlockField does not catch IllegalArgumentException — both are critical.
• 🟠 High: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:284 — Logger.warn(...) uses this as the logger category, but MapToContentletPopulator is not annotated with @Logger and does not declare a Logger field. This will cause Logger.warn(null, ...) to be called, which logs to null category — effectively silent. Must use Logger.getLogger(MapToContentletPopulator.class) or declare private static final Logger log = Logger.getLogger(MapToContentletPopulator.class);.
• 🟠 High: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:275 — toStoryBlockJson calls TiptapMarkdown.toTiptap(value) which may throw Exception — but the catch block logs the message and returns the original value. However, if TiptapMarkdown.toTiptap throws an unchecked exception (e.g., NullPointerException from malformed input), it is caught and logged — correct. But if TiptapMarkdown.isTiptapDoc(existing) throws IOException (e.g., malformed existing JSON), it returns false, so the guard is skipped — this is acceptable per spec. But the real issue is the uncaught IllegalArgumentException in processStoryBlockField and the NullPointerException in isTiptapDoc.

Existing

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:284 — Logger.warn(this, ...) uses this as logger category — this was already broken in pre-existing code and remains broken. The fix should have used a static logger, but it didn't.

Resolved

• ✅ dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:118 — isTiptapDoc now guards against null and empty strings with stripLeading() and isEmpty() check — fixed the prior NullPointerException risk.
• ✅ dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:159 — isMarkdownRepresentable(String) now catches IOException and returns true — fixed prior risk of unhandled parse failure.
• ✅ dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:165 — isMarkdownRepresentable(JsonNode) now handles null input — fixed prior risk.
• ✅ dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — updated to reflect Markdown-only guidance — consistent with new behavior.
• ✅ dotcms-integration/src/test/java/com/dotcms/rest/StoryBlockMarkdownPopulatorTest.java — comprehensive integration tests added for all critical paths — closes test gap.
• ✅ dotCMS/src/test/java/com/dotcms/tiptap/TiptapMarkdownDocDetectionTest.java — unit tests added for isTiptapDoc and isMarkdownRepresentable — closes test gap.

---

_{Run: #28113820145 · tokens: in: 11031 · out: 1484 · total: 12515}

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-3 — REST / GraphQL / Headless API Contract Change
• Risk Level: 🟡 MEDIUM
• Why it's unsafe: The save path for Story Block (Block Editor) fields now auto-converts Markdown input to ProseMirror JSON on write. After N deploys, any headless or AI-agent client that submits Markdown and reads back the field expects a structured ProseMirror JSON object. Rolling back to N-1 reverts to the old pass-through behavior: Markdown is stored unchanged, and those clients receive a plain Markdown string instead of ProseMirror JSON — breaking their expectation. (Content already stored as ProseMirror JSON by N continues to read correctly on N-1, so the risk is narrowly limited to future Markdown submissions after rollback.)
• Code that makes it unsafe: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java lines 258-313 — the new processStoryBlockField / toStoryBlockJson methods that intercept Story Block values and convert Markdown to TiptapMarkdown.toTiptap(value).toString(). Test expectation changes in dotcms-postman/src/main/resources/postman/BringBack.postman_collection.json, JsScriptAPI.postman_collection.json, VersionableResource.postman_collection.json, and test-karate/src/test/java/tests/defaults/CheckingJSONAttributes.feature confirm the response shape changes from a Markdown string to a ProseMirror JSON object.
• Alternative (if possible): The change is inherently additive (ProseMirror JSON written by N is natively readable by N-1), so a two-phase approach is not required for data safety. To protect newly-written headless clients from a rollback, document the Markdown auto-conversion as a rollback-incompatible behavior in the release notes, and advise consumers that this feature is only available on version N and above.