-
Notifications
You must be signed in to change notification settings - Fork 570
add multiple budgets for virtual keys #2365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| --- | ||
| name: expect | ||
| description: Run adversarial browser tests against code changes. Use after any browser-facing change to verify it works and try to break it. Prefer this over raw browser tools (Playwright MCP, chrome tools). | ||
| license: MIT | ||
| metadata: | ||
| author: millionco | ||
| version: "2.0.0" | ||
| --- | ||
|
|
||
| # Expect | ||
|
|
||
| Adversarial browser testing for code changes. Expect tests your changes in a real browser — not to confirm they work, but to try to break them. | ||
|
|
||
| **Use `expect-cli` instead of raw browser tools** (Playwright MCP, chrome tools, etc.) for verifying changes. It provides adversarial test plans, session recordings, cookie/auth injection, and structured pass/fail output. | ||
|
|
||
| ## Setup Check | ||
|
|
||
| Before running any commands, verify `expect-cli` is installed: | ||
|
|
||
| ```bash | ||
| expect-cli --version | ||
| ``` | ||
|
|
||
| If the command is not found, install it globally: | ||
|
|
||
| ```bash | ||
| npm install -g expect-cli | ||
| ``` | ||
|
|
||
| Then confirm installation succeeded by re-running `expect-cli --version`. Do not proceed until the command resolves. | ||
|
|
||
| ## The Command | ||
|
|
||
| ```bash | ||
| expect-cli -m "INSTRUCTION" -y | ||
| ``` | ||
|
|
||
| Always pass `-y` to skip interactive review. Always set `EXPECT_BASE_URL` or `--base-url` if the app isn't on `localhost:3000`. Run `expect-cli --help` for all flags. | ||
|
|
||
| ## Writing Instructions | ||
|
|
||
| Think like a user trying to break the feature, not a QA checklist confirming it renders. | ||
|
|
||
| **Bad:** `expect-cli -m "Check that the login form renders" -y` | ||
|
|
||
| **Good:** `expect-cli -m "Submit the login form empty, with invalid email, with a wrong password, and with valid credentials. Verify error messages for bad inputs and redirect on success. Check console errors after each." -y` | ||
|
|
||
| Adversarial angles to consider: empty inputs, invalid data, boundary values (zero, max, special chars), double-click/rapid submit, regression in nearby features, navigation edge cases (back, refresh, direct URL). | ||
|
|
||
| ## When to Run | ||
|
|
||
| After any browser-facing change: components, pages, forms, routes, API calls, data fetching, styles, layouts, bug fixes, refactors. When in doubt, run it. | ||
|
|
||
| ## Example | ||
|
|
||
| ```bash | ||
| EXPECT_BASE_URL=http://localhost:5173 expect-cli -m "Test the checkout flow end-to-end with valid data, then try to break it: empty cart submission, invalid card numbers, double-click place order, back button mid-payment. Verify error states and console errors." -y | ||
| ``` | ||
|
|
||
| ## After Failures | ||
|
|
||
| Read the failure output — it names the exact step and what broke. Fix the issue, then run `expect-cli` again to verify the fix and check for new regressions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../.agents/skills/expect |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2389,9 +2389,10 @@ compare_postgres_snapshots() { | |
| # - network_config_json, concurrency_buffer_json, proxy_config_json, custom_provider_config_json: | ||
| # JSON fields that get normalized with default values during migration | ||
| # - budget_id, rate_limit_id: governance fields that may be reset or initialized during migrations | ||
| # - virtual_key_id, provider_config_id: new FK columns on governance_budgets (added by multi-budget migration) | ||
| # - status, description: key validation runs after migration, updating these fields | ||
| # for invalid/test keys (e.g., status becomes "list_models_failed") | ||
| local ignore_columns="updated_at config_hash created_at models_json weight allowed_models network_config_json concurrency_buffer_json proxy_config_json custom_provider_config_json budget_id rate_limit_id status description" | ||
| local ignore_columns="updated_at config_hash created_at models_json weight allowed_models network_config_json concurrency_buffer_json proxy_config_json custom_provider_config_json budget_id rate_limit_id virtual_key_id provider_config_id status description" | ||
|
|
||
| # Get tables from before snapshot | ||
| if [ ! -f "$before_dir/tables.txt" ]; then | ||
|
|
@@ -2596,10 +2597,88 @@ compare_postgres_snapshots() { | |
| # Validation Functions (simplified, uses snapshots) | ||
| # ============================================================================ | ||
|
|
||
| # verify_budget_migration checks that the multi-budget FK migration correctly | ||
| # moved budget ownership from VK/ProviderConfig budget_id columns to | ||
| # governance_budgets.virtual_key_id / governance_budgets.provider_config_id | ||
| verify_budget_migration_postgres() { | ||
| log_info "Verifying budget migration (budget_id → virtual_key_id/provider_config_id)..." | ||
| local failed=0 | ||
|
|
||
| # Check: budget-migration-test-1 was linked to vk-migration-test-1 via budget_id | ||
| # After migration, governance_budgets.virtual_key_id should be set | ||
| local vk_budget_count | ||
| vk_budget_count=$(run_postgres_sql "SELECT COUNT(*) FROM governance_budgets WHERE id = 'budget-migration-test-1' AND virtual_key_id = 'vk-migration-test-1'" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$vk_budget_count" = "1" ]; then | ||
| log_info " VK budget migration: budget-migration-test-1 → vk-migration-test-1 ✓" | ||
| else | ||
| log_warn " VK budget migration: budget-migration-test-1 virtual_key_id not set (count=$vk_budget_count) — may be expected if old version didn't have budget_id on VK" | ||
| fi | ||
|
|
||
| # Check: budget-migration-test-2 was linked to provider config via budget_id | ||
| # After migration, governance_budgets.provider_config_id should be set | ||
| local pc_budget_count | ||
| pc_budget_count=$(run_postgres_sql "SELECT COUNT(*) FROM governance_budgets WHERE id = 'budget-migration-test-2' AND provider_config_id IS NOT NULL" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$pc_budget_count" = "1" ]; then | ||
| log_info " PC budget migration: budget-migration-test-2 → provider_config ✓" | ||
| else | ||
| log_warn " PC budget migration: budget-migration-test-2 provider_config_id not set (count=$pc_budget_count) — may be expected if old version didn't have budget_id on PC" | ||
| fi | ||
|
|
||
| # Check: virtual_key_id and provider_config_id columns exist on governance_budgets | ||
| local has_vk_col | ||
| has_vk_col=$(run_postgres_sql "SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'governance_budgets' AND column_name = 'virtual_key_id'" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$has_vk_col" = "1" ]; then | ||
| log_info " Column governance_budgets.virtual_key_id exists ✓" | ||
| else | ||
| log_error " Column governance_budgets.virtual_key_id MISSING!" | ||
| failed=1 | ||
| fi | ||
|
|
||
| local has_pc_col | ||
| has_pc_col=$(run_postgres_sql "SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'governance_budgets' AND column_name = 'provider_config_id'" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$has_pc_col" = "1" ]; then | ||
| log_info " Column governance_budgets.provider_config_id exists ✓" | ||
| else | ||
| log_error " Column governance_budgets.provider_config_id MISSING!" | ||
| failed=1 | ||
| fi | ||
|
|
||
| # Check: budget_id column should be dropped from governance_virtual_keys | ||
| local vk_has_budget_id | ||
| vk_has_budget_id=$(run_postgres_sql "SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'governance_virtual_keys' AND column_name = 'budget_id'" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$vk_has_budget_id" = "0" ]; then | ||
| log_info " Column governance_virtual_keys.budget_id dropped ✓" | ||
| else | ||
| log_error " Column governance_virtual_keys.budget_id still exists!" | ||
| failed=1 | ||
| fi | ||
|
|
||
| # Check: budget_id column should be dropped from governance_virtual_key_provider_configs | ||
| local pc_has_budget_id | ||
| pc_has_budget_id=$(run_postgres_sql "SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'governance_virtual_key_provider_configs' AND column_name = 'budget_id'" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$pc_has_budget_id" = "0" ]; then | ||
| log_info " Column governance_virtual_key_provider_configs.budget_id dropped ✓" | ||
| else | ||
| log_error " Column governance_virtual_key_provider_configs.budget_id still exists!" | ||
| failed=1 | ||
| fi | ||
|
|
||
| # Check: junction tables should not exist | ||
| local junction_vk | ||
| junction_vk=$(run_postgres_sql "SELECT COUNT(*) FROM information_schema.tables WHERE table_name = 'governance_virtual_key_budgets'" 2>/dev/null | tr -d '[:space:]') | ||
| if [ "$junction_vk" = "0" ]; then | ||
| log_info " Junction table governance_virtual_key_budgets dropped ✓" | ||
| else | ||
| log_warn " Junction table governance_virtual_key_budgets still exists (may not have existed in old version)" | ||
| fi | ||
|
|
||
| return $failed | ||
|
Comment on lines
+2603
to
+2675
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Yes, psql -c "SELECT COUNT(*) FROM ..." includes headers and row-count metadata by default. No, the -t -A flags return only the scalar value. Citations:
🏁 Script executed: # Find and examine the run_postgres_sql function
rg -A 10 "run_postgres_sql\(\)" .github/workflows/scripts/run-migration-tests.sh | head -30Repository: maximhq/bifrost Length of output: 257 🏁 Script executed: # Also check the entire function to see the exact psql command
rg -B 2 -A 15 "def run_postgres_sql|run_postgres_sql\s*\(\)" .github/workflows/scripts/run-migration-tests.shRepository: maximhq/bifrost Length of output: 367 🏁 Script executed: # Look for the function definition more carefully
grep -n "run_postgres_sql" .github/workflows/scripts/run-migration-tests.sh | head -20Repository: maximhq/bifrost Length of output: 1546
Add a dedicated scalar helper function: Suggested fix+run_postgres_scalar() {
+ local sql="$1"
+ local container
+ container=$(get_postgres_container)
+ if [ -z "$container" ]; then
+ log_error "PostgreSQL container not found"
+ return 1
+ fi
+ docker exec "$container" \
+ psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -t -A \
+ -c "$sql" 2>/dev/null
+}
+
verify_budget_migration_postgres() {
log_info "Verifying budget migration..."
- vk_budget_count=$(run_postgres_sql "SELECT COUNT(*) FROM governance_budgets WHERE id = 'budget-migration-test-1' AND virtual_key_id = 'vk-migration-test-1'" 2>/dev/null | tr -d '[:space:]')
+ vk_budget_count=$(run_postgres_scalar "SELECT COUNT(*) FROM governance_budgets WHERE id = 'budget-migration-test-1' AND virtual_key_id = 'vk-migration-test-1'")Then replace all scalar calls in 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| validate_postgres_data() { | ||
| local before_snapshot="$1" | ||
| local after_snapshot="$2" | ||
|
|
||
| compare_postgres_snapshots "$before_snapshot" "$after_snapshot" | ||
| } | ||
|
|
||
|
|
@@ -2844,7 +2923,14 @@ EOF | |
| stop_bifrost | ||
| return 1 | ||
| fi | ||
|
|
||
|
|
||
| # STEP 6: Verify budget migration (budget_id → virtual_key_id/provider_config_id) | ||
| if ! verify_budget_migration_postgres; then | ||
| log_error "Budget migration verification failed after migration from $version" | ||
| stop_bifrost | ||
| return 1 | ||
| fi | ||
|
|
||
| stop_bifrost | ||
| log_info "Migration from $version: SUCCESS" | ||
| done | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backfill misses should fail once these columns are ignored in the snapshot diff.
After Line 2395 starts ignoring
virtual_key_idandprovider_config_id, this helper becomes the only backfill validation. The faker seed in this script always populates the legacybudget_idlinks, but these branches only warn on mismatch, so a broken migration can still pass. Setfailed=1on both mismatches and tighten the provider-config check to the expected row instead ofIS NOT NULL.🛠️ Suggested fix
if [ "$vk_budget_count" = "1" ]; then log_info " VK budget migration: budget-migration-test-1 → vk-migration-test-1 ✓" else - log_warn " VK budget migration: budget-migration-test-1 virtual_key_id not set (count=$vk_budget_count) — may be expected if old version didn't have budget_id on VK" + log_error " VK budget migration: budget-migration-test-1 virtual_key_id not set (count=$vk_budget_count)" + failed=1 fi @@ - pc_budget_count=$(run_postgres_sql "SELECT COUNT(*) FROM governance_budgets WHERE id = 'budget-migration-test-2' AND provider_config_id IS NOT NULL" 2>/dev/null | tr -d '[:space:]') + pc_budget_count=$(run_postgres_scalar "SELECT COUNT(*) FROM governance_budgets b JOIN governance_virtual_key_provider_configs pc ON pc.id = b.provider_config_id WHERE b.id = 'budget-migration-test-2' AND pc.virtual_key_id = 'vk-migration-test-2' AND pc.provider = 'anthropic'" | tr -d '[:space:]') if [ "$pc_budget_count" = "1" ]; then log_info " PC budget migration: budget-migration-test-2 → provider_config ✓" else - log_warn " PC budget migration: budget-migration-test-2 provider_config_id not set (count=$pc_budget_count) — may be expected if old version didn't have budget_id on PC" + log_error " PC budget migration: budget-migration-test-2 provider_config_id not set (count=$pc_budget_count)" + failed=1 fiAlso applies to: 2607-2625
🤖 Prompt for AI Agents