Skip to content

feat: make an ENABLE_LITELLM=false install usable out of the box - #538

Open
dylan-openhands wants to merge 5 commits into
enable-litellm-flag-modelsfrom
enable-litellm-flag-off-mode
Open

dylan-openhands wants to merge 5 commits into
enable-litellm-flag-modelsfrom
enable-litellm-flag-off-mode

Conversation

@dylan-openhands

@dylan-openhands dylan-openhands commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

HUMAN:

Makes an install with LiteLLM off work from the first login: BYOK is forced on, the install-time provider key becomes the default model, the managed-LLM-key section on the API Keys page is hidden, and orgs still on the LiteLLM default are moved to the install default.

  • A human has tested these changes.

AGENT:


Why

With ENABLE_LITELLM=false, a fresh install had no working LLM path: BYOK could stay off, new orgs defaulted to a litellm_proxy/ model on the gateway URL, and on Replicated installs the web client never saw the flag. Top of the stack #519 → #520 → #521 → this PR.

Summary

  • Resolve the web-client enable_litellm from the ENABLE_LITELLM env at request time. Replicated sets OH_WEB_CLIENT_FEATURE_FLAGS_* vars, which build the flags without reading ENABLE_LITELLM, so it stayed true and Budgets stayed visible.
  • Force BYOK on when LiteLLM is off, and let the direct install default (OPENHANDS_LLM_PROVIDER_ROUTE=direct) work without a base URL. With LiteLLM off and no install default, new orgs and users get the SDK default model instead of a gateway model.
  • Settings → API Keys hides the managed LLM key section and skips its fetch when off.
  • With LiteLLM off there is no OpenHands-managed default model. Migration 160 seeds openhands/deepseek-v4-flash as the verified-models default on every install, and SaasSettingsStore.load() made it every member's active "Default" profile, overriding the install default even on a fresh all-off install.
  • Orgs still on the LiteLLM default (e.g. created while LiteLLM was on) move to the deployment default on their next load, reusing the lazy version-bump repair in OrgStore._validate_org_version. The install key is stored as the org key, which outranks members' dead LiteLLM keys. BYOK orgs are untouched; a member who saved a managed model in their own settings still has to pick a new one.

Issue Number

How to Test

  • uv run pytest tests/unit/server/test_constants.py tests/unit/app_server/test_default_web_client_config_injector.py tests/unit/test_verified_model/ tests/unit/test_org_store.py tests/unit/test_saas_settings_store.py tests/unit/storage/test_org_app_settings_store.py
  • cd frontend && npm run test -- --run __tests__/components/features/settings/api-keys-manager.test.tsx __tests__/hooks/query/use-llm-api-key.test.tsx
  • End to end with feat: add an install-time switch to run without LiteLLM OpenHands-Cloud#1301: fresh install with "Enable LiteLLM gateway" off and an Anthropic key. New users get anthropic/<first model> preselected with the key filled in, BYOK is available, Budgets and the managed LLM key are hidden, and no LiteLLM pod runs.

