fix: catch XML validation errors and unknown tool errors in processToolUse - #1337
awhite0030 wants to merge 5 commits into
Conversation
…olUse Move the checks for `__xml_validation_error__`, missing tool registry, and unknown tools inside the try/catch block of `processToolUse`. This prevents these validation cases from throwing uncaught errors and ensures they are correctly caught and wrapped in a formatted ToolResult with `isError: true` so the model can self-correct on the next turn.
nc-review: needs work — 1 blocking, 1 important, 1 nit@awhite0030 — there is a blocking item below. The fix itself is correct and minimal: moving the XML-validation-error, uninitialized-registry, and unknown-tool checks inside the 🔴 blocking · The diff for 🟠 important · Issue #1303 lists three checks that throw outside the try block: ⚪ nit · Worth flagging for awareness, not blocking: in the interactive loop, 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
Description
Root cause: The checks for
__xml_validation_error__, uninitialized tool registry, and unknown tools inprocessToolUsewere placed outside of thetry/catchblock. Consequently, when triggered, they threw uncaught exceptions instead of returning a properly formattedToolResulterror object for the LLM.Fix: Moved the initial validation checks inside the
tryblock insource/message-handler.tsso exceptions thrown by these checks are properly caught and formatted into a{ role: 'tool', isError: true, content: 'Error: ...' }structure. Tests were updated insource/message-handler.spec.tsto expectToolResultoutputs rather than thrown exceptions for these cases. A changeset was added.Validation:
Fixes #1303
Fixes #1303
Type of Change
Changeset
pnpm changeset) describing this change for the changelogDocs-only or internal chores need no changeset (or run
pnpm changeset --emptyto note that intentionally).Testing
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)Manual Testing
Checklist