github/OpenHands
1
0
Fork 0
mirror of https://github.com/OpenHands/OpenHands.git synced 2026-10-06 14:33:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

Remove obsolete OpenHands proxy base URL handling (#1321)

Browse Source 
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: Engel Nyst <engel.nyst@gmail.com>
This commit is contained in:
Rohit Malhotra
2026-06-12 00:40:21 +00:00
committed by GitHub
co-authored by openhands Engel Nyst
parent 1016affac1
commit 270ef6a876
6 changed files with 33 additions and 425 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/e2e/mock-llm/mock-llm-profile-management.spec.ts
+17 -212
View File
@@ -1,7 +1,7 @@
/**
* Mock-LLM E2E tests: LLM profile management regressions.
*
* Covers three scenarios that previously had no end-to-end guard:
* Covers two scenarios that previously had no end-to-end guard:
*
* 1. Active profile deletion + reconciliation:
* The active LLM profile IS deletable (the PR #1127 disable-guard was
@@ -16,19 +16,10 @@
* user selected, not the first alphabetical match. The fix
* stamps the active profile name on client-side conversation
* metadata at creation and on per-conversation switches.
*
* 3. Proxy base_url preservation for litellm_proxy profiles (PR #1148):
* When the SDK rewrites an `openhands/*` model to `litellm_proxy/*`
* with the All-Hands proxy base_url, re-saving the profile from the
* Basic tab must not strip the base_url. Without it the profile is
* stranded and LiteLLM reroutes requests to the wrong endpoint.
*/
import { test, expect } from "@playwright/test";
import {
BACKEND_URL,
SESSION_API_KEY,
MOCK_LLM_AGENT_URL,
seedLocalStorage,
routeSessionApiKey,
dismissAnalyticsModal,
@@ -48,28 +39,6 @@ import {
const MOCK_MODEL = "openai/mock-test-model";
/**
* Read-only helper: fetch a profile's persisted config via the API.
* Used to verify that UI-driven saves persisted the expected values.
*/
async function getProfileConfig(
request: import("@playwright/test").APIRequestContext,
name: string,
): Promise<Record<string, unknown>> {
const resp = await request.get(
`${BACKEND_URL}/api/profiles/${encodeURIComponent(name)}`,
{
headers: {
"X-Session-API-Key": SESSION_API_KEY,
"X-Expose-Secrets": "encrypted",
},
},
);
expect(resp.ok(), `GET /api/profiles/${name}: ${resp.status()}`).toBe(true);
const data = await resp.json();
return (data.config ?? {}) as Record<string, unknown>;
}
test.describe.configure({ mode: "serial" });
// ═══════════════════════════════════════════════════════════════════════
@@ -116,8 +85,14 @@ test.describe("active profile deletion + reconciliation", () => {
await deleteProfileIfExists(page, INACTIVE_PROFILE);
// Create both profiles through the Settings UI
await createProfileViaUI(page, { profileName: ACTIVE_PROFILE, model: MOCK_MODEL });
await createProfileViaUI(page, { profileName: INACTIVE_PROFILE, model: MOCK_MODEL });
await createProfileViaUI(page, {
profileName: ACTIVE_PROFILE,
model: MOCK_MODEL,
});
await createProfileViaUI(page, {
profileName: INACTIVE_PROFILE,
model: MOCK_MODEL,
});
// Activate the first profile through the UI
await activateProfileViaUI(page, ACTIVE_PROFILE);
@@ -289,8 +264,14 @@ test.describe("same-model profile identity", () => {
await deleteProfileIfExists(page, PROFILE_ALPHA);
await deleteProfileIfExists(page, PROFILE_BETA);
await createProfileViaUI(page, { profileName: PROFILE_ALPHA, model: SHARED_MODEL });
await createProfileViaUI(page, { profileName: PROFILE_BETA, model: SHARED_MODEL });
await createProfileViaUI(page, {
profileName: PROFILE_ALPHA,
model: SHARED_MODEL,
});
await createProfileViaUI(page, {
profileName: PROFILE_BETA,
model: SHARED_MODEL,
});
await activateProfileViaUI(page, PROFILE_BETA);
// Register a trajectory for the conversation.
@@ -347,179 +328,3 @@ test.describe("same-model profile identity", () => {
});
});
});
// ═══════════════════════════════════════════════════════════════════════
// Test 3 — Proxy base_url preservation for litellm_proxy profiles
// (PR #1148, issue #1146)
// ═══════════════════════════════════════════════════════════════════════
/**
* Assert that a proxy profile config is valid after a Basic-tab re-save.
*
* Agent-server ≥1.28 normalises `litellm_proxy/*` → `openhands/*` on storage
* and manages the proxy URL internally (returning base_url: null). Both
* representations are accepted here. The original issue #1146 regression
* (litellm_proxy/* with null base_url) is still guarded explicitly.
*/
function assertProxyProfileConfig(
config: Record<string, unknown>,
litellmProxyModel: string,
openHandsEquivalentModel: string,
proxyBaseUrl: string,
) {
const model = config.model as string | null | undefined;
const baseUrl = config.base_url as string | null | undefined;
const isLitellmForm =
typeof model === "string" && model.startsWith("litellm_proxy/");
const isOpenHandsForm =
typeof model === "string" && model.startsWith("openhands/");
expect(
isLitellmForm || isOpenHandsForm,
`model must be ${litellmProxyModel} or ${openHandsEquivalentModel}; got: ${model}`,
).toBe(true);
if (isLitellmForm) {
// A litellm_proxy/* profile without the proxy URL is stranded (issue #1146).
// The frontend must always inject the URL when saving from the Basic tab.
expect(
baseUrl,
"base_url must be the All-Hands proxy URL for litellm_proxy/* profiles " +
"(dropping it strands the profile — issue #1146)",
).toBe(proxyBaseUrl);
}
// For openhands/* (agent-server ≥1.28 rewrite), null base_url is correct;
// the server routes the request through the proxy internally.
}
test.describe("litellm_proxy proxy base_url preservation", () => {
// Simulates the state the SDK persists after onboarding through the
// OpenHands provider: openhands/* is rewritten to litellm_proxy/* and
// paired with the All-Hands proxy base URL.
const PROXY_PROFILE = "proxy-base-url-test";
const LITELLM_PROXY_MODEL = "litellm_proxy/claude-opus-4-8";
const OPENHANDS_PROXY_BASE_URL = "https://llm-proxy.app.all-hands.dev/";
test.beforeEach(async ({ page }) => {
await seedLocalStorage(page);
});
test.afterAll(async ({ browser }) => {
const page = await browser.newPage();
try {
await seedLocalStorage(page);
await routeSessionApiKey(page);
await page.goto("/settings/llm", { waitUntil: "domcontentloaded" });
await dismissAnalyticsModal(page);
await waitForTestId(page, "add-llm-profile");
await deleteProfileIfExists(page, PROXY_PROFILE);
} catch {
// best-effort
} finally {
await page.close();
}
});
test("re-saving a litellm_proxy profile from Basic view preserves the proxy base_url", async ({
page,
request,
}) => {
// Agent-server ≥1.28 normalises litellm_proxy/* → openhands/* on storage
// and manages the proxy URL internally (returning base_url: null). The old
// assertions hard-coded the pre-1.28 storage format.
const OPENHANDS_EQUIVALENT_MODEL = "openhands/claude-opus-4-8";
// ── Setup: create a profile with the SDK-rewritten litellm_proxy
// model + proxy base_url through the Settings UI, exactly as
// the agent-server persists it after an openhands/* model
// selection during onboarding. ──
await routeSessionApiKey(page);
await page.goto("/settings/llm", { waitUntil: "domcontentloaded" });
await dismissAnalyticsModal(page);
await waitForTestId(page, "add-llm-profile");
await deleteProfileIfExists(page, PROXY_PROFILE);
await createProfileViaUI(page, {
profileName: PROXY_PROFILE,
model: LITELLM_PROXY_MODEL,
baseUrl: OPENHANDS_PROXY_BASE_URL,
});
// ── Find the profile row and open edit mode ──
await test.step("open profile in edit mode", async () => {
const profileRows = page.getByTestId("profile-row");
const rowCount = await profileRows.count();
let targetRow: ReturnType<typeof profileRows.nth> | null = null;
for (let i = 0; i < rowCount; i++) {
const row = profileRows.nth(i);
const text = await row.textContent();
if (text?.includes(PROXY_PROFILE)) {
targetRow = row;
break;
}
}
expect(
targetRow,
`Could not find profile row for "${PROXY_PROFILE}"`,
).not.toBeNull();
await targetRow!.getByTestId("profile-menu-trigger").click();
await waitForTestId(page, "profile-actions-menu");
await page.getByTestId("profile-edit").click();
// Wait for the editor to load with the profile data
await expect(page.getByTestId("profile-name-input")).toHaveValue(
PROXY_PROFILE,
{ timeout: 10_000 },
);
});
// ── Ensure we are on the Basic tab, then save ──
await test.step("switch to Basic view and save", async () => {
// The litellm_proxy provider + proxy base_url is recognized as a
// known default, so the form may already be on Basic. Click the
// toggle to be explicit — this is the same user action the unit
// test for PR #1148 exercises.
const basicToggle = page.getByTestId("sdk-section-basic-toggle");
if (await basicToggle.isVisible().catch(() => false)) {
await basicToggle.click();
}
const saveButton = page.getByTestId("save-profile-btn");
await expect(saveButton).toBeEnabled({ timeout: 10_000 });
await saveButton.click();
// Wait for save to complete — the UI navigates back to the
// profile list after a successful save.
await waitForTestId(page, "add-llm-profile");
});
// ── Verify: the proxy setup survived the Basic-tab save ──
await test.step("verify proxy setup is preserved after save", async () => {
const config = await getProfileConfig(request, PROXY_PROFILE);
assertProxyProfileConfig(
config,
LITELLM_PROXY_MODEL,
OPENHANDS_EQUIVALENT_MODEL,
OPENHANDS_PROXY_BASE_URL,
);
});
// ── Verify: the profile also looks correct after a page reload ──
await test.step("profile survives page reload", async () => {
await page.reload({ waitUntil: "domcontentloaded" });
await waitForTestId(page, "add-llm-profile");
// Re-read via API to confirm persistence is durable
const config = await getProfileConfig(request, PROXY_PROFILE);
assertProxyProfileConfig(
config,
LITELLM_PROXY_MODEL,
OPENHANDS_EQUIVALENT_MODEL,
OPENHANDS_PROXY_BASE_URL,
);
});
});
});