From 8b1483e6013d35d4963eacc7c3324955161bf378 Mon Sep 17 00:00:00 2001 From: Dirk Date: Thu, 13 Aug 2026 00:08:22 +0100 Subject: [PATCH] fix(ui): render board approval payload prose as markdown (#9817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The approval queue is how agents surface decisions that need a human, so the approval card is often the only thing an operator reads before approving or rejecting > - Agents author those payloads in markdown, because markdown is what they produce everywhere else in the product > - `ApprovalPayload.tsx` renders the four prose fields of a board approval as bare text nodes, while `CommentThread` on the same page renders through `MarkdownBody` — same authors, two different renderers > - So the operator sees literal `##`, `**bold**`, backticks and `[link](url)` in the payload, and correctly formatted text in the comments directly below it > - This pull request routes those four fields through the existing `MarkdownBody` component > - The benefit is that the highest-stakes text in the product becomes readable, with no new dependency and no schema change ## Linked Issues or Issue Description Refs #4911 — prior art, see the note at the bottom of this description. No open issue covers this, so per (B) here is the bug report: **What happened:** On a board approval, the `summary`, `recommendedAction`, `nextActionOnApproval` and `risks` fields display raw markdown source. Headings appear as literal `##` mid-paragraph, inline code keeps its backticks, links show as `[text](url)`, and both levels of a nested bullet list collapse into one run-on paragraph. **What was expected:** The same rendering the comment thread further down the same page already gives, since both are agent-authored markdown. **Steps to reproduce:** Open any `request_board_approval` whose `summary` contains markdown — headings, a nested list, code spans or links. **Where:** `ui/src/components/ApprovalPayload.tsx`, `BoardApprovalPayloadContent`. ## What Changed - `ui/src/components/ApprovalPayload.tsx`: import `MarkdownBody` and render `summary`, `recommendedAction`, `nextActionOnApproval` and each `risks` entry through it instead of `

` / `` text nodes. `MarkdownBody` defaults `softBreaks` to `true`, which is the same behaviour `CommentThread` opts into explicitly, so paragraph handling matches the comments. - `stripLeadingListMarker`: risks already render inside a custom bullet row, so an authored leading `-` / `*` / `•` would nest a second bullet inside the first. One leading marker is stripped per entry. - The risk bullet dot gains `shrink-0` so it keeps its shape next to block-level markdown content. - `title` stays plain text — it is a one-liner and markdown there is noise. - `proposedComment` stays a verbatim `

` block — it is draft text
intended to be posted elsewhere, so it must not be reinterpreted.
- `ui/src/components/ApprovalPayload.test.tsx`: tests for markdown
rendering in all four fields, the leading-list-marker strip, and that
`title` and `proposedComment` remain verbatim.

## Verification

- `npx vitest run ui/src/components/ApprovalPayload.test.tsx` — 5
passed.
- `npx vitest run ui/src/components/ApprovalPayload.test.tsx
ui/src/components/CommentThread.test.tsx` — 12 passed, confirming the
shared `MarkdownBody` path is not disturbed.
- Manual, measured rather than eyeballed: I ran a patched build in a
throwaway container beside an unpatched one and pointed both at the same
real approval payload, then counted nodes in the rendered DOM.

  | | unpatched | patched |
  |---|---|---|
  | `.paperclip-markdown` nodes | 0 | 3 |
  | raw backticks in visible text | yes | no |
  | rendered `h2` | 0 | 5 |
  | rendered `li` | 0 | 16 |

## Risks

Low, and confined to the board approval card.

- Rendering scope widens from text to markdown on four fields. A payload
that contains markdown-significant punctuation but was authored as prose
could render differently than before. This is the intended change, and
it matches how the same author's text is already rendered in comments on
the same page.
- `stripLeadingListMarker` removes one leading list marker per risk
entry. A risk that genuinely begins with a literal hyphen followed by a
space loses that hyphen. Chosen over the alternative of a visible double
bullet on the common case.
- No schema change, no migration, no new dependency. `MarkdownBody` is
already used elsewhere in the same directory.

## Model Used

Claude Opus 4.8 (`claude-opus-4-8`), via Claude Code, with extended
thinking and tool use (repository search, file editing, local test
execution, and headless-browser DOM measurement of the before/after
renders).

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes — no
docs describe this rendering behaviour, so there was nothing to update
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green — pending first CI run on this PR
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups —
pending first review
- [x] I will address all Greptile and reviewer comments before
requesting merge

---

### On the prior PR

@alxhrzg opened #4911 for this same bug first, and reached the same
conclusion I did: route the four fields through `MarkdownBody`. Credit
for spotting it and for the diagnosis goes there.

That PR has been conflicting against base and untouched since May.
Rather than let the fix sit, this PR reapplies the idea on current
`master` and adds what #4911 was missing: test coverage, the
nested-bullet fix for `risks`, and the `shrink-0` on the bullet dot. I
could not push to #4911 directly as it is on another contributor's fork.

@alxhrzg, if you would rather finish #4911 yourself, I am happy to close
this and hand over the tests and the two risk-row fixes for you to take
across.

---------

Co-authored-by: Claude Opus 4.8 
---
 ui/src/components/ApprovalPayload.test.tsx | 162 ++++++++++++++++++---
 ui/src/components/ApprovalPayload.tsx      |  25 +++-
 2 files changed, 160 insertions(+), 27 deletions(-)

diff --git a/ui/src/components/ApprovalPayload.test.tsx b/ui/src/components/ApprovalPayload.test.tsx
index c11405e92d..9533947473 100644
--- a/ui/src/components/ApprovalPayload.test.tsx
+++ b/ui/src/components/ApprovalPayload.test.tsx
@@ -4,6 +4,7 @@ import { act } from "react";
 import { createRoot } from "react-dom/client";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
 import { ApprovalPayloadRenderer, approvalLabel } from "./ApprovalPayload";
+import { ThemeProvider } from "../context/ThemeContext";
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 (globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
@@ -35,17 +36,19 @@ describe("ApprovalPayloadRenderer", () => {
 
     act(() => {
       root.render(
-        ,
+        
+          
+        ,
       );
     });
 
@@ -62,19 +65,140 @@ describe("ApprovalPayloadRenderer", () => {
     });
   });
 
+  it("renders markdown in board approval prose fields", () => {
+    const root = createRoot(container);
+
+    act(() => {
+      root.render(
+        
+          
+        ,
+      );
+    });
+
+    const bodies = container.querySelectorAll(".paperclip-markdown");
+    expect(bodies.length).toBe(4);
+
+    const summary = bodies[0];
+    expect(summary.querySelector("strong")?.textContent).toBe("Bold");
+    expect(summary.querySelector("code")?.textContent).toBe("code");
+    const link = summary.querySelector("a");
+    expect(link?.getAttribute("href")).toBe("https://example.com");
+    expect(link?.textContent).toBe("a link");
+
+    // The raw markdown characters must not survive into the rendered text.
+    expect(container.textContent).not.toContain("**Bold**");
+    expect(container.textContent).not.toContain("[a link](https://example.com)");
+
+    expect(bodies[1].querySelector("strong")?.textContent).toBe("frog");
+    expect(bodies[2].querySelector("code")?.textContent).toBe("frog");
+    expect(bodies[3].querySelector("strong")?.textContent).toBe("frog");
+
+    act(() => {
+      root.unmount();
+    });
+  });
+
+  it("does not nest a second bullet when a risk is authored as a markdown list item", () => {
+    const root = createRoot(container);
+
+    act(() => {
+      root.render(
+        
+          
+        ,
+      );
+    });
+
+    const bodies = container.querySelectorAll(".paperclip-markdown");
+    expect(bodies.length).toBe(5);
+    for (const body of bodies) {
+      expect(body.querySelector("ul")).toBeNull();
+      expect(body.querySelector("ol")).toBeNull();
+      expect(body.querySelector("li")).toBeNull();
+    }
+
+    expect(bodies[0].querySelector("strong")?.textContent).toBe("Leading dash");
+    expect(container.textContent).toContain("Leading star risk.");
+    expect(container.textContent).toContain("Leading dot risk.");
+    expect(container.textContent).toContain("Leading number risk.");
+    expect(container.textContent).toContain("Leading paren risk.");
+    expect(container.textContent).not.toContain("- **Leading dash**");
+
+    act(() => {
+      root.unmount();
+    });
+  });
+
+  it("renders every risk when two entries collapse to the same text after marker stripping", () => {
+    const root = createRoot(container);
+    const errors: unknown[] = [];
+    const originalError = console.error;
+    console.error = (...args: unknown[]) => {
+      errors.push(args);
+    };
+
+    try {
+      act(() => {
+        root.render(
+          
+            
+          ,
+        );
+      });
+
+      expect(container.querySelectorAll(".paperclip-markdown").length).toBe(2);
+      expect(errors).toEqual([]);
+    } finally {
+      console.error = originalError;
+      act(() => {
+        root.unmount();
+      });
+    }
+  });
+
   it("can hide the repeated title when the card header already shows it", () => {
     const root = createRoot(container);
 
     act(() => {
       root.render(
-        ,
+        
+          
+        ,
       );
     });
 
diff --git a/ui/src/components/ApprovalPayload.tsx b/ui/src/components/ApprovalPayload.tsx
index 165ee8a223..6f8f9752d8 100644
--- a/ui/src/components/ApprovalPayload.tsx
+++ b/ui/src/components/ApprovalPayload.tsx
@@ -1,4 +1,5 @@
 import { UserPlus, Lightbulb, ShieldAlert, ShieldCheck } from "lucide-react";
+import { MarkdownBody } from "./MarkdownBody";
 import { formatCents } from "../lib/utils";
 
 export const typeLabel: Record = {
@@ -161,11 +162,19 @@ export function BoardApprovalPayload({
   );
 }
 
+/**
+ * Risks render inside a custom bullet row, so a leading markdown list marker
+ * would nest a second bullet inside the first. Strip one leading marker.
+ */
+function stripLeadingListMarker(value: string): string {
+  return value.replace(/^(?:[-*•]|\d+[.)])\s+/, "");
+}
+
 function BoardApprovalPayloadContent({ payload }: { payload: Record }) {
   const risks = Array.isArray(payload.risks)
     ? payload.risks
         .filter((value): value is string => typeof value === "string")
-        .map((value) => value.trim())
+        .map((value) => stripLeadingListMarker(value.trim()))
         .filter(Boolean)
     : [];
   const title = firstNonEmptyString(payload.title);
@@ -185,7 +194,7 @@ function BoardApprovalPayloadContent({ payload }: { payload: Record
           
Summary

-          
{summary}

+          {summary}
         
       )}
       {recommendedAction && (
@@ -193,23 +202,23 @@ function BoardApprovalPayloadContent({ payload }: { payload: Record
             Recommended action
           

-          
{recommendedAction}

+          {recommendedAction}
         
       )}
       {nextActionOnApproval && (
         

           
On approval

-          
{nextActionOnApproval}

+          {nextActionOnApproval}
         

       )}
       {risks.length > 0 && (
         

           
Risks

           
    
-            {risks.map((risk) => (
-              
  • 
-                
-                {risk}
+            {risks.map((risk, index) => (
+              
  • 
+                
+                {risk}
               
    • 
             ))}