Merge pull request #837 from siteboon/fix/tool-result-error-rendering

This commit is contained in:
Simos Mikelatos
2026-06-05 17:40:54 +02:00
committed by GitHub
3 changed files with 38 additions and 61 deletions

View File

@@ -7,6 +7,12 @@ import type { NormalizedMessage } from '../../../stores/useSessionStore';
import type { ChatMessage, SubagentChildTool } from '../types/types'; import type { ChatMessage, SubagentChildTool } from '../types/types';
import { decodeHtmlEntities, unescapeWithMathProtection, formatUsageLimitText } from '../utils/chatFormatting'; import { decodeHtmlEntities, unescapeWithMathProtection, formatUsageLimitText } from '../utils/chatFormatting';
function formatToolResultContent(content: unknown): string {
const text = typeof content === 'string' ? content : JSON.stringify(content);
const toolUseErrorMatch = /^<tool_use_error>([\s\S]*)<\/tool_use_error>$/.exec(text.trim());
return toolUseErrorMatch ? toolUseErrorMatch[1] : text;
}
/** /**
* Convert NormalizedMessage[] from the session store into ChatMessage[] * Convert NormalizedMessage[] from the session store into ChatMessage[]
* that the existing UI components expect. * that the existing UI components expect.
@@ -20,7 +26,12 @@ export function normalizedToChatMessages(messages: NormalizedMessage[]): ChatMes
// First pass: collect tool results for attachment // First pass: collect tool results for attachment
const toolResultMap = new Map<string, NormalizedMessage>(); const toolResultMap = new Map<string, NormalizedMessage>();
const toolUseIds = new Set<string>();
for (const msg of messages) { for (const msg of messages) {
if (msg.kind === 'tool_use' && msg.toolId) {
toolUseIds.add(msg.toolId);
}
if (msg.kind === 'tool_result' && msg.toolId) { if (msg.kind === 'tool_result' && msg.toolId) {
toolResultMap.set(msg.toolId, msg); toolResultMap.set(msg.toolId, msg);
} }
@@ -97,7 +108,7 @@ export function normalizedToChatMessages(messages: NormalizedMessage[]): ChatMes
const toolResult = tr const toolResult = tr
? { ? {
content: typeof tr.content === 'string' ? tr.content : JSON.stringify(tr.content), content: formatToolResultContent(tr.content),
isError: Boolean(tr.isError), isError: Boolean(tr.isError),
toolUseResult: (tr as any).toolUseResult, toolUseResult: (tr as any).toolUseResult,
} }
@@ -191,8 +202,25 @@ export function normalizedToChatMessages(messages: NormalizedMessage[]): ChatMes
break; break;
// tool_result is handled via attachment to tool_use above // tool_result is handled via attachment to tool_use above
case 'tool_result': case 'tool_result': {
if (msg.toolId && toolUseIds.has(msg.toolId)) {
break;
}
const content = formatToolResultContent(msg.content || '');
if (!content.trim()) {
break;
}
converted.push({
type: msg.isError ? 'error' : 'assistant',
content,
timestamp: msg.timestamp,
toolId: msg.toolId,
...sharedMetadata,
});
break; break;
}
default: default:
break; break;

View File

@@ -564,11 +564,15 @@ export function shouldHideToolResult(toolName: string, toolResult: any): boolean
if (!config.result) return false; if (!config.result) return false;
// Hidden/success-only configs suppress noisy successful output, but errors
// still need to be visible so failed tool calls are diagnosable.
if (toolResult?.isError) return false;
// Always hidden // Always hidden
if (config.result.hidden) return true; if (config.result.hidden) return true;
// Hide on success only // Hide on success only
if (config.result.hideOnSuccess && toolResult && !toolResult.isError) { if (config.result.hideOnSuccess && toolResult) {
return true; return true;
} }

View File

@@ -1,5 +1,6 @@
import { memo, useEffect, useMemo, useRef, useState } from 'react'; import { memo, useEffect, useMemo, useRef, useState } from 'react';
import { useTranslation } from 'react-i18next'; import { useTranslation } from 'react-i18next';
import SessionProviderLogo from '../../../llm-logo-provider/SessionProviderLogo'; import SessionProviderLogo from '../../../llm-logo-provider/SessionProviderLogo';
import type { import type {
ChatMessage, ChatMessage,
@@ -8,10 +9,10 @@ import type {
Provider, Provider,
} from '../../types/types'; } from '../../types/types';
import { formatUsageLimitText } from '../../utils/chatFormatting'; import { formatUsageLimitText } from '../../utils/chatFormatting';
import { getClaudePermissionSuggestion } from '../../utils/chatPermissions';
import type { Project } from '../../../../types/app'; import type { Project } from '../../../../types/app';
import { ToolRenderer, shouldHideToolResult } from '../../tools'; import { ToolRenderer, shouldHideToolResult } from '../../tools';
import { Reasoning, ReasoningTrigger, ReasoningContent } from '../../../../shared/view/ui'; import { Reasoning, ReasoningTrigger, ReasoningContent } from '../../../../shared/view/ui';
import { Markdown } from './Markdown'; import { Markdown } from './Markdown';
import MessageCopyControl from './MessageCopyControl'; import MessageCopyControl from './MessageCopyControl';
@@ -41,10 +42,9 @@ type InteractiveOption = {
isSelected: boolean; isSelected: boolean;
}; };
type PermissionGrantState = 'idle' | 'granted' | 'error';
const COPY_HIDDEN_TOOL_NAMES = new Set(['Bash', 'Edit', 'Write', 'ApplyPatch']); const COPY_HIDDEN_TOOL_NAMES = new Set(['Bash', 'Edit', 'Write', 'ApplyPatch']);
const MessageComponent = memo(({ message, prevMessage, createDiff, onFileOpen, onShowSettings, onGrantToolPermission, autoExpandTools, showRawParameters, showThinking, selectedProject, provider }: MessageComponentProps) => { const MessageComponent = memo(({ message, prevMessage, createDiff, onFileOpen, autoExpandTools, showRawParameters, showThinking, selectedProject, provider }: MessageComponentProps) => {
const { t } = useTranslation('chat'); const { t } = useTranslation('chat');
const isGrouped = prevMessage && prevMessage.type === message.type && const isGrouped = prevMessage && prevMessage.type === message.type &&
((prevMessage.type === 'assistant') || ((prevMessage.type === 'assistant') ||
@@ -53,8 +53,6 @@ const MessageComponent = memo(({ message, prevMessage, createDiff, onFileOpen, o
(prevMessage.type === 'error')); (prevMessage.type === 'error'));
const messageRef = useRef<HTMLDivElement | null>(null); const messageRef = useRef<HTMLDivElement | null>(null);
const [isExpanded, setIsExpanded] = useState(false); const [isExpanded, setIsExpanded] = useState(false);
const permissionSuggestion = getClaudePermissionSuggestion(message, provider);
const [permissionGrantState, setPermissionGrantState] = useState<PermissionGrantState>('idle');
const userCopyContent = String(message.content || ''); const userCopyContent = String(message.content || '');
const formattedMessageContent = useMemo( const formattedMessageContent = useMemo(
() => formatUsageLimitText(String(message.content || '')), () => formatUsageLimitText(String(message.content || '')),
@@ -73,10 +71,6 @@ const MessageComponent = memo(({ message, prevMessage, createDiff, onFileOpen, o
!message.isThinking; !message.isThinking;
useEffect(() => {
setPermissionGrantState('idle');
}, [permissionSuggestion?.entry, message.toolId]);
useEffect(() => { useEffect(() => {
const node = messageRef.current; const node = messageRef.current;
if (!autoExpandTools || !node || !message.isToolUse) return; if (!autoExpandTools || !node || !message.isToolUse) return;
@@ -241,55 +235,6 @@ const MessageComponent = memo(({ message, prevMessage, createDiff, onFileOpen, o
<Markdown className="prose prose-sm prose-red max-w-none dark:prose-invert"> <Markdown className="prose prose-sm prose-red max-w-none dark:prose-invert">
{String(message.toolResult.content || '')} {String(message.toolResult.content || '')}
</Markdown> </Markdown>
{permissionSuggestion && (
<div className="mt-4 border-t border-red-200/60 pt-3 dark:border-red-800/60">
<div className="flex flex-wrap items-center gap-2">
<button
type="button"
onClick={() => {
if (!onGrantToolPermission) return;
const result = onGrantToolPermission(permissionSuggestion);
if (result?.success) {
setPermissionGrantState('granted');
} else {
setPermissionGrantState('error');
}
}}
disabled={permissionSuggestion.isAllowed || permissionGrantState === 'granted'}
className={`inline-flex items-center gap-2 rounded-md border px-3 py-1.5 text-xs font-medium transition-colors ${permissionSuggestion.isAllowed || permissionGrantState === 'granted'
? 'cursor-default border-green-300/70 bg-green-100 text-green-800 dark:border-green-800/60 dark:bg-green-900/30 dark:text-green-200'
: 'border-red-300/70 bg-white/80 text-red-700 hover:bg-white dark:border-red-800/60 dark:bg-gray-900/40 dark:text-red-200 dark:hover:bg-gray-900/70'
}`}
>
{permissionSuggestion.isAllowed || permissionGrantState === 'granted'
? t('permissions.added')
: t('permissions.grant', { tool: permissionSuggestion.toolName })}
</button>
{onShowSettings && (
<button
type="button"
onClick={(e) => { e.stopPropagation(); onShowSettings(); }}
className="text-xs text-red-700 underline hover:text-red-800 dark:text-red-200 dark:hover:text-red-100"
>
{t('permissions.openSettings')}
</button>
)}
</div>
<div className="mt-2 text-xs text-red-700/90 dark:text-red-200/80">
{t('permissions.addTo', { entry: permissionSuggestion.entry })}
</div>
{permissionGrantState === 'error' && (
<div className="mt-2 text-xs text-red-700 dark:text-red-200">
{t('permissions.error')}
</div>
)}
{(permissionSuggestion.isAllowed || permissionGrantState === 'granted') && (
<div className="mt-2 text-xs text-green-700 dark:text-green-200">
{t('permissions.retry')}
</div>
)}
</div>
)}
</div> </div>
</div> </div>
) : ( ) : (