refactor(chat-core): dispatch ChatPage's transcript rows through the renderer registry

website/src/pages/chat/transcriptRenderers.tsx

@@ -1,7 +1,12 @@
 /**
  * transcriptRenderers — the dashboard's row set for the shared chat transcript.
  *
- * The single-chat surface (ChatPage) draws its rows from a local role chain.
+ * The single-chat surface (ChatPage) dispatches through the same registry with
+ * its OWN host list (see `chatPageRenderers` in pages/ChatPage.tsx -- chat-core
+ * P5-a); the two dashboard host lists share ids (`tool`, `file`, `nudge`,
+ * `recovery_inject`, `thinking_block`, `error`, `workflow_completion`,
+ * `subagent_completion`) so they can converge by override, but they are still
+ * two lists until a P5 follow-up spreads this factory into ChatPage's.
  * Every OTHER dashboard surface draws through app-sdk/ChatMessageList, whose
  * default registry is deliberately store-free and therefore renders a WEAKER
  * transcript: a static pill instead of the live tool line, and nothing at all

website/src/test/ChatPage.mcpOAuth.test.tsx

@@ -1,10 +1,11 @@
 /**
  * Guards the MCP OAuth banner wiring in ChatPage.
  *
- * ChatPage.renderMessage must route messages with role 'mcp_oauth' to
- * renderMcpOAuthMessage so the Authorize banner renders inline. If that branch
- * (or its import) is dropped, the message falls through to AssistantMessage and
- * the raw "🔐 … requires authentication." text is shown instead of the banner.
+ * ChatPage's renderer entries must route messages with role 'mcp_oauth' to
+ * renderMcpOAuthMessage so the Authorize banner renders inline. If that entry
+ * (or its import) is dropped, the message falls to the registry default, which
+ * draws the banner inside the page's generic row wrapper instead of as a keyed
+ * page row; the entry is pinned here so that wiring stays explicit.
  *
  * This is a source-contract test: ChatPage's message list is driven by the
  * custom virtualizer (useVirtualChat), which mounts an empty window under jsdom
@@ -28,12 +29,15 @@ describe('ChatPage – MCP OAuth banner wiring', () => {
   })
 
   it('routes the mcp_oauth message role to the banner renderer', () => {
-    expect(chatPageSrc).toMatch(/role\s*===\s*['"]mcp_oauth['"]/)
+    // Since chat-core P5-a the page dispatches rows through the app-sdk
+    // registry: the banner is the page's `mcp_oauth` HOST ENTRY (same id as
+    // the registry default it overrides, claiming the role), not an if-branch.
+    expect(chatPageSrc).toMatch(/id:\s*'mcp_oauth',\s*\n\s*roles:\s*\['mcp_oauth'\]/)
     expect(chatPageSrc).toMatch(/renderMcpOAuthMessage\s*\(/)
   })
 
-  it('keeps the banner branch and its renderer call in the same render path', () => {
-    const idxRole = chatPageSrc.search(/role\s*===\s*['"]mcp_oauth['"]/)
+  it('keeps the banner entry and its renderer call in the same render path', () => {
+    const idxRole = chatPageSrc.search(/id:\s*'mcp_oauth',\s*\n\s*roles:\s*\['mcp_oauth'\]/)
     const idxCall = chatPageSrc.indexOf('renderMcpOAuthMessage(')
     expect(idxRole).toBeGreaterThanOrEqual(0)
     expect(idxCall).toBeGreaterThanOrEqual(0)

website/src/test/NoticeCard.test.tsx

@@ -163,7 +163,9 @@ describe('registry wiring', () => {
   it("ChatPage renders the notice role through NoticeCard, not a hand-rolled box", () => {
     const here = dirname(fileURLToPath(import.meta.url))
     const src = readFileSync(resolve(here, '../pages/ChatPage.tsx'), 'utf8')
-    expect(src).toMatch(/m\.role === 'notice'.*<NoticeCard/)
+    // Since chat-core P5-a the page dispatches through the app-sdk registry;
+    // the notice row is the page's `notice` host entry (same id as the default).
+    expect(src).toMatch(/id: 'notice', roles: \['notice'\], render: [^\n]*<NoticeCard/)
     // The old inline branch carried its own class recipe; its return must be
     // gone so the style cannot fork again at this call site.
     expect(src).not.toMatch(/m\.role === 'notice'.*className=/)

website/src/test/RecoveryCard.test.tsx

@@ -350,14 +350,18 @@ describe('ChatPage – recovery card wiring', () => {
   })
 
   it('checks for a card BEFORE the generic inject bubble renders', () => {
-    // The generic `isInject` branch paints any injected text as a full-width
-    // warning bubble. If the resolver check lands after it, the card is dead
-    // code and the raw prompt reappears.
-    const card = src.indexOf('resolveInjectCard(m)')
-    const generic = src.indexOf("const isInject = m.role === 'inject'")
-    expect(card).toBeGreaterThanOrEqual(0)
-    expect(generic).toBeGreaterThanOrEqual(0)
-    expect(card).toBeLessThan(generic)
+    // Since chat-core P5-a the page dispatches through the app-sdk registry
+    // and precedence is the order of its host entries. The generic bubble
+    // entry (which paints any injected text as a full-width warning bubble)
+    // is the LAST entry; the `inject_recovery` shape entry must sit before it,
+    // or the card is dead code and the raw prompt reappears.
+    const list = src.indexOf('const renderers = mergeRenderers([')
+    const card = src.indexOf("id: 'recovery_inject'", list)
+    const generic = src.indexOf('\n      bubble,\n    ])', list)
+    expect(list).toBeGreaterThanOrEqual(0)
+    expect(card).toBeGreaterThan(list)
+    expect(generic).toBeGreaterThan(card)
+    expect(src).toMatch(/id: 'recovery_inject',\s*\n\s*roles: \['inject'\],\s*\n\s*match: m => resolveInjectCard\(m\) != null/)
   })
 })
 

website/src/test/chatRolesParity.contract.test.ts

@@ -1,81 +1,71 @@
 /**
- * Role-parity contract between the two transcript render paths.
+ * Role-parity contract between ChatPage and the transcript renderer registry.
  *
- * The dashboard renders chat messages through TWO paths: ChatPage's inline
- * `renderMessage` if-chain, and the registry in `app-sdk/messageRenderers`
- * that every other surface (SideChat, ChatPane, ChatEmbed) consumes via
- * ChatMessageList. A role wired in only one path ships a surface where that
- * message renders as raw text or not at all — `mcp_oauth` shipped exactly
- * this way once, wired in app-sdk but not in the main chat.
+ * Every chat surface renders rows through `app-sdk/messageRenderers`. Until
+ * chat-core P5-a, ChatPage was the exception: an inline `renderMessage`
+ * if-chain that had to be kept in step with the registry by hand, and the
+ * defect class that bought this test -- `mcp_oauth` wired in app-sdk and
+ * rendered as raw text in the main chat -- lived in that gap. ChatPage now
+ * DISPATCHES through the registry (`resolveRenderer` over
+ * `mergeRenderers(chatPageRenderers)`), so a role registered once renders on
+ * every surface by construction. What is left to guard:
  *
- * Until ChatPage consumes the registry directly (the chat-core extraction's
- * later phase), this contract is the guard: every role literal that ChatPage's
- * source dispatches on must be CLAIMED by the registry, or be explicitly
- * listed here as chrome-only with a reason. Adding a role branch to ChatPage
- * without touching either list fails this test.
+ * 1. The dispatch stays registry-driven: the renderer block in ChatPage
+ *    contains no `if (m.role === '…')` dispatch of its own.
+ * 2. ChatPage's host entries either OVERRIDE a default (same id, page chrome
+ *    layered on the shared row) or are one of the documented page-only shape
+ *    entries below -- an undocumented id is a fork of the registry in disguise.
+ * 3. Role literals ChatPage still uses OUTSIDE the renderer (chrome logic:
+ *    footer rules, queue rail, last-error lookup, permission grouping) name
+ *    roles the registry claims, or are allowlisted as chrome with a reason.
  */
 import { describe, it, expect } from 'vitest'
 import { readFileSync } from 'node:fs'
 import { resolve } from 'node:path'
 import { defaultMessageRenderers } from '../app-sdk/messageRenderers'
