Conversation
18 tasks
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThese changes update migration test fixtures to reflect schema ownership changes for budget/calendar fields while adding per-user OAuth table support, and replace GORM-based backfill iteration with efficient SQL subquery updates in the configstore migrations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
|
|
This was referenced Apr 20, 2026
Collaborator
Author
Pratham-Mishra04
force-pushed
the
04-20-fix_migration_tests_fixes
branch
from
deae6ea to
f7087f0
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
b9107b1 to
8f2c722
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_migration_tests_fixes
branch
from
f7087f0 to
ffc9f68
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
8f2c722 to
bd0b53b
Compare
Contributor
Confidence Score: 4/5The Go migration fix is safe; the shell script has a P1 omission that prevents the new test coverage from running. The migrateCalendarAlignedToBudgetsAndRateLimitsTable fix in migrations.go is correct and addresses the SQLSTATE 0A000 bug. The bulk of the shell-script changes (column coverage, snapshot logic, per-user OAuth inserts) are well-structured. The single blocking issue is that v1.5.0-prerelease4 was never added to the prereleases list in get_previous_versions, so the prerelease4 test baseline is never tested despite all the supporting infrastructure being present. .github/workflows/scripts/run-migration-tests.sh line 143 — the prereleases variable must include v1.5.0-prerelease4. Important Files Changed
|
Contributor
Merge activity
|
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/scripts/run-migration-tests.sh (1)
729-741:⚠️ Potential issue | 🟠 MajorAdd prerelease2/prerelease4 to the version matrix.
These new generators only help if the harness starts from a schema that already contains those prerelease-era tables/columns. In the current script,
get_previous_versions()still explicitly adds onlyv1.5.0-prerelease1, so the per-user OAuth tables and thecalendar_alignedprerelease path are never actually exercised here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/scripts/run-migration-tests.sh around lines 729 - 741, get_previous_versions() currently only seeds the matrix with v1.5.0-prerelease1 so the prerelease2/prerelease4 migration paths (which affect generators like generate_per_user_oauth_tables_insert_postgres / generate_per_user_oauth_tables_insert_sqlite and the calendar_aligned path exercised via append_dynamic_columns_postgres) are never tested; update get_previous_versions() to include "v1.5.0-prerelease2" and "v1.5.0-prerelease4" (or the canonical prerelease tags used elsewhere) so the script will run those generator branches and exercise the per-user OAuth and prerelease calendar-aligned migrations. Ensure the version strings match existing tags used by the harness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/scripts/run-migration-tests.sh:
- Around line 3031-3036: The oauth_user_sessions and oauth_user_tokens inserts
reference oauth-config-migration-001 but the script seeds oauth_configs as
oauth-config-migration-test-001 / oauth-config-migration-test-002, breaking the
FK relationship; update the oauth_config_id value used in the INSERT statements
for oauth_user_sessions (function/statement inserting 'oauth-user-session-001')
and oauth_user_tokens (statement inserting 'oauth-user-token-001') to the
corresponding seeded id (e.g., oauth-config-migration-test-001 or -002 as
appropriate), and apply the same correction to the duplicate inserts around the
3084-3089 block so all fixtures reference the actual seeded oauth_configs ids.
- Around line 1490-1494: The fixtures for the new calendar_aligned column are
all set to false so a broken backfill could pass; update the seed statements in
the governance_virtual_keys block (the column_exists_postgres check that echoes
UPDATE ... SET calendar_aligned ...) to make at least one pre-migration owner
row truthy (SET calendar_aligned = true/1 for e.g. 'vk-migration-test-1'), and
keep the other as false to test both paths; apply the same change to the other
blocks mentioned (the similar governance_virtual_keys echo blocks around the
ranges 1561-1569, 2361-2365, and 2399-2408) and ensure the corresponding
budget/rate-limit verification logic in the migration test still asserts the
destination rows become truthy after migration.
In `@framework/configstore/migrations.go`:
- Around line 6722-6738: The migration's backfill only updates rows owned
directly by governance_virtual_keys; extend it to also propagate
calendar_aligned for rows owned via governance_virtual_key_provider_configs. Add
additional UPDATE statements (near the existing tx.Exec calls) to: 1) set
governance_rate_limits.calendar_aligned = true where governance_rate_limits.id
matches governance_virtual_key_provider_configs.rate_limit_id and that provider
config's virtual_key_id references a governance_virtual_keys row with
calendar_aligned = true; and 2) set governance_budgets.calendar_aligned = true
where governance_budgets.provider_config_id matches
governance_virtual_key_provider_configs.id whose virtual_key_id points to a
governance_virtual_keys row with calendar_aligned = true. Use the same tx.Exec
pattern and error handling as the existing updates.
---
Outside diff comments:
In @.github/workflows/scripts/run-migration-tests.sh:
- Around line 729-741: get_previous_versions() currently only seeds the matrix
with v1.5.0-prerelease1 so the prerelease2/prerelease4 migration paths (which
affect generators like generate_per_user_oauth_tables_insert_postgres /
generate_per_user_oauth_tables_insert_sqlite and the calendar_aligned path
exercised via append_dynamic_columns_postgres) are never tested; update
get_previous_versions() to include "v1.5.0-prerelease2" and "v1.5.0-prerelease4"
(or the canonical prerelease tags used elsewhere) so the script will run those
generator branches and exercise the per-user OAuth and prerelease
calendar-aligned migrations. Ensure the version strings match existing tags used
by the harness.
🪄 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: a22569be-b0fe-4c11-8280-2f07adc0453d
📒 Files selected for processing (2)
.github/workflows/scripts/run-migration-tests.shframework/configstore/migrations.go
Comment on lines
+1490
to
+1494
| # governance_virtual_keys.calendar_aligned (added in v1.5.0-prerelease2 via migrationAddMultiBudgetTables) | ||
| if column_exists_postgres "governance_virtual_keys" "calendar_aligned"; then | ||
| echo "UPDATE governance_virtual_keys SET calendar_aligned = false WHERE id = 'vk-migration-test-1';" >> "$output_file" | ||
| echo "UPDATE governance_virtual_keys SET calendar_aligned = false WHERE id = 'vk-migration-test-2';" >> "$output_file" | ||
| fi |
Contributor
There was a problem hiding this comment.
Use a truthy calendar_aligned fixture.
Every new calendar_aligned seed here is false/0. That means a broken backfill can still pass by leaving the destination at its default. Please make at least one pre-migration owner row truthy and verify the corresponding budget/rate-limit row is truthy after migration.
Also applies to: 1561-1569, 2361-2365, 2399-2408
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/scripts/run-migration-tests.sh around lines 1490 - 1494,
The fixtures for the new calendar_aligned column are all set to false so a
broken backfill could pass; update the seed statements in the
governance_virtual_keys block (the column_exists_postgres check that echoes
UPDATE ... SET calendar_aligned ...) to make at least one pre-migration owner
row truthy (SET calendar_aligned = true/1 for e.g. 'vk-migration-test-1'), and
keep the other as false to test both paths; apply the same change to the other
blocks mentioned (the similar governance_virtual_keys echo blocks around the
ranges 1561-1569, 2361-2365, and 2399-2408) and ensure the corresponding
budget/rate-limit verification logic in the migration test still asserts the
destination rows become truthy after migration.
Comment on lines
+3031
to
+3036
| echo "INSERT INTO oauth_user_sessions (id, mcp_client_id, oauth_config_id, state, redirect_uri, code_verifier, session_token, session_token_hash, gateway_session_id, virtual_key_id, user_id, status, encryption_status, expires_at, created_at, updated_at) VALUES ('oauth-user-session-001', 'mcp-migration-test-001', 'oauth-config-migration-001', 'migration-test-state-002', 'http://localhost:3000/callback', 'migration-test-verifier-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'gateway-session-001', 'vk-migration-test-1', NULL, 'authorized', 'plain_text', $now + INTERVAL '15 minutes', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file" | ||
|
|
||
| # oauth_user_tokens (stores per-user access/refresh tokens - no enforced FK) | ||
| echo "" >> "$output_file" | ||
| echo "-- oauth_user_tokens (per-user OAuth credentials)" >> "$output_file" | ||
| echo "INSERT INTO oauth_user_tokens (id, session_token, session_token_hash, virtual_key_id, user_id, mcp_client_id, oauth_config_id, access_token, refresh_token, token_type, expires_at, scopes, last_refreshed_at, encryption_status, created_at, updated_at) VALUES ('oauth-user-token-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'vk-migration-test-1', NULL, 'mcp-migration-test-001', 'oauth-config-migration-001', 'migration-test-user-access-token-001', '', 'Bearer', $now + INTERVAL '1 hour', '[\"openid\"]', NULL, 'plain_text', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file" |
Contributor
There was a problem hiding this comment.
Use the seeded OAuth config id.
oauth_user_sessions and oauth_user_tokens point at oauth-config-migration-001, but this script only inserts oauth-config-migration-test-001 / oauth-config-migration-test-002 into oauth_configs. That severs the relationship and can make these fixtures fail or miss migration logic that joins on oauth_config_id.
Proposed fix
- echo "INSERT INTO oauth_user_sessions (id, mcp_client_id, oauth_config_id, state, redirect_uri, code_verifier, session_token, session_token_hash, gateway_session_id, virtual_key_id, user_id, status, encryption_status, expires_at, created_at, updated_at) VALUES ('oauth-user-session-001', 'mcp-migration-test-001', 'oauth-config-migration-001', 'migration-test-state-002', 'http://localhost:3000/callback', 'migration-test-verifier-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'gateway-session-001', 'vk-migration-test-1', NULL, 'authorized', 'plain_text', $now + INTERVAL '15 minutes', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
+ echo "INSERT INTO oauth_user_sessions (id, mcp_client_id, oauth_config_id, state, redirect_uri, code_verifier, session_token, session_token_hash, gateway_session_id, virtual_key_id, user_id, status, encryption_status, expires_at, created_at, updated_at) VALUES ('oauth-user-session-001', 'mcp-migration-test-001', 'oauth-config-migration-test-001', 'migration-test-state-002', 'http://localhost:3000/callback', 'migration-test-verifier-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'gateway-session-001', 'vk-migration-test-1', NULL, 'authorized', 'plain_text', $now + INTERVAL '15 minutes', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
- echo "INSERT INTO oauth_user_tokens (id, session_token, session_token_hash, virtual_key_id, user_id, mcp_client_id, oauth_config_id, access_token, refresh_token, token_type, expires_at, scopes, last_refreshed_at, encryption_status, created_at, updated_at) VALUES ('oauth-user-token-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'vk-migration-test-1', NULL, 'mcp-migration-test-001', 'oauth-config-migration-001', 'migration-test-user-access-token-001', '', 'Bearer', $now + INTERVAL '1 hour', '[\"openid\"]', NULL, 'plain_text', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
+ echo "INSERT INTO oauth_user_tokens (id, session_token, session_token_hash, virtual_key_id, user_id, mcp_client_id, oauth_config_id, access_token, refresh_token, token_type, expires_at, scopes, last_refreshed_at, encryption_status, created_at, updated_at) VALUES ('oauth-user-token-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'vk-migration-test-1', NULL, 'mcp-migration-test-001', 'oauth-config-migration-test-001', 'migration-test-user-access-token-001', '', 'Bearer', $now + INTERVAL '1 hour', '[\"openid\"]', NULL, 'plain_text', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
- echo "INSERT INTO oauth_user_sessions (id, mcp_client_id, oauth_config_id, state, redirect_uri, code_verifier, session_token, session_token_hash, gateway_session_id, virtual_key_id, user_id, status, encryption_status, expires_at, created_at, updated_at) VALUES ('oauth-user-session-001', 'mcp-migration-test-001', 'oauth-config-migration-001', 'migration-test-state-002', 'http://localhost:3000/callback', 'migration-test-verifier-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'gateway-session-001', 'vk-migration-test-1', NULL, 'authorized', 'plain_text', datetime('now', '+15 minutes'), $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
+ echo "INSERT INTO oauth_user_sessions (id, mcp_client_id, oauth_config_id, state, redirect_uri, code_verifier, session_token, session_token_hash, gateway_session_id, virtual_key_id, user_id, status, encryption_status, expires_at, created_at, updated_at) VALUES ('oauth-user-session-001', 'mcp-migration-test-001', 'oauth-config-migration-test-001', 'migration-test-state-002', 'http://localhost:3000/callback', 'migration-test-verifier-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'gateway-session-001', 'vk-migration-test-1', NULL, 'authorized', 'plain_text', datetime('now', '+15 minutes'), $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
- echo "INSERT INTO oauth_user_tokens (id, session_token, session_token_hash, virtual_key_id, user_id, mcp_client_id, oauth_config_id, access_token, refresh_token, token_type, expires_at, scopes, last_refreshed_at, encryption_status, created_at, updated_at) VALUES ('oauth-user-token-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'vk-migration-test-1', NULL, 'mcp-migration-test-001', 'oauth-config-migration-001', 'migration-test-user-access-token-001', '', 'Bearer', datetime('now', '+1 hour'), '[\"openid\"]', NULL, 'plain_text', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"
+ echo "INSERT INTO oauth_user_tokens (id, session_token, session_token_hash, virtual_key_id, user_id, mcp_client_id, oauth_config_id, access_token, refresh_token, token_type, expires_at, scopes, last_refreshed_at, encryption_status, created_at, updated_at) VALUES ('oauth-user-token-001', 'migration-test-session-token-001', 'a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae7', 'vk-migration-test-1', NULL, 'mcp-migration-test-001', 'oauth-config-migration-test-001', 'migration-test-user-access-token-001', '', 'Bearer', datetime('now', '+1 hour'), '[\"openid\"]', NULL, 'plain_text', $now, $now) ON CONFLICT DO NOTHING;" >> "$output_file"Also applies to: 3084-3089
🧰 Tools
🪛 Betterleaks (1.1.2)
[high] 3031-3031: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/scripts/run-migration-tests.sh around lines 3031 - 3036,
The oauth_user_sessions and oauth_user_tokens inserts reference
oauth-config-migration-001 but the script seeds oauth_configs as
oauth-config-migration-test-001 / oauth-config-migration-test-002, breaking the
FK relationship; update the oauth_config_id value used in the INSERT statements
for oauth_user_sessions (function/statement inserting 'oauth-user-session-001')
and oauth_user_tokens (statement inserting 'oauth-user-token-001') to the
corresponding seeded id (e.g., oauth-config-migration-test-001 or -002 as
appropriate), and apply the same correction to the duplicate inserts around the
3084-3089 block so all fixtures reference the actual seeded oauth_configs ids.
Comment on lines
+6722
to
+6738
| if err := tx.Exec(` | ||
| UPDATE governance_rate_limits | ||
| SET calendar_aligned = true | ||
| WHERE id IN ( | ||
| SELECT rate_limit_id FROM governance_virtual_keys | ||
| WHERE calendar_aligned = true AND rate_limit_id IS NOT NULL | ||
| ) | ||
| `).Error; err != nil { | ||
| return fmt.Errorf("failed to propagate calendar_aligned to rate limits: %w", err) | ||
| } | ||
| if err := tx.Exec(` | ||
| UPDATE governance_budgets | ||
| SET calendar_aligned = true | ||
| WHERE virtual_key_id IN ( | ||
| SELECT id FROM governance_virtual_keys WHERE calendar_aligned = true | ||
| ) | ||
| `).Error; err != nil { |
Contributor
There was a problem hiding this comment.
Propagate calendar_aligned through provider-config ownership too.
These updates only backfill rows owned directly by governance_virtual_keys, but this stack also stores budgets/rate limits under governance_virtual_key_provider_configs (governance_budgets.provider_config_id and governance_virtual_key_provider_configs.rate_limit_id). For an aligned VK with provider-config budgets or rate limits, those rows stay false after migration.
💡 Suggested fix
if err := tx.Exec(`
UPDATE governance_rate_limits
SET calendar_aligned = true
WHERE id IN (
SELECT rate_limit_id FROM governance_virtual_keys
WHERE calendar_aligned = true AND rate_limit_id IS NOT NULL
+ UNION
+ SELECT pc.rate_limit_id
+ FROM governance_virtual_key_provider_configs pc
+ JOIN governance_virtual_keys vk ON vk.id = pc.virtual_key_id
+ WHERE vk.calendar_aligned = true AND pc.rate_limit_id IS NOT NULL
)
`).Error; err != nil {
return fmt.Errorf("failed to propagate calendar_aligned to rate limits: %w", err)
}
if err := tx.Exec(`
UPDATE governance_budgets
SET calendar_aligned = true
WHERE virtual_key_id IN (
SELECT id FROM governance_virtual_keys WHERE calendar_aligned = true
+ ) OR provider_config_id IN (
+ SELECT pc.id
+ FROM governance_virtual_key_provider_configs pc
+ JOIN governance_virtual_keys vk ON vk.id = pc.virtual_key_id
+ WHERE vk.calendar_aligned = true
)
`).Error; err != nil {
return fmt.Errorf("failed to propagate calendar_aligned to budgets: %w", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := tx.Exec(` | |
| UPDATE governance_rate_limits | |
| SET calendar_aligned = true | |
| WHERE id IN ( | |
| SELECT rate_limit_id FROM governance_virtual_keys | |
| WHERE calendar_aligned = true AND rate_limit_id IS NOT NULL | |
| ) | |
| `).Error; err != nil { | |
| return fmt.Errorf("failed to propagate calendar_aligned to rate limits: %w", err) | |
| } | |
| if err := tx.Exec(` | |
| UPDATE governance_budgets | |
| SET calendar_aligned = true | |
| WHERE virtual_key_id IN ( | |
| SELECT id FROM governance_virtual_keys WHERE calendar_aligned = true | |
| ) | |
| `).Error; err != nil { | |
| if err := tx.Exec(` | |
| UPDATE governance_rate_limits | |
| SET calendar_aligned = true | |
| WHERE id IN ( | |
| SELECT rate_limit_id FROM governance_virtual_keys | |
| WHERE calendar_aligned = true AND rate_limit_id IS NOT NULL | |
| UNION | |
| SELECT pc.rate_limit_id | |
| FROM governance_virtual_key_provider_configs pc | |
| JOIN governance_virtual_keys vk ON vk.id = pc.virtual_key_id | |
| WHERE vk.calendar_aligned = true AND pc.rate_limit_id IS NOT NULL | |
| ) | |
| `).Error; err != nil { | |
| return fmt.Errorf("failed to propagate calendar_aligned to rate limits: %w", err) | |
| } | |
| if err := tx.Exec(` | |
| UPDATE governance_budgets | |
| SET calendar_aligned = true | |
| WHERE virtual_key_id IN ( | |
| SELECT id FROM governance_virtual_keys WHERE calendar_aligned = true | |
| ) OR provider_config_id IN ( | |
| SELECT pc.id | |
| FROM governance_virtual_key_provider_configs pc | |
| JOIN governance_virtual_keys vk ON vk.id = pc.virtual_key_id | |
| WHERE vk.calendar_aligned = true | |
| ) | |
| `).Error; err != nil { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@framework/configstore/migrations.go` around lines 6722 - 6738, The
migration's backfill only updates rows owned directly by
governance_virtual_keys; extend it to also propagate calendar_aligned for rows
owned via governance_virtual_key_provider_configs. Add additional UPDATE
statements (near the existing tx.Exec calls) to: 1) set
governance_rate_limits.calendar_aligned = true where governance_rate_limits.id
matches governance_virtual_key_provider_configs.rate_limit_id and that provider
config's virtual_key_id references a governance_virtual_keys row with
calendar_aligned = true; and 2) set governance_budgets.calendar_aligned = true
where governance_budgets.provider_config_id matches
governance_virtual_key_provider_configs.id whose virtual_key_id points to a
governance_virtual_keys row with calendar_aligned = true. Use the same tx.Exec
pattern and error handling as the existing updates.
akshaydeo
changed the base branch from
04-20-fix_add_support_for_pricing_in_ocr_requests
to
graphite-base/2870
dominictayloruk
pushed a commit
to dominictayloruk/bifrost
that referenced
this pull request
Apr 21, 2026
…aligned propagation (maximhq#2870) ## Summary Extends migration test coverage to include `v1.5.0-prerelease4` and adds test data for the per-user OAuth tables introduced in that release. It also fixes a `pgx` prepared-statement cache invalidation bug in `migrateCalendarAlignedToBudgetsAndRateLimitsTable` that caused `SQLSTATE 0A000` errors when multiple migrations ran in the same transaction. ## Changes - Added `v1.5.0-prerelease4` to the explicit prerelease list tested by the migration script alongside `v1.5.0-prerelease1`. - Removed `calendar_aligned` from the static `governance_budgets` INSERT (it was added in prerelease1, dropped in prerelease2, and re-added in prerelease4) and added dynamic UPDATE coverage for it in both Postgres and SQLite paths. - Removed `budget_id` from the static `governance_teams`, `governance_virtual_keys`, and `governance_virtual_key_provider_configs` INSERTs to reflect schema changes across the prerelease series, with dynamic UPDATE fallbacks for older schemas that still carry those columns. - Added `governance_budgets.team_id` to the snapshot comparison ignore list, since it is backfilled during the prerelease4 migration. - Corrected the `compare_postgres_snapshots` dropped-column logic: `budget_id` is now excluded for `governance_teams` (not `governance_budgets`), and the stale `calendar_aligned` exclusion for `governance_budgets` is removed since that column was re-added. - Added `generate_per_user_oauth_tables_insert_postgres` and `generate_per_user_oauth_tables_insert_sqlite` functions that insert test rows into `oauth_per_user_clients`, `oauth_per_user_sessions`, `oauth_per_user_codes`, `oauth_per_user_pending_flows`, `oauth_user_sessions`, and `oauth_user_tokens` when those tables exist. - Added dynamic INSERT coverage for `config_mcp_clients.discovered_tools_json` and `tool_name_mapping_json` (added in prerelease2) for both Postgres and SQLite. - Added broad dynamic UPDATE coverage for all prerelease2 and prerelease4 schema additions across config, log, and prompt repo tables (e.g., `logs.alias`, `logs.has_object`, governance context columns, `logs.user_name`, `logs.attempt_trail`, `logs.selected_prompt_*`, `logs.ocr_input`, `prompt_versions.variables_json`, OCR pricing columns, `routing_rules.chain_rule`, `allow_all_keys`, compat columns on `config_client`). - Replaced the GORM ORM-based `calendar_aligned` propagation loop in `migrateCalendarAlignedToBudgetsAndRateLimitsTable` with two raw SQL `UPDATE … FROM` statements to avoid pgx prepared-statement cache invalidation (`SQLSTATE 0A000`) caused by earlier migrations in the same run adding columns to the affected tables. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh # Run migration tests against prerelease1 and prerelease4 baselines .github/workflows/scripts/run-migration-tests.sh # Verify Go migrations compile and unit tests pass go test ./framework/configstore/... ``` The migration test script will spin up Postgres and SQLite databases seeded from each tagged prerelease version, apply all pending migrations, and compare before/after snapshots. Confirm that no snapshot diff errors are reported for any of the tested versions. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No new secrets, auth flows, or PII handling introduced in the test harness. The per-user OAuth table inserts use clearly fake tokens and hashes (`migration-test-*` prefixed values) that carry no real credentials. ## 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) - [x] I verified the CI pipeline passes locally if applicable
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
3 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
Extends migration test coverage to include
v1.5.0-prerelease4and adds test data for the per-user OAuth tables introduced in that release. It also fixes apgxprepared-statement cache invalidation bug inmigrateCalendarAlignedToBudgetsAndRateLimitsTablethat causedSQLSTATE 0A000errors when multiple migrations ran in the same transaction.Changes
v1.5.0-prerelease4to the explicit prerelease list tested by the migration script alongsidev1.5.0-prerelease1.calendar_alignedfrom the staticgovernance_budgetsINSERT (it was added in prerelease1, dropped in prerelease2, and re-added in prerelease4) and added dynamic UPDATE coverage for it in both Postgres and SQLite paths.budget_idfrom the staticgovernance_teams,governance_virtual_keys, andgovernance_virtual_key_provider_configsINSERTs to reflect schema changes across the prerelease series, with dynamic UPDATE fallbacks for older schemas that still carry those columns.governance_budgets.team_idto the snapshot comparison ignore list, since it is backfilled during the prerelease4 migration.compare_postgres_snapshotsdropped-column logic:budget_idis now excluded forgovernance_teams(notgovernance_budgets), and the stalecalendar_alignedexclusion forgovernance_budgetsis removed since that column was re-added.generate_per_user_oauth_tables_insert_postgresandgenerate_per_user_oauth_tables_insert_sqlitefunctions that insert test rows intooauth_per_user_clients,oauth_per_user_sessions,oauth_per_user_codes,oauth_per_user_pending_flows,oauth_user_sessions, andoauth_user_tokenswhen those tables exist.config_mcp_clients.discovered_tools_jsonandtool_name_mapping_json(added in prerelease2) for both Postgres and SQLite.logs.alias,logs.has_object, governance context columns,logs.user_name,logs.attempt_trail,logs.selected_prompt_*,logs.ocr_input,prompt_versions.variables_json, OCR pricing columns,routing_rules.chain_rule,allow_all_keys, compat columns onconfig_client).calendar_alignedpropagation loop inmigrateCalendarAlignedToBudgetsAndRateLimitsTablewith two raw SQLUPDATE … FROMstatements to avoid pgx prepared-statement cache invalidation (SQLSTATE 0A000) caused by earlier migrations in the same run adding columns to the affected tables.Type of change
Affected areas
How to test
The migration test script will spin up Postgres and SQLite databases seeded from each tagged prerelease version, apply all pending migrations, and compare before/after snapshots. Confirm that no snapshot diff errors are reported for any of the tested versions.
Breaking changes
Related issues
N/A
Security considerations
No new secrets, auth flows, or PII handling introduced in the test harness. The per-user OAuth table inserts use clearly fake tokens and hashes (
migration-test-*prefixed values) that carry no real credentials.Checklist
docs/contributing/README.mdand followed the guidelines