fix: force file-wins sync for plugins, governance entities, and client config when source_of_truth=config.json - #4381
Merged
akshaydeo merged 1 commit intoJun 15, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR extends the config.json source-of-truth reconciliation by introducing a forced-sync pattern that overrides stored entity state from file data despite matching ConfigHash values. It applies this pattern across client config, governance entities (budgets, rate limits, customers, teams, virtual keys, routing rules, pricing overrides, model configs, and provider bindings), refines database update queries for budget unlinking, switches plugin sync to update-only semantics, and adds comprehensive tests for plugin override behavior. ChangesConfig reconciliation with forced sync and refined persistence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BearTS
marked this pull request as ready for review
June 15, 2026 05:29
BearTS
changed the title
UpdatePlugin instead of UpsertPlugin when syncing plugins from config.json source of truth
Contributor
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to the The No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix: use updatePlugin when source of tru..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
transports/bifrost-http/lib/config.go (1)
3844-3857:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve DB-backed plugin metadata before replacing the row.
UpdatePlugindeletes/recreates the row, so this now overwrites DB-backedversion,placement, andorderwith config.json/default values. Undersource_of_truth=config.json, only file-owned fields likeenabledandconfigshould override DB state; preserve existing metadata when buildingtablePlugin.Suggested direction
+ existingByName := make(map[string]*configstoreTables.TablePlugin, len(existing)) for _, plugin := range existing { + if plugin != nil { + existingByName[plugin.Name] = plugin + } if plugin != nil && !keep[plugin.Name] { if err := config.ConfigStore.DeletePlugin(ctx, plugin.Name, tx); err != nil { return fmt.Errorf("failed to delete plugin %s: %w", plugin.Name, err) } } @@ - if plugin.Version == nil { - plugin.Version = bifrost.Ptr(int16(1)) - } + version := int16(1) + var placement *schemas.PluginPlacement + var order *int + if existingPlugin := existingByName[plugin.Name]; existingPlugin != nil { + version = existingPlugin.Version + placement = existingPlugin.Placement + order = existingPlugin.Order + } tablePlugin := &configstoreTables.TablePlugin{ Name: plugin.Name, Enabled: plugin.Enabled, Config: pluginConfigCopy, Path: plugin.Path, - Version: *plugin.Version, - Placement: plugin.Placement, - Order: plugin.Order, + Version: version, + Placement: placement, + Order: order, }As per coding guidelines,
transports/config.schema.jsonis the source of truth and pluginversion,placement, andorderare DB-backed-only metadata while config.json overrides should focus onenabledandconfig.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/bifrost-http/lib/config.go` around lines 3844 - 3857, The UpdatePlugin call in the plugin configuration block is overwriting database-backed metadata (version, placement, and order) with config.json values, which violates the source of truth principle. Before constructing the tablePlugin struct, fetch the existing plugin record from the database to retrieve its current version, placement, and order values. When building the tablePlugin struct around line 3848-3857, use the preserved database values for the Version, Placement, and Order fields instead of the values from the config.json plugin object; only allow config.json to override the Enabled and Config fields. This ensures that DB-backed metadata is preserved while config.json changes are correctly applied only to file-owned fields.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/bifrost-http/lib/config_test.go`:
- Around line 13016-13051: Add four new test functions following the pattern of
TestSourceOfTruthConfigJSON_PluginsPresentFileOverridesDB to cover additional
edge cases for the source_of_truth=config.json reconciliation behavior. Create
test functions for: (1) file version greater than DB version to verify file
override still applies, (2) file version equal to DB version to ensure correct
behavior when versions match, (3) plugin existing in file but not in DB to
validate new plugin creation, and (4) plugin existing in DB but not in file to
confirm the plugin is preserved and not deleted. Each test should follow the
same setup pattern with loadPlugins and appropriate assertions to verify the
expected behavior in each scenario.
- Around line 1271-1278: Combine the two separate loops iterating through
m.plugins (the version check loop starting with the comparison of plugin.Version
and the subsequent filter-rebuild loop) into a single loop to reduce iteration
overhead. Within this unified loop, perform both the version check (returning
nil if plugin.Version is less than p.Version) and the filter-rebuild operations
in one pass through the m.plugins collection. This optimization maintains the
same logic and behavior while eliminating the duplicate iteration.
---
Outside diff comments:
In `@transports/bifrost-http/lib/config.go`:
- Around line 3844-3857: The UpdatePlugin call in the plugin configuration block
is overwriting database-backed metadata (version, placement, and order) with
config.json values, which violates the source of truth principle. Before
constructing the tablePlugin struct, fetch the existing plugin record from the
database to retrieve its current version, placement, and order values. When
building the tablePlugin struct around line 3848-3857, use the preserved
database values for the Version, Placement, and Order fields instead of the
values from the config.json plugin object; only allow config.json to override
the Enabled and Config fields. This ensures that DB-backed metadata is preserved
while config.json changes are correctly applied only to file-owned fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 053c73a1-c713-468c-b383-0ff6d8aba789
📒 Files selected for processing (2)
transports/bifrost-http/lib/config.gotransports/bifrost-http/lib/config_test.go
BearTS
force-pushed
the
06-14-fix_use_updateplugin_when_source_of_truth_is_config_json
branch
from
June 15, 2026 07:43
0810a22 to
0250183
Compare
BearTS
changed the title
UpdatePlugin instead of UpsertPlugin when syncing plugins from config.json source of truthsource_of_truth=config.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/bifrost-http/lib/config_test.go`:
- Around line 13012-13176: Consolidate the five separate test functions
(TestSourceOfTruthConfigJSON_PluginsPresentFileOverridesDB,
TestSourceOfTruthConfigJSON_FileVersionGreaterThanDB,
TestSourceOfTruthConfigJSON_FileVersionEqualToDBVersion,
TestSourceOfTruthConfigJSON_PluginInFileNotInDB, and
TestSourceOfTruthConfigJSON_PluginInDBNotInFile) into a single table-driven
test. Define a test case struct containing the input parameters (DB plugins,
file plugin config, source of truth) and expected output assertions (enabled
status, config values, store updates). Create a slice of test cases representing
each scenario, then iterate through them with a loop that calls loadPlugins and
verifies all assertions for each case. This eliminates the repeated setup/assert
blocks and makes adding new scenarios simpler and safer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a3dd045-d276-4834-b7f9-826ba0a6c030
📒 Files selected for processing (2)
transports/bifrost-http/lib/config.gotransports/bifrost-http/lib/config_test.go
BearTS
force-pushed
the
06-14-fix_use_updateplugin_when_source_of_truth_is_config_json
branch
from
June 15, 2026 07:54
0250183 to
fefb0f2
Compare
Contributor
Merge activity
|
akshaydeo
deleted the
06-14-fix_use_updateplugin_when_source_of_truth_is_config_json
branch
June 15, 2026 09:10
akshaydeo
pushed a commit
that referenced
this pull request
Jun 15, 2026
…t config when `source_of_truth=config.json` (#4381) ## Summary When `source_of_truth=config.json`, the config file should always be the authoritative source for plugins and governance entities, regardless of what is stored in the database. Previously, UI/API edits could cause drift that persisted across restarts because hash comparisons incorrectly treated a matching `ConfigHash` as proof the DB row was unchanged — even though `ConfigHash` is not updated on UI/API edits. Additionally, `UpsertPlugin` was being called during file sync, allowing a higher-versioned DB record to silently win over the file definition. This PR introduces a `forceFileSync` / `forceClientSync` flag for the `source_of_truth=config.json` path and switches to `UpdatePlugin` for the file sync path to enforce file authority unconditionally. ## Changes - Introduced `forceClientSync` in `loadClientConfig` so that when `source_of_truth=config.json` and the `client` section is present in the file, the file always wins regardless of the stored `ConfigHash`. - Introduced `forceFileSync` in `mergeGovernanceConfig` so that all governance entities (budgets, rate limits, customers, teams, virtual keys, routing rules, pricing overrides, model configs) are re-synced from the file when `source_of_truth=config.json`, bypassing hash comparison. - Replaced `UpsertPlugin` with `UpdatePlugin` in `syncPluginsFromFile` so that file-defined plugins always overwrite DB state without version comparison. - Switched `Update` to `UpdateColumn` for `customer_id` budget unlinking in `updateGovernanceConfigInStore` and `linkCustomerBudgetID` to avoid unintended GORM hook side effects. - Implemented `UpdatePlugin` in `MockConfigStore` to properly replace the matching plugin by name. - Implemented `UpsertPlugin` in `MockConfigStore` with correct version-gating logic (skips update if the incoming version is lower than the stored version), making it accurate for non-file-sync paths. - Added tests covering file-overrides-DB scenarios for plugins under `source_of_truth=config.json`: lower file version, higher file version, equal version, plugin only in file, and plugin only in DB. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./transports/bifrost-http/lib/... -run TestSourceOfTruth ``` The new tests confirm that file-defined plugins and governance entities always override DB state when `source_of_truth=config.json`, regardless of version numbers or hash matches. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only affects how configuration authority is resolved between the config file and the database. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
18 tasks
Closed
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
source_of_truth=config.json, the config file should always be the authoritative source for plugins and governance entities, regardless of what is stored in the database. Previously, UI/API edits could cause drift that persisted across restarts because hash comparisons incorrectly treated a matchingConfigHashas proof the DB row was unchanged — even thoughConfigHashis not updated on UI/API edits. Additionally,UpsertPluginwas being called during file sync, allowing a higher-versioned DB record to silently win over the file definition. This PR introduces aforceFileSync/forceClientSyncflag for thesource_of_truth=config.jsonpath and switches toUpdatePluginfor the file sync path to enforce file authority unconditionally.Changes
forceClientSyncinloadClientConfigso that whensource_of_truth=config.jsonand theclientsection is present in the file, the file always wins regardless of the storedConfigHash.forceFileSyncinmergeGovernanceConfigso that all governance entities (budgets, rate limits, customers, teams, virtual keys, routing rules, pricing overrides, model configs) are re-synced from the file whensource_of_truth=config.json, bypassing hash comparison.UpsertPluginwithUpdatePlugininsyncPluginsFromFileso that file-defined plugins always overwrite DB state without version comparison.UpdatetoUpdateColumnforcustomer_idbudget unlinking inupdateGovernanceConfigInStoreandlinkCustomerBudgetIDto avoid unintended GORM hook side effects.UpdatePlugininMockConfigStoreto properly replace the matching plugin by name.UpsertPlugininMockConfigStorewith correct version-gating logic (skips update if the incoming version is lower than the stored version), making it accurate for non-file-sync paths.source_of_truth=config.json: lower file version, higher file version, equal version, plugin only in file, and plugin only in DB.Type of change
Affected areas
How to test
go test ./transports/bifrost-http/lib/... -run TestSourceOfTruthThe new tests confirm that file-defined plugins and governance entities always override DB state when
source_of_truth=config.json, regardless of version numbers or hash matches.Screenshots/Recordings
N/A
Breaking changes
Related issues
Security considerations
No security implications. This change only affects how configuration authority is resolved between the config file and the database.
Checklist
docs/contributing/README.mdand followed the guidelines