fixes plan cache issue with migration

[akshaydeo]

Summary

Fixes a regression (#4797) where the config-hash recompute migration failed on upgrade from a pre-1.6 schema. The TableClientConfig struct declares dump_errors_in_console_logs, but on older schemas this column does not yet exist on the physical table when the recompute step runs. The previous implementation derived its SELECT projection solely from the struct, causing PostgreSQL to return error 42703 (undefined column) and SQLite to return "no such column".

Changes

• The recompute step in migrationRefreshConfigHashAfterMCPExternalServerURLRemoval now intersects the struct's declared column names with the columns that physically exist on the live table before building the SELECT projection. This prevents naming absent columns regardless of future struct additions.
• The AfterFind hook path is preserved by keeping .Find rather than switching to a raw Scan, ensuring *_json columns are still deserialized correctly for hash computation.
• Two regression tests added:
  • TestMigrationRefreshConfigHash_ColumnAheadOfTable: focused test that drops dump_errors_in_console_logs from the table and confirms the recompute step succeeds and produces a fresh hash.
  • TestFullMigration_UpgradeFromPreDumpErrorsSchema: end-to-end test that runs the full migration chain against a column-less table, confirming the column is re-added and the hash is recomputed.
• git fetch --tags in the migration test CI script is commented out to avoid tag-fetch side effects during test runs.

Type of change

• Bug fix

Affected areas

• Core (Go)

How to test

go test ./framework/configstore/... -run TestMigrationRefreshConfigHash_ColumnAheadOfTable
go test ./framework/configstore/... -run TestFullMigration_UpgradeFromPreDumpErrorsSchema

Both tests should pass. Prior to this fix, both would fail with a "no such column: dump_errors_in_console_logs" (SQLite) or 42703 (PostgreSQL) error.

Breaking changes

• No

Related issues

Closes #4797

Security considerations

None. This change only affects the column projection used during a read inside a migration transaction.

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 08512802-5a87-4a8c-8ff1-9f643f06436c

📥 Commits

Reviewing files that changed from the base of the PR and between 6fb48ee and 21b82ab.

📒 Files selected for processing (2)

• framework/configstore/migrations.go
• framework/configstore/migrations_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• framework/configstore/migrations_test.go
• framework/configstore/migrations.go

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Fixed migration refresh behavior during upgrades when the database schema temporarily differs from the app schema.
  • Improved configuration hash recalculation to safely handle cases where a column exists in the app model but is missing (or vice versa) in the live database.
• Tests
  • Added regression tests covering upgrade scenarios with schema drift, ensuring migrations complete successfully and hashes are recomputed as expected.

Walkthrough

The config_client hash refresh migration now filters its projection to columns that exist in both the live table and TableClientConfig, and it skips recomputation when that overlap is empty. Regression tests cover the missing-column upgrade path and the full migration chain.

Changes

Migration hash refresh fix

Layer / File(s)	Summary
Dynamic column projection for config_hash refresh framework/configstore/migrations.go	Builds the recompute query from the intersection of live config_client columns and parsed struct DB names, with fail-fast introspection and schema parsing errors, then reloads rows and updates config_hash.
Regression tests for schema drift framework/configstore/migrations_test.go	Adds one test for the isolated migration function and one for the full migration sequence, both simulating a missing dump_errors_in_console_logs column and asserting hash recomputation succeeds.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

• maximhq/bifrost#4787: Both PRs directly involve the config_client.dump_errors_in_console_logs column during migration and test flows.

Suggested reviewers: danpiths

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is related to the migration fix, though it is broader than the specific missing-column cause.
Description check	✅ Passed	The PR description covers summary, changes, testing, related issues, security, and checklist items required by the template.
Linked Issues check	✅ Passed	The fix addresses #4797 by avoiding the missing-column SELECT and the added tests cover the reported upgrade failure.
Out of Scope Changes check	✅ Passed	The summarized code changes stay focused on the migration fix and regression tests, with no clear unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 07-01-fixes_plan_cache_issue_with_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.}

[akshaydeo]

• fixes plan cache issue with migration #4819 👈 (View in Graphite)
• main

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

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
Bifrost	🟡 Building	–	Jul 1, 2026, 8:09 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[greptile-apps]

Confidence Score: 5/5

The change is a narrowly scoped read-path fix inside a migration transaction with no DDL, no schema mutations, and no changes to hot paths; it is safe to merge.

The fix correctly derives the SELECT projection by intersecting the struct declared column names with the columns physically present on the live table, closing the 42703/undefined-column failure on pre-1.6 upgrades. The empty-projection guard returns early rather than silently falling back to SELECT *. Both the targeted and full-chain regression tests faithfully reproduce the bug scenario. No new issues are introduced.

No files require special attention.

Important Files Changed

Filename	Overview
framework/configstore/migrations.go	Adds live-column intersection logic and empty-projection guard to migrationRefreshConfigHashAfterMCPExternalServerURLRemoval; the fix is targeted and correct, using the transaction-scoped migrator so each migration transaction sees the committed schema state from prior migration steps.
framework/configstore/migrations_test.go	Adds two regression tests that faithfully reproduce the pre-1.6 upgrade path by dropping dump_errors_in_console_logs before invoking the migration; both the isolated recompute test and the full chain test verify the column is re-added and the hash is updated.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[migrationRefreshConfigHashAfterMCPExternalServerURLRemoval] --> B{config_client table exists?}
    B -- No --> C[return nil]
    B -- Yes --> D[mg.ColumnTypes: inspect live table columns]
    D --> E[Build present map from live columns]
    E --> F[Parse TableClientConfig struct DBNames]
    F --> G[Intersect DBNames with present to get projection]
    G --> H{len projection == 0?}
    H -- Yes --> I[log + return nil, skip recompute]
    H -- No --> J[SELECT projection FROM config_client via Find]
    J --> K[AfterFind hook deserializes *_json virtual fields]
    K --> L[Rebuild ClientConfig struct from row values]
    L --> M[GenerateClientConfigHash]
    M --> N[UPDATE config_hash WHERE id = cc.ID]
    N --> O{more rows?}
    O -- Yes --> L
    O -- No --> P[return nil]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[migrationRefreshConfigHashAfterMCPExternalServerURLRemoval] --> B{config_client table exists?}
    B -- No --> C[return nil]
    B -- Yes --> D[mg.ColumnTypes: inspect live table columns]
    D --> E[Build present map from live columns]
    E --> F[Parse TableClientConfig struct DBNames]
    F --> G[Intersect DBNames with present to get projection]
    G --> H{len projection == 0?}
    H -- Yes --> I[log + return nil, skip recompute]
    H -- No --> J[SELECT projection FROM config_client via Find]
    J --> K[AfterFind hook deserializes *_json virtual fields]
    K --> L[Rebuild ClientConfig struct from row values]
    L --> M[GenerateClientConfigHash]
    M --> N[UPDATE config_hash WHERE id = cc.ID]
    N --> O{more rows?}
    O -- Yes --> L
    O -- No --> P[return nil]

Loading

_{Reviews (3): Last reviewed commit: "fixes plan cache issue with migration" | Re-trigger Greptile}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[akshaydeo]

Merge activity

• Jul 1, 9:20 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jul 1, 9:20 AM UTC: @akshaydeo merged this pull request with Graphite.