fix: marked v15 + marked-terminal v7 incompat in markdown-renderer (#138)

marked v15's use() iterates 'for (prop in pack.renderer)' and validates every enumerable key against its known renderer method list, throwing "renderer 'o' does not exist" at module init.

The legacy 'new TerminalRenderer(opts)' route assigns config to own enumerable properties (this.o, this.tab, ...), so the first iteration hits an unknown key and crashes. This broke the agent on every 'just start' since PR #135 landed; CI never noticed because no test imports the module.

Switch to the modern markedTerminal() factory which returns a clean MarkedExtension containing only renderer method keys, and add a regression test that import-loads the module and smoke-tests rendering so a future bump can't reintroduce this class of crash.

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Author: simongdavies

src/agent/markdown-renderer.ts

@@ -8,8 +8,8 @@
 //   import { renderMarkdown } from "./markdown-renderer.js";
 //   console.log(renderMarkdown("# Hello\n**bold** and `code`"));
 
-import { Marked, type MarkedOptions } from "marked";
-import TerminalRenderer from "marked-terminal";
+import { Marked, type MarkedExtension } from "marked";
+import { markedTerminal } from "marked-terminal";
 import { createRequire } from "node:module";
 import { resolve } from "node:path";
 import kqlLanguage from "./hljs-kql.js";
@@ -28,20 +28,27 @@ hljsInstance.registerLanguage("kql", kqlLanguage);
 // Use a local Marked instance so we don't mutate the global marked
 // singleton — other code importing marked won't accidentally get
 // terminal-rendered output instead of HTML.
-const terminalRenderer = new TerminalRenderer({
-  // Indent code blocks for visual separation
-  tab: 2,
-  // Show URLs inline rather than as footnotes
-  showSectionPrefix: true,
-  // Convert HTML entities back to characters
-  unescape: true,
-});
-
-// marked-terminal's renderer type doesn't match marked v15's _Renderer
-// exactly, but it works at runtime. Cast to satisfy the type checker.
-const localMarked = new Marked({
-  renderer: terminalRenderer as unknown as MarkedOptions["renderer"],
-});
+//
+// `markedTerminal()` returns a MarkedExtension (`{ renderer, useNewRenderer }`)
+// suitable for marked v15's strict `use()` validation.  Constructing a
+// `TerminalRenderer` instance directly and passing it as `{ renderer }` no
+// longer works in marked >=15 because TerminalRenderer's constructor sets
+// own enumerable properties (`this.o`, `this.tab`, …) and marked iterates
+// every enumerable key of the renderer object, rejecting anything that
+// isn't a known renderer method.
+//
+// The @types/marked-terminal declaration mis-types the return as
+// `TerminalRenderer`; cast through `unknown` to the correct shape.
+const localMarked = new Marked(
+  markedTerminal({
+    // Indent code blocks for visual separation
+    tab: 2,
+    // Show URLs inline rather than as footnotes
+    showSectionPrefix: true,
+    // Convert HTML entities back to characters
+    unescape: true,
+  }) as unknown as MarkedExtension,
+);
 
 /**
  * Render a markdown string as ANSI-formatted terminal output.

tests/markdown-renderer.test.ts

@@ -0,0 +1,95 @@
+// ── Markdown Renderer Tests ────────────────────────────────────────
+//
+// These tests exist primarily to detect *module-load crashes* in
+// `markdown-renderer.ts`.  The module wires `marked` + `marked-terminal`
+// at import time and any incompatibility between the two pinned versions
+// will throw on first require — long before any test exercises the
+// rendered output.
+//
+// Regression: in marked v15 + marked-terminal v7, passing a
+// `TerminalRenderer` *instance* via `new Marked({ renderer })` throws
+// "renderer 'o' does not exist" because marked enumerates every key on
+// the renderer object and the legacy `Renderer` constructor stores
+// config as own properties (`this.o`, `this.tab`, …).  The fix is to
+// use the `markedTerminal()` factory which returns a clean
+// `MarkedExtension`.  These tests would have caught that bug.
+//
+// ────────────────────────────────────────────────────────────────────
+
+import { describe, it, expect } from "vitest";
+import {
+  renderMarkdown,
+  looksLikeMarkdown,
+} from "../src/agent/markdown-renderer.js";
+
+describe("renderMarkdown", () => {
+  it("renders plain text without crashing", () => {
+    // The smoke test that matters most — if module init fails this
+    // throws before reaching the assertion.
+    const out = renderMarkdown("hello world");
+    expect(typeof out).toBe("string");
+    expect(out.length).toBeGreaterThan(0);
+  });
+
+  it("renders headings, code, lists and code fences", () => {
+    const md = [
+      "# Title",
+      "",
+      "**bold** and `inline code`",
+      "",
+      "- one",
+      "- two",
+      "",
+      "```js",
+      "const x = 1;",
+      "```",
+    ].join("\n");
+
+    const out = renderMarkdown(md);
+
+    // We don't assert on ANSI codes (they vary by terminal capability)
+    // — just that the meaningful content survived rendering.
+    expect(out).toContain("Title");
+    expect(out).toContain("bold");
+    expect(out).toContain("inline code");
+    expect(out).toContain("one");
+    expect(out).toContain("two");
+    expect(out).toContain("const x = 1");
+  });
+
+  it("trims trailing newlines", () => {
+    const out = renderMarkdown("hello\n\n");
+    expect(out.endsWith("\n")).toBe(false);
+  });
+
+  it("handles empty input", () => {
+    expect(renderMarkdown("")).toBe("");
+  });
+});
+
+describe("looksLikeMarkdown", () => {
+  it("detects headings", () => {
+    expect(looksLikeMarkdown("# Heading")).toBe(true);
+    expect(looksLikeMarkdown("###### h6")).toBe(true);
+  });
+
+  it("detects fenced code blocks", () => {
+    expect(looksLikeMarkdown("```js\nconst x = 1;\n```")).toBe(true);
+  });
+
+  it("detects tables", () => {
+    expect(looksLikeMarkdown("| a | b |\n| - | - |\n| 1 | 2 |")).toBe(true);
+  });
+
+  it("detects ordered lists", () => {
+    expect(looksLikeMarkdown("1. first\n2. second")).toBe(true);
+  });
+
+  it("rejects plain text and weak signals", () => {
+    // Bold markers and unordered bullets alone are too noisy to count
+    // as markdown (false positives on git output, log lines, etc.).
+    expect(looksLikeMarkdown("just a sentence")).toBe(false);
+    expect(looksLikeMarkdown("**not strong enough**")).toBe(false);
+    expect(looksLikeMarkdown("- bullet alone")).toBe(false);
+  });
+});