Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion core/internal/mcptests/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func TestPlugin_MultiplePlugins(t *testing.T) {

// Setup Bifrost with multiple plugins in pipeline
bifrost, err := core.Init(context.Background(), schemas.BifrostConfig{
Account: &testAccount{},
Account: &testAccount{},
MCPPlugins: []schemas.MCPPlugin{
loggingPlugin,
modifyPlugin,
Expand Down Expand Up @@ -725,3 +725,30 @@ func TestPlugin_CustomTestPlugin(t *testing.T) {

t.Logf("✅ Custom test plugin test completed successfully")
}

func TestPlugin_PreMCPHookShortCircuitsBeforeToolResolution(t *testing.T) {
t.Parallel()

manager := setupMCPManager(t)
shortCircuitPlugin := NewTestShortCircuitPlugin()
shortCircuitPlugin.SetShouldShortCircuit(true)
shortCircuitPlugin.SetShortCircuitMessage("Blocked before tool resolution")

bifrost, err := core.Init(context.Background(), schemas.BifrostConfig{
Account: &testAccount{},
MCPPlugins: []schemas.MCPPlugin{shortCircuitPlugin},
Logger: core.NewDefaultLogger(schemas.LogLevelInfo),
})
require.NoError(t, err)
bifrost.SetMCPManager(manager)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

ctx := createTestContext()
missingToolCall := CreateToolCall("missing-tool", "missing-client-echo", map[string]interface{}{"message": "test"})
result, bifrostErr := bifrost.ExecuteChatMCPTool(ctx, &missingToolCall)

require.Nil(t, bifrostErr)
require.NotNil(t, result)
require.NotNil(t, result.Content)
require.NotNil(t, result.Content.ContentStr)
assert.Contains(t, *result.Content.ContentStr, "Blocked before tool resolution")
}
44 changes: 15 additions & 29 deletions core/mcp/exec.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package mcp

import (
"errors"
"fmt"
"strings"
"sync"
Expand Down Expand Up @@ -82,37 +81,24 @@ func (m *MCPManager) executeToolWithHooks(
}
}

// Resolve the upstream client and acquire its connection BEFORE the plugin
// gate runs. Connection lifecycle is the orchestrator's concern, not the
// plugin op's — the plugin pipeline only wraps the actual CallTool. When
// AcquireClientConn fails (e.g. *MCPAuthRequiredError for per-user
// clients that need re-auth or headers submission), the plugin gate is
// never invoked.
state, conn, release, prepErr := m.prepareToolExecution(ctx, request)
if prepErr != nil {
bErr := &schemas.BifrostError{
IsBifrostError: false,
Error: &schemas.ErrorField{Message: prepErr.Error()},
ExtraFields: schemas.BifrostErrorExtraFields{RequestType: requestType, MCPRequestType: request.RequestType},
}
var authRequiredErr *schemas.MCPAuthRequiredError
if errors.As(prepErr, &authRequiredErr) {
bErr.ExtraFields.MCPAuthRequired = authRequiredErr
resp, bErr := m.RunWithPluginPipeline(ctx, request, func(preReq *schemas.BifrostMCPRequest) (*schemas.BifrostMCPResponse, error) {
// Resolve and acquire after PreMCPHook so policy plugins can reject denied
// MCP clients before any auth or transport work.
state, conn, release, prepErr := m.prepareToolExecution(ctx, preReq)
if prepErr != nil {
return nil, prepErr
}
return nil, bErr
}
defer release()
defer release()

// state == nil signals a code-mode tool: pass nil conn/config/mapping and
// ToolsManager.ExecuteTool routes directly to CodeMode.
var executionConfig *schemas.MCPClientConfig
var toolNameMapping map[string]string
if state != nil {
executionConfig = state.ExecutionConfig
toolNameMapping = state.ToolNameMapping
}
// state == nil signals a code-mode tool: pass nil conn/config/mapping and
// ToolsManager.ExecuteTool routes directly to CodeMode.
var executionConfig *schemas.MCPClientConfig
var toolNameMapping map[string]string
if state != nil {
executionConfig = state.ExecutionConfig
toolNameMapping = state.ToolNameMapping
}

resp, bErr := m.RunWithPluginPipeline(ctx, request, func(preReq *schemas.BifrostMCPRequest) (*schemas.BifrostMCPResponse, error) {
result, opErr := m.toolsManager.ExecuteTool(ctx, preReq, conn, executionConfig, toolNameMapping)
if opErr != nil {
return nil, opErr
Expand Down
12 changes: 11 additions & 1 deletion core/mcp/pluginpipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package mcp

import (
"context"
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -120,11 +121,16 @@ func (m *MCPManager) RunWithPluginPipeline(
if pipeline == nil {
resp, opErr := op(req)
if opErr != nil {
return resp, &schemas.BifrostError{
bErr := &schemas.BifrostError{
IsBifrostError: false,
Error: &schemas.ErrorField{Message: opErr.Error()},
ExtraFields: schemas.BifrostErrorExtraFields{MCPRequestType: mcpReqType},
}
var authRequiredErr *schemas.MCPAuthRequiredError
if errors.As(opErr, &authRequiredErr) {
bErr.ExtraFields.MCPAuthRequired = authRequiredErr
}
return resp, bErr
}
return resp, nil
}
Expand Down Expand Up @@ -196,6 +202,10 @@ func (m *MCPManager) RunWithPluginPipeline(
Error: &schemas.ErrorField{Message: opErr.Error()},
ExtraFields: schemas.BifrostErrorExtraFields{MCPRequestType: mcpReqType},
}
var authRequiredErr *schemas.MCPAuthRequiredError
if errors.As(opErr, &authRequiredErr) {
bErr.ExtraFields.MCPAuthRequired = authRequiredErr
}
}

finalResp, finalErr := pipeline.RunMCPPostHooks(ctx, resp, bErr, preCount)
Expand Down
1 change: 1 addition & 0 deletions core/schemas/bifrost.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ const (
BifrostContextKeyParentMCPRequestID BifrostContextKey = "bf-parent-mcp-request-id" // string (parent request ID for nested tool calls from executeCode)
BifrostContextKeyStructuredOutputToolName BifrostContextKey = "bifrost-structured-output-tool-name" // string (to store the name of the structured output tool (set by bifrost))
BifrostContextKeyUserAgent BifrostContextKey = "bifrost-user-agent" // string (set by bifrost)
BifrostContextKeyApp BifrostContextKey = "app" // string (canonical app key such as claude-code; set by plugins)
Comment thread
greptile-apps[bot] marked this conversation as resolved.
BifrostContextKeySkipBudgetAndRateLimits BifrostContextKey = "bifrost-skip-budget-and-rate-limits" // bool (set by bifrost for read-only requests like list models that don't consume quota)
BifrostContextKeySkipVirtualKeyUsageTracking BifrostContextKey = "bifrost-skip-virtual-key-usage-tracking" // bool (set by governance callers to skip VK usage while preserving VK auth/attribution)
BifrostContextKeyTraceID BifrostContextKey = "bifrost-trace-id" // string (trace ID for distributed tracing - set by tracing middleware)
Expand Down
9 changes: 9 additions & 0 deletions core/schemas/useragents.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,15 @@ func DetectAppFromUserAgent(userAgent string) string {
return UserAgentAppOther
}

// AppKeyFromName returns the canonical policy key for a detected app name.
func AppKeyFromName(name string) string {
name = strings.TrimSpace(name)
if name == "" || name == UserAgentAppOther {
return ""
}
return strings.Join(strings.Fields(strings.ToLower(name)), "-")
}

// MatchUserAgent reports whether a User-Agent matches a pattern using the given match type.
// An empty matchType defaults to UserAgentMappingMatchTypeContains.
//
Expand Down
23 changes: 23 additions & 0 deletions core/schemas/useragents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ func TestDetectAppFromUserAgent(t *testing.T) {
{name: "fasthttp api", userAgent: "fasthttp", want: "API"},
{name: "codex cli", userAgent: "codex-cli/0.1.0", want: "Codex CLI"},
{name: "codex tui", userAgent: "codex-tui/0.1.0", want: "Codex CLI"},
{name: "codex tui terminal capture", userAgent: "codex-tui/0.137.0 (Mac OS 14.1.0; arm64) iTerm.app/3.6.6 (codex-tui; 0.137.0)", want: "Codex CLI"},
{name: "claude cowork runtime capture", userAgent: "claude-cli/2.1.170 (external, local-agent, agent-sdk/0.3.170)", want: "Claude Code"},
{name: "codex desktop mac", userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) Codex/1.0 Electron/41.0", want: "Codex Desktop"},
{name: "codex desktop windows", userAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64) Codex/1.0 Electron/41.0", want: "Codex Desktop"},
{name: "codex desktop linux", userAgent: "Mozilla/5.0 (X11; Linux x86_64) Codex/1.0 Electron/41.0", want: "Codex Desktop"},
Expand All @@ -39,6 +41,27 @@ func TestDetectAppFromUserAgent(t *testing.T) {
}
}

func TestAppKeyFromName(t *testing.T) {
tests := []struct {
name string
in string
want string
}{
{name: "claude code", in: "Claude Code", want: "claude-code"},
{name: "custom app", in: " Internal Claude Wrapper ", want: "internal-claude-wrapper"},
{name: "collapsed spaces", in: "Gemini CLI", want: "gemini-cli"},
{name: "other ignored", in: UserAgentAppOther, want: ""},
{name: "empty ignored", in: "", want: ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := AppKeyFromName(tt.in); got != tt.want {
t.Fatalf("AppKeyFromName(%q) = %q, want %q", tt.in, got, tt.want)
}
})
}
}

func TestMatchUserAgent(t *testing.T) {
tests := []struct {
name string
Expand Down
21 changes: 2 additions & 19 deletions plugins/logging/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,23 +582,6 @@ func userAgentFromContext(ctx *schemas.BifrostContext) string {
return ua
}

// appFromContext returns the canonical app key from the request header map.
func appFromContext(ctx *schemas.BifrostContext) string {
allHeaders, _ := ctx.Value(schemas.BifrostContextKeyRequestHeaders).(map[string]string)
if allHeaders == nil {
return ""
}
if app := strings.TrimSpace(allHeaders["x-bf-app"]); app != "" {
return app
}
for key, value := range allHeaders {
if strings.EqualFold(key, "x-bf-app") && strings.TrimSpace(value) != "" {
return strings.TrimSpace(value)
}
}
return ""
}

// captureLoggingHeaders extracts configured logging headers and x-bf-lh-* prefixed headers
// from the request context. Returns a new metadata map, or nil if no headers were captured.
// System entries (e.g. isAsyncRequest) should be set AFTER calling this so they take precedence.
Expand Down Expand Up @@ -706,8 +689,8 @@ func (p *LoggerPlugin) PreLLMHook(ctx *schemas.BifrostContext, req *schemas.Bifr
// maps it to a client app such as Claude Code, Codex, or Cursor).
initialData.UserAgent = userAgentFromContext(ctx)
initialData.App = p.detectAppFromUserAgent(initialData.UserAgent)
if app := appFromContext(ctx); app != "" {
initialData.App = app
if appKey := schemas.AppKeyFromName(initialData.App); appKey != "" {
ctx.SetValue(schemas.BifrostContextKeyApp, appKey)
}

if p.contentLoggingEnabled(ctx) {
Expand Down
124 changes: 124 additions & 0 deletions plugins/logging/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,87 @@ func TestUserAgentFromContextFallsBackToUserAgentKey(t *testing.T) {
}
}

func TestPreLLMHookSetsAppContextFromDetectedApp(t *testing.T) {
store := newTestStore(t)
defer store.Close(context.Background())
plugin, err := Init(context.Background(), &Config{}, testLogger{}, store, nil, nil)
if err != nil {
t.Fatalf("Init() error = %v", err)
}
t.Cleanup(func() {
if cleanupErr := plugin.Cleanup(); cleanupErr != nil {
t.Errorf("Cleanup() error = %v", cleanupErr)
}
})
ctx := schemas.NewBifrostContext(context.Background(), schemas.NoDeadline)
ctx.SetValue(schemas.BifrostContextKeyRequestID, "req-app-context")
ctx.SetValue(schemas.BifrostContextKeyRequestHeaders, map[string]string{
"user-agent": "claude-cli/2.1.168 (external, cli)",
"x-bf-vk": "vk-test",
"x-bf-user-id": "user-test",
})

_, _, err = plugin.PreLLMHook(ctx, &schemas.BifrostRequest{
RequestType: schemas.ChatCompletionRequest,
ChatRequest: &schemas.BifrostChatRequest{
Provider: schemas.OpenAI,
Model: "gpt-4o-mini",
Params: &schemas.ChatParameters{},
},
})
if err != nil {
t.Fatalf("PreLLMHook() error = %v", err)
}
if got, _ := ctx.Value(schemas.BifrostContextKeyApp).(string); got != "claude-code" {
t.Fatalf("app context = %q, want claude-code", got)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// TestPreLLMHookContextKeyComesFromUserAgentNotAgentHeader replays the header
// shape the desktop agent stamps for a Claude Cowork request: Cowork's runtime
// shares Claude Code's CLI User-Agent, so the logging plugin derives
// claude-code for the context key. Header-based app enforcement (X-Bf-App)
// lives in the enterprise agent policy plugin, which prefers the header over
// this context key.
func TestPreLLMHookContextKeyComesFromUserAgentNotAgentHeader(t *testing.T) {
store := newTestStore(t)
defer store.Close(context.Background())
plugin, err := Init(context.Background(), &Config{}, testLogger{}, store, nil, nil)
if err != nil {
t.Fatalf("Init() error = %v", err)
}
t.Cleanup(func() {
if cleanupErr := plugin.Cleanup(); cleanupErr != nil {
t.Errorf("Cleanup() error = %v", cleanupErr)
}
})
ctx := schemas.NewBifrostContext(context.Background(), schemas.NoDeadline)
ctx.SetValue(schemas.BifrostContextKeyRequestID, "req-cowork-context")
// Captured from the agent's MITM → GATEWAY forwarding of a Cowork request.
ctx.SetValue(schemas.BifrostContextKeyRequestHeaders, map[string]string{
"user-agent": "claude-cli/2.1.170 (external, local-agent, agent-sdk/0.3.170)",
"anthropic-client-platform": "desktop_app",
"x-app": "cli",
"x-bf-app": "claude-cowork",
"x-bifrost-agent": "bifrost-agent/0.1.0",
})

_, _, err = plugin.PreLLMHook(ctx, &schemas.BifrostRequest{
RequestType: schemas.ChatCompletionRequest,
ChatRequest: &schemas.BifrostChatRequest{
Provider: schemas.Anthropic,
Model: "claude-opus-4-8",
Params: &schemas.ChatParameters{},
},
})
if err != nil {
t.Fatalf("PreLLMHook() error = %v", err)
}
if got, _ := ctx.Value(schemas.BifrostContextKeyApp).(string); got != "claude-code" {
t.Fatalf("app context = %q, want claude-code (derived from User-Agent)", got)
}
}

func TestCustomUserAgentMappingOverridesBuiltInDetection(t *testing.T) {
store := newTestStore(t)
defer store.Close(context.Background())
Expand Down Expand Up @@ -124,6 +205,49 @@ func TestCustomUserAgentMappingOverridesBuiltInDetection(t *testing.T) {
}
}

func TestPreLLMHookSetsAppContextFromCustomMapping(t *testing.T) {
store := newTestStore(t)
defer store.Close(context.Background())
plugin, err := Init(context.Background(), &Config{}, testLogger{}, store, nil, nil)
if err != nil {
t.Fatalf("Init() error = %v", err)
}
t.Cleanup(func() {
if cleanupErr := plugin.Cleanup(); cleanupErr != nil {
t.Errorf("Cleanup() error = %v", cleanupErr)
}
})
_, err = plugin.CreateUserAgentMapping(context.Background(), &logstore.UserAgentMapping{
Pattern: `custom-wrapper/\d+`,
MatchType: string(schemas.UserAgentMappingMatchTypeRegex),
App: "Internal Claude Wrapper",
IsActive: true,
})
if err != nil {
t.Fatalf("CreateUserAgentMapping() error = %v", err)
}
ctx := schemas.NewBifrostContext(context.Background(), schemas.NoDeadline)
ctx.SetValue(schemas.BifrostContextKeyRequestID, "req-custom-app-context")
ctx.SetValue(schemas.BifrostContextKeyRequestHeaders, map[string]string{
"user-agent": "custom-wrapper/42",
})

_, _, err = plugin.PreLLMHook(ctx, &schemas.BifrostRequest{
RequestType: schemas.ChatCompletionRequest,
ChatRequest: &schemas.BifrostChatRequest{
Provider: schemas.OpenAI,
Model: "gpt-4o-mini",
Params: &schemas.ChatParameters{},
},
})
if err != nil {
t.Fatalf("PreLLMHook() error = %v", err)
}
if got, _ := ctx.Value(schemas.BifrostContextKeyApp).(string); got != "internal-claude-wrapper" {
t.Fatalf("app context = %q, want internal-claude-wrapper", got)
}
}

func TestPostLLMHookNoPendingErrorPreservesMetadata(t *testing.T) {
store := newTestStore(t)
loggingHeaders := []string{"x-custom-log"}
Expand Down
3 changes: 3 additions & 0 deletions transports/bifrost-http/handlers/mcpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ func (h *MCPServerHandler) syncServer(server *server.MCPServer, availableTools [
toolName := tool.Function.Name

handler := func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
logger.Info("[mcp-server] tool handler start tool=%q arg_count=%d", toolName, len(request.GetArguments()))
// Inject tool filter into execution context if present
if toolFilter != nil {
ctx = context.WithValue(ctx, schemas.MCPContextKeyIncludeTools, toolFilter)
Expand All @@ -354,6 +355,7 @@ func (h *MCPServerHandler) syncServer(server *server.MCPServer, availableTools [
// Execute the tool via tool executor
toolMessage, err := h.toolManager.ExecuteChatMCPTool(ctx, &toolCall)
if err != nil {
logger.Info("[mcp-server] tool handler error tool=%q error=%s", toolName, bifrost.GetErrorMessage(err))
if authReq := err.ExtraFields.MCPAuthRequired; authReq != nil {
// Two surfaces share this error: per-user OAuth uses
// AuthorizeURL (the upstream provider's authorize page);
Expand All @@ -373,6 +375,7 @@ func (h *MCPServerHandler) syncServer(server *server.MCPServer, availableTools [
}
return mcp.NewToolResultError(fmt.Sprintf("Tool execution failed: %v", bifrost.GetErrorMessage(err))), nil
}
logger.Info("[mcp-server] tool handler success tool=%q", toolName)

// Extract content from tool message
var resultText string
Expand Down
Loading
Loading