Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughTranscription responses now carry a provider-selected ResponseFormat and, for plain-text formats (text/srt/vtt), are propagated and returned as raw bytes through the router and converters instead of being JSON-marshaled; accompanying schema backfill and provider/converter/router updates implement this short‑circuit. Minor formatting edits applied. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Provider
participant Schema
participant Converter
Client->>Router: POST transcription request (includes params.response_format)
Router->>Provider: call provider.Transcription(...)
Provider-->>Router: returns BifrostTranscriptionResponse (may include provider raw body)
Router->>Schema: BackfillParams(req) — copy ResponseFormat into response
Router->>Converter: TranscriptionResponseConverter(response)
alt ResponseFormat is plain-text
Converter-->>Router: raw []byte(resp.Text)
Router->>Client: set headers, HTTP 200, write raw bytes
else Non-plain format (JSON)
Converter-->>Router: structured response
Router->>Client: write JSON-encoded response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
core/providers/openai/openai.go (1)
2521-2521: Follow up on the TODO in transcription response parsing.At Line 2521, the TODO leaves this path using bespoke raw-response parsing rather than the shared handler. I can draft a focused cleanup PR if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` at line 2521, Replace the bespoke transcription response parsing at the TODO with the shared handler: call HandleProviderResponse (the existing shared response handler) instead of custom raw parsing in the transcription response path, adapt the transcription-specific code to transform the provider output into the shape HandleProviderResponse expects (or add a small adapter function e.g., adaptTranscriptionToProviderResponse), and remove the inline parsing logic so all provider responses go through HandleProviderResponse for consistent error handling, logging, and struct normalization (update any local variables or return types in the surrounding transcription handling function to use the handler's output).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/openai/openai.go`:
- Around line 2455-2462: The code in isPlainTextTranscriptionFormat branch sets
providerResponseHeaders["Content-Type"], introducing mixed-case keys; change it
to use a lowercased header key ("content-type") when assigning the value from
resp.Header.ContentType(), ensuring providerResponseHeaders is created if nil
and that all provider response header keys are normalized to lowercase for
downstream filtering/lookup (reference symbols: isPlainTextTranscriptionFormat,
providerResponseHeaders, resp.Header.ContentType()).
In `@transports/bifrost-http/integrations/router.go`:
- Around line 1068-1074: The code reads ProviderResponseHeaders["Content-Type"]
with exact casing which can miss variants like "content-type"; change to a
case-insensitive lookup: scan
transcriptionResponse.ExtraFields.ProviderResponseHeaders for a key that equals
"Content-Type" case-insensitively (e.g. using strings.EqualFold or
http.CanonicalHeaderKey on keys) and assign its value to ct, then call
isPlainTextTranscriptionResponse(ct) and proceed with the existing plain-text
bypass; reference ProviderResponseHeaders, isPlainTextTranscriptionResponse, and
ExtractProviderResponseHeaders to locate and update the lookup logic.
---
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Line 2521: Replace the bespoke transcription response parsing at the TODO with
the shared handler: call HandleProviderResponse (the existing shared response
handler) instead of custom raw parsing in the transcription response path, adapt
the transcription-specific code to transform the provider output into the shape
HandleProviderResponse expects (or add a small adapter function e.g.,
adaptTranscriptionToProviderResponse), and remove the inline parsing logic so
all provider responses go through HandleProviderResponse for consistent error
handling, logging, and struct normalization (update any local variables or
return types in the surrounding transcription handling function to use the
handler's output).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d66050b0-19db-45af-9e75-1bcafd7cf476
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/providers/openai/utils.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/utils.go
7b404aa to
2970a18
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/openai/openai.go (1)
2458-2465:⚠️ Potential issue | 🟠 MajorNormalize provider response header key to lowercase.
This still writes mixed-case
"Content-Type"intoproviderResponseHeaders, which can break downstream lookups expecting normalized keys.🔧 Proposed fix
- providerResponseHeaders["Content-Type"] = string(resp.Header.ContentType()) + if contentType := strings.TrimSpace(string(resp.Header.ContentType())); contentType != "" { + providerResponseHeaders["content-type"] = contentType + }Based on learnings: provider response headers should be normalized to lowercase before forwarding/filtering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2458 - 2465, The code currently sets providerResponseHeaders["Content-Type"] which can break downstream lookups expecting lowercase keys; update the logic in the block that checks request.Params and schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) so it writes the header under the normalized lowercase key "content-type" (still ensuring providerResponseHeaders is initialized) and use resp.Header.ContentType() for the value; modify the assignment to providerResponseHeaders to use the lowercase key instead of "Content-Type".
🧹 Nitpick comments (1)
core/providers/openai/openai.go (1)
2524-2524: Resolve the TODO in the JSON transcription path.Line 2524 leaves an open gap in consistency (
HandleProviderResponseintegration). Please either implement it in this PR or track it explicitly before merge.I can draft the refactor diff for this path if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` at line 2524, The JSON transcription path left a TODO where HandleProviderResponse should be invoked; replace the TODO with a call to HandleProviderResponse (the existing function used elsewhere for normalizing provider outputs) passing the provider response object, the current context/metadata (e.g., transcription request/response structs), and the logger, then handle its returned structured result and error—propagate errors upstream or convert them to the same error handling used in sibling paths, and ensure any required types or fields (e.g., providerResponse, ctx, transcriptionID/metadata) are prepared before the call; if the current function signature lacks values needed by HandleProviderResponse, extend it minimally to accept them so behavior is consistent with other provider paths that already use HandleProviderResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/openai/openai.go`:
- Around line 2458-2465: The code currently sets
providerResponseHeaders["Content-Type"] which can break downstream lookups
expecting lowercase keys; update the logic in the block that checks
request.Params and
schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) so it
writes the header under the normalized lowercase key "content-type" (still
ensuring providerResponseHeaders is initialized) and use
resp.Header.ContentType() for the value; modify the assignment to
providerResponseHeaders to use the lowercase key instead of "Content-Type".
---
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Line 2524: The JSON transcription path left a TODO where
HandleProviderResponse should be invoked; replace the TODO with a call to
HandleProviderResponse (the existing function used elsewhere for normalizing
provider outputs) passing the provider response object, the current
context/metadata (e.g., transcription request/response structs), and the logger,
then handle its returned structured result and error—propagate errors upstream
or convert them to the same error handling used in sibling paths, and ensure any
required types or fields (e.g., providerResponse, ctx, transcriptionID/metadata)
are prepared before the call; if the current function signature lacks values
needed by HandleProviderResponse, extend it minimally to accept them so behavior
is consistent with other provider paths that already use HandleProviderResponse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7eebff3a-7a25-48d1-815d-f9831a0590e2
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
🚧 Files skipped from review as they are similar to previous changes (1)
- transports/bifrost-http/integrations/router.go
2970a18 to
2d3c5f8
Compare
|
@coderabbitai full review I changed my approach. The |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/providers/openai/openai.go (1)
2516-2516: Please resolve or track the TODO in this parsing path before merge.Line 2516 leaves known divergence in response handling behavior for this endpoint. If you want, I can draft the
HandleProviderResponseintegration patch and a focused test case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` at line 2516, The parsing path currently leaves a TODO where HandleProviderResponse should be invoked; replace the TODO with a call to HandleProviderResponse passing the raw provider response object, the parsed result structure, and context/metadata (e.g., request ID, provider name) so the unified response handling, error mapping, and metrics/telemetry behavior is preserved; ensure you handle returned error from HandleProviderResponse (propagate or convert to the existing error flow used in this function) and add/update any unit tests around the endpoint to cover both successful and error cases for HandleProviderResponse integration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/openai/openai.go`:
- Around line 2397-2399: The plain-text detection is currently gated only on
request.Params.ResponseFormat which misses cases where format arrives via
another path; update the handling around schemas.IsPlainTextTranscriptionFormat
so it first computes a single boolean (e.g., isPlainText) by checking
request.Params.ResponseFormat if present and falling back to the lpResult
response content-type header (or other request-level format) and then reuse that
boolean in both branches where lpResult.ResponseBody is processed (the blocks
referencing schemas.IsPlainTextTranscriptionFormat, response.Text,
sonic.Unmarshal, and lpResult.ResponseBody) so plain-text bodies set
response.Text and non-plain JSON bodies go through sonic.Unmarshal consistently.
---
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Line 2516: The parsing path currently leaves a TODO where
HandleProviderResponse should be invoked; replace the TODO with a call to
HandleProviderResponse passing the raw provider response object, the parsed
result structure, and context/metadata (e.g., request ID, provider name) so the
unified response handling, error mapping, and metrics/telemetry behavior is
preserved; ensure you handle returned error from HandleProviderResponse
(propagate or convert to the existing error flow used in this function) and
add/update any unit tests around the endpoint to cover both successful and error
cases for HandleProviderResponse integration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3018ac61-8515-4406-bbf7-e00a7c7b49f0
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/openai/openai.go (1)
2397-2399:⚠️ Potential issue | 🟠 MajorPlain-text detection is still coupled to
request.Params.ResponseFormat.Line 2397 and Line 2494 only enable plain-text handling when
request.Paramsis populated. If format is provided via another request path, Line 2502 can still attempt JSON unmarshal on plain text.🔧 Proposed fix (single boolean + content-type fallback)
+isPlainTextResponse := request.Params != nil && schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) +if !isPlainTextResponse { + if ct, ok := providerResponseHeaders["content-type"]; ok { + ct = strings.ToLower(strings.TrimSpace(strings.Split(ct, ";")[0])) + isPlainTextResponse = ct == "text/plain" || ct == "text/vtt" || ct == "application/x-subrip" + } +} -if request.Params != nil && schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) { +if isPlainTextResponse { response.Text = string(copiedResponseBody) if sendBackRawResponse { rawResponse = string(copiedResponseBody) } } else if customResponseHandler != nil {Also applies to: 2494-2503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2397 - 2399, Compute a single boolean like isPlainText before deciding how to decode lpResult: set isPlainText = (request.Params != nil && schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat)) || fallback check (e.g., inspect lpResult.ContentType header for "text/plain" or otherwise detect plain-text by attempting a safe sniff of lpResult.ResponseBody). Then use that isPlainText to choose between setting response.Text = string(lpResult.ResponseBody) and calling sonic.Unmarshal(lpResult.ResponseBody, response), so both the plain-text branch and the JSON-unmarshal branch consistently use the same detection logic; update the code paths around where lpResult, request.Params, response and sonic.Unmarshal are used to reference this new boolean.
🧹 Nitpick comments (3)
core/schemas/transcriptions.go (1)
29-40: Either make this helper OpenAI-specific, or includetxt.This package already defines
TranscriptionExportOptionsTxt, butIsPlainTextTranscriptionFormatonly matches"text","srt", and"vtt". The current name reads provider-agnostic, so later reuse can silently miss a plain-text format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/schemas/transcriptions.go` around lines 29 - 40, The helper IsPlainTextTranscriptionFormat is provider-agnostic but currently omits the package-defined TranscriptionExportOptionsTxt; update the function to either (A) remain provider-agnostic and include "txt" in its switch (add case "txt"), or (B) make it explicitly OpenAI-specific by renaming it (e.g., IsOpenAIPlainTextTranscriptionFormat) and documenting that it only checks "text","srt","vtt"; modify callers accordingly to use the new name if you choose option B. Ensure references to TranscriptionExportOptionsTxt are covered by the chosen change.transports/bifrost-http/integrations/router.go (1)
1069-1079: Add a regression test for this raw-byte bypass.This early return is now the behavior that keeps
text,srt, andvttresponses from being JSON-encoded again. A transport-level test that asserts the exact body bytes plus propagatedContent-Typewould make future refactors much safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/router.go` around lines 1069 - 1079, Add a transport-level regression test that exercises the raw-byte bypass path when the converter returns []byte: simulate a provider response for a text-format transcription (e.g., "text", "srt", "vtt"), have the converter return a []byte body and providerResponseHeaders including Content-Type, and assert that the handler writes the exact raw bytes (ctx.SetBody) and sets the provider headers on the response (ctx.Response.Header / Content-Type) without JSON-encoding; target the code paths around the rawBytes type assertion and early return so future refactors preserve this behavior.core/providers/openai/openai.go (1)
2516-2516: Resolve the TODO in this parsing branch before merge.Line 2516 leaves a known divergence in a response hot path; please either switch this branch to
HandleProviderResponsenow or track a linked follow-up issue.I can draft the exact
HandleProviderResponseintegration patch if you want.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` at line 2516, The TODO at the parsing branch must be resolved by invoking the shared response handler: replace the "// TODO: add HandleProviderResponse here" placeholder with a call to HandleProviderResponse(...) from the current branch, passing the same response object, provider metadata, and any parsed output/error values used in this function so the unified post-processing path is executed; if immediate integration isn't possible, add a linked follow-up issue reference and a clear comment explaining which variables (the provider response, parsing result, and error) need to be forwarded to HandleProviderResponse so the future patch can wire it exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/openai/openai.go`:
- Around line 2397-2399: Compute a single boolean like isPlainText before
deciding how to decode lpResult: set isPlainText = (request.Params != nil &&
schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat)) ||
fallback check (e.g., inspect lpResult.ContentType header for "text/plain" or
otherwise detect plain-text by attempting a safe sniff of
lpResult.ResponseBody). Then use that isPlainText to choose between setting
response.Text = string(lpResult.ResponseBody) and calling
sonic.Unmarshal(lpResult.ResponseBody, response), so both the plain-text branch
and the JSON-unmarshal branch consistently use the same detection logic; update
the code paths around where lpResult, request.Params, response and
sonic.Unmarshal are used to reference this new boolean.
---
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Line 2516: The TODO at the parsing branch must be resolved by invoking the
shared response handler: replace the "// TODO: add HandleProviderResponse here"
placeholder with a call to HandleProviderResponse(...) from the current branch,
passing the same response object, provider metadata, and any parsed output/error
values used in this function so the unified post-processing path is executed; if
immediate integration isn't possible, add a linked follow-up issue reference and
a clear comment explaining which variables (the provider response, parsing
result, and error) need to be forwarded to HandleProviderResponse so the future
patch can wire it exactly.
In `@core/schemas/transcriptions.go`:
- Around line 29-40: The helper IsPlainTextTranscriptionFormat is
provider-agnostic but currently omits the package-defined
TranscriptionExportOptionsTxt; update the function to either (A) remain
provider-agnostic and include "txt" in its switch (add case "txt"), or (B) make
it explicitly OpenAI-specific by renaming it (e.g.,
IsOpenAIPlainTextTranscriptionFormat) and documenting that it only checks
"text","srt","vtt"; modify callers accordingly to use the new name if you choose
option B. Ensure references to TranscriptionExportOptionsTxt are covered by the
chosen change.
In `@transports/bifrost-http/integrations/router.go`:
- Around line 1069-1079: Add a transport-level regression test that exercises
the raw-byte bypass path when the converter returns []byte: simulate a provider
response for a text-format transcription (e.g., "text", "srt", "vtt"), have the
converter return a []byte body and providerResponseHeaders including
Content-Type, and assert that the handler writes the exact raw bytes
(ctx.SetBody) and sets the provider headers on the response (ctx.Response.Header
/ Content-Type) without JSON-encoding; target the code paths around the rawBytes
type assertion and early return so future refactors preserve this behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dce07248-d95e-4c01-afc6-dee689a8279e
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/openai/openai.go (1)
2397-2399:⚠️ Potential issue | 🟠 MajorPlain-text detection is still too narrowly tied to
request.Params.ResponseFormat.At Line 2397 and Line 2494, plain-text handling only triggers when
request.Params.ResponseFormatis set. If format reaches this handler through another path, Line 2502 can still try JSON unmarshal on plain text and fail.Use this verification script to confirm all transcription request construction paths always populate
request.Params.ResponseFormatbefore reaching this code:#!/bin/bash set -euo pipefail echo "== Bifrost transcription request shape ==" rg -n --type=go 'type\s+BifrostTranscriptionRequest\s+struct|ResponseFormat|response_format|TranscriptionParameters' echo echo "== Callers of OpenAI transcription handler ==" rg -n --type=go 'HandleOpenAITranscriptionRequest\(|\.Transcription\(' echo echo "== Construction/population sites for transcription request params ==" rg -n --type=go 'BifrostTranscriptionRequest|Params:\s*&?schemas\.TranscriptionParameters|ResponseFormat:'Also applies to: 2494-2503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2397 - 2399, Broaden plain-text detection: in the block using request.Params.ResponseFormat, change the logic so you attempt JSON unmarshal first when the format is not explicitly plain-text and fall back to plain-text on unmarshal error; specifically, around the code that references request.Params.ResponseFormat, schemas.IsPlainTextTranscriptionFormat, sonic.Unmarshal, lpResult.ResponseBody and response.Text, keep the existing explicit plain-text branch, but in the other branch call sonic.Unmarshal into response and if it returns an error treat the payload as plain text by assigning response.Text = string(lpResult.ResponseBody); apply the identical change to the other occurrence that currently only checks request.Params.ResponseFormat so any path that reaches this handler without Params still correctly handles plain-text payloads.
🧹 Nitpick comments (1)
transports/bifrost-http/integrations/openai.go (1)
461-470: Deduplicate transcription conversion logic to avoid divergence.Both converters implement identical plain-text/raw-response fallback behavior. Extracting a shared helper will keep future fixes consistent.
♻️ Proposed refactor
+func convertOpenAITranscriptionResponse(resp *schemas.BifrostTranscriptionResponse) interface{} { + if schemas.IsPlainTextTranscriptionFormat(resp.ResponseFormat) { + return []byte(resp.Text) + } + if resp.ExtraFields.Provider == schemas.OpenAI && resp.ExtraFields.RawResponse != nil { + return resp.ExtraFields.RawResponse + } + return resp +} + ... TranscriptionResponseConverter: func(ctx *schemas.BifrostContext, resp *schemas.BifrostTranscriptionResponse) (interface{}, error) { - if schemas.IsPlainTextTranscriptionFormat(resp.ResponseFormat) { - return []byte(resp.Text), nil - } - if resp.ExtraFields.Provider == schemas.OpenAI { - if resp.ExtraFields.RawResponse != nil { - return resp.ExtraFields.RawResponse, nil - } - } - return resp, nil + return convertOpenAITranscriptionResponse(resp), nil }, ... TranscriptionResponseConverter: func(ctx *schemas.BifrostContext, resp *schemas.BifrostTranscriptionResponse) (interface{}, error) { - if schemas.IsPlainTextTranscriptionFormat(resp.ResponseFormat) { - return []byte(resp.Text), nil - } - if resp.ExtraFields.Provider == schemas.OpenAI { - if resp.ExtraFields.RawResponse != nil { - return resp.ExtraFields.RawResponse, nil - } - } - return resp, nil + return convertOpenAITranscriptionResponse(resp), nil },Also applies to: 858-867
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/openai.go` around lines 461 - 470, Extract the duplicated plain-text/raw-response fallback logic into a single helper function (e.g., convertTranscriptionResponse or NormalizeTranscriptionResponse) and have both TranscriptionResponseConverter callbacks call that helper; specifically, move the checks for schemas.IsPlainTextTranscriptionFormat(resp.ResponseFormat), resp.ExtraFields.Provider == schemas.OpenAI, and returning resp.ExtraFields.RawResponse / []byte(resp.Text) into the helper and return resp as the final fallback so both instances (the one at TranscriptionResponseConverter and the other at lines ~858-867) use the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/openai/openai.go`:
- Around line 2397-2399: Broaden plain-text detection: in the block using
request.Params.ResponseFormat, change the logic so you attempt JSON unmarshal
first when the format is not explicitly plain-text and fall back to plain-text
on unmarshal error; specifically, around the code that references
request.Params.ResponseFormat, schemas.IsPlainTextTranscriptionFormat,
sonic.Unmarshal, lpResult.ResponseBody and response.Text, keep the existing
explicit plain-text branch, but in the other branch call sonic.Unmarshal into
response and if it returns an error treat the payload as plain text by assigning
response.Text = string(lpResult.ResponseBody); apply the identical change to the
other occurrence that currently only checks request.Params.ResponseFormat so any
path that reaches this handler without Params still correctly handles plain-text
payloads.
---
Nitpick comments:
In `@transports/bifrost-http/integrations/openai.go`:
- Around line 461-470: Extract the duplicated plain-text/raw-response fallback
logic into a single helper function (e.g., convertTranscriptionResponse or
NormalizeTranscriptionResponse) and have both TranscriptionResponseConverter
callbacks call that helper; specifically, move the checks for
schemas.IsPlainTextTranscriptionFormat(resp.ResponseFormat),
resp.ExtraFields.Provider == schemas.OpenAI, and returning
resp.ExtraFields.RawResponse / []byte(resp.Text) into the helper and return resp
as the final fallback so both instances (the one at
TranscriptionResponseConverter and the other at lines ~858-867) use the same
implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26673d36-65ba-4936-a16a-d9f76b9ec471
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/openai/openai.go (1)
2397-2399:⚠️ Potential issue | 🟠 MajorUse a content-type fallback for plain-text detection.
At Line 2397 and Line 2494, plain-text mode only depends on
request.Params.ResponseFormat. If that field is unset but the provider returns plain-text content-type, Line 2399/Line 2502 still attempts JSON unmarshal and fails. Please compute oneisPlainTextflag using both request format and response content-type, and reuse it in both branches.Also applies to: 2494-2503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2397 - 2399, Compute a single boolean isPlainText (e.g., isPlainText := (request.Params != nil && schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat)) || strings.HasPrefix(strings.ToLower(lpResult.ContentType), "text/") || strings.Contains(strings.ToLower(lpResult.ContentType), "plain") ) before the JSON-unmarshal branches and use that flag instead of only checking request.Params.ResponseFormat; then in both places where you currently set response.Text = string(lpResult.ResponseBody) or call sonic.Unmarshal(..., response) (the branches around lpResult.ResponseBody and sonic.Unmarshal), use isPlainText to choose plain-text assignment vs JSON unmarshal so responses with plain text content-type are handled even when ResponseFormat is unset.
🧹 Nitpick comments (1)
core/providers/openai/openai.go (1)
2516-2516: Please resolve the TODO in this hot path.Line 2516 leaves transcription response handling split between manual parsing and
HandleProviderResponse. This should be implemented (or tracked explicitly) to avoid drift.I can draft the
HandleProviderResponserefactor for this block if you want.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` at line 2516, The TODO indicates transcription response handling is still done manually here instead of using HandleProviderResponse; replace the manual parsing/branching at this hot path with a call to HandleProviderResponse, passing the provider response, context (request/req/ctx), and any needed metadata (e.g., model, request ID, and original audio source), and ensure HandleProviderResponse returns the unified transcription result or error that this caller expects; update any local code that depended on the old manual parsing to consume the unified result (or propagate errors) so the hot path uses HandleProviderResponse consistently and avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/openai/openai.go`:
- Around line 2397-2399: Compute a single boolean isPlainText (e.g., isPlainText
:= (request.Params != nil &&
schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat)) ||
strings.HasPrefix(strings.ToLower(lpResult.ContentType), "text/") ||
strings.Contains(strings.ToLower(lpResult.ContentType), "plain") ) before the
JSON-unmarshal branches and use that flag instead of only checking
request.Params.ResponseFormat; then in both places where you currently set
response.Text = string(lpResult.ResponseBody) or call sonic.Unmarshal(...,
response) (the branches around lpResult.ResponseBody and sonic.Unmarshal), use
isPlainText to choose plain-text assignment vs JSON unmarshal so responses with
plain text content-type are handled even when ResponseFormat is unset.
---
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Line 2516: The TODO indicates transcription response handling is still done
manually here instead of using HandleProviderResponse; replace the manual
parsing/branching at this hot path with a call to HandleProviderResponse,
passing the provider response, context (request/req/ctx), and any needed
metadata (e.g., model, request ID, and original audio source), and ensure
HandleProviderResponse returns the unified transcription result or error that
this caller expects; update any local code that depended on the old manual
parsing to consume the unified result (or propagate errors) so the hot path uses
HandleProviderResponse consistently and avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a3742135-c97d-4220-a173-5ad8e374cfab
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
2f8d1a1 to
06cefd4
Compare
2d3c5f8 to
4538bc0
Compare
4538bc0 to
49fe4b9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/openai/openai.go (1)
2519-2526: Replace this TODO with the shared response handler to avoid parser drift.This JSON branch still does manual unmarshal/raw parsing. Converging on
providerUtils.HandleProviderResponsewill keep behavior aligned with other handlers and remove duplicated parsing logic.♻️ Proposed refactor
- // TODO: add HandleProviderResponse here - - // Parse raw response for RawResponse field - if sendBackRawResponse { - if err := sonic.Unmarshal(copiedResponseBody, &rawResponse); err != nil { - return nil, providerUtils.NewBifrostOperationError(schemas.ErrProviderRawResponseUnmarshal, err, providerName) - } - } + _, rawResponse, bifrostErr = providerUtils.HandleProviderResponse( + copiedResponseBody, + response, + nil, + false, + sendBackRawResponse, + ) + if bifrostErr != nil { + return nil, bifrostErr + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2519 - 2526, The manual raw JSON parsing under the sendBackRawResponse branch should be replaced with the shared handler: call providerUtils.HandleProviderResponse instead of sonic.Unmarshal to parse copiedResponseBody into rawResponse and handle errors consistently; update the surrounding code in openai.go to pass copiedResponseBody, sendBackRawResponse, a pointer to rawResponse and providerName into providerUtils.HandleProviderResponse and propagate any returned error (instead of constructing providerUtils.NewBifrostOperationError locally) so this code uses the centralized parsing/response logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Around line 2519-2526: The manual raw JSON parsing under the
sendBackRawResponse branch should be replaced with the shared handler: call
providerUtils.HandleProviderResponse instead of sonic.Unmarshal to parse
copiedResponseBody into rawResponse and handle errors consistently; update the
surrounding code in openai.go to pass copiedResponseBody, sendBackRawResponse, a
pointer to rawResponse and providerName into
providerUtils.HandleProviderResponse and propagate any returned error (instead
of constructing providerUtils.NewBifrostOperationError locally) so this code
uses the centralized parsing/response logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e45de41a-541c-48b1-9627-009f115658df
📒 Files selected for processing (4)
core/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/integrations/router.go
- transports/bifrost-http/integrations/openai.go
6c6c894 to
399b52a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/openai/openai.go`:
- Around line 2400-2402: The plain-text transcription branch currently sets
response.Text but doesn't preserve ExtraFields.RawResponse like the
large-payload path does; update the branch that checks
schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) (and the
corresponding large-payload handling around lpResult.ResponseBody and
sonic.Unmarshal) to also set response.ExtraFields.RawResponse =
string(lpResult.ResponseBody) when sendBackRawResponse is enabled (or otherwise
mirror the same RawResponse assignment logic used in the large-payload path) so
RawResponse is consistent for both standard and large transcription flows.
In `@core/schemas/transcriptions.go`:
- Around line 29-36: The BackfillParams method currently checks the wrong
condition and can panic when req.Params is nil; change it so that after guarding
r and req for nil you first ensure req.Params is non-nil and then only assign
req.Params.ResponseFormat to r.ResponseFormat when r.ResponseFormat is nil
(i.e., invert the current condition) to avoid overwriting provider-populated
values and to prevent a nil dereference; locate the method BackfillParams on
type BifrostTranscriptionResponse and update the conditional logic that
references r.ResponseFormat and req.Params.ResponseFormat accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9cde90f6-47bb-45ed-a6ff-557e4f51019e
📒 Files selected for processing (5)
core/bifrost.gocore/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/integrations/router.go
- transports/bifrost-http/integrations/openai.go
11f1f5f to
ddb0f95
Compare
399b52a to
f82d0ac
Compare
f82d0ac to
217b180
Compare
ddb0f95 to
11f1f5f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/openai/openai.go (1)
2422-2424:⚠️ Potential issue | 🟡 MinorKeep the plain-text branches behaviorally identical.
Lines 2519-2521 preserve
RawResponse, but Lines 2422-2424 only fillText. Largetext/srt/vtttranscriptions will therefore loseExtraFields.RawResponsepurely based on payload size. Mirroring the same assignment in the large-payload path — or extracting a tiny shared helper for both branches — will keep the two flows aligned.🩹 Minimal fix
if len(lpResult.ResponseBody) > 0 { response := &schemas.BifrostTranscriptionResponse{} + var rawResponse interface{} if request.Params != nil && schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) { - response.Text = string(lpResult.ResponseBody) + text := string(lpResult.ResponseBody) + response.Text = text + if sendBackRawResponse { + rawResponse = text + } } else if err := sonic.Unmarshal(lpResult.ResponseBody, response); err != nil { return nil, providerUtils.NewBifrostOperationError(schemas.ErrProviderResponseUnmarshal, err, providerName) } - response.ExtraFields = schemas.BifrostResponseExtraFields{Provider: providerName, ModelRequested: request.Model, RequestType: schemas.TranscriptionRequest, Latency: lpResult.Latency} + response.ExtraFields = schemas.BifrostResponseExtraFields{ + Provider: providerName, + ModelRequested: request.Model, + RequestType: schemas.TranscriptionRequest, + Latency: lpResult.Latency, + RawResponse: rawResponse, + } return response, nil }Also applies to: 2517-2522
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2422 - 2424, The plain-text transcription branch (where schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) is true) only sets response.Text and therefore omits populating response.ExtraFields.RawResponse like the large-payload path does; update the plain-text branch in the openai provider so it mirrors the large-payload behavior by assigning response.ExtraFields.RawResponse = lpResult.ResponseBody (or refactor into a small helper used by both branches) before returning, keeping the use of lpResult.ResponseBody and sonic.Unmarshal for the non-plain-text branch unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/openai/openai.go`:
- Around line 2422-2424: The plain-text transcription branch (where
schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) is true)
only sets response.Text and therefore omits populating
response.ExtraFields.RawResponse like the large-payload path does; update the
plain-text branch in the openai provider so it mirrors the large-payload
behavior by assigning response.ExtraFields.RawResponse = lpResult.ResponseBody
(or refactor into a small helper used by both branches) before returning,
keeping the use of lpResult.ResponseBody and sonic.Unmarshal for the
non-plain-text branch unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 874fee69-da94-4b9c-8de1-fb8cfa7a6b07
📒 Files selected for processing (5)
core/bifrost.gocore/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
🚧 Files skipped from review as they are similar to previous changes (3)
- transports/bifrost-http/integrations/openai.go
- core/bifrost.go
- core/schemas/transcriptions.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/openai/openai.go (1)
2422-2427:⚠️ Potential issue | 🟡 MinorPreserve
RawResponsein the large-payload plain-text path.At Line 2422-Line 2427, plain-text LP responses set
response.Textbut never populateExtraFields.RawResponse, while the standard path does this at Line 2519-Line 2521.sendBackRawResponse=truebehaves differently based on payload path.🩹 Proposed fix
if len(lpResult.ResponseBody) > 0 { response := &schemas.BifrostTranscriptionResponse{} + var rawResponse interface{} if request.Params != nil && schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) { response.Text = string(lpResult.ResponseBody) + if sendBackRawResponse { + rawResponse = response.Text + } } else if err := sonic.Unmarshal(lpResult.ResponseBody, response); err != nil { return nil, providerUtils.NewBifrostOperationError(schemas.ErrProviderResponseUnmarshal, err, providerName) } - response.ExtraFields = schemas.BifrostResponseExtraFields{Provider: providerName, ModelRequested: request.Model, RequestType: schemas.TranscriptionRequest, Latency: lpResult.Latency} + response.ExtraFields = schemas.BifrostResponseExtraFields{ + Provider: providerName, + ModelRequested: request.Model, + RequestType: schemas.TranscriptionRequest, + Latency: lpResult.Latency, + RawResponse: rawResponse, + } return response, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` around lines 2422 - 2427, The plain-text large-payload branch in the transcription path sets response.Text but never populates response.ExtraFields.RawResponse, causing sendBackRawResponse to differ from the normal path; update the plain-text branch in the same block that uses schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) to populate response.ExtraFields (schemas.BifrostResponseExtraFields) with the same fields used in the normal path and include RawResponse from lpResult.ResponseBody (convert to the correct type expected by RawResponse, e.g., string or json.RawMessage) alongside Provider, ModelRequested, RequestType, and Latency so the raw payload is preserved.
🧹 Nitpick comments (1)
core/providers/openai/openai.go (1)
2539-2539: Track or resolve this TODO before it gets stale.At Line 2539, please either complete the
HandleProviderResponsemigration now or attach an issue reference so this path doesn’t drift.I can draft the follow-up issue (or the actual patch) if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/openai/openai.go` at line 2539, The TODO for HandleProviderResponse must not be left unresolved—either implement the function or attach an issue reference; locate the placeholder comment for HandleProviderResponse in the openai provider (symbol: HandleProviderResponse) and replace it with a concrete implementation that mirrors the project's provider response contract (validate provider response, normalize fields, handle errors/edge cases and return the expected response type), or if implementation is deferred add a clear TODO with an issue URL/ID and a short comment describing what the implementation must do (inputs, outputs, error handling) so the purpose won’t drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/openai/openai.go`:
- Around line 2422-2427: The plain-text large-payload branch in the
transcription path sets response.Text but never populates
response.ExtraFields.RawResponse, causing sendBackRawResponse to differ from the
normal path; update the plain-text branch in the same block that uses
schemas.IsPlainTextTranscriptionFormat(request.Params.ResponseFormat) to
populate response.ExtraFields (schemas.BifrostResponseExtraFields) with the same
fields used in the normal path and include RawResponse from
lpResult.ResponseBody (convert to the correct type expected by RawResponse,
e.g., string or json.RawMessage) alongside Provider, ModelRequested,
RequestType, and Latency so the raw payload is preserved.
---
Nitpick comments:
In `@core/providers/openai/openai.go`:
- Line 2539: The TODO for HandleProviderResponse must not be left
unresolved—either implement the function or attach an issue reference; locate
the placeholder comment for HandleProviderResponse in the openai provider
(symbol: HandleProviderResponse) and replace it with a concrete implementation
that mirrors the project's provider response contract (validate provider
response, normalize fields, handle errors/edge cases and return the expected
response type), or if implementation is deferred add a clear TODO with an issue
URL/ID and a short comment describing what the implementation must do (inputs,
outputs, error handling) so the purpose won’t drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65804c10-d84d-432d-a4e8-3ac2fcf6e5b6
📒 Files selected for processing (5)
core/bifrost.gocore/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
✅ Files skipped from review due to trivial changes (1)
- core/bifrost.go
🚧 Files skipped from review as they are similar to previous changes (3)
- transports/bifrost-http/integrations/router.go
- core/schemas/transcriptions.go
- transports/bifrost-http/integrations/openai.go
217b180 to
7f1c246
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/schemas/transcriptions.go (1)
29-34:⚠️ Potential issue | 🟠 MajorPrevent
BackfillParamsfrom clobbering provider-populated format.Line 33 unconditionally assigns
r.ResponseFormat, which can overwrite provider-set values and misroute downstream plain-text handling. Only backfill whenr.ResponseFormatis nil.💡 Proposed fix
func (r *BifrostTranscriptionResponse) BackfillParams(req *BifrostTranscriptionRequest) { if r == nil || req == nil || req.Params == nil || req.Params.ResponseFormat == nil { return } - r.ResponseFormat = req.Params.ResponseFormat + if r.ResponseFormat == nil { + r.ResponseFormat = req.Params.ResponseFormat + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/schemas/transcriptions.go` around lines 29 - 34, In BifrostTranscriptionResponse.BackfillParams, avoid clobbering a provider-set format by only assigning r.ResponseFormat from req.Params.ResponseFormat when r.ResponseFormat is nil; check r.ResponseFormat == nil before setting it so existing provider-populated values are preserved (referencing BifrostTranscriptionResponse.BackfillParams, BifrostTranscriptionResponse.ResponseFormat and BifrostTranscriptionRequest.Params.ResponseFormat).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/schemas/transcriptions.go`:
- Around line 29-34: In BifrostTranscriptionResponse.BackfillParams, avoid
clobbering a provider-set format by only assigning r.ResponseFormat from
req.Params.ResponseFormat when r.ResponseFormat is nil; check r.ResponseFormat
== nil before setting it so existing provider-populated values are preserved
(referencing BifrostTranscriptionResponse.BackfillParams,
BifrostTranscriptionResponse.ResponseFormat and
BifrostTranscriptionRequest.Params.ResponseFormat).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00df0cb0-94d7-41d0-9edd-f75538fe5d81
📒 Files selected for processing (5)
core/bifrost.gocore/providers/openai/openai.gocore/schemas/transcriptions.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.go
✅ Files skipped from review due to trivial changes (2)
- core/bifrost.go
- core/providers/openai/openai.go
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/integrations/openai.go
- transports/bifrost-http/integrations/router.go
* refactor: standardize empty array conventions for VK Provider & MCP Configs, and makes Provider Config weight optional for routing (#1932)
## Summary
Changes Virtual Key provider and MCP configurations from "allow-all by default" to "deny-by-default" security model. Virtual Keys now require explicit provider and MCP client configurations to allow access, improving security posture.
## Changes
- **Provider Configs**: Empty `provider_configs` now blocks all providers instead of allowing all
- **MCP Configs**: Empty `mcp_configs` now blocks all MCP tools instead of allowing all
- **Weight Field**: Changed provider `weight` from required `float64` to optional `*float64` - null weight excludes provider from weighted routing
- **Migration**: Added automatic backfill migration to preserve existing Virtual Key behavior by adding all available providers/MCP clients to VKs with empty configs
- **Documentation**: Updated all references to reflect new deny-by-default behavior
- **UI Updates**: Modified Virtual Key creation/editing interface to reflect new behavior and weight handling
## Type of change
- [x] Feature
- [x] Refactor
- [x] Documentation
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
Test Virtual Key creation and provider/MCP access:
```sh
# Core/Transports
go version
go test ./...
# Test Virtual Key with no provider configs blocks requests
curl -X POST http://localhost:8080/v1/chat/completions \
-H "Authorization: Bearer sk-bf-empty-vk" \
-H "Content-Type: application/json" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Should return error about no providers configured
# Test Virtual Key with provider configs allows requests
curl -X POST http://localhost:8080/v1/chat/completions \
-H "Authorization: Bearer sk-bf-configured-vk" \
-H "Content-Type: application/json" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Should work normally
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
## Breaking changes
- [x] Yes
**Impact**: Existing Virtual Keys with empty `provider_configs` or `mcp_configs` would be blocked after this change.
**Migration**: Automatic migration `migrationBackfillEmptyVirtualKeyConfigs` runs on startup to backfill existing Virtual Keys with all available providers/MCP clients, preserving current behavior. New Virtual Keys created after this change will use deny-by-default.
## Security considerations
This change significantly improves security posture by requiring explicit configuration of allowed providers and MCP tools for Virtual Keys. The automatic migration ensures no disruption to existing deployments while new Virtual Keys benefit from the more secure default behavior.
## 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
* feat: add MCP auto tool injection toggle (#1933)
## Summary
Adds a new configuration option `DisableAutoToolInject` to the MCP (Model Context Protocol) system that allows disabling automatic tool injection into requests. When enabled, MCP tools are only included when explicitly requested via context headers or filters, providing more granular control over tool availability.
## Changes
- Added `DisableAutoToolInject` field to `MCPToolManagerConfig` schema with runtime update support
- Implemented atomic boolean storage in `ToolsManager` to safely handle concurrent access
- Added logic in `ParseAndAddToolsToRequest` to respect the disable flag and only inject tools when explicit context filters are present
- Extended configuration management with database migration, UI controls, and API endpoints
- Added hot-reload capability through `UpdateMCPDisableAutoToolInject` methods across the stack
- Updated UI with a toggle switch and clear documentation about the feature's behavior
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Validate the new MCP auto tool injection toggle:
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Test the feature:
1. Configure MCP clients and tools
2. Enable "Disable Auto Tool Injection" in the MCP configuration UI
3. Make requests without explicit tool headers - tools should not be injected
4. Make requests with `x-bf-mcp-include-tools` header - tools should be injected
5. Verify hot-reload works by toggling the setting without server restart
## Screenshots/Recordings
UI changes include a new toggle switch in the MCP configuration view with descriptive text explaining when tools are injected based on explicit headers.
## Breaking changes
- [ ] Yes
- [x] No
This is a backward-compatible addition with a default value of `false` (auto injection enabled).
## Related issues
This addresses the need for more granular control over MCP tool injection behavior in request processing.
## Security considerations
The feature provides better control over tool exposure by allowing administrators to require explicit opt-in for tool injection, potentially reducing unintended tool access.
## 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
* feat: VK MCP config now works as an AllowList (#1940)
## Summary
This PR implements MCP tool governance by enforcing virtual key MCP configurations as an execution-time allow-list. When virtual keys have empty MCPConfigs, all MCP tools are denied. When non-empty, each tool is validated against the configured allow-list at both inference time and MCP tool execution.
## Changes
- **Context parameter updates**: Changed MCP-related functions to use `*schemas.BifrostContext` instead of `context.Context` to enable tool tracking
- **Tool tracking**: Added `BifrostContextKeyMCPAddedTools` context key to track which MCP tools are added to requests
- **Governance enforcement**: Virtual key MCP configurations now act as execution-time allow-lists with validation in both `PreMCPHook` and `evaluateGovernanceRequest`
- **Auto-injection control**: Added `DisableAutoToolInject` configuration option that respects the toggle and skips auto-injection when headers are already set by callers
- **Decision type**: Added `DecisionMCPToolBlocked` for MCP tool governance violations
- **UI improvements**: Updated MCP view description and sidebar item naming for better clarity
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Test MCP tool governance with virtual keys:
```sh
# Core/Transports
go version
go test ./...
# Test with virtual key having empty MCPConfigs (should deny all MCP tools)
curl -X POST /v1/chat/completions \
-H "x-bf-virtual-key: test-vk-empty-mcp" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Test with virtual key having specific MCP tool allowlist
curl -X POST /v1/chat/completions \
-H "x-bf-virtual-key: test-vk-with-mcp" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Test disable auto tool inject configuration
curl -X PUT /v1/config/mcp/disable-auto-tool-inject \
-d '{"disable": true}'
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
New configuration options:
- `disable_auto_tool_inject`: Boolean flag to disable automatic MCP tool injection
- Virtual key `MCPConfigs`: Array of MCP client configurations that act as allow-lists
## Screenshots/Recordings
UI changes include updated MCP configuration view with clearer descriptions for the disable auto tool injection toggle and improved sidebar navigation labels.
## Breaking changes
- [x] Yes
- [ ] No
**Impact**: MCP-related function signatures now require `*schemas.BifrostContext` instead of `context.Context`. Virtual keys with empty MCPConfigs will now deny all MCP tools by default.
**Migration**: Update any custom MCP integrations to use the new context parameter type. Configure MCPConfigs on virtual keys that need MCP tool access.
## Related issues
Implements MCP tool governance and execution-time validation for virtual key configurations.
## Security considerations
- **Access control**: Virtual key MCP configurations now enforce strict allow-lists for tool execution
- **Context isolation**: Tool tracking is isolated per request context to prevent cross-request leakage
- **Validation**: Both pre-execution and execution-time validation prevent unauthorized tool access
## 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
* refactor: standardize empty array conventions for VK Provider Config Allowed Keys (#2006)
## Summary
Migrates VK provider config allowed keys from implicit allow-all semantics to explicit deny-by-default behavior. Adds `AllowAllKeys` boolean field to enable granular key access control while maintaining backward compatibility.
## Changes
- Added `AllowAllKeys` boolean field to `TableVirtualKeyProviderConfig` with database migration
- Backfilled existing configs with `allow_all_keys=true` to preserve current behavior
- Updated key resolution logic: empty keys now denies all access, `["*"]` wildcard allows all keys
- Modified governance resolver to set empty `includeOnlyKeys` slice when no keys are configured
- Enhanced HTTP handlers to recognize `["*"]` wildcard and set `AllowAllKeys` flag appropriately
- Updated UI to display "Allow All Keys" option and show deny-by-default messaging
- Added JSON unmarshaling support for `["*"]` wildcard in config files
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Validate the migration and new key access control behavior:
```sh
# Core/Transports
go version
go test ./...
# Test migration runs successfully
go run main.go migrate
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Test scenarios:
1. Create VK with empty `key_ids` - should deny all keys
2. Create VK with `key_ids: ["*"]` - should allow all keys
3. Create VK with specific key IDs - should allow only those keys
4. Verify existing VKs maintain their current behavior after migration
## Screenshots/Recordings
UI now shows:
- "Allow All Keys" option in key selection dropdown
- "No keys allowed" vs "All keys allowed" status indicators
- "No providers configured (deny-by-default)" messaging
## Breaking changes
- [ ] Yes
- [x] No
The migration preserves existing behavior by setting `allow_all_keys=true` for configs that previously had no keys specified.
## Related issues
Part of VK access control enhancement initiative.
## Security considerations
Improves security posture by implementing deny-by-default semantics for key access. Existing deployments maintain current access patterns through automatic backfill migration.
## 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
* refactor: standardize empty array conventions for allowed models (#2113)
## Summary
Standardizes empty array conventions across Bifrost to implement deny-by-default security semantics. Previously, empty arrays for `allowed_models` and `Models` fields meant "allow all", creating potential security gaps. Now `["*"]` explicitly means "allow all" while empty arrays mean "deny all".
## Changes
- **Core Logic**: Updated model filtering in `bifrost.go` and `selectKeyFromProviderForModel` to treat empty `Models` arrays as deny-all and `["*"]` as allow-all
- **Database Migration**: Added `migrationBackfillAllowedModelsWildcard` to convert existing empty arrays to `["*"]` preserving current behavior for existing records
- **Model Catalog**: Updated `IsModelAllowedForProvider` to use wildcard semantics with deny-by-default fallback
- **Schema Defaults**: Changed default `Models` value from `[]` to `["*"]` in table definitions and form schemas
- **UI Components**: Enhanced `ModelMultiselect` with `allowAllOption` prop and updated virtual key forms to handle wildcard selection
- **Documentation**: Updated JSON schemas, comments, and tooltips to reflect new conventions
- **Governance**: Updated provider config filtering logic to use new wildcard semantics
- **Server Bootstrap**: Added wildcard filtering when loading models to prevent literal "*" from appearing as a model name
## Type of change
- [x] Refactor
- [ ] Bug fix
- [ ] Feature
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
Validate the migration and new semantics:
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Test scenarios:
1. Create new virtual keys - should default to `["*"]` for allowed models
2. Create new provider keys - should default to `["*"]` for models
3. Verify existing keys with empty arrays are migrated to `["*"]`
4. Test that empty arrays now deny all models/keys as expected
5. Verify UI shows "All models allowed" for wildcard and "No models (deny all)" for empty arrays
## Screenshots/Recordings
UI changes include:
- Model multiselect now shows "Allow All Models" option
- Virtual key details display "All Models" badge for wildcard vs "No models (deny all)" for empty
- Form placeholders updated to reflect new semantics
## Breaking changes
- [x] Yes
- [ ] No
**Migration Impact**: The database migration automatically converts existing empty `allowed_models` and `models_json` arrays to `["*"]`, preserving current behavior. However, any new configurations with empty arrays will now deny access instead of allowing all. Applications relying on "empty = allow all" semantics must be updated to use `["*"]` explicitly.
## Related issues
Part of security hardening initiative to implement explicit allow-lists and deny-by-default semantics across Bifrost configuration.
## Security considerations
This change significantly improves security posture by:
- Eliminating ambiguous "empty means allow all" semantics
- Implementing explicit deny-by-default for new configurations
- Requiring intentional wildcard usage via `["*"]` for broad access
- Maintaining backward compatibility through automatic migration
## 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
* refactor: replace string slices with WhiteList for allowlist fields (#2125)
## Summary
Introduces a new `WhiteList` type to standardize whitelist behavior across the codebase, replacing manual slice operations and string comparisons with semantic methods for handling allow/deny lists.
## Changes
- Added `WhiteList` type with methods `IsAllowed()`, `IsUnrestricted()`, `IsEmpty()`, `Contains()`, and `Validate()`
- Replaced `[]string` fields with `WhiteList` for model restrictions, tool filtering, and key access controls
- Updated all whitelist logic to use semantic methods instead of manual `slices.Contains()` checks
- Added validation to ensure wildcards ("*") aren't mixed with specific values and prevent duplicates
- Improved case-insensitive matching for whitelist comparisons
## Type of change
- [x] Refactor
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Plugins
## How to test
Verify that whitelist behavior remains consistent across all affected components:
```sh
# Core/Transports
go version
go test ./...
# Test specific whitelist scenarios:
# - Empty lists deny all access
# - ["*"] allows all access
# - Specific lists only allow listed items
# - Mixed wildcards and specific items are rejected
# - Duplicate entries are rejected
```
Test key model filtering, MCP tool execution, and virtual key configurations to ensure whitelist logic works correctly.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
The `WhiteList` type maintains the same JSON serialization format as `[]string`, so existing configurations remain compatible.
## Related issues
N/A
## Security considerations
Improves security by standardizing deny-by-default behavior and adding validation to prevent misconfigured whitelists that could inadvertently grant excessive permissions.
## 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
* feat: add request-level extra headers support for MCP tool execution (#2126)
## Summary
This PR adds support for request-level extra headers in MCP tool execution, allowing callers to forward specific headers to MCP servers at runtime based on a per-client allowlist configuration.
## Changes
- Added `AllowedExtraHeaders` field to MCP client configuration with allowlist semantics (empty array = deny all, `["*"]` = allow all)
- Introduced `BifrostContextKeyMCPExtraHeaders` context key to track headers forwarded to MCP tools
- Created `core/mcp/utils` package with `GetHeadersForToolExecution` function to merge static and dynamic headers
- Updated MCP tool execution in both regular tool manager and Starlark code mode to use the new header forwarding system
- Added database migration for `allowed_extra_headers_json` column in MCP client table
- Updated UI to include allowed extra headers configuration in MCP client management
- Enhanced auth demo server example to demonstrate tool-execution level authentication patterns
## Type of change
- [x] Feature
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] UI (Next.js)
## How to test
1. Configure an MCP client with allowed extra headers:
```json
{
"name": "test-client",
"connection_string": "http://localhost:3002/",
"auth_type": "headers",
"headers": {
"X-API-Key": "connection-secret"
},
"allowed_extra_headers": ["X-Tool-Token"],
"tools_to_execute": ["*"]
}
```
2. Make requests with extra headers that should be forwarded:
```bash
curl -X POST http://localhost:8080/v1/chat/completions \
-H "Authorization: Bearer your-key" \
-H "X-Tool-Token: tool-execution-secret" \
-d '{
"model": "gpt-4",
"messages": [{"role": "user", "content": "Use the secret_data tool"}],
"tools": [{"type": "function", "function": {"name": "secret_data"}}]
}'
```
3. Test the auth demo server:
```bash
cd examples/mcps/auth-demo-server
go run main.go
# Server demonstrates two-tier auth: connection-level (X-API-Key) and tool-level (X-Tool-Token)
```
4. Run tests:
```sh
go test ./core/mcp/...
go test ./transports/bifrost-http/...
cd ui
pnpm test
pnpm build
```
## Breaking changes
- [ ] Yes
- [x] No
This is a backward-compatible addition. Existing MCP clients will have empty `allowed_extra_headers` (deny all extra headers) which maintains current behavior.
## Security considerations
- Extra headers are filtered through a strict allowlist per MCP client
- Security denylist prevents auth header overrides via extra headers
- Two-tier authentication pattern demonstrated: connection-level + tool-execution level
- Headers are only forwarded to MCP servers that explicitly allow them
## 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
* fix: apply MCP tool filtering headers to tools/list response when using bifrost as MCP gateway (#2127)
## Summary
Adds support for `x-bf-mcp-include-clients` and `x-bf-mcp-include-tools` request headers to filter MCP tools/list response when using Bifrost as an MCP gateway. This ensures that tool filtering is respected at the MCP protocol level, not just during inference.
## Changes
- Implemented dynamic tool filtering in MCP server handlers that respects per-request include headers
- Added `makeIncludeClientsFilter()` function that filters tools based on request context values
- Registered the tool filter on both global and virtual key MCP servers during initialization
- Updated documentation to clarify that `mcp-include-tools` requires `clientName-toolName` format
- Enhanced examples in documentation to show proper tool naming format
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [x] Docs
## How to test
Test MCP gateway functionality with tool filtering:
```sh
# Test tools/list filtering with include-tools header
curl --location 'http://localhost:8080/mcp/tools/list' \
--header 'x-bf-mcp-include-tools: gmail-send_email,filesystem-read_file' \
--header 'Authorization: Bearer your-vk-here'
# Test tools/list filtering with include-clients header
curl --location 'http://localhost:8080/mcp/tools/list' \
--header 'x-bf-mcp-include-clients: gmail,filesystem' \
--header 'Authorization: Bearer your-vk-here'
# Verify chat completions still respect the same headers
curl --location 'http://localhost:8080/v1/chat/completions' \
--header 'x-bf-mcp-include-tools: gmail-send_email' \
--header 'Content-Type: application/json' \
--data '{
"model": "openai/gpt-4o-mini",
"messages": [{"role": "user", "content": "What tools are available?"}]
}'
```
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
The tool filtering mechanism ensures that virtual key restrictions are properly enforced at the MCP protocol level, preventing unauthorized access to tools that should be filtered out based on request headers.
## 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
* refactor: parallelize model listing for providers to speed up startup time (#2151)
## Summary
Parallelizes model listing operations for providers during server startup and provider reloading to significantly reduce initialization time. Previously, model listing was performed sequentially for each provider, causing slower startup times especially when multiple providers were configured.
## Changes
- Added concurrent execution using goroutines and sync.WaitGroup for model listing operations in three key functions: `ReloadProvider`, `ForceReloadPricing`, and `Bootstrap`
- In `ReloadProvider`, both filtered and unfiltered model listing requests now run concurrently for the same provider
- In `ForceReloadPricing` and `Bootstrap`, model listing for different providers now runs in parallel instead of sequentially
- Moved provider key retrieval earlier in `ReloadProvider` to ensure it happens before concurrent model listing
- Added proper context cancellation with defer statements for bifrost contexts
## Type of change
- [x] Refactor
## Affected areas
- [x] Transports (HTTP)
## How to test
Test server startup time with multiple providers configured to verify the performance improvement:
```sh
# Core/Transports
go version
go test ./...
# Test with multiple providers configured
# Measure startup time before and after the change
time go run main.go
```
Configure multiple providers in your bifrost configuration and observe faster startup times, especially noticeable when providers have high latency or many models.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications. The change maintains the same authentication and authorization patterns while improving performance through parallelization.
## 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
* fix: reorder migrations and set AllowAllKeys to true for virtual key provider configs (#2158)
## Summary
Fixes database migration ordering issue and ensures virtual key configurations are properly initialized with the AllowAllKeys field set to true.
## Changes
- Reordered database migrations to execute `migrationAddAllowAllKeysToProviderConfig` before `migrationBackfillEmptyVirtualKeyConfigs` to ensure the AllowAllKeys column exists before backfilling
- Added `AllowAllKeys: true` to provider configurations created during virtual key backfill migration to enable unrestricted key access by default
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that database migrations run successfully and virtual key configurations are created with proper defaults:
```sh
# Core/Transports
go version
go test ./...
```
Test migration ordering by running against a fresh database to ensure no column reference errors occur.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
This change enables unrestricted key access by default for virtual key configurations, which may have security implications depending on the intended access control model.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: implement scoped pricing override
* refactor: custom pricing refactor
* fix: resolve merge conflicts in config loading and governance functions (#2230)
## Summary
Resolves Git merge conflicts in the bifrost-http configuration loading code by cleaning up duplicate function definitions and consolidating the configuration initialization flow.
## Changes
- Removed Git merge conflict markers and duplicate code blocks from `LoadConfig` function
- Consolidated governance configuration loading by keeping both `loadGovernanceConfigFromFile` and `loadGovernanceConfig` functions with distinct purposes
- Removed duplicate `convertSchemasMCPClientConfigToTable` function definition
- Moved pricing overrides initialization logic to `initFrameworkConfig` function for better organization
- Cleaned up extensive duplicate default configuration loading code that was causing merge conflicts
- Changed error handling for pricing overrides from returning error to logging warning
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that configuration loading works correctly without merge conflicts:
```sh
# Core/Transports
go version
go test ./...
go build ./transports/bifrost-http/...
```
Test configuration loading with various scenarios:
- Config file present
- Config file absent (default loading)
- Store-based configuration
- Governance and MCP configuration loading
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications - this is a merge conflict resolution that maintains existing functionality.
## 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
* feat: add Stability AI model support for Bedrock image generation (#2180)
## Summary
Adds support for Stability AI image generation models (stability.stable-image-*) to the Bedrock provider, enabling text-to-image generation with models like stability.stable-image-core-v1:1 and stability.stable-image-ultra-v1:1.
## Changes
- Added `isStabilityAIModel()` function to detect Stability AI models by "stability." prefix
- Created `ToStabilityAIImageGenerationRequest()` to convert Bifrost requests to Stability AI's flat request format
- Implemented `StabilityAIImageGenerationRequest` type with support for prompt, mode, aspect_ratio, output_format, seed, and negative_prompt parameters
- Added conditional routing in `ImageGeneration()` to use Stability AI request format when appropriate
- Extended known fields for image generation parameters to include "aspect_ratio" and "input_images"
- Updated documentation comment to reflect Stability AI model support
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test Stability AI image generation through the Bedrock provider:
```sh
# Core/Transports
go version
go test ./...
# Test with a Stability AI model
curl -X POST http://localhost:8080/v1/images/generations \
-H "Content-Type: application/json" \
-H "Authorization: Bearer your-key" \
-d '{
"model": "stability.stable-image-core-v1:1",
"prompt": "A beautiful sunset over mountains",
"aspect_ratio": "16:9",
"output_format": "PNG"
}'
```
Ensure AWS credentials are configured for Bedrock access and the Stability AI models are available in your region.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No additional security implications beyond existing Bedrock provider authentication and AWS credential handling.
## 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
* feat: add Stability AI image edit models support to Bedrock provider (#2225)
## Summary
Adds support for Stability AI image editing models in the Bedrock provider, expanding image editing capabilities beyond the existing Titan and Nova Canvas models.
## Changes
- Added `getStabilityAIEditTaskType()` function to infer edit task types from Stability AI model names (inpaint, outpaint, recolor, search-replace, erase-object, remove-bg, control-sketch, control-structure, style-guide, style-transfer, upscale-creative, upscale-conservative, upscale-fast)
- Created `ToStabilityAIImageEditRequest()` function to convert Bifrost requests to Stability AI's flat JSON format, with task-specific field validation
- Added `StabilityAIImageEditRequest` struct with comprehensive field support for all Stability AI edit operations
- Enhanced `BedrockImageGenerationResponse` with Seeds and FinishReasons fields for Stability AI compatibility
- Modified `ImageEdit()` method to route requests to appropriate conversion function based on model type
- Updated documentation to reflect expanded model support
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test with various Stability AI edit models through the Bedrock provider:
```sh
# Core/Transports
go version
go test ./...
# Test image editing with Stability AI models
# Example: stable-image-inpaint, stable-outpaint, stable-creative-upscale, etc.
```
Verify that task-specific parameters are correctly mapped and invalid fields are filtered out based on the detected task type.
## Screenshots/Recordings
N/A - Backend functionality only
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
Image data is handled as base64-encoded strings. Mask and image parameters are properly validated before processing.
## 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
* fix: send back accumulated usage in MCP agent mode (#2246)
## Summary
This PR fixes token usage tracking in MCP agent mode by accumulating usage across all LLM calls in the agent loop and returning the total usage in the final response.
## Changes
- Added usage accumulation logic in the MCP agent execution loop to track token consumption across multiple LLM calls
- Implemented `mergeUsage` function to combine token counts and costs from multiple `BifrostLLMUsage` values, handling all detail sub-fields including prompt tokens, completion tokens, and cost breakdowns
- Extended agent API adapters with `extractUsage` and `applyUsage` methods to handle usage extraction and application for both Chat API and Responses API
- Applied accumulated usage to the final response before returning it to the client
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test MCP agent mode with multiple tool calls to verify usage accumulation:
```sh
# Core/Transports
go version
go test ./...
# Test MCP agent mode with multiple LLM calls
# Verify that the returned usage reflects the sum of all calls in the agent loop
# Check that both token counts and cost details are properly accumulated
```
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications - this change only affects usage tracking and reporting.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* [codemode]: fixing string escape corruption, enable top-level control flow in starlark, refining the prompt of executecode tool (#2206)
## Changes
- **Enhanced Starlark dialect configuration**: Enabled top-level control flow statements (if/for/while), while loops, set() builtin, global variable reassignment, and recursive functions for a more Python-like experience
- **Improved string escape handling**: Removed automatic `\n` to newline conversion, allowing Starlark's native string escape processing to handle `\n`, `\t`, and other escape sequences correctly
- **Updated tool description**: Streamlined the executeToolCode tool description with clearer syntax notes, explicit documentation of Starlark differences from Python (no try/except, no classes, no imports, no f-strings), and emphasis on fresh isolated scope per execution
- **Enhanced error hints**: Added specific error messages for unsupported Python features like try/except/finally/raise, with guidance on alternative approaches and scope persistence warnings
- **Comprehensive test coverage**: Added tests for dialect options, string escape preservation, unsupported feature detection, and end-to-end JSON deserialization scenarios
## Type of change
- [ ] Feature
- [ ] Bug fix
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go) - Starlark CodeMode improvements
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test the enhanced Starlark features with MCP CodeMode:
```sh
# Test dialect options (top-level control flow, while loops, etc.)
make test-mcp TESTCASE=TestStarlarkDialectOptions
# Test string escape handling
make test-mcp PATTERN=TestStarlarkStringEscape
# Test unsupported feature detection
make test-mcp PATTERN=TestStarlarkUnsupportedFeatures
```
## Breaking changes
- [ ] Yes
- [x] No
The Starlark changes are additive and maintain backward compatibility while enabling more Python-like syntax.
## Security considerations
Starlark CodeMode maintains its existing sandboxing with no additional network or filesystem access. The dialect enhancements only affect language features within the existing security boundary.
* logging in plugins (#2215)
## Summary
Reorders middleware initialization in the Bifrost HTTP server to ensure tracing middleware is added before transport interceptor middleware in the inference pipeline.
## Changes
- Moved tracing middleware initialization and setup earlier in the bootstrap process
- Reordered middleware registration so tracing middleware is prepended before transport interceptor middleware
- Updated comments to clarify the middleware ordering logic and rationale
The change ensures that tracing context and trace IDs are properly established before other middleware components process requests.
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that tracing middleware executes before transport interceptor middleware by checking trace logs and middleware execution order.
```sh
# Core/Transports
go version
go test ./...
```
Test with tracing enabled to ensure trace IDs are properly set in context before subsequent middleware processing.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications - this is a middleware ordering change that affects observability components.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* fix: handling text, vtt, srt response format for transcriptions (#2102)
* feat: add virtual key access management for MCP clients (#2255)
## Summary
Adds virtual key access management to MCP client configuration, allowing administrators to control which virtual keys can access specific MCP servers and which tools they can execute on a per-VK basis.
## Changes
- Added `vk_configs` field to MCP client update API that accepts an array of virtual key configurations
- Each VK config specifies a virtual key ID and the tools it's allowed to execute on that MCP server
- When `vk_configs` is provided, it atomically replaces all existing VK assignments for the MCP client
- Added database method `GetVirtualKeyMCPConfigsByMCPClientID` to retrieve VK configs by MCP client
- Updated OpenAPI documentation to describe the new VK configuration functionality
- Enhanced UI with virtual key access management section in the MCP client sheet
- Added Go SDK context keys for MCP tool filtering: `MCPContextKeyIncludeClients`, `MCPContextKeyIncludeTools`, and `BifrostContextKeyMCPExtraHeaders`
- Updated context keys documentation with comprehensive MCP configuration examples
## Type of change
- [x] Feature
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] UI (Next.js)
- [x] Docs
## How to test
1. Create an MCP client with tools available
2. Create virtual keys in the system
3. Update the MCP client with VK configurations:
```sh
curl -X PUT /api/mcp/client/{id} \
-H "Content-Type: application/json" \
-d '{
"name": "test-client",
"vk_configs": [
{
"virtual_key_id": "vk-123",
"tools_to_execute": ["*"]
},
{
"virtual_key_id": "vk-456",
"tools_to_execute": ["read_file", "write_file"]
}
]
}'
```
4. Verify VK assignments are created/updated in the database
5. Test the UI by opening an MCP client sheet and managing virtual key access
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
## Screenshots/Recordings
The UI now includes a "Virtual Key Access" section in the MCP client configuration sheet where administrators can:
- Add virtual keys to grant access to the MCP server
- Configure which specific tools each virtual key can execute
- Remove virtual key access entirely
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
This enables fine-grained access control for MCP servers at the virtual key level, complementing the existing governance and budgeting features.
## Security considerations
- VK access controls are enforced through the governance plugin during MCP tool execution
- The atomic replacement of VK assignments prevents partial updates that could leave the system in an inconsistent state
- Tool-level restrictions allow principle of least privilege by limiting which MCP tools each virtual key can access
## 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
* feat: adds option to allow MCP clients to run on all virtual keys (#2258)
## Summary
Adds a new `AllowOnAllVirtualKeys` configuration option for MCP clients that enables them to be accessible to all virtual keys without requiring explicit per-key assignment. When enabled, all tools from the MCP client are available to every virtual key.
## Changes
- Added `AllowOnAllVirtualKeys` boolean field to `MCPClientConfig` schema and database table
- Updated MCP client manager to handle the new field during client updates
- Modified governance plugin to check for clients with `AllowOnAllVirtualKeys` enabled and automatically include their tools for all virtual keys
- Added database migration to add the new column to `TableMCPClient`
- Updated UI to include a toggle for the new setting with tooltip explanation
- Added OpenAPI documentation for the new field
- Updated configuration store methods to persist and retrieve the new field
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
1. Create or update an MCP client with `allow_on_all_virtual_keys: true`
2. Verify that the client's tools are available to all virtual keys without explicit assignment
3. Test that the governance plugin correctly allows tools from such clients
4. Verify the UI toggle works correctly in the MCP client edit sheet
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
The new configuration field `allow_on_all_virtual_keys` defaults to `false` to maintain backward compatibility.
## Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
## Breaking changes
- [ ] Yes
- [x] No
This is a backward-compatible addition with the new field defaulting to `false`.
## Related issues
Link related issues and discussions. Example: Closes #123
## Security considerations
This feature reduces access control granularity by allowing MCP clients to bypass virtual key restrictions when enabled. Administrators should carefully consider which MCP clients should have this permission as it grants broad access across all virtual keys.
## 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
* feat: add provider keys CRUD to configstore and in-memory store (#2159)
## Summary
Adds dedicated CRUD operations for individual provider keys at the data layer
(configstore interface + RDB implementation) and in-memory store. This enables
key-level operations without replacing the entire provider key set, which is
required for the new `/api/providers/{provider}/keys/*` endpoints.
## Changes
- Added `GetProviderKeys`, `GetProviderKey`, `CreateProviderKey`,
`UpdateProviderKey`, `DeleteProviderKey` to `ConfigStore` interface
- Implemented all five methods in `RDBConfigStore` with proper GORM queries,
error handling, and `ErrNotFound` propagation
- Extracted `schemaKeyFromTableKey` and `tableKeyFromSchemaKey` helpers to
deduplicate key conversion logic (previously inlined in `GetProvidersConfig`
and `GetProviderConfig`)
- Added `AddProviderKey`, `UpdateProviderKey`, `RemoveProviderKey` to in-memory
`Config` with mutex locking, DB persistence, and rollback on client update
failure
- Added `GetProviderKeysRaw`, `GetProviderKeysRedacted`, `GetProviderKeyRaw`,
`GetProviderKeyRedacted` read methods
- Added `TestProviderKeyCRUD` and `TestProviderKeyCRUD_ProviderMustExist`
integration tests
- Updated `MockConfigStore` with all five new interface methods
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
```sh
# Run configstore tests
go test ./framework/configstore/... -v -run TestProviderKeyCRUD
# Run config tests (mock store)
go test ./transports/bifrost-http/lib/... -v
```
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
Key values are handled through existing redaction infrastructure. No new secret
exposure paths introduced.
## Checklist
- [x] 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
* feat: add provider keys HTTP handlers and refactor optional keys (#2160)
## Summary
Adds HTTP handlers for the dedicated provider keys CRUD endpoints and removes
`keys` from provider API responses and payloads. Keys are now exclusively
managed via `/api/providers/{provider}/keys/*`. Also fixes a context timeout bug
in `ReloadProvider` where model discovery could exhaust the shared context
budget, causing subsequent DB calls to fail.
## Changes
### Provider keys handlers (`provider_keys.go`)
- New file with five handlers: `listProviderKeys`, `getProviderKey`,
`createProviderKey`, `updateProviderKey`, `deleteProviderKey`
- Includes `mergeUpdatedKey` (redacted value preservation logic used by
`updateProviderKey`)
- Key handlers enforce keyless provider validation and trigger model discovery
after mutations
### Provider handlers cleanup (`providers.go`)
- Registered new key routes: `GET/POST /api/providers/{provider}/keys`,
`GET/PUT/DELETE /api/providers/{provider}/keys/{key_id}`
- Extracted inline anonymous structs into named `providerCreatePayload` and
`providerUpdatePayload` types (without `Keys` field)
- Removed `Keys` field from `ProviderResponse`
- Switched `addProvider` from `json.Unmarshal` to `sonic.Unmarshal`
- Removed `oldConfigRedacted` fetch and the entire key merge block
(`mergeKeys`, `hasKeys`, `slices` usage) from `updateProvider`
- Removed `Keys` from `getProviderResponseFromConfig` response builder
- Removed unused `encoding/json` import
### Context timeout fix (`server.go`)
- Split shared `bfCtx` in `ReloadProvider` into separate contexts:
`filteredBfCtx` (15s) for filtered `ListModelsRequest` and `unfilteredBfCtx`
(fresh 15s) for unfiltered `ListModelsRequest`, each cancelled after use
- Changed `GetKeysByProvider` to use `context.Background()` since it's a local
DB call that shouldn't be gated by model discovery timeouts
- Added `hasNoKeys` check to emit warn-level logs instead of errors when model
discovery fails because no keys are configured
- Read in-memory key count via `GetProviderKeysRaw` for the `hasNoKeys` check
### Tests (`providers_test.go`)
- Cleared file (contained only tests for removed inline struct decoding)
## Type of change
- [x] Feature
- [x] Bug fix
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
```sh
# Build
go build ./transports/bifrost-http/...
# Manual: start Bifrost, then test key CRUD
curl -X POST localhost:8080/api/providers/openai/keys -d '{"name":"test-key","value":"sk-test"}'
curl localhost:8080/api/providers/openai/keys
curl -X PUT localhost:8080/api/providers/openai/keys/{key_id} -d '{"name":"updated","value":"sk-new"}'
curl -X DELETE localhost:8080/api/providers/openai/keys/{key_id}
# Verify provider endpoints no longer return keys
curl localhost:8080/api/providers/openai | jq 'has("keys")' # should be false
```
## Screenshots/Recordings
N/A
## Breaking changes
- [x] Yes
- [ ] No
Provider API responses no longer include `keys` field. Provider create/update
payloads no longer accept `keys`. Clients must use the dedicated
`/api/providers/{provider}/keys/*` endpoints for key management.
## Related issues
N/A
## Security considerations
- Key handlers use existing redaction infrastructure (`GetProviderKeyRedacted`)
before returning responses
- Keyless provider validation prevents key creation on providers that don't
support keys
## Checklist
- [x] 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
* feat: migrate frontend to dedicated provider keys API (#2161)
## Summary
Migrates the frontend from reading provider keys via `provider.keys` (removed
from provider API response in PR #2160) to the dedicated `getProviderKeys`
query and `/api/keys` endpoint. Removes `keys` from all provider TypeScript
types. Key mutations patch caches from authoritative server responses; provider
updates invalidate the `ProviderKeys` tag to refresh key statuses after model
discovery. Also adds a read-only routing rule info sheet.
## Changes
### Types (`config.ts`, `schemas.ts`)
- Removed `keys` field from `ModelProviderConfig`, `AddProviderRequest`, and
`UpdateProviderRequest`
- Added `CreateProviderKeyRequest`, `UpdateProviderKeyRequest`,
`ListProviderKeysResponse` types
### Store (`providersApi.ts`, `baseApi.ts`)
- Added `ProviderKeys` tag type to `baseApi`
- Changed `getProviderKeys`/`getProviderKey` from `Providers` tag to
`ProviderKeys` tag (avoids invalidating provider cache on key changes)
- Added `invalidatesTags: [ProviderKeys, DBKeys]` on `updateProvider` mutation
(refreshes key statuses after model discovery)
- Removed `getProvider`/`getProviders` cache patches from `createProviderKey`,
`updateProviderKey`, `deleteProviderKey` (providers no longer carry keys)
- Added duplicate-check guards on `createProviderKey` cache patches to prevent
ghost keys
- Each key mutation patches `getProviderKeys` and `getAllKeys` caches from
authoritative server response
### Components
- **`modelProviderKeysTableView.tsx`**: Already uses `useGetProviderKeysQuery`;
formatting/indentation fixes
- **`page.tsx`**: Removed `keys: []` from fallback provider object and
`createProvider` call; simplified `KeyDiscoveryFailedBadge` to only check
provider-level status (removed per-key status check since keys are no longer
on provider)
- **`routingRuleSheet.tsx`**: `TargetRow` now receives `allKeys` prop (from
`useGetAllKeysQuery`) instead of `providersData` with `.keys`; filters keys
by target provider
- **`routingRuleInfoSheet.tsx`**: New read-only sheet component that displays
routing rule details (conditions, targets with provider icons and weight bars,
fallback chain, scope, priority, timestamps)
- **`settingsPanel.tsx`**: Uses `useGetAllKeysQuery` to determine configured
providers (replaces `p.keys.length > 0` check) and derive
`providerKeyConfigs` per provider
### Other frontend changes (from prior commit, unchanged)
- Added `getProviderKeys`, `getProviderKey` RTK Query endpoints
- Added `createProviderKey`, `updateProviderKey`, `deleteProviderKey` mutations
- Added `buildProviderUpdatePayload` utility for key-free provider updates
- Migrated `providerKeyForm.tsx` to separate create/update mutations
- Updated `addNewKeySheet.tsx` props from `keyIndex` to `keyId`
- Updated all 6 provider form fragments to use `buildProviderUpdatePayload`
- Removed dead `selectedProvider.keys` sync matchers from `providerSlice.ts`
## Type of change
- [x] Feature
- [x] Refactor
- [ ] Bug fix
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
```sh
cd ui
npm run build
npm run lint
```
Manual testing:
1. Navigate to Providers page, select a provider with keys
2. Verify keys table loads correctly from dedicated API
3. Create a new key — verify it appears immediately (no ghost/duplicate)
4. Toggle enable/disable — verify switch updates immediately
5. Edit a key — verify form pre-populates, save works
6. Delete a key — verify it disappears immediately
7. Update provider settings — verify key statuses refresh after save
8. Check sidebar badge shows provider-level discovery failures
9. Open Playground settings — verify provider/key dropdowns work
10. Open Routing Rules — verify target key selector works
11. Click a routing rule row — verify info sheet opens with correct details
(conditions, targets, fallbacks, scope, priority)
## Screenshots/Recordings
N/A — no visual changes to existing features; routing rule info sheet is new.
## Breaking changes
- [ ] Yes
- [x] No
Frontend-only changes consuming the new API shape from PR #2160.
## Related issues
N/A
## Security considerations
No new security considerations. Key values continue to be handled through
existing redaction on the backend.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [ ] 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
* refactor: replace string slice with WhiteList type for model restrictions (#2282)
## Summary
Refactored model access control logic by replacing string slice with a dedicated `WhiteList` type for the `Models` field in `TableKey`. This change introduces a more structured approach to handling wildcard permissions and improves code readability.
## Changes
- Changed `Models` field type from `[]string` to `schemas.WhiteList` in `TableKey` struct
- Replaced manual wildcard checking (`model == "*"`) with `IsUnrestricted()` method calls across multiple functions
- Added missing mock method `GetVirtualKeyMCPConfigsByMCPClientIDs` to test configuration store
- Applied the refactoring consistently in `ReloadProvider`, `ForceReloadPricing`, and `Bootstrap` methods
## Type of change
- [x] Refactor
- [ ] Bug fix
- [ ] Feature
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that model access control continues to work correctly with both wildcard and specific model permissions:
```sh
# Core/Transports
go version
go test ./...
# Test specific areas affected by the changes
go test ./framework/configstore/tables/...
go test ./transports/bifrost-http/...
```
Test scenarios should include:
- Keys with wildcard permissions (`["*"]`)
- Keys with specific model restrictions
- Keys with empty model lists (deny-by-default behavior)
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
This refactoring maintains the existing security model for API key permissions. The deny-by-default behavior and wildcard functionality remain unchanged, just implemented through a more structured type system.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: add Plus icon and responsive text to pricing override create button (#2285)
## Summary
Improves the visual design and mobile responsiveness of the pricing overrides section by adding a Plus icon to the create button and optimizing the button text for different screen sizes.
## Changes
- Added Plus icon import from lucide-react
- Enhanced the "Create Override" button with a Plus icon and responsive text that shows "New Override" on larger screens and hides text on mobile
- Adjusted container spacing by removing top margin and changing flex alignment from `items-start` to `items-center` for better visual balance
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Navigate to the custom pricing overrides page and verify:
1. The "New Override" button displays with a Plus icon
2. On mobile screens, only the Plus icon is visible
3. On larger screens (sm and above), both icon and "New Override" text are visible
4. The button functionality remains unchanged when clicked
```sh
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
## Screenshots/Recordings
Before/after screenshots showing the button design changes and responsive behavior would be helpful.
## Breaking changes
- [x] Yes
- [ ] No
## Related issues
## Security considerations
No security implications - this is a purely visual enhancement.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* refactor: blacklist models on new convention (#2305)
## Summary
Implements comprehensive blacklist support for model filtering across all providers. This adds the ability to explicitly deny access to specific models at the key level, with blacklist rules taking precedence over allowlist rules.
## Changes
- Added `BlackList` type with semantic validation (supports wildcard "*" for block-all)
- Updated key selection logic to check both allowlist and blacklist constraints
- Modified all provider model listing functions to filter out blacklisted models
- Enhanced UI to support blacklist configuration with improved UX for wildcard selection
- Added blacklist filtering to model catalog and provider handlers
- Updated test cases to verify blacklist functionality
Key design decisions:
- Blacklist always wins over allowlist when conflicts occur
- Wildcard "*" in blacklist blocks all models for that key
- Empty blacklist blocks nothing (permissive default)
- Consistent filtering logic across all providers (Anthropic, Azure, Bedrock, Cohere, etc.)
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
Test blacklist functionality with provider keys:
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Example configuration to test:
```json
{
"keys": [{
"id": "test-key",
"models": ["*"],
"blacklisted_models": ["gpt-4", "claude-3"]
}]
}
```
Verify that blacklisted models are excluded from model listings and key selection.
## Screenshots/Recordings
UI now shows "Blocked Models" field with improved tooltips and wildcard handling for denying access to specific models.
## Breaking changes
- [ ] Yes
- [x] No
The `blacklisted_models` field was already present in the schema but not fully implemented. This change makes it functional without breaking existing configurations.
## Related issues
Enhances model access control capabilities for fine-grained permission management.
## Security considerations
Improves security by allowing explicit denial of access to sensitive or expensive models at the key level. Blacklist rules cannot be bypassed by allowlist configurations.
## 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
* minor fix add blacklisted model field in tableKeyFromSchemaKey (#2324)
## Summary
This PR adds support for the `BlacklistedModels` field when converting schema keys to table keys in the configuration store's RDB implementation.
## Changes
- Added `BlacklistedModels: key.BlacklistedModels` field mapping in the `tableKeyFromSchemaKey` function
- Ensures that blacklisted model information is properly preserved when converting between schema and table representations
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that configuration keys with blacklisted models are properly stored and retrieved from the RDB configstore.
```sh
# Core/Transports
go version
go test ./...
```
Test creating configuration entries with `BlacklistedModels` specified and ensure they persist correctly through the RDB layer.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
None - this change only adds field mapping for existing blacklisted models functionality.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: add image edit input view on logs (#2321)
## Summary
Adds support for logging image edit and image variation requests by introducing new database columns and UI components to track and display these image manipulation operations alongside existing image generation functionality.
## Changes
- Added `image_edit_input` and `image_variation_input` columns to the logs table with corresponding database migrations
- Extended the Log struct with new fields for storing and parsing image edit/variation input data
- Updated logging plugin to capture image edit and variation request data with large payload threshold handling
- Enhanced UI to display input images and prompts for image edit operations and input images for variation operations
- Added image MIME type detection for proper display of base64-encoded images in the UI
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [ ] Do…

Summary
Adds support for plain-text transcription response formats (text, srt, vtt) in OpenAI transcription requests. Previously, the system only handled JSON responses, but OpenAI's transcription API can return raw text depending on the
response_formatparameter.Changes
isPlainTextTranscriptionFormat()utility function to detect when transcription requests use plain-text formatsisPlainTextTranscriptionResponse()helper to identify plain-text responses by Content-TypeType of change
Affected areas
How to test
Test transcription requests with different response formats:
Screenshots/Recordings
N/A
Breaking changes
This change maintains backward compatibility while adding support for additional response formats.
Related issues
N/A
Security considerations
No security implications - this change only affects response format handling and preserves existing authentication and validation flows.
Checklist
docs/contributing/README.mdand followed the guidelines