fix(error-interception): address CodeRabbit review findings

Apply all 11 CodeRabbit review findings from PR Zoo-Code-Org#1009:

MAJOR fixes:
- Use module-scoped interceptor singleton instead of per-block creation
  so per-task WeakMap counters and circuit breakers persist across blocks
- Move pendingNativeProtocolGuide from undeclared cline property onto
  TaskErrorState with get/set/clear; consume in every tool_result path
- Reset PARAM_TYPE_MISMATCH state when structural fingerprint changes
  to prevent stale circuit state from affecting different tools
- Enforce requiresToolContext in ErrorClassifier both matching passes;
  skip tool-bound patterns when signal lacks toolName/toolCallId

MINOR fixes:
- Gate validateCwdParameter to execute_command tool only
- Preserve original error message alongside guided payload in validation
- Path-scoped cycle detection in StructuralValidator (delete after children)
- Preserve non-text blocks (images) in array result transformation
- Match JSON-RPC -32602 as both string and number
- Use TextEncoder for UTF-8 byte counting in MessageTransformer
- Update non-ASCII test to exercise multibyte truncation with byteLimit

Author: k1yt

src/core/assistant-message/presentAssistantMessage.ts

@@ -48,6 +48,14 @@ import { codebaseSearchTool } from "../tools/CodebaseSearchTool"
 import { formatResponse } from "../prompts/responses"
 import { sanitizeToolUseId } from "../../utils/tool-id"
 