-import { REASONING_ROLES } from '../pages/chat/groupDisplayItems'
 
 /**
- * Roles ChatPage handles that are deliberately NOT a registry row. Each entry
- * must say why it is chrome rather than a transcript row type — an entry
- * without a reason is a parity gap hiding behind the allowlist.
+ * Roles ChatPage's CHROME logic names that are deliberately NOT a registry
+ * row. Each entry must say why -- an entry without a reason is a parity gap
+ * hiding behind the allowlist. (`queued` and `streaming` left this list with
+ * P5-a: the registry claims both -- `undrawn` and the assistant entry.)
  */
 const CHROME_ONLY_ROLES: Record<string, string> = {
-  // Rendered as the QueueStack card rail above the composer, not as a
-  // transcript row (the registry deliberately draws nothing for it).
-  queued: 'composer rail, not a transcript row',
-  // Approval flow: resolved inline into grouped tool rows; the standalone
-  // role is chrome that the permission cards own.
+  // Approval flow: resolved inline into grouped tool rows (GROUPED_ROLES);
+  // the standalone role is chrome that the permission cards own. ChatPage's
+  // own `permission` entry draws nothing for the same reason.
   permission: 'approval cards own it; grouped, never a standalone row',
-  // Pseudo-role: normalized to `assistant` before dispatch on both paths.
-  streaming: 'alias of assistant during a live turn',
 }
 
 /**
- * Roles only the registry claims, with the reason each is legitimate. These
- * arrive on surfaces that read the raw snapshot endpoints (app embeds, side
- * sessions), whose payloads carry wire-shape roles ChatPage's normalized
- * store never sees.
+ * ChatPage host entries that do not override a default. Each is a SHAPE entry
+ * (`roles: ['*']` + `match`) or a deliberately undrawn role, with the reason
+ * it is page-only rather than a registry default.
  */
