Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions packages/server/tests/display/display.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
mergeDisplayConfig,
isValidDisplayConfig,
isValidToolVisibility,
classifySystemMessage,
shouldShowSystemMessage,
type DisplayConfig,
} from '@flightdeck-ai/shared';

Expand Down Expand Up @@ -46,6 +48,70 @@ 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('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');
});

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', () => {
Expand Down
80 changes: 80 additions & 0 deletions packages/shared/src/display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ 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), 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) */
toolOverrides?: Record<string, ToolVisibility>;
}
Expand All @@ -28,24 +40,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;

Expand Down Expand Up @@ -128,6 +144,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,
};
}
Expand All @@ -145,6 +163,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<string, unknown>)) {
Expand All @@ -153,3 +172,64 @@ 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 = ['⚠️', '❌', '🛑'];
// 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)) || FAILURE_RE.test(content)) {
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
}
36 changes: 36 additions & 0 deletions packages/web/src/__tests__/Chat.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Chat />);
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(<Chat />);
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(<Chat />);
// 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 <details> for collapsing
expect(document.querySelector('details')).toBeTruthy();
});
});
});
12 changes: 11 additions & 1 deletion packages/web/src/components/DisplaySettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -115,6 +116,15 @@ export function DisplaySettings({ onClose }: { onClose: () => void }) {
onChange={v => setDisplayConfig({ agentMessages: v })}
/>
</div>

{/* System messages */}
<div className="flex items-center justify-between">
<span className="text-sm" title="Operational notices, forwarded steers, scout/orchestrator chatter. Errors always show regardless of this setting.">System messages</span>
<VisibilitySelector
value={displayConfig.systemMessages ?? 'off'}
onChange={v => setDisplayConfig({ systemMessages: v })}
/>
</div>
</div>

{/* Advanced: per-tool overrides */}
Expand Down
Loading
Loading