fix: supports backup for governance migration

[roroghost17]

Summary

Closed because all models support is being added in this release. So, there won't be any case to what this PR was fixing

When migrating provider governance to model configs, a conflict can occur if a user-created (global, provider, '*') wildcard row already occupies the slot that the migration would otherwise create. In that case, the provider's own budget and rate-limit rows become stranded — detached from the provider FK but not folded into any model config. This PR adds a backup mechanism that snapshots those stranded rows into governance_provider_budgets_backup and governance_provider_rate_limits_backup before the FKs are cleared, giving support a data record from which governance can be restored on request.

Changes

• When a wildcard conflict is detected during migrationMigrateProviderGovernanceToModelConfigs, the provider's existing budget and rate-limit rows are snapshotted into backup tables (governance_provider_budgets_backup / governance_provider_rate_limits_backup) keyed by provider_name.
• Backup tables are created on demand using CREATE TABLE IF NOT EXISTS ... WHERE 1 = 0 so they only exist when a conflict actually occurs.
• The backup runs inside a savepoint so that a failure (e.g. a pre-existing incompatible backup table) is rolled back and logged without blocking the migration — the provider FKs are still cleared and the pre-existing wildcard row is left untouched.
• Removed an extraneous blank line in migrations.go.
• Added three new test cases:
  • Asserts backup tables are not created when no conflict occurs.
  • Asserts that stranded governance rows are correctly snapshotted when a conflict occurs, the pre-existing wildcard row is untouched, provider FKs are cleared, and re-running the migration does not duplicate backup rows.
  • Asserts that a backup failure (simulated by a pre-existing incompatible backup table) is non-blocking: the migration still succeeds, the savepoint rollback discards the failed attempt, and the wildcard row and provider FK state are correct.

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

go test ./framework/configstore/... -run TestMigrationMigrateProviderGovernanceToModelConfigs

The three relevant test functions are:

• TestMigrationMigrateProviderGovernanceToModelConfigs — verifies backup tables are absent when no conflict occurs.
• TestMigrationMigrateProviderGovernanceToModelConfigsConflictBackup — verifies correct snapshotting on conflict and idempotency.
• TestMigrationMigrateProviderGovernanceToModelConfigsBackupFailureNonBlocking — verifies a failing backup does not abort the migration.

Breaking changes

• Yes
• No

Security considerations

Backup tables contain budget and rate-limit configuration data (limits, durations). No credentials or PII are stored. Tables are only created inside the existing migration transaction when a conflict is detected, and are scoped to the same database as the rest of the config 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

• Bug Fixes
  • Improved provider governance configuration migration to better preserve data during conflicts and prevent relationship loss.
  • Enhanced migration robustness to continue successfully even if backup steps encounter issues.
• Tests
  • Added comprehensive test coverage for provider governance migration conflict scenarios and failure recovery.

[coderabbitai]

Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range.

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Migration conflict handling enhanced to preserve stranded governance relationships. When a user-created wildcard model config conflicts with provider migration, the code now creates savepoint-backed snapshots of governance budgets and rate limits into provider-keyed backup tables, logs failures non-blockingly, and rolls back on error instead of abandoning data preservation attempts.

Changes

Provider Governance Migration Conflict Handling with Backup

Layer / File(s)	Summary
Conflict-handling and backup implementation framework/configstore/migrations.go	Migration conflict path updated to conditionally snapshot stranded config_providers governance rows into governance_provider_budgets_backup and governance_provider_rate_limits_backup tables keyed by provider_name; uses savepoint to ensure rollback on backup failures, logs errors instead of blocking migration, and clears provider foreign keys regardless of backup success. Documentation comments explain the conditional backup and non-blocking skip behavior.
Test coverage for conflict and failure scenarios framework/configstore/migrations_test.go	No-conflict cases verified to not create backup tables. Conflict cases verify pre-existing wildcard model configs remain unchanged, governance rows are snapshotted into backup tables, and re-runs do not duplicate rows. Backup schema sabotage test confirms migration still succeeds, foreign keys clear, wildcard row persists, and failed backup rows do not persist (savepoint rollback behavior).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• maximhq/bifrost#4093: Both modify migrationMigrateProviderGovernanceToModelConfigs handling of existing wildcard provider governance rows; this PR adds savepoint-backed backup behavior while #4093 makes the migration idempotent for missing foreign keys.
• maximhq/bifrost#3938: Introduced the provider-governance-to-model-config migration that this PR extends with conflict backup preservation.

Poem

🐰 A rabbit hops through conflict zones with care,
Savepoints and backups floating in the air,
When wildcard rows block the migration's way,
We snapshot and rollback—no data shall stray! 🌾

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'fix: supports backup for governance migration' clearly summarizes the main change—adding backup support to the governance migration process.
Description check	✅ Passed	The PR description comprehensively covers the summary, changes, type, affected areas, testing instructions, and security considerations as specified in the template.
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-06-fix_supports_backup_for_governance_migration

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.}

[CLAassistant]

All committers have signed the CLA.

[roroghost17]

• fix: supports backup for governance migration #4101 👈 (View in Graphite)
• dev

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

[greptile-apps]

Confidence Score: 1/5

The PR introduces a compile-breaking syntax error that will prevent the binary from building.

The switch { case X: ... } else { ... } construct at line 3977–3998 is not valid Go. The Go specification only allows else to follow an if block, not a switch block. Combined with the removal of the case err != nil error-return path, any database error from the First() call would silently be treated as a conflict rather than returned, potentially stranding governance data and clearing FKs incorrectly. The code cannot build until the switch is refactored into if/else if/else with the error branch restored.

The critical area is the switch/else block in migrationMigrateProviderGovernanceToModelConfigs in migrations.go (lines 3977–3998). migrations_test.go is otherwise sound but will not compile until the production code is corrected.

Important Files Changed

Filename	Overview
framework/configstore/migrations.go	Adds backup logic for stranded governance rows on wildcard conflict, but introduces a compile-breaking syntax error: a switch block followed by } else { is not valid Go. The case err != nil error-handling path was also removed, so transient DB errors from the First() call now silently fall into the backup/detach path instead of returning an error.
framework/configstore/migrations_test.go	Three new test cases covering no-conflict, conflict-with-backup, and non-blocking backup failure scenarios. Tests are well-structured and verify idempotency, but will not compile until the syntax error in migrations.go is fixed.

Comments Outside Diff (1)

1. framework/configstore/migrations.go, line 3977-3998 (link)

   Invalid Go syntax: switch cannot have an else clause, and err != nil error handling is dropped

   The refactoring converted the three-case switch to switch { case err == gorm.ErrRecordNotFound: ... } else { ... }, but Go's grammar does not allow an else clause after a switch statement — else is only valid after an if block. This will produce a compile error and prevent the binary from building.

   In addition, the original case err != nil: return fmt.Errorf(...) branch was deleted along with the default: case. Any transient database error from the First() call (connection drop, lock timeout, etc.) now falls into the else branch, where the code attempts a backup savepoint, logs on failure, and then unconditionally nulls the provider FK columns — silently discarding governance data on a DB error rather than surfacing it.

   The fix requires replacing the switch/case header with an if/else if/else chain and restoring the error-returning branch.

_{Reviews (1): Last reviewed commit: "fix: supports backup for governance migr..." | Re-trigger Greptile}