Skip to content

adds dynamic ui #655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Parent: dynamic plugin architecture
wants to merge 1 commit into
base: 10-16-dynamic_plugin_architecture
Choose a base branch
from
Open

adds dynamic ui #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions core/schemas/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@ type PluginShortCircuit struct {
Error *BifrostError // If set, short-circuit with this error (can set AllowFallbacks field)
}

// PluginStatus constants
const (
PluginStatusActive = "active"
PluginStatusError = "error"
PluginStatusDisabled = "disabled"
PluginStatusLoading = "loading"
PluginStatusUninitialized = "uninitialized"
PluginStatusUnloaded = "unloaded"
PluginStatusLoaded = "loaded"
)

// PluginStatus represents the status of a plugin.
type PluginStatus struct {
Name string `json:"name"`
Status string `json:"status"`
Logs []string `json:"logs"`
}

// Plugin defines the interface for Bifrost plugins.
// Plugins can intercept and modify requests and responses at different stages
// of the processing pipeline.
Expand Down
10 changes: 9 additions & 1 deletion framework/configstore/migrations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,13 +600,18 @@ func migrationAddVirtualKeyMCPConfigsTable(ctx context.Context, db *gorm.DB) err
// migrationAddPluginPathColumn adds the path column to the plugin table
func migrationAddPluginPathColumn(ctx context.Context, db *gorm.DB) error {
m := migrator.New(db, migrator.DefaultOptions, []*migrator.Migration{{
ID: "add_plugin_path_column",
ID: "update_plugins_table_for_custom_plugins",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

CRITICAL: Verify migration ID change impact before merging.

Changing the migration ID from "add_plugin_path_column" to "update_plugins_table_for_custom_plugins" will cause the migration system to treat this as a completely new migration. If the old ID was already executed in any environment (dev, staging, production), the system will attempt to run this migration again, potentially causing conflicts even with the HasColumn checks.

Please verify:

  1. Has the migration with ID "add_plugin_path_column" been deployed to any environment?
  2. If yes, you'll need a different approach (e.g., a separate additive migration for the is_custom column)
  3. If no, document that this change is safe because the old ID was never released

Run this script to check if the old migration ID exists in the migration history:

🏁 Script executed:

#!/bin/bash
# Search for any references to the old migration ID in the codebase and git history
echo "=== Checking for old migration ID in current codebase ==="
rg -n "add_plugin_path_column" --type go

echo -e "\n=== Checking git history for when old migration ID was introduced ==="
git log --all --oneline --grep="add_plugin_path_column" || echo "No commits found with old migration ID in message"

echo -e "\n=== Checking if old migration ID exists in git history ==="
git log -p --all -S "add_plugin_path_column" -- "*.go" | head -50

Length of output: 188

🏁 Script executed:

#!/bin/bash
# Check the git history of the migrations.go file
echo "=== Git history of migrations.go file ==="
git log --oneline framework/configstore/migrations.go | head -20

echo -e "\n=== When was this specific migration function added? ==="
git log -p --all -- framework/configstore/migrations.go | grep -A5 -B5 "migrationAddPluginPathColumn" | head -40

echo -e "\n=== Current state of the function around line 600-630 ==="
sed -n '600,630p' framework/configstore/migrations.go

Length of output: 306

🏁 Script executed:

#!/bin/bash
# Check the git history of the migrations.go file
echo "=== Git history of migrations.go file ==="
git log --oneline framework/configstore/migrations.go | head -20

echo -e "\n=== When was this specific migration function added? ==="
git log -p --all -- framework/configstore/migrations.go | grep -A10 -B5 "migrationAddPluginPathColumn" | head -50

echo -e "\n=== Current state of the function around line 600-636 ==="
sed -n '600,636p' framework/configstore/migrations.go

echo -e "\n=== Check for rollback defensive checks ==="
sed -n '619,629p' framework/configstore/migrations.go

Length of output: 2291

🏁 Script executed:

#!/bin/bash
# Search for the old migration ID across entire git history
echo "=== Search for any migration with old ID 'add_plugin_path_column' across all history ==="
git log -p --all -S "add_plugin_path_column" -- framework/configstore/migrations.go | head -100

echo -e "\n=== Check when migrationAddPluginPathColumn was first added ==="
git log --all --source --remotes --branches -p -- framework/configstore/migrations.go | grep -A30 "func migrationAddPluginPathColumn" | head -50

echo -e "\n=== Check if this function exists in main/master branch ==="
git show origin/main:framework/configstore/migrations.go 2>/dev/null | grep -A5 "migrationAddPluginPathColumn" || echo "Function not found in main branch"

Length of output: 2118

CRITICAL: Migration ID change will re-execute already-deployed migration in production.

The git history confirms that the migration ID "add_plugin_path_column" was already deployed. Changing it to "update_plugins_table_for_custom_plugins" will cause the migration system to treat this as a completely new migration and attempt to execute it again across all environments, even though the HasColumn checks exist.

Action required: Either:

  • Revert the ID change to "add_plugin_path_column" and create a separate new migration for the is_custom column, OR
  • Confirm this migration was never actually deployed and document why the ID change is safe

Secondary issue: The rollback logic lacks defensive checks. Add HasColumn guards before DropColumn calls (lines 625–627) to prevent failures if the migration partially executed.

🤖 Prompt for AI Agents 
In framework/configstore/migrations.go around line 603 (where ID:
"update_plugins_table_for_custom_plugins" is set) revert the migration ID back
to the original "add_plugin_path_column" unless you can prove this migration was
never deployed and document that justification; if you need to add the new
is_custom column, create a separate migration file with a new unique ID instead
of renaming the existing one. Additionally, in the rollback section around lines
625–627, add defensive HasColumn checks before each DropColumn call so the
rollback only attempts to drop columns that exist, preventing failures from
partially-applied migrations.

Migrate: func(tx *gorm.DB) error {
tx = tx.WithContext(ctx)
migrator := tx.Migrator()
if !migrator.HasColumn(&tables.TablePlugin{}, "path") {
if err := migrator.AddColumn(&tables.TablePlugin{}, "path"); err != nil {
return err
}
}
if !migrator.HasColumn(&tables.TablePlugin{}, "is_custom") {
if err := migrator.AddColumn(&tables.TablePlugin{}, "is_custom"); err != nil {
return err
}
}
Comment on lines +612 to 616
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify default value is applied to existing rows.

When adding the is_custom column, existing plugin rows should default to false. The GORM struct tag gorm:"default:false" should handle this, but the behavior can vary across database engines.

Consider running this verification script after the migration to confirm existing rows have is_custom = false:

🏁 Script executed:

#!/bin/bash
# Verify that all existing plugins have is_custom set to false after migration
# This requires database access credentials to be configured

echo "This script requires manual execution with database access"
echo "Example SQL to verify after migration:"
echo ""
echo "SELECT name, is_custom, path FROM config_plugins WHERE is_custom IS NULL OR is_custom = true;"
echo ""
echo "Expected: No rows returned (all existing plugins should have is_custom = false)"

Length of output: 273


---

🏁 Script executed:

```shell
#!/bin/bash
# Examine the is_custom field definition in tables/plugin.go
fd -type f -name "plugin.go" --path "*/tables/*" --exec cat {} \; | grep -A5 -B5 "is_custom"

Length of output: 170

🏁 Script executed:

#!/bin/bash
# View the migration code context around lines 612-616
cat framework/configstore/migrations.go | sed -n '605,625p'

Length of output: 694

🏁 Script executed:

#!/bin/bash
# Check if there are any explicit ALTER statements or default value assignments in the migration
rg -n "is_custom" framework/configstore/migrations.go -B3 -A3

Length of output: 483


---

🏁 Script executed:

```shell
#!/bin/bash
# Find plugin.go in tables directory
fd -type f -name "plugin.go"

Length of output: 170

🏁 Script executed:

#!/bin/bash
# Once found, search for is_custom field definition with GORM tags
rg -n "is_custom" --type go -B2 -A2 | grep -A2 -B2 "is_custom"

Length of output: 750


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for TablePlugin struct definition in Go files
rg -n "type TablePlugin" -A30 | head -60

Length of output: 2320

🏁 Script executed:

#!/bin/bash
# Search for is_custom across all files to find the struct definition
find . -name "*.go" -type f | xargs grep -l "is_custom" | head -10

Length of output: 37

🏁 Script executed:

#!/bin/bash
# Look for tables directory and plugin definitions
find . -path "*/tables/*.go" -type f 2>/dev/null | head -20

Length of output: 802


---

🏁 Script executed:

```shell
#!/bin/bash
# Check the full migration block around is_custom to see if there's an UPDATE statement
rg -n "is_custom" framework/configstore/migrations.go -B10 -A10

Length of output: 1157

🏁 Script executed:

#!/bin/bash
# Look for UPDATE statements in the migration function around line 612
sed -n '600,635p' framework/configstore/migrations.go

Length of output: 1170

return nil
Expand All @@ -617,6 +622,9 @@ func migrationAddPluginPathColumn(ctx context.Context, db *gorm.DB) error {
if err := migrator.DropColumn(&tables.TablePlugin{}, "path"); err != nil {
return err
}
if err := migrator.DropColumn(&tables.TablePlugin{}, "is_custom"); err != nil {
return err
}
return nil
},
}})
Expand Down
15 changes: 12 additions & 3 deletions framework/configstore/rdb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,12 +825,12 @@ func (s *RDBConfigStore) DeleteModelPrices(ctx context.Context, tx ...*gorm.DB)

// PLUGINS METHODS

func (s *RDBConfigStore) GetPlugins(ctx context.Context) ([]tables.TablePlugin, error) {
var plugins []tables.TablePlugin
func (s *RDBConfigStore) GetPlugins(ctx context.Context) ([]*tables.TablePlugin, error) {
var plugins []*tables.TablePlugin
if err := s.db.WithContext(ctx).Find(&plugins).Error; err != nil {
return nil, err
}
return plugins, nil
return plugins, nil
}

func (s *RDBConfigStore) GetPlugin(ctx context.Context, name string) (*tables.TablePlugin, error) {
Expand All @@ -851,6 +851,10 @@ func (s *RDBConfigStore) CreatePlugin(ctx context.Context, plugin *tables.TableP
} else {
txDB = s.db
}
// Mark plugin as custom if path is not empty
if plugin.Path != nil && *plugin.Path != "" {
plugin.IsCustom = true
}
Comment on lines +854 to +857
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Derive IsCustom deterministically (also when path is cleared).

Currently only sets true; if a plugin’s path becomes empty, IsCustom may stay stale. Compute from Path every time.

-// Mark plugin as custom if path is not empty
-if plugin.Path != nil && *plugin.Path != "" {
-    plugin.IsCustom = true
-}
+// Derive IsCustom from Path (trimmed) on every write
+if plugin.Path != nil && strings.TrimSpace(*plugin.Path) != "" {
+    plugin.IsCustom = true
+} else {
+    plugin.IsCustom = false
+}

Add import:

 import (
   "context"
   "encoding/json"
   "errors"
   "fmt"
+  "strings"
 )
📝 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
// Mark plugin as custom if path is not empty
if plugin.Path != nil && *plugin.Path != "" {
plugin.IsCustom = true
}
// Derive IsCustom from Path (trimmed) on every write
if plugin.Path != nil && strings.TrimSpace(*plugin.Path) != "" {
plugin.IsCustom = true
} else {
plugin.IsCustom = false
}
🤖 Prompt for AI Agents 
In framework/configstore/rdb.go around lines 854 to 857, the code only sets
plugin.IsCustom true when Path is non-empty, leaving it stale if Path is later
cleared; change the logic to deterministically compute IsCustom from Path on
every update (e.g., set plugin.IsCustom to the boolean expression (plugin.Path
!= nil && *plugin.Path != "")) so it becomes false when Path is cleared; update
any related code paths accordingly and add any necessary import if you reference
helpers for nil/empty checks.

return txDB.WithContext(ctx).Create(plugin).Error
}

Expand All @@ -866,6 +870,11 @@ func (s *RDBConfigStore) UpdatePlugin(ctx context.Context, plugin *tables.TableP
localTx = true
}

// Mark plugin as custom if path is not empty
if plugin.Path != nil && *plugin.Path != "" {
plugin.IsCustom = true
}

Comment on lines +873 to +877
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Apply the same IsCustom derivation in UpdatePlugin.

Mirror the deterministic computation here as well to avoid divergence.

-// Mark plugin as custom if path is not empty
-if plugin.Path != nil && *plugin.Path != "" {
-    plugin.IsCustom = true
-}
+// Derive IsCustom from Path consistently
+if plugin.Path != nil && strings.TrimSpace(*plugin.Path) != "" {
+    plugin.IsCustom = true
+} else {
+    plugin.IsCustom = false
+}
📝 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
// Mark plugin as custom if path is not empty
if plugin.Path != nil && *plugin.Path != "" {
plugin.IsCustom = true
}
// Derive IsCustom from Path consistently
if plugin.Path != nil && strings.TrimSpace(*plugin.Path) != "" {
plugin.IsCustom = true
} else {
plugin.IsCustom = false
}
🤖 Prompt for AI Agents 
In framework/configstore/rdb.go around lines 873-877, the code sets
plugin.IsCustom = true when plugin.Path is non-nil and non-empty; apply the same
deterministic derivation inside UpdatePlugin so updates can't diverge: in the
UpdatePlugin implementation recompute plugin.IsCustom = (plugin.Path != nil &&
*plugin.Path != "") before saving (and handle the false case explicitly),
ensuring the updated record persists the derived IsCustom value consistently.

if err := txDB.WithContext(ctx).Delete(&tables.TablePlugin{}, "name = ?", plugin.Name).Error; err != nil {
if localTx {
txDB.Rollback()
Expand Down
2 changes: 1 addition & 1 deletion framework/configstore/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type ConfigStore interface {
UpdateConfig(ctx context.Context, config *tables.TableConfig, tx ...*gorm.DB) error

// Plugins CRUD
GetPlugins(ctx context.Context) ([]tables.TablePlugin, error)
GetPlugins(ctx context.Context) ([]*tables.TablePlugin, error)
GetPlugin(ctx context.Context, name string) (*tables.TablePlugin, error)
CreatePlugin(ctx context.Context, plugin *tables.TablePlugin, tx ...*gorm.DB) error
UpdatePlugin(ctx context.Context, plugin *tables.TablePlugin, tx ...*gorm.DB) error
Expand Down
1 change: 1 addition & 0 deletions framework/configstore/tables/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type TablePlugin struct {
ConfigJSON string `gorm:"type:text" json:"-"` // JSON serialized plugin.Config
CreatedAt time.Time `gorm:"index;not null" json:"created_at"`
UpdatedAt time.Time `gorm:"index;not null" json:"updated_at"`
IsCustom bool `gorm:"default:false" json:"isCustom"`

// Virtual fields for runtime use (not stored in DB)
Config any `gorm:"-" json:"config,omitempty"`
Expand Down
3 changes: 3 additions & 0 deletions framework/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/redis/go-redis/v9 v9.14.0 h1:u4tNCjXOyzfgeLN+vAZaW1xUooqWDqVEsZN0U01jfAE=
github.com/redis/go-redis/v9 v9.14.0/go.mod h1:huWgSWd8mW6+m0VPhJjSSQ+d6Nh1VICQ6Q5lHuCH/Iw=
github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc=
github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0=
Expand All @@ -188,7 +189,9 @@ github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyC
github.com/valyala/fasthttp v1.67.0 h1:tqKlJMUP6iuNG8hGjK/s9J4kadH7HLV4ijEcPGsezac=
github.com/valyala/fasthttp v1.67.0/go.mod h1:qYSIpqt/0XNmShgo/8Aq8E3UYWVVwNS2QYmzd8WIEPM=
github.com/weaviate/weaviate v1.33.1 h1:fV69ffJSH0aO3LvLiKYlVZ8wFa94oQ1g3uMyZGTb838=
github.com/weaviate/weaviate v1.33.1/go.mod h1:SnxXSIoiusZttZ/gI9knXhFAu0UYqn9N/ekgsNnXbNw=
github.com/weaviate/weaviate-go-client/v5 v5.5.0 h1:+5qkHodrL3/Qc7kXvMXnDaIxSBN5+djivLqzmCx7VS4=
github.com/weaviate/weaviate-go-client/v5 v5.5.0/go.mod h1:Zdm2MEXG27I0Nf6fM0FZ3P2vLR4JM0iJZrOxwc+Zj34=
github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc=
github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw=
github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU=
Expand Down
2 changes: 1 addition & 1 deletion plugins/otel/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ configs:
content: |
server:
http_listen_port: 3200
grpc_listen_port: 3200
grpc_listen_port: 3201
log_level: info

distributor:
Expand Down
2 changes: 1 addition & 1 deletion transports/bifrost-http/handlers/inference.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (h *CompletionHandler) textCompletion(ctx *fasthttp.RequestCtx) {
// chatCompletion handles POST /v1/chat/completions - Process chat completion requests
func (h *CompletionHandler) chatCompletion(ctx *fasthttp.RequestCtx) {
var req ChatRequest
if err := sonic.Unmarshal(ctx.PostBody(), &req); err != nil {
if err := sonic.Unmarshal(ctx.PostBody(), &req); err != nil {
SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("Invalid request format: %v", err), h.logger)
return
}
Expand Down
89 changes: 79 additions & 10 deletions transports/bifrost-http/handlers/plugins.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
type PluginsLoader interface {
ReloadPlugin(ctx context.Context, name string, path *string, pluginConfig any) error
RemovePlugin(ctx context.Context, name string) error
GetPluginStatus() []schemas.PluginStatus
}

// PluginsHandler is the handler for the plugins API
Expand All @@ -41,11 +42,13 @@ type CreatePluginRequest struct {
Name string `json:"name"`
Enabled bool `json:"enabled"`
Config map[string]any `json:"config"`
Path *string `json:"path"`
}

// UpdatePluginRequest is the request body for updating a plugin
type UpdatePluginRequest struct {
Enabled bool `json:"enabled"`
Path *string `json:"path"`
Config map[string]any `json:"config"`
}

Expand All @@ -66,10 +69,64 @@ func (h *PluginsHandler) getPlugins(ctx *fasthttp.RequestCtx) {
SendError(ctx, 500, "Failed to retrieve plugins", h.logger)
return
}

// Fetching status
pluginStatus := h.pluginsLoader.GetPluginStatus()
// Creating ephemeral struct for the plugins
finalPlugins := []struct{
Name string `json:"name"`
Enabled bool `json:"enabled"`
Config any `json:"config"`
IsCustom bool `json:"isCustom"`
Path *string `json:"path"`
Status schemas.PluginStatus `json:"status"`
}{}
// Iterating over plugin status to get the plugin info
for _,pluginStatus := range pluginStatus {
var pluginInfo *configstoreTables.TablePlugin
for _, plugin := range plugins {
if plugin.Name == pluginStatus.Name {
pluginInfo = plugin
break
}
}
if pluginInfo == nil {
finalPlugins = append(finalPlugins, struct{
Name string `json:"name"`
Enabled bool `json:"enabled"`
Config any `json:"config"`
IsCustom bool `json:"isCustom"`
Path *string `json:"path"`
Status schemas.PluginStatus `json:"status"`
}{
Name: pluginStatus.Name,
Enabled: pluginStatus.Status != schemas.PluginStatusDisabled,
Config: map[string]any{},
IsCustom: false,
Path: nil,
Status: pluginStatus,
})
continue
}
finalPlugins = append(finalPlugins, struct{
Name string `json:"name"`
Enabled bool `json:"enabled"`
Config any `json:"config"`
IsCustom bool `json:"isCustom"`
Path *string `json:"path"`
Status schemas.PluginStatus `json:"status"`
}{
Name: pluginInfo.Name,
Enabled: pluginInfo.Enabled,
Config: pluginInfo.Config,
IsCustom: pluginInfo.IsCustom,
Path: pluginInfo.Path,
Status: pluginStatus,
})
}
// Creating ephemeral struct
SendJSON(ctx, map[string]any{
"plugins": plugins,
"count": len(plugins),
"plugins": finalPlugins,
"count": len(plugins),
}, h.logger)
Comment on lines +72 to 130
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix count inconsistency and reduce duplication.

Two issues:

  1. Count mismatch: Line 129 returns len(plugins) (database plugin count) but the response contains finalPlugins (runtime-merged plugins). These counts can differ if runtime plugins aren't persisted. The count should match what's returned.

  2. Duplicate struct definitions: The anonymous struct at lines 75-82, 93-100, and 110-117 is repeated three times. Extract to a named type to improve maintainability.

Apply this diff to fix the count:

 	SendJSON(ctx, map[string]any{
 		"plugins": finalPlugins,
-		"count":   len(plugins),		
+		"count":   len(finalPlugins),		
 	}, h.logger)

To reduce duplication, define a named type before the function:

type PluginWithStatus struct {
	Name     string                 `json:"name"`
	Enabled  bool                   `json:"enabled"`
	Config   any                    `json:"config"`
	IsCustom bool                   `json:"isCustom"`
	Path     *string                `json:"path"`
	Status   schemas.PluginStatus   `json:"status"`
}

Then replace the anonymous struct with PluginWithStatus throughout lines 75-124.

🤖 Prompt for AI Agents 
In transports/bifrost-http/handlers/plugins.go around lines 72 to 130, the
response uses an anonymous struct repeated three times and returns count as
len(plugins) which can mismatch the runtime-merged finalPlugins; define a named
type PluginWithStatus (with fields Name, Enabled, Config, IsCustom, Path
*string, Status schemas.PluginStatus) at top of the file or immediately before
the handler, replace all anonymous struct usages with PluginWithStatus when
building finalPlugins, and change the returned count to len(finalPlugins) so the
count matches the response.

}

Expand Down Expand Up @@ -134,6 +191,8 @@ func (h *PluginsHandler) createPlugin(ctx *fasthttp.RequestCtx) {
Name: request.Name,
Enabled: request.Enabled,
Config: request.Config,
Path: request.Path,
IsCustom: true,
}); err != nil {
h.logger.Error("failed to create plugin: %v", err)
SendError(ctx, 500, "Failed to create plugin", h.logger)
Expand All @@ -149,7 +208,7 @@ func (h *PluginsHandler) createPlugin(ctx *fasthttp.RequestCtx) {

// We reload the plugin if its enabled
if request.Enabled {
if err := h.pluginsLoader.ReloadPlugin(ctx, request.Name, nil, request.Config); err != nil {
if err := h.pluginsLoader.ReloadPlugin(ctx, request.Name, request.Path, request.Config); err != nil {
h.logger.Error("failed to load plugin: %v", err)
SendJSON(ctx, map[string]any{
"message": fmt.Sprintf("Plugin created successfully; but failed to load plugin with new config: %v", err),
Expand Down Expand Up @@ -188,16 +247,21 @@ func (h *PluginsHandler) updatePlugin(ctx *fasthttp.RequestCtx) {
SendError(ctx, 400, "Empty 'name' parameter not allowed", h.logger)
return
}

var plugin *configstoreTables.TablePlugin
var err error
// Check if plugin exists
if _, err := h.configStore.GetPlugin(ctx, name); err != nil {
plugin, err = h.configStore.GetPlugin(ctx, name)
if err != nil {
// If doesn't exist, create it
if errors.Is(err, configstore.ErrNotFound) {
if err := h.configStore.CreatePlugin(ctx, &configstoreTables.TablePlugin{
plugin = &configstoreTables.TablePlugin{
Name: name,
Enabled: false,
Config: map[string]any{},
}); err != nil {
Path: nil,
IsCustom: true,
}
Comment on lines 195 to +263
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Inconsistent auto-creation: IsCustom true but Path nil.

When auto-creating a missing plugin, the code sets IsCustom: true but Path: nil (line 261-262). This creates an inconsistent state where the plugin is marked as custom without a path.

Consider either:

  1. Set IsCustom: false for auto-created plugins (since they have no path yet)
  2. Or rely on the deterministic IsCustom derivation in rdb.go (once that's fixed)
 			plugin = &configstoreTables.TablePlugin{
 				Name:    name,
 				Enabled: false,
 				Config:  map[string]any{},
 				Path:    nil,
-				IsCustom: true,
+				IsCustom: false,  // No path means not custom
 			}
🤖 Prompt for AI Agents 
transports/bifrost-http/handlers/plugins.go around lines 256 to 263: the
auto-created missing-plugin sets IsCustom: true while Path is nil, producing an
inconsistent state; change the auto-creation to set IsCustom: false for these
generated entries (leave Path as nil) so the plugin isn’t marked custom until a
real path is provided or until IsCustom is deterministically derived elsewhere.

if err := h.configStore.CreatePlugin(ctx, plugin); err != nil {
h.logger.Error("failed to create plugin: %v", err)
SendError(ctx, 500, "Failed to create plugin", h.logger)
return
Expand All @@ -209,24 +273,28 @@ func (h *PluginsHandler) updatePlugin(ctx *fasthttp.RequestCtx) {
}
}

// Unmarshalling the request body
var request UpdatePluginRequest
if err := json.Unmarshal(ctx.PostBody(), &request); err != nil {
h.logger.Error("failed to unmarshal update plugin request: %v", err)
SendError(ctx, 400, "Invalid request body", h.logger)
return
}

// Updating the plugin
if err := h.configStore.UpdatePlugin(ctx, &configstoreTables.TablePlugin{
Name: name,
Enabled: request.Enabled,
Config: request.Config,
Path: request.Path,
IsCustom: plugin.IsCustom,
}); err != nil {
h.logger.Error("failed to update plugin: %v", err)
SendError(ctx, 500, "Failed to update plugin", h.logger)
return
}

plugin, err := h.configStore.GetPlugin(ctx, name)
plugin, err = h.configStore.GetPlugin(ctx, name)
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
SendError(ctx, fasthttp.StatusNotFound, "Plugin not found", h.logger)
Expand All @@ -238,7 +306,7 @@ func (h *PluginsHandler) updatePlugin(ctx *fasthttp.RequestCtx) {
}
// We reload the plugin if its enabled, otherwise we stop it
if request.Enabled {
if err := h.pluginsLoader.ReloadPlugin(ctx, name, nil, request.Config); err != nil {
if err := h.pluginsLoader.ReloadPlugin(ctx, name, request.Path, request.Config); err != nil {
h.logger.Error("failed to load plugin: %v", err)
SendJSON(ctx, map[string]any{
"message": fmt.Sprintf("Plugin updated successfully; but failed to load plugin with new config: %v", err),
Expand All @@ -247,6 +315,7 @@ func (h *PluginsHandler) updatePlugin(ctx *fasthttp.RequestCtx) {
return
}
} else {
ctx.SetUserValue("isDisabled", true)
if err := h.pluginsLoader.RemovePlugin(ctx, name); err != nil {
h.logger.Error("failed to stop plugin: %v", err)
SendJSON(ctx, map[string]any{
Expand Down
Loading