-
Notifications
You must be signed in to change notification settings - Fork 221
feat: error-interception-middleware (2/3) #1126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
myk1yt
wants to merge
8
commits into
Zoo-Code-Org:main
Choose a base branch
from
myk1yt:pr/b02-error-runtime-v2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
8491155
feat(error): define error contracts and classification types
14ad8eb
feat(error): add error transformation and interception runtime
e0ea632
fix(error-interception): add null guard to getTaskState to prevent We…
99ac95e
fix(error-interception): guard WeakMap accessors against non-object t…
367d4b6
chore: remove temp file progress.txt
1a4ff0a
Merge branch 'main' into pr/b02-error-runtime-v2
myk1yt c5c69d1
test(e2e): add error-interception runtime suite
45b024f
fix(e2e): correct eslint-disable rule name in error-interception-runt…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
608 changes: 608 additions & 0 deletions
608
apps/vscode-e2e/src/suite/error-interception-runtime.test.ts
Large diffs are not rendered by default.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,272 @@ | ||
| import { ERROR_PATTERNS } from "./errorPatterns" | ||
| import type { ClassifyOptions, ErrorClassification, ErrorPattern, InterceptionSignal } from "./types" | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Safe-identifier validation (prompt-injection prevention) | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| const SAFE_IDENTIFIER_RE = /^[a-zA-Z_][\w.]*$/ | ||
| const MAX_PARAM_NAME_LENGTH = 128 | ||
|
|
||
| /** | ||
| * Returns `true` only when `name` is a safe identifier suitable for | ||
| * interpolation into model-facing guidance text. | ||
| * | ||
| * Accepts plain identifiers (`path`, `file_pattern`) and dotted member | ||
| * access chains (`options.timeout`). Rejects anything that could carry | ||
| * prompt-injection payloads: newlines, quotes, angle brackets, brackets, | ||
| * shell metacharacters, backslashes, and overlength strings. | ||
| */ | ||
| export function isValidIdentifier(name: string | undefined): boolean { | ||
| if (typeof name !== "string") return false | ||
| if (name.length === 0 || name.length > MAX_PARAM_NAME_LENGTH) return false | ||
| if (!SAFE_IDENTIFIER_RE.test(name)) return false | ||
| // Reject instruction-like patterns. | ||
| if (/[\n\r"'><\[\]{}()|;`\\]/.test(name)) return false | ||
| return true | ||
| } | ||
|
|
||
| const SAFE_FACT_KEYS = new Set<string>([ | ||
| "category", | ||
| "code", | ||
| "commandSubmitted", | ||
| "contextLengthExceeded", | ||
| "contextOverflow", | ||
| "contextWindowExceeded", | ||
| "errorCode", | ||
| "errorName", | ||
| "errorSource", | ||
| "errorStage", | ||
| "errorType", | ||
| "emptyArguments", | ||
| "fileNotFound", | ||
| "fileRestriction", | ||
| "invalidProtocol", | ||
| "missingNativeArgs", | ||
| "missingParameter", | ||
| "missingRequiredParameters", | ||
| "modeRestriction", | ||
| "parameterName", | ||
| "parseFailureKind", | ||
| "pathEmpty", | ||
| "repetitionCount", | ||
| "retryDisposition", | ||
| "server", | ||
| "shellIntegrationError", | ||
| "status", | ||
| "tool", | ||
| "toolName", | ||
| "type", | ||
| "typeMismatch", | ||
| "unknownTool", | ||
| "validSiblingPresent", | ||
| "xmlToolCall", | ||
| ]) | ||
|
|
||
| const SENSITIVE_KEYS = new Set<string>([ | ||
| "command", | ||
| "commandText", | ||
| "cwd", | ||
| "env", | ||
| "environmentVariable", | ||
| "path", | ||
| "absolutePath", | ||
| "homePath", | ||
| "apiKey", | ||
| "api_key", | ||
| "token", | ||
| "secret", | ||
| "password", | ||
| "prompt", | ||
| "response", | ||
| "resultText", | ||
| "mcpArguments", | ||
| "arguments", | ||
| "args", | ||
| ]) | ||
|
|
||
| function isSafeFactKey(key: string): boolean { | ||
| if (!SAFE_FACT_KEYS.has(key)) return false | ||
| return !SENSITIVE_KEYS.has(key) | ||
| } | ||
|
|
||
| function hasToolContext(signal: InterceptionSignal): boolean { | ||
| return signal.toolName !== undefined || signal.toolCallId !== undefined | ||
| } | ||
|
|
||
| /** | ||
| * Extract a parameter name from an error message or result text. | ||
| * | ||
| * Common patterns from tool execution errors: | ||
| * - "Required parameter 'path' is missing" | ||
| * - "The 'path' parameter must be a string" | ||
| * - "Missing required parameter: command" | ||
| * - "parameter 'path' is required" | ||
| */ | ||
| function extractParameterName(signal: InterceptionSignal): string | undefined { | ||
| // Check metadata first (explicitly provided by the caller). | ||
| const metaName = signal.metadata["parameterName"] | ||
| if (typeof metaName === "string" && metaName.length > 0) return metaName | ||
|
|
||
| // Try to extract from error.message. | ||
| if (signal.error !== null && typeof signal.error === "object") { | ||
| const message = (signal.error as { message?: unknown }).message | ||
| if (typeof message === "string") { | ||
| const name = tryExtractParamNameFromText(message) | ||
| if (name) return name | ||
| } | ||
| } | ||
|
|
||
| // Try to extract from result.text. | ||
| if (typeof signal.result === "object" && signal.result !== null) { | ||
| const text = (signal.result as { text?: unknown }).text | ||
| if (typeof text === "string") { | ||
| const name = tryExtractParamNameFromText(text) | ||
| if (name) return name | ||
| } | ||
| } | ||
|
|
||
| return undefined | ||
| } | ||
|
|
||
| function tryExtractParamNameFromText(text: string): string | undefined { | ||
| // Pattern: "parameter 'name'" or "parameter \"name\"" or "parameter: name" | ||
| const paramQuoteMatch = text.match(/parameter\s*['"']([^'"']+)['"']/i) | ||
| if (paramQuoteMatch) return paramQuoteMatch[1] | ||
|
|
||
| // Pattern: "Required parameter 'name'" — already covered above, but also | ||
| // try "Missing required parameter: name" (colon-separated, no quotes). | ||
| const colonMatch = text.match(/(?:missing|required)\s+parameter\s*[:\s]+(\w+)/i) | ||
| if (colonMatch) return colonMatch[1] | ||
|
|
||
| // Pattern: "The 'name' parameter must be..." — extract the quoted name | ||
| // before the word "parameter". | ||
| const theParamMatch = text.match(/the\s+['"']([^'"']+)['"']\s+parameter/i) | ||
| if (theParamMatch) return theParamMatch[1] | ||
|
|
||
| return 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> = {} | ||
|
|
||
| for (const key of Object.keys(signal.metadata)) { | ||
| if (!isSafeFactKey(key)) continue | ||
|
|
||
| const value = signal.metadata[key] | ||
| if (value === undefined || value === null) continue | ||
|
|
||
| if (typeof value === "boolean" || typeof value === "number" || typeof value === "string") { | ||
| facts[key] = value | ||
| continue | ||
| } | ||
|
|
||
| // Arrays of primitive tool/server identifiers only. | ||
| if (Array.isArray(value) && value.every((item) => typeof item === "string")) { | ||
| facts[key] = value | ||
| } | ||
| } | ||
|
|
||
| // Validate metadata-provided parameterName through the same | ||
| // safe-identifier check. The loop above copies metadata values | ||
| // verbatim, so an unsafe parameterName from metadata would bypass | ||
| // the extraction-path validation below. | ||
| if (typeof facts.parameterName === "string" && !isValidIdentifier(facts.parameterName)) { | ||
| delete facts.parameterName | ||
| } | ||
|
|
||
| facts.pattern = pattern.id | ||
| facts.category = pattern.category | ||
| facts.errorSource = signal.source | ||
|
|
||
| // Inject extracted parameter name for PARAM_MISSING and generic | ||
| // PARAM_TYPE_MISMATCH patterns so the transformer can include it in | ||
| // guidance messages. Skip the CWD_OBJECT_MISUSE and NESTED_PARAM_OVERFLOW | ||
| // variants — they have their own specific guidance. | ||
| if ( | ||
| pattern.category === "PARAM_MISSING" || | ||
| (pattern.category === "PARAM_TYPE_MISMATCH" && pattern.id === "EI/PARAM_TYPE_MISMATCH/001") | ||
| ) { | ||
| if (facts.parameterName === undefined) { | ||
| const paramName = extractParameterName(signal) | ||
| // Only store the parameter name if it passes the safe-identifier | ||
| // check. Untrusted content (file contents, shell/MCP output) can | ||
| // flow through error messages and result text, so we must reject | ||
| // anything that looks like a prompt-injection payload. | ||
| if (paramName !== undefined && isValidIdentifier(paramName)) { | ||
| facts.parameterName = paramName | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return Object.freeze(facts) | ||
| } | ||
|
|
||
| export function classifyError(signal: InterceptionSignal, _options?: ClassifyOptions): ErrorClassification { | ||
| // First pass: exact/structural matchers only. | ||
| for (const pattern of ERROR_PATTERNS) { | ||
| if (!isEligible(pattern, signal)) continue | ||
| if (pattern.matches(signal)) { | ||
| return { | ||
| category: pattern.category, | ||
| patternId: pattern.id, | ||
| confidence: "exact", | ||
| retryPolicy: pattern.retryPolicy, | ||
| facts: sanitizeFacts(signal, pattern), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Second pass: heuristic fallback matchers, excluding the UNCLASSIFIED | ||
| // catch-all at the end of the list. | ||
| for (const pattern of ERROR_PATTERNS) { | ||
| if (!isEligible(pattern, signal)) continue | ||
| if (pattern.fallback?.(signal)) { | ||
| return { | ||
| category: pattern.category, | ||
| patternId: pattern.id, | ||
| confidence: "heuristic", | ||
| retryPolicy: pattern.retryPolicy, | ||
| facts: sanitizeFacts(signal, pattern), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // UNCLASSIFIED catch-all. | ||
| const fallback = ERROR_PATTERNS[ERROR_PATTERNS.length - 1] | ||
| return { | ||
| category: fallback.category, | ||
| patternId: fallback.id, | ||
| confidence: "heuristic", | ||
| retryPolicy: fallback.retryPolicy, | ||
| facts: sanitizeFacts(signal, fallback), | ||
| } | ||
| } | ||
|
|
||
| /** Convenience helper to classify a structured tool result directly. */ | ||
| export function classifyToolResult( | ||
| result: InterceptionSignal["result"], | ||
| taskId: string, | ||
| toolCallId?: string, | ||
| ): ErrorClassification { | ||
| const metadata: Record<string, unknown> = {} | ||
| if (result && typeof result === "object") { | ||
| if (result.status) metadata.status = result.status | ||
| if (result.type) metadata.type = result.type | ||
| } | ||
|
|
||
| const signal: InterceptionSignal = { | ||
| source: "tool_result", | ||
| stage: "result", | ||
| taskId, | ||
| toolCallId, | ||
| result: result ?? undefined, | ||
| metadata, | ||
| } | ||
| return classifyError(signal) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
classifyToolResultcannot match tool-bound patterns whentoolCallIdis omitted.The signal built at Lines 263-270 sets no
toolName, andtoolCallIdis optional.hasToolContextthen returnsfalse,isEligiblerejects every pattern that setsrequiresToolContext, and the function returnsUNCLASSIFIED.ToolErrorInterceptor.transformToolResultforwards an optionaltoolCallId, so a caller that passes onlytaskIdsilently receives no guidance. Accept an optionaltoolNameand forward it.🐛 Proposed fix
Then forward
toolNamefromToolErrorInterceptor.transformToolResultoptions.🤖 Prompt for AI Agents