fix: use providerCfgInFile instead of existingCfg when preserving failed provider config

[BearTS]

Summary

When processing an authoritative provider fails, the provider's file-based config should still be written to the store rather than silently falling back to the previously persisted config. The old behavior preserved the existing store entry on error, which could mask bad config and prevent updates from taking effect.

Changes

• On a failed processAuthoritativeProvider call, the provider entry in authoritativeProviders is now set to providerCfgInFile (the config as read from the file) instead of existingCfg (the previously persisted config).
• This ensures that a malformed or invalid provider config in the file is surfaced and written through, rather than silently falling back to stale data that could hide the problem.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

Deploy a configuration with a provider entry that triggers a processing error and verify that the provider's config in the store reflects the file-based config rather than the previously persisted value.

go test ./...

Screenshots/Recordings

N/A

Breaking changes

• Yes
• No

Related issues

N/A

Security considerations

No direct security implications. Ensuring the file-based config is authoritative prevents stale or unintended provider configs from persisting silently in the store.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

Summary by CodeRabbit

• Refactor

  • Authoritative provider entries from config files are now always processed during reconciliation; validation issues are logged as warnings and no longer stop processing. Missing key IDs are generated and provider/key data normalized.

• Bug Fix

  • When a provider already exists, keys from config are merged with stored keys while preserving the provider's existing status and description to avoid accidental pruning.

[BearTS]

• fix: use providerCfgInFile instead of existingCfg when preserving failed provider config #4258 👈 (View in Graphite)
• fix: generate UUID for virtual key if ID is empty before creation #4248
• dev

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

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b21121a7-ea1b-4064-8bcf-e53dfe118a98

📥 Commits

Reviewing files that changed from the base of the PR and between fd682f5 and ffef137.

📒 Files selected for processing (1)

• transports/bifrost-http/lib/config.go

---

📝 Walkthrough

Walkthrough

Reconciliation now always builds authoritative provider entries from config.json. processAuthoritativeProvider no longer returns errors; validation failures are logged as warnings and processing (normalization, key ID generation, hash computation, and merging with stored provider fields) continues.

Changes

Provider Configuration Reconciliation

Layer / File(s)	Summary
Source-of-truth provider config reconciliation transports/bifrost-http/lib/config.go	In source_of_truth=config.json mode, loadProviders now invokes processAuthoritativeProvider unconditionally (no error handling/fallback to existingCfg), making file entries authoritative during reconciliation.
processAuthoritativeProvider signature and validation changes transports/bifrost-http/lib/config.go	processAuthoritativeProvider no longer returns an error; prior custom-provider and per-key alias validations in the authoritative path now emit warnings and do not abort processing. It still normalizes names, ensures/generates key IDs, computes provider config hash, and merges keys/status/description when a stored provider exists.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

• akshaydeo

Poem

I nibble at lines where configs meet the sun,
A file now rules where stored echoes run,
Warnings whisper as names and keys are made,
Old statuses kept while new keys parade,
🐇📄✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: using providerCfgInFile instead of existingCfg when preserving failed provider config, which is the core fix in the changeset.
Description check	✅ Passed	The description follows the template with complete sections including Summary, Changes, Type, Affected areas, How to test, Breaking changes, and Security considerations, providing clear context for the bug fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 06-10-feat_overwrite_even_if_not_valid_fields

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 @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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.go`:
- Around line 1162-1164: When processAuthoritativeProvider(providerName,
providerCfgInFile, existingCfg, exists, authoritativeProviders) returns an
error, don't stash the raw providerCfgInFile into authoritativeProviders as-is;
instead merge the identifying fields and metadata from existingCfg (key IDs,
each key's ID, Status, Description, and any provider-level metadata) into the
fallback before assigning authoritativeProviders[provider] = providerCfgInFile
so that keepKeys (built later in syncAuthoritativeProvidersInStore) retains
existing key IDs and provider/key Status/Description; implement this merge right
inside the error branch where logger.Warn is called, referencing
processAuthoritativeProvider, providerCfgInFile, existingCfg,
authoritativeProviders, and syncAuthoritativeProvidersInStore to ensure
preserved IDs and metadata are copied into the fallback entry.

🪄 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: 1e242039-fd57-4e8a-89b0-0bd7b48388fe

📥 Commits

Reviewing files that changed from the base of the PR and between 223f010 and 972dba0.

📒 Files selected for processing (1)

• transports/bifrost-http/lib/config.go

[greptile-apps]

Confidence Score: 5/5

Safe to merge. The change is narrow, the write-through intent is consistent across all code paths, and key/status/description merging with existing DB entries is preserved correctly.

The refactor is minimal and well-scoped: removing the error-return from processAuthoritativeProvider and dropping the existingCfg fallback at the call site. The mergeProviderKeys / status / description merge logic that protects existing DB metadata is left intact. The previous review threads confirmed the write-through behavior is intentional and no new issues are introduced.

No files require special attention.

Important Files Changed

Filename	Overview
transports/bifrost-http/lib/config.go	processAuthoritativeProvider changed to void: validation errors now log warnings and write through instead of short-circuiting. Call site removes the existingCfg fallback. Logic is consistent and correct for the declared write-through intent.

_{Reviews (7): Last reviewed commit: "feat: overwrite even if not valid fields" | Re-trigger Greptile}

[coderabbitai]

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.go`:
- Around line 1162-1163: The call to processAuthoritativeProvider currently
writes provider entries without validating CustomProviderConfig or key aliases,
allowing malformed input into config.json; restore provider-level validation by
invoking the same validation logic that was removed (validate
CustomProviderConfig structure and each key alias) before persisting and add a
failure path that logs/returns a warning/error for that provider so it is not
written-through on invalid input; update processAuthoritativeProvider (and its
callers where providerName and providerCfgInFile are passed) to perform these
checks and produce clear validation warnings (or skip/purge the provider entry)
while preserving write-through behavior for valid providers.

