CI security + Cohere v2 parity + Bedrock ResponseOverrides#135
Closed
jpr5 wants to merge 36 commits intofix/omnibus-cr-followupfrom
Closed
CI security + Cohere v2 parity + Bedrock ResponseOverrides#135jpr5 wants to merge 36 commits intofix/omnibus-cr-followupfrom
jpr5 wants to merge 36 commits intofix/omnibus-cr-followupfrom
Conversation
…recording - Guard res.writeHead(502) with headersSent check in proxy catch block - Guard res.setHeader with headersSent check on disk-failure path - Add client disconnect detection in SSE relay (destroyed/writableEnded guards) - Add logger parameter to buildFixtureResponse, log base64 decode failures - Log encoding_format JSON parse errors instead of silently swallowing - Relay raw buffer for audio/* content types to prevent UTF-8 corruption - Return ContentWithToolCallsResponse when both content and toolCalls exist in streaming collapse, OpenAI, Anthropic, Gemini, Bedrock, and Ollama branches
… thought parts Add source: "proxy" to journal.add() calls in the proxy code path (proxyAndRecord success) for gemini.ts, messages.ts, and responses.ts handlers. Add source?: "proxy" to JournalEntry response type. Filter out thought parts (parts with thought: true) when converting Gemini-format messages to OpenAI-format in geminiToCompletionRequest(), preventing internal reasoning content from leaking into converted text.
… comment, VideoStateMap TTL - Add source: "proxy" to all proxy-path journal.add() calls in embeddings, images, speech, transcription, and video handlers - Add optional source field to JournalEntry response type - Accept optional providerKey parameter in handleEmbeddings (defaults to "openai") for Azure embedding routing - Replace extractFormField JSDoc with fragility documentation noting regex runs against binary audio data - Replace VideoStateMap type alias with a class that enforces 10k max entries and 1-hour TTL with lazy eviction - Update server.ts and index.ts for VideoStateMap class change
…journal source field Add isContentWithToolCallsResponse type guard to all 4 Bedrock handler functions (handleBedrock, handleBedrockStream, handleConverse, handleConverseStream). Fixtures with both content and toolCalls now produce combined responses instead of falling through to 500 errors. Add source: "proxy" to proxy-path journal entries in both files so proxy-originated requests are distinguishable from fixture-served ones. Add source field to JournalEntry response type.
… to cohere/ollama handlers Add isContentWithToolCallsResponse type guard to handleCohere, handleOllama, and handleOllamaGenerate so fixtures with both content and toolCalls no longer fall through to 500 errors. For handleOllamaGenerate, tool call fixtures now return a clear 400 error directing users to /api/chat instead of a misleading 500 "unknown type" error. Add source: "proxy" to all proxy-path journal entries in both handlers. Add the optional source field to JournalEntry type.
1. Update aimock_fixtures_loaded gauge after control API mutations (POST /fixtures, DELETE /fixtures, POST /reset) 2. Bind errors in control API catch blocks, log via defaults.logger, include error details in response bodies for debuggability 3. Fix one-shot error fixture race condition by removing fixture synchronously instead of deferring via queueMicrotask 4. Add 10MB size limit to readBody to prevent unbounded accumulation 5. Replace magic number 7 with "/openai".length in normalizeCompatPath 6. Pass provider key to handleEmbeddings for Azure embedding routing 7. Standardize handler dispatch from .then/.catch chains to try/await
When a Gemini model turn has both text parts and functionCall parts, the text was silently discarded because content was hardcoded to null. Now the text is extracted and included as content alongside tool_calls. Also defaults missing functionCall args to empty object.
…uilders
buildCohereToolCallResponse and buildCohereContentWithToolCallsResponse
validated JSON but still passed through the original (possibly malformed)
string. Now they use the same let-argsJson pattern as the streaming
equivalents, falling back to "{}" on parse failure.
- Re-export buildContentWithToolCallsStreamEvents from src/index.ts (was exported from responses.ts but missing from barrel) - Remove unnecessary 50ms setTimeout in control-api test that referenced removed queueMicrotask cleanup - Replace "LLMock" with "aimock" in embeddings.ts and responses.ts module doc comments - Use local `logger` instead of `defaults.logger` in messages.ts to match the rest of the file
…nect Copy base64 embedding buffer to an aligned Uint8Array before creating Float32Array, eliminating RangeError from pooled Buffer byteOffset misalignment. Remove now-unnecessary try/catch and unused logger param. Surface clientDisconnected flag from makeUpstreamRequest and skip fixture persistence entirely when the client disconnects mid-stream, preventing truncated fixtures from being saved to disk or memory.
…during await refactor
Video detection guard: reject responses with choices/content/candidates/ message/data/object fields that indicate non-video API formats. Error detection guard: require obj.error to be an object with a string message property, matching the actual ErrorResponse shape instead of matching any truthy error field. Cohere v2 native detection: add a dedicated branch before the Ollama fallback that recognizes top-level finish_reason + message.content array of typed objects, avoiding misclassification through the Ollama path.
…eaming error format
handleCompletions now validates that body.messages is an array after
JSON parsing, returning a 400 with a clear error message when missing.
handleBedrockStream error responses now use the Anthropic envelope
format ({ type: "error", error: { type, message } }) matching the
non-streaming handleBedrock handler.
…int gap Use a running counter across the entire conversation in geminiToCompletionRequest instead of per-block index, preventing duplicate tool_call_id values when the same function appears at the same position in different turns. Add a comment noting unrecognized Gemini roles are intentionally dropped (no logger in scope). Add fieldEqual(a.endpoint, b.endpoint) to matchCriteriaEqual so fixtures differing only by endpoint are not treated as siblings for sequence counting.
- Accept plain string for ResponsesRequest.input (matching real OpenAI API) instead of silently iterating over characters when a string is passed - Add explicit comment for skipped unrecognized item types in responsesInputToMessages (item_reference, local_shell_call, etc.) - Validate embeddingReq.input is present before normalizing, returning a 400 error instead of silently producing "undefined" text - Update provider-compat test to verify successful fixture match now that string input is handled correctly
Ollama defaults to stream: true when absent, but ollamaToCompletionRequest was passing through undefined. Add ?? true to match Ollama's actual default. Add required field validation for images (prompt), speech (input), and video (prompt) endpoints that was missing after JSON parsing.
… matchFixture matchesPattern (helpers.ts) is case-insensitive by design for search/rerank/ moderation. matchFixture (router.ts) uses case-sensitive matching for chat fixture authors who expect exact string values. Added JSDoc comments to both sites documenting the intentional divergence.
…xtureResponse Cohere v2 responses place tool calls at message.tool_calls (message level), not as typed blocks inside the content array. The existing Cohere detector only checked content blocks for type: "tool_call", silently dropping all tool calls when recording Cohere responses with both text and tool_calls. Now also checks msg.tool_calls alongside the existing content-block scan, matching how Ollama detection already handles message-level tool_calls.
… updateProviderCounts, extractFeatures computeChanges compared currentCell === "No" but actual cells contain <span class="no">✗</span>. Now checks for class="no", unicode cross-mark, or ✗ entity. applyChanges regex looked for <td class="no"> but actual structure is <td><span class="no">. Updated to match all <td> elements and check inner content for no-class spans. updateProviderCounts replaced all "N providers" strings globally, risking corruption of aimock's own claims. Now scoped to the competitor's table column and prose paragraphs mentioning the competitor by name. extractFeatures keywords like "embed", "video", "image", "docker" produced false positives on general text. Tightened to multi-word patterns and API paths (e.g. /v1/embeddings instead of embed).
- Replace --admin with --auto on gh pr merge so branch protection is respected (fix-drift.yml) - Replace grep -oP (Perl regex) with grep -oE (extended regex) for portability, add guard for missing PR URL (fix-drift.yml) - Pass SLACK_WEBHOOK via env vars instead of interpolating secrets directly in shell commands across all five workflow files - Add SLACK_WEBHOOK guard (skip gracefully when unset) to every Slack notification step
…rock ResponseOverrides Cohere: - Add reasoning support to all 3 response builders (text, tool call, content+tool calls) for both streaming and non-streaming - Add webSearches warning logs matching the pattern in messages.ts - Forward response_format from CohereRequest to ChatCompletionRequest - Map assistant tool_calls in cohereToCompletionRequest (was dropping them, only mapping role+content) - Add cohereFinishReason/cohereUsage helpers to wire ResponseOverrides into finish_reason and usage fields (was only using id override) Bedrock (invoke + invoke-with-response-stream): - Wire extractOverrides into all response branches (text, tool call, content+tool calls) for both handlers - Add bedrockStopReason/bedrockUsage helpers for provider-specific finish reason and usage mapping - Add webSearches warning logs for all response types - Remove duplicate buildBedrockStreamContentWithToolCallsEvents and duplicate isContentWithToolCallsResponse handler blocks Bedrock Converse (converse + converse-stream): - Add extractOverrides + ResponseOverrides imports - Add converseStopReason/converseUsage helpers with camelCase field mapping (inputTokens/outputTokens/totalTokens) - Wire overrides into all 3 response builders - Add webSearches warning logs for text and content+tool calls in both handlers Tests: 24 new tests covering all 5 issues across providers
…erse errors, loadFixtures warning, Ollama generate recording
…ta type, patchEnv save/restore
…collapse path buildFixtureResponse: all 4 provider branches (OpenAI, Anthropic, Gemini, Bedrock Converse) now include reasoning in tool-call-only returns, not just content+toolCalls returns. Streaming collapse: proxyAndRecord now propagates collapsed.reasoning into the fixtureResponse for all branch paths (content-only, toolCalls-only, content+toolCalls).
…t error propagation
Recorder buildFixtureResponse: Anthropic, Gemini, Bedrock Converse branches
now use filter+join for text blocks instead of find (preserves all text blocks,
not just the first). Anthropic thinking-only responses (no text, no tool_use)
now return { content: "", reasoning } instead of falling through to error.
makeUpstreamRequest: req.destroy() on client disconnect now passes an Error
argument so the upstream response stream emits an error event and the promise
settles immediately instead of hanging until the 30s body timeout.
…rough to error fixture
OpenAI, Gemini, Bedrock Converse branches in buildFixtureResponse now return
{ content: "" } when the response shape is recognized but content is empty
(e.g., content filtering, zero max_tokens). Previously these fell through all
detection branches and were saved as "Could not detect response format" error
fixtures.
Also reverts req.destroy(Error) back to req.destroy() on client disconnect —
the error arg caused the clientDisconnected code path (fixture-skip logic) to
become unreachable dead code.
jpr5
added a commit
that referenced
this pull request
Apr 23, 2026
…rock overrides + recorder correctness Folds PR #135 into PR #133. 5 commits: - CI workflow security hardening (5 workflows) - Cohere v2 complete parity (reasoning, webSearches, response_format, tool_calls, overrides) - Bedrock ResponseOverrides + stream format + Converse error wrapping - Recorder correctness (tool-call IDs, reasoning, multi-block text, empty-content, Ollama generate) - Router RegExp g-flag safety + test framework patchEnv save/restore + loadFixtures warnings
jpr5
force-pushed
the
fix/omnibus-cr-followup
branch
2 times, most recently
from
6ce5dfa to
6d3f4dc
Compare
Contributor
Author
|
Folded into PR #133 — all commits regrouped by area of concern. |
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
Stacked on PR #133 (omnibus CR follow-up). Addresses bucket (d) items 2 and 3 from the PR #133 CR rounds.
CI Security Hardening
--adminwith--autoon fix-drift auto-merge (respects branch protection)grep -oPwith portablegrep -oECohere v2 Complete Parity
Bedrock ResponseOverrides
Test plan