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 48s —— View job

---

Rollback Safety Analysis

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

---

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 plain-text (Markdown) input to a ProseMirror JSON object on write. After deploying N, any headless or AI-agent client that submits a plain-text body and reads it back now receives a structured {"type":"doc","content":[...]} object instead of the original plain string. Rolling back to N-1 reverts to the old pass-through behavior: Markdown is stored unchanged, and those clients receive a raw string again — breaking any client that was already adapted to the new structured response shape. Content already stored as ProseMirror JSON by N continues to read correctly on N-1, so the risk is narrowly limited to clients that begin relying on the new structured read-back after N is deployed.
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java lines 256–319 — the new processStoryBlockField / toStoryBlockJson methods intercept every Story Block value on the shared save path and convert plain text (Markdown) to TiptapMarkdown.toTiptap(value).toString(), permanently changing what is stored and subsequently returned by the REST/GraphQL API.
  • dotcms-postman/src/main/resources/postman/BringBack.postman_collection.json lines 115, 192, 328 — response assertions changed from pm.expect(jsonData.entity.body).to.eql("Test content…") (a string) to pm.expect(jsonData.entity.body.content[0].content[0].text).to.eql("Test content…") (a nested JSON object), confirming the response shape has changed.
  • test-karate/src/test/java/tests/defaults/CheckingJSONAttributes.feature lines 69–71 — the previous "body": "This is my Test Generic Content" string assertion is replaced with match response.entity.body.type == 'doc' + match response.entity.body.content[0].content[0].text == ..., confirming the API now returns a JSON object for Story Block fields rather than a plain string.
• Alternative (if possible): The change is inherently additive at the data layer — ProseMirror JSON written by N is natively readable by N-1 — so a two-phase data migration is not needed for storage safety. To protect newly-adapted headless clients from a rollback, document the Markdown auto-conversion as a rollback-incompatible behavior in the release notes and advise consumers that the structured response shape is only available on version N and above. Clients relying on the new JSON response shape should implement a typeof body === 'object' guard as a defensive measure.

Label AI: Not Safe To Rollback has been applied.

[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

[fabrizzio-dotCMS]

Isn't this a bit too aggressive?? Perhaps all we need to do is log the error, but not interrupt the application flow

[fabrizzio-dotCMS]

Perhaps all we need to do here is log the error or a Warning

[fabrizzio-dotCMS]

test first, and the last chars seem a bit more like a solid test

[fabrizzio-dotCMS]

or empty

[fabrizzio-dotCMS]

Should we add some logging here ? in case of a failure

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