+/**
+ * Module-scoped interceptor singleton. A single shared instance keeps the
+ * per-task WeakMap alive across content blocks within the same Task, so
+ * occurrence counters and circuit breakers persist between tool blocks.
+ * Recreating one per block would reset all per-task counters to empty.
+ */
+const toolErrorInterceptor = createToolErrorInterceptor()
+
 /**
  * Processes and presents assistant message content to the user interface.
  *
@@ -114,7 +122,7 @@ export async function presentAssistantMessage(cline: Task) {
 			// These are converted to the same execution path as use_mcp_tool but preserve
 			// their original name in API history
 			const mcpBlock = block as McpToolUse
-			const interceptor = createToolErrorInterceptor()
+			const interceptor = toolErrorInterceptor
 
 			if (cline.didRejectTool) {
 				// For native protocol, we must send a tool_result for every tool_use to avoid API errors
@@ -124,10 +132,13 @@ export async function presentAssistantMessage(cline: Task) {
 					: `MCP tool ${mcpBlock.name} was interrupted and not executed due to user rejecting a previous tool.`
 
 				if (toolCallId) {
+					// Consume any pending native protocol guide so it cannot leak
+					// into later turns when this early tool_result path is taken.
+					const rejectedMcpGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
 					cline.pushToolResultToUserContent({
 						type: "tool_result",
 						tool_use_id: sanitizeToolUseId(toolCallId),
-						content: errorMessage,
+						content: rejectedMcpGuide ? `${errorMessage}\n\n${rejectedMcpGuide}` : errorMessage,
 						is_error: true,
 					})
 				}
@@ -175,6 +186,12 @@ export async function presentAssistantMessage(cline: Task) {
 				}
 
 				if (toolCallId) {
+					// Merge any pending XML_NATIVE_DUAL_PROTOCOL guide into this
+					// tool_result and clear it so it cannot leak into later turns.
+					const mcpPendingGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
+					if (mcpPendingGuide) {
+						resultContent = `${resultContent}\n\n${mcpPendingGuide}`
+					}
 					cline.pushToolResultToUserContent({
 						type: "tool_result",
 						tool_use_id: sanitizeToolUseId(toolCallId),
@@ -346,9 +363,10 @@ export async function presentAssistantMessage(cline: Task) {
 							"INVALID_TOOL_PROTOCOL",
 							"INVALID_TOOL_PROTOCOL|XML_NATIVE_DUAL_PROTOCOL|text-block",
 						)
-						;(cline as any).pendingNativeProtocolGuide =
+						taskErrorState.setPendingNativeProtocolGuide(
 							`[XML_NATIVE_DUAL_PROTOCOL occurrence=${occurrence}] XML tool calls are not supported. ` +
-							`Use native tool_use only. The XML markup was removed from the visible text; only the native tool call was executed.`
+								`Use native tool_use only. The XML markup was removed from the visible text; only the native tool call was executed.`,
+						)
 					}
 				}
 			}
@@ -359,7 +377,7 @@ export async function presentAssistantMessage(cline: Task) {
 		case "tool_use": {
 			// Native tool calling is the only supported tool calling mechanism.
 			// A tool_use block without an id is invalid and cannot be executed.
-			const interceptor = createToolErrorInterceptor()
+			const interceptor = toolErrorInterceptor
 			const toolCallId = (block as any).id as string | undefined
 			if (!toolCallId) {
 				const errorMessage =
@@ -464,10 +482,13 @@ export async function presentAssistantMessage(cline: Task) {
 					? `Skipping tool ${toolDescription()} due to user rejecting a previous tool.`
 					: `Tool ${toolDescription()} was interrupted and not executed due to user rejecting a previous tool.`
 
+				// Consume any pending native protocol guide so it cannot leak
+				// into later turns when this early tool_result path is taken.
+				const rejectedGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
 				cline.pushToolResultToUserContent({
 					type: "tool_result",
 					tool_use_id: sanitizeToolUseId(toolCallId),
-					content: errorMessage,
+					content: rejectedGuide ? `${errorMessage}\n\n${rejectedGuide}` : errorMessage,
 					is_error: true,
 				})
 
@@ -511,13 +532,15 @@ export async function presentAssistantMessage(cline: Task) {
 
 					// Push tool_result directly without setting didAlreadyUseTool so streaming can
 					// continue gracefully.
+					const missingArgsGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
+					const missingArgsBase = guided ?? formatResponse.toolError(errorMessage)
 					cline.pushToolResultToUserContent({
 						type: "tool_result",
 						tool_use_id: sanitizeToolUseId(toolCallId),
-						content: guided ?? formatResponse.toolError(errorMessage),
+						content: missingArgsGuide ? `${missingArgsBase}\n\n${missingArgsGuide}` : missingArgsBase,
 						is_error: true,
 					})
-
+	
 					break
 				}
 			}
@@ -550,10 +573,9 @@ export async function presentAssistantMessage(cline: Task) {
 				// Merge any pending XML_NATIVE_DUAL_PROTOCOL guide into this native
 				// tool's result. The native result remains primary; the warning is
 				// appended once and then cleared so it cannot leak into later turns.
-				const pendingGuide = (cline as any).pendingNativeProtocolGuide as string | undefined
+				const pendingGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
 				if (pendingGuide) {
 					resultContent = `${resultContent}\n\n${pendingGuide}`
-					;(cline as any).pendingNativeProtocolGuide = undefined
 				}
 
 				// Merge approval feedback into tool result (GitHub #10465)
@@ -679,14 +701,22 @@ export async function presentAssistantMessage(cline: Task) {
 			if (!block.partial && block.nativeArgs) {
 				const taskErrorState = getTaskErrorState(cline)
 				const structuralSignals = [
-					validateCwdParameter(block.nativeArgs as Record<string, unknown>, String(block.name)),
+					...(block.name === "execute_command"
+						? [validateCwdParameter(block.nativeArgs as Record<string, unknown>, String(block.name))]
+						: []),
 					validateNestedParams(block.nativeArgs as Record<string, unknown>, String(block.name)),
 				].filter((s): s is NonNullable<typeof s> => s != null)
 
 				if (structuralSignals.length > 0) {
 					const signal = structuralSignals[0]
 					const variant = (signal.metadata?.variant as string | undefined) ?? "STRUCTURAL_MISUSE"
 					const fingerprint = `PARAM_TYPE_MISMATCH|${variant}|${String(block.name)}|${(signal.metadata?.parameter as string | undefined) ?? ""}`
+					// If the structural failure shape changed (different tool, variant, or
+					// parameter), reset the circuit so the new shape gets fresh guidance
+					// instead of inheriting MODEL_STUCK_LOOP from an unrelated pattern.
+					if (taskErrorState.getFingerprint("PARAM_TYPE_MISMATCH") !== fingerprint) {
+						taskErrorState.reset("PARAM_TYPE_MISMATCH")
+					}
 					taskErrorState.setFingerprint("PARAM_TYPE_MISMATCH", fingerprint)
 					const occurrence = taskErrorState.incrementOccurrence("PARAM_TYPE_MISMATCH")
 					const circuitOpen = taskErrorState.isOpen("PARAM_TYPE_MISMATCH")
@@ -716,10 +746,12 @@ export async function presentAssistantMessage(cline: Task) {
 						metadata: { ...signal.metadata, structuralPreflight: true, occurrence, circuitOpen },
 					})
 
+					const structuralGuide = taskErrorState.consumePendingNativeProtocolGuide()
+					const structuralBase = guided ?? formatResponse.toolError(errorMessage)
 					cline.pushToolResultToUserContent({
 						type: "tool_result",
 						tool_use_id: sanitizeToolUseId(toolCallId),
-						content: guided ?? formatResponse.toolError(errorMessage),
+						content: structuralGuide ? `${structuralBase}\n\n${structuralGuide}` : structuralBase,
 						is_error: true,
 					})
 
@@ -789,13 +821,15 @@ export async function presentAssistantMessage(cline: Task) {
 						metadata: validationMetadata,
 					})
 					// Push tool_result directly without setting didAlreadyUseTool
+					const validationGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
+					const validationBase = guided ? `${guided}\n\n${errorMessage}` : errorMessage
 					cline.pushToolResultToUserContent({
 						type: "tool_result",
 						tool_use_id: sanitizeToolUseId(toolCallId),
-						content: guided ?? errorMessage,
+						content: validationGuide ? `${validationBase}\n\n${validationGuide}` : validationBase,
 						is_error: true,
 					})
-
+	
 					break
 				}
 			}
@@ -1119,10 +1153,12 @@ export async function presentAssistantMessage(cline: Task) {
 						toolName: block.name,
 						metadata: { typeMismatch: true },
 					})
+					const unknownToolGuide = getTaskErrorState(cline).consumePendingNativeProtocolGuide()
+					const unknownToolBase = guided ?? formatResponse.toolError(errorMessage)
 					cline.pushToolResultToUserContent({
 						type: "tool_result",
 						tool_use_id: sanitizeToolUseId(toolCallId),
-						content: guided ?? formatResponse.toolError(errorMessage),
+						content: unknownToolGuide ? `${unknownToolBase}\n\n${unknownToolGuide}` : unknownToolBase,
 						is_error: true,
 					})
 					break

src/core/tools/error-interception/ErrorClassifier.ts

@@ -57,6 +57,15 @@ function isSafeFactKey(key: string): boolean {
 	return !SENSITIVE_KEYS.has(key)
 }
 
+function hasToolContext(signal: InterceptionSignal): boolean {
+	return signal.toolName !== undefined || signal.toolCallId !== undefined
+}
+
+function isEligible(pattern: ErrorPattern, signal: InterceptionSignal): boolean {
+	if (pattern.category === "UNCLASSIFIED") return false
+	return !pattern.requiresToolContext || hasToolContext(signal)
+}
+
 function sanitizeFacts(signal: InterceptionSignal, pattern: ErrorPattern): Readonly<Record<string, unknown>> {
 	const facts: Record<string, unknown> = {}
 
@@ -87,7 +96,7 @@ function sanitizeFacts(signal: InterceptionSignal, pattern: ErrorPattern): Reado
 export function classifyError(signal: InterceptionSignal, _options?: ClassifyOptions): ErrorClassification {
 	// First pass: exact/structural matchers only.
 	for (const pattern of ERROR_PATTERNS) {
-		if (pattern.category === "UNCLASSIFIED") continue
+		if (!isEligible(pattern, signal)) continue
 		if (pattern.matches(signal)) {
 			return {
 				category: pattern.category,
@@ -102,7 +111,7 @@ export function classifyError(signal: InterceptionSignal, _options?: ClassifyOpt
 	// Second pass: heuristic fallback matchers, excluding the UNCLASSIFIED
 	// catch-all at the end of the list.
 	for (const pattern of ERROR_PATTERNS) {
-		if (pattern.category === "UNCLASSIFIED") continue
+		if (!isEligible(pattern, signal)) continue
 		if (pattern.fallback?.(signal)) {
 			return {
 				category: pattern.category,

src/core/tools/error-interception/MessageTransformer.ts

@@ -8,22 +8,7 @@ import {
 import type { ErrorCategory, ErrorClassification, ErrorSource, GuidancePayload, TransformOptions } from "./types"
 
 function countUtf8Bytes(text: string): number {
-	let count = 0
-	for (let i = 0; i < text.length; i++) {
-		const code = text.charCodeAt(i)
-		if (code <= 0x7f) {
-			count += 1
-		} else if (code <= 0x7ff) {
-			count += 2
-		} else if (code >= 0xd800 && code <= 0xdbff) {
-			// Surrogate pair: count 4 bytes for the pair.
-			count += 4
-			i++
-		} else {
-			count += 3
-		}
-	}
-	return count
+	return new TextEncoder().encode(text).length
 }
 
 function clampNextItems(next: string[]): string[] {
@@ -34,7 +19,7 @@ function clampNextItems(next: string[]): string[] {
 		if (candidate.length > NEXT_ITEM_CHAR_LIMIT) {
 			candidate = candidate.slice(0, NEXT_ITEM_CHAR_LIMIT)
 		}
-		candidate = candidate.replace(/[\udc00-\udfff]/g, "")
+		candidate = candidate.replace(/[\ud800-\udbff](?![\udc00-\udfff])|(?<![\ud800-\udbff])[\udc00-\udfff]/g, "")
 		if (candidate.length === 0) continue
 		clamped.push(candidate)
 	}

src/core/tools/error-interception/StructuralValidator.ts

@@ -219,6 +219,7 @@ function visitNested(
 				return nested
 			}
 		}
+		state.seen.delete(value)
 		return { found: false }
 	}
 
@@ -233,6 +234,7 @@ function visitNested(
 			return nested
 		}
 	}
+	state.seen.delete(value)
 	return { found: false }
 }
 

src/core/tools/error-interception/TaskErrorState.ts

@@ -34,6 +34,13 @@ interface CategoryState {
 export class TaskErrorState {
 	private readonly perCategory = new Map<string, CategoryState>()
 
+	/**
+	 * Pending XML_NATIVE_DUAL_PROTOCOL guidance queued by the text-block
+	 * handler. Consumed (read + cleared) by every path that emits a
+	 * tool_result for the turn so it cannot leak into later turns.
+	 */
+	private pendingGuide: string | undefined
+
 	private getOrCreate(category: string): CategoryState {
 		let state = this.perCategory.get(category)
 		if (!state) {
@@ -102,6 +109,31 @@ export class TaskErrorState {
 		}
 		this.perCategory.clear()
 	}
+
+	/** Returns the pending native protocol guide without clearing it. */
+	public getPendingNativeProtocolGuide(): string | undefined {
+		return this.pendingGuide
+	}
+
+	/** Queues a native protocol guide to be merged into the next tool_result. */
+	public setPendingNativeProtocolGuide(guide: string): void {
+		this.pendingGuide = guide
+	}
+
+	/** Clears any pending native protocol guide. */
+	public clearPendingNativeProtocolGuide(): void {
+		this.pendingGuide = undefined
+	}
+
+	/**
+	 * Atomically reads and clears the pending native protocol guide.
+	 * Returns undefined when no guide is queued.
+	 */
+	public consumePendingNativeProtocolGuide(): string | undefined {
+		const guide = this.pendingGuide
+		this.pendingGuide = undefined
+		return guide
+	}
 }
 
 /**

src/core/tools/error-interception/ToolErrorInterceptor.ts

@@ -218,7 +218,11 @@ export class ToolErrorInterceptor {
 				})
 				const transformed = this.transformSignal(task, signal, taskState)
 				if (transformed !== undefined) {
-					;(rawPushToolResult as (content: ToolResponse, ...rest: unknown[]) => void)(transformed, ...rest)
+					const nonTextBlocks = content.filter((item) => item.type !== "text")
+					;(rawPushToolResult as (content: ToolResponse, ...rest: unknown[]) => void)(
+						[{ type: "text", text: transformed } as (typeof content)[number], ...nonTextBlocks],
+						...rest,
+					)
 					return
 				}
 			}