fix: address review findings from PR #475 and #477#479
Merged
Conversation
- Add default case to parseAnthropicResponseJSON() to preserve unknown
block types as text instead of silently dropping them (C1)
- Validate scope enum values ('project' | 'global') in filterOps()
instead of accepting any string (C2)
- Add whitespace trimming and rejection to looksLikeApiKey() (M2)
- Remove overly broad /onnxruntime/i pattern from beforeSend filter,
keep only the three specific ONNX patterns (M4)
- Narrow 'Incorrect API key' pattern to 'Incorrect API key provided'
to avoid matching unrelated auth errors (M5)
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
1 participant
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
Addresses findings from the adversarial self-review of PR #475 and #477.
Changes
C1 — Unknown block types silently dropped in
parseAnthropicResponseJSONAdded a
defaultcase to the content block switch that preserves unknown block types as serialized JSON text. This matches the pattern used intoGatewayBlock()for request parsing and prevents silent data loss if Anthropic adds new content block types.C2 — Invalid scope values accepted by
filterOps()Changed scope validation from
typeof o.scope === "string"too.scope === "project" || o.scope === "global". Previously, an LLM producing an invalid scope like"session"or""would pass validation and create all entries withprojectPath: undefined(global scope) regardless of intent.M2 —
looksLikeApiKey()doesn't trim or reject whitespaceAdded
key.trim()and a/\s/check. Environment variables can contain trailing newlines which would pass the length check but fail at the API.M4 —
/onnxruntime/ipattern too broadRemoved the overly broad
/onnxruntime/ipattern fromTRANSIENT_ERROR_PATTERNS. The three specific patterns that follow it (Cannot find package,LoadLibrary failed,Protobuf parsing failed) already cover the known ONNX init failures without risking silencing real bugs.M5 —
/Incorrect API key/icould match unrelated auth errorsNarrowed to
/Incorrect API key provided/iwhich is the specific error message format returned by the OpenAI SDK, avoiding false positives from other auth error formats.Test Plan