From 1eec8b764c8d777ea85f807c85aa89957d19a8c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Tue, 18 Aug 2026 23:04:21 +0200 Subject: [PATCH 1/4] Add a safe markdown renderer for chat text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assistant replies reach the browser as HTML-escaped plain text, so the markdown the model emits — **bold**, 1. lists, backticks — displays as literal characters. This adds the renderer only; nothing calls it yet. Two layers, and both are required. Commonmarker parses with unsafe: false, which replaces raw HTML with a placeholder comment and empties javascript:, data: and vbscript: URLs in the parser itself. Rails::HTML5::SafeListSanitizer then reduces the result to `p br strong em code pre ul ol li` with no attributes at all. Neither suffices alone: given the sanitizer strips the tag but leaves the CSS visible as text, and the parser is one config flag away from unsafe: true. That matters more than usual here, because content_security_policy.rb sets script_src to :self, :https with no strict-dynamic — any https origin is an allowed script source, so CSP would not stop an injected -> ""). It stays as a third guard on the one flag this file warns about — if + # unsafe ever flips to true, the worst tags are still escaped rather than live. + OPTIONS = { + render: { unsafe: false, hardbreaks: true }, + extension: { + header_ids: nil, + tasklist: false, + table: false, + autolink: false, + strikethrough: true, + shortcodes: false, + tagfilter: true + } + }.freeze + + PLUGINS = { syntax_highlighter: nil }.freeze + + # Minimal on purpose. Anything not listed loses its tag but keeps its text, so + # headings, blockquotes and strikethrough flatten to plain prose. Omitting + # is what drops markdown images; omitting is what drops links. + # + # 
 is here because it is the only reliable marker for "this code is a block
+  # rather than a span": a fence inside a list item lands as a child of 
  • , so
+  # position cannot tell the two apart. It carries no attributes past ATTRIBUTES
+  # (lang="ruby" is stripped), so allowing it adds no attack surface.
+  TAGS = %w[p br strong em code pre ul ol li].freeze
+
+  # No attributes at all — nothing in TAGS needs one.
+  ATTRIBUTES = [].freeze
+
+  SANITIZER = Rails::HTML5::SafeListSanitizer.new
+
+  def self.render(text)
+    return "".html_safe if text.blank?
+
+    html = Commonmarker.to_html(text.to_s, options: OPTIONS, plugins: PLUGINS)
+    SANITIZER.sanitize(html, tags: TAGS, attributes: ATTRIBUTES).to_s.html_safe
+  end
+end
diff --git a/test/lib/markdown_test.rb b/test/lib/markdown_test.rb
new file mode 100644
index 0000000..1339dc0
--- /dev/null
+++ b/test/lib/markdown_test.rb
@@ -0,0 +1,231 @@
+require "test_helper"
+
+class MarkdownTest < ActiveSupport::TestCase
+  # Every construct this suite feeds the renderer, reused by the standing
+  # invariants at the bottom — no anchors, no inline styles, no spans, ever.
+  SAMPLES = [
+    "**bold**",
+    "*italics*",
+    "1. **A** first\n\n2. **B** second",
+    "- one\n- two",
+    "use `x = 1` here",
+    "```ruby\ndef x\n  1\nend\n```",
+    "### Plan",
+    "~~gone~~",
+    "> quoted",
+    "| a | b |\n|---|---|\n| 1 | 2 |",
+    "- [ ] todo\n- [x] done",
+    ":tada:",
+    "",
+    %q{},
+    "![x](https://example.com/a.png)",
+    "[click](https://example.com)",
+    "[click](javascript:alert(1))",
+    "https://example.com/plain",
+    %q{
    x
    },
+    ""
+  ].freeze
+
+  # --- Rendering -----------------------------------------------------------
+
+  test "bold becomes strong" do
+    assert_equal "
    hi
    \n", Markdown.render("**hi**")
+  end
+
+  test "italics become em" do
+    assert_equal "
    hi
    \n", Markdown.render("*hi*")
+  end
+
+  # The shape the chat agent actually produces: blank line between items, which
+  # makes the list loose and wraps each item's content in 
    .
