fix: request validation + OpenAI-compat correctness (#110)#117
Merged
Conversation
Three correctness/compat defects on the request path:
1. Non-array `messages` (e.g. {"messages":"x"}) passed the `!messages?.length`
guard but threw in `messages.reduce(...)`; since the handler runs without
await, the rejection silently hung the client until socket timeout. Now
guarded with `Array.isArray(messages) && length>0` → 400 invalid_request_error,
placed before the first use of `messages`.
2. OpenAI array `content` ([{type:"text",text},{type:"image_url",...}]) was
JSON.stringify'd into the prompt as literal noise. New `contentToText()` helper
flattens text parts and replaces non-text parts with a placeholder; used in
messagesToPrompt, extractSystemPrompt, and all promptChars char-count sites
(zero `JSON.stringify(m.content)` remain).
3. A streamed upstream error arriving after eager SSE headers emitted a bare
finish_reason:"stop" + [DONE] — byte-identical to a successful empty completion.
Both streaming error paths (the parsed.error result branch AND the non-zero-exit
close-handler branch — the latter folded in per reviewer) now emit an SSE
`data:{"error":{...}}` frame so clients can distinguish failure from empty.
Error-text sanitization across all emit sites is intentionally deferred to #110's
sibling issue #111 (security layer).
ALIGNMENT.md: these are OpenAI-compat shim + input-validation behaviors; OCP does
not forward, add, or alter any Anthropic API operation here (cli.js does not speak
the OpenAI wire format), so a cli.js citation is N/A under Rule 2. No blacklisted
tokens or port literals introduced; alignment.yml passes.
Independent fresh-context reviewer (opus): APPROVE WITH MINOR (Iron Rule 10) —
the close-handler sibling fold-in addresses MINOR #1; this commit body addresses
MINOR #2.
Closes #110.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Fixes the three request-path correctness/OpenAI-compat defects from the 2026-05-31 audit (#110).
messageshang —{"messages":"x"}passed!messages?.lengththen threw inmessages.reduce(...); the un-awaited handler turned it into an unhandledRejection that hung the client. NowArray.isArray(messages) && length>0→ 400invalid_request_error, before first use.contentflattening — newcontentToText()helper replacesJSON.stringify(m.content)inmessagesToPrompt,extractSystemPrompt, and all promptChars char-count sites; OpenAI array content (text parts /image_url) is now flattened to text + placeholder instead of raw JSON noise. ZeroJSON.stringify(m.content)remain.finish_reason:"stop"+[DONE](looks like success). Both streaming error paths (theparsed.errorresult branch and the non-zero-exit close-handler branch) now emit an SSEdata:{"error":{...}}frame.Error-text sanitization across emit sites is intentionally deferred to sibling issue #111 (security layer).
ALIGNMENT.md (server.mjs hard requirements)
alignment.ymlpasses.messagesuse (no bypass path),contentToTextcorrectness incl. hoisting, no promptChars correctness regression (string counts byte-identical; stats-only sink), the error-frame fix preserves theerrored/no-cache accounting, ALIGNMENT N/A agreed, andnpm test→ 148 passed, 0 failed. Verdict APPROVE WITH MINOR; both minors addressed (close-handler sibling folded in; this body carries the cli.js-N/A note).Tests
8 new tests (
contentToText5 cases + messages-guard truth table).npm test→ 148 passed, 0 failed.Closes #110.
🤖 Generated with Claude Code