-const REGISTRY_ONLY_ROLES: Record<string, string> = {
-  tool_call: 'raw snapshot wire shape; ChatPage store normalizes to tool',
-  tool_result: 'raw snapshot wire shape; ChatPage store normalizes to tool',
-  system: 'lifecycle marker in raw snapshots; deliberately undrawn',
-  done: 'lifecycle marker in raw snapshots; deliberately undrawn',
+const PAGE_ONLY_ENTRY_IDS: Record<string, string> = {
+  thinking_block: 'ThinkingBlock with page disclosure state; the registry folds reasoning into the group summary (same id as transcriptRenderers)',
+  recovery_inject: 'RecoveryCard for gateway-authored inject rows; the registry renders inject as prose (resolveInjectCard decides, shared; same id as transcriptRenderers)',
+  permission: 'undrawn here; the registry leaves it to GROUPED_ROLES',
+  workflow_completion: 'WorkflowCompletionCard needs page-only session/folder/panel hand-offs',
+  hidden_invisible_assistant: 'zero-width-space quiet-cycle rows; the registry applies the same skip inside its assistant entry',
+  bubble: 'the page\'s user / inject / assistant row, with fork, pin, footer, regenerate and search-scope chrome',
+}
+
+const src = readFileSync(resolve(__dirname, '../pages/ChatPage.tsx'), 'utf8')
+
+function rendererBlock(): string {
+  const start = src.indexOf('fallback: bubbleRenderer } = useMemo')
+  const end = src.indexOf('const renderMessage = useCallback', start)
+  if (start < 0 || end < 0) throw new Error('ChatPage renderer block not found -- did the P5-a dispatch move?')
+  return src.slice(start, end)
 }
 
 function chatPageRoleLiterals(): Set<string> {
-  const src = readFileSync(resolve(__dirname, '../pages/ChatPage.tsx'), 'utf8')
   const roles = new Set<string>()
   // Both dispatch shapes used in the file: `m.role === 'x'` and
-  // `messages[i].role === 'x'` (and their !== variants — a negative dispatch
+  // `messages[i].role === 'x'` (and their !== variants -- a negative dispatch
   // still means the code KNOWS the role).
   for (const m of src.matchAll(/\.role\s*[!=]==\s*'([a-z_]+)'/g)) roles.add(m[1])
-  // Reasoning rows dispatch through the shared predicate (isReasoningRole /
-  // hasReasoningContent from pages/chat/groupDisplayItems — the #6406
-  // single-definition consolidation) rather than a role literal. Credit the
-  // shared list's roles ONLY when ChatPage imports BOTH predicates from the
-  // shared module AND both dispatch statements are present. This is a textual
-  // check, not an AST binding check: a comment spelling the exact dispatch
-  // shape could keep the credit alive — accepted, because the companion
-  // predicate-idiom guard below and the reasoningBurst structural scan bound
-  // what ChatPage can contain, and losing either import drops the credit
-  // (the orphan check then reddens). If the dispatch shape is refactored,
-  // update these patterns.
-  const importsShared =
-    /import \{[^}]*\bhasReasoningContent\b[^}]*\} from '\.\/chat\/groupDisplayItems'/.test(src) &&
-    /import \{[^}]*\bisReasoningRole\b[^}]*\} from '\.\/chat\/groupDisplayItems'/.test(src)
-  const dispatchesReasoning =
-    /hasReasoningContent\(\w+\)\)\s*return\s*<ThinkingBlock/.test(src) &&
-    /isReasoningRole\(\w+\)\)\s*return\s*null/.test(src)
-  if (importsShared && dispatchesReasoning) {
-    for (const role of REASONING_ROLES) roles.add(role)
-  }
   return roles
 }
 