+  test "loose ordered list wraps items in p" do
+    html = Markdown.render("1. **A** first\n\n2. **B** second")
+
+    assert_match %r{
      }, html
+    assert_match %r{
    1. \s*
      A first
      \s*
      2. }, html
+    assert_match %r{
    2. \s*
      B second
      \s*
      3. }, html
+  end
+
+  test "bullets become ul and li" do
+    assert_equal "
        \n
      • one
        • \n
      • two
        • \n
      \n", Markdown.render("- one\n- two")
+  end
+
+  test "inline code becomes code" do
+    assert_equal "
      use x = 1 here
      \n", Markdown.render("use `x = 1` here")
+  end
+
+  # 
       is the block marker the CSS keys on — verify it survives the allowlist
+  # at top level, and that the info string's lang attribute does not.
+  test "fenced code renders as pre wrapping code" do
+    assert_equal "
      def x\n  1\nend\n
      \n",
+                 Markdown.render("```ruby\ndef x\n  1\nend\n```")
+  end
+
+  # The case position alone cannot detect: a child of 
    3. , neither a child of
+  # the body nor nested in a 
      .
+  test "fenced code inside a list item also renders as pre wrapping code" do
+    html = Markdown.render("1. do:\n\n   ```\n   x = 1\n   y = 2\n   ```")
+
+    assert_match %r{
    4. .*
      x = 1\ny = 2\n
      .*
      5. }m, html
+  end
+
+  # Guards render.hardbreaks — the current look depends on single newlines
+  # surviving as 
      .
+  test "single newlines survive as br" do
+    assert_equal "
      a
      \nb
      \n", Markdown.render("a\nb")
+  end
+
+  test "blank input renders empty and html_safe" do
+    [ nil, "", "   " ].each do |blank|
+      rendered = Markdown.render(blank)
+
+      assert_equal "", rendered
+      assert_predicate rendered, :html_safe?
+    end
+  end
+
+  test "rendered output is html_safe" do
+    assert_predicate Markdown.render("**hi**"), :html_safe?
+  end
+
+  # --- Flattening (the minimal-allowlist contract) -------------------------
+
+  test "heading renders as plain text with no h3 and no anchor" do
+    html = Markdown.render("### Plan")
+
+    assert_includes html, "Plan"
+    refute_includes html, " quoted")
+
+    assert_includes html, "
      quoted
      "
+    refute_includes html, " is stripped.
+  test "task list keeps its literal markers and emits no input element" do
+    html = Markdown.render("- [ ] todo\n- [x] done")
+
+    assert_includes html, "[ ] todo"
+    assert_includes html, "[x] done"
+    refute_includes html, "alert(1)")
+
+    refute_includes html, "})
+
+    refute_includes html, "click      }
+    ].each do |source|
+      html = Markdown.render(source)
+
+      refute_includes html, "javascript"
+      refute_includes html, "href"
+    end
+  end
+
+  test "entity-smuggled script stays escaped text" do
+    html = Markdown.render("<script>alert(1)</script>")
+
+    assert_includes html, "<script>"
+    refute_includes html, "x
      })
+
+    refute_includes html, "style"
+    refute_includes html, "onclick"
+  end
+
+  # Layer 2 alone would strip the "), "display:none"
+  end
+
+  test "fence info string cannot inject an attribute" do
+    html = Markdown.render(%Q{```">\nx\n```})
+
+    assert_equal "
      x\n
      \n", html
+  end
+
+  # --- Standing invariants over every sample -------------------------------
+
+  # Guards plugins.syntax_highlighter — the default injects
+  # style="background-color:#2b303b" and .
+  test "output never contains an inline style or a span" do
+    SAMPLES.each do |source|
+      html = Markdown.render(source)
+
+      refute_includes html, "style=", "inline style survived for #{source.inspect}"
+      refute_includes html, ", no autolink extension, no
+  # post-pass. Nothing may produce an anchor.
+  test "output never contains an anchor tag" do
+    SAMPLES.each do |source|
+      refute_includes Markdown.render(source), "
Date: Tue, 18 Aug 2026 23:04:37 +0200
Subject: [PATCH 2/4] Render assistant chat replies as formatted markdown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Wires the renderer into the message partial behind a streaming gate, ships the
.msg-prose component, and tells the agent which subset to emit.

