fix: e2e ui tests for providers management

[sammaji]

Summary

Updates e2e tests for provider governance budgets and custom provider validation to align with recent UI changes, and fixes base URLs used in test fixtures to use real provider endpoints.

Changes

• Added addBudgetLine() and getBudgetAmountInput() helper methods to ProvidersPage to interact with the new budget line UI, replacing the old static #providerBudgetMaxLimit locator
• Updated governance budget tests to call addBudgetLine() before interacting with the budget amount input, reflecting the new add-then-fill flow
• Replaced placeholder/fake base URLs in custom provider test fixtures with real provider endpoints (https://api.openai.com/v1, https://api.anthropic.com) to avoid false failures from URL validation
• Added a new test case that verifies an error toast is shown when a custom provider is saved with an invalid/non-resolvable hostname

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

cd tests/e2e
pnpm test --grep "Providers|Governance"

Expected outcomes:

• Custom provider creation tests pass with real base URLs
• Governance budget tab tests correctly add a budget line before asserting input visibility
• A new test confirms that submitting a custom provider with an invalid hostname (https://api.nonexistent-provider.invalid/v1) surfaces an "Invalid base URL" error toast and keeps the sheet open

Screenshots/Recordings

N/A

Breaking changes

• Yes
• No

Related issues

N/A

Security considerations

No security implications. Test fixtures now use real provider hostnames, but no real credentials are used.

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: d6dafc7d-7942-4c51-8493-c77f8a172084

📥 Commits

Reviewing files that changed from the base of the PR and between 64ffcd7 and 79c6471.

📒 Files selected for processing (2)

• tests/e2e/features/providers/pages/providers.page.ts
• tests/e2e/features/providers/providers.spec.ts

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

• tests/e2e/features/providers/pages/providers.page.ts
• tests/e2e/features/providers/providers.spec.ts

---

📝 Walkthrough

Summary by CodeRabbit

• Tests
  • Improved E2E coverage for custom provider setup, including canonical base URL usage, validation for an invalid (non-resolvable) hostname, and verifying the provider sheet stays open after the error toast.
  • Updated governance budget E2E flows to edit multiple budget lines (amount and reset period) using stable UI selectors, aligning behavior with the “calendar aligned” toggle.

Walkthrough

ProvidersPage.setGovernanceConfig() is refactored to handle multiple governance budget lines, each with amount and reset period. A resetPeriodLabels lookup translates period keys to UI labels. E2E tests are updated to use canonical OpenAI and Anthropic base URLs, a new test validates "Invalid base URL" error handling, and existing governance budget tests are refactored to use the new configuration API.

Changes

Provider E2E Test Updates

Layer / File(s)	Summary
ProvidersPage governance budget line editor tests/e2e/features/providers/pages/providers.page.ts	Adds resetPeriodLabels mapping to translate reset period keys to UI labels. Reworks setGovernanceConfig() to accept budgets array (each with amount and resetPeriod) and aligned flag; automatically syncs visible budget rows, fills each row's amount input, selects reset period via the lookup, and toggles calendar-aligned switch when applicable.
Canonical base URLs and invalid-hostname validation test tests/e2e/features/providers/providers.spec.ts	Updates custom provider creation tests to use canonical base URLs: OpenAI (https://api.openai.com/v1), Anthropic (https://api.anthropic.com), and delete-test variant. Adds a new test that submits an invalid non-resolvable hostname, asserts "Invalid base URL" error toast, and confirms the custom provider sheet remains open.
Governance budget configuration tests updated tests/e2e/features/providers/providers.spec.ts	Budget configuration and set-budget-limit tests now call providersPage.setGovernanceConfig() with the new budgets array API. Budget amount inputs are located by provider-governance-budgets-amount-0 test id instead of prior DOM selectors. Final describe block closure is aligned.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• maximhq/bifrost#3460: Both PRs refactor page object helpers to manage multiple budget line items using per-line amount/reset-period selection patterns and array-based configuration APIs.

Suggested reviewers

• danpiths

Poem

🐇 A budget line splits into many—
With reset periods mapped from resetPeriodLabels, not just one penny!
Invalid URLs get their toast of red,
The canonical endpoints rise instead.
From single limits to arrays we dance,
E2E tests configured by the rabbit's advance! 🌙

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: updating e2e UI tests for providers management. It is concise, clear, and specifically related to the changeset.
Description check	✅ Passed	The PR description is comprehensive and well-structured. It includes a clear summary, detailed changes, affected areas, testing instructions, and a completed checklist. All required sections are addressed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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-18-fix_e2e_ui_tests_for_providers_management

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sammaji]

• fix: e2e ui tests for mcp registry #4558
• fix: e2e ui tests for model limts #4519
• fix: e2e ui tests for providers management #4517 👈 (View in Graphite)
• fix: adds explicit content type header for container delete #4490
• dev

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

[greptile-apps]

Confidence Score: 5/5

Safe to merge — changes are confined to E2E test helpers and specs with no impact on production code paths.

Both changed files are test-only. The page-object rewrite correctly models the new multi-budget UI (add/remove/fill), the removal loop iterates from the highest index downward so earlier indices stay stable, and the governance spec tests map cleanly onto the updated helper. No production logic is touched.

No files require special attention.

Important Files Changed

Filename	Overview
tests/e2e/features/providers/pages/providers.page.ts	Replaces the single static #providerBudgetMaxLimit locator with a flexible multi-budget API in setGovernanceConfig; adds a resetPeriodLabels map; removal logic correctly iterates from the last index so earlier indices stay stable.
tests/e2e/features/providers/providers.spec.ts	Governance tests now call setGovernanceConfig with a budget array instead of the removed #providerBudgetMaxLimit locator; custom-provider base URLs updated to real endpoints; adds an invalid-hostname test that relies on .invalid TLD DNS failure.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[setGovernanceConfig called] --> B[selectConfigTab governance]
    B --> C{config.budgets?}
    C -- yes --> D[count existing budget lines]
    D --> E{existingCount < target?}
    E -- yes --> F[click add-btn, existingCount++]
    F --> E
    E -- no --> G{existingCount > target?}
    G -- yes --> H[existingCount--, click remove-existingCount]
    H --> G
    G -- no --> I[for each budget: fill amount + resetPeriod]
    I --> J{config.aligned?}
    J -- yes --> K[toggle calendar-aligned switch]
    J -- no --> L[handle tokenLimit / requestLimit]
    K --> L
    C -- no --> L
    L --> M[done]

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[setGovernanceConfig called] --> B[selectConfigTab governance]
    B --> C{config.budgets?}
    C -- yes --> D[count existing budget lines]
    D --> E{existingCount < target?}
    E -- yes --> F[click add-btn, existingCount++]
    F --> E
    E -- no --> G{existingCount > target?}
    G -- yes --> H[existingCount--, click remove-existingCount]
    H --> G
    G -- no --> I[for each budget: fill amount + resetPeriod]
    I --> J{config.aligned?}
    J -- yes --> K[toggle calendar-aligned switch]
    J -- no --> L[handle tokenLimit / requestLimit]
    K --> L
    C -- no --> L
    L --> M[done]

Loading

_{Reviews (3): Last reviewed commit: "fix: e2e ui tests for providers manageme..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

tests/e2e/features/providers/pages/providers.page.ts (1)

666-672: ⚡ Quick win

Return and await the exact budget row created by addBudgetLine.

On Line 667, this helper only clicks. Downstream callers then default to index 0, which can target the wrong row when budgets already exist and make tests state-dependent. Prefer capturing the pre-click row count, waiting for that new input, and returning it.

♻️ Proposed update

-  async addBudgetLine(): Promise<void> {
-    await this.page.getByTestId('provider-governance-budgets-add-btn').click()
-  }
+  async addBudgetLine(): Promise<Locator> {
+    const nextIndex = await this.page.getByTestId(/provider-governance-budgets-amount-/).count()
+    await this.page.getByTestId('provider-governance-budgets-add-btn').click()
+    const input = this.getBudgetAmountInput(nextIndex)
+    await expect(input).toBeVisible({ timeout: 5000 })
+    return input
+  }

-      await providersPage.addBudgetLine();
-      const budgetInput = providersPage.getBudgetAmountInput();
+      const budgetInput = await providersPage.addBudgetLine();

🤖 Prompt for 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.

In `@tests/e2e/features/providers/pages/providers.page.ts` around lines 666 - 672,
The addBudgetLine() method only clicks the button but does not return the newly
created budget row, forcing downstream callers to default to index 0 which
targets the wrong row when budgets already exist. Modify the addBudgetLine()
method to first capture the current number of budget rows by counting existing
elements matching the test ID pattern used in getBudgetAmountInput(), then click
the add button, wait for the new budget input at the next index to be visible
using waitFor(), and finally return the Locator for that newly created input.
This ensures callers always reference the correct budget row that was just
added.

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

Nitpick comments:
In `@tests/e2e/features/providers/pages/providers.page.ts`:
- Around line 666-672: The addBudgetLine() method only clicks the button but
does not return the newly created budget row, forcing downstream callers to
default to index 0 which targets the wrong row when budgets already exist.
Modify the addBudgetLine() method to first capture the current number of budget
rows by counting existing elements matching the test ID pattern used in
getBudgetAmountInput(), then click the add button, wait for the new budget input
at the next index to be visible using waitFor(), and finally return the Locator
for that newly created input. This ensures callers always reference the correct
budget row that was just added.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a82ecbd-1834-49c2-b60a-abb90d94593a

📥 Commits

Reviewing files that changed from the base of the PR and between 1c050bf and 90c96b7.

📒 Files selected for processing (2)

• tests/e2e/features/providers/pages/providers.page.ts
• tests/e2e/features/providers/providers.spec.ts

[akshaydeo]

Merge activity

• Jun 19, 6:09 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 19, 6:10 PM UTC: @akshaydeo merged this pull request with Graphite.

The base branch was changed.