Skip to content

feat: add dynamic plugin loading via configuration files#134

Draft
Pratham-Mishra04 wants to merge 147 commits intomainfrom
06-27-feat_go_plugins_implemented_for_dynamic_plugin_loading
Draft

feat: add dynamic plugin loading via configuration files#134
Pratham-Mishra04 wants to merge 147 commits intomainfrom
06-27-feat_go_plugins_implemented_for_dynamic_plugin_loading

Conversation

@Pratham-Mishra04
Copy link
Copy Markdown
Collaborator

Dynamic Plugin Loading for Bifrost

This PR adds support for dynamic plugin loading via configuration files, enabling more flexible plugin management without code changes. Key features include:

  • Added standardized Init(config json.RawMessage) (schemas.Plugin, error) function signature for all plugins
  • Implemented Just-In-Time (JIT) compilation for both remote and local plugins
  • Added configuration-based plugin loading through the plugins section in config.json
  • Updated existing plugins (maxim, mocker) to support dynamic loading
  • Enhanced documentation with detailed plugin development guidelines
  • Added environment variable substitution in plugin configurations
  • Ensured dependency version compatibility between main app and plugins
  • Removed direct dependency on maxim plugin in HTTP transport

The implementation supports two plugin types:

  1. Remote plugins: Go modules hosted on public repositories
  2. Local plugins: Directories containing Go plugin source code

This change maintains backward compatibility with the traditional plugin instantiation approach while providing a more flexible configuration-driven alternative.

akshaydeo and others added 30 commits April 30, 2025 11:37
feat: add system message support for Anthropic provider and minor fixes
…dded

docs: add quickstart guide with HTTP server and Go package examples
feat: add retryable status codes for error handling
# Add Google Vertex AI Provider Support

This PR adds support for Google Vertex AI as a new provider in Bifrost, allowing users to access Google's AI models like Gemini and third-party models hosted on Vertex AI.

Key changes:
- Implemented a new `VertexProvider` that supports chat completion with Google's models
- Added authentication via Google Cloud credentials
- Created region-specific model routing for Vertex AI endpoints
- Refactored provider code to extract common request preparation logic
- Added support for both OpenAI-style and Anthropic-style models on Vertex
- Enhanced error handling and debugging for all providers