Parsing happens once, on the final render. Three broadcast paths reach the
partial, and each passes streaming: explicitly rather than inferring state from
a billing column: mid-stream and the create append stay plain, the final replace
formats. The append needs the gate even though the row is empty when it commits,
because broadcast_append_later_to renders inside
Turbo::Streams::ActionBroadcastJob 100-300ms later from a GlobalID reload, by
which point the streaming job has written chunks. It is also the render the user
sees first: every replace fired before it targets a dom_id not yet in the DOM,
and Turbo drops those silently. Ungated, it would format half-written markdown
on every reply.

One path the gate cannot reach is a page load landing mid-reply, which formats a
half-written message. It self-repairs on the next chunk and the window is a few
seconds, so it is accepted rather than fixed; the deterministic fix wants a
terminal-state column on messages, which cancel/retry would need anyway.

.msg-prose must stay after .msg-body in the stylesheet — both are single-class
selectors on the same element, so source order breaks the white-space tie.
.msg-body also gains overflow-wrap: anywhere, so long paths and URLs stop
overflowing the bubble on every path, formatted or not. Fenced blocks get
white-space: pre and scroll horizontally instead of breaking a line mid-token.

The prompt gains one bullet naming the supported subset and asking for bare
URLs. It opens with "write short paragraphs", which sits in tension with the two
rules that keep a tool call prose-free, so turn-taking was re-checked against
the live model: the tool call still arrives with empty content, and the
post-tool response is still empty.

Tests cover all three broadcast paths — the two that must stay plain and the one
that must format — plus the helper's three-way branch, the page-load path, and
the case where a tool-call message also carries prose, which the partial can
represent regardless of what the model does.
---
 app/assets/tailwind/application.css           | 53 ++++++++++++++++++-
 app/helpers/messages_helper.rb                | 19 +++++++
 app/jobs/chat_respond_job.rb                  |  5 +-
 app/models/message.rb                         |  7 ++-
 .../generator_agent/instructions.txt.erb      |  1 +
 app/views/messages/_message.html.erb          |  3 +-
 docs/02-architecture/04-design-system.md      |  2 +-
 .../projects_controller_show_test.rb          | 35 ++++++++++++
 test/helpers/messages_helper_test.rb          | 43 +++++++++++++++
 test/jobs/chat_respond_job_test.rb            | 24 +++++++++
 test/models/message_test.rb                   | 32 +++++++++++
 11 files changed, 219 insertions(+), 5 deletions(-)

diff --git a/app/assets/tailwind/application.css b/app/assets/tailwind/application.css
index 51c7e4e..0375bee 100644
--- a/app/assets/tailwind/application.css
+++ b/app/assets/tailwind/application.css
@@ -861,7 +861,7 @@ body {
   letter-spacing: var(--tracking-caps);
   margin-bottom: 4px;
 }
