fixing test cases

[akshaydeo]

Summary

Removes the calendar_aligned column from governance_budgets and governance_rate_limits, consolidating calendar alignment as a virtual-key-level and team-level setting. Budget and rate-limit reset logic now derives the calendar alignment value from the owning VK or team at reset time rather than storing it redundantly on each budget/rate-limit row.

Changes

• Adds migrationDropLegacyCalendarAlignedColumns to drop calendar_aligned from governance_budgets and governance_rate_limits, with migration test assertions updated to confirm the columns are absent post-migration and present only on governance_virtual_keys and governance_teams.
• Updates migrationRefreshConfigHashAfterMCPExternalServerURLRemoval to use an explicit, schema-derived column projection instead of SELECT * when fetching config_client rows. This prevents PostgreSQL's cached plan must not change result type error (SQLSTATE 0A000) that occurs when earlier migrations in the same run add and drop columns, causing a cached SELECT * plan to drift from the current schema.
• Updates the migration test snapshot comparison script to account for calendar_aligned being intentionally dropped from governance_budgets and governance_rate_limits, with an updated comment explaining the full column history.
• Adds TempToken to the encryption test store setup to satisfy schema requirements.
• Comments out git fetch --tags in the migration test runner script.

Type of change

• Refactor
• Bug fix

Affected areas

• Core (Go)

How to test

go test ./framework/configstore/... -run TestMigrationCalendarAligned_WiredIntoTriggerMigrations
go test ./framework/configstore/... -run TestMigration
go test ./framework/configstore/...

After running migrations on a fresh database, confirm:

• governance_budgets does not have a calendar_aligned column
• governance_rate_limits does not have a calendar_aligned column
• governance_virtual_keys has a calendar_aligned column
• governance_teams has a calendar_aligned column

Breaking changes

• Yes

The calendar_aligned column is removed from governance_budgets and governance_rate_limits. Any code or queries reading calendar_aligned directly from those tables must be updated to derive the value from the owning virtual key or team instead.

Security considerations

None.

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]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Migration validations now tolerate intentional table/column removals and expanded ignored legacy columns to reduce false-positive snapshot diffs.

• Tests

  • Updated migration tests to reflect changed schema expectations for the calendar_aligned column across migrations.
  • Added a temporary-token table to the encryption test setup for more reliable startup checks.

• Chores

  • Made migration refresh logic more robust by using explicit column selection to avoid plan-change failures.

Walkthrough

Adds a dropped-tables allowlist and expanded dropped-column ignores to the migration snapshot comparer, updates migration test assertions for calendar_aligned, includes TempToken in the encryption test AutoMigrate, and loads config_client rows using a schema-derived Select instead of implicit Find.

Changes

Migration robustness and snapshot handling

Layer / File(s)	Summary
Test runner: dropped oauth_per_user tables & missing-after-snapshot handling .github/workflows/scripts/run-migration-tests.sh	Adds dropped_tables allowlist for oauth_per_user_clients, oauth_per_user_codes, oauth_per_user_pending_flows, and oauth_per_user_sessions, and skips comparison when those tables are missing in after-snapshots.
Dropped-column allowlist updates for snapshots .github/workflows/scripts/run-migration-tests.sh	Expands dropped_columns to ignore calendar_aligned for governance_budgets and governance_rate_limits, and adds legacy session/token columns (session_token, session_token_hash, gateway_session_id) for OAuth tables.
Config client hash migration with explicit column projection framework/configstore/migrations.go	Parses tables.TableClientConfig via gorm.Statement and uses Select(schemaStmt.Schema.DBNames) instead of tx.Find(&clientConfigs) when loading rows for config-hash recomputation.
Encryption test schema: TempToken migration framework/configstore/encryption_test.go	Adds tables.TempToken{} to the AutoMigrate list in setupEncryptionTestStore so the TempToken table is created in the in-memory test DB.
Dropped calendar_aligned test assertions framework/configstore/migrations_test.go	Updates TestMigrationCalendarAligned_WiredIntoTriggerMigrations to assert calendar_aligned exists on virtual keys and teams and is removed from budgets and rate_limits.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• maximhq/bifrost#3434: Related calendar_aligned ownership and removal changes.
• maximhq/bifrost#3535: Changes to migration backfill/aliasing for calendar_aligned.
• maximhq/bifrost#3614: Migration that drops legacy calendar_aligned columns.

