settings: trust public OpenHands provider models (#1280)

* settings: trust public OpenHands provider models
* test: update OpenHands profile E2E expectation
* test: allow transitional OpenHands model shape in E2E
* test: update OpenHands provider profile E2E
* ci: pin mock e2e to SDK provider PR

Temporarily run mock-LLM E2E against OpenHands/software-agent-sdk#3548 so the provider settings refactor is validated with the matching SDK end state.

* test: stabilize automation trajectory for SDK pin

Use non-empty safety replies for the automation-run conversation so the mock trajectory completes whether the pinned SDK does or does not consume an internal LLM turn first.

* ci: isolate SDK PR pin to smoke test

Keep the full mock-LLM and Docker E2E suites on the released agent-server/automation stack, and add a targeted profile-management E2E job that runs against the SDK PR git ref.

* ci: remove temporary SDK PR pin

Revert the temporary OpenHands/software-agent-sdk#3548 git ref before switching the PR to the release-branch SDK pin.

* ci: pin SDK smoke to release PR

Point the temporary SDK smoke-test pin at OpenHands/software-agent-sdk#3638 (Release v1.28.0) so PR #1280 validates against the release branch hash.

Co-authored-by: openhands <openhands@all-hands.dev>

Remove the temporary SDK release-branch pin now that software-agent-sdk v1.28.0 is published, and run the mock E2E workflow against the released agent-server package.

* test: align agent server version references

Update docs and drift-detection expectations for the 1.28.0 agent-server pin.

---------

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Engel Nyst
2026-06-13 01:56:11 +00:00
committed by GitHub
co-authored by openhands
parent ee09546928
commit af9c653f3c
10 changed files with 208 additions and 162 deletions
@@ -6,7 +6,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { ModelSelector } from "#/components/shared/modals/settings/model-selector";
import { server } from "#/mocks/node";
describe("ModelSelector — OpenHands round-trip", () => {
describe("ModelSelector — OpenHands provider", () => {
let providersCount = 0;
let verifiedCount = 0;
let modelsCount = 0;
@@ -20,7 +20,9 @@ describe("ModelSelector — OpenHands round-trip", () => {
server.use(
http.get("*/api/llm/providers", () => {
providersCount += 1;
return HttpResponse.json({ providers: ["anthropic", "openai"] });
return HttpResponse.json({
providers: ["openhands", "anthropic", "openai"],
});
}),
http.get("*/api/llm/models/verified", () => {
verifiedCount += 1;
@@ -47,25 +49,13 @@ describe("ModelSelector — OpenHands round-trip", () => {
);
}
it("shows OpenHands as the provider for a persisted litellm_proxy/<m> + All-Hands proxy base URL, fetching each LLM endpoint exactly once", async () => {
// Arrange — mirrors the post-save state: the SDK rewrote openhands/<m>
// to litellm_proxy/<m> on disk and pinned the All-Hands proxy base URL.
renderWithQuery(
<ModelSelector
currentModel="litellm_proxy/claude-opus-4-7"
currentBaseUrl="https://llm-proxy.app.all-hands.dev/"
/>,
);
it("shows OpenHands as the provider for a public openhands/<m> model", async () => {
renderWithQuery(<ModelSelector currentModel="openhands/claude-opus-4-7" />);
// Act / Assert — the bootstrap effect must wait for the openhands
// verified list to load before resolving the displayed provider.
await waitFor(() => {
expect(screen.getByLabelText("LLM$PROVIDER")).toHaveValue("OpenHands");
});
// Assert — the three queries that all need verified-models data share
// a single cache entry, and the bootstrap effect picks the right
// provider on its first run, so /models is not fetched twice.
expect(providersCount).toBe(1);
expect(verifiedCount).toBe(1);
expect(modelsCount).toBe(1);
@@ -593,7 +593,10 @@ describe("LlmSettingsLocalView", () => {
});
describe("Basic tab save", () => {
it("drops hidden base_url values for OpenHands models", async () => {
it("drops stale base_url for OpenHands models in Basic mode", async () => {
// Arrange — a profile whose stored config pairs an OpenHands model with a
// stale base_url. In Basic mode the public provider prefix is enough; the
// SDK derives provider transport details when making LLM calls.
const user = userEvent.setup();
vi.mocked(ProfilesService.getProfile).mockResolvedValue({
name: "gpt-4-profile",
@@ -608,6 +611,7 @@ describe("LlmSettingsLocalView", () => {
renderWithProviders(<LlmSettingsLocalView />);
// Act — open the profile in edit mode, force the Basic tab, and save.
await user.click(screen.getAllByTestId("profile-menu-trigger")[0]);
await user.click(screen.getByTestId("profile-edit"));
await waitFor(() => {
@@ -621,11 +625,54 @@ describe("LlmSettingsLocalView", () => {
});
await user.click(screen.getByTestId("save-profile-btn"));
// Assert — the saved LLM config keeps the OpenHands model and drops the
// stale base_url instead of re-stamping a LiteLLM proxy detail.
await waitFor(() => expect(mockSaveMutateAsync).toHaveBeenCalled());
const savedLlm = mockSaveMutateAsync.mock.calls[0][0].request.llm;
expect(savedLlm.model).toBe("openhands/claude-opus-4-5-20251101");
expect(savedLlm).not.toHaveProperty("base_url");
});
it("drops base_url for stored litellm_proxy profiles in Basic mode", async () => {
// Arrange — legacy profiles may still contain a LiteLLM proxy model and
// proxy base_url. The SDK migration owns that compatibility; the Basic tab
// should not keep re-stamping transport details.
const user = userEvent.setup();
vi.mocked(ProfilesService.getProfile).mockResolvedValue({
name: "gpt-4-profile",
api_key_set: true,
config: {
model: "litellm_proxy/claude-opus-4-8",
api_key: "gAAAA_encrypted_key",
base_url: "https://llm-proxy.app.all-hands.dev/",
},
});
mockSaveMutateAsync.mockResolvedValueOnce({ success: true });
renderWithProviders(<LlmSettingsLocalView />);
// Act — open the profile in edit mode, force the Basic tab, and save
// without touching the model dropdown.
await user.click(screen.getAllByTestId("profile-menu-trigger")[0]);
await user.click(screen.getByTestId("profile-edit"));
await waitFor(() => {
expect(screen.getByTestId("profile-name-input")).toHaveValue(
"gpt-4-profile",
);
});
await user.click(await screen.findByTestId("sdk-section-basic-toggle"));
await waitFor(() => {
expect(screen.getByTestId("save-profile-btn")).not.toBeDisabled();
});
await user.click(screen.getByTestId("save-profile-btn"));
// Assert — the legacy model is preserved, but the Basic tab drops the
// base_url instead of reverse-mapping it in the frontend.
await waitFor(() => expect(mockSaveMutateAsync).toHaveBeenCalled());
const savedLlm = mockSaveMutateAsync.mock.calls[0][0].request.llm;
expect(savedLlm.model).toBe("litellm_proxy/claude-opus-4-8");
expect(savedLlm).not.toHaveProperty("base_url");
});
});
describe("All tab save", () => {
@@ -1,39 +0,0 @@
import { describe, expect, it } from "vitest";
import { normalizeDisplayModel } from "#/utils/normalize-display-model";
const OPENHANDS_VERIFIED = ["claude-opus-4-7", "gpt-5.5"];
const OPENHANDS_PROXY_BASE_URL = "https://llm-proxy.app.all-hands.dev/";
describe("normalizeDisplayModel", () => {
it("rewrites litellm_proxy/<m> to openhands/<m> when the base URL is the All-Hands proxy and the model is in the openhands verified list", () => {
// Arrange — mirrors the SDK round-trip: openhands/<m> on save becomes
// litellm_proxy/<m> on disk plus the All-Hands proxy base URL.
const persistedModel = "litellm_proxy/claude-opus-4-7";
// Act
const result = normalizeDisplayModel(
persistedModel,
OPENHANDS_PROXY_BASE_URL,
OPENHANDS_VERIFIED,
);
// Assert
expect(result).toBe("openhands/claude-opus-4-7");
});
it("leaves litellm_proxy/<m> untouched when the base URL is not the All-Hands proxy", () => {
// Arrange — a user-configured litellm_proxy pointed at a non-OpenHands
// gateway must not be re-labelled as OpenHands.
const persistedModel = "litellm_proxy/claude-opus-4-7";
// Act
const result = normalizeDisplayModel(
persistedModel,
"https://other-proxy.example.com/",
OPENHANDS_VERIFIED,
);
// Assert
expect(result).toBe("litellm_proxy/claude-opus-4-7");
});
});
-28
View File
@@ -1,28 +0,0 @@
import { describe, expect, it } from "vitest";
import {
OPENHANDS_LLM_PROXY_BASE_URL,
isOpenHandsProxyBaseUrl,
} from "#/utils/openhands-llm";
describe("openhands LLM helpers", () => {
it("exports the All-Hands LiteLLM proxy base URL", () => {
expect(OPENHANDS_LLM_PROXY_BASE_URL).toBe(
"https://llm-proxy.app.all-hands.dev/",
);
});
it("recognizes the All-Hands proxy base URL regardless of trailing slash or /v1", () => {
expect(isOpenHandsProxyBaseUrl(OPENHANDS_LLM_PROXY_BASE_URL)).toBe(true);
expect(isOpenHandsProxyBaseUrl("https://llm-proxy.app.all-hands.dev")).toBe(
true,
);
expect(
isOpenHandsProxyBaseUrl("https://llm-proxy.app.all-hands.dev/v1"),
).toBe(true);
expect(isOpenHandsProxyBaseUrl("https://other-proxy.example.com")).toBe(
false,
);
expect(isOpenHandsProxyBaseUrl(null)).toBe(false);
expect(isOpenHandsProxyBaseUrl(undefined)).toBe(false);
});
});
@@ -15,13 +15,10 @@ import { HelpLink } from "#/ui/help-link";
import { PRODUCT_URL } from "#/utils/constants";
import { useSearchProviders } from "#/hooks/query/use-search-providers";
import { useProviderModels } from "#/hooks/query/use-provider-models";
import { useOpenhandsVerifiedModels } from "#/hooks/query/use-openhands-verified-models";
import { normalizeDisplayModel } from "#/utils/normalize-display-model";
interface ModelSelectorProps {
isDisabled?: boolean;
currentModel?: string;
currentBaseUrl?: string;
onChange?: (provider: string | null, model: string | null) => void;
onDefaultValuesChanged?: (
provider: string | null,
@@ -34,7 +31,6 @@ interface ModelSelectorProps {
export function ModelSelector({
isDisabled,
currentModel,
currentBaseUrl,
onChange,
onDefaultValuesChanged,
wrapperClassName,
@@ -47,7 +43,6 @@ export function ModelSelector({
const [selectedModel, setSelectedModel] = React.useState<string | null>(null);
const { data: providers = [] } = useSearchProviders();
const { data: openhandsVerifiedModels } = useOpenhandsVerifiedModels();
const {
data: providerModels = [],
isLoading: isLoadingModels,
@@ -73,24 +68,15 @@ export function ModelSelector({
);
React.useEffect(() => {
// Wait for the openhands verified list before initializing — otherwise a
// persisted `litellm_proxy/<m>` model would first land as `litellm_proxy`
// and only later flip to `openhands`, triggering a redundant /api/llm/models
// fetch for the throwaway provider value.
if (currentModel && openhandsVerifiedModels !== undefined) {
const displayModel = normalizeDisplayModel(
currentModel,
currentBaseUrl,
openhandsVerifiedModels,
);
const { provider, model } = extractModelAndProvider(displayModel);
if (currentModel) {
const { provider, model } = extractModelAndProvider(currentModel);
setLitellmId(displayModel);
setLitellmId(currentModel);
setSelectedProvider(provider || null);
setSelectedModel(model);
onDefaultValuesChanged?.(provider || null, model);
}
}, [currentModel, currentBaseUrl, openhandsVerifiedModels]);
}, [currentModel]);
const handleChangeProvider = (provider: string) => {
setSelectedProvider(provider);
@@ -70,7 +70,6 @@ export function SettingsForm({ settings, onClose }: SettingsFormProps) {
const isLLMKeySet = settings.llm_api_key_set;
const currentModel = getAgentSettingValue(settings, "llm.model");
const currentBaseUrl = getAgentSettingValue(settings, "llm.base_url");
return (
<div>
@@ -85,9 +84,6 @@ export function SettingsForm({ settings, onClose }: SettingsFormProps) {
currentModel={
typeof currentModel === "string" ? currentModel : undefined
}
currentBaseUrl={
typeof currentBaseUrl === "string" ? currentBaseUrl : undefined
}
wrapperClassName="!flex-col !gap-[17px]"
labelClassName={SETTINGS_FORM.LABEL_CLASSNAME}
/>
-10
View File
@@ -33,7 +33,6 @@ import {
resolveLlmAuthType,
} from "#/constants/llm-subscription";
import { useOpenAISubscriptionModels } from "#/hooks/query/use-llm-subscription-models";
import { OPENHANDS_LLM_PROXY_BASE_URL } from "#/utils/openhands-llm";
const LLM_EXCLUDED_KEYS = new Set([
"llm.model",
@@ -58,14 +57,6 @@ const getSchemaFieldDefaultValue = (
const KNOWN_PROVIDER_DEFAULT_BASE_URLS: Partial<Record<string, Set<string>>> = {
openai: new Set(["https://api.openai.com", "https://api.openai.com/v1"]),
openhands: new Set([
OPENHANDS_LLM_PROXY_BASE_URL,
"https://llm-proxy.app.all-hands.dev/v1",
]),
litellm_proxy: new Set([
OPENHANDS_LLM_PROXY_BASE_URL,
"https://llm-proxy.app.all-hands.dev/v1",
]),
};
const normalizeBaseUrl = (baseUrl: string) => {
@@ -350,7 +341,6 @@ export function LlmSettingsScreen({
<>
<ModelSelector
currentModel={modelValue || undefined}
currentBaseUrl={baseUrlValue || undefined}
onChange={(provider, model) => {
const nextModel = buildModelId(provider, model);
if (nextModel) {
-24
View File
@@ -1,24 +0,0 @@
import { isOpenHandsProxyBaseUrl } from "#/utils/openhands-llm";
const LITELLM_PROXY_PREFIX = "litellm_proxy/";
/**
* Reverse the SDK's `openhands/* -> litellm_proxy/*` rewrite for display.
*
* The agent-server's LLM validator stores curated OpenHands models as
* `litellm_proxy/<m>` against the All-Hands proxy base URL. Without
* normalization the GUI re-loads such settings as `litellm_proxy`, so the
* provider dropdown silently switches off "OpenHands" after every save.
*/
export function normalizeDisplayModel(
rawModel: string | null | undefined,
baseUrl: string | null | undefined,
openhandsVerifiedModels: readonly string[],
): string {
if (!rawModel) return "";
if (!rawModel.startsWith(LITELLM_PROXY_PREFIX)) return rawModel;
if (!isOpenHandsProxyBaseUrl(baseUrl)) return rawModel;
const modelName = rawModel.slice(LITELLM_PROXY_PREFIX.length);
if (!openhandsVerifiedModels.includes(modelName)) return rawModel;
return `openhands/${modelName}`;
}
-22
View File
@@ -1,22 +0,0 @@
export const OPENHANDS_LLM_PROXY_BASE_URL =
"https://llm-proxy.app.all-hands.dev/";
// Accepted spellings of the All-Hands LiteLLM proxy base URL, normalized
// without a trailing slash. The agent-server stores curated OpenHands models
// against one of these once its validator rewrites `openhands/*` to
// `litellm_proxy/*`.
const OPENHANDS_LLM_PROXY_BASE_URLS = new Set([
"https://llm-proxy.app.all-hands.dev",
"https://llm-proxy.app.all-hands.dev/v1",
]);
/**
* True when `baseUrl` points at the All-Hands LiteLLM proxy, ignoring any
* trailing slash.
*/
export function isOpenHandsProxyBaseUrl(baseUrl: unknown): baseUrl is string {
return (
typeof baseUrl === "string" &&
OPENHANDS_LLM_PROXY_BASE_URLS.has(baseUrl.trim().replace(/\/+$/, ""))
);
}
@@ -16,10 +16,18 @@
* 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. OpenHands provider base_url normalization:
* The public `openhands/` model namespace is enough to preserve
* profile identity. Re-saving an OpenHands profile from the Basic tab
* must drop stale LiteLLM proxy base_url details so the SDK owns
* transport-time mapping.
*/
import { test, expect } from "@playwright/test";
import {
BACKEND_URL,
SESSION_API_KEY,
seedLocalStorage,
routeSessionApiKey,
dismissAnalyticsModal,
@@ -39,6 +47,28 @@ 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" });
// ═══════════════════════════════════════════════════════════════════════
@@ -328,3 +358,123 @@ test.describe("same-model profile identity", () => {
});
});
});
// ═══════════════════════════════════════════════════════════════════════
// Test 3 — OpenHands provider base_url normalization
// ═══════════════════════════════════════════════════════════════════════
test.describe("OpenHands provider base_url normalization", () => {
// The public OpenHands provider model is the target persisted identity. Older
// agent-server releases may still report the transitional litellm_proxy form,
// but Basic-mode saves should not continue stamping proxy transport details
// in either case.
const OPENHANDS_PROFILE = "openhands-basic-save-test";
const OPENHANDS_MODEL = "openhands/claude-opus-4-5-20251101";
const LEGACY_TRANSPORT_MODEL = "litellm_proxy/claude-opus-4-5-20251101";
const EXPECTED_OPENHANDS_MODELS = [OPENHANDS_MODEL, LEGACY_TRANSPORT_MODEL];
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, OPENHANDS_PROFILE);
} catch {
// best-effort
} finally {
await page.close();
}
});
test("re-saving an OpenHands profile from Basic view drops proxy base_url", async ({
page,
request,
}) => {
// ── Setup: create a public OpenHands profile with a stale proxy base_url
// through the Settings UI. This mirrors legacy/manual profile state while
// keeping the public openhands/* model namespace that this PR trusts. ──
await routeSessionApiKey(page);
await page.goto("/settings/llm", { waitUntil: "domcontentloaded" });
await dismissAnalyticsModal(page);
await waitForTestId(page, "add-llm-profile");
await deleteProfileIfExists(page, OPENHANDS_PROFILE);
await createProfileViaUI(page, {
profileName: OPENHANDS_PROFILE,
model: OPENHANDS_MODEL,
baseUrl: OPENHANDS_PROXY_BASE_URL,
});
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(OPENHANDS_PROFILE)) {
targetRow = row;
break;
}
}
expect(
targetRow,
`Could not find profile row for "${OPENHANDS_PROFILE}"`,
).not.toBeNull();
await targetRow!.getByTestId("profile-menu-trigger").click();
await waitForTestId(page, "profile-actions-menu");
await page.getByTestId("profile-edit").click();
await expect(page.getByTestId("profile-name-input")).toHaveValue(
OPENHANDS_PROFILE,
{ timeout: 10_000 },
);
});
await test.step("switch to Basic view and save", async () => {
// Click the Basic toggle explicitly so the save path exercises the view
// that hides base_url and therefore drops stale provider transport data.
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();
await waitForTestId(page, "add-llm-profile");
});
// ── Verify: Basic-tab save removed the stale proxy base_url ──
await test.step("verify base_url is dropped after save", async () => {
const config = await getProfileConfig(request, OPENHANDS_PROFILE);
expect(
config.base_url ?? null,
"base_url should not be persisted for public OpenHands models after " +
"a Basic-tab re-save; the SDK owns transport-time proxy mapping",
).toBeNull();
expect(EXPECTED_OPENHANDS_MODELS).toContain(config.model);
});
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, OPENHANDS_PROFILE);
expect(config.base_url ?? null).toBeNull();
expect(EXPECTED_OPENHANDS_MODELS).toContain(config.model);
});
});
});