-.msg-body { color: var(--fg); white-space: pre-wrap; line-height: 1.55; }
+.msg-body { color: var(--fg); white-space: pre-wrap; line-height: 1.55; overflow-wrap: anywhere; }
 .msg-pill {
   font-family: var(--hi-font-mono);
   font-size: 20px;
@@ -880,6 +880,57 @@ body {
   animation: hi-blink 1.6s ease-in-out infinite;
 }
 
+/* Assistant replies rendered from markdown (lib/markdown.rb). Replaces .msg-body's
+   pre-wrap — markdown supplies 
      /
       structure, and pre-wrap on top of that
+   would double every gap. Must stay after .msg-body: equal specificity, so source
+   order decides. */
+.msg-prose { white-space: normal; }
+.msg-prose > :first-child { margin-block-start: 0; }
+.msg-prose > :last-child { margin-block-end: 0; }
+.msg-prose p { margin-block: 0 10px; }
+.msg-prose ul,
+.msg-prose ol {
+  margin-block: 0 10px;
+  padding-inline-start: 1.4em;
+}
+.msg-prose ul { list-style: disc; }
+.msg-prose ol { list-style: decimal; }
+.msg-prose li + li { margin-block-start: 6px; }
+/* The chat model separates numbered items with blank lines, which makes the list
+   loose and wraps each item's content in 
      . Collapse those. */
+.msg-prose li > p { margin-block: 0; }
+.msg-prose li > p + p { margin-block-start: 6px; }
+.msg-prose strong { font-weight: 600; }
+/* Inline code. A fenced block is 
      , handled below, so this can never
+   receive a newline and needs no white-space of its own. */
+.msg-prose code {
+  font-family: var(--hi-font-mono);
+  font-size: 0.92em;
+  background: var(--bg-sunken);
+  padding: 1px 6px;
+  border-radius: var(--radius-sm);
+}
+/* Fenced block, at any depth — top level or inside an 
    5. . The panel (padding +
+   background) lives on 
       rather than on the inner  so it stays put
+   while the content scrolls horizontally instead of sliding out from under it. */
+.msg-prose pre {
+  margin-block: 0 10px;
+  padding: 8px 10px;
+  background: var(--bg-sunken);
+  border-radius: var(--radius-sm);
+  /* `pre`, not `pre-wrap`: a code line must not be broken mid-token. It also
+     neutralises the inherited overflow-wrap: anywhere, which would otherwise
+     split a long URL or path inside the block. With wrapping off the line can
+     overflow, which is what overflow-x scrolls. */
+  white-space: pre;
+  overflow-x: auto;
+}
+.msg-prose pre > code {
+  background: none;
+  padding: 0;
+  border-radius: 0;
+}
+
 /* ============================================================
    COMPOSER
    ============================================================ */
diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb
index 19b552e..5dc16b9 100644
--- a/app/helpers/messages_helper.rb
+++ b/app/helpers/messages_helper.rb
@@ -15,4 +15,23 @@ def tool_call_pill_text(message)
       "running: #{message.tool_calls.map(&:name).uniq.join(", ")}"
     end
   end
+
+  # Assistant replies render as markdown once the stream has finished; user text and
+  # mid-stream text stay plain. Returns an html_safe fragment in the formatted case
+  # and a plain String otherwise, so the view's <%= %> escapes it.
+  def message_body_html(message, streaming: false)
+    return message.content.to_s unless format_message_body?(message, streaming)
+
+    Markdown.render(message.content)
+  end
+
+  def message_body_class(message, streaming: false)
+    format_message_body?(message, streaming) ? "msg-body msg-prose" : "msg-body"
+  end
+
+  private
+
+  def format_message_body?(message, streaming)
+    !streaming && message.role == "assistant"
+  end
 end
diff --git a/app/jobs/chat_respond_job.rb b/app/jobs/chat_respond_job.rb
index 9b442c2..1a700d1 100644
--- a/app/jobs/chat_respond_job.rb
+++ b/app/jobs/chat_respond_job.rb
@@ -81,7 +81,10 @@ def broadcast_replace(project, message)
       project,
       target: ActionView::RecordIdentifier.dom_id(message),
       partial: "messages/message",
-      locals: { message: message }
+      # Mid-stream: markdown is still incomplete (an unclosed ``` fence would
+      # swallow the rest of the reply), so render plain text and let the final
+      # broadcast from Message#broadcast_replace_message format it.
+      locals: { message: message, streaming: true }
     )
   end
 end
diff --git a/app/models/message.rb b/app/models/message.rb
index e6ae0a0..66a91d2 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -25,7 +25,12 @@ def broadcast_append_message
     broadcast_append_later_to chat.project,
       target: "messages",
       partial: "messages/message",
-      locals: { message: self }
+      # `_later` renders inside Turbo::Streams::ActionBroadcastJob, ~100-300ms
+      # after this commit, from a GlobalID reload — by which point ChatRespondJob
+      # has already written chunks. An append is only ever a user message or
+      # RubyLLM's empty assistant placeholder, never a finished reply, so it must
+      # never format.
+      locals: { message: self, streaming: true }
   end
 
   def broadcast_replace_message
