Repository navigation
TXE-5086: Merge upstream transports/v2.2.3 as transport v2.2.3 - #23
Merged
Merged
Conversation
## Summary Replaces the per-chunk `MarshalJSON` serialization used to track replay buffer memory with a lightweight field-walking estimator, eliminating redundant serialization on the hot streaming path and fixing byte-counter drift during drain. ## Changes - Introduced `gateReplayEntry`, pairing each buffered chunk with its size computed once at append time, so the byte counter never re-serializes held chunks during drain or transform. - Replaced `chunkBytes` (which called `MarshalJSON` on every size check) with `estimateChunkBytes`, which walks only the carriers that grow with response size: passthrough bodies, raw request/response bytes, delta/text/argument strings, and composite item copies. Everything else is covered by a fixed `gateChunkSizeFloor` of 1 KB. - Added `gateRawBytes`, `gateStrPtrBytes`, `gateChoiceBytes`, and `gateCompositeBytes` as targeted helpers so composite objects (full items, parts, terminal message copies) are serialized only on item-boundary events, never on per-delta chunks. - `drainBufferLocked` now reads the cached size from the entry rather than recomputing it, making the byte counter exact rather than approximate. - `TransformPausedBuffer` pre-computes replacement sizes once and installs them alongside the new entries, avoiding a second sizing pass inside the lock. - Updated `gateReplayBuf` type from `[]*schemas.BifrostStreamChunk` to `[]gateReplayEntry` throughout `gate.go`, `types.go`, and `gate_test.go`. - Added tests covering the estimator's bulk-carrier counting and verifying that the byte counter returns to exactly zero after a pause-resume-drain cycle without re-serialization. ## Type of change - [x] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/streaming/... ``` Pause a stream carrying large passthrough or chat-delta chunks and verify `gateReplayBufBytes` matches `estimateChunkBytes` for each buffered entry. Resume and confirm the counter reaches zero after drain. ## Breaking changes - [x] No ## Security considerations None. This change affects only internal buffer accounting and does not touch authentication, secrets, or data leaving the process. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
## Summary Adds test coverage for the guardrails full-hold seam in the streaming gate, specifically targeting the passthrough SSE path used by the native Anthropic (claude-cli) integration. These tests validate the pause → transform → replay lifecycle under three distinct verdicts: redact-and-release, block-and-drop, and buffer overflow. ## Changes - Added `TestGate_FullHoldRedactReplaySSE`: verifies that chunks held in a paused gate do not escape to the wire, that `TransformPausedStreamBuffer` performs copy-on-write redaction correctly (original buffered chunks remain unmodified), that SSE event order is preserved on replay, and that the secret (`sk-ant-SECRET123`) is absent from the wire while the redaction placeholder is present. - Added `TestGate_BlockVerdictClearsBufferAndDeliversTerminal`: verifies that `ClearPausedStreamBuffer` drops all held content and that the client receives only the synthetic block-error terminal chunk; also asserts that subsequent sends are rejected after the stream is ended. - Added `TestGate_OverflowCapWithSSEPayloads`: drives large SSE payloads (~5 MB each) into a paused gate to confirm the 100 MB buffer cap trips at the expected chunk boundary, that the `paused_replay_buffer_overflow` error is delivered to the client, and that the gate is terminally closed afterward. - Added `TestGate_TransformInPlaceMutationIsRepriced`: pins the accounting guarantee that in-place mutations to chunk bodies inside a transform callback are re-priced after the callback returns, keeping `gateReplayBufBytes` and the per-entry cached size consistent with the actual post-mutation payload size. - Added helpers `ssePassthroughChunk`, `toolCarryingSSEStream`, and `passthroughBody` to construct realistic Anthropic SSE passthrough fixtures for use across the new tests. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/streaming/... -run "TestGate_FullHoldRedactReplaySSE|TestGate_BlockVerdictClearsBufferAndDeliversTerminal|TestGate_OverflowCapWithSSEPayloads|TestGate_TransformInPlaceMutationIsRepriced" -v ``` Expected: all four tests pass. `TestGate_OverflowCapWithSSEPayloads` will log the chunk index at which the cap was tripped; it must fall between 18 and 21 (inclusive). ## Breaking changes - [x] No ## Security considerations The redaction test explicitly asserts that a bearer token (`sk-ant-SECRET123`) does not appear anywhere on the wire after a transform, and that the copy-on-write contract is upheld so the original buffer is not silently mutated. This directly exercises the secret-scrubbing guarantee that guardrails rely on for the native Anthropic SSE path. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…nterface with scoped, identity-aware key bindings (maximhq#7176) ## Summary Session stickiness was implemented directly inside `selectKeyFromProviderForModel` as an inline block of KV store logic. This PR extracts that logic into a `SessionAffinity` interface, ships a default implementation (`keyAffinity`) that preserves the existing behaviour, and allows deployments to register their own policy through `BifrostConfig.SessionAffinity`. ## Changes - Introduced `schemas.SessionAffinity` interface with a single `ResolveKey` method, added to `BifrostConfig` as an optional field. - Extracted all session stickiness logic from `selectKeyFromProviderForModelWithPool` into `core/sessionaffinity.go` as `keyAffinity`, the default implementation installed when `BifrostConfig.SessionAffinity` is nil. - `keyAffinity` scopes session bindings to the virtual key and user from the request's grant identity, so a session ID reused by a different caller does not share bindings. The old implementation scoped only by provider, model, and session ID. - Session state keys are now built by `sessionStateKey`, which hashes the session ID, virtual key ID, user ID, provider, and model together under a `session:v2:` prefix. The old `buildSessionKey` helper and `hashSHA256` utility are removed. - `getCachedKeyFromStore` is removed; its logic lives in `keyAffinity.lookup` and `sessionStateString`. - The `sonic` import is moved from `bifrost.go` into `sessionaffinity.go` where it is actually used. - Added `core/sessionaffinity_test.go` covering key scoping, TTL handling, bind-then-reuse, stale binding replacement, decline conditions, concurrent first-request convergence, and per-caller isolation. - Updated the session stickiness documentation to mention `BifrostConfig.SessionAffinity` and the identity-scoped binding behaviour. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test ```sh go test ./core/... -run TestSessionAffinity go test ./core/... -run TestSelectKeyFromProviderForModel_SessionStickiness go test ./core/... -run TestSelectKeyFromProviderForModel_NoStickinessWithoutSessionID go test ./core/... ``` To exercise a custom policy, implement `schemas.SessionAffinity` and pass it as `BifrostConfig.SessionAffinity`. Requests carrying `BifrostContextKeySessionID` will be routed through `ResolveKey`; returning `ok=false` falls through to ordinary key selection. ## Breaking changes - [x] Yes - [ ] No Session state keys written by the previous implementation (`session:<provider>:<hashed-session-id>:<hashed-model>`) are not read by the new implementation (`session:v2:key:<hash-of-identity+session+provider+model>`). Existing KV entries will be ignored and will expire naturally; sessions will rebind on their next request. ## Security considerations Session IDs, virtual key IDs, user IDs, provider names, and model names are all hashed before being written to the KV store key, so no caller-supplied text or PII appears in store keys. Bindings are scoped to the grant identity attributed to the request, preventing session ID collisions across callers. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…h `x-bf-session-affinity` header opt-out (maximhq#7177) ## Summary Session affinity previously bound a session to a key at selection time, before any request was served. This meant the binding could be written by a request that ultimately failed, and the session had no concept of which provider or route served it — only which key was selected. This PR rewrites session affinity so that bindings are written only when a request is successfully served, introduces route-level affinity (provider stickiness alongside key stickiness), and exposes the full lifecycle to custom implementations through a revised `SessionAffinity` interface. ## Changes - **`SessionAffinity` interface extended**: Added `ResolveRoute` and `Observe` methods. `ResolveRoute` is called after routing hooks run and can reorder the provider/fallback chain. `Observe` is called once per request with the route and key that served it, or the error it ended in. Bindings are now written only from `Observe`, never from `ResolveKey`. - **Route-level stickiness**: The shipped `sessionAffinity` (formerly `keyAffinity`) now tracks which provider last served a session for a given requested route. On subsequent requests, if that provider appears anywhere in the chain, it is moved to the front. If it is already first, the chain is left unchanged and no routing decision is recorded. - **Bindings written on outcome, not on selection**: `ResolveKey` no longer selects a key for an unbound session or writes to the store. It only answers when a binding already exists and the bound key is still eligible. `Observe` writes the route and key bindings after a request is served, using `SetNX` for a first binding (so concurrent first requests converge on whichever finishes first) and `Set` for a refresh or move. - **`IsSessionAffinityActive` guard**: A new `BifrostContextKeySessionAffinity` context key (`x-bf-session-affinity` header) lets callers opt individual requests out of affinity. `IsSessionAffinityActive` checks both that a session ID is present and that affinity has not been switched off. Core consults this before every affinity call, so custom implementations never see requests that do not take part. - **`resolveSessionRoute` and `observeSessionOutcome`**: Two new internal helpers on `Bifrost` wire the new interface methods into `handleRequest` and `handleStreamRequest`. `resolveSessionRoute` runs after pre-request hooks and applies the chain the affinity returns. `observeSessionOutcome` is deferred and fires once with the served route and key, or the error. - **Key selection guard**: `selectKeyFromProviderForModelWithPool` now checks `IsSessionAffinityActive` before calling `ResolveKey`, so requests without a session or with affinity switched off never reach the affinity implementation. - **`RoutingEngineSessionAffinity` constant**: Added to the routing engine log vocabulary so the request trail records when a session moved a provider to the front or reused a key. - **Tests updated throughout**: `TestSelectKeyFromProviderForModel_SessionStickiness` and the no-rotation test are updated to reflect the new lifecycle. New tests cover `ResolveRoute`, `Observe`, `IsSessionAffinityActive`, `resolveSessionRoute`, `observeSessionOutcome`, and the key-pool guard. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... -run TestSessionAffinity go test ./core/... -run TestSelectKeyFromProviderForModel_SessionStickiness go test ./core/... -run TestResolveSessionRoute go test ./core/... -run TestObserveSessionOutcome go test ./core/... -run TestKeyPoolAsksAffinityOnlyForRequestsThatTakePart go test ./... ``` To exercise route affinity end-to-end, send requests with `x-bf-session-id` set and confirm that after the first request is served, subsequent requests to the same session are routed to the same provider even when the routing chain would otherwise propose a different one. To opt a request out, send `x-bf-session-affinity: off` and confirm the session binding is neither consulted nor updated for that request. ## Breaking changes - [x] Yes - [ ] No The `SessionAffinity` interface has two new required methods: `ResolveRoute` and `Observe`. Any custom implementation registered via `BifrostConfig.SessionAffinity` must implement both. `ResolveKey` no longer receives unbound sessions and will not be called for requests that do not carry a session ID or have affinity switched off. ## Related issues ## Security considerations The `x-bf-session-affinity` header is parsed and validated before being applied; unrecognised values are ignored with a warning. No session state is written for failed requests, reducing the risk of poisoning a session binding through a deliberately induced failure. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…affinity control and document provider-level session binding behavior (maximhq#7178) ## Summary Introduces a `BifrostContextKeySessionAffinity` context key (and corresponding `x-bf-session-affinity` HTTP header) that lets callers opt individual requests out of session-driven routing. Previously, session stickiness was all-or-nothing once a session ID was present; now a single request can bypass both provider-level and key-level affinity without affecting the session state for other requests. Documentation is updated to reflect the two-level affinity model (provider ordering and key selection) and the UI gains a `session-affinity` routing engine entry with its own label, color, and icon. ## Changes - Added `BifrostContextKeySessionAffinity` (`x-bf-session-affinity`) as a new `bool` context key; `on` (default) preserves existing behavior, `off` skips affinity for that request and writes no session state. - Clarified session stickiness docs to describe the two-level model: provider affinity (tried first when routing still offers it) and key affinity (used while the key remains eligible), with bindings written only on success and refreshed by every served request. - Updated session TTL description from "session-to-key binding" to "session bindings" to reflect that bindings now cover both provider and key. - Added `session-affinity` as a recognized routing engine in the UI, with an amber color scheme and a `Link2` icon, so it appears correctly in routing logs and engine-used badges. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [x] Docs ## How to test ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` To validate the affinity switch: 1. Make a request with a session ID and confirm the session binds to a provider and key. 2. Make a subsequent request with the same session ID and `x-bf-session-affinity: off`; confirm routing logs do not include the `session-affinity` engine and the bound provider/key is not preferred. 3. Make a follow-up request with `x-bf-session-affinity: on` (or omit the header); confirm the binding from step 1 is still in effect (not overwritten by step 2). ## Screenshots/Recordings The `session-affinity` engine badge now appears in routing logs with an amber background and a chain-link icon, consistent with the other engine badges. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No new secrets or PII surfaces. The affinity switch is a per-request routing hint with no elevated privilege; disabling it for a request simply falls back to standard routing and key selection. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…lpers; unify complexity session keying under shared session state (maximhq#7179) ## Summary Several internal session affinity helpers were private to the `core` package, preventing external components (such as plugins) from building on the same session-scoping and key-derivation logic. This PR promotes those helpers to exported symbols and migrates the complexity routing plugin to use them, eliminating a duplicate, divergent implementation of session key scoping. ## Changes - `newSessionAffinity`, `sessionStateKey`, `sessionStateString`, `sessionTTLFromContext`, and `routeString` are now exported as `NewSessionAffinity`, `SessionStateKey`, `SessionStateString`, `SessionTTLFromContext`, and `RouteStateValue`. - `ParseRouteState` is introduced as the exported inverse of `RouteStateValue`, replacing an inline `strings.Cut` call and adding a validity check on the parsed provider. - The complexity routing plugin's `buildComplexitySessionKey` (which maintained its own SHA-256 scoping logic, virtual-key/user scope classification, and key prefix) is removed. `complexitySessionKey` now delegates directly to `bifrost.SessionStateKey` with a `"complexity"` kind, inheriting the canonical scoping and hashing from core. - The `virtualKeyID` parameter is removed from `computeComplexity` since the key is now derived entirely from the context. - Tests are updated throughout to call the exported names and to reflect the new key prefix (`session:v2:complexity:` instead of `complexity-session:v1:`). ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./plugins/routing/... ``` Verify that session stickiness tests pass end-to-end and that complexity routing session keys are scoped correctly per virtual key, user, and session ID. ## Breaking changes - [x] Yes - [ ] No The complexity session store key format changes from `complexity-session:v1:<hash>` to `session:v2:complexity:<hash>`. Any in-flight session state written under the old prefix will not be found after this change, causing affected sessions to start a new complexity epoch. No code migration is required; sessions will re-establish naturally on the next request. Any code outside this repository that called the previously unexported helpers by copying them will need to be updated to use the exported versions. ## Related issues ## Security considerations Session keys continue to hash all caller-supplied identifiers (virtual key ID, user ID, session ID) with SHA-256 before writing to the KV store, so no PII or caller-controlled text appears in stored keys. The complexity plugin now inherits this guarantee from core rather than maintaining a parallel implementation. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…agent, and request-options docs (maximhq#7210) ## Summary Adds a dedicated Session Affinity documentation page and cross-links it from every relevant location in the docs. Previously, session stickiness behavior was only described inline within Request Options with no standalone reference, making it hard to discover and understand how it composes with routing engines, Adaptive Load Balancing, and the circuit breaker. ## Changes - Added `docs/providers/session-affinity.mdx`: a full reference page covering the two-level affinity model (provider and key), binding lifecycle, how affinity fits into the routing order, health-aware invalidation (Enterprise), controls, observability trail entries, walk-throughs for common scenarios, and known limits. - Registered the new page in `docs.json` navigation under the Providers section. - Added `<Tip>` callouts to the Claude Code, Codex CLI, and OpenCode pages noting that each harness already sends a session header, so affinity works with no configuration, with a link to the new page. - Updated the Adaptive Load Balancing page to clarify that the system is not session-aware and that Session Affinity runs after it, linking to the new page. - Updated the Complexity Router page to note that affinity and the router are independent but use the same session identity and run alongside each other. - Updated the Provider Routing page to explain where in the routing order affinity sits and what it changes versus what routing decides. - Updated the Request Options page to point to the new guide for the full binding lifecycle and composition details. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test Review the rendered docs to confirm: - `providers/session-affinity` resolves and appears in the sidebar under Providers. - The `<Tip>` blocks appear on the Claude Code, Codex CLI, and OpenCode pages with working links. - Cross-links from Adaptive Load Balancing, Complexity Router, Provider Routing, and Request Options all resolve to the new page. - The walk-throughs, lifecycle table, and controls table render correctly. ## Breaking changes - [x] No ## Related issues ## Security considerations None. This is documentation only. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…nes-used, log unweighted provider exclusions, clarify docs (maximhq#7289) ## Summary Session affinity now records its decision in the routing trail and engines-used list even when the bound provider already matches what routing proposed, making it possible to distinguish "session agreed with routing" from "session was never consulted." This PR also improves the governance plugin's routing log to report the weighted candidate pool rather than the broader eligible pool, and logs providers excluded for having no weight assigned. ## Changes - `sessionAffinity.ResolveRoute` now appends `RoutingEngineSessionAffinity` to the engines-used list and writes a log entry (`Session stays on <provider> for <model>, which routing also proposed`) before the early return when the bound provider is already first in the chain, matching the behaviour of the reorder path. - The test for "bound provider already first" is updated to assert that the engine is recorded and the trail contains the agreement message, rather than asserting the opposite. - `GovernancePlugin.LoadBalanceProvider` now logs each provider excluded for having no weight assigned, for the same reason every other exclusion is named: the counts in the trail are otherwise impossible to reconcile. - The governance selection log now reports the weighted pool (`from %d weighted`) instead of the eligible pool (`from %d eligible`), because the roll ran over the weighted set and the eligible set includes candidates the selection could never have landed on. - Documentation updated to note that the UI labels the `session-affinity` engine **Session**, to add the new trail entry for agreement with routing, to clarify that the circuit-breaker and load-balancer entries are Enterprise-only, to document that governance refusals before routing leave the session binding untouched, and to correct the description of what the provider level records. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [x] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [x] Docs ## How to test ```sh go test ./core/... ./plugins/governance/... ``` Verify that a request whose session is already bound to the provider routing proposes first now shows `session-affinity` in the engines-used list and a `which routing also proposed` entry in the routing trail. Verify that a provider with no weight assigned produces an exclusion log entry in the governance trail. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
OpenAI Responses web search can return specialized API sources
({"type":"api","name":"oai-weather"}) that carry a name and no URL. The typed
source schema only modeled type and a required url, so decode dropped name and
re-encode fabricated an empty "url":""; the OpenAI request-side source
sanitization rebuilt sources without name as well. url is now omitempty and
name survives both the schema round-trip and the sanitization path.
Affected packages:
- core/schemas/responses.go - add Name to web search action sources, make url omitempty
- core/providers/openai/types.go - keep Name in the OpenAI source sanitization path
- core/schemas/responses_test.go - round-trip regression tests
- core/providers/openai/responses_test.go - sanitization regression test
- core/changelog.md - changelog entry
- tests/e2e/api/collections/provider-harness.json - replay regression case
Closes maximhq#7356
…request (maximhq#7348) * fix(anthropic): carry allowed_tools and parallel_tool_calls into the request Two tool restrictions a caller sets were discarded on the way to Anthropic. Both requests still succeeded, so the caller had no way to learn the restriction was not in force. **allowed_tools became "call anything"** `chat.go` mapped the whole subset to a single word: case schemas.ChatToolChoiceTypeAllowedTools: toolChoice.Type = "any" A caller saying "you may call only tool_a" had every declared tool forwarded and the upstream told to call any of them. Mode was lost too, so an "auto" subset was turned into a mandatory call. Anthropic's tool_choice has no subset form (auto, any, tool, none), so the restriction is expressed where it can be: a tool that is not declared cannot be called. The declarations are now filtered to the allowed names, and the mode maps to auto or any rather than always any. An allowed list naming tools that are not in the request keeps the original declarations. Sending none would convert a restriction into a request that cannot call anything, which is a larger change than the caller asked for. **parallel_tool_calls had nowhere to go** `AnthropicToolChoice.DisableParallelToolUse` has existed in types.go all along; nothing ever set it. `ParallelToolCalls` appears nowhere in this provider, so the instruction was dropped and calls stayed parallel. It is now carried across, with the sense inverted. Anthropic accepts the field on tool_choice only, so when a caller disables parallel calls without expressing any preference about which tool runs, an explicit "auto" is emitted to hold it. That is Anthropic's own default, so it adds no restriction that was not already there. Reported in maximhq#7344, cases 3 and 6. The Gemini cases in that issue are a separate provider and are not touched here. Six tests, all failing against the previous revision: the subset is enforced, auto mode does not force a call, unknown names keep the declarations, the parallel flag is carried both with and without an existing tool choice, and parallel_tool_calls: true still sends nothing because it is the default. go build, go vet and the full anthropic package pass. The package was green before this change as well, so the delta is only the new cases. * fix(anthropic): an allowed list matching nothing permits nothing Raised in review by CodeRabbit, and it is right. An allowed list naming only tools the request does not declare left an empty intersection, and the filter returned the original declarations. So a caller who restricted the model to a tool that is not present had every tool they excluded forwarded instead - the same failure this conversion exists to stop, reached from the other direction. My reasoning was that a request which cannot call anything is a larger change than the caller asked for. That was wrong: the caller asked for exactly one set, the intersection of that set with the declarations is empty, and "nothing" is the faithful answer rather than "everything". The intersection is now honoured literally, and the tool choice says so. "any" or "auto" with no declarations is a request Anthropic rejects, so an empty result maps to "none". TestToAnthropicChatRequest_AllowedToolsUnknownNamesKeepDeclarations asserted the old behaviour and has been replaced by TestToAnthropicChatRequest_AllowedToolsUnknownNamesPermitNothing, which checks both halves: no tool is forwarded, and the choice is "none". * fix(anthropic): an allowed_tools filter must not strip server tools Found while re-reading my own change. The filter compared every declaration against the allowed names, and anthropicReq.Tools can hold server tools - MCP toolsets, web search, computer use - built by convertServerToolToAnthropic a few lines above. An OpenAI allowed_tools list describes function tools: entries are {type: "function", function: {name}}. A server tool can never appear in one, so matching it on a name it cannot have removed it every time a caller set allowed_tools. Restricting function calling to tool_a silently switched off a web search the caller had enabled elsewhere in the same request. That is the same harm as the bug this PR fixes, inverted: a stated intent quietly discarded. Server tools now pass through the filter untouched; only nameable declarations are subject to the allowed list. The empty-intersection case follows. "None of the allowed tools are declared" is a statement about nameable tools, so the tool_choice now becomes "none" when no function tool survives, rather than when the list is empty - otherwise a request carrying a server tool would keep "any" over nothing the model could be told to call. Two tests, both failing against the previous revision: a server tool survives an allowed_tools filter that excludes every function tool, and a filter that allows nothing nameable still yields "none" while leaving the server tool in place. * test(anthropic): move the tool-choice tests into chat_test.go Requested in review by @akshaydeo: no new test files. The seven cases move into providers/anthropic/chat_test.go, which already holds the TestToAnthropicChatRequest_* tests they belong with, and toolchoicerestrictions_test.go is removed. testify is added to that file's imports; it is already used elsewhere in the package. No test content changed. go vet and the full anthropic package pass. * fix(anthropic): an MCP toolset is a server tool too Raised in review by CodeRabbit, and it is right. My previous commit used Type != nil to recognise a server tool, which is true of web search, web fetch and the computer-use family but not of an MCP toolset. convertServerToolToAnthropic returns one as: return AnthropicTool{MCPToolset: toolset}, true with Type nil and Name empty. The struct says so itself: // MCP toolset (mcp-client-2025-11-20 format) — embedded when Type is nil // and MCPToolset is set MCPToolset *AnthropicMCPToolsetTool `json:"-"` So the filter matched an MCP toolset's empty name against the allowlist and removed it - the same silent capability loss the previous commit set out to fix, still happening for the one server-tool shape I did not check. I had read Type: "mcp_toolset" a few lines up and not noticed it was on the nested type rather than on AnthropicTool. Both shapes are now preserved, and countFunctionTools excludes both, so a request whose nameable tools are all excluded still yields "none" rather than "any" over nothing callable. The test fixture carries both shapes rather than one, which is what let this through: the previous test used Type to stand in for a server tool, so it could not have caught an MCP toolset. Both cases fail against the previous revision.
…maximhq#7360) The Anthropic Messages API accepts a message's content either as an array of content blocks or as a plain string, and clients mix the two: a rolling cache breakpoint produces block arrays for the turns carrying cache_control and bare strings for the rest. BedrockMessage.Content is typed []BedrockContentBlock, so one bare string anywhere in messages[] fails the standard unmarshal and diverts the whole request into the AI21 fallback. That branch rebuilt the messages but never called applyMessageContentCacheControl, so every cache_control in the request was dropped, not only the one on the string message. Prompt caching went off silently, with only the system cachePoint surviving on its separate path. The fallback now makes the same translation the standard path does. Fixes maximhq#7336
## Summary Extracts repeated label-value splicing logic in the telemetry plugin into a shared `spliceLabelValues` helper, and fixes an over-allocated map in the governance plugin. ## Changes - Introduced `spliceLabelValues(promLabelValues []string, nDefault int, extras ...string) []string` in `plugins/telemetry/utils.go`. This helper inserts metric-specific extra label values (e.g. `is_success`, `status_code`+`error_type`, `cache_type`) between the default labels and custom labels without mutating the input slice, replacing three identical inline `make`/`append` patterns in `PostLLMHook`. - Added `TestSpliceLabelValues` to pin the exact label-value ordering for latency, error, and cache-hit metrics. Because Prometheus metric vectors match values to label names purely by position, any reordering silently attributes values to the wrong labels; the test guards against this. - Fixed `alreadyCharged` map initialization in `reportBatchModelUsage` (governance plugin): the capacity hint `len(usage.BudgetIDs)+len(usage.RateLimitIDs)` was incorrect because `RateLimitIDs` entries are stored under a `"ratelimit:"` prefix and the map is populated in a separate loop, so the hint was misleading. The map is now initialized without a capacity hint. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./plugins/telemetry/... ./plugins/governance/... ``` The new `TestSpliceLabelValues` test verifies: - Extra values are inserted at the correct position between default and custom labels. - Multiple extras preserve their relative order. - The original input slice is never mutated. - Edge cases where there are no custom labels or no extras behave correctly. ## Screenshots/Recordings N/A ## Breaking changes - [x] No ## Related issues N/A ## Security considerations None. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary A permit that grants every provider names no provider permits for those providers, so the methods that return a list of providers had nothing to enumerate and would return only the providers that happened to have explicit overrides. This made the listing and routing layers refuse providers that the request path correctly admitted. This PR completes those lists from the deployment's configured provider set when an allow-all permit is in effect. ## Changes - Added a `ProviderLister` function type and `WithConfiguredProviders` option to `Access`, so a resolver can supply the deployment's provider set without affecting resolvers that have nothing to give. - `ProvidersForModel` and `GrantedProvidersForModel` now call `providersGrantedByAllowAll`, which enumerates configured providers not already named by any provider permit and checks each through `IsModelAllowed` — the same gate the request path uses — so the two cannot drift. - A provider that a provider permit explicitly refused is tracked in a `named` set and is not reopened by the allow-all path. - An intersecting scope is not widened: the allow-all completion still passes through `IsModelAllowed`, which applies composition, so a project-scoped intersection that permits only one provider continues to permit only that one. - `GovernancePlugin.ResolveAccess` now passes `configuredProviders()` to `NewAccess`. The lister reads the in-memory store at call time rather than capturing a snapshot, so a provider added after resolution is granted by the same rule. - The `listManagementModels` handler path in the HTTP transport is covered by a new test confirming that a caller permitted every provider sees models from providers it holds no explicit permit for. ## Type of change - [x] Bug fix ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Plugins ## How to test ```sh go test ./framework/grant/... ./transports/bifrost-http/handlers/... ./plugins/governance/... ``` The new `TestAllowAllProvidersCompletesTheProviderLists` suite in `framework/grant/access_test.go` covers the four cases: granted providers include one no permit names, candidates include that provider unweighted and unrestricted, a provider its own permit refused is not reopened, and an intersecting scope is not widened. `TestListModels_VKFilterListsProviderAllowedOnlyByAllowAll` covers the HTTP handler path end-to-end. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations The allow-all completion path gates every candidate through `IsModelAllowed`, which enforces blacklists and composition mode, so no provider or model is admitted beyond what the existing permission logic already permits. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Adds support for new typesafe models. We are also introducing new /v1/decisions endpoint for systemone class of models ## Changes - Adds new typesafe provider - Adds new /v1/decisions endpoint along with BifrostDecisionRequest and BifrostDecisionResponse - Adds new /typesafe integration ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [x] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [x] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [x] No If yes, describe impact and migration instructions. ## Related issues NA ## Security considerations NA ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…ers (maximhq#7361) ## Summary Providers that lack native decision support previously returned an `unsupported_operation` error. This PR introduces a transparent emulation layer that intercepts that error and re-answers the decision request through the same provider's chat model using forced tool-calling (or native structured output where available). This covers both the case where an LLM is named directly as the decision model and the case where an LLM is reached as a fallback after a native provider fails. ## Changes - When `handleProviderRequest` receives an `unsupported_operation` error from `provider.Decision`, it now calls `emulateDecisionViaChat` before propagating the error, allowing any tool-capable chat model to fulfill the request transparently. - `core/decisionemulation.go` implements the emulation: it builds a forced `emit_decision` function tool from the question set, calls `provider.ChatCompletion`, extracts the tool-call arguments (with a fallback to native structured-output content), and maps the result back to `BifrostDecisionResponse`. - `core/providers/utils/decisionemulation.go` provides the reusable schema and parsing layer: `BuildDecisionTool` encodes all question kinds (`noul`, `choice`, `score`) as a single JSON-schema function tool; `ParseDecisionAnswers` decodes and validates the tool-call arguments against each question's kind, rejecting out-of-range, wrong-typed, or missing answers. - `DecisionEmulation` test scenario and `DecisionEmulationModel` config field added to the integration test harness; `RunDecisionEmulationTest` exercises the emulation path end-to-end. - OpenAI provider test config opts into `DecisionEmulation` using `gpt-4o-mini`. - E2E Postman collection extended with three new cases: LLM as primary decision model, LLM as fallback after a failing primary, and the `/typesafe/v1/systemone` native-shape emulation path. - Python and TypeScript integration tests added under `TestDecisionLLMEmulation` / `Decision LLM emulation`, covering primary emulation, fallback, and a side-by-side comparison against the native `jev` judgment model. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh # Unit tests for the schema builder and answer parser go test ./core/providers/utils/... # Full provider integration test (requires OPENAI_API_KEY) go test ./core/providers/openai/... -run TestOpenAI/DecisionEmulation -v # Python integration tests (requires a running Bifrost instance) cd tests/integrations/python pytest tests/test_typesafe.py::TestDecisionLLMEmulation -v # TypeScript integration tests cd tests/integrations/typescript npx jest tests/test-typesafe.test.ts --testNamePattern="Decision LLM emulation" # E2E Postman collection # Import tests/e2e/api/collections/provider-harness.json and run folder # "106. Decision LLM emulation and fallback" ``` Set `DecisionEmulationModel` in the provider test config to a tool-capable chat model (e.g. `gpt-4o-mini`) and enable `Scenarios.DecisionEmulation: true`. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The emulation path forwards the decision `state` payload as a chat message to the configured LLM provider. No new secrets or credentials are introduced; the same provider key used for chat completions is reused. Callers should be aware that state content is sent to the chat endpoint rather than a dedicated judgment endpoint. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Tightens the decision emulation pipeline so that probability distributions are always exactly normalized before being returned to callers, enforces that a chosen option must have the highest probability in its distribution, and fixes a double-gutter rendering bug in the Monaco code editor. ## Changes - Renamed `validateProbabilities` to `normalizeProbabilities`, which now returns a rescaled copy of the distribution whenever the raw sum is within the 5% tolerance but not already exact. This ensures callers always receive probabilities that sum to exactly 1, and that score values derived from the weighted sum are consistent with the normalized distribution. - Added a post-normalization check in `ParseDecisionAnswers` that rejects any choice answer whose selected option does not have the highest (or tied-highest) probability in the distribution. - Updated the decision system prompt and JSON schema descriptions to instruct the model that probabilities must sum to 1 (not "about 1") and that the choice field must select the option with the highest probability. - Disabled `glyphMargin` unconditionally in the Monaco editor and reduced `lineDecorationsWidth` from 18 to 2 when folding is enabled. The glyph margin is a separate breakpoint column that was never used; enabling it alongside folding was doubling the left gutter width. Folding arrows render in the `lineDecorations` column, which Monaco already widens automatically. ## Type of change - [x] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` Verify that: - A choice answer whose selected option does not have the highest probability is rejected with an error containing "highest probability". - A near-one distribution (e.g. summing to 0.99 or 1.01) is accepted and the returned probabilities sum to exactly 1. - A distribution whose sum deviates by more than 5% is still rejected. - The Monaco editor no longer shows a doubled left gutter when folding is enabled. ## Screenshots/Recordings Before: Monaco editor with folding enabled rendered a wide double gutter on the left side. After: Gutter is compact; folding arrows appear in the line decorations column as expected. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. No auth, secrets, PII, or sandboxing changes. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
Upstream maximhq#7332 replaced marshalling each held chunk with a field-sum estimate, which skipped safeguard results, search results, videos, citations, annotations and the error fields. An event carried in one of them was charged only the 512-byte floor, so a paused stream could hold far more than the 100 MB cap. They now count. Codex connector P1 on PR #23. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The string form drops an unknown provider prefix while decoding, so the validation caught it, but the object form added with pinned fallback keys (upstream maximhq#7380) named its provider directly and only had to be non-empty. A typo was stored and failed only when the fallback was needed. Both forms now check the provider against the known set. Codex connector P2 on PR #23. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…he proxy Upstream maximhq#7437 restores HTTP_PROXY/HTTPS_PROXY for MCP and checks only IP-literal destinations on the proxied path, handing a hostname to the proxy unresolved. A name that resolves, at the proxy, to a metadata or other blocked address therefore bypassed the guard; fork v2.2.1 never proxied MCP at all. A proxied hostname must now resolve on the gateway with every address passing the private-network policy, and a name the gateway cannot resolve is refused. The upstream test that pinned the opposite contract is rewritten to this one; docs and changelog say so. Codex security review P2 on PR #23; residual proxy-side re-resolution stays with TXE-5088. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream maximhq#7375's api-management row "/v1/models - fan-out reaches the unnamed provider" needs anthropic, granted to the VK by allow_all_providers alone, to list models. The fork's release job has no provider secrets, so anthropic's real endpoint failed and the row failed: one assertion of 758, which would block the release. The list-models stub now also serves anthropic's list in Anthropic's shape (list-models only; no chat route), and the suite passes 758/758. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3cf5f08c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… affinity f3775b7 held the route for a direct key; an explicit x-bf-api-key name or x-bf-api-key-id is the same kind of choice, naming a key of the request's own provider, and a session binding could still move the primary to another provider with the selector left on the context. Such a request now keeps its route too. A routing-rule pin is not the caller's and still moves with its route. Codex connector P1 on PR #23 (round 2). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
93d32f1 prices a decision without a decisions row from the model's text row, but overrides were still looked up for the decision request type, so an override written for the Responses call emulation makes never applied. A decision priced from a text row now takes a Responses override when no decision-specific override matches. Codex connector P1 on PR #23 (round 2). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f619b351b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rved on Emulation copied the Responses call's usage onto the decision but not its service tier, speed or inference geography, so a decision served at priority, fast or US-resident rates was priced at the base rate. The decision response now carries all three, cost extraction reads them, and the billed usage on a rejected answer keeps speed and geography as the bare-usage path allows. Codex connector P1 on PR #23 (round 3); PR #5 round-3 M13. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With send_back_raw_response on, the systemone route returned the provider body kept before post-hooks even when a post-hook had changed an answer, so the change never reached the client. The raw body is now relayed only while its answers match the processed response; otherwise the processed response is rendered. Codex connector P2 on PR #23 (round 3); PR #5 R2-M105. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12fabd56c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The native Typesafe converter accepted any returned choice, so a malformed response surfaced an impossible answer as a successful decision; emulation already refuses one. The converter now checks the choice against the question's options, and the rejection keeps the billed usage. Codex connector P2 on PR #23 (round 4). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 519f922383
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A rejected emulated decision is billed from BilledUsage, and the bare-usage pricing path read only speed and inference geography, so a priority or flex call was priced at the base rate. BifrostLLMUsage gains an internal ServiceTier (json:"-", deep-copied like the other two), the emulation error path sets it, and CalculateCostForUsage applies it. Codex connector P2 on PR #23 (round 5). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faa1255392
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A native score below zero or above the last level index was accepted as a successful decision, though it cannot be a score over that rubric; emulation derives a bounded one. The converter now checks a score against 0..levels-1, keeping the billed usage on rejection. Codex connector P2 on PR #23 (round 6). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream's computer-toolset support (maximhq#7434, maximhq#7441) adds ToolsetName to ResponsesToolMessage, but neither Responses deep-copy helper copied it, so the stream accumulator's snapshots and the schema copy dropped the toolset identity. Both now copy it. Codex connector P2 on PR #23 (round 6). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3463b4b217
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 3463b4b217
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
The merge's memory clause on the image and release jobs accepted a skipped test-memory only under skip-tests. test-memory runs only for a core or bifrost-http release, so an image-only republish (a missing image destination) skipped it and every Docker job with it: the image could never be republished. A skip now also passes when neither component the gate covers is being released. test-release-gates.py gains a MemoryGate class; dropping the new branch fails its image-only cases. Codex connector P2 on PR #23 (round 7). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d7a61ea49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… tool Upstream maximhq#7348 carries tool_choice.allowed_tools to Anthropic by filtering the tool declarations, but left an explicit empty list alone, so every declared function stayed callable although the caller allowed none. An empty list (or one naming no function) now removes every function declaration; server tools stay, and the existing no-function branch sends tool_choice none. Codex connector P1 on PR #23 (round 8). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…g repricing ServedModel() did not read decision responses, so an emulated decision served by a router model was priced and logged without its served model; and the logging plugin's cost recalculation rebuilt a decision without the served tier its log row stores, repricing it at the standard rate. Both now carry them. Codex connector P2s on PR #23 (round 8). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afa10d3a24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tion An allowed_tools list that permits no function keeps the declared server tools (web search, web fetch, computer use, MCP toolsets), since the list restricts functions only, but the conversion then sent tool_choice none, which Anthropic applies to every tool. It now sends none only when no tool is left, and auto when server tools remain. Codex connector P2 on PR #23 (round 9), following up the round-8 fix. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99ee1285f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
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.
Fourth upstream batch (epic TXE-4803). This PR merges upstream
transports/v2.2.3(maximhq/bifrost411d62b28b03b03bd3b4025b2cfab50af45f05f4) onto fork main120793b75(v2.2.1). The result releases as transport v2.2.3.git merge(64acf1e8b, parents120793b75+411d62b28) with no squash and no-s ours, so411d62b28becomes an ancestor of main. The merge base is6493abd3d, the upstream v2.2.1 that PR TXE-5038: Merge upstream transports/v2.2.1 as transport v2.2.1 #22 merged.Conflict-resolution ledger (59 files)
go.modof framework, the 12 plugins, transports, hello-world andtests/cmd/{seed,seedvks}github.com/capsohq/bifrost/...paths: core v1.10.2, framework v1.7.4, plugins as bumped.tests/cmdis tidied: seed and seedvks to core v1.10.2, and e2eseed keeps the fork's direct seed require.core/changelog.md, framework and the 12 plugin changelogstransports/changelog.md.github/workflows/release-pipeline.yml(6 hunks)test-memoryand its break-glass jobapprove-memory-regression, and gates core, bifrost-http and the four Docker jobs on them. The new jobs use the fork's runner labels. The sixif:lines keep the fork's stricter rule that a skipped test counts only underskip-tests, and gain the memory clause in that same form.core/bifrost.gotypesafe.core/schemas/utils.goDeepCopyResponsesMessageblocks, a superset of the fork's: the ImageGenerationCall copy and the bare-string action copy are both upstream's now.transports/bifrost-http/server/server_test.godocs/providers/supported-providers/overview.mdxtests/e2e/features/routing-rules/pages/routing-rules.page.tsui/lib/constants/icons.tsxand 17 import-only Go conflictsMerge adaptations needed for the build, inside the merge commit:
github.com/maximhq/bifrost/...Go import is rewritten to capsohq; issue URLs are left alone.go.workgains replaces for the new module versions.Decisionis new on theProviderinterface (upstream typesafe models support, new decisions endpoint maximhq/bifrost#7355). The fork-only GLM, Qwen, MiniMax, Moonshot and Volcengine/ModelArk providers return unsupported, as upstream's own non-Typesafe providers do. Core then answers by emulation (feat(decision): emulate decisions via LLM tool-calling for all providers maximhq/bifrost#7361).plugins/telemetry/main_test.go: the fork'suser_labels_enabledomission check declares its ownout, errafter upstream's reflection rewrite of the test.integrations/router_test.go: duplicate imports from both sides removed.Outside the conflicts, I diffed every file where main differed from upstream v2.2.1. Each still differs from upstream v2.2.3, except
core/providers/openai/types_test.go(one blank line) andcore/schemas/utils.go(both fork additions are now upstream's).Commits on top of the merge
ac82b3810xAI's native Messages route strips Anthropic-only Claude Code fields.safeguards(adds safeguard support for passthrough requests maximhq/bifrost#7393),diagnostics(fix: strip diagnostics if unsupported maximhq/bifrost#7243) and per-requestx-anthropic-billing-headersystem line (readact anthropic billing headers to avoid cache spoiling maximhq/bifrost#7431) for every non-Anthropic provider, but only on its converted path.anthropicmsgclient, so all three still reached xAI. The billing line changes on every request, so it also defeated xAI's prompt cache.anthropicmsg.Config.StripAnthropicOnlyFields, set by xAI and not by Anthropic, on the buffered and streaming paths. A body with nothing to strip is forwarded byte for byte.93d32f160Emulated decisions are billed.decisions-mode row, which only Typesafe has. So every emulated decision (OpenAI, Anthropic, Bedrock, and so on) found no row and cost $0: a paid provider call outside every budget.decisionsrows keep token-only pricing.d2f174929(R2 H7/H8). The test failed before the fix; dropping the text-cost branch fails it again.48b1eda18Rejected decision answers are charged.BilledUsageand the served model, which governance and logging already charge.autotool_choice from the base provider, so it applies to custom providers wrapping Mantle.6cc018841(R2-M24/M26) and13deec109(R2-M11, Mantle part), extended to emulation. Each mutation fails a test.3b2967c7fA paused stream's overflow releases no unapproved chunks.0e64dc5c2(R2 H9) with its test; the mutation fails it.e0a3b8867Egress gate. Upstream's harness row 12.B17 (test fixes maximhq/bifrost#7440) sends Anthropicmcp_serverswithhttps://mcp.deepwiki.com/mcp, which Anthropic's MCP connector dials server-side. The host joinsNOT_DIALLED_HOSTS, andcheck-egress-allowlist.shpasses again.95d6f4934Fork section of the transport changelog.b3cf5f08cOffline API suite stubs anthropic's list-models.Codex connector round 1 (head
95d6f4934): 2 P1 and 2 P2, all fixed, with replies and resolved threadsf3775b7f7(P1) Session affinity and direct keys. A direct key stays on the context for the primary attempt. Session route affinity (feat: route and key session affinity driven by observed outcomes, withx-bf-session-affinityheader opt-out maximhq/bifrost#7177) could move the primary to another provider and send the caller's credential there. A direct-key request now keeps its route. The mutation fails the regression test.5a37d355f(P1) Replay cap. stream gate memory fixes maximhq/bifrost#7332's field-sum estimate skipped safeguard results, search results, videos, citations, annotations and the error fields, so an event in one of them was charged 512 bytes. They now count. The mutation fails the test.8b94aa1ca(P2) Object-form routing fallbacks now check the provider against the same known set as the string form.0a3ef2e9c(P2, security) Proxied MCP hostnames.Codex connector round 2 (head
b3cf5f08c): 2 P1 fixed, 2 P2 tracked; all threads replied and resolvedb0c800d94(P1) An explicitx-bf-api-keyname, or anx-bf-api-key-idthat is not the routing-rule pin, now holds the session route as a direct key does. A routing-rule pin still moves with its route.0f619b351(P1) A decision priced from a text row takes a Responses pricing override (the emulated call) when no decision-specific override matches.Codex connector round 3 (head
0f619b351): 1 P1 and 1 P2 fixed, 1 P2 tracked; threads replied and resolvede0afc6b26(P1) Emulated decisions carry the served service tier, speed and inference geography, and are priced on them. This was PR TXE-4803: Merge upstream Bifrost through v2.3.2 #5 R3-M13.12fabd56c(P2) The Typesafe raw body is relayed only while its answers match the processed response. This was PR TXE-4803: Merge upstream Bifrost through v2.3.2 #5 R2-M105.Codex connector round 4 (head
12fabd56c): 1 P2 fixed519f92238(P2) The native Typesafe converter rejects a choice outside the question's options, as emulation already did, and the rejection keeps the billed usage.Codex connector round 5 (head
519f92238): 1 P2 fixedfaa125539(P2) Bare billed usage keeps the served service tier: an internal, deep-copiedBifrostLLMUsage.ServiceTier, applied byCalculateCostForUsage. A rejected emulated decision served at priority or flex is billed at that rate.Codex connector round 6 (head
faa125539,codex-reviewpassed): 2 P2 fixede8bea148c(P2) The native Typesafe converter rejects a score outside the rubric's 0..levels-1.3463b4b21(P2) Both Responses deep-copy helpers keep a toolset member call'sToolsetName(fix: computer toolset compatibility maximhq/bifrost#7434/fix: computer toolset model capability maximhq/bifrost#7441).Codex connector round 7 (head
3463b4b21,codex-reviewpassed): 1 P2 fixed, 3 P2 tracked5d7a61ea4(P2) This merge's memory clause stranded an image-only republish, becausetest-memoryskips when no core or bifrost-http release is due. A skip now passes in exactly that case.test-release-gates.pygains aMemoryGateclass whose image-only cases fail without the fix.mcp_approval_responserename not reflected in OpenAPI or the logs UI: upstream. Filed as TXE-5098.0a3ef2e9cleaves. Recorded on TXE-5088.Codex connector round 8 (head
5d7a61ea4; review stalled, then completed after one@codex review): 1 P1 and 2 P2 fixedba609ed32(P1) An explicit emptytool_choice.allowed_toolslist now permits no function tool on Anthropic. Upstream fix(anthropic): carry allowed_tools and parallel_tool_calls into the request maximhq/bifrost#7348 left every declaration callable. The regression test fails with the old early return restored.afa10d3a2(P2 ×2)ServedModel()reads decision responses, and the logging plugin's cost recalculation restores a decision's served tier.Codex connector round 9 (head
afa10d3a2; review stalled, then completed after one@codex review): 1 P2 fixed, 1 P2 tracked99ee1285f(P2) My round-8 fix senttool_choice: noneeven when server tools remained. It now sendsnoneonly when no tool is left, andautowhen server tools remain.Behaviour changes to note
HTTP_PROXY/HTTPS_PROXYsupport for MCP connections while blocking link-local targets via pre-proxy destination guard maximhq/bifrost#7437). MCP HTTP and SSE connections honourHTTP_PROXY/HTTPS_PROXY/NO_PROXYagain; fork v2.2.1 never proxied them. Through a proxy, an IP-literal link-local, unspecified or metadata destination is refused before the proxy is contacted. Unlike upstream, a hostname must resolve on the gateway to permitted addresses, so a deployment whose DNS lives only behind the proxy must let the gateway resolve its MCP servers. This applies only where the gateway process has proxy variables set. The residual gap is TXE-5088.approve-memory-regressionuses the environmentmemory-regression-override, which does not exist yet.flaky-test-overrideexists, but with no protection rules. A job targeting an environment without required reviewers does not pause, so both gates auto-approve on failure. The flaky gate already behaved this way. This PR leaves repository settings alone; filed as TXE-5089.PR #5 applicability (frozen reference
90bc81550, not modified)48 of PR #5's 109 fix commits (
309dd79a7..90bc81550) touch a file this batch changes. Each was test-applied to this head; where it did not apply, I checked by hand whether its code exists at v2.2.3.d2f174929(R2 H7/H8) emulated-decision pricing93d32f160)6cc018841(R2-M24/M26) charge rejected answers48b1eda18)13deec109(R2-M11) base-provider tool_choice48b1eda18)0e64dc5c2(R2 H9) overflow releases unapproved chunks3b2967c7f)9f2ffe48b(R2-M57) shell/patch carriers in the replay capf4f185795(R2-M105) Typesafe raw body vs post-hook answers12fabd56c(connector round 3)b67778c7b,f6f162790,9bbf012ae(R2 H5) proxied MCP destinations2d30737ce,8aa52278d(R2 H12) method overrides, passthrough tuningf109475b0(R2-M13) SSE MCP verification9f0721436(R2-M1) memory gate on framework/plugin releasesec79c6a73,f1c18f816,f9ea76180,e9ef4da27,433b5ca03,9c78936ba,ba3141e96,8fccf9054,f026a5ce7,68800f0e2,ff8186c2fe0afc6b26(connector round 3)tests/cmd/seedusemoduleFork behaviour checked after the merge
anthropicmsg, with usage extraction. readact anthropic billing headers to avoid cache spoiling maximhq/bifrost#7431's billing-header restore runs inprepareResponsesRequest, after the fork's native passthrough has already taken the request, as upstream intends for raw passthrough.handleStreaming.ek_bf_boundary: upstream changed no realtime handler in this range. The legacy/GA adapters and provider/key-bound redemption are untouched, and their tests pass.release-bifrost-http-prep.sh 2.2.3reportsverified: transports/go.mod matches every module version file, with no changes.Tests (offline, every
*_API_KEY|_KEY|_TOKEN|_SECRETvariable unset,GOTOOLCHAIN=auto CGO_ENABLED=1 GOFLAGS=-trimpath=false)internal/mcptestsonce the example servers are built (make setup-mcp-tests).test-memoryset passes on core and transports, including the coverage scan over the fork's providers. It caught a loop in my first version ofac82b3810, which was unrolled.go test -race ./...passes all 28 packages in my owntests/docker-compose.ymlstack (projectbifrost-txe5086, subnet 172.56, no host ports, runner container).vectorstorehit one Qdrant upsert deadline under concurrent host load; that package is untouched in this batch and passes on its own rerun.go test -short -racepasses all 7 packages.test-migration-fixtures.py.test-plugin-release-dependencies.py, which no workflow runs, fails identically on main.validate-go-config-fields.sh, which no workflow runs, reports the same 6 MCP OAuth fields missing on main as here.npm ci,vite buildandtscpass; vitest passes 612/612; oxlint reports 0 errors.transports/Dockerfilebuilds for real (arm64)./healthis ok and the UI returns 200./api/versionreadsvunknownlocally because the release jobs passVERSIONas a build arg, which I did not.run-migration-tests.sh allpasses on Postgres and SQLite from v2.1.3 (built from330d65925), v2.2.0 (30e432651) and v2.2.1 (120793b75), the three baselines CI picks. It ran on my own Postgres (projectbifrost-txe5086-mig, 127.0.0.1:55439) behind a fail-closed docker shim. The first attempt hit a transient upstream outage ofgetbifrost.ai/datasheet/model-parameters(23:34–23:46 +08:00), and the rerun passed.test-api-integrations.shpasses all six collections against this code (758, 864, 37, 27, 133 and 62 assertions, 0 failed), with openai, mistral, groq and now anthropic on the offline stubs. It ran from a scratch copy that uses my Postgres, port 18283 and a 180 s startup wait. Beforeb3cf5f08c, exactly one row failed (the fix: allow all providers access and api models response maximhq/bifrost#7375 fan-out row).-race.Release plan
gh pr merge --merge --match-head-commitand a--skip-core-testsubject. That skips only the live-provider core suite; every offline suite stays required.120793b75is never rebuilt. Origin has notransports/v2.2.3tag and GHCR has nov2.2.3*tag; both are re-checked before merge.Follow-ups filed (Backlog, under TXE-4803)
mcp_approval_responserename is not reflected in OpenAPI or the logs UI.Closes TXE-5086
🤖 Generated with Claude Code