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 overwrite is ignored (existing document preserved) 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 is now ignored and logged, preserving the existing document, 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 ignored.
• Regression re-run clean: MapToContentletPopulatorTest (20), StoryBlockValidationTest (28).

[claude]

Claude finished @hassandotcms's task in 2m 21s —— View job

---

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get the full PR diff
• Analyze diff against all unsafe categories
• Post results

---

Result: 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 (MapToContentletPopulator.java:269-327). After deploying version N, any headless client or AI agent that submits Markdown to a Story Block field will receive a ProseMirror JSON document back when reading the field (e.g., {"type":"doc","content":[...]}). Rolling back to N-1 reverts to the old pass-through behavior — Markdown is stored and returned as a raw string — breaking any client built against N's response shape.

Code that makes it unsafe:

• dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java lines 256–327 — the new processStoryBlockField / toStoryBlockJson methods that detect Markdown input and convert it via TiptapMarkdown.toTiptap(value).toString().
• Test contract changes confirm the shape change from string to ProseMirror JSON:
  • dotcms-postman/src/main/resources/postman/BringBack.postman_collection.json: 3 assertions changed from jsonData.entity.body == "Test content…" to jsonData.entity.body.content[0].content[0].text == "Test content…".
  • test-karate/src/test/java/tests/defaults/CheckingJSONAttributes.feature line 71: removed the "body": "This is my Test Generic Content" string assertion and replaced it with match response.entity.body.type == 'doc' and match response.entity.body.content[0].content[0].text.

Alternative:

The change is inherently additive (ProseMirror JSON written by N is natively readable by N-1 as structured content). No two-phase DB migration is required. To protect newly-built 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.

Label AI: Not Safe To Rollback has been added to this PR.

[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.

[fabrizzio-dotCMS]

Generally looks OK
I would only double-check that the illegalStateException is really necessary

[github-actions]

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

New Issues

• 🔴 Critical: dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:127 — isTiptapDoc calls trimmed.charAt(0) on trimmed after stripLeading() but does not re-check trimmed.isEmpty() after the null check — if value is null, stripLeading() returns "", and charAt(0) throws StringIndexOutOfBoundsException
• 🔴 Critical: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:275 — processStoryBlockField calls toStoryBlockJson which throws IllegalArgumentException on rich content, but this exception is not caught — causes save to fail despite spec requiring silent fallback (same as prior finding, now re-introduced)
• 🟠 High: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:284 — Logger.warn(this, ...) uses instance this instead of static Logger — causes silent logging due to null category (same as prior finding, now re-introduced)

Existing

• 🔴 Critical: dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:127 — isTiptapDoc calls trimmed.charAt(0) on null after stripLeading() when value is null, causing NPE (still present — fix was not applied)
• 🔴 Critical: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:275 — processStoryBlockField throws IllegalArgumentException that is not caught, causing save to fail despite spec requiring silent fallback (still present — fix was not applied)
• 🔴 Critical: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:284 — Logger.warn uses 'this' instead of static Logger, causing silent logging due to null category (still present — fix was not applied)
• 🟠 High: dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:284 — Logger.warn uses 'this' instead of static Logger, causing silent logging due to null category (still present — fix was not applied)

Resolved

• ✅ dotCMS/src/main/java/com/dotcms/tiptap/TiptapMarkdown.java:118 — prior NPE in isTiptapDoc was fixed by adding if (value == null) return false; — but this was undone in the diff by removing the null check before stripLeading()
• ✅ dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:275 — prior IllegalArgumentException not caught was fixed in prior PR — now re-introduced by new code path
• ✅ dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:284 — prior Logger.warn(this, ...) was fixed in prior PR — now re-introduced by unchanged line

---

_{Run: #28191562589 · tokens: in: 10246 · out: 1092 · total: 11338}

[github-actions]

🤖 dotBot Review (Bedrock)

Reviewed 9 file(s); 2 candidate(s) → 2 confirmed, 0 uncertain (unverified, kept for review).

Confirmed findings

• 🟠 High dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml:18898 — OpenAPI documentation does not reflect 400 error for Markdown overwrites
  The OpenAPI documentation states that Markdown values are 'ignored' when existing content has unsupported rich blocks, but the PR's behavior change introduces a 400 error in this scenario. The documentation in line 18898 of openapi.yaml does not mention the 400 response, creating a discrepancy between actual API behavior and documentation.
• 🟡 Medium dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:302 — Potential NullPointerException when existing content is null
  In MapToContentletPopulator.java line 302, existingVal could be null for new contentlets. The method TiptapMarkdown.isTiptapDoc is called with existingVal, which may not handle null inputs, leading to a NullPointerException. The code should check if existingVal is null before passing it to isTiptapDoc.

---

_{us.deepseek.r1-v1:0 · Run: #28449277404 · tokens: in: 37901 · out: 10198 · total: 48099 · calls: 12 · est. ~$0.106}

[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 (e.g., {"type":"doc","content":[...]}). 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 269–327 — 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 (3 assertions changed from body as a string to body.content[0].content[0].text) and test-karate/src/test/java/tests/defaults/CheckingJSONAttributes.feature (match response.entity.body.type == 'doc') 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.

[fabrizzio-dotCMS]

I'm approving it but Im keeping my concerns about raising the IllegalStateException

[hassandotcms]

I'm approving it but Im keeping my concerns about raising the IllegalStateException

now we are logging, not throwing and interrupting the save flow.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request