🪄 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: 8dda7e40-b4e2-4241-a913-17b0cafb5aee

📥 Commits

Reviewing files that changed from the base of the PR and between 972dba0 and d621fa7.

📒 Files selected for processing (1)

• transports/bifrost-http/lib/config.go

[coderabbitai]

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.go`:
- Around line 2764-2766: The UUID for new file-defined virtual keys is being
assigned after virtualKeysToAdd has already been appended to
config.GovernanceConfig.VirtualKeys in mergeGovernanceConfig, so the live
in-memory config still contains entries with ID == "". Move the assignment of
virtualKey.ID = uuid.NewString() into the code path that constructs/populates
virtualKeysToAdd (i.e., when you create each virtualKey before you append it to
the slice in mergeGovernanceConfig), ensuring every virtual key has an ID prior
to append; retain the current check (if virtualKey.ID == "" { virtualKey.ID =
uuid.NewString() }) only as a defensive assert in the later function to cover
any unexpected cases.

🪄 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: 21fe24ae-1dbf-4a23-9fef-ac3757c5bfca

📥 Commits

Reviewing files that changed from the base of the PR and between d621fa7 and e90f7d4.

📒 Files selected for processing (1)

• transports/bifrost-http/lib/config.go

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

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.go`:
- Around line 2764-2766: The UUID for new file-defined virtual keys is being
assigned after virtualKeysToAdd has already been appended to
config.GovernanceConfig.VirtualKeys in mergeGovernanceConfig, so the live
in-memory config still contains entries with ID == "". Move the assignment of
virtualKey.ID = uuid.NewString() into the code path that constructs/populates
virtualKeysToAdd (i.e., when you create each virtualKey before you append it to
the slice in mergeGovernanceConfig), ensuring every virtual key has an ID prior
to append; retain the current check (if virtualKey.ID == "" { virtualKey.ID =
uuid.NewString() }) only as a defensive assert in the later function to cover
any unexpected cases.

🪄 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: 21fe24ae-1dbf-4a23-9fef-ac3757c5bfca

📥 Commits

Reviewing files that changed from the base of the PR and between d621fa7 and e90f7d4.

📒 Files selected for processing (1)

• transports/bifrost-http/lib/config.go

🛑 Comments failed to post (1)

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

2764-2766: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Assign the virtual-key ID before the merge result is copied into in-memory config.

mergeGovernanceConfig appends virtualKeysToAdd into config.GovernanceConfig.VirtualKeys before this function runs, so generating the UUID here only mutates the local virtualKeysToAdd copy. The live in-memory governance config still carries ID == "" for newly added file-defined virtual keys until a later restart/name-match reconcile fixes it. Generate the ID when building virtualKeysToAdd (before the append at Line 2334), and keep this branch as a defensive assert at most.

🤖 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 2764 - 2766, The UUID for
new file-defined virtual keys is being assigned after virtualKeysToAdd has
already been appended to config.GovernanceConfig.VirtualKeys in
mergeGovernanceConfig, so the live in-memory config still contains entries with
ID == "". Move the assignment of virtualKey.ID = uuid.NewString() into the code
path that constructs/populates virtualKeysToAdd (i.e., when you create each
virtualKey before you append it to the slice in mergeGovernanceConfig), ensuring
every virtual key has an ID prior to append; retain the current check (if
virtualKey.ID == "" { virtualKey.ID = uuid.NewString() }) only as a defensive
assert in the later function to cover any unexpected cases.

[BearTS]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Pratham-Mishra04]

Merge activity

• Jun 11, 7:43 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 11, 7:46 AM UTC: Graphite rebased this pull request as part of a merge.
• Jun 11, 7:48 AM UTC: Graphite rebased this pull request as part of a merge.
• Jun 11, 7:48 AM UTC: @Pratham-Mishra04 merged this pull request with Graphite.