diff --git a/app/prompts/generator_agent/instructions.txt.erb b/app/prompts/generator_agent/instructions.txt.erb
index d773548..53c6f4e 100644
--- a/app/prompts/generator_agent/instructions.txt.erb
+++ b/app/prompts/generator_agent/instructions.txt.erb
@@ -17,6 +17,7 @@ GENERAL RULES:
 - Do NOT generate an implementation plan yourself. Do NOT list models, controllers, or files. That's the backend's job.
 - After `create_application` or `modify_application` returns, leave your text response empty. Do not narrate that a build started or finished — those events surface in the UI on their own; you don't need to announce them.
 - Call each tool AT MOST ONCE per user turn. After a tool returns, write your final reply (or no reply at all, per the rule above) and stop — do not call the same tool again until the user replies. Calling the same tool twice in one turn corrupts the conversation history.
+- Formatting: write short paragraphs. Use **bold** for emphasis, `backticks` for code, file and command names, and `-` or `1.` lists where a list genuinely helps. Do not use headings, tables, images, or raw HTML. Write URLs plainly rather than as markdown links. If you need to show markup, put it in backticks.
 
 Current project state:
 <%= current_state %>
diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb
index a8941bd..fc63f16 100644
--- a/app/views/messages/_message.html.erb
+++ b/app/views/messages/_message.html.erb
@@ -1,3 +1,4 @@
+<% streaming = local_assigns.fetch(:streaming, false) %>
 
      
   <% if message.visible_in_chat? %>
     
      
@@ -7,7 +8,7 @@
         
      <%= tool_call_pill_text(message) %>
      
       <% end %>
       <% if message.content.to_s.strip.present? %>
-        
      <%= message.content %>
      
+        
      <%= message_body_html(message, streaming: streaming) %>
      
       <% end %>
     
      
   <% end %>
diff --git a/docs/02-architecture/04-design-system.md b/docs/02-architecture/04-design-system.md
index dc81666..e3e312b 100644
--- a/docs/02-architecture/04-design-system.md
+++ b/docs/02-architecture/04-design-system.md
@@ -84,7 +84,7 @@ that consume it.
 | `.tag` (`--pending` `--gen` `--ok` `--err` `--running` `--starting` `--stopped` `--failed` `--new` `--generating` `--ready`) + `.tag-dot` | same | revisions/_revision, projects/index, projects/show, projects/_state_tag, previews/_* — the build-state variants (`--new` `--generating` `--ready` `--failed`) via the `project_state_tag` helper (app/helpers/projects_helper.rb) |
 | `.project-card` (+ stripe + status modifier) | same | projects/index |
 | `.revisions`, `.revision-row` (+ `--ok` `--gen` `--pend` `--err`) | same | revisions/_list, _revision |
-| `.msg-bubble`, `.msg-role`, `.msg-pill` | same | messages/_message |
+| `.msg-bubble`, `.msg-role`, `.msg-pill`, `.msg-prose` | same | messages/_message — `.msg-prose` is added alongside `.msg-body` on a **finished assistant** reply only (`message_body_class`), and styles the markdown subset `lib/markdown.rb` emits: `p br strong em code pre ul ol li`. It overrides `.msg-body`'s `white-space: pre-wrap` by source order (equal specificity), so it must stay **after** `.msg-body` in the file. Mid-stream and user text keep plain `.msg-body`. |
 | `.composer`, `.suggestion-chip` | same | messages/_form, suggestions/_frame, projects/new |
 | `.preview-pane` (+ header / body / empty / error) + `.preview-frame` | same | previews/_pane, _running, _starting, _stopped, _failed |
 | `.h-display`, `.h-section`, `.lede`, `.eyebrow`, `.numeral`, `.kanji`, `.mono` | same | home/index (display + lede + kanji), layouts (nav brand kanji), home/dashboard (the one remaining `h-section` "Welcome back" + eyebrow + numeral). `.eyebrow` doubles as the page **breadcrumb** on projects/index ("projects"), projects/new + projects/show (`