feat: improve error display with custom titles and expandable UI

implementation-plan.md

@@ -0,0 +1,113 @@
+# Implementation Plan: Improve Error Display in Webview
+
+## Overview
+
+Currently, `cline.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with:
+
+1. A custom title that can be passed as metadata
+2. An expandable/collapsible UI pattern
+3. The error text shown when expanded
+
+## Current Implementation Analysis
+
+### 1. Error Display Flow
+
+- **Backend (Task.ts)**: `say("error", text)` method sends error messages
+- **Frontend (ChatRow.tsx)**: Renders error messages with red text styling
+- **Diff Error Pattern**: Already implements the expandable UI pattern we want to replicate
+
+### 2. Diff Error Implementation (lines 960-1048 in ChatRow.tsx)
+
+The diff_error display has:
+
+- Warning icon with yellow/orange color
+- Bold title ("Edit Unsuccessful")
+- Copy button
+- Expand/collapse chevron
+- Expandable content area showing the error details
+
+## Implementation Steps
+
+### Step 1: Extend the say method signature
+
+**File**: `src/core/task/Task.ts`
+
+The say method already accepts an `options` parameter with metadata support. We need to:
+
+- Document that error messages can include a `title` in metadata
+- No changes needed to the method signature itself
+
+### Step 2: Update ChatRow.tsx to handle enhanced error display
+
+**File**: `webview-ui/src/components/chat/ChatRow.tsx`
+
+Changes needed:
+
+1. Add state for error expansion (similar to `isDiffErrorExpanded`)
+2. Extract metadata from error messages
+3. Render errors with the expandable UI pattern
+4. Use custom title from metadata or default to "Error"
+
+### Step 3: Update translation files
+
+**Files**: All files in `webview-ui/src/i18n/locales/*/chat.json`
+
+Add new translation keys:
+
+- `error.defaultTitle`: Default title when no custom title is provided
+- Keep existing `error` key for backward compatibility
+
+### Step 4: Update existing error calls
+
+Search for all `say("error", ...)` calls and optionally add metadata with custom titles where appropriate.
+
+## Technical Details
+
+### Message Structure
+
+```typescript
+// When calling say with error and custom title:
+await this.say(
+	"error",
+	"Detailed error message here",
+	undefined, // images
+	false, // partial
+	undefined, // checkpoint
+	undefined, // progressStatus
+	{
+		metadata: {
+			title: "Custom Error Title",
+		},
+	},
+)
+```
+
+### Frontend Rendering Logic
+
+```typescript
+// In ChatRow.tsx, for case "error":
+// 1. Extract title from metadata or use default
+// 2. Render expandable UI similar to diff_error
+// 3. Show error text in expanded section
+```
+
+## Benefits
+
+1. **Consistency**: Error display matches the existing "Edit Unsuccessful" pattern
+2. **Clarity**: Custom titles provide immediate context about the error type
+3. **User Experience**: Collapsible errors reduce visual clutter
+4. **Flexibility**: Backward compatible - existing error calls continue to work
+
+## Testing Considerations
+
+1. Test with errors that have custom titles
+2. Test with errors without custom titles (should use default)
+3. Test expand/collapse functionality
+4. Test copy button functionality
+5. Verify all translations work correctly
+
+## Migration Strategy
+
+- The implementation is backward compatible
+- Existing `say("error", ...)` calls will continue to work
+- We can gradually update error calls to include custom titles where beneficial

packages/types/src/message.ts

@@ -224,6 +224,7 @@ export const clineMessageSchema = z.object({
 					reasoning_summary: z.string().optional(),
 				})
 				.optional(),
+			title: z.string().optional(), // Custom title for error messages
 		})
 		.optional(),
 })

src/core/assistant-message/presentAssistantMessage.ts

@@ -317,12 +317,15 @@ export async function presentAssistantMessage(cline: Task) {
 				return await askApproval("tool", toolMessage)
 			}
 
-			const handleError = async (action: string, error: Error) => {
+			const handleError = async (action: string, error: Error, title?: string) => {
 				const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}`
 
 				await cline.say(
 					"error",
 					`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
+					undefined,
+					undefined,
+					title ? { title } : undefined,
 				)
 
 				pushToolResult(formatResponse.toolError(errorString))

src/core/task/Task.ts

@@ -1164,15 +1164,41 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	}
 
 	async sayAndCreateMissingParamError(toolName: ToolName, paramName: string, relPath?: string) {
+		const relPathFormatted = relPath ? ` for '${relPath.toPosix()}'` : ""
 		await this.say(
 			"error",
-			`Roo tried to use ${toolName}${
-				relPath ? ` for '${relPath.toPosix()}'` : ""
-			} without value for required parameter '${paramName}'. Retrying...`,
+			t("tools:common.errors.missingParameterMessage", {
+				toolName,
+				relPath: relPathFormatted,
+				paramName,
+			}),
+			undefined,
+			undefined,
+			undefined,
+			undefined,
+			{
+				metadata: {
+					title: t("tools:common.errors.missingParameter"),
+				},
+			},
 		)
 		return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
 	}
 
+	/**
+	 * Helper method to say an error with a custom title
+	 * @param title - The title to display for the error
+	 * @param text - The error message text
+	 * @param images - Optional images to include
+	 */
+	async sayError(title: string, text: string, images?: string[]) {
+		await this.say("error", text, images, undefined, undefined, undefined, {
+			metadata: {
+				title,
+			},
+		})
+	}
+
 	// Lifecycle
 	// Start / Resume / Abort / Dispose
 