Video/Screenshots

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • Off → on is out of scope: no LiteLLM provisioning for users created while off.
  • _uses_managed_default_llm now also treats litellm_proxy/* on an in-cluster (or empty) base URL as managed, so a version bump with LiteLLM on also re-points such orgs at the current default.

Enterprise server image for this PR:

ghcr.io/openhands/enterprise-server:sha-a7ce137

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  openhands/app_server/web_client
  default_web_client_config_injector.py
  server
  constants.py
  server/verified_models
  default_profile.py
  litellm_proxy_model_router.py
  storage
  org_app_settings_store.py
  org_store.py
Project Total  

This report was generated by python-coverage-comment-action

@dylan-openhands
dylan-openhands force-pushed the enable-litellm-flag-off-mode branch from be2813c to e225546 Compare September 26, 2026 22:42
@dylan-openhands
dylan-openhands marked this pull request as ready for review September 29, 2026 14:25
@aivong-openhands

Copy link
Copy Markdown
Contributor

Mutation review of this PR's tests

I hand-wrote targeted mutants against the five test files this PR adds and ran each against a pristine copy of the branch (gh pr diff scope only). Green baselines: test_constants.py 77, test_default_web_client_config_injector.py 89, use-llm-api-key.test.tsx 4, api-keys-manager.test.tsx 7, test_org_store.py (gateway subset) 8. This is about test strength, not correctness — every fix below was verified to pass on the branch and fail against its mutant.

Controls (must die) — these prove the suites work

Mutant Result
constants: re-require base_url for the direct route (revert the core fix) ❌ caught
constants: drop the LiteLLM-off model branch (fall back to gateway model) ❌ caught
constants: drop the LiteLLM-off base_url branch (return gateway URL) ❌ caught
injector: stop forcing BYOK on when LiteLLM is off ❌ caught
injector: stop re-reading ENABLE_LITELLM for the structured-flags path ❌ caught
hook: drop the litellmEnabled gate on the byor query ❌ caught
hook: default enable_litellm to false when unset ❌ caught
component: default enable_litellm to false when unset ❌ caught
org_store: drop the gateway-off repair trigger ❌ caught
org_store: stop sharing the install key onto the repaired org ❌ caught

Credit where it's due: the backend suites are genuinely strong. test_constants.py asserts the exact returned model/base_url for every route×flag combination (so reverting the fix in either direction dies), and the org_store DB tests assert on the reloaded org's agent_settings['llm'] and llm_api_key.get_secret_value() after a real get_org_by_id, plus a second load with _update_org_kwargs wrapped to prove idempotency — that combination kills the "repair on every load" and "run repair even when the gateway is on" mutants outright.

One control that did not die — the component's central claim is unasserted

Mutant Result
component: always render the managed LLM key section (revert the litellmEnabled && gate) ✅ SURVIVED

api-keys-manager.test.tsx → "renders no managed LLM key section when enable_litellm is off" is the one test asserting the PR's headline behaviour, but it passes whether or not the section is hidden. It stubs useLlmApiKey with data: undefined, and LlmApiKeyManager early-returns with no refresh button when llmApiKey is falsy (api-keys-manager.tsx line 111). So queryByRole("button", { name: "SETTINGS$REFRESH_LLM_API_KEY" }) is absent even when the gate is removed and the section renders — the test cannot tell "hidden" from "rendered but empty."

Fix (one line — give it a key like the sibling tests, so the button would appear if the section rendered):

     mockUseLlmApiKey.mockReturnValue({
-      data: undefined,
+      data: { key: "sk-byor-key" },
       error: null,
       isLoading: false,
       isPaymentRequired: false,
     } as never);

Verified: with this change the "always render" mutant dies, and the other two component mutants stay dead.

Survivors (gaps)

Mutant Result
hook: gate only the self-hosted (oss) branch, leave the SaaS branch fetching when LiteLLM is off ✅ SURVIVED
constants: ignore OPENHANDS_LLM_PROVIDER_ROUTE (treat any route with a model as direct) ✅ SURVIVED
injector: ignore the structured feature_flags.enable_litellm operand (honor only the env var) ✅ SURVIVED
org_store: _is_in_cluster_url no longer recognizes bare .svc service DNS ✅ SURVIVED
org_store: _deployment_default_llm shares the install key even when the gateway is on ✅ SURVIVED (likely equivalent — see below)

1. The SaaS branch of useLlmApiKey is never tested with LiteLLM off

All four hook tests use app_mode: "oss". Moving litellmEnabled so it only guards the self-hosted branch leaves the SaaS branch (app_mode === "saas" ? !!organizationId : ...) still firing the byor request when LiteLLM is off. That path is real — self-hosted enterprise runs app_mode: "saas" too, and per your own comment the byor endpoints 403 outright when LiteLLM is off, so the query would 403-loop on those installs.

it("does not fetch on SaaS with an org selected when enable_litellm is off", async () => {
  const defaultConfig = createMockWebClientConfig();
  const configSpy = vi.spyOn(OptionService, "getConfig").mockResolvedValue(
    createMockWebClientConfig({
      app_mode: "saas",
      feature_flags: {
        ...defaultConfig.feature_flags,
        enable_byor_export: true,
        enable_litellm: false,
      },
    }),
  );
  const getSpy = vi.spyOn(openHands, "get");
  useSelectedOrganizationStore.setState({ organizationId: "org-123" });

  const { result } = renderHook(() => useLlmApiKey(), { wrapper: createWrapper() });

  await waitFor(() => expect(configSpy).toHaveBeenCalled());
  await waitFor(() =>
    expect(queryClient.getQueryState(["web-client-config"])?.status).toBe("success"),
  );

  expect(getSpy).not.toHaveBeenCalled();
  expect(result.current.data).toBeUndefined();
});

Verified: passes on the branch, fails against the "gate only the oss branch" mutant.

2. should_use_direct_llm_defaults doesn't pin the route check

Every test that exercises the route uses route='direct' (or route=None with model=None). Replacing OPENHANDS_LLM_PROVIDER_ROUTE == 'direct' with True — i.e. treating any route as direct as long as a model is configured — survives. In production this means a gateway-routed deployment that also has OPENHANDS_DEFAULT_LLM_MODEL set would silently bypass the gateway and hand the raw model + configured base_url to users.

def test_non_direct_route_with_model_still_uses_gateway(self, constants):
    p = self._patch(
        constants,
        route='gateway',
        model='anthropic/claude-x',
        base_url=None,
        litellm=True,
    )
    with p[0], p[1], p[2], p[3], p[4], p[5]:
        assert constants.should_use_direct_llm_defaults() is False
        assert constants.get_default_llm_model() == 'litellm_proxy/m'
        assert constants.get_default_llm_base_url() == 'http://litellm:4000'

Verified: passes on the branch, fails against the == 'direct' → True mutant. (Uses the same _patch helper as TestDefaultLlmSettings.)

3. The injector's structured-flags operand is one-sided

test_..._resolves_litellm_from_structured_env proves the env-var half of self.feature_flags.enable_litellm and _env_flag_enabled('ENABLE_LITELLM', 'true') (structured flags default on, plain ENABLE_LITELLM=false still disables). But replacing self.feature_flags.enable_litellm with True survives — nothing asserts the symmetric case where a structured OH_WEB_CLIENT_FEATURE_FLAGS_ENABLE_LITELLM=false disables LiteLLM while the plain env var is on/unset.

@pytest.mark.asyncio
async def test_structured_flag_off_disables_litellm_even_when_env_on(self):
    from openhands.agent_server.env_parser import from_env
    from openhands.app_server.types import AppMode
    from openhands.app_server.web_client.default_web_client_config_injector import (
        DefaultWebClientConfigInjector,
    )

    class _FakeService:
        async def resolve(self, key):
            return False

    class _FakeGlobalConfig:
        app_mode = AppMode.SAAS

    with (
        patch(
            'openhands.app_server.config.get_global_config',
            return_value=_FakeGlobalConfig(),
        ),
        patch.dict(
            os.environ,
            {
                'ENABLE_LITELLM': 'true',
                'OH_WEB_CLIENT_FEATURE_FLAGS_ENABLE_LITELLM': 'false',
                'OH_WEB_CLIENT_FEATURE_FLAGS_ALLOW_USER_LLM_CONFIGURATION': 'false',
            },
        ),
        self._fake_service_module(_FakeService()),
    ):
        injector = from_env(DefaultWebClientConfigInjector, 'OH_WEB_CLIENT')
        config = await injector.get_web_client_config()
    assert config.feature_flags.enable_litellm is False
    assert config.feature_flags.allow_user_llm_configuration is True

Verified: passes on the branch, fails against the True and _env_flag_enabled(...) mutant.

4. _is_in_cluster_url doesn't cover bare .svc service DNS

The parametrized cases cover None, a dotless host (openhands-litellm), and a fully-qualified ...svc.cluster.local host. Dropping .svc from the endswith(('.svc', '.cluster.local')) tuple survives, because no case uses the bare service.namespace.svc form — which Kubernetes resolves in-cluster just like the FQDN. An org whose gateway-era base_url used that short form would be misclassified as external/BYOK and never repaired, leaving it stuck on the dead proxy.

Fix — add one param to test_bundled_proxy_route_is_managed_without_its_url_env:

    @pytest.mark.parametrize(
        'proxy_url',
        [
            None,
            'http://openhands-litellm:4000',
            'http://oh-main-litellm.openhands.svc:4000',
            'http://oh-main-litellm.openhands.svc.cluster.local:4000',
        ],
    )

Verified: passes on the branch, fails against the ('.cluster.local',) mutant.

Not a test gap

  • _deployment_default_llm gateway-on key guard (survivor build(deps): bump docker/build-push-action from 7.1.0 to 7.3.0 #5). Dropping the None if gateway_enabled guard survives, but I think this is an equivalent mutant for any realistic deployment: get_default_llm_api_key() only returns a key when should_use_direct_llm_defaults() is true (direct route + model), and a pure gateway install isn't on the direct route, so both branches yield None. It's only observable in a contradictory "gateway on and direct route with a key" config, and it's reached only via the org_version < ORG_SETTINGS_VERSION upgrade path (not the gateway-off repair path these tests drive). Either add an upgrade-path test with a direct key configured, or treat the guard as defensive — your call.
  • The redundant and OPENHANDS_DEFAULT_LLM_MODEL in get_default_llm_model is also an equivalent mutant: should_use_direct_llm_defaults() already guarantees the model is truthy, so deleting it changes nothing.

This comment was generated by an AI assistant on behalf of the user.

@dylan-openhands
dylan-openhands force-pushed the enable-litellm-flag-off-mode branch from e225546 to 82b2a3f Compare October 1, 2026 19:37
@dylan-openhands
dylan-openhands force-pushed the enable-litellm-flag-off-mode branch from 82b2a3f to cb86ba8 Compare October 5, 2026 01:55

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants