From 7fb3e38544339abbac290ad53842a1e12ae78439 Mon Sep 17 00:00:00 2001 From: Justin Chu Date: Sun, 14 Jun 2026 09:07:54 -0700 Subject: [PATCH 1/2] web: improve system message display + add systemMessages verbosity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit System messages in the chat were all rendered as a centered pill, which looks bad for long content (spawn errors with stack traces, forwarded steer text). They were also always visible at every verbosity level. Rendering (Chat.tsx): - Classify system messages as error / notice / debug. - error → alert card with left border; long content collapses into a one-line summary + expandable body (stack traces no longer overflow). - notice → short stays a pill; long becomes a centered collapsible one-liner. - debug → same as notice but muted. Verbosity (shared/display.ts): - New `systemMessages` ToolVisibility on DisplayConfig + presets (minimal/summary: off, detail: summary, debug: detail). - `classifySystemMessage` + `shouldShowSystemMessage` helpers. - Errors ALWAYS show regardless of the setting, so failures are never hidden; notices show at summary+, debug-class only at detail. - Chat filtering honors the setting; DisplaySettings gains a "System messages" selector. Tests: classification + visibility unit tests (shared), system message rendering/gating tests (web). Full server suite 838 passing, web 132 passing, no new lint errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/server/tests/display/display.test.ts | 52 +++++++++++ packages/shared/src/display.ts | 72 +++++++++++++++ packages/web/src/__tests__/Chat.test.tsx | 36 ++++++++ .../web/src/components/DisplaySettings.tsx | 12 ++- packages/web/src/pages/Chat.tsx | 90 +++++++++++++++++-- 5 files changed, 252 insertions(+), 10 deletions(-) diff --git a/packages/server/tests/display/display.test.ts b/packages/server/tests/display/display.test.ts index c15a3261..3e1cfed9 100644 --- a/packages/server/tests/display/display.test.ts +++ b/packages/server/tests/display/display.test.ts @@ -9,6 +9,8 @@ import { mergeDisplayConfig, isValidDisplayConfig, isValidToolVisibility, + classifySystemMessage, + shouldShowSystemMessage, type DisplayConfig, } from '@flightdeck-ai/shared'; @@ -46,6 +48,56 @@ describe('DisplayConfig', () => { expect(p.toolCalls).toBe('detail'); expect(p.flightdeckTools).toBe('summary'); }); + + it('systemMessages: hidden except at detail/debug presets', () => { + expect(DISPLAY_PRESETS.minimal.systemMessages).toBe('off'); + expect(DISPLAY_PRESETS.summary.systemMessages).toBe('off'); + expect(DISPLAY_PRESETS.detail.systemMessages).toBe('summary'); + expect(DISPLAY_PRESETS.debug.systemMessages).toBe('detail'); + }); + }); + + describe('classifySystemMessage', () => { + it('classifies error-prefixed content as error', () => { + expect(classifySystemMessage({ content: '⚠️ Failed to spawn agent worker-1' })).toBe('error'); + expect(classifySystemMessage({ content: '❌ something broke' })).toBe('error'); + expect(classifySystemMessage({ content: 'Director spawn FAILED after retries' })).toBe('error'); + }); + + it('classifies system DMs (forwarded steers) as debug', () => { + expect(classifySystemMessage({ content: 'do the thing', channel: 'dm:worker-1' })).toBe('debug'); + expect(classifySystemMessage({ content: 'do the thing', recipient: 'worker-1' })).toBe('debug'); + }); + + it('classifies scout/orchestrator chatter as debug', () => { + expect(classifySystemMessage({ content: '[scout] Analysis requested for spec s1' })).toBe('debug'); + expect(classifySystemMessage({ content: 'task escalated', authorId: 'orchestrator' })).toBe('debug'); + }); + + it('classifies plain operational confirmations as notice', () => { + expect(classifySystemMessage({ content: 'Plan approved. 3 tasks moved to pending.' })).toBe('notice'); + }); + }); + + describe('shouldShowSystemMessage', () => { + it('always shows errors regardless of visibility', () => { + for (const v of ['off', 'summary', 'detail', undefined] as const) { + expect(shouldShowSystemMessage('error', v)).toBe(true); + } + }); + + it('notices show at summary and detail only', () => { + expect(shouldShowSystemMessage('notice', 'off')).toBe(false); + expect(shouldShowSystemMessage('notice', undefined)).toBe(false); + expect(shouldShowSystemMessage('notice', 'summary')).toBe(true); + expect(shouldShowSystemMessage('notice', 'detail')).toBe(true); + }); + + it('debug shows at detail only', () => { + expect(shouldShowSystemMessage('debug', 'off')).toBe(false); + expect(shouldShowSystemMessage('debug', 'summary')).toBe(false); + expect(shouldShowSystemMessage('debug', 'detail')).toBe(true); + }); }); describe('isFlightdeckTool', () => { diff --git a/packages/shared/src/display.ts b/packages/shared/src/display.ts index d7abf28e..20bed881 100644 --- a/packages/shared/src/display.ts +++ b/packages/shared/src/display.ts @@ -17,6 +17,14 @@ export interface DisplayConfig { * one-liner (default), 'detail' shows full bubbles with sender → recipient. */ agentMessages?: ToolVisibility; + /** + * System message visibility in the main chat (operational notices, forwarded + * steers, scout/orchestrator chatter). 'off' hides them, 'summary' shows a + * compact pill / collapsed one-liner, 'detail' expands long content. + * NOTE: error/failure system messages are ALWAYS shown regardless of this + * setting — they render as an alert card so the user never misses a failure. + */ + systemMessages?: ToolVisibility; /** Per-tool overrides (tool name → visibility) */ toolOverrides?: Record; } @@ -28,24 +36,28 @@ export const DISPLAY_PRESETS = { toolCalls: 'off' as const, flightdeckTools: 'off' as const, agentMessages: 'off' as const, + systemMessages: 'off' as const, }, summary: { thinking: false, toolCalls: 'summary' as const, flightdeckTools: 'off' as const, agentMessages: 'summary' as const, + systemMessages: 'off' as const, }, detail: { thinking: true, toolCalls: 'detail' as const, flightdeckTools: 'summary' as const, agentMessages: 'detail' as const, + systemMessages: 'summary' as const, }, debug: { thinking: true, toolCalls: 'detail' as const, flightdeckTools: 'detail' as const, agentMessages: 'detail' as const, + systemMessages: 'detail' as const, }, } as const; @@ -128,6 +140,8 @@ export function mergeDisplayConfig( flightdeckTools: partial.flightdeckTools ?? base.flightdeckTools, // Persisted configs may predate this field — default to collapsed agentMessages: partial.agentMessages ?? base.agentMessages ?? 'summary', + // Persisted configs may predate this field — default to hidden + systemMessages: partial.systemMessages ?? base.systemMessages ?? 'off', toolOverrides, }; } @@ -145,6 +159,7 @@ export function isValidDisplayConfig(v: unknown): v is PartialDisplayConfig { if (obj.toolCalls !== undefined && !isValidToolVisibility(obj.toolCalls)) return false; if (obj.flightdeckTools !== undefined && !isValidToolVisibility(obj.flightdeckTools)) return false; if (obj.agentMessages !== undefined && !isValidToolVisibility(obj.agentMessages)) return false; + if (obj.systemMessages !== undefined && !isValidToolVisibility(obj.systemMessages)) return false; if (obj.toolOverrides !== undefined) { if (typeof obj.toolOverrides !== 'object' || obj.toolOverrides === null || Array.isArray(obj.toolOverrides)) return false; for (const val of Object.values(obj.toolOverrides as Record)) { @@ -153,3 +168,60 @@ export function isValidDisplayConfig(v: unknown): v is PartialDisplayConfig { } return true; } + +// ── System message classification ── + +/** + * Classification of a persisted `system` chat message, used to decide how it + * renders and whether the `systemMessages` visibility setting applies. + * + * - `error` — operational failures (spawn errors, stack traces). ALWAYS shown. + * - `notice` — short operational confirmations. Governed by `systemMessages`. + * - `debug` — noisy internals: forwarded steers (system DMs), scout/orchestrator + * chatter. Governed by `systemMessages` (only at 'detail'). + */ +export type SystemMessageClass = 'error' | 'notice' | 'debug'; + +/** Minimal shape needed to classify a system message (subset of ChatMessage). */ +export interface SystemMessageLike { + content: string; + authorId?: string | null; + channel?: string | null; + channelId?: string | null; + recipient?: string | null; +} + +const ERROR_PREFIXES = ['⚠️', '❌', '🛑']; + +/** Classify a `system`-authored chat message. */ +export function classifySystemMessage(msg: SystemMessageLike): SystemMessageClass { + const content = msg.content ?? ''; + const trimmed = content.trimStart(); + if (ERROR_PREFIXES.some(p => trimmed.startsWith(p)) || /\bfailed\b/i.test(content.slice(0, 80))) { + return 'error'; + } + // Forwarded steer copies are persisted as system DMs (recipient / dm channel). + const isDm = !!(msg.recipient || msg.channel?.startsWith('dm:') || msg.channel === 'dm' || msg.channelId?.startsWith('dm:')); + if (isDm) return 'debug'; + // Scout / orchestrator internal chatter. + if (msg.authorId === 'orchestrator' || trimmed.startsWith('[scout]')) return 'debug'; + return 'notice'; +} + +/** + * Decide whether a system message should be visible given its class and the + * configured `systemMessages` visibility. + * - error: always visible. + * - notice: visible at 'summary' and 'detail'. + * - debug: visible only at 'detail'. + */ +export function shouldShowSystemMessage( + cls: SystemMessageClass, + visibility: ToolVisibility | undefined, +): boolean { + if (cls === 'error') return true; + const v = visibility ?? 'off'; + if (v === 'off') return false; + if (cls === 'notice') return v === 'summary' || v === 'detail'; + return v === 'detail'; // debug +} diff --git a/packages/web/src/__tests__/Chat.test.tsx b/packages/web/src/__tests__/Chat.test.tsx index d6dbf7f9..8a9145dc 100644 --- a/packages/web/src/__tests__/Chat.test.tsx +++ b/packages/web/src/__tests__/Chat.test.tsx @@ -125,4 +125,40 @@ describe('Chat page', () => { expect(screen.getByText('Replying to')).toBeInTheDocument(); expect(screen.getAllByText(/Hello from lead/).length).toBeGreaterThan(0); }); + + describe('system messages', () => { + it('error system messages always render (even when systemMessages off)', () => { + mockMessages = [{ + id: 'sys-err', authorType: 'system', authorId: null, + content: '⚠️ Failed to spawn agent worker-1', + createdAt: new Date().toISOString(), + }]; + render(); + expect(screen.getByText(/Failed to spawn agent worker-1/)).toBeInTheDocument(); + }); + + it('notice system messages are hidden when systemMessages is off', () => { + mockMessages = [{ + id: 'sys-notice', authorType: 'system', authorId: null, + content: 'Plan approved. 3 tasks moved to pending.', + createdAt: new Date().toISOString(), + }]; + render(); + expect(screen.queryByText(/Plan approved/)).not.toBeInTheDocument(); + }); + + it('long error content renders collapsed (summary visible, body hidden until expanded)', () => { + const stack = 'Error: boom\n' + 'at foo (a.ts:1)\n'.repeat(10); + mockMessages = [{ + id: 'sys-err-long', authorType: 'system', authorId: null, + content: `⚠️ Failed to spawn agent worker-2\n\n\`\`\`\n${stack}\`\`\``, + createdAt: new Date().toISOString(), + }]; + render(); + // Summary line is shown (also present in the collapsible body markdown) + expect(screen.getAllByText(/Failed to spawn agent worker-2/).length).toBeGreaterThan(0); + // It's inside a
for collapsing + expect(document.querySelector('details')).toBeTruthy(); + }); + }); }); diff --git a/packages/web/src/components/DisplaySettings.tsx b/packages/web/src/components/DisplaySettings.tsx index b1c25c14..6d324ec6 100644 --- a/packages/web/src/components/DisplaySettings.tsx +++ b/packages/web/src/components/DisplaySettings.tsx @@ -18,7 +18,8 @@ export function DisplaySettings({ onClose }: { onClose: () => void }) { return preset.thinking === displayConfig.thinking && preset.toolCalls === displayConfig.toolCalls && preset.flightdeckTools === displayConfig.flightdeckTools - && preset.agentMessages === (displayConfig.agentMessages ?? 'summary'); + && preset.agentMessages === (displayConfig.agentMessages ?? 'summary') + && preset.systemMessages === (displayConfig.systemMessages ?? 'off'); }) ?? 'custom'; // M9: Basic focus trap — keep keyboard focus inside the dialog @@ -115,6 +116,15 @@ export function DisplaySettings({ onClose }: { onClose: () => void }) { onChange={v => setDisplayConfig({ agentMessages: v })} /> + + {/* System messages */} +
+ System messages + setDisplayConfig({ systemMessages: v })} + /> +
{/* Advanced: per-tool overrides */} diff --git a/packages/web/src/pages/Chat.tsx b/packages/web/src/pages/Chat.tsx index 27c2440b..4a3ac863 100644 --- a/packages/web/src/pages/Chat.tsx +++ b/packages/web/src/pages/Chat.tsx @@ -8,7 +8,7 @@ import { useDisplay } from '../hooks/useDisplay.tsx'; import type { StreamChunk, ToolCallState } from '../hooks/useChat.tsx'; import type { ChatMessage, Thread } from '../lib/types.ts'; import { api } from '../lib/api.ts'; -import { shouldShow, type ContentType } from '@flightdeck-ai/shared/display'; +import { shouldShow, classifySystemMessage, shouldShowSystemMessage, type ContentType } from '@flightdeck-ai/shared/display'; import { ChatSidePanel } from '../components/ChatSidePanel.tsx'; // L2: Hoist regex outside component to avoid re-creation per call @@ -76,6 +76,77 @@ function scrollToMessage(id: string) { setTimeout(() => el.classList.remove('animate-highlight'), 1500); } +const SYSTEM_LONG_THRESHOLD = 140; + +function systemSummary(s: string): string { + const nl = s.indexOf('\n'); + const line = (nl === -1 ? s : s.slice(0, nl)).trim(); + return line || 'System message'; +} + +/** Renders a `system` chat message, dispatching on its classification: + * error → alert card (long content collapsible), notice → pill / collapsible, + * debug → muted pill / collapsible. */ +const SystemMessage = memo(function SystemMessage({ msg }: { msg: ChatMessage }) { + const cls = classifySystemMessage(msg); + const content = msg.content ?? ''; + const isLong = content.length > SYSTEM_LONG_THRESHOLD || content.includes('\n'); + const summary = systemSummary(content); + + if (cls === 'error') { + return ( +
+ {isLong ? ( +
+ + + {summary.replace(/^[⚠️❌🛑\uFE0F]\s*/u, '')} + expand + +
+ +
+
+ ) : ( +
+ + {content} +
+ )} +
+ ); + } + + const muted = cls === 'debug'; + + // Short notice/debug → centered pill (unchanged look for the common case). + if (!isLong) { + return ( +
+ + {content} + +
+ ); + } + + // Long notice/debug → centered collapsible one-liner instead of a giant pill. + return ( +
+
+ + + {summary} + · expand + +
+ +
+
+
+ ); +}); + const MessageBubble = memo(function MessageBubble({ msg, messages, replyCountMap, onReply, highlighted, agents }: { msg: ChatMessage; messages?: ChatMessage[]; replyCountMap?: Map; onReply: (m: ChatMessage) => void; highlighted?: boolean; agents?: Array<{ id: string; role: string; runtime?: string; runtimeName?: string; model?: string; status?: string }> }) { const style = AUTHOR_STYLES[msg.authorType] ?? AUTHOR_STYLES.system; const isUser = msg.authorType === 'user'; @@ -83,13 +154,7 @@ const MessageBubble = memo(function MessageBubble({ msg, messages, replyCountMap const parentMsgs = msg.parentIds && messages ? msg.parentIds.map(pid => messages.find(m => m.id === pid)).filter(Boolean) as ChatMessage[] : []; if (msg.authorType === 'system') { - return ( -
- - {msg.content} - -
- ); + return ; } const replies = replyCountMap?.get(msg.id); @@ -653,8 +718,15 @@ export default function Chat() { // off → dropped, summary → collapsed expandable run, detail → full bubbles const renderItems = useMemo(() => { const visibility = displayConfig.agentMessages ?? 'summary'; + const sysVisibility = displayConfig.systemMessages ?? 'off'; const items: Array<{ kind: 'msg'; msg: ChatMessage } | { kind: 'dmGroup'; key: string; msgs: ChatMessage[] }> = []; for (const m of filteredMessages) { + if (m.authorType === 'system') { + // error system messages always show; notices/debug gated by config + if (!shouldShowSystemMessage(classifySystemMessage(m), sysVisibility)) continue; + items.push({ kind: 'msg', msg: m }); + continue; + } if (isAgentDm(m)) { if (visibility === 'off') continue; if (visibility === 'summary') { @@ -667,7 +739,7 @@ export default function Chat() { items.push({ kind: 'msg', msg: m }); } return items; - }, [filteredMessages, displayConfig.agentMessages]); + }, [filteredMessages, displayConfig.agentMessages, displayConfig.systemMessages]); const handleReply = useCallback((m: ChatMessage) => setReplyTo(m), []); From bbb842a51ad019bd103ec8e28ad3d5cb1ec5d4d9 Mon Sep 17 00:00:00 2001 From: Justin Chu Date: Sun, 14 Jun 2026 10:26:57 -0700 Subject: [PATCH 2/2] web: address PR review on system message display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Chat.tsx: fix emoji-stripping regex in the error summary header so it consumes the U+FE0F variation selector (and repeated emoji), preventing a stray combining character at the start of "⚠️"-prefixed summaries. - display.ts: reword the systemMessages JSDoc to describe only the gating contract enforced by shouldShowSystemMessage; layout (pill vs collapsible) is documented as a renderer concern, not part of the shared contract. - display.ts: classifySystemMessage now scans the WHOLE message for failure keywords (failed/failure/error/exception/stack trace/panic/crash) instead of only the first 80 chars, so a failure keyword appearing after a prefix line still classifies as 'error' and is always shown. A failure keyword now also wins over the DM/scout debug heuristics. Tests: added cases for late failure keywords and failure-keyword DMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/server/tests/display/display.test.ts | 14 ++++++++++++++ packages/shared/src/display.ts | 18 +++++++++++++----- packages/web/src/pages/Chat.tsx | 2 +- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/server/tests/display/display.test.ts b/packages/server/tests/display/display.test.ts index 3e1cfed9..7d89130a 100644 --- a/packages/server/tests/display/display.test.ts +++ b/packages/server/tests/display/display.test.ts @@ -64,6 +64,20 @@ describe('DisplayConfig', () => { expect(classifySystemMessage({ content: 'Director spawn FAILED after retries' })).toBe('error'); }); + it('detects failure keywords anywhere in the content (not just the first line)', () => { + const msg = { + content: 'Spawning agent worker-9\nsource: orchestrator\n---\nUncaught exception: boom\nstack trace follows', + }; + // failure keyword appears well past the first 80 chars / first line + expect(classifySystemMessage(msg)).toBe('error'); + }); + + it('failure-keyword DMs are still classified as error (always shown)', () => { + // even though it looks like a forwarded steer (dm channel), a failure + // must win so it is never hidden by the systemMessages setting + expect(classifySystemMessage({ content: 'the build failed', channel: 'dm:worker-1' })).toBe('error'); + }); + it('classifies system DMs (forwarded steers) as debug', () => { expect(classifySystemMessage({ content: 'do the thing', channel: 'dm:worker-1' })).toBe('debug'); expect(classifySystemMessage({ content: 'do the thing', recipient: 'worker-1' })).toBe('debug'); diff --git a/packages/shared/src/display.ts b/packages/shared/src/display.ts index 20bed881..a92a7c01 100644 --- a/packages/shared/src/display.ts +++ b/packages/shared/src/display.ts @@ -19,10 +19,14 @@ export interface DisplayConfig { agentMessages?: ToolVisibility; /** * System message visibility in the main chat (operational notices, forwarded - * steers, scout/orchestrator chatter). 'off' hides them, 'summary' shows a - * compact pill / collapsed one-liner, 'detail' expands long content. - * NOTE: error/failure system messages are ALWAYS shown regardless of this - * setting — they render as an alert card so the user never misses a failure. + * steers, scout/orchestrator chatter), used by `shouldShowSystemMessage` to + * gate non-error system messages: + * - 'off' — hide notices and debug chatter + * - 'summary' — show 'notice'-class messages + * - 'detail' — show 'notice' and 'debug'-class messages + * NOTE: 'error'-class system messages are ALWAYS shown regardless of this + * setting. How a visible message is laid out (pill vs collapsible) is a UI + * concern decided by the renderer, not by this value. */ systemMessages?: ToolVisibility; /** Per-tool overrides (tool name → visibility) */ @@ -192,12 +196,16 @@ export interface SystemMessageLike { } const ERROR_PREFIXES = ['⚠️', '❌', '🛑']; +// Failure keywords. Matched across the WHOLE message (not just the first line) +// so a failure system message is never misclassified as notice/debug and then +// hidden — failures must always be shown. +const FAILURE_RE = /\b(failed|failure|error|exception|stack trace|panic|crashed?)\b/i; /** Classify a `system`-authored chat message. */ export function classifySystemMessage(msg: SystemMessageLike): SystemMessageClass { const content = msg.content ?? ''; const trimmed = content.trimStart(); - if (ERROR_PREFIXES.some(p => trimmed.startsWith(p)) || /\bfailed\b/i.test(content.slice(0, 80))) { + if (ERROR_PREFIXES.some(p => trimmed.startsWith(p)) || FAILURE_RE.test(content)) { return 'error'; } // Forwarded steer copies are persisted as system DMs (recipient / dm channel). diff --git a/packages/web/src/pages/Chat.tsx b/packages/web/src/pages/Chat.tsx index 4a3ac863..c96c1110 100644 --- a/packages/web/src/pages/Chat.tsx +++ b/packages/web/src/pages/Chat.tsx @@ -100,7 +100,7 @@ const SystemMessage = memo(function SystemMessage({ msg }: { msg: ChatMessage })
- {summary.replace(/^[⚠️❌🛑\uFE0F]\s*/u, '')} + {summary.replace(/^[⚠❌🛑\u26A0\uFE0F]+\s*/u, '')} expand