src/core/tools/__tests__/generateImageTool.test.ts

@@ -277,7 +277,17 @@ describe("generateImageTool", () => {
 				mockRemoveClosingTag,
 			)
 
-			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Input image not found"))
+			expect(mockCline.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("Input image not found"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "Input Image Not Found" },
+				},
+			)
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Input image not found"))
 		})
 
@@ -302,7 +312,17 @@ describe("generateImageTool", () => {
 				mockRemoveClosingTag,
 			)
 
-			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Unsupported image format"))
+			expect(mockCline.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("Unsupported image format"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "Unsupported Image Format" },
+				},
+			)
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Unsupported image format"))
 		})
 	})

src/core/tools/__tests__/insertContentTool.spec.ts

@@ -46,6 +46,22 @@ vi.mock("../../ignore/RooIgnoreController", () => ({
 	},
 }))
 
+vi.mock("../../../i18n", () => ({
+	t: vi.fn((key: string, params?: any) => {
+		// Return the key without the namespace prefix for testing
+		const keyWithoutNamespace = key.replace(/^[^:]+:/, "")
+		if (params) {
+			// Simple parameter replacement for testing
+			let result = keyWithoutNamespace
+			Object.entries(params).forEach(([key, value]) => {
+				result = result.replace(`{{${key}}}`, String(value))
+			})
+			return result
+		}
+		return keyWithoutNamespace
+	}),
+}))
+
 describe("insertContentTool", () => {
 	const testFilePath = "test/file.txt"
 	// Use a consistent mock absolute path for testing
@@ -226,7 +242,19 @@ describe("insertContentTool", () => {
 			expect(mockedFsReadFile).not.toHaveBeenCalled()
 			expect(mockCline.consecutiveMistakeCount).toBe(1)
 			expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content")
-			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file"))
+			expect(mockCline.say).toHaveBeenCalledWith(
+				"error",
+				"insertContent.errors.cannotInsertIntoNonExistent",
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				expect.objectContaining({
+					metadata: expect.objectContaining({
+						title: "insertContent.errors.invalidLineNumber",
+					}),
+				}),
+			)
 			expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled()
 			expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled()
 		})

src/core/tools/__tests__/useMcpToolTool.spec.ts

@@ -151,7 +151,17 @@ describe("useMcpToolTool", () => {
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
 			expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
-			expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("invalid JSON argument"))
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("invalid JSON argument"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "Invalid JSON Arguments" },
+				},
+			)
 			expect(mockPushToolResult).toHaveBeenCalledWith("Tool error: Invalid args for test_server:test_tool")
 		})
 	})
@@ -343,7 +353,17 @@ describe("useMcpToolTool", () => {
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
 			expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
-			expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("does not exist"))
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("does not exist"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "MCP Tool Not Found" },
+				},
+			)
 			// Check that the error message contains available tools
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("existing-tool-1"))
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("existing-tool-2"))
@@ -390,7 +410,17 @@ describe("useMcpToolTool", () => {
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
 			expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
-			expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("does not exist"))
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("does not exist"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "MCP Tool Not Found" },
+				},
+			)
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("No tools available"))
 		})
 
@@ -484,7 +514,17 @@ describe("useMcpToolTool", () => {
 			// Assert
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
 			expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
-			expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("not configured"))
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("not configured"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "MCP Server Not Found" },
+				},
+			)
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("s1"))
 			expect(callToolMock).not.toHaveBeenCalled()
 			expect(mockAskApproval).not.toHaveBeenCalled()
@@ -527,7 +567,17 @@ describe("useMcpToolTool", () => {
 			// Assert
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
 			expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool")
-			expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("not configured"))
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				expect.stringContaining("not configured"),
+				undefined,
+				undefined,
+				undefined,
+				undefined,
+				{
+					metadata: { title: "MCP Server Not Found" },
+				},
+			)
 			expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("No servers available"))
 			expect(callToolMock).not.toHaveBeenCalled()
 			expect(mockAskApproval).not.toHaveBeenCalled()

src/core/tools/__tests__/writeToFileTool.spec.ts

@@ -403,7 +403,11 @@ describe("writeToFileTool", () => {
 
 			await executeWriteFileTool({})
 
-			expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error))
+			expect(mockHandleError).toHaveBeenCalledWith(
+				"writing file",
+				expect.any(Error),
+				"writeToFile.errors.writeFileError",
+			)
 			expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
 		})
 
@@ -412,7 +416,11 @@ describe("writeToFileTool", () => {
 
 			await executeWriteFileTool({}, { isPartial: true })
 
-			expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error))
+			expect(mockHandleError).toHaveBeenCalledWith(
+				"writing file",
+				expect.any(Error),
+				"writeToFile.errors.writeFileError",
+			)
 			expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
 		})
 	})