@@ -87,64 +77,67 @@ function registryClaimedRoles(): Set<string> {
   return roles
 }
 
-describe('chat role parity (ChatPage renderMessage vs app-sdk registry)', () => {
-  it('every role ChatPage dispatches on is claimed by the registry or allowlisted as chrome', () => {
+function hostEntryIds(): string[] {
+  return [...rendererBlock().matchAll(/^\s+id: '([a-z_]+)',?$/gm)].map(m => m[1])
+}
+
+describe('chat role parity (ChatPage consumes the app-sdk registry)', () => {
+  it('ChatPage dispatches rows through the registry, not an if-chain of its own', () => {
+    expect(src).toMatch(/import \{[^}]*\bmergeRenderers\b[^}]*\} from '\.\.\/app-sdk\/messageRenderers'/)
+    expect(src).toMatch(/import \{[^}]*\bresolveRenderer\b[^}]*\} from '\.\.\/app-sdk\/messageRenderers'/)
+    expect(src).toContain('resolveRenderer(m, chatPageRenderers)')
+    // Variant flags INSIDE one entry (`const isUser = m.role === 'user'`) are
+    // fine; a dispatch statement is not -- it would select a row the registry
+    // never sees. Both spellings of a dispatch are rejected, so a chain cannot
+    // come back as a `switch`.
+    expect(rendererBlock()).not.toMatch(/if \(m\.role\s*[!=]==\s*'[a-z_]+'\)/)
+    expect(rendererBlock()).not.toMatch(/switch \(m\.role\)/)
+  })
+
+  it('a role nobody claims falls back to the BUBBLE by reference, never to an SDK default by position', () => {
+    // mergeRenderers returns [...shapeMatched, ...hostEntries, ...roleKeyedDefaults],
+    // so the merged list's tail is the SDK `undrawn` default (render: () => null);
+    // indexing it would make an unregistered role -- the drift this contract
+    // exists to catch -- vanish from the main chat instead of rendering as text.
+    expect(rendererBlock()).toContain('return { renderers, fallback: bubble }')
+    expect(src).toContain('return (entry ?? bubbleRenderer).render(m, ctx)')
+    expect(src).not.toMatch(/chatPageRenderers\[chatPageRenderers\.length - 1\]/)
+    // And the page's own `undrawn` override leaves `system` / `done` unclaimed,
+    // so they keep the if-chain's fall-through instead of the SDK's null.
+    const undrawn = rendererBlock().match(/id: 'undrawn',\s*\n\s*roles: \[([^\]]*)\]/)
+    expect(undrawn).not.toBeNull()
+    expect(undrawn![1]).not.toMatch(/'system'|'done'/)
+  })
+
+  it('every ChatPage host entry overrides a default or is a documented page-only entry', () => {
+    const defaults = new Set(defaultMessageRenderers.map(r => r.id))
+    const ids = hostEntryIds()
+    expect(ids.length).toBeGreaterThan(5)
+    const undocumented = ids.filter(id => !defaults.has(id) && !(id in PAGE_ONLY_ENTRY_IDS))
+    // An entry with a NEW id that also claims a role the defaults render would
+    // shadow the shared row on this page only -- the fork this contract exists
+    // to stop. Reuse the default's id to override it, or document why the
+    // entry is page-only.
+    expect(undocumented).toEqual([])
+    // And the documentation cannot outlive the entry.
+    const stale = Object.keys(PAGE_ONLY_ENTRY_IDS).filter(id => !ids.includes(id))
+    expect(stale).toEqual([])
+  })
+
+  it('every role ChatPage still names is claimed by the registry or allowlisted as chrome', () => {
     const claimed = registryClaimedRoles()
     const missing = [...chatPageRoleLiterals()].filter(
       role => !claimed.has(role) && !(role in CHROME_ONLY_ROLES),
     )
-    // A failure here means a role renders in the main chat but is invisible
-    // (or raw) in SideChat / ChatPane / ChatEmbed — register it in
-    // app-sdk/messageRenderers, or add it to CHROME_ONLY_ROLES with a reason.
+    // A role ChatPage's chrome reasons about but no renderer claims would be
+    // rendered by the page's bubble fallback and by nothing on the other
+    // surfaces -- register it, or add it to CHROME_ONLY_ROLES with a reason.
     expect(missing).toEqual([])
   })
 
   it('the chrome allowlist carries no stale entries', () => {
     const known = chatPageRoleLiterals()
     const stale = Object.keys(CHROME_ONLY_ROLES).filter(role => !known.has(role))
-    // An allowlist entry for a role ChatPage no longer mentions is dead
-    // weight that would silently excuse a future regression — remove it.
-    expect(stale).toEqual([])
-  })
-
-  it('the registry itself only claims roles ChatPage knows (no orphaned surface-only roles)', () => {
-    const known = chatPageRoleLiterals()
-    const orphaned = [...registryClaimedRoles()].filter(
-      role => !known.has(role) && !(role in REGISTRY_ONLY_ROLES),
-    )
-    // A role only the registry knows renders on app surfaces but as raw text
-    // in the MAIN chat — the exact defect class this contract exists to stop
-    // (that is how mcp_oauth shipped). Wire it into ChatPage too, or record
-    // why it is a wire-shape role in REGISTRY_ONLY_ROLES.
-    expect(orphaned).toEqual([])
-  })
-
-  it('the registry-only allowlist carries no stale entries', () => {
-    const claimed = registryClaimedRoles()
-    const stale = Object.keys(REGISTRY_ONLY_ROLES).filter(role => !claimed.has(role))
     expect(stale).toEqual([])
   })
-
-  it('fails closed: every role dispatch in ChatPage uses the shape the extractor parses', () => {
-    const src = readFileSync(resolve(__dirname, '../pages/ChatPage.tsx'), 'utf8')
-    // The extractor understands two idioms: `<expr>.role ===/!== '<literal>'`
-    // and the shared reasoning predicates credited above. Any other dispatch
-    // idiom — switch(m.role), a lookup map, comparison against a variable —
-    // would be invisible to the parity checks above, so its mere presence
-    // fails this contract. If you add one, extend the extractor in this file
-    // to parse it rather than allowlisting it here.
-    const comparisons = [...src.matchAll(/\.role\s*[!=]==\s*(\S)/g)]
-    const nonLiteral = comparisons.filter(m => m[1] !== "'")
-    expect(nonLiteral.map(m => m[0])).toEqual([])
-    expect([...src.matchAll(/switch\s*\([^)]*\.role/g)].map(m => m[0])).toEqual([])
-    // Predicate-shaped role dispatch (the #6406 idiom): only the two shared
-    // reasoning predicates are parsed. A future is<X>Role(...) / has<X>Content(...)
-    // helper called in ChatPage is a role dispatch the extractor cannot see,
-    // so its presence fails here until the extractor learns it.
-    const predicateCalls = [...src.matchAll(/\b(is[A-Z]\w*Role|has[A-Z]\w*Content)\s*\(/g)].map(m => m[1])
-    const unknownPredicates = predicateCalls.filter(
-      name => name !== 'isReasoningRole' && name !== 'hasReasoningContent',
-    )
-    expect(unknownPredicates).toEqual([])
-  })
 })