Suggested reviewers

• danpiths
• roroghost17

Poem

🐇 I hop through diffs with careful paws,
Skipping tables lost to migration laws,
Columns move to virtual keys and teams,
TempTokens join in test-run dreams,
A rabbit cheers: the snapshots pass!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fixing test cases' is vague and overly generic; it does not convey the actual substantive changes (column removal, migration logic updates, schema projections).	Revise the title to be more specific, e.g., 'Remove calendar_aligned column from budgets and rate-limits' or 'Refactor calendar alignment and fix config hash migration query'.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description is comprehensive, well-structured, and addresses all major template sections including Summary, Changes, Type of change, Affected areas, How to test, and Breaking changes.
Docstring Coverage	✅ Passed	Docstring coverage is 80.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 05-20-fixing_test_cases

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

[akshaydeo]

• fixing test cases #3639 👈 (View in Graphite)
• main

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

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

[github-actions]

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #3639

[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 @.github/workflows/scripts/run-migration-tests.sh:
- Around line 27-28: The script currently skips fetching git tags which lets
get_previous_versions() operate on an incomplete tag set and default to latest;
restore deterministic behavior by re-enabling the tags fetch (uncomment or add
back the git fetch --tags step in run-migration-tests.sh) so all repository tags
are available before get_previous_versions() runs, ensuring migration version
selection is based on the full tag list.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2ca717b2-3fa8-44f7-88a3-24d8c5ddcc1a

📥 Commits

Reviewing files that changed from the base of the PR and between f234cdb and d94da9b.

📒 Files selected for processing (4)

• .github/workflows/scripts/run-migration-tests.sh
• framework/configstore/encryption_test.go
• framework/configstore/migrations.go
• framework/configstore/migrations_test.go

[greptile-apps]

Confidence Score: 5/5

The migration chain is safe to merge: the drop is idempotent (errors are logged and swallowed), the preceding backfill migration preserves alignment data on upgrading DBs, and test coverage confirms the expected post-migration schema.

The logic is straightforward — a drop migration with swallowed errors, a backfill guard before the drop, and updated tests that confirm the schema state. The only issues found are two inaccurate doc comments in migrations.go that do not affect runtime behaviour.

framework/configstore/migrations.go — two inaccurate doc comments (IF EXISTS claim and VK-only scope) worth correcting before the next reader touches these functions.

Important Files Changed

Filename	Overview
framework/configstore/migrations.go	Adds migrationDropLegacyCalendarAlignedColumns (drops calendar_aligned from governance_budgets/rate_limits, errors logged as warnings) and fixes migrationRefreshConfigHashAfterMCPExternalServerURLRemoval to use explicit column projection instead of SELECT * to avoid PostgreSQL cached-plan errors. Two inaccurate doc comments present but no logic issues.
framework/configstore/migrations_test.go	TestMigrationCalendarAligned_WiredIntoTriggerMigrations updated to assert calendar_aligned is absent from governance_budgets/rate_limits and present on governance_virtual_keys/governance_teams — correctly reflects the new schema after the drop migration.
framework/configstore/encryption_test.go	Adds tables.TempToken{} to the AutoMigrate call in setupEncryptionTestStore to satisfy schema requirements introduced elsewhere; straightforward fix.
.github/workflows/scripts/run-migration-tests.sh	Adds dropped_tables handling for oauth_per_user_* tables, registers calendar_aligned as an intentionally dropped column for governance_budgets/rate_limits, and adds oauth session token and config_client column drop tracking — all correctly documents expected schema changes.

_{Reviews (3): Last reviewed commit: "fixing test cases" | Re-trigger Greptile}