feat: Add Error Interception Middleware for guided AI self-correcting tool errors#1009
Open
myk1yt wants to merge 3 commits into
Open
feat: Add Error Interception Middleware for guided AI self-correcting tool errors#1009myk1yt wants to merge 3 commits into
myk1yt wants to merge 3 commits into
Conversation
…ware Implements a middleware layer that intercepts, classifies, and transforms tool execution errors into structured WHAT/WHY/NEXT guidance messages, enabling AI models to self-correct without user intervention. New modules: - errorPatterns.ts: 11-category pattern DB with variant entries - ErrorClassifier.ts: Two-pass deterministic classifier - MessageTransformer.ts: Versioned JSON guidance serializer - ToolErrorInterceptor.ts: Interceptor facade with per-task state - TaskErrorState.ts: WeakMap-based task-scoped persistent state - StructuralValidator.ts: CWD + nested parameter preflight validator - DIFF_MATCH_FAILED pattern for apply_diff near-miss errors - Improved parallel tool call guidance for CWD_OBJECT_MISUSE and NESTED_PARAM_OVERFLOW Closes Zoo-Code-Org#1000
myk1yt
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
July 24, 2026 18:25
Contributor
📝 WalkthroughWalkthroughChangesGuided error interception
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (10)
src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts (1)
182-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuided-payload assertions were weakened to a shared substring across both assistant-message specs.
guided_tool_erroris the payloadtypefor every non-api_requestcategory, so all three assertions pass even if the protocol violation degrades toUNCLASSIFIED— the interception wiring is no longer pinned to a classification.
src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts#L182-L182: parse the text block as JSON and assertcategory === "INVALID_TOOL_PROTOCOL"(and optionallypattern_id).src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts#L286-L286: apply the same parsed-category assertion for the rejected-subsequent-call case.src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts#L130-L130: apply the same parsed-category assertion for the missing-idlegacy tool call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts` at line 182, Replace the shared substring checks with JSON parsing and assert category === "INVALID_TOOL_PROTOCOL" in presentAssistantMessage-images.spec.ts at lines 182-182 and 286-286, and presentAssistantMessage-unknown-tool.spec.ts at line 130; optionally also assert pattern_id, while preserving each test’s existing scenario and ensuring classification cannot degrade to UNCLASSIFIED.Source: Path instructions
src/core/tools/error-interception/ErrorClassifier.ts (1)
118-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the catch-all by category rather than by array position.
ERROR_PATTERNS[length - 1]silently changes meaning if a pattern is ever appended afterEI/UNCLASSIFIED/001, and it types as possibly-undefined undernoUncheckedIndexedAccess.♻️ Proposed refactor
- const fallback = ERROR_PATTERNS[ERROR_PATTERNS.length - 1] + const fallback = ERROR_PATTERNS.find((pattern) => pattern.category === "UNCLASSIFIED") + if (!fallback) { + throw new Error("errorPatterns: missing UNCLASSIFIED catch-all pattern") + } return {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/ErrorClassifier.ts` around lines 118 - 125, Update the fallback selection in ErrorClassifier to locate the catch-all pattern by its stable category or identifier, rather than relying on ERROR_PATTERNS array position. Ensure the lookup is narrowed or validated so it cannot produce an undefined fallback under noUncheckedIndexedAccess, while preserving the existing fallback response fields and heuristic behavior.src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts (1)
96-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the three new pattern variants.
EI/PARAM_TYPE_MISMATCH/002(CWD_OBJECT_MISUSE),EI/PARAM_TYPE_MISMATCH/003(NESTED_PARAM_OVERFLOW), andEI/INVALID_TOOL_PROTOCOL/002(XML_NATIVE_DUAL_PROTOCOL) are the PR's headline classifications but have no direct assertions — including the priority ordering that must keep 002/003 ahead of the genericEI/PARAM_TYPE_MISMATCH/001, and theerrorMessageIncludestext fallbacks that couple toStructuralValidatorwording.As per path instructions, "Use package-local unit tests for pure logic, parsing, state transitions, validation, serialization, request construction, retry decisions, and error handling".
💚 Suggested tests
+ it("classifies CWD_OBJECT_MISUSE ahead of the generic type mismatch", () => { + const signal = baseSignal({ + source: "validation", + stage: "preflight", + toolName: "execute_command", + metadata: { variant: "CWD_OBJECT_MISUSE", typeMismatch: true }, + }) + const result = classifyError(signal) + expect(result.patternId).toBe("EI/PARAM_TYPE_MISMATCH/002") + }) + + it("classifies NESTED_PARAM_OVERFLOW from the error message fallback", () => { + const signal = baseSignal({ + source: "validation", + stage: "preflight", + toolName: "read_file", + error: { message: "Detected nested tool input object in parameter" }, + metadata: {}, + }) + const result = classifyError(signal) + expect(result.patternId).toBe("EI/PARAM_TYPE_MISMATCH/003") + }) + + it("classifies XML markup alongside a native call as XML_NATIVE_DUAL_PROTOCOL", () => { + const signal = baseSignal({ + source: "parser", + stage: "parse", + metadata: { xmlNativeDualProtocol: true, xmlToolCall: true }, + }) + const result = classifyError(signal) + expect(result.patternId).toBe("EI/INVALID_TOOL_PROTOCOL/002") + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts` around lines 96 - 178, Extend the classifyError tests around baseSignal and classifyError with direct coverage for CWD_OBJECT_MISUSE (EI/PARAM_TYPE_MISMATCH/002), NESTED_PARAM_OVERFLOW (003), and XML_NATIVE_DUAL_PROTOCOL (EI/INVALID_TOOL_PROTOCOL/002). Assert their categories and pattern IDs, including cases where errorMessageIncludes text triggers each classification. Add precedence cases proving 002 and 003 win over generic PARAM_TYPE_MISMATCH/001, and verify the StructuralValidator wording fallbacks.Source: Path instructions
src/core/tools/error-interception/types.ts (1)
60-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the local
ToolResponseto avoid shadowing the existing public tool contract.The module re-exports this local type from
index.ts, so importingToolResponsehere produces a structurally incompatible type compared with the shared contract (string | Array<Anthropic.TextBlockParam | Anthropic.ImageBlockParam>), regardless of whether it is currently consumed outside this module.♻️ Suggested rename
-export interface ToolResponse { +export interface StructuredToolResult { type?: string status?: string error?: unknown text?: string toolUseId?: string [key: string]: unknown }Update
InterceptionSignal.result?: ToolResponse, theerrorPatterns.ts/ErrorClassifier.tsreferences, andsrc/core/tools/error-interception/index.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/types.ts` around lines 60 - 71, Rename the local ToolResponse interface to a distinct interception-specific type, then update InterceptionSignal.result, all references in errorPatterns.ts and ErrorClassifier.ts, and the re-export in error-interception/index.ts to use the new name. Keep the shared public ToolResponse contract unshadowed and unchanged.src/core/tools/error-interception/__tests__/StructuralValidator.spec.ts (2)
150-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese bound tests pass even with the bounds removed.
The deep fixture bottoms out at
{ leaf: 1 }and the wide fixture uses{ child: i }— neither is a tool signature, so detection would returnnullregardless ofNESTED_DETECTION_MAX_DEPTH/NESTED_DETECTION_MAX_NODES. Plant a detectable signature beyond the bound so the assertion actually pins the limit.💚 Make the fixtures detectable-but-out-of-bounds
it("bounds recursion to NESTED_DETECTION_MAX_DEPTH", () => { - let deep: Record<string, unknown> = { leaf: 1 } + // Tool-shaped leaf placed deeper than the bound: must NOT be detected. + let deep: Record<string, unknown> = { command: "x" } for (let i = 0; i < NESTED_DETECTION_MAX_DEPTH + 3; i += 1) { deep = { wrap: deep } } @@ it("bounds total visited nodes to NESTED_DETECTION_MAX_NODES", () => { const wide: Record<string, unknown> = {} for (let i = 0; i < NESTED_DETECTION_MAX_NODES + 10; i += 1) { - wide[`k${i}`] = { child: i } + wide[`k${i}`] = { child: i } } + // Tool-shaped node beyond the visit budget: must NOT be detected. + wide[`k${NESTED_DETECTION_MAX_NODES + 10}`] = { command: "x" } expect(NESTED_DETECTION_MAX_NODES).toBeGreaterThan(0)As per coding guidelines, "Use package-local unit tests for pure logic, parsing, state transitions, validation, serialization, request construction, retry decisions, and error handling."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/__tests__/StructuralValidator.spec.ts` around lines 150 - 168, Update the deep and wide fixtures in the tests named “bounds recursion to NESTED_DETECTION_MAX_DEPTH” and “bounds total visited nodes to NESTED_DETECTION_MAX_NODES” so each contains a recognizable tool-signature pattern only beyond its respective depth or node limit. Keep the existing validateNestedParams assertions expecting null, ensuring the tests would fail if either bound were removed.Source: Coding guidelines
101-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis case exercises the explicit signature set, not the multi-known-keys heuristic.
{ path, regex }already matchesTOOL_SIGNATURE_KEY_SETS. Use two keys that only appear inKNOWN_PARAMETER_KEYS(e.g.{ mode: "x", message: "y" }) and assertstructuralReasonisnested-tool-input:multi-known-keys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/__tests__/StructuralValidator.spec.ts` around lines 101 - 104, Update the test case around validateNestedParams to use two keys that are only in KNOWN_PARAMETER_KEYS, such as mode and message, rather than the explicit signature pair path and regex. Assert that the returned signal’s structuralReason equals nested-tool-input:multi-known-keys.src/core/tools/error-interception/MessageTransformer.ts (1)
10-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHand-rolled UTF-8 counting duplicates the
TextEncoderhelper below and mis-sizes unpaired surrogates.Line 18 assumes every high surrogate is followed by a low surrogate, counting 4 bytes and skipping the next code unit; for an unpaired high surrogate followed by a multi-byte char the count is short (
"\ud800€"→ 4 vs 6 actual), which undermines the "byte length <= byteLimit" guarantee documented at Line 155. The module already has a correct implementation ingetPayloadByteLength(Line 172).♻️ Use the encoder for both call sites
-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 -} +const utf8Encoder = new TextEncoder() + +function countUtf8Bytes(text: string): number { + return utf8Encoder.encode(text).length +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/MessageTransformer.ts` around lines 10 - 27, Replace the hand-rolled countUtf8Bytes implementation with the module’s existing TextEncoder-based getPayloadByteLength logic, and update both call sites to reuse that implementation. Remove the surrogate-specific counting so unpaired surrogates are encoded correctly and the documented byteLimit guarantee is preserved.src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts (1)
56-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the push/handle ordering this test claims, and cover array error content.
The implementation pushes the guided payload before awaiting
rawHandleError(ToolErrorInterceptor.ts Lines 153-173), so the title is inverted and the ordering — which the callers' exactly-once guard depends on — is unverified.mock.invocationCallOrderpins it. Also worth adding a case where the error result is[text, image], since that path currently forwards only the transformed string.💚 Suggested assertion
- it("pushes a transformed result after the raw error", async () => { + it("pushes a transformed result before forwarding the raw error", async () => { @@ expect(pushToolResult).toHaveBeenCalledTimes(1) + expect(pushToolResult.mock.invocationCallOrder[0]).toBeLessThan(handleError.mock.invocationCallOrder[0]) const result = JSON.parse((pushToolResult.mock.calls[0] as [string])[0])As per coding guidelines, "Use package-local unit tests for pure logic, parsing, state transitions, validation, serialization, request construction, retry decisions, and error handling."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts` around lines 56 - 77, Update the test around createInterceptor and decoratedHandleError to assert the claimed ordering with mock.invocationCallOrder, verifying raw handleError completes before pushToolResult is invoked; rename the test if needed to reflect that order. Add a focused case covering an error result containing [text, image] and assert the pushed payload preserves both elements rather than only the transformed string.Source: Coding guidelines
src/core/tools/error-interception/ToolErrorInterceptor.ts (2)
63-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the circuit-open literal as
GuidancePayloadso it can't drift from the contract.The literal duplicates
GUIDANCE_VERSIONand isn't checked againstGuidancePayload(types.ts:110-121); it also reportsoccurrence: 1while the real count is available at Lines 258-262, which hides how many shell failures actually occurred.♻️ Proposed change
-/** Circuit-open payload used when the shell integration breaker trips. */ -const CIRCUIT_OPEN_MESSAGE = { - version: 1, +/** Circuit-open payload used when the shell integration breaker trips. */ +const CIRCUIT_OPEN_MESSAGE: GuidancePayload = { + version: GUIDANCE_VERSION, status: "error",Add the imports:
-import type { ErrorCategory, ErrorClassification, ErrorSource, ErrorStage, InterceptionSignal } from "./types" +import { GUIDANCE_VERSION } from "./errorPatterns" +import type { + ErrorCategory, + ErrorClassification, + ErrorSource, + ErrorStage, + GuidancePayload, + InterceptionSignal, +} from "./types"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/ToolErrorInterceptor.ts` around lines 63 - 78, Type CIRCUIT_OPEN_MESSAGE as GuidancePayload and reuse GUIDANCE_VERSION instead of duplicating the version literal. Update its occurrence field at the construction or emission site to use the actual shell-failure count available in the circuit-breaker flow around lines 258–262, rather than the fixed value 1.
153-174: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the exactly-once contract part of the interceptor API.
decoratedHandleErroremits onerawPushToolResult(transformed(...)), then delegates torawHandleError, whose current callers also callrawPushToolResult(formatResponse.toolError(...)). Existing call sites avoid doubletool_resultby managing their ownhasToolResultflag elsewhere, but that contract is only local; new callers or guard removals can emit the UI/raw error payload twice. Explicit this requirement inDecoratedCallbacks/createInterceptor, change the wrapper to swallow the raw push after a guided payload, or move deduplication into the interceptor layer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/ToolErrorInterceptor.ts` around lines 153 - 174, Make the exactly-once tool-result contract explicit in DecoratedCallbacks/createInterceptor and enforce it in decoratedHandleError. When transformSignal produces a guided payload and rawPushToolResult is called, prevent the subsequent rawHandleError path from emitting another tool result; preserve the raw error emission when no transformed payload exists. Update callback wiring or deduplication at the interceptor layer so callers no longer need separate hasToolResult guards.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts`:
- Around line 96-97: Update the unknown-tool error path exercised by
presentAssistantMessage so the guided payload preserves the offending native
tool name and the instruction pointing to available tools. Add these facts to
the emitted signal or prefix the guided payload, while retaining the existing
guided_tool_error structure and metadata.
In `@src/core/assistant-message/presentAssistantMessage.ts`:
- Line 362: Use one module-scoped ToolErrorInterceptor in
presentAssistantMessage.ts so its perTask state persists across content blocks.
Replace the per-block createToolErrorInterceptor() at lines 362-362 and reuse
the same instance at lines 117-118 for the mcp_tool_use case, allowing both tool
paths to share task counters and circuit state.
- Around line 341-354: Move pendingNativeProtocolGuide from the undeclared cline
property onto the task-scoped state returned by getTaskErrorState(cline), adding
a declared Task/state field. Consume and clear the guide whenever this turn
emits a tool_result, including direct pushToolResultToUserContent paths and the
no-tool early-break path, while preserving the existing rawPushToolResult
behavior so it cannot leak into a later turn.
- Around line 679-684: In the structural signal construction within
presentAssistantMessage, invoke validateCwdParameter only when block.name
identifies the execute_command tool; continue running validateNestedParams for
all tools and preserve the existing filtering and error handling.
- Around line 770-795: Replace the validation-message substring checks in the
preflight error handling around validateToolUse with a typed error or stable
error code exposed by validateToolUse, mapping that code to validationMetadata
without relying on wording. When constructing the tool_result in
presentAssistantMessage, preserve the original errorMessage alongside any
interceptor-guided payload so the model retains the specific tool and
restriction details.
- Around line 689-701: The PARAM_TYPE_MISMATCH path in presentAssistantMessage
must reset its persisted error state when the structural fingerprint changes, so
different tools or parameters receive fresh guidance instead of inheriting an
open circuit. Update the logic around taskErrorState.setFingerprint,
incrementOccurrence, and isOpen to compare the prior fingerprint and reset the
PARAM_TYPE_MISMATCH state before incrementing when it differs; preserve existing
same-fingerprint circuit behavior.
In `@src/core/tools/error-interception/__tests__/MessageTransformer.spec.ts`:
- Around line 143-153: Update the test around transformErrorToMessage to provide
non-ASCII content through the classification/template input and set a byteLimit
that forces fitPayloadWithinByteLimit to truncate it. Assert the resulting
message remains valid JSON and preserves correct handling of surrogate pairs
after truncation, rather than relying on the ASCII EI/PARAM_MISSING/001 template
without a byte limit.
In `@src/core/tools/error-interception/ErrorClassifier.ts`:
- Around line 87-115: Enforce pattern.requiresToolContext in both matching
passes of classifyError: skip any pattern requiring tool context when the signal
lacks toolName and toolCallId. Preserve exact matching, heuristic fallback, and
UNCLASSIFIED exclusions for eligible patterns, and ensure bare signals cannot
select tool-bound classifications.
In `@src/core/tools/error-interception/errorPatterns.ts`:
- Around line 163-179: Update the invalid-argument pattern in errorPatterns.ts
to match JSON-RPC code -32602 as a number using the appropriate typed code
matcher instead of errorCodeIs. In ErrorClassifier.spec.ts, change the
corresponding fixture error to use numeric code -32602 with message "Invalid
params"; preserve the existing classification expectations.
In `@src/core/tools/error-interception/MessageTransformer.ts`:
- Around line 10-27: Update countUtf8Bytes to use the module’s shared
TextEncoder-based byte-counting helper instead of the manual UTF-8 loop. In the
surrogate-cleaning helper at
src/core/tools/error-interception/MessageTransformer.ts lines 29-42, remove only
orphaned high and low surrogates, preserving valid surrogate pairs during
clamping.
- Around line 137-149: The minimal fallback in MessageTransformer must still
honor the caller-supplied byteLimit. Update the fallback path that builds the
minimal GuidancePayload and calls serializePayload to ensure its serialized
result is within byteLimit, clamping degradable fields as needed while
preserving category and pattern_id; alternatively, explicitly document and
enforce a minimum supported byteLimit.
In `@src/core/tools/error-interception/StructuralValidator.ts`:
- Around line 35-82: The global KNOWN_PARAMETER_KEYS two-key heuristic
over-classifies legitimate nested schema objects as tool invocations. Update
validateNestedParams to validate nested objects only against the current tool’s
declared schema and explicitly allowed object parameters, removing or narrowing
the shared-key scan while preserving genuine nested invocation detection.
- Around line 194-237: Update visitNested so state.seen tracks only objects on
the current recursion path: add a value when entering it and remove it before
every return after its children are processed. Preserve cycleDetected for
back-edges while allowing shared non-cyclic references in sibling branches to be
traversed without reporting a cycle.
In `@src/core/tools/error-interception/ToolErrorInterceptor.ts`:
- Around line 207-224: Update the array-result handling around transformSignal
so transformed error results retain all non-text content blocks, including
images, alongside the guided text. Build the transformed response from the
existing text and the original content items rather than forwarding only the
string, while preserving the current fail-open behavior and metadata handling.
---
Nitpick comments:
In `@src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts`:
- Line 182: Replace the shared substring checks with JSON parsing and assert
category === "INVALID_TOOL_PROTOCOL" in presentAssistantMessage-images.spec.ts
at lines 182-182 and 286-286, and presentAssistantMessage-unknown-tool.spec.ts
at line 130; optionally also assert pattern_id, while preserving each test’s
existing scenario and ensuring classification cannot degrade to UNCLASSIFIED.
In `@src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts`:
- Around line 96-178: Extend the classifyError tests around baseSignal and
classifyError with direct coverage for CWD_OBJECT_MISUSE
(EI/PARAM_TYPE_MISMATCH/002), NESTED_PARAM_OVERFLOW (003), and
XML_NATIVE_DUAL_PROTOCOL (EI/INVALID_TOOL_PROTOCOL/002). Assert their categories
and pattern IDs, including cases where errorMessageIncludes text triggers each
classification. Add precedence cases proving 002 and 003 win over generic
PARAM_TYPE_MISMATCH/001, and verify the StructuralValidator wording fallbacks.
In `@src/core/tools/error-interception/__tests__/StructuralValidator.spec.ts`:
- Around line 150-168: Update the deep and wide fixtures in the tests named
“bounds recursion to NESTED_DETECTION_MAX_DEPTH” and “bounds total visited nodes
to NESTED_DETECTION_MAX_NODES” so each contains a recognizable tool-signature
pattern only beyond its respective depth or node limit. Keep the existing
validateNestedParams assertions expecting null, ensuring the tests would fail if
either bound were removed.
- Around line 101-104: Update the test case around validateNestedParams to use
two keys that are only in KNOWN_PARAMETER_KEYS, such as mode and message, rather
than the explicit signature pair path and regex. Assert that the returned
signal’s structuralReason equals nested-tool-input:multi-known-keys.
In `@src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts`:
- Around line 56-77: Update the test around createInterceptor and
decoratedHandleError to assert the claimed ordering with
mock.invocationCallOrder, verifying raw handleError completes before
pushToolResult is invoked; rename the test if needed to reflect that order. Add
a focused case covering an error result containing [text, image] and assert the
pushed payload preserves both elements rather than only the transformed string.
In `@src/core/tools/error-interception/ErrorClassifier.ts`:
- Around line 118-125: Update the fallback selection in ErrorClassifier to
locate the catch-all pattern by its stable category or identifier, rather than
relying on ERROR_PATTERNS array position. Ensure the lookup is narrowed or
validated so it cannot produce an undefined fallback under
noUncheckedIndexedAccess, while preserving the existing fallback response fields
and heuristic behavior.
In `@src/core/tools/error-interception/MessageTransformer.ts`:
- Around line 10-27: Replace the hand-rolled countUtf8Bytes implementation with
the module’s existing TextEncoder-based getPayloadByteLength logic, and update
both call sites to reuse that implementation. Remove the surrogate-specific
counting so unpaired surrogates are encoded correctly and the documented
byteLimit guarantee is preserved.
In `@src/core/tools/error-interception/ToolErrorInterceptor.ts`:
- Around line 63-78: Type CIRCUIT_OPEN_MESSAGE as GuidancePayload and reuse
GUIDANCE_VERSION instead of duplicating the version literal. Update its
occurrence field at the construction or emission site to use the actual
shell-failure count available in the circuit-breaker flow around lines 258–262,
rather than the fixed value 1.
- Around line 153-174: Make the exactly-once tool-result contract explicit in
DecoratedCallbacks/createInterceptor and enforce it in decoratedHandleError.
When transformSignal produces a guided payload and rawPushToolResult is called,
prevent the subsequent rawHandleError path from emitting another tool result;
preserve the raw error emission when no transformed payload exists. Update
callback wiring or deduplication at the interceptor layer so callers no longer
need separate hasToolResult guards.
In `@src/core/tools/error-interception/types.ts`:
- Around line 60-71: Rename the local ToolResponse interface to a distinct
interception-specific type, then update InterceptionSignal.result, all
references in errorPatterns.ts and ErrorClassifier.ts, and the re-export in
error-interception/index.ts to use the new name. Keep the shared public
ToolResponse contract unshadowed and unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 591abfc9-8f7f-474a-b10d-3e3c23ea80c8
📒 Files selected for processing (16)
src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.tssrc/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.tssrc/core/assistant-message/presentAssistantMessage.tssrc/core/tools/error-interception/ErrorClassifier.tssrc/core/tools/error-interception/MessageTransformer.tssrc/core/tools/error-interception/StructuralValidator.tssrc/core/tools/error-interception/TaskErrorState.tssrc/core/tools/error-interception/ToolErrorInterceptor.tssrc/core/tools/error-interception/__tests__/ErrorClassifier.spec.tssrc/core/tools/error-interception/__tests__/MessageTransformer.spec.tssrc/core/tools/error-interception/__tests__/StructuralValidator.spec.tssrc/core/tools/error-interception/__tests__/TaskErrorState.spec.tssrc/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.tssrc/core/tools/error-interception/errorPatterns.tssrc/core/tools/error-interception/index.tssrc/core/tools/error-interception/types.ts
Comment on lines
+137
to
+149
| const minimal: GuidancePayload = { | ||
| version: GUIDANCE_VERSION, | ||
| status: "error", | ||
| type, | ||
| category: payload.category, | ||
| what: "Error.", | ||
| why: "Error.", | ||
| next: [], | ||
| retryable: payload.retryable, | ||
| occurrence: payload.occurrence, | ||
| pattern_id: payload.pattern_id, | ||
| } | ||
| return serializePayload(minimal) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Minimal fallback is returned without re-checking byteLimit.
byteLimit is caller-supplied (TransformOptions.byteLimit), and category + pattern_id are not degradable, so for small limits this return value can exceed the bound the docblock at Line 155 guarantees. Either clamp the remaining fields or document that byteLimit has a floor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/tools/error-interception/MessageTransformer.ts` around lines 137 -
149, The minimal fallback in MessageTransformer must still honor the
caller-supplied byteLimit. Update the fallback path that builds the minimal
GuidancePayload and calls serializePayload to ensure its serialized result is
within byteLimit, clamping degradable fields as needed while preserving category
and pattern_id; alternatively, explicitly document and enforce a minimum
supported byteLimit.
Comment on lines
+35
to
+82
| const OBJECT_ALLOWED_PARAMETERS: Readonly<Record<string, ReadonlySet<string>>> = { | ||
| read_file: new Set(["indentation"]), | ||
| use_mcp_tool: new Set(["arguments"]), | ||
| } | ||
|
|
||
| /** | ||
| * Known tool-shaped signatures. A nested object is treated as a tool input | ||
| * only when it contains at least one of these key sets. Matching requires | ||
| * all listed keys to be present in the same object. | ||
| */ | ||
| const TOOL_SIGNATURE_KEY_SETS: ReadonlyArray<ReadonlyArray<string>> = [ | ||
| ["command"], | ||
| ["path", "regex"], | ||
| ["query", "path"], | ||
| ["server_name", "tool_name"], | ||
| ["path", "content"], | ||
| ["pattern", "file_pattern"], | ||
| ] | ||
|
|
||
| /** | ||
| * Recognized parameter keys used for the "multiple known keys from a | ||
| * different invocation" heuristic. Two or more of these keys appearing | ||
| * together inside a nested object is treated as a tool input signature. | ||
| */ | ||
| const KNOWN_PARAMETER_KEYS: ReadonlySet<string> = new Set([ | ||
| "command", | ||
| "cwd", | ||
| "path", | ||
| "regex", | ||
| "file_pattern", | ||
| "query", | ||
| "content", | ||
| "diff", | ||
| "pattern", | ||
| "server_name", | ||
| "tool_name", | ||
| "arguments", | ||
| "uri", | ||
| "line_number", | ||
| "offset", | ||
| "limit", | ||
| "mode", | ||
| "prompt", | ||
| "slug", | ||
| "name", | ||
| "message", | ||
| "todos", | ||
| ]) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect native argument schemas to see which tools legitimately nest objects/arrays of objects.
fd -t f -e ts . packages/types/src | head -30
rg -nP 'apply_diff|write_to_file|read_file|update_todo_list' -g 'packages/types/**/*.ts' -C6 | head -160
# Native arg shapes consumed by the tools themselves
rg -nP '\bnativeArgs\b' -g '!**/__tests__/**' -C4 src/core/tools | head -160Repository: Zoo-Code-Org/Zoo-Code
Length of output: 1453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== structural validator outline =="
ast-grep outline src/core/tools/error-interception/StructuralValidator.ts --view expanded || true
echo "== structural validator relevant lines =="
sed -n '1,240p' src/core/tools/error-interception/StructuralValidator.ts | nl -ba
echo "== presentAssistantMessage relevant lines =="
rg -n "consecutiveMistakeCount|tool_result|StructuralValidator" src/core -C 5 | head -220
echo "== tool schemas mentioning apply_diff/write_to_file/update_todo_list =="
rg -n "apply_diff|write_to_file|update_todo_list|read_file|use_mcp_tool|command.*path|path.*content|diff|content" packages/types/src -C 8 | head -260
echo "== files mentioning StructuralValidator =="
rg -n "StructuralValidator|interceptError|interceptTool" src/core packages/types -C 3 | head -260
echo "== package tool files =="
fd -t f . src/core/tools | sort | sed -n '1,200p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 1329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== structural validator relevant lines =="
sed -n '1,260p' src/core/tools/error-interception/StructuralValidator.ts | cat -n
echo "== presentAssistantMessage / tool_result / StructuralValidator usages =="
rg -n "consecutiveMistakeCount|tool_result|StructuralValidator|validateNestedParams|validateCwdParameter|InterceptionSignal" src/core packages/types -C 5 | head -260
echo "== tool schemas mentioning native argument keys =="
rg -n "apply_diff|write_to_file|update_todo_list|read_file|use_mcp_tool|command.*path|path.*content|arguments|diff|content|todos" packages/types/src src/core -C 8 | head -340
echo "== files mentioning StructuralValidator =="
rg -n "StructuralValidator|interceptError|interceptTool" src/core packages/types -C 3 | head -260
echo "== repository TypeScript files listing (sample) =="
git ls-files '**/*.ts' | sed -n '1,200p'
echo "== candidate tool schema files =="
git ls-files | grep -E '(packages/types/src|src/core/tools).*(schema|tool|argument|typescript).*ts$' | sed -n '1,200p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== structural validator signals =="
sed -n '239,295p' src/core/tools/error-interception/StructuralValidator.ts | cat -n
echo "== exact validator imports/usages =="
rg -n "validateNestedParams|validateCwdParameter|INTERCEPT.*SIG|interceptTool|interceptError|StructuralValidator" src/core packages/types --glob '*.ts' --glob '!**/node_modules/**' --glob '!**/__tests__/**' | head -220
echo "== call sites with context =="
rg -n "validateNestedParams|validateCwdParameter" src/core packages/types --glob '*.ts' --glob '!**/node_modules/**' --glob '!**/__tests__/**' -C 20 | head -260Repository: Zoo-Code-Org/Zoo-Code
Length of output: 21032
Scope nested-param validation to schema-approved object values
validateNestedParams runs for every native tool and treats any nested object containing two KNOWN_PARAMETER_KEYS as a nested tool invocation, then blocks execution with tool_result. This breaks legitimate schemas such as edit.apply_diff.entry, write_to_file.diff, update_todo_list, and use_mcp_tool.arguments; it also makes read_file.indentation pass only by the narrow current allowlist. Remove or narrow the global two-key heuristic, or validate nested object parameters against each tool’s declared schema instead of scanning a shared key vocabulary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/tools/error-interception/StructuralValidator.ts` around lines 35 -
82, The global KNOWN_PARAMETER_KEYS two-key heuristic over-classifies legitimate
nested schema objects as tool invocations. Update validateNestedParams to
validate nested objects only against the current tool’s declared schema and
explicitly allowed object parameters, removing or narrowing the shared-key scan
while preserving genuine nested invocation detection.
Comment on lines
+194
to
+237
| function visitNested( | ||
| value: unknown, | ||
| topParameter: string, | ||
| depth: number, | ||
| state: { visited: number; seen: Set<unknown> }, | ||
| ): NestedSearchResult { | ||
| if (value === null || typeof value !== "object") { | ||
| return { found: false } | ||
| } | ||
| if (state.seen.has(value)) { | ||
| return { found: false, cycleDetected: true } | ||
| } | ||
| state.seen.add(value) | ||
| state.visited += 1 | ||
| if (state.visited > NESTED_DETECTION_MAX_NODES) { | ||
| return { found: false, nodeLimitExceeded: true } | ||
| } | ||
| if (depth > NESTED_DETECTION_MAX_DEPTH) { | ||
| return { found: false, depthExceeded: true } | ||
| } | ||
|
|
||
| if (Array.isArray(value)) { | ||
| for (const item of value) { | ||
| const nested = visitNested(item, topParameter, depth + 1, state) | ||
| if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) { | ||
| return nested | ||
| } | ||
| } | ||
| return { found: false } | ||
| } | ||
|
|
||
| const record = value as Record<string, unknown> | ||
| const signature = detectToolSignature(record) | ||
| if (signature !== undefined) { | ||
| return { found: true, parameter: topParameter, signature } | ||
| } | ||
| for (const child of Object.values(record)) { | ||
| const nested = visitNested(child, topParameter, depth + 1, state) | ||
| if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) { | ||
| return nested | ||
| } | ||
| } | ||
| return { found: false } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
seen is a global visited set, so shared (non-cyclic) references are reported as cycles.
Entries are added at Line 206 and never removed, so a DAG where two sibling keys reference the same object yields cycleDetected — which the caller turns into a blocking NESTED_PARAM_OVERFLOW result (presentAssistantMessage.ts Lines 719-726). Scope the set to the current traversal path.
🐛 Path-scoped cycle detection
if (Array.isArray(value)) {
for (const item of value) {
const nested = visitNested(item, topParameter, depth + 1, state)
if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) {
return nested
}
}
+ state.seen.delete(value)
return { found: false }
}
@@
for (const child of Object.values(record)) {
const nested = visitNested(child, topParameter, depth + 1, state)
if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) {
return nested
}
}
+ state.seen.delete(value)
return { found: false }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function visitNested( | |
| value: unknown, | |
| topParameter: string, | |
| depth: number, | |
| state: { visited: number; seen: Set<unknown> }, | |
| ): NestedSearchResult { | |
| if (value === null || typeof value !== "object") { | |
| return { found: false } | |
| } | |
| if (state.seen.has(value)) { | |
| return { found: false, cycleDetected: true } | |
| } | |
| state.seen.add(value) | |
| state.visited += 1 | |
| if (state.visited > NESTED_DETECTION_MAX_NODES) { | |
| return { found: false, nodeLimitExceeded: true } | |
| } | |
| if (depth > NESTED_DETECTION_MAX_DEPTH) { | |
| return { found: false, depthExceeded: true } | |
| } | |
| if (Array.isArray(value)) { | |
| for (const item of value) { | |
| const nested = visitNested(item, topParameter, depth + 1, state) | |
| if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) { | |
| return nested | |
| } | |
| } | |
| return { found: false } | |
| } | |
| const record = value as Record<string, unknown> | |
| const signature = detectToolSignature(record) | |
| if (signature !== undefined) { | |
| return { found: true, parameter: topParameter, signature } | |
| } | |
| for (const child of Object.values(record)) { | |
| const nested = visitNested(child, topParameter, depth + 1, state) | |
| if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) { | |
| return nested | |
| } | |
| } | |
| return { found: false } | |
| } | |
| function visitNested( | |
| value: unknown, | |
| topParameter: string, | |
| depth: number, | |
| state: { visited: number; seen: Set<unknown> }, | |
| ): NestedSearchResult { | |
| if (value === null || typeof value !== "object") { | |
| return { found: false } | |
| } | |
| if (state.seen.has(value)) { | |
| return { found: false, cycleDetected: true } | |
| } | |
| state.seen.add(value) | |
| state.visited += 1 | |
| if (state.visited > NESTED_DETECTION_MAX_NODES) { | |
| return { found: false, nodeLimitExceeded: true } | |
| } | |
| if (depth > NESTED_DETECTION_MAX_DEPTH) { | |
| return { found: false, depthExceeded: true } | |
| } | |
| if (Array.isArray(value)) { | |
| for (const item of value) { | |
| const nested = visitNested(item, topParameter, depth + 1, state) | |
| if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) { | |
| return nested | |
| } | |
| } | |
| state.seen.delete(value) | |
| return { found: false } | |
| } | |
| const record = value as Record<string, unknown> | |
| const signature = detectToolSignature(record) | |
| if (signature !== undefined) { | |
| return { found: true, parameter: topParameter, signature } | |
| } | |
| for (const child of Object.values(record)) { | |
| const nested = visitNested(child, topParameter, depth + 1, state) | |
| if (nested.found || nested.cycleDetected || nested.depthExceeded || nested.nodeLimitExceeded) { | |
| return nested | |
| } | |
| } | |
| state.seen.delete(value) | |
| return { found: false } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/tools/error-interception/StructuralValidator.ts` around lines 194 -
237, Update visitNested so state.seen tracks only objects on the current
recursion path: add a value when entering it and remove it before every return
after its children are processed. Preserve cycleDetected for back-edges while
allowing shared non-cyclic references in sibling branches to be traversed
without reporting a cycle.
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
Fix e2e apply_diff fixture to expect guided JSON payload format instead of raw error text strings. The interceptor now transforms apply_diff errors into structured DIFF_MATCH_FAILED guidance. Add 16 new tests: - 8 integration tests for presentAssistantMessage error-interception paths (XML detection, structural preflight, guide consumption) - 8 edge case tests for ToolErrorInterceptor (array results with images, isErrorResult/inferStatus branches)
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts (1)
623-644: 📐 Maintainability & Code Quality | 🔵 TrivialStrengthen assertion in the file-not-found
inferStatustest.This test only checks
toHaveBeenCalledTimes(1)— that's true whether the content was transformed or passed through unchanged, so it doesn't actually verify thatinferStatus/classification detected "File does not exist" the way the sibling test above (Lines 596-621) does.♻️ Suggested stronger assertion
decoratedPushToolResult(content) expect(pushToolResult).toHaveBeenCalledTimes(1) + const pushed = (pushToolResult.mock.calls[0] as unknown as [Array<Record<string, unknown>>])[0] + expect(String(pushed[0].text)).toContain("guided_tool_error") + expect(String(pushed[0].text)).toContain("FILE_NOT_FOUND") })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts` around lines 623 - 644, Strengthen the assertion in the “infers 'file-not-found' status when text contains 'File does not exist'” test to verify the argument passed to pushToolResult includes the expected file-not-found classification, matching the sibling inferStatus test’s assertion pattern. Keep the existing invocation-count check and test setup unchanged.ci-fix-commit.ps1 (1)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPersonal dev-workflow artifacts committed to the repo; consider removing before merge. All three files are local git-automation helpers (hardcoded paths/remote/branch) that bypass commit hooks, with no functional relationship to the interception middleware feature itself.
ci-fix-commit.ps1#L1-L16: remove, or move out of version control (e.g. local-only script /.gitignored dotfile) since it hardcodescd Zoo-Code, usesgit commit --no-verify, and disables Husky via$env:HUSKY = "0".commit-and-push.ps1#L1-L5: remove for the same reason; also note$env:HUSKY = "0"is set after thegit commitcall it's meant to affect, so it has no effect there.commit-message.txt#L1-L23: remove alongside the scripts above, as it's only consumed bycommit-and-push.ps1 -F commit-message.txt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-fix-commit.ps1` around lines 1 - 16, Remove the personal git-automation artifacts from version control: delete ci-fix-commit.ps1, commit-and-push.ps1, and commit-message.txt. Do not replace them with repository scripts; keep these local-only if needed and exclude them via gitignore.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ci-fix-commit.ps1`:
- Around line 1-16: Remove the personal git-automation artifacts from version
control: delete ci-fix-commit.ps1, commit-and-push.ps1, and commit-message.txt.
Do not replace them with repository scripts; keep these local-only if needed and
exclude them via gitignore.
In `@src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts`:
- Around line 623-644: Strengthen the assertion in the “infers 'file-not-found'
status when text contains 'File does not exist'” test to verify the argument
passed to pushToolResult includes the expected file-not-found classification,
matching the sibling inferStatus test’s assertion pattern. Keep the existing
invocation-count check and test setup unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca838f27-d1e4-4679-ab8d-2e729b854bc6
📒 Files selected for processing (6)
apps/vscode-e2e/src/fixtures/apply-diff.tsci-fix-commit.ps1commit-and-push.ps1commit-message.txtsrc/core/assistant-message/__tests__/presentAssistantMessage-error-interception.spec.tssrc/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts
github-actions
Bot
added
the
awaiting-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements a deterministic error interception middleware that transforms raw LLM tool execution errors into structured WHAT/WHY/NEXT guidance messages, enabling AI models to self-correct without user intervention.
Closes #1000
Architecture
New Modules
errorPatterns.tsErrorClassifier.tsMessageTransformer.tsToolErrorInterceptor.tsTaskErrorState.tsStructuralValidator.tsCovered Error Categories (11)
Design Principles
Test Coverage
tsc --noEmit)pnpm buildsuccessfulPR Scope
This PR includes error-interception middleware only. The
Task.tsMODEL_STUCK_LOOP circuit breaker extension is deferred to a follow-up PR due to cross-branch dependencies with the stats feature.Summary by CodeRabbit
cwdand nested tool-shaped inputs) and improved handling when legacy and native tool markup conflict.