The implementation uses Google's OAuth2 authentication and doesn't require API keys like other providers, instead using service account credentials. The PR also updates the gitignore to exclude credential files for security.
…ort (#30)

# Add Google Gemini API Compatibility Layer

This PR adds support for the Google Gemini API by implementing a compatibility layer that allows Bifrost to handle Gemini API requests. The implementation includes:

- A new `/genai/v1beta/models/{model}` endpoint that accepts Gemini API requests
- Conversion utilities to transform between Gemini and Bifrost request/response formats
- Support for Vertex AI provider configuration in the example config file
- An extensible router architecture to allow for additional API compatibility layers

The implementation handles Gemini chat requests, including proper conversion of message formats, tool calls, and function calls between the two APIs. This allows applications built for Gemini to use Bifrost as a drop-in replacement without code changes.
# Move Vertex region from model parameters to meta configuration

This PR moves the Vertex region configuration from model parameters to the provider's meta configuration. This change improves the architecture by placing the region where it logically belongs - in the provider-specific meta configuration rather than in the general model parameters.

Key changes:
- Added `Region` field to `VertexMetaConfig` struct
- Implemented `GetRegion()` method for `VertexMetaConfig` to return the region
- Updated error message to reflect that region should be set in meta config
- Updated tests and example configuration to use the new region location
- Removed region parameter from Gemini chat request conversion
- Updated the core dependency in transports to v1.0.5

This change ensures that Vertex API region configuration is properly managed at the provider level rather than being passed with each model request.
# Add Prometheus Metrics and Docker Container Naming

This PR adds Prometheus metrics tracking to the Bifrost HTTP service and improves Docker container configuration. The changes include:

1. Added Prometheus metrics collection for:
   - HTTP server metrics (request counts, latency, sizes)
   - Upstream provider metrics (request counts, latency, errors)

2. Implemented a Prometheus plugin that hooks into Bifrost's request lifecycle to track provider-specific metrics.

3. Added Docker Compose configuration for Prometheus and Grafana monitoring stack.

4. Refactored the codebase by moving account and configuration management to a dedicated `lib` package.

5. Updated the Docker run command in the README to include container naming (`--name bifrost-api`), making it easier to reference in monitoring configurations.

6. Improved code documentation with detailed comments explaining the purpose and functionality of each component.

These changes enable comprehensive monitoring of the Bifrost API service, providing insights into both HTTP server performance and upstream provider metrics.
# Add function call support to Gemini integration

This PR enhances the Gemini integration by adding support for function calls in the request conversion process. When a Gemini request contains a function call, it's now properly converted to the Bifrost format with appropriate tool calls.

Key changes:
- Renamed `main.go` to `types.go` to better reflect its content
- Added handling for function calls in the message conversion logic
- Implemented proper mapping of function call parameters
- Added a utility `StrPtr` function for string pointer conversion
- Updated the Bifrost core dependency to v1.0.5

This change ensures that function calls in Gemini requests are properly processed and converted to the Bifrost format, enabling more advanced interactions with the Gemini API.
# Add support for Maxim Logger plugin

This PR adds support for the Maxim Logger plugin in the Bifrost HTTP transport. The implementation includes:

- Added new command line arguments for plugin configuration:
  - `-plugins`: Comma-separated list of plugins to load
  - `-maxim-logger-id`: ID for the Maxim logger

- Updated the Dockerfile to support these new arguments with environment variables:
  - `APP_PLUGINS`
  - `APP_MAXIM_LOGGER_ID`

- Implemented plugin loading logic that:
  - Parses the plugins string into a list
  - Handles the "maxim" plugin specifically, checking for required configuration
  - Requires `MAXIM_API_KEY` environment variable for the Maxim plugin

- Added the Maxim plugin dependency to go.mod

The Prometheus plugin is now included in the loadedPlugins list alongside any other enabled plugins.
# Reorganized Documentation Structure and Plugin System

This PR reorganizes the documentation structure and improves the plugin system organization. Key changes include:

1. Added a dedicated `docs/` directory with comprehensive documentation for:
   - Memory management and concurrency settings
   - Logger configuration and customization
   - Plugin system architecture and development guidelines
   - Provider configuration options and development guidelines
   - Fallback system implementation and best practices

2. Restructured the plugins directory to use a more modular approach:
   - Changed from a single file (`maxim-logger.go`) to a directory structure (`maxim/`)
   - This enables better organization of plugin code and dependencies

3. Updated the README.md to:
   - Include the new docs directory in the project structure
   - Replace inline documentation with links to dedicated documentation files
   - Update the plugin import instructions to reflect the new structure
   - Enhance the contribution guidelines with more specific instructions

These changes make the documentation more maintainable and accessible while providing a clearer structure for plugin development and contribution.
# Add JSON tags to Bifrost schemas and enhance Maxim SDK logging

This PR enhances the Bifrost codebase in two key areas:

1. Adds JSON struct tags to the Bifrost request and related schemas to ensure proper serialization/deserialization.

2. Significantly improves the Maxim SDK logging plugin:
   - Replaces timestamp-based trace IDs with UUID-based identifiers
   - Adds structured logging for different request types (chat vs. text completion)
   - Captures detailed information about requests including model parameters
   - Implements proper generation tracking with separate generation IDs
   - Adds proper trace and generation lifecycle management (ending traces and flushing logs)
   - Extracts and logs message content in a more structured way

The PR also updates the plugins module dependencies to include the Google UUID package.
# Refactor Maxim Logger Plugin for Better Reusability

This PR refactors the Maxim logger plugin to improve its reusability across the codebase:

- Changed the Maxim plugin from `package main` to `package maxim` to make it importable
- Exported key types and constants to make them accessible from other packages:
  - Renamed `contextKey` to `ContextKey` and made it public
  - Renamed and exported `traceIDKey` to `TraceIDKey` and `generationIDKey` to `GenerationIDKey`
  - Updated the key string values to use kebab-case format

- Moved context conversion functionality from `tracking/utils.go` to a new `lib/ctx.go` file
- Enhanced `ConvertToBifrostContext` to handle Maxim-specific headers (x-bf-maxim-*)
- Updated imports throughout the codebase to reference the new package structure
- Made `PrometheusContextKey` public in the tracking package for cross-package usage

These changes enable better integration between the HTTP transport and Maxim plugin by allowing trace and generation IDs to be passed through HTTP headers.
# Add Default Network and Concurrency Configurations

This PR introduces default configurations for network settings and concurrency parameters to simplify provider configuration:

- Added `DefaultNetworkConfig` and `DefaultConcurrencyAndBufferSize` constants in the provider schema
- Updated the `BaseAccount.GetConfigForProvider` implementation to use these default configurations
- Simplified provider configuration in test accounts
- Improved plugin usage documentation in README with correct import paths
- Added documentation for Prometheus support and plugin integration in the transports README

These changes make it easier to configure providers with sensible defaults while still allowing for customization when needed.
…ex provider (#38)

# Use Vertex credentials directly instead of reading from file

This PR changes the Vertex provider to accept authentication credentials directly as a string rather than reading them from a file path. This simplifies configuration and deployment by:

1. Replacing `auth_credential_path` with `auth_credentials` in the Vertex meta configuration
2. Modifying the provider interface to use `GetAuthCredentials()` instead of `GetAuthCredentialPath()`
3. Updating the Vertex provider to use the credentials directly instead of reading from a file
4. Updating test configurations and example config to use the new approach

This change allows credentials to be passed directly as environment variables or configuration values without requiring file system access.
akshaydeo and others added 22 commits June 21, 2025 16:38
### TL;DR

Updated GitHub Actions workflow to trigger on tag pushes instead of branch pushes.

### What changed?

Modified the `transport-ci.yml` workflow to:
- Remove the branch trigger for `test/gh-actions`
- Uncomment and enable tag-based triggers for `core/v*` and `transports/v*`

### How to test?

1. Push a tag with the format `core/v*` to verify dependency update is triggered
2. Push a tag with the format `transports/v*` to verify docker build is triggered

### Why make this change?

This change moves the workflow from a testing phase to production usage, ensuring that the CI pipeline only runs when specific version tags are pushed rather than on branch commits. This provides better control over when dependency updates and Docker builds are triggered.
# Improve Docker build process for bifrost transports

This PR refines the Docker build process for bifrost transports by:

- Removing unnecessary GOPATH environment variable and echo statement
- Replacing the `go install` approach with a proper module initialization and build process
- Creating a local go.mod file with `go mod init bifrost-build`
- Using `go build` with output directly to the target location instead of moving binaries after installation
- Maintaining the same optimization flags and UPX compression for the final binary

These changes provide a cleaner, more explicit build process while maintaining the same functionality.
…#113)

# Update MCP Client Library to mark3labs/mcp-go v0.32.0

This PR updates the MCP client library from `metoro-io/mcp-golang` v0.13.0 to `mark3labs/mcp-go` v0.32.0, which brings significant improvements to the MCP integration in Bifrost.

## Key Changes

- Added support for Server-Sent Events (SSE) connections alongside existing HTTP and STDIO connections
- Improved connection management with proper context handling and cleanup
- Streamlined MCP client initialization and tool registration
- Removed dependency on local HTTP server port for tool hosting
- Updated documentation with examples for all connection types
- Simplified dependency tree by removing several unnecessary packages

## Implementation Details

- Refactored `MCPManager` to use the new client library's API
- Updated connection types to include SSE alongside HTTP and STDIO
- Renamed `HTTPConnectionString` to `ConnectionString` for both HTTP and SSE connections
- Enhanced error handling and logging for all connection types
- Added proper cleanup for all connection types including SSE context cancellation
- Updated example code and documentation to reflect the new API

This update provides better performance, more connection options, and improved reliability for MCP tool integration.
…docs enhancement (#105)

# Enhanced Plugin System with Short-Circuit Control and Fallback Management

This PR introduces a significant enhancement to the Bifrost plugin system, providing more control over request flow and error handling. The key improvements include:

1. **New PluginShortCircuit Type**: Replaces direct response returns with a structured approach that allows plugins to return either a response or an error when short-circuiting.

2. **Fallback Control Mechanism**: Adds an `AllowFallbacks` field to `BifrostError`, giving plugins explicit control over whether fallback providers should be attempted when errors occur:
   - `AllowFallbacks = &true`: Try fallback providers
   - `AllowFallbacks = &false`: Return error immediately, no fallbacks
   - `AllowFallbacks = nil`: Default to allowing fallbacks for resilience

3. **Improved Error Handling**: The system now properly processes short-circuit errors and respects fallback preferences in both text and chat completion flows.

4. **Comprehensive Documentation**: Completely rewrites the plugin documentation with detailed explanations, diagrams, examples, and best practices for plugin development.

These changes enable more sophisticated plugin behaviors such as:
- Authentication plugins that can block requests without trying fallbacks
- Rate limiting plugins that allow fallbacks to other providers
- Caching plugins that can return cached responses directly

The implementation maintains backward compatibility while providing plugin developers with greater control over the request pipeline.
…onses (#106)

# Add Mocker Plugin for Bifrost

This PR introduces a new Mocker plugin for Bifrost that allows intercepting and mocking AI provider responses. The plugin provides flexible rule-based mocking capabilities for testing, development, and simulation purposes.

## Features

- Rule-based mocking with priority ordering and probability controls
- Support for custom success and error responses
- Template-based responses with variable substitution
- Configurable latency simulation
- Comprehensive statistics tracking
- High-performance design optimized for benchmarking

## Implementation Details

- Added core plugin implementation with pre-compiled regex patterns for performance
- Implemented weighted response selection for simulating varied behaviors
- Added support for provider/model matching and message content regex matching
- Included detailed validation for configuration parameters
- Added atomic counters for high-performance statistics tracking
- Created comprehensive test suite with various mocking scenarios

## Documentation

- Added detailed README with configuration reference, examples, and best practices
- Included performance optimization guidelines and troubleshooting tips
- Provided comprehensive API documentation for all configuration options

The plugin is designed to be lightweight when disabled and highly performant for benchmarking scenarios, with minimal overhead in the no-match path.
# Update Bifrost Core to v1.1.5

This PR updates the Bifrost Core dependency from v1.1.4 to v1.1.5 across multiple test modules and transports. The update includes:

- Replaced `metoro-io/mcp-golang` with `mark3labs/mcp-go v0.32.0`
- Added new dependencies like `google/uuid`, `spf13/cast`, and `yosida95/uritemplate/v3`
- Removed several unused dependencies including Gin framework components
- Updated the `PrometheusPlugin.PreHook` method signature in `tracking/plugin.go` to use `*schemas.PluginShortCircuit` instead of `*schemas.BifrostResponse`
- Updated the Maxim plugin from v1.0.5 to v1.0.6

These changes align the codebase with the latest core functionality while reducing dependency bloat.
Fixed conditional logic for Docker build job to ensure it runs when the dependency update job is skipped but a valid tag exists. The updated condition checks for either a successful dependency update OR a combination of a skipped update with a valid transport version tag.
# Improved MCP Tool Handling and Logging

This PR makes several improvements to the MCP (Multi-Client Protocol) manager:

1. Changed tool execution logging from `Info` to `Debug` level to reduce log verbosity
2. Simplified the `convertMCPToolToBifrostSchema` function by directly using properties from the MCP tool schema
3. Enhanced `extractTextFromMCPResponse` to properly handle different content types:
   - Text content
   - Image content
   - Audio content
   - Embedded resources
   - Fallback handling for unknown content types

The improved response extraction provides better formatting and more detailed information about the content type being returned from MCP tool executions.
# Add System Architecture Documentation for Bifrost

This PR adds comprehensive system architecture documentation for Bifrost, a high-performance middleware gateway to multiple AI model providers. The documentation covers:

- Core architecture principles including asynchronous request processing, memory pool management, provider isolation, and plugin-first design
- Detailed component architecture with diagrams for request flow, memory management, and worker pools
- High-performance features including connection pooling, dynamic key management, fallback systems, and plugin architecture
- MCP (Model Context Protocol) integration for external tool capabilities
- Performance benchmarks and scaling configuration examples
- Multi-provider support details for 8 AI model providers
- Network and security features including proxy support and API key rotation
- Transport layer architecture with HTTP, Go SDK, and planned future transports
- Configuration management and error handling patterns
- Development and extension guidelines

The documentation is designed to help users understand how Bifrost achieves its 10,000+ RPS performance target through sophisticated concurrency management, memory optimization, and connection pooling strategies.
# Add environment variable support for MCP connection strings

This PR adds functionality to read environment variables for MCP connection strings in the Bifrost HTTP transport. The implementation:

- Adds a new `ReadMCPKeys()` method to the `BifrostHTTPConfig` struct
- Allows connection strings prefixed with "env." to be replaced with values from environment variables
- Processes each MCP client configuration and updates connection strings accordingly
- Returns appropriate error messages when required environment variables are missing
- Integrates the new functionality in the main application flow

This enhancement provides a more secure way to handle sensitive connection information by keeping it out of configuration files.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 26, 2025

Summary by CodeRabbit

  • New Features

    • Introduced dynamic plugin loading via configuration files, supporting both remote and local plugins.
    • Added standardized plugin initialization through an exported Init function for all plugins.
    • Expanded configuration options to allow specifying plugins and their settings in the main config file.
    • Updated example configuration to include plugin definitions for "maxim" and "mocker".
  • Documentation

    • Significantly enhanced plugin system documentation with detailed guides on dynamic loading, configuration, development, and troubleshooting.
    • Clarified plugin structure, initialization requirements, and Docker usage.
    • Added references and examples for new plugin loading mechanisms.
  • Bug Fixes

    • Improved handling and validation of plugin configuration and initialization errors.
  • Refactor

    • Simplified and unified plugin loading logic, removing hardcoded dependencies and environment variable checks.
    • Streamlined context handling for plugin-related headers.
  • Chores

    • Updated module dependencies and Dockerfile to support new plugin system and improve build consistency.

Walkthrough

This update introduces dynamic plugin loading to the system, requiring plugins to implement an Init(json.RawMessage) (Plugin, error) function for runtime instantiation from configuration files. Documentation, configuration examples, and plugin implementations have been updated accordingly. The HTTP transport now loads plugins dynamically, and a plugin compiler manages just-in-time compilation and dependency resolution.

Changes

File(s) Change Summary
README.md, docs/plugins.md, docs/system-architecture.md Expanded documentation to detail dynamic plugin loading, required Init function, Docker usage, troubleshooting, and plugin development guidelines.
core/schemas/plugin.go Added exported type alias: Init func(config json.RawMessage) (Plugin, error) to standardize plugin initialization.
plugins/maxim/main.go, plugins/mocker/main.go Added exported Init function for dynamic loading; introduced/updated config structs for plugins.
plugins/maxim/go.mod, plugins/mocker/go.mod Updated dependency on github.com/maximhq/bifrost/core from v1.1.5 to v1.1.6.
transports/bifrost-http/lib/config.go Added PluginConfig struct and Plugins []PluginConfig field to main config struct for plugin configuration.
transports/bifrost-http/lib/ctx.go Removed dependency on maxim package; introduced local ContextKey type and unified header processing logic.
transports/bifrost-http/lib/plugin_compiler.go Added new file implementing PluginCompiler for just-in-time plugin compilation, dependency management, and dynamic loading.
transports/bifrost-http/main.go Refactored plugin loading to use dynamic mechanism via configuration and plugin compiler; removed hardcoded plugin logic.
transports/config.example.json Added top-level plugins array with example configurations for "maxim" and "mocker" plugins.
transports/go.mod Removed dependencies on github.com/maximhq/bifrost/plugins/maxim and github.com/maximhq/maxim-go.
transports/Dockerfile Changed base image to golang:1.24-alpine and streamlined directory creation.

Sequence Diagram(s)

sequenceDiagram
    participant Main as Main (bifrost-http)
    participant Compiler as PluginCompiler
    participant Plugin as Plugin (dynamic)
    participant Config as Config File

    Main->>Config: Load configuration (includes plugins array)
    Main->>Compiler: Initialize PluginCompiler
    loop For each plugin in config.Plugins
        Main->>Compiler: LoadPlugin(pluginConfig)
        alt Remote plugin
            Compiler->>Compiler: Download and prepare remote plugin source
        else Local plugin
            Compiler->>Compiler: Copy and prepare local plugin source
        end
        Compiler->>Compiler: Enforce dependency versions
        Compiler->>Compiler: Compile plugin to .so
        Compiler->>Compiler: Load .so with Go plugin loader
        Compiler->>Plugin: Call Init(config json.RawMessage)
        Plugin-->>Compiler: Return Plugin instance
        Compiler-->>Main: Return Plugin instance
    end
    Main->>Main: Aggregate loaded plugins
    Main->>Main: Start server with loaded plugins
Loading

Possibly related PRs

Suggested reviewers

  • akshaydeo
  • danpiths

Poem

In the garden of code where plugins now grow,
A rabbit hops by, admiring the show.
With dynamic delight, they spring into play,
Compiled on the fly, in a modular way.
"Init" is the magic, configuration the key—
Oh, what a hoppy new system to see!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in Comment
  • Commit Unit Tests in branch 06-27-feat_go_plugins_implemented_for_dynamic_plugin_loading

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c559c93 and 02ee6ad.

⛔ Files ignored due to path filters (3)
  • plugins/maxim/go.sum is excluded by !**/*.sum
  • plugins/mocker/go.sum is excluded by !**/*.sum
  • transports/go.sum is excluded by !**/*.sum
📒 Files selected for processing (15)
  • README.md (1 hunks)
  • core/schemas/plugin.go (2 hunks)
  • docs/plugins.md (17 hunks)
  • docs/system-architecture.md (1 hunks)
  • plugins/maxim/go.mod (1 hunks)
  • plugins/maxim/main.go (1 hunks)
  • plugins/mocker/go.mod (1 hunks)
  • plugins/mocker/main.go (2 hunks)
  • transports/Dockerfile (1 hunks)
  • transports/bifrost-http/lib/config.go (1 hunks)
  • transports/bifrost-http/lib/ctx.go (2 hunks)
  • transports/bifrost-http/lib/plugin_compiler.go (1 hunks)
  • transports/bifrost-http/main.go (1 hunks)
  • transports/config.example.json (1 hunks)
  • transports/go.mod (0 hunks)
💤 Files with no reviewable changes (1)
  • transports/go.mod
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
plugins/mocker/go.mod (10)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/go.mod:30-36
Timestamp: 2025-06-04T04:52:31.748Z
Learning: github.com/stretchr/testify v1.10.0 was released on November 23, 2024 and is the latest stable version as of 2024-2025. It includes security fixes for CVE-2022-28948 in gopkg.in/yaml.v3 dependency.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/automatic_function_calling.go:22-22
Timestamp: 2025-06-16T04:55:11.886Z
Learning: In the Bifrost test suite (tests/core-providers), parallel tests using t.Parallel() are not being implemented currently. The team plans to add parallel test execution in future enhancements.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/schemas/bifrost.go:20-23
Timestamp: 2025-06-18T15:16:23.127Z
Learning: In the Bifrost project, BifrostConfig struct is never marshaled/unmarshaled, so serialization tags (json, yaml) are not needed for its fields.
plugins/maxim/go.mod (11)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T03:55:16.949Z
Learning: Go 1.24 was released in February 2025 and is stable and available for use in go.mod files.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/go.mod:3-4
Timestamp: 2025-06-16T04:27:53.538Z
Learning: In Go module files, `go 1.24.1` (with patch version) can work fine in some setups, contrary to the general rule that go directives should only include major.minor versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/go.mod:30-36
Timestamp: 2025-06-04T04:52:31.748Z
Learning: github.com/stretchr/testify v1.10.0 was released on November 23, 2024 and is the latest stable version as of 2024-2025. It includes security fixes for CVE-2022-28948 in gopkg.in/yaml.v3 dependency.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.
transports/Dockerfile (2)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#97
File: transports/Dockerfile:21-24
Timestamp: 2025-06-19T12:17:31.200Z
Learning: In transports/Dockerfile, the user prefers to keep the binary move and upx compression steps in separate RUN commands for better Docker layer caching, rather than combining them to reduce image layers.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
core/schemas/plugin.go (2)
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/schemas/bifrost.go:186-190
Timestamp: 2025-06-15T14:18:32.703Z
Learning: In core/schemas/bifrost.go, the ToolChoice UnmarshalJSON validation intentionally only checks for empty Type fields and lets providers handle validation of specific tool choice values. This architectural decision keeps schema validation focused on structure while allowing provider-specific semantic validation.
plugins/mocker/main.go (2)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#88
File: core/providers/openai.go:202-210
Timestamp: 2025-06-16T06:56:51.547Z
Learning: In Go, when JSON unmarshaling into a pooled struct, slices within the struct are freshly allocated by the JSON unmarshaler, not part of the pooled struct's original memory. Defensive copying of these slices is unnecessary since they don't reference pooled memory that could be reused.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#88
File: core/providers/azure.go:0-0
Timestamp: 2025-06-15T19:04:55.993Z
Learning: In Go, when JSON unmarshaling populates pooled structs, the slice and string fields point to newly allocated memory created by json.Unmarshal(). Shallow copying these fields (copying slice headers) is safe because the underlying data is not part of the pooled memory - it's owned by the JSON unmarshaling process. Deep copying is unnecessary and wasteful in this pattern.
README.md (10)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/schemas/bifrost.go:20-23
Timestamp: 2025-06-18T15:16:23.127Z
Learning: In the Bifrost project, BifrostConfig struct is never marshaled/unmarshaled, so serialization tags (json, yaml) are not needed for its fields.
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.
transports/bifrost-http/main.go (7)
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#67
File: transports/bifrost-http/integrations/anthropic/router.go:26-34
Timestamp: 2025-06-10T11:19:29.604Z
Learning: The Generic router in transports/bifrost-http/integrations/utils.go already handles nil pointers from RequestConverter functions. When a RequestConverter returns a nil *schemas.BifrostRequest, the Generic router automatically returns an HTTP 400 error with "Invalid request" message, making additional nil checks in individual router implementations redundant.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use `package main` for test files in the tests/core-providers directory rather than more descriptive package names like `coreproviders_test`.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/router.go:19-33
Timestamp: 2025-06-09T16:46:32.018Z
Learning: In the GenericRouter (transports/bifrost-http/integrations), ResponseFunc is not called if the BifrostResponse parameter is nil, providing built-in protection against nil response marshaling.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
transports/bifrost-http/lib/config.go (2)
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/schemas/bifrost.go:20-23
Timestamp: 2025-06-18T15:16:23.127Z
Learning: In the Bifrost project, BifrostConfig struct is never marshaled/unmarshaled, so serialization tags (json, yaml) are not needed for its fields.
transports/bifrost-http/lib/ctx.go (8)
Learnt from: connyay
PR: maximhq/bifrost#92
File: core/providers/utils.go:60-64
Timestamp: 2025-06-17T14:04:21.104Z
Learning: The bifrost codebase uses string alias types (like `type ContextKey string`) for context keys consistently across multiple packages including plugins/maxim/main.go and transports/bifrost-http/tracking/plugin.go. This pattern should be maintained for consistency rather than switching to the canonical struct{} approach.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and handles the conversion internally, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:62-64
Timestamp: 2025-06-09T11:27:00.925Z
Learning: The `lib.ConvertToBifrostContext` function in the bifrost HTTP transport never returns nil and always returns a pointer to a valid context. It starts with context.Background() and only adds values to it, so nil checks are not needed when calling this function.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#64
File: transports/bifrost-http/integrations/genai/types.go:273-313
Timestamp: 2025-06-09T16:35:26.914Z
Learning: In convertGenerationConfigToParams method in transports/bifrost-http/integrations/genai/types.go, pre-allocating the ExtraParams map is preferred over lazy allocation because the method has multiple potential ExtraParams assignments, making the computational overhead of conditional checks exceed the memory savings of an empty map.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/schemas/bifrost.go:20-23
Timestamp: 2025-06-18T15:16:23.127Z
Learning: In the Bifrost project, BifrostConfig struct is never marshaled/unmarshaled, so serialization tags (json, yaml) are not needed for its fields.
transports/config.example.json (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#97
File: transports/Dockerfile:37-42
Timestamp: 2025-06-19T09:06:25.750Z
Learning: In Docker configurations for this project, plugin-specific environment variables (like MAXIM_LOG_REPO_ID) should not be included in the Dockerfile's ENV section. The architectural goal is to keep Docker images plugin-agnostic and externalize all plugin configuration to runtime via docker run -e flags, rather than baking plugin config into the image.
docs/plugins.md (18)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/schemas/provider.go:148-148
Timestamp: 2025-06-04T03:57:50.981Z
Learning: Breaking changes in the Bifrost codebase are managed by first merging and tagging core schema changes, then updating dependent code references in subsequent steps after the core version is released.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#63
File: transports/bifrost-http/integrations/openai/router.go:0-0
Timestamp: 2025-06-09T14:03:34.227Z
Learning: In the Bifrost HTTP transport layer (transports/bifrost-http/integrations/), request validation like checking for empty messages should be handled by the provider rather than at the transport layer. The transport layer should forward requests to Bifrost core/providers for validation.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#79
File: core/bifrost.go:94-103
Timestamp: 2025-06-14T04:06:58.240Z
Learning: In core/bifrost.go, the count parameter in RunPostHooks method is intentionally kept separate from p.executedPreHooks to support circuit breaker plugins that may need to trigger PostHooks for only a subset of executed plugins when detecting failure conditions mid-execution.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#94
File: core/bifrost.go:823-845
Timestamp: 2025-06-18T15:15:51.323Z
Learning: In the Bifrost project, the team prioritizes maintaining consistent error handling patterns over exposing detailed error context. All errors should be wrapped in the standard `BifrostError` structure rather than creating specific error types or exposing richer error details like exit codes or stderr output.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.418Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/end_to_end_tool_calling.go:43-45
Timestamp: 2025-06-16T04:13:55.437Z
Learning: In the Bifrost codebase, errors returned from client methods like ChatCompletionRequest are of type BifrostError, not the standard error interface. For testing these errors, use require.Nilf instead of require.NoErrorf since BifrostError doesn't work with the standard error assertion methods.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/simple_chat.go:39-41
Timestamp: 2025-06-16T04:13:42.755Z
Learning: In the Bifrost codebase, errors returned from methods like ChatCompletionRequest are of type BifrostError (a custom error type) rather than the standard Go error interface. Therefore, require.Nilf should be used for error assertions instead of require.NoErrorf.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/config/account.go:55-101
Timestamp: 2025-06-16T04:25:00.816Z
Learning: In the Bifrost test account implementation, the user prefers to let Bifrost itself handle missing API key errors rather than adding early validation in the GetKeysForProvider method.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#81
File: tests/core-providers/scenarios/complete_end_to_end.go:39-41
Timestamp: 2025-06-16T04:12:05.427Z
Learning: In the Bifrost system, error returns are of type `BifrostError` rather than the standard Go `error` interface. Therefore, use `require.Nilf(t, err, ...)` instead of `require.NoError(t, err)` when checking for errors in Bifrost function calls.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:168-170
Timestamp: 2025-06-15T14:24:49.882Z
Learning: In the Bifrost codebase, performance is prioritized over defensive copying for HTTP service operations. Specifically, shallow slice assignments in provider response handling are acceptable due to object pool reset patterns and JSON unmarshaling behavior that minimize practical data corruption risks.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: core/providers/ollama.go:59-61
Timestamp: 2025-06-15T14:34:29.401Z
Learning: The `CheckAndSetDefaults()` method on `*schemas.ProviderConfig` in Go does not return any error - it has a void return type and only sets default values on the configuration struct.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#88
File: core/providers/openai.go:202-210
Timestamp: 2025-06-16T06:56:51.547Z
Learning: In Go, when JSON unmarshaling into a pooled struct, slices within the struct are freshly allocated by the JSON unmarshaler, not part of the pooled struct's original memory. Defensive copying of these slices is unnecessary since they don't reference pooled memory that could be reused.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#88
File: core/providers/azure.go:0-0
Timestamp: 2025-06-15T19:04:55.993Z
Learning: In Go, when JSON unmarshaling populates pooled structs, the slice and string fields point to newly allocated memory created by json.Unmarshal(). Shallow copying these fields (copying slice headers) is safe because the underlying data is not part of the pooled memory - it's owned by the JSON unmarshaling process. Deep copying is unnecessary and wasteful in this pattern.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#88
File: core/providers/ollama.go:171-179
Timestamp: 2025-06-16T06:56:52.953Z
Learning: In Go, when json.Unmarshal populates slice fields in structs, it allocates fresh backing arrays independently of the struct's memory. The struct only stores slice headers (pointer, length, capacity). When pooled structs are reset with *resp = ResponseType{}, only the slice headers are cleared, not the underlying backing arrays, so no deep copying is needed to avoid memory corruption.
transports/bifrost-http/lib/plugin_compiler.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using `go get`, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
🧬 Code Graph Analysis (1)
plugins/maxim/main.go (2)
core/schemas/plugin.go (2)
  • Init (70-70)
  • Plugin (45-65)
plugins/mocker/main.go (1)
  • Init (21-28)
🪛 LanguageTool
docs/system-architecture.md

[uncategorized] ~656-~656: You might be missing the article “a” here.
Context: ...: Implement custom auth mechanisms For complete plugin development guide, see [Plugin D...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

docs/plugins.md

[uncategorized] ~953-~953: Loose punctuation mark.
Context: ...ugin was built with a different version`: Version mismatch between main app and p...

(UNLIKELY_OPENING_PUNCTUATION)

🪛 golangci-lint (1.64.8)
transports/bifrost-http/main.go

166-166: Error return value of pluginCompiler.Cleanup is not checked

(errcheck)

🪛 markdownlint-cli2 (0.17.2)
docs/plugins.md

532-532: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)


589-589: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)


779-779: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)


788-788: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)


800-800: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)

🔇 Additional comments (14)
plugins/maxim/go.mod (1)

5-8: Core bump looks good – confirm tag v1.1.6 is published

The version sync to github.com/maximhq/bifrost/core v1.1.6 is expected.
Just double-check the tag exists to avoid CI breakage on first go mod tidy.

plugins/mocker/go.mod (1)

5-8: Dependency aligned with core; no issues spotted

The module now tracks core v1.1.6, matching the Maxim plugin. Nothing else changed.

plugins/mocker/main.go (1)

20-28: LGTM! Clean implementation of the dynamic plugin loading interface.

The Init function follows the standardized signature defined in core/schemas/plugin.go and properly handles JSON unmarshaling with appropriate error wrapping. The implementation is consistent with the pattern used in other plugins.

README.md (1)

257-276: Excellent documentation of the new plugin system capabilities.

The updated documentation clearly explains both traditional instantiation and dynamic loading modes, providing a practical code example for traditional usage and appropriately referencing the detailed plugin documentation for dynamic loading. This serves as a good entry point for users to understand the plugin system.

plugins/maxim/main.go (1)

20-41: Well-implemented dynamic plugin loading interface.

The MaximConfig struct and Init function are properly implemented with:

  • Appropriate JSON field tags
  • Required field validation with clear error messages
  • Consistent error wrapping pattern
  • Proper delegation to the existing NewMaximLoggerPlugin constructor

This follows the same pattern as other plugins and meets the plugin system requirements.

transports/bifrost-http/lib/config.go (1)

28-42: Well-designed configuration structure for dynamic plugin loading.

The PluginConfig struct effectively supports the plugin system with:

  • Clear field naming and JSON tags
  • json.RawMessage for flexible plugin-specific configuration
  • Proper integration into BifrostHTTPConfig
  • Updated documentation reflecting the new plugin support

This provides a clean foundation for the dynamic plugin loading system.

transports/bifrost-http/lib/ctx.go (1)

43-43: Good approach using local ContextKey type.

Introducing a local ContextKey type alias removes the dependency on the external maxim package while maintaining consistency with the established pattern in the Bifrost codebase.

transports/bifrost-http/main.go (2)

162-180: Excellent refactoring to dynamic plugin loading.

The migration from hardcoded plugin loading to configuration-driven dynamic loading is well implemented. The error handling appropriately logs warnings while allowing the service to continue, and the cleanup is properly deferred.


186-189: Good logging for operational visibility.

The logging of loaded plugin count and names provides excellent operational visibility for debugging and monitoring plugin loading behavior.

transports/config.example.json (1)

160-199: Well-structured plugin configuration.

The plugin configuration section demonstrates excellent usage of the new dynamic loading system:

  • Clear separation between plugin metadata (name, source, type) and plugin-specific config
  • Proper use of environment variable substitution with env. prefix
  • Good examples for both maxim and mocker plugins
  • Configuration structure aligns well with the documented plugin system
docs/plugins.md (1)

22-270: Comprehensive and excellent plugin documentation.

This documentation update is outstanding:

  • Covers all aspects of the new dynamic plugin loading system
  • Provides clear examples for both local and remote plugins
  • Includes detailed troubleshooting guidance
  • Docker usage patterns are well-documented
  • The plugin interface examples are accurate and helpful

This will significantly improve the developer experience for plugin development and deployment.

transports/bifrost-http/lib/plugin_compiler.go (3)

42-80: Excellent plugin loading workflow design.

The LoadPlugin method demonstrates solid software engineering:

  • Clear separation of concerns with well-defined steps
  • Proper error handling with context-rich error messages
  • Good use of the workspace pattern for isolation
  • Appropriate logging for debugging and operations

319-367: Sophisticated dependency resolution strategy.

The dependency enforcement logic is well-designed:

  • Hybrid approach using both local go.mod and runtime dependencies
  • Graceful degradation when main app dependencies can't be determined
  • Proper preservation of plugin-specific dependencies
  • Good logging for operational visibility

This approach should handle both development and production deployment scenarios effectively.


273-301: Robust plugin loading and initialization.

The compiled plugin loading logic correctly:

  • Uses Go's plugin.Open API appropriately
  • Performs proper type checking for the Init function signature
  • Handles initialization errors gracefully
  • Follows the standardized plugin interface

Comment thread core/schemas/plugin.go
Comment on lines +4 to +7
import (
"context"
"encoding/json"
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Import block is fine – but run goimports before merge

encoding/json is now required for the new type alias, but a quick goimports run will ensure canonical grouping / pruning if future edits remove it.

🤖 Prompt for AI Agents
In core/schemas/plugin.go around lines 4 to 7, the import block includes
"encoding/json" which is needed for the new type alias, but to maintain proper
import formatting and remove any unused imports, run the `goimports` tool before
merging. This will automatically organize and prune the imports to follow Go
conventions.

Comment thread core/schemas/plugin.go
Comment on lines +66 to +70

// Init defines the standardized constructor signature for all plugins.
// All plugins should implement: Init(config json.RawMessage) (Plugin, error)
// This is enforced at development time through documentation and examples.
type Init func(config json.RawMessage) (Plugin, error)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider a clearer alias name than Init to avoid ambiguity

Init is already a very common free-standing function name across packages, including every plugin being required to declare an Init func. Defining a type with the same identifier in the root schemas package can be mildly confusing in IDE go-to-definition / autocomplete flows.

A more descriptive name such as PluginInitFunc (or LoaderFunc) avoids the clash while keeping the call-site trivial:

-// Init defines the standardized constructor signature for all plugins.
-// All plugins should implement: Init(config json.RawMessage) (Plugin, error)
-type Init func(config json.RawMessage) (Plugin, error)
+// PluginInitFunc is the standard constructor signature every plugin must expose.
+// All plugins must provide:  Init(config json.RawMessage) (schemas.Plugin, error)
+type PluginInitFunc func(config json.RawMessage) (Plugin, error)

Refactoring early prevents a breaking rename later if the alias ever needs to be exported/imported explicitly.

🤖 Prompt for AI Agents
In core/schemas/plugin.go around lines 66 to 70, the type alias named Init
conflicts with common function names and can cause confusion in IDE navigation.
Rename the alias from Init to a more descriptive name like PluginInitFunc or
LoaderFunc to clearly indicate its purpose as the plugin constructor signature.
Update all references to this alias accordingly to maintain consistency and
clarity.

Comment thread transports/Dockerfile
Comment on lines +32 to 38
FROM golang:1.24-alpine
WORKDIR /app

# Copy necessary files and create structure in one layer
COPY --from=builder /app/main .
RUN mkdir -p /app/config && \
RUN mkdir -p /app/config /app/plugins && \
adduser -D -s /bin/sh appuser && \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

Runtime image switched to full Go toolchain – weigh size vs. need

Moving the runtime stage from alpine:3.19 (~7 MB) to golang:1.24-alpine (~400 MB) gives the container the Go compiler needed for JIT plugin builds.
This balloons the final image and the attack surface.

If in-container compilation proves necessary, consider:

  1. Multi-stage copy of the go binary + $GOROOT from builder instead of the entire image.
  2. Pre-compiling remote plugins during an init container / sidecar to keep production image lean.

Not blocking, but worth a ticket if image size matters to deployments.

🤖 Prompt for AI Agents
In transports/Dockerfile around lines 32 to 38, the runtime image was changed
from a minimal alpine base to the full golang:1.24-alpine image, significantly
increasing image size and attack surface. To fix this, revert to using a minimal
base image like alpine:3.19 for the runtime stage and implement a multi-stage
build that copies only the necessary Go binaries and libraries (such as the go
binary and GOROOT) from the builder stage, or alternatively pre-compile plugins
outside the runtime container to keep the final image lean.

Comment on lines +649 to +657
- **Dynamic Loading**: All plugins must implement `Init(config json.RawMessage) (schemas.Plugin, error)` function
- **Request Processing**: Modify requests before provider calls
- **Response Processing**: Transform responses after provider calls
- **Caching**: Implement custom caching strategies
- **Monitoring**: Add custom metrics and logging
- **Authentication**: Implement custom auth mechanisms

For complete plugin development guide, see [Plugin Documentation](./plugins.md).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Minor wording & article nitpick

“For complete plugin development guide, see…”

Insert the article “a” for smoother reading:

-For complete plugin development guide, see
+For a complete plugin development guide, see

Also consider code-formatting json.RawMessage in the bullet for consistency with other code references.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- **Dynamic Loading**: All plugins must implement `Init(config json.RawMessage) (schemas.Plugin, error)` function
- **Request Processing**: Modify requests before provider calls
- **Response Processing**: Transform responses after provider calls
- **Caching**: Implement custom caching strategies
- **Monitoring**: Add custom metrics and logging
- **Authentication**: Implement custom auth mechanisms
For complete plugin development guide, see [Plugin Documentation](./plugins.md).
- **Dynamic Loading**: All plugins must implement `Init(config json.RawMessage) (schemas.Plugin, error)` function
- **Request Processing**: Modify requests before provider calls
- **Response Processing**: Transform responses after provider calls
- **Caching**: Implement custom caching strategies
- **Monitoring**: Add custom metrics and logging
- **Authentication**: Implement custom auth mechanisms
- For a complete plugin development guide, see [Plugin Documentation](./plugins.md).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~656-~656: You might be missing the article “a” here.
Context: ...: Implement custom auth mechanisms For complete plugin development guide, see [Plugin D...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🤖 Prompt for AI Agents
In docs/system-architecture.md around lines 649 to 657, update the sentence "For
complete plugin development guide, see…" to "For a complete plugin development
guide, see…" by adding the article "a" for smoother reading. Also, format
`json.RawMessage` as inline code in the bullet point describing the Init
function to maintain consistency with other code references.

Comment on lines 57 to 73
if strings.HasPrefix(keyStr, "x-bf-maxim-") {
labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")

if labelName == string(maxim.GenerationIDKey) {
bifrostCtx = context.WithValue(bifrostCtx, maxim.ContextKey(labelName), string(value))
if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
return
}
}

if labelName == string(maxim.TraceIDKey) {
bifrostCtx = context.WithValue(bifrostCtx, maxim.ContextKey(labelName), string(value))
}
if strings.HasPrefix(keyStr, "x-bf-maxim-") {
labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")

if labelName == string(maxim.SessionIDKey) {
bifrostCtx = context.WithValue(bifrostCtx, maxim.ContextKey(labelName), string(value))
if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
return
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix duplicate header processing logic.

The x-bf-maxim header processing logic is duplicated - lines 57-64 and lines 66-73 contain nearly identical code blocks that will process the same headers twice. This appears to be a copy-paste error.

Remove the duplicate block:

 		if strings.HasPrefix(keyStr, "x-bf-maxim-") {
 			labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")
 
 			if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
 				bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
 				return
 			}
 		}
-
-		if strings.HasPrefix(keyStr, "x-bf-maxim-") {
-			labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")
-
-			if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
-				bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
-				return
-			}
-		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if strings.HasPrefix(keyStr, "x-bf-maxim-") {
labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")
if labelName == string(maxim.GenerationIDKey) {
bifrostCtx = context.WithValue(bifrostCtx, maxim.ContextKey(labelName), string(value))
if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
return
}
}
if labelName == string(maxim.TraceIDKey) {
bifrostCtx = context.WithValue(bifrostCtx, maxim.ContextKey(labelName), string(value))
}
if strings.HasPrefix(keyStr, "x-bf-maxim-") {
labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")
if labelName == string(maxim.SessionIDKey) {
bifrostCtx = context.WithValue(bifrostCtx, maxim.ContextKey(labelName), string(value))
if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
return
}
}
if strings.HasPrefix(keyStr, "x-bf-maxim-") {
labelName := strings.TrimPrefix(keyStr, "x-bf-maxim-")
if labelName == "session-id" || labelName == "trace-id" || labelName == "generation-id" {
bifrostCtx = context.WithValue(bifrostCtx, ContextKey(labelName), string(value))
return
}
}
🤖 Prompt for AI Agents
In transports/bifrost-http/lib/ctx.go around lines 57 to 73, there is a
duplicated block of code that processes headers starting with "x-bf-maxim-".
Remove the second occurrence of this block (lines 66 to 73) to eliminate
redundant processing and keep only one instance of the header handling logic.

if len(config.Plugins) > 0 {
// Initialize plugin compiler for dynamic loading
pluginCompiler := lib.NewPluginCompiler()
defer pluginCompiler.Cleanup()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle the error return from Cleanup().

The static analysis correctly identified that the error return from pluginCompiler.Cleanup() is not checked. While this is in a defer statement, it's good practice to handle cleanup errors appropriately.

-		defer pluginCompiler.Cleanup()
+		defer func() {
+			if err := pluginCompiler.Cleanup(); err != nil {
+				log.Printf("warning: failed to cleanup plugin compiler: %v", err)
+			}
+		}()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
defer pluginCompiler.Cleanup()
defer func() {
if err := pluginCompiler.Cleanup(); err != nil {
log.Printf("warning: failed to cleanup plugin compiler: %v", err)
}
}()
🧰 Tools
🪛 golangci-lint (1.64.8)

166-166: Error return value of pluginCompiler.Cleanup is not checked

(errcheck)

🤖 Prompt for AI Agents
In transports/bifrost-http/main.go at line 166, the defer statement calls
pluginCompiler.Cleanup() but ignores its error return. Modify the defer function
to capture the error returned by Cleanup(), and handle it appropriately, such as
logging the error, to ensure cleanup issues are not silently ignored.

Comment thread docs/plugins.md

### Basic Plugin Structure

**For Dynamic Loading (Required)**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Fix markdown heading formatting.

The static analysis correctly identified that emphasis (bold text) is being used instead of proper markdown headings for section markers.

-**For Dynamic Loading (Required)**
+#### For Dynamic Loading (Required)

-**For Direct Instantiation (Legacy)**
+#### For Direct Instantiation (Legacy)

-**1. Init Function (Mandatory)**
+#### 1. Init Function (Mandatory)

-**2. Configuration Struct**
+#### 2. Configuration Struct

-**3. Plugin Implementation**
+#### 3. Plugin Implementation

Also applies to: 589-589, 779-779, 788-788, 800-800

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

532-532: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)

🤖 Prompt for AI Agents
In docs/plugins.md at lines 532, 589, 779, 788, and 800, the section titles are
formatted using bold text instead of proper markdown headings. Replace the bold
text markers with appropriate markdown heading syntax (e.g., starting the line
with one or more '#' characters) to correctly format these as headings.

Comment on lines +143 to +146
cmd := exec.Command("cp", "-r", filepath.Join(dir, localPath)+"/.", workDir)
if output, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("failed to copy local plugin: %s", string(output))
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve cross-platform compatibility for local plugin copying.

The current implementation uses the Unix cp command which won't work on Windows. Consider using Go's built-in file operations for better portability.

-	// Copy all files from the source directory directly to workDir
-	cmd := exec.Command("cp", "-r", filepath.Join(dir, localPath)+"/.", workDir)
-	if output, err := cmd.CombinedOutput(); err != nil {
-		return fmt.Errorf("failed to copy local plugin: %s", string(output))
-	}
+	// Copy all files from the source directory directly to workDir
+	srcPath := filepath.Join(dir, localPath)
+	err = copyDir(srcPath, workDir)
+	if err != nil {
+		return fmt.Errorf("failed to copy local plugin: %w", err)
+	}

You'll need to implement a copyDir helper function using Go's standard library:

func copyDir(src, dst string) error {
	return filepath.Walk(src, func(path string, info os.FileInfo, err error) error {
		if err != nil {
			return err
		}
		
		relPath, err := filepath.Rel(src, path)
		if err != nil {
			return err
		}
		
		dstPath := filepath.Join(dst, relPath)
		
		if info.IsDir() {
			return os.MkdirAll(dstPath, info.Mode())
		}
		
		return copyFile(path, dstPath)
	})
}

func copyFile(src, dst string) error {
	sourceFile, err := os.Open(src)
	if err != nil {
		return err
	}
	defer sourceFile.Close()
	
	destFile, err := os.Create(dst)
	if err != nil {
		return err
	}
	defer destFile.Close()
	
	_, err = io.Copy(destFile, sourceFile)
	return err
}
🤖 Prompt for AI Agents
In transports/bifrost-http/lib/plugin_compiler.go around lines 143 to 146,
replace the use of the Unix-specific "cp" command with a Go-native directory
copy implementation for cross-platform compatibility. Implement a helper
function copyDir that recursively walks the source directory, creates
directories as needed, and copies files using standard library functions. Also
implement a copyFile helper to handle individual file copying with proper open,
create, and io.Copy calls. Then call copyDir with the source and destination
paths instead of exec.Command.

Comment on lines +232 to +235
oldPackageDecl := fmt.Sprintf("package %s", packageName)
newPackageDecl := "package main"

modifiedContent = strings.Replace(modifiedContent, oldPackageDecl, newPackageDecl, 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider more robust package name replacement.

The current string replacement approach for changing package declarations might be fragile if the package name appears elsewhere in the code or if there are multiple package declarations.

Consider using Go's AST parsing for more reliable package declaration replacement:

import (
	"go/ast"
	"go/format"
	"go/parser"
	"go/token"
)

func (pc *PluginCompiler) convertPackageToMain(pluginDir, originalPackage string) error {
	packageName := filepath.Base(originalPackage)
	
	files, err := filepath.Glob(filepath.Join(pluginDir, "*.go"))
	if err != nil {
		return fmt.Errorf("failed to find .go files: %w", err)
	}
	
	for _, file := range files {
		fset := token.NewFileSet()
		node, err := parser.ParseFile(fset, file, nil, parser.ParseComments)
		if err != nil {
			return fmt.Errorf("failed to parse file %s: %w", file, err)
		}
		
		// Only change if it matches the expected package name
		if node.Name.Name == packageName {
			node.Name.Name = "main"
			
			// Write back the modified AST
			f, err := os.Create(file)
			if err != nil {
				return fmt.Errorf("failed to create file %s: %w", file, err)
			}
			defer f.Close()
			
			err = format.Node(f, fset, node)
			if err != nil {
				return fmt.Errorf("failed to write modified file %s: %w", file, err)
			}
		}
	}
	
	return nil
}

However, the current approach is simpler and likely sufficient for most cases.

🤖 Prompt for AI Agents
In transports/bifrost-http/lib/plugin_compiler.go around lines 232 to 235, the
current package name replacement uses a simple string replacement which can be
unreliable if the package name appears elsewhere or multiple times. To fix this,
replace the string replacement logic with Go's AST parsing: parse each Go source
file in the plugin directory, check if the package name matches the original
package, then update the package name to "main" in the AST, and write the
modified AST back to the file. This ensures only the package declaration is
changed safely and correctly.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 7, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 4 committers have signed the CLA.

❌ github-actions[bot]
❌ akshaydeo
❌ Connor Hindley
❌ Pratham-Mishra04


Connor Hindley seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants